From 2dcb2a9c9130c3f8ff9581dbc4f586c84da12610 Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Thu, 20 Aug 2026 02:35:55 +0300 Subject: [PATCH 1/2] Cloud runs' work is adopted onto the run's record (fix #1601) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A web run's cloud session works on a claude/* branch of its own naming, never the designated run branch, so every surface keyed to the recorded branch said "nothing committed" while the work sat on origin — and the armed draft PR never opened. The hand-off now pushes an anchor (an empty commit unique to the run, minted without moving any branch) as the ref the session clones at, so the session's branch is recognizable later by plain ancestry. A daemon pass matches each settled web run to the one claude/* head descending from its anchor and patches the run's archive with the branch and its PR — opening the armed draft PR itself when the session never did. Web-run teardown stops pushing the empty tf-agent-* branch to origin; the scratch-ref sweep clears the ones already there and learns that an anchor tip (an empty commit on a landed parent) holds no work. --- .changeset/adopt-cloud-work.md | 5 + FEATURES-SPEC.md | 1 + .../the-framework/src/agent-telemetry.test.ts | 15 + packages/the-framework/src/agent-telemetry.ts | 3 + .../src/cloud-scratch-refs.SPEC.md | 3 +- .../src/cloud-scratch-refs.test.SPEC.md | 2 +- .../src/cloud-scratch-refs.test.ts | 62 +++++ .../the-framework/src/cloud-scratch-refs.ts | 36 ++- packages/the-framework/src/cloud-work.SPEC.md | 13 + .../the-framework/src/cloud-work.test.SPEC.md | 5 + packages/the-framework/src/cloud-work.test.ts | 193 +++++++++++++ packages/the-framework/src/cloud-work.ts | 262 ++++++++++++++++++ .../the-framework/src/daemon-services.SPEC.md | 1 + packages/the-framework/src/daemon-services.ts | 14 +- .../src/dashboard/agent-handoff.SPEC.md | 1 + .../src/dashboard/agent-handoff.test.SPEC.md | 2 +- .../src/dashboard/agent-handoff.test.ts | 34 ++- .../src/dashboard/agent-handoff.ts | 28 ++ .../the-framework/src/driver/cloud.SPEC.md | 2 +- .../src/driver/cloud.test.SPEC.md | 2 +- .../the-framework/src/driver/cloud.test.ts | 45 ++- packages/the-framework/src/driver/cloud.ts | 34 ++- packages/the-framework/src/driver/types.ts | 5 +- packages/the-framework/src/events.SPEC.md | 2 +- packages/the-framework/src/events.ts | 8 + .../src/store/agent-store.SPEC.md | 2 +- .../src/store/agent-store.test.ts | 16 ++ .../the-framework/src/store/agent-store.ts | 38 +++ packages/the-framework/src/store/index.ts | 1 + packages/the-framework/src/terminal.ts | 2 + packages/the-framework/src/worktrees.SPEC.md | 1 + .../the-framework/src/worktrees.test.SPEC.md | 2 +- packages/the-framework/src/worktrees.test.ts | 54 ++++ packages/the-framework/src/worktrees.ts | 26 +- 34 files changed, 895 insertions(+), 25 deletions(-) create mode 100644 .changeset/adopt-cloud-work.md create mode 100644 packages/the-framework/src/cloud-work.SPEC.md create mode 100644 packages/the-framework/src/cloud-work.test.SPEC.md create mode 100644 packages/the-framework/src/cloud-work.test.ts create mode 100644 packages/the-framework/src/cloud-work.ts diff --git a/.changeset/adopt-cloud-work.md b/.changeset/adopt-cloud-work.md new file mode 100644 index 000000000..18e935f7c --- /dev/null +++ b/.changeset/adopt-cloud-work.md @@ -0,0 +1,5 @@ +--- +'@gemstack/the-framework': minor +--- + +A cloud run's record now follows the branch its session actually worked on (#1601). A web run hands the task to claude.ai and ends; the session does the work on a `claude/*` branch of the cloud's own naming, never the designated run branch — so every surface keyed to the recorded branch (the session row, PR resolution, CI watch, merge) said "nothing committed" while the work sat on origin, and the armed draft PR never opened. The hand-off now pushes an anchor — an empty commit unique to the run, minted without moving any branch — as the ref the session clones at, so the session's branch is recognizable later by plain ancestry. A daemon pass matches each settled web run to the one `claude/*` head descending from its anchor and patches the run's archive with the branch and its PR, opening the armed draft PR itself when the session never did. Web-run teardown stops pushing the empty `tf-agent-*` branch to origin (the scratch-ref sweep clears the ones already there, and learns that an anchor tip — an empty commit on a landed parent — holds no work). diff --git a/FEATURES-SPEC.md b/FEATURES-SPEC.md index 65be8b32d..7fefe1d69 100644 --- a/FEATURES-SPEC.md +++ b/FEATURES-SPEC.md @@ -157,6 +157,7 @@ happens while nobody is at the keyboard. - Answer a cloud agent's question from the dashboard (typed back into claude.ai) - Browser-bridge token setting - Web runs trust the project for Claude Code automatically — no manual trust step +- A cloud run's row follows the session's real branch and PR, with its armed draft PR opened when the session opens none ## Notifications diff --git a/packages/the-framework/src/agent-telemetry.test.ts b/packages/the-framework/src/agent-telemetry.test.ts index 7736b3ea3..6d9c18a7b 100644 --- a/packages/the-framework/src/agent-telemetry.test.ts +++ b/packages/the-framework/src/agent-telemetry.test.ts @@ -40,6 +40,21 @@ test('the result repeating the announced id does not emit a second session-updat assert.equal(events.filter(e => e.kind === 'session-update').length, 1) }) +test('a result carrying the hand-off anchor emits it as its own event (#1601)', () => { + // The anchor is how the daemon later recognizes which `claude/*` branch is this run's, and + // only an event reaches the meta the daemon reads after this process is gone. + const events: FrameworkEvent[] = [] + handler(events).onDriverEvent({ type: 'result', text: 'done', sessionId: 's1', anchorSha: 'a'.repeat(40) }) + assert.deepEqual( + events.filter(e => e.kind === 'cloud-anchor'), + [{ kind: 'cloud-anchor', sha: 'a'.repeat(40) }], + ) + // And a result without one — every non-cloud driver — emits none. + const bare: FrameworkEvent[] = [] + handler(bare).onDriverEvent({ type: 'result', text: 'done' }) + assert.equal(bare.filter(e => e.kind === 'cloud-anchor').length, 0) +}) + test('a result with a fresh id still emits, the pre-#1322 path unchanged', () => { const events: FrameworkEvent[] = [] handler(events).onDriverEvent({ type: 'result', text: 'done', sessionId: 's2' }) diff --git a/packages/the-framework/src/agent-telemetry.ts b/packages/the-framework/src/agent-telemetry.ts index f42bd1d13..8addff5b5 100644 --- a/packages/the-framework/src/agent-telemetry.ts +++ b/packages/the-framework/src/agent-telemetry.ts @@ -79,6 +79,9 @@ export function createDriverEventHandler(opts: DriverEventHandlerOptions): Drive } emit({ kind: 'driver', event }) if (event.type !== 'result') return + // The hand-off anchor (#1601) reaches the meta the same way the session id does: it is a + // fact about the run the daemon needs after this process is gone, so only an event carries it. + if (event.anchorSha) emit({ kind: 'cloud-anchor', sha: event.anchorSha }) if (event.sessionId && event.sessionId !== lastSessionId) { lastSessionId = event.sessionId // A driver that knows its session's real URL (#1317, the cloud hand-off) beats the diff --git a/packages/the-framework/src/cloud-scratch-refs.SPEC.md b/packages/the-framework/src/cloud-scratch-refs.SPEC.md index 2a32fbac1..eeb2d50ea 100644 --- a/packages/the-framework/src/cloud-scratch-refs.SPEC.md +++ b/packages/the-framework/src/cloud-scratch-refs.SPEC.md @@ -2,9 +2,10 @@ Deletes the two dead refs every Claude-web hand-off leaves on origin — the pre ## TLDR -- A web run pushes a `cloud-*` ref for the cloud session to clone at, and its run branch reaches origin when the worktree is reclaimed. The session then works on its own branch and opens its PR from there, so nothing ever consumes either ref again. +- A web run pushes a `cloud-*` ref for the cloud session to clone at; the session then works on its own branch and opens its PR from there, so nothing ever consumes the ref again. Run branches used to reach origin too, pushed empty when a web run's worktree was reclaimed — teardown no longer pushes them, and the sweep clears the ones already there. - The driver must not delete its own ref: it only learns "session created", never "clone finished", and a ref deleted in between strands the session. So the daemon sweeps instead, hourly, and waits out the race. - A ref goes only when every gate clears: it is about a day old, its commits are already on the default branch (the proof it holds no work — this is what protects a local run's branch carrying unmerged commits), it has no open pull request, and its agent is not one the daemon is still running. +- The hand-off anchor is the one tip the default branch never absorbs — an empty commit no merge ever lands — so it clears the work gate its own way: a tip that changes nothing against its parent, on a parent that landed, holds no work. - A run branch's age is in its name; a `cloud-*` ref's is not, so the sweep remembers when it first saw one and ages it from there — which also keeps refs pushed by another machine safe, since each machine only deletes what it has itself watched for a day. - Conservative and quiet: anything unprovable simply stays for the next pass, and only actual deletions (and failures) are announced. diff --git a/packages/the-framework/src/cloud-scratch-refs.test.SPEC.md b/packages/the-framework/src/cloud-scratch-refs.test.SPEC.md index 58911655f..917f9bf93 100644 --- a/packages/the-framework/src/cloud-scratch-refs.test.SPEC.md +++ b/packages/the-framework/src/cloud-scratch-refs.test.SPEC.md @@ -1,4 +1,4 @@ -Covers the cloud-scratch sweep: only the driver's exact ref shapes are ever candidates, each safety gate (age, work landed, open PR, busy agent) keeps a ref on its own, a `cloud-*` ref is first watched for a day before it may go, records are pruned when refs disappear, a refused deletion is retried without restarting the clock, a repo without a remote sweeps nothing, and the daemon-facing service announces deletions and failures but not kept refs. +Covers the cloud-scratch sweep: only the driver's exact ref shapes are ever candidates, each safety gate (age, work landed, open PR, busy agent) keeps a ref on its own, a hand-off anchor tip (an empty commit on a landed parent) still counts as landed while a tip that changes anything does not, a `cloud-*` ref is first watched for a day before it may go, records are pruned when refs disappear, a refused deletion is retried without restarting the clock, a repo without a remote sweeps nothing, and the daemon-facing service announces deletions and failures but not kept refs. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/cloud-scratch-refs.test.ts b/packages/the-framework/src/cloud-scratch-refs.test.ts index e87f5c5b4..539851977 100644 --- a/packages/the-framework/src/cloud-scratch-refs.test.ts +++ b/packages/the-framework/src/cloud-scratch-refs.test.ts @@ -37,6 +37,8 @@ function fakeGit(opts: { defaultBranch?: string /** The shas reachable from the default branch ('all' for every one). Default 'all'. */ landed?: Set | 'all' + /** Locally-known commits (#1601): sha -> its tree and parent. Everything else is not local. */ + commits?: Record refuseDeletes?: boolean noRemote?: boolean }) { @@ -54,6 +56,29 @@ function fakeGit(opts: { if (landed === 'all' || landed.has(args[2]!)) return '' throw new Error('exit 1') // not an ancestor } + if (args[0] === 'cat-file') { + const sha = /^([0-9a-f]+)\^\{commit\}$/.exec(args[2] ?? '')?.[1] + if (sha && opts.commits?.[sha]) return '' + throw new Error('exit 1') // not a local object + } + if (args[0] === 'rev-parse') { + const match = /^([0-9a-f]+)(\^?)(?:\^\{tree\})?$/.exec(args[1] ?? '') + const commit = match ? opts.commits?.[match[1]!] : undefined + if (!match || !commit) throw new Error('exit 128') + const spec = args[1]! + if (spec.endsWith('^^{tree}')) { + const parent = commit.parent ? opts.commits?.[commit.parent] : undefined + if (!parent) throw new Error('exit 128') + return `${parent.tree}\n` + } + if (spec.endsWith('^{tree}')) return `${commit.tree}\n` + if (spec.endsWith('^')) { + if (!commit.parent) throw new Error('exit 128') + return `${commit.parent}\n` + } + throw new Error(`unexpected rev-parse ${spec}`) + } + if (args[0] === 'fetch') throw new Error('exit 128') // the canned origin serves no objects if (args[0] === 'push' && args[2] === '--delete') { if (opts.refuseDeletes) throw new Error('remote hung up') deleted.push(args[3]!) @@ -189,6 +214,43 @@ test('a cloud-* ref watched for less than the safe age is kept', async () => { assert.deepEqual(result.kept, [{ ref: CLOUD_REF, reason: 'young' }]) }) +test('a hand-off anchor tip still counts as holding no work: empty commit, landed parent (#1601)', async () => { + // The anchor is an empty commit on top of the run's HEAD, and no merge ever lands it — a + // squash merge rewrites the session's history without it — so reachability alone would keep + // its ref forever. + const ANCHOR_SHA = 'c'.repeat(40) + const { git, deleted } = fakeGit({ + heads: { main: MAIN_SHA, [CLOUD_REF]: ANCHOR_SHA }, + landed: new Set([SHA, MAIN_SHA]), + commits: { + [ANCHOR_SHA]: { tree: 'tree1', parent: SHA }, + [SHA]: { tree: 'tree1' }, + }, + }) + const { files, fs } = memFs() + seenState(files, { [CLOUD_REF]: new Date(NOW - SCRATCH_REF_SAFE_AGE_MS).toISOString() }) + const result = await sweepCloudScratchRefs('/repo', { git, fs, prs: noPrs, now: () => NOW }) + assert.deepEqual(result.deleted, [CLOUD_REF]) + assert.deepEqual(deleted, [CLOUD_REF]) +}) + +test('a tip that changes something against its parent is not an anchor: kept as holding work (#1601)', async () => { + const TIP = 'c'.repeat(40) + const { git, deleted } = fakeGit({ + heads: { main: MAIN_SHA, [CLOUD_REF]: TIP }, + landed: new Set([SHA, MAIN_SHA]), + commits: { + [TIP]: { tree: 'tree2', parent: SHA }, + [SHA]: { tree: 'tree1' }, + }, + }) + const { files, fs } = memFs() + seenState(files, { [CLOUD_REF]: new Date(NOW - SCRATCH_REF_SAFE_AGE_MS).toISOString() }) + const result = await sweepCloudScratchRefs('/repo', { git, fs, prs: noPrs, now: () => NOW }) + assert.deepEqual(deleted, []) + assert.deepEqual(result.kept, [{ ref: CLOUD_REF, reason: 'holds-work' }]) +}) + test('a record for a ref no longer on origin is pruned — someone else already deleted it', async () => { const { git } = fakeGit({ heads: { main: MAIN_SHA } }) const { files, fs } = memFs() diff --git a/packages/the-framework/src/cloud-scratch-refs.ts b/packages/the-framework/src/cloud-scratch-refs.ts index bc6cfa9d1..d920bf484 100644 --- a/packages/the-framework/src/cloud-scratch-refs.ts +++ b/packages/the-framework/src/cloud-scratch-refs.ts @@ -190,6 +190,37 @@ async function landed(git: GitRunner, cwd: string, sha: string, defaultBranch: s return false } +/** + * Whether `sha` is an empty commit sitting on a landed parent — the hand-off anchor's shape + * (#1601): its tree is its parent's tree, so it holds no work of its own, and the parent being + * on the default branch means everything under it landed. The commit object may not be local + * (another machine pushed the ref), so the ref is fetched first when needed; anything still + * unprovable reads as "holds work", which keeps the ref for a later sweep. + */ +async function emptyTipOnLandedParent( + git: GitRunner, + cwd: string, + ref: string, + sha: string, + defaultBranch: string | undefined, + heads: RemoteHead[], +): Promise { + const present = await git(['cat-file', '-e', `${sha}^{commit}`], cwd).then( + () => true, + () => false, + ) + if (!present && !(await git(['fetch', 'origin', `refs/heads/${ref}`], cwd).then(() => true, () => false))) return false + try { + const tree = (await git(['rev-parse', `${sha}^{tree}`], cwd)).trim() + const parent = (await git(['rev-parse', `${sha}^`], cwd)).trim() + const parentTree = (await git(['rev-parse', `${sha}^^{tree}`], cwd)).trim() + if (!tree || !parent || tree !== parentTree) return false + return await landed(git, cwd, parent, defaultBranch, heads) + } catch { + return false + } +} + /** * Sweep one repo's origin for the dead refs cloud hand-offs left behind (#1547), deleting the * ones that clear every gate. Never throws: a repo with no remote (or offline) sweeps nothing, @@ -255,7 +286,10 @@ export async function sweepCloudScratchRefs(cwd: string, deps: ScratchSweepDeps for (const { ref, sha } of candidates) { // The work gate first: it is local and free, and it is the one that must never be wrong. - if (!(await landed(git, cwd, sha, defaultBranch, heads))) { + // A tip the default branch never absorbs still clears it when it is a hand-off anchor + // (#1601): an empty commit whose parent landed changes nothing, and no merge ever lands + // the anchor itself — a squash merge rewrites the session's history without it. + if (!(await landed(git, cwd, sha, defaultBranch, heads)) && !(await emptyTipOnLandedParent(git, cwd, ref, sha, defaultBranch, heads))) { result.kept.push({ ref, reason: 'holds-work' }) continue } diff --git a/packages/the-framework/src/cloud-work.SPEC.md b/packages/the-framework/src/cloud-work.SPEC.md new file mode 100644 index 000000000..3de62cb3f --- /dev/null +++ b/packages/the-framework/src/cloud-work.SPEC.md @@ -0,0 +1,13 @@ +Adopts the branch a cloud session actually worked on: each settled web run is matched to the `claude/*` branch that grew out of its hand-off, and that branch — and its pull request — is recorded on the run. + +## TLDR + +- A web run hands the task to claude.ai and ends; the cloud session does the work on a branch of its own naming, never the branch the run was born on. Without adoption, every surface keyed to the run's branch — its dashboard row, its PR, CI watch, merge — stared at an empty branch and said "nothing committed" while the work sat on origin. +- The match is exact, never guessed: the hand-off pushed a commit unique to the run for the session to clone at, so the session's branch — and only it — descends from that commit. A run matching no branch (the session has not pushed, or never will) or more than one is simply asked again next pass, and a run past the window (two days) stops being asked about. +- What gets recorded: the branch, and the pull request the session opened for it. A run that was armed for a PR the session never opened gets its draft PR opened by the daemon — the armed handoff finally resolving against the facts — unless the branch carries nothing beyond the hand-off itself. +- Daemon-side by necessity: the branch does not exist yet when the run's own process ends — the cloud VM is still provisioning — so a later pass patches the run's record, the same way a late-opened PR already is. +- Adoptions and failures are said out loud; a run still waiting is not, because waiting is its normal state. + +## Before modifying/creating SPEC.md files + +You must always read and respect https://raw.githubusercontent.com/brillout/sdd/refs/heads/main/sdd.md diff --git a/packages/the-framework/src/cloud-work.test.SPEC.md b/packages/the-framework/src/cloud-work.test.SPEC.md new file mode 100644 index 000000000..ceeb2e0cf --- /dev/null +++ b/packages/the-framework/src/cloud-work.test.SPEC.md @@ -0,0 +1,5 @@ +Covers cloud-work adoption: the one branch descending from a run's hand-off anchor is adopted with the PR the session opened, a run armed for a PR the session never opened gets its draft opened and recorded, an unarmed run gets only its branch, a branch carrying nothing beyond the hand-off gets no PR, zero or two matching branches adopt nothing, runs outside the pass (non-web, live, anchorless, already answered, past the window) cost not even a remote listing, an unreachable remote never throws, and the daemon-facing service announces adoptions and failures while joining overlapping ticks. + +## Before modifying/creating SPEC.md files + +You must always read and respect https://raw.githubusercontent.com/brillout/sdd/refs/heads/main/sdd.md diff --git a/packages/the-framework/src/cloud-work.test.ts b/packages/the-framework/src/cloud-work.test.ts new file mode 100644 index 000000000..bc02f6db8 --- /dev/null +++ b/packages/the-framework/src/cloud-work.test.ts @@ -0,0 +1,193 @@ +import { strict as assert } from 'node:assert' +import { test } from 'node:test' +import { adoptCloudWork, startCloudWorkAdoption, CLOUD_ADOPTION_WINDOW_MS, type CloudWorkDeps, type CloudWorkResult } from './cloud-work.js' +import type { AgentMeta } from './store/index.js' +import type { LinkedPr } from './dashboard/gh.js' + +// #1601: a web run's work lands on the cloud session's own `claude/*` branch, which this pass +// recognizes by ancestry from the run's hand-off anchor and records onto the run's archive. + +const CWD = '/repo' +const AT = '2026-08-20T10:00:00.000Z' +const ID = '2026-08-20T10-00-00-000Z' +const NOW = Date.parse(AT) + 60 * 60 * 1000 // an hour after the run started +const ANCHOR = 'a'.repeat(40) +const HEAD_SHA = 'b'.repeat(40) + +function webRun(over: Partial = {}): AgentMeta { + return { + id: ID, + status: 'done', + startedAt: AT, + updatedAt: AT, + target: 'web', + branch: `tf-agent-${ID}`, + cloudAnchor: ANCHOR, + ...over, + } +} + +/** + * A git runner speaking just enough of the pass's dialect: `ls-remote` answers with `heads`, + * `cat-file -e` says every object is local, and `merge-base --is-ancestor` answers from + * `descends` (anchor -> the head shas descending from it). + */ +function fakeGit(heads: { ref: string; sha: string }[], descends: Record = { [ANCHOR]: [HEAD_SHA] }) { + const calls: string[][] = [] + const run = async (args: string[], _cwd: string): Promise => { + calls.push([...args]) + if (args[0] === 'ls-remote') return heads.map(h => `${h.sha}\trefs/heads/${h.ref}`).join('\n') + if (args[0] === 'cat-file') return '' + if (args[0] === 'merge-base') { + const [anchor, sha] = [args[2]!, args[3]!] + if (descends[anchor]?.includes(sha)) return '' + throw new Error('not an ancestor') + } + throw new Error(`unexpected git ${args.join(' ')}`) + } + return { calls, run } +} + +interface Recorded { + branches: { agentId: string; branch: string }[] + prs: { agentId: string; number: number }[] + opened: string[] +} + +function deps(agents: AgentMeta[], git = fakeGit([{ ref: 'claude/fix-it', sha: HEAD_SHA }]), prs: LinkedPr[] = []) { + const recorded: Recorded = { branches: [], prs: [], opened: [] } + const d: CloudWorkDeps = { + git: git.run, + agents: async () => agents, + prs: async () => prs, + adoptBranch: async (_cwd, agentId, branch) => { + recorded.branches.push({ agentId, branch }) + return true + }, + recordPr: async (_cwd, agentId, pr) => { + recorded.prs.push({ agentId, number: pr.number }) + return true + }, + openPr: async (_cwd, _agent, branch) => { + recorded.opened.push(branch) + return { ok: true, url: 'https://x/pull/7', number: 7 } + }, + now: () => NOW, + } + return { d, recorded, git } +} + +test('the head descending from the run anchor is adopted as its branch, with the PR the session opened (#1601)', async () => { + const pr: LinkedPr = { number: 42, url: 'https://x/pull/42', state: 'OPEN', title: 't', createdAt: new Date(NOW).toISOString() } + const { d, recorded } = deps([webRun()], fakeGit([{ ref: 'claude/fix-it', sha: HEAD_SHA }]), [pr]) + const result = await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, [{ agentId: ID, branch: 'claude/fix-it' }]) + assert.deepEqual(recorded.prs, [{ agentId: ID, number: 42 }]) + assert.deepEqual(recorded.opened, [], 'the session already opened its PR, so none is opened here') + assert.deepEqual(result.adopted, [{ agentId: ID, branch: 'claude/fix-it', pr: { number: 42, url: 'https://x/pull/42' } }]) +}) + +test('a run armed for a PR the session never opened gets its draft PR opened and recorded (#1601)', async () => { + const { d, recorded } = deps([webRun()]) + const result = await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.opened, ['claude/fix-it']) + assert.deepEqual(recorded.prs, [{ agentId: ID, number: 7 }]) + assert.equal(result.adopted[0]?.opened, true) +}) + +test('an unarmed run gets its branch recorded and nothing opened (#1601)', async () => { + const { d, recorded } = deps([webRun({ handoff: { push: true, pr: false } })]) + await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, [{ agentId: ID, branch: 'claude/fix-it' }]) + assert.deepEqual(recorded.opened, []) + assert.deepEqual(recorded.prs, []) +}) + +test('a head that is just the anchor gets no PR: the session pushed nothing beyond the hand-off (#1601)', async () => { + const { d, recorded } = deps([webRun()], fakeGit([{ ref: 'claude/empty', sha: ANCHOR }], { [ANCHOR]: [ANCHOR] })) + await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, [{ agentId: ID, branch: 'claude/empty' }]) + assert.deepEqual(recorded.opened, [], 'a PR over nothing helps nobody') +}) + +test('zero or two matching heads adopt nothing: unmatched is the normal waiting state (#1601)', async () => { + const none = deps([webRun()], fakeGit([{ ref: 'claude/other', sha: HEAD_SHA }], { [ANCHOR]: [] })) + assert.deepEqual((await adoptCloudWork(CWD, none.d)).adopted, []) + assert.deepEqual(none.recorded.branches, []) + + const two = deps( + [webRun()], + fakeGit( + [ + { ref: 'claude/one', sha: HEAD_SHA }, + { ref: 'claude/two', sha: 'c'.repeat(40) }, + ], + { [ANCHOR]: [HEAD_SHA, 'c'.repeat(40)] }, + ), + ) + assert.deepEqual((await adoptCloudWork(CWD, two.d)).adopted, [], 'ancestry alone cannot arbitrate two descendants') +}) + +test('runs outside the pass: non-web, still running, no anchor, already adopted, too old (#1601)', async () => { + const settled = [ + webRun({ target: 'local' }), + webRun({ status: 'running' }), + // No anchor recorded (the pre-hand-off push failed): nothing to match by. + (({ cloudAnchor: _drop, ...rest }) => rest)(webRun()), + // Adopted with its PR recorded: fully answered. + webRun({ branch: 'claude/done', pr: { number: 1, url: 'u' } }), + // Adopted, unarmed: the branch was the whole answer. + webRun({ branch: 'claude/done', handoff: { push: true, pr: false } }), + // Older than the window: no longer asked about. + webRun({ startedAt: new Date(NOW - CLOUD_ADOPTION_WINDOW_MS - 1000).toISOString() }), + ] + const git = fakeGit([{ ref: 'claude/fix-it', sha: HEAD_SHA }]) + const { d, recorded } = deps(settled, git) + await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, []) + assert.deepEqual(git.calls, [], 'nothing waiting means not even an ls-remote') +}) + +test('a run already adopted but still owed its armed PR keeps being asked about (#1601)', async () => { + // The session pushed its branch but had not opened a PR when the branch was adopted; a later + // pass finds the PR (or opens the armed draft) without re-recording the branch. + const { d, recorded } = deps([webRun({ branch: 'claude/fix-it' })]) + await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, [], 'the branch is already on the record') + assert.deepEqual(recorded.opened, ['claude/fix-it']) +}) + +test('no remote, or a remote that cannot be reached, adopts nothing and never throws (#1601)', async () => { + const git = { + run: async (args: string[]): Promise => { + if (args[0] === 'ls-remote') throw new Error('no remote') + throw new Error('unexpected') + }, + } + const { d } = deps([webRun()]) + const result = await adoptCloudWork(CWD, { ...d, git: git.run }) + assert.deepEqual(result, { adopted: [], failed: [] }) +}) + +test('startCloudWorkAdoption says adoptions out loud and joins overlapping ticks (#1601)', async () => { + const lines: string[] = [] + let calls = 0 + const adoption = startCloudWorkAdoption({ + projects: async () => [{ path: CWD }], + log: line => lines.push(line), + adopt: async (): Promise => { + calls++ + return { + adopted: [{ agentId: ID, branch: 'claude/fix-it', pr: { number: 7, url: 'https://x/pull/7' }, opened: true }], + failed: [{ agentId: 'other', error: 'boom' }], + } + }, + }) + await Promise.all([adoption.tick(), adoption.tick()]) + assert.equal(calls, 1, 'overlapping ticks join the pass already running') + assert.ok(lines.some(l => l.includes('claude/fix-it') && l.includes(ID) && l.includes('draft PR'))) + assert.ok(lines.some(l => l.includes('boom'))) + adoption.stop() + await adoption.tick() + assert.equal(calls, 1, 'a stopped pass runs nothing') +}) diff --git a/packages/the-framework/src/cloud-work.ts b/packages/the-framework/src/cloud-work.ts new file mode 100644 index 000000000..725433f9d --- /dev/null +++ b/packages/the-framework/src/cloud-work.ts @@ -0,0 +1,262 @@ +import { nodeGitRunner, type GitRunner } from './project.js' +import { ghPrsForBranch, nodeGhRunner, pickAgentPr, type GhRunner, type LinkedPr } from './dashboard/gh.js' +import { openRemoteBranchPullRequest, type HandoffResult } from './dashboard/agent-handoff.js' +import { agentBranchName } from './branch-names.js' +import { adoptAgentBranch, listAgents, recordAgentPr, startedAtFromAgentId, type AgentMeta } from './store/index.js' + +// Adopt the branch a cloud session actually worked on (#1601). +// +// A web run is a local wrapper that hands the task to claude.ai and ends; the cloud session does +// the work on a branch of its own naming (`claude/*`), never the designated run branch. Nothing +// ever told the run's record about that branch, so every surface keyed to it — the session row, +// the PR resolution, CI watch, merge — stared at an empty `tf-agent-*` branch, and the run read +// as "nothing committed" while its work sat on origin. +// +// The hand-off anchor (#1601) makes the match exact rather than guessed: the driver pushes an +// empty commit unique to the run as the ref the session clones at, so the session's branch — and +// only it — descends from that commit. This pass walks origin's `claude/*` heads, matches each +// waiting run by that ancestry, and records what it finds onto the run's archive: the branch +// (the same patch-in-place `recordAgentPr` uses), the PR the session opened for it — and when the +// run was armed for a PR the session never opened, it opens the draft PR itself, which is the +// armed handoff finally resolving against the facts. +// +// Conservative on every unprovable case: a run matching no head (the session has not pushed, or +// did nothing) or more than one (ancestry alone cannot say which) is simply retried next pass, +// and a run older than the window stops being asked about at all. + +/** How long after its start a run is still asked about: safely past any cloud session's life. */ +export const CLOUD_ADOPTION_WINDOW_MS = 48 * 60 * 60 * 1000 + +/** What one adoption did, for the daemon to say out loud. */ +export interface CloudAdoption { + agentId: string + branch: string + /** The PR now on the run's record, when one was found or opened. */ + pr?: { number: number; url: string } + /** True when the PR above was opened by this pass (the armed draft), not found. */ + opened?: boolean +} + +/** What {@link adoptCloudWork} did to one project. */ +export interface CloudWorkResult { + adopted: CloudAdoption[] + /** Failures worth a log line; unmatched runs are not one, they are the normal waiting state. */ + failed: { agentId: string; error: string }[] +} + +/** Injectable seams so the pass is unit-testable off disk, off the network and off GitHub. */ +export interface CloudWorkDeps { + git?: GitRunner + gh?: GhRunner + /** The branch's full PR history (default {@link ghPrsForBranch}). */ + prs?: (cwd: string, branch: string) => Promise + /** The project's run records (default {@link listAgents}). */ + agents?: (cwd: string) => Promise + /** Record the adopted branch (default {@link adoptAgentBranch}). */ + adoptBranch?: (cwd: string, agentId: string, branch: string) => Promise + /** Record the PR (default {@link recordAgentPr}). */ + recordPr?: (cwd: string, agentId: string, pr: { number: number; url: string }) => Promise + /** Open the armed draft PR for a remote-only branch (default {@link openRemoteBranchPullRequest}). */ + openPr?: (cwd: string, agent: AgentMeta, branch: string) => Promise + /** The current time in ms (injected so tests can age runs deterministically). */ + now?: () => number +} + +/** One `claude/*` branch on origin: its short name and the commit it points at. */ +interface CloudHead { + ref: string + sha: string +} + +/** Whether the run's record still carries the branch it was born on, i.e. no adoption happened. */ +function onBirthBranch(meta: AgentMeta): boolean { + return meta.branch === undefined || meta.branch === agentBranchName(meta.id) +} + +/** Whether the run's handoff was armed to open a PR. Absent means armed, matching the agent. */ +function prArmed(meta: AgentMeta): boolean { + return meta.handoff?.pr !== false +} + +/** The settled web runs this pass still owes an answer. */ +function waitingRuns(agents: AgentMeta[], now: number): AgentMeta[] { + return agents.filter(meta => { + if (meta.target !== 'web' || meta.status === 'running' || !meta.cloudAnchor) return false + const startedAt = meta.startedAt ?? startedAtFromAgentId(meta.id) + const startedMs = startedAt === undefined ? NaN : Date.parse(startedAt) + if (!Number.isFinite(startedMs) || now - startedMs > CLOUD_ADOPTION_WINDOW_MS) return false + // Owed: the branch is not adopted yet, or it is and the armed PR is still unaccounted for. + // An unarmed run stops being asked about once its branch is recorded — a PR someone opens + // later is still found live, by branch name, by every surface that shows PRs. + return onBirthBranch(meta) || (meta.pr === undefined && prArmed(meta) && meta.status === 'done') + }) +} + +/** Parse `git ls-remote origin 'refs/heads/claude/*'`. */ +function parseCloudHeads(listing: string): CloudHead[] { + const heads: CloudHead[] = [] + for (const line of listing.split('\n')) { + const head = /^([0-9a-f]{40,64})\t+refs\/heads\/(claude\/.+)$/.exec(line) + if (head) heads.push({ ref: head[2]!, sha: head[1]! }) + } + return heads +} + +/** + * Whether `head` descends from `anchor` — the proof the branch is this run's. The head's + * objects may not be local (the cloud VM pushed them), so the ref is fetched once when needed; + * the fetch also (re)supplies the anchor commit itself, being an ancestor. Unprovable reads as + * "not this run's", retried next pass. + */ +async function descendsFrom(git: GitRunner, cwd: string, anchor: string, head: CloudHead): Promise { + const isAncestor = () => + git(['merge-base', '--is-ancestor', anchor, head.sha], cwd).then( + () => true, + () => false, + ) + const present = await git(['cat-file', '-e', `${head.sha}^{commit}`], cwd).then( + () => true, + () => false, + ) + if (present) return isAncestor() + if (!(await git(['fetch', 'origin', `refs/heads/${head.ref}`], cwd).then(() => true, () => false))) return false + return isAncestor() +} + +/** + * Adopt one project's cloud work (#1601): match each waiting web run to the `claude/*` head + * descending from its hand-off anchor, and record the branch and its PR onto the run's archive — + * opening the armed draft PR when the session never did. Never throws: a repo with no remote + * (or offline) adopts nothing, and every unmatched run is retried next pass. + */ +export async function adoptCloudWork(cwd: string, deps: CloudWorkDeps = {}): Promise { + const git = deps.git ?? nodeGitRunner() + const gh = deps.gh ?? nodeGhRunner() + const prs = deps.prs ?? ghPrsForBranch + const agents = deps.agents ?? listAgents + const adoptBranch = deps.adoptBranch ?? adoptAgentBranch + const recordPr = deps.recordPr ?? recordAgentPr + const openPr = deps.openPr ?? ((c: string, agent: AgentMeta, branch: string) => openRemoteBranchPullRequest(c, agent, branch, { gh })) + const now = deps.now ? deps.now() : Date.now() + const result: CloudWorkResult = { adopted: [], failed: [] } + + const waiting = waitingRuns(await agents(cwd).catch((): AgentMeta[] => []), now) + if (waiting.length === 0) return result + + let listing: string + try { + listing = await git(['ls-remote', 'origin', 'refs/heads/claude/*'], cwd) + } catch { + return result // no remote, or it cannot be reached: nothing to match against + } + const heads = parseCloudHeads(listing) + if (heads.length === 0) return result + + for (const run of waiting) { + const matches: CloudHead[] = [] + for (const head of heads) { + if (await descendsFrom(git, cwd, run.cloudAnchor!, head)) matches.push(head) + } + // Exactly one, or nothing happens: zero is a session that has not pushed (or never will), + // and two is a history this pass cannot arbitrate — both are the next pass's question. + if (matches.length !== 1) continue + const head = matches[0]! + const branch = head.ref + + if (onBirthBranch(run) && !(await adoptBranch(cwd, run.id, branch))) { + result.failed.push({ agentId: run.id, error: `could not record ${branch} on the run's archive` }) + continue + } + + // The PR the session opened for its branch, if any — filtered by the run's start so a + // predecessor's PR on a reused name is never this run's, `latest` so the last PR that saw + // the branch answers (#1512). + const since = run.startedAt ?? startedAtFromAgentId(run.id) + let pr = pickAgentPr(await prs(cwd, branch).catch((): LinkedPr[] => []), since, 'latest') + let opened = false + + // The armed handoff, finally resolving (#1601): the run was armed for a PR, the session + // opened none, and the branch carries something beyond the hand-off itself — so the draft + // PR the wrapper's epilogue could never open (it saw only the empty run branch) opens now. + // A run whose PR arming was off gets its branch recorded and nothing else. + if (!pr && prArmed(run) && run.status === 'done' && head.sha !== run.cloudAnchor) { + const openedPr = await openPr(cwd, run, branch) + if (openedPr.ok && openedPr.number !== undefined && openedPr.url) { + pr = { number: openedPr.number, url: openedPr.url, state: 'OPEN', title: '' } + opened = true + } else if (!openedPr.ok) { + result.failed.push({ agentId: run.id, error: `could not open the armed draft PR for ${branch}: ${openedPr.error}` }) + } + } + + if (pr && run.pr === undefined) await recordPr(cwd, run.id, { number: pr.number, url: pr.url }) + result.adopted.push({ + agentId: run.id, + branch, + ...(pr ? { pr: { number: pr.number, url: pr.url } } : {}), + ...(opened ? { opened: true } : {}), + }) + } + return result +} + +/** A running adoption pass, in the shape the daemon's other background services use. */ +export interface CloudWorkAdoption { + /** Run one pass now, awaiting it. Exposed for tests and for a caller that wants it on demand. */ + tick: () => Promise + stop: () => void +} + +/** What {@link startCloudWorkAdoption} needs from the daemon. */ +export interface CloudWorkAdoptionOptions { + /** The registered projects to pass over. */ + projects: () => Promise + log: (message: string) => void + /** The per-project pass (default {@link adoptCloudWork}). */ + adopt?: (cwd: string) => Promise +} + +/** + * Adopt every registered project's cloud work (#1601), one turn per call. + * + * Adoptions and failures are said out loud, unmatched runs are not: a session that has not + * pushed yet is the normal state of every run this watches, and a line per tick about it would + * be noise. A run's row changing branch with no line explaining why would read as a bug. + */ +export function startCloudWorkAdoption(opts: CloudWorkAdoptionOptions): CloudWorkAdoption { + const adopt = opts.adopt ?? adoptCloudWork + let stopped = false + + const passAll = async (): Promise => { + for (const project of await opts.projects().catch(() => [])) { + if (stopped) break + const { adopted, failed } = await adopt(project.path).catch((): CloudWorkResult => ({ adopted: [], failed: [] })) + for (const adoption of adopted) { + const prLine = adoption.pr ? (adoption.opened ? `; opened its armed draft PR ${adoption.pr.url}` : `; its PR is ${adoption.pr.url}`) : '' + opts.log(`[framework] session ${adoption.agentId}'s cloud work landed on ${adoption.branch} — adopted as its branch (#1601)${prLine}`) + } + for (const failure of failed) { + opts.log(`[framework] cloud work adoption for session ${failure.agentId}: ${failure.error}`) + } + } + } + + // Overlapping ticks join the pass already running rather than being dropped, so awaiting + // `tick()` means the pass finished — same rule as the worktree sweep, for the same reason. + let inflight: Promise | undefined + const tick = (): Promise => { + if (stopped) return Promise.resolve() + inflight ??= passAll().finally(() => { + inflight = undefined + }) + return inflight + } + + // No timer of its own (E4): the daemon's one clock calls `tick`. + return { + tick, + stop: () => { + stopped = true + }, + } +} diff --git a/packages/the-framework/src/daemon-services.SPEC.md b/packages/the-framework/src/daemon-services.SPEC.md index 22a0ef62f..c32a6a9d5 100644 --- a/packages/the-framework/src/daemon-services.SPEC.md +++ b/packages/the-framework/src/daemon-services.SPEC.md @@ -8,6 +8,7 @@ Everything the daemon runs in the background beside serving the dashboard: Disco - Auto PM spends idle quota on the roadmap: it fans out up to the configured number of unattended agents, each pinned to one queue entry, and retires an entry on the data branch once its agent's ending reports the work published; the daemon, never the agent, writes queue check-offs and ticket locks. - The CI watch merges a watched PR once its checks pass, and puts a fix agent on one whose checks fail. - An hourly sweep deletes the dead refs Claude-web hand-offs leave on origin, once they are old enough and provably hold no work. +- Settled web runs are matched to the `claude/*` branch that grew out of their hand-off, and the branch and its PR are adopted onto the run's record — with the armed draft PR opened when the session never opened one. - The Discord notification watchers are rebuilt when the webhook changes, so a value pasted into the dashboard works immediately. - The data branch is pulled eagerly, so this machine reads what other machines and cloud sessions pushed without waiting for its own next write. - Every background start forces unattended mode, so gates auto-answer instead of parking forever on an absent human. diff --git a/packages/the-framework/src/daemon-services.ts b/packages/the-framework/src/daemon-services.ts index 6b5a8a2e5..52564e970 100644 --- a/packages/the-framework/src/daemon-services.ts +++ b/packages/the-framework/src/daemon-services.ts @@ -23,6 +23,7 @@ import { readFile, writeFile } from 'node:fs/promises' import { startMergedWorktreeSweep, type MergedSweepOptions } from './merged-worktrees.js' import { startBranchLinksPass } from './branch-links.js' import { startCloudScratchSweep } from './cloud-scratch-refs.js' +import { startCloudWorkAdoption } from './cloud-work.js' import { resolveAgentPr } from './dashboard/agent-handoff.js' import { sendChoice, sendMessage, sendStop } from './dashboard-rpc/control.js' import type { ProjectSummary } from './dashboard/projects.js' @@ -32,7 +33,8 @@ import type { StartAgentOptions, StartAgentResult } from './dashboard/types.js' /** * Everything the daemon runs in the background beside serving the dashboard: the two Discord * notification watchers (#627), auto PM (#685/#773), the CI watch (#1418), the session-archive - * committer (#912/#1179), the worktree sweep (#1036) and the cloud-scratch sweep (#1547). + * committer (#912/#1179), the worktree sweep (#1036), the cloud-scratch sweep (#1547) and the + * cloud work adoption (#1601). * * All of it used to sit inline in `runDaemon`, which meant its body was a lifecycle narrative with * ~200 lines of service wiring in the middle of it. Each of these is gated the same way (an env @@ -318,6 +320,12 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background // (~a day) and only deletes refs whose work is provably on the default branch. const cloudScratch = startCloudScratchSweep({ projects, log, busy: deps.busyAgentIds }) + // Adopt the branch a cloud session actually worked on (#1601): match each settled web run to + // the `claude/*` head descending from its hand-off anchor, record the branch and PR on the + // run's archive, and open the armed draft PR the session never did. Daemon-side by necessity: + // the branch does not exist yet when the wrapper ends — the cloud VM is still provisioning. + const cloudWork = startCloudWorkAdoption({ projects, log }) + // `resolve` matters: projectId hashes the path string, and `--cwd` reaches us verbatim, so a // relative path would hash to an id no project lookup can resolve. Same derivation the runtime uses. const homeId = projectId(resolve(deps.cwd)) @@ -439,6 +447,9 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background // sweep ages those refs from when it first saw them, so the sooner it looks, the sooner // a leftover can go. { name: 'cloud scratch sweep', every: CLOUD_SCRATCH_EVERY, run: () => cloudScratch.tick() }, + // Ten minutes: the cloud session it waits on lives minutes-to-hours itself, and the pass + // costs an `ls-remote` per project only while a settled web run is actually waiting. + { name: 'cloud work adoption', every: AUTO_PM_EVERY, run: () => cloudWork.tick() }, ], }) @@ -453,6 +464,7 @@ export function startBackgroundServices(deps: BackgroundServiceDeps): Background mergedWorktrees.stop() branchLinks.stop() cloudScratch.stop() + cloudWork.stop() }, reloadDiscord, // Awaitable (#1433) so the trigger button can wait for the sweep's answer; a caller that diff --git a/packages/the-framework/src/dashboard/agent-handoff.SPEC.md b/packages/the-framework/src/dashboard/agent-handoff.SPEC.md index dab41c751..735e2fc93 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.SPEC.md +++ b/packages/the-framework/src/dashboard/agent-handoff.SPEC.md @@ -7,6 +7,7 @@ How a finished agent's work is handed back to the human: measure what its branch - Push and a draft PR are armed by default; drafts keep the automatic path out of reviewers' inboxes, and uncommitted leftovers are swept into a commit first (guarded so only the agent's own checkout and branch are ever committed). - The PR number is recorded on the agent the moment one is opened for it, so every surface reads the same integer instead of re-deriving it. It used to be re-resolved from three candidate branch names filtered by the agent's start time — a guess assembled at read time, standing in for one fact nobody had written down. Its *state* is still read live, because that changes without the agent doing anything. - A pull request opened after the agent's process is gone is recorded too, by patching its archive: it is the same fact, and a surface should not have to know which of the two paths produced it. +- A branch that exists only on the remote — a cloud session's own, pushed from a VM this machine never sees — can still get its draft PR opened: there is nothing to push first, the PR request itself is the whole action. - The branch's own PR history is still consulted for a different question — does this branch already have one — because a branch name pinned by a prompt is reused across agents, so a reused branch never wears an old PR and a branch with one never gets a second. - Configuration arms an automatic merge; only the agent's declared-done signal plus an empty backlog of its own authorizes it, and a withheld merge still pushes and opens the draft for a human. diff --git a/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md b/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md index 92f2e2b52..f1b0297c6 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md +++ b/packages/the-framework/src/dashboard/agent-handoff.test.SPEC.md @@ -1,4 +1,4 @@ -The tests cover the whole handoff story: reading a branch's work (empty, bookkeeping-only, gone, unpushed, and no-remote cases, against fakes and real repos), push and PR-opening with git's own reason on failure, the armed push/draft-PR/merge combinations including never opening a second PR, resolving an agent's PR across its candidate branch names and start time, merge authorization, and the human Merge action with its refusals. +The tests cover the whole handoff story: reading a branch's work (empty, bookkeeping-only, gone, unpushed, and no-remote cases, against fakes and real repos), push and PR-opening with git's own reason on failure, the push-free draft PR for a remote-only branch with gh's refusal reported rather than thrown, the armed push/draft-PR/merge combinations including never opening a second PR, resolving an agent's PR across its candidate branch names and start time, merge authorization, and the human Merge action with its refusals. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/dashboard/agent-handoff.test.ts b/packages/the-framework/src/dashboard/agent-handoff.test.ts index 9b4652ed1..b8b2cfe10 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.test.ts +++ b/packages/the-framework/src/dashboard/agent-handoff.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { execFile } from 'node:child_process' import { promisify } from 'node:util' -import { readAgentHandoff, resolveAgentPr, mergeAgentPr, agentBranchFor, pushAgentBranch, openBranchPullRequest, openAgentPullRequest, gitReason, agentAutoHandoff, isAgentBranch, prBaseName, commitAgentWork, withheldMerge } from './agent-handoff.js' +import { readAgentHandoff, resolveAgentPr, mergeAgentPr, agentBranchFor, pushAgentBranch, openBranchPullRequest, openRemoteBranchPullRequest, openAgentPullRequest, gitReason, agentAutoHandoff, isAgentBranch, prBaseName, commitAgentWork, withheldMerge } from './agent-handoff.js' import { pickAgentPr } from './gh.js' import { nodeGitRunner, GIT_SLOW_TIMEOUT_MS, type GitRunner } from '../project.js' import { CliTimeoutError, isCliTimeout } from '../cli-exec.js' @@ -201,6 +201,38 @@ test('opening a PR pushes first and returns the URL gh printed', async () => { assert.ok(!args.includes('--draft')) }) +test('a remote-only branch gets its draft PR without any push (#1601)', async () => { + // A cloud session's `claude/*` branch was pushed from a VM this machine never sees: there is + // nothing local to push, and gh's default base is the right one. + const ghCalls: string[][] = [] + const result = await openRemoteBranchPullRequest( + '/repo', + { id: 'r1', sessionName: 'fix the thing', intent: 'fix it' }, + 'claude/fix-the-thing', + { + gh: async args => { + ghCalls.push(args) + return 'https://github.com/o/r/pull/13\n' + }, + }, + ) + assert.deepEqual(result, { ok: true, url: 'https://github.com/o/r/pull/13', number: 13 }) + const args = ghCalls[0] ?? [] + assert.deepEqual(args.slice(0, 4), ['pr', 'create', '--head', 'claude/fix-the-thing']) + assert.ok(args.includes('--draft'), 'a PR the framework opens by itself must not request review') + assert.ok(!args.includes('--base'), "gh's default base is the repo's default branch") +}) + +test('a remote-only PR that gh refuses is a reported failure, never a throw (#1601)', async () => { + const result = await openRemoteBranchPullRequest('/repo', { id: 'r1' }, 'claude/x', { + gh: async () => { + throw new Error('gh: no commits between main and claude/x') + }, + }) + assert.equal(result.ok, false) + assert.match(result.ok === false ? result.error : '', /no commits/) +}) + test('a PR is not opened when the push fails', async () => { let ghRan = false const result = await openBranchPullRequest( diff --git a/packages/the-framework/src/dashboard/agent-handoff.ts b/packages/the-framework/src/dashboard/agent-handoff.ts index 28e77a5d3..796d5b437 100644 --- a/packages/the-framework/src/dashboard/agent-handoff.ts +++ b/packages/the-framework/src/dashboard/agent-handoff.ts @@ -503,6 +503,34 @@ export async function openBranchPullRequest( } } +/** + * Open a draft PR for a branch that exists only on the remote (#1601): a cloud session's own + * `claude/*` branch was pushed from a VM this machine never sees, so there is nothing to push + * here — `gh pr create --head` against the remote branch is the whole action, and gh's default + * base (the repo's default branch) is the right one. Draft for the same reason the auto-handoff + * opens drafts: a PR the framework opens by itself must not put a review request in anyone's + * inbox, and the interventions queue keeps listing a session's draft. + */ +export async function openRemoteBranchPullRequest( + cwd: string, + agent: HandoffAgent, + branch: string, + deps: { gh?: GhRunner } = {}, +): Promise { + const gh = deps.gh ?? nodeGhRunner() + try { + const out = (await gh(['pr', 'create', '--head', branch, '--title', agentPrTitle(agent), '--body', agentPrBody(agent), '--draft'], cwd)).trim() + forgetPr(cwd, branch) + forgetBranchPrs(cwd, branch) + const url = out.split('\n').filter(Boolean).at(-1) + if (!url) return { ok: true } + const number = prNumberFromUrl(url) + return { ok: true, url, ...(number !== undefined ? { number } : {}) } + } catch (err) { + return { ok: false, error: errorMessage(err) } + } +} + /** * Whether the session kept committing after its PR merged or closed (#1512): the PR carries a * head, the branch has a tip, and they disagree. False for an open PR (pushed commits still land diff --git a/packages/the-framework/src/driver/cloud.SPEC.md b/packages/the-framework/src/driver/cloud.SPEC.md index 65bc739fd..f80adccf4 100644 --- a/packages/the-framework/src/driver/cloud.SPEC.md +++ b/packages/the-framework/src/driver/cloud.SPEC.md @@ -7,7 +7,7 @@ A driver that hands the whole task to Claude Code on the web: it starts a real c - The project root is trusted for the CLI before the hand-off — worktrees inherit the root's trust, and starting a web agent is itself the user's trust decision — so the CLI's interactive trust question, which a background run could never answer, does not fire. A dialog that appears anyway (the write failed or was rejected) still fails fast with the manual fix named instead of timing out with nothing to show. - Nothing the user typed can ever reach a shell as syntax. - The session it creates is repo-bound, not a bundle (#1320): with nonessential traffic disabled the CLI's server-side bundle experiment reads off, so a failed GitHub-App preflight falls through to a session that clones from GitHub and can push — instead of silently uploading a local bundle whose work can never leave the VM (anthropics/claude-code#81776). -- Before the hand-off, HEAD is pushed to origin under the agent's own id: the CLI's default revision pin is the current local branch — which an agent workspace's local-only branch fails — and a slash-carrying ref never resolves on the cloud side even when pushed (anthropics/claude-code#87235), so the ref is minted slash-free and handed over explicitly. A push that fails degrades to the old behavior and says so, naming `--teleport` as the recovery path. +- Before the hand-off, the anchor is pushed to origin under the agent's own id: an empty commit on top of HEAD, unique to this run and minted without moving any branch. The session clones at it, so the branch it does its work on — a name of the cloud's own choosing — descends from it, and that ancestry is how the daemon later recognizes which branch is this run's. The ref is explicit and slash-free because the CLI's default revision pin is the current local branch — which an agent workspace's local-only branch fails — and a slash-carrying ref never resolves on the cloud side even when pushed (anthropics/claude-code#87235). A push that fails degrades to the old behavior and says so, naming `--teleport` as the recovery path; a repo where the anchor cannot be minted hands off plain HEAD, and the run is simply never matched to its branch. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/driver/cloud.test.SPEC.md b/packages/the-framework/src/driver/cloud.test.SPEC.md index 8fbf564c8..92b471146 100644 --- a/packages/the-framework/src/driver/cloud.test.SPEC.md +++ b/packages/the-framework/src/driver/cloud.test.SPEC.md @@ -1,4 +1,4 @@ -Covers the cloud hand-off: creating exactly one web session per agent no matter how often the loop prompts, surfacing the session link the way other targets surface theirs, trusting the project root on the user's behalf before the hand-off (visibly, best-effort, never clobbering a config it cannot parse) and still failing fast with the manual fix when the dialog appears anyway, keeping user text out of the shell, the web location — not the driver — being the one that ends an agent at its hand-off, and the repo-bound handshake (#1320): HEAD pushed under the slash-free agent id and handed over as the session's ref, a failed push degrading to no ref with the recovery path named, and the nonessential-traffic switch pinned as what keeps the session off the bundle path. +Covers the cloud hand-off: creating exactly one web session per agent no matter how often the loop prompts, surfacing the session link the way other targets surface theirs, trusting the project root on the user's behalf before the hand-off (visibly, best-effort, never clobbering a config it cannot parse) and still failing fast with the manual fix when the dialog appears anyway, keeping user text out of the shell, the web location — not the driver — being the one that ends an agent at its hand-off, and the repo-bound handshake (#1320): the hand-off anchor — an empty commit unique to the run — pushed under the slash-free agent id, handed over as the session's ref and reported on the result, a repo that cannot mint the anchor handing off plain HEAD with none reported, a failed push degrading to no ref with the recovery path named, and the nonessential-traffic switch pinned as what keeps the session off the bundle path. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/driver/cloud.test.ts b/packages/the-framework/src/driver/cloud.test.ts index bec33f526..90b66403d 100644 --- a/packages/the-framework/src/driver/cloud.test.ts +++ b/packages/the-framework/src/driver/cloud.test.ts @@ -38,13 +38,23 @@ function fakePty(output: string, calls: AgentPtyOptions[] = []) { } } -/** A git runner that records its calls; `fail` makes every call reject (#1320). */ -function fakeGit(calls: string[][] = [], fail = false) { +/** The anchor sha the fake git mints for `commit-tree` (#1601). */ +const ANCHOR = 'a'.repeat(40) + +/** + * A git runner that records its calls; `fail` makes every call reject (#1320), `noAnchor` + * makes only the anchor mint fail (#1601). `commit-tree` answers with a fixed sha. + */ +function fakeGit(calls: string[][] = [], fail = false, noAnchor = false) { return { calls, run: async (args: string[], _cwd: string): Promise => { calls.push([...args]) if (fail) throw new Error('no pushable remote') + if (args[0] === 'commit-tree') { + if (noAnchor) throw new Error('cannot mint the anchor') + return `${ANCHOR}\n` + } return '' }, } @@ -340,16 +350,35 @@ test('the web location is the hand-off, so a run ends at the first prompt (#1225 assert.equal(isHandsOff('actions'), false, 'an Actions runner streams its own replies') }) -test('the hand-off pushes HEAD under the agent id and hands the session that ref (#1320)', async () => { +test('the hand-off pushes the anchor under the agent id and hands the session that ref (#1320/#1601)', async () => { const git = fakeGit() + const events: DriverEvent[] = [] const ptyCalls: AgentPtyOptions[] = [] - const session = await driverWith(CREATED, ptyCalls, git).start({ cwd: '/repo' }) + const session = await driverWith(CREATED, ptyCalls, git).start({ cwd: '/repo', onEvent: e => events.push(e) }) await session.prompt('go') - // One push, of HEAD, under the agent's own id — which contains no slash, because a - // slash-carrying ref never resolves on the cloud side (anthropics/claude-code#87235). - assert.deepEqual(git.calls, [['push', 'origin', `HEAD:refs/heads/${session.id}`]]) + // The anchor commit first (#1601): an empty commit on top of HEAD, minted without moving any + // branch, whose message carries the run's own id. Then one push, of that anchor, under the + // agent's own id — which contains no slash, because a slash-carrying ref never resolves on + // the cloud side (anthropics/claude-code#87235). + assert.deepEqual(git.calls, [ + ['commit-tree', 'HEAD^{tree}', '-p', 'HEAD', '-m', `[The Framework] web hand-off ${session.id}`], + ['push', 'origin', `${ANCHOR}:refs/heads/${session.id}`], + ]) assert.ok(!session.id.includes('/')) assert.equal(ptyCalls[0]?.ref, session.id) + // The anchor reaches the meta through the result (#1601): it is how the daemon later + // recognizes which `claude/*` branch is this run's. + assert.ok(events.some(e => e.type === 'result' && e.anchorSha === ANCHOR)) +}) + +test('a repo where the anchor cannot be minted hands off plain HEAD, with no anchor reported (#1601)', async () => { + const git = fakeGit([], false, true) + const events: DriverEvent[] = [] + const session = await driverWith(CREATED, [], git).start({ cwd: '/repo', onEvent: e => events.push(e) }) + const turn = await session.prompt('go') + assert.equal(turn.sessionId, SESSION) + assert.ok(git.calls.some(args => args.join(' ') === `push origin HEAD:refs/heads/${session.id}`)) + assert.ok(events.every(e => e.type !== 'result' || e.anchorSha === undefined)) }) test('a failed pre-push falls back to no ref, says so, and still hands off (#1320)', async () => { @@ -362,6 +391,8 @@ test('a failed pre-push falls back to no ref, says so, and still hands off (#132 const notice = events.find((e): e is DriverEvent & { type: 'notice' } => e.type === 'notice') assert.ok(notice && /could not push/.test(notice.message)) assert.ok(notice && /--teleport/.test(notice.message), 'the notice names the recovery path') + // A push that failed leaves nothing on origin containing the anchor, so none is reported (#1601). + assert.ok(events.every(e => e.type !== 'result' || e.anchorSha === undefined)) }) test('the ref rides the fixed command as its own guarded flag, after the model (#1320)', () => { diff --git a/packages/the-framework/src/driver/cloud.ts b/packages/the-framework/src/driver/cloud.ts index 2c8754f0e..d5c39fce1 100644 --- a/packages/the-framework/src/driver/cloud.ts +++ b/packages/the-framework/src/driver/cloud.ts @@ -181,6 +181,8 @@ export class CloudSession implements DriverSession { private disposed = false /** The cloud session this agent was handed to, once it exists. Set at most once. */ private handedOff: { url: string; sessionId: string } | undefined + /** The hand-off anchor commit (#1601), once it is on origin. Set at most once, with the hand-off. */ + private anchorSha: string | undefined constructor( private readonly config: CloudDriverOptions, @@ -249,13 +251,29 @@ export class CloudSession implements DriverSession { // and its two failure modes are both local facts — the CLI's default pin is the current // branch, which an agent worktree's local-only run branch fails, and a slash-carrying // name (every `the-framework/...` branch) never resolves on the cloud side even when - // pushed (anthropics/claude-code#87235). So push HEAD under this agent's own id — unique, + // pushed (anthropics/claude-code#87235). So push under this agent's own id — unique, // slash-free — and hand the session that. Best-effort: a repo with no pushable remote // falls back to the CLI's default, which still works wherever it worked before, and the // notice names what a stranded session will look like. + // + // What gets pushed is not HEAD itself but the hand-off anchor (#1601): an empty commit on + // top of HEAD, minted with `commit-tree` so no local branch moves. The session does its + // work on a branch of its own naming (`claude/*`), never the designated run branch — and + // since every commit it makes descends from what it cloned, a commit unique to this run is + // the one exact mark by which the daemon can later recognize which `claude/*` branch is + // this run's. A repo where the anchor cannot be minted hands off plain HEAD: the session + // still works, the run is simply never matched to its branch. + const git = this.config.git ?? nodeGitRunner() + let anchor: string | undefined + try { + anchor = (await git(['commit-tree', 'HEAD^{tree}', '-p', 'HEAD', '-m', `[The Framework] web hand-off ${this.id}`], this.cwd)).trim() || undefined + } catch { + anchor = undefined + } let ref: string | undefined = this.id try { - await (this.config.git ?? nodeGitRunner())(['push', 'origin', `HEAD:refs/heads/${this.id}`], this.cwd) + await git(['push', 'origin', `${anchor ?? 'HEAD'}:refs/heads/${this.id}`], this.cwd) + this.anchorSha = anchor } catch (err) { ref = undefined this.emit({ @@ -336,9 +354,15 @@ export class CloudSession implements DriverSession { `View the session: ${session.url}`, `Continue it here: claude --teleport ${session.sessionId}`, ].join('\n') - // The result also carries the session's real URL (#1317): the action above is what the - // agent view links through, the result is what reaches the meta. - this.emit({ type: 'result', text: summary, sessionId: session.sessionId, sessionLink: session.url }) + // The result also carries the session's real URL (#1317) and the hand-off anchor (#1601): + // the action above is what the agent view links through, the result is what reaches the meta. + this.emit({ + type: 'result', + text: summary, + sessionId: session.sessionId, + sessionLink: session.url, + ...(this.anchorSha ? { anchorSha: this.anchorSha } : {}), + }) return { text: summary, sessionId: session.sessionId } } diff --git a/packages/the-framework/src/driver/types.ts b/packages/the-framework/src/driver/types.ts index b85ad69a8..87574f04b 100644 --- a/packages/the-framework/src/driver/types.ts +++ b/packages/the-framework/src/driver/types.ts @@ -276,8 +276,11 @@ export type DriverEvent = * The turn settled with this final text. `sessionLink` is the real URL of the session, * for a driver whose session has one of its own (#1317) — the cloud hand-off — so the * meta can link there instead of the generic entry point; drivers without one omit it. + * `anchorSha` is the hand-off anchor commit (#1601), for a driver whose session does its + * work on a branch of its own naming that this machine can only recognize later by + * ancestry; drivers whose work stays on the designated branch omit it. */ - | { type: 'result'; text: string; sessionId?: string; sessionLink?: string; usage?: DriverUsage } + | { type: 'result'; text: string; sessionId?: string; sessionLink?: string; anchorSha?: string; usage?: DriverUsage } /** Where the account's subscription quota stands (#517). */ | { type: 'rate-limit'; limit: DriverRateLimit } /** The agent (or its transport) errored. */ diff --git a/packages/the-framework/src/events.SPEC.md b/packages/the-framework/src/events.SPEC.md index 26fb1b68f..476bc66f6 100644 --- a/packages/the-framework/src/events.SPEC.md +++ b/packages/the-framework/src/events.SPEC.md @@ -3,7 +3,7 @@ The single event stream an agent narrates itself over: one timeline uniting the ## TLDR - The framework owns the stream rather than exposing the driver's transport, so every surface — terminal, dashboard, chat — renders the same story. -- Events are the agent's durable record: anything a dashboard tab opened later must know (the ticket being implemented, the branch, the pull request opened for the work, what the end-of-work handoff is armed to do) travels as an event, because only events reach its stored history. +- Events are the agent's durable record: anything a dashboard tab opened later must know (the ticket being implemented, the branch, the pull request opened for the work, the hand-off anchor a cloud run's branch is later recognized by, what the end-of-work handoff is armed to do) travels as an event, because only events reach its stored history. - Interactive gates are events too: a choice pauses the agent until a pick is posted back, and both the question and who answered it are on the record. - Every skipped or withheld outcome carries its reason, so "it was on and nothing happened" always has an answer in the log. diff --git a/packages/the-framework/src/events.ts b/packages/the-framework/src/events.ts index a7ed87b3b..e1ce16675 100644 --- a/packages/the-framework/src/events.ts +++ b/packages/the-framework/src/events.ts @@ -270,6 +270,14 @@ export type FrameworkEvent = * teardown (#799), so any read before that guessed between three naming schemes. */ | { kind: 'branch'; branch: string } + /** + * The hand-off anchor a cloud run pushed for its session to clone at (#1601): an empty commit + * unique to this run, so the branch the session actually works on — a `claude/*` name of the + * cloud's own choosing, never the designated run branch — is recognizable later by plain + * ancestry. Folded to `AgentMeta.cloudAnchor`, which the daemon's adoption pass matches + * against origin's `claude/*` heads once the session has pushed its work. + */ + | { kind: 'cloud-anchor'; sha: string } /** * What the end-of-session handoff actually did (#1102): pushed and/or opened a draft PR, * declined for a reason that is not a fault, or failed at one of the two steps. diff --git a/packages/the-framework/src/store/agent-store.SPEC.md b/packages/the-framework/src/store/agent-store.SPEC.md index a49abbb10..a111ae58e 100644 --- a/packages/the-framework/src/store/agent-store.SPEC.md +++ b/packages/the-framework/src/store/agent-store.SPEC.md @@ -14,7 +14,7 @@ Agent persistence: an agent's history is its append-only event log, and everythi - Listing a project's history reads every user's archive and the transient one, shows an agent once when it appears in both, and prefers an agent's live copy to its archived one. - The snapshot is renamed into place rather than written over, so a reader outside the agent's process sees a whole snapshot or the whole previous one. One that arrives unreadable is read again before it is called corrupt. - An agent whose process is provably dead has its missing ending written on its behalf — into the log as well as the snapshot. An owner that cannot be probed is left alone until boot. -- An archived snapshot can be patched afterwards with a fact discovered once the agent's process is gone, such as the pull request opened for its work. +- An archived snapshot can be patched afterwards with a fact discovered once the agent's process is gone, such as the pull request opened for its work or the branch a cloud session's work landed on. - Ids are timestamps made path-safe, so id order is time order. ## Rationales diff --git a/packages/the-framework/src/store/agent-store.test.ts b/packages/the-framework/src/store/agent-store.test.ts index 8b37730a0..26c63b3e6 100644 --- a/packages/the-framework/src/store/agent-store.test.ts +++ b/packages/the-framework/src/store/agent-store.test.ts @@ -11,6 +11,7 @@ import { readLiveMetas, archiveWorktreeAgent, recordAgentPr, + adoptAgentBranch, restoreArchivedAgent, listWorktreeDirs, reconcileOrphanedAgents, @@ -679,6 +680,21 @@ test('recordAgentPr patches the archived meta, so a PR opened after the run stil assert.deepEqual((await listAgents(CWD, fs)).find(r => r.id === 'r1')?.pr, { number: 42, url: 'https://x/pull/42' }) }) +test('adoptAgentBranch patches the archived meta with the branch the cloud work landed on (#1601)', async () => { + // The cloud VM pushes its `claude/*` branch after the wrapper's process is gone, so the fact + // arrives the same way a late PR does: patched onto the archive, read by every surface. + const fs = memFs(worktreeFiles('r1', { version: 1, status: 'done', id: 'r1', startedAt: AT, updatedAt: AT, branch: 'tf-agent-r1' })) + await archiveWorktreeAgent(worktreeAt('r1'), CWD, fs) + assert.equal(await adoptAgentBranch(CWD, 'r1', 'claude/fix-the-thing', fs), true) + assert.equal((await listAgents(CWD, fs)).find(r => r.id === 'r1')?.branch, 'claude/fix-the-thing') +}) + +test('adoptAgentBranch leaves the record as it was when there is nothing to patch (#1601)', async () => { + const fs = memFs() + assert.equal(await adoptAgentBranch(CWD, 'nope', 'claude/x', fs), false) + assert.equal(await adoptAgentBranch(CWD, '../escape', 'claude/x', fs), false, 'and an unsafe id is refused') +}) + test('recordAgentPr leaves the record as it was when there is nothing to patch (E6)', async () => { // Best-effort: the cost of missing it is one surface having to ask gh, which is what all of them // used to do anyway. diff --git a/packages/the-framework/src/store/agent-store.ts b/packages/the-framework/src/store/agent-store.ts index 5a6dbf9d3..c3d5fdee9 100644 --- a/packages/the-framework/src/store/agent-store.ts +++ b/packages/the-framework/src/store/agent-store.ts @@ -121,6 +121,14 @@ export interface AgentMeta { * the run-id branch is guaranteed to be the one holding the commits. */ branch?: string + /** + * The hand-off anchor a cloud run pushed for its session to clone at (#1601): an empty commit + * unique to this run, folded from the `cloud-anchor` event. The session works on a `claude/*` + * branch of the cloud's own naming, and this is the ancestor by which the daemon's adoption + * pass recognizes which of origin's `claude/*` heads is this run's. Absent on non-web runs + * and on web runs whose pre-hand-off push failed. + */ + cloudAnchor?: string /** * The ticket this agent is implementing (#1117), repo-relative (`tickets/.md`). * @@ -360,6 +368,9 @@ export function applyEventToMeta(meta: AgentMeta, event: FrameworkEvent, at: str case 'branch': next.branch = event.branch break + case 'cloud-anchor': + next.cloudAnchor = event.sha + break case 'settled': next.settledAt = at break @@ -1165,3 +1176,30 @@ export async function recordAgentPr( return false } } + +/** + * Record the branch a cloud session's work actually landed on (#1601), patching the archived + * meta in place the same way {@link recordAgentPr} does and for the same reason: the fact + * becomes known only after the agent's process is gone — the cloud VM pushes its `claude/*` + * branch while the local wrapper is already history — so there is no event stream left to + * carry it. Every surface resolves the recorded branch first, so this one write is what turns + * a run's "nothing committed" row into its real branch, PR and merge state. + */ +export async function adoptAgentBranch( + cwd: string, + agentId: string, + branch: string, + fs: StoreFs = nodeStoreFs(), +): Promise { + if (!isSafeAgentId(agentId)) return false + try { + const archive = await findArchive(fs, cwd, agentId) + if (!archive) return false + const meta = await readMetaFile(fs, archive.meta) + if (!meta) return false + await fs.write(archive.meta, JSON.stringify({ ...meta, branch })) + return true + } catch { + return false + } +} diff --git a/packages/the-framework/src/store/index.ts b/packages/the-framework/src/store/index.ts index 1e0c6c63e..350bbbb8d 100644 --- a/packages/the-framework/src/store/index.ts +++ b/packages/the-framework/src/store/index.ts @@ -15,6 +15,7 @@ export { archiveWorktreeAgent, archivedAgentPaths, recordAgentPr, + adoptAgentBranch, restoreArchivedAgent, listWorktreeDirs, worktreeDirEntries, diff --git a/packages/the-framework/src/terminal.ts b/packages/the-framework/src/terminal.ts index 452928322..bd6da050d 100644 --- a/packages/the-framework/src/terminal.ts +++ b/packages/the-framework/src/terminal.ts @@ -37,6 +37,8 @@ export function formatFrameworkEvent(event: FrameworkEvent): string { return ` implementing ${event.path}` case 'branch': return ` branch: ${event.branch}` + case 'cloud-anchor': + return ` hand-off anchor: ${event.sha.slice(0, 7)}` case 'pull-request': return ` pull request: #${event.number}` case 'on-before-mergeable': diff --git a/packages/the-framework/src/worktrees.SPEC.md b/packages/the-framework/src/worktrees.SPEC.md index 01462f5b7..8f580fe5f 100644 --- a/packages/the-framework/src/worktrees.SPEC.md +++ b/packages/the-framework/src/worktrees.SPEC.md @@ -5,6 +5,7 @@ Cleans up the per-agent checkouts a project retains — one implementation behin - **One rule: only what is on the remote may go.** Removing a checkout commits whatever it is still holding to the agent's branch, pushes that branch, and deletes the checkout only once the remote has it — so nothing local is ever the last copy of work, and every deletion is recoverable with `git worktree add`. - A repo with nowhere to push keeps every checkout, which is the honest answer rather than a special case: there is nowhere for the work to be recoverable from. - A session set to publish nothing (`handoff: local`) keeps its unpushed checkout the same way: the push exists to make removal recoverable, not to publish work the session said must stay local. That decision comes before anything commits — a kept checkout is a place someone works, and grabbing their half-typed edits as a commit on the way to a refusal would repeat every sweep pass — so its checkout goes only from a clean tree on a tip already on the remote, where removing it publishes nothing. +- A web run's checkout is the one carve-out from the push: the hand-off already pushed everything the cloud session clones at, and the work lands on the session's own remote branch — so pushing the empty local run branch would only put a dead ref on origin per web run. It goes without a push once it provably holds nothing (a clean tree whose tip is inside what the hand-off pushed); any doubt falls back to the ordinary rule. - A record that cannot be read keeps the checkout too: "no record was ever written" is a boot death and takes the recoverable default, but unreadable cannot tell a publish-nothing session from any other, so removal refuses rather than guesses and a later pass retries. - One failure mode, and it is legible: the push did not land, so the checkout stays and the reason says why. It replaced a retention policy that asked what state the agent ended in, which is a question with three answers and no bearing on whether the work is safe. - Deleting an agent is the other thing entirely: its archived records leave the dashboard for good and uncommitted work is discarded with the checkout — but the branch and its commits stay, because silently deleting a branch that may carry merged work or an open pull request is not a dashboard's call. diff --git a/packages/the-framework/src/worktrees.test.SPEC.md b/packages/the-framework/src/worktrees.test.SPEC.md index bfcd5d64e..c930702f8 100644 --- a/packages/the-framework/src/worktrees.test.SPEC.md +++ b/packages/the-framework/src/worktrees.test.SPEC.md @@ -1,4 +1,4 @@ -Covers worktree cleanup against real git: removal preserves a checkout's uncommitted work on the agent's branch and on the remote before deleting anything, refuses when the commit fails or the branch cannot reach the remote, keeps a publish-nothing session's unpushed checkout rather than pushing it — without committing its edits on the way to the refusal, while one whose branch already reached the remote still goes — keeps a checkout whose record exists but cannot be read, deletion clears the agent's records and checkout while the branch and its commits survive, record-only agents still delete cleanly, and unsafe or unknown agent ids are refused before anything is touched. +Covers worktree cleanup against real git: removal preserves a checkout's uncommitted work on the agent's branch and on the remote before deleting anything, refuses when the commit fails or the branch cannot reach the remote, keeps a publish-nothing session's unpushed checkout rather than pushing it — without committing its edits on the way to the refusal, while one whose branch already reached the remote still goes — keeps a checkout whose record exists but cannot be read, lets a web run's clean checkout go without pushing its empty run branch while one holding more than its hand-off carried falls back to the ordinary rule, deletion clears the agent's records and checkout while the branch and its commits survive, record-only agents still delete cleanly, and unsafe or unknown agent ids are refused before anything is touched. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/worktrees.test.ts b/packages/the-framework/src/worktrees.test.ts index 691cd96d1..15443df2f 100644 --- a/packages/the-framework/src/worktrees.test.ts +++ b/packages/the-framework/src/worktrees.test.ts @@ -147,6 +147,60 @@ test('a publish-nothing session whose branch is already on the remote still lets } }) +test("a web run's checkout goes without pushing its empty run branch to origin (#1601)", async () => { + // The hand-off pushed everything the cloud session clones at, and the work lands on the + // session's own remote branch — pushing the local run branch just to satisfy the remote rule + // is what accreted one dead `tf-agent-*` ref on origin per web run. + const { repo, path, branch } = await repoWithDirtyWorktree() + const git = nodeGitRunner() + try { + // A web run's wrapper never edits, so its tree is clean — and the framework's own + // bookkeeping is git-excluded, as the install leaves every activated repo (#1600). + await git(['checkout', '--', '.'], path) + await mkdir(join(repo, '.git', 'info'), { recursive: true }) + await writeFile(join(repo, '.git', 'info', 'exclude'), '.the-framework/\n') + const anchor = (await git(['commit-tree', 'HEAD^{tree}', '-p', 'HEAD', '-m', 'hand-off'], path)).trim() + const now = new Date().toISOString() + await mkdir(join(path, '.the-framework'), { recursive: true }) + await writeFile( + join(path, '.the-framework', 'agent.json'), + JSON.stringify({ status: 'done', id: RUN_ID, startedAt: now, updatedAt: now, target: 'web', cloudAnchor: anchor }), + ) + assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true }) + await assert.rejects(() => stat(path), 'the checkout is gone') + await assert.rejects( + () => git(['rev-parse', '--verify', `refs/remotes/origin/${branch}`], repo), + 'and no empty run branch reached origin', + ) + } finally { + await rm(repo, { recursive: true, force: true }) + } +}) + +test('a web run whose checkout holds more than the hand-off carried falls back to the ordinary rule (#1601)', async () => { + // The dirty tree is exactly the doubt the carve-out must not swallow: the ordinary rule + // commits and pushes, which is never worse than what every web run got before. + const { repo, path, branch } = await repoWithDirtyWorktree() + const git = nodeGitRunner() + try { + const anchor = (await git(['commit-tree', 'HEAD^{tree}', '-p', 'HEAD', '-m', 'hand-off'], path)).trim() + const now = new Date().toISOString() + await mkdir(join(path, '.the-framework'), { recursive: true }) + await writeFile( + join(path, '.the-framework', 'agent.json'), + JSON.stringify({ status: 'done', id: RUN_ID, startedAt: now, updatedAt: now, target: 'web', cloudAnchor: anchor }), + ) + assert.deepEqual(await removeProjectWorktree(repo, RUN_ID), { ok: true }) + assert.match( + await git(['show', `refs/remotes/origin/${branch}:index.html`], repo), + /Welcome!/, + 'the edit survived on the remote, exactly as a non-web run would have it', + ) + } finally { + await rm(repo, { recursive: true, force: true }) + } +}) + test('a worktree whose work cannot be committed is refused, not force-removed (#982)', async () => { const { repo, path } = await repoWithDirtyWorktree() try { diff --git a/packages/the-framework/src/worktrees.ts b/packages/the-framework/src/worktrees.ts index 9dcd04dff..4bdf1c531 100644 --- a/packages/the-framework/src/worktrees.ts +++ b/packages/the-framework/src/worktrees.ts @@ -22,6 +22,7 @@ import { } from './store/index.js' import { pushAgentBranch } from './dashboard/agent-handoff.js' import { dataWorktreePath, withDataBranch } from './data-branch.js' +import { nodeGitRunner } from './project.js' /** A retained worktree and the agent that left it behind (#752). */ export interface WorktreeRow { @@ -154,7 +155,16 @@ export async function removeProjectWorktree( error: `session ${agentId}'s record could not be read (${errorMessage(err)}); its worktree was kept`, } } - if (meta?.handoff && !meta.handoff.push) { + if (meta?.target === 'web' && meta.cloudAnchor && (await webCheckoutCovered(path, branch, meta.cloudAnchor))) { + // A web run's checkout never holds the work (#1601): the hand-off pushed everything the + // cloud session clones at, and the work itself lands on the session's own remote branch. + // Pushing the local run branch just to satisfy the remote rule is what accreted one empty + // `tf-agent-*` ref on origin per web run — so the checkout goes without a push, once it is + // provably holding nothing: a clean tree whose tip is inside what the hand-off pushed (the + // recorded anchor). Anything short of that proof — no anchor recorded, the anchor's object + // gone, a tree or tip that moved — falls through to the ordinary rule below, which is never + // worse than what every web run got before. + } else if (meta?.handoff && !meta.handoff.push) { // A publish-nothing session's checkout goes only once everything it holds is already on // the remote by someone's explicit act: a clean tree on a pushed tip — then removing it // publishes nothing. Anything short of that would take a commit or a push of removal's @@ -195,6 +205,20 @@ export async function removeProjectWorktree( } } +/** + * Whether a web run's checkout provably holds nothing its hand-off did not carry (#1601): the + * tree is clean and the branch tip is an ancestor of the pushed anchor. False on any doubt — + * the caller then treats the checkout like every other run's. + */ +async function webCheckoutCovered(path: string, branch: string, anchor: string): Promise { + if (!(await worktreeClean(path))) return false + const git = nodeGitRunner() + return git(['merge-base', '--is-ancestor', branch, anchor], path).then( + () => true, + () => false, + ) +} + /** * The meta the keep decision reads: the live copy in the checkout, else the archived one. * From 084b1400347b067cad12af185893537a79f625c8 Mon Sep 17 00:00:00 2001 From: Suleiman Shahbari Date: Thu, 20 Aug 2026 14:45:04 +0300 Subject: [PATCH 2/2] An adopted branch is recorded as a commit on the data branch, not a file the next sync wipes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seen live: the adoption pass patched the run's archived record straight into the tf-data checkout; a minute later the data sync's rebase refused the dirty tree and the funnel hard-reset it, so the run flipped back to its birth branch every pass. One `patchArchivedAgent` replaces `recordAgentPr` + `adoptAgentBranch`, and `patchArchivedAgentOnDataBranch` is the funneled form (sync, patch, commit, push) the pass and the Open PR button now use. The pass also treats a failed `gh pr list` as "could not tell" rather than "none" (it opened a second draft PR otherwise), leaves alone a run whose record names some other branch, and writes one commit per adoption. The driver mints and pushes the anchor in one step — the anchor-or-HEAD fallback was a second hand-off path that could not occur. SPECs that still described the pre-#1601 pair of dead refs, and the test SPECs missing the new cases, are brought current. Co-Authored-By: Claude Fable 5 --- .../src/agent-telemetry.test.SPEC.md | 2 +- .../src/archived-agent-patch.SPEC.md | 9 +++ .../src/archived-agent-patch.test.SPEC.md | 5 ++ .../src/archived-agent-patch.test.ts | 73 ++++++++++++++++++ .../the-framework/src/archived-agent-patch.ts | 26 +++++++ .../src/cloud-scratch-refs.SPEC.md | 4 +- packages/the-framework/src/cloud-work.SPEC.md | 2 +- .../the-framework/src/cloud-work.test.SPEC.md | 2 +- packages/the-framework/src/cloud-work.test.ts | 34 +++++++-- packages/the-framework/src/cloud-work.ts | 75 +++++++++++-------- .../src/dashboard-rpc/control.ts | 5 +- packages/the-framework/src/dashboard/gh.ts | 20 ++++- .../the-framework/src/driver/cloud.SPEC.md | 2 +- .../src/driver/cloud.test.SPEC.md | 2 +- .../the-framework/src/driver/cloud.test.ts | 21 +----- packages/the-framework/src/driver/cloud.ts | 13 +--- .../src/store/agent-store.test.ts | 23 ++---- .../the-framework/src/store/agent-store.ts | 52 ++++--------- packages/the-framework/src/store/index.ts | 4 +- 19 files changed, 243 insertions(+), 131 deletions(-) create mode 100644 packages/the-framework/src/archived-agent-patch.SPEC.md create mode 100644 packages/the-framework/src/archived-agent-patch.test.SPEC.md create mode 100644 packages/the-framework/src/archived-agent-patch.test.ts create mode 100644 packages/the-framework/src/archived-agent-patch.ts diff --git a/packages/the-framework/src/agent-telemetry.test.SPEC.md b/packages/the-framework/src/agent-telemetry.test.SPEC.md index 98357786e..ce23beb03 100644 --- a/packages/the-framework/src/agent-telemetry.test.SPEC.md +++ b/packages/the-framework/src/agent-telemetry.test.SPEC.md @@ -1,4 +1,4 @@ -Tests that the session id surfaces the moment a turn starts (so a stopped turn cannot lose the resume handle), never repeats for the same id, and that the agent's opening event records the model it was started with. +Tests that the session id surfaces the moment a turn starts (so a stopped turn cannot lose the resume handle), never repeats for the same id, that the agent's opening event records the model it was started with, and that a hand-off carrying an anchor commit reports it as its own event while one without reports none. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/archived-agent-patch.SPEC.md b/packages/the-framework/src/archived-agent-patch.SPEC.md new file mode 100644 index 000000000..99f973a5d --- /dev/null +++ b/packages/the-framework/src/archived-agent-patch.SPEC.md @@ -0,0 +1,9 @@ +Records a fact learned about a run after it ended — the pull request its work is on, the branch a cloud session landed it on — onto the run's archived record, as a commit on the data branch. + +## TLDR + +- The write goes through the data branch's one write funnel — sync with origin, patch, commit, push — so the fact is shared with every machine and survives the next sync; written straight into the checkout it would be wiped within a minute, since the sync hard-resets a dirty checkout. + +## Before modifying/creating SPEC.md files + +You must always read and respect https://raw.githubusercontent.com/brillout/sdd/refs/heads/main/sdd.md diff --git a/packages/the-framework/src/archived-agent-patch.test.SPEC.md b/packages/the-framework/src/archived-agent-patch.test.SPEC.md new file mode 100644 index 000000000..83cc7928b --- /dev/null +++ b/packages/the-framework/src/archived-agent-patch.test.SPEC.md @@ -0,0 +1,5 @@ +Covers the archive patch against real git: a patch lands as a commit on the data branch, pushed to origin, with the checkout left clean and the fact surviving the next sync; a run with no archive is reported as not patched and commits nothing. + +## Before modifying/creating SPEC.md files + +You must always read and respect https://raw.githubusercontent.com/brillout/sdd/refs/heads/main/sdd.md diff --git a/packages/the-framework/src/archived-agent-patch.test.ts b/packages/the-framework/src/archived-agent-patch.test.ts new file mode 100644 index 000000000..286a79e33 --- /dev/null +++ b/packages/the-framework/src/archived-agent-patch.test.ts @@ -0,0 +1,73 @@ +import { strict as assert } from 'node:assert' +import { test } from 'node:test' +import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' +import { dataWorktreePath, withDataBranch } from './data-branch.js' +import { patchArchivedAgentOnDataBranch } from './archived-agent-patch.js' + +const git = promisify(execFile) + +/** A real repo with a bare origin and one archived run seeded on the data branch. */ +async function repoWithArchive(): Promise<{ project: string; cleanup: () => Promise }> { + const project = await mkdtemp(join(tmpdir(), 'framework-archive-patch-')) + const origin = await mkdtemp(join(tmpdir(), 'framework-archive-patch-origin-')) + await git('git', ['init', '-q', '--bare'], { cwd: origin }) + await git('git', ['init', '-q', '-b', 'main'], { cwd: project }) + await git('git', ['config', 'user.email', 'test@example.com'], { cwd: project }) + await git('git', ['config', 'user.name', 'Test'], { cwd: project }) + await git('git', ['config', 'commit.gpgsign', 'false'], { cwd: project }) + await git('git', ['remote', 'add', 'origin', origin], { cwd: project }) + const seeded = await withDataBranch(project, 'seed', async dir => { + await mkdir(join(dir, 'agents', 'u'), { recursive: true }) + const meta = { version: 1, status: 'done', id: 'r1', startedAt: '2026-08-20T10:00:00.000Z', updatedAt: '2026-08-20T10:00:00.000Z', branch: 'tf-agent-r1' } + await writeFile(join(dir, 'agents', 'u', 'r1.json'), JSON.stringify(meta)) + await writeFile(join(dir, 'agents', 'u', 'r1.jsonl'), '') + }) + assert.ok(seeded.ok && seeded.pushed, 'the fixture archive must land on origin') + return { + project, + cleanup: async () => { + await rm(project, { recursive: true, force: true }) + await rm(origin, { recursive: true, force: true }) + }, + } +} + +test('an archive patch lands as a commit on the data branch, pushed, leaving the checkout clean (#1601)', async () => { + const { project, cleanup } = await repoWithArchive() + try { + const dir = dataWorktreePath(project) + assert.equal( + await patchArchivedAgentOnDataBranch(project, 'r1', { branch: 'claude/fix-it', pr: { number: 7, url: 'https://x/pull/7' } }, '[The Framework] adopt r1'), + true, + ) + const meta = JSON.parse(await readFile(join(dir, 'agents', 'u', 'r1.json'), 'utf8')) as { branch: string; pr: { number: number } } + assert.equal(meta.branch, 'claude/fix-it') + assert.equal(meta.pr.number, 7) + // Committed, not merely written: a dirty data checkout is what the next sync hard-resets. + assert.equal((await git('git', ['status', '--porcelain'], { cwd: dir })).stdout.trim(), '') + assert.equal((await git('git', ['log', '-1', '--format=%s'], { cwd: dir })).stdout.trim(), '[The Framework] adopt r1') + assert.equal((await git('git', ['rev-list', '--count', 'origin/tf-data..tf-data'], { cwd: dir })).stdout.trim(), '0', 'and pushed') + // The sync the daemon runs a minute later keeps it. + await withDataBranch(project, '[The Framework] data sync', async () => {}) + const after = JSON.parse(await readFile(join(dir, 'agents', 'u', 'r1.json'), 'utf8')) as { branch: string } + assert.equal(after.branch, 'claude/fix-it') + } finally { + await cleanup() + } +}) + +test('a run with no archive is reported as not patched, and nothing is committed (#1601)', async () => { + const { project, cleanup } = await repoWithArchive() + try { + const dir = dataWorktreePath(project) + const before = (await git('git', ['rev-parse', 'HEAD'], { cwd: dir })).stdout.trim() + assert.equal(await patchArchivedAgentOnDataBranch(project, 'nope', { branch: 'claude/x' }, '[The Framework] adopt nope'), false) + assert.equal((await git('git', ['rev-parse', 'HEAD'], { cwd: dir })).stdout.trim(), before) + } finally { + await cleanup() + } +}) diff --git a/packages/the-framework/src/archived-agent-patch.ts b/packages/the-framework/src/archived-agent-patch.ts new file mode 100644 index 000000000..c3202867a --- /dev/null +++ b/packages/the-framework/src/archived-agent-patch.ts @@ -0,0 +1,26 @@ +import { withDataBranch } from './data-branch.js' +import { patchArchivedAgent, type ArchivePatch } from './store/index.js' + +/** + * Patch a settled run's archived record on the data branch (#1601): synced with origin, patched, + * committed, pushed — the same funnel every other data write goes through (#1582). + * + * A patch written straight into the data checkout is not a fact yet: the next sync's rebase + * refuses a dirty tree and the funnel hard-resets it, so the patch is gone within a minute and + * no other machine ever saw it. Seen live on the cloud-work adoption before it went through here. + * + * True when the record now carries the patch, committed; a push that could not go out rides the + * next cycle, which is the funnel's owed-push rule. False when the run has no archive to patch. + */ +export async function patchArchivedAgentOnDataBranch( + cwd: string, + agentId: string, + patch: ArchivePatch, + message: string, +): Promise { + let patched = false + const result = await withDataBranch(cwd, message, async () => { + patched = await patchArchivedAgent(cwd, agentId, patch) + }) + return patched && (result.ok || result.committed) +} diff --git a/packages/the-framework/src/cloud-scratch-refs.SPEC.md b/packages/the-framework/src/cloud-scratch-refs.SPEC.md index eeb2d50ea..f97109824 100644 --- a/packages/the-framework/src/cloud-scratch-refs.SPEC.md +++ b/packages/the-framework/src/cloud-scratch-refs.SPEC.md @@ -1,8 +1,8 @@ -Deletes the two dead refs every Claude-web hand-off leaves on origin — the pre-hand-off `cloud-*` ref and the run branch — once it is provably safe, so they stop accumulating one pair per web run. +Deletes the dead refs web runs leave on origin — the `cloud-*` ref a hand-off pushes for the session to clone at, and any run branch that holds no work — once it is provably safe, so they stop accumulating. ## TLDR -- A web run pushes a `cloud-*` ref for the cloud session to clone at; the session then works on its own branch and opens its PR from there, so nothing ever consumes the ref again. Run branches used to reach origin too, pushed empty when a web run's worktree was reclaimed — teardown no longer pushes them, and the sweep clears the ones already there. +- A web run pushes a `cloud-*` ref for the cloud session to clone at; the session then works on its own branch and opens its PR from there, so nothing ever consumes the ref again. A web run's own branch never reaches origin — its checkout is reclaimed without a push once the cloud session has what it needs — while a local run's branch does, and is swept only once its work has landed. - The driver must not delete its own ref: it only learns "session created", never "clone finished", and a ref deleted in between strands the session. So the daemon sweeps instead, hourly, and waits out the race. - A ref goes only when every gate clears: it is about a day old, its commits are already on the default branch (the proof it holds no work — this is what protects a local run's branch carrying unmerged commits), it has no open pull request, and its agent is not one the daemon is still running. - The hand-off anchor is the one tip the default branch never absorbs — an empty commit no merge ever lands — so it clears the work gate its own way: a tip that changes nothing against its parent, on a parent that landed, holds no work. diff --git a/packages/the-framework/src/cloud-work.SPEC.md b/packages/the-framework/src/cloud-work.SPEC.md index 3de62cb3f..28ace4b41 100644 --- a/packages/the-framework/src/cloud-work.SPEC.md +++ b/packages/the-framework/src/cloud-work.SPEC.md @@ -4,7 +4,7 @@ Adopts the branch a cloud session actually worked on: each settled web run is ma - A web run hands the task to claude.ai and ends; the cloud session does the work on a branch of its own naming, never the branch the run was born on. Without adoption, every surface keyed to the run's branch — its dashboard row, its PR, CI watch, merge — stared at an empty branch and said "nothing committed" while the work sat on origin. - The match is exact, never guessed: the hand-off pushed a commit unique to the run for the session to clone at, so the session's branch — and only it — descends from that commit. A run matching no branch (the session has not pushed, or never will) or more than one is simply asked again next pass, and a run past the window (two days) stops being asked about. -- What gets recorded: the branch, and the pull request the session opened for it. A run that was armed for a PR the session never opened gets its draft PR opened by the daemon — the armed handoff finally resolving against the facts — unless the branch carries nothing beyond the hand-off itself. +- What gets recorded, as one commit on the data branch so every machine learns it: the branch, and the pull request the session opened for it. A run that was armed for a PR the session never opened gets its draft PR opened by the daemon — the armed handoff finally resolving against the facts — unless the branch carries nothing beyond the hand-off itself, or the session's pull requests could not be listed that pass: not knowing is never read as none, since the cost would be a second pull request. - Daemon-side by necessity: the branch does not exist yet when the run's own process ends — the cloud VM is still provisioning — so a later pass patches the run's record, the same way a late-opened PR already is. - Adoptions and failures are said out loud; a run still waiting is not, because waiting is its normal state. diff --git a/packages/the-framework/src/cloud-work.test.SPEC.md b/packages/the-framework/src/cloud-work.test.SPEC.md index ceeb2e0cf..427e7bb34 100644 --- a/packages/the-framework/src/cloud-work.test.SPEC.md +++ b/packages/the-framework/src/cloud-work.test.SPEC.md @@ -1,4 +1,4 @@ -Covers cloud-work adoption: the one branch descending from a run's hand-off anchor is adopted with the PR the session opened, a run armed for a PR the session never opened gets its draft opened and recorded, an unarmed run gets only its branch, a branch carrying nothing beyond the hand-off gets no PR, zero or two matching branches adopt nothing, runs outside the pass (non-web, live, anchorless, already answered, past the window) cost not even a remote listing, an unreachable remote never throws, and the daemon-facing service announces adoptions and failures while joining overlapping ticks. +Covers cloud-work adoption: the one branch descending from a run's hand-off anchor is adopted with the PR the session opened, a run armed for a PR the session never opened gets its draft opened and recorded, an unarmed run gets only its branch, a branch carrying nothing beyond the hand-off gets no PR, a PR listing that fails records the branch but opens no PR, a run whose record names some other branch is left alone, an adopted run still owed its armed PR is asked again until it has one, zero or two matching branches adopt nothing, runs outside the pass (non-web, live, anchorless, already answered, past the window) cost not even a remote listing, an unreachable remote never throws, and the daemon-facing service announces adoptions and failures while joining overlapping ticks. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/cloud-work.test.ts b/packages/the-framework/src/cloud-work.test.ts index bc02f6db8..b0ec13687 100644 --- a/packages/the-framework/src/cloud-work.test.ts +++ b/packages/the-framework/src/cloud-work.test.ts @@ -60,12 +60,9 @@ function deps(agents: AgentMeta[], git = fakeGit([{ ref: 'claude/fix-it', sha: H git: git.run, agents: async () => agents, prs: async () => prs, - adoptBranch: async (_cwd, agentId, branch) => { - recorded.branches.push({ agentId, branch }) - return true - }, - recordPr: async (_cwd, agentId, pr) => { - recorded.prs.push({ agentId, number: pr.number }) + patch: async (_cwd, agentId, patch) => { + if (patch.branch !== undefined) recorded.branches.push({ agentId, branch: patch.branch }) + if (patch.pr !== undefined) recorded.prs.push({ agentId, number: patch.pr.number }) return true }, openPr: async (_cwd, _agent, branch) => { @@ -191,3 +188,28 @@ test('startCloudWorkAdoption says adoptions out loud and joins overlapping ticks await adoption.tick() assert.equal(calls, 1, 'a stopped pass runs nothing') }) + +test('a PR listing that fails records the branch but opens nothing: "none" and "could not tell" must not look alike (#1601)', async () => { + // A transient gh failure used to read as "the session opened no PR" and open a second draft on + // a branch that already had one. + const { d, recorded } = deps([webRun()]) + d.prs = async () => { + throw new Error('gh: rate limited') + } + const result = await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, [{ agentId: ID, branch: 'claude/fix-it' }], 'the branch is a fact regardless') + assert.deepEqual(recorded.opened, [], 'no PR is opened on a listing this pass could not read') + assert.deepEqual(recorded.prs, []) + assert.equal(result.failed.length, 1) + assert.match(result.failed[0]!.error, /could not list the PRs/) +}) + +test('a run whose record names a branch that is neither its birth branch nor the matched head is left alone (#1601)', async () => { + // Its PR would otherwise be opened from the claude/* head and recorded against a branch it + // does not live on. + const { d, recorded } = deps([webRun({ branch: 'tf-renamed-by-hand' })]) + const result = await adoptCloudWork(CWD, d) + assert.deepEqual(recorded.branches, []) + assert.deepEqual(recorded.opened, []) + assert.deepEqual(result.adopted, []) +}) diff --git a/packages/the-framework/src/cloud-work.ts b/packages/the-framework/src/cloud-work.ts index 725433f9d..42c3b3be3 100644 --- a/packages/the-framework/src/cloud-work.ts +++ b/packages/the-framework/src/cloud-work.ts @@ -1,8 +1,10 @@ import { nodeGitRunner, type GitRunner } from './project.js' -import { ghPrsForBranch, nodeGhRunner, pickAgentPr, type GhRunner, type LinkedPr } from './dashboard/gh.js' +import { ghPrsForBranchOrThrow, pickAgentPr, type LinkedPr } from './dashboard/gh.js' import { openRemoteBranchPullRequest, type HandoffResult } from './dashboard/agent-handoff.js' import { agentBranchName } from './branch-names.js' -import { adoptAgentBranch, listAgents, recordAgentPr, startedAtFromAgentId, type AgentMeta } from './store/index.js' +import { listAgents, startedAtFromAgentId, type AgentMeta, type ArchivePatch } from './store/index.js' +import { patchArchivedAgentOnDataBranch } from './archived-agent-patch.js' +import { errorMessage } from './error-message.js' // Adopt the branch a cloud session actually worked on (#1601). // @@ -15,10 +17,10 @@ import { adoptAgentBranch, listAgents, recordAgentPr, startedAtFromAgentId, type // The hand-off anchor (#1601) makes the match exact rather than guessed: the driver pushes an // empty commit unique to the run as the ref the session clones at, so the session's branch — and // only it — descends from that commit. This pass walks origin's `claude/*` heads, matches each -// waiting run by that ancestry, and records what it finds onto the run's archive: the branch -// (the same patch-in-place `recordAgentPr` uses), the PR the session opened for it — and when the -// run was armed for a PR the session never opened, it opens the draft PR itself, which is the -// armed handoff finally resolving against the facts. +// waiting run by that ancestry, and records what it finds onto the run's archive as one commit +// on the data branch: the branch, the PR the session opened for it — and when the run was armed +// for a PR the session never opened, it opens the draft PR itself, which is the armed handoff +// finally resolving against the facts. // // Conservative on every unprovable case: a run matching no head (the session has not pushed, or // did nothing) or more than one (ancestry alone cannot say which) is simply retried next pass, @@ -47,15 +49,12 @@ export interface CloudWorkResult { /** Injectable seams so the pass is unit-testable off disk, off the network and off GitHub. */ export interface CloudWorkDeps { git?: GitRunner - gh?: GhRunner - /** The branch's full PR history (default {@link ghPrsForBranch}). */ + /** The branch's full PR history; a listing that fails must throw (default {@link ghPrsForBranchOrThrow}). */ prs?: (cwd: string, branch: string) => Promise /** The project's run records (default {@link listAgents}). */ agents?: (cwd: string) => Promise - /** Record the adopted branch (default {@link adoptAgentBranch}). */ - adoptBranch?: (cwd: string, agentId: string, branch: string) => Promise - /** Record the PR (default {@link recordAgentPr}). */ - recordPr?: (cwd: string, agentId: string, pr: { number: number; url: string }) => Promise + /** Record the adopted branch and PR on the run's archive (default {@link patchArchivedAgentOnDataBranch}). */ + patch?: (cwd: string, agentId: string, patch: ArchivePatch, message: string) => Promise /** Open the armed draft PR for a remote-only branch (default {@link openRemoteBranchPullRequest}). */ openPr?: (cwd: string, agent: AgentMeta, branch: string) => Promise /** The current time in ms (injected so tests can age runs deterministically). */ @@ -131,12 +130,10 @@ async function descendsFrom(git: GitRunner, cwd: string, anchor: string, head: C */ export async function adoptCloudWork(cwd: string, deps: CloudWorkDeps = {}): Promise { const git = deps.git ?? nodeGitRunner() - const gh = deps.gh ?? nodeGhRunner() - const prs = deps.prs ?? ghPrsForBranch + const prs = deps.prs ?? ghPrsForBranchOrThrow const agents = deps.agents ?? listAgents - const adoptBranch = deps.adoptBranch ?? adoptAgentBranch - const recordPr = deps.recordPr ?? recordAgentPr - const openPr = deps.openPr ?? ((c: string, agent: AgentMeta, branch: string) => openRemoteBranchPullRequest(c, agent, branch, { gh })) + const patchArchive = deps.patch ?? patchArchivedAgentOnDataBranch + const openPr = deps.openPr ?? openRemoteBranchPullRequest const now = deps.now ? deps.now() : Date.now() const result: CloudWorkResult = { adopted: [], failed: [] } @@ -162,24 +159,30 @@ export async function adoptCloudWork(cwd: string, deps: CloudWorkDeps = {}): Pro if (matches.length !== 1) continue const head = matches[0]! const branch = head.ref - - if (onBirthBranch(run) && !(await adoptBranch(cwd, run.id, branch))) { - result.failed.push({ agentId: run.id, error: `could not record ${branch} on the run's archive` }) - continue - } + // A run whose record names some other branch — neither the one it was born on nor this head + // — is not this pass's to answer: a PR opened here would be recorded against a branch it + // does not live on. + if (!onBirthBranch(run) && run.branch !== branch) continue // The PR the session opened for its branch, if any — filtered by the run's start so a // predecessor's PR on a reused name is never this run's, `latest` so the last PR that saw - // the branch answers (#1512). + // the branch answers (#1512). "None" and "could not list" must not look alike here: a + // listing that fails opens nothing this pass (a second draft PR on a branch that already + // has one is the cost of guessing), and the run is asked again next time. const since = run.startedAt ?? startedAtFromAgentId(run.id) - let pr = pickAgentPr(await prs(cwd, branch).catch((): LinkedPr[] => []), since, 'latest') + const listing = await prs(cwd, branch).then( + found => ({ ok: true as const, pr: pickAgentPr(found, since, 'latest') }), + (err: unknown) => ({ ok: false as const, error: errorMessage(err) }), + ) + let pr = listing.ok ? listing.pr : undefined let opened = false - - // The armed handoff, finally resolving (#1601): the run was armed for a PR, the session - // opened none, and the branch carries something beyond the hand-off itself — so the draft - // PR the wrapper's epilogue could never open (it saw only the empty run branch) opens now. - // A run whose PR arming was off gets its branch recorded and nothing else. - if (!pr && prArmed(run) && run.status === 'done' && head.sha !== run.cloudAnchor) { + if (!listing.ok) { + result.failed.push({ agentId: run.id, error: `could not list the PRs of ${branch} (${listing.error}), so no draft PR was opened this pass` }) + } else if (!pr && prArmed(run) && run.status === 'done' && head.sha !== run.cloudAnchor) { + // The armed handoff, finally resolving (#1601): the run was armed for a PR, the session + // opened none, and the branch carries something beyond the hand-off itself — so the draft + // PR the wrapper's epilogue could never open (it saw only the empty run branch) opens now. + // A run whose PR arming was off gets its branch recorded and nothing else. const openedPr = await openPr(cwd, run, branch) if (openedPr.ok && openedPr.number !== undefined && openedPr.url) { pr = { number: openedPr.number, url: openedPr.url, state: 'OPEN', title: '' } @@ -189,7 +192,17 @@ export async function adoptCloudWork(cwd: string, deps: CloudWorkDeps = {}): Pro } } - if (pr && run.pr === undefined) await recordPr(cwd, run.id, { number: pr.number, url: pr.url }) + // One commit on the data branch carries whatever this pass learned: the branch (first time + // only), the PR (once known). Nothing learned, nothing written — and nothing announced. + const patch: ArchivePatch = { + ...(onBirthBranch(run) ? { branch } : {}), + ...(pr && run.pr === undefined ? { pr: { number: pr.number, url: pr.url } } : {}), + } + if (Object.keys(patch).length === 0) continue + if (!(await patchArchive(cwd, run.id, patch, `[The Framework] adopt session ${run.id}'s cloud work`))) { + result.failed.push({ agentId: run.id, error: `could not record ${branch} on the run's archive` }) + continue + } result.adopted.push({ agentId: run.id, branch, diff --git a/packages/the-framework/src/dashboard-rpc/control.ts b/packages/the-framework/src/dashboard-rpc/control.ts index aedb9b50c..cc119f573 100644 --- a/packages/the-framework/src/dashboard-rpc/control.ts +++ b/packages/the-framework/src/dashboard-rpc/control.ts @@ -7,9 +7,10 @@ import { appendFlatTodoEntry, ticketForPrompt } from '../todo-loop.js' import { TICKETS_DIR, todoPriorityForTicket } from '../tickets.js' import { isTicketFile } from '../dashboard/tickets.js' import { releaseTicketLock } from '../ticket-locks.js' -import { findAgent, isSafeAgentId, recordAgentPr, worktreePath, type AgentMeta } from '../store/index.js' +import { findAgent, isSafeAgentId, worktreePath, type AgentMeta } from '../store/index.js' import { withAgentLock } from '../agent-locks.js' import { removeProjectWorktree, deleteProjectAgent } from '../worktrees.js' +import { patchArchivedAgentOnDataBranch } from '../archived-agent-patch.js' import { commitAgentWork, mergeAgentPr, openAgentPullRequest, pushAgentBranch, agentBranchFor, type HandoffResult } from '../dashboard/agent-handoff.js' import type { ChoiceBy } from '../events.js' import { isHandoffLevel, type HandoffLevel } from '../handoff-level.js' @@ -289,7 +290,7 @@ export async function sendOpenPullRequest(projectId: string, agentId: string): P // stream to carry the fact — but it is the same fact, and every surface reads it from the same // place either way rather than re-deriving it from branch names. if (opened.ok && opened.number !== undefined && opened.url) { - await recordAgentPr(target.cwd, agentId, { number: opened.number, url: opened.url }) + await patchArchivedAgentOnDataBranch(target.cwd, agentId, { pr: { number: opened.number, url: opened.url } }, `[The Framework] record the PR of session ${agentId}`) } return opened }, { ok: false, error: 'could not reach the device' }) diff --git a/packages/the-framework/src/dashboard/gh.ts b/packages/the-framework/src/dashboard/gh.ts index 04ab320cc..a4f03d0cd 100644 --- a/packages/the-framework/src/dashboard/gh.ts +++ b/packages/the-framework/src/dashboard/gh.ts @@ -144,9 +144,23 @@ function prCacheKey(cwd: string, branch?: string): string { * indistinguishable from "no PRs", which is what every caller would do with a failure anyway. */ export async function ghPrsForBranch(cwd: string, branch: string): Promise { - const fields = 'number,url,state,title,createdAt,headRefOid' - const args = ['pr', 'list', '--head', branch, '--state', 'all', '--limit', '20', '--json', fields] - const prs = await ghJson(args, cwd, []) + return linkedPrs(await ghJson(prListArgs(branch), cwd, [])) +} + +/** + * {@link ghPrsForBranch} for a caller about to *open* a PR (#1601): a listing that fails throws + * instead of reading as "no PRs", because "none" and "could not tell" must not look alike there — + * the difference is a second draft PR on a branch that already has one. + */ +export async function ghPrsForBranchOrThrow(cwd: string, branch: string): Promise { + return linkedPrs(JSON.parse(await readGh(prListArgs(branch), cwd)) as LinkedPr[]) +} + +function prListArgs(branch: string): string[] { + return ['pr', 'list', '--head', branch, '--state', 'all', '--limit', '20', '--json', 'number,url,state,title,createdAt,headRefOid'] +} + +function linkedPrs(prs: LinkedPr[]): LinkedPr[] { return prs.map(pr => ({ number: pr.number, url: pr.url, diff --git a/packages/the-framework/src/driver/cloud.SPEC.md b/packages/the-framework/src/driver/cloud.SPEC.md index f80adccf4..471bbd23d 100644 --- a/packages/the-framework/src/driver/cloud.SPEC.md +++ b/packages/the-framework/src/driver/cloud.SPEC.md @@ -7,7 +7,7 @@ A driver that hands the whole task to Claude Code on the web: it starts a real c - The project root is trusted for the CLI before the hand-off — worktrees inherit the root's trust, and starting a web agent is itself the user's trust decision — so the CLI's interactive trust question, which a background run could never answer, does not fire. A dialog that appears anyway (the write failed or was rejected) still fails fast with the manual fix named instead of timing out with nothing to show. - Nothing the user typed can ever reach a shell as syntax. - The session it creates is repo-bound, not a bundle (#1320): with nonessential traffic disabled the CLI's server-side bundle experiment reads off, so a failed GitHub-App preflight falls through to a session that clones from GitHub and can push — instead of silently uploading a local bundle whose work can never leave the VM (anthropics/claude-code#81776). -- Before the hand-off, the anchor is pushed to origin under the agent's own id: an empty commit on top of HEAD, unique to this run and minted without moving any branch. The session clones at it, so the branch it does its work on — a name of the cloud's own choosing — descends from it, and that ancestry is how the daemon later recognizes which branch is this run's. The ref is explicit and slash-free because the CLI's default revision pin is the current local branch — which an agent workspace's local-only branch fails — and a slash-carrying ref never resolves on the cloud side even when pushed (anthropics/claude-code#87235). A push that fails degrades to the old behavior and says so, naming `--teleport` as the recovery path; a repo where the anchor cannot be minted hands off plain HEAD, and the run is simply never matched to its branch. +- Before the hand-off, the anchor is pushed to origin under the agent's own id: an empty commit on top of HEAD, unique to this run and minted without moving any branch. The session clones at it, so the branch it does its work on — a name of the cloud's own choosing — descends from it, and that ancestry is how the daemon later recognizes which branch is this run's. The ref is explicit and slash-free because the CLI's default revision pin is the current local branch — which an agent workspace's local-only branch fails — and a slash-carrying ref never resolves on the cloud side even when pushed (anthropics/claude-code#87235). A hand-off whose anchor cannot be minted or pushed goes ahead with no ref and says so, naming `--teleport` as the recovery path; such a run is simply never matched to its branch. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/driver/cloud.test.SPEC.md b/packages/the-framework/src/driver/cloud.test.SPEC.md index 92b471146..98b7d14fd 100644 --- a/packages/the-framework/src/driver/cloud.test.SPEC.md +++ b/packages/the-framework/src/driver/cloud.test.SPEC.md @@ -1,4 +1,4 @@ -Covers the cloud hand-off: creating exactly one web session per agent no matter how often the loop prompts, surfacing the session link the way other targets surface theirs, trusting the project root on the user's behalf before the hand-off (visibly, best-effort, never clobbering a config it cannot parse) and still failing fast with the manual fix when the dialog appears anyway, keeping user text out of the shell, the web location — not the driver — being the one that ends an agent at its hand-off, and the repo-bound handshake (#1320): the hand-off anchor — an empty commit unique to the run — pushed under the slash-free agent id, handed over as the session's ref and reported on the result, a repo that cannot mint the anchor handing off plain HEAD with none reported, a failed push degrading to no ref with the recovery path named, and the nonessential-traffic switch pinned as what keeps the session off the bundle path. +Covers the cloud hand-off: creating exactly one web session per agent no matter how often the loop prompts, surfacing the session link the way other targets surface theirs, trusting the project root on the user's behalf before the hand-off (visibly, best-effort, never clobbering a config it cannot parse) and still failing fast with the manual fix when the dialog appears anyway, keeping user text out of the shell, the web location — not the driver — being the one that ends an agent at its hand-off, and the repo-bound handshake (#1320): the hand-off anchor — an empty commit unique to the run — pushed under the slash-free agent id, handed over as the session's ref and reported on the result, a failed push degrading to no ref with the recovery path named, and the nonessential-traffic switch pinned as what keeps the session off the bundle path. ## Before modifying/creating SPEC.md files diff --git a/packages/the-framework/src/driver/cloud.test.ts b/packages/the-framework/src/driver/cloud.test.ts index 90b66403d..7164c5db6 100644 --- a/packages/the-framework/src/driver/cloud.test.ts +++ b/packages/the-framework/src/driver/cloud.test.ts @@ -42,19 +42,16 @@ function fakePty(output: string, calls: AgentPtyOptions[] = []) { const ANCHOR = 'a'.repeat(40) /** - * A git runner that records its calls; `fail` makes every call reject (#1320), `noAnchor` - * makes only the anchor mint fail (#1601). `commit-tree` answers with a fixed sha. + * A git runner that records its calls; `fail` makes every call reject (#1320). `commit-tree` + * answers with a fixed sha. */ -function fakeGit(calls: string[][] = [], fail = false, noAnchor = false) { +function fakeGit(calls: string[][] = [], fail = false) { return { calls, run: async (args: string[], _cwd: string): Promise => { calls.push([...args]) if (fail) throw new Error('no pushable remote') - if (args[0] === 'commit-tree') { - if (noAnchor) throw new Error('cannot mint the anchor') - return `${ANCHOR}\n` - } + if (args[0] === 'commit-tree') return `${ANCHOR}\n` return '' }, } @@ -371,16 +368,6 @@ test('the hand-off pushes the anchor under the agent id and hands the session th assert.ok(events.some(e => e.type === 'result' && e.anchorSha === ANCHOR)) }) -test('a repo where the anchor cannot be minted hands off plain HEAD, with no anchor reported (#1601)', async () => { - const git = fakeGit([], false, true) - const events: DriverEvent[] = [] - const session = await driverWith(CREATED, [], git).start({ cwd: '/repo', onEvent: e => events.push(e) }) - const turn = await session.prompt('go') - assert.equal(turn.sessionId, SESSION) - assert.ok(git.calls.some(args => args.join(' ') === `push origin HEAD:refs/heads/${session.id}`)) - assert.ok(events.every(e => e.type !== 'result' || e.anchorSha === undefined)) -}) - test('a failed pre-push falls back to no ref, says so, and still hands off (#1320)', async () => { const events: DriverEvent[] = [] const ptyCalls: AgentPtyOptions[] = [] diff --git a/packages/the-framework/src/driver/cloud.ts b/packages/the-framework/src/driver/cloud.ts index d5c39fce1..722c051b3 100644 --- a/packages/the-framework/src/driver/cloud.ts +++ b/packages/the-framework/src/driver/cloud.ts @@ -261,18 +261,13 @@ export class CloudSession implements DriverSession { // work on a branch of its own naming (`claude/*`), never the designated run branch — and // since every commit it makes descends from what it cloned, a commit unique to this run is // the one exact mark by which the daemon can later recognize which `claude/*` branch is - // this run's. A repo where the anchor cannot be minted hands off plain HEAD: the session - // still works, the run is simply never matched to its branch. + // this run's. Minting and pushing are one step: a checkout that cannot mint an empty commit + // on its HEAD has no HEAD to push either. const git = this.config.git ?? nodeGitRunner() - let anchor: string | undefined - try { - anchor = (await git(['commit-tree', 'HEAD^{tree}', '-p', 'HEAD', '-m', `[The Framework] web hand-off ${this.id}`], this.cwd)).trim() || undefined - } catch { - anchor = undefined - } let ref: string | undefined = this.id try { - await git(['push', 'origin', `${anchor ?? 'HEAD'}:refs/heads/${this.id}`], this.cwd) + const anchor = (await git(['commit-tree', 'HEAD^{tree}', '-p', 'HEAD', '-m', `[The Framework] web hand-off ${this.id}`], this.cwd)).trim() + await git(['push', 'origin', `${anchor}:refs/heads/${this.id}`], this.cwd) this.anchorSha = anchor } catch (err) { ref = undefined diff --git a/packages/the-framework/src/store/agent-store.test.ts b/packages/the-framework/src/store/agent-store.test.ts index 26c63b3e6..c1c54dbad 100644 --- a/packages/the-framework/src/store/agent-store.test.ts +++ b/packages/the-framework/src/store/agent-store.test.ts @@ -10,8 +10,7 @@ import { readLiveMeta, readLiveMetas, archiveWorktreeAgent, - recordAgentPr, - adoptAgentBranch, + patchArchivedAgent, restoreArchivedAgent, listWorktreeDirs, reconcileOrphanedAgents, @@ -670,37 +669,31 @@ test('archiveWorktreeAgent records a run that died mid-flight as stopped, not ru assert.equal((JSON.parse(fs.files.get(join(CWD, '.the-framework', 'agents', 'r1.json'))!) as AgentMeta).status, 'stopped') }) -test('recordAgentPr patches the archived meta, so a PR opened after the run still lands on it (E6)', async () => { +test('patchArchivedAgent records a PR opened after the run on its archived meta (E6)', async () => { // The dashboard's Open PR button runs after the session's process is gone, so there is no event // stream left to carry the fact — but it is the same fact, and every surface reads it from the // same place either way. const fs = memFs(worktreeFiles('r1', { version: 1, status: 'done', id: 'r1', startedAt: AT, updatedAt: AT })) await archiveWorktreeAgent(worktreeAt('r1'), CWD, fs) - assert.equal(await recordAgentPr(CWD, 'r1', { number: 42, url: 'https://x/pull/42' }, fs), true) + assert.equal(await patchArchivedAgent(CWD, 'r1', { pr: { number: 42, url: 'https://x/pull/42' } }, fs), true) assert.deepEqual((await listAgents(CWD, fs)).find(r => r.id === 'r1')?.pr, { number: 42, url: 'https://x/pull/42' }) }) -test('adoptAgentBranch patches the archived meta with the branch the cloud work landed on (#1601)', async () => { +test('patchArchivedAgent records the branch the cloud work landed on (#1601)', async () => { // The cloud VM pushes its `claude/*` branch after the wrapper's process is gone, so the fact // arrives the same way a late PR does: patched onto the archive, read by every surface. const fs = memFs(worktreeFiles('r1', { version: 1, status: 'done', id: 'r1', startedAt: AT, updatedAt: AT, branch: 'tf-agent-r1' })) await archiveWorktreeAgent(worktreeAt('r1'), CWD, fs) - assert.equal(await adoptAgentBranch(CWD, 'r1', 'claude/fix-the-thing', fs), true) + assert.equal(await patchArchivedAgent(CWD, 'r1', { branch: 'claude/fix-the-thing' }, fs), true) assert.equal((await listAgents(CWD, fs)).find(r => r.id === 'r1')?.branch, 'claude/fix-the-thing') }) -test('adoptAgentBranch leaves the record as it was when there is nothing to patch (#1601)', async () => { - const fs = memFs() - assert.equal(await adoptAgentBranch(CWD, 'nope', 'claude/x', fs), false) - assert.equal(await adoptAgentBranch(CWD, '../escape', 'claude/x', fs), false, 'and an unsafe id is refused') -}) - -test('recordAgentPr leaves the record as it was when there is nothing to patch (E6)', async () => { +test('patchArchivedAgent leaves the record as it was when there is nothing to patch (E6/#1601)', async () => { // Best-effort: the cost of missing it is one surface having to ask gh, which is what all of them // used to do anyway. const fs = memFs() - assert.equal(await recordAgentPr(CWD, 'nope', { number: 1, url: 'u' }, fs), false) - assert.equal(await recordAgentPr(CWD, '../escape', { number: 1, url: 'u' }, fs), false, 'and an unsafe id is refused') + assert.equal(await patchArchivedAgent(CWD, 'nope', { pr: { number: 1, url: 'u' } }, fs), false) + assert.equal(await patchArchivedAgent(CWD, '../escape', { branch: 'claude/x' }, fs), false, 'and an unsafe id is refused') }) test('archiveWorktreeAgent is forgiving of a worktree with no run', async () => { diff --git a/packages/the-framework/src/store/agent-store.ts b/packages/the-framework/src/store/agent-store.ts index c3d5fdee9..2c6c3db61 100644 --- a/packages/the-framework/src/store/agent-store.ts +++ b/packages/the-framework/src/store/agent-store.ts @@ -1146,49 +1146,23 @@ export async function readEventLog(cwd: string, fs: StoreFs = nodeStoreFs()): Pr } } -/** - * Record the pull request a finished agent's work is on (E6). - * - * A PR opened by the dashboard's button happens *after* the agent's process is gone, so there is no - * event stream left to carry it — but the fact is exactly as worth recording as the one the agent - * emits for itself, and every surface reads it from the same place either way. So the archived meta - * is patched in place. - * - * Best-effort and idempotent: an agent with no archive yet, an unreadable meta, or a failed write - * simply leaves the record as it was. The cost of missing it is one surface having to ask `gh`, - * which is what all of them used to do. - */ -export async function recordAgentPr( - cwd: string, - agentId: string, - pr: { number: number; url: string }, - fs: StoreFs = nodeStoreFs(), -): Promise { - if (!isSafeAgentId(agentId)) return false - try { - const archive = await findArchive(fs, cwd, agentId) - if (!archive) return false - const meta = await readMetaFile(fs, archive.meta) - if (!meta) return false - await fs.write(archive.meta, JSON.stringify({ ...meta, pr })) - return true - } catch { - return false - } -} +/** The facts a settled run learns after its process is gone: the PR its work is on, the branch it landed on. */ +export type ArchivePatch = Partial> /** - * Record the branch a cloud session's work actually landed on (#1601), patching the archived - * meta in place the same way {@link recordAgentPr} does and for the same reason: the fact - * becomes known only after the agent's process is gone — the cloud VM pushes its `claude/*` - * branch while the local wrapper is already history — so there is no event stream left to - * carry it. Every surface resolves the recorded branch first, so this one write is what turns - * a run's "nothing committed" row into its real branch, PR and merge state. + * Patch an archived run's record with a fact discovered once the agent's process is gone (E6, + * #1601): the pull request opened for its work, or the branch a cloud session's work landed + * on. There is no event stream left to carry it, and every surface reads the record, so this + * one write is what turns a "nothing committed" row into its real branch and PR. + * + * A plain file write: the archive lives on the data branch's checkout, and a fact written there + * is only durable once committed — {@link patchArchivedAgentOnDataBranch} is the funneled form + * every caller outside a test uses. */ -export async function adoptAgentBranch( +export async function patchArchivedAgent( cwd: string, agentId: string, - branch: string, + patch: ArchivePatch, fs: StoreFs = nodeStoreFs(), ): Promise { if (!isSafeAgentId(agentId)) return false @@ -1197,7 +1171,7 @@ export async function adoptAgentBranch( if (!archive) return false const meta = await readMetaFile(fs, archive.meta) if (!meta) return false - await fs.write(archive.meta, JSON.stringify({ ...meta, branch })) + await fs.write(archive.meta, JSON.stringify({ ...meta, ...patch })) return true } catch { return false diff --git a/packages/the-framework/src/store/index.ts b/packages/the-framework/src/store/index.ts index 350bbbb8d..2e2fb95f5 100644 --- a/packages/the-framework/src/store/index.ts +++ b/packages/the-framework/src/store/index.ts @@ -14,8 +14,8 @@ export { readLiveMetas as readLiveMetas, archiveWorktreeAgent, archivedAgentPaths, - recordAgentPr, - adoptAgentBranch, + patchArchivedAgent, + type ArchivePatch, restoreArchivedAgent, listWorktreeDirs, worktreeDirEntries,