Domain errors and ok:false JSON exit 0 — automation cannot detect CLI failures

[Nih1tGupta]

Area

Other

Potpie version

potpie v2.0.0b3

Install source

uv tool install potpie

System

macOS

Coding agent and model

No response

Command or workflow

Commands run:

# Capability not implemented
potpie --json cloud status
echo "exit: $?"

# Domain / pot scope failure
potpie --json graph status
echo "exit: $?"

# Graph workbench failure (ok:false in JSON)
potpie --json graph nudge --event bogus --session s1 --pot <pot-id>
echo "exit: $?"

# Shell chain (automation)
potpie --json cloud status && echo "should not print"

What happened?

The CLI printed the correct structured error output, but the process still exited with code 0 (success).

Command	Output	Exit code
potpie --json cloud status	"code": "not_implemented"	0
potpie --json graph status	"ok": false (for example, ambiguous_pot)	0
potpie --json graph nudge --event bogus ...	"ok": false with error details	0
potpie --json cloud status && echo "should not print"	The second command executed because the first returned success	0

So the JSON correctly indicated a failure, but $? reported success.

Parser and usage errors behaved correctly and returned a non-zero exit code. For example:

potpie pot create
# Error: Missing argument 'NAME'
echo "exit: $?"
# exit: 2

Source or build details

No response

Output or logs

No additional logs.

[linear-code]

POT-1869

[coderabbitai]

Coding Plan

Summary

• Fix the true root cause once at the boundary: run_cli() must propagate the exit code that Typer/Click's standalone_mode=False returns instead of discarding it, restoring correct exit codes for all fail()/contract() domain errors (including cloud status and pot-scope failures).
• Close the graph-specific gap by centralizing the ok:false → nonzero-exit guard in _emit_graph_result(), so graph nudge and sibling commands exit nonzero on error envelopes.
• Add regression tests at the real exit-code boundary (not CliRunner) and document the invariant so the fix stays enforced.

Design Choices

Design Choice 1: How should graph commands that emit ok:false payloads be made to exit nonzero?

Options Considered:

1. Centralize the guard inside _emit_graph_result() so it raises typer.Exit(EXIT_VALIDATION) whenever the emitted payload's ok is False, and remove now-redundant per-command checks.
2. Add an explicit if not result.ok: raise typer.Exit(...) to each individual command that currently lacks it.

Chosen Option: 1

Rationale: Centralizing in the single shared emitter fixes all missing-check callers uniformly, matches the existing intent of helpers like _emit_inbox_result/_emit_quality_result, and eliminates the possibility of future commands re-introducing the same omission.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Propagate command exit codes through the console-script boundary

This phase fixes the primary root cause: exit codes chosen inside command bodies are silently discarded. It resolves cloud status, graph status (ambiguous_pot / no_active_pot via fail()), and every other fail()/contract()-based domain error in one place, without touching individual commands.

Capture and re-propagate the app's return value in run_cli()

Ensure a typer.Exit raised inside a command (converted by Typer/Click's standalone_mode=False machinery into a returned exit code) is turned back into a real nonzero process exit.

• Update run_cli() in potpie/context-engine/adapters/inbound/cli/host_cli.py to assign the result of app(args, standalone_mode=False) to a variable instead of discarding it.
• After the try/except (which continues to handle Abort/ClickException exactly as today), if the captured value is a truthy integer, raise typer.Exit(code=<value>) so main()'s existing typer.Exit → SystemExit conversion carries the correct code to the OS.
• Treat None/0 as success (normal exit), preserving current behavior for success paths, --version, and no_args_is_help.
• Do not change the ClickException/Abort branches — they already produce the correct usage-error exit codes (2 human, 1 JSON) and must remain intact.

🤖 Prompt for AI agents

Implement the primary fix that restores correct process exit codes for
domain-level errors raised via `fail()`/`contract()` (e.g. `cloud status`,
`graph status` ambiguous_pot / no_active_pot), without modifying individual
commands.

- In `potpie/context-engine/adapters/inbound/cli/host_cli.py`, update
`run_cli()`:
- Capture the return value of `app(args, standalone_mode=False)` into a variable
instead of discarding it.
- Keep the existing try/except block that handles `Abort`/`ClickException`
exactly as it is today — do not change these branches, since they already
produce correct usage-error exit codes (2 for human output, 1 for JSON).
- After the try/except, check the captured return value: if it is a truthy
integer, raise `typer.Exit(code=<value>)` so that `main()`'s existing
`typer.Exit` → `SystemExit` conversion propagates the correct code to the OS.
- Treat `None` or `0` as success and allow normal exit, preserving current
behavior for success paths, `--version`, and `no_args_is_help`.

Phase 2: Ensure ok:false graph envelopes exit nonzero

This phase fixes the secondary root cause specific to the graph workbench: graph nudge --event bogus returns a GraphNudgeResult(ok=False) with no exception, so nothing raises an exit code even after Phase 1.

Task 1: Centralize the ok:false guard in _emit_graph_result()

Make the shared graph emitter responsible for raising a nonzero exit whenever it renders an error envelope.

• Update _emit_graph_result() in potpie/context-engine/adapters/inbound/cli/commands/graph.py so that, after emitting the envelope, it raises typer.Exit(code=EXIT_VALIDATION) when the payload's ok is False.
• This makes graph_nudge, graph_status, graph_describe, graph_neighborhood, graph_history, graph_catalog, graph_search_entities, and other read commands exit nonzero on failure without per-command edits.
• Preserve the distinct not_implemented path: _emit_graph_not_implemented() continues to raise EXIT_UNAVAILABLE explicitly and is unaffected.

Task 2: Remove now-redundant per-command ok checks

Avoid double-handling once the guard is centralized, keeping the control flow clear.

• In potpie/context-engine/adapters/inbound/cli/commands/graph.py, simplify _emit_inbox_result/_emit_quality_result and the inline post-emit checks in graph_mutate, graph_propose, graph_commit, graph_bulk_apply, and _emit_graph_read so they rely on the centralized guard instead of re-raising on ok:false.
• Retain any checks that key off a different field than the emitted payload's top-level ok (e.g. verification sub-results, proposal-vs-commit distinctions) where the centralized guard would not cover them.
• Confirm the intentional non-error paths (e.g. the graph_mutate "review required" branch, which emits an ok:true/info envelope) continue to return normally.

🤖 Prompt for AI agents

Fix the graph-specific gap where `ok:false` result payloads (e.g. from `graph
nudge --event bogus` returning `GraphNudgeResult(ok=False)`) produce no
exception and therefore no nonzero exit even after Phase 1's fix.

- In `potpie/context-engine/adapters/inbound/cli/commands/graph.py`, update
`_emit_graph_result()` so that after emitting the envelope, it raises
`typer.Exit(code=EXIT_VALIDATION)` whenever the payload's `ok` field is `False`.
This should uniformly cover `graph_nudge`, `graph_status`, `graph_describe`,
`graph_neighborhood`, `graph_history`, `graph_catalog`, `graph_search_entities`,
and other read commands that call this emitter, without requiring per-command
changes.
- Leave `_emit_graph_not_implemented()` untouched — it must continue to raise
`EXIT_UNAVAILABLE` explicitly for the `not_implemented` path, which is distinct
from the `ok:false` guard.
- Once the centralized guard is in place, simplify
`_emit_inbox_result`/`_emit_quality_result` and the inline post-emit `ok` checks
in `graph_mutate`, `graph_propose`, `graph_commit`, `graph_bulk_apply`, and
`_emit_graph_read` to rely on the centralized guard rather than re-raising on
`ok:false` themselves.
- Retain any checks in those commands that key off fields other than the
top-level `ok` (e.g. verification sub-results, proposal-vs-commit distinctions),
since the centralized guard won't cover those.
- Verify that intentional non-error paths — such as the `graph_mutate` "review
required" branch, which emits an `ok:true`/info envelope — continue to return
normally without triggering the new guard.

Phase 3: Regression coverage and contract documentation

This phase locks in the fix at the true OS-level boundary (which no existing test exercises for error paths) and records the intended invariant.

Task 1: Add exit-code regression tests for domain errors and ok:false

Cover the exact scenarios from the ticket at the run_cli/process boundary rather than via CliRunner (which uses standalone mode and hides the bug).

• Add tests (e.g. in potpie/context-engine/tests/unit/test_cli_usage_errors.py or a focused new module under tests/unit/) that drive host_cli.run_cli([...]) and assert a nonzero exit for: --json cloud status (not_implemented → exit 2), a graph command that reaches an ambiguous_pot/no_active_pot fail() (exit 1), and --json graph nudge --event bogus --session s1 --pot <id> (ok:false → exit 1).
• Assert the JSON payload still shows the correct error code/ok:false alongside the nonzero exit, to prevent regressing the output contract while fixing the exit code.
• Optionally extend the existing subprocess-based integration journey test to assert a nonzero returncode for one representative domain-error command, since current subprocess tests only assert returncode == 0.

Task 2: Document the exit-code invariant

Make the implicit rule explicit so future commands follow it.

• Update potpie/context-engine/adapters/inbound/cli/README.md and/or docs/context-graph/cli-flow.md to state that any error output — including --json payloads with ok:false or an error code — must always correspond to a nonzero process exit code, and briefly note that command exit codes are propagated through run_cli().

🤖 Prompt for AI agents

Add regression tests that exercise the real process-exit boundary and document
the invariant that error output must always correspond to a nonzero exit code.

- Add tests (e.g. in `potpie/context-engine/tests/unit/test_cli_usage_errors.py`
or a new focused module under `tests/unit/`) that drive
`host_cli.run_cli([...])` directly rather than via `CliRunner` (which uses
standalone mode and would hide the bug). Cover:
  - `--json cloud status` → not_implemented → exit code 2.
- A `graph` command that reaches an `ambiguous_pot`/`no_active_pot` `fail()` →
exit code 1.
- `--json graph nudge --event bogus --session s1 --pot <id>` → `ok:false` → exit
code 1.
- In each test, also assert the JSON payload still shows the correct error
`code`/`ok:false` alongside the nonzero exit, to guard against regressing the
output contract while fixing the exit code.
- Optionally extend the existing subprocess-based integration journey test to
assert a nonzero `returncode` for one representative domain-error command, since
the current subprocess tests only assert `returncode == 0`.
- Update `potpie/context-engine/adapters/inbound/cli/README.md` and/or
`docs/context-graph/cli-flow.md` to state that any error output — including
`--json` payloads with `ok:false` or an error `code` — must always correspond to
a nonzero process exit code, and briefly note that command exit codes are
propagated through `run_cli()`.

Research

Ticket Summary

The potpie CLI prints correct error output for domain failures (code: not_implemented, ok:false) but the OS process exits 0, so automation and shell chains cannot detect failures. Two independent root causes must be fixed so that any error output yields a nonzero exit code, while preserving the already-correct exit codes for parse/usage errors.

Codebase Summary

The CLI is a Typer app under potpie/context-engine/adapters/inbound/cli/. host_cli.py builds the root app and exposes the console-script entrypoint via main() → run_cli(), which invokes app(args, standalone_mode=False). Shared plumbing in commands/_common.py defines the exit-code constants (EXIT_OK/VALIDATION/UNAVAILABLE/DEGRADED/AUTH), the fail() helper (always raises typer.Exit), and the contract() error boundary that maps domain exceptions to exit codes. The graph command group (commands/graph.py) layers _graph_command() and _emit_graph_result() on top, with only some commands checking result.ok before raising typer.Exit.

File-Level Change Summary

• potpie/context-engine/adapters/inbound/cli/host_cli.py — run_cli(): capture the return value of app(args, standalone_mode=False) and re-raise typer.Exit for any nonzero code (core fix).
• potpie/context-engine/adapters/inbound/cli/commands/graph.py — _emit_graph_result(): raise typer.Exit(EXIT_VALIDATION) on ok:false; simplify redundant per-command/_emit_* checks accordingly.
• potpie/context-engine/tests/unit/ (usage-errors module or new file) — add exit-code regression tests for cloud/graph domain errors and graph nudge --event bogus; optionally extend tests/integration subprocess coverage.
• potpie/context-engine/adapters/inbound/cli/README.md and/or docs/context-graph/cli-flow.md — document the "error output ⇒ nonzero exit" invariant.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement the primary fix that restores correct process exit codes for
domain-level errors raised via `fail()`/`contract()` (e.g. `cloud status`,
`graph status` ambiguous_pot / no_active_pot), without modifying individual
commands.

- In `potpie/context-engine/adapters/inbound/cli/host_cli.py`, update
`run_cli()`:
- Capture the return value of `app(args, standalone_mode=False)` into a variable
instead of discarding it.
- Keep the existing try/except block that handles `Abort`/`ClickException`
exactly as it is today — do not change these branches, since they already
produce correct usage-error exit codes (2 for human output, 1 for JSON).
- After the try/except, check the captured return value: if it is a truthy
integer, raise `typer.Exit(code=<value>)` so that `main()`'s existing
`typer.Exit` → `SystemExit` conversion propagates the correct code to the OS.
- Treat `None` or `0` as success and allow normal exit, preserving current
behavior for success paths, `--version`, and `no_args_is_help`.
===============================================================================

Task: 2

Fix the graph-specific gap where `ok:false` result payloads (e.g. from `graph
nudge --event bogus` returning `GraphNudgeResult(ok=False)`) produce no
exception and therefore no nonzero exit even after Phase 1's fix.

- In `potpie/context-engine/adapters/inbound/cli/commands/graph.py`, update
`_emit_graph_result()` so that after emitting the envelope, it raises
`typer.Exit(code=EXIT_VALIDATION)` whenever the payload's `ok` field is `False`.
This should uniformly cover `graph_nudge`, `graph_status`, `graph_describe`,
`graph_neighborhood`, `graph_history`, `graph_catalog`, `graph_search_entities`,
and other read commands that call this emitter, without requiring per-command
changes.
- Leave `_emit_graph_not_implemented()` untouched — it must continue to raise
`EXIT_UNAVAILABLE` explicitly for the `not_implemented` path, which is distinct
from the `ok:false` guard.
- Once the centralized guard is in place, simplify
`_emit_inbox_result`/`_emit_quality_result` and the inline post-emit `ok` checks
in `graph_mutate`, `graph_propose`, `graph_commit`, `graph_bulk_apply`, and
`_emit_graph_read` to rely on the centralized guard rather than re-raising on
`ok:false` themselves.
- Retain any checks in those commands that key off fields other than the
top-level `ok` (e.g. verification sub-results, proposal-vs-commit distinctions),
since the centralized guard won't cover those.
- Verify that intentional non-error paths — such as the `graph_mutate` "review
required" branch, which emits an `ok:true`/info envelope — continue to return
normally without triggering the new guard.
===============================================================================

Task: 3

Add regression tests that exercise the real process-exit boundary and document
the invariant that error output must always correspond to a nonzero exit code.

- Add tests (e.g. in `potpie/context-engine/tests/unit/test_cli_usage_errors.py`
or a new focused module under `tests/unit/`) that drive
`host_cli.run_cli([...])` directly rather than via `CliRunner` (which uses
standalone mode and would hide the bug). Cover:
  - `--json cloud status` → not_implemented → exit code 2.
- A `graph` command that reaches an `ambiguous_pot`/`no_active_pot` `fail()` →
exit code 1.
- `--json graph nudge --event bogus --session s1 --pot <id>` → `ok:false` → exit
code 1.
- In each test, also assert the JSON payload still shows the correct error
`code`/`ok:false` alongside the nonzero exit, to guard against regressing the
output contract while fixing the exit code.
- Optionally extend the existing subprocess-based integration journey test to
assert a nonzero `returncode` for one representative domain-error command, since
the current subprocess tests only assert `returncode == 0`.
- Update `potpie/context-engine/adapters/inbound/cli/README.md` and/or
`docs/context-graph/cli-flow.md` to state that any error output — including
`--json` payloads with `ok:false` or an error `code` — must always correspond to
a nonzero process exit code, and briefly note that command exit codes are
propagated through `run_cli()`.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!