IW-437: The phase CI gate finishes a phase without manual intervention #475

Merged

mph merged 40 commits from IW-437 into main 2026-08-28 07:55:46 +00:00

Conversation 0 Commits 40 Files changed 43 +6040 -270

mph commented 2026-08-28 07:22:08 +00:00

Owner

Copy link

Three defects in the phase CI gate. Together they made iw phase-pr unable to finish a phase on a repository whose pipeline runs longer than the poll window — which is this repository. The gate wrote a ci_failed card although nothing had failed, and a person had to merge by hand.

Each defect was reproduced with a failing test before it was fixed.

Defects fixed

1. phase-pr moved the branch head on an unfinished wait (phase 1, closes #445 #450)

markPending and heartbeat rewrite the card every poll, and ReviewStateUpdater.merge always refreshes last_updated, so the card was dirty on every exit. When the poll window ended before CI decided, finish committed and pushed it. The new head cancelled the run in flight (concurrency.cancel-in-progress) and started a fresh one, so a resumed wait could never converge. --batch reached the same end through CommitPolicy.CommitBeforeGate.

The card is no longer pushed on an unfinished wait; it stays uncommitted until the verdict is terminal. doBatchMerge now commits it immediately before the merge, keeping the clean tree the merge needs.

2. A 409 Conflict was not recognised as a duplicate pull request (phase 2, closes #431)

resolvePullRequest's fallback to the card's PR url was gated on the error prose matching "already exists". ForgejoClient collapsed every unhandled status into Forgejo API error: <code>, so a 409 never matched and the resume failed on this project's own forge.

The create error is now typed as ForgeCreateError, so a duplicate is recognised by what the forge said rather than by how an adapter phrased it.

3. The CI wait budget refused before it read the forge, and could not be reset (phase 3, closes #433)

CiGate.loop evaluated both budgets before any forge call, so a card carrying a spent budget returned TimedOut without a single API request — a terminal-green pipeline reported as a timeout. Separately, the start of the wait was derived by subtracting timeoutMs from ci_deadline, which markPending added straight back; timeoutMs cancelled, so --timeout was inert on exactly the invocation where a larger one is needed.

The forge is now read once first, a terminal verdict wins outright, and the budget is checked only on a Running verdict. ci_deadline is replaced by ci_wait_started, so this invocation's --timeout decides the budget.

Breaking change

ci_deadline is gone with no backward compatibility, by a decision recorded in analysis.md. A card still carrying it is rejected as an unrecognized root key. The field is only present during an active CI wait, so in-flight waits are what this affects; finish them before upgrading, or re-run the gate to write a fresh card.

Testing

• ./mill core.test — 186/186
• ./iw ./test — core 186, dashboard 241, dashboard.itest 240, BATS e2e 190/190

Every regression test was written RED first, and each assertion was proved fallible by mutating the fix and confirming the specific test went red. Four assertions that pinned the old contract ("the budget is checked before the forge is read") were deliberately inverted; each comment was rewritten rather than deleted.

Deliberately not fixed

Recorded in LESSONS.md and filed rather than folded in: the startOfWait fallback on a corrupt or future timestamp (#473, byte-for-byte unchanged from baseline), unenforced date-time validation on card timestamps (#474), the duplicate fallback reading a card keyed on the issue rather than the phase (#467), and unsanitized forge-controlled console output through the gh/glab adapters (#469).

PhaseMerge.isStateOnlyDelta is untouched and stays load-bearing: the terminal paths still push a card-only commit.

Release notes

project-management/issues/IW-437/release-notes.md (Czech, user-facing).

Closes #437
Closes #431
Closes #433
Closes #445
Closes #450

🤖 Generated with Claude Code

Three defects in the phase CI gate. Together they made `iw phase-pr` unable to finish a phase on a repository whose pipeline runs longer than the poll window — which is this repository. The gate wrote a `ci_failed` card although nothing had failed, and a person had to merge by hand. Each defect was reproduced with a failing test before it was fixed. ## Defects fixed **1. `phase-pr` moved the branch head on an unfinished wait** (phase 1, closes #445 #450) `markPending` and `heartbeat` rewrite the card every poll, and `ReviewStateUpdater.merge` always refreshes `last_updated`, so the card was dirty on every exit. When the poll window ended before CI decided, `finish` committed **and pushed** it. The new head cancelled the run in flight (`concurrency.cancel-in-progress`) and started a fresh one, so a resumed wait could never converge. `--batch` reached the same end through `CommitPolicy.CommitBeforeGate`. The card is no longer pushed on an unfinished wait; it stays uncommitted until the verdict is terminal. `doBatchMerge` now commits it immediately before the merge, keeping the clean tree the merge needs. **2. A 409 Conflict was not recognised as a duplicate pull request** (phase 2, closes #431) `resolvePullRequest`'s fallback to the card's PR url was gated on the error *prose* matching "already exists". `ForgejoClient` collapsed every unhandled status into `Forgejo API error: <code>`, so a 409 never matched and the resume failed on this project's own forge. The create error is now typed as `ForgeCreateError`, so a duplicate is recognised by what the forge said rather than by how an adapter phrased it. **3. The CI wait budget refused before it read the forge, and could not be reset** (phase 3, closes #433) `CiGate.loop` evaluated both budgets before any forge call, so a card carrying a spent budget returned `TimedOut` without a single API request — a terminal-green pipeline reported as a timeout. Separately, the start of the wait was *derived* by subtracting `timeoutMs` from `ci_deadline`, which `markPending` added straight back; `timeoutMs` cancelled, so `--timeout` was inert on exactly the invocation where a larger one is needed. The forge is now read once first, a terminal verdict wins outright, and the budget is checked only on a `Running` verdict. `ci_deadline` is replaced by `ci_wait_started`, so this invocation's `--timeout` decides the budget. ## Breaking change `ci_deadline` is gone with no backward compatibility, by a decision recorded in `analysis.md`. A card still carrying it is rejected as an unrecognized root key. The field is only present during an active CI wait, so in-flight waits are what this affects; finish them before upgrading, or re-run the gate to write a fresh card. ## Testing - `./mill core.test` — 186/186 - `./iw ./test` — core 186, dashboard 241, dashboard.itest 240, BATS e2e 190/190 Every regression test was written RED first, and each assertion was proved fallible by mutating the fix and confirming the specific test went red. Four assertions that pinned the *old* contract ("the budget is checked before the forge is read") were deliberately inverted; each comment was rewritten rather than deleted. ## Deliberately not fixed Recorded in `LESSONS.md` and filed rather than folded in: the `startOfWait` fallback on a corrupt or future timestamp (#473, byte-for-byte unchanged from baseline), unenforced `date-time` validation on card timestamps (#474), the duplicate fallback reading a card keyed on the issue rather than the phase (#467), and unsanitized forge-controlled console output through the `gh`/`glab` adapters (#469). `PhaseMerge.isStateOnlyDelta` is untouched and stays load-bearing: the terminal paths still push a card-only commit. ## Release notes `project-management/issues/IW-437/release-notes.md` (Czech, user-facing). Closes #437 Closes #431 Closes #433 Closes #445 Closes #450 🤖 Generated with Claude Code

mph added 39 commits 2026-08-28 07:22:08 +00:00

workflow(dx): Create diagnostic analysis for IW-437 ... 12dda25afd

Phase CI gate: phase-pr restarts CI on every resume, cannot resume on
Forgejo, and expires against a green pipeline

Artifacts:
- analysis.md

🤖 Generated with Claude Code

workflow: Resolve clarifications for IW-437 ... d041b9ccbf

Resolved:
- analysis.md: What should the CI wait budget measure? → take the
  recommendation in full — read the forge before refusing (D), keep
  elapsed wall-clock (A), replace ci_deadline with a stored wait-start,
  and drop the old field (no backward compatibility)
- analysis.md: Does wf-implement Step 3 need to tolerate a dirty card? →
  no. Step 2 stops on an open PR before Step 3 checks the tree, so no
  workflow step can meet the dirty card. No dev-docs change needed.

🤖 Generated with Claude Code

workflow(dx): Create task index for IW-437 ... 907e106b8c

Phase CI gate: phase-pr restarts CI on every resume, cannot resume on
Forgejo, and expires against a green pipeline

Artifacts:
- tasks.md (3 phases, sequenced defect 3 -> 1 -> 2)
- review-state.json (tasks_ready)
- analysis.md (total range corrected to 7.5-13h after the budget
  scope was settled)

🤖 Generated with Claude Code

Phase 1: context ready ... 0f64de4039

Issue: IW-437
Phase: 1 - phase-pr moves the branch head on an unfinished wait

This commit marks the checkpoint for phase 1 context generation.
Use dx-fix IW-437 to return to this point.

🤖 Generated with Claude Code

chore(IW-437): update review-state to context_ready f2d2301a5e

chore(IW-437): update review-state ca5d21b2b0

workflow(dx): Record IW-437 lessons (checkpoint format, stale citation) 3889e64fc5

workflow(dx): Generate Phase 1 tasks for IW-437 ... 918326c141

phase-pr moves the branch head on an unfinished wait

Artifacts:
- phase-01-tasks.md

🤖 Generated with Claude Code

chore(IW-437): update review-state to tasks_ready 0d6135cb37

chore(IW-437): update review-state for phase 01 2da359a1dd

phase-pr no longer moves the branch head on an unfinished wait ... 10d52e8f79

- Remove persist from the CiVerdict.Running case so the card stays uncommitted until the verdict is terminal
- Use CommitPolicy.DeferToCaller unconditionally in gate to close the --batch door
- Commit the card in doBatchMerge immediately before the merge so the checkout still meets a clean tree
- Add FakeTracker.onMergeSquashAndDelete so a test can prove the commit lands before the merge
- Add four regression tests: exit-3 non-batch and --batch make no commit; both --batch merge paths commit first

Merge pull request 'Phase 1: phase-pr no longer moves the branch head on an unfinished wait' (#448) from IW-437-phase-01 into IW-437 ... 13eec3ac20

Reviewed-on: #448

chore(IW-437): update review-state for phase 01 eafbebc99d

chore(IW-437): mark phase 01 complete in tasks.md 5be62b33a2

Phase 2: context ready ... b66e166ea6

Issue: IW-437
Phase: 2 - A 409 Conflict is not recognised as a duplicate pull request

This commit marks the checkpoint for phase 2 context generation.
Use dx-fix IW-437 to return to this point.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Pwo2RcHqPuziyjj49RMFp

chore(IW-437): update review-state to context_ready f294ed100a

chore(IW-437): update review-state daec82db92

workflow(dx): Record checkpoint-format recurrence at IW-437 phase 2 ... 37e5696cc5

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Pwo2RcHqPuziyjj49RMFp

workflow(dx): Generate Phase 2 tasks for IW-437 ... 449824b15e

A 409 Conflict is not recognised as a duplicate pull request

Artifacts:
- phase-02-tasks.md

🤖 Generated with Claude Code

chore(IW-437): update review-state to tasks_ready e6946d2213

chore(IW-437): update review-state for phase 02 7a6a4849a1

A 409 Conflict is not recognised as a duplicate pull request ... e7d20f56b0

- Type the createPullRequest error channel as ForgeCreateError, so a duplicate
  is recognised by what the forge said and not by how the adapter phrased it
- Classify Forgejo's 409 by status alone - v16.0.1 answers it only for a duplicate
- Move the gh/glab prose match into the adapters that own each CLI's vocabulary
- Stop claiming manual PR creation is the next step on a failure that is not a duplicate
- Sanitise and cap the response body that reaches the terminal

Issue: IW-437
Phase: 2 - A 409 Conflict is not recognised as a duplicate pull request

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Pwo2RcHqPuziyjj49RMFp

chore(IW-437): update review-state for phase 02 91e79c6747

Merge pull request 'Phase 2: A 409 Conflict is not recognised as a duplicate pull request' (#454) from IW-437-phase-02 into IW-437 ... 66c9e8ac50

Reviewed-on: #454

chore(IW-437): update review-state for phase 02 e7aa75fb15

chore(IW-437): mark phase 02 complete in tasks.md ed21f58b5d

Phase 3: context ready ... ceb861057e

Issue: IW-437
Phase: 3 - The CI wait budget refuses before it reads the forge, and cannot be reset

This commit marks the checkpoint for phase 3 context generation.
Use dx-fix IW-437 to return to this point.

🤖 Generated with Claude Code

chore(IW-437): update review-state to context_ready 349a04a760

chore(IW-437): restore lost phase checkpoints, record the cause ... 25162b33c6

--checkpoint replaces the whole phase_checkpoints map, so each phase's
Step 6a update destroyed the previous phase's entry. Phase 1 and phase 2
were read back out of the card's git history and re-appended with
--append-checkpoint. Recorded as LESSONS entry 11.

🤖 Generated with Claude Code

workflow(dx): Generate Phase 3 tasks for IW-437 ... 567da556c5

The CI wait budget refuses before it reads the forge, and cannot be reset

Artifacts:
- phase-03-tasks.md

🤖 Generated with Claude Code

chore(IW-437): update review-state to tasks_ready 81826a32f0

chore(IW-437): update review-state for phase 03 01f0353075

The CI wait budget reads the forge before it refuses, and a resume can extend it ... 906a72bfbc

- CiGate.loop reads the forge once first and lets a terminal verdict win over a spent budget
- ci_deadline replaced by ci_wait_started so elapsed wall-clock is stored and --timeout works on a resume
- Four deliberate assertions inverted with rewritten comments - analysis.md and tasks.md both miscount them as three

chore(IW-437): update review-state for phase 03 fefb00d840

Merge pull request 'Phase 3: The CI wait budget reads the forge before it refuses, and a resume can extend it' (#458) from IW-437-phase-03 into IW-437 ... a23c415369

Reviewed-on: #458

chore(IW-437): update review-state for phase 03 a71cef4792

chore(IW-437): mark phase 03 complete in tasks.md 832b9238f0

docs(IW-437): add release notes 55199a4386

workflow: Triage lessons for IW-437 ... 09879b8d4b

Promoted:
- dx-fix checkpoint snippet (entries 1, 11) -> dev-docs#187; CLI half already
  tracked as iw-cli#436, evidence added there
- Code review runs on uncommitted work (entry 3) -> dev-docs#188
- Push failure drops the hook's stderr (entry 4) -> iw-cli#465
- ./iw ./test green on unpushable code (entry 5, in part) -> iw-cli#466
- Duplicate fallback can return an earlier phase's PR url (entry 7) -> iw-cli#467
- Non-deterministic tmux BATS tests (entry 8) -> iw-cli#468
- Unsanitized forge-controlled console output (entry 9) -> iw-cli#469
- phase-commit --items cannot carry a comma (entry 10) -> iw-cli#471
- Re-derive counts and ranges from source (entries 6, 12) -> dev-docs note #192
- Corrupt or future ci_wait_started disables the budget (entry 14) -> iw-cli#473
- Timestamp fields never parsed against the schema (entry 15) -> iw-cli#474

Rejected:
- Stale wf-implement line citation (entry 2): the claim is unaffected
- Review skills should read .scalafix.conf (entry 5, kanon half): lint config is
  a project-level decision, not a shared guide's business
- Contradictory acceptance criteria (entry 13): the two bullets are sequential,
  not contradictory

Retained:
- Stale citations in this issue's own planning documents (entry 6)

Tracker hygiene done during triage: iw-cli#464 and #446 closed as duplicates of
#436; dev-docs#187 cross-linked with note #183.

🤖 Generated with Claude Code

mph added 1 commit 2026-08-28 07:23:10 +00:00

chore(IW-437): update review-state to all_complete 527e12a99e

mph merged commit 4a102239e9 into main 2026-08-28 07:55:46 +00:00

mph referenced this pull request from a commit 2026-08-28 07:55:48 +00:00

Merge pull request 'IW-437: The phase CI gate finishes a phase without manual intervention' (#475) from IW-437 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

agent-ready

  

bug

Something isn't working

  

contract

Run the tool-contract suite on this PR

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

needs-human

  

needs-input

Intake asked the human a question on the issue; waiting for the answer

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

agent-ready

bug

contract

documentation

duplicate

enhancement

good first issue

help wanted

invalid

needs-human

needs-input

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

iterative-works/iw-cli!475

No description provided.