From 227b76695e40ea662a070f0a39616870f4a6e7cd Mon Sep 17 00:00:00 2001 From: Alexander Ivanov Date: Fri, 11 Sep 2026 20:32:20 +0300 Subject: [PATCH] A lease says who The workspace lease records the git identity of the working directory that took it, and the CLI can be asked who holds a workspace without attempting to start a run and reading the refusal. Attribution, never authentication. Anybody can set user.email to anything, so the value is called "git author" wherever it is shown and nothing is permitted or refused on the strength of it. It is read once where a host starts up, never in the heartbeat, which renews every five seconds for a value that cannot change during a run. A lease taken where no identity is configured is valid and records none, exactly like every lease written before this. "lease release" clears a lease only where it can establish the holder is gone: the heartbeat is already stale, or the holder is on this machine and its process is not running, checked with a signal that delivers nothing. There is deliberately no --force. A holder that died already self-heals once its heartbeat goes stale; a holder that is alive still has the workspace open, and taking its lease would permit a second mutating run against files it is still holding, which is what the lease exists to prevent. A stuck holder is stopped, not robbed. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/a-lease-says-who.md | 31 ++++ .../changes/a-lease-says-who/.openspec.yaml | 5 + openspec/changes/a-lease-says-who/design.md | 92 ++++++++++++ openspec/changes/a-lease-says-who/proposal.md | 63 ++++++++ .../a-lease-says-who/specs/ci-cli/spec.md | 86 +++++++++++ openspec/changes/a-lease-says-who/tasks.md | 99 +++++++++++++ packages/cli/src/lease-command.test.ts | 118 +++++++++++++++ packages/cli/src/lease-command.ts | 121 +++++++++++++++ packages/cli/src/main.test.ts | 31 ++++ packages/cli/src/main.ts | 27 +++- packages/cli/src/run-change.ts | 7 +- packages/core/src/git.ts | 32 ++++ .../core/src/harness-chain-runner.test.ts | 1 + packages/core/src/index.ts | 1 + packages/core/src/lease-release.test.ts | 139 ++++++++++++++++++ packages/core/src/lease-release.ts | 103 +++++++++++++ packages/core/src/workbench-recovery.ts | 10 +- packages/core/src/workspace-lease.test.ts | 42 ++++++ packages/core/src/workspace-lease.ts | 47 +++++- packages/extension/src/extension.ts | 8 +- 20 files changed, 1053 insertions(+), 10 deletions(-) create mode 100644 .changeset/a-lease-says-who.md create mode 100644 openspec/changes/a-lease-says-who/.openspec.yaml create mode 100644 openspec/changes/a-lease-says-who/design.md create mode 100644 openspec/changes/a-lease-says-who/proposal.md create mode 100644 openspec/changes/a-lease-says-who/specs/ci-cli/spec.md create mode 100644 openspec/changes/a-lease-says-who/tasks.md create mode 100644 packages/cli/src/lease-command.test.ts create mode 100644 packages/cli/src/lease-command.ts create mode 100644 packages/core/src/lease-release.test.ts create mode 100644 packages/core/src/lease-release.ts diff --git a/.changeset/a-lease-says-who.md b/.changeset/a-lease-says-who.md new file mode 100644 index 00000000..bba7f2df --- /dev/null +++ b/.changeset/a-lease-says-who.md @@ -0,0 +1,31 @@ +--- +"@openspec-ui/core": minor +"@openspec-ui/cli": minor +"openspec-ui-vscode": patch +--- + +A workspace lease says who took it, and can be asked about. + +The lease records the git identity of the working directory that took it +— `user.email`, falling back to `user.name`. It is attribution, never +authentication: anybody can set that value to anything, so it is called +"git author" wherever it is shown and nothing is permitted or refused on +the strength of it. A lease taken where no identity is configured is +valid and records none, exactly like every lease written before this. + +It is read once where a host starts up, never in the heartbeat — that +renews every five seconds, and the value cannot change during a run. + +`openspec-ui-cli lease` answers who holds a workspace without trying to +start a run and reading the refusal, which was the only way to ask +before. It exits `0` held or free: the question was answered either way. + +`openspec-ui-cli lease release` clears a lease only where it can +establish that the holder is gone — the heartbeat is already stale, or +the holder is on this machine and its process is not running, checked +with a signal that delivers nothing. There is deliberately no `--force`. +A holder that died already self-heals once its heartbeat goes stale; a +holder that is alive still has the workspace open, and taking its lease +would permit a second mutating run against files it is still holding, +which is what the lease exists to prevent. A stuck holder is stopped, +not robbed. diff --git a/openspec/changes/a-lease-says-who/.openspec.yaml b/openspec/changes/a-lease-says-who/.openspec.yaml new file mode 100644 index 00000000..67232720 --- /dev/null +++ b/openspec/changes/a-lease-says-who/.openspec.yaml @@ -0,0 +1,5 @@ +schema: spec-driven +created: 2026-09-11 +follows: + - cross-host-workspace-lease + - changes-run-side-by-side diff --git a/openspec/changes/a-lease-says-who/design.md b/openspec/changes/a-lease-says-who/design.md new file mode 100644 index 00000000..fb6b0314 --- /dev/null +++ b/openspec/changes/a-lease-says-who/design.md @@ -0,0 +1,92 @@ +# Design + +## Decision: attribution, and the word is load-bearing + +The lease gains the git identity of the working directory that took it — +`user.email`, falling back to `user.name`. It is the same value that +signs every commit in the repository, so it is already the name people +recognise each other by here. + +Anybody can set it to anything. That is stated in the type, in the +message, and in this file, because the failure mode of recording it is +not that it is wrong: it is that a later reader treats a self-declared +label as an audit trail. The message says "git author", never "user", +and nothing gates on it. + +## Decision: read once, not per heartbeat + +`acquireOrRenew` runs every five seconds while a mutating run is +active. Reading `git config user.email` there would spawn a process +twelve times a minute for a value that cannot change during a run. + +So the identity is gathered once by whoever constructs the manager — +three places, one call each at activation — and passed in. A manager +given none records none, which is what every lease written before this +change already looks like. + +## Decision: a stuck lease is mostly not a thing, and the remedy is not force + +The tempting design is `lease release --force`. It is wrong, and working +out why changed this change's shape. + +A holder that **died** stops renewing, and the next acquirer reclaims +the lease automatically once the heartbeat is older than the staleness +window. That case already heals; nothing is needed. + +A holder that is **alive and still renewing** has the workspace open. It +may be stuck — an agent waiting forever on something — but taking its +lease would let a second mutating run start against files the first one +still has open, which is the exact scenario the lease exists to prevent. +The remedy there is to stop that process, not to steal from it. + +So `release` clears a lease only where it can establish the holder is +gone: + +- the heartbeat is already stale — the holder is gone by the definition + the lease has always used; or +- the holder is on **this** hostname and its pid is not running. + +`process.kill(pid, 0)` answers the second without signalling anything: +`ESRCH` means no such process, `EPERM` means it exists and belongs to +somebody else. A holder on another hostname cannot be checked from here +at all, and the refusal says that rather than guessing. + +## Decision: asking is its own command, not a side effect of being refused + +Today the only way to learn who holds a workspace is to try to start a +run and read the refusal. That is a strange way to ask a question, and +it means the answer is only available at the moment you are being told +no. + +`openspec-ui-cli lease` prints the holder — kind, hostname, pid, how +long since its heartbeat, and the git author where one was recorded — +or says the workspace is free. + +## Decision: exit codes say what was found, not whether the tool worked + +`lease` exits `0` whether or not the workspace is held: it answered the +question either way, and a script checking "is it free" should read the +output rather than infer from a failure code the way `validate` does. + +`lease release` exits `0` when it cleared one, `1` when it refused +because the holder is alive or unverifiable, and `2` when it could not +look at all. + +## Non-Goals + +Authentication. Per-person permissions. Identity in the audit log, which +is a larger change through a runner that has no notion of a workspace. +Clearing a lease held by a live process. + +## Risks / Trade-offs + +A pid on this hostname can be reused. A holder that died and whose pid +was taken by an unrelated process reads as alive, and `release` refuses +where it could safely have cleared. That is the direction to fail in: +the staleness window still clears it twenty seconds later, so the cost +is a wait, not a stuck workspace. + +Recording an email address in a file inside the repository's working +directory — `.openspec-ui/` is gitignored, so it does not leave the +machine, but it is written where a person might not expect it. Named +here so the choice is visible. diff --git a/openspec/changes/a-lease-says-who/proposal.md b/openspec/changes/a-lease-says-who/proposal.md new file mode 100644 index 00000000..33893762 --- /dev/null +++ b/openspec/changes/a-lease-says-who/proposal.md @@ -0,0 +1,63 @@ +# A lease says who + +## Why + +An outside review of how this tool behaves for several people at once, +relayed on 2026-09-11, made two points about the workspace lease. Both +are correct, and one of them has a different remedy than it looks. + +**It does not say who.** The lease records a host kind, a hostname and a +pid. Two people sharing a machine, or one person with two checkouts, see +"terminal run on HPP-NTB63, pid 4242" and learn nothing about whose run +it is. The audit log is worse: `AuditEntry` carries `runId`, `agent`, +`cwd`, `changeDir` and `usage`, and nothing at all about who caused it. + +There is a signal already in every repository and not being used: the +git identity of the working directory, the same `user.email` that signs +every commit. It is **attribution and not authentication** — anybody can +set it to anything — and recorded as such it is exactly what a person +needs in order to know whose run is holding the workspace. + +**There is no way to clear a stuck lease.** True, and the obvious +remedy is wrong. Where a holder has died, its heartbeat stops and the +next acquirer reclaims the lease automatically — that case already +heals. The case that does not heal is a holder that is alive and still +renewing while doing nothing useful, and taking the lease from it would +let a second mutating run start while the first still has the workspace +open. That is what the lease exists to prevent. + +So what is missing is not a `--force`. It is the ability to ask who +holds the workspace without attempting a run, and a release that +establishes the holder is gone before clearing anything. + +## Capabilities + +### New + +- A lease records the git identity of the working directory that took + it, so a person can tell whose run holds the workspace. +- The holder of a workspace can be asked about directly, rather than + discovered by trying to start a run and being refused. +- A lease can be cleared where the holder can be shown to be gone, and + refused where it cannot. + +### Modified + +- A refusal naming the holder names the git identity too, where the + holder recorded one. + +## Out of scope + +Authentication, access control, or per-person permissions. Anybody can +set `user.email` to anything, and a system that treated it as proof +would be worse than one that records nothing — it would look like an +audit trail while being a self-declared label. This records who says +they are running it, and says so in those words. + +Identity in the audit log. Worth doing and a larger question: audit +entries are written per run by an agent runner that has no notion of a +workspace, and threading it there is its own change. + +Clearing a lease whose holder is alive. That is what stopping the +process is for. A command that did it would defeat the isolation the +lease provides, at the moment somebody is most tempted to use it. diff --git a/openspec/changes/a-lease-says-who/specs/ci-cli/spec.md b/openspec/changes/a-lease-says-who/specs/ci-cli/spec.md new file mode 100644 index 00000000..8c0af3e2 --- /dev/null +++ b/openspec/changes/a-lease-says-who/specs/ci-cli/spec.md @@ -0,0 +1,86 @@ +## ADDED Requirements + +### Requirement: A lease records who took it, as attribution + +A workspace lease SHALL record the git identity of the working directory +that took it, where one is configured. + +It SHALL be reported as what it is: a self-declared label, the same one +that signs the repository's commits, which anybody can set to anything. +Nothing SHALL be permitted or refused on the strength of it. + +A lease taken where no identity is configured SHALL be valid and SHALL +record none, exactly as every lease written before this existed. + +#### Scenario: A run in a working directory with a git identity + +- **WHEN** a run takes the workspace in a directory that has a git + identity configured +- **THEN** the lease records it, and a refusal naming the holder names + it too + +#### Scenario: No identity configured + +- **WHEN** no git identity is configured +- **THEN** the lease is taken and records none + +### Requirement: The holder of a workspace can be asked about + +It SHALL be possible to ask who holds a workspace without attempting to +start a run. + +The answer SHALL name the kind of host, where it is running, its process, +how long since it last reported itself, and its git identity where one +was recorded. Where nothing holds the workspace, it SHALL say so. + +Asking SHALL succeed whether or not the workspace is held: the question +was answered either way. + +#### Scenario: Asking about a held workspace + +- **WHEN** the holder is asked for and a live lease exists +- **THEN** it is described, and the command reports success + +#### Scenario: Asking about a free workspace + +- **WHEN** nothing holds the workspace +- **THEN** it says so, and the command reports success + +### Requirement: A lease is cleared only where its holder is shown to be gone + +Clearing a lease SHALL require establishing that its holder is gone. + +A heartbeat older than the staleness window establishes it — that is +what the lease has always meant by a holder no longer being there. + +A holder on this same machine whose process is no longer running +establishes it. Checking SHALL NOT signal the process. + +Where the holder is on another machine, or its process is still running, +clearing SHALL be refused, saying which of the two it was. Taking a lease +from a live holder would permit a second mutating run against files the +first still holds open, which is what the lease exists to prevent — and +a holder that is stuck is stopped, not robbed. + +#### Scenario: A holder whose process has gone + +- **WHEN** clearing is requested and the holder is on this machine with + no such process running +- **THEN** the lease is cleared + +#### Scenario: A holder that is still running + +- **WHEN** clearing is requested and the holder's process is running +- **THEN** it is refused, and the message says the holder is alive and + that stopping it is the remedy + +#### Scenario: A holder somewhere else + +- **WHEN** the holder is on another machine +- **THEN** clearing is refused, saying that it cannot be checked from + here + +#### Scenario: A lease already stale + +- **WHEN** the heartbeat is older than the staleness window +- **THEN** the lease is cleared without needing to check any process diff --git a/openspec/changes/a-lease-says-who/tasks.md b/openspec/changes/a-lease-says-who/tasks.md new file mode 100644 index 00000000..2f3dc26b --- /dev/null +++ b/openspec/changes/a-lease-says-who/tasks.md @@ -0,0 +1,99 @@ +The lease records a host kind, a hostname and a pid, and the audit log +records nobody at all. Two people on one machine cannot tell whose run +holds the workspace, and there is no way to ask who does without trying +to start a run and being refused. + +## 1. Who took it + +- [x] 1.1 `WorkspaceLeaseDocument` gains an optional git identity — + `user.email`, falling back to `user.name`. Optional because every + lease written before this has none, and a directory with no identity + configured must still be able to take one. +- [x] 1.2 Named and documented as **attribution, never authentication**. + Anybody can set `user.email` to anything. The failure mode of + recording it is not that it is wrong; it is that a later reader treats + a self-declared label as an audit trail, so the message says "git + author" and nothing is ever gated on it. +- [x] 1.3 Read once by whoever constructs the manager, not inside + `acquireOrRenew`. That runs every five seconds while a run is active, + and reading git config there would spawn a process twelve times a + minute for a value that cannot change mid-run. +- [x] 1.4 `GitWrapper` gains a read of the configured identity. Absent + config, absent git, or an error all mean none — never a guess. +- [x] 1.5 The three construction sites pass it: the CLI run, the + standalone server's recovery service, and the extension's activation. +- [x] 1.6 `describeWorkspaceLeaseConflict` names it where present and + reads the same without it. + +## 2. Asking who holds it + +- [x] 2.1 `openspec-ui-cli lease`: the holder's kind, host, pid, + heartbeat age and git identity, or that the workspace is free. +- [x] 2.2 Exit `0` either way. The question was answered whether or not + the workspace is held, and a script wanting "is it free" should read + the output rather than infer from a failure code. +- [x] 2.3 `--format json` for a machine, the same shape the reader + returns. + +## 3. Clearing one + +- [x] 3.1 `openspec-ui-cli lease release` clears only where the holder + is established to be gone: the heartbeat is already stale, or the + holder is on this hostname and its pid is not running. +- [x] 3.2 The liveness check does not signal the process. + `process.kill(pid, 0)` answers it: `ESRCH` is no such process, `EPERM` + is one that exists and belongs to somebody else. +- [x] 3.3 A holder on another hostname cannot be checked from here. + Refuse and say that, rather than guess. +- [x] 3.4 A live holder is refused, and the message says stopping that + process is the remedy. Taking its lease would let a second mutating + run start against files it still has open, which is the whole point of + the lease. +- [x] 3.5 Exit `0` cleared, `1` refused, `2` could not look. + +## 4. Tests + +- [x] 4.1 Core: a lease records the identity it was given, and one + written without it stays readable. +- [x] 4.2 Core: the conflict description names the identity where + present and reads correctly without it. +- [x] 4.3 Core: clearing a stale lease succeeds without consulting any + process. +- [x] 4.4 Core: clearing is refused for a live pid on this hostname — + driven with this test process's own pid, which is certainly alive. +- [x] 4.5 Core: clearing is refused for another hostname, saying it + cannot be checked. +- [x] 4.6 Core: clearing a dead pid on this hostname succeeds. A pid + that is certainly not running is needed, so the test must establish + that rather than assume a number. +- [x] 4.7 CLI: `lease` exits 0 held and free; `lease release` exits 0, + 1 and 2 for the three outcomes. + +## 5. Verification + +- [x] 5.1 This change validates strictly. `check(validate-change)` + `openspec validate --strict --changes` — 2 passed, 0 failed, this + change among them. +- [x] 5.2 `npm run verify` unpiped, after the last edit, with everything + staged. Record the run and the per-package test counts. + 2026-09-11, exit 0. Typecheck and lint clean across all five packages + (including `lint:english`, `lint:source-text`, `lint:changesets`). + Tests: cli 107, core 1075, vscode 327, server 80, webui 379 — 1968 + across 155 files, 0 failed. +- [x] 5.3 A pending changeset exists. `check(changeset-present)` + `.changeset/a-lease-says-who.md`: core and cli minor, the extension + patch. + A live smoke on the way past, recorded because it exercised the + backward-compatible path for real rather than from a written file: the + workspace was held by a VS Code extension built *before* this change, + so its lease carries no identity. `lease` described it (exit 0) and + omitted the git author line entirely; `lease release` refused it + (exit 1) because its pid was running, naming stopping that process as + the remedy. What this did NOT show is a lease written by this build, + which is what 5.4 is for. +- [ ] 5.4 **Delegated to `claude-cli`**: with a real run holding a + workspace, ask who holds it and try to clear it; then after that run + ends, ask again. Evidence: the lease file, both outputs, and the exit + codes. The unit tests drive the reader with written files; only a real + run shows that the identity a chain records is the one the command + reports. diff --git a/packages/cli/src/lease-command.test.ts b/packages/cli/src/lease-command.test.ts new file mode 100644 index 00000000..438e2305 --- /dev/null +++ b/packages/cli/src/lease-command.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it } from "vitest"; +import type { LeaseReleaseOutcome, WorkspaceLeaseConflict } from "@openspec-ui/core"; +import { leaseCommand } from "./lease-command.js"; + +function collectingIo() { + const out: string[] = []; + const err: string[] = []; + return { out, err, stdout: (line: string) => out.push(line), stderr: (line: string) => err.push(line) }; +} + +const HOLDER: WorkspaceLeaseConflict = { + hostKind: "cli", + hostname: "a-machine", + pid: 777, + heartbeatAgeMs: 3_000, + author: "ada@example.com", +}; + +describe("leaseCommand", () => { + it("describes the holder and succeeds", async () => { + const io = collectingIo(); + + const code = await leaseCommand( + { workspaceRoot: "/repo", format: "text" }, + { ...io, read: async () => HOLDER }, + ); + + expect(code).toBe(0); + const text = io.out.join("\n"); + expect(text).toContain("a-machine"); + expect(text).toContain("pid 777"); + expect(text).toContain("Git author ada@example.com"); + }); + + it("says a free workspace is free, and still succeeds", async () => { + const io = collectingIo(); + + // Exit 0 either way: the question was answered. A script asking "is + // it free" reads the output rather than inferring from a code. + const code = await leaseCommand( + { workspaceRoot: "/repo", format: "text" }, + { ...io, read: async () => undefined }, + ); + + expect(code).toBe(0); + expect(io.out.join("\n")).toContain("Nothing holds this workspace."); + expect(io.err).toEqual([]); + }); + + it("reports the holder as json in the reader's own shape", async () => { + const io = collectingIo(); + + const code = await leaseCommand( + { workspaceRoot: "/repo", format: "json" }, + { ...io, read: async () => HOLDER }, + ); + + expect(code).toBe(0); + expect(JSON.parse(io.out.join("\n"))).toEqual({ held: true, holder: HOLDER }); + }); + + it("exits 0 when a lease was cleared", async () => { + const io = collectingIo(); + const outcome: LeaseReleaseOutcome = { kind: "cleared", holder: HOLDER, because: "process-gone" }; + + const code = await leaseCommand( + { workspaceRoot: "/repo", action: "release", format: "text" }, + { ...io, release: async () => outcome }, + ); + + expect(code).toBe(0); + expect(io.out.join("\n")).toContain("Cleared."); + }); + + it("exits 1 when releasing was refused, and says why on stderr", async () => { + const io = collectingIo(); + const outcome: LeaseReleaseOutcome = { + kind: "refused", + holder: HOLDER, + reason: "process 777 is still running. ... stop that process instead.", + }; + + const code = await leaseCommand( + { workspaceRoot: "/repo", action: "release", format: "text" }, + { ...io, release: async () => outcome }, + ); + + expect(code).toBe(1); + expect(io.out).toEqual([]); + expect(io.err.join("\n")).toContain("stop that process instead"); + }); + + it("exits 2 when the lease could not be looked at", async () => { + const io = collectingIo(); + + const code = await leaseCommand( + { workspaceRoot: "/repo", action: "release", format: "text" }, + { + ...io, + release: async () => { + throw new Error("EACCES"); + }, + }, + ); + + expect(code).toBe(2); + expect(io.err.join("\n")).toContain("EACCES"); + }); + + it("refuses an action it does not have", async () => { + const io = collectingIo(); + + const code = await leaseCommand({ workspaceRoot: "/repo", action: "steal", format: "text" }, io); + + expect(code).toBe(2); + expect(io.err.join("\n")).toContain("unknown lease action 'steal'"); + }); +}); diff --git a/packages/cli/src/lease-command.ts b/packages/cli/src/lease-command.ts new file mode 100644 index 00000000..73201bd0 --- /dev/null +++ b/packages/cli/src/lease-command.ts @@ -0,0 +1,121 @@ +// `openspec-ui-cli lease` and `lease release` — a-lease-says-who. +// +// Presentation only. Who holds a workspace is read by core +// (`readWorkspaceLeaseHolder`), and whether a lease may be cleared is +// decided by core (`releaseWorkspaceLease`); this turns either into +// something a person reads and an exit code. +// +// Until now the only way to learn who holds a workspace was to try to +// start a run and read the refusal, which is a strange way to ask a +// question and only answers it at the moment you are being told no. + +import { + WORKSPACE_LEASE_STALE_AFTER_MS, + hostKindLabel, + readWorkspaceLeaseHolder, + releaseWorkspaceLease, + type LeaseReleaseOutcome, + type WorkspaceLeaseConflict, +} from "@openspec-ui/core"; + +export interface LeaseOptions { + workspaceRoot: string; + /** `undefined` asks who holds it; `"release"` tries to clear it. */ + action?: string; + format: "text" | "json"; +} + +export interface LeaseDeps { + stdout: (line: string) => void; + stderr: (line: string) => void; + /** Test seams, the same shape `validateAll` already is. */ + read?: typeof readWorkspaceLeaseHolder; + release?: typeof releaseWorkspaceLease; +} + +export async function leaseCommand(options: LeaseOptions, deps: LeaseDeps): Promise { + if (options.action === "release") return await releaseLease(options, deps); + if (options.action !== undefined) { + deps.stderr(`openspec-ui-cli: unknown lease action '${options.action}' (supported: release)`); + return 2; + } + return await describeHolder(options, deps); +} + +/** Always `0`, held or free. + * + * The question was answered either way, and a script asking "is it free" + * should read the output rather than infer it from a failure code — the + * same reasoning `ready` uses for a repository where nothing can start. */ +async function describeHolder(options: LeaseOptions, deps: LeaseDeps): Promise { + let holder: WorkspaceLeaseConflict | undefined; + try { + holder = await (deps.read ?? readWorkspaceLeaseHolder)(options.workspaceRoot); + } catch (error) { + deps.stderr(`openspec-ui-cli: could not read the workspace lease: ${message(error)}`); + return 2; + } + + if (options.format === "json") { + deps.stdout(JSON.stringify({ held: holder !== undefined, ...(holder ? { holder } : {}) }, null, 2)); + return 0; + } + + if (!holder) { + deps.stdout("Nothing holds this workspace."); + return 0; + } + for (const line of holderLines(holder)) deps.stdout(line); + return 0; +} + +/** `0` cleared, `1` refused, `2` could not look. */ +async function releaseLease(options: LeaseOptions, deps: LeaseDeps): Promise { + let outcome: LeaseReleaseOutcome; + try { + outcome = await (deps.release ?? releaseWorkspaceLease)({ workspaceRoot: options.workspaceRoot }); + } catch (error) { + deps.stderr(`openspec-ui-cli: could not release the workspace lease: ${message(error)}`); + return 2; + } + + if (options.format === "json") { + deps.stdout(JSON.stringify(outcome, null, 2)); + return outcome.kind === "cleared" ? 0 : 1; + } + + if (outcome.kind === "cleared") { + if (outcome.because === "already-free") { + deps.stdout("Nothing held this workspace."); + } else if (outcome.because === "stale") { + const seconds = Math.round(WORKSPACE_LEASE_STALE_AFTER_MS / 1000); + deps.stdout(`Cleared a lease whose holder had not reported itself for over ${seconds}s.`); + } else { + deps.stdout("Cleared. Its holder:"); + for (const line of holderLines(outcome.holder as WorkspaceLeaseConflict)) deps.stdout(` ${line}`); + deps.stdout(" that process is no longer running."); + } + return 0; + } + + deps.stderr("openspec-ui-cli: will not release this lease."); + for (const line of holderLines(outcome.holder)) deps.stderr(` ${line}`); + deps.stderr(` ${outcome.reason}`); + return 1; +} + +function holderLines(holder: WorkspaceLeaseConflict): string[] { + const lines = [ + `Held by ${hostKindLabel(holder.hostKind)} on ${holder.hostname}, pid ${holder.pid}.`, + `Last reported itself ${Math.round(holder.heartbeatAgeMs / 1000)}s ago.`, + ]; + // "git author", never "user": the value is self-declared and nothing + // is gated on it. A line calling it a user would read as an identity + // this system had established (a-lease-says-who). + if (holder.author) lines.push(`Git author ${holder.author}.`); + return lines; +} + +function message(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/packages/cli/src/main.test.ts b/packages/cli/src/main.test.ts index bab39b1d..00e9030b 100644 --- a/packages/cli/src/main.test.ts +++ b/packages/cli/src/main.test.ts @@ -129,6 +129,37 @@ describe("runMain", () => { }); }); +describe("runMain — lease", () => { + it("asks who holds the workspace with no action", async () => { + const io = collectingIo(); + const leaseCommand = vi.fn().mockResolvedValue(0); + + const code = await runMain(["lease", "--cwd", "/repo"], { leaseCommand, ...io }); + + expect(code).toBe(0); + expect(leaseCommand).toHaveBeenCalledWith({ workspaceRoot: "/repo", format: "text" }, expect.anything()); + }); + + it("passes the release action through", async () => { + const io = collectingIo(); + const leaseCommand = vi.fn().mockResolvedValue(1); + + const code = await runMain(["lease", "release", "--cwd", "/repo"], { leaseCommand, ...io }); + + expect(code).toBe(1); + expect(leaseCommand.mock.calls[0]?.[0]).toMatchObject({ action: "release" }); + }); + + it("passes --format json through to lease", async () => { + const io = collectingIo(); + const leaseCommand = vi.fn().mockResolvedValue(0); + + await runMain(["lease", "--format", "json"], { leaseCommand, ...io }); + + expect(leaseCommand.mock.calls[0]?.[0]).toMatchObject({ format: "json" }); + }); +}); + describe("runMain — run and check", () => { it("passes the change name and the repository root to run", async () => { const io = collectingIo(); diff --git a/packages/cli/src/main.ts b/packages/cli/src/main.ts index ad27a34f..df660988 100644 --- a/packages/cli/src/main.ts +++ b/packages/cli/src/main.ts @@ -7,6 +7,7 @@ import { readChangeGraph } from "@openspec-ui/core"; import { renderChangeAncestry, renderChangeTree } from "./change-graph-render.js"; import { checkChange } from "./check-change.js"; +import { leaseCommand } from "./lease-command.js"; import { runChange, type CheckpointPrompt } from "./run-change.js"; import { readyCommand } from "./ready-command.js"; import { worktreeCommand } from "./worktree-command.js"; @@ -25,6 +26,8 @@ Usage: openspec-ui-cli run [--cwd ] [--format text|json] openspec-ui-cli check [--cwd ] [--format text|json] openspec-ui-cli ready [--cwd ] [--base ] [--format text|json] + openspec-ui-cli lease [--cwd ] [--format text|json] + openspec-ui-cli lease release [--cwd ] [--format text|json] openspec-ui-cli worktree add [--cwd ] [--path ] [--base ] openspec-ui-cli worktree list [--cwd ] [--format text|json] @@ -71,6 +74,14 @@ Exit codes: unreadable package.json, a change whose configuration this terminal cannot honour, another host holding the workspace) +'lease' exits 0 whether or not the workspace is held: it answered the +question either way. 'lease release' exits 0 when it cleared a lease and +1 when it refused. It clears one only where the holder can be shown to be +gone — its heartbeat is already stale, or it is on this machine and its +process is not running. A live holder is refused: taking its lease would +let a second mutating run start against files it still has open, which is +what the lease exists to prevent. + A run does only what the change's own harness configuration already permits. There is no flag that starts a chain for a change configured to run one stage at a time, and none that answers a confirmation the change @@ -112,6 +123,7 @@ export interface MainDeps { worktreeCommand?: typeof worktreeCommand; readyCommand?: typeof readyCommand; checkChange?: typeof checkChange; + leaseCommand?: typeof leaseCommand; /** How a checkpoint is put to a person, and how their answer comes * back. Absent `ask` means nobody is there, which is what makes a * change configured to pause refuse to start rather than hang. @@ -244,6 +256,19 @@ export async function runMain(argv: string[], deps: MainDeps = {}): Promise` puts its + // subject, so it arrives as the same positional. + ...(options.changeName !== undefined ? { action: options.changeName } : {}), + format: options.format === "json" ? "json" : "text", + }, + { stdout, stderr }, + ); + } + if (command === "worktree") { const action = options.changeName; if (action !== "add" && action !== "list" && action !== "remove") { @@ -295,7 +320,7 @@ export async function runMain(argv: string[], deps: MainDeps = {}): Promise { if (hold.reclaimedFrom) deps.stderr(`openspec-ui-cli: ${describeWorkspaceLeaseReclamation(hold.reclaimedFrom)}`); return await driveChain( diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts index 08fe7807..e76e765c 100644 --- a/packages/core/src/git.ts +++ b/packages/core/src/git.ts @@ -87,6 +87,15 @@ export interface GitWrapper { * runs is the command that was decided on. */ worktreeAdd(options: { path: string; branch: string; base: string }): Promise; worktreeRemove(path: string): Promise; + /** The git identity configured for this working directory — + * `user.email`, falling back to `user.name` — or `undefined` where + * none is set. + * + * ATTRIBUTION, NEVER AUTHENTICATION. Anybody can set this to + * anything; it is the same self-declared label that signs every + * commit. Nothing may be permitted or refused on the strength of it. + * See a-lease-says-who. */ + configuredIdentity(): Promise; /** A remote's URL, or `undefined` where the remote does not exist. * Read-only, and never inferred: where a repository is hosted is the * only thing that decides whether a Dependabot config could mean @@ -152,6 +161,18 @@ export function createGitWrapper(options: GitWrapperOptions): GitWrapper { // somebody who means it. await git.raw(["worktree", "remove", worktreePath]); }, + async configuredIdentity(): Promise { + for (const key of ["user.email", "user.name"]) { + try { + const value = (await git.raw(["config", "--get", key])).trim(); + if (value.length > 0) return value; + } catch { + // Unset, or no git at all. Both mean there is no identity to + // record — never a guess. + } + } + return undefined; + }, async remoteUrl(remote: string): Promise { try { const out = await git.raw(["remote", "get-url", remote]); @@ -181,3 +202,14 @@ export function createGitWrapper(options: GitWrapperOptions): GitWrapper { }, }; } + +/** The git identity to record on a workspace lease, or `undefined`. + * + * ATTRIBUTION, NEVER AUTHENTICATION (a-lease-says-who). Its one caller + * shape is: read this once where a host starts up, and hand the result + * to `WorkspaceLeaseManager`. Never call it from a heartbeat — the value + * cannot change during a run, and the heartbeat runs every five + * seconds. */ +export async function readGitAuthor(cwd: string): Promise { + return await createGitWrapper({ cwd }).configuredIdentity(); +} diff --git a/packages/core/src/harness-chain-runner.test.ts b/packages/core/src/harness-chain-runner.test.ts index e81859c9..7e108043 100644 --- a/packages/core/src/harness-chain-runner.test.ts +++ b/packages/core/src/harness-chain-runner.test.ts @@ -221,6 +221,7 @@ function makeGitStageDeps(options: { pathExistsInRef: vi.fn(async () => true), changedFilesBetween: vi.fn(async () => []), remoteUrl: vi.fn(async () => undefined), + configuredIdentity: vi.fn(async () => undefined), }; const gateway: PullRequestGateway = { createPullRequest: vi.fn(async () => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index fc17964a..1c8406a9 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -25,6 +25,7 @@ export * from "./harness-config.js"; export * from "./harness-chain-runner.js"; export * from "./process-scheduler.js"; export * from "./workspace-lease.js"; +export * from "./lease-release.js"; export * from "./checkpoint.js"; export * from "./workbench-run-journal.js"; export * from "./workbench-recovery.js"; diff --git a/packages/core/src/lease-release.test.ts b/packages/core/src/lease-release.test.ts new file mode 100644 index 00000000..cd3660d6 --- /dev/null +++ b/packages/core/src/lease-release.test.ts @@ -0,0 +1,139 @@ +import { spawn } from "node:child_process"; +import { access, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { releaseWorkspaceLease } from "./lease-release.js"; +import { WORKSPACE_LEASE_VERSION, type WorkspaceLeaseDocument } from "./workspace-lease.js"; + +// every-varying-check-has-a-budget: +// measured 2026-09-11 for this file alone at 0.3s idle, its slowest +// single test being the one that waits for a real child to exit. +vi.setConfig({ testTimeout: 15_000 }); + +const roots: string[] = []; + +async function temporaryRoot(): Promise { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-ui-release-")); + roots.push(root); + return root; +} + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); +}); + +function leasePath(root: string): string { + return path.join(root, ".openspec-ui", "workspace.lease.json"); +} + +async function writeLease(root: string, overrides: Partial): Promise { + const now = new Date().toISOString(); + const document: WorkspaceLeaseDocument = { + version: WORKSPACE_LEASE_VERSION, + holderId: "holder-1", + hostKind: "cli", + hostname: os.hostname(), + pid: process.pid, + acquiredAt: now, + heartbeatAt: now, + ...overrides, + }; + await mkdir(path.dirname(leasePath(root)), { recursive: true }); + await writeFile(leasePath(root), JSON.stringify(document, null, 2), "utf8"); +} + +async function exists(filePath: string): Promise { + try { + await access(filePath); + return true; + } catch { + return false; + } +} + +/** A pid that is certainly not running, established rather than assumed. + * + * Picking a large number and hoping would make this test pass for the + * wrong reason on a machine that happened to have it. Spawning something + * and waiting for it to exit is the only way to know. */ +async function deadPid(): Promise { + const child = spawn(process.execPath, ["-e", ""], { stdio: "ignore" }); + const pid = child.pid; + if (pid === undefined) throw new Error("could not spawn a process to exhaust"); + await new Promise((resolve) => child.on("exit", () => resolve())); + return pid; +} + +describe("releaseWorkspaceLease", () => { + it("clears a stale lease without consulting any process", async () => { + const root = await temporaryRoot(); + await writeLease(root, { heartbeatAt: new Date(Date.now() - 60_000).toISOString() }); + const isRunning = vi.fn(() => true); + + const outcome = await releaseWorkspaceLease({ workspaceRoot: root, isRunning }); + + expect(outcome).toEqual({ kind: "cleared", because: "stale" }); + // Staleness is what this system has always meant by a holder being + // gone, so no liveness question arises at all. + expect(isRunning).not.toHaveBeenCalled(); + expect(await exists(leasePath(root))).toBe(false); + }); + + it("reports a workspace that was never held", async () => { + const root = await temporaryRoot(); + + const outcome = await releaseWorkspaceLease({ workspaceRoot: root }); + + expect(outcome).toEqual({ kind: "cleared", because: "already-free" }); + }); + + it("refuses a holder whose process is still running", async () => { + const root = await temporaryRoot(); + // This test's own pid, which is certainly alive. + await writeLease(root, { pid: process.pid, author: "ada@example.com" }); + + const outcome = await releaseWorkspaceLease({ workspaceRoot: root }); + + expect(outcome.kind).toBe("refused"); + if (outcome.kind === "refused") { + expect(outcome.holder.author).toBe("ada@example.com"); + expect(outcome.reason).toContain("still running"); + // The remedy is stopping it, not taking its lease. + expect(outcome.reason).toContain("stop that process"); + } + expect(await exists(leasePath(root))).toBe(true); + }); + + it("refuses a holder on another machine, saying it cannot be checked", async () => { + const root = await temporaryRoot(); + await writeLease(root, { hostname: "some-other-box", pid: 4242 }); + const isRunning = vi.fn(() => false); + + const outcome = await releaseWorkspaceLease({ workspaceRoot: root, isRunning }); + + expect(outcome.kind).toBe("refused"); + if (outcome.kind === "refused") { + expect(outcome.reason).toContain("some-other-box"); + expect(outcome.reason).toContain("cannot be checked from here"); + } + // A pid on another machine says nothing about a process on this one, + // so it is never even asked about. + expect(isRunning).not.toHaveBeenCalled(); + expect(await exists(leasePath(root))).toBe(true); + }); + + it("clears a lease whose holder on this machine has gone", async () => { + const root = await temporaryRoot(); + await writeLease(root, { pid: await deadPid(), author: "ada@example.com" }); + + const outcome = await releaseWorkspaceLease({ workspaceRoot: root }); + + expect(outcome.kind).toBe("cleared"); + if (outcome.kind === "cleared") { + expect(outcome.because).toBe("process-gone"); + expect(outcome.holder?.author).toBe("ada@example.com"); + } + expect(await exists(leasePath(root))).toBe(false); + }); +}); diff --git a/packages/core/src/lease-release.ts b/packages/core/src/lease-release.ts new file mode 100644 index 00000000..7b5c0823 --- /dev/null +++ b/packages/core/src/lease-release.ts @@ -0,0 +1,103 @@ +// Asking who holds a workspace, and clearing a lease whose holder is +// gone — see openspec/changes/a-lease-says-who/design.md. +// +// The tempting design is a `--force`, and it is wrong. A holder that +// DIED stops renewing and the next acquirer reclaims the lease on its +// own once the heartbeat is stale; that case already heals. A holder +// that is ALIVE and still renewing has the workspace open, and taking +// its lease would let a second mutating run start against files it is +// still holding — which is the exact thing the lease exists to prevent. +// A stuck holder is stopped, not robbed. +// +// So clearing requires establishing that the holder is gone, and the +// two ways to establish it are below. + +import { access, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { + WORKSPACE_LEASE_STALE_AFTER_MS, + readWorkspaceLeaseHolder, + type WorkspaceLeaseConflict, +} from "./workspace-lease.js"; + +export type LeaseReleaseOutcome = + /** The lease file is gone, or there was none to begin with. */ + | { kind: "cleared"; holder?: WorkspaceLeaseConflict; because: "stale" | "process-gone" | "already-free" } + /** The holder could not be shown to be gone, so nothing was touched. */ + | { kind: "refused"; holder: WorkspaceLeaseConflict; reason: string }; + +/** Whether a process exists, without signalling it. + * + * Signal `0` performs the permission and existence checks and delivers + * nothing. `ESRCH` is no such process; `EPERM` is one that exists and + * belongs to somebody else, which still counts as alive. */ +export function isProcessRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + const code = error instanceof Error && "code" in error ? error.code : undefined; + if (code === "EPERM") return true; + return false; + } +} + +export interface LeaseReleaseOptions { + workspaceRoot: string; + /** Test seams. Production reads the machine. */ + hostname?: string; + isRunning?: (pid: number) => boolean; + staleAfterMs?: number; +} + +/** Clears the workspace's lease only where its holder can be shown to + * be gone, and reports which of the two ways established it. */ +export async function releaseWorkspaceLease(options: LeaseReleaseOptions): Promise { + const workspaceRoot = path.resolve(options.workspaceRoot); + const staleAfterMs = options.staleAfterMs ?? WORKSPACE_LEASE_STALE_AFTER_MS; + const filePath = path.join(workspaceRoot, ".openspec-ui", "workspace.lease.json"); + + // `readWorkspaceLeaseHolder` already reports a stale lease as nobody, + // which is the first of the two ways: a heartbeat older than the + // window is what this system has always meant by a holder no longer + // being there. A file left behind by one is cleared on the way past. + const holder = await readWorkspaceLeaseHolder(workspaceRoot, { staleAfterMs }); + if (!holder) { + const existed = await removeIfPresent(filePath); + return { kind: "cleared", because: existed ? "stale" : "already-free" }; + } + + const hostname = options.hostname ?? os.hostname(); + if (holder.hostname !== hostname) { + return { + kind: "refused", + holder, + reason: `the holder is on ${holder.hostname} and this is ${hostname}, so whether its process is still` + + " running cannot be checked from here. Clear it there, or wait for its lease to go stale.", + }; + } + + const isRunning = options.isRunning ?? isProcessRunning; + if (isRunning(holder.pid)) { + return { + kind: "refused", + holder, + reason: `process ${holder.pid} is still running. Taking its lease would let a second mutating run start` + + " against files it still has open, which is what the lease prevents — stop that process instead.", + }; + } + + await removeIfPresent(filePath); + return { kind: "cleared", holder, because: "process-gone" }; +} + +async function removeIfPresent(filePath: string): Promise { + try { + await access(filePath); + } catch { + return false; + } + await rm(filePath, { force: true }); + return true; +} diff --git a/packages/core/src/workbench-recovery.ts b/packages/core/src/workbench-recovery.ts index ecffe1bb..84724666 100644 --- a/packages/core/src/workbench-recovery.ts +++ b/packages/core/src/workbench-recovery.ts @@ -9,6 +9,7 @@ import { type RollbackResult, type WorkbenchCheckpoint, } from "./checkpoint.js"; +import { readGitAuthor } from "./git.js"; import { WorkbenchProcessScheduler, type StartProcessOptions, type WorkbenchProcess } from "./process-scheduler.js"; import { WorkbenchRunJournal, @@ -47,16 +48,19 @@ export class WorkbenchRecoveryService { private scheduler = new WorkbenchProcessScheduler(); private readonly sessions = new Map(); - private constructor(root: string, options: WorkbenchRunJournalOptions) { + private constructor(root: string, options: WorkbenchRunJournalOptions, author: string | undefined) { this.journal = new WorkbenchRunJournal(root, options); - this.lease = new WorkspaceLeaseManager(root, { hostKind: "standalone-server" }); + this.lease = new WorkspaceLeaseManager(root, { hostKind: "standalone-server", author }); } static async open( root: string, options: WorkbenchRunJournalOptions = {}, ): Promise { - const service = new WorkbenchRecoveryService(root, options); + // Gathered here rather than in the constructor because reading it + // costs a subprocess, and because the lease's heartbeat must never + // be the thing that reads it (a-lease-says-who). + const service = new WorkbenchRecoveryService(root, options, await readGitAuthor(root)); await service.initialize(); return service; } diff --git a/packages/core/src/workspace-lease.test.ts b/packages/core/src/workspace-lease.test.ts index 27c3b90f..ba42d01b 100644 --- a/packages/core/src/workspace-lease.test.ts +++ b/packages/core/src/workspace-lease.test.ts @@ -45,6 +45,48 @@ describe("WorkspaceLeaseManager", () => { expect(document).toMatchObject({ version: WORKSPACE_LEASE_VERSION, hostKind: "standalone-server" }); }); + it("records the git author it was given, and names it in a refusal", async () => { + const root = await temporaryRoot(); + const holder = new WorkspaceLeaseManager(root, { + hostKind: "standalone-server", + author: "ada@example.com", + }); + await holder.acquireOrRenew(); + + expect((await readLease(root)).author).toBe("ada@example.com"); + + const contender = new WorkspaceLeaseManager(root, { hostKind: "cli" }); + const result = await contender.acquireOrRenew(); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.conflict.author).toBe("ada@example.com"); + // "git author", never "user": nothing is gated on a self-declared + // label, and the message must not imply otherwise. + expect(describeWorkspaceLeaseConflict(result.conflict)).toContain("git author ada@example.com"); + } + }); + + it("takes and describes a lease where no git author is configured", async () => { + const root = await temporaryRoot(); + const holder = new WorkspaceLeaseManager(root, { hostKind: "standalone-server" }); + await holder.acquireOrRenew(); + + // Absent, not empty: every lease written before this field existed + // looks exactly like this one, and must stay readable. + expect((await readLease(root)).author).toBeUndefined(); + + const contender = new WorkspaceLeaseManager(root, { hostKind: "cli" }); + const result = await contender.acquireOrRenew(); + expect(result.ok).toBe(false); + if (!result.ok) { + const described = describeWorkspaceLeaseConflict(result.conflict); + expect(described).not.toContain("git author"); + expect(described).toContain("standalone server"); + // No stray punctuation where the name would have been. + expect(described).not.toContain(", ,"); + } + }); + it("renews its own lease, keeping the original acquiredAt", async () => { const root = await temporaryRoot(); const manager = new WorkspaceLeaseManager(root, { hostKind: "vscode-extension" }); diff --git a/packages/core/src/workspace-lease.ts b/packages/core/src/workspace-lease.ts index 07adca66..0f1163ff 100644 --- a/packages/core/src/workspace-lease.ts +++ b/packages/core/src/workspace-lease.ts @@ -36,11 +36,27 @@ export interface WorkspaceLeaseDocument { pid: number; acquiredAt: string; heartbeatAt: string; + /** The git identity of the working directory that took this lease. + * + * ATTRIBUTION, NEVER AUTHENTICATION — see a-lease-says-who. Anybody + * can set `user.email` to anything; this is the same self-declared + * label that signs every commit, recorded so a person can tell whose + * run holds the workspace. Nothing is permitted or refused on it. + * + * Optional because every lease written before this field existed has + * none, and a directory with no identity configured must still be + * able to take one. */ + author?: string; } export interface WorkspaceLeaseManagerOptions { hostKind: WorkspaceLeaseHostKind; staleAfterMs?: number; + /** Gathered ONCE by whoever constructs this, never read here. + * `acquireOrRenew` runs every five seconds while a run is active, and + * reading git config there would spawn a process twelve times a + * minute for a value that cannot change mid-run. */ + author?: string; } /** Details of the lease holder a conflicting or reclaimed acquire attempt @@ -50,6 +66,9 @@ export interface WorkspaceLeaseConflict { hostname: string; pid: number; heartbeatAgeMs: number; + /** See `WorkspaceLeaseDocument.author`: attribution, not + * authentication. Absent where the holder recorded none. */ + author?: string; } export type WorkspaceLeaseAcquireResult = @@ -71,15 +90,19 @@ const HOST_KIND_LABELS: Readonly> = { * "standalone server" — a wrong answer that reads as a plausible one, and * so survives. A lease written by a build newer than the reader still * falls through to the raw string rather than to someone else's name. */ -function hostKindLabel(hostKind: WorkspaceLeaseHostKind): string { +export function hostKindLabel(hostKind: WorkspaceLeaseHostKind): string { return HOST_KIND_LABELS[hostKind] ?? String(hostKind); } export function describeWorkspaceLeaseConflict(conflict: WorkspaceLeaseConflict): string { const heartbeatAgeSeconds = Math.round(conflict.heartbeatAgeMs / 1000); + // "git author" and not "user": the value is self-declared, and a + // message that called it a user would read as an identity this system + // had established. + const author = conflict.author ? `, git author ${conflict.author}` : ""; return ( `Another OpenSpec UI host (${hostKindLabel(conflict.hostKind)} on ` + - `${conflict.hostname}, pid ${conflict.pid}, last active ${heartbeatAgeSeconds}s ago) ` + + `${conflict.hostname}, pid ${conflict.pid}${author}, last active ${heartbeatAgeSeconds}s ago) ` + `is currently running a mutating operation on this workspace. Wait for it to ` + `finish, or close it, before starting one here.` ); @@ -130,6 +153,7 @@ export async function readWorkspaceLeaseHolder( hostname: document.hostname, pid: document.pid, heartbeatAgeMs, + ...(document.author !== undefined ? { author: document.author } : {}), }; } @@ -142,11 +166,13 @@ export class WorkspaceLeaseManager { private readonly holderId = randomUUID(); private readonly hostKind: WorkspaceLeaseHostKind; private readonly staleAfterMs: number; + private readonly author: string | undefined; constructor(root: string, options: WorkspaceLeaseManagerOptions) { this.filePath = path.join(path.resolve(root), ".openspec-ui", "workspace.lease.json"); this.hostKind = options.hostKind; this.staleAfterMs = options.staleAfterMs ?? WORKSPACE_LEASE_STALE_AFTER_MS; + this.author = options.author; } /** Acquires the lease if unheld or stale, or renews it if already held by @@ -159,13 +185,25 @@ export class WorkspaceLeaseManager { if (heartbeatAgeMs <= this.staleAfterMs) { return { ok: false, - conflict: { hostKind: existing.hostKind, hostname: existing.hostname, pid: existing.pid, heartbeatAgeMs }, + conflict: { + hostKind: existing.hostKind, + hostname: existing.hostname, + pid: existing.pid, + heartbeatAgeMs, + ...(existing.author !== undefined ? { author: existing.author } : {}), + }, }; } await this.write(); return { ok: true, - reclaimedFrom: { hostKind: existing.hostKind, hostname: existing.hostname, pid: existing.pid, heartbeatAgeMs }, + reclaimedFrom: { + hostKind: existing.hostKind, + hostname: existing.hostname, + pid: existing.pid, + heartbeatAgeMs, + ...(existing.author !== undefined ? { author: existing.author } : {}), + }, }; } // Renewing our own, already-held lease: keep the original `acquiredAt` @@ -213,6 +251,7 @@ export class WorkspaceLeaseManager { pid: process.pid, acquiredAt: acquiredAt ?? now, heartbeatAt: now, + ...(this.author !== undefined ? { author: this.author } : {}), }; const directory = path.dirname(this.filePath); const temporaryPath = `${this.filePath}.${randomUUID()}.tmp`; diff --git a/packages/extension/src/extension.ts b/packages/extension/src/extension.ts index c94cdd9b..65992539 100644 --- a/packages/extension/src/extension.ts +++ b/packages/extension/src/extension.ts @@ -14,6 +14,7 @@ import { WorkspaceLeaseManager, auditLogPath, buildDefaultAgentRunners, + readGitAuthor, resolveCheckScripts, resolveRunner as resolveAgentRunner, runDelegatedItem, @@ -120,8 +121,13 @@ export async function activate(context: vscode.ExtensionContext): Promise {