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
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
140 changes: 140 additions & 0 deletions tests/test_prefetch_prompt_rewire.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
#!/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"
}

# --- 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"
}

@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
# Verify implementation: cascade-action must contain no live fetch commands.
! grep -q 'gh pr view' "$ACTION"
! grep -q 'gh pr diff' "$ACTION"
}
Loading