diff --git a/package-lock.json b/package-lock.json index f15560fc..2fa7f9fc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -53,7 +53,7 @@ "playwright": "^1.47.0", "tsx": "^4.21.0", "typescript": "^5.9.3", - "vitest": "^4.0.18" + "vitest": "^4.1.11" }, "engines": { "node": ">=22" @@ -1300,9 +1300,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1319,9 +1316,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1338,9 +1332,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1357,9 +1348,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1376,9 +1364,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1395,9 +1380,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1414,9 +1396,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1433,9 +1412,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1452,9 +1428,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1477,9 +1450,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1502,9 +1472,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1527,9 +1494,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1552,9 +1516,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1577,9 +1538,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1602,9 +1560,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1627,9 +1582,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -2948,7 +2900,7 @@ "version": "19.2.17", "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.17.tgz", "integrity": "sha512-MXfmqaVPEVgkBT/aY0aGCkRWWtByiYQXo3xdQ8r5RzuFrPiRn8Gar2tQdXSUQ2GKV3bkXckek89V8wQBY2Q/Aw==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "csstype": "^3.2.2" @@ -3001,16 +2953,16 @@ } }, "node_modules/@vitest/expect": { - "version": "4.1.8", - "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.8.tgz", - "integrity": "sha512-h3nDO677RDLEGlBxyQ5CW8RlMThSKSRLUePLOx09gNIWRL40edgA1GCZSZgf1W55MFAG6/Sw14KeaAnqv0NKdQ==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.11.tgz", + "integrity": "sha512-VX2x5vNJXET47KAFzwERI+KRMtTTCSWTfSMKsW7JsUsXV4psq++e3DvZpuTDOpHcxytiDs6p2nhVb2tVDiiUYw==", "dev": true, "license": "MIT", "dependencies": { "@standard-schema/spec": "^1.1.0", "@types/chai": "^5.2.2", - "@vitest/spy": "4.1.8", - "@vitest/utils": "4.1.8", + "@vitest/spy": "4.1.11", + "@vitest/utils": "4.1.11", "chai": "^6.2.2", "tinyrainbow": "^3.1.0" }, @@ -3018,14 +2970,42 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/@vitest/expect/node_modules/@vitest/pretty-format": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.11.tgz", + "integrity": "sha512-yiZzPbGTS9Sr/JpFl8zHrcIkAofNbFV6k21vIgQN/cY/oxZeXhJv5sc/MBJ5jFKWmWs+oJHw0UXLZjmf931+Vw==", + "dev": true, + "license": "MIT", + "dependencies": { + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, + "node_modules/@vitest/expect/node_modules/@vitest/utils": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.11.tgz", + "integrity": "sha512-zTCVGpyFsGWBhllOyKlTw/vnr6D9qxsfSDyfbyZmTyjHw5N/VuvzHpHoQjm2ZJzn4RJgx5w4r7V0er69CmLgPQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@vitest/pretty-format": "4.1.11", + "convert-source-map": "^2.0.0", + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, "node_modules/@vitest/mocker": { - "version": "4.1.8", - "resolved": "https://registry.npmjs.org/@vitest/mocker/-/mocker-4.1.8.tgz", - "integrity": "sha512-LEiN/xe4OSIbKe9HQIp5OC24agGD9J5CnmMgsLohVVoOPWL9a2sBoR6VBx43jQZb7Kr1l4RCuyCJzcAa0+dojw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/mocker/-/mocker-4.1.11.tgz", + "integrity": "sha512-2XJVD55d1o5AZous5CCGKS74g/riOj9odEt2bQpCVZeblHyHdnMeFl4jl0XjU21stf4mbjUkew2eXQZt65g5CQ==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/spy": "4.1.8", + "@vitest/spy": "4.1.11", "estree-walker": "^3.0.3", "magic-string": "^0.30.21" }, @@ -3059,28 +3039,56 @@ } }, "node_modules/@vitest/runner": { - "version": "4.1.8", - "resolved": "https://registry.npmjs.org/@vitest/runner/-/runner-4.1.8.tgz", - "integrity": "sha512-EmVxeBAfMJvycdjd6Hm+RbFBbA9fKvo0Kx37hNpBYoYeavH3RNsBXWDooR1mgD52dCrxIIuP7UotpfiwOikvcg==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/runner/-/runner-4.1.11.tgz", + "integrity": "sha512-LztvUgdwMNJMIkj3hQnnxiC2Xy1zNxq928W/xhjCLaNCzqTZOudjwbQf6v9IntZGPw132i2Lq2rgTRZHD3JHNw==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/utils": "4.1.8", + "@vitest/utils": "4.1.11", "pathe": "^2.0.3" }, "funding": { "url": "https://opencollective.com/vitest" } }, + "node_modules/@vitest/runner/node_modules/@vitest/pretty-format": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.11.tgz", + "integrity": "sha512-yiZzPbGTS9Sr/JpFl8zHrcIkAofNbFV6k21vIgQN/cY/oxZeXhJv5sc/MBJ5jFKWmWs+oJHw0UXLZjmf931+Vw==", + "dev": true, + "license": "MIT", + "dependencies": { + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, + "node_modules/@vitest/runner/node_modules/@vitest/utils": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.11.tgz", + "integrity": "sha512-zTCVGpyFsGWBhllOyKlTw/vnr6D9qxsfSDyfbyZmTyjHw5N/VuvzHpHoQjm2ZJzn4RJgx5w4r7V0er69CmLgPQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@vitest/pretty-format": "4.1.11", + "convert-source-map": "^2.0.0", + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, "node_modules/@vitest/snapshot": { - "version": "4.1.8", - "resolved": "https://registry.npmjs.org/@vitest/snapshot/-/snapshot-4.1.8.tgz", - "integrity": "sha512-acfZboRmAIf05DEKcBQy33VXojFJjtUdLyo7oOmV9kebb2xdU01UknNiPuPZoJZQyO7DF0gZdTGTpeAzET9QPQ==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/snapshot/-/snapshot-4.1.11.tgz", + "integrity": "sha512-pN7ikn1ON7h8ee4gIAp4AzyK+zBtJPzVbqOgu5LCEh4VaJVbPQcgYQYJIMGQPXVeJJq1fnfazis7a5pFNPahog==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/pretty-format": "4.1.8", - "@vitest/utils": "4.1.8", + "@vitest/pretty-format": "4.1.11", + "@vitest/utils": "4.1.11", "magic-string": "^0.30.21", "pathe": "^2.0.3" }, @@ -3088,10 +3096,38 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/@vitest/snapshot/node_modules/@vitest/pretty-format": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.11.tgz", + "integrity": "sha512-yiZzPbGTS9Sr/JpFl8zHrcIkAofNbFV6k21vIgQN/cY/oxZeXhJv5sc/MBJ5jFKWmWs+oJHw0UXLZjmf931+Vw==", + "dev": true, + "license": "MIT", + "dependencies": { + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, + "node_modules/@vitest/snapshot/node_modules/@vitest/utils": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.11.tgz", + "integrity": "sha512-zTCVGpyFsGWBhllOyKlTw/vnr6D9qxsfSDyfbyZmTyjHw5N/VuvzHpHoQjm2ZJzn4RJgx5w4r7V0er69CmLgPQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@vitest/pretty-format": "4.1.11", + "convert-source-map": "^2.0.0", + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, "node_modules/@vitest/spy": { - "version": "4.1.8", - "resolved": "https://registry.npmjs.org/@vitest/spy/-/spy-4.1.8.tgz", - "integrity": "sha512-6EevtBp6OZOPF7bmz36HrGMeP3txgVSrgebWxHOafDXGkhIzfXK14f8KF6MuFfgXXUeHxmpD3BQxkV00/3s5mA==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/spy/-/spy-4.1.11.tgz", + "integrity": "sha512-apNa/prQy2qCeywhnixOHPRCgGNhvg7T4Dapfl1GahLp/R+uhBm5cPyFoNVyqsNd2h1nJxL6BqqdIjiABL60YA==", "dev": true, "license": "MIT", "funding": { @@ -4050,7 +4086,7 @@ "version": "3.2.3", "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.2.3.tgz", "integrity": "sha512-z1HGKcYy2xA8AGQfwrn0PAy+PB7X/GSj3UVJW9qKyn43xWa+gl5nXmU4qqLMRzWVLFC8KusUX8T/0kCiOYpAIQ==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/data-urls": { @@ -7727,19 +7763,19 @@ } }, "node_modules/vitest": { - "version": "4.1.8", - "resolved": "https://registry.npmjs.org/vitest/-/vitest-4.1.8.tgz", - "integrity": "sha512-flY6ScbCIt9HThs+C5HS7jvGOB560DJtk/Z15IQROTA6zEy49Nh8T/dofWTQL+n3vswqn87sbJNiuqw1SDp5Ig==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/vitest/-/vitest-4.1.11.tgz", + "integrity": "sha512-fhACrNXUidIbGSBr5FlbuBkO7VWC1ZyLl0DO4CU2DrQoAPxX84Ysxs+HeGQpii5lZWV1Q4gBZTTu49mF+A6Edw==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/expect": "4.1.8", - "@vitest/mocker": "4.1.8", - "@vitest/pretty-format": "4.1.8", - "@vitest/runner": "4.1.8", - "@vitest/snapshot": "4.1.8", - "@vitest/spy": "4.1.8", - "@vitest/utils": "4.1.8", + "@vitest/expect": "4.1.11", + "@vitest/mocker": "4.1.11", + "@vitest/pretty-format": "4.1.11", + "@vitest/runner": "4.1.11", + "@vitest/snapshot": "4.1.11", + "@vitest/spy": "4.1.11", + "@vitest/utils": "4.1.11", "es-module-lexer": "^2.0.0", "expect-type": "^1.3.0", "magic-string": "^0.30.21", @@ -7767,12 +7803,12 @@ "@edge-runtime/vm": "*", "@opentelemetry/api": "^1.9.0", "@types/node": "^20.0.0 || ^22.0.0 || >=24.0.0", - "@vitest/browser-playwright": "4.1.8", - "@vitest/browser-preview": "4.1.8", - "@vitest/browser-webdriverio": "4.1.8", - "@vitest/coverage-istanbul": "4.1.8", - "@vitest/coverage-v8": "4.1.8", - "@vitest/ui": "4.1.8", + "@vitest/browser-playwright": "4.1.11", + "@vitest/browser-preview": "4.1.11", + "@vitest/browser-webdriverio": "4.1.11", + "@vitest/coverage-istanbul": "4.1.11", + "@vitest/coverage-v8": "4.1.11", + "@vitest/ui": "4.1.11", "happy-dom": "*", "jsdom": "*", "vite": "^6.0.0 || ^7.0.0 || ^8.0.0" @@ -7816,6 +7852,34 @@ } } }, + "node_modules/vitest/node_modules/@vitest/pretty-format": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.11.tgz", + "integrity": "sha512-yiZzPbGTS9Sr/JpFl8zHrcIkAofNbFV6k21vIgQN/cY/oxZeXhJv5sc/MBJ5jFKWmWs+oJHw0UXLZjmf931+Vw==", + "dev": true, + "license": "MIT", + "dependencies": { + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, + "node_modules/vitest/node_modules/@vitest/utils": { + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.11.tgz", + "integrity": "sha512-zTCVGpyFsGWBhllOyKlTw/vnr6D9qxsfSDyfbyZmTyjHw5N/VuvzHpHoQjm2ZJzn4RJgx5w4r7V0er69CmLgPQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@vitest/pretty-format": "4.1.11", + "convert-source-map": "^2.0.0", + "tinyrainbow": "^3.1.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + } + }, "node_modules/w3c-xmlserializer": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/w3c-xmlserializer/-/w3c-xmlserializer-5.0.0.tgz", diff --git a/package.json b/package.json index 54953ef0..0b43e178 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,7 @@ "playwright": "^1.47.0", "tsx": "^4.21.0", "typescript": "^5.9.3", - "vitest": "^4.0.18" + "vitest": "^4.1.11" }, "engines": { "node": ">=22" diff --git a/src/support/serviceInstanceLock.ts b/src/support/serviceInstanceLock.ts index 1215ad68..a4adf552 100644 --- a/src/support/serviceInstanceLock.ts +++ b/src/support/serviceInstanceLock.ts @@ -9,9 +9,12 @@ export interface ServiceInstanceLock { } /** - * Acquire the daemon's lifetime lock before any external client or scheduler is - * initialized. Port probing alone is a check-then-bind TOCTOU: two processes can - * both observe a free port and connect side-effecting services before one loses + * Acquire a process-lifetime SQLite writer lock (`BEGIN IMMEDIATE`). + * Used for the daemon instance lock and, during AGT-4024, the task-state + * store mutex at `~/.openswarm/task-state-lock.db`. + * + * Port probing alone is a check-then-bind TOCTOU: two processes can both + * observe a free port and connect side-effecting services before one loses * the later listen(2). SQLite's writer lock is kernel-owned and is released on * crash, so it does not need unsafe stale-PID deletion. */ diff --git a/src/taskState/store.test.ts b/src/taskState/store.test.ts index ff57c19d..54186dc1 100644 --- a/src/taskState/store.test.ts +++ b/src/taskState/store.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, unlinkSync, utimesSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; -import { hostname, tmpdir } from 'node:os'; +import { tmpdir } from 'node:os'; import { spawn } from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { @@ -23,14 +23,9 @@ import { buildLockPayload, type OpenSwarmTaskState, } from './store.js'; -import { PROCESS_STARTED_AT_MS, isProofCapableSpace, processNamespaceId } from '../support/processLiveness.js'; +import { processNamespaceId } from '../support/processLiveness.js'; import { getInstanceId } from '../support/healthEndpoint.js'; - -// The fast-path proof needs a REAL pid space, which only Linux can give (boot -// id + pid-namespace inode). Elsewhere the recorded id is a machine hint, good -// for ruling a record out but never for the proof — so these cases cannot arise -// there at all. Asserted on Linux in CI. -const itWithPidSpace = isProofCapableSpace(processNamespaceId()) ? it : it.skip; +import { acquireServiceInstanceLock } from '../support/serviceInstanceLock.js'; describe('task state store', () => { @@ -60,6 +55,7 @@ describe('task state store', () => { stateDir = mkdtempSync(join(tmpdir(), 'openswarm-task-state-')); stateFile = join(stateDir, 'state.json'); process.env.OPENSWARM_TASK_STATE_FILE = stateFile; + process.env.OPENSWARM_TASK_STATE_LOCK_DB = join(stateDir, 'task-state-lock.db'); resetTaskStateStoreForTests(); }); @@ -107,6 +103,8 @@ describe('task state store', () => { } rmSync(stateDir, { recursive: true, force: true }); delete process.env.OPENSWARM_TASK_STATE_FILE; + delete process.env.OPENSWARM_TASK_STATE_LOCK_DB; + delete process.env.OPENSWARM_TASK_STATE_LOCK_TIMEOUT_MS; delete process.env.OPENSWARM_TASK_STATE_TRUSTED_COMMENT_USERS; }); @@ -813,18 +811,32 @@ describe('task state store', () => { expect(readFileSync(stateFile, 'utf8')).toBe('{not-json'); }); - describe('stale lock reclamation (AGT-4023)', () => { - // Measured on vela: a container recreate left a well-formed lock behind, - // the new daemon came up on the SAME pid (a container assigns it - // deterministically), so the pid probe answered "alive" for the daemon's - // own ghost. Fifteen straight heartbeats died on it, zero tasks ran for 75 - // minutes, and a manual rm was the only way out. + describe('dual-lock transition (AGT-4024)', () => { + // Mutual exclusion is the kernel-owned SQLite writer lock. The dot-lock is + // kept only so old CLIs (dot-lock only) still exclude new binaries (both). const lockFile = () => `${stateFile}.lock`; - it('reclaims a well-formed lock left at our own pid once it is long expired', () => { + it('takes the SQLite lock before mutating, at the configured DB path', () => { + const lockDb = process.env.OPENSWARM_TASK_STATE_LOCK_DB!; + // Holding the same DB path must block store writers (BUSY → wait → reclaim path). + const held = acquireServiceInstanceLock(lockDb); + process.env.OPENSWARM_TASK_STATE_LOCK_TIMEOUT_MS = '80'; + try { + expect(() => upsertTaskState('ISSUE-SQLITE-BUSY', { execution: { status: 'todo', retryCount: 0 } })) + .toThrow(/Timed out waiting for task state SQLite lock/); + } finally { + held.release(); + } + upsertTaskState('ISSUE-SQLITE-BUSY', { execution: { status: 'todo', retryCount: 0 } }); + expect(getTaskState('ISSUE-SQLITE-BUSY')?.execution.status).toBe('todo'); + expect(existsSync(lockDb)).toBe(true); + }); + + it('reclaims an abandoned compatibility dot-lock after the wait deadline', () => { + // Crash of a dual-lock holder leaves the file behind; SQLite was released + // by the kernel. With a short deadline we reclaim without PID/age heuristics. writeFileSync(lockFile(), JSON.stringify({ pid: process.pid, token: 'ghost-token' }), 'utf8'); - const longAgo = new Date(Date.now() - 900_000); - utimesSync(lockFile(), longAgo, longAgo); + process.env.OPENSWARM_TASK_STATE_LOCK_TIMEOUT_MS = '80'; upsertTaskState('ISSUE-LOCK-1', { execution: { status: 'todo', retryCount: 0 } }); @@ -832,177 +844,24 @@ describe('task state store', () => { expect(existsSync(lockFile())).toBe(false); }); - it('respects a well-formed lock right up to the expiry, on its own clock', () => { - // Pins the policy from below: a lock older than the 30s malformed clock - // but inside the 10-minute one is still someone's — it may belong to a - // live writer in another pid namespace sharing the mounted state file. - // Shortening LOCK_ABANDON_MS toward LOCK_STALE_MS fails here. - writeFileSync(lockFile(), JSON.stringify({ pid: process.pid, token: 'possibly-live' }), 'utf8'); - const almostExpired = new Date(Date.now() - 570_000); - utimesSync(lockFile(), almostExpired, almostExpired); - - expect(() => upsertTaskState('ISSUE-LOCK-2', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); - - // AGT-4068: the 10-minute wait above is a timer, not evidence, and it cost - // ten minutes of dead heartbeats after every container restart — measured - // on vela 2026-08-29, where the new daemon came up on pid 7 holding a lock - // pid 7 had written 0.7s before the container was recreated. A pid is - // unique among live processes, so a lock carrying OUR pid that predates our - // start belongs to a process that has exited. No wait required. - itWithPidSpace('reclaims a same-namespace lock at our own pid that predates this process, well inside the expiry', () => { - writeFileSync(lockFile(), JSON.stringify({ - pid: process.pid, token: 'prior-generation', ns: processNamespaceId(), - }), 'utf8'); - const beforeWeStarted = new Date(PROCESS_STARTED_AT_MS - 60_000); // and only a minute old - utimesSync(lockFile(), beforeWeStarted, beforeWeStarted); - - upsertTaskState('ISSUE-LOCK-5', { execution: { status: 'todo', retryCount: 0 } }); - - expect(getTaskState('ISSUE-LOCK-5')?.execution.status).toBe('todo'); - expect(existsSync(lockFile())).toBe(false); - }); - - it('does not reason about a pid from another pid space, however old the lock', () => { - // Two containers sharing this mounted state file each have their own - // pid 1, so a matching pid number means nothing across them. Such a lock - // gets the age rule and nothing else — it may be a live writer. - writeFileSync(lockFile(), JSON.stringify({ - pid: process.pid, token: 'other-container', ns: 'some-other-host:pid:[4026531999]', - }), 'utf8'); - const beforeWeStarted = new Date(PROCESS_STARTED_AT_MS - 60_000); - utimesSync(lockFile(), beforeWeStarted, beforeWeStarted); - - expect(() => upsertTaskState('ISSUE-LOCK-6', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); - - it('does not reclaim a foreign-namespace lock just because its pid is dead here (AGT-4068)', () => { - // The pid probe runs before any age rule, so an ESRCH answer reclaims - // outright. That pid belongs to another container's space, where it may - // be a live writer; our local probe is answering a different question. - writeFileSync(lockFile(), JSON.stringify({ - pid: 2_147_483_647, // certainly not alive HERE - token: 'other-container', - ns: 'some-other-host:pid:[4026531999]', - }), 'utf8'); - const recent = new Date(Date.now() - 60_000); - utimesSync(lockFile(), recent, recent); - - expect(() => upsertTaskState('ISSUE-LOCK-7', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); - - it('still reclaims a legacy lock with no namespace recorded when its pid is dead', () => { - // Locks written before the field existed keep the original behaviour. - writeFileSync(lockFile(), JSON.stringify({ pid: 2_147_483_647, token: 'legacy' }), 'utf8'); - const recent = new Date(Date.now() - 60_000); - utimesSync(lockFile(), recent, recent); - - upsertTaskState('ISSUE-LOCK-8', { execution: { status: 'todo', retryCount: 0 } }); - - expect(getTaskState('ISSUE-LOCK-8')?.execution.status).toBe('todo'); - }); - - it('does not probe a lock whose writer could name no pid space at all (AGT-4068)', () => { - // `ns: null` is a writer that could not read its own pid namespace, so - // our pid table is not its pid table and a local ESRCH means nothing. - // Distinct from an ABSENT ns, which predates the field and keeps the - // original probe — an omitted key would have conflated the two. - writeFileSync(lockFile(), JSON.stringify({ - pid: 2_147_483_647, token: 'unknown-space', ns: null, - }), 'utf8'); - const recent = new Date(Date.now() - 60_000); - utimesSync(lockFile(), recent, recent); - - expect(() => upsertTaskState('ISSUE-LOCK-9', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); - - it('withholds the fast-path proof from a lock with no named pid space (AGT-4068)', () => { - // Our own pid, written before we started — the proof would reclaim this - // at once if the space were named and matched. Unnamed, it must wait out - // the age rule like any pre-field lock. - writeFileSync(lockFile(), JSON.stringify({ - pid: process.pid, token: 'unknown-space-live-pid', ns: null, - }), 'utf8'); - const beforeWeStarted = new Date(PROCESS_STARTED_AT_MS - 60_000); - utimesSync(lockFile(), beforeWeStarted, beforeWeStarted); - - expect(() => upsertTaskState('ISSUE-LOCK-10', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); + it('reclaims a malformed abandoned compatibility lock the same way', () => { + writeFileSync(lockFile(), 'not-json-at-all', 'utf8'); + process.env.OPENSWARM_TASK_STATE_LOCK_TIMEOUT_MS = '80'; - it('does not let a matching machine HINT license the pid proof (AGT-4068)', () => { - // `host:` matches ours, which keeps the local probe — but it does - // not establish that our pid numbering is the writer's, so the proof - // stays off and the age rule governs. - writeFileSync(lockFile(), JSON.stringify({ - pid: process.pid, token: 'hint-not-proof', ns: `host:${hostname()}`, - }), 'utf8'); - const beforeWeStarted = new Date(PROCESS_STARTED_AT_MS - 60_000); - utimesSync(lockFile(), beforeWeStarted, beforeWeStarted); - - expect(() => upsertTaskState('ISSUE-LOCK-11', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); + upsertTaskState('ISSUE-LOCK-4', { execution: { status: 'todo', retryCount: 0 } }); - itWithPidSpace('reclaims a prior-generation lock written half a second before we started (AGT-4071)', () => { - // The production case, at production timing: the outgoing container wrote - // this at 17:12:12.670 and its successor started at 17:12:13.231. The - // timestamp path cannot see 0.561 s past a 1 s margin; the owner id can. - writeFileSync(lockFile(), JSON.stringify({ - pid: process.pid, - token: 'prior-generation', - ns: processNamespaceId(), - instance: 'a-previous-boot', - }), 'utf8'); - const justBeforeWeStarted = new Date(PROCESS_STARTED_AT_MS - 500); - utimesSync(lockFile(), justBeforeWeStarted, justBeforeWeStarted); - - upsertTaskState('ISSUE-LOCK-12', { execution: { status: 'todo', retryCount: 0 } }); - - expect(getTaskState('ISSUE-LOCK-12')?.execution.status).toBe('todo'); + expect(getTaskState('ISSUE-LOCK-4')?.execution.status).toBe('todo'); expect(existsSync(lockFile())).toBe(false); }); - itWithPidSpace('does not reclaim a lock this very process holds, however its mtime reads (AGT-4071)', () => { - // A coarse-granularity filesystem can report our own fresh lock as older - // than our start. The owner id keeps that from mattering. - writeFileSync(lockFile(), JSON.stringify({ - pid: process.pid, - token: 'ours-right-now', - ns: processNamespaceId(), - instance: getInstanceId(), - }), 'utf8'); - const impossiblyOld = new Date(PROCESS_STARTED_AT_MS - 500); - utimesSync(lockFile(), impossiblyOld, impossiblyOld); - - expect(() => upsertTaskState('ISSUE-LOCK-13', { execution: { status: 'todo', retryCount: 0 } })) - .toThrow(/Timed out waiting for task state lock/); - }); - - it('records who holds it, so a successor can decide ownership without a clock (AGT-4071)', () => { + it('records who holds the compatibility lock (observability only)', () => { const payload = buildLockPayload('tok-1'); expect(payload.pid).toBe(process.pid); expect(payload.token).toBe('tok-1'); expect(payload.instance).toBe(getInstanceId()); - // `null`, not absent: an omitted key reads as "written before the field - // existed", which is entitled to a local pid probe. expect('ns' in payload).toBe(true); expect(payload.ns).toBe(processNamespaceId() ?? null); }); - - it('still ages out a lock with no readable owner on the shorter clock', () => { - writeFileSync(lockFile(), 'not-json-at-all', 'utf8'); - const longAgo = new Date(Date.now() - 120_000); - utimesSync(lockFile(), longAgo, longAgo); - - upsertTaskState('ISSUE-LOCK-4', { execution: { status: 'todo', retryCount: 0 } }); - - expect(getTaskState('ISSUE-LOCK-4')?.execution.status).toBe('todo'); - }); }); }); diff --git a/src/taskState/store.ts b/src/taskState/store.ts index ee41a824..24a09ad8 100644 --- a/src/taskState/store.ts +++ b/src/taskState/store.ts @@ -20,8 +20,9 @@ import { homedir } from 'node:os'; import { randomUUID } from 'node:crypto'; import { z } from 'zod'; import type { TaskItem } from '../orchestration/decisionEngine.js'; -import { isProofCapableSpace, processAppearsAlive, processNamespaceId, sameProcessNamespace, writerProvablyGone } from '../support/processLiveness.js'; +import { processNamespaceId } from '../support/processLiveness.js'; import { getInstanceId } from '../support/healthEndpoint.js'; +import { acquireServiceInstanceLock, type ServiceInstanceLock } from '../support/serviceInstanceLock.js'; const TASK_STATE_MARKER = ''; @@ -110,10 +111,8 @@ type TaskStateStore = z.infer; let cache: TaskStateStore | null = null; let cacheStamp: string | null = null; -const LOCK_STALE_MS = 30_000; -const LOCK_ABANDON_MS = 600_000; const LOCK_WAIT_MS = 10; -const LOCK_TIMEOUT_MS = 5_000; +const DEFAULT_LOCK_TIMEOUT_MS = 5_000; const lockWaitBuffer = new Int32Array(new SharedArrayBuffer(4)); /** `ns`: the writer's pid space. A string identifies it; `null` records that @@ -135,24 +134,13 @@ function lockNamespaceOf(raw: unknown): string | null | undefined { * `finally`, so there is no way to observe the real thing after the fact — a * mutation dropping `instance` passed the whole suite until this existed. * - * `instance` is what lets a successor decide ownership without a clock; - * `ns` scopes the pid; `?? null` on the namespace is deliberate — an omitted - * key would be indistinguishable from a lock written before the field existed. + * `instance` / `ns` remain on the compatibility dot-lock payload for observability; + * mutual exclusion no longer depends on probing them (AGT-4024). */ export function buildLockPayload(token: string): StoreLockOwner { return { pid: process.pid, token, ns: processNamespaceId() ?? null, instance: getInstanceId() }; } -/** Whether this lock's pid can be probed from here at all. */ -function lockPidIsJudgeable(owner: StoreLockOwner): boolean { - // Absent keeps the original probe; null fails closed (the writer could name - // no space, so our pid table is not its pid table); a string must be ours. - // See the matching note in worktreeManager, including its AGT-4069 caveat. - if (owner.ns === undefined) return true; - if (owner.ns === null) return false; - return sameProcessNamespace(owner.ns); -} - function readStoreLockOwner(lockPath: string): StoreLockOwner | null { try { const value = JSON.parse(readFileSync(lockPath, 'utf8')) as Partial; @@ -173,6 +161,12 @@ function getStorePath(): string { return process.env.OPENSWARM_TASK_STATE_FILE || join(homedir(), '.openswarm', 'task-state.json'); } +/** Kernel-owned mutex for task-state writers (BEGIN IMMEDIATE). */ +function taskStateLockDbPath(): string { + return process.env.OPENSWARM_TASK_STATE_LOCK_DB + || join(homedir(), '.openswarm', 'task-state-lock.db'); +} + function ensureStoreLoaded(): TaskStateStore { const path = getStorePath(); const currentStamp = existsSync(path) @@ -213,120 +207,96 @@ export function resetTaskStateStoreForTests(): void { cacheStamp = null; } +/** + * Serialize mutations of task-state.json. + * + * Transition (AGT-4024): acquire BOTH the kernel-owned SQLite writer lock at + * `~/.openswarm/task-state-lock.db` (`BEGIN IMMEDIATE` via + * {@link acquireServiceInstanceLock}) and the legacy advisory dot-lock. + * Order is SQLite first, then the dot-lock, so a new binary (both locks) and + * an old CLI (dot-lock only) still exclude each other during version skew — + * the new binary holds the compatibility lock while the old waits for it, and + * vice versa. Release in reverse: drop the dot-lock, then ROLLBACK/close the + * SQLite transaction. + * + * SQLite's writer lock is kernel-owned and released on crash, so it needs no + * PID / age staleness heuristic. The dot-lock remains only for mixed-version + * mutual exclusion; a follow-up can drop it once the CLI floor has moved. + * While we hold the SQLite mutex, an abandoned compatibility lock (crash of a + * dual-lock holder, or an old CLI that exited without unlinking) is reclaimed + * after the wait deadline — no other new writer can be in the critical section. + */ function withStoreLock(operation: () => T): T { const path = getStorePath(); const directory = dirname(path); const lockPath = `${path}.lock`; mkdirSync(directory, { recursive: true }); - const deadline = Date.now() + LOCK_TIMEOUT_MS; + const lockTimeoutMs = Number(process.env.OPENSWARM_TASK_STATE_LOCK_TIMEOUT_MS) || DEFAULT_LOCK_TIMEOUT_MS; + const deadline = Date.now() + lockTimeoutMs; let lockFd: number | undefined; const lockToken = randomUUID(); + const sqliteLockPath = taskStateLockDbPath(); - while (lockFd === undefined) { + let sqliteLock: ServiceInstanceLock | undefined; + while (sqliteLock === undefined) { try { - lockFd = openSync(lockPath, 'wx', 0o600); - // `?? null` deliberately: an omitted key would be indistinguishable from - // a pre-field lock, which readers are entitled to probe locally. - writeFileSync(lockFd, JSON.stringify(buildLockPayload(lockToken)), 'utf8'); - fsyncSync(lockFd); + // Kernel-owned writer lock at ~/.openswarm/task-state-lock.db (or OPENSWARM_TASK_STATE_LOCK_DB). + sqliteLock = acquireServiceInstanceLock(sqliteLockPath); } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== 'EEXIST') throw error; - try { - const owner = readStoreLockOwner(lockPath); - const judgedMtimeMs = statSync(lockPath).mtimeMs; - const lockAgeMs = Date.now() - judgedMtimeMs; - const staleMalformedLock = !owner && lockAgeMs > LOCK_STALE_MS; - // A pid only means something inside the space it was issued in. This - // file can be a projection two containers share, so probing a lock - // that names a different space answers about whatever holds that - // number locally — and an ESRCH there would reclaim a lock a live - // writer still holds. Such a lock gets the age rule and nothing else. - // A lock with no space recorded predates the field and keeps the - // original probe behaviour. (Caught by the commit-gate review.) - const ownerPidIsJudgeable = owner !== null && lockPidIsJudgeable(owner); - const abandonedLock = owner !== null && ownerPidIsJudgeable && !processAppearsAlive(owner.pid); - // The pid probe above cannot see a generation change, so a lock the - // previous container left behind reads as held against the new daemon - // that inherited its pid. Settling that case outright, instead of - // waiting out LOCK_ABANDON_MS, is worth a full ten minutes of dead - // heartbeats after every restart. (AGT-4068) - // - // Gated on the recorded pid namespace, because that is the scope in - // which a pid is unique — and this file may be a mounted projection - // two containers share, each with its own pid 1. A lock from a - // different namespace, or one written before this field existed, is - // NOT reasoned about by pid: it falls through to the age rule below, - // which is exactly what the AGT-4023 policy test pins. - const priorGenerationLock = owner !== null - && isProofCapableSpace(owner.ns ?? undefined) && sameProcessNamespace(owner.ns ?? undefined) - && writerProvablyGone({ - pid: owner.pid, - ownerId: owner.instance, - ourOwnerId: getInstanceId(), - writtenAtMs: judgedMtimeMs, - }); - // A pid probe cannot see past its own namespace, and a container - // assigns the daemon the same pid every start — so a lock left behind - // by a killed container reads as "alive" against the new daemon - // itself, and nothing ever frees it (AGT-4023: 15 straight heartbeats - // dead, zero tasks for 75 minutes, manual rm the only exit). Age is - // the one signal that stays true across namespaces and generations. - // - // What this buys and what it costs. The guarded work is synchronous — - // read, parse, write, fsync, rename, measured at tens of milliseconds - // — and an out-of-space write fails fast rather than blocking, so - // reaching this threshold takes a frozen process (SIGSTOP, docker - // pause, uninterruptible I/O) or a wall-clock jump, since mtime is - // compared against a non-monotonic clock. A process frozen that long - // is serving nothing anyway. If one is evicted and later resumes, it - // overwrites with its own snapshot: a lost update, never a torn file - // (persistStore renames atomically) and never a cascade (the exit - // unlink is token-guarded). The run ledger — not this projection — - // owns leases and remote effects, so a rollback here cannot - // double-execute anything, and the next Linear sync re-derives it. - const expiredLock = lockAgeMs > LOCK_ABANDON_MS; - if (staleMalformedLock || abandonedLock || priorGenerationLock || expiredLock) { - // Reclaim only the lock that was actually judged. Between the - // judgement above and this unlink the holder can release and a third - // process can take a fresh lock; deleting THAT one would put two - // writers in the store. Re-reading identity here narrows the window - // to a pair of syscalls — it does not close it, because POSIX has no - // compare-and-unlink for a regular file. Closing it needs a - // kernel-owned mutex (AGT-4024). - const currentMtimeMs = statSync(lockPath).mtimeMs; - const currentOwner = readStoreLockOwner(lockPath); - const sameLock = currentMtimeMs === judgedMtimeMs - && currentOwner?.token === owner?.token; - if (sameLock) unlinkSync(lockPath); - continue; - } - } catch (statError) { - if ((statError as NodeJS.ErrnoException).code === 'ENOENT') continue; - throw statError; - } + const busy = error instanceof Error && /owns the instance lock/i.test(error.message); + if (!busy) throw error; if (Date.now() >= deadline) { - throw new Error(`Timed out waiting for task state lock: ${lockPath}`); + throw new Error(`Timed out waiting for task state SQLite lock: ${sqliteLockPath}`, { cause: error }); } Atomics.wait(lockWaitBuffer, 0, 0, LOCK_WAIT_MS); } } try { - // Another process may have committed since this process populated cache. - cache = null; - return operation(); - } finally { - closeSync(lockFd); + while (lockFd === undefined) { + try { + lockFd = openSync(lockPath, 'wx', 0o600); + // `?? null` deliberately: an omitted key would be indistinguishable from + // a pre-field lock. Payload is observational; exclusion is SQLite-owned. + writeFileSync(lockFd, JSON.stringify(buildLockPayload(lockToken)), 'utf8'); + fsyncSync(lockFd); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== 'EEXIST') throw error; + // Wait for an old CLI (or a peer finishing unlink). After the deadline, + // reclaim the compatibility lock: we already hold the kernel SQLite + // mutex, so no other new binary is in the critical section. + if (Date.now() >= deadline) { + try { + unlinkSync(lockPath); + } catch (unlinkError) { + if ((unlinkError as NodeJS.ErrnoException).code !== 'ENOENT') throw unlinkError; + } + continue; + } + Atomics.wait(lockWaitBuffer, 0, 0, LOCK_WAIT_MS); + } + } + try { - // Only the process/token that created the current path may unlink it. If - // an operator or recovery path replaced the lock, leave the replacement. - if (readStoreLockOwner(lockPath)?.token === lockToken) unlinkSync(lockPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { - console.warn(`[TaskState] Failed to remove lock ${lockPath}: ${error instanceof Error ? error.message : String(error)}`); + // Another process may have committed since this process populated cache. + cache = null; + return operation(); + } finally { + closeSync(lockFd); + try { + // Only the process/token that created the current path may unlink it. If + // an operator or recovery path replaced the lock, leave the replacement. + if (readStoreLockOwner(lockPath)?.token === lockToken) unlinkSync(lockPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + console.warn(`[TaskState] Failed to remove lock ${lockPath}: ${error instanceof Error ? error.message : String(error)}`); + } } } + } finally { + // Reverse of acquisition: dot-lock already dropped above; release SQLite last. + sqliteLock.release(); } }