[Bugfix #1113] Anchor consult --type integration diff on an integration-branch base by waleedkadous · Pull Request #1114 · cluesmith/codev

waleedkadous · 2026-06-29T00:31:02Z

Summary

consult --type integration pr <N> could produce a 10k+ line diff that overflows a reviewer model, in repos with a long-lived integration branch ahead of the default branch (e.g. a ci branch well ahead of main). The integration review had no way to anchor the diff on the integration branch — it always inherited the PR's host-recorded base.

This adds a base override for the integration path, reusing the verified machinery the --type impl path already has (#777/#784), so a ci-ahead-of-main repo can point integration reviews at ci and get a small, correct diff. Everyone else is unaffected.

Root Cause

--type integration (both builder and architect context) → buildPRQuery(prId) → fetchPRDiff → the pr-diff forge concept → gh pr diff <N>. gh pr diff is anchored on the PR's base branch as recorded on the host (merge-base of base↔head). When a PR's base is the trunk (main) while the head was branched from an integration branch (ci) well ahead of trunk, "changes vs main" legitimately swallow the entire ci-over-main delta. The impl path was already hardened for exactly this diff-base class; the integration path never received that treatment.

Fix

Give the integration path a diff-base override, mirroring the hardened impl path:

--base <ref> flag + consult.integrationBranch config (.codev/config.json). Precedence: flag → config → unset.
computeLocalPRDiff() — when a base override is present, fetch both refs, verify both origin/<ref> resolve, then compute the diff locally as git diff origin/<base>...origin/<head> (three-dot, merge-base anchored). Refs are passed as single argv elements (execFileSync) so branch names with shell metacharacters can't break out.
Fails loudly with an actionable git fetch hint if either ref is unresolvable — it never silently degrades to reviewing the checked-out tree (the cmap-3 degradation guarded against in the impl path).
buildPRQuery(prId, localDiff?) takes an optional local-diff override; PR info/comments are still fetched normally, and the on-disk diff-file handoff is preserved (no inlining).
Default behavior unchanged: with no flag/config, the integration review still uses gh pr diff (the PR's host base).
Fail-fast: --base is rejected outside --type integration. Config-load errors propagate (no silent revert to gh pr diff).

The local-diff path computes the diff with plain git (provider-agnostic), so it works for GitHub and GitLab alike without touching the pr-diff concept.

Out of scope (follow-ups, per the issue)

Auto-detecting the integration branch without configuration; changing the default base; deeper GitLab-specific work.

Test Plan

Regression suites bugfix-1113-integration-base.test.ts (helpers) and bugfix-1113-integration-dispatch.test.ts (wiring):

Override path (small diff): a git fixture with main → ci (well ahead) → feature (off ci); computeLocalPRDiff(ci, feature) three-dot yields only feature.txt, excluding the ci-over-main delta.
Bug demonstration: anchoring on main sweeps in the whole ci delta (ci-big.txt) — the overflow source.
Command-path wiring: drives the real --type integration dispatch (resolveArchitectQuery → resolveIntegrationBase → computeLocalPRDiff → buildPRQuery) with --base ci (forge pr-view/pr-diff/comments stubbed); asserts the query consumed the local three-dot diff (changed-file set = feature.txt only; the temp diff file contains the real change, not the stubbed gh pr diff output).
Fail-loud: unresolvable base ref and unresolvable head ref both throw with a git fetch hint (no silent local-tree fallback).
resolveIntegrationBase precedence: flag > config > undefined (no override → default gh pr diff, unchanged); malformed config propagates the error; --base works even with a broken config.
Guard: --base rejected outside --type integration.

Proven locally vs. deferred ⚠️

Proven here: the diff math, the dispatch wiring (that --type integration --base <ref> computes and consumes the local three-dot diff rather than gh pr diff), the fail-loud ref checks, precedence/guard, and config error propagation. Full unit suite green (0 failures); pnpm build green.
Deferred (NOT run locally): the true end-to-end headline — that a genuinely overflowing PR now fits a reviewer model and returns a verdict with --base ci. codev itself has no ci-ahead-of-main topology, so this requires an adopter repo with the real topology. Recommend closing this against the real repro PR after merge.

Fixes #1113

…gration-branch base consult --type integration always diffed via gh pr diff (the PR's host-recorded base), so in repos with a long-lived integration branch (e.g. ci) ahead of main the diff swallowed the whole ci-over-main delta and overflowed the reviewer. Add a base override for the integration path, mirroring the hardened --type impl machinery: - --base <ref> flag + consult.integrationBranch config (.codev/config.json). - computeLocalPRDiff(): fetch both refs, verify both origin/<ref> resolve (fail loudly with a git fetch hint, no silent fallback to the checked-out tree), then git diff origin/<base>...origin/<head> (three-dot, merge-base anchored). - buildPRQuery() takes an optional local-diff override; PR info/comments still fetched normally; on-disk diff-file handoff preserved. - Wired both integration cases (builder head = current branch; architect head = pr.headRefName). No override -> gh pr diff, unchanged. - Fail-fast: --base is rejected outside --type integration.

…base - computeLocalPRDiff three-dot against the integration branch yields ONLY the PR change (excludes the ci-over-main delta that overflows the reviewer). - bug demo: anchoring on main sweeps in the whole integration delta. - fail-loud on unresolvable base/head refs (no silent local-tree fallback). - resolveIntegrationBase precedence: flag > config > undefined (default unchanged). - --base rejected outside --type integration.

…ionBase (CMAP) CMAP (codex) REQUEST_CHANGES: the blanket try/catch swallowed loadConfig errors (malformed .codev/config.json, legacy af-config.json, invalid harness config) and silently reverted to gh pr diff — the exact overflow this fix prevents, with no signal why. Remove the catch so those errors propagate like every other loadConfig caller. The explicit --base flag still short-circuits before the read, so it works even with a broken config. Add regression tests: malformed config throws (not silent undefined); --base works despite a broken config.

…leton (CMAP) CMAP (codex) REQUEST_CHANGES round 2: the consult.md doc update was applied to codev/resources/commands/ but not mirrored to the shipped codev-skeleton/ template, breaking the two-tree framework-doc mirroring rule and leaving adopters with stale CLI docs. Mirror the identical --base / consult.integrationBranch section into codev-skeleton/resources/commands/consult.md. (packages/codev/skeleton/ is a gitignored build artifact regenerated by copy-skeleton — not committed.)

waleedkadous · 2026-06-29T00:43:43Z

CMAP review (Gemini · Codex · Claude)

Reviewer	Verdict	Confidence
Claude	✅ APPROVE	HIGH
Gemini	✅ APPROVE	HIGH
Codex	✅ APPROVE	HIGH

Codex initially returned REQUEST_CHANGES twice; both findings were valid and have been addressed:

Config errors were silently swallowed — resolveIntegrationBase() wrapped loadConfig() in a blanket try/catch, so a malformed .codev/config.json (or legacy af-config.json) would quietly revert to gh pr diff — the exact overflow this fix prevents. Removed the catch so those errors propagate like every other loadConfig caller; the explicit --base flag still short-circuits before the read. Added regression tests (43507a4b).
Skeleton doc not mirrored — the consult.md update was applied to codev/resources/commands/ but not the shipped codev-skeleton/ template (two-tree mirroring rule). Mirrored it (542b9048). packages/codev/skeleton/ is a gitignored build artifact, regenerated by copy-skeleton.

After both fixes, Codex re-reviewed → APPROVE (HIGH), no issues.

Verification: full unit suite green (0 failures), pnpm build green, porch Fix-phase checks (npm run build, npm test) green.

…quested

…ack) Architect noted the suite proved the diff helpers in isolation but never drove the actual --type integration command path. Add a dispatch wiring test that exercises resolveArchitectQuery -> resolveIntegrationBase -> computeLocalPRDiff -> buildPRQuery with --base ci (forge pr-view/pr-diff/comments stubbed; git runs for real), and asserts the query consumed the local three-dot diff (changed-file set = feature.txt; temp diff file holds the real change, not the stubbed gh pr diff) rather than the host-base diff. Export resolveArchitectQuery for the test.

…st + proven-vs-deferred)

…sume + PR #1095/#1114 Other fixes + PR #1109 Polish (post-v3.2.1)

waleedkadous added 7 commits June 28, 2026 17:07

chore(porch): bugfix-1113 init bugfix

b163475

chore(porch): bugfix-1113 fix phase-transition

95fea50

chore(porch): bugfix-1113 pr phase-transition

7f4c830

waleedkadous added 4 commits June 28, 2026 17:44

[Bugfix #1113] Update thread: CMAP results, fixes applied, pr gate re…

7c4d88b

…quested

chore(porch): bugfix-1113 pr gate-requested

47f5ea0

[Bugfix #1113] Update thread: architect feedback addressed (wiring te…

0e87498

…st + proven-vs-deferred)

waleedkadous merged commit f519cfd into main Jun 29, 2026
6 checks passed

amrmelsayed added a commit that referenced this pull request Jun 29, 2026

release notes: PR #1059 codex architect + PR #1116 multi-architect re…

102a242

…sume + PR #1095/#1114 Other fixes + PR #1109 Polish (post-v3.2.1)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Bugfix #1113] Anchor consult --type integration diff on an integration-branch base#1114

[Bugfix #1113] Anchor consult --type integration diff on an integration-branch base#1114
waleedkadous merged 11 commits into
mainfrom
builder/bugfix-1113

waleedkadous commented Jun 29, 2026 •

edited

Loading

Uh oh!

waleedkadous commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

waleedkadous commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Out of scope (follow-ups, per the issue)

Test Plan

Proven locally vs. deferred ⚠️

Uh oh!

waleedkadous commented Jun 29, 2026

CMAP review (Gemini · Codex · Claude)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

waleedkadous commented Jun 29, 2026 •

edited

Loading