Conversation
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
a06cdd8 to
27315df
Compare
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
27315df to
54e813a
Compare
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
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 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? |
54e813a to
1be1932
Compare
1be1932 to
99e8d0c
Compare
99e8d0c to
801a4ba
Compare
801a4ba to
61ef999
Compare
61ef999 to
624321d
Compare
624321d to
a3e7316
Compare
Summary
Session field ownership now covers named
SessionStore.updatepatches and whole-record copies, including the capture-admission adapter. Opaque, computed, spread, async and reentrant patches fail R7. Only the store and three declared draft constructors may copy a whole session.The same scanner measures baseline and head: 24 written fields remain, with owner/module claims reduced from 35 to 32. No ratchet or baseline was raised.
Three tooling files changed. Part of #3116, stacked on #3154.
Validation
Validated
54e813a807: 44 scanner/model tests and the complete layering gate pass. Nineteen new controls fail against the previous scanner. Six real planted regressions—including aliased/destructured app-log copies and foreign-owner writes—fail R7; original production bytes were restored.pnpm check:affected --base fix/session-admission-review --runpasses every selected runnable check. Lint, typecheck and Fallow pass. Local alias controls failed before their fix. Independent read-only review found alias gaps; their controls now pass, and re-review found no remaining findings. CI is pending; this tooling layer does not change a device route.