Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions .github/workflows/review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,6 @@ jobs:
NEEDLEFISH_MODEL_INPUT: ${{ inputs.model || github.event.inputs.model }}
NEEDLEFISH_TIMEOUT_MS_INPUT: ${{ inputs.timeout_ms || github.event.inputs.timeout_ms }}
CODEX_REASONING_EFFORT: ${{ inputs.codex_reasoning_effort || github.event.inputs.codex_reasoning_effort }}
# Deliberate on this disposable runner; the read-only toolset is probe-verified.
NEEDLEFISH_ALLOW_PI_RUNNER: "1"
# ubuntu-needlefish routes pi through the local CLIProxyAPI (ccp);
# credentials live in the proxy, provider registered in ~/.pi/agent/models.json.
PI_PROVIDER: cliproxy
Expand All @@ -234,12 +232,6 @@ jobs:
echo "Missing $HOME/.local/bin/needlefish. Run scripts/deploy-ubuntu.sh on the self-hosted runner." >&2
exit 1
fi
# Grok 4.5 emits valid review JSON only in its unsandboxed mode;
# opt into that mode for the explicitly selected Grok lane only.
unset NEEDLEFISH_ALLOW_GROK_UNSANDBOXED
if [ "$NEEDLEFISH_RUNNER_INPUT" = "grok" ]; then
export NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1
fi
args=(--github --pr "${{ inputs.pr_number || github.event.inputs.pr_number || github.event.pull_request.number }}")
if [ -z "$NEEDLEFISH_MODEL_INPUT" ]; then
case "$NEEDLEFISH_RUNNER_INPUT" in
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/weekly-eval.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ jobs:
fetch-depth: 0
- name: Run full eval
env:
NEEDLEFISH_ALLOW_PI_RUNNER: "1"
# pi routes through the local CLIProxyAPI (ccp) on ubuntu-needlefish.
PI_PROVIDER: cliproxy
PI_BIN: /home/termtek/.npm-global/bin/pi
Expand Down
7 changes: 4 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Frank owns this. Keep replies and docs terse unless formal prose is requested.

## OVERVIEW

Needlefish is a strict TypeScript PR review CLI. It collects local or GitHub PR diffs, sends them through read-only model runners, prunes weak findings with a critic pass, and derives the final verdict deterministically from validated findings.
Needlefish is a strict TypeScript PR review CLI. It collects local or GitHub PR diffs, sends them through isolated model runners, prunes weak findings with a critic pass, and derives the final verdict deterministically from validated findings.

## STRUCTURE

Expand Down Expand Up @@ -36,7 +36,7 @@ needlefish/
| Verdict rules | `src/core/verdict.ts` | Deterministic; do not let model prose decide pass/fail. |
| Local review | `src/adapters/local.ts` | Writes `~/.cache/needlefish/<repo>/last-review.json`. |
| GitHub review | `src/adapters/github.ts` | Posts COMMENT review plus `Needlefish` check-run. |
| Runner invocation | `src/shared/codex.ts`, `src/shared/runner-process.ts` | Timeout, retry, sandbox, and runner env behavior live here. |
| Runner invocation | `src/shared/codex.ts`, `src/shared/runner-process.ts` | Timeout, retry, target isolation, and runner env behavior live here. |
| Git/PR bundle shape | `src/shared/repo.ts`, `src/shared/schema.ts` | `agentsMd` is read from target repo root only. |
| Prompt behavior | `prompts/*.md` | Must remain read-only and output JSON contracts exactly. |
| Tests | `src/**/*.test.ts`, `scripts/test.mjs` | Node test runner, no Jest/Vitest. |
Expand Down Expand Up @@ -66,6 +66,7 @@ needlefish/
- TypeScript is strict/no-emit with `moduleResolution: "bundler"`.
- Use `unknown` at JSON/model/GitHub boundaries, then validate or narrow.
- All NEEDLEFISH_* boolean flags go through `envFlagOn` in `src/shared/env.ts`; only `"1"` is on.
- Production model CLIs intentionally run without process-level permission or sandbox restrictions on trusted self-hosted runners. Do not restore those restrictions; preserve the throwaway target clone and post-run integrity checks instead.
- Keep tests beside the code path as `src/**/*.test.ts`.
- Use Node built-ins (`node:test`, `assert/strict`, `spawnSync`, temp dirs) before adding dependencies.
- Stub external CLIs in tests with temp scripts and env vars.
Expand All @@ -77,7 +78,7 @@ needlefish/
- Do not implement `--fix` or multi-repo config unless explicitly requested.
- `--recheck` forces a full re-review (bypassing the same-head dedupe in GitHub mode); it is never incremental verification.
- Do not substitute global `AGENTS.md`, `~/.codex/*`, or CLI-injected files as target repo review policy. Only bundle `agentsMd` counts.
- Do not add new runner permissions that can mutate target repos; current model runners are read-only/sandboxed by design.
- Do not weaken the throwaway clone, token stripping, fixed `HEAD`, or post-run mutation checks that isolate unrestricted model runners from the original target repo.

## EVAL DISCIPLINE

Expand Down
57 changes: 23 additions & 34 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

[繁體中文](README.zh-TW.md)

> Strict, local, read-only PR review that acts like a senior engineer — it
> Strict, local PR review that acts like a senior engineer — it
> flags only real defects and stays silent on everything else.

<p align="center">
Expand All @@ -26,9 +26,8 @@ duplicate behavior — never style.
- **Deterministic verdicts.** The `pass` / `needs_human` / `changes_requested`
verdict is derived from the surviving findings by fixed rules, never
freehanded by the model.
- **Sandboxed, read-only runners.** Reviews run read-only by default; non-Codex
runners execute in a throwaway clean clone and are checked for tampering after
every model call.
- **Isolated review targets.** Reviews run against a throwaway clean clone and
are checked for tampering after every model call.
- **Guarded evals.** Every prompt/pipeline change is measured against an
84-scenario harness with active anti-cheat guards before it ships (see
[Benchmarks](#benchmarks)).
Expand Down Expand Up @@ -311,10 +310,8 @@ gh workflow run review.yml -R frankekn/needlefish --ref main \
-f pr_number="$PR_NUMBER" -f runner=grok -f model=grok-4.5
```

The Grok 4.5 lane deliberately opts out of Grok's process-level plan mode so
the CLI returns valid review JSON. Use it only on a self-hosted runner you
control; the workflow sets `NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1` only when
`runner=grok` is selected.
All production model runners execute without their own process-level permission
restrictions. Use them only on a self-hosted runner you control.

Because the caller pins `@main`, fixes to needlefish's `review.yml` propagate to
every target repo automatically. The runner must have needlefish deployed at
Expand All @@ -330,9 +327,9 @@ SHA so review jobs can fail before spending model tokens when a runner is stale.
```bash
ssh termtek@ubuntu 'sh -s' < scripts/deploy-ubuntu.sh
```
For a fleet, dispatch the same release SHA to all six selected runners and
verify each runner reports the same installed metadata before trusting the
fleet.
The current production fleet uses one shared x64 installation plus two
separate ARM installations. Deploy the same release SHA to all three
installations and verify their installed metadata before trusting the fleet.
3. Ensure the runner has `gh` and the selected model CLI on `PATH`.
4. On that runner, auth the selected CLI once. For Codex:
```bash
Expand Down Expand Up @@ -439,32 +436,24 @@ When neither `--runner` nor `NEEDLEFISH_RUNNER` is set and none of `codex`,
`claude`, or `opencode` can be found, Needlefish exits with install commands
for those three CLIs instead of a stack trace.

Codex runs with `--ignore-user-config -c model_reasoning_effort="<effort>" -s
read-only`. `medium` is the default; set `CODEX_REASONING_EFFORT=high` to
restore the old default, or `xhigh` for the highest-effort mode. Claude Code runs with
`--permission-mode plan`, `--safe-mode`, and no session persistence. grok runs
with `--permission-mode plan` by default. The self-hosted GitHub Grok 4.5 lane
sets `NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1` for valid JSON output, which omits
that restraint and is why it is limited to an explicitly selected runner.
opencode
runs with `--pure` and never uses `--dangerously-skip-permissions`, but as of
this writing opencode's headless `run` mode has **no** documented read-only or
permission flag — a live probe confirmed it executes shell/tool calls with no
gate at all in that mode. Because of that gap, the `opencode` runner refuses to
start unless `NEEDLEFISH_ALLOW_OPENCODE_RUNNER=1` is set explicitly; set it only
if you've separately sandboxed the environment opencode runs in. If opencode's
CLI later ships a real permission/sandbox flag, this opt-in gate should replace
the flag instead of the env-var check. pi runs with `--no-session --mode text
--provider openai-codex --thinking <level>`; pi advertises a `--tools
read,grep,find,ls` read-only mode, but that restriction is unverified under
headless invocation (same class as opencode), so the `pi` runner refuses to
start unless `NEEDLEFISH_ALLOW_PI_RUNNER=1` is set explicitly. ACP runs a
Codex runs with `--ignore-user-config --ignore-rules
--dangerously-bypass-approvals-and-sandbox` so its inspection commands are not
blocked by execpolicy rules, approval prompts, or the host sandbox. Needlefish
still runs it inside a throwaway clean clone, strips GitHub tokens, fixes the
expected `HEAD`, and rejects any worktree mutation. `medium` is the default; set
`CODEX_REASONING_EFFORT=high` to restore the old default, or `xhigh` for the
highest-effort mode. Claude Code runs with
`--dangerously-skip-permissions`, `--safe-mode`, and no session persistence.
Grok runs with `--always-approve --permission-mode bypassPermissions --no-plan
--sandbox off`. opencode runs with `--auto` in headless mode and an inline
`permission: "allow"` override for its global and build-agent permissions. pi
runs with `--no-session --mode text --provider openai-codex --thinking <level>`
and its default full toolset. ACP runs a
JSON-RPC 2.0 Agent Client Protocol process over stdio from `NEEDLEFISH_ACP_BIN`;
Needlefish sends `session/cancel` on timeout, then applies the same process-group
kill path as the CLI runners. Closed PRs are skipped before diffing or model
invocation. Non-Codex runners execute inside a throwaway clean clone at the
review head commit;
needlefish checks that sandbox with
invocation. All CLI runners execute inside a throwaway clean clone at the review
head commit; needlefish checks that clone with
`git status --porcelain --untracked-files=all --ignored=matching` and verifies
`HEAD` did not move after each successful model call.

Expand Down
34 changes: 20 additions & 14 deletions README.zh-TW.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ Needlefish 會在 merge 前檢查 diff,只回報真正的缺陷:錯誤、回
修的就捨棄。沒有風格挑剔,沒有雜訊。
- **確定性 verdict。** `pass`/`needs_human`/`changes_requested` 由保留下來的
finding 依固定規則推導,不由模型自由決定。
- **Sandboxed、唯讀 runner。** 審查預設唯讀;非 Codex runner 會在 throwaway
clean clone 中執行,並在每次模型呼叫後檢查是否遭竄改
- **隔離的審查目標。** 審查會在 throwaway clean clone 中執行,並在每次模型
呼叫後檢查是否遭竄改
- **有防護的 evals。** 每次 prompt/pipeline 變更上線前,都會用 84 個情境的
harness(啟用 anti-cheat guards)量測(見 [Benchmarks](#benchmarks))。

Expand Down Expand Up @@ -272,15 +272,16 @@ gh workflow run review.yml -R frankekn/needlefish --ref main \
-f pr_number="$PR_NUMBER" -f runner=grok -f model=grok-4.5
```

Grok 4.5 lane 會刻意停用 Grok 的 process-level plan mode,才能輸出有效
JSON。workflow 只在明確選擇 `runner=grok` 時設定
`NEEDLEFISH_ALLOW_GROK_UNSANDBOXED=1`,因此只能在你控制的 runner 上使用。
所有 production model runner 都不套用各 runner 自己的 process-level 權限限制,
因此只能在你控制的 self-hosted runner 上使用。

1. 在目標 repo 註冊 self-hosted runner,並限制在自己控制的機器。
2. 在 runner 部署 Needlefish;`main` 的 push 會觸發 `needlefish-deploy`:
```bash
ssh termtek@ubuntu 'sh -s' < scripts/deploy-ubuntu.sh
```
目前 production fleet 是一份共用 x64 安裝加兩份獨立 ARM 安裝。三份安裝都要
部署相同 release SHA,並確認 installed metadata 一致。
3. 確認 `gh` 與選定的模型 CLI 位於 `PATH`。
4. 以 runner service account 登入 CLI。Codex 例如:
```bash
Expand Down Expand Up @@ -348,15 +349,20 @@ Fork PR 預設不會收到 secrets,workflow 會跳過它們;不要在不了
| Codex reasoning effort | `CODEX_REASONING_EFFORT` | `medium`(reusable workflow:`gpt-5.6-terra` 時為 `high`) |
| timeout | `NEEDLEFISH_TIMEOUT_MS` | `600000` |

Codex 使用 `--ignore-user-config` 與唯讀 sandbox;Claude 使用 plan mode;
Grok 預設使用 `--permission-mode plan`,但 self-hosted Grok 4.5 lane 會
明確停用它以取得有效 JSON。opencode 與 pi 的 headless 唯讀能力未完全驗證,
因此必須分別設定 `NEEDLEFISH_ALLOW_OPENCODE_RUNNER=1` 或
`NEEDLEFISH_ALLOW_PI_RUNNER=1`。ACP 透過 `NEEDLEFISH_ACP_BIN` 使用 JSON-RPC
2.0 stdio process,timeout 時會先送 `session/cancel` 再終止 process group。

非 Codex runner 會在 review head 的 throwaway clean clone 中執行;每次成功
呼叫後都會確認 sandbox 沒有未提交變更且 `HEAD` 沒有移動。
Codex 使用 `--ignore-user-config --ignore-rules
--dangerously-bypass-approvals-and-sandbox`,避免檢查命令遭 execpolicy rule、
approval prompt 或 host sandbox 阻擋。Needlefish 仍會把它放在 throwaway clean
clone 內執行、移除 GitHub token、固定預期 `HEAD`,並拒絕任何 worktree 變更。
Claude 使用 `--dangerously-skip-permissions`;Grok 使用 `--always-approve
--permission-mode bypassPermissions --no-plan --sandbox off`;opencode 使用
`--auto` headless mode,並以 inline `permission: "allow"` 覆寫 global 與
build-agent 權限;pi 使用預設完整 toolset。這些 production runner 都不需要額外的
unsandboxed opt-in。
ACP 透過 `NEEDLEFISH_ACP_BIN` 使用 JSON-RPC 2.0 stdio process,timeout 時會先送
`session/cancel` 再終止 process group。

所有 CLI runner 都會在 review head 的 throwaway clean clone 中執行;每次成功
呼叫後都會確認 clone 沒有未提交變更且 `HEAD` 沒有移動。

### Runner subprocess 環境

Expand Down
15 changes: 1 addition & 14 deletions action.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: Needlefish PR Review
description: >-
Strict PR review agent. Reviews the pull request diff with a read-only model
Strict PR review agent. Reviews the pull request diff with an isolated model
runner and posts a COMMENT review plus a Needlefish check-run.
inputs:
pr_number:
Expand Down Expand Up @@ -47,16 +47,6 @@ runs:
corepack enable
corepack prepare "$(node -p 'require("./package.json").packageManager')" --activate
pnpm install --frozen-lockfile
- name: Enable unprivileged user namespaces (Linux hosted runners)
shell: bash
run: |
# Ubuntu 23.10+ AppArmor blocks unprivileged user namespaces, which
# breaks the codex CLI sandbox (bwrap) on GitHub-hosted VMs and
# degrades reviews to diff-only. Best-effort: a locked-down runner
# without sudo just keeps the degraded mode.
if [ "$(uname -s)" = "Linux" ] && sudo -n true 2>/dev/null; then
sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 || true
fi
- name: Install model runner CLI
shell: bash
env:
Expand Down Expand Up @@ -107,9 +97,6 @@ runs:
echo "No PR number: pass pr_number or trigger on pull_request events." >&2
exit 1
fi
# CI runs on a disposable VM, so the pi runner's unverified-sandbox gate
# (src/shared/codex.ts) is deliberately satisfied here.
if [ "$RUNNER_INPUT" = "pi" ]; then export NEEDLEFISH_ALLOW_PI_RUNNER=1; fi
args=(--github --pr "$pr" --repo "$TARGET_REPO_PATH")
if [ -n "$RUNNER_INPUT" ]; then args+=(--runner "$RUNNER_INPUT"); fi
if [ -n "$MODEL_INPUT" ]; then args+=(--model "$MODEL_INPUT"); fi
Expand Down
10 changes: 10 additions & 0 deletions scripts/deploy-ubuntu.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,13 @@ test("deploy does not install an optional model runner", async () => {

assert.doesNotMatch(script, /npm install -g @mariozechner\/pi/);
});

test("hosted action does not require runner permission opt-ins", async () => {
const action = await readFile("action.yml", "utf8");

assert.doesNotMatch(
action,
/NEEDLEFISH_ALLOW_(?:OPENCODE_RUNNER|PI_RUNNER|GROK_UNSANDBOXED)/,
);
assert.doesNotMatch(action, /apparmor_restrict_unprivileged_userns/);
});
6 changes: 3 additions & 3 deletions src/shared/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## OVERVIEW

`src/shared/` is the high-blast-radius layer: git/gh subprocesses, model runner sandboxing, JSON normalization, schema contracts, rendering, and file classification.
`src/shared/` is the high-blast-radius layer: git/gh subprocesses, model runner isolation, JSON normalization, schema contracts, rendering, and file classification.

## WHERE TO LOOK

Expand All @@ -19,13 +19,13 @@

- Treat subprocess output and model/GitHub JSON as untrusted.
- Use typed unions from `runner.ts` and `schema.ts`; do not widen to strings.
- Preserve runner safety checks: clean sandbox, fixed `HEAD`, stripped GitHub tokens, read-only runner modes.
- Preserve target isolation checks: clean throwaway clone, fixed `HEAD`, stripped GitHub tokens, and post-run dirty-worktree rejection.
- Keep process timeout behavior durable. A child that traps `SIGTERM` must not hang the review path.
- For test repos, use temp dirs and the fixture helpers in `codex-runner-test-fixtures.ts`.

## ANTI-PATTERNS

- Do not add a new runner without sandbox, timeout, token-stripping, and dirty-worktree tests.
- Do not add a new runner without target isolation, timeout, token-stripping, and dirty-worktree tests.
- Do not read target policy from anywhere except target root `AGENTS.md` or `git show <head>:AGENTS.md`.
- Do not swallow `spawn`/buffer/timeout failures into a passing review.
- Do not weaken schema normalization to accept malformed findings.
5 changes: 0 additions & 5 deletions src/shared/codex-ephemeral-home.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,6 @@ test("runCodex ephemeral HOME: opencode accepts explicitly passed provider API k
const fakeHome = mkdtempSync(path.join(os.tmpdir(), "needlefish-fakehome-"));
const previous = {
bin: process.env.OPENCODE_BIN,
allowOpenCode: process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER,
ephemeral: process.env.NEEDLEFISH_EPHEMERAL_HOME,
retry: process.env.NEEDLEFISH_NO_RETRY,
home: process.env.HOME,
Expand All @@ -504,7 +503,6 @@ test("runCodex ephemeral HOME: opencode accepts explicitly passed provider API k
t.after(() => {
for (const [name, value] of Object.entries({
OPENCODE_BIN: previous.bin,
NEEDLEFISH_ALLOW_OPENCODE_RUNNER: previous.allowOpenCode,
NEEDLEFISH_EPHEMERAL_HOME: previous.ephemeral,
NEEDLEFISH_NO_RETRY: previous.retry,
HOME: previous.home,
Expand Down Expand Up @@ -532,7 +530,6 @@ test("runCodex ephemeral HOME: opencode accepts explicitly passed provider API k
);
chmodSync(bin, 0o755);
process.env.OPENCODE_BIN = bin;
process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1";
process.env.NEEDLEFISH_EPHEMERAL_HOME = "1";
process.env.NEEDLEFISH_NO_RETRY = "1";
process.env.HOME = fakeHome;
Expand Down Expand Up @@ -616,7 +613,6 @@ test("runCodex ephemeral HOME: opencode stages custom XDG auth roots into dispos
const previous = Object.fromEntries(
[
"OPENCODE_BIN",
"NEEDLEFISH_ALLOW_OPENCODE_RUNNER",
"NEEDLEFISH_EPHEMERAL_HOME",
"NEEDLEFISH_NO_RETRY",
"HOME",
Expand Down Expand Up @@ -651,7 +647,6 @@ test("runCodex ephemeral HOME: opencode stages custom XDG auth roots into dispos
);
chmodSync(bin, 0o755);
process.env.OPENCODE_BIN = bin;
process.env.NEEDLEFISH_ALLOW_OPENCODE_RUNNER = "1";
process.env.NEEDLEFISH_EPHEMERAL_HOME = "1";
process.env.NEEDLEFISH_NO_RETRY = "1";
process.env.HOME = fakeHome;
Expand Down
Loading
Loading