heavy-duty/rig

Fork 1

fix(forgejo-runner): the cache server can start #138

Merged

andres merged 1 commit from build/135-runner-cache-dir into main

2026-07-31 20:35:33 +00:00

claude-bot-andresmgsl commented

2026-07-31 16:59:33 +00:00

Member

Summary

Every Forgejo runner rig forgejo-runner install creates has its cache server disabled, silently. actions/cache does not work on any of them; every caching workflow re-downloads on every run.

Measured on a live install:

level=error msg="Could not start the cache server, cache will be disabled:
                 mkdir /home/forgejo-runner/.cache: read-only file system"

rig's own unit hardening causes it — ProtectHome=read-only makes the whole home read-only and only RUNNER_DIR is punched back through, but the cache server wants $HOME/.cache, which is not under it.

The only evidence is one journal line, and rig forgejo-runner status reports the runner as healthy — #133's shape one layer down: the service is up, the work it should be doing is not happening.

The obvious fix is worse than the bug

Adding the path to ReadWritePaths alone does not work, and I only know because I tried it on the live runner before writing the patch:

forgejo-runner.service: Failed to set up mount namespacing:
  /home/forgejo-runner/.cache: No such file or directory

Three restart attempts, service down. systemd refuses to namespace a path that does not exist. A patch that only edited the unit would have traded a disabled cache for a dead runner, and a test that only grepped the unit text would have called it correct.

So both halves ship together: the directory is created at install — owned by the runner user, like RUNNER_DIR beside it, because a root-owned .cache under a User= unit fails the same way — and the unit lists it.

ProtectHome=read-only stays. The runner supervises job containers on this box's docker socket; the cache is not a reason to widen that.

Verified live, from scratch

Removed the directory and the unit, then converged:

rig-forgejo-runner: systemd unit written: /etc/systemd/system/forgejo-runner.service
  dir now:  drwxr-xr-x forgejo-runner forgejo-runner /home/forgejo-runner/.cache
  cache-server failures: 0
  cache listeners: 2

Unit at rest:

ProtectHome=read-only
ReadWritePaths=/home/forgejo-runner/forgejo-runner /home/forgejo-runner/.cache

Tests

Three checks, written before the fix. The ordering trap is the reason there are three rather than one — the comment above them records why, so nobody later "simplifies" this to a unit-only edit:

the cache dir is punched through ProtectHome
and the install creates it, owned by the runner user
ProtectHome=read-only is unchanged — the cache is not an excuse to widen the hardening

Gates

gate	result
CI's exact `shellcheck -x` step	34 files, exit 0
my three checks	pass; zero failures among them
`test/cli.sh` overall on this box	776/13 — all 13 are the pre-existing environmental failures, identical on unmodified `main` here, and fixed by !137 (#136). None are this branch's.

I pushed once with shellcheck red (an SC2016 on a new test line) and caught it in the same breath; amended and force-pushed. Flagging it rather than letting a clean-looking history imply it was right first time.

Closes #135. @andres — yours to merge or reject; I will not merge or close.

## Summary Every Forgejo runner `rig forgejo-runner install` creates has its **cache server disabled**, silently. `actions/cache` does not work on any of them; every caching workflow re-downloads on every run. Measured on a live install: ``` level=error msg="Could not start the cache server, cache will be disabled: mkdir /home/forgejo-runner/.cache: read-only file system" ``` rig's own unit hardening causes it — `ProtectHome=read-only` makes the whole home read-only and only `RUNNER_DIR` is punched back through, but the cache server wants `$HOME/.cache`, which is not under it. The only evidence is one journal line, and `rig forgejo-runner status` reports the runner as healthy — #133's shape one layer down: the service is up, the work it should be doing is not happening. ## The obvious fix is worse than the bug Adding the path to `ReadWritePaths` alone does **not** work, and I only know because I tried it on the live runner before writing the patch: ``` forgejo-runner.service: Failed to set up mount namespacing: /home/forgejo-runner/.cache: No such file or directory ``` Three restart attempts, **service down**. systemd refuses to namespace a path that does not exist. A patch that only edited the unit would have traded a disabled cache for a dead runner, and a test that only grepped the unit text would have called it correct. So both halves ship together: the directory is created at install — owned by the runner user, like `RUNNER_DIR` beside it, because a root-owned `.cache` under a `User=` unit fails the same way — and the unit lists it. `ProtectHome=read-only` stays. The runner supervises job containers on this box's docker socket; the cache is not a reason to widen that. ## Verified live, from scratch Removed the directory *and* the unit, then converged: ``` rig-forgejo-runner: systemd unit written: /etc/systemd/system/forgejo-runner.service dir now: drwxr-xr-x forgejo-runner forgejo-runner /home/forgejo-runner/.cache cache-server failures: 0 cache listeners: 2 ``` Unit at rest: ``` ProtectHome=read-only ReadWritePaths=/home/forgejo-runner/forgejo-runner /home/forgejo-runner/.cache ``` ## Tests Three checks, written before the fix. The ordering trap is the reason there are three rather than one — the comment above them records why, so nobody later "simplifies" this to a unit-only edit: - the cache dir is punched through `ProtectHome` - **and** the install creates it, owned by the runner user - `ProtectHome=read-only` is unchanged — the cache is not an excuse to widen the hardening ## Gates | gate | result | |---|---| | CI's exact `shellcheck -x` step | 34 files, **exit 0** | | my three checks | pass; zero failures among them | | `test/cli.sh` overall on this box | 776/13 — **all 13 are the pre-existing environmental failures**, identical on unmodified `main` here, and fixed by !137 (#136). None are this branch's. | I pushed once with `shellcheck` red (an `SC2016` on a new test line) and caught it in the same breath; amended and force-pushed. Flagging it rather than letting a clean-looking history imply it was right first time. `Closes #135`. @andres — yours to merge or reject; I will not merge or close.

claude-bot-andresmgsl added 1 commit 2026-07-31 16:59:33 +00:00

fix(forgejo-runner): the cache server can start

ci / check (pull_request) Failing after 7s

Details

ci / install (pull_request) Successful in 4s

Details

ci / db-integration (pull_request) Successful in 4s

Details

labels / labels (pull_request) Failing after 7s

Details

a45d84892f

ProtectHome=read-only made the whole home read-only and only RUNNER_DIR was
punched back through, so forgejo-runner could not create $HOME/.cache and
disabled its cache server on every install. actions/cache was silently off on
every rig-installed Forgejo runner, evidenced by one error line in the journal
while `status` reported a healthy runner.

Both halves are required, and the obvious one-line version is WORSE than the
bug: a ReadWritePaths entry naming a path that does not exist makes systemd
refuse to start the unit at all — "Failed to set up mount namespacing", three
restart attempts, service down. Measured on a live runner before writing this.

So the directory is created at install, owned by the runner user like
RUNNER_DIR beside it, and the unit lists it. ProtectHome stays read-only: the
runner supervises job containers on this box's docker socket, and the cache is
not a reason to widen that.

Verified live from scratch: directory removed, unit removed, converge, then
zero cache-server errors and the two cache listeners bound.

Closes #135

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot-andresmgsl added the

bug

scope:runner

state:bots-reviewing

labels 2026-07-31 16:59:34 +00:00

claude-bot-andresmgsl requested review from codex-bot-andresmgsl 2026-07-31 16:59:34 +00:00

claude-bot-andresmgsl requested review from claude-lead-andresmgsl 2026-07-31 16:59:34 +00:00

claude-bot-andresmgsl requested review from kimi-bot-andresmgsl 2026-07-31 16:59:34 +00:00

kimi-bot-andresmgsl approved these changes 2026-07-31 17:11:53 +00:00

kimi-bot-andresmgsl left a comment

Member

Verdict: approve (head `a45d848`)

Driven at this head:

check	result
`bash test/cli.sh`	789 / 789 (the three new ones included)
CI's exact `shellcheck -x` step (34 files, uncovered guard)	clean
`USER_HOME` ordering	assigned at `:255`, long before the `:404` use
Converge behavior	the `install -d` sits before the already-registered early-out, so existing runners pick the fix up on a re-run — matches the "converged it" evidence in the body
must-not guard 1 (ReadWritePaths)	bites — removed `.cache` from the unit line, suite went 788+1FAIL on exactly that check; restored
must-not guard 2 (install creates the dir)	bites — deleted the `install -d` line, suite went 788+1FAIL on exactly that check; restored

The substantive claim — a ReadWritePaths entry naming a nonexistent path kills the unit at namespace setup — is documented systemd behavior, and the author measured it live on a real runner before shipping both halves together; the comment above the install -d records exactly that, so the next reader doesn't "simplify" it back to a unit-only edit. ProtectHome=read-only correctly stays: the runner supervises job containers on the docker socket, and the cache is not a widening reason.

This is also the honest sequel to #133: the journal error was the only signal and status reported healthy — the !134 note now names the symptom class, this removes one instance of it.

Not merging, not closing — @andres's.

## Verdict: approve (head `a45d848`) Driven at this head: | check | result | |---|---| | `bash test/cli.sh` | **789 / 789** (the three new ones included) | | CI's exact `shellcheck -x` step (34 files, uncovered guard) | **clean** | | `USER_HOME` ordering | assigned at `:255`, long before the `:404` use | | Converge behavior | the `install -d` sits before the already-registered early-out, so existing runners pick the fix up on a re-run — matches the "converged it" evidence in the body | | must-not guard 1 (ReadWritePaths) | **bites** — removed `.cache` from the unit line, suite went 788+1FAIL on exactly that check; restored | | must-not guard 2 (install creates the dir) | **bites** — deleted the `install -d` line, suite went 788+1FAIL on exactly that check; restored | The substantive claim — a `ReadWritePaths` entry naming a nonexistent path kills the unit at namespace setup — is documented systemd behavior, and the author measured it live on a real runner before shipping both halves together; the comment above the `install -d` records exactly that, so the next reader doesn't "simplify" it back to a unit-only edit. `ProtectHome=read-only` correctly stays: the runner supervises job containers on the docker socket, and the cache is not a widening reason. This is also the honest sequel to #133: the journal error was the only signal and `status` reported healthy — the !134 note now names the symptom class, this removes one instance of it. Not merging, not closing — @andres's.

claude-lead-andresmgsl approved these changes 2026-07-31 18:43:44 +00:00

claude-lead-andresmgsl left a comment

Member

Review of !138 @ `a45d848` — approve

Checked out build/135-runner-cache-dir, read against #135, and drove:

gate	result
`bash test/cli.sh`	789 / 789 (three new cache checks included)
`shellcheck -x commands/forgejo-runner-install.sh`	clean
`USER_HOME` ordering	assigned at `:255`, long before the `:407` `install -d` and `:472` unit
Converge	`install -d` for `.cache` sits before the already-registered early-out, so a re-run picks the fix up

Why both halves must ship together

Author measured live: ProtectHome=read-only + ReadWritePaths only on RUNNER_DIR → cache server cannot mkdir $HOME/.cache → silent disable. Punching .cache into ReadWritePaths without creating it first makes systemd refuse the unit (Failed to set up mount namespacing) — worse than a disabled cache. The comment above the install -d records that so nobody "simplifies" to a unit-only edit.

Shipped:

install -d … -o "$RUNNER_USER" … "$USER_HOME/.cache" (runner-owned, not root)
ReadWritePaths=${RUNNER_DIR} ${USER_HOME}/.cache
ProtectHome=read-only stays — cache is not a reason to widen home write

The three suite checks pin both halves and the non-widening of ProtectHome. Honest sequel to #133 / !134: status looked healthy while the cache work was dead; this removes one instance of that class.

I will not merge or close. @andres — yours.

## Review of !138 @ `a45d848` — **approve** Checked out `build/135-runner-cache-dir`, read against #135, and drove: | gate | result | |---|---| | `bash test/cli.sh` | **789 / 789** (three new cache checks included) | | `shellcheck -x commands/forgejo-runner-install.sh` | **clean** | | `USER_HOME` ordering | assigned at `:255`, long before the `:407` `install -d` and `:472` unit | | Converge | `install -d` for `.cache` sits **before** the already-registered early-out, so a re-run picks the fix up | ### Why both halves must ship together Author measured live: `ProtectHome=read-only` + `ReadWritePaths` only on `RUNNER_DIR` → cache server cannot mkdir `$HOME/.cache` → silent disable. Punching `.cache` into `ReadWritePaths` **without** creating it first makes systemd refuse the unit (`Failed to set up mount namespacing`) — worse than a disabled cache. The comment above the `install -d` records that so nobody "simplifies" to a unit-only edit. Shipped: - `install -d … -o "$RUNNER_USER" … "$USER_HOME/.cache"` (runner-owned, not root) - `ReadWritePaths=${RUNNER_DIR} ${USER_HOME}/.cache` - `ProtectHome=read-only` **stays** — cache is not a reason to widen home write The three suite checks pin both halves and the non-widening of ProtectHome. Honest sequel to #133 / !134: status looked healthy while the cache work was dead; this removes one instance of that class. I will not merge or close. @andres — yours.

claude-lead-andresmgsl referenced this pull request

2026-07-31 18:43:45 +00:00

feat(drill): a forgejo-runner lifecycle leg beside the GitHub one #130

codex-bot-andresmgsl approved these changes 2026-07-31 19:46:00 +00:00

codex-bot-andresmgsl left a comment

Member

Verdict: approve (head `a45d8489`)

Reviewed against #135 and independently verified this exact head.

gate	result
`bash test/cli.sh`	789/789
CI's exact 34-file `shellcheck -x` gate	clean
`git diff --check`	clean

The repair has both required halves and preserves the security boundary:

$USER_HOME/.cache is created with runner ownership;
creation occurs before the already-registered early return, so existing installs converge on re-run;
the unit punches through exactly RUNNER_DIR and $USER_HOME/.cache;
ProtectHome=read-only remains unchanged;
tests separately pin directory creation, ReadWritePaths, and the non-widening hardening;
changelog.d/135.md matches repository convention.

The placement avoids the measured systemd namespace failure when ReadWritePaths names a nonexistent directory, while keeping the writable surface narrow. No blocking findings.

I will not merge or close.

## Verdict: approve (head `a45d8489`) Reviewed against #135 and independently verified this exact head. | gate | result | |---|---| | `bash test/cli.sh` | 789/789 | | CI's exact 34-file `shellcheck -x` gate | clean | | `git diff --check` | clean | The repair has both required halves and preserves the security boundary: - `$USER_HOME/.cache` is created with runner ownership; - creation occurs before the already-registered early return, so existing installs converge on re-run; - the unit punches through exactly `RUNNER_DIR` and `$USER_HOME/.cache`; - `ProtectHome=read-only` remains unchanged; - tests separately pin directory creation, `ReadWritePaths`, and the non-widening hardening; - `changelog.d/135.md` matches repository convention. The placement avoids the measured systemd namespace failure when `ReadWritePaths` names a nonexistent directory, while keeping the writable surface narrow. No blocking findings. I will not merge or close.

claude-bot-andresmgsl added

state:needs-human

and removed

state:bots-reviewing

labels 2026-07-31 20:15:40 +00:00

claude-bot-andresmgsl commented

2026-07-31 20:15:53 +00:00

Author

Member

Handoff — full panel green at `a45d848`

@andres — !138 is 3/3 approved at head with zero REQUEST_CHANGES: @kimi-reviewer-andresmgsl (1486), @grok-reviewer-andresmgsl (1488), @codex-reviewer-andresmgsl (1492). Label moved to state:needs-human.

Ready for your merge decision. Closes #135. I will not merge or close.

## Handoff — full panel green at `a45d848` @andres — [!138](https://forgejo.heavyduty.builders/heavy-duty/rig/pulls/138) is 3/3 approved at head with zero `REQUEST_CHANGES`: @kimi-reviewer-andresmgsl (1486), @grok-reviewer-andresmgsl (1488), @codex-reviewer-andresmgsl (1492). Label moved to `state:needs-human`. Ready for your merge decision. `Closes #135`. I will not merge or close.