fix(tui): autocomplete plugin commands like active-memory#73984
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 12:57 AM ET / 04:57 UTC. Summary PR surface: Source +156, Tests +121. Total +277 across 12 files. Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Review detailsBest possible solution: Retry the Codex review after fixing the execution failure. Do we have a high-confidence way to reproduce the issue? Unclear. The review failed before ClawSweeper could establish a reproduction path. Is this the best way to solve the issue? Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model gpt-5.5, reasoning high; reviewed against b9933b2ec119. Label changesLabel justifications:
Evidence reviewedPR surface: Source +156, Tests +121. Total +277 across 12 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
Greptile SummaryThis PR introduces
Confidence Score: 3/5Safe to merge for the happy path, but the concurrent-refresh race can leave plugin commands stale after rapid agent/model switching. One P1 concurrency defect (forced refresh defers to in-flight, never issues a catch-up fetch for the new context) caps the score at 4; the additional stale-closure concern on the cache key pulls it to 3. Both are in newly added TUI logic not covered by the existing tests for the race scenario. src/tui/tui.ts — refreshRemoteCommands function (lines 583–627) Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/tui/tui.ts
Line: 591-594
Comment:
**Stale autocomplete after concurrent forced refresh**
When `force=true` arrives while another fetch is already in flight, the function awaits the existing promise and returns without issuing a new fetch for the *current* context. After the in-flight completes, `remoteCommandsCacheKey` is set to the *old* context key. Because the non-forced path then sees the key as a cache hit, the wrong command set persists until the next agent/model/session change.
A realistic trigger: `/agent Work` starts a fetch tagged `work:openai`; the user immediately runs `/model anthropic/<model>` which calls `refreshRemoteCommands(true)` for `work:anthropic`. That second call defers to the first in-flight and returns — provider-specific plugin commands are never fetched.
The fix is to re-check the context key after the awaited in-flight resolves, and recurse if it has drifted from `remoteCommandsCacheKey`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/tui/tui.ts
Line: 613
Comment:
**`refreshKey` closure captures pre-fetch context**
`refreshKey` is captured before the `await client.listCommands` call at line 587, so if `currentAgentId` or `sessionInfo.modelProvider` changes during the fetch (via an external session event), `remoteCommandsCacheKey` will be set to the stale key even though `seq === remoteCommandsRefreshSeq`. Consider re-computing the cache key from the live state at completion time, or ensure the seq guard is sufficient to invalidate it.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/tui/tui-backend.ts
Line: 116
Comment:
**`listCommands` is optional on the interface but always called without a null-check in tests**
Marking `listCommands?` as optional is fine for backward-compat, but the call sites in `tui.ts` guard with `typeof client.listCommands !== "function"` while `tui-command-handlers.test.ts` always provides the mock. Consider documenting that implementations should provide the method, or make the guard explicit in all call sites to avoid a silent no-op when a new backend type omits it.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(tui): autocomplete plugin commands l..." | Re-trigger Greptile |
| if (remoteCommandsRefreshInFlight) { | ||
| await remoteCommandsRefreshInFlight; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Stale autocomplete after concurrent forced refresh
When force=true arrives while another fetch is already in flight, the function awaits the existing promise and returns without issuing a new fetch for the current context. After the in-flight completes, remoteCommandsCacheKey is set to the old context key. Because the non-forced path then sees the key as a cache hit, the wrong command set persists until the next agent/model/session change.
A realistic trigger: /agent Work starts a fetch tagged work:openai; the user immediately runs /model anthropic/<model> which calls refreshRemoteCommands(true) for work:anthropic. That second call defers to the first in-flight and returns — provider-specific plugin commands are never fetched.
The fix is to re-check the context key after the awaited in-flight resolves, and recurse if it has drifted from remoteCommandsCacheKey.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/tui/tui.ts
Line: 591-594
Comment:
**Stale autocomplete after concurrent forced refresh**
When `force=true` arrives while another fetch is already in flight, the function awaits the existing promise and returns without issuing a new fetch for the *current* context. After the in-flight completes, `remoteCommandsCacheKey` is set to the *old* context key. Because the non-forced path then sees the key as a cache hit, the wrong command set persists until the next agent/model/session change.
A realistic trigger: `/agent Work` starts a fetch tagged `work:openai`; the user immediately runs `/model anthropic/<model>` which calls `refreshRemoteCommands(true)` for `work:anthropic`. That second call defers to the first in-flight and returns — provider-specific plugin commands are never fetched.
The fix is to re-check the context key after the awaited in-flight resolves, and recurse if it has drifted from `remoteCommandsCacheKey`.
How can I resolve this? If you propose a fix, please make it concise.| })) | ||
| .filter((command) => Boolean(command.name)) | ||
| : []; | ||
| remoteCommandsCacheKey = refreshKey; |
There was a problem hiding this comment.
refreshKey closure captures pre-fetch context
refreshKey is captured before the await client.listCommands call at line 587, so if currentAgentId or sessionInfo.modelProvider changes during the fetch (via an external session event), remoteCommandsCacheKey will be set to the stale key even though seq === remoteCommandsRefreshSeq. Consider re-computing the cache key from the live state at completion time, or ensure the seq guard is sufficient to invalidate it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/tui/tui.ts
Line: 613
Comment:
**`refreshKey` closure captures pre-fetch context**
`refreshKey` is captured before the `await client.listCommands` call at line 587, so if `currentAgentId` or `sessionInfo.modelProvider` changes during the fetch (via an external session event), `remoteCommandsCacheKey` will be set to the stale key even though `seq === remoteCommandsRefreshSeq`. Consider re-computing the cache key from the live state at completion time, or ensure the seq guard is sufficient to invalidate it.
How can I resolve this? If you propose a fix, please make it concise.| @@ -107,4 +112,5 @@ export type TuiBackend = { | |||
| resetSession: (key: string, reason?: "new" | "reset") => Promise<unknown>; | |||
| getGatewayStatus: () => Promise<unknown>; | |||
| listModels: () => Promise<TuiModelChoice[]>; | |||
| listCommands?: (opts?: { agentId?: string; provider?: string }) => Promise<TuiCommandChoice[]>; | |||
| }; | |||
There was a problem hiding this comment.
listCommands is optional on the interface but always called without a null-check in tests
Marking listCommands? as optional is fine for backward-compat, but the call sites in tui.ts guard with typeof client.listCommands !== "function" while tui-command-handlers.test.ts always provides the mock. Consider documenting that implementations should provide the method, or make the guard explicit in all call sites to avoid a silent no-op when a new backend type omits it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/tui/tui-backend.ts
Line: 116
Comment:
**`listCommands` is optional on the interface but always called without a null-check in tests**
Marking `listCommands?` as optional is fine for backward-compat, but the call sites in `tui.ts` guard with `typeof client.listCommands !== "function"` while `tui-command-handlers.test.ts` always provides the mock. Consider documenting that implementations should provide the method, or make the guard explicit in all call sites to avoid a silent no-op when a new backend type omits it.
How can I resolve this? If you propose a fix, please make it concise.7719365 to
69feb6c
Compare
|
Could a maintainer please rerun the failed checks for this PR? I don’t have permission to rerun jobs in this repository. check-lint: failing on existing lint issues in other files outside this PR’s touched surface. |
Merge remote plugin/active-memory command entries into TUI slash autocomplete and refresh them on agent/model/session context changes while preserving local argument completion behavior on name collisions. Made-with: Cursor
Re-check command refresh context after awaiting an in-flight request and pin request agent/provider values to avoid dropping forced refreshes during rapid agent/model switches. Made-with: Cursor
Replace the remaining getSlashCommands test call with getBuiltinSlashCommands to match the renamed API and avoid lint/test-types failures. Made-with: Cursor
90d080d to
66217f6
Compare
## Summary - Problem: gateway-connected TUI slash autocomplete used its local/static command list, so plugin commands already exposed by the running Gateway through `commands.list` were invisible in TUI suggestions. - Why it matters: users can verify plugin commands such as `/phone`, `/pair`, or `/dreaming` in the Gateway command surface, but typing the same prefix in `openclaw tui` did not suggest those plugin-owned commands. - What changed: `GatewayChatClient` now exposes `commands.list`; TUI fetches text-scope command entries from the connected Gateway when supported and merges their aliases into slash autocomplete while keeping built-ins first. - What did NOT change (scope boundary): this does not change plugin command registration, command execution/dispatch semantics, Discord native slash command sync, or embedded/local TUI command discovery. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor required for the fix - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] Integrations - [ ] API / contracts - [x] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes # - Related openclaw#78347 - Related openclaw#73984 - Related openclaw#61966 - [x] This PR fixes a bug or regression ## Real behavior proof (required for external PRs) - Behavior or issue addressed: plugin commands present in the running Gateway command registry did not appear in `openclaw tui` slash autocomplete. - Real environment tested: patched OpenClaw Docker image built from this branch (`se7en/openclaw:tui-plugin-autocomplete-test`), OpenClaw `2026.5.17`, loopback Gateway, temporary plugin command setup. - Exact steps or command run after this patch: 1. Confirm a plugin command exists in the running Gateway text command surface. Example from an existing OpenClaw setup: ```bash openclaw gateway call commands.list --params '{"scope":"text","includeArgs":false}' --json \ | jq '.commands[] | select(.name == "phone")' ``` 2. Start the patched TUI against a Gateway with a plugin command loaded. 3. Type the plugin command prefix, for example `/pho` for `/phone` or `/nemo` in the temporary NemoClaw validation setup. 4. Submit the plugin command through TUI. - Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): ```text Existing Gateway plugin command example: { "name": "phone", "textAliases": ["/phone"], "description": "Arm/disarm high-risk phone node commands (camera/screen/writes).", "source": "plugin", "scope": "both", "acceptsArgs": true } Patched Docker/TUI validation command surface contained: { "name": "nemoclaw", "nativeName": "nemoclaw", "textAliases": ["/nemoclaw"], "description": "NemoClaw sandbox management (status, eject).", "source": "plugin", "scope": "both", "acceptsArgs": true } openclaw tui autocomplete after typing /nemo showed: → nemoclaw NemoClaw sandbox management (status, eject). openclaw tui --message '/nemoclaw status' returned: NemoClaw: No operations performed yet. Run nemoclaw onboard to get started. ``` - Observed result after fix: plugin command aliases from the running Gateway appeared in TUI slash autocomplete and the command could be submitted through the TUI/Gateway path. - What was not tested: Discord native slash command registration/sync, `openclaw agent --message` plugin dispatch, and embedded/local TUI dynamic plugin discovery. - Before evidence (optional but encouraged): current installed host OpenClaw `2026.5.12` and sandbox OpenClaw `2026.4.24` TUI bundles used static `getSlashCommands(...)` autocomplete and did not call `commands.list`. ## Root Cause (if applicable) - Root cause: `src/tui/tui.ts` built its autocomplete provider from `getSlashCommands(...)`, which combined built-in/local command metadata but did not consume the running Gateway's plugin command registry. - Missing detection / guardrail: there was no TUI-side regression coverage proving `commands.list` entries are requested and merged into slash autocomplete. - Contributing context (if known): plugin commands can be registered and exposed by the Gateway independently from the static TUI command list, so a connected TUI needs to use the Gateway command surface as the source of truth for dynamic commands. ## Regression Test Plan (if applicable) - Coverage level that should have caught this: - [x] Unit test - [x] Seam / integration test - [ ] End-to-end test - [ ] Existing coverage already sufficient - Target test or file: - `src/tui/commands.test.ts` - `src/tui/gateway-chat.test.ts` - Scenario the test should lock in: dynamic command entries from the Gateway are requested through `commands.list`, merged into TUI slash commands, deduplicated, and do not replace built-in commands on name collisions. - Why this is the smallest reliable guardrail: it tests the TUI command merge behavior and the Gateway client RPC without requiring a real plugin package or a channel integration. - Existing test that already covers this (if any): none before this change. - If no new test is added, why not: N/A; focused tests were added. ## User-visible / Behavior Changes Gateway-connected `openclaw tui` can now suggest plugin-owned slash command aliases that are already exposed by the running Gateway command surface. Built-in/static suggestions remain first and keep priority on collisions. ## Diagram (if applicable) ```text Before: [user types /pho in openclaw tui] -> TUI local/static slash list only -> plugin command from Gateway commands.list is not suggested After: [user types /pho in openclaw tui] -> TUI local/static slash list -> Gateway commands.list text-scope entries -> merged autocomplete suggestions include plugin aliases such as /phone ``` ## Security Impact (required) - New permissions/capabilities? (`Yes/No`): No - Secrets/tokens handling changed? (`Yes/No`): No - New/changed network calls? (`Yes/No`): Yes - Command/tool execution surface changed? (`Yes/No`): No - Data access scope changed? (`Yes/No`): No - If any `Yes`, explain risk + mitigation: the TUI may call the existing authenticated Gateway RPC method `commands.list` over the already-established Gateway connection. This reads command metadata only; it does not execute commands or expose secrets. Failures are caught and ignored so autocomplete falls back to the existing static list. ## Repro + Verification ### Environment - OS: Linux - Runtime/container: Docker image built from patched OpenClaw source (`se7en/openclaw:tui-plugin-autocomplete-test`) - Model/provider: not relevant for autocomplete; sandbox test used `nvidia/nemotron-3-super-120b-a12b` for the NemoClaw environment - Integration/channel (if any): TUI connected to Gateway - Relevant config (redacted): loopback Gateway with temporary local plugin registration; no secrets included in evidence ### Steps 1. Install or load a plugin that registers a slash command through `api.registerCommand(...)`. 2. Confirm the running Gateway exposes it: ```bash openclaw gateway call commands.list --params '{"scope":"text","includeArgs":false}' --json ``` 3. Start `openclaw tui` against that Gateway and type the plugin command prefix. ### Expected - TUI autocomplete includes plugin command aliases from the running Gateway command surface. ### Actual - Before this patch, TUI autocomplete only included the local/static slash command list. - After this patch, plugin command aliases from `commands.list` are merged into autocomplete. ## Evidence Attach at least one: - [x] Failing test/log before + passing after - [x] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) Focused verification run after the final narrowing change: ```bash COREPACK_HOME=/tmp/corepack PNPM_HOME=/tmp/pnpm \ node scripts/run-vitest.mjs run \ src/tui/commands.test.ts \ src/tui/gateway-chat.test.ts \ src/tui/embedded-backend.test.ts # 3 files passed, 43 tests passed COREPACK_HOME=/tmp/corepack PNPM_HOME=/tmp/pnpm \ pnpm exec tsc --noEmit --pretty false --project tsconfig.core.json # passed git diff --check # passed ``` ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: - Patched Docker image built from the OpenClaw branch. - Gateway `commands.list` returned a plugin command entry for `/nemoclaw` in the temporary plugin validation setup. - Patched `openclaw tui` autocomplete showed `→ nemoclaw NemoClaw sandbox management (status, eject).` - `/nemoclaw status` submitted through the patched TUI/Gateway path returned plugin output. - Focused TUI command merge/client tests passed after narrowing the code path to gateway backends. - Edge cases checked: - Dynamic plugin commands do not override built-in command names. - `commands.list` failures are soft-failed and leave the static autocomplete list intact. - Backends without `listCommands` keep the previous behavior. - What you did **not** verify: - Discord native slash command sync/ack behavior. - `openclaw agent --message` plugin command dispatch. - Embedded/local TUI dynamic plugin discovery. ## Review Conversations - [ ] I replied to or resolved every bot review conversation I addressed in this PR. - [ ] I left unresolved only the conversations that still need reviewer or maintainer judgment. N/A — no PR review conversations exist yet. ## Compatibility / Migration - Backward compatible? (`Yes/No`): Yes - Config/env changes? (`Yes/No`): No - Migration needed? (`Yes/No`): No - If yes, exact upgrade steps: N/A ## Risks and Mitigations - Risk: a Gateway may be older or may reject/fail `commands.list`. - Mitigation: the call is optional and failures are ignored; TUI keeps the existing static slash autocomplete. - Risk: plugin command names could collide with built-in commands. - Mitigation: built-in/static commands are added first and dynamic commands are deduplicated without overriding them.
|
This pull request has been automatically marked as stale due to inactivity. |
Summary
commands.listinto TUI slash-command autocomplete so plugin commands (for example/active-memory) appear in suggestions./agent,/model, and session changes that alter agent/provider), while preserving local command argument-completion behavior on name collisions.Test plan
pnpm test src/tui/tui.test.tspnpm test src/tui/tui-command-handlers.test.tspnpm tui, switch via/agentstoclaude-4, type/act, confirm/active-memoryappears in autocomplete./model <provider/model>and verify autocomplete updates to the current command surface.Made with Cursor