Reclaim our own offline registration instead of crashlooping on it

[leonarduk]

Closes #153

The bug

A container killed rather than stopped comes back under the same runner name and collides with the registration its previous incarnation left on GitHub, then crashloops. Six containers of the issue-worm-pro pool were doing exactly this after the host hit its commit limit:

A runner exists with the same name steves-big-laptop-d96452eb2dd3-1.

The name was assumed unique per process. It is not:

• entrypoint.sh is PID 1 (exec-form ENTRYPOINT, no init), so $$ is always 1 — every runner name on every host ends in -1.
• $(hostname) is the container's Config.Hostname, which a restart preserves.

Only the comment claimed otherwise.

The existing stale-cleanup block cannot cover this. Its GitHub-side removal is nested inside the remove-token call, so when minting that token fails — which is what a starved host does — nothing is removed there, yet .runner is deleted locally anyway. config.sh then registers under a name GitHub already knows.

The fix

reclaim_orphaned_name() deletes a same-named registration only when GitHub reports it offline.

Deliberately not --replace, preserving the stance the RUNNER_NAME comment already took: silently replacing a live runner is worse than refusing to start. An offline runner under our own exact name is by definition our own orphan. An online one is left alone and we still refuse, exactly as before.

Also adds an api_method helper for the GET/DELETE this needs, kept separate from api() so no existing token-minting POST can acquire a caller-supplied verb, and corrects the two comments that claimed names are regenerated per process.

Verification

Tested against the real API by mounting the entrypoint over the existing image, so no rebuild was involved:

Scenario	Result
Offline orphan under our name	Reclaimed (id 114), runner came up and listened
Unpatched entrypoint, same scenario	A runner exists with the same name, exit 1 — the control
Online same-named runner	Refused, live runner left untouched and still online

Plus bash -n, shellcheck clean across all six shell scripts (same koalaman/shellcheck image CI uses), and tests/pools_test.sh at 107 checks passing.

The negative control earned its keep: the first draft had a URL bug (.../runners/?per_page=100, a stray slash) that made the listing 404 silently, so the reclaim never ran and the container recovered by luck. A failed listing is treated as "nothing to reclaim", which is why that failure mode is now called out in the comment.

Known limitation

GitHub keeps a killed runner online for roughly a minute, and reclaim deliberately will not touch an online runner. A container restarting inside that window still collides once — but restart: always retries and recovers as soon as the status flips, rather than looping forever. Trading a bounded retry for never replacing a live runner seemed the right way round; happy to revisit if you would rather it waited for the status to settle.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f98c7c7a-14d4-4cf4-a975-b01cd8b8a6ef

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

GPT AI Code Review

Skipped: API key not configured

The OPENAI_API_KEY repository secret is not set.

To enable GPT reviews, add OPENAI_API_KEY as a repository secret:

1. Go to Settings > Secrets and variables > Actions in this repo.
2. Click New repository secret.
3. Name: OPENAI_API_KEY
4. Value: your GPT API key.

Once the secret is configured, re-run this workflow or push a new commit to trigger a fresh review.

REVIEW SKIPPED - API KEY NOT CONFIGURED

---

Reviewed by GPT via gpt-pr-review.yml. Advisory only.

[github-actions]

DeepSeek AI Code Review

1. Acceptance criteria

The core AC — reclaim our own offline registration instead of crashlooping — is met. The new reclaim_orphaned_name() looks up the runner by exact name, checks status == "offline", and deletes only in that case. A live same-named runner still refuses to start, preserving the deliberate no---replace stance. The stale comment at the old line 103 is corrected, and the new comment block above RUNNER_NAME documents the PID-1 / Config.Hostname reality.

One gap: the issue's "Why the stale-cleanup path does not save it" section describes a second, independent bug — the rm -f .runner running unconditionally even when api remove-token fails, so the local state is cleared while the GitHub registration is not. This PR adds a recovery path for that state but does not fix the cause. That's arguably fine (the reclaim makes the leftover harmless), but the diff doesn't say so, and a reader could reasonably expect the nesting to be corrected too. Worth a one-line note in the PR description or a follow-up.

2. Bugs and logic errors

• jq is now a hard runtime dependency of entrypoint.sh and is not verified to exist. The script previously used only curl. If jq is absent, id="$(jq ...)" fails, id is empty, [[ -n "${id}" ]] || return 0 returns success, and the function silently does nothing — the crashloop returns with no diagnostic. Either add a command -v jq guard that logs and returns, or parse with grep/sed. At minimum, the failure should be visible.

• head -n1 on the jq output is a no-op given the select, but the two separate jq invocations are a latent inconsistency. id and status are extracted by two independent jq runs over the same listing. If the listing somehow contains two entries with the same name (it shouldn't, but the API has been known to lag), id and status could come from different entries. Extract both in one pass: jq -r --arg n "$RUNNER_NAME" '.runners[]? | select(.name==$n) | "\(.id) \(.status)"'.

• api_method DELETE "/${id}" discards the response body and only checks exit status. curl -fsS returns non-zero on 4xx/5xx, so a failed delete is caught — good. But a 404 (already gone) is treated as failure and logged as "could not delete", which is misleading noise. Minor.

• The 2>/dev/null on the listing call swallows the reason for failure. The subsequent log says "could not list runners" but not why (auth failure vs. rate limit vs. network). Given the issue's own root cause was a rate-limited/starved host, this is the exact diagnostic you'd want. Consider capturing stderr.

3. CLI and API safety

• Token scope: the new GET/DELETE on /actions/runners requires the same repo (classic) or Administration: read/write (fine-grained) scope as remove-token. The comment above api() documents the scope for token minting; the new api_method() comment does not restate it. Not blocking, but the scope requirement for DELETE is stricter than for GET, and a PAT that can mint a registration token may not be able to delete a runner. Worth confirming in the PR description that the documented PAT scope covers DELETE.

• Rate limiting: api_method uses --retry 3 --retry-delay 3 but does not honor Retry-After or X-RateLimit-Reset. On a starved host (the exact scenario in the issue), three retries at 3s will not clear a rate-limit window. The function degrades gracefully (returns 0, logs, continues), so this is not a correctness bug — but it means the reclaim will silently no-op under the conditions that caused the original failure. Consider --retry-all-errors and a longer backoff, or at least note the limitation.

• No CLI surface change. entrypoint.sh is not the Python CLI; no Click/argparse signatures touched, no pyproject.toml change needed. Correct.

• Injection: RUNNER_NAME is interpolated into jq --arg (safe) and into the URL only via id (numeric from the API). No shell injection vector introduced.

4. Test coverage

No tests are added. The reproduce steps in the issue (hard-kill, restart, observe crashloop) are the natural validation, but they are manual and require a live GitHub repo. Given this is a shell entrypoint with no existing test harness, that's understandable — but the two branches that matter (offline → delete, online → refuse) are exactly the kind of thing a bats test or a mocked-curl shell test would pin down. At minimum, the PR should state that the reproduce steps were run and the crashloop is gone.

5. Suggested follow-up issues

• Fix the unconditional rm -f .runner in the stale-cleanup block so local and remote state stay in sync when remove-token fails
• Add a command -v jq guard (or drop the jq dependency) in entrypoint.sh
• Extract id and status in a single jq pass to avoid cross-entry inconsistency
• Honor Retry-After / X-RateLimit-Reset in api_method so reclaim survives a rate-limited host
• Add a bats (or mocked-curl) test covering the offline-reclaim and online-refuse branches

APPROVE — the fix is correct and scoped to the AC; the jq-missing silent no-op and the swallowed stderr are worth addressing but do not block, since the failure mode degrades to today's behaviour rather than making it worse.

---

Reviewed by DeepSeek via deepseek-pr-review.yml. Advisory only.