Repository navigation
consult --type {spec,plan,impl} resolves artifact from invoking workspace (not the builder's branch); impl-review diff base hardcoded to main #777
Description
Activity
Adding evidence to narrow the fix surface for Defect B.
Confirmed hardcoded sites (exact strings):
packages/codev/src/commands/consult/index.ts:989—git merge-base HEAD main(thebuildImplQueryfallback).packages/codev/src/commands/consult/index.ts:1297—git merge-base main origin/${pr.headRefName}(the impl-review path).
Test infrastructure assumes the same.
packages/codev/src/__tests__/bugfix-280-consult-diff.test.ts:31initialises withgit init -b main, and line 54 usesgit merge-base HEAD main. Non-main-default-branch repos aren't currently exercised.Origin trace:
9e95a2ab("Fix architect auto-restart, consultation loops, and maxIterations escape") introduced the pattern conceptually.92300008("[Spec 325][Phase: cli-rewrite] Rewrite consult CLI with flag-based mode routing") is whatgit log -L 989,989:packages/codev/src/commands/consult/index.tsand-L 1297,1297:...both blame for the current line shape — the Spec 325 rewrite carried the assumption forward without re-examination.
Existing fix pattern in the codebase (the bigger find):
packages/vscode/src/commands/view-diff.ts:262-271already does the right thing —let defaultBranch = 'main'; try { const ref = execFileSync('git', ['-C', wt, 'symbolic-ref', '--short', 'refs/remotes/origin/HEAD']).toString().trim(); if (ref) { defaultBranch = ref.replace(/^origin\//, ''); } } catch { // origin/HEAD not set — keep `main`. }
Read
origin/HEADfor the actual default branch; fall back tomainonly when unset. Suggests a tight fix surface:- Extract
resolveDefaultBranch(workspaceRoot)into a shared helper (e.g.@cluesmith/codev-core/git). - Use it at
consult/index.ts:989and:1297. - Replace
view-diff.ts:262-271's inline implementation with the helper so the three sites stay in sync.
Defect A (workspace-rooted artifact resolution) is a separate concern and is not addressed by this fix.
PIR Layer 2 is complete on
feat/pir-and-vscode-updates-2(commit01f733d3).The eight PIR-specific sites listed in the issue body now resolve the default branch dynamically via the in-tree
view-diff.tsresolver pattern:$(git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null | sed 's|^origin/||' || echo main)Mirrored into both
codev/protocols/pir/andcodev-skeleton/protocols/pir/(so newly-adopted projects get the fix and existing consumers can pull the same change into their checked-in copies). Behaviour-preserving onmain-default repos; correct on non-mainones. PIR is the first protocol to adopt this pattern in markdown prompts; the other protocols' Layer 2 sites should follow the same shape when they're touched.Remaining on this issue (out of scope of the PIR commit, still tracked):
- Layer 1 —
packages/codev/src/commands/consult/index.ts:989, 1297(protocol-agnostic; affects every protocol equally). Recommended next step: extractresolveDefaultBranch(workspaceRoot)into@cluesmith/codev-core, use at both consult sites, replace the inline copy inview-diff.ts:262-271with the helper. - Layer 2 — other protocols. BUGFIX (
protocol.md:327literal command +consult-types/pr-review.md:44review question); the cross-protocol "up to date with main?" question in SPIR/ASPIR/AIR/MAINTAINconsult-types/pr-review.md; and the SPIR/ASPIR "Commit tomain..." convention inspir/protocol.md:148, 243. Each should follow PIR's pattern. - Layer 3 —
packages/codev/src/__tests__/bugfix-280-consult-diff.test.ts:31test setup (git init -b main); add a non-main-default test scenario. - Optional — porch-side prompt substitution so prompts can carry a
{{DEFAULT_BRANCH}}placeholder porch fills in before sending to the agent.
Defect A (workspace-rooted artifact resolution) remains untouched.
- Layer 1 —
Sibling defect spotted in the wild — reviewers defaulting to two-dot diff semantics.
An impl-review consult on a non-
main-default repo recently flagged "scope creep" listing four files that aren't in the PR. The mechanism wasn't this issue's hardcodedmain— the reviewer (codex) had correctly resolved the integration branch (used the repo's actual default). The error was diff shape: it rangit diff <base>..HEAD(two-dot) instead ofgit diff <base>...HEAD(three-dot).Why this matters: two-dot and three-dot diverge whenever the integration branch has advanced since the builder's branch base.
A..Bis a snapshot comparison and reverse-includes every commit that landed onAafter the divergence point — those commits look like file deletions/changes inB's "scope". Three-dot uses the merge-base and shows only what's been added onB's side since divergence, which is what GitHub's PR diff andgh pr diff --name-onlycompute. The rebuttal verified by runninggit diff --stat <base>...HEAD— clean, only the builder's files.Independent of the existing layers on this issue:
- This issue's Layer 1 / Layer 2 / Layer 3 are all about the branch the diff is computed against (hardcoded
main). - The two-dot/three-dot defect is about the shape of the diff (snapshot vs commit-set-since-divergence).
- Same symptom (false-positive "scope creep") via different mechanisms. They can compound on a non-
main-default repo, or fire independently — the case observed had the integration branch resolved correctly but the wrong diff shape.
Where to investigate:
packages/codev/src/commands/consult/index.ts— the impl-review path around line 1297. Pattern isgit merge-base main origin/<headRef>followed (typically) bygit diff ${mergeBase}..origin/<headRef>. Once the merge-base is correct that pattern IS structurally three-dot. So the question is whether consult includes the diff bytes verbatim in the prompt (in which case the reviewer can't deviate) or only references the diff (in which case reviewers compute their own and may default to two-dot — the more common idiom in training data).codev/protocols/*/consult-types/impl-review.md— if a prompt asks the reviewer to "compute the diff" or "check what changed" without specifying semantics, the freedom is what allows the two-dot fallback.
Fix shape (independent of the branch-name fix):
- If consult emits the diff itself: ensure the
git diffinvocation uses merge-base form (which is structurally three-dot) — never raw two-dotA..B. Audit and constrain. - If reviewers compute their own: either (a) include the diff bytes in the prompt verbatim so reviewers don't need to run anything, or (b) specify the canonical command explicitly in the prompt —
git diff $(git merge-base origin/<base> HEAD) HEADorgit diff origin/<base>...HEAD. Prompt explicitness > assuming reviewers default to the right shape.
Worth tracking as a third "Defect C" on this issue or a sibling issue, depending on how the fix surface should be grouped.
- This issue's Layer 1 / Layer 2 / Layer 3 are all about the branch the diff is computed against (hardcoded
- added a commit that references this issue
on May 20, 2026 Fresh reproduction of Defect B Layer 1 (line ~989
git merge-base HEAD mainfallback inbuildImplQuery), from a PIR impl-review consult run today.Setup
- Project's PR base:
ci(project convention — PRs targetci, notmain). - Builder branch:
builder/pir-<N>, off ofci. cihas accumulated many PRs since the last sync tomain.- PR has 7 actual files (4 code + plan + porch status.yaml + review). Verified via
gh pr view <N> --json files.
What the consult CLI sent to the reviewers
git merge-base HEAD main→ resolved to a commit predating every recent merge intoci.git diff --stat <merge-base>..HEAD→ 77 files, +4154/-521, summary plus### File Listblock embedded in the prompt.- The block included files from 9 unrelated already-merged PRs:
apps/agent/src/shannon/agent/dispatcher/*.py,apps/marketing/src/pages/alpha-terms.astro,packages/db/src/migrations/app/0020_*,tools/shanutil-py/...,codev/plans/14*-*.md,codev/reviews/bugfix-*.md, etc. None of those are in the PR.
Reviewer behaviour split
- Gemini: APPROVE. Re-derived scope from disk; ignored the bloated list.
- Claude: APPROVE. Same — re-derived from disk.
- Codex: REQUEST_CHANGES citing "the review packet includes many additional changed files outside the approved scope" and asked us to "clean the branch down to the planned files or update the review artifact." Codex took the 77-file list at face value as authoritative; the verdict is the exact "phantom scope creep" symptom described in this issue's Observed Impact Context Window Optimization - AI Needs Project Knowledge Without Breaking Limits #2.
The whole conversation is in a builder pane today; happy to share the consult output txt if useful (it's gitignored locally, but I can paste the relevant prompt block).
Confirms
- Defect B Layer 1 fires whenever the integration branch has advanced past
mainand the builder branched off the integration branch — i.e., the normal steady state for any repo using aci-style integration branch. - The split-decision behaviour (Codex trusts the prompt's framing, Gemini/Claude re-verify from disk) maps cleanly onto Research: universal Codex REQUEST_CHANGES patterns + targeted Codex false-alarm prompt #753's Codex-false-alarm pattern.
No new issue — this one already captures the mechanism and fix surface accurately. Just adding reproduction evidence weight.
- Project's PR base:
- addedbugSomething isn't workingSomething isn't workingarea/consultArea: Consult CLI / consultation toolingArea: Consult CLI / consultation tooling
on May 21, 2026 - added a commit that references this issue
on May 23, 2026 - added 17 commits that reference this issue
on May 28, 2026
Problem
Two related artifact-resolution defects in the consult harness produce verdicts that don't reflect the artifact the architect actually intends to review.
Defect A — workspace-rooted artifact resolution
consult --type {spec,plan,impl} --issue Nresolves the artifact (spec/plan/PR diff) from the invoking process's workspace root rather than from the builder's branch. When run from an architect's checkout (typically the repo's integration branch), the consult reviews the version of the artifact that's on the integration branch, not the builder's reworked version onbuilder/.... Verdicts are therefore about the stale artifact, not the work-in-progress one the architect intends to review.commands/consult/index.ts:findPlanContent(workspaceRoot, issueId)/findSpecContent/ impl-diff resolution all read fromworkspaceRoot— wherever the architect's invocation is rooted. There is no flag/parameter to point the consult at a specific branch/ref/worktree.Defect B —
mainhardcoded as default integration branch (multi-layer, uneven distribution)A multi-layer defect: the same assumption is fossilised in both the porch CLI implementation and the protocol instruction markdown. Fixing one without the other leaves the symptom intact. Distribution across protocols is uneven — PIR is roughly 4× more exposed at Layer 2 than its siblings.
Layer 1 — porch CLI / consult code (2 sites, protocol-agnostic)
packages/codev/src/commands/consult/index.ts:989—git merge-base HEAD main(thebuildImplQueryfallback).packages/codev/src/commands/consult/index.ts:1297—git merge-base main origin/${pr.headRefName}(the impl-review path).This layer hits every protocol equally — any
--type implconsult on a non-main-default repo produces false-positive "scope creep" verdicts: the diff sweeps in every commit that's on the integration branch but not onmain, attributing them to the branch under review.Layer 2 — protocol .md instruction files (per-protocol breakdown)
builder-prompt.md:85,prompts/implement.md:21,110,prompts/review.md:41,250consult-types/pr-review.md:25implement.md:136,review.md:242protocol.md:327consult-types/pr-review.md:44consult-types/pr-review.md:31protocol.md:148,243("Commit tomain...")consult-types/pr-review.md:31consult-types/pr-review.md:37consult-types/pr-review.md:31Categories:
Hard-fail (literal shell commands agents execute): the prompt instructs the agent to run
git diff main,git diff --stat main, orgit fetch origin main && git rebase origin/main. On a non-main-default repo these either error (nomainbranch) or produce diffs against an irrelevant branch.Wrong-answer (review questions agents must answer): the prompt asks "Is the branch up to date with main?" On a non-
main-default repo this is always "no" and the consult typically suggests rebase to main — the wrong action.Convention (semantically right, literally wrong on the wire): the prompt says "Don't push to main", "Commit to main so builder worktrees include the artifact", "merge to main", etc. The semantic intent is correct ("the integration branch"), but the literal name is fossilised. SPIR/ASPIR's "Commit to
main..." instruction in particular misleads an architect on a non-mainrepo into committing to a branch builders won't see.Why PIR is more exposed: PIR's prompts (introduced more recently) lean on concrete command examples to guide the agent. SPIR/ASPIR use more abstract phrasing. PIR's design choice happens to fossilise the
mainassumption more visibly than its older siblings.codev-skeleton propagation: Identical copies under
codev-skeleton/protocols/ship to every project that adopts codev viacodev init/codev adopt, so consumer repos inherit the hardcoded prompts. Fixing codev's protocols upstream doesn't retroactively fix already-adopted projects — they have static checked-in copies.Layer 3 — test infrastructure
packages/codev/src/__tests__/bugfix-280-consult-diff.test.ts:31—git init -b main. No existing test exercises a non-main-default repo through either Layer 1 or Layer 2.Origin
9e95a2ab("Fix architect auto-restart, consultation loops, and maxIterations escape", 2026-02-12) — pattern introduced conceptually.92300008("[Spec 325][Phase: cli-rewrite] Rewrite consult CLI with flag-based mode routing", 2026-02-16) — current line shape; the rewrite carried the assumption forward without re-examination.Existing fix pattern (in-tree)
packages/vscode/src/commands/view-diff.ts:262-271already does the right thing — readsorigin/HEADviagit symbolic-ref --short refs/remotes/origin/HEAD, falls back tomainonly when unset.Fix surface (scoped by layer and exposure)
Layer 1 (CLI, protocol-agnostic):
resolveDefaultBranch(workspaceRoot)into@cluesmith/codev-core(or similar shared module).consult/index.ts:989and:1297.view-diff.ts:262-271's inline implementation with the helper so the three sites stay in sync.Layer 2 (per-protocol edits, scoped by exposure — PIR is the bulk of the work):
4. PIR-specific pass (8 sites in PIR + skeleton copies): replace literal
git diff main/git rebase origin/maincommands with branch-agnostic phrasing or a placeholder porch substitutes at send time. Replace conventions ("Don't push to main") with "Don't push to the integration branch". (Shipped onfeat/pir-and-vscode-updates-2, commit01f733d3.)5. BUGFIX-specific pass (1 hard-fail in
protocol.md:327): same treatment.6. Cross-protocol one-line edit: replace "Is the branch up to date with main?" in PIR/SPIR/ASPIR/AIR/BUGFIX/MAINTAIN
consult-types/pr-review.mdfiles with branch-agnostic phrasing (6 identical edits).7. SPIR/ASPIR convention pass:
spir/protocol.md:148,243(and ASPIR equivalents) — replace "Commit tomain..." with "Commit to the integration branch...".Layer 3 (tests):
8. Add a non-
main-default repo test (e.g.git init -b devworkspace) that exercises both Layer 1 (consult CLI) and one or two of the more egregious Layer 2 prompts.Optional (cleaner, larger scope):
9. porch resolves the default branch once per workspace and substitutes a placeholder into prompts before handing them to agents — avoids relying on agent-side variable expansion in markdown.
Note on layer interaction: fixing only Layer 1 (the CLI code) leaves the protocol-prompt commands hardcoded; agents will still run
git diff mainliterally even when the consult harness internally resolves the right base. Both layers must move together for consumer-facing behaviour to change. The bulk of Layer 2 work is concentrated in PIR specifically; SPIR/ASPIR/AIR/MAINTAIN need only the cross-protocol one-line edit plus (for SPIR/ASPIR) the convention rewording.Defect C — two-dot diff semantics produce reverse-included upstream "scope creep"
Independent of Defect B's branch-name issue but observable in the same wild scenarios. A reviewer-level defect rooted in three sites in the same consult CLI codepath.
Mechanism. The impl-review prompt has two failure modes that allow a reviewer (e.g. codex) to compute its own diff with two-dot semantics (
git diff <base>..HEAD) instead of three-dot (git diff <base>...HEAD). Two-dot reverse-includes every commit that landed on the base branch since the builder's branch base — those commits appear as file changes in the builder's "scope" even though they aren't in the PR. Three-dot (what GitHub PR diff andgh pr diff --name-onlycompute) is clean.Sites (all in
packages/codev/src/commands/consult/index.ts):Silent fallback when merge-base fails (lines 990-995, 1029-1030):
When the merge-base call fails (e.g., on a non-
main-default repo without a localmain),changedFilesstays empty and the prompt drops to "explore the filesystem." The reviewer is given no scope anchor and naturally defaults togit diff <integration-branch>..HEAD(two-dot, the more common idiom in agent training data).Prompt actively discourages the canonical diff (lines 1026-1028):
Even when consult emits a correct three-dot-equivalent file list, this instruction tells reviewers to look at disk instead. Reviewers reading the worktree see whatever's there — including upstream-rebased files that aren't in their PR.
Range syntax in
getDiffStat(lines 772-773):When
refis the range string${mergeBase}..origin/${pr.headRefName}(passed from line 1297), this expands togit diff --stat A..B— two-dot syntax. Output is correct because A is the merge-base (soA..B≡A...B), but the code reads as two-dot and survives copy-paste. Cosmetic but worth tightening.Observed in the wild. An impl-review on a non-
main-default repo flagged four files as "scope creep" that aren't in the PR. The reviewer had correctly resolved the integration branch (so Defect B didn't fire), but used two-dot diff against it. Three-dot diff was clean.Fix surface (all in
consult/index.ts):resolveDefaultBranch(workspaceRoot). Eliminates the most common reason merge-base lookup fails.gh pr view ${pr.number} --json baseRefName— the canonical answer for what GitHub considers the PR's scope. Only as last-resort drop to "explore the filesystem", and even then include a clear directive: "if you compute your own diff, usegit diff origin/<base>...HEAD(three-dot)".changedFilesis non-empty, state explicitly that the listed files are the canonical scope (three-dot diff equivalent to GitHub's PR view); replace the "Do NOT rely on git diffs" line with something that directs reviewers to the list rather than away from it. Suggested wording: "The files above are the canonical scope of this PR (three-dot diff against the PR's base, equivalent to GitHub's PR view). Do not flag files outside this list, even if you see uncommitted changes in the worktree."buildImplQuerycall): use...(three-dot) instead of..(two-dot) — semantically equivalent given the merge-base on the LHS, but disambiguates in code review and in any prompt text that surfaces the command.Layer. All sites are in the CLI (
commands/consult/index.ts). The protocol .md files have nogit diffinvocations across any protocol (confirmed via grep). No Layer 2 / Layer 3 surface for Defect C — the protocol prompts are clean. Fix surface is purely CLI, identical shape to Defect B Layer 1; the same PR can carry both.Observed impact
Architect-supplied verify workaround broken. When porch's verify is wedged (see sibling issue —
porch verify-wedge) and the architect runsconsult --type plan --issue Nmanually to supply the missing review, Defect A causes the consult to read the stale on-integration-branch plan, not the builder's reworked plan. All three reviewers return REQUEST_CHANGES citing findings that the reworked plan has already corrected. Verdicts are useless for the artifact actually under review — verified by grepping the on-integration-branch plan for the symbols the consult flagged (still present) vs. the rework branch (corrected).Phantom "scope creep" verdicts on impl review in non-
main-integration repos (Defect B Layer 1, all protocols equally): codex returns REQUEST_CHANGES citing files the PR never touched; every flagged file is present on the integration branch but absent onmain. Verified by:gh pr diff --name-only(true scope) ≠ codex's listed-files (polluted scope), with the difference being exactly the set of commits betweenmainand the integration branch.Hard-fail or misleading prompt commands (Defect B Layer 2, primarily PIR + BUGFIX): agents on non-
main-default repos that literally execute the prompt'sgit diff maineither error (nomainbranch) or produce a diff against an irrelevant branch, polluting their planning/review reasoning. SPIR/ASPIR/AIR/MAINTAIN are less affected at this layer — their Layer 2 manifestation is limited to the "up to date with main?" review question and (for SPIR/ASPIR) the "commit to main" architect instruction.Phantom "scope creep" from two-dot reviewer diffs (Defect C, observable independently of Defect B): even when the reviewer resolves the integration branch correctly, two-dot diff against it reverse-includes upstream churn since the branch base, attributing it to the builder. Verified by:
gh pr diff --name-only(true scope) vs the reviewer's flagged files matches the set of commits the integration branch added since the builder's branch base.Together these silently invalidate the consult signal whenever the architect needs it most (supplying a missing verify, or reviewing PRs against a non-
mainintegration branch, or both even on amain-default repo where the integration branch has advanced since the builder branched). They also stack with the sibling porch verify-wedge issue to make the independent 3-way review structurally unreachable for reworked SPIR artifacts.