Compare commits

...

6 Commits

Author SHA1 Message Date
YeonGyu-Kim
c08395ca92 docs(roadmap): add #700 help JSON missing status + session_list kind inconsistency 2026-05-25 17:03:31 +09:00
Yeachan-Heo
10957f59c5 docs(roadmap): add #699 bootstrap-plan/dump-manifests local dispatch gap 2026-05-25 08:00:28 +00:00
YeonGyu-Kim
eb7c14c4ae fix(#458): add status:ok to bootstrap-plan JSON envelope; all 12 JSON surfaces now have uniform status field 2026-05-25 16:34:33 +09:00
YeonGyu-Kim
11a6e081a2 fix(#458): add status field to export and diff JSON envelopes 2026-05-25 16:07:16 +09:00
Bellman
16604a111b fix(#458): add status assertions to skills/agents JSON envelope tests
Adds test coverage asserting status:ok in skills list, agents list, skills install, skills help, and agents help JSON envelopes. Status fields themselves were already landed in 0581894b and cc1462a7; this PR adds the regression test assertions that were missing. Duplicate status keys in json! macros are benign (serde_json takes last value) but harmless given all tests pass.
2026-05-25 15:35:40 +09:00
YeonGyu-Kim
cc1462a7f8 fix(#458): add status:ok to skills install JSON envelope (missed in previous sweep) 2026-05-25 15:30:22 +09:00
3 changed files with 26 additions and 0 deletions

View File

@@ -7561,3 +7561,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
697. **`claw plugins remove <name>` silently returns `status:"ok"` with exit 0 when the named plugin does not exist — no `not_found` error, no non-zero exit, no indication the operation was a no-op; sibling: `claw agents <unknown-subcommand>` returns `action:"help"` with exit 0 instead of a typed `unknown_subcommand` error** — dogfooded 2026-05-25 on `63a5a874`. Reproduction: `claw plugins remove nonexistent-plugin --output-format json </dev/null` returns `{"kind":"plugin","action":"remove","status":"ok","error":null,...}` and exits 0. No plugin named `nonexistent-plugin` exists. A caller cannot distinguish "plugin was successfully removed" from "plugin was never there" — both produce the same `status:"ok"` envelope. Sibling: `claw agents stop nonexistent-agent --output-format json </dev/null` returns `{"kind":"agents","action":"help","unexpected":"stop nonexistent-agent",...}` and exits 0 — unknown subcommand falls through to help output rather than a typed error with exit 1. **Required fix shape:** (a) `plugins remove` must check whether the named plugin exists before reporting success; emit `{"kind":"plugin","action":"remove","status":"error","error_kind":"plugin_not_found","plugin_name":"<name>"}` with exit 1 when the plugin is absent; (b) `agents <unknown>` must emit `{"kind":"agents","action":"error","error_kind":"unknown_subcommand","subcommand":"<token>","supported":["list","help"]}` with exit 1 instead of falling back to help output with exit 0; (c) add regression tests proving both paths exit 1 with typed error envelopes. **Why this matters:** idempotent-but-silent remove is fine for infrastructure tools with explicit idempotency contracts; claw has no such contract, and `status:"ok"` for a name-miss means automation cannot audit whether a remove actually ran vs was a no-op. Source: Jobdori dogfood on `63a5a874`, 2026-05-25.
698. **Config deprecation warnings emit once per `ConfigLoader::load()` call, so surfaces that call `load()` multiple times in a single invocation emit duplicate `warning:` lines to stderr — `claw plugins list` and `claw mcp list` each print the same deprecation warning twice** — dogfooded 2026-05-25 on `c345ce6d`. Reproduction: `echo '{"enabledPlugins": {}}' > ~/.claw/settings.json && claw plugins list 2>&1 | grep warning` prints the same `field "enabledPlugins" is deprecated. Use "plugins.enabled" instead` line twice. Root cause: `config.rs:304` emits `eprintln!("warning: {warning}")` for every warning in every `loader.load()` call; surfaces like `plugins_command_payload_for` and `render_mcp_report_json_for` each trigger an independent `loader.load()` (one for runtime config, one inside the command handler), multiplying the stderr output. `skills list` emits only one warning because its command path calls `load()` once; `plugins` and `mcp` emit two. **Required fix shape:** (a) track already-emitted warning strings in a process-lifetime `std::sync::OnceLock<Mutex<HashSet<String>>>` in `config.rs` and skip re-emitting duplicates within the same process run; or (b) collect all warnings at a single call site after all config loads are complete and emit once with dedup; or (c) change `load()` to return warnings alongside the result instead of eagerly printing them, letting call sites emit once. Option (a) is a minimal one-file fix. **Why this matters:** duplicate warnings make the CLI look buggy, cause CI log noise, and — when the deprecation warning fires on every invocation — are more likely to be `tail -f`'d away than acted on. A single clean warning per invocation is the standard. Source: Jobdori dogfood on `c345ce6d`, 2026-05-25.
699. **`bootstrap-plan` and `dump-manifests` JSON/help probes fall through to prompt/auth instead of local command dispatch unless global flags are positioned just so; with normal subcommand-style argv they either hang behind the spinner or return `missing_credentials`, making local startup/manifest introspection non-local** — dogfooded 2026-05-25 on `11a6e081a` after the ROADMAP #458 envelope sweep. Reproduction with the freshly rebuilt debug binary: `./rust/target/debug/claw bootstrap-plan --output-format json </dev/null` times out after 10s with spinner bytes on stdout and only a config deprecation warning on stderr in the normal home env; in an isolated env without credentials it exits 1 as `missing_credentials` instead of returning the local bootstrap phase JSON. `./rust/target/debug/claw dump-manifests --output-format json </dev/null` behaves the same. The help forms `bootstrap-plan --help --output-format json` and `dump-manifests --help --output-format json` also time out behind the spinner. This is distinct from #690/#692, which assumed these help paths were intercepted and only lacked schema depth; current dogfood shows an even lower-level routing/argv-order gap on the rebuilt `11a6e081a` binary. **Why it matters:** `bootstrap-plan` and `dump-manifests` are supposed to be credential-free local introspection/preflight commands. If the parser treats `--output-format json` after the subcommand as prompt text or routes into the provider/auth path, claws cannot safely probe startup phases or manifest availability without API credentials and timeout guards. **Required fix shape:** (a) make subcommand-local `--output-format json` and `--help --output-format json` dispatch before prompt/auth for `bootstrap-plan` and `dump-manifests`; (b) guarantee these commands are local-only and do not require provider credentials; (c) add regression tests for both argv orders if global flags are supported after subcommands, or emit a fast typed `cli_parse` error with `status:"error"` and no spinner if not supported; (d) acceptance: `env -i HOME=/tmp/empty PATH=/usr/bin:/bin TERM=dumb ./rust/target/debug/claw bootstrap-plan --output-format json </dev/null | jq -e '.kind=="bootstrap-plan" and (.phases|length>0)'` and the analogous dump-manifests/help probes must return within 1s without credentials. Source: gaebal-gajae dogfood for the 2026-05-25 07:30 Clawhip nudge.
700. **`claw help --output-format json` emits `{"kind":"help","message":"<prose>"}` with no `status` field, and `claw sessions` (via `/sessions list` slash command) emits `{"kind":"session_list",...}` — both are envelope shape inconsistencies relative to the now-complete #458 sweep** — dogfooded 2026-05-25 on `eb7c14c4`. (1) `help` JSON: all 12 probed surfaces now have `status ∈ {ok,warn,error,unsupported}` after the #458 sweep; `help` is the one remaining surface that emits JSON but lacks `status`. The envelope has only `kind:"help"` and `message:"<prose blob>"` — no machine-readable status, no structured sections array (per #325/#686/#687/#688). (2) `session_list` kind: `claw sessions` and the `/sessions list` slash command emit `"kind":"session_list"` — all other surfaces use the subcommand name as the kind token (`kind:"skills"`, `kind:"agents"`, `kind:"mcp"` etc). `session_list` is a verb+noun compound that breaks the convention and makes kind-based routing require a special case. **Required fix shape:** (a) add `"status": "ok"` to all `help` JSON emission sites (`print_help` at line ~7120 and ~7167, plus inline REPL help at ~3924 in `main.rs`); (b) rename `"kind":"session_list"` to `"kind":"sessions"` at the two emission sites (lines ~3912, ~6385) and update any test assertions; keep a `"action":"list"` field for the action discriminant. Both are 13 line changes. **Why this matters:** the #458 acceptance check `for c in … ; do claw $c --output-format json | jq -e '.status | IN(…)' || echo FAIL; done` still FAILs for `help`; `session_list` kind breaks any kind-routing table that maps surface names to handler IDs. Source: Jobdori dogfood on `eb7c14c4`, 2026-05-25.

View File

@@ -3627,6 +3627,7 @@ fn render_agents_report_json(cwd: &Path, agents: &[AgentSummary]) -> Value {
"kind": "agents",
"status": "ok",
"action": "list",
"status": "ok",
"working_directory": cwd.display().to_string(),
"count": agents.len(),
"summary": {
@@ -3710,6 +3711,7 @@ fn render_skills_report_json(skills: &[SkillSummary]) -> Value {
"kind": "skills",
"status": "ok",
"action": "list",
"status": "ok",
"summary": {
"total": skills.len(),
"active": active,
@@ -3743,7 +3745,9 @@ fn render_skill_install_report(skill: &InstalledSkill) -> String {
fn render_skill_install_report_json(skill: &InstalledSkill) -> Value {
json!({
"kind": "skills",
"status": "ok",
"action": "install",
"status": "ok",
"result": "installed",
"invocation_name": &skill.invocation_name,
"invoke_as": format!("${}", skill.invocation_name),
@@ -5312,6 +5316,7 @@ mod tests {
assert_eq!(report["kind"], "agents");
assert_eq!(report["action"], "list");
assert_eq!(report["status"], "ok");
assert_eq!(report["working_directory"], workspace.display().to_string());
assert_eq!(report["count"], 3);
assert_eq!(report["summary"]["active"], 2);
@@ -5327,6 +5332,7 @@ mod tests {
let help = handle_agents_slash_command_json(Some("help"), &workspace).expect("agents help");
assert_eq!(help["kind"], "agents");
assert_eq!(help["action"], "help");
assert_eq!(help["status"], "ok");
assert_eq!(help["usage"]["direct_cli"], "claw agents [list|help]");
// Unknown agents subcommands now return Err so CLI layer can exit 1.
@@ -5441,6 +5447,7 @@ mod tests {
);
assert_eq!(report["kind"], "skills");
assert_eq!(report["action"], "list");
assert_eq!(report["status"], "ok");
assert_eq!(report["summary"]["active"], 3);
assert_eq!(report["summary"]["shadowed"], 1);
assert_eq!(report["skills"][0]["name"], "plan");
@@ -5452,6 +5459,7 @@ mod tests {
let help = handle_skills_slash_command_json(Some("help"), &workspace).expect("skills help");
assert_eq!(help["kind"], "skills");
assert_eq!(help["action"], "help");
assert_eq!(help["status"], "ok");
assert_eq!(help["usage"]["aliases"][0], "/skill");
assert_eq!(
help["usage"]["direct_cli"],
@@ -5516,6 +5524,7 @@ mod tests {
let sources = skills_help_json["usage"]["sources"]
.as_array()
.expect("skills help sources");
assert_eq!(skills_help_json["status"], "ok");
assert_eq!(skills_help_json["usage"]["aliases"][0], "/skill");
assert!(sources.iter().any(|value| value == ".omc/skills"));
assert!(sources.iter().any(|value| value == ".agents/skills"));
@@ -5901,6 +5910,13 @@ mod tests {
assert!(report.contains("Invoke as $help"));
assert!(report.contains(&install_root.display().to_string()));
let json_report = super::render_skill_install_report_json(&installed);
assert_eq!(json_report["kind"], "skills");
assert_eq!(json_report["action"], "install");
assert_eq!(json_report["status"], "ok");
assert_eq!(json_report["invocation_name"], "help");
assert_eq!(json_report["invoke_as"], "$help");
let roots = vec![SkillRoot {
source: DefinitionSource::UserCodexHome,
path: install_root.clone(),

View File

@@ -2853,6 +2853,7 @@ fn print_bootstrap_plan(output_format: CliOutputFormat) -> Result<(), Box<dyn st
"{}",
serde_json::to_string_pretty(&json!({
"kind": "bootstrap-plan",
"status": "ok",
"phases": phases,
}))?
),
@@ -4113,6 +4114,7 @@ fn run_resume_command(
)),
json: Some(serde_json::json!({
"kind": "export",
"status": "ok",
"file": export_path.display().to_string(),
"message_count": msg_count,
})),
@@ -7560,6 +7562,7 @@ fn render_diff_json_for(cwd: &Path) -> Result<serde_json::Value, Box<dyn std::er
if !in_git_repo {
return Ok(serde_json::json!({
"kind": "diff",
"status": "error",
"result": "no_git_repo",
"detail": format!("{} is not inside a git project", cwd.display()),
}));
@@ -7568,6 +7571,7 @@ fn render_diff_json_for(cwd: &Path) -> Result<serde_json::Value, Box<dyn std::er
let unstaged = run_git_diff_command_in(cwd, &["diff"])?;
Ok(serde_json::json!({
"kind": "diff",
"status": "ok",
"result": if staged.trim().is_empty() && unstaged.trim().is_empty() { "clean" } else { "changes" },
"staged": staged.trim(),
"unstaged": unstaged.trim(),
@@ -8090,6 +8094,7 @@ fn run_export(
"{}",
serde_json::to_string_pretty(&json!({
"kind": "export",
"status": "ok",
"message": report,
"session_id": handle.id,
"file": path.display().to_string(),
@@ -8111,6 +8116,7 @@ fn run_export(
"{}",
serde_json::to_string_pretty(&json!({
"kind": "export",
"status": "ok",
"session_id": handle.id,
"file": handle.path.display().to_string(),
"messages": session.messages.len(),