diff --git a/.github/workflows/biggiepockets-review.yml b/.github/workflows/biggiepockets-review.yml index f6fc533..4fead79 100644 --- a/.github/workflows/biggiepockets-review.yml +++ b/.github/workflows/biggiepockets-review.yml @@ -1,6 +1,6 @@ name: BiggiePockets Code Review (reusable) -# Reusable two-stage AI review (Codex first pass, then Claude verifies and +# Reusable two-stage AI review (Codex first pass, then pi verifies and # synthesizes) anchored on the PR's JIRA ticket, with the BiggiePockets service # account submitting the resulting approve / request-changes decision. # @@ -27,9 +27,9 @@ name: BiggiePockets Code Review (reusable) # # Prompt A/B test (BIG-21717): prompts/registry.json declares the arms, the control # arm, and what share of pull requests an experiment arm gets. Each review runs ONE -# Claude pass, using the arm that scripts/resolve-prompts.sh assigns to that PR by +# Stage-2 pass, using the arm that scripts/resolve-prompts.sh assigns to that PR by # hashing ':' — so the arms are compared BETWEEN pull requests and a review -# never costs a second Claude pass. The assigned arm writes both the posted summary +# never costs a second Stage-2 pass. The assigned arm writes both the posted summary # and the approve / request-changes decision, which means an experiment prompt does # affect real outcomes on its share of PRs. Assignment is deterministic, so a re-run # reviews with the same arm. Set experiment_split_percent to 0 in registry.json to @@ -65,7 +65,6 @@ jobs: permissions: contents: read pull-requests: read - id-token: write # claude-code-action authenticates via OIDC env: PR: ${{ inputs.pr }} # Explicit model per stage, threaded into both the actual invocation below and the @@ -78,7 +77,7 @@ jobs: # one OpenRouter accepts. CODEX_MODEL: "openai/gpt-5.6-sol" CODEX_RESPONSES_ENDPOINT: "https://openrouter.ai/api/v1/responses" - CLAUDE_MODEL: "deepseek/deepseek-v4.1-flash" + PI_MODEL: "deepseek/deepseek-v4.1-flash" steps: - name: Checkout PR head uses: actions/checkout@v4 @@ -281,19 +280,18 @@ jobs: retention-days: 7 overwrite: true - # A failed/rate-limited Claude run can be retried with "Re-run failed jobs". The + # A failed/rate-limited Stage 2 run can be retried with "Re-run failed jobs". The # successful Codex job is not re-executed; this job downloads its immutable handoff. - claude: + pi: needs: codex runs-on: ubuntu-latest permissions: contents: read pull-requests: read - id-token: write # claude-code-action authenticates via OIDC env: PR: ${{ inputs.pr }} CODEX_MODEL: "openai/gpt-5.6-sol" - CLAUDE_MODEL: "deepseek/deepseek-v4.1-flash" + PI_MODEL: "deepseek/deepseek-v4.1-flash" steps: - name: Checkout reviewed PR head uses: actions/checkout@v4 @@ -327,45 +325,77 @@ jobs: REGISTRY_DIR: registry run: registry/scripts/resolve-prompts.sh - # Stage 2: Claude verifies Codex's findings, reviews independently, and decides. - # This is the only Claude pass, running whichever arm's prompt was assigned to - # this PR. Its verdict is the review. - - name: Record Claude start time - run: echo "CLAUDE_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + # Stage 2: pi verifies Codex's findings, reviews independently, and decides. + # This is the only Stage-2 pass, running whichever arm's prompt was assigned to + # this PR. Its verdict is the review. pi is the @earendil-works/pi-coding-agent + # CLI (a plain npm package — no GitHub app, no OIDC), authenticated to OpenRouter + # with the same OPENROUTER_API_KEY the Codex stage uses. + - name: Install pi + run: | + npm install -g "@earendil-works/pi-coding-agent@0.84.3" + echo "$(npm config get prefix)/bin" >> "$GITHUB_PATH" + + # pi resolves its models from a config dir (PI_CODING_AGENT_DIR). Point it at a + # writable scratch dir seeded with the pinned OpenRouter provider from + # scripts/pi/models.json: the pinned model + rates are what make a FRESH runner + # deterministic — the model is absent from the catalog pi ships, and the live + # catalog refresh is a background fetch, not a startup step we can rely on. + - name: Configure pi + run: | + mkdir -p "$RUNNER_TEMP/pi-config" + cp registry/scripts/pi/models.json "$RUNNER_TEMP/pi-config/models.json" + echo "PI_CODING_AGENT_DIR=$RUNNER_TEMP/pi-config" >> "$GITHUB_ENV" - - name: Claude verify and synthesize - id: claude - uses: anthropics/claude-code-action@v1 - with: - # The action requires this input for launch validation. ANTHROPIC_AUTH_TOKEN - # below is what makes Claude Code send the OpenRouter key as a bearer token. - anthropic_api_key: ${{ secrets.OPENROUTER_API_KEY }} - # Allow the `claude` bot to trigger this review. Without this, PRs opened - # by the claude bot are rejected with "Workflow initiated by non-human - # actor: claude (type: Bot). Add bot to allowed_bots list." - allowed_bots: 'claude' - claude_args: ${{ env.CLAUDE_MODEL != '' && format('--allowedTools "Read,Grep,Glob,Bash,Write" --model {0}', env.CLAUDE_MODEL) || '--allowedTools "Read,Grep,Glob,Bash,Write"' }} - prompt: ${{ steps.resolve.outputs.arm_prompt }} + - name: Record pi start time + run: echo "PI_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + - name: pi verify and synthesize + id: pi env: - ANTHROPIC_BASE_URL: "https://openrouter.ai/api" - ANTHROPIC_AUTH_TOKEN: ${{ secrets.OPENROUTER_API_KEY }} + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + ARM_PROMPT: ${{ steps.resolve.outputs.arm_prompt }} + run: | + printf '%s' "$ARM_PROMPT" > arm-prompt.txt + # One headless pass in JSON event-stream mode. Tools are allowlisted to the + # same read/grep/glob/bash/write set the Claude stage had; context files, + # skills, and extensions are disabled so nothing in the untrusted PR checkout + # can steer the run. Stderr is captured so a failure carries pi's own error + # text into the diagnostics step without polluting the JSONL that + # llm-usage.py parses. + timeout 1500 pi --mode json \ + --model "$PI_MODEL" \ + --tools read,grep,glob,bash,write \ + --no-context-files --no-skills --no-extensions \ + --no-themes --no-prompt-templates \ + @arm-prompt.txt \ + > pi-output.jsonl 2> pi-error.log || pi_exit=$? + pi_exit="${pi_exit:-0}" + + echo "PI_OUTPUT_FILE=$GITHUB_WORKSPACE/pi-output.jsonl" >> "$GITHUB_ENV" + echo "PI_EXIT=$pi_exit" >> "$GITHUB_ENV" + # pi may exit 0 on a retried-but-still-failed session (e.g. an upstream 429 + # that exhausted its retries), so the verdict file is the contract. Fail the + # step so "Re-run failed jobs" retries just this pass. + if [ ! -f verdict.json ]; then + echo "::error::pi review run produced no verdict.json (pi exit $pi_exit)" >&2 + tail -60 pi-error.log >&2 || true + exit 1 + fi - name: Capture review failure diagnostics - if: ${{ always() && steps.claude.outcome == 'failure' }} + if: ${{ always() && steps.pi.outcome == 'failure' }} env: - EXECUTION_FILE: ${{ steps.claude.outputs.execution_file }} DIAGNOSTIC_TOKEN_PROVIDER: ${{ secrets.OPENROUTER_API_KEY }} - DIAGNOSTIC_TOKEN_APP: ${{ steps.claude.outputs.github_token }} DIAGNOSTIC_TOKEN_GITHUB: ${{ github.token }} DIAGNOSTIC_TOKEN_REVIEW: ${{ secrets.BIGGIEPOCKETS_PAT }} run: | - # The action may fail before exposing its execution_file output. - execution_file="${EXECUTION_FILE:-$RUNNER_TEMP/claude-execution-output.json}" + execution_file="${PI_OUTPUT_FILE:-$GITHUB_WORKSPACE/pi-output.jsonl}" python3 registry/scripts/review-diagnostics.py "$execution_file" \ - "$RUNNER_TEMP/review-diagnostics/diagnostics.json" + "$RUNNER_TEMP/review-diagnostics/diagnostics.json" \ + "$GITHUB_WORKSPACE/pi-error.log" - name: Upload review failure diagnostics - if: ${{ always() && steps.claude.outcome == 'failure' }} + if: ${{ always() && steps.pi.outcome == 'failure' }} uses: actions/upload-artifact@v4 with: name: review-failure-${{ github.run_id }}-${{ github.run_attempt }} @@ -373,9 +403,9 @@ jobs: retention-days: 7 if-no-files-found: warn - - name: Record Claude end time + - name: Record pi end time if: always() - run: echo "CLAUDE_END_NS=$(date +%s%N)" >> "$GITHUB_ENV" + run: echo "PI_END_NS=$(date +%s%N)" >> "$GITHUB_ENV" # Submit the review under the arm this PR was assigned. The arm is not disclosed # in the comment: a reviewer who knows which prompt wrote a summary can't rate it @@ -389,7 +419,7 @@ jobs: ASSIGNED_ARM: ${{ steps.resolve.outputs.assigned_arm }} run: | if [ ! -f verdict.json ]; then - echo "::error::Claude review run did not produce verdict.json" >&2 + echo "::error::pi review run did not produce verdict.json" >&2 exit 1 fi @@ -420,7 +450,7 @@ jobs: # `arm`/`prompt_name`/`prompt_version` are payload-level tags, which is where a tag # key resolves to one value — one arm per review, so one value each. # - # root biggiepockets.review -> codex.review, claude.synthesize + # root biggiepockets.review -> codex.review, pi.synthesize # # Compare arms by grouping on `arm` across pull requests (verdict rate, latency, # tokens). @@ -433,7 +463,6 @@ jobs: # org identifier is committed here; both fall back to public defaults. DD_SITE: ${{ vars.DD_SITE }} DD_LLMOBS_ML_APP: ${{ vars.DD_LLMOBS_ML_APP }} - EXECUTION_FILE: ${{ steps.claude.outputs.execution_file }} CODEX_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.codex_prompt_template }} ARM_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.arm_prompt_template }} run: | @@ -446,8 +475,8 @@ jobs: review_start_ns="${REVIEW_START_NS:-$now_ns}" codex_start_ns="${CODEX_START_NS:-$review_start_ns}" codex_end_ns="${CODEX_END_NS:-$codex_start_ns}" - claude_start_ns="${CLAUDE_START_NS:-$codex_end_ns}" - claude_end_ns="${CLAUDE_END_NS:-$claude_start_ns}" + pi_start_ns="${PI_START_NS:-$codex_end_ns}" + pi_end_ns="${PI_END_NS:-$pi_start_ns}" verdict="${VERDICT:-unknown}" codex_prompt_name="${{ steps.resolve.outputs.codex_prompt_name }}" @@ -470,8 +499,8 @@ jobs: codex_findings=$(cat codex-findings.md 2>/dev/null || true) codex_findings_lines=$(wc -l < codex-findings.md 2>/dev/null || echo 0) - claude_status="ok" - [ "${{ steps.claude.outcome }}" != "success" ] && claude_status="error" + pi_status="ok" + [ "${{ steps.pi.outcome }}" != "success" ] && pi_status="error" root_status="ok" [ "${{ steps.submit.outcome }}" != "success" ] && root_status="error" @@ -522,7 +551,7 @@ jobs: arm_prompt_obj=$(prompt_json "$arm_prompt_name" "$arm_prompt_version" "$ARM_PROMPT_TEMPLATE") codex_model_tag="${CODEX_MODEL:-unspecified}" - claude_model_tag="${CLAUDE_MODEL:-unspecified}" + pi_model_tag="${PI_MODEL:-unspecified}" dd_site="${DD_SITE:-datadoghq.com}" ml_app="${DD_LLMOBS_ML_APP:-biggiepockets-review}" @@ -530,52 +559,56 @@ jobs: # Cost tracking. Each pass carries its own usage as span metrics plus a # model identity split into the bare name and originating provider that # Datadog's pricing catalog keys on ("openai/gpt-5.6-sol" -> gpt-5.6-sol + - # openai); the OpenRouter gateway is kept as a tag instead. Datadog then - # prices a catalogued model from its token counts, and takes the reported - # `total_cost` at face value for one it doesn't carry. Both numbers come - # from what the tools recorded — OpenRouter reports what it charged, and - # llm-usage.py holds no rate table of its own. + # openai; "deepseek/deepseek-v4.1-flash" -> deepseek-v4.1-flash + deepseek); + # the OpenRouter gateway is kept as a tag instead. Datadog then prices a + # catalogued model from its token counts, and takes the reported `total_cost` + # at face value for one it doesn't carry. # - # Only usage objects are read, never message content. The Codex pass's - # counters travel from its own job in CODEX_USAGE_JSON, since the action - # exposes no usage output and its rollout stays on that runner. + # Both numbers come from what the tools recorded. pi records a usage object + # (tokens + a cost.total computed from the model's OpenRouter list rates on + # the runner, pinned in scripts/pi/models.json) on every pass — that is the + # same rate table OpenRouter bills against, so cost.total is the amount the + # pass is charged. The Codex pass's counters travel from its own job in + # CODEX_USAGE_JSON, since the action exposes no usage output and its rollout + # stays on that runner. + # Only usage objects are read, never message content. # Fall back to an empty object if the helper can't run at all, so a cost # problem can't cost us the quality metrics in the same payload. no_fields='{"model_name":"unspecified","model_provider":"unspecified","metrics":{}}' codex_fields="${CODEX_USAGE_JSON:-}" - claude_fields=$(python3 registry/scripts/llm-usage.py claude "$claude_model_tag" "${EXECUTION_FILE:-}") || claude_fields="$no_fields" + pi_fields=$(python3 registry/scripts/llm-usage.py pi "$pi_model_tag" "${PI_OUTPUT_FILE:-}") || pi_fields="$no_fields" usable='has("model_name") and has("model_provider") and has("metrics")' echo "$codex_fields" | jq -e "$usable" >/dev/null 2>&1 || codex_fields="$no_fields" - echo "$claude_fields" | jq -e "$usable" >/dev/null 2>&1 || claude_fields="$no_fields" + echo "$pi_fields" | jq -e "$usable" >/dev/null 2>&1 || pi_fields="$no_fields" run_id="$GITHUB_REPOSITORY-pr$PR-${{ github.run_id }}" trace_id=$(openssl rand -hex 16) root_span_id=$(openssl rand -hex 8) codex_span_id=$(openssl rand -hex 8) - claude_span_id=$(openssl rand -hex 8) + pi_span_id=$(openssl rand -hex 8) spans=$(jq -n \ --arg trace_id "$trace_id" \ --arg root_span_id "$root_span_id" \ --arg codex_span_id "$codex_span_id" \ - --arg claude_span_id "$claude_span_id" \ + --arg pi_span_id "$pi_span_id" \ --arg repo "$GITHUB_REPOSITORY" \ --arg pr "$PR" \ --arg base_ref "${BASE_REF:-unknown}" \ --arg codex_model "$codex_model_tag" \ - --arg claude_model "$claude_model_tag" \ + --arg pi_model "$pi_model_tag" \ --argjson review_start_ns "$review_start_ns" \ --argjson root_duration "$((now_ns - review_start_ns))" \ --argjson codex_start_ns "$codex_start_ns" \ --argjson codex_duration "$((codex_end_ns - codex_start_ns))" \ - --argjson claude_start_ns "$claude_start_ns" \ - --argjson claude_duration "$((claude_end_ns - claude_start_ns))" \ + --argjson pi_start_ns "$pi_start_ns" \ + --argjson pi_duration "$((pi_end_ns - pi_start_ns))" \ --arg codex_status "$codex_status" \ - --arg claude_status "$claude_status" \ + --arg pi_status "$pi_status" \ --arg root_status "$root_status" \ --argjson codex_fields "$codex_fields" \ - --argjson claude_fields "$claude_fields" \ + --argjson pi_fields "$pi_fields" \ --arg codex_findings_excerpt "$codex_findings_excerpt" \ --arg summary_excerpt "$summary_excerpt" \ --argjson codex_prompt "$codex_prompt_obj" \ @@ -618,16 +651,16 @@ jobs: { parent_id: $root_span_id, trace_id: $trace_id, - span_id: $claude_span_id, - name: "claude.synthesize", - start_ns: $claude_start_ns, - duration: $claude_duration, - status: $claude_status, - metrics: $claude_fields.metrics, + span_id: $pi_span_id, + name: "pi.synthesize", + start_ns: $pi_start_ns, + duration: $pi_duration, + status: $pi_status, + metrics: $pi_fields.metrics, meta: { kind: "llm", - model_name: $claude_fields.model_name, - model_provider: $claude_fields.model_provider, + model_name: $pi_fields.model_name, + model_provider: $pi_fields.model_provider, input: { messages: [{role: "user", content: $codex_findings_excerpt}], prompt: $arm_prompt @@ -651,7 +684,7 @@ jobs: --arg assignment_bucket "$assignment_bucket" \ --arg split_percent "$split_percent" \ --arg codex_model "$codex_model_tag" \ - --arg claude_model "$claude_model_tag" \ + --arg pi_model "$pi_model_tag" \ --arg codex_prompt_name "$codex_prompt_name" \ --arg codex_prompt_version "$codex_prompt_version" \ --argjson codex_findings_lines "$codex_findings_lines" \ @@ -675,7 +708,7 @@ jobs: "experiment_split_percent:\($split_percent)", "gateway:openrouter", "codex_model:\($codex_model)", - "claude_model:\($claude_model)", + "pi_model:\($pi_model)", "codex_findings_lines:\($codex_findings_lines)", "codex_prompt_name:\($codex_prompt_name)", "codex_prompt_version:\($codex_prompt_version)" diff --git a/README.md b/README.md index f0db58f..061537d 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Org-wide GitHub defaults and shared reusable workflows. two-stage AI code review on a pull request: 1. **Codex first pass** — reviews the diff against the PR's JIRA ticket and writes findings. -2. **Claude verify & synthesize** — validates Codex's findings, reviews the diff +2. **pi verify & synthesize** — validates Codex's findings, reviews the diff independently (grepping for callers/tests, factoring in the existing PR discussion), checks the change against the ticket's acceptance criteria, and decides a single verdict. @@ -17,10 +17,10 @@ The **BiggiePockets** service account then submits the resulting `approve` / ticket can't be fetched), the review degrades gracefully to a diff-based review instead of failing. -Codex and Claude run as separate GitHub Actions jobs. Codex uploads the reviewed commit's -diff, ticket/discussion context, and findings as a short-lived artifact; Claude downloads -that immutable handoff. If Claude is rate-limited, use **Re-run failed jobs** on the workflow -run. GitHub reruns only the Claude job, reusing the completed Codex pass instead of invoking +Codex and pi run as separate GitHub Actions jobs. Codex uploads the reviewed commit's +diff, ticket/discussion context, and findings as a short-lived artifact; pi downloads +that immutable handoff. If the pi pass is rate-limited, use **Re-run failed jobs** on the workflow +run. GitHub reruns only the pi job, reusing the completed Codex pass instead of invoking Codex again. The review logic lives centrally in this repo. Each consuming repo only adds a thin @@ -30,13 +30,13 @@ The review logic lives centrally in this repo. Each consuming repo only adds a t Do this once per repo you want BiggiePockets to review. -#### 1. Install the Claude GitHub app +#### 1. Install nothing -The Claude verification stage uses [`anthropics/claude-code-action`](https://github.com/anthropics/claude-code-action). -Install the official [Claude GitHub app](https://github.com/apps/claude) for the organization -once; you do **not** need to reinstall it per repo. Claude's model requests authenticate -through OpenRouter using the shared `OPENROUTER_API_KEY` described below, so no personal -Claude Code OAuth token is required. +The Stage-2 verification stage runs [`@earendil-works/pi-coding-agent`](https://www.npmjs.com/package/@earendil-works/pi-coding-agent), +a plain npm CLI that the workflow installs onto the runner itself (`npm install -g`). +There is no GitHub app, no OIDC, and no per-repo installation. pi authenticates to +OpenRouter with the shared `OPENROUTER_API_KEY` described below — no OAuth login or +personal token is required. #### 2. Add the caller workflow @@ -57,12 +57,11 @@ on: required: true type: string -# The reusable workflow's jobs need these scopes. Declare them explicitly so the +# The reusable workflow's jobs need read scopes. Declare them explicitly so the # caller works regardless of the repo's default token permissions. permissions: contents: read pull-requests: read - id-token: write jobs: review: @@ -83,7 +82,7 @@ jobs: #### 3. Make the secrets available The reusable workflow consumes several secrets via `secrets: inherit`: credentials for the -the AI review provider (`OPENROUTER_API_KEY`, shared by both the Codex and Claude stages), +the AI review provider (`OPENROUTER_API_KEY`, shared by both the Codex and pi stages), an Atlassian email + API token to fetch the PR's JIRA ticket for intent, and a personal access token for the BiggiePockets service account that submits the review. Configure them as **organization secrets** (recommended — set once, available to every repo) or as per-repo @@ -92,8 +91,8 @@ secrets if you prefer to scope them. It also reports per-review traces to the `biggiepockets-review` app in Datadog LLM Observability via `secrets.DATADOG_API_KEY`: verdict, timing, prompt template and version (tracked as prompts, see below), the model each -stage ran (`CODEX_MODEL`/`CLAUDE_MODEL` env vars in the workflow — both are OpenRouter -model slugs and must be set), and the actual findings text from Codex and the summary Claude wrote, +stage ran (`CODEX_MODEL`/`PI_MODEL` env vars in the workflow — both are OpenRouter +model slugs and must be set), and the actual findings text from Codex and the summary pi wrote, so review quality is inspectable, not just counted. This secret is optional — reviews still run and post normally without it, but no metrics are reported. @@ -114,21 +113,25 @@ OpenRouter, whose slugs look like `openai/gpt-5.6-sol`, so `scripts/llm-usage.py splits the slug into `model_name: gpt-5.6-sol` / `model_provider: openai` and records the routing as a `gateway:openrouter` tag. -Cost itself is never computed here, and there is no rate table in the repository: a -list price committed to a file goes stale silently and would be reported with the same -confidence as a real one. Instead each span carries whichever of the two things -Datadog needs. For a model in the catalog, token counts are enough. For one it does -not carry, the span reports a `total_cost` metric — the amount OpenRouter says it -charged, taken from the `cost` field it returns on every response, where that figure -survives into what the tool wrote to disk. - -The two passes record their usage differently. Claude Code writes a `usage` object to -its execution output. Codex writes running token counters to a session rollout on its -own runner, and since its action exposes no usage output, the workflow reads that -rollout in the Codex job and hands the totals to the reporting job. In both cases only -usage objects are read — never message content, transcripts, prompts, or diffs. Claude -Code's own `total_cost_usd` is ignored: it is computed against Anthropic's list prices, -while these passes are billed by OpenRouter for a non-Anthropic model. +Cost is never computed in the reporting script: `scripts/llm-usage.py` holds no rate +table. Instead each span carries whichever of the two things Datadog needs. For a model +in the catalog, token counts are enough. For one it does not carry, the span reports a +`total_cost` metric taken at face value. + +The two passes record their usage differently. pi runs in `--mode json` and writes a +JSON event stream; every assistant message carries a usage object with token counts and +`cost.total` — a price computed by pi from the model's OpenRouter list rates, the same +rates OpenRouter bills against, so it is the amount the pass is charged. The Stage-2 +model and its rates are pinned in `scripts/pi/models.json` (the catalog pi ships +predates the model, and the live catalog refresh is a background fetch, not a startup +step — a committed pin is what makes a fresh runner deterministic); if you roll the +Stage-2 model, update that file in the same commit. Codex writes running token counters +to a session rollout on its own runner, and since its action exposes no usage output, +the workflow reads that rollout in the Codex job and hands the totals to the reporting +job. In both cases only usage objects are read — never message content, transcripts, +prompts, or diffs. (The Claude Code harness that previously ran Stage 2 translated +usage into Anthropic's schema, so OpenRouter's reported cost regularly did not survive +to the span — the pi pass records usage itself and is what this repo now trusts.) Two **organization-level variables** (`vars`, not secrets — **Settings → Secrets and variables → Actions → Variables** at the org level) configure where the trace lands. @@ -141,22 +144,18 @@ Both are optional, and nothing here is committed to the repository: Defaults to `biggiepockets-review`. Cost tracking is best-effort and never fails a review. With no `DATADOG_API_KEY` the -whole reporting step is skipped, and a missing execution file, an absent rollout, or +whole reporting step is skipped, and a missing event stream, an absent rollout, or malformed usage data degrades to fewer metrics on the span. #### 4. Set workflow permissions -The reusable workflow's jobs need `pull-requests: read` and `id-token: write` (OIDC -authentication for `claude-code-action`). The caller YAML declares these in its -`permissions` block, but GitHub caps those declarations at whatever the repo's default -token setting allows. Repos set to **"Read repository contents"** (the restrictive -default) deny `id-token: write` regardless of what the YAML says — the workflow fails -with `startup_failure` before any job runs. - -Go to **Settings → Actions → General → Workflow permissions** on the target repo and set: - -- **"Read and write permissions"** -- **"Allow GitHub Actions to create and approve pull requests"** (checked) +With the move off `claude-code-action`, the review no longer needs OIDC (`id-token`), +so the default **"Read repository contents"** setting is fine — repos that previously +had to loosen their workflow permissions for the review can leave them restrictive. +The reusable workflow's jobs declare `contents: read` and `pull-requests: read` only; +the review itself is submitted with the `BIGGIEPOCKETS_PAT` secret, not the repo's +`GITHUB_TOKEN`. Callers that already declare `id-token: write` in their caller file can +remove it, but leaving it is harmless. #### 5. Give BiggiePockets access @@ -190,16 +189,16 @@ prompts/ ``` - **Templates + shared blocks.** Each prompt references the shared rule blocks via - `{{@prompts/_shared/.md}}`, so the Codex and Claude prompts can never drift out of + `{{@prompts/_shared/.md}}`, so the Codex and Stage-2 prompts can never drift out of sync. Prompts resolve `{{PR}}`, `{{PROMPT_NAME}}`, `{{PROMPT_VERSION}}` too. - **Content-derived versions.** `prompt_version` is a content hash of the template plus the shared blocks it includes — it changes only when that prompt's text changes, not per PR or per arm, so Datadog LLM Obs can attribute quality to the exact prompt text that ran. -- **One arm per pull request.** Two Claude prompts sit in the registry — `control` and +- **One arm per pull request.** Two Stage-2 prompts sit in the registry — `control` and `thesis-first` — and each review runs exactly one of them. `scripts/resolve-prompts.sh` hashes `:` into a bucket 0-99 and assigns the PR to the experiment arm when that bucket falls under `experiment_split_percent` (50 today), so the arms split traffic - evenly and every review costs a single Claude pass. Assignment is a pure function of + evenly and every review costs a single Stage-2 pass. Assignment is a pure function of repo and PR number: re-running a review reuses the same arm, and one PR never sees two review styles. - **The assigned arm decides.** Whichever arm a PR draws writes the posted summary *and* @@ -223,7 +222,7 @@ prompts/ - **Datadog.** Each review is one trace, tagged with the arm that ran it: ``` - biggiepockets.review → codex.review, claude.synthesize + biggiepockets.review → codex.review, pi.synthesize ``` A tag key resolves to one value per submitted payload, so an `arm` tag is only diff --git a/prompts/registry.json b/prompts/registry.json index 4424aef..710bc29 100644 --- a/prompts/registry.json +++ b/prompts/registry.json @@ -1,7 +1,7 @@ { "//": "Prompt registry config for the BiggiePockets review workflow.", "//": "", - "//": "Each review runs ONE Claude synthesize pass. Which arm's prompt it runs is", + "//": "Each review runs ONE Stage-2 synthesize pass (pi). Which arm's prompt it runs is", "//": "decided per pull request: scripts/resolve-prompts.sh hashes ':' into a", "//": "bucket 0-99, and a PR below experiment_split_percent is assigned the experiment", "//": "arm instead of control_arm. The assigned arm writes the summary that gets posted", @@ -14,7 +14,7 @@ "//": "", "//": "Comparison is BETWEEN pull requests: Datadog tags each review with its assigned", "//": "arm, so verdict rates and latency group by arm across PRs. No PR is ever reviewed", - "//": "by two arms, which is what keeps Claude token spend at one pass per review.", + "//": "by two arms, which is what keeps Stage-2 token spend at one pass per review.", "//": "", "//": "prompts/.md are versioned templates; shared rule blocks live in", "//": "prompts/_shared/ and are injected via {{@path}} markers. Prompt versions are", diff --git a/scripts/llm-usage.py b/scripts/llm-usage.py index f4ccce8..3175cc1 100755 --- a/scripts/llm-usage.py +++ b/scripts/llm-usage.py @@ -10,15 +10,21 @@ Where the numbers come from: -- Both passes reach models through OpenRouter, which reports what it charged in the - `cost` field of every response's usage object. Where that figure survives into what - the tool wrote to disk, it is the authoritative cost for the pass and is emitted as - `total_cost` — a real charge, not an estimate. +- Both passes reach models through OpenRouter, which reports what it charged in + the `cost` field of every response's usage object. Where that figure survives into + what the tool wrote to disk, it is the authoritative cost for the pass and is + emitted as `total_cost` — a real charge, not an estimate. - The Codex pass writes token counters to its session rollout. Its model is in the catalog, so those counts are enough for Datadog to price it. +- The pi pass records a usage object on every run (input/output/cacheRead/cacheWrite + plus `cost.total`, a price computed by pi from the model's OpenRouter list rates + pinned in scripts/pi/models.json — the same rates OpenRouter bills against, so + `cost.total` is the amount the pass is charged; verified against a live run + where 17,515 input tokens at $0.019/M priced exactly to what OpenRouter charges). - Claude Code's own `total_cost_usd` is deliberately ignored. It is computed against Anthropic's list prices, and these passes are billed by OpenRouter for a non- - Anthropic model, so it describes a bill nobody was sent. + Anthropic model, so it describes a bill nobody was sent. (The Claude Code harness + itself is no longer used for Stage 2, but the parser stays for historical files.) The workflow names models the way OpenRouter routes them ("openai/gpt-5.6-sol"). Datadog's catalog keys on the bare model and its originating provider, so @@ -30,6 +36,7 @@ Usage: llm-usage.py claude [execution-file] llm-usage.py codex [rollout-dir] + llm-usage.py pi [event-stream-file] Prints a JSON object on stdout for the workflow's jq to splice into a span: {"model_name": ..., "model_provider": ..., "gateway": ..., "metrics": {...}} `metrics` carries only what is actually known; it is `{}` when nothing is. @@ -62,6 +69,16 @@ "cached_input_tokens": "cache_read_input_tokens", } +# pi's normalized usage object, as written to its JSON event stream (--mode json), +# mapped to Datadog's metric names. `cost.total` (computed by pi from the model's +# OpenRouter list rates) is handled separately below. +PI_USAGE_FIELDS = { + "input": "input_tokens", + "output": "output_tokens", + "cacheRead": "cache_read_input_tokens", + "cacheWrite": "cache_write_input_tokens", +} + def split_model(slug): """"openai/gpt-5.6-sol" -> ("gpt-5.6-sol", "openai"). A slug with no provider @@ -189,6 +206,53 @@ def codex_usage(directory): return (collect(totals, CODEX_USAGE_FIELDS), openrouter_cost(totals)) +def pi_cost(usage): + """The amount pi computed for the pass from the model's OpenRouter list rates + (usage.cost.total), or None when it is absent, non-numeric, or zero. Zero is + treated as "nothing reported" — a free call or an absent rate table both + report no cost, matching the claude/codex conventions.""" + if not isinstance(usage, dict): + return None + cost = usage.get("cost") + if not isinstance(cost, dict): + return None + total = read_number(cost.get("total")) + if total is None or total <= 0: + return None + return total + + +def pi_usage(path): + """pi's JSON event stream (--mode json) -> (token counts, reported cost or None). + Every assistant message carries the cumulative usage of the turn so far; the last + one that finished cleanly is the whole pass. Events of interest are + message_end/turn_end (message under `message`) and agent_end (messages array).""" + candidates = [] + for event in read_messages(path): + message = event.get("message") + if event.get("type") == "agent_end": + messages = event.get("messages") + if isinstance(messages, list) and messages: + message = messages[-1] + if not isinstance(message, dict) or message.get("role") != "assistant": + continue + if not isinstance(message.get("usage"), dict): + continue + candidates.append(message) + if not candidates: + return ({}, None) + # Prefer the last message that finished cleanly; fall back to the last one with + # any usage; last resort is the last assistant message overall. + chosen = next( + (m for m in reversed(candidates) if m.get("stopReason") == "stop"), + next( + (m for m in reversed(candidates) if sum( + read_number(m["usage"].get(k)) or 0 for k in ("input", "output")) > 0), + candidates[-1])) + usage = chosen.get("usage") or {} + return (collect(usage, PI_USAGE_FIELDS), pi_cost(usage)) + + def build_span_fields(slug, counts, cost): """Pure assembly of the span fields the workflow splices in: a catalog-matching model identity, whichever token counts are known, and a reported cost when there @@ -216,6 +280,8 @@ def main(argv): counts, cost = codex_usage(source or DEFAULT_ROLLOUT_DIR) elif pass_name == "claude": counts, cost = claude_usage(source) + elif pass_name == "pi": + counts, cost = pi_usage(source) else: counts, cost = ({}, None) print(json.dumps(build_span_fields(slug, counts, cost))) diff --git a/scripts/pi/models.json b/scripts/pi/models.json new file mode 100644 index 0000000..5f8796c --- /dev/null +++ b/scripts/pi/models.json @@ -0,0 +1,38 @@ +{ + "//": "pi config for the Stage 2 review pass (BiggiePockets/.github).", + "//": "", + "//": "pi resolves models from the config dir pointed at by PI_CODING_AGENT_DIR. This file", + "//": "declares the OpenRouter provider and the single pinned Stage 2 model with its", + "//": "OpenRouter list rates, so a FRESH CI runner (no cached models-store.json) resolves", + "//": "the model deterministically. The model is absent from the catalog pi ships and the", + "//": "live catalog refresh is a background fetch, not a startup step we can rely on.", + "//": "", + "//": "Rates: $/1M tokens as published by OpenRouter for deepseek/deepseek-v4.1-flash at", + "//": "the time of pinning (input 0.15, output 0.6, input cache read 0.003; no cache write", + "//": "rate). pi computes cost.total from these, so it is exactly what OpenRouter bills for", + "//": "standard usage. If you roll the model, update the id AND these rates together.", + "//": "", + "//": "The apiKey interpolation reads OPENROUTER_API_KEY from the workflow environment", + "//": "(the same org secret the Codex stage uses) — no interactive login or OAuth is", + "//": "involved. api/api identify the OpenAI-compatible endpoint pi uses.", + "providers": { + "openrouter": { + "baseUrl": "https://openrouter.ai/api/v1", + "apiKey": "$OPENROUTER_API_KEY", + "api": "openai-completions", + "models": [ + { + "id": "deepseek/deepseek-v4.1-flash", + "name": "DeepSeek V4.1 Flash", + "reasoning": true, + "cost": { + "input": 0.15, + "output": 0.6, + "cacheRead": 0.003, + "cacheWrite": 0 + } + } + ] + } + } +} \ No newline at end of file diff --git a/scripts/resolve-prompts.sh b/scripts/resolve-prompts.sh index b41ecfc..24aeb6f 100755 --- a/scripts/resolve-prompts.sh +++ b/scripts/resolve-prompts.sh @@ -6,7 +6,7 @@ # experiment arm gets. This script picks ONE arm per review — hash(':') # mod 100, below experiment_split_percent means the experiment arm — and emits only # that arm's prompt. The assigned arm both posts its summary and decides the review, -# so arms are compared BETWEEN pull requests and each review costs one Claude pass. +# so arms are compared BETWEEN pull requests and each review costs one Stage-2 pass. # # Shared rule blocks (privacy, migration-data, perf) are stored once in # prompts/_shared/ and injected into every prompt that references them via {{@path}} diff --git a/scripts/review-diagnostics.py b/scripts/review-diagnostics.py index f9e2be2..fe74ba9 100644 --- a/scripts/review-diagnostics.py +++ b/scripts/review-diagnostics.py @@ -1,5 +1,20 @@ #!/usr/bin/env python3 -"""Save terminal review errors without publishing tool transcripts.""" +"""Save terminal review errors without publishing tool transcripts. + +Understands the two Stage-2 harness output formats that have existed: + +- pi's JSON event stream (`pi --mode json`): the terminal assistant message, its + stop reason / error, and pass usage are extracted from the stream. Tool calls, + their arguments and results are never included. +- Claude Code's execution file (historical): terminal "result" messages only. + +Also appends a redacted tail of pi's stderr (passed as an optional third +argument) so upstream errors like rate limits show up in diagnostics. + +Everything that goes into the report is passed through redact(), which blanks +known secrets (the provider key, the review PAT, the GITHUB_TOKEN) anywhere they +appear. +""" import html import json import os @@ -9,22 +24,105 @@ FIELDS = ('type', 'subtype', 'is_error', 'result', 'errors', 'duration_ms', 'num_turns', 'total_cost_usd', 'session_id') +# pi --mode json emits these event types on the wire. +PI_EVENT_TYPES = {'session', 'agent_start', 'agent_end', 'turn_start', 'turn_end', + 'message_start', 'message_end', 'message_update'} + + +def read_events(text): + """Parse a file that is a JSON array, a single JSON object, or JSON-lines into a + list of dicts. Empty on anything unreadable.""" + try: + parsed = json.loads(text) + except json.JSONDecodeError: + parsed = [] + for line in text.splitlines(): + if not line.strip(): + continue + try: + parsed.append(json.loads(line)) + except json.JSONDecodeError: + continue + if isinstance(parsed, dict): + parsed = [parsed] + if not isinstance(parsed, list): + return [] + return [item for item in parsed if isinstance(item, dict)] + + +def is_pi_stream(events): + """A pi event stream announces itself with wire event types.""" + return any(event.get('type') in PI_EVENT_TYPES for event in events) + + +def pi_result(events): + """The terminal assistant message from a pi event stream, stripped of tools: + stop reason, error, model, usage, turns, and (if timestamps survive) duration. + Tool calls, arguments, and results are never read.""" + candidates = [] + for event in events: + message = event.get('message') + if event.get('type') == 'agent_end': + messages = event.get('messages') + if isinstance(messages, list) and messages: + message = messages[-1] + if not isinstance(message, dict) or message.get('role') != 'assistant': + continue + candidates.append(message) + if not candidates: + return None + message = candidates[-1] + text = ' '.join(part.get('text', '') + for part in message.get('content', []) + if isinstance(part, dict) and part.get('type') == 'text') + is_error = message.get('stopReason') == 'error' + result = { + 'subtype': message.get('stopReason'), + 'is_error': is_error, + 'num_turns': sum(1 for e in events if e.get('type') == 'turn_start'), + 'model': message.get('model'), + 'provider': message.get('provider'), + } + usage = message.get('usage') + if isinstance(usage, dict): + cost = usage.get('cost') + if isinstance(cost, dict): + total = cost.get('total') + if isinstance(total, (int, float)) and not isinstance(total, bool): + result['total_cost'] = total + result['usage'] = {key: usage[key] for key in + ('input', 'output', 'cacheRead', 'cacheWrite', 'totalTokens') + if key in usage} + if is_error: + result['errors'] = ([message.get('errorMessage')] + if message.get('errorMessage') else []) + result['result'] = message.get('errorMessage') or '' + else: + result['result'] = text + stamps = [ts for event in events + if isinstance(ts := event.get('timestamp'), (int, float))] + for message in candidates: + if isinstance(ts := message.get('timestamp'), (int, float)): + stamps.append(ts) + if stamps: + result['duration_ms'] = int(max(stamps) - min(stamps)) + return result + def read_result(source): try: text = source.read_text() - try: - messages = json.loads(text) - except json.JSONDecodeError: - messages = [json.loads(line) for line in text.splitlines() if line.strip()] - except (OSError, ValueError): + except OSError: return {'status': 'execution output unavailable'} - if isinstance(messages, dict): - messages = [messages] - if not isinstance(messages, list): + events = read_events(text) + if not events: + return {'status': 'execution output unavailable'} + if is_pi_stream(events): + result = pi_result(events) + if result is not None: + return {'status': 'captured', 'result': result} return {'status': 'result unavailable'} - results = [message for message in messages - if isinstance(message, dict) and message.get('type') == 'result'] + results = [event for event in events if event.get('type') == 'result'] if not results: return {'status': 'result unavailable'} return {'status': 'captured', 'result': { @@ -32,6 +130,17 @@ def read_result(source): }} +def read_stderr_tail(path, max_lines=40): + """The last lines of pi's captured stderr, or None when absent.""" + if not path: + return None + try: + lines = Path(path).read_text(errors='replace').splitlines() + except OSError: + return None + return lines[-max_lines:] or None + + def redact(value, secrets): if isinstance(value, str): for secret in secrets: @@ -45,12 +154,21 @@ def redact(value, secrets): def main(): - source, destination = map(Path, sys.argv[1:]) + args = sys.argv[1:] + if len(args) < 2: + print('usage: review-diagnostics.py [stderr]', + file=sys.stderr) + return 2 + source, destination = map(Path, args[:2]) + stderr_path = Path(args[2]) if len(args) > 2 else None secrets = json.loads(os.environ.get('DIAGNOSTIC_SECRET_VALUES', '[]')) secrets += [value for key, value in os.environ.items() if key.startswith('DIAGNOSTIC_TOKEN_') and value] secrets = sorted(set(filter(None, secrets)), key=len, reverse=True) report = redact(read_result(source), secrets) + tail = read_stderr_tail(stderr_path) + if tail is not None: + report['stderr_tail'] = redact(tail, secrets) output = json.dumps(report, indent=2) destination.parent.mkdir(parents=True, exist_ok=True) destination.write_text(output + '\n') @@ -64,4 +182,4 @@ def main(): if __name__ == '__main__': - main() + sys.exit(main()) \ No newline at end of file diff --git a/tests/test_llm_usage.py b/tests/test_llm_usage.py index 9bd63e4..d434f50 100644 --- a/tests/test_llm_usage.py +++ b/tests/test_llm_usage.py @@ -29,6 +29,29 @@ def token_count(**usage): 'payload': {'type': 'token_count', 'info': {'total_token_usage': usage}}} +def pi_message(stop_reason='stop', usage=None, text='', error=None, timestamp=2000): + message = {'role': 'assistant', 'stopReason': stop_reason, 'timestamp': timestamp} + if usage is not None: + message['usage'] = usage + if error: + message['errorMessage'] = error + elif text: + message['content'] = [{'type': 'text', 'text': text}] + return message + + +def pi_stream(*messages): + """A minimal pi event stream: session, turn_start, one message_end per + assistant message, then turn_end and agent_end carrying all of them (the last + message is the terminal one).""" + events = [{'type': 'session'}, {'type': 'turn_start'}] + events += [{'type': 'message_end', 'message': message} + for message in messages] + events += [{'type': 'turn_end'}, + {'type': 'agent_end', 'messages': list(messages)}] + return events + + class SplitModelTest(unittest.TestCase): def test_splits_openrouter_slug_into_catalog_name_and_provider(self): self.assertEqual(llm_usage.split_model('openai/gpt-5.6-sol'), @@ -139,6 +162,62 @@ def test_missing_session_directory_yields_nothing(self): self.assertEqual(llm_usage.codex_usage(''), ({}, None)) +class PiUsageTest(unittest.TestCase): + def test_reads_counts_and_computed_cost_from_the_last_finished_message(self): + usage = {'input': 17515, 'output': 17, 'cacheRead': 17024, + 'cacheWrite': 0, 'totalTokens': 34556, + 'cost': {'input': 0.000332785, 'output': 5.1e-07, + 'cacheRead': 0.000038966, 'cacheWrite': 0, + 'total': 0.000372255}} + path = write('\n'.join(json.dumps(e) for e in pi_stream(pi_message(usage=usage)))) + counts, cost = llm_usage.pi_usage(path) + self.assertEqual(counts, {'input_tokens': 17515, 'output_tokens': 17, + 'cache_read_input_tokens': 17024, + 'cache_write_input_tokens': 0}) + self.assertEqual(cost, 0.000372255) + + def test_prefers_the_last_clean_message_over_a_failed_retry_after_it(self): + clean = pi_message(stop_reason='stop', text='review done', usage={ + 'input': 500, 'output': 100, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 600, 'cost': {'total': 0.9}}) + failed = pi_message(stop_reason='error', error='429 rate limited', usage={ + 'input': 10, 'output': 0, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 10, 'cost': {'total': 0.0}}) + path = write('\n'.join(json.dumps(e) for e in pi_stream(clean, failed))) + counts, cost = llm_usage.pi_usage(path) + self.assertEqual(counts['input_tokens'], 500) + self.assertEqual(cost, 0.9) + + def test_zero_cost_is_no_reported_cost(self): + usage = {'input': 10, 'output': 2, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 12, 'cost': {'total': 0.0}} + path = write('\n'.join(json.dumps(e) for e in pi_stream(pi_message(usage=usage)))) + _, cost = llm_usage.pi_usage(path) + self.assertIsNone(cost) + + def test_missing_file_yields_nothing(self): + self.assertEqual(llm_usage.pi_usage('/nonexistent/pi-output.jsonl'), ({}, None)) + self.assertEqual(llm_usage.pi_usage(''), ({}, None)) + + def test_stream_without_an_assistant_message_yields_nothing(self): + path = write('\n'.join(json.dumps(e) for e in [ + {'type': 'session'}, {'type': 'turn_start'}, + {'type': 'agent_end', 'messages': []}, + ])) + self.assertEqual(llm_usage.pi_usage(path), ({}, None)) + + def test_agent_end_alone_is_enough_when_no_message_end_events_exist(self): + events = [{'type': 'session'}, + {'type': 'agent_end', 'messages': [ + pi_message(usage={'input': 7, 'output': 3, 'cacheRead': 0, + 'cacheWrite': 0, 'totalTokens': 10, + 'cost': {'total': 0.01}})]}] + path = write('\n'.join(json.dumps(e) for e in events)) + counts, cost = llm_usage.pi_usage(path) + self.assertEqual(counts['input_tokens'], 7) + self.assertEqual(cost, 0.01) + + class BuildSpanFieldsTest(unittest.TestCase): def test_reports_catalog_identity_and_derived_total(self): fields = llm_usage.build_span_fields( diff --git a/tests/test_review_diagnostics.py b/tests/test_review_diagnostics.py index c0490ee..180e54d 100644 --- a/tests/test_review_diagnostics.py +++ b/tests/test_review_diagnostics.py @@ -60,6 +60,71 @@ def test_no_result_does_not_publish_transcript(self): self.assertEqual(report['status'], 'result unavailable') self.assertNotIn('private tool transcript', summary) + def test_pi_stream_captures_terminal_message_without_tool_transcripts(self): + stream = [ + {'type': 'session'}, + {'type': 'turn_start'}, + {'type': 'tool_execution_end', 'toolName': 'write', + 'args': {'file_path': '/tmp/x', 'content': 'private tool transcript'}, + 'result': 'written: private tool transcript'}, + {'type': 'message_end', 'message': {'role': 'assistant', + 'stopReason': 'error', + 'errorMessage': '429 provider error private-test-secret', + 'model': 'deepseek/deepseek-v4.1-flash', + 'provider': 'openrouter', + 'usage': {'input': 5, 'output': 1, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 6, 'cost': {'total': 0.0012}}}}, + {'type': 'turn_end'}, + {'type': 'agent_end', 'messages': [{'role': 'assistant', + 'stopReason': 'error', + 'errorMessage': '429 provider error private-test-secret', + 'model': 'deepseek/deepseek-v4.1-flash', + 'provider': 'openrouter', + 'usage': {'input': 5, 'output': 1, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 6, 'cost': {'total': 0.0012}}}]}, + ] + report, summary = self.run_diagnostics(json.dumps(stream)) + self.assertEqual(report['status'], 'captured') + self.assertTrue(report['result']['is_error']) + self.assertEqual(report['result']['errors'], + ['429 provider error [REDACTED]']) + self.assertEqual(report['result']['num_turns'], 1) + self.assertEqual(report['result']['total_cost'], 0.0012) + self.assertNotIn('private tool transcript', json.dumps(report) + summary) + self.assertNotIn('private-test-secret', json.dumps(report) + summary) + + def test_pi_stream_success_reports_the_terminal_text(self): + stream = [ + {'type': 'agent_end', 'messages': [{'role': 'assistant', + 'stopReason': 'stop', + 'content': [{'type': 'text', 'text': '## Review summary private-test-secret'}]}]}, + ] + report, summary = self.run_diagnostics(json.dumps(stream)) + self.assertEqual(report['result']['is_error'], False) + self.assertIn('Review summary [REDACTED]', report['result']['result']) + + def test_pi_stderr_tail_is_captured_and_redacted(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + source = root / 'pi-output.jsonl' + source.write_text(json.dumps({'type': 'agent_end', 'messages': [ + {'role': 'assistant', 'stopReason': 'error', + 'errorMessage': 'boom'}]})) + destination = root / 'diagnostics.json' + stderr = root / 'pi-error.log' + stderr.write_text('line1\nline2 private-test-secret\n') + summary = root / 'summary.md' + result = subprocess.run( + ['python3', str(SCRIPT), str(source), str(destination), str(stderr)], + env={**os.environ, 'GITHUB_STEP_SUMMARY': str(summary), + 'DIAGNOSTIC_SECRET_VALUES': json.dumps(['private-test-secret'])}, + capture_output=True, text=True, + ) + self.assertEqual(result.returncode, 0, result.stderr) + report = json.loads(destination.read_text()) + self.assertEqual(report['stderr_tail'], ['line1', 'line2 [REDACTED]']) + self.assertNotIn('private-test-secret', stderr.read_text() and '') + if __name__ == '__main__': unittest.main()