Repository navigation
fix: triage inlines PR context into prompt; drops plan mode - #19
Conversation
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes Tier 1 triage reliability by ensuring the model receives the actual PR context (metadata/diff/prior review) inline in the prompt, and by preventing tool-driven “wandering”/context contamination that broke the JSON contract.
Changes:
- Build a self-contained triage prompt file by concatenating
prompts/triage.mdwith inlined PR context inreview-one-pr.sh. - Update
run_triageto remove--permission-mode planand deny tools explicitly for the Claude engine. - Harden triage JSON parsing by stripping Markdown code fences before
jqvalidation and update the triage prompt wording to reflect the new context source.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/review-one-pr.sh | Generates /tmp/cascade/triage-prompt.md with inlined PR context; strips JSON fences before validation. |
| scripts/engine.sh | Adjusts Claude triage invocation to remove plan mode and deny tools. |
| prompts/triage.md | Updates prompt instructions to reference inlined context and reinforce “raw JSON only” output. |
Comments suppressed due to low confidence (1)
scripts/engine.sh:85
- run_triage (copilot engine) passes the entire prompt via a command-line argument: -p "$(cat "$prompt_file")". With PR metadata + up to 3000 diff lines inlined, this can exceed ARG_MAX ("Argument list too long") on large/minified diffs, causing triage to silently fail and default-escalate. Prefer feeding the prompt via stdin or a file-based flag if the copilot CLI supports it, or otherwise enforce a byte-size cap when building the prompt file.
copilot)
copilot \
-p "$(cat "$prompt_file")" \
--model "$ENGINE_TRIAGE_MODEL" \
-s --no-ask-user
;;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export REVIEW_MODE="triage" | ||
| # Build the triage prompt: static template + inlined PR context. | ||
| TRIAGE_PROMPT_FILE="/tmp/cascade/triage-prompt.md" |
There was a problem hiding this comment.
TRIAGE_PROMPT_FILE is a fixed, predictable path in /tmp. That can cause collisions across concurrent runs (multiple PRs / parallel jobs) and is vulnerable to symlink clobbering on shared hosts. Consider using mktemp (e.g., under /tmp/cascade) and ensure the file is removed on exit (trap) after triage completes.
| TRIAGE_PROMPT_FILE="/tmp/cascade/triage-prompt.md" | |
| mkdir -p /tmp/cascade | |
| TRIAGE_PROMPT_FILE="$(mktemp /tmp/cascade/triage-prompt.XXXXXX.md)" | |
| cleanup_triage_prompt_file() { | |
| rm -f "$TRIAGE_PROMPT_FILE" | |
| } | |
| trap cleanup_triage_prompt_file EXIT |
|
|
||
| # Strip ```json ... ``` markdown fences if the model wrapped its JSON in | ||
| # them. Haiku tends to add fences despite explicit instructions not to. | ||
| TRIAGE_RESULT=$(printf '%s' "$TRIAGE_RESULT" | sed -E '/^```[a-zA-Z]*$/d; /^```$/d') |
There was a problem hiding this comment.
The fence-stripping sed only removes lines that are exactly or with no trailing whitespace/CR. If the model outputs fences like "```json " (trailing space) or uses uppercase/language tags with punctuation, jq validation will still fail and force escalation. Consider making the pattern more permissive (trim whitespace, accept optional language token) and/or normalizing CRLF first.
| TRIAGE_RESULT=$(printf '%s' "$TRIAGE_RESULT" | sed -E '/^```[a-zA-Z]*$/d; /^```$/d') | |
| # Normalize CRLF first, then drop fence lines with optional indentation, | |
| # optional language tags, and trailing whitespace. | |
| TRIAGE_RESULT=$( | |
| printf '%s' "$TRIAGE_RESULT" | | |
| tr -d '\r' | | |
| sed -E '/^[[:space:]]*```([^[:space:]]+)?[[:space:]]*$/d' | |
| ) |
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
The triage tier was returning conversational text instead of JSON for every PR, masked since the cascade was introduced (fbec114) by a silent-escalate fallback. Commit 4d2aee3 (on the practical-cannon branch) replaced that fallback with a hard-fail, surfacing the underlying bug. Three things were broken: 1. The prompt referenced `$PR_METADATA`, `$PR_DIFF`, etc., expecting shell interpolation. `claude --print < prompts/triage.md` does no such substitution, so the model literally saw the text `$PR_METADATA` and correctly reported "the env vars aren't set." 2. `--permission-mode plan` told Claude to propose a plan and wait for approval. Under `--print` with no human in the loop, that surfaces as conversational text ("Before I proceed with the plan, I need to understand your intent…") — not JSON. 3. With Read/Glob available in plan mode, the model wandered the cwd, found `prs.txt` (the candidate pool), and assumed the task was about all 31 PRs. Hence the "list of 31 PRs" leakage. Fix: - `scripts/review-one-pr.sh` now builds a self-contained triage prompt at /tmp/cascade/triage-prompt.md that concatenates the static template with the actual PR context (URL, SHA, metadata JSON, diff, prior review body). No more reliance on env-var interpolation that was never happening. - `scripts/engine.sh::run_triage` drops `--permission-mode plan` and explicitly disallows every standard tool. With no tools the model has nothing to do but answer. - `prompts/triage.md` is rewritten to reference the inlined "Pre-fetched PR context" section instead of `$VAR` references, and forbids markdown fences in the output. - `review-one-pr.sh` strips ```json ... ``` fences before JSON validation as belt-and-braces — Haiku still tends to fence its JSON despite the instruction. Verified end-to-end against combsmith/ContentTwin#100 (the PR named in the bug report). Triage now returns `{"escalate": false, "risk": "LOW", ...}` and the cascade proceeds to the single-reviewer Opus confirmation as designed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
The triage tier has been silently broken since the cascade landed (
fbec114): the prompt expected$PR_METADATA/$PR_DIFFto be interpolated into the prompt body, butclaude --printdoes no shell-variable substitution, so Haiku always saw the raw$PR_METADATAtext and reported the data as missing. Asilent-escalatefallback inreview-one-pr.shsynthesized a fakeescalate=MEDIUMverdict, which kicked every PR up to the deep-review tier — burning Sonnet/Opus tokens on every single PR while the workflow looked healthy.The session circuit breaker / triage hard-fail in commit
4d2aee3(onclaude/practical-cannon-f1a4d4) correctly removed that fallback and finally surfaced the underlying bug, with Tier 1 returning conversational text on combsmith/ContentTwin#100 instead of JSON.This PR fixes three things:
scripts/review-one-pr.shnow builds a self-contained triage prompt at/tmp/cascade/triage-prompt.mdby concatenating the static template with the actual PR context (URL, SHA, metadata JSON, truncated diff, prior review body). The model gets the data inline, not via env-var interpolation that never happened.--permission-mode planwas wrong: under--printwith no human, plan mode makes the model propose a plan and ask for approval — that's exactly the "Before I proceed with the plan, I need to understand your intent…" output we saw. Removed; explicit--disallowed-toolscovers every standard tool so the model has no choice but to answer.prs.txtcontext contamination: with Read/Glob available in plan mode, Haiku wandered the cwd, found the 31-line candidate pool, and assumed the task was about all of them. Disabling tools eliminates the contamination at the source.Belt-and-braces: a
sedstep strips```json ... ```fences beforejq empty, since Haiku still tends to fence its JSON despite the instruction.Verification
prs.txtto cwd produced the "31 PRs" output verbatim.scripts/review-one-pr.shagainst PR fix(pr-review): correct bot username to donpetry-bot (no hyphen) #100 indry_run=truemode. Triage now returnsescalate=false risk=LOWand the cascade proceeds to the single-reviewer Opus confirmation as designed. The Opus body even self-diagnosed: "The recurring re-reviews are caused by atriage-output-invalidinfrastructure failure in the triage tier" — exactly the bug this PR fixes.Out of scope
Per the bug report, this PR does not touch the session circuit breaker / per-tier timeouts / triage hard-fail logic that just landed in
4d2aee3. That code is correct — it caught this bug. This is a fix onmainand merges independently of the practical-cannon branch.Test plan
claude --print --model claude-haiku-4-5-20251001 --disallowed-tools "..." < /tmp/cascade/triage-prompt.mdreturns valid JSONjq emptyscripts/review-one-pr.shdry run on fix: update code-quality ruleset to full required check list ContentTwin#100 completes through Tier 1 → single confirmation → would-post🤖 Generated with Claude Code