Skip to content

bcli: mutation commands compute the correct exit-code taxonomy for --result-out but exit the OS process with 1 anyway #38

Description

@igor-ctrl

Summary

bcli's documented exit-code taxonomy (bcli.exit_codes, 0–8) is computed correctly and written correctly into the --result-out/--result-fd envelope, but the OS-level process exit code for every mutating command (post, patch, delete, attach upload, action) is hardcoded to 1 regardless of what the envelope says. A caller branching on the raw process exit code ($?) cannot distinguish "not found" from "remote 4xx" from "remote 5xx" — every failure looks like exit 1 ("uncategorised error"), even though bcli describe's own exit_codes field and AGENTS.md/CHANGELOG.md promise otherwise.

The bug is not batch-specific — it reproduces on plain single-command post/patch/delete/attach upload/action invocations, which is where it was actually observed. bcli batch run's own aggregate-failure envelope happens to not exhibit a visible mismatch (both sides land on 1), but that's incidental, not because the aggregate path is exempt from the same discard pattern (see Root cause, item 5).

Steps to reproduce

Live sandbox reproduction from Turbine Slice 0 / Spike 4 (admin-sandbox, SBEnvAug26, company LLC):

bcli --profile admin-sandbox --env SBEnvAug26 --company LLC --format json \
  patch salesInvoiceHeaders aa3d5b06-7f26-4284-82fb-99df814fb071 \
  --data '{"shipToName":"TURBINE-SPIKE4-PROBE-SHOULD-4XX"}' \
  --yes --etag '*' \
  --result-out /tmp/patch-result.json
echo "process exit code: $?"
cat /tmp/patch-result.json

Observed:

  • echo $?1
  • /tmp/patch-result.json"status": "failed", "exit_code": 6 (remote 4xx — BC rejected the PATCH)

Evidence on disk (this exact run): docs/slice0/evidence/04-write-contracts/result-out/patch-1787530378623-n7nq6y.json (envelope, exit_code: 6) cross-referenced with the harness's own process-level log at docs/slice0/evidence/04-write-contracts/session-commands.log:206 ("exitCode":1,"exitLabel":"unknown(1)" for the same --result-out path). The same mismatch shape recurs across the run for every failing post/patch/action invocation in that log (e.g. post-1787529614520-tuc5cd.json, action-1787530623000-alomda.json).

Same code path — same bug — reproduces identically for bcli post and bcli delete (see Root cause below); only patch happened to be the one exercised against a real 4xx in the live run.

Expected

Per AGENTS.md's "Exit code taxonomy" section and bcli describe's exit_codes field, the process exit code — not just the envelope's exit_code field — should carry the taxonomy value (4 not found, 6 remote 4xx, 7 remote 5xx, etc.), so a caller can case $? in 6) ... ;; 7) ... ;; esac without ever opening the envelope. CHANGELOG.md's "Policy refusal exit code 1 → 8" entry explicitly frames this as a promise about the process exit code, not just the JSON payload — and that specific case (policy refusal) is the one case that actually works end-to-end today (see Root cause item 4), which is why the regression on the other paths was not caught.

Root cause

The taxonomy is computed twice, in two different places, and only one of the two call chains reaches sys.exit() with the computed value:

  1. src/bcli_cli/_envelope_wrap.py:228-248 (Capture.emit_failure) — correctly computes exit_code = exit_code_for_status(getattr(exc, "status_code", None)) and writes it into the envelope via _build_envelope(..., exit_code=exit_code, ...). This is the value that ends up in --result-out.

  2. Immediately after calling emit_failure, every mutating command's except block discards that computed value and hardcodes the OS-level exit instead:

    • src/bcli_cli/commands/post_cmd.py:96-99
      except Exception as e:
          cap.emit_failure(e)
          console.print(f"[red]Error:[/red] {e}")
          raise typer.Exit(1)
    • src/bcli_cli/commands/patch_cmd.py:98-101 — identical pattern.
    • src/bcli_cli/commands/delete_cmd.py:94-97 — identical pattern.
    • src/bcli_cli/commands/attach_cmd.py:153-156 (the upload subcommand) — identical pattern.
    • src/bcli_cli/commands/action_cmd.py:220-223 — identical pattern. This is not a batch-only or hypothetical gap: it's the exact call site behind the evidence cited above (action-1787530623000-alomda.json, envelope exit_code: 6; session-commands.log:236, process exitCode: 1).

    typer.Exit(1) raises SystemExit(1). That propagates straight up through the Typer app() dispatch to main()'s outer handler at src/bcli_cli/app.py:295-298:

    except SystemExit:
        # ``typer.Exit`` raises SystemExit — propagate to let the
        # configured exit code surface unchanged.
        raise

    — which passes the hardcoded 1 straight through to the OS, unchanged, exactly as its own comment says it will.

  3. Separately, src/bcli_cli/_error_handler.py:47-69 (map_error_to_exit_code) implements the full taxonomy — including the parts exit_code_for_status alone can't reach (AuthError → 3, NotFoundError/RegistryError → 4, ValidationError → 5, ConfigError → 2, SafetyError → 8) — and src/bcli_cli/app.py:299-344 calls it correctly, but only for a BCLIError that escapes app() entirely uncaught. Because every mutating command's local except Exception at (2) intercepts the exception first and re-raises a plain typer.Exit(1), main()'s except SystemExit: raise (line 295-298) fires before its except BaseException / map_error_to_exit_code branch ever gets a chance — the two code paths for computing "the right exit code" never talk to each other for these five commands.

  4. This is why the one taxonomy value that does reach the OS process correctly is EXIT_POLICY (8): src/bcli_cli/_safety.py:65 and :74 (confirm_write_or_exit) call raise typer.Exit(EXIT_POLICY) directly at the raise site, before the command's local except Exception wrapper is even entered (the confirm call sits outside the try: block in post_cmd.py/patch_cmd.py/delete_cmd.py). CHANGELOG.md's "Policy refusal exit code 1 → 8" fix (search CHANGELOG.md for that heading) landed for that one raise site specifically; the generic remote-4xx/5xx/not-found/etc. path was never given the same treatment, so it silently regressed back to 1 for everything else.

  5. src/bcli_cli/commands/batch_cmd.py has the same "compute in the envelope, discard for the OS" shape at least twice (:390-403 for the write-refusal path — which does pass exit_code=EXIT_POLICY explicitly into emit_failure, matching the raise it re-raises, so no mismatch there; and :453-461 for the aggregate "N of M steps failed" path, which wraps the failure in a bare RuntimeError with no status_code, so exit_code_for_status(None) happens to also resolve to 1 — matching the hardcoded raise typer.Exit(1) that follows it by coincidence, not by design). Batch's aggregate envelope also discards whatever more-specific code the first failing step actually had (e.g. a 6 or 7), collapsing it to 1 — a related but distinct loss of information worth a follow-up, not folded into this fix.

Suggested fix

Make Capture.emit_failure the single source of truth for "the code," and have each call site raise with the value it actually returns, using the fuller taxonomy from map_error_to_exit_code (not just the HTTP-status subset exit_code_for_status covers) so AuthError/ValidationError/etc. raised inside a mutating command also get the right process exit code, not just 6/7/1:

  1. In src/bcli_cli/_envelope_wrap.py, change emit_failure to accept the fuller mapping and return the exit code it wrote:

    def emit_failure(self, exc: BaseException, *, exit_code: int | None = None) -> int:
        if exit_code is None:
            from bcli_cli._error_handler import map_error_to_exit_code
            exit_code = map_error_to_exit_code(exc)
        ...
        return exit_code

    (map_error_to_exit_code already falls back to exit_code_for_status for a bare BCLIError, and to EXIT_GENERIC_ERROR for anything else — a strict superset of what emit_failure computes today, so this is behavior-additive for the envelope, not just the process exit.)

  2. At each of the five call sites, replace the hardcoded exit with the returned value:

    except Exception as e:
        code = cap.emit_failure(e)
        console.print(f"[red]Error:[/red] {e}")
        raise typer.Exit(code)

    post_cmd.py:96-99, patch_cmd.py:98-101, delete_cmd.py:94-97, attach_cmd.py:153-156, action_cmd.py:220-223 (five sites, not four).

  3. batch_cmd.py:453-461 can adopt the same one-line change for consistency (code = cap.emit_failure(err); raise typer.Exit(code)), though as noted it won't change today's observed value. batch_cmd.py:291-292 (the invalid-YAML path — except WorkflowError as e: cap.emit_failure(e); raise typer.Exit(1)) has the same hardcoded-1 shape and could be folded into the same pass as a sixth site, though it wasn't part of the live-run evidence.

  4. Add a regression test that runs post/patch/delete/action against a mocked 404/409/500 response and asserts the process exit code (not just the envelope's exit_code field) equals the documented taxonomy value — the existing test suite apparently only ever asserted the envelope side, which is how this shipped.

Notes

  • Confirmed against HEAD 4f9a1a4 / tag v0.8.2 in the local clone; not yet checked whether this predates v0.8.2 (the "Policy refusal exit code 1 → 8" CHANGELOG.md entry that fixed the one working case is from an earlier release — exact version not pinned here).
  • attach_cmd.py also has an unrelated except Exception as e: ... raise typer.Exit(1) at :214-216 inside the test (end-to-end diagnostic) subcommand, but that subcommand never wires capture()/--result-out at all, so there's no taxonomy promise being broken there — left out of scope for this fix.
  • Root-caused from docs/slice0/REPORT-04-write-contracts.md §5b item 1 (Turbine Slice 0, Spike 4 — BC write-contract validation), which flagged this while validating bcli's mutation envelope against a live BC sandbox; not something Turbine itself needs to work around beyond "always read --result-out, never trust $? for anything finer than ok/not-ok," which the report already recorded as the interim guidance.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions