refactor(daemon): session script publication behind one capability (#1478 P4a) - #1532
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
Re-reviewed exact head One blocking owner-action gate remains: Fallow Code Quality is red. Remove the unused exported |
|
Fixed in
Verified at the CI's own scope, not just the delta: Generated by Claude Code |
thymikee
added
the
ready-for-human
|
Re-reviewed exact head Residual: #1533 documents a real pre-existing aborted-authoring/bare-close quirk intentionally preserved by this refactor; it needs an explicit product decision after the stack. The generic device smoke jobs do not specifically prove a live |
First step of P4a. Nine co-resident optional SessionState fields encode two lifecycles plus a shared output target, with nothing in the shape saying the lifecycles are disjoint — so readers re-derived that from field combinations and writers had to remember which siblings to clear. The aggregate makes both invariants structural: a session publishes nothing, authors ordinarily, or is under repair; and force lives inside the target, so retargeting replaces the authorization along with the path. Three corrections after an adversarial review of the first draft: - The target is a default|explicit union, not a mandatory path. A bare 'open --save-script' arms with no path and lets the writer resolve a daemon-owned destination at write time, and force can be granted before any path exists. Eagerly materializing a default path would have silently changed retarget semantics, because today's check requires a previously persisted path — so 'open --save-script --force' then 'close --save-script=out.ad' is not currently a retarget and the grant survives. That behavior is preserved here and flagged in the docblock as a probable #1258 gap; tightening it is a product change and belongs in its own commit. - The repair status relation is not linear. A failed commit followed by 'replay --from' demotes complete back to armed, so demoteRepairToArmed exists and deliberately RETAINS the close receipt: the platform close already succeeded for that operation identity, and dropping it would re-dispatch a close on retry — which is also how a migrator ends up reaching for the caller-computed platformCloseSucceeded boolean the brief forbids. - The receipt doc no longer claims it is set only at close-succeeded and later, since the demotion path makes {armed, receipt set} reachable. Still to come in this PR: both projections, and the writer migration. Note the brief's seven-file writer inventory omits session-open.ts, which holds the only two writers of the authoring armed/aborted states. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8
#1478 P4a) The eight co-resident SessionState fields (scriptRecordingState, saveScriptPath, saveScriptForce, saveScriptBoundary, saveScriptComplete, saveScriptCommitted, repairPlatformCloseReceipt, repairSourcePath) are gone; SessionState.scriptPublication holds the aggregate, and every writer migrated in this commit — no shadow state. Two daemon-private projections own the writes, enforced by the R7 ownership gate: - session-replay-transaction.ts (ReplaySessionTransaction): repair arm/demote/ complete/abort, close receipts, and the uncommitted/boundary/sourcePath reads that idle-reap, tombstones, divergence-hold, and the recorder's exclusion key off. - session-script-publication-capability.ts (SessionScriptPublication): authoring arm on open, the recorded --save-script flag ingress, active publication, the published transitions, and the effective per-target force decision (#1258). The writer keeps the commit transition so idempotence stays colocated with the atomic publish. Failure/retry transitions pinned as the brief requires: platform-close failure leaves state unchanged (no receipt, retry re-dispatches); publication failure retains target+force+receipt (same-identity retry skips close dispatch); committed and aborted are explicit terminal states that drop the receipt. Design decisions resolved: - Force retention across a default->explicit retarget is preserved as-is and still flagged in resolveScriptTarget's docblock as a probable #1258 gap; tightening it stays a separate product change. - The never-armed 'close --save-script' whole-log publication folds into the authoring lifecycle (armed at the recorded close, published in the same request) instead of a fourth variant: every close path that reaches the write deletes the session, so the transient armed state cannot leak into 'session save-script' eligibility, whose not-armed-before-this-journey rejection is untouched. One real bug caught by the migrated tests and fixed in resolveScriptTarget: a bare (pathless) re-arm collapsed an already-materialized explicit target back to the daemon default, wiping the healed-sibling path on every per-step repair re-arm and defeating the persisted-force preflight bypass. A bare re-arm now keeps the previous target and only adds a live force grant. R7 rows consolidated to one scriptPublication entry (three owners) and the recordSession row narrowed; the R10 baseline drops to 22 writer-owned fields / 28 owner claims so the consolidation cannot regrow. Gates: typecheck, lint, format, layering clean; 624 files / 5220 tests pass (two known contention-flake timeouts reproduce only under full-suite load and pass in isolation). Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com>
…t suppressing - scriptPublicationTarget is module-private; both public target reads (scriptTargetPath/scriptTargetForce) go through it and nothing else did. - validatePublicationEligibility splits into a pure ineligibility classifier and an error table, so the four rejections read as one decision each. - prepareSaveScriptSession hands its two arm-time rejections (authoring re-arm, EEXIST preflight) to rejectSaveScriptArming and keeps only the demote-and-arm flow. - The repair-record-exclusion provider scenario extracts its three phases (arm-and-hold, exclusion contrast, healed-script contract) into named helpers; the test body is the journey again. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com>
a145b83 to
e6508ec
Compare
|
Re-reviewed exact head Residual product follow-up is unchanged: #1533 still needs explicit triage for the intentionally preserved aborted-authoring/bare-close behavior. |
|
2 participants
#1478 P4a: four supported publication paths consume one capability and one tagged state machine.
What moved
Commit 1 (from the prior session) added the value module: the
scriptPublicationaggregate —none | authoring{armed,aborted,published} | repair{armed,complete,close-succeeded,committed,aborted}— with the shared target and per-target--forceauthorization (#1258) inside the target, plus pure transitions.Commit 2 completes P4a atomically:
SessionStatedrops the eight co-resident fields (scriptRecordingState,saveScriptPath,saveScriptForce,saveScriptBoundary,saveScriptComplete,saveScriptCommitted,repairPlatformCloseReceipt,repairSourcePath) for onescriptPublicationfield. No shadow state, no old writer path.session-replay-transaction.ts(ReplaySessionTransaction): repair arm (per-step, boundary stamps once, explicit<out>wins over the healed sibling),--fromdemotion, completion, abort, close receipts, and the reads idle-reap/tombstones/divergence-hold/record-exclusion key off.session-script-publication-capability.ts(SessionScriptPublication): authoring arm onopen, the recorded--save-scriptflag ingress (fix(daemon): reject unsupported raw --save-script flags at request ingress #1501 pins every other command closed at the router), active publication, published transitions, and the effective write-force decision. The writer keeps the commit transition so idempotence stays colocated with the atomic publish.committed/abortedare explicit and drop the receipt. No caller-computedplatformCloseSucceededanywhere.scriptPublicationrow with exactly the three writer modules,recordSessionnarrowed; R10 baseline lowered to 22 fields / 28 claims so the consolidation cannot regrow.The two open design decisions
resolveScriptTarget's docblock as a probable Add --force/--overwrite flag for --save-script now that publish refuses any existing target #1258 gap. Tightening is a product change for its own PR.close --save-script: folded into the authoring lifecycle (armed at the recorded close action, published in the same request) rather than a fourth variant. Every close path that reaches the write deletes the session, so the transient armed state cannot reachsession save-scripteligibility; its "not armed before this journey began" rejection is unchanged (and covered by the existing eligibility tests).A bug the migration caught
The aggregate's original
resolveScriptTargetcollapsed an already-materialized explicit target back todefaulton every bare (pathless) re-arm — the per-step repair armer would wipe the healed-sibling path after step 1 and defeat the persisted-force preflight bypass. The migrated repair tests caught it (5 failures tracing to one root cause); a bare re-arm now keeps the previous target and only adds a live force grant.Verification
check:layeringclean at head (R7 writes verified AST-level against the new owner set).request-save-script-transports,client-metro) pass in isolation and match the known contention class — failure set changes between runs, timeouts only.Per the brief's STOP list: no parallel repair state, no caller-computed close boolean, and no receipt clear before terminal cleanup.