Lock takeover, five robustness fixes, and the game team as extensibility proof - #28
Merged
Merged
Conversation
…generalises Implements audit finding F6, which said "seventeen specialists" was padded by four personas the positioning does not serve, while the interesting story went untold: the persona framework generalises to any domain, and a WW2 tabletop studio inside a software tool is the proof. The count stays at seventeen. The four stop being a bonus roster and become the argument, which also answers finding F7: there was no response to "why not wire up the two or three specialists I care about myself, in an afternoon?" The honest answer is that you could, because the mechanism was never the hard part. What does not paste in from a system prompt is an interlocking team. That argument is shown rather than asserted, from three checkable facts about the profiles. Each persona is defined as much by the lane it will not cross as the one it owns. The Handoff Briefs name each other in two closed loops, Reiner to Piper and back, Cornelius and Ernie passing a line over what the record supports. And all four were assembled from the same profile structure and the same generator as the other thirteen, for a domain the tool was never built for. The section is written self-contained, with no references to surrounding material, so it survives the planned move of the dossiers to TEAM.md as a unit. gtm.md carried a stale roster throughout: "Ten of them", "one of ten", and "10 named specialist personas" in the flagship post, plus a "call X for Y" list missing Sage, Kai, Iris, and the studio entirely. All corrected to seventeen, with the studio as its own mini-table in the highest-traffic asset and a separate content-calendar thread rather than being forced into the existing series. Deliberately not claimed: that a game shipped. Nothing in the repo supports it, and the verifiable asset is the team's structure, not a product. Also left alone: gtm.md predates the no-emdash house style and is full of them, which is a separate cleanup rather than part of this finding. 223 tests unchanged; nothing here is under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019j5DHEZsoeCGRueTbNLuTb
Six findings from the systems audit. The headline one did not go the way I specified, and the correction matters more than the fix. I accepted the recommendation to make lock breaking single-winner by renaming the stale lock directory aside instead of removing it. Measured, that changes nothing: 5 lost updates in 30 rounds against 6 unfixed. Renaming makes the removal single-winner, but the staleness verdict still gets applied to whatever occupies the path afterwards. An instrumented run caught the real sequence: A renames the stale lock aside, B takes the freed path with a fresh mkdir, C renames away B's live lock, and three processes hold the lock at once. Any design that empties the path frees it for a moment, and a mkdir can land in that moment. So a contender no longer removes the lock directory at all. It takes the lock over in place: it renames the owner record aside, which exactly one contender can do, verifies what it actually claimed, then writes its own record. Only the owner ever removes the directory. Measured 0 lost updates in 40 rounds under the same 12-writer storm with CPU pressure. The directory-rename claim survives only for the ownerless deadline path, where there is no record to take over. Liveness now outranks the age rule. kill -0 decides first, and the 120 second age test is the fallback for cases where liveness cannot be established: a foreign host or a non-numeric PID. A reboot can leave a lock whose PID has been reused by a live process, and that lock now wedges until the timeout rather than being broken. The timeout already prints the rm -rf command, so that is loud and recoverable, which beats silently discarding a completed write. tmp_beside now returns through _TMP_FILE rather than stdout, because every caller wrapped it in a command substitution and the subshell stranded the assignment, so the EXIT trap could never remove a temp file. A SIGINT mid-rewrite left a full partial copy beside CLAUDE.md. tmp_commit clears the registration only after the rename succeeds. A titleless .md in the profiles directory no longer kills the pipeline under set -euo pipefail. list exited 1 printing nothing with empty stderr, and help exited 0 with a silently empty roster. Both now skip the file and name it. install.sh survives stdin at end of file. A bare read returned non-zero and ended the install mid-prompt, so a piped or CI run exited 1 with no summary after every earlier step had already applied. End of file now skips the coordinator and says why, because nobody is present to consent to writing a global file. Enter still means casual. cmd_sync explains a failed copy, naming the three surfaces and that rerunning heals it. My diagnosis here was wrong: set -e already stopped the command, so the defect was that it was unexplained rather than silent. Three things found on the way. lock_acquire could hang forever, because the stale branch skipped the deadline check, so a lock that could never be claimed spun with no timeout and no message; latent before, reachable after, now dies loudly. A single local declaration referenced its own earlier variable, which is not in scope yet. And assert_count could never pass with an expected 0, because grep -c prints 0 and also exits 1, so the || fallback produced two lines and bash raised a syntax error instead of comparing. 268 tests, up from 223. Seventeen fail against main, one per finding, including one that names the eight personas help silently dropped. Suite runtime 9.4s to 15.5s, which is a real cost and is flagged rather than absorbed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019j5DHEZsoeCGRueTbNLuTb
The macOS job failed on "the interrupted write did not land". Linux and shellcheck passed. The assertion was testing timing rather than correctness. Its guard waits for the temp file to appear and then sends SIGINT, and a comment claims that proves the interrupt arrived before the rename. It does not: the rename can land in the microseconds between the temp file becoming visible and the signal being delivered. On macOS, where process spawn is slower and the window sits differently, the write completed first and the assertion failed on a build where nothing was wrong. Whether the rename beats the signal is not a property the design promises. What it does promise is that a reader sees the whole old file or the whole new one, never a partial write, because the temp file and its target share a filesystem and the commit is a rename. So the test now asserts coherence: the marker count is zero or one, with START and END balanced, in either outcome. It also asserts all 2000 user lines survive, since preserving the user's own content is the point of writing through a temp file at all. Verified against both outcomes directly: signal first gives 0 start and 0 end, rename first gives 1 and 1, and all 2000 lines and zero orphans in both. The orphan check is untouched and still fails against main, finding the temp file that the unregistered _TMP_FILE left behind. That is the bug this block exists for. The two replaced assertions now pass against main as well, which is correct: main never wrote a partial file, it only failed to clean up after itself. 269 tests. shellcheck clean locally, and CI is the authority on that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019j5DHEZsoeCGRueTbNLuTb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Akira's remaining systems findings plus Toni's positioning work, from the three-way audit that graded B− systems / B− tests / C+ product.
268 tests (was 223). 17 fail against
main, one per finding.The fix I specified was wrong, and Akira measured it
I accepted his recommendation to make lock breaking single-winner by renaming the stale lock directory aside instead of removing it. Measured, it changes nothing: 5 lost updates in 30 rounds against 6 unfixed.
Renaming makes the removal single-winner, but the staleness verdict still gets applied to whatever occupies the path afterwards. An instrumented run caught the real sequence:
Any design that empties the path frees it for a moment, and a
mkdircan land in that moment.So a contender never removes the lock directory at all. It takes the lock over in place: renames the owner record aside (exactly one contender can), verifies what it actually claimed, then writes its own record. Only the owner ever removes the directory.
rm -rf "$lock_dir")He flagged it rather than shipping a change that measured the same as before. That's the second time today a persona overruled my technical direction and was right.
The other five
Liveness outranks the age rule.
kill -0decides first; the 120s age test is the fallback where liveness can't be established. Tradeoff written into the comment: a post-reboot lock with a reused PID now wedges until the timeout rather than being broken. The timeout already prints therm -rf, so that's loud and recoverable, which beats silently discarding a completed write._TMP_FILEcleanup made real. Every caller wrappedtmp_besidein a command substitution, so the subshell stranded the assignment and the EXIT trap could never fire. A SIGINT mid-rewrite left a full partial copy besideCLAUDE.md.A titleless
.mdno longer killslist. It exited 1 printing nothing with empty stderr;helpexited 0 with a silently truncated roster. The control test names the eight personashelpwas dropping.install.shsurvives stdin at EOF. A barereadended the install mid-prompt, so any piped or CI run exited 1 with no summary after every earlier step had applied. EOF now skips the coordinator and says why; Enter still means casual.cmd_syncexplains a failed copy. My diagnosis was wrong here:set -ealready stopped it, so the defect was unexplained, not silent.Three bugs found on the way
lock_acquirecould hang forever — the stale branch skipped the deadline check, so a lock that could never be claimed spun with no timeout and no message. Latent before, reachable after his change, now dies loudly.assert_countcould never pass with an expected 0 —grep -cprints0and exits 1, so the|| echo 0fallback produced two lines and bash raised a syntax error instead of comparing.localdeclaration referencing its own earlier variable, which isn't in scope yet.Toni: the game team as extensibility proof
Reiner, Cornelius, Ernie, and Piper are now packaged as a mini team that answers "why not wire up two or three personas myself in an afternoon?" The honest answer is that you could, because the mechanism was never the hard part.
Shown from three checkable facts, not asserted: each persona is defined as much by the lane it won't cross as the one it owns; the Handoff Briefs name each other in two closed loops (Reiner↔Piper, Cornelius↔Ernie, verified); and all four came from the same profile structure and generator as the other thirteen, for a domain the tool was never built for.
gtm.mdalso carried a stale roster throughout — "Ten of them", "one of ten", "10 named specialist personas" in the flagship post, and a "call X for Y" list missing Sage, Kai, Iris, and the studio. All at seventeen now.Deliberately not claimed: that a game shipped. Nothing supports it.
Honest limits
install.shruns, the 12-writer storm, and a 1s liveness observation window.mkdirand its owner write still wedges that file for 15s per writer. Deliberate, but nothing tests the message the user actually sees.Generated by Claude Code