From 5c659a52b62560c765cdb26c2202df13e99391fe Mon Sep 17 00:00:00 2001 From: bgagent Date: Fri, 10 Jul 2026 04:25:54 +0000 Subject: [PATCH 1/2] fix(orchestration): stop pinning stale main as the epic integration base (ABCA-688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The synthetic integration / diamond fan-in node hardcoded its base branch to the platform default ('main'). On a repo whose real default branch is a different long-lived branch, the integration node branched off a stale 'main', merged the default-branch-based leaf branches into it, and opened its PR against 'main' — so the diff was the ENTIRE branch divergence (100 commits / 700+ files) instead of the handful of files actually changed. Root children already omitted the base (agent detects the real default), which is why single-issue tasks were correctly scoped and only the epic flow bloated. Fix: only the LINEAR stack case pins a base (a real predecessor branch). The diamond / integration node now OMITS base_branch so the agent branches off the repo's detected default, and still sees every predecessor via merge_branches. repo.py's off-HEAD branch path now runs the predecessor merges too. Adds regression tests on both sides (CDK release shape threading + agent off-default branching/merge). Co-Authored-By: Claude Opus 4.8 Task-Id: 01KX4Z66HE9STXPJN102CFX264 Prompt-Version: 1c9c10e027a2 --- agent/src/repo.py | 24 +++++-- agent/tests/test_repo.py | 54 ++++++++++++++++ .../handlers/shared/orchestration-release.ts | 21 ++++++- .../shared/orchestration-release.test.ts | 62 +++++++++++++++++++ 4 files changed, 153 insertions(+), 8 deletions(-) diff --git a/agent/src/repo.py b/agent/src/repo.py index 33ccf5c4..4cd31199 100644 --- a/agent/src/repo.py +++ b/agent/src/repo.py @@ -262,9 +262,9 @@ def setup_repo(config: TaskConfig, progress: Any = None) -> RepoSetup: for pred_branch in config.merge_branches: _merge_predecessor_branch(repo_dir, pred_branch, notes) elif config.base_branch: - # #247 A4: stacked child. Branch from the predecessor's branch - # (linear) or from main (diamond) so the child sees predecessor - # code without waiting for a human merge. fetch the base first — + # #247 A4: LINEAR stacked child. Branch from the single predecessor's + # branch so the child sees the predecessor's code without waiting for a + # human merge, and its PR shows only its OWN diff. fetch the base first — # it is an unmerged sibling branch that the fresh clone may not # have locally. log("SETUP", f"Creating branch {branch} from base {config.base_branch}") @@ -292,14 +292,28 @@ def setup_repo(config: TaskConfig, progress: Any = None) -> RepoSetup: log("SETUP", f"Base branch not found; creating {branch} off HEAD") run_cmd(["git", "checkout", "-b", branch], label="create-branch", cwd=repo_dir) - # Diamond: merge each predecessor branch into this child's branch - # so it sees ALL predecessors' code (the base only gave it one). + # A linear stack may still carry merge_branches on the restack path; + # merge each so the child sees ALL predecessors' code. for pred_branch in config.merge_branches: _merge_predecessor_branch(repo_dir, pred_branch, notes) else: + # Root OR diamond (#247 A4 / ABCA-688). Branch off the cloned HEAD, + # which is the repo's REAL default branch (``gh repo clone`` checks out + # the default). This is deliberately NOT pinned to 'main': pinning a + # stale 'main' as the diamond/integration base — on a repo whose default + # is a different branch — made the epic's integration PR diff the entire + # branch divergence (ABCA-688: 100 commits / 700+ files). Branching off + # the real default keeps the PR base correct so it shows only the + # actual changes. log("SETUP", f"Creating branch: {branch}") run_cmd(["git", "checkout", "-b", branch], label="create-branch", cwd=repo_dir) + # Diamond fan-in: merge each predecessor branch into this child's branch + # (off the real default) so it sees ALL predecessors' code. Empty for a + # true root — the loop is then a no-op and behavior is unchanged. + for pred_branch in config.merge_branches: + _merge_predecessor_branch(repo_dir, pred_branch, notes) + # Trust mise config files in the cloned repo (required before mise install # AND before every `mise run `). ``mise trust `` trusts only the # ROOT ``mise.toml`` — but a monorepo has per-package config ROOTS diff --git a/agent/tests/test_repo.py b/agent/tests/test_repo.py index a9030401..c90e6e72 100644 --- a/agent/tests/test_repo.py +++ b/agent/tests/test_repo.py @@ -375,6 +375,60 @@ def fake_run(*args, **kwargs): assert repo.detect_default_branch("owner/repo", "/tmp/x") == "main" +class TestDiamondBranchesOffRealDefault: + """ABCA-688: the diamond / synthetic integration child must branch off the + repo's REAL default branch (the cloned HEAD), NOT a pinned 'main', and still + merge every predecessor. Pinning 'main' as the diamond base on a repo whose + default is a different long-lived branch made the epic's integration PR diff + the entire branch divergence (100 commits / 700+ files). The platform now + OMITS base_branch for the diamond and threads only merge_branches; repo.py + must fall into the off-HEAD branch AND run the predecessor merges there.""" + + def test_diamond_no_base_branch_branches_off_head_and_merges_preds(self, monkeypatch): + fake = _fake_run_cmd() + _patch_common(monkeypatch, fake) + # The repo's real default is NOT 'main' — mirrors the failing fork. + monkeypatch.setattr(repo, "detect_default_branch", lambda url, d: "linear-vercel") + + setup = repo.setup_repo( + _config( + is_pr_workflow=False, + branch_name="bgagent/01INTTASK/orch-integration", + # Diamond: no base_branch, but predecessor branches to merge. + base_branch=None, + merge_branches=["bgagent/taskB/b", "bgagent/taskC/c"], + ) + ) + + labels = fake.labels() + # Branched off the cloned HEAD (the real default) — NOT from a pinned base. + assert "create-branch" in labels + assert "create-branch-from-base" not in labels + # Every predecessor branch was fetched + merged so the child sees all code. + merge_cmds = [c["cmd"] for c in fake.calls if "merge-predecessor" in c["label"]] + merged_refs = {cmd[-1] for cmd in merge_cmds} + assert merged_refs == {"origin/bgagent/taskB/b", "origin/bgagent/taskC/c"} + # The PR base + commit-diff range is the real default, not 'main'. + assert setup.default_branch == "linear-vercel" + + def test_linear_child_still_pins_predecessor_base(self, monkeypatch): + # A single-predecessor (linear) child keeps stacking on its predecessor's + # branch — that path is correct and unchanged by the ABCA-688 fix. + fake = _fake_run_cmd() + _patch_common(monkeypatch, fake) + setup = repo.setup_repo( + _config( + is_pr_workflow=False, + branch_name="bgagent/01CHILD/step-b", + base_branch="bgagent/taskA/step-a", + ) + ) + labels = fake.labels() + assert "create-branch-from-base" in labels + # base_branch wins as the PR base for a linear stack (no detection call). + assert setup.default_branch == "bgagent/taskA/step-a" + + class TestPlatformBranchNameVerbatim: """The agent MUST use the platform-provided ``config.branch_name`` verbatim when present, for EVERY workflow — never re-deriving its own slug. A diff --git a/cdk/src/handlers/shared/orchestration-release.ts b/cdk/src/handlers/shared/orchestration-release.ts index d003ff94..401e66e2 100644 --- a/cdk/src/handlers/shared/orchestration-release.ts +++ b/cdk/src/handlers/shared/orchestration-release.ts @@ -423,9 +423,24 @@ export async function releaseReadyChildren( ...(releaseContext.channel_source !== undefined && { channelSource: releaseContext.channel_source as ChannelSource, }), - // Root → 'main' base, no merges (omit so today's off-main behavior - // is unchanged). Linear → predecessor branch. Diamond → main + merges. - ...(selection.shape !== 'root' && { baseBranch: selection.base_branch }), + // Linear (single predecessor) → pin the child's base to that + // predecessor's branch so it stacks and its PR shows only its OWN diff. + // Root AND diamond → DO NOT pin a base branch: omit it so the agent + // detects the repo's REAL default branch and branches off it. + // + // ABCA-688: pinning ``selection.base_branch`` (which is the hardcoded + // ``defaultBranch``, historically 'main') for the diamond / synthetic + // integration node was the epic-PR bloat bug. On a repo whose default + // branch is NOT 'main' (e.g. a fork whose default is an integration + // branch), the integration node branched off a stale 'main', merged the + // default-branch-based leaf branches into it, and opened its PR against + // 'main' — so the diff was the ENTIRE branch divergence (100 commits / + // 700+ files) rather than the handful of files actually changed. Roots + // already omitted the base (and produced correctly-scoped PRs), which is + // why single-issue tasks were fine and only the epic flow bloated. The + // diamond still sees every predecessor's code via ``mergeBranches`` (the + // agent merges them onto the detected-default HEAD). + ...(selection.shape === 'linear' && { baseBranch: selection.base_branch }), ...(selection.merge_branches.length > 0 && { mergeBranches: selection.merge_branches }), createTaskCore, now, diff --git a/cdk/test/handlers/shared/orchestration-release.test.ts b/cdk/test/handlers/shared/orchestration-release.test.ts index 9521ea90..d975916b 100644 --- a/cdk/test/handlers/shared/orchestration-release.test.ts +++ b/cdk/test/handlers/shared/orchestration-release.test.ts @@ -469,6 +469,68 @@ describe('releaseReadyChildren — #331 concurrency throttle', () => { }); }); +describe('releaseReadyChildren — ABCA-688: base branch threading by shape', () => { + function createOk() { + let i = 0; + return jest.fn().mockImplementation(() => + Promise.resolve({ statusCode: 201, body: JSON.stringify({ data: { task_id: `T-${i++}` } }) })); + } + + test('LINEAR child (one predecessor) pins its base to the predecessor branch', async () => { + const ddb = { send: jest.fn().mockResolvedValue({}) }; + const createTaskCore = createOk(); + const pred = makeRow({ sub_issue_id: 'A', child_status: 'succeeded', child_branch_name: 'bgagent/taskA/a' }); + const child = makeRow({ sub_issue_id: 'B', depends_on: ['A'], child_status: 'ready' }); + await releaseReadyChildren( + ddb as never, 'OrchTable', [child], { platform_user_id: 'u1' } as never, + createTaskCore as never, NOW, [pred, child], 'main', undefined, + ); + const ctx = createTaskCore.mock.calls[0][1]; + expect(ctx.channelMetadata.orchestration_base_branch).toBe('bgagent/taskA/a'); + // Linear stack carries no merge list (it stacks directly). + expect(ctx.channelMetadata.orchestration_merge_branches).toBeUndefined(); + }); + + test('DIAMOND / integration child (2+ predecessors) does NOT pin a base branch — regression for the huge epic PR', async () => { + // ABCA-688: pinning the hardcoded default ('main') as the diamond base made + // the integration PR diff the whole branch divergence on a repo whose real + // default is not 'main'. The diamond must OMIT the base so the agent + // branches off the repo's detected default, and merge predecessors instead. + const ddb = { send: jest.fn().mockResolvedValue({}) }; + const createTaskCore = createOk(); + const b = makeRow({ sub_issue_id: 'B', child_status: 'succeeded', child_branch_name: 'bgagent/taskB/b' }); + const c = makeRow({ sub_issue_id: 'C', child_status: 'succeeded', child_branch_name: 'bgagent/taskC/c' }); + const integration = makeRow({ + sub_issue_id: 'orch_abc__integration', depends_on: ['B', 'C'], child_status: 'ready', + }); + await releaseReadyChildren( + ddb as never, 'OrchTable', [integration], { platform_user_id: 'u1' } as never, + // A non-'main' default here mirrors the failing repo (default = a + // long-lived integration branch). The bug pinned this as the base. + createTaskCore as never, NOW, [b, c, integration], 'linear-vercel', undefined, + ); + const ctx = createTaskCore.mock.calls[0][1]; + // The key assertion: NO base branch pinned (agent detects the real default). + expect(ctx.channelMetadata.orchestration_base_branch).toBeUndefined(); + // But it still sees every predecessor's code via the merge list. + expect(ctx.channelMetadata.orchestration_merge_branches) + .toBe(JSON.stringify(['bgagent/taskB/b', 'bgagent/taskC/c'])); + }); + + test('ROOT child pins no base branch and carries no merges (unchanged)', async () => { + const ddb = { send: jest.fn().mockResolvedValue({}) }; + const createTaskCore = createOk(); + const root = makeRow({ sub_issue_id: 'R', depends_on: [], child_status: 'ready' }); + await releaseReadyChildren( + ddb as never, 'OrchTable', [root], { platform_user_id: 'u1' } as never, + createTaskCore as never, NOW, [root], 'linear-vercel', undefined, + ); + const ctx = createTaskCore.mock.calls[0][1]; + expect(ctx.channelMetadata.orchestration_base_branch).toBeUndefined(); + expect(ctx.channelMetadata.orchestration_merge_branches).toBeUndefined(); + }); +}); + describe('readConcurrencyBudget — #331', () => { test('free budget = cap - active_count', async () => { const ddb = { send: jest.fn().mockResolvedValue({ Item: { active_count: 3 } }) }; From e873398d9ad054e64d819291cfd9f79e72dea616 Mon Sep 17 00:00:00 2001 From: bgagent Date: Fri, 10 Jul 2026 15:42:12 +0000 Subject: [PATCH 2/2] test(orchestration): update A4-DIAMOND e2e assertion for omitted base branch (ABCA-688) Co-Authored-By: Claude Opus 4.8 Task-Id: 01KX6912TNZJ99KCZJ3C1NRHKQ Prompt-Version: 1c9c10e027a2 --- .../integration/orchestration-e2e.test.ts | Bin 23596 -> 23675 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/cdk/test/integration/orchestration-e2e.test.ts b/cdk/test/integration/orchestration-e2e.test.ts index af5997dca70511ea1f9b1f36ba7ead7cbb3e3e68..03d94531af277e39d3f5dd8b61eea97329ec0226 100644 GIT binary patch delta 150 zcmZ3pgYowc#tl;=>m8k(9d*qtEUXk1V;Nj}% pqNxMYk*AQGT9ghn!3n4>5onDwkWMO^d?M0f@{MST&7x7_OaM`GGL!%S delta 56 zcmeypgK^Cc#tl;=WnD58bMy03tQ1@nl8O@Zk~30^74p;46mkTvi}YjHPzSNq KH-|;3G64X40Tjgm