Skip to content

SAP-3151: Create writable idempotent project subsessions - #808

Open
ynadge wants to merge 20 commits into
conductor/sap-3150-role-neutral-focused-briefsfrom
conductor/sap-3151-writable-idempotent-project-subsessions
Open

SAP-3151: Create writable idempotent project subsessions#808
ynadge wants to merge 20 commits into
conductor/sap-3150-role-neutral-focused-briefsfrom
conductor/sap-3151-writable-idempotent-project-subsessions

Conversation

@ynadge

@ynadge ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Bug fix
  • Documentation
  • Feature
  • Tests
  • Dependency update
  • Maintenance or refactor

Problem and motivation

Project agents need a role-neutral way to delegate focused work to ordinary writable sessions without creating duplicate sessions, processes, kickoff turns, or tabs on retry. Recovery also needs to remain strictly scoped to coordinator-owned sessions so manual user sessions are never adopted or mutated.

Summary and scope

  • Adds bounded single/batch project_subsession_delegate contracts with capability-derived project, user, and parent-session scope, canonical request digests, exact optional assignment/map-node/brief focus, and deterministic real-session results.
  • Persists atomic request receipts, bindings, reserved session IDs, lifecycle epochs, and fenced spawn/context/kickoff claims in a separate durable subsessions.json aggregate.
  • Creates, resumes, or narrowly fresh-restarts coordinator-owned sessions through the ordinary writable SessionManager path, with two-sided ownership proof and explicit user-close protection.
  • Gates kickoff on canonical adapter readiness and exact runtime identity, records tracked write phases and acknowledgement watermarks, and preserves sticky uncertain delivery rather than resending blindly.
  • Correlates concurrent Codex rollout ownership per process epoch and fails closed when transcript identity is ambiguous.
  • Registers delegation for every ordinary project capability, including delegated children, and retains exact focused-context provenance/staleness without changing tools or execution policy.
  • Lets each project agent idempotently release its own coordinator-owned child bindings by delegation key, closes only the exact two-sided Harness session, and protects manual or foreign sessions from adoption or mutation.
  • Bounds nested delegation to four levels and 64 active or explicitly re-referenced coordinator-owned sessions per project. Dormant exited/failed bindings retain exact resume identity without consuming an active slot; their reactivation is atomically claimed and capacity-fenced across coordinator instances.
  • Fences every expired spawn claim before same-owner create/resume shortcuts, so two coordinators racing an expired self-owned claim still converge on one takeover epoch and process.
  • Treats unknown or expired release keys as identity-free already-released results while releasing known siblings independently. After durable coordinator closure, SessionManager prunes exact private ownership markers and retries partial cleanup safely across restart.
  • Lets any current project agent run a bounded, server-selected release-dormant recovery for exited/failed coordinator bindings in its project, regardless of parent liveness. It accepts no session IDs, atomically rechecks and closes each selected child before private cleanup, preserves ordinary session history, and never selects active or manual sessions.
  • Distinguishes project-wide dormant eviction from an explicit parent/user close: affected prior request receipts expire with request_key_expired / new_request_key, while a fresh request key may create exactly one new binding, session, and process for the same delegation key.
  • Emits one content-free release event for every newly committed dormant eviction, and reports the durable released outcome truthfully even when exact private SessionManager cleanup must be retried.
  • Reports durable-history exhaustion with machine-readable recovery: "release_dormant"; a genuinely all-active live cap remains inspect_session and never suggests dormant cleanup.
  • Reclaims released capacity and compacts closed bindings into bounded idempotency/ownership tombstones so coordinator and private SessionManager history do not grow monotonically under create/release churn.
  • Adds content-free telemetry, common-prompt/docs updates, concurrency/recovery/manual-protection/adapter/MCP/browser coverage, and a Changeset.

This PR does not add consent, approvals, planner/builder roles, read-only execution modes, special map-mounted tabs, repository/deployed-agent orchestration, or SAP-3152 cutover work.

Related work

Related issue or discussion: SAP-3151 — Create writable idempotent project subsessions

Stacked on #807 / conductor/sap-3150-role-neutral-focused-briefs at acb2dbae6a3e533e01a065c43fa4109bdd82ca14.

Validation

pnpm --filter @sapiom/harness test — passed: 222 files / 3,608 tests; 3 performance files / 10 tests
pnpm --filter @sapiom/harness build — passed
pnpm --filter @sapiom/harness typecheck — passed
pnpm --filter @sapiom/harness lint — passed
pnpm --filter @sapiom/harness exec vitest run src/core/subsession-coordinator-store.test.ts src/core/subsession-coordinator.test.ts — passed: 2 files / 50 tests
pnpm --filter @sapiom/harness exec playwright test --config web/e2e/playwright.config.ts web/e2e/subsession-delegation.spec.ts — passed: 1 Chromium test
pnpm --filter @sapiom/harness exec vitest run src/shared/subsession-delegation-codec.test.ts src/core/subsession-coordinator-store.test.ts src/core/session-manager.test.ts src/core/subsession-coordinator.test.ts src/server/agent-map-mcp.test.ts src/server/agent-map-mcp-wiring.test.ts — passed: 6 files / 258 tests
exact HEAD ca747232e0b775b5be5c69c5427c778b3774dd4f — local and remote head check passed after publication
pnpm build — exit 0 across 18 workspace packages
pnpm typecheck — exit 0 across 18 workspace packages
pnpm lint — exit 0 with zero errors; existing warnings remain in unrelated packages
git diff --check — passed
pnpm test — not rerun for this review fix because no agent-core source changed; the prior only exception remains the unchanged sandbox chmod environment case in src/bundle-error.spec.ts

Tests and documentation

Added contract/codec, durable-store, SessionManager, coordinator, Codex rollout broker, MCP/wiring, adapter behavior, telemetry, and browser navigation tests. Release coverage proves exact ownership, manual-session protection, identity-free mixed unknown keys, idempotent retry, exited/failed release, real PTY closure, bounded sidecar/tombstone retention, restart recovery after cleanup failure, dormant-vs-active capacity accounting, atomically rechecked active-child exclusion, retained ordinary session history, and project/manual-session safety. The active-parent eviction regression proves a sibling can sweep a dormant child, emits exactly one content-free release event, expires the original request with bounded request_key_expired / new_request_key recovery, and lets a fresh request create one different real session/process under the same delegation key. A cleanup-failure regression proves that a durable dormant eviction remains truthfully released with bounded inspection guidance while the exact private marker is retained for idempotent retry, then pruned successfully on replay without another release event. Error-contract coverage proves durable-history capacity names release_dormant while a live cap retains inspect_session. Spawn coverage also proves two coordinator instances racing an expired self-owned claim create exactly one child process. Updated the shared build-plan guide, public tool schema, changeset, and common project-agent prompt to document writable nested delegation, recovery, focus, destructive dormant release, retention, and manual-session protection.

Compatibility and release impact

  • Breaking or externally visible changes: Internal embedders constructing the Agent Map MCP router must provide the shared SubsessionCoordinator; transcript-backed session hosts must complete exact runtime identity correlation before trusted background kickoff. This is a minor change while @sapiom/harness is pre-1.0.
  • Changeset: Added .changeset/writable-project-subsessions.md for @sapiom/harness minor.

Security

  • I have not included secrets, credentials, private data, or unsanitized logs.
  • This pull request does not publicly disclose a suspected vulnerability. I
    will follow the
    Security Policy for
    private reporting.

AI assistance

  • I did not use AI assistance for this change.
  • I used AI assistance and have described it below.

OpenAI Codex implemented the focused SAP-3151 slices, tests, and documentation under the supplied project/ticket contracts. Every change was reviewed against the repository interfaces and validated with the focused, package-wide, browser, and root commands listed above.

Checklist

  • I read CONTRIBUTING.md, and this contribution follows the direct-PR or issue-first policy.
  • This pull request addresses one focused problem and contains no unrelated cleanup.
  • I added or updated tests, or explained above why tests are not applicable.
  • I ran the relevant build, typecheck, lint, and test commands, or explained
    any N/A checks above.
  • I updated documentation for user-facing changes, or marked it N/A above.
  • I added a Changeset for a published-package change, or explained why it is not applicable.
  • I can explain and maintain every submitted change, including any AI-assisted work.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review — PR #808 (SAP-3151, writable idempotent project subsessions)

Public-copy hygiene: clean. The changeset, docs/shared-build-plan.md, the project-agent
prompt, and the e2e fixture (acme-app, already the repo's placeholder) contain no company
names, business arrangements, or internal context. Nothing to flag.

1. Recursive delegation has no depth or live-session cap

agent-map-mcp-tools.ts:386 registers project_subsession_delegate for every project
capability, and agent-map-mcp-wiring.test.ts:161 confirms a delegated child gets the tool
too. SubsessionBindingRecord carries no depth or parent-binding field, reserveDelegations
enforces no per-parent limit, and SessionManager has no live-session cap (grep maxSessions
→ nothing). PROJECT_SUBSESSION_DELEGATION_LIMIT = 16 bounds one request, not the tree.

Failure: a project agent that decomposes three levels deep — a normal reading of "delegate
further when useful" in profiles/project-agent.ts:15 — reserves 16 + 256 + 4096 bindings,
each of which ensureSession spawns as a real claude/codex PTY. The only backstop is
SUBSESSION_COORDINATOR_BINDING_LIMIT = 8_192, i.e. thousands of agent processes on a
desktop user's machine before anything says no. Needs a depth bound in the binding record
and/or a cap on concurrently-live coordinator-owned sessions.

2. Request receipts never expire — delegation dead-ends permanently at 8,192

subsession-coordinator-store.ts declares requestTombstones in the aggregate, parses it
(:532), and enforces its invariants (:520, :554) — but nothing ever pushes to it. Every
sibling store implements the expiry half (agent-map-proposal-service.ts:330,
build-plan-service.ts:612, agent-map-workspace-store.ts:369); this one omits it.

Failure: requestReceipts grows monotonically. On receipt 8,192 the guard at :857/:981
throws capacity_exceeded, which wholeCallError reports as reduce_request — advice that
can never help, because the limit is on history, not on the current request. Every
delegate and refresh for that project fails forever, recoverable only by hand-editing
~/.sapiom/.../subsessions.json. bindings (:1007) has the same shape with no reclaim path
for closed/terminal bindings.

3. execute() throws away the codec's error code and issue list

subsession-coordinator.ts:167-172 catches every parseProjectSubsessionRequest failure and
replaces it with a fixed invalid_request / correct. So SubsessionDelegationValidationError
— which goes to deliberate trouble to bound issues to 32×(256,128) chars for exactly this
purpose (subsession-delegation-codec.ts:44) — is discarded, and its unsupported_schema /
capacity_exceeded codes collapse into invalid_request.

Consequence for the published contract: DelegationError.issues, exported from
src/index.ts, is populated by no code path in the repo. The SubsessionCoordinatorError
branch in errorResult (agent-map-mcp-tools.ts:168) passes error.detail straight
through, so a consumer's model gets "invalid_request, correct it" with no indication of
which field. Concretely reachable: outcome: z.string().max(4_096) in the tool schema
counts UTF-16 code units, while PROJECT_SUBSESSION_OUTCOME_BYTES is 4 KiB — a ~2,000-char
CJK outcome passes Zod, fails the codec on byteLength, and the caller is told nothing.
Either propagate code/issues, or drop issues from the exported type before it ships.

4. SubsessionFreshRestartForbiddenError is never mapped, so a terminal state says "retry"

itemError (subsession-coordinator.ts:1152) handles SubsessionBindingMismatchError,
SessionNotReadyError, and store errors, but not SubsessionFreshRestartForbiddenError, so
it falls through to session_create_failed with retryable: true, recovery: "retry".

Failure: a delegated session whose kickoff delivery is still pending but which recorded
turns (the user typed into the tab) exits with no resumable vendor transcript.
recoverExitedSession passes the !resumable && state === "pending" guard at :665, calls
restartFreshBound, and that throws SubsessionFreshRestartForbiddenError on
hasRecordedTurns (session-manager.ts:1539). The condition is permanent, but the parent
agent is told to retry, and will — every retry re-walks the same path.

5. DELETE /api/sessions/:id can now 500 and leave the PTY alive

rest.ts:910 swaps fire-and-forget kill for await sessionManager.close(id). close
persists userClosedSubsessions before killing and rethrows on failure
(session-manager.ts:1888-1897), so an fsync/EACCES failure on
sessions.json.subsession-bindings.json returns 500 and skips the kill — the user cannot
close a delegated tab at all. Previously the endpoint always killed and always returned 200.
Kill first, or persist best-effort. This behavior change to a REST endpoint also isn't in the
changeset.


Verdict: request changes — findings 1 and 2 are unbounded-growth defects that stop the
feature working (or stop the machine working) with no user-visible recovery.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since 1f9fd2f1)

One commit moved (0cd41b91). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md) is clean.

Fixed: #1 (depth 4 + 64 live sessions, enforced in reserveDelegations and re-checked
on parse), #3 (SubsessionDelegationValidationError.code/issues now propagate through
error.detail), #4 (SubsessionFreshRestartForbiddenError → terminal
session_restart_failed), #5 (close() kills before persisting, mirrors the close into
the aggregate).

New — delivery history is the one history the compaction pass skipped

subsession-coordinator-store.ts:1089 now throws history_quota_exceeded at
SUBSESSION_COORDINATOR_DELIVERY_LIMIT, which itemError maps to
capacity_exceeded / retryable:false / recovery:"none" — where it previously said
reduce_request. But compactTerminalHistory compacts receipts and closed bindings only;
target.deliveries.push (:1101) is append-only and nothing ever prunes acknowledged
deliveries. Failure: after 63 refresh-focused-context calls against one child — normal
for a long-lived coordinator — that child is permanently un-refreshable, and the tool now
tells the agent there is no recovery. Parse already allows pruning (unique epochs, last
must equal contextEpoch), so dropping acknowledged non-current deliveries is safe.

Not fixed — #2 (request history) is relabelled, not bounded

Retention is 1,024 receipts + 8,192 tombstones, both monotonic, so the per-project ceiling
moved from 8,192 to ~9,216 and is now documented as terminal (recovery:"none") with no
remediation beyond hand-editing subsessions.json. Every focused-context refresh burns a
receipt too, so the ceiling counts routine traffic, not just delegations. Compounding:
compactTerminalHistory also runs inside closeBinding (:1002), so once history is
exhausted, closing a delegated tab throws and the binding never records closed
permanently consuming one of the 64 live slots.

Verdict: request changes — both remaining findings are permanent dead-ends reported to
the caller as unrecoverable.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since 0cd41b91)

One commit moved (a67fc46a). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md) is clean.

Fixed: round-2's delivery-history finding — acknowledged/unsent epochs are now pruned on
refresh, and history_quota_exceeded no longer fires from compactTerminalHistory, so
closeBinding can't dead-end.

New — refreshing focused context is blocked, non-retryably, for the whole unacknowledged window

subsession-coordinator-store.ts:1088 widened the claim conflict from uncertain only to
["claimed", "submitted-unacknowledged", "uncertain"]. submitted-unacknowledged is the
normal state between kickoff submission (:1652) and the child's acknowledgement watermark
(:1737) — it is not evidence of anything ambiguous. The pruning filter at :1096 already keeps
those three states, so the widening buys nothing that the retention change needed.

Failure: a coordinator delegates and then refines the child's focus before that child emits its
first ack-able event — the ordinary sequence. refresh-focused-context throws claim_conflict,
which wholeCallError(refresh=true) maps to kickoff_failed, retryable:false, recovery:"inspect_session" (subsession-coordinator.ts:1188). The agent is told a permanent
failure for a state that clears on its own in seconds, and will not retry. If the child never
produces an acknowledgeable event (idle or exited early), that child is permanently
un-refreshable — the exact dead-end class this commit set out to remove. No test covers a
refresh against a submitted-unacknowledged delivery. Either keep the conflict at uncertain,
or map the transient states to retryable:true, recovery:"retry".

Not fixed — binding history is still monotonic for sessions that exit

compactTerminalHistory:746 reclaims only sessionState === "closed", and the only writer of
closed is an explicit user tab close (closeBinding, via the two-sided marker). A child that
simply finishes and exits stays exited in aggregate.bindings forever, as does every binding
of every dead parent session. So the bindings.length >= 8_192 guard (:1253) is still a
monotonic per-project ceiling reported as capacity_exceeded / retryable:false / recovery:"none", and the whole aggregate is re-parsed on every transaction long before that.
The changeset now claims "binding … history use bounded retention", which is true only of
user-closed bindings.

Nit: request_key_expired is reported to callers as request_key_reused
(subsession-coordinator.ts:1145) — the recovery is right, the code names something the caller
did not do.

Verdict: request changes — the refresh block is a new non-retryable failure on a routine
path.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since a67fc46a)

One commit moved (3a1ea262). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md) is clean.

Fixed: round-3's refresh block (claim conflict is back to uncertain only) and the
request_key_expired mislabel (now its own code, added to the exported
DelegationErrorCode). Binding history is now bounded — but see below for how.

New — reclaiming exited/failed bindings destroys the resume path this PR exists for

compactTerminalHistory:747 now tombstones and deletes bindings in closed, exited, and
failed
state once no retained receipt references them. exited and failed are not
terminal: transitions (:663-664) allows both → spawn-claimed, reserveDelegations:1233
counts them as live-resumable and returns the existing binding, and ensureSession:572 routes
exited into recoverExitedSession. Resuming a child requires the binding record.

Failure: parent delegates research; the child finishes its focused work and exits (also the
state of every child after an ordinary app quit). The parent then issues other delegations or
focused-context refreshes — each burns a receipt, and retention just dropped 1_024 → 256
(:39). Receipt R1 expires, the binding is reclaimed, and now:

  • re-delegating research with the identical digest hits the tombstone at :1239 and throws
    session_closedretryable:false, recovery:"inspect_session" — a permanent error naming
    a user close that never happened, on a key that can never be reused;
  • refresh-focused-context on it throws binding_not_foundcontext_not_found,
    non-retryable.

So whether an exited child is resumable or permanently gone depends on unrelated request
volume from its parent — directly contradicting docs/shared-build-plan.md:107 ("Identical
retries converge on the same durable binding and real Harness session ID"). The new test
(subsession-coordinator-store.test.ts:633) asserts the compaction but deliberately re-delegates
under a different key, so the resume-after-compaction case is uncovered. Reclaim closed
only, or gate exited/failed on an explicit non-resumable marker; the changeset should also
say resumability is now bounded, since it currently claims only that dead-ends are removed.

Verdict: request changes — the bound was needed, but as written it converts recoverable
sessions into permanently closed ones.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since 3a1ea262)

One commit moved (ac7a22b7). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md) is clean.

Fixed: round-4's finding — compactTerminalHistory:748 now reclaims closed only, so
an exited/failed binding is no longer tombstoned out from under its own resume path.

New — the 64-session cap became a lifetime cap, reported as retryable

The same commit moved exited and failed into the live filter
(subsession-coordinator-store.ts:1198-1207), where they were previously only counted via
additionalLive when a request actually re-referenced them. Combined with the compaction
change, nothing ever removes an exited/failed binding from aggregate.bindings, so every
child a project ever spawns permanently consumes one of the 64 slots.

The only writer of closed is an explicit human tab close (closeBinding:999, reached from
wasSubsessionClosedByUser at subsession-coordinator.ts:539 or the REST delete). The agent
has no release tool — project_subsession_delegate is the only one registered
(agent-map-mcp-tools.ts:385). Children also all land in exited on an ordinary app quit.

Failure: after 64 delegated children have finished across a project's lifetime, every further
delegation throws live_session_limit_reached, mapped at subsession-coordinator.ts:1163 to
capacity_exceeded / retryable:true / recovery:"retry" — so the agent is told to retry a
condition that can never clear without a human closing 64 tabs one by one. The new test
(subsession-coordinator-store.test.ts:684) asserts exactly this rejection with
liveSessionLimit: 2, so it is deliberate, but nothing covers recovery from it.
bindings.length >= 8_192 (:1249) is monotonic again for the same reason.

Reclaim exited/failed bindings that no retained receipt references from the live count
while keeping the record durable, or give the coordinator a release path. The changeset says
exited/failed bindings "remain durable so their real Harness sessions can still resume or
recover" without saying they permanently consume delegation capacity.

Verdict: request changes — round-4's fix traded a lost-resume dead-end for a
delegation-capacity dead-end that misreports itself as retryable.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since ac7a22b7)

One commit moved (97dc184a). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md) is clean.

Not fixed — #5 was relabelled, not resolved, and the new label names a recovery the caller cannot perform

The commit changes exactly one line of logic: live_session_limit_reached now maps to
capacity_exceeded / retryable:false / recovery:"inspect_session"
(subsession-coordinator.ts:1165). Round 5 asked for one of two things — reclaim
exited/failed bindings that no retained receipt references from the live count, or give
the coordinator a release path. Neither happened:

  • exited and failed are still in the live filter
    (subsession-coordinator-store.ts:1200-1208);
  • the only writer of closed is still a human tab close
    (closeBindingwasSubsessionClosedByUser at subsession-coordinator.ts:539, or
    closeOwnedBinding from the REST delete);
  • project_subsession_delegate is still the only registered tool
    (agent-map-mcp-tools.ts:386).

So the 64-slot bound remains a lifetime cap per project — every child that ever finishes, and
every child of every dead parent after an ordinary app quit, holds a slot forever. The 65th
delegation fails permanently, and the agent is now told to "inspect the session" with no tool
that can inspect or close one. Dropping retry removes a retry storm; it does not remove the
dead-end, and bindings.length >= SUBSESSION_COORDINATOR_BINDING_LIMIT (:1253) stays monotonic
for the same reason.

The published copy now asserts that missing affordance. The changeset — unretractable once in
CHANGELOG.md — says "callers inspect and close sessions instead of blindly retrying when that
bound is met", and docs/shared-build-plan.md:104 repeats it. The caller is the project agent,
which has neither capability; the only real remediation is a human closing 64 tabs by hand.
Either ship a release path, or say plainly that capacity is reclaimed only when a user closes a
delegated tab.

Verdict: request changes — the sole remaining finding is unchanged in substance, and the
docs/changeset now describe a recovery the shipped tool surface does not offer.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since 97dc184a)

One commit moved (d00469d7). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md, project-agent.ts prompt) is clean.

Fixed, partly: round-6's dead-end. release is a real tool variant, closed is
excluded from the live filter (subsession-coordinator-store.ts:1310-1320), so a parent
can now reclaim its own slots.

Partly fixed — orphaned children still hold live slots with no agent-reachable release

reserveReleases:1076 matches only parentSessionId === identity.sessionId, and nothing
cascades a close to children (no parentBindingId walk anywhere in
subsession-coordinator.ts). The live count is project-wide.

Failure: parent P delegates 5 children; the user closes P's tab, or P is itself a child that
exits. Those bindings sit in exited/ready and count toward the project's 64 forever — the
new parent session has a different sessionId and cannot name them. After enough dead
parents the 65th delegation still fails capacity_exceeded / retryable:false / inspect_session, and a human closing tabs is again the only remedy. No test covers a release
against a dead parent's bindings. Either scope release by project + binding ownership chain,
or cascade on parent close. The changeset's "recovering capacity" should say capacity is
reclaimed only while the delegating parent session is alive.

New — agent-driven churn can make the binding sidecar unloadable, and Studio won't start

close() never removes the entry from subsessionBindings, and userClosedSubsessions
only grows; both are serialized whole into sessions.json.subsession-bindings.json
(session-manager.ts:3653). Before this commit the 64-slot lifetime cap bounded that file;
release makes create→release→create an unbounded loop. At 8_192 markers or 2 MiB,
persistSubsessionBindings throws — every delegated create fails — and loadSubsessionBindings
then throws from init() (:1135), which is uncaught, so the whole SessionManager fails to
start for all sessions, not just delegated ones. Prune the marker and its
closedSessionIds entry on release/close.

New — one unknown key fails the whole batch, mislabelled session_closed

reserveReleases:1085 throws binding_not_found from inside the .map, so a 16-key release
containing one key whose tombstone has aged out releases none of the other 15 and returns
session_closed / retryable:false / inspect_session — naming a close that never happened for
a key the coordinator simply never had. delegate reports per-item failures; release should
too, or at least treat an unknown key as already-released.

Nit: SubsessionCoordinatorStoreEvent gained "subsession.released" (:117) but the store
never emits it — only the coordinator does.

Verdict: request changes — capacity reclamation still has a reachable dead end, and the
new churn path can brick session-manager startup.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since d00469d7)

One commit moved (fa5d2f97). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md) is clean.

Fixed: round-7's sidecar growth (close() now prunes the ownership marker and
closedSessionIds entry after a durable coordinator close, and re-adds them if the
final persist fails); the whole-batch binding_not_found on an aged-out release key
(now converges as absent → already-released); the unemitted
"subsession.released" store event.

New — a self-owned spawn claim is trusted even after it expires

subsession-coordinator.ts:730 and :844 short-circuit to {claimed: true} whenever
spawnClaim.ownerId === this.ownerId, before any expiry check. Previously both paths
routed an expired claim through takeoverExpiredSpawnClaim, whose atomic
expiredClaimId + epoch bump is what makes the fence single-winner.

Failure: a 16-item batch where earlier spawns exceed PROJECT_SUBSESSION_CLAIM_TTL_MS
(120 s) leaves item 16's own claim expired. A second coordinator instance retrying the
same request sees the expired claim, takes it over atomically, and spawns — while this
one proceeds on the stale claim and also calls createReserved/resumeBound. The
epoch fence only rejects the later attachSpawnedRuntime, after both processes
exist. Check expiresAt before the self-owned short-circuit and renew via takeover
with our own claimId. No test covers an expired self-owned claim (:545 passes a
fixed ownerId but never expires it).

Partly fixed — dormant bindings still accumulate without bound

Dormancy removes the 64-slot dead-end. But compactTerminalHistory:772 reclaims
closed only, and reserveReleases:1076 still matches parentSessionId === identity.sessionId, so children of a dead parent are unreleasable and
aggregate.bindings stays monotonic to history_quota_exceeded at 8_192 — the same
dead-end, four orders of magnitude out.

Verdict: request changes — the release path is sound now, but the new self-owner
fast path removes the concurrency fence it was meant to skip.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since fa5d2f97)

One commit moved (7efe577d). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md, project-agent.ts prompt, tool description) is clean.

Fixed: round-8's expired self-owned spawn claim — both fast paths now require
spawnClaim.expiresAt > now (subsession-coordinator.ts:785, :900) and fall through to
claimSpawn/takeoverExpiredSpawnClaim, with a two-coordinator test at
subsession-coordinator.test.ts ("atomically renews an expired self-owned spawn claim").
Also fixed: dormant bindings of a dead parent are now reclaimable via release-dormant,
which selects server-side, skips anything not exited/failed inside the reserve
transaction, and compacts through finalizeReleasedBinding — so aggregate.bindings is no
longer monotonic and the operation stays callable at the binding limit.

New — the one error release-dormant exists to clear still reports itself unrecoverable

history_quota_exceeded maps to capacity_exceeded / retryable:false / recovery:"none"
(subsession-coordinator.ts:1395-1400) — unchanged by this commit.

Failure: a project reaches SUBSESSION_COORDINATOR_BINDING_LIMIT with dormant children of
dead parents. The next delegate returns recovery:"none", which tells the agent there is
no remedy, while the shipped remedy is one release-dormant call away. The agent's only
pointer to it is prose in project-agent.ts, not the machine-readable field it is built to
branch on — and the changeset and docs/shared-build-plan.md:130 both publish the claim
that the condition is recoverable. live_session_limit_reached has the same shape
(inspect_session, not "release"), but at least names an action. Map both to a recovery
value that names the release path, or add one.

Nit: releaseDormant proves parent liveness from a read outside the transaction
(subsession-coordinator.ts:307-317); reserveDormantReleases re-checks sessionState but
not the parent, so a parent that becomes reachable in that window still loses its child's
resume.

Verdict: approve with a nit — the substantive gaps from rounds 7–8 are closed; the
remaining issue is that the error contract does not advertise the recovery this PR adds.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since 7efe577d)

One commit moved (78ea86d8). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md, project-agent.ts prompt, tool description) is clean.

Fixed: round-9's finding — history_quota_exceeded now maps to
capacity_exceeded / retryable:false / recovery:"release_dormant"
(subsession-coordinator.ts:1390-1393), with live_session_limit_reached left on
inspect_session. Round-9's nit is moot: the out-of-transaction parent-liveness read is
gone entirely.

New — a sibling agent's sweep permanently bricks a live parent's delegation key

Dropping the parent-liveness filter (subsession-coordinator.ts:311) makes selection
project-wide, so any project agent can now tombstone an active parent's exited child.
Tombstoning is terminal for that key:

  • identical retry of the original request → replay finds the tombstone and throws
    session_closed (subsession-coordinator-store.ts:1516-1520);
  • fresh request, same delegationKeysession_closed again (:1594-1602).

Both surface as session_closed / retryable:false / recovery:"inspect_session"
(subsession-coordinator.ts:1399) — naming a user close that never happened, on a key the
live parent can never reuse. Failure: P delegates research; the child exits; sibling agent
Q hits durable capacity, follows the newly-advertised release_dormant recovery, and the
oldest-updatedAt candidate is P's child. P's next identical retry hard-fails, contradicting
docs/shared-build-plan.md:107 ("Identical retries converge on the same durable binding and
real Harness session ID"). This is round-4's dead-end re-entered through a different door;
the parent-liveness filter was what previously bounded it to keys nobody could still name.

The new test (subsession-coordinator.test.ts:507) deliberately covers the active-parent
sweep, but only asserts the parent process survives — no test re-delegates that key
afterwards. Either scope the sweep to bindings whose parent is absent/exited inside the
transaction, or make a dormant-released key behave as absent (re-creatable) rather than
session_closed. The changeset says the binding "is no longer automatically resumable"; it
does not say a live parent's identical retry then fails permanently.

Verdict: request changes — the recovery code is right now, but making the sweep
project-wide lets one agent void another's idempotency contract.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since 78ea86d8)

One commit moved (a66e4d86). Public-copy hygiene on the delta (changeset,
docs/shared-build-plan.md, project-agent.ts prompt) is clean.

Fixed: round-10's finding. Dormant eviction now writes a
disposition: "dormant-evicted" tombstone that the terminal lookup
(subsession-coordinator-store.ts:1629) and the parse uniqueness check (:637-648)
both skip, and it expires every non-release-dormant receipt that referenced the
evicted binding (:1300-1318). A live parent's identical retry now returns
request_key_expired / new_request_key, and a fresh request key recreates one
binding/session under the same delegation key — covered by
subsession-coordinator.test.ts:507.

New — release-dormant now emits no subsession.released telemetry

Eviction returns {state: "released"} instead of {state: "bound"}
(subsession-coordinator-store.ts:1291), so releaseReserved takes the branch at
subsession-coordinator.ts:350, which never emits. The only subsession.released
emit is at :432, in the bound branch the dormant path no longer reaches.

Failure: a project-wide sweep closes N delegated sessions and the event stream shows
nothing — subsession.requested in, sessions gone, no release event. The one
destructive, project-wide operation this PR adds is the one with no observability.
The existing telemetry test (subsession-coordinator.test.ts:415) covers the explicit
release path only, so nothing caught it.

New — a failed private close reports failed for an eviction that already committed

Same branch: the durable tombstone and receipt commit inside
reserveDormantReleases, before closeBound. If the marker check at
subsession-coordinator.ts:356-367 throws, the item comes back
outcome: "failed" / binding_session_mismatch while the binding is already destroyed
and its receipts expired. Previously the mismatch aborted before any durable write.
The agent is told the child survived; its next resume of that key won't find it.

Verdict: approve with changes — round-10's dead-end is genuinely closed; both new
items are in the eviction result path, not the durable contract.

ynadge commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #808 (delta since a66e4d86)

One commit moved (ca747232). Public-copy hygiene on the delta (changeset addition,
docs/shared-build-plan.md) is clean — no company names, arrangements, or internal context.

Both round-11 findings are fixed. reserveDormantReleases now returns a distinct
state: "evicted" for the binding it just committed (subsession-coordinator-store.ts:1296),
so releaseReserved emits exactly one content-free subsession.released
(subsession-coordinator.ts:353-359) and a replay — which resolves to "released" — emits
none. The cleanup-failure branch no longer overrides outcome to failed, so a durable
eviction reports released plus the bounded error. Both are covered by
subsession-coordinator.test.ts:648 (telemetry count 1, marker retained then pruned on replay)
and the store test at :1044.

New — a stranded private marker becomes unreachable once the sweep receipt expires

Retrying the failed cleanup requires replaying the same request key: the evicted binding is
removed from aggregate.bindings (subsession-coordinator-store.ts:1316), and
releaseDormant selects candidates only from aggregate.bindings
(subsession-coordinator.ts:310-319), so no future sweep under a new key can ever re-select it.
Once that receipt ages past receiptRetentionLimit into requestTombstones, the same-key
replay is refused with request_key_expired (:1267) and the exact private marker — and,
since closeBound failed, possibly a live child PTY — is orphaned with no coordinator-side
route to clean it. The changeset now promises the opposite: "retains exact proof for idempotent
cleanup after the indicated recovery." Either exempt receipts with unfinished cleanup from
expiry, or let release-dormant also consider tombstones whose private marker still exists.

Nothing the previous round said needs correcting.

Verdict: approve with changes — the two reported items are genuinely closed; the remaining
gap is a bounded retry window on the same cleanup-failure path, not the durable contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant