diff --git a/docker/Dockerfile b/docker/Dockerfile index 25c9024..39cb4c0 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -17,9 +17,10 @@ # none of the rest of the monorepo, nor a pnpm virtual store — just the deployed directory. # (`--legacy` is required: pnpm v10's default injected-workspace deploy needs # `inject-workspace-packages=true`, which this workspace does not set.) -# - The daemon supplies mounts (/work ro, /bundle ro, /run/radial rw), env -# (RADIAL_SIDECAR_SOCKET, RADIAL_TURN_TOKEN, ANTHROPIC_API_KEY, HTTPS_PROXY/HTTP_PROXY — see -# turn.ts), and the argv (currently `sh -c 'claude --print ... < brief.md'`, see harness.ts) at +# - The daemon supplies mounts (/work rw for implementation turns and ro otherwise, /bundle ro, +# /run/radial rw), env +# (RADIAL_SIDECAR_SOCKET, RADIAL_TURN_TOKEN, ANTHROPIC_API_KEY, GH_TOKEN — see +# turn.ts), and the argv (`claude -p ...`, see harness.ts) at # `docker run` time. This image deliberately sets no ENTRYPOINT/CMD and no opinionated WORKDIR # (turn.ts passes `-w /work` on every run). # - `/tmp` and `/home/radial` are tmpfs-mounted by the daemon at run time @@ -41,14 +42,9 @@ RUN pnpm --filter=@radial/sidecar deploy /out --prod --legacy FROM node:24-slim AS runtime -# git: the daemon clones the project repo on the HOST (daemon-side, no credentials in the container) -# and bind-mounts the checkout into the sandbox; git is still needed IN the container so an -# implementation turn can `git add`/`commit`/`bundle` its work in /work and write the ranged bundle -# to the rw /export mount (the daemon fetches that bundle and does the push/PR — the container never -# pushes and never sees a git/forge credential, §13). ca-certificates: claude's API calls (and any -# in-container git that reads https config) need a trust store to validate TLS. +# Implementation turns use git and gh directly, with the operator's ephemeral GH_TOKEN. RUN apt-get update \ - && apt-get install -y --no-install-recommends git ca-certificates \ + && apt-get install -y --no-install-recommends git gh ca-certificates \ && rm -rf /var/lib/apt/lists/* # @anthropic-ai/claude-code: harness.ts's ClaudeCodeHarness shells out to `claude --print ...`. diff --git a/docker/proxy.Dockerfile b/docker/proxy.Dockerfile deleted file mode 100644 index 238a7da..0000000 --- a/docker/proxy.Dockerfile +++ /dev/null @@ -1,34 +0,0 @@ -# radial-proxy: the daemon-managed allowlisting egress proxy container (design §13, "egress -# enforced NOW via internal Docker network + allowlisting proxy, no bridge fallback"). Attached to -# both the internal network (so turn containers can reach it by name) and an external "egress" -# network (so it alone can reach the model API) — see packages/daemon/src/egress.ts's -# createEgressManager, which is what starts/stops this container and connects the two networks. -# -# Runs packages/daemon/dist/proxy.js's standalone CLI entry directly. That file imports only -# node:fs, node:http, node:net, node:process, node:url (verified against the built output — see -# the phase-3 workstream E-final report), so this image copies that single compiled file rather -# than the whole @radial/daemon package or a node_modules tree. -# -# Build from the repo root (after `pnpm build` has produced packages/daemon/dist/proxy.js): -# docker build -f docker/proxy.Dockerfile -t radial-proxy:latest . - -FROM node:24-slim AS runtime - -# Non-root uid/gid 1000, matching this repo's sandbox convention for every container it runs. -# node:24-slim already ships uid/gid 1000 (the `node` user); only the uid matters at runtime. - -COPY packages/daemon/dist/proxy.js /opt/radial/proxy.js -RUN chmod 0644 /opt/radial/proxy.js - -USER 1000:1000 -EXPOSE 8080 - -# `--port 8080` is proxy.js's own default; spelled out here for clarity. proxy.ts's standalone -# entry binds 0.0.0.0 (not 127.0.0.1 — see proxy.ts's `listen(port, host)`), which is required for -# the turn container, on the same internal network, to reach this container by name. -ENTRYPOINT ["node", "/opt/radial/proxy.js", "--port", "8080"] - -# The allowlist: egress.ts's `proxyRunArgs` passes `--allow ` (repeatable) as this -# container's trailing argv/CMD, appended by docker after ENTRYPOINT. This default is only used if -# the container is run directly without args. -CMD ["--allow", "api.anthropic.com"] diff --git a/packages/core/src/turn-protocol.ts b/packages/core/src/turn-protocol.ts index f94d258..6ec8a47 100644 --- a/packages/core/src/turn-protocol.ts +++ b/packages/core/src/turn-protocol.ts @@ -1,16 +1,9 @@ import type { StrongRef } from './generated/records.js' -export interface SubmitArtifactRpc { method: 'submitArtifact'; token: string; body: string; criteria?: string[]; commit?: string } +export interface SubmitArtifactRpc { method: 'submitArtifact'; token: string; body: string; criteria?: string[]; branch?: string; commit?: string; pr?: string } export interface AskQuestionRpc { method: 'askQuestion'; token: string; body: string } export type TurnRpcRequest = SubmitArtifactRpc | AskQuestionRpc -export type TurnRpcResponse = - // A synchronous write path (plan-style turns) returns the strongref of the record it wrote. - | { ok: true; ref: StrongRef } - // An implementation turn's submission is only persisted as a pending finish (no record yet), so - // the daemon acknowledges acceptance without a ref — the artifact record is written later by the - // finish pump. See turn-socket.ts #submitArtifact. - | { ok: true; accepted: true } - | { ok: false; error: string } +export type TurnRpcResponse = { ok: true; ref: StrongRef } | { ok: false; error: string } export const TURN_LIMITS = { artifactBodyChars: 100_000, messageBodyChars: 30_000, diff --git a/packages/daemon/README.md b/packages/daemon/README.md index b3c753c..29e2955 100644 --- a/packages/daemon/README.md +++ b/packages/daemon/README.md @@ -1,302 +1,41 @@ -# `radiald` — the turn daemon +# Radial daemon -`radiald run` polls a set of spaces, hands each eligible open `plan` request to -a sandboxed `radial-turn` container, and durably tracks every attempt in a -turn ledger. This is an operator runbook for driving one real end-to-end loop -with the real `claude` CLI — everything else in this repo's test suite runs -against fakes (`FakeContainerRunner`, `LocalPds`) because Docker isn't -available in CI/dev sandboxes; this document is what to do on a real machine -with Docker installed. +`radiald` dispatches explicit artifact requests into isolated containers. The container has open +egress by design, but never receives Radial/atproto credentials: the local sidecar remains its only +protocol write path. Operators are responsible for blocking cloud metadata endpoints in their +deployment environment. -## 1. Prerequisites +## GitHub possession -### Build the images +Implementation turns act directly as the operator on GitHub. `radiald init` reuses `gh auth` or +starts `gh auth login --hostname github.com --git-protocol https --web`. A repository-scoped +fine-grained PAT may instead be supplied as `GH_TOKEN` (legacy `GITHUB_TOKEN` is accepted). Tokens +are never stored in radial configuration or protocol records. Keep `GH_TOKEN` set for the daemon; +it is injected only into implementation turns and is also used for observation polling. -```sh -pnpm images # node scripts/build-images.mjs — builds the workspace, then - # `docker build`s radial-turn:latest and radial-proxy:latest -``` - -`docker/Dockerfile` (`radial-turn`) bundles the `@radial/sidecar` CLI (`radial`, -on `PATH`), `git`, and a global install of `@anthropic-ai/claude-code` (`claude` -on `PATH`), running as uid/gid 1000. `docker/proxy.Dockerfile` (`radial-proxy`) -is a single-file image running `packages/daemon/dist/proxy.js`'s standalone -CLI — the allowlisting egress proxy described in §4 below. - -### `radial.json` - -`radiald run` reads the nearest `radial.json` (or `$RADIAL_CONFIG`, or -`~/.config/radial/radial.json`) for the operator's identity/harness/model -defaults plus a `run` block. Every `run` field, and its default if omitted: - -| Field | Default | Meaning | -| --- | --- | --- | -| `spaces` | *(required)* | `at://` space URIs to poll for dispatchable requests. | -| `stateDir` | `/run` | Where the ledger (`ledger.db`), the sync/records SQLite files, and each turn's scratch directory (`/turns//`) live. | -| `image` | `radial-turn:latest` | The turn image to run. | -| `concurrency` | `1` | Max turns running at once. | -| `timeoutMs` | `900000` (15m) | Per-turn container timeout; a turn that outlives it is killed and reported `timedOut`. | -| `cooldownMs` | `300000` (5m) | Backoff after a crash before a request is eligible again. | -| `retryBound` | `3` | Crashes before a request is marked "gave up" and stops being dispatched. | -| `network` | `radial-internal` (when `egress` is on) | The docker network turn containers attach to. Also doubles as the egress manager's internal-network name — see §4. | -| `egress` | `true` | Egress-enforcement toggle — see §4. | -| `egressNetwork` | `radial-egress` | The external-route network the `radial-proxy` container attaches to. | -| `proxyImage` | `radial-proxy:latest` | The proxy image `radiald run` starts. | -| `allowlist` | `["api.anthropic.com"]` | Hosts the proxy will `CONNECT` to. | -| `gitSchemes` | `["https"]` | Git URL schemes `checkoutRepo` will clone. Add `"file"` for local/dev — see §5. | - -A minimal `run` block, relying entirely on defaults: - -```json -{ - "identifier": "operator.example", - "harness": "claude", - "models": ["claude-opus-4-8=high"], - "agents": { - "planner": { "artifactTypes": ["plan"] } - }, - "run": { - "spaces": ["at://did:plc:human/com.disnetdev.radial.space/space1"] - } -} -``` - -### Register agent profiles - -```sh -printf '%s\n' "$AGENT_APP_PASSWORD" | radiald init --password-stdin -``` - -registers every profile in `agents` (or pass profile names to register only -some); `radiald run` refuses to start if no profile initialized successfully. - -### `ANTHROPIC_API_KEY` - -Set in the **daemon's own** environment (not the container's — `radiald run` -threads it into each turn's `ContainerSpec.env` itself, see `turn.ts`): - -```sh -export ANTHROPIC_API_KEY=sk-ant-... -radiald run -``` - -Container `HTTPS_PROXY`/`HTTP_PROXY` are also set automatically when egress -is enforced (§4) — `claude`'s own HTTPS client honors them for the one host -(`api.anthropic.com` by default) the proxy allows through. - -## 2. The loop, end to end - -A human creates a goal and a `plan` request assigned to an agent, using the -`radial` CLI (`packages/sidecar`) exactly as in the human-only loop -(`packages/sidecar/README.md`) — the daemon changes nothing about how work is -requested, only who does it: - -```sh -printf '%s\n' "$HUMAN_APP_PASSWORD" | radial auth login \ - --profile human --identifier human.example --password-stdin - -SPACE=$(radial space create --profile human --name radial-e2e) -radial member add --profile human --space "$SPACE" \ - --actor agent.example --kind agent --role member - -PROJECT=$(radial project create --profile human --space "$SPACE" \ - --name radial-ng --git-url https://example.com/radial-ng.git) -GOAL=$(radial goal create --profile human --project "$PROJECT" \ - --title 'Ship the feature' --body 'Acceptance criteria go here') - -REQUEST=$(radial request create --profile human --goal "$GOAL" \ - --type plan --assignee agent.example) -``` - -With `radiald run` already polling `$SPACE` (§1), the next poll tick picks up -`$REQUEST` (`dispatch.ts`'s `selectDispatchable`): it's a human-authored, -goal-scoped plan request assigned to an active agent member `radiald init` -registered, not already running/cooling down/given up per the ledger. The -daemon starts a `radial-turn` container attached to the internal network -(§4), mounts the bundle/checkout/turn-socket, and runs `claude --print ...` -against the brief. When `claude` calls the sidecar's turn-socket mode -(`radial artifact submit --body-file ...`, invoked with -`RADIAL_SIDECAR_SOCKET`/`RADIAL_TURN_TOKEN` already in its env — never atproto -credentials, see §6), the daemon writes the real `com.disnetdev.radial.artifact` -record and the turn is marked fulfilled. - -Watch it land two independent ways: - -```sh -# 1. the daemon's own SQLite-backed record cache (radiald sync keeps this -# warm; radiald run does too, as part of its poll loop) -radiald index "$SPACE" --records "$STATE_DIR/records.db" - -# 2. an independent read path that never touches the daemon's own state — -# export the goal's records to a directory of JSON files and read them -# with radial-debug (packages/core's own debug CLI), e.g. via -# `com.atproto.repo.listRecords` against the PDS, one file per record/page -radial-debug index "$GOAL" --dir ./exported-records/ -``` - -Both should show the `plan` request's open-request line gone and a new entry -in the goal's artifact timeline pointing at the fulfilled plan artifact. - -### Killing a turn mid-flight - -```sh -docker ps --filter name=radial.turn # find the running container (container - # name == its label, "radial.turn.") -docker kill -``` - -`turn.ts` classifies strictly by what the harness told the turn socket, never -by exit code — a killed container produces no `submitArtifact`/`askQuestion` -observation, so the turn comes back `crashed`. The ledger (`ledger.ts`) moves -the request into cooldown and, once `cooldownMs` elapses, it's dispatchable -again automatically; after `retryBound` crashes it's marked "gave up" and -stops being dispatched at all. To force a gave-up (or any other) row back to -dispatchable immediately: - -```sh -radiald turn reset "$REQUEST" -``` - -Startup also does this automatically for any row still `running` from a -process that died mid-turn (`reconcileOrphan` in `cli.ts`): it kills whatever -container survives and clears the row, so nothing is stuck forever just -because the daemon itself was killed. - -## 3. Preflight: does egress "enforced" actually apply? - -`radiald run` logs `egress proxy ready: network ..., proxy ...` on a -successful `egress.start()`. If it can't start the proxy/network at all, it -does not fall back to unrestricted egress — it refuses to start and exits -with `egress enforcement is enabled but the proxy/network could not be -started (...); refusing to run turns with unrestricted egress.` (see §4). -Confirm the enforced case actually landed before trusting it in front of a -real API key: - -```sh -docker network inspect radial-internal # "Internal": true -docker network inspect radial-egress -docker inspect radial-proxy --format '{{json .NetworkSettings.Networks}}' - # should list both radial-internal and radial-egress -``` - -## 4. The egress model +Use a dedicated, spend-capped `ANTHROPIC_API_KEY` for turns. Check containers are deliberately +secret-free (`env: {}`) and may use normal network access for dependency installation. -Per the locked design decision (design doc §13): **egress is enforced, not -opt-in** — a turn container never gets a direct route to the internet. It -runs on `radial-internal`, a `docker network create --internal` network with -no default route out. The only path to the model API is `radial-proxy`, a -daemon-managed container (`packages/daemon/src/egress.ts`) attached to both -`radial-internal` and `radial-egress` (the network with an actual external -route); it's an HTTP `CONNECT` proxy that only tunnels to hosts on -`run.allowlist` (`api.anthropic.com` by default, see `proxy.ts`'s -`isHostAllowed`) and 403s everything else before ever opening a socket to it. -`radiald run` sets `HTTPS_PROXY`/`HTTP_PROXY` in every turn container's env -to `http://radial-proxy:8080` — the turn container resolves that hostname -over the internal network's embedded DNS, since the proxy container is -attached there too. +## Direct PR workflow -**Default behavior**: `radiald run` enforces this by default — no -configuration required. On startup it calls `egress.start()` (ensuring both -networks exist, starting/joining the proxy container) *before* the dispatch -loop, and points the `TurnDispatcher` and every turn's `ContainerSpec` at the -resulting internal network and proxy. On shutdown it calls `egress.stop()` -(`docker rm -f radial-proxy`, best-effort) in the same `finally` that kills -in-flight turn containers. +An implementation turn configures `gh` git authentication, works on its reserved `radial/impl-*` +branch, commits with the agent/DID attribution trailer, pushes, and creates or updates its PR. Its +PR body includes the deterministic Radial artifact URI. It then calls: -**Opting out**: set `"egress": false` in the `run` block. Turn containers then -run on whatever `run.network` names (or the default docker bridge — i.e. -*unrestricted* egress — if `network` is also unset), with no proxy and no -`HTTPS_PROXY` injected. This is meant for local/dev only, and it is an -explicit, informed choice the operator makes in `radial.json` — not something -`radiald run` falls into on its own. - -**Fail closed, never fail open**: those are the *only* two allowed outcomes — -egress enforced, or the operator explicitly opted out. There is no silent -third state. If `egress` is on (the default) and `egress.start()` itself -fails — Docker unreachable, the `radial-proxy` image missing, a -network-create permission error, ... — `radiald run` does **not** continue -with an unenforced network. It closes what it had already opened (just the -turn ledger at that point) and refuses to start at all, with: - -``` -egress enforcement is enabled but the proxy/network could not be started (); -refusing to run turns with unrestricted egress. Fix Docker/networking, or set -"run": { "egress": false } to explicitly opt out. -``` - -Fix Docker/networking and restart, or make the informed call to set -`"egress": false` yourself. A daemon that silently ran turns with open egress -because Docker networking hiccuped would be exactly the "bridge fallback" -the design decision (§13) forbids. - -## 5. `file://` git checkouts (local/dev only) - -`bundle-writer.ts`'s `checkoutRepo` refuses any git URL whose scheme isn't in -`run.gitSchemes` (default `["https"]`) before ever invoking `git clone` — -`assertGitUrlAllowed` runs first, and a hardened `git` invocation second (no -credential helpers, no `GIT_*` env leaking through). Add `"file"` to -`gitSchemes` only for local/dev against a repo on the same filesystem as the -daemon: - -```json -{ "run": { "gitSchemes": ["https", "file"] } } ``` - -Do **not** enable `file://` in production: it lets any `plan` request's -project record point the checkout step at an arbitrary path on the daemon's -own host. - -## 6. macOS / Docker Desktop (dev mode) - -The turn socket (harness -> daemon RPC, §7) defaults to a bind-mounted unix -socket, which is the correct, production design on Linux. On Docker Desktop -for Mac it does not work: the host bind-mounts the socket file into the VM -where dockerd actually runs, and the *file* is visible there, but AF_UNIX -`connect()` cannot cross the macOS<->VM boundary — the container's connection -attempt gets ECONNREFUSED even though `ls` sees the socket. This is a -platform limitation, not a bug in `turn-socket.ts`. - -For local development on such a machine, opt into the TCP turn transport: - -```json -{ - "run": { - "turnTransport": "tcp", - "egress": false - } -} +radial artifact submit --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)" --pr "$(gh pr view --json url -q .url)" --body-file summary.md ``` -`resolveRunConfig` (`cli.ts`) enforces this pairing and fails closed: setting -`turnTransport: "tcp"` without an *explicit* `"egress": false` refuses to -start, because TCP reaches the daemon via `host.docker.internal`, a route an -enforced internal-only docker network (§4) blocks by design — egress -enforcement is simply not possible together with the TCP transport, so -`radiald run` never lets the two combine silently. When TCP mode is active, -`radiald run` also logs a prominent startup warning every time. - -**Security note**: with no bind-mounted socket file to gate by filesystem -permissions, the turn socket instead binds `0.0.0.0:0` (an ephemeral port) so -the container can reach it over the docker bridge from `host.docker.internal`. -That port is consequently briefly reachable from anything else on the local -network for the lifetime of one turn. The per-turn, cryptographically random -`RADIAL_TURN_TOKEN` (`crypto.randomUUID()`, shared by both transports, see -`turn.ts`) is what actually gates every RPC — it *is* the auth boundary in -this mode, not the network path. +The daemon synchronously verifies a lowercase full SHA and canonical same-repository PR URL, then +writes the typed artifact record. Turns must not merge. Forge reconnaissance is intentionally +allowed: `gh pr view`, `gh api`, and arbitrary web fetches work in a turn. -Production and Linux deployments should leave `turnTransport` unset (the -`unix` default) and keep egress enforcement on — nothing above changes their -behavior. +## Networking -## 7. Credentials never enter the container +With `run.network` omitted Docker uses its ordinary bridge. A custom network is supported, but +`host` is rejected. This is a possession boundary, not an egress allowlist: protect the daemon +socket and cloud metadata service at the operator layer. -The turn container never sees an atproto session, app password, or DID key. -`turn.ts` stands up a `TurnSocketServer` bound to a unix socket mounted -read-write at `/run/radial`, authenticated by a single-use, per-turn -`RADIAL_TURN_TOKEN`; the only two operations the sidecar's turn-socket mode -(`socket.ts`'s `buildTurnRpc`) will forward over it are submitting the -artifact and asking a can't-complete question (`radial artifact submit`, -`radial message post`) — both proxied through the daemon's own already -authenticated `CredentialClient`, which runs on the host, not in the -container. `ANTHROPIC_API_KEY` is the one secret that *does* enter the -container (`claude` needs it directly), scoped to that turn's env only. +`turnTransport: "tcp"` is a local-development escape hatch. It binds an ephemeral, token-gated +server on all host interfaces so Docker can reach it; firewall it, never expose it to a LAN, and do +not use it for production deployment. diff --git a/packages/daemon/src/bundle-writer.ts b/packages/daemon/src/bundle-writer.ts index c5be6cd..6259b76 100644 --- a/packages/daemon/src/bundle-writer.ts +++ b/packages/daemon/src/bundle-writer.ts @@ -1,41 +1,8 @@ import { spawn } from 'node:child_process' -import { constants as fsConstants } from 'node:fs' -import { mkdir, open, writeFile } from 'node:fs/promises' -import { dirname, join } from 'node:path' +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' import type { PlanTurnBundle } from '@radial/core' -/** Cap on a snapshotted bundle: a ranged bundle of one turn's commits is small; this bounds a - * hostile/broken container from handing the daemon a huge file. */ -export const MAX_BUNDLE_BYTES = 64 * 1024 * 1024 - -/** - * Snapshot the container-written bundle into a daemon-owned path the container CANNOT touch, so the - * bytes the finish step verifies are exactly the bytes captured at submit time — a later container - * mutation is irrelevant. Hardened against a hostile /export: `O_NOFOLLOW` rejects a symlink at the - * path, `O_NONBLOCK` avoids blocking on a FIFO, and the fstat'd FD must be a regular file within the - * size cap. Returns nothing; throws (rejecting the submit) on anything that isn't a plain file. - */ -export async function snapshotBundleFile(source: string, dest: string, maxBytes = MAX_BUNDLE_BYTES): Promise { - let handle - try { - handle = await open(source, fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW | fsConstants.O_NONBLOCK) - } catch (error) { - // ELOOP (symlink), ENXIO/EWOULDBLOCK (some FIFO cases), ENOENT, etc. — all a rejected submit. - throw new Error(`bundle at ${source} is not a readable regular file: ${error instanceof Error ? error.message : String(error)}`) - } - try { - const stat = await handle.stat() - if (!stat.isFile()) throw new Error(`bundle at ${source} is not a regular file (symlink/FIFO/device rejected)`) - if (stat.size > maxBytes) throw new Error(`bundle at ${source} is ${stat.size} bytes, exceeds the ${maxBytes}-byte cap`) - const bytes = await handle.readFile() - if (bytes.length > maxBytes) throw new Error(`bundle at ${source} exceeds the ${maxBytes}-byte cap`) - await mkdir(dirname(dest), { recursive: true }) - await writeFile(dest, bytes) - } finally { - await handle.close() - } -} - export interface WrittenBundle { dir: string bundleJsonPath: string @@ -93,8 +60,8 @@ export function assertGitUrlAllowed(gitUrl: string, allowedSchemes: string[]): v throw new Error(`git url scheme "${scheme}" is not allowed (allowed: ${allowedSchemes.join(', ')})`) } // A member-authored project.gitUrl is untrusted: a URL carrying userinfo (`token@` or - // `user:pass@`) would embed a credential into the checkout/finish `.git/config`. Reject it — the - // daemon supplies credentials out-of-band (finish.ts's inline helper), never via the URL. + // `user:pass@`) would embed a credential into the checkout `.git/config`. Reject it — the + // daemon supplies credentials out-of-band, never via the URL. if (url.username !== '' || url.password !== '') { throw new Error('git url must not embed credentials (userinfo)') } @@ -153,6 +120,34 @@ export function scrubbedGitEnv(env: Record): Record< return scrubbed } +interface GitCheckoutEnv { + env: Record + authenticated: boolean +} + +/** Adds an env-only Git credential helper for a validated github.com HTTPS URL. The helper also + * checks the credential request's host, so redirects cannot make it disclose the token elsewhere. + * The token is never placed in a URL, argv, or on-disk git configuration. */ +function gitEnvForCheckout(gitUrl: string, token?: string): GitCheckoutEnv { + const env = scrubbedGitEnv(process.env) + if (!token) return { env, authenticated: false } + const url = new URL(gitUrl) + if (url.protocol !== 'https:' || url.hostname.toLowerCase() !== 'github.com') { + return { env, authenticated: false } + } + env.GH_TOKEN = token + env.GIT_CONFIG_COUNT = '1' + env.GIT_CONFIG_KEY_0 = 'credential.helper' + env.GIT_CONFIG_VALUE_0 = + '!f() { test "$1" = get || exit 0; host=; while IFS="=" read -r key value; do test "$key" = host && host="$value"; done; test "$host" = github.com || exit 0; printf "username=x-access-token\\npassword=%s\\n" "$GH_TOKEN"; }; f' + return { env, authenticated: true } +} + +function checkoutError(operation: string, result: { code: number; stderr: string }, token?: string): Error { + const stderr = token ? result.stderr.split(token).join('[redacted]') : result.stderr + return new Error(`${operation} failed (exit ${result.code}): ${stderr}`) +} + /** * Shallow, single-branch, no-tags, no-submodules clone of an allowlisted-scheme git URL, with * hooks and credential helpers disabled so a malicious repo can't execute code or exfiltrate @@ -164,18 +159,19 @@ export async function checkoutRepo(input: { dest: string allowedSchemes?: string[] exec?: GitExec + githubToken?: string }): Promise<{ path: string; commit: string }> { const allowedSchemes = input.allowedSchemes ?? ['https'] assertGitUrlAllowed(input.gitUrl, allowedSchemes) const exec = input.exec ?? defaultGitExec() - const env = scrubbedGitEnv(process.env) + const checkoutEnv = gitEnvForCheckout(input.gitUrl, input.githubToken) + const { env } = checkoutEnv const clone = await exec( [ 'git', '-c', 'core.hooksPath=/dev/null', - '-c', - 'credential.helper=', + ...(checkoutEnv.authenticated ? [] : ['-c', 'credential.helper=']), 'clone', '--depth', '1', @@ -188,9 +184,9 @@ export async function checkoutRepo(input: { ], { env }, ) - if (clone.code !== 0) throw new Error(`git clone failed (exit ${clone.code}): ${clone.stderr}`) + if (clone.code !== 0) throw checkoutError('git clone', clone, input.githubToken) const rev = await exec(['git', '-C', input.dest, 'rev-parse', 'HEAD'], { env }) - if (rev.code !== 0) throw new Error(`git rev-parse failed (exit ${rev.code}): ${rev.stderr}`) + if (rev.code !== 0) throw checkoutError('git rev-parse', rev, input.githubToken) return { path: input.dest, commit: rev.stdout.trim() } } @@ -209,6 +205,7 @@ export async function checkoutCommit(input: { dest: string allowedSchemes?: string[] exec?: GitExec + githubToken?: string }): Promise<{ path: string; commit: string }> { const allowedSchemes = input.allowedSchemes ?? ['https'] assertGitUrlAllowed(input.gitUrl, allowedSchemes) @@ -219,9 +216,10 @@ export async function checkoutCommit(input: { throw new Error(`commit "${input.commit}" is not a valid git object name`) } const exec = input.exec ?? defaultGitExec() - const env = scrubbedGitEnv(process.env) + const checkoutEnv = gitEnvForCheckout(input.gitUrl, input.githubToken) + const { env } = checkoutEnv const init = await exec(['git', 'init', input.dest], { env }) - if (init.code !== 0) throw new Error(`git init failed (exit ${init.code}): ${init.stderr}`) + if (init.code !== 0) throw checkoutError('git init', init, input.githubToken) const fetch = await exec( [ 'git', @@ -229,8 +227,7 @@ export async function checkoutCommit(input: { input.dest, '-c', 'core.hooksPath=/dev/null', - '-c', - 'credential.helper=', + ...(checkoutEnv.authenticated ? [] : ['-c', 'credential.helper=']), 'fetch', '--depth', '1', @@ -240,11 +237,11 @@ export async function checkoutCommit(input: { ], { env }, ) - if (fetch.code !== 0) throw new Error(`git fetch ${input.commit} failed (exit ${fetch.code}): ${fetch.stderr}`) + if (fetch.code !== 0) throw checkoutError(`git fetch ${input.commit}`, fetch, input.githubToken) const checkout = await exec( ['git', '-C', input.dest, '-c', 'core.hooksPath=/dev/null', 'checkout', '--detach', 'FETCH_HEAD'], { env }, ) - if (checkout.code !== 0) throw new Error(`git checkout FETCH_HEAD failed (exit ${checkout.code}): ${checkout.stderr}`) + if (checkout.code !== 0) throw checkoutError('git checkout FETCH_HEAD', checkout, input.githubToken) return { path: input.dest, commit: input.commit } } diff --git a/packages/daemon/src/check-runner.ts b/packages/daemon/src/check-runner.ts index afef299..b3d70c7 100644 --- a/packages/daemon/src/check-runner.ts +++ b/packages/daemon/src/check-runner.ts @@ -101,6 +101,8 @@ export interface CheckRunInput { /** Container memory limit (docker `--memory`). Floored at `4g` when unset — a check must never run * with unbounded memory, same rule as a turn. */ memory?: string + /** Optional daemon-owned GitHub credential for a private, validated github.com checkout. */ + githubToken?: string } export interface CheckRunDeps { @@ -110,6 +112,7 @@ export interface CheckRunDeps { commit: string dest: string allowedSchemes: string[] + githubToken?: string /** Aborted when the whole-run timeout fires so a stalled checkout can be torn down. */ signal?: AbortSignal }) => Promise<{ path: string; commit: string }> @@ -208,6 +211,7 @@ export async function runCheckRun(input: CheckRunInput, deps: CheckRunDeps): Pro commit: input.commit, dest: workDir, allowedSchemes: input.allowedSchemes, + ...(input.githubToken ? { githubToken: input.githubToken } : {}), signal: controller.signal, }).then( () => 'ok' as const, @@ -253,7 +257,6 @@ export async function runCheckRun(input: CheckRunInput, deps: CheckRunDeps): Pro // No network at all: a check needs none (the checkout already happened daemon-side) and must // not be able to phone home. The check image must already contain the project toolchain. // (A future `checkEgress` config seam would relax this; unimplemented for now.) - network: 'none', workdir: '/work', timeoutMs: input.timeoutMs, tmpfs: ['/tmp', '/home/radial'], diff --git a/packages/daemon/src/cli.ts b/packages/daemon/src/cli.ts index 923f4b1..dc17b58 100644 --- a/packages/daemon/src/cli.ts +++ b/packages/daemon/src/cli.ts @@ -27,9 +27,9 @@ import { CheckLedger } from './check-ledger.js' import { runCheckRun, type CheckRunInput } from './check-runner.js' import { DockerRunner, type ContainerRunner } from './container.js' import { TurnDispatcher } from './dispatch.js' -import { createEgressManager, type EgressManager } from './egress.js' -import { FinishPump } from './finish.js' import { GitHubForge, type ForgeAdapter } from './forge.js' +import { resolveGitHubToken } from './github-auth.js' +import { ensureGitHubAuth } from './github-auth.js' import { ClaudeCodeHarness } from './harness.js' import { initializeAgent } from './init.js' import { TurnLedger } from './ledger.js' @@ -130,15 +130,9 @@ export interface ResolvedRunConfig { retryBound: number mergePollIntervalMs: number mergePollBackoffMaxMs: number - /** Also doubles as the egress manager's internal-network name when `egress` is true. */ network?: string /** `unix` (default) or the dev-only `tcp` escape hatch — see `DaemonRunConfig.turnTransport`. */ turnTransport: 'unix' | 'tcp' - /** Egress-enforcement default is ON (design §13) — see `DaemonRunConfig.egress`. */ - egress: boolean - egressNetwork: string - proxyImage: string - allowlist: string[] gitSchemes: string[] /** Container memory limit (docker `--memory` syntax, e.g. `4g`). No default at this layer — * `turn.ts` supplies the `4g` floor when unset (a turn must never run unbounded). */ @@ -150,17 +144,11 @@ export interface ResolvedRunConfig { checkImage: string checkCooldownMs: number checkRetryBound: number - /** Forge integration (design §10): the adapter is built only when this is set AND `GITHUB_TOKEN` - * is present in the daemon's environment. */ + /** Forge observation (design §10): the adapter is built only when this is set and operator + * GitHub authentication is available. */ forge?: { kind: 'github' } - /** Implementation finish-pump concurrency (push/PR/record-write). */ - finishConcurrency: number } -const DEFAULT_INTERNAL_NETWORK = 'radial-internal' -const DEFAULT_EGRESS_NETWORK = 'radial-egress' -const DEFAULT_PROXY_IMAGE = 'radial-proxy:latest' -const DEFAULT_PROXY_NAME = 'radial-proxy' /** A default under the operator's atproto state directory, falling back to a project-local * directory when there is no resolvable data dir (e.g. no $HOME in a minimal environment). */ @@ -172,28 +160,10 @@ function defaultRunStateDir(): string { } } -/** `config.js` only validates the raw "run" block; the daemon itself applies runtime defaults. - * Egress is enforced by default (design §13): `network` defaults to `radial-internal` (the name - * the egress manager creates as a `--internal` docker network for turn containers) whenever egress - * is on, so an operator who sets no knobs at all still gets the enforced sandbox. Exported (not - * just for `runCommand`) so its defaults — in particular that `egress` is `true` unless the - * operator explicitly sets `run.egress: false` — are directly unit-testable without touching - * Docker; see cli.test.mjs. */ +/** `config.js` validates the raw `run` block; the daemon applies runtime defaults. */ export function resolveRunConfig(run: DaemonRunConfig): ResolvedRunConfig { - const egress = run.egress ?? true const turnTransport = run.turnTransport ?? 'unix' - // Fail closed: tcp reaches the host via host.docker.internal, which an internal egress network - // blocks by design — egress enforcement is impossible in this mode, so it is only ever allowed - // alongside an *explicit* `"egress": false` (never the default-true egress, and never merely - // "egress happens to resolve false" for some other reason — the raw config must say so itself). - if (turnTransport === 'tcp' && run.egress !== false) { - throw new Error( - 'turn transport "tcp" requires "egress": false to be set explicitly. TCP transport reaches ' + - 'the host via host.docker.internal, which an internal egress network blocks — so egress ' + - 'enforcement is impossible in this mode. It is a dev-only configuration (macOS/Docker ' + - "Desktop, where bind-mounted unix sockets don't work).", - ) - } + if (run.network?.toLowerCase() === 'host' || /^container:/i.test(run.network ?? '')) throw new Error('run.network must not use host or container networking') const mergePollInterval = Math.max(1, run.mergePollIntervalMs ?? 60_000) return { spaces: run.spaces, @@ -208,16 +178,8 @@ export function resolveRunConfig(run: DaemonRunConfig): ResolvedRunConfig { // never yield an uncapped first delay (the poller also caps its first step defensively). mergePollIntervalMs: mergePollInterval, mergePollBackoffMaxMs: Math.max(mergePollInterval, run.mergePollBackoffMaxMs ?? 30 * 60_000), - ...(run.network !== undefined - ? { network: run.network } - : egress - ? { network: DEFAULT_INTERNAL_NETWORK } - : {}), + ...(run.network !== undefined ? { network: run.network } : {}), turnTransport, - egress, - egressNetwork: run.egressNetwork ?? DEFAULT_EGRESS_NETWORK, - proxyImage: run.proxyImage ?? DEFAULT_PROXY_IMAGE, - allowlist: run.allowlist ?? ['api.anthropic.com'], gitSchemes: run.gitSchemes ?? ['https'], ...(run.memory !== undefined ? { memory: run.memory } : {}), checkConcurrency: run.checkConcurrency ?? 1, @@ -226,7 +188,6 @@ export function resolveRunConfig(run: DaemonRunConfig): ResolvedRunConfig { checkCooldownMs: run.checkCooldownMs ?? 300_000, checkRetryBound: run.checkRetryBound ?? 3, ...(run.forge !== undefined ? { forge: run.forge } : {}), - finishConcurrency: run.finishConcurrency ?? 2, } } @@ -272,14 +233,8 @@ async function turnResetCommand(args: string[]): Promise { cooldownMs: run.cooldownMs, }) try { - // If the request gave up while a finish was still pending, restore it to `finishing` (its - // submitted work + runDir snapshot are preserved) rather than discarding the job (finding 8). - if (ledger.reopenFinishing(requestUri)) { - console.log(`restored ${requestUri} to finishing (finish job + runDir preserved)`) - } else { - ledger.reset(requestUri) - console.log(`reset ${requestUri}`) - } + ledger.reset(requestUri) + console.log(`reset ${requestUri}`) } finally { ledger.close() } @@ -334,13 +289,6 @@ export async function reconcileOrphans(runner: ContainerRunner, ledger: TurnLedg await reconcileOrphan(runner, row.containerLabel, row.requestUri) ledger.markCrashed(row.requestUri) } - // A `finishing` row already submitted; its container may still be alive from the prior process. - // Kill it (best-effort) so it can't keep running, but KEEP the row: the finish pump resumes it - // from the durable snapshot (finding 3b). Never markCrashed — that would discard the handoff. - for (const row of ledger.finishing()) { - const label = row.containerLabel ?? turnContainerLabel(row.requestUri, row.requestCid) - await reconcileOrphan(runner, label, row.requestUri) - } } /** Startup orphan reconciliation for check runs — the exact parallel of `reconcileOrphans`: any @@ -362,14 +310,14 @@ async function runCommand(args: string[]): Promise { console.log( `radiald starting: ${run.spaces.length} space(s), interval ${interval}ms, concurrency ${run.concurrency}, ` + - `egress ${run.egress ? 'on' : 'off'}\n config: ${configPath}\n state: ${run.stateDir}`, + `open egress\n config: ${configPath}\n state: ${run.stateDir}`, ) for (const spaceUri of run.spaces) console.log(` polling ${spaceUri}`) if (run.turnTransport === 'tcp') { console.warn( '⚠️ turn transport = tcp (dev mode): turns reach the daemon over TCP on host.docker.internal, ' + - 'egress is NOT enforced. Do not use in production.', + 'this is a development transport; do not expose the daemon socket.', ) } @@ -401,57 +349,17 @@ async function runCommand(args: string[]): Promise { await reconcileOrphans(runner, ledger) await reconcileCheckOrphans(runner, checkLedger) - // Egress enforcement (design §13, "enforced NOW, no bridge fallback"): on by default (opt out - // with `run.egress: false` in radial.json). There are only two allowed outcomes: egress is - // enforced (the proxy + internal network come up), or the operator explicitly opted out. There - // is no third, silent "continue with open egress" state — if `run.egress` is true and the proxy - // fails to start (Docker unreachable, a network-create error, ...), the daemon refuses to start - // rather than ever running a turn with unrestricted egress by accident. Only `ledger` has been - // opened by this point (see the shape of this function below), so it alone needs closing before - // the error propagates out of `runCommand` to `main`'s top-level catch. - let egressManager: EgressManager | undefined - let dispatcherNetwork = run.network - let httpsProxy: string | undefined - if (run.egress) { - const manager = createEgressManager({ - internalNetwork: run.network ?? DEFAULT_INTERNAL_NETWORK, - egressNetwork: run.egressNetwork, - proxyImage: run.proxyImage, - proxyName: DEFAULT_PROXY_NAME, - allowlist: run.allowlist, - }) - try { - const plan = await manager.start() - egressManager = manager - dispatcherNetwork = plan.network - httpsProxy = plan.httpsProxy - console.log(`egress proxy ready: network ${plan.network}, proxy ${plan.httpsProxy}`) - } catch (error) { - ledger.close() - checkLedger.close() - const cause = error instanceof Error ? error.message : String(error) - throw new Error( - `egress enforcement is enabled but the proxy/network could not be started (${cause}); ` + - `refusing to run turns with unrestricted egress. Fix Docker/networking, or set ` + - `"run": { "egress": false } to explicitly opt out.`, - ) - } - } - // Forge adapter (design §10): built only when `run.forge` is configured AND a daemon-side - // GITHUB_TOKEN is present (the credential never enters a container — §13). It drives the - // implementation finish pump (push/PR), the v2 predecessor-PR check in dispatch, and merge - // observation. Absent forge → implementation turns can't finish and merges aren't polled. - const githubToken = process.env.GITHUB_TOKEN + // Forge observation is available when GitHub is configured and daemon authentication resolves. + const githubToken = await resolveGitHubToken() let forge: ForgeAdapter | undefined if (run.forge?.kind === 'github') { if (githubToken) { forge = new GitHubForge({ fetch, token: githubToken }) - console.log('forge: github adapter enabled (finish + merge observation)') + console.log('forge: github observation adapter enabled') } else { console.warn( - 'run.forge is "github" but GITHUB_TOKEN is not set in the environment; implementation ' + - 'finishing and merge observation are disabled until it is provided.', + 'run.forge is "github" but GitHub authentication is unavailable; merge observation is disabled.', ) } } @@ -462,7 +370,7 @@ async function runCommand(args: string[]): Promise { runner, harness, ...(process.env.ANTHROPIC_API_KEY ? { anthropicApiKey: process.env.ANTHROPIC_API_KEY } : {}), - ...(httpsProxy ? { httpsProxy } : {}), + ...(githubToken ? { githubToken } : {}), log: (message) => console.log(message), }) @@ -472,39 +380,21 @@ async function runCommand(args: string[]): Promise { runDirFor: (requestUri) => turnRunDir(run.stateDir, requestUri), image: run.image, timeoutMs: run.timeoutMs, - ...(dispatcherNetwork !== undefined ? { network: dispatcherNetwork } : {}), + ...(run.network !== undefined ? { network: run.network } : {}), allowedSchemes: run.gitSchemes, ...(run.memory !== undefined ? { memory: run.memory } : {}), turnTransport: run.turnTransport, ...(forge ? { forge } : {}), + implementationEnabled: !!githubToken, runTurn: boundRunTurn, log: (message) => console.log(message), }) - // The implementation finish pump (design §10): drives `finishing` ledger rows (submitted turns) - // to written artifact records — fresh daemon-owned clone, push the pushed sha, open/adopt the PR, - // write the record last. Its own concurrency budget. Requires the forge + token; startup - // reconciliation of `finishing` orphans is just its first pump (every finishing row is picked up). - const finishPump = - forge && githubToken - ? new FinishPump({ - ledger, - concurrency: run.finishConcurrency, - clientFor: (did) => actors.byDid.get(did)?.[0]?.client, - runDirFor: (requestUri) => turnRunDir(run.stateDir, requestUri), - githubToken, - forge, - now: () => Date.now(), - // Hold off finishing a request whose container is still in flight in the turn dispatcher. - isInFlight: (requestUri) => dispatcher.hasInFlight(requestUri), - log: (message) => console.log(message), - }) - : undefined - - // The check runner deliberately gets NONE of the turn's secrets/proxy/network: its container runs - // untrusted project code with `--network none` and an empty env (see check-runner.ts). It runs on - // its own concurrency budget and ledger so checks never contend with turn dispatch. - const boundRunCheckRun = (input: CheckRunInput) => runCheckRun(input, { runner }) + + // Checks have an empty container environment. A GitHub token, when available, is used solely by + // the daemon-host checkout helper for private repositories and is never added to the container. + const boundRunCheckRun = (input: CheckRunInput) => + runCheckRun(githubToken ? { ...input, githubToken } : input, { runner }) const checkDispatcher = new CheckDispatcher({ ledger: checkLedger, concurrency: run.checkConcurrency, @@ -550,7 +440,6 @@ async function runCommand(args: string[]): Promise { dispatcher, checkDispatcher, ...(mergePoller ? { mergePoller } : {}), - ...(finishPump ? { finishPump } : {}), actors, intervalMs: interval, signal: controller.signal, @@ -566,9 +455,7 @@ async function runCommand(args: string[]): Promise { } await dispatcher.drain().catch(() => {}) await checkDispatcher.drain().catch(() => {}) - await finishPump?.drain().catch(() => {}) for (const { runtime } of spaces) runtime.close() - await egressManager?.stop().catch(() => {}) ledger.close() checkLedger.close() } @@ -625,6 +512,7 @@ export async function main(args = argv.slice(2)): Promise { artifactTypes: values(args, '--artifact-type'), }) console.log(JSON.stringify(result)) + await ensureGitHubAuth({ skip: args.includes('--skip-github-auth') }) return } const path = await findConfigPath(values(args, '--config')[0]) @@ -655,6 +543,7 @@ export async function main(args = argv.slice(2)): Promise { } } if (failed) process.exitCode = 1 + else await ensureGitHubAuth({ skip: args.includes('--skip-github-auth') }) } const entry = argv[1] diff --git a/packages/daemon/src/config.ts b/packages/daemon/src/config.ts index d6ea6bd..332302d 100644 --- a/packages/daemon/src/config.ts +++ b/packages/daemon/src/config.ts @@ -27,7 +27,7 @@ export interface RadialConfig { /** * The daemon's run-path settings: which spaces to serve turns for, and the sandboxing knobs for - * the container/proxy it spawns. Only parsed and validated here — `run.concurrency` and friends + * the containers it spawns. Only parsed and validated here — `run.concurrency` and friends * get their runtime defaults applied by the daemon (D2), not by config parsing. */ export interface DaemonRunConfig { @@ -46,17 +46,8 @@ export interface DaemonRunConfig { /** Turn-socket transport: `unix` (default) is the production/Linux path — a bind-mounted socket * file. `tcp` is a dev-only escape hatch for macOS/Docker Desktop, where a bind-mounted AF_UNIX * socket is visible inside the VM but `connect()` gets ECONNREFUSED. See `resolveRunConfig` in - * cli.ts for the fail-closed rule that requires `egress: false` alongside it. */ + * cli.ts applies this transport setting at runtime. */ turnTransport?: 'unix' | 'tcp' - /** Egress-enforcement toggle (design §13, "enforced NOW"): defaults to `true` when omitted, so - * `radiald run` is egress-enforced (internal docker network + allowlisting proxy container) by - * default. Set to `false` to opt out — e.g. local/dev without Docker networking support — which - * degrades to the pre-existing behavior of running turn containers on `network` (or the default - * docker bridge, unrestricted, if `network` is also unset) with no proxy. */ - egress?: boolean - egressNetwork?: string - proxyImage?: string - allowlist?: string[] gitSchemes?: string[] /** Container memory limit (docker `--memory` syntax, e.g. `4g`/`512m`). Unset means "use the * daemon's default floor" (see `turn.ts`, which floors at `4g` — a turn must never run @@ -72,12 +63,8 @@ export interface DaemonRunConfig { checkCooldownMs?: number checkRetryBound?: number /** Forge integration (design §10). `{ kind: 'github' }` turns on the GitHub adapter for - * implementation-turn finishing (push/PR) and merge observation — active only when a daemon-side - * `GITHUB_TOKEN` is also present. Unset means no forge (implementation turns can't finish). */ + * implementation observation and merge polling. */ forge?: { kind: 'github' } - /** Concurrency budget for the implementation finish pump (push/PR/record-write), separate from - * turn and check dispatch. Runtime default 2. */ - finishConcurrency?: number } const object = (value: unknown): value is Record => @@ -173,10 +160,6 @@ export function parseRunConfig(value: unknown): DaemonRunConfig { const mergePollBackoffMaxMs = num(value.mergePollBackoffMaxMs, 'run.mergePollBackoffMaxMs') const network = text(value.network, 'run.network') const turnTransport = turnTransportValue(value.turnTransport, 'run.turnTransport') - const egress = bool(value.egress, 'run.egress') - const egressNetwork = text(value.egressNetwork, 'run.egressNetwork') - const proxyImage = text(value.proxyImage, 'run.proxyImage') - const allowlist = strings(value.allowlist, 'run.allowlist') const gitSchemes = strings(value.gitSchemes, 'run.gitSchemes') const memory = text(value.memory, 'run.memory') const checkConcurrency = num(value.checkConcurrency, 'run.checkConcurrency') @@ -185,7 +168,6 @@ export function parseRunConfig(value: unknown): DaemonRunConfig { const checkCooldownMs = num(value.checkCooldownMs, 'run.checkCooldownMs') const checkRetryBound = num(value.checkRetryBound, 'run.checkRetryBound') const forge = forgeValue(value.forge, 'run.forge') - const finishConcurrency = num(value.finishConcurrency, 'run.finishConcurrency') return { spaces, ...(stateDir !== undefined ? { stateDir } : {}), @@ -198,10 +180,6 @@ export function parseRunConfig(value: unknown): DaemonRunConfig { ...(mergePollBackoffMaxMs !== undefined ? { mergePollBackoffMaxMs } : {}), ...(network !== undefined ? { network } : {}), ...(turnTransport !== undefined ? { turnTransport } : {}), - ...(egress !== undefined ? { egress } : {}), - ...(egressNetwork !== undefined ? { egressNetwork } : {}), - ...(proxyImage !== undefined ? { proxyImage } : {}), - ...(allowlist !== undefined ? { allowlist } : {}), ...(gitSchemes !== undefined ? { gitSchemes } : {}), ...(memory !== undefined ? { memory } : {}), ...(checkConcurrency !== undefined ? { checkConcurrency } : {}), @@ -210,7 +188,6 @@ export function parseRunConfig(value: unknown): DaemonRunConfig { ...(checkCooldownMs !== undefined ? { checkCooldownMs } : {}), ...(checkRetryBound !== undefined ? { checkRetryBound } : {}), ...(forge !== undefined ? { forge } : {}), - ...(finishConcurrency !== undefined ? { finishConcurrency } : {}), } } diff --git a/packages/daemon/src/container.ts b/packages/daemon/src/container.ts index ee552ee..b723b7c 100644 --- a/packages/daemon/src/container.ts +++ b/packages/daemon/src/container.ts @@ -45,6 +45,7 @@ const DEFAULT_USER = '1000:1000' * timed-out run can be killed by name (docker does not support killing by label directly). */ export function dockerRunArgs(spec: ContainerSpec): string[] { + if (spec.network?.toLowerCase() === 'host' || /^container:/i.test(spec.network ?? '')) throw new Error('host and container networks are not permitted for Radial containers') const args: string[] = ['run', '--rm', '--name', spec.label] args.push('--cap-drop=ALL') args.push('--security-opt', 'no-new-privileges') @@ -60,16 +61,14 @@ export function dockerRunArgs(spec: ContainerSpec): string[] { for (const mount of spec.mounts) { args.push('-v', `${mount.source}:${mount.target}${mount.readOnly ? ':ro' : ''}`) } - for (const [key, value] of Object.entries(spec.env)) { - args.push('-e', `${key}=${value}`) - } + for (const key of Object.keys(spec.env)) args.push('-e', key) args.push(spec.image, ...spec.argv) return args } -function execDocker(args: string[]): Promise<{ code: number; stdout: string; stderr: string }> { +function execDocker(args: string[], env?: Record): Promise<{ code: number; stdout: string; stderr: string }> { return new Promise((resolvePromise, reject) => { - const child = spawn('docker', args) + const child = spawn('docker', args, env ? { env } : undefined) const decoder = new TextDecoder() let stdout = '' let stderr = '' @@ -93,7 +92,10 @@ export class DockerRunner implements ContainerRunner { async run(spec: ContainerSpec): Promise { const args = dockerRunArgs(spec) return new Promise((resolvePromise, reject) => { - const child = spawn('docker', args) + const env: Record = {} + for (const [key, value] of Object.entries(process.env)) if (value !== undefined) env[key] = value + Object.assign(env, spec.env) + const child = spawn('docker', args, { env }) let settled = false const timer = setTimeout(() => { if (settled) return @@ -145,7 +147,7 @@ export class DockerRunner implements ContainerRunner { } /** - * In-memory stand-in for a container runner: `run` simply invokes the injected function (D2's + * In-memory stand-in for a container runner: `run` simply invokes the injected function (turn * tests play a fake harness against `spec.env.RADIAL_SIDECAR_SOCKET`). `listByLabel`/`kill` track * started instances so orphan-reconciliation logic can be exercised without Docker. */ diff --git a/packages/daemon/src/dispatch.ts b/packages/daemon/src/dispatch.ts index 4835dc4..8df7447 100644 --- a/packages/daemon/src/dispatch.ts +++ b/packages/daemon/src/dispatch.ts @@ -14,8 +14,8 @@ import { } from '@radial/core' import type { ActorRegistry, LoadedActor } from './actors.js' import { parseGitHubPullUrl, parseGitHubRepo, type ForgeAdapter } from './forge.js' -import type { FinishJob, TurnLedger } from './ledger.js' -import { implBranchName, type PendingSubmission } from './turn-socket.js' +import type { TurnLedger } from './ledger.js' +import { implBranchName } from './turn-socket.js' import { IMPLEMENTATION_TYPE, turnContainerLabel, type TurnInput, type TurnResult } from './turn.js' /** A resolved v2 predecessor for an implementation request: the exact prior artifact plus its @@ -276,16 +276,16 @@ export interface DispatcherDeps { /** Optional forge adapter: used to VALIDATE a v2 predecessor's PR (open, head ref/repo + base all * matching this project) before reusing its branch. Without it, a v2 turn always opens a fresh * `radial/impl-` branch + new PR. */ - forge?: Pick + forge?: Pick + /** Disable implementation launches when daemon GitHub possession is unavailable. */ + implementationEnabled?: boolean log?: (message: string) => void } /** * Bounded-concurrency, tracked dispatcher: `pump` launches eligible turns in the background and * returns immediately. Every launched promise is tracked (by request uri) and every settlement - * updates the ledger before the promise is dropped — a turn's fate is never lost. Plan-style turns - * settle to fulfilled/awaiting_input/crashed; an `implementation` turn's socket submit persists a - * `finishing` row itself (via onSubmission), so its `submitted` settlement leaves the ledger alone. + * updates the ledger before the promise is dropped — a turn's fate is never lost. */ export class TurnDispatcher { readonly #deps: DispatcherDeps @@ -300,9 +300,7 @@ export class TurnDispatcher { return this.#inFlight.size } - /** True while a container for `requestUri` is still tracked here (dispatched, not yet settled). - * The finish pump consults this so it never races a still-running container that already submitted - * (its `finishing` row exists, but the container hasn't exited — finding 3). */ + /** True while a container for `requestUri` is still tracked here (dispatched, not yet settled). */ hasInFlight(requestUri: string): boolean { return this.#inFlight.has(requestUri) } @@ -333,6 +331,10 @@ export class TurnDispatcher { if (this.#inFlight.size >= this.#deps.concurrency) break const uri = item.request.uri if (this.#inFlight.has(uri)) continue + if (item.artifactType.name === IMPLEMENTATION_TYPE && this.#deps.implementationEnabled === false) { + this.#deps.log?.(`not dispatching implementation ${uri}: GitHub authentication is unavailable`) + continue + } const goal = item.request.value.goal if (!goal) continue // goal-scoped guard; defensive against a malformed record @@ -363,8 +365,7 @@ export class TurnDispatcher { } } - /** Prepare and run one turn, then settle the ledger. Impl turns resolve their v2 branch/PR first - * and wire the deferred-finish onSubmission; plan turns run straight through. */ + /** Prepare and run one turn, then settle the ledger. */ async #launch( item: Dispatchable, ctx: { uri: string; cid: string; goal: StrongRef; runDir: string; label: string }, @@ -373,11 +374,11 @@ export class TurnDispatcher { const isImpl = item.artifactType.name === IMPLEMENTATION_TYPE let checkoutRef: string | undefined - let onSubmission: ((submission: PendingSubmission, baseCommit: string, bundlePath: string) => void) | undefined + let branch: string | undefined if (isImpl) { const base = item.bundle.project.defaultBranch const gitUrl = item.bundle.project.gitUrl - let branch = implBranchName(uri, cid) + branch = implBranchName(uri, cid) const predecessor = item.implementation?.predecessor // v2 branch reuse is deliberately narrow (finding 6): only reuse the predecessor's branch when // EVERY guard holds — its name is in the daemon's own `radial/impl-` namespace, its PR lives in @@ -390,14 +391,9 @@ export class TurnDispatcher { const projectRepo = parseGitHubRepo(gitUrl) const projectFullName = `${projectRepo.owner}/${projectRepo.repo}` const { repo: prRepo, number } = parseGitHubPullUrl(predecessor.prUrl) - if (prRepo.owner === projectRepo.owner && prRepo.repo === projectRepo.repo) { - const pr = await this.#deps.forge.getPullRequest(prRepo, number) - if ( - pr.state === 'open' && - pr.headRef === predecessor.branch && - pr.headRepoFullName === projectFullName && - pr.baseRef === base - ) { + if (prRepo.owner.toLowerCase() === projectRepo.owner.toLowerCase() && prRepo.repo.toLowerCase() === projectRepo.repo.toLowerCase()) { + const pr = await this.#deps.forge.getPullRequestState(predecessor.prUrl) + if (pr.state === 'open' && pr.headRef === predecessor.branch && pr.headRepoFullName.toLowerCase() === projectFullName.toLowerCase() && pr.baseRef === base) { checkoutRef = predecessor.branch branch = predecessor.branch } else { @@ -408,24 +404,6 @@ export class TurnDispatcher { this.#deps.log?.(`could not validate predecessor PR ${predecessor.prUrl} for ${uri}: ${error instanceof Error ? error.message : String(error)}; opening a fresh branch`) } } - const prev = predecessor?.ref - onSubmission = (submission, baseCommit, bundlePath): void => { - const job: FinishJob = { - goal, - actorDid: item.actor.did, - gitUrl, - base, - branch, - baseCommit, - bundlePath, - commit: submission.commit, - body: submission.body, - title: item.bundle.goal.title, - ...(submission.criteria && submission.criteria.length ? { criteria: submission.criteria } : {}), - ...(prev ? { prev } : {}), - } - this.#deps.ledger.markSubmitted(uri, cid, job) - } } const turnInput: TurnInput = { @@ -442,17 +420,13 @@ export class TurnDispatcher { ...(this.#deps.memory !== undefined ? { memory: this.#deps.memory } : {}), ...(this.#deps.turnTransport !== undefined ? { turnTransport: this.#deps.turnTransport } : {}), ...(checkoutRef !== undefined ? { checkoutRef } : {}), - ...(onSubmission ? { onSubmission } : {}), + ...(isImpl && branch !== undefined ? { branch, ...(item.implementation?.predecessor?.ref ? { prev: item.implementation.predecessor.ref } : {}) } : {}), } try { const result = await this.#deps.runTurn(turnInput) if (result.outcome === 'fulfilled') { this.#deps.ledger.markFulfilled(uri, result.acceptedRef as StrongRef) - } else if (result.outcome === 'submitted') { - // The ledger is already in `finishing` (onSubmission ran at submit time); the finish pump - // takes over. Nothing to update here. - this.#deps.log?.(`turn ${uri} submitted an implementation; handed to the finish pump (label ${label})`) } else if (result.outcome === 'awaiting_input') { this.#deps.ledger.markAwaitingInput(uri) } else { diff --git a/packages/daemon/src/egress.ts b/packages/daemon/src/egress.ts deleted file mode 100644 index 045d4ef..0000000 --- a/packages/daemon/src/egress.ts +++ /dev/null @@ -1,177 +0,0 @@ -import { spawn } from 'node:child_process' - -export interface EgressPlan { - network: string - httpsProxy: string -} - -export interface EgressManager { - start(): Promise - stop(): Promise -} - -export type DockerExec = (argv: string[]) => Promise<{ code: number; stdout: string; stderr: string }> - -function defaultExec(argv: string[]): Promise<{ code: number; stdout: string; stderr: string }> { - return new Promise((resolvePromise, reject) => { - const child = spawn('docker', argv) - const decoder = new TextDecoder() - let stdout = '' - let stderr = '' - child.stdout?.on('data', (chunk) => { - stdout += decoder.decode(chunk) - }) - child.stderr?.on('data', (chunk) => { - stderr += decoder.decode(chunk) - }) - child.on('error', reject) - child.on('close', (code) => resolvePromise({ code: code ?? -1, stdout, stderr })) - }) -} - -/** Pure `docker network create` argv builder. `internal` omits the internal network from any - * default route out of the host — the whole point of the internal network the turn container - * runs on (design §13: "internal Docker network + allowlisting proxy, no bridge fallback"). */ -export function networkCreateArgs(name: string, internal: boolean): string[] { - const args = ['network', 'create'] - if (internal) args.push('--internal') - args.push(name) - return args -} - -/** Pure `docker run` argv builder for the `radial-proxy` container: attached to the external - * (non-internal) egress network so it alone can reach the model API, with the allowlist passed - * through as repeated `--allow` CLI args to `docker/proxy.Dockerfile`'s entrypoint (`proxy.js`'s - * standalone CLI, which binds 0.0.0.0:8080 — see proxy.ts). Joining the internal network is a - * second, separate `docker network connect` issued by `start()` below (a single `docker run` - * cannot attach two networks with different `--internal` flags in one shot). `input.internalNetwork` - * is accepted (not embedded in argv) purely so callers don't have to thread it separately when - * building both the run and the following connect. */ -export function proxyRunArgs(input: { - name: string - image: string - internalNetwork: string - egressNetwork: string - allowlist: string[] -}): string[] { - const args = ['run', '-d', '--name', input.name, '--network', input.egressNetwork, '--label', input.name] - args.push(input.image) - // An explicit empty allowlist means deny-all and must be passed through as such: zero `--allow` - // args would be indistinguishable from "no allowlist configured at all", which proxy.ts's CLI - // parser treats as "fall back to the default allowlist" — silently turning deny-all into - // allow-Anthropic. `--allow-none` makes deny-all explicit end to end (see proxy.ts). - if (input.allowlist.length === 0) { - args.push('--allow-none') - } else { - for (const host of input.allowlist) args.push('--allow', host) - } - return args -} - -/** `docker network inspect -f '{{.Internal}}' ` trimmed to a boolean. Used to verify a - * pre-existing network really is internal before trusting it (see `ensureNetwork` below). */ -async function isNetworkInternal(exec: DockerExec, name: string): Promise { - const result = await exec(['network', 'inspect', '-f', '{{.Internal}}', name]) - return result.code === 0 && result.stdout.trim() === 'true' -} - -async function ensureNetwork(exec: DockerExec, name: string, internal: boolean): Promise { - const result = await exec(networkCreateArgs(name, internal)) - if (result.code !== 0 && !/already exists/i.test(result.stderr)) { - throw new Error(`docker network create ${name} failed (exit ${result.code}): ${result.stderr}`) - } - if (result.code !== 0 && internal) { - // "Already exists" for the internal network: a pre-existing bridge network under this name - // would silently give turns unrestricted egress while the daemon still reports enforcement. - // Fail closed unless docker itself confirms the existing network really is internal. - if (!(await isNetworkInternal(exec, name))) { - throw new Error( - `docker network "${name}" already exists but is not internal; refusing to reuse it for ` + - `turn egress enforcement (a non-internal network would give turns unrestricted egress)`, - ) - } - } -} - -/** - * Manages the lifecycle of the daemon-owned `radial-proxy` container that is the only egress path - * out of the turn sandbox (design §13, "enforced NOW"): `start()` ensures the internal (no - * external route) and egress (has external route) docker networks exist, runs the proxy container - * on the egress network, then joins it to the internal network too so a turn container — which is - * only ever attached to the internal network — can reach it by container name. `stop()` tears the - * proxy container down; it never removes the networks (other concurrent daemon runs, or the - * operator, may still be using them). - */ -export function createEgressManager(input: { - internalNetwork: string - egressNetwork: string - proxyImage: string - proxyName: string - allowlist: string[] - exec?: DockerExec - /** Bounded readiness-poll knobs for the post-`docker run` check (see below). Small defaults; - * overridable so tests can run this fast without waiting on real container start latency. */ - readyAttempts?: number - readyDelayMs?: number -}): EgressManager { - const exec = input.exec ?? defaultExec - const readyAttempts = input.readyAttempts ?? 10 - const readyDelayMs = input.readyDelayMs ?? 200 - - /** Polls `docker inspect -f '{{.State.Running}}' ` until it reports `true`, up to a small - * bounded number of attempts. A successful `docker run -d` only means the container was created - * — it may still exit immediately (bad image, crash on boot) or never bind; without this check - * turns would dispatch onto a proxy that isn't actually there and burn retries with no egress at - * all. Throws (fail closed) if the proxy never reports ready. */ - async function waitUntilRunning(name: string): Promise { - for (let attempt = 1; attempt <= readyAttempts; attempt += 1) { - const result = await exec(['inspect', '-f', '{{.State.Running}}', name]) - if (result.code === 0 && result.stdout.trim() === 'true') return - if (attempt < readyAttempts) await new Promise((resolvePromise) => setTimeout(resolvePromise, readyDelayMs)) - } - throw new Error( - `radial-proxy container "${name}" did not report running after ${readyAttempts} check(s); ` + - `refusing to treat egress as enforced`, - ) - } - - return { - async start(): Promise { - await ensureNetwork(exec, input.internalNetwork, true) - await ensureNetwork(exec, input.egressNetwork, false) - - const runResult = await exec( - proxyRunArgs({ - name: input.proxyName, - image: input.proxyImage, - internalNetwork: input.internalNetwork, - egressNetwork: input.egressNetwork, - allowlist: input.allowlist, - }), - ) - if (runResult.code !== 0) { - throw new Error(`docker run (radial-proxy) failed (exit ${runResult.code}): ${runResult.stderr}`) - } - - const connectResult = await exec(['network', 'connect', input.internalNetwork, input.proxyName]) - if (connectResult.code !== 0) { - throw new Error( - `docker network connect ${input.internalNetwork} ${input.proxyName} failed ` + - `(exit ${connectResult.code}): ${connectResult.stderr}`, - ) - } - - await waitUntilRunning(input.proxyName) - - return { network: input.internalNetwork, httpsProxy: `http://${input.proxyName}:8080` } - }, - - async stop(): Promise { - try { - await exec(['rm', '-f', input.proxyName]) - } catch { - // Best effort: the container (or docker itself) may already be gone. - } - }, - } -} diff --git a/packages/daemon/src/finish.ts b/packages/daemon/src/finish.ts deleted file mode 100644 index 3eeddaa..0000000 --- a/packages/daemon/src/finish.ts +++ /dev/null @@ -1,332 +0,0 @@ -import { rm } from 'node:fs/promises' -import { join } from 'node:path' -import { XrpcError } from '@radial/atproto' -import { COLLECTIONS, type ArtifactRecord, type StrongRef } from '@radial/core' -import { assertGitUrlAllowed, defaultGitExec, scrubbedGitEnv, type GitExec } from './bundle-writer.js' -import { compareUrl, parseGitHubRepo, type ForgeAdapter } from './forge.js' -import type { FinishJob, TurnLedger } from './ledger.js' -import { implArtifactRkey, sha256Hex, type TurnRecordWriter } from './turn-socket.js' - -const sameRef = (a?: StrongRef, b?: StrongRef): boolean => - a === undefined || b === undefined ? a === b : a.uri === b.uri && a.cid === b.cid - -/** An existing record at the deterministic rkey may be adopted only if it is the SAME implementation - * this job would write: type, linked commit + branch, prev, and body (by hash) all match. */ -function adoptedRecordMatches(existing: ArtifactRecord, job: FinishJob): boolean { - return ( - existing.type === 'implementation' && - existing.links?.commit === job.commit && - existing.links?.branch === job.branch && - sameRef(existing.prev, job.prev) && - sha256Hex(existing.body) === sha256Hex(job.body) - ) -} - -/** The env var the inline git credential helper reads the token from (see `finishImplementationTurn`). - * The token is supplied to git ONLY through the child-process env for the authed push/fetch — never - * in argv (visible in the process list), never in the remote URL, never persisted in .git/config. */ -const FORGE_TOKEN_ENV = 'RADIAL_FORGE_TOKEN' - -/** - * The CREDENTIAL-FREE remote URL the finish clone talks to (design §13). For a GitHub repo this is - * the plain `https://github.com/owner/repo.git`; the token is injected per-invocation via an inline - * credential helper with the token in the child env only, so it never lands in a git error string, - * the process list, or the clone's .git/config. For a non-GitHub URL (e.g. a local bare repo in - * tests) the URL is used verbatim. - */ -export function plainRemoteUrl(gitUrl: string): string { - try { - const repo = parseGitHubRepo(gitUrl) - return `https://github.com/${repo.owner}/${repo.repo}.git` - } catch { - return gitUrl - } -} - -/** Redact every occurrence of `secret` from `text` (defense in depth before any git output reaches - * an Error message, a log line, or the ledger). No-op for an empty secret. */ -export function scrubSecret(text: string, secret: string): string { - return secret ? text.split(secret).join('***') : text -} - -export interface FinishInput { - requestUri: string - requestCid: string - job: FinishJob - /** The writing actor's record client (resolved from `job.actorDid` by the pump). */ - client: TurnRecordWriter - /** The kept per-turn runDir; its `export/impl.bundle` is what the finish step fetches. */ - runDir: string - /** Daemon-side forge token; supplied to git via the child env only (never argv/URL/config). */ - githubToken: string -} - -export interface FinishDeps { - forge: ForgeAdapter - exec?: GitExec - now?: () => string - log?: (message: string) => void - /** Override the credential-free remote URL derivation (tests point it at a local bare repo). */ - remoteUrl?: (gitUrl: string) => string -} - -export type FinishOutcome = - // The artifact record was written (idempotently). Terminal success. - | { outcome: 'fulfilled'; ref: StrongRef } - // The submitted sha is not in the bundle (a lying/corrupt submission): no push, no record. - | { outcome: 'rejected'; reason: string } - -const SHA = /^[0-9a-f]{40}$/ - -/** - * Drive one implementation submission to a written artifact record — the whole daemon-side finish - * (design §10, §13). Idempotent and crash-resumable: a fresh daemon-owned clone (hooks off, scrubbed - * GIT_* env) is created under the runDir; the base commit is fetched so the ranged bundle's - * prerequisite is present; the bundle FILE (never the container's /work/.git) is fetched; the - * submitted sha is verified present (absent → `rejected`, no push, no record); the sha is pushed to - * its branch (a same-sha re-push is a no-op — "Everything up-to-date"); the remote head is verified - * to equal the sha; the PR is opened or adopted; and the artifact record is written LAST under a - * deterministic rkey (a RecordAlreadyExists collision is adopted, so a resumed finish never - * duplicates the record or the PR). Transient failures (git/network/forge) throw — the pump cools - * down and retries the same job. - */ -export async function finishImplementationTurn(input: FinishInput, deps: FinishDeps): Promise { - const { job } = input - const token = input.githubToken - const exec = deps.exec ?? defaultGitExec() - const now = deps.now ?? (() => new Date().toISOString()) - // Base env for all git calls: no inherited GIT_*/credentials, hooks-off, no terminal prompt. - const baseEnv = scrubbedGitEnv(process.env) - // The token reaches git ONLY here, in the child env of authed calls — never in argv, the remote - // URL, or .git/config. The inline credential helper below reads it from this var. - const authedEnv = { ...baseEnv, [FORGE_TOKEN_ENV]: token } - // Credential-free remote URL (design §13): the token is NOT embedded, so it can't leak into a git - // error string or persist in .git/config. - const remoteUrl = (deps.remoteUrl ?? plainRemoteUrl)(job.gitUrl) - const repo = parseGitHubRepo(job.gitUrl) - - // A member-authored gitUrl is untrusted: reject any embedded credential before it can reach the - // finish clone's config (defense in depth — plainRemoteUrl also strips it for github URLs). - assertGitUrlAllowed(job.gitUrl, ['https']) - if (!SHA.test(job.commit)) return { outcome: 'rejected', reason: `submitted commit "${job.commit}" is not a full 40-hex sha` } - if (!SHA.test(job.baseCommit)) throw new Error(`base commit "${job.baseCommit}" is not a full 40-hex sha`) - - const bundlePath = job.bundlePath - const finishDir = join(input.runDir, 'finish') - - // Start each finish attempt from a clean scratch clone (a prior crashed attempt must not leave a - // half-built repo behind). The runDir itself — and its bundle snapshot — is preserved. - await rm(finishDir, { recursive: true, force: true }) - - // `authed` calls carry the token in the child env AND clear+set the inline credential helper; - // every git error string is scrubbed of the token before it can reach an Error/log/ledger. - const git = async ( - args: string[], - label: string, - options: { authed?: boolean } = {}, - ): Promise<{ stdout: string; stderr: string }> => { - const result = await exec(['git', ...args], { env: options.authed ? authedEnv : baseEnv }) - if (result.code !== 0) { - throw new Error(`${label} failed (exit ${result.code}): ${scrubSecret(result.stderr.trim(), token)}`) - } - return { stdout: result.stdout, stderr: result.stderr } - } - // Network-touching git commands run with hooks and any inherited credential helpers off (the - // daemon's own clone is trusted, but this keeps a stray global gitconfig from redirecting or - // executing anything). - const hardened = ['-c', 'core.hooksPath=/dev/null', '-c', 'credential.helper='] - // Authed calls append an inline credential helper AFTER the reset above: git invokes it (via sh) - // only when the https remote actually needs auth, and it echoes the token from the child env — - // never argv (which is visible in the process list), never the URL, never .git/config. - const credentialHelper = `credential.helper=!f(){ echo username=x-access-token; echo "password=$${FORGE_TOKEN_ENV}"; };f` - const authed = [...hardened, '-c', credentialHelper] - - await git(['init', '-q', finishDir], 'git init') - // The remote URL persisted in .git/config is credential-free. - await git(['-C', finishDir, 'remote', 'add', 'origin', remoteUrl], 'git remote add') - - // PROVENANCE (finding 4): the submitted sha must come from THIS bundle, not be smuggled in from - // origin. First require it to be a declared tip of the bundle (list-heads reads the header, no - // prerequisites needed). A sha that exists at origin but is not a bundle head is rejected here — - // before any origin fetch, so origin can never "provide" the sha. - const heads = await git(['-C', finishDir, 'bundle', 'list-heads', bundlePath], 'git bundle list-heads') - const headShas = new Set( - heads.stdout - .split('\n') - .map((line) => line.trim().split(/\s+/)[0]) - .filter((sha): sha is string => Boolean(sha)), - ) - if (!headShas.has(job.commit)) { - return { outcome: 'rejected', reason: `submitted commit ${job.commit} is not a head of the submitted bundle` } - } - // Fetch ONLY the base prerequisite (a distinct commit from the submitted head), so `git bundle - // verify` can confirm the bundle's pack is self-contained down to the base — this catches a bundle - // whose header lies about a tip it doesn't actually carry the objects for. - await git(['-C', finishDir, ...authed, 'fetch', '--no-tags', 'origin', job.baseCommit], 'git fetch base', { authed: true }) - const verify = await exec(['git', '-C', finishDir, 'bundle', 'verify', bundlePath], { env: baseEnv }) - if (verify.code !== 0) { - return { outcome: 'rejected', reason: `submitted bundle failed verification: ${scrubSecret(verify.stderr.trim(), token)}` } - } - // Import the bundle objects (the FILE — never the agent's /work/.git, whose config/hooks are - // untrusted), then confirm the submitted sha's objects are genuinely present. - await git(['-C', finishDir, ...hardened, 'fetch', '--no-tags', bundlePath], 'git fetch bundle') - const present = await exec(['git', '-C', finishDir, 'rev-parse', '--verify', '--quiet', `${job.commit}^{commit}`], { env: baseEnv }) - if (present.code !== 0) { - return { outcome: 'rejected', reason: `submitted commit ${job.commit} is not present in the bundle` } - } - - // Push the exact sha to its branch. A same-sha re-push (idempotent resume, or a v2 fast-forward) - // is a no-op. `no force` is correct: a fresh branch is created; a v2 revision descends from the - // prior head, so it fast-forwards. - await git( - ['-C', finishDir, ...authed, 'push', 'origin', `${job.commit}:refs/heads/${job.branch}`], - 'git push', - { authed: true }, - ) - - // Verify the remote head is our sha before opening/adopting the PR. - const remoteHead = await deps.forge.getBranchHead(repo, job.branch) - if (remoteHead !== job.commit) { - throw new Error(`remote branch ${job.branch} head is ${remoteHead ?? 'missing'}, expected ${job.commit}`) - } - - const pr = await deps.forge.openOrGetPullRequest(repo, job.branch, job.base, job.title, job.body) - - // Write the artifact record LAST (daemon-stamped type + prev + links), under the deterministic - // rkey — a resumed finish that already wrote it adopts the existing record rather than duplicating. - const record: ArtifactRecord = { - $type: COLLECTIONS.artifact, - request: { uri: input.requestUri, cid: input.requestCid }, - goal: job.goal, - type: 'implementation', - ...(job.prev ? { prev: job.prev } : {}), - body: job.body, - links: { branch: job.branch, commit: job.commit, pr: pr.url }, - ...(job.criteria && job.criteria.length ? { criteria: job.criteria } : {}), - createdAt: now(), - } - const rkey = implArtifactRkey(input.requestUri, input.requestCid) - let ref: StrongRef - try { - ref = await input.client.create(COLLECTIONS.artifact, record, { rkey }) - } catch (error) { - if (error instanceof XrpcError && error.status === 400 && error.error === 'RecordAlreadyExists') { - // Adopt ONLY a record that matches this exact pending job (finding 7): a resumed finish is - // deterministic, so a mismatch at the deterministic rkey is a genuine anomaly (a colliding or - // tampered record) — hard-error rather than mark fulfilled against the wrong artifact. - const existing = await input.client.getOwnRecord(COLLECTIONS.artifact, rkey) - if (!existing) throw error - if (!adoptedRecordMatches(existing.value as ArtifactRecord, job)) { - throw new Error(`refusing to adopt implementation record at ${rkey}: it does not match the pending finish job`) - } - ref = { uri: existing.uri, cid: existing.cid } - } else { - throw error - } - } - deps.log?.(`finished implementation ${input.requestUri} -> ${ref.uri} (pr ${pr.url}, ${compareUrl(repo, job.base, job.commit)})`) - return { outcome: 'fulfilled', ref } -} - -export interface FinishPumpDeps { - ledger: TurnLedger - /** Own concurrency budget (default 2), separate from turn/check dispatch. */ - concurrency: number - /** DID → record client for the actor that must author the record; undefined if not loaded here. */ - clientFor: (did: string) => TurnRecordWriter | undefined - runDirFor: (requestUri: string) => string - githubToken: string - finish?: (input: FinishInput, deps: FinishDeps) => Promise - forge: ForgeAdapter - now?: () => number - /** True while the turn dispatcher still has a container in flight for this request: the finish is - * held off until the container has exited, so it never races a still-running turn (finding 3). */ - isInFlight?: (requestUri: string) => boolean - log?: (message: string) => void -} - -/** - * Bounded-concurrency pump that drives `finishing` ledger rows to written records — the same shape - * as `TurnDispatcher`/`CheckDispatcher`, on its own budget so a finish never contends with a turn. - * `pump()` launches due finishes in the background and returns immediately. On success the row goes - * `fulfilled` and the runDir is deleted; on a rejected submission or a transient failure the row - * stays `finishing` (never a state the container dispatcher would re-launch) with a cooldown, until - * the retry bound gives it up. Startup reconciliation is just the first `pump()` — every finishing - * row is picked up here, so a process that died mid-finish resumes idempotently. - */ -export class FinishPump { - readonly #deps: FinishPumpDeps - readonly #inFlight = new Map>() - - constructor(deps: FinishPumpDeps) { - this.#deps = deps - } - - get inFlight(): number { - return this.#inFlight.size - } - - /** Launches due finishes for every eligible `finishing` row; returns immediately. */ - pump(): void { - const now = this.#deps.now?.() ?? Date.now() - const finish = this.#deps.finish ?? finishImplementationTurn - for (const row of this.#deps.ledger.finishing()) { - if (this.#inFlight.size >= this.#deps.concurrency) break - const uri = row.requestUri - if (this.#inFlight.has(uri)) continue - // Never finish while the turn dispatcher still has this request's container in flight (it - // submitted, but hasn't exited yet) — wait for the next tick after it settles (finding 3). - if (this.#deps.isInFlight?.(uri)) continue - // The finish pump's own cooldown: a failed attempt sets next_eligible_at. - if (row.nextEligibleAt !== undefined && Date.parse(row.nextEligibleAt) > now) continue - const job = row.submission - if (!job) { - this.#deps.log?.(`finishing row ${uri} has no submission payload; giving up`) - this.#deps.ledger.giveUp(uri, row.requestCid) - continue - } - const client = this.#deps.clientFor(job.actorDid) - if (!client) { - this.#deps.log?.(`no loaded actor ${job.actorDid} to finish ${uri}; will retry`) - this.#deps.ledger.markFinishFailed(uri) - continue - } - - const input: FinishInput = { - requestUri: uri, - requestCid: row.requestCid, - job, - client, - runDir: this.#deps.runDirFor(uri), - githubToken: this.#deps.githubToken, - } - const promise = finish(input, { forge: this.#deps.forge, ...(this.#deps.log ? { log: this.#deps.log } : {}) }) - .then(async (result) => { - if (result.outcome === 'fulfilled') { - this.#deps.ledger.markFulfilled(uri, result.ref) - await rm(input.runDir, { recursive: true, force: true }).catch(() => {}) - this.#deps.log?.(`finish complete ${uri}: ${result.ref.uri}`) - } else { - // A bogus submission can't be salvaged by retrying — give up (a `turn reset` re-enables). - this.#deps.log?.(`finish rejected ${uri}: ${result.reason}`) - this.#deps.ledger.giveUp(uri, row.requestCid) - } - }) - .catch((error: unknown) => { - const message = error instanceof Error ? error.message : String(error) - const outcome = this.#deps.ledger.markFinishFailed(uri) - this.#deps.log?.(`finish failed ${uri}: ${message}; now ${outcome.state} (attempt ${outcome.attempts})`) - }) - .finally(() => { - this.#inFlight.delete(uri) - }) - - this.#inFlight.set(uri, promise) - } - } - - /** Awaits every in-flight finish (best-effort graceful shutdown; each promise never rejects). */ - async drain(): Promise { - await Promise.all([...this.#inFlight.values()]) - } -} diff --git a/packages/daemon/src/forge.ts b/packages/daemon/src/forge.ts index 32f31cc..402730b 100644 --- a/packages/daemon/src/forge.ts +++ b/packages/daemon/src/forge.ts @@ -1,70 +1,29 @@ import type { FetchLike } from '@radial/atproto' -/** - * The forge adapter (design §10): the small, structured surface the daemon drives a code forge - * through. GitHub is the first implementation. Every method is daemon-side — the adapter holds the - * forge credential, which NEVER enters a turn container (design §13). The daemon pushes with git - * (see finish.ts); this adapter only makes API calls (branch head lookup, PR open/adopt, PR state). - */ export interface GitHubRepo { owner: string repo: string } -export interface PullRequestRef { - number: number - url: string -} - export interface PullRequestState { state: 'open' | 'merged' | 'closed' mergedAt?: string - headSha: string - /** The PR head branch ref (e.g. `radial/impl-abc123`). */ headRef: string - /** The PR head repo `owner/repo` (guards against a cross-fork PR redirect). */ headRepoFullName: string - /** The PR base branch ref. */ baseRef: string } +/** Phase 4.5 keeps forge access observation-only: turns perform every GitHub write themselves. */ export interface ForgeAdapter { - /** The sha at the tip of `branch`, or null if the branch does not exist. */ - getBranchHead(repo: GitHubRepo, branch: string): Promise - /** - * Open a PR for `head` (a branch name) into `base`, or adopt the existing open one. Idempotent: - * on a 422 "a pull request already exists" it re-lists PRs for this head and adopts the matching - * OPEN one (verifying head/base), and NEVER adopts a merged/closed PR as if it were open. - */ - openOrGetPullRequest( - repo: GitHubRepo, - head: string, - base: string, - title: string, - body: string, - ): Promise - /** Full state of PR `number` (open/merged/closed + head sha + merge time). */ - getPullRequest(repo: GitHubRepo, number: number): Promise - /** - * Merge-observation entry point (design §10): the state of a PR named by its canonical GitHub URL. - * Parses+validates the URL and rebuilds the API call from the parsed parts — the token never - * attaches to a raw, record-supplied URL. Structurally satisfies merge-poll.ts's `ForgeStateSource`. - */ - getPullRequestState(prUrl: string): Promise<{ state: 'open' | 'merged' | 'closed'; mergedAt?: string }> - /** Pure compare URL for a base..commit range. */ + getPullRequestState(prUrl: string): Promise compareUrl(repo: GitHubRepo, base: string, commit: string): string } -/** - * Parse a GitHub repo out of a git URL. Handles the forms Radial stores (see `normalizeGitUrl` in - * the sidecar): `https://github.com/owner/repo(.git)`, `ssh://git@github.com/owner/repo(.git)`, and - * the raw SCP remote `git@github.com:owner/repo.git`. Throws for anything that is not a GitHub repo - * URL — a non-github host, or a URL without an `owner/repo` path. - */ export function parseGitHubRepo(gitUrl: string): GitHubRepo { const fail = (): never => { throw new Error(`not a GitHub repository URL: ${gitUrl}`) } + let host: string let path: string if (/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//.test(gitUrl)) { @@ -77,21 +36,18 @@ export function parseGitHubRepo(gitUrl: string): GitHubRepo { host = url.hostname.toLowerCase() path = url.pathname } else { - // SCP-style remote: user@host:owner/repo(.git) const scp = /^(?:[^@/:]+@)?([^@/:]+):(.+)$/.exec(gitUrl) if (!scp) return fail() host = (scp[1] as string).toLowerCase() path = scp[2] as string } + if (host !== 'github.com' && host !== 'www.github.com') return fail() - const segments = path.replace(/^\/+/, '').replace(/\.git$/, '').split('/') - if (segments.length < 2 || !segments[0] || !segments[1]) return fail() - return { owner: segments[0], repo: segments[1] } + const [owner, repo] = path.replace(/^\/+/, '').replace(/\.git$/, '').split('/') + if (!owner || !repo) return fail() + return { owner, repo } } -/** The canonical PR URL Radial reads a merge from: `https://github.com/owner/repo/pull/` only — - * and nothing else. Any userinfo, non-default port, query string, or fragment is rejected so a - * record-supplied URL can never redirect the token-bearing API call somewhere unexpected. */ export function parseGitHubPullUrl(prUrl: string): { repo: GitHubRepo; number: number } { let url: URL try { @@ -99,40 +55,31 @@ export function parseGitHubPullUrl(prUrl: string): { repo: GitHubRepo; number: n } catch { throw new Error(`invalid pull request URL: ${prUrl}`) } - if (url.protocol !== 'https:') throw new Error(`pull request URL must be https: ${prUrl}`) - if (url.hostname.toLowerCase() !== 'github.com') throw new Error(`pull request URL must be on github.com: ${prUrl}`) - if (url.username !== '' || url.password !== '') throw new Error(`pull request URL must not contain userinfo: ${prUrl}`) - if (url.port !== '') throw new Error(`pull request URL must use the default port: ${prUrl}`) - if (url.search !== '') throw new Error(`pull request URL must not contain a query string: ${prUrl}`) - if (url.hash !== '') throw new Error(`pull request URL must not contain a fragment: ${prUrl}`) - const match = /^\/([^/]+)\/([^/]+)\/pull\/([0-9]+)$/.exec(url.pathname) - if (!match) throw new Error(`not a canonical github.com/owner/repo/pull/ URL: ${prUrl}`) - return { repo: { owner: match[1] as string, repo: match[2] as string }, number: Number(match[3]) } + if ( + url.protocol !== 'https:' || + url.hostname.toLowerCase() !== 'github.com' || + url.username || + url.password || + url.port || + url.search || + url.hash + ) { + throw new Error(`not a canonical GitHub pull request URL: ${prUrl}`) + } + const match = /^\/([^/]+)\/([^/]+)\/pull\/([1-9][0-9]*)$/.exec(url.pathname) + if (!match) { + throw new Error(`not a canonical github.com/owner/repo/pull/ URL: ${prUrl}`) + } + return { + repo: { owner: match[1] as string, repo: match[2] as string }, + number: Number(match[3]), + } } export function compareUrl(repo: GitHubRepo, base: string, commit: string): string { return `https://github.com/${repo.owner}/${repo.repo}/compare/${base}...${commit}` } -interface GitHubPull { - number: number - html_url: string - state: string - merged?: boolean - merged_at?: string | null - head: { ref: string; sha: string; repo?: { full_name?: string } | null } - base: { ref: string } -} - -const mapState = (pull: Pick): 'open' | 'merged' | 'closed' => - pull.merged ? 'merged' : pull.state === 'open' ? 'open' : 'closed' - -/** - * GitHub implementation of the forge adapter. Takes an injected `FetchLike` (reusing @radial/atproto's - * type) and a token; the token serves both the API here and the git push in finish.ts (v1: one - * daemon-side `GITHUB_TOKEN` with Contents read/write — GitHub has no branch-scoped write, which is - * exactly why the credential stays daemon-side, never in a container). - */ export class GitHubForge implements ForgeAdapter { readonly #fetch: FetchLike readonly #token: string @@ -144,101 +91,34 @@ export class GitHubForge implements ForgeAdapter { this.#apiBase = options.apiBase ?? 'https://api.github.com' } - async #request(method: string, path: string, body?: unknown): Promise<{ status: number; json: unknown }> { - const response = await this.#fetch(`${this.#apiBase}${path}`, { - method, - headers: { - authorization: `Bearer ${this.#token}`, - accept: 'application/vnd.github+json', - 'x-github-api-version': '2022-11-28', - ...(body !== undefined ? { 'content-type': 'application/json' } : {}), + async getPullRequestState(prUrl: string): Promise { + const { repo, number } = parseGitHubPullUrl(prUrl) + const response = await this.#fetch( + `${this.#apiBase}/repos/${repo.owner}/${repo.repo}/pulls/${number}`, + { + headers: { + authorization: `Bearer ${this.#token}`, + accept: 'application/vnd.github+json', + 'x-github-api-version': '2022-11-28', + }, }, - ...(body !== undefined ? { body: JSON.stringify(body) } : {}), - }) - const text = await response.text() - let json: unknown - try { - json = text ? JSON.parse(text) : undefined - } catch { - json = undefined - } - return { status: response.status, json } - } - - async getBranchHead(repo: GitHubRepo, branch: string): Promise { - const { status, json } = await this.#request( - 'GET', - `/repos/${repo.owner}/${repo.repo}/git/ref/heads/${encodeURIComponent(branch)}`, - ) - if (status === 404) return null - if (status !== 200) throw new Error(`getBranchHead ${branch} failed (${status})`) - const sha = (json as { object?: { sha?: unknown } } | undefined)?.object?.sha - if (typeof sha !== 'string') throw new Error(`getBranchHead ${branch}: malformed response`) - return sha - } - - async openOrGetPullRequest( - repo: GitHubRepo, - head: string, - base: string, - title: string, - body: string, - ): Promise { - const create = await this.#request('POST', `/repos/${repo.owner}/${repo.repo}/pulls`, { - title, - head, - base, - body, - }) - if (create.status === 201) { - const pull = create.json as GitHubPull - return { number: pull.number, url: pull.html_url } - } - if (create.status !== 422) { - throw new Error(`openPullRequest failed (${create.status}): ${JSON.stringify(create.json)}`) - } - // 422: a PR for this head already exists (or the branches are otherwise unmergeable). List ONLY - // the OPEN PRs for this exact head (GitHub permits at most one open PR per head branch, so there - // is no pagination concern) and adopt the head/base match — a merged/closed PR is never in this - // list, so it can never be dressed up as open. - const { status, json } = await this.#request( - 'GET', - `/repos/${repo.owner}/${repo.repo}/pulls?state=open&head=${encodeURIComponent(`${repo.owner}:${head}`)}`, ) - if (status !== 200 || !Array.isArray(json)) { - throw new Error(`openPullRequest: 422 on create and could not list existing PRs (${status})`) - } - const pulls = json as GitHubPull[] - const open = pulls.find((pull) => pull.head.ref === head && pull.base.ref === base && pull.state === 'open') - if (!open) { - throw new Error( - `openPullRequest: create returned 422 but no OPEN pull request exists for ${repo.owner}:${head} -> ${base}`, - ) + if (response.status !== 200) { + throw new Error(`getPullRequestState #${number} failed (${response.status})`) } - return { number: open.number, url: open.html_url } - } - - async getPullRequest(repo: GitHubRepo, number: number): Promise { - const { status, json } = await this.#request('GET', `/repos/${repo.owner}/${repo.repo}/pulls/${number}`) - if (status !== 200) throw new Error(`getPullRequest #${number} failed (${status})`) - const pull = json as GitHubPull - const state = mapState(pull) - return { - state, - headSha: pull.head.sha, - headRef: pull.head.ref, - headRepoFullName: pull.head.repo?.full_name ?? '', - baseRef: pull.base.ref, - ...(pull.merged_at ? { mergedAt: pull.merged_at } : {}), + const value = JSON.parse(await response.text()) as { + state?: string + merged?: boolean + merged_at?: string | null + head?: { ref?: string; repo?: { full_name?: string } | null } + base?: { ref?: string } } - } - - async getPullRequestState(prUrl: string): Promise<{ state: 'open' | 'merged' | 'closed'; mergedAt?: string }> { - const { repo, number } = parseGitHubPullUrl(prUrl) - const pull = await this.getPullRequest(repo, number) return { - state: pull.state, - ...(pull.mergedAt !== undefined ? { mergedAt: pull.mergedAt } : {}), + state: value.merged ? 'merged' : value.state === 'open' ? 'open' : 'closed', + headRef: value.head?.ref ?? '', + headRepoFullName: value.head?.repo?.full_name ?? '', + baseRef: value.base?.ref ?? '', + ...(value.merged_at ? { mergedAt: value.merged_at } : {}), } } @@ -247,67 +127,14 @@ export class GitHubForge implements ForgeAdapter { } } -interface FakePull { - number: number - head: string - base: string - state: 'open' | 'merged' | 'closed' - mergedAt?: string - headSha: string - url: string - /** Optional cross-fork head repo override (`owner/repo`); defaults to the queried repo. */ - headRepoFullName?: string -} - -/** - * In-memory forge for tests. `getBranchHead` reads from `branches` unless a `branchHeadResolver` is - * injected (the finish test points it at a real local bare repo, so getBranchHead reflects the - * actual push). PRs are tracked in `pulls`; `openOrGetPullRequest` adopts an existing OPEN one for - * the same head/base and otherwise mints a new one. Preset `pulls` to model a v2 merged/open prior PR. - */ export class FakeForge implements ForgeAdapter { - readonly branches = new Map() - readonly pulls: FakePull[] = [] - branchHeadResolver?: (repo: GitHubRepo, branch: string) => Promise - #nextNumber = 1 - - constructor(options: { branchHeadResolver?: (repo: GitHubRepo, branch: string) => Promise } = {}) { - if (options.branchHeadResolver) this.branchHeadResolver = options.branchHeadResolver - } - - async getBranchHead(repo: GitHubRepo, branch: string): Promise { - if (this.branchHeadResolver) return this.branchHeadResolver(repo, branch) - return this.branches.get(branch) ?? null - } - - async openOrGetPullRequest(repo: GitHubRepo, head: string, base: string): Promise { - const open = this.pulls.find((pull) => pull.head === head && pull.base === base && pull.state === 'open') - if (open) return { number: open.number, url: open.url } - const number = this.#nextNumber++ - const headSha = (await this.getBranchHead(repo, head)) ?? 'unknown' - const url = `https://github.com/${repo.owner}/${repo.repo}/pull/${number}` - this.pulls.push({ number, head, base, state: 'open', headSha, url }) - return { number, url } - } - - async getPullRequest(repo: GitHubRepo, number: number): Promise { - const pull = this.pulls.find((entry) => entry.number === number) - if (!pull) throw new Error(`FakeForge: no pull request #${number}`) - return { - state: pull.state, - headSha: pull.headSha, - headRef: pull.head, - headRepoFullName: pull.headRepoFullName ?? `${repo.owner}/${repo.repo}`, - baseRef: pull.base, - ...(pull.mergedAt !== undefined ? { mergedAt: pull.mergedAt } : {}), - } - } + readonly pulls = new Map() - async getPullRequestState(prUrl: string): Promise<{ state: 'open' | 'merged' | 'closed'; mergedAt?: string }> { - const { number } = parseGitHubPullUrl(prUrl) - const pull = this.pulls.find((entry) => entry.number === number) - if (!pull) throw new Error(`FakeForge: no pull request at ${prUrl}`) - return { state: pull.state, ...(pull.mergedAt !== undefined ? { mergedAt: pull.mergedAt } : {}) } + async getPullRequestState(url: string): Promise { + parseGitHubPullUrl(url) + const pull = this.pulls.get(url) + if (!pull) throw new Error(`FakeForge: no pull request at ${url}`) + return pull } compareUrl(repo: GitHubRepo, base: string, commit: string): string { diff --git a/packages/daemon/src/github-auth.ts b/packages/daemon/src/github-auth.ts new file mode 100644 index 0000000..dc639c5 --- /dev/null +++ b/packages/daemon/src/github-auth.ts @@ -0,0 +1,68 @@ +import { spawn } from 'node:child_process' + +export type GitHubAuthRunner = ( + command: string, + args: string[], + options?: { interactive?: boolean }, +) => Promise<{ code: number; stdout: string; stderr: string }> + +const nodeRunner: GitHubAuthRunner = (command, args, options) => + new Promise((resolve, reject) => { + const child = spawn(command, args, options?.interactive ? ({ stdio: 'inherit' } as never) : undefined) + let stdout = '' + let stderr = '' + child.stdout?.on('data', (chunk: Uint8Array) => { + stdout += new TextDecoder().decode(chunk) + }) + child.stderr?.on('data', (chunk: Uint8Array) => { + stderr += new TextDecoder().decode(chunk) + }) + child.on('error', reject) + child.on('close', (code: number | null) => resolve({ code: code ?? 1, stdout, stderr })) + }) + +/** Resolves GitHub possession without ever writing a token to config or command arguments. */ +export async function resolveGitHubToken( + env: Record = process.env, + runner: GitHubAuthRunner = nodeRunner, +): Promise { + if (env.GH_TOKEN) return env.GH_TOKEN + if (env.GITHUB_TOKEN) return env.GITHUB_TOKEN + try { + const result = await runner('gh', ['auth', 'token', '-h', 'github.com']) + return result.code === 0 && result.stdout.trim() ? result.stdout.trim() : undefined + } catch (error: unknown) { + if ((error as { code?: string })?.code === 'ENOENT') return undefined + throw error + } +} + +export async function ensureGitHubAuth( + options: { + env?: Record + runner?: GitHubAuthRunner + skip?: boolean + } = {}, +): Promise { + const env = options.env ?? process.env + const runner = options.runner ?? nodeRunner + const existing = await resolveGitHubToken(env, runner) + if (existing || options.skip) return existing + let login + try { + login = await runner( + 'gh', + ['auth', 'login', '--hostname', 'github.com', '--git-protocol', 'https', '--web'], + { interactive: true }, + ) + } catch (error: unknown) { + if ((error as { code?: string })?.code === 'ENOENT') { + throw new Error('GitHub CLI (gh) is required; install it from https://cli.github.com/') + } + throw error + } + if (login.code !== 0) throw new Error('GitHub login failed') + const token = await resolveGitHubToken(env, runner) + if (!token) throw new Error('GitHub login completed but gh did not provide a token') + return token +} diff --git a/packages/daemon/src/harness.ts b/packages/daemon/src/harness.ts index e1328e6..652498c 100644 --- a/packages/daemon/src/harness.ts +++ b/packages/daemon/src/harness.ts @@ -10,8 +10,8 @@ export interface HarnessInvocationInput { bundleDir: string workdir: string models: AgentModel[] - /** True for an implementation turn: the prompt tells the agent to commit + bundle + submit with a - * commit sha (design §10) rather than write a plan body. */ + /** True for an implementation turn: the prompt tells the agent to commit, push, open or update + * the PR, and submit the resulting branch/commit/PR links. */ implementation?: boolean } @@ -53,17 +53,21 @@ function buildPrompt(input: { bundleDir: string; workdir: string; implementation if (input.implementation) { return [ ...context, - `The project is checked out READ-WRITE at ${input.workdir}. Implement the requested change directly in that working tree.`, + `The project is checked out READ-WRITE at ${input.workdir}. Before making any changes, prepare the daemon-selected branch.`, '', - 'When your implementation is complete, deliver it by running exactly these steps from the checkout:', + 'Prepare it from the checkout before implementing:', `1. \`cd ${input.workdir}\``, - '2. `git add -A && git commit -m ""` to commit your work.', - '3. `git bundle create /export/impl.bundle "$RADIAL_BASE_COMMIT..HEAD"` to bundle exactly your new commits (a RANGED bundle — the checkout is shallow).', - '4. Write a concise implementation summary (branch/PR provenance is filled in by the system, not you) to a file, then run `radial artifact submit --body-file --commit "$(git rev-parse HEAD)"`.', + '2. `gh auth setup-git`. If `git ls-remote --exit-code origin "refs/heads/$RADIAL_BRANCH"` succeeds, fetch and check out that branch; otherwise create it from the current checkout. This supports both fresh work and a reused predecessor branch.', + '3. If `$RADIAL_BRANCH` is already checked out, leave it checked out. Now implement the requested change directly in that working tree.', + '', + 'When your implementation is complete:', + '4. Commit your work with a `Co-Authored-By: $RADIAL_AGENT_NAME ($RADIAL_AGENT_DID) <$RADIAL_AGENT_EMAIL>` trailer and push `$RADIAL_BRANCH`.', + '5. Find an existing PR for `$RADIAL_BRANCH`; use `gh pr edit` when one exists, otherwise `gh pr create --base "$RADIAL_BASE_BRANCH"`. Its body must include the literal Markdown link `[Radial artifact]($RADIAL_ARTIFACT_URI)`.', + '6. Write a concise implementation summary to a file, then run `radial artifact submit --body-file --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)" --pr "$(gh pr view --json url -q .url)"`.', '', 'If you cannot complete the brief and need input first: run `radial message post --body ""` instead, do NOT commit, and stop.', '', - 'Do NOT push, and do NOT look for git or forge credentials — there are none in this container; the daemon owns pushing and opening the pull request. `radial` is on PATH and is already authenticated for this turn (unix-socket mode).', + 'Forge reconnaissance (`gh pr view`, `gh api`, and ordinary web fetches) is allowed. Do not merge. `radial` is on PATH and is authenticated only for protocol submission.', ].join('\n') } return [ @@ -83,8 +87,8 @@ function buildPrompt(input: { bundleDir: string; workdir: string; implementation * sandbox container. Docker cannot run in this development environment, so this shape is * best-effort: it is unit-tested only for its argv/env shape here, and must be validated against * a real `claude` binary in the real-claude end-to-end test before it carries production traffic. - * Secrets (ANTHROPIC_API_KEY, the egress proxy vars) are intentionally NOT set here — the run - * path (D2) injects them into `ContainerSpec.env` alongside `RADIAL_SIDECAR_SOCKET` / + * Secrets are intentionally NOT set here — the run path injects them into `ContainerSpec.env` + * alongside `RADIAL_SIDECAR_SOCKET` / * `RADIAL_TURN_TOKEN`, so this harness never has to know them. */ export class ClaudeCodeHarness implements Harness { diff --git a/packages/daemon/src/index.ts b/packages/daemon/src/index.ts index d6f768a..8b25b5e 100644 --- a/packages/daemon/src/index.ts +++ b/packages/daemon/src/index.ts @@ -6,14 +6,12 @@ export * from './check-runner.js' export * from './config.js' export * from './container.js' export * from './dispatch.js' -export * from './egress.js' -export * from './finish.js' export * from './forge.js' +export * from './github-auth.js' export * from './harness.js' export * from './init.js' export * from './ledger.js' export * from './merge-poll.js' -export * from './proxy.js' export * from './runtime.js' export * from './turn.js' export * from './turn-socket.js' diff --git a/packages/daemon/src/ledger.ts b/packages/daemon/src/ledger.ts index b388d52..d0f4fe6 100644 --- a/packages/daemon/src/ledger.ts +++ b/packages/daemon/src/ledger.ts @@ -1,48 +1,7 @@ import { DatabaseSync } from 'node:sqlite' import type { StrongRef } from '@radial/core' -/** - * Everything the daemon-side finish pump needs to drive one implementation submission to a written - * artifact record — stored durably in the `finishing` ledger row so the finish is self-contained and - * survives a process restart (it never needs to re-derive branch/prev/actor from a live index). The - * container-supplied parts (commit, body, criteria) plus daemon-side context (base commit, target - * branch, PR base, predecessor to stamp, the writing actor). - */ -export interface FinishJob { - goal: StrongRef - /** The DID whose repo the artifact record is written into (the actor that fulfilled the turn). */ - actorDid: string - gitUrl: string - /** The PR base branch (the project's default branch). */ - base: string - /** The branch the submitted commit is pushed to: a fresh `radial/impl-` or, for a v2 turn - * reusing an open predecessor PR, the predecessor's branch. */ - branch: string - /** RADIAL_BASE_COMMIT: the checkout head the ranged bundle was cut against. */ - baseCommit: string - /** Daemon-owned snapshot of the container's ranged bundle (captured at submit time; the container - * cannot touch it). The finish step verifies + fetches exactly these bytes. */ - bundlePath: string - /** The submitted HEAD sha (from the container). */ - commit: string - body: string - criteria?: string[] - /** The pull-request title (the goal title, known at dispatch). */ - title: string - /** v2 only: the predecessor artifact this revision descends from, stamped as `prev` at write time. */ - prev?: StrongRef -} - -export type TurnState = - | 'pending' - | 'running' - // An implementation turn has submitted; its record is not written yet. The container dispatcher - // never touches a finishing row (eligible() === false) — only the finish pump drives it. - | 'finishing' - | 'awaiting_input' - | 'fulfilled' - | 'crashed' - | 'gave_up' +export type TurnState = 'pending' | 'running' | 'awaiting_input' | 'fulfilled' | 'crashed' | 'gave_up' export interface TurnRow { requestUri: string @@ -53,8 +12,6 @@ export interface TurnRow { containerLabel?: string checkoutPath?: string acceptedRef?: StrongRef - /** Present only on a `finishing` row: the pending implementation finish job the finish pump drives. */ - submission?: FinishJob updatedAt: string } @@ -74,71 +31,26 @@ interface Row { checkout_path: string | null accepted_ref_uri: string | null accepted_ref_cid: string | null - submission_json: string | null updated_at: string | null } -const isRef = (value: unknown): value is StrongRef => - typeof value === 'object' && - value !== null && - typeof (value as Record).uri === 'string' && - typeof (value as Record).cid === 'string' - -function parseSubmission(json: string | null): FinishJob | undefined { - if (json === null) return undefined - try { - const value = JSON.parse(json) as unknown - if (typeof value !== 'object' || value === null) return undefined - const v = value as Record - if (!isRef(v.goal)) return undefined - const strings = ['actorDid', 'gitUrl', 'base', 'branch', 'baseCommit', 'bundlePath', 'commit', 'body', 'title'] as const - if (strings.some((key) => typeof v[key] !== 'string')) return undefined - return { - goal: v.goal, - actorDid: v.actorDid as string, - gitUrl: v.gitUrl as string, - base: v.base as string, - branch: v.branch as string, - baseCommit: v.baseCommit as string, - bundlePath: v.bundlePath as string, - commit: v.commit as string, - body: v.body as string, - title: v.title as string, - ...(Array.isArray(v.criteria) && v.criteria.every((c) => typeof c === 'string') - ? { criteria: v.criteria as string[] } - : {}), - ...(isRef(v.prev) ? { prev: v.prev } : {}), - } - } catch { - return undefined - } -} - function toRow(row: Row): TurnRow { return { requestUri: row.request_uri, requestCid: row.request_cid, state: (row.state ?? 'pending') as TurnState, attempts: row.attempts, - ...(row.next_eligible_at !== null ? { nextEligibleAt: row.next_eligible_at } : {}), - ...(row.container_label !== null ? { containerLabel: row.container_label } : {}), - ...(row.checkout_path !== null ? { checkoutPath: row.checkout_path } : {}), - ...(row.accepted_ref_uri !== null && row.accepted_ref_cid !== null + ...(row.next_eligible_at ? { nextEligibleAt: row.next_eligible_at } : {}), + ...(row.container_label ? { containerLabel: row.container_label } : {}), + ...(row.checkout_path ? { checkoutPath: row.checkout_path } : {}), + ...(row.accepted_ref_uri && row.accepted_ref_cid ? { acceptedRef: { uri: row.accepted_ref_uri, cid: row.accepted_ref_cid } } : {}), - ...(() => { - const submission = parseSubmission(row.submission_json) - return submission ? { submission } : {} - })(), updatedAt: row.updated_at ?? '', } } -/** - * Durable record of every turn the daemon has attempted, keyed by the plan/implementation - * request it answers. Attempts and cooldowns survive process restart (see `running()` for orphan - * reconciliation on daemon startup, and D2's use of it). - */ +/** Durable dispatch state. `submission_json` remains schema-compatible only; no turn submission is deferred. */ export class TurnLedger { readonly #database: DatabaseSync readonly #retryBound: number @@ -149,238 +61,169 @@ export class TurnLedger { this.#database = new DatabaseSync(path) this.#database.exec(` CREATE TABLE IF NOT EXISTS turns ( - request_uri TEXT PRIMARY KEY, request_cid TEXT NOT NULL, - state TEXT, attempts INTEGER NOT NULL DEFAULT 0, next_eligible_at TEXT, - container_label TEXT, checkout_path TEXT, - accepted_ref_uri TEXT, accepted_ref_cid TEXT, submission_json TEXT, updated_at TEXT + request_uri TEXT PRIMARY KEY, + request_cid TEXT NOT NULL, + state TEXT, + attempts INTEGER NOT NULL DEFAULT 0, + next_eligible_at TEXT, + container_label TEXT, + checkout_path TEXT, + accepted_ref_uri TEXT, + accepted_ref_cid TEXT, + submission_json TEXT, + updated_at TEXT ) STRICT; `) - // Additive migration for ledger.db files created before implementation turns existed: add the - // submission payload column if it isn't there yet. (SQLite has no `ADD COLUMN IF NOT EXISTS`.) const columns = this.#database.prepare('PRAGMA table_info(turns)').all() as Array<{ name: string }> if (!columns.some((column) => column.name === 'submission_json')) { this.#database.exec('ALTER TABLE turns ADD COLUMN submission_json TEXT') } + this.#retryBound = options.retryBound ?? 3 this.#cooldownMs = options.cooldownMs ?? 300_000 this.#now = options.now ?? (() => new Date().toISOString()) - } - get(requestUri: string): TurnRow | undefined { - const row = this.#database.prepare('SELECT * FROM turns WHERE request_uri = ?').get(requestUri) as - | Row - | undefined - return row ? toRow(row) : undefined - } - - eligible(requestUri: string, now: string = this.#now()): boolean { - const row = this.get(requestUri) - if (!row) return true - switch (row.state) { - case 'running': - case 'finishing': // owned by the finish pump — the container dispatcher must never re-launch it - case 'fulfilled': - case 'gave_up': - return false - case 'crashed': - return row.nextEligibleAt !== undefined && now >= row.nextEligibleAt - default: // pending, awaiting_input - return true - } - } - - markRunning(requestUri: string, requestCid: string, fields: { containerLabel: string; checkoutPath: string }): void { - const now = this.#now() + // A pre-4.5 handoff can safely be retried as a normal turn; its saved submission is no longer + // trusted or used because the turn now pushes and writes its artifact synchronously. + const migratedAt = this.#now() this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, next_eligible_at, container_label, checkout_path, updated_at) - VALUES (?, ?, 'running', 0, NULL, ?, ?, ?) - ON CONFLICT (request_uri) DO UPDATE SET - request_cid = excluded.request_cid, - state = 'running', - next_eligible_at = NULL, - container_label = excluded.container_label, - checkout_path = excluded.checkout_path, - updated_at = excluded.updated_at - `) - .run(requestUri, requestCid, fields.containerLabel, fields.checkoutPath, now) + .prepare( + `UPDATE turns + SET state = 'crashed', + attempts = 0, + submission_json = NULL, + next_eligible_at = ?, + updated_at = ? + WHERE state = 'finishing' + OR (state = 'gave_up' AND submission_json IS NOT NULL)`, + ) + .run(migratedAt, migratedAt) + } + + get(uri: string): TurnRow | undefined { + const row = this.#database.prepare('SELECT * FROM turns WHERE request_uri = ?').get(uri) as Row | undefined + return row ? toRow(row) : undefined } - markFulfilled(requestUri: string, acceptedRef: StrongRef): void { - const now = this.#now() - const existing = this.get(requestUri) - this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, accepted_ref_uri, accepted_ref_cid, updated_at) - VALUES (?, ?, 'fulfilled', ?, ?, ?, ?) - ON CONFLICT (request_uri) DO UPDATE SET - state = 'fulfilled', - accepted_ref_uri = excluded.accepted_ref_uri, - accepted_ref_cid = excluded.accepted_ref_cid, - updated_at = excluded.updated_at - `) - .run(requestUri, existing?.requestCid ?? '', existing?.attempts ?? 0, acceptedRef.uri, acceptedRef.cid, now) + eligible(uri: string, now = this.#now()): boolean { + const row = this.get(uri) + return ( + !row || + row.state === 'pending' || + row.state === 'awaiting_input' || + (row.state === 'crashed' && !!row.nextEligibleAt && now >= row.nextEligibleAt) + ) } - /** - * An implementation turn submitted: durably record the pending submission and move the row to - * `finishing`. Called from the turn socket's onSubmission the instant the harness submits (before - * the container even exits), so a crash between submit and finish never loses the work — the - * finish pump (and startup reconciliation) picks the row up from here. Attempts/cooldown reset so - * the finish step starts fresh. - */ - markSubmitted(requestUri: string, requestCid: string, job: FinishJob): void { + markRunning( + uri: string, + cid: string, + fields: { containerLabel: string; checkoutPath: string }, + ): void { const now = this.#now() this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, next_eligible_at, submission_json, updated_at) - VALUES (?, ?, 'finishing', 0, NULL, ?, ?) - ON CONFLICT (request_uri) DO UPDATE SET - request_cid = excluded.request_cid, - state = 'finishing', - attempts = 0, - next_eligible_at = NULL, - submission_json = excluded.submission_json, - updated_at = excluded.updated_at - `) - .run(requestUri, requestCid, JSON.stringify(job), now) - } - - /** Rows in `finishing` — pending implementation submissions the finish pump drives to a record. */ - finishing(): TurnRow[] { - const rows = this.#database.prepare("SELECT * FROM turns WHERE state = 'finishing'").all() as Row[] - return rows.map(toRow) - } - - /** The pending finish job for a request, if its row is (or was) in `finishing`. */ - submission(requestUri: string): FinishJob | undefined { - return this.get(requestUri)?.submission - } - - /** - * A finish attempt failed: keep the row in `finishing` (never revert to a state the container - * dispatcher would re-launch), bump attempts, and cool down before the next finish attempt — the - * finish step, never a container, is what resumes. Past the retry bound the row is given up on for - * good (`gave_up`), so a permanently failing finish stops rather than looping forever; a `turn - * reset` re-enables it. Mirrors `markCrashed`'s bound/cooldown, but the non-terminal state stays - * `finishing`. - */ - markFinishFailed(requestUri: string): { state: TurnState; attempts: number } { + .prepare( + `INSERT INTO turns ( + request_uri, request_cid, state, attempts, next_eligible_at, + container_label, checkout_path, submission_json, updated_at + ) VALUES (?, ?, 'running', 0, NULL, ?, ?, NULL, ?) + ON CONFLICT(request_uri) DO UPDATE SET + request_cid = excluded.request_cid, + state = 'running', + next_eligible_at = NULL, + container_label = excluded.container_label, + checkout_path = excluded.checkout_path, + submission_json = NULL, + updated_at = excluded.updated_at`, + ) + .run(uri, cid, fields.containerLabel, fields.checkoutPath, now) + } + + markFulfilled(uri: string, ref: StrongRef): void { + const old = this.get(uri) const now = this.#now() - const existing = this.get(requestUri) - const attempts = (existing?.attempts ?? 0) + 1 - const gaveUp = attempts >= this.#retryBound - const state: TurnState = gaveUp ? 'gave_up' : 'finishing' - const nextEligibleAt = gaveUp ? null : new Date(Date.parse(now) + this.#cooldownMs).toISOString() this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, next_eligible_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?) - ON CONFLICT (request_uri) DO UPDATE SET - state = excluded.state, - attempts = excluded.attempts, - next_eligible_at = excluded.next_eligible_at, - updated_at = excluded.updated_at - `) - .run(requestUri, existing?.requestCid ?? '', state, attempts, nextEligibleAt, now) - return { state, attempts } - } - - markAwaitingInput(requestUri: string): void { + .prepare( + `INSERT INTO turns ( + request_uri, request_cid, state, attempts, accepted_ref_uri, + accepted_ref_cid, submission_json, updated_at + ) VALUES (?, ?, 'fulfilled', ?, ?, ?, NULL, ?) + ON CONFLICT(request_uri) DO UPDATE SET + state = 'fulfilled', + accepted_ref_uri = excluded.accepted_ref_uri, + accepted_ref_cid = excluded.accepted_ref_cid, + submission_json = NULL, + updated_at = excluded.updated_at`, + ) + .run(uri, old?.requestCid ?? '', old?.attempts ?? 0, ref.uri, ref.cid, now) + } + + markAwaitingInput(uri: string): void { + const old = this.get(uri) const now = this.#now() - const existing = this.get(requestUri) this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, next_eligible_at, updated_at) - VALUES (?, ?, 'awaiting_input', 0, NULL, ?) - ON CONFLICT (request_uri) DO UPDATE SET - state = 'awaiting_input', - attempts = 0, - next_eligible_at = NULL, - updated_at = excluded.updated_at - `) - .run(requestUri, existing?.requestCid ?? '', now) - } + .prepare( + `INSERT INTO turns ( + request_uri, request_cid, state, attempts, next_eligible_at, submission_json, updated_at + ) VALUES (?, ?, 'awaiting_input', 0, NULL, NULL, ?) + ON CONFLICT(request_uri) DO UPDATE SET + state = 'awaiting_input', + attempts = 0, + next_eligible_at = NULL, + submission_json = NULL, + updated_at = excluded.updated_at`, + ) + .run(uri, old?.requestCid ?? '', now) + } + + markCrashed(uri: string): { state: TurnState; attempts: number } { + const old = this.get(uri) + if (old && (old.state === 'fulfilled' || old.state === 'gave_up')) { + return { state: old.state, attempts: old.attempts } + } - /** - * Increments attempts; past `retryBound` the turn is given up on for good. Caller logs loudly on - * `gave_up`. Only a `running` row transitions here: a turn that already handed off to the finish - * pump (state `finishing`) or completed must NOT be clobbered back to `crashed` by a late - * container-runner error arriving after the submission callback already ran (finding 5). A row in - * any non-`running` state is returned unchanged. - */ - markCrashed(requestUri: string): { state: TurnState; attempts: number } { + const attempts = (old?.attempts ?? 0) + 1 + const state: TurnState = attempts >= this.#retryBound ? 'gave_up' : 'crashed' const now = this.#now() - const existing = this.get(requestUri) - // Never clobber a handed-off (`finishing`) or terminal (`fulfilled`/`gave_up`) row: a late - // container-runner error arriving after the submission callback already moved the row to - // `finishing` must leave it there for the finish pump (finding 5). - if (existing && (existing.state === 'finishing' || existing.state === 'fulfilled' || existing.state === 'gave_up')) { - return { state: existing.state, attempts: existing.attempts } - } - const attempts = (existing?.attempts ?? 0) + 1 - const gaveUp = attempts >= this.#retryBound - const state: TurnState = gaveUp ? 'gave_up' : 'crashed' - const nextEligibleAt = gaveUp ? null : new Date(Date.parse(now) + this.#cooldownMs).toISOString() + const next = state === 'crashed' ? new Date(Date.parse(now) + this.#cooldownMs).toISOString() : null this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, next_eligible_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?) - ON CONFLICT (request_uri) DO UPDATE SET - state = excluded.state, - attempts = excluded.attempts, - next_eligible_at = excluded.next_eligible_at, - updated_at = excluded.updated_at - `) - .run(requestUri, existing?.requestCid ?? '', state, attempts, nextEligibleAt, now) + .prepare( + `INSERT INTO turns ( + request_uri, request_cid, state, attempts, next_eligible_at, submission_json, updated_at + ) VALUES (?, ?, ?, ?, ?, NULL, ?) + ON CONFLICT(request_uri) DO UPDATE SET + state = excluded.state, + attempts = excluded.attempts, + next_eligible_at = excluded.next_eligible_at, + submission_json = NULL, + updated_at = excluded.updated_at`, + ) + .run(uri, old?.requestCid ?? '', state, attempts, next, now) return { state, attempts } } - /** Force a request to terminal `gave_up` (a `turn reset` re-enables it). Used for a permanent, - * non-retryable rejection — e.g. an implementation request with >1 same-type predecessor, where - * retrying can't help until a human fixes the request. */ - giveUp(requestUri: string, requestCid: string): void { - const now = this.#now() - const existing = this.get(requestUri) + giveUp(uri: string, cid: string): void { + const old = this.get(uri) this.#database - .prepare(` - INSERT INTO turns (request_uri, request_cid, state, attempts, next_eligible_at, updated_at) - VALUES (?, ?, 'gave_up', ?, NULL, ?) - ON CONFLICT (request_uri) DO UPDATE SET - state = 'gave_up', - next_eligible_at = NULL, - updated_at = excluded.updated_at - `) - .run(requestUri, existing?.requestCid ?? requestCid, existing?.attempts ?? 0, now) + .prepare( + `INSERT INTO turns ( + request_uri, request_cid, state, attempts, next_eligible_at, submission_json, updated_at + ) VALUES (?, ?, 'gave_up', ?, NULL, NULL, ?) + ON CONFLICT(request_uri) DO UPDATE SET + state = 'gave_up', + next_eligible_at = NULL, + submission_json = NULL, + updated_at = excluded.updated_at`, + ) + .run(uri, old?.requestCid ?? cid, old?.attempts ?? 0, this.#now()) } - reset(requestUri: string): void { - this.#database.prepare('DELETE FROM turns WHERE request_uri = ?').run(requestUri) - } - - /** - * If a request gave up while a finish was still pending (a `gave_up` row that still carries its - * finish job payload), restore it to `finishing` with attempts/cooldown cleared so the finish pump - * retries it — the submitted work + its runDir snapshot are preserved, not thrown away (finding 8). - * Returns true iff it restored such a row; callers fall back to `reset` (delete) otherwise. - */ - reopenFinishing(requestUri: string): boolean { - const existing = this.get(requestUri) - if (!existing || existing.state !== 'gave_up' || !existing.submission) return false - const now = this.#now() - this.#database - .prepare(` - UPDATE turns SET state = 'finishing', attempts = 0, next_eligible_at = NULL, updated_at = ? - WHERE request_uri = ? - `) - .run(now, requestUri) - return true + reset(uri: string): void { + this.#database.prepare('DELETE FROM turns WHERE request_uri = ?').run(uri) } - /** Rows left in `running` — orphans to reconcile if the daemon restarted mid-turn. */ running(): TurnRow[] { - const rows = this.#database.prepare("SELECT * FROM turns WHERE state = 'running'").all() as Row[] - return rows.map(toRow) + return (this.#database.prepare("SELECT * FROM turns WHERE state = 'running'").all() as Row[]).map(toRow) } close(): void { diff --git a/packages/daemon/src/proxy.ts b/packages/daemon/src/proxy.ts deleted file mode 100644 index 85a0824..0000000 --- a/packages/daemon/src/proxy.ts +++ /dev/null @@ -1,155 +0,0 @@ -import { realpathSync } from 'node:fs' -import { createServer, type IncomingMessage } from 'node:http' -import { connect, type Socket } from 'node:net' -import { argv } from 'node:process' -import { pathToFileURL } from 'node:url' - -/** Exact, case-insensitive host match (port stripped). No wildcards in v1 — see design §13. */ -export function isHostAllowed(host: string, allowlist: string[]): boolean { - const bare = (host.split(':')[0] ?? '').toLowerCase() - if (bare.length === 0) return false - return allowlist.some((entry) => entry.toLowerCase() === bare) -} - -export interface EgressProxy { - /** `host` defaults to `127.0.0.1` (safe for the existing in-process tests); the standalone - * container entry below binds `0.0.0.0` so a turn container on the internal docker network can - * reach this proxy by the proxy container's name. */ - listen(port?: number, host?: string): Promise - close(): Promise - readonly port: number -} - -function targetFromConnect(url: string): { host: string; port: number } | undefined { - const separatorIndex = url.lastIndexOf(':') - if (separatorIndex === -1) return url.length > 0 ? { host: url, port: 443 } : undefined - const host = url.slice(0, separatorIndex) - const port = Number(url.slice(separatorIndex + 1)) - return host.length > 0 && Number.isFinite(port) ? { host, port } : undefined -} - -type Connect = typeof connect - -class HttpEgressProxy implements EgressProxy { - readonly #allowlist: string[] - readonly #connect: Connect - #server: ReturnType | undefined - #port = 0 - - constructor(allowlist: string[], connectFn: Connect = connect) { - this.#allowlist = allowlist - this.#connect = connectFn - } - - get port(): number { - return this.#port - } - - async listen(port = 0, host = '127.0.0.1'): Promise { - const server = createServer((_req, res) => { - res.statusCode = 400 - res.end('this proxy only serves CONNECT\n') - }) - server.on('connect', (req, clientSocket, head) => this.#handleConnect(req, clientSocket, head)) - await new Promise((resolvePromise, reject) => { - server.on('error', reject) - server.listen(port, host, () => resolvePromise()) - }) - this.#server = server - const address = server.address() - this.#port = typeof address === 'object' && address !== null ? address.port : port - return this.#port - } - - async close(): Promise { - const server = this.#server - if (!server) return - this.#server = undefined - await new Promise((resolvePromise, reject) => { - server.close((error) => (error ? reject(error) : resolvePromise())) - }) - } - - #handleConnect(req: IncomingMessage, clientSocket: Socket, head: Uint8Array): void { - const target = targetFromConnect(req.url ?? '') - // 443-only by default (design §13: the allowlist is for HTTPS API egress, not arbitrary TCP). - // `targetFromConnect` parses both host and port from the CONNECT target; without this check an - // allowlisted host on a non-443 port (e.g. `api.anthropic.com:22`) would still be tunneled. - // Per-host port extension could be added later by widening this check if it's ever needed. - if (!target || target.port !== 443 || !isHostAllowed(target.host, this.#allowlist)) { - clientSocket.end('HTTP/1.1 403 Forbidden\r\n\r\n') - return - } - const upstream = this.#connect({ host: target.host, port: target.port }, () => { - clientSocket.write('HTTP/1.1 200 Connection Established\r\n\r\n') - if (head.length > 0) upstream.write(head) - // pipe() (rather than manual data listeners) forwards end-of-stream in both directions, so - // a half-close on either side of the tunnel propagates to the other instead of hanging it. - clientSocket.pipe(upstream) - upstream.pipe(clientSocket) - }) - upstream.on('error', () => clientSocket.destroy()) - clientSocket.on('error', () => upstream.destroy()) - } -} - -/** - * An allowlisting HTTP CONNECT proxy: the only egress path out of the run sandbox (design §13). - * Any CONNECT target whose host isn't in `allowlist`, or whose port isn't `443`, is rejected with - * 403 before a socket to it is ever opened; an allowed `host:443` target gets a raw byte-for-byte - * tunnel. This is what `docker/proxy.Dockerfile` runs standalone in its own container (see the CLI - * entry below). `connectFn` is an optional injection point (defaults to `node:net`'s `connect`, - * exactly as in production) purely so tests can exercise the real allowlist/port gate against a - * literal `:443` CONNECT target while redirecting the actual upstream dial to a local, unprivileged - * test server — binding a real listener on port 443 requires root on most systems. - */ -export function createEgressProxy(allowlist: string[], connectFn?: Connect): EgressProxy { - return new HttpEgressProxy(allowlist, connectFn) -} - -/** - * Standalone-CLI arg parsing (exported for unit testing). Deny-all must survive here: an explicit - * `--allow-none` (emitted by `egress.ts`'s `proxyRunArgs` for an explicit empty allowlist) yields - * `allow: []`. Only when NEITHER any `--allow` NOR `--allow-none` is given — i.e. no allowlist - * decision was ever communicated to this process — does it fall back to the default allowlist. - * Without this distinction, deny-all (`[]`) and "unset" would both produce zero `--allow` flags and - * be indistinguishable, silently turning deny-all into allow-Anthropic. - */ -export function parseProxyArgs(args: string[]): { allow: string[]; port: number } { - const allow: string[] = [] - let port = 8080 - let sawAllowFlag = false - for (let index = 0; index < args.length; index += 1) { - const flag = args[index] - const value = args[index + 1] - if (flag === '--allow' && value !== undefined) { - allow.push(value) - sawAllowFlag = true - index += 1 - } else if (flag === '--allow-none') { - sawAllowFlag = true - } else if (flag === '--port' && value !== undefined) { - port = Number(value) - index += 1 - } - } - return { allow: sawAllowFlag ? allow : ['api.anthropic.com'], port } -} - -const entry = argv[1] -if (entry && import.meta.url === pathToFileURL(realpathSync(entry)).href) { - const { allow, port } = parseProxyArgs(argv.slice(2)) - const proxy = createEgressProxy(allow) - // Bind 0.0.0.0: this standalone entry is what `docker/proxy.Dockerfile` runs as the - // radial-proxy container's entrypoint, and it must be reachable from the turn container - // over the internal docker network by the proxy container's name (see egress.ts). - proxy - .listen(port, '0.0.0.0') - .then((bound) => { - console.log(`radial egress proxy listening on 0.0.0.0:${bound}, allowlist: ${allow.join(', ')}`) - }) - .catch((error: unknown) => { - console.error(error instanceof Error ? error.message : error) - process.exitCode = 1 - }) -} diff --git a/packages/daemon/src/runtime.ts b/packages/daemon/src/runtime.ts index 45b8532..051ecb4 100644 --- a/packages/daemon/src/runtime.ts +++ b/packages/daemon/src/runtime.ts @@ -125,9 +125,6 @@ export interface RunDaemonOptions { /** Optional merge-observation poller (design §10); pumped with the same fresh index right after * the dispatcher, fire-and-forget. Omitted when no forge adapter is wired (e.g. no forge config). */ mergePoller?: { pump(index: MaterializedIndex, actors: ActorRegistry): void } - /** Optional implementation finish pump (design §10); pumped each tick off the ledger's `finishing` - * rows (it needs no index), fire-and-forget on its own budget. Omitted when no forge is wired. */ - finishPump?: { pump(): void } actors: ActorRegistry intervalMs: number log?: (message: string) => void @@ -164,9 +161,6 @@ export async function runDaemon(options: RunDaemonOptions): Promise { options.log?.(`sync failed for ${spaceUri}: ${error instanceof Error ? error.message : String(error)}`) } } - // The finish pump reads the ledger, not any space index, so it runs once per tick regardless of - // per-space sync outcomes — a submitted implementation still finishes even if a space sync failed. - options.finishPump?.pump() if (stopped()) return await sleep(options.intervalMs, options.signal) if (stopped()) return diff --git a/packages/daemon/src/turn-socket.ts b/packages/daemon/src/turn-socket.ts index b2fe4d0..39c473b 100644 --- a/packages/daemon/src/turn-socket.ts +++ b/packages/daemon/src/turn-socket.ts @@ -3,6 +3,7 @@ import { chmod, rm } from 'node:fs/promises' import { createServer, type Server, type Socket } from 'node:net' import { dirname } from 'node:path' import { XrpcError } from '@radial/atproto' +import { parseGitHubPullUrl, parseGitHubRepo } from './forge.js' import { COLLECTIONS, TURN_LIMITS, @@ -33,35 +34,20 @@ export function sha256Hex(text: string): string { return createHash('sha256').update(text).digest('hex') } -export interface PendingSubmission { - commit: string - body: string - criteria?: string[] -} - export interface TurnRequestContext { token: string /** The goal-scoped request being fulfilled. */ request: { uri: string; cid: string; goal: StrongRef } client: TurnRecordWriter now?: () => string - /** The artifact type this turn produces (daemon-side, never from the container's envelope). - * Defaults to `plan`. `implementation` selects the deferred finish path below; every other type - * uses the synchronous socket write path. */ + /** The artifact type this turn produces, owned by the daemon. */ artifactType?: string - /** Implementation turns only: persist the pending submission durably (the ledger `finishing` - * row) the instant the harness submits, so a crash before the container exits never loses it. - * When present AND `artifactType === 'implementation'`, `#submitArtifact` records the submission - * and returns `accepted` WITHOUT writing the artifact record — the daemon's finish pump owns the - * push/PR/record-write later. No forge/git/record-write ever happens in this handler. May be - * async (it snapshots the bundle daemon-side); a rejection fails the submit and persists nothing. */ - onSubmission?: (submission: PendingSubmission) => void | Promise + /** Daemon-owned implementation provenance. */ + implementation?: { gitUrl: string; branch: string; prev?: StrongRef } } export interface TurnObservation { artifact?: StrongRef - /** Set for an implementation turn once the harness submits: the deferred-finish payload. */ - submission?: PendingSubmission questions: StrongRef[] } @@ -103,9 +89,8 @@ export function base32Encode(bytes: Uint8Array): string { /** * Deterministic rkey for the artifact answering a request: `` plus 24 base32 characters of - * sha256(requestUri#requestCid). A retried turn (or finish step) recomputes the same rkey, so a - * duplicate write collides on the PDS instead of creating a second record (see the - * RecordAlreadyExists adoption paths in #submitArtifact and the finish pump). The 24-char base32 + * sha256(requestUri#requestCid). A retried turn recomputes the same rkey, so a duplicate write + * collides on the PDS instead of creating a second record (see #submitArtifact adoption). The 24-char base32 * body is prefix-independent, so `turnContainerLabel` (which strips the prefix) is stable across * plan/implementation turns for the same request. */ @@ -119,7 +104,7 @@ export function planArtifactRkey(requestUri: string, requestCid: string): string return artifactRkey(requestUri, requestCid, 'plan-') } -/** `impl-` prefixed rkey for an implementation artifact; written daemon-side by the finish pump. */ +/** `impl-` prefixed rkey for a synchronous implementation artifact. */ export function implArtifactRkey(requestUri: string, requestCid: string): string { return artifactRkey(requestUri, requestCid, 'impl-') } @@ -149,7 +134,9 @@ interface ParsedEnvelope { token: string body: string criteria?: string[] + branch?: string commit?: string + pr?: string } const isObject = (value: unknown): value is Record => @@ -162,7 +149,7 @@ const isShaShaped = (value: unknown): value is string => function parseEnvelope(raw: unknown): ParsedEnvelope | undefined { if (!isObject(raw)) return undefined - const { method, token, body, criteria, commit } = raw + const { method, token, body, criteria, branch, commit, pr } = raw if (method !== 'submitArtifact' && method !== 'askQuestion') return undefined if (typeof token !== 'string' || typeof body !== 'string') return undefined if (method === 'submitArtifact') { @@ -174,12 +161,16 @@ function parseEnvelope(raw: unknown): ParsedEnvelope | undefined { // `commit` is optional at the envelope layer (plan turns never send it); when present it must be // SHA-shaped. #submitArtifact enforces that an implementation turn actually supplies one. if (commit !== undefined && !isShaShaped(commit)) return undefined + if (branch !== undefined && typeof branch !== 'string') return undefined + if (pr !== undefined && typeof pr !== 'string') return undefined return { method, token, body, ...(parsedCriteria ? { criteria: parsedCriteria } : {}), ...(commit !== undefined ? { commit: commit as string } : {}), + ...(branch !== undefined ? { branch: branch as string } : {}), + ...(pr !== undefined ? { pr: pr as string } : {}), } } return { method, token, body } @@ -338,7 +329,7 @@ export class TurnSocketServer { } async #submitArtifact(envelope: ParsedEnvelope): Promise { - if (this.#observation.artifact || this.#observation.submission || this.#submitReserved) { + if (this.#observation.artifact || this.#submitReserved) { return { ok: false, error: 'an artifact was already submitted for this request' } } // Reserve synchronously, before the first `await` below, so a second concurrent submit (a @@ -349,43 +340,42 @@ export class TurnSocketServer { const { request, client } = this.#context const type = this.#context.artifactType ?? 'plan' - // Implementation turns take the deferred-finish path: no forge/git/record-write happens here - // (the 30s socket timeout makes any of that unsafe in-handler). Validate the SHA-shaped commit - // the harness pushed into its bundle, persist the pending submission durably via onSubmission - // (wired to the ledger `finishing` row), and acknowledge acceptance — the daemon's finish pump - // owns the branch/PR/record-write. The type is taken from the daemon-side context, NEVER the - // container's envelope. + let links: ArtifactRecord['links'] = {} + let prev: StrongRef | undefined if (type === 'implementation') { - const commit = envelope.commit - if (commit === undefined) { + if ( + !envelope.branch?.startsWith('radial/impl-') || + envelope.branch !== this.#context.implementation?.branch + ) { this.#submitReserved = false - return { ok: false, error: 'implementation submit requires a --commit (the pushed HEAD sha)' } + return { + ok: false, + error: 'implementation branch does not match the daemon-selected reserved branch', + } } - // Require a canonical, FULL 40-char lowercase hex sha — reject abbreviated or uppercased forms - // with a clear message (a downstream git op must never receive an ambiguous object name). - if (!/^[0-9a-f]{40}$/.test(commit)) { + if (!envelope.commit || !/^[0-9a-f]{40}$/.test(envelope.commit)) { this.#submitReserved = false return { ok: false, error: 'commit must be a full 40-character lowercase hex sha' } } - if (!this.#context.onSubmission) { + if (!envelope.pr || !this.#context.implementation) { this.#submitReserved = false - return { ok: false, error: 'implementation turns are not accepted on this socket (no submission sink)' } - } - const submission: PendingSubmission = { - commit, - body: envelope.body, - ...(envelope.criteria && envelope.criteria.length ? { criteria: envelope.criteria } : {}), + return { ok: false, error: 'implementation submit requires --branch, --commit, and --pr' } } - // onSubmission snapshots the bundle + persists the finishing row; a rejection (e.g. a - // symlink/FIFO/oversize bundle) fails the submit and records nothing. try { - await this.#context.onSubmission(submission) + const project = parseGitHubRepo(this.#context.implementation.gitUrl) + const parsed = parseGitHubPullUrl(envelope.pr) + if ( + parsed.repo.owner.toLowerCase() !== project.owner.toLowerCase() || + parsed.repo.repo.toLowerCase() !== project.repo.toLowerCase() + ) { + throw new Error('PR is not for this project repository') + } } catch (error) { this.#submitReserved = false - return { ok: false, error: `submission could not be persisted: ${error instanceof Error ? error.message : String(error)}` } + return { ok: false, error: error instanceof Error ? error.message : String(error) } } - this.#observation.submission = submission - return { ok: true, accepted: true } + links = { branch: envelope.branch, commit: envelope.commit, pr: envelope.pr } + prev = this.#context.implementation.prev } const record: ArtifactRecord = { @@ -394,11 +384,15 @@ export class TurnSocketServer { goal: request.goal, type, body: envelope.body, - links: {}, + links, + ...(prev ? { prev } : {}), ...(envelope.criteria && envelope.criteria.length ? { criteria: envelope.criteria } : {}), createdAt: this.#now(), } - const rkey = planArtifactRkey(request.uri, request.cid) + const rkey = + type === 'implementation' + ? implArtifactRkey(request.uri, request.cid) + : planArtifactRkey(request.uri, request.cid) let ref: StrongRef try { ref = await client.create(COLLECTIONS.artifact, record, { rkey }) @@ -411,14 +405,21 @@ export class TurnSocketServer { this.#submitReserved = false throw error } - // Sanity-verify the adopted record is the same artifact type before adopting it (the rkey is - // deterministic per request+cid and daemon-controlled, so a type mismatch is a genuine - // anomaly, never a legitimate retry). A differing body IS allowed here: a retried plan turn - // may produce different prose, and first-write-wins adoption is the intended idempotency. - const existingType = (existing.value as ArtifactRecord).type - if (existingType !== type) { + const current = existing.value as ArtifactRecord + if ( + current.$type !== COLLECTIONS.artifact || + current.type !== type || + current.request.uri !== request.uri || + current.request.cid !== request.cid || + current.goal?.uri !== request.goal.uri || + current.goal?.cid !== request.goal.cid || + JSON.stringify(current.links) !== JSON.stringify(links) || + JSON.stringify(current.prev) !== JSON.stringify(prev) || + JSON.stringify(current.criteria ?? []) !== JSON.stringify(envelope.criteria ?? []) || + current.body !== envelope.body + ) { this.#submitReserved = false - throw new Error(`refusing to adopt record at ${rkey}: type "${String(existingType)}" != expected "${type}"`) + throw new Error(`refusing to adopt tampered record at ${rkey}`) } ref = { uri: existing.uri, cid: existing.cid } } else { diff --git a/packages/daemon/src/turn.ts b/packages/daemon/src/turn.ts index 7d5751c..68df3b2 100644 --- a/packages/daemon/src/turn.ts +++ b/packages/daemon/src/turn.ts @@ -1,23 +1,21 @@ import { chmod, mkdir, rm } from 'node:fs/promises' import { join } from 'node:path' -import type { ArtifactRequestRecord, ArtifactTypeRecord, TurnBundle, StrongRef } from '@radial/core' +import { COLLECTIONS, type ArtifactRequestRecord, type ArtifactTypeRecord, type TurnBundle, type StrongRef } from '@radial/core' import type { LoadedActor } from './actors.js' -import { checkoutRepo, snapshotBundleFile, writeBundle } from './bundle-writer.js' +import { checkoutRepo, writeBundle } from './bundle-writer.js' import type { ContainerRunner, ContainerSpec } from './container.js' import { composeBrief, type Harness } from './harness.js' -import { TurnSocketServer, planArtifactRkey, type PendingSubmission } from './turn-socket.js' +import { TurnSocketServer, implArtifactRkey, planArtifactRkey } from './turn-socket.js' -/** The exact built-in artifact-type name that gets the implementation code path (rw /work + /export, - * git identity, deferred daemon-side finish). Every other type dispatches as a plan-style turn. */ +/** The exact built-in artifact-type name that gets the implementation code path (rw /work and + * git identity). Every other type dispatches as a plan-style turn. */ export const IMPLEMENTATION_TYPE = 'implementation' -export type TurnOutcome = 'fulfilled' | 'awaiting_input' | 'submitted' | 'crashed' +export type TurnOutcome = 'fulfilled' | 'awaiting_input' | 'crashed' export interface TurnResult { outcome: TurnOutcome acceptedRef?: StrongRef - /** Set when `outcome === 'submitted'`: the pending implementation submission the finish pump drives. */ - submission?: PendingSubmission label: string } @@ -37,11 +35,9 @@ export interface TurnInput { * whose predecessor PR is still open, dispatch overrides this with the predecessor's branch so the * new commit descends from the prior head (a plain fast-forward push, no force machinery). */ checkoutRef?: string - /** Implementation turns only: durably persist the pending submission the instant the harness - * submits (wired to the ledger `finishing` row). Receives the container's submission, the checkout - * head sha (RADIAL_BASE_COMMIT), and the daemon-owned bundle snapshot path — the last two only - * runTurn knows. Absent for plan-style turns. */ - onSubmission?: (submission: PendingSubmission, baseCommit: string, bundlePath: string) => void | Promise + /** Implementation provenance stamped by the daemon. */ + prev?: StrongRef + branch?: string /** Container memory limit (docker `--memory` syntax, e.g. `4g`). Defaults to `4g` when unset — * a turn must never run with unbounded memory. */ memory?: string @@ -49,8 +45,7 @@ export interface TurnInput { * production/Linux path. `tcp` is the dev-only escape hatch for macOS/Docker Desktop, where a * bind-mounted AF_UNIX socket is visible inside the VM but `connect()` gets ECONNREFUSED: it * binds the turn socket on 0.0.0.0 instead and points the container at - * `host.docker.internal`. See `resolveRunConfig`'s fail-closed check in cli.ts — this is only - * ever reachable with egress enforcement explicitly off. */ + * `host.docker.internal`. */ turnTransport?: 'unix' | 'tcp' } @@ -62,11 +57,12 @@ export interface TurnDeps { ref: string dest: string allowedSchemes: string[] + githubToken?: string }) => Promise<{ path: string; commit: string }> now?: () => string makeToken?: () => string anthropicApiKey?: string - httpsProxy?: string + githubToken?: string log?: (message: string) => void } @@ -94,7 +90,6 @@ export async function runTurn(input: TurnInput, deps: TurnDeps): Promise` fail on a non-empty destination. await rm(input.runDir, { recursive: true, force: true }) - // On a successful implementation submit the runDir is deliberately KEPT (it holds - // /export/impl.bundle, which the daemon-side finish pump fetches) — set only right before the - // `submitted` return. Every other outcome (and any throw) still tears runDir down in `finally`. - let keepRunDir = false let server: TurnSocketServer | undefined try { await mkdir(bundleDir, { recursive: true }) await mkdir(checkoutDir, { recursive: true }) - if (isImpl) await mkdir(exportDir, { recursive: true }) // The unix transport mounts this rw at /run/radial for the bind-mounted socket file; the tcp // transport (dev-only, see TurnInput.turnTransport) needs no such mount at all. if (transport === 'unix') await mkdir(socketDir, { recursive: true }) @@ -127,6 +117,7 @@ export async function runTurn(input: TurnInput, deps: TurnDeps): Promise => { - await snapshotBundleFile(join(exportDir, 'impl.bundle'), snapshotPath) - await input.onSubmission?.(submission, baseCommit, snapshotPath) - keepRunDir = true - } - : undefined + const artifactUri = `at://${input.actor.did}/${COLLECTIONS.artifact}/${implArtifactRkey(input.request.uri, input.request.cid)}` const context = { token, request: input.request, client: input.actor.client, artifactType: input.artifactType.name, - ...(onSubmission ? { onSubmission } : {}), + ...(isImpl && input.branch ? { implementation: { gitUrl: input.bundle.project.gitUrl, branch: input.branch, ...(input.prev ? { prev: input.prev } : {}) } } : {}), ...(deps.now ? { now: deps.now } : {}), } let sidecarSocket: string @@ -187,15 +164,20 @@ export async function runTurn(input: TurnInput, deps: TurnDeps): Promise = isImpl ? { RADIAL_BASE_COMMIT: baseCommit, - GIT_AUTHOR_NAME: input.actor.session.handle, + RADIAL_ARTIFACT_URI: artifactUri, + RADIAL_BRANCH: input.branch ?? '', + RADIAL_BASE_BRANCH: input.bundle.project.defaultBranch, + RADIAL_AGENT_NAME: input.actor.session.handle, + RADIAL_AGENT_DID: input.actor.did, + RADIAL_AGENT_EMAIL: `${input.actor.session.handle}@noreply.radial`, + GIT_AUTHOR_NAME: `${input.actor.session.handle} (${input.actor.did})`, GIT_AUTHOR_EMAIL: `${input.actor.session.handle}@noreply.radial`, - GIT_COMMITTER_NAME: input.actor.session.handle, + GIT_COMMITTER_NAME: `${input.actor.session.handle} (${input.actor.did})`, GIT_COMMITTER_EMAIL: `${input.actor.session.handle}@noreply.radial`, } : {} @@ -206,14 +188,12 @@ export async function runTurn(input: TurnInput, deps: TurnDeps): Promise 0) { return { outcome: 'awaiting_input', label } } return { outcome: 'crashed', label } } finally { await server?.close() // idempotent - if (!keepRunDir) { - // A non-submitted turn's runDir is ephemeral; log (never swallow) a cleanup failure so a - // leaked directory that would break the next attempt's clone is at least visible. - await rm(input.runDir, { recursive: true, force: true }).catch((error: unknown) => { - deps.log?.(`failed to remove runDir ${input.runDir}: ${error instanceof Error ? error.message : String(error)}`) - }) - } + // Every turn runDir is ephemeral; log (never swallow) a cleanup failure so a + // leaked directory that would break the next attempt's clone is at least visible. + await rm(input.runDir, { recursive: true, force: true }).catch((error: unknown) => { + deps.log?.(`failed to remove runDir ${input.runDir}: ${error instanceof Error ? error.message : String(error)}`) + }) } } diff --git a/packages/daemon/test/bundle-writer.test.mjs b/packages/daemon/test/bundle-writer.test.mjs index 51b2e9a..86c1240 100644 --- a/packages/daemon/test/bundle-writer.test.mjs +++ b/packages/daemon/test/bundle-writer.test.mjs @@ -1,10 +1,10 @@ import assert from 'node:assert/strict' import { spawnSync } from 'node:child_process' -import { mkdir, mkdtemp, readFile, symlink, writeFile } from 'node:fs/promises' +import { mkdtemp, readFile, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { it } from 'node:test' -import { assertGitUrlAllowed, checkoutCommit, checkoutRepo, snapshotBundleFile, writeBundle } from '../dist/index.js' +import { assertGitUrlAllowed, checkoutCommit, checkoutRepo, writeBundle } from '../dist/index.js' const bundle = { request: { uri: 'at://req/1', cid: 'req-cid', type: 'plan', basedOn: [] }, @@ -95,6 +95,45 @@ it('checkoutRepo throws with stderr context on a non-zero git exit', async () => ) }) +it('private GitHub checkout supplies its token only through the child environment and redacts failures', async () => { + const token = 'token-that-must-not-escape' + const calls = [] + const exec = async (argv, opts) => { + calls.push({ argv, opts }) + return { code: 128, stdout: '', stderr: `authentication failed: ${token}` } + } + await assert.rejects( + checkoutRepo({ gitUrl: 'https://github.com/acme/private.git', ref: 'main', dest: '/tmp/private', githubToken: token, exec }), + (error) => !String(error).includes(token) && /redacted/.test(String(error)), + ) + assert.equal(calls[0].argv.join(' ').includes(token), false) + assert.equal(calls[0].opts.env.GH_TOKEN, token) + assert.equal(calls[0].opts.env.GIT_CONFIG_KEY_0, 'credential.helper') + assert.match(calls[0].opts.env.GIT_CONFIG_VALUE_0, /host.*github\.com/) +}) + +it('does not expose a GitHub token to a non-GitHub checkout', async () => { + const calls = [] + const exec = async (argv, opts) => { + calls.push({ argv, opts }) + return argv.includes('clone') + ? { code: 0, stdout: '', stderr: '' } + : { code: 0, stdout: 'deadbeef\n', stderr: '' } + } + + await checkoutRepo({ + gitUrl: 'https://git.example.test/acme/private.git', + ref: 'main', + dest: '/tmp/non-github-private', + githubToken: 'github-only-secret', + exec, + }) + + assert.match(calls[0].argv.join(' '), /-c credential\.helper=/) + assert.equal(calls[0].opts.env.GH_TOKEN, undefined) + assert.equal(calls[0].opts.env.GIT_CONFIG_COUNT, undefined) +}) + const SHA = '1234567890abcdef1234567890abcdef12345678' it('checkoutCommit inits, fetches the exact sha, and checks out FETCH_HEAD (hardened, GIT_* scrubbed)', async () => { @@ -246,34 +285,3 @@ it('checkoutRepo clones a real local repo over file:// (opt-in) and resolves a r const readme = await readFile(join(dest, 'README.md'), 'utf8') assert.equal(readme, '# test repo\n') }) - -// --- snapshotBundleFile: hardened capture of the container-written bundle (finding 2) --- - -it('snapshotBundleFile copies a regular file and rejects symlink/FIFO/oversize', async () => { - const dir = await mkdtemp(join(tmpdir(), 'radial-snap-')) - const src = join(dir, 'export', 'impl.bundle') - const dest = join(dir, 'snap', 'impl.bundle') - await mkdir(join(dir, 'export'), { recursive: true }) - - // Happy path: a regular file is copied byte-for-byte into the daemon-owned dest. - await writeFile(src, 'BUNDLE-BYTES') - await snapshotBundleFile(src, dest) - assert.equal(await readFile(dest, 'utf8'), 'BUNDLE-BYTES') - - // Oversize is rejected. - await assert.rejects(snapshotBundleFile(src, join(dir, 'snap2', 'b'), 4), /exceeds the 4-byte cap/) - - // A symlink at the bundle path is rejected (O_NOFOLLOW). - const secret = join(dir, 'secret') - await writeFile(secret, 'SECRET') - const linkSrc = join(dir, 'export', 'link.bundle') - await symlink(secret, linkSrc) - await assert.rejects(snapshotBundleFile(linkSrc, join(dir, 'snap3', 'b')), /regular file/) - - // A FIFO is rejected (fstat is not a regular file). Skipped if mkfifo is unavailable. - const fifo = join(dir, 'export', 'fifo.bundle') - const made = spawnSync('mkfifo', [fifo]) - if (made.status === 0) { - await assert.rejects(snapshotBundleFile(fifo, join(dir, 'snap4', 'b')), /regular file/) - } -}) diff --git a/packages/daemon/test/check-runner.test.mjs b/packages/daemon/test/check-runner.test.mjs index 0c81821..085a073 100644 --- a/packages/daemon/test/check-runner.test.mjs +++ b/packages/daemon/test/check-runner.test.mjs @@ -139,7 +139,7 @@ it('locks the container down: no env secrets, no network, read-write /work, no b const spec = captured.spec assert.deepEqual(spec.env, {}, 'env must carry NOTHING — no secrets') - assert.equal(spec.network, 'none') + assert.equal(spec.network, undefined) assert.equal(spec.user, '1000:1000') assert.equal(spec.readOnlyRootfs, true) assert.equal(spec.memory, '4g') diff --git a/packages/daemon/test/cli.test.mjs b/packages/daemon/test/cli.test.mjs index 012421d..284c1ec 100644 --- a/packages/daemon/test/cli.test.mjs +++ b/packages/daemon/test/cli.test.mjs @@ -7,22 +7,7 @@ import { resolveRunConfig } from '../dist/cli.js' // `resolveRunConfig` — the thing `runCommand` consults to decide whether to enforce egress at all // — defaults to enforcing it, per the "no bridge fallback" design decision (§13): an operator who // configures nothing gets the enforced sandbox, not silent open egress. -it('resolveRunConfig defaults egress to enforced, with the internal/egress network + proxy image defaults', () => { - const resolved = resolveRunConfig({ spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'] }) - assert.equal(resolved.egress, true) - assert.equal(resolved.network, 'radial-internal') - assert.equal(resolved.egressNetwork, 'radial-egress') - assert.equal(resolved.proxyImage, 'radial-proxy:latest') -}) -it('resolveRunConfig honors an explicit "egress": false opt-out, and drops the network default with it', () => { - const resolved = resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - egress: false, - }) - assert.equal(resolved.egress, false) - assert.equal(resolved.network, undefined) -}) it('resolveRunConfig defaults merge poll timing and clamps invalid values', () => { const SPACE = 'at://did:plc:human/com.disnetdev.radial.space/space1' @@ -42,67 +27,18 @@ it('resolveRunConfig defaults merge poll timing and clamps invalid values', () = assert.equal(clamped.mergePollIntervalMs, 120_000) assert.equal(clamped.mergePollBackoffMaxMs, 120_000) }) - it('resolveRunConfig defaults turnTransport to "unix"', () => { const resolved = resolveRunConfig({ spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'] }) assert.equal(resolved.turnTransport, 'unix') }) -it('resolveRunConfig fails closed: turnTransport "tcp" without an explicit "egress": false throws', () => { - assert.throws( - () => - resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - turnTransport: 'tcp', - }), - /turn transport "tcp" requires "egress": false to be set explicitly/, - ) - // Even if egress happens to be truthy-explicit, only an explicit `false` satisfies the gate. - assert.throws( - () => - resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - turnTransport: 'tcp', - egress: true, - }), - /turn transport "tcp" requires "egress": false/, - ) -}) -it('resolveRunConfig accepts turnTransport "tcp" alongside an explicit "egress": false', () => { - const resolved = resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - turnTransport: 'tcp', - egress: false, - }) - assert.equal(resolved.turnTransport, 'tcp') - assert.equal(resolved.egress, false) -}) -it('resolveRunConfig keeps an explicit run.network even with egress enforced, instead of overriding it', () => { - const resolved = resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - network: 'operator-custom-net', - }) - assert.equal(resolved.egress, true) - assert.equal(resolved.network, 'operator-custom-net') -}) // FIX #12: an explicit `allowlist: []` (deny-all) is not the same as "unset", and `?? []`-style // fallbacks must not conflate them — `[]` is non-nullish, so `run.allowlist ?? [default]` already // preserves it correctly; this pins that down against a regression. -it('resolveRunConfig preserves an explicit empty allowlist (deny-all) rather than defaulting it to Anthropic', () => { - const resolved = resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - allowlist: [], - }) - assert.deepEqual(resolved.allowlist, []) -}) -it('resolveRunConfig defaults the allowlist to Anthropic only when truly unset', () => { - const resolved = resolveRunConfig({ spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'] }) - assert.deepEqual(resolved.allowlist, ['api.anthropic.com']) -}) // FIX #11: run.memory threads through resolveRunConfig with no default applied at this layer // (turn.ts supplies the 4g floor when unset). @@ -129,24 +65,3 @@ it('resolveRunConfig applies check-runner defaults (checkImage falls back to the // checkImage defaults to the resolved run image when not set explicitly. assert.equal(resolved.checkImage, 'radial-turn:custom') }) - -it('resolveRunConfig honors an explicit checkImage distinct from the run image', () => { - const resolved = resolveRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - image: 'radial-turn:latest', - checkImage: 'radial-check:latest', - checkConcurrency: 4, - }) - assert.equal(resolved.checkImage, 'radial-check:latest') - assert.equal(resolved.checkConcurrency, 4) -}) - -it('resolveRunConfig defaults finishConcurrency to 2 and leaves forge unset unless configured', () => { - const SPACE = 'at://did:plc:human/com.disnetdev.radial.space/space1' - const defaults = resolveRunConfig({ spaces: [SPACE] }) - assert.equal(defaults.finishConcurrency, 2) - assert.equal(defaults.forge, undefined) - const withForge = resolveRunConfig({ spaces: [SPACE], forge: { kind: 'github' }, finishConcurrency: 5 }) - assert.deepEqual(withForge.forge, { kind: 'github' }) - assert.equal(withForge.finishConcurrency, 5) -}) diff --git a/packages/daemon/test/config.test.mjs b/packages/daemon/test/config.test.mjs index 44f4ca4..a530ea9 100644 --- a/packages/daemon/test/config.test.mjs +++ b/packages/daemon/test/config.test.mjs @@ -220,11 +220,6 @@ it('parseRunConfig leaves run.turnTransport unset when not configured', () => { assert.equal('turnTransport' in parsed, false) }) -it('parseRunConfig preserves an explicit empty run.allowlist (deny-all) rather than dropping it', () => { - const parsed = parseRunConfig({ spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], allowlist: [] }) - assert.deepEqual(parsed.allowlist, []) - assert.equal('allowlist' in parsed, true) -}) it('refuses to clobber an existing config unless forced', async () => { const directory = await mkdtemp(join(tmpdir(), 'radial-clobber-')) @@ -236,27 +231,3 @@ it('refuses to clobber an existing config unless forced', async () => { await writeConfig(path, buildDefaultConfig('second.example'), { force: true }) assert.equal((await loadConfig(path)).identifier, 'second.example') }) - -// --- Forge + finish concurrency (Phase 4 issue 1) --------------------------- - -it('parseRunConfig accepts a github forge block and a finishConcurrency', () => { - const parsed = parseRunConfig({ - spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'], - forge: { kind: 'github' }, - finishConcurrency: 4, - }) - assert.deepEqual(parsed.forge, { kind: 'github' }) - assert.equal(parsed.finishConcurrency, 4) -}) - -it('parseRunConfig rejects an unknown forge kind and a non-number finishConcurrency', () => { - assert.throws(() => parseRunConfig({ spaces: ['at://x'], forge: { kind: 'gitlab' } }), /run\.forge\.kind must be "github"/) - assert.throws(() => parseRunConfig({ spaces: ['at://x'], forge: 'github' }), /run\.forge must be an object/) - assert.throws(() => parseRunConfig({ spaces: ['at://x'], finishConcurrency: 'lots' }), /run\.finishConcurrency must be a number/) -}) - -it('parseRunConfig leaves forge unset and finishConcurrency undefined when not configured', () => { - const parsed = parseRunConfig({ spaces: ['at://did:plc:human/com.disnetdev.radial.space/space1'] }) - assert.equal(parsed.forge, undefined) - assert.equal(parsed.finishConcurrency, undefined) -}) diff --git a/packages/daemon/test/container.test.mjs b/packages/daemon/test/container.test.mjs index d2e0473..037f48d 100644 --- a/packages/daemon/test/container.test.mjs +++ b/packages/daemon/test/container.test.mjs @@ -11,7 +11,7 @@ const baseSpec = { { source: '/host/bundle', target: '/bundle', readOnly: true }, { source: '/host/checkout', target: '/workspace', readOnly: false }, ], - network: 'radial-egress', + network: 'radial-bridge', workdir: '/workspace', timeoutMs: 60_000, tmpfs: ['/tmp', '/run'], @@ -50,7 +50,7 @@ it('dockerRunArgs hardens the container and preserves deterministic ordering', ( assert.equal(args[nameIndex + 1], 'radial-turn-1') const networkIndex = args.indexOf('--network') - assert.equal(args[networkIndex + 1], 'radial-egress') + assert.equal(args[networkIndex + 1], 'radial-bridge') const workdirIndex = args.indexOf('-w') assert.equal(args[workdirIndex + 1], '/workspace') @@ -61,8 +61,10 @@ it('dockerRunArgs hardens the container and preserves deterministic ordering', ( assert.ok(!args.includes('/host/checkout:/workspace:ro')) assert.ok(args.includes('-e')) - assert.ok(args.includes('FOO=bar')) - assert.ok(args.includes('BAZ=qux')) + assert.ok(args.includes('FOO')) + assert.ok(args.includes('BAZ')) + assert.equal(args.includes('FOO=bar'), false) + assert.equal(args.includes('BAZ=qux'), false) const imageIndex = args.indexOf('radial/harness:latest') assert.ok(imageIndex !== -1) diff --git a/packages/daemon/test/dispatch.test.mjs b/packages/daemon/test/dispatch.test.mjs index 6c449b5..6d1c9ec 100644 --- a/packages/daemon/test/dispatch.test.mjs +++ b/packages/daemon/test/dispatch.test.mjs @@ -8,7 +8,16 @@ import { it } from 'node:test' import { createSession, CredentialClient } from '../../atproto/dist/index.js' import { COLLECTIONS, MemoryRecordStore, materialize } from '../../core/dist/index.js' import { LocalPds } from '../../atproto/test/local-pds.mjs' -import { ClaudeCodeHarness, FakeContainerRunner, TurnDispatcher, TurnLedger, planArtifactRkey, runTurn, selectDispatchable } from '../dist/index.js' +import { + ClaudeCodeHarness, + FakeContainerRunner, + TurnDispatcher, + TurnLedger, + implBranchName, + planArtifactRkey, + runTurn, + selectDispatchable, +} from '../dist/index.js' import { reconcileOrphan, reconcileOrphans } from '../dist/cli.js' const ROOT = 'did:plc:root' @@ -441,6 +450,149 @@ it('flags >1 same-goal implementation predecessors as ambiguous (still returned, ledger.close() }) +it('dispatcher reuses a predecessor branch only after observing a clean same-repository open PR', async () => { + const { goal } = buildScenario() + const predecessor = implPredecessor(goal, 'impl-v1', { + branch: 'radial/impl-v1', + pr: 'https://github.com/o/r/pull/7', + }) + const { records, request } = buildScenario({ + requestOverrides: { type: 'implementation', basedOn: [ref(predecessor.art)] }, + extra: [IMPL_TYPE, predecessor.req, predecessor.art], + }) + const githubRecords = records.map((record) => + record.collection === COLLECTIONS.project + ? { ...record, value: { ...record.value, gitUrl: 'https://github.com/o/r.git' } } + : record, + ) + const index = materialize(store(githubRecords), { spaceUri: SPACE_URI }) + const actors = registryFor([actorFor(AGENT, ['implementation'])]) + const ledger = new TurnLedger() + let captured + const dispatcher = new TurnDispatcher({ + ledger, + concurrency: 1, + runDirFor: () => '/tmp/radial-v2-reuse', + image: 'radial-turn:test', + timeoutMs: 30_000, + allowedSchemes: ['https'], + implementationEnabled: true, + forge: { + getPullRequestState: async () => ({ + state: 'open', + headRef: 'radial/impl-v1', + headRepoFullName: 'o/r', + baseRef: 'main', + }), + }, + runTurn: async (input) => { + captured = input + return { + outcome: 'fulfilled', + acceptedRef: { uri: 'at://did:plc:agent/artifact/impl', cid: 'cid-impl' }, + label: 'radial.turn.impl', + } + }, + }) + + dispatcher.pump(index, actors) + await dispatcher.drain() + + assert.equal(captured.checkoutRef, 'radial/impl-v1') + assert.equal(captured.branch, 'radial/impl-v1') + assert.deepEqual(captured.prev, ref(predecessor.art)) + assert.equal(ledger.get(request.uri).state, 'fulfilled') + ledger.close() +}) + +it('dispatcher opens a fresh reserved branch when predecessor PR observation is unsafe', async () => { + const { goal } = buildScenario() + const predecessor = implPredecessor(goal, 'impl-v1', { + branch: 'radial/impl-v1', + pr: 'https://github.com/o/r/pull/7', + }) + const { records, request } = buildScenario({ + requestOverrides: { type: 'implementation', basedOn: [ref(predecessor.art)] }, + extra: [IMPL_TYPE, predecessor.req, predecessor.art], + }) + const githubRecords = records.map((record) => + record.collection === COLLECTIONS.project + ? { ...record, value: { ...record.value, gitUrl: 'https://github.com/o/r.git' } } + : record, + ) + const index = materialize(store(githubRecords), { spaceUri: SPACE_URI }) + const actors = registryFor([actorFor(AGENT, ['implementation'])]) + const ledger = new TurnLedger() + let captured + const dispatcher = new TurnDispatcher({ + ledger, + concurrency: 1, + runDirFor: () => '/tmp/radial-v2-fresh', + image: 'radial-turn:test', + timeoutMs: 30_000, + allowedSchemes: ['https'], + implementationEnabled: true, + forge: { + getPullRequestState: async () => ({ + state: 'open', + headRef: 'radial/impl-v1', + headRepoFullName: 'attacker/fork', + baseRef: 'main', + }), + }, + runTurn: async (input) => { + captured = input + return { + outcome: 'fulfilled', + acceptedRef: { uri: 'at://did:plc:agent/artifact/impl', cid: 'cid-impl' }, + label: 'radial.turn.impl', + } + }, + }) + + dispatcher.pump(index, actors) + await dispatcher.drain() + + assert.equal(captured.checkoutRef, undefined) + assert.equal(captured.branch, implBranchName(request.uri, request.cid)) + assert.deepEqual(captured.prev, ref(predecessor.art)) + ledger.close() +}) + +it('dispatcher skips implementation requests without GitHub possession', () => { + const { records, request } = buildScenario({ + requestOverrides: { type: 'implementation' }, + extra: [IMPL_TYPE], + }) + const index = materialize(store(records), { spaceUri: SPACE_URI }) + const actors = registryFor([actorFor(AGENT, ['implementation'])]) + const ledger = new TurnLedger() + const logs = [] + let launched = false + const dispatcher = new TurnDispatcher({ + ledger, + concurrency: 1, + runDirFor: () => '/tmp/radial-disabled', + image: 'radial-turn:test', + timeoutMs: 30_000, + allowedSchemes: ['https'], + implementationEnabled: false, + runTurn: async () => { + launched = true + return { outcome: 'crashed', label: 'radial.turn.impl' } + }, + log: (message) => logs.push(message), + }) + + dispatcher.pump(index, actors) + + assert.equal(launched, false) + assert.equal(dispatcher.inFlight, 0) + assert.equal(ledger.get(request.uri), undefined) + assert.ok(logs.some((message) => message.includes('GitHub authentication is unavailable'))) + ledger.close() +}) + // --- FIX: unassigned awaiting-input guard keys off the SELECTED actor DID ---- // The stale-index guard previously matched question authors against `request.value.assignee`, which // is undefined for an unassigned request — so an unassigned request's ANSWERED question could never @@ -828,172 +980,6 @@ it('startup orphan reconciliation kills the tracked container and resets the led ledger.close() }) -// --- Dispatcher launch: v2 branch/PR decision + ambiguous rejection --------- - -function implDispatcher(ledger, { forge, runTurn }) { - return new TurnDispatcher({ - ledger, - concurrency: 1, - runDirFor: (uri) => join('/tmp', 'radial-impl-launch', uri.replace(/[^a-z0-9]/gi, '_')), - image: 'radial-turn:test', - timeoutMs: 30_000, - allowedSchemes: ['https'], - ...(forge ? { forge } : {}), - runTurn, - }) -} - -// The project's gitUrl must be a GitHub repo for v2 PR validation to parse; buildScenario uses a -// non-github URL, so remap it (and match the predecessor PR's owner/repo to it). -function githubProject(records) { - return records.map((r) => - r.collection === COLLECTIONS.project ? { ...r, value: { ...r.value, gitUrl: 'https://github.com/o/r.git' } } : r, - ) -} -const CAPTURED_SUBMISSION = { commit: 'c'.repeat(40), body: 'impl body' } -function captureRunTurn(box) { - return async (input) => { - box.captured = input - await input.onSubmission?.(CAPTURED_SUBMISSION, 'b'.repeat(40), '/state/impl.bundle') - return { outcome: 'submitted', submission: CAPTURED_SUBMISSION, label: 'l' } - } -} - -it('v2 open predecessor PR (all guards pass): checks out + reuses the branch and stamps prev', async () => { - const { goal } = buildScenario() - const pred = implPredecessor(goal, 'impl-v1', { branch: 'radial/impl-v1', pr: 'https://github.com/o/r/pull/7' }) - const { records, request } = buildScenario({ - requestOverrides: { type: 'implementation', basedOn: [ref(pred.art)] }, - extra: [IMPL_TYPE, pred.req, pred.art], - }) - const index = materialize(store(githubProject(records)), { spaceUri: SPACE_URI }) - const ledger = new TurnLedger() - const actors = registryFor([actorFor(AGENT, ['implementation'])]) - const box = {} - const dispatcher = implDispatcher(ledger, { - forge: { getPullRequest: async () => ({ state: 'open', headRef: 'radial/impl-v1', headRepoFullName: 'o/r', baseRef: 'main', headSha: 'x' }) }, - runTurn: captureRunTurn(box), - }) - dispatcher.pump(index, actors) - await dispatcher.drain() - assert.equal(box.captured.checkoutRef, 'radial/impl-v1') // predecessor-branch checkout intent - const job = ledger.submission(request.uri) - assert.equal(job.branch, 'radial/impl-v1') // reuse the predecessor branch (fast-forward push) - assert.equal(job.bundlePath, '/state/impl.bundle') - assert.deepEqual(job.prev, ref(pred.art)) - assert.equal(ledger.get(request.uri).state, 'finishing') - ledger.close() -}) - -it('v2 merged predecessor PR: default-branch checkout, a fresh branch, prev still stamped', async () => { - const { goal } = buildScenario() - const pred = implPredecessor(goal, 'impl-v1', { branch: 'radial/impl-v1', pr: 'https://github.com/o/r/pull/7' }) - const { records, request } = buildScenario({ - requestOverrides: { type: 'implementation', basedOn: [ref(pred.art)] }, - extra: [IMPL_TYPE, pred.req, pred.art], - }) - const index = materialize(store(githubProject(records)), { spaceUri: SPACE_URI }) - const ledger = new TurnLedger() - const actors = registryFor([actorFor(AGENT, ['implementation'])]) - const box = {} - const dispatcher = implDispatcher(ledger, { - forge: { getPullRequest: async () => ({ state: 'merged', headRef: 'radial/impl-v1', headRepoFullName: 'o/r', baseRef: 'main', headSha: 'x' }) }, - runTurn: captureRunTurn(box), - }) - dispatcher.pump(index, actors) - await dispatcher.drain() - assert.equal(box.captured.checkoutRef, undefined) // default-branch checkout - const job = ledger.submission(request.uri) - assert.match(job.branch, /^radial\/impl-[0-9a-f]{12}$/) // fresh deterministic branch - assert.notEqual(job.branch, 'radial/impl-v1') - assert.deepEqual(job.prev, ref(pred.art)) - ledger.close() -}) - -it('v2: a predecessor branch OUTSIDE the radial/impl- namespace is never reused (fresh branch)', async () => { - const { goal } = buildScenario() - // links.branch "main" is not in the daemon's namespace: no forge call, fresh branch. - const pred = implPredecessor(goal, 'impl-v1', { branch: 'main', pr: 'https://github.com/o/r/pull/7' }) - const { records, request } = buildScenario({ - requestOverrides: { type: 'implementation', basedOn: [ref(pred.art)] }, - extra: [IMPL_TYPE, pred.req, pred.art], - }) - const index = materialize(store(githubProject(records)), { spaceUri: SPACE_URI }) - const ledger = new TurnLedger() - const actors = registryFor([actorFor(AGENT, ['implementation'])]) - const box = {} - let forgeCalled = false - const dispatcher = implDispatcher(ledger, { - forge: { getPullRequest: async () => { forgeCalled = true; return { state: 'open', headRef: 'main', headRepoFullName: 'o/r', baseRef: 'main', headSha: 'x' } } }, - runTurn: captureRunTurn(box), - }) - dispatcher.pump(index, actors) - await dispatcher.drain() - assert.equal(forgeCalled, false) // never even queried a non-radial branch - assert.equal(box.captured.checkoutRef, undefined) - assert.match(ledger.submission(request.uri).branch, /^radial\/impl-[0-9a-f]{12}$/) - ledger.close() -}) - -it('v2: a predecessor PR whose head ref does not match the branch is not reused (fresh branch)', async () => { - const { goal } = buildScenario() - const pred = implPredecessor(goal, 'impl-v1', { branch: 'radial/impl-v1', pr: 'https://github.com/o/r/pull/7' }) - const { records, request } = buildScenario({ - requestOverrides: { type: 'implementation', basedOn: [ref(pred.art)] }, - extra: [IMPL_TYPE, pred.req, pred.art], - }) - const index = materialize(store(githubProject(records)), { spaceUri: SPACE_URI }) - const ledger = new TurnLedger() - const actors = registryFor([actorFor(AGENT, ['implementation'])]) - const box = {} - const dispatcher = implDispatcher(ledger, { - // Open PR, but its head ref is a DIFFERENT branch than the artifact links (a redirect attempt). - forge: { getPullRequest: async () => ({ state: 'open', headRef: 'radial/impl-other', headRepoFullName: 'o/r', baseRef: 'main', headSha: 'x' }) }, - runTurn: captureRunTurn(box), - }) - dispatcher.pump(index, actors) - await dispatcher.drain() - assert.equal(box.captured.checkoutRef, undefined) // not reused - assert.match(ledger.submission(request.uri).branch, /^radial\/impl-[0-9a-f]{12}$/) - ledger.close() -}) - -it('ambiguous predecessors: posts an explanation, gives up, and never runs a turn', async () => { - const { goal } = buildScenario() - const p1 = implPredecessor(goal, 'impl-a', { branch: 'radial/impl-a', pr: 'https://github.com/o/r/pull/1' }) - const p2 = implPredecessor(goal, 'impl-b', { branch: 'radial/impl-b', pr: 'https://github.com/o/r/pull/2' }) - const { records, request } = buildScenario({ - requestOverrides: { type: 'implementation', basedOn: [ref(p1.art), ref(p2.art)] }, - extra: [IMPL_TYPE, p1.req, p1.art, p2.req, p2.art], - }) - const index = materialize(store(records), { spaceUri: SPACE_URI }) - const ledger = new TurnLedger() - const posted = [] - const client = { - create: async (collection, value) => { - posted.push({ collection, value }) - return { uri: 'at://did:plc:agent/msg/1', cid: 'msgcid' } - }, - } - const actors = registryFor([{ ...actorFor(AGENT, ['implementation']), client }]) - let ran = false - const dispatcher = implDispatcher(ledger, { - runTurn: async () => { - ran = true - return { outcome: 'crashed', label: 'l' } - }, - }) - dispatcher.pump(index, actors) - await dispatcher.drain() - assert.equal(ran, false) // no container turn - assert.equal(posted.length, 1) - assert.equal(posted[0].collection, COLLECTIONS.message) - assert.match(posted[0].value.body, /more than one implementation/) - assert.deepEqual(posted[0].value.re, { uri: request.uri, cid: request.cid }) - assert.equal(ledger.get(request.uri).state, 'gave_up') // request stays open in the fold; ledger gave up - ledger.close() -}) - // --- finding 9: unassigned request blocks on an unanswered question ---------- it('unassigned request with an unanswered agent question is not dispatched (fresh ledger)', () => { @@ -1025,58 +1011,3 @@ it('unassigned request with an unanswered agent question is not dispatched (fres assert.deepEqual(uris(selectDispatchable(index2, actors, ledger)), [base.request.uri]) ledger.close() }) - -// --- finding 5: a late runner error never demotes a finishing row ----------- - -it('a runner error AFTER the submission callback leaves the row finishing (crash-window handoff)', async () => { - const { goal } = buildScenario() - const pred = undefined - void pred - const { records, request } = buildScenario({ requestOverrides: { type: 'implementation' }, extra: [IMPL_TYPE] }) - const index = materialize(store(records), { spaceUri: SPACE_URI }) - const ledger = new TurnLedger() - const actors = registryFor([actorFor(AGENT, ['implementation'])]) - const dispatcher = new TurnDispatcher({ - ledger, - concurrency: 1, - runDirFor: (uri) => join('/tmp', 'radial-crashwin', uri.replace(/[^a-z0-9]/gi, '_')), - image: 'radial-turn:test', - timeoutMs: 30_000, - allowedSchemes: ['https'], - runTurn: async (input) => { - // Submit (persists the finishing row), then the runner throws — as if it died right after. - await input.onSubmission?.({ commit: 'c'.repeat(40), body: 'b' }, 'a'.repeat(40), '/state/impl.bundle') - throw new Error('docker died after submit') - }, - log: () => {}, - }) - dispatcher.pump(index, actors) - await dispatcher.drain() - // markCrashed was called by the catch path, but the finishing row was NOT clobbered. - assert.equal(ledger.get(request.uri).state, 'finishing') - assert.ok(ledger.submission(request.uri)) // handoff payload intact - void goal - ledger.close() -}) - -// --- finding 3b: reconcileOrphans kills finishing containers, keeps the row -- - -it('reconcileOrphans kills a finishing row\'s container but keeps the row for the finish pump', async () => { - const { request } = buildScenario({ requestOverrides: { type: 'implementation' }, extra: [IMPL_TYPE] }) - const ledger = new TurnLedger() - const label = 'radial.turn.orphanfinish' - ledger.markRunning(request.uri, request.cid, { containerLabel: label, checkoutPath: '/tmp/x' }) - ledger.markSubmitted(request.uri, request.cid, { - goal: request.value.goal, actorDid: AGENT, gitUrl: 'https://github.com/o/r.git', - base: 'main', branch: 'radial/impl-x', baseCommit: 'a'.repeat(40), bundlePath: '/s/impl.bundle', commit: 'b'.repeat(40), body: 'b', title: 't', - }) - const runner = new FakeContainerRunner(async () => ({ exitCode: 0, timedOut: false })) - await runner.run({ label, image: 'x', argv: [], env: {}, mounts: [], timeoutMs: 1000 }) - - await reconcileOrphans(runner, ledger) - - assert.deepEqual(await runner.listByLabel(label), []) // container killed - assert.equal(ledger.get(request.uri).state, 'finishing') // row preserved (never markCrashed) - assert.ok(ledger.submission(request.uri)) - ledger.close() -}) diff --git a/packages/daemon/test/docker-smoke.test.mjs b/packages/daemon/test/docker-smoke.test.mjs index 8e8a00f..1180e91 100644 --- a/packages/daemon/test/docker-smoke.test.mjs +++ b/packages/daemon/test/docker-smoke.test.mjs @@ -5,7 +5,7 @@ // Skipped entirely unless RADIAL_DOCKER_TESTS=1 (node:test's `skip` option — a clean skip, not a // failure, and no docker/child_process work happens at all when skipped). To run it for real: // -// pnpm images # builds radial-turn:latest (and radial-proxy) +// pnpm images # builds radial-turn:latest // RADIAL_DOCKER_TESTS=1 node --test packages/daemon/test/docker-smoke.test.mjs // // It intentionally never invokes the real `claude` CLI — the container command is a deterministic @@ -23,14 +23,11 @@ import { COLLECTIONS } from '../../core/dist/index.js' import { LocalPds } from '../../atproto/test/local-pds.mjs' import { DockerRunner, - FakeForge, TurnSocketServer, checkrunRkey, - finishImplementationTurn, implArtifactRkey, runCheckRun, } from '../dist/index.js' -import { chmod } from 'node:fs/promises' const RUN_DOCKER_TESTS = process.env.RADIAL_DOCKER_TESTS === '1' const TURN_IMAGE = process.env.RADIAL_TURN_IMAGE ?? 'radial-turn:latest' @@ -240,13 +237,11 @@ describe('docker smoke: check runner against a real Docker daemon', { skip: !RUN }) }) -// Implementation turn end-to-end: a real container commits + bundles + submits over the socket, then -// the daemon-side finish step pushes to a LOCAL bare repo (standing in for GitHub) and opens a PR via -// a FakeForge before writing the artifact record. Gated on RADIAL_DOCKER_TESTS=1 like the others. -describe('docker smoke: implementation turn produces a bundle + daemon finish', { skip: !RUN_DOCKER_TESTS }, () => { - it('a container commits + bundles + submits; the finish step pushes, opens a PR, and writes the record', async () => { +describe('docker smoke: synchronous implementation submit', { skip: !RUN_DOCKER_TESTS }, () => { + it('a container has gh, commits, and immediately writes the validated implementation artifact', async () => { const { pds, client } = await makeSignedClient() const token = 'radial-docker-smoke-impl-token' + const branch = 'radial/impl-smoke123456' const gitEnv = { GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t' } const git = (cwd, ...args) => { const r = spawnSync('git', args, { cwd, encoding: 'utf8', env: { ...process.env, ...gitEnv } }) @@ -255,50 +250,30 @@ describe('docker smoke: implementation turn produces a bundle + daemon finish', } await withTmpDir(async (runDir) => { - // Local bare origin with a base commit on main (stands in for the project's GitHub repo). - const origin = join(runDir, 'origin.git') - const seed = join(runDir, 'seed') - await mkdir(seed, { recursive: true }) - git(runDir, 'init', '-q', '--bare', origin) - git(seed, 'init', '-q', '.') - await writeFile(join(seed, 'README'), 'base\n') - git(seed, 'add', '-A') - git(seed, 'commit', '-q', '-m', 'base') - git(seed, 'branch', '-M', 'main') - git(seed, 'remote', 'add', 'origin', origin) - git(seed, 'push', '-q', 'origin', 'main') - const workDir = join(runDir, 'work') - const exportDir = join(runDir, 'export') const socketDir = join(runDir, 'run') - git(runDir, 'clone', '-q', origin, workDir) - const baseCommit = git(workDir, 'rev-parse', 'HEAD') - await mkdir(exportDir, { recursive: true }) + await mkdir(workDir, { recursive: true }) await mkdir(socketDir, { recursive: true }) - // Let the uid-1000 container write the tree, .git, and /export. - await chmod(exportDir, 0o777) spawnSync('chmod', ['-R', '0777', workDir]) - let captured const server = new TurnSocketServer(join(socketDir, 'turn.sock'), { token, request: REQUEST, client, artifactType: 'implementation', - onSubmission: (submission) => { - captured = submission - }, + implementation: { gitUrl: 'https://github.com/acme/widget.git', branch }, }) await server.listen() try { const script = [ 'cd /work', + 'gh --version', + 'git init -q', 'echo change > impl.txt', 'git add -A', 'git commit -q -m impl', - 'git bundle create /export/impl.bundle "$RADIAL_BASE_COMMIT..HEAD"', 'printf "impl summary" > /tmp/body.md', - 'radial artifact submit --body-file /tmp/body.md --commit "$(git rev-parse HEAD)"', + `radial artifact submit --body-file /tmp/body.md --branch ${branch} --commit "$(git rev-parse HEAD)" --pr https://github.com/acme/widget/pull/7`, ].join(' && ') const result = await new DockerRunner().run({ label: 'radial-docker-smoke-impl', @@ -307,12 +282,10 @@ describe('docker smoke: implementation turn produces a bundle + daemon finish', env: { RADIAL_SIDECAR_SOCKET: '/run/radial/turn.sock', RADIAL_TURN_TOKEN: token, - RADIAL_BASE_COMMIT: baseCommit, ...gitEnv, }, mounts: [ { source: workDir, target: '/work', readOnly: false }, - { source: exportDir, target: '/export', readOnly: false }, { source: socketDir, target: '/run/radial', readOnly: false }, ], workdir: '/work', @@ -323,43 +296,18 @@ describe('docker smoke: implementation turn produces a bundle + daemon finish', }) assert.equal(result.timedOut, false) assert.equal(result.exitCode, 0, `expected a clean exit, got ${result.exitCode}`) - assert.ok(captured, 'expected the socket to observe an implementation submission') - assert.match(captured.commit, /^[0-9a-f]{7,64}$/) + assert.ok(server.observation.artifact, 'expected the socket to observe an implementation submission') } finally { await server.close() } - // Daemon-side finish against the local bare repo + a FakeForge. - const forge = new FakeForge({ - branchHeadResolver: async (_repo, branch) => { - const out = git(runDir, 'ls-remote', origin, `refs/heads/${branch}`) - return out ? out.split('\t')[0] : null - }, - }) - const job = { - goal: GOAL, - actorDid: AGENT_DID, - gitUrl: 'https://github.com/acme/widget.git', - base: 'main', - branch: 'radial/impl-smoke', - baseCommit, - bundlePath: join(exportDir, 'impl.bundle'), - commit: captured.commit, - body: captured.body, - title: 'Smoke it', - } - const finish = await finishImplementationTurn( - { requestUri: REQUEST.uri, requestCid: REQUEST.cid, job, client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ) - assert.equal(finish.outcome, 'fulfilled') - assert.equal(git(runDir, 'ls-remote', origin, 'refs/heads/radial/impl-smoke').split('\t')[0], captured.commit) const written = [...pds.records.values()].find((r) => r.value?.$type === COLLECTIONS.artifact) - assert.ok(written, 'expected an artifact record from the finish step') + assert.ok(written, 'expected an implementation artifact record') assert.equal(written.uri.split('/').pop(), implArtifactRkey(REQUEST.uri, REQUEST.cid)) assert.equal(written.value.type, 'implementation') - assert.equal(written.value.links.commit, captured.commit) - assert.equal(written.value.links.pr, forge.pulls[0].url) + assert.equal(written.value.links.branch, branch) + assert.match(written.value.links.commit, /^[0-9a-f]{40}$/) + assert.equal(written.value.links.pr, 'https://github.com/acme/widget/pull/7') }) }) }) diff --git a/packages/daemon/test/egress.test.mjs b/packages/daemon/test/egress.test.mjs deleted file mode 100644 index b106b1a..0000000 --- a/packages/daemon/test/egress.test.mjs +++ /dev/null @@ -1,293 +0,0 @@ -import assert from 'node:assert/strict' -import { it } from 'node:test' -import { createEgressManager, networkCreateArgs, proxyRunArgs } from '../dist/index.js' - -it('networkCreateArgs builds a plain network create by default and adds --internal when asked', () => { - assert.deepEqual(networkCreateArgs('radial-egress', false), ['network', 'create', 'radial-egress']) - assert.deepEqual(networkCreateArgs('radial-internal', true), [ - 'network', - 'create', - '--internal', - 'radial-internal', - ]) -}) - -it('proxyRunArgs attaches the proxy to the egress network and passes the allowlist as repeated --allow flags', () => { - const args = proxyRunArgs({ - name: 'radial-proxy', - image: 'radial-proxy:latest', - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - allowlist: ['api.anthropic.com', 'example.test'], - }) - - assert.equal(args[0], 'run') - assert.ok(args.includes('-d')) - - const nameIndex = args.indexOf('--name') - assert.equal(args[nameIndex + 1], 'radial-proxy') - - const networkIndex = args.indexOf('--network') - assert.equal(args[networkIndex + 1], 'radial-egress') - // the internal network is joined separately by a follow-up `docker network connect`, not here - assert.ok(!args.includes('radial-internal')) - - const imageIndex = args.indexOf('radial-proxy:latest') - assert.ok(imageIndex !== -1) - assert.deepEqual(args.slice(imageIndex + 1), ['--allow', 'api.anthropic.com', '--allow', 'example.test']) -}) - -it('proxyRunArgs emits --allow-none for an explicit empty allowlist (deny-all), not zero --allow flags', () => { - const args = proxyRunArgs({ - name: 'radial-proxy', - image: 'radial-proxy:latest', - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - allowlist: [], - }) - const imageIndex = args.indexOf('radial-proxy:latest') - assert.deepEqual(args.slice(imageIndex + 1), ['--allow-none']) - assert.ok(!args.includes('--allow')) -}) - -it('proxyRunArgs emits one --allow per host and no --allow-none for a non-empty allowlist', () => { - const args = proxyRunArgs({ - name: 'radial-proxy', - image: 'radial-proxy:latest', - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - allowlist: ['api.anthropic.com', 'example.test'], - }) - const imageIndex = args.indexOf('radial-proxy:latest') - assert.deepEqual(args.slice(imageIndex + 1), ['--allow', 'api.anthropic.com', '--allow', 'example.test']) - assert.ok(!args.includes('--allow-none')) -}) - -function recordingExec(results) { - const calls = [] - const exec = async (argv) => { - calls.push(argv) - const scripted = results.shift() - return scripted ?? { code: 0, stdout: '', stderr: '' } - } - return { exec, calls } -} - -it('createEgressManager.start() creates both networks, runs the proxy, joins the internal network, verifies readiness, and returns the plan', async () => { - const { exec, calls } = recordingExec([ - { code: 0, stdout: '', stderr: '' }, // network create --internal - { code: 0, stdout: '', stderr: '' }, // network create (egress) - { code: 0, stdout: '', stderr: '' }, // run -d - { code: 0, stdout: '', stderr: '' }, // network connect - { code: 0, stdout: 'true\n', stderr: '' }, // docker inspect -f '{{.State.Running}}' radial-proxy - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: ['api.anthropic.com'], - exec, - }) - - const plan = await manager.start() - - assert.deepEqual(plan, { network: 'radial-internal', httpsProxy: 'http://radial-proxy:8080' }) - assert.deepEqual(calls, [ - ['network', 'create', '--internal', 'radial-internal'], - ['network', 'create', 'radial-egress'], - ['run', '-d', '--name', 'radial-proxy', '--network', 'radial-egress', '--label', 'radial-proxy', 'radial-proxy:latest', '--allow', 'api.anthropic.com'], - ['network', 'connect', 'radial-internal', 'radial-proxy'], - ['inspect', '-f', '{{.State.Running}}', 'radial-proxy'], - ]) -}) - -it('createEgressManager.start() tolerates "network already exists" (verifying it really is internal) but rethrows other network-create failures', async () => { - const alreadyExists = recordingExec([ - { code: 1, stdout: '', stderr: 'Error: network with name radial-internal already exists' }, // network create --internal - { code: 0, stdout: 'true\n', stderr: '' }, // docker network inspect -f '{{.Internal}}' radial-internal - { code: 0, stdout: '', stderr: '' }, // network create (egress) - { code: 0, stdout: '', stderr: '' }, // run -d - { code: 0, stdout: '', stderr: '' }, // network connect - { code: 0, stdout: 'true\n', stderr: '' }, // readiness: docker inspect -f '{{.State.Running}}' - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: alreadyExists.exec, - }) - await manager.start() - assert.equal(alreadyExists.calls.length, 6) // did not stop short on the "already exists" result - assert.deepEqual(alreadyExists.calls[1], ['network', 'inspect', '-f', '{{.Internal}}', 'radial-internal']) - - const otherFailure = recordingExec([{ code: 1, stdout: '', stderr: 'permission denied' }]) - const failing = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: otherFailure.exec, - }) - await assert.rejects(() => failing.start(), /permission denied/) -}) - -// --- FIX #3: verify a pre-existing "already exists" internal network is actually internal ----- - -it('createEgressManager.start() fails closed when a pre-existing internal-network name is NOT actually internal', async () => { - const notInternal = recordingExec([ - { code: 1, stdout: '', stderr: 'Error: network with name radial-internal already exists' }, // network create --internal - { code: 0, stdout: 'false\n', stderr: '' }, // docker network inspect reports NOT internal - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: notInternal.exec, - }) - await assert.rejects(() => manager.start(), /not internal/) - // Fails closed before ever running the proxy container. - assert.deepEqual( - notInternal.calls, - [ - ['network', 'create', '--internal', 'radial-internal'], - ['network', 'inspect', '-f', '{{.Internal}}', 'radial-internal'], - ], - ) -}) - -it('createEgressManager.start() proceeds when a pre-existing internal-network name really is internal', async () => { - const isInternal = recordingExec([ - { code: 1, stdout: '', stderr: 'Error: network with name radial-internal already exists' }, - { code: 0, stdout: 'true\n', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: 'true\n', stderr: '' }, - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: isInternal.exec, - }) - const plan = await manager.start() - assert.deepEqual(plan, { network: 'radial-internal', httpsProxy: 'http://radial-proxy:8080' }) -}) - -// --- FIX #14: verify proxy readiness before returning the EgressPlan, else fail closed --------- - -it('createEgressManager.start() resolves once docker inspect reports the proxy container Running', async () => { - const { exec } = recordingExec([ - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: 'true\n', stderr: '' }, // readiness check succeeds on the first attempt - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec, - readyAttempts: 3, - readyDelayMs: 1, - }) - const plan = await manager.start() - assert.deepEqual(plan, { network: 'radial-internal', httpsProxy: 'http://radial-proxy:8080' }) -}) - -it('createEgressManager.start() rejects (fails closed) when the proxy never reports Running', async () => { - const { exec, calls } = recordingExec([ - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: 'false\n', stderr: '' }, - { code: 0, stdout: 'false\n', stderr: '' }, - { code: 1, stdout: '', stderr: 'no such container' }, - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec, - readyAttempts: 3, - readyDelayMs: 1, // keep the test fast; bounded attempts are what matter, not real wall time - }) - await assert.rejects(() => manager.start(), /did not report running/) - // Exactly the bounded number of readiness attempts were made — not an unbounded retry loop. - const inspectCalls = calls.filter((c) => c[0] === 'inspect') - assert.equal(inspectCalls.length, 3) -}) - -it('createEgressManager.start() surfaces a failed proxy run or a failed network connect', async () => { - const runFails = recordingExec([ - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 1, stdout: '', stderr: 'no such image' }, - ]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: runFails.exec, - }) - await assert.rejects(() => manager.start(), /no such image/) - - const connectFails = recordingExec([ - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: '', stderr: '' }, - { code: 0, stdout: 'abc123\n', stderr: '' }, - { code: 1, stdout: '', stderr: 'endpoint already exists' }, - ]) - const manager2 = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: connectFails.exec, - }) - await assert.rejects(() => manager2.start(), /endpoint already exists/) -}) - -it('createEgressManager.stop() force-removes the proxy container by name, best-effort', async () => { - const { exec, calls } = recordingExec([]) - const manager = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec, - }) - await manager.stop() - assert.deepEqual(calls, [['rm', '-f', 'radial-proxy']]) - - const throwing = async () => { - throw new Error('docker not found') - } - const manager2 = createEgressManager({ - internalNetwork: 'radial-internal', - egressNetwork: 'radial-egress', - proxyImage: 'radial-proxy:latest', - proxyName: 'radial-proxy', - allowlist: [], - exec: throwing, - }) - await manager2.stop() // must not throw -}) diff --git a/packages/daemon/test/finish.test.mjs b/packages/daemon/test/finish.test.mjs deleted file mode 100644 index c472c53..0000000 --- a/packages/daemon/test/finish.test.mjs +++ /dev/null @@ -1,558 +0,0 @@ -import assert from 'node:assert/strict' -import { execFile } from 'node:child_process' -import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises' -import { tmpdir } from 'node:os' -import { join } from 'node:path' -import { it } from 'node:test' -import { promisify } from 'node:util' -import { createSession, CredentialClient } from '../../atproto/dist/index.js' -import { COLLECTIONS } from '../../core/dist/index.js' -import { LocalPds } from '../../atproto/test/local-pds.mjs' -import { - FakeForge, - FinishPump, - TurnLedger, - defaultGitExec, - finishImplementationTurn, - implArtifactRkey, - plainRemoteUrl, - scrubSecret, -} from '../dist/index.js' - -const execFileP = promisify(execFile) -const AGENT = 'did:plc:agent' -const REQUEST_URI = `at://did:plc:human/${COLLECTIONS.artifactRequest}/impl-req` -const REQUEST_CID = 'impl-req-cid' -const GOAL = { uri: `at://did:plc:human/${COLLECTIONS.goal}/g1`, cid: 'goal-cid' } - -const GIT_ENV = { - GIT_AUTHOR_NAME: 't', - GIT_AUTHOR_EMAIL: 't@t', - GIT_COMMITTER_NAME: 't', - GIT_COMMITTER_EMAIL: 't@t', -} - -async function git(args, cwd) { - const { stdout } = await execFileP('git', args, { ...(cwd ? { cwd } : {}), env: { ...process.env, ...GIT_ENV } }) - return stdout.trim() -} - -async function withTempDir(run) { - const dir = await mkdtemp(join(tmpdir(), 'radial-finish-')) - try { - return await run(dir) - } finally { - await rm(dir, { recursive: true, force: true }).catch(() => {}) - } -} - -/** Seed a bare origin with a base commit on main; return { origin, baseCommit }. */ -async function seedOrigin(dir) { - const origin = join(dir, 'origin.git') - await git(['init', '-q', '--bare', origin]) - const seed = join(dir, 'seed') - await git(['init', '-q', seed]) - await writeFile(join(seed, 'README'), 'base\n') - await git(['-C', seed, 'add', '-A']) - await git(['-C', seed, 'commit', '-q', '-m', 'base']) - await git(['-C', seed, 'branch', '-M', 'main']) - await git(['-C', seed, 'remote', 'add', 'origin', origin]) - await git(['-C', seed, 'push', '-q', 'origin', 'main']) - const baseCommit = await git(['-C', seed, 'rev-parse', 'HEAD']) - return { origin, baseCommit } -} - -/** Simulate a container turn: clone, commit, write a ranged bundle to /export/impl.bundle. - * Returns the new HEAD sha. */ -async function produceBundle(dir, origin, baseCommit, runDir, { extraCommitMessage = 'impl' } = {}) { - const checkout = join(dir, `checkout-${Math.random().toString(36).slice(2)}`) - await git(['clone', '-q', '--branch', 'main', origin, checkout]) - await writeFile(join(checkout, 'feature.txt'), `${extraCommitMessage}\n`) - await git(['-C', checkout, 'add', '-A']) - await git(['-C', checkout, 'commit', '-q', '-m', extraCommitMessage]) - const head = await git(['-C', checkout, 'rev-parse', 'HEAD']) - const exportDir = join(runDir, 'export') - await mkdir(exportDir, { recursive: true }) - await git(['-C', checkout, 'bundle', 'create', join(exportDir, 'impl.bundle'), `${baseCommit}..HEAD`]) - return head -} - -async function makeClient(pds) { - const session = await createSession(pds.service, pds.handle, 'pw', pds.fetch.bind(pds)) - return new CredentialClient(session, pds.fetch.bind(pds)) -} - -function forgeForOrigin(origin) { - return new FakeForge({ - branchHeadResolver: async (_repo, branch) => { - const out = await git(['ls-remote', origin, `refs/heads/${branch}`]) - const sha = out.split('\t')[0] - return sha || null - }, - }) -} - -function baseJob(baseCommit, commit, runDir, overrides = {}) { - return { - goal: GOAL, - actorDid: AGENT, - gitUrl: 'https://github.com/acme/widget.git', - base: 'main', - branch: 'radial/impl-abc123', - baseCommit, - bundlePath: join(runDir, 'export', 'impl.bundle'), - commit, - body: 'implementation summary', - title: 'Ship it', - ...overrides, - } -} - -it('finish: bundle-fetch -> verify -> push -> PR -> record (links branch/commit/pr)', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - const head = await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - - const result = await finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, head, runDir), client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ) - assert.equal(result.outcome, 'fulfilled') - - // The sha landed on the branch in the origin. - assert.equal((await git(['ls-remote', origin, 'refs/heads/radial/impl-abc123'])).split('\t')[0], head) - - // Exactly one PR opened. - assert.equal(forge.pulls.length, 1) - - // The artifact record was written under the deterministic impl rkey, with full links. - const rkey = implArtifactRkey(REQUEST_URI, REQUEST_CID) - const stored = [...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact) - assert.equal(stored.length, 1) - const record = stored[0] - assert.ok(record.uri.endsWith(`/${rkey}`)) - assert.equal(record.value.type, 'implementation') - assert.equal(record.value.links.commit, head) - assert.equal(record.value.links.branch, 'radial/impl-abc123') - assert.equal(record.value.links.pr, forge.pulls[0].url) - assert.deepEqual(record.value.request, { uri: REQUEST_URI, cid: REQUEST_CID }) - }) -}) - -it('finish: a commit absent from the bundle is rejected — no push, no PR, no record', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - - const bogus = 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef' - const result = await finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, bogus, runDir), client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ) - assert.equal(result.outcome, 'rejected') - assert.equal((await git(['ls-remote', origin, 'refs/heads/radial/impl-abc123'])), '') - assert.equal(forge.pulls.length, 0) - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 0) - }) -}) - -it('finish: a sha that exists at origin but is not a bundle head is rejected (provenance, finding 4)', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - await produceBundle(dir, origin, baseCommit, runDir) // bundle head is the container's new commit - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - - // baseCommit is a real object present at origin, but it is NOT a head of the submitted bundle - // (the bundle's head is the new commit). It must be rejected before any push. - const result = await finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, baseCommit, runDir), client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ) - assert.equal(result.outcome, 'rejected') - assert.match(result.reason, /not a head of the submitted bundle/) - assert.equal((await git(['ls-remote', origin, 'refs/heads/radial/impl-abc123'])), '') - assert.equal(forge.pulls.length, 0) - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 0) - }) -}) - -it('finish: idempotent resume converges on one record and one PR (same-sha re-push is a no-op)', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - const head = await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - const input = { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, head, runDir), client, runDir, githubToken: 'ghtok' } - - const first = await finishImplementationTurn(input, { forge, remoteUrl: () => origin }) - const second = await finishImplementationTurn(input, { forge, remoteUrl: () => origin }) - assert.equal(first.outcome, 'fulfilled') - assert.equal(second.outcome, 'fulfilled') - assert.deepEqual(second.ref, first.ref) - assert.equal(forge.pulls.length, 1) - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 1) - }) -}) - -it('finish: a PDS write failure after PR creation resumes without duplicating the PR or record', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - const head = await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const real = await makeClient(pds) - const forge = forgeForOrigin(origin) - - // A client that throws on the FIRST create (a transient PDS error after the PR was opened). - let failedOnce = false - const client = { - create: async (collection, value, options) => { - if (!failedOnce) { - failedOnce = true - throw new Error('PDS unreachable') - } - return real.create(collection, value, options) - }, - getOwnRecord: (collection, rkey) => real.getOwnRecord(collection, rkey), - } - const input = { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, head, runDir), client, runDir, githubToken: 'ghtok' } - - await assert.rejects(finishImplementationTurn(input, { forge, remoteUrl: () => origin }), /PDS unreachable/) - assert.equal(forge.pulls.length, 1) // PR was created before the write failed - - const resumed = await finishImplementationTurn(input, { forge, remoteUrl: () => origin }) - assert.equal(resumed.outcome, 'fulfilled') - assert.equal(forge.pulls.length, 1) // adopted, not duplicated - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 1) - }) -}) - -it('finish: refuses to adopt a colliding record that does NOT match the pending job (finding 7)', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - const head = await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - - // First finish writes the record with body "implementation summary". - await finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, head, runDir), client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ) - // A resume with a DIFFERENT body collides at the deterministic rkey; the record doesn't match - // the (deterministic) job, so it hard-errors rather than adopting the wrong artifact. - const tampered = baseJob(baseCommit, head, runDir, { body: 'a different body' }) - await assert.rejects( - finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: tampered, client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ), - /does not match the pending finish job/, - ) - // Still exactly one record (nothing overwritten) and one PR. - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 1) - assert.equal(forge.pulls.length, 1) - }) -}) - -it('finish: v2 reuse pushes to the predecessor branch and adopts its already-open PR', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - const head = await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - // Model an already-open predecessor PR on the reused branch. - forge.pulls.push({ - number: 3, - head: 'radial/impl-v1', - base: 'main', - state: 'open', - headSha: 'oldsha', - url: 'https://github.com/acme/widget/pull/3', - }) - - const prev = { uri: `at://${AGENT}/${COLLECTIONS.artifact}/impl-prev`, cid: 'prevcid' } - const job = baseJob(baseCommit, head, runDir, { branch: 'radial/impl-v1', prev }) - const result = await finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job, client, runDir, githubToken: 'ghtok' }, - { forge, remoteUrl: () => origin }, - ) - assert.equal(result.outcome, 'fulfilled') - // Pushed to the predecessor branch; the same PR (#3) reused, none created. - assert.equal((await git(['ls-remote', origin, 'refs/heads/radial/impl-v1'])).split('\t')[0], head) - assert.equal(forge.pulls.length, 1) - const record = [...pds.records.values()].find((r) => r.value.$type === COLLECTIONS.artifact) - assert.equal(record.value.links.pr, 'https://github.com/acme/widget/pull/3') - assert.deepEqual(record.value.prev, prev) // prev stamped daemon-side - }) -}) - -// --- FinishPump (drives finishing rows; crash-resumable) -------------------- - -const SHA_A = 'a'.repeat(40) -const SHA_B = 'b'.repeat(40) -const PUMP_JOB = { - goal: GOAL, - actorDid: AGENT, - gitUrl: 'https://github.com/acme/widget.git', - base: 'main', - branch: 'radial/impl-pump', - baseCommit: SHA_A, - bundlePath: '/nonexistent/impl.bundle', - commit: SHA_B, - body: 'summary', - title: 'Ship it', -} - -function pumpDeps(ledger, overrides = {}) { - return { - ledger, - concurrency: 2, - clientFor: () => ({ create: async () => ({ uri: 'x', cid: 'y' }), getOwnRecord: async () => undefined }), - runDirFor: (uri) => join(tmpdir(), 'radial-pump-nope', uri.replace(/[^a-z0-9]/gi, '_')), - githubToken: 'ghtok', - forge: new FakeForge(), - now: () => Date.now(), - ...overrides, - } -} - -it('FinishPump drives a finishing row to fulfilled and removes its runDir', async () => { - await withTempDir(async (dir) => { - const ledger = new TurnLedger() - ledger.markSubmitted('at://req/impl', 'cid-i', PUMP_JOB) - const runDir = join(dir, 'run') - await mkdir(runDir, { recursive: true }) - await writeFile(join(runDir, 'marker'), 'x') - - let seen - const pump = new FinishPump( - pumpDeps(ledger, { - runDirFor: () => runDir, - finish: async (input) => { - seen = input - return { outcome: 'fulfilled', ref: { uri: 'at://a/b/c', cid: 'refcid' } } - }, - }), - ) - pump.pump() - await pump.drain() - - assert.equal(seen.job.commit, SHA_B) - assert.equal(ledger.get('at://req/impl').state, 'fulfilled') - await assert.rejects(stat(runDir)) // runDir removed after fulfillment - ledger.close() - }) -}) - -it('FinishPump: a rejected submission gives up (no endless retry)', async () => { - const ledger = new TurnLedger() - ledger.markSubmitted('at://req/impl', 'cid-i', PUMP_JOB) - const pump = new FinishPump(pumpDeps(ledger, { finish: async () => ({ outcome: 'rejected', reason: 'bogus sha' }) })) - pump.pump() - await pump.drain() - assert.equal(ledger.get('at://req/impl').state, 'gave_up') - ledger.close() -}) - -it('FinishPump: a thrown transient failure keeps the row finishing with a cooldown', async () => { - let now = 1_000_000 - const ledger = new TurnLedger(':memory:', { retryBound: 5, cooldownMs: 5000, now: () => new Date(now).toISOString() }) - ledger.markSubmitted('at://req/impl', 'cid-i', PUMP_JOB) - const pump = new FinishPump(pumpDeps(ledger, { now: () => now, finish: async () => { throw new Error('network') } })) - pump.pump() - await pump.drain() - const row = ledger.get('at://req/impl') - assert.equal(row.state, 'finishing') - assert.equal(row.attempts, 1) - // Cooling down: another pump before the cooldown elapses launches nothing. - pump.pump() - assert.equal(pump.inFlight, 0) - ledger.close() -}) - -it('crash-resume: a finishing row in a reopened ledger is resumed to a written record', async () => { - const dir = await mkdtemp(join(tmpdir(), 'radial-resume-')) - try { - const path = join(dir, 'ledger.db') - // Prior process: submitted, then died before finishing. - const before = new TurnLedger(path) - before.markSubmitted('at://req/impl', 'cid-i', PUMP_JOB) - before.close() - - // Fresh process: a new ledger over the same file, a fresh FinishPump — no in-memory state. - const after = new TurnLedger(path) - assert.equal(after.get('at://req/impl').state, 'finishing') - let resumed = false - const pump = new FinishPump( - pumpDeps(after, { - finish: async () => { - resumed = true - return { outcome: 'fulfilled', ref: { uri: 'at://a/b/c', cid: 'refcid' } } - }, - runDirFor: () => join(dir, 'run-nonexistent'), - }), - ) - pump.pump() - await pump.drain() - assert.ok(resumed) - assert.equal(after.get('at://req/impl').state, 'fulfilled') - after.close() - } finally { - await rm(dir, { recursive: true, force: true }).catch(() => {}) - } -}) - -// --- Token-leak defenses (credential-free URL + scrubbing) ------------------ - -it('plainRemoteUrl and scrubSecret keep the token out of URLs/text', () => { - const url = plainRemoteUrl('https://github.com/acme/widget.git') - assert.equal(url, 'https://github.com/acme/widget.git') - assert.equal(url.includes('token'), false) - assert.equal(scrubSecret('fatal: https://x-access-token:SEKRET@github.com/... 403', 'SEKRET'), 'fatal: https://x-access-token:***@github.com/... 403') - assert.equal(scrubSecret('no secret here', 'SEKRET'), 'no secret here') - assert.equal(scrubSecret('anything', ''), 'anything') // empty token is a no-op (not a match-all) -}) - -it('finish: the push carries no token in argv and none in the clone .git/config (token is env-only)', async () => { - await withTempDir(async (dir) => { - const { origin, baseCommit } = await seedOrigin(dir) - const runDir = join(dir, 'run') - const head = await produceBundle(dir, origin, baseCommit, runDir) - const pds = new LocalPds(AGENT) - const client = await makeClient(pds) - const forge = forgeForOrigin(origin) - const TOKEN = 'ghp_secret_TOKEN_value' - - const recorded = [] - const realExec = defaultGitExec() - const recordingExec = async (argv, opts) => { - recorded.push(argv) - // Defense: the token must never appear in argv (visible in the process list). - for (const a of argv) assert.equal(String(a).includes(TOKEN), false, `token leaked into argv: ${argv.join(' ')}`) - return realExec(argv, opts) - } - - const result = await finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job: baseJob(baseCommit, head, runDir), client, runDir, githubToken: TOKEN }, - { forge, remoteUrl: () => origin, exec: recordingExec }, - ) - assert.equal(result.outcome, 'fulfilled') - assert.ok(recorded.some((argv) => argv.includes('push'))) - // The finish clone's persisted config carries no token. - const config = await readFile(join(runDir, 'finish', '.git', 'config'), 'utf8') - assert.equal(config.includes(TOKEN), false) - }) -}) - -it('finish: a git failure whose stderr embeds the token throws a SCRUBBED error (no token leak)', async () => { - const TOKEN = 'ghp_leaky_value' - const commit = 'c'.repeat(40) - // A fake exec: provenance (list-heads) lists the commit, verify/rev-parse pass, and only the push - // fails — with the token embedded in stderr the way a real credentialed git would print it. - const fakeExec = async (argv) => { - const cmd = argv.join(' ') - if (cmd.includes('bundle list-heads')) return { code: 0, stdout: `${commit} HEAD\n`, stderr: '' } - if (cmd.includes(' push ')) { - return { code: 128, stdout: '', stderr: `fatal: unable to access 'https://x-access-token:${TOKEN}@github.com/acme/widget.git/': The requested URL returned error: 403` } - } - return { code: 0, stdout: '', stderr: '' } - } - const job = { - goal: GOAL, - actorDid: AGENT, - gitUrl: 'https://github.com/acme/widget.git', - base: 'main', - branch: 'radial/impl-leak', - baseCommit: 'a'.repeat(40), - bundlePath: join(tmpdir(), 'radial-leak-nope', 'impl.bundle'), - commit, - body: 'summary', - title: 'Ship it', - } - await assert.rejects( - finishImplementationTurn( - { requestUri: REQUEST_URI, requestCid: REQUEST_CID, job, client: { create: async () => ({ uri: 'x', cid: 'y' }), getOwnRecord: async () => undefined }, runDir: join(tmpdir(), 'radial-leak-nope'), githubToken: TOKEN }, - { forge: new FakeForge(), exec: fakeExec, remoteUrl: () => 'unused' }, - ), - (error) => { - assert.equal(error.message.includes(TOKEN), false, 'token leaked into the thrown error message') - assert.ok(error.message.includes('***'), 'expected the token to be redacted with ***') - return true - }, - ) -}) - -it('FinishPump: a token-bearing git failure produces a scrubbed log line (no token in logs/ledger)', async () => { - const TOKEN = 'ghp_pumpleak' - const ledger = new TurnLedger() - ledger.markSubmitted('at://req/impl', 'cid-i', { - goal: GOAL, actorDid: AGENT, gitUrl: 'https://github.com/acme/widget.git', - base: 'main', branch: 'radial/impl-leak', baseCommit: 'a'.repeat(40), bundlePath: '/nope/impl.bundle', commit: 'b'.repeat(40), body: 's', title: 't', - }) - const logs = [] - // The pump forwards the (already-scrubbed) error message from finishImplementationTurn. - const pump = new FinishPump({ - ledger, - concurrency: 1, - clientFor: () => ({ create: async () => ({ uri: 'x', cid: 'y' }), getOwnRecord: async () => undefined }), - runDirFor: () => join(tmpdir(), 'radial-pumpleak-nope'), - githubToken: TOKEN, - forge: new FakeForge(), - now: () => Date.now(), - log: (m) => logs.push(m), - finish: async () => { - // Mimic finishImplementationTurn: it scrubs git stderr before throwing. - throw new Error(scrubSecret(`git push failed (exit 128): fatal: ...x-access-token:${TOKEN}@github.com... 403`, TOKEN)) - }, - }) - pump.pump() - await pump.drain() - assert.ok(logs.length > 0) - for (const line of logs) assert.equal(line.includes(TOKEN), false, `token leaked into a log line: ${line}`) - assert.ok(logs.some((line) => line.includes('***'))) - ledger.close() -}) - -it('FinishPump holds off a row whose container is still in flight (finding 3a)', async () => { - const ledger = new TurnLedger() - ledger.markSubmitted('at://req/impl', 'cid-i', PUMP_JOB) - let finishCalls = 0 - const deps = pumpDeps(ledger, { - isInFlight: () => true, // dispatcher still running the container - finish: async () => { - finishCalls += 1 - return { outcome: 'fulfilled', ref: { uri: 'at://a/b/c', cid: 'refcid' } } - }, - }) - const pump = new FinishPump(deps) - pump.pump() - await pump.drain() - assert.equal(finishCalls, 0) // held off - assert.equal(ledger.get('at://req/impl').state, 'finishing') - - // Once the container is no longer in flight, it finishes. - const pump2 = new FinishPump({ ...deps, isInFlight: () => false }) - pump2.pump() - await pump2.drain() - assert.equal(finishCalls, 1) - assert.equal(ledger.get('at://req/impl').state, 'fulfilled') - ledger.close() -}) diff --git a/packages/daemon/test/forge.test.mjs b/packages/daemon/test/forge.test.mjs index 2508452..1abd996 100644 --- a/packages/daemon/test/forge.test.mjs +++ b/packages/daemon/test/forge.test.mjs @@ -38,102 +38,20 @@ it('compareUrl is a pure base..commit URL', () => { assert.equal(compareUrl(REPO, 'main', 'deadbeef'), 'https://github.com/acme/widget/compare/main...deadbeef') }) -// --- getBranchHead ---------------------------------------------------------- - -it('getBranchHead returns the sha on 200 and null on 404', async () => { - const present = fetcher([{ match: (u) => u.includes('/git/ref/heads/'), status: 200, body: { object: { sha: 'abc123' } } }]) - const forge = new GitHubForge({ fetch: present.fetch, token: 't' }) - assert.equal(await forge.getBranchHead(REPO, 'radial/impl-x'), 'abc123') - - const missing = fetcher([{ match: () => true, status: 404, body: { message: 'Not Found' } }]) - const forge2 = new GitHubForge({ fetch: missing.fetch, token: 't' }) - assert.equal(await forge2.getBranchHead(REPO, 'nope'), null) -}) - -// --- openOrGetPullRequest --------------------------------------------------- - -it('openOrGetPullRequest creates a PR on 201', async () => { - const { fetch, calls } = fetcher([ - { match: (u, i) => i?.method === 'POST', status: 201, body: { number: 12, html_url: 'https://github.com/acme/widget/pull/12' } }, - ]) - const forge = new GitHubForge({ fetch, token: 't' }) - const pr = await forge.openOrGetPullRequest(REPO, 'radial/impl-x', 'main', 'title', 'body') - assert.deepEqual(pr, { number: 12, url: 'https://github.com/acme/widget/pull/12' }) - assert.equal(calls[0].method, 'POST') -}) - -it('openOrGetPullRequest adopts the existing OPEN PR on 422 (queried with state=open&head)', async () => { - const { fetch, calls } = fetcher([ - { match: (u, i) => i?.method === 'POST', status: 422, body: { message: 'A pull request already exists' } }, - { - match: (u) => u.includes('/pulls?state=open&head='), - status: 200, - body: [ - { number: 5, html_url: 'https://github.com/acme/widget/pull/5', state: 'open', head: { ref: 'radial/impl-x', sha: 's' }, base: { ref: 'main' } }, - ], - }, - ]) - const forge = new GitHubForge({ fetch, token: 't' }) - const pr = await forge.openOrGetPullRequest(REPO, 'radial/impl-x', 'main', 'title', 'body') - assert.deepEqual(pr, { number: 5, url: 'https://github.com/acme/widget/pull/5' }) - // The adoption listing is scoped to open PRs for this exact head — no merged/closed can appear. - assert.ok(calls[1].url.includes('state=open&head=acme%3Aradial%2Fimpl-x')) -}) - -it('openOrGetPullRequest NEVER adopts a merged PR as open (422 + empty open list → throws)', async () => { - const { fetch } = fetcher([ - { match: (u, i) => i?.method === 'POST', status: 422, body: { message: 'exists' } }, - // A merged PR is not in the state=open results, so the list is empty and nothing is adopted. - { match: (u) => u.includes('/pulls?state=open&head='), status: 200, body: [] }, - ]) - const forge = new GitHubForge({ fetch, token: 't' }) - await assert.rejects(forge.openOrGetPullRequest(REPO, 'radial/impl-x', 'main', 't', 'b'), /no OPEN pull request/) -}) - -// --- getPullRequest state mapping ------------------------------------------- - -it('getPullRequest maps merged/open/closed and returns head ref/repo + base ref', async () => { - const head = { ref: 'radial/impl-x', sha: 'h1', repo: { full_name: 'acme/widget' } } - const merged = new GitHubForge({ - fetch: fetcher([{ match: () => true, status: 200, body: { number: 1, html_url: 'x', state: 'closed', merged: true, merged_at: '2026-01-02T00:00:00Z', head, base: { ref: 'main' } } }]).fetch, - token: 't', - }) - assert.deepEqual(await merged.getPullRequest(REPO, 1), { - state: 'merged', headSha: 'h1', headRef: 'radial/impl-x', headRepoFullName: 'acme/widget', baseRef: 'main', mergedAt: '2026-01-02T00:00:00Z', - }) - - const open = new GitHubForge({ - fetch: fetcher([{ match: () => true, status: 200, body: { number: 2, html_url: 'x', state: 'open', merged: false, head: { ref: 'radial/impl-y', sha: 'h2' }, base: { ref: 'main' } } }]).fetch, - token: 't', - }) - // A missing head.repo (deleted fork) maps to an empty full_name. - assert.deepEqual(await open.getPullRequest(REPO, 2), { - state: 'open', headSha: 'h2', headRef: 'radial/impl-y', headRepoFullName: '', baseRef: 'main', - }) - - const closed = new GitHubForge({ - fetch: fetcher([{ match: () => true, status: 200, body: { number: 3, html_url: 'x', state: 'closed', merged: false, head, base: { ref: 'dev' } } }]).fetch, - token: 't', - }) - assert.deepEqual(await closed.getPullRequest(REPO, 3), { - state: 'closed', headSha: 'h1', headRef: 'radial/impl-x', headRepoFullName: 'acme/widget', baseRef: 'dev', - }) -}) - // --- getPullRequestState URL validation ------------------------------------- it('parseGitHubPullUrl accepts only canonical github.com/owner/repo/pull/', () => { assert.deepEqual(parseGitHubPullUrl('https://github.com/acme/widget/pull/42'), { repo: REPO, number: 42 }) - assert.throws(() => parseGitHubPullUrl('https://gitlab.com/acme/widget/pull/42'), /github\.com/) + assert.throws(() => parseGitHubPullUrl('https://gitlab.com/acme/widget/pull/42'), /canonical GitHub/) assert.throws(() => parseGitHubPullUrl('https://github.com/acme/widget/pull/abc'), /canonical/) - assert.throws(() => parseGitHubPullUrl('http://github.com/acme/widget/pull/42'), /https/) + assert.throws(() => parseGitHubPullUrl('http://github.com/acme/widget/pull/42'), /canonical GitHub/) assert.throws(() => parseGitHubPullUrl('https://github.com/acme/widget/pulls/42'), /canonical/) // Strictness (finding 11): userinfo, non-default port, query, and fragment are all rejected. - assert.throws(() => parseGitHubPullUrl('https://user:pass@github.com/acme/widget/pull/42'), /userinfo/) - assert.throws(() => parseGitHubPullUrl('https://token@github.com/acme/widget/pull/42'), /userinfo/) - assert.throws(() => parseGitHubPullUrl('https://github.com:8443/acme/widget/pull/42'), /default port/) - assert.throws(() => parseGitHubPullUrl('https://github.com/acme/widget/pull/42?x=1'), /query string/) - assert.throws(() => parseGitHubPullUrl('https://github.com/acme/widget/pull/42#frag'), /fragment/) + assert.throws(() => parseGitHubPullUrl('https://user:pass@github.com/acme/widget/pull/42'), /canonical GitHub/) + assert.throws(() => parseGitHubPullUrl('https://token@github.com/acme/widget/pull/42'), /canonical GitHub/) + assert.throws(() => parseGitHubPullUrl('https://github.com:8443/acme/widget/pull/42'), /canonical GitHub/) + assert.throws(() => parseGitHubPullUrl('https://github.com/acme/widget/pull/42?x=1'), /canonical GitHub/) + assert.throws(() => parseGitHubPullUrl('https://github.com/acme/widget/pull/42#frag'), /canonical GitHub/) }) it('getPullRequestState parses the URL and builds the API call from the parsed parts', async () => { @@ -142,7 +60,7 @@ it('getPullRequestState parses the URL and builds the API call from the parsed p ]) const forge = new GitHubForge({ fetch, token: 't' }) const state = await forge.getPullRequestState('https://github.com/acme/widget/pull/42') - assert.deepEqual(state, { state: 'merged', mergedAt: '2026-03-01T00:00:00Z' }) + assert.deepEqual(state, { state: 'merged', headRef: 'b', headRepoFullName: '', baseRef: 'main', mergedAt: '2026-03-01T00:00:00Z' }) // The token-bearing call targets the parsed api path, never the raw record URL. assert.ok(calls[0].url.includes('api.github.com/repos/acme/widget/pulls/42')) }) @@ -150,6 +68,6 @@ it('getPullRequestState parses the URL and builds the API call from the parsed p it('getPullRequestState rejects a non-github URL before making any request', async () => { const { fetch, calls } = fetcher([{ match: () => true, status: 200, body: {} }]) const forge = new GitHubForge({ fetch, token: 't' }) - await assert.rejects(forge.getPullRequestState('https://evil.example/acme/widget/pull/1'), /github\.com/) + await assert.rejects(forge.getPullRequestState('https://evil.example/acme/widget/pull/1'), /canonical GitHub/) assert.equal(calls.length, 0) }) diff --git a/packages/daemon/test/github-auth.test.mjs b/packages/daemon/test/github-auth.test.mjs new file mode 100644 index 0000000..7c4f710 --- /dev/null +++ b/packages/daemon/test/github-auth.test.mjs @@ -0,0 +1,71 @@ +import assert from 'node:assert/strict' +import test from 'node:test' +import { ensureGitHubAuth, resolveGitHubToken } from '../dist/index.js' + +test('GH_TOKEN has priority over legacy and gh', async () => { + const token = await resolveGitHubToken({ GH_TOKEN: 'fine', GITHUB_TOKEN: 'legacy' }, async () => { + throw new Error('called') + }) + assert.equal(token, 'fine') +}) + +test('uses legacy token then an existing gh login', async () => { + assert.equal( + await resolveGitHubToken({ GITHUB_TOKEN: 'legacy' }, async () => ({ + code: 0, + stdout: 'x', + stderr: '', + })), + 'legacy', + ) + assert.equal( + await resolveGitHubToken({}, async () => ({ code: 0, stdout: ' gh-token\n', stderr: '' })), + 'gh-token', + ) +}) + +test('missing gh resolves as unauthenticated', async () => { + assert.equal( + await resolveGitHubToken({}, async () => { + const error = new Error('spawn gh ENOENT') + error.code = 'ENOENT' + throw error + }), + undefined, + ) +}) + +test('login fallback is interactive and returns the resolved token', async () => { + const calls = [] + const token = await ensureGitHubAuth({ + env: {}, + runner: async (command, args, options) => { + calls.push({ command, args, options }) + if (args[1] === 'login') return { code: 0, stdout: '', stderr: '' } + return { code: 0, stdout: calls.length > 2 ? 'token' : '', stderr: '' } + }, + }) + + assert.equal(token, 'token') + const login = calls.find((call) => call.args[1] === 'login') + assert.deepEqual(login, { + command: 'gh', + args: ['auth', 'login', '--hostname', 'github.com', '--git-protocol', 'https', '--web'], + options: { interactive: true }, + }) +}) + +test('missing gh during login reports an actionable error', async () => { + await assert.rejects( + ensureGitHubAuth({ + env: {}, + runner: async (_command, args) => { + if (args[1] === 'token') return { code: 1, stdout: '', stderr: '' } + const error = new Error('spawn gh ENOENT') + error.code = 'ENOENT' + throw error + }, + }), + /GitHub CLI \(gh\) is required/, + ) +}) diff --git a/packages/daemon/test/harness.test.mjs b/packages/daemon/test/harness.test.mjs index 72ad92a..fc4371b 100644 --- a/packages/daemon/test/harness.test.mjs +++ b/packages/daemon/test/harness.test.mjs @@ -85,3 +85,29 @@ it('ClaudeCodeHarness.invocation tolerates an empty models array by omitting --m }) assert.ok(!invocation.argv.includes('--model')) }) + +it('implementation prompt owns the GitHub branch, commit, push, and PR workflow', () => { + const invocation = new ClaudeCodeHarness().invocation({ + briefPath: '/bundle/brief.md', + bundleDir: '/bundle', + workdir: '/work', + models: [], + implementation: true, + }) + const prompt = invocation.argv[2] + + assert.ok( + prompt.indexOf('prepare the daemon-selected branch') < prompt.indexOf('implement the requested change'), + 'the branch must be selected before files are changed', + ) + assert.match(prompt, /gh auth setup-git/) + assert.match(prompt, /git ls-remote/) + assert.match(prompt, /Co-Authored-By: \$RADIAL_AGENT_NAME \(\$RADIAL_AGENT_DID\)/) + assert.match(prompt, /push `\$RADIAL_BRANCH`/) + assert.match(prompt, /gh pr edit/) + assert.match(prompt, /gh pr create --base "\$RADIAL_BASE_BRANCH"/) + assert.match(prompt, /\[Radial artifact\]\(\$RADIAL_ARTIFACT_URI\)/) + assert.match(prompt, /--branch "\$RADIAL_BRANCH" --commit/) + assert.match(prompt, /--pr "\$\(gh pr view/) + assert.match(prompt, /Do not merge/) +}) diff --git a/packages/daemon/test/ledger.test.mjs b/packages/daemon/test/ledger.test.mjs index 668647b..f70b461 100644 --- a/packages/daemon/test/ledger.test.mjs +++ b/packages/daemon/test/ledger.test.mjs @@ -1,8 +1,9 @@ import assert from 'node:assert/strict' -import { mkdtemp } from 'node:fs/promises' +import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { it } from 'node:test' +import { DatabaseSync } from 'node:sqlite' import { TurnLedger } from '../dist/index.js' it('a fresh request has no row and is eligible', () => { @@ -124,90 +125,7 @@ it('attempts persist across a process restart (durability)', async () => { second.close() }) -// --- Implementation finishing-state transitions ----------------------------- - -const FINISH_JOB = { - goal: { uri: 'at://did:plc:human/com.disnetdev.radial.goal/g', cid: 'gc' }, - actorDid: 'did:plc:agent', - gitUrl: 'https://github.com/acme/widget.git', - base: 'main', - branch: 'radial/impl-abc', - baseCommit: 'a'.repeat(40), - bundlePath: '/state/turns/abc/impl.bundle', - commit: 'b'.repeat(40), - body: 'summary', - title: 'Ship it', -} - -it('markSubmitted -> finishing: ineligible to the container dispatcher, surfaced to the finish pump', () => { - const ledger = new TurnLedger() - ledger.markRunning('at://req/impl', 'cid-i', { containerLabel: 'l', checkoutPath: '/tmp/x' }) - ledger.markSubmitted('at://req/impl', 'cid-i', FINISH_JOB) - const row = ledger.get('at://req/impl') - assert.equal(row.state, 'finishing') - assert.equal(ledger.eligible('at://req/impl'), false) // container dispatcher never re-launches it - assert.deepEqual(ledger.finishing().map((r) => r.requestUri), ['at://req/impl']) - assert.deepEqual(ledger.submission('at://req/impl'), FINISH_JOB) - ledger.close() -}) - -it('markFinishFailed keeps the row finishing (with cooldown) until the retry bound gives it up', () => { - let now = '2026-01-01T00:00:00.000Z' - const ledger = new TurnLedger(':memory:', { retryBound: 2, cooldownMs: 1000, now: () => now }) - ledger.markSubmitted('at://req/impl', 'cid-i', FINISH_JOB) - const first = ledger.markFinishFailed('at://req/impl') - assert.equal(first.state, 'finishing') - assert.equal(first.attempts, 1) - assert.ok(ledger.get('at://req/impl').nextEligibleAt) // cooldown set - const second = ledger.markFinishFailed('at://req/impl') - assert.equal(second.state, 'gave_up') // retry bound reached - assert.equal(ledger.finishing().length, 0) - ledger.close() -}) -it('markFulfilled from finishing clears it to fulfilled with the accepted ref', () => { - const ledger = new TurnLedger() - ledger.markSubmitted('at://req/impl', 'cid-i', FINISH_JOB) - ledger.markFulfilled('at://req/impl', { uri: 'at://did:plc:agent/art/impl-x', cid: 'artcid' }) - const row = ledger.get('at://req/impl') - assert.equal(row.state, 'fulfilled') - assert.deepEqual(row.acceptedRef, { uri: 'at://did:plc:agent/art/impl-x', cid: 'artcid' }) - assert.equal(ledger.finishing().length, 0) - ledger.close() -}) - -it('giveUp forces a terminal gave_up state (for ambiguous-predecessor rejection)', () => { - const ledger = new TurnLedger() - ledger.giveUp('at://req/ambiguous', 'cid-a') - assert.equal(ledger.get('at://req/ambiguous').state, 'gave_up') - assert.equal(ledger.eligible('at://req/ambiguous'), false) - ledger.close() -}) - -it('the finishing submission payload survives a reopen (durable finish job)', async () => { - const dir = await mkdtemp(join(tmpdir(), 'radial-ledger-finish-')) - const path = join(dir, 'ledger.db') - const first = new TurnLedger(path) - first.markSubmitted('at://req/impl', 'cid-i', FINISH_JOB) - first.close() - const second = new TurnLedger(path) - assert.deepEqual(second.submission('at://req/impl'), FINISH_JOB) - assert.deepEqual(second.finishing().map((r) => r.requestUri), ['at://req/impl']) - second.close() -}) - -// --- finding 5: markCrashed never clobbers a finishing/terminal row -------- - -it('markCrashed does not clobber a finishing row (a late runner error is ignored)', () => { - const ledger = new TurnLedger() - ledger.markRunning('at://req/impl', 'cid-i', { containerLabel: 'l', checkoutPath: '/tmp/x' }) - ledger.markSubmitted('at://req/impl', 'cid-i', FINISH_JOB) - const result = ledger.markCrashed('at://req/impl') // late container-runner error after submit - assert.equal(result.state, 'finishing') - assert.equal(ledger.get('at://req/impl').state, 'finishing') - assert.deepEqual(ledger.submission('at://req/impl'), FINISH_JOB) // payload intact - ledger.close() -}) it('markCrashed does not clobber a fulfilled/gave_up row', () => { const ledger = new TurnLedger() @@ -218,24 +136,51 @@ it('markCrashed does not clobber a fulfilled/gave_up row', () => { ledger.close() }) -// --- finding 8: turn reset restores a gave-up finish to finishing ---------- - -it('reopenFinishing restores a gave_up row (with payload) to finishing; returns false otherwise', () => { - let now = '2026-01-01T00:00:00.000Z' - const ledger = new TurnLedger(':memory:', { retryBound: 1, cooldownMs: 1000, now: () => now }) - ledger.markSubmitted('at://req/impl', 'cid-i', FINISH_JOB) - assert.equal(ledger.markFinishFailed('at://req/impl').state, 'gave_up') // retryBound 1 - assert.equal(ledger.get('at://req/impl').state, 'gave_up') - assert.deepEqual(ledger.submission('at://req/impl'), FINISH_JOB) // payload preserved on give-up - - assert.equal(ledger.reopenFinishing('at://req/impl'), true) - assert.equal(ledger.get('at://req/impl').state, 'finishing') - assert.equal(ledger.get('at://req/impl').attempts, 0) - assert.deepEqual(ledger.submission('at://req/impl'), FINISH_JOB) - - // A plain gave_up row with no payload (or a fresh row) is not reopenable. - ledger.giveUp('at://req/plain', 'c') - assert.equal(ledger.reopenFinishing('at://req/plain'), false) - assert.equal(ledger.reopenFinishing('at://req/none'), false) - ledger.close() +it('migrates legacy finishing handoffs and submission-bearing gave_up rows back to retryable crashes', async () => { + const dir = await mkdtemp(join(tmpdir(), 'radial-ledger-migration-')) + const path = join(dir, 'ledger.sqlite') + const database = new DatabaseSync(path) + database.exec(` + CREATE TABLE turns ( + request_uri TEXT PRIMARY KEY, + request_cid TEXT NOT NULL, + state TEXT, + attempts INTEGER NOT NULL DEFAULT 0, + next_eligible_at TEXT, + container_label TEXT, + checkout_path TEXT, + accepted_ref_uri TEXT, + accepted_ref_cid TEXT, + submission_json TEXT, + updated_at TEXT + ) STRICT; + `) + const insert = database.prepare( + 'INSERT INTO turns (request_uri, request_cid, state, attempts, submission_json, updated_at) VALUES (?, ?, ?, ?, ?, ?)', + ) + insert.run('at://req/finishing', 'cid-1', 'finishing', 2, '{"legacy":true}', '2026-01-01T00:00:00Z') + insert.run('at://req/gave-up', 'cid-2', 'gave_up', 3, '{"legacy":true}', '2026-01-01T00:00:00Z') + database.close() + + try { + const ledger = new TurnLedger(path, { now: () => '2026-07-23T00:00:00.000Z' }) + for (const uri of ['at://req/finishing', 'at://req/gave-up']) { + const row = ledger.get(uri) + assert.equal(row.state, 'crashed') + assert.equal(row.attempts, 0) + assert.equal(row.nextEligibleAt, '2026-07-23T00:00:00.000Z') + assert.equal(ledger.eligible(uri), true) + } + ledger.close() + + const inspected = new DatabaseSync(path) + const submissions = inspected.prepare('SELECT submission_json FROM turns ORDER BY request_uri').all() + assert.deepEqual( + submissions.map((row) => row.submission_json), + [null, null], + ) + inspected.close() + } finally { + await rm(dir, { recursive: true, force: true }) + } }) diff --git a/packages/daemon/test/proxy.test.mjs b/packages/daemon/test/proxy.test.mjs deleted file mode 100644 index 62136b2..0000000 --- a/packages/daemon/test/proxy.test.mjs +++ /dev/null @@ -1,139 +0,0 @@ -import assert from 'node:assert/strict' -import { connect, createServer } from 'node:net' -import { it } from 'node:test' -import { createEgressProxy, isHostAllowed, parseProxyArgs } from '../dist/index.js' - -it('isHostAllowed matches exactly, case-insensitively, and strips a trailing port', () => { - assert.equal(isHostAllowed('api.anthropic.com', ['api.anthropic.com']), true) - assert.equal(isHostAllowed('API.ANTHROPIC.COM', ['api.anthropic.com']), true) - assert.equal(isHostAllowed('api.anthropic.com:443', ['api.anthropic.com']), true) - assert.equal(isHostAllowed('evil.test', ['api.anthropic.com']), false) - assert.equal(isHostAllowed('sub.api.anthropic.com', ['api.anthropic.com']), false) - assert.equal(isHostAllowed('', ['api.anthropic.com']), false) -}) - -// --- FIX #12: deny-all (--allow-none) must survive CLI arg parsing distinctly from "unset" ----- - -it('parseProxyArgs(["--allow-none"]) is explicit deny-all: allow: []', () => { - assert.deepEqual(parseProxyArgs(['--allow-none']), { allow: [], port: 8080 }) -}) - -it('parseProxyArgs([]) with no --allow/--allow-none at all falls back to the default allowlist', () => { - assert.deepEqual(parseProxyArgs([]), { allow: ['api.anthropic.com'], port: 8080 }) -}) - -it('parseProxyArgs(["--allow", "x.test"]) uses exactly the given allowlist', () => { - assert.deepEqual(parseProxyArgs(['--allow', 'x.test']), { allow: ['x.test'], port: 8080 }) -}) - -it('parseProxyArgs honors --port alongside --allow', () => { - assert.deepEqual(parseProxyArgs(['--allow', 'x.test', '--port', '9090']), { allow: ['x.test'], port: 9090 }) -}) - -function connectRaw(port) { - return new Promise((resolvePromise, reject) => { - const socket = connect({ host: '127.0.0.1', port }) - socket.on('connect', () => resolvePromise(socket)) - socket.on('error', reject) - }) -} - -function readOnce(socket) { - return new Promise((resolvePromise) => { - socket.once('data', (chunk) => resolvePromise(chunk.toString('utf8'))) - }) -} - -it('denies a CONNECT to a host that is not on the allowlist', async () => { - const proxy = createEgressProxy(['allowed.test']) - const port = await proxy.listen(0) - try { - const socket = await connectRaw(port) - const responsePromise = readOnce(socket) - socket.write('CONNECT evil.test:443 HTTP/1.1\r\nHost: evil.test:443\r\n\r\n') - const response = await responsePromise - assert.match(response, /403/) - await new Promise((resolvePromise) => socket.on('close', resolvePromise)) - } finally { - await proxy.close() - } -}) - -// `createEgressProxy`'s optional third argument lets a test override the upstream `connect` used -// once a CONNECT target clears the allowlist/port gate. This is purely a testing seam (production -// always uses the real `node:net#connect`, unmodified) that redirects the *actual* TCP dial to a -// local, unprivileged test target while still exercising the real gate logic against a literal -// `:443` CONNECT target — binding a real listener on port 443 itself requires root on most systems. -function connectToRealTargetInsteadOf443(realPort) { - return (_options, listener) => connect({ host: '127.0.0.1', port: realPort }, listener) -} - -it('tunnels an allowlisted :443 CONNECT target byte-for-byte', async () => { - const target = createServer((socket) => { - socket.on('data', (chunk) => socket.write(chunk)) - }) - await new Promise((resolvePromise) => target.listen(0, '127.0.0.1', resolvePromise)) - const targetPort = target.address().port - - const proxy = createEgressProxy(['127.0.0.1'], connectToRealTargetInsteadOf443(targetPort)) - const proxyPort = await proxy.listen(0) - - try { - const socket = await connectRaw(proxyPort) - const connectResponsePromise = readOnce(socket) - socket.write('CONNECT 127.0.0.1:443 HTTP/1.1\r\nHost: 127.0.0.1:443\r\n\r\n') - const connectResponse = await connectResponsePromise - assert.match(connectResponse, /200/) - - const echoPromise = readOnce(socket) - socket.write('ping') - const echoed = await echoPromise - assert.equal(echoed, 'ping') - - socket.end() - await new Promise((resolvePromise) => socket.on('close', resolvePromise)) - } finally { - await proxy.close() - await new Promise((resolvePromise) => target.close(resolvePromise)) - } -}) - -// --- FIX #9: CONNECT is restricted to port 443, even for an allowlisted host -------------------- - -it('denies a CONNECT to an allowlisted host on a non-443 port', async () => { - const proxy = createEgressProxy(['allowed.test']) - const port = await proxy.listen(0) - try { - const socket = await connectRaw(port) - const responsePromise = readOnce(socket) - socket.write('CONNECT allowed.test:22 HTTP/1.1\r\nHost: allowed.test:22\r\n\r\n') - const response = await responsePromise - assert.match(response, /403/) - await new Promise((resolvePromise) => socket.on('close', resolvePromise)) - } finally { - await proxy.close() - } -}) - -it('tunnels the same allowlisted host on port 443 (denied on 22 above, allowed here)', async () => { - const target = createServer((socket) => { - socket.on('data', (chunk) => socket.write(chunk)) - }) - await new Promise((resolvePromise) => target.listen(0, '127.0.0.1', resolvePromise)) - const targetPort = target.address().port - - const proxy = createEgressProxy(['allowed.test'], connectToRealTargetInsteadOf443(targetPort)) - const proxyPort = await proxy.listen(0) - try { - const socket = await connectRaw(proxyPort) - const connectResponsePromise = readOnce(socket) - socket.write('CONNECT allowed.test:443 HTTP/1.1\r\nHost: allowed.test:443\r\n\r\n') - const connectResponse = await connectResponsePromise - assert.match(connectResponse, /200/) - socket.end() - await new Promise((resolvePromise) => socket.on('close', resolvePromise)) - } finally { - await proxy.close() - await new Promise((resolvePromise) => target.close(resolvePromise)) - } -}) diff --git a/packages/daemon/test/turn-socket.test.mjs b/packages/daemon/test/turn-socket.test.mjs index e6b5cd3..a779d9f 100644 --- a/packages/daemon/test/turn-socket.test.mjs +++ b/packages/daemon/test/turn-socket.test.mjs @@ -7,7 +7,7 @@ import { it } from 'node:test' import { CredentialClient, createSession } from '../../atproto/dist/index.js' import { COLLECTIONS } from '../../core/dist/index.js' import { LocalPds } from '../../atproto/test/local-pds.mjs' -import { TurnSocketServer, planArtifactRkey } from '../dist/index.js' +import { TurnSocketServer, implArtifactRkey, planArtifactRkey } from '../dist/index.js' const REQUEST = { uri: 'at://did:plc:human/com.disnetdev.radial.artifactRequest/req1', @@ -277,159 +277,97 @@ it('planArtifactRkey is deterministic and a valid record key', () => { assert.notEqual(rkey, planArtifactRkey(REQUEST.uri, 'other-cid')) }) -// --- Implementation submit path (deferred finish) --------------------------- - -it('implementation submit: accepted without a ref, fires onSubmission, writes NO record', async () => { +it('implementation submit synchronously writes the deterministic artifact with validated links and prev', async () => { await withTempDir(async (directory) => { const pds = new LocalPds('did:plc:agent-impl') const client = await makeClient(pds) - const submissions = [] + const prev = { uri: 'at://did:plc:agent/com.disnetdev.radial.artifact/impl-old', cid: 'oldcid' } + const branch = 'radial/impl-123456789abc' const { server, socketPath } = await startServer(directory, client, { - context: { artifactType: 'implementation', onSubmission: (s) => submissions.push(s) }, + context: { artifactType: 'implementation', implementation: { gitUrl: 'https://github.com/acme/widget.git', branch, prev } }, }) try { - const response = await sendLine(socketPath, { - method: 'submitArtifact', - token: TOKEN, - body: 'impl summary', - commit: '9b81c449326f345795402bedc72e1c95ea944458', - criteria: ['builds'], - }) + const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'done', branch, commit: 'a'.repeat(40), pr: 'https://github.com/acme/widget/pull/7' }) assert.equal(response.ok, true) - assert.equal(response.accepted, true) - assert.equal(response.ref, undefined) - assert.deepEqual(submissions, [{ commit: '9b81c449326f345795402bedc72e1c95ea944458', body: 'impl summary', criteria: ['builds'] }]) - // The socket writes NO artifact record for an implementation turn. - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 0) - assert.deepEqual(server.observation.submission, submissions[0]) - } finally { - await server.close() - } + assert.equal(response.ref.uri, `at://${pds.did}/${COLLECTIONS.artifact}/${implArtifactRkey(REQUEST.uri, REQUEST.cid)}`) + const record = [...pds.records.values()].find((entry) => entry.value.$type === COLLECTIONS.artifact).value + assert.deepEqual(record.links, { branch, commit: 'a'.repeat(40), pr: 'https://github.com/acme/widget/pull/7' }) + assert.deepEqual(record.prev, prev) + } finally { await server.close() } }) }) -it('implementation submit without a commit is rejected', async () => { +it('implementation submit rejects the wrong branch, noncanonical PR URLs, and cross-repository PRs', async () => { await withTempDir(async (directory) => { - const pds = new LocalPds('did:plc:agent-impl-nocommit') + const pds = new LocalPds('did:plc:agent-impl-validation') const client = await makeClient(pds) + const branch = 'radial/impl-123456789abc' const { server, socketPath } = await startServer(directory, client, { - context: { artifactType: 'implementation', onSubmission: () => {} }, - }) - try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'x' }) - assert.equal(response.ok, false) - assert.match(response.error, /--commit/) - } finally { - await server.close() - } - }) -}) - -it('a malformed (non-SHA) commit is rejected at the envelope layer', async () => { - await withTempDir(async (directory) => { - const pds = new LocalPds('did:plc:agent-impl-badsha') - const client = await makeClient(pds) - const { server, socketPath } = await startServer(directory, client, { - context: { artifactType: 'implementation', onSubmission: () => {} }, + context: { + artifactType: 'implementation', + implementation: { gitUrl: 'https://github.com/acme/widget.git', branch }, + }, }) - try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'x', commit: 'not-a-sha' }) - assert.equal(response.ok, false) - assert.match(response.error, /invalid turn RPC envelope/) - } finally { - await server.close() - } - }) -}) - -it('the artifact type comes from the daemon-side context, never from the envelope', async () => { - await withTempDir(async (directory) => { - const pds = new LocalPds('did:plc:agent-type-context') - const client = await makeClient(pds) - // A plan-context server; the envelope tries to smuggle a different `type`. - const { server, socketPath } = await startServer(directory, client, { context: { artifactType: 'plan' } }) - try { - const response = await sendLine(socketPath, { + const submit = (overrides) => + sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, - body: 'the plan body', - type: 'implementation', // ignored — not part of the envelope schema + body: 'done', + branch, + commit: 'a'.repeat(40), + pr: 'https://github.com/acme/widget/pull/7', + ...overrides, }) - assert.equal(response.ok, true) - const stored = [...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact) - assert.equal(stored.length, 1) - assert.equal(stored[0].value.type, 'plan') // context wins - } finally { - await server.close() - } - }) -}) -// --- finding 10: implementation commit must be a full 40-char lowercase sha -- - -it('an abbreviated commit is rejected with a clear error', async () => { - await withTempDir(async (directory) => { - const pds = new LocalPds('did:plc:agent-abbrev') - const client = await makeClient(pds) - const { server, socketPath } = await startServer(directory, client, { - context: { artifactType: 'implementation', onSubmission: () => {} }, - }) try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'x', commit: '9b81c449326f' }) - assert.equal(response.ok, false) - assert.match(response.error, /full 40-character lowercase hex/) + const wrongBranch = await submit({ branch: 'radial/impl-wrong' }) + assert.equal(wrongBranch.ok, false) + assert.match(wrongBranch.error, /daemon-selected/) + + const noncanonical = await submit({ pr: 'https://github.com/acme/widget/pull/7?diff=split' }) + assert.equal(noncanonical.ok, false) + assert.match(noncanonical.error, /canonical/) + + const crossRepo = await submit({ pr: 'https://github.com/attacker/widget/pull/7' }) + assert.equal(crossRepo.ok, false) + assert.match(crossRepo.error, /not for this project/) + assert.equal(pds.records.size, 0) } finally { await server.close() } }) }) -it('an uppercased commit is rejected with a clear error', async () => { +it('implementation adoption refuses an existing deterministic record with different criteria', async () => { await withTempDir(async (directory) => { - const pds = new LocalPds('did:plc:agent-upper') + const pds = new LocalPds('did:plc:agent-impl-adoption') const client = await makeClient(pds) - const { server, socketPath } = await startServer(directory, client, { - context: { artifactType: 'implementation', onSubmission: () => {} }, - }) - try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'x', commit: '9B81C449326F345795402BEDC72E1C95EA944458' }) - assert.equal(response.ok, false) - assert.match(response.error, /full 40-character lowercase hex/) - } finally { - await server.close() + const branch = 'radial/impl-123456789abc' + const context = { + artifactType: 'implementation', + implementation: { gitUrl: 'https://github.com/acme/widget.git', branch }, + } + const frame = { + method: 'submitArtifact', + token: TOKEN, + body: 'done', + branch, + commit: 'a'.repeat(40), + pr: 'https://github.com/acme/widget/pull/7', } - }) -}) -// --- finding 7 (plan path): adoption verifies the record type --------------- + const first = await startServer(directory, client, { context }) + const firstResponse = await sendLine(first.socketPath, { ...frame, criteria: ['original'] }) + await first.server.close() + assert.equal(firstResponse.ok, true) -it('plan-path adoption refuses a colliding record of a different type', async () => { - await withTempDir(async (directory) => { - const pds = new LocalPds('did:plc:agent-adopt') - const client = await makeClient(pds) - // Pre-seed a WRONG-type record at the deterministic plan rkey so create() collides. - const rkey = planArtifactRkey(REQUEST.uri, REQUEST.cid) - const uri = `at://${pds.did}/${COLLECTIONS.artifact}/${rkey}` - pds.records.set(uri, { - uri, - cid: 'seed-cid', - value: { - $type: COLLECTIONS.artifact, - request: { uri: REQUEST.uri, cid: REQUEST.cid }, - goal: REQUEST.goal, - type: 'implementation', // WRONG type at the plan rkey - body: 'x', - links: {}, - createdAt: NOW, - }, - }) - const { server, socketPath } = await startServer(directory, client, { context: { artifactType: 'plan' } }) + const second = await startServer(directory, client, { context }) try { - const response = await sendLine(socketPath, { method: 'submitArtifact', token: TOKEN, body: 'the plan body' }) - assert.equal(response.ok, false) - assert.match(response.error, /refusing to adopt/) + const secondResponse = await sendLine(second.socketPath, { ...frame, criteria: ['tampered'] }) + assert.equal(secondResponse.ok, false) + assert.match(secondResponse.error, /refusing to adopt tampered record/) } finally { - await server.close() + await second.server.close() } }) }) diff --git a/packages/daemon/test/turn.test.mjs b/packages/daemon/test/turn.test.mjs index 608f57a..3159c28 100644 --- a/packages/daemon/test/turn.test.mjs +++ b/packages/daemon/test/turn.test.mjs @@ -7,7 +7,13 @@ import { it } from 'node:test' import { createSession, CredentialClient } from '../../atproto/dist/index.js' import { COLLECTIONS } from '../../core/dist/index.js' import { LocalPds } from '../../atproto/test/local-pds.mjs' -import { ClaudeCodeHarness, FakeContainerRunner, planArtifactRkey, runTurn } from '../dist/index.js' +import { + ClaudeCodeHarness, + FakeContainerRunner, + implArtifactRkey, + planArtifactRkey, + runTurn, +} from '../dist/index.js' const GOAL = { uri: 'at://did:plc:human/com.disnetdev.radial.goal/g1', cid: 'goal-cid' } @@ -158,6 +164,78 @@ it('fulfilled: submitArtifact over the derived socket produces a fulfilled outco }) }) +it('implementation turn injects only operator GitHub/model credentials and synchronously records its PR links', async () => { + const { pds, actor } = await makeActor('did:plc:agent-implementation') + const branch = 'radial/impl-123456789abc' + const commit = 'a'.repeat(40) + const pr = 'https://github.com/acme/radial-ng/pull/45' + const implementationBundle = { + ...BUNDLE, + request: { ...BUNDLE.request, type: 'implementation' }, + project: { ...BUNDLE.project, gitUrl: 'https://github.com/acme/radial-ng.git' }, + } + + await withRunDir(async (runDir) => { + const runner = new FakeContainerRunner(async (spec) => { + const expectedUri = `at://${actor.did}/${COLLECTIONS.artifact}/${implArtifactRkey( + BUNDLE.request.uri, + BUNDLE.request.cid, + )}` + assert.equal(spec.env.GH_TOKEN, 'github-token') + assert.equal(spec.env.ANTHROPIC_API_KEY, 'model-token') + assert.equal(spec.env.RADIAL_ARTIFACT_URI, expectedUri) + assert.equal(spec.env.RADIAL_BRANCH, branch) + assert.equal(spec.env.RADIAL_BASE_BRANCH, 'main') + assert.equal(spec.env.RADIAL_AGENT_DID, actor.did) + assert.match(spec.env.GIT_AUTHOR_NAME, new RegExp(actor.did)) + assert.equal(spec.env.GIT_COMMITTER_NAME, spec.env.GIT_AUTHOR_NAME) + assert.equal(spec.env.accessJwt, undefined) + assert.equal(spec.env.refreshJwt, undefined) + assert.equal(spec.env.PDS_PASSWORD, undefined) + assert.equal(spec.mounts.find((mount) => mount.target === '/work').readOnly, false) + assert.equal(spec.mounts.some((mount) => mount.target === '/export'), false) + + const response = await sendLine(hostSocketPath(spec), { + method: 'submitArtifact', + token: spec.env.RADIAL_TURN_TOKEN, + body: 'Implemented phase 4.5.', + branch, + commit, + pr, + }) + assert.equal(response.ok, true) + assert.equal(response.ref.uri, expectedUri) + return { exitCode: 0, timedOut: false } + }) + + const result = await runTurn( + baseInput(runDir, { + actor, + bundle: implementationBundle, + artifactType: IMPL_ARTIFACT_TYPE, + requestRecord: IMPL_REQUEST_RECORD, + branch, + }), + { + runner, + harness: new ClaudeCodeHarness(), + checkout: noopCheckout, + githubToken: 'github-token', + anthropicApiKey: 'model-token', + }, + ) + + assert.equal(result.outcome, 'fulfilled') + assert.equal( + result.acceptedRef.uri, + `at://${actor.did}/${COLLECTIONS.artifact}/${implArtifactRkey(BUNDLE.request.uri, BUNDLE.request.cid)}`, + ) + const record = [...pds.records.values()].find((entry) => entry.value.$type === COLLECTIONS.artifact) + assert.deepEqual(record.value.links, { branch, commit, pr }) + await assert.rejects(stat(runDir)) + }) +}) + it('crashed: a container that produces no socket observation is classified crashed regardless of exit code', async () => { const { actor } = await makeActor('did:plc:agent-crash') await withRunDir(async (runDir) => { @@ -308,130 +386,3 @@ it('propagates a container-runner infra error to the caller (never silently swal await assert.rejects(stat(runDir)) }) }) - -// --- Implementation turn mounts/env matrix ---------------------------------- - -const implCheckout = async ({ dest }) => { - await mkdir(dest, { recursive: true }) - return { path: dest, commit: 'abc1234' } // RADIAL_BASE_COMMIT -} - -it('plan turn: /work is read-only, there is no /export mount, and no git identity env', async () => { - const { actor } = await makeActor('did:plc:agent-plan-env') - await withRunDir(async (runDir) => { - let captured - const runner = new FakeContainerRunner(async (spec) => { - captured = spec - return { exitCode: 0, timedOut: false } - }) - await runTurn(baseInput(runDir, { actor }), { runner, harness: new ClaudeCodeHarness(), checkout: noopCheckout }) - const work = captured.mounts.find((m) => m.target === '/work') - assert.equal(work.readOnly, true) - assert.equal(captured.mounts.find((m) => m.target === '/export'), undefined) - assert.equal(captured.env.RADIAL_BASE_COMMIT, undefined) - assert.equal(captured.env.GIT_AUTHOR_NAME, undefined) - }) -}) - -it('implementation turn: rw /work + rw /export, GIT_* identity + RADIAL_BASE_COMMIT, and NO tokens', async () => { - const { actor } = await makeActor('did:plc:agent-impl-env') - await withRunDir(async (runDir) => { - let captured - const runner = new FakeContainerRunner(async (spec) => { - captured = spec - return { exitCode: 0, timedOut: false } - }) - await runTurn( - baseInput(runDir, { actor, artifactType: IMPL_ARTIFACT_TYPE, requestRecord: IMPL_REQUEST_RECORD }), - { runner, harness: new ClaudeCodeHarness(), checkout: implCheckout }, - ) - const work = captured.mounts.find((m) => m.target === '/work') - const exp = captured.mounts.find((m) => m.target === '/export') - assert.equal(work.readOnly, false) - assert.ok(exp && exp.readOnly === false) - assert.equal(captured.env.RADIAL_BASE_COMMIT, 'abc1234') - assert.equal(captured.env.GIT_AUTHOR_NAME, actor.session.handle) - assert.equal(captured.env.GIT_COMMITTER_NAME, actor.session.handle) - assert.ok(captured.env.GIT_AUTHOR_EMAIL.includes(actor.session.handle)) - // No credentials of any kind reach the container (deps supplied none). - assert.equal(captured.env.ANTHROPIC_API_KEY, undefined) - assert.equal(captured.env.GITHUB_TOKEN, undefined) - }) -}) - -const SUBMIT_SHA = '9b81c449326f345795402bedc72e1c95ea944458' // full 40-char lowercase hex - -it('implementation turn: a submit-with-commit yields the `submitted` outcome, fires onSubmission, and KEEPS runDir', async () => { - const { pds, actor } = await makeActor('did:plc:agent-impl-submit') - await withRunDir(async (runDir) => { - const submissions = [] - const runner = new FakeContainerRunner(async (spec) => { - // The container writes its ranged bundle to the rw /export mount before submitting; the daemon - // snapshots it at submit time (a missing/irregular bundle would fail the submit). - const exportMount = spec.mounts.find((m) => m.target === '/export') - await writeFile(join(exportMount.source, 'impl.bundle'), 'BUNDLE-BYTES') - const response = await sendLine(hostSocketPath(spec), { - method: 'submitArtifact', - token: spec.env.RADIAL_TURN_TOKEN, - body: 'impl summary', - commit: SUBMIT_SHA, - }) - assert.equal(response.ok, true) - assert.equal(response.accepted, true) // no record ref yet - assert.equal(response.ref, undefined) - return { exitCode: 0, timedOut: false } - }) - const result = await runTurn( - baseInput(runDir, { - actor, - artifactType: IMPL_ARTIFACT_TYPE, - requestRecord: IMPL_REQUEST_RECORD, - onSubmission: (submission, baseCommit, bundlePath) => submissions.push({ submission, baseCommit, bundlePath }), - }), - { runner, harness: new ClaudeCodeHarness(), checkout: implCheckout }, - ) - assert.equal(result.outcome, 'submitted') - assert.equal(result.submission.commit, SUBMIT_SHA) - assert.equal(result.submission.body, 'impl summary') - // onSubmission fired at submit time with the container payload + the checkout head sha + snapshot. - assert.equal(submissions.length, 1) - assert.equal(submissions[0].submission.commit, SUBMIT_SHA) - assert.equal(submissions[0].baseCommit, 'abc1234') - assert.ok(submissions[0].bundlePath.endsWith('impl.bundle')) - // The snapshot is a daemon-owned copy under runDir, NOT the container-mounted /export path. - assert.equal(await readFile(submissions[0].bundlePath, 'utf8'), 'BUNDLE-BYTES') - // No artifact record was written by the socket (the daemon finish step owns that). - assert.equal([...pds.records.values()].filter((r) => r.value.$type === COLLECTIONS.artifact).length, 0) - // runDir is KEPT for the finish pump (holds the bundle snapshot + finishing handoff). - await stat(runDir) - }) -}) - -it('implementation turn: a submit whose /export/impl.bundle is a symlink is rejected (no submission)', async () => { - const { actor } = await makeActor('did:plc:agent-impl-symlink') - await withRunDir(async (runDir) => { - let socketError - const runner = new FakeContainerRunner(async (spec) => { - const exportMount = spec.mounts.find((m) => m.target === '/export') - // A hostile container replaces the bundle with a symlink to an out-of-tree file. - const target = join(runDir, 'secret') - await writeFile(target, 'SECRET') - await symlink(target, join(exportMount.source, 'impl.bundle')) - const response = await sendLine(hostSocketPath(spec), { - method: 'submitArtifact', - token: spec.env.RADIAL_TURN_TOKEN, - body: 'impl summary', - commit: SUBMIT_SHA, - }) - socketError = response - return { exitCode: 0, timedOut: false } - }) - const result = await runTurn( - baseInput(runDir, { actor, artifactType: IMPL_ARTIFACT_TYPE, requestRecord: IMPL_REQUEST_RECORD, onSubmission: () => {} }), - { runner, harness: new ClaudeCodeHarness(), checkout: implCheckout }, - ) - assert.equal(socketError.ok, false) - assert.match(socketError.error, /could not be persisted/) - assert.equal(result.outcome, 'crashed') // no submission observed - }) -}) diff --git a/packages/sidecar/src/cli.ts b/packages/sidecar/src/cli.ts index b667ca7..a595535 100644 --- a/packages/sidecar/src/cli.ts +++ b/packages/sidecar/src/cli.ts @@ -61,15 +61,8 @@ export async function main(args = argv.slice(2)): Promise { const rpc = await buildTurnRpc(args, token, async (path) => (path === '-' ? readStdin() : readFile(path, 'utf8'))) const response = await sendTurnRpc(process.env.RADIAL_SIDECAR_SOCKET, rpc) if (!response.ok) throw new Error(response.error) - // An implementation submission is accepted without a record ref (the daemon writes the artifact - // record later, in its finish step); a plan-style submission / question returns the record ref. - if ('ref' in response) { - if (args.includes('--json')) console.log(JSON.stringify({ ref: response.ref })) - else console.log(`${response.ref.uri}#${response.ref.cid}`) - } else { - if (args.includes('--json')) console.log(JSON.stringify({ accepted: true })) - else console.log('accepted') - } + if (args.includes('--json')) console.log(JSON.stringify({ ref: response.ref })) + else console.log(`${response.ref.uri}#${response.ref.cid}`) return } diff --git a/packages/sidecar/src/socket.ts b/packages/sidecar/src/socket.ts index 96d33c0..cdf045e 100644 --- a/packages/sidecar/src/socket.ts +++ b/packages/sidecar/src/socket.ts @@ -43,12 +43,16 @@ export async function buildTurnRpc( // Implementation turns pass the pushed HEAD sha via --commit; the daemon validates its shape and // owns everything else (branch naming, push, PR). Plan-style submits omit it. const commit = lastValue(args, '--commit') + const branch = lastValue(args, '--branch') + const pr = lastValue(args, '--pr') return { method: 'submitArtifact', token, body, ...(criteria.length ? { criteria } : {}), ...(commit !== undefined ? { commit } : {}), + ...(branch !== undefined ? { branch } : {}), + ...(pr !== undefined ? { pr } : {}), } } diff --git a/scripts/build-images.mjs b/scripts/build-images.mjs index a0c0065..aace741 100644 --- a/scripts/build-images.mjs +++ b/scripts/build-images.mjs @@ -1,6 +1,5 @@ #!/usr/bin/env node -// Dependency-free operator build for the two Radial Docker images (docker/Dockerfile, -// docker/proxy.Dockerfile). Run as `pnpm images` from the repo root, or directly as +// Dependency-free operator build for the Radial turn image. Run as `pnpm images` from the repo root, or directly as // `node scripts/build-images.mjs [--tag ]`. // // Docker cannot be run in this development environment (the daemon is down), so this script is @@ -55,16 +54,12 @@ function main() { const { tag } = parseArgs(process.argv.slice(2)) requireDocker() - // Build the workspace first: both Dockerfiles either copy build output directly - // (docker/proxy.Dockerfile copies packages/daemon/dist/proxy.js) or re-run `pnpm build` inside - // their own build stage (docker/Dockerfile) — running it here too fails fast, before any image + // Build the workspace first; running it here fails fast before the image build if the workspace // build, if the workspace itself doesn't build. run('pnpm', ['build']) run('docker', ['build', '-f', join('docker', 'Dockerfile'), '-t', `radial-turn:${tag}`, '.']) - run('docker', ['build', '-f', join('docker', 'proxy.Dockerfile'), '-t', `radial-proxy:${tag}`, '.']) - - console.log(`\nBuilt radial-turn:${tag} and radial-proxy:${tag}`) + console.log(`\nBuilt radial-turn:${tag}`) } try {