Skip to content

bump-hrx: hold the mirror for the release — the orchestrator's decision on #1945 (Fixes #1945) - #2139

Merged
bong-water-water-bong merged 1 commit into
mainfrom
cl-flm/1945-hold
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
cl-flm/1945-hold

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

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.config names 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 daily bump-hrx run 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_TOKEN unset, AMD_LLAMA=9bf5871310a8…, the engine's pins read from main:

$ scripts/bump-hrx-sync.sh llama                                                                    rc=0
AMD pin 9bf5871310a8…, mirror 1bit/hrx-vulkan 44ee5710d19f…, our line 1bit/hrx-vulkan-patched b6f65a273f8a…
::notice::1bit/hrx-vulkan is held at 44ee5710d19f by config/llama-mirror-hold.config: AMD's pin 9bf5871310a8 does not descend from it (AMD rewrote its line). Nothing was moved; …
summary: - llama.cpp: mirror `1bit/hrx-vulkan` HELD at `44ee5710d19f` (config/llama-mirror-hold.config); AMD's pin is `9bf5871310a8`, on a line that does not contain it. Not merged.
rebased=1703bfeaaaae16686e0ea5fd22b18b6fa2163fa1          <- the engine's pin on main today, unchanged

$ scripts/bump-hrx-sync.sh hrx                                                                      rc=0
hrx_rebased=7f4236924aadc069fb7425681525dddbd1535567      <- the engine's hrx-system pin, held as before

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 --check rc 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 is 1703bfeaa now (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.md on 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

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>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

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.

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1945 - Partially compliant

Compliant requirements:

  • The PR correctly implements the orchestrator's decision to hold the mirror for the release
  • The config/llama-mirror-hold.config file is updated to name the mirror's commit (44ee5710d19f7f3af79b99bbcbe5fd13979d1f67)
  • The scripts/bump-hrx-sync.sh script is updated to handle the held case properly by not moving the engine's pin
  • The tests/bump_hrx_sync_test.sh is updated to verify the correct behavior of the hold file
  • The docs/hrx.md is updated to document the hold and its purpose

Non-compliant requirements:

Requires further human verification:

  • The actual merge of AMD's rebased line into 1bit/hrx-vulkan-patched needs to be verified separately
  • The scheduled bump-hrx run should be confirmed to succeed with the hold in place
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Incorrect Pin Handling in Held State

In the held state, the script sets rebased=${OURS_LLAMA:-$patched} which means it will use the engine's own pin (OURS_LLAMA) if set, otherwise the patched version. However, this could be problematic because OURS_LLAMA might not be the correct commit to report when the mirror is held, as it should reflect the commit that would have been used if the sync was not held. The current approach may not properly distinguish between the engine's pin and what would have been the sync target.

output "rebased=${OURS_LLAMA:-$patched}"
Test Logic for Hold File Validation

The test logic for validating the shipped hold file has been updated to check that it contains at most one commit hash and that all non-comment, non-blank lines are valid commit hashes. However, the test does not verify that the commit hash in the hold file matches the expected commit that should be held, which is important for ensuring the correct behavior of the hold mechanism.

n=$(grep -v '^[[:space:]]*#' "$shipped" | grep -cE '^[0-9a-f]{40}' || true)
if [ "$n" -le 1 ]; then ok "the shipped hold file names at most one commit"; else fail "the shipped hold file names at most one commit (found $n)"; fi
junk=$(grep -v '^[[:space:]]*#' "$shipped" | grep -v '^[[:space:]]*$' | grep -vcE '^[0-9a-f]{40}([[:space:]].*)?$' || true)
expect "every line of the shipped hold file is a comment, blank, or a full commit hash" "$junk" "0"

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-tree onto main: no conflicts. tests/bump_hrx_sync_test.sh: "bump_hrx_sync: ok". tools/copyright.py --check: rc 0.
  • CI at this head: build succeeded (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 emits rebased=b6f65a273…, our line's tip, so the workflow would open a bump PR, the opposite of the decision. bump_hrx_sync_test.sh still passes with that mutant. One arm that drives a held llama run with OURS_LLAMA behind our line, and asserts rebased=$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 absolute HOLD_FILE is silently not found, while llama_hold_sha accepts 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.

@bong-water-water-bong
bong-water-water-bong merged commit 0d50eaa into main Nov 11, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the cl-flm/1945-hold branch November 11, 2026 07:38
bong-water-water-bong pushed a commit that referenced this pull request Nov 11, 2026
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>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 39e4283 — fresh configure rc 0, 11 tests pass in the bump_hrx|mirror|hold selection (60 s), and the shipped config names the commit the decision was about:

$ cat config/llama-mirror-hold.config | grep -v '^#' | head -1
44ee5710d19f7f3af79b99bbcbe5fd13979d1f67

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:

  1. The decision is one file, reviewable in one line, and it is the non-workflow file it belongs in — so it lands without the workflow scope no lane token has, and the pin keeps following our own line.
  2. A hold that is stale or unneeded FAILS the run (the mechanism landed in 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): so a hold cannot quietly outlive its reason. That is the difference between a hold and an excuse.
  3. The merge it defers is named, not implied — a reader can see which commits are waiting and what would let them in.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bump-hrx: the scheduled HRX pin sync is failing

1 participant