Skip to content

fix: support /plugins slash command in resume mode#2973

Open
code-yeongyu wants to merge 3 commits intomainfrom
fix/resume-plugins-slash-command
Open

fix: support /plugins slash command in resume mode#2973
code-yeongyu wants to merge 3 commits intomainfrom
fix/resume-plugins-slash-command

Conversation

@code-yeongyu
Copy link
Copy Markdown
Collaborator

Bug

claw --resume session.jsonl /plugins --output-format json returns:

{"command":"/plugins","error":"unsupported resumed slash command","type":"error"}

But /mcp and /skills both work in resume mode. The /plugins handler exists in the REPL and non-resume CLI paths — it was just never wired into run_resume_command.

Fix

  • Move SlashCommand::Plugins out of the unsupported catch-all in run_resume_command
  • Add a handler arm that calls handle_plugins_slash_command for list/help
  • Mutation actions (install/uninstall/enable/disable) are rejected with a clear error since there is no runtime to reload in resume mode
  • Add /plugins coverage to resumed_inventory_commands_emit_structured_json_when_requested test

Verification

cargo test -p rusty-claude-cli -- resumed_inventory_commands
# 1 passed, 0 failed

Before: exit 1, error envelope
After: exit 0, {kind: "plugin", action: "list", reload_runtime: false, target: null, message: "..."}

Move SlashCommand::Plugins out of the 'unsupported resumed slash
command' catch-all and add a handler arm in run_resume_command that
calls handle_plugins_slash_command for list/help actions.

Mutation actions (install/uninstall/enable/disable) are rejected with
a clear error since there is no runtime to reload in resume mode.

Add /plugins coverage to resumed_inventory_commands test in
output_format_contract.rs: kind, action, reload_runtime, target.

Before: claw --resume session.jsonl /plugins --output-format json
-> {error: 'unsupported resumed slash command', type: 'error'}, exit 1

After: claw --resume session.jsonl /plugins --output-format json
-> {kind: 'plugin', action: 'list', ...}, exit 0
@code-yeongyu code-yeongyu force-pushed the fix/resume-plugins-slash-command branch from cf38875 to b9a17db Compare April 30, 2026 21:42
@Yeachan-Heo
Copy link
Copy Markdown
Contributor

Review verdict: REQUEST_CHANGES

Reviewed current head 39bae88 against current base 8e45f185. CI is green: cargo fmt, clippy, test, and docs source-of-truth all pass.

Changed files:

  • rust/crates/rusty-claude-cli/src/main.rs
  • rust/crates/rusty-claude-cli/tests/output_format_contract.rs

Local targeted tests passed:

  • cargo test -p rusty-claude-cli --test output_format_contract resumed_inventory_commands_emit_structured_json_when_requested -- --nocapture
  • cargo test -p rusty-claude-cli direct_slash_commands_surface_shared_validation_errors -- --nocapture
  • cargo test -p commands validate_slash_command_input_rejects_extra_single_value_arguments -- --nocapture

I also manually exercised resume-mode /plugins commands against the built binary.

This PR changes user-facing resume-mode slash/JSON behavior, so owner confirmation is required before merge even after blockers are fixed.

Blockers:

  1. /plugins update <id> is still allowed through resume mode.

The new resume handler rejects install, uninstall, enable, and disable, but not update. The parser supports ["update", target] as SlashCommand::Plugins { action: Some("update"), ... }, and handle_plugins_slash_command can perform manager.update(...) with reload_runtime: true.

Manual evidence: /plugins update demo reaches the plugin update path and errors only because demo is not installed. If the plugin exists, this can mutate plugin state in resume mode without runtime reload.

  1. Claimed list/help support is incomplete: /plugins help and /plugins --help fail before the resume handler.

The resume handler comment says only list/help are supported, but parse_plugin_command does not parse help or --help as a valid plugin action. Manual results for /plugins help, /plugins --help, /plugin help, and /plugin --help are errors/exit 2.

Requested changes:

  • Reject update in resume mode alongside the other mutation commands.
  • Either implement /plugins help / /plugins --help parsing and resume output, or remove the help-support claim from PR description/comment/tests.
  • Add regression tests for /plugins update <id> being rejected in resume mode before mutation. Ideally cover all mutation verbs: install/uninstall/enable/disable/update.
  • If help is intended, add tests for /plugins help and/or /plugins --help; if not, make the contract list-only.

Merge risk: medium until mutation handling is fixed; likely low after fixes, subject to owner confirmation of the new resume-mode /plugins JSON contract.


[repo owner's gaebal-gajae (clawdbot) 🦞]

Address REQUEST_CHANGES from OMX review:
1. Add 'update' to the blocked mutation actions in resume mode
   (previously only install/uninstall/enable/disable were blocked)
2. Fix comment: 'Only list is supported' instead of 'Only list/help'
   since /plugins help doesn't actually parse as a valid action
@Yeachan-Heo
Copy link
Copy Markdown
Contributor

Re-review verdict: APPROVE

Reviewed current head ae65619cd9eb342974a0c182d4fa4d56a70160b3 after the REQUEST_CHANGES fix.

Previous blockers are resolved:

  1. /plugins update <id> is now rejected in resume mode before plugin manager construction / handler dispatch.

    • main.rs now rejects install, uninstall, enable, disable, and update.
    • Plugin manager setup happens after this guard, so update cannot reach the mutation path in resume mode.
  2. The false list/help claim is removed.

    • The contract/comment now says list-only: Only list is supported in resume mode (no runtime to reload).

CI for ae65619 is green 4/4:

  • cargo clippy --workspace: success
  • cargo fmt: success
  • cargo test --workspace: success
  • docs source-of-truth: success

Targeted verification:

  • cargo test -p rusty-claude-cli --test output_format_contract resumed_inventory_commands_emit_structured_json_when_requested -- --nocapture passed.
  • Manual smoke: --resume <session> /plugins returns JSON with kind=plugin, action=list, target=null, reload_runtime=false.
  • Manual smoke: --resume <session> /plugins update bogus exits nonzero with the interactive-only resume mutation error.
  • Update-first smoke confirmed no plugin registry file was created before rejection.

Merge risk: LOW.

Blockers: none from code review.

Process gate: owner confirmation is still required before merge because this changes user-facing resume-mode /plugins slash/JSON behavior.


[repo owner's gaebal-gajae (clawdbot) 🦞]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants