Commit Graph

27 Commits

Author SHA1 Message Date
Colin McDonnell a0576a702a opencode v2: harness adapted to opencode-ai 1.15+ SDK-v2 / Effect-ts CLI rewrite (#767)
* opencode v2: harness adapted to opencode-ai 1.15+ SDK-v2 / Effect-ts CLI rewrite

Bumps `opencode-ai` from `1.1.56` → `1.15.1` and ports the harness to the
v2 NDJSON event contract. The legacy `opencode.ts` is kept as reference;
`opencode_v2.ts` is the active runner via `agents/index.ts`.

Why: `1.1.56` doesn't echo Gemini `thought_signature` back through the
MCP tool-call serializer, so direct-Google reviews 400 on the 3rd-ish
tool call. The fix only exists in the `1.14.x`+ line, which also ships
the SDK-v2 / Effect-ts CLI rewrite — taking the rewrite is mandatory.
Also unblocks the Codex ChatGPT-subscription auth path.

Surface area:

  - drop `init` / `message` / `result` / `tool_result` event types and
    handlers (no longer emitted at v1.14+ per upstream
    `cli/cmd/run.ts:588-601`).
  - `tool_use` is now a single event covering both `state.status:
    "completed"` and `"error"`. duration / subagent-finish bookkeeping
    moves from the v1 `tool_result` handler into the consolidated
    `tool_use` handler.
  - new `reasoning` event handler — gated on `--thinking`, surfaces
    Gemini-3 / OpenAI / Anthropic thinking blocks. `--thinking` added to
    `baseArgs`.
  - drop `pendingTaskDispatches` FIFO + `knownNonTaskCallIDs` set: at
    v1.15 the `task` tool callID is stable across the whole
    `tool-input-* → tool-call → tool-result/tool-error` chain
    (`session/processor.ts:282-330`). exact-match map is sufficient.
  - drop `experimental.batch_tool: true` from injected config — declared
    but inert at v1.15. re-add once upstream wires it back.
  - bin path: `bin/opencode` → `bin/opencode.exe` (postinstall renames
    the platform-specific binary into `opencode.exe` for every OS now).

Validated locally:

  - `pnpm test` 610/610 ✓
  - `pnpm play --raw` end-to-end with Anthropic via OpenRouter ✓
  - `pnpm play --raw` with `google/gemini-3.1-pro-preview`: 6 tool calls,
    multiple reasoning blocks visible, `set_output` propagates, exit 0 ✓
    (this is the headline `thought_signature` fix)
  - runtest opencode: smoke ✓, restricted ✓, nobash ✓, token-exfil ✓
  - runtest opencode: skill-invoke and mcpmerge fail (model-behavior
    drift on the new system prompt; wiring confirmed intact via direct
    repro showing both `robinMCP` and `pullfrog` MCP tools exposed).
    Tracked for follow-up; does not gate the migration.

Plugin (`opencodePlugin.ts`) and skill discovery paths are unchanged at
v1.15 — verified upstream and reused as-is. Bus subscription via
`bus.subscribeAll()` and the `event` hook still fan out every payload.

* model-smoke: bump opencode bin path to opencode.exe (v1.14+ rename)

The v1.14+ postinstall.mjs renames the platform-specific binary to
`bin/opencode.exe` for every OS (incl. linux/darwin), not just Windows.
Mirrors the fix in action/agents/opencode_v2.ts.

* opencode v2: set PWD env explicitly to fix skill / project-config discovery

Root cause for skill-invoke + mcpmerge harness regressions: opencode-ai 1.15
reads `process.env.PWD` first (with `process.cwd()` as fallback) when
resolving the SDK client's `directory` parameter — see upstream
`cli/cmd/run.ts:282`:

  const root = Filesystem.resolve(process.env.PWD ?? process.cwd())

We pass `cwd: repoDir` to spawn, but the child inherits the harness's PWD
via `...process.env`. Under `pnpm runtest` (and `pnpm play`) PWD is the
`action/` directory, not the cloned test repo. Result: opencode creates
two instances per session — one at `process.cwd()` (correct) and one at
`PWD` (wrong) — and the agent's session runs in the PWD-derived one,
which can't see the project's `.opencode/skills/` or `.claude/skills/`.

Empirically traced via the full opencode stderr trace under the runtest
harness: `service=skill count=3 init` (no `pullfrog-skill-check`) plus a
second `service=default directory=<harness-pwd> creating instance` line
per run. With `PWD=repoDir` set explicitly, `count=4 init` includes the
test skill, the agent reaches for `skill({"name":"pullfrog-skill-check"})`
exactly as the validator expects, and mcpmerge's `robinMCP_get_test_value`
becomes accessible too.

Validated locally: skill-invoke-opencode ✓, mcpmerge-opencode ✓, smoke ✓,
restricted ✓, nobash ✓, token-exfil ✓ (flaked once on a model-narration
match, passes on retry; unrelated to PWD).

* opencode v2: drop ThinkingTimer; use opencode's reasoning.part.time directly

opencode-ai 1.15 emits `reasoning` parts with `time.start` / `time.end`
on terminal state (`cli/cmd/run.ts:671`), giving us a precise per-block
"thought for X s" duration straight from the runtime. The v1
ThinkingTimer heuristic — measuring wall-clock between markToolResult
and the next markToolCall — was an approximation when no native source
existed; with v2 it's redundant and noisy (it would log alongside the
real reasoning event, and conflated network latency with model thinking).

Removed: `ThinkingTimer` import, `thinkingTimers` Map, `timerFor()`
helper, both `markToolCall` / `markToolResult` call sites in `tool_use`.
The `reasoning` handler now reads `part.time.start/end` directly and
prefixes the visible preview with `(X.Ys)`.

Output before: `» thinking: <preview>` + `» thought for 4.0s` (separate)
Output now:    `» thinking (4.0s): <preview>` (one line, sourced)

For models that don't emit reasoning (Sonnet without extended thinking,
GPT-4o, etc.), there's just no thinking line — which matches reality
better than the gap-heuristic, which would fire on any pause >3s
including provider-side latency that wasn't actual model reasoning.

Validated locally: skill-invoke ✓, mcpmerge ✓, smoke ✓, Gemini play
shows `» thinking (4.0s)` and `» thinking (0.8s)` from real durations.

* claude.ts: same PWD fix as opencode v2; entryPost: refresh stale comment

claude-code 2.1.x reads `process.env.PWD` and registers it as a "session"
additional-working-directory when it differs from `process.cwd()` (per the
bundled cli.js: `let H = process.env.PWD; if (H && H !== Y7() && ...)
j.set(H, { path: H, source: "session" })`). Without overriding PWD on the
spawn env, claude inherits the harness's PWD via `...process.env` — under
`pnpm runtest` / `pnpm play` that's `action/`, not the cloned test repo —
and adds the wrong dir to the agent's allowed working set.

Symmetric to the opencode v2 fix in 52337f9. Pre-empts the same class of
"agent's session sees the wrong cwd" failures on the claude side.

Also refresh the stale `action/agents/opencode.ts` reference in
entryPost.ts to point at opencode_v2.ts (the active runner), with the v1
file noted as kept-for-reference.

* opencode: extract shared helpers into opencodeShared.ts; v2 cleanup

Code-quality pass on the v2 work:

1. New `agents/opencodeShared.ts` (144 lines) for genuinely-shared helpers
   between v1 and v2:
   - `OpenCodeConfig` type
   - `geminiHighThinkingOverrides()` (registry-driven Gemini thinking pin)
   - `buildReviewerAgentConfig()` (reviewfrog config builder, was in v1
     and re-imported by v2 via a back-reference)
   - `installOpencodeCli({ binPath })` (parameterized — v1 passes
     `bin/opencode`, v2 passes `bin/opencode.exe` via a per-version
     `installCli` lambda; matches each pinned version's npm shape)
   - `autoSelectModel()` + `getOpenCodeModels()` model-registry fallback

   v2 drops the `import { ... } from "./opencode.ts"` back-reference; v1
   keeps a one-line `export { geminiHighThinkingOverrides }` re-export
   so `opencode.test.ts` keeps working unchanged. Once v1 is retired
   (post burn-in) opencodeShared collapses back into v2.

2. `opencode_v2.ts` cleanup:
   - drop dead state (`currentStepId`, `stepHistory` were write-only —
     their reader was the v1 `tool_result` handler we deleted)
   - hoist `state` in `tool_use` handler; replace nested-ternary payload
     extraction with a `terminalPayload(state)` helper
   - extract `formatPartDuration(time)` for the reasoning-block
     "(X.Ys)" suffix
   - tighten `OpenCodeBusEnvelopeEvent` type to include `tool` /
     `callID` fields directly, drop the `partWithToolFields` cast
   - trim docblocks per AGENTS.md "≤ 2-3 lines per code line": reasoning
     handler, tool_use handler, bus envelope handler all shortened
   - `step_start` becomes an explicit `() => {}` no-op so the dispatcher
     doesn't log "unhandled event" for every step

3. `subagentRegistration.test.ts` retargeted at the new file split —
   reads opencodeShared.ts for the buildReviewerAgentConfig assertions
   and opencode_v2.ts for the orchestrator-model wire-through.

Net: -306 source lines (1339+1130 → 1228+1031+144). Tests + lint + format
+ typecheck all green; skill-invoke-opencode ✓ and smoke ✓ verified
against the refactored v2 runtime.

* opencode v2: address PR review feedback

Three fixes from the inline review threads on #767:

1. Activity-diagnostic ordering bug (Copilot review at L705): the chunk-
   level `markActivity()` resets the module-level idle counter, so the
   per-event `getIdleMs()` sample inside the dispatch loop was always
   ~0ms — the "no activity for Xs" diagnostic never fired. Replaced with
   a runner-local `lastEventAt` so we measure real event-to-event silence
   instead of chunk-arrival latency. Drop the unused `getIdleMs` import.

2. TDZ-defensive hoist (Pullfrog review nit): `agentErrorEvent`,
   `lastProviderError`, and `recentStderr` are closed over by the
   `handlers` const but were declared after it. No current bug because
   handlers only fire inside the awaited `spawn()`, but a future
   refactor that triggers a handler synchronously during setup would
   surface a TDZ. Hoisted above `handlers`.

3. `step_finish.part.tokens.reasoning` follow-up (Pullfrog review at
   L566): leave a `TODO` comment marking the gap until `AgentUsage`
   grows a `reasoningTokens` field — separate PR with schema work.
   Cost totals stay correct because `part.cost` is summed independently.

Other thread states for the record:
- Copilot L63 (geminiHighThinkingOverrides import from legacy): already
  fixed by the opencodeShared.ts extraction in 83a7cab.
- Copilot L672 (ThinkingTimer over-reports on terminal events): already
  fixed by dropping ThinkingTimer in a1e536b — we use opencode's own
  `reasoning.part.time.{start,end}` for thinking durations now.
- Pullfrog L642 (onToolUse double-fire on subagent dispatch): re-checked
  the bus-envelope flow; the plugin filters orchestrator events except
  for status=running task dispatches, and bus-envelope returns before
  calling handlers.tool_use on those. No double-fire under current code.

Validated: 610/610 unit tests, lint + format + typecheck clean,
skill-invoke-opencode ✓.

* DX: flip pnpm play / pnpm runtest to docker-by-default

Restores the script shape wiki/docker.md has documented since the docker
rewrite (#750). PR #756 inadvertently reverted action/package.json's
gha/play/runtest scripts to host-only and dropped the :local variants;
the wiki kept the new shape, so docs and reality drifted. The OpenCode-v2
migration agent ran `pnpm play --raw …` host-side throughout because the
host entry was the only thing that existed.

scripts (root → action):
- pnpm play       → pnpm -C action gha play.ts          (docker, default)
- pnpm play:local → pnpm -C action play:local           (host)
- pnpm runtest    → pnpm -C action gha test/run.ts      (docker, default)
- pnpm runtest:local → pnpm -C action runtest:local     (host)
- pnpm gha is restored in action/package.json (re-adds `node gha.ts`)

action/package.json deliberately ships only the :local variants — bare
`pnpm -C action play` now errors instead of silently bypassing docker.
This is a tradeoff per the user prompt's "consider whether NAMES should
change" hint: the explicit error is worth the small CI churn.

CI workflows: `.github/workflows/test.yml` and
`action/.github/workflows/test.yml` flipped from `pnpm runtest …` to
`pnpm runtest:local …`. Semantics unchanged — they still execute
`node test/run.ts` directly on the GHA Linux runner; nesting docker on
GHA is unnecessary overhead. Only the script name changed to match the
new package.json.

Webhook tester: the existing root `pnpm play` was actually a webhook
handler smoke harness (root play.ts), unrelated to the action runtime.
Renamed root play.ts → webhook.ts and exposed it as `pnpm webhook` so
`pnpm play` can carry the docker-by-default action shortcut without
collision. README updated.

File headers updated:
- action/play.ts: invocation block now points at `pnpm play` /
  `pnpm play:local`
- action/test/run.ts: same
- action/gha.ts: usage block calls out the new shortcut wrappers

AGENTS.md: extended the existing "local sanity checks of action tool
logic" rule with the play / play:local / runtest / runtest:local
selection guidance and the `cd action; pnpm play` footgun note.

wiki/docker.md unchanged — already described the now-real shape.

* test/crossagent: add codex-auth smoke

Pins openai/gpt-5.5 (in opencode's Codex ALLOWED_MODELS) and runs the
full opencode harness against the env-provided CODEX_AUTH_JSON. Verifies:

  - installCodexAuth() materializes auth.json under the test HOME
  - opencode routes openai requests through ChatGPT subscription auth
    (no OPENAI_API_KEY in env, AT path forced via expires: 0)
  - the refresh chain advances during the run (refresh_token rotates)
  - detectCodexRefresh() would surface the rotation to entryPost.ts

The post-hook write-back fetch isn't reachable from `pnpm runtest`
(it's a separate GHA `post:` step). The integration boundary that
matters end-to-end is "did the on-disk auth.json change in a way
detectCodexRefresh recognizes" — that's exactly what this test asserts.

CI wiring (already committed in a1c1fd4f as part of the DX flip):
  - .github/workflows/test.yml: CODEX_AUTH_JSON via secrets in
    action-agents env block
  - action/.github/workflows/test.yml: same; codex-auth in the
    hardcoded test matrix with a claude exclude

The provisioning step on the user's side is `gh secret set
CODEX_AUTH_JSON --repo pullfrog/app < auth.json`.

ci.test.ts: expectedAgentEnvVars now includes provider
`managedCredentials` so the "env vars cover all provider API keys"
invariant stays self-correcting as more managed credentials land.

* docs(codex-auth): make storage requirement unmissable

A previous reviewing agent on this branch came away thinking
`CODEX_AUTH_JSON` could live in GitHub Actions secrets. It can't —
`entryPost.ts` rewrites the rotated refresh token after every run, and GH
Actions secrets are immutable at runtime, so any non-Pullfrog-Postgres
storage breaks the refresh chain on the first rotation (~1h silent
expiry).

- wiki/codex-auth.md: prominent `[!IMPORTANT]` callout above the fold,
  with the words "GitHub Actions secrets DO NOT WORK" verbatim and an
  enumeration of broken alternatives.
- action/utils/codexHome.ts + action/entryPost.ts: header comments now
  loudly contrast Pullfrog secret store vs GH Actions and explain the
  writeback constraint.
- AGENTS.md: terse one-bullet rule next to the model-resolution rule so
  future agents don't repeat the mistake.
- .github/workflows/test.yml + action/.github/workflows/test.yml: added a
  comment marking the existing `secrets.CODEX_AUTH_JSON` injection as a
  CI smoke-testing shortcut, not the canonical pattern. CI wiring itself
  unchanged per scope.

* auth codex: auto-open device URL, drop --scope flag

- detect `https://auth.openai.com/codex/device...` from codex CLI output
  and best-effort launch it in the user's default browser (open / xdg-open
  / cmd start, wslview fallback on linux). gated so we only open once per
  flow; failures are swallowed so manual copy-paste still works.
- drop the `--scope` flag entirely. the device-code flow is fundamentally
  interactive (browser approval), so a "skip-the-prompt" flag for just one
  of the prompts was dead weight. collapses scope selection to "always
  prompt on org-owned, always account on user-owned".

* rename gha→docker, flip play/runtest defaults to host

the previous shape conflated "real GitHub Actions" with the local docker
container that mocks it, and made the slow docker path the default for
fast-iteration scripts.

- `action/gha.ts` → `action/docker.ts` (banner, --doctor, --help, image
  tag `pullfrog-docker:*`, volume `pullfrog-docker-node-modules-*`,
  tmpdir, error messages)
- `pnpm play` / `pnpm runtest` now default to host (fast iteration);
  `pnpm play:docker` / `pnpm runtest:docker` run inside the container
- `pnpm gha` → `pnpm docker` (the container runner shortcut)
- `pnpm webhook` → `pnpm play:webhook` (fits the play: namespace; the
  bare name implied a webhook server, which hookdeck-cli already is)
- update docs (`wiki/{docker,action-tests,billing,adversarial,browser}.md`,
  `README.md`, `AGENTS.md`), CI workflows
  (`.github/workflows/test.yml`, `action/.github/workflows/test.yml`),
  and code headers (`action/{play,test/run,utils/runFixture}.ts`,
  `webhook.ts`, `action/test/coverage.ts`)

`action/commands/gha.ts` keeps its name — it's the real GitHub Actions
entry point for the `pullfrog gha` CLI command (not the docker mock).

* fix(codex): route post-hook writeback through apiFetch + conditional skip

Three threads addressing PR #767 followups.

action/entryPost.ts: replace raw fetch() with apiFetch() so the
PUT /api/runtime/secret call carries the x-vercel-protection-bypass
header/query when targeting a preview deployment. raw fetch silently
401s against the Vercel SSO gate, so every preview-env Codex run was
losing its rotated refresh token. production is unaffected (no SSO).

action/test/crossagent/codexAuth.ts: gate the test on CODEX_AUTH_JSON
via new TestRunnerOptions.skipIf hook. when the secret is absent
(forks, contributors without it), runTestForAgent short-circuits to a
passing-with-skipped ValidationResult before any agent spawn — so the
matrix's fail-fast: true setting doesn't cascade-cancel siblings. CI
on pullfrog/app and dev-local with .env both still run the test for
real. printSingleValidation/printResults now render skipped entries
distinctly.

doc/comment drift:
- docs/codex-auth.mdx, wiki/codex-auth.md: drop stale --scope flag
  mention (removed in 10be96db, scope is now always interactively
  prompted or implicit).
- wiki/codex-auth.md: tighten Claude-defense wording — materialization
  is agent-gated (opencode/opencode_v2 harness), not model-gated;
  opencode runs with non-OpenAI models still materialize the file,
  it's just not read.
- action/Dockerfile, action/docker-entrypoint.sh: pnpm gha / gha.ts
  → pnpm docker / docker.ts (renamed in a2a63929).
- app/api/runtime/secret/route.ts: refer to the save-time scope prompt
  instead of the dropped --scope flag.

* smoke: force ≥2 tool calls; document test-bar in wiki + AGENTS

upgrade crossagent/smoke prompt to call pullfrog_git status before
set_output. this exercises the 2nd model→agent round-trip across every
providers-live flagship, catching bugs like the Gemini thought_signature
echo that single-tool-call tests can't see.

also adds the "bar for adding new LLM-driven tests" section to
wiki/action-tests.md and an extension to the existing AGENTS.md
no-tests rule pointing at it — prefer upgrading existing matrix entries
over adding new ones.

local: pnpm runtest smoke opencode passes against both
anthropic/claude-sonnet-4-6 and google/gemini-pro.

---------

Co-authored-by: Colin McDonnell <colinmcd94@M1chelle.local>
2026-05-20 04:05:16 +00:00
Colin McDonnell 76879b27ec docker testing rewrite: bake the image, drop the allowlist, kill the quoting (#750)
* docker testing rewrite: bake the image, drop the allowlist, kill the quoting

- new `pnpm gha <script>` wrapper. one entry point for running any node
  script in the GHA-like container; replaces the runtime apt-get +
  useradd + chown ceremony in `action/utils/docker.ts`.
- `action/Dockerfile` bakes ubuntu:24.04 + node 24 + gh + jq + sudo +
  testuser at uid 1000. `action/docker-entrypoint.sh` remaps to the host
  uid/gid and `exec`s the requested command — no `bash -c` nesting, no
  `escapeForDoubleQuotes`.
- env passthrough: full `process.env` (+ `.env` via dotenv) flows through
  `--env-file`, multi-line values via `-e` fallback. drops
  `EnvFilterMode` / `testEnvAllowList`.
- image rebuild is content-hash gated on Dockerfile + entrypoint; volume
  is versioned by hash so a stale `node_modules` cache from an old image
  can't poison a new one.
- `action/play.ts` slimmed to a CLI; `run()` extracted to
  `action/utils/runFixture.ts`. drops the `--local` / `PLAY_LOCAL` dual
  mode in favor of explicit `play:local` / `runtest:local` scripts.
- `action/test/run.ts` no longer self-relaunches into docker — that's
  `gha`'s job now.
- `action/test/coverage.ts` `ALWAYS_RUN_ALL` updated to track the new
  files.
- `wiki/docker.md` rewritten (243 → 105 lines). `wiki/action-tests.md`,
  `wiki/billing.md`, `wiki/adversarial.md`, `README.md`, `AGENTS.md` all
  updated to drop `--local` / `PLAY_LOCAL` references.

verified end-to-end: `pnpm play` runs the default fixture against
pullfrog/scratch, exit 0; `sudo unshare --pid` still works inside the
container; `pnpm runtest` boots through the wrapper.

* gha: address review feedback + 3 related issues found locally

review-flagged:
- bare `pnpm gha --build` now builds the image and exits 0 (was
  printing help and exiting 1 — docs claimed it was a valid standalone)
- `initVolumeOwnership` skipped when the named volume already exists;
  saves the ~240ms `docker run … chown` on every warm invocation
- `GIT_SSH_COMMAND` gate widened to any `id_*` private key (was hard-
  coded to `id_rsa`, leaving ed25519-only linux contributors with the
  default ssh config). dropped `-i` so ssh picks whichever key exists
- new `action/.dockerignore` — partial mitigation noted: BuildKit
  (default since docker 23) only sends files referenced by the
  Dockerfile (~42B in practice), so the perf concern is mostly
  hypothetical. file is still worth keeping for `DOCKER_BUILDKIT=0`
  fallback and as documented intent for future `COPY . .` additions

related issues found while validating locally:
- `parseArgs` now stops flag-parsing at the first positional (or
  literal `--`); `pnpm gha test/run.ts --build` previously
  intercepted `--build` as a gha flag instead of forwarding to
  `test/run.ts`
- new `pnpm gha --clean` command prunes orphan `pullfrog-gha:*`
  images and `pullfrog-gha-node-modules-*` volumes whose hash
  doesn't match the current Dockerfile (each Dockerfile/entrypoint
  edit creates a fresh hash and orphans the prior pair, ~600MB +
  ~200MB each — without a cleaner they accumulate silently)
- `--shell` without a TTY now fails fast with an actionable message
  before docker is invoked, instead of producing the confusing
  `the input device is not a TTY` from docker run

wiki updated: documents `--clean`, the parseArgs passthrough rule,
and a new "Reclaiming disk" section.

* gha: fidelity, flexibility, and signal-safety improvements

investigated local fidelity vs the real GHA ubuntu-24.04 runner and
addressed the gaps that have actually bitten contributors or could.

fidelity (image now matches GHA closer):
- bake build-essential, wget, xz-utils, file alongside the existing
  toolset. gh, jq, git, python3, sudo, ssh, build-essential, wget,
  xz, file, unzip, curl all present. native module builds (node-gyp,
  any package missing arm64 prebuilts) now work; common agent shell
  calls don't hit ENOENT
- `host.docker.internal:host-gateway` flag wires the host into the
  container's DNS on linux (macOS Docker Desktop bakes it in). lets
  scripts that hit a local dev server use `API_URL=http://host.docker.
  internal:3100` and work identically on both platforms
- `--init` makes tini PID 1, fixing signal forwarding during the
  pre-exec warmup window (Ctrl-C was previously taking up to 10s to
  tear down because bash-as-PID-1 swallowed the signal)
- pnpm version is correctly pinned via the workspace's
  `packageManager` field — corepack resolves it at install time;
  verified via the new `--doctor` command

flexibility (new affordances):
- `pnpm gha --doctor` runs an inside-the-container fidelity audit:
  os + arch + node/pnpm/python versions, version snapshots of every
  baked tool, env vars (CI, HOME, TMPDIR), uid/gid, and the
  host.docker.internal resolution. useful for "works in CI fails
  locally" or vice versa
- `pnpm gha --build --no-cache` busts the docker layer cache when
  an apt mirror, base image, or external download has changed
  upstream
- entrypoint's `pnpm install` warmup is now wrapped in a `flock` on
  a file in the shared node_modules volume — concurrent `pnpm gha`
  invocations (e.g. play in one terminal, runtest in another)
  serialize their install instead of racing

docs:
- new "Gaps (known)" section in wiki/docker.md explicitly calling
  out the things this system can't do yet, including the missing
  `uses: ./action` semantics gap that
  `.github/workflows/action-gha-e2e-adhoc.yml` currently fills via
  GHA only (designing a local `pnpm gha-action <fixture>` is on the
  roadmap), service containers, parallel-run sharing, and arch
  differences (arm64 vs amd64)

* docs: audit + corrections after testing fronts

self-audit pass for stale references and incomplete pointers:

- wiki/browser.md: `Docker (node:24)` → `pnpm gha container (ubuntu:24.04)`.
  the substance was right (chrome not preinstalled) but the base image
  reference was stale.
- wiki/docker.md: the "Permission errors" troubleshooting line claimed
  the node_modules volume is chowned on every run; now correctly says
  "owned by the host uid on first creation; warm runs skip the chown"
  to match the actual behavior after the initVolumeOwnership fix.
- wiki/action-tests.md: `API_URL` env-var doc now mentions BOTH paths
  (`localhost:` from play:local, `host.docker.internal:` from inside
  the container). Proxy/router recipe now shows both invocations
  side-by-side instead of saying "must use play:local".
- wiki/billing.md: same dual-recipe update for the loop-including-the-
  action proxy walkthrough.
- gha.ts header: expanded the usage block to include --clean / --doctor /
  --no-cache / --shell-TTY, added the host.docker.internal note, and
  pointed at wiki/docker.md for design rationale.

self-document check: a future agent landing on this code can answer
"how do I run a fixture / debug in shell / add a tool / diagnose
fidelity / reach a local dev server" purely from gha.ts header +
wiki/docker.md without spelunking through the entrypoint or git
history.
2026-05-16 03:12:25 +00:00
David Blass b835d53d83 add /anneal + pullfrog-reviewer named subagent + Build self-review polish (#550)
* cherry-pick updated /anneal command from billing branch + add as Claude Code slash command

mirrors origin/billing:.cursor/commands/anneal.md (commit 4f389a8f) into
both .cursor/commands/ and .claude/commands/ so the parallel-lens annealing
prompt is available in both editors. content is identical between the two
files.

* anneal: drop REVIEW.md pointer, surface-agnostic dispatch wording, fix modes.ts self-review contradictions

Anneal pass over the /anneal slash command and the Build-mode self-review step:

- Drop REVIEW.md references in both anneal.md copies. The file does not
  exist on the Claude Code surface (only .cursor/commands/), and its
  contents (correctness/security/impact framing) directly contradict the
  prescribed single-lens, no-pre-shaping discipline.
- Replace "Task tool calls" with surface-agnostic "parallel subagent
  calls" so the meta-prompt does not couple to either CLI's tool naming.
- Hedge the "verify via web search" instruction to acknowledge subagents
  may not have web search available.
- modes.ts: drop "and the changed files" — the same step's don't-list
  forbids handing subagents a curated reading list (in-file contradiction).
- modes.ts: restore the "skim only, don't pre-review" warning that the
  long-form treats as load-bearing.
- modes.ts: drop "NO MCP tools" — overbroad; the actual safety property
  is captured by "no writes, no shell commands, no side effects".

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal: two-round self-anneal of /anneal + modes.ts self-review

Expand the multi-lens parallel-review protocol with fixes surfaced by
running /anneal on this branch twice. Material additions:

/anneal canonical (.claude/commands/anneal.md + .cursor mirror):
- promote orientation-vs-defect-hunting distinction to a load-bearing
  framing in the opening paragraphs
- add an empty-target early exit ("nothing to anneal" stop) at §1
- spell out the read-only constraint with the no-op-if-reverted test,
  and forbid recursive subagent dispatch (incl. agentic MCP tools)
- add cleanup-and-debt sub-categories (env vars, feature flags, dangling
  symbols), supply-chain, test-integrity lenses to the catalog
- §1 lens-count rule: explicit trivial/typical/high-risk tiers; "treat
  as typical" tiebreaker for the unsure case
- §2 example uses bare `git diff <primary-branch>` to capture
  uncommitted edits (three-dot syntax is committed-only)
- §5 targeted-follow-up cross-references the fresh-eyes carve-out in
  Delegation discipline
- final-message format spells out coverage shape, findings-table
  shape, dry-run fix-plan branch, and plan/doc summary branch
- stopping criteria distinguish "trivial" from "small / low-risk"

action/modes.ts Build mode step 4 (self-review one-pass anneal):
- empty-diff early exit; "step 4 mandatory whenever there is a diff"
  resolves the prior contradiction with the always-runs assertion
- lens count by risk (2-3 typical / 4 high-risk single-round-cap /
  exactly 1 trivial) with separate Tiebreaker
- expand swap-in lens menu (research-validated assumptions, security,
  user-journey, ops, integration, test integrity, supply chain,
  performance, holistic) so the catalog is a starting menu, not a
  closed set
- rename `cleanup & scope` to `diff hygiene` to avoid colliding with
  the canonical's broader `cleanup & debt`
- delegation discipline bulletized (don't lens-review yourself,
  don't summarize, don't curate, don't pre-shape, don't mention other
  lenses); independence rationale stated inline
- explicit research-discipline reminder for any lens that touches
  external contracts (web search, quote URLs)
- comment block enumerates deliberate omissions vs the canonical
  (dry-run, severity categorization, read-only shell) and the
  deliberate scope decision (sibling diff-producing modes stay solo)

action/modes.ts Review + IncrementalReview subagent-dispatch wording:
- propagate the no-recursive-dispatch rule (was missing)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* add set_plan/get_plan + restructure Review/IncrementalReview as parallel-subagent orchestrators

Build mode's self-review and Review/IncrementalReview now follow the multi-lens
parallel-subagent fan-out pattern from the canonical /anneal protocol. New
set_plan/get_plan MCP tools (orchestrator-only) persist the implementation plan
in tool state so the self-review's plan-adherence lens can verify the diff
against the original intent rather than reconstructing it post-hoc.

Subagent "read-only / no further dispatch" is currently enforced via prompt
prose only — neither claude-code's --disallowedTools nor opencode's per-agent
tools allowlist is configured to scope subagent MCP access. Documented as a
deferred ~30-50 LOC follow-up in the modes.ts header comment.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* revert Review/IncrementalReview mode prompts to main; keep Build self-review changes

E2e testing on this branch only exercised the trivial-1-lens path for Review (preview
repo had only docs PRs). Multi-lens Review fan-out was never directly validated against
a real code PR. Splitting the Review/IncrementalReview restructure to its own branch
(review-mode-orchestrator, draft PR #555) pending focused validation.

Keep on this branch:
- set_plan/get_plan MCP tools
- Build mode multi-lens self-review (Test 3 directly validated 2-subagent parallel
  fan-out on a 2-file diff)
- /anneal command updates (.claude/ and .cursor/ mirrors)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* require plan parameter when selecting Build mode

Adds an arktype .narrow on SelectModeParams that rejects select_mode({mode:"Build"})
unless a non-empty 'plan' string is also provided. When valid, the plan is stored
into ctx.toolState.plan at mode-selection time, so step 4's plan-adherence lens
always has a comparison target.

This closes the e2e finding that agents never reached for set_plan on their own
(5 of 6 runs in production). Build mode prompt updated to reflect that plan is
already populated at mode selection; set_plan remains as the mid-task replan
tool. Other modes are unaffected.

Validation surfaces the error to the agent with a descriptive message including
the path ('plan') and recovery instructions, so a failing call is recoverable
on the next turn rather than a hard fail.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* move Build-mode plan-required check from arktype .narrow to execute()

arktype .narrow predicates aren't JSON-Schema serializable — FastMCP's
toJsonSchema() emitted a {code: "predicate", predicate: Function} object
instead of a serialized schema. Effect: agents couldn't see select_mode
in their tool list (verified by 5 consecutive runs across two models
silently bypassing select_mode entirely after the prior commit).

Fix: keep the param schema clean (.narrow removed) and check
selectedMode.name === "Build" && !params.plan in the execute() body,
returning a structured error response. The agent now sees select_mode
normally, gets a clear actionable error if it forgets the plan, and can
recover on the next turn by retrying with the plan included.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* flip lens architecture: Build = single fresh-eyes subagent, Review/IncrementalReview = multi-lens

Build mode self-review previously fanned out 1-4 lenses on the agent's own diff. The
bias-mitigation argument for fan-out is weaker for self-review than for reviewing
someone else's PR — the orchestrator just wrote the code, so what matters is one
fresh-eyes subagent that doesn't share the implementation context, not breadth across
parallel angles. Build now dispatches exactly one subagent that gets the original
user request and the diff and evaluates whether the diff fulfills the request.

Review and IncrementalReview now use the multi-lens orchestrator pattern (triage →
parallel read-only fan-out → aggregate → draft comments → submit). For someone else's
PR, parallel lenses (correctness, security, research-validated, user-journey, etc.)
provide breadth that a single subagent can't carry coherently. Was previously parked
on the review-mode-orchestrator branch (PR #555).

Removes set_plan/get_plan MCP tools, ToolState.plan field, and the plan parameter on
select_mode. Validated end-to-end that those didn't cause agents to actually use plan
tracking (5 of 6 e2e runs skipped them); the original user request from the prompt
body is the source of truth and the orchestrator already has it.

Drops timeout test plan-param workaround that was added for the prior validation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* split Review/IncrementalReview multi-lens back out to review-mode-orchestrator branch

The multi-lens orchestrator restructure for Review/IncrementalReview was bundled
into this branch in commit e964ae0c, but it hasn't been validated against a
real code-heavy PR (the e2e exercised it only on docs PRs). Splitting it back
out keeps this branch focused on the validated half — Build → single fresh-eyes
subagent — and lets the Review changes ship in a focused PR (#555 reopened).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal: fix Build prompt contract bugs found by 3-lens review

Major fixes:
- checkout_pr returns the field as `base`, not `baseRef` (per checkout.ts:611-616).
  The prompt was telling agents to read `result.baseRef` which would be undefined.
- The base-ref fallback "after fetching" is unreachable via the `git` MCP tool
  (it blocks `fetch` per AUTH_REQUIRED_REDIRECT). Now names `git_fetch` explicitly.
- Boundary-tag wrapping for the user request had no escape rule for input that
  contains the literal close marker, and no fallback for an empty request. Both
  are now documented with a nonce-suffix mitigation.
- PR reference updated #555#557 (the active PR for the multi-lens
  review-mode-orchestrator branch; #555 was closed after the rebase).

Minor fixes:
- Retry predicate tightened: "errors out (tool error) or returns an empty body",
  not "returns nothing usable" (which is unfalsifiable and lets an orchestrator
  declare any output not-usable to skip review).
- Subagent read-only constraints rephrased as prescriptive ("MUST NOT call")
  rather than descriptive ("you have only"), since on inheriting runtimes the
  subagent does in fact have access to write tools and the constraint is
  prompt-only.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal round 2: tighten Build prompt edge cases (workflow_dispatch, base-ref, footer-strip, skip marker)

Cross-lens findings from holistic + user-journey + research-validated lenses:

- workflow_dispatch + empty diff: report_progress silently no-ops when there's
  no parent issue/PR. Now also call set_output with a "no-op" summary so the
  user gets surfacable feedback.
- base-ref resolution: clarified `base` from checkout_pr is a bare ref name,
  added explicit `git remote show origin` path for repos whose primary is not
  `main` (master, trunk, etc.).
- bare `git diff` description: tightened from "shows working tree" to
  "shows unstaged working-tree changes" — bare diff misses staged changes too,
  not just committed ones.
- prompt-body stripping: explicitly call out the leading `> ` blockquote
  prefix (added by the *YOUR TASK* section formatting) and the entire Pullfrog
  footer block, not just one example link.
- boundary-tag nonce: always-on now, not conditional on detecting a close
  marker. Cost is one random short string; failure mode (prompt injection if
  input contains literal close marker) is silent.
- subagent-skip marker: structured `Self-review: SKIPPED (subagent error: ...)`
  on its own commit-message line, so the gap is greppable.

Header comment also documents:
- AddressReviews/Fix/Task asymmetry (deliberately deferred)
- Subagent-runtime-fence deferred fix must explicitly deny Skill / agentic
  MCP tools, not just destructive tools (claude-code blocks recursive Task
  spawn but not alternative dispatch paths).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal round 3: targeted re-review of round-2 changes catches real regressions

Round 2's "fixes" introduced two real bugs that round 3's targeted correctness
re-review caught:

CRITICAL (fixed): tier-3 base-ref resolution used `git remote show origin`,
which requires network auth — the MCP `git` tool runs commands through plain
spawn() without auth, so this hangs on private repos. Replaced with
`git symbolic-ref refs/remotes/origin/HEAD` (local symref, no network),
which actions/checkout populates.

MAJOR (fixed): the eventInstructions fallback was incoherent — the agent has
no separately-addressable eventInstructions field; whatever it received in
*YOUR TASK* is its only input. Removed the misleading reference.

MAJOR (fixed): per-line `> ` strip was ambiguous, could destructively flatten
user-pasted markdown blockquotes. Now: "strip exactly one leading `> ` per line".

MAJOR (fixed): tier-1 base-ref preferred bare `<base>` over `origin/<base>`,
which fails on the rare alreadyOnBranch path in checkout_pr where the local
ref isn't re-created. Now prefers `origin/<base>` (always populated post-fetch).

MINOR (fixed): footer-strip anchor was `<sup>`/`<picture>`, both of which
appear in legitimate user content (footnotes, etc.). Switched to the
PULLFROG_DIVIDER sentinel which is purpose-built for this.

MAJOR (acknowledged, partial fix): 4-hex nonce is theatrical security; bumped
to 8 hex and explicitly noted it's a typo-guard, not a security boundary,
and that the structural fix (separate task() argument) is the real solution.

REJECTED (verified false positive): subagent claimed `set_output` is not
registered for workflow_dispatch. Verified at action/utils/payload.ts:118 —
workflow_dispatch from `gh workflow run` resolves to trigger:"unknown",
which IS standalone, which IS registered with set_output. E2e logs from
prior tests confirm agents successfully call pullfrog_set_output on
workflow_dispatch runs.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal round 4: drop broken symbolic-ref tier, simplify base-ref resolution

Round 3's tier-2 (`git symbolic-ref refs/remotes/origin/HEAD`) is
empirically broken: actions/checkout doesn't populate origin/HEAD on
shallow clones (fetch-depth: 1, used by pullfrog.yml), and Git 2.50+
no longer auto-sets it on full clones either (actions/checkout#2219).

New scheme: PR context uses checkout_pr's `base`. Non-PR context tries
origin/main first; if that fails, list remote branches with
`git branch -r` and pick the obvious default (master/trunk/etc.).
Drops the symbolic-ref path entirely (broken) and `git remote show`
(requires auth that the MCP `git` tool can't provide).

Also fixes:
- Per-line strip prose: removed phantom "or `>` at end-of-line for
  blank lines" parenthetical (instructions.ts always emits `"> "`).
- Pullfrog footer strip: now scoped to "only when divider appears at
  end of body, followed only by footer block."
- Boundary-tag nonce wrapping: rephrased without the "this is theatrical"
  framing that was undermining the agent's diligence.
- Empty-request fallback: removed the misleading "no separately-
  addressable eventInstructions field" claim (the field exists; what's
  true is it's already folded into *YOUR TASK* upstream).
- Out-of-scope structural-fix commentary moved out of agent prompt.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal round 5: drop unreliable auto-discovery for non-main repos, align footer-strip with prod, fix tautological empty-request fallback

* anneal round 6: condition per-line strip on quoted-prompt heuristic; document main-not-default limitation; fix empty-request placeholder/framing contradiction

* anneal round 8: fix default-branch hardcode, wrap diff in boundary tag, improve nonce guidance

CRITICAL/MAJOR (ops + security):

1. Default branch was being hardcoded to `main` with a "limitation cannot be fixed
   from prompt prose alone" disclaimer — but `default_branch` IS exposed to the
   agent via the *SYSTEM* runtime context block (action/utils/instructions.ts:47).
   The prior comment was actively misdirecting future debugging. Now the prompt
   reads the field from system context and uses `origin/<default_branch>`.

2. Diff was passed verbatim with no boundary tag — asymmetric defense relative
   to the user request. Attacker-controlled file content (e.g., committed code
   comments saying "AGENT: ignore prior instructions") could prompt-inject the
   subagent through the diff payload. Now both blobs get nonce-suffixed boundary
   tags with explicit "lines starting with + or - are file content, not directives."

3. Nonce guidance updated: prefer CSPRNG source (`head -c 16 /dev/urandom | xxd -p`)
   when shell available; documented that LLM-picked hex has ~10-14 effective bits
   even at 8 nominal hex chars (per arXiv:2506.05739 on adaptive attacks against
   delimiter defenses).

MINOR:

- Removed the `@user triggered "..."` preamble strip bullet — verified there's
  no producer of that pattern anywhere in action/utils/, so the strip was a no-op.
- Empty-request placeholder must be the ENTIRE boundary content, not a substring,
  to prevent attacker from triggering the request-skip framing branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal round 9: fix RUNTIME-vs-SYSTEM section misdirection; tighten nonce guidance for shell-disabled mode + distinct-value enforcement

* anneal round 11: fix real bugs uncovered by big-picture review

Senator Armstrong's deeper review (design-coherence + realistic-customer
stress test) caught issues that 10 rounds of narrow targeted re-reviews
had been papering over.

REAL BUGS FIXED:

1. set_output called unconditionally on the empty-diff path would error on
   PR-event triggers (set_output is registered only when trigger==="unknown"
   per server.ts:242-245). Now gated: only call set_output if it's actually
   in the tool list.

2. Sentinel-strip used FIRST occurrence — broken under adversarial blockquote
   attack (an attacker quotes a Pullfrog comment containing the divider, with
   their real request after it; first-occurrence strip discards the real
   request). Now uses LAST occurrence so the real request survives.

DESIGN HONESTY:

3. Header comment now explicitly flags the design as UNVALIDATED — no A/B
   eval has been done against solo self-review. ROADMAP_RESEARCH.md flags
   benchmarking as the prerequisite. Header documents the validation gap
   and what would justify reverting.

4. Header comment elevates the runtime-fence gap from a TODO to a SECURITY
   GAP that must ship before the prompt protocol can be considered
   production-hardened. Ordering: runtime fence FIRST, prompt protocol
   SECOND.

SIMPLIFICATIONS (per senior-engineer review):

5. Dropped the second nonce on the diff — the diff is the artifact under
   review; suspicious instruction-shaped lines in commits are exactly what
   the subagent should flag, not something to fence off.
6. Dropped CSPRNG-vs-LLM-fallback branching prose — just "16+ hex chars,
   use /dev/urandom if shell available, otherwise pick."
7. Dropped the regenerate-if-collide rule (vanishingly unlikely with 16
   hex chars, costs tokens to enforce).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* anneal round 12: revert round-11 regressions (sentinel-strip, set_output gate, diff nonce)

Round 12's sharper review caught three regressions round 11 introduced:

1. Sentinel-strip last-occurrence was strictly worse than first-occurrence
   for the common "user references a prior Pullfrog comment" case. The
   adversarial-quote scenario it was defending against is contrived (an
   attacker can put hostile payload anywhere; strip discipline doesn't
   change attack surface). Reverted to first-occurrence to align with
   canonical stripExistingFooter() and avoid silently swallowing user
   reference context.

2. set_output "gate" via "if it's in your tool list" relied on tool
   introspection that LLMs cannot reliably perform. Replaced with: just
   call report_progress; document the workflow_dispatch limitation as
   acceptable (job log is feedback-of-last-resort) rather than asking the
   agent to conditional-call a tool that may not exist.

3. Diff was de-nonced in round 11 on the assumption runtime fence ships
   first, but until that runtime fence lands the plain label is forgeable
   (committed file content can include "--- END DIFF ---" + injection).
   Restored nonce wrapping. The cost is one extra hex string; the benefit
   is real until runtime fence ships.

Also added explicit caveat on the self-attested skip marker: the proper
fix is MCP-layer dispatch-counting, not commit-message annotation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* ruthless cut: revert Build self-review elaboration to compact form

main already had subagent dispatch (4 compact lines). This branch added 70+ lines
of elaboration — header warnings, base-ref dance, footer-strip rules, nonce-
suffixed boundary tags, retry-once skip markers, delegation-discipline list — all
predicated on a runtime fence that doesn't exist and validation that never ran.
Senior-engineer review (round 11) explicitly recommended cutting; ROADMAP_RESEARCH
flags A/B benchmarking as the prerequisite for this design.

Net change vs main now matches what the user actually asked for:
  - drop the optional plan step (and its "follow the plan" / Notes references)
  - subagent receives the original user request alongside the diff, evaluated
    against base ref, with explicit no-further-dispatch constraint

Everything else reverts to main's prose. ~10 lines net change instead of 70+.

* anneal round 13: tighten self-review prompt inputs to runtime-resolvable values

Two underspecified inputs flagged by parallel holistic + mechanics review:

1. "the original user request" is empty for non-@pullfrog-tagged auto-triggers
   (sync, check_suite, opened, etc.); only YOUR TASK is reliably present in
   the assembled prompt across all event types. Replace.

2. "base ref (PR base or repo default branch)" requires the agent to resolve
   and fetch the default branch on non-PR runs (origin/<default> typically
   not fetched). Drop the elaboration — bare git diff captures all changes
   at step-3 time since step 2 doesn't commit. Aligns with 3ed2c55a's
   ruthless-cut philosophy: less elaboration, not more.

Verified in round 14: YOUR TASK is the literal section header in
instructions.ts (buildTaskSection); bare git diff scope is correct.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* restore plan step to Build mode prompt

The plan step was removed alongside the MCP-contract plan-required work,
but the user only wanted it gone from the MCP contract, not from the
prompt itself. Restores step 1 (plan), the "follow the plan" build
sub-bullet, the trailing Notes section, and renumbers learningsStep
back to 6.

Made-with: Cursor

* add pullfrog-reviewer named subagent; standardize review fence to non-mutative+non-recursive

Defines a constrained `pullfrog-reviewer` named subagent for the Build
mode self-review and /anneal lens dispatch, with a single source of
truth in action/agents/reviewer.ts (allowed tools, denied mutating MCP
tools, system prompt).

Enforcement:
- opencode: real fence via agent.pullfrog-reviewer block in
  buildSecurityConfig — denies edit/bash/task and globs each mutating
  pullfrog_* MCP tool to false.
- claude-code: forward-looking only. Per-agent disallowedTools is
  upstream-broken (anthropics/claude-agent-sdk-typescript#172, open as
  of latest update Mar 2026 — subagent child processes still see and
  can call disallowed tools, including Task). The --agents JSON is
  defined anyway so the fence becomes real when upstream fixes #172;
  until then the prompt prose constraint is the actual fence. The
  PreToolUse hook workaround that does enforce is out of scope.

Read-only MCP tools (get_*, list_*) intentionally remain enabled so
the reviewer can pull PR/issue/check context without dispatching
state changes.

Both modes.ts Build self-review and the two anneal.md files now share
the same "non-mutative + non-recursive" framing — file reads, grep,
search, web search/fetch, read-only shell, and read-only MCP queries
allowed; writes, state-changing MCP, and nested subagent dispatch
denied. Resolves the previous inconsistency where /anneal allowed
read-only shell and Build self-review banned all shell.

Made-with: Cursor

* Build self-review: pass build-phase failure summary to reviewer subagent

Adds an instruction in step 4's dispatch: along with YOUR TASK and
git diff, pass a tight plain-text summary of any lint/typecheck/test
failures fixed during build (what broke, root cause, the fix) — or
"no build-phase failures" if clean. Goal: let the reviewer check
that fixes addressed root causes rather than suppressed symptoms
(e.g., editing a test to make it pass instead of fixing the bug).

Implemented as agent self-summarization rather than piping raw build
output to avoid context flooding — typecheck/test output can be
hundreds to thousands of lines per failure. The agent has the
failure trail in its own conversation history and summarizes from
memory; the reviewer sees a few lines per failure, not raw stderr.

Caveat: this is a plausible-but-unvalidated quality improvement.
The mechanical justification (signal already produced, currently
not passed on) is real; "this catches more bugs" is a hypothesis
that will need actual run data to confirm. Downside is bounded
(reviewer gets slightly more context, no behavior change if the
summary is empty or ignored).

Made-with: Cursor

* Build self-review: distill /anneal delegation + research discipline into dispatch instructions

Lifts the codified learnings from /anneal's "Delegation discipline" and
"Research discipline" sections into Build mode step 4. These rules are
about how-to-prompt the reviewer (not about parallelism), so they
transfer losslessly to single-agent dispatch and address bias modes the
prior prompt was silent on:

- Don't summarize what you implemented (biases toward shape-validation)
- Don't curate a reading list (your curation is itself a lens)
- Don't pre-shape output with severity/category (leaks hypotheses)
- Don't defect-hunt in parallel (reintroduces the implementation bias
  the subagent is meant to mitigate)
- For diffs touching third-party API contracts / SDK semantics /
  framework directives / DB engine specifics, instruct the reviewer to
  verify load-bearing claims via web search and quote URLs rather than
  trust training data

Restructures step 4 from one paragraph into three (constraints, inputs,
discipline) plus a final review-and-commit paragraph for readability.

These are validated learnings from many anneal rounds, not theoretical
best practices — they're the single substantive piece this branch was
missing.

Made-with: Cursor

* pullfrog-reviewer: drop MCP deny-list, rely on prose constraint

Per-PR-review feedback: hand-maintaining MUTATING_MCP_TOOLS against
action/mcp/server.ts was fragile — a future mutating tool added to the
MCP server without updating this list would silently grant write access
to the reviewer. Inverting to an allowlist or adding a structural test
both keep the drift problem.

Drop the list and all per-agent runtime denies (claude disallowedTools,
opencode tools/permission map). Strengthen REVIEWER_SYSTEM_PROMPT to
spell out the categories of state-changing MCP tools by example and
explicitly tell the model to apply the no-op-if-reverted invariant to
tools added after the prompt was written — the rule is the invariant,
not the enumeration. Keep the named subagent so the prompt is reliably
injected. Update modes.ts and both anneal.md copies to drop the
runtime-enforces-where-supported claim.

Co-authored-by: Cursor <cursoragent@cursor.com>

* pullfrog-reviewer: fix description to allow read-only shell

The description field was overstating the constraint as 'must not shell',
but the system prompt explicitly allows read-only commands like git diff,
git log, cat, ls. Align description with the actual contract.

Co-authored-by: Cursor <cursoragent@cursor.com>

* restructure Review/IncrementalReview as multi-lens parallel-subagent orchestrators

For someone else's PR, parallel lenses (correctness, security, research-validated
claims, user-journey, etc.) provide breadth across angles that a single subagent
can't carry coherently. The orchestrator does triage → parallel read-only subagent
fan-out → aggregate → draft comments → submit. Lens count by risk: 1 lens for
trivial PRs, 2-3 for typical, 4 for high-risk surfaces (billing, auth, migrations).

This branch contains ONLY the Review/IncrementalReview multi-lens prompts.
Build mode keeps its single-fresh-eyes-subagent shape (different problem —
orchestrator just wrote the code; bias-mitigation comes from one subagent that
doesn't share the implementation context). The Build changes ship in a separate
PR (self-review-subagents → main).

Pending validation against a real code-heavy PR before merge — e2e on a docs-only
preview repo only exercised the trivial-1-lens path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* Review/IncrementalReview: dispatch fan-out via reviewfrog named subagent

The fan-out steps previously said "launch one read-only subagent per lens" without naming the
subagent. That bypassed the only enforcement layer the named subagent provides: a baked-in
system prompt that restates the non-mutative + non-recursive contract regardless of what the
orchestrator sends. Both modes now dispatch via REVIEWER_AGENT_NAME (matching Build mode's
self-review wiring) and restate the constraint inline so the rule is present twice.

* rename pullfrog-reviewer → reviewfrog

Mechanical rename of the named subagent. Constant names (REVIEWER_AGENT_NAME, REVIEWER_SYSTEM_PROMPT)
and file paths (action/agents/reviewer.ts) stay as-is — only the agent identifier string and prose
references in anneal.md and code comments change.

* modes/anneal: trivial PRs skip review entirely; lens count is judgment, not table; allow subsystem lenses

Three coupled changes to Review/IncrementalReview/Build self-review and the canonical /anneal
command:

1. Trivial-skip: trivial diffs (single-line, formatting/comment-only, doc typo, low-risk dep
   bump, no behavior change) skip the fan-out / self-review entirely. Build mode skips its
   self-review subagent; Review submits a bare "Reviewed — no issues found." without
   dispatching lenses; IncrementalReview takes the existing non-substantive submit path.
   Tiebreaker on uncertainty: treat as non-trivial.

2. Drop prescriptive lens counts. Replaces "2-3 typical / 4 high-risk cap / 1 trivial" with
   judgment-based guidance: pick as many lenses as the target has distinct surfaces of risk
   worth investigating independently; one is sometimes enough; bias toward more (and toward
   follow-up rounds in /anneal) for high-stakes subsystems; 5+ is a smell that lenses are
   overlapping rather than covering distinct ground.

3. Subsystem lenses. Adds an explicit second flavor of lens — domain-scoped frames like
   "the auth lens", "the billing lens", "the schema-migration lens" — alongside the existing
   themed lenses (correctness, security, user-journey, etc.). Stack themed + subsystem freely.

modes.ts and anneal.md (.cursor/ + .claude/, kept byte-identical) move together so the
canonical pattern doc and the orchestrator prompt agree on the protocol.

* add SessionLabeler so parallel subagent log lines are differentiable

When the orchestrator dispatches multiple `reviewfrog` subagents in a single
assistant turn (the parallel fan-out the multi-lens prompt now requires),
their tool_use / tool_result / text events arrive on opencode's NDJSON
stream tagged with distinct `sessionID`s but go through a single
`[Pullfrog]` log prefix. Result: log readers can't attribute which lens
issued which tool call, making CI logs unreadable for any review with 2+
lenses.

SessionLabeler:
- Binds the first-seen sessionID to "orchestrator" and subsequent new
  sessionIDs to FIFO-popped lens labels seeded from task tool_use inputs.
- Derives labels from `lens: <name>` markers in the dispatch prompt, the
  Task `description` field, the `subagent_type`, or `subagent#N` fallback.
- Keeps state local to a single runOpenCode invocation.

Wiring:
- opencode.ts: every event handler (init, message, text, tool_use,
  tool_result) now looks up the per-event label and prefixes log output
  via formatWithLabel(). Subagent finalOutput/token-reset paths gated on
  ORCHESTRATOR_LABEL so child sessions can't clobber parent state.
- claude.ts: claude rolls subagent activity into a single tool_result
  block (no per-event session_id), so it gets a minimal "» dispatching
  subagent: <label>" log line on Task tool_use as the only attribution.
- modes.ts (Review + IncrementalReview): orchestrator instructed to set
  the Task `description` to the lens name, since that's what the labeler
  reads when no explicit `lens:` marker is in the prompt.

Tests: 18 unit tests covering label derivation, FIFO binding, interleaved
sessions, fallback paths, and a realistic four-lens parallel fan-out
simulation. Full action test suite stays green (400 passing).

This is the pre-flight instrumentation that the multi-lens validation
runs depend on — without it, post-hoc log analysis can't tell two
subagents apart.

* log subagent dispatch + finish at info level for per-lens visibility

OpenCode's runtime currently encapsulates subagent execution inside the
`task` tool — subagent-internal tool_use/tool_result events do not surface
on the parent's NDJSON stream. The SessionLabeler I added in 0c4647f4
therefore can't actually differentiate concurrent subagent log lines
(there are no concurrent log lines on the parent stream to differentiate).

What CAN be observed on the parent stream is the dispatch and the result
of each `task` tool call. This patch surfaces both at info level:

  » dispatching subagent: lens:security (subagent_type=reviewfrog)
  ...
  » subagent finished: lens:security (15.3s, status=completed) — ...

Without this, a 4-lens parallel fan-out looks like 4 dispatches in close
succession followed by a long quiet gap and then an aggregation turn —
you can't see when each lens finished or how the durations overlapped.
With it, parallel execution is visible from the timestamps on the
"finished" lines.

The dispatched label comes from SessionLabeler.recordTaskDispatch (so
both lines share the same lens identity). taskDispatchInfo maps callID to
{label, startedAt} so the matching tool_result can compute duration and
emit the finished line.

Also added a defensive comment on the SessionLabeler instantiation
documenting that the per-event session-prefix path is currently dormant
in the opencode runtime, but kept in place so attribution flips on
automatically if/when opencode begins streaming subagent sessions.

* fix subagent-finished log: hybrid exact+FIFO callID matching

opencode does not consistently surface a tool_result callID matching the
originating tool_use callID for the `task` tool, so the previous
exact-match-only finish line never fired. Now we:

- Dual-index task dispatches by callID AND in a FIFO queue.
- Track non-task callIDs so we can identify "unrecognised callID" results
  as likely-task-with-mismatched-id.
- On tool_result, exact-match first; fall back to FIFO when the output
  looks like a subagent reply (>300 chars) and the callID is unknown.
- Flush leftover dispatches at run end with an "(inferred at run-end)"
  suffix so the gap is visible if subagent results arrive entirely off
  the tool_result event path (e.g. inlined into the next assistant
  message).

* fix subagent-finished log: move run-end flush to post-subprocess block

Investigation on T3 + finish-log-validation runs revealed two real issues
with my prior attempt:

1. The `result` event handler is dead — opencode never emits a
   `result`-typed event over its NDJSON stream, so the inferred-at-run-end
   flush I had placed there never fired. Move the flush to right after
   `runSubprocess` returns where it actually executes.

2. The FIFO heuristic was too strict — the >300-char output check
   excluded short or empty outputs that opencode's `task` tool_result
   appears to carry (the subagent's full reply seems to arrive via a
   separate channel, not the result event itself). Drop the size check;
   rely solely on `knownNonTaskCallIDs` to keep genuinely-non-task
   tool_results from popping a pending task.

Net effect: every `task` tool dispatch gets a matching `» subagent
finished` line in the logs, either from the FIFO fallback during the run
or from the run-end flush as a backstop.

* modes/anneal: anchor lens calibration in worked examples

The prior trivial-skip definition ("single-line fix, formatting-only,
…") was anchored on diff size, but real-world risk is anchored on diff
*shape*: a 5000-line lockfile regen IS trivial, and a 1-line SQL
operator flip in a billing path is NOT. The prior lens-count guidance
("there's no fixed count, bias toward more for high-stakes
subsystems") gave the agent no concrete shapes to anchor against, so
runs varied between under-pick (4 generic lenses on a billing PR) and
over-pick (5 overlapping themed lenses on a refactor).

This commit hardens both:

- Trivial definition gets explicit "looks trivial but isn't"
  anti-patterns: SQL operator flips, money/tax/timeout constants,
  feature-flag defaults, comparison operator changes, semantic 1-liners
  buried in whitespace, public-API renames, new direct deps. Skip lists
  get explicit "size doesn't matter" calibration for lockfile regens
  and mechanical renames.

- Lens count gets a worked-example ladder: 1 lens (refactor / new test
  file / isolated fix), 2-3 lenses (typical features), 4-5 lenses
  (high-stakes subsystem touches), 6+ is a smell.

- Subsystem lenses get an explicit recommendation to lead over generic
  themed equivalents for high-stakes domains, with the reasoning:
  domain framing primes the subagent for domain-specific failure modes
  (double-charges, refund races, dispute flows) the generic lens
  misses.

Mirrored byte-identical into both anneal.md copies; modes.ts updates
all three review surfaces (Build self-review, Review triage,
IncrementalReview triage).

* fix harness false-failure when Review submits without todowrite

Review and IncrementalReview prompts explicitly forbid calling
report_progress (the review IS the durable record). The post-run
harness in action/utils/run.ts errors with "agent completed without
reporting progress" when toolState.wasUpdated is false at exit. Until
now, the only path that set wasUpdated for these modes was the
todoTracker's debounced publish — which only fires if the agent
happens to call todowrite during the run. Adversarial run on PR #16
(misleading-trivial billing tweak) hit exactly this case: agent went
straight from triage → fan-out → review submission with no todowrite
calls, and the harness reported failure even though the substantive
review was successfully submitted with two inline comments.

Fix: create_pull_request_review now marks wasUpdated=true (and
finalSummaryWritten=true) on every terminal path — successful submit,
empty-content skip, and all-comments-dropped skip. Submitting a review
is unambiguously a "done" signal in these modes.

Found via adversarial testing of the multi-lens orchestrator on a
1-line tax constant change. Logged in /tmp/pullfrog-validation/v3/.

* fix harness false-failure when Review submits without todowrite (correctly)

Replaces the prior fix (acc2bd65) which set wasUpdated=true inside
create_pull_request_review. That approach worked for the harness check
but broke the orphan-comment cleanup: with wasUpdated=true and
finalSummaryWritten=true, the (!wasUpdated || trackerWasLastWriter)
condition in main.ts evaluated false and the "Leaping into action"
progress comment was left behind on every Review run — the exact
behavior the cleanup logic was designed to prevent (see
plans/review_progress_comment_cleanup_b0120f6c.plan.md).

Correct fix: change the harness check in action/utils/run.ts to
recognize a submitted PR review as an alternate completion signal
alongside wasUpdated. wasUpdated stays false on purpose so cleanup
deletes the orphan, but the run no longer false-fails when the agent
followed the Review-mode contract (submit a review, never call
report_progress).

The bug was discovered during adversarial testing of PR #16
(misleading-trivial billing tweak) where the agent went straight from
triage → fan-out → review submission without using todowrite, causing
the harness to error even though the substantive review (a CAUTION
blocking review with two inline comments catching a 10x tax cut) was
successfully posted.

* fix harness false-failure for Review modes (mode-based carve-out)

Replaces the prior carve-out (4c0f69aa) which gated on
toolState.review.id. That worked for runs where the review tool
actually populated the toolState (validation-2 succeeded), but failed
for runs that took a slightly different path where the assignment
didn't propagate visibly to handleAgentResult — even when the review
verifiably posted to GitHub.

Found this empirically: PR #19 (pure mechanical rename across 20
files) opened with the prior fix in place, the agent picked exactly
one impact lens (correct calibration!), confirmed no stale references,
submitted "Reviewed — no issues found." successfully (visible in
GitHub API), and the harness STILL errored with "agent completed
without reporting progress." Same SHA, same branch, same code as
validation-2 which passed. The toolState.review.id check turns out
not to be reliably visible from the run.ts handler in all paths.

Better fix: gate on toolState.selectedMode. Review and
IncrementalReview modes are designed to never call report_progress
(the review is the durable record, and IncrementalReview's
non-substantive path produces no artifact at all by design). The
harness completion check makes no sense for these modes — skip it
entirely. The agent's clean subprocess exit is the completion signal.

This also handles edge cases the previous fix missed: IncrementalReview's
non-substantive path (no review submitted by design) and any future
Review-flow shape that doesn't end at create_pull_request_review.

* ci: trigger Test run to validate models-live timeout/concurrency changes

* ci: prune passthrough models from live smoke matrix

openrouter/* aliases and keyed opencode/* aliases are routing-layer
wrappers around models we already smoke-test directly. running every
passthrough burns CI minutes (~30 min/run) without catching anything
the direct smoke doesn't — slug drift is already covered by the
models-catalog job.

keep one canary per routing layer (openrouter/claude-sonnet,
opencode/claude-sonnet) to validate auth + tool-call translation. free
opencode models stay in the matrix since they're unique to the provider.
INCLUDE_ALL_PASSTHROUGHS=1 bypasses the prune for full validation.

matrix size: 37 → 20 jobs.

* fix isRateLimited false-positive on UUIDs/timestamps containing 429

The bare "429" substring pattern was matching MCP session IDs (e.g.
`...-4429-...`) and microsecond timestamps in agent stdout, sending
transient failures down the 60s rate-limit retry path. With the new
4-minute per-step CI timeout, that backoff plus a slow retry pushed the
step past its budget and timed out.

Switch to regex patterns and gate the numeric code on `\b429\b` so word
boundaries prevent the substring false-match. Verified locally that the
UUID `97287d2f-ae1d-4429-8627-73e2454e80ca` and timestamp `02:04:50.9429654`
no longer match while real `HTTP 429` / `"status":429` strings still do.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>
Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-05-04 19:13:51 +00:00
Colin McDonnell 5e6ff67623 move models.dev drift tests to main-only; add per-alias live smoke matrix
PR CI kept breaking on upstream catalog drift (new model ships on models.dev,
OpenRouter renames an id, etc.) — failures unrelated to the PR's contents.
split the model-alias test suite so PRs only see pure-logic checks, and push
the external-state drift + end-to-end coverage to main.

test organization:
- action/test/models.test.ts keeps pure invariants: openRouterResolve
  completeness and fallback-chain resolution. runs on every PR.
- action/test/models-catalog.main.test.ts gets the 4 network-dependent
  describes (models.dev validity x2, OpenRouter API validity, latest-model
  snapshot). runs only on main push via a dedicated vitest config
  (vitest.main.config.ts + `pnpm test:catalog`).

new CI jobs in .github/workflows/test.yml:
- models-catalog: `pnpm test:catalog` on every main push. detects upstream
  catalog drift so we can react at the next convenient window.
- models-live: 38-entry matrix that invokes the agent harness end-to-end
  against the real provider for each alias in models.ts. generated from
  action/test/list-aliases.ts. runs only on main push AND only when
  resolution-affecting files changed (action/models.ts, action/package.json,
  action/agents/**) — the exact shape of the opus 4.7 incident.

test/run.ts: PULLFROG_MODEL now flows through from process.env so the live
matrix can pin an alias per job without the per-agent default clobbering it.

Made-with: Cursor
2026-04-16 21:10:15 +00:00
Colin McDonnell 4b3c5ca905 rename agent key to opencode and add skill invocation coverage (#541)
* rename agent key to opencode and add skill invocation coverage.

add skill-invoke tests for claude and opencode with local play-based validation signals, update CI matrices, and include the current tracked refactors in this branch for review.

Made-with: Cursor

* exclude agent-specific skill-invoke tests from wrong agent in CI matrix

* address review follow-up and preserve workflow run UI tweak.

switch changed-agents ci coverage to exercise the opentoad implementation path while keeping the opencode expectation, and include the local workflow run client interaction updates requested on this branch.

Made-with: Cursor

* remove opentoad agent filename from runtime.

rename the opencode harness implementation file from opentoad.ts to opencode.ts and update ci coverage input accordingly so action code no longer carries the old filename.

Made-with: Cursor

* ensure security prompt bypass is set on every test fixture.

this keeps adversarial and permissions harnesses from being blocked by the default prompt-injection refusal path during CI security tests.

Made-with: Cursor

---------

Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-04-15 19:38:36 +00:00
Colin McDonnell 8cd36d221a sandbox native filesystem tools to prevent /proc/self/environ exfiltration (#509)
* sandbox native filesystem tools to prevent /proc/self/environ exfiltration

the agent's native Read/Grep/Edit tools can bypass the shell sandbox by
reading /proc/self/environ directly. this adds agent-native filesystem
restrictions using the highest-precedence, non-overridable config for each CLI:

OpenCode: OPENCODE_PERMISSION env var with external_directory deny-all + /tmp allow,
plus deletion of untrusted .opencode/plugins/ and .opencode/tools/ before launch.

Claude Code: managed-settings.json at /etc/claude-code/ with denyRead, permissions.deny,
allowManagedPermissionRulesOnly, allowManagedHooksOnly. also --setting-sources user and
--disallowedTools path patterns as belt-and-suspenders.

Made-with: Cursor

* add Glob to Claude Code /proc and /sys deny lists

closes gap identified in review — Glob can enumerate /proc entries.
added to both managed-settings.json permissions.deny and --disallowedTools.

Made-with: Cursor

* run token-exfil test with both agents, hint at native /proc reads

changed tag from "agnostic" (opentoad-only) to "security" so the test
runs with both opentoad and claude. updated prompt to explicitly instruct
the agent to try reading /proc/self/environ via native Read tool.
added API keys to action-agnostic CI job for claude support.

Made-with: Cursor

* move token-exfil to crossagent matrix, remove redundant permissions.deny

- moved token-exfil from agnostic/ to crossagent/ so it runs via the
  agent matrix (claude + opentoad in parallel) instead of sequentially
- removed permissions.deny per-tool rules from managed-settings.json;
  sandbox.filesystem.denyRead is the single enforcement mechanism
- reverted action-agnostic env vars to minimal set
- updated wiki to match

Made-with: Cursor

* document post-spawn API key deletion analysis in security wiki

evaluated whether API key env vars can be deleted from agent processes
after spawn. OpenCode snapshots env at startup (safe to delete), but
Claude Code re-reads process.env per request (not viable). documented
as further exploration item with per-agent breakdown and caveats.

Made-with: Cursor

* fix stale tokenExfil path references in wiki docs

moved from test/agnostic/ to test/crossagent/ in directory tree
and adversarial test example.

Made-with: Cursor

* revert accidental prisma.config.ts changes

Made-with: Cursor

* hardcode PULLFROG_MODEL per agent in test runner to avoid DB model mismatch

when PULLFROG_AGENT forces a specific agent, the DB-configured model may
belong to a different provider (e.g. openai model with claude agent).
PULLFROG_MODEL short-circuits the DB slug resolution entirely.

Made-with: Cursor
2026-04-02 22:31:41 +00:00
Colin McDonnell b1f9878877 extract resolveModel() to run before agent selection
model resolution was duplicated inside each agent (opentoad, claude) and
PULLFROG_MODEL override was not considered when choosing the agent. now
resolveModel() runs first in main.ts, its result feeds into resolveAgent()
for agent selection, and the resolved model is passed to the agent via
ctx.resolvedModel. agents only handle their own fallback (opentoad: auto-select
via opencode models, claude: strip provider prefix).

also removes the hardcoded anthropic/claude-sonnet test runner default since
ANTHROPIC_API_KEY is no longer in CI.

Made-with: Cursor
2026-04-01 06:51:18 +00:00
Colin McDonnell 1f1e3995f9 fix test runner model: claude-sonnet-4-5 → claude-sonnet-4-6
Made-with: Cursor
2026-04-01 06:34:54 +00:00
Colin McDonnell e6d34ee01b add OpenRouter proxy for managed model routing and OSS program (#488)
* add OpenRouter proxy for managed model routing and OSS program

proxy layer that mints ephemeral OpenRouter keys for users without BYOK
API keys. two paths: pro plan users get their selected model proxied via
OpenRouter; OSS program repos (hard-coded allowlist in config.ts) get
free Claude Opus. BYOK env vars (PULLFROG_MODEL/OPENCODE_MODEL) always
take precedence.

frontend: OSS repos see a static "Opus (Free)" badge with the model
dropdown disabled and no API key requirement. all models now carry
openRouterResolve metadata for proxy target resolution.

Made-with: Cursor

* implement OSS program: proxy infrastructure for free model credits

server-side OSS allowlist determines eligible repos. action mints
ephemeral OpenRouter keys via OIDC-authenticated /api/proxy-token
endpoint (idempotent on runId, $10 per-key safety limit). keys are
disabled on workflow completion when no running refs remain. HWM-based
usage sync tracks cumulative spend per account.

schema: ProxyKey model, Account.usageUsd/activeKeyId, WorkflowRun.proxyKeyId
action: OIDC credential stashing, resolveProxyModel uses server oss flag
frontend: isOss flows from server page to components (no client allowlist)
Made-with: Cursor

* address PR review: repo cross-check, key retirement lifecycle, dead code removal

- proxy-token: verify runId belongs to OIDC-authenticated repo via repo relation
- add retireKey() shared primitive: disable in OpenRouter first, then mark disabled in DB
- rotateKey: retire old active key after swap to prevent orphans
- webhook: replace inline cleanupProxyKey with retireKey calls
- syncAccountUsage: skip disabled keys
- remove vestigial AccountPlan/plan field from action types
- add disabled field to ProxyKey schema + migration

Made-with: Cursor

* replace deprecated opencode/mimo-v2-flash-free with mimo-v2-pro-free

Made-with: Cursor

* fix migration ordering: rename disabled migration to sort after table creation

Made-with: Cursor

* squash proxy key migrations into single migration

Made-with: Cursor

* add preview repo to OSS allowlist for testing

Made-with: Cursor

* populate OSS allowlist from oss-program-invitees.json

Made-with: Cursor

* format oss-program-invitees.json

Made-with: Cursor

* add installed public repos to OSS allowlist

split ossRepos into three provenance-tracked lists:
- internalRepos (pullfrog, colinhacks, RobinTail)
- installedPublicRepos (external public non-fork repos with active installs)
- invitees (from oss-program-invitees.json)

also adds scripts/list-oss-candidates.ts to regenerate the installed list

Made-with: Cursor

* fix: resolve tokens before clearing OIDC env vars

resolveTokens → acquireNewToken → isOIDCAvailable() checks
ACTIONS_ID_TOKEN_REQUEST_URL/TOKEN env vars. The new OIDC
stashing code was deleting them in restricted shell mode before
resolveTokens ran, causing it to fall through to the GitHub App
path which requires GITHUB_APP_ID/GITHUB_PRIVATE_KEY.

Made-with: Cursor

* derive proxy-token auth from OIDC claims, add ensureWorkflowRun upsert

- proxy-token no longer requires body.runId; uses claims.run_id + claims.repository
- shared ensureWorkflowRun upsert called from both webhook and proxy-token
- workflow_run_requested handler now eagerly creates WorkflowRun records
- eliminates race condition between webhook and action proxy-token call

Made-with: Cursor

* hardcode PULLFROG_ACCOUNT_ID, document preview debugging lessons

GitHub node IDs are constant — no reason for this to be an env var.
Removes the trailing-newline bug that caused P2025 errors.
Adds wiki docs on workflow testing, Vercel env gotchas, and Neon
preview branch discovery.

Made-with: Cursor

* fix: parse OpenRouter create-key response correctly

the API returns `key` at the top level, not inside `data`

Made-with: Cursor

* onboarding cards, unlock OSS model selection, simplify console

- add OnboardingCard component with two states: workflow install
  and model+test (dispatches "Tell me a joke" for test run)
- replace PromptBox overlay gates with dedicated onboarding cards;
  PromptBox is now just the form, always enabled
- use hasWorkflowRuns DB check to decide onboarding vs promptbox
- unlock ModelSelector for OSS repos (was locked to Opus badge);
  resolve proxyModel from repo's selected model alias in run-context
- rename "API key" row to "Model costs" with pure client-side states:
  OSS covered, auto-resolve, free model, BYOK with env var names
- add "(Recommended)" badge to model aliases with recommended: true
- remove OSS_MODEL_DISPLAY_NAME constant and secrets-fetching logic

Made-with: Cursor

* update stale xai model snapshot

Made-with: Cursor

* rename Permissions to Security, hide git push toggle, add shell isolation toggle with disabled state for public repos, remove opentoad agent name from logs

Made-with: Cursor

* chevron hover states, sidebar hooks/security entries

Made-with: Cursor

* address PR review feedback: rename recommended→preferred, fix dispatch orphan, update wiki docs

- rename `recommended` to `preferred` in model alias registry to distinguish
  from the UI "Recommended" badge (which is hardcoded for opus + codex only)
- cancel precreated WorkflowRun when workflow lookup fails in dispatch-workflow
- replace run_sql/vercel env pull in wiki docs with neonctl + Prisma pattern
- extend scripts/neon-branch.ts to output DATABASE_URL via neonctl
- add Recommended badge to GPT Codex alongside Claude Opus in ModelSelector

Made-with: Cursor
2026-03-25 17:19:55 +00:00
Colin McDonnell 6d25adfd1a Agent & model refactor (#478)
* agent & model refactor with ASKPASS git auth, UI restructure, clerk v7

Made-with: Cursor

* fix stale agent/effort refs, add tests for askpass + model resolution

- reviewCleanup.ts: payload.agent -> payload.model, remove effort
- selectMode.ts PlanEdit: remove delegation/subagent/effort references
- pullfrog.yml.ts: update env vars (drop GOOGLE_API_KEY/CURSOR_API_KEY,
  add GOOGLE_GENERATIVE_AI_API_KEY/XAI_API_KEY/MOONSHOT_API_KEY/OPENCODE_API_KEY)
- FlagsSettings/RepoInstructionsSection: remove stale effort/timeout copy
- new: gitAuthServer.test.ts (10 tests — lifecycle, token delivery, tamper detection, script gen)
- new: agent.test.ts (4 tests — default opentoad, AGENT_OVERRIDE, invalid override)
- new: models.test.ts (19 tests — parseModel, resolution, registry invariants)
- update models.dev snapshot

Made-with: Cursor

* fix changed-agents.sh to filter legacy agent files from CI matrix

legacy agent files (claude.ts, codex.ts, etc.) are @ts-nocheck and not
exported from index.ts. changed-agents.sh now reads index.ts imports to
build the active agent set and treats changes to inactive files as
non-agent changes (opentoad canary only).

Made-with: Cursor

* remove MCP file tools, old agent harnesses, and obsolete security tests

ASKPASS-based git auth makes the old MCP file tool security layer unnecessary:
- token never in subprocess env, so symlink/gitattributes/hook attacks can't exfiltrate it
- agents now use native file tools (OpenCode builtin read/edit)

deleted:
- action/mcp/file.ts (file_read, file_write, file_edit, file_delete, list_directory)
- action/mcp/index.ts (dead re-export)
- agent harnesses: claude.ts, codex.ts, cursor.ts, gemini.ts, opencode.ts
- opencode-runner.ts (inlined into opentoad.ts)
- security tests that validated MCP file tool restrictions
- commented-out three-step review flow (~300 lines)
- sanitizeSchema/wrapSchema dead code from mcp/shared.ts
- OPENCODE_MODEL_MINI/MAX env vars (effort-level model overrides removed)

updated test prompts to use generic file ops instead of MCP tool names.
restored pkg-json-scripts + requirements-txt-attack (test --ignore-scripts defense).

Made-with: Cursor

* bump actions/checkout v4 → v6 (node 24)

node 20 actions deprecated june 2, 2026.

Made-with: Cursor

* temporarily disable fail-fast on agnostic tests to debug checkout@v6

Made-with: Cursor

* re-enable fail-fast on agnostic tests

Made-with: Cursor

* fix test token mismatch: mint OIDC tokens scoped to target repo

CI tests override GITHUB_REPOSITORY to pullfrog/test-repo but inherit
the runner's GITHUB_TOKEN (scoped to pullfrog/app), causing 401s on
every run-context fetch. Clear GITHUB_TOKEN in the test subprocess so
ensureGitHubToken() mints a properly scoped token via OIDC.

Also centralizes the default GITHUB_REPOSITORY in runAgentStreaming
instead of repeating it in every test file, and fixes preview-cleanup
to remove workers from all queues (not just name-matching ones).

Made-with: Cursor

* fix ensureGitHubToken to try OIDC when app credentials are absent

ensureGitHubToken only attempted token minting when GITHUB_APP_ID and
GITHUB_PRIVATE_KEY were set. In CI, OIDC is available but app creds
aren't exposed — so the guard prevented minting entirely.

Made-with: Cursor

* dead code cleanup: remove remnants of deleted agents, file tools, effort system

remove unused @anthropic-ai/claude-agent-sdk and @openai/codex-sdk deps,
orphaned file-tool security tests, dead GEMINI_MODEL passthrough, stale
opencode-runner wiki refs, deleted test file references, and MCP file tool
docs. rename docs/effort → docs/models. fix vitest setup: move dotenv to
globalSetup (runs once before forks instead of per-file, 19s → 200ms).

Made-with: Cursor

* address review feedback: remove dead code, update stale references

- remove AGENT_OVERRIDE (only opentoad exists)
- remove shellToolName plumbing (always restricted shell)
- bump action version to 0.0.179
- remove CURSOR_API_KEY from all workflows/configs
- remove OPENCODE_MODEL_MINI/MAX from workflows/docs
- delete wiki/effort.md, rewrite docs/effort.mdx as "Models"
- rewrite wiki/modes.md: orchestrator/subagent → single agent
- simplify flag system: drop builtin flag extraction (debug, effort,
  timeout, agent), keep custom flag replacement only
- reserve all legacy flag names to prevent custom flag conflicts

Made-with: Cursor

* regenerate lockfile after removing claude-agent-sdk and codex-sdk

Made-with: Cursor

* fix import ordering, add lockfile check to pre-push hook

Made-with: Cursor

* remove dead debug payload field, stale packageExtensions

Made-with: Cursor

* merge proc-sandbox and token-exfil into a single test

proc-sandbox and token-exfil were duplicative — both tested that
SANDBOX_TEST_TOKEN couldn't be exfiltrated. consolidated into
token-exfil with shell:restricted (which actually exercises filterEnv)
and the /proc attack vector hints from proc-sandbox.

Made-with: Cursor

* fix wiki adversarial.md to match actual tokenExfil validator

Made-with: Cursor
2026-03-12 05:22:51 +00:00
David Blass cfd38d82fc refactor delegation system, add PR summary comments, and improve code quality (#334)
* refactor delegation system and add PR summary comments

Delegation system:
- replace mode-based delegation with select_mode → delegate two-step flow
- orchestrator crafts self-contained subagent prompts (clean context — no system/repo/event instructions leak)
- add role-based tool filtering via FastMCP authenticate hook (?role=subagent hides orchestrator-only tools)
- add select_mode tool for orchestrator guidance per mode
- add ask_question tool for lightweight research subagents
- extract shared subagent lifecycle into subagent.ts (create, complete, stdout, instructions)
- route set_output to per-subagent state when activeSubagentId is set
- track per-subagent state (SubagentState Map) replacing boolean delegationActive flag
- capture and aggregate AgentUsage across all agents (claude, codex, gemini, opencode)
- write usage summary table to GitHub job summary
- block built-in subagent spawning (Task for Claude, Task(*) for Cursor)
- increase activity timeout from 60s to 300s (subagent thinking phases)
- fix gh CLI misguidance in system prompt — explicitly forbid usage

PR summary comments:
- add prSummaryComment trigger (DB schema + migrations + Zod + UI toggle)
- dispatch mini-effort summary job alongside PR review on pr.created
- add update_pull_request_body MCP tool
- add defaultEffort option to webhook dispatch

Hardening:
- rewrite delegate/selectMode tests with simulated state management
- add toolFiltering.test.ts for role extraction, canAccess, set_output routing
- remove non-null assertions for PULLFROG_TEMP_DIR (proper error throws)
- use fetchWithRetry for direct tarball downloads
- DRY fix for rate limit check in test runner

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: add type keyword to Effort import in handleWebhook.ts

Co-authored-by: Cursor <cursoragent@cursor.com>

* clean up delegation system, improve code quality across the codebase

- simplify delegate tool to instructions + effort params with subagent lifecycle in subagent.ts
- add select_mode and ask_question orchestrator-only tools with canAccess filtering
- replace delegate.test.ts/selectMode.test.ts with toolFiltering.test.ts (live MCP integration)
- add set_output routing for subagent context and AgentUsage tracking across all agents
- add PR summary comment trigger (schema, UI, webhook dispatch with silent flag)
- add update_pull_request_body MCP tool
- fix changed-agents.sh to always include claude canary for non-agent action changes
- fix cursor pagination bug in getSelectedInstallationReposPage
- remove destructuring patterns, inline type definitions, and unsafe type casts
- replace non-null assertions with explicit checks in install.ts
- convert multi-param functions to single param objects (postCleanup, runActionLocal, etc.)
- use isHttpError helper in API routes instead of catch-any patterns
- add adhoc test fixtures for delegation scenarios (context isolation, error handling, synthesis, etc.)

Co-authored-by: Cursor <cursoragent@cursor.com>

* no subagent mutation, one mcp per subagent

* address review feedback: parallel-safe usage tracking, subagent isolation, minor improvements

* fix subagent state isolation: replace Object.freeze with shallow copy

Object.freeze throws TypeErrors when subagent tools (checkout_pr,
report_progress) write scalar properties to toolState. A shallow copy
achieves the same isolation for scalar fields while allowing tools to
work normally. Shared references (subagents Map, usageEntries array)
remain shared for coordination.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-02-22 14:12:43 +00:00
pullfrog[bot] ee100354da fix: replace domain-specific exit handler with generic signal handler registry (#299)
* fix: replace domain-specific exit handler with generic signal handler registry

Rewrite exitHandler.ts as a generic, domain-agnostic exit signal module
that exports onExitSignal(handler) returning a dispose function.

- subprocess.ts now registers via onExitSignal instead of direct
  process.on(SIGINT/SIGTERM) calls
- resolveTokens registers a signal handler that captures tokens by
  closure, fixing the race condition where the exit handler would
  read the wrong token after disposal
- Remove setupExitHandler and runCleanup — domain cleanup is handled
  by post.ts + await using

Co-authored-by: Cursor <cursoragent@cursor.com>

* tweaks

* simplify handler installation

* extract to util

* fix race in dispose

* wrap dispose body in try/finally to ensure disposingRef always settles

---------

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-02-20 10:23:56 +00:00
Colin McDonnell b80c78bdbe Tweak 2026-02-14 03:58:27 +00:00
Colin McDonnell 8fd2b6aacb Tweak 2026-02-14 03:52:08 +00:00
Colin McDonnell 6313b09e30 switch opencode tests to codex, gemini tests to flash-preview
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-14 03:24:21 +00:00
Colin McDonnell b753c67d0a switch test default models to gemini-2.5-pro
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-14 03:12:47 +00:00
Colin McDonnell 4789a2b5e3 respect GEMINI_MODEL and OPENCODE_MODEL env vars in test runner
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-14 02:49:13 +00:00
Colin McDonnell 06683c1e0a respect GEMINI_MODEL and OPENCODE_MODEL env vars in test runner
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-14 02:46:39 +00:00
Mateusz Burzyński 8037c118cc Generate tokens before running action/play.ts (#296)
* Generate tokens before running `action/play.ts`

* Extract `ensureGitHubToken` utility

---------

Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-02-13 20:09:54 +00:00
David Blass ceadb3120a pin all CLI installations to explicit versions and use pro models by default (#283)
* pin all CLI installations to explicit versions and use pro models by default

- codex: pin @openai/codex to 0.101.0 (was "latest")
- opencode: pin opencode-ai to 1.1.56 (was "latest")
- gemini: pin gemini-cli to v0.28.2 via new tag param on installFromGithub
- cursor: pin to 2026.01.28-fd13201 via direct tarball download (replaces curl install script)
- add installFromDirectTarball to install.ts for versioned tarball URLs
- gemini auto effort now uses pro-preview instead of flash-preview
- test runner model overrides updated to use pro-preview consistently

Co-authored-by: Cursor <cursoragent@cursor.com>

* increase activity timeout

* fix delegation timeout

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-12 22:48:29 +00:00
David Blass 9071c0ae6c refactor mode selection into delegate tool that spawns subagents (#265) 2026-02-12 19:34:47 +00:00
David Blass bc28c658f2 harden sandbox escape vectors for bash disabled/restricted modes (#257)
* harden sandbox escape vectors for bash disabled/restricted modes

block git config injection (-c flag as subcommand), dangerous subcommands
(config, submodule, rebase, bisect), code-executing arg flags (--exec,
--extcmd), .gitattributes/.gitmodules writes, and package lifecycle scripts.
add retry logic to test runner for transient failures. add security unit
tests and adhoc attack tests.

Co-authored-by: Cursor <cursoragent@cursor.com>

* only filter subcommands in nobash, remove nobash from ui

* use regex matching

* iterate on tests

* simplify githooks

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-11 02:02:41 +00:00
David Blass 19df8372cd add file_read/file_write tools, sandbox tests, CI improvements (#239)
* migrate to flags

* init

* iterate on file write lockdown tests

* improve ci

* fix lockfile

* fix typecheck

* fix lint

* improve pushRestricted

* ok

* fix more

* ok

* remove process.env spreading rule

Co-authored-by: Cursor <cursoragent@cursor.com>

* enhanced fs rw tools

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Colin McDonnell <colinmcd94@gmail.com>
2026-02-10 06:35:47 +00:00
Colin McDonnell 3a7145db1a Scope installation token permissions in restricted mode (#226)
* Scope installation token permissions in restricted mode

In restricted/disabled bash mode, the installation token is now scoped
to match the workflow's permissions block. This preserves fork push
capability while limiting what the agent can do with the token.

- Read workflow permissions from pullfrog.yml at runtime
- Pass permissions to API when acquiring installation token
- Clear OIDC env vars in restricted mode to prevent token minting
- Simplify setupGit by moving token resolution to main.ts

* Address review feedback: fail closed with default permissions

- Add restrictive default permissions (contents:read, pull_requests:read,
  issues:read) as fallback when workflow permissions can't be read
- Add support for job-level permissions via GITHUB_JOB env var
- Fix misleading comment about token resolution in restricted mode
- Add documentation about fork PR checkout behavior

* Simplify to separate git/MCP tokens without workflow permission scoping

- gitToken: minimal contents:write only (assumed exfiltratable)
- mcpToken: full installation token (not exfiltratable via MCP tools)
- Remove workflowPermissions.ts - security-conscious users can pass
  their own token via GH_TOKEN or inputs.token
- Add type-safe InstallationTokenPermissions to github.ts and API route

* Rename `write` permission to `push` and remove vestigial tool blocking

The `write` permission was previously used to block local file write tools
in agents. This was security theater since bash can write files anyway.

Now `push` only controls the git token scope:
- push: enabled → contents:write (can push commits)
- push: disabled → contents:read (read-only, can't push)

Changes:
- Rename `write` to `push` in action.yml, Prisma schema, and all TS types
- Remove vestigial write tool blocking from all agents (claude, cursor,
  gemini, opencode, codex)
- Add data-preserving Prisma migration using RENAME COLUMN
- Update UI: "Write files" → "Git push" with updated description

* add PID namespace isolation for bash sandbox

when running in CI, attempts to use unshare --pid to create a new PID
namespace for bash subprocesses. this prevents the /proc/$PPID/environ
attack where a malicious command could read secrets from the parent
process's environment.

the protection works by:
1. creating a new PID namespace (subprocess becomes PID 1)
2. mounting fresh /proc showing only sandbox PIDs
3. parent PIDs become invisible (PPID = 0, /proc/0 doesn't exist)

combined with filterEnv(), this provides complete protection against
/proc-based secret theft. falls back gracefully if namespaces aren't
available.

includes test script to verify the protection works.

* add PID namespace test to CI workflow

tests whether unshare --pid works on GHA runners out of the box,
and if not, whether enabling via sysctl helps. also runs the
pidNamespace.ts test to verify the full protection.

* fix pnpm setup and add procIsolation agent test

- fix pnpm/action-setup by specifying package_json_file path
- add procIsolation crossagent test that has agent attempt to
  read secrets via /proc/$PPID/environ
- add procIsolation to CI test matrix

* add pid-namespace test job to main workflow

this job tests unshare --pid capabilities on GHA runners and runs
the pidNamespace.ts adhoc test to verify /proc isolation works

* test bubblewrap's sysctl approach for enabling namespaces

- write to /etc/sysctl.d/99-userns.conf and run sysctl --system
- try aa-complain on unshare binary
- more detailed diagnostics

* fix pidNamespace test and add sudo-unshare fallback for GHA

- fix reference error in pidNamespace.ts (renamed function but didn't update calls)
- add sudo-unshare as fallback method for GHA runners where unprivileged
  namespaces are blocked but sudo is available
- update bash.ts to detect and use sudo unshare when unprivileged fails

* consolidate security docs and document PID namespace isolation

- update security.md with current implementation details
  - document sudo unshare fallback for GHA runners
  - add testing instructions for local Docker and CI
  - add "Further Exploration" section with Landlock and path validation ideas
- delete bash-sandbox.md and landlock.md (consolidated into security.md)

* move procIsolation test to adhoc folder

the procIsolation test requires PID namespace capabilities that aren't
available in the Docker test environment. moved to adhoc/ so it's excluded
from default test runs and can be run explicitly when needed (e.g. via
the pid-namespace CI job or locally with --privileged docker).

* fix Docker test environment for PID namespace isolation

- add CI and GITHUB_ACTIONS to testEnvAllowList so sandbox detection runs
- add --privileged to Docker run for PID namespace support (unshare)

this fixes the test environment to properly test the sandbox. in production,
the action runs directly on GHA runner where sudo unshare works.

* fix getJobToken() to work in test environment

add fallback to GH_TOKEN and GITHUB_TOKEN when INPUT_TOKEN is not set.
this allows tests to run without requiring workflow-level token input.

the token resolution order is:
1. INPUT_TOKEN (from workflow `with: token:`)
2. GH_TOKEN (external token override)
3. GITHUB_TOKEN (pre-acquired in tests or from GHA env)

* security: filter secrets from all subprocess environments

- extract filterEnv() to shared utils/secrets.ts
- make $() utility filter secrets by default (git, npm, etc. don't need them)
- disable git hooks via core.hooksPath to prevent hook-based exfiltration
- git auth uses token embedded in URL, not env vars

this prevents malicious git hooks, npm postinstall scripts, and other
code execution vectors from exfiltrating GITHUB_TOKEN and API keys.

* docs: clarify defense-in-depth security model

update security.md to explain why BOTH layers are required:
- filterEnv(): cleans child's own /proc/self/environ
- PID namespace: hides parent's /proc entries

PID namespace alone isn't sufficient - with --mount-proc, the child
becomes PID 1, so /proc/1/environ is the child's OWN environment.
without filterEnv(), secrets would still be accessible there.

* add procSandbox crossagent test for PID namespace security

- add crossagent/procSandbox.ts: security test that instructs agent to try
  various /proc attack vectors and validates sandbox blocks them
- update wiki/security.md: document PID namespace isolation details, add
  verification commands, explain why sudo inside sandbox doesn't break security
- update docker.ts: use node:24 with sudo for GHA-like test environment
- update instructions.ts: allow disabling security messaging for pentests
- clean up adhoc test files (procIsolation.ts, securityAudit.ts)

the procSandbox test sets SANDBOX_TEST_TOKEN (matches *_TOKEN filter), gives
the agent explicit hints about attack vectors (/proc, sudo, nsenter, etc.),
disables prompt-based mitigations, and validates the secret is never leaked.

* move procSandbox test to agnostic/ (runs with one agent)

* WIP

* docs: add agent testing guide (pnpm play, Docker, pentesting)

* docs: add CI details to agent testing guide

* docs: add interesting findings and gotchas from pentesting

* improve test fidelity: auto-set CI=true, verify sandbox active

- docker.ts: always set CI=true in container so sandbox activates
- docker.ts: skip sudo/user setup if already done (faster reruns)
- bash.ts: export getSandboxMethod() for diagnostics
- bash.ts: add debug log when sandbox disabled
- procSandbox.ts: add sandbox_was_active check to catch vacuous passes

the CI=true change is critical: without it, PID namespace isolation
is skipped and security tests pass without actually testing anything.

* docs: update agent-testing.md with CI=true auto-set note

* docs: clarify log format is agent-specific

* fix git auth, simplify MCP tools, add adversarial tests

- fix gitWithAuth to use Basic auth format (Bearer doesn't work with git's http.extraheader)
- fix token.ts: push:restricted now correctly gets contents:write
- fix github.ts: pass permissions when acquiring installation tokens locally
- remove commit_files and create_branch MCP tools (redundant, don't require credentials)
- remove containsSecrets() - trivially bypassable, not sound security
- add agnostic adversarial tests: pushDisabled, pushRestricted, tokenExfil
- update instructions.ts to clearly list available git MCP tools
- add wiki/git.md documenting credential isolation and push permission tiers
- update wiki/docker.md with custom image considerations
- update wiki/agent-testing.md with adversarial testing patterns

* fix type errors after rebase

- change ResolveTokensParams.push from ToolPermission to PushPermission
- use tags: ["agnostic"] instead of agnostic: true in test files

* fix cleanup permission error in sandbox tests

when sandbox isolation is enabled (CI=true), files created by the unshare
subprocess may have different ownership, causing rmSync to fail with EACCES.
this error in the finally block was overriding the test's success result.

fix: wrap cleanup in try-catch and fall back to sudo rm if rmSync fails.

* Add adhoc

* Handle git config/remote bypasses

* add git hooks protection and simplify ToolState

- disable git hooks in restricted mode via -c core.hooksPath=/dev/null
- add gitHooks adversarial test to verify hook protection works
- unify prNumber/issueNumber into single issueNumber field
- add pushUrl to ToolState for push validation
- add generateTestMarker() for simpler single-agent test markers
- export SENSITIVE_PATTERNS and isSensitiveEnvName from secrets.ts
- remove redundant pidNamespace.ts (duplicated by procSandbox.ts)
- update documentation

* harden $git() auth: subcommand whitelist, binary tamper detection

- rename gitWithAuth() to $git() with explicit subcommand first arg
- restrict to "fetch" | "push" at type level (filters don't run for these)
- resolve git binary path at startup via resolveGit(), sha256 fingerprint
- verify hash before each $git() call to detect binary replacement
- rename disableHooks to restricted for cleaner semantics
- document filter exfiltration attack and empirical verification in wiki

* remove redundant pid-namespace CI job

the PID namespace isolation testing is now handled by
action/test/agnostic/procSandbox.ts via pnpm runtest agnostic

* fix push_branch for new branches and improve token leak detection

- getPushDestination now falls back to origin/<branch> when @{push}
  is not configured (happens for new branches created locally)
- gitPerms validator now checks for actual token patterns instead
  of matching "x-access-token" string in test instructions

* use kebab-case for test names

* simplify shell env API: "restricted" | "inherit" | object

replace passFullEnv boolean with cleaner env option that accepts:
- "restricted" (default): filterEnv() to prevent secret leakage
- "inherit": full process.env
- object: custom env merged with restricted base

* share EnvMode and resolveEnv between shell.ts and bash.ts

move shared env resolution logic to secrets.ts

* add env option to bash tool (default: restricted)

* delete agent-testing.md (renamed to adversarial.md)

* Add checkout tests

* reframe githooks test prompt to avoid claude safety refusal

claude was refusing to execute the test because the prompt used words
like "malicious" and "security testing". reframed as a debugging task
with innocuous env var name (TESTING_DEBUG_TAG) per adversarial.md guidance.

Co-authored-by: Cursor <cursoragent@cursor.com>

* clean up verbose token acquisition logs

move logging responsibility to call sites which have better context
(git token vs MCP token). remove redundant intermediate OIDC logs
and unused "(permission-scoped)" suffix.

Co-authored-by: Cursor <cursoragent@cursor.com>

* isolate agnostic tests with matrix strategy, fix .pullfrog-env secret leak

- split action-agnostic into per-test matrix jobs for isolated logs and filesystems
- only write explicitly opted-in env vars to .pullfrog-env via fileAgentEnv
  (fixes token-exfil test where claude found SANDBOX_TEST_TOKEN on disk)
- mcpmerge test opts in via fileAgentEnv for cursor's repo-level MCP fallback

Co-authored-by: Cursor <cursoragent@cursor.com>

* remove env parameter from bash tool to prevent agents bypassing filterEnv

the bash tool exposed an `env` parameter accepting "restricted" | "inherit"
which allowed agents to pass env: "inherit" and see all secrets including
SANDBOX_TEST_TOKEN, bypassing the restricted environment filtering entirely.
env mode is now determined internally (always restricted).

Co-authored-by: Cursor <cursoragent@cursor.com>

* use pullfrog/test-repo for push tests to stop polluting main repo

push tests were creating branches and tags on pullfrog/app directly.
now all push tests (push-restricted, push-disabled, push-enabled,
git-permissions) target pullfrog/test-repo instead.

Co-authored-by: Cursor <cursoragent@cursor.com>

* use pullfrog/test-repo for all tests, not just push tests

no test should clone or operate on pullfrog/app directly.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix token scoping for test-repo and bash timeout defaults

- acquireTokenViaOIDC now includes GITHUB_REPOSITORY repo in token
  scope so push tests work against pullfrog/test-repo
- bash tool default timeout: 120s -> 30s, cap: 600s -> 120s
- activity timeout: 30s -> 60s
- prevents hung bash commands (e.g. find /) from killing the agent
  via activity timeout

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-02-06 06:26:26 +00:00
David Blass 6fbff21fca add agent and debug macros, improve activity timeouts, migrate claude and codex to cli (#224) 2026-02-04 22:29:45 +00:00
David Blass 18ba8e5fd0 improve runtest, optimize CI batching (#210) 2026-02-01 21:48:53 +00:00
David Blass 943409c417 add #timeout, macro errors, refactor tests (#191) 2026-01-28 21:06:57 +00:00