From 7f87a452d1e1ce33a35b82e81aa7f9ba3393e9fa Mon Sep 17 00:00:00 2001 From: Tim Disney Date: Sat, 8 Aug 2026 16:16:55 -0700 Subject: [PATCH] fix pnpm and git issues for agents --- docker/Dockerfile | 11 ++++++++- docs/adr-tangled-forge.md | 7 ++++++ packages/daemon/src/harness.ts | 5 ++-- packages/daemon/test/docker-smoke.test.mjs | 20 +++++++++++++--- packages/daemon/test/harness.test.mjs | 27 +++++++++++++++++++--- 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index e1dba5f..03c2e9b 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -8,7 +8,7 @@ # Assumptions, called out because Docker cannot be run in this development environment to verify # them directly (see the phase-3 workstream E-final report for what was and wasn't exercised): # - The build stage needs network access to the npm registry (`pnpm install`); the runtime stage -# needs it too, to `npm install -g` the three agent CLIs. Both are operator/build-time +# needs it too, to `npm install -g` pnpm and the three agent CLIs. Both are operator/build-time # concerns, not exercised by this repo's own `pnpm build`/`test` gate. # - `pnpm deploy --legacy --prod` was verified locally (outside Docker) against this workspace: # it assembles a self-contained `@radial/sidecar` install, resolving its workspace dependencies @@ -62,6 +62,15 @@ RUN apt-get update \ && apt-get install -y --no-install-recommends git gh openssh-client ripgrep ca-certificates \ && rm -rf /var/lib/apt/lists/* +# Project checks in a turn invoke pnpm directly. Corepack was enabled in the discarded build stage +# only, so without a runtime install agents first fail with "pnpm: not found" and have to discover +# an incidental package-manager path. Install the exact workspace version as a global executable; +# unlike a Corepack shim this never needs to download a package manager after the read-only +# container has started. The smoke test below exercises it as uid 1000, the same user as a turn. +RUN npm install -g --ignore-scripts pnpm@10.28.2 \ + && pnpm --version \ + && npm cache clean --force + # @anthropic-ai/claude-code: harness.ts's ClaudeCodeHarness shells out to `claude --print ...`. # Installed globally so it's on PATH for every user of the image, not just whoever installs it. # diff --git a/docs/adr-tangled-forge.md b/docs/adr-tangled-forge.md index 05f9635..ccd8383 100644 --- a/docs/adr-tangled-forge.md +++ b/docs/adr-tangled-forge.md @@ -272,6 +272,13 @@ Host keys are **pinned** from operator config. Radial will not fall back to `StrictHostKeyChecking=no`; an unpinned push is a MITM-able push, and that would quietly weaken the possession-based containment story. A tangled turn is given no `GH_TOKEN`. +The push uses an **explicit force-with-lease expectation** read from `git ls-remote` immediately +before the write. `RADIAL_PUSH_REMOTE` is an ssh URL, not a configured tracking remote, so bare +`--force-with-lease` has no remote-tracking ref to compare and rejects an otherwise safe push as +`stale info`. Naming `refs/heads/:` preserves the actual rule: a rebase may +replace the predecessor's history, but a concurrent move after the observation is refused. An empty +observed SHA means the branch must still not exist. + ### D6 — Spindles are out of scope Tangled has its own CI. Radial's check runner keeps running the project's `checks` in its own diff --git a/packages/daemon/src/harness.ts b/packages/daemon/src/harness.ts index 378e663..eb071d1 100644 --- a/packages/daemon/src/harness.ts +++ b/packages/daemon/src/harness.ts @@ -92,7 +92,8 @@ export function buildPrompt(input: { }): string { const context = [ `Read your brief at ${input.bundleDir}/brief.md — the artifact-type brief plus this request's extra instructions.`, - `For full context, also read ${input.bundleDir}/bundle.md (a human-readable summary of the goal, project, based-on artifacts, prior review findings, and thread) and ${input.bundleDir}/bundle.json (the same data, structured).`, + `Then read ${input.bundleDir}/bundle.md — the concise index of the goal, project, based-on artifacts, current system artifacts, prior review findings, and thread.`, + `${input.bundleDir}/bundle.json carries the full structured records, including artifact bodies that bundle.md intentionally lists only by title and URI. Inspect only the fields and bodies relevant to this task (for example with jq or a short script); do not dump or read the whole file by default. Keep every tool result focused: prefer searches and narrow ranges over whole files or broad command output.`, ] // Said once, in the same words, for every artifact-producing prompt below. The title is a protocol // field a human reads in a list of a hundred records, so what it must NOT be is worth stating: the @@ -146,7 +147,7 @@ export function buildPrompt(input: { '', 'When your implementation is complete:', '5. Commit your work with a `Co-Authored-By: $RADIAL_AGENT_NAME ($RADIAL_AGENT_DID) <$RADIAL_AGENT_EMAIL>` trailer.', - '6. Push it: `git push --force-with-lease "$RADIAL_PUSH_REMOTE" "HEAD:refs/heads/$RADIAL_BRANCH"`. The lease is there because step 3 rebases a branch a predecessor may already have pushed, which rewrites it; `--force-with-lease` still refuses if someone else moved the branch, and you must not turn it into a plain `--force`. The ssh key for this is already configured (`$GIT_SSH_COMMAND`); do not generate one, and do not disable host-key checking. If the push is rejected because this agent is not a collaborator on the repository, do NOT commit around it — run `radial message post --body ""` so a human can grant it, and stop.', + '6. Push it with an explicit lease, as one shell command: `remote_tip="$(git ls-remote "$RADIAL_PUSH_REMOTE" "refs/heads/$RADIAL_BRANCH" | awk \'NR == 1 {print $1}\')"; git push --force-with-lease="refs/heads/$RADIAL_BRANCH:$remote_tip" "$RADIAL_PUSH_REMOTE" "HEAD:refs/heads/$RADIAL_BRANCH"`. `$RADIAL_PUSH_REMOTE` is a URL rather than a tracking remote, so a bare `--force-with-lease` has no local remote-tracking ref and fails with "stale info" even when nobody moved the branch. This command snapshots the remote tip immediately before the push (an empty tip safely expects a still-new branch), and the explicit lease refuses if anyone moves it in between. The lease is required because step 3 may rebase predecessor history; never turn it into a plain `--force`. The ssh key for this is already configured (`$GIT_SSH_COMMAND`); do not generate one, and do not disable host-key checking. If the push is rejected because this agent is not a collaborator on the repository, do NOT commit around it — run `radial message post --body ""` so a human can grant it, and stop.', '7. Do NOT try to open a pull request — tangled has no CLI for it and Radial opens it for you from the branch you pushed, as the same identity that signs your artifact.', '8. Write a concise implementation summary to a file, then run `radial artifact submit --title "" --body-file --branch "$RADIAL_BRANCH" --commit "$(git rev-parse HEAD)"`. There is no `--pr` on this forge.', ` ${titleRule}`, diff --git a/packages/daemon/test/docker-smoke.test.mjs b/packages/daemon/test/docker-smoke.test.mjs index d7a5385..0709ced 100644 --- a/packages/daemon/test/docker-smoke.test.mjs +++ b/packages/daemon/test/docker-smoke.test.mjs @@ -2,8 +2,9 @@ // docker-exec paths in container.ts (`DockerRunner`) that cannot be exercised anywhere else in // this repo's test suite — Docker itself is not running in the default development environment. // -// 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: +// The real-Docker describes are skipped unless RADIAL_DOCKER_TESTS=1 (node:test's `skip` option — +// a clean skip, and no docker/child_process work happens when skipped). The source-only pnpm pin +// guard still runs in the ordinary suite. To run the Docker cases for real: // // pnpm images # builds radial-turn:latest // RADIAL_DOCKER_TESTS=1 node --test packages/daemon/test/docker-smoke.test.mjs @@ -45,6 +46,18 @@ const REQUEST = { goal: GOAL, } +it('the turn image installs the pnpm version pinned by the workspace', async () => { + const root = new URL('../../../', import.meta.url) + const manifest = JSON.parse(await readFile(new URL('package.json', root), 'utf8')) + const dockerfile = await readFile(new URL('docker/Dockerfile', root), 'utf8') + assert.equal(typeof manifest.packageManager, 'string') + assert.match(manifest.packageManager, /^pnpm@[^\s]+$/) + assert.ok( + dockerfile.includes(`npm install -g --ignore-scripts ${manifest.packageManager}`), + 'expected the runtime image to install the workspace packageManager exactly', + ) +}) + async function makeSignedClient() { const pds = new LocalPds(AGENT_DID) const session = await createSession(pds.service, pds.handle, 'pw', pds.fetch.bind(pds)) @@ -157,7 +170,7 @@ describe('docker smoke: radial-turn against a real Docker daemon', { skip: !RUN_ }) }) - it('the image ships every harness the daemon can select, and each one starts', async () => { + it('the image ships pnpm and every harness the daemon can select, and each one starts', async () => { // The failure mode this exists for is the one that already bit the Claude install: a package // that installs "successfully" and ships a stub which exits 1 on the first turn (see the // install.cjs note in docker/Dockerfile). `--version` proves the binary exists AND runs, with @@ -165,6 +178,7 @@ describe('docker smoke: radial-turn against a real Docker daemon', { skip: !RUN_ // in this suite can catch a harness the daemon would select and the image does not have. const runner = new DockerRunner() for (const [harness, argv] of [ + ['pnpm', ['pnpm', '--version']], ['claude', ['claude', '--version']], ['pi', ['pi', '--version']], ['codex', ['codex', '--version']], diff --git a/packages/daemon/test/harness.test.mjs b/packages/daemon/test/harness.test.mjs index 762fcf0..60e949c 100644 --- a/packages/daemon/test/harness.test.mjs +++ b/packages/daemon/test/harness.test.mjs @@ -88,6 +88,23 @@ it('ClaudeCodeHarness.invocation tolerates an empty models array by omitting --m assert.ok(!invocation.argv.includes('--model')) }) +it('the shared context prompt starts concise and makes full bundle bodies opt-in', () => { + const prompt = new ClaudeCodeHarness().invocation({ + briefPath: '/bundle/brief.md', + bundleDir: '/bundle', + workdir: '/work', + models: [], + }).argv[2] + + assert.ok(prompt.indexOf('/bundle/brief.md') < prompt.indexOf('/bundle/bundle.md')) + assert.ok(prompt.indexOf('/bundle/bundle.md') < prompt.indexOf('/bundle/bundle.json')) + assert.match(prompt, /artifact bodies that bundle\.md intentionally lists only by title and URI/) + assert.match(prompt, /Inspect only the fields and bodies relevant to this task/) + assert.match(prompt, /do not dump or read the whole file by default/) + assert.match(prompt, /prefer searches and narrow ranges over whole files or broad command output/) + assert.doesNotMatch(prompt, /the same data, structured/) +}) + it('review prompt directs the agent to judge the subject and submit a verdict without modifying the checkout', () => { const invocation = new ClaudeCodeHarness().invocation({ briefPath: '/bundle/brief.md', @@ -244,9 +261,13 @@ it('a tangled implementation pushes over ssh and does NOT open a pull request }) const prompt = invocation.argv[2] - // The push half: an ssh remote the daemon supplies, with the key it mounted. The lease is there - // because this forge's prompt rebases, which rewrites a branch a predecessor may have pushed. - assert.match(prompt, /git push --force-with-lease "\$RADIAL_PUSH_REMOTE" "HEAD:refs\/heads\/\$RADIAL_BRANCH"/) + // The push half: an ssh URL the daemon supplies, with the key it mounted. A URL does not provide + // the remote-tracking ref an implicit lease needs, so the prompt snapshots the exact remote tip + // and names it in the lease. This remains safe for both an existing and a not-yet-created branch. + assert.match(prompt, /remote_tip="\$\(git ls-remote "\$RADIAL_PUSH_REMOTE" "refs\/heads\/\$RADIAL_BRANCH"/) + assert.match(prompt, /--force-with-lease="refs\/heads\/\$RADIAL_BRANCH:\$remote_tip"/) + assert.match(prompt, /an empty tip safely expects a still-new branch/) + assert.doesNotMatch(prompt, /git push --force-with-lease "\$RADIAL_PUSH_REMOTE"/) assert.match(prompt, /Co-Authored-By: \$RADIAL_AGENT_NAME \(\$RADIAL_AGENT_DID\)/) assert.match(prompt, /do not disable host-key checking/) // The submit half: no --pr, because the daemon opens the pull and stamps the link itself. -- 2.51.2