[thymikee]

Summary

• Keep every live request abort controller registered until explicit cleanup. Cancellation markers remain bounded.
• Add a 50,002-registration regression and correct ADR 0005: dispatch classification controls mutation replay; command accounting controls runner handoff.
• Three files, two commits (fix, then docs); no scope expansion. Related validation blocker: test(provider): fix recording timeout reproduced by the related-test gate on unchanged base #4060.

Validation

Base: a0e7c7dc239922bf93adaceff20a2f7e78579146.
Head: d88649c9c82704c4c0f1e9188ffa09b1fa5ec9f3.

• pnpm install --frozen-lockfile && pnpm build: passed.
• pnpm exec vitest run --project unit-core packages/host-kit/src/internal/request-cancel.test.ts: base plus regression 1 failed / 18 passed, early signal remained un-aborted; fixed head 19 passed.
• pnpm format and pnpm check:doc-prose: passed (17 prose tests).
• pnpm check:affected --run --head d88649c9c82704c4c0f1e9188ffa09b1fa5ec9f3: failed. Receipt run-AJzIzW/receipt.json: exactCommit=true, checkoutChanged=false; format, lint, typecheck, layering, Fallow, build passed.
• Related suite: head and isolated unchanged base both 1 failed / 1,226 passed, same record timeout at apple-platform-output-guard.test.ts:385 (test(provider): fix recording timeout reproduced by the related-test gate on unchanged base #4060).
• pnpm test:unit: 2 failed / 16,964 passed / 1 skipped. Five-second timeouts at committed-source-tree.test.ts:13 and macos-helper-package-contents.test.ts:65; focused head 2 failed / 3 passed, focused base 5 passed. Cause unproven.

Cold self-review only; no findings. No live device evidence. GitHub Integration Tests, Coverage, and platform lanes remain required. Ready for review as requested; validation remains blocked. Scratch copies removed; no timeout changes or skips added.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
agent-device-website	Ready	Preview	Nov 2, 2026 9:42am UTC

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.69 MB	5.69 MB	-117 B
Package (unpacked)	5.69 MB	5.69 MB	-117 B
Package (download)	1.73 MB	1.73 MB	-42 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	32.9 ms	30.1 ms	-2.7 ms
CLI --help	60.0 ms	61.3 ms	+1.3 ms

[cubic-dev-ai]

No issues found across 3 files

_{View guided diff | Turn on auto-fix | Re-trigger cubic}

[apex-sentinel-staging]

Apex Sentinel

Verdict: approve

1 finding(s): 1 minor. Verdict: approve. reached max rounds (1).

🟡 Minor (1)

Where	Finding
docs/adr/0005-ios-runner-interaction-lifecycle.md:139	Removed 50k/10k bound on the request-abort registry is not recorded in any doc or help the repository owns

Reply under an inline comment with @apex-sentinel-staging note ... or @apex-sentinel-staging pivot ... to re-check that finding.
Comment @apex-sentinel-staging review on this conversation to run a full review.

[apex-sentinel-staging]

Apex Sentinel: 1 new finding on d88649c. The verdict and the full list are in the summary comment.

[apex-sentinel-staging]

🟡 Minor · Removed 50k/10k bound on the request-abort registry is not recorded in any doc or help the repository owns

The registry bound in request-cancel.ts was a deliberate memory safety valve for the long-lived daemon, and this change removes it, changing a daemon-wide resource-lifetime property. Nothing in the tree records it: the only doc references to this module (docs/adr/0023-end-state-hop-trace.md:80, docs/adr/0033-collocation-decision-table.md:83) speak about throwIfRequestCanceled and file placement, and the only ADR hunk in this PR (docs/adr/0005 line 139) is about the runner's closed-id window, an unrelated journal inside the runner process. The repository's own instruction says "When behavior changes, update the owning help and user docs", and ADR 0005 shows this project documents comparable safety valves and their accepted residual bounds. The unbounded registry with no stated bound or residual-risk acceptance is the kind of decision that belongs in an ADR (the change already ships an ADR edit, so the place exists).

Suggested change

Add an ADR (or a paragraph in the daemon-lifecycle ADR) stating that requestAbortControllers is retained until explicit cleanup, why eviction of live entries was wrong (uncancelable early requests), and what now bounds it or why unbounded is accepted.

Details

Also raised here

Also raised at this location by senior-developer: Request-abort registry lost its only bound; a leaked registration now permanently poisons its request id — Removing evictOldestEntries(requestAbortControllers()) means the requestAbortControllers map is no longer bounded at all: an entry now leaves only through clearRequestCanceled/clearRequestAbortRegistration. canceledRequestIds keeps its bound (evictOldestSetEntries, line 105), so the map is now the one unbounded piece of per-runtime state in this module, and every one of the 10 production call sites relies on an explicit cleanup running (http-server lifetime.release() in the handleRpcRequest/invokeAdmittedRpcRequest finallys, transport.ts finally at line 104, withRequestDeadline's finally, bindReplayTestAttemptCancellation.release()). The consequence of any missed cleanup is worse than unbounded memory: the entry keeps requestAbortControllers().has(requestId) true forever, so registerRequestAbort throws duplicate_request_id (line 84-89) for every later request that reuses that id. …

Also raised at this location by javascript: Removing the controllers map's safety valve leaves no bound and permanently poisons a leaked request id — registerRequestAbort no longer prunes requestAbortControllers, so the map is bounded only by every caller reaching its cleanup. The sibling collection keeps its bound (markRequestCanceled still calls evictOldestSetEntries, request-cancel.ts:105), so the module now relies on the unproved invariant that all five production call sites always settle their work: http-server.ts beginRpcRequestLifetime().release() in the finally at http-server.ts:313, transport.ts:104, withRequestDeadline's finally at request-cancel.ts:247, and bindReplayTestAttemptCancellation().release() in the .finally() at session-test-runtime.ts:95-97. Concrete consequence: a handler that never settles — a wedged runner is a documented state in this codebase (RUNNER_WEDGED) — leaks its registration. …

…and 1 more (maintainability), left out to fit the comment.

persona: senior-developer, javascript, maintainability

Reply @apex-sentinel-staging note ... to add context, or @apex-sentinel-staging pivot ... if this is wrong — I will re-check the cited code.

Useful? Reply @apex-sentinel-staging like, or @apex-sentinel-staging dislike <why> if it is not.

[thymikee]

Independent read-only review of head d88649c9c82704c4c0f1e9188ffa09b1fa5ec9f3, merge-base a0e7c7dc239922bf93adaceff20a2f7e78579146.

Verdict: no actionable code findings. Both requested changes are correct. Local unit validation failed and must not be described as green.

• request-cancel.ts:78-100: registration no longer evicts controllers. An early live request retains its signal and cancellation ownership after 50,002 registrations. Cancel-before-register aborts the subsequently registered controller synchronously. Clear-after-cancel removes both records; ID reuse starts fresh; stale registration-aware cleanup cannot remove a newer controller. Standalone probes also verified that cancellation-marker eviction does not lose an already-aborted live signal.
• Residual memory bound: the controller map has no hard cap. It grows with registrations that have not been explicitly cleared, including aborted registrations whose work never settles. A cleanup leak therefore permits unbounded growth and keeps that ID reserved. The cancellation-marker set remains bounded at at most 50,001 entries (eviction runs before insertion and removes 10,000); cleared entries are removed immediately. Preserving live controllers is the requested ownership invariant. The existing bot comment correctly identifies this tradeoff, but identifies no newly introduced missed-cleanup path. I do not treat its request for another ADR as a blocker: FORMAT.md requires a hard-to-reverse architectural decision, and this local correction does not establish that threshold.
• ADR 0005:139 changes only the replay-safety claim. shouldRestartRunnerBeforeCommandSend permits a mutating command only with proven dispatched: no; the read-only exception does not permit mutation replay. RunnerCommandAccounting.hasOutstandingCharges gates runner handoff in resolveRunnerDetachDecision. The added prose uses short sentences, one instruction, an initial runner-replacement condition, and exact technical terms. No unrelated ADR rewrite is needed.

Independent execution evidence

• Frozen install and build passed in the reviewed checkout and the isolated base checkout.
• pnpm exec vitest run --project unit-core packages/host-kit/src/internal/request-cancel.test.ts: head 19 passed. On base production code with only the PR's test delta applied: 1 failed / 18 passed, specifically signal.aborted was false at line 28. The regression detects the intended defect.
• pnpm test:unit, once on head: 11 failed / 16,955 passed / 1 skipped, 9 failed files; four slow-test gate violations. On unchanged base with the same command and default worker limit: 16,965 passed / 1 skipped, exit 0. No CI wait was used.
• Exact focused timeout comparison on each unchanged revision: pnpm exec vitest run --project unit-core scripts/__tests__/committed-source-tree.test.ts test/repo-guards/macos-helper-package-contents.test.ts. Both head and base produced 1 failed / 4 passed, with the same five-second timeout at macos-helper-package-contents.test.ts:65. committed-source-tree.test.ts:13 passed in both focused runs and in the full base run.

Timeout verdict: the packaging timeout is intermittent and reproduced without this diff; it is not a new regression introduced here. The Git-history timeout was transient in the full head run. Its focused head/base passes and full base pass are consistent with a load flake, but do not prove its root cause. Other worktrees ran tests during both broad runs; head host load was approximately 49. There is no demonstrated causal path from this diff to either timeout. The nine additional head failures remain unattributed; the full base pass alone does not classify them. The full head run remains failed, separately from the previously reported provider blocker #4060.

Logs and experiment inputs are retained locally as /tmp/r4164-{head,base}-{unit,timeouts,regression}.log, /tmp/r4164-head-ordering.log, and /tmp/r4164-regression.patch. Only the disposable base checkout was modified for regression detection and then removed. The reviewed checkout remains clean at the named head.

[thymikee]

The code looks good: I found no problems in the code or ADR changes at d88649c. Not blocking, and you can take or leave it: bindAttemptCancellation registers the abort controller before prepareReplayTestTimingTrace runs, and that call can throw (for example ENOSPC or EACCES) before runReplay starts, so release() never runs. This gap existed before, but the old 50k eviction eventually dropped such an entry and now nothing does, so every registerRequestAbort should pair with a cleanup on every exit path. Binding after the trace setup, or calling cancellation.release() when the setup throws, would cover it, and a follow-up is fine.

Smoke Tests is still queued, not failed, and I have no failure output from CI. This diff only removes eviction above 50,000 uncleared registrations, which a smoke run does not reach, so a smoke failure would be unlikely to come from it. I ran no tests myself. The unit failures seen locally by you and the maintainer were not traced to this diff and may be host contention. There are no conflicts. Before merge, Smoke Tests and the other required CI lanes need to pass on d88649c.

The open inline thread at #4164 (comment) does not apply: no doc recorded the removed 50k/10k bound, and the registry is now bounded by in-flight requests, so you can resolve it.