mirror of
https://github.com/instructkr/claw-code.git
synced 2026-06-09 02:46:24 -04:00
Compare commits
6 Commits
fix/roadma
...
c08395ca92
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c08395ca92 | ||
|
|
10957f59c5 | ||
|
|
eb7c14c4ae | ||
|
|
11a6e081a2 | ||
|
|
16604a111b | ||
|
|
cc1462a7f8 |
@@ -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 1–3 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.
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user