Cloud-work adoption asks git once per run, not once per run per head

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, 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.
+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. One case runs against real git rather than a stand-in — a checkout that has never seen the session's branches picks out the one descending from its anchor and leaves the one forked before it — so that the ancestry question is checked against git's own answer and not only against a fake that agrees with whatever it is asked.
 
 ## Before modifying/creating SPEC.md files
 

packages/the-framework/src/cloud-work.test.ts

@@ -1,9 +1,16 @@
 import { strict as assert } from 'node:assert'
 import { test } from 'node:test'
+import { mkdtemp, 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 { 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'
 
+const sh = promisify(execFile)
+
 // #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.
 
@@ -28,20 +35,21 @@ function webRun(over: Partial<AgentMeta> = {}): AgentMeta {
 }
 
 /**
- * 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).
+ * A git runner speaking just enough of the pass's dialect: `fetch` succeeds, and `for-each-ref
+ * --contains=<anchor>` answers with the `heads` whose sha is listed under that anchor in
+ * `descends`, formatted as the remote-tracking refs the fetch would have written.
  */
 function fakeGit(heads: { ref: string; sha: string }[], descends: Record<string, string[]> = { [ANCHOR]: [HEAD_SHA] }) {
   const calls: string[][] = []
   const run = async (args: string[], _cwd: string): Promise<string> => {
     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')
+    if (args[0] === 'fetch') return ''
+    if (args[0] === 'for-each-ref') {
+      const anchor = /^--contains=(.+)$/.exec(args[1] ?? '')?.[1] ?? ''
+      return heads
+        .filter(h => descends[anchor]?.includes(h.sha))
+        .map(h => `${h.sha} refs/remotes/origin/${h.ref}`)
+        .join('\n')
     }
     throw new Error(`unexpected git ${args.join(' ')}`)
   }
@@ -142,7 +150,7 @@ test('runs outside the pass: non-web, still running, no anchor, already adopted,
   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')
+  assert.deepEqual(git.calls, [], 'nothing waiting means not even a fetch')
 })
 
 test('a run already adopted but still owed its armed PR keeps being asked about (#1601)', async () => {
@@ -157,7 +165,7 @@ test('a run already adopted but still owed its armed PR keeps being asked about
 test('no remote, or a remote that cannot be reached, adopts nothing and never throws (#1601)', async () => {
   const git = {
     run: async (args: string[]): Promise<string> => {
-      if (args[0] === 'ls-remote') throw new Error('no remote')
+      if (args[0] === 'fetch') throw new Error('no remote')
       throw new Error('unexpected')
     },
   }
@@ -213,3 +221,119 @@ test('a run whose record names a branch that is neither its birth branch nor the
   assert.deepEqual(recorded.opened, [])
   assert.deepEqual(result.adopted, [])
 })
+
+/**
+ * A real repo standing in for the daemon's checkout, plus a separate clone standing in for the
+ * cloud VM. The `claude/*` branches are pushed only from the clone, so the daemon's checkout has
+ * never seen them — which is the whole point: a fixture that pushed them from the checkout under
+ * test would already hold the remote-tracking refs `git push` writes, and the pass's fetch would
+ * have nothing left to prove.
+ */
+async function repoWithCloudHeads(): Promise<{
+  project: string
+  anchor: string
+  strandedAnchor: string
+  cleanup: () => Promise<void>
+}> {
+  const origin = await mkdtemp(join(tmpdir(), 'framework-cloud-work-origin-'))
+  const project = await mkdtemp(join(tmpdir(), 'framework-cloud-work-'))
+  const session = await mkdtemp(join(tmpdir(), 'framework-cloud-work-session-'))
+  const identify = async (cwd: string) => {
+    for (const cfg of [
+      ['user.email', 'test@example.com'],
+      ['user.name', 'Test'],
+      ['commit.gpgsign', 'false'],
+    ]) {
+      await sh('git', ['config', ...cfg], { cwd })
+    }
+  }
+  await sh('git', ['init', '-q', '--bare'], { cwd: origin })
+  await sh('git', ['init', '-q', '-b', 'main'], { cwd: project })
+  await identify(project)
+  await sh('git', ['remote', 'add', 'origin', origin], { cwd: project })
+
+  const git = (...args: string[]) => sh('git', args, { cwd: project })
+  const head = async () => (await git('rev-parse', 'HEAD')).stdout.trim()
+  await writeFile(join(project, 'a.txt'), 'base')
+  await git('add', '-A')
+  await git('commit', '-qm', 'base')
+  const base = await head()
+  await git('push', '-q', 'origin', 'main')
+
+  // The two hand-off anchors the driver pushes from this checkout: one the session will build
+  // on, one whose session never pushes anything at all.
+  await git('checkout', '-q', '-b', 'run', base)
+  await git('commit', '-q', '--allow-empty', '-m', 'hand-off anchor')
+  const anchor = await head()
+  await git('push', '-q', 'origin', 'HEAD:refs/heads/tf-agent-run')
+  await git('checkout', '-q', '-b', 'stranded', base)
+  await git('commit', '-q', '--allow-empty', '-m', 'hand-off anchor 2')
+  const strandedAnchor = await head()
+  await git('push', '-q', 'origin', 'HEAD:refs/heads/tf-agent-stranded')
+  await git('checkout', '-q', 'main')
+
+  // The cloud VM: a different clone, which is where every `claude/*` branch is pushed from.
+  await sh('git', ['clone', '-q', origin, session])
+  await identify(session)
+  const vm = (...args: string[]) => sh('git', args, { cwd: session })
+  await vm('checkout', '-q', '-b', 'work', anchor)
+  await vm('commit', '-q', '--allow-empty', '-m', 'session work')
+  await vm('push', '-q', 'origin', 'HEAD:refs/heads/claude/this-run')
+  // And a `claude/*` branch forked before the anchor exists: ancestry must rule it out.
+  await vm('checkout', '-q', '-b', 'unrelated', base)
+  await vm('commit', '-q', '--allow-empty', '-m', 'someone else')
+  await vm('push', '-q', 'origin', 'HEAD:refs/heads/claude/not-this-run')
+
+  return {
+    project,
+    anchor,
+    strandedAnchor,
+    cleanup: async () => {
+      for (const dir of [project, origin, session]) await rm(dir, { recursive: true, force: true })
+    },
+  }
+}
+
+test('against real git: the anchor picks out its own `claude/*` head and no other (#1601/#1607)', async () => {
+  // The other tests speak to a fake git, which will agree with whatever commands the pass sends
+  // it. This one runs the real ones, so a wrong refspec or a wrong ancestry query is caught here.
+  const { project, anchor, strandedAnchor, cleanup } = await repoWithCloudHeads()
+  try {
+    // Nothing local knows about the session's branches yet: the fetch has to go and get them.
+    const before = await sh('git', ['for-each-ref', '--format=%(refname)', 'refs/remotes/origin/claude/'], { cwd: project })
+    assert.equal(before.stdout.trim(), '', 'the checkout under test has never seen a `claude/*` head')
+
+    const recorded: { agentId: string; branch: string }[] = []
+    const seams = (cloudAnchor: string): CloudWorkDeps => ({
+      agents: async () => [webRun({ cloudAnchor, handoff: { push: true, pr: false } })],
+      prs: async () => [],
+      patch: async (_cwd, agentId, patch) => {
+        if (patch.branch !== undefined) recorded.push({ agentId, branch: patch.branch })
+        return true
+      },
+      now: () => NOW,
+    })
+
+    const matched = await adoptCloudWork(project, seams(anchor))
+    assert.deepEqual(
+      matched.adopted.map(a => a.branch),
+      ['claude/this-run'],
+      'the branch descending from the anchor, and not the one forked before it',
+    )
+    assert.deepEqual(recorded, [{ agentId: ID, branch: 'claude/this-run' }])
+    assert.deepEqual(matched.failed, [])
+
+    // The fetch wrote remote-tracking refs rather than leaving the objects reachable only from
+    // `FETCH_HEAD`, which is what keeps the next garbage collection from throwing them away.
+    const after = (await sh('git', ['for-each-ref', '--format=%(refname)', 'refs/remotes/origin/claude/'], { cwd: project })).stdout
+    assert.match(after, /refs\/remotes\/origin\/claude\/this-run/)
+
+    // A run whose session pushed nothing matches nothing, and is left for the next pass.
+    recorded.length = 0
+    const stranded = await adoptCloudWork(project, seams(strandedAnchor))
+    assert.deepEqual(stranded.adopted, [], 'no head descends from that anchor')
+    assert.deepEqual(recorded, [])
+  } finally {
+    await cleanup()
+  }
+})

packages/the-framework/src/cloud-work.ts

@@ -91,35 +91,50 @@ function waitingRuns(agents: AgentMeta[], now: number): AgentMeta[] {
   })
 }
 
-/** 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
+/** Where the pass keeps origin's `claude/*` heads locally. */
+const CLOUD_HEAD_PREFIX = 'refs/remotes/origin/claude/'
+
+/**
+ * Bring origin's `claude/*` heads local, once for the whole pass rather than once per head.
+ *
+ * The objects belong to a cloud VM, so they have to be fetched before any ancestry can be read.
+ * This replaces a fetch per unmatched head per waiting run (#1607) — the saving is the call
+ * count, not the transfer: a second fetch of heads already local negotiates and sends nothing.
+ *
+ * The destination refspec is named rather than left to chance. An ordinary checkout would write
+ * these refs anyway, because git opportunistically updates the remote-tracking branches its
+ * configured refspec covers even when the command line names its own; a checkout cloned with a
+ * narrower refspec — `--single-branch` — would not, and would re-fetch every pass forever.
+ *
+ * Pruned, because these refs are now a standing local copy of a list that used to be read live
+ * from origin each pass: without it a `claude/*` branch deleted on origin would go on matching.
+ */
+async function fetchCloudHeads(git: GitRunner, cwd: string): Promise<void> {
+  await git(['fetch', '--prune', 'origin', `+refs/heads/claude/*:${CLOUD_HEAD_PREFIX}*`], cwd)
 }
 
 /**
- * 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.
+ * The `claude/*` heads descending from `anchor` — the proof a branch is this run's, asked of git
+ * once for the whole set rather than once per head.
+ *
+ * `--contains` is the same ancestry question `merge-base --is-ancestor` answered one head at a
+ * time, and asking it this way also answers "how many" in the same call. Unprovable reads as no
+ * match and is retried next pass: an anchor whose object is not local is exactly the case where
+ * the session has pushed nothing for it to be an ancestor of.
  */
-async function descendsFrom(git: GitRunner, cwd: string, anchor: string, head: CloudHead): Promise<boolean> {
-  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()
+async function headsDescendingFrom(git: GitRunner, cwd: string, anchor: string): Promise<CloudHead[]> {
+  const listing = await git(
+    ['for-each-ref', `--contains=${anchor}`, '--format=%(objectname) %(refname)', CLOUD_HEAD_PREFIX],
+    cwd,
+  ).catch(() => '')
+  const heads: CloudHead[] = []
+  for (const line of listing.split('\n')) {
+    const head = /^([0-9a-f]{40,64}) (.+)$/.exec(line.trim())
+    if (head?.[2]?.startsWith(CLOUD_HEAD_PREFIX)) {
+      heads.push({ ref: `claude/${head[2].slice(CLOUD_HEAD_PREFIX.length)}`, sha: head[1]! })
+    }
+  }
+  return heads
 }
 
 /**
@@ -140,20 +155,14 @@ export async function adoptCloudWork(cwd: string, deps: CloudWorkDeps = {}): Pro
   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)
+    await fetchCloudHeads(git, 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)
-    }
+    const matches = await headsDescendingFrom(git, cwd, run.cloudAnchor!)
     // 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