feat(harness): manage owned child sessions [Agent Map 13/15]

[ynadge]

Primary change type

• Feature

Problem and motivation

A coordinator must reuse or close only the exact child it owns, and concurrent Codex processes must not claim the same native conversation.

Summary and scope

Add trusted private binding markers, reserved create/resume/restart/close operations, explicit user-close protection, and a Codex rollout broker over the ordinary SessionManager path.

Allow an exact retry after a binding marker commits but process launch fails. Release pending Codex discovery registrations on timeout or exit, and prevent released discovery from claiming a later rollout. Await owned close from the session deletion route.

How this increment fits

Private child operations are available before public tool activation. Native Codex first-turn discovery integration is exercised and completed in part 15.

Stack and review boundary

• Part 13 of 15 in the Agent Map review stack; review this increment against its predecessor.
• Base: review/agent-map-12-delegation-state.
• Current head: 4aa478488ac0f8b9b3851e48969ac6773e5075ca; 1,627 changed lines across 11 files, counting additions and deletions including tests.
• Repackages the corresponding final behavior from #808. Original code and review history remain preserved.
• Complete coworker testing branch: fix/studio-onboarding-followups.
• The stack remains unmerged. Dependent PRs target their predecessor, so their diffs do not repeat earlier increments.

Related work

Agent Map checkpoint SAP-3147; relevant work SAP-3151. This packaging follows the maintainer-approved 15-PR split.

Validation

Root checks ran against cabf531fadb3a6cef18f8e4cc9babc6957687516. The final head changes only README terminology or commit ancestry; a complete tracked-file comparison confirms identical executable source and build inputs. The terminology gate was rerun on 4aa478488ac0f8b9b3851e48969ac6773e5075ca.

pnpm build — passed (exit 0)
pnpm typecheck — passed (exit 0)
pnpm lint — passed (exit 0)
pnpm test — passed (exit 0)

Tests and documentation

Regression coverage: Exact binding checks, reserved create/resume, launch-failure retry, explicit user close, fresh-restart fencing, concurrent rollout attribution, and discovery cleanup races.

See part 15 for integrated browser, native CLI, and Mac journey validation. The checks above were run independently on this PR’s own commit.

Linux tests run with ordinary user filesystem permissions; the sandbox's extra ambient capabilities are dropped. Hosted CI and automated review are separate from these recorded local results.

Compatibility and release impact

• Compatibility: Adds public owned-session lifecycle methods and binding/restart error types. close() is asynchronous and must be awaited because durable closure bookkeeping can reject.
• Changeset: Included: .changeset/owned-child-session-lifecycle.md

Security

• No secrets, credentials, private user data, or unsanitized logs are included.
• This PR does not publicly disclose a suspected vulnerability.

AI assistance

• Codex assembled the implementation, addressed reproduced defects, supplied tests and documentation, inspected the diff, and ran the checks above. Reviews are handled by hosted PR automation.

Checklist

• Read CONTRIBUTING.md; implementation follows the requested 15-PR split.
• Description reflects this PR's actual predecessor-relative diff.
• Relevant tests accompany the changed behavior.
• Root build, typecheck, lint, and test evidence matches the final implementation; any documentation-only update is identified above.
• Release/documentation treatment is explained above.

[github-actions]

Review — PR #832 (Agent Map 13/15)

No confidentiality findings: the changeset, JSDoc, comments and test fixtures name only Sapiom, Claude Code and Codex, and no business arrangement, private host or internal link appears in the published prose.

Findings

1. A pending broker registration that never claims can steal the next session's rollout

packages/harness/src/core/collector/codex-rollout-broker.ts:299-317, packages/harness/src/server/index.ts:1507-1520, 4185-4200, 4243-4258

codexRolloutBroker.register() fires on every transcript-tail runtime-epoch transition, but the only release paths are onStatusChange with a non-null context.runtimeEpoch, and onRuntimeEpochTransition(session, null). Two live paths bypass both:

• startCodexTailerFor gives up after CODEX_ROLLOUT_DISCOVERY_TIMEOUT_MS (15 s), sets "unavailable"/"ambiguous" and returns without codexRolloutBroker.release(...). The session stays running and stays pending in the broker.
• transitionExited(session, null) from sweepDeadSessions() (no-pty grace path, session-manager.ts:1663) and from kill()'s stale-record path emits status with runtimeEpoch: null, so the release call in the "exited" branch is skipped.

Failure: session A launches codex in /repo, its rollout never materialises (different CODEX_HOME, codex died before writing session_meta, or A was swept). A stays pending with cwd=/repo. Session B launches in /repo and writes rollout rB. claimFresh builds the same-cwd group {A, B}; A's candidate set is {rB} (A's sinceMs is older), B's is {rB}. Both are singletons; the localeCompare sort picks one, and if A sorts first, assign(Akey, rB) tombstones rB in claimedPaths. B then gets pending on every poll — excludePaths now hides rB from it — and after 15 s is marked "unavailable". B's tailer never starts, so its transcript analytics and readiness are permanently lost for that runtime, while A holds a path it will never read.

Release the registration on the discovery-failure return, and release by session id (not (id, epoch)) on any exited transition so epoch-less exits are covered.

2. close() ships kill()'s JSDoc verbatim, and the contract it states is now false

packages/harness/src/core/session-manager.ts:1456-1479 (27 lines copied from 1546-1569)

close() awaits persistSubsessionBindings() and the caller-supplied onSubsessionUserClosed before resolving, and rethrows their failures. The copied block promises the opposite on all three counts:

• "worst-case resolution time is KILL_ESCALATION_MS + KILL_ESCALATION_CONFIRM_MS … never infinite" — an onSubsessionUserClosed that hangs has no timeout, so close() is unbounded.
• "an unawaited Promise is fine … when suppressed with void" — close() rejects on a sidecar write failure (the PR's own test asserts this), so void sessionManager.close(id) is an unhandled rejection, which terminates the process under Node's default --unhandled-rejections=throw.
• "Returns false … when the session has no live pty" — it can throw instead.

Write the block that describes close(): it kills first so a persistence failure cannot leave a delegated PTY alive, then rejects with the persistence error while leaving the in-memory tombstone for a retry.

3. Two new error classes added to the published entry point under a patch changeset

packages/harness/src/index.ts:52-53, .changeset/owned-child-session-lifecycle.md

SubsessionBindingMismatchError and SubsessionFreshRestartForbiddenError land in dist/index.d.ts, so they are a contract commitment. But SessionManager is not exported from index.ts and nothing in this repo calls createReserved/closeBound/restartFreshBound/resumeBound yet, so no consumer of the public API can currently observe either error — they are unreachable public surface. The changeset calls this "internal lifecycle support", which is consistent with the stack's other patch entries only if the classes stay internal.

Pick one: drop them from src/index.ts (they remain available to server/index.ts via core/errors.js) and keep the patch, or keep the export, bump to minor, and say in the changeset which public call can throw them.

4. resumeBound lacks the already-advanced-marker tolerance restartFreshBound has

packages/harness/src/core/session-manager.ts:1326-1335 vs 3577-3583

In resume() the marker advances to next and the sidecar commits before adapter.launch/spawn. If the spawn then fails, the stored marker is next while the coordinator's expected is still the old one, and every retry throws SubsessionBindingMismatchError — the child cannot be resumed again under the marker the caller holds. restartFreshBound explicitly handles this window ("A retry may observe the already-advanced marker after the sidecar write committed"); apply the same sameSubsessionBinding(current, next) fallback in the resume() transition check.

Nit

• findRolloutFile (codex-tailer.ts:353) now has no production caller — only its own test. Either delete it with the tests folded into findRolloutCandidates, or note why it stays.

Verdict: Request changes — finding 1 is a live rollout-attribution bug reachable today on the ordinary codex path, independent of the still-dormant delegation API; 2 and 3 are cheap to fix before publish.

[github-actions]

Review round 2 — PR #832 (Agent Map 13/15)

Delta reviewed: 5ec7105 → cabf531 (PR-only, excluding the rebased base from #831).

Earlier findings — all four resolved

1. Leaked pending broker registration — fixed. startCodexTailerFor now calls release() on the discovery-failure return before the epoch re-check (server/index.ts:4197), the "exited" branch switched to releaseSession(session.id) so epoch-less exits are covered (:4269), the running/epoch bail-out releases (:4216), and discoverCodexRolloutPath re-checks isCurrentRuntimeEpoch at the top of each poll (:4160) so a dead runtime cannot re-register itself. The extra claimFresh guard that drops non-pending keys from possibilities (codex-rollout-broker.ts:104-108) closes the release-during-findRolloutCandidates window. Covered by two new wiring tests and one broker test.
2. close() carrying kill()'s JSDoc — fixed; the block now states the kill-first ordering, the unbounded coordinator callback, and that callers must await and handle rejection (session-manager.ts:1461-1471).
3. New error classes under a patch — fixed; changeset bumped to minor and now names both errors and the awaited close(). The claim that startServer's SessionManager exposes the methods checks out (HarnessServer.sessionManager, server/index.ts:400, exported from src/index.ts:81).
4. resumeBound retry after a committed marker — fixed; resume() accepts current === next in both the validation and the persist step (session-manager.ts:1299-1300, 1332-1341), and the new test confirms the tolerance does not admit a foreign bindingId or a skipped incarnation.

New findings

None. The DELETE /sessions/:id reroute to close() is coherent — the web client already awaits killSession and keeps a session visible when it rejects (use-harness-state.ts:1683-1687), and removals fan out via Promise.allSettled, so the added latency is bounded by the kill escalation window rather than serialised.

Correction to round 1

Nothing in round 1 was wrong.

Nit (unchanged from round 1)

• findRolloutFile (codex-tailer.ts:353) still has no production caller — the last import went away in this delta, leaving an exported function backed only by its own tests and a stale reference in the server/index.ts:237 comment.

Verdict: Approve. Every round-1 finding is addressed and the delta introduces no new ones.