From 99a700fb907cec9144ce309c22ce4e0e2971222b Mon Sep 17 00:00:00 2001 From: Toby V Date: Sat, 27 Jun 2026 15:16:35 +1000 Subject: [PATCH] =?UTF-8?q?feat(ENG-017):=20local-git=20DeliveryAdapter=20?= =?UTF-8?q?=E2=80=94=20commit=20the=20agent's=20code=20without=20GitHub?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds LocalGitDeliveryAdapter behind the DeliveryAdapter port: commits the coding agent's files to a local git repo (DELIVERY_BACKEND=local) in the working-code workspace, with an in-process serialization queue for concurrent tickets. verifyTicket now feeds the committed files to the QA agent so it can verify real code (api backend is tool-less), addressing the doomed-rework cost. Real-git unit tests; typecheck + lint + 98 tests green. Live run also needs fix/worker-temporal-connection. Co-Authored-By: Claude Opus 4.8 (1M context) --- .env.example | 5 + apps/server/src/temporal/activities.ts | 42 +++- docker-compose.yml | 6 + packages/integrations/src/index.ts | 5 + .../integrations/src/local/delivery.test.ts | 66 +++++++ packages/integrations/src/local/delivery.ts | 179 ++++++++++++++++++ project/ENG-017-local-git-delivery.md | 54 ++++++ project/README.md | 6 + 8 files changed, 357 insertions(+), 6 deletions(-) create mode 100644 packages/integrations/src/local/delivery.test.ts create mode 100644 packages/integrations/src/local/delivery.ts create mode 100644 project/ENG-017-local-git-delivery.md diff --git a/.env.example b/.env.example index 58520e2..3878cd3 100644 --- a/.env.example +++ b/.env.example @@ -10,6 +10,11 @@ TEMPORAL_TASK_QUEUE=engineering ANTHROPIC_API_KEY=sk-ant-... # WORKER_MODE=api # or "cli" +# Delivery backend (ENG-017): "local" commits the agent's code to a local git repo under ./workspace +# (no GitHub needed — QA verifies real code, tickets can reach `done` offline). Leave unset to use the +# GitHub adapter when the GITHUB_* vars below are set, or to no-op delivery when they aren't. +DELIVERY_BACKEND= + # GitHub delivery (optional). When set, the worker opens/merges PRs for tickets. GITHUB_TOKEN= GITHUB_OWNER= diff --git a/apps/server/src/temporal/activities.ts b/apps/server/src/temporal/activities.ts index 2e5a422..b47af8b 100644 --- a/apps/server/src/temporal/activities.ts +++ b/apps/server/src/temporal/activities.ts @@ -5,6 +5,7 @@ import { estimateRunCostCents, proposeFileChanges, proposeTickets, + repoWorkspacePath, } from "@eng/agents" import { type DeliveryAdapter, @@ -14,7 +15,11 @@ import { type TicketStatus, } from "@eng/core" import { createApproval, getBudgetRemaining, reconcileSpend, reserveBudget } from "@eng/db" -import { createGitHubDelivery } from "@eng/integrations" +import { + createGitHubDelivery, + LocalGitDeliveryAdapter, + localGitBranchFiles, +} from "@eng/integrations" import { persistenceFromEnv } from "../persistence" import { artifactPath, SHAPING_STAGES } from "../shaping" import { startTicketLifecycle } from "./client" @@ -224,15 +229,24 @@ export async function pickUpBacklog(): Promise { // --- Delivery (GitHub) ------------------------------------------------------- +const BASE_BRANCH = process.env.GITHUB_BASE_BRANCH ?? "main" + +/** The local working-code repo path for DELIVERY_BACKEND=local (ENG-017). */ +function localRepoDir(): string { + return repoWorkspacePath(process.env.GITHUB_OWNER || "local", process.env.GITHUB_REPO || "app") +} + function deliveryFromEnv(): DeliveryAdapter | null { + // Local-git delivery (ENG-017): commit the agent's code to a local repo — no GitHub needed. + if (process.env.DELIVERY_BACKEND === "local") { + return new LocalGitDeliveryAdapter({ repoDir: localRepoDir(), baseBranch: BASE_BRANCH }) + } const token = process.env.GITHUB_TOKEN const owner = process.env.GITHUB_OWNER const repo = process.env.GITHUB_REPO if (!token || !owner || !repo) return null return createGitHubDelivery({ token, owner, repo, baseBranch: process.env.GITHUB_BASE_BRANCH }) } - -const BASE_BRANCH = process.env.GITHUB_BASE_BRANCH ?? "main" const DEPLOY_WORKFLOW = process.env.GITHUB_DEPLOY_WORKFLOW const DEPLOY_REF = process.env.GITHUB_DEPLOY_REF ?? BASE_BRANCH @@ -436,14 +450,30 @@ export async function verifyTicket( persistence.tracker.get(ticketId), persistence.hierarchy.traceContext(ticketId), ]) + + // ENG-017: when delivery commits to a local repo, hand the QA agent the actual committed files so it + // can verify real code (the api backend is tool-less and otherwise can't see the changes). This also + // distinguishes "nothing was committed" from "committed but failed verification". + let evidence = "" + if (process.env.DELIVERY_BACKEND === "local") { + const files = await localGitBranchFiles(localRepoDir(), `ticket/${ticketId}`) + evidence = files.length + ? `\n\nThe following changes were committed for this ticket — verify them against the criteria:\n${files + .map((f) => `\n----- ${f.path} -----\n${f.content}`) + .join("\n")}` + : "\n\nNote: no code was committed for this ticket; there is nothing to verify." + } + try { const verdict = await assess({ role: role.id, systemPrompt: role.systemPrompt, goalContext, - task: ticket - ? `Verify ticket "${ticket.title}" meets its acceptance criteria: ${ticket.acceptanceCriteria.join("; ") || "(none specified)"}.` - : `Verify ticket ${ticketId}.`, + task: `${ + ticket + ? `Verify ticket "${ticket.title}" meets its acceptance criteria: ${ticket.acceptanceCriteria.join("; ") || "(none specified)"}.` + : `Verify ticket ${ticketId}.` + }${evidence}`, budgetCentsRemaining: remaining, }) await reconcileSpend(role.id, held, verdict.costCents) diff --git a/docker-compose.yml b/docker-compose.yml index 910aa24..0cc5f97 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -103,7 +103,13 @@ services: GITHUB_DEPLOY_REF: ${GITHUB_DEPLOY_REF:-main} PERSISTENCE_BACKEND: ${PERSISTENCE_BACKEND:-postgres} GITHUB_DOCS_PREFIX: ${GITHUB_DOCS_PREFIX:-docs} + # Delivery backend (ENG-017): set DELIVERY_BACKEND=local to commit the agent's code to a local + # git repo under ./workspace (no GitHub needed); unset/`github` uses the GitHub adapter. + DELIVERY_BACKEND: ${DELIVERY_BACKEND:-} command: ["pnpm", "--filter", "@eng/server", "worker"] + # Surface the working-code workspace (local-git commits) on the host so you can inspect the repo. + volumes: + - ./workspace:/app/workspace web: build: diff --git a/packages/integrations/src/index.ts b/packages/integrations/src/index.ts index d22d4c0..a647483 100644 --- a/packages/integrations/src/index.ts +++ b/packages/integrations/src/index.ts @@ -10,3 +10,8 @@ export { GitHubDeliveryAdapter } from "./github/delivery" export { GitHubHierarchy } from "./github/hierarchy" export { GitHubIssueTracker } from "./github/issues" export { GitHubKnowledgeBase } from "./github/knowledge" +export { + LocalGitDeliveryAdapter, + type LocalGitDeliveryConfig, + localGitBranchFiles, +} from "./local/delivery" diff --git a/packages/integrations/src/local/delivery.test.ts b/packages/integrations/src/local/delivery.test.ts new file mode 100644 index 0000000..94429fb --- /dev/null +++ b/packages/integrations/src/local/delivery.test.ts @@ -0,0 +1,66 @@ +import { mkdtemp, rm } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { afterEach, beforeEach, describe, expect, it } from "vitest" +import { LocalGitDeliveryAdapter, localGitBranchFiles } from "./delivery" + +describe("LocalGitDeliveryAdapter", () => { + let dir: string + let adapter: LocalGitDeliveryAdapter + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "eng-localgit-")) + adapter = new LocalGitDeliveryAdapter({ repoDir: dir, baseBranch: "main" }) + }) + afterEach(async () => { + await rm(dir, { recursive: true, force: true }) + }) + + it("commits the agent's files to a local branch, then merges to main", async () => { + await adapter.createBranch("main", "ticket/1") + const { sha } = await adapter.commitFiles("ticket/1", "feat: add server", [ + { path: "src/index.ts", content: "export const x = 1\n" }, + { path: "README.md", content: "# app\n" }, + ]) + expect(sha).toMatch(/^[0-9a-f]{7,}$/) + + const files = await localGitBranchFiles(dir, "ticket/1") + expect(files.map((f) => f.path).sort()).toEqual(["README.md", "src/index.ts"]) + expect(files.find((f) => f.path === "src/index.ts")?.content).toContain("export const x = 1") + + const pr = await adapter.openPullRequest({ branch: "ticket/1", title: "t", body: "b" }) + await adapter.merge(pr) + const onMain = await localGitBranchFiles(dir, "main") + expect(onMain.map((f) => f.path).sort()).toEqual(["README.md", "src/index.ts"]) + }) + + it("serializes concurrent ticket commits without cross-contamination", async () => { + await Promise.all([ + (async () => { + await adapter.createBranch("main", "ticket/A") + await adapter.commitFiles("ticket/A", "A", [{ path: "a.txt", content: "A" }]) + })(), + (async () => { + await adapter.createBranch("main", "ticket/B") + await adapter.commitFiles("ticket/B", "B", [{ path: "b.txt", content: "B" }]) + })(), + ]) + const a = await localGitBranchFiles(dir, "ticket/A") + const b = await localGitBranchFiles(dir, "ticket/B") + expect(a.find((f) => f.path === "a.txt")?.content).toBe("A") + expect(a.find((f) => f.path === "b.txt")).toBeUndefined() + expect(b.find((f) => f.path === "b.txt")?.content).toBe("B") + expect(b.find((f) => f.path === "a.txt")).toBeUndefined() + }) + + it("rejects writes that escape the repo", async () => { + await adapter.createBranch("main", "ticket/x") + await expect( + adapter.commitFiles("ticket/x", "evil", [{ path: "../escape.txt", content: "no" }]), + ).rejects.toThrow(/outside the repo/) + }) + + it("returns [] for a missing repo/branch", async () => { + expect(await localGitBranchFiles(join(dir, "nope"), "main")).toEqual([]) + expect(await localGitBranchFiles(dir, "no-such-branch")).toEqual([]) + }) +}) diff --git a/packages/integrations/src/local/delivery.ts b/packages/integrations/src/local/delivery.ts new file mode 100644 index 0000000..263a755 --- /dev/null +++ b/packages/integrations/src/local/delivery.ts @@ -0,0 +1,179 @@ +import { spawn } from "node:child_process" +import { existsSync } from "node:fs" +import { mkdir, writeFile } from "node:fs/promises" +import { dirname, join, resolve as pathResolve } from "node:path" +import type { + CheckStatus, + DeliveryAdapter, + DeploymentRun, + FileChange, + OpenPullRequestArgs, + PullRequestRef, +} from "@eng/core" + +// Commit identity supplied per-commit so a freshly `git init`-ed repo doesn't need global git config. +const IDENT = ["-c", "user.email=agent@engineering.local", "-c", "user.name=engineering agent"] + +function git(cwd: string, args: string[]): Promise { + return new Promise((resolve, reject) => { + const child = spawn("git", args, { cwd }) + let out = "" + let err = "" + child.stdout.on("data", (d) => (out += d)) + child.stderr.on("data", (d) => (err += d)) + child.on("error", reject) + child.on("close", (code) => + code === 0 + ? resolve(out.trim()) + : reject(new Error(`git ${args.join(" ")} → ${code}: ${err.trim()}`)), + ) + }) +} + +export interface LocalGitDeliveryConfig { + repoDir: string + baseBranch?: string +} + +/** + * A `DeliveryAdapter` that commits to a **local** git repo (ENG-017) — no GitHub account or remote + * required. The coding agent's proposed file changes become real, inspectable commits in the + * working-code workspace, so QA can verify actual code and a ticket can reach `done` entirely offline. + * + * Concurrency: the worker is a single process, so all git operations are serialized through an + * in-process queue and each `commitFiles`/`merge` is a self-contained `checkout -f → write → commit` + * unit — concurrent ticket lifecycles can't race on the shared working tree. A "PR" is a local + * branch; "merge" is a local `--no-ff` merge into the base branch; deploy hooks are no-ops (deploy + * stays unconfigured locally). + */ +export class LocalGitDeliveryAdapter implements DeliveryAdapter { + private readonly repoDir: string + private readonly baseBranch: string + private queue: Promise = Promise.resolve() + + constructor(cfg: LocalGitDeliveryConfig) { + this.repoDir = pathResolve(cfg.repoDir) + this.baseBranch = cfg.baseBranch || "main" + } + + private lock(fn: () => Promise): Promise { + const run = this.queue.then(fn, fn) + this.queue = run.then( + () => undefined, + () => undefined, + ) + return run + } + + private async ensureRepo(): Promise { + if (existsSync(join(this.repoDir, ".git"))) return + await mkdir(this.repoDir, { recursive: true }) + await git(this.repoDir, ["-c", `init.defaultBranch=${this.baseBranch}`, "init"]) + await git(this.repoDir, [ + ...IDENT, + "commit", + "--allow-empty", + "-m", + "chore: initialize repository", + ]) + } + + async createBranch(base: string, name: string): Promise { + await this.lock(async () => { + await this.ensureRepo() + await git(this.repoDir, ["branch", "-f", name, base || this.baseBranch]) + }) + } + + async commitFiles( + branch: string, + message: string, + files: FileChange[], + ): Promise<{ sha: string }> { + return this.lock(async () => { + await this.ensureRepo() + await git(this.repoDir, ["checkout", "-f", branch]) + for (const f of files) { + const abs = pathResolve(this.repoDir, f.path) + if (abs !== this.repoDir && !abs.startsWith(`${this.repoDir}/`)) { + throw new Error(`refusing to write outside the repo: ${f.path}`) + } + await mkdir(dirname(abs), { recursive: true }) + await writeFile(abs, f.content) + } + await git(this.repoDir, ["add", "-A"]) + await git(this.repoDir, [...IDENT, "commit", "-m", message]) + return { sha: await git(this.repoDir, ["rev-parse", "HEAD"]) } + }) + } + + async openPullRequest(args: OpenPullRequestArgs): Promise { + return { number: 0, url: `file://${this.repoDir}#${args.branch}`, branch: args.branch } + } + + async getChecks(): Promise { + return [] // no CI locally → aggregates to success + } + + async merge(pr: PullRequestRef): Promise { + await this.lock(async () => { + await git(this.repoDir, ["checkout", "-f", this.baseBranch]) + await git(this.repoDir, [...IDENT, "merge", "--no-ff", pr.branch, "-m", `merge ${pr.branch}`]) + }) + } + + // Deploy is a no-op locally (the deploy workflow stays unconfigured); present to satisfy the port. + dispatchWorkflow(): Promise { + return Promise.resolve() + } + latestRunId(): Promise { + return Promise.resolve(null) + } + deploymentRunAfter(): Promise { + return Promise.resolve(null) + } +} + +/** + * Read the files committed on `branch` of a local repo, for QA verification (ENG-017). Reads via git + * (not the working tree) so it's safe under concurrent ticket runs, and is bounded so a large tree + * can't blow up the prompt. Returns `[]` when the repo or branch doesn't exist yet. + */ +export async function localGitBranchFiles( + repoDir: string, + branch: string, + opts: { maxFiles?: number; maxBytes?: number } = {}, +): Promise { + const dir = pathResolve(repoDir) + if (!existsSync(join(dir, ".git"))) return [] + const maxFiles = opts.maxFiles ?? 40 + const maxBytes = opts.maxBytes ?? 60_000 + let list: string + try { + list = await git(dir, ["ls-tree", "-r", "--name-only", branch]) + } catch { + return [] // branch doesn't exist + } + const paths = list + .split("\n") + .map((p) => p.trim()) + .filter(Boolean) + .slice(0, maxFiles) + const files: FileChange[] = [] + let bytes = 0 + for (const p of paths) { + let content: string + try { + content = await git(dir, ["show", `${branch}:${p}`]) + } catch { + continue + } + bytes += content.length + if (bytes > maxBytes) { + files.push({ path: p, content: "(omitted — size budget reached)" }) + break + } + files.push({ path: p, content }) + } + return files +} diff --git a/project/ENG-017-local-git-delivery.md b/project/ENG-017-local-git-delivery.md new file mode 100644 index 0000000..51d1420 --- /dev/null +++ b/project/ENG-017-local-git-delivery.md @@ -0,0 +1,54 @@ +# ENG-017 — Local-git DeliveryAdapter (commit without GitHub) + +- **Status:** done +- **Priority:** P1 (unblocks a real end-to-end run with no GitHub account) +- **Stage:** implementation +- **Assignee role:** staff_engineer (with lead_system_design input) +- **Area:** packages/integrations · apps/server + +## Problem + +A live run without GitHub couldn't produce real code: delivery is GitHub-API-only (the sole +`DeliveryAdapter` is `GitHubDeliveryAdapter`, whose `commitFiles` uses the GitHub Git Data API). With +no `GITHUB_*` configured, `deliveryFromEnv()` returns `null`, the coding agent's proposed `files[]` are +**discarded**, and the QA agent (tool-less `api` backend) has nothing to verify — so every ticket +reworks to the cap and `blocked`. (Confirmed in a live run: 6/6 tickets blocked, ~$2 spent on doomed +rework.) + +## Outcome (built) + +A **`LocalGitDeliveryAdapter`** behind the existing `DeliveryAdapter` port — no GitHub account/remote +needed, so a ticket can reach `done` entirely offline with real, inspectable code on disk. + +- **`packages/integrations/src/local/delivery.ts`** — implements the full port with local `git`: + `createBranch` (`git branch -f`), `commitFiles` (`checkout -f` → write files → `add -A` → `commit`, + returns the SHA), `openPullRequest` (synthetic `file://` ref), `getChecks` (`[]` → success), `merge` + (local `--no-ff` into the base branch); deploy hooks are no-ops. Commit identity is supplied + per-commit so a fresh `git init` needs no global config; writes are path-traversal-guarded. +- **Concurrency-safe:** all git ops run through an in-process queue (the worker is one process) and each + `commitFiles`/`merge` is a self-contained `checkout -f → … → commit` unit, so the heartbeat + auto-starting all backlog tickets can't corrupt the shared repo. +- **`localGitBranchFiles(repoDir, branch)`** — reads a branch's committed files via git (not the + working tree, so it's concurrency-safe), bounded by file count/size. +- **Wiring (`activities.ts`):** `deliveryFromEnv()` returns the local adapter when + `DELIVERY_BACKEND=local` (repo at `workspace//`); `implementTicket` needs no + change — the port abstraction means it commits to the local repo. **QA now verifies real code:** + `verifyTicket` injects the committed files into the assess prompt (and says "nothing to verify" when + none) so the tool-less `api` backend can actually check the work — addressing the doomed-rework cost. +- **Ops:** `DELIVERY_BACKEND` passthrough + a `./workspace` bind-mount in compose (so the committed + repo is visible on the host); `.env.example` documents it. + +## Verification + +- Real-git unit tests (`delivery.test.ts`): commit→branch→merge, concurrent commits don't + cross-contaminate, path-traversal rejected, missing repo/branch → `[]`. +- typecheck · lint · full test suite green. + +## Deferred / notes + +- **Live run needs the worker fix too** (separate PR, `fix/worker-temporal-connection`) — without it + the worker can't reach Temporal in Docker. +- `getChecks` returns success (no local CI). A natural follow-up: run the target repo's tests as the + "CI" check before the merge gate. +- The `cli` backend would make this even stronger (it edits files in the repo cwd directly); this + ticket makes the `api` backend path work end-to-end. diff --git a/project/README.md b/project/README.md index f67f99d..ec569ec 100644 --- a/project/README.md +++ b/project/README.md @@ -25,9 +25,15 @@ mirror the `TicketStatus`, `LifecycleStage`, and `RoleId` enums in `packages/cor | [ENG-014](ENG-014-decision-log-and-provenance-tree.md) | Per-agent decision logs & a traceable decision tree | P2 | core · db · server | | [ENG-015](ENG-015-evaluate-surrealdb-backend.md) | Evaluate SurrealDB as a persistence backend (decision spike) | P2 | core · db · infra | | [ENG-016](ENG-016-implement-kappa-consensus.md) | Implement Kappa-style consensus (PRD-001) | P2 | core · agents · server | +| [ENG-017](ENG-017-local-git-delivery.md) | Local-git DeliveryAdapter (commit without GitHub) | P1 | integrations · server | **Priority key:** P0 = do before any live autonomous run · P1 = high · P2 = medium · P3 = low. +> ENG-017 and the `fix/worker-temporal-connection` worker fix were added after a first live run +> surfaced them (the worker couldn't reach Temporal in Docker; and a no-GitHub run produced no +> verifiable code). Status: Waves 0–3 done (14 tickets); **ENG-014 and ENG-016 (Wave 4) remain**; +> ENG-017 done. + ## Product requirements (PRDs) | Doc | Summary |