diff --git a/infra/emdash-bot/.flue/agents/investigate.ts b/infra/emdash-bot/.flue/agents/investigate.ts index 8ca61560bc..c4c0039fbb 100644 --- a/infra/emdash-bot/.flue/agents/investigate.ts +++ b/infra/emdash-bot/.flue/agents/investigate.ts @@ -24,18 +24,43 @@ import { getCloudflareContext } from "@flue/runtime/cloudflare"; import { env as workerEnv } from "cloudflare:workers"; import * as v from "valibot"; -import { type ContainerBackend, ExecEnv, fromSandbox, quote } from "../lib/exec-env.js"; +import { + publishCandidate, + requireCandidatePublication, + type CandidateGitHub, + type CandidatePublication, + type CandidateSnapshot, +} from "../lib/candidate-publisher.js"; +import { + type ContainerBackend, + ExecEnv, + type ExecResult, + fromSandbox, + quote, +} from "../lib/exec-env.js"; import { createPushCapability, PUSH_CAPABILITY_HEADER } from "../lib/github-proxy.js"; import { + createBranch, + createGitBlob, + createGitCommit, + createGitTree, getBranchSha, + getGitCommit, mintInstallationToken, readAppCreds, readRepoContext, + updateBranch, } from "../lib/github.js"; import { applyInvestigationResult } from "../lib/investigation-result.js"; import { untarInto } from "../lib/untar.js"; +import { + assertVerificationCommand, + passingVerificationRecords, + type VerificationRecord, +} from "../lib/verification.js"; import diagnoseSkill from "../skills/diagnose/SKILL.md"; import fixSkill from "../skills/fix/SKILL.md"; +import implementSkill from "../skills/implement/SKILL.md"; import investigateSkill from "../skills/investigate/SKILL.md"; import reproAdminSkill from "../skills/repro-admin/SKILL.md"; import reproApiSkill from "../skills/repro-api/SKILL.md"; @@ -53,6 +78,7 @@ const DEADLINES = { defaultTimeoutMs: DEFAULT_RPC_TIMEOUT_MS, execGraceMs: EXEC_ * exceeds the model's context window and the run dies mid-flight. */ const TOOL_RESULT_LIMIT = 49_152; +const RESULT_SUMMARY_LIMIT = 2_000; function truncateToolResult(text: string): string { if (text.length <= TOOL_RESULT_LIMIT) return text; @@ -102,7 +128,8 @@ const resultSchema = v.pipe( rootCauseFound: v.optional(v.boolean(), false), fixed: v.optional(v.boolean()), verdict: v.optional(v.picklist(["bug", "intended-behavior", "unclear"])), - summary: v.pipe(v.string(), v.minLength(10), v.maxLength(400)), + summary: v.pipe(v.string(), v.minLength(10), v.maxLength(RESULT_SUMMARY_LIMIT)), + failureStage: v.optional(v.picklist(["workspace", "verification", "publication", "reporting"])), /** Reproduction screenshots pushed to bot/artifacts-, rendered in the ask comment. */ screenshots: v.optional(v.array(screenshotSchema)), }), @@ -114,34 +141,75 @@ const resultSchema = v.pipe( ), ); +const implementationResultSchema = v.object({ + skipped: v.optional(v.boolean()), + implemented: v.boolean(), + summary: v.pipe(v.string(), v.minLength(10), v.maxLength(RESULT_SUMMARY_LIMIT)), + failureStage: v.optional(v.picklist(["workspace", "verification", "publication", "reporting"])), + screenshots: v.optional(v.array(screenshotSchema)), +}); + +const publicationSchema = v.object({ + branch: v.string(), + commitSha: v.string(), + files: v.array(v.string()), +}); + +const verificationRecordSchema = v.object({ + name: v.string(), + command: v.string(), + exitCode: v.number(), + candidateTreeSha: v.string(), +}); + const reportedResultSchema = v.object({ - result: resultSchema, + result: v.union([resultSchema, implementationResultSchema]), ok: v.boolean(), pushed: v.boolean(), + runId: v.string(), + publication: v.nullable(publicationSchema), + verification: v.array(verificationRecordSchema), }); type InvestigateData = v.InferOutput; type InvestigationResult = v.InferOutput; +type ImplementationResult = v.InferOutput; + +interface RunFailure { + stage: "workspace" | "verification" | "publication" | "reporting"; + message: string; +} export function Investigate({ id }: AgentProps) { const input = useInitialData(); const [setupComplete, setSetupComplete] = usePersistentState("setup-complete", false); const [reported, setReported] = usePersistentState("reported", false); const [reminded, setReminded] = usePersistentState("report-reminded", false); + const [publication, setPublication] = usePersistentState( + "publication", + null, + ); + const [verification, setVerification] = usePersistentState( + "verification", + [], + ); + const [lastFailure, setLastFailure] = usePersistentState("last-failure", null); const writeResult = useDataWriter("investigation", { schema: reportedResultSchema }); const env = execEnvFor(id, input); useModel("cloudflare/@cf/moonshotai/kimi-k2.7-code"); - useSkill(investigateSkill); - useSkill(diagnoseSkill); - useSkill(verifySkill); - useSkill(reproApiSkill); - useSkill(reproAdminSkill); - useSkill(reproPublicSkill); - // Every mode but diagnose may end in a fix attempt (`repro` is - // "reproduce and attempt a fix" in machine.ts). - if (input.mode !== "diagnose") { + if (input.mode === "implement") { + useSkill(implementSkill); + } else { + useSkill(investigateSkill); + useSkill(diagnoseSkill); + useSkill(verifySkill); + useSkill(reproApiSkill); + useSkill(reproAdminSkill); + useSkill(reproPublicSkill); + } + if (input.mode !== "diagnose" && input.mode !== "implement") { useSkill(fixSkill); } @@ -151,11 +219,20 @@ export function Investigate({ id }: AgentProps) { await env.ensureRepo({ dir: REPO_DIR, ref: cloneRef(input) }); setSetupComplete(true); } catch (error) { + setLastFailure({ stage: "workspace", message: safeFailureMessage(error) }); const result = failedResult( `I couldn't prepare the investigation workspace: ${errorMessage(error)}`, + "workspace", ); await applyInvestigationResult(input, result, false, false); - writeResult({ result, ok: false, pushed: false }); + writeResult({ + result, + ok: false, + pushed: false, + runId: input.runId, + publication: null, + verification: [], + }); setReported(true); log.error("workspace setup failed", { error: errorMessage(error) }); } @@ -290,54 +367,212 @@ export function Investigate({ id }: AgentProps) { }), ); - useTool( - defineTool({ - name: "report_result", - description: - "Report the final structured investigation result to the issue orchestrator. reproduced=true means you demonstrated the defect the reporter described, in this checkout. The demonstration does NOT need to copy their exact steps: a failing unit test that exercises the same defect a UI report describes is a full reproduction of the issue -- report it as one, without hedging. It must be the same defect, though: an adjacent or latent bug you demonstrated, an out-of-repo infrastructure symptom, or a root cause from reading code alone is not a reproduction. Three distinct non-reproduced outcomes -- pick the honest one: rootCauseFound=true when you identified the reporter's defect but could not confirm it with a demonstration (environment limits, browser-only path) -- this is a first-class 'diagnosed' verdict; plain reproduced=false when you investigated and found nothing wrong or a different/adjacent issue (describe findings in summary); verdict='unclear' when the issue lacks the information an attempt would need -- say what is missing. Fill demonstration and demonstratedReportedIssue truthfully. If demonstration attempts are not converging after a couple of angles, stop and report the diagnosis with rootCauseFound rather than grinding.", - input: resultSchema, - output: reportedResultSchema, - durable: true, - async run({ data, step, log }) { - const pushed = await step.do("detect-push", () => - detectPush(input.issueNumber, input.previousBranchSha), - ); - await step.do("apply-agent-result", () => - applyInvestigationResult(input, data, true, pushed), - ); - const reportedResult = { result: data, ok: true, pushed }; - writeResult(reportedResult); - setReported(true); - log.info("investigation reported", { - runId: input.runId, - issueNumber: input.issueNumber, - pushed, - }); - return { output: reportedResult }; - }, - }), - ); + if (input.mode !== "diagnose") { + useTool( + defineTool({ + name: "run_check", + description: + "Run a required read-only verification command and bind its real exit status to the exact candidate tree. The command must not modify source files; use edit_file/write_file for changes and check-only formatter commands. Do not add output pipelines or success fallbacks; the tool rejects them. Reuse a stable name such as test, lint, typecheck, or format when rerunning a check after a fix. Rerun every required check after any source change.", + input: v.object({ + name: v.pipe(v.string(), v.minLength(1), v.maxLength(40)), + command: v.pipe(v.string(), v.minLength(1), v.maxLength(1_000)), + cwd: v.optional(v.string()), + timeoutMs: v.optional(v.number()), + }), + async run({ data }) { + let result: ExecResult; + let candidateTreeSha: string; + try { + assertVerificationCommand(data.command); + ({ result, candidateTreeSha } = await env.runCheck(data.command, { + ...(data.cwd ? { cwd: data.cwd } : {}), + ...(data.timeoutMs ? { timeoutMs: data.timeoutMs } : {}), + })); + } catch (error) { + setLastFailure({ stage: "verification", message: safeFailureMessage(error) }); + throw error; + } + const record = { + name: data.name, + command: data.command, + exitCode: result.exitCode, + candidateTreeSha, + } satisfies VerificationRecord; + setVerification((current) => [...current, record]); + if (result.exitCode !== 0) { + setLastFailure({ + stage: "verification", + message: `${data.name} failed with exit ${result.exitCode}`, + }); + } + return truncateToolResult( + [`exit ${result.exitCode}`, result.stdout, result.stderr].filter(Boolean).join("\n"), + ); + }, + }), + ); - useAgentFinish(async ({ response, append, log }) => { - const reportCall = response.toolCalls.some( - (call) => call.tool === "report_result" && !call.isError, + useTool( + defineTool({ + name: "publish_candidate", + description: + "Publish the verified working tree to this issue's candidate branch. The trusted Worker snapshots the changes, creates the Git objects, updates only bot/fix-, and verifies the remote SHA. Do not run git commit or git push yourself.", + input: v.object({ + commitMessage: v.pipe(v.string(), v.minLength(5), v.maxLength(200)), + }), + output: publicationSchema, + durable: true, + async run({ data, step }) { + try { + passingVerificationRecords(verification); + } catch (error) { + setLastFailure({ stage: "verification", message: safeFailureMessage(error) }); + throw error; + } + let snapshot: CandidateSnapshot; + try { + snapshot = await env.snapshotCandidate(); + } catch (error) { + setLastFailure({ stage: "publication", message: safeFailureMessage(error) }); + throw error; + } + try { + passingVerificationRecords(verification, snapshot.treeSha); + } catch (error) { + setLastFailure({ stage: "verification", message: safeFailureMessage(error) }); + throw error; + } + try { + const published = await step.do("publish-candidate", () => + publishCandidateForRun(input, data.commitMessage, snapshot), + ); + setPublication(published); + setLastFailure(null); + return { output: published }; + } catch (error) { + setLastFailure({ stage: "publication", message: safeFailureMessage(error) }); + throw error; + } + }, + }), + ); + } + + if (input.mode === "implement") { + useTool( + defineTool({ + name: "report_implementation", + description: + "Report the implementation outcome. Set implemented=true only after publish_candidate succeeds. The Worker attaches authoritative verification and publication details.", + input: implementationResultSchema, + output: reportedResultSchema, + durable: true, + async run({ data, step, log }) { + requireCandidatePublication(data.implemented, publication); + const pushed = await step.do("verify-publication", () => + detectPublication(input.issueNumber, publication), + ); + const failure = + data.implemented && !pushed + ? (lastFailure ?? { + stage: "publication" as const, + message: "The candidate branch could not be verified at its published commit.", + }) + : lastFailure; + const result = withRunFailure(data, failure); + await step.do("apply-agent-result", () => + applyInvestigationResult(input, result, true, pushed), + ); + const reportedResult = reportPayload( + input.runId, + result, + pushed, + publication, + verification, + ); + writeResult(reportedResult); + setReported(true); + log.info("implementation reported", { + runId: input.runId, + issueNumber: input.issueNumber, + pushed, + }); + return { output: reportedResult }; + }, + }), ); + } else { + useTool( + defineTool({ + name: "report_result", + description: + "Report the final structured investigation result to the issue orchestrator. reproduced=true means you demonstrated the defect the reporter described, in this checkout. The demonstration does NOT need to copy their exact steps: a failing unit test that exercises the same defect a UI report describes is a full reproduction of the issue -- report it as one, without hedging. It must be the same defect, though: an adjacent or latent bug you demonstrated, an out-of-repo infrastructure symptom, or a root cause from reading code alone is not a reproduction. Three distinct non-reproduced outcomes -- pick the honest one: rootCauseFound=true when you identified the reporter's defect but could not confirm it with a demonstration (environment limits, browser-only path) -- this is a first-class 'diagnosed' verdict; plain reproduced=false when you investigated and found nothing wrong or a different/adjacent issue (describe findings in summary); verdict='unclear' when the issue lacks the information an attempt would need -- say what is missing. Fill demonstration and demonstratedReportedIssue truthfully. If demonstration attempts are not converging after a couple of angles, stop and report the diagnosis with rootCauseFound rather than grinding.", + input: resultSchema, + output: reportedResultSchema, + durable: true, + async run({ data, step, log }) { + requireCandidatePublication(data.fixed === true, publication); + const pushed = await step.do("verify-publication", () => + detectPublication(input.issueNumber, publication), + ); + const failure = + data.fixed && !pushed + ? (lastFailure ?? { + stage: "publication" as const, + message: "The candidate branch could not be verified at its published commit.", + }) + : lastFailure; + const result = withRunFailure(data, failure); + await step.do("apply-agent-result", () => + applyInvestigationResult(input, result, true, pushed), + ); + const reportedResult = reportPayload( + input.runId, + result, + pushed, + publication, + verification, + ); + writeResult(reportedResult); + setReported(true); + log.info("investigation reported", { + runId: input.runId, + issueNumber: input.issueNumber, + pushed, + }); + return { output: reportedResult }; + }, + }), + ); + } + + useAgentFinish(async ({ response, append, log }) => { + const reportTool = input.mode === "implement" ? "report_implementation" : "report_result"; + const reportCall = response.toolCalls.some((call) => call.tool === reportTool && !call.isError); if (reported || reportCall) return; if (!reminded) { setReminded(true); append({ kind: "signal", type: "investigation.report-required", - body: "You have not reported the result. Call report_result now with your final findings. Do not do more investigation.", + body: `You have not reported the result. Call ${reportTool} now with your final findings. Do not do more investigation.`, }); return; } const result = failedResult( "I couldn't complete this run because the agent stopped without reporting a result.", + "reporting", ); await applyInvestigationResult(input, result, false, false); - writeResult({ result, ok: false, pushed: false }); + writeResult({ + result, + ok: false, + pushed: false, + runId: input.runId, + publication, + verification, + }); setReported(true); log.warn("agent stopped without reporting", { runId: input.runId }); }); @@ -591,57 +826,147 @@ function cloneRef(input: InvestigateData): string { return input.mode === "revise" ? `bot/fix-${input.issueNumber}` : "main"; } -function failedResult(summary: string): InvestigationResult { +function failedResult(summary: string, failureStage?: RunFailure["stage"]): InvestigationResult { return { summary: truncateSummary(summary), fixed: false, reproduced: false, + demonstration: "none", + demonstratedReportedIssue: false, + rootCauseFound: false, verdict: "unclear", + ...(failureStage ? { failureStage } : {}), }; } function truncateSummary(text: string): string { - return text.length <= 400 ? text : `${text.slice(0, 399)}…`; + return text.length <= RESULT_SUMMARY_LIMIT ? text : `${text.slice(0, RESULT_SUMMARY_LIMIT - 1)}…`; } function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } -async function detectPush(issueNumber: number, previousBranchSha: string | null): Promise { +async function detectPublication( + issueNumber: number, + publication: CandidatePublication | null, +): Promise { + if (!publication) return false; const repo = readRepoContext(workerEnv); const creds = readAppCreds(workerEnv); if (!repo || !creds) return false; try { const token = await mintInstallationToken(creds); const currentBranchSha = await getBranchSha(token, repo, `bot/fix-${issueNumber}`); - return currentBranchSha !== null && currentBranchSha !== previousBranchSha; + return currentBranchSha === publication.commitSha; } catch (error) { - // An unusable App credential must not fail the report itself. - console.warn("[investigate] push detection failed", { error: errorMessage(error) }); + console.warn("[investigate] publication verification failed", { error: errorMessage(error) }); return false; } } +async function publishCandidateForRun( + input: InvestigateData, + commitMessage: string, + snapshot: CandidateSnapshot, +): Promise { + const repo = readRepoContext(workerEnv); + const creds = readAppCreds(workerEnv); + if (!repo || !creds) throw new Error("GitHub App credentials or repository context missing"); + const token = await mintInstallationToken(creds); + return publishCandidate( + { + branch: `bot/fix-${input.issueNumber}`, + runId: input.runId, + commitMessage, + expectedPreviousSha: input.previousBranchSha, + snapshot, + }, + candidateGitHub(token, repo), + ); +} + +function candidateGitHub( + token: string, + repo: NonNullable>, +): CandidateGitHub { + return { + getBranchSha: (branch) => getBranchSha(token, repo, branch), + getCommit: (sha) => getGitCommit(token, repo, sha), + createBlob: (content) => createGitBlob(token, repo, content), + createTree: (baseTreeSha, entries) => createGitTree(token, repo, baseTreeSha, entries), + createCommit: (message, treeSha, parentSha) => + createGitCommit(token, repo, message, treeSha, parentSha), + createBranch: (branch, commitSha) => createBranch(token, repo, branch, commitSha), + updateBranch: (branch, commitSha) => updateBranch(token, repo, branch, commitSha), + }; +} + +function reportPayload( + runId: string, + result: InvestigationResult | ImplementationResult, + pushed: boolean, + publication: CandidatePublication | null, + verification: readonly VerificationRecord[], +) { + return { + result, + ok: true, + pushed, + runId, + publication, + verification: [...verification], + }; +} + +function withRunFailure( + result: T, + failure: RunFailure | null, +): T & { failureStage?: RunFailure["stage"] } { + if (!failure) return result; + return { + ...result, + failureStage: failure.stage, + summary: truncateSummary(`${result.summary}\n\n${failure.message}`), + }; +} + +function safeFailureMessage(error: unknown): string { + return errorMessage(error) + .replaceAll(/[\r\n]+/g, " ") + .slice(0, 500); +} + function buildPrompt(input: InvestigateData): string { const argSection = input.arg ? ["", "## Directive", "", input.arg, ""].join("\n") : ""; const diagnose = input.mode === "diagnose"; + const implement = input.mode === "implement"; const method = diagnose ? [ "- Read AGENTS.md, find the relevant code, and attempt to reproduce the bug.", "- Diagnose the root cause. Do NOT write or push a fix -- this is investigation only.", "- Report `reproduced` and put the diagnosis in `summary`. Use verdict `unclear` only when you are blocked on information that only the reporter can supply.", ] - : [ - "- Read AGENTS.md, find the relevant code, attempt to reproduce, build, or revise.", - "- Write tests where they make sense.", - "- Touch only files relevant to the issue. Do not bulk-format or modify .github/workflows.", - `- When done, commit and push from a container: \`exec\` with target container running \`git checkout -B bot/fix-${input.issueNumber} && git add && git commit -m '' && git push -u origin HEAD --force-with-lease\`.`, - `- If you captured reproduction screenshots in \`.bot-artifacts/\`, keep them off the fix branch (\`git reset HEAD .bot-artifacts\` before committing) and push them to an orphan artifacts branch from a scratch tree: copy \`.bot-artifacts\` aside, \`git init -b bot/artifacts-${input.issueNumber}\`, add and commit only \`.bot-artifacts\`, then \`git push -u origin HEAD --force\`. Report each screenshot's basename and a one-line description in \`screenshots\`.`, - ]; + : implement + ? [ + "- Read AGENTS.md and implement the requested change directly; this mode has no bug-reproduction gate.", + "- Edit with edit_file/write_file. Use exec for exploration only and run every required final check with run_check.", + "- After all latest named checks pass, call publish_candidate with the commit message. Do not run git commit or git push.", + "- Call report_implementation exactly once. implemented=true is valid only after publish_candidate succeeds.", + ] + : [ + "- Read AGENTS.md, find the relevant code, attempt to reproduce, build, or revise.", + "- Write tests where they make sense.", + "- Touch only files relevant to the issue. Do not bulk-format or modify .github/workflows.", + "- Run every required final check with run_check; output pipelines and success fallbacks are rejected.", + "- When the change is verified, call publish_candidate. Do not run git commit or git push yourself.", + `- Reproduction screenshots may still be pushed only to \`bot/artifacts-${input.issueNumber}\`; keep \`.bot-artifacts/\` off the candidate branch and report each screenshot's basename and description.`, + ]; const closing = diagnose ? "Call report_result exactly once when finished. Do not set fixed; report reproduced and your verdict with the diagnosis in summary." - : "Call report_result exactly once when finished. fixed may only be true if a fix and test passed and the branch was pushed."; + : implement + ? "Call report_implementation exactly once when finished." + : "Call report_result exactly once when finished. fixed may only be true if the fix passed verification and publish_candidate succeeded."; return [ `Investigate issue #${input.issueNumber} in mode: ${input.mode}.`, "", diff --git a/infra/emdash-bot/.flue/cloudflare.ts b/infra/emdash-bot/.flue/cloudflare.ts index f3406947e0..64faf61cad 100644 --- a/infra/emdash-bot/.flue/cloudflare.ts +++ b/infra/emdash-bot/.flue/cloudflare.ts @@ -4,8 +4,8 @@ import { Sandbox as BaseSandbox } from "@cloudflare/sandbox"; import { - gateGithubRequest, githubAuthHeader, + inspectGithubRequest, PUSH_CAPABILITY_HEADER, verifyPushCapability, } from "./lib/github-proxy.js"; @@ -62,22 +62,28 @@ async function handleAuthenticatedGithub(request: Request, env: Env): Promise; + getCommit(sha: string): Promise<{ treeSha: string; message: string }>; + createBlob(content: Uint8Array): Promise; + createTree(baseTreeSha: string, entries: readonly GitTreeEntry[]): Promise; + createCommit(message: string, treeSha: string, parentSha: string): Promise; + createBranch(branch: string, commitSha: string): Promise; + updateBranch(branch: string, commitSha: string): Promise; +} + +export interface GitTreeEntry { + readonly path: string; + readonly mode: GitTreeMode; + readonly type: "blob"; + readonly sha: string | null; +} + +export interface CandidatePublication { + readonly branch: string; + readonly commitSha: string; + readonly files: string[]; +} + +export interface PublishCandidateInput { + readonly branch: string; + readonly runId: string; + readonly commitMessage: string; + readonly expectedPreviousSha: string | null; + readonly snapshot: CandidateSnapshot; +} + +export function requireCandidatePublication( + claimed: boolean, + publication: CandidatePublication | null, +): void { + if (claimed && !publication) { + throw new Error("publish_candidate must complete before reporting a published change"); + } +} + +export async function publishCandidate( + input: PublishCandidateInput, + github: CandidateGitHub, +): Promise { + if (input.snapshot.changes.length === 0) throw new Error("candidate has no changes to publish"); + const files = input.snapshot.changes.map((change) => change.path); + const runMarker = `EmDash-Run: ${input.runId}`; + const liveBefore = await github.getBranchSha(input.branch); + if (liveBefore !== input.expectedPreviousSha) { + if (liveBefore) { + const liveCommit = await github.getCommit(liveBefore); + if (liveCommit.message.includes(runMarker)) { + return { branch: input.branch, commitSha: liveBefore, files }; + } + } + throw new Error( + `candidate branch changed since this run started (expected ${input.expectedPreviousSha ?? "absent"}, found ${liveBefore ?? "absent"})`, + ); + } + + const baseCommit = await github.getCommit(input.snapshot.baseCommitSha); + const entries: GitTreeEntry[] = []; + for (const change of input.snapshot.changes) { + entries.push({ + path: change.path, + mode: change.mode, + type: "blob", + sha: change.content === null ? null : await github.createBlob(change.content), + }); + } + const treeSha = await github.createTree(baseCommit.treeSha, entries); + if (treeSha !== input.snapshot.treeSha) { + throw new Error( + `GitHub created tree ${treeSha}, which does not match the verified candidate ${input.snapshot.treeSha}`, + ); + } + const message = `${input.commitMessage.trim()}\n\n${runMarker}`; + const parentSha = input.expectedPreviousSha ?? input.snapshot.baseCommitSha; + const commitSha = await github.createCommit(message, treeSha, parentSha); + + const liveAtUpdate = await github.getBranchSha(input.branch); + if (liveAtUpdate !== input.expectedPreviousSha) { + throw new Error("candidate branch changed while the publication was being prepared"); + } + if (liveAtUpdate === null) await github.createBranch(input.branch, commitSha); + else await github.updateBranch(input.branch, commitSha); + + const publishedSha = await github.getBranchSha(input.branch); + if (publishedSha !== commitSha) { + throw new Error( + `candidate branch verification failed (expected ${commitSha}, found ${publishedSha ?? "absent"})`, + ); + } + return { branch: input.branch, commitSha, files }; +} diff --git a/infra/emdash-bot/.flue/lib/comments.ts b/infra/emdash-bot/.flue/lib/comments.ts index 43c09967be..3767016467 100644 --- a/infra/emdash-bot/.flue/lib/comments.ts +++ b/infra/emdash-bot/.flue/lib/comments.ts @@ -1,4 +1,4 @@ -import type { StateId } from "./machine.js"; +import type { Kind, StateId } from "./machine.js"; import { artifactsBranch, fixBranch, previewInstallCommand } from "./preview.js"; import type { Decision } from "./router.js"; @@ -39,11 +39,11 @@ export function renderReadonlyReply(state: StateId | null): string { case "needs_info": return "I need more to go on -- see my last comment for what's missing."; case "fixing": - return "Building a candidate fix."; + return "Building a candidate change."; case "preview_building": - return "Building a preview so you can try the fix."; + return "Building a preview so you can try the change."; case "awaiting_reporter": - return "Try the preview from my last comment. Reply `@emdashbot confirm` if it's fixed, or describe what's still wrong."; + return "Try the preview from my last comment. Reply `@emdashbot confirm` if it works, or describe what needs to change."; default: { const _exhaustive: never = state; return `State: \`${String(_exhaustive)}\`.`; @@ -62,10 +62,19 @@ export function renderAgentComment( decision: Extract, anchorNumber: number, agentSummary?: string, + failure?: { runId?: string; failureStage?: string }, + previewPackage = "emdash", ): string { const summary = agentSummary?.trim(); if (!decision.event.startsWith("agent.")) return ""; if (!summary) return ""; + if (decision.event === "agent.failed") { + const details = [ + failure?.failureStage ? `Failed stage: \`${failure.failureStage}\`` : "", + failure?.runId ? `Run: \`${failure.runId}\`` : "", + ].filter(Boolean); + return details.length > 0 ? `${summary}\n\n${details.join(" · ")}` : summary; + } switch (decision.event) { case "agent.fix_ready": @@ -80,7 +89,7 @@ export function renderAgentComment( "Try it:", "", "```sh", - `pnpm add https://pkg.pr.new/emdash-cms/emdash@bot/fix-${anchorNumber}`, + previewInstallCommand(anchorNumber, previewPackage), "```", "", "Reply `@emdashbot confirm` if it works and I'll open the PR, or `@emdashbot revise ` to push changes.", @@ -118,7 +127,7 @@ function mdEscape(text: string): string { } /** - * Compose the ask comment posted when a candidate fix's preview has published. + * Compose the ask comment posted when a candidate change's preview has published. * The reporter verifies the change against their own site via the pkg.pr.new * install command, then replies to confirm or reject. * @@ -132,6 +141,7 @@ export function renderPreviewReadyAsk(input: { owner: string; repo: string; issueNumber: number; + previewPackage?: string; at: string; notes?: string | null; screenshots?: readonly PreviewScreenshot[]; @@ -144,24 +154,24 @@ export function renderPreviewReadyAsk(input: { `![${mdEscape(shot.description ?? shot.filename)}](https://raw.githubusercontent.com/${input.owner}/${input.repo}/${artifactsBranch(input.issueNumber)}/.bot-artifacts/${shot.filename})`, ); const reporterAsk = input.reporterLogin - ? `@${input.reporterLogin} could you try this and reply here with whether it resolves the issue? A simple "yes, fixed" or "no, still broken" is enough.` - : "Could the reporter please try this and reply with whether it resolves the issue?"; + ? `@${input.reporterLogin} could you try this and reply here with whether it works as requested? A simple "yes" or "no" is enough.` + : "Could the reporter please try this and reply with whether it works as requested?"; return [ ``, - "The investigation reproduced this issue and pushed a candidate fix.", + "A candidate change is ready to preview.", "", input.notes?.trim() ?? "", "", - "Try the fix against your own site:", + "Try the change against your own site:", "", "```bash", - previewInstallCommand(input.issueNumber), + previewInstallCommand(input.issueNumber, input.previewPackage), "```", "", ...(shots.length > 0 ? ["**Screenshots:**", "", shots.join("\n\n"), ""] : []), reporterAsk, "", - "Maintainers can act on the reporter's behalf: `@emdashbot confirm` to accept the fix and open a draft PR, or `@emdashbot reject` (with details) to reap the branch and revise.", + "Maintainers can act on the reporter's behalf: `@emdashbot confirm` to accept the change and open a draft PR, or `@emdashbot reject` (with details) to reap the branch and revise.", "", `Fix branch: \`${fixBranch(input.issueNumber)}\` · Artifacts branch: \`${artifactsBranch(input.issueNumber)}\``, ] @@ -170,23 +180,26 @@ export function renderPreviewReadyAsk(input: { } /** - * Body for the draft PR opened when the reporter confirms the fix. References + * Body for the draft PR opened when the reporter confirms the change. References * the issue (so merging closes it), points at the preview the reporter just - * verified, and flags that a maintainer must review before merge. The fix run - * left a regression test on the branch; the reviewer confirms it on the diff. + * verified, and flags that a maintainer must review before merge. */ -export function renderDraftPrBody(issueNumber: number): string { +export function renderDraftPrBody(issueNumber: number, previewPackage?: string): string { return [ `Closes #${issueNumber}.`, "", - "A candidate fix the reporter confirmed against their own site via the preview build:", + "A candidate change the reporter confirmed against their own site via the preview build:", "", "```bash", - previewInstallCommand(issueNumber), + previewInstallCommand(issueNumber, previewPackage), "```", "", - "The fix run left a regression test on the branch -- confirm it covers the reported case on review.", + "Review the candidate diff and its verification before merging.", "", "Opened automatically by emdashbot as a draft. A maintainer must review before merge.", ].join("\n"); } + +export function renderPullRequestTitle(issueNumber: number, kind: Kind): string { + return `${kind === "bug" ? "Fix" : "Implement"} #${issueNumber}`; +} diff --git a/infra/emdash-bot/.flue/lib/exec-env.ts b/infra/emdash-bot/.flue/lib/exec-env.ts index 11dc8c327d..102e5bff4a 100644 --- a/infra/emdash-bot/.flue/lib/exec-env.ts +++ b/infra/emdash-bot/.flue/lib/exec-env.ts @@ -15,6 +15,7 @@ import type { Sandbox } from "@cloudflare/sandbox"; +import type { CandidateChange, CandidateSnapshot, GitTreeMode } from "./candidate-publisher.js"; import { withDeadline } from "./sandbox-deadline.js"; export interface ExecResult { @@ -97,6 +98,11 @@ const META_DIR = "/.emdash-bot"; const HYDRATED_MARKER = `${META_DIR}/hydrated`; const CHANGE_LOG = `${META_DIR}/changes.json`; const GREP_MATCH_LIMIT = 200; +const CANDIDATE_FILE_LIMIT = 200; +const CANDIDATE_FILE_SIZE_LIMIT = 2 * 1024 * 1024; +const CANDIDATE_TOTAL_SIZE_LIMIT = 10 * 1024 * 1024; +const DISALLOWED_CANDIDATE_PATHS = [".git/", ".github/workflows/", ".bot-artifacts/"]; +const RAW_DIFF_HEADER = /^:([0-7]{6}) ([0-7]{6}) ([0-9a-f]+) ([0-9a-f]+) ([A-Z])$/; export class ExecEnv { readonly #state: IsolateState; @@ -191,12 +197,192 @@ export class ExecEnv { const container = await this.container(); await this.#materializeChanges(container); return withDeadline( - container.exec(command, { cwd, ...(timeoutMs ? { timeoutMs } : {}) }), + container.exec(pipefailCommand(command), { cwd, ...(timeoutMs ? { timeoutMs } : {}) }), deadlineMs, "container exec", ); } + async runCheck( + command: string, + options: ExecOptions = {}, + ): Promise<{ result: ExecResult; candidateTreeSha: string }> { + const timeoutMs = options.timeoutMs; + const deadlineMs = timeoutMs + ? timeoutMs + this.#deadlines.execGraceMs + : this.#deadlines.defaultTimeoutMs; + const cwd = options.cwd ?? this.#repoDir; + const container = await this.container(); + await this.#materializeChanges(container); + const beforeTreeSha = await this.#candidateTreeSha(container); + const result = await withDeadline( + container.exec(pipefailCommand(command), { cwd, ...(timeoutMs ? { timeoutMs } : {}) }), + deadlineMs, + "container check", + ); + const candidateTreeSha = await this.#candidateTreeSha(container); + if (candidateTreeSha !== beforeTreeSha) { + await this.#restoreContainerCandidate(container); + throw new Error( + "verification command modified the candidate; apply source changes with edit_file/write_file and rerun a check-only command", + ); + } + return { result, candidateTreeSha }; + } + + /** Stage the working tree and return a bounded snapshot for Worker-owned publication. */ + async snapshotCandidate(): Promise { + const container = await this.container(); + await this.#materializeChanges(container); + await this.#stageCandidate(container); + const [base, tree, diff] = await Promise.all([ + this.#bounded( + container.exec(pipefailCommand("git rev-parse HEAD"), { cwd: this.#repoDir }), + "candidate base", + ), + this.#bounded( + container.exec(pipefailCommand("git write-tree"), { cwd: this.#repoDir }), + "candidate tree", + ), + this.#bounded( + container.exec( + pipefailCommand("git diff --cached --raw --abbrev=64 --no-renames -z HEAD --"), + { + cwd: this.#repoDir, + }, + ), + "candidate diff", + ), + ]); + if (base.exitCode !== 0) throw new Error(`candidate base lookup failed: ${lastOutput(base)}`); + if (tree.exitCode !== 0) throw new Error(`candidate tree lookup failed: ${lastOutput(tree)}`); + if (diff.exitCode !== 0) throw new Error(`candidate diff failed: ${lastOutput(diff)}`); + const entries = parseRawGitDiff(diff.stdout); + if (entries.length === 0) throw new Error("candidate has no staged changes"); + if (entries.length > CANDIDATE_FILE_LIMIT) { + throw new Error( + `candidate changes ${entries.length} files; limit is ${CANDIDATE_FILE_LIMIT}`, + ); + } + + const changes: CandidateChange[] = []; + let totalBytes = 0; + for (const entry of entries) { + assertCandidatePath(entry.path); + if (entry.deleted) { + changes.push({ path: entry.path, mode: entry.mode, content: null }); + continue; + } + if (!entry.blobSha) throw new Error(`candidate staged blob is missing for ${entry.path}`); + const content = await this.#readStagedBlob(container, entry.blobSha); + if (content.byteLength > CANDIDATE_FILE_SIZE_LIMIT) { + throw new Error( + `candidate file ${entry.path} is ${content.byteLength} bytes; limit is ${CANDIDATE_FILE_SIZE_LIMIT}`, + ); + } + totalBytes += content.byteLength; + if (totalBytes > CANDIDATE_TOTAL_SIZE_LIMIT) { + throw new Error(`candidate content exceeds ${CANDIDATE_TOTAL_SIZE_LIMIT} bytes`); + } + await assertGitBlobContent(content, entry.blobSha, entry.path); + changes.push({ path: entry.path, mode: entry.mode, content }); + } + return { baseCommitSha: base.stdout.trim(), treeSha: tree.stdout.trim(), changes }; + } + + async candidateTreeSha(options: { materialize?: boolean } = {}): Promise { + const container = await this.container(); + if (options.materialize !== false) await this.#materializeChanges(container); + return this.#candidateTreeSha(container); + } + + async #candidateTreeSha(container: ContainerBackend): Promise { + await this.#stageCandidate(container); + const tree = await this.#bounded( + container.exec(pipefailCommand("git write-tree"), { cwd: this.#repoDir }), + "candidate tree", + ); + if (tree.exitCode !== 0) throw new Error(`candidate tree lookup failed: ${lastOutput(tree)}`); + return tree.stdout.trim(); + } + + async #restoreContainerCandidate(container: ContainerBackend): Promise { + const restore = await this.#bounded( + container.exec( + pipefailCommand("git reset --hard HEAD && git clean -fd --exclude=.bot-artifacts/ -- ."), + { cwd: this.#repoDir }, + ), + "candidate restore", + ); + if (restore.exitCode !== 0) { + throw new Error(`candidate restore failed: ${lastOutput(restore)}`); + } + await this.#materializeChanges(container); + } + + async #stageCandidate(container: ContainerBackend): Promise { + const stage = await this.#bounded( + container.exec( + pipefailCommand("git add --all -- . && git reset --quiet HEAD -- .bot-artifacts"), + { + cwd: this.#repoDir, + }, + ), + "candidate stage", + ); + if (stage.exitCode !== 0) { + throw new Error(`candidate staging failed: ${lastOutput(stage)}`); + } + } + + async #readStagedBlob(container: ContainerBackend, blobSha: string): Promise { + const tempPath = `/tmp/emdash-candidate-${crypto.randomUUID()}`; + let content: Uint8Array | undefined; + let readFailure: unknown; + try { + const materialize = await this.#bounded( + container.exec( + pipefailCommand(`git cat-file blob ${quote(blobSha)} > ${quote(tempPath)}`), + { cwd: this.#repoDir }, + ), + "candidate staged file", + ); + if (materialize.exitCode !== 0) { + throw new Error(`candidate staged file read failed: ${lastOutput(materialize)}`); + } + content = await this.#bounded( + container.readFileBytes(tempPath), + "candidate staged file read", + ); + } catch (error) { + readFailure = error; + } + + let cleanupFailure: unknown; + try { + const cleanup = await this.#bounded( + container.exec(pipefailCommand(`rm -f -- ${quote(tempPath)}`), { cwd: "/" }), + "candidate staged file cleanup", + ); + if (cleanup.exitCode !== 0) { + throw new Error(`candidate staged file cleanup failed: ${lastOutput(cleanup)}`); + } + } catch (error) { + cleanupFailure = error; + } + + if (readFailure && cleanupFailure) { + throw new AggregateError( + [readFailure, cleanupFailure], + "candidate staged file read and cleanup failed", + ); + } + if (readFailure) throw readFailure; + if (cleanupFailure) throw cleanupFailure; + if (!content) throw new Error("candidate staged file read returned no content"); + return content; + } + /** * Read a container-produced artifact (a screenshot) for egress. `name` is a * bare filename under `/.bot-artifacts/`; a path separator, `.`, `..`, @@ -267,11 +453,87 @@ export class ExecEnv { const TRAILING_SLASH = /\/+$/; const PATH_SEPARATOR = /[/\\]/; +interface RawDiffEntry { + path: string; + mode: GitTreeMode; + deleted: boolean; + blobSha: string | null; +} + +export function parseRawGitDiff(raw: string): RawDiffEntry[] { + if (raw === "") return []; + const tokens = raw.split("\0"); + if (tokens.at(-1) !== "" || tokens.length % 2 !== 1) { + throw new Error("malformed staged diff"); + } + const entries: RawDiffEntry[] = []; + for (let index = 0; index < tokens.length - 1; index += 2) { + const header = tokens[index]; + const path = tokens[index + 1]; + if (!header || !path) throw new Error("malformed staged diff"); + const match = RAW_DIFF_HEADER.exec(header); + if (!match) throw new Error(`unsupported staged diff entry: ${header}`); + const [, oldMode, newMode, , newSha, status] = match; + if (status === "U") throw new Error(`candidate contains an unresolved merge at ${path}`); + const deleted = status === "D"; + const mode = gitTreeMode(deleted ? oldMode : newMode); + if (mode === "120000") throw new Error(`candidate cannot publish symlink: ${path}`); + entries.push({ path, mode, deleted, blobSha: deleted ? null : (newSha ?? null) }); + } + return entries; +} + +async function assertGitBlobContent( + content: Uint8Array, + expectedSha: string, + path: string, +): Promise { + const algorithm = + expectedSha.length === 40 ? "SHA-1" : expectedSha.length === 64 ? "SHA-256" : null; + if (!algorithm) throw new Error(`unsupported candidate blob SHA for ${path}`); + const header = new TextEncoder().encode(`blob ${content.byteLength}\0`); + const input = new Uint8Array(header.byteLength + content.byteLength); + input.set(header); + input.set(content, header.byteLength); + const digest = new Uint8Array(await crypto.subtle.digest(algorithm, input)); + let actualSha = ""; + for (const byte of digest) actualSha += byte.toString(16).padStart(2, "0"); + if (actualSha !== expectedSha) { + throw new Error(`candidate content for ${path} does not match staged blob ${expectedSha}`); + } +} + +function gitTreeMode(mode: string | undefined): GitTreeMode { + if (mode === "100644" || mode === "100755" || mode === "120000") return mode; + throw new Error(`unsupported candidate file mode: ${mode ?? "missing"}`); +} + +function assertCandidatePath(path: string): void { + if ( + path === "" || + path.startsWith("/") || + path.split("/").includes("..") || + DISALLOWED_CANDIDATE_PATHS.some( + (prefix) => path === prefix.slice(0, -1) || path.startsWith(prefix), + ) + ) { + throw new Error(`candidate cannot publish path: ${path}`); + } +} + +function lastOutput(result: ExecResult): string { + return (result.stderr || result.stdout || `exit ${result.exitCode}`).trim().slice(-500); +} + /** Single-quote a shell argument for a container command line. */ export function quote(value: string): string { return `'${value.replaceAll("'", "'\\''")}'`; } +export function pipefailCommand(command: string): string { + return `bash -o pipefail -c ${quote(command)}`; +} + /** Adapt a @cloudflare/sandbox session to the ContainerBackend seam. */ export function fromSandbox(sandbox: Sandbox): ContainerBackend { return { diff --git a/infra/emdash-bot/.flue/lib/github-proxy.ts b/infra/emdash-bot/.flue/lib/github-proxy.ts index cc1209442d..b98835e4a3 100644 --- a/infra/emdash-bot/.flue/lib/github-proxy.ts +++ b/infra/emdash-bot/.flue/lib/github-proxy.ts @@ -1,5 +1,5 @@ -const GIT_REF = /refs\/(?:heads|tags)\/\S+/g; const PKT_LINE_HEADER = /^[0-9a-fA-F]{4}$/; +const WHITESPACE = /\s/; const MAX_RECEIVE_PACK_COMMAND_BYTES = 64 * 1024; export const PUSH_CAPABILITY_HEADER = "X-EmDash-Push-Capability"; @@ -91,6 +91,27 @@ export async function gateGithubRequest( repo: string, issueNumber?: number, ): Promise { + const result = await inspectGithubRequest(request, url, owner, repo, issueNumber); + return result.allowed ? null : result.reason; +} + +export type GithubGateResult = + | { allowed: true; stage: "allowed"; refs?: readonly string[] } + | { + allowed: false; + stage: "repository" | "capability" | "receive-pack"; + reason: string; + refs?: readonly string[]; + parseError?: string; + }; + +export async function inspectGithubRequest( + request: Request, + url: URL, + owner: string, + repo: string, + issueNumber?: number, +): Promise { const method = request.method.toUpperCase(); const host = url.host; @@ -101,12 +122,26 @@ export async function gateGithubRequest( (method === "GET" || method === "HEAD") && (url.pathname === repoPath || url.pathname === `${repoPath}/`) ) { - return null; + return { allowed: true, stage: "allowed" }; } if (url.pathname === `${gitPath}/git-receive-pack` && method === "POST") { - return issueNumber !== undefined && (await hasOnlyBotBranchUpdates(request, issueNumber)) - ? null - : "git push may only update the current issue's bot fix branch"; + if (issueNumber === undefined) { + return { + allowed: false, + stage: "capability", + reason: "git push requires a valid issue-scoped capability", + }; + } + const inspection = await inspectReceivePack(request, issueNumber); + return inspection.allowed + ? { allowed: true, stage: "allowed", refs: inspection.refs } + : { + allowed: false, + stage: "receive-pack", + reason: "git push may only update the current issue's bot artifacts branch", + refs: inspection.refs, + parseError: inspection.parseError, + }; } if ( (url.pathname === gitPath || @@ -115,23 +150,35 @@ export async function gateGithubRequest( url.pathname === `${gitPath}/git-upload-pack`) && (method === "GET" || method === "HEAD" || method === "POST") ) { - return null; + return { allowed: true, stage: "allowed" }; } - return `github.com request outside configured repository git operations`; + return { + allowed: false, + stage: "repository", + reason: "github.com request outside configured repository git operations", + }; } if (host === "codeload.github.com") { if ((method === "GET" || method === "HEAD") && url.pathname.startsWith(`/${owner}/${repo}/`)) { - return null; + return { allowed: true, stage: "allowed" }; } - return "codeload request outside configured repository"; + return { + allowed: false, + stage: "repository", + reason: "codeload request outside configured repository", + }; } if (host === "raw.githubusercontent.com") { if ((method === "GET" || method === "HEAD") && url.pathname.startsWith(`/${owner}/${repo}/`)) { - return null; + return { allowed: true, stage: "allowed" }; } - return "raw content request outside configured repository"; + return { + allowed: false, + stage: "repository", + reason: "raw content request outside configured repository", + }; } if (host === "api.github.com") { @@ -140,17 +187,28 @@ export async function gateGithubRequest( (method === "GET" || method === "HEAD") && (url.pathname === repoBase || url.pathname.startsWith(`${repoBase}/`)) ) { - return null; + return { allowed: true, stage: "allowed" }; } - return "GitHub API access is read-only and limited to the configured repository"; + return { + allowed: false, + stage: "repository", + reason: "GitHub API access is read-only and limited to the configured repository", + }; } - return `host ${host} is not allowed through the authenticated proxy`; + return { + allowed: false, + stage: "repository", + reason: `host ${host} is not allowed through the authenticated proxy`, + }; } -async function hasOnlyBotBranchUpdates(request: Request, issueNumber: number): Promise { +async function inspectReceivePack( + request: Request, + issueNumber: number, +): Promise<{ allowed: boolean; refs: string[]; parseError?: string }> { const reader = request.clone().body?.getReader(); - if (!reader) return false; + if (!reader) return { allowed: false, refs: [], parseError: "request body is missing" }; let buffer = new Uint8Array(); let offset = 0; const refs: string[] = []; @@ -160,21 +218,24 @@ async function hasOnlyBotBranchUpdates(request: Request, issueNumber: number): P for (;;) { while (buffer.length - offset >= 4) { const header = decoder.decode(buffer.subarray(offset, offset + 4)); - if (!PKT_LINE_HEADER.test(header)) return false; + if (!PKT_LINE_HEADER.test(header)) { + return { allowed: false, refs, parseError: "invalid pkt-line header" }; + } const length = Number.parseInt(header, 16); if (length === 0) { - const allowed = new Set([ - `refs/heads/bot/fix-${issueNumber}`, - `refs/heads/bot/artifacts-${issueNumber}`, - ]); - return refs.length > 0 && refs.every((ref) => allowed.has(ref)); + const allowed = new Set([`refs/heads/bot/artifacts-${issueNumber}`]); + return { allowed: refs.length > 0 && refs.every((ref) => allowed.has(ref)), refs }; + } + if (length < 4 || length > MAX_RECEIVE_PACK_COMMAND_BYTES) { + return { allowed: false, refs, parseError: "invalid pkt-line length" }; } - if (length < 4 || length > MAX_RECEIVE_PACK_COMMAND_BYTES) return false; if (buffer.length - offset < length) break; - const payload = decoder - .decode(buffer.subarray(offset + 4, offset + length)) - .replaceAll(String.fromCharCode(0), " "); - refs.push(...(payload.match(GIT_REF) ?? [])); + const payload = decoder.decode(buffer.subarray(offset + 4, offset + length)); + const ref = receivePackCommandRef(payload); + if (!ref) { + return { allowed: false, refs, parseError: "invalid receive-pack command" }; + } + refs.push(ref); offset += length; } @@ -182,9 +243,11 @@ async function hasOnlyBotBranchUpdates(request: Request, issueNumber: number): P buffer = buffer.slice(offset); offset = 0; } - if (buffer.length >= MAX_RECEIVE_PACK_COMMAND_BYTES) return false; + if (buffer.length >= MAX_RECEIVE_PACK_COMMAND_BYTES) { + return { allowed: false, refs, parseError: "receive-pack command prefix is too large" }; + } const { done, value } = await reader.read(); - if (done) return false; + if (done) return { allowed: false, refs, parseError: "receive-pack ended before flush" }; const remaining = MAX_RECEIVE_PACK_COMMAND_BYTES - buffer.length; const chunk = value.subarray(0, remaining); const next = new Uint8Array(buffer.length + chunk.length); @@ -196,3 +259,16 @@ async function hasOnlyBotBranchUpdates(request: Request, issueNumber: number): P void reader.cancel().catch(() => undefined); } } + +function receivePackCommandRef(payload: string): string | null { + const capabilitySeparator = payload.indexOf(String.fromCharCode(0)); + let command = payload.slice(0, capabilitySeparator === -1 ? payload.length : capabilitySeparator); + if (command.endsWith("\n")) command = command.slice(0, -1); + const firstSpace = command.indexOf(" "); + const secondSpace = command.indexOf(" ", firstSpace + 1); + if (firstSpace <= 0 || secondSpace <= firstSpace + 1 || command.includes(" ", secondSpace + 1)) { + return null; + } + const ref = command.slice(secondSpace + 1); + return ref.startsWith("refs/") && !WHITESPACE.test(ref) ? ref : null; +} diff --git a/infra/emdash-bot/.flue/lib/github.ts b/infra/emdash-bot/.flue/lib/github.ts index 4371851a3c..fd8210b64a 100644 --- a/infra/emdash-bot/.flue/lib/github.ts +++ b/infra/emdash-bot/.flue/lib/github.ts @@ -183,6 +183,118 @@ export async function getBranchSha( return json.commit?.sha ?? null; } +export async function getGitCommit( + token: string, + ctx: RepoContext, + sha: string, +): Promise<{ treeSha: string; message: string }> { + const res = await githubFetch( + `${GITHUB_API}/repos/${ctx.owner}/${ctx.repo}/git/commits/${encodeURIComponent(sha)}`, + { headers: authHeaders(token) }, + ); + if (!res.ok) throw new Error(`getGitCommit failed: ${res.status} ${await res.text()}`); + const json = await res.json<{ tree?: { sha?: string }; message?: string }>(); + if (!json.tree?.sha) throw new Error("getGitCommit response had no tree SHA"); + return { treeSha: json.tree.sha, message: json.message ?? "" }; +} + +export async function createGitBlob( + token: string, + ctx: RepoContext, + content: Uint8Array, +): Promise { + const res = await githubFetch(`${GITHUB_API}/repos/${ctx.owner}/${ctx.repo}/git/blobs`, { + method: "POST", + headers: authHeaders(token, { "content-type": "application/json" }), + body: JSON.stringify({ content: bytesToBase64(content), encoding: "base64" }), + }); + if (!res.ok) throw new Error(`createGitBlob failed: ${res.status} ${await res.text()}`); + const json = await res.json<{ sha?: string }>(); + if (!json.sha) throw new Error("createGitBlob response had no SHA"); + return json.sha; +} + +export interface GitTreeInput { + path: string; + mode: "100644" | "100755" | "120000"; + type: "blob"; + sha: string | null; +} + +export async function createGitTree( + token: string, + ctx: RepoContext, + baseTreeSha: string, + entries: readonly GitTreeInput[], +): Promise { + const res = await githubFetch(`${GITHUB_API}/repos/${ctx.owner}/${ctx.repo}/git/trees`, { + method: "POST", + headers: authHeaders(token, { "content-type": "application/json" }), + body: JSON.stringify({ base_tree: baseTreeSha, tree: entries }), + }); + if (!res.ok) throw new Error(`createGitTree failed: ${res.status} ${await res.text()}`); + const json = await res.json<{ sha?: string }>(); + if (!json.sha) throw new Error("createGitTree response had no SHA"); + return json.sha; +} + +export async function createGitCommit( + token: string, + ctx: RepoContext, + message: string, + treeSha: string, + parentSha: string, +): Promise { + const res = await githubFetch(`${GITHUB_API}/repos/${ctx.owner}/${ctx.repo}/git/commits`, { + method: "POST", + headers: authHeaders(token, { "content-type": "application/json" }), + body: JSON.stringify({ message, tree: treeSha, parents: [parentSha] }), + }); + if (!res.ok) throw new Error(`createGitCommit failed: ${res.status} ${await res.text()}`); + const json = await res.json<{ sha?: string }>(); + if (!json.sha) throw new Error("createGitCommit response had no SHA"); + return json.sha; +} + +export async function createBranch( + token: string, + ctx: RepoContext, + branch: string, + commitSha: string, +): Promise { + const res = await githubFetch(`${GITHUB_API}/repos/${ctx.owner}/${ctx.repo}/git/refs`, { + method: "POST", + headers: authHeaders(token, { "content-type": "application/json" }), + body: JSON.stringify({ ref: `refs/heads/${branch}`, sha: commitSha }), + }); + if (!res.ok) throw new Error(`createBranch failed: ${res.status} ${await res.text()}`); +} + +export async function updateBranch( + token: string, + ctx: RepoContext, + branch: string, + commitSha: string, +): Promise { + const res = await githubFetch( + `${GITHUB_API}/repos/${ctx.owner}/${ctx.repo}/git/refs/heads/${encodeURIComponent(branch)}`, + { + method: "PATCH", + headers: authHeaders(token, { "content-type": "application/json" }), + body: JSON.stringify({ sha: commitSha, force: false }), + }, + ); + if (!res.ok) throw new Error(`updateBranch failed: ${res.status} ${await res.text()}`); +} + +function bytesToBase64(bytes: Uint8Array): string { + const chunks: string[] = []; + for (let offset = 0; offset < bytes.length; offset += 32_768) { + chunks.push(String.fromCharCode(...bytes.subarray(offset, offset + 32_768))); + } + return btoa(chunks.join("")); +} + /** Deletes a branch ref. A 404/422 means it is already gone, which is fine. */ export async function deleteBranch(token: string, ctx: RepoContext, branch: string): Promise { const res = await githubFetch( diff --git a/infra/emdash-bot/.flue/lib/machine.json b/infra/emdash-bot/.flue/lib/machine.json index 83e0af8588..ab63abab1c 100644 --- a/infra/emdash-bot/.flue/lib/machine.json +++ b/infra/emdash-bot/.flue/lib/machine.json @@ -177,7 +177,7 @@ "fixing": { "label": "bot:fixing", "boardColumn": "Fixing", - "description": "A maintainer-triggered fix run is in flight: build a candidate change on bot/fix-. No PR yet.", + "description": "A maintainer-triggered delivery run is building a candidate on bot/fix-.", "terminal": false, "transient": true, "offeredCommands": [ @@ -187,7 +187,7 @@ "preview_building": { "label": "bot:preview-building", "boardColumn": "Building preview", - "description": "The candidate fix is pushed; a preview deploy is building so the reporter can try the change before a PR exists.", + "description": "The candidate change is published; a preview is building so the reporter can try it before a PR exists.", "terminal": false, "transient": true, "offeredCommands": [ @@ -197,7 +197,7 @@ "awaiting_reporter": { "label": "bot:awaiting-reporter", "boardColumn": "Awaiting reporter", - "description": "Preview link posted; waiting for the reporter to confirm the fix. On confirm a draft PR opens; on denial or 14-day silence the branch is reaped.", + "description": "Preview link posted; waiting for the reporter to confirm the change. On confirm a draft PR opens; on denial or 14-day silence the branch is reaped.", "terminal": false, "offeredCommands": [ "confirm", @@ -347,7 +347,7 @@ ] }, "agent.fix_ready": { - "description": "Reproduced and fixed; a verified change is staged on bot/fix-.", + "description": "A verified candidate change is published on bot/fix-.", "actors": [ "system" ] @@ -395,7 +395,7 @@ ] }, "preview.ready": { - "description": "The preview deploy for the candidate fix is live; link ready to post.", + "description": "The preview deploy for the candidate change is live; link ready to post.", "actors": [ "system" ] @@ -423,9 +423,9 @@ { "from": "unmanaged", "event": "implement", - "to": "working", + "to": "fixing", "action": "investigate.implement", - "note": "implement works straight from an untriaged issue" + "note": "implement works straight from an untriaged issue and enters the preview-gated delivery lane" }, { "from": "unmanaged", @@ -441,7 +441,7 @@ { "from": "triage", "event": "implement", - "to": "working", + "to": "fixing", "action": "investigate.implement", "note": "enhancement/feature lane -- no repro gate" }, @@ -488,7 +488,7 @@ { "from": "blocked", "event": "implement", - "to": "working", + "to": "fixing", "action": "investigate.implement" }, { @@ -682,7 +682,7 @@ { "from": "failed", "event": "implement", - "to": "working", + "to": "fixing", "action": "investigate.implement" }, { @@ -869,7 +869,11 @@ "from": "preview_building", "event": "preview.failed", "to": "reproduced", - "note": "candidate branch retained; the diagnosis still holds, so fall back to the reproduced verdict" + "toByKind": { + "enhancement": "blocked", + "task": "blocked" + }, + "note": "bugs return to the reproduced verdict; directed changes rest in blocked so implement can retry" }, { "from": "awaiting_reporter", @@ -881,15 +885,23 @@ "from": "awaiting_reporter", "event": "reject", "to": "reproduced", + "toByKind": { + "enhancement": "blocked", + "task": "blocked" + }, "action": "reapBranch", - "note": "denial reaps the unvalidated branch; feedback is recorded for the next fix" + "note": "denial reaps the unvalidated branch; directed changes remain retryable through implement" }, { "from": "awaiting_reporter", "event": "expire", "to": "reproduced", + "toByKind": { + "enhancement": "blocked", + "task": "blocked" + }, "action": "reapBranch", - "note": "14-day silence reaps the branch; the reproduced verdict survives" + "note": "14-day silence reaps the branch; bugs retain their verdict and directed changes remain retryable" }, { "from": "awaiting_reporter", diff --git a/infra/emdash-bot/.flue/lib/machine.ts b/infra/emdash-bot/.flue/lib/machine.ts index bcd8ab713e..dd3093dd5c 100644 --- a/infra/emdash-bot/.flue/lib/machine.ts +++ b/infra/emdash-bot/.flue/lib/machine.ts @@ -215,8 +215,7 @@ export const STATES: Record = { fixing: { label: "bot:fixing", boardColumn: "Fixing", - description: - "A maintainer-triggered fix run is in flight: build a candidate change on bot/fix-. No PR yet.", + description: "A maintainer-triggered delivery run is building a candidate on bot/fix-.", terminal: false, transient: true, offeredCommands: ["status"], @@ -225,7 +224,7 @@ export const STATES: Record = { label: "bot:preview-building", boardColumn: "Building preview", description: - "The candidate fix is pushed; a preview deploy is building so the reporter can try the change before a PR exists.", + "The candidate change is published; a preview is building so the reporter can try it before a PR exists.", terminal: false, transient: true, offeredCommands: ["status"], @@ -234,7 +233,7 @@ export const STATES: Record = { label: "bot:awaiting-reporter", boardColumn: "Awaiting reporter", description: - "Preview link posted; waiting for the reporter to confirm the fix. On confirm a draft PR opens; on denial or 14-day silence the branch is reaped.", + "Preview link posted; waiting for the reporter to confirm the change. On confirm a draft PR opens; on denial or 14-day silence the branch is reaped.", terminal: false, offeredCommands: ["confirm", "reject", "decline", "take_over"], }, @@ -438,7 +437,7 @@ export const EVENTS: Record = { actors: ["system"], }, "agent.fix_ready": { - description: "Reproduced and fixed; a verified change is staged on bot/fix-.", + description: "A verified candidate change is published on bot/fix-.", actors: ["system"], }, // Next-generation: the investigation ran but is blocked on reporter-only @@ -469,7 +468,7 @@ export const EVENTS: Record = { }, // --- preview-deploy lifecycle (next-generation fix loop) --- "preview.ready": { - description: "The preview deploy for the candidate fix is live; link ready to post.", + description: "The preview deploy for the candidate change is live; link ready to post.", actors: ["system"], }, "preview.failed": { @@ -509,6 +508,8 @@ export interface Transition { from: StateId; event: EventId; to: StateId; + /** Kind-specific destinations; `to` remains the fallback when no override exists. */ + toByKind?: Partial>; /** Agent action the router dispatches on this transition, if any. */ action?: ActionId; /** Human-readable note for the generated table. */ @@ -521,9 +522,9 @@ export const TRANSITIONS: Transition[] = [ { from: "unmanaged", event: "implement", - to: "working", + to: "fixing", action: "investigate.implement", - note: "implement works straight from an untriaged issue", + note: "implement works straight from an untriaged issue and enters the preview-gated delivery lane", }, { from: "unmanaged", event: "decline", to: "declined" }, @@ -532,7 +533,7 @@ export const TRANSITIONS: Transition[] = [ { from: "triage", event: "implement", - to: "working", + to: "fixing", action: "investigate.implement", note: "enhancement/feature lane -- no repro gate", }, @@ -562,7 +563,7 @@ export const TRANSITIONS: Transition[] = [ { from: "working", event: "agent.failed", to: "failed" }, // --- blocked: every reason accepts the same overrides (kills the sinks) --- - { from: "blocked", event: "implement", to: "working", action: "investigate.implement" }, + { from: "blocked", event: "implement", to: "fixing", action: "investigate.implement" }, { from: "blocked", event: "repro", to: "working", action: "investigate.repro" }, { from: "blocked", event: "retry", to: "working", action: "investigate.repro" }, { from: "blocked", event: "decline", to: "declined" }, @@ -646,7 +647,7 @@ export const TRANSITIONS: Transition[] = [ // --- failed: retryable --- { from: "failed", event: "retry", to: "working", action: "investigate.repro" }, - { from: "failed", event: "implement", to: "working", action: "investigate.implement" }, + { from: "failed", event: "implement", to: "fixing", action: "investigate.implement" }, { from: "failed", event: "repro", to: "working", action: "investigate.repro" }, { from: "failed", event: "decline", to: "declined" }, @@ -756,7 +757,8 @@ export const TRANSITIONS: Transition[] = [ from: "preview_building", event: "preview.failed", to: "reproduced", - note: "candidate branch retained; the diagnosis still holds, so fall back to the reproduced verdict", + toByKind: { enhancement: "blocked", task: "blocked" }, + note: "bugs return to the reproduced verdict; directed changes rest in blocked so implement can retry", }, { from: "awaiting_reporter", event: "confirm", to: "in_review", action: "openDraftPr" }, @@ -764,15 +766,17 @@ export const TRANSITIONS: Transition[] = [ from: "awaiting_reporter", event: "reject", to: "reproduced", + toByKind: { enhancement: "blocked", task: "blocked" }, action: "reapBranch", - note: "denial reaps the unvalidated branch; feedback is recorded for the next fix", + note: "denial reaps the unvalidated branch; directed changes remain retryable through implement", }, { from: "awaiting_reporter", event: "expire", to: "reproduced", + toByKind: { enhancement: "blocked", task: "blocked" }, action: "reapBranch", - note: "14-day silence reaps the branch; the reproduced verdict survives", + note: "14-day silence reaps the branch; bugs retain their verdict and directed changes remain retryable", }, { from: "awaiting_reporter", event: "take_over", to: "human_owned" }, { @@ -820,6 +824,14 @@ export function findTransition(from: StateId, event: EventId): Transition | unde return TRANSITIONS.find((t) => t.from === from && t.event === event); } +export function transitionTarget(transition: Transition, kind: Kind | null): StateId { + return (kind ? transition.toByKind?.[kind] : undefined) ?? transition.to; +} + +export function transitionTargets(transition: Transition): StateId[] { + return [...new Set([transition.to, ...Object.values(transition.toByKind ?? {})])]; +} + /** Events that are valid commands from a given state (for status/help replies). */ export function commandsFrom(from: StateId): CommandVerb[] { return STATES[from].offeredCommands; @@ -850,11 +862,13 @@ export function validateMachine(): MachineProblem[] { message: `Non-deterministic: two transitions for ${key}`, }); seen.add(key); - if (!STATES[t.to]) - problems.push({ - severity: "error", - message: `Transition ${key} targets unknown state ${t.to}`, - }); + for (const target of transitionTargets(t)) { + if (!STATES[target]) + problems.push({ + severity: "error", + message: `Transition ${key} targets unknown state ${target}`, + }); + } } // Unique labels across kinds + states. @@ -884,9 +898,12 @@ export function validateMachine(): MachineProblem[] { while (grew) { grew = false; for (const t of TRANSITIONS) { - if (reachable.has(t.from) && !reachable.has(t.to)) { - reachable.add(t.to); - grew = true; + if (!reachable.has(t.from)) continue; + for (const target of transitionTargets(t)) { + if (!reachable.has(target)) { + reachable.add(target); + grew = true; + } } } } @@ -920,7 +937,9 @@ function canReachTerminal(start: StateId): boolean { if (seen.has(id)) continue; seen.add(id); if (STATES[id].terminal) return true; - for (const next of TRANSITIONS.filter((t) => t.from === id)) stack.push(next.to); + for (const next of TRANSITIONS.filter((t) => t.from === id)) { + stack.push(...transitionTargets(next)); + } } return false; } diff --git a/infra/emdash-bot/.flue/lib/orchestrator.ts b/infra/emdash-bot/.flue/lib/orchestrator.ts index ca178d317a..232ebabe52 100644 --- a/infra/emdash-bot/.flue/lib/orchestrator.ts +++ b/infra/emdash-bot/.flue/lib/orchestrator.ts @@ -13,6 +13,7 @@ import { type PreviewScreenshot, renderAgentComment, renderDraftPrBody, + renderPullRequestTitle, renderPreviewReadyAsk, renderReadonlyReply, shouldPostReadonlyReply, @@ -106,6 +107,9 @@ export interface NormalizedEvent { readonly dryRun?: boolean; /** Agent's structured summary, surfaced in the post-run comment. */ readonly agentSummary?: string; + /** Durable run metadata appended to failed comments for operational lookup. */ + readonly agentRunId?: string; + readonly agentFailureStage?: string; /** Reproduction screenshots the fix run pushed, carried into the ask comment. */ readonly agentScreenshots?: readonly PreviewScreenshot[]; /** @@ -132,7 +136,9 @@ export interface AgentResult { readonly skipped?: boolean; readonly reproduced?: boolean; readonly fixed?: boolean; + readonly implemented?: boolean; readonly verdict?: string; + readonly failureStage?: string; readonly screenshots?: readonly PreviewScreenshot[]; readonly [key: string]: unknown; } @@ -449,7 +455,11 @@ export class OrchestratorDO extends DurableObject { labels, needsClassify: false, settlesRunId: input.runId, + agentRunId: input.runId, ...(agentSummary ? { agentSummary } : {}), + ...(typeof input.result.failureStage === "string" + ? { agentFailureStage: input.result.failureStage } + : {}), ...(agentScreenshots ? { agentScreenshots } : {}), }); await this.clearRun(input.runId); @@ -543,11 +553,11 @@ export class OrchestratorDO extends DurableObject { } /** - * Poll pkg.pr.new for the candidate fix's preview while the item sits in + * Poll pkg.pr.new for the candidate change's preview while the item sits in * `preview_building`. One probe per alarm tick (the alarm cadence IS the * poll interval -- no unbounded loop in the DO). A 200 fires `preview.ready` * and advances to the reporter ask; exhausting the overall budget fires - * `preview.failed`, which retains the branch and falls back to the diagnosis. + * `preview.failed`, which retains the branch for inspection. */ private async pollPreviewBuild(now: number): Promise { const [state, deadline, nextAt] = await Promise.all([ @@ -560,7 +570,22 @@ export class OrchestratorDO extends DurableObject { const anchorNumber = await this.ctx.storage.get(STORAGE.anchorNumber); if (anchorNumber === undefined) return "idle"; - const ready = await probePreviewReady(previewUrl(anchorNumber)); + let candidatePreviewUrl: string; + try { + candidatePreviewUrl = previewUrl(anchorNumber, this.env.PREVIEW_PACKAGE); + } catch (error) { + console.error("[orchestrator] invalid preview configuration", { + error: errorMessage(error), + }); + await this.firePreviewEvent( + anchorNumber, + "preview.failed", + "The preview package configuration is invalid. The candidate branch was retained for inspection.", + ); + return "failed"; + } + + const ready = await probePreviewReady(candidatePreviewUrl); if (ready) { await this.firePreviewReady(anchorNumber); return "ready"; @@ -600,6 +625,7 @@ export class OrchestratorDO extends DurableObject { owner: repo.owner, repo: repo.repo, issueNumber: anchorNumber, + previewPackage: this.env.PREVIEW_PACKAGE, at: new Date().toISOString(), notes, ...(screenshots ? { screenshots } : {}), @@ -616,11 +642,16 @@ export class OrchestratorDO extends DurableObject { }); } - private async firePreviewEvent(anchorNumber: number, event: EventId): Promise { + private async firePreviewEvent( + anchorNumber: number, + event: EventId, + failureComment?: string, + ): Promise { const labels = await this.projectLabels(); const commentBodyOverride = event === "preview.failed" - ? `The preview build for the candidate fix didn't publish within ${Math.round(PREVIEW_BUILD_TIMEOUT_MS / 60_000)} minutes. The diagnosis still holds -- a maintainer can \`@emdashbot fix\` to rebuild the candidate.` + ? (failureComment ?? + `The preview build for the candidate change didn't publish within ${Math.round(PREVIEW_BUILD_TIMEOUT_MS / 60_000)} minutes. The candidate branch was retained for inspection.`) : undefined; await this.processEvent({ event, @@ -1127,15 +1158,16 @@ export class OrchestratorDO extends DurableObject { ): Promise { const token = await this.getInstallationToken(creds); const headBranch = `bot/fix-${anchorNumber}`; + const kind = (await this.ctx.storage.get(STORAGE.kind)) ?? "bug"; try { const created = (await getOpenPullRequest(token, repo, headBranch)) ?? (await createPullRequest(token, repo, { headBranch, baseBranch: "main", - title: `Fix #${anchorNumber}`, + title: renderPullRequestTitle(anchorNumber, kind), body: draft - ? renderDraftPrBody(anchorNumber) + ? renderDraftPrBody(anchorNumber, this.env.PREVIEW_PACKAGE) : `Fixes #${anchorNumber}.\n\nAutomated PR opened by emdashbot.`, draft, })); @@ -1382,7 +1414,16 @@ export class OrchestratorDO extends DurableObject { removeLabels: decision.removeLabels, commentBody: input.commentBodyOverride ?? - renderComment(decision, anchorNumber, input.agentSummary), + renderComment( + decision, + anchorNumber, + input.agentSummary, + { + runId: input.agentRunId, + failureStage: input.agentFailureStage, + }, + this.env.PREVIEW_PACKAGE, + ), commentMarker: ``, commentMayExist: false, ...(input.commentFirst ? { commentFirst: true } : {}), @@ -1693,15 +1734,28 @@ export class OrchestratorDO extends DurableObject { /** Test-only: land directly in `preview_building` with the ask's persisted * inputs, so the preview-poll path can be exercised without dispatching the * (runtime-less in tests) investigate agent through fixing. */ - async debugPrimePreviewBuilding(anchorNumber: number, notes: string): Promise { + async debugPrimePreviewBuilding( + anchorNumber: number, + notes: string, + kind: Kind = "bug", + ): Promise { await Promise.all([ this.ctx.storage.put(STORAGE.state, "preview_building" satisfies StateId), - this.ctx.storage.put(STORAGE.kind, "bug" satisfies Kind), + this.ctx.storage.put(STORAGE.kind, kind), this.ctx.storage.put(STORAGE.anchorNumber, anchorNumber), this.ctx.storage.put(STORAGE.previewNotes, notes), ]); } + /** Test-only: land in `fixing` without dispatching the investigate agent. */ + async debugPrimeFixing(anchorNumber: number): Promise { + await Promise.all([ + this.ctx.storage.put(STORAGE.state, "fixing" satisfies StateId), + this.ctx.storage.put(STORAGE.kind, "enhancement" satisfies Kind), + this.ctx.storage.put(STORAGE.anchorNumber, anchorNumber), + ]); + } + /** Test-only: inject dispatch recovery state without invoking Flue. */ async debugSetPendingDispatch(input: { runId: string; @@ -1807,6 +1861,8 @@ function renderComment( decision: Extract, anchorNumber: number, agentSummary?: string, + failure?: { runId?: string; failureStage?: string }, + previewPackage?: string, ): string { - return renderAgentComment(decision, anchorNumber, agentSummary); + return renderAgentComment(decision, anchorNumber, agentSummary, failure, previewPackage); } diff --git a/infra/emdash-bot/.flue/lib/preview.ts b/infra/emdash-bot/.flue/lib/preview.ts index 49c4d8e3f1..b50205dac5 100644 --- a/infra/emdash-bot/.flue/lib/preview.ts +++ b/infra/emdash-bot/.flue/lib/preview.ts @@ -8,6 +8,7 @@ // ask comment only advertises a preview that has actually resolved. const PREVIEW_PROBE_TIMEOUT_MS = 10_000; +const PREVIEW_PACKAGE_CHARS = /^[a-zA-Z0-9@._/-]+$/; /** The fix branch pkg.pr.new keys the preview to. */ export function fixBranch(issueNumber: number): string { @@ -38,13 +39,22 @@ export function branchesToReap(issueNumber: number, hasOpenFixPr: boolean): stri * that 404s. This same URL is what the readiness probe polls and what the ask * comment advertises, so there is one source of truth. */ -export function previewUrl(issueNumber: number): string { - return `https://pkg.pr.new/emdash@${fixBranch(issueNumber)}`; +export function previewUrl(issueNumber: number, previewPackage = "emdash"): string { + if ( + previewPackage === "" || + previewPackage.startsWith("/") || + previewPackage.endsWith("/") || + !PREVIEW_PACKAGE_CHARS.test(previewPackage) || + previewPackage.split("/").some((part) => part === "." || part === "..") + ) { + throw new Error(`invalid preview package: ${previewPackage}`); + } + return `https://pkg.pr.new/${previewPackage}@${fixBranch(issueNumber)}`; } /** The one-line install command posted in the ask comment. */ -export function previewInstallCommand(issueNumber: number): string { - return `npm i ${previewUrl(issueNumber)}`; +export function previewInstallCommand(issueNumber: number, previewPackage = "emdash"): string { + return `npm i ${previewUrl(issueNumber, previewPackage)}`; } /** diff --git a/infra/emdash-bot/.flue/lib/router.ts b/infra/emdash-bot/.flue/lib/router.ts index 579b28c6e8..533e9e7b3c 100644 --- a/infra/emdash-bot/.flue/lib/router.ts +++ b/infra/emdash-bot/.flue/lib/router.ts @@ -14,6 +14,7 @@ import { findTransition, KINDS, STATES, + transitionTarget, type Actor, type EventId, type Kind, @@ -218,7 +219,8 @@ export function resolve({ labels, event, arg, actor }: ResolveInput): Decision { if (!from) return { kind: "noop", reason: "item has conflicting state labels" }; const t = findTransition(from, event); if (!t) return { kind: "noop", reason: `no transition for ${from} + ${event}`, from }; - const toLabel = STATES[t.to].label; + const to = transitionTarget(t, currentKind(labels)); + const toLabel = STATES[to].label; const removeLabels = STATE_LABELS.filter((l) => l !== toLabel); // Entry from unmanaged or triage: ensure the kind label matches the verb. @@ -243,7 +245,7 @@ export function resolve({ labels, event, arg, actor }: ResolveInput): Decision { return { kind: "transition", from, - to: t.to, + to, action: t.action ?? null, addLabel: toLabel, addLabels, @@ -325,6 +327,7 @@ export interface AgentResult { reproduced?: boolean; rootCauseFound?: boolean; fixed?: boolean; + implemented?: boolean; verdict?: string; [key: string]: unknown; } @@ -365,6 +368,9 @@ export function outcomeFromResult({ if (effectiveMode === "fix") { return result.fixed === true && pushed === true ? "agent.fix_ready" : "agent.failed"; } + if (effectiveMode === "implement") { + return result.implemented === true && pushed === true ? "agent.fix_ready" : "agent.failed"; + } if (effectiveMode === "repro" && result.reproduced !== true) { return result.rootCauseFound === true ? "agent.diagnosed" : "agent.not_reproduced"; } diff --git a/infra/emdash-bot/.flue/lib/verification.ts b/infra/emdash-bot/.flue/lib/verification.ts new file mode 100644 index 0000000000..7eda320c76 --- /dev/null +++ b/infra/emdash-bot/.flue/lib/verification.ts @@ -0,0 +1,60 @@ +export interface VerificationRecord { + readonly name: string; + readonly command: string; + readonly exitCode: number; + readonly candidateTreeSha: string; +} + +const PIPE_OPERATOR = /\|/; +const STATUS_MASKING_SHELL_CONTROL = /[;&\r\n]/; +const LEADING_SHELL_NEGATION = /^\s*!/; + +export function assertVerificationCommand(command: string): void { + if (PIPE_OPERATOR.test(command)) { + throw new Error( + "verification commands cannot contain a pipeline or || fallback; run the check directly so its exit code is authoritative", + ); + } + if (STATUS_MASKING_SHELL_CONTROL.test(command)) { + throw new Error( + "verification commands cannot contain shell control operators that can replace the check's exit code", + ); + } + if (LEADING_SHELL_NEGATION.test(command)) { + throw new Error("verification commands cannot negate a check to replace its exit code"); + } +} + +export function passingVerificationRecords( + records: readonly VerificationRecord[], + candidateTreeSha?: string, +): VerificationRecord[] { + const latest = new Map(); + const commands = new Map(); + for (const record of records) { + const previousCommand = commands.get(record.name); + if (previousCommand !== undefined && previousCommand !== record.command) { + throw new Error(`verification check ${record.name} changed command between runs`); + } + commands.set(record.name, record.command); + latest.set(record.name, record); + } + if (latest.size === 0) throw new Error("run at least one verification check before publishing"); + const failed = [...latest.values()].filter((record) => record.exitCode !== 0); + if (failed.length > 0) { + throw new Error( + `verification checks are not passing: ${failed.map((record) => record.name).join(", ")}`, + ); + } + if (candidateTreeSha !== undefined) { + const stale = [...latest.values()].filter( + (record) => record.candidateTreeSha !== candidateTreeSha, + ); + if (stale.length > 0) { + throw new Error( + `candidate changed after verification checks: ${stale.map((record) => record.name).join(", ")}`, + ); + } + } + return [...latest.values()]; +} diff --git a/infra/emdash-bot/.flue/skills/fix/SKILL.md b/infra/emdash-bot/.flue/skills/fix/SKILL.md index 574e9f2735..ef20633607 100644 --- a/infra/emdash-bot/.flue/skills/fix/SKILL.md +++ b/infra/emdash-bot/.flue/skills/fix/SKILL.md @@ -7,21 +7,22 @@ description: Implement diagnose's proposed fix when verify says bug, the cause i You are here because a maintainer issued a **fix** directive, verify returned `bug`, diagnose pinned the cause with at least `medium` confidence, and diagnose rated the fix `mechanical` or `clear-best-option`. Diagnose handed you a **proposed fix** -- a concrete plan naming the file and the change. Implement that plan, prove it works, and leave the change verified. The hard reasoning is done; do not re-litigate the diagnosis unless reading the code convinces you it is wrong (then abandon -- see below). -**What your output is, and is not.** You are not merging and not opening a PR. You commit and push your change to the issue's `bot/fix-` candidate branch as the spine instructs; the push triggers a **preview build** the workflow posts to the issue; the reporter is asked to confirm it fixes _their_ case. **Only after the reporter confirms** does a draft PR open, and a maintainer reviews before anything reaches `main`. So the bar is "a correct, conventions-respecting change that makes the repro test pass" -- not "a perfect, unimprovable patch." A clear, test-backed fix is worth shipping for verification even when it is more than a one-liner. Equally: do not gold-plate, do not expand scope, do not refactor beyond the diagnosed bug. +**What your output is, and is not.** You are not merging and not opening a PR. The trusted `publish_candidate` tool publishes your change to the issue's `bot/fix-` candidate branch; that triggers a **preview build** the workflow posts to the issue. **Only after the reporter confirms** does a draft PR open, and a maintainer reviews before anything reaches `main`. So the bar is "a correct, conventions-respecting change that makes the repro test pass" -- not "a perfect, unimprovable patch." A clear, test-backed fix is worth shipping for verification even when it is more than a one-liner. Equally: do not gold-plate, do not expand scope, do not refactor beyond the diagnosed bug. ## Environment - **Edit in the VFS** with the `edit_file` / `write_file` tools; read surrounding code with `read_file` and `grep`. Every VFS edit is replayed onto the container checkout before each container command. -- **Run tests, lint, typecheck, and format in an attached container** -- none of the toolchain exists in the VFS. Attach once you are ready to verify, and do all `pnpm` and git work there. +- **Run final tests, lint, typecheck, and format checks through `run_check`** -- none of the toolchain exists in the VFS. Verification commands must not modify source files. Apply formatting with `edit_file`/`write_file`, then use a check-only formatter command. Use `exec` only for exploratory commands whose result is not a release gate. ## Do not -- No `git tag` and no PR creation. Push only the issue's `bot/fix-` branch, with `--force-with-lease`, exactly as the spine instructs -- the push capability rejects every other ref. The workflow owns the preview and the PR. +- No `git commit`, `git push`, `git tag`, or PR creation. `publish_candidate` owns the issue's candidate branch. The workflow owns the preview and the PR. - No GitHub writes. Read-only API GETs only. - No network beyond the clone, the proxy-signed GitHub API, and the npm registry. - No `pnpm publish` / `npm publish`. - No drive-by edits. Touch only the files the diagnosed bug and its test need. A problem in a nearby file is a human's -- scope discipline. - Do not modify Lingui catalogs (`packages/admin/src/locales/*/messages.po`); the extract workflow handles them on merge. +- Do not edit after final verification. Publication requires every latest named `run_check` result to match the exact candidate tree; rerun all required checks after any source change. ## Procedure @@ -39,12 +40,13 @@ You are here because a maintainer issued a **fix** directive, verify returned `b - `import.meta.env.DEV`, never `process.env.NODE_ENV`. - Migrations are forward-only and additive; register in `runner.ts` via `StaticMigrationProvider`. - Prefer additive changes. A breaking change needs an explicit changeset -- do not introduce one for an automated fix without compelling justification. -4. **Run the repro test (container).** It must now pass. If not, your fix is wrong or incomplete -- investigate, adjust, or abandon. Never weaken the test to make it pass. -5. **Run the affected package's suite (container).** `pnpm --filter test`. Read the output. New failures in tests you did not write are regressions -- fix them or abandon the whole change. Do not push regressions through. -6. **Typecheck (container).** `pnpm typecheck` for packages, `pnpm typecheck:demos` if a demo was involved. No new errors. -7. **Lint (container).** `pnpm lint:quick`. If the count looks off, snapshot with `pnpm lint:json | jq '.diagnostics | length'` -- a clean baseline stays clean. -8. **Format (container).** `pnpm format` (oxfmt, tabs). Do not bypass it. Format only the files you touched -- a repo-wide format reformats already-committed files and blows scope. +4. **Run the repro test with `run_check`.** It must now pass. If not, your fix is wrong or incomplete -- investigate, adjust, or abandon. Never weaken the test to make it pass. +5. **Run the affected package's suite with `run_check`.** `pnpm --filter test`. New failures in tests you did not write are regressions -- fix them or abandon the whole change. +6. **Typecheck with `run_check`.** `pnpm typecheck` for packages, `pnpm typecheck:demos` if a demo was involved. No new errors. +7. **Lint with `run_check`.** Run `pnpm lint:quick`; a clean baseline stays clean. +8. **Check formatting with `run_check`.** Apply any needed formatting with `edit_file`/`write_file`, then run `pnpm format:check` or the narrow check-only formatter command appropriate to the files you touched. Do not bulk-format unrelated files. 9. **Add a changeset when a published package changed.** Create the file under `.changeset/` (patch bump for a bug fix unless diagnosis says otherwise). Write it as release notes for someone upgrading -- lead with a verb, describe the observable effect, reference the issue -- not as a commit message. Include it in your fix commit. +10. **Publish with `publish_candidate`.** Do not reproduce its work with shell commands. Report `fixed: true` only after it succeeds. ## When to abandon diff --git a/infra/emdash-bot/.flue/skills/implement/SKILL.md b/infra/emdash-bot/.flue/skills/implement/SKILL.md new file mode 100644 index 0000000000..15c61ad77f --- /dev/null +++ b/infra/emdash-bot/.flue/skills/implement/SKILL.md @@ -0,0 +1,29 @@ +--- +name: implement +description: Implement a maintainer-directed EmDash enhancement or change without forcing it through bug-reproduction fields. Verify the change with authoritative checks and publish it through the trusted candidate publisher. +--- + +# Implement + +A maintainer explicitly asked you to build the issue's requested change. Treat the issue body and directive as the specification. This lane is for enhancements and directed changes; do not invent a bug verdict or describe an enhancement as reproduced. + +## Procedure + +1. Read `AGENTS.md` and the relevant implementation, tests, and contributor guidance before editing. +2. Resolve ambiguity from existing APIs, sibling code, and backwards-compatible behavior. If a missing decision would materially change the public contract, stop and report it instead of guessing. +3. Edit through `edit_file` and `write_file`. Keep the change scoped to the request. Do not modify `.github/workflows` or generated Lingui catalogs. +4. Add behavior-level tests where the change has testable behavior. For a directed bug fix, follow the repository's failing-test-first rule. +5. Use `run_check` for every final verification. At minimum run the focused test and the repository-prescribed lint/typecheck commands that apply. Verification commands must not modify source files; apply formatting with `edit_file`/`write_file`, then run a check-only formatter command. Give checks stable names when rerunning them; publication requires every latest named result to pass. +6. Add a changeset when a published package changes. Write it as user-facing release notes. +7. Call `publish_candidate` with a conventional commit message. The trusted Worker owns Git objects and the `bot/fix-` ref; never run `git commit`, `git push`, or create a PR yourself. +8. Call `report_implementation` exactly once. Set `implemented: true` only after publication succeeds. Summarize the observable change and verification, not a bug verdict. + +## Boundaries + +- No direct GitHub writes, tags, package publication, or workflow edits. +- No source-modifying commands, output pipelines, or `|| true` on final checks. `run_check` rejects candidate mutations and status-masking commands. +- No edits after final verification. Publication requires every latest named check to match the exact candidate tree; rerun all required checks after any source change. +- No drive-by refactors or broad cleanup. +- Do not weaken a test to make it pass. + +The candidate preview and draft-PR lifecycle remain owned by the orchestrator. diff --git a/infra/emdash-bot/.flue/skills/investigate/SKILL.md b/infra/emdash-bot/.flue/skills/investigate/SKILL.md index cd7685a822..1c5c96009c 100644 --- a/infra/emdash-bot/.flue/skills/investigate/SKILL.md +++ b/infra/emdash-bot/.flue/skills/investigate/SKILL.md @@ -93,7 +93,7 @@ Run **`fix`** only when **all** hold: Any other combination: stop after verify. Post the diagnosis (proposed fix, or the options for a design decision) and the verify reasoning; a human takes it from there. -**The fix loop does not open a PR.** Fix produces a verified change, committed and pushed to the issue's `bot/fix-` candidate branch -- the only ref the push capability can update. The push triggers a **preview build** the workflow posts to the issue, and the reporter is asked to confirm it resolves _their_ case. **Only after the reporter confirms** does a draft PR open (carrying the repro test, referencing the issue). Reporter denial or silence reaps the branch. Nothing you do here lands on `main`; a maintainer reviews the eventual PR. +**The fix loop does not open a PR.** Fix produces a verified change and hands it to `publish_candidate`, which updates only the issue's `bot/fix-` candidate branch. The update triggers a **preview build** the workflow posts to the issue, and the reporter is asked to confirm it resolves _their_ case. **Only after the reporter confirms** does a draft PR open (carrying the repro test, referencing the issue). Reporter denial or silence reaps the branch. Nothing you do here lands on `main`; a maintainer reviews the eventual PR. ## Output diff --git a/infra/emdash-bot/BOT_STATE_MACHINE.md b/infra/emdash-bot/BOT_STATE_MACHINE.md index 512f49f348..a937ff6e32 100644 --- a/infra/emdash-bot/BOT_STATE_MACHINE.md +++ b/infra/emdash-bot/BOT_STATE_MACHINE.md @@ -51,7 +51,7 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. | `agent.by_design` | agent result | system | — | Agent verified the behaviour as intended. | | `agent.reproduced` | agent result | system | — | Reproduced, but the fix needs a human decision. | | `agent.diagnosed` | agent result | system | — | Root cause identified without a confirming reproduction. | -| `agent.fix_ready` | agent result | system | — | Reproduced and fixed; a verified change is staged on bot/fix-. | +| `agent.fix_ready` | agent result | system | — | A verified candidate change is published on bot/fix-. | | `agent.needs_info` | agent result | system | — | Investigation is blocked on information only the reporter can supply. | | `agent.failed` | agent result | system | — | Agent run errored or produced no usable result. | | `pr.opened` | pr lifecycle | system | — | A bot PR was opened for this item. | @@ -59,7 +59,7 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. | `pr.closed` | pr lifecycle | system | — | The bot PR was closed without merging. | | `pr.changes_requested` | pr lifecycle | system | — | A reviewer requested changes (review sub-state). | | `pr.approved` | pr lifecycle | system | — | A reviewer approved the PR (review sub-state). | -| `preview.ready` | preview | system | — | The preview deploy for the candidate fix is live; link ready to post. | +| `preview.ready` | preview | system | — | The preview deploy for the candidate change is live; link ready to post. | | `preview.failed` | preview | system | — | The preview deploy failed to build. | | `expire` | timer | system | — | The reporter-confirmation window elapsed without a reply. | @@ -68,10 +68,10 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. | From | Event | To | Action | | --- | --- | --- | --- | | `unmanaged` | `repro` | `working` | `investigate.repro` | -| `unmanaged` | `implement` | `working` | `investigate.implement` | +| `unmanaged` | `implement` | `fixing` | `investigate.implement` | | `unmanaged` | `decline` | `declined` | — | | `triage` | `repro` | `working` | `investigate.repro` | -| `triage` | `implement` | `working` | `investigate.implement` | +| `triage` | `implement` | `fixing` | `investigate.implement` | | `triage` | `decline` | `declined` | — | | `working` | `agent.skipped` | `blocked` | — | | `working` | `agent.not_reproduced` | `blocked` | — | @@ -79,7 +79,7 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. | `working` | `agent.reproduced` | `blocked` | — | | `working` | `agent.fix_ready` | `awaiting_feedback` | — | | `working` | `agent.failed` | `failed` | — | -| `blocked` | `implement` | `working` | `investigate.implement` | +| `blocked` | `implement` | `fixing` | `investigate.implement` | | `blocked` | `repro` | `working` | `investigate.repro` | | `blocked` | `retry` | `working` | `investigate.repro` | | `blocked` | `decline` | `declined` | — | @@ -114,7 +114,7 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. | `done` | `reopen` | `triage` | — | | `declined` | `reopen` | `triage` | — | | `failed` | `retry` | `working` | `investigate.repro` | -| `failed` | `implement` | `working` | `investigate.implement` | +| `failed` | `implement` | `fixing` | `investigate.implement` | | `failed` | `repro` | `working` | `investigate.repro` | | `failed` | `decline` | `declined` | — | | `unmanaged` | `investigate` | `investigating` | `investigate.diagnose` | @@ -147,10 +147,10 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. | `fixing` | `agent.by_design` | `blocked` | — | | `fixing` | `agent.skipped` | `blocked` | — | | `preview_building` | `preview.ready` | `awaiting_reporter` | — | -| `preview_building` | `preview.failed` | `reproduced` | — | +| `preview_building` | `preview.failed` | default: `reproduced`; `enhancement`: `blocked`; `task`: `blocked` | — | | `awaiting_reporter` | `confirm` | `in_review` | `openDraftPr` | -| `awaiting_reporter` | `reject` | `reproduced` | `reapBranch` | -| `awaiting_reporter` | `expire` | `reproduced` | `reapBranch` | +| `awaiting_reporter` | `reject` | default: `reproduced`; `enhancement`: `blocked`; `task`: `blocked` | `reapBranch` | +| `awaiting_reporter` | `expire` | default: `reproduced`; `enhancement`: `blocked`; `task`: `blocked` | `reapBranch` | | `awaiting_reporter` | `take_over` | `human_owned` | — | | `awaiting_reporter` | `decline` | `declined` | `reapBranch` | | `investigating` | `reset` | `triage` | — | @@ -167,10 +167,10 @@ Entry state: `unmanaged`. Kinds: `bug`, `enhancement`, `task`. stateDiagram-v2 [*] --> unmanaged unmanaged --> working: repro / investigate.repro - unmanaged --> working: implement / investigate.implement + unmanaged --> fixing: implement / investigate.implement unmanaged --> declined: decline triage --> working: repro / investigate.repro - triage --> working: implement / investigate.implement + triage --> fixing: implement / investigate.implement triage --> declined: decline working --> blocked: agent.skipped working --> blocked: agent.not_reproduced @@ -178,7 +178,7 @@ stateDiagram-v2 working --> blocked: agent.reproduced working --> awaiting_feedback: agent.fix_ready working --> failed: agent.failed - blocked --> working: implement / investigate.implement + blocked --> fixing: implement / investigate.implement blocked --> working: repro / investigate.repro blocked --> working: retry / investigate.repro blocked --> declined: decline @@ -213,7 +213,7 @@ stateDiagram-v2 done --> triage: reopen declined --> triage: reopen failed --> working: retry / investigate.repro - failed --> working: implement / investigate.implement + failed --> fixing: implement / investigate.implement failed --> working: repro / investigate.repro failed --> declined: decline unmanaged --> investigating: investigate / investigate.diagnose @@ -246,10 +246,13 @@ stateDiagram-v2 fixing --> blocked: agent.by_design fixing --> blocked: agent.skipped preview_building --> awaiting_reporter: preview.ready - preview_building --> reproduced: preview.failed + preview_building --> reproduced: preview.failed [default] + preview_building --> blocked: preview.failed [enhancement, task] awaiting_reporter --> in_review: confirm / openDraftPr - awaiting_reporter --> reproduced: reject / reapBranch - awaiting_reporter --> reproduced: expire / reapBranch + awaiting_reporter --> reproduced: reject [default] / reapBranch + awaiting_reporter --> blocked: reject [enhancement, task] / reapBranch + awaiting_reporter --> reproduced: expire [default] / reapBranch + awaiting_reporter --> blocked: expire [enhancement, task] / reapBranch awaiting_reporter --> human_owned: take_over awaiting_reporter --> declined: decline / reapBranch investigating --> triage: reset diff --git a/infra/emdash-bot/evals/README.md b/infra/emdash-bot/evals/README.md index 40b52d10be..867f4d34a1 100644 --- a/infra/emdash-bot/evals/README.md +++ b/infra/emdash-bot/evals/README.md @@ -99,3 +99,27 @@ fixed, `pre_fix` (the fixing merge commit + its parents) so the run checks out the pre-fix commit; an unfixed confirmed bug (`fixing_pr: null`, e.g. #1193) runs at `main`, where the bug is still live. See `dataset.md` for the human-readable rationale and per-case notes. + +## Implementation delivery smoke + +The diagnose corpus does not exercise writes. Before cutting over a publisher, +run the full implementation smoke against a staging worker configured for a +disposable repository and issue containing a deterministic fixture-edit task. +Set the staging Worker's `PREVIEW_PACKAGE` variable to the pkg.pr.new package +path that repository's preview workflow publishes; this keeps its readiness +probe isolated from production previews. + +```sh +ALLOW_GITHUB_WRITES=1 \ +WORKER_URL=https://emdash-bot-staging.example.workers.dev \ +ADMIN_TOKEN=... GH_TOKEN=... REPO=owner/disposable-repo \ +ISSUE_NUMBER=123 SMOKE_ACTOR=maintainer-login \ +DIRECTIVE='Update the implementation-canary fixture as this issue specifies' \ +pnpm evals:implementation +``` + +The test sends signed webhook commands through the real orchestrator and +requires all of these outcomes: candidate branch creation, a changed remote +SHA, preview readiness (`bot:awaiting-reporter`), reporter confirmation, and a +draft PR. It deliberately leaves the draft PR and branch for inspection. It +refuses `emdash-cms/emdash` unless `ALLOW_PRODUCTION_REPO=1` is also set. diff --git a/infra/emdash-bot/evals/bin/implementation-smoke.ts b/infra/emdash-bot/evals/bin/implementation-smoke.ts new file mode 100644 index 0000000000..6a942c4f17 --- /dev/null +++ b/infra/emdash-bot/evals/bin/implementation-smoke.ts @@ -0,0 +1,173 @@ +// Live cutover smoke for the complete implementation delivery path. +// Use only with a staging worker configured for a disposable repository. + +import { createHmac, randomUUID } from "node:crypto"; + +const DEFAULT_TIMEOUT_MS = 30 * 60 * 1000; +const DEFAULT_POLL_MS = 15 * 1000; +const TRAILING_SLASH = /\/$/; + +function required(name: string): string { + const value = process.env[name]?.trim(); + if (!value) fail(`missing required env ${name}`); + return value; +} + +function fail(message: string): never { + console.error(`implementation smoke failed: ${message}`); + process.exit(1); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +interface Issue { + number: number; + state: string; + user?: { login?: string }; + labels?: Array<{ name?: string }>; +} + +interface PullRequest { + number: number; + draft: boolean; + html_url: string; +} + +async function github(path: string, token: string): Promise { + const response = await fetch(`https://api.github.com${path}`, { + headers: { + authorization: `Bearer ${token}`, + accept: "application/vnd.github+json", + "user-agent": "emdash-bot-implementation-smoke", + "x-github-api-version": "2022-11-28", + }, + }); + if (!response.ok) + fail(`GitHub ${path} returned ${response.status}: ${(await response.text()).slice(0, 500)}`); + return response.json(); +} + +async function branchSha(owner: string, repo: string, issueNumber: number, token: string) { + const response = await fetch( + `https://api.github.com/repos/${owner}/${repo}/branches/${encodeURIComponent(`bot/fix-${issueNumber}`)}`, + { + headers: { + authorization: `Bearer ${token}`, + accept: "application/vnd.github+json", + "user-agent": "emdash-bot-implementation-smoke", + }, + }, + ); + if (response.status === 404) return null; + if (!response.ok) fail(`branch lookup returned ${response.status}`); + const body = await response.json<{ commit?: { sha?: string } }>(); + return body.commit?.sha ?? null; +} + +async function postCommand(input: { + workerUrl: string; + secret: string; + issue: Issue; + body: string; + actor: string; +}): Promise { + const payload = JSON.stringify({ + action: "created", + issue: { + number: input.issue.number, + user: input.issue.user, + labels: input.issue.labels, + }, + comment: { + body: input.body, + author_association: "MEMBER", + user: { login: input.actor }, + }, + sender: { login: input.actor }, + }); + const signature = createHmac("sha256", input.secret).update(payload).digest("hex"); + const response = await fetch(`${input.workerUrl.replace(TRAILING_SLASH, "")}/webhook/github`, { + method: "POST", + headers: { + "content-type": "application/json", + "x-github-event": "issue_comment", + "x-github-delivery": `implementation-smoke-${randomUUID()}`, + "x-hub-signature-256": `sha256=${signature}`, + }, + body: payload, + }); + if (!response.ok) + fail(`worker webhook returned ${response.status}: ${(await response.text()).slice(0, 500)}`); +} + +async function waitFor( + description: string, + timeoutMs: number, + pollMs: number, + probe: () => Promise, +): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + const value = await probe(); + if (value !== null) return value; + if (Date.now() >= deadline) fail(`timed out waiting for ${description}`); + await sleep(pollMs); + } +} + +async function main(): Promise { + if (process.env.ALLOW_GITHUB_WRITES !== "1") { + fail("set ALLOW_GITHUB_WRITES=1; this test creates a branch, comments, labels, and a draft PR"); + } + const workerUrl = required("WORKER_URL"); + const secret = required("ADMIN_TOKEN"); + const githubToken = required("GH_TOKEN"); + const directive = required("DIRECTIVE"); + const actor = required("SMOKE_ACTOR"); + const issueNumber = Number(required("ISSUE_NUMBER")); + if (!Number.isSafeInteger(issueNumber) || issueNumber <= 0) fail("ISSUE_NUMBER must be positive"); + const [owner, repo] = required("REPO").split("/"); + if (!owner || !repo) fail("REPO must be owner/name"); + if (`${owner}/${repo}` === "emdash-cms/emdash" && process.env.ALLOW_PRODUCTION_REPO !== "1") { + fail( + "refusing production repository; use a disposable staging repo or set ALLOW_PRODUCTION_REPO=1", + ); + } + const timeoutMs = Number(process.env.TIMEOUT_MS ?? DEFAULT_TIMEOUT_MS); + const pollMs = Number(process.env.POLL_MS ?? DEFAULT_POLL_MS); + const issue = await github(`/repos/${owner}/${repo}/issues/${issueNumber}`, githubToken); + if (issue.state !== "open") fail(`issue #${issueNumber} is not open`); + const before = await branchSha(owner, repo, issueNumber, githubToken); + + console.log(`dispatching implementation smoke for ${owner}/${repo}#${issueNumber}`); + await postCommand({ workerUrl, secret, issue, body: `@emdashbot implement ${directive}`, actor }); + + const published = await waitFor("candidate branch publication", timeoutMs, pollMs, async () => { + const sha = await branchSha(owner, repo, issueNumber, githubToken); + return sha && sha !== before ? sha : null; + }); + console.log(`candidate published at ${published}`); + + await waitFor("preview-ready state", timeoutMs, pollMs, async () => { + const current = await github( + `/repos/${owner}/${repo}/issues/${issueNumber}`, + githubToken, + ); + return current.labels?.some((label) => label.name === "bot:awaiting-reporter") ? true : null; + }); + console.log("preview published and reporter confirmation requested"); + + await postCommand({ workerUrl, secret, issue, body: "@emdashbot confirm", actor }); + const pull = await waitFor("draft pull request", timeoutMs, pollMs, async () => { + const pulls = await github( + `/repos/${owner}/${repo}/pulls?state=open&head=${encodeURIComponent(`${owner}:bot/fix-${issueNumber}`)}`, + githubToken, + ); + return pulls.find((candidate) => candidate.draft) ?? null; + }); + console.log(`implementation smoke passed: ${pull.html_url}`); +} + +await main(); diff --git a/infra/emdash-bot/evals/src/client.ts b/infra/emdash-bot/evals/src/client.ts index 098b50c214..4efb2dd1b8 100644 --- a/infra/emdash-bot/evals/src/client.ts +++ b/infra/emdash-bot/evals/src/client.ts @@ -131,12 +131,14 @@ export async function waitForResult( function looksLikeReported(value: unknown): value is ReportedResult { if (!isRecord(value)) return false; if (typeof value.ok !== "boolean" || typeof value.pushed !== "boolean") return false; + if (typeof value.runId !== "string" || !Array.isArray(value.verification)) return false; + if (value.publication !== null && !isRecord(value.publication)) return false; return isRecord(value.result) && typeof value.result.summary === "string"; } /** * Find the reported investigation result anywhere in a conversation snapshot. - * The agent emits `{ result, ok, pushed }` via both a data writer and the + * The agent emits the reported result via both a data writer and the * `report_result` tool output; rather than couple to Flue's exact snapshot * envelope, this scans for that payload shape and returns the last one emitted * (the final report). Stringified JSON payloads are parsed and searched too. diff --git a/infra/emdash-bot/evals/src/types.ts b/infra/emdash-bot/evals/src/types.ts index 5052f08205..6a8639ad49 100644 --- a/infra/emdash-bot/evals/src/types.ts +++ b/infra/emdash-bot/evals/src/types.ts @@ -39,12 +39,28 @@ export interface Dataset { /** * The investigate agent's structured result, reported via `report_result`. - * Mirrors the agent's `reportedResultSchema` ({ result, ok, pushed }). + * Mirrors the agent's `reportedResultSchema`. */ export interface ReportedResult { readonly result: AgentResult; readonly ok: boolean; readonly pushed: boolean; + readonly runId: string; + readonly publication: CandidatePublication | null; + readonly verification: readonly VerificationRecord[]; +} + +export interface CandidatePublication { + readonly branch: string; + readonly commitSha: string; + readonly files: readonly string[]; +} + +export interface VerificationRecord { + readonly name: string; + readonly command: string; + readonly exitCode: number; + readonly candidateTreeSha: string; } export interface AgentResult { @@ -52,8 +68,10 @@ export interface AgentResult { readonly reproduced?: boolean; readonly rootCauseFound?: boolean; readonly fixed?: boolean; + readonly implemented?: boolean; readonly verdict?: string; readonly summary?: string; + readonly failureStage?: "workspace" | "verification" | "publication" | "reporting"; readonly screenshots?: readonly unknown[]; } diff --git a/infra/emdash-bot/package.json b/infra/emdash-bot/package.json index 02d2419dc9..13795c333a 100644 --- a/infra/emdash-bot/package.json +++ b/infra/emdash-bot/package.json @@ -11,6 +11,7 @@ "cf-typegen": "wrangler types", "bot:generate": "node --experimental-strip-types scripts/generate-machine.ts", "evals": "node --experimental-strip-types evals/bin/run.ts", + "evals:implementation": "node --experimental-strip-types evals/bin/implementation-smoke.ts", "pretypecheck": "wrangler types", "typecheck": "tsc --noEmit", "test": "pnpm test:unit && pnpm test:workers", diff --git a/infra/emdash-bot/scripts/machine-artifacts.ts b/infra/emdash-bot/scripts/machine-artifacts.ts index 3d473e0075..35d42c4316 100644 --- a/infra/emdash-bot/scripts/machine-artifacts.ts +++ b/infra/emdash-bot/scripts/machine-artifacts.ts @@ -16,6 +16,7 @@ import { machineSnapshot, STATES, TRANSITIONS, + transitionTargets, } from "../.flue/lib/machine.ts"; export function renderMachineJson(): string { @@ -73,7 +74,7 @@ function eventsTable(): string { function transitionsTable(): string { const rows = TRANSITIONS.map( (t) => - `| ${code(t.from)} | ${code(t.event)} | ${code(t.to)} | ${t.action ? code(t.action) : "—"} |`, + `| ${code(t.from)} | ${code(t.event)} | ${transitionDestination(t)} | ${t.action ? code(t.action) : "—"} |`, ); return [ "## Transitions", @@ -84,9 +85,29 @@ function transitionsTable(): string { ].join("\n"); } +function transitionDestination(transition: (typeof TRANSITIONS)[number]): string { + const overrides = Object.entries(transition.toByKind ?? {}); + if (overrides.length === 0) return code(transition.to); + return [ + `default: ${code(transition.to)}`, + ...overrides.map(([kind, target]) => `${code(kind)}: ${code(target)}`), + ].join("; "); +} + function diagram(): string { - const edges = TRANSITIONS.map( - (t) => ` ${t.from} --> ${t.to}: ${t.event}${t.action ? ` / ${t.action}` : ""}`, + const edges = TRANSITIONS.flatMap((transition) => + transitionTargets(transition).map((target) => { + const kinds = Object.entries(transition.toByKind ?? {}) + .filter(([, kindTarget]) => kindTarget === target) + .map(([kind]) => kind); + const qualifier = + target === transition.to + ? transition.toByKind + ? " [default]" + : "" + : ` [${kinds.join(", ")}]`; + return ` ${transition.from} --> ${target}: ${transition.event}${qualifier}${transition.action ? ` / ${transition.action}` : ""}`; + }), ); return [ "## Diagram", diff --git a/infra/emdash-bot/tests/integration/orchestrator.test.ts b/infra/emdash-bot/tests/integration/orchestrator.test.ts index 03782504c7..b9430c2735 100644 --- a/infra/emdash-bot/tests/integration/orchestrator.test.ts +++ b/infra/emdash-bot/tests/integration/orchestrator.test.ts @@ -26,6 +26,7 @@ import type { NormalizedEvent } from "../../.flue/lib/orchestrator.js"; interface TestEnv { Orchestrator: Env["Orchestrator"]; GITHUB_APP_PRIVATE_KEY: string; + PREVIEW_PACKAGE: string; } const testEnv = env as unknown as TestEnv; @@ -48,11 +49,17 @@ function makeEvent(overrides: Partial = {}): NormalizedEvent { }; } +function parseJsonBody(body: unknown): unknown { + if (typeof body !== "string") throw new Error("expected a string request body"); + return JSON.parse(body); +} + describe("OrchestratorDO (workers-pool)", () => { // The credential-injecting tests below mutate shared env and global fetch; // reset both after every test so nothing leaks into a later case. afterEach(() => { testEnv.GITHUB_APP_PRIVATE_KEY = ""; + testEnv.PREVIEW_PACKAGE = "emdash"; vi.unstubAllGlobals(); }); @@ -76,7 +83,7 @@ describe("OrchestratorDO (workers-pool)", () => { expect(outcome.kind).toBe("transition"); const persisted = await stub.getPersistedState(); - expect(persisted.state).toBe("working"); + expect(persisted.state).toBe("fixing"); // `implement` from unmanaged is an entry transition with default kind. // machine.ts's implement event sets defaultKind: "enhancement" // (verified separately in router tests). @@ -94,7 +101,7 @@ describe("OrchestratorDO (workers-pool)", () => { expect(entry.event).toBe("implement"); expect(entry.actor).toBe("maintainer"); expect(entry.from).toBe("unmanaged"); - expect(entry.to).toBe("working"); + expect(entry.to).toBe("fixing"); expect(entry.deliveryId).toBe("delivery-abc"); expect(typeof entry.t).toBe("number"); }); @@ -153,12 +160,12 @@ describe("OrchestratorDO (workers-pool)", () => { test("applyAgentResult commits the transition before clearing run markers", async () => { const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); await stub.event(makeEvent()); - await stub.debugSetStaleRun("active-run", Date.now()); + await stub.debugSetStaleRun("active-run", Date.now(), undefined, "implement"); const outcome = await stub.applyAgentResult({ runId: "active-run", - result: { reproduced: true, fixed: false, summary: "The issue reproduces." }, - pushed: false, + result: { implemented: true, summary: "Implemented the requested change." }, + pushed: true, ok: true, }); expect(outcome.kind).toBe("transition"); @@ -177,13 +184,71 @@ describe("OrchestratorDO (workers-pool)", () => { const outcome = await stub.applyAgentResult({ runId: "implement-run", - result: { fixed: true, summary: "Implemented the requested change." }, + result: { implemented: true, summary: "Implemented the requested change." }, pushed: true, ok: true, }); expect(outcome.kind).toBe("transition"); - expect((await stub.getPersistedState()).state).toBe("awaiting_feedback"); + expect((await stub.getPersistedState()).state).toBe("preview_building"); + }); + + test("a rejected implementation returns to a state where implement can be retried", async () => { + const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); + await stub.event(makeEvent()); + await stub.debugSetStaleRun("implement-run", Date.now(), undefined, "implement"); + await stub.applyAgentResult({ + runId: "implement-run", + result: { implemented: true, summary: "Implemented the requested change." }, + pushed: true, + ok: true, + }); + await stub.event( + makeEvent({ event: "preview.ready", arg: null, actor: "system", anchorNumber: 42 }), + ); + + const rejected = await stub.event( + makeEvent({ event: "reject", arg: "needs revision", actor: "reporter", anchorNumber: 42 }), + ); + + expect(rejected.kind).toBe("transition"); + expect((await stub.getPersistedState()).state).toBe("blocked"); + const retry = await stub.event( + makeEvent({ event: "implement", arg: "apply the feedback", anchorNumber: 42 }), + ); + expect(retry.kind).toBe("transition"); + if (retry.kind === "transition") expect(retry.decision.action).toBe("investigate.implement"); + }); + + test("a failed run comment carries its stage and durable run id", async () => { + const calls: string[] = []; + const comments: string[] = []; + testEnv.GITHUB_APP_PRIVATE_KEY = "test-key-present"; + vi.stubGlobal("fetch", githubCallRecorder(calls, 201, comments)); + const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); + await stub.debugSetTokenCache("cached-token", Date.now() + 60 * 60 * 1000); + await stub.debugPrimeFixing(42); + await stub.debugSetStaleRun( + "implement-run-123", + Date.now(), + "investigate-42-implement-run-123", + "implement", + ); + + await stub.applyAgentResult({ + runId: "implement-run-123", + result: { + implemented: false, + failureStage: "verification", + summary: "The required typecheck failed.", + }, + pushed: false, + ok: true, + }); + + expect((await stub.getPersistedState()).state).toBe("failed"); + expect(comments.at(-1)).toContain("Failed stage: `verification`"); + expect(comments.at(-1)).toContain("Run: `implement-run-123`"); }); test("tick recovers a stale run", async () => { @@ -508,7 +573,7 @@ describe("OrchestratorDO (workers-pool)", () => { ]); const persisted = await stub.getPersistedState(); - expect(persisted.state).toBe("working"); + expect(persisted.state).toBe("fixing"); const log = await stub.getEventLog(); expect(log.length).toBe(1); }); @@ -567,6 +632,19 @@ describe("OrchestratorDO (workers-pool)", () => { expect((await stub.getPersistedState()).state).toBe("reproduced"); }); + test("invalid preview package configuration fails instead of retrying forever", async () => { + testEnv.PREVIEW_PACKAGE = "../invalid"; + const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); + await driveToPreviewBuilding(stub, 42); + + await stub.debugSetPreviewPoll(Date.now() + 60_000, Date.now() - 1_000); + const tick = await stub.tick(); + + expect(tick.previewPoll).toBe("failed"); + expect(tick.recoveryError).toBeNull(); + expect((await stub.getPersistedState()).state).toBe("reproduced"); + }); + test("preview poll holds off before the next scheduled probe", async () => { const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); await driveToPreviewBuilding(stub, 42); @@ -584,6 +662,7 @@ describe("OrchestratorDO (workers-pool)", () => { function githubCallRecorder( calls: string[], commentStatus: number, + comments: string[] = [], ): (input: Parameters[0], init?: Parameters[1]) => Promise { return (input, init) => { const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; @@ -604,6 +683,15 @@ describe("OrchestratorDO (workers-pool)", () => { } if (method === "POST" && url.endsWith("/comments")) { calls.push("comment"); + const body = parseJsonBody(init?.body); + if ( + typeof body === "object" && + body !== null && + "body" in body && + typeof body.body === "string" + ) { + comments.push(body.body); + } return Promise.resolve(new Response("{}", { status: commentStatus })); } if (url.includes("/labels")) { @@ -651,6 +739,57 @@ describe("OrchestratorDO (workers-pool)", () => { expect(await stub.getPendingSideEffectCount()).toBe(1); }); + test("draft PR titles distinguish bug fixes from directed implementations", async () => { + const pullRequests: unknown[] = []; + let pullNumber = 100; + testEnv.GITHUB_APP_PRIVATE_KEY = "test-key-present"; + vi.stubGlobal( + "fetch", + (input: Parameters[0], init?: Parameters[1]) => { + const url = + typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + const method = (init?.method ?? "GET").toUpperCase(); + if (method === "GET" && url.includes("/pulls?")) { + return Promise.resolve( + new Response("[]", { headers: { "content-type": "application/json" } }), + ); + } + if (method === "POST" && url.endsWith("/pulls")) { + pullRequests.push(parseJsonBody(init?.body)); + pullNumber += 1; + return Promise.resolve( + new Response( + JSON.stringify({ number: pullNumber, html_url: "https://example.test/pr" }), + { + status: 201, + headers: { "content-type": "application/json" }, + }, + ), + ); + } + return Promise.resolve(new Response("{}", { status: 200 })); + }, + ); + + for (const [anchorNumber, kind] of [ + [42, "bug"], + [43, "enhancement"], + ] as const) { + const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); + await stub.debugSetTokenCache("cached-token", Date.now() + 60 * 60 * 1000); + await stub.debugPrimePreviewBuilding(anchorNumber, "Candidate notes.", kind); + await stub.event( + makeEvent({ event: "preview.ready", arg: null, actor: "system", anchorNumber }), + ); + await stub.event(makeEvent({ event: "confirm", arg: null, actor: "reporter", anchorNumber })); + } + + expect(pullRequests).toMatchObject([ + { title: "Fix #42", draft: true }, + { title: "Implement #43", draft: true }, + ]); + }); + test("cleanupOnClose is a no-op without live credentials", async () => { const stub = testEnv.Orchestrator.getByName(uniqueIssueName()); const outcome = await stub.cleanupOnClose(42); diff --git a/infra/emdash-bot/tests/unit/candidate-publisher.test.ts b/infra/emdash-bot/tests/unit/candidate-publisher.test.ts new file mode 100644 index 0000000000..12f566ed93 --- /dev/null +++ b/infra/emdash-bot/tests/unit/candidate-publisher.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, test, vi } from "vitest"; + +import { + publishCandidate, + requireCandidatePublication, + type CandidateGitHub, +} from "../../.flue/lib/candidate-publisher.js"; + +function fakeGitHub(overrides: Partial = {}): CandidateGitHub { + return { + getBranchSha: vi.fn(async () => null), + getCommit: vi.fn(async () => ({ treeSha: "base-tree", message: "base" })), + createBlob: vi.fn(async (_content: Uint8Array) => "blob-sha"), + createTree: vi.fn(async () => "tree-sha"), + createCommit: vi.fn(async () => "commit-sha"), + createBranch: vi.fn(async () => {}), + updateBranch: vi.fn(async () => {}), + ...overrides, + }; +} + +describe("publishCandidate", () => { + test("requires a completed publication before a model can claim delivery", () => { + expect(() => requireCandidatePublication(true, null)).toThrow(/publish_candidate/); + expect(() => requireCandidatePublication(false, null)).not.toThrow(); + expect(() => + requireCandidatePublication(true, { + branch: "bot/fix-1", + commitSha: "sha", + files: ["x.ts"], + }), + ).not.toThrow(); + }); + + test("creates and verifies an issue-scoped candidate branch", async () => { + const getBranchSha = vi + .fn<() => Promise>() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce("commit-sha"); + const github = fakeGitHub({ getBranchSha }); + + const result = await publishCandidate( + { + branch: "bot/fix-2299", + runId: "run-123", + commitMessage: "Fix the CLI", + expectedPreviousSha: null, + snapshot: { + baseCommitSha: "base-commit", + treeSha: "tree-sha", + changes: [ + { + path: "src/index.ts", + mode: "100644", + content: new TextEncoder().encode("export {};\n"), + }, + ], + }, + }, + github, + ); + + expect(result).toEqual({ + branch: "bot/fix-2299", + commitSha: "commit-sha", + files: ["src/index.ts"], + }); + expect(github.createTree).toHaveBeenCalledWith("base-tree", [ + { path: "src/index.ts", mode: "100644", type: "blob", sha: "blob-sha" }, + ]); + expect(github.createCommit).toHaveBeenCalledWith( + expect.stringContaining("EmDash-Run: run-123"), + "tree-sha", + "base-commit", + ); + expect(github.createBranch).toHaveBeenCalledWith("bot/fix-2299", "commit-sha"); + }); + + test("refuses to overwrite a branch that changed after the run started", async () => { + const github = fakeGitHub({ getBranchSha: vi.fn(async () => "someone-elses-commit") }); + + await expect( + publishCandidate( + { + branch: "bot/fix-42", + runId: "run-42", + commitMessage: "Change", + expectedPreviousSha: "expected", + snapshot: { + baseCommitSha: "base", + treeSha: "tree-sha", + changes: [{ path: "x.ts", mode: "100644", content: new Uint8Array([1]) }], + }, + }, + github, + ), + ).rejects.toThrow(/changed since this run started/); + expect(github.createBlob).not.toHaveBeenCalled(); + }); + + test("treats a repeated run marker as an idempotent successful publication", async () => { + const github = fakeGitHub({ + getBranchSha: vi.fn(async () => "already-published"), + getCommit: vi.fn(async () => ({ + treeSha: "tree", + message: "Change\n\nEmDash-Run: run-42", + })), + }); + + await expect( + publishCandidate( + { + branch: "bot/fix-42", + runId: "run-42", + commitMessage: "Change", + expectedPreviousSha: null, + snapshot: { + baseCommitSha: "base", + treeSha: "tree-sha", + changes: [{ path: "x.ts", mode: "100644", content: new Uint8Array([1]) }], + }, + }, + github, + ), + ).resolves.toEqual({ + branch: "bot/fix-42", + commitSha: "already-published", + files: ["x.ts"], + }); + expect(github.createBlob).not.toHaveBeenCalled(); + }); + + test("represents deletions as null tree entries", async () => { + const getBranchSha = vi + .fn<() => Promise>() + .mockResolvedValueOnce("old") + .mockResolvedValueOnce("old") + .mockResolvedValueOnce("commit-sha"); + const github = fakeGitHub({ getBranchSha }); + + await publishCandidate( + { + branch: "bot/fix-1", + runId: "run-1", + commitMessage: "Delete obsolete file", + expectedPreviousSha: "old", + snapshot: { + baseCommitSha: "old", + treeSha: "tree-sha", + changes: [{ path: "obsolete.ts", mode: "100644", content: null }], + }, + }, + github, + ); + + expect(github.createBlob).not.toHaveBeenCalled(); + expect(github.createTree).toHaveBeenCalledWith("base-tree", [ + { path: "obsolete.ts", mode: "100644", type: "blob", sha: null }, + ]); + expect(github.updateBranch).toHaveBeenCalledWith("bot/fix-1", "commit-sha"); + }); + + test("refuses a GitHub tree that differs from the verified candidate", async () => { + const github = fakeGitHub(); + + await expect( + publishCandidate( + { + branch: "bot/fix-1", + runId: "run-1", + commitMessage: "Change candidate", + expectedPreviousSha: null, + snapshot: { + baseCommitSha: "base", + treeSha: "verified-tree", + changes: [{ path: "x.ts", mode: "100644", content: new Uint8Array([1]) }], + }, + }, + github, + ), + ).rejects.toThrow(/tree.*verified candidate/); + expect(github.createCommit).not.toHaveBeenCalled(); + }); + + test("parents an update to the expected branch head so the ref update can be fast-forward-only", async () => { + const getBranchSha = vi + .fn<() => Promise>() + .mockResolvedValueOnce("previous-candidate") + .mockResolvedValueOnce("previous-candidate") + .mockResolvedValueOnce("commit-sha"); + const github = fakeGitHub({ getBranchSha }); + + await publishCandidate( + { + branch: "bot/fix-9", + runId: "run-9", + commitMessage: "Update candidate", + expectedPreviousSha: "previous-candidate", + snapshot: { + baseCommitSha: "new-main-head", + treeSha: "tree-sha", + changes: [{ path: "x.ts", mode: "100644", content: new Uint8Array([1]) }], + }, + }, + github, + ); + + expect(github.createCommit).toHaveBeenCalledWith( + expect.any(String), + "tree-sha", + "previous-candidate", + ); + }); +}); diff --git a/infra/emdash-bot/tests/unit/evals-client.test.ts b/infra/emdash-bot/tests/unit/evals-client.test.ts index 42e350415c..cd3fa5b477 100644 --- a/infra/emdash-bot/tests/unit/evals-client.test.ts +++ b/infra/emdash-bot/tests/unit/evals-client.test.ts @@ -10,6 +10,9 @@ const REPORTED = { result: { reproduced: true, summary: "reproduced the bug" }, ok: true, pushed: false, + runId: "run-1", + publication: null, + verification: [], }; describe("extractInvestigationResult", () => { diff --git a/infra/emdash-bot/tests/unit/evals-scorer.test.ts b/infra/emdash-bot/tests/unit/evals-scorer.test.ts index f5184b697e..51d8fd6ffc 100644 --- a/infra/emdash-bot/tests/unit/evals-scorer.test.ts +++ b/infra/emdash-bot/tests/unit/evals-scorer.test.ts @@ -26,7 +26,7 @@ function makeCase(overrides: Partial = {}): EvalCase { } function reported(result: ReportedResult["result"], ok = true, pushed = false): ReportedResult { - return { result, ok, pushed }; + return { result, ok, pushed, runId: "eval-run", publication: null, verification: [] }; } describe("diagnoseOutcome mirrors the machine's outcomeFromResult (diagnose mode)", () => { diff --git a/infra/emdash-bot/tests/unit/exec-env.test.ts b/infra/emdash-bot/tests/unit/exec-env.test.ts index 1fc6087937..471c60705e 100644 --- a/infra/emdash-bot/tests/unit/exec-env.test.ts +++ b/infra/emdash-bot/tests/unit/exec-env.test.ts @@ -1,6 +1,11 @@ import { describe, expect, test, vi } from "vitest"; -import { type ContainerBackend, ExecEnv, type IsolateState } from "../../.flue/lib/exec-env.js"; +import { + type ContainerBackend, + ExecEnv, + type IsolateState, + parseRawGitDiff, +} from "../../.flue/lib/exec-env.js"; function fakeState(initial?: Record): { state: IsolateState; @@ -72,22 +77,28 @@ function fakeContainer(): { execs: string[]; writes: Array<{ path: string; content: string }>; setExecResult: (result: { exitCode: number; stdout: string; stderr: string }) => void; + queueExecResults: ( + ...results: Array<{ exitCode: number; stdout: string; stderr: string }> + ) => void; + setReadFileBytes: (read: (path: string) => Uint8Array) => void; hangExec: () => void; } { const execs: string[] = []; const writes: Array<{ path: string; content: string }> = []; let execResult = { exitCode: 0, stdout: "container-ran", stderr: "" }; + const queuedExecResults: Array<{ exitCode: number; stdout: string; stderr: string }> = []; + let readFileBytes: (path: string) => Uint8Array = (_path) => new Uint8Array([1, 2, 3]); let hang = false; const container: ContainerBackend = { exec: async (command) => { execs.push(command); if (hang) return new Promise(() => {}); - return execResult; + return queuedExecResults.shift() ?? execResult; }, writeFile: async (path, content) => { writes.push({ path, content }); }, - readFileBytes: async () => new Uint8Array([1, 2, 3]), + readFileBytes: async (path) => readFileBytes(path), }; return { container, @@ -96,6 +107,12 @@ function fakeContainer(): { setExecResult: (result) => { execResult = result; }, + queueExecResults: (...results) => { + queuedExecResults.push(...results); + }, + setReadFileBytes: (read) => { + readFileBytes = read; + }, hangExec: () => { hang = true; }, @@ -130,7 +147,7 @@ describe("ExecEnv container exec", () => { const result = await env.exec("pnpm test"); expect(result.stdout).toBe("container-ran"); - expect(con.execs).toEqual(["pnpm test"]); + expect(con.execs).toEqual(["bash -o pipefail -c 'pnpm test'"]); }); test("materializes logged VFS edits before the command runs", async () => { @@ -142,7 +159,51 @@ describe("ExecEnv container exec", () => { await env.exec("pnpm test"); expect(con.writes).toEqual([{ path: "/repo/src/x.ts", content: "v2" }]); - expect(con.execs).toEqual(["pnpm test"]); + expect(con.execs).toEqual(["bash -o pipefail -c 'pnpm test'"]); + }); + + test("runs pipelines with pipefail so a failed producer cannot look successful", async () => { + const con = fakeContainer(); + const env = makeEnv({ container: con.container }); + + await env.exec("pnpm test 2>&1 | tail -20"); + + expect(con.execs).toEqual(["bash -o pipefail -c 'pnpm test 2>&1 | tail -20'"]); + }); + + test("rejects and discards source changes made by a verification command", async () => { + const con = fakeContainer(); + con.queueExecResults( + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "before-tree\n", stderr: "" }, + { exitCode: 0, stdout: "formatted", stderr: "" }, + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "after-tree\n", stderr: "" }, + { exitCode: 0, stdout: "", stderr: "" }, + ); + const env = makeEnv({ container: con.container }); + + await expect(env.runCheck("pnpm format")).rejects.toThrow( + /verification command modified the candidate/, + ); + expect(con.execs.at(-1)).toContain("git reset --hard HEAD"); + }); + + test("returns the verified candidate tree for a read-only check", async () => { + const con = fakeContainer(); + con.queueExecResults( + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "candidate-tree\n", stderr: "" }, + { exitCode: 0, stdout: "passed", stderr: "" }, + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "candidate-tree\n", stderr: "" }, + ); + const env = makeEnv({ container: con.container }); + + await expect(env.runCheck("pnpm format:check")).resolves.toEqual({ + result: { exitCode: 0, stdout: "passed", stderr: "" }, + candidateTreeSha: "candidate-tree", + }); }); test("an edit in one instance is materialized when another execs over the same VFS", async () => { @@ -185,6 +246,125 @@ describe("ExecEnv container exec", () => { }); }); +describe("ExecEnv candidate snapshots", () => { + const zeroSha = "0".repeat(40); + const blobSha = "1".repeat(40); + + test("parses the null-delimited raw format emitted by git diff --cached", () => { + const raw = [ + `:000000 100644 ${zeroSha} ${blobSha} A`, + "src/new file.ts", + `:100755 100755 ${blobSha} ${blobSha} M`, + "bin/run", + `:100644 000000 ${blobSha} ${zeroSha} D`, + "src/old.ts", + "", + ].join("\0"); + + expect(parseRawGitDiff(raw)).toEqual([ + { path: "src/new file.ts", mode: "100644", deleted: false, blobSha }, + { path: "bin/run", mode: "100755", deleted: false, blobSha }, + { path: "src/old.ts", mode: "100644", deleted: true, blobSha: null }, + ]); + }); + + test("snapshots added and deleted files from the staged diff", async () => { + const stagedBlobSha = "3e757656cf36eca53338e520d134963a44f793f8"; + const raw = [ + `:000000 100644 ${zeroSha} ${stagedBlobSha} A`, + "src/new.ts", + `:100644 000000 ${blobSha} ${zeroSha} D`, + "src/old.ts", + "", + ].join("\0"); + const con = fakeContainer(); + con.queueExecResults( + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "base-commit\n", stderr: "" }, + { exitCode: 0, stdout: "candidate-tree\n", stderr: "" }, + { exitCode: 0, stdout: raw, stderr: "" }, + ); + con.setReadFileBytes(() => new TextEncoder().encode("new\n")); + const env = makeEnv({ container: con.container }); + + await expect(env.snapshotCandidate()).resolves.toEqual({ + baseCommitSha: "base-commit", + treeSha: "candidate-tree", + changes: [ + { path: "src/new.ts", mode: "100644", content: new TextEncoder().encode("new\n") }, + { path: "src/old.ts", mode: "100644", content: null }, + ], + }); + expect( + con.execs.some( + (command) => command.includes("git cat-file blob") && command.includes(stagedBlobSha), + ), + ).toBe(true); + }); + + test("rejects content that does not match the staged blob", async () => { + const con = fakeContainer(); + con.queueExecResults( + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "base\n", stderr: "" }, + { exitCode: 0, stdout: "candidate-tree\n", stderr: "" }, + { + exitCode: 0, + stdout: `:000000 100644 ${zeroSha} 3e757656cf36eca53338e520d134963a44f793f8 A\0src/new.ts\0`, + stderr: "", + }, + ); + con.setReadFileBytes(() => new TextEncoder().encode("changed after staging\n")); + + await expect(makeEnv({ container: con.container }).snapshotCandidate()).rejects.toThrow( + /does not match staged blob/, + ); + }); + + test("rejects malformed raw diffs, symlinks, and workflow changes", async () => { + expect(() => parseRawGitDiff(`:000000 100644 ${zeroSha} ${blobSha} A\0src/x.ts`)).toThrow( + /malformed staged diff/, + ); + expect(() => parseRawGitDiff(`:000000 120000 ${zeroSha} ${blobSha} A\0link\0`)).toThrow( + /symlink/, + ); + + const con = fakeContainer(); + con.queueExecResults( + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "base\n", stderr: "" }, + { exitCode: 0, stdout: "candidate-tree\n", stderr: "" }, + { + exitCode: 0, + stdout: `:000000 100644 ${zeroSha} ${blobSha} A\0.github/workflows/pwn.yml\0`, + stderr: "", + }, + ); + await expect(makeEnv({ container: con.container }).snapshotCandidate()).rejects.toThrow( + /cannot publish path/, + ); + }); + + test("rejects a candidate file larger than the publication limit", async () => { + const con = fakeContainer(); + con.queueExecResults( + { exitCode: 0, stdout: "", stderr: "" }, + { exitCode: 0, stdout: "base\n", stderr: "" }, + { exitCode: 0, stdout: "candidate-tree\n", stderr: "" }, + { + exitCode: 0, + stdout: `:000000 100644 ${zeroSha} ${blobSha} A\0large.bin\0`, + stderr: "", + }, + ); + con.setReadFileBytes(() => new Uint8Array(2 * 1024 * 1024 + 1)); + + await expect(makeEnv({ container: con.container }).snapshotCandidate()).rejects.toThrow( + /file large\.bin is .* limit/, + ); + }); +}); + describe("ExecEnv deadlines", () => { test("container exec adds the grace margin to its own timeout", async () => { vi.useFakeTimers(); @@ -234,7 +414,10 @@ describe("ExecEnv container lifecycle", () => { await env.exec("pnpm test"); expect(attach).toHaveBeenCalledTimes(1); - expect(con.execs).toEqual(["pnpm install", "pnpm test"]); + expect(con.execs).toEqual([ + "bash -o pipefail -c 'pnpm install'", + "bash -o pipefail -c 'pnpm test'", + ]); }); }); diff --git a/infra/emdash-bot/tests/unit/github-proxy.test.ts b/infra/emdash-bot/tests/unit/github-proxy.test.ts index 45d2dc7620..2ca13f4066 100644 --- a/infra/emdash-bot/tests/unit/github-proxy.test.ts +++ b/infra/emdash-bot/tests/unit/github-proxy.test.ts @@ -4,6 +4,7 @@ import { createPushCapability, gateGithubRequest, githubAuthHeader, + inspectGithubRequest, verifyPushCapability, } from "../../.flue/lib/github-proxy.js"; @@ -80,14 +81,14 @@ describe("gateGithubRequest", () => { ).resolves.toMatch(/read-only/); }); - test("allows pushes only to bot fix branches", async () => { + test("rejects direct sandbox pushes to candidate and unrelated branches", async () => { const url = "https://github.com/emdash-cms/emdash.git/git-receive-pack"; await expect( gate(url, { method: "POST", body: `${pktLine("old new refs/heads/bot/fix-123\0 report-status\n")}0000PACKpayload`, }), - ).resolves.toBeNull(); + ).resolves.toMatch(/artifacts branch/); await expect( gate(url, { method: "POST", @@ -102,6 +103,24 @@ describe("gateGithubRequest", () => { ).resolves.toMatch(/current issue/); }); + test("distinguishes a missing capability from a rejected receive-pack body", async () => { + const url = new URL("https://github.com/emdash-cms/emdash.git/git-receive-pack"); + const request = new Request(url, { + method: "POST", + body: `${pktLine("old new refs/heads/bot/fix-123\0 report-status\n")}0000`, + }); + + await expect(inspectGithubRequest(request, url, OWNER, REPO)).resolves.toMatchObject({ + allowed: false, + stage: "capability", + }); + await expect(inspectGithubRequest(request, url, OWNER, REPO, 456)).resolves.toMatchObject({ + allowed: false, + stage: "receive-pack", + refs: ["refs/heads/bot/fix-123"], + }); + }); + test("allows pushes to the issue's artifacts branch", async () => { const url = "https://github.com/emdash-cms/emdash.git/git-receive-pack"; await expect( @@ -118,6 +137,16 @@ describe("gateGithubRequest", () => { ).resolves.toMatch(/current issue/); }); + test("checks the command ref rather than ref-like capability text", async () => { + const url = "https://github.com/emdash-cms/emdash.git/git-receive-pack"; + await expect( + gate(url, { + method: "POST", + body: `${pktLine("old new refs/meta/evil\0 refs/heads/bot/artifacts-123\n")}0000PACKpayload`, + }), + ).resolves.toMatch(/artifacts branch/); + }); + test("rejects an unbounded receive-pack command prefix", async () => { const url = "https://github.com/emdash-cms/emdash.git/git-receive-pack"; const oversizedPrefix = "f".repeat(64 * 1024); @@ -136,7 +165,7 @@ describe("gateGithubRequest", () => { if (pullCount === 1) { controller.enqueue( new TextEncoder().encode( - `${pktLine("old new refs/heads/bot/fix-123\0 report-status\n")}0000`, + `${pktLine("old new refs/heads/bot/artifacts-123\0 report-status\n")}0000`, ), ); return; diff --git a/infra/emdash-bot/tests/unit/github.test.ts b/infra/emdash-bot/tests/unit/github.test.ts new file mode 100644 index 0000000000..a5ee61be43 --- /dev/null +++ b/infra/emdash-bot/tests/unit/github.test.ts @@ -0,0 +1,99 @@ +import { afterEach, describe, expect, test, vi } from "vitest"; + +import { + createBranch, + createGitBlob, + createGitCommit, + createGitTree, + getGitCommit, + updateBranch, +} from "../../.flue/lib/github.js"; + +const repo = { owner: "emdash-cms", repo: "emdash" }; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +function parseJsonBody(body: unknown): unknown { + if (typeof body !== "string") throw new Error("expected a string request body"); + return JSON.parse(body); +} + +describe("GitHub Git Data requests", () => { + afterEach(() => vi.unstubAllGlobals()); + + test("uses the documented blob, tree, and commit request shapes", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(jsonResponse({ tree: { sha: "base-tree" }, message: "base" })) + .mockResolvedValueOnce(jsonResponse({ sha: "blob-sha" })) + .mockResolvedValueOnce(jsonResponse({ sha: "tree-sha" })) + .mockResolvedValueOnce(jsonResponse({ sha: "commit-sha" })); + vi.stubGlobal("fetch", fetchMock); + + await expect(getGitCommit("token", repo, "base/sha")).resolves.toEqual({ + treeSha: "base-tree", + message: "base", + }); + await expect(createGitBlob("token", repo, new Uint8Array([0, 255]))).resolves.toBe("blob-sha"); + await expect( + createGitTree("token", repo, "base-tree", [ + { path: "src/x.ts", mode: "100644", type: "blob", sha: "blob-sha" }, + { path: "src/old.ts", mode: "100644", type: "blob", sha: null }, + ]), + ).resolves.toBe("tree-sha"); + await expect(createGitCommit("token", repo, "Fix it", "tree-sha", "parent-sha")).resolves.toBe( + "commit-sha", + ); + + expect(fetchMock.mock.calls.map(([url]) => url)).toEqual([ + "https://api.github.com/repos/emdash-cms/emdash/git/commits/base%2Fsha", + "https://api.github.com/repos/emdash-cms/emdash/git/blobs", + "https://api.github.com/repos/emdash-cms/emdash/git/trees", + "https://api.github.com/repos/emdash-cms/emdash/git/commits", + ]); + expect(parseJsonBody(fetchMock.mock.calls[1]?.[1]?.body)).toEqual({ + content: "AP8=", + encoding: "base64", + }); + expect(parseJsonBody(fetchMock.mock.calls[2]?.[1]?.body)).toEqual({ + base_tree: "base-tree", + tree: [ + { path: "src/x.ts", mode: "100644", type: "blob", sha: "blob-sha" }, + { path: "src/old.ts", mode: "100644", type: "blob", sha: null }, + ], + }); + expect(parseJsonBody(fetchMock.mock.calls[3]?.[1]?.body)).toEqual({ + message: "Fix it", + tree: "tree-sha", + parents: ["parent-sha"], + }); + }); + + test("creates the scoped ref and updates it without force", async () => { + const fetchMock = vi.fn().mockResolvedValue(jsonResponse({})); + vi.stubGlobal("fetch", fetchMock); + + await createBranch("token", repo, "bot/fix-2299", "commit-sha"); + await updateBranch("token", repo, "bot/fix-2299", "next-sha"); + + expect(fetchMock.mock.calls[0]?.[0]).toBe( + "https://api.github.com/repos/emdash-cms/emdash/git/refs", + ); + expect(parseJsonBody(fetchMock.mock.calls[0]?.[1]?.body)).toEqual({ + ref: "refs/heads/bot/fix-2299", + sha: "commit-sha", + }); + expect(fetchMock.mock.calls[1]?.[0]).toBe( + "https://api.github.com/repos/emdash-cms/emdash/git/refs/heads/bot%2Ffix-2299", + ); + expect(parseJsonBody(fetchMock.mock.calls[1]?.[1]?.body)).toEqual({ + sha: "next-sha", + force: false, + }); + }); +}); diff --git a/infra/emdash-bot/tests/unit/orchestrator-comments.test.ts b/infra/emdash-bot/tests/unit/orchestrator-comments.test.ts index bb7186d0a8..38afb61467 100644 --- a/infra/emdash-bot/tests/unit/orchestrator-comments.test.ts +++ b/infra/emdash-bot/tests/unit/orchestrator-comments.test.ts @@ -4,6 +4,7 @@ import { renderAgentComment, renderDraftPrBody, renderPreviewReadyAsk, + renderReadonlyReply, shouldPostReadonlyReply, } from "../../.flue/lib/comments.js"; import type { Decision } from "../../.flue/lib/router.js"; @@ -22,11 +23,46 @@ function fixReadyDecision(): Extract { }; } +function failedDecision(): Extract { + return { + kind: "transition", + from: "working", + to: "failed", + action: null, + addLabel: "bot:failed", + addLabels: ["bot:failed"], + removeLabels: ["bot:working"], + event: "agent.failed", + arg: null, + }; +} + describe("renderAgentComment", () => { - test("agent.fix_ready uses the canonical pkg.pr.new owner/repo URL", () => { + test("agent.fix_ready uses the default production preview package", () => { const body = renderAgentComment(fixReadyDecision(), 1234, "Fixed the bug."); - expect(body).toContain("pnpm add https://pkg.pr.new/emdash-cms/emdash@bot/fix-1234"); - expect(body).not.toContain("https://pkg.pr.new/emdash@bot/fix-"); + expect(body).toContain("npm i https://pkg.pr.new/emdash@bot/fix-1234"); + expect(body).not.toContain("https://pkg.pr.new/emdash-cms/emdash@bot/fix-"); + }); + + test("agent.fix_ready uses the configured staging preview package", () => { + const body = renderAgentComment( + fixReadyDecision(), + 1234, + "Fixed the bug.", + undefined, + "owner/canary/package", + ); + expect(body).toContain("npm i https://pkg.pr.new/owner/canary/package@bot/fix-1234"); + expect(body).not.toContain("https://pkg.pr.new/emdash-cms/emdash@bot/fix-1234"); + }); + + test("failed comments identify the failed stage and durable run", () => { + const body = renderAgentComment(failedDecision(), 1234, "Publication did not complete.", { + runId: "run-abc", + failureStage: "publication", + }); + expect(body).toContain("Failed stage: `publication`"); + expect(body).toContain("Run: `run-abc`"); }); }); @@ -59,6 +95,12 @@ describe("renderPreviewReadyAsk", () => { expect(body).toContain("Could the reporter please try this"); }); + test("uses the staging preview package supplied by the orchestrator", () => { + const body = ask({ previewPackage: "owner/canary/canary-package" }); + expect(body).toContain("npm i https://pkg.pr.new/owner/canary/canary-package@bot/fix-77"); + expect(body).not.toContain("https://pkg.pr.new/emdash@bot/fix-77"); + }); + test("renders screenshots from the artifacts branch with escaped alt text", () => { const body = ask({ screenshots: [{ filename: "step-1.png", description: "broken [state] (here)" }], @@ -83,6 +125,12 @@ describe("renderPreviewReadyAsk", () => { test("omits the screenshots block entirely when there are none", () => { expect(ask({ screenshots: [] })).not.toContain("**Screenshots:**"); }); + + test("does not describe a directed implementation as a reproduced bug", () => { + const body = ask({ notes: "Added the requested export." }); + expect(body).toContain("candidate change"); + expect(body).not.toMatch(/reproduced|candidate fix/i); + }); }); describe("renderDraftPrBody", () => { @@ -90,12 +138,21 @@ describe("renderDraftPrBody", () => { const body = renderDraftPrBody(77); expect(body).toContain("Closes #77."); expect(body).toContain("npm i https://pkg.pr.new/emdash@bot/fix-77"); - expect(body).toContain("regression test"); + expect(body).toContain("candidate change"); + expect(body).not.toMatch(/candidate fix|regression test/i); expect(body).toContain("draft"); }); }); describe("shouldPostReadonlyReply", () => { + test("uses change-neutral copy for the shared delivery states", () => { + expect(renderReadonlyReply("fixing")).toBe("Building a candidate change."); + expect(renderReadonlyReply("preview_building")).toBe( + "Building a preview so you can try the change.", + ); + expect(renderReadonlyReply("awaiting_reporter")).toContain("if it works"); + }); + test("suppresses GitHub comments for dry runs", () => { expect(shouldPostReadonlyReply(true)).toBe(false); expect(shouldPostReadonlyReply(false)).toBe(true); diff --git a/infra/emdash-bot/tests/unit/preview.test.ts b/infra/emdash-bot/tests/unit/preview.test.ts index e39a4b949a..414e88934a 100644 --- a/infra/emdash-bot/tests/unit/preview.test.ts +++ b/infra/emdash-bot/tests/unit/preview.test.ts @@ -16,6 +16,16 @@ describe("preview branch + URL helpers", () => { expect(previewUrl(42)).toBe("https://pkg.pr.new/emdash@bot/fix-42"); expect(previewInstallCommand(42)).toBe("npm i https://pkg.pr.new/emdash@bot/fix-42"); }); + + test("supports a staging package without probing the production preview", () => { + expect(previewUrl(42, "owner/canary-repo/canary-package")).toBe( + "https://pkg.pr.new/owner/canary-repo/canary-package@bot/fix-42", + ); + expect(previewInstallCommand(42, "owner/canary-repo/canary-package")).toBe( + "npm i https://pkg.pr.new/owner/canary-repo/canary-package@bot/fix-42", + ); + expect(() => previewUrl(42, "https://attacker.test/x")).toThrow(/invalid preview package/); + }); }); describe("branchesToReap", () => { diff --git a/infra/emdash-bot/tests/unit/router.test.ts b/infra/emdash-bot/tests/unit/router.test.ts index 41f5e92a96..d49a5dbe3f 100644 --- a/infra/emdash-bot/tests/unit/router.test.ts +++ b/infra/emdash-bot/tests/unit/router.test.ts @@ -68,7 +68,7 @@ describe("router", () => { }); assertTransition(d); expect(d.from).toBe("unmanaged"); - expect(d.to).toBe("working"); + expect(d.to).toBe("fixing"); expect(d.action).toBe("investigate.implement"); }); @@ -120,11 +120,11 @@ describe("router", () => { actor: "maintainer", }); assertTransition(d); - expect(d.to).toBe("working"); + expect(d.to).toBe("fixing"); expect(d.action).toBe("investigate.implement"); - expect(d.addLabel).toBe("bot:working"); + expect(d.addLabel).toBe("bot:fixing"); expect(d.removeLabels).toContain("bot:blocked"); - expect(d.removeLabels).not.toContain("bot:working"); + expect(d.removeLabels).not.toContain("bot:fixing"); }); test("resolve: in_review accepts revise (PR feedback bridge)", () => { @@ -146,7 +146,7 @@ describe("router", () => { actor: "maintainer", }); assertTransition(d); - expect(d.to).toBe("working"); + expect(d.to).toBe("fixing"); expect(d.action).toBe("investigate.implement"); }); @@ -406,16 +406,15 @@ describe("router", () => { ); }); - test("outcomeFromResult allows implement and revise runs to produce a fix", () => { - for (const mode of ["implement", "revise"] as const) { - const input = { + test("outcomeFromResult allows revise runs to produce a fix", () => { + expect( + outcomeFromResult({ ok: true, result: { fixed: true }, pushed: true, - mode, - }; - expect(outcomeFromResult(input)).toBe("agent.fix_ready"); - } + mode: "revise", + }), + ).toBe("agent.fix_ready"); }); test("outcomeFromResult feeds resolve to advance the machine end-to-end", () => { @@ -532,6 +531,30 @@ describe("router: investigation + fix loop", () => { expect(d.to).toBe("reproduced"); }); + test("enhancement delivery failures return to a retryable implementation state", () => { + const previewFailed = resolve({ + labels: ["bot:enhancement", "bot:preview-building"], + event: "preview.failed", + actor: "system", + }); + assertTransition(previewFailed); + expect(previewFailed.to).toBe("blocked"); + + for (const event of ["reject", "expire"] as const) { + const decision = resolve({ + labels: ["bot:enhancement", "bot:awaiting-reporter"], + event, + actor: event === "reject" ? "reporter" : "system", + }); + assertTransition(decision); + expect(decision.to).toBe("blocked"); + expect(decision.action).toBe("reapBranch"); + } + + const commands = new Set(classifierCommands("blocked").map((command) => command.event)); + expect(commands.has("implement")).toBe(true); + }); + test("confirm opens a draft PR; reject and expire reap the branch", () => { const confirm = resolve({ labels: ["bot:bug", "bot:awaiting-reporter"], @@ -592,4 +615,26 @@ describe("router: investigation + fix loop", () => { "agent.failed", ); }); + + test("outcomeFromResult implement mode uses implementation fields and verified publication", () => { + expect( + outcomeFromResult({ + ok: true, + result: { implemented: true }, + pushed: true, + mode: "implement", + }), + ).toBe("agent.fix_ready"); + expect( + outcomeFromResult({ + ok: true, + result: { implemented: true }, + pushed: false, + mode: "implement", + }), + ).toBe("agent.failed"); + expect( + outcomeFromResult({ ok: true, result: { fixed: true }, pushed: true, mode: "implement" }), + ).toBe("agent.failed"); + }); }); diff --git a/infra/emdash-bot/tests/unit/verification.test.ts b/infra/emdash-bot/tests/unit/verification.test.ts new file mode 100644 index 0000000000..6cbe45fcd6 --- /dev/null +++ b/infra/emdash-bot/tests/unit/verification.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test } from "vitest"; + +import { + assertVerificationCommand, + passingVerificationRecords, +} from "../../.flue/lib/verification.js"; + +describe("verification commands", () => { + test("rejects pipelines and explicit success fallbacks that hide failures", () => { + expect(() => assertVerificationCommand("pnpm test 2>&1 | tail -20")).toThrow(/pipeline/); + expect(() => assertVerificationCommand("pnpm test || true")).toThrow(/pipeline/); + expect(() => assertVerificationCommand("pnpm test; true")).toThrow(/shell control/); + expect(() => assertVerificationCommand("pnpm test & wait")).toThrow(/shell control/); + expect(() => assertVerificationCommand("pnpm test\ntrue")).toThrow(/shell control/); + expect(() => assertVerificationCommand("! pnpm test")).toThrow(/negate/); + }); + + test("accepts direct checks and requires the latest result for each name to pass", () => { + expect(() => assertVerificationCommand("pnpm --filter emdash test")).not.toThrow(); + expect( + passingVerificationRecords([ + { name: "tests", command: "pnpm test", exitCode: 1, candidateTreeSha: "tree" }, + { name: "tests", command: "pnpm test", exitCode: 0, candidateTreeSha: "tree" }, + { + name: "lint", + command: "pnpm lint:quick", + exitCode: 0, + candidateTreeSha: "tree", + }, + ]), + ).toEqual([ + { name: "tests", command: "pnpm test", exitCode: 0, candidateTreeSha: "tree" }, + { + name: "lint", + command: "pnpm lint:quick", + exitCode: 0, + candidateTreeSha: "tree", + }, + ]); + }); + + test("refuses publication when the latest named check failed", () => { + expect(() => + passingVerificationRecords([ + { name: "tests", command: "pnpm test", exitCode: 0, candidateTreeSha: "tree" }, + { name: "tests", command: "pnpm test", exitCode: 1, candidateTreeSha: "tree" }, + ]), + ).toThrow(/tests/); + }); + + test("does not let a failed named check be replaced by a different command", () => { + expect(() => + passingVerificationRecords([ + { name: "tests", command: "pnpm test", exitCode: 1, candidateTreeSha: "tree" }, + { name: "tests", command: "true", exitCode: 0, candidateTreeSha: "tree" }, + ]), + ).toThrow(/changed command/); + }); + + test("does not publish a candidate changed after verification", () => { + expect(() => + passingVerificationRecords( + [ + { + name: "tests", + command: "pnpm test", + exitCode: 0, + candidateTreeSha: "verified-tree", + }, + ], + "published-tree", + ), + ).toThrow(/candidate changed/); + }); +}); diff --git a/infra/emdash-bot/worker-configuration.d.ts b/infra/emdash-bot/worker-configuration.d.ts index 9bd0ca5678..27c90fa0ab 100644 --- a/infra/emdash-bot/worker-configuration.d.ts +++ b/infra/emdash-bot/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types` (hash: 3f3f654dfbcaa21e42691751e4c140b4) +// Generated by Wrangler by running `wrangler types` (hash: c717dce49317a0071623b110c32e5f22) // Runtime types generated with workerd@1.20260611.1 2026-07-17 nodejs_compat interface __BaseEnv_Env { BOT_WORKSPACE: R2Bucket; @@ -9,6 +9,7 @@ interface __BaseEnv_Env { GITHUB_APP_INSTALLATION_ID: "120963314"; GITHUB_OWNER: "emdash-cms"; GITHUB_REPO: "emdash"; + PREVIEW_PACKAGE: "emdash"; GITHUB_WEBHOOK_SECRET: string; GITHUB_APP_PRIVATE_KEY: string; Sandbox: DurableObjectNamespace /* Sandbox */; @@ -22,7 +23,7 @@ type StringifyValues> = { [Binding in keyof EnvType]: EnvType[Binding] extends string ? EnvType[Binding] : string; }; declare namespace NodeJS { - interface ProcessEnv extends StringifyValues> {} + interface ProcessEnv extends StringifyValues> {} } // Begin runtime types diff --git a/infra/emdash-bot/wrangler.jsonc b/infra/emdash-bot/wrangler.jsonc index 8e11b0b416..cc4c7a8505 100644 --- a/infra/emdash-bot/wrangler.jsonc +++ b/infra/emdash-bot/wrangler.jsonc @@ -96,6 +96,7 @@ "GITHUB_APP_INSTALLATION_ID": "120963314", "GITHUB_OWNER": "emdash-cms", "GITHUB_REPO": "emdash", + "PREVIEW_PACKAGE": "emdash", }, // `wrangler secret put ` for production. Local dev reads .env. diff --git a/infra/emdash-bot/wrangler.test.jsonc b/infra/emdash-bot/wrangler.test.jsonc index f4392b7067..f3d95dbd52 100644 --- a/infra/emdash-bot/wrangler.test.jsonc +++ b/infra/emdash-bot/wrangler.test.jsonc @@ -72,5 +72,6 @@ "GITHUB_APP_INSTALLATION_ID": "120963314", "GITHUB_OWNER": "emdash-cms", "GITHUB_REPO": "emdash-test", + "PREVIEW_PACKAGE": "emdash", }, }