fix: verify broker name release before reuse

.github/workflows/ci.yml

@@ -18,6 +18,10 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v6
+        with:
+          # PR checkouts otherwise use GitHub's synthetic merge commit, while
+          # the attestation is required to bind the exact reviewed head SHA.
+          ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
       - name: Setup Node
         uses: actions/setup-node@v6

scripts/verify-packed-e2e.mjs

@@ -21,6 +21,11 @@ const checks = []
 try {
   assert.match(headSha, /^[0-9a-f]{40}$/u, 'head SHA must be a full Git object ID')
   assert.match(testedCheckoutSha, /^[0-9a-f]{40}$/u, 'tested checkout SHA must be a full Git object ID')
+  assert.equal(
+    headSha,
+    testedCheckoutSha,
+    'FACTORY_E2E_HEAD_SHA must match the checkout being packed and tested',
+  )
   checks.push('head-sha-bound')
 
   mkdirSync(packDir, { recursive: true })

src/fleet/internal-fleet-client.test.ts

@@ -38,6 +38,7 @@ class FakeHarnessDriverClient implements HarnessDriverClientLike {
     if (this.throwOnRelease) {
       throw new Error('release failed')
     }
+    this.agents = this.agents.filter((agent) => agent.name !== name)
     return { name }
   }
 
@@ -532,6 +533,7 @@ describe('InternalFleetClient', () => {
               data: { error: 'internal reply dropped', success: false },
             })
           }
+          this.agents = this.agents.filter((agent) => agent.name !== name)
           return { name }
         }
       }
@@ -905,11 +907,47 @@ describe('InternalFleetClient', () => {
 
     expect(harness.released).toEqual([{ name: 'ar-1-impl', reason: 'done' }])
     await expect(fleet.roster()).resolves.toEqual({
-      agents: [{ name: 'ar-1-impl' }],
+      agents: [],
       nodes: [{ name: 'self', capabilities: ['spawn:claude', 'spawn:codex', 'workflow:run'], live: true }],
     })
   })
 
+  it('retries a successful release acknowledgement until the broker name disappears', async () => {
+    vi.useFakeTimers()
+    try {
+      class RetainedFirstReleaseHarnessDriverClient extends FakeHarnessDriverClient {
+        attempts = 0
+
+        override async release(name: string, reason?: string): Promise<{ name: string }> {
+          this.attempts += 1
+          this.released.push({ name, reason })
+          if (this.attempts > 1) {
+            this.agents = this.agents.filter((agent) => agent.name !== name)
+          }
+          return { name }
+        }
+      }
+
+      const harness = new RetainedFirstReleaseHarnessDriverClient()
+      harness.agents = [{ name: 'ar-stale-review', cli: 'codex' }]
+      const logger = { warn: vi.fn() }
+      const fleet = new InternalFleetClient({ client: harness, logger })
+
+      const released = fleet.release('ar-stale-review', 'reconciled-missing')
+      await vi.advanceTimersByTimeAsync(250)
+      await released
+
+      expect(harness.attempts).toBe(2)
+      expect(harness.agents).toEqual([])
+      expect(logger.warn).toHaveBeenCalledWith(
+        '[factory-sdk] released broker name is still present; retrying release',
+        expect.objectContaining({ name: 'ar-stale-review', attempt: 1, maxAttempts: 3 }),
+      )
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('re-adopts tracked workers and synthesizes exits for workers missing after restart', async () => {
     const harness = new FakeHarnessDriverClient()
     harness.agents = [{ name: 'ar-1-impl' }]

src/fleet/internal-fleet-client.ts

@@ -283,7 +283,6 @@ export class InternalFleetClient implements FleetClient {
     for (let attempt = 1; attempt <= RELEASE_RETRY_MAX_ATTEMPTS; attempt += 1) {
       try {
         await this.#client.release(name, reason)
-        return
       } catch (error) {
         if (attempt >= RELEASE_RETRY_MAX_ATTEMPTS || !isRetryableReleaseError(error)) {
           throw error
@@ -295,7 +294,47 @@ export class InternalFleetClient implements FleetClient {
           error,
         })
         await sleep(RELEASE_RETRY_BACKOFF_MS * attempt)
+        continue
       }
+
+      // A successful HTTP acknowledgement is not sufficient proof that the
+      // broker name is reusable. Under concurrent startup reconciliation the
+      // worker can disappear while a stale inventory row remains, making an
+      // immediate same-name resume fail with `agent already exists`. Verify the
+      // raw Harness Driver inventory and repeat the public release until the
+      // broker itself no longer advertises the name.
+      let retained: boolean
+      try {
+        const agents = await this.#client.listAgents()
+        if (!Array.isArray(agents)) {
+          throw new TypeError('Expected Harness Driver listAgents() to return an array')
+        }
+        retained = agents.some((agent) => agent?.name === name)
+      } catch (error) {
+        if (attempt >= RELEASE_RETRY_MAX_ATTEMPTS) throw error
+        this.#logger?.warn?.('[factory-sdk] unable to verify released broker name; retrying release', {
+          name,
+          attempt,
+          maxAttempts: RELEASE_RETRY_MAX_ATTEMPTS,
+          error,
+        })
+        await sleep(RELEASE_RETRY_BACKOFF_MS * attempt)
+        continue
+      }
+      if (!retained) return
+
+      const retainedError = Object.assign(
+        new Error(`Broker still reports agent ${name} after a successful release acknowledgement`),
+        { code: 'release_not_observed', retryable: true },
+      )
+      if (attempt >= RELEASE_RETRY_MAX_ATTEMPTS) throw retainedError
+      this.#logger?.warn?.('[factory-sdk] released broker name is still present; retrying release', {
+        name,
+        attempt,
+        maxAttempts: RELEASE_RETRY_MAX_ATTEMPTS,
+        error: retainedError,
+      })
+      await sleep(RELEASE_RETRY_BACKOFF_MS * attempt)
     }
   }
 

src/packed-e2e-attestation.test.ts

@@ -0,0 +1,23 @@
+import { spawnSync } from 'node:child_process'
+import { resolve } from 'node:path'
+
+import { describe, expect, it } from 'vitest'
+
+describe('packed E2E attestation', () => {
+  it('rejects an attestation head that differs from the tested checkout', () => {
+    const root = resolve(import.meta.dirname, '..')
+    const result = spawnSync(process.execPath, [resolve(root, 'scripts/verify-packed-e2e.mjs')], {
+      cwd: root,
+      encoding: 'utf8',
+      env: {
+        ...process.env,
+        FACTORY_E2E_HEAD_SHA: '0000000000000000000000000000000000000000',
+      },
+    })
+
+    expect(result.status).not.toBe(0)
+    expect(result.stderr).toContain(
+      'FACTORY_E2E_HEAD_SHA must match the checkout being packed and tested',
+    )
+  })
+})