chore(gates): enforce session identity through copies and destructuring

[thymikee]

Summary

Finish the session-write scanner review for #3155 and #3116. Optional chains, record clones/rest copies and destructured refs keep their ownership identity. Nested writes through a tracked SessionRef.session reach R7; unrelated request session names stay outside that recognition.

Patch-return collection skips nested functions. Fixtures must parse as valid modules, preventing parser recovery from masking invalid examples. Two files changed; the scanner remains a scoped AST gate.

Validation

Reconciled head: 14c30ab142d4e977ab6467f3fc84a070f59e0294. The review base pins #3170 at 111f50f783b703adc6731537f9b3581bae4b040e; #3170 remains a prerequisite.

• Exact-head pnpm check:affected --base 111f50f783b703adc6731537f9b3581bae4b040e --run passes all runnable checks.
• git range-diff confirms the reviewed patch is unchanged; both files are byte-identical to the prior head.
• Retained regression proof: 30 scanner controls pass; nine production violations are rejected by R7, with original source restored.
• Prior CI passed Typecheck & Package, Coverage and Integration Tests at 2ce928f5e6. Checks on the reconciled head are pending. No device run is required for this tooling-only diff.

[cubic-dev-ai]

1 issue found across 380 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/capture-kit/src/durable-capture/session-binding.fixtures.ts">

<violation number="1" location="packages/capture-kit/src/durable-capture/session-binding.fixtures.ts:22">
P3: `lookup` throws "Test session retired" whenever the address has no entry, including the first-time case where `set` was never called for it (all callers `set` before binding). A never-registered session isn't retired, so the message misleads when a test binds to a typo'd or unseeded address. Use a distinct message for the unregistered case, e.g. "Test session not found".</violation>
</file>

<sub>Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.

Re-trigger cubic</sub>

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.96 MB	4.96 MB	0 B
Package (unpacked)	4.96 MB	4.96 MB	0 B
Package (download)	1.49 MB	1.49 MB	-12 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	26.6 ms	26.0 ms	-0.6 ms
CLI --help	82.3 ms	80.9 ms	-1.4 ms

[thymikee]

Rechecked the original 26-thread review against the runtime tree inherited from #3170 at 408192c52f and the updated scanner. #3172 contains two scanner files.

The scanner fixes are at 2ce928f5e6: tracked ref.session.field writes reach R7, and fixtures fail on parser errors. Thirty focused tests pass; a nested foreign lease write planted in the production app-log owner produces the expected R7 violation. The existing copy, optional-chain and destructuring controls remain.

Four threads are resolved with evidence: log truncation preserves its inode; the legacy guard control never waits behind an external guard; release already waits for the guard; and a real exclusive write against the hardened lock directory returns handled EEXIST on this Mac.

The remaining owning-layer reviews stay open:

Finding	Owner	Required follow-up
Missing process birth times compare equal during reuse	#3133	Require a proved identity match, with the nearest missing-proof negative control
Startup cleanup omits the private-directory capability	#3131	Let the retirement helper read the capability from its settings for every caller
Owned launch retirement requires birth-time proof before joining	#3127	Separate monitored-child completion from the proof needed to signal a process
Failed adoption skips its fenced durable transition after retirement	#3136	Check resource authority separately from current session-slot authority
Guard unlink failure after acquisition	#3122	Review the retained-guard contract, typed failure and recovery control; do not remove uncertain guards unconditionally
Fill response requires a retired session after native completion	#3151	Audit the completed-result boundary alongside the targeted-touch path and prove its reachable cancellation schedule

Test-control and documentation findings are tracked too: distinguish successor envelopes, assert cleanup reports, remove stale fixture projections/no-op methods, make the zombie control match its title, and strengthen the remaining fixture controls. The guard-held release assertion and ADR release wording are now corrected in the later parent 94807d36594b816167b40b2827bdcd6301636d57: the test asserts the claim and guard still exist while release waits, and the ADR includes unreadable owner records and removal failures as unverified outcomes. Binding-retention and second-adoption claims still need a reachable production path; the record-only wrapper is currently local to one start operation.

The earlier “fake process group” diagnosis was too strong. In #3131’s exact-head Integration Tests job, screenshot cleanup signals group -4399 outside the hermetic guard’s live-child set. The fixture launches real Node subprocesses. The screenshot path aborts its concurrent rotation probe in finally; the relevant exit/abort schedule still needs diagnosis. Keep the signaling guard intact.

These follow-ups remain tracked in #3116. Review findings are assessed at their owning boundary, including regression controls, before resolution.

[thymikee]

At 2ce928f the R7 gate still misses foreign session writes in most files that hold a SessionRef, so this needs one more change before merge. hasNamedType in session-state.ts only matches a bare SessionRef annotation, and sessionRefPropertyPaths only reads inline type literals. Three common shapes never enter sessionRefBindings: ref: SessionRef | undefined (record-runtime.ts, session-app-deployment.ts, session-runtime-command.ts, snapshot-alert.ts, snapshot-settings.ts), sessionRef: SessionRef | undefined (generic-settle.ts, interaction-session.ts), and ref: SessionRef inside a named params type that is destructured or accessed (interaction-runtime.ts, interaction-touch-fill.ts). In those files if (ref) ref.session.lease = x; or params.ref.session.appLogFailure = e; makes isSessionRecord false, so R7 reports nothing. That is the gap #3155 asked to close, and the new tests only use bare : SessionRef or inline literals, so they cannot show it.

The rule to enforce is this: every <expr>.session.<declared SessionState field> write and every ...<expr>.session copy is a session write, whatever the declared type of <expr>. The simplest way is to keep the old memberName(node) === 'session' test in the member branch of isSessionRecord, together with the declared-field filter that isSessionBinding already pairs it with. If you keep typed tracking instead, hasNamedType must accept unions that contain SessionRef, and property lookup must cover named interfaces. Either way, please add ref: SessionRef | undefined and named-params controls that fail on the planted write.

Could the SessionRef type tracking (isSessionRefValue, sessionRefPropertyPaths, the objectDestructurings loop) go entirely, keeping the name rule plus declared-field filter and everything else that closes real holes (ChainExpression unwrapping, MemberExpression write targets, Object.assign/structuredClone, object-rest, collectPatchReturns, the parse-error assertion)? That would be roughly 100 lines smaller and would also cover the union-typed and named-interface refs. The only reason I can see for narrowing is a non-record session property, and I found no production instance of one.

Not blocking: the same ~120-line tracking layer exists only to avoid a false match when session is a string, which cannot be spread into a record or field-written anyway, so you can take or leave dropping it.

The open cubic-dev-ai threads do not apply to this 2-file diff, so please resolve them. They point at files outside this change (inherited from the #3170 base or not in this diff): interaction-touch-fill.ts:213, daemon-client-lifecycle.ts:232, session-repair-tombstone.ts:74, session-test-suite.test.ts:500, screen-recording-session-binding.ts:15, screen-recording-session-binding.ts:19, capture-kit adoption.ts:66, daemon-registration-owner.ts:318, selector-runtime-backend.ts:90, host-kit process-lock.ts:189, installation.md:123, store-factory.ts:22, session-binding.fixtures.ts:22, session-replay-divergence-observation.test.ts:130, app-log-session-resource.test.ts:382, adoption.test.ts:91, failed-finish.test.ts:149, daemon-exit-wait.test.ts:223, durable-capture-resource.fixtures.ts:67, daemon-client-lifecycle.ts:429.

I read the code but did not run the scanner or its tests, so the gap above comes from reading hasNamedType and sessionRefPropertyPaths at head. I also did not confirm that the new tests fail on the base beyond reading its code. Smoke Tests is still running. This diff touches only the layering scanner and its test, which the device smoke route never runs, so a failure there would likely be unrelated. There are no conflicts. Before merge, R7 must flag every <expr>.session.<declared field> write and ...<expr>.session copy, including SessionRef | undefined and named-interface refs, with controls for those shapes.

[thymikee]

Consolidated into #3155 as part of reducing #3116 to seven PRs. The composition preserves the complete pre-consolidation source tree, including tests and later review corrections. This PR is superseded; its review discussion and native evidence remain available. Outstanding findings transfer to the owning keeper in the implementation record.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-03 21:16 UTC