fix(p5): make background agents actually launch (+ a test that really launches pi via tmux)

[lantisprime]

Summary

Background agents (/agents bg) were non-functional despite 63 passing P5 unit tests and the full review chain. Driving the real tmux backend end-to-end surfaced two integration bugs the tests hid with consistent seams/fakes. Both fixed; verified by an actual run that launches pi through tmux.

Bug 1 — manifest dir mismatch

tmux-terminal validated manifestPath under ~/.pi/bg-state; preflight writes manifests under ~/.pi/agent/bg (bg-state.ts getBgStateDir). Every launch → invalid manifest path.
Fix: tmux-terminal/index.ts uses <os.userInfo().homedir>/.pi/agent/bg.

Bug 2 — the worker re-invoked itself as pi

getPiInvocation re-runs process.argv[1] as the pi entrypoint. Foreground argv[1] is pi, but the detached bg-worker is a separate script, so the child became node bg-worker.ts --mode json … -p and exited 1 in ~90ms — the agent never ran.
Fix: getPiInvocation only re-invokes argv[1] when its basename is pi; a non-pi parent falls through to pi on PATH.

Note: I first tried recording the resolved pi path in the signed manifest (options.piCommand) for version-pinning. The new tmux e2e test caught that child-args SAFE_CLI_TOKEN_RE rejects absolute paths as piCommand — so it would have re-broken bg in production. Dropped it; the backstop guard is the clean fix.

The test that was missing

test-bg-e2e-tmux.mjs / run-bg-e2e-tmux.sh — launches pi through an actual tmux session: real preflight → real tmux backend → real worker → real pi, then asserts result.json is completed with the agent's marker. Self-skips when tmux/pi/model are unavailable (foreground pre-check), so it never fails for environment reasons — only on an actual bg-launch regression. This is the test class that would have caught both bugs, and it caught the piCommand regression above.

test-pi-invocation.mjs updated to the guarded contract + a regression: a non-pi script (bg-worker.ts) must fall through to pi, not be re-invoked.

Verification

Real run: register agent → signed preflight → launch via the real tmux backend into a real session → worker spawns pi → result.json = completed, output … BG_E2E_TMUX_OK. All P4-2/3/4/7, bg-state, child-args/runner, and the 63 P5 tests pass.

Lesson

Test pi extensions by driving them through real tmux, not just unit tests (saved to memory).

🤖 Generated with Claude Code

[lantisprime]

Review — bot pass (approval left to maintainer).

Scope: 6 files, +83/-15. Both fixes verified against an actual launch, not just unit tests.

Bug 1 (bgStateDir) — correct. <os.userInfo().homedir>/.pi/agent/bg now matches getBgStateDir() exactly (same resolveTrustedHome() root + subpath). The inline comment documents the cross-extension coupling and why it can't import the constant (REQ-13). Worth a follow-up: expose the canonical bg-state dir through the shared bg-terminal.ts contract so this can't drift again.

Bug 2 (worker pi-invocation) — sound, defense-in-depth:

• options.piCommand rides in the MAC-signed manifest, so it's tamper-evident; the worker spawning it is as trusted as the rest of the manifest.
• preflight only records it when basename === "pi", so a non-pi preflight context (test/wrapper) records nothing and the worker falls back safely.
• The getPiInvocation guard is conservative: it only narrows when argv[1] is re-invoked (must be basename pi). Foreground (/opt/homebrew/bin/pi) is unaffected; the only behavior change is that non-pi parents now get pi on PATH instead of re-running themselves — which is the fix. Low foreground-regression risk: the sole edge case is pi launched from a non-pi-named entry with pi absent from PATH.

Tests — test-pi-invocation.mjs updated to the new contract with a real regression (non-pi script → pi) using temp-file fixtures; deterministic, no model dependency. Manifest schema change covered by existing bg-state/preflight/worker suites (all green).

Not in scope (noted in PR): the diagnosability gap — the worker writes status: failed with empty resultText and discards the child's error. Worth a small follow-up so a failed bg run records why.

No blockers.

[lantisprime]

Updated: dropped the manifest piCommand approach (the new tmux e2e test proved SAFE_CLI_TOKEN_RE rejects absolute paths, so it would re-break bg in production). Final fix is bgStateDir + the getPiInvocation basename guard. Added test-bg-e2e-tmux.mjs which actually launches pi through tmux and asserts completion (self-skips without tmux/pi/model). No blockers.

[lantisprime]

Independent review dispositions (an out-of-loop reviewer adversarially checked this diff):

• Stale comment (child-args.ts) — ACCEPTED, fixed. The old comment claimed the bg-worker "passes an explicit piCommand and never reaches here." That was true of an abandoned approach; the worker DOES reach the guard (basename bg-worker → falls through to pi). Rewrote the comment to describe the real load-bearing path + the tradeoff. (This was exactly the doc-drift class that hid the original bug, so worth catching.)
• Foreground regression risk from basename==="pi" — evaluated, KEPT basename (modification rejected). The reviewer suggested switching to an explicit PI_BG_WORKER signal. I implemented it, then caught that it regresses every non-pi caller that isn't the worker — including this e2e test's own foreground precheck (node test-bg-e2e-tmux.mjs would re-invoke the test file as pi). The basename guard handles the worker AND test harnesses AND tools; its only gap is pi-run-from-source with a non-pi entry and no pi on PATH — narrower and recoverable. Documented the tradeoff in the guard comment.
• e2e false-skip too broad — ACCEPTED, narrowed. Added a pi --version liveness gate: a fundamentally broken pi skips, while the model-auth skip remains. (Fully distinguishing "env" from "code" in the foreground precheck needs the worker to record the child's error — tracked as the diagnosability follow-up.)
• Nits (24s poll, SIGKILL real-home leak, regex) — acknowledged, acceptable for an e2e.

bgStateDir verified provably-equal to getBgStateDir(). No blockers.

[lantiscooperdev]

Bot review (lantiscooperdev) — --comment only; human approval still required per Rule 17.

Scope: 2-file polish on top of the bg-launch fix already in main (4cc5232). Reviewed for correctness, no behavior regression, test integrity.

agents/lib/child-args.ts — Comment-only change. The revised prose accurately describes the scriptBase === "pi" guard and, importantly, now documents the accepted tradeoff: a source-run pi (node <entry>.js, basename ≠ "pi") with no pi on PATH would fall through and fail to spawn. That's the right call to write down since it's a non-obvious failure mode. No code behavior change. ✅

agents/test-fixtures/test-bg-e2e-tmux.mjs — Adds a pi --version liveness gate that skips (rather than fails) when pi itself is fundamentally broken in the environment. This sharpens the test's signal: a hard failure now means a real bg-launch regression, not a missing/broken pi. Uses pi from PATH, which resolveOnPath("pi") already verified two lines above, so it's consistent. ✅

No blockers. LGTM for merge once human-approved.

[lantiscooperdev]

Verification follow-up: ran the real tmux e2e (run-bg-e2e-tmux.sh) on this branch — fix/p5-bg-agents-launch — with the pi --version liveness gate present. pi was live so the gate correctly did not skip, and the full chain (real preflight → real tmux backend → real worker → real pi → result.json) launched and completed with the marker: ✓ launched pi through a real tmux session; bg run completed with marker. The earlier bot review validated the diff statically; this confirms the test edit itself runs green on-branch.