diff --git a/apps/web/components/run/cockpit.tsx b/apps/web/components/run/cockpit.tsx index 2872d05..8151207 100644 --- a/apps/web/components/run/cockpit.tsx +++ b/apps/web/components/run/cockpit.tsx @@ -2,8 +2,10 @@ import { isBuilderMode, runObjectiveText } from "@facility/run-objective"; import { Button, Cell, cx, Eyebrow, HairlineGrid, Metric, StatusDot, toneFor } from "@facility/ui"; +import Link from "next/link"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { AiIdentity } from "@/components/ai-identity"; +import { runErrorPresentation } from "@/components/run/run-error"; import { RunTranscript } from "@/components/run/transcript"; import { engineIdentity } from "@/lib/ai-identity"; import type { Project, Run, RunEvent } from "@/lib/api"; @@ -272,6 +274,24 @@ function githubArtifacts(run: Run) { ].filter((item): item is { label: string; text: string; href: string | null } => Boolean(item)); } +function RunErrorLine({ error, projectId }: { error: string; projectId: string | null }) { + const presentation = runErrorPresentation(error, projectId); + if (!presentation) return

{error}

; + return ( +
+

{presentation.message}

+ {presentation.href ? ( + + configure acceptance checks + + ) : null} +
+ ); +} + function checkItems(events: RunEvent[]) { return events .filter((event) => event.type === "check") @@ -613,7 +633,7 @@ export function RunCockpit({ - {run.error ?

{run.error}

: null} + {run.error ? : null} diff --git a/apps/web/components/run/run-error.ts b/apps/web/components/run/run-error.ts new file mode 100644 index 0000000..4d94ec7 --- /dev/null +++ b/apps/web/components/run/run-error.ts @@ -0,0 +1,51 @@ +export const CHECKS_NOT_CONFIGURED = "checks_not_configured"; +export const DELIVERY_REPO_NOT_CONFIGURED = "delivery_repo_not_configured"; + +// Extract the machine code from a run's stored error: the runner posts JSON +// fault payloads (possibly followed by stderr tail), and the platform marks +// early failures with the same JSON shape. Anything else is not a coded error. +export function runErrorCode(error: string | null | undefined): string | null { + if (!error) return null; + const trimmed = error.trim(); + if (!trimmed) return null; + if (trimmed.startsWith("{")) { + const end = trimmed.indexOf("}"); + if (end > 1) { + try { + const parsed = JSON.parse(trimmed.slice(0, end + 1)) as { code?: unknown }; + if (typeof parsed.code === "string") return parsed.code; + } catch { + // Not a JSON error payload — render the raw string below. + } + } + } + return /^[a-z][a-z0-9_]*$/.test(trimmed) ? trimmed : null; +} + +export type RunErrorPresentation = { + code: string; + message: string; + href: string | null; +}; + +export function runErrorPresentation( + error: string | null | undefined, + projectId: string | null, +): RunErrorPresentation | null { + const code = runErrorCode(error); + if (code === DELIVERY_REPO_NOT_CONFIGURED) { + return { + code, + message: + "This builder run has no repository configured, so it cannot create a branch or pull request. Connect a repository in Settings and retry.", + href: projectId ? `/projects/${projectId}/settings` : null, + }; + } + if (code !== CHECKS_NOT_CONFIGURED) return null; + return { + code, + message: + "No acceptance checks are configured for this project, so the builder run could not deliver.", + href: projectId ? `/projects/${projectId}/settings` : null, + }; +} diff --git a/apps/web/test/run-error.test.ts b/apps/web/test/run-error.test.ts new file mode 100644 index 0000000..ab31994 --- /dev/null +++ b/apps/web/test/run-error.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it } from "vitest"; +import { runErrorCode, runErrorPresentation } from "@/components/run/run-error"; + +describe("runErrorCode", () => { + it("extracts the runner's JSON fault code", () => { + expect(runErrorCode('{"code":"checks_not_configured"}')).toBe("checks_not_configured"); + }); + + it("still reads the code when the runner appends stderr tail after the JSON", () => { + expect( + runErrorCode('{"code":"checks_not_configured"} npm error something\nnpm ERR! exit 1'), + ).toBe("checks_not_configured"); + }); + + it("accepts a bare machine code", () => { + expect(runErrorCode("checks_not_configured")).toBe("checks_not_configured"); + }); + + it("returns null for unknown codes, plain text, and empty errors", () => { + expect(runErrorCode('{"code":"provision_failed"}')).toBe("provision_failed"); + expect(runErrorCode("the database exploded")).toBeNull(); + expect(runErrorCode('{"message":"not a code"}')).toBeNull(); + expect(runErrorCode("")).toBeNull(); + expect(runErrorCode(null)).toBeNull(); + expect(runErrorCode(undefined)).toBeNull(); + }); +}); + +describe("runErrorPresentation", () => { + it("maps delivery_repo_not_configured to a human explanation with a settings link", () => { + expect(runErrorPresentation('{"code":"delivery_repo_not_configured"}', "project_1")).toEqual({ + code: "delivery_repo_not_configured", + message: + "This builder run has no repository configured, so it cannot create a branch or pull request. Connect a repository in Settings and retry.", + href: "/projects/project_1/settings", + }); + }); + + it("maps checks_not_configured to a human explanation with a settings link", () => { + expect(runErrorPresentation('{"code":"checks_not_configured"}', "project_1")).toEqual({ + code: "checks_not_configured", + message: + "No acceptance checks are configured for this project, so the builder run could not deliver.", + href: "/projects/project_1/settings", + }); + }); + + it("drops the settings link when no project is known", () => { + expect(runErrorPresentation("checks_not_configured", null)).toMatchObject({ href: null }); + }); + + it("leaves unknown errors to the raw renderer", () => { + expect(runErrorPresentation("error: something broke", "project_1")).toBeNull(); + expect(runErrorPresentation(null, "project_1")).toBeNull(); + }); +}); diff --git a/services/api/src/github/router.ts b/services/api/src/github/router.ts index 69c2667..81c5112 100644 --- a/services/api/src/github/router.ts +++ b/services/api/src/github/router.ts @@ -150,6 +150,9 @@ export async function routeTrigger( if (accepted?.blockedRunId) { return { routed: false, reason: "plan_already_accepted", runId: accepted.blockedRunId }; } + // The connected repository is already resolved here. GitHub CI owns + // acceptance for this lane; dispatchRun remains the universal preflight for + // repo-less delivery runs from other entry points. const githubTrigger = { type: "github_comment", githubLogin: sender, diff --git a/services/api/src/sandbox/delivery-gate.ts b/services/api/src/sandbox/delivery-gate.ts new file mode 100644 index 0000000..301f035 --- /dev/null +++ b/services/api/src/sandbox/delivery-gate.ts @@ -0,0 +1,17 @@ +import type { RunBundle } from "./state.js"; + +type DeliveryBundle = { mode: string; repo: Pick }; + +export const DELIVERY_REPO_NOT_CONFIGURED = "delivery_repo_not_configured"; + +// Keep delivery-mode detection aligned with the runner so only runs that need a +// branch and pull request are subject to the repository preflight. +export function requiresDelivery(mode: string) { + return mode === "builder" || mode.endsWith("-builder"); +} + +// A delivery-mode run without a repository cannot create its branch or pull +// request, so fail it before provisioning any spend-capable resources. +export function deliveryRepoConfigured(bundle: DeliveryBundle) { + return !requiresDelivery(bundle.mode) || Boolean(bundle.repo.cloneUrl); +} diff --git a/services/api/src/sandbox/orchestrator.ts b/services/api/src/sandbox/orchestrator.ts index f4b653d..0d095a1 100644 --- a/services/api/src/sandbox/orchestrator.ts +++ b/services/api/src/sandbox/orchestrator.ts @@ -65,6 +65,7 @@ import type { AppConfig } from "../types.js"; import { raisePlatformIssue, resolvePlatformIssue } from "../watchtower/issues.js"; import { sandboxCachePartition, sandboxNamespace } from "./cache.js"; import { nestedDockerEnabled, provisioningDepth } from "./capabilities.js"; +import { DELIVERY_REPO_NOT_CONFIGURED, deliveryRepoConfigured } from "./delivery-gate.js"; import { DockerSandboxDriver } from "./docker.js"; import type { LaunchSpec, SandboxDriver, SandboxDriverName } from "./driver.js"; import { sandboxDriver } from "./driver.js"; @@ -102,6 +103,21 @@ export async function dispatchRun(config: AppConfig, job: DispatchJob, deps: Dis try { const run = await loadRun(db, job.orgId, job.runId); if (run?.status !== "queued") return; + const { bundle, profile, agentPermissions } = await buildRunBundle(db, run, config); + // Refuse before any sandbox or spend-capable key exists: a delivery-mode + // agent without a repository cannot create a branch or pull request. The + // atomic failRun transition also closes the race where a concurrent worker + // already claimed and launched. + if (!deliveryRepoConfigured(bundle)) { + await failRun( + db, + job.orgId, + job.runId, + `{"code":"${DELIVERY_REPO_NOT_CONFIGURED}"}`, + DELIVERY_REPO_NOT_CONFIGURED, + ); + return; + } // Claim the run atomically. If a duplicate queue delivery raced us and // another worker already moved it out of "queued", the update touches no // rows and we must NOT launch a second sandbox for the same run. @@ -113,7 +129,6 @@ export async function dispatchRun(config: AppConfig, job: DispatchJob, deps: Dis if (claimed.length === 0) return; await appendRunEvents(db, run.orgId, run.id, [{ type: "provisioning", data: {} }]); - const { bundle, profile, agentPermissions } = await buildRunBundle(db, run, config); const virtualKey = await generateApiKey("fvk"); await db.insert(virtualKeys).values({ id: virtualKey.id, @@ -1659,27 +1674,7 @@ async function buildRunBundle( if (agent.orgId !== run.orgId || agent.projectId !== run.projectId) { throw new Error("agent_not_in_project"); } - const profile = ( - await db - .select() - .from(sandboxProfiles) - .where( - and( - eq(sandboxProfiles.orgId, run.orgId), - agent.sandboxProfileId - ? eq(sandboxProfiles.id, agent.sandboxProfileId) - : or(eq(sandboxProfiles.id, "sbx_dev_default"), isNull(sandboxProfiles.projectId)), - ), - ) - // Deterministic fallback when an org has several global profiles: the - // canonical seeded default wins, otherwise the oldest global profile — never - // an arbitrary row (limit(1) with no order returned a nondeterministic pick). - .orderBy( - sql`case when ${sandboxProfiles.id} = 'sbx_dev_default' then 0 else 1 end`, - sandboxProfiles.createdAt, - ) - .limit(1) - )[0]; + const profile = await agentSandboxProfile(db, run.orgId, agent); if (!profile) throw new Error("run_missing_sandbox_profile"); // Clone the repo the run is ACTUALLY about: an issue/resume trigger records it // in run.gh (owner/repo). Only fall back to the project's oldest repo when the @@ -2455,11 +2450,11 @@ function arrayField(value: unknown, key: string) { : []; } -// Resolve the run's acceptance-gate commands: a sandbox profile's explicit +// Resolve the run's acceptance commands: a sandbox profile's explicit // setup.check_cmds override wins; otherwise the project's own configured checks // (settings.check_cmds). Empty when neither is set. export function resolveCheckCmds( - profile: { setup: unknown }, + profile: { setup?: unknown }, renderAnswers: unknown, projectSettings: unknown, ): string[] { @@ -2471,6 +2466,34 @@ export function resolveCheckCmds( return arrayField(projectSettings, "check_cmds"); } +// The sandbox profile a run would use: the agent's explicit pick, else a +// deterministic global fallback (the canonical seeded default, then the oldest +// global profile — never an arbitrary row). +async function agentSandboxProfile( + db: ReturnType["db"], + orgId: string, + agent: { sandboxProfileId: string | null }, +) { + return ( + await db + .select() + .from(sandboxProfiles) + .where( + and( + eq(sandboxProfiles.orgId, orgId), + agent.sandboxProfileId + ? eq(sandboxProfiles.id, agent.sandboxProfileId) + : or(eq(sandboxProfiles.id, "sbx_dev_default"), isNull(sandboxProfiles.projectId)), + ), + ) + .orderBy( + sql`case when ${sandboxProfiles.id} = 'sbx_dev_default' then 0 else 1 end`, + sandboxProfiles.createdAt, + ) + .limit(1) + )[0]; +} + export function resolveRepoEngineConfig( agentName: string, base: unknown, diff --git a/services/api/test/github-platform-lane.test.ts b/services/api/test/github-platform-lane.test.ts index 6e8b7ad..a8f1004 100644 --- a/services/api/test/github-platform-lane.test.ts +++ b/services/api/test/github-platform-lane.test.ts @@ -3404,6 +3404,57 @@ describe("github platform lane", async () => { app.githubClientFactory = undefined; }); + it("accepts and enqueues a no-checks GitHub builder because GitHub CI owns acceptance", async () => { + const enqueued: { queue: string; data: Record }[] = []; + app.enqueue = async (queue, data) => { + enqueued.push({ queue, data }); + return null; + }; + // No `.facility.json` checks and no project-level check commands. A + // connected GitHub repo is still accepted and enqueued because GitHub CI + // owns acceptance; this is not the repo-less dispatch safety net. + app.githubClientFactory = async () => + ({ + rest: { + issues: { + get: async (input: { issue_number: number }) => ({ + data: { + number: input.issue_number, + title: `Issue ${input.issue_number}`, + body: "Body", + user: { login: "author" }, + labels: [], + html_url: `https://github.test/issues/${input.issue_number}`, + }, + }), + listComments: async () => ({ data: [] }), + createComment: async () => ({ data: { id: 1, html_url: "https://example.test/c" } }), + addAssignees: async () => ({ data: { assignees: [] } }), + }, + repos: repositoryApiWithoutFacilityManifest(), + git: {}, + pulls: {}, + }, + }) as never; + const repo = await insertRepoWithInstallation(`nochecks-${Date.now()}`); + await insertIssue(repo.id, 51, "open", "2026-03-01T00:00:06Z"); + await insertAgent("builder"); + + const response = await app.inject({ + method: "POST", + url: `/v1/projects/${projectId}/issues/51/trigger`, + headers: { cookie }, + payload: { agent: "builder" }, + }); + expect(response.statusCode, response.body).toBe(200); + expect(response.json().gh.issueNumber).toBe(51); + expect(enqueued).toContainEqual({ + queue: "runs.dispatch", + data: { runId: response.json().id, orgId }, + }); + app.githubClientFactory = undefined; + }); + it("pins project-scoped keys to their project across the issue mirror (404 elsewhere)", async () => { // A key pinned to ANOTHER project — with full engineer permissions — must // not read this project's issues nor trigger runs in it. diff --git a/services/api/test/orchestrator-checks.test.ts b/services/api/test/orchestrator-checks.test.ts index 46510db..a18918e 100644 --- a/services/api/test/orchestrator-checks.test.ts +++ b/services/api/test/orchestrator-checks.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { harnessFragmentForBundle } from "../src/harness.js"; +import { deliveryRepoConfigured, requiresDelivery } from "../src/sandbox/delivery-gate.js"; import { boundedResumeFallbackScope, platformDeliveryFailure, @@ -22,7 +23,35 @@ describe("platform delivery boundaries", () => { }); }); -describe("resolveCheckCmds — acceptance-gate source of truth", () => { +describe("dispatch delivery repository preflight", () => { + const githubBuilderBundle = { + mode: "builder", + checkCmds: [], + repo: { cloneUrl: "https://github.com/acme/app.git" }, + }; + const noRepoBuilderBundle = { mode: "builder", checkCmds: [], repo: { cloneUrl: null } }; + + it("refuses delivery-mode agents without a repository", () => { + expect(requiresDelivery("builder")).toBe(true); + expect(requiresDelivery("codex-builder")).toBe(true); + expect(deliveryRepoConfigured(noRepoBuilderBundle)).toBe(false); + expect(deliveryRepoConfigured({ ...noRepoBuilderBundle, mode: "codex-builder" })).toBe(false); + const noRepoBuilderWithChecks = { ...noRepoBuilderBundle, checkCmds: ["pnpm verify"] }; + expect(deliveryRepoConfigured(noRepoBuilderWithChecks)).toBe(false); + }); + + it("allows a connected GitHub repository when check commands are empty", () => { + expect(deliveryRepoConfigured(githubBuilderBundle)).toBe(true); + }); + + it("allows non-delivery modes without a repository", () => { + expect(requiresDelivery("architect")).toBe(false); + expect(deliveryRepoConfigured({ mode: "architect", repo: { cloneUrl: null } })).toBe(true); + expect(deliveryRepoConfigured({ mode: "custom", repo: { cloneUrl: null } })).toBe(true); + }); +}); + +describe("resolveCheckCmds — runner acceptance command source of truth", () => { it("uses the project's configured checks when the sandbox profile has none", () => { expect(resolveCheckCmds({ setup: {} }, {}, { check_cmds: ["pnpm test", "pnpm lint"] })).toEqual( ["pnpm test", "pnpm lint"], diff --git a/services/api/test/sandbox.test.ts b/services/api/test/sandbox.test.ts index 81d8f38..0f70992 100644 --- a/services/api/test/sandbox.test.ts +++ b/services/api/test/sandbox.test.ts @@ -277,6 +277,20 @@ describe("sandbox api", async () => { config: {}, }), ]); + const permissionRepo = ( + await db + .insert(repos) + .values({ + id: newId("repo"), + orgId, + projectId, + owner: `sandbox-permissions-${suffix}`, + name: "repo", + defaultBranch: "main", + }) + .returning() + )[0]; + if (!permissionRepo) throw new Error("permission repository fixture missing"); const driver: SandboxDriver = { name: "docker", launch: async (spec) => ({ ref: `fake-${spec.runId}` }), @@ -357,6 +371,127 @@ describe("sandbox api", async () => { { virtual: null, platform: null }, { virtual: null, platform: null }, ]); + await db.delete(repos).where(eq(repos.id, permissionRepo.id)); + }); + + it("fails a builder dispatch before provisioning when no delivery repository is configured", async () => { + const suffix = Date.now(); + const gateProject = ( + await db + .insert(projects) + .values({ + id: newId("proj"), + orgId, + name: `Gate Refusal Project ${suffix}`, + slug: `gate-refusal-${suffix}`, + settings: {}, + }) + .returning() + )[0]; + if (!gateProject) throw new Error("gate refusal project fixture missing"); + const contract = ( + await db + .insert(registryItems) + .values({ + id: newId("item"), + orgId, + scope: "project", + projectId: gateProject.id, + kind: "agent_contract", + name: `gate-contract-${suffix}`, + latestVersion: 1, + }) + .returning() + )[0]; + if (!contract) throw new Error("gate refusal contract fixture missing"); + await db.insert(registryVersions).values({ + id: newId("ver"), + orgId, + itemId: contract.id, + version: 1, + content: "Exercise the dispatch gate.", + contentHash: `gate-refusal-${suffix}`, + status: "active", + }); + const profile = ( + await db + .insert(sandboxProfiles) + .values({ + id: newId("sbx"), + orgId, + projectId: gateProject.id, + name: `gate-refusal-${suffix}`, + driver: "docker", + image: "facility-runner:test", + resources: { timeout_min: 5 }, + }) + .returning() + )[0]; + if (!profile) throw new Error("gate refusal profile fixture missing"); + const agent = ( + await db + .insert(agentDefs) + .values({ + id: newId("agent"), + orgId, + projectId: gateProject.id, + name: `gate-builder-${suffix}`, + engine: "byo", + model: { cmd: "true" }, + contractItemId: contract.id, + sandboxProfileId: profile.id, + triggers: [], + permissions: [], + enabled: true, + }) + .returning() + )[0]; + if (!agent) throw new Error("gate refusal agent fixture missing"); + const run = ( + await db + .insert(runs) + .values({ + id: newId("run"), + orgId, + projectId: gateProject.id, + agentDefId: agent.id, + mode: "builder", + engine: "byo", + trigger: {}, + createdBy: { type: "user", id: "gate-test" }, + }) + .returning() + )[0]; + if (!run) throw new Error("gate refusal run fixture missing"); + const launched: string[] = []; + const driver: SandboxDriver = { + name: "docker", + launch: async (spec) => { + launched.push(spec.runId); + return { ref: `fake-${spec.runId}` }; + }, + status: async () => "running", + async *logs() {}, + stop: async () => undefined, + destroy: async () => undefined, + }; + + await dispatchRun(config, { runId: run.id, orgId }, { sandboxDriver: async () => driver }); + + const [failed] = await db.select().from(runs).where(eq(runs.id, run.id)); + expect(failed?.status).toBe("failed"); + expect(failed?.error).toBe('{"code":"delivery_repo_not_configured"}'); + expect(failed?.sandbox).toEqual({}); + await expect( + db.select().from(virtualKeys).where(eq(virtualKeys.runId, run.id)), + ).resolves.toEqual([]); + await expect(db.select().from(apiKeys).where(eq(apiKeys.runId, run.id))).resolves.toEqual([]); + expect(launched).toEqual([]); + const events = await db + .select({ type: runEvents.type }) + .from(runEvents) + .where(eq(runEvents.runId, run.id)); + expect(events.map((event) => event.type)).toEqual(["result"]); }); it("dispatch persists engine-specific model policy on each run key", async () => {