Repository navigation
feat(rpc): ordered host acceptance lane, worker frame ordinals and bootstrap before admission (part of #4272) - #4333
Merged
Merged
Conversation
…otstrap before admission (part of #4272) S04a of R3.C1.1 behind the engine-socket compact-updates flag (off by default): one acceptance lane per routing session minting sessionEpoch and accepted sequence before SessionEventWriter.enqueue; worker frames checked by ordinal, applied, then credited; bootstrap accepted before open; correlation transitions sequenced; producer-minted messageId and lane-minted turnId (ruling 12); event-resync shed freezes and releases roots on every path.
One named test per contract case, split by behaviour: Q1 root release on takeSnapshot, drain failure, seal by close and by abort, and peer drop; Q2 a new sessionEpoch on new_session, fork and switch_session, with refs never interleaved across epochs; Q4 gap, duplicate and reorder as separate tests asserting the typed failure, denied credit and a bounded worker exit; Q6 correlation sequenced with its content across worker frames; bootstrap accepted before the entry opens, router admission and the open reply in both runtimes. The Q1 and Q4 it.each blocks move into the new files. No production change.
…pted entry ids (part of #4272) Review fold for #4333. A correlatable record without client identity after a correlation stood (a trigger turn, an extension steer) is now sequenced as the end of that correlation, so a cut attributes it to no client message. The host lane keeps each accepted transcript append as its entry id only, with no serialized copy and no stringify on the host loop. The writer-refusal test now exercises accept-then-refusal with the session still open.
…t and drop stringify from correlation compare (part of #4272) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
…d-lane # Conflicts: # packages/coding-agent/src/modes/rpc/session-worker-client.ts
…d-lane # Conflicts: # packages/coding-agent/src/core/agent-session.ts
…lane now requires (part of #4272)
…d-lane # Conflicts: # packages/coding-agent/src/core/agent-session.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #4272.
Summary
S04a of contract clause R3.C1.1 (ruling 10 split): one ordered acceptance lane per routing session. With the engine-socket compact-updates flag on (
<brand>_ENGINE_SOCKET_COMPACT_UPDATES, explicit-true, OFF by default), every in-process record and every worker output frame enters the lane beforeSessionEventWriter.enqueue. The lane is host-owned. It mints asessionEpochper AgentSession incarnation and an acceptedsequence, and it holds a minimal transcript root containing only accepted appends. Worker frames carry a per-incarnation ordinal; the host checks the ordinal, applies the frame, and only then grants credit. Bootstrap is accepted before the session opens. Off, nothing is constructed and every byte, frame and timing is unchanged.Ruling 12 places the
messageId/turnIdmint here: the producer (AgentSession) mintsmessageId, and the lane mintsturnIdand capturesmessageIdat acceptance.What changed
session-acceptance-lane.ts(new):bootstrap,accept,freeze,release,retire.entry_appendedandsession_abort. Responses and every other record pass through in FIFO order with no sequence.clientMessageId/clientTurnId/clientMessages) is its own transition, sequenced just before its record, so the flushed turn prefix is sequenced in order with its content (Q6). The end of a correlation is a transition too: a correlatable record with no client identity (a trigger turn, an extension steer) after a correlation stood is sequenced as "no correlation".count.core/session-recovery-identity.ts(new) andagent-session.ts: opaquerandomUUID()message ids.message_start. Amessage_endwith no start mints at that end; an update with no start gets none.ClientMessageEvents,tagSessionRecord) forward the carrier.recovery.session-registry-open.tsbootstraps the entry's lane beforeentry.state = "open", so router admission and its reply come later.session-binding.tsaccepts each record beforewriter.enqueue;session_replacedbootstraps the next epoch. The isolate's own registry (workerIsolate) opens no lane, so the host owns exactly one.session-worker*.ts):commit { acceptance }asks the isolate for a bootstrap onready(accepted beforecommitresolves) and for ordered output frames (ordinal,recovery, plusbootstraponsession_replaced).session_worker_frame_gap/session_worker_frame_duplicate.writer.enqueue; credit follows the existing backpressure wait.takeSnapshot, on actor close (peer drop), on drain failure, and on session seal (lane retire). Resync bytes, reasons and order are unchanged (byte test).RED / GREEN (contract test ids)
RED ran at base production code:
src/stashed, new tests present. GREEN ran at this head. Logs:s04a-red.log,s04a-green.log,s04a-green-worker.log.4272-worker-acceptance-cut"T5/X03 holds worker acceptance…"4272-worker-frame-order(see the contract map below)4272-worker-frame-order"Q4 writer refusal…"4272-inprocess-acceptance"T5 sequences in-process records…"correlation … expected null)turnIdfor a turn with noturn_start, nothing invented4272-resync-root-release-paths×5 (see the contract map below)registerAcceptancemissing4272-resync-cut-release"keeps the resync bytes"RED summary at base: 3 files, 12 failed / 3 passed (exit 1). GREEN: the three S04a files plus 33 neighbouring RPC suites (bindings, writer, fanout, resync, structured sink incl. its byte test, worker routing/reservations/isolation/output/rebind, socket host, multi-session) at the formatted tree: 36 files, 198 tests, exit 0 (
s04a-neighbours.log). The worker file alone: 6/6, exit 0 (s04a-green-worker.log). The earliers04a-green.logis the first GREEN attempt, before three test-side fixes: prompt dispatched inside its connection, held reconcile selected by thesession_replacedframe, transcript count read from disk.Contract test map (ruling 10)
One named test per case. Paths are under
packages/coding-agent/test/suite/regressions/. Worker cases use the realSessionWorkerClient,WorkerSessionRegistryand router through4272-worker-acceptance-support.ts. Every wait there is bounded, so a hang fails the test instead of stalling it.4272-resync-root-release-paths.test.ts:144272-resync-root-release-paths.test.ts:214272-resync-root-release-paths.test.ts:274272-resync-root-release-paths.test.ts:334272-resync-root-release-paths.test.ts:394272-resync-cut-release.test.ts:64272-epoch-per-replacement.test.ts:1144272-epoch-per-replacement.test.ts:1184272-epoch-per-replacement.test.ts:1224272-worker-acceptance-cut.test.ts:204272-worker-frame-order.test.ts:504272-worker-frame-order.test.ts:594272-worker-frame-order.test.ts:704272-worker-frame-order.test.ts:794272-inprocess-acceptance.test.ts:394272-worker-correlation-order.test.ts:114272-worker-correlation-order.test.ts:774272-inprocess-acceptance.test.ts:1924272-bootstrap-before-admission.test.ts:704272-bootstrap-before-admission.test.ts:95What each asserts:
session_replacedall carry the old epoch, and every later ref carries the new one, with no interleaving. The new epoch's numbering restarts at 1, below the old epoch's last sequence, so sequences order only within their epoch.conflict, the registry gets exactly one typed failure, the lane is retired, the sequence and epoch are unchanged, and the worker exits within the bound. A reorder reaches the host as the later frame first, so its typed kind issession_worker_frame_gap. The late frame is refused as well, with no second failure.turn_startframe advances the sequence by exactly 2: the correlation transition at n-1, its record at n. The next frame takes n+1, and every later frame keeps the same correlation transition and takes at most one sequence.opening), then commit resolves (worker; entry stillopening), then router admission (entryopen), then theopen_sessionreply. All four see one epoch, at sequence 0 through admission.The new tests cover existing behaviour and needed no production change. Mutation checks, each run against the file it targets with the source restored afterwards:
bootstraponsession_replacedtakeSnapshotstops releasing its cutentry.state = "open"Logs:
s04a-gaps-green.log,s04a-gaps-mut-M{1,2,3,4}.log.Review fold (review at 73348f2: FIX_FIRST, 2 MEDIUM + LOW-1)
Fold commit
05e56d1d5b. Each fix had its RED first (s04a-fold-red.log, production code atfc3d965cba), then GREEN (s04a-fold-green.log).session-acceptance-lane.ts:162-163. A record with neitherclientMessageIdnorclientTurnIdstill returned early unless a correlation stood and the record is a correlated kind.correlatedRecord(client-message-events.ts:10) is the one predicate for those kinds;ClientMessageEvents.output(:95) uses it too, so the lane and the emitter cannot drift. The end takes its own sequence just before the first uncorrelated record, and later uncorrelated records add no transition. Test4272-worker-correlation-order.test.ts:77, "Q6 worker: a trigger turn after a client turn sequences the end of the correlation…": a real worker getsprompt { clientMessageId: "c-1" }and thensend_custom_message { triggerTurn: true }. RED at the parent:the cut attributes the trigger turn to no client message: expected 'c-1' to be undefined. GREEN: pass.session-acceptance-lane.ts:182-187keeps each accepted entry's id (TranscriptRoot.entryIds) and drops theJSON.stringify(entry). No serialized copy is needed for correctness until S04b, because nothing reads content from the root in S04a. That fits S04-design's persisted base{fileIdentity, count, idFingerprint}: S04b resolves content through its base and references, and ids cannot be mutated by mirror trimming. So I removed the copy and added no byte cap. Test4272-inprocess-acceptance.test.ts:192, "keeps host retention per accepted append bounded…": 64 appends of 256 KiB entries. It asserts zeroJSON.stringifycalls during accept, ids in order up tocount, no alias after the entries are mutated, and a root under 64×16+128 bytes. RED at the parent:no serialization per append on the host loop: expected 64 to be +0. GREEN: pass.4272-worker-frame-order.test.ts:79. The session stays open and the real writer refuses only the injected frame. The test asserts the lane accepted the frame before the refusal (sequence +1, not retired), creditconflict, the typed failure, the lane retired, zero live roots (a root held across the frame is released) and no acceptance afterwards. Mutation proof, with the source restored afterwards:!writer.enqueue(...) && false): the new test fails witha refused frame gets no credit: expected 1 to be 2(s04a-fold-low1-mutB-new.log).writer.enqueuesucceeds (breaks the accept-then-refusal order): the new test fails withthe lane accepted the frame before the writer refused it: expected { sequence: 1, retired: false } to deeply equal { sequence: 2, retired: false }. The old parent test still passes under M-C, which is the LOW-1 defect (s04a-fold-low1-mutC-{new,old}.log).fail()no longer retires the lane: both tests still pass, becausecallbacks.failurealso retires the lane through the registry'sforgetSession. Retirement has two routes, so M-A is not a break (s04a-fold-low1-mutA-{new,old}.log).session-recovery-identity.ts:43: no current path sends a direct assistantmessage_startwhile a stream is open. LOW-3, the flag accepting1as well astrue: a wording choice for the contract owner.Moved to S04b, and why
{fileIdentity, count, idFingerprint}) and Q3's typed mismatch failure belong to S04b. So do branch/reset/replacement roots and the worker fork bootstrap carrying the branched root. In S04a a replacement bootstrap carries only the new leaf and an empty appended list.runtimeItemKey(ruling 12 condition 3) belongs to the S05/S06 row model; S04a adds no row.Production size
Production diff against origin/main after the review fold: 17 files, +405 / -27 (the fold added +25 / -9 net in
session-acceptance-lane.tsandclient-message-events.ts). Before the fold: +380 / -18. 312 added lines are code, the rest are comments. The new files aresession-acceptance-lane.ts(147 pure LOC),session-recovery-identity.ts(30) andengine-socket-flag.ts(5). Tests: 10 files, +1053 (the conformance-gap commit added +504 / -114, no production lines). This is within the ~340-390 estimate and under the 600 stop.Gates
At head 4055ecb:
bun run buildexit 0;tsc --noEmitexit 0 (re-run after the final type-export edit);node scripts/lint-ratchet.mjsexit 0 (every count matches the baseline);oxfmt --checkon changed files exit 0;bun run checkexit 0. Logss04a-{build,tsc,tsc-final,ratchet,oxfmt,check}.log. No senpi-qa CLI run: flag off means no user-reachable change.At head 05e56d1 (review fold):
bun run buildexit 0;tsc --noEmitexit 0;oxfmt --checkandoxlint --deny-warningson the 5 changed source/test files exit 0;git addthennode scripts/lint-ratchet.mjsexit 0 (every count matches the baseline);check-pr-changelog.mjs --base origin/main --labels no-changelogexit 0 (the PR carries the label). All eight4272-*files plussession-record-stream-projection,rpc-client-message-identityandrpc-client-message-ledger: 11 files, 43 tests, exit 0. Logss04a-fold-{build,tsc,oxfmt,oxlint,ratchet,changelog-label,red,green}.log.At head fc3d965 (conformance-gap tests, no production change):
bun run buildexit 0;tsc --noEmitexit 0;git addthennode scripts/lint-ratchet.mjsexit 0 (every count matches the baseline);oxfmt --checkon the 9 changed files exit 0; all eight4272-*test files, 22 tests, exit 0. Logss04a-gaps-{build,tsc,ratchet,oxfmt,green}.log.Notes for review
core/engine-socket-flag.tswith the env name S02b uses (ENGINE_SOCKET_COMPACT_UPDATES). S02b'sstructuredLaneEnabled()should import it when it rebases (LOW conflict).SessionAcceptanceLane.liveRootshas no production reader. It is the observable for the Q1 leak test;retiredis read by the fanout.session-event-writer.ts(1080 lines) andagent-session.tsare already far over the 250-line file ceiling. This PR adds 6 and 8 lines there and puts the logic in new files.