Skip to content

Commit d5c5349

Browse files
committed
Merge branch 'fix/lv-review-blockers' into lv-merge-blockers
# Conflicts: # agent/tests/test_post_hooks.py # cdk/src/handlers/fanout-task-events.ts # cdk/src/handlers/linear-webhook-processor.ts # cdk/src/handlers/shared/linear-subissue-fetch.ts # cdk/src/handlers/shared/orchestration-release.ts # cdk/test/handlers/fanout-task-events.test.ts
2 parents 1fc26af + 71a09b1 commit d5c5349

28 files changed

Lines changed: 1091 additions & 103 deletions

agent/src/pipeline.py

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,13 +1448,26 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
14481448
if pr_url:
14491449
progress.write_agent_milestone("pr_created", pr_url)
14501450
# Move the Jira card In Progress → In Review now that a PR is
1451-
# open (issue #572). Only fires when a PR was actually opened —
1452-
# failed / no-PR tasks leave the card where humans can see the
1453-
# failure comment. No-op for non-Jira tasks; best-effort.
1454-
transition_pr_opened(
1455-
config.channel_source,
1456-
config.channel_metadata,
1457-
)
1451+
# open (issue #572) — but ONLY when the build passed. ensure_pr
1452+
# deliberately opens a PR even on a FAILED build (so the human
1453+
# sees the broken diff), so gating on pr_url alone moved the card
1454+
# to In Review on a red build, telling the board the work is ready
1455+
# for review when it isn't (review blocker #9a). Gate on build_ok
1456+
# to mirror the Linear twin, which only transitions on success
1457+
# (react_task_finished: `if transition_state and success`). A
1458+
# build-failed PR leaves the card In Progress with the failure
1459+
# comment. No-op for non-Jira tasks; best-effort.
1460+
if build_passed:
1461+
transition_pr_opened(
1462+
config.channel_source,
1463+
config.channel_metadata,
1464+
)
1465+
else:
1466+
log(
1467+
"TASK",
1468+
"Jira card NOT moved to In Review — PR opened with a FAILED build "
1469+
"(build_ok=False); leaving it In Progress with the failure comment (#9a).",
1470+
)
14581471

14591472
# Memory write — capture task episode and repo learnings
14601473
memory_written = False

agent/src/post_hooks.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,79 @@ def _note_unpushed_commits(repo_dir: str, branch: str, config: TaskConfig) -> No
489489
log("WARN", f"Failed to post un-pushed-commits note: {type(e).__name__}: {e}")
490490

491491

492+
def _reconcile_pr_base(repo_dir: str, branch: str, config: TaskConfig, expected_base: str) -> None:
493+
"""Deterministically retarget an existing PR onto ``expected_base``.
494+
495+
The PR is created by the AGENT (its own ``gh pr create`` in the prompt
496+
workflow), so the ``--base`` it chose is a model judgment call, not the
497+
orchestrator's. Live-caught on the #247 chain stress test (2026-07-18):
498+
a stacked child that branched off its predecessor's branch STILL opened
499+
its PR against ``main`` — the agent reasoned "this was based off the
500+
chain-Y branch, let me open the PR against main" and even a root whose
501+
``detect_default_branch`` correctly returned ``linear-vercel`` was pointed
502+
at ``main``. Wrong base ⇒ the PR shows the whole default-branch divergence
503+
(100s of files) instead of the child's real delta, and a stacked child's
504+
PR merges onto the wrong branch.
505+
506+
``expected_base`` is ``setup.default_branch`` — which is the orchestrator's
507+
``base_branch`` for a stacked child (the predecessor's branch, or ``main``
508+
for a diamond) and ``detect_default_branch`` for a root. This post-hook
509+
reads the PR's current base and, if it disagrees, retargets it via
510+
``gh pr edit --base`` — removing the agent's discretion without forbidding
511+
it from opening the PR (which keeps the agent-authored title/body).
512+
513+
Best-effort: any failure is logged, never fatal — a mis-based PR is a
514+
presentation/merge-target defect, not a reason to fail the whole task.
515+
"""
516+
try:
517+
view = subprocess.run(
518+
[
519+
"gh",
520+
"pr",
521+
"view",
522+
branch,
523+
"--repo",
524+
config.repo_url,
525+
"--json",
526+
"baseRefName",
527+
"-q",
528+
".baseRefName",
529+
],
530+
cwd=repo_dir,
531+
capture_output=True,
532+
text=True,
533+
timeout=60,
534+
)
535+
except (OSError, subprocess.SubprocessError) as exc:
536+
log("WARN", f"Could not read PR base to reconcile ({type(exc).__name__}) — leaving as-is")
537+
return
538+
if view.returncode != 0 or not view.stdout.strip():
539+
# No PR / unreadable base — nothing to reconcile (creation path handles
540+
# the no-PR case; a transient read error just leaves the base as-is).
541+
return
542+
current_base = view.stdout.strip()
543+
if current_base == expected_base:
544+
return
545+
log(
546+
"POST",
547+
f"Retargeting PR base '{current_base}' → '{expected_base}' "
548+
f"(deterministic; agent chose the wrong base)",
549+
)
550+
result = run_cmd(
551+
["gh", "pr", "edit", branch, "--repo", config.repo_url, "--base", expected_base],
552+
label="reconcile-pr-base",
553+
cwd=repo_dir,
554+
check=False,
555+
)
556+
if result.returncode != 0:
557+
stderr_msg = result.stderr.strip()[:200] if result.stderr else "(no stderr)"
558+
log(
559+
"WARN",
560+
f"Failed to retarget PR base to '{expected_base}' (gh exit {result.returncode}): "
561+
f"{stderr_msg} — PR remains based on '{current_base}'",
562+
)
563+
564+
492565
def ensure_pr(
493566
config: TaskConfig,
494567
setup: RepoSetup,
@@ -582,6 +655,10 @@ def ensure_pr(
582655
if result.returncode == 0 and result.stdout.strip():
583656
pr_url = result.stdout.strip()
584657
log("POST", f"PR already exists: {pr_url}")
658+
# The agent opened this PR and picked its own --base; correct it to the
659+
# orchestrator-supplied / detected base if it disagrees (stacked-child
660+
# + root wrong-base fix, 2026-07-18).
661+
_reconcile_pr_base(repo_dir, branch, config, default_branch)
585662
return pr_url
586663

587664
# Check if there are any commits on this branch beyond the default branch

agent/src/prompts/new_task.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,12 @@
8585
- Any patterns or conventions you discovered about this repo
8686
- Suggestions for future tasks on this repo
8787
88-
Run:
88+
Run this EXACTLY — the `--base` value is chosen for you (for a stacked \
89+
sub-issue it is the predecessor's branch so the PR shows only your delta; for \
90+
a normal task it is the repo default). Do NOT substitute a different base such \
91+
as `main`/`master` even if this branch was cut from another branch — targeting \
92+
the wrong base makes the PR show the whole branch divergence instead of your \
93+
change:
8994
```
9095
gh pr create --repo {repo_url} --head {branch_name} --base {default_branch} --title "<type>(<module>): <description>" --body "<body>"
9196
```

agent/tests/test_pipeline.py

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -876,6 +876,129 @@ async def fake_run_agent(_prompt, _system_prompt, _config, cwd=None, trajectory=
876876

877877
mock_ensure_pr.assert_called_once()
878878

879+
@patch("runner.run_agent")
880+
@patch("pipeline.build_system_prompt")
881+
@patch("pipeline.discover_project_config")
882+
@patch("repo.setup_repo")
883+
@patch("pipeline.task_span")
884+
def test_jira_card_not_moved_to_in_review_on_build_failure(
885+
self,
886+
mock_task_span,
887+
mock_setup_repo,
888+
_mock_discover,
889+
_mock_build_prompt,
890+
mock_run_agent,
891+
monkeypatch,
892+
):
893+
"""review blocker #9a: ensure_pr opens a PR even on a FAILED build (so the
894+
human sees the broken diff), so the Jira In Progress → In Review transition
895+
must gate on build_passed — not merely on pr_url — or the board lies that
896+
the work is ready for review. Mirrors the Linear success-only twin."""
897+
monkeypatch.setenv("GITHUB_TOKEN", "ghp_test")
898+
monkeypatch.setenv("AWS_REGION", "us-east-1")
899+
mock_setup_repo.return_value = RepoSetup(
900+
repo_dir="/workspace/repo",
901+
branch="bgagent/test/branch",
902+
build_before=True,
903+
)
904+
905+
async def fake_run_agent(_prompt, _system_prompt, _config, cwd=None, trajectory=None):
906+
return AgentResult(status="success", turns=2, cost_usd=0.01, num_turns=2)
907+
908+
mock_run_agent.side_effect = fake_run_agent
909+
910+
mock_span = MagicMock()
911+
mock_span.__enter__ = MagicMock(return_value=mock_span)
912+
mock_span.__exit__ = MagicMock(return_value=False)
913+
mock_task_span.return_value = mock_span
914+
915+
mock_transition = MagicMock()
916+
with (
917+
patch("pipeline.ensure_committed", return_value=False),
918+
# FAILED build — ensure_pr still opens the PR below.
919+
patch("pipeline.verify_build", return_value=VerifyOutcome(passed=False)),
920+
patch("pipeline.verify_lint", return_value=VerifyOutcome(passed=True)),
921+
patch("pipeline.ensure_pr", return_value="https://github.com/o/r/pull/9"),
922+
patch("pipeline.transition_pr_opened", mock_transition),
923+
patch("pipeline.get_disk_usage", return_value=0),
924+
patch("pipeline.print_metrics"),
925+
patch("pipeline.task_state") as mock_task_state_mod,
926+
):
927+
mock_task_state_mod.get_task = MagicMock(return_value={"status": "RUNNING"})
928+
mock_task_state_mod.TaskFetchError = Exception # type: ignore[attr-defined]
929+
from pipeline import run_task
930+
931+
run_task(
932+
repo_url="o/r",
933+
task_description="x",
934+
github_token="ghp_test",
935+
aws_region="us-east-1",
936+
task_id="t-jira-failbuild",
937+
channel_source="jira",
938+
channel_metadata={"jira_issue_key": "ABC-1"},
939+
)
940+
# PR opened, but the Jira card was NOT advanced to In Review on the red build.
941+
mock_transition.assert_not_called()
942+
943+
@patch("runner.run_agent")
944+
@patch("pipeline.build_system_prompt")
945+
@patch("pipeline.discover_project_config")
946+
@patch("repo.setup_repo")
947+
@patch("pipeline.task_span")
948+
def test_jira_card_moved_to_in_review_on_build_success(
949+
self,
950+
mock_task_span,
951+
mock_setup_repo,
952+
_mock_discover,
953+
_mock_build_prompt,
954+
mock_run_agent,
955+
monkeypatch,
956+
):
957+
"""The positive twin: a PASSING build DOES move the Jira card to In Review."""
958+
monkeypatch.setenv("GITHUB_TOKEN", "ghp_test")
959+
monkeypatch.setenv("AWS_REGION", "us-east-1")
960+
mock_setup_repo.return_value = RepoSetup(
961+
repo_dir="/workspace/repo",
962+
branch="bgagent/test/branch",
963+
build_before=True,
964+
)
965+
966+
async def fake_run_agent(_prompt, _system_prompt, _config, cwd=None, trajectory=None):
967+
return AgentResult(status="success", turns=2, cost_usd=0.01, num_turns=2)
968+
969+
mock_run_agent.side_effect = fake_run_agent
970+
971+
mock_span = MagicMock()
972+
mock_span.__enter__ = MagicMock(return_value=mock_span)
973+
mock_span.__exit__ = MagicMock(return_value=False)
974+
mock_task_span.return_value = mock_span
975+
976+
mock_transition = MagicMock()
977+
with (
978+
patch("pipeline.ensure_committed", return_value=False),
979+
patch("pipeline.verify_build", return_value=VerifyOutcome(passed=True)),
980+
patch("pipeline.verify_lint", return_value=VerifyOutcome(passed=True)),
981+
patch("pipeline.ensure_pr", return_value="https://github.com/o/r/pull/9"),
982+
patch("pipeline.transition_pr_opened", mock_transition),
983+
patch("pipeline.get_disk_usage", return_value=0),
984+
patch("pipeline.print_metrics"),
985+
patch("pipeline.task_state") as mock_task_state_mod,
986+
):
987+
mock_task_state_mod.get_task = MagicMock(return_value={"status": "RUNNING"})
988+
mock_task_state_mod.TaskFetchError = Exception # type: ignore[attr-defined]
989+
from pipeline import run_task
990+
991+
run_task(
992+
repo_url="o/r",
993+
task_description="x",
994+
github_token="ghp_test",
995+
aws_region="us-east-1",
996+
task_id="t-jira-okbuild",
997+
channel_source="jira",
998+
channel_metadata={"jira_issue_key": "ABC-1"},
999+
)
1000+
mock_transition.assert_called_once()
1001+
8791002
@patch("runner.run_agent")
8801003
@patch("pipeline.build_system_prompt")
8811004
@patch("pipeline.discover_project_config")

0 commit comments

Comments
 (0)