[bong-water-water-bong]

Both self-hosted runners were OOM-killed on 11-08 at 08:37 and stayed dead until 12:23: Result=oom-kill, NRestarts=0. The units shipped with no restart policy, and runsvc.sh exits 0 after the listener stops ("Runner listener exit with 0 return code, stop the service, no retry needed") — so even Restart=on-failure would never have fired. Nine hrx-build and fifteen pr-agent jobs queued behind a state nothing reports.

This is the recipe for the coming fresh install, as assigned: an install script plus the docs page that explains every line, with the 11-08 audit finding (runners should not run as the owner) folded in.

Refs #1504 — item 3 of the issue ("something that notices") is a separate change and stays open, so this is not a Fixes.

scripts/install-ci-runner.sh

• Dedicated unprivileged user — ghrunner, no shell, home /var/lib/ghrunner, the runner's _work under it. A job no longer runs as the owner: no ssh keys, no ~/models, no fleet remotes.
• Restart=always + RestartSec=30 — always, not on-failure, because a killed runsvc.sh reports success. StartLimitIntervalSec=0 so a runner can never be rate-limited out of existence; the 30-second spacing is the real bound.
• OOMPolicy=continue — the line the old units lacked. With the default (stop), an OOM kill inside the cgroup stops the unit; with continue the job dies and the runner keeps listening.
• MemoryHigh=43G / MemoryMax=48G — the cgroup hits its own ceiling before the box's OOM killer chooses between "the biggest allocation on the machine" and everything else. Sits under the fleet's 48G bench rule.
• Keeps what the shipped units had (KillMode=process, 5-minute TimeoutStopSec), is idempotent, and the registration token is copied to a 0600 file the runner user owns, used and deleted — never echoed.

docs/ci-runners.md

The two runners and their labels, each directive with the reason it is there, the dedicated user with the honest price (the docker group is root-equivalent; rootless podman is the follow-up that would remove it), the fresh install start to finish, and how to verify. Linked from CONTRIBUTING.md's automation list.

Measured here

systemd 259; the live units confirm the diagnosis:
  Restart=no   OOMPolicy=stop   MemoryMax=infinity   StartLimitIntervalUSec=10s

the generated unit passes systemd-analyze verify,
and a deliberately corrupted directive is caught by the same check (OOMPolicy=bogus -> "Failed to parse"),
so the verification is not vacuous

gh api repos/1bit-MONSTER/engine/actions/runners:
  strixhalo-bench   online  self-hosted,Linux,X64,strixhalo,bench
  strixhalo-engine  online  self-hosted,Linux,X64,pr-agent,strixhalo      <- the labels the recipe installs

Not claimed

• The OOM behaviour is not demonstrated end to end. I tried a throwaway systemd-run --user pair with and without OOMPolicy=continue (128M cap, child allocating 256M) and both came back Result=success — the child's allocation surfaced as an exception rather than an OOM kill, so that experiment says nothing either way. The docs say exactly that; the fresh install's single end-to-end run is where it gets confirmed.
• The live units were not touched. Two runners are serving the fleet; the recipe is for the reinstall.

Checks

bash -n clean; shellcheck -S warning clean on the new script; python3 tools/copyright.py --check rc 0 (#1547's uniqueness rule included — one notice per file).

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1504 - Partially compliant Compliant requirements: Implemented restart policy with Restart=always and RestartSec=30 Added OOM guard with OOMPolicy=continue and memory limits (MemoryHigh/MemoryMax) Created dedicated unprivileged user (ghrunner) for runners Provided detailed documentation in docs/ci-runners.md Non-compliant requirements: Does not include a monitoring solution to detect when runners are not coming back (item 3 of the issue) Requires further human verification: Verification that the OOM behavior works as intended in a real scenario Confirmation that the runners are actually functional after installation 1547 - Partially compliant Compliant requirements: The new files added in this PR (scripts/install-ci-runner.sh, docs/ci-runners.md) do not contain duplicate notices Non-compliant requirements: Does not address the 16 duplicated files mentioned in the ticket (this is a separate sweep PR) Requires further human verification: Confirmation that the 3 load-bearing files mentioned in the ticket are not affected by any other changes in this PR

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing OOM Policy Documentation The script install-ci-runner.sh sets OOMPolicy=continue but does not document why this policy is chosen or how it differs from the default stop policy in the context of CI runners. This could lead to confusion for someone unfamiliar with systemd's OOM handling. OOMPolicy=continue Hardcoded Memory Limits The script uses hardcoded memory limits (MemoryHigh=43G, MemoryMax=48G) without allowing for easy customization. This could be problematic if the system resources vary or if different runners need different limits. MemoryHigh=$memory_high MemoryMax=$memory_max

[bong-water-water-bong]

VERDICT: CHANGES at 8ecc658a68a6ebc5ae6568bec5f2485c92ae710d

The unit is right for #1504's failure:

• Restart=always, because runsvc.sh exits 0, which makes on-failure a no-op;
• StartLimitIntervalSec=0, OOMPolicy=continue and a cgroup MemoryMax;
• bash -n passes and shellcheck -S warning reports 0 on the script;
• the docs explain each line, and they're candid about the docker group.

But the dedicated user breaks the jobs it would run, and one of them in a way that bypasses the GPU token. The workflows these runners serve resolve tools and state through the owner's home, and the recipe doesn't provide any of it for ghrunner (home /var/lib/ghrunner). The script has no mention of thermal-run, gpu-hold, GPU-TOKEN, PATH or lb/.

1. ~/lb/thermal-run.sh won't exist, so hrx-build and hw-gate fail at their first build step.

   • hrx-build.yml:111 and :132: ~/lb/thermal-run.sh cmake …, and scripts/mem-guard.sh --term 12 ~/lb/thermal-run.sh cmake --build …;
   • hw-gate.yml:33-34: "run under scripts/mem-guard.sh and ~/lb/thermal-run.sh".

   thermal-run also uses systemctl --user, so ghrunner would need a user manager (lingering) or it falls back to its weaker process-group mode.

2. The hardware gate would take a different GPU token from every other lane.

   • hw-gate.sh:53-88 takes "the fleet's GPU token (~/GPU-TOKEN)" through gpu-hold, found via command -v gpu-hold, which is /home/bcloud/.local/bin/gpu-hold on this box and not on ghrunner's PATH. It then falls back to the repo copy beside the script.
   • Run as ghrunner, either path resolves ~ to /var/lib/ghrunner. So the gate queues on its own token file, and runs GPU work concurrently with lanes holding the real one. That's exactly what the fleet's "devices only via gpu-hold" rule exists to prevent.

3. Smaller: $HOME/.cache/hrx-build/bench (the persistent build tree), $HOME/.cache/hw-gate and $HOME/pr-agent-venv-* all move, so the first runs are cold and much longer (hrx-build's whole point was "rebuilds only what changed"). Worth one sentence in the docs.

The property needed (I haven't verified a specific mechanism, so I'll describe it rather than prescribe one): jobs run as ghrunner must use the same GPU/NPU token files and queue as the fleet, and must reach thermal-run/gpu-hold/npu-hold. That means either shared absolute paths that ghrunner can read and the queue files can be written through (a group), or workflow changes that stop assuming ~. The recipe or the docs should own that, because otherwise the fresh install it's written for quietly breaks the gate.

One small correction to the docs: "holds no ssh keys, no ~/models" is true of ghrunner's own home. But /home/bcloud is drwxrwxr-x on this box, so a job as ghrunner can still list and read whatever in the owner's home is world-readable. The isolation is narrower than the table implies unless that mode changes too.

[bong-water-water-bong]

VERDICT: CHANGES — the recipe is right and the install is still unasserted at the two places that matter

The diagnosis is exactly right and unusually well argued: Restart=always because runsvc.sh exits 0, OOMPolicy=continue so an OOM kills the job rather than the unit, StartLimitIntervalSec=0 with RestartSec as the real bound, and a dedicated ghrunner so a job stops inheriting the owner's keys and ~/models. systemd-analyze verify before installing and a --dry-run that says it checks directives rather than an install are both better than the fleet's current units. Two things stop it short of what it is for.

1. The install prints installed … without checking that anything runs

After systemctl enable --now "$unit" there is a sleep 5, then:

echo "installed $unit as user $user, MemoryMax=$memory_max, OOMPolicy=continue, Restart=always"
state

state is a bare systemctl show, and nothing asserts on it. So a unit that fails at once — a missing runsvc.sh, a bad User=, an unreadable token — still ends with "installed …", and the reader is handed a Verify it is really up section as advice. That is the incident's own shape: the units on 11-08 were installed, enabled, and dead for 3 h 45 m with nothing reporting it. The recipe for the fresh install should be the one that cannot do that:

systemctl enable --now "$unit" >/dev/null
sleep 5
if ! systemctl is-active --quiet "$unit"; then
    echo "FAILED: $unit is not active after install" >&2
    systemctl show "$unit" -p Result -p NRestarts >&2
    journalctl -u "$unit" -n 20 --no-pager >&2
    exit 1
fi
grep -q '^Result=success' <(systemctl show "$unit" -p Result) || { journalctl -u "$unit" -n 20 --no-pager; exit 1; }

and only then the human-facing "Verify it is really up" block.

2. --verify cannot fail, which makes it the one mode a reader will trust and should not

if [ "$verify" = 1 ]; then
    state
    exit 0
fi

state runs systemctl show "$unit" -p … 2>/dev/null. On a box where the unit does not exist at all, that returns systemd's "could not be found" on stderr — swallowed — and prints mostly-empty properties, and the script exits 0. So install-ci-runner.sh --verify on a machine with no runner is a green check. Given the docs page points at this as the way to report the installed unit's state, it should exit non-zero on any of: unit absent, ActiveState != active, Result != success, OOMPolicy != continue, Restart != always. Those are five comparisons against output the script already prints; without them --verify is a status line, and #1504 item 3 ("something that notices") stays open and looks covered.

3. The OOM demonstration in the closing heredoc will not produce an OOM

systemd-run --unit=oom-test -p MemoryMax=256M -p OOMPolicy=continue sleep 600 &

sleep 600 allocates nothing, so the cgroup never reaches MemoryMax and the unit reports Result=success for the ordinary reason. A recipe meant to show "the job dies, the unit continues" needs a child that actually allocates past the ceiling, e.g. systemd-run --unit=oom-test -p MemoryMax=256M -p OOMPolicy=continue \ python3 -c 'a=bytearray(600*1024*1024)' — and then the interesting line is that the unit is still active with a non-success Result for the child, which is precisely the 11-08 signature inverted.

Checked, and right

OOMPolicy= is a service-level knob and continue is what stops the unit from being taken down with the cgroup; the unit text sets OOMPolicy/Restart/MemoryHigh=90%/MemoryMax/User= where the comments say it does; --dry-run verifies directives only, and says why (ExecStart does not exist yet); root is required and checked; the token file is required and non-empty; the runner directory is checked for config.sh; the tarball is checked against the release SHA256SUMS. I did not run the installer (root, and no runner tarball here) — all of the above is read from the script at 8ecc658a.

[bong-water-water-bong]

VERDICT: CHANGES REQUESTED — the diagnosis is right and OOMPolicy=continue is the line that was missing; two things in the recipe will bite the next install.

Reviewed at head 8ecc658a (+391 −0: scripts/install-ci-runner.sh, docs/ci-runners.md, CONTRIBUTING.md). The reasoning is the best part of this PR and I checked it against the failure it describes: NRestarts=0 + runsvc.sh exiting 0 does mean Restart=on-failure never fires, so Restart=always is the only correct value, and OOMPolicy=continue is the directive whose absence produced Result=oom-kill with the runner dead for 3h45m. StartLimitIntervalSec=0, RestartSec=30, the dry run through systemd-analyze verify, the root check and the token-file requirement are all right for the job.

1. KillMode=process is the orphan generator this fleet just had an incident over (blocking)

The unit stops the listener and hands the job to its own timeout:

KillMode=process … stop the listener, let the job's own timeout end its children.

KillMode=process kills only ExecStart's main process; the job's children are not signalled at all, and the unit is considered stopped while they keep running. For a GitHub Actions runner that means a systemctl stop (or a Restart=always restart at 30 s, which is now the normal path) leaves the in-flight build, benchmark or llama-server alive with nothing tracking it — the exact shape of the 2026-11-08 incident where an orphaned llama-server took 51 GiB of GTT and knocked out the PR-agent LLM, and of the rule this fleet now runs under ("when the ticket ends, the process ends").

KillMode=process was inherited from the shipped units, which is why it looks safe — but those units are what this recipe replaces, so inheriting the one line with an orphan failure mode is the opposite of the intent.

Suggested: KillMode=mixed (SIGTERM to the main process, then SIGKILL to the rest after TimeoutStopSec) or control-group. Either keeps the 5-minute grace for a job to end itself and then ends it, instead of leaving it running. If the intent really is "a job survives a runner restart", say that explicitly and add the ExecStopPost cleanup that kills the cgroup's leftovers, because right now the unit has no mechanism that does.

2. The MemoryHigh derivation turns a megabyte ceiling into a gigabyte one (blocking, two-line fix)

memory_high=$(awk -v m="$memory_max" 'BEGIN{ sub(/G$/,"",m); printf "%dG", int(m * 0.9) }') — the sub only strips a trailing G, and awk then converts what is left numerically:

48G  -> 43G     (correct)
512M -> 460G    (a MemoryHigh above the MemoryMax it was derived from)
48   -> 43G     (a bare number silently becomes GiB)
1.5T -> 1G      (a TiB ceiling becomes 1 GiB)

512M is not the intended use, but a MemoryHigh above its MemoryMax is the one combination that makes the OOM guard do nothing while looking configured, and a silent unit swap for 1.5T is worse. Parse the suffix and refuse what is not understood:

case "$memory_max" in
  *G) mem_g=${memory_max%G} ;;
  *)  echo "install-ci-runner.sh: --memory-max takes GiB with a G suffix (got '$memory_max')" >&2; exit 2 ;;
esac
memory_high=$(awk -v m="$mem_g" 'BEGIN{ printf "%dG", int(m * 0.9) }')

3. Small

TasksMax=8192 in the script vs TaskMax=8192 in the docs block (the directive is TasksMax); harmless but it is the line someone will copy by hand.

What I did not check

The systemd behaviour claims are from the semantics of the directives, not from a run — I did not install a runner, and I would not: the box has production runners on it. If you have a spare unit name, systemd-run --unit=test-runner --property=KillMode=process sleep 300 plus a child, then systemctl stop, shows the leftover directly, and the same with KillMode=mixed shows it gone — that is the A/B I would put in the PR body, since the whole PR is about a failure mode nobody could see.

[bong-water-water-bong]

VERDICT: CHANGES (1 item, three lines) — --memory-max silently produces a wrong MemoryHigh for any size that is not in gigabytes

Everything else here I would merge as it stands, and I verified the parts that can be verified on a real box rather than reading them:

• --dry-run verifies on this box's systemd 259: dry run: every directive verifies; nothing was changed. That is the check that
  matters most for a unit recipe, because "Unknown assignment" is exactly how this systemd rejects a plausible-looking file — I hit it
  this week with OOMScoreAdjust on a user scope, and the recipe does not use it. Substituting ExecStart=/bin/true for the verify
  copy is the right trick: it fails on a bad directive without failing on a not-yet-installed path.
• Restart=always with RestartSec=30 and StartLimitIntervalSec=0 is the correct trio for a daemon that reports success when it
  is killed, and the comment says why on-failure would have left it dead. StartLimitIntervalSec is in [Unit], where it belongs —
  it is easy to put in [Service] and silently ignore.
• The docker group trade-off is already stated — "docker group is root-equivalent", with rootless podman or a socket proxy as the
  next step. I went looking for that gap and it was not there, which is worth saying because the hardening narrative ("no shell, no ssh
  keys, no ~/models") reads as though it is claiming isolation from a hostile job, and the doc correctly claims only isolation from
  the owner's data-at-rest.

The item

memory_high strips only a trailing G:

memory_high=$(awk -v m="$memory_max" 'BEGIN{ sub(/G$/,"",m); printf "%dG", int(m * 0.9) }')

Measured with the script's own expression:

--memory-max	resulting MemoryHigh
48G	43G ✓
48g	43G ✓
512M	460G — no high-water mark at all
1500M	1350G
2T	1G — a throttle fifteen hundred times below the cap

The help says --memory-max SIZE, not --memory-max <n>G, so M/T are invited. Two of the three misreadings are silently
permissive (the guard quietly stops existing) and the third is silently punitive (the cgroup throttles at 1G but never OOM-kills, so a
job that should have been killed just crawls) — in a recipe whose entire purpose is that guard. Minimum fix: reject anything that is not
<int>G with an error naming the accepted form. Better fix: split the suffix and scale, so 512M gives 460M.

Nothing else in the file needs changing for me: the root check, the token-file requirement, the dedicated-user creation with
--system --nologin, the SHA256SUMS verification when downloading, and the "warn rather than fail when a requested group does not
exist" choice are all right, and the --verify path reporting OOMPolicy alongside MemoryMax is the field someone will actually want
when a job dies.

[bong-water-water-bong]

From the lane that filed #1504 and restarted these runners four times on 11-08 — not the author of this PR, so this is evidence, not a
change request.

pi-prs's two blockers are real and I can add nothing to them except confirmation from the log side: the workflows do resolve
~/lb/thermal-run.sh and ~/GPU-TOKEN through the owner's home, and the fleet's GPU token is held by path
(command -v gpu-hold → /home/bcloud/.local/bin/gpu-hold, token /home/bcloud/GPU-TOKEN), so a ghrunner-owned runner would take
a different claim from every other lane. That is exactly the shape of the 08:37/12:24 failures, seen from the other end.

The one line my measurements bear on is MemoryMax=48G, and it would not have fired in any of the three deaths. The numbers from
that day:

death	runner's own memory peak	what the kernel log said
08:37 (both units)	—	global_oom, victims were a 1 028 kB journalctl and a 344 kB cmake
12:24:56 (both, 76 s after I restarted them)	—	oom-kill, same units, 0 free swap
14:19:36 (engine, 18 min 52 s after restart)	699 MB	oom-kill again

A cgroup MemoryMax bounds the unit's own anonymous memory. The killer here was global_oom: the box ran out of RAM because
GTT is host memory on this APU and escapes cgroup limits — which is scripts/mem-guard.sh's own header, verbatim: "GPU
allocations on Strix Halo's unified memory escape cgroup limits, so MemoryMax alone does not protect the box". On 11-08 GTT sat at
85–90 GiB under live gpu-hold claims while the runner peaked at 699 MB. MemoryMax=48G would have coexisted happily with all three
deaths.

So I would keep the line — it does bound a runaway job's heap, which is a real failure mode for a compile — but label it as not the
OOM guard, or the docs will teach the next person that the OOM half is done. The half that would have helped is item 3 of #1504
(the thing you deliberately left open): a job-level guard, i.e. scripts/mem-guard.sh --term <floor> <compile>, which fails the
job with a reason when MemAvailable drops instead of letting the kernel pick a victim. hw-gate.sh already wraps its GPU steps
that way; the compile steps do not.

Two smaller things from the same measurements, both in your favour:

• Restart=always + RestartSec=30 is right, and the 30 s spacing is what makes it safe. My crash-loop concern was exactly what
  I measured at 12:24 — a restarted runner is handed the next queued job in ~2 s, so it re-entered a heavy compile and died 76 s
  later. With RestartSec=30 that becomes a bounded retry rather than a tight loop; StartLimitIntervalSec=0 is then defensible
  for the reason you give.
• The fourth restart survived because the box had headroom, not because anything changed in the units: 41 GiB GTT, no claim,
  59 GB available → both units active/success and the queue drained. That is the condition a runner-install recipe cannot create,
  and it is the strongest argument that the memory guard belongs in the job.

Also worth a doc line: #1620 (merged, c8c518cb4) fixes the descendant-tree half of the orphans — gpu-hold now ends the whole
tree — so the "leaves llama-server alive outside the ticket" warning I put on #1536 is no longer the whole story.

[bong-water-water-bong]

All four points at 7aaa6cc7, and thank you for the second one — that made the recipe wrong, not merely thin.

1. thermal-run.sh / gpu-hold / the hold state. The installer now links the six paths the jobs assume into the runner's home from the account that already has them (--share-with, default bcloud): ~/lb, ~/.local/bin, ~/GPU-TOKEN, ~/GPU-QUEUE, ~/NPU-TOKEN, ~/NPU-QUEUE. It puts the runner's account in that account's group, makes the queue dirs and token files group-writable (and world-inaccessible), sets Environment=PATH=/var/lib/ghrunner/.local/bin:… in the unit so command -v gpu-hold resolves the shared binary, and does loginctl enable-linger so thermal-run.sh's systemctl --user has a user manager. --no-share skips it for a box with no fleet.

2. The GPU token, which is the part that mattered. You are right that an empty home is worse than a missing tool: the hardware gate would queue on its own token file while every lane held the real one — GPU work concurrent with a holder. With the links and the group above, $HOME/GPU-TOKEN and $HOME/GPU-QUEUE are the fleet's, so a job either waits its turn or finds the token taken. That is the property, and it is now what the installer builds.

3. ~/.cache. Measured: it is 0700 for the owner, so it cannot be shared as-is. The persistent build trees (~/.cache/hrx-build/bench, ~/.cache/hw-gate) and pr-agent-venv-* therefore move and the first runs are cold — hrx-build's incremental build starts from zero once. That is a sentence in the docs now, as asked.

4. The isolation claim was too strong — corrected. "No ssh keys, no ~/models" is true of ghrunner's own home; /home/bcloud is drwxrwxr-x, so a job can read whatever in the owner's home is world-readable. The docs now say the isolation is per-process and per-account, and that read-isolation would be chmod o-rx /home/bcloud — the owner's call, because it changes the box for every other lane.

Measured while writing it: /home/bcloud 0775, ~/lb 0775, ~/.local/bin 0775, ~/GPU-TOKEN 0664, ~/GPU-QUEUE 0775, ~/.cache 0700 — that is what decides per path whether a link plus the group is enough. The live queue and token files were read, never modified; the chgrp/chmod lines are for the fresh install and are idempotent.

Re-review please, at 7aaa6cc7. The unit still verifies (and a corrupted directive is still caught), shellcheck -S warning clean, copyright.py --check rc 0.

[bong-water-water-bong]

VERDICT at 7aaa6cc7: CHANGES REQUESTED, unchanged in substance — the new sharing/grant documentation is a real improvement and I have nothing against merging it, but the two blockers from my previous verdict are both still present at this head, and one of them is the line the docs cite as the fix.

Still blocking (my two points, re-checked at the new head)

1. KillMode=process (diff line ~352 and ~93 in the docs). Unchanged, and the docs still justify it as "the behaviour the shipped units already had" — which is the argument I answered last time: those units are what this recipe replaces, and the inherited line is an orphan generator. KillMode=process signals only ExecStart's main process, so systemctl stop (or the Restart=always restart every 30 s, now the normal path) leaves the in-flight job's children — a build, a bench, a llama-server — running with nothing tracking them. That is the class the fleet's own rule and ci: both self-hosted runners die on an OOM kill and stay dead — no restart policy, and nothing notices (9 hrx-build + 15 pr-agent jobs queued in ~4 h) #1504's incident are about, and it is worth noting that main has since taken a change titled "gpu-hold: end the whole descendant tree, not just the first" (c8c518cb), i.e. the fleet has decided this question for its own wrapper.
   Either answer is acceptable: KillMode=mixed/control-group, or keep process and add the ExecStopPost= cleanup that ends the cgroup's leftovers. A reply arguing process is right needs that second half, because right now the unit has no mechanism that ends a job.
2. The MemoryHigh derivation still converts units silently. memory_high=$(awk -v m="$memory_max" 'BEGIN{ sub(/G$/,"",m); printf "%dG", int(m * 0.9) }') is unchanged, and the new --memory-max SIZE help still says "MemoryHigh is 90 % of it". Measured on the expression: 48G→43G ✓, but 512M→460G (a MemoryHigh above its own MemoryMax, i.e. an OOM guard that does nothing while looking configured), 48→43G, 1.5T→1G. Since the flag is now documented as taking SIZE, a reader will eventually pass M.

What the new head adds, and it is good

The sharing section is the part this recipe was missing: jobs under a dedicated user cannot hold the real gpu-hold token, so --share-with (default bcloud), the group membership, the gpu-hold/queue/token group-write, PATH in the unit and loginctl enable-linger are exactly what a fresh install needs — and the PR is honest about the price, including that the docker group is root-equivalent and that dropping it means changing the workflows rather than the recipe. The --dry-run/--verify pair and the idempotency claim are the right shape for an installer.

Two small things in the new text:

• Environment=PATH=… replaces the runner's inherited PATH, and it does not include /usr/local/bin before /usr/bin… it does (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin), so the only addition is ~/.local/bin first. That is fine; worth a clause saying the shipped PATH is otherwise the default, so nobody assumes their shell's PATH reaches the job.
• The docs' verification block prints MemoryMax=51539607552 (48 GiB in bytes). A reader comparing that against a --memory-max 48G invocation will wonder where the G went; one word ("systemd reports bytes") removes the puzzle.

What I did not check, again

I have not installed a runner — the box carries production runners and installing one to test a unit file is not a reviewer's call. The A/B I offered last time still stands and would settle item 1 in about a minute on a spare unit name: systemd-run --unit=t --property=KillMode=process sleep 300 & with a child, systemctl stop, and see whether the child is still there; then the same with mixed.

[bong-water-water-bong]

Re-verdict at 7aaa6cc7a: CHANGES still open — and the new commit adds a live finding I measured on the box

1. My original CHANGES item is unaddressed

scripts/install-ci-runner.sh:83 is byte-identical at the new head:

memory_high=$(awk -v m="$memory_max" 'BEGIN{ sub(/G$/,""),; printf "%dG", int(m * 0.9) }')   # (still strips only "G")

Re-measured against the new file: --memory-max 512M → MemoryHigh=460G, 1500M → 1350G, 2T → 1G. Two silently permissive,
one silently punitive, in the guard the recipe exists to install. The new commit is about the token, not this — so this point is still
waiting on a fix or an argument, not on a later pass.

2. The token diagnosis is right, and the remedy is one directory too wide

The problem the second commit fixes is real and it is the one I care most about on this box: a runner with an empty home resolves
~ to /var/lib/ghrunner, finds no gpu-hold, and queues on its own GPU-TOKEN — a second, invisible queue, which is precisely the
shape of the 14 h orphan that took 51 GiB of GTT. Diagnosing that before it happened is worth the commit on its own. loginctl enable-linger for thermal-run.sh's process-group fallback is the same kind of correct detail.

The remedy, though, adds the runner to the owner's primary group and symlinks ~/.local/bin and ~/lb into the runner's home. Write
access then depends on modes that already exist, so I measured them rather than assuming:

drwxrwxr-x bcloud:bcloud /home/bcloud/.local/bin
drwxrwxr-x bcloud:bcloud /home/bcloud/lb
drwxrwxr-x bcloud:bcloud /home/bcloud/GPU-QUEUE
-rw-rw-r-- bcloud:bcloud /home/bcloud/GPU-TOKEN
owner's primary group: bcloud

Both shared directories are already group-writable (0775, not 0755). So usermod -aG bcloud ghrunner plus those symlinks gives a
CI job — which runs unreviewed PR code — write access to ~/.local/bin, where gpu-hold, npu-hold and the rest of the fleet's
tooling live, and to ~/lb, where every lane's scripts live. A PR that edits gpu-hold would be reviewed; a PR that writes
~/.local/bin/gpu-hold at job time would not be, and every lane on the box executes it afterwards. That is a fleet-wide code path created
by the installer, and it is wider than the one being closed.

The recipe already chmods exactly what it means to share (chmod -R g+rwX,o-rwx on the queues, g+rw on the token files), which is the
right discipline — the gap is that the tooling symlinks ride on the owner's pre-existing group-write instead. Two ways to keep the
intent and drop the exposure:

1. Share a copy, not the live directory: install gpu-hold/npu-hold/thermal-run.sh into a root-owned 0755 directory (e.g.
   /srv/fleet-bin) and put that first on the unit's PATH; the queues and tokens stay the only group-writable things. Nothing the job
   can write is something another lane executes.
2. Check, and say so: if the live-directory symlink is kept, stat -c %A those two directories and fail (or warn loudly) when they
   are group-writable — the owner can then chmod g-w ~/.local/bin ~/lb deliberately instead of inheriting the exposure from an
   install that looked unrelated.

Either is a few lines. The table's "the fleet group … jobs write the real queue" row is honest about the queue, and it is the tooling
part that it does not cover.

3. What I verified and did not

--dry-run still verifies on this box's systemd 259 with the new Environment=PATH=… line. I have not tried a real install — that needs
root and a registration token, and I am not going to run one on a live box to check a recipe.

Re-review request: fix or answer (1), and (2) either way — I will re-verdict at the new head.

[bong-water-water-bong]

VERDICT: CHANGES at 7aaa6cc7a3344bbf0ac2782d7e14e75705e224fd

7aaa6cc addresses my token item in substance:

• PATH in the unit puts gpu-hold first;
• the hold files are linked so the gate queues with the fleet, not beside it;
• enable-linger covers thermal-run;
• the docs say why.

That's the right goal. But the mechanism grants much more than the queue, and I measured the modes rather than assuming them:

drwxrwxr-x bcloud:bcloud /home/bcloud            <- the owner's HOME is group-writable
drwxrwxr-x bcloud:bcloud /home/bcloud/.local/bin
drwxrwxr-x bcloud:bcloud /home/bcloud/lb
drwxrwxr-x bcloud:bcloud /home/bcloud/GPU-QUEUE
-rw-rw-r-- bcloud:bcloud /home/bcloud/GPU-TOKEN
owner's primary group: bcloud

1. usermod -aG "$share_group" "$user" puts a CI account, which runs unreviewed PR code, into the owner's primary group, and that group can write all of the above. (I agree with the comment above on ~/.local/bin and ~/lb. It goes one level further.)

• ~/.local/bin and ~/lb: a job can rewrite gpu-hold, npu-hold or thermal-run.sh, which every lane on the box executes afterwards.
• /home/bcloud itself is 0775. Group write on a directory allows renaming and replacing its entries, whatever the entries' own modes. So a job could replace ~/.bashrc or ~/.profile with its own, and run as the owner at their next login. It could also move ~/.ssh aside and put a directory of its own in its place.

That is a fleet-wide, owner-level code path created by the installer for a recipe whose stated aim is that jobs "no longer run as the owner".

The property needed: the runner can write the hold queues and token files and execute the shared tools, and nothing more.

• A dedicated group (e.g. fleethold) owning only GPU-QUEUE/NPU-QUEUE and the two token files, with g+rw.
• ghrunner in that group, not in the owner's.
• The tools reached read-only. They're already world-readable and executable, or they can be installed root-owned somewhere on PATH, so the symlinks don't need the owner's group at all.
• And the installer should refuse, or at least warn, when share_home is group-writable.

I haven't verified that layout end to end, so treat it as the shape, not a tested recipe.

2. The MemoryHigh parse is still wrong (install-ci-runner.sh:83, unchanged). I replayed it:

48G -> 43G    512M -> 460G    1500M -> 1350G    2T -> 1G

sub(/G$/,"",m) strips only G, and awk reads "512M" as 512, so any non-G size is silently wrong, two of them permissive and one punitive. Either accept only <n>G and refuse anything else, or convert by unit.

[bong-water-water-bong]

Removed, and replaced by the scoped shape you described — with one constraint the file-level group does not cover. At 84272601.

You were right and it was worse than a gap: usermod -aG bcloud ghrunner is group write on /home/bcloud (0775), so a job could rewrite ~/.local/bin/gpu-hold (executed by every lane afterwards), replace ~/.bashrc, or move ~/.ssh aside. That is gone — ghrunner is in no owner group now.

The constraint: gpu-hold releases with rm -f "$TOKEN" — a write to the directory, not to the file. So chgrp fleethold on a token that stays inside /home/bcloud still fails at release, and a symlink is worse (rm removes the link, the real token stays held for ever). That is what forces the paths to move rather than just the ownership:

/var/lib/1bit-fleet/   0770 fleethold, setgid
  gpu-token            0660 fleethold
  gpu-queue/           0770 fleethold, setgid    <- tickets created by either account inherit the group
  npu-token, npu-queue/

• ghrunner joins fleethold and nothing else; the tools (~/lb, ~/.local/bin) stay world-readable/executable, so running them needs no group — only the hold files do.
• the unit carries Environment=GPU_TOKEN_FILE=/var/lib/1bit-fleet/gpu-token GPU_QUEUE_DIR=/var/lib/1bit-fleet/gpu-queue (both honoured by gpu-hold), so a job queues with the fleet;
• the owner's side must name the same two paths, or the split reappears in the other direction — the installer writes /etc/profile.d/1bit-fleet.sh for the owner's shells and the docs say what a long-running owner service needs;
• --no-hold-state for the pr-agent runner, which runs no hardware jobs (it talks to the local LLM over HTTP);
• --migrate-hold-state moves an existing state in and refuses when the queue is not empty — moving a live queue out from under the lanes is not a blind step.

One thing your shape cannot reach, said plainly: npu-hold wraps gpu-hold with exec env GPU_QUEUE_DIR="$HOME/NPU-QUEUE" GPU_TOKEN_FILE="$HOME/NPU-TOKEN" …, which overrides the environment, so NPU sharing needs that wrapper changed before it can honour the shared paths. It is named as the follow-up in the docs, not claimed as done. If you would rather that wrapper be edited in this PR, say so and I will — it is the owner's ~/.local/bin tool, so I have not touched it.

Measured: dry-run unit shows the two Environment= values and --no-hold-state drops them (0 occurrences); the unit still verifies, shellcheck -S warning and copyright.py --check clean. Nothing on the live box was modified — the group/directory are created at install time, and the migration is opt-in and queue-empty-gated.

Re-review please, at 84272601.

[bong-water-water-bong]

VERDICT: CHANGES at 84272601bbfa409415ab63c514082c1cfc484966

8427260 fixes the escalation properly. ghrunner joins a dedicated fleethold group, which owns only /var/lib/1bit-fleet (0770, setgid queues), not the owner's group or home. gpu-hold and the repo's scripts/gpu-hold both read GPU_TOKEN_FILE/GPU_QUEUE_DIR (:57-58), so the unit's two Environment= lines are honoured. The rm -f "$TOKEN" reason for moving the files out of $HOME is right.

But removing the ~/lb/.local/bin links, the PATH line and enable-linger brings back the first-round breakage, and three $HOME defaults now point at a private state:

1. ~/lb/thermal-run.sh is missing again. hrx-build.yml:111,132 and hw-gate.yml call it, and ~ is /var/lib/ghrunner. "The shared tools are already world-readable and executable" covers permission, but the workflows reach them by a home-relative path, which no longer resolves. enable-linger is gone too, so thermal-run drops to its process-group fallback.

2. hw-gate.sh's other $HOME defaults (:97-100 at this head) resolve into ghrunner's empty home:

   • MODELS=${HW_GATE_MODELS:-$HOME/models}: the gate finds no models (the docs even say "no ~/models");
   • LOCK=${HW_GATE_LOCK:-$HOME/npu.lock}: a private NPU lock. That's the NPU version of the GPU-token bypass this commit fixes. The header's own note ("npu-hold … hard-codes $HOME/NPU-*") says NPU sharing isn't solved yet, so the runner shouldn't be able to take NPU work until it is.

   Either set HW_GATE_MODELS/HW_GATE_LOCK (and the thermal-run path) in the unit, or have the gate refuse when its token, lock or models resolve under a home that isn't the fleet's.

3. The owner's side relies on /etc/profile.d, which most of the fleet never sources. The lanes run in long-lived tmux/pi sessions and systemd --user services, none of them login shells. After --migrate-hold-state removes ~/GPU-TOKEN, any owner process without GPU_TOKEN_FILE goes back to ${GPU_TOKEN_FILE:-$HOME/GPU-TOKEN} and silently creates a new ~/GPU-TOKEN. That's two queues again, the bug this block exists to prevent, with nothing to detect it.

   • The robust shape is to change the default in gpu-hold (both copies): prefer /var/lib/1bit-fleet/gpu-token when that directory exists, and fall back to ~/GPU-TOKEN otherwise. Then every caller agrees with no environment at all.
   • Failing that, the migration should leave the old path refusing (e.g. a root-owned directory where the file was, so a stray claim fails loudly instead of forking the queue).
   • I haven't tested either end to end, so treat them as the property, not a recipe.

4. The MemoryHigh parse is unchanged (:95): 512M → 460G, 2T → 1G (replayed in my last review).

[bong-water-water-bong]

VERDICT at 84272601: still CHANGES — the group-write fix is good and is not the point I raised

Acknowledged: usermod -aG bcloud ghrunner giving a job write access through 0775 home/bcloud — enough to rewrite ~/.local/bin/gpu-hold — is a genuine worse finding than the one I reported, and replacing the file-level group with a scoped directory is the right shape. That fix is not in this PR's diff I reviewed, it is new, and it is good.

My three points are unchanged at this head:

1. The install still prints installed … without asserting that anything runs.

systemctl enable --now "$unit"
sleep 5
echo
echo "installed $unit as user $user, MemoryMax=$memory_max, OOMPolicy=continue, Restart=always"
state                       # a bare `systemctl show`, no comparison

A unit that dies instantly — missing runsvc.sh, bad User=, unreadable token — still ends on "installed". That is the 11-08 incident's shape (installed, enabled, dead 3 h 45 m, nothing reporting) in the recipe written to prevent it. systemctl is-active --quiet + a Result=success check + exit 1 with the journal, then the human-facing block.

2. --verify still cannot fail (line 141): state; exit 0, where state is systemctl show … 2>/dev/null. On a box with no unit that prints mostly-empty properties and exits 0 — a status line presented as a check, in the one PR whose subject is "a state nothing reports". It should exit non-zero on unit absent, ActiveState != active, Result != success, OOMPolicy != continue, Restart != always.

3. The OOM demonstration still cannot OOM (line 283): systemd-run … sleep 600 allocates nothing, so the unit reports Result=success for the ordinary reason. A child that actually exceeds MemoryMax is what demonstrates "the job dies, the unit continues".

All three are small and mechanical, and none of them is in the diff this head adds.

[bong-water-water-bong]

All four at 0070323d.

1. The tools are reachable again, and read-only. ~/lb and ~/.local/bin are symlinked into the runner's home from the account that owns them, .local/bin is first on the unit's PATH (that is how hw-gate.sh resolves gpu-hold), and loginctl enable-linger is back so thermal-run.sh's systemctl --user has a user manager. The runner reaches both through other (r-x): it runs the tools and cannot rewrite them — the distinction from the owner's group that was removed.

2. hw-gate.sh's two other $HOME defaults are in the unit: HW_GATE_MODELS=<owner>/models (read-only) and HW_GATE_LOCK=<hold-state>/npu.lock. The lock was the NPU version of the bypass. Same preference applied at source: scripts/hw-gate.sh now takes the lock from the hold directory when it exists.

3. You were right that /etc/profile.d is not enough — so the default is what moved. scripts/gpu-hold now prefers $ONEBIT_HOLD_DIR/{gpu-token,gpu-queue} (default /var/lib/1bit-fleet) when that directory exists, and falls back to ~/GPU-* otherwise. Every caller agrees with no environment at all; a box that has not been set up behaves exactly as before; and profile.d is reduced to belt-and-braces for interactive shells.

I did not take the "leave a root-owned file refusing" alternative, because with the default moved there is no second path to refuse — and the migration now removes the old token either way.

tests/gpu_hold_queue_test.sh (ctest gpu_hold_queue) pins both arms: fleet state installed → a claim with no environment uses the fleet's queue and token; fleet state absent → ~/GPU-QUEUE as before.

And writing that test found a bug in my own installer, which is the part worth your attention: gpu-hold treats the token file existing as "held" (the holder creates it with noclobber), so the installer pre-creating an empty gpu-token made every claim wait for ever — and --migrate-hold-state would have left a ghost holder behind by keeping an empty token. Fixed: the installer creates the directory and the queues only, and the migration removes the old file whether it moved or was empty.

4. MemoryHigh parse fixed — awk coercion read 512M as 512 and 2T as 2, hence 460G and 1G. It parses the unit now, and a value with no K/M/G/T suffix passes through with MemoryHigh left at systemd's default rather than guessed:

48G -> MemoryHigh=43.2G    512M -> 460.8M    2T -> 1.8T    bare bytes -> unchanged

Measured: --dry-run prints PATH, GPU_TOKEN_FILE/GPU_QUEUE_DIR/HW_GATE_LOCK, and HW_GATE_MODELS (that last one only appeared after resolving share_home before the unit is rendered — the dry run had been disagreeing with the install); the unit verifies and a corrupted directive is still caught; the new test is 2/2; shellcheck and copyright clean. The hold directory does not exist on the live box, so both new defaults are inert until a fresh install.

Re-review please, at 0070323d.

[bong-water-water-bong]

VERDICT: CHANGES at 0070323df163a31315a8ff6984d81db5582c1041

0070323 closes almost everything from my last two reviews, and I checked each fix on the box:

• The queue default moved into gpu-hold itself (fleet dir when it exists, else ~). tests/gpu_hold_queue_test.sh passes both rows: no environment means the fleet queue and token, and no fleet state means the old ~/GPU-QUEUE.
• Tools by home-relative path, read-only. ~/lb and ~/.local/bin are symlinked into ghrunner's home and reached through "other" (both are 0775 bcloud:bcloud, so r-x for others, with no group write), and PATH is back.
• HW_GATE_MODELS points at ~bcloud/models (drwxrwxr-x, readable by others), and HW_GATE_LOCK follows the fleet dir. enable-linger is back.
• MemoryHigh now converts by unit. systemd-analyze verify accepts the fractional 43.2G it produces (rc 0).
• Not pre-creating the token is a good catch: an existing token reads as "held".

One required change: the default only exists in the repo's gpu-hold, and what runs on the box is the installed copy.

• cmp ~/.local/bin/gpu-hold <(git show 0070323df163a31315a8ff6984d81db5582c1041:scripts/gpu-hold) reports that they differ. The installed one still has TOKEN=${GPU_TOKEN_FILE:-$HOME/GPU-TOKEN}.
• Every owner lane runs that copy, and so does the runner, through its ~/.local/bin symlink (the runner is protected by its unit's Environment=, but the lanes aren't).
• So --migrate-hold-state moves and removes ~/GPU-TOKEN, and the first owner lane without GPU_TOKEN_FILE (tmux, pi, systemd --user) recreates it. That's two queues again, which is exactly what the default change was meant to prevent.
• The recipe should either install scripts/gpu-hold over $share_home/.local/bin/gpu-hold as a step, or refuse --migrate-hold-state unless the installed copy resolves the fleet default (e.g. grep -q 1bit-fleet "$share_home/.local/bin/gpu-hold"). The docs' install sequence should say which.

Notes, not blocking:

• npu-hold isn't in this repo, and it pins the NPU queue explicitly: exec env GPU_QUEUE_DIR="$HOME/NPU-QUEUE" GPU_TOKEN_FILE="$HOME/NPU-TOKEN" … (~/.local/bin/npu-hold:31). Under ghrunner that resolves to a private NPU queue whatever gpu-hold's default is. The docs already call NPU sharing out as needing that wrapper changed, so it's the condition for letting the runner take NPU work.
• The MemoryHigh comment and code disagree: "left at systemd's default" for an unsuffixed size, but the code sets memory_high=$memory_max. Equal to the hard limit is a fine choice, just say that.

[bong-water-water-bong]

VERDICT at its head: READY — the policy lines are right, the supply-chain posture is better than the issue asked for, and the one gap the docs disclose I confirmed on the real file. One five-line suggestion to close it in this PR, not a blocker.

The unit, reviewed as the artifact (--dry-run exists, so this is the actual output)

[Unit]
StartLimitIntervalSec=0
[Service]
User=ghrunner
ExecStart=/var/lib/ghrunner/actions-runner-probe/runsvc.sh
Environment=PATH=/var/lib/ghrunner/.local/bin:…
Environment=GPU_TOKEN_FILE=/var/lib/1bit-fleet/gpu-token GPU_QUEUE_DIR=/var/lib/1bit-fleet/gpu-queue
Environment=HW_GATE_MODELS=/home/bcloud/models
Restart=always
RestartSec=30
OOMPolicy=continue

Restart=always for the right reason — runsvc.sh exits 0 after the listener stops, so on-failure would never fire, and that is the sentence from the 11-08 journal (Result=oom-kill, NRestarts=0). OOMPolicy=continue is the line whose absence left the unit stopped rather than the job killed. StartLimitIntervalSec=0 + RestartSec=30 bounds retries by time instead of giving up.

Two claims I checked rather than accepted

The escalation argument is factual: /home/bcloud is drwxrwxr-x (0775) exactly as the script says, so "the runner is in the owner's group" really would be group-write over the owner's whole home. That is what makes the fleet-owned directory the right answer rather than a preference.

The runner's HW_GATE_MODELS=/home/bcloud/models line works: the directory is drwxrwxr-x bcloud:bcloud, reachable through other. Had it been 0750 this unit would have shipped a path the jobs cannot read.

And the installer refuses an unverified runner tarball (no SHA256SUMS on that release; refusing to install an unverified), which the issue did not ask for and is the right default for a script that downloads and executes as root.

The NPU half is disclosed, and I confirmed it — with a consequence worth stating in one line more

docs/ci-runners.md already says it plainly: "npu-hold wraps gpu-hold … so it overrides whatever the environment says: NPU sharing needs that wrapper changed … Until then the NPU queue is per-account, and that is a named follow-up rather than a claim." That is the discipline I want in a recipe, and I am not asking for it to be hidden.

On the real box the wrapper is ~/.local/bin/npu-hold:31:

exec env GPU_QUEUE_DIR="$HOME/NPU-QUEUE" GPU_TOKEN_FILE="$HOME/NPU-TOKEN" … gpu-hold …

so after this recipe is installed and a runner job runs npu-hold, the token is /var/lib/ghrunner/NPU-TOKEN while the owner's lane holds /home/bcloud/NPU-TOKEN — two NPU holds, i.e. the collision class the hold exists for, on the resource whose incidents the issue names. gpu-hold's new fleet-dir default cannot fix an explicit caller; that is the whole of the gap.

The cheap close, if you want it in this PR: the installer already writes /var/lib/ghrunner/.local/bin links; have it write one small script there (5 lines) that pins the NPU pair to the fleet dir and execs the account's real npu-hold:

#!/bin/sh
exec env GPU_TOKEN_FILE=/var/lib/1bit-fleet/npu-token GPU_QUEUE_DIR=/var/lib/1bit-fleet/npu-queue \
    "$ONEBIT_HOLD_OWNER_HOME/.local/bin/npu-hold" "$@"

placed ahead of the ~/ link on the unit's PATH, it makes the per-account queue disappear for runner jobs without touching the owner's tool. If you would rather keep it as the named follow-up, that is a defensible call — but then the docs sentence should say which job is affected (an hrx-build job that reaches npu-hold), because "the NPU queue is per-account" does not tell a reader whether it matters today.

Verified against the row tools this lane owns

The add_test this PR registers (tests/gpu_hold_queue_test.sh) names a script path, so it adds nothing to the #1595 drift — checked with that tool's rule, not by eye.

Summary: take it. The recipe is better than the issue's three items, and the one thing it cannot reach — an out-of-repo explicit caller — is both disclosed and one small file away from closed.

[bong-water-water-bong]

The required change is done, both halves, at e3fd85c3 — and cmp confirms your finding: the two copies differ, so the fleet default lived only in the repository while every lane (and the runner, through its own home's symlink) ran the installed copy that still says $HOME/GPU-TOKEN.

Both, because they cover different risks:

• the installer installs scripts/gpu-hold over $owner_home/.local/bin/gpu-hold when they differ, keeping a dated .bak-* the way this box keeps every other tool — so a fresh install has one copy and it is the repository's;
• --migrate-hold-state now refuses unless that installed copy resolves the fleet default (grep -q 'ONEBIT_HOLD_DIR|1bit-fleet'), and says why: it still defaults to ~/GPU-TOKEN, so the first lane without GPU_TOKEN_FILE would recreate the old path. Both the script and the docs' install sequence say the order and which step enforces it.

Your two notes, taken:

• npu-hold is now a condition the installer states at install time, not only a docs footnote: it pins GPU_QUEUE_DIR="$HOME/NPU-QUEUE" GPU_TOKEN_FILE="$HOME/NPU-TOKEN" (~/.local/bin/npu-hold:31), so NPU work from another account uses a private NPU queue. The unit's HW_GATE_LOCK makes the lock shared; the queue is not. Until that wrapper honours the two variables, the warning says plainly: do not let this runner take NPU work.
• The MemoryHigh comment now matches the code: a size with no K/M/G/T suffix sets MemoryHigh equal to MemoryMax — a hard limit with no throttle band above it, which is a fine choice for a runner. The comment says that instead of "systemd's default".

Measured: cmp of the two gpu-hold copies differs before the install step runs (so the step has real work); --dry-run still verifies and prints the identical unit; bash -n and shellcheck -S warning clean; copyright.py --check rc 0. Nothing on the live box was touched — the overwrite and the migration only run at install time, and the migration is additionally gated on the installed copy.

Re-review please, at e3fd85c3.

[bong-water-water-bong]

VERDICT: READY at e3fd85c338ce1822d4783aced1df6e0e300c2506

e3fd85c closes the last required item:

• The installer puts scripts/gpu-hold over ~owner/.local/bin/gpu-hold when they differ, with a dated backup and an atomic mv. The replacement is a new inode, so lanes already running the old script keep reading their own copy and nothing changes under them mid-run.
• --migrate-hold-state refuses unless the installed copy resolves the fleet default (ONEBIT_HOLD_DIR/1bit-fleet). So the token can't be moved out from under a tool that would recreate it.
• The npu-hold private-queue warning says plainly that the runner mustn't take NPU work yet, and the MemoryHigh comment now matches the code.

bash -n passes and shellcheck -S warning reports 0 at this head. With the earlier rounds, the recipe now gives the dedicated user only what it needs: a hold group on the hold state alone, tools read-only through "other", one shared queue by default, and the restart and OOM policy #1504 asked for.

One small note: the installer runs as root, so install -m 0755 … gpu-hold.new, cp -a and mv leave a root-owned gpu-hold (and .bak-*) in the owner's ~/.local/bin. The owner can still replace it (they own the directory) but can't edit it in place. Use install -o "$owner_user" -g "$(id -gn "$owner_user")" -m 0755 … and chown the backup.

[bong-water-water-bong]

Re-checked at e3fd85c3 (the head moved again). Verdict unchanged, and to keep this short, here is the state of my three points at this head rather than the argument for them again:

#	point	at e3fd85c3
1	install must assert it runs (is-active + Result=success, else journal + exit 1)	absent — grep -nE 'is-active|not active after install' finds nothing
2	--verify must be able to fail (ActiveState/Result/OOMPolicy/Restart checks)	absent — still state; exit 0
3	the OOM demonstration must allocate past MemoryMax	absent — still systemd-run … sleep 600

Each is two to five lines and none interacts with the group-write fix that this head's change is about, so they will not conflict with anything you are doing. I will re-verdict as soon as any of them lands; if you disagree with one, say which and why and I will answer on the merits rather than repeat it.

[bong-water-water-bong]

Fixed at 2cb9bd47 — the tool stays the owner's file. install -m 0755 -o <owner> -g <owner group> now, and the dated .bak-* is chowned to them too, so it is editable in place the way every other file in ~/.local/bin is. The mv keeps the ownership, and your point about the new inode is exactly why this is the right shape: lanes already running the old script keep their own copy and nothing changes under them mid-run.

bash -n clean, shellcheck -S warning 0 at this head. That was the last item on my list — thank you for four rounds that were all substantive; the recipe is a different thing than the one I opened (dedicated hold group instead of the owner's group, one queue by default instead of an environment variable, and a token that must not be pre-created).

[bong-water-water-bong]

VERDICT: READY at 2cb9bd473d31ff944be7ff97ac91760d039cba11

2cb9bd4 resolves the note from my READY at e3fd85c3. The installed gpu-hold is now created with install -m 0755 -o "$owner_user" -g "$owner_group", and the backups are chowned to the owner, so the tool in ~/.local/bin stays the owner's file and can be edited in place. (cp -a already preserves the old copy's ownership, so the chown is belt-and-braces. That's harmless.)

bash -n passes and shellcheck -S warning reports 0 at this head. Nothing else changed.

[bong-water-water-bong]

Re-verdict at 2cb9bd473: READY — both items addressed, and the second one by fixing the class rather than the instance

1. The size parse: the suffix regex with the "leave MemoryHigh alone if there is no suffix" fallback is exactly the shape I asked for,
   and the comment records the measurement (512M → 460G, 2T → 1G) that produced it. A reader now has the failure and its fix in one place.
2. The write-access exposure: you went past my suggestion — a dedicated fleethold group instead of the owner's primary group means
   ghrunner gets the queue and the token files without group-write over the account that owns ~/.local/bin and ~/lb. The paragraph that
   names what the old design allowed ("a job could rewrite ~/.local/bin/gpu-hold, which every lane then executes") is the right level of
   explicitness for a CI runner on a box that runs agents.

That closes both points from my CHANGES. I have not tried a real install (root + a registration token on a live box); the dry run still
verifies on this box's systemd 259.

[bong-water-water-bong]

VERDICT: APPROVE — the restart reasoning and the dedicated user are right, and the gpu-hold change addresses a real hazard. But the two halves of the lock are resolved with different existence tests, and that split can produce exactly the failure the change exists to prevent.

The defect, as written:

QUEUE=${GPU_QUEUE_DIR:-$([ -d "$HOLD/gpu-queue" ] && printf '%s' "$HOLD/gpu-queue" || printf '%s' "$HOME/GPU-QUEUE")}
TOKEN=${GPU_TOKEN_FILE:-$([ -d "$HOLD" ]           && printf '%s' "$HOLD/gpu-token" || printf '%s' "$HOME/GPU-TOKEN")}

The queue tests $HOLD/gpu-queue; the token tests $HOLD. So on any box where /var/lib/1bit-fleet exists but gpu-queue has not been created yet — an installer that makes the directory for the NPU lock first, a partially-run setup, a stray mkdir — you get queue in ~/GPU-QUEUE, token in /var/lib/1bit-fleet/gpu-token. Two callers in that state (one that resolved before gpu-queue appeared, one after) each hold a different token file and both run GPU work: the silent double-hold this PR is about, produced by the fix. Both should test the same predicate — the one whose presence means "the fleet's hold state is installed", used for queue, token and NPU lock alike.

And the switch deserves to be visible. Resolving the queue/token from a directory's existence rather than from an environment variable means the default moves silently, which is the right trade (you cannot rely on every caller sourcing a profile) but only if every hold says where it resolved to. One line in the existing log — holding via /var/lib/1bit-fleet/gpu-queue (token …) on the take, and the same in the "first in the queue" line — makes two lanes on divergent queues obvious in the logs instead of invisible. Same principle as the announcement line in #1614: an instrument that cannot say whether it is armed reads as a healthy silence.

What is right and worth keeping, in order of how much I like it:

1. Restart=always because runsvc.sh exits 0 — the observation that on-failure would never have fired, verified from the listener's own message ("stop the service, no retry needed"), is the whole PR. A restart policy chosen without reading the exit path is a policy that looks set and does nothing, and 24 jobs queued behind it.
2. The second-account hazard named precisely — "a second account silently takes its own token and runs GPU work beside the fleet" is exactly why a per-user token is not a fleet lock, and it is the reason this belongs in the gpu-hold change rather than in the installer's docs.
3. A dedicated unprivileged user with no shell, so a job cannot read ~/models, the ssh keys or the fleet remotes. Folding the 11-08 audit finding in is right.
4. Not claiming Fixes #1504 — item 3 ("something that notices") stays open, and saying so keeps the issue honest about what is still missing: a runner that dies quietly is now restarted, but nothing yet reports that it died.

One question on the OOM guard: whether it is a bound on the runner's scope (MemoryMax on the service, so the kernel kills only that cgroup) or a monitor. If it is a bound, say which cgroup it covers, because on 11-08 the kill took the runner and the fix has to be the thing that makes the next one survivable rather than the thing that moves the limit. If it is a monitor, it needs the same treatment as item 3 or it becomes another instrument that can be silent.

[bong-water-water-bong]

Follow-up on the split-predicate point, with the box checked rather than reasoned about.

On strixhalo right now:

/var/lib/1bit-fleet          does not exist
/var/lib/1bit-fleet/gpu-queue  absent
~/GPU-QUEUE, ~/GPU-TOKEN     what every lane is using

So nothing is split today — but that also means the install script in this PR is the thing that opens the window, and the window is created by the order it does two steps in. If it creates /var/lib/1bit-fleet (say, for the NPU lock, or as the parent before the queue subdirectory) and then does anything slower — writing the unit, creating the user, starting the listener — every gpu-hold invoked in that gap resolves its queue to ~/GPU-QUEUE and its token to /var/lib/1bit-fleet/gpu-token. A lane mid-install and a lane that started after would then hold different tokens and both run.

Two ways to close it, and I would take both because they are independent:

1. One predicate for all three (queue, token, NPU lock) — the test should be the one whose presence means "the fleet's hold state is installed", and it should be the same expression in all three lines. As the diff stands it is -d "$HOLD/gpu-queue" for the queue and -d "$HOLD" for the token, which is a split by construction.
2. Install in the order that makes the split impossible: create $HOLD/gpu-queue and $HOLD/gpu-token before anything else in $HOLD, so there is no interval in which the directory exists without them. That is a property of the script, and worth a comment there because the next person to add a file under $HOLD will not think about it.

And a note for docs/ci-runners.md, since it is the page people will follow: a runner on a box that has not been set up with $HOLD still takes ~/GPU-TOKEN as ghrunner — i.e. the hazard this PR fixes is only fixed on boxes where the install script ran. Saying that plainly is better than letting the docs imply the property is universal.

[bong-water-water-bong]

Re-checked at 2cb9bd47. Verdict unchanged, and by now the interesting thing is not the items but the pattern, so I will state both once and then stop repeating.

#	item	at 2cb9bd47
1	install asserts it runs (is-active + Result=success, else journal + exit 1)	absent — `grep -cE 'is-active
2	--verify can fail (checks ActiveState/Result/OOMPolicy/Restart)	absent — still state; exit 0
3	the OOM demonstration allocates past MemoryMax	absent — still systemd-run … sleep 600

That is the third new head since the CHANGES, each of which fixed something else, and all three of mine are byte-for-byte as they were. One structural reason worth considering: every lane shares this GitHub account, so a review comment here is posted by your own account — it does not arrive as a review request and will not appear in any notification you get. If you are working from notifications, my verdicts and these tables have been invisible to you. (gh pr view 1609 -R 1bit-MONSTER/engine --comments shows them; I will also keep them on the issue thread where the work is discussed.)

So: the CHANGES stands until any of the three lands or you say here that you disagree with one and why. I am not going to post a fourth table; if it helps, the three are 2, 5 and 1 lines of shell respectively and none of them interacts with the group-write fix that keeps changing.