bug: manifest-precheck.sh reads local checkout, misses already-landed remote state (v0.10.2 cut re-run failure) #140

New issue

Closed

opened 2026-06-26 23:10:24 +02:00 by quartermaster · 0 comments

quartermaster commented

2026-06-26 23:10:24 +02:00

Owner

Empirical artifact

Task 13376 (release-toolkit v0.10.2 cut re-run) failed at the manifest direct-push step with non-fast-forward rejection:

[release-decide] mode=cut version=0.10.2
[draft-release] release v0.10.2 already exists at expected state — idempotent skip   ← #131 release pre-check WORKED
manifest update: path (α) direct-push
[main 4be9dbc] chore(manifest): post-cut bookkeeping for v0.10.2                     ← local commit created
hint: use 'git pull' before pushing again                                            ← push REJECTED
⚙️ [runner]: exitcode '1': failure

Root cause

scripts/manifest-precheck.sh (shipped in #136) reads .release-toolkit-manifest.json from the workflow's local checkout state (MANIFEST_PATH arg). This works for the canonical re-run case (operator manually re-runs the workflow on a fresh checkout that includes the prior run's manifest commit). But it MISSES the case where:

Initial cut workflow (task 13373 on 59e02780) substantively COMPLETED the cut (release-create + manifest direct-push to main)
Workflow got cancelled mid-flight (self-cancel pattern per #139)
Operator manually re-ran the cancelled task → task 13376
Re-run's checkout is at GITHUB_SHA = 59e02780 (the rolling-PR-merge SHA) — the manifest at that checkout is the pre-cut state (still pointing at v0.10.1's SHA)
Remote main has already advanced to 8e1c952 (the post-cut manifest commit landed by task 13373)
Pre-check sees local manifest with old last_released_sha, says "proceed"
Workflow creates a new manifest commit locally → push fails (non-fast-forward; remote is 1 commit ahead)

Why my #128/#131 idempotency design missed this

Surveyor 4a44's partial-completion matrix specifically called out: "the design must handle the partial-completion case... the exact scenario: manifest pushed + release published, THEN cancelled, THEN re-run." I correctly identified this in the PR body's matrix:

Release exists?	Manifest at HEAD_SHA?	Re-run action
Yes (matches)	Yes	Both skip; idempotent noop

But my implementation checked the local manifest for the "Yes" cell, not remote. The matrix logic was right; the source of "Manifest at HEAD_SHA" was wrong. Substrate-state-care failure: I checked the more-convenient source (local file) instead of the authoritative source (remote main).

The fix

Check remote main's manifest state as the primary source; fall back to local for cases where remote isn't accessible (test/first-cut/bootstrap).

# Remote-first check
git fetch --quiet origin main 2>/dev/null || true
REMOTE_CONTENT=$(git show "origin/main:$MANIFEST_PATH" 2>/dev/null || true)
if [[ -n "$REMOTE_CONTENT" ]]; then
    REMOTE_LAST=$(printf '%s' "$REMOTE_CONTENT" | jq -r '.last_released_sha // empty')
    if [[ -n "$REMOTE_LAST" && "$REMOTE_LAST" == "$HEAD_SHA" ]]; then
        printf 'skip\n'
        exit 0
    fi
fi
# Fall back to local checkout (original behavior + first-cut bootstrap)
# ... existing logic

Test seam MANIFEST_REMOTE_FIXTURE=/path/to/file lets bats simulate "remote main manifest content" without requiring an actual git fetch.

New bats cases needed

Remote at HEAD_SHA, local at older SHA → skip (the v0.10.2 bug case)
Remote at older SHA, local at older SHA → proceed (normal cut)
Remote manifest absent, local at HEAD_SHA → skip (fallback to local)
Remote manifest absent, local absent → proceed (first cut)
Remote unreachable (fixture file doesn't exist), local check governs → existing behavior preserved

Plus existing 9 tests preserved.

#128 / #131 (idempotent cut pipeline): this bug is a SUB-CASE of the partial-completion matrix that the original implementation missed. Closing this bug completes the idempotency design.
#139 (self-cancel noise): related but separate. #139 would make the cancellation rarer; this bug makes the re-run-after-cancellation correct. Both should ship; this one first.
#134 (workflow test seam, closed in #136): the extraction this builds on. The remote-aware check is a script-level change, fully bats-testable.

Severity

Real bug (not cosmetic). The v0.10.2 cut substantively succeeded (release published + manifest on main) but the operator's re-run after cancellation produced exit-1, exactly like v0.10.0 did pre-#131 — except with a different mechanism (non-fast-forward push instead of release-create 409). The fix completes the partial-completion-matrix design.

Implementation surface

scripts/manifest-precheck.sh: ~10 lines of additional logic (remote check + fallback) + test seam
tests/manifest-precheck.bats: 5 new tests for remote-aware behavior + adjust existing tests
No _release.yml change needed (script's interface unchanged)

Refs

Empirical artifact: task 13376 (re-run of v0.10.2 cut workflow), 2026-06-26 23:05
Operator engagement 2026-06-26 23:??: surfaced the re-run failure
The original design point: Surveyor 4a44's partial-completion matrix (which was correct; my implementation missed the remote-state check)
Closes the idempotency arc started by: #128

## Empirical artifact Task 13376 (release-toolkit v0.10.2 cut re-run) failed at the manifest direct-push step with `non-fast-forward` rejection: ``` [release-decide] mode=cut version=0.10.2 [draft-release] release v0.10.2 already exists at expected state — idempotent skip ← #131 release pre-check WORKED manifest update: path (α) direct-push [main 4be9dbc] chore(manifest): post-cut bookkeeping for v0.10.2 ← local commit created hint: use 'git pull' before pushing again ← push REJECTED ⚙️ [runner]: exitcode '1': failure ``` ## Root cause `scripts/manifest-precheck.sh` (shipped in [#136](https://git.frankenbit.de/frankenbit/release-toolkit/pulls/136)) reads `.release-toolkit-manifest.json` from the workflow's **local checkout state** (`MANIFEST_PATH` arg). This works for the canonical re-run case (operator manually re-runs the workflow on a fresh checkout that includes the prior run's manifest commit). But it MISSES the case where: 1. Initial cut workflow (task 13373 on `59e02780`) substantively COMPLETED the cut (release-create + manifest direct-push to main) 2. Workflow got cancelled mid-flight (self-cancel pattern per [#139](https://git.frankenbit.de/frankenbit/release-toolkit/issues/139)) 3. Operator manually re-ran the cancelled task → task 13376 4. Re-run's checkout is at `GITHUB_SHA = 59e02780` (the rolling-PR-merge SHA) — the manifest at that checkout is the **pre-cut state** (still pointing at v0.10.1's SHA) 5. Remote `main` has already advanced to `8e1c952` (the post-cut manifest commit landed by task 13373) 6. Pre-check sees local manifest with old `last_released_sha`, says "proceed" 7. Workflow creates a new manifest commit locally → push fails (non-fast-forward; remote is 1 commit ahead) ## Why my #128/#131 idempotency design missed this Surveyor 4a44's partial-completion matrix specifically called out: *"the design must handle the partial-completion case... the exact scenario: manifest pushed + release published, THEN cancelled, THEN re-run."* I correctly identified this in the PR body's matrix: | Release exists? | Manifest at HEAD_SHA? | Re-run action | |---|---|---| | Yes (matches) | **Yes** | Both skip; idempotent noop | But my implementation checked the **local manifest** for the "Yes" cell, not remote. The matrix logic was right; the source of "Manifest at HEAD_SHA" was wrong. Substrate-state-care failure: I checked the more-convenient source (local file) instead of the authoritative source (remote main). ## The fix Check remote main's manifest state as the primary source; fall back to local for cases where remote isn't accessible (test/first-cut/bootstrap). ```bash # Remote-first check git fetch --quiet origin main 2>/dev/null || true REMOTE_CONTENT=$(git show "origin/main:$MANIFEST_PATH" 2>/dev/null || true) if [[ -n "$REMOTE_CONTENT" ]]; then REMOTE_LAST=$(printf '%s' "$REMOTE_CONTENT" | jq -r '.last_released_sha // empty') if [[ -n "$REMOTE_LAST" && "$REMOTE_LAST" == "$HEAD_SHA" ]]; then printf 'skip\n' exit 0 fi fi # Fall back to local checkout (original behavior + first-cut bootstrap) # ... existing logic ``` Test seam `MANIFEST_REMOTE_FIXTURE=/path/to/file` lets bats simulate "remote main manifest content" without requiring an actual git fetch. ## New bats cases needed 1. **Remote at HEAD_SHA, local at older SHA** → skip (the v0.10.2 bug case) 2. **Remote at older SHA, local at older SHA** → proceed (normal cut) 3. **Remote manifest absent, local at HEAD_SHA** → skip (fallback to local) 4. **Remote manifest absent, local absent** → proceed (first cut) 5. **Remote unreachable (fixture file doesn't exist), local check governs** → existing behavior preserved Plus existing 9 tests preserved. ## Composition with related work - **[#128](https://git.frankenbit.de/frankenbit/release-toolkit/issues/128) / [#131](https://git.frankenbit.de/frankenbit/release-toolkit/pulls/131)** (idempotent cut pipeline): this bug is a SUB-CASE of the partial-completion matrix that the original implementation missed. Closing this bug completes the idempotency design. - **[#139](https://git.frankenbit.de/frankenbit/release-toolkit/issues/139)** (self-cancel noise): related but separate. #139 would make the cancellation rarer; this bug makes the re-run-after-cancellation correct. Both should ship; this one first. - **[#134](https://git.frankenbit.de/frankenbit/release-toolkit/issues/134)** (workflow test seam, closed in #136): the extraction this builds on. The remote-aware check is a script-level change, fully bats-testable. ## Severity **Real bug** (not cosmetic). The v0.10.2 cut substantively succeeded (release published + manifest on main) but the operator's re-run after cancellation produced exit-1, exactly like v0.10.0 did pre-#131 — except with a different mechanism (non-fast-forward push instead of release-create 409). The fix completes the partial-completion-matrix design. ## Implementation surface - `scripts/manifest-precheck.sh`: ~10 lines of additional logic (remote check + fallback) + test seam - `tests/manifest-precheck.bats`: 5 new tests for remote-aware behavior + adjust existing tests - No `_release.yml` change needed (script's interface unchanged) ## Refs - **Empirical artifact**: task 13376 (re-run of v0.10.2 cut workflow), 2026-06-26 23:05 - **Operator engagement 2026-06-26 23:??**: surfaced the re-run failure - **The original design point**: Surveyor 4a44's partial-completion matrix (which was correct; my implementation missed the remote-state check) - **Closes the idempotency arc started by**: [#128](https://git.frankenbit.de/frankenbit/release-toolkit/issues/128)

quartermaster referenced this issue from a commit

2026-06-26 23:14:29 +02:00

fix: manifest-precheck.sh remote-aware (closes #140)

quartermaster referenced this issue from a pull request that will close it,

2026-06-26 23:14:54 +02:00

fix: manifest-precheck.sh remote-aware (closes #140) #141

quartermaster closed this issue

2026-06-26 23:21:16 +02:00