fix(jira,linear): board state on build failure + re-open revert (review blocker 9)

9a (Jira board lies): pipeline.py moved the Jira card to In Review the moment a
PR opened — but ensure_pr deliberately opens a PR even on a FAILED build, so a
red build was reported as ready-for-review. Gate transition_pr_opened on
build_passed, mirroring the Linear twin (react_task_finished transitions only on
success). A build-failed PR now leaves the card In Progress with the failure
comment. +2 tests (fail → no transition, pass → transition).

9b (Linear re-open revert was a silent no-op): the rollup's re-opened-epic revert
(In Review → In Progress) never fired because transitionIssueState's backward-move
guard blocks a within-type regression (both are 'started' type; In Progress sits
at a lower position). Added an allowSameTypeRegression opt-in that relaxes ONLY
the same-type position tiebreak — cross-type demotion (Done → In Progress) stays
blocked. The rollup re-open passes it. +3 tests (blocked by default, allowed with
flag, cross-type still blocked). agent + cdk gates green.

Author: isadeks

agent/src/pipeline.py

@@ -1343,13 +1343,26 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
             if pr_url:
                 progress.write_agent_milestone("pr_created", pr_url)
                 # Move the Jira card In Progress → In Review now that a PR is
-                # open (issue #572). Only fires when a PR was actually opened —
-                # failed / no-PR tasks leave the card where humans can see the
-                # failure comment. No-op for non-Jira tasks; best-effort.
-                transition_pr_opened(
-                    config.channel_source,
-                    config.channel_metadata,
-                )
+                # open (issue #572) — but ONLY when the build passed. ensure_pr
+                # deliberately opens a PR even on a FAILED build (so the human
+                # sees the broken diff), so gating on pr_url alone moved the card
+                # to In Review on a red build, telling the board the work is ready
+                # for review when it isn't (review blocker #9a). Gate on build_ok
+                # to mirror the Linear twin, which only transitions on success
+                # (react_task_finished: `if transition_state and success`). A
+                # build-failed PR leaves the card In Progress with the failure
+                # comment. No-op for non-Jira tasks; best-effort.
+                if build_passed:
+                    transition_pr_opened(
+                        config.channel_source,
+                        config.channel_metadata,
+                    )
+                else:
+                    log(
+                        "TASK",
+                        "Jira card NOT moved to In Review — PR opened with a FAILED build "
+                        "(build_ok=False); leaving it In Progress with the failure comment (#9a).",
+                    )
 
             # Memory write — capture task episode and repo learnings
             memory_written = False

agent/tests/test_pipeline.py

@@ -800,6 +800,129 @@ async def fake_run_agent(_prompt, _system_prompt, _config, cwd=None, trajectory=
 
         mock_ensure_pr.assert_called_once()
 
+    @patch("runner.run_agent")
+    @patch("pipeline.build_system_prompt")
+    @patch("pipeline.discover_project_config")
+    @patch("repo.setup_repo")
+    @patch("pipeline.task_span")
+    def test_jira_card_not_moved_to_in_review_on_build_failure(
+        self,
+        mock_task_span,
+        mock_setup_repo,
+        _mock_discover,
+        _mock_build_prompt,
+        mock_run_agent,
+        monkeypatch,
+    ):
+        """review blocker #9a: ensure_pr opens a PR even on a FAILED build (so the
+        human sees the broken diff), so the Jira In Progress → In Review transition
+        must gate on build_passed — not merely on pr_url — or the board lies that
+        the work is ready for review. Mirrors the Linear success-only twin."""
+        monkeypatch.setenv("GITHUB_TOKEN", "ghp_test")
+        monkeypatch.setenv("AWS_REGION", "us-east-1")
+        mock_setup_repo.return_value = RepoSetup(
+            repo_dir="/workspace/repo",
+            branch="bgagent/test/branch",
+            build_before=True,
+        )
+
+        async def fake_run_agent(_prompt, _system_prompt, _config, cwd=None, trajectory=None):
+            return AgentResult(status="success", turns=2, cost_usd=0.01, num_turns=2)
+
+        mock_run_agent.side_effect = fake_run_agent
+
+        mock_span = MagicMock()
+        mock_span.__enter__ = MagicMock(return_value=mock_span)
+        mock_span.__exit__ = MagicMock(return_value=False)
+        mock_task_span.return_value = mock_span
+
+        mock_transition = MagicMock()
+        with (
+            patch("pipeline.ensure_committed", return_value=False),
+            # FAILED build — ensure_pr still opens the PR below.
+            patch("pipeline.verify_build", return_value=VerifyOutcome(passed=False)),
+            patch("pipeline.verify_lint", return_value=VerifyOutcome(passed=True)),
+            patch("pipeline.ensure_pr", return_value="https://github.com/o/r/pull/9"),
+            patch("pipeline.transition_pr_opened", mock_transition),
+            patch("pipeline.get_disk_usage", return_value=0),
+            patch("pipeline.print_metrics"),
+            patch("pipeline.task_state") as mock_task_state_mod,
+        ):
+            mock_task_state_mod.get_task = MagicMock(return_value={"status": "RUNNING"})
+            mock_task_state_mod.TaskFetchError = Exception  # type: ignore[attr-defined]
+            from pipeline import run_task
+
+            run_task(
+                repo_url="o/r",
+                task_description="x",
+                github_token="ghp_test",
+                aws_region="us-east-1",
+                task_id="t-jira-failbuild",
+                channel_source="jira",
+                channel_metadata={"jira_issue_key": "ABC-1"},
+            )
+        # PR opened, but the Jira card was NOT advanced to In Review on the red build.
+        mock_transition.assert_not_called()
+
+    @patch("runner.run_agent")
+    @patch("pipeline.build_system_prompt")
+    @patch("pipeline.discover_project_config")
+    @patch("repo.setup_repo")
+    @patch("pipeline.task_span")
+    def test_jira_card_moved_to_in_review_on_build_success(
+        self,
+        mock_task_span,
+        mock_setup_repo,
+        _mock_discover,
+        _mock_build_prompt,
+        mock_run_agent,
+        monkeypatch,
+    ):
+        """The positive twin: a PASSING build DOES move the Jira card to In Review."""
+        monkeypatch.setenv("GITHUB_TOKEN", "ghp_test")
+        monkeypatch.setenv("AWS_REGION", "us-east-1")
+        mock_setup_repo.return_value = RepoSetup(
+            repo_dir="/workspace/repo",
+            branch="bgagent/test/branch",
+            build_before=True,
+        )
+
+        async def fake_run_agent(_prompt, _system_prompt, _config, cwd=None, trajectory=None):
+            return AgentResult(status="success", turns=2, cost_usd=0.01, num_turns=2)
+
+        mock_run_agent.side_effect = fake_run_agent
+
+        mock_span = MagicMock()
+        mock_span.__enter__ = MagicMock(return_value=mock_span)
+        mock_span.__exit__ = MagicMock(return_value=False)
+        mock_task_span.return_value = mock_span
+
+        mock_transition = MagicMock()
+        with (
+            patch("pipeline.ensure_committed", return_value=False),
+            patch("pipeline.verify_build", return_value=VerifyOutcome(passed=True)),
+            patch("pipeline.verify_lint", return_value=VerifyOutcome(passed=True)),
+            patch("pipeline.ensure_pr", return_value="https://github.com/o/r/pull/9"),
+            patch("pipeline.transition_pr_opened", mock_transition),
+            patch("pipeline.get_disk_usage", return_value=0),
+            patch("pipeline.print_metrics"),
+            patch("pipeline.task_state") as mock_task_state_mod,
+        ):
+            mock_task_state_mod.get_task = MagicMock(return_value={"status": "RUNNING"})
+            mock_task_state_mod.TaskFetchError = Exception  # type: ignore[attr-defined]
+            from pipeline import run_task
+
+            run_task(
+                repo_url="o/r",
+                task_description="x",
+                github_token="ghp_test",
+                aws_region="us-east-1",
+                task_id="t-jira-okbuild",
+                channel_source="jira",
+                channel_metadata={"jira_issue_key": "ABC-1"},
+            )
+        mock_transition.assert_called_once()
+
     @patch("runner.run_agent")
     @patch("pipeline.build_system_prompt")
     @patch("pipeline.discover_project_config")

cdk/src/handlers/shared/linear-feedback.ts

@@ -717,6 +717,16 @@ export async function transitionIssueState(
   issueId: string,
   targetType: 'started' | 'completed',
   preferredNames: readonly string[] = [],
+  /**
+   * review blocker #9b: allow a WITHIN-TYPE regression (e.g. In Review → In
+   * Progress, both ``started``). By default the backward-move guard blocks it —
+   * correct for most callers, but it silently no-op'd the orchestration rollup's
+   * deliberate re-open (a settled epic getting a new/re-run child must go back
+   * from In Review to In Progress, and both are ``started`` type so the
+   * position-tiebreak blocked it). Cross-TYPE demotion (completed → started) is
+   * still ALWAYS blocked — this only relaxes the same-type position tiebreak.
+   */
+  allowSameTypeRegression = false,
 ): Promise<boolean> {
   const token = await resolveToken(ctx);
   if (!token) return false;
@@ -755,12 +765,18 @@ export async function transitionIssueState(
     };
     const curRank = TYPE_RANK[current.type] ?? 0;
     const tgtRank = TYPE_RANK[target.type] ?? 0;
-    const backward = curRank > tgtRank || (curRank === tgtRank && current.position >= target.position);
+    const crossTypeDemotion = curRank > tgtRank;
+    const sameTypeRegression = curRank === tgtRank && current.position >= target.position;
+    // Cross-type demotion (e.g. completed → started) is NEVER allowed — a human
+    // or automation advanced it. A same-type regression (In Review → In Progress)
+    // is allowed ONLY when the caller opts in (#9b: the rollup re-open).
+    const backward = crossTypeDemotion || (sameTypeRegression && !allowSameTypeRegression);
     if (backward) {
       logger.info('Linear state transition: skipping backward move', {
         issue_id: issueId,
         current_state: current.name,
         target_state: target.name,
+        cross_type: crossTypeDemotion,
       });
       return false;
     }

cdk/src/handlers/shared/orchestration-rollup.ts

@@ -468,8 +468,12 @@ export async function upsertEpicPanel(params: UpsertEpicPanelParams): Promise<st
     const anyBad = rows.some((r) => r.child_status === 'failed' || r.child_status === 'skipped');
     try {
       if (inProgress) {
-        // Re-opened (or running): back to In Progress + 👀.
-        await transitionIssueState(params.ctx, params.parentLinearIssueId, 'started', ['In Progress']);
+        // Re-opened (or running): back to In Progress + 👀. Pass
+        // allowSameTypeRegression (#9b) — a settled epic is In Review (started),
+        // and In Progress is ALSO started but at a lower position, so the default
+        // backward-move guard silently blocked this deliberate re-open. Cross-type
+        // demotion (a human moved it to Done) is still blocked by the guard.
+        await transitionIssueState(params.ctx, params.parentLinearIssueId, 'started', ['In Progress'], true);
         await swapIssueReaction(params.ctx, params.parentLinearIssueId, 'eyes');
       } else if (!anyBad) {
         // Clean completion: work done, awaiting human merge → In Review + ✅.

cdk/test/handlers/shared/linear-feedback.test.ts

@@ -503,6 +503,31 @@ describe('linear-feedback', () => {
       expect(fetchMock).toHaveBeenCalledTimes(1);
     });
 
+    // review blocker #9b: the rollup re-open (In Review → In Progress, BOTH
+    // 'started') was silently blocked by the same-type position tiebreak.
+    test('same-type regression In Review → In Progress is BLOCKED by default', async () => {
+      fetchMock.mockResolvedValueOnce(statesResp(cur('s-inreview')));
+      const ok = await transitionIssueState(CTX, ISSUE_ID, 'started', ['In Progress']);
+      expect(ok).toBe(false); // In Progress (pos 2) < In Review (pos 1002) → backward
+      expect(fetchMock).toHaveBeenCalledTimes(1); // no mutation
+    });
+
+    test('same-type regression In Review → In Progress SUCCEEDS with allowSameTypeRegression (rollup re-open)', async () => {
+      fetchMock
+        .mockResolvedValueOnce(statesResp(cur('s-inreview')))
+        .mockResolvedValueOnce(jsonResponse({ data: { issueUpdate: { success: true } } }));
+      const ok = await transitionIssueState(CTX, ISSUE_ID, 'started', ['In Progress'], true);
+      expect(ok).toBe(true);
+      expect(JSON.parse(fetchMock.mock.calls[1][1].body).variables.stateId).toBe('s-inprogress');
+    });
+
+    test('allowSameTypeRegression does NOT permit a cross-type demotion (Done → In Progress still blocked)', async () => {
+      fetchMock.mockResolvedValueOnce(statesResp(cur('s-done')));
+      const ok = await transitionIssueState(CTX, ISSUE_ID, 'started', ['In Progress'], true);
+      expect(ok).toBe(false); // completed → started is cross-type; still refused
+      expect(fetchMock).toHaveBeenCalledTimes(1);
+    });
+
     test('returns false when token cannot be resolved', async () => {
       resolveLinearOauthTokenMock.mockResolvedValueOnce(null);
       const ok = await transitionIssueState(CTX, ISSUE_ID, 'started', ['In Review']);