fix: support /plugins slash command in resume mode#2973
Conversation
cf38875 to
b9a17db
Compare
|
Review verdict: REQUEST_CHANGES Reviewed current head Changed files:
Local targeted tests passed:
I also manually exercised resume-mode This PR changes user-facing resume-mode slash/JSON behavior, so owner confirmation is required before merge even after blockers are fixed. Blockers:
The new resume handler rejects Manual evidence:
The resume handler comment says only list/help are supported, but Requested changes:
Merge risk: medium until mutation handling is fixed; likely low after fixes, subject to owner confirmation of the new resume-mode — |
|
Re-review verdict: APPROVE Reviewed current head Previous blockers are resolved:
CI for
Targeted verification:
Merge risk: LOW. Blockers: none from code review. Process gate: owner confirmation is still required before merge because this changes user-facing resume-mode — |
Review ChecklistUser-facing behavior added
Config keys / CLI flags changed
Migration or compatibility notes
Tests run locally
Known risks / non-goals
|
|
QA FAIL: |
|
QA note: |
resumed_status_command_emits_structured_json_when_requested was reading the real ~/.claw/settings.json, causing loaded_config_files to be 1 instead of the expected 0 on machines with user config present. Root cause: unlike other tests (e.g. resumed_config_command_loads_settings_files), this test did not pass an isolated CLAW_CONFIG_HOME env var to run_claw, so claw fell back to the real HOME and loaded the developer's settings file. Fix: create a temp config-home dir and pass it as CLAW_CONFIG_HOME via run_claw_with_env. This gives the assertion a clean 0-file baseline. Unblocks PRs #2973, #2988, #2990 which all failed this same test on main. Ref: ROADMAP #65
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
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
ae65619 to
c956e46
Compare
Bug
claw --resume session.jsonl /plugins --output-format jsonreturns:{"command":"/plugins","error":"unsupported resumed slash command","type":"error"}But
/mcpand/skillsboth work in resume mode. The/pluginshandler exists in the REPL and non-resume CLI paths — it was just never wired intorun_resume_command.Fix
SlashCommand::Pluginsout of the unsupported catch-all inrun_resume_commandhandle_plugins_slash_commandfor list/help/pluginscoverage toresumed_inventory_commands_emit_structured_json_when_requestedtestVerification
Before: exit 1, error envelope
After: exit 0,
{kind: "plugin", action: "list", reload_runtime: false, target: null, message: "..."}