Make dump-manifests recoverable outside the inferred build tree

The backlog sweep found that the user-cited #21-#23 items were already
closed, and the next real pain point was `claw dump-manifests` failing
without a direct way to point at the upstream manifest source. This adds
an explicit `--manifests-dir` path, upgrades the failure messages to say
whether the source root or required files are missing, and updates the
ROADMAP closeout to reflect that #45 is now fixed.

Constraint: Preserve existing dump-manifests behavior when no explicit override is supplied
Rejected: Require CLAUDE_CODE_UPSTREAM for every invocation | breaks existing build-tree workflows and is unnecessarily rigid
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep manifest-source override guidance centralized so future error-path edits do not drift
Tested: cargo fmt --all; cargo clippy --workspace --all-targets -- -D warnings; cargo test --workspace; architect review APPROVE
Not-tested: Manual invocation against every legacy env-based manifest lookup layout
This commit is contained in:
Yeachan-Heo 2026-04-12 02:56:48 +00:00
parent 1d83e67802
commit 4f83a81cf6
3 changed files with 182 additions and 52 deletions

View File

@ -453,12 +453,15 @@ Model name prefix now wins unconditionally over env-var presence. Regression tes
42. **`--output-format json` errors emitted as prose, not JSON** — dogfooded 2026-04-09. When `claw --output-format json prompt` hits an API error, the error was printed as plain text (`error: api returned 401 ...`) to stderr instead of a JSON object. Any tool or CI step parsing claw's JSON output gets nothing parseable on failure — the error is invisible to the consumer. **Fix (`a...`):** detect `--output-format json` in `main()` at process exit and emit `{"type":"error","error":"<message>"}` to stderr instead of the prose format. Non-JSON path unchanged. **Done** in this nudge cycle.
43. **Hook ingress opacity: typed hook-health/delivery report missing****verified likely external tracking on 2026-04-12:** repo-local searches for `/hooks/health`, `/hooks/status`, and hook-ingress route code found no implementation surface outside `ROADMAP.md`, and the prior state-surface note below already records that the HTTP server is not owned by claw-code. Treat this as likely upstream/server-surface tracking rather than an immediate claw-code task. **Original filing below.**
43. **Hook ingress opacity: typed hook-health/delivery report missing** — dogfooded 2026-04-09 while wiring the agentika timer→hook→session bridge. Debugging hook delivery required manual HTTP probing and inferring state from raw status codes (404 = no route, 405 = route exists, 400 = body missing required field). No typed endpoint exists to report: route present/absent, accepted methods, mapping matched/not matched, target session resolved/not resolved, last delivery failure class. Fix shape: add `GET /hooks/health` (or `/hooks/status`) returning a structured JSON diagnostic — no auth exposure, just routing/matching/session state. Source: gaebal-gajae dogfood 2026-04-09.
44. **Broad-CWD guardrail is warning-only; needs policy-level enforcement** — dogfooded 2026-04-09. `5f6f453` added a stderr warning when claw starts from `$HOME` or filesystem root (live user kapcomunica scanned their whole machine). Warning is a mitigation, not a guardrail: the agent still proceeds with unbounded scope. Follow-up fix shape: (a) add `--allow-broad-cwd` flag to suppress the warning explicitly (for legitimate home-dir use cases); (b) in default interactive mode, prompt "You are running from your home directory — continue? [y/N]" and exit unless confirmed; (c) in `--output-format json` or piped mode, treat broad-CWD as a hard error (exit 1) with `{"type":"error","error":"broad CWD: running from home directory requires --allow-broad-cwd"}`. Source: kapcomunica in #claw-code 2026-04-09; gaebal-gajae ROADMAP note same cycle.
45. **`claw dump-manifests` fails with opaque "No such file or directory"** — dogfooded 2026-04-09. `claw dump-manifests` emits `error: failed to extract manifests: No such file or directory (os error 2)` with no indication of which file or directory is missing. **Partial fix at `47aa1a5`+1**: error message now includes `looked in: <path>` so the build-tree path is visible, what manifests are, or how to fix it. Fix shape: (a) surface the missing path in the error message; (b) add a pre-check that explains what manifests are and where they should be (e.g. `.claw/manifests/` or the plugins directory); (c) if the command is only valid after `claw init` or after installing plugins, say so explicitly. Source: Jobdori dogfood 2026-04-09.
45. **`claw dump-manifests` fails with opaque `No such file or directory`** — **done (verified 2026-04-12):** current `main` now accepts `claw dump-manifests --manifests-dir PATH`, pre-checks for the required upstream manifest files (`src/commands.ts`, `src/tools.ts`, `src/entrypoints/cli.tsx`), and replaces the opaque os error with guidance that points users to `CLAUDE_CODE_UPSTREAM` or `--manifests-dir`. Fresh proof: parser coverage for both flag forms, unit coverage for missing-manifest and explicit-path flows, and `output_format_contract` JSON coverage via the new flag all pass. **Original filing below.**
45. **`claw dump-manifests` fails with opaque `No such file or directory`** — **done (verified 2026-04-12):** current `main` now accepts `claw dump-manifests --manifests-dir PATH`, pre-checks for the required upstream manifest files (`src/commands.ts`, `src/tools.ts`, `src/entrypoints/cli.tsx`), and replaces the opaque os error with guidance that points users to `CLAUDE_CODE_UPSTREAM` or `--manifests-dir`. Fresh proof: parser coverage for both flag forms, unit coverage for missing-manifest and explicit-path flows, and `output_format_contract` JSON coverage via the new flag all pass. **Original filing below.**
46. **`/tokens`, `/cache`, `/stats` were dead spec — parse arms missing** — dogfooded 2026-04-09. All three had spec entries with `resume_supported: true` but no parse arms, producing the circular error "Unknown slash command: /tokens — Did you mean /tokens". Also `SlashCommand::Stats` existed but was unimplemented in both REPL and resume dispatch. **Done at `60ec2ae` 2026-04-09**: `"tokens" | "cache"` now alias to `SlashCommand::Stats`; `Stats` is wired in both REPL and resume path with full JSON output. Source: Jobdori dogfood.
47. **`/diff` fails with cryptic "unknown option 'cached'" outside a git repo; resume /diff used wrong CWD** — dogfooded 2026-04-09. `claw --resume <session> /diff` in a non-git directory produced `git diff --cached failed: error: unknown option 'cached'` because git falls back to `--no-index` mode outside a git tree. Also resume `/diff` used `session_path.parent()` (the `.claw/sessions/<id>/` dir) as CWD for the diff — never a git repo. **Done at `aef85f8` 2026-04-09**: `render_diff_report_for()` now checks `git rev-parse --is-inside-work-tree` first and returns a clear "no git repository" message; resume `/diff` uses `std::env::current_dir()`. Source: Jobdori dogfood.
@ -493,7 +496,7 @@ Model name prefix now wins unconditionally over env-var presence. Regression tes
62. **Worker state file surface not implemented****done (verified 2026-04-12):** current `main` already wires `emit_state_file(worker)` into the worker transition path in `rust/crates/runtime/src/worker_boot.rs`, atomically writes `.claw/worker-state.json`, and exposes the documented reader surface through `claw state` / `claw state --output-format json` in `rust/crates/rusty-claude-cli/src/main.rs`. Fresh proof exists in `runtime` regression `emit_state_file_writes_worker_status_on_transition`, the end-to-end `tools` regression `recovery_loop_state_file_reflects_transitions`, and direct CLI parsing coverage for `state` / `state --output-format json`. Source: Jobdori dogfood.
**Scope note (verified 2026-04-12):** ROADMAP #31 and #63-#68 describe acpx/droid or OMX/Ultraclaw orchestration/runtime behavior, not claw-code source already present in this repository. Repo-local searches for `acpx`, `use-droid`, `run-acpx`, `commit-wrapper`, `ultraclaw`, `roadmap-nudge-10min`, and `OMX_TMUX_INJECT` found no implementation hits outside `ROADMAP.md`, so treat those entries as external tracking notes rather than immediate claw-code implementation backlog. With #31 retired as external, no repo-local Immediate Backlog item remains open on current main.
**Scope note (verified 2026-04-12):** ROADMAP #31, #43, and #63-#68 currently appear to describe acpx/droid or upstream OMX/server orchestration behavior, not claw-code source already present in this repository. Repo-local searches for `acpx`, `use-droid`, `run-acpx`, `commit-wrapper`, `ultraclaw`, `roadmap-nudge-10min`, `OMX_TMUX_INJECT`, `/hooks/health`, and `/hooks/status` found no implementation hits outside `ROADMAP.md`, and the earlier state-surface note already records that the HTTP server is not owned by claw-code. With #45 now fixed, the remaining unresolved items in this section look like external tracking notes rather than confirmed repo-local backlog; re-check if new repo-local evidence appears.
63. **Droid session completion semantics broken: code arrives after "status: completed"** — dogfooded 2026-04-12. Ultraclaw droid sessions (use-droid via acpx) report `session.status: completed` before file writes are fully flushed/synced to the working tree. Discovered +410 lines of "late-arriving" droid output that appeared after I had already assessed 8 sessions as "no code produced." This creates false-negative assessments and duplicate work. **Fix shape:** (a) droid agent should only report completion after explicit file-write confirmation (fsync or existence check); (b) or, claw-code should expose a `pending_writes` status that indicates "agent responded, disk flush pending"; (c) lane orchestrators should poll for file changes for N seconds after completion before final assessment. **Blocker:** none. Source: Jobdori ultraclaw dogfood 2026-04-12.

View File

@ -177,7 +177,10 @@ fn merge_prompt_with_stdin(prompt: &str, stdin_content: Option<&str>) -> String
fn run() -> Result<(), Box<dyn std::error::Error>> {
let args: Vec<String> = env::args().skip(1).collect();
match parse_args(&args)? {
CliAction::DumpManifests { output_format } => dump_manifests(output_format)?,
CliAction::DumpManifests {
output_format,
manifests_dir,
} => dump_manifests(manifests_dir.as_deref(), output_format)?,
CliAction::BootstrapPlan { output_format } => print_bootstrap_plan(output_format)?,
CliAction::Agents {
args,
@ -274,6 +277,7 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
enum CliAction {
DumpManifests {
output_format: CliOutputFormat,
manifests_dir: Option<PathBuf>,
},
BootstrapPlan {
output_format: CliOutputFormat,
@ -623,7 +627,7 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
let permission_mode = permission_mode_override.unwrap_or_else(default_permission_mode);
match rest[0].as_str() {
"dump-manifests" => Ok(CliAction::DumpManifests { output_format }),
"dump-manifests" => parse_dump_manifests_args(&rest[1..], output_format),
"bootstrap-plan" => Ok(CliAction::BootstrapPlan { output_format }),
"agents" => Ok(CliAction::Agents {
args: join_optional_args(&rest[1..]),
@ -1213,6 +1217,39 @@ fn parse_export_args(args: &[String], output_format: CliOutputFormat) -> Result<
})
}
fn parse_dump_manifests_args(
args: &[String],
output_format: CliOutputFormat,
) -> Result<CliAction, String> {
let mut manifests_dir: Option<PathBuf> = None;
let mut index = 0;
while index < args.len() {
let arg = &args[index];
if arg == "--manifests-dir" {
let value = args
.get(index + 1)
.ok_or_else(|| String::from("--manifests-dir requires a path"))?;
manifests_dir = Some(PathBuf::from(value));
index += 2;
continue;
}
if let Some(value) = arg.strip_prefix("--manifests-dir=") {
if value.is_empty() {
return Err(String::from("--manifests-dir requires a path"));
}
manifests_dir = Some(PathBuf::from(value));
index += 1;
continue;
}
return Err(format!("unknown dump-manifests option: {arg}"));
}
Ok(CliAction::DumpManifests {
output_format,
manifests_dir,
})
}
fn parse_resume_args(args: &[String], output_format: CliOutputFormat) -> Result<CliAction, String> {
let (session_path, command_tokens): (PathBuf, &[String]) = match args.first() {
None => (PathBuf::from(LATEST_SESSION_REFERENCE), &[]),
@ -1915,32 +1952,58 @@ fn looks_like_slash_command_token(token: &str) -> bool {
.any(|spec| spec.name == name || spec.aliases.contains(&name))
}
fn dump_manifests(output_format: CliOutputFormat) -> Result<(), Box<dyn std::error::Error>> {
fn dump_manifests(
manifests_dir: Option<&Path>,
output_format: CliOutputFormat,
) -> Result<(), Box<dyn std::error::Error>> {
let workspace_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..");
dump_manifests_at_path(&workspace_dir, output_format)
dump_manifests_at_path(&workspace_dir, manifests_dir, output_format)
}
const DUMP_MANIFESTS_OVERRIDE_HINT: &str =
"Hint: set CLAUDE_CODE_UPSTREAM=/path/to/upstream or pass `claw dump-manifests --manifests-dir /path/to/upstream`.";
// Internal function for testing that accepts a workspace directory path.
fn dump_manifests_at_path(
workspace_dir: &std::path::Path,
manifests_dir: Option<&Path>,
output_format: CliOutputFormat,
) -> Result<(), Box<dyn std::error::Error>> {
// Surface the resolved path in the error so users can diagnose missing
// manifest files without guessing what path the binary expected.
// ROADMAP #45: this path is only correct when running from the build tree;
// a proper fix would ship manifests alongside the binary.
let resolved = workspace_dir
.canonicalize()
.unwrap_or_else(|_| workspace_dir.to_path_buf());
let paths = if let Some(dir) = manifests_dir {
let resolved = dir.canonicalize().unwrap_or_else(|_| dir.to_path_buf());
UpstreamPaths::from_repo_root(resolved)
} else {
// Surface the resolved path in the error so users can diagnose missing
// manifest files without guessing what path the binary expected.
let resolved = workspace_dir
.canonicalize()
.unwrap_or_else(|_| workspace_dir.to_path_buf());
UpstreamPaths::from_workspace_dir(&resolved)
};
let paths = UpstreamPaths::from_workspace_dir(&resolved);
// Pre-check: verify manifest directory exists
let manifest_dir = paths.repo_root();
if !manifest_dir.exists() {
let source_root = paths.repo_root();
if !source_root.exists() {
return Err(format!(
"Manifest files (commands.ts, tools.ts) define CLI commands and tools.\n Expected at: {}\n Run `claw init` to create them or specify --manifests-dir.",
manifest_dir.display()
"Manifest source directory does not exist.\n looked in: {}\n {DUMP_MANIFESTS_OVERRIDE_HINT}",
source_root.display(),
)
.into());
}
let required_paths = [
("src/commands.ts", paths.commands_path()),
("src/tools.ts", paths.tools_path()),
("src/entrypoints/cli.tsx", paths.cli_path()),
];
let missing = required_paths
.iter()
.filter_map(|(label, path)| (!path.is_file()).then_some(*label))
.collect::<Vec<_>>();
if !missing.is_empty() {
return Err(format!(
"Manifest source files are missing.\n repo root: {}\n missing: {}\n {DUMP_MANIFESTS_OVERRIDE_HINT}",
source_root.display(),
missing.join(", "),
)
.into());
}
@ -1966,7 +2029,7 @@ fn dump_manifests_at_path(
Ok(())
}
Err(error) => Err(format!(
"failed to extract manifests: {error}\n looked in: {path}",
"failed to extract manifests: {error}\n looked in: {path}\n {DUMP_MANIFESTS_OVERRIDE_HINT}",
path = paths.repo_root().display()
)
.into()),
@ -8048,7 +8111,7 @@ fn print_help_to(out: &mut impl Write) -> io::Result<()> {
out,
" Diagnose local auth, config, workspace, and sandbox health"
)?;
writeln!(out, " claw dump-manifests")?;
writeln!(out, " claw dump-manifests [--manifests-dir PATH]")?;
writeln!(out, " claw bootstrap-plan")?;
writeln!(out, " claw agents")?;
writeln!(out, " claw mcp")?;
@ -9063,6 +9126,33 @@ mod tests {
);
}
#[test]
fn dump_manifests_subcommand_accepts_explicit_manifest_dir() {
assert_eq!(
parse_args(&[
"dump-manifests".to_string(),
"--manifests-dir".to_string(),
"/tmp/upstream".to_string(),
])
.expect("dump-manifests should parse"),
CliAction::DumpManifests {
output_format: CliOutputFormat::Text,
manifests_dir: Some(PathBuf::from("/tmp/upstream")),
}
);
assert_eq!(
parse_args(&[
"dump-manifests".to_string(),
"--manifests-dir=/tmp/upstream".to_string()
])
.expect("inline dump-manifests flag should parse"),
CliAction::DumpManifests {
output_format: CliOutputFormat::Text,
manifests_dir: Some(PathBuf::from("/tmp/upstream")),
}
);
}
#[test]
fn local_command_help_flags_stay_on_the_local_parser_path() {
assert_eq!(
@ -11554,43 +11644,78 @@ mod sandbox_report_tests {
#[cfg(test)]
mod dump_manifests_tests {
use super::{dump_manifests_at_path, CliOutputFormat};
use std::fs;
#[test]
fn dump_manifests_shows_helpful_error_when_manifests_missing() {
// Create a temp directory without manifest files
let temp_dir = std::env::temp_dir().join(format!(
let root = std::env::temp_dir().join(format!(
"claw_test_missing_manifests_{}",
std::process::id()
));
std::fs::create_dir_all(&temp_dir).expect("failed to create temp dir");
let workspace = root.join("workspace");
std::fs::create_dir_all(&workspace).expect("failed to create temp workspace");
// Clean up at the end of the test
let _cleanup = std::panic::catch_unwind(|| {
// Call dump_manifests_at_path with the temp directory
let result = dump_manifests_at_path(&temp_dir, CliOutputFormat::Text);
let result = dump_manifests_at_path(&workspace, None, CliOutputFormat::Text);
assert!(
result.is_err(),
"expected an error when manifests are missing"
);
// Assert that the call fails
assert!(
result.is_err(),
"expected an error when manifests are missing"
);
let error_msg = result.unwrap_err().to_string();
let error_msg = result.unwrap_err().to_string();
assert!(
error_msg.contains("Manifest source files are missing"),
"error message should mention missing manifest sources: {error_msg}"
);
assert!(
error_msg.contains(&root.display().to_string()),
"error message should contain the resolved repo root path: {error_msg}"
);
assert!(
error_msg.contains("src/commands.ts"),
"error message should mention missing commands.ts: {error_msg}"
);
assert!(
error_msg.contains("CLAUDE_CODE_UPSTREAM"),
"error message should explain how to supply the upstream path: {error_msg}"
);
// Assert the error message contains "Manifest files (commands.ts, tools.ts)"
assert!(
error_msg.contains("Manifest files (commands.ts, tools.ts)"),
"error message should mention manifest files: {error_msg}"
);
let _ = std::fs::remove_dir_all(&root);
}
// Assert the error message contains the expected path
assert!(
error_msg.contains(&temp_dir.display().to_string()),
"error message should contain the expected path: {error_msg}"
);
});
#[test]
fn dump_manifests_uses_explicit_manifest_dir() {
let root = std::env::temp_dir().join(format!(
"claw_test_explicit_manifest_dir_{}",
std::process::id()
));
let workspace = root.join("workspace");
let upstream = root.join("upstream");
fs::create_dir_all(workspace.join("nested")).expect("workspace should exist");
fs::create_dir_all(upstream.join("src/entrypoints"))
.expect("upstream fixture should exist");
fs::write(
upstream.join("src/commands.ts"),
"import FooCommand from './commands/foo'\n",
)
.expect("commands fixture should write");
fs::write(
upstream.join("src/tools.ts"),
"import ReadTool from './tools/read'\n",
)
.expect("tools fixture should write");
fs::write(
upstream.join("src/entrypoints/cli.tsx"),
"startupProfiler()\n",
)
.expect("cli fixture should write");
// Clean up temp directory
let _ = std::fs::remove_dir_all(&temp_dir);
let result = dump_manifests_at_path(&workspace, Some(&upstream), CliOutputFormat::Text);
assert!(
result.is_ok(),
"explicit manifest dir should succeed: {result:?}"
);
let _ = fs::remove_dir_all(&root);
}
}

View File

@ -174,13 +174,15 @@ fn dump_manifests_and_init_emit_json_when_requested() {
fs::create_dir_all(&root).expect("temp dir should exist");
let upstream = write_upstream_fixture(&root);
let manifests = assert_json_command_with_env(
let manifests = assert_json_command(
&root,
&["--output-format", "json", "dump-manifests"],
&[(
"CLAUDE_CODE_UPSTREAM",
&[
"--output-format",
"json",
"dump-manifests",
"--manifests-dir",
upstream.to_str().expect("utf8 upstream"),
)],
],
);
assert_eq!(manifests["kind"], "dump-manifests");
assert_eq!(manifests["commands"], 1);