From aac8af5e1ffd2ac98bfd274ba2958825fb5e2dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 21 Aug 2026 20:02:24 +0000 Subject: [PATCH] one attempt per release, and the rule is code rather than a paragraph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first watcher had an unbounded retry in it. A release whose deploy failed left no .done file, so the next poll two minutes later spawned a fresh headless agent, and the one after that, all night, each one costing money. The loop was careful and its logging was honest; the defect was structural. It is structural because the loop is written FRESH by an agent every time one arms. A rule that lives only in the prompt gets re-implemented from scratch each time, and the wrong implementation is the one that looks right: test the .done file. That file only exists on success, so testing it is exactly the retry storm. So scripts/deploy-claim.sh owns it. `claim ` exits 0 once and 1 for ever after, whatever became of the attempt that held it — a failed release is not retried, it is looked at. Handing the claim back is `rm -rf` by a person, on purpose; a loop cannot undo its own claim, which is what makes the storm impossible rather than unlikely. Three choices in it worth keeping: - `mkdir` is the claim. One atomic syscall that both tests and takes, with no window between checking and acting — two pollers a second apart is not hypothetical when a watcher gets restarted. - Bash, not bun. The thing that stops a retry storm must not need a healthy checkout; a deploy that dies on `bun install` is precisely when it has to work. - A version is digits and dots or it is refused. It arrives off a git tag and is concatenated into a path. A pre-existing .done with no claim beside it still counts as deployed, so deploy/2026.08.21.01 — rolled out by hand before any of this existed — is not re-attempted. docs/servers.md corrected from the commander's own report, which was right and specific: on macbookpro APPEND /opt/homebrew/bin for bun rather than replacing the PATH (this file said "do not help the PATH", which is right for pm2 and is what made plugins:update fail there), `ls` is aliased to an eza that is not installed, plugins:update bounces every sidecar before the plan's own restart step, and fofos cannot publish affordances for what is probably the same ownership problem as its capabilities sync. Co-Authored-By: Claude Opus 5 --- docs/servers.md | 18 +++++- prompts/deploy-commander.md | 54 +++++++++++++++++- scripts/deploy-claim.sh | 107 +++++++++++++++++++++++++++++++++++ scripts/deploy-claim.test.ts | 60 ++++++++++++++++++++ 4 files changed, 234 insertions(+), 5 deletions(-) create mode 100755 scripts/deploy-claim.sh create mode 100644 scripts/deploy-claim.test.ts diff --git a/docs/servers.md b/docs/servers.md index 68ea5ce4..9a7eb55a 100644 --- a/docs/servers.md +++ b/docs/servers.md @@ -60,13 +60,27 @@ All verified, none cosmetic. _login_ shell does not read. Use `ssh '$SHELL -ic "pm2 jlist"'`. `-lc` is **not** enough — it reports "command not found" on machines where pm2 is plainly running, which cost two wrong conclusions on 2026-08-21. -- **On `macbookpro`, do not help the PATH.** Exporting one to reach `bun` breaks `pm2`, which is only on - the interactive shell's. `$SHELL -ic` and nothing else. +- **On `macbookpro`, APPEND to the PATH for `bun`. Never replace it.** This said "do not help the PATH" + until 2026-08-21, and that was half right in a way that cost a deploy step: `pm2` is only on the + interactive shell's PATH, so replacing it breaks `pm2` — but `bun` is at `/opt/homebrew/bin/bun` and + `.zshrc` does not put that directory on the interactive PATH either, so `bun run plugins:update` fails + `command not found` before it runs a line. `export PATH=$PATH:/opt/homebrew/bin` for that one command + shadows nothing: `pm2` stays at `~/.local/bin/pm2` and `node` at `node@22`. Run `pm2` as a separate + command. +- **`$SHELL -ic` on `macbookpro` aliases `ls` to `eza`, which is not installed.** A probe using `ls` + reports a missing file that is plainly there. Use `command ls`, or a non-interactive shell for file tests. - **The git remotes differ.** `fofos` pulls over HTTPS; the others over SSH. Never assume a URL — pull in place. - **A "dirty tree" means TRACKED changes.** `git status --porcelain` counts untracked files, and a stray script a fast-forward could never touch is not a reason to skip a host. Use `--untracked-files=no`. - **`macbookpro` is a laptop.** It may be asleep. Unreachable is a skip with a note, not a failed deploy. +- **`bun run plugins:update` restarts sidecars itself**, printing `sidecar: started` per plugin — on + `alpha` that is twelve processes bouncing during the plugins step, before any restart the plan asked for. + Not a fault, but a deploy report that lists only `officer` under "restarted" is understating what moved. +- **`fofos` cannot sync its capabilities**, and could not publish gitea affordances on 2026-08-21: + `dubious ownership` from git and `EACCES … mkdir '…/capabilities/affordances/officerdev'`. Both point at + that capabilities checkout being owned by somebody other than `pastilhas`. Deliberately not papered over + with a `safe.directory` entry until somebody has run one `stat` on it. **Unexplained.** ## Not platforms — ignore for now diff --git a/prompts/deploy-commander.md b/prompts/deploy-commander.md index a836ab5c..410df00e 100644 --- a/prompts/deploy-commander.md +++ b/prompts/deploy-commander.md @@ -30,6 +30,37 @@ names its own version, and `git ls-remote` answers "is there one" in a single ca - **No untagged release** → say "no release pending" and stop. Do not pull, do not touch a server. - **A `deploy/*` tag with no local record** → that is the release. Continue. +### One attempt per release, and you do not enforce that yourself + +**Before you spawn or run anything for a version, claim it:** + +```bash +scripts/deploy-claim.sh claim # exit 0 = it is yours, exit 1 = do not proceed +``` + +Exit 1 means somebody already attempted this release. **Stop. Do not retry it, do not deploy it, and do +not decide the previous attempt does not count.** Say what the script said and move on. + +On success, record it through the same script: + +```bash +scripts/deploy-claim.sh done +``` + +**Why this is a script and not a rule you follow.** A watcher loop is written fresh each time one arms, so +a rule that lives only in this file gets re-implemented from scratch every time — and the wrong version of +it looks right: test for the `.done` file, which exists only on SUCCESS. Write that and a failing release +spawns a new agent every poll, all night, each one costing money. That is not hypothetical. It is what the +first watcher did on 2026-08-21: a careful loop, honest logging, and an unbounded retry inside it. + +So the rule is code. `mkdir` is the claim — atomic, one syscall that both tests and takes, no window +between checking and acting. The script refuses a second claim and keeps refusing, whatever happened to the +first attempt. + +**A failed release is therefore not retried. It is looked at.** Clearing a claim is a deliberate human act +(`rm -rf …/.claim`) or a new tag. A loop cannot undo its own claim, which is the property that +makes the storm impossible rather than unlikely. + ### The record is LOCAL, not in git ``` @@ -155,7 +186,24 @@ that are hard to reconstruct afterwards. --- -## What is missing, as of writing +## Being woken -**Nothing wakes you yet.** A watcher, a cron entry or a repeating prompt has to exist and does not. Until -then this runs when a person starts it. +A background job does it, armed from the composer's background-job dialog with **Exit and hand back to the +agent, then re-arm**. The wait happens in the shell, not in a model tick — a loop polling +`git ls-remote`, which is one network call and no tokens. + +The loop cannot do a deploy: it cannot read a plan, judge a host or write a report. So on firing it spawns +a headless agent — `claude -p "" --dangerously-skip-permissions +--output-format=stream-json --verbose` — and pipes the events through `jq` so the job panel shows progress +instead of a blank box for twenty minutes. + +Two things that loop must get right, both learned by getting them wrong on 2026-08-21: + +- **Claim the version first**, per the section above. This is the one that costs money when missed. +- **Set `OFFICER_ROOT` explicitly.** It is empty in the sidecar's environment, so the lock path resolves to + `/data/deploys` and the release looks permanently undeployed. + +The deploy therefore runs in a **separate** agent, not in the commander's own conversation: no Command HQ +row, no cost on the commander's line, and its reasoning lives in that child's transcript. That is the price +of putting the wait in the shell, and it is worth it — a model-side tick that re-reads a whole conversation +to learn nothing is the thing being avoided. diff --git a/scripts/deploy-claim.sh b/scripts/deploy-claim.sh new file mode 100755 index 00000000..af4f717d --- /dev/null +++ b/scripts/deploy-claim.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# +# One attempt per release, enforced by the filesystem. +# +# A watcher loop asks this before it spawns anything. Exit 0 means "you have the claim, go". Exit 1 means +# "somebody already had it" — and that answer never changes back, whatever happened to the attempt that +# held it. A release that fails is not retried; it is looked at. +# +# WHY THIS IS A SCRIPT AND NOT A PARAGRAPH IN prompts/deploy-commander.md +# +# The watcher loop is written fresh by an agent each time it arms, so a rule that lives only in prose is +# re-implemented from scratch on every arming, and "skip a version already attempted" has a wrong version +# that looks right: test the .done file, which only exists on SUCCESS. Write that and a failing deploy +# spawns a fresh agent every poll, all night, each one costing money. That is exactly what happened on +# 2026-08-21 — the loop was careful, logged honestly, and still had an unbounded retry in it. +# +# WHY BASH AND NOT bun +# +# The thing that stops a retry storm must not depend on the checkout being healthy. A deploy that dies on +# `bun install` is precisely when this has to work. +# +# WHY mkdir +# +# It is atomic on every POSIX filesystem and it fails if the target exists — one syscall that both tests +# and claims. `[ -f x ] && ...` is two steps with a race between them, and two pollers a second apart is +# not a hypothetical when a watcher is restarted. +set -u + +DEPLOYS_DIR="${DEPLOYS_DIR:-$HOME/officer-dev/data/deploys}" + +usage() { + cat >&2 <<'USAGE' +usage: deploy-claim.sh claim take the one attempt for this release (exit 0 = yours, 1 = taken) + deploy-claim.sh done record that the attempt succeeded + deploy-claim.sh state print unclaimed | claimed | done + deploy-claim.sh list every release this host knows about + +To retry a release deliberately: rm -rf /.claim +That is a human act on purpose. A loop cannot undo its own claim. +USAGE + exit 2 +} + +version_ok() { + # The version reaches here from a git tag, so it is attacker-adjacent input used to build a path. + case "$1" in + *[!0-9.]* | '' | *..*) return 1 ;; + *) return 0 ;; + esac +} + +CMD="${1:-}" +[ -n "$CMD" ] || usage + +case "$CMD" in + list) + mkdir -p "$DEPLOYS_DIR" + found=0 + for d in "$DEPLOYS_DIR"/*.claim; do + [ -e "$d" ] || continue + found=1 + v=$(basename "$d" .claim) + if [ -f "$DEPLOYS_DIR/$v.done" ]; then s="done"; else s="claimed, no success recorded"; fi + printf '%-16s %s\n' "$v" "$s" + done + [ "$found" = 1 ] || echo "no releases attempted on this host" + ;; + + claim | done | state) + V="${2:-}" + [ -n "$V" ] || usage + version_ok "$V" || { echo "deploy-claim: refusing a version that is not digits and dots: $V" >&2; exit 2; } + mkdir -p "$DEPLOYS_DIR" || exit 2 + CLAIM="$DEPLOYS_DIR/$V.claim" + DONE="$DEPLOYS_DIR/$V.done" + + case "$CMD" in + state) + if [ -f "$DONE" ]; then echo done + elif [ -d "$CLAIM" ]; then echo claimed + else echo unclaimed; fi + ;; + claim) + # A pre-existing .done with no .claim is a release deployed before this script existed. Honour it. + if [ -f "$DONE" ]; then + echo "deploy-claim: $V is already deployed" >&2 + exit 1 + fi + if mkdir "$CLAIM" 2>/dev/null; then + printf '%s claimed on %s by pid %s\n' "$(date -Iseconds)" "$(hostname)" "$$" > "$CLAIM/by" + echo "deploy-claim: $V is yours" + exit 0 + fi + printf 'deploy-claim: %s was already attempted — %s\n' "$V" "$(cat "$CLAIM/by" 2>/dev/null || echo 'claim exists')" >&2 + echo "deploy-claim: NOT retrying. Look at what failed, then rm -rf $CLAIM to allow another attempt." >&2 + exit 1 + ;; + done) + [ -d "$CLAIM" ] || echo "deploy-claim: warning — recording success for $V that was never claimed" >&2 + printf '%s succeeded on %s\n' "$(date -Iseconds)" "$(hostname)" >> "$DONE" + echo "deploy-claim: $V recorded as deployed" + ;; + esac + ;; + + *) usage ;; +esac diff --git a/scripts/deploy-claim.test.ts b/scripts/deploy-claim.test.ts new file mode 100644 index 00000000..a8a5cfe4 --- /dev/null +++ b/scripts/deploy-claim.test.ts @@ -0,0 +1,60 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const SCRIPT = join(import.meta.dir, 'deploy-claim.sh'); +let dir: string; + +const run = (...args: string[]) => { + const p = Bun.spawnSync([SCRIPT, ...args], { env: { ...process.env, DEPLOYS_DIR: dir } }); + return { code: p.exitCode, out: p.stdout.toString(), err: p.stderr.toString() }; +}; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'deploy-claim-')); +}); +afterEach(() => rmSync(dir, { recursive: true, force: true })); + +// The whole point of this script is that the SECOND caller is refused. Everything else is scaffolding. +describe('deploy-claim', () => { + it('grants once and refuses after, whatever the outcome was', () => { + expect(run('claim', '2026.09.01.01').code).toBe(0); + expect(run('claim', '2026.09.01.01').code).toBe(1); + // Still refused after a failure — no .done was ever written. This is the retry storm not happening. + expect(run('claim', '2026.09.01.01').code).toBe(1); + expect(run('state', '2026.09.01.01').out.trim()).toBe('claimed'); + }); + + it('refuses a release already deployed, even with no claim beside it', () => { + // A release deployed by hand before this script existed. `deploy/2026.08.21.01` is exactly this case. + writeFileSync(join(dir, '2026.08.21.01.done'), 'deployed by hand\n'); + expect(run('claim', '2026.08.21.01').code).toBe(1); + expect(run('claim', '2026.08.21.01').err).toContain('already deployed'); + }); + + it('lets a human hand the claim back, and only a human', () => { + expect(run('claim', '2026.09.01.02').code).toBe(0); + rmSync(join(dir, '2026.09.01.02.claim'), { recursive: true }); + expect(run('claim', '2026.09.01.02').code).toBe(0); + }); + + it('records success separately from the claim', () => { + run('claim', '2026.09.01.03'); + run('done', '2026.09.01.03'); + expect(run('state', '2026.09.01.03').out.trim()).toBe('done'); + expect(existsSync(join(dir, '2026.09.01.03.done'))).toBe(true); + }); + + // The version comes off a git tag, and it is concatenated into a path. + it('refuses anything that is not digits and dots', () => { + for (const bad of ['../../etc/passwd', 'x; rm -rf /', '2026.08.21.01/..', '', 'v1']) { + expect(run('claim', bad).code).toBe(2); + } + expect(existsSync(join(dir, '..', 'etc'))).toBe(false); + }); + + it('says so plainly when nothing has been attempted', () => { + expect(run('list').out).toContain('no releases attempted'); + }); +});