Repository navigation
Conversation
The runner kept command lifecycle in two places: the transport's in-flight id set and the command journal. They were cleared at different moments, so a repeat of a commandId whose answer had already been delivered was admitted again and ran the command a second time — a second tap. Pruning could also evict a command that was still executing, which the daemon reads as notAccepted, and a late transition for a pruned id could invent an entry the runner never claimed. RunnerCommandJournal now answers each inbound request with one verdict: execute, join, or terminal. A terminal id is answered from the recorded outcome, then the recorded error, then COMMAND_ALREADY_FINISHED. Pruning drops only closed entries, and an update never opens a record again. The journal closes a commandId at the last fast RUNNER_BUSY or RUNNER_WEDGED refusal, so the read-only busy resend keeps its reused id (ADR 0005). The transport keeps only the waiter list.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
thymikee
left a comment
thymikee
left a comment
There was a problem hiding this comment.
Independent review: needs changes. Reviewed exactly merge-base f7879cf2d266ee1dd476d0342b2819bdeceb647b → head e1c2cae64d14ec04da1d03a92782f110578e7835 against the coordinator's once-per-command-id admission requirement.
-
P1 — Pruning makes a completed mutation executable again.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandJournal.swift:209-213(source). Finish a dispatched tap with idT, then finish 64 other commands. Pruning deletes every record ofT;accept(T)takes the missing-entry branch at lines 62–63 and returnsexecute, so a delayed resend can tap again. This is also immediate under overload: with 65 non-terminal entries, finishing one can evict that very entry. Retaining only response bodies for a window is reasonable, but deleting the admission history violates the required “a completed or failed command id is never executed again.” Fix: preserve closed-id admission history independently of retained outcomes, so an evicted outcome yields a typed refusal rather than a new execution. If admission history must be bounded, enforce a safe admission/session boundary instead of silently treating previously executed ids as new. Add a transport regression that resends a finished mutation after retention pressure. -
P2 — A joined request can miss delivery and remain retained forever.
apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Transport.swift:279-287(source). Keep 65 commands non-terminal. OntransportQueue,accept(T)returnsjoin. BeforeattachToInFlightExecutionacquires its lock,commandExecutionQueuefinishesT, pruning its now-terminal entry, anddeliverCommandResultdrains its waiters. Attachment then finds noclosedOutcome, appends the completion, and no execution remains to wake it. The connection receives no answer and its completion remains ininFlightCommandWaiters. The same issue applies tofail, which uses the same pruning update. The journal still has its ownNSLock; journal closure does not acquireinFlightCommandLockas the attachment comment claims. Fix: make admission and joiner subscription one atomic lifecycle operation, and preserve the outcome until all admitted joiners can receive it. Merely checking the current entry after the separate admission decision is insufficient. Add a deterministic test for this interleaving on both finish and fail.
Verification:
- In a disposable checkout,
pnpm install --frozen-lockfile,pnpm build, andpnpm check:xctest-selectionpassed. Focused apple-runner dispatch disclosure, session types, busy resend, and error classification tests: 75 passed across 4 files. - A standalone macOS XCTest executable used the actual journal source and extracted transport attach/deliver methods with minimal command/response fixtures. The PR's two pure journal regression test bodies both fail on base for the intended assertions (2 tests, 2 failures) and pass on head. The head harness ran 9 tests: the two findings above fail with 3 assertions; concurrent admission (100 sends → one
execute), ordinary failed-execution waiter delivery, terminal-state immutability, and busy-refusal re-admission pass. This is focused source-level evidence, not a full runner/Xcode lane pass; I did not independently run the PR's listener regression on base. - At capacity with every entry non-terminal, the journal does not refuse admission: all 256 probe ids returned
executeand remainedaccepted. The runner enqueues them; there is no capacity-specific response or hard bound. Non-terminal entries are preserved and continue reportingaccepted/started, matchingRUNNER_IN_FLIGHT_LIFECYCLE_STATES; the problem is what happens when an entry then closes. COMMAND_ALREADY_FINISHEDcurrently becomes a structured, non-retryable error withdispatched: unknown; it is neither success nor a busy/pre-send refusal. No retry storm reproduced. No TypeScript files changed. The intentional pre-dispatchRUNNER_BUSYresend remains available.- Simulator coverage needs qualification: the source-derived PR iOS Simulator lane selects none of the five new/changed admission tests. The macOS host lane and full nightly iOS Simulator lane include all five. These changed journal/transport methods and tests compile on both platforms, so a manually selected iOS run exercises the shared logic, but the normal PR simulator selection does not replace the unavailable host run. I did not rerun the full native host or simulator lanes or wait for CI.
Reviewed checkout remains clean; no source edits, commits, or pushes.
Apex SentinelVerdict: Since the last review: 8 new · 0 still open · 6 no longer raised 8 finding(s): 1 minor, 7 nit. Verdict: approve. reached max rounds (1). 🟡 Minor (1)
🔵 Nit (7)
Reply under an inline comment with |
![apex-sentinel-staging[bot]](https://avatars.githubusercontent.com/in/5196457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Apex Sentinel: 4 new findings on e1c2cae. The verdict and the full list are in the summary comment.
|
At e1c2cae, the admission change breaks recovery for a read whose reply was lost. The mutation side looks right. The daemon resends a read with the same commandId when it did not receive the reply (index.ts:141-160 sets the id once, outside the retry loop; runner-command-recovery.ts:111-120 rethrows the transport error so the loop resends). Not blocking, and you can take or leave these: the two new tests in Of the open inline threads, these still apply: terminal wording in the ADR, entry stays open wording, replay drops error code, status answered before accept, and vacuous coalesce assertion. Five lower-priority threads also still apply. These do not apply, so please resolve them: uptime anchor on replay (the response stamps current uptime at send time and replay stores no stamp), post-dispatch busy refusals ( The PR body's live run was at d22b69a, not e1c2cae, and it covered only the mutation repeat. Please post a run at the final head on a simulator, sending each of these twice straight to the listener with one fixed commandId, and quote both The Repo Guards failure comes from this PR: the jscpd clone at |
… one decision Pruning an entry kept the entry's admission fact with the body it shared, so a pruned id became executable again and a delayed resend could drive the device a second time. Pruning now leaves the id in a bounded closed set that answers admission with the typed refusal, and a pruned refusal keeps its id free for the busy resend (ADR 0005). The join verdict and registering the send for the execution's answer were two steps, so a close that landed between them left the send waiting on a delivery that had already gone out. They are one operation the journal's lock covers.
|
Dispositions for both needs-changes findings. Head P1 — pruned id executable again ( Rejected the watermark: Two marker rules: a pruned entry that never dispatched leaves its id free, otherwise the P2 — join decided before the close ( Fail-then-pass, iOS Simulator lane, disposable worktrees. P1 at base Checks: focused Swift lane Note for lane planning: the PR iOS simulator lane selects only the 5 shared-branch tests, so these admission tests run on the macOS host lane and the nightly, not the PR simulator lane. The host lane cannot enter automation mode on this machine. |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Apex Sentinel: 8 new findings on 3bbe6a8. The verdict and the full list are in the summary comment.
thymikee
left a comment
thymikee
left a comment
There was a problem hiding this comment.
Round-2 independent review: needs changes. Reviewed e1c2cae64d14ec04da1d03a92782f110578e7835 → 3bbe6a889005ec13c898b347b21989b57a243e0a and affected runner/daemon interactions.
P1 remains — FIFO marker eviction makes a completed mutation executable again. apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandJournal.swift:232–233. Complete a dispatched tap T, then complete 4,159 distinct dispatched commands: its outcome is pruned but its marker still refuses a resend with COMMAND_ALREADY_FINISHED. Complete one more distinct dispatched command: inserting the 4,097th marker removes T via removeFirst(). Resend T: accept falls through to claim at line 88 and returns execute, so the transport enqueues the tap again. I reproduced these exact verdicts with the actual journal source in a standalone Swift probe. Marker access does not refresh FIFO retention.
The disposition explicitly acknowledges this residual behavior. The PR body says “bounded closed-id window,” and ADR 0005 specifies 4,096 ids, but neither establishes a safe replay/admission boundary; the ADR still requires “Never execute a commandId twice.” Thus the original correctness failure is postponed, not resolved. Random UUIDs explain why an ordering watermark is unavailable, but do not make silently forgetting executed ids safe. Fix: retain admission history for the runner's admission lifetime, or enforce a typed refusal/session-generation boundary before dropping markers that could still be resent. Add a regression crossing the marker limit. If the intended contract instead permits duplicate execution outside this window, that is a deliberate weakening of the once-per-id requirement that needs explicit acceptance and consistent wording.
P2 resolved. accept(command:registeringJoin:) holds the journal lock through registerJoiner. The latter only appends under inFlightCommandLock; the only other production acquisition drains the waiter array and unlocks before invoking completions. No production path acquires the journal lock while holding inFlightCommandLock. A source-level probe forced both finish and fail to race registration while closing immediately pruned the entry: both waited for registration, delivered the joined answer once, and left no waiter.
Other requested interactions checked:
COMMAND_ALREADY_FINISHEDremainsok: falseand surfaces asAppError.codeanddetails.runnerErrorCode = COMMAND_ALREADY_FINISHED, withdispatched: unknown, no retriable flag, and structured-failure classification. Direct assertions confirmed no success, busy resend, pre-send refusal, or restart classification. The read-only resend budget is one attempt for this error.- A marker-only id reports
notAccepted. Daemon charge settlement does not treat that as terminal; lost-reply recovery invalidates conservatively and fails a mutation asrunner_reply_lost, rather than reporting success or replaying it. Therunner-session-types.tscomment saying “never held” is now stale, but its state handling remains conservative. - Both fast busy refusals precede
markDispatched. An undispatched refusal can be re-admitted, including after pruning; once dispatch is marked, the entry stays terminal. The command execution queue delivers the original result before executing its next queued command.
Validation in a disposable checkout: pnpm install --frozen-lockfile, pnpm build, and pnpm check:xctest-selection passed (527 declared methods, zero unreachable). Six affected apple-runner TypeScript suites passed: 156 tests (dispatch disclosure, session types, busy resend, error classification, command recovery, recovery wiring). The initial unit-core invocation selected no tests; the actual run used --project apple-runner. The Swift probe used the production journal and extracted register/deliver methods with minimal wire fixtures; it is not a full native runner lane or device run. No CI wait. Both checkouts remain clean; no source edits, commits, or pushes.
|
Thanks for the update. At 3bbe6a8 the join race and the fast-refusal re-admission are fixed, but a lost-reply resend of a read still fails, and this change makes that failure last longer. I read the code only. I did not run Swift tests or a device, so the fail-then-pass counts, the 76-test lane and the check:affected receipt are unverified. The earlier finding on RunnerTests+CommandJournal.swift:96 still stands, and this delta widens it. The daemon mints one commandId and resends a read with the same id after a lost reply (index.ts:143-160). A snapshot is marked dispatched and keeps no journal body (Models.swift:283), and the same holds for screenshot and any read over 16 KB. So the resend gets The admission route at Transport.swift:176 changed again in this delta, but the posted live run covers only a pasteboard mutation repeat and quotes no runner.log excerpt. The read repeat, which is the route that regressed, was never run. After the fix above, please post a simulator run at the final head that sends each command twice straight to the listener with one fixed commandId, and quote both runner.log excerpts. For tap or pasteboardWrite, I expect one Could the same trait make this simpler overall? If "can re-running change device state" is declared once, reads never spend an id because they re-run or replay, and only dispatched mutations spend one and leave a closed marker. That removes the read regression, makes the closed-id set mutation-only (smaller, and the FIFO-eviction residual then applies only to mutations, which the daemon sends once at index.ts:144-146), and avoids adding another guard around Smoke Tests (apple.yml smoke-apple) is still running, and I have no failure excerpt, so the outcome at this head is unknown. That job drives real runner commands through On the open inline threads: these still apply as P2 and need an answer: weak 'already stopped' assertion, test uses a mutating command, ADR 'terminal' wording, replay loses error code, ADR 'stays open' wording and ADR 'once per request' wording. Five lower-priority threads still apply as well. These do not apply and can be resolved: uptime stamp on replay (the journal JSON is stored unstamped, so replay pairs current uptime with the current receipt); fixed at this head: atomic join registration; 4160-command window (no producer reaches it, so please only state the bound at ADR:132); pruned-id test (prune removes the refused entry, and an optional status assertion would only add clarity); RUNNER_BUSY/WEDGED (both return before Before merge, the read trait, the ADR amendment, the listener snapshot-twice test and the two-route live run at the final head are needed. |
…ion boundary The daemon holds no per-id instant after which it never resends a commandId: its resend budgets are attempt counts and per-exchange timeouts redrawn per attempt, and a restart-and-replay posts the same id after a replacement boot. Tieing marker retention to such a deadline would need a wire-contract change, so the journal keeps a spent id for the life of the runner process and the 4096 cap is named for what it is: a memory safety valve. The eviction site and ADR 0005 now state the residual bound, and one test pins both halves of the rule — refused for the whole window, oldest id released by exactly one close past it.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on b9aa7ae. The verdict and the full list are in the summary comment.
|
Disposition for the round-2 P1 bound. Head The deadline option is not available without a contract change, so this PR does not add one. I checked every path that resends a
The runner's own What this PR does instead. The 4096 cap is renamed to say what it is — Regression crossing the marker limit — Checks at this head: focused Swift lane (journal, transport, dispatch, execution, recording, main-thread, models, alert) |
|
Coordinator check of round 3 (
No blocking findings. P2 was resolved in round 2. |
|
I reviewed b9aa7ae against the earlier review at 3bbe6a8. One finding from it is still open, and there is one new ask for live evidence. All 19 checks pass, and no conflicts are known. Before merge, the read-repeat regression must be fixed and the simulator run posted. A lost reply to This PR changes the admission route at RunnerTests+Transport.swift#L176 that every runner command takes. The posted live run covers one pasteboard repeat and quotes no Not blocking: in ADR 0005 line 137, "A journal that survives a runner restart is a new journal" contradicts itself, so "A runner restart starts a new journal." is clearer, and you can take or leave it. The open inline threads from other reviewers still apply for these topics: dispatch assertion in test, recordStop test route, ADR terminal wording, failure code lost, ADR "stays open" wording and "once per request" wording. Five lower-priority threads also still apply. These threads do not apply, so please resolve them: r4435424717 (the journal stores the unstamped response, so a replay pairs current uptime with the current receipt); r4435768779 (the 4096-id bound is stated and accepted, and no producer resends a mutation); r4435768783 (the This was a read-only review. I did not run the Swift lanes, |
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on 3f8401d. The verdict and the full list are in the summary comment.
…ancy The daemon marked the runner drained on any structured failure it did not classify as occupancy. A repeat refused by the command journal is that shape: the journal answered from its record without executing, so it observed nothing about the main thread, and a refusal landing mid-drain was read as proof the abandoned work had drained. Retention and handoff act on that report, so a runner still grinding XCTest work became eligible for reuse. Occupancy is now one three-valued verdict from the classifier, keyed on the typed runner code, and the silent reading leaves the report standing. The runner keeps stamping failures as it did: a stamp on a failure would move every other failure route, not this one.
Review round 4 cleanups, none of them a behavior change. The admission doc names the selector the code has, accept(command:registeringJoin:), and so does ADR 0005. The COMMAND_ALREADY_FINISHED code comment now states the disclosure the daemon actually publishes, unknown. ADR 0005 defines terminal as finished execution rather than an effect on the device, says a pre-dispatch refusal closes its entry as failed while leaving the id reusable, says a runner restart starts a new journal, and scopes the 4096 bound to lost-response resends, since an id refused before dispatch never enters the window. The daemon comment keeps one clause for notAccepted, because the wire never distinguishes a still-marked pruned id. In tests: the busy-resend test drops a stray try and its read-only claim, the pruned-refusal test asserts its own prune precondition, the closed-id window test proves the valve releases exactly one id by still refusing the next-oldest, and the duplicated transport comment goes. The already-finished refusal now builds two whole messages rather than splicing a clause into another sentence.
3f8401d to
bb83f90
Compare
There was a problem hiding this comment.
Apex Sentinel: 9 new findings on bb83f90. The verdict and the full list are in the summary comment.
|
Round-4 disposition: the occupancy defect is fixed and the queued review nits are folded in. Head The defect, and why the fix is daemon-side. A I took the daemon branch you directed rather than stamping the terminal reply, for the reason in the code: The fix. Occupancy is now one verdict from the classifier that owns runner error meaning: Tests. Queued nits, one commit, no behavior change. The admission doc now names Checks at this head. Focused Swift lane (journal, transport, dispatch, execution, recording, main-thread, models, alert): |
|
Thanks for the update. At bb83f90 the validator test now asserts The journal still answers a repeat of a read command, and this was raised at 3bbe6a8 and b9aa7ae. The daemon resends read-only commands (snapshot, screenshot, reads) with the same commandId after a lost reply (index.ts:143-160). The runner marks the first send dispatched (CommandDispatch.swift:195), so Every runner command takes the admission route at Transport.swift:176-202, and this PR changes it. No simulator run at bb83f90 is posted, and the read repeat that regressed has never run on one. After the fix above, please post a simulator run at the final head. Send each command twice straight to the listener with one fixed commandId and quote both runner.log excerpts. For tap or pasteboardWrite, show one Could the daemon avoid special cases for journal replays? If the journal never answers or spends a read-class id, On the open inline threads: the one on the replayed failure read as drained (r4436220817, r4436258832) still stands and the read trait closes it. Five lower-priority threads also still apply. These threads do not apply and can be resolved: r4436212225 is fixed at this head; r4436220821 and r4436258825 because the shutdown delivery always calls The only check not passing is a review-app check waiting for action. It is not a build or test job. The other 18 checks pass, but green CI does not cover the read repeat, because no test sends a read twice through the listener. I did not run the Swift lanes, vitest or a simulator, so the 81-test and 881-test counts are unverified. I also did not measure how often the daemon's per-exchange timeout lands in the window that the Before merge, please add the read trait with the ADR change and the listener test, then post the simulator run at that head. |
A repeat of a `commandId` the runner already finished is answered from the journal's record, so that answer has to carry what the answer the caller lost carried. Two things did not: a command that closed by throwing was recorded without its wire code, so a replayed `MAIN_THREAD_TIMEOUT` reached the daemon as an unclassified refusal and cleared the main-thread occupancy report at the moment the timed-out command was what occupied the thread; and a replayed `shutdown` answered without the flag that ends the runner's own test run, so the runner kept listening until disposal escalated to a signal. Both now derive from the same owner as the live answer: `RunnerTests.wireErrorCode(for:)` names the code the transport encodes and the journal records, and one `finishesRunner` flag decides the finish for every route a `shutdown` answer travels. The cross-language tie in `runner-swift-settlement-fixtures.ts` reads the Swift declaration so the daemon's occupancy verdict and the code the runner writes for it cannot drift apart silently.
There was a problem hiding this comment.
All reported issues were addressed across 8 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
9f017aa to
c2ca3ae
Compare
There was a problem hiding this comment.
Apex Sentinel: 5 new findings on c2ca3ae. The verdict and the full list are in the summary comment.
`terminal` answers a repeat from the record the journal holds, so which answer ends the runner is that record's fact. Deriving it from the repeat's own request let a caller end the runner on some other command's answer: a spent `commandId` reused under the name `shutdown` received that end, while a recorded `shutdown` answer named as another command withheld it. The verdict now carries `answersShutdown`, read from the command the entry names, and ADR 0005 states the ownership.
The live path finishes the runner only on a `shutdown` that succeeded, so the journal's replay has to read the outcome before deciding. `terminal` now gates `answersShutdown` on the recorded answer being `ok`: replaying a refused `shutdown` kept asking the runner to finish, which would end a runner whose caller was still being told it stayed up. States the two ownership facts this round's review turned up, and neither needs a guard: only the `execute` verdict can reach `markDispatched`, so no busy refusal can follow the mark for the command it refuses; and the request's `close` is what ends the deadline bookkeeping, so the journal answer is the only writer for that connection. ADR 0005 records what `notAccepted` costs a lost charge.
There was a problem hiding this comment.
Apex Sentinel: 6 new findings on a08339f. The verdict and the full list are in the summary comment.
A `commandId` the runner refuses before dispatch is claimed again, and the transport keyed its joiners by the id, so the delivery of that refusal could answer the sends that joined the execution which followed it — one execution's answer handed to another execution's callers. The journal now mints an identity per claim and hands it with the verdict, and the transport keys waiters by it. `markDispatched` sets only its own mark, so the retention order keeps recording claim order, and both it and the state transitions go through one locked mutation. The main-thread-timeout hint keys on the error the watchdog throws rather than on the code derived from it. ADR 0005 states the per-claim identity, and the classification test pins the published shape of `COMMAND_ALREADY_FINISHED` the way its siblings are pinned.
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on 6db42ef. The verdict and the full list are in the summary comment.
|
Review follow-up at
Fail-first on the pre-fix working tree: the read-class matrix fails for all eight classes, the second HTTP snapshot returns Live at this head on Direct HTTP listener evidence from the same device and final-head unit build: Both snapshot responses are ok with nonempty nodes; the repeat write proposes a different value but the pasteboard remains The earlier Smoke failure is a 30-second host Full PR scope relative to |
thymikee
changed the title
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Apex Sentinel: 4 new findings on c252743. The verdict and the full list are in the summary comment.
|
Cubic follow-up at
Fail-first against
Live at this head on mad2026-sol, with the worktree build and private state |
|
Native duplicate fix at Reproduced the Repo Guards failure locally with The three tests now use one private This is test-only: production source is identical to |
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on f418517. The verdict and the full list are in the summary comment.
|
Apex follow-up at
Validation at this head: focused Swift 81 passed, focused TypeScript 77 passed, Live on mad2026-sol with this build and private state |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
Review follow-up at
Thread r4437579952 is fixed and resolved; all 86 threads are resolved, with no additional unresolved thread at the final head. No merge or label change. |
|
Fixed the CI failure in |
|
At 7c883d7 I found no correctness problems, and the evidence you gave is enough. All 20 checks pass, including Apple Smoke Tests, which failed at an earlier head on a host-label assumption in a test that the last commit removed. There are no conflicts. I read the code and the gate output; I did not run the Swift lanes, vitest, check:affected or a simulator, so test counts and gate receipts are yours. The direct-listener runner.log comes from the c252743 build, not from 7c883d7. I accepted it because the journal and transport production code are identical from c252743 to head. The snapshot replies and the unchanged pasteboard are your statements, not quoted output. Compared with the earlier review (#4140 (comment)), a thrown failure now keeps its wire code and hint, reads re-execute instead of replaying, waiters are keyed per claim, and the dispatch-disclosure row and docs are in. The correctness questions are settled. One design question remains open before merge: whether Not blocking, take or leave: journalRecordsDerivedCode in runner-swift-settlement-fixtures.ts (line 200) regex-tests Swift source text, so a behaviour-identical refactor turns it red and a discarded call keeps it green; keeping the RunnerWireErrorCode reads and relying on the Swift behaviour tests is enough. Swift isReadOnly and TS RUNNER_COMMAND_TRAITS.readOnly are two declarations of one classification that the admission rule needs to agree, so one parity test over every queued CommandType would catch the next drift. The new justifying comments (the closed-id proof in pruneIfNeeded, the terminal comment in Transport.swift, the executionToken doc) can shrink to the one constraint the code cannot show. For the inline threads: #4140 (comment) (source-text regex) still applies as a P4 note above, and #4140 (comment), #4140 (comment) and #4140 (comment) (shutdown finish) apply only as the question above. The other threads do not apply: they are fixed at head or benign by design (pruning keeps non-terminal entries, the serial queue bounds the backlog, the single writer after inbound.close(), the 4096 window arithmetic, and mutation failures replaying with their own code), and they are already resolved. Before merge, please settle the shutdown question: drop |
Summary
The Swift journal owns execute/join/terminal admission, preventing completed interactions from running twice. An explicit read trait lets finished reads execute again after a lost reply and excludes them from the closed-id window. Execution tokens isolate joiners across re-admission. Recorded failures preserve their code and hint, and successful shutdown replays retain finish behavior.
20 files: 9 production, 7 tests, 1 fixture, 2 docs, 1 patch changeset. The closed-id marker window remains bounded at 4096; queued and running entries remain queryable.
Validation
Head
7c883d7aa617ca757de61304fb8b6cae957717ec, basef7879cf2d266ee1dd476d0342b2819bdeceb647b.pnpm check:xctest-selectionandpnpm build:xcuitestpassed.run-p6s57A,exactCommit=true,checkoutChanged=false.9c13f73live route (final commit changes only tests) on mad2026-sol: open, snapshot, tap, close; runner exited. Same-ID listener tests prove a mutation runs once and a snapshot runs twice.