chore: enforce session state write ownership

[thymikee]

Summary

Enforce session-state owners and captured-reference mutation through R7. Include alias/parser controls and remove superseded whole-record construction authority. Depends on #3152. Related to #3116.

Validation

3312b9869b: pnpm check:affected --base d5472b0523 --run passed. Test Files 1 passed (1); Tests 12 passed (12). Selected tooling passed. Rebased onto the corrected optional recorder without expanding this layer. Fresh GitHub CI remains pending; ancestor native evidence and approval are separate requirements. User handles merges.

[github-actions]

Size Report

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

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.0 ms	26.1 ms	-1.0 ms
CLI --help	79.1 ms	75.7 ms	-3.4 ms

[cubic-dev-ai]

All reported issues were addressed across 3 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 2 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[thymikee]

I reviewed 54e813a and found no blocking problems in the code. CI is still pending: Smoke Tests is running, and it covers daemon and device routes this diff does not touch, so a failure there would be unrelated. There are no conflicts. I did not run the layering gate or the scanner tests, and I did not check every production update() call site for false positives. I relied on your report that the gate passes, and I did not verify the field and claim counts or the claim that 19 controls fail on the old scanner.

Not blocking, and you can take or leave these: restore the Catches/Evidence/Cost/Kill-criterion header in session-state.ts and update Cost to the new LOC, since the sibling rule files still carry it. The [whole-record-spread] and [patch-shape] checks at line 330 skip the declared-field filter that the isSessionBinding comment says must pair with the /session/i name test, so { ...providerSession } or any { ...x.session } under src/daemon now fails R7. A spread should count as a record copy only when its operand comes from a store read or has a SessionState type. The publishedDraft rule is also hard-coded to /session-open-state.ts, so it could move into the SESSION_DRAFT_CONSTRUCTORS entry and leave one table that defines it.

The four open Cubic threads still apply: the nested-return false positive in patchObjects, the missed copies via Object.assign, structuredClone and rest patterns, the optional-chain gap in unwrapExpression, and the computed destructure key. Please fix them, or narrow the whole-record-copy rule so that it matches what the scanner can check. Then let Smoke Tests finish.

Could a smaller design close this class of bypass instead? Cubic keeps finding one syntax form at a time, because the scanner tracks aliases by name. If SessionStore returned a readonly SessionState from get() and exposed owner-keyed update methods, a whole-record copy could not be written back except through the store. The scanner would then only check that each owner method comes from its declared module. That change to the SessionStore type, under #3116, would have to land before the scanner grows further. Would that work here?

[thymikee]

The code in f869e90 looks good. The session state write-ownership gate and its tests are unchanged since the earlier review at 62505d4 (#3155 (comment)), so the earlier review still holds. I did not re-run the layering gate over the production sources for this pass.

The six cubic-dev-ai threads all look fixed at this head, so the author can resolve them. The two P1 threads are the alias-spread control at https://github.com/callstack/agent-device/blob/f869e90/scripts/layering/session-state.test.ts#L83 (#3155 (comment)) and the patch-shape and reentrant-patch rules at https://github.com/callstack/agent-device/blob/f869e90/scripts/layering/check.ts#L318-L322 (#3155 (comment)). The P2 and P3 threads are covered by the nested-return, optional-chain and computed-destructure controls and by the wider copy detection in the same test file: #3155 (comment), #3155 (comment), #3155 (comment) and #3155 (comment). I checked these against the test controls at this head, not with a fresh mutation run.

Coverage failed on this head, and I have no failure log, so I cannot say yet whether the cause is in the changed layering tests. The patch only touches scripts/layering, and Coverage runs the unit suite, which can include those tests. Please check the Coverage log. If the failure is in scripts/layering, it comes from this PR. The other jobs were still queued or running when I checked. Before merge, Coverage, Lint & Format, Repo Guards, Typecheck & Package, Smoke Tests and Integration Tests all need to pass.

The head moved to 17dac94 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 f869e90. I will check the new head separately.

[thymikee]

The earlier review at 62505d4 stands, and 33831df is still clean. The six inline threads from the first pass were already settled as fixed, and no open threads remain. This commit is a rebase onto upstream, and the patch still touches only the layering gate in scripts/layering (check.ts, session-state.ts, session-resource-ownership.ts and their tests). I did not run the layering gate over the full head tree. The claim that production sources have zero R68 or R7 violations rests on a grep, so please confirm that the layering gate passes in CI on 33831df. The Smoke Tests and Bundle Size runs were cancelled or still running when I looked, which points to superseded runs, not failures. This patch does not ship in the bundle and the smoke route does not exercise it, so those checks look unrelated. Once the re-run Smoke Tests and Bundle Size jobs finish green and the layering gate passes in CI, this is ready to merge. I know of no conflicts.