From ba3b67a3af31e0e07532dc569641d26891a689be Mon Sep 17 00:00:00 2001 From: willytop8 Date: Sat, 1 Aug 2026 23:04:45 -0500 Subject: [PATCH 1/2] fix(persistence): keep contended sessions usable Keep non-owner sessions in a passive goal mode and retry ownership only at an explicit control boundary. Harden mixed-version lease acquisition with an atomic compatibility guard and immutable owner claims. Fixes #41 --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 5 + README.md | 13 +- docs/compatibility.md | 30 +- scripts/mutation-contract.mjs | 293 +++++++- scripts/packed-host-contract.mjs | 111 +++ src/goal-plugin.js | 571 +++++++++++++--- src/persistence-lease.js | 653 ++++++++++++++++-- test/goal-plugin.test.js | 114 +++- test/host-lifecycle.test.js | 979 +++++++++++++++++++++++++- test/passive-retention.test.js | 76 +++ test/persistence-lease.test.js | 1094 +++++++++++++++++++++++++++++- test/session-concurrency.test.js | 169 +++-- 13 files changed, 3858 insertions(+), 252 deletions(-) create mode 100644 test/passive-retention.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b37b242..3bbe0ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,7 +89,7 @@ jobs: - name: Install locked dependencies run: npm ci - name: Run portable lifecycle and filesystem tests - run: node --test test/host-lifecycle.test.js test/persistence-lease.test.js test/public-hook-cancellation.test.js + run: node --test test/host-lifecycle.test.js test/persistence-lease.test.js test/public-hook-cancellation.test.js test/session-concurrency.test.js - name: Compile installed-package type contracts on Windows if: runner.os == 'Windows' run: npm run type:check diff --git a/CHANGELOG.md b/CHANGELOG.md index ac263aa..80f548a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +- Keep a second OpenCode process usable when it opens a session whose goal-state shard is already leased: ordinary chat and unrelated tools remain available, while goal commands and tools fail safely without reading, changing, or prompting from that session's goal state. After the owner exits, the next explicit goal command or tool retries ownership and reloads any active goal paused. Lease contention is now typed and owner metadata is sanitized; unrelated filesystem failures still fail closed. +- Harden the single-writer lease with immutable per-owner claims so delayed publication, simultaneous stale reclaim, and concurrent release cannot remove a replacement owner's lease. A complete compatibility guard is published atomically with no replacement, preventing old/current startup races; legacy, incomplete, tampered, or unsupported lease layouts fail closed for explicit manual recovery. Owner reads remain bounded and symlink-safe, slow passive command guards keep blocking tools until an authenticated new boundary, delayed control errors cannot pause a newer goal run, takeover retains Plan/model execution context, and advisory host logging cannot stall loading or disposal. + ## 0.6.7 — 2026-08-01 - Fix `/goal` routing on OpenCode 1.17.15 and 1.18.10 by replacing the host-retained prompt parts in place, authenticating each resolved command turn (including host-expanded file attachments), framing control results with direct reporting instructions, blocking tools during those reporting turns, and excluding control responses from goal completion and progress analysis. Unreadable command attachments now become read-only error reports and pause safely without losing command provenance. diff --git a/README.md b/README.md index 506e3de..ed654df 100644 --- a/README.md +++ b/README.md @@ -255,6 +255,10 @@ Alongside each session shard the plugin keeps an **append-only lifecycle ledger* Recovered active goals are loaded in a **paused** state with a recovery note, so unattended auto-continue does not resume blindly after a restart. Set `"persistState": false` to keep purely in-memory behavior (this also disables the ledger). +Only one OpenCode process may own a given session shard at a time. If the same session is opened in a second process, that process enters **passive goal mode** instead of failing the whole session: ordinary chat and unrelated tools continue to work, but `/goal` commands and goal tools report that another process owns the workflow. Canonical goal tools return the stable envelope code `error: "session_owned_elsewhere"`. The passive process does not read, mutate, persist, or auto-continue that session's goal state. After the owner exits, retry an explicit goal command or goal tool; the process will acquire the shard and load any recovered active goal paused. To work concurrently without waiting, create a new session with `opencode --continue --fork` (or `opencode --session --fork`). + +Lease ownership uses immutable per-process claim files so a delayed stale-lock cleanup or duplicate release cannot delete a newer owner's lease. The plugin publishes a complete regular-file compatibility guard atomically at `/state.json.lock`, then elects the current owner from claims in the sibling `/state.json.lock.claims-v2/` directory. That no-replace publication makes startup safe against older releases: either the older lock directory wins and the current plugin stays passive, or the guard file wins and the older release cannot reclaim it. Automatic ownership handoff requires the current release. Legacy, incomplete, tampered, or unsupported lease layouts fail closed instead of being rewritten online; filesystems must support regular-file hard links and preserve the guard's future timestamp. After confirming that every process using the session is closed and upgraded, either fork or remove only the affected shard's adjacent `.lock` file or legacy directory **and** `.lock.claims-v2` directory; keep its state and ledger. + `/goal resume` continues the same objective with a fresh local budget window. This lets you continue after pause, blocker, no-progress pause, rate-limit failures, or a limit stop without retyping the objective. ### Per-goal flags @@ -393,7 +397,7 @@ For `/goal status`, `/goal history`, `/goal list`, `/goal pause`, and `/goal cle The plugin still registers `experimental.chat.system.transform` as defense in depth for hosts that invoke it. Real OpenCode 1.17.15 and 1.18.10 do not call that hook, so the command-control protections above are deliberately self-contained. Other OpenCode plugin hooks may change between versions. -Distinct OpenCode sessions may own shards under the same `stateFilePath` concurrently. A second process using the same session shard is still rejected with the owning PID/host instead of risking last-writer-wins state loss; use the original session or wait for its owner to dispose. +Distinct OpenCode sessions may own shards under the same `stateFilePath` concurrently. A second process using the same session shard remains usable in passive goal mode, but goal commands and tools are denied until it can acquire that shard. The passive process never falls back to an unpersisted copy of the same goal workflow, which avoids divergent state and last-writer-wins data loss. Use the owner, wait and retry an explicit goal control after it exits, or fork to a new session. A no-replace compatibility guard prevents an older release and the current release from both acquiring during startup; current immutable claims protect takeover and release among upgraded processes. Older processes cannot take over a guarded shard, so all processes participating in automatic same-session handoff must run the current release. ## Diagnostics and recovery @@ -403,9 +407,10 @@ If a goal does not continue: 1. Check for a deliberate pause: user intervention, a hard limit, repeated tool-free/no-progress turns, prompt failures, or a rejected completion audit all stop unattended work by design. 2. Run `/goal resume` only after resolving the reported reason. Resume creates a fresh local budget window; it does not erase the objective or history. -3. Check OpenCode's structured logs for persistence, SDK-shape, prompt, or auditor errors. -4. Confirm the configured project directory and state-path precedence described under [Safety limits](#safety-limits). A daemon started elsewhere can otherwise make a manually configured relative path surprising. -5. Run `npm run verify`, `npm run smoke`, and `npm run smoke:packed-host` against the installed source when diagnosing registration or packaging problems. Maintainers can run `npm run release:check` for the complete artifact and quality gate. `npm run benchmark:behavior` exercises completion, false-completion, loop, interruption, compaction, and restart behavior without a provider call. +3. If a goal control reports that another process owns the session, close that owner and retry the control, or fork to a new session. If it instead reports an older, incomplete, tampered, or unsupported lease, close every process that could own the session and upgrade them first; if the report persists, remove only the affected shard's adjacent `.lock` file or legacy directory and `.lock.claims-v2` directory, or fork. Keep the state and ledger. Do not point two copies of the same session at different state paths: that creates divergent goal histories. +4. Check OpenCode's structured logs for persistence, SDK-shape, prompt, or auditor errors. +5. Confirm the configured project directory and state-path precedence described under [Safety limits](#safety-limits). A daemon started elsewhere can otherwise make a manually configured relative path surprising. +6. Run `npm run verify`, `npm run smoke`, and `npm run smoke:packed-host` against the installed source when diagnosing registration or packaging problems. Maintainers can run `npm run release:check` for the complete artifact and quality gate. `npm run benchmark:behavior` exercises completion, false-completion, loop, interruption, compaction, and restart behavior without a provider call. Do not paste `state.json`, its ledger, or verbose logs into a public issue without reviewing them first: they can contain goal text, assistant checkpoints, blockers, local paths, and command evidence. Prefer the bounded status/history output and redact project-specific content. There is intentionally no broad "dump diagnostics" tool: exposing process-wide session state or persistence paths to the model would add more privacy risk than troubleshooting value. diff --git a/docs/compatibility.md b/docs/compatibility.md index c79abab..b01594d 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -11,7 +11,8 @@ The latest published release is the supported line. Public compatibility covers: - the six canonical goal tools and five legacy tool aliases - persisted-state recovery from versions documented in the changelog - concurrent persistence for distinct OpenCode sessions in one project, with - single-writer protection retained per session + single-writer protection retained per session and passive goal behavior for + a same-session process that does not own the lease The package requires Node.js 18 or newer and OpenCode 1.17.15 through the latest compatible 1.x release. CI runs the complete unit suite on Node 18, 20, 22, and @@ -25,6 +26,33 @@ them; the plugin does not claim that Windows provides equivalent POSIX semantics The Windows job also runs the installed-package type, host, and tool contracts so their portable npm launcher path is exercised in CI. +When two processes open the same OpenCode session, only the lease owner may read +or change that session's goal workflow. The contender keeps ordinary chat and +unrelated tools available, but goal controls are denied and ambient hooks do not +attempt a takeover. Canonical goal tools return the stable envelope code +`session_owned_elsewhere`; a `/goal` slash command instead produces a +human-readable denial through its normal model-rendered command turn. Once the +owner exits, an explicit goal command or tool may acquire the shard; recovered +active goals load paused and require an explicit resume. Forking creates a +distinct session shard and remains the supported way to work concurrently from +the same conversation. + +The immutable-claim lease protocol atomically hard-links a complete regular-file +compatibility guard at `/state.json.lock`; active owners publish unique +claims in the sibling `/state.json.lock.claims-v2/` directory. Publication +is no-replace: an older lock directory and the current guard cannot both win the +same startup race. Older releases treat the future-dated guard as non-reclaimable, +while current releases determine ownership only from immutable claims. Automatic +takeover requires all participating processes to run the current release. +Legacy, incomplete, tampered, or unsupported lease layouts fail closed rather +than being rewritten online. If that condition persists, first close every +OpenCode process that could own the session and upgrade them; then either fork +the session or manually remove only the affected shard's adjacent `.lock` file +or legacy directory and `.lock.claims-v2` directory. Do not remove its state or +lifecycle ledger. The local filesystem must support regular-file hard links +and preserve the guard's future timestamp; the plugin does not fall back to a +weaker publication protocol. + ## OpenCode host compatibility OpenCode's experimental hooks and SDK request shapes may change within the 1.x diff --git a/scripts/mutation-contract.mjs b/scripts/mutation-contract.mjs index ca031f4..254a2b5 100644 --- a/scripts/mutation-contract.mjs +++ b/scripts/mutation-contract.mjs @@ -157,8 +157,8 @@ const mutants = [ { name: "control responses do not advance progress timestamps", file: "src/goal-plugin.js", - from: ' parentOwner?.policy === "control" &&\n', - to: " false &&\n", + from: ' parentOwner?.policy === "control" &&\n', + to: " false &&\n", test: "test/goal-plugin.test.js", }, { @@ -168,10 +168,297 @@ const mutants = [ to: "false", test: "test/goal-plugin.test.js", }, + { + name: "only typed lease contention enters passive mode", + file: "src/persistence-lease.js", + from: "return error instanceof PersistenceLeaseContendedError", + to: "return true", + test: "test/persistence-lease.test.js", + }, + { + name: "stored owner hostnames are validated separately from diagnostics", + file: "src/persistence-lease.js", + from: " validStoredHostname(owner.hostname)", + to: " validDisplayHostname(owner.hostname)", + test: "test/persistence-lease.test.js", + }, + { + name: "bounded owner records tolerate legal short reads", + file: "src/persistence-lease.js", + from: " while (bytesReadTotal < buffer.length) {", + to: " if (bytesReadTotal < buffer.length) {", + test: "test/persistence-lease.test.js", + }, + { + name: "overlapping immutable lease claimants cannot both win", + file: "src/persistence-lease.js", + from: " if (!observed.ownFound || observed.blocked) {", + to: " if (false) {", + test: "test/persistence-lease.test.js", + }, + { + name: "compatibility guard publication is atomic and no-replace", + file: "src/persistence-lease.js", + from: " await linkGuard(temporaryPath, lockPath)", + to: " await fs.rename(temporaryPath, lockPath)", + test: "test/persistence-lease.test.js", + }, + { + name: "compatibility guards are complete before publication", + file: "src/persistence-lease.js", + from: " await handle.utimes(guardDate, guardDate)", + to: " await Promise.resolve()", + test: "test/persistence-lease.test.js", + }, + { + name: "compatibility guards require the exact sentinel schema", + file: "src/persistence-lease.js", + from: ' Object.keys(owner).sort().join(",") ===\n "createdAt,hostname,pid,protocol,sentinel,token" &&', + to: " true &&", + test: "test/persistence-lease.test.js", + }, + { + name: "vanished version-1 guards retry after publication contention", + file: "src/persistence-lease.js", + from: ' if (error?.code === "EEXIST") return null', + to: " if (false) return null", + test: "test/persistence-lease.test.js", + }, + { + name: "ownership is returned only while the compatibility guard exists", + file: "src/persistence-lease.js", + from: " await ensureLegacyGuard(lockPath, { beforeGuardLink, afterGuardLink, linkGuard })\n return createLease(", + to: " await Promise.resolve()\n return createLease(", + test: "test/persistence-lease.test.js", + }, + { + name: "unsupported hard-link filesystems fail closed", + file: "src/persistence-lease.js", + from: " if (hardLinkUnsupported(error)) throw persistenceLeaseHardLinkError()", + to: " if (false) throw persistenceLeaseHardLinkError()", + test: "test/persistence-lease.test.js", + }, + { + name: "unknown future claim filenames block conservatively", + file: "src/persistence-lease.js", + from: " if (isClaimLikeName(entry.name)) {", + to: " if (false) {", + test: "test/persistence-lease.test.js", + }, + { + name: "future lease protocols block conservatively", + file: "src/persistence-lease.js", + from: " if (record.owner.protocol !== LEASE_PROTOCOL_VERSION) {", + to: " if (false) {", + test: "test/persistence-lease.test.js", + }, + { + name: "lease release teardown is single-entry", + file: "src/persistence-lease.js", + from: " if (released || releasing) return false", + to: " if (released) return false", + test: "test/persistence-lease.test.js", + }, + { + name: "disposed instances do not perform delayed legacy migration", + file: "src/goal-plugin.js", + from: " try {\n if (currentRuntime().disposed) return\n if (await pathExists(persistenceOptions.migrationMarkerPath)) return", + to: " try {\n if (false) return\n if (await pathExists(persistenceOptions.migrationMarkerPath)) return", + test: "test/goal-plugin.test.js", + }, + { + name: "disposed command continuations cannot mutate state", + file: "src/goal-plugin.js", + from: " const loadResult = await ensureSessionLoaded(sessionID, {\n retryPassive: true,\n freshCommandBoundary: true,\n })\n if (currentRuntime().disposed || loadResult.kind === \"disposed\") return\n const commandTurn = registerPendingCommandTurn(sessionID, output)", + to: " const loadResult = await ensureSessionLoaded(sessionID, {\n retryPassive: true,\n freshCommandBoundary: true,\n })\n if (loadResult.kind === \"disposed\") return\n const commandTurn = registerPendingCommandTurn(sessionID, output)", + test: "test/host-lifecycle.test.js", + }, + { + name: "disposed tool continuations cannot invoke handlers", + file: "src/goal-plugin.js", + from: ' if (disposed || loadResult?.kind === "disposed") {', + to: ' if (loadResult?.kind === "disposed") {', + test: "test/host-lifecycle.test.js", + }, + { + name: "session storage failures remain fatal", + file: "src/goal-plugin.js", + from: " } catch (error) {\n if (!isPersistenceLeaseContendedError(error)) throw error\n return enterPassiveSession(sessionID, error)\n }", + to: " } catch (error) {\n if (false) throw error\n return enterPassiveSession(sessionID, error)\n }", + test: "test/goal-plugin.test.js", + }, + { + name: "overlapping hooks await one complete session load", + file: "src/goal-plugin.js", + from: " const existingLoad = runtime.sessionLoadPromises.get(sessionID)\n if (existingLoad) return existingLoad\n if (runtime.sessionPersistence.has(sessionID)) return ACTIVE_PERSISTENCE_OWNED", + to: " if (runtime.sessionPersistence.has(sessionID)) return ACTIVE_PERSISTENCE_OWNED\n const existingLoad = runtime.sessionLoadPromises.get(sessionID)\n if (existingLoad) return existingLoad", + test: "test/goal-plugin.test.js", + }, + { + name: "passive load results cannot masquerade as active", + file: "src/goal-plugin.js", + from: " const passiveLoadResult = (entry) => ({\n kind: \"passive\",\n code: SESSION_OWNED_ELSEWHERE,", + to: " const passiveLoadResult = (entry) => ({\n kind: \"active\",\n code: SESSION_OWNED_ELSEWHERE,", + test: "test/host-lifecycle.test.js", + }, + { + name: "legacy lease recovery reason reaches passive controls", + file: "src/goal-plugin.js", + from: " reason: entry.reason,", + to: ' reason: "owned_elsewhere",', + test: "test/host-lifecycle.test.js", + }, + { + name: "host logging never retains a persistence lease", + file: "src/goal-plugin.js", + from: " void Promise.resolve(call()).catch(onFailure)", + to: " return Promise.resolve(call()).catch(onFailure)", + test: "test/host-lifecycle.test.js", + }, + { + name: "disposed idle lookups cannot repopulate command state", + file: "src/goal-plugin.js", + from: " if (runtime.disposed) return { ready: false, messages: null }\n const commandMessages = Array.isArray(commandHostMessages)\n ? commandHostMessages.slice(-messageLimit)\n : []\n if (runtime.activeCommandTurns.get(sessionID) !== activeCommandTurn) {", + to: " if (false) return { ready: false, messages: null }\n const commandMessages = Array.isArray(commandHostMessages)\n ? commandHostMessages.slice(-messageLimit)\n : []\n if (false) {", + test: "test/host-lifecycle.test.js", + }, + { + name: "a delayed idle lookup cannot retire a newer command guard", + file: "src/goal-plugin.js", + from: " if (runtime.activeCommandTurns.get(sessionID) !== activeCommandTurn) {", + to: " if (false) {", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive command guards survive stale idle events", + file: "src/goal-plugin.js", + from: " await retireCompletedCommandTurnOnIdle(\n eventSessionID,\n defaultGoalOptions.maxRecentMessages,\n )", + to: " currentRuntime().activeCommandTurns.delete(eventSessionID)", + test: "test/host-lifecycle.test.js", + }, + { + name: "takeover preserves the observed execution context", + file: "src/goal-plugin.js", + from: " if (!preserveExecutionContext) runtime.sessionExecutionContexts.delete(sessionID)", + to: " runtime.sessionExecutionContexts.delete(sessionID)", + test: "test/host-lifecycle.test.js", + }, + { + name: "partial tool context preserves model and variant", + file: "src/goal-plugin.js", + from: " const merged = {\n ...previous,\n ...observed,", + to: " const merged = {\n ...observed,", + test: "test/host-lifecycle.test.js", + }, + { + name: "authoritative host context clears stale variants", + file: "src/goal-plugin.js", + from: " rememberSessionExecutionContext(sessionID, input, { replace: true })", + to: " rememberSessionExecutionContext(sessionID, input)", + test: "test/host-lifecycle.test.js", + }, + { + name: "canonical tools contribute their execution context", + file: "src/goal-plugin.js", + from: " executionContext: ctx,\n })\n const unavailable = inactiveGoalToolResult(\n loadResult,\n commandName,\n isDisposed(),\n commandRegistered,\n )\n if (unavailable) return serializeGoalToolResult", + to: " executionContext: undefined,\n })\n const unavailable = inactiveGoalToolResult(\n loadResult,\n commandName,\n isDisposed(),\n commandRegistered,\n )\n if (unavailable) return serializeGoalToolResult", + test: "test/host-lifecycle.test.js", + }, + { + name: "tool-only mode receives an actionable contention hint", + file: "src/goal-plugin.js", + from: " const retryTarget = commandRegistered\n ?", + to: " const retryTarget = true\n ?", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive state remains isolated to its session", + file: "src/goal-plugin.js", + from: " const passive = runtime.passiveSessions.get(sessionID)\n pruneExpiredPendingCommandTurns(sessionID)\n const commandTurnInFlight =", + to: " const passive = runtime.passiveSessions.values().next().value\n pruneExpiredPendingCommandTurns(sessionID)\n const commandTurnInFlight =", + test: "test/host-lifecycle.test.js", + }, + { + name: "expired pending passive command guards do not block takeover forever", + file: "src/goal-plugin.js", + from: " pruneExpiredPendingCommandTurns(sessionID)\n const commandTurnInFlight =", + to: " if (false) pruneExpiredPendingCommandTurns(sessionID)\n const commandTurnInFlight =", + test: "test/host-lifecycle.test.js", + }, + { + name: "only a fresh command may retry past an accepted passive command guard", + file: "src/goal-plugin.js", + from: " freshCommandBoundary: true,", + to: " freshCommandBoundary: false,", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive session tombstones are retained until explicit takeover", + file: "src/goal-plugin.js", + from: " runtime.passiveSessions.set(sessionID, entry)\n if (!previous?.warned) {", + to: " runtime.passiveSessions.set(sessionID, entry)\n while (runtime.passiveSessions.size > 1000) runtime.passiveSessions.delete(runtime.passiveSessions.keys().next().value)\n if (!previous?.warned) {", + test: "test/passive-retention.test.js", + }, + { + name: "ambient hooks never acquire a formerly contended session", + file: "src/goal-plugin.js", + from: " (!retryPassive || commandTurnInFlight || Date.now() < passive.retryAt)", + to: " (false || commandTurnInFlight || Date.now() < passive.retryAt)", + test: "test/host-lifecycle.test.js", + }, + { + name: "an active passive-command turn prevents lease takeover", + file: "src/goal-plugin.js", + from: " (!retryPassive || commandTurnInFlight || Date.now() < passive.retryAt)", + to: " (!retryPassive || Date.now() < passive.retryAt)", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive slash commands cannot mutate goal state", + file: "src/goal-plugin.js", + from: ' if (loadResult.kind === "passive") {', + to: " if (false) {", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive command replies retain their denial provenance", + file: "src/goal-plugin.js", + from: " commandTurn.passive = true", + to: " commandTurn.passive = false", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive legacy goal tools cannot invoke handlers", + file: "src/goal-plugin.js", + from: " if (unavailable) return unavailable.message", + to: " if (false) return unavailable.message", + test: "test/host-lifecycle.test.js", + }, + { + name: "passive canonical goal tools return the ownership error", + file: "src/goal-plugin.js", + from: " if (unavailable) return serializeGoalToolResult(operation, unavailable)", + to: " if (false) return serializeGoalToolResult(operation, unavailable)", + test: "test/host-lifecycle.test.js", + }, + { + name: "takeover preserves passive command security tombstones", + file: "src/goal-plugin.js", + from: " if (!preserveCommandSecurity) {", + to: " if (true) {", + test: "test/host-lifecycle.test.js", + }, + { + name: "late passive command replies stay excluded after takeover", + file: "src/goal-plugin.js", + from: " if (controlCommandAssistant) {", + to: " if (false) {", + test: "test/host-lifecycle.test.js", + }, { name: "goal tools register without an external helper", file: "src/goal-plugin.js", - from: "hooks.tool = buildAgentTools(bundledToolHelper, agentToolHandlers, ensureSessionLoaded)", + from: " hooks.tool = buildAgentTools(\n bundledToolHelper,\n agentToolHandlers,\n ensureSessionLoaded,\n commandName,\n () => runtime.disposed,\n registerCommand,\n )", to: "hooks.tool = {}", test: "test/goal-plugin.test.js", }, diff --git a/scripts/packed-host-contract.mjs b/scripts/packed-host-contract.mjs index c99f5d0..792bac5 100644 --- a/scripts/packed-host-contract.mjs +++ b/scripts/packed-host-contract.mjs @@ -1,5 +1,6 @@ import assert from "node:assert/strict" import { execFileSync } from "node:child_process" +import { createHash } from "node:crypto" import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises" import { tmpdir } from "node:os" import { join } from "node:path" @@ -209,6 +210,116 @@ try { await hooks.dispose() await hooks.dispose() + // Prove the installed artifact keeps same-session contention graceful while + // retaining one durable writer and requiring an explicit takeover. + const persistentSessionID = "packed-host-passive-session" + const stateFilePath = join(projectDirectory, "packed-state.json") + const sessionKey = createHash("sha256").update(persistentSessionID).digest("hex") + const shardStatePath = join(`${stateFilePath}.sessions`, sessionKey, "state.json") + const shardLedgerPath = `${shardStatePath}.ledger.jsonl` + const passivePromptCalls = [] + let persistentMessages = [] + const persistentClient = { + app: { log: async () => {} }, + session: { + messages: async () => ({ data: persistentMessages }), + promptAsync: async (input) => { + passivePromptCalls.push(input) + return {} + }, + }, + } + const owner = await installed.GoalPlugin( + { client: persistentClient, directory: projectDirectory }, + { stateFilePath, minDelayMs: 1, noToolCallTurnsBeforePause: 10 }, + ) + const contender = await installed.GoalPlugin( + { client: persistentClient, directory: projectDirectory }, + { stateFilePath, minDelayMs: 1, noToolCallTurnsBeforePause: 10 }, + ) + try { + await owner["command.execute.before"]( + { + command: "goal", + sessionID: persistentSessionID, + arguments: "persisted installed-artifact objective", + }, + { parts: [] }, + ) + const stateBefore = await readFile(shardStatePath, "utf8") + const ledgerBefore = await readFile(shardLedgerPath, "utf8") + + await contender["chat.params"]({ sessionID: persistentSessionID, agent: "build" }) + const passiveMessageID = "packed-passive-command" + const passiveOutput = { + message: { id: passiveMessageID, role: "user", sessionID: persistentSessionID }, + parts: [], + } + await contender["command.execute.before"]( + { command: "goal", sessionID: persistentSessionID, arguments: "status" }, + passiveOutput, + ) + assert.match(passiveOutput.parts[0].text, /Goal controls are unavailable/) + Object.assign(passiveOutput.parts[0], { + id: "packed-passive-command-part", + messageID: passiveMessageID, + sessionID: persistentSessionID, + }) + await contender["chat.message"]( + { sessionID: persistentSessionID, messageID: passiveMessageID, agent: "build" }, + passiveOutput, + ) + const passiveAssistant = { + info: { + id: "packed-passive-command-assistant", + parentID: passiveMessageID, + role: "assistant", + sessionID: persistentSessionID, + }, + parts: [{ type: "text", text: "Ownership denial reported." }], + } + persistentMessages = [passiveAssistant] + await contender.event({ + event: { + type: "message.updated", + properties: { info: passiveAssistant.info }, + }, + }) + await contender.event({ + event: { + type: "session.status", + properties: { sessionID: persistentSessionID, status: { type: "idle" } }, + }, + }) + const passiveStatus = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID: persistentSessionID }), + ) + assert.equal(passiveStatus.ok, false) + assert.equal(passiveStatus.error, "session_owned_elsewhere") + assert.equal(await readFile(shardStatePath, "utf8"), stateBefore) + assert.equal(await readFile(shardLedgerPath, "utf8"), ledgerBefore) + assert.equal(passivePromptCalls.length, 0) + + await owner.dispose() + await new Promise((resolve) => setTimeout(resolve, 300)) + const takeover = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID: persistentSessionID }), + ) + assert.equal(takeover.ok, true) + assert.match(takeover.message, /persisted installed-artifact objective/) + assert.match(takeover.message, /recovered after restart|paused|stopped/i) + await contender.event({ + event: { + type: "session.status", + properties: { sessionID: persistentSessionID, status: { type: "idle" } }, + }, + }) + assert.equal(passivePromptCalls.length, 0) + } finally { + await owner.dispose() + await contender.dispose() + } + console.log( `packed host contract passed (${installedManifest.name}@${installedManifest.version}; ${packResult[0].size} byte tarball)`, ) diff --git a/src/goal-plugin.js b/src/goal-plugin.js index deaf19e..3997740 100644 --- a/src/goal-plugin.js +++ b/src/goal-plugin.js @@ -19,7 +19,10 @@ import { createOpenCodeSessionApi } from "./opencode-session-api.js" import { applyNativeGoalConfig } from "./native-agent-config.js" import { serializeCompletionClaim } from "./completion-claim.js" import { goalToolFailure, goalToolSuccess, serializeGoalToolResult } from "./goal-tool-result.js" -import { acquirePersistenceLease } from "./persistence-lease.js" +import { + acquirePersistenceLease, + isPersistenceLeaseContendedError, +} from "./persistence-lease.js" const STATE_FILE_VERSION = 1 // Default state now follows the project: /.opencode/goals/state.json. @@ -56,6 +59,11 @@ const DEFAULT_LEDGER_RETENTION_FILES = 3 const MAX_LEDGER_LINE_BYTES = 16 * 1024 const MIGRATION_LEASE_RETRIES = 200 const MIGRATION_LEASE_DELAY_MS = 25 +const PASSIVE_SESSION_RETRY_MS = 250 +const SESSION_OWNED_ELSEWHERE = "session_owned_elsewhere" +const ACTIVE_PERSISTENCE_DISABLED = Object.freeze({ kind: "active", persistence: "disabled" }) +const ACTIVE_PERSISTENCE_OWNED = Object.freeze({ kind: "active", persistence: "owned" }) +const PLUGIN_DISPOSED = Object.freeze({ kind: "disposed" }) const DEFAULT_OPTIONS = { maxTurns: 10, @@ -104,6 +112,7 @@ function createRuntimeState() { ledgerSink: null, sessionPersistence: new Map(), sessionLoadPromises: new Map(), + passiveSessions: new Map(), disposed: false, } } @@ -115,6 +124,19 @@ function currentRuntime() { return runtimeStorage.getStore() || lastRuntime } +function runtimeSessionDiagnostics(sessionID) { + const runtime = currentRuntime() + return Object.freeze({ + disposed: runtime.disposed, + loadInFlight: runtime.sessionLoadPromises.has(sessionID), + persistenceOwned: runtime.sessionPersistence.has(sessionID), + passive: runtime.passiveSessions.has(sessionID), + suppressedAssistantCount: [...runtime.suppressedCommandAssistants.values()] + .filter((ownerSessionID) => ownerSessionID === sessionID) + .length, + }) +} + // Route the existing domain helpers to the plugin instance associated with the // current async hook/tool execution. OpenCode caches imported plugin modules but // initializes their factories per workspace, so module-global Maps would let a @@ -343,6 +365,24 @@ function normalizeExecutionContext(value) { } } +function rememberSessionExecutionContext(sessionID, value, { replace = false } = {}) { + if (!sessionID) return null + const observed = normalizeExecutionContext(value) + if (!observed) return null + const runtime = currentRuntime() + if (replace) { + runtime.sessionExecutionContexts.set(sessionID, observed) + return observed + } + const previous = normalizeExecutionContext(runtime.sessionExecutionContexts.get(sessionID)) || {} + const merged = { + ...previous, + ...observed, + } + runtime.sessionExecutionContexts.set(sessionID, merged) + return merged +} + function continuationContextInput(goal) { const context = normalizeExecutionContext(goal?.executionContext) return context ? { ...context } : {} @@ -841,9 +881,13 @@ function clearRuntimeState() { runtime.activeCommandTurns.clear() runtime.ownedPluginMessages.clear() runtime.suppressedCommandAssistants.clear() + runtime.passiveSessions.clear() } -function clearSessionRuntimeState(sessionID) { +function clearSessionRuntimeState( + sessionID, + { preserveCommandSecurity = false, preserveExecutionContext = false } = {}, +) { const runtime = currentRuntime() for (const goal of sessionGoals.get(sessionID)?.values() || []) { for (const messageID of goal.messageIDs || []) { @@ -862,14 +906,17 @@ function clearSessionRuntimeState(sessionID) { runtime.continuationControllers.delete(sessionID) runtime.promptInFlightSessions.delete(sessionID) runtime.sessionStatuses.delete(sessionID) - runtime.sessionExecutionContexts.delete(sessionID) - runtime.pendingCommandTurns.delete(sessionID) - runtime.activeCommandTurns.delete(sessionID) - for (const [messageID, owner] of runtime.ownedPluginMessages) { - if (owner?.sessionID === sessionID) runtime.ownedPluginMessages.delete(messageID) - } - for (const [messageID, ownerSessionID] of runtime.suppressedCommandAssistants) { - if (ownerSessionID === sessionID) runtime.suppressedCommandAssistants.delete(messageID) + if (!preserveExecutionContext) runtime.sessionExecutionContexts.delete(sessionID) + runtime.passiveSessions.delete(sessionID) + if (!preserveCommandSecurity) { + runtime.pendingCommandTurns.delete(sessionID) + runtime.activeCommandTurns.delete(sessionID) + for (const [messageID, owner] of runtime.ownedPluginMessages) { + if (owner?.sessionID === sessionID) runtime.ownedPluginMessages.delete(messageID) + } + for (const [messageID, ownerSessionID] of runtime.suppressedCommandAssistants) { + if (ownerSessionID === sessionID) runtime.suppressedCommandAssistants.delete(messageID) + } } } @@ -1441,7 +1488,12 @@ async function applyParsedStateFile(raw, client, onlySessionID = null) { ) } - if (onlySessionID) clearSessionRuntimeState(onlySessionID) + if (onlySessionID) { + clearSessionRuntimeState(onlySessionID, { + preserveCommandSecurity: true, + preserveExecutionContext: true, + }) + } else clearRuntimeState() const focusBySession = new Map() @@ -1552,7 +1604,7 @@ async function acquireMigrationLease(stateFilePath, migrationMarkerPath) { try { return await acquirePersistenceLease(stateFilePath) } catch (error) { - if (!String(error?.message || error).includes("goal persistence is already owned")) throw error + if (!isPersistenceLeaseContendedError(error)) throw error lastError = error await new Promise((resolve) => setTimeout(resolve, MIGRATION_LEASE_DELAY_MS)) } @@ -1682,6 +1734,7 @@ async function migrateLegacyState(persistenceOptions, client) { ) if (!migrationLease) return try { + if (currentRuntime().disposed) return if (await pathExists(persistenceOptions.migrationMarkerPath)) return const state = await readPersistedStateFile(candidate.stateFilePath, client) @@ -1736,6 +1789,7 @@ async function migrateLegacyState(persistenceOptions, client) { // A fresh project has no aggregate or legacy state. Mark the namespace so a // later session does not repeatedly probe global fallback paths. + if (currentRuntime().disposed) return await writeMigrationMarker(persistenceOptions.migrationMarkerPath) } @@ -1777,7 +1831,12 @@ async function reconstructFromLedger(persistenceOptions, client, onlySessionID = ) if (!reconstructed.length) return "missing" - if (onlySessionID) clearSessionRuntimeState(onlySessionID) + if (onlySessionID) { + clearSessionRuntimeState(onlySessionID, { + preserveCommandSecurity: true, + preserveExecutionContext: true, + }) + } else clearRuntimeState() const focusCandidates = new Map() for (const stub of reconstructed) { @@ -1847,24 +1906,46 @@ async function persistState(persistence, client, sessionID) { } } -async function logPluginError(client, message, error) { +function dispatchAdvisoryHostCall(call, onFailure = () => {}) { + try { + // Host notices are diagnostic only. Start the SDK request immediately, + // contain both synchronous and asynchronous failures, and never let a + // stalled host promise retain a persistence lease or block goal controls. + void Promise.resolve(call()).catch(onFailure) + } catch (error) { + onFailure(error) + } +} + +async function logPluginMessage(client, level, message, error) { + const fallback = () => { + const logger = level === "warn" ? console.warn : console.error + logger("[goal-plugin]", message, error || "") + } if (client?.app?.log) { - try { - await client.app.log({ + return dispatchAdvisoryHostCall( + () => client.app.log({ body: { service: "opencode-goal-plugin", - level: "error", + level, message, - extra: { error: error?.message || error?.name || String(error) }, + ...(error === undefined + ? {} + : { extra: { error: error?.message || error?.name || String(error) } }), }, - }) - return - } catch { - // Logging must never poison persistence or leak an acquired lease. - } + }), + fallback, + ) } + fallback() +} - console.error("[goal-plugin]", message, error || "") +async function logPluginError(client, message, error) { + return logPluginMessage(client, "error", message, error) +} + +async function logPluginWarning(client, message) { + return logPluginMessage(client, "warn", message) } function parseGoalArguments(args, defaults) { @@ -2459,7 +2540,14 @@ function pluginMessageMatches(message, kind, correlationID) { return Boolean(correlationID) && pluginMessageCorrelationID(message, kind) === correlationID } -function rememberOwnedPluginMessage(message, sessionID, kind, correlationID, policy = "") { +function rememberOwnedPluginMessage( + message, + sessionID, + kind, + correlationID, + policy = "", + passive = false, +) { const id = messageID(message) if (!id) return setBoundedMessageValue(currentRuntime().ownedPluginMessages, id, { @@ -2467,9 +2555,34 @@ function rememberOwnedPluginMessage(message, sessionID, kind, correlationID, pol kind, correlationID, ...(policy ? { policy } : {}), + ...(passive ? { passive: true } : {}), }) } +function suppressControlCommandAssistant(message) { + const currentMessageID = messageID(message) + const currentSessionID = messageSessionID(message) + if (!currentMessageID || !currentSessionID) return false + const runtime = currentRuntime() + const parentOwner = runtime.ownedPluginMessages.get(messageParentID(message)) + const isControlCommandAssistant = + messageRole(message) === "assistant" && + parentOwner?.kind === "command" && + parentOwner?.policy === "control" && + parentOwner?.sessionID === currentSessionID + if (!isControlCommandAssistant) return false + // A control command may produce several assistant messages (for example, a + // blocked tool-call step followed by a final report). Authenticate each + // response through its owned parent user message and suppress it immediately + // so later idle processing cannot treat it as goal progress or completion. + setBoundedMessageValue( + runtime.suppressedCommandAssistants, + currentMessageID, + currentSessionID, + ) + return parentOwner?.passive === true ? "passive" : "control" +} + function isOwnedPluginMessage(message, kind, ownedMessages = currentRuntime().ownedPluginMessages) { const id = messageID(message) const correlationID = pluginMessageCorrelationID(message, kind) @@ -2522,17 +2635,27 @@ function isPluginGeneratedMessage(message, ownedMessages = currentRuntime().owne ) } +function pruneExpiredPendingCommandTurns(sessionID, now = Date.now()) { + const runtime = currentRuntime() + const pending = runtime.pendingCommandTurns.get(sessionID) + if (pending) { + for (const [id, turn] of pending) { + if (now - turn.createdAt > COMMAND_TURN_TTL_MS) pending.delete(id) + } + if (pending.size === 0) runtime.pendingCommandTurns.delete(sessionID) + } + +} + function registerPendingCommandTurn(sessionID, output) { const runtime = currentRuntime() const now = Date.now() + pruneExpiredPendingCommandTurns(sessionID, now) let pending = runtime.pendingCommandTurns.get(sessionID) if (!pending) { pending = new Map() runtime.pendingCommandTurns.set(sessionID, pending) } - for (const [id, turn] of pending) { - if (now - turn.createdAt > COMMAND_TURN_TTL_MS) pending.delete(id) - } while (pending.size >= MAX_PENDING_COMMAND_TURNS_PER_SESSION) { pending.delete(pending.keys().next().value) } @@ -2930,12 +3053,70 @@ function agentToolSessionID(ctx) { // helper's unrelated SDK/effect dependency graph in every consumer project. const bundledToolHelper = Object.assign((definition) => definition, { schema: z }) -function buildAgentTools(toolHelper, handlers, ensureSessionLoaded = async () => true) { +function sessionOwnedElsewhereMessage( + commandName = "goal", + commandRegistered = true, + reason = "owned_elsewhere", +) { + const retryTarget = commandRegistered + ? `\`/${commandName} status\`` + : "the `goal_status` tool" + if (reason === "legacy_lock") { + return ( + "Goal controls are unavailable because this session has an older or incomplete persistence lease. " + + "No goal state was read or changed here. Ordinary chat remains available. " + + "Close every OpenCode process using this session and upgrade them first. If the report persists, remove only the affected session shard's adjacent lease artifacts (`.lock` and `.lock.claims-v2`) or open a fork with `opencode --continue --fork`, " + + `then retry ${retryTarget}.` + ) + } + return ( + "Goal controls are unavailable in this OpenCode instance because another process owns this session's goal workflow. " + + "No goal state was read or changed here. Ordinary chat remains available. " + + `Close the owning process or open a fork with \`opencode --continue --fork\`, then retry ${retryTarget}.` + ) +} + +function inactiveGoalToolResult( + loadResult, + commandName = "goal", + disposed = false, + commandRegistered = true, +) { + if (disposed || loadResult?.kind === "disposed") { + return goalToolFailure("plugin_disposed", "The goal plugin is no longer active in this process.") + } + if (loadResult?.kind === "passive") { + return goalToolFailure( + SESSION_OWNED_ELSEWHERE, + sessionOwnedElsewhereMessage(commandName, commandRegistered, loadResult.reason), + ) + } + return null +} + +function buildAgentTools( + toolHelper, + handlers, + ensureSessionLoaded = async () => ACTIVE_PERSISTENCE_DISABLED, + commandName = "goal", + isDisposed = () => false, + commandRegistered = true, +) { const schema = toolHelper.schema const run = (handler) => async (args, ctx) => { const sessionID = agentToolSessionID(ctx) if (!sessionID) return "No session id available for the goal tool." - await ensureSessionLoaded(sessionID) + const loadResult = await ensureSessionLoaded(sessionID, { + retryPassive: true, + executionContext: ctx, + }) + const unavailable = inactiveGoalToolResult( + loadResult, + commandName, + isDisposed(), + commandRegistered, + ) + if (unavailable) return unavailable.message return handler(sessionID, args || {}) } // Canonical tools use a small, versioned machine-readable envelope. Keep the @@ -2949,7 +3130,17 @@ function buildAgentTools(toolHelper, handlers, ensureSessionLoaded = async () => goalToolFailure("missing_session", "No session id available for the goal tool."), ) } - await ensureSessionLoaded(sessionID) + const loadResult = await ensureSessionLoaded(sessionID, { + retryPassive: true, + executionContext: ctx, + }) + const unavailable = inactiveGoalToolResult( + loadResult, + commandName, + isDisposed(), + commandRegistered, + ) + if (unavailable) return serializeGoalToolResult(operation, unavailable) return serializeGoalToolResult(operation, await handler(sessionID, args || {})) } @@ -3118,24 +3309,24 @@ function formatGoalList(sessionID, commandName = "goal") { // once a non-prompting message API is available. async function defaultAuditMessenger(client, sessionID, text) { if (client?.app?.log) { - await client.app.log({ + dispatchAdvisoryHostCall(() => client.app.log({ body: { service: "opencode-goal-plugin", level: "info", message: text, extra: { sessionID, kind: "goal-audit" }, }, - }) + })) } if (client?.tui?.showToast) { - await client.tui.showToast({ + dispatchAdvisoryHostCall(() => client.tui.showToast({ body: { title: "Goal workflow", message: summarizeText(text, 500), variant: /rejected|failed|blocked/i.test(text) ? "warning" : "info", duration: 6000, }, - }) + })) } } @@ -3295,11 +3486,69 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) return persistence.persistChain } - const ensureSessionLoaded = async (sessionID) => { - if (!persistenceOptions.persistState || !sessionID) return true + const passiveLoadResult = (entry) => ({ + kind: "passive", + code: SESSION_OWNED_ELSEWHERE, + reason: entry.reason, + owner: entry.owner, + retryAt: entry.retryAt, + }) + + const enterPassiveSession = async (sessionID, error) => { + const previous = runtime.passiveSessions.get(sessionID) + clearSessionRuntimeState(sessionID, { + preserveCommandSecurity: Boolean(previous), + preserveExecutionContext: true, + }) + const entry = { + code: SESSION_OWNED_ELSEWHERE, + reason: error.reason, + owner: error.owner, + firstObservedAt: previous?.firstObservedAt || Date.now(), + retryAt: Date.now() + PASSIVE_SESSION_RETRY_MS, + warned: true, + } + runtime.passiveSessions.set(sessionID, entry) + if (!previous?.warned) { + const owner = entry.owner?.pid && entry.owner?.hostname + ? `pid ${entry.owner.pid} on ${entry.owner.hostname}` + : "another process" + const warning = entry.reason === "legacy_lock" + ? "Goal controls are passive for this session because its persistence lease is from an older release or is incomplete. Ordinary chat remains available. Close every OpenCode process using this session and upgrade them; if the report persists, remove only the affected session shard's adjacent lease artifacts (`.lock` and `.lock.claims-v2`) or fork the session before retrying goal controls." + : `Goal controls are passive for this session because ${owner} owns its persistence lease. Ordinary chat remains available; close the owner or fork the session before retrying goal controls.` + // Host logging is advisory. A broken or backpressured logger must not + // turn passive mode back into the session-wide hang it is meant to + // prevent, and the contained rejection avoids an unhandled promise. + void logPluginWarning( + client, + warning, + ).catch(() => {}) + } + return passiveLoadResult(entry) + } + + const ensureSessionLoaded = async ( + sessionID, + { retryPassive = false, executionContext, freshCommandBoundary = false } = {}, + ) => { + if (runtime.disposed) return PLUGIN_DISPOSED + rememberSessionExecutionContext(sessionID, executionContext) + if (!persistenceOptions.persistState || !sessionID) return ACTIVE_PERSISTENCE_DISABLED const existingLoad = runtime.sessionLoadPromises.get(sessionID) if (existingLoad) return existingLoad - if (runtime.sessionPersistence.has(sessionID)) return true + if (runtime.sessionPersistence.has(sessionID)) return ACTIVE_PERSISTENCE_OWNED + + const passive = runtime.passiveSessions.get(sessionID) + pruneExpiredPendingCommandTurns(sessionID) + const commandTurnInFlight = + runtime.pendingCommandTurns.has(sessionID) || + (!freshCommandBoundary && runtime.activeCommandTurns.has(sessionID)) + if ( + passive && + (!retryPassive || commandTurnInFlight || Date.now() < passive.retryAt) + ) { + return passiveLoadResult(passive) + } const load = (async () => { const paths = sessionPathsFor(persistenceOptions, sessionID) @@ -3307,20 +3556,36 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) ...persistenceOptions, stateFilePath: paths.stateFilePath, }) - const lease = await acquirePersistenceLease(paths.stateFilePath) + let lease + try { + lease = await acquirePersistenceLease(paths.stateFilePath) + } catch (error) { + if (!isPersistenceLeaseContendedError(error)) throw error + return enterPassiveSession(sessionID, error) + } + const releaseDisposedSession = async () => { + runtime.sessionPersistence.delete(sessionID) + await lease.release().catch(() => false) + return PLUGIN_DISPOSED + } + if (runtime.disposed) return releaseDisposedSession() const persistence = { ...persistenceOptions, ...paths, persistChain: Promise.resolve(true), lease, } + runtime.passiveSessions.delete(sessionID) runtime.sessionPersistence.set(sessionID, persistence) try { await migrateLegacyState(persistenceOptions, client) + if (runtime.disposed) return releaseDisposedSession() const status = await loadPersistedSessionState(persistence, client, sessionID) + if (runtime.disposed) return releaseDisposedSession() pruneGoalResults(defaultGoalOptions) if (status === "loaded" || status === "missing" || status === "reconstructed") await persist(sessionID) - return true + if (runtime.disposed) return releaseDisposedSession() + return ACTIVE_PERSISTENCE_OWNED } catch (error) { runtime.sessionPersistence.delete(sessionID) await lease.release().catch(() => false) @@ -3525,6 +3790,42 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) return goal } + const retireCompletedCommandTurnOnIdle = async (sessionID, messageLimit) => { + const runtime = currentRuntime() + const activeCommandTurn = runtime.activeCommandTurns.get(sessionID) + if (!activeCommandTurn) return { ready: true, messages: null } + + const commandHostMessages = await sessionApi.messages(sessionID, { + limit: messageLimit, + }) + if (runtime.disposed) return { ready: false, messages: null } + const commandMessages = Array.isArray(commandHostMessages) + ? commandHostMessages.slice(-messageLimit) + : [] + if (runtime.activeCommandTurns.get(sessionID) !== activeCommandTurn) { + return { ready: false, messages: commandMessages } + } + const commandAssistant = findLatestAssistantMessage(commandMessages) + if ( + !commandAssistant || + messageParentID(commandAssistant) !== activeCommandTurn.messageID + ) { + return { ready: false, messages: commandMessages } + } + if (activeCommandTurn.policy === "control") { + const commandAssistantID = messageID(commandAssistant) + if (commandAssistantID) { + setBoundedMessageValue( + runtime.suppressedCommandAssistants, + commandAssistantID, + sessionID, + ) + } + } + runtime.activeCommandTurns.delete(sessionID) + return { ready: true, messages: commandMessages } + } + const hooks = { config: async (config) => { applyNativeGoalConfig(config, { @@ -3535,20 +3836,29 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) }, "chat.params": async (input) => { if (!input?.sessionID) return - await ensureSessionLoaded(input.sessionID) - const context = normalizeExecutionContext({ - agent: input.agent, - model: input.model, - variant: input?.message?.model?.variant, + const loadResult = await ensureSessionLoaded(input.sessionID, { + executionContext: input, }) - if (context) currentRuntime().sessionExecutionContexts.set(input.sessionID, context) + if (currentRuntime().disposed || loadResult.kind === "disposed") return + rememberSessionExecutionContext( + input.sessionID, + { + agent: input.agent, + model: input.model, + variant: + input.variant ?? input?.model?.variant ?? input?.message?.model?.variant, + }, + { replace: true }, + ) }, "chat.message": async (input, output) => { const sessionID = input?.sessionID if (!sessionID) return - await ensureSessionLoaded(sessionID) - const context = normalizeExecutionContext(input) - if (context) currentRuntime().sessionExecutionContexts.set(sessionID, context) + const loadResult = await ensureSessionLoaded(sessionID, { + executionContext: input, + }) + if (currentRuntime().disposed) return + rememberSessionExecutionContext(sessionID, input, { replace: true }) const message = { info: isPlainObject(output?.message) @@ -3581,6 +3891,7 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) "command", commandTurn.id, commandTurn.policy, + commandTurn.passive === true, ) return } @@ -3590,6 +3901,7 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) // in flight; public synthetic/metadata fields alone are never trusted. runtime.pendingCommandTurns.delete(sessionID) runtime.activeCommandTurns.delete(sessionID) + if (loadResult.kind !== "active") return const continuationID = activeContinues.get(sessionID) if ( currentMessageID && @@ -3615,6 +3927,7 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) const sessionID = input?.sessionID if (!sessionID) return await ensureSessionLoaded(sessionID) + if (currentRuntime().disposed) return if (currentRuntime().activeCommandTurns.get(sessionID)?.policy !== "control") return throw new Error( `This /${commandName} control command has already been handled. Tool "${input?.tool || "unknown"}" was blocked because no tool calls are allowed while its result is being reported. Wait for a separate user turn before using tools or modifying work or goal state.`, @@ -3625,8 +3938,26 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) const sessionID = input.sessionID if (!sessionID) return - await ensureSessionLoaded(sessionID) - registerPendingCommandTurn(sessionID, output) + // A fresh slash command is an authenticated boundary that may retry a + // passive lease without waiting forever for an orphaned older reply. + // Keep the old active guard installed during the asynchronous load so + // tools from that older turn remain blocked; accepting this new command + // in chat.message atomically replaces the guard. + const loadResult = await ensureSessionLoaded(sessionID, { + retryPassive: true, + freshCommandBoundary: true, + }) + if (currentRuntime().disposed || loadResult.kind === "disposed") return + const commandTurn = registerPendingCommandTurn(sessionID, output) + + if (loadResult.kind === "passive") { + commandTurn.passive = true + replaceCommandOutputText( + output, + sessionOwnedElsewhereMessage(commandName, true, loadResult.reason), + ) + return + } if (typeof input.arguments !== "string") { replaceCommandOutputText(output, "Goal command arguments must be text.") @@ -4061,9 +4392,13 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) event: async ({ event }) => { const eventSessionID = getSessionID(event) || messageSessionID(messageInfoFromEvent(event)) - if (eventSessionID) await ensureSessionLoaded(eventSessionID) + const loadResult = eventSessionID + ? await ensureSessionLoaded(eventSessionID) + : ACTIVE_PERSISTENCE_DISABLED + if (currentRuntime().disposed || loadResult.kind === "disposed") return + const passive = loadResult.kind === "passive" - if (event?.type === "session.status") { + if (!passive && event?.type === "session.status") { const sessionID = getSessionID(event) const status = event?.properties?.status?.type || event?.data?.status?.type if (sessionID && status) currentRuntime().sessionStatuses.set(sessionID, status) @@ -4071,22 +4406,42 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) if (event?.type === "session.updated") { const sessionID = getSessionID(event) - const context = normalizeExecutionContext(event?.properties?.info || event?.data?.info) - if (sessionID && context) currentRuntime().sessionExecutionContexts.set(sessionID, context) + rememberSessionExecutionContext( + sessionID, + event?.properties?.info || event?.data?.info, + ) } - if (event?.type === "message.updated") { + if (!passive && event?.type === "message.updated") { const message = messageInfoFromEvent(event) if (messageRole(message) === "user") { - const context = normalizeExecutionContext(message) const sessionID = messageSessionID(message) || getSessionID(event) - if (sessionID && context) currentRuntime().sessionExecutionContexts.set(sessionID, context) + rememberSessionExecutionContext(sessionID, message) } } + const updatedMessage = event?.type === "message.updated" + ? messageInfoFromEvent(event) + : null + const controlCommandAssistant = updatedMessage + ? suppressControlCommandAssistant(updatedMessage) + : false + const terminal = terminalEvent(event) if (terminal?.sessionID) { const runtime = currentRuntime() + if (controlCommandAssistant) { + // A provider error on a plugin-owned control reply belongs to that + // read-only command turn, not to whichever goal may be active now. + // This is especially important after passive takeover: a delayed + // denial reply from the old lease epoch must not pause a newly + // resumed goal. Retire only the exact active guard it answers. + const activeCommandTurn = runtime.activeCommandTurns.get(terminal.sessionID) + if (activeCommandTurn?.messageID === messageParentID(updatedMessage)) { + runtime.activeCommandTurns.delete(terminal.sessionID) + } + return + } const pendingTurns = runtime.pendingCommandTurns.get(terminal.sessionID) const resolvingCommandTurn = [...(pendingTurns?.values() || [])].reverse().find( (turn) => turn.preservedFileCount > 0, @@ -4110,6 +4465,7 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) } if (!resolvingCommandAttachments) runtime.pendingCommandTurns.delete(terminal.sessionID) runtime.activeCommandTurns.delete(terminal.sessionID) + if (passive) return await pauseActiveGoal(terminal.sessionID, { ...(resolvingCommandAttachments ? { @@ -4126,6 +4482,24 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) return } + if (event?.type === "message.updated") { + if (passive || controlCommandAssistant === "passive") return + } + + if (passive) { + if (isIdleEvent(event) && eventSessionID) { + // A session-scoped idle can be stale or unrelated. Keep the passive + // command guard until the latest assistant is proven to answer the + // plugin-owned denial turn, matching the active-mode correlation + // contract below. + await retireCompletedCommandTurnOnIdle( + eventSessionID, + defaultGoalOptions.maxRecentMessages, + ) + } + return + } + if (event?.type === "session.compacted") { const sessionID = getSessionID(event) const goal = goalStates.get(sessionID) @@ -4144,25 +4518,6 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) if (!currentMessageID) return const currentSessionID = messageSessionID(message) const runtime = currentRuntime() - const parentOwner = runtime.ownedPluginMessages.get(messageParentID(message)) - const isControlCommandAssistant = - messageRole(message) === "assistant" && - parentOwner?.kind === "command" && - parentOwner?.policy === "control" && - parentOwner?.sessionID === currentSessionID - if (isControlCommandAssistant) { - // A control command may produce several assistant messages (for - // example, a blocked tool-call step followed by a final report), and - // another plugin turn may overlap before all message.updated events - // arrive. Authenticate each response through its owned parent user - // message instead of relying on the session's single latest-command - // slot, then suppress it immediately for later idle processing. - setBoundedMessageValue( - runtime.suppressedCommandAssistants, - currentMessageID, - currentSessionID, - ) - } const goal = goalStates.get(currentSessionID) if (!goal) return @@ -4247,37 +4602,15 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) // remain suppressed in a bounded map so a later duplicate idle cannot // reinterpret the same report as goal progress or completion. const runtime = currentRuntime() - const activeCommandTurn = runtime.activeCommandTurns.get(sessionID) - let commandMessages = null - if (activeCommandTurn) { - const commandMessageLimit = - goalStates.get(sessionID)?.options.maxRecentMessages || - defaultGoalOptions.maxRecentMessages - const commandHostMessages = await sessionApi.messages(sessionID, { - limit: commandMessageLimit, - }) - commandMessages = Array.isArray(commandHostMessages) - ? commandHostMessages.slice(-commandMessageLimit) - : [] - const commandAssistant = findLatestAssistantMessage(commandMessages) - if ( - !commandAssistant || - messageParentID(commandAssistant) !== activeCommandTurn.messageID - ) { - return - } - if (activeCommandTurn.policy === "control") { - const commandAssistantID = messageID(commandAssistant) - if (commandAssistantID) { - setBoundedMessageValue( - runtime.suppressedCommandAssistants, - commandAssistantID, - sessionID, - ) - } - } - runtime.activeCommandTurns.delete(sessionID) - } + const commandMessageLimit = + goalStates.get(sessionID)?.options.maxRecentMessages || + defaultGoalOptions.maxRecentMessages + const commandTurnState = await retireCompletedCommandTurnOnIdle( + sessionID, + commandMessageLimit, + ) + if (!commandTurnState.ready) return + const commandMessages = commandTurnState.messages const goal = goalStates.get(sessionID) if (!goal || goal.stopped || activeContinues.has(sessionID)) return @@ -4809,11 +5142,12 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) "experimental.chat.system.transform": async (input, output) => { if (!input.sessionID) return - await ensureSessionLoaded(input.sessionID) + const loadResult = await ensureSessionLoaded(input.sessionID) + if (currentRuntime().disposed || loadResult.kind === "disposed") return const activeCommandTurn = currentRuntime().activeCommandTurns.get(input.sessionID) const commandGuarded = activeCommandTurn?.policy === "control" - const goal = goalStates.get(input.sessionID) + const goal = loadResult.kind === "active" ? goalStates.get(input.sessionID) : null if (!goal && !commandGuarded) return const blockID = goal?.goalId || `command-${activeCommandTurn.id}` const systemBlocks = Array.isArray(output.system) ? [...output.system] : [] @@ -4870,7 +5204,8 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) "experimental.session.compacting": async (input, output) => { if (!input?.sessionID || !output) return - await ensureSessionLoaded(input.sessionID) + const loadResult = await ensureSessionLoaded(input.sessionID) + if (currentRuntime().disposed || loadResult.kind !== "active") return const goal = goalStates.get(input.sessionID) if (!goal) return const context = buildCompactionContext(goal) @@ -4890,7 +5225,8 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) // auto-continue to avoid two continuations racing after a compaction. // Paused/stopped goals leave the native behavior untouched. if (!input?.sessionID || !output) return - await ensureSessionLoaded(input.sessionID) + const loadResult = await ensureSessionLoaded(input.sessionID) + if (currentRuntime().disposed || loadResult.kind !== "active") return const goal = goalStates.get(input.sessionID) if (!goal || goal.stopped) return output.enabled = false @@ -4907,7 +5243,14 @@ async function createGoalPlugin({ client, directory } = {}, pluginOptions = {}) // makes this deterministic for normal npm installs; `registerTools: false` // remains the explicit opt-out. if (pluginOptions.registerTools !== false) { - hooks.tool = buildAgentTools(bundledToolHelper, agentToolHandlers, ensureSessionLoaded) + hooks.tool = buildAgentTools( + bundledToolHelper, + agentToolHandlers, + ensureSessionLoaded, + commandName, + () => runtime.disposed, + registerCommand, + ) } return hooks @@ -4987,6 +5330,7 @@ export default { } export const testInternals = { + commandTurnTtlMs: COMMAND_TURN_TTL_MS, activeGoal, agentToolSessionID, buildAgentToolHandlers, @@ -5042,6 +5386,7 @@ export const testInternals = { parseTokenBudget, pruneGoalResults, resolveStateFilePath, + runtimeSessionDiagnostics, stopReason, xdgStateFilePath, } diff --git a/src/persistence-lease.js b/src/persistence-lease.js index 50a97f8..3773905 100644 --- a/src/persistence-lease.js +++ b/src/persistence-lease.js @@ -1,7 +1,91 @@ import { randomUUID } from "node:crypto" -import { promises as fs } from "node:fs" +import { constants as fsConstants, promises as fs } from "node:fs" import { hostname } from "node:os" -import { dirname } from "node:path" +import { dirname, join } from "node:path" + +export const PERSISTENCE_LEASE_CONTENDED = "GOAL_PERSISTENCE_LEASE_CONTENDED" +const LEASE_PROTOCOL_VERSION = 2 +const LEGACY_SENTINEL_TOKEN = "opencode-goal-plugin-immutable-claims-v2" +const LEGACY_SENTINEL_HOSTNAME = "opencode-goal-plugin-v2.invalid" +const LEGACY_GUARD_MTIME_MS = Date.UTC(2100, 0, 1) +const LEGACY_GUARD_MTIME_TOLERANCE_MS = 2_000 +const CLAIM_DIRECTORY_SUFFIX = ".claims-v2" +const CLAIM_PREFIX = "claim-" +const CLAIM_SUFFIX = ".json" +const MAX_OWNER_FILE_BYTES = 4 * 1024 +const MAX_OWNER_TOKEN_LENGTH = 256 +const MAX_OWNER_HOSTNAME_LENGTH = 255 +const MAX_ACQUIRE_ATTEMPTS = 5 + +function validStoredHostname(value) { + return ( + typeof value === "string" && + value.length >= 1 && + value.length <= MAX_OWNER_HOSTNAME_LENGTH + ) +} + +function validDisplayHostname(value) { + return validStoredHostname(value) && /^[A-Za-z0-9._-]+$/.test(value) +} + +function validOwner(owner) { + return ( + owner !== null && + typeof owner === "object" && + !Array.isArray(owner) && + typeof owner.token === "string" && + owner.token.length >= 1 && + owner.token.length <= MAX_OWNER_TOKEN_LENGTH && + Number.isSafeInteger(owner.pid) && + owner.pid > 0 && + validStoredHostname(owner.hostname) + ) +} + +function sanitizeOwner(owner) { + const pid = Number.isSafeInteger(owner?.pid) && owner.pid > 0 ? owner.pid : null + const hostname = validDisplayHostname(owner?.hostname) ? owner.hostname : null + return Object.freeze({ pid, hostname }) +} + +function describeOwner(owner) { + return owner?.pid && owner?.hostname + ? `pid ${owner.pid} on ${owner.hostname}` + : "an unknown owner" +} + +export class PersistenceLeaseContendedError extends Error { + constructor(owner, reason = "owned_elsewhere") { + const safeOwner = sanitizeOwner(owner) + const safeReason = reason === "legacy_lock" ? "legacy_lock" : "owned_elsewhere" + super(safeReason === "legacy_lock" + ? "goal persistence uses a legacy or incomplete lease; close every OpenCode instance using this session, then remove its lease artifacts before retrying" + : `goal persistence is already owned by ${describeOwner(safeOwner)}; close the other OpenCode instance or open a fork`) + this.name = "PersistenceLeaseContendedError" + this.code = PERSISTENCE_LEASE_CONTENDED + this.owner = safeOwner + this.reason = safeReason + } +} + +export function isPersistenceLeaseContendedError(error) { + return error instanceof PersistenceLeaseContendedError +} + +function persistenceLeasePathError() { + const error = new Error("goal persistence lease paths must use their expected real file types") + error.code = "ERR_GOAL_PERSISTENCE_LEASE_PATH" + return error +} + +function persistenceLeaseHardLinkError() { + const error = new Error( + "goal persistence requires same-filesystem hard-link support for its compatibility guard", + ) + error.code = "ERR_GOAL_PERSISTENCE_LEASE_HARDLINK" + return error +} function processIsAlive(pid) { if (!Number.isSafeInteger(pid) || pid <= 0) return null @@ -14,81 +98,532 @@ function processIsAlive(pid) { } } -async function readOwner(lockPath) { +async function assertLockDirectory(lockPath) { + let lockInfo try { - return JSON.parse(await fs.readFile(`${lockPath}/owner.json`, "utf8")) - } catch { + lockInfo = await fs.lstat(lockPath) + } catch (error) { + if (error?.code === "ENOENT") return null + throw error + } + if (lockInfo.isSymbolicLink() || !lockInfo.isDirectory()) { + throw persistenceLeasePathError() + } + return lockInfo +} + +async function readBoundedOwnerFile(handle, expectedInfo) { + const buffer = Buffer.alloc(MAX_OWNER_FILE_BYTES + 1) + let bytesReadTotal = 0 + while (bytesReadTotal < buffer.length) { + const { bytesRead } = await handle.read( + buffer, + bytesReadTotal, + buffer.length - bytesReadTotal, + bytesReadTotal, + ) + if (bytesRead === 0) break + bytesReadTotal += bytesRead + } + + const finalInfo = await handle.stat() + if ( + !finalInfo.isFile() || + bytesReadTotal === 0 || + bytesReadTotal > MAX_OWNER_FILE_BYTES || + bytesReadTotal !== expectedInfo.size || + finalInfo.size !== expectedInfo.size || + finalInfo.dev !== expectedInfo.dev || + finalInfo.ino !== expectedInfo.ino + ) { return null } + return buffer.toString("utf8", 0, bytesReadTotal) +} + +async function readOwnerRecord(ownerPath) { + let ownerInfo + try { + ownerInfo = await fs.lstat(ownerPath) + } catch (error) { + if (error?.code === "ENOENT") return { status: "missing", owner: null, info: null } + throw error + } + if ( + ownerInfo.isSymbolicLink() || + !ownerInfo.isFile() || + ownerInfo.size === 0 || + ownerInfo.size > MAX_OWNER_FILE_BYTES + ) { + return { status: "malformed", owner: null, info: ownerInfo } + } + + let handle + try { + const flags = + fsConstants.O_RDONLY | + (fsConstants.O_NOFOLLOW ?? 0) | + (fsConstants.O_NONBLOCK ?? 0) + handle = await fs.open(ownerPath, flags) + const openedInfo = await handle.stat() + if ( + !openedInfo.isFile() || + openedInfo.size === 0 || + openedInfo.size > MAX_OWNER_FILE_BYTES || + openedInfo.dev !== ownerInfo.dev || + openedInfo.ino !== ownerInfo.ino + ) { + return { status: "malformed", owner: null, info: ownerInfo } + } + + const raw = await readBoundedOwnerFile(handle, openedInfo) + if (raw === null) return { status: "malformed", owner: null, info: ownerInfo } + const parsed = JSON.parse(raw) + return validOwner(parsed) + ? { status: "valid", owner: parsed, info: ownerInfo } + : { status: "malformed", owner: null, info: ownerInfo } + } catch (error) { + if (error instanceof SyntaxError || error?.code === "ENOENT" || error?.code === "ELOOP") { + return { status: "malformed", owner: null, info: ownerInfo } + } + throw error + } finally { + await handle?.close().catch(() => {}) + } +} + +async function readOwner(lockPath) { + let lockInfo + try { + lockInfo = await fs.lstat(lockPath) + } catch (error) { + if (error?.code === "ENOENT") return null + throw error + } + if (lockInfo.isSymbolicLink()) throw persistenceLeasePathError() + const ownerPath = lockInfo.isDirectory() + ? join(lockPath, "owner.json") + : lockInfo.isFile() + ? lockPath + : null + if (!ownerPath) throw persistenceLeasePathError() + const record = await readOwnerRecord(ownerPath) + return record.status === "valid" ? record.owner : null +} + +function ownerIsBlocking(owner, localHostname) { + if (owner.hostname !== localHostname) return true + return processIsAlive(owner.pid) !== false +} + +function claimNameFor(token) { + return `${CLAIM_PREFIX}${token}${CLAIM_SUFFIX}` +} + +function isClaimLikeName(name) { + return name.startsWith(CLAIM_PREFIX) && name.endsWith(CLAIM_SUFFIX) +} + +function tokenFromClaimName(name) { + if (!isClaimLikeName(name)) return null + const token = name.slice(CLAIM_PREFIX.length, -CLAIM_SUFFIX.length) + return /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(token) + ? token + : null +} + +async function writeAtomicJSON(path, value) { + const temporaryPath = `${path}.${process.pid}.${randomUUID()}.tmp` + try { + await fs.writeFile(temporaryPath, JSON.stringify(value), { mode: 0o600, flag: "wx" }) + await fs.rename(temporaryPath, path) + } finally { + await fs.unlink(temporaryPath).catch((error) => { + if (error?.code !== "ENOENT") throw error + }) + } +} + +function legacySentinel() { + return { + protocol: LEASE_PROTOCOL_VERSION, + sentinel: true, + token: LEGACY_SENTINEL_TOKEN, + pid: 1, + hostname: LEGACY_SENTINEL_HOSTNAME, + createdAt: Date.now(), + } +} + +function validLegacySentinel(owner) { + return ( + validOwner(owner) && + Object.keys(owner).sort().join(",") === + "createdAt,hostname,pid,protocol,sentinel,token" && + owner.protocol === LEASE_PROTOCOL_VERSION && + owner.sentinel === true && + owner.token === LEGACY_SENTINEL_TOKEN && + owner.pid === 1 && + owner.hostname === LEGACY_SENTINEL_HOSTNAME && + Number.isFinite(owner.createdAt) && + owner.createdAt >= 0 + ) +} + +function legacyGuardMtimeIsSafe(info) { + return ( + Number.isFinite(info?.mtimeMs) && + info.mtimeMs >= LEGACY_GUARD_MTIME_MS - LEGACY_GUARD_MTIME_TOLERANCE_MS + ) +} + +function claimDirectoryPathFor(lockPath) { + return `${lockPath}${CLAIM_DIRECTORY_SUFFIX}` +} + +async function inspectLegacyGuard(lockPath) { + let info + try { + info = await fs.lstat(lockPath) + } catch (error) { + if (error?.code === "ENOENT") return { status: "missing", owner: null, info: null } + throw error + } + if (info.isSymbolicLink() || (!info.isFile() && !info.isDirectory())) { + throw persistenceLeasePathError() + } + if (info.isDirectory()) { + const legacy = await readOwnerRecord(join(lockPath, "owner.json")) + return { status: "legacy", owner: legacy.owner, info } + } + + const record = await readOwnerRecord(lockPath) + if ( + record.status === "valid" && + validLegacySentinel(record.owner) && + legacyGuardMtimeIsSafe(record.info) + ) { + return { status: "valid", owner: record.owner, info: record.info } + } + return { status: "incomplete", owner: record.owner, info: record.info } +} + +function throwForGuardStatus(guard) { + if (guard.status === "valid" || guard.status === "missing") return + throw new PersistenceLeaseContendedError(guard.owner, "legacy_lock") +} + +function hardLinkUnsupported(error) { + return ["EPERM", "EOPNOTSUPP", "ENOTSUP", "EXDEV"].includes(error?.code) +} + +async function publishLegacyGuard( + lockPath, + { beforeGuardLink, afterGuardLink, linkGuard = (source, target) => fs.link(source, target) } = {}, +) { + const temporaryPath = `${lockPath}.guard.${process.pid}.${randomUUID()}.tmp` + const sentinel = legacySentinel() + let handle + let preparedInfo + let linked = false + try { + handle = await fs.open(temporaryPath, fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL, 0o600) + await handle.writeFile(JSON.stringify(sentinel)) + await handle.sync() + const guardDate = new Date(LEGACY_GUARD_MTIME_MS) + await handle.utimes(guardDate, guardDate) + await handle.sync() + preparedInfo = await handle.stat() + if (!preparedInfo.isFile() || !legacyGuardMtimeIsSafe(preparedInfo)) { + throw persistenceLeasePathError() + } + await handle.close() + handle = null + + await beforeGuardLink?.({ lockPath, temporaryPath, sentinel: { ...sentinel } }) + try { + await linkGuard(temporaryPath, lockPath) + linked = true + } catch (error) { + const racedGuard = await inspectLegacyGuard(lockPath) + if (racedGuard.status !== "missing") { + throwForGuardStatus(racedGuard) + return racedGuard + } + if (error?.code === "EEXIST") return null + if (hardLinkUnsupported(error)) throw persistenceLeaseHardLinkError() + throw error + } + await afterGuardLink?.({ lockPath, temporaryPath, sentinel: { ...sentinel } }) + + const guard = await inspectLegacyGuard(lockPath) + if (guard.status === "missing") return null + throwForGuardStatus(guard) + if ( + !linked || + guard.info.dev !== preparedInfo.dev || + guard.info.ino !== preparedInfo.ino + ) { + throw persistenceLeasePathError() + } + return guard + } finally { + await handle?.close().catch(() => {}) + await fs.unlink(temporaryPath).catch((error) => { + if (error?.code !== "ENOENT") throw error + }) + } +} + +async function ensureLegacyGuard(lockPath, hooks = {}) { + for (let attempt = 0; attempt < MAX_ACQUIRE_ATTEMPTS; attempt += 1) { + const guard = await inspectLegacyGuard(lockPath) + if (guard.status === "valid") return guard + if (guard.status !== "missing") { + throwForGuardStatus(guard) + } + const published = await publishLegacyGuard(lockPath, hooks) + if (published) return published + await retryDelay(randomUUID(), attempt) + } + throw new PersistenceLeaseContendedError(null) +} + +async function ensureClaimDirectory(claimDirectoryPath) { + try { + await fs.mkdir(claimDirectoryPath, { mode: 0o700 }) + } catch (error) { + if (error?.code !== "EEXIST") throw error + } + return assertLockDirectory(claimDirectoryPath) +} + +async function removeUniqueClaim(claimPath) { + try { + await fs.unlink(claimPath) + return true + } catch (error) { + if (error?.code === "ENOENT") return false + throw error + } +} + +async function inspectClaims( + lockPath, + ownToken, + localHostname, + { malformedGraceMs, now }, +) { + const entries = await fs.readdir(lockPath, { withFileTypes: true }) + let ownFound = false + for (const entry of entries.sort((left, right) => left.name.localeCompare(right.name))) { + const expectedToken = tokenFromClaimName(entry.name) + if (!expectedToken) { + if (isClaimLikeName(entry.name)) { + return { blocker: null, blocked: true, ownFound } + } + continue + } + const claimPath = join(lockPath, entry.name) + const record = await readOwnerRecord(claimPath) + if (record.status === "missing") continue + + const structurallyValidClaim = + record.status === "valid" && + record.owner.token === expectedToken + if (!structurallyValidClaim) { + const age = record.info ? now() - record.info.mtimeMs : 0 + if (age < malformedGraceMs) { + return { blocker: null, blocked: true, ownFound } + } + await removeUniqueClaim(claimPath) + continue + } + + // A token-matching claim from an unknown protocol is authoritative to that + // protocol. Never infer that it is stale or safe to remove. + if (record.owner.protocol !== LEASE_PROTOCOL_VERSION) { + return { blocker: record.owner, blocked: true, ownFound } + } + + if (record.owner.token === ownToken) { + ownFound = true + continue + } + if (ownerIsBlocking(record.owner, localHostname)) { + return { blocker: record.owner, blocked: true, ownFound } + } + await removeUniqueClaim(claimPath) + } + return { blocker: null, blocked: false, ownFound } +} + +function retryDelay(token, attempt) { + const offset = Number.parseInt(token.slice(attempt * 2, attempt * 2 + 2), 16) || 0 + return new Promise((resolve) => setTimeout(resolve, 1 + (offset % 7))) +} + +function createLease( + lockPath, + claimDirectoryPath, + claimPath, + owner, + { beforeClaimRemove } = {}, +) { + let releasing = false + let released = false + return { + lockPath, + claimDirectoryPath, + owner, + async release() { + if (released || releasing) return false + releasing = true + try { + const claim = await readOwnerRecord(claimPath) + if (claim.status === "missing") { + released = true + return false + } + if (claim.status !== "valid" || claim.owner.token !== owner.token) return false + + await beforeClaimRemove?.({ + lockPath, + claimDirectoryPath, + claimPath, + owner: { ...owner }, + }) + const removed = await removeUniqueClaim(claimPath) + if (removed) released = true + return removed + } finally { + releasing = false + } + }, + } } /** - * Hold an exclusive session lease for the plugin instance lifetime. This - * deliberately rejects a second writer instead of allowing stale snapshots to - * overwrite the same session's state. + * Hold an exclusive session lease for the plugin instance lifetime. + * + * Version 2 atomically publishes a long-lived regular-file guard at the legacy + * `.lock` path. Its far-future mtime makes version-1's malformed-directory + * recovery fail closed, while hard-link publication ensures the legacy path is + * never visible partially initialized. Version-2 peers elect ownership from + * never-reused UUID claims in a stable sibling directory. Stale cleanup and + * release therefore unlink only an immutable claim; neither operation can + * delete a newer owner's lease after a delayed filesystem call. */ -export async function acquirePersistenceLease( +async function acquirePersistenceLeaseWithHooks( stateFilePath, { malformedGraceMs = 30_000, now = () => Date.now() } = {}, + hooks = {}, ) { + const { + beforeGuardLink, + afterGuardLink, + linkGuard, + beforeOwnerWrite, + afterOwnerWrite, + beforeClaimRemove, + } = hooks const lockPath = `${stateFilePath}.lock` + const claimDirectoryPath = claimDirectoryPathFor(lockPath) + const localHostname = hostname() await fs.mkdir(dirname(stateFilePath), { recursive: true, mode: 0o700 }) - const owner = { - token: randomUUID(), - pid: process.pid, - hostname: hostname(), - createdAt: Date.now(), - } + let lastBlocker = null - for (let attempt = 0; attempt < 3; attempt += 1) { + for (let attempt = 0; attempt < MAX_ACQUIRE_ATTEMPTS; attempt += 1) { + await ensureLegacyGuard(lockPath, { beforeGuardLink, afterGuardLink, linkGuard }) + if (!(await ensureClaimDirectory(claimDirectoryPath))) { + continue + } + + const existing = await inspectClaims( + claimDirectoryPath, + null, + localHostname, + { malformedGraceMs, now }, + ) + if (existing.blocked) throw new PersistenceLeaseContendedError(existing.blocker) + const owner = { + protocol: LEASE_PROTOCOL_VERSION, + token: randomUUID(), + pid: process.pid, + hostname: localHostname, + createdAt: Date.now(), + } + const claimPath = join(claimDirectoryPath, claimNameFor(owner.token)) + let claimPublished = false try { - await fs.mkdir(lockPath, { mode: 0o700 }) - await fs.writeFile(`${lockPath}/owner.json`, JSON.stringify(owner), { mode: 0o600 }) - return { + await beforeOwnerWrite?.({ lockPath, - owner, - async release() { - const current = await readOwner(lockPath) - if (current?.token !== owner.token) return false - await fs.rm(lockPath, { recursive: true, force: true }) - return true - }, - } - } catch (error) { - if (error?.code !== "EEXIST") { - await fs.rm(lockPath, { recursive: true, force: true }).catch(() => {}) - throw error - } - const existing = await readOwner(lockPath) - const sameHost = existing?.hostname === owner.hostname - let reclaimableMalformed = false - if (!existing) { - try { - const info = await fs.lstat(lockPath) - reclaimableMalformed = now() - info.mtimeMs >= malformedGraceMs - } catch (statError) { - if (statError?.code === "ENOENT") continue - } - } - if ((sameHost && processIsAlive(existing?.pid) === false) || reclaimableMalformed) { - const stalePath = `${lockPath}.stale.${randomUUID()}` - try { - await fs.rename(lockPath, stalePath) - await fs.rm(stalePath, { recursive: true, force: true }) - continue - } catch (reclaimError) { - if (reclaimError?.code === "ENOENT") continue - } + claimDirectoryPath, + claimPath, + owner: { ...owner }, + attempt, + }) + await writeAtomicJSON(claimPath, owner) + claimPublished = true + await afterOwnerWrite?.({ + lockPath, + claimDirectoryPath, + claimPath, + owner: { ...owner }, + attempt, + }) + + const observed = await inspectClaims( + claimDirectoryPath, + owner.token, + localHostname, + { malformedGraceMs, now }, + ) + if (!observed.ownFound || observed.blocked) { + lastBlocker = observed.blocker + await removeUniqueClaim(claimPath) + claimPublished = false + await retryDelay(owner.token, attempt) + continue } - const description = existing - ? `pid ${existing.pid} on ${existing.hostname}` - : "an unknown owner" - throw new Error( - `goal persistence is already owned by ${description}; close the other OpenCode instance or configure a different stateFilePath`, + + await ensureLegacyGuard(lockPath, { beforeGuardLink, afterGuardLink, linkGuard }) + return createLease( + lockPath, + claimDirectoryPath, + claimPath, + owner, + { beforeClaimRemove }, ) + } catch (error) { + if (claimPublished) await removeUniqueClaim(claimPath).catch(() => false) + if (error?.code === "ENOENT") continue + throw error } } - throw new Error("could not acquire goal persistence lease") + throw new PersistenceLeaseContendedError(lastBlocker) +} + +export async function acquirePersistenceLease(stateFilePath, options = {}) { + return acquirePersistenceLeaseWithHooks(stateFilePath, options) } -export const persistenceLeaseInternals = Object.freeze({ processIsAlive, readOwner }) +export const persistenceLeaseInternals = Object.freeze({ + acquirePersistenceLeaseWithHooks, + claimDirectoryPathFor, + claimNameFor, + inspectLegacyGuard, + inspectClaims, + legacyGuardMtimeIsSafe, + legacySentinel, + publishLegacyGuard, + processIsAlive, + readBoundedOwnerFile, + readOwner, + readOwnerRecord, + sanitizeOwner, + validLegacySentinel, + validDisplayHostname, + validOwner, + validStoredHostname, +}) diff --git a/test/goal-plugin.test.js b/test/goal-plugin.test.js index b161874..3351316 100644 --- a/test/goal-plugin.test.js +++ b/test/goal-plugin.test.js @@ -50,6 +50,7 @@ const { readLedgerEntries, reconstructGoalsFromLedger, resolveStateFilePath, + runtimeSessionDiagnostics, sessionPathsFor, setLedgerSink, stopReason, @@ -4000,27 +4001,33 @@ test("overlapping hooks for one session await its in-flight lazy load", async () hooks = await GoalPlugin({ client }, { persistState: true, stateFilePath, minDelayMs: 1 }) loading = hooks["chat.params"]({ sessionID, agent: "build" }) - const shardLockPath = `${sessionStatePath(stateFilePath, sessionID)}.lock` - let shardLeaseObserved = false + let loadingStateObserved = false for (let attempt = 0; attempt < 100; attempt += 1) { - try { - await stat(shardLockPath) - shardLeaseObserved = true + const diagnostics = runtimeSessionDiagnostics(sessionID) + if (diagnostics.persistenceOwned && diagnostics.loadInFlight) { + loadingStateObserved = true break - } catch (error) { - if (error?.code !== "ENOENT") throw error - await new Promise((resolve) => setTimeout(resolve, 5)) } + await new Promise((resolve) => setTimeout(resolve, 5)) } - assert.equal(shardLeaseObserved, true) + assert.equal( + loadingStateObserved, + true, + "the lease must be installed while the same session load remains in flight", + ) let creationFinished = false creating = runGoal(hooks, sessionID, "new goal created during load").then((text) => { creationFinished = true return text }) - await new Promise((resolve) => setTimeout(resolve, 50)) + for (let turn = 0; turn < 10; turn += 1) await Promise.resolve() assert.equal(creationFinished, false, "the second hook must wait for the session load") + assert.equal( + currentGoal(sessionID), + null, + "the second hook must not mutate goal state while the session load is in flight", + ) await migrationBlocker.release() await loading @@ -4036,6 +4043,93 @@ test("overlapping hooks for one session await its in-flight lazy load", async () } }) +test("dispose during a blocked lazy load releases ownership without running goal controls", async () => { + const directory = await mkdtemp(join(tmpdir(), "goal-plugin-dispose-load-race-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "dispose-load-race" + const shardStatePath = sessionStatePath(stateFilePath, sessionID) + const migrationMarkerPath = join(`${stateFilePath}.sessions`, ".migration-v1-complete") + await writeFile( + stateFilePath, + JSON.stringify({ version: 1, goals: [], results: [] }), + ) + + const migrationBlocker = await acquirePersistenceLease(stateFilePath) + const client = { + app: { log: async () => {} }, + session: { + messages: async () => ({ data: [] }), + promptAsync: async () => ({}), + }, + } + const originalParts = [{ type: "text", text: "host-owned sentinel" }] + const output = { parts: originalParts } + let hooks + let command + let toolCall + let disposing + let replacement + try { + hooks = await GoalPlugin({ client }, { persistState: true, stateFilePath, minDelayMs: 1 }) + command = hooks["command.execute.before"]( + { command: "goal", sessionID, arguments: "must never be created" }, + output, + ) + toolCall = hooks.tool.goal_set.execute( + { objective: "must never be set by a tool" }, + { sessionID }, + ) + + const shardLockPath = `${shardStatePath}.lock` + let blockedMigrationObserved = false + for (let attempt = 0; attempt < 100; attempt += 1) { + const diagnostics = runtimeSessionDiagnostics(sessionID) + if (diagnostics.persistenceOwned && diagnostics.loadInFlight) { + blockedMigrationObserved = true + break + } + await new Promise((resolve) => setTimeout(resolve, 5)) + } + assert.equal( + blockedMigrationObserved, + true, + "the session must own its shard while blocked on the legacy migration lease", + ) + await stat(shardLockPath) + + disposing = hooks.dispose() + await migrationBlocker.release() + await disposing + await command + const toolResult = JSON.parse(await toolCall) + + assert.strictEqual(output.parts, originalParts) + assert.deepEqual(output.parts, [{ type: "text", text: "host-owned sentinel" }]) + assert.equal(toolResult.ok, false) + assert.equal(toolResult.error, "plugin_disposed") + assert.equal(currentGoal(sessionID), null) + await assert.rejects(readFile(shardStatePath, "utf8"), { code: "ENOENT" }) + const legacyGuard = JSON.parse(await readFile(shardLockPath, "utf8")) + assert.equal(legacyGuard.sentinel, true) + assert.deepEqual( + (await readdir(`${shardLockPath}.claims-v2`)).filter((name) => name.startsWith("claim-")), + [], + ) + await assert.rejects(stat(migrationMarkerPath), { code: "ENOENT" }) + assert.match(await readFile(stateFilePath, "utf8"), /"version":1/) + + replacement = await GoalPlugin({ client }, { persistState: true, stateFilePath, minDelayMs: 1 }) + const replacementText = await runGoal(replacement, sessionID, "replacement owns the session") + assert.match(replacementText, /New active goal: replacement owns the session/) + } finally { + await migrationBlocker.release().catch(() => false) + await Promise.allSettled([command, toolCall, disposing].filter(Boolean)) + await replacement?.dispose() + await hooks?.dispose() + await rm(directory, { recursive: true, force: true }) + } +}) + test("buildLimitWarning reports remaining seconds when duration is nearly exhausted", () => { const warning = buildLimitWarning({ turnCount: 0, diff --git a/test/host-lifecycle.test.js b/test/host-lifecycle.test.js index 13f372e..d9882fc 100644 --- a/test/host-lifecycle.test.js +++ b/test/host-lifecycle.test.js @@ -1,7 +1,7 @@ import assert from "node:assert/strict" import { promises as fs } from "node:fs" import { tmpdir } from "node:os" -import { join } from "node:path" +import { dirname, join } from "node:path" import test from "node:test" import { GoalPlugin, testInternals } from "../src/goal-plugin.js" @@ -72,6 +72,68 @@ async function idle(hooks, sessionID) { }) } +let commandMessageCounter = 0 +async function acceptCommandTurn(hooks, sessionID, output) { + commandMessageCounter += 1 + const messageID = `command-user-${commandMessageCounter}` + output.message = { id: messageID, role: "user", sessionID } + for (const [index, part] of output.parts.entries()) { + Object.assign(part, { + id: part.id || `command-part-${commandMessageCounter}-${index}`, + messageID, + sessionID, + }) + } + await hooks["chat.message"]( + { sessionID, messageID, agent: "build" }, + output, + ) + return messageID +} + +async function finishControlTurn( + hooks, + sessionID, + parentID, + text = "Reported.", + observeAssistant, +) { + const info = { + id: `command-assistant-${parentID}`, + parentID, + role: "assistant", + sessionID, + tokens: { input: 5, output: 5, reasoning: 0 }, + } + observeAssistant?.({ info, parts: [{ type: "text", text }] }) + await hooks.event({ + event: { + type: "message.updated", + properties: { info }, + }, + }) + await idle(hooks, sessionID) +} + +async function readMaybe(path) { + try { + return await fs.readFile(path, "utf8") + } catch (error) { + if (error?.code === "ENOENT") return null + throw error + } +} + +async function assertNoLeaseOwner(lockPath) { + const claims = (await fs.readdir(`${lockPath}.claims-v2`)) + .filter((name) => name.startsWith("claim-")) + assert.deepEqual(claims, []) + const legacyGuard = await readMaybe(lockPath) + if (legacyGuard !== null) { + assert.equal(JSON.parse(legacyGuard).sentinel, true) + } +} + test("initializing a second workspace does not clear or take ownership of the first workspace", async () => { const firstCalls = [] const first = await createPlugin( @@ -243,6 +305,32 @@ test("dispose prevents a delayed idle continuation from reaching the host", asyn assert.equal(promptCalls.length, 0) }) +test("dispose wins goal command and tool continuations that already received an active load result", async () => { + const hooks = await GoalPlugin( + { client: hostClient() }, + { persistState: false, minDelayMs: 1 }, + ) + const parts = [{ type: "text", text: "host-owned sentinel" }] + const output = { parts } + + const command = hooks["command.execute.before"]( + { command: "goal", sessionID: "dispose-control-race", arguments: "must not run" }, + output, + ) + const toolCall = hooks.tool.goal_set.execute( + { objective: "must not run from tool" }, + { sessionID: "dispose-control-race" }, + ) + const disposing = hooks.dispose() + + await Promise.all([command, disposing]) + const toolResult = JSON.parse(await toolCall) + assert.strictEqual(output.parts, parts) + assert.deepEqual(output.parts, [{ type: "text", text: "host-owned sentinel" }]) + assert.equal(toolResult.ok, false) + assert.equal(toolResult.error, "plugin_disposed") +}) + test("resuming keeps stable goal identity and invalidates the prior run epoch", async () => { const hooks = await createPlugin(hostClient(), "/workspace/resume-identity") const sessionID = "session-resume-identity" @@ -328,6 +416,895 @@ test("disposed instance cannot persist over a replacement instance after a late } }) +test("a same-session contender stays passive across chat, commands, hooks, and control security", async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-hooks-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "passive-hooks-session" + const paths = sessionPaths(stateFilePath, sessionID) + const logs = [] + const messagesCalls = [] + const promptCalls = [] + let hostMessages = [] + let owner + let contender + try { + owner = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, minDelayMs: 1, noToolCallTurnsBeforePause: 10 }, + ) + await setGoal(owner, sessionID, "owner objective") + const stateBefore = await fs.readFile(paths.stateFilePath, "utf8") + const ledgerBefore = await readMaybe(paths.ledgerFilePath) + const lockBefore = await fs.readFile(`${paths.stateFilePath}.lock`, "utf8") + + contender = await GoalPlugin( + { + client: { + app: { log: async (input) => logs.push(input) }, + session: { + messages: async (input) => { + messagesCalls.push(input) + return { data: hostMessages } + }, + promptAsync: async (input) => { + promptCalls.push(input) + return {} + }, + }, + }, + directory, + }, + { + stateFilePath, + commandName: "objective", + minDelayMs: 1, + noToolCallTurnsBeforePause: 10, + }, + ) + + await contender["chat.params"]({ + sessionID, + agent: "build", + model: { providerID: "test", modelID: "ordinary" }, + }) + const ordinaryParts = [{ type: "text", text: "ordinary chat must remain unchanged" }] + const ordinaryOutput = { + message: { id: "ordinary-user", role: "user", sessionID }, + parts: ordinaryParts, + } + await contender["chat.message"]( + { sessionID, messageID: "ordinary-user", agent: "build" }, + ordinaryOutput, + ) + assert.strictEqual(ordinaryOutput.parts, ordinaryParts) + assert.equal(ordinaryOutput.parts[0].text, "ordinary chat must remain unchanged") + await assert.doesNotReject(() => + contender["tool.execute.before"]({ sessionID, tool: "read" }), + ) + + const systemOutput = { system: ["base system"] } + await contender["experimental.chat.system.transform"]({ sessionID }, systemOutput) + assert.deepEqual(systemOutput, { system: ["base system"] }) + const compactOutput = { context: ["base context"] } + await contender["experimental.session.compacting"]({ sessionID }, compactOutput) + assert.deepEqual(compactOutput, { context: ["base context"] }) + const autoOutput = { enabled: true } + await contender["experimental.compaction.autocontinue"]({ sessionID }, autoOutput) + assert.deepEqual(autoOutput, { enabled: true }) + await contender.event({ + event: { + type: "session.updated", + properties: { sessionID, info: { agent: "build" } }, + }, + }) + await idle(contender, sessionID) + + const commandForms = [ + "", + "status", + "history", + "list", + "pause", + "clear", + "stop", + "off", + "reset", + "none", + "cancel", + "resume", + "edit private revised objective", + "focus 1", + "add private background objective", + "sequence private alpha; private beta", + "sisyphus private alpha; private beta", + "private replacement objective", + ] + for (const args of commandForms) { + const retained = [ + { type: "text", text: args }, + { type: "file", url: "file:///private/proof.txt", mime: "text/plain" }, + ] + const output = { parts: retained } + await contender["command.execute.before"]( + { command: "objective", sessionID, arguments: args }, + output, + ) + assert.strictEqual(output.parts, retained) + assert.equal(output.parts.length, 1) + assert.equal(output.parts[0].synthetic, true) + assert.match(output.parts[0].text, /Goal controls are unavailable/) + assert.match(output.parts[0].text, /\/objective status/) + assert.doesNotMatch(output.parts[0].text, /private (?:revised|background|replacement|alpha|beta)/) + assert.doesNotMatch(output.parts[0].text, /file:\/\/|state\.json|pid \d+/i) + const parentID = await acceptCommandTurn(contender, sessionID, output) + if (args === commandForms[0]) { + await idle(contender, sessionID) + await assert.rejects( + contender["tool.execute.before"]({ sessionID, tool: "write" }), + /no tool calls are allowed/i, + ) + } + await assert.rejects( + contender["tool.execute.before"]({ sessionID, tool: "goal_set" }), + /no tool calls are allowed/i, + ) + const guardedSystem = { system: [] } + await contender["experimental.chat.system.transform"]( + { sessionID }, + guardedSystem, + ) + assert.equal(guardedSystem.system.length, 1) + assert.match(guardedSystem.system[0], /control-command/) + assert.doesNotMatch(guardedSystem.system[0], /owner objective|pid \d+|state\.json/i) + await finishControlTurn( + contender, + sessionID, + parentID, + "Reported.", + (message) => { hostMessages = [message] }, + ) + await assert.doesNotReject(() => + contender["tool.execute.before"]({ sessionID, tool: "read" }), + ) + } + + assert.ok(messagesCalls.length >= commandForms.length + 1) + assert.equal(promptCalls.length, 0) + assert.equal(logs.length, 1) + assert.equal(logs[0].body.level, "warn") + assert.match(logs[0].body.message, /Ordinary chat remains available/) + assert.doesNotMatch(logs[0].body.message, /state\.json|token/i) + assert.equal(await fs.readFile(paths.stateFilePath, "utf8"), stateBefore) + assert.equal(await readMaybe(paths.ledgerFilePath), ledgerBefore) + assert.equal(await fs.readFile(`${paths.stateFilePath}.lock`, "utf8"), lockBefore) + } finally { + await contender?.dispose() + await owner?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + +test("legacy or incomplete leases stay passive with actionable manual recovery", async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-legacy-lock-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "passive-legacy-lock-session" + const paths = sessionPaths(stateFilePath, sessionID) + const persistedState = JSON.stringify({ + version: 1, + goals: [{ + sessionID, + condition: "private legacy objective must not be loaded", + startedAt: Date.now(), + options: {}, + }], + results: [], + }) + const logs = [] + let hooks + try { + await fs.mkdir(dirname(paths.stateFilePath), { recursive: true }) + await fs.writeFile(paths.stateFilePath, persistedState) + await fs.mkdir(`${paths.stateFilePath}.lock`) + await fs.writeFile( + `${paths.stateFilePath}.lock/owner.json`, + JSON.stringify({ token: "legacy-owner", pid: 4242, hostname: "legacy.remote.example" }), + ) + + hooks = await GoalPlugin( + { + client: { + ...hostClient(), + app: { log: async (entry) => logs.push(entry) }, + }, + directory, + }, + { stateFilePath, minDelayMs: 1 }, + ) + await hooks["chat.params"]({ sessionID, agent: "build" }) + + assert.equal(testInternals.currentGoal(sessionID), null) + assert.equal(await fs.readFile(paths.stateFilePath, "utf8"), persistedState) + + const status = JSON.parse( + await hooks.tool.goal_status.execute({}, { sessionID, agent: "build" }), + ) + assert.equal(status.ok, false) + assert.equal(status.error, "session_owned_elsewhere") + assert.match(status.message, /older or incomplete persistence lease/i) + assert.match(status.message, /remove only the affected session shard's adjacent lease artifacts/i) + assert.doesNotMatch(status.message, /private legacy objective|state\.json|legacy\.remote/i) + + const command = { parts: [] } + await hooks["command.execute.before"]( + { command: "goal", sessionID, arguments: "status" }, + command, + ) + assert.match(command.parts[0].text, /older or incomplete persistence lease/i) + assert.equal(logs.length, 1) + assert.match(logs[0].body.message, /older release or is incomplete/i) + assert.doesNotMatch(logs[0].body.message, /state\.json|legacy\.remote/i) + } finally { + await hooks?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + +test("passive goal tools reject honestly, remain per-session, and take over paused", async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-tools-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "passive-tools-session" + const otherSessionID = "passive-tools-other-session" + const paths = sessionPaths(stateFilePath, sessionID) + const logs = [] + const messagesCalls = [] + const promptCalls = [] + let hostMessages = [] + let owner + let contender + try { + owner = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, minDelayMs: 1, noToolCallTurnsBeforePause: 10 }, + ) + await setGoal(owner, sessionID, "durable owner objective") + const stateBefore = await fs.readFile(paths.stateFilePath, "utf8") + const ledgerBefore = await readMaybe(paths.ledgerFilePath) + + contender = await GoalPlugin( + { + client: { + app: { log: async (input) => logs.push(input) }, + session: { + messages: async (input) => { + messagesCalls.push(input) + return { data: hostMessages } + }, + promptAsync: async (input) => { + promptCalls.push(input) + return {} + }, + }, + }, + directory, + }, + { + stateFilePath, + commandName: "objective", + minDelayMs: 1, + noToolCallTurnsBeforePause: 10, + }, + ) + + const passiveCommand = { parts: [] } + await contender["command.execute.before"]( + { command: "objective", sessionID, arguments: "status" }, + passiveCommand, + ) + const oldCommandParentID = await acceptCommandTurn( + contender, + sessionID, + passiveCommand, + ) + await finishControlTurn( + contender, + sessionID, + oldCommandParentID, + "Reported.", + (message) => { hostMessages = [message] }, + ) + + const canonical = [ + ["goal_status", {}, "status"], + ["goal_set", { objective: "must not replace" }, "set"], + ["goal_pause", {}, "pause"], + ["goal_resume", {}, "resume"], + ["goal_block", { blocker: "must not block" }, "block"], + ["goal_complete", { summary: "must not complete" }, "complete"], + ] + for (const [name, args, operation] of canonical) { + const result = JSON.parse(await contender.tool[name].execute(args, { sessionID })) + assert.deepEqual( + { version: result.version, operation: result.operation, ok: result.ok, error: result.error }, + { version: 1, operation, ok: false, error: "session_owned_elsewhere" }, + ) + assert.match(result.message, /Goal controls are unavailable/) + assert.match(result.message, /\/objective status/) + } + + const legacy = [ + ["get_goal", {}], + ["get_goal_history", {}], + ["set_goal", { objective: "must not replace" }], + ["update_goal", { status: "complete", evidence: "must not complete" }], + ["clear_goal", {}], + ] + for (const [name, args] of legacy) { + const result = await contender.tool[name].execute(args, { sessionID }) + assert.match(result, /Goal controls are unavailable/) + assert.match(result, /\/objective status/) + } + assert.equal(await fs.readFile(paths.stateFilePath, "utf8"), stateBefore) + assert.equal(await readMaybe(paths.ledgerFilePath), ledgerBefore) + + const otherResult = JSON.parse( + await contender.tool.goal_set.execute( + { objective: "independent session objective" }, + { sessionID: otherSessionID }, + ), + ) + assert.equal(otherResult.ok, true) + const otherState = JSON.parse( + await fs.readFile(sessionPaths(stateFilePath, otherSessionID).stateFilePath, "utf8"), + ) + assert.deepEqual(otherState.goals.map((goal) => goal.condition), ["independent session objective"]) + + const activeDenial = { parts: [] } + await contender["command.execute.before"]( + { command: "objective", sessionID, arguments: "status" }, + activeDenial, + ) + const activeDenialParentID = await acceptCommandTurn( + contender, + sessionID, + activeDenial, + ) + + await owner.dispose() + owner = null + await new Promise((resolve) => setTimeout(resolve, 300)) + + const guardedRetry = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID }), + ) + assert.equal(guardedRetry.ok, false) + assert.equal(guardedRetry.error, "session_owned_elsewhere") + await assertNoLeaseOwner(`${paths.stateFilePath}.lock`) + await finishControlTurn( + contender, + sessionID, + activeDenialParentID, + "Reported.", + (message) => { hostMessages = [message] }, + ) + + const ordinaryOutput = { + message: { id: "passive-after-release", role: "user", sessionID }, + parts: [{ type: "text", text: "ordinary chat still must not seize the lease" }], + } + await contender["chat.message"]( + { sessionID, messageID: "passive-after-release", agent: "build" }, + ordinaryOutput, + ) + await assertNoLeaseOwner(`${paths.stateFilePath}.lock`) + + const takeover = JSON.parse(await contender.tool.goal_status.execute({}, { sessionID })) + assert.equal(takeover.ok, true) + assert.match(takeover.message, /durable owner objective/) + assert.match(takeover.message, /recovered after restart|paused|stopped/i) + const stateAfterTakeover = await fs.readFile(paths.stateFilePath, "utf8") + + await contender.event({ + event: { + type: "message.updated", + properties: { + info: { + id: "late-passive-denial-assistant", + parentID: oldCommandParentID, + role: "assistant", + sessionID, + tokens: { input: 9000, output: 9000, reasoning: 0 }, + }, + }, + }, + }) + assert.equal(await fs.readFile(paths.stateFilePath, "utf8"), stateAfterTakeover) + const resumed = JSON.parse(await contender.tool.goal_resume.execute({}, { sessionID })) + assert.equal(resumed.ok, true) + const stateAfterResume = await fs.readFile(paths.stateFilePath, "utf8") + assert.equal(testInternals.currentGoal(sessionID).stopped, false) + + await contender.event({ + event: { + type: "message.updated", + properties: { + info: { + id: "late-passive-denial-error", + parentID: oldCommandParentID, + role: "assistant", + sessionID, + error: { name: "ProviderError", message: "late denial reply failed" }, + }, + }, + }, + }) + assert.equal(testInternals.currentGoal(sessionID).stopped, false) + assert.equal(await fs.readFile(paths.stateFilePath, "utf8"), stateAfterResume) + + await idle(contender, sessionID) + assert.equal(promptCalls.length, 1) + assert.ok(messagesCalls.length >= 1) + assert.equal(logs.length, 1) + } finally { + await contender?.dispose() + await owner?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + +test("expired passive command guards require a fresh command boundary for takeover", async () => { + for (const accepted of [false, true]) { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-expired-command-")) + const stateFilePath = join(directory, "state.json") + const sessionID = `passive-expired-${accepted ? "active" : "pending"}` + let owner + let contender + const originalNow = Date.now + try { + owner = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, minDelayMs: 1 }, + ) + await setGoal(owner, sessionID, "durable objective for expired guard takeover") + + contender = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, minDelayMs: 1 }, + ) + const denial = { parts: [] } + await contender["command.execute.before"]( + { command: "goal", sessionID, arguments: "status" }, + denial, + ) + if (accepted) await acceptCommandTurn(contender, sessionID, denial) + + await owner.dispose() + owner = null + const afterTtl = originalNow() + testInternals.commandTurnTtlMs + 1_000 + Date.now = () => afterTtl + + if (accepted) { + await assert.rejects( + contender["tool.execute.before"]({ sessionID, tool: "write" }), + /no tool calls are allowed/i, + ) + const guarded = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID, agent: "build" }), + ) + assert.equal(guarded.ok, false) + assert.equal(guarded.error, "session_owned_elsewhere") + + const takeoverCommand = { parts: [] } + const takeoverStarted = contender["command.execute.before"]( + { command: "goal", sessionID, arguments: "status" }, + takeoverCommand, + ) + const oldTurnTool = contender["tool.execute.before"]({ + sessionID, + tool: "write", + }) + const [takeoverResult, oldTurnToolResult] = await Promise.allSettled([ + takeoverStarted, + oldTurnTool, + ]) + assert.equal(takeoverResult.status, "fulfilled") + assert.equal(oldTurnToolResult.status, "rejected") + assert.match(oldTurnToolResult.reason.message, /no tool calls are allowed/i) + assert.match(takeoverCommand.parts[0].text, /durable objective for expired guard takeover/) + assert.match(takeoverCommand.parts[0].text, /recovered after restart|paused|stopped/i) + } else { + const takeover = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID, agent: "build" }), + ) + assert.equal(takeover.ok, true, "pending guard expired") + assert.match(takeover.message, /durable objective for expired guard takeover/) + assert.match(takeover.message, /recovered after restart|paused|stopped/i) + } + } finally { + Date.now = originalNow + await contender?.dispose() + await owner?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } + } +}) + +test("passive entry and disposal do not wait for a stalled host logger", { timeout: 2_000 }, async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-logger-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "passive-stalled-logger-session" + let owner + let contender + let contenderSettled = false + try { + owner = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, registerTools: false, minDelayMs: 1 }, + ) + await setGoal(owner, sessionID, "owner objective") + + contender = await GoalPlugin( + { + client: { + app: { log: () => new Promise(() => {}) }, + session: hostClient().session, + }, + directory, + }, + { stateFilePath, registerTools: false, minDelayMs: 1 }, + ) + const hookResult = await Promise.race([ + contender["chat.params"]({ sessionID, agent: "build" }).then(() => "settled"), + new Promise((resolve) => setTimeout(() => resolve("timed-out"), 250)), + ]) + contenderSettled = hookResult === "settled" + assert.equal(hookResult, "settled") + + const disposeResult = await Promise.race([ + contender.dispose().then(() => "settled"), + new Promise((resolve) => setTimeout(() => resolve("timed-out"), 250)), + ]) + assert.equal(disposeResult, "settled") + contender = null + } finally { + if (contenderSettled) await contender?.dispose() + await owner?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + +test("persistence recovery and disposal do not wait for a stalled host logger", { timeout: 2_000 }, async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-error-logger-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "stalled-error-logger-session" + const paths = sessionPaths(stateFilePath, sessionID) + let releaseLogger + const stalledLogger = new Promise((resolve) => { releaseLogger = resolve }) + let logCalls = 0 + let hooks + let loadPromise + let disposePromise + try { + await fs.mkdir(dirname(paths.stateFilePath), { recursive: true }) + await fs.writeFile(paths.stateFilePath, "{ malformed") + const client = hostClient() + client.app.log = () => { + logCalls += 1 + return stalledLogger + } + hooks = await GoalPlugin( + { client, directory }, + { stateFilePath, registerTools: false, minDelayMs: 1 }, + ) + + loadPromise = hooks["chat.params"]({ sessionID, agent: "build" }) + assert.equal( + await Promise.race([ + loadPromise.then(() => "settled"), + new Promise((resolve) => setTimeout(() => resolve("timed-out"), 250)), + ]), + "settled", + ) + assert.ok(logCalls >= 1) + await fs.stat(`${paths.stateFilePath}.lock`) + await fs.stat(`${paths.stateFilePath}.lock.claims-v2`) + + disposePromise = hooks.dispose() + assert.equal( + await Promise.race([ + disposePromise.then(() => "settled"), + new Promise((resolve) => setTimeout(() => resolve("timed-out"), 250)), + ]), + "settled", + ) + await assertNoLeaseOwner(`${paths.stateFilePath}.lock`) + hooks = null + } finally { + releaseLogger?.() + await Promise.allSettled([loadPromise, disposePromise].filter(Boolean)) + await hooks?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + +test("a delayed idle lookup cannot repopulate command state after disposal", async () => { + const sessionID = "disposed-idle-command-state" + let resolveMessages + let messagesStarted + const started = new Promise((resolve) => { messagesStarted = resolve }) + const messages = new Promise((resolve) => { resolveMessages = resolve }) + const hooks = await GoalPlugin( + { + client: hostClient({ + messages: async () => { + messagesStarted() + return messages + }, + }), + }, + { persistState: false, minDelayMs: 1 }, + ) + try { + await setGoal(hooks, sessionID, "dispose while command correlation is loading") + const status = await setGoal(hooks, sessionID, "status") + const parentID = await acceptCommandTurn(hooks, sessionID, status) + const delayedIdle = idle(hooks, sessionID) + await started + + await hooks.dispose() + assert.equal(testInternals.runtimeSessionDiagnostics(sessionID).suppressedAssistantCount, 0) + resolveMessages({ + data: [{ + info: { + id: "late-command-assistant", + parentID, + role: "assistant", + sessionID, + tokens: { input: 1, output: 1, reasoning: 0 }, + }, + parts: [{ type: "text", text: "Reported." }], + }], + }) + await delayedIdle + + assert.equal(testInternals.runtimeSessionDiagnostics(sessionID).disposed, true) + assert.equal(testInternals.runtimeSessionDiagnostics(sessionID).suppressedAssistantCount, 0) + } finally { + resolveMessages?.({ data: [] }) + await hooks.dispose() + } +}) + +test("a delayed idle lookup cannot retire a newer command turn", async () => { + const sessionID = "superseded-idle-command-turn" + let resolveFirstMessages + let signalFirstLookup + let hostMessages = [] + let messageCalls = 0 + const firstLookupStarted = new Promise((resolve) => { signalFirstLookup = resolve }) + const firstMessages = new Promise((resolve) => { resolveFirstMessages = resolve }) + const hooks = await GoalPlugin( + { + client: hostClient({ + messages: async () => { + messageCalls += 1 + if (messageCalls === 1) { + signalFirstLookup() + return firstMessages + } + return { data: hostMessages } + }, + }), + }, + { persistState: false, minDelayMs: 1 }, + ) + try { + await setGoal(hooks, sessionID, "keep the newest command guard") + + const firstStatus = await setGoal(hooks, sessionID, "status") + const firstParentID = await acceptCommandTurn(hooks, sessionID, firstStatus) + const delayedIdle = idle(hooks, sessionID) + await firstLookupStarted + + const secondStatus = await setGoal(hooks, sessionID, "status") + const secondParentID = await acceptCommandTurn(hooks, sessionID, secondStatus) + resolveFirstMessages({ + data: [{ + info: { + id: "first-command-assistant", + parentID: firstParentID, + role: "assistant", + sessionID, + tokens: { input: 1, output: 1, reasoning: 0 }, + }, + parts: [{ type: "text", text: "Reported first status." }], + }], + }) + await delayedIdle + + await assert.rejects( + hooks["tool.execute.before"]({ sessionID, tool: "write" }), + /no tool calls are allowed/i, + ) + + hostMessages = [{ + info: { + id: "second-command-assistant", + parentID: secondParentID, + role: "assistant", + sessionID, + tokens: { input: 1, output: 1, reasoning: 0 }, + }, + parts: [{ type: "text", text: "Reported second status." }], + }] + await idle(hooks, sessionID) + await assert.doesNotReject(() => + hooks["tool.execute.before"]({ sessionID, tool: "read" }), + ) + } finally { + resolveFirstMessages?.({ data: [] }) + await hooks.dispose() + } +}) + +test("passive Plan context survives explicit takeover and prevents auto-continue", async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-plan-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "passive-plan-session" + const promptCalls = [] + let owner + let contender + try { + owner = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, minDelayMs: 1, noToolCallTurnsBeforePause: 10 }, + ) + await setGoal(owner, sessionID, "never continue while Plan is active") + + contender = await GoalPlugin( + { + client: hostClient({ + promptAsync: async (input) => { + promptCalls.push(input) + return {} + }, + }), + directory, + }, + { stateFilePath, minDelayMs: 1, noToolCallTurnsBeforePause: 10 }, + ) + await contender["chat.params"]({ + sessionID, + agent: "Plan", + model: { providerID: "test", modelID: "plan" }, + }) + + await owner.dispose() + owner = null + await new Promise((resolve) => setTimeout(resolve, 300)) + + const resumed = JSON.parse( + await contender.tool.goal_resume.execute({}, { sessionID, agent: "Plan" }), + ) + assert.equal(resumed.ok, true) + await idle(contender, sessionID) + + assert.equal(promptCalls.length, 0) + const status = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID, agent: "Plan" }), + ) + assert.equal(status.ok, true) + assert.match(status.message, /Plan|plan agent active/i) + } finally { + await contender?.dispose() + await owner?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + +test("partial tool context preserves the model and variant observed from the host", async () => { + const sessionID = "partial-tool-context-session" + const hooks = await GoalPlugin( + { client: hostClient(), directory: "/workspace/partial-tool-context" }, + { persistState: false, minDelayMs: 1 }, + ) + try { + await hooks["chat.params"]({ + sessionID, + agent: "build", + model: { providerID: "openrouter", modelID: "model-a" }, + message: { model: { variant: "high" } }, + }) + const result = JSON.parse( + await hooks.tool.goal_set.execute( + { objective: "preserve the initiating execution context" }, + { sessionID, agent: "goal-worker" }, + ), + ) + assert.equal(result.ok, true) + assert.deepEqual(testInternals.currentGoal(sessionID).executionContext, { + agent: "goal-worker", + model: { providerID: "openrouter", modelID: "model-a" }, + variant: "high", + }) + } finally { + await hooks.dispose() + } +}) + +test("authoritative model changes clear an omitted variant before partial tool context merges", async () => { + const sessionID = "authoritative-model-change-session" + const hooks = await GoalPlugin( + { client: hostClient(), directory: "/workspace/authoritative-model-change" }, + { persistState: false, minDelayMs: 1 }, + ) + try { + await hooks["chat.params"]({ + sessionID, + agent: "build", + model: { providerID: "openrouter", modelID: "model-a" }, + message: { model: { variant: "high" } }, + }) + await hooks["chat.message"]( + { + sessionID, + messageID: "model-b-user-message", + agent: "build", + model: { providerID: "openrouter", modelID: "model-b" }, + }, + { + message: { id: "model-b-user-message", role: "user", sessionID }, + parts: [{ type: "text", text: "switch models" }], + }, + ) + + const result = JSON.parse( + await hooks.tool.goal_set.execute( + { objective: "use the current model without a stale variant" }, + { sessionID, agent: "goal-worker" }, + ), + ) + assert.equal(result.ok, true) + assert.deepEqual(testInternals.currentGoal(sessionID).executionContext, { + agent: "goal-worker", + model: { providerID: "openrouter", modelID: "model-b" }, + }) + } finally { + await hooks.dispose() + } +}) + +test("passive tool recovery stays actionable when slash-command registration is disabled", async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-tools-only-")) + const stateFilePath = join(directory, "state.json") + const sessionID = "passive-tools-only-session" + let owner + let contender + try { + owner = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, minDelayMs: 1 }, + ) + await setGoal(owner, sessionID, "owner objective") + + contender = await GoalPlugin( + { client: hostClient(), directory }, + { stateFilePath, registerCommand: false, minDelayMs: 1 }, + ) + assert.equal(contender["command.execute.before"], undefined) + const result = JSON.parse( + await contender.tool.goal_status.execute({}, { sessionID, agent: "build" }), + ) + assert.equal(result.ok, false) + assert.equal(result.error, "session_owned_elsewhere") + assert.match(result.message, /goal_status/) + assert.doesNotMatch(result.message, /\/goal status/) + } finally { + await contender?.dispose() + await owner?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) + test("workspace persistence and lifecycle ledgers remain isolated", async () => { const root = await fs.mkdtemp(join(tmpdir(), "goal-plugin-workspaces-")) const firstDirectory = join(root, "one") diff --git a/test/passive-retention.test.js b/test/passive-retention.test.js new file mode 100644 index 0000000..121bd58 --- /dev/null +++ b/test/passive-retention.test.js @@ -0,0 +1,76 @@ +import assert from "node:assert/strict" +import { promises as fs } from "node:fs" +import { tmpdir } from "node:os" +import { join } from "node:path" +import test from "node:test" +import { GoalPlugin, testInternals } from "../src/goal-plugin.js" + +function sessionPaths(stateFilePath, sessionID) { + return testInternals.sessionPathsFor({ sessionDirectory: `${stateFilePath}.sessions` }, sessionID) +} + +test("passive session tombstones are not evicted before an explicit takeover", async () => { + const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-passive-retention-")) + const stateFilePath = join(directory, "state.json") + const sessionIDs = Array.from({ length: 1001 }, (_, index) => `contended-session-${index}`) + const firstSessionID = sessionIDs[0] + const legacyOwner = JSON.stringify({ + token: "legacy-owner", + pid: 4242, + hostname: "legacy.remote.example", + }) + const originalNow = Date.now + const observedAt = originalNow() + let hooks + try { + Date.now = () => observedAt + hooks = await GoalPlugin( + { + client: { + app: { log: async () => {} }, + session: { + messages: async () => ({ data: [] }), + promptAsync: async () => ({}), + }, + }, + directory, + }, + { stateFilePath, registerTools: false, minDelayMs: 1 }, + ) + + for (const sessionID of sessionIDs) { + const paths = sessionPaths(stateFilePath, sessionID) + await fs.mkdir(`${paths.stateFilePath}.lock`, { recursive: true }) + await fs.writeFile(`${paths.stateFilePath}.lock/owner.json`, legacyOwner) + await hooks["chat.params"]({ sessionID, agent: "build" }) + } + + assert.equal(testInternals.runtimeSessionDiagnostics(firstSessionID).passive, true) + const firstLockPath = `${sessionPaths(stateFilePath, firstSessionID).stateFilePath}.lock` + await fs.rm(firstLockPath, { recursive: true }) + + const ordinaryOutput = { + message: { id: "ordinary-after-many-passive", role: "user", sessionID: firstSessionID }, + parts: [{ type: "text", text: "ordinary chat must not acquire the lease" }], + } + Date.now = () => observedAt + 1_000 + await hooks["chat.message"]( + { sessionID: firstSessionID, messageID: "ordinary-after-many-passive", agent: "build" }, + ordinaryOutput, + ) + assert.equal(ordinaryOutput.parts[0].text, "ordinary chat must not acquire the lease") + await assert.rejects(fs.stat(firstLockPath), { code: "ENOENT" }) + + const explicitStatus = { parts: [] } + await hooks["command.execute.before"]( + { command: "goal", sessionID: firstSessionID, arguments: "status" }, + explicitStatus, + ) + assert.match(explicitStatus.parts[0].text, /No active goal/) + assert.equal(testInternals.runtimeSessionDiagnostics(firstSessionID).persistenceOwned, true) + } finally { + Date.now = originalNow + await hooks?.dispose() + await fs.rm(directory, { recursive: true, force: true }) + } +}) diff --git a/test/persistence-lease.test.js b/test/persistence-lease.test.js index 19124be..6ac068d 100644 --- a/test/persistence-lease.test.js +++ b/test/persistence-lease.test.js @@ -1,52 +1,787 @@ import assert from "node:assert/strict" import test from "node:test" -import { mkdtemp, readFile, rm, writeFile, mkdir, utimes } from "node:fs/promises" -import { tmpdir } from "node:os" +import { randomUUID } from "node:crypto" +import { + mkdir, + mkdtemp, + lstat, + link, + readFile, + readdir, + rename as fsRename, + rm, + stat, + symlink, + utimes, + writeFile, +} from "node:fs/promises" +import { hostname, tmpdir } from "node:os" import { join } from "node:path" import { spawn } from "node:child_process" import { once } from "node:events" -import { acquirePersistenceLease } from "../src/persistence-lease.js" +import { + acquirePersistenceLease, + isPersistenceLeaseContendedError, + PERSISTENCE_LEASE_CONTENDED, + PersistenceLeaseContendedError, + persistenceLeaseInternals, +} from "../src/persistence-lease.js" + +async function createV2LeaseDirectory(lockPath) { + const guard = await persistenceLeaseInternals.publishLegacyGuard(lockPath) + await mkdir(persistenceLeaseInternals.claimDirectoryPathFor(lockPath)) + return guard.owner +} + +async function claimNames(lockPath) { + return (await readdir(persistenceLeaseInternals.claimDirectoryPathFor(lockPath))) + .filter((name) => name.startsWith("claim-")) + .sort() +} + +function deferred() { + let resolve + const promise = new Promise((settle) => { resolve = settle }) + return { promise, resolve } +} + +async function readVersion1Owner(lockPath) { + try { + return JSON.parse(await readFile(join(lockPath, "owner.json"), "utf8")) + } catch { + return null + } +} + +async function acquireVersion1Lease( + stateFilePath, + { + malformedGraceMs = 30_000, + now = () => Date.now(), + beforeOwnerWrite, + beforeStaleRename, + afterStaleRemoval, + } = {}, +) { + const lockPath = `${stateFilePath}.lock` + const owner = { + token: randomUUID(), + pid: process.pid, + hostname: hostname(), + createdAt: Date.now(), + } + + for (let attempt = 0; attempt < 3; attempt += 1) { + try { + await mkdir(lockPath, { mode: 0o700 }) + await beforeOwnerWrite?.({ attempt, lockPath, owner: { ...owner } }) + await writeFile(join(lockPath, "owner.json"), JSON.stringify(owner), { mode: 0o600 }) + return { + owner, + async release() { + const current = await readVersion1Owner(lockPath) + if (current?.token !== owner.token) return false + await rm(lockPath, { recursive: true, force: true }) + return true + }, + } + } catch (error) { + if (error?.code !== "EEXIST") { + await rm(lockPath, { recursive: true, force: true }).catch(() => {}) + throw error + } + const existing = await readVersion1Owner(lockPath) + const sameHost = existing?.hostname === owner.hostname + let reclaimableMalformed = false + if (!existing) { + try { + const info = await lstat(lockPath) + reclaimableMalformed = now() - info.mtimeMs >= malformedGraceMs + } catch (statError) { + if (statError?.code === "ENOENT") continue + } + } + if ( + (sameHost && persistenceLeaseInternals.processIsAlive(existing?.pid) === false) || + reclaimableMalformed + ) { + await beforeStaleRename?.({ attempt, lockPath, owner: { ...owner } }) + const stalePath = `${lockPath}.stale.${randomUUID()}` + try { + await fsRename(lockPath, stalePath) + await rm(stalePath, { recursive: true, force: true }) + await afterStaleRemoval?.({ attempt, lockPath, owner: { ...owner } }) + continue + } catch (reclaimError) { + if (reclaimError?.code === "ENOENT") continue + } + } + throw new Error("version-1 lease contended") + } + } + throw new Error("version-1 lease exhausted") +} test("persistence lease rejects a concurrent owner and releases by token", async () => { const dir = await mkdtemp(join(tmpdir(), "goal-lease-")) const state = join(dir, "state.json") const first = await acquirePersistenceLease(state) - await assert.rejects(acquirePersistenceLease(state), /already owned/) + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(error instanceof PersistenceLeaseContendedError, true) + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.code, PERSISTENCE_LEASE_CONTENDED) + assert.equal(error.owner.pid, process.pid) + assert.equal(typeof error.owner.hostname, "string") + assert.equal(Object.isFrozen(error.owner), true) + assert.equal(error.reason, "owned_elsewhere") + assert.equal("token" in error.owner, false) + assert.doesNotMatch(error.message, /state\.json|[0-9a-f]{8}-[0-9a-f-]{27}/i) + return true + }) + const sentinelBeforeRelease = await readFile(`${state}.lock`, "utf8") + assert.equal( + persistenceLeaseInternals.validLegacySentinel(JSON.parse(sentinelBeforeRelease)), + true, + ) + assert.equal( + isPersistenceLeaseContendedError({ code: PERSISTENCE_LEASE_CONTENDED }), + false, + ) assert.equal(await first.release(), true) + assert.equal(await readFile(`${state}.lock`, "utf8"), sentinelBeforeRelease) + assert.deepEqual(await claimNames(`${state}.lock`), []) const second = await acquirePersistenceLease(state) assert.equal(await second.release(), true) + assert.equal(await readFile(`${state}.lock`, "utf8"), sentinelBeforeRelease) await rm(dir, { recursive: true, force: true }) }) -test("persistence lease reclaims a dead same-host owner", async () => { +test("version 1 wins safely when its legacy directory is created first", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-v1-first-")) + const state = join(dir, "state.json") + const atOwnerWrite = deferred() + const allowOwnerWrite = deferred() + let version1Lease + try { + const version1 = acquireVersion1Lease(state, { + beforeOwnerWrite: async () => { + atOwnerWrite.resolve() + await allowOwnerWrite.promise + }, + }) + await atOwnerWrite.promise + + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + + allowOwnerWrite.resolve() + version1Lease = await version1 + assert.equal( + JSON.parse(await readFile(`${state}.lock/owner.json`, "utf8")).token, + version1Lease.owner.token, + ) + await assert.rejects(stat(persistenceLeaseInternals.claimDirectoryPathFor(`${state}.lock`)), { + code: "ENOENT", + }) + } finally { + allowOwnerWrite.resolve() + await version1Lease?.release() + await rm(dir, { recursive: true, force: true }) + } +}) + +test("an atomically published version-2 guard makes exact version 1 fail closed", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-v2-first-")) + const state = join(dir, "state.json") + const guardLinked = deferred() + const allowV2Claims = deferred() + let version2Lease + try { + const version2 = persistenceLeaseInternals.acquirePersistenceLeaseWithHooks( + state, + {}, + { + afterGuardLink: async () => { + guardLinked.resolve() + await allowV2Claims.promise + }, + }, + ) + await guardLinked.promise + + await assert.rejects(acquireVersion1Lease(state), /version-1 lease contended/) + const guardInfo = await stat(`${state}.lock`) + assert.equal(guardInfo.isFile(), true) + assert.equal(persistenceLeaseInternals.legacyGuardMtimeIsSafe(guardInfo), true) + + allowV2Claims.resolve() + version2Lease = await version2 + assert.deepEqual(await claimNames(`${state}.lock`), [ + persistenceLeaseInternals.claimNameFor(version2Lease.owner.token), + ]) + } finally { + allowV2Claims.resolve() + await version2Lease?.release() + await rm(dir, { recursive: true, force: true }) + } +}) + +test("version 2 stays passive while version 1 is paused after a stale decision", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-v1-stale-decision-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const staleDecision = deferred() + const allowStaleRename = deferred() + let version1Lease + try { + await mkdir(lock) + await writeFile(join(lock, "owner.json"), JSON.stringify({ + token: "old", + pid: 2_147_483_647, + hostname: hostname(), + })) + const version1 = acquireVersion1Lease(state, { + beforeStaleRename: async () => { + staleDecision.resolve() + await allowStaleRename.promise + }, + }) + await staleDecision.promise + + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + + allowStaleRename.resolve() + version1Lease = await version1 + assert.equal( + JSON.parse(await readFile(join(lock, "owner.json"), "utf8")).token, + version1Lease.owner.token, + ) + } finally { + allowStaleRename.resolve() + await version1Lease?.release() + await rm(dir, { recursive: true, force: true }) + } +}) + +test("version 2 wins safely while version 1 pauses after removing a stale directory", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-v1-stale-removed-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const staleRemoved = deferred() + const allowVersion1Retry = deferred() + let version2Lease + try { + await mkdir(lock) + await writeFile(join(lock, "owner.json"), JSON.stringify({ + token: "old", + pid: 2_147_483_647, + hostname: hostname(), + })) + const version1 = acquireVersion1Lease(state, { + afterStaleRemoval: async () => { + staleRemoved.resolve() + await allowVersion1Retry.promise + }, + }) + await staleRemoved.promise + + version2Lease = await acquirePersistenceLease(state) + allowVersion1Retry.resolve() + await assert.rejects(version1, /version-1 lease contended/) + assert.deepEqual(await claimNames(lock), [ + persistenceLeaseInternals.claimNameFor(version2Lease.owner.token), + ]) + } finally { + allowVersion1Retry.resolve() + await version2Lease?.release() + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease fails closed on a dead legacy owner until manual recovery", async () => { const dir = await mkdtemp(join(tmpdir(), "goal-lease-stale-")) const state = join(dir, "state.json") const lock = `${state}.lock` await mkdir(lock) - await writeFile(`${lock}/owner.json`, JSON.stringify({ token: "old", pid: 2_147_483_647, hostname: (await import("node:os")).hostname() })) + const legacyOwner = JSON.stringify({ token: "old", pid: 2_147_483_647, hostname: hostname() }) + await writeFile(`${lock}/owner.json`, legacyOwner) + await utimes(lock, new Date(0), new Date(0)) + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 0 }), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + assert.match(error.message, /legacy or incomplete lease.*remove its lease artifacts/i) + return true + }) + assert.equal(await readFile(`${lock}/owner.json`, "utf8"), legacyOwner) + assert.deepEqual(await readdir(lock), ["owner.json"]) + + await rm(lock, { recursive: true }) const lease = await acquirePersistenceLease(state) - const owner = JSON.parse(await readFile(`${lock}/owner.json`, "utf8")) - assert.notEqual(owner.token, "old") - await lease.release() + assert.equal(await lease.release(), true) await rm(dir, { recursive: true, force: true }) }) -test("persistence lease protects fresh malformed locks and reclaims old ones", async () => { +test("persistence lease fails closed on fresh and old malformed legacy locks", async () => { const dir = await mkdtemp(join(tmpdir(), "goal-lease-malformed-")) const state = join(dir, "state.json") const lock = `${state}.lock` await mkdir(lock) await writeFile(`${lock}/owner.json`, "{truncated") - await assert.rejects( - acquirePersistenceLease(state, { malformedGraceMs: 30_000 }), - /unknown owner/, - ) + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 30_000 }), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: null, hostname: null }) + assert.equal(error.reason, "legacy_lock") + return true + }) await utimes(lock, new Date(0), new Date(0)) - const lease = await acquirePersistenceLease(state, { malformedGraceMs: 1 }) - assert.equal(await lease.release(), true) + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 1 }), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + assert.equal(await readFile(`${lock}/owner.json`, "utf8"), "{truncated") await rm(dir, { recursive: true, force: true }) }) +test("persistence lease fails closed on a regular guard without its exact sentinel", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-incomplete-v2-")) + try { + const invalidGuards = [ + "{truncated", + JSON.stringify({ ...persistenceLeaseInternals.legacySentinel(), sentinel: false }), + ] + for (const [index, contents] of invalidGuards.entries()) { + const state = join(dir, `state-${index}.json`) + const lock = `${state}.lock` + await writeFile(lock, contents) + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + assert.equal(await readFile(lock, "utf8"), contents) + await assert.rejects(stat(persistenceLeaseInternals.claimDirectoryPathFor(lock)), { + code: "ENOENT", + }) + } + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease fails closed when an exact sentinel lacks its future mtime", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-undated-sentinel-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const sentinel = persistenceLeaseInternals.legacySentinel() + try { + await writeFile(lock, JSON.stringify(sentinel)) + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + assert.deepEqual(JSON.parse(await readFile(lock, "utf8")), sentinel) + await assert.rejects(stat(persistenceLeaseInternals.claimDirectoryPathFor(lock)), { + code: "ENOENT", + }) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease rejects a future-dated sentinel with extra fields", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-sentinel-extra-field-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const sentinel = { ...persistenceLeaseInternals.legacySentinel(), extra: "tampered" } + try { + await writeFile(lock, JSON.stringify(sentinel)) + const guardDate = new Date("2100-01-01T00:00:00.000Z") + await utimes(lock, guardDate, guardDate) + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + assert.deepEqual(JSON.parse(await readFile(lock, "utf8")), sentinel) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease fails safely when the filesystem cannot publish a hard link", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-no-hardlink-")) + const state = join(dir, "state.json") + try { + await assert.rejects( + persistenceLeaseInternals.acquirePersistenceLeaseWithHooks( + state, + {}, + { + linkGuard: async () => { + const error = new Error("hard links disabled") + error.code = "ENOTSUP" + throw error + }, + }, + ), + (error) => { + assert.equal(error.code, "ERR_GOAL_PERSISTENCE_LEASE_HARDLINK") + assert.match(error.message, /hard-link support/) + return true + }, + ) + await assert.rejects(stat(`${state}.lock`), { code: "ENOENT" }) + await assert.rejects( + stat(persistenceLeaseInternals.claimDirectoryPathFor(`${state}.lock`)), + { code: "ENOENT" }, + ) + assert.deepEqual((await readdir(dir)).filter((name) => name.includes(".guard.")), []) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("guard publication retries when a version-1 directory disappears after EEXIST", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-guard-eexist-release-")) + const state = join(dir, "state.json") + let linkCalls = 0 + let lease + try { + lease = await persistenceLeaseInternals.acquirePersistenceLeaseWithHooks( + state, + {}, + { + linkGuard: async (source, target) => { + linkCalls += 1 + if (linkCalls === 1) { + await mkdir(target) + await rm(target, { recursive: true }) + const error = new Error("legacy directory existed during link") + error.code = "EEXIST" + throw error + } + await link(source, target) + }, + }, + ) + assert.equal(linkCalls, 2) + assert.equal((await stat(`${state}.lock`)).isFile(), true) + assert.deepEqual(await claimNames(`${state}.lock`), [ + persistenceLeaseInternals.claimNameFor(lease.owner.token), + ]) + } finally { + await lease?.release() + await rm(dir, { recursive: true, force: true }) + } +}) + +test("a missing final guard cannot return ownership and cleans its published claim", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-final-guard-missing-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + let linkCalls = 0 + try { + await assert.rejects( + persistenceLeaseInternals.acquirePersistenceLeaseWithHooks( + state, + {}, + { + linkGuard: async (source, target) => { + linkCalls += 1 + if (linkCalls === 1) { + await link(source, target) + return + } + const error = new Error("legacy path repeatedly raced and disappeared") + error.code = "EEXIST" + throw error + }, + afterOwnerWrite: async () => { + await rm(lock, { force: true }) + }, + }, + ), + (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "owned_elsewhere") + return true + }, + ) + assert.equal(linkCalls, 6) + await assert.rejects(stat(lock), { code: "ENOENT" }) + assert.deepEqual(await claimNames(lock), []) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease treats schema-invalid JSON owners as malformed", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-invalid-owner-schema-")) + const invalidOwners = [ + {}, + [], + null, + { pid: process.pid, hostname: "valid.example" }, + { token: "token", pid: 0, hostname: "valid.example" }, + { token: "token", pid: process.pid, hostname: "" }, + ] + try { + for (const [index, owner] of invalidOwners.entries()) { + const state = join(dir, `state-${index}.json`) + const lock = `${state}.lock` + await mkdir(lock) + await writeFile(`${lock}/owner.json`, JSON.stringify(owner)) + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: null, hostname: null }) + assert.equal(error.reason, "legacy_lock") + return true + }) + await utimes(lock, new Date(0), new Date(0)) + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 1 }), (error) => { + assert.equal(error.reason, "legacy_lock") + return true + }) + } + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease keeps display-unsafe hostnames valid but hides them from diagnostics", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-display-unsafe-host-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const storedHostname = "valid storage hostname\nnot safe for logs" + await mkdir(lock) + await writeFile( + `${lock}/owner.json`, + JSON.stringify({ token: "live-owner", pid: process.pid, hostname: storedHostname }), + ) + await utimes(lock, new Date(0), new Date(0)) + try { + await assert.rejects( + acquirePersistenceLease(state, { malformedGraceMs: 1 }), + (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: process.pid, hostname: null }) + assert.equal(error.reason, "legacy_lock") + assert.doesNotMatch(error.message, /valid storage hostname|not safe for logs/) + return true + }, + ) + assert.equal( + JSON.parse(await readFile(`${lock}/owner.json`, "utf8")).hostname, + storedHostname, + ) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("bounded owner reads consume legal short reads before parsing", async () => { + const raw = Buffer.from( + JSON.stringify({ token: "short-read", pid: process.pid, hostname: "valid.example" }), + ) + const info = { + size: raw.length, + dev: 41, + ino: 42, + isFile: () => true, + } + let reads = 0 + const handle = { + async read(buffer, offset, length, position) { + const bytesRead = Math.min(3, length, Math.max(0, raw.length - position)) + if (bytesRead > 0) raw.copy(buffer, offset, position, position + bytesRead) + reads += 1 + return { bytesRead, buffer } + }, + async stat() { + return info + }, + } + + assert.equal( + await persistenceLeaseInternals.readBoundedOwnerFile(handle, info), + raw.toString("utf8"), + ) + assert.ok(reads > 1) +}) + +test("a delayed claim publication cannot overwrite a replacement lease", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-publication-race-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + let releaseFirstWrite + let firstWriteStarted + const firstAtWrite = new Promise((resolve) => { firstWriteStarted = resolve }) + const writeBarrier = new Promise((resolve) => { releaseFirstWrite = resolve }) + let secondLease + let results + try { + const firstLease = persistenceLeaseInternals.acquirePersistenceLeaseWithHooks( + state, + { malformedGraceMs: 1 }, + { + beforeOwnerWrite: async () => { + firstWriteStarted() + await writeBarrier + }, + }, + ) + await firstAtWrite + const sentinelBefore = await readFile(lock, "utf8") + assert.equal( + persistenceLeaseInternals.validLegacySentinel(JSON.parse(sentinelBefore)), + true, + ) + assert.equal((await stat(lock)).isFile(), true) + secondLease = await acquirePersistenceLease(state, { malformedGraceMs: 1 }) + releaseFirstWrite() + results = await Promise.allSettled([firstLease, Promise.resolve(secondLease)]) + + assert.equal(results.filter((result) => result.status === "fulfilled").length, 1) + assert.equal(results[0].status, "rejected") + assert.equal(isPersistenceLeaseContendedError(results[0].reason), true) + assert.equal(await readFile(lock, "utf8"), sentinelBefore) + assert.deepEqual(await claimNames(lock), [ + persistenceLeaseInternals.claimNameFor(secondLease.owner.token), + ]) + } finally { + releaseFirstWrite?.() + if (results?.[0]?.status === "fulfilled") await results[0].value.release() + await secondLease?.release() + await rm(dir, { recursive: true, force: true }) + } +}) + +test("simultaneous stale observers cannot both acquire the replacement lease", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-stale-observers-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const sentinel = await createV2LeaseDirectory(lock) + const staleToken = "00000000-0000-4000-8000-000000000000" + const staleClaimPath = join( + persistenceLeaseInternals.claimDirectoryPathFor(lock), + persistenceLeaseInternals.claimNameFor(staleToken), + ) + await writeFile(staleClaimPath, JSON.stringify({ + protocol: 2, + token: staleToken, + pid: 2_147_483_647, + hostname: hostname(), + createdAt: 0, + })) + + let beforeCount = 0 + let afterCount = 0 + let releaseBefore + let releaseAfter + let bothBefore + let bothAfter + const beforeBarrier = new Promise((resolve) => { releaseBefore = resolve }) + const afterBarrier = new Promise((resolve) => { releaseAfter = resolve }) + const bothAtBefore = new Promise((resolve) => { bothBefore = resolve }) + const bothAtAfter = new Promise((resolve) => { bothAfter = resolve }) + const hooks = { + beforeOwnerWrite: async ({ attempt }) => { + if (attempt !== 0) return + beforeCount += 1 + if (beforeCount === 2) bothBefore() + await beforeBarrier + }, + afterOwnerWrite: async ({ attempt }) => { + if (attempt !== 0) return + afterCount += 1 + if (afterCount === 2) bothAfter() + await afterBarrier + }, + } + + let leases = [] + try { + const first = persistenceLeaseInternals.acquirePersistenceLeaseWithHooks(state, {}, hooks) + const second = persistenceLeaseInternals.acquirePersistenceLeaseWithHooks(state, {}, hooks) + await bothAtBefore + releaseBefore() + await bothAtAfter + releaseAfter() + + const results = await Promise.allSettled([first, second]) + leases = results + .filter((result) => result.status === "fulfilled") + .map((result) => result.value) + assert.equal(leases.length, 1, "exactly one overlapping stale reclaimer must win") + const rejected = results.find((result) => result.status === "rejected") + assert.equal(isPersistenceLeaseContendedError(rejected?.reason), true) + + const mirror = JSON.parse(await readFile(lock, "utf8")) + assert.deepEqual(mirror, sentinel) + const claims = await claimNames(lock) + assert.deepEqual(claims, [persistenceLeaseInternals.claimNameFor(leases[0].owner.token)]) + } finally { + releaseBefore?.() + releaseAfter?.() + await Promise.allSettled(leases.map((lease) => lease.release())) + await rm(dir, { recursive: true, force: true }) + } +}) + +test("concurrent release calls cannot delete a replacement lease", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-release-race-")) + const state = join(dir, "state.json") + let allowClaimRemoval + let claimRemovalStarted + const removalBarrier = new Promise((resolve) => { allowClaimRemoval = resolve }) + const atClaimRemoval = new Promise((resolve) => { claimRemovalStarted = resolve }) + let lease + let replacement + try { + lease = await persistenceLeaseInternals.acquirePersistenceLeaseWithHooks( + state, + {}, + { + beforeClaimRemove: async () => { + claimRemovalStarted() + await removalBarrier + }, + }, + ) + const sentinelBefore = await readFile(`${state}.lock`, "utf8") + const firstRelease = lease.release() + await atClaimRemoval + assert.equal(await lease.release(), false, "a second release must not enter teardown") + allowClaimRemoval() + assert.equal(await firstRelease, true) + + replacement = await acquirePersistenceLease(state) + assert.equal(await readFile(`${state}.lock`, "utf8"), sentinelBefore) + assert.deepEqual(await claimNames(`${state}.lock`), [ + persistenceLeaseInternals.claimNameFor(replacement.owner.token), + ]) + assert.equal(await lease.release(), false) + assert.equal( + await readFile(`${state}.lock`, "utf8"), + sentinelBefore, + ) + } finally { + allowClaimRemoval?.() + await replacement?.release().catch(() => false) + await lease?.release().catch(() => false) + await rm(dir, { recursive: true, force: true }) + } +}) + test("persistence lease prevents a second Node process from owning the same state", async () => { const dir = await mkdtemp(join(tmpdir(), "goal-lease-process-")) const state = join(dir, "state.json") @@ -63,7 +798,11 @@ test("persistence lease prevents a second Node process from owning the same stat const [chunk] = await once(child.stdout, "data") output += chunk.toString() } - await assert.rejects(acquirePersistenceLease(state), /already owned/) + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(error.code, PERSISTENCE_LEASE_CONTENDED) + assert.equal(isPersistenceLeaseContendedError(error), true) + return true + }) child.stdin.write("stop\n") const [code] = await once(child, "exit") assert.equal(code, 0) @@ -74,3 +813,324 @@ test("persistence lease prevents a second Node process from owning the same stat await rm(dir, { recursive: true, force: true }) } }) + +test("an aged token-matching future-protocol claim blocks and remains untouched", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-future-claim-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const token = "00000000-0000-4000-8000-000000000003" + const claimPath = join( + persistenceLeaseInternals.claimDirectoryPathFor(lock), + persistenceLeaseInternals.claimNameFor(token), + ) + const futureClaim = JSON.stringify({ + protocol: 3, + token, + pid: process.pid, + hostname: hostname(), + createdAt: 0, + }) + try { + await createV2LeaseDirectory(lock) + await writeFile(claimPath, futureClaim) + await utimes(claimPath, new Date(0), new Date(0)) + const claimInfoBefore = await stat(claimPath) + + await assert.rejects( + acquirePersistenceLease(state, { malformedGraceMs: 1, now: () => Date.now() }), + (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "owned_elsewhere") + assert.deepEqual(error.owner, { pid: process.pid, hostname: hostname() }) + return true + }, + ) + assert.equal(await readFile(claimPath, "utf8"), futureClaim) + assert.equal((await stat(claimPath)).mtimeMs, claimInfoBefore.mtimeMs) + assert.deepEqual(await claimNames(lock), [persistenceLeaseInternals.claimNameFor(token)]) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("a future-protocol claim remains authoritative even when its recorded process is dead", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-future-dead-claim-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const token = "00000000-0000-4000-8000-000000000004" + const claimPath = join( + persistenceLeaseInternals.claimDirectoryPathFor(lock), + persistenceLeaseInternals.claimNameFor(token), + ) + const futureClaim = JSON.stringify({ + protocol: 3, + token, + pid: 2_147_483_647, + hostname: hostname(), + createdAt: 0, + }) + try { + await createV2LeaseDirectory(lock) + await writeFile(claimPath, futureClaim) + await utimes(claimPath, new Date(0), new Date(0)) + + await assert.rejects( + acquirePersistenceLease(state, { malformedGraceMs: 1, now: () => Date.now() }), + (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "owned_elsewhere") + return true + }, + ) + assert.equal(await readFile(claimPath, "utf8"), futureClaim) + assert.deepEqual(await claimNames(lock), [persistenceLeaseInternals.claimNameFor(token)]) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("an unknown claim filename blocks conservatively and remains untouched", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-future-claim-name-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const claimDirectory = persistenceLeaseInternals.claimDirectoryPathFor(lock) + const claimPath = join(claimDirectory, "claim-next-owner.json") + const futureClaim = JSON.stringify({ + protocol: 3, + token: "next-owner", + pid: process.pid, + hostname: hostname(), + createdAt: 0, + }) + try { + await createV2LeaseDirectory(lock) + await writeFile(claimPath, futureClaim) + await utimes(claimPath, new Date(0), new Date(0)) + + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 1 }), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "owned_elsewhere") + assert.deepEqual(error.owner, { pid: null, hostname: null }) + return true + }) + assert.equal(await readFile(claimPath, "utf8"), futureClaim) + assert.deepEqual(await readdir(claimDirectory), ["claim-next-owner.json"]) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease sanitizes hostile owner metadata", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-hostile-owner-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const secretToken = "secret-token-that-must-not-leak" + await mkdir(lock) + await writeFile( + `${lock}/owner.json`, + JSON.stringify({ + token: secretToken, + pid: "123\n456", + hostname: `evil-host\n${"x".repeat(300)}`, + createdAt: 123, + stateFilePath: "/private/secret/state.json", + }), + ) + try { + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: null, hostname: null }) + assert.equal(error.reason, "legacy_lock") + assert.doesNotMatch(error.message, /secret-token|private\/secret|evil-host|\n/) + assert.deepEqual(Object.keys(error.owner).sort(), ["hostname", "pid"]) + return true + }) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease bounds oversized owner records before parsing", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-oversized-owner-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const secret = "oversized-secret-".repeat(400) + await mkdir(lock) + await writeFile( + `${lock}/owner.json`, + JSON.stringify({ token: secret, pid: process.pid, hostname: "valid.example" }), + ) + try { + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: null, hostname: null }) + assert.equal(error.reason, "legacy_lock") + assert.doesNotMatch(error.message, /oversized-secret/) + return true + }) + await utimes(lock, new Date(0), new Date(0)) + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 1 }), (error) => { + assert.equal(error.reason, "legacy_lock") + return true + }) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease never follows a symlinked owner record", async (t) => { + if (process.platform === "win32") { + t.skip("symlink creation is not reliably available on Windows CI") + return + } + const dir = await mkdtemp(join(tmpdir(), "goal-lease-owner-symlink-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const outside = join(dir, "outside-owner.json") + const secret = "outside-secret-owner-token" + await mkdir(lock) + await writeFile( + outside, + JSON.stringify({ token: secret, pid: process.pid, hostname: "valid.example" }), + ) + await symlink(outside, `${lock}/owner.json`) + try { + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: null, hostname: null }) + assert.equal(error.reason, "legacy_lock") + assert.doesNotMatch(error.message, /outside-secret/) + return true + }) + await utimes(lock, new Date(0), new Date(0)) + await assert.rejects(acquirePersistenceLease(state, { malformedGraceMs: 1 }), (error) => { + assert.equal(error.reason, "legacy_lock") + return true + }) + assert.match(await readFile(outside, "utf8"), /outside-secret-owner-token/) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease rejects a symlinked lock directory without traversing it", async (t) => { + if (process.platform === "win32") { + t.skip("symlink creation is not reliably available on Windows CI") + return + } + const dir = await mkdtemp(join(tmpdir(), "goal-lease-lock-symlink-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const outside = join(dir, "outside-lock") + const ownerPath = join(outside, "owner.json") + const secret = "outside-lock-secret-token" + await mkdir(outside) + await writeFile( + ownerPath, + JSON.stringify({ token: secret, pid: process.pid, hostname: "valid.example" }), + ) + await symlink(outside, lock, "dir") + try { + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), false) + assert.equal(error.code, "ERR_GOAL_PERSISTENCE_LEASE_PATH") + assert.doesNotMatch(error.message, /outside-lock-secret/) + return true + }) + assert.match(await readFile(ownerPath, "utf8"), /outside-lock-secret-token/) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease rejects a symlinked claim directory without traversing it", async (t) => { + if (process.platform === "win32") { + t.skip("symlink creation is not reliably available on Windows CI") + return + } + const dir = await mkdtemp(join(tmpdir(), "goal-lease-claims-symlink-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + const claimDirectory = persistenceLeaseInternals.claimDirectoryPathFor(lock) + const outside = join(dir, "outside-claims") + const secretPath = join(outside, "secret.txt") + try { + await persistenceLeaseInternals.publishLegacyGuard(lock) + await mkdir(outside) + await writeFile(secretPath, "outside-claims-secret") + await symlink(outside, claimDirectory, "dir") + + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), false) + assert.equal(error.code, "ERR_GOAL_PERSISTENCE_LEASE_PATH") + assert.doesNotMatch(error.message, /outside-claims-secret/) + return true + }) + assert.equal(await readFile(secretPath, "utf8"), "outside-claims-secret") + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease treats a remote legacy owner as manual-recovery contention", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-remote-owner-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + await mkdir(lock) + await writeFile( + `${lock}/owner.json`, + JSON.stringify({ token: "remote", pid: 4242, hostname: "remote.example" }), + ) + try { + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.deepEqual(error.owner, { pid: 4242, hostname: "remote.example" }) + assert.equal(error.reason, "legacy_lock") + return true + }) + assert.equal(JSON.parse(await readFile(`${lock}/owner.json`, "utf8")).token, "remote") + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("the long-lived regular-file guard prevents version-1 stale reclamation", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-v1-compatibility-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + try { + const lease = await acquirePersistenceLease(state) + const sentinelBefore = JSON.parse(await readFile(lock, "utf8")) + const guardInfo = await stat(lock) + + assert.equal(persistenceLeaseInternals.validLegacySentinel(sentinelBefore), true) + assert.equal(guardInfo.isFile(), true) + assert.equal(persistenceLeaseInternals.legacyGuardMtimeIsSafe(guardInfo), true) + await assert.rejects(acquireVersion1Lease(state), /version-1 lease contended/) + + assert.equal(await lease.release(), true) + assert.deepEqual( + JSON.parse(await readFile(lock, "utf8")), + sentinelBefore, + ) + assert.deepEqual(await claimNames(lock), []) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test("persistence lease treats an invalid regular guard as manual-recovery contention", async () => { + const dir = await mkdtemp(join(tmpdir(), "goal-lease-owner-io-")) + const state = join(dir, "state.json") + const lock = `${state}.lock` + await writeFile(lock, "not a directory") + try { + await assert.rejects(acquirePersistenceLease(state), (error) => { + assert.equal(isPersistenceLeaseContendedError(error), true) + assert.equal(error.reason, "legacy_lock") + return true + }) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) diff --git a/test/session-concurrency.test.js b/test/session-concurrency.test.js index 2a2ed1f..37c2c44 100644 --- a/test/session-concurrency.test.js +++ b/test/session-concurrency.test.js @@ -1,43 +1,18 @@ import assert from "node:assert/strict" import { createHash } from "node:crypto" -import { mkdtemp, readFile, rm } from "node:fs/promises" +import { mkdtemp, readFile, readdir, rm } from "node:fs/promises" import { tmpdir } from "node:os" import { join } from "node:path" import { spawn } from "node:child_process" import test from "node:test" +import { GoalPlugin } from "../src/goal-plugin.js" function sessionStatePath(stateFilePath, sessionID) { const key = createHash("sha256").update(sessionID).digest("hex") return join(`${stateFilePath}.sessions`, key, "state.json") } -function spawnGoalProcess(moduleURL, stateFilePath, sessionID, objective, { expectLeaseError = false } = {}) { - const action = expectLeaseError - ? ` - try { - await hooks["command.execute.before"]( - { command: "goal", sessionID: ${JSON.stringify(sessionID)}, arguments: ${JSON.stringify(objective)} }, - { parts: [] }, - ) - process.stdout.write("UNEXPECTED\\n") - await hooks.dispose() - process.exit(1) - } catch (error) { - process.stdout.write("ERROR:" + (error?.message || error) + "\\n") - process.exit(0) - } - ` - : ` - await hooks["command.execute.before"]( - { command: "goal", sessionID: ${JSON.stringify(sessionID)}, arguments: ${JSON.stringify(objective)} }, - { parts: [] }, - ) - process.stdout.write("READY\\n") - process.stdin.once("data", async () => { - await hooks.dispose() - process.exit(0) - }) - ` +function spawnGoalProcess(moduleURL, stateFilePath, sessionID, objective) { const source = ` import { GoalPlugin } from ${JSON.stringify(moduleURL)} const client = { @@ -48,7 +23,15 @@ function spawnGoalProcess(moduleURL, stateFilePath, sessionID, objective, { expe { client, directory: ${JSON.stringify(tmpdir())} }, { stateFilePath: ${JSON.stringify(stateFilePath)}, registerTools: false, minDelayMs: 1 }, ) - ${action} + await hooks["command.execute.before"]( + { command: "goal", sessionID: ${JSON.stringify(sessionID)}, arguments: ${JSON.stringify(objective)} }, + { parts: [] }, + ) + process.stdout.write("READY\\n") + process.stdin.once("data", async () => { + await hooks.dispose() + process.exit(0) + }) ` const child = spawn(process.execPath, ["--input-type=module", "-e", source], { stdio: ["pipe", "pipe", "pipe"], @@ -64,7 +47,7 @@ function spawnGoalProcess(moduleURL, stateFilePath, sessionID, objective, { expe } child.stdout.on("data", (chunk) => { stdout += chunk.toString() - if (stdout.includes(expectLeaseError ? "ERROR:" : "READY")) { + if (stdout.includes("READY")) { settled = true resolve({ stdout, stderr }) } @@ -115,29 +98,129 @@ test("independent sessions in one project persist concurrently in separate shard } }) -test("the same session remains single-writer across processes", async () => { +test("a same-session process contender stays passive and takes over only after an explicit retry", async () => { const directory = await mkdtemp(join(tmpdir(), "goal-plugin-single-session-")) const stateFilePath = join(directory, "state.json") const moduleURL = new URL("../src/goal-plugin.js", import.meta.url).href const first = spawnGoalProcess(moduleURL, stateFilePath, "session-single-writer", "hold the session") - let second + const promptCalls = [] + const messageCalls = [] + const logs = [] + let hostMessages = [] + let contender try { await first.ready - second = spawnGoalProcess( - moduleURL, - stateFilePath, - "session-single-writer", - "take the session", - { expectLeaseError: true }, + const persistedBefore = await readFile( + sessionStatePath(stateFilePath, "session-single-writer"), + "utf8", + ) + + contender = await GoalPlugin( + { + client: { + app: { log: async (input) => logs.push(input) }, + session: { + messages: async (input) => { + messageCalls.push(input) + return { data: hostMessages } + }, + promptAsync: async (input) => { + promptCalls.push(input) + return {} + }, + }, + }, + directory, + }, + { stateFilePath, registerTools: false, minDelayMs: 1 }, + ) + + await assert.doesNotReject(() => + contender["chat.params"]({ sessionID: "session-single-writer", agent: "build" }), + ) + const denial = { + message: { id: "passive-command", role: "user", sessionID: "session-single-writer" }, + parts: [], + } + await contender["command.execute.before"]( + { command: "goal", sessionID: "session-single-writer", arguments: "status" }, + denial, + ) + assert.match(denial.parts[0].text, /Goal controls are unavailable/) + Object.assign(denial.parts[0], { + id: "passive-command-part", + messageID: "passive-command", + sessionID: "session-single-writer", + }) + await contender["chat.message"]( + { sessionID: "session-single-writer", messageID: "passive-command", agent: "build" }, + denial, + ) + hostMessages = [{ + info: { + id: "passive-command-assistant", + parentID: "passive-command", + role: "assistant", + sessionID: "session-single-writer", + }, + parts: [{ type: "text", text: "Ownership denial reported." }], + }] + await contender.event({ + event: { + type: "session.status", + properties: { sessionID: "session-single-writer", status: { type: "idle" } }, + }, + }) + assert.equal( + await readFile(sessionStatePath(stateFilePath, "session-single-writer"), "utf8"), + persistedBefore, + ) + assert.equal(promptCalls.length, 0) + assert.equal(messageCalls.length, 1) + assert.equal(logs.length, 1) + + first.child.stdin.write("stop\n") + assert.equal(await waitForExit(first.child), 0) + await new Promise((resolve) => setTimeout(resolve, 300)) + + const ordinaryParts = [{ type: "text", text: "ordinary chat after owner exit" }] + await contender["chat.message"]( + { sessionID: "session-single-writer", messageID: "ordinary-after-exit", agent: "build" }, + { + message: { id: "ordinary-after-exit", role: "user", sessionID: "session-single-writer" }, + parts: ordinaryParts, + }, + ) + assert.equal(ordinaryParts[0].text, "ordinary chat after owner exit") + const releasedLockPath = `${sessionStatePath(stateFilePath, "session-single-writer")}.lock` + const sentinel = JSON.parse(await readFile(releasedLockPath, "utf8")) + assert.equal(sentinel.protocol, 2) + assert.equal(sentinel.sentinel, true) + assert.deepEqual( + (await readdir(`${releasedLockPath}.claims-v2`)) + .filter((name) => name.startsWith("claim-")), + [], ) - const result = await second.ready - assert.match(result.stdout, /goal persistence is already owned by pid/) - assert.equal(await waitForExit(second.child), 0) + + const takeover = { parts: [] } + await contender["command.execute.before"]( + { command: "goal", sessionID: "session-single-writer", arguments: "status" }, + takeover, + ) + assert.match(takeover.parts[0].text, /hold the session/) + assert.match(takeover.parts[0].text, /recovered after restart|paused|stopped/i) + await contender.event({ + event: { + type: "session.status", + properties: { sessionID: "session-single-writer", status: { type: "idle" } }, + }, + }) + assert.equal(promptCalls.length, 0) } finally { if (first.child.exitCode === null) first.child.stdin.write("stop\n") - await Promise.all([waitForExit(first.child), second ? waitForExit(second.child) : Promise.resolve()]) + await waitForExit(first.child) + await contender?.dispose() first.child.kill() - second?.child.kill() await rm(directory, { recursive: true, force: true }) } }) From 05146d0cbf051d63386e32d351d025f9de1fe854 Mon Sep 17 00:00:00 2001 From: willytop8 Date: Sat, 1 Aug 2026 23:25:45 -0500 Subject: [PATCH 2/2] test(ci): harden portable lease assertions Check the exact private lease token instead of rejecting UUID-shaped hostnames, and retain a finite logger hang detector without imposing a 250ms filesystem deadline. --- test/host-lifecycle.test.js | 6 +++--- test/persistence-lease.test.js | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/host-lifecycle.test.js b/test/host-lifecycle.test.js index d9882fc..68b4f5c 100644 --- a/test/host-lifecycle.test.js +++ b/test/host-lifecycle.test.js @@ -972,7 +972,7 @@ test("passive entry and disposal do not wait for a stalled host logger", { timeo } }) -test("persistence recovery and disposal do not wait for a stalled host logger", { timeout: 2_000 }, async () => { +test("persistence recovery and disposal do not wait for a stalled host logger", { timeout: 5_000 }, async () => { const directory = await fs.mkdtemp(join(tmpdir(), "goal-plugin-error-logger-")) const stateFilePath = join(directory, "state.json") const sessionID = "stalled-error-logger-session" @@ -1000,7 +1000,7 @@ test("persistence recovery and disposal do not wait for a stalled host logger", assert.equal( await Promise.race([ loadPromise.then(() => "settled"), - new Promise((resolve) => setTimeout(() => resolve("timed-out"), 250)), + new Promise((resolve) => setTimeout(() => resolve("timed-out"), 1_500)), ]), "settled", ) @@ -1012,7 +1012,7 @@ test("persistence recovery and disposal do not wait for a stalled host logger", assert.equal( await Promise.race([ disposePromise.then(() => "settled"), - new Promise((resolve) => setTimeout(() => resolve("timed-out"), 250)), + new Promise((resolve) => setTimeout(() => resolve("timed-out"), 1_500)), ]), "settled", ) diff --git a/test/persistence-lease.test.js b/test/persistence-lease.test.js index 6ac068d..747f609 100644 --- a/test/persistence-lease.test.js +++ b/test/persistence-lease.test.js @@ -135,7 +135,8 @@ test("persistence lease rejects a concurrent owner and releases by token", async assert.equal(Object.isFrozen(error.owner), true) assert.equal(error.reason, "owned_elsewhere") assert.equal("token" in error.owner, false) - assert.doesNotMatch(error.message, /state\.json|[0-9a-f]{8}-[0-9a-f-]{27}/i) + assert.doesNotMatch(error.message, /state\.json/i) + assert.equal(error.message.includes(first.owner.token), false) return true }) const sentinelBeforeRelease = await readFile(`${state}.lock`, "utf8")