Repository navigation
feat(babysitter): keep the fixer's PR checkout across reused sandboxes (F3a) #635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -607,8 +607,9 @@ var SUMMARY_MAX_CHARS = 4e3; | |
| var REFUSED_PATHS = String.raw`^\.github/workflows/|(^|/)\.env($|\.)|\.(pem|key|p12|pfx|jks)$|(^|/)id_(rsa|dsa|ecdsa|ed25519)(\.pub)?$|(^|/)\.(npmrc|netrc|pypirc)$|(^|/)secrets?/`; | ||
| async function checkoutHead(c) { | ||
| const { execFileSync } = await import("node:child_process"); | ||
| const { existsSync, mkdirSync, rmSync, writeFileSync } = await import("node:fs"); | ||
| const { resolve } = await import("node:path"); | ||
| const { createHash } = await import("node:crypto"); | ||
| const { existsSync, mkdirSync, readdirSync, readFileSync, rmSync } = await import("node:fs"); | ||
| const { join, resolve } = await import("node:path"); | ||
| if (!process.env.GH_TOKEN) throw new Error("GH_TOKEN required"); | ||
| const dir = resolve("babysitter-checkout"); | ||
| const auth = Buffer.from(`x-access-token:${process.env.GH_TOKEN}`).toString("base64"); | ||
|
|
@@ -626,21 +627,69 @@ async function checkoutHead(c) { | |
| GIT_CONFIG_VALUE_2: "false" | ||
| }; | ||
| const git = (...args) => String(execFileSync("git", ["-C", dir, ...args], { env, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] })).trim(); | ||
| const reused = existsSync(`${dir}/.git`); | ||
| if (!reused) { | ||
| mkdirSync(dir, { recursive: true }); | ||
| git("init", "-q"); | ||
| } | ||
| writeFileSync(`${dir}/.git/config`, "[core]\n repositoryformatversion = 0\n filemode = true\n bare = false\n"); | ||
| rmSync(`${dir}/.git/hooks`, { recursive: true, force: true }); | ||
| rmSync(`${dir}/.git/info/attributes`, { force: true }); | ||
| git("fetch", "-q", "--no-tags", "--depth=50", `https://github.com/${c.owner}/${c.repo}.git`, c.head); | ||
| git("checkout", "-q", "--force", "--detach", c.head); | ||
| const MANIFESTS = /^(package\.json|package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb?|requirements[\w.-]*\.txt|pyproject\.toml|poetry\.lock|Pipfile\.lock|uv\.lock|Cargo\.lock|go\.sum|Gemfile\.lock|composer\.lock)$/; | ||
| const SKIP = /* @__PURE__ */ new Set([".git", "node_modules", "vendor", "target", "dist", "build", ".venv", "venv"]); | ||
| const fingerprint = () => { | ||
| const hash = createHash("sha256"); | ||
| const walk = (at, rel, depth) => { | ||
| if (depth > 6) return; | ||
| for (const entry2 of readdirSync(at, { withFileTypes: true }).sort((a, b) => a.name.localeCompare(b.name))) { | ||
| if (entry2.isDirectory() && !SKIP.has(entry2.name)) walk(join(at, entry2.name), `${rel}${entry2.name}/`, depth + 1); | ||
| else if (entry2.isFile() && MANIFESTS.test(entry2.name)) hash.update(`${rel}${entry2.name}\0`).update(readFileSync(join(at, entry2.name))).update("\0"); | ||
| } | ||
| }; | ||
| walk(dir, "", 0); | ||
| return hash.digest("hex"); | ||
| }; | ||
| const reused = existsSync(dir); | ||
| mkdirSync(dir, { recursive: true }); | ||
| const before = fingerprint(); | ||
| rmSync(join(dir, ".git"), { recursive: true, force: true }); | ||
| git("init", "-q"); | ||
| git("fetch", "-q", "--no-tags", "--depth=50", c.origin ?? `https://github.com/${c.owner}/${c.repo}.git`, c.head); | ||
| git("reset", "-q", "--hard", c.head); | ||
| git("clean", "-q", "-fd"); | ||
| git("clean", "-q", reused && fingerprint() !== before ? "-fdx" : "-fd"); | ||
| if (git("rev-parse", "HEAD") !== c.head) throw new Error("Checkout is not at the bound head"); | ||
| process.stdout.write(JSON.stringify({ dir, head: c.head, reused })); | ||
| } | ||
| async function restoreCheckout(c) { | ||
| const { existsSync, renameSync, rmSync } = await import("node:fs"); | ||
| const { homedir } = await import("node:os"); | ||
| const { join, resolve } = await import("node:path"); | ||
| const cache = join(c.home ?? homedir(), ".babysitter", c.owner.toLowerCase(), c.repo.toLowerCase(), String(c.number), "checkout"); | ||
| const dir = resolve("babysitter-checkout"); | ||
| let restored = false; | ||
| try { | ||
| if (existsSync(cache) && !existsSync(dir)) { | ||
| renameSync(cache, dir); | ||
| restored = true; | ||
| } | ||
| } catch { | ||
| try { | ||
| rmSync(cache, { recursive: true, force: true }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Concurrent runs for the same PR can delete each other's cached checkout: this removes a checkout another run may have just stashed before renaming its own. Serialize cache moves per PR so one run cannot discard another run's dependency cache. Prompt for AI agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not applicable with cloud C2c (#4356). The per-PR lease admits one run at a time per PR and artifact (a concurrent wake gets a retryable |
||
| } catch { | ||
| } | ||
| } | ||
| process.stdout.write(JSON.stringify({ restored })); | ||
| } | ||
| async function stashCheckout(c) { | ||
| const { existsSync, mkdirSync, renameSync, rmSync } = await import("node:fs"); | ||
| const { homedir } = await import("node:os"); | ||
| const { dirname, join, resolve } = await import("node:path"); | ||
| const cache = join(c.home ?? homedir(), ".babysitter", c.owner.toLowerCase(), c.repo.toLowerCase(), String(c.number), "checkout"); | ||
| const dir = resolve("babysitter-checkout"); | ||
| let stashed = false; | ||
| try { | ||
| if (existsSync(dir)) { | ||
| rmSync(cache, { recursive: true, force: true }); | ||
| mkdirSync(dirname(cache), { recursive: true }); | ||
| renameSync(dir, cache); | ||
| stashed = true; | ||
| } | ||
| } catch { | ||
| } | ||
| process.stdout.write(JSON.stringify({ stashed })); | ||
| } | ||
| async function proposeChanges(c) { | ||
| const { execFileSync } = await import("node:child_process"); | ||
| const { rmSync, writeFileSync } = await import("node:fs"); | ||
|
|
@@ -677,6 +726,12 @@ async function proposeChanges(c) { | |
| if (Buffer.byteLength(out) > c.limits.proposalBytes) return refuse("proposal exceeds the journal output bound"); | ||
| process.stdout.write(out); | ||
| } | ||
| async function restore(f, pr) { | ||
| return JSON.parse(await f.run(nodeCommand(restoreCheckout, { owner: pr.owner, repo: pr.repo, number: pr.number }), { timeout: "2m" })).restored === true; | ||
| } | ||
| async function stash(f, pr) { | ||
| await f.run(nodeCommand(stashCheckout, { owner: pr.owner, repo: pr.repo, number: pr.number }), { timeout: "2m" }); | ||
| } | ||
| async function checkout(f, pr, head) { | ||
| const value = JSON.parse(await f.run(nodeCommand(checkoutHead, { owner: pr.owner, repo: pr.repo, head }), { timeout: "5m" })); | ||
| if (value.head !== head || typeof value.dir !== "string") throw new Error("Checkout did not report the bound head"); | ||
|
|
@@ -730,27 +785,36 @@ function createStandaloneFixer(policy, runtime = capabilities) { | |
| ); | ||
| if (!a) return; | ||
| const { pr, wake, origin, c, head, report } = a; | ||
| const work = await checkout(f, pr, head); | ||
| const task = agentTask(origin, `${pr.owner}/${pr.repo}#${pr.number}`, head, a.changed, "fix"); | ||
| const result = origin.source === "codex" ? await f.agent("babysitter-fix", { cli: "codex", model: "gpt-5.6-sol", cwd: work.dir, permissions: { accessPreset: "readwrite" }, task }) : await f.agent("babysitter-fix", { cli: "claude", model: "claude-sonnet-5", cwd: work.dir, permissions: { accessPreset: "readwrite" }, task }); | ||
| const final = await readState(f, c); | ||
| if (final.headSha !== head || outOfScope(final, c, configured.label)) { | ||
| await report(`${wake.id}: head moved or PR left scope while fixing; no proposal for ${head}`); | ||
| return f.done("declined"); | ||
| } | ||
| const outcome = parseOutcome(result.summary); | ||
| const proposal = await propose(f, { | ||
| dir: work.dir, | ||
| head, | ||
| pullRequest: { owner: pr.owner, repo: pr.repo, number: pr.number }, | ||
| summary: neutralise(outcome.summary, origin.firstPrompt).slice(0, SUMMARY_MAX_CHARS), | ||
| replies: threadReplies(a, outcome.replies) | ||
| }); | ||
| if (proposal.kind === "babysitter-refusal") { | ||
| await report(`${wake.id}: proposal refused: ${proposal.reason}`); | ||
| return f.done("needs_human", { detail: `Babysitter proposal refused: ${proposal.reason}` }); | ||
| await restore(f, pr); | ||
| let verdict; | ||
| try { | ||
| const work = await checkout(f, pr, head); | ||
| const task = agentTask(origin, `${pr.owner}/${pr.repo}#${pr.number}`, head, a.changed, "fix"); | ||
| const result = origin.source === "codex" ? await f.agent("babysitter-fix", { cli: "codex", model: "gpt-5.6-sol", cwd: work.dir, permissions: { accessPreset: "readwrite" }, task }) : await f.agent("babysitter-fix", { cli: "claude", model: "claude-sonnet-5", cwd: work.dir, permissions: { accessPreset: "readwrite" }, task }); | ||
| const final = await readState(f, c); | ||
| if (final.headSha !== head || outOfScope(final, c, configured.label)) { | ||
| await report(`${wake.id}: head moved or PR left scope while fixing; no proposal for ${head}`); | ||
| verdict = { reason: "declined" }; | ||
| } else { | ||
| const outcome = parseOutcome(result.summary); | ||
| const proposal = await propose(f, { | ||
| dir: work.dir, | ||
| head, | ||
| pullRequest: { owner: pr.owner, repo: pr.repo, number: pr.number }, | ||
| summary: neutralise(outcome.summary, origin.firstPrompt).slice(0, SUMMARY_MAX_CHARS), | ||
| replies: threadReplies(a, outcome.replies) | ||
| }); | ||
| if (proposal.kind === "babysitter-refusal") { | ||
| await report(`${wake.id}: proposal refused: ${proposal.reason}`); | ||
| verdict = { reason: "needs_human", detail: `Babysitter proposal refused: ${proposal.reason}` }; | ||
| } else { | ||
| verdict = { reason: "success", detail: `Babysitter proposal for ${head}: ${proposal.files.length} file(s)` }; | ||
| } | ||
| } | ||
| } finally { | ||
| await stash(f, pr); | ||
| } | ||
| f.done("success", { detail: `Babysitter proposal for ${head}: ${proposal.files.length} file(s)` }); | ||
| f.done(verdict.reason, verdict.detail ? { detail: verdict.detail } : void 0); | ||
| }; | ||
| return subscriptions.reduce( | ||
| (handle, subscription) => handle.on(subscription.trigger, body), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # fix.ts/fixer.ts at 01149375; tests from the F3 commit. | ||
| $ node --experimental-strip-types --test tests/fixer.test.ts | ||
| ✔ the fixer checks out the bound head, runs one agent there with fix rules, and journals one proposal (2.870232ms) | ||
| ✔ the fixer never writes to GitHub itself: no push, merge, review, or comment calls (0.630366ms) | ||
| ✔ thread replies go only to inline feedback this run was woken by, once each, marked and neutralised (0.535859ms) | ||
| ✔ a head that moves while fixing declines without a proposal (0.317191ms) | ||
| ✔ a refused proposal ends needs_human (0.397742ms) | ||
| ✔ by default the fixer dispatches no agent and checks nothing out (gate 8 / #442) (0.938991ms) | ||
| ✔ the agent outcome is read from a JSON message, a fenced JSON block, or falls back to prose (0.195023ms) | ||
| ✔ a real proposal covers edits, new files and the agent's own commits, all against the bound head (72.917016ms) | ||
| ✔ workflow, secret, oversized and too-wide proposals are refused whole (328.176771ms) | ||
| ✔ an untouched checkout proposes an empty patch (replies and summary may still publish) (43.603598ms) | ||
| ✔ the proposal script runs as f.run runs it: a stringified `node -e` command in a shell (75.060774ms) | ||
| ✔ the proposal script lifted from the shipped artifact runs under `node -e` (72.127132ms) | ||
| ✔ the proposal never runs code the agent planted in the checkout's git config, and its diff format is fixed (52.673283ms) | ||
| ✔ paths git would have to quote are refused; a newline cannot split a name past the workflow rule (118.653171ms) | ||
| ✔ a non-ASCII name is proposed unquoted, so Cloud parses the same path (42.946322ms) | ||
| ✔ the fixer runs the origin session's own CLI with its literal pinned model (1.037575ms) | ||
| ✖ a PR checkout survives between runs: restored into the run root, stashed back after (0.61106ms) | ||
| ✖ the fixer restores the cached checkout before checking out, and stashes it even when the agent fails (1.443883ms) | ||
| ℹ tests 18 | ||
| ℹ suites 0 | ||
| ℹ pass 16 | ||
| ℹ fail 2 | ||
| ℹ cancelled 0 | ||
| ℹ skipped 0 | ||
| ℹ todo 0 | ||
| ℹ duration_ms 934.900586 | ||
|
|
||
| ✖ failing tests: | ||
|
|
||
| test at tests/fixer.test.ts:267:1 | ||
| ✖ a PR checkout survives between runs: restored into the run root, stashed back after (0.61106ms) | ||
| TypeError: restoreCheckout is not a function | ||
| at file:///home/khaliqgant/Projects/AgentWorkforce/flows-worktrees/babysitter-f2/examples/babysitter/tests/fixer.test.ts:280:40 | ||
| at capture (file:///home/khaliqgant/Projects/AgentWorkforce/flows-worktrees/babysitter-f2/examples/babysitter/tests/fixer.test.ts:276:17) | ||
| at TestContext.<anonymous> (file:///home/khaliqgant/Projects/AgentWorkforce/flows-worktrees/babysitter-f2/examples/babysitter/tests/fixer.test.ts:280:26) | ||
| at async Test.run (node:internal/test_runner/test:1409:7) | ||
| at async Test.processPendingSubtests (node:internal/test_runner/test:974:7) | ||
|
|
||
| test at tests/fixer.test.ts:297:1 | ||
| ✖ the fixer restores the cached checkout before checking out, and stashes it even when the agent fails (1.443883ms) | ||
| AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal: | ||
| + actual - expected | ||
|
|
||
| [ | ||
| - 'restoreCheckout', | ||
| 'checkoutHead', | ||
| 'proposeChanges', | ||
| - 'stashCheckout' | ||
| ] | ||
|
|
||
| at TestContext.<anonymous> (file:///home/khaliqgant/Projects/AgentWorkforce/flows-worktrees/babysitter-f2/examples/babysitter/tests/fixer.test.ts:301:10) | ||
| at async Test.run (node:internal/test_runner/test:1409:7) | ||
| at async Test.processPendingSubtests (node:internal/test_runner/test:974:7) { | ||
| generatedMessage: true, | ||
| code: 'ERR_ASSERTION', | ||
| actual: [ 'checkoutHead', 'proposeChanges' ], | ||
| expected: [ 'restoreCheckout', 'checkoutHead', 'proposeChanges', 'stashCheckout' ], | ||
| operator: 'deepStrictEqual', | ||
| diff: 'simple' | ||
| } | ||
| exit=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The cached tree is agent-writable, but fingerprinting propagates
readdirSyncandreadFileSyncerrors before rebuilding the checkout. An unreadable directory or manifest therefore makes the next fixer run fail instead of starting fresh; treat fingerprint failures as a corrupt cache and fall back to a clean checkout.Prompt for AI agents