From ec3ba0c9e2e0e671aa6747e3e9c20b8bedacc8d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 13:47:02 +0000 Subject: [PATCH 1/3] refactor(daemon): add the tagged script-publication aggregate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 --- .../session-script-publication-state.ts | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 src/daemon/session-script-publication-state.ts diff --git a/src/daemon/session-script-publication-state.ts b/src/daemon/session-script-publication-state.ts new file mode 100644 index 000000000..1e851dbd4 --- /dev/null +++ b/src/daemon/session-script-publication-state.ts @@ -0,0 +1,154 @@ +/** + * The tagged script-publication aggregate (#1478 P4a). + * + * Nine co-resident optional fields on `SessionState` encode two lifecycles plus a shared output + * target: `scriptRecordingState` (the ADR 0016 ordinary authoring lifecycle), the ADR 0012 + * decision 6 repair transaction (`saveScriptBoundary`, `saveScriptComplete`, + * `saveScriptCommitted`, `repairPlatformCloseReceipt`, `repairSourcePath`), and the target + * itself (`saveScriptPath`, `saveScriptForce`). + * + * Nothing in that shape says the two lifecycles are disjoint, so every reader re-derived it from + * field combinations and every writer had to remember which siblings to clear. This aggregate + * makes the disjointness structural: a session is publishing nothing, authoring ordinarily, or + * under repair. + * + * Values and pure transitions only; no authority. The capability that performs close sequencing + * and atomic publication is separate, and no engine can reach either. + */ + +/** + * Where a script publishes. + * + * `default` is not an absence — it is the state produced by a bare `open --save-script`, which + * arms authoring with no path and lets the writer resolve a daemon-owned + * `sessions/-.ad` destination at write time. Modelling it as a variant rather + * than an undefined path is what keeps the retarget rule below expressible. + */ +export type SessionScriptTarget = + | Readonly<{ kind: 'default'; force: boolean }> + | Readonly<{ kind: 'explicit'; path: string; force: boolean }>; + +/** + * ADR 0012 decision 6 repair status. + * + * The success path is armed -> complete -> close-succeeded -> committed, and `aborted` is + * terminal. It is NOT a linear-only relation: `complete` can regress to `armed` when a + * `replay --from` continues a repair whose commit failed, and the close receipt survives that + * regression (see `demoteRepairToArmed`). + */ +export type SessionScriptRepairStatus = + | 'armed' + | 'complete' + | 'close-succeeded' + | 'committed' + | 'aborted'; + +export type SessionScriptPublicationState = + | Readonly<{ kind: 'none' }> + /** ADR 0016 ordinary open-to-destination authoring. Disjoint from repair by construction. */ + | Readonly<{ + kind: 'authoring'; + status: 'armed' | 'aborted' | 'published'; + target: SessionScriptTarget; + }> + | Readonly<{ + kind: 'repair'; + status: SessionScriptRepairStatus; + target: SessionScriptTarget; + /** + * `session.actions.length` when `replay --save-script` armed this session. The healed + * `.ad` serializes only actions from this index onward, so a reused session's earlier, + * unrelated actions never leak into the healed script (R6). + */ + boundary: number; + /** + * The original replay input path, stashed so an idle-reap tombstone can hand the agent an + * actionable `replay --save-script` re-run instead of a bare SESSION_NOT_FOUND + * (C5a). + */ + sourcePath?: string; + /** + * Identity of the platform-close operation that succeeded, so a publication retry for the + * SAME operation skips close dispatch while a different identity dispatches afresh. + * + * Retained across status regressions — a failed commit followed by a `replay --from` + * demotes completion but must NOT drop the receipt, or the retry re-dispatches a close + * that already succeeded. Only terminal lifecycle cleanup clears it. + */ + closeReceipt?: string; + }>; + +export const NO_SCRIPT_PUBLICATION: SessionScriptPublicationState = { kind: 'none' }; + +/** + * Applies an arming request, honoring per-target force authorization (#1258). + * + * `--force` is a per-target grant, not a session-wide one: re-arming a DIFFERENT explicit target + * without a live `--force` must clear it, so a later retarget cannot silently overwrite a file + * the caller never opted into. Because authorization is a field of the target, replacing the + * target replaces its authorization — there is no separate flag left behind to forget. + * + * Two retentions are deliberate, and both reproduce today's `applySaveScriptRetarget`: + * + * - re-arming the SAME explicit path keeps an existing grant; a bare re-arm is not a withdrawal; + * - moving from `default` to an explicit path keeps it. Today's retarget check requires a + * previously PERSISTED path, so `open --save-script --force` followed by + * `close --save-script=out.ad` is not treated as a retarget and the grant survives. + * + * That second case is arguably a #1258 gap — the caller authorized overwriting an unnamed + * default, not `out.ad`. It is preserved here so the migration changes no behavior; tightening + * it is a deliberate product change and belongs in its own commit, not smuggled into a refactor. + */ +export function resolveScriptTarget( + previous: SessionScriptTarget | undefined, + requested: Readonly<{ path?: string; force: boolean }>, +): SessionScriptTarget { + const retainsAuthorization = + previous?.force === true && + (previous.kind === 'default' || + previous.path === requested.path || + requested.path === undefined); + const force = requested.force || retainsAuthorization; + return requested.path === undefined + ? { kind: 'default', force } + : { kind: 'explicit', path: requested.path, force }; +} + +/** + * Continues a repair whose commit failed, after a `replay --from` re-runs the plan. + * + * Completion is demoted because the plan must reach its final executable step again, but the + * close receipt is retained: the platform close already succeeded for that operation identity, + * and dropping it would re-dispatch a close on the eventual retry. + */ +export function demoteRepairToArmed( + state: SessionScriptPublicationState, +): SessionScriptPublicationState { + if (state.kind !== 'repair') return state; + return { ...state, status: 'armed' }; +} + +/** The target a state publishes to, or `undefined` when it publishes nothing. */ +export function scriptPublicationTarget( + state: SessionScriptPublicationState, +): SessionScriptTarget | undefined { + return state.kind === 'none' ? undefined : state.target; +} + +/** + * Whether a repair transaction has reached its final executable step with no outstanding + * divergence, which is what gates commit (C2). `close-succeeded` is a sub-state of complete — + * every existing commit gate keys off completeness, and the dispatch-skip decision keys off + * receipt identity rather than status, so no caller needs to distinguish them. + */ +export function isRepairCommittable(state: SessionScriptPublicationState): boolean { + return ( + state.kind === 'repair' && (state.status === 'complete' || state.status === 'close-succeeded') + ); +} + +/** A committed publication is idempotent: a second write no-ops rather than republishing. */ +export function isScriptPublished(state: SessionScriptPublicationState): boolean { + if (state.kind === 'repair') return state.status === 'committed'; + return state.kind === 'authoring' && state.status === 'published'; +} From 2dd629d2eec7886d59775a8882e88df84755a6ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 31 Jul 2026 21:05:38 +0200 Subject: [PATCH 2/3] refactor(daemon): migrate script publication onto the tagged aggregate (#1478 P4a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- scripts/layering/daemon-modularity.ts | 4 +- scripts/layering/session-state.ts | 37 ++-- src/__tests__/test-utils/session-factories.ts | 79 ++++++-- .../request-router-repair-expired.test.ts | 9 +- .../request-router-typed-error.test.ts | 6 +- .../request-save-script-transports.test.ts | 5 +- .../__tests__/selector-recording.test.ts | 29 ++- .../__tests__/session-action-recorder.test.ts | 47 ++++- .../__tests__/session-script-writer.test.ts | 48 ++--- src/daemon/__tests__/session-store.test.ts | 30 +-- .../__tests__/session-close-script.test.ts | 7 +- .../__tests__/session-device-claims.test.ts | 6 +- .../session-replay-repair-acceptance.test.ts | 3 +- .../session-replay-repair-empty-tail.test.ts | 29 ++- .../session-replay-repair-loop.test.ts | 48 +++-- ...ion-replay-repair-record-exclusion.test.ts | 8 +- .../session-replay-repair-transaction.test.ts | 91 ++++++--- .../session-script-publication.test.ts | 39 ++-- src/daemon/handlers/session-close-script.ts | 31 ++- src/daemon/handlers/session-close.ts | 23 ++- src/daemon/handlers/session-open.ts | 15 +- src/daemon/handlers/session-replay-runtime.ts | 81 ++++---- .../handlers/session-script-publication.ts | 28 +-- src/daemon/server/daemon-idle-reap.test.ts | 37 +++- src/daemon/server/daemon-idle-reap.ts | 3 +- src/daemon/session-action-recorder.ts | 59 +----- src/daemon/session-replay-transaction.ts | 119 ++++++++++++ .../session-script-publication-capability.ts | 136 ++++++++++++++ .../session-script-publication-state.ts | 176 +++++++++++++++--- src/daemon/session-script-writer.ts | 54 ++++-- src/daemon/session-store.ts | 21 ++- src/daemon/types.ts | 63 +------ .../active-session-script-publication.test.ts | 14 +- .../replay-repair-record-exclusion.test.ts | 5 +- 34 files changed, 978 insertions(+), 412 deletions(-) create mode 100644 src/daemon/session-replay-transaction.ts create mode 100644 src/daemon/session-script-publication-capability.ts diff --git a/scripts/layering/daemon-modularity.ts b/scripts/layering/daemon-modularity.ts index e67af83f4..6f72eaa5b 100644 --- a/scripts/layering/daemon-modularity.ts +++ b/scripts/layering/daemon-modularity.ts @@ -13,8 +13,8 @@ const LARGEST_TYPE_CYCLE_ZONE_CEILINGS: Readonly> = { export const DAEMON_MODULARITY_BASELINE = { sessionState: { - writerOwnedFields: 29, - ownerFileClaims: 40, + writerOwnedFields: 22, + ownerFileClaims: 28, }, largestTypeCycle: { zoneMembers: LARGEST_TYPE_CYCLE_ZONE_CEILINGS, diff --git a/scripts/layering/session-state.ts b/scripts/layering/session-state.ts index 46a53a8eb..0ebb1f79d 100644 --- a/scripts/layering/session-state.ts +++ b/scripts/layering/session-state.ts @@ -49,35 +49,23 @@ export const SESSION_STATE_FIELD_OWNERS: Readonly published - // lifecycle; `recordSession` is the broader "record actions" flag and is deliberately set - // on its own by paths that record without arming a publication. - scriptRecordingState: [ - 'src/daemon/handlers/session-open.ts', - 'src/daemon/handlers/session-script-publication.ts', + // #1478 P4a script publication. The tagged aggregate replaced the eight co-resident + // `saveScript*`/`scriptRecordingState`/`repair*` fields; its ONLY writers are the two + // daemon-private projections (`session-replay-transaction.ts`, + // `session-script-publication-capability.ts`) and the writer's commit transition. + // `recordSession` is the broader "record actions" flag and is deliberately set on its own by + // paths that record without arming a publication. + scriptPublication: [ + 'src/daemon/session-replay-transaction.ts', + 'src/daemon/session-script-publication-capability.ts', + 'src/daemon/session-script-writer.ts', ], recordSession: [ 'src/daemon/handlers/session-close-script.ts', - 'src/daemon/handlers/session-open.ts', - 'src/daemon/handlers/session-replay-runtime.ts', - 'src/daemon/handlers/session-script-publication.ts', - 'src/daemon/session-action-recorder.ts', + 'src/daemon/session-replay-transaction.ts', + 'src/daemon/session-script-publication-capability.ts', ], - saveScriptPath: [ - 'src/daemon/handlers/session-replay-runtime.ts', - 'src/daemon/handlers/session-script-publication.ts', - 'src/daemon/session-action-recorder.ts', - ], - saveScriptForce: [ - 'src/daemon/handlers/session-replay-runtime.ts', - 'src/daemon/handlers/session-script-publication.ts', - 'src/daemon/session-action-recorder.ts', - ], - saveScriptBoundary: ['src/daemon/handlers/session-replay-runtime.ts'], - saveScriptCommitted: ['src/daemon/session-script-writer.ts'], - repairSourcePath: ['src/daemon/handlers/session-replay-runtime.ts'], pendingRecordAndHeal: ['src/daemon/handlers/session-replay-resume.ts'], - repairPlatformCloseReceipt: ['src/daemon/handlers/session-close.ts'], trace: ['src/daemon/handlers/record-trace.ts'], recording: ['src/daemon/handlers/record-trace-recording.ts'], @@ -93,7 +81,6 @@ export const SESSION_STATE_FIELD_OWNERS: Readonly): SessionState { return { name, @@ -25,22 +61,18 @@ export function makeMacOsSession(name: string, overrides?: Partial // --- Script-authoring session states --- // -// The three factories below name the states a session-script session can be in, -// instead of leaving each test to re-derive them from a pile of `saveScript*` -// booleans. They exist because the fields are NOT independent: `recordSession` -// without a boundary is an ordinary recording, a boundary without -// `saveScriptComplete` is an ARMED-but-uncommittable repair, and only the -// COMPLETE combination publishes. Spelling that out per test made the -// distinction the tests are actually about (ordinary vs repair, armed vs -// complete) the hardest thing to see in them. +// The three factories below name the states a session-script session can be in. +// Since #1478 P4a those states are structural (`scriptPublication` is a tagged +// aggregate), but the factories keep naming the states production actually +// produces: ordinary recording, an ARMED-but-uncommittable repair, and the +// COMPLETE combination that publishes. // -// A test that deliberately exercises an odd combination (a boundary with no -// recording, say) should still build it inline — these are for the states -// production actually produces. +// A test that deliberately exercises an odd combination (a repair variant with +// no recording, say) should still build it inline. /** * ADR 0016 ordinary authoring recording: `recordSession` armed, NO repair - * boundary. This is a plain `open --save-script` / `close --save-script` + * variant. This is a plain `open --save-script` / `close --save-script` * session, and the baseline for every authoring-side handler test (target-v1 * evidence, parameterized fills, landmark waits) that only needs the session to * be recording its actions. @@ -57,8 +89,7 @@ export function makeAuthoringSession( /** * ADR 0012 decision 6: a session ARMED for repair by `replay --save-script` — - * recording plus the repair-run boundary watermark, which is what the writer's - * `repairArmed` actually keys off (`saveScriptBoundary !== undefined`). + * recording plus the repair variant with its boundary watermark at 0. * * ARMED, not COMPLETE: a writer handed this session ABORTS (publishes no * prefix) rather than committing. Use `makeRepairCompleteSession` for a @@ -68,7 +99,15 @@ export function makeRepairArmedSession( name: string, overrides?: Partial, ): SessionState { - return makeAuthoringSession(name, { saveScriptBoundary: 0, ...overrides }); + return makeAuthoringSession(name, { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + ...overrides, + }); } /** @@ -80,5 +119,13 @@ export function makeRepairCompleteSession( name: string, overrides?: Partial, ): SessionState { - return makeRepairArmedSession(name, { saveScriptComplete: true, ...overrides }); + return makeAuthoringSession(name, { + scriptPublication: { + kind: 'repair', + status: 'complete', + target: { kind: 'default', force: false }, + boundary: 0, + }, + ...overrides, + }); } diff --git a/src/daemon/__tests__/request-router-repair-expired.test.ts b/src/daemon/__tests__/request-router-repair-expired.test.ts index 6d56ceb23..f3a155944 100644 --- a/src/daemon/__tests__/request-router-repair-expired.test.ts +++ b/src/daemon/__tests__/request-router-repair-expired.test.ts @@ -42,8 +42,13 @@ function tombstonedSession(name: string): SessionState { device: { platform: 'apple', id: 'sim-1', name: 'iPhone', kind: 'simulator', booted: true }, createdAt: Date.now(), actions: [], - saveScriptBoundary: 0, - repairSourcePath: '/flows/login.ad', + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + sourcePath: '/flows/login.ad', + }, }; } diff --git a/src/daemon/__tests__/request-router-typed-error.test.ts b/src/daemon/__tests__/request-router-typed-error.test.ts index c5e23caf7..ab49f5b72 100644 --- a/src/daemon/__tests__/request-router-typed-error.test.ts +++ b/src/daemon/__tests__/request-router-typed-error.test.ts @@ -189,7 +189,11 @@ test('#1391: an ordinary close-time script-save failure surfaces details.reason/ `agent-device-router-typed-error-${Date.now()}-${Math.random().toString(36).slice(2)}.ad`, ); fs.writeFileSync(targetPath, 'pre-existing\n'); - session.saveScriptPath = targetPath; + session.scriptPublication = { + kind: 'authoring', + status: 'armed', + target: { kind: 'explicit', path: targetPath, force: false }, + }; sessionStore.set('typed-error', session); try { diff --git a/src/daemon/__tests__/request-save-script-transports.test.ts b/src/daemon/__tests__/request-save-script-transports.test.ts index 2cc7f129a..428e82831 100644 --- a/src/daemon/__tests__/request-save-script-transports.test.ts +++ b/src/daemon/__tests__/request-save-script-transports.test.ts @@ -11,6 +11,7 @@ * `.ad` artifact behind. */ import fs from 'node:fs'; +import { NO_SCRIPT_PUBLICATION, scriptTargetPath } from '../session-script-publication-state.ts'; import net from 'node:net'; import os from 'node:os'; import path from 'node:path'; @@ -192,7 +193,7 @@ for (const [transport, send] of TRANSPORTS) { expect(session.actions).toEqual([]); // No arming: neither the recording marker nor the publication target moved. expect(session.recordSession).toBe(undefined); - expect(session.saveScriptPath).toBe(undefined); + expect(session.scriptPublication).toBe(undefined); // No artifact: the write a later close/teardown would attempt publishes nothing. expect(sessionStore.writeSessionLog(session)).toEqual({ written: false }); expect(listAdArtifacts(root)).toEqual([]); @@ -283,7 +284,7 @@ test('an owner-armed session still records its target and publishes its script', result: { session: SESSION }, }); expect(session.recordSession).toBe(true); - expect(session.saveScriptPath).toBe(target); + expect(scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBe(target); const result = sessionStore.writeSessionLog(session); expect(result).toEqual({ written: true, path: target, actionCount: 1 }); diff --git a/src/daemon/__tests__/selector-recording.test.ts b/src/daemon/__tests__/selector-recording.test.ts index 6d5cd9950..33da69a35 100644 --- a/src/daemon/__tests__/selector-recording.test.ts +++ b/src/daemon/__tests__/selector-recording.test.ts @@ -32,7 +32,14 @@ function planStepReq(command: string, flags: DaemonRequest['flags'] = {}): Daemo test('a repair-armed session excludes get/is/find by default but keeps recording wait', () => { const store = makeStore(); - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); store.set('default', session); recordIfSession(store, 'default', req('get'), {}); @@ -50,7 +57,14 @@ test('a repair-armed session excludes get/is/find by default but keeps recording // with --record to keep them. test('a repair-armed session still records get/is/find dispatched as replay plan steps (authored provenance)', () => { const store = makeStore(); - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); store.set('default', session); recordIfSession(store, 'default', planStepReq('get'), {}); @@ -62,7 +76,14 @@ test('a repair-armed session still records get/is/find dispatched as replay plan test('--record forces get/is/find through even while repair-armed', () => { const store = makeStore(); - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); store.set('default', session); recordIfSession(store, 'default', req('get', { record: true }), {}); @@ -75,7 +96,7 @@ test('--record forces get/is/find through even while repair-armed', () => { test('outside a repair-armed session, get/is/find/wait all record normally', () => { const store = makeStore(); const session = makeIosSession('default'); - expect(session.saveScriptBoundary).toBeUndefined(); + expect(session.scriptPublication).toBeUndefined(); store.set('default', session); recordIfSession(store, 'default', req('get'), {}); diff --git a/src/daemon/__tests__/session-action-recorder.test.ts b/src/daemon/__tests__/session-action-recorder.test.ts index d463b7097..87ab249f8 100644 --- a/src/daemon/__tests__/session-action-recorder.test.ts +++ b/src/daemon/__tests__/session-action-recorder.test.ts @@ -10,7 +10,14 @@ import { recordActionEntry } from '../session-action-recorder.ts'; import { makeIosSession } from '../../__tests__/test-utils/session-factories.ts'; test('an observation-only action is excluded while repair-armed and no --record is given', () => { - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); const action = recordActionEntry(session, { command: 'get', positionals: ['attrs', 'id="save"'], @@ -23,7 +30,14 @@ test('an observation-only action is excluded while repair-armed and no --record }); test('--record forces an observation-only action through while repair-armed', () => { - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); const action = recordActionEntry(session, { command: 'get', positionals: ['attrs', 'id="save"'], @@ -37,7 +51,7 @@ test('--record forces an observation-only action through while repair-armed', () test('an observation-only action records normally outside a repair-armed session (ordinary authoring recording is unchanged)', () => { const session = makeIosSession('default'); - expect(session.saveScriptBoundary).toBeUndefined(); + expect(session.scriptPublication).toBeUndefined(); const action = recordActionEntry(session, { command: 'is', positionals: ['visible', 'id="save"'], @@ -50,7 +64,14 @@ test('an observation-only action records normally outside a repair-armed session }); test('a mutating action is never excluded, repair-armed or not', () => { - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); const action = recordActionEntry(session, { command: 'press', positionals: ['@e5'], @@ -62,7 +83,14 @@ test('a mutating action is never excluded, repair-armed or not', () => { }); test('a command explicitly marked NOT observation-only (e.g. the top-level `wait`) always records, even while repair-armed', () => { - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); const action = recordActionEntry(session, { command: 'wait', positionals: ['500'], @@ -75,7 +103,14 @@ test('a command explicitly marked NOT observation-only (e.g. the top-level `wait }); test('--no-record still takes precedence over an observation-only action, repair-armed or not', () => { - const session = makeIosSession('default', { saveScriptBoundary: 0 }); + const session = makeIosSession('default', { + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }); const action = recordActionEntry(session, { command: 'get', positionals: ['attrs', 'id="save"'], diff --git a/src/daemon/__tests__/session-script-writer.test.ts b/src/daemon/__tests__/session-script-writer.test.ts index 9ccebf6d8..896017504 100644 --- a/src/daemon/__tests__/session-script-writer.test.ts +++ b/src/daemon/__tests__/session-script-writer.test.ts @@ -8,7 +8,11 @@ import { makeAuthoringSession, makeRepairArmedSession, makeRepairCompleteSession, + repairPublication, + authoringPublication, } from '../../__tests__/test-utils/session-factories.ts'; +import { markRepairTransactionComplete } from '../session-replay-transaction.ts'; +import { NO_SCRIPT_PUBLICATION, scriptTargetPath } from '../session-script-publication-state.ts'; import { parseReplayScriptDetailed } from '../../replay/script.ts'; import type { SessionAction } from '../types.ts'; @@ -35,7 +39,7 @@ test('write() slices session.actions from saveScriptBoundary onward, excluding p // (`close --save-script`) — COMPLETE here to isolate THIS test's own concern // (boundary slicing), covered separately below. const session = makeRepairCompleteSession('default', { - saveScriptBoundary: 2, + scriptPublication: repairPublication('complete', { boundary: 2 }), actions: [ action({ command: 'open', positionals: ['Demo'] }), action({ command: 'click', positionals: ['label="Old"'] }), @@ -69,7 +73,7 @@ test('a boundary-sliced script still strips diagnostic snapshot actions', () => const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-script-writer-snapshot-strip-')); const writer = new SessionScriptWriter(path.join(root, 'sessions')); const session = makeRepairCompleteSession('default', { - saveScriptBoundary: 1, + scriptPublication: repairPublication('complete', { boundary: 1 }), actions: [ action({ command: 'open', positionals: ['Demo'] }), action({ command: 'snapshot', positionals: [] }), @@ -189,7 +193,7 @@ test('write() publishes cleanly when the target does not exist yet', () => { const healedPath = path.join(root, 'flows', 'login.healed.ad'); const session = makeRepairCompleteSession('default', { - saveScriptPath: healedPath, + scriptPublication: repairPublication('complete', { path: healedPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -219,7 +223,7 @@ test('write() refuses to clobber an existing COMPLETE DEFAULT .healed.ad', () => const before = fs.readFileSync(healedPath, 'utf8'); const session = makeRepairCompleteSession('default', { - saveScriptPath: healedPath, + scriptPublication: repairPublication('complete', { path: healedPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -248,7 +252,7 @@ test('write() now refuses to clobber a stale PARTIAL (non-sentinel) .healed.ad a const before = fs.readFileSync(healedPath, 'utf8'); const session = makeRepairCompleteSession('default', { - saveScriptPath: healedPath, + scriptPublication: repairPublication('complete', { path: healedPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -272,7 +276,7 @@ test('write(session, { force: true }) overwrites an existing COMPLETE target ato ); const session = makeRepairCompleteSession('default', { - saveScriptPath: healedPath, + scriptPublication: repairPublication('complete', { path: healedPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -295,7 +299,7 @@ test('write(session, { force: true }) overwrites an existing target for ORDINARY // Ordinary open/close --save-script recording, not a repair. const session = makeAuthoringSession('default', { - saveScriptPath: outPath, + scriptPublication: authoringPublication('armed', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -314,7 +318,7 @@ test('write(session) without { force: true } still refuses, even when a prior wr const before = fs.readFileSync(outPath, 'utf8'); const session = makeAuthoringSession('default', { - saveScriptPath: outPath, + scriptPublication: authoringPublication('armed', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -335,11 +339,11 @@ test('two writers racing on the SAME ABSENT target: exactly one linkSync wins, t fs.mkdirSync(path.dirname(healedPath), { recursive: true }); const sessionA = makeRepairCompleteSession('writer-a', { - saveScriptPath: healedPath, + scriptPublication: repairPublication('complete', { path: healedPath }), actions: [action({ command: 'click', positionals: ['id="from-a"'] })], }); const sessionB = makeRepairCompleteSession('writer-b', { - saveScriptPath: healedPath, + scriptPublication: repairPublication('complete', { path: healedPath }), actions: [action({ command: 'click', positionals: ['id="from-b"'] })], }); @@ -402,7 +406,7 @@ test('write() refuses to clobber an existing COMPLETE artifact at an EXPLICIT -- // An explicit, caller-DIRECTED target — the protection must apply here too, // not just the default healed sibling. const session = makeRepairCompleteSession('default', { - saveScriptPath: explicitOut, + scriptPublication: repairPublication('complete', { path: explicitOut }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -427,7 +431,7 @@ test('write() now refuses an explicit --save-script= pointing at an existi // The caller directed this path explicitly rather than defaulting to the // healed sibling. const session = makeRepairCompleteSession('default', { - saveScriptPath: outPath, + scriptPublication: repairPublication('complete', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -459,7 +463,7 @@ test('an ordinary (non-repair) recording now refuses an existing target too (beh // An ordinary open/close --save-script recording, never armed via `replay // --save-script` — this is NOT a repair. const session = makeAuthoringSession('default', { - saveScriptPath: outPath, + scriptPublication: authoringPublication('armed', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -485,7 +489,7 @@ test('an ordinary (non-repair) recording still publishes cleanly when its target // Ordinary recording, not a repair. const session = makeAuthoringSession('default', { - saveScriptPath: outPath, + scriptPublication: authoringPublication('armed', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -506,7 +510,7 @@ test('close --save-script= re-points a defaulted-healed repair, a // The repair defaulted its target to the `.healed.ad` sibling. const session = makeRepairArmedSession('default', { - saveScriptPath: defaultedHealed, + scriptPublication: repairPublication('armed', { path: defaultedHealed }), actions: [action({ command: 'click', positionals: ['id="new"'] })], }); @@ -519,11 +523,11 @@ test('close --save-script= re-points a defaulted-healed repair, a positionals: [], flags: { saveScript: explicitOut }, }); - expect(session.saveScriptPath).toBe(explicitOut); + expect(scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBe(explicitOut); // `recordActionEntry` is the low-level action recorder `close`'s handler // calls on its way to setting the finalize signal (Fix 2) — set here to // isolate this test's own concern (the retarget). - session.saveScriptComplete = true; + markRepairTransactionComplete(session); const result = writer.write(session); expect(result.written).toBe(true); @@ -551,7 +555,7 @@ test('C2 abort-before-complete: a repair-armed but NOT-complete write discards expect(result).toEqual({ written: false }); expect(fs.existsSync(path.join(root, 'sessions'))).toBe(false); // Not committed — teardown will tombstone it (C5a). - expect(session.saveScriptCommitted).toBeFalsy(); + expect(session.scriptPublication).toMatchObject({ kind: 'repair', status: 'armed' }); }); test('C2 commit-when-complete: a repair-armed COMPLETE write publishes and marks the session COMMITTED', () => { @@ -560,14 +564,14 @@ test('C2 commit-when-complete: a repair-armed COMPLETE write publishes and marks const outPath = path.join(root, 'flows', 'flow.healed.ad'); fs.mkdirSync(path.dirname(outPath), { recursive: true }); const session = makeRepairCompleteSession('default', { - saveScriptPath: outPath, + scriptPublication: repairPublication('complete', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="save"'] })], }); const result = writer.write(session); expect(result.written).toBe(true); expect(fs.readFileSync(outPath, 'utf8')).toContain(HEAL_COMPLETE_SENTINEL); - expect(session.saveScriptCommitted).toBe(true); + expect(session.scriptPublication).toMatchObject({ kind: 'repair', status: 'committed' }); }); test('C2 idempotent post-commit: a second write on a COMMITTED session no-ops (no re-publish, no error)', () => { @@ -576,7 +580,7 @@ test('C2 idempotent post-commit: a second write on a COMMITTED session no-ops (n const outPath = path.join(root, 'flows', 'flow.healed.ad'); fs.mkdirSync(path.dirname(outPath), { recursive: true }); const session = makeRepairCompleteSession('default', { - saveScriptPath: outPath, + scriptPublication: repairPublication('complete', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="save"'] })], }); @@ -626,7 +630,7 @@ test('write() publishes atomically: no stray temp file survives a successful rep const outPath = path.join(root, 'flows', 'atomic.healed.ad'); fs.mkdirSync(path.dirname(outPath), { recursive: true }); const session = makeRepairCompleteSession('default', { - saveScriptPath: outPath, + scriptPublication: repairPublication('complete', { path: outPath }), actions: [action({ command: 'click', positionals: ['id="save"'] })], }); diff --git a/src/daemon/__tests__/session-store.test.ts b/src/daemon/__tests__/session-store.test.ts index 7d24528ca..f390e6fb9 100644 --- a/src/daemon/__tests__/session-store.test.ts +++ b/src/daemon/__tests__/session-store.test.ts @@ -9,6 +9,7 @@ import { buildRequestFinishedEvent } from '../session-event-log.ts'; import type { TargetAnnotationV1 } from '../../replay/target-identity.ts'; import { HEAL_COMPLETE_SENTINEL } from '../session-script-writer.ts'; import { parseReplayScriptDetailed } from '../../replay/script.ts'; +import { repairPublication } from '../../__tests__/test-utils/session-factories.ts'; type RecordActionEntry = Parameters[1]; @@ -723,8 +724,10 @@ test('writeRepairTombstone/readRepairTombstone round-trips owner + source path', const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-tombstone-')); const store = new SessionStore(path.join(root, 'sessions')); const session = makeSession('default'); - session.saveScriptBoundary = 0; - session.repairSourcePath = '/flows/login.ad'; + session.scriptPublication = repairPublication('armed', { + boundary: 0, + sourcePath: '/flows/login.ad', + }); store.writeRepairTombstone(session); const tombstone = store.readRepairTombstone('default'); @@ -774,10 +777,11 @@ test('BLOCKER 2: finalizeRepairTeardown of a COMPLETE transaction whose commit F const session = makeSession('default'); session.recordSession = true; - session.saveScriptBoundary = 0; - session.saveScriptComplete = true; - session.saveScriptPath = healedPath; - session.repairSourcePath = '/flows/login.ad'; + session.scriptPublication = repairPublication('complete', { + boundary: 0, + path: healedPath, + sourcePath: '/flows/login.ad', + }); session.actions = [{ ts: 1, command: 'open', positionals: ['Demo'], flags: {} }]; // Idle-reap/shutdown teardown (never routes through close's handler). @@ -788,7 +792,10 @@ test('BLOCKER 2: finalizeRepairTeardown of a COMPLETE transaction whose commit F assert.equal(fs.readFileSync(healedPath, 'utf8'), before); // Never committed (the write failed), so the ordinary success bookkeeping // never ran. - assert.notEqual(session.saveScriptCommitted, true); + assert.notEqual( + session.scriptPublication?.kind === 'repair' ? session.scriptPublication.status : undefined, + 'committed', + ); const tombstone = store.readRepairTombstone('default'); assert.ok(tombstone, 'expected a tombstone to preserve the failed-commit outcome'); @@ -811,9 +818,7 @@ test('BLOCKER 3: finalizeRepairTeardown auto-commit records a terminal close, pr const session = makeSession('default'); session.recordSession = true; - session.saveScriptBoundary = 0; - session.saveScriptComplete = true; - session.saveScriptPath = healedPath; + session.scriptPublication = repairPublication('complete', { boundary: 0, path: healedPath }); session.actions = [ { ts: 1, command: 'open', positionals: ['Demo'], flags: {} }, { ts: 2, command: 'click', positionals: ['id="save-v2"'], flags: {} }, @@ -824,7 +829,10 @@ test('BLOCKER 3: finalizeRepairTeardown auto-commit records a terminal close, pr // must synthesize it itself before auto-committing. store.finalizeRepairTeardown(session); - assert.equal(session.saveScriptCommitted, true); + assert.equal( + session.scriptPublication?.kind === 'repair' ? session.scriptPublication.status : undefined, + 'committed', + ); assert.equal(store.readRepairTombstone('default'), undefined); const script = fs.readFileSync(healedPath, 'utf8'); assert.ok(script.includes(HEAL_COMPLETE_SENTINEL)); diff --git a/src/daemon/handlers/__tests__/session-close-script.test.ts b/src/daemon/handlers/__tests__/session-close-script.test.ts index 546042c23..45ccd095c 100644 --- a/src/daemon/handlers/__tests__/session-close-script.test.ts +++ b/src/daemon/handlers/__tests__/session-close-script.test.ts @@ -54,7 +54,12 @@ test('failed repair publication removes only its synthetic close before retry', test('repair close failure keeps normalized metadata and is explicitly retriable', () => { const { session } = setup('repair-error'); - session.saveScriptPath = '/tmp/repaired.ad'; + session.scriptPublication = { + kind: 'repair', + status: 'armed', + target: { kind: 'explicit', path: '/tmp/repaired.ad', force: false }, + boundary: 0, + }; const failure = new AppError('COMMAND_FAILED', 'publish failed', { reason: 'target-exists', hint: 'Choose another path.', diff --git a/src/daemon/handlers/__tests__/session-device-claims.test.ts b/src/daemon/handlers/__tests__/session-device-claims.test.ts index ad11fdfa6..f027b7743 100644 --- a/src/daemon/handlers/__tests__/session-device-claims.test.ts +++ b/src/daemon/handlers/__tests__/session-device-claims.test.ts @@ -270,7 +270,11 @@ test('#1391: a close-time script save failure still clears the advisory claim an const session = makeAuthoringSession('close-save-script-failure', { device: android, deviceClaim: acquired.ownership, - saveScriptPath: targetPath, + scriptPublication: { + kind: 'authoring', + status: 'armed', + target: { kind: 'explicit', path: targetPath, force: false }, + }, }); store.set('close-save-script-failure', session); mockDispatch.mockResolvedValue({}); diff --git a/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts index 24de8543f..d5eacefe9 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-acceptance.test.ts @@ -6,6 +6,7 @@ * the original recorded, so the fresh replay needs no hand-fixing. */ import { test, expect, vi, beforeEach } from 'vitest'; +import { markRepairTransactionComplete } from '../../session-replay-transaction.ts'; vi.mock('../../../core/dispatch.ts', async (importOriginal) => { const actual = await importOriginal(); @@ -127,7 +128,7 @@ test('a healed script survives repair + fresh-session replay: self-contained ope // path reuses exactly this writer; ADR 0012 decision 6 Fix 2 gates a // repair-armed write on the same explicit finalize signal `close // --save-script` sets). --- - session.saveScriptComplete = true; + markRepairTransactionComplete(session); sessionStore.writeSessionLog(session); const healedPath = path.join(root, 'flow.healed.ad'); expect(fs.existsSync(healedPath)).toBe(true); diff --git a/src/daemon/handlers/__tests__/session-replay-repair-empty-tail.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-empty-tail.test.ts index 8f4b045ed..dae2dc4fc 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-empty-tail.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-empty-tail.test.ts @@ -36,6 +36,8 @@ import { runReplayScriptFile } from '../session-replay-runtime.ts'; import { SessionStore } from '../../session-store.ts'; import { dispatchCommand } from '../../../core/dispatch.ts'; import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; +import { repairSessionBoundary } from '../../session-replay-transaction.ts'; +import type { SessionState } from '../../types.ts'; import { baseReplayRequest as baseReq, writeReplayFile, @@ -47,6 +49,13 @@ import { toSnapshotNodes, } from './session-replay-target-classification-fixtures.ts'; +/** Repair-transaction status, or `undefined` outside a repair publication. */ +function sessionRepairStatus(session: SessionState | undefined) { + return session?.scriptPublication?.kind === 'repair' + ? session.scriptPublication.status + : undefined; +} + const mockDispatchCommand = vi.mocked(dispatchCommand); beforeEach(() => { @@ -104,7 +113,7 @@ test('a record-and-heal divergence on the LAST step resumes with an empty tail a const session = sessionStore.get(sessionName)!; expect(session.actions.map((a) => a.command)).toEqual(['open']); - expect(session.saveScriptComplete).toBeFalsy(); + expect(sessionRepairStatus(session)).toBe('armed'); // --- A blind resume at the reported target BEFORE performing the // corrective press is rejected: no new action was recorded since the @@ -163,7 +172,7 @@ test('a record-and-heal divergence on the LAST step resumes with an empty tail a expect((leg2.data as { replayed: number }).replayed).toBe(0); } expect(session.actions.map((a) => a.command)).toEqual(['open', 'press']); - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); // --- Commit: the transaction is COMPLETE, so the healed script actually // publishes — the corrective press survives, "click" (never recorded) does @@ -231,7 +240,7 @@ test('a manual divergence (unannotated action-failure) on the LAST step resumes const session = sessionStore.get(sessionName)!; expect(session.actions.map((a) => a.command)).toEqual(['open']); - expect(session.saveScriptComplete).toBeFalsy(); + expect(sessionRepairStatus(session)).toBe('armed'); // The watermark IS now stamped for `manual` (#1262) — targeting N + 1 (3), // a DIFFERENT ordinal than the unshifted `resume.from` (2) above. expect(session.pendingRecordAndHeal).toEqual({ expectedFrom: 3, actionsCountAtDivergence: 1 }); @@ -285,7 +294,7 @@ test('a manual divergence (unannotated action-failure) on the LAST step resumes expect((leg2.data as { replayed: number }).replayed).toBe(0); } expect(session.actions.map((a) => a.command)).toEqual(['open', 'press']); - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); // --- Commit: the transaction is COMPLETE, so the healed script actually // publishes — the corrective press survives, "click" (never recorded, @@ -362,7 +371,7 @@ test('a caution (identity-mismatch) divergence on the LAST step resumes with an const session = sessionStore.get(sessionName)!; expect(session.actions.map((a) => a.command)).toEqual(['open']); - expect(session.saveScriptComplete).toBeFalsy(); + expect(sessionRepairStatus(session)).toBe('armed'); expect(session.pendingRecordAndHeal).toEqual({ expectedFrom: 3, actionsCountAtDivergence: 1 }); // --- A blind resume at the empty-tail target BEFORE performing the @@ -412,7 +421,7 @@ test('a caution (identity-mismatch) divergence on the LAST step resumes with an expect((leg2.data as { replayed: number }).replayed).toBe(0); } expect(session.actions.map((a) => a.command)).toEqual(['open', 'press']); - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); // --- Commit: COMPLETE, so the healed script publishes the corrective // press; the pre-action "click" (never dispatched) does not appear. --- @@ -519,7 +528,7 @@ test('--from N stays legal for a caution divergence even after the N + 1 empty-t expect((resumeAtN.data as { replayed: number }).replayed).toBe(1); } expect(session.actions.map((a) => a.command)).toEqual(['open', 'click']); - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); }); test('an unauthorized --from one past the plan end is rejected on an ARMED session whose last-step divergence hint is state-repair, not record-and-heal', async () => { @@ -581,7 +590,7 @@ test('an unauthorized --from one past the plan end is rejected on an ARMED sessi expect(divergence.resume.from).toBe(2); const session = sessionStore.get(sessionName)!; expect(session.pendingRecordAndHeal).toBeUndefined(); - expect(session.saveScriptBoundary).toBeDefined(); // genuinely armed + expect(repairSessionBoundary(session)).toBeDefined(); // genuinely armed // --- Exploit attempt: `--from 3` (one past the plan's end) — exactly the // ordinal a record-and-heal empty-tail resume would use — on an armed @@ -604,7 +613,7 @@ test('an unauthorized --from one past the plan end is rejected on an ARMED sessi expect(exploitAttempt.error.code).toBe('INVALID_ARGS'); expect(exploitAttempt.error.message).toMatch(/out of range/); } - expect(session.saveScriptComplete).toBeFalsy(); + expect(sessionRepairStatus(session)).toBe('armed'); }); test('a stale --plan-digest on an empty-tail resume is rejected WITHOUT consuming the watermark, so a subsequent correct retry still succeeds', async () => { @@ -699,6 +708,6 @@ test('a stale --plan-digest on an empty-tail resume is rejected WITHOUT consumin invoke, }); expect(retry.ok).toBe(true); - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); expect(session.pendingRecordAndHeal).toBeUndefined(); }); diff --git a/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts index a2dafd56b..0df51ba98 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-loop.test.ts @@ -22,14 +22,26 @@ import path from 'node:path'; import { runReplayScriptFile } from '../session-replay-runtime.ts'; import { SessionStore } from '../../session-store.ts'; import { dispatchCommand } from '../../../core/dispatch.ts'; -import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; -import type { DaemonRequest } from '../../types.ts'; +import { + makeIosSession, + repairPublication, +} from '../../../__tests__/test-utils/session-factories.ts'; +import { repairSessionBoundary } from '../../session-replay-transaction.ts'; +import { NO_SCRIPT_PUBLICATION, scriptTargetPath } from '../../session-script-publication-state.ts'; +import type { DaemonRequest, SessionState } from '../../types.ts'; import { baseReplayRequest as baseReq, writeReplayFile, } from './session-replay-runtime.fixtures.ts'; import { freshEvidence, makeRecordingReplayInvoke } from './session-replay-repair.fixtures.ts'; +/** Repair-transaction status, or `undefined` outside a repair publication. */ +function sessionRepairStatus(session: SessionState | undefined) { + return session?.scriptPublication?.kind === 'repair' + ? session.scriptPublication.status + : undefined; +} + const mockDispatchCommand = vi.mocked(dispatchCommand); beforeEach(() => { @@ -98,7 +110,7 @@ test('R1/R2/R6: prefix steps get fresh evidence, corrective + resumed steps land if (leg1.ok) return; const session = sessionStore.get(sessionName)!; expect(session.recordSession).toBe(true); - expect(session.saveScriptBoundary).toBe(0); + expect(repairSessionBoundary(session)).toBe(0); expect(session.actions.map((a) => a.command)).toEqual(['open', 'click']); // R1: the recorded prefix step carries FRESH evidence, never the .ad's own // "Recorded Original" annotation copied through. @@ -133,7 +145,7 @@ test('R1/R2/R6: prefix steps get fresh evidence, corrective + resumed steps land }); expect(leg2.ok).toBe(true); - expect(session.saveScriptBoundary).toBe(0); // sticky — NOT reset to 3 + expect(repairSessionBoundary(session)).toBe(0); // sticky — NOT reset to 3 expect(session.actions.map((a) => a.command)).toEqual(['open', 'click', 'press', 'click']); expect(session.actions.map((a) => a.positionals[0])).toEqual([ 'Demo', @@ -141,7 +153,7 @@ test('R1/R2/R6: prefix steps get fresh evidence, corrective + resumed steps land '@e9', 'id="confirm"', ]); - expect(session.actions.slice(session.saveScriptBoundary ?? 0)).toHaveLength(4); + expect(session.actions.slice(repairSessionBoundary(session) ?? 0)).toHaveLength(4); }); test('R2: a fresh FULL replay --save-script on an already-armed session is rejected with INVALID_ARGS', async () => { @@ -158,7 +170,7 @@ test('R2: a fresh FULL replay --save-script on an already-armed session is rejec invoke, }); expect(first.ok).toBe(true); - expect(sessionStore.get(sessionName)!.saveScriptBoundary).toBe(0); + expect(repairSessionBoundary(sessionStore.get(sessionName))).toBe(0); // A SECOND full (non---from) replay --save-script would re-append the prefix. const spy: DaemonRequest[] = []; @@ -194,7 +206,7 @@ test('R2 bypass guard: a PLAIN full replay (no --save-script) on an armed sessio }); expect(first.ok).toBe(true); const armed = sessionStore.get(sessionName)!; - expect(armed.saveScriptBoundary).toBe(0); + expect(repairSessionBoundary(armed)).toBe(0); expect(armed.recordSession).toBe(true); const armedActionCount = armed.actions.length; @@ -257,7 +269,7 @@ test('R6 no amputation: a pre-populated session whose step-1 open REPLACES the s const session = sessionStore.get(sessionName)!; // The healed slice is EXACTLY this run (open + both clicks) — the open is not // amputated, and the prior waits (on the discarded session) never leak in. - const healed = session.actions.slice(session.saveScriptBoundary ?? 0); + const healed = session.actions.slice(repairSessionBoundary(session) ?? 0); expect(healed.map((a) => a.command)).toEqual(['open', 'click', 'click']); expect(healed[0]?.positionals[0]).toBe('Demo'); expect(session.actions.some((a) => a.command === 'wait')).toBe(false); @@ -292,9 +304,9 @@ test('R6 preserved session: prior actions are excluded from the healed slice', a expect(response.ok).toBe(true); const session = sessionStore.get(sessionName)!; - expect(session.saveScriptBoundary).toBe(2); + expect(repairSessionBoundary(session)).toBe(2); expect(session.actions.map((a) => a.command)).toEqual(['wait', 'wait', 'open', 'click']); - const healed = session.actions.slice(session.saveScriptBoundary ?? 0); + const healed = session.actions.slice(repairSessionBoundary(session) ?? 0); expect(healed.map((a) => a.command)).toEqual(['open', 'click']); }); @@ -323,8 +335,8 @@ test('opt-in: without --save-script, replay neither arms recording nor records e const session = sessionStore.get(sessionName)!; expect(session.recordSession).toBeFalsy(); - expect(session.saveScriptBoundary).toBeUndefined(); - expect(session.saveScriptPath).toBeUndefined(); + expect(repairSessionBoundary(session)).toBeUndefined(); + expect(scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBeUndefined(); expect(session.actions.every((a) => a.targetEvidence === undefined)).toBe(true); }); @@ -352,7 +364,7 @@ test('a thrown/failed dispatch never lands a partial action in session.actions', test('a --no-record state-fix action never enters session.actions', () => { const { sessionStore, sessionName } = setup('agent-device-replay-repair-norecord-', { recordSession: true, - saveScriptBoundary: 0, + scriptPublication: repairPublication('armed', { boundary: 0 }), }); const session = sessionStore.get(sessionName)!; @@ -399,7 +411,7 @@ test('R1 bootstrap: a session created by step 1 (open) arms in time for step 2 t expect(response.ok).toBe(true); const session = sessionStore.get(sessionName)!; expect(session.recordSession).toBe(true); - expect(session.saveScriptBoundary).toBe(0); + expect(repairSessionBoundary(session)).toBe(0); expect(session.actions[1]?.targetEvidence).toBeDefined(); }); @@ -426,13 +438,13 @@ test('BLOCKER 4: a minimal [open, terminal close] cold-start script arms the tra // ARMED-before-step-1 semantics satisfied: the transaction armed even though // `open` created the session. expect(session.recordSession).toBe(true); - expect(session.saveScriptBoundary).toBe(0); + expect(repairSessionBoundary(session)).toBe(0); // The terminal `close` was recognized as lifecycle and SKIPPED (never // dispatched), leaving the session alive for finalize. expect(spy.map((r) => r.command)).toEqual(['open']); expect(sessionStore.get(sessionName)).toBeDefined(); // The commit-state machine applies: a completed armed transaction. - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); }); // --- ADR 0012 decision 6 (Fix 3): the source plan's own terminal `close` is @@ -470,7 +482,7 @@ test("Fix 3: the source plan's terminal close is skipped (never dispatched, neve // C4: skipping the terminal close does NOT delete the session — it stays // alive and COMPLETE so the agent can finalize it with `close --save-script`. expect(sessionStore.get(sessionName)).toBeDefined(); - expect(session.saveScriptComplete).toBe(true); + expect(sessionRepairStatus(session)).toBe('complete'); }); test('Fix 3: an ordinary (non-repair) replay still dispatches its terminal close normally', async () => { @@ -617,7 +629,7 @@ test('a --from continuation WITHOUT --save-script that diverges is still held al }; expect(leg1Divergence.resume.repairSessionHeld).toBe(true); const armed = sessionStore.get(sessionName)!; - expect(armed.saveScriptBoundary).not.toBeUndefined(); + expect(repairSessionBoundary(armed)).not.toBeUndefined(); // Leg 2: the `--from 3` continuation carries NO --save-script. It re-diverges // at step 3 — and must STILL be held alive, keyed off the persisted armed diff --git a/src/daemon/handlers/__tests__/session-replay-repair-record-exclusion.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-record-exclusion.test.ts index 861cf8cc1..0aef43875 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-record-exclusion.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-record-exclusion.test.ts @@ -215,7 +215,9 @@ test('diagnostic get/is reads mid-repair are excluded from the healed script by }); expect(leg2.ok).toBe(true); expect(session.actions.map((a) => a.command)).toEqual(['open', 'press', 'click']); - expect(session.saveScriptComplete).toBe(true); + expect(session.scriptPublication?.kind === 'repair' && session.scriptPublication.status).toBe( + 'complete', + ); // --- Finalize: `close --save-script` commits the healed `.ad`. --- const closeResponse = await handleCloseCommand({ @@ -391,11 +393,11 @@ test('empty-segment guard: a --from resume refuses with an actionable --record h test('non-repair authoring recording is unchanged: a read in a fresh `open --save-script` session still records with no flag needed', async () => { const ctx = setup('agent-device-repair-record-exclusion-authoring-'); - // An ordinary, non-repair recording session: no `saveScriptBoundary` (never + // An ordinary, non-repair recording session: no repair variant (never // armed by a repair `replay --save-script`). const session = ctx.sessionStore.get(ctx.sessionName)!; session.recordSession = true; - expect(session.saveScriptBoundary).toBeUndefined(); + expect(session.scriptPublication).toBeUndefined(); ctx.sessionStore.recordAction(session, { command: 'get', diff --git a/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts b/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts index bab883975..b17a213de 100644 --- a/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts @@ -65,10 +65,21 @@ import { AppError } from '@agent-device/kernel/errors'; import { makeIosSession, makeRepairCompleteSession, + repairPublication, } from '../../../__tests__/test-utils/session-factories.ts'; import { HEAL_COMPLETE_SENTINEL } from '../../session-script-writer.ts'; +import { + repairSessionBoundary, + repairSessionSourcePath, + markRepairTransactionComplete, +} from '../../session-replay-transaction.ts'; +import { + NO_SCRIPT_PUBLICATION, + scriptTargetPath, + scriptTargetForce, +} from '../../session-script-publication-state.ts'; import { parseReplayScriptDetailed } from '../../../replay/script.ts'; -import type { DaemonRequest } from '../../types.ts'; +import type { DaemonRequest, SessionState } from '../../types.ts'; import { baseReplayRequest as baseReq, writeReplayFile, @@ -77,6 +88,23 @@ import { freshEvidence, makeRecordingReplayInvoke } from './session-replay-repai const mockDispatchCommand = vi.mocked(dispatchCommand); +/** The persisted per-target overwrite grant (#1258), or `false` outside any publication. */ +function sessionTargetForce(session: SessionState | undefined): boolean { + return scriptTargetForce(session?.scriptPublication ?? NO_SCRIPT_PUBLICATION); +} + +/** The explicit publication target path, or `undefined` for none/default. */ +function sessionTargetPath(session: SessionState | undefined): string | undefined { + return scriptTargetPath(session?.scriptPublication ?? NO_SCRIPT_PUBLICATION); +} + +/** The recorded platform-close receipt identity, or `undefined` outside a repair transaction. */ +function sessionCloseReceipt(session: SessionState | undefined): string | undefined { + return session?.scriptPublication?.kind === 'repair' + ? session.scriptPublication.closeReceipt + : undefined; +} + beforeEach(() => { mockDispatchCommand.mockReset(); // The "current" app state: "save" was renamed to "save-v2" (why step 2 @@ -162,7 +190,10 @@ test('end-to-end repair transaction: cold divergence stays alive, corrective res expect(sessionStore.get(sessionName)!.actions.map((a) => a.command)).toEqual(['open']); // C2: the transaction is NOT complete yet — a `close` here would abort, not // commit a prefix. - expect(sessionStore.get(sessionName)!.saveScriptComplete).toBeFalsy(); + expect(sessionStore.get(sessionName)!.scriptPublication).toMatchObject({ + kind: 'repair', + status: 'armed', + }); // --- Agent performs the corrective press (blessed @ref), recorded live. --- const session = sessionStore.get(sessionName)!; @@ -193,7 +224,7 @@ test('end-to-end repair transaction: cold divergence stays alive, corrective res expect(session.actions.some((a) => a.command === 'close')).toBe(false); // C2: the resume reached the last executable step (terminal close skipped) — // the transaction is now COMPLETE and commit-eligible. - expect(session.saveScriptComplete).toBe(true); + expect(session.scriptPublication).toMatchObject({ kind: 'repair', status: 'complete' }); // --- The agent finalizes: `close --save-script` (the real handler, not a // direct writer call) commits the now-COMPLETE healed `.ad`. --- @@ -307,8 +338,8 @@ test('C5a: an incomplete repair reaped by idle-reap leaves a tombstone (no heale }); expect(leg1.ok).toBe(false); const session = sessionStore.get(sessionName)!; - expect(session.saveScriptComplete).toBeFalsy(); - expect(session.repairSourcePath).toBe(filePath); + expect(session.scriptPublication).toMatchObject({ kind: 'repair', status: 'armed' }); + expect(repairSessionSourcePath(session)).toBe(filePath); // Idle-reap tears the still-incomplete repair session down: the writer commits // nothing (not complete) and a tombstone is left behind (the exact teardown @@ -355,7 +386,7 @@ test('C5a/BLOCKER 3: teardown of a COMPLETE repair auto-commits a self-contained }); expect(response.ok).toBe(true); const session = sessionStore.get(sessionName)!; - expect(session.saveScriptComplete).toBe(true); + expect(session.scriptPublication).toMatchObject({ kind: 'repair', status: 'complete' }); // Fix 3: the source plan's terminal `close` never dispatched or recorded — // this is exactly the skip BLOCKER 3 must still account for at teardown. expect(session.actions.map((a) => a.command)).toEqual(['open', 'click']); @@ -431,7 +462,7 @@ test('BLOCKER 1: a --from continuation on a reaped session returns SESSION_NOT_F function makeCompleteRepairSession(sessionStore: SessionStore, sessionName: string, root: string) { const session = makeRepairCompleteSession(sessionName, { appBundleId: 'com.example.app', - saveScriptPath: path.join(root, 'flow.healed.ad'), + scriptPublication: repairPublication('complete', { path: path.join(root, 'flow.healed.ad') }), actions: [ { ts: 1, command: 'open', positionals: ['Demo'], flags: {} }, { @@ -585,7 +616,7 @@ test('#1258 arm-time preflight: an existing --save-script target rejects BEFORE expect(spy).toHaveLength(0); // The session was never armed (no boundary stamped) either — the whole // repair-arm side effect is skipped, not just the dispatch. - expect(sessionStore.get(sessionName)?.saveScriptBoundary).toBeUndefined(); + expect(repairSessionBoundary(sessionStore.get(sessionName))).toBeUndefined(); }); test('#1258: --force skips the arm-time preflight and the replay proceeds despite the existing target', async () => { @@ -613,7 +644,7 @@ test('#1258: --force skips the arm-time preflight and the replay proceeds despit expect(spy.map((r) => r.command)).toEqual(['open']); // `force` is persisted on the session from arm time, so a LATER commit // (e.g. a bare `close`, or teardown) still honors the overwrite. - expect(sessionStore.get(sessionName)?.saveScriptForce).toBe(true); + expect(sessionTargetForce(sessionStore.get(sessionName))).toBe(true); }); test('#1258 preflight honors PERSISTED force: a --from continuation without --force is NOT rejected on an existing target a prior --force leg authorized', async () => { @@ -650,7 +681,7 @@ test('#1258 preflight honors PERSISTED force: a --from continuation without --fo if (leg1.ok) return; expect(leg1.error.code).toBe('REPLAY_DIVERGENCE'); const divergence = leg1.error.details?.divergence as { resume: { planDigest: string } }; - expect(sessionStore.get(sessionName)?.saveScriptForce).toBe(true); + expect(sessionTargetForce(sessionStore.get(sessionName))).toBe(true); // The agent's corrective press (blessed @ref), recorded live. const session = sessionStore.get(sessionName)!; @@ -678,9 +709,12 @@ test('#1258 preflight honors PERSISTED force: a --from continuation without --fo // Not rejected by the arm-time preflight (no "already exists"): the // transaction reached completion instead. expect(leg2.ok).toBe(true); - expect(sessionStore.get(sessionName)?.saveScriptComplete).toBe(true); + expect(sessionStore.get(sessionName)?.scriptPublication).toMatchObject({ + kind: 'repair', + status: 'complete', + }); // A bare-boolean continuation never retargets, so force stays persisted. - expect(sessionStore.get(sessionName)?.saveScriptForce).toBe(true); + expect(sessionTargetForce(sessionStore.get(sessionName))).toBe(true); }); test('#1258 preflight is per-target: a --from continuation RETARGETING to an existing WITHOUT live force is refused BEFORE dispatch, preserves the prior COMPLETE transaction, and a later close still commits the ORIGINAL ', async () => { @@ -721,8 +755,8 @@ test('#1258 preflight is per-target: a --from continuation RETARGETING to an exi if (leg1.ok) return; const divergence = leg1.error.details?.divergence as { resume: { planDigest: string } }; const session = sessionStore.get(sessionName)!; - expect(session.saveScriptPath).toBe(targetA); - expect(session.saveScriptForce).toBe(true); + expect(sessionTargetPath(session)).toBe(targetA); + expect(sessionTargetForce(session)).toBe(true); // The agent's corrective press. sessionStore.recordAction(session, { command: 'press', @@ -736,7 +770,7 @@ test('#1258 preflight is per-target: a --from continuation RETARGETING to an exi // convention, to isolate THIS test's concern: a later retarget REJECTION must // not corrupt this flag (BLOCKER 2 — the C2 `saveScriptComplete = false` // reset must run AFTER the preflight's early-return, never before it). - session.saveScriptComplete = true; + markRepairTransactionComplete(session); const dispatchesBeforeLeg2 = spy.length; // Leg 2: `--from N --save-script=` (explicit RETARGET, NO live force) — @@ -763,11 +797,14 @@ test('#1258 preflight is per-target: a --from continuation RETARGETING to an exi expect(spy.length).toBe(dispatchesBeforeLeg2); // READ-ONLY: the rejected request left the session target untouched — still // armed/forced for , never retargeted to . - expect(sessionStore.get(sessionName)?.saveScriptPath).toBe(targetA); - expect(sessionStore.get(sessionName)?.saveScriptForce).toBe(true); + expect(sessionTargetPath(sessionStore.get(sessionName))).toBe(targetA); + expect(sessionTargetForce(sessionStore.get(sessionName))).toBe(true); // BLOCKER 2 (a): the prior COMPLETE transaction SURVIVES the rejection — the // C2 completion reset never ran, because the preflight returned first. - expect(sessionStore.get(sessionName)?.saveScriptComplete).toBe(true); + expect(sessionStore.get(sessionName)?.scriptPublication).toMatchObject({ + kind: 'repair', + status: 'complete', + }); // is byte-for-byte untouched. expect(fs.readFileSync(targetB, 'utf8')).toBe(beforeB); @@ -796,7 +833,11 @@ test('#1258 force is per-target: re-arming --save-script= WITHOUT --force dro ); // Armed and forced for target (flow.healed.ad). const session = makeCompleteRepairSession(sessionStore, sessionName, root); - session.saveScriptForce = true; + session.scriptPublication = repairPublication('complete', { + boundary: 0, + path: path.join(root, 'flow.healed.ad'), + force: true, + }); // A DIFFERENT, unrelated file already sits at the retarget destination // (flow.promoted.ad) — nobody opted to overwrite THIS one. const promotedPath = path.join(root, 'flow.promoted.ad'); @@ -829,7 +870,7 @@ test('#1258 force is per-target: re-arming --save-script= WITHOUT --force dro // is untouched, and the session is kept for retry. expect(fs.readFileSync(promotedPath, 'utf8')).toBe(before); expect(sessionStore.get(sessionName)).toBeDefined(); - expect(sessionStore.get(sessionName)?.saveScriptForce).toBeUndefined(); + expect(sessionTargetForce(sessionStore.get(sessionName))).toBe(false); }); test('#1258 force per-target, contrast: re-arming --save-script= WITH --force DOES overwrite ', async () => { @@ -837,7 +878,11 @@ test('#1258 force per-target, contrast: re-arming --save-script= WITH --force 'agent-device-repair-transaction-retarget-force-overwrites-', ); const session = makeCompleteRepairSession(sessionStore, sessionName, root); - session.saveScriptForce = true; + session.scriptPublication = repairPublication('complete', { + boundary: 0, + path: path.join(root, 'flow.healed.ad'), + force: true, + }); const promotedPath = path.join(root, 'flow.promoted.ad'); fs.writeFileSync( promotedPath, @@ -985,7 +1030,7 @@ test('BLOCKER 3 (second follow-up): a retry after a SUCCESSFUL platform close bu if (!closeResponse.ok) expect(closeResponse.error.message).toMatch(/already exists/); expect(sessionStore.get(sessionName)).toBeDefined(); expect(fs.readFileSync(healedPath, 'utf8')).toBe(before); - expect(sessionStore.get(sessionName)!.repairPlatformCloseReceipt).toBeDefined(); + expect(sessionCloseReceipt(sessionStore.get(sessionName))).toBeDefined(); // Retry with an explicit path: the ALREADY-SUCCEEDED platform close must // NEVER be dispatched again — a non-idempotent backend could fail (or @@ -1116,7 +1161,7 @@ test('BLOCKER 3 (third follow-up): a retry targeting a DIFFERENT app than the su }); expect(first.ok).toBe(false); expect(mockDispatchCommand).toHaveBeenCalledTimes(1); - expect(sessionStore.get(sessionName)!.repairPlatformCloseReceipt).toBeDefined(); + expect(sessionCloseReceipt(sessionStore.get(sessionName))).toBeDefined(); // Retry targets a DIFFERENT app (app-b) — a genuinely different platform // operation. The prior session-wide marker would wrongly treat app-b as diff --git a/src/daemon/handlers/__tests__/session-script-publication.test.ts b/src/daemon/handlers/__tests__/session-script-publication.test.ts index 209cddcce..5ad625e97 100644 --- a/src/daemon/handlers/__tests__/session-script-publication.test.ts +++ b/src/daemon/handlers/__tests__/session-script-publication.test.ts @@ -4,10 +4,19 @@ import path from 'node:path'; import { beforeEach, expect, test } from 'vitest'; import { INTERNAL_COMMANDS } from '../../../command-catalog.ts'; import { makeIosSession, makeAuthoringSession } from '../../../__tests__/test-utils/index.ts'; +import { + authoringPublication, + repairPublication, +} from '../../../__tests__/test-utils/session-factories.ts'; import type { TargetAnnotationV1 } from '../../../replay/target-identity.ts'; import { SessionStore } from '../../session-store.ts'; import type { DaemonRequest, SessionState } from '../../types.ts'; import { handleSessionScriptPublication } from '../session-script-publication.ts'; +import { + NO_SCRIPT_PUBLICATION, + scriptTargetPath, + scriptTargetForce, +} from '../../session-script-publication-state.ts'; const TARGET_EVIDENCE: TargetAnnotationV1 = { id: 'continue', @@ -40,7 +49,7 @@ beforeEach(() => { function armedSession(overrides: Partial = {}): SessionState { return makeAuthoringSession('authoring', { - scriptRecordingState: 'armed', + scriptPublication: authoringPublication('armed'), actions: [ { ts: 1, command: 'open', positionals: ['Demo'], flags: { saveScript: true } }, { @@ -90,7 +99,7 @@ test('publishes without close, returns the path/count, and leaves a terminal liv expect(fs.readFileSync(outputPath, 'utf8')).toContain('wait "id=\\"screen-x\\""'); expect(fs.readFileSync(outputPath, 'utf8')).not.toContain('\nclose'); expect(store.get('authoring')).toBe(session); - expect(session.scriptRecordingState).toBe('published'); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'published' }); expect(session.recordSession).toBe(false); const repeated = handleSessionScriptPublication({ @@ -117,8 +126,8 @@ test('no-clobber failure preserves bytes and armed state, then --force retries s }); expect(refused).toMatchObject({ ok: false, error: { retriable: true } }); expect(fs.readFileSync(outputPath, 'utf8')).toBe('original\n'); - expect(session.scriptRecordingState).toBe('armed'); - expect(session.saveScriptPath).toBe(outputPath); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'armed' }); + expect(scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBe(outputPath); const replaced = handleSessionScriptPublication({ req: request(outputPath, true), @@ -127,7 +136,7 @@ test('no-clobber failure preserves bytes and armed state, then --force retries s }); expect(replaced?.ok).toBe(true); expect(fs.readFileSync(outputPath, 'utf8')).toContain('context platform=ios'); - expect(session.scriptRecordingState).toBe('published'); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'published' }); }); test('retargeting without --force clears force authorization from the previous target', () => { @@ -135,8 +144,7 @@ test('retargeting without --force clears force authorization from the previous t const retargetPath = path.join(root, 'retarget.ad'); fs.writeFileSync(retargetPath, 'protected\n'); const session = armedSession({ - saveScriptPath: originalPath, - saveScriptForce: true, + scriptPublication: authoringPublication('armed', { path: originalPath, force: true }), }); store.set('authoring', session); @@ -148,9 +156,9 @@ test('retargeting without --force clears force authorization from the previous t expect(response).toMatchObject({ ok: false, error: { retriable: true } }); expect(fs.readFileSync(retargetPath, 'utf8')).toBe('protected\n'); - expect(session.scriptRecordingState).toBe('armed'); - expect(session.saveScriptPath).toBe(retargetPath); - expect(session.saveScriptForce).toBeUndefined(); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'armed' }); + expect(scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBe(retargetPath); + expect(scriptTargetForce(session.scriptPublication ?? NO_SCRIPT_PUBLICATION)).toBe(false); }); test('refuses unarmed and repair-owned sessions before filesystem work', () => { @@ -167,7 +175,10 @@ test('refuses unarmed and repair-owned sessions before filesystem work', () => { }); expect(fs.existsSync(path.dirname(outputPath))).toBe(false); - store.set('authoring', armedSession({ saveScriptBoundary: 0 })); + store.set( + 'authoring', + armedSession({ scriptPublication: repairPublication('armed', { boundary: 0 }) }), + ); const repair = handleSessionScriptPublication({ req: request(outputPath), sessionName: 'authoring', @@ -194,7 +205,7 @@ test('rejects an explicitly empty destination path', () => { ok: false, error: { code: 'INVALID_ARGS', message: expect.stringMatching(/path cannot be empty/) }, }); - expect(session.scriptRecordingState).toBe('armed'); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'armed' }); }); test('invalid destination guard remains armed and creates no target directory', () => { @@ -216,7 +227,7 @@ test('invalid destination guard remains armed and creates no target directory', ok: false, error: { message: expect.stringMatching(/destination guard/), retriable: true }, }); - expect(session.scriptRecordingState).toBe('armed'); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'armed' }); expect(fs.existsSync(path.dirname(outputPath))).toBe(false); }); @@ -249,7 +260,7 @@ test('missing initial open is non-retriable within the armed session', () => { retriable: false, }, }); - expect(session.scriptRecordingState).toBe('armed'); + expect(session.scriptPublication).toMatchObject({ kind: 'authoring', status: 'armed' }); expect(fs.existsSync(path.dirname(outputPath))).toBe(false); }); diff --git a/src/daemon/handlers/session-close-script.ts b/src/daemon/handlers/session-close-script.ts index 560fa2a16..acb2e418c 100644 --- a/src/daemon/handlers/session-close-script.ts +++ b/src/daemon/handlers/session-close-script.ts @@ -3,6 +3,13 @@ import { successText } from '../../utils/success-text.ts'; import type { SessionStore } from '../session-store.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; import { recordSessionAction } from './handler-utils.ts'; +import { NO_SCRIPT_PUBLICATION, scriptTargetPath } from '../session-script-publication-state.ts'; +import { + effectiveWriteForce, + isSessionScriptPublished, + markCloseGeneratedPublicationDone, +} from '../session-script-publication-capability.ts'; +import { abortRepairTransaction, isRepairArmedSession } from '../session-replay-transaction.ts'; export type RepairCloseCommit = | { kind: 'not-armed' } @@ -10,24 +17,21 @@ export type RepairCloseCommit = | { kind: 'aborted' } | { kind: 'failed'; error: AppError }; -function shouldOverwriteSavedScript(req: DaemonRequest, session: SessionState): boolean { - return Boolean(req.flags?.force || session.saveScriptForce); -} - export function commitRepairScriptBeforeClose( sessionStore: SessionStore, session: SessionState, req: DaemonRequest, ): RepairCloseCommit { - if (session.saveScriptBoundary === undefined) return { kind: 'not-armed' }; + if (!isRepairArmedSession(session)) return { kind: 'not-armed' }; const actionsBeforeClose = session.actions.length; recordSessionAction(sessionStore, session, req, 'close', { session: session.name, ...successText(`Closed: ${session.name}`), }); + const alreadyPublished = isSessionScriptPublished(session); const result = sessionStore.writeSessionLog(session, { - force: shouldOverwriteSavedScript(req, session), + force: effectiveWriteForce(session, req.flags?.force), }); if (result.written) return { kind: 'committed', path: result.path }; if (result.error) { @@ -35,7 +39,9 @@ export function commitRepairScriptBeforeClose( session.actions.length = actionsBeforeClose; return { kind: 'failed', error: result.error }; } - return session.saveScriptComplete ? { kind: 'committed' } : { kind: 'aborted' }; + if (alreadyPublished) return { kind: 'committed' }; + abortRepairTransaction(session); + return { kind: 'aborted' }; } export function buildRetriableRepairCloseFailureResponse( @@ -43,6 +49,7 @@ export function buildRetriableRepairCloseFailureResponse( error: AppError, ): DaemonResponse { const normalized = normalizeError(error); + const savedScript = scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION); return { ok: false, error: { @@ -50,7 +57,7 @@ export function buildRetriableRepairCloseFailureResponse( details: { ...normalized.details, session: session.name, - ...(session.saveScriptPath ? { savedScript: session.saveScriptPath } : {}), + ...(savedScript ? { savedScript } : {}), }, retriable: true, }, @@ -70,12 +77,16 @@ export function finalizeOrdinaryCloseScript(params: { ...successText(`Closed: ${session.name}`), }); } + // The recorded close action already armed target/force through the recorder's flag ingress. + // On a platform-close failure that action was never recorded; the log still publishes, but — + // as before this migration — to the session default, not the request's explicit path. if (req.flags?.saveScript) session.recordSession = true; try { - sessionStore.writeSessionLog(session, { - force: shouldOverwriteSavedScript(req, session), + const result = sessionStore.writeSessionLog(session, { + force: effectiveWriteForce(session, req.flags?.force), }); + if (result.written) markCloseGeneratedPublicationDone(session, result.path); return undefined; } catch (error) { return toOrdinaryCloseSaveScriptFailure(error); diff --git a/src/daemon/handlers/session-close.ts b/src/daemon/handlers/session-close.ts index 6cf5d148c..4f587960a 100644 --- a/src/daemon/handlers/session-close.ts +++ b/src/daemon/handlers/session-close.ts @@ -27,6 +27,11 @@ import { stopSessionRecordingForTeardown } from './record-trace-recording.ts'; import type { LeaseRegistry } from '../lease-registry.ts'; import { releaseSessionLease } from '../lease-lifecycle.ts'; import type { LeaseLifecycleProvider } from '@agent-device/contracts/device'; +import { + hasRepairPlatformCloseReceipt, + isRepairArmedSession, + recordRepairPlatformClose, +} from '../session-replay-transaction.ts'; import { reportSessionCleanupFailures, restoreSessionAndroidIme, @@ -152,11 +157,13 @@ async function prepareRepairClose(params: { sessionStore: SessionStore; }): Promise { const { req, session, logPath, sessionStore } = params; - const repairArmed = session.saveScriptBoundary !== undefined; + const repairArmed = isRepairArmedSession(session); const closeReceipt = buildRepairPlatformCloseReceipt(req); - if (repairArmed && session.repairPlatformCloseReceipt !== closeReceipt) { + if (repairArmed && !hasRepairPlatformCloseReceipt(session, closeReceipt)) { const platformCloseError = await dispatchTargetedPlatformClose({ req, session, logPath }); if (platformCloseError) { + // Platform-close failure leaves the transaction state unchanged: no receipt is recorded, + // so the retry dispatches afresh. return { response: buildRetriableRepairCloseFailureResponse( session, @@ -164,15 +171,16 @@ async function prepareRepairClose(params: { ), }; } - session.repairPlatformCloseReceipt = closeReceipt; + recordRepairPlatformClose(session, closeReceipt); } const repairCommit = commitRepairScriptBeforeClose(sessionStore, session, req); if (repairCommit.kind === 'failed') { + // Publication failure retains target, force, and the close receipt; the same-identity retry + // skips close dispatch above. return { response: buildRetriableRepairCloseFailureResponse(session, repairCommit.error), }; } - session.repairPlatformCloseReceipt = undefined; return { repairArmed, ...(repairCommit.kind === 'committed' && repairCommit.path @@ -278,12 +286,11 @@ async function stopOrRetainAppleRunnerAfterClose( function assertTerminalRecordingCloseAllowed(req: DaemonRequest, session: SessionState): void { if (!req.flags?.saveScript) return; - if (session.scriptRecordingState !== 'aborted' && session.scriptRecordingState !== 'published') { - return; - } + const state = session.scriptPublication; + if (state?.kind !== 'authoring' || state.status === 'armed') return; throw new AppError( 'INVALID_ARGS', - `close --save-script cannot ${session.scriptRecordingState === 'published' ? 're-publish' : 'publish'} this terminal recording. Retry with plain close; it will tear down the session without writing.`, + `close --save-script cannot ${state.status === 'published' ? 're-publish' : 'publish'} this terminal recording. Retry with plain close; it will tear down the session without writing.`, ); } diff --git a/src/daemon/handlers/session-open.ts b/src/daemon/handlers/session-open.ts index d7269969d..3481d17aa 100644 --- a/src/daemon/handlers/session-open.ts +++ b/src/daemon/handlers/session-open.ts @@ -1,5 +1,9 @@ import path from 'node:path'; import { dispatchCommand, resolveTargetDevice } from '../../core/dispatch.ts'; +import { + abortAuthoringOnSecondOpen, + armAuthoringOnOpen, +} from '../session-script-publication-capability.ts'; import type { SessionSurface } from '@agent-device/contracts/session'; import { contextFromFlags } from '../context.ts'; import { createRequestCanceledError, isRequestCanceled } from '../../request/cancel.ts'; @@ -94,13 +98,14 @@ function applyOrdinaryScriptRecordingOpenOutcome(params: { }): void { const { session, existingSession, saveScriptRequested, responseData } = params; if (!existingSession && saveScriptRequested) { - session.scriptRecordingState = 'armed'; - session.recordSession = true; + // The recorded `open` action's flag ingress applies the explicit path/force right after + // this arm (`applyRecordedSaveScriptFlags`), exactly as the field writers used to split it. + armAuthoringOnOpen(session, {}); return; } - if (existingSession?.scriptRecordingState !== 'armed') return; - session.scriptRecordingState = 'aborted'; - session.recordSession = false; + const existingState = existingSession?.scriptPublication; + if (existingState?.kind !== 'authoring' || existingState.status !== 'armed') return; + abortAuthoringOnSecondOpen(session); const warnings = Array.isArray(responseData.warnings) ? responseData.warnings.filter((warning): warning is string => typeof warning === 'string') : []; diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index 2f78e9213..9f3a7e095 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -13,7 +13,6 @@ import { SessionStore } from '../session-store.ts'; import { clearPendingRecordAndHealWatermark } from './session-replay-resume.ts'; import { expandSessionPath } from '../session-paths.ts'; import { buildReplayScriptPlatformFlags } from '../replay-device-selection.ts'; -import { applySaveScriptRetarget } from '../session-action-recorder.ts'; import { computeReplayPlanDigest } from '../../replay/plan-digest.ts'; import type { TargetAnnotationV1 } from '../../replay/target-identity.ts'; import { errorResponse, noActiveSessionError } from './response.ts'; @@ -59,6 +58,18 @@ import { buildReplayBuiltinVars } from './session-replay-vars.ts'; import { runTypedMaestroReplayFile } from './session-replay-maestro-runtime.ts'; import type { ReplayTestAttemptStep, ReplayTestAttemptStepSink } from '@agent-device/replay-test'; import { getRequestSignal } from '../../request/cancel.ts'; +import { + NO_SCRIPT_PUBLICATION, + scriptTargetForce, + scriptTargetPath, +} from '../session-script-publication-state.ts'; +import { + armRepairStep, + isUncommittedRepairSession, + markRepairTransactionComplete, + repairSessionBoundary, + resetRepairCompletionForRerun, +} from '../session-replay-transaction.ts'; /** Per-run invariants for a single replay step (ADR 0012 step 4 verify + dispatch + guard). */ type ReplayStepContext = { @@ -219,7 +230,7 @@ export async function runReplayScriptFile(params: { return errorResponse('INVALID_ARGS', maestroBackendRequiredMessage('replay', filePath)); } if (resolveReplayFormat(resolved, req.flags?.replayBackend) === 'maestro') { - if (sessionStore.get(sessionName)?.saveScriptBoundary !== undefined) { + if (repairSessionBoundary(sessionStore.get(sessionName)) !== undefined) { return errorResponse( 'INVALID_ARGS', 'This session has an active .ad --save-script repair run; finish it with replay --from or close before running Maestro YAML.', @@ -436,8 +447,8 @@ function completeReplayRun(params: { } = params; armSaveScript(); const completedSession = sessionStore.get(sessionName); - if (completedSession?.saveScriptBoundary !== undefined) { - completedSession.saveScriptComplete = true; + if (completedSession && repairSessionBoundary(completedSession) !== undefined) { + markRepairTransactionComplete(completedSession); sessionStore.set(sessionName, completedSession); } const replayedCount = actions.length - entryIndex; @@ -647,30 +658,28 @@ function prepareSaveScriptSession(params: { const { req, sessionStore, sessionName, sourcePath } = params; const preRunSession = sessionStore.get(sessionName); const { saveScript, force } = req.flags ?? {}; - const { - saveScriptForce: persistedForce, - saveScriptPath: existingSaveScriptPath, - saveScriptBoundary, - } = preRunSession ?? {}; - if (saveScript && preRunSession?.scriptRecordingState !== undefined) { + const preRunState = preRunSession?.scriptPublication ?? NO_SCRIPT_PUBLICATION; + if (saveScript && preRunState.kind === 'authoring') { return { ok: false, response: errorResponse( 'INVALID_ARGS', - `replay --save-script cannot re-arm an ordinary recording in terminal/active state ${preRunSession.scriptRecordingState}. Close this session and use a fresh one for repair authoring.`, + `replay --save-script cannot re-arm an ordinary recording in terminal/active state ${preRunState.status}. Close this session and use a fresh one for repair authoring.`, ), }; } const saveScriptPreflight = preflightSaveScriptTarget({ saveScript, liveForce: force, - persistedForce, + persistedForce: scriptTargetForce(preRunState) || undefined, sourcePath, - existingSaveScriptPath, + existingSaveScriptPath: scriptTargetPath(preRunState), }); if (saveScriptPreflight) return { ok: false, response: saveScriptPreflight }; - if (preRunSession && saveScriptBoundary !== undefined) preRunSession.saveScriptComplete = false; + if (preRunSession && repairSessionBoundary(preRunSession) !== undefined) { + resetRepairCompletionForRerun(preRunSession); + } return { ok: true, armSaveScript: createReplaySaveScriptArmer({ @@ -717,7 +726,7 @@ function preflightReplayAgainstActiveRepair(params: { }): DaemonResponse | undefined { const { entryIndex, sessionStore, sessionName } = params; if (entryIndex > 0) return undefined; - if (sessionStore.get(sessionName)?.saveScriptBoundary === undefined) return undefined; + if (repairSessionBoundary(sessionStore.get(sessionName)) === undefined) return undefined; return errorResponse( 'INVALID_ARGS', 'This session has an active --save-script repair run; continue it with replay --from --plan-digest , or finish with close, before starting a fresh full replay.', @@ -736,15 +745,15 @@ function preflightReplayAgainstActiveRepair(params: { * sibling) WITHOUT needing the session to exist yet, so it runs before step 1 * dispatches even when that step is the `open` that creates the session. * READ-ONLY: it never mutates the session (it runs before - * `applySaveScriptRetarget`). + * `resolveScriptTarget`). * - * The effective-force decision MATCHES `applySaveScriptRetarget`'s per-target + * The effective-force decision MATCHES `resolveScriptTarget`'s per-target * contract, computed against the target THIS request resolves to: a live - * `--force`/`--overwrite` always bypasses; a PERSISTED `saveScriptForce` + * `--force`/`--overwrite` always bypasses; a PERSISTED per-target grant * bypasses ONLY when this request writes to the SAME target it was granted for * (`targetPath === existingSaveScriptPath`). An explicit RETARGET to a * different path without a live force does NOT bypass here — because - * `applySaveScriptRetarget` will CLEAR that persisted force for the new target + * `resolveScriptTarget` will CLEAR that persisted force for the new target * before publication anyway, so letting the run execute (mutating the session * mid-flight) only to refuse the existing target at the end is exactly what * this preflight exists to prevent. A no-op when `--save-script` was not passed. @@ -795,7 +804,7 @@ function isRepairArmedTerminalClose(params: { const { action, index, totalActions, sessionStore, sessionName } = params; if (action.command !== 'close') return false; if (index !== totalActions - 1) return false; - return sessionStore.get(sessionName)?.saveScriptBoundary !== undefined; + return repairSessionBoundary(sessionStore.get(sessionName)) !== undefined; } /** @@ -841,29 +850,13 @@ function armReplaySaveScriptStep(params: { const { sessionStore, sessionName, saveScript, force, sourcePath, firstArm } = params; const session = sessionStore.get(sessionName); if (!session) return; - session.recordSession = true; - if (typeof saveScript === 'string') { - // An EXPLICIT `--save-script=` retargets. Which of the two ways the - // path was chosen does not affect the publish decision: the writer's - // refuse-on-exist guard is uniform (`publishHealedScriptAtomically`) and - // refuses ANY pre-existing target, an explicit caller-directed path - // included, exactly like the default healed sibling. - applySaveScriptRetarget(session, expandSessionPath(saveScript), force); - } else if (session.saveScriptPath === undefined) { - session.saveScriptPath = healedScriptSiblingPath(sourcePath); - } - // #1258: force is per-target — a LIVE `--force`/`--overwrite` persists onto - // the session (`saveScriptForce`) so a LATER `--from` continuation leg or an - // unattended auto-commit teardown (no live request) still honors it. Set - // AFTER `applySaveScriptRetarget` so a live flag always wins over a - // retarget-clear. - if (force) session.saveScriptForce = true; - if (session.saveScriptBoundary === undefined) { - session.saveScriptBoundary = firstArm ? session.actions.length : 0; - } - // ADR 0012 decision 6, R7 (C5a): stash the original replay input so a reap - // tombstone can hand back an actionable `replay --save-script` re-run. - if (session.repairSourcePath === undefined) session.repairSourcePath = sourcePath; + armRepairStep(session, { + saveScript, + force, + sourcePath, + healedSiblingPath: healedScriptSiblingPath(sourcePath), + firstArm, + }); sessionStore.set(sessionName, session); } @@ -888,7 +881,7 @@ function markRepairSessionHeldIfArmed(params: { // A `replay --from` continuation (which does not repeat `--save-script`, per // R2) is therefore still held on divergence and stays in the transaction. const session = sessionStore.get(sessionName); - if (session?.saveScriptBoundary === undefined || session.saveScriptCommitted) return response; + if (!isUncommittedRepairSession(session)) return response; const resume = readDivergenceResumeRecord(response); if (resume) resume.repairSessionHeld = true; return response; diff --git a/src/daemon/handlers/session-script-publication.ts b/src/daemon/handlers/session-script-publication.ts index 8dd2daaa4..2e7c8ff44 100644 --- a/src/daemon/handlers/session-script-publication.ts +++ b/src/daemon/handlers/session-script-publication.ts @@ -1,8 +1,12 @@ import { INTERNAL_COMMANDS } from '../../command-catalog.ts'; import { AppError, normalizeError } from '@agent-device/kernel/errors'; import { successText } from '../../utils/success-text.ts'; -import { applySaveScriptRetarget } from '../session-action-recorder.ts'; -import { expandSessionPath } from '../session-paths.ts'; +import { + effectiveWriteForce, + markActivePublicationDone, + retargetActivePublication, +} from '../session-script-publication-capability.ts'; +import { isRepairArmedSession } from '../session-replay-transaction.ts'; import { SessionStore } from '../session-store.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; @@ -34,13 +38,10 @@ export function handleSessionScriptPublication(params: { if (req.positionals?.[0] !== undefined && !explicitPath) { return failure(new AppError('INVALID_ARGS', 'session save-script path cannot be empty.')); } - if (explicitPath) { - applySaveScriptRetarget(session, expandSessionPath(explicitPath), req.flags?.force); - } - if (req.flags?.force) session.saveScriptForce = true; + retargetActivePublication(session, { explicitPath, liveForce: req.flags?.force }); const result = sessionStore.writeSessionLog(session, { - force: Boolean(req.flags?.force || session.saveScriptForce), + force: effectiveWriteForce(session, req.flags?.force), publication: 'active', }); if (!result.written) { @@ -53,9 +54,7 @@ export function handleSessionScriptPublication(params: { ); } - session.scriptRecordingState = 'published'; - session.recordSession = false; - session.saveScriptPath = result.path; + markActivePublicationDone(session, result.path); return { ok: true, data: { @@ -68,7 +67,7 @@ export function handleSessionScriptPublication(params: { } function validatePublicationEligibility(session: SessionState): AppError | undefined { - if (session.saveScriptBoundary !== undefined) { + if (isRepairArmedSession(session)) { return new AppError( 'COMMAND_FAILED', 'This session has an active .ad repair transaction and cannot use ordinary active-session publication.', @@ -77,19 +76,20 @@ function validatePublicationEligibility(session: SessionState): AppError | undef }, ); } - if (session.scriptRecordingState === 'aborted') { + const state = session.scriptPublication; + if (state?.kind === 'authoring' && state.status === 'aborted') { return new AppError( 'COMMAND_FAILED', 'This script recording was aborted by a second successful open and cannot be published.', { hint: 'Close this session and start a fresh one with open --save-script[=].' }, ); } - if (session.scriptRecordingState === 'published') { + if (state?.kind === 'authoring' && state.status === 'published') { return new AppError('COMMAND_FAILED', 'This script recording has already been published.', { hint: 'Continue using the live session, or close it and start a fresh authoring session.', }); } - if (session.scriptRecordingState !== 'armed' || !session.recordSession) { + if (state?.kind !== 'authoring' || state.status !== 'armed' || !session.recordSession) { return new AppError( 'COMMAND_FAILED', 'Script recording was not armed before this journey began; session history cannot be published without recording-time target evidence.', diff --git a/src/daemon/server/daemon-idle-reap.test.ts b/src/daemon/server/daemon-idle-reap.test.ts index f87c96b37..00f1a08ad 100644 --- a/src/daemon/server/daemon-idle-reap.test.ts +++ b/src/daemon/server/daemon-idle-reap.test.ts @@ -91,13 +91,33 @@ test('isDaemonIdle requires no in-flight requests, no sessions, and no recording // reaped (with a tombstone) rather than pinning the daemon forever. --- test('a repair-armed, un-committed session does NOT block idle-reap', () => { - sessionStore.set('default', makeSession({ saveScriptBoundary: 0 })); + sessionStore.set( + 'default', + makeSession({ + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }), + ); assert.equal(hasReapBlockingOpenSessions(sessionStore), false); assert.equal(isDaemonIdle({ sessionStore, inFlightRequestCount: 0 }), true); }); test('a repair-armed session that has already COMMITTED still blocks idle-reap (until close deletes it)', () => { - sessionStore.set('default', makeSession({ saveScriptBoundary: 0, saveScriptCommitted: true })); + sessionStore.set( + 'default', + makeSession({ + scriptPublication: { + kind: 'repair', + status: 'committed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }), + ); assert.equal(hasReapBlockingOpenSessions(sessionStore), true); assert.equal(isDaemonIdle({ sessionStore, inFlightRequestCount: 0 }), false); }); @@ -109,7 +129,18 @@ test('an ordinary (non-repair) open session still blocks idle-reap', () => { }); test('a normal session alongside a reapable repair session still blocks idle-reap', () => { - sessionStore.set('default', makeSession({ name: 'default', saveScriptBoundary: 0 })); + sessionStore.set( + 'default', + makeSession({ + name: 'default', + scriptPublication: { + kind: 'repair', + status: 'armed', + target: { kind: 'default', force: false }, + boundary: 0, + }, + }), + ); sessionStore.set('other', makeSession({ name: 'other' })); assert.equal(hasReapBlockingOpenSessions(sessionStore), true); assert.equal(isDaemonIdle({ sessionStore, inFlightRequestCount: 0 }), false); diff --git a/src/daemon/server/daemon-idle-reap.ts b/src/daemon/server/daemon-idle-reap.ts index 79f80ae7e..19ad08c68 100644 --- a/src/daemon/server/daemon-idle-reap.ts +++ b/src/daemon/server/daemon-idle-reap.ts @@ -1,6 +1,7 @@ import { emitDiagnostic } from '../../utils/diagnostics.ts'; import type { SessionStore } from '../session-store.ts'; import type { SessionState } from '../types.ts'; +import { isUncommittedRepairSession } from '../session-replay-transaction.ts'; // Bounds the daemon's own lifetime when nothing is using it. Each // AGENT_DEVICE_STATE_DIR spawns a dedicated daemon that otherwise never exits @@ -46,7 +47,7 @@ export function hasReapBlockingOpenSessions(sessionStore: SessionStore): boolean } function isReapableRepairSession(session: SessionState): boolean { - return session.saveScriptBoundary !== undefined && session.saveScriptCommitted !== true; + return isUncommittedRepairSession(session); } // Recording lifecycle is session-scoped (session.recording), so a recording diff --git a/src/daemon/session-action-recorder.ts b/src/daemon/session-action-recorder.ts index 941ca1aeb..3d8d92a59 100644 --- a/src/daemon/session-action-recorder.ts +++ b/src/daemon/session-action-recorder.ts @@ -2,7 +2,8 @@ import type { CommandFlags } from '../core/dispatch.ts'; import { SCREENSHOT_ACTION_FLAG_KEYS } from '@agent-device/contracts/capture'; import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { DaemonRequest, SessionAction, SessionRuntimeHints, SessionState } from './types.ts'; -import { expandSessionPath } from './session-paths.ts'; +import { applyRecordedSaveScriptFlags } from './session-script-publication-capability.ts'; +import { repairSessionBoundary } from './session-replay-transaction.ts'; import type { TargetAnnotationV1 } from '../replay/target-identity.ts'; import { inferFillText } from './action-utils.ts'; import { @@ -37,61 +38,13 @@ export type RecordActionEntry = { interactiveObservation?: boolean; }; -/** - * #1258: point the session at `nextPath` and, when this is a RETARGET to a - * different path than the one currently persisted AND no live - * `--force`/`--overwrite` accompanies it, drop any `saveScriptForce` persisted - * for the OLD target. Force is authorization for the target it was opted into - * (`--save-script=a.ad --force`), NOT a session-wide standing grant that - * silently follows a later `--save-script=b.ad` and overwrites a file nobody - * opted to overwrite. A live `force` on this same retarget re-grants it for the - * new target (handled by the caller, AFTER this — so a live flag always wins). - * Shared by both re-arming paths: `armReplaySaveScriptStep` (replay) and - * `recordActionEntry` (close --save-script). - */ -export function applySaveScriptRetarget( - session: SessionState, - nextPath: string, - liveForce: boolean | undefined, -): void { - const retargeted = session.saveScriptPath !== undefined && session.saveScriptPath !== nextPath; - if (retargeted && !liveForce) session.saveScriptForce = undefined; - session.saveScriptPath = nextPath; -} - export function recordActionEntry( session: SessionState, entry: RecordActionEntry, ): SessionAction | undefined { if (entry.flags?.noRecord) return undefined; if (isExcludedRepairSegmentObservation(session, entry)) return undefined; - if (entry.flags?.saveScript) { - session.recordSession = true; - if (typeof entry.flags.saveScript === 'string') { - // ADR 0012 decision 6: an explicit `--save-script=` (e.g. `close - // --save-script=`) retargets away from the defaulted healed - // sibling. How the path was chosen plays no role in the publish - // decision: the writer's refuse-on-exist guard is uniform (see - // `publishHealedScriptAtomically`) and refuses ANY pre-existing target — - // an explicit, caller-DIRECTED path included. Directing the path is not - // the same as authorizing an overwrite. - applySaveScriptRetarget( - session, - expandSessionPath(entry.flags.saveScript), - entry.flags.force, - ); - } - // #1258: persist `--force`/`--overwrite`, like `saveScriptPath`, so a - // LATER write that does not repeat the flag (a bare `close` finishing a - // session opened with `open --save-script --force`, or an unattended - // auto-commit teardown with no live request) still honors it. Sticky FOR - // THE SAME TARGET — a retarget to a different path without a live `force` - // drops it (see `applySaveScriptRetarget`); a same-path later action - // carrying `saveScript` without `force` still must not clear it. - if (entry.flags.force) { - session.saveScriptForce = true; - } - } + if (entry.flags) applyRecordedSaveScriptFlags(session, entry.flags); const recordedEntry = parameterizeRecordedFill(entry); const action: SessionAction = { ts: Date.now(), @@ -184,7 +137,7 @@ const OBSERVATION_ONLY_COMMANDS: ReadonlySet = new Set(['snapshot', 'get * (2) is why this is a PROVENANCE rule, not a command-class rule. Replayed * plan steps dispatch through the ordinary request path and land in * `session.actions` like any other action; the healed script is that slice - * (`buildOptimizedActions` over `session.actions.slice(saveScriptBoundary)`). + * (`buildOptimizedActions` over `session.actions` from the repair boundary). * So excluding by command class alone would replay an authored `is visible` * assertion and then silently drop it from its own heal — the healed flow * would quietly stop checking what it used to check. Authored observations @@ -199,7 +152,7 @@ export function isInteractiveObservation(req: DaemonRequest): boolean { /** * #1271 stage 2 (ADR 0012 amendment): the repair-segment default exclusion. * - * `session.saveScriptBoundary !== undefined` is set ONLY by a repair-armed + * A repair boundary is set ONLY by a repair-armed * `replay --save-script` (decision 6, R1/R6) — an ordinary, non-repair * `open --save-script`/`close --save-script` authoring recording never sets * it (see the ADR's "Scope" note under decision 6). Gating on this field, @@ -224,7 +177,7 @@ function isExcludedRepairSegmentObservation( entry: RecordActionEntry, ): boolean { if (!entry.interactiveObservation) return false; - if (session.saveScriptBoundary === undefined) return false; + if (repairSessionBoundary(session) === undefined) return false; return entry.flags?.record !== true; } diff --git a/src/daemon/session-replay-transaction.ts b/src/daemon/session-replay-transaction.ts new file mode 100644 index 000000000..8265f7acd --- /dev/null +++ b/src/daemon/session-replay-transaction.ts @@ -0,0 +1,119 @@ +import type { SessionState } from './types.ts'; +import { + NO_SCRIPT_PUBLICATION, + abortRepair, + armRepair, + demoteRepairToArmed, + isUncommittedRepair, + markRepairComplete, + recordRepairCloseSucceeded, + repairBoundary, + repairSourcePath, +} from './session-script-publication-state.ts'; +import { expandSessionPath } from './session-paths.ts'; + +/** + * `ReplaySessionTransaction` (#1478 P4a): the daemon-private projection through which the ADR + * 0012 decision 6 repair lifecycle is written. Handlers arm, complete, demote, and stamp close + * receipts here; nothing else assigns the repair variant. Reads stay on the pure helpers in + * `session-script-publication-state.ts`. Engines can reach neither. + */ + +function publicationState(session: SessionState | undefined) { + return session?.scriptPublication ?? NO_SCRIPT_PUBLICATION; +} + +/** + * ADR 0012 decision 6, R1/R6: arms recording on the CURRENT session and records the boundary + * watermark once. `firstArm` captures the pre-run action count on the pre-loop session, so a + * reused session's earlier actions stay excluded; a LATER arm reaching a repair-less session + * means step-1 `open` REPLACED it with a fresh `actions: []`, so the boundary is 0, keeping the + * healed `open` in the slice. An explicit `` always wins; absent one, the healed script + * defaults to the `.healed.ad` sibling (R6), materialized as an explicit target. + * + * #1258: a LIVE `--force` persists into the target authorization so a later `--from` + * continuation leg or an unattended auto-commit teardown still honors it; an explicit retarget + * without live force drops the previous target's grant (`resolveScriptTarget`). + */ +export function armRepairStep( + session: SessionState, + params: { + saveScript: boolean | string; + force: boolean | undefined; + sourcePath: string; + healedSiblingPath: string; + firstArm: boolean; + }, +): void { + session.recordSession = true; + const state = publicationState(session); + const explicitPath = + typeof params.saveScript === 'string' + ? expandSessionPath(params.saveScript) + : state.kind === 'repair' + ? undefined + : params.healedSiblingPath; + session.scriptPublication = armRepair(state, { + requested: { path: explicitPath, force: params.force === true }, + boundary: params.firstArm ? session.actions.length : 0, + sourcePath: params.sourcePath, + }); +} + +/** + * A `replay --from` continuation re-runs the plan, so completion is no longer proven; the close + * receipt survives the demotion (see `demoteRepairToArmed`). + */ +export function resetRepairCompletionForRerun(session: SessionState): void { + session.scriptPublication = demoteRepairToArmed(publicationState(session)); +} + +/** The plan reached its final executable step with no outstanding divergence (C2). */ +export function markRepairTransactionComplete(session: SessionState): void { + session.scriptPublication = markRepairComplete(publicationState(session)); +} + +/** The platform close for `operationIdentity` succeeded; retries with the same identity skip dispatch. */ +export function recordRepairPlatformClose(session: SessionState, operationIdentity: string): void { + session.scriptPublication = recordRepairCloseSucceeded( + publicationState(session), + operationIdentity, + ); +} + +/** + * Terminal failure: the transaction never completed, so a close/teardown that reaches it + * publishes nothing, forever. Receipt cleanup is part of terminality (`abortRepair`). + */ +export function abortRepairTransaction(session: SessionState): void { + session.scriptPublication = abortRepair(publicationState(session)); +} + +/** Whether the platform close for `operationIdentity` already succeeded for this transaction. */ +export function hasRepairPlatformCloseReceipt( + session: SessionState, + operationIdentity: string, +): boolean { + const state = publicationState(session); + return state.kind === 'repair' && state.closeReceipt === operationIdentity; +} + +/** Whether this session carries a repair transaction at all (any status). */ +export function isRepairArmedSession(session: SessionState | undefined): boolean { + return publicationState(session).kind === 'repair'; +} + +/** Uncommitted repair: held on divergence, reapable, and tombstoned on teardown. */ +export function isUncommittedRepairSession(session: SessionState | undefined): boolean { + return isUncommittedRepair(publicationState(session)); +} + +/** The repair watermark (R6), or `undefined` when the session is not under repair. */ +export function repairSessionBoundary(session: SessionState | undefined): number | undefined { + return repairBoundary(publicationState(session)); +} + +/** The original replay input path for reap tombstones (C5a). */ +export function repairSessionSourcePath(session: SessionState | undefined): string | undefined { + return repairSourcePath(publicationState(session)); +} diff --git a/src/daemon/session-script-publication-capability.ts b/src/daemon/session-script-publication-capability.ts new file mode 100644 index 000000000..f6ff1e73f --- /dev/null +++ b/src/daemon/session-script-publication-capability.ts @@ -0,0 +1,136 @@ +import type { CommandFlags } from '@agent-device/contracts/command'; +import type { SessionState } from './types.ts'; +import { + NO_SCRIPT_PUBLICATION, + abortAuthoring, + armAuthoring, + armRepair, + isScriptPublished, + markAuthoringPublished, + resolveScriptTarget, + scriptTargetForce, +} from './session-script-publication-state.ts'; +import { expandSessionPath } from './session-paths.ts'; + +/** + * `SessionScriptPublication` (#1478 P4a): the daemon-private projection owning every supported + * ordinary-authoring write — arming on `open`, the shared `--save-script` flag ingress on + * recorded actions, active publication (`session save-script`), and the published transition. + * Repair transitions live in `session-replay-transaction.ts`; the two variants stay disjoint by + * construction of the aggregate. Engines can reach neither. + */ + +function publicationState(session: SessionState) { + return session.scriptPublication ?? NO_SCRIPT_PUBLICATION; +} + +/** ADR 0016: `open --save-script[=]` on a fresh session arms ordinary authoring. */ +export function armAuthoringOnOpen( + session: SessionState, + requested: { path?: string; force?: boolean }, +): void { + session.recordSession = true; + session.scriptPublication = armAuthoring({ + path: requested.path !== undefined ? expandSessionPath(requested.path) : undefined, + force: requested.force === true, + }); +} + +/** + * ADR 0016: a second successful `open` on an armed authoring session terminates the recording — + * the journey no longer starts where the script says it does. + */ +export function abortAuthoringOnSecondOpen(session: SessionState): void { + session.recordSession = false; + session.scriptPublication = abortAuthoring(publicationState(session)); +} + +/** + * The shared `--save-script` ingress on a RECORDED action (`open`/`close`; #1501 pins every + * other command's raw flag closed at the router). Arms recording and applies target/force to + * whichever lifecycle the session is in: + * + * - `none` -> ordinary authoring armed. This is how a never-armed `close --save-script` + * publishes the whole log: the close request arms at record time and publishes moments later + * in the same request, folding the former "third mode" into the authoring lifecycle. The + * session is deleted by every close path that gets this far, so the transient armed state is + * unobservable to `session save-script` eligibility. + * - `authoring` -> retarget under the #1258 per-target force rule (`resolveScriptTarget`). + * - `repair` -> retarget the repair target the same way (a replayed step may carry the flag). + */ +export function applyRecordedSaveScriptFlags(session: SessionState, flags: CommandFlags): void { + if (!flags.saveScript) return; + session.recordSession = true; + const requested = { + path: typeof flags.saveScript === 'string' ? expandSessionPath(flags.saveScript) : undefined, + force: flags.force === true, + }; + const state = publicationState(session); + if (state.kind === 'repair') { + session.scriptPublication = armRepair(state, { + requested, + boundary: state.boundary, + sourcePath: state.sourcePath, + }); + return; + } + if (state.kind === 'authoring') { + session.scriptPublication = { + ...state, + target: resolveScriptTarget(state.target, requested), + }; + return; + } + session.scriptPublication = armAuthoring(requested); +} + +/** + * Retarget for active publication (`session save-script `), same #1258 rule; a live + * `--force` re-grants for the (possibly new) target. + */ +export function retargetActivePublication( + session: SessionState, + params: { explicitPath?: string; liveForce: boolean | undefined }, +): void { + const state = publicationState(session); + if (state.kind !== 'authoring') return; + session.scriptPublication = { + ...state, + target: resolveScriptTarget(state.target, { + path: params.explicitPath !== undefined ? expandSessionPath(params.explicitPath) : undefined, + force: params.liveForce === true, + }), + }; +} + +/** Active publication succeeded: record the written path and stop recording (ADR 0016). */ +export function markActivePublicationDone(session: SessionState, writtenPath: string): void { + session.scriptPublication = markAuthoringPublished(publicationState(session), writtenPath); + session.recordSession = false; +} + +/** Ordinary close publication succeeded: the authoring lifecycle reached `published`. */ +export function markCloseGeneratedPublicationDone( + session: SessionState, + writtenPath: string, +): void { + const state = publicationState(session); + if (state.kind !== 'authoring') return; + session.scriptPublication = markAuthoringPublished(state, writtenPath); +} + +/** + * The effective overwrite decision at a write site (#1258): a live `--force` on this request, or + * the per-target grant persisted at arm time. + */ +export function effectiveWriteForce( + session: SessionState, + liveForce: boolean | undefined, +): boolean { + return liveForce === true || scriptTargetForce(publicationState(session)); +} + +/** A committed/published session's second write must no-op rather than republish. */ +export function isSessionScriptPublished(session: SessionState): boolean { + return isScriptPublished(publicationState(session)); +} diff --git a/src/daemon/session-script-publication-state.ts b/src/daemon/session-script-publication-state.ts index 1e851dbd4..0514faa90 100644 --- a/src/daemon/session-script-publication-state.ts +++ b/src/daemon/session-script-publication-state.ts @@ -1,19 +1,16 @@ /** - * The tagged script-publication aggregate (#1478 P4a). + * The tagged script-publication aggregate (#1478 P4a): `SessionState.scriptPublication`. * - * Nine co-resident optional fields on `SessionState` encode two lifecycles plus a shared output - * target: `scriptRecordingState` (the ADR 0016 ordinary authoring lifecycle), the ADR 0012 - * decision 6 repair transaction (`saveScriptBoundary`, `saveScriptComplete`, - * `saveScriptCommitted`, `repairPlatformCloseReceipt`, `repairSourcePath`), and the target - * itself (`saveScriptPath`, `saveScriptForce`). + * One state machine holds both publication lifecycles and their shared output target: the ADR + * 0016 ordinary authoring lifecycle, the ADR 0012 decision 6 repair transaction, and the + * per-target force authorization (#1258). The disjointness of the two lifecycles is structural — + * a session publishes nothing, authors ordinarily, or is under repair — so no reader re-derives + * it from field combinations and no writer clears siblings. * - * Nothing in that shape says the two lifecycles are disjoint, so every reader re-derived it from - * field combinations and every writer had to remember which siblings to clear. This aggregate - * makes the disjointness structural: a session is publishing nothing, authoring ordinarily, or - * under repair. - * - * Values and pure transitions only; no authority. The capability that performs close sequencing - * and atomic publication is separate, and no engine can reach either. + * Values and pure transitions only; no authority. Writes go through the daemon-private + * projections (`session-replay-transaction.ts`, `session-script-publication-capability.ts`) and + * the writer's commit transition — the R7 ownership gate enforces exactly that set — and no + * engine can reach any of them. */ /** @@ -88,12 +85,12 @@ export const NO_SCRIPT_PUBLICATION: SessionScriptPublicationState = { kind: 'non * the caller never opted into. Because authorization is a field of the target, replacing the * target replaces its authorization — there is no separate flag left behind to forget. * - * Two retentions are deliberate, and both reproduce today's `applySaveScriptRetarget`: + * Two retentions are deliberate: * * - re-arming the SAME explicit path keeps an existing grant; a bare re-arm is not a withdrawal; - * - moving from `default` to an explicit path keeps it. Today's retarget check requires a - * previously PERSISTED path, so `open --save-script --force` followed by - * `close --save-script=out.ad` is not treated as a retarget and the grant survives. + * - moving from `default` to an explicit path keeps it, so `open --save-script --force` + * followed by `close --save-script=out.ad` is not treated as a retarget and the grant + * survives. * * That second case is arguably a #1258 gap — the caller authorized overwriting an unnamed * default, not `out.ad`. It is preserved here so the migration changes no behavior; tightening @@ -101,17 +98,19 @@ export const NO_SCRIPT_PUBLICATION: SessionScriptPublicationState = { kind: 'non */ export function resolveScriptTarget( previous: SessionScriptTarget | undefined, - requested: Readonly<{ path?: string; force: boolean }>, + requested: Readonly<{ path?: string | undefined; force: boolean }>, ): SessionScriptTarget { + if (requested.path === undefined) { + // A bare re-arm is not a retarget: the previous target — explicit path included — survives + // untouched, gaining a live force grant if one accompanies the re-arm. This is what keeps a + // per-step repair armer or a repeated `--save-script` from wiping the materialized healed + // sibling back to the daemon default. + const base = previous ?? { kind: 'default', force: false }; + return requested.force ? { ...base, force: true } : base; + } const retainsAuthorization = - previous?.force === true && - (previous.kind === 'default' || - previous.path === requested.path || - requested.path === undefined); - const force = requested.force || retainsAuthorization; - return requested.path === undefined - ? { kind: 'default', force } - : { kind: 'explicit', path: requested.path, force }; + previous?.force === true && (previous.kind === 'default' || previous.path === requested.path); + return { kind: 'explicit', path: requested.path, force: requested.force || retainsAuthorization }; } /** @@ -152,3 +151,128 @@ export function isScriptPublished(state: SessionScriptPublicationState): boolean if (state.kind === 'repair') return state.status === 'committed'; return state.kind === 'authoring' && state.status === 'published'; } + +/** ADR 0016: `open --save-script` on a fresh session arms ordinary authoring. */ +export function armAuthoring( + requested: Readonly<{ path?: string | undefined; force: boolean }>, +): SessionScriptPublicationState { + return { kind: 'authoring', status: 'armed', target: resolveScriptTarget(undefined, requested) }; +} + +/** + * ADR 0016: a second successful `open` on an armed authoring session terminates the recording — + * the journey no longer starts where the script says it does. Any other state passes through. + */ +export function abortAuthoring( + state: SessionScriptPublicationState, +): SessionScriptPublicationState { + if (state.kind !== 'authoring' || state.status !== 'armed') return state; + return { ...state, status: 'aborted' }; +} + +/** Active publication succeeded; the written path becomes the explicit target. */ +export function markAuthoringPublished( + state: SessionScriptPublicationState, + writtenPath: string, +): SessionScriptPublicationState { + if (state.kind !== 'authoring') return state; + return { + kind: 'authoring', + status: 'published', + target: { kind: 'explicit', path: writtenPath, force: state.target.force }, + }; +} + +/** + * ADR 0012 decision 6, R1/R6: `replay --save-script` arms (or re-arms, per step) the repair + * transaction. The boundary and source path stamp once — later arms of the same run retarget and + * re-authorize but never move the watermark or forget the original input. + */ +export function armRepair( + state: SessionScriptPublicationState, + params: Readonly<{ + requested: Readonly<{ path?: string | undefined; force: boolean }>; + boundary: number; + sourcePath?: string | undefined; + }>, +): SessionScriptPublicationState { + const previous = state.kind === 'repair' ? state : undefined; + const sourcePath = previous?.sourcePath ?? params.sourcePath; + return { + kind: 'repair', + status: previous?.status ?? 'armed', + target: resolveScriptTarget(previous?.target, params.requested), + boundary: previous?.boundary ?? params.boundary, + ...(sourcePath !== undefined ? { sourcePath } : {}), + ...(previous?.closeReceipt !== undefined ? { closeReceipt: previous.closeReceipt } : {}), + }; +} + +/** The plan reached its final executable step with no outstanding divergence (C2). */ +export function markRepairComplete( + state: SessionScriptPublicationState, +): SessionScriptPublicationState { + if (state.kind !== 'repair' || state.status !== 'armed') return state; + return { ...state, status: 'complete' }; +} + +/** + * The platform close for `operationIdentity` succeeded. `close-succeeded` is only reachable from + * completeness; an incomplete repair's close still records the receipt so a retry after a failed + * commit does not re-dispatch, which is the receipt's whole job. + */ +export function recordRepairCloseSucceeded( + state: SessionScriptPublicationState, + operationIdentity: string, +): SessionScriptPublicationState { + if (state.kind !== 'repair') return state; + return { + ...state, + status: state.status === 'complete' ? 'close-succeeded' : state.status, + closeReceipt: operationIdentity, + }; +} + +/** Terminal success: the healed script is on disk. Receipt cleanup is part of terminality. */ +export function commitRepair(state: SessionScriptPublicationState): SessionScriptPublicationState { + if (state.kind !== 'repair') return state; + const { closeReceipt: _closeReceipt, ...rest } = state; + return { ...rest, status: 'committed' }; +} + +/** Terminal failure: the transaction never completed, so publication is refused forever. */ +export function abortRepair(state: SessionScriptPublicationState): SessionScriptPublicationState { + if (state.kind !== 'repair') return state; + const { closeReceipt: _closeReceipt, ...rest } = state; + return { ...rest, status: 'aborted' }; +} + +/** The repair watermark (R6), or `undefined` when the session is not under repair. */ +export function repairBoundary(state: SessionScriptPublicationState): number | undefined { + return state.kind === 'repair' ? state.boundary : undefined; +} + +/** The original `replay` input path stashed for reap tombstones (C5a). */ +export function repairSourcePath(state: SessionScriptPublicationState): string | undefined { + return state.kind === 'repair' ? state.sourcePath : undefined; +} + +/** + * An uncommitted repair transaction — the state that blocks nothing (idle-reap proceeds) but + * demands a tombstone when torn down. Aborted repairs still qualify: they hold a boundary and + * never committed, exactly the sessions R7's expiry tombstone exists for. + */ +export function isUncommittedRepair(state: SessionScriptPublicationState): boolean { + return state.kind === 'repair' && state.status !== 'committed'; +} + +/** The explicit output path, or `undefined` for none/default (writer resolves its own). */ +export function scriptTargetPath(state: SessionScriptPublicationState): string | undefined { + const target = scriptPublicationTarget(state); + return target?.kind === 'explicit' ? target.path : undefined; +} + +/** The persisted per-target overwrite authorization (#1258). */ +export function scriptTargetForce(state: SessionScriptPublicationState): boolean { + return scriptPublicationTarget(state)?.force === true; +} diff --git a/src/daemon/session-script-writer.ts b/src/daemon/session-script-writer.ts index 72dcee630..dceb0bcab 100644 --- a/src/daemon/session-script-writer.ts +++ b/src/daemon/session-script-writer.ts @@ -18,6 +18,13 @@ import { stripRecordedRefGeneration, } from '../replay/script-utils.ts'; import type { SessionAction, SessionState } from './types.ts'; +import { + NO_SCRIPT_PUBLICATION, + commitRepair, + isRepairCommittable, + scriptTargetPath, +} from './session-script-publication-state.ts'; +import { isRepairArmedSession, repairSessionBoundary } from './session-replay-transaction.ts'; import { assertActivePublicationPortability, toActivePublicationFailure, @@ -51,8 +58,8 @@ export type SessionScriptWriteOptions = { * only recognizes the `target-v1` prefix), so it never participates in the * target-annotation binding rule. Written only when a repair-armed session's * write reaches this point at all, since `write()` already gated that on the - * transaction being COMPLETE (`saveScriptComplete`) — so every write carrying - * it IS a complete, committed transaction. + * transaction being COMPLETE — so every write carrying it IS a complete, + * committed transaction. */ export const HEAL_COMPLETE_SENTINEL = '# agent-device:heal-complete'; @@ -66,7 +73,7 @@ export const HEAL_COMPLETE_SENTINEL = '# agent-device:heal-complete'; * after a divergence but before the plan finishes from committing a PREFIX; * every non-completion teardown (divergence-only exit, daemon shutdown, * idle-reap) lands here too. - * Ordinary (non-repair) recording is never blocked here (no `saveScriptBoundary`) — + * Ordinary (non-repair) recording is never blocked here (no repair variant) — * this gate only decides whether `write()` attempts a publish AT ALL. It says * nothing about what happens once it does: `publishHealedScriptAtomically`'s * refuse-on-exist applies to that attempted publish uniformly, repair-armed or @@ -74,9 +81,10 @@ export const HEAL_COMPLETE_SENTINEL = '# agent-device:heal-complete'; * but it can still be refused if the target already exists. */ function isRepairArmedWriteBlocked(session: SessionState): boolean { - if (session.saveScriptBoundary === undefined) return false; - if (session.saveScriptCommitted) return true; - return !session.saveScriptComplete; + const state = session.scriptPublication ?? NO_SCRIPT_PUBLICATION; + if (state.kind !== 'repair') return false; + if (state.status === 'committed') return true; + return !isRepairCommittable(state); } export class SessionScriptWriter { @@ -87,7 +95,7 @@ export class SessionScriptWriter { } write(session: SessionState, options?: SessionScriptWriteOptions): SessionScriptWriteResult { - const repairArmed = session.saveScriptBoundary !== undefined; + const repairArmed = isRepairArmedSession(session); const activePublication = options?.publication === 'active'; let scriptPath: string | undefined; try { @@ -101,16 +109,20 @@ export class SessionScriptWriter { const scriptDir = path.dirname(scriptPath); if (!fs.existsSync(scriptDir)) fs.mkdirSync(scriptDir, { recursive: true }); // #1258: `options.force` is the caller's already-merged decision - // (typically `req.flags?.force || session.saveScriptForce` — see - // `session-close.ts`/`session-store.ts`), not read from `session` - // directly here, so this stays a pure formatting+publish step. + // (`effectiveWriteForce` — a live flag or the per-target grant), not + // read from `session` directly here, so this stays a pure + // formatting+publish step. publishHealedScriptAtomically({ scriptPath, script: prepared.script, force: options?.force, }); // COMMITTED: idempotent guard above + teardown's abort/tombstone routing. - if (repairArmed) session.saveScriptCommitted = true; + if (repairArmed) { + session.scriptPublication = commitRepair( + session.scriptPublication ?? NO_SCRIPT_PUBLICATION, + ); + } return { written: true, path: scriptPath, actionCount: prepared.actionCount }; } catch (error) { return handleSessionScriptWriteFailure({ @@ -124,8 +136,9 @@ export class SessionScriptWriter { } private resolveScriptPath(session: SessionState): string { - if (session.saveScriptPath) { - return expandSessionPath(session.saveScriptPath); + const targetPath = scriptTargetPath(session.scriptPublication ?? NO_SCRIPT_PUBLICATION); + if (targetPath) { + return expandSessionPath(targetPath); } const safeName = safeSessionName(session.name); const timestamp = new Date(session.createdAt).toISOString().replace(/[:.]/g, '-'); @@ -280,13 +293,14 @@ function buildOptimizedActions( session: SessionState, options: { strictPortableRefs?: boolean } = {}, ): SessionAction[] { - // ADR 0012 decision 6, R6: a repair-armed session (`saveScriptBoundary` set - // by `replay --save-script`) serializes only the actions from that - // watermark onward — the repair run's own execution path — never the - // whole session history. Absent a boundary (ordinary `open`/`close - // --save-script`), this slices from 0: unchanged, full-history behavior. - const repairArmed = session.saveScriptBoundary !== undefined; - const relevantActions = session.actions.slice(session.saveScriptBoundary ?? 0); + // ADR 0012 decision 6, R6: a repair-armed session (armed by `replay + // --save-script`) serializes only the actions from its boundary watermark + // onward — the repair run's own execution path — never the whole session + // history. Absent a boundary (ordinary `open`/`close --save-script`), this + // slices from 0: unchanged, full-history behavior. + const boundary = repairSessionBoundary(session); + const repairArmed = boundary !== undefined; + const relevantActions = session.actions.slice(boundary ?? 0); const optimized: SessionAction[] = []; for (const action of relevantActions) { if (action.command === 'snapshot') continue; diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index e1541ba5c..71bb70f66 100644 --- a/src/daemon/session-store.ts +++ b/src/daemon/session-store.ts @@ -4,6 +4,12 @@ import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { SessionRuntimeHints, SessionState } from './types.ts'; import { recordActionEntry, type RecordActionEntry } from './session-action-recorder.ts'; import { expandSessionPath, safeSessionName } from './session-paths.ts'; +import { NO_SCRIPT_PUBLICATION, isRepairCommittable } from './session-script-publication-state.ts'; +import { effectiveWriteForce } from './session-script-publication-capability.ts'; +import { + isUncommittedRepairSession, + repairSessionSourcePath, +} from './session-replay-transaction.ts'; import { SessionScriptWriter, type SessionScriptWriteOptions, @@ -173,8 +179,10 @@ export class SessionStore { // #1258: no live request here (idle-reap/daemon-shutdown teardown), so // the only source of `force` is whatever was persisted on the session at // arm time. - const result = this.writeSessionLog(session, { force: session.saveScriptForce }); - if (session.saveScriptBoundary !== undefined && session.saveScriptCommitted !== true) { + const result = this.writeSessionLog(session, { + force: effectiveWriteForce(session, undefined), + }); + if (isUncommittedRepairSession(session)) { if (!result.written && result.error) { this.writeRepairTombstone(session, REPAIR_TOMBSTONE_TTL_MS, { code: String(result.error.code), @@ -195,9 +203,8 @@ export class SessionStore { * nothing to make self-contained. */ private recordRepairFinalizeCloseIfCommitting(session: SessionState): void { - if (session.saveScriptBoundary === undefined) return; - if (session.saveScriptComplete !== true) return; - if (session.saveScriptCommitted === true) return; + const state = session.scriptPublication ?? NO_SCRIPT_PUBLICATION; + if (!isRepairCommittable(state)) return; this.recordAction(session, { command: 'close', positionals: [], @@ -229,7 +236,9 @@ export class SessionStore { owner: session.name, reapedAt: Date.now(), expiresAt: Date.now() + ttlMs, - ...(session.repairSourcePath ? { sourcePath: session.repairSourcePath } : {}), + ...(repairSessionSourcePath(session) + ? { sourcePath: repairSessionSourcePath(session) } + : {}), ...(commitFailure ? { commitFailure } : {}), }; fs.writeFileSync(this.repairTombstonePath(session.name), `${JSON.stringify(tombstone)}\n`); diff --git a/src/daemon/types.ts b/src/daemon/types.ts index d6bddd8e4..8027b2d71 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -26,6 +26,7 @@ import type { SnapshotDiagnosticsState } from '@agent-device/contracts/capture'; import type { DeviceLease } from '@agent-device/contracts/device'; import type { AudioProbeSource } from '@agent-device/contracts/platform'; import type { AndroidNativePerfSession } from '../platforms/android/perf.ts'; +import type { SessionScriptPublicationState } from './session-script-publication-state.ts'; import type { AppleXctracePerfCapture, AppleXctracePerfMode, @@ -367,63 +368,15 @@ export type SessionState = { /** Session was created by record start and should be released when recording stops. */ recordOnlySession?: boolean; recordSession?: boolean; - /** ADR 0016 ordinary open-to-destination authoring lifecycle. Repair state is separate. */ - scriptRecordingState?: 'armed' | 'aborted' | 'published'; - saveScriptPath?: string; /** - * #1258: `--force`/`--overwrite` captured at the moment `--save-script` was - * armed (`open --save-script --force`, `close --save-script --force`, or - * `replay --save-script --force`'s first arm) — persisted here, like - * `saveScriptPath`, so a LATER write that does not repeat the flag (a bare - * `close` finishing a session opened with `open --save-script --force`, a - * `--from` continuation leg, or an unattended auto-commit teardown with no - * live request at all) still honors the overwrite the caller opted into up - * front. The effective decision at any write site is `req.flags?.force || - * session.saveScriptForce`. - * - * Force is PER-TARGET, not a session-wide standing grant: it stays set while - * the target is unchanged, but re-arming a DIFFERENT `--save-script=` - * without a live `--force` CLEARS it (`applySaveScriptRetarget`), so a later - * retarget can never silently overwrite a file the caller never opted into. - * A live `--force` on the retarget re-grants it for the new target. - */ - saveScriptForce?: boolean; - /** - * ADR 0012 decision 6, R6: `session.actions.length` at the `replay - * --save-script` invocation that armed this session — the repair-run - * boundary. The healed `.ad` serializes only `session.actions` from this - * index onward, so a reused session's earlier, unrelated actions never - * leak into the healed script. - */ - saveScriptBoundary?: number; - /** - * ADR 0012 decision 6, R7 + commit semantics (C2): the repair TRANSACTION - * completion flag. `true` iff the last repair-armed replay run reached its - * final EXECUTABLE step with no outstanding divergence (the terminal source - * `close` is excluded — C4). Commit is gated on this, NOT merely on a - * `close`: `SessionScriptWriter.write` publishes a repair-armed session's - * healed `.ad` only when complete, so a `close`/`close --save-script` - * issued after a divergence but before the plan finishes discards a prefix - * instead of committing it. States: ARMED (`saveScriptBoundary` set, - * complete unset) -> COMPLETE (this true) -> COMMITTED (`saveScriptCommitted`). - */ - saveScriptComplete?: boolean; - /** - * ADR 0012 decision 6 (C2): set by the writer after a repair-armed session's - * healed `.ad` is atomically published. Makes re-publish idempotent (a second - * `writeSessionLog` no-ops) and lets teardown distinguish a COMMITTED session - * (nothing to tombstone) from an aborted/reaped one. - */ - saveScriptCommitted?: boolean; - /** Target identity of a successful repair close awaiting script commit. */ - repairPlatformCloseReceipt?: string; - /** - * ADR 0012 decision 6, R7 (C5a): the original replay input path of an armed - * repair, stashed so an idle-reap tombstone can hand the agent an actionable - * `replay --save-script` re-run command instead of a bare - * SESSION_NOT_FOUND. + * The tagged script-publication aggregate (#1478 P4a): ordinary authoring (ADR 0016), the + * ADR 0012 decision 6 repair transaction, and the shared output target with its per-target + * force authorization, in one state machine. `undefined` means `NO_SCRIPT_PUBLICATION` — + * mutate only through the daemon-private `ReplaySessionTransaction`/`SessionScriptPublication` + * projections; ordinary readers use the read helpers in + * `session-script-publication-state.ts`. */ - repairSourcePath?: string; + scriptPublication?: SessionScriptPublicationState; /** * ADR 0012 decision 6, R2/R3, extended per #1262: set whenever a * `record-and-heal` divergence's `resume` reports `allowed: true` — its diff --git a/test/integration/provider-scenarios/active-session-script-publication.test.ts b/test/integration/provider-scenarios/active-session-script-publication.test.ts index 87844d29a..8d11dd651 100644 --- a/test/integration/provider-scenarios/active-session-script-publication.test.ts +++ b/test/integration/provider-scenarios/active-session-script-publication.test.ts @@ -8,6 +8,14 @@ import { assertRpcError, assertRpcOk } from './assertions.ts'; import { androidSettingsXml, createAndroidSettingsWorld } from './android-world.ts'; import { withProviderScenarioResource } from './harness.ts'; +/** The authoring lifecycle status of the world's live session, or `undefined` outside authoring. */ +function authoringPublicationStatus(world: { + daemon: { session: () => { scriptPublication?: { kind: string; status?: string } } | undefined }; +}): string | undefined { + const publication = world.daemon.session()?.scriptPublication; + return publication?.kind === 'authoring' ? publication.status : undefined; +} + test('provider route publishes and replays an open-to-destination script with a live handoff', async () => { await withProviderScenarioResource(createAndroidSettingsWorld, async (world) => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-active-script-provider-')); @@ -25,7 +33,7 @@ test('provider route publishes and replays an open-to-destination script with a assert.equal(published.savedScript, scriptPath); assert.equal(published.session, 'default'); assert.equal(published.actionCount, 2); - assert.equal(world.daemon.session()?.scriptRecordingState, 'published'); + assert.equal(authoringPublicationStatus(world), 'published'); const liveSnapshot = await client.capture.snapshot({ interactiveOnly: true }); assert.ok(liveSnapshot.nodes.some((node) => node.label === 'Search')); @@ -132,7 +140,7 @@ test('a second successful open aborts publication and terminal save flags fail b saveScript: scriptPath, }); assertRpcError(rearm, 'INVALID_ARGS', /only arm a fresh session/); - assert.equal(world.daemon.session()?.scriptRecordingState, 'armed'); + assert.equal(authoringPublicationStatus(world), 'armed'); const second = await world.daemon.callCommand('open', ['settings'], { ...world.selection, @@ -140,7 +148,7 @@ test('a second successful open aborts publication and terminal save flags fail b }); const secondData = assertRpcOk<{ warnings?: string[] }>(second); assert.match(String(secondData.warnings), /publication was aborted/i); - assert.equal(world.daemon.session()?.scriptRecordingState, 'aborted'); + assert.equal(authoringPublicationStatus(world), 'aborted'); const publication = await world.daemon.callCommand('session_save_script', [scriptPath]); assertRpcError(publication, 'COMMAND_FAILED', /aborted by a second successful open/); diff --git a/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts b/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts index 80a55c1e3..433bee5c7 100644 --- a/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts +++ b/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts @@ -69,9 +69,10 @@ test('Provider-backed integration: a repair-armed segment excludes diagnostic re // R7/C1: the divergence reports the repair transaction as held — the exact // signal the #1271 stage-1 guidance clause is gated on. assert.equal(divergenceReport.resume.repairSessionHeld, true); - // The session is repair-armed: `saveScriptBoundary` is the boundary the + // The session is repair-armed: the repair variant's boundary is what the // exclusion keys off (an ordinary `open --save-script` never sets it). - const armedBoundary = daemon.session()?.saveScriptBoundary; + const publication = daemon.session()?.scriptPublication; + const armedBoundary = publication?.kind === 'repair' ? publication.boundary : undefined; assert.equal(typeof armedBoundary, 'number'); // --- The exclusion contrast: the SAME observation-only command, against the From e6508ecd1d56770b5e4d6081d8f41d5674c65543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 31 Jul 2026 21:27:38 +0200 Subject: [PATCH 3/3] refactor(daemon): satisfy the Fallow gate by extracting decisions, not 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 --- src/daemon/handlers/session-replay-runtime.ts | 46 +++-- .../handlers/session-script-publication.ts | 48 +++-- .../session-script-publication-state.ts | 2 +- .../replay-repair-record-exclusion.test.ts | 189 ++++++++++-------- 4 files changed, 168 insertions(+), 117 deletions(-) diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts index 9f3a7e095..10078c6a9 100644 --- a/src/daemon/handlers/session-replay-runtime.ts +++ b/src/daemon/handlers/session-replay-runtime.ts @@ -62,6 +62,7 @@ import { NO_SCRIPT_PUBLICATION, scriptTargetForce, scriptTargetPath, + type SessionScriptPublicationState, } from '../session-script-publication-state.ts'; import { armRepairStep, @@ -649,6 +650,32 @@ function validateReplaySessionEntry(params: { return undefined; } +/** + * Rejects arming a repair over an ordinary authoring recording (R2's disjointness) and runs the + * arm-time EEXIST preflight against the target this request resolves to. + */ +function rejectSaveScriptArming(params: { + saveScript: boolean | string | undefined; + force: boolean | undefined; + preRunState: SessionScriptPublicationState; + sourcePath: string; +}): DaemonResponse | undefined { + const { saveScript, force, preRunState, sourcePath } = params; + if (saveScript && preRunState.kind === 'authoring') { + return errorResponse( + 'INVALID_ARGS', + `replay --save-script cannot re-arm an ordinary recording in terminal/active state ${preRunState.status}. Close this session and use a fresh one for repair authoring.`, + ); + } + return preflightSaveScriptTarget({ + saveScript, + liveForce: force, + persistedForce: scriptTargetForce(preRunState) || undefined, + sourcePath, + existingSaveScriptPath: scriptTargetPath(preRunState), + }); +} + function prepareSaveScriptSession(params: { req: DaemonRequest; sessionStore: SessionStore; @@ -658,24 +685,13 @@ function prepareSaveScriptSession(params: { const { req, sessionStore, sessionName, sourcePath } = params; const preRunSession = sessionStore.get(sessionName); const { saveScript, force } = req.flags ?? {}; - const preRunState = preRunSession?.scriptPublication ?? NO_SCRIPT_PUBLICATION; - if (saveScript && preRunState.kind === 'authoring') { - return { - ok: false, - response: errorResponse( - 'INVALID_ARGS', - `replay --save-script cannot re-arm an ordinary recording in terminal/active state ${preRunState.status}. Close this session and use a fresh one for repair authoring.`, - ), - }; - } - const saveScriptPreflight = preflightSaveScriptTarget({ + const rejection = rejectSaveScriptArming({ saveScript, - liveForce: force, - persistedForce: scriptTargetForce(preRunState) || undefined, + force, + preRunState: preRunSession?.scriptPublication ?? NO_SCRIPT_PUBLICATION, sourcePath, - existingSaveScriptPath: scriptTargetPath(preRunState), }); - if (saveScriptPreflight) return { ok: false, response: saveScriptPreflight }; + if (rejection) return { ok: false, response: rejection }; if (preRunSession && repairSessionBoundary(preRunSession) !== undefined) { resetRepairCompletionForRerun(preRunSession); diff --git a/src/daemon/handlers/session-script-publication.ts b/src/daemon/handlers/session-script-publication.ts index 2e7c8ff44..2cc768038 100644 --- a/src/daemon/handlers/session-script-publication.ts +++ b/src/daemon/handlers/session-script-publication.ts @@ -66,37 +66,47 @@ export function handleSessionScriptPublication(params: { }; } -function validatePublicationEligibility(session: SessionState): AppError | undefined { - if (isRepairArmedSession(session)) { - return new AppError( +type PublicationIneligibility = 'repair' | 'aborted' | 'published' | 'not-armed'; + +/** Why this session cannot publish its active recording, or `undefined` when it can. */ +function publicationIneligibility(session: SessionState): PublicationIneligibility | undefined { + if (isRepairArmedSession(session)) return 'repair'; + const state = session.scriptPublication; + if (state?.kind !== 'authoring') return 'not-armed'; + if (state.status === 'armed') return session.recordSession ? undefined : 'not-armed'; + return state.status; +} + +const PUBLICATION_INELIGIBILITY_ERRORS: Record AppError> = { + repair: () => + new AppError( 'COMMAND_FAILED', 'This session has an active .ad repair transaction and cannot use ordinary active-session publication.', { hint: 'Finish or abort the repair through replay --from and its existing close/teardown protocol.', }, - ); - } - const state = session.scriptPublication; - if (state?.kind === 'authoring' && state.status === 'aborted') { - return new AppError( + ), + aborted: () => + new AppError( 'COMMAND_FAILED', 'This script recording was aborted by a second successful open and cannot be published.', { hint: 'Close this session and start a fresh one with open --save-script[=].' }, - ); - } - if (state?.kind === 'authoring' && state.status === 'published') { - return new AppError('COMMAND_FAILED', 'This script recording has already been published.', { + ), + published: () => + new AppError('COMMAND_FAILED', 'This script recording has already been published.', { hint: 'Continue using the live session, or close it and start a fresh authoring session.', - }); - } - if (state?.kind !== 'authoring' || state.status !== 'armed' || !session.recordSession) { - return new AppError( + }), + 'not-armed': () => + new AppError( 'COMMAND_FAILED', 'Script recording was not armed before this journey began; session history cannot be published without recording-time target evidence.', { hint: 'Close this session and start a fresh one with open --save-script[=].' }, - ); - } - return undefined; + ), +}; + +function validatePublicationEligibility(session: SessionState): AppError | undefined { + const reason = publicationIneligibility(session); + return reason ? PUBLICATION_INELIGIBILITY_ERRORS[reason]() : undefined; } function failure(error: AppError): Extract { diff --git a/src/daemon/session-script-publication-state.ts b/src/daemon/session-script-publication-state.ts index 0514faa90..cc8ecad5d 100644 --- a/src/daemon/session-script-publication-state.ts +++ b/src/daemon/session-script-publication-state.ts @@ -128,7 +128,7 @@ export function demoteRepairToArmed( } /** The target a state publishes to, or `undefined` when it publishes nothing. */ -export function scriptPublicationTarget( +function scriptPublicationTarget( state: SessionScriptPublicationState, ): SessionScriptTarget | undefined { return state.kind === 'none' ? undefined : state.target; diff --git a/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts b/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts index 433bee5c7..2deb779d6 100644 --- a/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts +++ b/test/integration/provider-scenarios/replay-repair-record-exclusion.test.ts @@ -53,69 +53,14 @@ test('Provider-backed integration: a repair-armed segment excludes diagnostic re ), ); - // --- Arm the repair transaction (R1): `replay --save-script` before step 1. --- - const divergenceError = await client.replay - .run({ path: repairPath, saveScript: true, ...selection }) - .then( - () => null, - (error: unknown) => error as { code?: string; details?: Record }, - ); - assert.ok(divergenceError, 'expected the armed replay to diverge on the missing selector'); - assert.equal(divergenceError.code, 'REPLAY_DIVERGENCE'); - const divergenceReport = divergenceError.details?.divergence as { - resume: { allowed: boolean; from: number; planDigest: string; repairSessionHeld?: boolean }; - }; - assert.equal(divergenceReport.resume.allowed, true); - // R7/C1: the divergence reports the repair transaction as held — the exact - // signal the #1271 stage-1 guidance clause is gated on. - assert.equal(divergenceReport.resume.repairSessionHeld, true); - // The session is repair-armed: the repair variant's boundary is what the - // exclusion keys off (an ordinary `open --save-script` never sets it). - const publication = daemon.session()?.scriptPublication; - const armedBoundary = publication?.kind === 'repair' ? publication.boundary : undefined; - assert.equal(typeof armedBoundary, 'number'); - - // --- The exclusion contrast: the SAME observation-only command, against the - // same selector, run twice inside the repair segment — differing only in - // `--record`. --- - const actionsBeforeReads = daemon.session()?.actions.length ?? 0; - - // (a) A diagnostic read used to LOCATE the target: excluded by default, - // with no `--no-record` needed (the #1271 stage-1 foot-gun). - const diagnosticRead = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { - ...selection, - }); - assertRpcOk(diagnosticRead); - assert.equal( - daemon.session()?.actions.length, - actionsBeforeReads, - 'a diagnostic read inside a repair segment must not be recorded', - ); - - // (b) The corrective read (the wave-3 E3 shape: the diverged step is itself - // a read), forced into the heal with `--record`. - const correctiveRead = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { - ...selection, - record: true, - }); - assertRpcOk(correctiveRead); - const armedActions = daemon.session()?.actions ?? []; - assert.equal(armedActions.length, actionsBeforeReads + 1); - assert.equal(armedActions.at(-1)?.command, 'get'); - - // (c) `--record` and `--no-record` are opposite intents for one action. - const conflictingFlags = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { - ...selection, - record: true, - noRecord: true, - }); - assertRpcError(conflictingFlags, 'INVALID_ARGS', /--record and --no-record are mutually/); + const resume = await armRepairAndAssertHeld({ client, daemon, repairPath, selection }); + await assertRepairSegmentReadExclusion({ daemon, selection }); // --- Resume past the diverged step, completing the plan (transaction COMPLETE). --- const resumed = await client.replay.run({ path: repairPath, - resumeFrom: divergenceReport.resume.from + 1, - resumePlanDigest: divergenceReport.resume.planDigest, + resumeFrom: resume.from + 1, + resumePlanDigest: resume.planDigest, ...selection, }); assert.equal(resumed.replayed, 1); @@ -125,29 +70,7 @@ test('Provider-backed integration: a repair-armed segment excludes diagnostic re assertRpcOk(close); const healedPath = path.join(tempRoot, 'repair.healed.ad'); assert.equal(fs.existsSync(healedPath), true, 'the completed repair must publish a healed .ad'); - const healedScript = fs.readFileSync(healedPath, 'utf8'); - - // The heal carries EXACTLY ONE `get` line: the `--record`ed corrective - // read. The identical diagnostic read that ran first is absent — the whole - // point of the amendment, and the reason a blanket read-exclusion would be - // unsafe (it would drop this line too, silently). - const getLines = healedLines(healedScript, 'get'); - assert.equal(getLines.length, 1, `expected exactly one recorded get, got:\n${healedScript}`); - assert.match(getLines[0]!, /com\.android\.settings:id\/search/); - - // PROVENANCE (the rule the exclusion actually keys off): the AUTHORED - // `is visible` plan step must survive into its own healed script. It is - // the same command class as the excluded diagnostic read above and carries - // no `--record`, so a command-class exclusion would silently drop it — - // leaving a healed flow that quietly stopped asserting what the original - // asserted. Users must never have to annotate their own `.ad` steps. - const isLines = healedLines(healedScript, 'is'); - assert.equal( - isLines.length, - 1, - `the authored 'is visible' step must survive the heal, got:\n${healedScript}`, - ); - assert.match(isLines[0]!, /com\.android\.settings:id\/search/); + assertHealedScriptContents(fs.readFileSync(healedPath, 'utf8')); } finally { fs.rmSync(tempRoot, { recursive: true, force: true }); await daemon.close(); @@ -156,6 +79,108 @@ test('Provider-backed integration: a repair-armed segment excludes diagnostic re // budget): this drives a full arm -> diverge -> resume -> commit chain. }, 15_000); +type ScenarioHarness = Awaited>; +type ScenarioSelection = { platform: 'android'; serial: string }; + +/** Arms the repair (R1), asserts the divergence holds the session (R7/C1), and returns its resume record. */ +async function armRepairAndAssertHeld(params: { + client: ReturnType; + daemon: ScenarioHarness; + repairPath: string; + selection: ScenarioSelection; +}): Promise<{ from: number; planDigest: string }> { + const { client, daemon, repairPath, selection } = params; + const divergenceError = await client.replay + .run({ path: repairPath, saveScript: true, ...selection }) + .then( + () => null, + (error: unknown) => error as { code?: string; details?: Record }, + ); + assert.ok(divergenceError, 'expected the armed replay to diverge on the missing selector'); + assert.equal(divergenceError.code, 'REPLAY_DIVERGENCE'); + const divergenceReport = divergenceError.details?.divergence as { + resume: { allowed: boolean; from: number; planDigest: string; repairSessionHeld?: boolean }; + }; + assert.equal(divergenceReport.resume.allowed, true); + // R7/C1: the divergence reports the repair transaction as held — the exact + // signal the #1271 stage-1 guidance clause is gated on. + assert.equal(divergenceReport.resume.repairSessionHeld, true); + // The session is repair-armed: the repair variant's boundary is what the + // exclusion keys off (an ordinary `open --save-script` never sets it). + const publication = daemon.session()?.scriptPublication; + const armedBoundary = publication?.kind === 'repair' ? publication.boundary : undefined; + assert.equal(typeof armedBoundary, 'number'); + return divergenceReport.resume; +} + +/** + * The exclusion contrast: the SAME observation-only command, against the same + * selector, run twice inside the repair segment — differing only in `--record`. + */ +async function assertRepairSegmentReadExclusion(params: { + daemon: ScenarioHarness; + selection: ScenarioSelection; +}): Promise { + const { daemon, selection } = params; + const actionsBeforeReads = daemon.session()?.actions.length ?? 0; + + // (a) A diagnostic read used to LOCATE the target: excluded by default, + // with no `--no-record` needed (the #1271 stage-1 foot-gun). + const diagnosticRead = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { + ...selection, + }); + assertRpcOk(diagnosticRead); + assert.equal( + daemon.session()?.actions.length, + actionsBeforeReads, + 'a diagnostic read inside a repair segment must not be recorded', + ); + + // (b) The corrective read (the wave-3 E3 shape: the diverged step is itself + // a read), forced into the heal with `--record`. + const correctiveRead = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { + ...selection, + record: true, + }); + assertRpcOk(correctiveRead); + const armedActions = daemon.session()?.actions ?? []; + assert.equal(armedActions.length, actionsBeforeReads + 1); + assert.equal(armedActions.at(-1)?.command, 'get'); + + // (c) `--record` and `--no-record` are opposite intents for one action. + const conflictingFlags = await daemon.callCommand('get', ['text', SEARCH_SELECTOR], { + ...selection, + record: true, + noRecord: true, + }); + assertRpcError(conflictingFlags, 'INVALID_ARGS', /--record and --no-record are mutually/); +} + +/** What must (and must not) have landed in the healed script. */ +function assertHealedScriptContents(healedScript: string): void { + // The heal carries EXACTLY ONE `get` line: the `--record`ed corrective + // read. The identical diagnostic read that ran first is absent — the whole + // point of the amendment, and the reason a blanket read-exclusion would be + // unsafe (it would drop this line too, silently). + const getLines = healedLines(healedScript, 'get'); + assert.equal(getLines.length, 1, `expected exactly one recorded get, got:\n${healedScript}`); + assert.match(getLines[0]!, /com\.android\.settings:id\/search/); + + // PROVENANCE (the rule the exclusion actually keys off): the AUTHORED + // `is visible` plan step must survive into its own healed script. It is + // the same command class as the excluded diagnostic read above and carries + // no `--record`, so a command-class exclusion would silently drop it — + // leaving a healed flow that quietly stopped asserting what the original + // asserted. Users must never have to annotate their own `.ad` steps. + const isLines = healedLines(healedScript, 'is'); + assert.equal( + isLines.length, + 1, + `the authored 'is visible' step must survive the heal, got:\n${healedScript}`, + ); + assert.match(isLines[0]!, /com\.android\.settings:id\/search/); +} + /** Recorded action lines for one command, ignoring the context header, `target-v1` annotations, and the sentinel. */ function healedLines(script: string, command: string): string[] { return script