amadan
Fix what the review found, including advice that would lose work
The Fable whole-branch review of this doc branch found two blocking problems. One of them is the reason a doc-only branch still gets the review. **The recovery advice was destructive.** I wrote that a failed `checkout -b` is recovered with `git reset --hard <sha>`. The failure carries any UNCOMMITTED edits into the new tree, so `reset --hard` there destroys them — advice that loses work, in an entry whose whole purpose is stopping someone panicking at a tree that looks emptied. The right recovery is `git checkout <new>`: the failed command already created the branch ref, and the tree already matches it, so this finishes the switch minus the tracking you cannot have. Reproduced here before rewriting it — with an untracked file present, `checkout -b recovery-probe origin/main` failed with HEAD still on this branch, the ref created at 1b6d392, and the file intact; `git checkout recovery-probe` then switched cleanly and kept it. `reset --hard` is now labelled as the option for when you have nothing uncommitted, and the entry says the ref exists so a retry of `-b` is refused. **`config.lock` was not the mechanism.** `/proc/mounts` carries FOUR read-only bind mounts under `.git`: `hooks`, `config`, `config.lock` and `config.worktree`. `config` itself being read-only is why config is unwritable; `config.lock` is merely the first barrier git hits, which is why the error says "File exists". My causal sentence was wrong and the bullet's opening line hung on it. Confirmed here with awk over /proc/mounts rather than taken from the review. Three overstatements corrected in the same pass: - Not "every git operation that writes config". `branch.autoSetupMerge` is unset here, so `checkout -b <new> <local-branch>` is fine; the trap needs a REMOTE-TRACKING base. That happens to be the base this file prescribes, so the advice stands, but the scope was wrong. - `.gitignore`'s placeholders are character devices, not bind mounts, and the credential-store error is the sandbox write allowlist rather than a mount at all. "Same mechanism" was wrong twice; they are cousins. - `browser-drive.sh` drives what is ON DISK, not what is committed — `//go:embed` takes it at build time, so an uncommitted rebuild would be tested. And a failed gate discards an UNTRACKED rebuild; a staged one survives `git clean` and merely still fails. "Deletes the bundle you just built" was half right. Also from the review: the pipe entry now says `| tail` hides a status only when pipefail is unset, and carries the measurement behind the SIGPIPE claim — a sha near the start of a 20MB binary returned 141 ten times out of ten, the same sha at the end returned 0 every time. A check whose answer depends on byte offset is worse than one that never works, which is the sharper point and was missing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WxucgadpWLE9XNWvfHBsnb
1 file changed,
+72
−45
AGENTS.md+72 −45
CI passed — run details
diff --git a/AGENTS.md b/AGENTS.md| index dee9ca2..a5431e9 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -349,63 +349,90 @@ time in a single afternoon: |
| on the formatting-and-export branch, by a browser drive, after an |
| evening of looking at the wrong file. |
| -- **`git checkout -b` in this repo fails *after* rewriting your working |
| - tree, and `.git/config.lock` is why.** That file is not a stale lock |
| - left by a crashed process — it is a permanent read-only bind mount the |
| - harness materialises (`/proc/mounts` shows it as `ext4 ro,...`; `rm` |
| - answers "Device or resource busy"), the same mechanism `.gitignore` |
| - already documents for `.gitconfig` and `.bashrc`. It makes git *config* |
| - unwritable while the rest of `.git` stays writable. **No session can |
| - clear it and none should try.** |
| - So every git operation that writes config fails, permanently, and the |
| - dangerous one is `git checkout -b <new> <base>`: it updates the index |
| - and the working tree, *then* fails on the config write for upstream |
| - tracking. HEAD stays on the old branch while the tree holds the new |
| - base's content, so a branch you have been working on appears to have |
| - lost every change, as deletions. Nothing is actually gone if it is |
| - committed — `git reset --hard <sha>` puts it back — but the first |
| - reading is that the tool ate your work. |
| +- **`git checkout -b <new> origin/<x>` fails *after* rewriting your |
| + working tree.** The harness mounts `.git/config` **and** `.git/config.lock` |
| + read-only (`/proc/mounts`, `ext4 ro,...`; `rm` answers "Device or |
| + resource busy"), so git config is permanently unwritable while the rest |
| + of `.git` stays writable. `config.lock` is only the first barrier git |
| + hits — hence "File exists" — and `config` being read-only is the actual |
| + reason. **No session can clear either and none should try.** It is a |
| + cousin of the placeholders `.gitignore` documents for `.gitconfig` and |
| + `.bashrc`, though those are character devices rather than bind mounts. |
| + |
| + The trap fires when the base is a **remote-tracking** ref, because that |
| + is when git tries to write an upstream. `branch.autoSetupMerge` is unset |
| + here, so `checkout -b <new> <local-branch>` is fine; `origin/main` — the |
| + base this file tells you to use — is not. It updates the index and the |
| + working tree, *then* fails on the config write. HEAD stays on the old |
| + branch while the tree holds the new base's content, so a branch you have |
| + been working on appears to have lost every change, as deletions. |
| + |
| + **Recovery: finish the switch with `git checkout <new>`.** The failed |
| + command *did* create the branch ref, and the tree already matches it, so |
| + this completes what you asked for minus the tracking you cannot have — |
| + and it keeps any uncommitted edits, which the failure carried across. |
| + Reach for `git reset --hard <sha>` only when you have nothing |
| + uncommitted, because there it is the destructive option. Note the ref |
| + now exists, so retrying `-b` with the same name is refused. |
| + |
| **Always `git checkout -b <new> --no-track <base>`, and never |
| - `git push -u`** (the push succeeds and then reports "unable to write |
| - upstream branch configuration"; plain `git push origin <branch>` is |
| - the shape). `--no-track` is not a workaround here, it is the permanent |
| - spelling. The "unable to get credential storage lock … Read-only file |
| - system" on fetch and push is the same family and is not worth chasing. |
| - *(Found 2026-09-03 while landing the triage branch; diagnosed as a |
| - bind mount by the paste-fidelity session after I had written it down |
| - as debris someone should clear, which was wrong.)* |
| -- **A pipeline hides the exit code of everything but its last command.** |
| - `bash .amadan/ci.d/10-gate | tail -40` reports `tail`'s status, which |
| - is always 0, so a real failure scrolls past inside output you have just |
| + `git push -u`** (the push itself succeeds and then reports "unable to |
| + write upstream branch configuration"; plain `git push origin <branch>` |
| + is the shape). `--no-track` is not a workaround here, it is the |
| + permanent spelling. The "unable to get credential storage lock … |
| + Read-only file system" on fetch and push is a different mechanism — the |
| + sandbox's write allowlist, not a mount — and is equally not worth |
| + chasing. |
| + *(Found 2026-09-03 landing the triage branch, after I first wrote it |
| + down as stale debris someone should clear. The paste-fidelity session |
| + caught that; a branch review then caught that `config.lock` was not the |
| + mechanism either, and that the recovery advice above originally said |
| + `reset --hard`, which would have destroyed uncommitted work.)* |
| + |
| +- **A pipeline hides the exit code of everything but its last command**, |
| + unless `pipefail` is set — and an interactive shell here does not set |
| + it. `bash .amadan/ci.d/10-gate | tail -40` reports `tail`'s status, |
| + which is 0, so a real failure scrolls past inside output you have just |
| read while the harness says success. Same for `amadan ci status … | |
| head`, which read as exit 0 while the command had actually failed with |
| "not an amadan remote". Run the thing, capture its status, then read |
| the log: `bash .amadan/ci.d/10-gate > gate.log 2>&1; echo "EXIT=$?"`. |
| - The `set -o pipefail` variant of this bites in the opposite direction: |
| + |
| + Under `set -o pipefail` it bites in the opposite direction: |
| `strings "$bin" | grep -q "$sha"` exits early on a match, SIGPIPEs |
| - `strings`, and pipefail then calls the pipeline failed — so a release |
| - binary that *was* correctly version-stamped read as unstamped. Count |
| - into a variable instead of using `grep -q`. |
| + `strings`, and pipefail then calls the pipeline failed (141) — so a |
| + release binary that *was* correctly version-stamped read as unstamped. |
| + It depends on where the match falls: measured, a sha near the start of |
| + a 20MB binary returned 141 on ten runs out of ten, while the same sha |
| + at the very end returned 0 every time. A check that works or not |
| + according to byte offset is worse than one that never works. Count into |
| + a variable instead of using `grep -q`. |
| - **A green run against a stale artifact**, which is the same failure as |
| the exit codes above and the worse half of it: a wrong exit code you |
| re-check in a second, a stale artifact you believe for hours. |
| - `scripts/browser-drive.sh` does not rebuild the bundles — it drives |
| - whatever is committed in `internal/docs/static/`. So a deliberate break |
| - of `fixtures/jump-to.mjs`, made to prove a new check could fail, came |
| - back **44/44 green**: `fixtures/node_modules` was missing, |
| - `node fixtures/build-bundles.mjs` failed, and the drive tested the old |
| - committed bundle. Nothing lied about a status; the verification simply |
| - ran against something it had never examined and had no way to say so. |
| - The gate's own bundle step is the other instance, and it is deliberate: |
| - on a mismatch it runs `git checkout --` and `git clean -fd` over |
| + `scripts/browser-drive.sh` does not rebuild the bundles — it builds the |
| + server, which `//go:embed`s whatever is sitting in |
| + `internal/docs/static/` at that moment. So a deliberate break of |
| + `fixtures/jump-to.mjs`, made to prove a new check could fail, came back |
| + **44/44 green**: `fixtures/node_modules` was missing, |
| + `node fixtures/build-bundles.mjs` failed, and the drive embedded and |
| + tested the *old* bundle still on disk. Nothing lied about a status; the |
| + verification ran against something it had never examined and had no way |
| + to say so. |
| + |
| + The gate's bundle step is the other instance, and it is deliberate: on a |
| + mismatch it runs `git checkout --` and `git clean -fd` over |
| `internal/docs/static/` (10-gate, and read its comment — restoring is |
| - what stops a *second* run passing on a rebuild nobody committed). The |
| - consequence for you is that a failed gate deletes the bundle you just |
| - built, so **rebuild and commit in one step**; `git add` is not enough, |
| - because staged-but-uncommitted still reads as drift. |
| + what stops a *second* run passing on a rebuild nobody committed). So a |
| + failed gate discards an **untracked** rebuild; a staged one survives |
| + `clean` but still fails, because any porcelain output there reads as |
| + drift. Either way **rebuild and commit in one step** — `git add` is not |
| + enough. |
| + |
| Before trusting a green run, make it fail on purpose — and check that |
| the artifact under test was actually rebuilt before you believe either |
| - answer. |
| + answer, because a break that never reached the thing you drove proves |
| + nothing in either direction. |
| The shape to watch for is a verification that cannot distinguish "no" from |
| "I can't see it". When you catch one, write it here rather than resolving |