v1.42.1.0 feat: gate terminal-agent teardown on ServerConfig.ownsTerminalAgent (unblocks gbrowser embedder)

[garrytan]

Summary

Adds ServerConfig.ownsTerminalAgent?: boolean (default true) so embedders that run their own PTY server can opt out of gstack's terminal-agent teardown. Fixes the gbrowser phoenix overlay's /health.terminalPort: null issue without changing CLI behavior bit-for-bit.

The fix:

• browse/src/server.ts:1312 wraps three teardown side effects in if (ownsTerminalAgent): the pkill -f terminal-agent\.ts spawnSync, plus safeUnlinkQuiet for terminal-port and terminal-internal-token. Default is true so the CLI path is preserved bit-for-bit; gbrowser passes false.
• Strict opt-out: cfg.ownsTerminalAgent === false ? false : true — only explicit false flips the gate. Defends against JS callers bypassing TS.
• CLI start() call site explicitly passes ownsTerminalAgent: true with a comment; static-grep test pins it.
• New __resetShuttingDown() test-only export at browse/src/server.ts:2628 mirrors __resetRegistry precedent. Module-scoped isShuttingDown otherwise silently no-ops a second shutdown() in the same test process.
• Drops dead try { safeUnlinkQuiet } catch {} wrappers inside the new gate (slop-scan flagged).

Bug fix downstream: gbrowser PR #21's mitigation (re-writing terminal-port on every overlay spawn) becomes redundant. Confirmed by reading that diff — purely client-side, no gstack-side coordination needed.

Test Coverage

CODE PATHS                                            EMBEDDER FLOWS
[+] browse/src/server.ts                              [+] gbrowser phoenix overlay teardown
  ├── ServerConfig.ownsTerminalAgent (new field)        ├── [★★★ TESTED] false preserves port file — case 1
  ├── buildFetchHandler capture (=== false ? false : true) ├── [★★★ TESTED] false preserves token file — case 1
  │   ├── [★★★ TESTED] default unset → true — case 3    ├── [★★★ TESTED] false skips pkill (spawnSync spy) — case 1
  │   ├── [★★★ TESTED] explicit true — case 2           └── (gbrowser PR #21 mitigation becomes redundant — verified manually)
  │   └── [★★★ TESTED] explicit false — case 1
  ├── shutdown() if (ownsTerminalAgent) gate           [+] gstack CLI teardown (preserve bit-for-bit)
  │   ├── [★★★ TESTED] true branch runs pkill — case 2   ├── [★★★ TESTED] default behavior unchanged — case 3
  │   └── [★★★ TESTED] true branch unlinks both files    ├── [★★★ TESTED] explicit true at start() — case 4 static-grep
  └── __resetShuttingDown() (test-only export)         [+] CI / dev-machine safety
      └── [★★★ TESTED] beforeEach reset enables case 2+   └── [★★★ TESTED] real pkill NEVER runs in tests (spawnSync stubbed)

COVERAGE: 9/9 paths tested (100%) | Code paths: 6/6 | Embedder flows: 3/3
QUALITY: ★★★:9 ★★:0 ★:0 | GAPS: 0

Tests: browse/test/server-factory.test.ts (28 pass, unchanged) + new browse/test/server-embedder-terminal-port.test.ts (4 pass) = 32 tests on the impacted surface, all green. Per-test stub of child_process.spawnSync ensures real pkill -f terminal-agent\.ts never runs on the developer's machine (would kill sibling gstack sessions). beforeAll/afterAll save and restore real-daemon terminal-port/terminal-internal-token so the test is safe to run while gstack is alive.

Pre-Landing Review

0 issues found. Code follows the existing xvfb? / proxyBridge? precedent; JSDoc enumerates all three gated actions + polarity inversion warning + pkill regex breadth caveat.

Pre-existing test failures (NOT introduced by this PR — verified via git stash + re-run on origin/main): 8 test files have failures: server-auth.test.ts (4), sidebar-integration.test.ts (11), sidebar-security.test.ts (1), sidebar-ux.test.ts (73), security-sidepanel-dom.test.ts (6), security-source-contracts.test.ts (1), gstack-config.test.ts (3), snapshot.test.ts (1). These predate this PR and live in sidebar/security/snapshot/config code untouched by these commits. Identical failure counts confirmed on clean origin/main.

Eval Results

No prompt-related files changed — evals skipped.

Scope Drift

Scope Check: CLEAN. Intent (user attachment): gate terminal-port deletion behind a ServerConfig flag. Delivered: the gate, plus hardening (3 case tests + static-grep + JSDoc + __resetShuttingDown + adversarial-flagged strict-bool + real-daemon save/restore).

Plan Completion

10/10 plan items DONE:

• T1 (ServerConfig field + JSDoc), T2 (buildFetchHandler capture), T3 (shutdown gate + D3 cleanup), T4 (CLI explicit : true), T5 (__resetShuttingDown export), T6 (buildFetchHandler JSDoc), T7 (test file with 4 cases including static-grep), T8/T9/T10 (3 followup TODOs filed).
• Adversarial pass surfaced 2 fixes integrated post-plan: strict-bool semantics (=== false ? false : true) and test beforeAll/afterAll save+restore for real-daemon files.

TODOS

3 P3 followups filed in TODOS.md:

• Identity-based terminal-agent kill (PID-tracked process.kill, replaces pkill -f regex)
• shutdown() reads module-level config (composition gap; parallel chromiumProfile gap)
• 4th-gate-collapse-to-ownership-object trigger

Documentation

• CLAUDE.md — added an "Embedder terminal-agent ownership" paragraph after the Sidebar architecture block. Documents the new ServerConfig.ownsTerminalAgent?: boolean gate (default true), the three side effects it controls (pkill -f terminal-agent\.ts plus two safeUnlinkQuiet calls), polarity inversion vs xvfb?/proxyBridge? (explicit bool vs presence), and the static-grep CI test that fails if a refactor drops the explicit : true at the CLI start() call site. Embedders (e.g. gbrowser phoenix overlay) must pass false to preserve their PTY discovery files.

Skipped (no update needed): README.md, ARCHITECTURE.md, CONTRIBUTING.md, VERSION (bumped), CHANGELOG.md (entry written).

PR body notes for gbrowser team

chromiumProfile is probably the next embedder-clobber. cleanSingletonLocks(resolveChromiumProfile()) at browse/src/server.ts:1298 runs unconditionally — if you pass cfg.chromiumProfile, gstack's shutdown will clean lock files in your profile dir on every teardown. Not fixed here; file a ticket if it bites.

Test plan

• bun test browse/test/server-factory.test.ts (28 pass)
• bun test browse/test/server-embedder-terminal-port.test.ts (4 pass)
• bun test browse/test/dual-listener.test.ts (25 pass)
• Static grep safeUnlinkQuiet.*terminal-(port|internal-token)|pkill.*terminal-agent — all 3 matches inside if (ownsTerminalAgent) block
• Static grep ownsTerminalAgent: true at CLI start() — matches at server.ts:2466 with comment

🤖 Generated with Claude Code

[github-actions]

E2E Evals: ✅ PASS

6/6 tests passed | $1.27 total cost | 12 parallel runners

Suite	Result	Status	Cost
e2e-browse	2/2	✅	$0.15
e2e-deploy	2/2	✅	$0.31
e2e-qa-workflow	1/1	✅	$0.79
llm-judge	1/1	✅	$0.02

---

12x ubicloud-standard-8 (Docker: pre-baked toolchain + deps) | wall clock ≈ slowest suite