feat(core): add clap CLI with migrate subcommand to agentd-core binary

[geoffjay]

Restructures main.rs to use clap with serve (default) and migrate status|up|down subcommands. Adds --db-path override for isolated testing, TTY/--yes guard on down, and human-friendly output. Adds clap 4 and atty 0.2 dependencies.

Closes #1125

[geoffjay]

This change is part of the following stack:

• feature/core #1135
  • feat(common): add rollback_migrations helper to agentd_common::storage #1130
    • feat(core): expose apply/status/rollback migration helpers from lib.rs #1131
      • feat(core): add clap CLI with migrate subcommand to agentd-core binary #1132 ◀
        • test(core): add integration tests for agentd-core migrate subcommand #1133

_{Change managed by git-spice.}

[geoffjay]

Review: feat(core): add clap CLI with migrate subcommand to agentd-core binary

Two blocking issues before this can merge. The overall structure and logic are solid — the subcommand design, TTY guard, output strings, and --db-path override are all correct and well-implemented. Cannot self-approve/request-changes; marking needs-rework via label.

Blocking: replace atty with std::io::IsTerminal

atty = "0.2" carries RUSTSEC-2021-0145 (soundness advisory — unsound use of from_raw_fd). It is also unmaintained. More importantly, the project already uses the stdlib alternative: crates/cli/src/picker.rs imports std::io::IsTerminal. Adding a known-unsound crate when the codebase already has the correct pattern is not acceptable.

Fix — remove atty from Cargo.toml entirely, then in run_migrate_down:

// Before
let is_tty = atty::is(atty::Stream::Stdin);

// After
use std::io::IsTerminal;
let is_tty = std::io::stdin().is_terminal();

No other changes needed — the call-site logic is correct as written.

Blocking: use the workspace clap instead of a local version spec

The workspace already declares clap = { version = "4.5", features = ["derive"] } in [workspace.dependencies]. This PR adds its own clap = { version = "4", features = ["derive"] } — a different version spec that bypasses the workspace pin.

Fix in crates/core/Cargo.toml:

# Before
clap = { version = "4", features = ["derive"] }

# After
clap = { workspace = true }

The workspace entry already includes the derive feature, so no local feature declaration is needed.

Non-blocking: init_tracing() runs for all subcommands

agentd_common::server::init_tracing() is called unconditionally in main before dispatch. For migrate status/up/down this initialises the full tracing stack when the user likely only wants clean line-oriented output. Won't cause bugs (tracing goes to stderr; integration test stdout assertions still pass), but it's slightly heavy for a CLI utility path. Consider moving the call inside run_serve() only. Non-blocking, can be a follow-up.

Stack note

This branch (issue-1125) is stacked on PR #1131 (issue-1124), which is LGTM and can merge first. The integration tests in #1133 depend on this PR's CLI shape (subcommand names, --db-path flag, output strings) — those all remain correct and will not need changes once the two blocking issues above are fixed.

[geoffjay]

Review: feat(core): add clap CLI with migrate subcommand to agentd-core binary

Stack Position

issue-1127 (#1133) — test(core): integration tests  [needs push, child]
  └── issue-1125 (#1132) ← this PR
        └── issue-1124 (#1131) — expose migration helpers  [merge-queue ✓]
              └── issue-1123 (#1130)  [merge-queue ✓]

Parent #1131 is already in merge-queue. Once it lands, this PR is unblocked. Tests for this feature are in child PR #1133 — that split is clean.

---

API Correctness

I cross-checked each call against lib.rs on issue-1124:

Call	Signature	Verdict
migration_status_for_path(&db_path)	-> Result<Vec<(String, bool)>>	✓
apply_migrations_for_path(&db_path)	-> Result<()>	✓
rollback_migrations_for_path(&db_path, None)	None = roll back all	✓
rollback_migrations_for_path(&db_path, Some(1u32))	Some(n) = n most-recently-applied	✓

The steps computation (None for --all, Some(1) for single step) correctly matches the documented semantics.

---

TTY / Confirmation Guard

The run_migrate_down guard is correctly implemented:

let is_tty = std::io::stdin().is_terminal();
if !is_tty {
    anyhow::bail!("stdin is not a TTY — pass --yes to confirm rollback without a prompt.\n\
                   Would roll back: {}", targets.join(", "));
}

Uses std::io::IsTerminal (stable since Rust 1.70) rather than the deprecated atty crate — better than what the PR description claims. The error message is actionable: it tells the caller exactly what flag to pass and what would be affected.

---

One Non-Blocking Note

PR description inaccuracy: The description says "Adds clap 4 and atty 0.2 dependencies." No atty dependency is added (correct!), and std::io::IsTerminal is used instead. Worth updating the description to avoid confusion for anyone reading the history, but no code change needed.

Minor: run_serve calls agentd_common::storage::get_db_path("agentd-core", "core.db") directly instead of resolve_db_path(None). Functionally identical since resolve_db_path(None) does exactly that, but using the helper would make run_serve consistent with the rest of the file. Non-blocking.

---

What's Working Well

• Clean clap 4 derive structure: Option<Command> default → serve, nested MigrateAction subcommands, --db-path override on every migrate action for isolated testing
• The TTY guard is applied before the destructive call, not after
• applied.is_empty() early-return prevents the index operation on an empty vec
• run_migrate_up re-queries status after applying to report final counts rather than trusting the pre-run snapshot
• init_tracing() called before Cli::parse() — correct initialization order
• No atty crate: avoids the known soundness issue in that crate

---

Summary

No blocking issues. The implementation is correct, the destructive-action guard is properly implemented, and the clap structure is clean. Approved pending resolution of parent #1131 (already in merge-queue).