diff --git a/CHANGELOG.md b/CHANGELOG.md index 515ba228..65f1e7a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes follow Semantic Versioning. - Improved `$zcode:setup` guidance when the ZCode CLI has no model provider configured, including the distinction between Desktop and CLI settings and API-key providers that do not require OAuth. - Added foreground activity output, a 20-second heartbeat, and durable status previews for long-running ZCode work. - Added bounded foreground `SIGINT` and `SIGTERM` handling: the plugin cancels before session creation or sends `session/stop` only to the exact persisted ZCode session, while background jobs continue until completion or explicit cancellation with `$zcode:cancel`. +- Added safe orphan settlement through best-effort `SessionEnd` handling and a reservation-time crash fallback, while preserving owner-only access and retaining the writable guard when liveness or remote-stop acknowledgement is uncertain. - Kept the package version at `0.1.0` for these Unreleased behavior changes. ## 0.1.0 - 2026-08-06 diff --git a/CONTEXT.md b/CONTEXT.md index ecfb9432..32f02f92 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -24,6 +24,14 @@ _Avoid_: Command execution, request A persisted record of a Companion Run, including its ownership, lifecycle state, progress, and stored result. _Avoid_: Process, thread +**Orphaned Job**: +A nonterminal Tracked Job whose exact worker-lifetime lease is no longer held, proving that its local executor has disappeared. +_Avoid_: Stale job, dead session + +**Lifecycle Maintenance Principal**: +Internal authority derived only from a validated Orphaned Job's original owner and used to settle that job without transferring user-visible ownership. +_Avoid_: Impersonated owner, adopted owner + **Review**: A read-only Companion Run that evaluates repository changes and returns findings without modifying the workspace. _Avoid_: Audit diff --git a/README.md b/README.md index 25292f78..e9175741 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,8 @@ To verify configuration, rerun `$zcode:setup`, then run `$zcode:rescue --fresh - Every run is reserved as a durable, owner-scoped job. Installed plugin state lives beneath `$CODEX_HOME/plugins/data/zcode-/workspaces//` with private permissions; prompts, results, session IDs, and logs are never written into the repository or plugin cache. `$zcode:status`, `$zcode:result`, and `$zcode:cancel` work across later turns in the same Codex session, while sibling sessions cannot adopt a job. +`SessionEnd` performs best-effort settlement of the ending session's writable Rescue. A claimed queued reservation remains unchanged while its worker lease is held. If the process exits before settlement completes, a later Rescue uses a reservation-time crash fallback and may settle a provably orphaned writable job; settlement does not transfer ownership, and only the original owner can access its result. During this reservation-time crash fallback, a held exact worker lease keeps the writable guard in place. An unacknowledged `session/stop` also keeps the writable guard in either settlement path. Other sessions can use `$zcode:status --all` only for redacted workspace inspection. + Foreground runs stream ZCode activity to the current terminal. If no new activity arrives, they emit a 20-second heartbeat so a long model or tool call remains visibly alive. The same safe activity is stored on the job; `$zcode:status ` shows its phase, last activity time, and recent progress previews. For example: ```text diff --git a/README.zh-CN.md b/README.zh-CN.md index 00c6f4cd..eb932a66 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -63,6 +63,8 @@ Setup 会把以下 schema 写入 `$CODEX_HOME/plugins/data/zcode-/w 每次运行都会先建立持久、带 owner 的 job。已安装插件的状态保存在 `$CODEX_HOME/plugins/data/zcode-/workspaces//`，使用私有权限；prompt、result、session ID 和日志都不会写进仓库或插件缓存。后续 turn 仍可使用 `$zcode:status`、`$zcode:result`、`$zcode:cancel`，但 sibling Codex session 无法接管任务。 +`SessionEnd` 会对结束会话的可写 Rescue 执行 best-effort 结算。已 claim 的 queued reservation 在其 worker lease 仍被持有时保持不变。若进程在结算完成前退出，后续 Rescue 会执行预留时的崩溃回退，并可结算可证明的孤儿可写 job；结算不会转移 ownership，仍只有原 owner 能读取其结果。在这个预留时的崩溃回退中，仍被持有的精确 worker lease 会保留 writable guard。未确认的 `session/stop` 在两条结算路径中也会保留 writable guard。其他会话只能通过 `$zcode:status --all` 查看脱敏后的 workspace 信息。 + 前台运行会把 ZCode 活动流式显示在当前终端。如果没有新活动，则每 20 秒输出一次心跳，让耗时较长的模型请求或工具调用仍然可见。同一份安全活动也会持久化到 job；`$zcode:status ` 会显示阶段、最后活动时间和近期进度预览。例如： ```text diff --git a/docs/adr/0011-settle-orphans-with-an-internal-maintenance-principal.md b/docs/adr/0011-settle-orphans-with-an-internal-maintenance-principal.md new file mode 100644 index 00000000..716a8fdb --- /dev/null +++ b/docs/adr/0011-settle-orphans-with-an-internal-maintenance-principal.md @@ -0,0 +1,15 @@ +--- +status: accepted +--- + +# Settle orphaned jobs with an internal maintenance principal + +When a writable Rescue's exact worker lease is free, internal lifecycle maintenance may derive the original broker owner ID only from that schema-validated durable job and use it to inspect, stop, and settle the job. This does not transfer ownership: public status, result, cancel, and resume selection remain bound to the original Codex session, and the maintenance path returns no old-job content to the session that triggered it. + +## Considered Options + +Same-owner-only recovery was rejected because workspace-global writable exclusion lets a dead owner permanently block every later session. SessionEnd-only settlement was rejected because crashes and force exits can skip the hook. Cross-session job adoption and force release were rejected because they would expose another session's work or permit two agents to mutate one workspace while the old remote session may still be active. + +## Consequences + +SessionEnd uses only an already healthy broker to read, stop, reread, and settle its exact active writable job before generic owner release; generic released mappings are not treated as stop acknowledgements. Writable reservation performs the crash fallback. Both paths use the existing per-job cancellation lock; claimed-job orphan detection additionally requires the exact worker lease to be free, while an unclaimed reservation retains the existing bounded worker-claim grace period. A remote stop that cannot be acknowledged retains the writable guard and an actionable bounded error rather than claiming a false terminal state. diff --git a/docs/superpowers/plans/2026-08-08-cross-session-orphan-rescue-lifecycle.md b/docs/superpowers/plans/2026-08-08-cross-session-orphan-rescue-lifecycle.md new file mode 100644 index 00000000..23994ef5 --- /dev/null +++ b/docs/superpowers/plans/2026-08-08-cross-session-orphan-rescue-lifecycle.md @@ -0,0 +1,491 @@ +# Cross-Session Orphan Rescue Lifecycle Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Safely settle a provably orphaned writable Rescue so another Codex session can reserve work without adopting the old job or claiming an unacknowledged remote stop. + +**Architecture:** Extend the recovery module with a policy-driven single-job settlement core and an internal workspace scavenger. Writable reservation retries once after scavenging. SessionEnd uses an existing-broker-only client and the same cancellation-lock settlement semantics before generic owner release. Public owner selection remains unchanged. + +**Tech Stack:** Node.js 22.13+ ESM, `node:test`, ZCode 0.16.1 JSON-lines protocol, native advisory locks, the existing managed broker and state store. + +--- + +## File Map + +- `scripts/lib/recovery.mjs`: shared settlement, cross-owner scavenging, post-stop reread, and SessionEnd settlement. +- `scripts/zcode-companion.mjs`: one writable reservation retry after scavenging. +- `scripts/lib/state.mjs`: honest writable-conflict remedy. +- `scripts/zcode-broker.mjs`: shared exact wire-profile identity calculation and bounded health probing. +- `scripts/lib/zcode-client.mjs`: exact-profile existing-broker-only client. +- `hooks/session-end-hook.mjs`: durable settlement before generic owner cleanup. +- `tests/recovery.test.mjs`, `tests/state.test.mjs`, `tests/integration/companion.test.mjs`, `tests/zcode-client.test.mjs`, `tests/hooks.test.mjs`: focused regression coverage. +- `tests/session-end.test.mjs`: isolated SessionEnd settlement matrix. +- `README.md`, `README.zh-CN.md`, `CHANGELOG.md`, `tests/release-contracts.test.mjs`: user-visible behavior. + +### Task 1: Cross-Owner Writable Scavenger + +**Files:** +- Modify: `scripts/lib/recovery.mjs` +- Modify: `tests/recovery.test.mjs` + +- [ ] **Step 1: Write failing tests for blocker selection, maintenance identity, and lease proof** + +Add these tests: + +- `cross-owner scavenging derives maintenance ownership from each durable writable blocker` +- `workspace scavenging never inspects a blocker whose exact worker lease is held` +- `workspace scavenging ignores read-only and terminal jobs` + +Use the wished-for API: + +```js +const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); +await scavengeWritableJobs({ + store, + dataRoot: fixture.dataRoot, + workspace: fixture.workspace, + reconcileOwnership: async ({ ownerId, ownedSessionIds }) => reconciled.push({ ownerId, ownedSessionIds }), + createClient: async (job, ownerId) => clients.get(job.id)(ownerId), +}); +``` + +For the held-lease case, call the scavenger while `withWorkerLease` holds the persisted lease. Assert zero ownership/client calls and unchanged state. The API must not accept caller owner or remote-session selectors. + +- [ ] **Step 2: Run the focused tests and verify RED** + +Run: + +```bash +node --test --test-name-pattern="cross-owner scavenging|workspace scavenging never|ignores read-only" tests/recovery.test.mjs +``` + +Expected: FAIL because `scavengeWritableJobs` is not exported. + +- [ ] **Step 3: Implement the selection and lock/lease shell** + +Add: + +```js +export async function scavengeWritableJobs(input) { + const jobs = (await input.store.listJobs(input.workspace)) + .filter((job) => job.command === 'rescue' && job.readOnly === false && !TERMINAL.has(job.status)); + const outcomes = []; + for (const job of jobs) { + outcomes.push(await settleSelectedJob({ + ...input, + selectedJobId: job.id, + expectedOwnerSessionId: job.ownerSessionId, + intent: 'scavenge', + }).catch(() => job)); + } + return outcomes; +} +``` + +`settleSelectedJob` acquires `withJobCancellationLock`, rereads the job, rechecks ID/owner/writable/nonterminal predicates, and probes a persisted lease with `withWorkerLease(..., timeoutMs: 0)`. `LOCK_TIMEOUT` returns the unchanged job without remote work. Never hold the workspace state lock around remote I/O. + +- [ ] **Step 4: Write failing policy tests** + +Add: + +- `workspace scavenging preserves an unclaimed reservation through claim grace and fails it after expiry` +- `workspace scavenging stops an active orphan and rereads completion before terminalizing` +- `workspace scavenging retains the writable guard when active stop is unacknowledged` +- `workspace scavenging maps paused running to failed but requires stop acknowledgement for cancelling` +- `workspace scavenging fails an orphan whose persisted remote session is missing` +- `terminal completion racing orphan settlement is never overwritten` + +Use an injected clock and assert unclaimed age from immutable `createdAt`. For active stop/reread, return `running`, acknowledge `stopSession`, then return `completed` with a current-turn result; assert `succeeded`, one stop, two reads, and a result artifact. For stop failure, assert `running` plus bounded `lastCancelError`. + +- [ ] **Step 5: Run the new tests and verify RED** + +```bash +node --test --test-name-pattern="claim grace|stops an active orphan|active stop is unacknowledged|paused running|remote session is missing|racing orphan" tests/recovery.test.mjs +``` + +Expected: FAIL against the current same-owner-only behavior. + +- [ ] **Step 6: Implement policy-driven settlement** + +Refactor the private recovery core to accept `intent: 'owner-recovery' | 'scavenge'`. Same-owner recovery continues to retain active `running` work. Scavenging stops active work. + +`stopThenSettle` must: + +1. call `session/stop` once; +2. retain `running + lastCancelError` if it is not acknowledged; +3. after acknowledgement, call `readSession` once; +4. publish a valid completed/idle current-turn result as `succeeded`; +5. otherwise publish `cancelled` when the durable pre-stop status was `cancelling`, or `failed` for scavenged `running`; +6. reread rather than overwrite a terminal/status-conflict winner. + +Map paused `running` to `failed`. Paused `cancelling` must repeat stop and require acknowledgement. For unclaimed queued jobs use: + +```js +const expired = now() - Date.parse(job.createdAt) >= LEGACY_QUEUED_STALE_MS; +``` + +- [ ] **Step 7: Verify GREEN and commit** + +```bash +node --test tests/recovery.test.mjs +git add scripts/lib/recovery.mjs tests/recovery.test.mjs +git commit -m "fix: scavenge orphaned writable rescues" +``` + +Expected: all recovery tests PASS without unhandled rejections. + +### Task 2: Reservation Retry and Owner Isolation + +**Files:** +- Modify: `scripts/zcode-companion.mjs` +- Modify: `scripts/lib/state.mjs` +- Modify: `tests/integration/companion.test.mjs` +- Modify: `tests/state.test.mjs` + +- [ ] **Step 1: Write failing integration and remedy tests** + +Add: + +- `a new owner scavenges one orphan blocker and retries writable reservation exactly once` +- `a live exact worker lease keeps a new owner blocked without remote inspection` +- `an unacknowledged orphan stop preserves WRITABLE_JOB_EXISTS with an honest remedy` +- `two new owners racing through scavenging admit at most one writable rescue` +- `the owner that triggers scavenging cannot status result cancel or resume the recovered job` +- `status --all reports a scavenged foreign job only through redacted other-owner metadata` +- `a recovered foreign completion remains readable only by its original owner` +- `writable exclusion remedy does not advertise a read-only rescue mode` + +Make the first admission test use a dead/free lease whose persisted remote session is absent: the old job must become `failed` and owner B must successfully reserve. The live-lease test runs a real held exact lease while owner B invokes Rescue; assert `WRITABLE_JOB_EXISTS`, unchanged owner-A state, and zero remote list/read/stop calls. + +The concurrency test invokes two distinct caller sessions concurrently after one orphan becomes recoverable. Assert one new reservation succeeds, one rejects with `WRITABLE_JOB_EXISTS`, and only one new writable job is active. Do not add a test-only admission lock. + +For isolation, owner B triggers recovery of owner A's completed orphan. Assert B gets `OWNED_JOB_NOT_FOUND` for status/result/cancel, cannot resume A's session, and sees only redacted `owned: false`/`owner: 'other'` metadata through `status --all`. Owner A must still read the recovered result. + +- [ ] **Step 2: Run tests and verify RED** + +```bash +node --test --test-name-pattern="scavenges one orphan|live exact worker lease|unacknowledged orphan|racing through scavenging|triggers scavenging|scavenged foreign|foreign completion" tests/integration/companion.test.mjs +node --test --test-name-pattern="does not advertise a read-only" tests/state.test.mjs +``` + +- [ ] **Step 3: Implement exactly one writable retry** + +Replace the direct public reservation with: + +```js +async function reservePublicJob(context, reservation) { + try { + return await context.store.reserveJob(reservation); + } catch (error) { + if (reservation.readOnly || !(error instanceof PluginError) || error.code !== 'WRITABLE_JOB_EXISTS') throw error; + await scavengeWritableJobs({ + store: context.store, + dataRoot: context.dataRoot, + workspace: context.cwd, + createClient: async (job, ownerId) => { + context.signal?.throwIfAborted(); + const launch = await discoverLaunch(context.env, context.dependencies); + return (context.dependencies?.createManagedZCodeClient ?? createManagedZCodeClient)({ + dataRoot: context.dataRoot, + workspace: context.cwd, + launch, + ownerId, + env: context.env, + ...managedWireOptionsForJob(job), + }); + }, + }); + return context.store.reserveJob(reservation); + } +} +``` + +Keep normal same-owner startup reconciliation. Do not return scavenged job payloads. Change the conflict remedy to exactly: + +```text +Retry later or inspect the redacted workspace list with $zcode:status --all. +``` + +- [ ] **Step 4: Verify GREEN and commit** + +```bash +node --test tests/state.test.mjs tests/integration/companion.test.mjs +git add scripts/zcode-companion.mjs scripts/lib/state.mjs tests/integration/companion.test.mjs tests/state.test.mjs +git commit -m "fix: retry rescue after orphan settlement" +``` + +### Task 3: Existing-Broker-Only Client Boundary + +**Files:** +- Modify: `scripts/zcode-broker.mjs` +- Modify: `scripts/lib/zcode-client.mjs` +- Modify: `tests/zcode-client.test.mjs` + +- [ ] **Step 1: Write failing exact-profile and no-spawn tests** + +Add: + +- `existing managed client connects to the exact healthy wire profile without ensuring a broker` +- `existing managed client returns null and never spawns when the broker is absent` +- `existing managed client does not fall back to a sibling wire profile` +- `existing managed client bounds an unhealthy broker probe` +- `existing managed client returns null when the broker dies between health and connect` + +The wished-for call intentionally has no `launch` or `env`: + +```js +const client = await createExistingManagedZCodeClient({ + dataRoot, + workspace, + ownerId, + requestTimeoutMs: 100, + maxFrameBytes: 16 * 1024 * 1024, + maxOutboundBytes: 16 * 1024 * 1024, +}); +``` + +Create default and hashed broker identities together and prove only the exact requested wire profile is contacted. Missing, wrong-profile, or unhealthy identity returns `null` within the request timeout. + +- [ ] **Step 2: Run and verify RED** + +```bash +node --test --test-name-pattern="existing managed client" tests/zcode-client.test.mjs +``` + +Expected: FAIL because the API is absent. + +- [ ] **Step 3: Share profile identity calculation** + +Export and use from both ensure and connect paths: + +```js +export function brokerIdentityNameForWireOptions(options = {}) { + const profile = options.maxFrameBytes === undefined + && options.maxOutboundBytes === undefined + && options.drainTimeoutMs === undefined + ? null + : createHash('sha256').update(JSON.stringify([ + options.maxFrameBytes ?? null, + options.maxOutboundBytes ?? null, + options.drainTimeoutMs ?? null, + ])).digest('hex').slice(0, 16); + return profile ? `identity-${profile}.json` : 'identity.json'; +} +``` + +Allow `probeBrokerHealth(record, requestTimeoutMs = 1_000)` with a validated bounded positive integer. + +- [ ] **Step 4: Implement the existing-only client** + +```js +export async function createExistingManagedZCodeClient(options) { + const storage = await resolveWorkspaceStorage(options); + const identityName = brokerIdentityNameForWireOptions(options); + const identity = await readHealthyBrokerIdentity(resolve(storage.directory, 'broker', identityName), { + healthProbe: (record) => probeBrokerHealth(record, options.requestTimeoutMs), + }); + if (!identity) return null; + try { + return await createZCodeClient({ + workspace: storage.workspacePath, + brokerEndpoint: identity.endpoint, + brokerToken: identity.brokerToken, + ownerId: options.ownerId, + requestTimeoutMs: options.requestTimeoutMs, + maxFrameBytes: options.maxFrameBytes, + maxOutboundBytes: options.maxOutboundBytes, + drainTimeoutMs: options.drainTimeoutMs, + }); + } catch { + return null; + } +} +``` + +Validate the same wire bounds as managed client construction. Never call `ensureZCodeBroker`, scan sibling profiles, reconcile ownership, or accept launch configuration. + +- [ ] **Step 5: Verify GREEN and commit** + +```bash +node --test tests/zcode-client.test.mjs tests/zcode-protocol.test.mjs +git add scripts/zcode-broker.mjs scripts/lib/zcode-client.mjs tests/zcode-client.test.mjs +git commit -m "feat: connect to existing zcode broker profiles" +``` + +### Task 4: Bounded SessionEnd Settlement + +**Files:** +- Modify: `scripts/lib/recovery.mjs` +- Modify: `hooks/session-end-hook.mjs` +- Create: `tests/session-end.test.mjs` +- Modify: `tests/hooks.test.mjs` + +- [ ] **Step 1: Write failing exact-owner settlement tests** + +Create `tests/session-end.test.mjs` with: + +- unclaimed queued becomes `cancelled` and a later claim fails; +- claimed queued with held lease stays queued, free lease becomes `cancelled`; +- completed first read becomes `succeeded` with an artifact and zero stops; +- active read, acknowledged stop, and noncompleted reread becomes `cancelled`; +- completion racing stop becomes `succeeded`; +- null existing client, lock contention, read timeout, and stop failure leave the job nonterminal; +- foreign-owner and read-only jobs remain untouched; +- a concurrent terminal executor outcome is never overwritten. + +Use: + +```js +await settleEndedOwnerWritableJob({ + store, + dataRoot, + workspace, + ownerSessionId: 'owner-a', + lockTimeoutMs: 0, + requestTimeoutMs: 250, + createClient: async (job, ownerId) => existingClients.get(job.id)?.(ownerId) ?? null, +}); +``` + +Assert the original owner and remote session come only from the reread job. + +- [ ] **Step 2: Run and verify RED** + +```bash +node --test tests/session-end.test.mjs +``` + +Expected: FAIL because `settleEndedOwnerWritableJob` is absent. + +- [ ] **Step 3: Implement bounded settlement by reusing the recovery core** + +Export `settleEndedOwnerWritableJob` from `scripts/lib/recovery.mjs`. Select only the ending owner's active writable Rescue. Acquire its cancellation lock with `lockTimeoutMs ?? 0`, reread, and recheck all predicates. + +Queued rules: + +```js +if (job.status === 'queued' && !isDigest(job.workerLeaseId)) return cancelQueued(job); +if (job.status === 'queued') { + try { + return await withWorkerLease({ ...leaseInput, timeoutMs: 0 }, () => cancelQueued(job)); + } catch (error) { + if (error instanceof PluginError && error.code === 'LOCK_TIMEOUT') return job; + throw error; + } +} +``` + +For `running`/`cancelling`, require an accepted session and a non-null existing-only client. Do not reconcile ownership. Under the same cancellation lock perform read → optional stop → reread. A valid completed/idle result becomes `succeeded`; acknowledged stop with no provable completion becomes `cancelled`; missing client or unacknowledged operation leaves the job nonterminal. Close the client in `finally`. + +Catch cancellation-lock `LOCK_TIMEOUT` as advisory unchanged output. Every protocol request uses the client-level `requestTimeoutMs`; never use the default 30-second lock timeout. + +- [ ] **Step 4: Verify settlement unit tests GREEN** + +```bash +node --test tests/session-end.test.mjs tests/recovery.test.mjs +``` + +- [ ] **Step 5: Write failing real-hook ordering tests** + +Add: + +- `SessionEnd settles its writable job before generic owner release and preserves siblings` +- `SessionEnd never starts a broker when exact existing settlement is unavailable` +- `generic releasedSessionIds never terminalize a durable job` +- `SessionEnd remains bounded when the existing broker or stop acknowledgement is unavailable` +- `a failed SessionEnd stop is later settled by reservation scavenging before owner B is admitted` + +The acknowledged case persists an owner-A running Rescue, creates a fake managed-broker session, runs the hook, then asserts owner A terminal and owner B unchanged. The unavailable case records zero ZCode spawns, keeps the job active, still cleans hook/identity state, and finishes within 2.5 seconds. The fallback sequence first makes SessionEnd stop fail, then releases the worker lease and changes the remote fixture to completed or missing; owner B's later Rescue must settle A through reservation scavenging and reserve successfully. + +- [ ] **Step 6: Wire the hook in settlement → release → cleanup order** + +```js +const ownerSessionId = input.session_id; +const ownerId = ownerIdForSession(ownerSessionId); +const store = createStateStore({ dataRoot }); +await settleEndedOwnerWritableJob({ + store, + dataRoot, + workspace: input.cwd, + ownerSessionId, + requestTimeoutMs: 250, + lockTimeoutMs: 0, + createClient: (job, derivedOwnerId) => createExistingManagedZCodeClient({ + dataRoot, + workspace: input.cwd, + ownerId: derivedOwnerId, + requestTimeoutMs: 250, + }), +}).catch(() => null); +await releaseManagedZCodeOwner({ dataRoot, workspace: input.cwd, ownerId, requestTimeoutMs: 500 }).catch(() => null); +await Promise.allSettled([ + cleanupSession(dataRoot, input.cwd, ownerSessionId), + createIdentityStore({ dataRoot }).cleanupSession(input.cwd, ownerSessionId), +]); +``` + +The only writable command is Rescue and uses the default profile; pin that invariant in a test. Never use generic `releasedSessionIds` to transition a job. + +- [ ] **Step 7: Verify GREEN and commit** + +```bash +node --test tests/session-end.test.mjs tests/hooks.test.mjs tests/recovery.test.mjs tests/zcode-client.test.mjs +git add scripts/lib/recovery.mjs hooks/session-end-hook.mjs tests/session-end.test.mjs tests/hooks.test.mjs +git commit -m "fix: settle writable rescue on session end" +``` + +### Task 5: Documentation and Full Verification + +**Files:** +- Modify: `README.md` +- Modify: `README.zh-CN.md` +- Modify: `CHANGELOG.md` +- Modify: `tests/release-contracts.test.mjs` + +- [ ] **Step 1: Write failing release-contract tests** + +Require both READMEs to state that a later Rescue settles a provable orphan without transferring ownership; held leases and unacknowledged stops retain the guard; `$zcode:status --all` is redacted inspection; and SessionEnd is best effort with reservation-time crash fallback. Require the Unreleased changelog to mention safe orphan settlement without a version bump. + +- [ ] **Step 2: Verify RED** + +```bash +node --test tests/release-contracts.test.mjs +``` + +- [ ] **Step 3: Update English, Chinese, and changelog text minimally** + +Do not advertise force release, cross-owner result access, a read-only Rescue, publishing, installation, or a version change. + +- [ ] **Step 4: Verify GREEN and commit** + +```bash +node --test tests/release-contracts.test.mjs +git add README.md README.zh-CN.md CHANGELOG.md tests/release-contracts.test.mjs +git commit -m "docs: explain orphan rescue settlement" +``` + +- [ ] **Step 5: Run focused regression suites** + +```bash +node --test tests/recovery.test.mjs tests/state.test.mjs tests/zcode-client.test.mjs tests/session-end.test.mjs tests/hooks.test.mjs tests/integration/companion.test.mjs tests/release-contracts.test.mjs +``` + +Expected: zero failures and zero cancellations. + +- [ ] **Step 6: Run complete CI-equivalent verification** + +```bash +npm run lint +npm run typecheck +npm test +npm run test:qualified +npm run check +git diff --check c753155...HEAD +git status --short --branch +``` + +Expected: all mandatory commands exit zero; real E2E tests may only report their documented explicit unqualified skips; no unstaged implementation changes remain. + +- [ ] **Step 7: Run independent final reviews** + +Using fixed point `c753155`, run parallel Standards and Spec reviews. The Spec reviewer compares the full diff to `/tmp/zcode-orphan-handoff.ETQEum/HANDOFF.md`, the amended lifecycle design, and ADR 0011. Resolve every blocking or high-severity finding, rerun affected tests, and repeat the relevant review until both axes pass. diff --git a/docs/superpowers/specs/2026-08-08-rescue-progress-lifecycle-design.md b/docs/superpowers/specs/2026-08-08-rescue-progress-lifecycle-design.md index f03192ba..2b0882ab 100644 --- a/docs/superpowers/specs/2026-08-08-rescue-progress-lifecycle-design.md +++ b/docs/superpowers/specs/2026-08-08-rescue-progress-lifecycle-design.md @@ -8,6 +8,12 @@ produce no visible output for minutes even while ZCode is actively running tools If the foreground companion is interrupted, its durable job may also remain `running` until a later same-owner reconciliation. +The same owner-only recovery becomes a permanent workspace denial of service +when the Codex session or worker disappears without completing the signal path. +Writable exclusion scans every active Rescue in the workspace, but startup +reconciliation currently scans only the new caller's jobs. A later Codex session +therefore cannot reserve a Rescue even when the old executor is provably gone. + The observed failure mode was not a CPU deadlock: the nested Codex task continued editing and running commands while the outer companion stayed silent. The user could not distinguish that state from a dead process. @@ -22,6 +28,11 @@ could not distinguish that state from a dead process. event. - Turn foreground `SIGINT` and `SIGTERM` into an acknowledged ZCode session stop and a durable cancelled job whenever the remote stop succeeds. +- Settle a writable Rescue whose exact executor has disappeared so it cannot + block the workspace forever after owner-session death, parent crash, or pipe + loss. +- Preserve exact user-visible ownership while allowing bounded internal + lifecycle maintenance to act on a validated durable job. - Preserve the existing result, ownership, permission, recovery, and background execution contracts. @@ -37,6 +48,11 @@ could not distinguish that state from a dead process. failure. A quiet but healthy task remains valid. - Do not change `--background` semantics. Background work survives the launching Codex turn and is stopped through `$zcode:cancel` or session lifecycle cleanup. +- Do not transfer, adopt, expose, or grant user-facing access to another Codex + session's job, result, permission snapshot, or ZCode session. +- Do not release the writable guard while an exact worker lease is held or a + remote stop remains unacknowledged. +- Do not add a public force-release command or a read-only Rescue mode. ## Approach @@ -119,6 +135,111 @@ existing failure path to settle the reservation. Background workers retain their current signal behavior so explicit cancellation remains the single owner of their terminal transition. +## Orphaned Writable Rescue + +A nonterminal claimed job is orphaned only when an internal nonblocking attempt +can acquire its exact worker lease. The operating system releases the advisory +lock when the worker exits; timestamps and PIDs are not used to evict a claimed +lease. A held lease ends recovery immediately without remote inspection, stop, +or state mutation. + +An unclaimed queued reservation has no persisted lease identity to probe. It +retains the existing five-minute worker-claim grace period: during that interval +maintenance leaves it untouched so a foreground caller or newly spawned child +can claim it. After the grace expires, the reservation becomes a failed launch +under the cancellation lock. This bounded compatibility rule closes the crash +window between reservation and lease claim without treating a known held lease +as stale. + +Before reserving a new writable Rescue, the companion first attempts the normal +same-owner reconciliation. If atomic reservation still reports +`WRITABLE_JOB_EXISTS`, it runs one internal workspace scavenging pass over active +writable blockers and retries reservation once. Each blocker is settled under +its existing per-job cancellation lock and is reread after lock acquisition. +The final reservation remains under the existing workspace state lock, so two +new sessions racing through scavenging can admit at most one writable Rescue. +No remote I/O occurs while the workspace state lock is held. + +The scavenger derives a Lifecycle Maintenance Principal from the validated +durable job's original `ownerSessionId`. It uses that stable internal principal +and the workspace-private broker credential only to restore the job's existing +broker ownership, inspect its one persisted ZCode session, stop it when required, +and settle local state. The new caller cannot supply this identity. The +scavenger does not change `ownerSessionId`, return old remote content, or bypass +the existing owner checks used by status, result, cancel, and resume. + +After a claimed worker lease is proven free, or an unclaimed reservation exceeds +the claim grace period: + +- A queued job, a job without an accepted remote session, or a missing remote + session becomes `failed` with a bounded lifecycle error. +- A remote `completed` or `idle` turn is extracted through the existing accepted + turn boundary, persisted as an owner-scoped result artifact, and becomes + `succeeded`. +- A remote terminal error or paused turn becomes `failed` when the durable job + was `running`. A `cancelling` job in paused state repeats `session/stop` and + becomes `cancelled` only after acknowledgement. +- A remote `running` or `waiting` turn is stopped. After acknowledgement, the + session is read once more: a provable completed result wins and becomes + `succeeded`; otherwise a `cancelling` job becomes `cancelled` and a `running` + lost-executor job becomes `failed`. +- Malformed, stale, or ambiguous remote state follows the same stop-proof rule. + An acknowledged stop permits status-appropriate terminalization: `cancelled` + from `cancelling`, or `failed` from `running`. An unacknowledged stop retains + `running`, records a bounded `lastCancelError`, and continues to guard the + workspace. + +Scavenging is internal lifecycle settlement, not a user cancellation. It returns +no old-job payload. If the blocker cannot be safely settled, the public command +keeps the stable `WRITABLE_JOB_EXISTS` envelope with an honest remedy to retry +later or inspect the redacted workspace list using `$zcode:status --all`. + +## SessionEnd Settlement + +The SessionEnd hook continues to target only the ending Codex session and never +starts a new broker or ZCode process. Before generic owner release, it prioritizes +the ending owner's active writable job and settles it under the job cancellation +lock. An unclaimed queued reservation is cancelled atomically; a later worker +claim then fails. A claimed queued job whose lease remains held is left for the +worker or later scavenging; if that exact lease is already free, SessionEnd +cancels the abandoned pre-remote reservation. + +For a job with an accepted remote session, the hook uses a bounded +existing-broker-only client for the job's exact wire profile and original owner. +This client may read, stop, and reread through a healthy broker that already +exists, but it cannot call broker ensure/start or spawn ZCode. It reads before +stopping so a completed turn becomes `succeeded`. If the turn is active, it +requests `session/stop`; after acknowledgement it reads once more so completion +that raced the stop still wins. Otherwise the explicit owner-session end becomes +`cancelled`. Missing broker, timeout, malformed state, or unacknowledged stop +leaves the job nonterminal for reservation-time scavenging. + +Only after this job settlement attempt does the hook call the existing generic +owner-release routine to stop untracked or read-only sessions and remove exact +owner mappings. Generic `releasedSessionIds` are cleanup results, not durable +cancellation evidence: a historical mapping can be released without a live +protocol or `session/stop`, so those IDs never drive job transitions. The hook +then removes ending-session caller, turn, and gate state. All work stays within +the existing bounded advisory hook budget; reservation-time scavenging is the +correctness fallback for crashes, missed hooks, and unavailable brokers. + +## Locking and Race Order + +The recovery lock order remains: + +1. per-job cancellation lock; +2. state reread and nonblocking exact worker-lease acquisition; +3. broker ownership/client operation; +4. result-artifact lock when completion is recovered; +5. atomic state transition. + +Executor completion, user cancellation, SessionEnd, and orphan scavenging all +serialize on the same cancellation lock before terminal publication. Terminal +state is never overwritten. A late worker cannot revive a terminal job, and a +second scavenger observes the first outcome idempotently. The workspace state +lock continues to serialize only state reads/writes and final reservation; it is +never nested around broker calls. + ## State and Security Invariants - `phase` is from a fixed public vocabulary. @@ -132,6 +253,11 @@ their terminal transition. accepted-turn boundaries, result artifacts, or terminal status. - Existing redaction still removes capabilities, tokens, and permission state from JSON output. +- Orphan settlement never changes the job's owner or exposes its result to the + session that triggered maintenance. +- Lifecycle maintenance accepts no public owner or remote-session identifier; + both come from a schema-validated job in the canonical workspace. +- A held worker lease or unacknowledged remote stop keeps the writable guard. ## Testing @@ -146,11 +272,20 @@ Tests follow red-green-refactor and cover: - interruption after accepted send, acknowledged stop, stop failure, correct exit codes, and absence of signal handling in background workers; - integration with the fake ZCode peer emitting intermediate notifications; +- cross-owner scavenging with held and released worker leases, remote completed, + missing, active-acknowledged, and active-unacknowledged states; +- concurrent scavenging and reservation admitting at most one writable Rescue; +- SessionEnd acknowledged, failed, queued, terminal-race, and sibling-owner + settlement; +- unchanged same-owner status, result, cancel, and resume isolation after an + internally recovered result; +- the corrected `WRITABLE_JOB_EXISTS` remedy without an invented read-only Rescue + surface; - the complete existing `npm run check` suite. ## Release Notes The patch updates both READMEs and `CHANGELOG.md` to describe live foreground -activity, status previews, heartbeat behavior, and the foreground cancellation -boundary. The package version remains unchanged until the release step selected -by the maintainer. +activity, status previews, heartbeat behavior, foreground cancellation, and safe +orphan settlement. The package version remains unchanged until the release step +selected by the maintainer. diff --git a/hooks/session-end-hook.mjs b/hooks/session-end-hook.mjs index 419741cc..4ff4e222 100644 --- a/hooks/session-end-hook.mjs +++ b/hooks/session-end-hook.mjs @@ -6,9 +6,38 @@ import { resolve } from 'node:path'; import { createIdentityStore } from '../scripts/lib/identity.mjs'; import { ownerIdForSession } from '../scripts/lib/job-control.mjs'; import { resolvePluginDataRoot } from '../scripts/lib/plugin-data.mjs'; -import { releaseManagedZCodeOwner } from '../scripts/lib/zcode-client.mjs'; +import { settleEndedOwnerWritableJob } from '../scripts/lib/recovery.mjs'; +import { createStateStore } from '../scripts/lib/state.mjs'; +import { createExistingManagedZCodeClient, releaseManagedZCodeOwner } from '../scripts/lib/zcode-client.mjs'; import { cleanupSession } from './lib/hook-state.mjs'; import { readHookInput } from './lib/hook-input.mjs'; -try { const input = await readHookInput('SessionEnd'); const dataRoot = resolvePluginDataRoot({ env: process.env, pluginRoot: resolve(fileURLToPath(new URL('../', import.meta.url))) }); await Promise.allSettled([releaseManagedZCodeOwner({ dataRoot, workspace: input.cwd, ownerId: ownerIdForSession(input.session_id), requestTimeoutMs: 750 }), cleanupSession(dataRoot, input.cwd, input.session_id), createIdentityStore({ dataRoot }).cleanupSession(input.cwd, input.session_id)]); } -catch (error) { process.stderr.write(`ZCode session cleanup advisory failed: ${error?.code ?? 'HOOK_FAILED'}\n`); process.exitCode = 1; } +try { + const input = await readHookInput('SessionEnd'); + const dataRoot = resolvePluginDataRoot({ env: process.env, pluginRoot: resolve(fileURLToPath(new URL('../', import.meta.url))) }); + const ownerSessionId = input.session_id; + const ownerId = ownerIdForSession(ownerSessionId); + const store = createStateStore({ dataRoot }); + await settleEndedOwnerWritableJob({ + store, + dataRoot, + workspace: input.cwd, + ownerSessionId, + requestTimeoutMs: 250, + lockTimeoutMs: 0, + createClient: (job, derivedOwnerId) => createExistingManagedZCodeClient({ + dataRoot, + workspace: input.cwd, + ownerId: derivedOwnerId, + requestTimeoutMs: 250, + }), + }).catch(() => null); + await releaseManagedZCodeOwner({ dataRoot, workspace: input.cwd, ownerId, requestTimeoutMs: 500 }).catch(() => null); + await Promise.allSettled([ + cleanupSession(dataRoot, input.cwd, ownerSessionId), + createIdentityStore({ dataRoot }).cleanupSession(input.cwd, ownerSessionId), + ]); +} catch (error) { + process.stderr.write(`ZCode session cleanup advisory failed: ${error?.code ?? 'HOOK_FAILED'}\n`); + process.exitCode = 1; +} diff --git a/scripts/lib/job-control.mjs b/scripts/lib/job-control.mjs index 5c5cec93..89ab0d95 100644 --- a/scripts/lib/job-control.mjs +++ b/scripts/lib/job-control.mjs @@ -195,7 +195,7 @@ function finalizeError(jobId, cause) { return new PluginError('JOB_CANCEL_FINALI /** @param {string} jobId @param {string} message @param {unknown} [cause] */ function cancelError(jobId, message, cause) { return new PluginError('JOB_CANCEL_FAILED', `Could not cancel job ${jobId}: ${message}`, { category: 'runtime', remedy: 'The job remains running; retry cancellation or inspect the ZCode session.', ...(cause ? { cause } : {}) }); } /** @param {string} message */ -function boundedCancelMessage(message) { +export function boundedCancelMessage(message) { let result = ''; let bytes = 0; for (const character of message) { const characterBytes = Buffer.byteLength(character); diff --git a/scripts/lib/recovery.mjs b/scripts/lib/recovery.mjs index 961bd301..a665341c 100644 --- a/scripts/lib/recovery.mjs +++ b/scripts/lib/recovery.mjs @@ -1,5 +1,5 @@ import { PluginError } from './errors.mjs'; -import { ownerIdForSession, withJobCancellationLock } from './job-control.mjs'; +import { boundedCancelMessage, ownerIdForSession, withJobCancellationLock } from './job-control.mjs'; import { extractFinalResult, writeResultArtifact } from './review.mjs'; import { withFileLock } from './fs.mjs'; import { resolveWorkspaceStorage } from './workspace.mjs'; @@ -18,71 +18,132 @@ export async function withWorkerLease(input, operation) { /** Reconcile only provably orphaned jobs owned by one exact Codex session. @param {{store:any,dataRoot:string,workspace:string,ownerSessionId:string,createClient:(job:any,ownerId:string)=>Promise,reconcileOwnership?:(input:any)=>Promise,now?:()=>number}} input */ export async function reconcileOwnedJobs(input) { - const reconcileOwnership = input.reconcileOwnership ?? reconcileBrokerOwnership; const jobs = (await input.store.listJobs(input.workspace)).filter((/** @type {any} */ job) => job.ownerSessionId === input.ownerSessionId && !TERMINAL.has(job.status)); const outcomes = []; for (const job of jobs) { - try { - outcomes.push(await withJobCancellationLock({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: job.id }, async () => { - const current = await input.store.readJob(input.workspace, job.id); - if (current.ownerSessionId !== input.ownerSessionId || TERMINAL.has(current.status)) return current; - if (current.status === 'queued' && !isDigest(current.workerLeaseId)) { - return (input.now ?? Date.now)() - Date.parse(current.updatedAt) >= LEGACY_QUEUED_STALE_MS - ? failJob(input, current, recoveryError('Legacy queued reservation exceeded the conservative worker-claim grace period.')) - : current; - } - if (!isDigest(current.workerLeaseId) && legacyWorkerAlive(current)) return current; - if (!isDigest(current.workerLeaseId)) return reconcileOrphan(input, current, reconcileOwnership); + try { outcomes.push(await settleSelectedJob({ ...input, selectedJobId: job.id, expectedOwnerSessionId: job.ownerSessionId, intent: 'owner-recovery' })); } + catch { outcomes.push(job); } + } + return outcomes; +} + +/** Settle provably orphaned writable Rescue blockers without adopting their public ownership. @param {{store:any,dataRoot:string,workspace:string,createClient:(job:any,ownerId:string)=>Promise,reconcileOwnership?:(input:any)=>Promise,now?:()=>number}} input */ +export async function scavengeWritableJobs(input) { + const jobs = (await input.store.listJobs(input.workspace)) + .filter((/** @type {any} */ job) => job.command === 'rescue' && job.readOnly === false && !TERMINAL.has(job.status)); + const outcomes = []; + for (const job of jobs) { + try { outcomes.push(await settleSelectedJob({ ...input, selectedJobId: job.id, expectedOwnerSessionId: job.ownerSessionId, intent: 'scavenge' })); } + catch { outcomes.push(job); } + } + return outcomes; +} + +/** + * Best-effort settlement for the ending owner's one active writable Rescue. + * Unlike orphan scavenging, SessionEnd is an explicit owner lifecycle signal, so + * an accepted remote turn may be stopped even while its worker lease is held. + * @param {{store:any,dataRoot:string,workspace:string,ownerSessionId:string,lockTimeoutMs?:number,requestTimeoutMs?:number,createClient:(job:any,ownerId:string)=>Promise}} input + */ +export async function settleEndedOwnerWritableJob(input) { + const selected = (await input.store.listJobs(input.workspace)) + .filter((/** @type {any} */ job) => job.ownerSessionId === input.ownerSessionId + && job.command === 'rescue' && job.readOnly === false && !TERMINAL.has(job.status)) + .at(-1); + if (!selected) return null; + try { + return await withJobCancellationLock({ + dataRoot: input.dataRoot, + workspace: input.workspace, + jobId: selected.id, + timeoutMs: input.lockTimeoutMs ?? 0, + }, async () => { + const current = await input.store.readJob(input.workspace, selected.id); + if (current.id !== selected.id || current.ownerSessionId !== input.ownerSessionId + || current.command !== 'rescue' || current.readOnly !== false || TERMINAL.has(current.status)) return current; + if (current.status === 'queued' && !isDigest(current.workerLeaseId)) return cancelQueuedJob(input, current); + if (current.status === 'queued') { try { - return await withWorkerLease({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: current.id, workerLeaseId: current.workerLeaseId, timeoutMs: 0 }, () => current.status === 'queued' - ? failJob(input, current, recoveryError('Claimed queued worker exited before execution started.')) - : reconcileOrphan(input, current, reconcileOwnership)); + return await withWorkerLease({ + dataRoot: input.dataRoot, + workspace: input.workspace, + jobId: current.id, + workerLeaseId: current.workerLeaseId, + timeoutMs: 0, + }, () => cancelQueuedJob(input, current)); } catch (error) { if (error instanceof PluginError && error.code === 'LOCK_TIMEOUT') return current; throw error; } - })); - } catch { outcomes.push(job); } + } + if (!['running', 'cancelling'].includes(current.status) || typeof current.zcodeSessionId !== 'string') return current; + return settleEndedRemoteJob(input, current); + }); + } catch (error) { + if (error instanceof PluginError && error.code === 'LOCK_TIMEOUT') return input.store.readJob(input.workspace, selected.id); + throw error; } - return outcomes; } -/** @param {any} input @param {any} job @param {(input:any)=>Promise} reconcileOwnership */ -async function reconcileOrphan(input, job, reconcileOwnership) { +/** @param {any} input */ +async function settleSelectedJob(input) { + return withJobCancellationLock({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: input.selectedJobId }, async () => { + const current = await input.store.readJob(input.workspace, input.selectedJobId); + if (current.id !== input.selectedJobId || current.ownerSessionId !== input.expectedOwnerSessionId || TERMINAL.has(current.status)) return current; + if (input.intent === 'scavenge' && (current.command !== 'rescue' || current.readOnly !== false)) return current; + if (current.status === 'queued' && !isDigest(current.workerLeaseId)) { + return (input.now ?? Date.now)() - Date.parse(current.createdAt) >= LEGACY_QUEUED_STALE_MS + ? failJob(input, current, recoveryError('Queued reservation exceeded the conservative worker-claim grace period.')) + : current; + } + if (!isDigest(current.workerLeaseId) && legacyWorkerAlive(current)) return current; + if (!isDigest(current.workerLeaseId)) return reconcileOrphan(input, current); + try { + return await withWorkerLease({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: current.id, workerLeaseId: current.workerLeaseId, timeoutMs: 0 }, () => current.status === 'queued' + ? failJob(input, current, recoveryError('Claimed queued worker exited before execution started.')) + : reconcileOrphan(input, current)); + } catch (error) { + if (error instanceof PluginError && error.code === 'LOCK_TIMEOUT') return current; + throw error; + } + }); +} + +/** @param {any} input @param {any} job */ +async function reconcileOrphan(input, job) { let client; try { if (job.status === 'queued') return failJob(input, job, recoveryError('Queued worker reservation is orphaned.')); if (typeof job.zcodeSessionId !== 'string') return failJob(input, job, recoveryError('Worker exited before a remote session was accepted.')); const ownerId = ownerIdForSession(job.ownerSessionId); - await reconcileOwnership({ dataRoot: input.dataRoot, workspace: input.workspace, ownerId, ownedSessionIds: [job.zcodeSessionId] }); + await (input.reconcileOwnership ?? reconcileBrokerOwnership)({ dataRoot: input.dataRoot, workspace: input.workspace, ownerId, ownedSessionIds: [job.zcodeSessionId] }); client = await input.createClient(job, ownerId); let listed; try { listed = await client.listSessions(); } - catch (error) { return settleAmbiguity(input, job, client, error); } - if (!Array.isArray(listed?.sessions)) return settleAmbiguity(input, job, client, recoveryError('ZCode session listing is malformed during recovery.')); + catch (error) { return stopThenSettle(input, job, client, error); } + if (!Array.isArray(listed?.sessions)) return stopThenSettle(input, job, client, recoveryError('ZCode session listing is malformed during recovery.')); if (!listed.sessions.some((/** @type {any} */ session) => session.sessionId === job.zcodeSessionId)) return failJob(input, job, recoveryError('ZCode session is missing during recovery.')); - if (job.command === 'transfer') return stopThenFail(input, job, client, recoveryError('Transfer worker exited before local finalization.')); - if (!hasBoundary(job)) return stopThenFail(input, job, client, recoveryError('The durable turn boundary is incomplete.')); + if (job.command === 'transfer') return stopThenSettle(input, job, client, recoveryError('Transfer worker exited before local finalization.')); + if (!hasBoundary(job)) return stopThenSettle(input, job, client, recoveryError('The durable turn boundary is incomplete.')); let snapshot; try { snapshot = await client.readSession(job.zcodeSessionId); } - catch (error) { return settleAmbiguity(input, job, client, error); } - if (!Number.isSafeInteger(snapshot?.runtime?.stateRevision) || snapshot.runtime.stateRevision < job.startRevision) return settleAmbiguity(input, job, client, recoveryError('ZCode recovery state is older than the accepted turn boundary.')); + catch (error) { return stopThenSettle(input, job, client, error); } + if (!Number.isSafeInteger(snapshot?.runtime?.stateRevision) || snapshot.runtime.stateRevision < job.startRevision) return stopThenSettle(input, job, client, recoveryError('ZCode recovery state is older than the accepted turn boundary.')); const remoteStatus = snapshot?.projection?.status; if (REMOTE_ACTIVE.has(remoteStatus)) { - return job.status === 'cancelling' ? stopThenCancel(input, job, client) : job; + if (job.status === 'cancelling' || input.intent === 'scavenge') return stopThenSettle(input, job, client, recoveryError('The remote turn remained active after its executor exited.')); + return job; } - if (remoteStatus === 'paused') return cancelJob(input, job); + if (remoteStatus === 'paused') return job.status === 'cancelling' + ? stopThenSettle(input, job, client, recoveryError('The cancelling remote turn is paused.')) + : failJob(input, job, recoveryError('The orphaned remote turn is paused.')); if (remoteStatus === 'error') return failJob(input, job, recoveryError(snapshot?.projection?.lastError?.message ?? 'ZCode reported a terminal error during recovery.')); - if (!['completed', 'idle'].includes(remoteStatus)) return settleAmbiguity(input, job, client, recoveryError('ZCode recovery state is ambiguous.')); - try { - const result = extractFinalResult(snapshot, job.command, { inputId: job.inputId, stateRevision: job.startRevision, beforeMessageIds: new Set(job.beforeMessageIds) }); - const resultArtifact = await writeResultArtifact({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: job.id, contents: result }); - return await input.store.transitionJob(input.workspace, job.id, ['running', 'cancelling'], 'succeeded', { resultArtifact, finishedAt: new Date().toISOString(), exitCode: 0 }); - } catch (error) { return failJob(input, job, error); } + if (!['completed', 'idle'].includes(remoteStatus)) return stopThenSettle(input, job, client, recoveryError('ZCode recovery state is ambiguous.')); + return completeJob(input, job, snapshot); } catch (error) { const current = await input.store.readJob(input.workspace, job.id); if (TERMINAL.has(current.status)) return current; - return client ? settleAmbiguity(input, current, client, error) : retainAfterStopFailure(input, current, error); + return client ? stopThenSettle(input, current, client, error) : retainAfterStopFailure(input, current, error); } finally { await client?.close().catch(() => {}); } } @@ -92,26 +153,81 @@ function hasBoundary(job) { return typeof job.inputId === 'string' && Number.isS async function failJob(input, job, error) { const current = await input.store.readJob(input.workspace, job.id); if (TERMINAL.has(current.status)) return current; - return input.store.transitionJob(input.workspace, job.id, [current.status], 'failed', { error: { message: recoveryMessage(error) }, finishedAt: new Date().toISOString(), exitCode: 1 }); + try { return await input.store.transitionJob(input.workspace, job.id, [current.status], 'failed', { error: { message: recoveryMessage(error) }, finishedAt: new Date().toISOString(), exitCode: 1 }); } + catch (transitionError) { return conflictWinner(input, job, transitionError); } } /** @param {any} input @param {any} job */ async function cancelJob(input, job) { const current = await input.store.readJob(input.workspace, job.id); if (TERMINAL.has(current.status)) return current; - if (current.status === 'running') await input.store.transitionJob(input.workspace, job.id, ['running'], 'cancelling'); - return input.store.transitionJob(input.workspace, job.id, ['cancelling'], 'cancelled', { finishedAt: new Date().toISOString(), exitCode: null }); + try { + if (current.status === 'running') await input.store.transitionJob(input.workspace, job.id, ['running'], 'cancelling'); + return await input.store.transitionJob(input.workspace, job.id, ['cancelling'], 'cancelled', { finishedAt: new Date().toISOString(), exitCode: null }); + } catch (error) { return conflictWinner(input, job, error); } } -/** @param {any} input @param {any} job @param {any} client @param {unknown} error */ -async function settleAmbiguity(input, job, client, error) { return stopThenFail(input, job, client, error); } -/** @param {any} input @param {any} job @param {any} client @param {unknown} error */ -async function stopThenFail(input, job, client, error) { - const stopped = await stopRemote(job, client); - return stopped.ok ? failJob(input, job, error) : retainAfterStopFailure(input, job, stopped.error); +/** @param {any} input @param {any} job */ +async function cancelQueuedJob(input, job) { + const current = await input.store.readJob(input.workspace, job.id); + if (TERMINAL.has(current.status) || current.status !== 'queued') return current; + try { return await input.store.transitionJob(input.workspace, job.id, ['queued'], 'cancelled', { finishedAt: new Date().toISOString(), exitCode: null }); } + catch (error) { return conflictWinner(input, job, error); } +} + +/** @param {any} input @param {any} job */ +async function settleEndedRemoteJob(input, job) { + let client; + try { + client = await input.createClient(job, ownerIdForSession(job.ownerSessionId)); + if (!client) return retainAfterStopFailure(input, job, recoveryError('The existing ZCode broker is unavailable during SessionEnd settlement.')); + let snapshot; + try { snapshot = await client.readSession(job.zcodeSessionId); } + catch (error) { return retainAfterStopFailure(input, job, error); } + const completed = await completeEndedJob(input, job, snapshot); + if (completed) return completed; + if (!REMOTE_ACTIVE.has(snapshot?.projection?.status)) return input.store.readJob(input.workspace, job.id); + try { await client.stopSession(job.zcodeSessionId); } + catch (error) { return retainAfterStopFailure(input, job, error); } + try { snapshot = await client.readSession(job.zcodeSessionId); } + catch { return cancelJob(input, job); } + return await completeEndedJob(input, job, snapshot) ?? cancelJob(input, job); + } catch (error) { + return retainAfterStopFailure(input, job, error); + } finally { await client?.close().catch(() => {}); } +} + +/** Return null when completion is not proven and leave the durable job active. @param {any} input @param {any} job @param {any} snapshot */ +async function completeEndedJob(input, job, snapshot) { + if (!hasBoundary(job) || !Number.isSafeInteger(snapshot?.runtime?.stateRevision) + || snapshot.runtime.stateRevision < job.startRevision || !['completed', 'idle'].includes(snapshot?.projection?.status)) return null; + try { + const result = extractFinalResult(snapshot, job.command, { inputId: job.inputId, stateRevision: job.startRevision, beforeMessageIds: new Set(job.beforeMessageIds) }); + const resultArtifact = await writeResultArtifact({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: job.id, contents: result }); + return await input.store.transitionJob(input.workspace, job.id, ['running', 'cancelling'], 'succeeded', { resultArtifact, finishedAt: new Date().toISOString(), exitCode: 0 }); + } catch (error) { + if (isTransitionConflict(error)) return input.store.readJob(input.workspace, job.id); + return null; + } } -/** @param {any} input @param {any} job @param {any} client */ -async function stopThenCancel(input, job, client) { +/** @param {any} input @param {any} job @param {any} snapshot @param {'fail'|'cancel'} [invalidResult] */ +async function completeJob(input, job, snapshot, invalidResult = 'fail') { + try { + const result = extractFinalResult(snapshot, job.command, { inputId: job.inputId, stateRevision: job.startRevision, beforeMessageIds: new Set(job.beforeMessageIds) }); + const resultArtifact = await writeResultArtifact({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: job.id, contents: result }); + return await input.store.transitionJob(input.workspace, job.id, ['running', 'cancelling'], 'succeeded', { resultArtifact, finishedAt: new Date().toISOString(), exitCode: 0 }); + } catch (error) { + if (isTransitionConflict(error)) return input.store.readJob(input.workspace, job.id); + return invalidResult === 'cancel' ? cancelJob(input, job) : failJob(input, job, error); + } +} +/** @param {any} input @param {any} job @param {any} client @param {unknown} error */ +async function stopThenSettle(input, job, client, error) { const stopped = await stopRemote(job, client); - return stopped.ok ? cancelJob(input, job) : retainAfterStopFailure(input, job, stopped.error); + if (!stopped.ok) return retainAfterStopFailure(input, job, stopped.error); + let snapshot; + try { snapshot = await client.readSession(job.zcodeSessionId); } catch { /* acknowledged stop is sufficient for status-appropriate settlement */ } + if (snapshot && hasBoundary(job) && Number.isSafeInteger(snapshot?.runtime?.stateRevision) && snapshot.runtime.stateRevision >= job.startRevision + && ['completed', 'idle'].includes(snapshot?.projection?.status)) return completeJob(input, job, snapshot, job.status === 'cancelling' ? 'cancel' : 'fail'); + return job.status === 'cancelling' ? cancelJob(input, job) : failJob(input, job, error); } /** @param {any} job @param {any} client */ async function stopRemote(job, client) { @@ -123,10 +239,22 @@ async function retainAfterStopFailure(input, job, error) { const current = await input.store.readJob(input.workspace, job.id); if (TERMINAL.has(current.status)) return current; const message = recoveryMessage(error); - return input.store.transitionJob(input.workspace, job.id, [current.status], 'running', { lastCancelError: message }); + try { return await input.store.transitionJob(input.workspace, job.id, [current.status], 'running', { lastCancelError: message }); } + catch (transitionError) { + const winner = await input.store.readJob(input.workspace, job.id); + if (TERMINAL.has(winner.status)) return winner; + return conflictWinner(input, job, transitionError); + } } +/** @param {any} input @param {any} job @param {unknown} error */ +async function conflictWinner(input, job, error) { + if (isTransitionConflict(error)) return input.store.readJob(input.workspace, job.id); + throw error; +} +/** @param {unknown} error */ +function isTransitionConflict(error) { return error instanceof PluginError && ['JOB_TERMINAL', 'JOB_STATUS_CONFLICT'].includes(error.code); } /** @param {unknown} error */ -function recoveryMessage(error) { return (error instanceof Error ? error.message : 'Unknown recovery failure').slice(0, 2_048); } +function recoveryMessage(error) { return boundedCancelMessage(error instanceof Error ? error.message : 'Unknown recovery failure'); } /** @param {string} message */ function recoveryError(message) { return new PluginError('JOB_RECOVERY_FAILED', message, { category: 'state', remedy: 'Inspect the durable job and its ZCode session.' }); } /** @param {string} directory @param {string} jobId @param {string} workerLeaseId */ diff --git a/scripts/lib/review.mjs b/scripts/lib/review.mjs index 2314d82d..3d3195d4 100644 --- a/scripts/lib/review.mjs +++ b/scripts/lib/review.mjs @@ -7,7 +7,7 @@ import { PluginError } from './errors.mjs'; import { resolveModel } from './args.mjs'; import { ensurePrivateDirectory, withFileLock } from './fs.mjs'; import { collectGitFacts } from './git.mjs'; -import { createJobController } from './job-control.mjs'; +import { createJobController, withJobCancellationLock } from './job-control.mjs'; import { createProgressReporter, waitForCompletionOrAbort } from './progress.mjs'; import { buildPrompt } from './prompts.mjs'; import { loadReviewOutputSchema, validateJsonSchema } from './review-schema.mjs'; @@ -107,11 +107,9 @@ export async function executeJob(input) { const finalSnapshot = await client.readSession(activeSessionId); remoteTerminalProven = true; const result = extractFinalResult(finalSnapshot, job.command, turnBoundary); - const resultArtifact = await writeArtifact({ dataRoot, workspace, directory: 'results', jobId: job.id, contents: result }, { syncDirectory: input.syncDirectory }); const terminalCleanupErrors = await cleanupProgress(); if (terminalCleanupErrors.length) throw progressFailure(terminalCleanupErrors); - const succeeded = await input.store.transitionJob(workspace, job.id, ['running'], 'succeeded', { resultArtifact, finishedAt: new Date().toISOString(), exitCode: 0 }); - output = { job: succeeded, result }; + output = await publishSuccessfulResult({ input, job, workspace, dataRoot, result }); } catch (error) { primaryError = error; const current = await input.store.readJob(workspace, job.id).catch(() => running); @@ -148,6 +146,35 @@ export async function executeJob(input) { return output; } +/** Serialize executor terminal publication with cancellation and lifecycle maintenance. @param {{input:any,job:any,workspace:string,dataRoot:string,result:string}} publication */ +async function publishSuccessfulResult({ input, job, workspace, dataRoot, result }) { + return withJobCancellationLock({ dataRoot, workspace, jobId: job.id }, async () => { + const current = await input.store.readJob(workspace, job.id); + if (current.status === 'succeeded') return { job: current, result: await readResultArtifact({ dataRoot, workspace, artifact: current.resultArtifact }) }; + if (['failed', 'cancelled'].includes(current.status)) throw terminalPublicationError(job.id, current.status); + if (current.status !== 'running') throw statusPublicationError(job.id, current.status); + const resultArtifact = await writeResultArtifact({ dataRoot, workspace, jobId: job.id, contents: result }, { syncDirectory: input.syncDirectory }); + try { + const succeeded = await input.store.transitionJob(workspace, job.id, ['running'], 'succeeded', { resultArtifact, finishedAt: new Date().toISOString(), exitCode: 0 }); + return { job: succeeded, result }; + } catch (error) { + const winner = await input.store.readJob(workspace, job.id).catch(() => null); + if (winner?.resultArtifact !== resultArtifact) await removeResultArtifact({ dataRoot, workspace, jobId: job.id, artifact: resultArtifact }).catch(() => {}); + throw error; + } + }); +} + +/** @param {string} jobId @param {string} status */ +function terminalPublicationError(jobId, status) { + return new PluginError('JOB_TERMINAL', `Job ${jobId} is already terminal.`, { category: 'state', remedy: 'Create a new job instead of changing a terminal job.', details: { jobId, status } }); +} + +/** @param {string} jobId @param {string} status */ +function statusPublicationError(jobId, status) { + return new PluginError('JOB_STATUS_CONFLICT', `Job ${jobId} changed status unexpectedly.`, { category: 'state', remedy: 'Reload the job and retry from its current status.', details: { actualStatus: status, expectedStatuses: ['running'], jobId } }); +} + /** @template T @param {()=>Promise} operation @param {AbortSignal|undefined} signal */ async function boundedStep(operation, signal) { signal?.throwIfAborted(); diff --git a/scripts/lib/state.mjs b/scripts/lib/state.mjs index 5f8426c0..f224f53a 100644 --- a/scripts/lib/state.mjs +++ b/scripts/lib/state.mjs @@ -58,7 +58,7 @@ export function createStateStore(options) { if (!reservation.readOnly && jobs.some(isActiveWritableJob)) { throw new PluginError('WRITABLE_JOB_EXISTS', 'This workspace already has an active writable rescue job.', { category: 'state', - remedy: 'Wait for the writable job to finish or run this job read-only.', + remedy: 'Retry later or inspect the redacted workspace list with $zcode:status --all.', details: { workspaceKey: storage.workspaceKey }, }); } diff --git a/scripts/lib/zcode-client.mjs b/scripts/lib/zcode-client.mjs index fa9483f7..98f2ac58 100644 --- a/scripts/lib/zcode-client.mjs +++ b/scripts/lib/zcode-client.mjs @@ -6,7 +6,7 @@ import { PluginError } from './errors.mjs'; import { isSafeIdentifier } from './identifier.mjs'; import { connectZCodeBroker, MAX_DRAIN_TIMEOUT_MS, spawnZCodeProtocol } from './zcode-protocol.mjs'; import { validSessionInfo, validSnapshot as snapshotValid } from './zcode-schema.mjs'; -import { ensureZCodeBroker, prioritizeBrokerOwnership, readHealthyBrokerIdentity } from '../zcode-broker.mjs'; +import { brokerIdentityNameForWireOptions, ensureZCodeBroker, prioritizeBrokerOwnership, probeBrokerHealth, readHealthyBrokerIdentity } from '../zcode-broker.mjs'; import { resolveWorkspaceStorage } from './workspace.mjs'; const THOUGHT_LEVELS = new Set(['none', 'minimal', 'low', 'medium', 'high', 'xhigh']); @@ -92,18 +92,19 @@ export class ZCodeClient { close() { return this.protocol.close(); } } -/** @param {{workspace:string,launch?:{command:string,args:string[],target?:string},brokerEndpoint?:string,brokerToken?:string,ownerId?:string,env?:NodeJS.ProcessEnv,requestTimeoutMs?:number,completionTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number}} options */ +/** @param {{workspace:string,launch?:{command:string,args:string[],target?:string},brokerEndpoint?:string,brokerToken?:string,ownerId?:string,existingProtocolOnly?:boolean,env?:NodeJS.ProcessEnv,requestTimeoutMs?:number,completionTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number}} options */ export async function createZCodeClient(options) { if (!plainObject(options) || !nonEmpty(options.workspace) || (options.brokerEndpoint === undefined) === (options.launch === undefined) || options.brokerEndpoint !== undefined && (!nonEmpty(options.brokerEndpoint) || !nonEmpty(options.brokerToken) || options.brokerToken.length < 32 || !nonEmpty(options.ownerId) || options.ownerId.length < 16) + || options.existingProtocolOnly !== undefined && (options.brokerEndpoint === undefined || typeof options.existingProtocolOnly !== 'boolean') || options.launch !== undefined && !plainObject(options.launch)) throw inputError(); const protocolOptions = { cwd: options.workspace, env: options.env, requestTimeoutMs: options.requestTimeoutMs, completionTimeoutMs: options.completionTimeoutMs, maxFrameBytes: options.maxFrameBytes, maxOutboundBytes: options.maxOutboundBytes, drainTimeoutMs: options.drainTimeoutMs, }; const protocol = options.brokerEndpoint - ? await connectZCodeBroker(options.brokerEndpoint, { ...protocolOptions, brokerToken: /** @type {string} */ (options.brokerToken), ownerId: /** @type {string} */ (options.ownerId) }) + ? await connectZCodeBroker(options.brokerEndpoint, { ...protocolOptions, brokerToken: /** @type {string} */ (options.brokerToken), ownerId: /** @type {string} */ (options.ownerId), ...(options.existingProtocolOnly === undefined ? {} : { existingProtocolOnly: options.existingProtocolOnly }) }) : await spawnZCodeProtocol(/** @type {{command:string,args:string[],target?:string}} */ (options.launch), protocolOptions); return new ZCodeClient(protocol); } @@ -116,6 +117,22 @@ export async function createManagedZCodeClient(options) { return createZCodeClient({ workspace: options.workspace, brokerEndpoint: identity.endpoint, brokerToken: identity.brokerToken, ownerId: options.ownerId, requestTimeoutMs: options.requestTimeoutMs, completionTimeoutMs: options.completionTimeoutMs, maxFrameBytes: options.maxFrameBytes, maxOutboundBytes: options.maxOutboundBytes, drainTimeoutMs: options.drainTimeoutMs }); } +/** @param {{dataRoot:string,workspace:string,ownerId:string,requestTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number}} options */ +export async function createExistingManagedZCodeClient(options) { + requireExactObject(options, ['dataRoot', 'workspace', 'ownerId'], ['requestTimeoutMs', 'maxFrameBytes', 'maxOutboundBytes', 'drainTimeoutMs']); + if (!nonEmpty(options.dataRoot) || !nonEmpty(options.workspace) || !nonEmpty(options.ownerId) || options.ownerId.length < 16 + || !boundedRequestOption(options.requestTimeoutMs) || !boundedWireOption(options.maxFrameBytes, 16 * 1024 * 1024) || !boundedWireOption(options.maxOutboundBytes, 64 * 1024 * 1024) || !boundedDrainOption(options.drainTimeoutMs)) throw inputError(); + const storage = await resolveWorkspaceStorage(options); + const identityName = brokerIdentityNameForWireOptions(options); + const identity = await readHealthyBrokerIdentity(resolve(storage.directory, 'broker', identityName), { + healthProbe: (record) => probeBrokerHealth(record, options.requestTimeoutMs), + }); + if (!identity) return null; + try { + return await createZCodeClient({ workspace: storage.workspacePath, brokerEndpoint: identity.endpoint, brokerToken: identity.brokerToken, ownerId: options.ownerId, existingProtocolOnly: true, requestTimeoutMs: options.requestTimeoutMs, maxFrameBytes: options.maxFrameBytes, maxOutboundBytes: options.maxOutboundBytes, drainTimeoutMs: options.drainTimeoutMs }); + } catch { return null; } +} + /** * Releases an exact lifecycle owner from brokers that already exist. This * function never calls ensureZCodeBroker and therefore cannot start ZCode from @@ -181,6 +198,8 @@ function normalizeImportedHistory(history) { function boundedWireOption(value, maximum) { return value === undefined || typeof value === 'number' && Number.isSafeInteger(value) && value >= 128 && value <= maximum; } /** @param {unknown} value */ function boundedDrainOption(value) { return value === undefined || typeof value === 'number' && Number.isSafeInteger(value) && value >= 1 && value <= MAX_DRAIN_TIMEOUT_MS; } +/** @param {unknown} value */ +function boundedRequestOption(value) { return value === undefined || typeof value === 'number' && Number.isSafeInteger(value) && value >= 1 && value <= 3_600_000; } /** @param {any} model */ function validateModel(model) { requireExactObject(model, ['providerId', 'modelId'], ['variant']); requireString(model.providerId); requireString(model.modelId); if (model.variant !== undefined) requireString(model.variant); } diff --git a/scripts/lib/zcode-protocol.mjs b/scripts/lib/zcode-protocol.mjs index 8ba21d30..a1323548 100644 --- a/scripts/lib/zcode-protocol.mjs +++ b/scripts/lib/zcode-protocol.mjs @@ -318,11 +318,19 @@ export async function spawnZCodeProtocol(launch, options = {}) { return new ZCodeProtocolClient(child, options); } -/** @param {string} endpoint @param {{brokerToken:string,ownerId:string,requestTimeoutMs?:number,completionTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number}} options */ +/** @param {string} endpoint @param {{brokerToken:string,ownerId:string,existingProtocolOnly?:boolean,requestTimeoutMs?:number,completionTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number}} options */ export async function connectZCodeBroker(endpoint, options) { - if (!nonEmpty(endpoint) || !nonEmpty(options.brokerToken) || options.brokerToken.length < 32 || !nonEmpty(options.ownerId) || options.ownerId.length < 16) throw protocolInputError(); + if (!nonEmpty(endpoint) || !plainObject(options) || !nonEmpty(options.brokerToken) || options.brokerToken.length < 32 || !nonEmpty(options.ownerId) || options.ownerId.length < 16 + || options.existingProtocolOnly !== undefined && typeof options.existingProtocolOnly !== 'boolean') throw protocolInputError(); + const requestTimeoutMs = boundedInteger(options.requestTimeoutMs, 30_000, 1, 3_600_000); const socket = net.createConnection(endpoint); - await new Promise((resolve, reject) => { socket.once('connect', resolve); socket.once('error', reject); }); + await new Promise((resolve, reject) => { + const cleanup = () => { clearTimeout(timer); socket.off('connect', onConnect); socket.off('error', onError); }; + const onConnect = () => { cleanup(); resolve(undefined); }; + const onError = (/** @type {Error} */ error) => { cleanup(); socket.destroy(); reject(error); }; + const timer = setTimeout(() => { cleanup(); socket.destroy(); reject(requestTimeout('broker/connect', requestTimeoutMs)); }, requestTimeoutMs); + timer.unref?.(); socket.once('connect', onConnect); socket.once('error', onError); + }); /** @type {any} */ const transport = { stdout: socket, stdin: socket, stderr: null, exitCode: null, signalCode: null, @@ -333,9 +341,19 @@ export async function connectZCodeBroker(endpoint, options) { }, kill() { transport.exitCode = 0; socket.destroy(); return true; }, }; - const protocol = new ZCodeProtocolClient(transport, options); - await protocol.request('broker/auth', { token: options.brokerToken, ownerId: options.ownerId }); - return protocol; + /** @type {ZCodeProtocolClient|undefined} */ + let protocol; + try { + protocol = new ZCodeProtocolClient(transport, options); + const authenticated = await protocol.request('broker/auth', { token: options.brokerToken, ownerId: options.ownerId, ...(options.existingProtocolOnly === undefined ? {} : { existingProtocolOnly: options.existingProtocolOnly }) }); + if (!plainObject(authenticated) || authenticated.authenticated !== true + || options.existingProtocolOnly === true && authenticated.existingProtocolOnly !== true) throw brokerCapabilityUnavailable(); + return protocol; + } catch (error) { + socket.destroy(); + await protocol?.close().catch(() => {}); + throw error; + } } /** @param {any} message @param {unknown} sessionId */ @@ -358,6 +376,9 @@ function nonEmpty(value) { return typeof value === 'string' && value.length > 0; function plainObject(value) { return value !== null && typeof value === 'object' && !Array.isArray(value); } function disconnected() { return new PluginError('ZCODE_DISCONNECTED', 'The ZCode connection is closed.', { category: 'runtime', remedy: 'Create a new client and retry.' }); } function protocolInputError() { return new PluginError('ZCODE_PROTOCOL_INPUT_INVALID', 'ZCode protocol input is invalid.', { category: 'validation', remedy: 'Provide a valid method, params, session, and bounded timeout.' }); } +function brokerCapabilityUnavailable() { return new PluginError('ZCODE_BROKER_CAPABILITY_UNAVAILABLE', 'The broker did not authenticate the requested connection capability.', { category: 'protocol', remedy: 'Restart the broker with the current ZCode plugin version.' }); } +/** @param {string} method @param {number} timeoutMs */ +function requestTimeout(method, timeoutMs) { return new PluginError('ZCODE_REQUEST_TIMEOUT', `ZCode request timed out: ${method}.`, { category: 'timeout', remedy: 'Retry the operation.', details: { method, timeoutMs } }); } function malformedFrame() { return new PluginError('ZCODE_PROTOCOL_MALFORMED', 'ZCode sent a malformed protocol frame.', { category: 'protocol', remedy: 'Restart ZCode and retry.' }); } function frameTooLarge() { return new PluginError('ZCODE_PROTOCOL_FRAME_TOO_LARGE', 'A ZCode protocol frame exceeded the configured limit.', { category: 'protocol', remedy: 'Reduce request size or inspect the peer for invalid output.' }); } /** @param {unknown} error @param {string} [stderrTail] */ diff --git a/scripts/zcode-broker.mjs b/scripts/zcode-broker.mjs index 4479c86c..afe8e9eb 100644 --- a/scripts/zcode-broker.mjs +++ b/scripts/zcode-broker.mjs @@ -58,6 +58,19 @@ export function brokerEndpointFor(options) { return join('/tmp', `zcode-${typeof process.getuid === 'function' ? process.getuid() : 'user'}`, `${digest}.sock`); } +export function brokerIdentityNameForWireOptions(options = {}) { + const profile = options.maxFrameBytes === undefined + && options.maxOutboundBytes === undefined + && options.drainTimeoutMs === undefined + ? null + : createHash('sha256').update(JSON.stringify([ + options.maxFrameBytes ?? null, + options.maxOutboundBytes ?? null, + options.drainTimeoutMs ?? null, + ])).digest('hex').slice(0, 16); + return profile ? `identity-${profile}.json` : 'identity.json'; +} + /** @param {string} path @param {{endpoint:string,pid?:number,instanceId?:string,brokerToken?:string}} input */ export async function writeBrokerIdentity(path, input) { if (!input || typeof input.endpoint !== 'string') throw brokerInputError(); @@ -79,11 +92,12 @@ export async function readHealthyBrokerIdentity(path, options = {}) { return value; } -/** @param {{endpoint:string,brokerToken:string,pid:number,instanceId:string}} record */ -export async function probeBrokerHealth(record) { +/** @param {{endpoint:string,brokerToken:string,pid:number,instanceId:string}} record @param {number} [requestTimeoutMs] */ +export async function probeBrokerHealth(record, requestTimeoutMs = 1_000) { + if (!Number.isSafeInteger(requestTimeoutMs) || requestTimeoutMs < 1 || requestTimeoutMs > 3_600_000) throw brokerInputError(); let protocol; try { - protocol = await connectZCodeBroker(record.endpoint, { brokerToken: record.brokerToken, ownerId: `health-${record.instanceId}`, requestTimeoutMs: 1_000 }); + protocol = await connectZCodeBroker(record.endpoint, { brokerToken: record.brokerToken, ownerId: `health-${record.instanceId}`, requestTimeoutMs }); const result = await protocol.request('broker/health', {}); return result?.ok === true && result.pid === record.pid && result.instanceId === record.instanceId; } catch { return false; } finally { await protocol?.close().catch(() => {}); } @@ -94,8 +108,9 @@ export async function ensureZCodeBroker(options) { if (!validWireOption(options?.maxFrameBytes, 16 * 1024 * 1024) || !validWireOption(options?.maxOutboundBytes, 64 * 1024 * 1024) || !validDrainOption(options?.drainTimeoutMs)) throw brokerInputError(); const storage = await resolveWorkspaceStorage(options); const brokerDirectory = join(storage.directory, 'broker'); - const profile = options.maxFrameBytes === undefined && options.maxOutboundBytes === undefined && options.drainTimeoutMs === undefined ? null : createHash('sha256').update(JSON.stringify([options.maxFrameBytes ?? null, options.maxOutboundBytes ?? null, options.drainTimeoutMs ?? null])).digest('hex').slice(0, 16); - const identityPath = join(brokerDirectory, profile ? `identity-${profile}.json` : 'identity.json'); + const identityName = brokerIdentityNameForWireOptions(options); + const profile = identityName === 'identity.json' ? null : identityName.slice('identity-'.length, -'.json'.length); + const identityPath = join(brokerDirectory, identityName); await ensurePrivateDirectory(brokerDirectory); return withFileLock(join(brokerDirectory, '.lock'), async () => { const existing = await readHealthyBrokerIdentity(identityPath); @@ -124,7 +139,7 @@ export async function ensureZCodeBroker(options) { export class ZCodeBroker { /** @param {{endpoint:string,ownershipPath?:string,brokerToken:string,launch:{command:string,args:string[],target?:string},workspace:string,launchCwd?:string,env?:NodeJS.ProcessEnv,idleTimeoutMs?:number,maxFrameBytes?:number,maxOutboundBytes?:number,drainTimeoutMs?:number,instanceId?:string}} options */ - constructor(options) { if (typeof options?.brokerToken !== 'string' || options.brokerToken.length < 32 || !validWireOption(options?.maxFrameBytes, 16 * 1024 * 1024) || !validWireOption(options?.maxOutboundBytes, 64 * 1024 * 1024) || !validDrainOption(options?.drainTimeoutMs) || isWindowsNamedPipe(options?.endpoint) && (typeof options?.ownershipPath !== 'string' || !options.ownershipPath)) throw brokerInputError(); this.options = options; this.ownershipPath = options.ownershipPath ?? `${options.endpoint}.owners.json`; this.ownershipStoreEstablished = false; this.server = null; this.protocol = null; this.protocolPromise = null; this.sockets = new Set(); this.socketWriters = new WeakMap(); this.authenticated = new WeakSet(); this.socketOwnerIds = new WeakMap(); this.sessionOwners = new Map(); this.permissionPending = new Map(); this.localTasks = new Set(); this.nextPermissionId = 1_000_000_000; this.owners = 0; this.activeSessions = new Set(); this.fastIdleRequested = false; this.idleTimer = null; this.closing = false; this.closePromise = null; } + constructor(options) { if (typeof options?.brokerToken !== 'string' || options.brokerToken.length < 32 || !validWireOption(options?.maxFrameBytes, 16 * 1024 * 1024) || !validWireOption(options?.maxOutboundBytes, 64 * 1024 * 1024) || !validDrainOption(options?.drainTimeoutMs) || isWindowsNamedPipe(options?.endpoint) && (typeof options?.ownershipPath !== 'string' || !options.ownershipPath)) throw brokerInputError(); this.options = options; this.ownershipPath = options.ownershipPath ?? `${options.endpoint}.owners.json`; this.ownershipStoreEstablished = false; this.server = null; this.protocol = null; this.protocolPromise = null; this.sockets = new Set(); this.socketWriters = new WeakMap(); this.authenticated = new WeakSet(); this.existingProtocolOnlySockets = new WeakSet(); this.socketOwnerIds = new WeakMap(); this.sessionOwners = new Map(); this.permissionPending = new Map(); this.localTasks = new Set(); this.nextPermissionId = 1_000_000_000; this.owners = 0; this.activeSessions = new Set(); this.fastIdleRequested = false; this.idleTimer = null; this.closing = false; this.closePromise = null; } async start() { if (this.server) return this; @@ -150,7 +165,7 @@ export class ZCodeBroker { const task = this.handleLocal(socket, line); this.localTasks.add(task); void task.then(() => { this.localTasks.delete(task); this.scheduleIdleShutdown(); }, () => { this.localTasks.delete(task); socket.destroy(); this.scheduleIdleShutdown(); }); } }); - socket.once('close', () => { clearTimeout(authTimer); this.socketWriters.get(socket)?.close(); this.sockets.delete(socket); for (const [id, pending] of this.permissionPending) if (pending.socket === socket) { clearTimeout(pending.timer); this.permissionPending.delete(id); pending.resolve(offeredDeny(pending.request)); } for (const owner of this.sessionOwners.values()) if (owner.socket === socket) owner.socket = null; if (this.authenticated.has(socket)) this.owners -= 1; this.scheduleIdleShutdown(); }); + socket.once('close', () => { clearTimeout(authTimer); this.socketWriters.get(socket)?.close(); this.sockets.delete(socket); this.existingProtocolOnlySockets.delete(socket); for (const [id, pending] of this.permissionPending) if (pending.socket === socket) { clearTimeout(pending.timer); this.permissionPending.delete(id); pending.resolve(offeredDeny(pending.request)); } for (const owner of this.sessionOwners.values()) if (owner.socket === socket) owner.socket = null; if (this.authenticated.has(socket)) this.owners -= 1; this.scheduleIdleShutdown(); }); } async handleLocal(socket, line) { @@ -158,11 +173,13 @@ export class ZCodeBroker { try { frame = JSON.parse(line); } catch { socket.destroy(); return; } if (!this.authenticated.has(socket)) { if (!frame || !Number.isSafeInteger(frame.id) || frame.method !== 'broker/auth' - || typeof frame.params?.token !== 'string' || !safeTokenEqual(frame.params.token, this.options.brokerToken) || typeof frame.params.ownerId !== 'string' || frame.params.ownerId.length < 16) { + || !frame.params || typeof frame.params !== 'object' || Object.keys(frame.params).some((key) => !['token', 'ownerId', 'existingProtocolOnly'].includes(key)) + || typeof frame.params.token !== 'string' || !safeTokenEqual(frame.params.token, this.options.brokerToken) || typeof frame.params.ownerId !== 'string' || frame.params.ownerId.length < 16 + || frame.params.existingProtocolOnly !== undefined && typeof frame.params.existingProtocolOnly !== 'boolean') { writeLocal(socket, { id: Number.isSafeInteger(frame?.id) ? frame.id : 0, error: { code: -32040, message: 'Broker authentication failed.' } }); socket.end(); return; } - clearTimeout(socket.authTimer); this.authenticated.add(socket); this.socketOwnerIds.set(socket, frame.params.ownerId); this.owners += 1; this.cancelIdleShutdown(); writeLocal(socket, { id: frame.id, result: { authenticated: true } }); return; + clearTimeout(socket.authTimer); this.authenticated.add(socket); if (frame.params.existingProtocolOnly === true) this.existingProtocolOnlySockets.add(socket); this.socketOwnerIds.set(socket, frame.params.ownerId); this.owners += 1; this.cancelIdleShutdown(); writeLocal(socket, { id: frame.id, result: { authenticated: true, ...(frame.params.existingProtocolOnly === true ? { existingProtocolOnly: true } : {}) } }); return; } if (frame && Number.isSafeInteger(frame.id) && !frame.method && (Object.hasOwn(frame, 'result') || Object.hasOwn(frame, 'error'))) { const pending = this.permissionPending.get(frame.id); @@ -188,7 +205,9 @@ export class ZCodeBroker { let claimToken = null; const previousOwner = existingOwner ? { ...existingOwner } : null; if (typeof requestedSessionId === 'string' && claimMethod) { claimToken = randomBytes(16).toString('hex'); this.sessionOwners.set(requestedSessionId, { ownerId, socket, claimToken }); } else if (existingOwner?.ownerId === ownerId) existingOwner.socket = socket; try { - const protocol = await this.getProtocol(); + let protocol; + if (this.existingProtocolOnlySockets.has(socket)) { if (!this.protocol) throw existingProtocolUnavailable(); protocol = this.protocol; } + else protocol = await this.getProtocol(); if (frame.method === 'session/send') protocol.beginTurn(frame.params.sessionId); let result; try { @@ -326,6 +345,7 @@ async function readOwnerStoreUnlocked(path, allowMissing) { let value; try { val function writeRequestError(socket, id, error) { const pluginError = error instanceof PluginError ? { code: error.code, category: error.category, remedy: error.remedy, details: error.details } : null; writeLocal(socket, { id, error: { code: -32000, message: error instanceof Error ? error.message : 'Broker request failed', ...(pluginError ? { data: { pluginError } } : {}) } }); } function ownerStoreInvalid(cause) { return new PluginError('ZCODE_OWNER_STORE_INVALID', 'The ZCode session owner store is missing or corrupt.', { category: 'storage', remedy: 'Reconcile ownership from validated durable job records before resuming sessions.', ...(cause === undefined ? {} : { cause }) }); } function ownerConflict() { return new PluginError('ZCODE_SESSION_OWNER_CONFLICT', 'The session already belongs to another broker owner.', { category: 'authorization', remedy: 'Use the original stable owner credential.' }); } +function existingProtocolUnavailable() { return new PluginError('ZCODE_BROKER_PROTOCOL_UNAVAILABLE', 'The existing ZCode protocol is unavailable.', { category: 'state', remedy: 'Retry after an active ZCode broker protocol is available.' }); } function brokerInputError() { return new PluginError('ZCODE_BROKER_INPUT_INVALID', 'ZCode broker input is invalid.', { category: 'validation', remedy: 'Provide a data root, workspace, endpoint, and launch target.' }); } function identityCleanupError(path, cause) { return new PluginError('ZCODE_BROKER_IDENTITY_CLEANUP_FAILED', 'The ZCode broker identity could not be safely removed.', { category: 'storage', remedy: 'Inspect the broker identity path and retry cleanup.', ...(cause === undefined ? {} : { cause }), details: { path } }); } diff --git a/scripts/zcode-companion.mjs b/scripts/zcode-companion.mjs index 3c4eef08..6fc8b403 100644 --- a/scripts/zcode-companion.mjs +++ b/scripts/zcode-companion.mjs @@ -19,7 +19,7 @@ import { createManagedZCodeClient } from './lib/zcode-client.mjs'; import { acknowledgeBackgroundStartup, startBackgroundWorker } from './lib/background-worker.mjs'; import { createInvocationStore, parseRecordedInvocation, requiresExecutionChoice } from './lib/invocation.mjs'; import { executeJob, readResultArtifact } from './lib/review.mjs'; -import { reconcileOwnedJobs, withWorkerLease } from './lib/recovery.mjs'; +import { reconcileOwnedJobs, scavengeWritableJobs, withWorkerLease } from './lib/recovery.mjs'; import { errorEnvelope, renderOutput } from './lib/render.mjs'; import { createForegroundSignalController } from './lib/signals.mjs'; import { createStateStore } from './lib/state.mjs'; @@ -102,7 +102,7 @@ async function startPublic(context) { } const permissionSnapshot = Object.freeze({ permissionMode: caller.permissionMode }); const transferSource = parsed.command === 'transfer' ? resolveTransferSource(parsed.options, caller) : undefined; - const job = await store.reserveJob({ workspace: cwd, ownerSessionId: caller.sessionId, ownerTurnId: caller.turnId, command: parsed.command, readOnly: parsed.command !== 'rescue', permissionSnapshot, ...(transferSource ? { codexThreadId: transferSource } : {}) }); + const job = await reservePublicJob(context, { workspace: cwd, ownerSessionId: caller.sessionId, ownerTurnId: caller.turnId, command: parsed.command, readOnly: parsed.command !== 'rescue', permissionSnapshot, ...(transferSource ? { codexThreadId: transferSource } : {}) }); if (parsed.command === 'transfer') { return executeTransfer({ job, workspace: job.workspace, dataRoot, store, sourceThreadId: /** @type {string} */ (transferSource), signal: context.signal, resolveLaunch: () => discoverLaunch(context.env), readThread: () => (context.dependencies?.readCodexThread ?? readCodexThread)(transferSource, codexAppServerOptions(context.env, job.workspace)), @@ -133,6 +133,28 @@ async function startPublic(context) { return executeWithWorkerLease({ ...context, job, spec }); } +/** @param {any} context @param {any} reservation */ +async function reservePublicJob(context, reservation) { + try { return await context.store.reserveJob(reservation); } + catch (error) { + if (reservation.readOnly || !(error instanceof PluginError) || error.code !== 'WRITABLE_JOB_EXISTS') throw error; + context.signal?.throwIfAborted(); + await scavengeWritableJobs({ + store: context.store, + dataRoot: context.dataRoot, + workspace: context.cwd, + createClient: async (job) => { + context.signal?.throwIfAborted(); + const launch = await discoverLaunch(context.env, context.dependencies); + context.signal?.throwIfAborted(); + return (context.dependencies?.createManagedZCodeClient ?? createManagedZCodeClient)({ dataRoot: context.dataRoot, workspace: job.workspace, launch, ownerId: ownerIdForSession(job.ownerSessionId), env: context.env, ...managedWireOptionsForJob(job) }); + }, + }); + context.signal?.throwIfAborted(); + return context.store.reserveJob(reservation); + } +} + /** @param {any} job */ function managedWireOptionsForJob(job) { return job?.command === 'transfer' ? { maxFrameBytes: TRANSFER_WIRE_LIMITS.maxFrameBytes, maxOutboundBytes: TRANSFER_WIRE_LIMITS.maxOutboundBytes, drainTimeoutMs: TRANSFER_WIRE_LIMITS.drainTimeoutMs } : {}; } diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 46aba752..6bc78816 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -12,6 +12,7 @@ import { createIdentityStore } from '../scripts/lib/identity.mjs'; import { createManagedZCodeClient, createZCodeClient, releaseManagedZCodeOwner } from '../scripts/lib/zcode-client.mjs'; import { ownerIdForSession } from '../scripts/lib/job-control.mjs'; import { brokerEndpointFor, ensureZCodeBroker, prioritizeBrokerOwnership, probeBrokerHealth, reconcileBrokerOwnership, writeBrokerIdentity } from '../scripts/zcode-broker.mjs'; +import { runCompanion } from '../scripts/zcode-companion.mjs'; const root = fileURLToPath(new URL('../', import.meta.url)); const fakeZCode = join(root, 'tests/fixtures/fake-zcode-cli.mjs'); @@ -63,6 +64,19 @@ async function workspace() { return { cwd, data, env: { PLUGIN_DATA: data } }; } +async function acceptedWritableJob({ data, cwd, ownerSessionId, remoteSessionId, peerEnv = {} }) { + const store = createStateStore({ dataRoot: data }); + let value = await store.reserveJob({ workspace: cwd, ownerSessionId, ownerTurnId: `turn-${ownerSessionId}`, command: 'rescue', readOnly: false, permissionSnapshot: { permissionMode: 'workspace-write' } }); + value = await store.claimJobWorker(cwd, value.id, { childPid: 999_999, workerLeaseId: 'd'.repeat(64) }); + const client = await createManagedZCodeClient({ dataRoot: data, workspace: cwd, launch: { command: process.execPath, args: [fakeZCode], target: fakeZCode }, ownerId: ownerIdForSession(ownerSessionId), env: { ...process.env, ...peerEnv } }); + await client.createSession({ workspace: cwd, sessionId: remoteSessionId }); + const sent = await client.send(remoteSessionId, 'recover this accepted turn'); + await client.close(); + value = await store.transitionJob(cwd, value.id, ['queued'], 'running', { startedAt: new Date().toISOString(), zcodeSessionId: remoteSessionId }); + value = await store.transitionJob(cwd, value.id, ['running'], 'running', { inputId: sent.inputId, startRevision: sent.stateRevision, beforeMessageIds: ['message-user-history', 'message-assistant-history'] }); + return { store, job: value }; +} + async function writeGateConfig(data, cwd, value) { const storage = await resolveWorkspaceStorage({ dataRoot: data, workspace: cwd }); await mkdir(join(storage.directory, 'config'), { recursive: true }); await writeFile(join(storage.directory, 'config/review-gate.json'), JSON.stringify(value)); } function stopFields(input) { const copy = { ...input }; delete copy.prompt; return copy; } function processAlive(pid) { try { process.kill(pid, 0); return true; } catch { return false; } } @@ -186,6 +200,53 @@ test('SessionEnd releases only its broker owner sessions and lets the idle broke assert.equal(processAlive(identity.pid), false, 'released idle broker must exit promptly'); }); +test('SessionEnd settles its writable job before generic owner release and preserves siblings', async () => { + const { cwd, data, env } = await workspace(); const record = join(data, 'settlement-order.jsonl'); const control = join(data, 'recovery-control.json'); + await writeFile(record, ''); await writeFile(control, JSON.stringify({ mode: 'active' })); + const { store, job } = await acceptedWritableJob({ data, cwd, ownerSessionId: 'settled-owner', remoteSessionId: 'settled-remote', peerEnv: { FAKE_ZCODE_RECORD: record, FAKE_ZCODE_RECOVERY_CONTROL: control } }); + let sibling = await store.reserveJob({ workspace: cwd, ownerSessionId: 'sibling-owner', ownerTurnId: 'sibling-turn', command: 'review', readOnly: true, permissionSnapshot: { permissionMode: 'default' } }); + sibling = await store.transitionJob(cwd, sibling.id, ['queued'], 'running', { startedAt: new Date().toISOString(), zcodeSessionId: 'sibling-remote' }); + const ended = await runHook('session-end-hook.mjs', { session_id: 'settled-owner', cwd, hook_event_name: 'SessionEnd', transcript_path: null, reason: 'other' }, env); + assert.equal(ended.code, 0, ended.stderr); assert.equal((await store.readJob(cwd, job.id)).status, 'cancelled'); assert.equal((await store.readJob(cwd, sibling.id)).status, 'running'); + const calls = (await readFile(record, 'utf8')).trim().split('\n').filter(Boolean).map(JSON.parse); const readIndex = calls.findIndex((call) => call.method === 'session/read' && call.params?.sessionId === 'settled-remote'); const stopIndex = calls.findIndex((call) => call.method === 'session/stop' && call.params?.sessionId === 'settled-remote'); + assert.ok(readIndex >= 0 && stopIndex > readIndex, 'durable read/stop settlement must precede generic release cleanup'); assert.ok(!calls.some((call) => call.params?.sessionId === 'sibling-remote')); +}); + +test('SessionEnd never starts a broker when exact existing settlement is unavailable', async () => { + const { cwd, data, env } = await workspace(); const record = join(data, 'no-spawn.jsonl'); const store = createStateStore({ dataRoot: data }); + let value = await store.reserveJob({ workspace: cwd, ownerSessionId: 'absent-owner', ownerTurnId: 'absent-turn', command: 'rescue', readOnly: false, permissionSnapshot: { permissionMode: 'workspace-write' } }); + value = await store.transitionJob(cwd, value.id, ['queued'], 'running', { startedAt: new Date().toISOString(), zcodeSessionId: 'absent-remote' }); + value = await store.transitionJob(cwd, value.id, ['running'], 'running', { inputId: 'accepted-input', startRevision: 1, beforeMessageIds: [] }); + const identity = createIdentityStore({ dataRoot: data }); await identity.createCallerContext({ sessionId: 'absent-owner', turnId: 'turn', workspace: cwd, permissionMode: 'default' }); + const ended = await runHook('session-end-hook.mjs', { session_id: 'absent-owner', cwd, hook_event_name: 'SessionEnd', transcript_path: null, reason: 'other' }, { ...env, ZCODE_PATH: fakeZCode, FAKE_ZCODE_RECORD: record }); + assert.equal(ended.code, 0, ended.stderr); assert.equal((await store.readJob(cwd, value.id)).status, 'running'); await assert.rejects(readFile(record, 'utf8'), { code: 'ENOENT' }); await assert.rejects(identity.resolveActiveTurn({ sessionId: 'absent-owner', workspace: cwd }), { code: 'ACTIVE_TURN_NOT_FOUND' }); + const hookSource = await readFile(join(root, 'hooks/session-end-hook.mjs'), 'utf8'); assert.match(hookSource, /createExistingManagedZCodeClient/); assert.doesNotMatch(hookSource, /maxFrameBytes|maxOutboundBytes|drainTimeoutMs/, 'writable Rescue is pinned to the default managed broker profile'); +}); + +test('SessionEnd existing-only settlement never lazily spawns ZCode and generic release never terminalizes the job', async () => { + const { cwd, data, env } = await workspace(); const record = join(data, 'historical-release.jsonl'); await writeFile(record, ''); const store = createStateStore({ dataRoot: data }); const ownerSessionId = 'historical-job-owner'; const ownerId = ownerIdForSession(ownerSessionId); + let value = await store.reserveJob({ workspace: cwd, ownerSessionId, ownerTurnId: 'historical-turn', command: 'rescue', readOnly: false, permissionSnapshot: { permissionMode: 'workspace-write' } }); value = await store.transitionJob(cwd, value.id, ['queued'], 'running', { startedAt: new Date().toISOString(), zcodeSessionId: 'historical-job-remote' }); value = await store.transitionJob(cwd, value.id, ['running'], 'running', { inputId: 'accepted-input', startRevision: 7, beforeMessageIds: [] }); + await reconcileBrokerOwnership({ dataRoot: data, workspace: cwd, ownerId, ownedSessionIds: [value.zcodeSessionId] }); await ensureZCodeBroker({ dataRoot: data, workspace: cwd, launch: { command: process.execPath, args: [fakeZCode], target: fakeZCode }, env: { ...process.env, FAKE_ZCODE_RECORD: record } }); + const ended = await runHook('session-end-hook.mjs', { session_id: ownerSessionId, cwd, hook_event_name: 'SessionEnd', transcript_path: null, reason: 'other' }, env); assert.equal(ended.code, 0, ended.stderr); const retained = await store.readJob(cwd, value.id); assert.equal(retained.status, 'running'); assert.match(retained.lastCancelError, /existing ZCode protocol is unavailable/i); assert.equal(await readFile(record, 'utf8'), ''); +}); + +test('SessionEnd remains bounded when existing stop acknowledgement is unavailable', async () => { + const { cwd, data, env } = await workspace(); const record = join(data, 'bounded-settlement.jsonl'); const control = join(data, 'bounded-control.json'); await writeFile(record, ''); await writeFile(control, JSON.stringify({ mode: 'active' })); + const { store, job } = await acceptedWritableJob({ data, cwd, ownerSessionId: 'bounded-owner', remoteSessionId: 'bounded-remote', peerEnv: { FAKE_ZCODE_RECORD: record, FAKE_ZCODE_RECOVERY_CONTROL: control, FAKE_ZCODE_SUPPRESS_METHOD: 'session/stop' } }); + const started = Date.now(); const ended = await runHook('session-end-hook.mjs', { session_id: 'bounded-owner', cwd, hook_event_name: 'SessionEnd', transcript_path: null, reason: 'other' }, env); + assert.equal(ended.code, 0, ended.stderr); assert.ok(Date.now() - started < 2_500); assert.ok(['running', 'cancelling'].includes((await store.readJob(cwd, job.id)).status)); +}); + +test('a failed SessionEnd stop is later settled by reservation scavenging before owner B is admitted', async (t) => { + const { cwd, data, env } = await workspace(); const record = join(data, 'fallback.jsonl'); const control = join(data, 'fallback-control.json'); await writeFile(record, ''); await writeFile(control, JSON.stringify({ mode: 'active' })); + const { store, job } = await acceptedWritableJob({ data, cwd, ownerSessionId: 'fallback-owner-a', remoteSessionId: 'fallback-remote', peerEnv: { FAKE_ZCODE_RECORD: record, FAKE_ZCODE_RECOVERY_CONTROL: control, FAKE_ZCODE_STOP_ERROR_PREFIX: 'fallback-remote' } }); + const storage = await resolveWorkspaceStorage({ dataRoot: data, workspace: cwd }); const brokerIdentity = JSON.parse(await readFile(join(storage.directory, 'broker/identity.json'), 'utf8')); t.after(() => { try { process.kill(brokerIdentity.pid, 'SIGTERM'); } catch { /* exited */ } }); + const ended = await runHook('session-end-hook.mjs', { session_id: 'fallback-owner-a', cwd, hook_event_name: 'SessionEnd', transcript_path: null, reason: 'other' }, env); assert.equal(ended.code, 0, ended.stderr); assert.ok(['running', 'cancelling'].includes((await store.readJob(cwd, job.id)).status)); + await writeFile(control, JSON.stringify({ mode: 'completed' })); + const admitted = await runCompanion(['rescue', '--background', '--fresh', 'owner B continues'], { cwd, env: { ...process.env, PLUGIN_DATA: data, ZCODE_DATA_ROOT: data, ZCODE_PATH: fakeZCode }, caller: { sessionId: 'fallback-owner-b', turnId: 'fallback-turn-b', permissionMode: 'workspace-write' }, autoLaunchBackground: false }); + assert.equal(admitted.type, 'background'); assert.equal(admitted.job.ownerSessionId, 'fallback-owner-b'); assert.ok(['succeeded', 'failed'].includes((await store.readJob(cwd, job.id)).status), 'reservation scavenging must safely terminalize the released orphan before admission'); +}); + test('SessionEnd drains deferred owner batches without touching siblings or looping on failures', async () => { const { cwd, data, env } = await workspace(); const record = join(data, 'zcode-calls.jsonl'); await writeFile(record, ''); const owner = ownerIdForSession('many'); const sibling = ownerIdForSession('sibling'); await reconcileBrokerOwnership({ dataRoot: data, workspace: cwd, ownerId: owner, ownedSessionIds: Array.from({ length: 17 }, (_, index) => `historical-${String(index).padStart(2, '0')}`) }); await reconcileBrokerOwnership({ dataRoot: data, workspace: cwd, ownerId: sibling, ownedSessionIds: ['sibling-session'] }); diff --git a/tests/integration/companion.test.mjs b/tests/integration/companion.test.mjs index 5b4e7c20..11a3b5b0 100644 --- a/tests/integration/companion.test.mjs +++ b/tests/integration/companion.test.mjs @@ -15,6 +15,7 @@ import { TRANSFER_WIRE_LIMITS } from '../../scripts/lib/transfer.mjs'; import { createManagedZCodeClient } from '../../scripts/lib/zcode-client.mjs'; import { resolveWorkspaceStorage } from '../../scripts/lib/workspace.mjs'; import { renderOutput } from '../../scripts/lib/render.mjs'; +import { withWorkerLease } from '../../scripts/lib/recovery.mjs'; import { runCompanion } from '../../scripts/zcode-companion.mjs'; const root = fileURLToPath(new URL('../..', import.meta.url)); @@ -71,6 +72,52 @@ async function waitFor(predicate, message) { throw new Error(message); } +/** @param {any} context @param {{ownerSessionId?:string,ownerTurnId?:string,workerLeaseId?:string,zcodeSessionId?:string}} [options] */ +async function reserveOrphan(context, options = {}) { + const store = createStateStore({ dataRoot: context.dataRoot }); + const ownerSessionId = options.ownerSessionId ?? 'departed-owner'; + const queued = await store.reserveJob({ workspace: context.workspace, ownerSessionId, ownerTurnId: options.ownerTurnId ?? 'departed-turn', command: 'rescue', readOnly: false, permissionSnapshot: { permissionMode: 'workspace-write' } }); + const workerLeaseId = options.workerLeaseId ?? 'd'.repeat(64); + await store.claimJobWorker(context.workspace, queued.id, { childPid: 999999, workerLeaseId }); + let running = await store.transitionJob(context.workspace, queued.id, ['queued'], 'running', { startedAt: new Date().toISOString(), zcodeSessionId: options.zcodeSessionId ?? 'orphan-session' }); + running = await store.transitionJob(context.workspace, queued.id, ['running'], 'running', { inputId: 'accepted-input', startRevision: 7, beforeMessageIds: ['historical'] }); + return { job: running, store, workerLeaseId }; +} + +/** @param {string} sessionId @param {string} [turnId] */ +function caller(sessionId, turnId = `${sessionId}-turn`) { return { sessionId, turnId, permissionMode: 'workspace-write' }; } + +/** @param {any[]} [calls] */ +function missingRemoteDependencies(calls = []) { + return { + discoverLaunch: async () => { calls.push('discover'); return { command: process.execPath, args: [fake], target: fake }; }, + createManagedZCodeClient: async (/** @type {any} */ options) => { + calls.push({ type: 'client', ownerId: options.ownerId }); + return { listSessions: async () => { calls.push('list'); return { sessions: [] }; }, close: async () => { calls.push('close'); } }; + }, + }; +} + +async function recoverForeignCompletion() { + const context = await fixture(); const ownerA = caller('departed-owner', 'owner-a-turn'); const ownerB = caller('new-owner', 'owner-b-turn'); + const { job: orphan, store } = await reserveOrphan(context, { ownerSessionId: ownerA.sessionId, ownerTurnId: ownerA.turnId }); + const dependencies = { + discoverLaunch: async () => ({ command: process.execPath, args: [fake], target: fake }), + createManagedZCodeClient: async (/** @type {any} */ options) => { + assert.equal(options.ownerId, ownerIdForSession(ownerA.sessionId)); + return { + listSessions: async () => ({ sessions: [{ sessionId: orphan.zcodeSessionId }] }), + readSession: async () => ({ projection: { status: 'completed' }, runtime: { stateRevision: 8 }, messages: [{ info: { role: 'assistant', messageId: 'recovered-answer', parentMessageId: orphan.inputId }, parts: [{ type: 'text', text: 'owner A recovered result' }] }] }), + close: async () => {}, + }; + }, + }; + const started = await runCompanion(['rescue', '--background', '--fresh', 'new owner work'], { cwd: context.workspace, env: context.env, caller: ownerB, dependencies }); + assert.equal(started.type, 'background'); assert.doesNotMatch(JSON.stringify(started), new RegExp(orphan.id)); + const recovered = await store.readJob(context.workspace, orphan.id); assert.equal(recovered.status, 'succeeded'); assert.equal(recovered.ownerSessionId, ownerA.sessionId); + return { context, orphan, ownerA, ownerB }; +} + test('module import has no CLI side effects', async () => { const result = await run(process.execPath, ['--input-type=module', '--eval', `await import(${JSON.stringify(new URL('../../scripts/zcode-companion.mjs', import.meta.url).href)}); process.stdout.write('imported')`]); assert.deepEqual({ code: result.code, stdout: result.stdout, stderr: result.stderr }, { code: 0, stdout: 'imported', stderr: '' }); @@ -283,6 +330,99 @@ test('status/list/result and queued cancellation enforce owned job semantics', a assert.equal(status.code, 0); assert.equal(status.json.job.status, 'cancelled'); }); +test('a new owner scavenges one orphan blocker and retries writable reservation exactly once', async () => { + const context = await fixture(); const { job: orphan, store } = await reserveOrphan(context); + /** @type {any[]} */ + const calls = []; + const output = await runCompanion(['rescue', '--background', '--fresh', 'repair after crash'], { cwd: context.workspace, env: context.env, caller: caller('new-owner'), dependencies: missingRemoteDependencies(calls) }); + assert.equal(output.type, 'background'); assert.notEqual(output.job.id, orphan.id); + assert.equal((await store.readJob(context.workspace, orphan.id)).status, 'failed'); + const jobs = await store.listJobs(context.workspace); assert.equal(jobs.filter((job) => ['queued', 'running', 'cancelling'].includes(job.status) && !job.readOnly).length, 1); + assert.equal(calls.filter((entry) => entry === 'list').length, 1); assert.doesNotMatch(JSON.stringify(output), new RegExp(orphan.id)); +}); + +test('a pre-aborted writable conflict propagates its reason before broker reconciliation', async () => { + const context = await fixture(); const { job: orphan, store } = await reserveOrphan(context); const controller = new AbortController(); const interruption = new PluginError('JOB_INTERRUPTED', 'abort before scavenging'); controller.abort(interruption); let discoveries = 0; let clients = 0; + await assert.rejects( + runCompanion(['rescue', '--background', '--fresh', 'do not scavenge'], { cwd: context.workspace, env: context.env, caller: caller('new-owner'), signal: controller.signal, dependencies: { discoverLaunch: async () => { discoveries += 1; throw new Error('must not discover'); }, createManagedZCodeClient: async () => { clients += 1; throw new Error('must not create'); } } }), + (error) => error === interruption, + ); + const storage = await resolveWorkspaceStorage({ dataRoot: context.dataRoot, workspace: context.workspace }); + await assert.rejects(readFile(join(storage.directory, 'broker', 'session-owners.json')), { code: 'ENOENT' }); + assert.equal(discoveries, 0); assert.equal(clients, 0); assert.deepEqual(await store.readJob(context.workspace, orphan.id), orphan); +}); + +test('an abort during writable-conflict scavenging propagates its reason before final reserve', async () => { + const context = await fixture(); const { job: orphan, store } = await reserveOrphan(context); const controller = new AbortController(); const interruption = new PluginError('JOB_INTERRUPTED', 'abort during scavenging'); let clients = 0; + await assert.rejects( + runCompanion(['rescue', '--background', '--fresh', 'stop before retry'], { cwd: context.workspace, env: context.env, caller: caller('new-owner'), signal: controller.signal, dependencies: { discoverLaunch: async () => { controller.abort(interruption); return { command: process.execPath, args: [fake], target: fake }; }, createManagedZCodeClient: async () => { clients += 1; throw new Error('must not create after abort'); } } }), + (error) => error === interruption, + ); + assert.equal(clients, 0); const jobs = await store.listJobs(context.workspace); assert.equal(jobs.length, 1); assert.equal(jobs[0].id, orphan.id); assert.equal(jobs[0].status, 'running'); +}); + +test('a live exact worker lease keeps a new owner blocked without remote inspection', async () => { + const context = await fixture(); const { job: orphan, store, workerLeaseId } = await reserveOrphan(context); let discoveries = 0; let clients = 0; + await withWorkerLease({ dataRoot: context.dataRoot, workspace: context.workspace, jobId: orphan.id, workerLeaseId }, async () => { + await assert.rejects( + runCompanion(['rescue', '--background', '--fresh', 'must wait'], { cwd: context.workspace, env: context.env, caller: caller('new-owner'), dependencies: { discoverLaunch: async () => { discoveries += 1; throw new Error('must not inspect'); }, createManagedZCodeClient: async () => { clients += 1; throw new Error('must not create'); } } }), + (error) => error instanceof PluginError && error.code === 'WRITABLE_JOB_EXISTS', + ); + }); + assert.equal(discoveries, 0); assert.equal(clients, 0); assert.deepEqual(await store.readJob(context.workspace, orphan.id), orphan); +}); + +test('an unacknowledged orphan stop preserves WRITABLE_JOB_EXISTS with an honest remedy', async () => { + const context = await fixture(); const { job: orphan, store } = await reserveOrphan(context); let stops = 0; + const dependencies = { + discoverLaunch: async () => ({ command: process.execPath, args: [fake], target: fake }), + createManagedZCodeClient: async () => ({ + listSessions: async () => ({ sessions: [{ sessionId: orphan.zcodeSessionId }] }), + readSession: async () => ({ projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] }), + stopSession: async () => { stops += 1; throw new Error('stop not acknowledged'); }, + close: async () => {}, + }), + }; + await assert.rejects( + runCompanion(['rescue', '--background', '--fresh', 'must remain blocked'], { cwd: context.workspace, env: context.env, caller: caller('new-owner'), dependencies }), + (error) => error instanceof PluginError && error.code === 'WRITABLE_JOB_EXISTS' && error.remedy === 'Retry later or inspect the redacted workspace list with $zcode:status --all.', + ); + const retained = await store.readJob(context.workspace, orphan.id); assert.equal(stops, 1); assert.equal(retained.status, 'running'); assert.match(retained.lastCancelError, /stop not acknowledged/); +}); + +test('two new owners racing through scavenging admit at most one writable rescue', async () => { + const context = await fixture(); const { job: orphan, store } = await reserveOrphan(context); + /** @type {any[]} */ + const calls = []; + const attempts = await Promise.allSettled(['new-owner-b', 'new-owner-c'].map((sessionId) => runCompanion(['rescue', '--background', '--fresh', `repair by ${sessionId}`], { cwd: context.workspace, env: context.env, caller: caller(sessionId), dependencies: missingRemoteDependencies(calls) }))); + assert.equal(attempts.filter((attempt) => attempt.status === 'fulfilled').length, 1); + const rejected = attempts.find((attempt) => attempt.status === 'rejected'); assert.ok(rejected && rejected.status === 'rejected'); assert.equal(rejected.reason.code, 'WRITABLE_JOB_EXISTS'); + assert.equal((await store.readJob(context.workspace, orphan.id)).status, 'failed'); + const activeWritable = (await store.listJobs(context.workspace)).filter((/** @type {any} */ job) => ['queued', 'running', 'cancelling'].includes(job.status) && !job.readOnly); + assert.equal(activeWritable.length, 1); assert.ok(['new-owner-b', 'new-owner-c'].includes(activeWritable[0].ownerSessionId)); +}); + +test('the owner that triggers scavenging cannot status result cancel or resume the recovered job', async () => { + const { context, orphan, ownerB } = await recoverForeignCompletion(); + for (const argv of [['status', orphan.id], ['result', orphan.id], ['cancel', orphan.id]]) { + await assert.rejects(runCompanion(argv, { cwd: context.workspace, env: context.env, caller: ownerB }), (error) => error instanceof PluginError && error.code === 'OWNED_JOB_NOT_FOUND'); + } + await assert.rejects(runCompanion(['rescue', '--resume', 'adopt foreign session'], { cwd: context.workspace, env: context.env, caller: ownerB }), (error) => error instanceof PluginError && error.code === 'RESUME_CANDIDATE_NOT_FOUND'); +}); + +test('status --all reports a scavenged foreign job only through redacted other-owner metadata', async () => { + const { context, orphan, ownerB } = await recoverForeignCompletion(); + const listed = await runCompanion(['status', '--all'], { cwd: context.workspace, env: context.env, caller: ownerB }); + const foreign = listed.jobs.find((/** @type {any} */ job) => job.id === orphan.id); assert.ok(foreign); assert.equal(foreign.owned, false); assert.equal(foreign.owner, 'other'); + assert.ok(!('ownerSessionId' in foreign) && !('ownerTurnId' in foreign) && !('permissionSnapshot' in foreign)); +}); + +test('a recovered foreign completion remains readable only by its original owner', async () => { + const { context, orphan, ownerA, ownerB } = await recoverForeignCompletion(); + await assert.rejects(runCompanion(['result', orphan.id], { cwd: context.workspace, env: context.env, caller: ownerB }), (error) => error instanceof PluginError && error.code === 'OWNED_JOB_NOT_FOUND'); + const result = await runCompanion(['result', orphan.id], { cwd: context.workspace, env: context.env, caller: ownerA }); assert.equal(result.result, 'owner A recovered result'); +}); + test('caller context is mandatory and diagnostics do not leak tokens or fake permission secrets', async () => { const context = await fixture(); const missing = await companion(context, ['review'], { ZCODE_CALLER_CONTEXT: context.caller }, {}); diff --git a/tests/recovery.test.mjs b/tests/recovery.test.mjs index 22863d8d..6e5957fb 100644 --- a/tests/recovery.test.mjs +++ b/tests/recovery.test.mjs @@ -89,7 +89,8 @@ async function waitForJob(store, workspace, jobId, predicate, timeoutMs = 5_000) async function orphanJob(fixture, options = {}) { const store = createStateStore({ dataRoot: fixture.dataRoot }); - const job = await store.reserveJob({ workspace: fixture.workspace, ownerSessionId: 'owner', ownerTurnId: options.turnId ?? 'orphan', command: options.command ?? 'rescue', ...(options.command === 'transfer' ? { codexThreadId: 'owner' } : {}), readOnly: options.readOnly ?? false, permissionSnapshot: { permissionMode: 'workspace-write' } }); + const ownerSessionId = options.ownerSessionId ?? 'owner'; + const job = await store.reserveJob({ workspace: fixture.workspace, ownerSessionId, ownerTurnId: options.turnId ?? 'orphan', command: options.command ?? 'rescue', ...(options.command === 'transfer' ? { codexThreadId: ownerSessionId } : {}), readOnly: options.readOnly ?? false, permissionSnapshot: { permissionMode: 'workspace-write' } }); const workerLeaseId = options.workerLeaseId ?? 'd'.repeat(64); if (options.claim !== false) await store.claimJobWorker(fixture.workspace, job.id, { childPid: 999999, workerLeaseId }); if (options.status === 'queued') return { job: await store.readJob(fixture.workspace, job.id), store, workerLeaseId }; @@ -108,6 +109,130 @@ function recoveryClient(job, options = {}) { }; } +test('cross-owner scavenging derives maintenance ownership from each durable writable blocker', async () => { + const fixture = await context(); const reconciled = []; const clientOwners = []; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + for (const ownerSessionId of ['departed-owner-a', 'departed-owner-b']) { + const { job, store } = await orphanJob(fixture, { ownerSessionId, turnId: ownerSessionId }); + await scavengeWritableJobs({ + store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, + reconcileOwnership: async (input) => { reconciled.push(input); }, + createClient: async (current, ownerId) => { clientOwners.push({ jobId: current.id, ownerId }); return recoveryClient(current, { missing: true }); }, + }); + assert.equal((await store.readJob(fixture.workspace, job.id)).status, 'failed'); + } + assert.deepEqual(reconciled.map(({ ownerId, ownedSessionIds }) => ({ ownerId, ownedSessionIds })), [ + { ownerId: ownerIdForSession('departed-owner-a'), ownedSessionIds: ['orphan-session'] }, + { ownerId: ownerIdForSession('departed-owner-b'), ownedSessionIds: ['orphan-session'] }, + ]); + assert.deepEqual(clientOwners.map(({ ownerId }) => ownerId), [ownerIdForSession('departed-owner-a'), ownerIdForSession('departed-owner-b')]); +}); + +test('workspace scavenging never inspects a blocker whose exact worker lease is held', async () => { + const fixture = await context(); const { job, store, workerLeaseId } = await orphanJob(fixture); let ownershipCalls = 0; let clientCalls = 0; + const { scavengeWritableJobs, withWorkerLease } = await import('../scripts/lib/recovery.mjs'); + await withWorkerLease({ dataRoot: fixture.dataRoot, workspace: fixture.workspace, jobId: job.id, workerLeaseId }, () => scavengeWritableJobs({ + store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, + reconcileOwnership: async () => { ownershipCalls += 1; }, + createClient: async () => { clientCalls += 1; throw new Error('held lease must prevent inspection'); }, + })); + assert.equal(ownershipCalls, 0); assert.equal(clientCalls, 0); + assert.equal((await store.readJob(fixture.workspace, job.id)).status, 'running'); +}); + +test('workspace scavenging ignores read-only and terminal jobs', async () => { + const fixture = await context(); const store = createStateStore({ dataRoot: fixture.dataRoot }); + const terminal = await store.reserveJob({ workspace: fixture.workspace, ownerSessionId: 'old-terminal', ownerTurnId: 'terminal', command: 'rescue', readOnly: false, permissionSnapshot: { permissionMode: 'workspace-write' } }); + await store.transitionJob(fixture.workspace, terminal.id, ['queued'], 'failed', { error: { message: 'already done' }, finishedAt: new Date().toISOString(), exitCode: 1 }); + const readOnly = await orphanJob(fixture, { ownerSessionId: 'old-reader', turnId: 'reader', readOnly: true }); + const writable = await orphanJob(fixture, { ownerSessionId: 'old-writer', turnId: 'writer' }); const inspected = []; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async (job) => { inspected.push(job.id); return recoveryClient(job, { missing: true }); } }); + assert.deepEqual(inspected, [writable.job.id]); + assert.equal((await store.readJob(fixture.workspace, terminal.id)).status, 'failed'); + assert.equal((await store.readJob(fixture.workspace, readOnly.job.id)).status, 'running'); +}); + +test('workspace scavenging preserves an unclaimed reservation through claim grace and fails it after expiry', async () => { + const fixture = await context(); const now = Date.now(); + const { job, store } = await orphanJob(fixture, { claim: false, status: 'queued' }); const storage = await resolveWorkspaceStorage({ dataRoot: fixture.dataRoot, workspace: fixture.workspace }); + await atomicWriteJson(join(storage.directory, 'jobs', `${job.id}.json`), { ...job, createdAt: new Date(now - 60_000).toISOString() }); + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + const input = { store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, now: () => now, reconcileOwnership: async () => { throw new Error('queued reservation needs no ownership'); }, createClient: async () => { throw new Error('queued reservation needs no client'); } }; + await scavengeWritableJobs(input); assert.equal((await store.readJob(fixture.workspace, job.id)).status, 'queued'); + await atomicWriteJson(join(storage.directory, 'jobs', `${job.id}.json`), { ...(await store.readJob(fixture.workspace, job.id)), createdAt: new Date(now - 600_000).toISOString() }); + await scavengeWritableJobs(input); assert.equal((await store.readJob(fixture.workspace, job.id)).status, 'failed'); +}); + +test('workspace scavenging stops an active orphan and rereads completion before terminalizing', async () => { + const fixture = await context(); const { job, store } = await orphanJob(fixture); let reads = 0; let stops = 0; + const completed = { projection: { status: 'completed' }, runtime: { stateRevision: 8 }, messages: [{ info: { role: 'assistant', messageId: 'answer', parentMessageId: 'accepted-input' }, parts: [{ type: 'text', text: 'completion won the stop race' }] }] }; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async () => ({ + listSessions: async () => ({ sessions: [{ sessionId: job.zcodeSessionId }] }), + readSession: async () => { reads += 1; return reads === 1 ? { projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] } : completed; }, + stopSession: async () => { stops += 1; }, close: async () => {}, + }) }); + const recovered = await store.readJob(fixture.workspace, job.id); + assert.equal(recovered.status, 'succeeded'); assert.equal(stops, 1); assert.equal(reads, 2); assert.ok(recovered.resultArtifact); + const storage = await resolveWorkspaceStorage({ dataRoot: fixture.dataRoot, workspace: fixture.workspace }); + assert.equal(await readFile(join(storage.directory, recovered.resultArtifact), 'utf8'), 'completion won the stop race'); +}); + +test('acknowledged stop cancels a cancelling orphan when post-stop completion has no valid result', async () => { + const fixture = await context(); const { job, store } = await orphanJob(fixture, { status: 'cancelling' }); let reads = 0; let stops = 0; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async () => ({ + listSessions: async () => ({ sessions: [{ sessionId: job.zcodeSessionId }] }), + readSession: async () => { reads += 1; return { projection: { status: reads === 1 ? 'running' : 'completed' }, runtime: { stateRevision: 8 }, messages: [] }; }, + stopSession: async () => { stops += 1; }, close: async () => {}, + }) }); + const recovered = await store.readJob(fixture.workspace, job.id); + assert.equal(recovered.status, 'cancelled'); assert.equal(stops, 1); assert.equal(reads, 2); assert.equal(recovered.resultArtifact, undefined); +}); + +test('workspace scavenging retains the writable guard when active stop is unacknowledged', async () => { + const fixture = await context(); const { job, store } = await orphanJob(fixture); const longError = `stop refused ${'x'.repeat(3_000)}`; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async () => recoveryClient(job, { stopError: new Error(longError) }) }); + const recovered = await store.readJob(fixture.workspace, job.id); + assert.equal(recovered.status, 'running'); assert.match(recovered.lastCancelError, /stop refused/); assert.ok(recovered.lastCancelError.length <= 2_048); + await assert.rejects(store.reserveJob({ workspace: fixture.workspace, ownerSessionId: 'new-owner', ownerTurnId: 'new', command: 'rescue', readOnly: false, permissionSnapshot: { permissionMode: 'workspace-write' } }), { code: 'WRITABLE_JOB_EXISTS' }); +}); + +test('workspace scavenging maps paused running to failed but requires stop acknowledgement for cancelling', async () => { + for (const [status, stopAcknowledged, expected] of [['running', true, 'failed'], ['cancelling', true, 'cancelled'], ['cancelling', false, 'running']]) { + const fixture = await context(); const { job, store } = await orphanJob(fixture, { status, turnId: `${status}-${stopAcknowledged}` }); let stops = 0; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async () => recoveryClient(job, { + snapshot: { projection: { status: 'paused' }, runtime: { stateRevision: 8 }, messages: [] }, onStop: () => { stops += 1; }, ...(stopAcknowledged ? {} : { stopError: new Error('paused stop refused') }), + }) }); + const recovered = await store.readJob(fixture.workspace, job.id); + assert.equal(recovered.status, expected, `${status}/${stopAcknowledged}`); assert.equal(stops, status === 'cancelling' ? 1 : 0); + } +}); + +test('workspace scavenging fails an orphan whose persisted remote session is missing', async () => { + const fixture = await context(); const { job, store } = await orphanJob(fixture); let stops = 0; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async () => recoveryClient(job, { missing: true, onStop: () => { stops += 1; } }) }); + assert.equal((await store.readJob(fixture.workspace, job.id)).status, 'failed'); assert.equal(stops, 0); +}); + +test('terminal completion racing orphan settlement is never overwritten', async () => { + const fixture = await context(); const { job, store } = await orphanJob(fixture); let raced = false; + const wrapped = { ...store, transitionJob: async (...args) => { + if (!raced && args[3] === 'failed') { + raced = true; + await store.transitionJob(fixture.workspace, job.id, ['running'], 'succeeded', { resultArtifact: `results/${job.id}.md`, finishedAt: new Date().toISOString(), exitCode: 0 }); + } + return store.transitionJob(...args); + } }; + const { scavengeWritableJobs } = await import('../scripts/lib/recovery.mjs'); + await scavengeWritableJobs({ store: wrapped, dataRoot: fixture.dataRoot, workspace: fixture.workspace, reconcileOwnership: async () => {}, createClient: async () => recoveryClient(job, { snapshot: { projection: { status: 'paused' }, runtime: { stateRevision: 8 }, messages: [] } }) }); + assert.equal((await store.readJob(fixture.workspace, job.id)).status, 'succeeded'); +}); + test('background preparation failures terminalize the reservation and release the writable slot', async () => { for (const dependency of ['writeJobSpec', 'createExecutionCapability']) { const fixture = await context(); const failure = Object.assign(new Error(`${dependency} failed`), { code: 'EIO' }); @@ -218,7 +343,7 @@ test('cancelling recovery distinguishes completed, stopped, active-acked, and ac await reconcileOwnedJobs({ store, dataRoot: fixture.dataRoot, workspace: fixture.workspace, ownerSessionId: 'owner', reconcileOwnership: async () => {}, createClient: async () => recoveryClient(job, { snapshot, onStop: () => { stops += 1; }, ...(mode === 'active-unacked' ? { stopError: new Error('retry stop') } : {}) }) }); const recovered = await store.readJob(fixture.workspace, job.id); assert.equal(recovered.status, mode === 'completed' ? 'succeeded' : mode === 'active-unacked' ? 'running' : 'cancelled', mode); - assert.equal(stops, mode.startsWith('active') ? 1 : 0, mode); + assert.equal(stops, mode === 'paused' || mode.startsWith('active') ? 1 : 0, mode); if (mode === 'active-unacked') assert.match(recovered.lastCancelError, /retry stop/); } }); @@ -264,7 +389,7 @@ test('orphan Transfer stops a known remote session before failure and retains it }); test('a crashed real background worker reconciles remote terminal state without failing remote active work', async (t) => { - for (const [remoteMode, expectedStatus] of [['completed', 'succeeded'], ['stopped', 'cancelled'], ['missing', 'failed']]) { + for (const [remoteMode, expectedStatus] of [['completed', 'succeeded'], ['stopped', 'failed'], ['missing', 'failed']]) { const fixture = await context(); const control = join(fixture.root, 'recovery-control.json'); await writeFile(control, JSON.stringify({ mode: 'active' })); const env = { ...fixture.env, ZCODE_PATH: fakeZCode, FAKE_ZCODE_RECOVERY_CONTROL: control, FAKE_ZCODE_SUPPRESS_FIRST_COMPLETION: '1' }; const started = await runCompanion(['rescue', '--background', '--fresh', `recover ${remoteMode}`], { cwd: fixture.workspace, env, authorization: { callerContext: fixture.callerContext }, autoLaunchBackground: true }); diff --git a/tests/release-contracts.test.mjs b/tests/release-contracts.test.mjs index f41a7d94..7bcbf225 100644 --- a/tests/release-contracts.test.mjs +++ b/tests/release-contracts.test.mjs @@ -66,6 +66,32 @@ test('Unreleased changelog records progress and interruption behavior without a assert.equal(JSON.parse(read('package.json')).version, '0.1.0'); }); +test('release docs explain safe orphan settlement without weakening job ownership', () => { + const english = read('README.md'); + assert.match(english, /SessionEnd.{0,160}best-effort/i); + assert.match(english, /claimed queued reservation.{0,160}worker lease is held/i); + assert.match(english, /later Rescue.{0,200}provably orphaned/i); + assert.match(english, /does not transfer ownership/i); + assert.match(english, /reservation-time crash fallback.{0,240}held exact worker lease.{0,160}writable guard/i); + assert.match(english, /unacknowledged `session\/stop`.{0,160}writable guard/i); + assert.match(english, /\$zcode:status --all.{0,160}redacted/i); + + const chinese = read('README.zh-CN.md'); + assert.match(chinese, /SessionEnd.{0,160}best-effort/i); + assert.match(chinese, /已 claim 的 queued reservation.{0,160}worker lease/i); + assert.match(chinese, /后续 Rescue.{0,200}可证明的孤儿/i); + assert.match(chinese, /不会转移 ownership/i); + assert.match(chinese, /预留时的崩溃回退.{0,240}持有的精确 worker lease.{0,160}writable guard/i); + assert.match(chinese, /未确认的 `session\/stop`.{0,160}writable guard/i); + assert.match(chinese, /\$zcode:status --all.{0,160}脱敏/i); + + const changelog = read('CHANGELOG.md'); + assert.match(changelog, /safe orphan settlement/i); + assert.match(changelog, /SessionEnd/i); + assert.match(changelog, /reservation-time crash fallback/i); + assert.equal(JSON.parse(read('package.json')).version, '0.1.0'); +}); + test('marketplace catalog and publisher describe an installable vitry snapshot', () => { const catalog = JSON.parse(read('marketplace/.agents/plugins/marketplace.json')); assert.equal(catalog.name, 'vitry'); diff --git a/tests/session-end.test.mjs b/tests/session-end.test.mjs new file mode 100644 index 00000000..71a003f4 --- /dev/null +++ b/tests/session-end.test.mjs @@ -0,0 +1,246 @@ +// @ts-nocheck +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { mkdir, mkdtemp, readFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { basename, join } from 'node:path'; +import test from 'node:test'; +import { fileURLToPath } from 'node:url'; + +import { PluginError } from '../scripts/lib/errors.mjs'; +import { ownerIdForSession } from '../scripts/lib/job-control.mjs'; +import { settleEndedOwnerWritableJob, withWorkerLease } from '../scripts/lib/recovery.mjs'; +import { executeJob } from '../scripts/lib/review.mjs'; +import { createStateStore } from '../scripts/lib/state.mjs'; +import { resolveWorkspaceStorage } from '../scripts/lib/workspace.mjs'; + +const cancelLockHolder = fileURLToPath(new URL('./fixtures/cancel-lock-holder.mjs', import.meta.url)); + +async function fixture() { + const root = await mkdtemp(join(tmpdir(), 'zcode-session-end-')); + const workspace = join(root, 'workspace'); + const dataRoot = join(root, 'data'); + await mkdir(workspace); + return { root, workspace, dataRoot, store: createStateStore({ dataRoot }) }; +} + +async function job(input, options = {}) { + let value = await input.store.reserveJob({ + workspace: input.workspace, + ownerSessionId: options.ownerSessionId ?? 'owner-a', + ownerTurnId: options.ownerTurnId ?? Math.random().toString(16), + command: options.command ?? 'rescue', + readOnly: options.readOnly ?? false, + permissionSnapshot: { permissionMode: 'workspace-write' }, + }); + if (options.claim !== false) { + value = await input.store.claimJobWorker(input.workspace, value.id, { + childPid: 999_999, + workerLeaseId: options.workerLeaseId ?? 'd'.repeat(64), + }); + } + if (options.status === 'queued') return value; + value = await input.store.transitionJob(input.workspace, value.id, ['queued'], 'running', { + startedAt: new Date().toISOString(), + ...(options.accepted === false ? {} : { zcodeSessionId: options.zcodeSessionId ?? 'remote-a' }), + }); + if (options.boundary !== false) value = await input.store.transitionJob(input.workspace, value.id, ['running'], 'running', { + inputId: 'input-a', startRevision: 7, beforeMessageIds: ['historical'], + }); + if (options.status === 'cancelling') value = await input.store.transitionJob(input.workspace, value.id, ['running'], 'cancelling'); + return value; +} + +function completed(text = 'session end completion') { + return { + projection: { status: 'completed' }, runtime: { stateRevision: 8 }, + messages: [{ info: { role: 'assistant', messageId: 'answer', parentMessageId: 'input-a' }, parts: [{ type: 'text', text }] }], + }; +} + +function clientFor(value, options = {}) { + let reads = 0; + return { + readSession: async (sessionId) => { + assert.equal(sessionId, value.zcodeSessionId); reads += 1; + if (options.readError) throw options.readError; + return options.reads?.[reads - 1] ?? { projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] }; + }, + stopSession: async (sessionId) => { + assert.equal(sessionId, value.zcodeSessionId); options.onStop?.(); + if (options.stopError) throw options.stopError; + }, + close: async () => { options.onClose?.(); }, + }; +} + +function executorClient(text = 'executor result') { + return { + createSession: async () => ({ session: { sessionId: 'remote-a' }, settings: { model: { current: { providerId: 'p', modelId: 'm' }, available: [] } }, messages: [] }), + setPermissionHandler: () => {}, subscribe: () => () => {}, + send: async () => ({ inputId: 'input-a', stateRevision: 7 }), waitForCompletion: async () => {}, + readSession: async () => ({ messages: [{ info: { role: 'assistant', messageId: 'executor-answer', parentMessageId: 'input-a' }, parts: [{ type: 'text', text }] }] }), + close: async () => {}, + }; +} + +async function settle(input, createClient, ownerSessionId = 'owner-a') { + return settleEndedOwnerWritableJob({ + store: input.store, dataRoot: input.dataRoot, workspace: input.workspace, + ownerSessionId, lockTimeoutMs: 0, requestTimeoutMs: 250, createClient, + }); +} + +test('SessionEnd cancels an unclaimed queued reservation and prevents a later claim', async () => { + const input = await fixture(); const value = await job(input, { claim: false, status: 'queued' }); let clients = 0; + await settle(input, async () => { clients += 1; throw new Error('queued jobs need no client'); }); + assert.equal((await input.store.readJob(input.workspace, value.id)).status, 'cancelled'); + assert.equal(clients, 0); + await assert.rejects(input.store.claimJobWorker(input.workspace, value.id, { childPid: process.pid, workerLeaseId: 'a'.repeat(64) }), { code: 'WORKER_LEASE_CONFLICT' }); +}); + +test('SessionEnd leaves a held claimed queued lease but cancels it after the lease is free', async () => { + const input = await fixture(); const lease = 'e'.repeat(64); const value = await job(input, { status: 'queued', workerLeaseId: lease }); + await withWorkerLease({ dataRoot: input.dataRoot, workspace: input.workspace, jobId: value.id, workerLeaseId: lease }, () => settle(input, async () => { throw new Error('queued jobs need no client'); })); + assert.equal((await input.store.readJob(input.workspace, value.id)).status, 'queued'); + await settle(input, async () => { throw new Error('queued jobs need no client'); }); + assert.equal((await input.store.readJob(input.workspace, value.id)).status, 'cancelled'); +}); + +test('SessionEnd publishes a completed first read with an artifact and never stops', async () => { + const input = await fixture(); const value = await job(input); let stops = 0; let derived; + await settle(input, async (current, ownerId) => { + assert.equal(current.id, value.id); assert.equal(current.ownerSessionId, 'owner-a'); derived = ownerId; + return clientFor(current, { reads: [completed('already complete')], onStop: () => { stops += 1; } }); + }); + const stored = await input.store.readJob(input.workspace, value.id); + assert.equal(derived, ownerIdForSession('owner-a')); assert.equal(stored.status, 'succeeded'); assert.equal(stops, 0); + const storage = await resolveWorkspaceStorage({ dataRoot: input.dataRoot, workspace: input.workspace }); + assert.equal(await readFile(join(storage.directory, stored.resultArtifact), 'utf8'), 'already complete'); +}); + +test('SessionEnd cancels an active turn only after acknowledged stop and noncompleted reread', async () => { + const input = await fixture(); const value = await job(input); let stops = 0; let closes = 0; + await settle(input, async (current) => clientFor(current, { + reads: [{ projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] }, { projection: { status: 'paused' }, runtime: { stateRevision: 8 }, messages: [] }], + onStop: () => { stops += 1; }, onClose: () => { closes += 1; }, + })); + assert.equal((await input.store.readJob(input.workspace, value.id)).status, 'cancelled'); assert.equal(stops, 1); assert.equal(closes, 1); +}); + +test('SessionEnd preserves a completion that races an acknowledged stop', async () => { + const input = await fixture(); const value = await job(input); let stops = 0; + await settle(input, async (current) => clientFor(current, { + reads: [{ projection: { status: 'waiting' }, runtime: { stateRevision: 8 }, messages: [] }, completed('race won')], onStop: () => { stops += 1; }, + })); + const stored = await input.store.readJob(input.workspace, value.id); assert.equal(stored.status, 'succeeded'); assert.equal(stops, 1); +}); + +test('SessionEnd keeps jobs nonterminal when the existing client, read, or stop is unavailable', async () => { + for (const scenario of ['null-client', 'read-timeout', 'stop-failure']) { + const input = await fixture(); const value = await job(input, { ownerTurnId: scenario }); let closes = 0; + await settle(input, async (current) => scenario === 'null-client' ? null : clientFor(current, { + ...(scenario === 'read-timeout' ? { readError: new PluginError('ZCODE_REQUEST_TIMEOUT', 'read timed out', { category: 'timeout', remedy: 'retry' }) } : {}), + ...(scenario === 'stop-failure' ? { stopError: new Error('stop refused') } : {}), onClose: () => { closes += 1; }, + })); + const stored = await input.store.readJob(input.workspace, value.id); + assert.ok(['running', 'cancelling'].includes(stored.status), scenario); + assert.ok(typeof stored.lastCancelError === 'string' && stored.lastCancelError.length > 0 && stored.lastCancelError.length <= 2_048, scenario); + if (scenario === 'null-client') assert.match(stored.lastCancelError, /existing ZCode broker is unavailable/i); + if (scenario === 'read-timeout') assert.match(stored.lastCancelError, /read timed out/i); + if (scenario === 'stop-failure') assert.match(stored.lastCancelError, /stop refused/i); + assert.equal(closes, scenario === 'null-client' ? 0 : 1); + } +}); + +test('SessionEnd maintenance failure never overwrites a terminal executor race', async () => { + const input = await fixture(); const value = await job(input); let raced = false; + const wrapped = { + ...input.store, + transitionJob: async (workspace, jobId, expected, next, patch = {}) => { + if (!raced && next === 'running' && patch.lastCancelError) { + raced = true; + await input.store.transitionJob(workspace, jobId, ['running'], 'failed', { error: { message: 'executor won maintenance failure race' }, finishedAt: new Date().toISOString(), exitCode: 1 }); + } + return input.store.transitionJob(workspace, jobId, expected, next, patch); + }, + }; + await settle({ ...input, store: wrapped }, async (current) => clientFor(current, { stopError: new Error('stop failed late') })); + const stored = await input.store.readJob(input.workspace, value.id); assert.equal(stored.status, 'failed'); assert.equal(stored.error.message, 'executor won maintenance failure race'); assert.equal(stored.lastCancelError, undefined); +}); + +test('SessionEnd bounds multibyte maintenance failures by UTF-8 bytes without splitting emoji', async () => { + const input = await fixture(); const value = await job(input); const failure = `停止失败🚫${'诊断🚧'.repeat(1_000)}`; + await settle(input, async (current) => clientFor(current, { stopError: new Error(failure) })); + const stored = await input.store.readJob(input.workspace, value.id); + assert.match(stored.lastCancelError, /^停止失败🚫诊断🚧/); assert.ok(Buffer.byteLength(stored.lastCancelError, 'utf8') <= 2_048); assert.doesNotMatch(stored.lastCancelError, /\uFFFD/); +}); + +test('SessionEnd ignores foreign-owner and read-only jobs', async () => { + const foreignInput = await fixture(); const foreign = await job(foreignInput, { ownerSessionId: 'owner-b' }); let clients = 0; + await settle(foreignInput, async () => { clients += 1; throw new Error('must not inspect foreign job'); }); + assert.equal((await foreignInput.store.readJob(foreignInput.workspace, foreign.id)).status, 'running'); + + const readOnlyInput = await fixture(); const readOnly = await job(readOnlyInput, { readOnly: true }); + await settle(readOnlyInput, async () => { clients += 1; throw new Error('must not inspect read-only job'); }); + assert.equal((await readOnlyInput.store.readJob(readOnlyInput.workspace, readOnly.id)).status, 'running'); assert.equal(clients, 0); +}); + +test('SessionEnd cancellation-lock contention returns immediately without remote work', async (t) => { + const input = await fixture(); const value = await job(input); let clients = 0; + const holder = spawn(process.execPath, [cancelLockHolder, input.dataRoot, input.workspace, value.id], { stdio: ['ignore', 'pipe', 'pipe'] }); + t.after(() => { try { holder.kill('SIGTERM'); } catch { /* exited */ } }); + await new Promise((resolve, reject) => { holder.stdout.once('data', resolve); holder.once('error', reject); holder.once('exit', (code) => reject(new Error(`lock holder exited ${code}`))); }); + const started = Date.now(); await settle(input, async () => { clients += 1; return null; }); + assert.ok(Date.now() - started < 250); assert.equal(clients, 0); assert.equal((await input.store.readJob(input.workspace, value.id)).status, 'running'); +}); + +test('SessionEnd never overwrites a terminal executor race', async () => { + const input = await fixture(); const value = await job(input); let raced = false; + const wrapped = { + ...input.store, + transitionJob: async (workspace, jobId, expected, next, patch = {}) => { + if (!raced && next === 'cancelling') { + raced = true; + await input.store.transitionJob(workspace, jobId, ['running'], 'failed', { error: { message: 'executor failed first' }, finishedAt: new Date().toISOString(), exitCode: 1 }); + } + return input.store.transitionJob(workspace, jobId, expected, next, patch); + }, + }; + await settle({ ...input, store: wrapped }, async (current) => clientFor(current, { reads: [{ projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] }, { projection: { status: 'paused' }, runtime: { stateRevision: 8 }, messages: [] }] })); + const stored = await input.store.readJob(input.workspace, value.id); assert.equal(stored.status, 'failed'); assert.equal(stored.error.message, 'executor failed first'); +}); + +test('executeJob holds the cancellation lock across result artifact publication', async () => { + const input = await fixture(); const reservation = await job(input, { claim: false, status: 'queued' }); let observed; + const output = await executeJob({ + job: reservation, workspace: input.workspace, dataRoot: input.dataRoot, store: input.store, client: executorClient(), task: 'finish', + syncDirectory: async (directory) => { + if (basename(directory) !== 'results') return; + observed = await settle(input, async (current) => clientFor(current, { reads: [{ projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] }, { projection: { status: 'paused' }, runtime: { stateRevision: 8 }, messages: [] }] })); + }, + }); + assert.equal(observed.status, 'running', 'nonblocking SessionEnd must lose while executor publishes under the cancellation lock'); assert.equal(output.job.status, 'succeeded'); +}); + +test('executeJob respects a SessionEnd completion winner without rewriting its result artifact', async () => { + const input = await fixture(); const reservation = await job(input, { claim: false, status: 'queued' }); + const output = await executeJob({ + job: reservation, workspace: input.workspace, dataRoot: input.dataRoot, store: input.store, client: executorClient('late executor result'), task: 'finish', + onBoundaryPersisted: async (running) => { + await settle(input, async (current) => clientFor(current, { reads: [{ ...completed('maintenance result'), messages: [{ info: { role: 'assistant', messageId: 'maintenance-answer', parentMessageId: running.inputId }, parts: [{ type: 'text', text: 'maintenance result' }] }] }] })); + }, + }); + const stored = await input.store.readJob(input.workspace, reservation.id); const storage = await resolveWorkspaceStorage({ dataRoot: input.dataRoot, workspace: input.workspace }); + assert.equal(output.job.status, 'succeeded'); assert.equal(output.result, 'maintenance result'); assert.equal(stored.status, 'succeeded'); assert.equal(await readFile(join(storage.directory, stored.resultArtifact), 'utf8'), 'maintenance result'); +}); + +test('executeJob does not write a result after SessionEnd cancellation wins', async () => { + const input = await fixture(); const reservation = await job(input, { claim: false, status: 'queued' }); + await assert.rejects(executeJob({ + job: reservation, workspace: input.workspace, dataRoot: input.dataRoot, store: input.store, client: executorClient(), task: 'finish', + onBoundaryPersisted: async () => settle(input, async (current) => clientFor(current, { reads: [{ projection: { status: 'running' }, runtime: { stateRevision: 8 }, messages: [] }, { projection: { status: 'paused' }, runtime: { stateRevision: 8 }, messages: [] }] })), + }), { code: 'JOB_TERMINAL' }); + const stored = await input.store.readJob(input.workspace, reservation.id); const storage = await resolveWorkspaceStorage({ dataRoot: input.dataRoot, workspace: input.workspace }); + assert.equal(stored.status, 'cancelled'); await assert.rejects(readFile(join(storage.directory, 'results', `${reservation.id}.md`)), { code: 'ENOENT' }); +}); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index c16b3ea0..a153c126 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -886,6 +886,8 @@ test('a workspace permits one writable job while read-only jobs remain concurren const rejection = attempts.find(({ status }) => status === 'rejected'); assert.ok(rejection && rejection.status === 'rejected'); assert.equal(rejection.reason.code, 'WRITABLE_JOB_EXISTS'); + assert.equal(rejection.reason.remedy, 'Retry later or inspect the redacted workspace list with $zcode:status --all.'); + assert.doesNotMatch(rejection.reason.remedy, /read-only/i); const readOnlyJobs = await Promise.all([ store.reserveJob({ workspace, ...jobInput, ownerTurnId: 'read-a', readOnly: true }), @@ -894,6 +896,17 @@ test('a workspace permits one writable job while read-only jobs remain concurren assert.equal(readOnlyJobs.length, 2); }); +test('writable exclusion remedy does not advertise a read-only rescue mode', async () => { + const { dataRoot, workspace } = await fixture(); const store = createStateStore({ dataRoot }); + await store.reserveJob({ workspace, ...jobInput }); + await assert.rejects( + store.reserveJob({ workspace, ...jobInput, ownerTurnId: 'blocked-turn' }), + (error) => error instanceof PluginError && error.code === 'WRITABLE_JOB_EXISTS' + && error.remedy === 'Retry later or inspect the redacted workspace list with $zcode:status --all.' + && !/read-only/i.test(error.remedy), + ); +}); + test('workspace storage hashes the real path and creates private directories', async () => { const { dataRoot, workspace } = await fixture(); const first = await resolveWorkspaceStorage({ dataRoot, workspace }); diff --git a/tests/zcode-client.test.mjs b/tests/zcode-client.test.mjs index 2fcc37cc..89186fcd 100644 --- a/tests/zcode-client.test.mjs +++ b/tests/zcode-client.test.mjs @@ -1,14 +1,14 @@ // @ts-nocheck import assert from 'node:assert/strict'; -import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import net from 'node:net'; import test from 'node:test'; -import { createManagedZCodeClient, createZCodeClient } from '../scripts/lib/zcode-client.mjs'; -import { brokerEndpointFor, ensureZCodeBroker, reconcileBrokerOwnership, ZCodeBroker as ZCodeBrokerClass } from '../scripts/zcode-broker.mjs'; +import { createExistingManagedZCodeClient, createManagedZCodeClient, createZCodeClient } from '../scripts/lib/zcode-client.mjs'; +import { brokerEndpointFor, brokerIdentityNameForWireOptions, ensureZCodeBroker, reconcileBrokerOwnership, writeBrokerIdentity, ZCodeBroker as ZCodeBrokerClass } from '../scripts/zcode-broker.mjs'; import { withFileLock } from '../scripts/lib/fs.mjs'; import { resolveWorkspaceStorage } from '../scripts/lib/workspace.mjs'; @@ -26,6 +26,39 @@ function newTestBroker(options) { return new ZCodeBrokerClass({ ...options, ...(ownershipPath === undefined ? {} : { ownershipPath }) }); } +async function createPersistedTestBroker({ dataRoot, workspace, tokenByte, instanceByte, record, ...wireOptions }) { + const storage = await resolveWorkspaceStorage({ dataRoot, workspace }); + const identityName = brokerIdentityNameForWireOptions(wireOptions); + const profile = identityName === 'identity.json' ? undefined : identityName.slice('identity-'.length, -'.json'.length); + const endpoint = brokerEndpointFor({ dataRoot, workspace: storage.workspacePath, ...(profile ? { identity: profile } : {}) }); + const brokerToken = tokenByte.repeat(64); + const instanceId = instanceByte.repeat(48); + const broker = await newTestBroker({ endpoint, brokerToken, instanceId, workspace: storage.workspacePath, launch: { command: process.execPath, args: [fixture], target: fixture }, env: { ...process.env, ...(record ? { FAKE_ZCODE_RECORD: record } : {}) }, ...wireOptions }).start(); + await writeBrokerIdentity(join(storage.directory, 'broker', identityName), { endpoint, pid: process.pid, instanceId, brokerToken }); + return broker; +} + +async function createHealthOnlyServer(endpoint, { brokerToken, instanceId, hangHealth = false, closeAfterHealth = false }) { + const sockets = new Set(); + const server = net.createServer((socket) => { + sockets.add(socket); socket.setEncoding('utf8'); let buffer = ''; + socket.on('data', (chunk) => { + buffer += chunk; let newline = buffer.indexOf('\n'); + while (newline !== -1) { + const frame = JSON.parse(buffer.slice(0, newline)); buffer = buffer.slice(newline + 1); newline = buffer.indexOf('\n'); + if (frame.method === 'broker/auth' && frame.params?.token === brokerToken) socket.write(`${JSON.stringify({ id: frame.id, result: { authenticated: true } })}\n`); + else if (frame.method === 'broker/health' && !hangHealth) { + socket.write(`${JSON.stringify({ id: frame.id, result: { ok: true, pid: process.pid, instanceId } })}\n`); + if (closeAfterHealth) server.close(); + } + } + }); + socket.once('close', () => sockets.delete(socket)); + }); + await new Promise((resolvePromise, reject) => { server.once('error', reject); server.listen(endpoint, resolvePromise); }); + return async () => { for (const socket of sockets) socket.destroy(); if (server.listening) await new Promise((resolvePromise) => server.close(resolvePromise)); }; +} + async function readRecordedCalls(record) { let content; try { content = await readFile(record, 'utf8'); } catch (error) { @@ -261,7 +294,7 @@ test('request failures retain only a bounded safe remote error code', async (t) FAKE_ZCODE_ERROR: 'session/list', FAKE_ZCODE_ERROR_DATA_CODE: 'model_config_missing', FAKE_ZCODE_ERROR_DATA_SECRET: 'remote-api-key-must-not-leak', - })); + }, { requestTimeoutMs: 2_000, completionTimeoutMs: 2_000 })); for (const [name, remoteCode] of [ ['oversized', 'x'.repeat(129)], @@ -274,7 +307,7 @@ test('request failures retain only a bounded safe remote error code', async (t) assert.deepEqual(error.details, { method: 'session/list', rpcCode: -32099 }); return true; }); - }, { FAKE_ZCODE_ERROR: 'session/list', FAKE_ZCODE_ERROR_DATA_CODE: remoteCode })); + }, { FAKE_ZCODE_ERROR: 'session/list', FAKE_ZCODE_ERROR_DATA_CODE: remoteCode }, { requestTimeoutMs: 2_000, completionTimeoutMs: 2_000 })); } }); @@ -383,6 +416,80 @@ test('managed broker clients require an explicit stable owner credential', async for (const drainTimeoutMs of [0, 30_001]) await assert.rejects(createManagedZCodeClient({ dataRoot: '/tmp/data', workspace: '/tmp/workspace', launch: { command: process.execPath, args: [] }, ownerId: 'bounded-drain-owner', drainTimeoutMs }), { code: 'ZCODE_INPUT_INVALID' }); }); +test('existing managed client connects to the exact healthy wire profile without ensuring a broker', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-existing-exact-')); + const exactWire = { maxFrameBytes: 16 * 1024 * 1024, maxOutboundBytes: 16 * 1024 * 1024 }; + assert.equal(brokerIdentityNameForWireOptions(exactWire), 'identity-fc55dc554b54c5fb.json'); + let defaultBroker; let exactBroker; let client; + try { + defaultBroker = await createPersistedTestBroker({ dataRoot: directory, workspace: directory, tokenByte: '1', instanceByte: 'a' }); + exactBroker = await createPersistedTestBroker({ dataRoot: directory, workspace: directory, tokenByte: '2', instanceByte: 'b', ...exactWire }); + client = await createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-exact-owner', requestTimeoutMs: 100, ...exactWire }); + assert.ok(client); + assert.equal(defaultBroker.owners, 0); + assert.equal(exactBroker.owners, 1); + } finally { + await client?.close().catch(() => {}); await exactBroker?.close(); await defaultBroker?.close(); await rm(directory, { recursive: true, force: true }); + } +}); + +test('existing managed client returns null and never spawns when the broker is absent', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-existing-absent-')); + try { + assert.equal(await createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-absent-owner', requestTimeoutMs: 50 }), null); + await assert.rejects(createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-no-launch-owner', requestTimeoutMs: 50, launch: { command: process.execPath, args: [fixture] } }), { code: 'ZCODE_INPUT_INVALID' }); + await assert.rejects(createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-no-env-owner', requestTimeoutMs: 50, env: process.env }), { code: 'ZCODE_INPUT_INVALID' }); + } finally { await rm(directory, { recursive: true, force: true }); } +}); + +test('existing managed client does not fall back to a sibling wire profile', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-existing-sibling-')); let defaultBroker; + try { + defaultBroker = await createPersistedTestBroker({ dataRoot: directory, workspace: directory, tokenByte: '3', instanceByte: 'c' }); + const client = await createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-sibling-owner', requestTimeoutMs: 50, maxFrameBytes: 4096 }); + assert.equal(client, null); + assert.equal(defaultBroker.owners, 0); + } finally { await defaultBroker?.close(); await rm(directory, { recursive: true, force: true }); } +}); + +test('existing managed client bounds an unhealthy broker probe', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-existing-hung-')); const wireOptions = { maxFrameBytes: 4096 }; let closeServer; + try { + for (const requestTimeoutMs of [0, 3_600_001]) await assert.rejects(createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-invalid-timeout', requestTimeoutMs, ...wireOptions }), { code: 'ZCODE_INPUT_INVALID' }); + const storage = await resolveWorkspaceStorage({ dataRoot: directory, workspace: directory }); const identityName = brokerIdentityNameForWireOptions(wireOptions); const profile = identityName.slice('identity-'.length, -'.json'.length); const endpoint = brokerEndpointFor({ dataRoot: directory, workspace: storage.workspacePath, identity: profile }); const brokerToken = '4'.repeat(64); const instanceId = 'd'.repeat(48); + closeServer = await createHealthOnlyServer(endpoint, { brokerToken, instanceId, hangHealth: true }); + await writeBrokerIdentity(join(storage.directory, 'broker', identityName), { endpoint, pid: process.pid, instanceId, brokerToken }); + const startedAt = Date.now(); + assert.equal(await createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-hung-owner', requestTimeoutMs: 40, ...wireOptions }), null); + assert.ok(Date.now() - startedAt < 500); + } finally { await closeServer?.(); await rm(directory, { recursive: true, force: true }); } +}); + +test('existing managed client returns null when the broker dies between health and connect', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-existing-race-')); let closeServer; + try { + const storage = await resolveWorkspaceStorage({ dataRoot: directory, workspace: directory }); const identityName = brokerIdentityNameForWireOptions(); const endpoint = brokerEndpointFor({ dataRoot: directory, workspace: storage.workspacePath }); const brokerToken = '5'.repeat(64); const instanceId = 'e'.repeat(48); + closeServer = await createHealthOnlyServer(endpoint, { brokerToken, instanceId, closeAfterHealth: true }); + await writeBrokerIdentity(join(storage.directory, 'broker', identityName), { endpoint, pid: process.pid, instanceId, brokerToken }); + assert.equal(await createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId: 'existing-race-owner', requestTimeoutMs: 100 }), null); + } finally { await closeServer?.(); await rm(directory, { recursive: true, force: true }); } +}); + +test('existing managed client cannot lazily spawn a child protocol while normal managed clients still can', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-existing-no-child-')); const record = join(directory, 'calls.jsonl'); const ownerId = 'existing-no-child-owner'; const remoteSessionId = 'existing-no-child-session'; let broker; let existingClient; let normalClient; + try { + const storage = await resolveWorkspaceStorage({ dataRoot: directory, workspace: directory }); const brokerDirectory = join(storage.directory, 'broker'); const ownershipPath = join(brokerDirectory, 'session-owners.json'); await mkdir(brokerDirectory, { recursive: true }); await writeFile(ownershipPath, JSON.stringify({ version: 1, sessions: { [remoteSessionId]: ownerId } })); + const endpoint = brokerEndpointFor({ dataRoot: directory, workspace: storage.workspacePath }); const brokerToken = '6'.repeat(64); const instanceId = 'f'.repeat(48); const launch = { command: process.execPath, args: [fixture], target: fixture }; + broker = await newTestBroker({ endpoint, ownershipPath, brokerToken, instanceId, workspace: storage.workspacePath, launch, env: { ...process.env, FAKE_ZCODE_RECORD: record } }).start(); + await writeBrokerIdentity(join(brokerDirectory, 'identity.json'), { endpoint, pid: process.pid, instanceId, brokerToken }); + existingClient = await createExistingManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId, requestTimeoutMs: 100 }); assert.ok(existingClient); + await assert.rejects(existingClient.readSession(remoteSessionId), { code: 'ZCODE_BROKER_PROTOCOL_UNAVAILABLE' }); assert.equal(broker.protocol, null); assert.equal(broker.protocolPromise, null); await assert.rejects(readFile(record, 'utf8'), { code: 'ENOENT' }); + await existingClient.close(); existingClient = null; + normalClient = await createManagedZCodeClient({ dataRoot: directory, workspace: directory, ownerId, launch, env: { ...process.env, FAKE_ZCODE_RECORD: record }, requestTimeoutMs: 500 }); + assert.equal((await normalClient.readSession(remoteSessionId)).session.sessionId, remoteSessionId); assert.ok((await readFile(record, 'utf8')).includes('session/read')); + } finally { await existingClient?.close().catch(() => {}); await normalClient?.close().catch(() => {}); await broker?.close(); await rm(directory, { recursive: true, force: true }); } +}); + test('named-pipe broker construction requires an explicit ownership path', () => { for (const endpoint of ['\\\\.\\pipe\\zcode-test', '\\\\.\\PIPE\\zcode-test']) assert.throws(() => new ZCodeBrokerClass({ endpoint, brokerToken: 'b'.repeat(64), workspace: '/tmp', launch: { command: process.execPath, args: [] } }), { code: 'ZCODE_BROKER_INPUT_INVALID' }); }); diff --git a/tests/zcode-protocol.test.mjs b/tests/zcode-protocol.test.mjs new file mode 100644 index 00000000..7bbe8ad5 --- /dev/null +++ b/tests/zcode-protocol.test.mjs @@ -0,0 +1,63 @@ +// @ts-nocheck +import assert from 'node:assert/strict'; +import { mkdtemp, rm } from 'node:fs/promises'; +import net from 'node:net'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { randomUUID } from 'node:crypto'; +import test from 'node:test'; + +import { connectZCodeBroker } from '../scripts/lib/zcode-protocol.mjs'; + +test('broker connect bounds authentication and closes the socket when the peer never answers', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-protocol-auth-')); + const endpoint = process.platform === 'win32' ? `\\\\.\\pipe\\zcode-protocol-${randomUUID()}` : join(directory, 'broker.sock'); + let peer; let resolveAccepted; let resolvePeerClosed; + const accepted = new Promise((resolvePromise) => { resolveAccepted = resolvePromise; }); + const peerClosed = new Promise((resolvePromise) => { resolvePeerClosed = resolvePromise; }); + const server = net.createServer((socket) => { + peer = socket; resolveAccepted(); + socket.once('close', () => resolvePeerClosed()); + socket.resume(); + }); + await new Promise((resolvePromise, reject) => { server.once('error', reject); server.listen(endpoint, resolvePromise); }); + try { + const startedAt = Date.now(); + const connecting = connectZCodeBroker(endpoint, { brokerToken: 'a'.repeat(64), ownerId: 'protocol-auth-timeout-owner', requestTimeoutMs: 40 }); + await accepted; + await assert.rejects(connecting, { code: 'ZCODE_REQUEST_TIMEOUT' }); + assert.ok(Date.now() - startedAt < 500); + assert.equal(await Promise.race([peerClosed.then(() => true), new Promise((resolvePromise) => { const timer = setTimeout(() => resolvePromise(false), 250); timer.unref?.(); })]), true); + } finally { + peer?.destroy(); + await new Promise((resolvePromise) => server.close(resolvePromise)); + await rm(directory, { recursive: true, force: true }); + } +}); + +test('broker connect rejects a malformed existing-protocol-only capability before opening a socket', async () => { + await assert.rejects(connectZCodeBroker('/definitely-missing-zcode-broker', { + brokerToken: 'a'.repeat(64), ownerId: 'protocol-capability-owner', existingProtocolOnly: 'yes', requestTimeoutMs: 40, + }), { code: 'ZCODE_PROTOCOL_INPUT_INVALID' }); +}); + +test('broker connect fails closed when an older broker does not acknowledge existing-protocol-only', async () => { + const directory = await mkdtemp(join(tmpdir(), 'zcode-protocol-capability-')); + const endpoint = process.platform === 'win32' ? `\\\\.\\pipe\\zcode-protocol-${randomUUID()}` : join(directory, 'broker.sock'); + const sockets = new Set(); + const server = net.createServer((socket) => { + sockets.add(socket); socket.setEncoding('utf8'); let buffer = ''; + socket.on('data', (chunk) => { + buffer += chunk; const newline = buffer.indexOf('\n'); if (newline === -1) return; + const frame = JSON.parse(buffer.slice(0, newline)); + socket.write(`${JSON.stringify({ id: frame.id, result: { authenticated: true } })}\n`); + }); + socket.once('close', () => sockets.delete(socket)); + }); + await new Promise((resolvePromise, reject) => { server.once('error', reject); server.listen(endpoint, resolvePromise); }); + try { + await assert.rejects(connectZCodeBroker(endpoint, { brokerToken: 'a'.repeat(64), ownerId: 'protocol-capability-owner', existingProtocolOnly: true, requestTimeoutMs: 100 }), { code: 'ZCODE_BROKER_CAPABILITY_UNAVAILABLE' }); + for (let turn = 0; turn < 20 && sockets.size; turn += 1) await new Promise((resolvePromise) => setImmediate(resolvePromise)); + assert.equal(sockets.size, 0); + } finally { for (const socket of sockets) socket.destroy(); await new Promise((resolvePromise) => server.close(resolvePromise)); await rm(directory, { recursive: true, force: true }); } +});