Uh oh!
There was an error while loading. Please reload this page.
fix: make harness PR reviewer evaluate against the PR base branch - #2106
Conversation
The AI PR reviewer's local context clones sit on the default branch (main), so PRs targeting the refactor branch were reviewed against the main source layout — producing findings about files and code paths that do not exist on refactor (e.g. src/cli / src/lib vs src/handlers / src/core). Instruct the reviewer, in both the system and review prompts, to sync the local clone to the PR's base branch before reading files for context, and to not raise findings premised on a file or path being absent without verifying against that base branch.
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
Nice, well-scoped fix — the base-branch-vs-main mismatch is a real problem that will bite refactor-targeted PRs, and the layout facts in both prompts (src/handlers/+src/core/ on refactor vs. src/cli/+src/lib/ on main) match what's actually on those branches. One concrete concern before merging:
The example curl uses $CLONE_TOKEN, which isn't present at container runtime
In .github/harness/prompts/review.md (the new block, roughly lines 11–30) the recipe is:
curl -sH "Authorization: Bearer $CLONE_TOKEN" https://api.github.com/repos/aws/agentcore-cli/pulls/<number> | jq -r .base.ref
But per .github/harness/Dockerfile and .github/harness/README.md, CLONE_TOKEN is a build-time ARG used only to configure git config --global url."https://${CLONE_TOKEN}@github.com/".insteadOf .... The README explicitly states: "The token is never sent to the Harness runtime or persisted in this image." So at review time $CLONE_TOKEN will be empty, and curl -H "Authorization: Bearer " against api.github.com returns HTTP 401 (verified). jq -r .base.ref then emits null, and the follow-up git checkout null will fail — dropping the agent back to main, i.e. the exact failure mode this PR is trying to prevent.
Options:
- Drop the auth header entirely.
aws/agentcore-cliis public, so an unauthenticated call to/repos/aws/agentcore-cli/pulls/<n>succeeds and returns.base.ref. Simplest fix and matches how the review agent already fetches PR metadata today. - Rely on
git ls-remote/ a plaingit fetch --allplus resolving the base ref from the diff/PR context the harness already passes in, rather than a fresh REST call. - If you actually want an authenticated call, arrange for the reusable workflow (
aws/agentcore-devx-devtools/.github/workflows/reusable-pr-ai-review.yml) to inject a token env var into the runtime container, and reference that variable here — and update.github/harness/README.mdto match.
Everything else (the system.md blurb, the "don't raise findings whose premise is a missing file until you've confirmed the base branch" guardrail, the file paths modified) looks good.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## refactor #2106 +/- ##
=========================================
Coverage 97.42% 97.42% =========================================
Files 429 429 Lines 26286 26286 =========================================
Hits 25609 25609 Misses 677 677 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
CLONE_TOKEN is a build-time ARG (git config only), not present at container runtime, so the Authorization header would send an empty bearer and 401 — yielding a null base ref and a failed checkout back to main. aws/agentcore-cli is public, so drop the header.
Claude Security Review: no high-confidence findings. (run) |
Uh oh!
There was an error while loading. Please reload this page.
Problem
The AI PR reviewer (
agentcore-devx-automation"AgentCore Harness Review") reviews PRs against the default branch (main) rather than the PR's actual base branch. Its local context clones (/opt/workspace/agentcore-cli) sit onmain, so PRs targetingrefactorare analyzed against themainsource layout.This produces confidently-wrong findings: e.g. on #2105 it reported a "split-brain read/write" regression citing
src/cli/commands/deploy/actions.ts,src/lib/schemas/io/path-resolver.ts, andsrc/cli/operations/init/files.ts— none of which exist onrefactor(which usessrc/handlers/andsrc/core/).Fix
The actual checkout logic lives in the reusable workflow in
aws/agentcore-devx-devtools(not editable here), so this fixes it at the lever we control — the prompts referenced bypr-automation.yml:review.md: instruct the reviewer to determine the PR'sbase.refandgit fetch/git checkoutit in the local clone before reading files for context, and to not raise findings premised on a file/path being absent without verifying against that base branch.system.md: document thatagentcore-clihas a long-livedrefactorbranch that diverges frommain, and that PRs must be analyzed against their own base branch.Notes
pull_request_targetreads the workflow + prompts from the base branch, so this must land onrefactorto fix refactor-targeted PRs. A parallel change onmainkeeps parity for main-targeted PRs.aws/agentcore-devx-devtools; this prompt-level mitigation is the in-repo stopgap.