Skip to content

feat: CLI audit + local/api parity: open-dispatch - #18

Open
andrei-hasna wants to merge 3 commits into
mainfrom
factory/669f8958-59ff-4265-9a2b-3fee768f-292c8ab5
Open

feat: CLI audit + local/api parity: open-dispatch#18
andrei-hasna wants to merge 3 commits into
mainfrom
factory/669f8958-59ff-4265-9a2b-3fee768f-292c8ab5

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Objective

CLI audit + local/api parity: open-dispatch

Repo: hasna/dispatch (local: /home/hasna/workspace/hasna/opensource/open-dispatch).

Audit this repo's CLI: (1) Find every DEAD command — commands that are broken, error out, reference removed code, or are declared but non-functional — and fix them or remove them with justification. (2) Find every command that only works in 'local' mode (on-box SQLite/files) and does NOT work against the 'api' (self-hosted/remote) path; implement full api-mode support for each so every command works identically against local AND api backends (route through the cloud-router/stage-A/HTTP path per this repo's existing pattern). Ensure both modes are fully implemented and tested. Add regression tests for the fixed commands and for local-vs-api parity. Open a PR; do not merge.

Verification

  • policy source: base cba1a55 (immutable commit — agent-proof)
  • ⚠️ GATE-INTEGRITY: agent touched verify-bearing config — REVIEW (src/cli/index.ts, src/index.ts, src/mcp/index.ts, src/sdk/index.ts)
  • install: pass
  • typecheck: pass
  • build: pass
  • test: pass

Run run_44b4d1862024 · backend codewith · task 669f8958-59ff-4265-9a2b-3fee768f9f8c
🏭 Generated by @hasnaxyz/factory


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…uests

Three reviewer findings on the local/api parity slice.

1. In API mode the CLI decided "did this route to the remote authority?" from the
   truthiness of the resolved response. withApiDaemon()/withApiClient() signalled
   "not API mode" with undefined, and request() also resolved undefined for an
   empty body, so a 204 or null answer from the authority silently handed the
   command to the local box: `daemon status`/`doctor` printed local SQLite daemon
   state, and `fleet summary` invoked the local tmux runner. Both helpers now
   return DispatchApiRouteResult<T>, whose `routed` flag comes from the route
   decision alone, and request() raises REMOTE_API_EMPTY_RESPONSE for a body-less
   or null success instead of resolving undefined.

2. Side-effecting POSTs (/dispatches, /dispatches/bulk, /exec, /keys, /recover)
   were retried up to 3x — including after a client-side timeout — on the strength
   of an Idempotency-Key contract no authority in this repo implements or is tested
   against. `dispatch exec` against a slow authority could submit a command into a
   live shell pane three times. Only HTTP-idempotent methods are retried now, and
   an in-flight abort/timeout is never replayed. The header is still sent as
   advisory metadata for the authority.

3. The credential-exfiltration assertion in api-client.test.ts was a tautology:
   its `|| call.headers.Authorization === "Bearer secret"` arm was true for every
   call, so it passed even against a fixture leaking the key into the request body.
   Replaced with a predicate over url+body (auth headers excluded) plus a negative
   control proving the predicate can fail.

Docs updated to match: README "Local and API mode" and docs/architecture.md
"Client route".
getDispatchApiConfigStatus validated both HASNA_DISPATCH_STORAGE_MODE and
its lower-precedence DISPATCH_STORAGE_MODE alias, so a stale or invalid
alias returned {selected: true, ok: false} even when the canonical name was
set and valid. Every CLI, MCP and SDK entry point funnels through this
function, so a leftover DISPATCH_STORAGE_MODE=sqlite hard-failed the whole
tool — including the default local mode, which never reads either variable.

Resolve precedence first via firstEnv (the same rule the API URL/key pair
already uses), then validate only the variable that actually won. An alias
that is never consulted can no longer veto the mode that outranks it.
Invalid values still fail closed when that variable is the one in effect.

Covers both directions: the new precedence tests fail without the fix, and
the negative control fails if the validation is dropped instead of scoped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant