Control UI: refresh slash commands from runtime command list#65620
Conversation
- Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable
openclaw-barnacle
Bot
added
app: web-ui
Greptile SummaryReplaces the static built-in slash-command list in the Control UI composer and command palette with a live Confidence Score: 5/5Safe to merge; only a P2 concurrency refinement remains. All correctness, fallback, and dedup logic is sound. The one open item is a stale-response race in refreshSlashCommands (no sequence guard), which is P2: it self-corrects on the next refresh and requires concurrent calls to manifest. ui/src/ui/chat/slash-commands.ts — refreshSlashCommands lacks a concurrency guard Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/ui/chat/slash-commands.ts
Line: 295-313
Comment:
**No concurrency guard against stale responses**
`SLASH_COMMANDS` is a shared mutable array. If `refreshChat` is called twice in quick succession (e.g., rapid agent switch or a reconnect event while the first request is still in-flight), both awaits resolve independently and whichever finishes *last* wins — which may be the older request's result. The fix is a simple sequence counter:
```suggestion
let _refreshSeq = 0;
export async function refreshSlashCommands(params: {
client: GatewayBrowserClient | null;
agentId?: string | null;
}): Promise<void> {
const seq = ++_refreshSeq;
const agentId = params.agentId?.trim();
if (!params.client || !agentId) {
replaceSlashCommands(buildFallbackSlashCommands());
return;
}
try {
const result = await params.client.request<CommandsListResult>("commands.list", {
agentId,
includeArgs: true,
scope: "text",
});
if (seq !== _refreshSeq) return;
replaceSlashCommands(buildSlashCommandsFromEntries(result?.commands ?? []));
} catch {
if (seq !== _refreshSeq) return;
replaceSlashCommands(buildFallbackSlashCommands());
}
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Refresh slash commands from runtime comm..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18d2dae776
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Addressed the Aisle security findings and the stale-refresh note in What changed:
Coverage added/updated:
Verification rerun after the fix:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a442db99ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up security fix landed in Additional hardening beyond the earlier command-trust fixes:
Verification after the update:
|
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Client-side DoS via unbounded processing of gateway-provided command arrays
DescriptionThe UI builds slash-command completion entries from the Observed issues:
Even though later code slices the number of commands/args/choices used, the initial mapping already incurs O(n) time and allocates intermediate arrays for the full payload. Vulnerable code (examples): return commands
.map((entry) => asRecord(entry))
.filter((entry): entry is CommandEntry => entry !== null);return rawArgs
.map((arg) => asRecord(arg))
.filter((arg): arg is Record<string, unknown> => arg !== null);RecommendationEnforce bounds before mapping/filtering to avoid work proportional to attacker-controlled payload sizes. Example fix: function getRemoteCommandEntries(result: CommandsListResult | null | undefined): CommandEntry[] {
const commands = result?.commands;
if (!Array.isArray(commands)) return [];
return commands
.slice(0, MAX_REMOTE_COMMANDS) // bound first
.map((entry) => asRecord(entry))
.filter((entry): entry is CommandEntry => entry !== null);
}
function getEntryArgs(entry: CommandEntry | Record<string, unknown>): Array<Record<string, unknown>> {
const rawArgs = "args" in entry ? entry.args : undefined;
if (!Array.isArray(rawArgs)) return [];
return rawArgs
.slice(0, MAX_REMOTE_ARGS) // bound first
.map((arg) => asRecord(arg))
.filter((arg): arg is Record<string, unknown> => arg !== null);
}
function getArgChoices(arg: Record<string, unknown>): LocalArgChoice[] {
if (arg.dynamic === true) return [];
const rawChoices = arg.choices;
if (!Array.isArray(rawChoices)) return [];
return rawChoices
.slice(0, MAX_REMOTE_CHOICES) // bound first
.map(/* ... */)
.filter(/* ... */);
}Additionally, consider adding a maximum WebSocket frame/message size and/or schema validation at the transport layer in Analyzed PR: #65620 at commit Last updated on: 2026-04-13T01:39:40Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1982d22ae6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| COMMAND_NAME_MAX_LENGTH, | ||
| ), | ||
| ...(nativeName ? { nativeName: clampString(nativeName, COMMAND_NAME_MAX_LENGTH) } : {}), | ||
| textAliases: [`/${clampString(textSpec.name, COMMAND_NAME_MAX_LENGTH)}`], |
There was a problem hiding this comment.
buildPluginCommandEntries clamps textSpec.name to COMMAND_NAME_MAX_LENGTH and then prepends /, so a plugin command name of length 200 still produces a 201-character alias. That exceeds the new CommandEntrySchema bound for textAliases and breaks the stated payload-size guarantee for commands.list in cases with long plugin command names. This can surface as schema-validation failures for strict clients consuming the protocol.
Useful? React with 👍 / 👎.
|
Addressed the malformed-payload security check in What changed:
Coverage:
Verification rerun:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b82728e810
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await refreshSlashCommands({ | ||
| client: host.client, | ||
| agentId: resolveAgentIdForSession(host), | ||
| }); |
There was a problem hiding this comment.
refreshChat now always calls refreshSlashCommands with resolveAgentIdForSession(host), and that resolver falls back to "main" when the session key is plain and snapshot.sessionDefaults is missing. Because sessionDefaults is optional in the gateway snapshot contract, this path can still happen (for older/partial snapshots), and in multi-agent configs where main is not a valid/default agent it makes commands.list fail and silently fall back to built-ins, so runtime dock/plugin/skill commands disappear right after chat refresh.
Useful? React with 👍 / 👎.
…w#65620) * Refresh slash commands from runtime command list - Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Control UI: harden runtime slash command discovery * Control UI: bound runtime slash command payloads * Control UI: use default agent for plain session keys * Control UI: guard malformed slash command payloads --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…w#65620) * Refresh slash commands from runtime command list - Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Control UI: harden runtime slash command discovery * Control UI: bound runtime slash command payloads * Control UI: use default agent for plain session keys * Control UI: guard malformed slash command payloads --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…w#65620) * Refresh slash commands from runtime command list - Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Control UI: harden runtime slash command discovery * Control UI: bound runtime slash command payloads * Control UI: use default agent for plain session keys * Control UI: guard malformed slash command payloads --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…w#65620) * Refresh slash commands from runtime command list - Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Control UI: harden runtime slash command discovery * Control UI: bound runtime slash command payloads * Control UI: use default agent for plain session keys * Control UI: guard malformed slash command payloads --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…w#65620) * Refresh slash commands from runtime command list - Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Control UI: harden runtime slash command discovery * Control UI: bound runtime slash command payloads * Control UI: use default agent for plain session keys * Control UI: guard malformed slash command payloads --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…w#65620) * Refresh slash commands from runtime command list - Load live slash commands into the chat UI and command palette - Keep builtin fallback behavior when runtime commands are unavailable * Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Control UI: harden runtime slash command discovery * Control UI: bound runtime slash command payloads * Control UI: use default agent for plain session keys * Control UI: guard malformed slash command payloads --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
1 participant
Summary
Describe the problem and fix in 2–5 bullets:
commands.listfor the active agent, keeps local UI-only commands layered on top, and reuses that live list in the command palette.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write
N/A. If the cause is unclear, writeUnknown.buildBuiltinChatCommands()directly instead of the gateway’s livecommands.listsurface, so runtime-generated dock commands, plugin commands, and skill aliases never reached the chat UI.Regression Test Plan (if applicable)
For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write
N/A.ui/src/ui/chat/slash-commands.node.test.ts,ui/src/ui/views/command-palette.test.tscommands.listshould surface runtime dock/plugin/skill commands in both the composer slash menu source and the command palette.Post-review Security Fixes
commands.listdata.executeLocalby name collision.commands.listpayloads are now bounded on both the gateway and Control UI sides so oversized command catalogs, aliases, args, choices, or descriptions are truncated before they can pressure the UI.User-visible / Behavior Changes
List user-visible changes (including defaults/config).
If none, write
None.commands.listentries cannot mark themselves local, cannot shadow reserved local UI commands, and must pass strict slash-identifier validation before appearing in the UI.Diagram (if applicable)
For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write
N/A.Security Impact (required)
Yes/No) NoYes/No) NoYes/No) YesYes/No) NoYes/No) NoYes, explain risk + mitigation: the UI now calls the existing authenticated gateway methodcommands.list; post-review hardening now keeps local command classification trusted-only, rejects unsafe remote slash identifiers, prevents remote collisions with reserved local commands, bounds command payload size on both the gateway and Control UI sides, and ignores stale refresh responses before they can overwrite a newer command set.Repro + Verification
Environment
Steps
/in chat or open the command palette.Expected
commands.list.Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.commands.listmay temporarily fail or return a narrower set than expected for the active agent.