diff --git a/.changeset/a-detached-directory-is-offered-for-removal.md b/.changeset/a-detached-directory-is-offered-for-removal.md new file mode 100644 index 00000000..d974c89a --- /dev/null +++ b/.changeset/a-detached-directory-is-offered-for-removal.md @@ -0,0 +1,6 @@ +--- +"@openspec-ui/core": patch +"openspec-ui-vscode": patch +--- + +A working directory on no branch, with a clean tree and no run recorded, is offered for removal under "Left behind" where the default branch already contains the commit it is at (a-detached-directory-is-offered-for-removal). Before, such a directory was never offered: it has no branch to be merged or gone, and it stood beside the checkout drawing cards for work that had landed. The sweep still keeps it; removing it is a press. diff --git a/openspec/changes/a-detached-directory-is-offered-for-removal/design.md b/openspec/changes/a-detached-directory-is-offered-for-removal/design.md new file mode 100644 index 00000000..0733fd12 --- /dev/null +++ b/openspec/changes/a-detached-directory-is-offered-for-removal/design.md @@ -0,0 +1,63 @@ +## Context + +`markFinishedDirectories` in `worktree-survey.ts` marks a directory +`finishedWith` where its branch is merged into the default branch or no ref +of that name is left, its tree is clean and no run is recorded. The mark +is what the "Left behind" list offers a press beside; it is "never a +removal" (`worktree-survey-facts.ts`). Directories on no branch were +excluded at the start, and the sweep excludes them again as `no-branch`. + +## Goals / Non-Goals + +**Goals** + +- A clean, runless directory whose head `main` already contains is offered + for removal, with the same words a merged branch gets. +- Nothing that is removed unasked. + +**Non-Goals** + +- Sweeping such a directory. `no-branch` stays in + `finished-directories.ts`. +- Reading the directory's own changes or standings. It is the commit, not + its content, that says the work is on the server. +- Changing how cards from such a directory are drawn. + +## Decisions + +### Ask `merge-base`, as for a branch + +The head is the tip. `git.mergeBase(mainCommit, head) === head` is the +same test a pushed branch gets, and it needs no new git call: the survey +already lists refs once and has `mergeBase`. The default branch is +`origin/main`, else `main`, as it is for a branch. + +### The reason is `merged`, and no branch is named + +`finishedWith.reason` is `"merged"` ("the default branch already contains +its tip"), which says exactly this. `branch` becomes optional; the +standings reading, the REST fallback and the client type already carry it +as optional, and compliance reads only the reason. + +### Offered, not swept + +A directory at `main`'s tip with a clean tree is also what a directory made +for work about to start looks like, and the product cannot tell the two +apart. Removing a clean directory loses nothing that is not on the server, +but it can remove one somebody is about to work in, or whose shell an agent +is standing in. A press leaves that to a person. Rejected: adding the same +test to the sweep, which would have been the shortest change and would have +removed exactly that directory. + +Also rejected: hiding the dashed cards a directory like this draws. +`foreignDetails` already says "archived on main" on a card whose change the +default branch carries archived, and a directory that is offered for removal +is where the rest of the clutter goes. + +## Risks / Trade-offs + +- **A directory made a minute ago for work to come is offered at once.** It + is a press, and a person who made it knows. This is the cost of offering + without being able to tell. +- **A head not on `origin/main` by a fetch not yet made** is not offered + until the next fetch; the survey reads the refs it has, as for branches. diff --git a/openspec/changes/a-detached-directory-is-offered-for-removal/proposal.md b/openspec/changes/a-detached-directory-is-offered-for-removal/proposal.md new file mode 100644 index 00000000..bdeba2de --- /dev/null +++ b/openspec/changes/a-detached-directory-is-offered-for-removal/proposal.md @@ -0,0 +1,54 @@ +## Why + +On 2026-10-13 the owner saw two dashed cards in the Pipeline for work that +had landed days before, one of them a change already archived on `main` +(`cli-is-visible-on-the-homepage`, 0/11) and one a second copy of a change +drawn above. Both came from one working directory, +`C:/Temp/osw-upstream-check`, made on 2026-10-12 for a check against +upstream and left on a detached head at a commit `main` already contained. + +Nothing offered to remove it. The directory was on no branch, so the +survey's "finished with" reading (a branch merged or gone) had nothing to +ask, and the standings reading skipped it because it held two changes +(`finishedWorkingDirectories` settles a directory by its one change). The +sweep keeps a directory on no branch on purpose (`no-branch`, +`finished-directories.ts`), and that stays: a clean detached directory at +`main`'s tip is also what a directory made a minute ago for work to come +looks like, and removing it unasked would take it from under somebody. + +`what-is-finished-is-tidied-away` and ADR-0034 settled that what is +finished is offered where it cannot be told from what is in use. This +applies that to a directory with no branch. + +## What Changes + +- **A directory on no branch is finished with where the default branch + contains its commit.** The survey asks `merge-base` of the directory's + head against the default branch, as it already does for a branch's tip, + and says `merged` where the head is an ancestor, the tree is clean and no + run is recorded against it. It names no branch, since it has none. +- **It is offered, never swept.** The answer appears in the "Left behind" + list beside a press to remove it, which is what `finishedWith` already + drives. The sweep does not act on it, and `no-branch` stays what it was + there. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `openspec-workbench`: a working directory on no branch can be finished + with, and is offered for removal rather than removed. + +## Impact + +- **`packages/core/src/worktree-survey.ts`** and + **`worktree-survey-facts.ts`**: a directory on no branch is a candidate + for `finishedWith`; its `branch` becomes optional. +- **`worktree-survey.test.ts`**: five tests, one per way the answer is + given or withheld. +- No new setting, no change to the sweep, the REST shape (`branch` was + already optional there) or the card. A changeset for `@openspec-ui/core`. diff --git a/openspec/changes/a-detached-directory-is-offered-for-removal/specs/openspec-workbench/spec.md b/openspec/changes/a-detached-directory-is-offered-for-removal/specs/openspec-workbench/spec.md new file mode 100644 index 00000000..1295bcf3 --- /dev/null +++ b/openspec/changes/a-detached-directory-is-offered-for-removal/specs/openspec-workbench/spec.md @@ -0,0 +1,37 @@ +## ADDED Requirements + +### Requirement: A working directory on no branch is finished with where the default branch contains its commit + +A surveyed working directory on no branch SHALL be reported as finished +with where the default branch already contains the commit it is at, its +tree is clean and no run is recorded against it. It SHALL be reported as +`merged` and SHALL name no branch. + +It SHALL be offered for removal beside a press and SHALL NOT be removed by +a sweep: a directory on no branch cannot be told from one made a moment ago +for work still to come, and removing it unasked can take it from under +somebody. + +#### Scenario: A detached directory at a commit main contains + +- **WHEN** a directory is on no branch, at a commit the default branch + contains, its tree is clean and no run is recorded against it +- **THEN** it is reported as finished with, naming no branch, and is + offered for removal + +#### Scenario: A detached directory ahead of main + +- **WHEN** a directory is on no branch and its commit is not contained in + the default branch +- **THEN** it is not reported as finished with + +#### Scenario: A detached directory that holds work or a run + +- **WHEN** such a directory's tree is not clean, or a run is recorded + against it +- **THEN** it is not reported as finished with + +#### Scenario: The sweep meets a detached directory + +- **WHEN** the workspace sweep reads a directory on no branch +- **THEN** it keeps it, as it did, and removes nothing diff --git a/openspec/changes/a-detached-directory-is-offered-for-removal/tasks.md b/openspec/changes/a-detached-directory-is-offered-for-removal/tasks.md new file mode 100644 index 00000000..fcb92e68 --- /dev/null +++ b/openspec/changes/a-detached-directory-is-offered-for-removal/tasks.md @@ -0,0 +1,53 @@ +Asked for by the owner on 2026-10-13, after a working directory on a +detached head stood beside the checkout for a day drawing dashed cards for +work that had landed: "the button can be made". + +## 1. The reading + +- [x] 1.1 `markFinishedDirectories` in `worktree-survey.ts` takes a + directory on no branch as a candidate where it has a head, and marks it + `finishedWith: { reason: "merged" }` where `git.mergeBase(main, head)` + returns the head, the tree is clean and no run is recorded. +- [x] 1.2 `finishedWith.branch` in `worktree-survey-facts.ts` is optional, + and its comment says why. +- [x] 1.3 `readBranchRefs` reads the refs where the only candidate is on no + branch. + +## 2. Tests + +- [x] 2.1 `worktree-survey.test.ts` covers: a clean detached directory at a + commit the default branch contains (finished, no branch named); one + ahead of it; one with an unclean tree; one with a run recorded; and one + where the default branch is not in the refs. The first fails without 1.1. + +## 3. Checks + +- [x] 3.1 `finished-directories.ts` is unchanged: the sweep keeps a + directory on no branch, and its existing test for that still passes. +- [x] 3.2 `npm run typecheck && npm run lint && npm run test`, run + unpiped. Record each package's count. + + Done 2026-10-14 by CI, run 38507448788 on this branch: "Typecheck, lint, + test, and build", "Extension integration and package" and "Standalone + browser and accessibility" succeeded. The first run failed on one test, + the message ratchet (331 against 330): the reason literal written in + place counted as one more message said in place, and is now written the + way the branch case beside it is. Locally, `message-register` and + `worktree-survey` give 45 of 45, and `tsc` on core is clean. +- [x] 3.3 `lint:english` after `git add`, `lint:changesets`, + `lint:test-budgets` and `lint:source-text` pass, and `openspec validate + a-detached-directory-is-offered-for-removal --strict` passes. + + Done 2026-10-14: all pass locally, and `lint` is part of CI's "Typecheck, + lint, test, and build". +- [x] 3.4 **Human-only, and the owner's alone.** In the standalone + application - the one place "Left behind" and its press are drawn; the + editor's Changes view does not draw them - make a directory on a detached + head at an older commit of `main` (`git worktree add --detach`) and see + it under "Left behind" with a press to remove it. + + Human-confirmed 2026-10-14: the owner ran the standalone application from + this branch and reported that it works. Before that, the survey run from + the branch against the owner's repository had marked `C:/Temp/qwerty` (a + detached head at `271bfa56`, clean, contained in `origin/main`) as + `finishedWith: { reason: "merged" }`. diff --git a/packages/core/src/worktree-survey-facts.ts b/packages/core/src/worktree-survey-facts.ts index 6738f9eb..159535d5 100644 --- a/packages/core/src/worktree-survey-facts.ts +++ b/packages/core/src/worktree-survey-facts.ts @@ -157,8 +157,10 @@ interface SurveyedDirectoryBase { * (the-workspace-clears-what-it-left-behind). Never a removal: a * directory whose branch merged can still hold somebody's uncommitted * work, so this is what a press is offered beside, not what a sweep - * acts on. */ - finishedWith?: { reason: "merged" | "branch-gone"; branch: string }; + * acts on. A directory on no branch has none to name: it is finished + * with, as "merged", where the default branch already contains the commit + * it is at (a-detached-directory-is-offered-for-removal). */ + finishedWith?: { reason: "merged" | "branch-gone"; branch?: string }; /** Where this directory's own change is not on the server: its branch was * never pushed, or the server's copy is at another commit - usually one it has not been pushed. Read from * the refs the survey already lists, so no git runs in the directory; a diff --git a/packages/core/src/worktree-survey.test.ts b/packages/core/src/worktree-survey.test.ts index 5106ccef..f8f818d2 100644 --- a/packages/core/src/worktree-survey.test.ts +++ b/packages/core/src/worktree-survey.test.ts @@ -763,6 +763,71 @@ describe("surveyWorktrees - a directory with nothing left to do", () => { }); }); +// a-detached-directory-is-offered-for-removal: a directory on no branch has +// no upstream to be gone, so the one thing asked of it is whether the +// default branch already contains the commit it is at. +describe("surveyWorktrees - a directory on no branch", () => { + function detachedGit(head: string, merged: string[], refs: Record = { "refs/remotes/origin/main": "main-commit" }) { + return { + worktreeList: async (): Promise> => [], + configuredIdentity: async () => "someone@example.com", + listRefs: async () => Object.entries(refs).map(([name, commit]) => ({ name, commit })), + mergeBase: async (_left: string, right: string) => (merged.includes(right) ? right : "0000000"), + head, + }; + } + + async function surveyed(options: { head: string; merged: string[]; clean?: boolean; withRun?: boolean; refs?: Record }) { + const { main, worktreeRoot, rootSources } = await repository(); + const second = path.join(worktreeRoot, "repo", "check"); + await makeChange(main, "change-a"); + await makeChange(second, "change-b"); + if (options.withRun) { + await writeStatus(agentStatusDirectory(worktreeRoot, main), { + instanceId: "run-b", + workingDirectory: second, + changeName: "change-b", + activity: "Bash: npm test", + }); + } + const base = detachedGit(options.head, options.merged, options.refs); + const git = { ...base, worktreeList: async () => [{ path: main, branch: "main" }, { path: second, head: options.head }] }; + return surveyWorktrees({ workspaceRoot: main, git, rootSources, isClean: async () => options.clean ?? true }); + } + + it("says it is finished with, naming no branch, where the default branch contains its commit and its tree is clean", async () => { + const survey = await surveyed({ head: "old-commit", merged: ["old-commit"] }); + + expect(survey.directories[1]?.branch).toBeUndefined(); + expect(survey.directories[1]?.finishedWith).toEqual({ reason: "merged" }); + }); + + it("says nothing where the default branch does not contain its commit", async () => { + const survey = await surveyed({ head: "ahead-commit", merged: [] }); + + expect(survey.directories[1]?.finishedWith).toBeUndefined(); + }); + + it("says nothing where its tree is not clean", async () => { + const survey = await surveyed({ head: "old-commit", merged: ["old-commit"], clean: false }); + + expect(survey.directories[1]?.finishedWith).toBeUndefined(); + }); + + it("says nothing where a run is recorded against it", async () => { + const survey = await surveyed({ head: "old-commit", merged: ["old-commit"], withRun: true }); + + expect(survey.directories[1]?.runs.length).toBeGreaterThan(0); + expect(survey.directories[1]?.finishedWith).toBeUndefined(); + }); + + it("says nothing where the default branch cannot be found in the refs", async () => { + const survey = await surveyed({ head: "old-commit", merged: ["old-commit"], refs: { "refs/remotes/origin/other": "x" } }); + + expect(survey.directories[1]?.finishedWith).toBeUndefined(); + }); +}); + describe("surveyWorktrees - a change the server does not have", () => { // a-change-is-committed-where-it-is-made: read from the refs the survey // lists anyway, so no git runs in the directory. diff --git a/packages/core/src/worktree-survey.ts b/packages/core/src/worktree-survey.ts index d8fffbbb..aab2eece 100644 --- a/packages/core/src/worktree-survey.ts +++ b/packages/core/src/worktree-survey.ts @@ -457,7 +457,7 @@ async function readBranchRefs( git: Partial>, options: WorktreeSurveyOptions, ): Promise | undefined> { - if (git.listRefs === undefined || !directories.some((directory) => !directory.isMain && directory.branch !== undefined)) return undefined; + if (git.listRefs === undefined || !directories.some((directory) => !directory.isMain && (directory.branch !== undefined || directory.head !== undefined))) return undefined; const remote = options.remote ?? "origin"; try { return new Map((await git.listRefs(["refs/heads", `refs/remotes/${remote}`])).map((ref) => [ref.name, ref.commit])); @@ -486,8 +486,10 @@ async function markFinishedDirectories( options: WorktreeSurveyOptions, branchRefs: Map | undefined, ): Promise { + // A directory on no branch is a candidate where it has a commit to ask + // about (a-detached-directory-is-offered-for-removal). const candidates = directories.filter( - (directory) => !directory.isMain && directory.branch !== undefined && directory.runs.length === 0, + (directory) => !directory.isMain && (directory.branch !== undefined || directory.head !== undefined) && directory.runs.length === 0, ); if (candidates.length === 0 || branchRefs === undefined || git.mergeBase === undefined) return; @@ -504,7 +506,23 @@ async function markFinishedDirectories( }); for (const directory of candidates) { - const branch = directory.branch as string; + if (directory.branch === undefined) { + // No branch, so nothing can be gone: the one thing to ask is whether + // the default branch already contains the commit this directory is + // at. It is the answer a merged branch gives, and no other. + const head = directory.head; + if (head === undefined || mainCommit === undefined) continue; + try { + if (await git.mergeBase(mainCommit, head) !== head) continue; + } catch { + continue; + } + if (!await isClean(directory.path)) continue; + const reason = "merged" as const; + directory.finishedWith = { reason }; + continue; + } + const branch = directory.branch; const localCommit = refs.get(`refs/heads/${branch}`); const remoteCommit = refs.get(`refs/remotes/${remote}/${branch}`); let reason: "merged" | "branch-gone" | undefined;