Skip to content

Commit e8b900b

Browse files
committed
fix crane completion gate staleness
1 parent da06413 commit e8b900b

6 files changed

Lines changed: 209 additions & 116 deletions

File tree

.github/aw/actions-lock.json

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,10 @@
3535
"version": "v7",
3636
"sha": "043fb46d1a93c77aae656e7c1c64a875d1fc6a0a"
3737
},
38-
"github/gh-aw-actions/setup-cli@v0.74.4": {
39-
"repo": "github/gh-aw-actions/setup-cli",
40-
"version": "v0.74.4",
41-
"sha": "d3abfe96a194bce3a523ed2093ddedd5704cdf62"
42-
},
43-
"github/gh-aw-actions/setup@v0.74.4": {
38+
"github/gh-aw-actions/setup@v0.77.5": {
4439
"repo": "github/gh-aw-actions/setup",
45-
"version": "v0.74.4",
46-
"sha": "d3abfe96a194bce3a523ed2093ddedd5704cdf62"
40+
"version": "v0.77.5",
41+
"sha": "3ea13c02d765410340d533515cb31a7eef2baaf0"
4742
},
4843
"github/gh-aw/actions/setup@v0.50.6": {
4944
"repo": "github/gh-aw/actions/setup",

.github/workflows/crane.lock.yml

Lines changed: 129 additions & 89 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/crane.md

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ The pre-step fetches open issues with the `crane-migration` label via the GitHub
247247
When a migration is issue-based, `/tmp/gh-aw/crane.json` includes:
248248
- **`selected_issue`**: The issue number (e.g., `42`) if the selected migration came from an issue, or `null` if it came from a file.
249249
- **`issue_migrations`**: A mapping of migration name -> issue number for all issue-based migrations found.
250-
- **`stale_completed_state`**: Issue-based migrations whose completion state is not trustworthy. This includes active issues that still have `crane-migration` even though repo-memory says `Completed: true`, and completed-label issues whose current PR-head completion gate did not pass.
250+
- **`stale_completed_state`**: Issue-based migrations whose completion state is not trustworthy. This includes active issues that still have `crane-migration` even though repo-memory says `Completed: true`, and completed-label issues whose current up-to-date PR-head completion gate did not pass.
251251

252252
### Reading Migrations
253253

@@ -261,7 +261,7 @@ The pre-step has already determined which migration to run. Read `/tmp/gh-aw/cra
261261
- **`selected_strategy`**: The `strategy` value from the migration's frontmatter -- one of `"in-place"`, `"greenfield"`, or `"auto"`. If `"auto"`, the agent must pick on the first iteration and write the chosen strategy back into the state file's Machine State table.
262262
- **`state_file_size_bytes`** / **`state_file_max_bytes`**: For rolling-compaction decisions (see [Update Rules](#update-rules)).
263263
- **`issue_migrations`**: A mapping of migration name -> issue number for all discovered issue-based migrations.
264-
- **`stale_completed_state`**: A list of issue-based migrations where repo-memory still says `Completed: true`, but the issue label state or PR-head completion gate says the migration is not safely complete; treat this as stale memory, not as permission to finish.
264+
- **`stale_completed_state`**: A list of issue-based migrations where repo-memory still says `Completed: true`, but the issue label state or up-to-date PR-head completion gate says the migration is not safely complete; treat this as stale memory, not as permission to finish.
265265
- **`deferred`**: Other migrations that were due but will be handled in future runs.
266266
- **`unconfigured`**: Migrations that still have the sentinel or placeholder content.
267267
- **`skipped`**: Migrations not due yet based on their per-migration schedule, or completed/paused.
@@ -277,7 +277,7 @@ If `selected` is not null:
277277
4. Read the state file `{selected}.md` from the repo-memory folder. This contains the Machine State table, the Migration Plan, lessons, blockers, and iteration history.
278278
5. If `selected_issue` is not null, also read the issue comments for any human steering input.
279279
6. If `selected` appears in `stale_completed_state`, ignore any pre-existing `Completed: true`, `Completed Reason`, target-satisfying `best_metric`, or `crane-completed` label as a completion signal. Restore the issue to active migration state by ensuring `crane-migration` is present and `crane-completed` is absent unless the deterministic completion gate passes later in this run. First run the current verification contract and only enter the completion-candidate path after a fresh accepted iteration satisfies the target metric.
280-
7. Before making code changes, inspect the state file's `Completion Candidate` field. If it is `true`, run the deterministic completion gate in [Halting Condition](#halting-condition). Only finalize completion if the current PR head checks pass. If the gate is failing, fix the failing PR checks instead of starting unrelated migration work. If the gate is pending or unavailable, leave the issue active and update `Completion Gate Status`.
280+
7. Before making code changes, inspect the state file's `Completion Candidate` field. If it is `true`, run the deterministic completion gate in [Halting Condition](#halting-condition). Only finalize completion if the current PR head contains the current base branch SHA and the current PR head checks pass. If the gate is failing, stale, behind, or diverged, fix the PR branch/checks instead of starting unrelated migration work. If the gate is pending or unavailable, leave the issue active and update `Completion Gate Status`.
281281

282282
## Multiple Migrations
283283

@@ -617,7 +617,7 @@ If `status == "failure"`, **fix and retry -- do not revert, do not accept**:
617617
- Update **[docs] Lessons Learned** if this iteration revealed something new (e.g. a bridging trick, a parity surprise, a perf trap).
618618
- Update **[scope] Future Work** if this iteration opened new threads.
619619
5. **Update the migration issue**: edit the status comment and post a per-iteration comment using the same shared accepted iteration summary.
620-
6. **Check halting condition** (see [Halting Condition](#halting-condition)): if `target-metric` is set, compare the new `best_metric` from this accepted iteration against it. For `higher` direction, the target is reached when `best_metric >= target-metric`. Reaching the target metric does **not** complete the migration in this run. It creates a completion candidate: set `Completion Candidate: true`, set `Completion Gate: pr-head-checks`, set `Completion Gate Status: pending`, keep `Completed: false`, keep `Completed Reason: --`, and leave the `crane-migration` label on the issue. Completion is finalized only by a later run after the pushed PR head's deterministic checks are observed green.
620+
6. **Check halting condition** (see [Halting Condition](#halting-condition)): if `target-metric` is set, compare the new `best_metric` from this accepted iteration against it. For `higher` direction, the target is reached when `best_metric >= target-metric`. Reaching the target metric does **not** complete the migration in this run. It creates a completion candidate: set `Completion Candidate: true`, set `Completion Gate: up-to-date-pr-head-checks`, set `Completion Gate Status: pending`, keep `Completed: false`, keep `Completed Reason: --`, and leave the `crane-migration` label on the issue. Completion is finalized only by a later run after the pushed PR head contains the current base SHA and its deterministic checks are observed green.
621621

622622
**If the score did not improve**:
623623
1. Discard the code changes (do not commit them to the long-running branch).
@@ -727,7 +727,7 @@ After **every iteration** (accepted, rejected, or error), post a **new comment**
727727

728728
## Halting Condition
729729

730-
Migrations are usually **goal-oriented** -- you want to finish. Set `target-metric: 1.0` in the frontmatter to nominate a migration for completion once the health score reaches 1.0 (which, with the recommended `correctness x progress` convention, means "fully migrated and verified"). The metric is necessary but not sufficient: final completion always requires a deterministic PR-head completion gate.
730+
Migrations are usually **goal-oriented** -- you want to finish. Set `target-metric: 1.0` in the frontmatter to nominate a migration for completion once the health score reaches 1.0 (which, with the recommended `correctness x progress` convention, means "fully migrated and verified"). The metric is necessary but not sufficient: final completion always requires a deterministic up-to-date PR-head completion gate.
731731

732732
### How It Works
733733

@@ -737,29 +737,30 @@ Migrations are usually **goal-oriented** -- you want to finish. Set `target-metr
737737
4. For `lower` direction: the target is reached when `best_metric <= target-metric`.
738738
5. When the target is reached, create a completion candidate instead of completing immediately:
739739
- Set `Completion Candidate: true`.
740-
- Set `Completion Gate: pr-head-checks`.
740+
- Set `Completion Gate: up-to-date-pr-head-checks`.
741741
- Set `Completion Gate Status: pending`.
742742
- Keep `Completed: false` and `Completed Reason: --`.
743743
- For issue-based migrations, keep the `crane-migration` label and do not add `crane-completed`.
744744
- Update the status comment to Active with a note that the migration is waiting on deterministic PR-head checks.
745745
6. On the next run while `Completion Candidate: true`, run the deterministic completion gate before making code changes:
746746
- Resolve the migration PR from `existing_pr` or the state file's `PR` field.
747-
- Query the PR's current head SHA via the GitHub API.
747+
- Query the PR's current head SHA and current base SHA via the GitHub API.
748+
- Query `GET /repos/{owner}/{repo}/compare/{base_sha}...{head_sha}`. The gate is stale unless the compare status is `ahead` or `identical`. If it is `behind`, `diverged`, or unavailable, merge/rebase the current base into the PR branch, push the PR branch, and wait for fresh checks. Do not mark the migration complete from checks produced before the current base SHA was included.
748749
- Query check-runs/check-suites for that exact PR head SHA, or use `gh pr checks "$PR" --json name,conclusion,state,startedAt,completedAt`.
749-
- The gate passes only if every check for the current PR head is terminal success. Treat missing checks, pending checks, queued checks, failing checks, cancelled checks, timed-out checks, stale checks, and action-required checks as not passing.
750+
- The gate passes only if every check for the current up-to-date PR head is terminal success. Treat missing checks, pending checks, queued checks, failing checks, cancelled checks, timed-out checks, stale checks, report-mode migration checks, and action-required checks as not passing.
750751
- If the gate is pending or missing, leave `Completion Candidate: true`, keep `Completed: false`, ensure `crane-migration` is present, ensure `crane-completed` is absent, set `Completion Gate Status: pending:<sha or reason>`, and end without completing.
751752
- If the gate fails, leave `Completion Candidate: true`, keep `Completed: false`, ensure `crane-migration` is present, ensure `crane-completed` is absent, set `Completion Gate Status: failing:<signature>`, and fix the failing PR checks.
752753
7. Only when the deterministic completion gate passes:
753754
- Set `Completed: true` in the Machine State table.
754-
- Set `Completed Reason` to a human-readable message that includes both the target metric and PR-head check evidence (e.g., `target metric 1.0 reached; PR #123 head abc1234 checks passed`).
755+
- Set `Completed Reason` to a human-readable message that includes the target metric, PR head SHA, current base SHA, and check evidence (e.g., `target metric 1.0 reached; PR #123 head abc1234 contains base def5678 and strict checks passed`).
755756
- Set `Completion Candidate: false`.
756757
- Set `Completion Gate Status: passed:<sha>`.
757758
- **For issue-based migrations**: remove the `crane-migration` label, add the `crane-completed` label.
758759
- Update the status comment to [+] Completed.
759760
- Post a celebratory per-iteration comment: `[+] **Migration complete!** {source} -> {target} finished after {N} iterations.`
760761
- The migration will not be selected for future runs.
761762

762-
Do not enter final completion from repo-memory alone. A stored `Completed: true`, old `Completed Reason`, historical `best_metric`, or same-run sandbox score is only evidence about a previous or local run. Final completion requires deterministic evidence from the current PR head after safe outputs have pushed the branch and GitHub Actions has reported the head checks.
763+
Do not enter final completion from repo-memory alone. A stored `Completed: true`, old `Completed Reason`, historical `best_metric`, or same-run sandbox score is only evidence about a previous or local run. Final completion requires deterministic evidence from the current PR head after safe outputs have pushed a branch that contains the current base SHA and GitHub Actions has reported strict checks for that up-to-date head.
763764

764765
### Open-Ended Migrations
765766

@@ -810,7 +811,7 @@ When creating or updating a migration's state file, use this structure:
810811
| Completed | false |
811812
| Completed Reason | -- |
812813
| Completion Candidate | false |
813-
| Completion Gate | pr-head-checks |
814+
| Completion Gate | up-to-date-pr-head-checks |
814815
| Completion Gate Status | -- |
815816
| Consecutive Errors | 0 |
816817
| Recent Statuses | -- |
@@ -911,7 +912,7 @@ All iterations in reverse chronological order (newest first).
911912
| Completed | `true` or `false` | Whether the deterministic completion gate has passed and the migration is final |
912913
| Completed Reason | text or `--` | e.g., `target metric 1.0 reached; PR #123 head abc1234 checks passed` |
913914
| Completion Candidate | `true` or `false` | Whether the target metric has been reached and the migration is waiting for deterministic PR-head checks |
914-
| Completion Gate | text | Deterministic gate required for final completion. Default: `pr-head-checks` |
915+
| Completion Gate | text | Deterministic gate required for final completion. Default: `up-to-date-pr-head-checks` |
915916
| Completion Gate Status | text or `--` | Latest gate result, such as `pending:<sha>`, `failing:<signature>`, or `passed:<sha>` |
916917
| Consecutive Errors | integer | Count of consecutive verification failures |
917918
| Recent Statuses | comma-separated | Last 10 outcomes: `accepted`, `rejected`, `error`, or `ci-fix-exhausted` |

.github/workflows/scripts/crane_scheduler.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,8 @@ def get_pr_head_check_gate(repo, pr_number, github_token, http_get_json=_http_ge
597597
"""Return ``(passed, reason)`` for the deterministic PR-head check gate.
598598
599599
``passed`` is:
600-
True - the current PR head has at least one check run and all are success
600+
True - the current PR head contains the current base and all check runs
601+
are success
601602
False - the PR exists, but checks are missing, pending, or failing
602603
None - the API could not provide enough data to decide
603604
"""
@@ -615,6 +616,26 @@ def get_pr_head_check_gate(repo, pr_number, github_token, http_get_json=_http_ge
615616
head_sha = head.get("sha") if isinstance(head, dict) else None
616617
if not head_sha:
617618
return None, "pr-head-sha-unavailable"
619+
base = pr_body.get("base") or {}
620+
base_sha = base.get("sha") if isinstance(base, dict) else None
621+
if not base_sha:
622+
return None, "pr-base-sha-unavailable:{}".format(head_sha[:12])
623+
624+
compare_url = "https://api.github.com/repos/{}/compare/{}...{}".format(
625+
repo,
626+
base_sha,
627+
head_sha,
628+
)
629+
compare_body, _ = http_get_json(compare_url, headers)
630+
if not isinstance(compare_body, dict):
631+
return None, "base-compare-unavailable:{}:{}".format(base_sha[:12], head_sha[:12])
632+
compare_status = compare_body.get("status")
633+
if compare_status not in {"ahead", "identical"}:
634+
return False, "stale-base:{}:{}:base:{}".format(
635+
head_sha[:12],
636+
compare_status or "unknown",
637+
base_sha[:12],
638+
)
618639

619640
check_runs = []
620641
next_url = "https://api.github.com/repos/{}/commits/{}/check-runs?per_page=100".format(

tests/unit/test_crane_scheduler.py

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,12 @@ def test_pr_head_gate_fails_when_any_check_is_not_success() -> None:
213213
def fake_http_get_json(url, _headers, timeout=30):
214214
del timeout
215215
if url.endswith("/pulls/102"):
216-
return {"head": {"sha": "abcdef1234567890"}}, None
216+
return {
217+
"base": {"sha": "1111111111111111"},
218+
"head": {"sha": "abcdef1234567890"},
219+
}, None
220+
if "/compare/1111111111111111...abcdef1234567890" in url:
221+
return {"status": "ahead"}, None
217222
if "/check-runs" in url:
218223
return {
219224
"check_runs": [
@@ -239,7 +244,12 @@ def test_pr_head_gate_passes_only_when_all_checks_succeed() -> None:
239244
def fake_http_get_json(url, _headers, timeout=30):
240245
del timeout
241246
if url.endswith("/pulls/102"):
242-
return {"head": {"sha": "abcdef1234567890"}}, None
247+
return {
248+
"base": {"sha": "1111111111111111"},
249+
"head": {"sha": "abcdef1234567890"},
250+
}, None
251+
if "/compare/1111111111111111...abcdef1234567890" in url:
252+
return {"status": "ahead"}, None
243253
if "/check-runs" in url:
244254
return {
245255
"check_runs": [
@@ -258,3 +268,29 @@ def fake_http_get_json(url, _headers, timeout=30):
258268

259269
assert passed is True
260270
assert reason == "passed:abcdef123456"
271+
272+
273+
def test_pr_head_gate_fails_when_pr_head_does_not_contain_current_base() -> None:
274+
def fake_http_get_json(url, _headers, timeout=30):
275+
del timeout
276+
if url.endswith("/pulls/117"):
277+
return {
278+
"base": {"sha": "e96b795d2540d4f991ee0c04cb4cfc67d8d74960"},
279+
"head": {"sha": "f02847267a544d9700e885f6edebe15f4c4aa406"},
280+
}, None
281+
if (
282+
"/compare/e96b795d2540d4f991ee0c04cb4cfc67d8d74960...f02847267a544d9700e885f6edebe15f4c4aa406"
283+
in url
284+
):
285+
return {"status": "diverged"}, None
286+
raise AssertionError(f"unexpected URL: {url}")
287+
288+
passed, reason = crane_scheduler.get_pr_head_check_gate(
289+
"githubnext/apm",
290+
117,
291+
"token",
292+
http_get_json=fake_http_get_json,
293+
)
294+
295+
assert passed is False
296+
assert reason == "stale-base:f02847267a54:diverged:base:e96b795d2540"

tests/unit/test_crane_workflow_prompt.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,17 @@ def test_crane_completion_is_two_phase_and_pr_head_gated() -> None:
4848

4949
assert "Reaching the target metric does **not** complete the migration in this run" in text
5050
assert "Completion Candidate: true" in text
51-
assert "Completion Gate: pr-head-checks" in text
51+
assert "Completion Gate: up-to-date-pr-head-checks" in text
5252
assert "leave the `crane-migration` label on the issue" in text
53-
assert "current PR head checks pass" in text
54-
assert "every check for the current PR head is terminal success" in text
53+
assert "current PR head contains the current base branch SHA" in text
54+
assert "every check for the current up-to-date PR head is terminal success" in text
5555
assert "Completion Gate Status: passed:<sha>" in text
5656

5757

5858
def test_crane_state_template_tracks_completion_candidate_gate() -> None:
5959
text = _workflow_text()
6060

6161
assert "| Completion Candidate | false |" in text
62-
assert "| Completion Gate | pr-head-checks |" in text
62+
assert "| Completion Gate | up-to-date-pr-head-checks |" in text
6363
assert "| Completion Gate Status | -- |" in text
6464
assert "Whether the target metric has been reached and the migration is waiting" in text

0 commit comments

Comments
 (0)