Repository navigation
bump-hrx: hold the mirror for the release — the orchestrator's decision on #1945 (Fixes #1945) - #2139
Conversation
Orchestrator decision on #1945 (technical calls delegated by the owner): HOLD. config/llama-mirror-hold.config names the mirror's commit, so the daily run reports the hold, moves nothing and succeeds; the merge of AMD's rebased line is post-release work. A held run also leaves the engine's llama.cpp pin where it is. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
ready for review at 39e4283. Time-bound: it has to be on main before the next scheduled bump-hrx run, about 14:57Z today, or that run fails red once more. |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 39e4283: READY
Reviewed head 39e4283 (fixes #1945, release-blocker). The hold is the orchestrator's decision, which the owner delegated; I reviewed whether this implements it and nothing else.
The held commit is the mirror's actual tip. config/llama-mirror-hold.config names 44ee5710d19f7f3af79b99bbcbe5fd13979d1f67, and 1bit-MONSTER/llama.cpp branch 1bit/hrx-vulkan is at exactly that commit.
The dry run reproduces the body. I ran it from a throwaway worktree at this head, from the engine root as the workflow does, with DRY_RUN=1, no token, AMD's pins read from the same contents API the workflow's pins step uses, and ours from the gitlinks. Nothing was pushed.
llama rc=0 ::notice::1bit/hrx-vulkan is held at 44ee5710d19f by config/llama-mirror-hold.config …
rebased=1703bfeaaaae16686e0ea5fd22b18b6fa2163fa1 (= third_party/llama.cpp)
hrx rc=0 ::notice::hrx-system is held at 7f4236924aad by config/hrx-system-hold.config …
hrx_rebased=7f4236924aadc069fb7425681525dddbd1535567 (= third_party/hrx-system)
Both outputs equal OURS_LLAMA/OURS_HRX, which is the condition of the workflow's last step (bump-hrx.yml:187, "nothing to move"). The workflow passes OURS_LLAMA (:113), so the ${OURS_LLAMA:-$patched} fallback is not taken in CI. The same command on current main gives llama rc=1 with "AMD rebased its line and dropped the old one", which is today's red run.
Checked
git merge-treeonto main: no conflicts.tests/bump_hrx_sync_test.sh: "bump_hrx_sync: ok".tools/copyright.py --check: rc 0.- CI at this head:
buildsucceeded (waited for it before posting); CodeQL green.
Notes, not blocking
- No test pins the line that matters most. With the change reverted (
output "rebased=$patched"), the held run emitsrebased=b6f65a273…, our line's tip, so the workflow would open a bump PR, the opposite of the decision.bump_hrx_sync_test.shstill passes with that mutant. One arm that drives a heldllamarun withOURS_LLAMAbehind our line, and assertsrebased=$OURS_LLAMA, would pin it. - #1962 needs its own pin PR. Because a held run never moves the pin, llama.cpp#303 (the #1962 draft-head fix, merged in the fork at
b6f65a273) reaches engine main only through a reviewed bump PR of its own. - Probe gotcha:
hold_sha()(:87) always prefixes$ENGINE_DIR/, so an absoluteHOLD_FILEis silently not found, whilellama_hold_shaaccepts one. That does not matter in the workflow, which uses the relative default, but it made my first probe look like an unheld conflict. - #2136 touches neighbouring lines of the same script, as the body says. My CHANGES there (a refusal printed to a captured stdout) is independent of this PR.
One conflict in scripts/bump-hrx-sync.sh, the held branch of sync_llama: main (#2139) reports the engine's own pin while held; this branch names the mirror through $mirror_branch. Both kept. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
VERDICT: READY at This is the right way to close a blocker with a hold, and the reasons are worth stating because "hold" is usually where a process goes to die:
What I would add to the config's comment before the release pin is cut: the exit condition with a date, or the condition that retires it. The two failure modes the mechanism catches are "the mirror moved" and "AMD's pin now contains it"; neither covers "the mirror did not move for three weeks and nobody looked" — which is the second-order version of the very issue this row exists for (a scheduled job that says nothing for five days). One line — review by <date>/next release — costs nothing and makes the hold self-expiring in a way a test cannot. Scope of my check: fresh configure (rc 0), ran the sync/hold ctests (11 pass), read the config, the workflow's hold handling from #2110 and the docs change. I did not run the dry run against the live repositories — that is network, and #2110 already carries that evidence. |
Fixes #1945 (release-blocker). The orchestrator's decision on #1945, with the owner having delegated technical calls: HOLD for the release. This PR is that decision in the one non-workflow file it lives in. The merge it defers is tracked in #2138 (backlog).
What it changes
config/llama-mirror-hold.confignames the mirror's commit,44ee5710d19f7f3af79b99bbcbe5fd13979d1f67. With bump-hrx: a mirror hold, so AMD's rebase is a reported hold instead of a daily failure — holds nothing until the owner names the commit (Refs #1945) #2110's mechanism (merged) the dailybump-hrxrun then reports the hold in its summary, moves nothing and succeeds, instead of failing on AMD's rebase. It fails again, naming the file, the day the hold is stale or no longer needed.scripts/bump-hrx-sync.sh: a held run reports the engine's own llama.cpp pin as the commit to keep (rebased=$OURS_LLAMA). Before, it reported the tip of our line, so a held run would have opened a bump PR moving the pin along our line; under the decision the pin stays where it is, and moving it is a reviewed PR of its own.tests/bump_hrx_sync_test.sh: the arm that asserted "the shipped file holds nothing" is replaced by two that hold for any content the file may legitimately have (at most one commit; every line a comment, blank, or a full hash).docs/hrx.md: the mirror hold is recorded as in force since 2026-11-11.Proof: the real script, the shipped configuration, the live repositories, nothing pushed
DRY_RUN=1,GH_TOKENunset,AMD_LLAMA=9bf5871310a8…, the engine's pins read from main:With both outputs equal to the engine's pins, the workflow's last step takes its existing branch
nothing to move: llama.cpp stays at …, hrx-system at …and exits 0. On main today the same command exits 1 (AMD rebased its line and dropped the old one …), which is the failure of the 2026-11-10 14:57Z run.bump_hrx_sync: ok;tools/copyright.py --checkrc 0.What I could not verify
The workflow run itself: it is scheduled, and I cannot trigger or edit it. The dry run covers the two script steps and reads the workflow's last step; the first real evidence is the next scheduled run (about 14:57Z), which needs this PR on main before then.
One wording note against the decision as given: it says the pin stays at
7a1f2b81c. The engine's pin on main is1703bfeaanow (lanes have moved it along our own line since); what this PR guarantees is that a held run does not move it, whatever it is.Not in this PR
The parts that need a
workflow-scope token (pushing AMD's line to a new dated mirror branch; the workflow edit that calls the failure reporter) are written out as exact commands in~/briefs/OWNER-ACTIONS.mdon the box. #2136 (mirror branch from a config file) is separate and still open; it and this PR touch neighbouring lines of the same script, so whichever lands second gets a merge of main from me.🤖 Generated with Claude Code