mando-cli full codebase audit (2026-07-22)

Full-codebase audit of mando-cli (~18.7k lines) done in four parallel passes — bugs (runtime+commands), bugs (workspace/support), over-engineering, and duplication+gaps. Every finding was verified against source by the reporting agents; no fixes were applied. Source of truth in-repo: /Volumes/bandi/coding/poc/mando-cli/.agents/research/2026-07-22-audit-mando-cli.md.

For Agents

This note is a triage index, not a task list — it records 37 verified bugs (2 HIGH / 18 MEDIUM / 17 LOW), a net −280-line / −5-dep over-engineering cut list, 9 duplication clusters, and the still-missing feature set (notably the entire simulator runtime spec being unimplemented on main). File:line refs are as of the 2026-07-22 audit against main; re-verify before acting since the piped-output fix (opts.rs, compose.rs) is still uncommitted and may shift line numbers. Recurring bug themes: exit-code-0-on-failure (migrate/get/pull), missing reqwest timeouts everywhere, and wiremock.enable() is a silent no-op.

Two HIGH-severity findings

(1) Unguarded mass-kill. host_process::stop() runs kill -TERM -{pid} with a pid read unverified from host-procs.json and no pid <= 1 guard — a stale/corrupt 0 signals the CLI’s own process group, and 1 becomes kill -TERM -1 (every process the user may signal). src/runtime/host_process.rs:108-133, via commands/down.rs:95,143,194. (2) Orphaned host processes on record failure. Host processes spawn detached (kill_on_drop=false, up.rs:252) but pids persist to host-procs.json only after the whole loop; if host_process::record fails, ? returns early and the processes run untracked forevermando down can never stop them. src/commands/up.rs:262-270.

Fix status

Block 1 landed in the working tree — 2026-07-22 (uncommitted by design; user commits manually)

First fix block closed both HIGH findings plus the three exit-code-0-on-failure MEDIUMs. Triple review (quality/bugs/dup) approved; re-review closed all residuals.

  • Bug #1 (HIGH, mass-kill) — fixed. host_process::stop() now rejects pid <= 1 before signaling. Guard sits in the shared stop(), so all callers are covered: down.rs (×3) and the up.rs spawn-cleanup path.
  • Bug #2 (HIGH, orphan leak) — fixed. up.rs wraps read+merge+record persistence in a result-capturing closure; on any persistence failure it SIGTERMs all just-spawned pids before propagating the error, so no host process is left untracked.
  • Bugs 5 (MEDIUM, exit-0-on-failure) — fixed. migrate / get / pull now exit nonzero on any failure via a shared exit_on_failure(bool) helper in commands/mod.rs returning CommandError::Exit(1).

Known-red clippy baseline (accepted for now)

8 pre-existing dead-code errors in schema.rs / backend.rs / project.rs are unchanged — slated for the over-engineering cleanup block (see Over-engineering (ponytail pass) — net −280 lines, −5 deps).

Parked follow-ups: host-procs.json non-atomic write (bug #19 class); command-level exit-code wiring is untested.

Block 2 landed in the working tree — 2026-07-22 (uncommitted by design; user commits manually)

Second fix block closed the --volumes / volume clear no-ops, the hardcoded-port stub count, the missing-timeout cluster, and the two WireMock findings. Triple review (quality/bugs/dup) approved; one Important surfaced and re-verified closed.

  • Bug #7 (MEDIUM, --volumes dropped per-service) — fixed. mando down <svc> --volumes now honored: named volumes discovered via docker inspect and removed explicitly, anonymous volumes via compose rm -v.
  • Bug #8 (MEDIUM, volume clear silent no-op) — fixed. Reordered to compose rm -s -f before volume rm so removal actually succeeds. Container discovery for both down and clear now goes through a shared service_container_ids helper using compose ps -aq (was ps -q, running-only) — fixes the stopped-container silent no-op. (This was the block’s one Important review finding — ps -q still missed stopped containers; re-verified CLOSED after switching to ps -aq.)
  • Bug #11 (MEDIUM, hardcoded stub-count port) — fixed. status stub-count now resolves WIREMOCK_PORT env (mirrors compose ${WIREMOCK_PORT:-8080}) instead of hardcoded 8080, with 2s connect / 3s total timeouts.
  • Bugs 13 (MEDIUM, missing reqwest timeouts) — fixed. All reqwest clients now carry timeouts via a new system/http.rs client(connect, total) helper: wiremock/check 2s/5s, rest 5s/60s, install connect-only 10s (leaving download body unbounded to protect slow links).
  • Bug #14 (MEDIUM, wiremock.enable() no-op) — fixed. Renamed enable()verify_service_stubs — it never toggled state and had no CLI caller, so validation-only semantics are now truthful rather than a speculative admin-API toggle.
  • Bug #30 (LOW, reset() ignores HTTP status) — fixed. wiremock.reset() now checks HTTP status on both admin calls.

Accepted minors (Block 2)

WIREMOCK_PORT env-file vs shell-var divergence left as-is; rest.rs now caps previously-unbounded queries at a 60s hard total. Gate: 347 tests green, clippy unchanged at the 8 known-baseline errors only.

Block 3 landed in the working tree — 2026-07-22 (uncommitted by design; user commits manually)

Third fix block hardened data-safety paths: flyway connection building, DB mutation ordering, git token exposure, atomic writes, and workspace-lock liveness. Triple review (quality/bugs/dup) approved; one reviewer-caught Important on the self-update path re-verified closed.

  • Bug #15 (MEDIUM, flyway raw-format! connection strings) — fixed. All 3 Postgres connections now built via one shared pg_config() using tokio_postgres::Config typed setters — hostile env values (spaces/quotes/backslashes) can no longer break libpq keyword parsing.
  • Bug #16 (MEDIUM, flyway mutate-before-bail) — fixed. apply() now runs check_flyway before ensure_history_table, so it no longer mutates a Flyway-managed DB before bailing.
  • Bug #34 (LOW, spurious CREATE DATABASE) — fixed. ensure_database only issues CREATE on a genuinely empty result; real query errors now propagate instead of being masked as “db absent”.
  • Bug #17 (MEDIUM, git token on argv) — fixed. Clone token moved off argv into GIT_CONFIG_COUNT/GIT_CONFIG_KEY_0/GIT_CONFIG_VALUE_0 env — no longer visible in ps (requires git ≥ 2.31).
  • Bug #18 (MEDIUM, EXDEV self-update) — fixed. Cross-device path now stages into the destination dir + atomic rename — fixes both the non-atomic overwrite of the running binary and ETXTBSY on Linux tmpfs. (This was the block’s one Important review finding on the first implementation; re-verified CLOSED.)
  • Bug #19 (MEDIUM, non-atomic state write) — fixed. New shared atomic_write (temp with pid+seq, then rename) protects .bessstate.json and host-procs.json from torn writes.
  • Bug #10 (MEDIUM, workspace-lock steals live locks) — fixed. Lock probe now treats EPERM as alive (no stealing live locks from other users); ESRCH predicate shared via kill_stderr_is_esrch.

Accepted minors / parked follow-ups (Block 3)

Gate: 362 lib tests green, clippy unchanged at the 8 baseline errors (slated for Block 4, in flight). Parked: REPO_PYMANDO_TOKEN still exposed as a docker build-arg (build_args.rs:95, pre-existing); .bess-credentials.json write_restricted still non-atomic (needs a 0600-aware atomic write); locale-sensitive ESRCH match means stale locks aren’t auto-cleared under non-English LC_MESSAGES (conservative).

Block 4 landed in the working tree — 2026-07-22 (uncommitted by design; user commits manually)

Fourth block executed the over-engineering cut list and cleared the known-red clippy baseline. Triple review (quality/bugs/dup) approved.

  • Deps dropped: bollard, bytes, tar, log, comfy-table removed from Cargo.toml.
  • Files deleted: runtime/docker_client.rs, workspace/git/git.rs, network/manager.rs, workspace/projects/{algo,dash,iac}.rs.
  • bollard inspect replaced by docker inspect --format '{{json .}}' shell-out — status --detailed rendering verified byte-identical by reviewer.
  • Single-impl traits collapsed: MockBackend→concrete WireMockBackend, MigrationRunnerFlywayMigrationRunner, ProjectContext→inherent fn, JoinProject inlined.
  • Dead layers deleted: per-project wrapper layer + 5 never-read MandoWorkspaceProject fields removed; query-feature schema types now cfg-gated.
  • Clippy baseline cleared: went 8 errors → ZERO under -D warnings.

Block 5 landed in the working tree — 2026-07-22 (uncommitted by design; user commits manually)

Fifth block made the two parsed-but-ignored mock up flags real, added a Dockerfile COPY-source preflight guard, recorded the default-profile decision, and corrected a false audit claim. Triple review (quality/bugs/dup) approved — 0 Critical/Important, 4 minors fixed.

  • mock up -p/--port <n> — now real. Injects WIREMOCK_PORT into the compose child env (template publishes ${WIREMOCK_PORT:-8080}:8080); warns via a compose port check if the container already runs on a different port; all admin calls target the requested port.
  • mock up --dir <path> — now real. Waits for WireMock readiness (30s cap), resets mappings, then loads sorted *.json stubs via admin API with per-file Pass/Fail rows and nonzero exit on any failure. Bare mock up is unchanged.
  • Clean-clone preflight guard — NEW src/runtime/preflight.rs. Dockerfile COPY-source guard wired into up and build (runs after cargo prebuild so it can’t block its own remedy). On a clean clone mando up now fails fast with the exact fixes (mando build mando --mando=artifact --cargo or --mando=pull) instead of a confusing docker COPY error. Handles comments (incl. inside line continuations), --from stages, wildcards, $vars, JSON-form COPY; best-effort skips unreadable files.
  • Decision — default profile mando stays →build (NOT flipped to pull). Evidence: pull.yml resolves registry-less ${MANDO_IMAGE:-mando:dev} to a bare Docker Hub ref, and private ECR needs a login — flipping would trade one clean-clone failure for another.
  • Audit correction — “mock reset does NOT clear the request log” was WRONG. WireMockBackend::reset DELETEs /requests too; mock-guide.md is now truthful-to-code (also fixed the mando down wiremockmando down mando-wiremock example).

Minors fixed / gate (Block 5)

4 review minors closed: fetch_stub_count now delegates to list_stubs; new WIREMOCK_CONTAINER_PORT const; down() uses WIREMOCK_SERVICE; comment-inside-line-continuation parsing hardened + test. Gate verified by orchestrator: build clean, clippy ZERO, 385 lib + 8 integration + smoke tests, 0 failed.

Session wrap — 2026-07-22

5 blocks (4 audit-fix + 1 feature); 22 audit findings fixed + review-caught issues (including an ETXTBSY self-update hazard caught in review). Final gate verified: build clean, clippy zero, 385 lib + 8 integration + smoke tests all green. Tree left uncommitted for the user. Open decisions recorded in the repo ledger .superpowers/sdd/progress.md: broken pre-existing --features query build (flight.rs vs mando-core drift), unimplemented simulator-runtime spec; remaining LOW sweep unfixed.

Validation pass (evening)

Full 4-judge council — PASS after one fix round (2026-07-22)

A full council of four judges (error-paths, spec-compliance / desired-output, DRY/SOLID, Alpiq-standards) validated the entire uncommitted tree and reached PASS after a single fix round. Report in-repo: .agents/council/2026-07-22-vibe-mando-cli.md.

Process learning — the two Important findings were CROSS-BLOCK seams

Both survived per-block triple review because they only appear when two blocks’ changes interact — invisible to any single block’s reviewers. Worth watching for in future multi-block work. (1) Datadog flag-off silent restart. The Datadog-off path did remove_file(...).ok(), and compose inclusion is existence-driven — so a failed delete would silently restart the agent on a plain mando up. Fixed: non-NotFound delete errors now abort with an actionable message. (2) remove_named_volumes exit-code lie. It rendered Fail rows but still exited 0, violating the exit_on_failure convention established the same session. Fixed: Result<bool> threaded through volume clear + down --volumes.

Also fixed in this pass: shared system/path::normalize (dedups preflight/init); runtime/paths.rs extracted out of the build_args.rs grab-bag; wiremock fetch status guards; http.rs panic→fallback; install.rs 300s download cap; preflight now skips COPY sources that escape the build context; teardown warns instead of failing hard.

Alpiq-standards verdict

0 non-test unwraps, 0 unsafe, format-args clean, Datadog feature BE-3482-conformant. No mando-cli-specific standards doc exists — validated against the org Datadog-conformance guidance + mando AGENTS.md conventions.

Final gate: clippy ZERO, 395 lib + 395 bin + 8 integration + smoke, 0 failures.

User-only remaining

Junk-file cleanup before commit (see Housekeeping); manual Datadog acceptance.

Bugs — 37 verified (2 HIGH, 18 MEDIUM, 17 LOW)

HIGH

#file:linebugproposed fix
1runtime/host_process.rs:108-133 (via commands/down.rs:95,143,194)stop() runs kill -TERM -{pid} with pid unverified from host-procs.json; no guard for pid <= 1 — stale 0 signals the CLI’s own process group, 1 becomes kill -TERM -1.Reject pid <= 1 (treat as already-gone) before signaling; validate pid range.
2commands/up.rs:262-270Host processes spawn detached (kill_on_drop=false, L252); pids persist only after the whole loop. If record fails, ? returns early and processes run untracked forever.Persist each pid immediately after spawn, or SIGTERM just-spawned procs on record failure.

MEDIUM

#file:linebugproposed fix
3commands/migrate.rs:174-229Failed migrations render as Fail rows but apply always returns Ok(()) → exit 0 on failure; CI can’t detect it.Track failures, return CommandError::Exit(1).
4commands/get.rs:58-68Clone failure rendered but execute returns Ok(()) → exit 0.Return Err/Exit(1) in the Err arm.
5commands/pull.rs:101-118Exit 0 even when every pull failed.Non-zero when any result is PullResult::Failed.
6commands/init.rs:186-193SSH→HTTPS fallback passes &None creds — anonymous retry always fails for private repos, defeating the fallback.Resolve creds for the fallback URL.
7commands/down.rs:116-215--volumes only honored in whole-stack branch; mando down <svc> --volumes silently drops the flag.Pass -v to compose rm or error out.
8commands/volume.rs:226-309clear stops but doesn’t remove the container, so docker volume rm fails “in use”, mapped to Warn — command silently no-ops.compose rm -sfv <svc> before volume rm.
9runtime/host_process.rs:107-133No verification pid/pgid still belongs to the recorded service; PID reuse between up and down SIGTERMs an unrelated process group.Record start-time/comm at spawn, re-verify before kill.
10runtime/workspace_lock.rs:133-143is_pid_alive via kill -0 treats EPERM (live process, other user) as dead → steals active locks in shared workspaces.Distinguish EPERM vs ESRCH.
11commands/status.rs:220,372-381Stub-count fetch hardcodes port 8080; custom WIREMOCK_PORT silently gets no stub count.Thread resolved wiremock port into fetch_stub_count.
12commands/status.rs:373-375reqwest::get with no timeout; hung WireMock admin endpoint hangs mando status indefinitely.Client with short .timeout(...).
13network/wiremock.rs:70, query/rest.rs:22, update/check.rs:29, update/install.rs:22No timeout on any reqwest client; binary download has no cap at all.Client::builder().timeout().connect_timeout().
14network/wiremock.rs:142-161enable(service) fetches stubs and returns Ok — never enables anything; silent no-op, callers believe state toggled.Implement via admin API or rename to a validation check.
15db/flyway.rs:35-37,55-58,105-108Connection strings via raw format! from env vars; space/quote/backslash breaks libpq keyword parsing.Build tokio_postgres::Config programmatically.
16db/flyway.rs:281-283apply() creates mando schema/history table before check_flyway() — mutates a Flyway-managed DB before bailing (status() has correct order).Check first, create after.
17workspace/git/git_handler.rs:131Token passed as -c http.extraHeader=PRIVATE-TOKEN: <token> on argv — visible in ps during clone.Credential helper or GIT_CONFIG_* env.
18update/install.rs:74-76EXDEV (cross-device rename) mis-routed to sudo mv; TMPDIR on another mount → spurious sudo prompt on every self-update.On EXDEV: fs::copy + remove_file.
19workspace/state.rs:310-312.bessstate.json written non-atomically; crash mid-write corrupts workspace state.Temp file + rename.
20commands/errors.rs:30-31Default path doc/errors.json vs documented docs/errors.json (errors-guide.md:21 says doc/, cli.rs:217 says docs/ — the two disagree).Align code, help text, and errors-guide.md.

LOW

#file:linebug
21runtime/compose.rs:82-167assemble(build_only) appends overrides for filtered-out services.
22commands/up.rs:206-207Filtered up <svc> rewrites override.builtin.yaml dropping other services’ entries.
23commands/exec.rs:39, logs.rs:35No mocked→{svc}-mocks name mapping unlike up/down.
24runtime/build_args.rs:152-160build_log_path dead.
25runtime/runprofile.rs:555-576discover_legacy_files nondeterministic winner on duplicate profile names.
26commands/config.rs:77while let Ok(Some(...)) silently truncates restore all on first read error.
27commands/release.rs:186unwrap_or_default() on CHANGELOG read → unreadable file treated as empty, history dropped.
28commands/override_cmd.rs:200-204Branch passed positionally to git worktree add; --leading name read as flag (mitigated by prior rev-parse).
29commands/migrate.rs:39-45toggle mutates state without workspace lock (races with up/down).
30network/wiremock.rs:251-263reset() ignores HTTP status — 4xx/5xx reported as success.
31network/wiremock.rs:230,237url.contains(service) substring match hits unrelated requests.
32workspace/projects/ctx.rs:35-49Default bessdef written to dir path, not the yaml file (latent, gated off).
33config/adapter/dotenv.rs:54trim_matches('"') strips all quotes, mangles values with meaningful quotes.
34db/flyway.rs:128-139Any query error treated as “db absent” → spurious CREATE DATABASE masking real error.
35query/flight.rs:185-187Negative nanosecond remainder for pre-1970 timestamps wraps u32 → falls back to raw int.
36workspace/git/git_handler.rs:69-85Rename porcelain entries yield “old new” as path.
37runtime/docker_client.rs:1-110Whole module #[allow(dead_code)] bollard leftover (see over-engineering #1).

Uncommitted piped-output fix — verified correct

The uncommitted diffs in src/opts.rs and src/runtime/compose.rs (the piped-output invisible-failures fix) were re-verified in this audit: no double-print, and the compose_cmd refactor preserves arg order / cwd / env. But it still lacks a regression test and a commit.

Over-engineering (ponytail pass) — net −280 lines, −5 deps

  1. native: bollard dep exists for one inspect_detail call in status --detailed — replace with docker inspect --format '{{json .}}' via system/cmd.rs. (runtime/docker_client.rs)
  2. delete deps with zero references: bytes, tar, log, comfy-table (unused even under --features query). (Cargo.toml)
  3. shrink: MockBackend — 9-method async trait, one impl (WireMock). Collapse to concrete. (network/backend.rs:43)
  4. shrink: MigrationRunner — 3-method trait, one impl (Flyway), Box<dyn>. Collapse. (db/runner.rs:53)
  5. delete: NetworkDependencyManager builder surface (with_backend/with_stub_dir/stub_dirs) — no non-test callers. (network/manager.rs:35)
  6. delete: MandoWorkspaceProject.network field + accessor — never read. (workspace/project.rs:128)
  7. shrink: JoinProject trait → free fn/inline. (workspace/project.rs:108)
  8. shrink: ProjectContext trait, one impl. (workspace/projects/ctx.rs:75)
  9. delete: DockerClient::ping(), StubLoadResult.{file,stub}, GitRepository wrapper.

Do NOT cut

EnvironmentDef / SavedQuery / etc. dead-code warnings — they live under the query feature and are load-bearing there.

Duplication — 9 clusters

  1. Compose-context prologue (default_profilerunprofile::loadbuild_project_indexcompose::assemble) copy-pasted across 8 command files: logs.rs:30, exec.rs:34, volume.rs:183, down.rs:87, profile.rs:119, mock.rs:62, up.rs:185, build.rs:133 → extract compose::prepare(ctx, profile_name, build_only). (biggest cluster)
  2. statusCommandError tail byte-identical in logs.rs:46 / exec.rs:51exit_from_status(ExitStatus).
  3. Ad-hoc Cmd::new("docker") + nonzero-exit error in volume.rs:113,243,278 vs compose.rs:221Cmd::run_checked(label).
  4. Order-preserving dedupe duplicated: compose.rs:335 / volume.rs:99 → generic dedupe<T>.
  5. Results-summary render block in down.rs:217, build.rs:226, up.rs:276, volume.rs:311buffer.render_results(results).
  6. Header+profile KV preamble down.rs:98 / volume.rs:191command_header(title, profile).
  7. Profile-load-with-infra-fallback down.rs:58 vs volume.rs:184runprofile::load_or_infra.
  8. Process signalling outside Cmd: workspace_lock.rs:136, host_process.rs:109 (+install.rs:102, main.rs:98) → route through Cmd/signal(pid,sig).
  9. compose-failure string built twice compose.rs:224 / :291 → shared formatter.

Missing / incomplete features

Simulator runtime entirely unimplemented on main

The simulator runtime (2026-05-28 spec + plan) has zero implementation on main — no simulator refs, no compose_project field, no Project::SimulatorRunner. It exists only on the feature/simulator-runtime branch. See also the simulator env contract.

  • mock up --dir <path>: flag parsed, ignored (cli.rs:415 vs commands/mock.rs:47,59). Same for -p/--port.
  • mock up documented stub-reset + mapping-load via admin API: only runs compose up -d mando-wiremock (mock-guide.md:33 vs mock.rs:59-82).
  • mock reset doc says “clears stubs and request log”: actually reloads mappings from disk, log untouched (mock-guide.md:99 vs wiremock.rs:251).
  • Doc mismatches: mando down wiremock example (service is mando-wiremock), README “four runconfigs” lists five, errors.json path doc-vs-code (bug #20).
  • Empty ProjectDef {} + dead per-project context layer (algo/dash/iac wrappers, method-less, #[allow(dead_code)]) — delete or fill.
  • state.artifacts / BuiltArtifact: always empty, no producer (state.rs:39,183).
  • OTLP telemetry: explicit skip, crate never added (telemetry/tracing_setup.rs:47).
  • Default-profile clean-clone gap NARROWED but present: mando-mocked-algos maps mando→build against the thin Dockerfile; escape hatches --mando=pull / --mando=artifact --cargo are wired. (See mando-cli-v0.4.0-mando-bess-binary-missing-2026-06-25.)
  • Resolved since last audit (don’t re-flag): runner impls, workspace bootstrap, artifact build path, docker_client wired to status --detailed.

Test coverage risk

Dense unit tests cover pure logic (compose::assemble, cmd, parsers, runprofile, build_args, templates). Riskiest untested paths: compose run/run_inherit/run_capture execution, volume clear + down side-effects, host_process spawn/stop signalling, workspace_lock stale-lock reclaim, wiremock admin calls.

Test-gate gotcha

cargo test --bin mando skips the tests/ directory — use plain cargo test. (Same class of --lib-only blind spot documented for the main workspace in mando-ci-lib-only-test-gate-2026-07-22.)

Housekeeping

Untracked junk in repo root to clean: 1, out.log, mando_errors.json, metis-wiremock.zip, dist/, .ai/. The uncommitted piped-output fix (opts.rs, compose.rs) is verified correct but needs a commit + regression test.