Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ jobs:
tests/test_safety_checks.bats \
tests/test_safety_checks_triage_inline.bats \
tests/test_prefetch_context.bats \
tests/test_prefetch_prompt_rewire.bats \
tests/test_engine_unavailable_notice.bats \
tests/test_batch_skip_reporting.bats \
tests/test_review_one_pr_ci_pending.bats \
Expand Down
6 changes: 6 additions & 0 deletions prompts/cascade-action.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ security audit) has produced a verdict in `$FINAL_RESULT`. Your job is to:

The review will be posted by the bash script, not by this prompt.

> **Pre-fed PR context (epic #1101):** This step synthesizes the already-resolved
> verdict in `$FINAL_RESULT` (plus prior-tier context) into the posting body. It
> **does not fetch** the PR diff or metadata — so it needs no pre-fed-context
> rewire and is explicitly **out of scope** for the Story-5 (#1105) audit/single
> prompt changes.

## Inputs (environment variables)

- `$FINAL_RESULT` — path to verdict JSON from the resolving tier
Expand Down
34 changes: 33 additions & 1 deletion prompts/security-audit.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,44 @@ reviewer, called only for PRs with real concerns.

**FORBIDDEN**: enumeration commands, actions on other PRs.

## Pre-fed PR context

When the shared context prefetch is enabled (epic #1101, Story 2), the
orchestrator fetches the FULL diff and a superset `gh pr view` metadata JSON
ONCE and persists them to SHA-bound files, exporting their paths:

- `$PR_CONTEXT_METADATA_FILE` — the superset metadata JSON, stamped with a
top-level `.pr_head_sha` field (read it with
`jq -r '.pr_head_sha' "$PR_CONTEXT_METADATA_FILE"`).
- `$PR_CONTEXT_DIFF_FILE` — the full diff, whose first line is
`# PR_HEAD_SHA: <sha>`.

**If** both variables are set, the files exist, **and** their stamp matches
`$PR_HEAD_SHA`, then read the pre-fed metadata and diff from these files
**instead of** running the `gh pr view` / `gh pr diff` in steps 2–3 below — it
is the same context, already fetched, so do not re-fetch it.

**Otherwise** (the prefetch flag is off, the variables are unset, a file is
missing, or the stamp does NOT match `$PR_HEAD_SHA` — meaning the PR moved and
the pre-fed copy is stale), fall back to running `gh pr view` / `gh pr diff`
exactly as written in steps 2–3. Behavior is byte-identical when the prefetch
is off.

This covers only the primary metadata + diff (steps 2–3). Everything else —
the linked-issue fetch (step 4), the org-standards fetch via `gh api`
(step 5), and (when enabled) LSP finding-verification — stays dynamic; keep
using `gh`, `gh api`, and the MCP/LSP tools for those.

## Steps

1. Read `$TRIAGE_RESULT` (triage signals) and the deep review verdict at
`$DEEP_RESULT` (its findings, risk, and reasoning).
2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,headRepository,headRepositoryOwner,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files`
3. `gh pr diff "$PR_URL"` — read the diff. Focus on the areas the deep review flagged.
— unless the metadata was pre-fed (see **Pre-fed PR context**), in which case
read it from `$PR_CONTEXT_METADATA_FILE` instead of running this command.
3. `gh pr diff "$PR_URL"` — read the diff. Focus on the areas the deep review
flagged. Unless the diff was pre-fed (see **Pre-fed PR context**), in which
case read it from `$PR_CONTEXT_DIFF_FILE` instead of running this command.
4. Fetch linked issues if any.
5. Read any CONTRIBUTING.md, AGENTS.md, CODEOWNERS in the repo to check
standards compliance (fetch via `gh api`).
Expand Down
36 changes: 35 additions & 1 deletion prompts/single-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,46 @@ You review **exactly one pull request**: `$PR_URL`. Nothing else.
- `gh search prs`, `gh pr list`, `gh pr status`, or any enumeration command.
- Any action on any PR other than `$PR_URL`.

## Pre-fed PR context

When the shared context prefetch is enabled (epic #1101, Story 2), the
orchestrator fetches the FULL diff and a superset `gh pr view` metadata JSON
ONCE and persists them to SHA-bound files, exporting their paths:

- `$PR_CONTEXT_METADATA_FILE` — the superset metadata JSON, stamped with a
top-level `.pr_head_sha` field (read it with
`jq -r '.pr_head_sha' "$PR_CONTEXT_METADATA_FILE"`).
- `$PR_CONTEXT_DIFF_FILE` — the full diff, whose first line is
`# PR_HEAD_SHA: <sha>`.

**If** both variables are set, the files exist, **and** their stamp matches
`$PR_HEAD_SHA`, then read the pre-fed metadata and diff from these files
**instead of** running the `gh pr view` / `gh pr diff` in steps 1–2 below — it
is the same context, already fetched, so do not re-fetch it. Still apply the
step-1 `isDraft` and `headRefOid == $PR_HEAD_SHA` skip checks against the
pre-fed metadata.

**Otherwise** (the prefetch flag is off, the variables are unset, a file is
missing, or the stamp does NOT match `$PR_HEAD_SHA` — meaning the PR moved and
the pre-fed copy is stale), fall back to running `gh pr view` / `gh pr diff`
exactly as written in steps 1–2. Behavior is byte-identical when the prefetch
is off.

This covers only the primary metadata + diff (steps 1–2). Everything else —
the incremental-mode `gh api .../compare` diff-since-prior-review (step 2), the
`run_secret_scanning` MCP scan (step 3), and the linked-issue fetch (step 4) —
stays dynamic; keep using `gh`, `gh api`, and the MCP tools for those.

## Context-gathering

1. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,headRepository,headRepositoryOwner,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files`
— unless the metadata was pre-fed (see **Pre-fed PR context**), in which case
read it from `$PR_CONTEXT_METADATA_FILE` instead of running this command.
- If `isDraft` → skip. Write the skip verdict to `$OUTPUT_FILE` using `jq` (e.g., `jq -n --arg pr "$PR_URL" '{pr: $pr, decision: "skip", reason: "draft"}' > "$OUTPUT_FILE"`) and exit.
- Verify `headRefOid == $PR_HEAD_SHA`. If not → write the skip verdict to `$OUTPUT_FILE` using `jq` (e.g., `jq -n --arg pr "$PR_URL" '{pr: $pr, decision: "skip", reason: "head-sha-changed"}' > "$OUTPUT_FILE"`) and exit.
2. `gh pr diff "$PR_URL"` — read the diff.
2. `gh pr diff "$PR_URL"` — read the diff. Unless the diff was pre-fed (see
**Pre-fed PR context**), in which case read it from `$PR_CONTEXT_DIFF_FILE`
instead of running this command.
- **Incremental mode**: also get the diff since the prior review. Derive
`<owner>` and `<repo>` from the `headRepository` field in the PR metadata
(i.e., `headRepository.owner.login` and `headRepository.name`):
Expand Down
137 changes: 137 additions & 0 deletions tests/test_prefetch_prompt_rewire.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
#!/usr/bin/env bats
# Structural guard for the tier-3 audit + single-review pre-fed-context rewire
# (epic #1101, Story 5 / #1105).
#
# Story 2 (#1103) persists the FULL diff + superset metadata ONCE to SHA-bound
# files exposed as PR_CONTEXT_DIFF_FILE / PR_CONTEXT_METADATA_FILE (gated
# DEFAULT-OFF behind PREFETCH_CONTEXT_ENABLED). This story rewires
# prompts/security-audit.md and prompts/single-review.md to CONSUME those files
# instead of re-running `gh pr view` / `gh pr diff` when they are present and
# fresh, while staying byte-identical when the flag is off (files absent), and
# keeping their dynamic `gh api` / MCP / LSP steps intact.
#
# These checks are deterministic and offline — the live single/audit smoke run
# (AC #5) needs the real cascade engine. They guard against the pre-fed section
# being silently removed or hollowed out by a future edit/template sync, the same
# silent-revert regression class the repo defends against elsewhere (#655, #823).

setup() {
ROOT="$(cd "$BATS_TEST_DIRNAME/.." && pwd)"
AUDIT="$ROOT/prompts/security-audit.md"
SINGLE="$ROOT/prompts/single-review.md"
ACTION="$ROOT/prompts/cascade-action.md"
}

# Extract the body of the "Pre-fed PR context" section (from its heading up to
# the next `## ` heading) so the pre-fed-specific assertions cannot accidentally
# match text elsewhere in the prompt.
_prefed_section() {
[ -f "${1:-}" ] || return 1
awk '
/^##[[:space:]]+Pre-fed PR context/ { insec=1; next }
insec && /^##[[:space:]]/ { insec=0 }
insec { print }
' "$1"
}
Comment thread
don-petry marked this conversation as resolved.
Resolved

# Extract the body of an arbitrary named section (heading word(s) passed as $1,
# file as $2) so fallback-step assertions cannot accidentally match prose in the
# Pre-fed PR context section that also mentions `gh pr view` / `gh pr diff`.
_section() {
local heading="$1" file="$2"
[ -f "${file:-}" ] || return 1
awk -v pat="^##[[:space:]]+${heading}" '
$0 ~ pat { insec=1; next }
insec && /^##[[:space:]]/ { insec=0 }
insec { print }
' "$file"
}

# --- security-audit.md ------------------------------------------------------

@test "audit prompt has a Pre-fed PR context section" {
grep -Eq '^##[[:space:]]+Pre-fed PR context' "$AUDIT"
}

@test "audit pre-fed section references both context env vars" {
section="$(_prefed_section "$AUDIT")"
grep -q 'PR_CONTEXT_METADATA_FILE' <<<"$section"
grep -q 'PR_CONTEXT_DIFF_FILE' <<<"$section"
}

@test "audit pre-fed section says to read the files INSTEAD of gh, else fall back" {
# The 'if set, read instead of gh; else run gh as before' contract keeps the
# flag-off path byte-identical.
section="$(_prefed_section "$AUDIT")"
grep -Eiq 'instead of' <<<"$section"
grep -Eiq 'fall back|else|otherwise' <<<"$section"
}

@test "audit pre-fed section verifies the PR_HEAD_SHA freshness stamp" {
section="$(_prefed_section "$AUDIT")"
grep -q 'PR_HEAD_SHA' <<<"$section"
grep -Eiq 'pr_head_sha|stamp|match|fresh' <<<"$section"
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

@test "audit prompt retains its dynamic gh api standards fetch" {
# The org-standards fetch (CONTRIBUTING/AGENTS/CODEOWNERS via gh api) is
# DYNAMIC and must survive the rewire — it is not part of the pre-fed context.
grep -q 'gh api' "$AUDIT"
}

@test "audit prompt still keeps gh pr view / gh pr diff as the fallback fetch" {
steps="$(_section 'Steps' "$AUDIT")"
grep -q 'gh pr view' <<<"$steps"
grep -q 'gh pr diff' <<<"$steps"
}
Comment thread
don-petry marked this conversation as resolved.
Resolved

# --- single-review.md -------------------------------------------------------

@test "single-review prompt has a Pre-fed PR context section" {
grep -Eq '^##[[:space:]]+Pre-fed PR context' "$SINGLE"
}

@test "single-review pre-fed section references both context env vars" {
section="$(_prefed_section "$SINGLE")"
grep -q 'PR_CONTEXT_METADATA_FILE' <<<"$section"
grep -q 'PR_CONTEXT_DIFF_FILE' <<<"$section"
}

@test "single-review pre-fed section says to read the files INSTEAD of gh, else fall back" {
section="$(_prefed_section "$SINGLE")"
grep -Eiq 'instead of' <<<"$section"
grep -Eiq 'fall back|else|otherwise' <<<"$section"
}

@test "single-review pre-fed section verifies the PR_HEAD_SHA freshness stamp" {
section="$(_prefed_section "$SINGLE")"
grep -q 'PR_HEAD_SHA' <<<"$section"
grep -Eiq 'pr_head_sha|stamp|match|fresh' <<<"$section"
}

@test "single-review prompt keeps its dynamic MCP secret scan + incremental compare" {
# These are beyond the pre-fed metadata/diff and must remain dynamic.
grep -q 'run_secret_scanning' "$SINGLE"
grep -q 'compare/' "$SINGLE"
}

@test "single-review prompt still keeps gh pr view / gh pr diff as the fallback fetch" {
ctx="$(_section 'Context-gathering' "$SINGLE")"
grep -q 'gh pr view' <<<"$ctx"
grep -q 'gh pr diff' <<<"$ctx"
}
Comment thread
don-petry marked this conversation as resolved.
Resolved

@test "single-review prompt preserves the draft / head-sha-changed skip checks" {
# The step-1 skip guards must not be lost when rewiring the fetch — they gate
# off the metadata regardless of whether it was pre-fed or fetched live.
grep -q 'isDraft' "$SINGLE"
grep -q 'head-sha-changed' "$SINGLE"
}

# --- cascade-action.md (AC #4: confirmed out of scope) ----------------------

@test "cascade-action prompt is documented as not fetching the diff (out of scope)" {
# It synthesizes prior verdicts from $FINAL_RESULT and never fetches the diff,
# so it needs no pre-fed-context rewire — documented explicitly per AC #4.
grep -Eiq 'does not fetch|no.*fetch|out of scope|synthes' "$ACTION"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
Loading