diff --git a/.claude/skills/epic-codegen/SKILL.md b/.claude/skills/epic-codegen/SKILL.md index 434f77f..4b1412e 100644 --- a/.claude/skills/epic-codegen/SKILL.md +++ b/.claude/skills/epic-codegen/SKILL.md @@ -94,7 +94,20 @@ python3 scripts/check_dependencies.py ${EPIC_ID} ``` Exit 0 means proceed; exit 1 names the dependencies that are not done, and -this run stops there. +this run stops there. Record the stop as `codegen_outcome=blocked`, never +`failed`: + +```bash +python3 scripts/frontmatter.py merge-run-metadata \ + artifacts/codegen-runs/${EPIC_ID}/run-metadata.yaml \ + epic_id=${EPIC_ID} codegen_outcome=blocked versions=0 +``` + +Declining to start is not failing. The pipeline reads this to decide whether +the epic keeps its turn: `failed` writes the terminal CI state `Failed` and +the epic is skipped on every future run until someone edits the data repo by +hand, which is what happened to RHAI-761. `blocked` sends it back to `Blocked` +to be retried once the dependency lands. Check `jira_status`, never the dependency's `status`. Every epic-task file is regenerated from Jira on each run with `status: Pending` hardcoded diff --git a/docs/bugs/fixed/bug-declined-codegen-marked-terminally-failed.md b/docs/bugs/fixed/bug-declined-codegen-marked-terminally-failed.md new file mode 100644 index 0000000..2b213fb --- /dev/null +++ b/docs/bugs/fixed/bug-declined-codegen-marked-terminally-failed.md @@ -0,0 +1,94 @@ +--- +id: bug-declined-codegen-marked-terminally-failed +title: Codegen declining to start was recorded as a terminal failure +type: bug +status: fixed +commits: ["a24e3e5"] +repos: [epic-code-gen] +decisions: [ADR-0025] +--- + +# Bug: Codegen declining to start was recorded as a terminal failure + +## Summary + +`/epic-codegen` runs a dependency gate before it generates anything. When the +gate says no, the skill stops — correctly. But it recorded that stop as +`codegen_outcome: failed`, and `_ci_handle_ready` had one branch for a codegen +that did not produce artifacts: + +```python +state["status"] = "Failed" +state["failure_reason"] = "codegen failed" +``` + +`Failed` is in `CI_TERMINAL_STATES`. A terminal state is never revisited, so +the epic is skipped on every subsequent run, forever, until someone edits the +data repo by hand. + +Refusing to start and trying and breaking are not the same event. The first is +a statement about the *world* — a dependency isn't done yet — and the world +changes between runs. The second is a statement about the epic. Only the second +justifies giving up on it. + +## Reproduction + +Run an epic whose dependency the gate reports as not done. In the data repo: + +```yaml +status: Failed +codegen_outcome: failed +failure_reason: codegen failed +``` + +All three are false. Nothing failed; nothing was even attempted. + +## Expected + +`status: Blocked`, `codegen_outcome: blocked`, `blocked_by:` naming the +dependency — a state the next run re-examines. + +## Actual + +RHAI-761 was marked terminally `Failed` at 19:07:29 by a gate misfire (see +[[bug-dependency-gate-read-stale-snapshot]]) and stayed out of the pipeline +across every later invocation. The stale snapshot cost one cycle; this defect +turned that into all of them. + +## Impact + +High. It converts any transient, self-correcting condition into permanent +removal from the pipeline, and it does so silently — the dashboard reads +`Failed` and shows a broken epic, so the operator looks for a bug in the epic +rather than in the pipeline's bookkeeping. + +## Fix + +Two halves, because there are two writers. + +The skill (`.claude/skills/epic-codegen/SKILL.md`) now records a gate stop as +`codegen_outcome=blocked`. `blocked` was added to `CODEGEN_OUTCOMES` in +`artifact_utils.py`, which is the single definition the `codegen-run` schema +enum and `merge_run_metadata`'s validation both derive from — so no other list +needed touching. + +The pipeline (`run_pipeline.py`) reads that outcome before deciding the CI +state. `blocked` sends the epic back to `Blocked` with its `blocked_by` intact +and no `failure_reason`; anything else still yields `Failed`. This works +because `_merge_run_metadata_into_state` folds the skill's fields into the live +state dict before the branch runs. + +The lowercase outcome `blocked` and the capitalised CI state `Blocked` are +deliberately distinct vocabularies over the same word, so +`read_codegen_outcome({"status": "Blocked"})` must keep returning `None` — +pinned by `test_blocked_ci_state_is_not_the_blocked_outcome` in +`tests/test_artifact_utils.py`. Behaviour is covered by +`TestCodegenDeclinedIsNotFailed` in `tests/test_ci_mode.py`. + +## Related + +- [[bug-dependency-gate-read-stale-snapshot]] — the misfire that exposed this. +- [[bug-clone-fault-marks-epic-failed]] — still open, and the same mistake one + layer up: a clone or credential fault also writes terminal `Failed`. This fix + covers the skill declining, not the environment breaking. +- ADR-0025 "unrunnable is not failed" — the principle both violate. diff --git a/docs/bugs/fixed/bug-dependency-gate-read-stale-snapshot.md b/docs/bugs/fixed/bug-dependency-gate-read-stale-snapshot.md new file mode 100644 index 0000000..f44e968 --- /dev/null +++ b/docs/bugs/fixed/bug-dependency-gate-read-stale-snapshot.md @@ -0,0 +1,91 @@ +--- +id: bug-dependency-gate-read-stale-snapshot +title: Dependency gate read a snapshot the same run had already invalidated +type: bug +status: fixed +commits: ["a24e3e5"] +repos: [epic-code-gen] +decisions: [ADR-0025] +--- + +# Bug: Dependency gate read a snapshot the same run had already invalidated + +## Summary + +`check_dependencies.py` decides whether an epic may start by reading each +dependency's `jira_status` out of the on-disk epic-task file: + +```python +jira_status = read_frontmatter(dep_path)[0].get("jira_status") +done = jira_status in DONE_STATUSES # Closed, Done, Resolved +``` + +Those files are written **once**, at the top of a run, by +`fetch_jira_epics.py`. Everything after that point reads a photograph of Jira +taken before the run started — including the parts of the run that change Jira. + +A single pipeline invocation therefore both wrote and invalidated its own +source of truth: + +``` +19:06:49 RHAI-760 → Done, transitioned and closed in Jira by run_pipeline.py +19:07:29 RHAI-761 starts; gate reads artifacts/epic-tasks/RHAI-760.md + → jira_status: In Progress (40 seconds stale) + → exit 1, "dependencies not done" +``` + +Nothing was wrong with the dependency. It had been satisfied by the same +process, 40 seconds earlier, in memory the gate could not see. + +## Reproduction + +```python +transition_issue("s", "u", "t", "RHAI-760", "Done", tasks_dir) +check_dependencies("RHAI-761", tasks_dir)["all_done"] # False, before the fix +``` + +## Expected + +An epic whose only blocker was closed earlier in the same run is eligible. + +## Actual + +The gate refuses. The skill then records the refusal as +`codegen_outcome: failed`, which the pipeline maps to the terminal CI state +`Failed` — so RHAI-761 was not merely delayed by one cycle, it was removed from +every future cycle until its state was edited by hand in the data repo. That +second half is its own defect; see +[[bug-declined-codegen-marked-terminally-failed]]. + +## Impact + +High, and it fires precisely where the DAG is doing its job: a chain of +dependent epics under one strategy is the case the dependency graph exists to +handle, and it is the only case where a run closes something another epic is +waiting on. Independent epics never hit it. + +## Fix + +Keep the snapshot in step with the transitions the run performs. +`sync_epic_task_jira_status()` in `run_pipeline.py` writes the new status back +to `artifacts/epic-tasks/.md`, and `transition_issue()` calls it on every +successful transition. All nine epic-level call sites pass the directory; the +two strategy-level sites do not, because strategy keys have no epic-task file +(the helper no-ops on a missing file rather than treating it as an error). + +The narrower alternative — have `check_dependencies.py` query Jira live — +was rejected: it puts a network call in a gate that runs once per epic, and it +leaves every *other* reader of the snapshot still stale. + +Regression coverage in `tests/test_run_pipeline.py`, +`TestSyncEpicTaskJiraStatus`, including the end-to-end case +`test_dependent_gate_passes_after_transition`, which closes a dependency +through `transition_issue` and then asserts the real +`check_dependencies` gate opens. + +## Related + +- [[bug-declined-codegen-marked-terminally-failed]] — what turned this + one-cycle delay into a permanent stall. +- [[bug-clone-fault-marks-epic-failed]] — same shape: an environment or timing + fault recorded as an epic-level failure, against ADR-0025. diff --git a/docs/bugs/open/bug-clone-fault-marks-epic-failed.md b/docs/bugs/open/bug-clone-fault-marks-epic-failed.md index c677171..1fc61c3 100644 --- a/docs/bugs/open/bug-clone-fault-marks-epic-failed.md +++ b/docs/bugs/open/bug-clone-fault-marks-epic-failed.md @@ -63,8 +63,16 @@ Classify the clone failure the way preflight already classifies a missing tool: Worth extracting the retryable-vs-terminal judgement into one helper shared with the preflight gate, rather than a second ad-hoc copy of the rule. +That helper now has a third caller waiting for it: +[[bug-declined-codegen-marked-terminally-failed]] fixed the same +retryable-written-as-terminal mistake for the codegen branch of +`_ci_handle_ready`, with its own inline rule. Two ad-hoc copies exist; the +clone branch would be the third. Fixing this one should fold all three +together. + ## Related +- [[bug-declined-codegen-marked-terminally-failed]] — the sibling case, fixed. - [[bug-slug-extractor-truncated-repo-names]] - [[task-toolchain-preflight]] - [[task-per-repo-github-identity]] diff --git a/scripts/artifact_utils.py b/scripts/artifact_utils.py index 3a3ae4f..315f185 100644 --- a/scripts/artifact_utils.py +++ b/scripts/artifact_utils.py @@ -33,7 +33,11 @@ "PRCreated", "PRChangesRequested", "Done", "Blocked", "Failed", ) -CODEGEN_OUTCOMES = ("completed", "exhausted", "failed", "error") +# `blocked` is a refusal, not a failure: the skill declined to generate because +# a dependency is not done. It exists so the pipeline can tell "this epic has +# nothing to do yet" from "this epic tried and broke", which decides whether +# the epic keeps its retry (RHAI-761 was marked terminally Failed for it). +CODEGEN_OUTCOMES = ("completed", "exhausted", "failed", "error", "blocked") # Fields in run-metadata.yaml that belong to the pipeline's state machine. No # other producer may set them, and a producer's own metadata must never diff --git a/scripts/run_pipeline.py b/scripts/run_pipeline.py index a164373..b918745 100644 --- a/scripts/run_pipeline.py +++ b/scripts/run_pipeline.py @@ -45,6 +45,7 @@ normalize_ci_status, read_codegen_outcome, read_frontmatter_validated, + update_frontmatter, ) from fetch_epic import fetch_strategy from fetch_jira_epics import ( @@ -103,12 +104,55 @@ } -def transition_issue(server, user, token, issue_key, target_status): +def sync_epic_task_jira_status(issue_key, jira_status, epic_tasks_dir): + """Bring the on-disk epic-task snapshot in step with a Jira transition. + + `check_dependencies.py` resolves a dependency by reading `jira_status` + from that dependency's epic-task file, and `fetch_jira_epics.py` writes + those files once, at the start of a run. So a transition this run performs + is invisible to the rest of the same run. + + That cost RHAI-761 a cycle: the pipeline marked RHAI-760 Done and closed it + in Jira at 19:06:49, invoked the dependent 18 seconds later, and the + dependent's gate read a file still saying `In Progress` and refused work it + was entitled to do. Writing the transition back here keeps the file honest + for every later step, so the pipeline and the skill cannot disagree about + a fact the pipeline itself just changed. + + No-op when the file does not exist — strategy keys have no epic-task, and + a `--no-report` run may have none at all. + """ + if not issue_key or not jira_status or not epic_tasks_dir: + return False + path = os.path.join(epic_tasks_dir, f"{issue_key}.md") + if not os.path.isfile(path): + return False + try: + update_frontmatter(path, {"jira_status": jira_status}, "epic-task") + except Exception as e: + # A stale snapshot only ever costs a retry, so never fail the run + # over one — but say so, because it looks like a dependency bug. + print(f" Warning: could not sync jira_status for {issue_key}: {e}", + file=sys.stderr) + return False + return True + + +def epic_tasks_dir_for(args): + """Where fetch_jira_epics.py writes the epic-task snapshots.""" + return os.path.join(args.output_dir, "epic-tasks") + + +def transition_issue(server, user, token, issue_key, target_status, + epic_tasks_dir=None): """Transition a Jira issue to the given status. Discovers available transitions and matches by name (case-insensitive). Falls back to STATUS_ALIASES when no exact match is found. + `epic_tasks_dir`, when given, keeps that issue's epic-task snapshot in + step with the transition — see sync_epic_task_jira_status(). + Returns: tuple: (success: bool, from_status: str) — from_status is the current status name before transition, or empty string on failure. @@ -133,6 +177,8 @@ def transition_issue(server, user, token, issue_key, target_status): try: do_transition(server, user, token, issue_key, t["id"]) print(f" {issue_key}: transitioned to '{to_name}'") + sync_epic_task_jira_status( + issue_key, to_name, epic_tasks_dir) return True, to_name except Exception as e: print(f" Warning: transition to '{to_name}' failed " @@ -821,7 +867,8 @@ def process_strategy(strategy_key, server, user, token, args): pr_url = known_pr_urls.get(key) if pr_url and check_pr_merged(pr_url): ok, _ = transition_issue( - server, user, token, key, "Done") + server, user, token, key, "Done", + epic_tasks_dir_for(args)) if ok: completed_keys.add(key) transitions_log[key] = [ @@ -855,7 +902,8 @@ def process_strategy(strategy_key, server, user, token, args): results[PROCESSED].append((epic_id, "reused completed run")) epic_transitions = [] ok, _ = transition_issue( - server, user, token, epic_id, "Review") + server, user, token, epic_id, "Review", + epic_tasks_dir_for(args)) epic_transitions.append({ "to": "Review", "success": ok}) pr_url = read_pr_url(epic_id, args.output_dir) @@ -879,7 +927,8 @@ def process_strategy(strategy_key, server, user, token, args): continue ok, _ = transition_issue( - server, user, token, epic_id, "In Progress") + server, user, token, epic_id, "In Progress", + epic_tasks_dir_for(args)) epic_transitions.append({ "to": "In Progress", "success": ok}) assign_issue(server, user, token, epic_id, @@ -892,7 +941,8 @@ def process_strategy(strategy_key, server, user, token, args): if success: results[PROCESSED].append((epic_id, "codegen completed")) ok, _ = transition_issue( - server, user, token, epic_id, "Review") + server, user, token, epic_id, "Review", + epic_tasks_dir_for(args)) epic_transitions.append({ "to": "Review", "success": ok}) @@ -904,7 +954,8 @@ def process_strategy(strategy_key, server, user, token, args): results[FAILED].append((epic_id, "codegen failed")) if original_status: ok, _ = transition_issue( - server, user, token, epic_id, original_status) + server, user, token, epic_id, original_status, + epic_tasks_dir_for(args)) epic_transitions.append({ "to": original_status, "success": ok}) @@ -1365,7 +1416,8 @@ def _ci_handle_ready(epic, state, args, server, user, token): if state.pop("tooling_missing", None): save_epic_state(args.data_repo, epic["strategy_key"], epic_id, state) - transition_issue(server, user, token, epic_id, "In Progress") + transition_issue(server, user, token, epic_id, "In Progress", + epic_tasks_dir_for(args)) assign_issue(server, user, token, epic_id, AUTOMATIONBOT_ACCOUNT_ID) state["status"] = "Generating" @@ -1401,12 +1453,29 @@ def _ci_handle_ready(epic, state, args, server, user, token): f"Codegen v{state['current_version']}; {detail}" return PROCESSED, "Ready", "ReviewPending", \ f"Codegen v{state['current_version']} completed" - else: - state["status"] = "Failed" - state["failure_reason"] = "codegen failed" + # The skill declining to start is not the skill failing. It reports + # `blocked` when a dependency is not done, which a later run can satisfy — + # so the epic goes back to Blocked and keeps its turn. Writing Failed here + # is terminal (CI_TERMINAL_STATES), and cost RHAI-761 every future run + # until its state was edited by hand. + if state.get("codegen_outcome") == "blocked": + state["status"] = "Blocked" + state.pop("failure_reason", None) + blocked_by = epic.get("dependencies") or state.get("blocked_by") or [] + if blocked_by: + state["blocked_by"] = blocked_by save_epic_state( args.data_repo, epic["strategy_key"], epic_id, state) - return FAILED, "Ready", "Failed", "codegen failed" + detail = "codegen declined: dependencies not done" + if blocked_by: + detail = f"{detail} ({', '.join(blocked_by)})" + return BLOCKED, "Ready", "Blocked", detail + + state["status"] = "Failed" + state["failure_reason"] = "codegen failed" + save_epic_state( + args.data_repo, epic["strategy_key"], epic_id, state) + return FAILED, "Ready", "Failed", "codegen failed" def _pr_is_live(pr_url): @@ -1493,7 +1562,8 @@ def _ci_handle_review_pending(epic, state, args, server, user, token): save_epic_state( args.data_repo, epic["strategy_key"], epic_id, state) - transition_issue(server, user, token, epic_id, "Review") + transition_issue(server, user, token, epic_id, "Review", + epic_tasks_dir_for(args)) link_pr_to_jira(server, user, token, epic_id, pr_url) return PROCESSED, "ReviewPending", "PRCreated", \ f"PR created (avg={avg:.1f})" @@ -1518,7 +1588,8 @@ def _ci_handle_review_pending(epic, state, args, server, user, token): save_epic_state( args.data_repo, epic["strategy_key"], epic_id, state) - transition_issue(server, user, token, epic_id, "Review") + transition_issue(server, user, token, epic_id, "Review", + epic_tasks_dir_for(args)) link_pr_to_jira(server, user, token, epic_id, pr_url) return PROCESSED, "ReviewPending", "PRCreated", \ f"Near-miss PR created (avg={avg:.1f})" @@ -1594,7 +1665,8 @@ def _ci_handle_pr_created(epic, state, args, server, user, token): args.data_repo, epic["strategy_key"], epic_id, state) if new_state == "Done": - transition_issue(server, user, token, epic_id, "Done") + transition_issue(server, user, token, epic_id, "Done", + epic_tasks_dir_for(args)) return PROCESSED, "PRCreated", "Done", "PR merged" elif new_state == "PRChangesRequested": return _ci_handle_pr_changes( @@ -1610,7 +1682,8 @@ def _ci_handle_pr_created(epic, state, args, server, user, token): state["pr_state"] = "merged" save_epic_state( args.data_repo, epic["strategy_key"], epic_id, state) - transition_issue(server, user, token, epic_id, "Done") + transition_issue(server, user, token, epic_id, "Done", + epic_tasks_dir_for(args)) return PROCESSED, "PRCreated", "Done", "PR merged (gh fallback)" return SKIPPED, "PRCreated", "PRCreated", "PR still open" diff --git a/tests/test_artifact_utils.py b/tests/test_artifact_utils.py index df7413e..946dc9a 100644 --- a/tests/test_artifact_utils.py +++ b/tests/test_artifact_utils.py @@ -477,6 +477,17 @@ def test_legacy_status_holding_an_outcome(self): def test_ci_state_is_not_an_outcome(self): assert read_codegen_outcome({"status": "PRCreated"}) is None + def test_blocked_ci_state_is_not_the_blocked_outcome(self): + """`Blocked` the CI state and `blocked` the outcome are different things. + + The state means "waiting on a dependency, per the DAG"; the outcome + means "the skill was invoked and declined to generate". They must not + be read as each other, or a legacy state file would look like a + declined run. + """ + assert read_codegen_outcome({"status": "Blocked"}) is None + assert read_codegen_outcome({"codegen_outcome": "blocked"}) == "blocked" + def test_missing(self): assert read_codegen_outcome({}) is None assert read_codegen_outcome(None) is None diff --git a/tests/test_ci_mode.py b/tests/test_ci_mode.py index ea995e6..d2c6f5d 100644 --- a/tests/test_ci_mode.py +++ b/tests/test_ci_mode.py @@ -1111,3 +1111,70 @@ def test_main_exits_nonzero_when_an_epic_fails(self, tmp_path, ]) assert rc == 1 + + +class TestCodegenDeclinedIsNotFailed: + """RHAI-761: declining to start must leave the epic retryable. + + `Failed` is terminal (CI_TERMINAL_STATES), so recording a refusal as a + failure skips the epic on every future run until its state is edited by + hand. A dependency that is not done yet is precisely the case a later run + fixes by itself. + """ + + def _run(self, tmp_path, monkeypatch, outcome, deps=None): + def fake_copy(data_repo, strategy_key, epic_id, output_dir, + state=None): + # The real one folds the skill's run-metadata into live state. + if state is not None and outcome is not None: + state["codegen_outcome"] = outcome + + monkeypatch.setattr( + "run_pipeline.generate_epic_task_from_jira", lambda *a, **k: None) + monkeypatch.setattr( + "run_pipeline.fetch_strategy", lambda *a, **k: None) + monkeypatch.setattr( + "run_pipeline.setup_target_repo", lambda *a, **k: True) + monkeypatch.setattr("run_pipeline._check_toolchain", + lambda *a, **k: {"ok": True, "missing": []}) + monkeypatch.setattr("run_pipeline.transition_issue", + lambda *a, **k: (True, "")) + monkeypatch.setattr("run_pipeline.assign_issue", lambda *a, **k: None) + monkeypatch.setattr("run_pipeline.invoke_codegen", + lambda *a, **k: False) + monkeypatch.setattr( + "run_pipeline._copy_codegen_artifacts_to_data_repo", fake_copy) + + epic = _epic("RHAI-761", deps=deps) + state = {"status": "Ready", "current_version": 0} + result = ci_process_epic( + epic, state, _args(tmp_path), "srv", "usr", "tok") + return result, load_epic_state(tmp_path, "RHAISTRAT-1", "RHAI-761") + + def test_blocked_outcome_returns_to_blocked(self, tmp_path, monkeypatch): + (action, _, to_state, detail), saved = self._run( + tmp_path, monkeypatch, "blocked", deps=["RHAI-760"]) + + assert action == BLOCKED + assert to_state == "Blocked" + assert saved["status"] == "Blocked" + assert saved["blocked_by"] == ["RHAI-760"] + assert "failure_reason" not in saved + assert "RHAI-760" in detail + + def test_genuine_failure_is_still_failed(self, tmp_path, monkeypatch): + (action, _, to_state, _), saved = self._run( + tmp_path, monkeypatch, "failed") + + assert action == FAILED + assert to_state == "Failed" + assert saved["status"] == "Failed" + assert saved["failure_reason"] == "codegen failed" + + def test_no_outcome_recorded_is_still_failed(self, tmp_path, monkeypatch): + """A skill that dies without writing anything is a real failure.""" + (action, _, to_state, _), saved = self._run( + tmp_path, monkeypatch, None) + + assert action == FAILED + assert saved["status"] == "Failed" diff --git a/tests/test_run_pipeline.py b/tests/test_run_pipeline.py index 7600989..cc13e47 100644 --- a/tests/test_run_pipeline.py +++ b/tests/test_run_pipeline.py @@ -10,6 +10,8 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "scripts")) +from artifact_utils import read_frontmatter +from check_dependencies import check_dependencies from fetch_jira_epics import SKIP_LABEL from run_pipeline import ( AUTOMATIONBOT_ACCOUNT_ID, @@ -29,6 +31,7 @@ read_pr_url, resolve_repo_via_llm, resolve_target_repo, + sync_epic_task_jira_status, transition_issue, invoke_codegen, setup_target_repo, @@ -41,6 +44,11 @@ # ─── Helpers ────────────────────────────────────────────────────────────────── +# Epic-level transitions carry the epic-tasks dir so the snapshot is refreshed +# alongside Jira; it derives from the args output_dir the tests below pass. +_TASKS_DIR = os.path.join("/tmp/test-artifacts", "epic-tasks") + + def _epic(epic_id, jira_status="New", dependencies=None, blocks=None, title=None, jira_labels=None): """Build a minimal epic data dict.""" @@ -1037,7 +1045,8 @@ def test_transitions_to_in_progress_before_codegen( calls = mock_trans.call_args_list assert calls[0] == mock.call("s", "u", "t", "RHAISTRAT-1", "In Progress") - assert calls[1] == mock.call("s", "u", "t", "RHAI-1", "In Progress") + assert calls[1] == mock.call( + "s", "u", "t", "RHAI-1", "In Progress", _TASKS_DIR) @mock.patch("run_pipeline.assign_issue") @mock.patch("run_pipeline.transition_issue") @@ -1060,7 +1069,8 @@ def test_transitions_to_in_review_after_success( calls = mock_trans.call_args_list assert len(calls) == 3 - assert calls[2] == mock.call("s", "u", "t", "RHAI-1", "Review") + assert calls[2] == mock.call( + "s", "u", "t", "RHAI-1", "Review", _TASKS_DIR) @mock.patch("run_pipeline.assign_issue") @mock.patch("run_pipeline.transition_issue") @@ -1084,8 +1094,9 @@ def test_failure_rolls_back_to_original_status( calls = mock_trans.call_args_list assert len(calls) == 3 assert calls[0] == mock.call("s", "u", "t", "RHAISTRAT-1", "In Progress") - assert calls[1] == mock.call("s", "u", "t", "RHAI-1", "In Progress") - assert calls[2] == mock.call("s", "u", "t", "RHAI-1", "New") + assert calls[1] == mock.call( + "s", "u", "t", "RHAI-1", "In Progress", _TASKS_DIR) + assert calls[2] == mock.call("s", "u", "t", "RHAI-1", "New", _TASKS_DIR) @mock.patch("run_pipeline.assign_issue") @mock.patch("run_pipeline.transition_issue") @@ -1590,7 +1601,7 @@ def test_merged_pr_transitions_to_done( _, results, transitions_log, _ = process_strategy( "RHAISTRAT-1", "s", "u", "t", args) - mock_trans.assert_any_call("s", "u", "t", "RHAI-1", "Done") + mock_trans.assert_any_call("s", "u", "t", "RHAI-1", "Done", _TASKS_DIR) assert any("merged" in s[1].lower() for s in results[SKIPPED] if s[0] == "RHAI-1") assert "RHAI-1" in transitions_log @@ -1720,3 +1731,96 @@ def test_dry_run_skips_assignment_and_strat_transition( mock_assign.assert_not_called() mock_trans.assert_not_called() + + +# ─── Mid-run Jira transitions must reach the epic-task snapshot ───────────── + +class TestSyncEpicTaskJiraStatus: + """RHAI-761: a transition this run makes must be visible to this run.""" + + _TRANSITIONS = [{"id": "31", "to": {"name": "Closed"}}] + + def _write_epic_task(self, tasks_dir, epic_id, jira_status): + os.makedirs(tasks_dir, exist_ok=True) + path = os.path.join(tasks_dir, f"{epic_id}.md") + with open(path, "w") as f: + f.write("---\n" + f"epic_id: {epic_id}\n" + f"title: Epic {epic_id}\n" + "strategy_key: RHAISTRAT-2671\n" + "target_repo: rh-forge/rh-forge-ui\n" + "status: Pending\n" + f"jira_status: {jira_status}\n" + "dependencies: []\n" + "---\n\n" + f"# {epic_id}\n") + return path + + def test_writes_new_status_to_epic_task(self, tmp_path): + tasks = str(tmp_path / "epic-tasks") + path = self._write_epic_task(tasks, "RHAI-760", "In Progress") + + assert sync_epic_task_jira_status("RHAI-760", "Closed", tasks) is True + assert read_frontmatter(path)[0]["jira_status"] == "Closed" + + def test_missing_epic_task_is_not_an_error(self, tmp_path): + # Strategy keys have no epic-task file. + assert sync_epic_task_jira_status( + "RHAISTRAT-2671", "In Progress", str(tmp_path)) is False + + def test_no_dir_is_not_an_error(self): + assert sync_epic_task_jira_status("RHAI-760", "Closed", None) is False + + @mock.patch("run_pipeline.do_transition") + @mock.patch("run_pipeline.get_transitions") + def test_transition_issue_syncs_the_snapshot(self, mock_get, mock_do, + tmp_path): + mock_get.return_value = self._TRANSITIONS + tasks = str(tmp_path / "epic-tasks") + path = self._write_epic_task(tasks, "RHAI-760", "In Progress") + + ok, to_name = transition_issue( + "s", "u", "t", "RHAI-760", "Done", tasks) + + assert (ok, to_name) == (True, "Closed") + assert read_frontmatter(path)[0]["jira_status"] == "Closed" + + @mock.patch("run_pipeline.do_transition", + side_effect=Exception("403 Forbidden")) + @mock.patch("run_pipeline.get_transitions") + def test_failed_transition_leaves_snapshot_alone(self, mock_get, mock_do, + tmp_path): + """A status we did not actually reach must not be written down.""" + mock_get.return_value = self._TRANSITIONS + tasks = str(tmp_path / "epic-tasks") + path = self._write_epic_task(tasks, "RHAI-760", "In Progress") + + ok, _ = transition_issue("s", "u", "t", "RHAI-760", "Done", tasks) + + assert ok is False + assert read_frontmatter(path)[0]["jira_status"] == "In Progress" + + @mock.patch("run_pipeline.do_transition") + @mock.patch("run_pipeline.get_transitions") + def test_dependent_gate_passes_after_transition(self, mock_get, mock_do, + tmp_path): + """The end-to-end regression: close a dependency, then gate on it. + + Before the fix the pipeline closed RHAI-760 and invoked RHAI-761 in + the same run, and check_dependencies read a snapshot written before + the transition — so the dependent was refused work it could do. + """ + mock_get.return_value = self._TRANSITIONS + tasks = str(tmp_path / "epic-tasks") + self._write_epic_task(tasks, "RHAI-760", "In Progress") + with open(os.path.join(tasks, "RHAI-761.md"), "w") as f: + f.write("---\nepic_id: RHAI-761\ntitle: Epic RHAI-761\n" + "strategy_key: RHAISTRAT-2671\n" + "target_repo: rh-forge/rh-forge-ui\n" + "status: Pending\n" + "jira_status: New\ndependencies:\n - RHAI-760\n" + "---\n\n# RHAI-761\n") + + assert check_dependencies("RHAI-761", tasks)["all_done"] is False + transition_issue("s", "u", "t", "RHAI-760", "Done", tasks) + assert check_dependencies("RHAI-761", tasks)["all_done"] is True