diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de8624..94ad1c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.7.2 - Unreleased +- Reclaimed dead same-host review locks automatically and added `clean-locks --stale-only` for safe scripted cleanup while preserving live and remote locks, thanks @goutamadwant. - Fixed diff-scoped review and CI runs to include changed features regardless of their previous review status, preventing warm-state gates from silently skipping changed code, thanks @youhaowei. - Updated pnpm, Node typings, formatter and linter tooling, and security workflow actions. - Added Rust seed context for Cargo manifests, paired crate entrypoints, and directly declared modules across crate roots and binary layouts, thanks @Tanmay-008. diff --git a/README.md b/README.md index 2d442ad..673133e 100644 --- a/README.md +++ b/README.md @@ -151,7 +151,7 @@ Supported provider names today: - `clawpatch revalidate --finding `: re-check one finding - `clawpatch revalidate --all`: re-check open findings with report-style filters - `clawpatch doctor`: check provider availability -- `clawpatch clean-locks`: clear feature locks +- `clawpatch clean-locks`: clear feature locks; add `--stale-only` to reclaim only dead local locks Useful flags: diff --git a/docs/code-review.md b/docs/code-review.md index 138c655..1c6fac9 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -81,9 +81,11 @@ appends a compact summary to `GITHUB_STEP_SUMMARY` when that file is available. Progress uses stderr so `--json` stdout remains machine-readable. The worker pool is per-process, and lock files under `.clawpatch/locks/` prevent -overlapping review processes from claiming the same feature. Interrupted runs -can leave recoverable lock files; clear them with `clawpatch clean-locks` after -confirming no review process is still active. `clawpatch status` includes both +overlapping review processes from claiming the same feature. Interrupted local +runs with dead process IDs are reclaimed automatically on the next claim. Use +`clawpatch clean-locks --stale-only` for conservative cleanup that preserves live +local locks and locks from other hosts; the unfiltered `clean-locks` command still +requires confirming that no review process is active. `clawpatch status` includes both feature-record locks and lock files in `activeLocks`, and reports the lock-file count as `lockFiles`. diff --git a/docs/safety.md b/docs/safety.md index d2c8a23..ee5bb40 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -20,7 +20,9 @@ Current safety rules: strictly sandboxed. See docs/providers.md. - provider output must pass runtime schema validation. - feature locks are stored in feature records and `.clawpatch/locks/`; `status` - surfaces both, and `clean-locks` clears both. + surfaces both. Claims automatically reclaim same-host locks whose process is + dead, and `clean-locks --stale-only` applies the same conservative rule without + clearing live local or remote locks. - the mapper skips symlinked directories and common generated directories. Not implemented today: diff --git a/package.json b/package.json index bb1f987..38a6bfb 100644 --- a/package.json +++ b/package.json @@ -31,10 +31,12 @@ "crabbox:warmup": "crabbox warmup" }, "dependencies": { + "proper-lockfile": "^4.1.2", "zod": "^4.4.3" }, "devDependencies": { "@types/node": "^26.1.2", + "@types/proper-lockfile": "^4.1.4", "oxfmt": "^0.61.0", "oxlint": "^1.76.0", "typescript": "^7.0.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6b07216..9c064bf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11,6 +11,9 @@ importers: .: dependencies: + proper-lockfile: + specifier: ^4.1.2 + version: 4.1.2 zod: specifier: ^4.4.3 version: 4.4.3 @@ -18,6 +21,9 @@ importers: '@types/node': specifier: ^26.1.2 version: 26.1.2 + '@types/proper-lockfile': + specifier: ^4.1.4 + version: 4.1.4 oxfmt: specifier: ^0.61.0 version: 0.61.0 @@ -415,6 +421,12 @@ packages: '@types/node@26.1.2': resolution: {integrity: sha512-Vu4a5UFA9rIIFJ7rB/Vaafh9lrCQszopTCx6KjFboXTGQbPNasehVR5TEiithSDGyd1DEiUByggTZsg8jukeIg==} + '@types/proper-lockfile@4.1.4': + resolution: {integrity: sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ==} + + '@types/retry@0.12.5': + resolution: {integrity: sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw==} + '@typescript/typescript-aix-ppc64@7.0.2': resolution: {integrity: sha512-MTKKkWB7p/0E9xi1d1tHtZ5PiLkGEMIq88pK2CubZjOsLtYTLqhgIgi6zepFa+9GHZ6h05NMCkQxGKiPXMxXtQ==} engines: {node: '>=16.20.0'} @@ -603,6 +615,9 @@ packages: engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} os: [darwin] + graceful-fs@4.2.11: + resolution: {integrity: sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==} + lightningcss-android-arm64@1.33.0: resolution: {integrity: sha512-gEpRTalKdosp4Bb8qWtc2iOgE5SeIHlpS1up9bFq2wAyYhl1UdTObYiHe98zEM9SQvSoqQZ1IQD0JNpg3Ml5pg==} engines: {node: '>= 12.0.0'} @@ -729,6 +744,13 @@ packages: resolution: {integrity: sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==} engines: {node: ^10 || ^12 || >=14} + proper-lockfile@4.1.2: + resolution: {integrity: sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==} + + retry@0.12.0: + resolution: {integrity: sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==} + engines: {node: '>= 4'} + rolldown@1.0.3: resolution: {integrity: sha512-i00lAJ2ks1BYr7rjNjKC7BcqAS7nVfiT3QX1SI5aY+AFHblCmaUf9OE9dbdzDvW6dJxbi2ZCZiy9v3CcwOiX3g==} engines: {node: ^20.19.0 || >=22.12.0} @@ -737,6 +759,9 @@ packages: siginfo@2.0.0: resolution: {integrity: sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g==} + signal-exit@3.0.7: + resolution: {integrity: sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==} + source-map-js@1.2.1: resolution: {integrity: sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==} engines: {node: '>=0.10.0'} @@ -1083,6 +1108,12 @@ snapshots: dependencies: undici-types: 8.3.0 + '@types/proper-lockfile@4.1.4': + dependencies: + '@types/retry': 0.12.5 + + '@types/retry@0.12.5': {} + '@typescript/typescript-aix-ppc64@7.0.2': optional: true @@ -1207,6 +1238,8 @@ snapshots: fsevents@2.3.3: optional: true + graceful-fs@4.2.11: {} + lightningcss-android-arm64@1.33.0: optional: true @@ -1322,6 +1355,14 @@ snapshots: picocolors: 1.1.1 source-map-js: 1.2.1 + proper-lockfile@4.1.2: + dependencies: + graceful-fs: 4.2.11 + retry: 0.12.0 + signal-exit: 3.0.7 + + retry@0.12.0: {} + rolldown@1.0.3: dependencies: '@oxc-project/types': 0.133.0 @@ -1345,6 +1386,8 @@ snapshots: siginfo@2.0.0: {} + signal-exit@3.0.7: {} + source-map-js@1.2.1: {} stackback@0.0.2: {} diff --git a/scripts/package-smoke.mjs b/scripts/package-smoke.mjs index 73c8c87..e24e3b4 100644 --- a/scripts/package-smoke.mjs +++ b/scripts/package-smoke.mjs @@ -6,7 +6,6 @@ import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; import { pathToFileURL } from "node:url"; -const moduleRequire = createRequire(import.meta.url); const root = process.cwd(); function createSmokeContext() { @@ -251,10 +250,24 @@ function packPath(destination, output) { } function runtimeDependencyPaths(rootPath = root) { - const packageJson = JSON.parse(readFileSync(join(rootPath, "package.json"), "utf8")); - return runtimeDependencyNames(packageJson).map((name) => - dirname(moduleRequire.resolve(`${name}/package.json`)), - ); + const dependencyPaths = new Map(); + + function collect(packageJsonPath, packageRequire) { + const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")); + for (const name of runtimeDependencyNames(packageJson)) { + const dependencyPackageJson = packageRequire.resolve(`${name}/package.json`); + const dependencyPath = dirname(dependencyPackageJson); + if (dependencyPaths.has(dependencyPath)) { + continue; + } + dependencyPaths.set(dependencyPath, dependencyPath); + collect(dependencyPackageJson, createRequire(dependencyPackageJson)); + } + } + + const packageJsonPath = join(rootPath, "package.json"); + collect(packageJsonPath, createRequire(packageJsonPath)); + return [...dependencyPaths.values()]; } function runtimeDependencyNames(packageJson) { @@ -331,4 +344,5 @@ export const packageSmokeTestHooks = { installArgs, packDependencyArgs, runtimeDependencyNames, + runtimeDependencyPaths, }; diff --git a/src/app.ts b/src/app.ts index 474f9a6..757cda8 100644 --- a/src/app.ts +++ b/src/app.ts @@ -12,6 +12,7 @@ import { emitProgress } from "./progress.js"; import { providerByName } from "./provider.js"; import { clearFeatureLockFiles, + clearStaleFeatureLocks, ensureStateDirs, readFeatures, readFeatureLockIds, @@ -266,8 +267,15 @@ export async function doctorCommand( }; } -export async function cleanLocksCommand(context: AppContext): Promise { +export async function cleanLocksCommand( + context: AppContext, + flags: Record = {}, +): Promise { const loaded = await loadProjectState(context); + if (flags["staleOnly"] === true) { + const cleared = await clearStaleFeatureLocks(loaded.paths); + return { cleared: cleared.featuresCleared, lockFilesCleared: cleared.lockFilesCleared }; + } const features = await readFeatures(loaded.paths); let cleared = 0; for (const feature of features) { diff --git a/src/cli.ts b/src/cli.ts index 0310bec..a6da8f0 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -249,7 +249,7 @@ const commandSpecs = { run: doctorCommand, }, "clean-locks": { - flags: [], + flags: ["staleOnly"], usage: ["clawpatch clean-locks [flags]"], run: cleanLocksCommand, }, @@ -383,6 +383,12 @@ const optionSpecs: Record = { force: { name: "force", kind: "boolean", target: "command", help: " --force" }, all: { name: "all", kind: "boolean", target: "command", help: " --all" }, draft: { name: "draft", kind: "boolean", target: "command", help: " --draft" }, + "stale-only": { + name: "staleOnly", + kind: "boolean", + target: "command", + help: " --stale-only", + }, "include-dirty": { name: "includeDirty", kind: "boolean", diff --git a/src/package-smoke.test.ts b/src/package-smoke.test.ts index 39c9c65..dcb6518 100644 --- a/src/package-smoke.test.ts +++ b/src/package-smoke.test.ts @@ -1,3 +1,4 @@ +import { readFileSync } from "node:fs"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; import { describe, expect, it } from "vitest"; @@ -16,6 +17,7 @@ type PackageSmokeTesting = { npmCache: string; }): string[]; runtimeDependencyNames(packageJson: { dependencies?: Record }): string[]; + runtimeDependencyPaths(rootPath?: string): string[]; }; }; @@ -37,6 +39,16 @@ describe("package smoke harness", () => { }); expect(dependencyNames).toEqual(["zod"]); + const packedDependencyNames = smoke.runtimeDependencyPaths().map((dependencyPath) => { + const packageJson = JSON.parse( + readFileSync(join(dependencyPath, "package.json"), "utf8"), + ) as { name: string }; + return packageJson.name; + }); + expect(packedDependencyNames).toEqual( + expect.arrayContaining(["proper-lockfile", "graceful-fs", "retry", "signal-exit", "zod"]), + ); + const packArgs = smoke.packDependencyArgs({ dependencyPath: dependencySource, destination: "/tmp", diff --git a/src/state.ts b/src/state.ts index 9e9a21b..b800a52 100644 --- a/src/state.ts +++ b/src/state.ts @@ -1,5 +1,7 @@ import { open, readdir, unlink } from "node:fs/promises"; +import { hostname as osHostname } from "node:os"; import { join } from "node:path"; +import lockfile from "proper-lockfile"; import { z } from "zod"; import { ClawpatchError } from "./errors.js"; import { ensureDir, nowIso, pathExists, readJson, writeJson } from "./fs.js"; @@ -10,6 +12,7 @@ import { ProjectRecord, RunRecord, featureRecordSchema, + featureLockSchema, findingRecordSchema, patchAttemptSchema, projectRecordSchema, @@ -28,6 +31,18 @@ export type StatePaths = { locks: string; }; +type FeatureLock = NonNullable; + +export type FeatureLockReclaimOptions = { + hostname?: string; + isPidAlive?: (pid: number) => boolean; +}; + +type ClaimFeatureOptions = { + allowNonPending?: boolean; + staleLock?: FeatureLockReclaimOptions; +}; + export function statePaths(stateDir: string): StatePaths { return { stateDir, @@ -84,34 +99,60 @@ export async function writeFeature(paths: StatePaths, feature: FeatureRecord): P export async function claimFeature( paths: StatePaths, featureId: string, - lock: NonNullable, - options: { allowNonPending?: boolean } = {}, + lock: FeatureLock, + options: ClaimFeatureOptions = {}, ): Promise { await ensureDir(paths.locks); + return withFeatureLockMutation(paths, featureId, () => + claimFeatureUnderMutationLock(paths, featureId, lock, options), + ); +} + +async function claimFeatureUnderMutationLock( + paths: StatePaths, + featureId: string, + lock: FeatureLock, + options: ClaimFeatureOptions, +): Promise { const lockPath = featureLockPath(paths, featureId); - let handle; - try { - handle = await open(lockPath, "wx"); - await handle.writeFile(`${JSON.stringify(lock, null, 2)}\n`, "utf8"); - } catch (error: unknown) { - if (isNodeError(error, "EEXIST")) { - throw new ClawpatchError(`feature locked: ${featureId}`, 7, "lock-conflict"); - } - if (handle !== undefined) { - await handle.close(); - handle = undefined; - await releaseFeatureLock(paths, featureId); + let lockFileCreated = false; + for (let attempt = 0; attempt < 2; attempt += 1) { + let handle; + try { + handle = await open(lockPath, "wx"); + await handle.writeFile(`${JSON.stringify(lock, null, 2)}\n`, "utf8"); + lockFileCreated = true; + break; + } catch (error: unknown) { + if (isNodeError(error, "EEXIST")) { + if (attempt === 0 && (await reclaimStaleFeatureLock(paths, featureId, options.staleLock))) { + continue; + } + throw new ClawpatchError(`feature locked: ${featureId}`, 7, "lock-conflict"); + } + if (handle !== undefined) { + await handle.close(); + handle = undefined; + await releaseFeatureLock(paths, featureId); + } + throw error; + } finally { + await handle?.close(); } - throw error; - } finally { - await handle?.close(); + } + if (!lockFileCreated) { + throw new ClawpatchError(`feature locked: ${featureId}`, 7, "lock-conflict"); } try { - const feature = await readFeature(paths, featureId); + let feature = await readFeature(paths, featureId); if (feature === null) { throw new ClawpatchError(`feature not found: ${featureId}`, 2, "feature-not-found"); } + if (feature.lock !== null && isStaleLocalFeatureLock(feature.lock, options.staleLock)) { + feature = clearFeatureRecordLock(feature); + await writeFeature(paths, feature); + } if (feature.lock !== null) { throw new ClawpatchError(`feature locked: ${featureId}`, 7, "lock-conflict"); } @@ -133,11 +174,19 @@ export async function claimFeature( } export async function releaseFeatureLock(paths: StatePaths, featureId: string): Promise { - await unlink(featureLockPath(paths, featureId)).catch((error: unknown) => { + await deleteFeatureLockFile(paths, featureId); +} + +async function deleteFeatureLockFile(paths: StatePaths, featureId: string): Promise { + try { + await unlink(featureLockPath(paths, featureId)); + return true; + } catch (error: unknown) { if (!isNodeError(error, "ENOENT")) { throw error; } - }); + return false; + } } export async function clearFeatureLockFiles(paths: StatePaths): Promise { @@ -148,6 +197,27 @@ export async function clearFeatureLockFiles(paths: StatePaths): Promise return lockIds.length; } +export async function clearStaleFeatureLocks( + paths: StatePaths, + options: FeatureLockReclaimOptions = {}, +): Promise<{ featuresCleared: number; lockFilesCleared: number }> { + const features = await readFeatures(paths); + const featureIds = new Set([ + ...features.map((feature) => feature.featureId), + ...(await readFeatureLockIds(paths)), + ]); + let featuresCleared = 0; + let lockFilesCleared = 0; + for (const featureId of featureIds) { + const cleared = await withFeatureLockMutation(paths, featureId, () => + clearStaleFeatureLockUnderMutationLock(paths, featureId, options), + ); + featuresCleared += cleared.featureCleared ? 1 : 0; + lockFilesCleared += cleared.lockFileCleared ? 1 : 0; + } + return { featuresCleared, lockFilesCleared }; +} + export async function readFeatureLockIds(paths: StatePaths): Promise { if (!(await pathExists(paths.locks))) { return []; @@ -221,6 +291,129 @@ function recordPath(directory: string, id: string): string { return join(directory, `${id}.json`); } +async function reclaimStaleFeatureLock( + paths: StatePaths, + featureId: string, + options: FeatureLockReclaimOptions = {}, +): Promise { + const [feature, fileLock] = await Promise.all([ + readFeature(paths, featureId), + readFeatureLockFile(paths, featureId), + ]); + const featureLock = feature?.lock ?? null; + if (featureLock === null && fileLock === null) { + return false; + } + if (featureLock !== null && !isStaleLocalFeatureLock(featureLock, options)) { + return false; + } + if (fileLock !== null && !isStaleLocalFeatureLock(fileLock, options)) { + return false; + } + if (featureLock !== null && feature !== null) { + await writeFeature(paths, clearFeatureRecordLock(feature)); + } + if (fileLock !== null) { + await deleteFeatureLockFile(paths, featureId); + } + return true; +} + +async function clearStaleFeatureLockUnderMutationLock( + paths: StatePaths, + featureId: string, + options: FeatureLockReclaimOptions, +): Promise<{ featureCleared: boolean; lockFileCleared: boolean }> { + const [feature, fileLock] = await Promise.all([ + readFeature(paths, featureId), + readFeatureLockFile(paths, featureId), + ]); + const featureLock = feature?.lock ?? null; + if (featureLock !== null && !isStaleLocalFeatureLock(featureLock, options)) { + return { featureCleared: false, lockFileCleared: false }; + } + if (fileLock !== null && !isStaleLocalFeatureLock(fileLock, options)) { + return { featureCleared: false, lockFileCleared: false }; + } + if (featureLock !== null && feature !== null) { + await writeFeature(paths, clearFeatureRecordLock(feature)); + } + const lockFileCleared = fileLock !== null && (await deleteFeatureLockFile(paths, featureId)); + return { featureCleared: featureLock !== null, lockFileCleared }; +} + +async function withFeatureLockMutation( + paths: StatePaths, + featureId: string, + operation: () => Promise, +): Promise { + await ensureDir(paths.locks); + const target = featureLockPath(paths, featureId); + let release: (() => Promise) | undefined; + try { + release = await lockfile.lock(target, { + realpath: false, + stale: 5_000, + update: 1_000, + retries: { retries: 10, factor: 1.5, minTimeout: 10, maxTimeout: 100 }, + }); + } catch (error: unknown) { + if (isNodeError(error, "ELOCKED")) { + throw new ClawpatchError(`feature locked: ${featureId}`, 7, "lock-conflict"); + } + throw error; + } + try { + return await operation(); + } finally { + await release(); + } +} + +function clearFeatureRecordLock(feature: FeatureRecord): FeatureRecord { + return { + ...feature, + status: feature.status === "claimed" ? "pending" : feature.status, + lock: null, + updatedAt: nowIso(), + }; +} + +async function readFeatureLockFile( + paths: StatePaths, + featureId: string, +): Promise { + try { + return await readJson(featureLockPath(paths, featureId), featureLockSchema); + } catch { + return null; + } +} + +export function isStaleLocalFeatureLock( + lock: FeatureLock, + options: FeatureLockReclaimOptions = {}, +): boolean { + const currentHostname = options.hostname ?? osHostname(); + const isPidAlive = options.isPidAlive ?? defaultIsPidAlive; + return lock.hostname === currentHostname && !isPidAlive(lock.pid); +} + +function defaultIsPidAlive(pid: number): boolean { + if (!Number.isInteger(pid) || pid <= 0) { + return false; + } + try { + process.kill(pid, 0); + return true; + } catch (error: unknown) { + if (isNodeError(error, "ESRCH")) { + return false; + } + return true; + } +} + function isNodeError(error: unknown, code: string): error is NodeJS.ErrnoException { return error instanceof Error && "code" in error && error.code === code; } diff --git a/src/workflow.test.ts b/src/workflow.test.ts index f9db138..3dec4bc 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -11,7 +11,10 @@ import { symlink, unlink, } from "node:fs/promises"; +import { hostname as osHostname } from "node:os"; import { delimiter, join } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import lockfile from "proper-lockfile"; import { fixCommand, cleanLocksCommand, @@ -259,6 +262,9 @@ describe("workflow", () => { expect(() => parseArgs(["--dry-run", "clean-locks"])).toThrow( "unsupported flag for clean-locks: --dry-run", ); + expect(parseArgs(["clean-locks", "--stale-only"]).flags).toMatchObject({ + staleOnly: true, + }); expect(parseArgs(["map", "--dry-run"]).flags).toMatchObject({ dryRun: true, }); @@ -1687,6 +1693,239 @@ describe("workflow", () => { writeFileSpy.mockRestore(); }); + it("reclaims dead local feature locks before claiming", async () => { + const root = await fixtureRoot("clawpatch-dead-local-lock-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "dead-local-lock", bin: { stale: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 1;\n"); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + + await initCommand(context, {}); + await mapCommand(context); + const feature = (await readFeatures(paths)).find((candidate) => + candidate.title.includes("CLI command"), + ); + expect(feature).toBeDefined(); + const staleLock = { + lockedByRunId: "interrupted", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 100, + }; + const nextLock = { + lockedByRunId: "run-next", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 101, + }; + await writeFeature(paths, { + ...feature!, + status: "claimed", + lock: staleLock, + updatedAt: new Date().toISOString(), + }); + await writeFixture( + root, + `.clawpatch/locks/${feature!.featureId}.json`, + `${JSON.stringify(staleLock, null, 2)}\n`, + ); + + const claimed = await claimFeature(paths, feature!.featureId, nextLock, { + staleLock: { hostname: "local-host", isPidAlive: () => false }, + }); + + expect(claimed.status).toBe("claimed"); + expect(claimed.lock).toMatchObject({ lockedByRunId: "run-next" }); + expect(await readdir(paths.locks)).toEqual([`${feature!.featureId}.json`]); + }); + + it("serializes stale reclamation with a replacement live lock", async () => { + const root = await fixtureRoot("clawpatch-stale-lock-replacement-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ name: "stale-lock-replacement", bin: { stale: "src/index.ts" } }), + ); + await writeFixture(root, "src/index.ts", "export const value = 1;\n"); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + + await initCommand(context, {}); + await mapCommand(context); + const feature = (await readFeatures(paths)).find((candidate) => + candidate.title.includes("CLI command"), + ); + expect(feature).toBeDefined(); + const staleLock = { + lockedByRunId: "interrupted", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 100, + }; + const replacementLock = { + lockedByRunId: "replacement", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 101, + }; + await writeFeature(paths, { + ...feature!, + status: "claimed", + lock: staleLock, + updatedAt: new Date().toISOString(), + }); + const lockPath = join(paths.locks, `${feature!.featureId}.json`); + await writeFixture( + root, + `.clawpatch/locks/${feature!.featureId}.json`, + `${JSON.stringify(staleLock)}\n`, + ); + const releaseMutationLock = await lockfile.lock(lockPath, { + realpath: false, + stale: 5_000, + update: 1_000, + }); + let claimSettled = false; + const pendingClaim = claimFeature( + paths, + feature!.featureId, + { + lockedByRunId: "delayed-reclaimer", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 102, + }, + { + staleLock: { + hostname: "local-host", + isPidAlive: (pid) => pid === replacementLock.pid, + }, + }, + ); + void pendingClaim.then( + () => { + claimSettled = true; + }, + () => { + claimSettled = true; + }, + ); + await delay(25); + expect(claimSettled).toBe(false); + + await writeFeature(paths, { + ...feature!, + status: "claimed", + lock: replacementLock, + updatedAt: new Date().toISOString(), + }); + await writeFixture( + root, + `.clawpatch/locks/${feature!.featureId}.json`, + `${JSON.stringify(replacementLock)}\n`, + ); + await releaseMutationLock(); + + await expect(pendingClaim).rejects.toMatchObject({ code: "lock-conflict" }); + expect( + (await readFeatures(paths)).find((item) => item.featureId === feature!.featureId)?.lock, + ).toMatchObject({ + lockedByRunId: "replacement", + }); + expect(JSON.parse(await readFile(lockPath, "utf8"))).toMatchObject({ + lockedByRunId: "replacement", + }); + }); + + it("keeps live local and remote feature locks claimed", async () => { + const root = await fixtureRoot("clawpatch-live-remote-locks-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ + name: "live-remote-locks", + bin: { live: "src/live.ts", remote: "src/remote.ts" }, + }), + ); + await writeFixture(root, "src/live.ts", "export const live = 1;\n"); + await writeFixture(root, "src/remote.ts", "export const remote = 1;\n"); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + + await initCommand(context, {}); + await mapCommand(context); + const features = (await readFeatures(paths)).filter((candidate) => + candidate.title.includes("CLI command"), + ); + expect(features).toHaveLength(2); + const liveLock = { + lockedByRunId: "live-run", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 100, + }; + const remoteLock = { + lockedByRunId: "remote-run", + lockedAt: new Date().toISOString(), + hostname: "remote-host", + pid: 100, + }; + for (const [feature, lock] of [ + [features[0]!, liveLock], + [features[1]!, remoteLock], + ] as const) { + await writeFeature(paths, { + ...feature, + status: "claimed", + lock, + updatedAt: new Date().toISOString(), + }); + await writeFixture( + root, + `.clawpatch/locks/${feature.featureId}.json`, + `${JSON.stringify(lock, null, 2)}\n`, + ); + } + + await expect( + claimFeature( + paths, + features[0]!.featureId, + { + lockedByRunId: "next-live", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 101, + }, + { + staleLock: { hostname: "local-host", isPidAlive: () => true }, + }, + ), + ).rejects.toMatchObject({ code: "lock-conflict" }); + await expect( + claimFeature( + paths, + features[1]!.featureId, + { + lockedByRunId: "next-remote", + lockedAt: new Date().toISOString(), + hostname: "local-host", + pid: 101, + }, + { + staleLock: { hostname: "local-host", isPidAlive: () => false }, + }, + ), + ).rejects.toMatchObject({ code: "lock-conflict" }); + expect(await readdir(paths.locks)).toEqual( + features.map((feature) => `${feature.featureId}.json`).toSorted(), + ); + }); + it("does not claim a stale feature after another run finishes it", async () => { const root = await fixtureRoot("clawpatch-stale-lock-"); await writeFixture( @@ -2484,6 +2723,93 @@ describe("workflow", () => { expect(await readdir(paths.locks)).toEqual([]); }); + it("clean-locks --stale-only clears only dead local locks", async () => { + const root = await fixtureRoot("clawpatch-clean-stale-locks-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ + name: "clean-stale-locks", + bin: { + stale: "src/stale.ts", + live: "src/live.ts", + remote: "src/remote.ts", + }, + }), + ); + await writeFixture(root, "src/stale.ts", "export const stale = 1;\n"); + await writeFixture(root, "src/live.ts", "export const live = 1;\n"); + await writeFixture(root, "src/remote.ts", "export const remote = 1;\n"); + const context = await makeContext(testOptions(root)); + const paths = statePaths(join(root, ".clawpatch")); + + await initCommand(context, {}); + await mapCommand(context); + const features = (await readFeatures(paths)).filter((candidate) => + candidate.title.includes("CLI command"), + ); + expect(features).toHaveLength(3); + const locks = [ + { + lockedByRunId: "stale-run", + lockedAt: new Date().toISOString(), + hostname: osHostname(), + pid: 0, + }, + { + lockedByRunId: "live-run", + lockedAt: new Date().toISOString(), + hostname: osHostname(), + pid: process.pid, + }, + { + lockedByRunId: "remote-run", + lockedAt: new Date().toISOString(), + hostname: "remote-host", + pid: 0, + }, + ]; + for (const [index, feature] of features.entries()) { + const lock = locks[index]!; + await writeFeature(paths, { + ...feature, + status: "claimed", + lock, + updatedAt: new Date().toISOString(), + }); + await writeFixture( + root, + `.clawpatch/locks/${feature.featureId}.json`, + `${JSON.stringify(lock, null, 2)}\n`, + ); + } + + const result = await cleanLocksCommand(context, { staleOnly: true }); + const cleaned = await readFeatures(paths); + + expect(result).toMatchObject({ cleared: 1, lockFilesCleared: 1 }); + expect(cleaned.find((feature) => feature.featureId === features[0]!.featureId)).toMatchObject({ + status: "pending", + lock: null, + }); + expect( + cleaned.find((feature) => feature.featureId === features[1]!.featureId)?.lock, + ).toMatchObject({ + lockedByRunId: "live-run", + }); + expect( + cleaned.find((feature) => feature.featureId === features[2]!.featureId)?.lock, + ).toMatchObject({ + lockedByRunId: "remote-run", + }); + expect(await readdir(paths.locks)).toEqual( + features + .slice(1) + .map((feature) => `${feature.featureId}.json`) + .toSorted(), + ); + }); + it("surfaces crash-window lock files in status", async () => { const root = await fixtureRoot("clawpatch-file-lock-status-"); await writeFixture(