diff --git a/docker-compose.dev-platform.yaml b/docker-compose.dev-platform.yaml index 0b0459aeb..30f143883 100644 --- a/docker-compose.dev-platform.yaml +++ b/docker-compose.dev-platform.yaml @@ -47,6 +47,35 @@ services: # Where the runner phones home. The daemon injects this into every job; the # runner reaches it across dev-control, never over the public internet. DEV_PLATFORM_RUNNER_BASE_URL: http://middleware:8080 + # The image the middleware derives into every job's policy (served over + # GET /internal/job-policy/:jobId, which the daemon fetches at provision + # time). Without this, jobPolicyConfig never builds and that endpoint + # 503s forever — every DockerBackend provision fails at the first real + # container (i.e. the implement phase; analyze/plan/clarify run without + # one). Same source var as the daemon's own DEV_RUNNER_IMAGES below, so + # both sides always agree on which image a job runs. + DEV_RUNNER_DEFAULT_IMAGE: ${DEV_RUNNER_IMAGE:-ghcr.io/byte5ai/omadia-dev-runner:latest} + # The LLM proxy's model allowlist (spec §5, wireDevPlatform.ts). An empty + # list is a deliberate fail-closed default (config.ts: "always mounted; + # an empty allowlist ⇒ it answers 500 'no LLM policy'"), so this is a + # real per-deployment setting, not a wiring bug -- but its absence looks + # EXACTLY like every earlier gate in this chain from the runner's side: + # the CLI reaches the proxy fine (gates 6/10 fixed that) and gets an + # instant, silent-to-the-runner 500 with zero tokens spent. Comma-separated, + # exact string match (llmProxy.ts: `policy.allowedModels.includes(model)`) + # against whatever `--model` the CLI actually reports in its init message. + DEV_PLATFORM_LLM_ALLOWED_MODELS: ${DEV_PLATFORM_LLM_ALLOWED_MODELS:-claude-opus-4-8[1m],claude-opus-4-8,claude-sonnet-4-8[1m],claude-sonnet-4-8} + # Egress allowlist entries every job gets in ADDITION to the middleware + # host + its own repo's forge host (deriveJobPolicy.ts). Absent by + # default (config.ts), which is correct for a repo needing no package + # install at all -- but for THIS repo (npm workspaces), a job with no + # bootstrap_command auto-detects `npm ci`/`npm install` (bootstrapDetect + # .ts) and, same as any implement-phase agent legitimately running one + # itself, needs a route to the registry or it just hangs retrying + # against a proxy default-deny (found live: "install is stalled, + # node_modules not growing", no clean error -- npm's own resilience + # masks the denial as a hang rather than a fast rejection). + DEV_EGRESS_BASE_ALLOWLIST: ${DEV_EGRESS_BASE_ALLOWLIST:-registry.npmjs.org} # Neutralise any docker engine address a stray `middleware/.env` (loaded via # the base file's env_file) might inject. `environment` wins over env_file, # so these empty values are the last word: the middleware CANNOT be handed a @@ -97,9 +126,18 @@ services: # The image allowlist is the boundary a compromised middleware cannot cross: # it may name a job, never an image. The daemon REFUSES TO BOOT without it. - # Digest-pinned by default (DEV_RUNNER_REQUIRE_DIGEST=0 to relax, locally). DEV_RUNNER_ALLOWED_IMAGES: ${DEV_RUNNER_ALLOWED_IMAGES:-ghcr.io/byte5ai/omadia-dev-runner} DEV_RUNNER_IMAGES: ${DEV_RUNNER_IMAGE:-ghcr.io/byte5ai/omadia-dev-runner:latest} + # Digest-pinned by default (true) — the comment above used to be the ONLY + # place this knob existed; the var was never actually forwarded into the + # container, so `env.DEV_RUNNER_REQUIRE_DIGEST` was always undefined and + # `parseRequireDigest` silently fell back to true. Every locally-built + # image is a floating tag (no digest, no registry to have digest-pinned + # it from), so EVERY provision was refused with the same generic + # "the middleware could not supply the job policy" 502 the runner-image + # gap produced — a second, distinct cause behind the identical symptom. + # Set DEV_RUNNER_REQUIRE_DIGEST=0 in .env to relax this, locally only. + DEV_RUNNER_REQUIRE_DIGEST: ${DEV_RUNNER_REQUIRE_DIGEST:-true} # Pull policy. `always` (the default, and prod on GHCR) re-pulls + digest-pins # + cosign-verifies every image on every provision. `if-not-present` skips the @@ -116,8 +154,21 @@ services: # The daemon refuses to boot on a half-configuration. DEV_RUNNER_EGRESS_PROXY_URL: http://172.28.5.3:3128 DEV_RUNNER_EGRESS_PROXY_CONTROL_URL: http://172.28.4.3:3129 - # The runner reaches the middleware directly, not through the proxy. - DEV_RUNNER_NO_PROXY: middleware,localhost,127.0.0.1 + # The runner does NOT reach the middleware directly -- it CANNOT: job + # containers are created by dind on their own per-job network, which has + # no route to dev-control (where `middleware` actually lives). Only the + # proxy is dual-homed onto both dev-egress (job-reachable) and dev-control + # (middleware-reachable), so phone-home traffic must go THROUGH it, not + # around it. The proxy's own egress policy (egressPolicy.mjs) already + # special-cases exactly this: a request whose host+port equals + # OMADIA_INTERNAL_API_URL (this same dev-egress-proxy's own env, set to + # http://middleware:8080) is allowed regardless of path -- so listing + # "middleware" here to bypass the proxy doesn't skip an unnecessary hop, + # it routes phone-home into a dead end: `getaddrinfo ENOTFOUND middleware` + # from inside the job's network, which is where every real job died after + # the runner-image/digest/token gates were fixed. Only localhost/127.0.0.1 + # (traffic that never leaves the container) belong on this bypass list. + DEV_RUNNER_NO_PROXY: localhost,127.0.0.1 # A lease says "still working"; it can never say "run forever". DEV_RUNNER_MAX_JOB_LIFETIME_MS: ${DEV_RUNNER_MAX_JOB_LIFETIME_MS:-21600000} @@ -144,7 +195,18 @@ services: # --- the nested engine: the only privileged service in the stack ---------- dev-dind: - image: docker:27-dind + # Thin wrapper around docker:27-dind (see middleware/sidecars/dev-dind): + # adds ONE static iptables rule in dind's OWN netns so a nested per-job + # container's direct-connect bypass attempt (confirmed live, 2026-07-29 — + # npm's own proxy resolution occasionally lands on a direct-connect code + # path) fails in milliseconds instead of a multi-minute TCP blackhole. + # dev-engine/dev-egress are already `internal: true`, so the bypass was + # ALWAYS doomed — this only makes the failure deterministic, closing the + # timing window that re-triggers npm's own ExitHandler race (npm/cli#9751). + # Zero capability change to the job container itself. + image: omadia-dev-dind:local + build: + context: middleware/sidecars/dev-dind restart: unless-stopped privileged: true environment: @@ -205,6 +267,17 @@ services: # reach the DATA plane at 172.28.5.3 (dev-egress). Two planes, two networks, # and the daemon never joins the one the jobs are on. ipv4_address: 172.28.4.3 + # The proxy's OWN route to the real internet. dev-egress and dev-control + # are BOTH `internal: true` -- correctly, they must never reach outside -- + # but that left the proxy itself with no path out either, so every job's + # egress request failed DNS resolution before the allowlist/CONNECT logic + # ever ran (`getaddrinfo EAI_AGAIN github.com` from inside this very + # container). The proxy deliberately does NOT join `omadia` for this -- + # sharing the app's own network would make it reachable from (and able to + # reach) middleware/web-ui laterally, which the whole point of a separate + # egress plane exists to avoid -- so this is a THIRD, dedicated network + # whose only member is the proxy. + dev-egress-external: {} # No `ports:` — the proxy is not reachable from the host. volumes: @@ -240,3 +313,9 @@ networks: ipam: config: - subnet: 172.28.5.0/24 + # The proxy's real path to the internet — deliberately NOT internal, and + # deliberately NOT `omadia` (see the service comment above). No pinned + # subnet/address: dev-egress-proxy is this network's only member, and + # nothing else ever needs to address it here. + dev-egress-external: + driver: bridge diff --git a/middleware/packages/dev-runner-shim/src/agentRunner.ts b/middleware/packages/dev-runner-shim/src/agentRunner.ts index 5f4b557b0..d7de13f6f 100644 --- a/middleware/packages/dev-runner-shim/src/agentRunner.ts +++ b/middleware/packages/dev-runner-shim/src/agentRunner.ts @@ -173,6 +173,22 @@ export function buildAgentEnv( if (opts.llmEnvAllowed === true) { if (opts.proxyBaseUrl) env['ANTHROPIC_BASE_URL'] = opts.proxyBaseUrl; if (opts.proxyToken) env['ANTHROPIC_AUTH_TOKEN'] = opts.proxyToken; + // Same reason gitOps.ts's runGit() forwards these to git: the job's + // isolated network has no route to `ANTHROPIC_BASE_URL` (the middleware) + // except through the daemon's egress proxy, and the `claude` CLI is a + // SEPARATE process from this shim -- it does not inherit the shim's own + // process.env, only what buildAgentEnv hands it here. Without these, the + // CLI's first request hangs against an unreachable host with no log + // output at all (the shim never sees a stderr line to translate, + // because the CLI's own network stack is still trying, not failing) -- + // the same undici-needs-NODE_USE_ENV_PROXY behaviour gate 6 already + // established for this shim's own fetch calls applies equally to the + // CLI subprocess, since it is also Node/undici-based. + for (const key of ['HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY', 'http_proxy', 'https_proxy', 'no_proxy']) { + const value = parent[key]; + if (value) env[key] = value; + } + if (parent['HTTP_PROXY'] || parent['HTTPS_PROXY']) env['NODE_USE_ENV_PROXY'] = '1'; } return env; } diff --git a/middleware/packages/dev-runner-shim/src/bootstrapDetect.ts b/middleware/packages/dev-runner-shim/src/bootstrapDetect.ts new file mode 100644 index 000000000..d1e96799d --- /dev/null +++ b/middleware/packages/dev-runner-shim/src/bootstrapDetect.ts @@ -0,0 +1,59 @@ +/** + * Epic #470 W2 — auto-detect a dependency-install command when the repo has no + * explicit `bootstrap_command` configured (`types.ts`'s own doc comment: "null + * = auto-detect at runtime"). Runs shim-side, not server-side: the middleware + * derives job policy before the repo is even cloned, so it has no filesystem to + * inspect — only the runner, once the workspace exists, can look. + * + * Root-level only: this looks at the CLONED REPO ROOT's own manifest/lockfile, + * not any subdirectory. A monorepo with per-workspace-directory manifests (no + * root `package.json`, e.g. `middleware/package.json` + `web-ui/package.json` + * with nothing at root) will not match anything here — that's intentional + * (see `detectBootstrapCommand`'s doc comment) rather than guessing which + * subdirectories matter; those repos need an explicit `bootstrap_command`. + */ + +/** npm-family lockfiles, checked ONLY when `package.json` is also present (see + * `detectBootstrapCommand`) — a lockfile alone is not installable: `npm ci` + * requires both files and fails outright without a manifest. Found live: a + * stray root `package-lock.json` (an 87-byte empty-packages stub, left over + * from before this repo moved to per-workspace-directory manifests) with no + * matching `package.json` made the old file-alone check run `npm ci` anyway + * and fail with exit 254. */ +const NPM_LOCKFILE_CHECKS: readonly { file: string; command: string }[] = [ + { file: 'package-lock.json', command: 'npm ci' }, + { file: 'npm-shrinkwrap.json', command: 'npm ci' }, + { file: 'yarn.lock', command: 'yarn install --frozen-lockfile' }, + { file: 'pnpm-lock.yaml', command: 'pnpm install --frozen-lockfile' }, +]; + +/** Checks with no `package.json`-style prerequisite — each file IS the whole + * signal for its ecosystem. */ +const STANDALONE_CHECKS: readonly { file: string; command: string }[] = [ + { file: 'requirements.txt', command: 'pip install -r requirements.txt' }, + { file: 'Pipfile', command: 'pipenv install' }, + { file: 'Cargo.toml', command: 'cargo fetch' }, + { file: 'go.mod', command: 'go mod download' }, +]; + +/** + * `entries` is the repo root's directory listing. Returns the first matching + * command in priority order (a lockfile beats the bare manifest — `npm ci` + * over `npm install` when both `package-lock.json` and `package.json` are + * present), or `null` when nothing recognizable is there — not every repo + * needs a distinct install step, and an undetectable one is not itself a + * failure. + */ +export function detectBootstrapCommand(entries: readonly string[]): string | null { + const present = new Set(entries); + if (present.has('package.json')) { + for (const check of NPM_LOCKFILE_CHECKS) { + if (present.has(check.file)) return check.command; + } + return 'npm install'; + } + for (const check of STANDALONE_CHECKS) { + if (present.has(check.file)) return check.command; + } + return null; +} diff --git a/middleware/packages/dev-runner-shim/src/eventTranslate.ts b/middleware/packages/dev-runner-shim/src/eventTranslate.ts index 329e09f69..241c08f25 100644 --- a/middleware/packages/dev-runner-shim/src/eventTranslate.ts +++ b/middleware/packages/dev-runner-shim/src/eventTranslate.ts @@ -14,7 +14,8 @@ * | assistant text deltas (coalesced per block)| `log {stream:'agent', text}` | * | `tool_use` block | `tool {name, inputPreview}` (≤2 KB) | * | `tool_result` block | `tool {name, ok, outputPreview}` (≤2 KB) | - * | `result` | `status {state:'agent_done', usage}` | + * | `result` (success) | `status {state:'agent_done', usage}` | + * | `result` (`is_error`/non-'success' subtype)| `status {state:'agent_error', subtype, errorText, usage}` | * * stderr lines are translated separately (`log {stream:'stderr', text}`) by the * agent runner; they never pass through here. @@ -143,6 +144,25 @@ export class CliEventTranslator { ? { costUsd: payload['total_cost_usd'] as number } : {}), }; + // Found live (a job whose CLI process exited non-zero despite an + // apparently-clean `result` line landing right beforehand): a `result` + // line is not automatically success. The CLI's own `subtype` names it + // ('success' | 'error_max_turns' | 'error_during_execution' | ...) and + // `is_error` flags it explicitly — surface that here instead of + // unconditionally reporting `agent_done`, so an error result is visible + // in dev_job_events at the moment it happens rather than only inferable + // later from the process's exit code with no explanation attached. + const subtype = asString(payload['subtype']); + const isError = payload['is_error'] === true || (subtype !== undefined && subtype !== 'success'); + if (isError) { + const errorText = asString(payload['result']); + return this.event('status', { + state: 'agent_error', + ...(subtype !== undefined ? { subtype } : {}), + ...(errorText !== undefined ? { errorText } : {}), + usage, + }); + } return this.event('status', { state: 'agent_done', usage }); } diff --git a/middleware/packages/dev-runner-shim/src/gitOps.ts b/middleware/packages/dev-runner-shim/src/gitOps.ts index 354a58cef..7b3f4b10e 100644 --- a/middleware/packages/dev-runner-shim/src/gitOps.ts +++ b/middleware/packages/dev-runner-shim/src/gitOps.ts @@ -66,6 +66,18 @@ export async function runGit(opts: GitOptions, args: string[], cwd: string): Pro GIT_CONFIG_GLOBAL: '/dev/null', LANG: 'C', }; + // Deployment topology, not a secret -- same category as PATH/HOME above, so + // it belongs on this explicit allowlist too. The job's network has no route + // to a forge host (github.com) except through the daemon's egress proxy; + // without these, git falls back to a direct DNS lookup that always fails + // ("Could not resolve host"). Both spellings: curl (git's HTTPS transport) + // historically only trusts lowercase http_proxy/https_proxy/no_proxy by + // default, but the daemon injects both cases (see policyClient.mjs), so + // forwarding both here keeps this in step with whichever it actually reads. + for (const key of ['HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY', 'http_proxy', 'https_proxy', 'no_proxy']) { + const value = process.env[key]; + if (value) env[key] = value; + } return new Promise((resolve, reject) => { const child = spawn(gitBin, args, { cwd, env, stdio: ['ignore', 'pipe', 'pipe'] }); const out: Buffer[] = []; diff --git a/middleware/packages/dev-runner-shim/src/index.ts b/middleware/packages/dev-runner-shim/src/index.ts index ac4b72696..875ec2348 100644 --- a/middleware/packages/dev-runner-shim/src/index.ts +++ b/middleware/packages/dev-runner-shim/src/index.ts @@ -117,10 +117,17 @@ export async function runShim(env: ShimEnv = readShimEnv(), deps: ShimDeps = {}) // `OMADIA_ANTHROPIC_*` pair is the middleware's own long-lived proxy // secret, so it crosses into the child ONLY when the backend was launched // with the jail acknowledgment and plumbed `OMADIA_LLM_ENV_ALLOWED=true`. - // W1's per-job, short-lived LLM-proxy tokens replace this passthrough. - const proxyBaseUrl = process.env['OMADIA_ANTHROPIC_BASE_URL']?.trim(); - const proxyToken = process.env['OMADIA_ANTHROPIC_AUTH_TOKEN']?.trim(); - if (proxyToken && !env.llmEnvAllowed) { + // W1's per-job, short-lived LLM-proxy tokens replace this passthrough: + // `ANTHROPIC_BASE_URL` (policy-supplied, deriveJobPolicy.ts) plus the + // per-job bearer already on ShimEnv (`jobToken`) ARE that replacement — a + // short-lived, per-job token is a different threat model from W0's + // long-lived secret, so its presence stands in for the jail + // acknowledgment rather than requiring it. + const w1BaseUrl = process.env['ANTHROPIC_BASE_URL']?.trim(); + const proxyBaseUrl = w1BaseUrl || process.env['OMADIA_ANTHROPIC_BASE_URL']?.trim(); + const proxyToken = w1BaseUrl ? env.jobToken : process.env['OMADIA_ANTHROPIC_AUTH_TOKEN']?.trim(); + const llmEnvAllowed = env.llmEnvAllowed || Boolean(w1BaseUrl); + if (!w1BaseUrl && process.env['OMADIA_ANTHROPIC_AUTH_TOKEN']?.trim() && !env.llmEnvAllowed) { log( 'OMADIA_ANTHROPIC_AUTH_TOKEN is set but OMADIA_LLM_ENV_ALLOWED!=true — ' + 'withholding LLM auth from the child (W0 jail acknowledgment missing)', @@ -135,7 +142,7 @@ export async function runShim(env: ShimEnv = readShimEnv(), deps: ShimDeps = {}) cwd: repoDir, homeDir: agentHome, spec, - llmEnvAllowed: env.llmEnvAllowed, + llmEnvAllowed, ...(proxyBaseUrl ? { proxyBaseUrl } : {}), ...(proxyToken ? { proxyToken } : {}), emit, diff --git a/middleware/packages/dev-runner-shim/src/phaseRunner.ts b/middleware/packages/dev-runner-shim/src/phaseRunner.ts index 161648305..d9fd07b10 100644 --- a/middleware/packages/dev-runner-shim/src/phaseRunner.ts +++ b/middleware/packages/dev-runner-shim/src/phaseRunner.ts @@ -9,12 +9,13 @@ */ import { spawn } from 'node:child_process'; -import { lstat, mkdir, readFile, realpath } from 'node:fs/promises'; +import { lstat, mkdir, readdir, readFile, realpath } from 'node:fs/promises'; import path from 'node:path'; import { HomeError } from './homeClient.js'; import { runGit, type GitOptions } from './gitOps.js'; import { runAgent } from './agentRunner.js'; +import { detectBootstrapCommand } from './bootstrapDetect.js'; import { buildPhasePrompt, PHASE_ARTIFACT_ENV, phaseWritesArtifactFile } from './phasePrompts.js'; import { isAgentSessionPhase, @@ -149,15 +150,24 @@ export class PhaseRunner { }; } - /** bootstrap — dependency install as a COMMAND (spec §4), not a CLI session. */ + /** bootstrap — dependency install as a COMMAND (spec §4), not a CLI session. + * An explicit `spec.bootstrap.command` always wins; absent that, auto-detect + * from the cloned repo root (`bootstrapDetect.ts`). Nothing explicit AND + * nothing detectable is not itself a failure — many repos have no separate + * install step — so bootstrap reports `ok: true` and moves on. */ private async runBootstrap(): Promise { - const boot = this.c.spec.bootstrap; - if (!boot?.command) { - return { phase: 'bootstrap', ok: false, error: 'no bootstrap command provisioned for this repo' }; + const explicit = this.c.spec.bootstrap?.command; + const command = explicit ?? (await this.detectBootstrapCommandAtRoot()); + if (!command) { + return { + phase: 'bootstrap', + ok: true, + artifact: { kind: 'bootstrap_report', content: JSON.stringify({ command: null, skipped: true }) }, + }; } - const timeoutMs = boot.timeoutMs ?? DEV_BOOTSTRAP_TIMEOUT_MS; + const timeoutMs = this.c.spec.bootstrap?.timeoutMs ?? DEV_BOOTSTRAP_TIMEOUT_MS; const started = Date.now(); - const result = await runCommand(boot.command, { + const result = await runCommand(command, { cwd: this.c.repoDir, env: bootstrapEnv(this.c.env.workspace), timeoutMs, @@ -165,10 +175,12 @@ export class PhaseRunner { }); const durationMs = Date.now() - started; const report = JSON.stringify({ - command: boot.command, + command, + detected: explicit === undefined, exitCode: result.code, timedOut: result.timedOut, durationMs, + outputTail: result.outputTail, }); if (result.code !== 0) { return { @@ -183,6 +195,15 @@ export class PhaseRunner { return { phase: 'bootstrap', ok: true, artifact: { kind: 'bootstrap_report', content: report } }; } + private async detectBootstrapCommandAtRoot(): Promise { + try { + const entries = await readdir(this.c.repoDir); + return detectBootstrapCommand(entries); + } catch { + return null; + } + } + /** Spawn a fresh `claude -p` session with a FRESH per-phase HOME (no session * state bleeds between phases) and the phase prompt on STDIN. */ private async runSession( @@ -194,14 +215,25 @@ export class PhaseRunner { const homeDir = path.join(this.c.env.workspace, 'home', `${phase}-${sessionIdx}`); await mkdir(homeDir, { recursive: true }); - const proxyBaseUrl = process.env['OMADIA_ANTHROPIC_BASE_URL']?.trim(); - const proxyToken = process.env['OMADIA_ANTHROPIC_AUTH_TOKEN']?.trim(); + // W1: `ANTHROPIC_BASE_URL` (policy-supplied, deriveJobPolicy.ts) plus the + // per-job bearer already on ShimEnv (`jobToken`, required, sourced from + // `OMADIA_JOB_TOKEN`) ARE the "W1's per-job, short-lived LLM-proxy tokens" + // ShimEnv.llmEnvAllowed's own doc comment says replace the W0 passthrough + // entirely -- a short-lived, per-job token is a different threat model + // from W0's long-lived middleware secret, so its presence stands in for + // the W0 jail acknowledgment rather than requiring it. Falls back to the + // legacy OMADIA_ANTHROPIC_* pair (still gated behind llmEnvAllowed) only + // when there is no W1 base URL, i.e. genuinely running under W0. + const w1BaseUrl = process.env['ANTHROPIC_BASE_URL']?.trim(); + const proxyBaseUrl = w1BaseUrl || process.env['OMADIA_ANTHROPIC_BASE_URL']?.trim(); + const proxyToken = w1BaseUrl ? this.c.env.jobToken : process.env['OMADIA_ANTHROPIC_AUTH_TOKEN']?.trim(); + const llmEnvAllowed = this.c.env.llmEnvAllowed || Boolean(w1BaseUrl); const agent = runAgent({ cliBin: this.c.env.cliBin, cwd: this.c.repoDir, homeDir, spec: this.c.spec, - llmEnvAllowed: this.c.env.llmEnvAllowed, + llmEnvAllowed, ...(proxyBaseUrl ? { proxyBaseUrl } : {}), ...(proxyToken ? { proxyToken } : {}), promptOverride: prompt, @@ -256,12 +288,27 @@ export function absorb(acc: Accumulated, phase: DevJobPhase, body: PhaseResultBo // Small helpers. // --------------------------------------------------------------------------- +/** Cap on captured command output — the tail is what matters for diagnosing + * a failure (npm/pip/etc. print their actual error at the end, not the + * start), and unbounded capture risks a memory/artifact-size blowup on a + * verbose or runaway command. */ +const MAX_COMMAND_OUTPUT_BYTES = 4096; + interface CommandResult { code: number; timedOut: boolean; + /** Last MAX_COMMAND_OUTPUT_BYTES of combined stdout+stderr. */ + outputTail: string; } -/** Run a shell command with its own timeout. Used only for `bootstrap`. */ +/** Run a shell command with its own timeout. Used only for `bootstrap`. + * + * Found live: this used to spawn with `stdio: ['ignore','pipe','pipe']` and + * never read either pipe — a failed bootstrap command (e.g. `npm ci` + * exiting 1 after 70s of real, successful-looking network activity) + * reported only an exit code, with the actual reason (npm's own error + * output) silently discarded and unrecoverable, not even via `docker logs` + * (piped streams never reach the container's own stdout/stderr). */ function runCommand( command: string, opts: { cwd: string; env: NodeJS.ProcessEnv; timeoutMs: number; setKill: SetKill }, @@ -272,30 +319,75 @@ function runCommand( env: opts.env, stdio: ['ignore', 'pipe', 'pipe'], }); + let output = ''; + const appendOutput = (chunk: Buffer): void => { + output += chunk.toString('utf8'); + // Keep memory bounded while the command is STILL RUNNING, not just at + // the end — a runaway command must not accumulate unboundedly. + if (output.length > MAX_COMMAND_OUTPUT_BYTES * 2) { + output = output.slice(-MAX_COMMAND_OUTPUT_BYTES); + } + }; + child.stdout?.on('data', appendOutput); + child.stderr?.on('data', appendOutput); let timedOut = false; const timer = setTimeout(() => { timedOut = true; child.kill('SIGKILL'); }, opts.timeoutMs); opts.setKill((signal: NodeJS.Signals = 'SIGTERM') => child.kill(signal)); - child.once('error', () => { + child.once('error', (err) => { clearTimeout(timer); - resolve({ code: -1, timedOut }); + appendOutput(Buffer.from(`\n[spawn error] ${err.message}`)); + resolve({ code: -1, timedOut, outputTail: output.slice(-MAX_COMMAND_OUTPUT_BYTES) }); }); child.once('close', (code) => { clearTimeout(timer); - resolve({ code: code ?? -1, timedOut }); + resolve({ code: code ?? -1, timedOut, outputTail: output.slice(-MAX_COMMAND_OUTPUT_BYTES) }); }); }); } -/** Minimal hermetic env for the bootstrap command — no LLM auth, job-scoped HOME. */ +/** + * Minimal hermetic env for the bootstrap command — no LLM auth, job-scoped + * HOME. "Minimal" deliberately excludes ANTHROPIC_* and OMADIA_JOB_TOKEN and + * any other LLM-session secret (bootstrap is a plain shell command, not a CLI + * session — it has no business seeing them). It must NOT exclude proxy + * config, though: bootstrap is a spawned child of THIS shim process, which + * does not inherit the shim's own process.env automatically (same reason + * agentRunner.ts's buildAgentEnv and gitOps.ts's runGit both forward these + * explicitly) — and the job's isolated network has no route to ANYTHING + * except through the daemon's egress proxy. Confirmed live (2026-07-29, + * epic #470): without this, `env` inside bootstrap showed ONLY + * PATH/HOME/LANG/PWD — no HTTPS_PROXY at all — so npm (or any tool) + * attempted direct connections for its entire run, which an unrelated + * infra fix (the dev-dind egress guard) then correctly rejected, but the + * REAL bug was here: bootstrap never had a route to succeed in the first + * place. This was very likely the root cause of the "Exit handler never + * called!" investigation's entire failure pattern, not any npm-internal + * proxy-bypass behavior. + */ function bootstrapEnv(workspace: string): NodeJS.ProcessEnv { - return { + const env: NodeJS.ProcessEnv = { PATH: process.env['PATH'] ?? '/usr/bin:/bin', HOME: path.join(workspace, 'home'), LANG: process.env['LANG'] ?? 'C.UTF-8', }; + for (const key of [ + 'HTTP_PROXY', + 'HTTPS_PROXY', + 'NO_PROXY', + 'http_proxy', + 'https_proxy', + 'no_proxy', + 'npm_config_proxy', + 'npm_config_https_proxy', + 'npm_config_noproxy', + ]) { + const value = process.env[key]; + if (value) env[key] = value; + } + return env; } /** Max artifact size the shim will read back (Forge #2 — bound before the 4 MiB diff --git a/middleware/packages/dev-runner-shim/test/agentRunner.test.ts b/middleware/packages/dev-runner-shim/test/agentRunner.test.ts index 3ea863ff7..b3e2d9a1e 100644 --- a/middleware/packages/dev-runner-shim/test/agentRunner.test.ts +++ b/middleware/packages/dev-runner-shim/test/agentRunner.test.ts @@ -133,6 +133,39 @@ describe('buildAgentEnv — allowlist, not scrub', () => { assert.equal(allowed['ANTHROPIC_AUTH_TOKEN'], 'bearer'); }); + it('forwards HTTP_PROXY/NO_PROXY + NODE_USE_ENV_PROXY into the CLI child, same reason gitOps.ts forwards them to git', () => { + // The `claude` CLI is a SEPARATE process — it does not inherit this + // shim's own process.env, only what buildAgentEnv hands it. The job's + // isolated network has no route to ANTHROPIC_BASE_URL except through the + // daemon's egress proxy, and the CLI is Node/undici-based like the + // shim's own fetch calls, so it needs NODE_USE_ENV_PROXY too (gate 6's + // finding applies here as much as to homeClient.ts's fetch). + process.env['HTTP_PROXY'] = 'http://job:token@172.28.5.3:3128/'; + process.env['HTTPS_PROXY'] = 'http://job:token@172.28.5.3:3128/'; + process.env['NO_PROXY'] = 'localhost,127.0.0.1'; + try { + const withoutAck = buildAgentEnv({ cwd: '/tmp/x', proxyBaseUrl: 'http://proxy', proxyToken: 'bearer' }); + assert.equal(withoutAck['HTTP_PROXY'], undefined, 'proxy vars stay scoped to the LLM-routing gate, same as the auth pair'); + + const withAck = buildAgentEnv({ cwd: '/tmp/x', proxyBaseUrl: 'http://proxy', proxyToken: 'bearer', llmEnvAllowed: true }); + assert.equal(withAck['HTTP_PROXY'], 'http://job:token@172.28.5.3:3128/'); + assert.equal(withAck['HTTPS_PROXY'], 'http://job:token@172.28.5.3:3128/'); + assert.equal(withAck['NO_PROXY'], 'localhost,127.0.0.1'); + assert.equal(withAck['NODE_USE_ENV_PROXY'], '1'); + } finally { + delete process.env['HTTP_PROXY']; + delete process.env['HTTPS_PROXY']; + delete process.env['NO_PROXY']; + } + }); + + it('omits proxy keys entirely when none are configured (no empty-string env pollution)', () => { + const env = buildAgentEnv({ cwd: '/tmp/x', proxyBaseUrl: 'http://proxy', proxyToken: 'bearer', llmEnvAllowed: true }); + for (const k of ['HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY', 'http_proxy', 'https_proxy', 'no_proxy', 'NODE_USE_ENV_PROXY']) { + assert.equal(env[k], undefined, `${k} must be absent, not an empty string`); + } + }); + it('HOME is job-scoped and the parent HOME never appears in the child env', () => { const prev = process.env['HOME']; const canary = '/tmp/parent-home-canary-9f3a1c'; diff --git a/middleware/packages/dev-runner-shim/test/bootstrapDetect.test.ts b/middleware/packages/dev-runner-shim/test/bootstrapDetect.test.ts new file mode 100644 index 000000000..c21af48b0 --- /dev/null +++ b/middleware/packages/dev-runner-shim/test/bootstrapDetect.test.ts @@ -0,0 +1,77 @@ +import { describe, it } from 'node:test'; +import { strict as assert } from 'node:assert'; + +import { detectBootstrapCommand } from '../src/bootstrapDetect.js'; + +describe('detectBootstrapCommand', () => { + it('returns null for an empty directory — not every repo needs a bootstrap step', () => { + assert.equal(detectBootstrapCommand([]), null); + }); + + it('returns null when nothing recognizable is present', () => { + assert.equal(detectBootstrapCommand(['README.md', 'src', '.git']), null); + }); + + it('detects npm ci from package-lock.json', () => { + assert.equal(detectBootstrapCommand(['package.json', 'package-lock.json']), 'npm ci'); + }); + + it('detects npm ci from npm-shrinkwrap.json', () => { + assert.equal(detectBootstrapCommand(['package.json', 'npm-shrinkwrap.json']), 'npm ci'); + }); + + it('detects yarn from yarn.lock', () => { + assert.equal(detectBootstrapCommand(['package.json', 'yarn.lock']), 'yarn install --frozen-lockfile'); + }); + + it('detects pnpm from pnpm-lock.yaml', () => { + assert.equal(detectBootstrapCommand(['package.json', 'pnpm-lock.yaml']), 'pnpm install --frozen-lockfile'); + }); + + it('falls back to npm install for a bare package.json with no lockfile', () => { + assert.equal(detectBootstrapCommand(['package.json']), 'npm install'); + }); + + it('prefers a lockfile over the bare manifest when both are present', () => { + assert.equal(detectBootstrapCommand(['package.json', 'package-lock.json', 'yarn.lock']), 'npm ci'); + }); + + it('detects pip from requirements.txt', () => { + assert.equal(detectBootstrapCommand(['requirements.txt']), 'pip install -r requirements.txt'); + }); + + it('detects pipenv from Pipfile', () => { + assert.equal(detectBootstrapCommand(['Pipfile']), 'pipenv install'); + }); + + it('detects cargo from Cargo.toml', () => { + assert.equal(detectBootstrapCommand(['Cargo.toml']), 'cargo fetch'); + }); + + it('detects go modules from go.mod', () => { + assert.equal(detectBootstrapCommand(['go.mod']), 'go mod download'); + }); + + it('does not run npm ci from a lockfile with no matching package.json', () => { + // Regression: found live against byte5ai/omadia's actual repo root — a + // stray, empty-packages package-lock.json survives from before the repo + // moved to per-workspace-directory manifests (middleware/package.json, + // web-ui/package.json), with no root package.json at all. `npm ci` + // fundamentally requires both files; running it anyway failed with a + // real, reported exit code (254) instead of gracefully skipping. + assert.equal(detectBootstrapCommand(['package-lock.json', 'README.md']), null); + }); + + it('does not run yarn/pnpm from a lockfile with no matching package.json either', () => { + assert.equal(detectBootstrapCommand(['yarn.lock']), null); + assert.equal(detectBootstrapCommand(['pnpm-lock.yaml']), null); + }); + + it('does not detect a manifest sitting in a subdirectory — root only', () => { + // Directory listings are flat (one level), so this case is really "the + // caller only passed root entries" — documented behavior, not a bug to + // fix here: a monorepo with per-workspace manifests needs an explicit + // bootstrap_command (see this module's doc comment). + assert.equal(detectBootstrapCommand(['middleware', 'web-ui', 'README.md']), null); + }); +}); diff --git a/middleware/packages/dev-runner-shim/test/eventTranslate.test.ts b/middleware/packages/dev-runner-shim/test/eventTranslate.test.ts index 304655738..147deff5b 100644 --- a/middleware/packages/dev-runner-shim/test/eventTranslate.test.ts +++ b/middleware/packages/dev-runner-shim/test/eventTranslate.test.ts @@ -74,6 +74,51 @@ describe('CliEventTranslator — event table', () => { assert.deepEqual(done?.payload, { state: 'agent_done', usage: { tokensIn: 12, tokensOut: 34, costUsd: 0.05 } }); }); + it('result with subtype:"success" → still status agent_done (explicit success is not flagged)', () => { + const events = drain([ + JSON.stringify({ type: 'result', subtype: 'success', usage: { input_tokens: 1, output_tokens: 1 } }), + ]); + const done = events.find((e) => e.type === 'status'); + assert.equal(done?.payload['state'], 'agent_done'); + }); + + it('result with a non-success subtype → status agent_error, not agent_done', () => { + // Regression: found live — a job whose CLI process exited non-zero despite + // a `result` line landing right beforehand that the OLD unconditional + // mapping would have reported as a clean agent_done, masking the failure + // from dev_job_events until the exit code contradicted it much later. + const events = drain([ + JSON.stringify({ + type: 'result', + subtype: 'error_max_turns', + result: 'exceeded max turns', + usage: { input_tokens: 12, output_tokens: 34 }, + total_cost_usd: 0.05, + }), + ]); + const done = events.find((e) => e.type === 'status'); + assert.deepEqual(done?.payload, { + state: 'agent_error', + subtype: 'error_max_turns', + errorText: 'exceeded max turns', + usage: { tokensIn: 12, tokensOut: 34, costUsd: 0.05 }, + }); + }); + + it('result with is_error:true (no subtype) → status agent_error', () => { + const events = drain([JSON.stringify({ type: 'result', is_error: true, usage: {} })]); + const done = events.find((e) => e.type === 'status'); + assert.equal(done?.payload['state'], 'agent_error'); + assert.equal(done?.payload['subtype'], undefined); + }); + + it('result with neither is_error nor a result-text field omits errorText rather than a placeholder', () => { + const events = drain([JSON.stringify({ type: 'result', subtype: 'error_during_execution', usage: {} })]); + const done = events.find((e) => e.type === 'status'); + assert.equal(done?.payload['state'], 'agent_error'); + assert.equal('errorText' in (done?.payload ?? {}), false); + }); + it('truncates a large input preview to the 2 KB cap', () => { const big = 'x'.repeat(5000); const [e] = drain([ diff --git a/middleware/packages/dev-runner-shim/test/gitOps.test.ts b/middleware/packages/dev-runner-shim/test/gitOps.test.ts index 919f8e7b3..67e3aea0f 100644 --- a/middleware/packages/dev-runner-shim/test/gitOps.test.ts +++ b/middleware/packages/dev-runner-shim/test/gitOps.test.ts @@ -304,4 +304,37 @@ describe('runGit — hermetic environment', () => { delete process.env['SHIM_TEST_LEAK_CANARY']; } }); + + it('DOES forward the proxy vars (deployment topology, not a secret) — otherwise a forge host is unreachable', async () => { + // The job's network has no route to github.com except through the daemon's + // egress proxy (same reason node's own fetch needs NODE_USE_ENV_PROXY). + // Without this, git falls back to a direct DNS lookup that always fails. + process.env['HTTP_PROXY'] = 'http://proxy.example:3128/'; + process.env['HTTPS_PROXY'] = 'http://proxy.example:3128/'; + process.env['NO_PROXY'] = 'localhost,127.0.0.1'; + process.env['http_proxy'] = 'http://proxy.example:3128/'; + try { + await cloneAtBaseSha(baseOpts(), { cloneUrl: CLONE_URL, defaultBranch: 'main', baseSha: '' }); + const clone = readLog().find((r) => r.sub === 'clone'); + assert.ok(clone, 'clone ran'); + assert.equal(clone.env['HTTP_PROXY'], 'http://proxy.example:3128/'); + assert.equal(clone.env['HTTPS_PROXY'], 'http://proxy.example:3128/'); + assert.equal(clone.env['NO_PROXY'], 'localhost,127.0.0.1'); + assert.equal(clone.env['http_proxy'], 'http://proxy.example:3128/'); + } finally { + delete process.env['HTTP_PROXY']; + delete process.env['HTTPS_PROXY']; + delete process.env['NO_PROXY']; + delete process.env['http_proxy']; + } + }); + + it('omits proxy keys entirely when none are configured (no empty-string env pollution)', async () => { + await cloneAtBaseSha(baseOpts(), { cloneUrl: CLONE_URL, defaultBranch: 'main', baseSha: '' }); + const clone = readLog().find((r) => r.sub === 'clone'); + assert.ok(clone, 'clone ran'); + for (const k of ['HTTP_PROXY', 'HTTPS_PROXY', 'NO_PROXY', 'http_proxy', 'https_proxy', 'no_proxy']) { + assert.equal(clone.env[k], undefined, `${k} must be absent, not an empty string`); + } + }); }); diff --git a/middleware/packages/dev-runner-shim/test/index.test.ts b/middleware/packages/dev-runner-shim/test/index.test.ts index ed2ab6af5..620612a45 100644 --- a/middleware/packages/dev-runner-shim/test/index.test.ts +++ b/middleware/packages/dev-runner-shim/test/index.test.ts @@ -222,6 +222,24 @@ describe('runShim — LLM auth gate + job-scoped HOME', () => { assert.equal(childEnv['ANTHROPIC_BASE_URL'], 'http://proxy.internal'); assert.equal(childEnv['HOME'], path.join(ws, 'home'), 'child HOME is a fresh dir inside the workspace'); }); + + it('W1: forwards LLM auth from the policy-supplied ANTHROPIC_BASE_URL + ShimEnv.jobToken, with NO jail acknowledgment', async () => { + // The docker backend's real path: deriveJobPolicy.ts sets plain + // ANTHROPIC_BASE_URL (no OMADIA_ prefix), and there is no + // OMADIA_ANTHROPIC_AUTH_TOKEN at all -- the per-job jobToken already on + // ShimEnv IS the bearer the LLM proxy (llmProxy.ts) resolves the calling + // job from. llmEnvAllowed stays false: the short-lived per-job token + // stands in for the W0 jail acknowledgment rather than requiring it. + process.env['ANTHROPIC_BASE_URL'] = 'http://middleware:8080/api/v1/dev-runner/llm'; + await writeFakeGit(false); + const home = new FakeHome(makeSpec()); + const code = await runShim({ ...env, llmEnvAllowed: false, jobToken: 'djr_w1-token' }, { home, gitBin, log: () => {} }); + assert.equal(code, 0); + const childEnv = await readEnvDump(); + assert.equal(childEnv['ANTHROPIC_AUTH_TOKEN'], 'djr_w1-token', 'the per-job bearer, not a middleware secret'); + assert.equal(childEnv['ANTHROPIC_BASE_URL'], 'http://middleware:8080/api/v1/dev-runner/llm'); + delete process.env['ANTHROPIC_BASE_URL']; + }); }); describe('runShim — wall-clock budget', () => { diff --git a/middleware/packages/dev-runner-shim/test/phaseLoop.test.ts b/middleware/packages/dev-runner-shim/test/phaseLoop.test.ts index a156bc4a4..a7d6a0c74 100644 --- a/middleware/packages/dev-runner-shim/test/phaseLoop.test.ts +++ b/middleware/packages/dev-runner-shim/test/phaseLoop.test.ts @@ -8,7 +8,7 @@ import { describe, it, beforeEach, afterEach } from 'node:test'; import { strict as assert } from 'node:assert'; -import { mkdtemp, rm, writeFile, chmod, readdir, readFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, rm, writeFile, chmod, readdir, readFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -115,7 +115,7 @@ async function writeFakeCli(): Promise { const fs = require('fs'); const path = require('path'); process.stdin.resume(); process.stdin.on('end', () => { const artifact = process.env.OMADIA_PHASE_ARTIFACT; - try { fs.writeFileSync(path.join(process.env.HOME, 'ran.json'), JSON.stringify({ home: process.env.HOME, artifact: artifact || null })); } catch {} + try { fs.writeFileSync(path.join(process.env.HOME, 'ran.json'), JSON.stringify({ home: process.env.HOME, artifact: artifact || null, anthropicAuthToken: process.env.ANTHROPIC_AUTH_TOKEN || null, anthropicBaseUrl: process.env.ANTHROPIC_BASE_URL || null })); } catch {} if (artifact) { const m = /artifact-(.+?)-\\d+\\.json$/.exec(path.basename(artifact)); const phase = m ? m[1] : ''; @@ -268,4 +268,178 @@ describe('runPhasedShim — bootstrap runs as a command, not a CLI session', () const homes = await listSessionHomes(); assert.deepEqual(homes, [], 'bootstrap starts no agent session'); }); + + it('forwards proxy env vars into the bootstrap command, but never LLM/job-auth secrets', async () => { + // Regression: found live (epic #470, 2026-07-29) -- bootstrapEnv() built + // an env of ONLY PATH/HOME/LANG, so a bootstrap command had literally no + // route to anything (the job's isolated network has no path except + // through the daemon's egress proxy). A real npm ci spent its entire + // ~240s budget attempting doomed direct connections instead. Bootstrap + // MUST see the proxy vars (same reason agentRunner.ts's buildAgentEnv + // and gitOps.ts's runGit both forward them) while staying "hermetic" + // about anything LLM-session-specific. + const originalEnv = { ...process.env }; + process.env['HTTPS_PROXY'] = 'http://job-id:token@egress-proxy:3128/'; + process.env['HTTP_PROXY'] = 'http://job-id:token@egress-proxy:3128/'; + process.env['NO_PROXY'] = 'localhost,127.0.0.1'; + process.env['npm_config_https_proxy'] = 'http://job-id:token@egress-proxy:3128/'; + process.env['npm_config_noproxy'] = 'localhost,127.0.0.1'; + // Something bootstrap must NEVER see, to prove this isn't just "forward everything". + process.env['ANTHROPIC_API_KEY'] = 'sk-this-must-not-leak-into-bootstrap'; + try { + const spec = makeSpec({ + phaseContext: { phase: 'bootstrap' }, + bootstrap: { command: 'env', timeoutMs: 30_000 }, + }); + const home = new ScriptedHome(spec, [{ directive: 'done' }]); + await runPhasedShim(env, { home, gitBin, log: () => {} }); + + const boot = home.phaseResults[0]; + const content = boot?.artifact?.content ?? ''; + assert.match(content, /HTTPS_PROXY=http:\/\/job-id:token@egress-proxy:3128/); + assert.match(content, /HTTP_PROXY=http:\/\/job-id:token@egress-proxy:3128/); + assert.match(content, /NO_PROXY=localhost,127\.0\.0\.1/); + assert.match(content, /npm_config_https_proxy=http:\/\/job-id:token@egress-proxy:3128/); + assert.match(content, /npm_config_noproxy=localhost,127\.0\.0\.1/); + assert.doesNotMatch(content, /ANTHROPIC_API_KEY/, 'bootstrap stays hermetic about LLM-session secrets'); + } finally { + process.env = originalEnv; + } + }); + + it('captures the command\'s own stdout+stderr into the report, not just its exit code', async () => { + // Regression: found live -- a real `npm ci` failure inside a job + // container reported only `exitCode:1` with zero further detail; the + // command's own output was silently discarded (piped but never read), + // unrecoverable even via `docker logs` (piped streams never reach the + // container's own stdout/stderr). + const spec = makeSpec({ + phaseContext: { phase: 'bootstrap' }, + bootstrap: { + command: 'echo "line one to stdout"; echo "line two to stderr" 1>&2; exit 1', + timeoutMs: 30_000, + }, + }); + const home = new ScriptedHome(spec, [{ directive: 'failed', reason: 'x' }]); + await runPhasedShim(env, { home, gitBin, log: () => {} }); + + const boot = home.phaseResults[0]; + assert.equal(boot?.ok, false); + const content = boot?.artifact?.content ?? ''; + assert.match(content, /"exitCode":1/); + assert.match(content, /line one to stdout/, 'stdout was captured'); + assert.match(content, /line two to stderr/, 'stderr was captured too'); + }); + + it('caps captured output to a bounded tail rather than growing unboundedly', async () => { + const spec = makeSpec({ + phaseContext: { phase: 'bootstrap' }, + bootstrap: { + // Print well past the cap, then a distinctive marker at the very + // end -- the tail (not the head) is what a real npm/pip failure + // needs, since the actual error line comes last. + command: 'for i in $(seq 1 20000); do printf "x"; done; printf "\\nTHE-ACTUAL-ERROR-IS-HERE\\n"', + timeoutMs: 30_000, + }, + }); + const home = new ScriptedHome(spec, [{ directive: 'done' }]); + await runPhasedShim(env, { home, gitBin, log: () => {} }); + + const boot = home.phaseResults[0]; + const parsed = JSON.parse(boot?.artifact?.content ?? '{}'); + assert.ok(parsed.outputTail.length < 20000, 'the captured tail is bounded, not the full 20k+ bytes'); + assert.match(parsed.outputTail, /THE-ACTUAL-ERROR-IS-HERE/, 'the end of the output (where the real error lives) survives truncation'); + }); + + it('auto-detects a command from the cloned repo root when none is provisioned', async () => { + // repoDir is `/repo` (gitOps.ts REPO_DIRNAME) — pre-seed it + // before the fake clone step runs; clone only adds `.git`, it never wipes + // the directory, so this file is still there when bootstrap reads it. + const repoDir = path.join(ws, 'repo'); + await mkdir(repoDir, { recursive: true }); + // A lockfile alone is not enough (bootstrapDetect.ts requires package.json + // too — `npm ci` needs both, found live as a real crash against a repo + // with a stray root lockfile and no root manifest). + await writeFile(path.join(repoDir, 'package.json'), '{}'); + await writeFile(path.join(repoDir, 'package-lock.json'), '{}'); + // The detected command runs with repoDir as cwd — prove that by having it + // write a marker INSIDE repoDir via a real shell command substituted in + // for the real package manager (this test only proves detection + exec, + // not that npm itself is installed in the test sandbox). + await writeFile(path.join(repoDir, 'npm'), `#!${process.execPath}\nrequire('fs').writeFileSync('bootstrap-detected-ran', '');\n`); + await chmod(path.join(repoDir, 'npm'), 0o755); + + const spec = makeSpec({ phaseContext: { phase: 'bootstrap' } }); // no explicit `bootstrap` field + const home = new ScriptedHome(spec, [{ directive: 'done' }]); + // bootstrapEnv() (phaseRunner.ts) reads PATH from the real process env at + // call time — prepend repoDir so the detected `npm ci` resolves to our fake + // npm, then restore it so this doesn't leak into other tests. + const originalPath = process.env['PATH']; + process.env['PATH'] = `${repoDir}:${originalPath ?? ''}`; + let code: number; + try { + code = await runPhasedShim(env, { home, gitBin, log: () => {} }); + } finally { + process.env['PATH'] = originalPath; + } + assert.equal(code, 0); + + const boot = home.phaseResults[0]; + assert.equal(boot?.phase, 'bootstrap'); + assert.equal(boot?.ok, true); + assert.match(boot?.artifact?.content ?? '', /"command":"npm ci"/, 'detected npm ci from package-lock.json'); + assert.match(boot?.artifact?.content ?? '', /"detected":true/); + const ran = await readFile(path.join(repoDir, 'bootstrap-detected-ran'), 'utf8').then(() => true).catch(() => false); + assert.ok(ran, 'the auto-detected command actually ran with repoDir as cwd'); + }); + + it('skips gracefully (ok:true) when nothing is provisioned and nothing is detectable', async () => { + const repoDir = path.join(ws, 'repo'); + await mkdir(repoDir, { recursive: true }); // empty — no manifest of any kind + + const spec = makeSpec({ phaseContext: { phase: 'bootstrap' } }); + const home = new ScriptedHome(spec, [{ directive: 'done' }]); + const code = await runPhasedShim(env, { home, gitBin, log: () => {} }); + assert.equal(code, 0); + + const boot = home.phaseResults[0]; + assert.equal(boot?.phase, 'bootstrap'); + assert.equal(boot?.ok, true, 'an undetectable bootstrap is a skip, not a failure'); + assert.match(boot?.artifact?.content ?? '', /"command":null/); + assert.match(boot?.artifact?.content ?? '', /"skipped":true/); + }); +}); + +describe('runPhasedShim — W1 LLM auth passthrough (the docker backend\'s real path)', () => { + afterEach(() => { + delete process.env['ANTHROPIC_BASE_URL']; + }); + + it('forwards the policy-supplied ANTHROPIC_BASE_URL + ShimEnv.jobToken into each phase session, with NO jail acknowledgment', async () => { + // deriveJobPolicy.ts sets plain ANTHROPIC_BASE_URL (no OMADIA_ prefix) on + // the container; there is no OMADIA_ANTHROPIC_AUTH_TOKEN at all for a real + // docker job — the per-job jobToken already on ShimEnv is the bearer the + // LLM proxy resolves the calling job from (llmProxy.ts). llmEnvAllowed + // stays false in the fixture: the short-lived per-job token stands in for + // the W0 jail acknowledgment rather than requiring it. + process.env['ANTHROPIC_BASE_URL'] = 'http://middleware:8080/api/v1/dev-runner/llm'; + const home = new ScriptedHome(makeSpec(), [ + { directive: 'next', phase: 'plan' }, + { directive: 'next', phase: 'clarify' }, + { directive: 'park' }, + ]); + const code = await runPhasedShim({ ...env, jobToken: 'djr_w1-gated-token' }, { home, gitBin, log: () => {} }); + assert.equal(code, 0, 'park exits 0'); + + const homes = await listSessionHomes(); + assert.equal(homes.length, 3, 'one fresh HOME per phase'); + for (const h of homes) { + const ran = JSON.parse(await readFile(path.join(ws, 'home', h, 'ran.json'), 'utf8')) as { + anthropicAuthToken: string | null; + anthropicBaseUrl: string | null; + }; + assert.equal(ran.anthropicAuthToken, 'djr_w1-gated-token', `${h}: the per-job bearer, not a middleware secret`); + assert.equal(ran.anthropicBaseUrl, 'http://middleware:8080/api/v1/dev-runner/llm', `${h}`); + } + }); }); diff --git a/middleware/sidecars/dev-dind/Dockerfile b/middleware/sidecars/dev-dind/Dockerfile new file mode 100644 index 000000000..41d1f1ee6 --- /dev/null +++ b/middleware/sidecars/dev-dind/Dockerfile @@ -0,0 +1,9 @@ +# Epic #470 — thin wrapper around the official docker:dind image, adding the +# deterministic egress-guard entrypoint (see entrypoint.sh). Everything else +# (dockerd itself, TLS cert generation, iptables) is the base image unchanged. +FROM docker:27-dind + +COPY entrypoint.sh /usr/local/bin/omadia-dind-entrypoint.sh +RUN chmod +x /usr/local/bin/omadia-dind-entrypoint.sh + +ENTRYPOINT ["/usr/local/bin/omadia-dind-entrypoint.sh"] diff --git a/middleware/sidecars/dev-dind/entrypoint.sh b/middleware/sidecars/dev-dind/entrypoint.sh new file mode 100644 index 000000000..8f527cf1b --- /dev/null +++ b/middleware/sidecars/dev-dind/entrypoint.sh @@ -0,0 +1,57 @@ +#!/bin/sh +# Epic #470 — deterministic fail-fast for any nested (per-job) container that +# somehow attempts a direct connection instead of going through the egress +# proxy at 172.28.5.3 (dev-egress). Confirmed live (2026-07-29): dev-dind's +# own two networks (dev-engine, dev-egress) are BOTH `internal: true`, so a +# bypass attempt was already structurally incapable of reaching the real +# internet — but depending on kernel/network state, the failure mode varied +# between an instant ENETUNREACH and a silent multi-minute TCP blackhole +# (observed: ~240s). That variable stall, not the bypass itself, is what +# re-triggered npm's own confirmed ExitHandler re-entrancy race (npm/cli#9751 +# — the same bug class behind the original EAI_AGAIN-driven crash this whole +# investigation started from). +# +# This adds a small, STATEFUL iptables ruleset in dev-dind's OWN network +# namespace (already `privileged: true` — no new capability granted +# anywhere): any FORWARDED packet — i.e. traffic dind is relaying from a +# nested per-job container, never dind's own process-level traffic, which +# uses OUTPUT, not FORWARD, and is untouched by this — gets REJECTed (not +# silently dropped) UNLESS it belongs to an already-established connection +# (conntrack ESTABLISHED,RELATED — required for the proxy's own RETURN +# traffic, whose destination is the job container's per-job IP, never the +# proxy's own subnet) or is headed to the proxy's own network (172.28.5.0/24, +# dev-egress, for the connection's initiating leg). Everything else fails in +# milliseconds instead of minutes. Zero change to the job container's own +# security clamp (CapDrop: ALL, no-new-privileges, single NetworkMode) — +# this lives entirely one layer down, in dind's netns. +set -eu + +# Start dockerd via the base image's own entrypoint, in the background, so +# DOCKER-USER (created by dockerd itself on boot) exists before we touch it. +/usr/local/bin/dockerd-entrypoint.sh "$@" & +DOCKERD_PID=$! + +# Poll for DOCKER-USER rather than a fixed sleep — dockerd's own boot time +# varies (image pulls, TLS cert generation). +until iptables -L DOCKER-USER >/dev/null 2>&1; do + sleep 0.2 +done + +# Idempotent: a restart of this container must not stack duplicate rules. +iptables -N OMADIA-EGRESS-GUARD 2>/dev/null || true +iptables -F OMADIA-EGRESS-GUARD +# RETURN-leg traffic for an ALREADY-established connection (proxy -> job +# container: TCP ACKs, the CONNECT response, tunnel data) is forwarded with +# its destination being the JOB CONTAINER's own per-job-network IP, never +# 172.28.5.0/24 — a destination-only rule rejects that return traffic too, +# breaking every legitimate proxy-bound connection after its first packet. +# Confirmed live (2026-07-29): with only the destination rule, the shim's +# own phone-home fetch failed instantly on every attempt; flushing the chain +# entirely fixed it immediately. This conntrack rule must come first. +iptables -A OMADIA-EGRESS-GUARD -m conntrack --ctstate ESTABLISHED,RELATED -j RETURN +iptables -A OMADIA-EGRESS-GUARD -d 172.28.5.0/24 -j RETURN +iptables -A OMADIA-EGRESS-GUARD -j REJECT --reject-with icmp-net-unreachable +iptables -C DOCKER-USER -j OMADIA-EGRESS-GUARD 2>/dev/null \ + || iptables -I DOCKER-USER 1 -j OMADIA-EGRESS-GUARD + +wait "$DOCKERD_PID" diff --git a/middleware/sidecars/dev-runner-daemon/src/clamp.mjs b/middleware/sidecars/dev-runner-daemon/src/clamp.mjs index 0175bae03..f11c713fa 100644 --- a/middleware/sidecars/dev-runner-daemon/src/clamp.mjs +++ b/middleware/sidecars/dev-runner-daemon/src/clamp.mjs @@ -21,6 +21,17 @@ * daemon-owned keys injected, egress canonicalised. This module does NOT re-derive * policy; it enforces the CONTAINER shape and refuses anything the clamp forbids * with a `spec_rejected`-shaped error rather than silently granting or dropping it. + * + * ONE exception to "does not re-derive policy": whether a floating tag is allowed + * at all. That is the operator's `DEV_RUNNER_REQUIRE_DIGEST` posture, and the clamp + * used to hardcode it ON — so `DEV_RUNNER_REQUIRE_DIGEST=0`, the documented local + * escape hatch, was a NO-OP and every locally-built (`docker load`ed, registry-less, + * therefore un-pinnable) image was refused here after the policy client had already + * been told to allow it. Two enforcement points reading the same posture is + * defence-in-depth; one of them ignoring it is a contradiction. So the posture is + * now passed in EXPLICITLY, defaulting to ON so the clamp fails closed if a caller + * forgets to thread it. What no posture relaxes: a digest that is PRESENT must be + * a real content address. */ /** @@ -33,9 +44,8 @@ import { parseImageReference } from './policyClient.mjs'; * A valid content-address digest: `algorithm:hex`, ≥32 hex chars — a stub like * `sha256:abc` is refused. Mirrors `policyClient`'s internal `DIGEST_RE`; the * `netClassify`↔`ssrfGuard` parity test is the model for keeping such copies - * honest, but a floating-tag reject here is defence-in-depth: the policy client - * already enforces the digest when `DEV_RUNNER_REQUIRE_DIGEST` is on (default), - * so this is the last line if that knob is ever turned off. + * honest. This shape check is UNCONDITIONAL: `DEV_RUNNER_REQUIRE_DIGEST` decides + * whether a digest is required, never whether a malformed one is tolerated. */ const DIGEST_RE = /^[a-z0-9]+(?:[.+_-][a-z0-9]+)*:[0-9a-f]{32,}$/; @@ -209,22 +219,35 @@ export function jobVolumeName(jobId) { * @param {string} args.volumeName Per-job workspace volume; the ONLY bind. * @param {string} args.createdBy Principal recorded in the `createdBy` label. * @param {ClampLimits} args.limits Resolved resource bounds. + * @param {boolean} [args.requireDigest] The operator's `DEV_RUNNER_REQUIRE_DIGEST` + * posture — must the image be digest-pinned? Defaults to TRUE (prod posture), so + * omitting it fails closed. Only an explicit `false` admits a floating tag, and + * only for the local-dev shape the knob exists for: an image `docker load`ed into + * the engine, with no registry to have pinned it from. * @param {boolean} [args.dockerInJob] Opt-in DinD (spec §8): the job reaches its * per-job sidecar over TLS. Adds the DOCKER_* env and a READ-ONLY certs bind — * and nothing else. Absent/false ⇒ byte-identical to the plain clamp. * @returns {import('dockerode').ContainerCreateOptions} */ export function buildContainerCreateOptions(args) { - const { jobId, policy, leaseExpiresAt, networkName, volumeName, createdBy, limits } = args; + const { jobId, policy, leaseExpiresAt, networkName, volumeName, createdBy, limits, extraHosts } = args; const dockerInJob = args.dockerInJob === true; + // Fail closed: only an explicit `false` relaxes the digest requirement. + const requireDigest = args.requireDigest !== false; - // (d) Canonicalise, THEN classify: the image must be digest-pinned. A floating - // tag is refused with a spec_rejected error, never launched. + // (d) Canonicalise, THEN classify. A floating tag is refused with a + // spec_rejected error under the prod posture, never launched. A digest that IS + // present must be a real content address whatever the posture — a malformed one + // is garbage input, not a relaxation the operator asked for. const { digest } = parseImageReference(policy.image); if (digest === undefined) { - throw new SpecRejectedError('image_not_digest_pinned', 'the job image is a floating tag, not a digest reference'); - } - if (!DIGEST_RE.test(digest)) { + if (requireDigest) { + throw new SpecRejectedError( + 'image_not_digest_pinned', + 'the job image is a floating tag, not a digest reference', + ); + } + } else if (!DIGEST_RE.test(digest)) { throw new SpecRejectedError('image_bad_digest', 'the job image digest is not a valid content address'); } @@ -296,6 +319,24 @@ export function buildContainerCreateOptions(args) { Ulimits: [{ Name: 'nofile', Soft: limits.nofile, Hard: limits.nofile }], // A job container never restarts — a dead job is a dead job. RestartPolicy: { Name: 'no' }, + // Static `host:ip` entries the DAEMON pre-resolved for this job's OWN + // egress allowlist (jobs.mjs's resolveAllowlistHosts, using the daemon's + // real internet DNS — the job's isolated network has none by design). + // This is NOT a general DNS override: unlike the forbidden `Dns` field + // (which would let a policy point resolution at an arbitrary server and + // escape the allowlist entirely), every entry here names a host the job + // could already reach through the CONNECT proxy — it only makes LOCAL + // resolution of that SAME already-permitted host succeed too. Root + // cause (2026-07-28): npm's own HTTP client (@npmcli/agent) resolves + // its target hostname locally before/alongside the CONNECT tunnel; the + // job network's embedded resolver (127.0.0.11) has no upstream route + // for external names and returns EAI_AGAIN instantly, which — hit for + // every concurrent package fetch — triggers npm's own confirmed + // ExitHandler re-entrancy race (npm/cli#9751, "Exit handler never + // called!"). Always present (possibly empty) so the clamp's own + // "exactly these keys" invariant holds regardless of whether this + // job's policy allowlisted anything. + ExtraHosts: extraHosts ?? [], }, }; } diff --git a/middleware/sidecars/dev-runner-daemon/src/jobs.mjs b/middleware/sidecars/dev-runner-daemon/src/jobs.mjs index 5a59e7bda..8127d0f69 100644 --- a/middleware/sidecars/dev-runner-daemon/src/jobs.mjs +++ b/middleware/sidecars/dev-runner-daemon/src/jobs.mjs @@ -21,16 +21,20 @@ * clamp unit fills them, so an accidental early call fails loudly. * * SEAM CONTRACT — `ContainerEngine.createJobContainer({ jobId, policy, - * leaseExpiresAt })`: given a job id, the SERVER-DERIVED `DerivedJobPolicy` - * (image/env/egressAllowlist, fetched by the daemon — never caller-supplied), - * and the computed lease expiry, create and start ONE hardened container and - * return its `{ containerId, networkId, volumeName, imageDigest }`. The clamp, - * per-job network, and workspace volume are the implementation's responsibility; - * the JobManager only stores what it returns. + * leaseExpiresAt, extraHosts })`: given a job id, the SERVER-DERIVED + * `DerivedJobPolicy` (image/env/egressAllowlist, fetched by the daemon — + * never caller-supplied), the computed lease expiry, and ALREADY-RESOLVED + * `host:ip` entries for the allowlist (JobManager#provision resolves them via + * the proxy client — the engine has no route of its own to the internet), + * create and start ONE hardened container and return its `{ containerId, + * networkId, volumeName, imageDigest }`. The clamp, per-job network, and + * workspace volume are the implementation's responsibility; the JobManager + * only stores what it returns. */ import { randomBytes } from 'node:crypto'; import { readFileSync } from 'node:fs'; +import { isIP } from 'node:net'; import { PassThrough, Readable } from 'node:stream'; import { join } from 'node:path'; @@ -55,6 +59,7 @@ import { ROLE_DIND, SpecRejectedError, } from './clamp.mjs'; +import { parseRequireDigest } from './policyClient.mjs'; /** * @typedef {import('./policyClient.mjs').DerivedJobPolicy} DerivedJobPolicy @@ -132,7 +137,7 @@ export const LEASE_EXPIRES_LABEL = 'ai.omadia.dev.leaseExpiresAt'; * mutating methods; this unit provides `ping` and the fake used by tests. * @typedef {object} ContainerEngine * @property {() => Promise} ping - * @property {(args: { jobId: string, policy: DerivedJobPolicy, leaseExpiresAt: string }) => Promise} createJobContainer + * @property {(args: { jobId: string, policy: DerivedJobPolicy, leaseExpiresAt: string, extraHosts?: readonly string[] }) => Promise} createJobContainer * @property {(container: JobContainer) => Promise} destroyJobContainer * @property {(container: JobContainer, opts: { follow: boolean }) => Promise} streamLogs * @property {(refs: readonly string[]) => Promise} warmImages @@ -417,6 +422,7 @@ export class JobManager { const leaseExpiresAt = this.#leaseExpiry(leaseTtlSec); const hardDeadlineAt = new Date(this.#clock.now() + this.#maxLifetimeMs).toISOString(); + let extraHosts = []; if (this.#proxyClient && proxyToken) { // BEFORE the container starts: a runner that boots first races its own first // request against this call. The TTL is the job's hard deadline, not its @@ -430,11 +436,23 @@ export class JobManager { proxyToken, ttlSec, }); + // Pre-resolve the allowlist THROUGH THE PROXY (the only component with a + // real route to the internet — confirmed live the daemon itself has none) + // so the container's static /etc/hosts covers whatever LOCAL resolution a + // tool inside it attempts (resolveAllowlistHosts's own doc comment has the + // full story: npm's exit-handler crash traced to exactly this gap). A + // resolution failure here must never abort provisioning — it only means + // one fewer /etc/hosts entry, not a broken job. + try { + extraHosts = await resolveAllowlistHosts(policy.egressAllowlist, (hosts) => this.#proxyClient.resolveHosts(hosts)); + } catch (err) { + this.#log(`[jobs] allowlist pre-resolution failed for ${jobId}, continuing without extra /etc/hosts entries: ${err instanceof Error ? err.message : String(err)}`); + } } let container; try { - container = await this.#engine.createJobContainer({ jobId, policy, leaseExpiresAt }); + container = await this.#engine.createJobContainer({ jobId, policy, leaseExpiresAt, extraHosts }); } catch (err) { // No container exists, so nothing may keep egress authorisation. Failing to // withdraw it is not fatal (it expires at the hard deadline) but it is never silent. @@ -802,6 +820,35 @@ async function ensureImage(docker, ref, pullPolicy = 'always') { }); } +/** + * Read an image's content address back FROM THE ENGINE — what actually got + * resolved, not what the reference claimed. Preference order: + * 1. the RepoDigest belonging to the repository in `ref` — an image pulled under + * several names carries one RepoDigest per repository and they are NOT + * interchangeable, so `[0]` can hand back a digest from another registry; + * 2. a digest carried by `ref` itself; + * 3. the local image `Id` — the ONLY content address a `docker load`ed image has, + * since it never came from a registry to be assigned a RepoDigest. + * + * Step 3 is not a weakening: it is reached only for a floating tag, which the clamp + * admits only under an explicit `DEV_RUNNER_REQUIRE_DIGEST=0`. It exists because + * `imageDigest` is a REQUIRED daemon↔middleware wire field (`z.string().min(1)`), + * so an empty one fails the protocol rather than the job — and because an audit + * chain wants the content address of what ran even when no registry vouched for it. + * + * @param {Docker} docker + * @param {string} ref + * @returns {Promise} '' only if the engine reports neither digest nor Id. + */ +async function resolveImageDigest(docker, ref) { + const info = await docker.getImage(ref).inspect(); + const repoDigests = Array.isArray(info.RepoDigests) ? info.RepoDigests : []; + const repository = repositoryOf(ref); + const match = repoDigests.find((rd) => typeof rd === 'string' && rd.startsWith(`${repository}@`)); + if (typeof match === 'string') return match.slice(match.indexOf('@') + 1); + return imageDigestOf(ref) ?? String(info.Id ?? ''); +} + /** Remove a container after a graceful SIGTERM/10s/SIGKILL stop, then VERIFY it is * gone (lesson (c): a remove call returning is not proof of removal). A 404 at any * step means already-gone (idempotent). Any surviving container or hard error is @@ -885,6 +932,50 @@ export function resolvePullPolicy(env) { ); } +/** + * Pre-resolve a job's egress allowlist so the container can be given static + * `/etc/hosts` entries (Docker's `ExtraHosts`) for hosts it can ALREADY reach + * through the CONNECT proxy — this adds no new egress capability, it only + * makes LOCAL name resolution of those SAME already-permitted hosts succeed. + * + * Root cause this exists for (2026-07-28, epic #470): npm's own HTTP client + * (`@npmcli/agent`) resolves its target hostname locally before/alongside + * the CONNECT tunnel. Confirmed live: a direct `dns.lookup()` inside a job + * container fails in 4ms with `EAI_AGAIN` — Docker's embedded resolver + * (127.0.0.11) has no upstream for external names (spec §6's "DNS-exfil + * defence": the job network has no route to a real resolver by design). Hit + * for every concurrent package fetch, this triggers npm's own confirmed + * ExitHandler re-entrancy race (npm/cli#9751, "Exit handler never called!"). + * Any tool doing local resolution of an allowlisted host hits the same + * wall — this fixes it at the root rather than chasing npm's internals. + * + * The resolution itself MUST go through the proxy's `resolveHosts` (not the + * daemon's own DNS) — confirmed live that the daemon container is ALSO on an + * isolated network with no internet route; only the egress proxy is. Batched + * into ONE call rather than per-host, since the proxy is reachable but the + * daemon otherwise has no network dependency on it for anything but this. + * + * A host that fails to resolve here is skipped, not fatal — the CONNECT + * tunnel path (the proxy's own, independent resolution) still works for it + * regardless; this is a best-effort improvement to LOCAL resolution, not a + * new correctness requirement for egress itself. An IP literal in the + * allowlist is skipped too — nothing to pre-resolve, and it would be a + * malformed `ExtraHosts` entry (Docker expects a NAME on the left). + * + * @param {readonly string[]} allowlist + * @param {(hosts: readonly string[]) => Promise | null }>>} resolveHosts + * The proxy client's batched resolver (tests inject a fake). + * @returns {Promise} `host:ip` entries, Docker's `ExtraHosts` format. + */ +export async function resolveAllowlistHosts(allowlist, resolveHosts) { + const toResolve = allowlist.filter((host) => isIP(host) === 0); + if (toResolve.length === 0) return []; + const results = await resolveHosts(toResolve); + return results + .filter((r) => r.addresses && r.addresses.length > 0) + .map((r) => `${r.host}:${/** @type {{ address: string }[]} */ (r.addresses)[0].address}`); +} + /** * A real dockerode-backed engine implementing the full §4 container lifecycle * behind the `ContainerEngine` seam. `createJobContainer` builds the create-options @@ -909,6 +1000,10 @@ export function createDockerEngine(opts = {}) { // Resolved once at engine construction: the same policy governs the per-job image // pull, the DinD sidecar image pull, and the warm loop, so all three agree. const pullPolicy = resolvePullPolicy(env); + // The SAME posture the policy client is constructed with, parsed by the SAME + // function — the clamp is a second enforcement point for one operator decision, + // not a second (contradicting) decision. Defaults to prod posture when unset. + const requireDigest = parseRequireDigest(env.DEV_RUNNER_REQUIRE_DIGEST); return { async ping() { @@ -921,10 +1016,13 @@ export function createDockerEngine(opts = {}) { } }, - async createJobContainer({ jobId, policy, leaseExpiresAt }) { + async createJobContainer({ jobId, policy, leaseExpiresAt, extraHosts = [] }) { const networkName = jobNetworkName(jobId); const volumeName = jobVolumeName(jobId); const dockerInJob = policy.dockerInJob === true; + // extraHosts arrives ALREADY resolved — the caller (JobManager#provision) + // is the one with a proxy client (this engine has none, and no route of + // its own to the internet regardless; see resolveAllowlistHosts's doc). // Build (and thereby VALIDATE) the create-options FIRST: a forbidden spec // (a floating-tag image) throws SpecRejectedError here, before any docker // resource is created — so a rejected job leaks nothing. @@ -936,16 +1034,26 @@ export function createDockerEngine(opts = {}) { volumeName, createdBy, limits, + extraHosts, + requireDigest, dockerInJob, }); - const imageDigest = imageDigestOf(policy.image); - if (imageDigest === undefined) { - // Unreachable: buildContainerCreateOptions already rejected a tag-only image. - throw new SpecRejectedError('image_not_digest_pinned', 'the job image resolved to no digest'); - } // Resolve the image BY DIGEST (never a floating tag) so the container is // created from exactly the vetted content. await ensureImage(docker, policy.image, pullPolicy); + // A pinned ref already IS the content address — the prod path resolves it + // without touching the engine, exactly as before. Only a floating tag (which + // the clamp admitted, so DEV_RUNNER_REQUIRE_DIGEST is explicitly off) has to + // be read back from the engine, which is why this runs AFTER ensureImage: + // an image that is not present yet has no Id to report. + const imageDigest = imageDigestOf(policy.image) ?? (await resolveImageDigest(docker, policy.image)); + if (imageDigest === '') { + // Reachable: the engine knows the image but reports neither RepoDigest nor + // Id. `imageDigest` is a required wire field, so an empty one would fail + // the middleware's response parse with an opaque protocol error instead of + // this named one — and would do it AFTER the container was already running. + throw new SpecRejectedError('image_not_digest_pinned', 'the job image resolved to no digest'); + } const labels = { [JOB_ID_LABEL]: jobId, @@ -1079,19 +1187,7 @@ export function createDockerEngine(opts = {}) { const digests = []; for (const ref of refs) { await ensureImage(docker, ref, pullPolicy); - const info = await docker.getImage(ref).inspect(); - const repoDigests = Array.isArray(info.RepoDigests) ? info.RepoDigests : []; - // An image pulled under several names carries one RepoDigest per - // repository, and they are NOT interchangeable — taking [0] can hand - // back a digest that belongs to a different registry than the ref we - // were asked to warm. Match on the repository we actually pulled. - const repository = repositoryOf(ref); - const match = repoDigests.find((rd) => typeof rd === 'string' && rd.startsWith(`${repository}@`)); - const resolved = - typeof match === 'string' - ? match.slice(match.indexOf('@') + 1) - : (imageDigestOf(ref) ?? String(info.Id ?? '')); - digests.push(resolved); + digests.push(await resolveImageDigest(docker, ref)); } return digests; }, diff --git a/middleware/sidecars/dev-runner-daemon/src/policyClient.mjs b/middleware/sidecars/dev-runner-daemon/src/policyClient.mjs index 9bae8b42a..6adba1d0f 100644 --- a/middleware/sidecars/dev-runner-daemon/src/policyClient.mjs +++ b/middleware/sidecars/dev-runner-daemon/src/policyClient.mjs @@ -197,15 +197,17 @@ const ALLOWED_ENV_KEYS = new Set([ * UUID-validated job id, not a value the middleware can skew. * - `OMADIA_WORKSPACE` — where the repo is cloned; must be the container's fixed * workspace path, not a policy-chosen directory. - * - `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` (and their lowercase spellings) — + * - `HTTP_PROXY` / `HTTPS_PROXY` / `NO_PROXY` (and their lowercase spellings, plus + * npm's own `npm_config_proxy` / `npm_config_https_proxy` / `npm_config_noproxy`) — * the container-wide egress-routing lever. A policy value would redirect every * http(s) client in the container (clone, SCM-token exchange, diff upload, - * git, node, curl) through an attacker-chosen proxy. The egress proxy's + * git, node, curl, npm) through an attacker-chosen proxy. The egress proxy's * address is deployment topology the daemon knows from its own config, so the * daemon injects it (`DEV_RUNNER_EGRESS_PROXY_URL` / `DEV_RUNNER_NO_PROXY`) - * and never accepts it from the policy. Both spellings are owned because - * curl/libcurl honour the lowercase names and git/node the uppercase ones — - * admitting either from the policy would reopen the lever the other closes. + * and never accepts it from the policy. Every spelling is owned because + * curl/libcurl honour the lowercase names, git/node the uppercase ones, and + * npm's own config layer resolves `npm_config_*` before either — admitting + * any one from the policy would reopen the lever the others close. * * A policy that CARRIES any of these is not a legitimate policy — it is a * compromised or spoofed middleware. So we REJECT it loudly (`assertPolicyEnv`) @@ -226,6 +228,9 @@ const DAEMON_OWNED_ENV_KEYS = new Set([ 'http_proxy', 'https_proxy', 'no_proxy', + 'npm_config_proxy', + 'npm_config_https_proxy', + 'npm_config_noproxy', ]); /** The container's fixed, job-scoped clone directory (W1 clamp: the per-job @@ -478,7 +483,7 @@ function injectDaemonOwnedEnv(policyEnv, jobId, owned) { if (owned.egressProxyUrl) { // The proxy is default-deny and authenticates every request as // `Proxy-Authorization: Basic base64(jobId:proxyToken)`. Standard http clients - // (curl, git, undici, python-requests) derive that header from the proxy URL's + // (curl, git, python-requests) derive that header from the proxy URL's // userinfo, so the credential travels in the injected value — NOT in the // operator-supplied DEV_RUNNER_EGRESS_PROXY_URL, which is still refused if it // carries userinfo. The token names exactly one job's allowlist, and it is the @@ -490,10 +495,33 @@ function injectDaemonOwnedEnv(policyEnv, jobId, owned) { env.HTTPS_PROXY = withCreds; env.http_proxy = withCreds; env.https_proxy = withCreds; + // npm's OWN config layer (lib/utils/config, @npmcli/config) resolves + // `proxy`/`https-proxy`/`noproxy` from `npm_config_*` env vars BEFORE it + // ever looks at generic HTTP_PROXY/HTTPS_PROXY — @npmcli/agent then reads + // the resolved npm config, not the raw env, for its own proxy-vs-direct + // decision. Pinning both layers closes a config-precedence class of bug + // (npm/cli#6835, npm/agent#125) as a contributing factor in the + // DNS-bypass-then-ENETUNREACH investigation (epic #470, 2026-07-29) — + // independent of whichever exact code path was choosing direct-connect. + env.npm_config_proxy = withCreds; + env.npm_config_https_proxy = withCreds; + // UNLIKE curl/git, Node's own global `fetch` (undici) does NOT read + // HTTP_PROXY/HTTPS_PROXY/NO_PROXY by default — that's opt-in, gated behind + // this exact flag (undici's EnvHttpProxyAgent). The shim's homeClient.ts is + // deliberately "Node's global fetch only — no dependency", so without this, + // every phone-home call (spec fetch, events, diff upload, result) ignores + // the proxy entirely and tries the middleware direct — which the runner's + // own per-job network has no route to (`getaddrinfo ENOTFOUND middleware`). + // MUST be a real process env var: setting it in-process after Node starts + // does nothing, since undici reads it once at dispatcher construction. + // Read-only for `MUST be set`; the value itself carries no secret and has + // no reason to ever be anything but '1' when a proxy is configured at all. + env.NODE_USE_ENV_PROXY = '1'; } if (owned.noProxy) { env.NO_PROXY = owned.noProxy; env.no_proxy = owned.noProxy; + env.npm_config_noproxy = owned.noProxy; } return env; } diff --git a/middleware/sidecars/dev-runner-daemon/src/proxy.mjs b/middleware/sidecars/dev-runner-daemon/src/proxy.mjs index 787039fb1..278aea8bd 100644 --- a/middleware/sidecars/dev-runner-daemon/src/proxy.mjs +++ b/middleware/sidecars/dev-runner-daemon/src/proxy.mjs @@ -50,6 +50,66 @@ import { /** DNS must not be a way to park a connection forever before the limiter sees it. */ export const DEFAULT_RESOLVE_TIMEOUT_MS = 5_000; +/** + * How long a successful resolution is reused before a fresh lookup, keyed by + * hostname and shared across every job. Short enough that the DNS-rebinding + * defence (resolve-once/connect-to-what-you-checked, spec §6) stays + * meaningful — the resolved addresses are still classified/pinned fresh on + * every CONNECT, only the lookup itself is reused; long enough to absorb a + * burst of concurrent fetches to the same host (npm's registry traffic: + * dozens of package tarballs from registry.npmjs.org at once). + */ +export const DEFAULT_RESOLVE_CACHE_TTL_MS = 30_000; + +/** + * Wrap a raw resolver with the cache + in-flight de-dup described above. npm + * ci fires up to `maxsockets` (default 15) concurrent CONNECTs to the SAME + * hostname within milliseconds of each other; without this, each one + * independently calls dns.lookup(), which runs on Node's libuv threadpool + * (default 4 workers, never tuned in this image) — so N concurrent same-host + * lookups compete for 4 slots instead of sharing one answer. Confirmed live + * (2026-07-28, epic #470): default npm concurrency crashed deterministically + * ~70s into `npm ci` with npm's own "Exit handler never called!" bug + * (npm/cli#9751 — a re-entrancy race triggered by near-simultaneous registry- + * fetch timeouts); raising `--maxsockets` to 1000 turned the same threadpool + * contention into outright `EAI_AGAIN` once lookups queued past the 5s + * resolve deadline; *lowering* it to 3 only delayed the same crash (70s→ + * 251s). All three point at resolution contention, not npm itself — this + * removes the contention at its source instead of guessing at npm's own + * concurrency. + * + * @param {(host: string) => Promise>} rawResolve + * @param {number} ttlMs + */ +function createCachedResolve(rawResolve, ttlMs) { + /** @type {Map, expiresAt: number }>} */ + const cache = new Map(); + /** @type {Map>>} */ + const inflight = new Map(); + + /** @param {string} host */ + return function cachedResolve(host) { + const cached = cache.get(host); + if (cached && cached.expiresAt > Date.now()) return Promise.resolve(cached.addresses); + + const existing = inflight.get(host); + if (existing) return existing; + + const promise = rawResolve(host) + .then((addresses) => { + // Only a SUCCESS is cached — a failed lookup (or a deadline timeout + // racing it from the caller side) must not poison the next attempt. + if (ttlMs > 0) cache.set(host, { addresses, expiresAt: Date.now() + ttlMs }); + return addresses; + }) + .finally(() => { + inflight.delete(host); + }); + inflight.set(host, promise); + return promise; + }; +} + export const DEFAULT_DATA_PORT = 3128; /** Control-plane port (spec §6). */ export const DEFAULT_CONTROL_PORT = 3129; @@ -102,6 +162,7 @@ async function defaultResolve(host) { * @property {ReadonlySet} [allowedPorts] * @property {(host: string) => Promise>} [resolve] DNS seam. * @property {number} [resolveTimeoutMs] Deadline on name resolution (default 5 s). + * @property {number} [resolveCacheTtlMs] How long a resolution is reused (default 30 s; 0 disables caching). * @property {Partial} [limits] * @property {{ warn?: (m: string) => void }} [logger] * @property {() => number} [now] @@ -119,8 +180,10 @@ export function createProxy(deps) { const allowedPorts = deps.allowedPorts ?? new Set(DEFAULT_ALLOWED_PORTS); const rawResolve = deps.resolve ?? defaultResolve; const resolveTimeoutMs = deps.resolveTimeoutMs ?? DEFAULT_RESOLVE_TIMEOUT_MS; + const resolveCacheTtlMs = deps.resolveCacheTtlMs ?? DEFAULT_RESOLVE_CACHE_TTL_MS; + const cachedResolve = createCachedResolve(rawResolve, resolveCacheTtlMs); /** @param {string} host */ - const resolve = (host) => withDeadline(rawResolve(host), resolveTimeoutMs, 'dns resolve'); + const resolve = (host) => withDeadline(cachedResolve(host), resolveTimeoutMs, 'dns resolve'); const limits = { ...DEFAULTS, ...(deps.limits ?? {}) }; const ctx = { registry: deps.registry, @@ -153,12 +216,33 @@ export function createProxy(deps) { }); }); dataServer.on('connect', (req, socket, head) => { - void handleConnect(req, socket, head).catch(() => { + // `socket` is the raw net.Socket the CONNECT upgrade hands us — an + // EventEmitter with NO listener for 'error' until handleConnect's success + // path reaches `clientSocket.on('error', teardown)`, well after DNS + // resolution / the allowlist decision. A client that resets the + // connection (ECONNRESET) at ANY point before that — observed live — + // fires an unhandled 'error' event, and Node's default for a + // listener-less EventEmitter 'error' is to throw, which crashed this + // entire process (taking down egress control-plane calls for every OTHER + // concurrent job until the container restarted). Attach a listener + // covering the socket's full lifetime, unconditionally, before anything + // else touches it; handleConnect's own later listener simply becomes a + // second listener on the same event once the tunnel exists — both firing + // is harmless, `destroySocket` is idempotent. + socket.on('error', (err) => { + logger.warn?.(`[dev-egress-proxy] client socket error: ${err instanceof Error ? err.message : String(err)}`); + destroySocket(socket); + }); + void handleConnect(req, socket, head).catch((err) => { + logger.warn?.(`[dev-egress-proxy] handleConnect threw: ${err instanceof Error ? (err.stack ?? err.message) : String(err)}`); destroySocket(socket); }); }); // A client that errors before/after the CONNECT upgrade must not throw globally. - dataServer.on('clientError', (_err, socket) => destroySocket(socket)); + dataServer.on('clientError', (err, socket) => { + logger.warn?.(`[dev-egress-proxy] clientError: ${err instanceof Error ? err.message : String(err)}`); + destroySocket(socket); + }); /** * CONNECT tunnel: authorise → decide → (only now) resolve → classify → pin → @@ -287,7 +371,10 @@ export function createProxy(deps) { armIdle(); }); - upstream.on('error', teardown); + upstream.on('error', (err) => { + logger.warn?.(`[dev-egress-proxy] upstream connection to ${host}:${port} (${pinnedIp}) failed: ${err instanceof Error ? err.message : String(err)}`); + teardown(); + }); upstream.on('close', teardown); clientSocket.on('error', teardown); clientSocket.on('close', teardown); @@ -406,6 +493,41 @@ export function createProxy(deps) { return; } const url = new URL(req.url ?? '/', 'http://proxy.local'); + + // POST /resolve — the daemon has no route to the internet by design (only + // the proxy does); this lets it pre-resolve a job's allowlist for static + // /etc/hosts entries (spec §470, the npm local-DNS-bypass root cause) + // using the SAME resolver the data plane trusts, without granting the + // daemon egress of its own. Bearer-authed like every other control route; + // NOT allowlist-gated — a resolution reveals only a public IP for a name + // the caller already supplied, nothing a public DNS query wouldn't. + if (url.pathname === '/resolve' && req.method === 'POST') { + let body; + try { + body = await readJsonBody(req); + } catch { + sendJson(res, 400, { code: 'proxy.bad_body', message: 'invalid JSON body' }); + return; + } + const hosts = Array.isArray(body?.hosts) ? body.hosts.filter((h) => typeof h === 'string') : null; + if (!hosts || hosts.length === 0 || hosts.length > 100) { + sendJson(res, 400, { code: 'proxy.bad_body', message: 'hosts must be a non-empty array of at most 100 strings' }); + return; + } + const results = await Promise.all( + hosts.map(async (host) => { + try { + const addresses = await resolve(host); + return { host, addresses }; + } catch { + return { host, addresses: null }; + } + }), + ); + sendJson(res, 200, { results }); + return; + } + const m = /^\/jobs\/([^/]+)$/.exec(url.pathname); if (!m) { sendJson(res, 404, { code: 'proxy.not_found', message: 'no such route' }); @@ -460,12 +582,38 @@ export function createProxy(deps) { } /** Write a raw HTTP status line to a CONNECT client socket (no res object here). + * + * ANNOUNCE THE CLOSE. Every non-2xx reply on this path is terminal — the caller + * `end()`s the socket the moment this returns, because node has already detached + * its HTTP parser from the socket at the `connect` event and hands it to us raw, + * so there is no second request to serve on it. + * + * That matters most for the 407. Proxy auth over CONNECT is a CHALLENGE-RESPONSE + * ON ONE CONNECTION: libcurl (and therefore `git`, whose default + * `http.proxyAuthMethod` is `anyauth`) sends an unauthenticated CONNECT first, + * reads the 407 + `Proxy-Authenticate`, then re-sends the CONNECT with + * `Proxy-Authorization` ON THAT SAME SOCKET. A 407 that FINs the socket without + * saying so leaves the client writing its authenticated retry into a connection + * we already closed; it reads EOF and reports `Proxy CONNECT aborted`, so auth + * can never succeed and every job's clone fails. `Connection: close` — plus the + * legacy `Proxy-Connection` spelling libcurl also honours — tells it to reconnect + * for the retry instead. `Content-Length: 0` keeps the (absent) body framed + * rather than delimited by the close. + * + * Clients that send credentials preemptively (node's `EnvHttpProxyAgent`, a + * hand-rolled CONNECT) never reach the 407 at all, which is exactly why this hid + * behind a working phone-home path. + * * @param {import('node:stream').Duplex} socket @param {number} code * @param {string} message @param {Record} [headers] */ function writeConnectStatus(socket, code, message, headers = {}) { if (socket.destroyed || socket.writableEnded) return; + const isTerminal = code < 200 || code >= 300; + const effective = isTerminal + ? { ...headers, 'Content-Length': '0', Connection: 'close', 'Proxy-Connection': 'close' } + : headers; let head = `HTTP/1.1 ${code} ${message}\r\n`; - for (const [k, v] of Object.entries(headers)) head += `${k}: ${v}\r\n`; + for (const [k, v] of Object.entries(effective)) head += `${k}: ${v}\r\n`; head += '\r\n'; try { socket.write(head); diff --git a/middleware/sidecars/dev-runner-daemon/src/proxyClient.mjs b/middleware/sidecars/dev-runner-daemon/src/proxyClient.mjs index 633dd9291..c021fb802 100644 --- a/middleware/sidecars/dev-runner-daemon/src/proxyClient.mjs +++ b/middleware/sidecars/dev-runner-daemon/src/proxyClient.mjs @@ -46,6 +46,10 @@ export class ProxyControlError extends Error { * @typedef {object} ProxyClient * @property {(jobId: string, entry: { allowlist: readonly string[], proxyToken: string, ttlSec: number }) => Promise} register * @property {(jobId: string) => Promise} unregister + * @property {(hosts: readonly string[]) => Promise | null }>>} resolveHosts + * Pre-resolve hostnames using the proxy's OWN internet-reachable resolver — + * the daemon has none by design. `addresses: null` for a host that failed + * to resolve; non-fatal by contract, the caller decides what to do with it. */ /** @@ -98,7 +102,7 @@ export function createProxyClient(deps) { try { json = await res.json(); } catch { - json = null; + // leave json null } return { status: res.status, json }; })(); @@ -134,5 +138,42 @@ export function createProxyClient(deps) { } return Boolean(/** @type {any} */ (json)?.deleted); }, + + async resolveHosts(hosts) { + if (hosts.length === 0) return []; + const url = `${origin}/resolve`; + const controller = new AbortController(); + const run = (async () => { + const res = await fetchImpl(url, { + method: 'POST', + redirect: 'error', + signal: controller.signal, + headers: { authorization: `Bearer ${deps.token}`, 'content-type': 'application/json' }, + body: JSON.stringify({ hosts }), + }); + let json = null; + try { + json = await res.json(); + } catch { + // leave json null + } + return { status: res.status, json }; + })(); + let status, json; + try { + ({ status, json } = await withDeadline(run, timeoutMs, () => controller.abort())); + } catch (err) { + throw new ProxyControlError( + 'proxy.control_unreachable', + `POST ${url} failed: ${err instanceof Error ? err.message : String(err)}`, + ); + } + if (status !== 200) { + const code = typeof (/** @type {any} */ (json)?.code) === 'string' ? json.code : 'proxy.control_rejected'; + throw new ProxyControlError(code, `proxy refused to resolve hosts (HTTP ${status})`, status); + } + const results = /** @type {any} */ (json)?.results; + return Array.isArray(results) ? results : []; + }, }; } diff --git a/middleware/sidecars/dev-runner-daemon/test/clamp.test.mjs b/middleware/sidecars/dev-runner-daemon/test/clamp.test.mjs index ef7b68cc8..54cab4794 100644 --- a/middleware/sidecars/dev-runner-daemon/test/clamp.test.mjs +++ b/middleware/sidecars/dev-runner-daemon/test/clamp.test.mjs @@ -84,6 +84,24 @@ describe('buildContainerCreateOptions — REQUIRED clamp fields present', () => }); }); +describe('buildContainerCreateOptions — ExtraHosts (pre-resolved allowlist entries)', () => { + it('defaults to an empty array when no extraHosts are given', () => { + const hc = build().HostConfig ?? {}; + assert.deepEqual(hc.ExtraHosts, []); + }); + + it('passes the given entries through verbatim — this function derives nothing itself', () => { + const entries = ['registry.npmjs.org:104.16.0.35', 'github.com:140.82.121.3']; + const hc = build({ extraHosts: entries }).HostConfig ?? {}; + assert.deepEqual(hc.ExtraHosts, entries); + }); + + it('is still distinct from the forbidden Dns field — a resolver override stays refused', () => { + const hc = build({ extraHosts: ['registry.npmjs.org:104.16.0.35'] }).HostConfig ?? {}; + assert.equal(hc.Dns, undefined); + }); +}); + describe('buildContainerCreateOptions — FORBIDDEN options are absent by construction', () => { const o = build(); const hc = /** @type {Record} */ (o.HostConfig ?? {}); @@ -94,6 +112,7 @@ describe('buildContainerCreateOptions — FORBIDDEN options are absent by constr const allowed = [ 'Binds', 'CapDrop', + 'ExtraHosts', 'Memory', 'MemorySwap', 'NanoCpus', @@ -161,6 +180,50 @@ describe('buildContainerCreateOptions — a forbidden image fails with spec_reje }); }); +// The clamp used to hardcode the digest requirement ON, which made the documented +// `DEV_RUNNER_REQUIRE_DIGEST=0` local escape hatch a NO-OP: the policy client was +// told to allow a floating tag and the clamp refused it anyway, one gate later. +describe('buildContainerCreateOptions — the DEV_RUNNER_REQUIRE_DIGEST posture', () => { + const FLOATING = 'omadia-dev-runner:latest'; + + it('fails CLOSED — an omitted posture refuses a floating tag (prod default)', () => { + assert.throws( + () => build({ policy: policy({ image: FLOATING }) }), + (err) => err instanceof SpecRejectedError && err.reason === 'image_not_digest_pinned', + ); + }); + + it('an explicit requireDigest: true refuses a floating tag', () => { + assert.throws( + () => build({ policy: policy({ image: FLOATING }), requireDigest: true }), + (err) => err instanceof SpecRejectedError && err.reason === 'image_not_digest_pinned', + ); + }); + + it('admits a floating tag ONLY when the operator explicitly relaxes the posture', () => { + const o = build({ policy: policy({ image: FLOATING }), requireDigest: false }); + assert.equal(o.Image, FLOATING, 'the locally-loaded image is launched verbatim'); + }); + + it('relaxing the posture relaxes NOTHING else — the full clamp still applies', () => { + const o = build({ policy: policy({ image: FLOATING }), requireDigest: false }); + const hc = o.HostConfig ?? {}; + assert.equal(o.User, '1000:1000'); + assert.equal(hc.ReadonlyRootfs, true); + assert.deepEqual(hc.CapDrop, ['ALL']); + assert.deepEqual(hc.SecurityOpt, ['no-new-privileges:true']); + assert.equal(hc.Privileged, false); + assert.deepEqual(hc.Binds, [`${jobVolumeName(JOB_ID)}:/workspace`]); + }); + + it('still refuses a MALFORMED digest with the posture relaxed — a knob about whether a digest is required never tolerates garbage', () => { + assert.throws( + () => build({ policy: policy({ image: 'ghcr.io/x/y@sha256:abc' }), requireDigest: false }), + (err) => err instanceof SpecRejectedError && err.reason === 'image_bad_digest', + ); + }); +}); + describe('resolveClampLimits — resource bounds are always present and env-tunable', () => { it('defaults to the §4 floor when nothing is set', () => { assert.deepEqual(resolveClampLimits({}), { diff --git a/middleware/sidecars/dev-runner-daemon/test/jobs.test.mjs b/middleware/sidecars/dev-runner-daemon/test/jobs.test.mjs index a7e4b4afb..166bb0786 100644 --- a/middleware/sidecars/dev-runner-daemon/test/jobs.test.mjs +++ b/middleware/sidecars/dev-runner-daemon/test/jobs.test.mjs @@ -19,7 +19,7 @@ import { describe, it } from 'node:test'; import Docker from 'dockerode'; import { SpecRejectedError } from '../src/clamp.mjs'; -import { createDockerEngine, JobCancelledError, JobCapacityError, JobCleanupError, JobManager, resolvePullPolicy } from '../src/jobs.mjs'; +import { createDockerEngine, JobCancelledError, JobCapacityError, JobCleanupError, JobManager, resolveAllowlistHosts, resolvePullPolicy } from '../src/jobs.mjs'; const JOB_ID = '11111111-1111-4111-8111-111111111111'; @@ -501,8 +501,9 @@ function makeFakeDocker(opts = {}) { volumes.add(o.Name); return { Name: o.Name }; }, - async createContainer(_o) { + async createContainer(o) { if (opts.containerCreateFail) throw opts.containerCreateFail; + if (opts.createContainerCalls) opts.createContainerCalls.push(o); const id = `ctr-${++seq}`; containers.add(id); return containerHandle(id); @@ -518,12 +519,62 @@ function makeFakeDocker(opts = {}) { if (opts.presentImages && !opts.presentImages.has(ref)) throw notFound('image'); // Real docker reports `repository@sha256:…` — never with a tag. const repo = (ref.split('@')[0] ?? ref).replace(/:[^:/]+$/, ''); - return { RepoDigests: [`${repo}@${DIGEST}`], Id: 'sha256:imgid' }; + // `noRepoDigests` models a `docker load`ed image: it never came from a + // registry, so docker reports NO RepoDigests and the Id is its only + // content address. That is the local-dev shape, not a hypothetical. + return { + RepoDigests: opts.noRepoDigests ? [] : [`${repo}@${DIGEST}`], + Id: opts.imageId ?? 'sha256:imgid', + }; }, }), }; } +describe('resolveAllowlistHosts — pre-resolves an allowlist for ExtraHosts', () => { + it('resolves each host in ONE batched call and formats Docker\'s host:ip ExtraHosts entries', async () => { + const calls = []; + const resolveHosts = async (hosts) => { + calls.push(hosts); + return [ + { host: 'registry.npmjs.org', addresses: [{ address: '104.16.0.35' }] }, + { host: 'github.com', addresses: [{ address: '140.82.121.3' }] }, + ]; + }; + const result = await resolveAllowlistHosts(['registry.npmjs.org', 'github.com'], resolveHosts); + assert.deepEqual(result.sort(), ['github.com:140.82.121.3', 'registry.npmjs.org:104.16.0.35'].sort()); + assert.equal(calls.length, 1, 'exactly one batched call, not one per host'); + assert.deepEqual(calls[0].sort(), ['github.com', 'registry.npmjs.org'].sort()); + }); + + it('skips a host that failed to resolve (addresses: null) — non-fatal, the CONNECT path still works for it', async () => { + const resolveHosts = async () => [ + { host: 'flaky.example.com', addresses: null }, + { host: 'good.example.com', addresses: [{ address: '203.0.113.10' }] }, + ]; + const result = await resolveAllowlistHosts(['flaky.example.com', 'good.example.com'], resolveHosts); + assert.deepEqual(result, ['good.example.com:203.0.113.10']); + }); + + it('skips an already-literal IP entry — nothing to pre-resolve, and it is not a valid ExtraHosts name', async () => { + let called = false; + const resolveHosts = async () => { + called = true; + return []; + }; + const result = await resolveAllowlistHosts(['203.0.113.5'], resolveHosts); + assert.deepEqual(result, []); + assert.equal(called, false, 'an IP literal is never handed to the batched resolver'); + }); + + it('an empty allowlist resolves to an empty array', async () => { + const result = await resolveAllowlistHosts([], async () => { + throw new Error('must not be called'); + }); + assert.deepEqual(result, []); + }); +}); + describe('createDockerEngine — createJobContainer applies the clamp and provisions cleanly', () => { it('pulls by digest, creates the per-job network+volume+container, starts it, returns the handle', async () => { const docker = makeFakeDocker(); @@ -546,6 +597,36 @@ describe('createDockerEngine — createJobContainer applies the clamp and provis assert.equal(docker.state.events.started.length, 1, 'the container was started'); }); + it('passes caller-supplied extraHosts straight through to HostConfig.ExtraHosts — this engine resolves nothing itself', async () => { + const createContainerCalls = []; + const docker = makeFakeDocker({ createContainerCalls }); + const engine = createDockerEngine({ docker, env: {} }); + + await engine.createJobContainer({ + jobId: JOB_ID, + policy: enginePolicy(), + leaseExpiresAt: '2026-07-10T12:00:00.000Z', + extraHosts: ['registry.npmjs.org:104.16.0.35'], + }); + + assert.equal(createContainerCalls.length, 1); + assert.deepEqual(createContainerCalls[0].HostConfig.ExtraHosts, ['registry.npmjs.org:104.16.0.35']); + }); + + it('an empty egress allowlist yields an empty ExtraHosts array — no change for a repo with none', async () => { + const createContainerCalls = []; + const docker = makeFakeDocker({ createContainerCalls }); + const engine = createDockerEngine({ docker, env: {} }); + + await engine.createJobContainer({ + jobId: JOB_ID, + policy: enginePolicy(), + leaseExpiresAt: '2026-07-10T12:00:00.000Z', + }); + + assert.deepEqual(createContainerCalls[0].HostConfig.ExtraHosts, []); + }); + it('refuses a floating-tag image with spec_rejected BEFORE creating any resource', async () => { const docker = makeFakeDocker(); const engine = createDockerEngine({ docker, env: {} }); @@ -768,6 +849,69 @@ describe('createDockerEngine — image pull policy (epic #470 local deploy #4)', }); }); +// The local-dev image is `docker load`ed straight into the nested dind engine: +// a floating tag, no registry, therefore no digest to pin and no RepoDigest to +// resolve. `DEV_RUNNER_REQUIRE_DIGEST=0` is the documented escape hatch for that +// shape; these prove the whole provision path honours it end-to-end, and that the +// prod path is untouched. +describe('createDockerEngine — DEV_RUNNER_REQUIRE_DIGEST and the locally-loaded image', () => { + const FLOATING = 'omadia-dev-runner:latest'; + /** A real `docker image inspect .Id` — the only content address a loaded image has. */ + const LOCAL_ID = 'sha256:697ba6492dcec1c6aa49dfc4a8891e81c7caad11636905a610123c01df790f46'; + const LEASE = '2026-07-10T12:00:00.000Z'; + + it('refuses a floating tag when the posture is unset — prod is unchanged, and nothing is created', async () => { + const docker = makeFakeDocker(); + const engine = createDockerEngine({ docker, env: {} }); + + await assert.rejects( + engine.createJobContainer({ jobId: JOB_ID, policy: enginePolicy(FLOATING), leaseExpiresAt: LEASE }), + (err) => err instanceof SpecRejectedError && err.reason === 'image_not_digest_pinned', + ); + assert.equal(docker.state.containers.size, 0, 'a refused spec creates no container'); + assert.equal(docker.state.networks.size, 0, 'a refused spec creates no network'); + assert.equal(docker.state.volumes.size, 0, 'a refused spec creates no volume'); + }); + + it('provisions a loaded floating tag under DEV_RUNNER_REQUIRE_DIGEST=0 and records the image Id as the content address', async () => { + const docker = makeFakeDocker({ + noRepoDigests: true, + imageId: LOCAL_ID, + presentImages: new Set([FLOATING]), + }); + const engine = createDockerEngine({ + docker, + env: { DEV_RUNNER_REQUIRE_DIGEST: '0', DEV_RUNNER_PULL_POLICY: 'if-not-present' }, + }); + + const handle = await engine.createJobContainer({ + jobId: JOB_ID, + policy: enginePolicy(FLOATING), + leaseExpiresAt: LEASE, + }); + + assert.equal(docker.state.containers.size, 1, 'the job container was created'); + assert.deepEqual(docker.state.events.pulled, [], 'a loaded image is never pulled (the proxy would 407)'); + // `imageDigest` is a REQUIRED wire field (z.string().min(1)) — an empty one + // fails the middleware's response parse AFTER the container is already up. + assert.equal(handle.imageDigest, LOCAL_ID, 'the local image Id is the recorded content address'); + assert.notEqual(handle.imageDigest, '', 'the required wire field is never empty'); + }); + + it('a digest-pinned ref still resolves from the ref itself — no engine round-trip, prod behaviour byte-identical', async () => { + const docker = makeFakeDocker({ noRepoDigests: true, imageId: LOCAL_ID }); + const engine = createDockerEngine({ docker, env: { DEV_RUNNER_REQUIRE_DIGEST: '0' } }); + + const handle = await engine.createJobContainer({ + jobId: JOB_ID, + policy: enginePolicy(), + leaseExpiresAt: LEASE, + }); + + assert.equal(handle.imageDigest, DIGEST, 'the pinned digest wins over anything the engine reports'); + }); +}); + // Real dockerode against the local engine. Skipped in the default suite; run with // DEV_RUNNER_DOCKER_IT=1 to exercise a genuine hardened container end-to-end. const RUN_DOCKER_IT = process.env.DEV_RUNNER_DOCKER_IT === '1'; @@ -913,14 +1057,23 @@ describe('JobManager — egress-proxy registration is part of provisioning', () if (overrides.unregisterError) throw overrides.unregisterError; return true; }, + async resolveHosts(hosts) { + calls.push({ op: 'resolveHosts', hosts }); + if (overrides.resolveHostsError) throw overrides.resolveHostsError; + return overrides.resolveHostsResult ?? hosts.map((host) => ({ host, addresses: null })); + }, }; } - /** An engine that records the order of operations against the proxy calls. */ + /** An engine that records the order of operations against the proxy calls + * and captures the exact args each createJobContainer call received. */ function orderedEngine(order, opts = {}) { return { - async createJobContainer() { + /** @type {any[]} */ + createJobContainerCalls: [], + async createJobContainer(args) { order.push('createContainer'); + this.createJobContainerCalls.push(args); if (opts.createError) throw opts.createError; return { jobId: JOB_ID, id: 'c1', networkId: 'n1', volumeName: 'v1', imageDigest: 'sha256:abc' }; }, @@ -1043,4 +1196,37 @@ describe('JobManager — egress-proxy registration is part of provisioning', () await jm.destroy(JOB_ID); assert.equal(jm.size(), 0); }); + + it('resolves the allowlist through the proxy and passes the result to createJobContainer as extraHosts', async () => { + const engine = orderedEngine([]); + const proxy = recordingProxyClient({ + resolveHostsResult: [{ host: 'registry.npmjs.org', addresses: [{ address: '104.16.0.35' }] }], + }); + const policyClient = { + async fetchJobPolicy(jobId) { + return { jobId, image: 'ghcr.io/x/y@sha256:abc', env: {}, egressAllowlist: ['registry.npmjs.org'] }; + }, + }; + const jm = new JobManager({ engine, policyClient, proxyClient: proxy }); + await jm.create(JOB_ID, 180); + + const resolveCall = proxy.calls.find((c) => c.op === 'resolveHosts'); + assert.deepEqual(resolveCall.hosts, ['registry.npmjs.org']); + assert.deepEqual(engine.createJobContainerCalls[0].extraHosts, ['registry.npmjs.org:104.16.0.35']); + }); + + it('a resolveHosts failure never aborts job creation — the job proceeds with no extra /etc/hosts entries', async () => { + const engine = orderedEngine([]); + const proxy = recordingProxyClient({ resolveHostsError: new Error('proxy unreachable') }); + const policyClient = { + async fetchJobPolicy(jobId) { + return { jobId, image: 'ghcr.io/x/y@sha256:abc', env: {}, egressAllowlist: ['registry.npmjs.org'] }; + }, + }; + const jm = new JobManager({ engine, policyClient, proxyClient: proxy }); + await jm.create(JOB_ID, 180); + + assert.deepEqual(engine.createJobContainerCalls[0].extraHosts, []); + assert.equal(jm.size(), 1, 'the job was still created despite the resolution failure'); + }); }); diff --git a/middleware/sidecars/dev-runner-daemon/test/policyClient.test.mjs b/middleware/sidecars/dev-runner-daemon/test/policyClient.test.mjs index c77a7a1ea..295e393e9 100644 --- a/middleware/sidecars/dev-runner-daemon/test/policyClient.test.mjs +++ b/middleware/sidecars/dev-runner-daemon/test/policyClient.test.mjs @@ -263,6 +263,11 @@ describe('policyClient — daemon-owned env keys are injected, never accepted', 'http_proxy', 'https_proxy', 'no_proxy', + // npm's own config-layer spellings — same egress-redirect risk as the + // generic env vars above, so daemon-owned on the same terms. + 'npm_config_proxy', + 'npm_config_https_proxy', + 'npm_config_noproxy', ]; // A policy that carries any of these is a compromised/spoofed middleware trying @@ -313,6 +318,33 @@ describe('policyClient — daemon-owned env keys are injected, never accepted', assert.equal(policy.env.no_proxy, 'middleware,localhost'); }); + it('ALSO pins npm\'s own config-layer proxy vars — npm resolves npm_config_* before generic HTTP_PROXY', async () => { + // Root cause context (epic #470, 2026-07-29): npm's proxy-vs-direct + // decision reads its OWN resolved config, not raw env, directly. A + // config-precedence bug (npm/cli#6835, npm/agent#125 class) could still + // let npm miss the generic HTTP_PROXY/HTTPS_PROXY vars even when they are + // set correctly — pinning npm's own env-var spelling closes that gap. + const client = clientWith(policyBody(), { + clientOpts: { egressProxyUrl: 'http://egress-proxy:3128', noProxy: 'middleware,localhost' }, + }); + const policy = await client.fetchJobPolicy(JOB_ID); + assert.equal(policy.env.npm_config_proxy, 'http://egress-proxy:3128'); + assert.equal(policy.env.npm_config_https_proxy, 'http://egress-proxy:3128'); + assert.equal(policy.env.npm_config_noproxy, 'middleware,localhost'); + }); + + it('injects NODE_USE_ENV_PROXY alongside a configured proxy — otherwise Node fetch ignores HTTP_PROXY entirely', async () => { + // Unlike curl/git, Node's global fetch (undici) does NOT read HTTP_PROXY by + // default; the shim's homeClient.ts uses fetch with no dependency, so + // without this flag every phone-home call silently bypasses the proxy and + // tries the middleware direct — unreachable from the job's own network. + const client = clientWith(policyBody(), { + clientOpts: { egressProxyUrl: 'http://egress-proxy:3128' }, + }); + const policy = await client.fetchJobPolicy(JOB_ID); + assert.equal(policy.env.NODE_USE_ENV_PROXY, '1'); + }); + it('splices the per-job proxy credentials into the injected proxy URL', async () => { // The proxy is default-deny and authenticates as Basic base64(jobId:proxyToken). // Standard http clients derive that header from the URL userinfo, so the @@ -324,7 +356,7 @@ describe('policyClient — daemon-owned env keys are injected, never accepted', }); const policy = await client.fetchJobPolicy(JOB_ID, { proxyToken: token }); const expected = `http://${JOB_ID}:${token}@egress-proxy:3128/`; - for (const k of ['HTTP_PROXY', 'HTTPS_PROXY', 'http_proxy', 'https_proxy']) { + for (const k of ['HTTP_PROXY', 'HTTPS_PROXY', 'http_proxy', 'https_proxy', 'npm_config_proxy', 'npm_config_https_proxy']) { assert.equal(policy.env[k], expected, `${k} must carry the job's own credential`); } }); @@ -339,7 +371,18 @@ describe('policyClient — daemon-owned env keys are injected, never accepted', it('injects NO proxy vars when the daemon has none configured', async () => { const client = clientWith(policyBody()); const policy = await client.fetchJobPolicy(JOB_ID); - for (const k of ['HTTP_PROXY', 'HTTPS_PROXY', 'http_proxy', 'https_proxy', 'NO_PROXY', 'no_proxy']) { + for (const k of [ + 'HTTP_PROXY', + 'HTTPS_PROXY', + 'http_proxy', + 'https_proxy', + 'NO_PROXY', + 'no_proxy', + 'NODE_USE_ENV_PROXY', + 'npm_config_proxy', + 'npm_config_https_proxy', + 'npm_config_noproxy', + ]) { assert.equal(policy.env[k], undefined, `${k} must be absent without a configured proxy`); } }); diff --git a/middleware/sidecars/dev-runner-daemon/test/proxy.test.mjs b/middleware/sidecars/dev-runner-daemon/test/proxy.test.mjs index fa067eed2..721730018 100644 --- a/middleware/sidecars/dev-runner-daemon/test/proxy.test.mjs +++ b/middleware/sidecars/dev-runner-daemon/test/proxy.test.mjs @@ -61,7 +61,7 @@ function closeServer(server) { /** * Boot a real proxy with capturing event sink + a resolver seam. - * @param {{ internalHost?: string, internalPort?: number, jobs?: Array<{ jobId: string, allowlist: string[], proxyToken: string, ttlSec?: number }>, resolveMap?: Record> }} opts + * @param {{ internalHost?: string, internalPort?: number, jobs?: Array<{ jobId: string, allowlist: string[], proxyToken: string, ttlSec?: number }>, resolveMap?: Record>, resolveCacheTtlMs?: number, resolveDelayMs?: number, customResolve?: (host: string) => Promise> }} opts */ async function startProxy(opts = {}) { const events = []; @@ -73,8 +73,13 @@ async function startProxy(opts = {}) { const eventClient = { record: (e) => events.push(e), flush: async () => {}, stop: () => {} }; const resolve = async (host) => { resolveCalls.push(host); + if (opts.customResolve) return opts.customResolve(host); // A nameserver that never answers: the tarpit the resolve deadline exists for. if (opts.resolveHangs) return new Promise(() => {}); + // A deliberate delay widens the dedup race window for concurrent-CONNECT + // tests — without it, a same-tick resolve can settle before a second + // caller even asks, which would still be correct but proves nothing. + if (opts.resolveDelayMs) await new Promise((r) => setTimeout(r, opts.resolveDelayMs)); return opts.resolveMap?.[host] ?? [{ address: '127.0.0.1', family: 4 }]; }; const proxy = createProxy({ @@ -87,6 +92,7 @@ async function startProxy(opts = {}) { logger: { warn() {} }, limits: { connectMs: 2000, idleMs: 2000, absoluteMs: 5000 }, ...(opts.resolveTimeoutMs !== undefined ? { resolveTimeoutMs: opts.resolveTimeoutMs } : {}), + ...(opts.resolveCacheTtlMs !== undefined ? { resolveCacheTtlMs: opts.resolveCacheTtlMs } : {}), }); const dataPort = await listen(proxy.dataServer); const controlPort = await listen(proxy.controlServer); @@ -111,7 +117,9 @@ function basicAuth(jobId = JOB_ID, token = PROXY_TOKEN) { /** * Send a raw CONNECT through the proxy and resolve once the status line is parsed. - * @returns {Promise<{ statusCode: number, socket: import('node:net').Socket, buffered: Buffer }>} + * `headers` is the lowercased response header block — a CONNECT reply has no + * `res` object, so the raw text is the only place its framing is observable. + * @returns {Promise<{ statusCode: number, socket: import('node:net').Socket, buffered: Buffer, headers: string }>} */ function sendConnect(dataPort, authority, authHeader) { return new Promise((resolve, reject) => { @@ -124,7 +132,7 @@ function sendConnect(dataPort, authority, authHeader) { socket.removeListener('data', onData); const headerText = buf.subarray(0, idx).toString('utf8'); const statusCode = Number(/^HTTP\/1\.1 (\d+)/.exec(headerText)?.[1] ?? 0); - resolve({ statusCode, socket, buffered: buf.subarray(idx + 4) }); + resolve({ statusCode, socket, buffered: buf.subarray(idx + 4), headers: headerText.toLowerCase() }); }; socket.on('data', onData); socket.on('error', reject); @@ -225,6 +233,72 @@ describe('egress proxy — CONNECT default-deny + DNS-exfil defence', () => { await p.close(); } }); + + // The 407 is a CHALLENGE, and a challenge the client cannot answer is a wall. + // libcurl (so `git`, whose `http.proxyAuthMethod` defaults to `anyauth`) sends an + // unauthenticated CONNECT, reads the 407, then re-sends it WITH credentials. This + // proxy cannot serve that retry on the same socket — node detaches its HTTP parser + // at the `connect` event — so it closes, and it MUST say so. When it did not, the + // client wrote its authenticated retry into an already-FIN'd socket, read EOF, and + // every real job's `git clone` died with "Proxy CONNECT aborted". + it('announces the close on a 407 so a challenged client can retry authenticated', async () => { + // A real tunnel target, so the retry is verified all the way to 200 rather than + // stopping at the decision — same internal-destination shape the end-to-end + // tunnel test uses (loopback is legitimately internal there). + const upstream = await startTcpEcho(); + const p = await startProxy({ + internalHost: 'mw.internal', + internalPort: upstream.port, + jobs: [{ jobId: JOB_ID, allowlist: [], proxyToken: PROXY_TOKEN }], + resolveMap: { 'mw.internal': [{ address: '127.0.0.1', family: 4 }] }, + }); + const authority = `mw.internal:${upstream.port}`; + try { + const challenge = await sendConnect(p.dataPort, authority, null); + assert.equal(challenge.statusCode, 407); + assert.match(challenge.headers, /proxy-authenticate: basic/); + // Both spellings: `Connection` is the standard one, `Proxy-Connection` the + // legacy hop-by-hop one libcurl also honours. + assert.match(challenge.headers, /\r\nconnection: close/); + assert.match(challenge.headers, /\r\nproxy-connection: close/); + assert.match(challenge.headers, /\r\ncontent-length: 0/); + // The announcement must match the behaviour: the proxy really does close. + await new Promise((resolve) => challenge.socket.once('end', resolve)); + challenge.socket.destroy(); + + // The retry libcurl then makes on a FRESH connection must reach the upstream. + const retry = await sendConnect(p.dataPort, authority, basicAuth()); + assert.equal(retry.statusCode, 200, 'the authenticated retry establishes the tunnel'); + // A 2xx must NOT carry the close — it is the tunnel, not a terminal reply. + assert.doesNotMatch(retry.headers, /connection: close/); + retry.socket.write('ping-after-challenge'); + const echoed = await nextChunk(retry.socket); + assert.equal(echoed.toString('utf8'), 'ping-after-challenge'); + retry.socket.destroy(); + } finally { + await p.close(); + await upstream.close(); + } + }); + + // Same trap, non-auth path: every non-2xx CONNECT reply is terminal here, so each + // one has to announce it rather than only the 407 that happened to be reported. + it('announces the close on every non-2xx CONNECT reply, not just the 407', async () => { + const p = await startProxy({ jobs: [{ jobId: JOB_ID, allowlist: ['good.test'], proxyToken: PROXY_TOKEN }] }); + try { + const denied = await sendConnect(p.dataPort, 'notallowed.test:443', basicAuth()); + assert.equal(denied.statusCode, 403); + assert.match(denied.headers, /\r\nconnection: close/); + denied.socket.destroy(); + + const badPort = await sendConnect(p.dataPort, 'good.test:22', basicAuth()); + assert.equal(badPort.statusCode, 403); + assert.match(badPort.headers, /\r\nconnection: close/); + badPort.socket.destroy(); + } finally { + await p.close(); + } + }); }); describe('egress proxy — rebinding defence', () => { @@ -282,6 +356,108 @@ describe('egress proxy — end-to-end tunnel through the vetted IP', () => { }); }); +describe('egress proxy — DNS resolution cache (concurrent same-host CONNECTs share one lookup)', () => { + it('N concurrent CONNECTs to the same host fire exactly ONE underlying resolve call', async () => { + const upstream = await startTcpEcho(); + const p = await startProxy({ + internalHost: 'mw.internal', + internalPort: upstream.port, + jobs: [{ jobId: JOB_ID, allowlist: [], proxyToken: PROXY_TOKEN }], + resolveMap: { 'mw.internal': [{ address: '127.0.0.1', family: 4 }] }, + // Wide enough that all 8 CONNECTs below are dispatched before the + // first underlying lookup would have settled without the cache. + resolveDelayMs: 100, + }); + try { + const results = await Promise.all( + Array.from({ length: 8 }, () => sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth())), + ); + for (const r of results) assert.equal(r.statusCode, 200); + assert.deepEqual(p.resolveCalls, ['mw.internal'], 'exactly one raw resolve call, not eight'); + for (const r of results) r.socket.destroy(); + } finally { + await p.close(); + await upstream.close(); + } + }); + + it('a resolution is reused within the cache TTL, without a second CONNECT even in flight', async () => { + const upstream = await startTcpEcho(); + const p = await startProxy({ + internalHost: 'mw.internal', + internalPort: upstream.port, + jobs: [{ jobId: JOB_ID, allowlist: [], proxyToken: PROXY_TOKEN }], + resolveMap: { 'mw.internal': [{ address: '127.0.0.1', family: 4 }] }, + resolveCacheTtlMs: 60_000, + }); + try { + const first = await sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth()); + assert.equal(first.statusCode, 200); + first.socket.destroy(); + await waitFor(() => p.events.some((e) => e.decision === 'close')); + const second = await sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth()); + assert.equal(second.statusCode, 200); + second.socket.destroy(); + assert.deepEqual(p.resolveCalls, ['mw.internal'], 'the second CONNECT reused the cached resolution'); + } finally { + await p.close(); + await upstream.close(); + } + }); + + it('a fresh lookup runs again once the cache entry expires', async () => { + const upstream = await startTcpEcho(); + const p = await startProxy({ + internalHost: 'mw.internal', + internalPort: upstream.port, + jobs: [{ jobId: JOB_ID, allowlist: [], proxyToken: PROXY_TOKEN }], + resolveMap: { 'mw.internal': [{ address: '127.0.0.1', family: 4 }] }, + resolveCacheTtlMs: 20, + }); + try { + const first = await sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth()); + assert.equal(first.statusCode, 200); + first.socket.destroy(); + await new Promise((r) => setTimeout(r, 40)); + const second = await sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth()); + assert.equal(second.statusCode, 200); + second.socket.destroy(); + assert.deepEqual(p.resolveCalls, ['mw.internal', 'mw.internal'], 'the expired entry triggers a fresh lookup'); + } finally { + await p.close(); + await upstream.close(); + } + }); + + it('a failed resolution is never cached — the next CONNECT gets a fresh attempt', async () => { + const upstream = await startTcpEcho(); + let calls = 0; + const p = await startProxy({ + internalHost: 'mw.internal', + internalPort: upstream.port, + jobs: [{ jobId: JOB_ID, allowlist: [], proxyToken: PROXY_TOKEN }], + // First CONNECT's lookup fails outright; the second must not reuse + // that failure (there is nothing to reuse) and must succeed on retry. + customResolve: async (_host) => { + calls += 1; + if (calls === 1) throw new Error('simulated transient DNS failure'); + return [{ address: '127.0.0.1', family: 4 }]; + }, + }); + try { + const first = await sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth()); + assert.equal(first.statusCode, 502, 'the first CONNECT sees the resolve failure'); + const second = await sendConnect(p.dataPort, `mw.internal:${upstream.port}`, basicAuth()); + assert.equal(second.statusCode, 200, 'the second CONNECT gets a fresh, successful lookup'); + second.socket.destroy(); + assert.equal(calls, 2, 'the failure was not cached — a real second attempt happened'); + } finally { + await p.close(); + await upstream.close(); + } + }); +}); + describe('egress proxy — absolute-form plain HTTP forward', () => { it('forwards a GET to the pinned IP and relays the response', async () => { const upstream = await startHttpUpstream(); @@ -368,6 +544,112 @@ describe('egress proxy — control plane (daemon-token, per-job allowlist push)' }); }); +describe('egress proxy — control plane: POST /resolve (the daemon has no internet route of its own)', () => { + it('rejects an unauthenticated resolve request', async () => { + const p = await startProxy({ resolveMap: { 'a.test': [{ address: '203.0.113.1', family: 4 }] } }); + try { + const res = await controlRequest(p.controlPort, 'POST', '/resolve', 'wrong', { hosts: ['a.test'] }); + assert.equal(res.statusCode, 401); + } finally { + await p.close(); + } + }); + + it('resolves every requested host in one call using the SAME resolver the data plane trusts', async () => { + const p = await startProxy({ + resolveMap: { + 'registry.npmjs.org': [{ address: '104.16.0.35', family: 4 }], + 'github.com': [{ address: '140.82.121.3', family: 4 }], + }, + }); + try { + const res = await controlRequest(p.controlPort, 'POST', '/resolve', DAEMON_TOKEN, { + hosts: ['registry.npmjs.org', 'github.com'], + }); + assert.equal(res.statusCode, 200); + const byHost = Object.fromEntries(res.body.results.map((r) => [r.host, r.addresses])); + assert.deepEqual(byHost['registry.npmjs.org'], [{ address: '104.16.0.35', family: 4 }]); + assert.deepEqual(byHost['github.com'], [{ address: '140.82.121.3', family: 4 }]); + assert.deepEqual(p.resolveCalls.sort(), ['github.com', 'registry.npmjs.org']); + } finally { + await p.close(); + } + }); + + it('reports null addresses for a host that fails to resolve — one bad host does not fail the whole batch', async () => { + const p = await startProxy({ + customResolve: async (host) => { + if (host === 'flaky.example.com') throw new Error('simulated DNS failure'); + return [{ address: '203.0.113.10', family: 4 }]; + }, + }); + try { + const res = await controlRequest(p.controlPort, 'POST', '/resolve', DAEMON_TOKEN, { + hosts: ['flaky.example.com', 'good.example.com'], + }); + assert.equal(res.statusCode, 200); + const byHost = Object.fromEntries(res.body.results.map((r) => [r.host, r.addresses])); + assert.equal(byHost['flaky.example.com'], null); + assert.deepEqual(byHost['good.example.com'], [{ address: '203.0.113.10', family: 4 }]); + } finally { + await p.close(); + } + }); + + it('400s on an empty, missing, or oversized hosts array — never a silent no-op', async () => { + const p = await startProxy(); + try { + const empty = await controlRequest(p.controlPort, 'POST', '/resolve', DAEMON_TOKEN, { hosts: [] }); + assert.equal(empty.statusCode, 400); + const missing = await controlRequest(p.controlPort, 'POST', '/resolve', DAEMON_TOKEN, {}); + assert.equal(missing.statusCode, 400); + const oversized = await controlRequest(p.controlPort, 'POST', '/resolve', DAEMON_TOKEN, { + hosts: Array.from({ length: 101 }, (_, i) => `h${i}.test`), + }); + assert.equal(oversized.statusCode, 400); + } finally { + await p.close(); + } + }); +}); + +describe('createProxyClient — resolveHosts (the daemon-side caller of POST /resolve)', () => { + it('calls POST /resolve with the bearer token and returns the results array', async () => { + const p = await startProxy({ + resolveMap: { 'registry.npmjs.org': [{ address: '104.16.0.35', family: 4 }] }, + }); + try { + const client = createProxyClient({ controlUrl: `http://127.0.0.1:${p.controlPort}`, token: DAEMON_TOKEN }); + const results = await client.resolveHosts(['registry.npmjs.org']); + assert.deepEqual(results, [{ host: 'registry.npmjs.org', addresses: [{ address: '104.16.0.35', family: 4 }] }]); + } finally { + await p.close(); + } + }); + + it('an empty hosts array short-circuits — no request is made', async () => { + const p = await startProxy(); + try { + const client = createProxyClient({ controlUrl: `http://127.0.0.1:${p.controlPort}`, token: DAEMON_TOKEN }); + const results = await client.resolveHosts([]); + assert.deepEqual(results, []); + assert.deepEqual(p.resolveCalls, []); + } finally { + await p.close(); + } + }); + + it('throws ProxyControlError on a wrong token, never silently returning empty', async () => { + const p = await startProxy(); + try { + const client = createProxyClient({ controlUrl: `http://127.0.0.1:${p.controlPort}`, token: 'wrong-token' }); + await assert.rejects(() => client.resolveHosts(['a.test']), /proxy refused to resolve hosts/); + } finally { + await p.close(); + } + }); +}); + /** PUT /jobs/:id on the control plane. */ function controlPut(controlPort, jobId, body, token) { return controlRequest(controlPort, 'PUT', `/jobs/${jobId}`, token, body); @@ -421,6 +703,71 @@ describe('proxy — a tarpit nameserver cannot park connections before the limit }); }); +describe('proxy — a client socket reset before the tunnel exists must not crash the process', () => { + it('an ECONNRESET during the DNS-resolution window is handled, not thrown as an unhandled socket error', async () => { + // Found live: `clientSocket` (the raw net.Socket a CONNECT upgrade hands + // over) has NO 'error' listener attached until handleConnect's success + // path reaches `clientSocket.on('error', teardown)` -- well after the + // allowlist decision AND the `await resolve(host)` call. A client + // resetting the connection during that window fires an unhandled + // 'error' event; Node's default for a listener-less EventEmitter + // 'error' is to throw, which crashed the ENTIRE egress proxy process -- + // taking every OTHER concurrent job's egress down with it, restarted + // only by the container's own restart policy. + // + // resolveHangs keeps the CONNECT stuck in exactly that vulnerable + // pre-tunnel window indefinitely, so the reset below is guaranteed to + // land while it's still open. + const p = await startProxy({ + jobs: [{ jobId: JOB_ID, allowlist: ['good.test'], proxyToken: PROXY_TOKEN }], + resolveHangs: true, + }); + try { + const socket = netConnect({ host: '127.0.0.1', port: p.dataPort }); + await new Promise((resolve, reject) => { + socket.once('connect', resolve); + socket.once('error', reject); + }); + socket.write(`CONNECT good.test:443 HTTP/1.1\r\nHost: good.test:443\r\nProxy-Authorization: ${basicAuth()}\r\n\r\n`); + // Give the proxy a moment to receive the CONNECT and enter + // handleConnect's `await resolve(host)` (resolveHangs keeps it + // pending forever, so this window stays open indefinitely). + await new Promise((r) => setTimeout(r, 50)); + // resetAndDestroy sends a real TCP RST (Node 16.17+) rather than a + // clean FIN, so the SERVER side observes an 'error' event (ECONNRESET), + // not just 'close' -- the actual crash-reproducing case, not a milder + // graceful-disconnect one `socket.destroy()` alone wouldn't exercise. + if (typeof socket.resetAndDestroy === 'function') socket.resetAndDestroy(); + else socket.destroy(new Error('simulated reset')); + + // If the bug were present, the proxy process would have thrown an + // uncaught exception and died right about now — no further code in + // this process would ever run again. Reaching this assertion at all + // (on a freshly-issued, unrelated request) is itself the proof; a + // dead process cannot answer it. (internalHost/resolveMap mirrors the + // "end-to-end tunnel" test above — a real upstream + allowInternal so + // the loopback resolution isn't itself rejected as a rebind.) + const upstream = await startTcpEcho(); + const other = await startProxy({ + internalHost: 'still-alive.internal', + internalPort: upstream.port, + jobs: [{ jobId: JOB_ID, allowlist: [], proxyToken: PROXY_TOKEN }], + resolveMap: { 'still-alive.internal': [{ address: '127.0.0.1', family: 4 }] }, + }); + try { + const res = await sendConnect(other.dataPort, `still-alive.internal:${upstream.port}`, basicAuth()); + assert.equal(res.statusCode, 200, 'the process survived the reset and can still serve a normal request'); + res.socket.destroy(); + } finally { + await other.close(); + await upstream.close(); + } + } finally { + await p.close(); + } + }); +}); + /** * Epic #470 W1 — the whole egress chain, end to end, over real sockets. * diff --git a/middleware/sidecars/dev-runner/Dockerfile b/middleware/sidecars/dev-runner/Dockerfile index 67ea86d22..508f1abee 100644 --- a/middleware/sidecars/dev-runner/Dockerfile +++ b/middleware/sidecars/dev-runner/Dockerfile @@ -37,9 +37,15 @@ ARG CLAUDE_CODE_VERSION=2.1.187 # git + ca-certificates: HTTPS-only clone (W1 has NO openssh-client — SSH clone # is out of scope, spec §3). ripgrep: the CLI's file search. tini: PID 1 that # reaps the CLI and git children on SIGTERM so a cancelled job leaves no -# zombies. The CLI itself is installed globally at the pinned version. +# zombies. python3/make/g++: node-gyp's compile-from-source fallback for +# native deps (e.g. better-sqlite3) whose prebuild-install step needs a +# github.com -> objects.githubusercontent.com redirect our default-deny egress +# proxy doesn't allowlist (confirmed live, epic #470, 2026-07-29) — building +# from source keeps the egress allowlist tight instead of opening it up for a +# CDN redirect chain. The CLI itself is installed globally at the pinned +# version. RUN apt-get update \ - && apt-get install -y --no-install-recommends git ca-certificates ripgrep tini \ + && apt-get install -y --no-install-recommends git ca-certificates ripgrep tini python3 make g++ \ && rm -rf /var/lib/apt/lists/* \ && npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}" \ && npm cache clean --force diff --git a/middleware/src/config.ts b/middleware/src/config.ts index 17d5416d4..de63ee5c9 100644 --- a/middleware/src/config.ts +++ b/middleware/src/config.ts @@ -236,7 +236,7 @@ const ConfigSchema = z.object({ // Epic #470 W4 — default per-job LLM cost budget (USD) applied when neither the // job nor its repo sets one (spec §5). Token budgets have NO default: they are // enforced only when explicitly set on the job or repo. - DEV_JOB_DEFAULT_BUDGET_USD: z.coerce.number().positive().default(5), + DEV_JOB_DEFAULT_BUDGET_USD: z.coerce.number().positive().default(100), // Postgres connection string for the Neon-backed knowledge graph. // When set, `bootstrapKnowledgeGraphFromEnv` installs the diff --git a/middleware/src/devplatform/devJobStore.ts b/middleware/src/devplatform/devJobStore.ts index 72421053e..f5d65affc 100644 --- a/middleware/src/devplatform/devJobStore.ts +++ b/middleware/src/devplatform/devJobStore.ts @@ -16,12 +16,13 @@ import type { Pool } from 'pg'; import * as artifacts from './devJobArtifactStore.js'; import type { DevJobEventBus } from './devJobEventBus.js'; import * as seams from './devJobWorkerSeams.js'; -import { hashRunnerToken, verifyRunnerToken as verifyToken } from './jobToken.js'; +import { hashRunnerToken, mintRunnerToken, verifyRunnerToken as verifyToken } from './jobToken.js'; import { asObj, iso, isoN, num, str, strN, type Row } from './pgMappers.js'; import { isLowValueEventType, type ArtifactCeilingOptions } from './retention.js'; import { isDevJobEventType, isTerminalDevJobStatus, + TERMINAL_DEV_JOB_STATUSES, type DevJob, type DevJobArtifact, type DevJobEvent, @@ -205,6 +206,16 @@ export class DevJobStore { // `status` is threaded so a gated trigger job can be born `'waiting'` in this // single INSERT (never transiently `'queued'` and therefore never claimable); // omitted ⇒ `'queued'` (the DB default, kept explicit here for the same value). + // + // `phase` defaults to `'analyze'`, not `'implement'`: every pipeline_mode + // (gated AND collapsed) is designed to start there per transitions.ts's own + // test suite ("collapsed mode skips THE GATE" still begins `analyze → + // implement`) and the dev-runner-shim's own default + // (`phaseLoop.ts`: `ctx?.phase ?? 'analyze'`). Only `kind === 'analyze'` + // jobs terminate immediately after that phase; `fix_issue` and `implement` + // jobs continue through bootstrap/plan/clarify/gate exactly like any other + // gated job — an explicit `phase` override (e.g. the gated-webhook trigger + // parking straight at `'await_human'`) still wins. const r = await this.pool.query( `INSERT INTO dev_jobs (repo_id, kind, brief, source, source_ref, base_sha, backend, agent_kind, auth_mode, @@ -222,7 +233,7 @@ export class DevJobStore { input.agentKind ?? 'claude-cli', input.authMode ?? 'api_key', input.provision ?? 1, - input.phase ?? 'implement', + input.phase ?? 'analyze', input.status ?? 'queued', input.branch ?? null, input.runnerTokenHash, @@ -237,6 +248,23 @@ export class DevJobStore { return r.rows[0] ? toJob(r.rows[0]) : null; } + /** + * Delete one job's row — `ON DELETE CASCADE` (0022) removes its events and + * artifacts in the same statement, same as `retention.purgeTerminalJobs` + * (spec §7), just for a single operator-named job instead of an age sweep. + * Scoped to terminal statuses only: an active job still has a live backend + * handle (container, Fly Machine) that deleting the row would orphan — + * terminate it first (which finalizes the job), then delete. + */ + async deleteJob(id: string): Promise<'deleted' | 'not_terminal' | 'not_found'> { + const r = await this.pool.query( + `DELETE FROM dev_jobs WHERE id = $1 AND status = ANY($2::text[])`, + [id, [...TERMINAL_DEV_JOB_STATUSES]], + ); + if ((r.rowCount ?? 0) > 0) return 'deleted'; + return (await this.getJob(id)) ? 'not_terminal' : 'not_found'; + } + async listJobs(filter: ListJobsFilter = {}): Promise { const where: string[] = []; const params: unknown[] = []; @@ -718,6 +746,35 @@ export class DevJobStore { } // --- tokens -------------------------------------------------------------- + /** + * Mint a fresh runner token for a job whose container the DockerBackend path + * has not yet spawned, and persist only its hash — same one-time-plaintext + * contract `mintRunnerToken` documents for every backend, just exercised at a + * different moment. + * + * Why this exists: `createJob` already stamps a `runner_token_hash` for every + * job, but for the docker backend that first token's plaintext is discarded + * unused — `DockerBackend.provision()` deliberately posts only + * `{ protocol, jobId, leaseTtlSec }` to the daemon (spec §4/§5, review finding + * S3: "a caller never dictates policy"), so the token can never ride that + * call. The daemon fetches the job's policy itself, later, on its own clock + * (`GET /internal/job-policy/:jobId`) — THAT is this backend's actual + * provision moment, so this reissues the token right there, replacing the + * unused original hash, and the plaintext rides the policy response's `env` + * (the daemon's own `ALLOWED_ENV_KEYS` already special-cases `OMADIA_JOB_TOKEN` + * as policy-supplied — see `policyClient.mjs`). One reissue per policy fetch, + * which is one per container spawn, so the token a runner receives always + * matches the runner_token_hash a `verifyRunnerToken` call against it will see. + */ + async reissueRunnerToken(jobId: string): Promise { + const { token, hash } = mintRunnerToken(); + await this.pool.query(`UPDATE dev_jobs SET runner_token_hash = $2, updated_at = now() WHERE id = $1`, [ + jobId, + hash, + ]); + return token; + } + /** sha256 + timing-safe check of a presented runner token against the stored * hash. Unknown job ⇒ false. */ async verifyRunnerToken(jobId: string, token: string): Promise { diff --git a/middleware/src/devplatform/wireDevPlatform.ts b/middleware/src/devplatform/wireDevPlatform.ts index 2d2bdd789..95f500791 100644 --- a/middleware/src/devplatform/wireDevPlatform.ts +++ b/middleware/src/devplatform/wireDevPlatform.ts @@ -457,9 +457,15 @@ export function assembleDevPlatform(deps: WireDevPlatformDeps): WiredDevPlatform // PhaseEngine's terminal choke point is the SAME boundFinalize every other // path uses — so a phase-driven fail/done revokes the job's scoped tokens too. // Adapt the signature: the engine passes a bare `reason`, boundFinalize takes - // a FinalizeContext (`reason` lands in the status event payload). + // a FinalizeContext. `reason` lands in the status event payload (`ctx.reason`) + // AND `dev_jobs.error` (`ctx.error`) — two distinct FinalizeContext fields; + // populating only `reason` left `dev_jobs.error` silently empty on every + // gated-pipeline phase failure (the real reason was only ever visible in the + // event trail, never on the job row itself). finalize: (jobId, status, reason) => - boundFinalize(jobId, status, reason !== undefined ? { reason } : undefined).then(() => undefined), + boundFinalize(jobId, status, reason !== undefined ? { reason, error: reason } : undefined).then( + () => undefined, + ), // A parked runner is exiting: revoke its scoped token WITHOUT finalizing the // still-`waiting` job. Same registry revoker the terminal paths use. revokeTokensForPark: async (job) => { diff --git a/middleware/src/index.ts b/middleware/src/index.ts index 82cf1e72e..bf629e577 100644 --- a/middleware/src/index.ts +++ b/middleware/src/index.ts @@ -2117,6 +2117,21 @@ async function main(): Promise { console.log('[middleware] dev-platform GitHub webhook router mounted at /api/webhooks/github (raw-body, before express.json)'); } + // The LLM proxy (`/api/v1/dev-runner/llm/*`, mounted later at `mountDevPlatform`) + // owns its own route-level `express.raw()` so it can canonicalise the exact bytes + // it validates before forwarding (see llmProxy.ts). A global body parser that runs + // BEFORE that route is reached would drain the request stream first: body-parser's + // `read()` bails out via `onFinished.isFinished(req)` on an already-consumed stream + // and never touches `req.body` again, so the route's own raw() would then see a + // pre-parsed object instead of a Buffer. Skip this one path here, mirroring the + // GitHub-webhook router's raw-body-before-json pattern above. + app.use((req, res, next) => { + if (req.path.startsWith('/api/v1/dev-runner/llm/')) { + next(); + return; + } + express.json({ limit: '10mb' })(req, res, next); + }); // Issue #437 — Conductor's generic inbound webhook route. Mounted unconditionally // (mirrors the forward-reference pattern, not the `if (graphPool)` gate above): on // the in-memory backend `conductorWebhookInboundDepsRef` never gets assigned, so the @@ -2596,13 +2611,20 @@ async function main(): Promise { // W2: role-principal gates resolve their live holder set against the same // conductor role store the conductor await gate uses. const devPlatformRoleStore = new ConductorRoleStore(graphPool); - // Epic #470 W4 — resolve the FlyMachinesBackend config when a dedicated runner - // app is set. The on-/off-Fly selection lives HERE so the assembly layer stays - // env-free: on Fly (FLY_APP_NAME injected) use the internal Machines API + a - // `.internal` 6PN phone-home address; off Fly use the public endpoints. Requires - // a digest-pinned image (DEV_RUNNER_IMAGE, falling back to DEV_RUNNER_DEFAULT_IMAGE). - // These operator URLs are DELIBERATELY not SSRF-guarded (`.internal` is valid here). - const flyRunnerImage = config.DEV_RUNNER_IMAGE ?? config.DEV_RUNNER_DEFAULT_IMAGE; + // The runner image, shared by every backend: FlyMachinesBackend (below) AND + // the DockerBackend job-policy config (assembleDevPlatform's `runnerImage`, + // further down) both derive from this one resolution. `DEV_RUNNER_IMAGE` + // wins when set (it's the name the daemon's own DEV_RUNNER_IMAGES/allowlist + // config uses too, so one operator-set var keeps every side in agreement); + // `DEV_RUNNER_DEFAULT_IMAGE` is the fallback. A digest-pinned image is + // required on Fly (enforced below); locally a floating tag is fine. + // + // Epic #470 W4 — the on-/off-Fly selection for the Machines backend lives + // HERE so the assembly layer stays env-free: on Fly (FLY_APP_NAME injected) + // use the internal Machines API + a `.internal` 6PN phone-home address; off + // Fly use the public endpoints. These operator URLs are DELIBERATELY not + // SSRF-guarded (`.internal` is valid here). + const resolvedRunnerImage = config.DEV_RUNNER_IMAGE ?? config.DEV_RUNNER_DEFAULT_IMAGE; // The runner app MUST be dedicated — NEVER this middleware's own Fly app, or a // job's ephemeral machine (running hostile repo code) would be provisioned into // the app that holds the middleware's machines, volumes, and app-level secrets @@ -2616,13 +2638,13 @@ async function main(): Promise { ); } const flyConfig = - config.DEV_FLY_RUNNER_APP && flyRunnerImage && !flyAppIsSelf + config.DEV_FLY_RUNNER_APP && resolvedRunnerImage && !flyAppIsSelf ? { runnerApp: config.DEV_FLY_RUNNER_APP, apiBase: config.FLY_APP_NAME ? 'http://_api.internal:4280/v1' : 'https://api.machines.dev/v1', - image: flyRunnerImage, + image: resolvedRunnerImage, phoneHomeUrl: config.DEV_FLY_PHONE_HOME_URL ?? (config.FLY_APP_NAME @@ -2638,7 +2660,7 @@ async function main(): Promise { ...(config.DEV_FLY_REGION ? { region: config.DEV_FLY_REGION } : {}), } : undefined; - if (config.DEV_FLY_RUNNER_APP && !flyRunnerImage) { + if (config.DEV_FLY_RUNNER_APP && !resolvedRunnerImage) { console.warn( '[middleware] DEV_FLY_RUNNER_APP set but no runner image (DEV_RUNNER_IMAGE / DEV_RUNNER_DEFAULT_IMAGE) — FlyMachinesBackend NOT registered', ); @@ -2665,7 +2687,7 @@ async function main(): Promise { ...(config.DEV_RUNNER_DAEMON_URL ? { daemonUrl: config.DEV_RUNNER_DAEMON_URL } : {}), backend: config.DEV_PLATFORM_BACKEND, leaseTtlSec: config.DEV_JOB_LEASE_TTL_SEC, - ...(config.DEV_RUNNER_DEFAULT_IMAGE ? { runnerImage: config.DEV_RUNNER_DEFAULT_IMAGE } : {}), + ...(resolvedRunnerImage ? { runnerImage: resolvedRunnerImage } : {}), ...(config.DEV_EGRESS_BASE_ALLOWLIST ? { egressBaseAllowlist: csvList(config.DEV_EGRESS_BASE_ALLOWLIST) } : {}), diff --git a/middleware/src/routes/devPlatform.ts b/middleware/src/routes/devPlatform.ts index a61742155..c410eea68 100644 --- a/middleware/src/routes/devPlatform.ts +++ b/middleware/src/routes/devPlatform.ts @@ -211,6 +211,30 @@ export function createDevPlatformRouter(deps: DevPlatformRouterDeps): Router { }), ); + // --- DELETE /jobs/:id ------------------------------------------------- + // Removes a terminal job's row (and its events/artifacts, via CASCADE) so it + // stops cluttering the operator's job list. Refuses an active job (409) — + // it still has a live backend handle; cancel it first, which finalizes it. + router.delete( + '/jobs/:id', + handler(async (req, res) => { + const caller = requireCaller(req); + const job = await loadAuthorizedJob(deps, req, caller); + const outcome = await deps.jobStore.deleteJob(job.id); + if (outcome === 'not_terminal') { + throw new DevPlatformError( + 409, + 'devplatform.job_not_terminal', + 'the job is still active — cancel it before deleting', + ); + } + // 'not_found' here means it was deleted between the authorize-load above + // and this call (e.g. the daily retention sweep); still a success from + // the caller's point of view — the job is gone either way. + res.status(204).end(); + }), + ); + // --- POST /jobs/:id/apply ------------------------------------------------- // Retry of the host-side apply. 409 unless `applying` or failed-after-diff. router.post( @@ -237,6 +261,23 @@ export function createDevPlatformRouter(deps: DevPlatformRouterDeps): Router { // W0: re-queue by cloning the job into a fresh queued row (a new runner token; // the row's one-time-token invariant forbids reusing the old one). Allowed // only once the source job has finished. + // + // `?resumeFromPhase=true` starts the clone at the SOURCE job's own `phase` + // (wherever it was when it failed) instead of always restarting at `analyze`. + // This is safe by construction: `dev_jobs.phase` is exactly the value the + // dev-runner-shim reads to decide where to begin (protocol.ts's own + // `ProvisionSpec.phase` doc: "Phase the runner begins at"), so handing a new + // job the old job's last-attempted phase reproduces the same starting point + // the runner already knows how to execute — no new runner-side code path. + // The one thing the runner-shim assumes that a fresh job wouldn't otherwise + // have is the artifacts EARLIER phases already produced (e.g. `plan` reads + // the `analysis` artifact by job id) — those are copied onto the new job's + // own row so any later phase finds them under its own id, same as if this + // job had produced them itself. Bootstrap-only failures (this feature's + // primary motivation — a real dev job costs ~$1-2 in `analyze` LLM tokens + // before ever reaching the free, no-LLM `bootstrap` shell step) need no + // artifact at all; this still copies whatever exists so it generalizes to + // any later phase without special-casing which phase needs which artifact. router.post( '/jobs/:id/retry', handler(async (req, res) => { @@ -250,6 +291,7 @@ export function createDevPlatformRouter(deps: DevPlatformRouterDeps): Router { if (!isPermittedLauncher(repo, caller)) { throw new DevPlatformError(403, 'devplatform.not_launcher', 'not a permitted launcher for this repository'); } + const resumeFromPhase = req.query['resumeFromPhase'] === 'true'; const minted = mintRunnerToken(); const next = await deps.jobStore.createJob({ repoId: job.repoId, @@ -261,8 +303,15 @@ export function createDevPlatformRouter(deps: DevPlatformRouterDeps): Router { authMode: job.authMode, createdBy: caller.sub, runnerTokenHash: minted.hash, + ...(resumeFromPhase ? { phase: job.phase } : {}), }); - res.status(202).json({ ok: true, jobId: next.id }); + if (resumeFromPhase) { + const priorArtifacts = await deps.jobStore.listArtifacts(job.id); + for (const artifact of priorArtifacts) { + await deps.jobStore.addArtifact(next.id, artifact.kind, artifact.content, artifact.meta); + } + } + res.status(202).json({ ok: true, jobId: next.id, ...(resumeFromPhase ? { resumedAtPhase: next.phase } : {}) }); }), ); diff --git a/middleware/src/routes/devPlatformShared.ts b/middleware/src/routes/devPlatformShared.ts index 1ee668ccb..ea50b55f6 100644 --- a/middleware/src/routes/devPlatformShared.ts +++ b/middleware/src/routes/devPlatformShared.ts @@ -58,6 +58,12 @@ export interface DevPlatformJobStore { listEvents(jobId: string, afterId?: number, limit?: number): Promise; listArtifacts(jobId: string): Promise; getArtifact(id: string): Promise; + deleteJob(id: string): Promise<'deleted' | 'not_terminal' | 'not_found'>; + /** Used only by `/jobs/:id/retry?resumeFromPhase=true` to seed a fresh job's + * artifacts from the failed job it's resuming, so a later phase (e.g. `plan` + * reading `analysis`) still finds what it needs without re-running the + * phases that already succeeded. */ + addArtifact(jobId: string, kind: string, content: string, meta?: Record): Promise; } /** The `DevRepoCredentialStore` surface. Never returns a token to the browser — diff --git a/middleware/src/routes/devRunnerApi.ts b/middleware/src/routes/devRunnerApi.ts index 37ff1fe6e..990190d72 100644 --- a/middleware/src/routes/devRunnerApi.ts +++ b/middleware/src/routes/devRunnerApi.ts @@ -58,6 +58,10 @@ import { export interface DevRunnerJobStore { verifyRunnerToken(jobId: string, token: string): Promise; getJob(jobId: string): Promise; + /** Mints and persists a fresh runner token, invalidating the previous one. + * See `devRunnerJobPolicyRoute.ts` — the docker backend's actual provision + * moment, since `DockerBackend.provision()` itself never carries a token. */ + reissueRunnerToken(jobId: string): Promise; markRunning(jobId: string): Promise; /** Liveness without an event. `appendEvents` returns early on an empty batch, * so an agent that thinks without emitting a tool call would otherwise be diff --git a/middleware/src/routes/devRunnerJobPolicyRoute.ts b/middleware/src/routes/devRunnerJobPolicyRoute.ts index 400826e1e..18c29f6e7 100644 --- a/middleware/src/routes/devRunnerJobPolicyRoute.ts +++ b/middleware/src/routes/devRunnerJobPolicyRoute.ts @@ -10,6 +10,15 @@ * bearer: a runner token is rejected here, or any runner could read another * job's policy. It therefore does NOT use the runner router's job-bearer * `authMw`; it has its own daemon-token guard below. + * + * This request IS the docker backend's actual provision moment (`DockerBackend + * .provision()` itself posts only `{ protocol, jobId, leaseTtlSec }` — no env, + * spec §4/§5). So `env` carries a freshly-reissued `OMADIA_JOB_TOKEN` alongside + * `deriveJobPolicy`'s non-secret fields — the daemon's own `ALLOWED_ENV_KEYS` + * already treats it as policy-supplied (`policyClient.mjs`). Reissuing here + * (rather than reusing `createJob`'s original, unused-for-this-backend token) + * replaces `runner_token_hash`, so exactly the token this response hands out is + * the one a later `verifyRunnerToken` call will accept. */ import { createHash, timingSafeEqual } from 'node:crypto'; @@ -25,7 +34,10 @@ import type { DevJob, DevRepo } from '../devplatform/types.js'; /** The narrow slices this route needs from the runner router's deps. */ export interface JobPolicyRouteDeps { - store: { getJob(jobId: string): Promise }; + store: { + getJob(jobId: string): Promise; + reissueRunnerToken(jobId: string): Promise; + }; repos: { getRepo(id: string): Promise | null>; }; @@ -124,10 +136,16 @@ export function mountJobPolicyRoute(router: Router, deps: JobPolicyRouteDeps): v fail(res, 500, code, 'job policy could not be derived'); return; } + // The docker backend's actual provision moment (see file docstring) — mint + // the token the runner authenticates back to the middleware with here, + // never earlier: `createJob`'s original token was never handed to a + // container for this backend, so reissuing (not reusing it) keeps exactly + // one plaintext copy in existence, matching every other backend's contract. + const runnerToken = await store.reissueRunnerToken(job.id); res.json({ jobId: job.id, image: policy.image, - env: policy.env, + env: { ...policy.env, OMADIA_JOB_TOKEN: runnerToken }, egressAllowlist: policy.egressAllowlist, // W5 (spec §8): the daemon reads this to decide whether to run a DinD sidecar. dockerInJob: policy.dockerInJob, diff --git a/middleware/test/devplatform/composeTopology.test.ts b/middleware/test/devplatform/composeTopology.test.ts index 4578edf84..2634c5726 100644 --- a/middleware/test/devplatform/composeTopology.test.ts +++ b/middleware/test/devplatform/composeTopology.test.ts @@ -251,6 +251,20 @@ describe('dev-platform compose overlay — one image, two services, two commands // never an image. `parseAllowedImages` throws when this is absent. assert.ok(overlay.services['dev-runner-daemon']!.environment!['DEV_RUNNER_ALLOWED_IMAGES']); }); + + it('actually forwards DEV_RUNNER_REQUIRE_DIGEST into the daemon container', () => { + // A var that only exists in a comment is not configuration. Before this key + // was added to `environment:`, `env.DEV_RUNNER_REQUIRE_DIGEST` was always + // undefined inside the container regardless of what .env said, and + // `parseRequireDigest` silently defaults undefined to `true` — so every + // locally-built, non-digest-pinned image was refused, no matter how the + // operator set the var. The key must be PRESENT (any value, incl. the + // default 'true'); its absence is the actual bug this guards. + assert.ok( + 'DEV_RUNNER_REQUIRE_DIGEST' in (overlay.services['dev-runner-daemon']!.environment ?? {}), + 'DEV_RUNNER_REQUIRE_DIGEST must be forwarded, not just documented in a comment', + ); + }); }); describe('dev-platform compose overlay — the MERGED config, not just the overlay map', { skip: !merged }, () => { @@ -272,3 +286,71 @@ describe('dev-platform compose overlay — the MERGED config, not just the overl } }); }); + +describe('dev-platform compose overlay — the middleware can actually derive a job policy', () => { + // Without a runner image, `wireDevPlatform`'s jobPolicyConfig never builds and + // GET /internal/job-policy/:jobId 503s forever — every DockerBackend provision + // fails at the first real container (the implement phase; analyze/plan/clarify + // don't need one, so this gap is invisible until a real job actually runs). + // This was true of the shipped overlay for the whole life of the epic. + it('gives the middleware a runner image, not just the daemon', () => { + const env = overlay.services['middleware']?.environment ?? {}; + assert.ok( + env['DEV_RUNNER_DEFAULT_IMAGE'] || env['DEV_RUNNER_IMAGE'], + 'middleware needs DEV_RUNNER_DEFAULT_IMAGE (or DEV_RUNNER_IMAGE) or every job dies at implement with a 502', + ); + }); + + it('agrees with the daemon on which image that is', () => { + // Same source var (DEV_RUNNER_IMAGE) feeds both sides, so an operator who + // sets it once cannot end up with the daemon allowing image A while the + // middleware's policy names image B. + const middlewareImage = overlay.services['middleware']?.environment?.['DEV_RUNNER_DEFAULT_IMAGE']; + const daemonImages = overlay.services['dev-runner-daemon']?.environment?.['DEV_RUNNER_IMAGES']; + assert.ok(middlewareImage, 'middleware image must be set to compare'); + assert.ok(daemonImages?.includes(middlewareImage as string), 'daemon and middleware must name the same image'); + }); + + it('never tells the runner to bypass the proxy for the middleware', () => { + // Job containers are created by dind on their own per-job network, which has + // NO route to dev-control -- the network `middleware` actually lives on. + // Only the proxy is dual-homed onto dev-egress (job-reachable) and + // dev-control (middleware-reachable). Bypassing the proxy for "middleware" + // routes phone-home into `getaddrinfo ENOTFOUND middleware` from inside the + // job's network -- exactly where every real job died after the + // runner-image/digest/token gates were fixed. The proxy's own egress policy + // already allows this host+port through (egressPolicy.mjs's `allowInternal` + // match against OMADIA_INTERNAL_API_URL), so there is no reason to bypass it. + const noProxy = overlay.services['dev-runner-daemon']?.environment?.['DEV_RUNNER_NO_PROXY'] ?? ''; + const entries = noProxy.split(',').map((s) => s.trim()); + assert.ok(!entries.includes('middleware'), 'middleware must route THROUGH the proxy, never around it'); + }); +}); + +describe('dev-platform compose overlay — the egress proxy can actually reach the internet', () => { + // Every job-egress network (dev-control, dev-engine, dev-egress) is + // deliberately `internal: true` -- correctly, none of them may reach + // outside. But dev-egress-proxy's ONLY job is being the one path a job + // container has to the real internet, and its `networks:` list used to name + // ONLY those internal ones -- so the proxy itself had no route out either, + // and every job's egress (git clone, npm install, ...) failed DNS resolution + // before the allowlist/CONNECT logic ever ran (verified live: + // `getaddrinfo EAI_AGAIN github.com` from inside the proxy container). + it('joins at least one network that is not internal: true', () => { + const proxyNetNames = networkNames(overlay.services['dev-egress-proxy']); + const external = proxyNetNames.filter((n) => overlay.networks?.[n]?.internal !== true); + assert.ok( + external.length > 0, + `dev-egress-proxy's networks (${proxyNetNames.join(', ')}) are ALL internal -- it has no path to the real internet`, + ); + }); + + it('does not reach that network by sharing `omadia` with the app services', () => { + // Sharing the app's own bridge would make the proxy reachable from (and + // able to reach) middleware/web-ui laterally -- exactly what a separate + // egress plane exists to avoid. Its external route must be a network + // dedicated to it alone. + const proxyNetNames = networkNames(overlay.services['dev-egress-proxy']); + assert.ok(!proxyNetNames.includes('omadia'), 'the proxy must not join the app network for its egress route'); + }); +}); diff --git a/middleware/test/devplatform/deriveJobPolicy.test.ts b/middleware/test/devplatform/deriveJobPolicy.test.ts index dd5307a64..2cf2d5eb9 100644 --- a/middleware/test/devplatform/deriveJobPolicy.test.ts +++ b/middleware/test/devplatform/deriveJobPolicy.test.ts @@ -282,10 +282,20 @@ describe('devRunnerApi — internal job-policy endpoint', () => { }; assert.equal(body.jobId, 'job-1'); assert.equal(body.image, CONFIG.image); - assert.deepEqual(body.env, { ANTHROPIC_BASE_URL: CONFIG.llmProxyBaseUrl, OMADIA_PIPELINE_MODE: 'gated' }); + // OMADIA_JOB_TOKEN is the ONE deliberate exception to "no secret in the + // derived env" (deriveJobPolicy.ts's own invariant, untouched): this route + // IS the docker backend's actual provision moment (DockerBackend.provision() + // itself never carries a token, spec S3), so it mints one here and the + // daemon's own ALLOWED_ENV_KEYS already treats it as policy-supplied. + assert.deepEqual(body.env, { + ANTHROPIC_BASE_URL: CONFIG.llmProxyBaseUrl, + OMADIA_PIPELINE_MODE: 'gated', + OMADIA_JOB_TOKEN: 'djr_reissued-1', + }); assert.ok(body.egressAllowlist.includes('artifactory.internal'), 'repo allowlist entry is present'); assert.ok(body.egressAllowlist.includes('middleware')); - assert.equal(hasCredentialKey(body.env), false, 'policy env carries no credential-like key'); + const { OMADIA_JOB_TOKEN: _theOneIntentionalToken, ...rest } = body.env; + assert.equal(hasCredentialKey(rest), false, 'no OTHER credential-like key beyond the one deliberate token'); }); it('REJECTS a per-job djr_ runner bearer (S3: no runner may read a policy)', async () => { diff --git a/middleware/test/devplatform/devJobStore.pg.test.ts b/middleware/test/devplatform/devJobStore.pg.test.ts index 3a8aa02d8..9c105afba 100644 --- a/middleware/test/devplatform/devJobStore.pg.test.ts +++ b/middleware/test/devplatform/devJobStore.pg.test.ts @@ -92,11 +92,14 @@ describe('devplatform/DevJobStore (pg)', { skip: !pgAvailable }, () => { await pool.end(); }); - it('createJob defaults: queued, provision 1, phase implement, api_key', async () => { + it('createJob defaults: queued, provision 1, phase analyze, api_key', async () => { const job = await newQueuedJob(repo.id); assert.equal(job.status, 'queued'); assert.equal(job.provision, 1); - assert.equal(job.phase, 'implement'); + // Every pipeline_mode starts at 'analyze' (transitions.ts's own test suite: + // even collapsed mode begins `analyze → implement`); an explicit `phase` + // override (e.g. the gated-webhook trigger parking at `await_human`) wins. + assert.equal(job.phase, 'analyze'); assert.equal(job.authMode, 'api_key'); // Only the sha256 hash is stored — never the plaintext token. assert.match(job.runnerTokenHash ?? '', /^[0-9a-f]{64}$/); @@ -230,6 +233,19 @@ describe('devplatform/DevJobStore (pg)', { skip: !pgAvailable }, () => { assert.equal(await store.verifyRunnerToken(randomUUID(), token), false, 'unknown job → false'); }); + it('reissueRunnerToken mints a fresh token and invalidates the one it replaces', async () => { + const job = await newQueuedJob(repo.id); + const original = mintRunnerToken().token; // the token createJob's hash matched, never a docker container's + // newQueuedJob mints its own; overwrite the row to a known original for this test. + await pool.query('UPDATE dev_jobs SET runner_token_hash = $2 WHERE id = $1', [job.id, hashRunnerToken(original)]); + assert.equal(await store.verifyRunnerToken(job.id, original), true, 'precondition: original verifies'); + + const reissued = await store.reissueRunnerToken(job.id); + assert.notEqual(reissued, original, 'a genuinely new token, not the input echoed back'); + assert.equal(await store.verifyRunnerToken(job.id, reissued), true, 'the reissued token verifies'); + assert.equal(await store.verifyRunnerToken(job.id, original), false, 'the replaced token no longer verifies'); + }); + it('resolveJobByToken verifies constant-time and excludes terminal jobs (no state oracle)', async () => { const { token } = mintRunnerToken(); const hash = hashRunnerToken(token); @@ -324,6 +340,27 @@ describe('devplatform/DevJobStore (pg)', { skip: !pgAvailable }, () => { assert.equal(again?.error, null, 'the no-op did not write the failure error'); }); + it('deleteJob refuses an active job, deletes a terminal one, and reports a missing id', async () => { + // Active (queued) — never deleted, it still has (or will have) a live backend + // handle; deleting the row out from under it would orphan a container/Machine. + const active = await newQueuedJob(repo.id); + assert.equal(await store.deleteJob(active.id), 'not_terminal'); + assert.ok(await store.getJob(active.id), 'the active job row is untouched'); + + // Terminal — deleted, and CASCADE (0022) takes its events with it. + const terminal = await newQueuedJob(repo.id); + await store.appendEvents(terminal.id, 1, [{ seq: 0, type: 'log', payload: { line: 'hi' } }]); + await store.finishTerminal(TERMINAL_FINISH_BRAND, terminal.id, 'failed', { error: 'x' }); + assert.equal(await store.deleteJob(terminal.id), 'deleted'); + assert.equal(await store.getJob(terminal.id), null, 'the row is gone'); + const events = await pool.query('SELECT 1 FROM dev_job_events WHERE job_id = $1', [terminal.id]); + assert.equal(events.rowCount, 0, 'its events cascaded away with it'); + + // Unknown id — distinct outcome from "exists but active", so the route can + // answer 404 instead of a misleading 409. + assert.equal(await store.deleteJob(randomUUID()), 'not_found'); + }); + it('findStalled surfaces active jobs past the heartbeat cutoff', async () => { const localRepo = await newRepo(); const job = await newQueuedJob(localRepo.id); @@ -421,7 +458,7 @@ describe('devplatform/DevJobStore (pg)', { skip: !pgAvailable }, () => { repoId: repo.id, kind: 'fix_issue', brief: 'fence', source: 'admin', backend: 'docker', createdBy: MARK, runnerTokenHash: hash, }); - // Job is at the default phase (implement/analyze), NOT await_human. + // Job is at its default starting phase ('analyze'), NOT await_human. assert.equal(await store.requeueAtPhase(job.id, 'implement'), false, 'a non-parked job is not re-queued'); const still = await store.getJob(job.id); assert.equal(still?.status, 'queued', 'and its status is untouched by the no-op'); @@ -430,7 +467,7 @@ describe('devplatform/DevJobStore (pg)', { skip: !pgAvailable }, () => { const lease = randomUUID(); let claimed = await store.claimNextQueued(lease); while (claimed && claimed.id !== job.id) claimed = await store.claimNextQueued(lease); - await store.advancePhase(job.id, 'implement', 'await_human'); + await store.advancePhase(job.id, job.phase, 'await_human'); await store.parkForGate(job.id); assert.equal(await store.requeueAtPhase(job.id, 'implement'), true, 'a parked job re-queues'); const requeued = await store.getJob(job.id); diff --git a/middleware/test/devplatform/devPlatform.e2e.test.ts b/middleware/test/devplatform/devPlatform.e2e.test.ts index a774ba43a..e801b1f62 100644 --- a/middleware/test/devplatform/devPlatform.e2e.test.ts +++ b/middleware/test/devplatform/devPlatform.e2e.test.ts @@ -523,6 +523,38 @@ describe('devplatform e2e (pg)', { skip: !pgAvailable }, () => { assert.ok(policy.egressAllowlist.includes('registry.npmjs.org'), 'operator base allowlist folded in'); }); + it('W1 job-policy endpoint reissues the runner token — DockerBackend.provision() never carries one', async () => { + // provisionedJob()'s token comes from prepareProvision(), the LocalProcess/Fly + // path's contract. DockerBackend.provision() posts only {protocol, jobId, + // leaseTtlSec} (spec S3) — that token is never handed to any container for + // this backend, so the job-policy fetch (the docker backend's actual + // provision moment) must mint its own and invalidate the unused original. + const { jobId, token: original } = await provisionedJob(); + + const res = await fetch(`${baseUrl}/api/v1/dev-runner/internal/job-policy/${jobId}`, { + headers: { Authorization: `Bearer ${E2E_DAEMON_TOKEN}` }, + }); + assert.equal(res.status, 200); + const policy = (await res.json()) as { env: Record }; + const reissued = policy.env['OMADIA_JOB_TOKEN']; + assert.ok(reissued, 'the policy env carries a fresh runner token'); + assert.notEqual(reissued, original, 'reissued, not the unused original'); + + // The reissued token authenticates the runner phone-home surface... + const withNew = await fetch(`${baseUrl}/api/v1/dev-runner/jobs/${jobId}/spec`, { + headers: { Authorization: `Bearer ${reissued}` }, + }); + assert.equal(withNew.status, 200, 'the reissued token is valid on the phone-home surface'); + + // ...and the original, never-used-for-docker token no longer is: reissuing + // replaced runner_token_hash, so a stale local/fly-shaped token cannot also + // authenticate a docker-backed job's runner. + const withOriginal = await fetch(`${baseUrl}/api/v1/dev-runner/jobs/${jobId}/spec`, { + headers: { Authorization: `Bearer ${original}` }, + }); + assert.equal(withOriginal.status, 401, 'the pre-reissue token is invalidated'); + }); + it('W1 job-policy endpoint accepts EVERY token in a rotation list, and nothing else', async () => { const { jobId } = await provisionedJob(); const ask = (token: string) => diff --git a/middleware/test/devplatform/devPlatformPipeline.wire.pg.test.ts b/middleware/test/devplatform/devPlatformPipeline.wire.pg.test.ts index 1d4a96cf2..715bd42e3 100644 --- a/middleware/test/devplatform/devPlatformPipeline.wire.pg.test.ts +++ b/middleware/test/devplatform/devPlatformPipeline.wire.pg.test.ts @@ -194,7 +194,7 @@ describe('dev-platform wiring — a real gated job, end to end through the assem source: 'admin', sourceRef: 'gh-issue:1', baseSha: BASE_SHA, - phase: 'analyze', // a gated pipeline starts at analyze (createJob defaults to implement) + phase: 'analyze', // explicit for clarity — matches createJob's own default now backend: 'local', createdBy: MARK, runnerTokenHash: hash, @@ -302,6 +302,40 @@ describe('dev-platform wiring — a real gated job, end to end through the assem assert.equal(res.status, 409, 'a stale phase result is rejected'); }); + it('a gated phase failure populates dev_jobs.error, not just the status event payload', async () => { + // Regression: found live when bootstrap correctly reported ok:false (no + // bootstrap_command configured for the repo) — dev_jobs.error stayed empty + // because the PhaseEngine→boundFinalize adapter (wireDevPlatform.ts) only + // ever passed the reason as FinalizeContext.reason (→ the status event + // payload), never as FinalizeContext.error (→ the dev_jobs.error column). + const BASE_SHA = 'basesha-failreason'; + const { hash } = mintRunnerToken(); + const job = await wired.jobStore.createJob({ + repoId, + kind: 'fix_issue', + brief: 'a job whose analyze phase fails', + source: 'admin', + sourceRef: null, + baseSha: BASE_SHA, + phase: 'analyze', + backend: 'local', + createdBy: MARK, + runnerTokenHash: hash, + }); + const token = await provision(job.id, BASE_SHA); + assert.equal(await getSpec(job.id, token), 200); + + const REASON = 'no bootstrap command provisioned for this repo'; + assert.deepEqual(await postPhase(job.id, token, { phase: 'analyze', ok: false, error: REASON }), { + directive: 'failed', + reason: REASON, + }); + + const failed = await wired.jobStore.getJob(job.id); + assert.equal(failed?.status, 'failed'); + assert.equal(failed?.error, REASON, 'the real failure reason lands on the job row, not just the event trail'); + }); + it('the gate-deadline worker expires an overdue gate and cancels the job (reason gate_expired), revoking its token', async () => { const BASE_SHA = 'basesha-expire'; const { hash } = mintRunnerToken(); diff --git a/middleware/test/devplatform/devPlatformRoutes.harness.ts b/middleware/test/devplatform/devPlatformRoutes.harness.ts index 26c699115..e91b98cd2 100644 --- a/middleware/test/devplatform/devPlatformRoutes.harness.ts +++ b/middleware/test/devplatform/devPlatformRoutes.harness.ts @@ -14,13 +14,14 @@ import { import { DevJobEventBus } from '../../src/devplatform/devJobEventBus.js'; import type { Ticket } from '../../src/devplatform/githubIssuesTracker.js'; import type { FinalizeContext } from '../../src/devplatform/finalizeDevJob.js'; -import type { - DevJob, - DevJobEvent, - DevJobStatus, - DevRepo, - NewDevJob, - NewDevRepo, +import { + TERMINAL_DEV_JOB_STATUSES, + type DevJob, + type DevJobEvent, + type DevJobStatus, + type DevRepo, + type NewDevJob, + type NewDevRepo, } from '../../src/devplatform/types.js'; /** @@ -116,6 +117,7 @@ export class FakeJobStore { id, repoId: input.repoId, kind: input.kind, brief: input.brief, source: input.source, sourceRef: input.sourceRef ?? null, backend: input.backend, authMode: input.authMode ?? 'api_key', createdBy: input.createdBy, runnerTokenHash: input.runnerTokenHash, status: 'queued', + phase: input.phase ?? 'analyze', }); this.jobs.set(id, job); return job; @@ -137,8 +139,17 @@ export class FakeJobStore { return [...this.artifacts.values()].filter((a) => a.jobId === jobId); } async getArtifact(id: string) { return this.artifacts.get(id) ?? null; } - addArtifact(a: { id: string; jobId: string; kind: string; content: string }): void { - this.artifacts.set(a.id, { ...a, meta: {}, createdAt: new Date().toISOString() }); + async addArtifact(jobId: string, kind: string, content: string, meta: Record = {}): Promise { + const id = `artifact-${String(++this.seq)}`; + this.artifacts.set(id, { id, jobId, kind, content, meta, createdAt: new Date().toISOString() }); + return id; + } + async deleteJob(id: string): Promise<'deleted' | 'not_terminal' | 'not_found'> { + const job = this.jobs.get(id); + if (!job) return 'not_found'; + if (!(TERMINAL_DEV_JOB_STATUSES as readonly DevJobStatus[]).includes(job.status)) return 'not_terminal'; + this.jobs.delete(id); + return 'deleted'; } } @@ -261,6 +272,10 @@ export async function postJson(url: string, headers: Record, bod return fetch(url, { method: 'POST', headers: { ...headers, 'content-type': 'application/json' }, body: JSON.stringify(body) }); } +export async function deleteReq(url: string, headers: Record) { + return fetch(url, { method: 'DELETE', headers }); +} + /** Assert that `fn` throws a DevPlatformError carrying the given code. */ export function throwsCode(fn: () => void, code: string): void { assert.throws(fn, (err: unknown) => (err as { code?: string }).code === code, `expected code ${code}`); diff --git a/middleware/test/devplatform/devPlatformRoutes.test.ts b/middleware/test/devplatform/devPlatformRoutes.test.ts index 5283f279d..ce87c27e9 100644 --- a/middleware/test/devplatform/devPlatformRoutes.test.ts +++ b/middleware/test/devplatform/devPlatformRoutes.test.ts @@ -18,6 +18,7 @@ import { Harness, PAT_TOKEN, authHeaders, + deleteReq, hasLeakedSecret, makeHarness, makeJob, @@ -241,6 +242,98 @@ describe('devPlatform — cancel routes through finalizeDevJob', () => { }); }); +describe('devPlatform — DELETE /jobs/:id', () => { + let h: Harness; + afterEach(async () => { if (h) await h.close(); }); + + it('204 and removes a terminal job', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice' })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'failed' })); + const res = await deleteReq(`${h.baseUrl}/jobs/job-1`, authHeaders()); + assert.equal(res.status, 204); + assert.equal(await h.jobStore.getJob('job-1'), null); + }); + + it('409 for an active job — never orphans a live backend handle', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice' })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'running' })); + const res = await deleteReq(`${h.baseUrl}/jobs/job-1`, authHeaders()); + assert.equal(res.status, 409); + assert.equal(((await res.json()) as { code: string }).code, 'devplatform.job_not_terminal'); + assert.ok(await h.jobStore.getJob('job-1'), 'the job survives the refused delete'); + }); + + it('404 for a job on a repo the caller may not launch (same as GET /jobs/:id)', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice', allowedLaunchers: [] })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'done' })); + const res = await deleteReq(`${h.baseUrl}/jobs/job-1`, authHeaders('bob', 'viewer')); + assert.equal(res.status, 404); + assert.ok(await h.jobStore.getJob('job-1'), 'unauthorized delete never touches the row'); + }); +}); + +describe('devPlatform — POST /jobs/:id/retry', () => { + let h: Harness; + afterEach(async () => { if (h) await h.close(); }); + + it('default: clones into a fresh job starting at analyze, no artifacts carried over', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice' })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'failed', phase: 'bootstrap' })); + await h.jobStore.addArtifact('job-1', 'analysis', '{"kind":"analysis"}'); + const res = await postJson(`${h.baseUrl}/jobs/job-1/retry`, authHeaders(), {}); + assert.equal(res.status, 202); + const body = (await res.json()) as { ok: boolean; jobId: string; resumedAtPhase?: string }; + assert.equal(body.ok, true); + assert.ok(!('resumedAtPhase' in body), 'default retry reports no resumedAtPhase'); + const next = await h.jobStore.getJob(body.jobId); + assert.equal(next?.phase, 'analyze', 'default retry always restarts at analyze'); + assert.deepEqual(await h.jobStore.listArtifacts(body.jobId), [], 'no artifacts copied without resumeFromPhase'); + }); + + it('resumeFromPhase=true: starts the clone at the failed job\'s own phase and copies its artifacts forward', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice' })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'failed', phase: 'bootstrap' })); + await h.jobStore.addArtifact('job-1', 'analysis', '{"kind":"analysis"}', { note: 'from job-1' }); + const res = await postJson(`${h.baseUrl}/jobs/job-1/retry?resumeFromPhase=true`, authHeaders(), {}); + assert.equal(res.status, 202); + const body = (await res.json()) as { ok: boolean; jobId: string; resumedAtPhase?: string }; + assert.equal(body.ok, true); + assert.equal(body.resumedAtPhase, 'bootstrap'); + const next = await h.jobStore.getJob(body.jobId); + assert.equal(next?.phase, 'bootstrap', 'the clone starts where the source job failed, not at analyze'); + const copied = await h.jobStore.listArtifacts(body.jobId); + assert.equal(copied.length, 1); + assert.equal(copied[0]?.kind, 'analysis'); + assert.equal(copied[0]?.content, '{"kind":"analysis"}'); + assert.deepEqual(copied[0]?.meta, { note: 'from job-1' }); + // The copy lands under the NEW job's own id, not the source job's. + assert.equal(copied[0]?.jobId, body.jobId); + }); + + it('resumeFromPhase=true on a job that failed during analyze itself behaves like a normal retry', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice' })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'failed', phase: 'analyze' })); + const res = await postJson(`${h.baseUrl}/jobs/job-1/retry?resumeFromPhase=true`, authHeaders(), {}); + assert.equal(res.status, 202); + const next = await h.jobStore.getJob(((await res.json()) as { jobId: string }).jobId); + assert.equal(next?.phase, 'analyze'); + }); + + it('409 for a non-terminal job, same as a normal retry, regardless of resumeFromPhase', async () => { + h = await makeHarness(); + h.repoStore.add(makeRepo({ id: 'repo-1', createdBy: 'alice' })); + h.jobStore.add(makeJob({ id: 'job-1', repoId: 'repo-1', status: 'running', phase: 'bootstrap' })); + const res = await postJson(`${h.baseUrl}/jobs/job-1/retry?resumeFromPhase=true`, authHeaders(), {}); + assert.equal(res.status, 409); + }); +}); + // --------------------------------------------------------------------------- // SSE — the single job-event tail. // --------------------------------------------------------------------------- diff --git a/middleware/test/devplatform/devRunnerApi.harness.ts b/middleware/test/devplatform/devRunnerApi.harness.ts index aa679fffd..302ddd9e4 100644 --- a/middleware/test/devplatform/devRunnerApi.harness.ts +++ b/middleware/test/devplatform/devRunnerApi.harness.ts @@ -71,6 +71,13 @@ export class FakeStore implements DevRunnerJobStore { async getJob(jobId: string): Promise { return this.jobs.get(jobId) ?? null; } + reissueCalls: string[] = []; + async reissueRunnerToken(jobId: string): Promise { + this.reissueCalls.push(jobId); + const fresh = `djr_reissued-${String(this.reissueCalls.length)}`; + this.tokens.set(jobId, fresh); + return fresh; + } readonly touchCalls: string[] = []; readonly artifactOwner = new Map(); diff --git a/middleware/test/devplatform/llmProxy.test.ts b/middleware/test/devplatform/llmProxy.test.ts index ef6e90eea..5e166edf5 100644 --- a/middleware/test/devplatform/llmProxy.test.ts +++ b/middleware/test/devplatform/llmProxy.test.ts @@ -618,3 +618,116 @@ describe('llmProxy — accounting failure is surfaced, not swallowed (review S-f assert.equal(fx.usageRows.length, 0, 'ledger is not written when the authoritative store failed'); }); }); + +describe('llmProxy — survives a global express.json() ahead of the router (index.ts mount order)', () => { + // `makeFixture` above mounts the runner router directly on a bare `express()` + // app, which never reproduces the real bug: in `index.ts`, a global + // `app.use(express.json())` runs for EVERY path before `mountDevPlatform` + // attaches the runner router further down in the boot sequence. body-parser's + // `read()` bails out on an already-consumed request stream + // (`onFinished.isFinished(req)`) without touching `req.body` again, so the + // route's own `express.raw()` would silently no-op and leave `req.body` as + // whatever `express.json()` parsed — never a Buffer. `llmProxy.ts`'s + // canonicalisation step only trusts a Buffer, so every real request 400'd + // with "must name a model". Reproduce that exact mount order here, gated by + // the same path exclusion index.ts uses, and prove the request still lands. + let server: Server; + let base: string; + afterEach(async () => { + if (server) await new Promise((r) => server.close(() => r())); + }); + + it('parses the body correctly when a global express.json() precedes the router', async () => { + const calls: FetchCall[] = []; + const fetchImpl = (async (url: string | URL | Request, init?: RequestInit) => { + const body = init?.body instanceof Buffer ? init.body.toString('utf8') : String(init?.body ?? ''); + calls.push({ url: String(url), headers: { ...(init?.headers as Record) }, body }); + return sseResponse(FULL_SSE); + }) as unknown as typeof fetch; + + const proxyDeps: LlmProxyDeps = { + resolveJobByToken: async (t) => (t === VALID ? { id: 'job-1', status: 'running', agentKind: 'claude-cli' } : null), + resolvePolicy: async () => ({ + provider: 'anthropic', + upstreamBaseUrl: 'https://upstream.test', + allowedModels: ['claude-opus-4-8'], + }), + resolveProviderKey: async () => REAL_KEY, + addJobUsage: async () => {}, + recordUsage: () => {}, + fetchImpl, + }; + const runnerDeps: DevRunnerRouterDeps = { + store: { + verifyRunnerToken: async () => false, + getJob: async () => null, + markRunning: async () => false, + touchHeartbeat: async () => false, + appendEvents: async () => 0, + addArtifact: async () => 'x', + artifactBelongsToJob: async () => false, + recordResult: async () => {}, + }, + repos: { getRepo: async () => null }, + scmTokens: { resolve: async () => undefined }, + finalizeDevJob: async () => null, + llmProxyRouter: createLlmProxyRouter(proxyDeps), + }; + + const app: Express = express(); + // Mirrors index.ts's path-exclusion gate ahead of the global JSON parser. + app.use((req, res, next) => { + if (req.path.startsWith('/api/v1/dev-runner/llm/')) { + next(); + return; + } + express.json({ limit: '10mb' })(req, res, next); + }); + app.use('/api/v1/dev-runner', createDevRunnerRouter(runnerDeps)); + + await new Promise((resolve) => { + server = app.listen(0, () => { + const port = (server.address() as AddressInfo).port; + base = `http://127.0.0.1:${String(port)}/api/v1/dev-runner`; + resolve(); + }); + }); + + const res = await post(base, OK_BODY, authed); + assert.equal(res.status, 200); + await res.text(); + assert.equal(calls.length, 1); + assert.equal(JSON.parse(calls[0]?.body ?? '{}').model, 'claude-opus-4-8'); + }); + + it('still parses unrelated JSON routes normally through the global express.json()', async () => { + const app: Express = express(); + app.use((req, res, next) => { + if (req.path.startsWith('/api/v1/dev-runner/llm/')) { + next(); + return; + } + express.json({ limit: '10mb' })(req, res, next); + }); + app.post('/other', (req, res) => { + res.json({ echoed: req.body as unknown }); + }); + + await new Promise((resolve) => { + server = app.listen(0, () => { + const port = (server.address() as AddressInfo).port; + base = `http://127.0.0.1:${String(port)}`; + resolve(); + }); + }); + + const res = await fetch(`${base}/other`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ hello: 'world' }), + }); + assert.equal(res.status, 200); + const json = (await res.json()) as { echoed: { hello: string } }; + assert.equal(json.echoed.hello, 'world'); + }); +}); diff --git a/web-ui/app/admin/dev-platform/_components/GateInbox.tsx b/web-ui/app/admin/dev-platform/_components/GateInbox.tsx index 4a77a98ad..ff6a1ae90 100644 --- a/web-ui/app/admin/dev-platform/_components/GateInbox.tsx +++ b/web-ui/app/admin/dev-platform/_components/GateInbox.tsx @@ -8,11 +8,13 @@ import { Button } from '@/app/_components/ui/Button'; import { ApiError } from '@/app/_lib/api'; import { DEV_ARTIFACT_PATH, + getArtifactText, listWaitingGates, resolveGate, type DevGateAnswer, type DevGateView, } from '../_lib/api'; +import { PrettyArtifact } from './PrettyArtifact'; /** * Epic #470 W2 — the operator gate inbox (UI spec §5). Lists every job parked at @@ -92,12 +94,46 @@ type ResolveState = | { kind: 'conflict' } | { kind: 'error' }; -function GateCard({ gate, onResolved }: { gate: DevGateView; onResolved: () => void }): React.ReactElement { +type PlanTextState = { kind: 'loading' } | { kind: 'ready'; text: string } | { kind: 'error' } | { kind: 'none' }; + +/** `compact`: drop the deadline/job-id header (the job-detail page already + * shows both) and the outer bordered card — used to embed the gate inline in + * the job's own phase flow instead of only in the standalone gate inbox. */ +export function GateCard({ + gate, + onResolved, + compact = false, +}: { + gate: DevGateView; + onResolved: () => void; + compact?: boolean; +}): React.ReactElement { const t = useTranslations('adminDevPlatform.gates'); const [answers, setAnswers] = useState>({}); const [note, setNote] = useState(''); const [busy, setBusy] = useState<'approve' | 'reject' | null>(null); const [resolveState, setResolveState] = useState({ kind: 'idle' }); + const [fetchedPlanText, setFetchedPlanText] = useState({ kind: 'loading' }); + // No artifact ⇒ no fetch ever happens — derive 'none' rather than storing it, + // so the effect below never needs a synchronous setState in its early return. + const planText: PlanTextState = gate.planArtifactId ? fetchedPlanText : { kind: 'none' }; + + useEffect(() => { + if (!gate.planArtifactId) return; + let cancelled = false; + setFetchedPlanText({ kind: 'loading' }); + void getArtifactText(gate.planArtifactId).then( + (text) => { + if (!cancelled) setFetchedPlanText({ kind: 'ready', text }); + }, + () => { + if (!cancelled) setFetchedPlanText({ kind: 'error' }); + }, + ); + return () => { + cancelled = true; + }; + }, [gate.planArtifactId]); const resolve = useCallback( (approved: boolean) => { @@ -134,43 +170,65 @@ function GateCard({ gate, onResolved }: { gate: DevGateView; onResolved: () => v ); return ( -
+
-
- {t('job')} {gate.jobId} -
+ {compact ? null : ( +
+ {t('job')} {gate.jobId} +
+ )}
{gate.deadlineAt ? t('deadline', { at: formatTs(gate.deadlineAt) }) : t('noDeadline')}
-
{t('plan')}
-
- {gate.planArtifactId ? ( - - {t('viewPlan')} - - ) : ( - {t('noPlan')} - )} - {gate.planSha256 ? ( - - {gate.planSha256.slice(0, 12)} - - ) : null} -
{t('holders')}
{gate.resolvedHolders.length > 0 ? gate.resolvedHolders.join(', ') : t('noHolders')}
+
+
+

+ {t('plan')} +

+
+ {gate.planSha256 ? ( + + {gate.planSha256.slice(0, 12)} + + ) : null} + {gate.planArtifactId ? ( + + {t('viewPlan')} + + ) : null} +
+
+ {planText.kind === 'none' ? ( +

{t('noPlan')}

+ ) : planText.kind === 'loading' ? ( +

{t('planLoading')}

+ ) : planText.kind === 'error' ? ( +

{t('planLoadError')}

+ ) : ( +
+ +
+ )} +
+ {gate.questions.length > 0 ? (

diff --git a/web-ui/app/admin/dev-platform/_components/JobLogPane.tsx b/web-ui/app/admin/dev-platform/_components/JobLogPane.tsx index a81c7f54a..3e05646e2 100644 --- a/web-ui/app/admin/dev-platform/_components/JobLogPane.tsx +++ b/web-ui/app/admin/dev-platform/_components/JobLogPane.tsx @@ -7,6 +7,9 @@ import { useTranslations } from 'next-intl'; import { ScrollToBottomButton } from '@/app/_components/ScrollToBottomButton'; import { useStickToBottom } from '@/app/_lib/useStickToBottom'; +import type { LogItem } from '../_lib/toolCallLog'; +import { ToolCallCard } from './ToolCallCard'; + /** * Epic #470 W0 — the live log pane (UI spec §5). Monospace, sunken surface, * stick-to-bottom via `useStickToBottom` (issue #404): follows while at the @@ -15,40 +18,34 @@ import { useStickToBottom } from '@/app/_lib/useStickToBottom'; * a token stream announced line-by-line is noise (§13); a separate polite * region carries the connection state instead. * - * Tool-invocation lines are `$`-prefixed in `--fg-strong`, stdout in - * `--fg-muted`, stderr in `--danger` — text color only, no filled gutters. - * The pane scrolls inside its own `overflow` box; the page never scrolls - * sideways. No toast on disconnect. + * Items come pre-folded from `toolCallLog.ts`: agent narration renders as + * plain text (stdout in `--fg-muted`, stderr in `--danger`), tool calls + * render as a collapsible `ToolCallCard` instead of a raw `$ Name {...json}` + * dump. The pane scrolls inside its own `overflow` box; the page never + * scrolls sideways. No toast on disconnect. */ -export type LogStream = 'tool' | 'agent' | 'stderr'; - -export interface LogLine { - id: string; - stream: LogStream; - text: string; -} +export type LogTextStream = 'agent' | 'stderr'; export type LogConnection = 'live' | 'reconnecting' | 'closed'; -const STREAM_CLASS: Record = { - tool: 'text-[color:var(--fg-strong)]', +const STREAM_CLASS: Record = { agent: 'text-[color:var(--fg-muted)]', stderr: 'text-[color:var(--danger)]', }; export function JobLogPane({ - lines, + items, connection, lastEventAgoSec, }: { - lines: LogLine[]; + items: LogItem[]; connection: LogConnection; lastEventAgoSec: number | null; }): React.ReactElement { const t = useTranslations('adminDevPlatform.detail'); const scrollRef = useRef(null); - const { isAtBottom, scrollToBottom } = useStickToBottom(scrollRef, [lines.length]); + const { isAtBottom, scrollToBottom } = useStickToBottom(scrollRef, [items.length]); const connectionText = connection === 'live' @@ -68,15 +65,18 @@ export function JobLogPane({ aria-live="off" className="max-h-[60vh] overflow-x-auto overflow-y-auto rounded-lg border border-[color:var(--border)] lume-surface-sunken p-4 font-mono text-xs leading-[1.6]" > - {lines.length === 0 ? ( + {items.length === 0 ? (
{t('logEmpty')}
) : ( - lines.map((line) => ( -
- {line.stream === 'tool' ? '$ ' : ''} - {line.text} -
- )) + items.map((item) => + item.kind === 'tool' ? ( + + ) : ( +
+ {item.text} +
+ ), + ) )}

void; + onDelete: (job: DevJobView) => void; }): React.ReactElement { const t = useTranslations('adminDevPlatform.jobs'); const tKind = useTranslations('adminDevPlatform.jobs.kinds'); const format = useFormatter(); const [pendingCancel, setPendingCancel] = useState(null); + const [pendingDelete, setPendingDelete] = useState(null); const repoName = (repoId: string): string => { const r = repos.find((x) => x.id === repoId); @@ -96,7 +99,11 @@ export function JobTable({ {t('view')} - {isTerminalStatus(job.status) ? null : ( + {isTerminalStatus(job.status) ? ( + + ) : ( @@ -123,6 +130,20 @@ export function JobTable({ setPendingCancel(null); }} /> + + setPendingDelete(null)} + onConfirm={() => { + if (pendingDelete) onDelete(pendingDelete); + setPendingDelete(null); + }} + /> ); } diff --git a/web-ui/app/admin/dev-platform/_components/PhaseArtifactPanel.tsx b/web-ui/app/admin/dev-platform/_components/PhaseArtifactPanel.tsx new file mode 100644 index 000000000..b11e399ba --- /dev/null +++ b/web-ui/app/admin/dev-platform/_components/PhaseArtifactPanel.tsx @@ -0,0 +1,101 @@ +'use client'; + +import { useEffect, useState } from 'react'; + +import { useTranslations } from 'next-intl'; + +import type { DevJobUiPhase } from '@/app/_components/devjobs/DevJobPhaseRail'; + +import { getArtifactText, listJobArtifacts, type DevJobArtifactKind } from '../_lib/api'; +import { PrettyArtifact } from './PrettyArtifact'; + +/** + * Epic #470 — a completed phase's own recorded output (plan/questions/ + * bootstrap_report/review_verdict), shown once the live SSE log has nothing + * left for it. The job detail page (`jobs/[id]/page.tsx`) fell back to a + * permanently-empty `JobLogPane` for any phase the operator navigated back + * to after it finished, because that pane is filtered from live-only state — + * there was never a second source once the phase's own log scrolled away or + * the operator reloaded the page. `GET /jobs/:id/artifacts` + + * `GET /artifacts/:id` already existed and already served the gate's own + * plan text (`GateInbox.tsx`'s `getArtifactText`); this reuses both for every + * other phase instead of just the currently-open gate. + */ + +/** Not every phase has a matching artifact kind (`implement`'s output is the + * diff, already linked from the PR stop; `gate`/`pr` render their own body). */ +const PHASE_ARTIFACT_KIND: Partial> = { + analyze: 'analysis', + bootstrap: 'bootstrap_report', + plan: 'plan', + clarify: 'questions', + review: 'review_verdict', +}; + +type FetchState = { kind: 'loading' } | { kind: 'empty' } | { kind: 'error' } | { kind: 'ready'; text: string }; +type PanelState = { kind: 'no-artifact-kind' } | FetchState; + +/** Renders the given phase's own artifact when one exists; `null` when the + * phase has no artifact kind at all (caller falls back to the log pane) or + * none has been recorded yet (also a `JobLogPane` fallback — the phase may + * still be running). */ +export function PhaseArtifactPanel({ + jobId, + phase, +}: { + jobId: string; + phase: DevJobUiPhase; +}): React.ReactElement | null { + const t = useTranslations('adminDevPlatform.detail'); + const kind = PHASE_ARTIFACT_KIND[phase]; + const [fetched, setFetched] = useState({ kind: 'loading' }); + // No artifact kind for this phase ⇒ no fetch ever happens — derive + // 'no-artifact-kind' here rather than storing it, so the effect below never + // needs a synchronous setState for that case (same idiom as GateInbox.tsx's + // `planText`). + const state: PanelState = kind ? fetched : { kind: 'no-artifact-kind' }; + + useEffect(() => { + if (!kind) return; + let cancelled = false; + setFetched({ kind: 'loading' }); + void listJobArtifacts(jobId).then( + (res) => { + if (cancelled) return; + // Multiple retries/resumes can leave several artifacts of the same + // kind (e.g. a retried bootstrap) — the most recent one is the phase's + // actual last outcome. + const latest = res.artifacts + .filter((a) => a.kind === kind) + .sort((a, b) => b.createdAt.localeCompare(a.createdAt))[0]; + if (!latest) { + setFetched({ kind: 'empty' }); + return; + } + void getArtifactText(latest.id).then( + (text) => { + if (!cancelled) setFetched({ kind: 'ready', text }); + }, + () => { + if (!cancelled) setFetched({ kind: 'error' }); + }, + ); + }, + () => { + if (!cancelled) setFetched({ kind: 'error' }); + }, + ); + return () => { + cancelled = true; + }; + }, [jobId, kind]); + + if (state.kind === 'no-artifact-kind' || state.kind === 'empty') return null; + if (state.kind === 'loading') { + return

{t('loading')}

; + } + if (state.kind === 'error') { + return

{t('artifactError')}

; + } + return ; +} diff --git a/web-ui/app/admin/dev-platform/_components/PrettyArtifact.tsx b/web-ui/app/admin/dev-platform/_components/PrettyArtifact.tsx new file mode 100644 index 000000000..645811b85 --- /dev/null +++ b/web-ui/app/admin/dev-platform/_components/PrettyArtifact.tsx @@ -0,0 +1,63 @@ +'use client'; + +import { parseArtifactRecord } from '../_lib/prettyArtifact'; + +/** + * Epic #470 — a readable render of a JSON artifact (plan, analysis, + * bootstrap_report, ...) instead of its raw text. Fully generic per field: + * a string renders as a wrapped paragraph (real line breaks now that JSON + * parsing has turned `\n` escapes into actual newline characters — the raw + * text view showed them as literal backslash-n), a string array as a bullet + * list, anything else as a small indented JSON block. Falls back to the raw + * text verbatim when it isn't parseable JSON or isn't a plain object at the + * top level — never worse than the previous behavior. + */ +export function PrettyArtifact({ text }: { text: string }): React.ReactElement { + const record = parseArtifactRecord(text); + if (!record) { + return ( + 
+        {text}
+
+ ); + } + + return ( +
+ {Object.entries(record).map(([key, value]) => ( +
+
+ {key} +
+
{renderValue(value)}
+
+ ))} +
+ ); +} + +function renderValue(value: unknown): React.ReactElement { + if (typeof value === 'string') { + return

{value}

; + } + if (Array.isArray(value) && value.every((v) => typeof v === 'string')) { + return ( +
    + {value.map((v: string, i) => ( +
  • {v}
    • + ))} +
+ ); + } + if (typeof value === 'boolean' || typeof value === 'number') { + return {String(value)}; + } + if (value === null || value === undefined) { + return —; + } + return ( + 
+      {JSON.stringify(value, null, 2)}
+
+ ); +} diff --git a/web-ui/app/admin/dev-platform/_components/ToolCallCard.tsx b/web-ui/app/admin/dev-platform/_components/ToolCallCard.tsx new file mode 100644 index 000000000..4a6e317f6 --- /dev/null +++ b/web-ui/app/admin/dev-platform/_components/ToolCallCard.tsx @@ -0,0 +1,168 @@ +'use client'; + +import { useState } from 'react'; + +import { useTranslations } from 'next-intl'; + +import type { DiffLine } from '../_lib/lineDiff'; +import { summarizeToolCall, type ToolCallDetail, type ToolCallEntry } from '../_lib/toolCallLog'; + +/** + * Epic #470 — one structured entry in the implement-phase log pane (see + * `JobLogPane.tsx`), replacing the previous flat `$ Name {...raw JSON...}` + * text line. Collapsed by default: an icon-free status glyph, the tool + * name, and `summarizeToolCall`'s one-line headline; expanding reveals the + * tool-shaped detail (a diff for `Edit`, a command + output for `Bash`, + * etc.). Text/edge-only state coloring, no spinners — this project's Lume + * design rules (see `lume-design-system-web-ui`). + */ + +const STATUS_GLYPH: Record = { + pending: '…', + ok: '✓', + error: '✕', +}; + +const STATUS_CLASS: Record = { + pending: 'text-[color:var(--fg-subtle)]', + ok: 'text-[color:var(--fg-strong)]', + error: 'text-[color:var(--danger)]', +}; + +const DIFF_LINE_LIMIT = 400; + +export function ToolCallCard({ entry }: { entry: ToolCallEntry }): React.ReactElement { + const t = useTranslations('adminDevPlatform.detail.toolCall'); + const [expanded, setExpanded] = useState(false); + const summary = summarizeToolCall(entry); + const statusLabel = entry.status === 'pending' ? t('pending') : entry.status === 'error' ? t('failed') : ''; + + return ( +
+ + {expanded ?
{renderDetail(summary.detail, t)}
: null} +
+ ); +} + +function renderDetail( + detail: ToolCallDetail, + t: ReturnType>, +): React.ReactElement { + switch (detail.kind) { + case 'diff': + return ; + case 'command': + return ( + <> + 
+          
+        
+      );
+    case 'file':
+      return ;
+    case 'agent':
+      return (
+        <>
+          {detail.prompt ? 
 : null}
+          
+        
+      );
+    case 'search':
+      return ;
+    case 'raw':
+      return (
+        <>
+          {detail.input ? 
 : null}
+          
+        
+      );
+  }
+}
+
+function OutputBlock({
+  text,
+  label,
+  t,
+}: {
+  text: string | undefined;
+  label?: string;
+  t: ReturnType>;
+}): React.ReactElement {
+  if (!text) return 
{t('noOutput')}
;
+  return 
;
+}
+
+function Pre({ text, label, tone = 'muted' }: { text: string; label?: string; tone?: 'muted' | 'strong' }): React.ReactElement {
+  return (
+    

+      {label ? 
{label}
 : null}
+      
+        {text}
+      

+    

+  );
+}
+
+function DiffView({
+  diff,
+  t,
+}: {
+  diff: DiffLine[];
+  t: ReturnType>;
+}): React.ReactElement {
+  const shown = diff.slice(0, DIFF_LINE_LIMIT);
+  const hidden = diff.length - shown.length;
+  return (
+    

+      
+        {shown.map((line, i) => (
+          

+            {line.type === 'add' ? '+' : line.type === 'remove' ? '-' : ' '} {line.text}
+          

+        ))}
+      

+      {hidden > 0 ? (
+        

+          {t('moreDiffLines', { count: hidden })}
+        

+      ) : null}
+    

+  );
+}
diff --git a/web-ui/app/admin/dev-platform/_lib/__tests__/lineDiff.test.ts b/web-ui/app/admin/dev-platform/_lib/__tests__/lineDiff.test.ts
new file mode 100644
index 000000000..6f5179202
--- /dev/null
+++ b/web-ui/app/admin/dev-platform/_lib/__tests__/lineDiff.test.ts
@@ -0,0 +1,70 @@
+import { describe, expect, it } from 'vitest';
+
+import { computeLineDiff } from '../lineDiff';
+
+describe('computeLineDiff', () => {
+  it('returns all-context for identical text', () => {
+    const diff = computeLineDiff('a\nb\nc', 'a\nb\nc');
+    expect(diff).toEqual([
+      { type: 'context', text: 'a' },
+      { type: 'context', text: 'b' },
+      { type: 'context', text: 'c' },
+    ]);
+  });
+
+  it('marks a single changed line as remove+add, keeping context around it', () => {
+    const diff = computeLineDiff('a\nb\nc', 'a\nx\nc');
+    expect(diff).toEqual([
+      { type: 'context', text: 'a' },
+      { type: 'remove', text: 'b' },
+      { type: 'add', text: 'x' },
+      { type: 'context', text: 'c' },
+    ]);
+  });
+
+  it('handles a pure insertion', () => {
+    const diff = computeLineDiff('a\nc', 'a\nb\nc');
+    expect(diff).toEqual([
+      { type: 'context', text: 'a' },
+      { type: 'add', text: 'b' },
+      { type: 'context', text: 'c' },
+    ]);
+  });
+
+  it('handles a pure deletion', () => {
+    const diff = computeLineDiff('a\nb\nc', 'a\nc');
+    expect(diff).toEqual([
+      { type: 'context', text: 'a' },
+      { type: 'remove', text: 'b' },
+      { type: 'context', text: 'c' },
+    ]);
+  });
+
+  it('handles an empty old string (pure addition)', () => {
+    const diff = computeLineDiff('', 'a\nb');
+    expect(diff).toEqual([
+      { type: 'add', text: 'a' },
+      { type: 'add', text: 'b' },
+    ]);
+  });
+
+  it('handles an empty new string (pure removal)', () => {
+    const diff = computeLineDiff('a\nb', '');
+    expect(diff).toEqual([
+      { type: 'remove', text: 'a' },
+      { type: 'remove', text: 'b' },
+    ]);
+  });
+
+  it('handles two empty strings', () => {
+    expect(computeLineDiff('', '')).toEqual([]);
+  });
+
+  it('falls back to a remove-all/add-all block for pathologically large inputs', () => {
+    const big = Array.from({ length: 3000 }, (_, i) => `line-${i}`).join('\n');
+    const bigger = Array.from({ length: 3000 }, (_, i) => `other-${i}`).join('\n');
+    const diff = computeLineDiff(big, bigger);
+    expect(diff.every((l, idx) => (idx < 3000 ? l.type === 'remove' : l.type === 'add'))).toBe(true);
+    expect(diff).toHaveLength(6000);
+  });
+});
diff --git a/web-ui/app/admin/dev-platform/_lib/__tests__/prettyArtifact.test.ts b/web-ui/app/admin/dev-platform/_lib/__tests__/prettyArtifact.test.ts
new file mode 100644
index 000000000..b6da9ac83
--- /dev/null
+++ b/web-ui/app/admin/dev-platform/_lib/__tests__/prettyArtifact.test.ts
@@ -0,0 +1,40 @@
+import { describe, expect, it } from 'vitest';
+
+import { parseArtifactRecord } from '../prettyArtifact';
+
+describe('parseArtifactRecord', () => {
+  it('parses a plain JSON object', () => {
+    expect(parseArtifactRecord('{"kind":"plan","approach":"do it"}')).toEqual({
+      kind: 'plan',
+      approach: 'do it',
+    });
+  });
+
+  it('turns escaped \\n sequences into real newline characters', () => {
+    const record = parseArtifactRecord('{"approach":"line one\\n\\nline two"}');
+    expect(record?.['approach']).toBe('line one\n\nline two');
+  });
+
+  it('returns null for malformed JSON', () => {
+    expect(parseArtifactRecord('not json')).toBeNull();
+  });
+
+  it('returns null for a top-level array', () => {
+    expect(parseArtifactRecord('[1,2,3]')).toBeNull();
+  });
+
+  it('returns null for a top-level primitive', () => {
+    expect(parseArtifactRecord('"just a string"')).toBeNull();
+    expect(parseArtifactRecord('42')).toBeNull();
+  });
+
+  it('returns null for JSON null', () => {
+    expect(parseArtifactRecord('null')).toBeNull();
+  });
+
+  it('preserves nested arrays and objects as-is for the caller to handle', () => {
+    const record = parseArtifactRecord('{"filesToTouch":["a.ts","b.ts"],"nested":{"x":1}}');
+    expect(record?.['filesToTouch']).toEqual(['a.ts', 'b.ts']);
+    expect(record?.['nested']).toEqual({ x: 1 });
+  });
+});
diff --git a/web-ui/app/admin/dev-platform/_lib/__tests__/toolCallLog.test.ts b/web-ui/app/admin/dev-platform/_lib/__tests__/toolCallLog.test.ts
new file mode 100644
index 000000000..e4fb36a0f
--- /dev/null
+++ b/web-ui/app/admin/dev-platform/_lib/__tests__/toolCallLog.test.ts
@@ -0,0 +1,253 @@
+import { describe, expect, it } from 'vitest';
+
+import type { DevJobEventMessage } from '@/app/_lib/useDevJobEvents';
+
+import {
+  INITIAL_LOG_STATE,
+  foldDevJobEvent,
+  summarizeToolCall,
+  type LogItem,
+  type LogState,
+  type ToolCallEntry,
+} from '../toolCallLog';
+
+function ev(
+  id: number,
+  type: DevJobEventMessage['type'],
+  payload: Record,
+): DevJobEventMessage {
+  return { id, jobId: 'job-1', provision: 1, seq: id, type, ts: '2026-01-01T00:00:00Z', payload };
+}
+
+/** Fold a sequence of events onto INITIAL_LOG_STATE and return just the items. */
+function foldItems(...evs: DevJobEventMessage[]): LogItem[] {
+  return evs.reduce((s: LogState, e) => foldDevJobEvent(s, e), INITIAL_LOG_STATE).items;
+}
+
+describe('foldDevJobEvent — tool pairing', () => {
+  it('appends a pending entry on the start event, stamped with the current phase', () => {
+    const items = foldItems(ev(1, 'tool', { name: 'Read', inputPreview: '{"file_path":"a.ts"}' }));
+    expect(items).toEqual([
+      {
+        kind: 'tool',
+        phase: 'analyze',
+        entry: { id: '1', name: 'Read', status: 'pending', inputPreview: '{"file_path":"a.ts"}' },
+      },
+    ]);
+  });
+
+  it('pairs the result event into the matching pending entry', () => {
+    const items = foldItems(
+      ev(1, 'tool', { name: 'Read', inputPreview: '{"file_path":"a.ts"}' }),
+      ev(2, 'tool', { name: 'Read', ok: true, outputPreview: 'file contents' }),
+    );
+    expect(items).toEqual([
+      {
+        kind: 'tool',
+        phase: 'analyze',
+        entry: {
+          id: '1',
+          name: 'Read',
+          status: 'ok',
+          inputPreview: '{"file_path":"a.ts"}',
+          outputPreview: 'file contents',
+        },
+      },
+    ]);
+  });
+
+  it('marks status error when ok is false', () => {
+    const items = foldItems(
+      ev(1, 'tool', { name: 'Bash', inputPreview: '{"command":"false"}' }),
+      ev(2, 'tool', { name: 'Bash', ok: false, outputPreview: 'exit 1' }),
+    );
+    expect((items[0] as { kind: 'tool'; entry: ToolCallEntry }).entry.status).toBe('error');
+  });
+
+  it('pairs same-name calls in order (FIFO) rather than the first pending one incorrectly', () => {
+    const items = foldItems(
+      ev(1, 'tool', { name: 'Read', inputPreview: '{"file_path":"a.ts"}' }),
+      ev(2, 'tool', { name: 'Read', ok: true, outputPreview: 'A' }),
+      ev(3, 'tool', { name: 'Read', inputPreview: '{"file_path":"b.ts"}' }),
+      ev(4, 'tool', { name: 'Read', ok: true, outputPreview: 'B' }),
+    );
+    const entries = items.map((i) => (i as { kind: 'tool'; entry: ToolCallEntry }).entry);
+    expect(entries).toEqual([
+      { id: '1', name: 'Read', status: 'ok', inputPreview: '{"file_path":"a.ts"}', outputPreview: 'A' },
+      { id: '3', name: 'Read', status: 'ok', inputPreview: '{"file_path":"b.ts"}', outputPreview: 'B' },
+    ]);
+  });
+
+  it('renders a standalone result when no matching start exists', () => {
+    const items = foldItems(ev(1, 'tool', { name: 'Read', ok: true, outputPreview: 'orphan' }));
+    expect(items).toEqual([
+      { kind: 'tool', phase: 'analyze', entry: { id: '1', name: 'Read', status: 'ok', outputPreview: 'orphan' } },
+    ]);
+  });
+});
+
+describe('foldDevJobEvent — log/other events', () => {
+  it('appends agent-stream text', () => {
+    const items = foldItems(ev(1, 'log', { text: 'thinking…', stream: 'agent' }));
+    expect(items).toEqual([{ kind: 'text', id: '1', phase: 'analyze', stream: 'agent', text: 'thinking…' }]);
+  });
+
+  it('routes stderr stream text', () => {
+    const items = foldItems(ev(1, 'log', { text: 'boom', stream: 'stderr' }));
+    expect(items).toEqual([{ kind: 'text', id: '1', phase: 'analyze', stream: 'stderr', text: 'boom' }]);
+  });
+
+  it('drops empty-text log events', () => {
+    expect(foldItems(ev(1, 'log', { text: '' }))).toEqual([]);
+  });
+
+  it('ignores status/heartbeat events entirely', () => {
+    expect(foldItems(ev(1, 'status', { state: 'agent_started' }))).toEqual([]);
+  });
+});
+
+describe('foldDevJobEvent — phase cursor', () => {
+  it('starts at analyze (matches devJobStore.createJob\'s own default)', () => {
+    expect(INITIAL_LOG_STATE.phase).toBe('analyze');
+  });
+
+  it('a phase event updates the cursor without producing a log item', () => {
+    const state = foldDevJobEvent(INITIAL_LOG_STATE, ev(1, 'phase', { phase: 'bootstrap', state: 'start' }));
+    expect(state.phase).toBe('bootstrap');
+    expect(state.items).toEqual([]);
+  });
+
+  it('stamps subsequent tool/log items with the phase in effect when they arrived', () => {
+    const items = foldItems(
+      ev(1, 'log', { text: 'analyzing…', stream: 'agent' }),
+      ev(2, 'phase', { phase: 'plan', state: 'start' }),
+      ev(3, 'tool', { name: 'Write', inputPreview: '{"file_path":"plan.md"}' }),
+      ev(4, 'phase', { phase: 'implement', state: 'start' }),
+      ev(5, 'log', { text: 'implementing…', stream: 'agent' }),
+    );
+    expect(items.map((i) => i.phase)).toEqual(['analyze', 'plan', 'implement']);
+  });
+
+  it('a result event keeps the phase of its own start, even if the phase cursor since moved on', () => {
+    const items = foldItems(
+      ev(1, 'tool', { name: 'Bash', inputPreview: '{"command":"echo hi"}' }),
+      ev(2, 'phase', { phase: 'plan', state: 'start' }), // moves on before the result lands
+      ev(3, 'tool', { name: 'Bash', ok: true, outputPreview: 'hi' }),
+    );
+    expect(items).toHaveLength(1);
+    expect(items[0]?.phase).toBe('analyze');
+  });
+
+  it('ignores an unset phase field, keeping the previous cursor', () => {
+    const state = foldDevJobEvent(INITIAL_LOG_STATE, ev(1, 'phase', {}));
+    expect(state.phase).toBe('analyze');
+  });
+});
+
+describe('summarizeToolCall', () => {
+  const entry = (overrides: Partial): ToolCallEntry => ({
+    id: '1',
+    name: 'Read',
+    status: 'ok',
+    ...overrides,
+  });
+
+  it('summarizes Read/Write as the file path', () => {
+    const summary = summarizeToolCall(
+      entry({ name: 'Read', inputPreview: '{"file_path":"src/a.ts"}', outputPreview: 'body' }),
+    );
+    expect(summary.headline).toBe('src/a.ts');
+    expect(summary.detail).toEqual({ kind: 'file', filePath: 'src/a.ts', preview: 'body' });
+  });
+
+  it('summarizes Edit as a diff with add/remove counts', () => {
+    const summary = summarizeToolCall(
+      entry({
+        name: 'Edit',
+        inputPreview: JSON.stringify({ file_path: 'src/a.ts', old_string: 'foo', new_string: 'bar' }),
+      }),
+    );
+    expect(summary.headline).toBe('src/a.ts');
+    expect(summary.detail.kind).toBe('diff');
+    if (summary.detail.kind === 'diff') {
+      expect(summary.detail.filePath).toBe('src/a.ts');
+      expect(summary.detail.added).toBe(1);
+      expect(summary.detail.removed).toBe(1);
+    }
+  });
+
+  it('summarizes Bash using the description when present, else the command', () => {
+    const withDescription = summarizeToolCall(
+      entry({
+        name: 'Bash',
+        inputPreview: JSON.stringify({ command: 'ls -la', description: 'List files' }),
+        outputPreview: 'total 0',
+      }),
+    );
+    expect(withDescription.headline).toBe('List files');
+    expect(withDescription.detail).toEqual({
+      kind: 'command',
+      command: 'ls -la',
+      description: 'List files',
+      output: 'total 0',
+    });
+
+    const withoutDescription = summarizeToolCall(
+      entry({ name: 'Bash', inputPreview: JSON.stringify({ command: 'ls -la' }) }),
+    );
+    expect(withoutDescription.headline).toBe('ls -la');
+  });
+
+  it('summarizes Agent/Task with description + subagent type', () => {
+    const summary = summarizeToolCall(
+      entry({
+        name: 'Agent',
+        inputPreview: JSON.stringify({ description: 'Find X', subagent_type: 'Explore', prompt: 'find x' }),
+      }),
+    );
+    expect(summary.headline).toBe('Find X (Explore)');
+    expect(summary.detail).toEqual({
+      kind: 'agent',
+      subagentType: 'Explore',
+      description: 'Find X',
+      prompt: 'find x',
+      output: undefined,
+    });
+  });
+
+  it('falls back to "Agent" headline when neither description nor subagent type is present', () => {
+    const summary = summarizeToolCall(entry({ name: 'Agent', inputPreview: '{}' }));
+    expect(summary.headline).toBe('Agent');
+  });
+
+  it('summarizes Grep/Glob using the pattern', () => {
+    const summary = summarizeToolCall(
+      entry({ name: 'Grep', inputPreview: JSON.stringify({ pattern: 'TODO', path: 'src' }) }),
+    );
+    expect(summary.headline).toBe('TODO');
+    expect(summary.detail).toEqual({ kind: 'search', pattern: 'TODO', scope: 'src', output: undefined });
+  });
+
+  it('falls back to raw input/output for unknown tool names', () => {
+    const summary = summarizeToolCall(
+      entry({ name: 'TodoWrite', inputPreview: '{"todos":[]}', outputPreview: 'ok' }),
+    );
+    expect(summary.headline).toBe('TodoWrite');
+    expect(summary.detail).toEqual({ kind: 'raw', input: '{"todos":[]}', output: 'ok' });
+  });
+
+  it('tolerates malformed JSON input without throwing', () => {
+    const summary = summarizeToolCall(entry({ name: 'Read', inputPreview: 'not json' }));
+    expect(summary.headline).toBe('?');
+  });
+
+  it('renders an orphan result (no captured start) as raw/output-only, not a misleading zero-diff', () => {
+    // Regression: a start event dropped at an SSE reconnect boundary leaves
+    // inputPreview undefined; summarizeEdit must not be reached — it would
+    // silently compute file_path '?' and a 0-line diff, looking like a
+    // legitimate empty edit rather than "input never captured".
+    const summary = summarizeToolCall(entry({ name: 'Edit', inputPreview: undefined, outputPreview: 'ok' }));
+    expect(summary.headline).toBe('Edit');
+    expect(summary.detail).toEqual({ kind: 'raw', output: 'ok' });
+  });
+});
diff --git a/web-ui/app/admin/dev-platform/_lib/api.ts b/web-ui/app/admin/dev-platform/_lib/api.ts
index c4b2605ec..ea662f567 100644
--- a/web-ui/app/admin/dev-platform/_lib/api.ts
+++ b/web-ui/app/admin/dev-platform/_lib/api.ts
@@ -286,16 +286,61 @@ export function cancelJob(id: string): Promise<{ ok: boolean; status: string }>
   return req(`/jobs/${encodeURIComponent(id)}/cancel`, { method: 'POST', body: JSON.stringify({}) });
 }
 
+/** Terminal jobs only — the route answers 409 for an active job (cancel it first). */
+export function deleteJob(id: string): Promise {
+  return req(`/jobs/${encodeURIComponent(id)}`, { method: 'DELETE' });
+}
+
 export function retryJob(id: string): Promise<{ ok: boolean; jobId: string }> {
   return req(`/jobs/${encodeURIComponent(id)}/retry`, { method: 'POST', body: JSON.stringify({}) });
 }
 
+/** Mirrors `middleware/src/devplatform/types.ts`'s `DEV_JOB_ARTIFACT_KINDS`. */
+export type DevJobArtifactKind =
+  | 'diff'
+  | 'test_report'
+  | 'analysis'
+  | 'plan'
+  | 'summary'
+  | 'bootstrap_report'
+  | 'questions'
+  | 'answers'
+  | 'review_verdict';
+
+export interface DevJobArtifactSummary {
+  id: string;
+  jobId: string;
+  kind: DevJobArtifactKind;
+  meta: Record | null;
+  bytes: number;
+  createdAt: string;
+}
+
+/** All artifacts recorded for a job (metadata only — fetch content per-id via
+ *  `getArtifactText`). Used to show a completed phase's own output (plan,
+ *  clarify questions, bootstrap log, ...) once the live SSE log has nothing
+ *  left to show for it. */
+export function listJobArtifacts(id: string): Promise<{ artifacts: DevJobArtifactSummary[] }> {
+  return req(`/jobs/${encodeURIComponent(id)}/artifacts`);
+}
+
 /** Same-origin URL for an artifact's text content (the plan is a text artifact).
  *  `GET /artifacts/:id` returns `text/plain`; opening it in a new tab shows the
  *  plan the operator is being asked to approve. */
 export const DEV_ARTIFACT_PATH = (artifactId: string): string =>
   `${BASE}/artifacts/${encodeURIComponent(artifactId)}`;
 
+/** Fetch an artifact's raw text content (e.g. the plan shown inline at the
+ *  gate) — `req()` above assumes a JSON body, `GET /artifacts/:id` does not. */
+export async function getArtifactText(artifactId: string): Promise {
+  const res = await fetch(DEV_ARTIFACT_PATH(artifactId), { credentials: 'include', cache: 'no-store' });
+  const text = await res.text();
+  if (!res.ok) {
+    throw new ApiError(res.status, `GET /artifacts/${artifactId} failed: ${res.status}`, text);
+  }
+  return text;
+}
+
 // ── GitHub App — manifest flow + registry (W2, spec §2/§9) ───────────────────
 
 export interface DevGithubAppSummary {
diff --git a/web-ui/app/admin/dev-platform/_lib/lineDiff.ts b/web-ui/app/admin/dev-platform/_lib/lineDiff.ts
new file mode 100644
index 000000000..2d0e51436
--- /dev/null
+++ b/web-ui/app/admin/dev-platform/_lib/lineDiff.ts
@@ -0,0 +1,67 @@
+/**
+ * Epic #470 — small line-based diff for the job log's `Edit` tool-call cards
+ * (`old_string`/`new_string` → an inline unified diff). Self-contained
+ * classic LCS diff — no external dependency for something this small.
+ */
+
+export interface DiffLine {
+  type: 'add' | 'remove' | 'context';
+  text: string;
+}
+
+/** Defensive cap on the O(n·m) LCS table; pathologically large inputs fall
+ *  back to a plain remove-all/add-all block instead of hanging the tab. */
+const MAX_DIFF_CELLS = 4_000_000;
+
+export function computeLineDiff(oldText: string, newText: string): DiffLine[] {
+  const a = oldText.length > 0 ? oldText.split('\n') : [];
+  const b = newText.length > 0 ? newText.split('\n') : [];
+
+  if (a.length * b.length > MAX_DIFF_CELLS) {
+    return [
+      ...a.map((text): DiffLine => ({ type: 'remove', text })),
+      ...b.map((text): DiffLine => ({ type: 'add', text })),
+    ];
+  }
+
+  const n = a.length;
+  const m = b.length;
+  const dp: number[][] = Array.from({ length: n + 1 }, () => new Array(m + 1).fill(0));
+  for (let i = n - 1; i >= 0; i--) {
+    const row = dp[i];
+    if (!row) continue;
+    for (let j = m - 1; j >= 0; j--) {
+      row[j] = a[i] === b[j] ? dpAt(dp, i + 1, j + 1) + 1 : Math.max(dpAt(dp, i + 1, j), dpAt(dp, i, j + 1));
+    }
+  }
+
+  const out: DiffLine[] = [];
+  let i = 0;
+  let j = 0;
+  while (i < n && j < m) {
+    if (a[i] === b[j]) {
+      out.push({ type: 'context', text: a[i] ?? '' });
+      i++;
+      j++;
+    } else if (dpAt(dp, i + 1, j) >= dpAt(dp, i, j + 1)) {
+      out.push({ type: 'remove', text: a[i] ?? '' });
+      i++;
+    } else {
+      out.push({ type: 'add', text: b[j] ?? '' });
+      j++;
+    }
+  }
+  while (i < n) {
+    out.push({ type: 'remove', text: a[i] ?? '' });
+    i++;
+  }
+  while (j < m) {
+    out.push({ type: 'add', text: b[j] ?? '' });
+    j++;
+  }
+  return out;
+}
+
+function dpAt(dp: number[][], i: number, j: number): number {
+  return dp[i]?.[j] ?? 0;
+}
diff --git a/web-ui/app/admin/dev-platform/_lib/prettyArtifact.ts b/web-ui/app/admin/dev-platform/_lib/prettyArtifact.ts
new file mode 100644
index 000000000..3c87285b8
--- /dev/null
+++ b/web-ui/app/admin/dev-platform/_lib/prettyArtifact.ts
@@ -0,0 +1,20 @@
+/**
+ * Epic #470 — parse a JSON artifact's text (plan, analysis, bootstrap_report,
+ * ...) into a plain record for readable rendering. Artifact schemas vary by
+ * `kind` and aren't rigidly typed client-side, so this stays fully generic:
+ * callers render each top-level field by its own JS type rather than a
+ * per-kind template. Returns `null` for anything that isn't parseable JSON or
+ * isn't a plain object (an array or primitive at the top level) — the caller
+ * falls back to showing the raw text verbatim in that case.
+ */
+export function parseArtifactRecord(text: string): Record | null {
+  let parsed: unknown;
+  try {
+    parsed = JSON.parse(text);
+  } catch {
+    return null;
+  }
+  return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)
+    ? (parsed as Record)
+    : null;
+}
diff --git a/web-ui/app/admin/dev-platform/_lib/toolCallLog.ts b/web-ui/app/admin/dev-platform/_lib/toolCallLog.ts
new file mode 100644
index 000000000..c9e5dbcbb
--- /dev/null
+++ b/web-ui/app/admin/dev-platform/_lib/toolCallLog.ts
@@ -0,0 +1,209 @@
+import type { DevJobEventMessage } from '@/app/_lib/useDevJobEvents';
+
+import { computeLineDiff, type DiffLine } from './lineDiff';
+
+/**
+ * Epic #470 — turns the raw `dev_job_events` SSE tail into renderable log
+ * items for EVERY phase's log pane, not just implement (analyze/bootstrap/
+ * plan/clarify each run a real `claude -p` session or the bootstrap command,
+ * emitting the same event shapes — see `phaseLoop.ts`). Three responsibilities:
+ *
+ *   1. `foldDevJobEvent` tracks a running "current phase" cursor, updated on
+ *      each `phase` event (`{phase, state:'start'}` — always the first event
+ *      of a provision), and stamps every subsequent tool/log item with it, so
+ *      the UI can filter the single flat event stream down to one phase's
+ *      view without a second query.
+ *   2. It also pairs a tool call's two independent wire events (`{name,
+ *      inputPreview}` at start, `{ok, name, outputPreview}` at result — no
+ *      shared correlation id) into one `ToolCallEntry`, by walking back to
+ *      the nearest still-pending entry with the same tool name. Safe for the
+ *      single-threaded CLI agent loop this feeds from: a tool cannot start a
+ *      second call under the same name before the first resolves.
+ *   3. `summarizeToolCall` turns a paired entry's raw JSON `inputPreview`
+ *      into a one-line headline + a tool-shaped detail (a diff for `Edit`,
+ *      a command for `Bash`, etc.) instead of the previous `$ Name {...raw
+ *      JSON...}` dump. Unknown tool names fall back to the raw
+ *      input/output text — nothing silently disappears.
+ */
+
+export type ToolCallStatus = 'pending' | 'ok' | 'error';
+
+export interface ToolCallEntry {
+  id: string;
+  name: string;
+  status: ToolCallStatus;
+  inputPreview?: string;
+  outputPreview?: string;
+}
+
+export type LogItem =
+  | { kind: 'text'; id: string; phase: string; stream: 'agent' | 'stderr'; text: string }
+  | { kind: 'tool'; phase: string; entry: ToolCallEntry };
+
+export interface LogState {
+  items: LogItem[];
+  /** The most recently started phase — new tool/log items are stamped with this. */
+  phase: string;
+}
+
+/** `'analyze'` matches devJobStore.createJob's own default starting phase. */
+export const INITIAL_LOG_STATE: LogState = { items: [], phase: 'analyze' };
+
+export function foldDevJobEvent(state: LogState, ev: DevJobEventMessage): LogState {
+  if (ev.type === 'phase') {
+    const phase = typeof ev.payload['phase'] === 'string' ? ev.payload['phase'] : state.phase;
+    return { ...state, phase };
+  }
+  if (ev.type === 'tool') return { ...state, items: foldToolEvent(state.items, ev, state.phase) };
+  if (ev.type === 'log') return { ...state, items: foldLogEvent(state.items, ev, state.phase) };
+  return state;
+}
+
+function foldToolEvent(items: LogItem[], ev: DevJobEventMessage, phase: string): LogItem[] {
+  const p = ev.payload;
+  const name = typeof p['name'] === 'string' ? p['name'] : 'tool';
+  const hasResult = typeof p['ok'] === 'boolean';
+
+  if (!hasResult) {
+    const inputPreview = typeof p['inputPreview'] === 'string' ? p['inputPreview'] : undefined;
+    return [...items, { kind: 'tool', phase, entry: { id: String(ev.id), name, status: 'pending', inputPreview } }];
+  }
+
+  const ok = p['ok'] === true;
+  const outputPreview = typeof p['outputPreview'] === 'string' ? p['outputPreview'] : undefined;
+  const idx = findLastPending(items, name);
+  if (idx === -1) {
+    // No matching start (e.g. a reconnect landed mid-call) — render standalone.
+    return [
+      ...items,
+      { kind: 'tool', phase, entry: { id: String(ev.id), name, status: ok ? 'ok' : 'error', outputPreview } },
+    ];
+  }
+
+  const target = items[idx];
+  if (!target || target.kind !== 'tool') return items;
+  const next = items.slice();
+  next[idx] = { kind: 'tool', phase: target.phase, entry: { ...target.entry, status: ok ? 'ok' : 'error', outputPreview } };
+  return next;
+}
+
+function foldLogEvent(items: LogItem[], ev: DevJobEventMessage, phase: string): LogItem[] {
+  const p = ev.payload;
+  const text = typeof p['text'] === 'string' ? p['text'] : '';
+  if (!text) return items;
+  return [
+    ...items,
+    { kind: 'text', id: String(ev.id), phase, stream: p['stream'] === 'stderr' ? 'stderr' : 'agent', text },
+  ];
+}
+
+function findLastPending(items: LogItem[], name: string): number {
+  for (let i = items.length - 1; i >= 0; i--) {
+    const item = items[i];
+    if (item && item.kind === 'tool' && item.entry.name === name && item.entry.status === 'pending') return i;
+  }
+  return -1;
+}
+
+// --- Summarization -----------------------------------------------------
+
+export type ToolCallDetail =
+  | { kind: 'diff'; filePath: string; diff: DiffLine[]; added: number; removed: number }
+  | { kind: 'command'; command: string; description?: string; output?: string }
+  | { kind: 'file'; filePath: string; preview?: string }
+  | { kind: 'agent'; subagentType?: string; description?: string; prompt?: string; output?: string }
+  | { kind: 'search'; pattern: string; scope?: string; output?: string }
+  | { kind: 'raw'; input?: string; output?: string };
+
+export interface ToolCallSummary {
+  headline: string;
+  detail: ToolCallDetail;
+}
+
+export function summarizeToolCall(entry: ToolCallEntry): ToolCallSummary {
+  // `inputPreview === undefined` means the start event was never captured
+  // client-side (e.g. dropped at an SSE reconnect boundary while the result
+  // still arrived) — NOT "the tool had no arguments" (a real start event
+  // always carries a JSON object, even if empty: `{}`). Per-tool summarizers
+  // below assume a present-but-possibly-empty input and would otherwise
+  // render a misleading zero-diff/empty headline for a call whose real
+  // arguments were simply never seen. Fall back to the same raw/output-only
+  // rendering used for unknown tool names instead.
+  if (entry.inputPreview === undefined) {
+    return { headline: entry.name, detail: { kind: 'raw', output: entry.outputPreview } };
+  }
+  const input = parseJsonObject(entry.inputPreview);
+  switch (entry.name) {
+    case 'Read':
+    case 'Write':
+      return summarizeFileTool(input, entry.outputPreview);
+    case 'Edit':
+      return summarizeEdit(input);
+    case 'Bash':
+      return summarizeBash(input, entry.outputPreview);
+    case 'Agent':
+    case 'Task':
+      return summarizeAgent(input, entry.outputPreview);
+    case 'Grep':
+    case 'Glob':
+      return summarizeSearch(entry.name, input, entry.outputPreview);
+    default:
+      return { headline: entry.name, detail: { kind: 'raw', input: entry.inputPreview, output: entry.outputPreview } };
+  }
+}
+
+function parseJsonObject(text: string | undefined): Record {
+  if (!text) return {};
+  try {
+    const parsed: unknown = JSON.parse(text);
+    return typeof parsed === 'object' && parsed !== null ? (parsed as Record) : {};
+  } catch {
+    return {};
+  }
+}
+
+function str(v: unknown): string | undefined {
+  return typeof v === 'string' && v.length > 0 ? v : undefined;
+}
+
+function summarizeFileTool(input: Record, output: string | undefined): ToolCallSummary {
+  const filePath = str(input['file_path']) ?? '?';
+  return { headline: filePath, detail: { kind: 'file', filePath, preview: output } };
+}
+
+function summarizeEdit(input: Record): ToolCallSummary {
+  const filePath = str(input['file_path']) ?? '?';
+  const oldString = str(input['old_string']) ?? '';
+  const newString = str(input['new_string']) ?? '';
+  const diff = computeLineDiff(oldString, newString);
+  const added = diff.filter((l) => l.type === 'add').length;
+  const removed = diff.filter((l) => l.type === 'remove').length;
+  return { headline: filePath, detail: { kind: 'diff', filePath, diff, added, removed } };
+}
+
+function summarizeBash(input: Record, output: string | undefined): ToolCallSummary {
+  const command = str(input['command']) ?? '';
+  const description = str(input['description']);
+  return { headline: description ?? command, detail: { kind: 'command', command, description, output } };
+}
+
+function summarizeAgent(input: Record, output: string | undefined): ToolCallSummary {
+  const description = str(input['description']);
+  const subagentType = str(input['subagent_type']);
+  const prompt = str(input['prompt']);
+  const headline = [description, subagentType ? `(${subagentType})` : undefined].filter(Boolean).join(' ');
+  return {
+    headline: headline.length > 0 ? headline : 'Agent',
+    detail: { kind: 'agent', subagentType, description, prompt, output },
+  };
+}
+
+function summarizeSearch(
+  name: string,
+  input: Record,
+  output: string | undefined,
+): ToolCallSummary {
+  const pattern = str(input['pattern']) ?? '';
+  const scope = str(input['path']) ?? str(input['glob']);
+  return { headline: pattern.length > 0 ? pattern : name, detail: { kind: 'search', pattern, scope, output } };
+}
diff --git a/web-ui/app/admin/dev-platform/jobs/[id]/page.tsx b/web-ui/app/admin/dev-platform/jobs/[id]/page.tsx
index aa1d25706..1d9867b84 100644
--- a/web-ui/app/admin/dev-platform/jobs/[id]/page.tsx
+++ b/web-ui/app/admin/dev-platform/jobs/[id]/page.tsx
@@ -13,46 +13,45 @@ import {
   DEV_JOB_UI_PHASES,
   DevJobPhaseRail,
   computePhaseStops,
+  phaseToUi,
   statusIsLive,
   type DevJobUiPhase,
 } from '@/app/_components/devjobs/DevJobPhaseRail';
+import { findGateForJob } from '@/app/_components/devjobs/devJobChatCardState';
 import { useDevJobEvents, type DevJobEventMessage } from '@/app/_lib/useDevJobEvents';
-import { JobLogPane, type LogConnection, type LogLine } from '../../_components/JobLogPane';
-import { cancelJob, getJob, isTerminalStatus, type DevJobView } from '../../_lib/api';
+import { GateCard } from '../../_components/GateInbox';
+import { JobLogPane, type LogConnection } from '../../_components/JobLogPane';
+import { PhaseArtifactPanel } from '../../_components/PhaseArtifactPanel';
+import {
+  cancelJob,
+  deleteJob,
+  getJob,
+  isTerminalStatus,
+  listWaitingGates,
+  type DevGateView,
+  type DevJobView,
+} from '../../_lib/api';
+import { INITIAL_LOG_STATE, foldDevJobEvent, type LogState } from '../../_lib/toolCallLog';
 
 /**
  * Epic #470 W0 — the job-detail signature screen (UI spec §5). Header, the
  * phase rail (keyboard-operable, deep-linkable via `?phase=`), then a two-column
  * body: the log pane (driven by rail selection) and a metadata sidebar. The
  * live log streams over SSE through `useDevJobEvents` and sticks to bottom via
- * `useStickToBottom`. W0 is minimal: only the `implement` phase has a live-log
- * pane; other phases show "no artifact yet" (W2 fills them in).
+ * `useStickToBottom`. Every non-`gate`/`pr` phase stacks a `PhaseArtifactPanel`
+ * (that phase's own recorded plan/questions/bootstrap_report/review_verdict,
+ * once it exists) above the same live log pane, filtered to that phase's own
+ * events (`toolCallLog.ts` stamps each item with the phase it happened in) —
+ * analyze/bootstrap/plan/clarify run real agent sessions too, not just
+ * implement. The log pane alone is live-only state, so navigating back to an
+ * already-finished phase (or reloading the page) left it permanently empty
+ * without the artifact panel as a second, persisted source.
  */
 
 function shortHash(id: string): string {
   return id.replace(/-/g, '').slice(0, 6);
 }
 
-function eventToLine(ev: DevJobEventMessage): LogLine | null {
-  const p = ev.payload as Record;
-  if (ev.type === 'tool') {
-    const name = typeof p['name'] === 'string' ? p['name'] : 'tool';
-    const preview =
-      typeof p['inputPreview'] === 'string'
-        ? p['inputPreview']
-        : typeof p['outputPreview'] === 'string'
-          ? p['outputPreview']
-          : '';
-    return { id: String(ev.id), stream: 'tool', text: preview ? `${name} ${preview}` : name };
-  }
-  if (ev.type === 'log') {
-    const text = typeof p['text'] === 'string' ? p['text'] : '';
-    if (!text) return null;
-    return { id: String(ev.id), stream: p['stream'] === 'stderr' ? 'stderr' : 'agent', text };
-  }
-  return null;
-}
-
 export default function JobDetailPage(): React.ReactElement {
   const t = useTranslations('adminDevPlatform.detail');
   const params = useParams<{ id: string }>();
@@ -62,12 +61,13 @@ export default function JobDetailPage(): React.ReactElement {
 
   const [job, setJob] = useState(null);
   const [notFound, setNotFound] = useState(false);
-  const [lines, setLines] = useState([]);
+  const [logState, setLogState] = useState(INITIAL_LOG_STATE);
   const [conn, setConn] = useState('reconnecting');
   const [lastEventAt, setLastEventAt] = useState(null);
   const [agoSec, setAgoSec] = useState(null);
   const [closedOnce, setClosedOnce] = useState(false);
   const [confirmCancel, setConfirmCancel] = useState(false);
+  const [confirmDelete, setConfirmDelete] = useState(false);
 
   const terminalRef = useRef(false);
   useEffect(() => {
@@ -85,8 +85,7 @@ export default function JobDetailPage(): React.ReactElement {
 
   const handleEvent = useCallback((ev: DevJobEventMessage) => {
     setLastEventAt(Date.now());
-    const line = eventToLine(ev);
-    if (line) setLines((prev) => [...prev, line]);
+    setLogState((prev) => foldDevJobEvent(prev, ev));
     if (ev.type === 'status' || ev.type === 'phase') {
       // Re-sync the authoritative view on lifecycle transitions.
       void getJob(ev.jobId).then(
@@ -125,6 +124,38 @@ export default function JobDetailPage(): React.ReactElement {
     return () => clearInterval(timer);
   }, [conn, lastEventAt]);
 
+  // Fetch the waiting gate for this job whenever it parks at the human gate —
+  // shown inline on the GATE stop instead of sending the operator to a
+  // separate tab to approve/reject (mirrors DevJobChatCard.tsx's pattern).
+  // `gate` is derived from the fetch + the job's live status rather than
+  // reset via a synchronous setState in the effect's early return: once the
+  // job leaves `waiting` this naturally reads as null without a second write.
+  const isWaiting = job?.status === 'waiting';
+  const [fetchedGate, setFetchedGate] = useState(null);
+  const gate = isWaiting ? fetchedGate : null;
+
+  useEffect(() => {
+    if (!isWaiting) return;
+    let cancelled = false;
+    void listWaitingGates().then(
+      (res) => {
+        if (!cancelled) setFetchedGate(findGateForJob(res.gates, id));
+      },
+      () => {},
+    );
+    return () => {
+      cancelled = true;
+    };
+  }, [isWaiting, id]);
+
+  const onGateResolved = useCallback(() => {
+    setFetchedGate(null);
+    void getJob(id).then(
+      (j) => setJob(j),
+      () => {},
+    );
+  }, [id]);
+
   // Deep-link: the viewed phase comes from `?phase=`.
   const rawPhase = search?.get('phase') ?? null;
   const selected: DevJobUiPhase | null =
@@ -178,6 +209,11 @@ export default function JobDetailPage(): React.ReactElement {
             {t('cancel.action')}
           
         ) : null}
+        {job && isTerminalStatus(job.status) ? (
+          
+        ) : null}
       
 
       {/* Phase rail */}
@@ -188,8 +224,10 @@ export default function JobDetailPage(): React.ReactElement {
       {/* Body */}
       
   );
 }
diff --git a/web-ui/app/admin/dev-platform/page.tsx b/web-ui/app/admin/dev-platform/page.tsx
index 4f23c489e..f56d72832 100644
--- a/web-ui/app/admin/dev-platform/page.tsx
+++ b/web-ui/app/admin/dev-platform/page.tsx
@@ -15,6 +15,7 @@ import { GateInbox } from './_components/GateInbox';
 import {
   cancelJob,
   checkRepo,
+  deleteJob,
   deleteRepo,
   listJobs,
   listRepos,
@@ -240,6 +241,9 @@ function JobsTab(): React.ReactElement {
         onCancel={(job) => {
           void cancelJob(job.id).then(load, load);
         }}
+        onDelete={(job) => {
+          void deleteJob(job.id).then(load, load);
+        }}
       />
     
   );
diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json
index 9518771a9..70eaf4af6 100644
--- a/web-ui/messages/de.json
+++ b/web-ui/messages/de.json
@@ -2981,6 +2981,8 @@
       "plan": "Plan",
       "viewPlan": "Plan ansehen",
       "noPlan": "Kein Plan-Artefakt",
+      "planLoading": "Plan wird geladen…",
+      "planLoadError": "Der Plan konnte nicht geladen werden.",
       "holders": "Holder",
       "noHolders": "keine",
       "questions": "Rückfragen",
@@ -3071,6 +3073,7 @@
       "age": "Alter",
       "view": "Ansehen",
       "cancel": "Abbrechen",
+      "delete": "Löschen",
       "empty": "Noch keine Jobs. Starte einen aus einer Repository-Zeile.",
       "live": "live",
       "liveLost": "Verbindung verloren — versucht erneut",
@@ -3100,6 +3103,12 @@
         "body": "Der Runner wird beendet. Branch, Log und ein bereits hochgeladener Diff bleiben erhalten.",
         "confirm": "Job abbrechen",
         "cancel": "Weiterlaufen lassen"
+      },
+      "deleteConfirm": {
+        "title": "Job löschen?",
+        "body": "Der Job, sein Log und seine Artefakte werden endgültig entfernt. Ein bereits erstellter Branch oder PR bleibt erhalten.",
+        "confirm": "Job löschen",
+        "cancel": "Behalten"
       }
     },
     "newJob": {
@@ -3216,8 +3225,17 @@
         "pr": "pr"
       },
       "phaseSkipped": "übersprungen — keine Fragen",
-      "noArtifact": "Für diese Phase gibt es noch kein Artefakt.",
+      "toolCall": {
+        "pending": "läuft",
+        "failed": "fehlgeschlagen",
+        "noOutput": "(keine Ausgabe)",
+        "prompt": "Prompt",
+        "result": "Ergebnis",
+        "output": "Ausgabe",
+        "moreDiffLines": "… {count, plural, one {# weitere Zeile} other {# weitere Zeilen}}"
+      },
       "openPr": "Pull Request öffnen",
+      "artifactError": "Das Ergebnis dieser Phase konnte nicht geladen werden.",
       "logEmpty": "Noch keine Log-Ausgabe.",
       "scrollToBottom": "Nach unten scrollen",
       "connection": {
@@ -3232,6 +3250,13 @@
         "confirm": "Job abbrechen",
         "cancelLabel": "Weiterlaufen lassen"
       },
+      "delete": {
+        "action": "Löschen",
+        "title": "Job löschen?",
+        "body": "Der Job, sein Log und seine Artefakte werden endgültig entfernt. Ein bereits erstellter Branch oder PR bleibt erhalten.",
+        "confirm": "Job löschen",
+        "cancelLabel": "Behalten"
+      },
       "sidebar": {
         "backend": "Backend",
         "agent": "Agent",
diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json
index d5815ac0c..e63991780 100644
--- a/web-ui/messages/en.json
+++ b/web-ui/messages/en.json
@@ -2981,6 +2981,8 @@
       "plan": "Plan",
       "viewPlan": "View plan",
       "noPlan": "No plan artifact",
+      "planLoading": "Loading plan…",
+      "planLoadError": "The plan could not be loaded.",
       "holders": "Holders",
       "noHolders": "none",
       "questions": "Questions",
@@ -3071,6 +3073,7 @@
       "age": "Age",
       "view": "View",
       "cancel": "Cancel",
+      "delete": "Delete",
       "empty": "No jobs yet. Start one from a repository row.",
       "live": "live",
       "liveLost": "connection lost — retrying",
@@ -3100,6 +3103,12 @@
         "body": "The runner is terminated. The branch, the log, and any uploaded diff are kept.",
         "confirm": "Cancel job",
         "cancel": "Keep running"
+      },
+      "deleteConfirm": {
+        "title": "Delete job?",
+        "body": "The job, its log, and its artifacts are removed permanently. Any branch or PR it created is kept.",
+        "confirm": "Delete job",
+        "cancel": "Keep it"
       }
     },
     "newJob": {
@@ -3216,8 +3225,17 @@
         "pr": "pr"
       },
       "phaseSkipped": "skipped — no questions",
-      "noArtifact": "No artifact for this phase yet.",
+      "toolCall": {
+        "pending": "running",
+        "failed": "failed",
+        "noOutput": "(no output)",
+        "prompt": "Prompt",
+        "result": "Result",
+        "output": "Output",
+        "moreDiffLines": "… {count, plural, one {# more line} other {# more lines}}"
+      },
       "openPr": "Open pull request",
+      "artifactError": "This phase's recorded output could not be loaded.",
       "logEmpty": "No log output yet.",
       "scrollToBottom": "Scroll to bottom",
       "connection": {
@@ -3232,6 +3250,13 @@
         "confirm": "Cancel job",
         "cancelLabel": "Keep running"
       },
+      "delete": {
+        "action": "Delete",
+        "title": "Delete job?",
+        "body": "The job, its log, and its artifacts are removed permanently. Any branch or PR it created is kept.",
+        "confirm": "Delete job",
+        "cancelLabel": "Keep it"
+      },
       "sidebar": {
         "backend": "Backend",
         "agent": "Agent",