refactor: publish session observations through captured references

[thymikee]

Summary

Resolve repair and idle tombstones through one request-scoped address helper. Preserve expired-repair and failed-commit errors, including their causes, for cwd and tenant sessions. Journal, observation and health writers retain the captured lifetime.

76 files; part of #3116. Base: #3135. #3127 must land before this group.

Validation

Commit 42ce91199259caba38c87364214a1ed854cb5635. The exact-head pnpm check:affected --base 9252d8251f --run gate passed. Test Files 391 passed (391); Tests 2734 passed (2734); Test Files 1 passed (1); Tests 12 passed (12).

Fresh GitHub CI and review remain separate. Required native confirmation remains pending; physical recording-health proof is blocked by Xcode signing. The user handles merges. Dependencies, regression evidence and remaining checks.

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.97 MB	4.97 MB	+1.9 kB
Package (unpacked)	4.97 MB	4.97 MB	+1.9 kB
Package (download)	1.49 MB	1.49 MB	+587 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.6 ms	27.1 ms	-0.5 ms
CLI --help	86.5 ms	80.9 ms	-5.6 ms

[cubic-dev-ai]

All reported issues were addressed across 23 files

<sub>Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.

Re-trigger cubic</sub>

[thymikee]

The reviewed commit is a3bf16c. I found one defect in the repair-tombstone change that needs a fix before merge.

The PR now writes the repair tombstone under the store address (session-store.ts:334 and :345), for example cwd:<hash>:default for an implicit workspace session. The reader at https://github.com/callstack/agent-device/blob/a3bf16c/src/daemon/request-router.ts#L619 still calls readRepairTombstone(req.session) with the raw name default, so it never finds that tombstone. Before this PR both sides used default and matched. Now a caller that runs replay --save-script without --session and loses its repair session to idle reaping or shutdown gets a bare SESSION_NOT_FOUND on the next command. It no longer gets REPAIR_SESSION_EXPIRED or REPAIR_COMMIT_FAILED, so the re-run guidance and the commit-failure cause from ADR 0012 decision 6 are lost. The rule to satisfy is that every end-of-lifetime marker, repair and idle alike, is read under the same store address the request resolves to. Could repairExpiredIfTombstoned use the same resolution readIdleExpiryTombstoneSafely already uses (scopeRequestSession plus resolveEffectiveSessionName with attachesToSession:false), shared as one helper for both readers? Please also add a router test that publishes at cwd:<hash>:default with meta.cwd set and expects REPAIR_SESSION_EXPIRED. That test should fail on this head. The current tests all use an address equal to the name, or read with the address directly, so none of them drives this path. I did not confirm that the CLI always sends meta.cwd for implicit sessions. I assumed resolveCallerScopeRoot gives a cwd scope for ordinary workspace runs, as session-routing.ts suggests.

The Smoke Tests failure looks unrelated to this diff. wait text Checkout form on the explicit session ios-e2e-smoke-dkw timed out with wait_deadline_exceeded and captureTruncated:true, and it returned a timeout rather than SESSION_NOT_FOUND. That is the wait and capture route, which this PR does not touch, and it matches the known iOS smoke capture-truncation flake. The run failed before the scenario's close step, so it gives no live evidence for the changed close route. I did not run tests or mutants. The regression check comes from reading the code paths before and after the change. There are no conflicts.

Before merge, please make the router read repair tombstones through the request's resolved store address, add the cwd-scoped router regression, and rerun Smoke Tests to green.

[thymikee]

I checked #3142 at 958983e against the repair-tombstone finding here. The new owner check fixes collisions between names that sanitize to the same directory. It does not fix the scoped-address mismatch: repairExpiredIfTombstoned in request-router.ts still reads the tombstone under the raw req.session name, while the reaper writes it under cwd:<hash>:default or <tenant>:<name>. So an implicit or tenant-scoped repair session that expires still returns SESSION_NOT_FOUND. The details and the suggested shared resolver are in the #3142 review. The other address fix there, storeSessionForTest, looks right.

[thymikee]

The repair tombstone mismatch from the earlier review (#3140 (comment)) is still open at 130fe6e, so this needs one more change. The Cubic owner-check thread is now fixed.

finalizeRepairTombstone and writeRepairTombstone (session-store.ts:326-345) write the marker under the scoped store address, for example cwd:<hash>:default or <tenant>:<name>, with that address as owner. repairExpiredIfTombstoned (request-router.ts:619) reads it with the raw name default. The new owner check in session-repair-tombstone.ts:39 also needs default, so both path and owner miss. An implicit workspace or tenant-scoped replay --save-script repair session that is idle-reaped or shut down now returns a bare SESSION_NOT_FOUND. The REPAIR_SESSION_EXPIRED and REPAIR_COMMIT_FAILED codes are lost, with the re-run guidance and the commit-failure cause. Before this PR, writer and reader both used default and matched. The rule: every end-of-lifetime marker, repair and idle alike, is read under the same store address the request resolves to. Could you move the scopeRequestSession and resolveEffectiveSessionName({attachesToSession:false}) resolution out of readIdleExpiryTombstoneSafely into one helper, and use it in both the repair reader and the idle reader? Please add a case to request-router-repair-expired.test.ts that publishes at cwd:<hash>:default with meta.cwd set, retires, sends a default request, and expects REPAIR_SESSION_EXPIRED. It should fail on this head.

Not blocking, and fine to take or leave: recordAction still takes a SessionState and resolves the event-log address through resolveStoredSessionName, which falls back to session.name (session-store.ts:209), so after an update(ref) a stale object can write its event line to the default directory, and selector-recording.ts:138 writes to the current occupant. Taking a SessionRef there and dropping that fallback would let the type checker list the sites, or the PR body could limit its claim to snapshots. Also, bindInteractionSession re-binds an optional sessionRef at six layers, which leaves eight sessionRef! assertions (interaction-session.ts:5).

On that last point, could binding.existingRef from prepareLockedRequestScope be threaded through RequestHandlerChainParams into the interaction, generic, selector and snapshot routes, with sessionRef required on InteractionRouteInput? That would delete bindInteractionSession, the per-route lookups and the assertions. First, RequestHandlerChainParams would need to carry the locked binding's ref, which prepareLockedRequestScope already computes.

On the other threads, the Cubic storeSessionForTest address thread (#3140 (comment)) is fixed at this head and can be resolved. The Cubic owner-check thread (#3140 (comment)) is fixed for colliding sanitized paths and can be resolved, because the scoped-address reader mismatch is tracked in the finding above.

CI is still pending. Nine jobs were cancelled by superseded runs, with no failure logs, and one Smoke Tests job is still running on 130fe6e. The diff touches the smoke route (snapshot, press and fill settle, close and script finalization on iOS and Android), so Smoke Tests must be green on this head before merge. The physical-iOS recording-health run (record start, interaction, record stop with the runner AVAssetWriter backend and showTouches, showing the recording survives with a runner session id) is still blocked on signing. Please run it on this head or note it as outstanding risk.

I ran no tests or mutants on this head, so whether each regression test fails without its fix comes from reading the code. I did not confirm that the CLI always sends meta.cwd for implicit sessions, and I found no production path that updates or retires a session during an in-flight interaction, so the reachability of the recordAction note is unproven. The next step before merge is to resolve the repair tombstone through the request's scoped store address and add the cwd-scoped router test.

[thymikee]

Updated review fixes are included at 42ce91199259caba38c87364214a1ed854cb5635. The exact-head affected gate passed: Test Files 391 passed (391); Tests 2734 passed (2734); Test Files 1 passed (1); Tests 12 passed (12).

Repair and idle readers share scoped address resolution. Cwd/tenant controls preserve expiration and commit-failure errors and their causes, and refuse another workspace’s marker. Three controls were red before the fix; all fourteen repair/idle controls pass. Nested interaction adapters already reuse supplied refs; the optional wider migration is deferred. The fixture directories have automatic per-run cleanup.

Current dependencies, exact-head gates and evidence still required. Fresh GitHub/native evidence and approval remain separate; pending or canceled runs are not passing evidence. The user handles all merges.

[thymikee]

The repair-marker finding from the earlier review at 130fe6e is fixed at 423be5b: the scoped-address reader now matches the writer in request-router.ts (lines 619-621), and the new regression cases cover it. I read the 130fe6e and 423be5b code to judge that each regression fails without the fix. I ran no tests or mutants, and I did not confirm that the CLI always sends meta.cwd for implicit sessions.

The code looks right to me, but the evidence is still missing. Smoke Tests need to be green on 423be5b on both the iOS simulator and the Android fixture lanes. The runs must reach snapshot, press/fill settle, and the close step with script finalization, because this PR changes those routes. A run that stops at the known wait-text capture-truncation flake before close does not count. The physical-iOS recording-health run stays noted as an outstanding risk, as the earlier review allowed.

CI is unclear. On 423be5b, Coverage and Compatibility & Provenance failed, and Smoke Tests, Lint, Repo Guards, Typecheck and Integration were cancelled or queued. I have no failure logs for this head, so I cannot attribute the failures. Coverage runs the unit suite, which exercises the daemon session-store, router and observation routes this PR changes, so please check that log first. I did not read the Compatibility & Provenance log. There are no conflicts.

Before merge, Smoke Tests (iOS and Android, through close and script finalization), Coverage and the rest of CI need to be green on 423be5b.

Not blocking, take or leave: the new test.each cases in https://github.com/callstack/agent-device/blob/423be5b/src/daemon/__tests__/request-router-repair-expired.test.ts#L176 compute the publish address with the same resolveEffectiveSessionName({attachesToSession:false}) the reader uses, while the production writer takes its address from open-time routing, so driving the publish through a real open request in the same cwd and tenant scope would test the pairing; the new cases also create mkdtemp roots without removing them, unlike the earlier tests in the file that call fs.rmSync(root).

On the open threads, the two cubic-dev-ai P2 threads no longer apply and can be resolved: #3140 (comment) (storeSessionForTest address parameter, already resolved and unchanged in this delta) and #3140 (comment) (the owner check is in session-repair-tombstone.ts, and the scoped-address reader mismatch is fixed at request-router.ts:619-621).

The head moved to 3f40d8d after this review. The new commit comes from the base branch (it joins daemon dispatches before shutdown retires session admission), so this verdict still describes 423be5b. I will check the new head separately.

[thymikee]

The scoped-address fix remains included in 42ce911992. Its exact-head affected gate passes 2,734 related tests and twelve documentation controls. The earlier Coverage failure was caused by eager-import growth from the capture binding barrel; #3135 now exports that factory through a dedicated facet. All 759 closure controls and twelve binding controls pass without relaxing budgets. Original failing Coverage job. Fresh Coverage and native jobs remain pending.

The temporary directories are already removed by scripts/vitest-tmpdir-global-setup.ts; mkdtempForTestSync documents that per-run ownership. No per-case cleanup is needed. I retained the focused writer/reader cases rather than adding a second full open fixture here. The requested native runs through snapshot, interaction settle and close/script finalization remain acceptance requirements, and the physical recording-health limitation remains recorded.

I also checked both failure logs at 423be5b: Coverage failed thirteen eager-closure controls, now fixed by the dedicated facet. Compatibility & Provenance reported SessionStore.clearRuntimeHints against the transient base 252ae96c5b, which had removed that still-required API. The corrected base restores it and its callers; current #3140 Compatibility & Provenance is green. No scanner exception was added. Historical provenance job.

Current-head Android Smoke Tests are green. The artifact step history reaches snapshot, form fill/readback and capture-close, then confirms the session is absent. It does not independently assert saved script contents; I am keeping that distinction and the pending iOS run explicit.

The current-head iOS Smoke Tests are also green. The fixture E2E artifacts contain 98 steps, including snapshots, semantic press, form fill and successful close/session removal. Both native runs reach ordinary close, which synchronously calls finalizeOrdinaryCloseScript and reports a script-finalization error before returning success (session-close-script.ts:76, session-close.ts:345). The artifacts do not independently retain/assert the saved script contents; that is a limit of the evidence, not a claim of an exported-file assertion. All current #3140 GitHub checks now pass.

[thymikee]

The PR is ready at 42ce911. The scoped-address problem from the earlier review (#3140 (comment)) is fixed, and I found no new problems in the changes since 130fe6e.

Not blocking, and you can take or leave these. A repair marker is keyed only by address, and publish at https://github.com/callstack/agent-device/blob/42ce911/src/daemon/session-store.ts#L100 clears only the idle-expiry marker, so a later session at the same address can report an earlier repair session's REPAIR_SESSION_EXPIRED or REPAIR_COMMIT_FAILED (the same publish and check order exist in the base, so this predates the PR; the rule would be that a marker explains only the lifetime that wrote it, so publish could clear both markers through the owner check). Also, the reader at https://github.com/callstack/agent-device/blob/42ce911/src/daemon/request-router.ts#L664 looks up its own address, so a follow-up request with a different --platform than the one that opened the session gets a bare SESSION_NOT_FOUND, and the idle reader has the same limit.

Both open inline threads on storeSessionForTest and the tombstone owner check are fixed at this head, so you can resolve them: #3140 (comment) and #3140 (comment).

CI is green: every job passes on the current-head runs, and the cancelled entries are superseded duplicate runs. There are no conflicts. I did not run tests or mutants on this head, so my read of the regression fix comes from the 130fe6e and 42ce911 code. I did not trace whether the CLI always sends the same meta.cwd and --platform on a follow-up to a repair session, so how often the second note occurs is unproven. The physical-iOS recording-health run is still blocked on signing, and you already record it as outstanding risk. The Smoke artifacts reach close and script finalization but do not assert saved script contents, as you note. Nothing else needs to happen before merge.

[thymikee]

I checked the two optional marker notes against their owning contract. I am retaining the current behavior here: ADR 0012, repair-session tombstone keys this recovery evidence by owner-scoped session key, retains it for a bounded window, and clears it when a fresh replay --save-script starts that transaction. It does not currently define an ordinary open as recovery of the earlier repair.

Clearing every marker from publish would also erase failed-commit evidence that findUnrecoveredRepairCommitFailure uses to prevent private replay-directory deletion. If we change the diagnostic to follow an individual lifetime, we need to separate its invalidation from durable failed-commit retention first; a blanket publish-time clear is unsafe. I have kept this as a non-blocking follow-up rather than changing the recovery contract in this PR.

A different platform scopes a different address. I have not added fallback searching across addresses to find an earlier marker: that would risk reporting another slot's recovery evidence and undo the scoped-address fix. The current-head native/CI evidence and your readiness verdict remain unchanged; physical recording-health signing remains the separately recorded limit.