Skip to content

Commit 867e53c

Browse files
suleimanshclaude
andauthored
The run-id branch an agent branched away from goes with its checkout (#1657) (#1658)
* The run-id branch an agent branched away from goes with its checkout (#1657) A run starts on `tf-agent-<id>`; the system prompt then has the agent create `tf-<name>` and check it out, so the rename never applies and the run-id branch stays behind at the commit the run began from — one per run, forever. The rig had nine after two days, two of them pushed to origin by the old teardown. Removal now judges the run-id branch alongside the checkout's: when the checkout ended on another branch that contains it, everything it holds is held again, and it goes with the checkout. One carrying a commit the kept branch lacks stays. Decided before either delete, since the containment reads both refs. The result and the removal lines name every branch that went. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * The guard test branches from the init commit, not from a branch named main (#1657) CI's git init names its first branch master. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 71c7cfa commit 867e53c

6 files changed

Lines changed: 121 additions & 16 deletions

File tree

FEATURES-SPEC.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ happens while nobody is at the keyboard.
129129
- CI watch: merge a PR once its checks pass
130130
- CI watch: one fix agent per red head commit, max two attempts
131131
- Reclaim the checkout of an agent whose work is on the remote — never by publishing what a `handoff: local` agent refused to
132-
- An agent that committed nothing leaves no branch behind: its empty branch goes with its checkout, never pushed, so a pinned routine is not stood down by its own last run
132+
- An agent that committed nothing leaves no branch behind: its empty branch goes with its checkout, never pushed, so a pinned routine is not stood down by its own last run — and the run-id branch it started on goes too, once the branch it moved to holds everything the run-id branch did
133133
- A directory under `branches/` that git does not know as a worktree is never committed, pushed, linked or deleted through — it is reported and left alone, so a leftover can never stand in for your own checkout
134134
- Release a pinned routine branch left behind by a closed PR — before the schedule fires the routine, and before its "Run now" does
135135
- The agent drains its own TODO backlog, one entry per turn

packages/the-framework/src/daemon-runtime.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { tailEvents, tailAgentEvents } from './dashboard-rpc/events-tail.js'
4040
import { resolveUserDir } from './agent-archive.js'
4141
import { withDataBranch } from './data-branch.js'
4242
import { removeProjectWorktree } from './worktrees.js'
43+
import { describeDeleted } from './merged-worktrees.js'
4344
import { reconcileBranchLinks } from './branch-links.js'
4445
import { scopedKey, parseScopedKey, keyBelongsTo } from './runtime-keys.js'
4546
import { addProject, listProjects, projectId } from './registry.js'
@@ -607,8 +608,8 @@ export function createProjectRuntime({ cwd, env, binPath, retryDelayMs, driverPr
607608
// never depends on the caller having one.
608609
const outcome = await removeProjectWorktree(projectCwd, agentId ?? agentIdFromWorktreeDir(basename(worktree)))
609610
if (!outcome.ok) console.log(`[framework] keeping worktree ${worktree}: ${outcome.error}`)
610-
else if (outcome.branchDeleted)
611-
console.log(`[framework] removed worktree ${worktree} and its branch ${outcome.branchDeleted}: the branch held nothing the remote lacks`)
611+
else if (outcome.branchesDeleted)
612+
console.log(`[framework] removed worktree ${worktree} and ${describeDeleted(outcome.branchesDeleted)}: nothing on it is missing elsewhere`)
612613
} catch {
613614
// A worktree we could not retire is a worktree left on disk, which is the safe direction.
614615
}

packages/the-framework/src/merged-worktrees.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ test('the sweep says what it removed and what it kept, per project (#1036)', asy
106106
const lines: string[] = []
107107
const results: Record<string, MergedSweepResult> = {
108108
'/a': { removed: [{ agentId: 'r1' }], failed: [] },
109-
'/b': { removed: [{ agentId: 'r2', branchDeleted: 'tf-triage-quick' }], failed: [{ agentId: 'r3', error: 'not on the remote' }] },
109+
'/b': { removed: [{ agentId: 'r2', branchesDeleted: ['tf-triage-quick', 'tf-agent-r2'] }], failed: [{ agentId: 'r3', error: 'not on the remote' }] },
110110
}
111111
const sweep = startMergedWorktreeSweep({
112112
projects: async () => [{ path: '/a' }, { path: '/b' }],
@@ -117,7 +117,7 @@ test('the sweep says what it removed and what it kept, per project (#1036)', asy
117117
sweep.stop()
118118
assert.match(lines[0] ?? '', /removed the worktree for session r1: its branch is on the remote/)
119119
assert.match(lines[0] ?? '', /The branch and the session are kept/, 'a checkout vanishing silently reads as a bug')
120-
assert.match(lines[1] ?? '', /removed the worktree for session r2 and its branch tf-triage-quick: the branch held nothing/, 'a branch going says so (#1650)')
120+
assert.match(lines[1] ?? '', /removed the worktree for session r2 and its branches tf-triage-quick and tf-agent-r2: nothing on it is missing elsewhere/, 'branches going say so (#1650, #1657)')
121121
assert.match(lines[2] ?? '', /kept the worktree for session r3: not on the remote/)
122122
})
123123

packages/the-framework/src/merged-worktrees.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import { repoHasRemote, worktreePath } from './store/index.js'
2323
export interface RemovedWorktree {
2424
/** The agent id, which is also the worktree's directory name. */
2525
agentId: string
26-
/** The run branch that went with it, when it held nothing the remote lacks (#1650). */
27-
branchDeleted?: string
26+
/** Branches that went with it: one holding nothing the remote lacks (#1650), a run-id branch the kept branch contains (#1657). */
27+
branchesDeleted?: string[]
2828
}
2929

3030
/** A worktree the sweep tried to reclaim and could not, and why. */
@@ -87,12 +87,17 @@ export async function removeMergedWorktrees(cwd: string, deps: MergedSweepDeps =
8787
}
8888
for (const row of rows) {
8989
const outcome = await withAgentLock(worktreePath(cwd, row.agentId), () => remove(cwd, row.agentId))
90-
if (outcome.ok) result.removed.push({ agentId: row.agentId, ...(outcome.branchDeleted ? { branchDeleted: outcome.branchDeleted } : {}) })
90+
if (outcome.ok) result.removed.push({ agentId: row.agentId, ...(outcome.branchesDeleted ? { branchesDeleted: outcome.branchesDeleted } : {}) })
9191
else result.failed.push({ agentId: row.agentId, error: outcome.error })
9292
}
9393
return result
9494
}
9595

96+
/** `its branch x` / `its branches x and y`, for the removal line. */
97+
export function describeDeleted(branches: readonly string[]): string {
98+
return branches.length === 1 ? `its branch ${branches[0]}` : `its branches ${branches.join(' and ')}`
99+
}
100+
96101
/** A running sweep, in the shape the daemon's other background services use. */
97102
export interface MergedWorktreeSweep {
98103
/** Run one sweep now, awaiting it. Exposed for tests and for a caller that wants it on demand. */
@@ -144,8 +149,8 @@ export function startMergedWorktreeSweep(opts: MergedSweepOptions): MergedWorktr
144149
for (const item of removed) {
145150
announced.delete(item.agentId)
146151
opts.log(
147-
item.branchDeleted
148-
? `[framework] removed the worktree for session ${item.agentId} and its branch ${item.branchDeleted}: the branch held nothing the remote lacks. The session is kept.`
152+
item.branchesDeleted
153+
? `[framework] removed the worktree for session ${item.agentId} and ${describeDeleted(item.branchesDeleted)}: nothing on it is missing elsewhere. The session is kept.`
149154
: `[framework] removed the worktree for session ${item.agentId}: its branch is on the remote. The branch and the session are kept.`,
150155
)
151156
}

packages/the-framework/src/worktrees.test.ts

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ test('a run branch holding nothing the remote lacks goes with its checkout, unpu
217217
const now = new Date().toISOString()
218218
await mkdir(join(path, '.the-framework'), { recursive: true })
219219
await writeFile(join(path, '.the-framework', 'agent.json'), JSON.stringify({ status: 'done', id: RUN_ID, startedAt: now, updatedAt: now }))
220-
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true, branchDeleted: branch })
220+
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true, branchesDeleted: [branch] })
221221
await assert.rejects(() => stat(path), 'the checkout is gone')
222222
await assert.rejects(() => git(['rev-parse', '--verify', `refs/remotes/origin/${branch}`], repo), 'nothing reached origin')
223223
await assert.rejects(() => git(['rev-parse', '--verify', `refs/heads/${branch}`], repo), 'and the branch went with the checkout')
@@ -276,7 +276,9 @@ test("a leftover checkout on a branch the framework did not mint keeps that bran
276276
await git(['checkout', '-q', '-b', 'release'], path)
277277
await mkdir(join(repo, '.git', 'info'), { recursive: true })
278278
await writeFile(join(repo, '.git', 'info', 'exclude'), '.the-framework/\n')
279-
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true })
279+
// The user's branch stays; the run-id branch it was cut from is ours and holds nothing
280+
// `release` does not, so that one goes (#1657).
281+
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true, branchesDeleted: [agentBranchName(RUN_ID)] })
280282
await assert.rejects(() => stat(path), 'the checkout is gone')
281283
await git(['rev-parse', '--verify', 'refs/heads/release'], repo)
282284
} finally {
@@ -311,6 +313,75 @@ test("a branches/ directory that is not a git worktree is refused before any git
311313
}
312314
})
313315

316+
test('the run-id branch the agent branched away from goes with the checkout when the kept branch contains it (#1657)', async () => {
317+
// The system prompt has the agent create `tf-<name>` rather than be renamed onto it, so every
318+
// run leaves `tf-agent-<id>` behind at the commit it started from — nine of them on the rig.
319+
const { repo, path, branch: runBranch } = await repoWithDirtyWorktree()
320+
const git = nodeGitRunner()
321+
try {
322+
await git(['push', '-q', 'origin', 'HEAD:main'], repo)
323+
await git(['checkout', '-q', '-b', 'tf-cool-name'], path)
324+
await git(['config', 'user.email', 't@t'], path)
325+
await git(['config', 'user.name', 't'], path)
326+
await git(['add', '-A'], path)
327+
await git(['commit', '-q', '-m', 'work'], path)
328+
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true, branchesDeleted: [runBranch] })
329+
assert.match(await git(['show', 'tf-cool-name:index.html'], repo), /Welcome!/, 'the work branch stays, pushed')
330+
assert.match(await git(['show', 'refs/remotes/origin/tf-cool-name:index.html'], repo), /Welcome!/)
331+
await assert.rejects(() => git(['rev-parse', '--verify', `refs/heads/${runBranch}`], repo), 'the run-id branch is gone')
332+
await assert.rejects(() => git(['rev-parse', '--verify', `refs/remotes/origin/${runBranch}`], repo), 'and was never pushed')
333+
} finally {
334+
await rm(repo, { recursive: true, force: true })
335+
}
336+
})
337+
338+
test('a commitless triage leaves neither its pinned branch nor its run-id branch (#1650, #1657)', async () => {
339+
// The rig case in full: the checkout ends on an empty `tf-triage-quick`, branched off an
340+
// equally empty run-id branch. Both hold nothing; both go; nothing is pushed.
341+
const { repo, path, branch: runBranch } = await repoWithDirtyWorktree()
342+
const git = nodeGitRunner()
343+
try {
344+
await git(['push', '-q', 'origin', 'HEAD:main'], repo)
345+
await git(['checkout', '--', '.'], path)
346+
await git(['checkout', '-q', '-b', 'tf-triage-quick'], path)
347+
await mkdir(join(repo, '.git', 'info'), { recursive: true })
348+
await writeFile(join(repo, '.git', 'info', 'exclude'), '.the-framework/\n')
349+
const now = new Date().toISOString()
350+
await mkdir(join(path, '.the-framework'), { recursive: true })
351+
await writeFile(join(path, '.the-framework', 'agent.json'), JSON.stringify({ status: 'done', id: RUN_ID, startedAt: now, updatedAt: now }))
352+
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true, branchesDeleted: ['tf-triage-quick', runBranch] })
353+
assert.equal((await git(['branch', '--list', 'tf-*'], repo)).trim(), '', 'no tf- branch is left')
354+
assert.equal((await git(['ls-remote', '--heads', 'origin', 'tf-*'], repo)).trim(), '', 'and none reached origin')
355+
} finally {
356+
await rm(repo, { recursive: true, force: true })
357+
}
358+
})
359+
360+
test('a run-id branch carrying a commit the kept branch lacks stays (#1657)', async () => {
361+
// The agent committed on the run-id branch, then branched from main and went on from there.
362+
// The run-id branch holds something the kept branch does not, so it is not the framework's
363+
// to delete.
364+
const { repo, path, branch: runBranch } = await repoWithDirtyWorktree()
365+
const git = nodeGitRunner()
366+
try {
367+
await git(['push', '-q', 'origin', 'HEAD:main'], repo)
368+
await git(['config', 'user.email', 't@t'], path)
369+
await git(['config', 'user.name', 't'], path)
370+
await git(['add', '-A'], path)
371+
await git(['commit', '-q', '-m', 'early work on the run-id branch'], path)
372+
// From the init commit, not from `main`: CI's `git init` names its branch `master`.
373+
const init = (await git(['rev-parse', 'HEAD'], repo)).trim()
374+
await git(['checkout', '-q', '-b', 'tf-other', init], path)
375+
await writeFile(join(path, 'other.txt'), 'later\n')
376+
await git(['add', '-A'], path)
377+
await git(['commit', '-q', '-m', 'later work elsewhere'], path)
378+
assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true })
379+
assert.match(await git(['show', `${runBranch}:index.html`], repo), /Welcome!/, 'the early commit is still on the run-id branch')
380+
} finally {
381+
await rm(repo, { recursive: true, force: true })
382+
}
383+
})
384+
314385
test('a worktree whose work cannot be committed is refused, not force-removed (#982)', async () => {
315386
const { repo, path } = await repoWithDirtyWorktree()
316387
try {

packages/the-framework/src/worktrees.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
worktreeClean,
1111
isWorktreeRoot,
1212
currentBranch,
13+
agentBranchName,
1314
removeWorktree,
1415
deleteBranch,
1516
pruneWorktrees,
@@ -57,8 +58,12 @@ export interface PruneResult {
5758
export type RemoveResult =
5859
| {
5960
ok: true
60-
/** The run branch went with the checkout, because it held nothing the remote lacks (#1650). */
61-
branchDeleted?: string
61+
/**
62+
* Branches that went with the checkout: the branch it was on, when that held nothing the
63+
* remote lacks (#1650); the run-id branch the agent branched away from, when everything on
64+
* it is in the branch that stays (#1657). Absent when nothing went.
65+
*/
66+
branchesDeleted?: string[]
6267
}
6368
| { ok: false; error: string }
6469

@@ -231,11 +236,25 @@ export async function removeProjectWorktree(
231236
await removeWorktree(cwd, path)
232237
await pruneWorktrees(cwd)
233238
// After the checkout: git refuses to delete a branch a worktree still has checked out.
239+
// The run-id branch the run started on (#1657). The system prompt has the agent *create*
240+
// `tf-<name>` rather than be renamed onto it, so the checkout ends elsewhere and the run-id
241+
// branch stays behind at the commit the run began from — one per run, forever. It goes when
242+
// the checkout's branch contains it: then everything it holds is held again by a branch that
243+
// either stays or (an empty one, above) is itself inside the remote. A run-id branch the agent
244+
// committed on and then abandoned for a branch from elsewhere is not contained, and stays.
245+
// Decided before anything is deleted: the containment reads both refs.
246+
const runBranch = agentBranchName(agentId)
247+
const runBranchGoes = runBranch !== branch && (await branchContains(cwd, branch, runBranch))
248+
const deleted: string[] = []
234249
if (emptyBranch) {
235250
await deleteBranch(cwd, branch)
236-
return { ok: true, branchDeleted: branch }
251+
deleted.push(branch)
237252
}
238-
return { ok: true }
253+
if (runBranchGoes) {
254+
await deleteBranch(cwd, runBranch)
255+
deleted.push(runBranch)
256+
}
257+
return deleted.length ? { ok: true, branchesDeleted: deleted } : { ok: true }
239258
} catch (err) {
240259
return { ok: false, error: errorMessage(err) }
241260
}
@@ -276,6 +295,15 @@ async function branchHoldsNothing(cwd: string, path: string, branch: string): Pr
276295
)
277296
}
278297

298+
/** Whether `inner` exists and is an ancestor of (or equal to) `outer` — everything on it is on `outer` too. */
299+
async function branchContains(cwd: string, outer: string, inner: string): Promise<boolean> {
300+
const git = nodeGitRunner()
301+
return git(['merge-base', '--is-ancestor', `refs/heads/${inner}`, `refs/heads/${outer}`], cwd).then(
302+
() => true,
303+
() => false,
304+
)
305+
}
306+
279307
/**
280308
* The meta the keep decision reads: the live copy in the checkout, else the archived one.
281309
*

0 commit comments

Comments
 (0)