refactor(cli): centralize process lifecycle

[jrusso1020]

What

Centralize CLI process termination and error signaling at the executable boundary.

Why

Command handlers wrote process.exit/process.exitCode directly, which made cleanup, telemetry finalization, testing, and machine-readable errors inconsistent.

How

Introduce CommandResult, CliUsageError, CliRuntimeError, and CliResultSignal. Route termination through cli.ts, preserve finalizers, and enforce ownership with an AST-based repository check.

Test plan

• CLI suite and build
• CLI process-ownership checker, lint, typecheck, and stack-wide tests

[jrusso1020]

• refactor(repo): resolve changed-code audit #2174
• test(engine): isolate peer abort regression #2173
• refactor(cli): decompose render phases #2172
• refactor(cli): centralize process lifecycle #2171 👈 (View in Graphite)
• fix(cloudrun): publish adapter contract #2170
• test(repo): execute packed subpaths #2169
• refactor(repo): centralize package subpaths #2168
• refactor(core): own edit protocol contract #2167
• test(compiler): enforce preview render parity #2166
• refactor(studio): gate SDK operation families #2165
• refactor(studio): gate Acorn writer migration #2164
• refactor(producer): make capture plans immutable #2163
• refactor(producer): unify render requests #2162
• refactor(engine): type capture failures #2161
• refactor(engine): manage child process lifecycles #2160
• refactor(engine): add fingerprinted browser leases #2159
• refactor(render): centralize execution ownership #2158
• refactor(core): unify composition contract #2157
• fix(studio): enforce optimistic file concurrency #2156
• fix(studio): make sdk cutover transactional #2155
• fix(render): publish artifacts atomically #2154
• fix(render): surface structured outcomes #2153
• fix(player): own connection and media resources #2152
• fix(core): preserve runtime transport contract #2151
• fix(player): version runtime protocol #2150
• test(repo): verify packed consumers #2149
• test(repo): enforce workspace contracts #2148
• fix(core): enforce strict runtime safety #2147
• test(producer): gate source tests by execution lane #2146
• fix(core): make timing masks safe for source loaders #2145
• chore(repo): forbid tracked generated artifacts #2144
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[jrusso1020]

Fixed the fresh-head lifecycle CI failures on c4ed311d70f46ad7ff7c64916c3d1d322950ef62 and restacked/published #2171–#2174.

• Replaced the remaining command-level process.exit(1) calls with failCommand() so termination stays owned by cli.ts.
• Updated cloud-command/error assertions to expect CliRuntimeError rather than mocked process termination.
• Validated locally: CLI process-ownership check, CLI typecheck, full lint, and full CLI suite (2,075 passed; 2 skipped).

Fresh exact-head CI is running.

[miguel-heygen]

The process-ownership refactor is structurally sound and the focused lifecycle/usage/ownership tests pass, but one blocking telemetry regression remains.

Verdict: REQUEST_CHANGES
Reasoning: Exact-head command failures are reported twice, which corrupts the cli_error signal this path is meant to preserve.
-- Magi

[jrusso1020]

Addressed Magi’s P1 on exact head d1b9d2ac7435d5e8dcae90fbffc650d7f4211f33 and restacked/published #2171–#2174.

executeCli is now the sole command-failure telemetry reporter. The command wrappers retain recursive unknown-flag validation and rethrow failures unchanged to that boundary, preventing duplicate cli_error events. A regression test locks the single-boundary reporting architecture, alongside wrapper propagation coverage.

Validated locally: focused command-failure/CLI-boundary tests, CLI ownership check, typecheck, full lint, and full CLI suite (2,075 passed; 2 skipped). Fresh exact-head CI is running.

[jrusso1020]

Addressed Magi’s P1 on exact head d1b9d2ac7435d5e8dcae90fbffc650d7f4211f33 and restacked/published #2171–#2174.

executeCli is now the sole command-failure telemetry reporter. The command wrappers retain recursive unknown-flag validation and rethrow failures unchanged to that boundary, preventing duplicate cli_error events. A regression test locks the single-boundary reporting architecture, alongside wrapper propagation coverage.

Validated locally: focused command-failure/CLI-boundary tests, CLI ownership check, typecheck, full lint, and full CLI suite (2,075 passed; 2 skipped). Fresh exact-head CI is running.

[miguel-heygen]

Reviewed exact head 73d3b4e1f491e5211a960bac86fbb674909be7ad against live main (6ad738b580adf157393fde02351af64669c9fbc5).

Audited: packages/cli/src/cli.ts, utils/commandResult.ts, utils/command-failure-tracking.ts and their focused tests, recursive usage resolution, and the AST ownership checker. I mechanically checked the command migration for remaining direct process.exit / process.exitCode writes. Trusting: the individual command bodies beyond their lifecycle substitutions, backed by the green exact-head CLI/full CI matrix.

The central result contract is a strong boundary: commandResult.ts:60-85 cleanly separates fatal signals, non-fatal exit status, and root-owned forced termination. The recursive wrapper also preserves leaf unknown-flag validation while propagating the original failure unchanged (command-failure-tracking.ts:34-70).

blocker — packages/cli/src/cli.ts:450: await the sole failure reporter before finalizing telemetry. reportCommandFailure() is async because it lazy-imports telemetry/events.js before enqueuing the cli_error (packages/cli/src/utils/command-failure-tracking.ts:80-83), but the new executable-boundary call drops its promise. Control immediately reaches finalizeCli() at line 460, which flushes the queue and marks finalized = true; if the lazy import settles afterward, the error is queued after the only async flush and the exit fallback returns early because the CLI is already finalized. The prior wrapper explicitly awaited this operation for exactly that reason. This can turn the fixed duplicate signal into an intermittently missing signal, especially for fast failures such as unknown flags. Please await reportCommandFailure(command, error) and add a boundary test that defers the reporter and proves flush/finalization happens only after the error is enqueued; the current source-count assertion proves one call site, not delivery ordering.

All exact-head required checks are green, and the focused ownership/result/wrapper tests pass; they do not exercise this ordering.

Verdict: REQUEST CHANGES
Reasoning: The duplicate reporter is removed, but the remaining sole reporter is not awaited before the one-shot telemetry flush, so cli_error delivery is still not reliable.

— Magi

[jrusso1020]

Fixed Magi’s telemetry-ordering P1 on exact head a9338a4e0f91b8482fe32bef50018209c29ce031 and restacked/published #2171–#2174.

executeCli() now awaits the sole reportCommandFailure() call before entering finalizeCli(). The new lifecycle regression defers the lazy telemetry-events import and proves cli_error is queued before the final flush.

Validated locally: targeted lifecycle test; CLI typecheck; process-ownership gate; full lint; full CLI suite (2,076 passed; 2 skipped); P1-file formatting and diff checks. The repository-wide format check remains blocked only by the preserved unrelated registry/examples/product-promo/index.html edit, and the commit hook’s Fallow step reports inherited stack findings outside this patch.

[github-actions]

Fallow audit report

Found 21 findings.

Duplication (12)

Severity	Rule	Location	Description
minor	fallow/code-duplication	packages/cli/src/auth/\_test-utils.ts:45	Code clone group 1 (6 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/auth/login.test.ts:68	Code clone group 1 (6 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/browser.ts:34	Code clone group 2 (10 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/browser.ts:48	Code clone group 3 (8 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/browser.ts:88	Code clone group 2 (10 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/browser.ts:118	Code clone group 3 (8 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/check.test.ts:321	Code clone group 4 (13 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/check.test.ts:356	Code clone group 4 (13 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/preview.ts:947	Code clone group 5 (15 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/commands/preview.ts:997	Code clone group 5 (15 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/utils/render-success-state.test.ts:39	Code clone group 6 (15 lines, 2 instances)
minor	fallow/code-duplication	packages/cli/src/utils/render-success-state.test.ts:125	Code clone group 6 (15 lines, 2 instances)

Health (8)

Severity	Rule	Location	Description
critical	fallow/high-crap-score	packages/cli/src/commands/add.ts:257	'run' has CRAP score 197.3 (threshold: 30.0, cyclomatic 28)
minor	fallow/high-crap-score	packages/cli/src/commands/batchRender.ts:195	'validateBatchVariables' has CRAP score 31.6 (threshold: 30.0, cyclomatic 10)
critical	fallow/high-crap-score	packages/cli/src/commands/benchmark.ts:60	'run' has CRAP score 238.6 (threshold: 30.0, cyclomatic 31)
minor	fallow/high-crap-score	packages/cli/src/commands/browser.ts:24	'runEnsure' has CRAP score 31.6 (threshold: 30.0, cyclomatic 10)
critical	fallow/high-crap-score	packages/cli/src/commands/catalog.ts:44	'run' has CRAP score 106.4 (threshold: 30.0, cyclomatic 20)
minor	fallow/high-cognitive-complexity	packages/cli/src/commands/lambda/policies.ts:219	'runPolicies' has cognitive complexity 24 (threshold: 15)
major	fallow/high-cognitive-complexity	packages/cli/src/commands/lambda/policies.ts:318	'validatePolicy' has cognitive complexity 27 (threshold: 15)
major	fallow/high-crap-score	packages/cli/src/commands/remove-background.ts:88	'run' has CRAP score 63.6 (threshold: 30.0, cyclomatic 15)

Suppressions (1)

Severity	Rule	Location	Description
minor	fallow/stale-suppression	packages/cli/src/commands/render.ts:1080	// fallow-ignore-next-line unused-export

Generated by fallow.

[miguel-heygen]

Exact-head re-review at a9338a4e0f91b8482fe32bef50018209c29ce031.

Audited: the complete two-file delta from the previously reviewed 73d3b4e1f head (packages/cli/src/cli.ts and the new packages/cli/src/cli.lifecycle.test.ts), plus exact-head required CI. Trusting: the restacked 76-file lifecycle migration previously reviewed; its tree is unchanged apart from this focused fix.

The telemetry-ordering P1 is resolved. executeCli() now awaits the sole lazy reportCommandFailure() call before entering finalizeCli(), so the final flush cannot run before cli_error is queued. The regression deliberately stalls the telemetry-events import, proves no flush occurs while reporting is blocked, then verifies cli_error precedes flush; this covers the asynchronous failure mode rather than merely counting source call sites.

All 8 required exact-head checks are green: Build, Test, runtime contract, Typecheck, regression, Windows tests/render, and semantic title. The non-required Fallow audit reports inherited stack findings outside this two-file delta.

Verdict: APPROVED — the blocker is fixed with direct lifecycle-ordering coverage and exact-head CI proof.

— Magi