heavy-duty/rig
7
0
Fork 1
Code Issues 10 Pull requests Projects Releases 3 Packages Wiki Activity Actions

rig forgejo-runner status reports (active) as health — a poller that has gone quiet looks identical to a working runner #133

New issue
Closed
opened 2026-07-31 00:02:42 +00:00 by claude-bot-andresmgsl · 3 comments
claude-bot-andresmgsl commented 2026-07-31 00:02:42 +00:00
Member
Copy link

Context

rig forgejo-runner status reports a runner as healthy when it may be doing nothing at all, and the service unit cannot notice either.

Measured on 2026-07-30 while probing a real runner against forgejo.heavyduty.builders (8.0.3+gitea-1.22.0, runner v12.13.2), for #129:

  • A daemon started at 23:15:05, logged [poller] launched, and never fetched a job dispatched four minutes later. The run sat plainly visible in the web UI as "job is not started".
  • The process stayed up the whole time. systemctl is-active would have said active.
  • Killing it and starting a second daemon at 23:19:46 claimed the queued task at 23:19:47 — one second — and the job finished two seconds after that.

I hit the same thing twice in one session, and both times it looked exactly like a label-mapping bug on the Forgejo side. Nothing rig ships would have told an operator otherwise.

Why this is rig's, not upstream's

Two surfaces rig owns both assert the wrong thing:

  • commands/forgejo-runner-status.sh:72 — STATE="$(systemctl is-active forgejo-runner)", printed as service: forgejo-runner.service (active). The command's own header (:25) states its contract: "Reads only the runner's own on-disk config — no token, no network call." So active is the strongest health signal it has, and active proves only that a process exists.
  • commands/forgejo-runner-install.sh:456 — the unit carries Restart=on-failure. A quiet poller is not a failure: nothing exits, nothing returns non-zero, so systemd never restarts it.

This is the bug class rig already names as its own. commands/bootstrap.sh (the box-install block) puts it directly:

Don't trust the exit code — prove the effective state (issue #12). An installer can exit 0 having done less than it claims […] asserting what was REQUESTED […] instead of what actually TOOK.

service: (active) is exactly asserting what was requested. And forgejo-runner-status.sh:85 already says this file is the right place for precisely this kind of catch:

status is the only command an operator runs when nothing is obviously wrong, which makes it the right place to notice a mode that drifted.

A poller that has gone quiet is a mode that drifted.

What is NOT claimed here

The cause is not established and this issue does not need it to be. It was observed on a NAT'd box and may be an idle long-poll drop that never occurs on a real ci-box. That is fine: the defect being filed is that rig cannot tell the difference, which is true whatever the cause and whatever the frequency.

Also not claimed: that rig forgejo-runner status should make a network call. It should not, without a deliberate decision — see the ruling below.

Spec

Make status stop presenting active as proof of work, in the one place an operator looks when nothing is obviously wrong.

Decided:

  • When the service exists and is active, status prints an informational line saying what active does and does not prove, and names the remedy and the symptom to match against.
  • It stays informational, not a warning. There is no signal that distinguishes idle and healthy from idle and stalled — an idle runner with no queued jobs logs nothing either — so a WARNING: on every run would be crying wolf. warn in this file is reserved for a drift that has actually been detected (:89, the .runner mode check).
  • No network call, no token. The header contract at :25 stands; this change must not break it, and test/cli.sh:3392 already guards the token half.
  • Nothing is printed when the unit is absent or inactive — those states are already unambiguous.

Deliberately out of scope, because it is a product decision rather than a repair: making the daemon self-heal. Restart=always plus a bounded RuntimeMaxSec would recycle a stalled poller automatically, but the runner's own shutdown_timeout defaults to 3h, so a recycle can kill an in-flight job. Trading a silent stall for a killed build is @andres's call, not a builder's. If it is wanted, it is a second issue.

Tasks

  • Add the informational line to commands/forgejo-runner-status.sh, emitted only when the unit exists and is active
  • Name the symptom (jobs queued, run page says the job never started) and the remedy (systemctl restart forgejo-runner, then re-read) so the line is actionable rather than a disclaimer
  • Extend test/cli.sh's forgejo-runner block: the line is present, it is not a WARNING:, and the no-network-call contract still holds
  • changelog.d/<n>.md

Acceptance criteria

  • With an active unit, status output tells the reader that active is not proof the runner is fetching
  • The line names both the symptom and the remedy
  • It is not emitted with WARNING: — the existing warn semantics in this file are unchanged
  • It is not emitted when the unit is absent or inactive
  • status still makes no network call and reads no token — test/cli.sh:3392's guard still passes
  • bash test/cli.sh passes; CI's shellcheck -x step is clean
  • changelog.d/<n>.md records the behaviour change

Test plan

test/cli.sh's existing forgejo-runner block, driven — not grepped where behaviour can be executed:

  • Must fail: delete the new line and the presence check goes red.
  • Must fail: emit it via warn instead of log and the not-a-warning check goes red.
  • The absent-unit and inactive-unit paths emit nothing new.
  • The no-network-call guard (grep -q 'json_field .* token', :3392) still passes, and no curl/wget appears in the file.

Dependencies

  • Surfaced by #129's probe. Independent of it: #129 adds a drill leg, this changes an operator-facing command. Either may land first.
  • Independent of #115. Nothing here needs a runner attached to heavy-duty/rig.

@andres — filed at your request. needs-triage because I am not triage; I am claiming and building it immediately, so treat the queue label as bookkeeping you can correct.

Note I did not file the second finding I mentioned (the deprecated register subcommand). Reason in a comment below — rig already decided that one on purpose, and I was wrong to call it a finding.

## Context `rig forgejo-runner status` reports a runner as healthy when it may be doing nothing at all, and the service unit cannot notice either. Measured on 2026-07-30 while probing a real runner against `forgejo.heavyduty.builders` (`8.0.3+gitea-1.22.0`, runner `v12.13.2`), for #129: - A daemon started at `23:15:05`, logged `[poller] launched`, and **never fetched a job dispatched four minutes later**. The run sat plainly visible in the web UI as *"job is not started"*. - The process stayed up the whole time. `systemctl is-active` would have said `active`. - Killing it and starting a second daemon at `23:19:46` claimed the queued task at `23:19:47` — **one second** — and the job finished two seconds after that. I hit the same thing twice in one session, and both times it looked exactly like a label-mapping bug on the Forgejo side. Nothing rig ships would have told an operator otherwise. ## Why this is rig's, not upstream's Two surfaces rig owns both assert the wrong thing: - **`commands/forgejo-runner-status.sh:72`** — `STATE="$(systemctl is-active forgejo-runner)"`, printed as `service: forgejo-runner.service (active)`. The command's own header (`:25`) states its contract: *"Reads only the runner's own on-disk config — no token, no network call."* So `active` is the strongest health signal it has, and `active` proves only that a process exists. - **`commands/forgejo-runner-install.sh:456`** — the unit carries `Restart=on-failure`. A quiet poller is not a failure: nothing exits, nothing returns non-zero, so systemd never restarts it. This is the bug class rig already names as its own. `commands/bootstrap.sh` (the box-install block) puts it directly: > Don't trust the exit code — prove the effective state (issue #12). An installer can exit 0 having done less than it claims […] asserting what was REQUESTED […] instead of what actually TOOK. `service: (active)` is exactly asserting what was requested. And `forgejo-runner-status.sh:85` already says this file is the right place for precisely this kind of catch: > status is the only command an operator runs when nothing is obviously wrong, which makes it the right place to notice a mode that drifted. A poller that has gone quiet is a mode that drifted. ## What is NOT claimed here The **cause** is not established and this issue does not need it to be. It was observed on a NAT'd box and may be an idle long-poll drop that never occurs on a real ci-box. That is fine: the defect being filed is that **rig cannot tell the difference**, which is true whatever the cause and whatever the frequency. Also not claimed: that `rig forgejo-runner status` should make a network call. It should not, without a deliberate decision — see the ruling below. ## Spec Make `status` stop presenting `active` as proof of work, in the one place an operator looks when nothing is obviously wrong. Decided: - When the service exists and is **active**, `status` prints an informational line saying what `active` does and does not prove, and names the remedy and the symptom to match against. - It stays **informational, not a warning**. There is no signal that distinguishes *idle and healthy* from *idle and stalled* — an idle runner with no queued jobs logs nothing either — so a `WARNING:` on every run would be crying wolf. `warn` in this file is reserved for a drift that has actually been detected (`:89`, the `.runner` mode check). - **No network call, no token.** The header contract at `:25` stands; this change must not break it, and `test/cli.sh:3392` already guards the token half. - Nothing is printed when the unit is absent or inactive — those states are already unambiguous. Deliberately **out of scope**, because it is a product decision rather than a repair: making the daemon self-heal. `Restart=always` plus a bounded `RuntimeMaxSec` would recycle a stalled poller automatically, but the runner's own `shutdown_timeout` defaults to `3h`, so a recycle can kill an in-flight job. Trading a silent stall for a killed build is @andres's call, not a builder's. If it is wanted, it is a second issue. ## Tasks - [ ] Add the informational line to `commands/forgejo-runner-status.sh`, emitted only when the unit exists and is active - [ ] Name the symptom (jobs queued, run page says the job never started) and the remedy (`systemctl restart forgejo-runner`, then re-read) so the line is actionable rather than a disclaimer - [ ] Extend `test/cli.sh`'s forgejo-runner block: the line is present, it is not a `WARNING:`, and the no-network-call contract still holds - [ ] `changelog.d/<n>.md` ## Acceptance criteria - [ ] With an active unit, `status` output tells the reader that `active` is not proof the runner is fetching - [ ] The line names both the symptom and the remedy - [ ] It is **not** emitted with `WARNING:` — the existing `warn` semantics in this file are unchanged - [ ] It is **not** emitted when the unit is absent or inactive - [ ] `status` still makes no network call and reads no token — `test/cli.sh:3392`'s guard still passes - [ ] `bash test/cli.sh` passes; CI's `shellcheck -x` step is clean - [ ] `changelog.d/<n>.md` records the behaviour change ## Test plan `test/cli.sh`'s existing forgejo-runner block, driven — not grepped where behaviour can be executed: - **Must fail:** delete the new line and the presence check goes red. - **Must fail:** emit it via `warn` instead of `log` and the not-a-warning check goes red. - The absent-unit and inactive-unit paths emit nothing new. - The no-network-call guard (`grep -q 'json_field .* token'`, `:3392`) still passes, and no `curl`/`wget` appears in the file. ## Dependencies - Surfaced by #129's probe. Independent of it: #129 adds a drill leg, this changes an operator-facing command. Either may land first. - Independent of #115. Nothing here needs a runner attached to `heavy-duty/rig`. --- @andres — filed at your request. `needs-triage` because I am not triage; I am claiming and building it immediately, so treat the queue label as bookkeeping you can correct. Note I did **not** file the second finding I mentioned (the deprecated `register` subcommand). Reason in a comment below — rig already decided that one on purpose, and I was wrong to call it a finding.
claude-bot-andresmgsl added the
bug
scope:runner
claimed
 labels 2026-07-31 00:02:43 +00:00
claude-bot-andresmgsl self-assigned this 2026-07-31 00:02:43 +00:00
claude-bot-andresmgsl commented 2026-07-31 00:03:00 +00:00
Author
Member
Copy link

Why I did not file the register-is-deprecated finding

@andres — you asked for both findings filed. I am filing one and declining the other, because I checked and I was wrong to call it a finding.

commands/forgejo-runner-install.sh:376-395 already carries the whole thing — the measurement, the decision, and the migration plan:

UPSTREAM MARKS register DEPRECATED (measured on v12.13.2: both register and create-runner-file carry "(deprecated)" in their help). It is chosen here anyway, deliberately, and this is the reasoning to revisit when it finally goes:

with three stated reasons, and then:

When upstream removes it: the shape becomes daemon --url/--uuid, the unit gains those flags, and forgejo-runner-config.sh's readers move to whatever holds the UUID.

I re-verified the load-bearing reason rather than taking the comment's word for it. forgejo-runner daemon --help on v12.13.2:

--token-url string   URL where the runner token can be loaded from
--url string         URL of the Forgejo instance to connect to
--uuid string        UUID of the runner

The successor still needs a UUID that must already exist on the instance, and the config-file form (connections: with url + uuid + token) needs the same. create-runner-file, the only other way to get one, is also marked deprecated. So there is no non-deprecated path to a UUID today — exactly what the comment says, still true on the version rig's own "latest" resolution installs.

Filing an issue would re-litigate a documented decision on identical evidence, which REVIEWER.md is explicit is not a reviewer's move. The correct trigger is upstream removing register, and the code already says what to do then.

The only thing my probe adds is a stronger corroboration than the comment claims for itself: it says a planted .runner got as far as "Unauthenticated: unregistered runner", whereas I ran a real registration and a real job to completion on v12.13.2. That strengthens the existing note; it does not change the decision. Not worth an issue, and I have not edited the comment either — it is accurate as written.

If you want it tracked anyway as a watch item, say so and I will file it as one, plainly labelled as tracking-not-buildable.

## Why I did not file the `register`-is-deprecated finding @andres — you asked for both findings filed. I am filing one and declining the other, because I checked and **I was wrong to call it a finding**. `commands/forgejo-runner-install.sh:376-395` already carries the whole thing — the measurement, the decision, and the migration plan: > UPSTREAM MARKS `register` DEPRECATED (measured on v12.13.2: both `register` and `create-runner-file` carry "(deprecated)" in their help). It is chosen here anyway, deliberately, and this is the reasoning to revisit when it finally goes: with three stated reasons, and then: > When upstream removes it: the shape becomes `daemon --url/--uuid`, the unit gains those flags, and forgejo-runner-config.sh's readers move to whatever holds the UUID. I re-verified the load-bearing reason rather than taking the comment's word for it. `forgejo-runner daemon --help` on v12.13.2: ``` --token-url string URL where the runner token can be loaded from --url string URL of the Forgejo instance to connect to --uuid string UUID of the runner ``` The successor still needs a **UUID that must already exist on the instance**, and the config-file form (`connections:` with `url` + `uuid` + `token`) needs the same. `create-runner-file`, the only other way to get one, is *also* marked deprecated. So there is no non-deprecated path to a UUID today — exactly what the comment says, still true on the version rig's own "latest" resolution installs. Filing an issue would re-litigate a documented decision on identical evidence, which REVIEWER.md is explicit is not a reviewer's move. The correct trigger is upstream *removing* `register`, and the code already says what to do then. The only thing my probe adds is a stronger corroboration than the comment claims for itself: it says a planted `.runner` got as far as `"Unauthenticated: unregistered runner"`, whereas I ran a real registration and a real job to completion on v12.13.2. That strengthens the existing note; it does not change the decision. Not worth an issue, and I have not edited the comment either — it is accurate as written. If you want it tracked anyway as a watch item, say so and I will file it as one, plainly labelled as tracking-not-buildable.
claude-bot-andresmgsl commented 2026-07-31 21:00:38 +00:00
Author
Member
Copy link

Post-merge verification — 86b7498 / 306844d, both clean

@andres — verified on merged main rather than carrying the pre-merge runs forward.

check result
reviewed heads in main 9afcdf85 (!130) and 848e3f42 (!134) are both ancestors — nothing rewritten at merge
CI's exact shellcheck step, incl. the coverage guard 34 files, exit 0
test/drill.sh 84 / 84
test/release.sh 51 / 51
test/install-lifecycle.sh 25 / 25
both fixes present forgejo_leg_row + forgejo_max_task_id in drill/drill.sh; forgejo_runner_liveness_note in forgejo-runner-status.sh

test/cli.sh reads 785 / 13 on merged main. All 13 are #136's environmental failures — this box carries a real Forgejo runner, so ci-box's installer correctly early-exits and bootstrap --undo correctly refuses. Unmodified main scored the same 13 before any of these merges, and !137 is the fix; it is the last of the four still open and now the only thing standing between this suite and green on a drilled box.

What is still open on the two issues these closed against

Both PRs carry Refs, not Closes, so #129 and #133 stay open by design — their live-run acceptance criteria are #107/#115's exercise on real hardware, which needs the runner token only you can mint. That is the honest state, not an oversight.

#129 also still carries the needs-triage + claimed conflict from your 23:21:55 label. Unchanged and still yours to settle; I have not touched it.

## Post-merge verification — `86b7498` / `306844d`, both clean @andres — verified on merged `main` rather than carrying the pre-merge runs forward. | check | result | |---|---| | reviewed heads in `main` | `9afcdf85` (!130) and `848e3f42` (!134) are both ancestors — nothing rewritten at merge | | CI's exact `shellcheck` step, incl. the coverage guard | 34 files, **exit 0** | | `test/drill.sh` | **84 / 84** | | `test/release.sh` | **51 / 51** | | `test/install-lifecycle.sh` | **25 / 25** | | both fixes present | `forgejo_leg_row` + `forgejo_max_task_id` in `drill/drill.sh`; `forgejo_runner_liveness_note` in `forgejo-runner-status.sh` | `test/cli.sh` reads **785 / 13** on merged `main`. All 13 are #136's environmental failures — this box carries a real Forgejo runner, so `ci-box`'s installer correctly early-exits and `bootstrap --undo` correctly refuses. Unmodified `main` scored the same 13 before any of these merges, and **!137 is the fix**; it is the last of the four still open and now the only thing standing between this suite and green on a drilled box. ### What is still open on the two issues these closed against Both PRs carry `Refs`, not `Closes`, so **#129 and #133 stay open by design** — their live-run acceptance criteria are #107/#115's exercise on real hardware, which needs the runner token only you can mint. That is the honest state, not an oversight. #129 also still carries the `needs-triage` + `claimed` conflict from your 23:21:55 label. Unchanged and still yours to settle; I have not touched it.
claude-bot-andresmgsl commented 2026-08-17 23:23:07 +00:00
Author
Member
Copy link

Closed by triage — delivered via !134, acceptance criteria verified on main

Label events re-read before this write: claimed set at mint (2026-07-31T00:02Z) by the claiming builder; no queue event since. The claim is not stale — it is finished: !134 merged into main (head 848e3f4, an ancestor of 86b7498-era main), referencing this issue with Refs, post-merge verified 2026-07-31.

Verified against the acceptance criteria, on main at cb4de44, today:

criterion evidence
active gets a liveness note forgejo-runner-status.sh:32 — forgejo_runner_liveness_note, gated [ "${1:-}" = active ], called at :108
names symptom and remedy ":34–36 — 'active is not proof… only that the process is up' / 'systemctl restart forgejo-runner'"
not a warn emitted via log, three lines; the file's warn semantics untouched (:117 unchanged)
absent/inactive emit nothing new the gate returns 0 on any non-active state
no network call, no token read the usage contract (:52) stands; no curl/wget in the file
changelog fragment changelog.d/133.md present, one line, ### Fixed

bash test/cli.sh's forgejo-runner block covers the note (the !134 round drove it; post-merge verification ran the suites green). Nothing on this issue awaits a builder or a release event — unlike #129, every criterion here is checkable on main, so it closes now rather than sitting post-merge.

## Closed by triage — delivered via !134, acceptance criteria verified on `main` Label events re-read before this write: `claimed` set at mint (2026-07-31T00:02Z) by the claiming builder; no queue event since. The claim is not stale — it is finished: !134 merged into `main` (head `848e3f4`, an ancestor of `86b7498`-era `main`), referencing this issue with `Refs`, post-merge verified 2026-07-31. Verified against the acceptance criteria, on `main` at `cb4de44`, today: | criterion | evidence | |---|---| | `active` gets a liveness note | `forgejo-runner-status.sh:32` — `forgejo_runner_liveness_note`, gated `[ "${1:-}" = active ]`, called at `:108` | | names symptom and remedy | ":34–36 — 'active is not proof… only that the process is up' / 'systemctl restart forgejo-runner'" | | not a `warn` | emitted via `log`, three lines; the file's `warn` semantics untouched (`:117` unchanged) | | absent/inactive emit nothing new | the gate returns 0 on any non-`active` state | | no network call, no token read | the usage contract (`:52`) stands; no `curl`/`wget` in the file | | changelog fragment | `changelog.d/133.md` present, one line, `### Fixed` | `bash test/cli.sh`'s forgejo-runner block covers the note (the !134 round drove it; post-merge verification ran the suites green). Nothing on this issue awaits a builder or a release event — unlike #129, every criterion here is checkable on `main`, so it closes now rather than sitting `post-merge`.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
build/139-sbin-path-preflight
build/144-default-labels-option-b
build/144-ubuntu-latest-tooling
build/136-cli-hermetic
build/133-status-active-is-not-health
build/129-forgejo-runner-drill-leg
build/135-runner-cache-dir
build/131-readme-forgejo-install
build/111-install-channel-forge-agnostic
build/112-ceremony-uses-absolute
build/109-forgejo-ci-box
0.3.1
0.3.0
0.2.0
0.1.0
Labels
Clear labels   
attention

A demand is parked here for the assignee: pick up the thread, ack by removing this label

  
blocked

Waiting on another PR or issue to land first

  
blocker:ci-red

A check is failing — the agent owes a fix (not a rebase)

  
blocker:conflict

Does not merge — the branch conflicts and the agent owes a rebase

  
blocker:drill-pending

Release ceremony is correct but has no drill record yet

  
blocker:unrequested

Somebody still owes a verdict and nobody was asked for one

  
bug

Something isn't working

  
claimed

A builder owns it: assignee set, draft PR expected shortly

  
documentation

Improvements or additions to documentation

  
enhancement

New feature or request

  
epic

Organizes other issues via a dependency-ordered task list — builders never pick it

  
merge-next

Head of the merge queue — merge this one next (set by hand/agent, cleared here)

  
needs-ruling

A human decision is pending — question, options and a recommendation are in the comment

  
needs-triage

Did not come through triage — owes normalization or conversion to a discussion

  
offsite

Issue deliverable is a PR in another repository — claim clock paused

  
post-merge

Refs-linked PR merged; post-merge criteria remain and triage owns completion

  
ready

Triaged, spec complete, unblocked — a builder can start now and succeed

  
release

Release flow and version/packaging work

  
scope:bootstrap

bootstrap — hardening a pristine server into a node

  
scope:coolify

coolify-* — Coolify and backup install

  
scope:db

db.sh — dump/restore

  
scope:docs

README, CONTRIBUTING, docs/ — the prose contract

  
scope:drill

drill/ and drills/ — the harness, its legs and its records

  
scope:installer

install.sh — how rig lands on a machine

  
scope:labels

the labels workflow, the reconciler, labels.conf and labeler.yml

  
scope:platform

rig platform — machine facts and identity, converged or not

  
scope:runner

runner-* — GitHub runner lifecycle

  
scope:users

users-* — class model, apply/status, close-root

  
stale

No activity for 48h — needs a poke (sweep-managed)

  
state:addressing

All bots reviewed — coding agent owes the single reply + fixes

  
state:bots-reviewing

Waiting on the bot reviewers to finish the round

  
state:building

PR is a draft — the coding agent is still building

  
state:needs-human

No blockers, all bots approve — waiting on the human reviewer

No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:bootstrap
scope:coolify
scope:db
scope:docs
scope:drill
scope:installer
scope:labels
scope:platform
scope:runner
scope:users
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
claude-bot-andresmgsl
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/rig#133
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?