fix: approval-required prints to stderr and exits 2 (was stdout + exit 0)

Merged
JW
3mo ago
+349-136

Problem

insta secrets --print --json > creds.json
echo $?   # 0 — script thinks it succeeded
cat creds.json
# approval required for secrets.read — run: insta approvals approve 64749a5a-…

Three contract violations at once, shared by every handleApproval caller (34 sites across branch/db/compute/deploy/secrets/storage/project/services):

  1. The approval hint went to stdout (handleApproval → info(), util.ts), so any redirect swallowed it.
  2. Callers did if (handleApproval(res)) return → exit 0 on a pending gate — scripts and &&-chains read "gated, nothing happened" as success.
  3. Under --json, the hint printed before the json branch, so stdout carried non-JSON.

Meanwhile run.ts used die() (stderr + exit 1) and compute exec's applyExecResult had its own 202 branch (envelope + exit 1) — three different behaviors for the same platform response.

Fix

handleApproval now owns the whole 202 contract in one place:

  • hint → stderr; stdout stays clean for redirects and parsers
  • process.exitCode = 2 — distinct from die()'s generic 1, so scripts/agents can branch on "approvable, re-run after approval"
  • --json → stdout carries the platform's raw 202 envelope (approvalId/action/nextActions), so scripted callers lift the id without scraping stderr

Call sites with a --json flag pass it through (22 sites); applyExecResult's 202 branch collapses into if (handleApproval(res, json)) return (exec's approval exit moves 1 → 2 — now distinguishable from the remote command's own exit 1, which keeps 0–255 pass-through untouched); run.ts's bespoke die() path is unified.

Verification

  • TDD: rewrote the handleApproval unit tests and the two applyExecResult 202 tests first (red), then implemented (green). Full suite: 64 files / 712 tests pass.
  • Live against prod with secrets.read=approve on a real project:
    • --json variant: redirected stdout = valid JSON envelope, hint on stderr, exit 2
    • original repro (--print > file): 0-byte stdout, hint on stderr, exit 2
    • cleanup: policy restored to allow, pending approval denied

Follow-up (separate PRs)

  • skills docs: document the exit-2 + stderr + json-envelope contract in the approval-relay section (insta-skills)
  • the platform's 202 message field itself says "have an admin run … then retry" — unchanged here

🤖 Generated with Claude Code

Summary by cubic

Approval-required responses now write the hint to stderr and set exit code 2. With --json, stdout carries the raw 202 envelope. This prevents scripts from treating a pending gate as success and keeps stdout parseable; --json is added across mutating commands with progress/diagnostics moved to stderr.

  • Centralizes 202 handling in handleApproval(res, json): stderr hint, process.exitCode = 2; with --json, stdout prints the envelope (approvalId, action, nextActions). Callers pass their --json flag through. compute exec now delegates to handleApproval (approval exit 1 → 2); run uses it and moves its banner to stderr.
  • Adds --json to: deploy, approvals approve/deny, project create/delete/link, branch create/delete/merge/switch, secrets set/unset, services remove, compute remove-domain, policy set, org create, env use. With --json, stdout emits one JSON document; progress and setup notes go to stderr.
  • Deploy --json: returns { image, machineId, url, branch, group, nextActions }; build progress tees to stderr via stderrBuildRunner. ensureFlyctl and the project auto-link note write to stderr to avoid polluting JSON.
  • Fixes compute remove-domain --json to print only the platform response via renderRemoveDomain. Stabilizes env use --json schema with envUseResult (same keys for no-op and real switch). project create --json hard-errors when no name is resolvable.

Migration

  • Treat exit code 2 as “approval required; retry after approval.” With --json, read the 202 envelope from stdout; do not scrape stderr.
  • When using --json, parse JSON from stdout and read progress/diagnostics from stderr. Update any && chains or success checks that assumed exit 0 on approvals; for compute exec, do not rely on exit 1 to mean approval.