spike(daemon): give the ADR-0014 ref frame private ownership - #2296
Conversation
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed exact head |
47dbfe2 to
62f994f
Compare
|
Addressed at |
|
Sentinel review at 62f994f: Independent delta review is source-clean. The expiry operation is now static on the unexported class, closing the external derivation escape; negative type tests cover construction, spread, editing and derivation. Frame identity/idempotent expiry semantics remain intact. This is still an intentional draft/spike requiring the maintainer adoption decision. Size CI fails on incompatible base-report js.rawBytes; other checks were still running. No merge-readiness claim. |
The four `refFrame*` fields on `SessionState` were policed only by the R7 ownership table: any daemon module could write them, and only a full-graph AST scan could say whose write it was. They are now one `RefFrame` value whose brand key is private to `src/daemon/ref-frame.ts`, so a module outside that file cannot construct one and cannot edit the one a session holds; the transitions replace it whole. Every transition, rejection reason and epoch rule is unchanged. Readers moved to the accessors ref-frame.ts exports (`refFrameState`, `refFrameScope`, `refFrameEpoch`, plus a new `refFrameTree` and `refFrame`). `internal-observation.ts` drops its four-field lineage copy and its field-by-field comparison: frame identity is now one `===`. Seen red: with the empty-result early return removed from `markSessionPartialRefsIssued`, the new frame-identity assertion in session-snapshot.test.ts fails; restored, it passes. A planted foreign writer module was rejected by tsc (TS2741 missing brand, TS2540 read-only property) before deletion.
A symbol brand on a plain object type stops construction from nothing, but not
`{ ...refFrame(session), state: 'active' }`: object spread copies the symbol key,
so any daemon module could mint an incoherent frame (active state, stale tree)
out of a coherent one and it type-checked. Proven before the fix with a throwaway
module doing exactly that write: tsc reported nothing.
The frame is now a class with `#`-private fields behind getters. That makes the
type nominal, so no object literal is assignable to it — the same probe now fails
with TS2739 (`missing #fields, scope, generation, expired`). Construction stays
inside ref-frame.ts, and the four claim sites (ADR 0014, the SessionState field
doc, and the two in the R7 owner table) now say what the type does and does not
judge: it cannot see a whole frame moved unchanged, which is why the R7 row stays.
Expiry is idempotent by identity again. `expired()` returns THIS frame when the
frame is already expired, rather than an equal copy, which is what the lineage
check in internal-observation.ts compares with `===`. Seen red: with that early
return removed, the tightened ref-frame test fails with "Values have same
structure but are not reference-equal"; green with it.
Also: the ADR 0014 stale-ref help sample seeds its epoch through a real frame
activation again, instead of leaning on the pre-frame snapshotGeneration
fallback, and a find test drops a `?? []` that can no longer be reached.
Behavior is unchanged: same frame contents, same transitions, same admission.
…ame value R7's owner table listed `refFrameState`, `refFrameScope`, `refFrameTree` and `refFrameGeneration` as four fields that had to be written together by one module; the code now carries them as one nominal value, so the table carries one row. R10 follows: 19 writer-owned fields to 16, 22 owner claims to 19. The row itself stays. The type stops construction, editing and spread-derivation of a frame outside ref-frame.ts, but it cannot judge a whole frame moved unchanged — clearing the field, or assigning another session's frame — and the table can. The comments say that rather than claiming full enforcement. Seen red: a planted `session.refFrame = undefined` in snapshot-session.ts fails R7 with "owned by src/daemon/ref-frame.ts"; green once reverted.
`RefFrame` exposed a public `expired()` method, so any module holding a frame could derive a new valid one and install it through a reconstructed session record, past the R7 field scan. Expiry is now a static on the unexported class, reachable only inside ref-frame.ts; the frame's surface is four getters. A type-level regression pins that no outside module can construct, spread, edit, or derive a frame (tsc covers src tests, so a directive that stops erroring fails typecheck).
Each file grew by exactly its new ref-frame import; one blank line between mock blocks goes so the files stay at their merge-base length.
62f994f to
838af72
Compare
|
1 participant
Summary
A spike, for an R7 decision. Before: four public
SessionStatefields any daemonmodule could write, policed only by the layering gate. After: one
refFramevalue of a nominal type no other module can construct, edit, orderive. Contents, transitions and behavior are unchanged.
Memo
38 files; four are production, +42 net, nearly all of that the frame class. The
rest is test call sites moving to accessors.
internal-observation.tsdrops itsfour-field lineage copy — frame identity is one
===—.tsc rejects foreign construction (TS2739), in-place edits (TS2540),
{ ...refFrame(s), state: 'expired' }(TS2739), and derivation (expiry is a static on the unexported class). That spread is why the frame is aclass with
#-private fields: a symbol brand copies straight through it, mintingan incoherent frame from a coherent one. Two whole-frame moves survive — clearing
the field, and assigning another session's frame — so the R7 row stays, at 1
owner instead of 4 (R10: 19→16 fields, 22→19 claims).
Fits
lease+deviceClaim: each already has one owner, ~54 production reads.scriptPublicationfits once its three writers collapse. Not snapshot lineage:.snapshotreads on 437 lines in 121 files.Recommendation: adopt for
lease+deviceClaim; leave snapshot lineage on R7.Validation
Tested commit: 838af72 — rebased onto main;
pnpm check:affected --rungreen except the pre-existing session-open runnerpkillleak (#2314, reproduced on main), which this diff does not touch.check:quickandcheck:layeringclean; 2630 unit-core daemon tests pass. Seenred: without the repeat-expiry early return the frame-identity test fails, and a
planted
session.refFrame = undefinedfails R7.