@@ -0,0 +1,3364 @@
1+ # Book amendments
2+
3+ Thirteen contradictions were found reading the book against
4+ `plans/docs/BUILD.md`, `plans/build.py` and the 24 mockups. All are resolved.
5+ The book is the source of truth, so the book was changed; this file records
6+ what changed and why, and is not a second specification.
7+
8+ Every resolution took the side that costs the user least.
9+
10+ ## Resolved in the book
11+
12+ **1. The security chapter reference.** `README.md` sent readers to chapter 36,
13+ which is Proposals. Now chapter 42, which is the security chapter.
14+
15+ **2. `build.py` now generates every page.** Five pages were hand-written and
16+ outside the generator: `signup`, `profile`, `repo-log`, `repo-config`,
17+ `thread`. They are in `PAGES` now, so `python3 build.py` writes all 25 files
18+ from one shared chrome, which is what the README always claimed. Their drift
19+ went with them: the card height was wrong by 24px and the separator glyph
20+ differed.
21+
22+ **3. Deleting a repository.** 33.12 said "gone at once", 44.4 said a 30-day
23+ trash window. The window wins, because an instant irreversible delete produces
24+ a support request forge has no channel to answer. 33.12 now states the window,
25+ and states that it is a window to notice a mistake rather than a backup.
26+
27+ **4. One name for the push limit.** `[limits] max_push_size_mb = 512` and
28+ `[behavior] max_push_mb = 2048` were the same setting. Now `[limits]
29+ max_push_mb = 2048`, with a rule that removes the ambiguity for every future
30+ key: **numbers live in `[limits]`, switches live in `[behavior]`.**
31+ 2048 wins over 512 because the push most likely to hit this limit is somebody's
32+ first import of an existing repository, and rejecting that is the worst
33+ possible first contact.
34+
35+ **5. The pre-receive pseudocode.** Appendix D nested the notes-namespace check
36+ and the catch-all rejection inside the blob-size loop, so a push to
37+ `refs/notes/threads/*` was never authorised and the catch-all never ran on a
38+ push that added no blobs. The ref chain is now exhaustive and size is judged
39+ once, after the refs, over the objects the push actually adds.
40+
41+ **6. The reserved-name list.** 42.5 listed eleven names but missed `inbox`,
42+ `tokens` and `auth`, all of which are routes an account name could shadow. The
43+ list is now every routed name plus four held for later, and the book says to
44+ derive it from the route table in code rather than copy it.
45+
46+ **7. Raw file content has a route.** 42.3 required a separate domain but no
47+ route existed and two mockups linked to one. Added
48+ `GET /<user>/<repo>/raw/<ref>/<path>` and `[server] raw_url`. When `raw_url` is
49+ empty, raw is served from the main host as a download with the four hardening
50+ headers, and the handler reads no session cookie, so it answers for public
51+ repositories only. Most people install on one hostname; a link that only works
52+ for operators who own a second domain is a link most users never get.
53+
54+ **8. The server has a command.** Appendix E named the ssh and hook entry points
55+ but never the daemon. `forge serve` now runs the server, and the ssh entry point
56+ that only `authorized_keys` invokes is `forge ssh`. The command a person types
57+ got the obvious name.
58+
59+ **9. `repo-config.html` showed invalid TOML.** `uproar.local = [...]` unquoted
60+ is a dotted key, meaning table `uproar`, key `local`. Now quoted, matching
61+ chapter 14. It matters on the one page whose entire point is that the config is
62+ a file you edit by hand.
63+
64+ **10. The thread page shows the offline commands.** Chapters 13 and 24 both
65+ require them and the mockup had none. They are the proof of the portability
66+ claim, on the page making the claim.
67+
68+ **11. The keys page has feed tokens.** Chapters 19.5 and 39.3 issue and revoke
69+ them there. The page now shows all three credentials and says plainly what each
70+ one can do, because a user pasting a token into a feed reader should know it
71+ cannot write.
72+
73+ **12. One separator glyph.** `·` throughout. These pages are full of diff stats
74+ where a hyphen is already a minus sign.
75+
76+ **13. Archiving was a one-way door.** Found while rewriting the hook pseudocode.
77+ 21.3 says unarchive by editing one line and pushing, but archiving rejected
78+ every push, so nothing could ever be unarchived. The owner is now exempt, for
79+ the same reason the owner can always push a broken config: owner access comes
80+ from the namespace, not from the file.
81+
82+ ## Found while building stage 1
83+
84+ **14. An empty repository is private, and nothing else can be.** Chapter 11 says
85+ push-to-create sets visibility private, but visibility is `[repo] visibility` in
86+ `.barerepo/config`, and a repository created by a push has no tree to hold that
87+ file. Appendix B's schema default was `public`, so a pushed-to-create repository
88+ was world-readable. A live clone confirmed the leak.
89+
90+ Resolved: **absent means private, and only the exact word `public` opens a
91+ repository.** An empty value, an unknown value and a typo all stay closed. The
92+ web form's visibility field stores nothing at creation either; choosing public
93+ adds the line that sets it to the block the empty-repository page tells you to
94+ paste. Chapters 11, 18, 24 and appendix B all say so now.
95+
96+ **15. A push must be challenged before it is answered.** A git client sends no
97+ credential until it gets a 401. On a public repository the ref advertisement for
98+ a push succeeded anonymously, so the client never sent its token and the push was
99+ then refused for the wrong reason: "you cannot push here" when the truth was
100+ "you were never asked who you are". Chapter 41.4 now says to return 401 on both
101+ halves of a push, and to never challenge a read.
102+
103+ ## Found while building stage 2
104+
105+ **16. The file view links to a history page that has no route.** `repo-file.html`
106+ and `repo-config.html` both show `history · raw` in the footer. Chapter 24 says
107+ the config page has "history, blame, and raw links". Appendix C routes neither
108+ `history` nor, until amendment 7, `raw`.
109+
110+ History would be the log filtered to one path, which git answers with
111+ `git log -- <path>` and which nothing else in the book describes. Left unbuilt
112+ and rendered as plain muted text rather than a link, so the footer keeps its
113+ shape without offering a page that is not there. It needs a route in appendix C
114+ before it can be built.
115+
116+ **17. The compare mockup has no submit button.** Chapter 24 says both sides are
117+ free text fields, and every other form mockup shows a button. `repo-compare.html`
118+ shows the two refs as boxes with nothing to press. Built with a `compare` button
119+ in the same style as the other forms, because a form a keyboard user can submit
120+ and a mouse user cannot is not finished.
121+
122+ ## Found while building stage 3
123+
124+ **18. The mockups are built from inline styles, which the product's own policy
125+ forbids.** Chapter 42.7 sets `style-src 'self'` with no `'unsafe-inline'`, so a
126+ `style="..."` attribute is dropped by the browser. Every mockup in `plans/` is
127+ made almost entirely of them.
128+
129+ That is fine for the mockups: they are opened as files, with no policy, and
130+ inline style is a reasonable way to write a static reference. It is not fine for
131+ the real pages, and it fails silently, which is the dangerous part. A template
132+ that carries a style attribute renders with that spacing missing and nothing
133+ says so.
134+
135+ Caught by measuring a live page against its mockup: the gap under the sign-in
136+ button was 0px where the mockup has 18px, because the attribute holding it was
137+ being dropped.
138+
139+ Every template now uses classes only, and `TestNoInlineStyles` fails the build
140+ if a style attribute or a script tag reappears. Anyone porting a mockup to a
141+ template has to move its spacing into `barerepo.css` on the way.
142+
143+ **19. The keys mockup shows token values it cannot possibly know.**
144+ `keys.html` listed `rt_live_7Kq2mXe` and `ft_live_9Xk2m` as the headline of each
145+ token row. Chapter 15 says a token is displayed once at creation, inside the
146+ command that uses it, and that the server stores a hash. So the keys page can
147+ never render either string.
148+
149+ The mockup now names what the row actually is: the machine a runner token is
150+ attached to, or the feed a feed token opens. The token value appears exactly
151+ once, on the page that issued it, with a line saying it will not be shown again.
152+
153+ **20. Signing in did not record which key was used.** Chapter 32.5 says the keys
154+ page shows a last-used time per key, "check this if you think a key is lost".
155+ `ssh-keygen -Y verify` answers only yes or no, so an allowed_signers file
156+ holding every key cannot say which one matched. The keys are now tried one at a
157+ time and the matching one is stamped.
158+
159+ Still missing: a push over ssh does not stamp the key either, because
160+ `authorized_keys` passes only `--account`. Adding the fingerprint to that forced
161+ command would fix it, and chapter 41.3 would need to say so.
162+
163+ ## Found while building stage 4
164+
165+ **21. Appendix A's revision refs cannot exist.** The layout said:
166+
167+ ```
168+ refs/proposals/<n> a proposal
169+ refs/proposals/<n>/rev/<k> a retained earlier revision
170+ ```
171+
172+ A ref is a file, so those two ask git for a file and a directory of the same
173+ name. git refuses outright:
174+
175+ ```
176+ cannot lock ref 'refs/proposals/47/rev/1':
177+ 'refs/proposals/47' exists; cannot create 'refs/proposals/47/rev/1'
178+ ```
179+
180+ `refs/proposals/47` cannot move, because chapter 36.5 tells every reviewer to
181+ fetch it by that name. So the revisions moved: **`refs/revisions/<n>/<k>`**.
182+ Appendix A, chapter 12, chapter 26 and chapter 43.5 all say so now, and a test
183+ creates both refs so the layout cannot drift back.
184+
185+ **22. Appendix D says `rewrite(ref -> ...)` without saying how.** A pre-receive
186+ hook cannot redirect a push; it can only accept or reject. The mechanism is
187+ git's `proc-receive` hook, and the config that enables it is
188+ `receive.procReceiveRefs` with a **prefix** value, not a glob.
189+
190+ Getting that wrong fails silently: with `refs/proposals/*` the hook is never
191+ called, no error appears anywhere, and the push creates a ref literally named
192+ `refs/proposals/new`, the one thing appendix A says never exists. Chapter 12
193+ now documents the mechanism and the trap.
194+
195+ **23. Chapter 13's note layout could not be read by git.** It said the note tree
196+ holds one blob per comment, named `<unix-timestamp>-<author>-<short-hash>`.
197+
198+ `git notes` looks a note up by the hash of the object it annotates, so the tree
199+ must be keyed by object hash. A tree keyed by anything else is invisible to
200+ every command chapter 35.4 tells the user to run, and the offline promise in
201+ chapter 3 stops being true.
202+
203+ Checked both halves against real git rather than assumed:
204+
205+ - `git notes --ref=threads/47 add` writes the blob at the path `<sha>`.
206+ - A `meta` blob alongside it does not disturb `git log --show-notes=threads/47`,
207+ because `meta` is not a valid hash and git ignores it.
208+
209+ Chapter 13 now says: one note per annotated object, comments as records inside
210+ it separated by `--`, and `meta` at the tree root for title, state and attached
211+ ref. Records are also the shape `union` merge resolves correctly, which is what
212+ chapter 7 already asked for.
213+
214+ **24. The sign-in page told users to run a program they do not have.** The
215+ mockup's only instruction was `forge auth john`. Signing a nonce needs nothing
216+ but OpenSSH, which is already installed, and Part VII opens by promising that no
217+ task needs the forge CLI.
218+
219+ Chapter 31.3 also made it harder than it is: three commands, writing the nonce
220+ to `/tmp/nonce`, leaving `/tmp/nonce.sig` behind, and using `echo -n`, which is
221+ not portable between shells. `ssh-keygen -Y sign` reads standard input and
222+ writes to standard output, so it is one line and leaves nothing:
223+
224+ ```
225+ printf '%s' '<nonce>' | ssh-keygen -Y sign -f ~/.ssh/id_ed25519 -n barerepo-auth -
226+ ```
227+
228+ Verified by copying the command the live page prints, verbatim, and signing in
229+ with it.
230+
231+ Rule 2 in chapter 5 now says this outright: the command a page shows must be one
232+ the user can already run, so the plain form comes first and `forge` is offered
233+ second as the shortcut it is. `TestTheCliIsNeverTheOnlyWay` fails the build if
234+ a template names a forge subcommand without its plain equivalent.
235+
236+ The runner page is the one honest exception, because a runner long-polls and a
237+ shell one-liner cannot. It now says so, and gives the four HTTP endpoints so
238+ anyone can write their own.
239+
240+ **25. The diff mockups have no line numbers, but chapter 35.3 says to press
241+ one.** "Open the proposal diff. Press the line number. Type your comment." The
242+ diff rows in `repo-commit.html`, `repo-compare.html` and `repo-log.html` render
243+ the code and nothing else, so there was no number to press, and an anchor of
244+ `config.go:43` pointed at a 43 the reader could not see.
245+
246+ The diff helper in `build.py` now numbers each line on the new side. A removed
247+ line gets a blank gutter, because it has no number on that side. The real diff
248+ parser carries the same number, and the number is a link wherever there is a
249+ thread to attach a comment to.
250+
251+ **26. Build results were unreadable by git, and the layout was a dead end.**
252+ Appendix A put them at `refs/notes/runs/<sha>`, one notes ref per built commit.
253+ Chapter 16 claims build history clones and is readable. Both halves fail:
254+
255+ - `git log --show-notes=runs` prints nothing, because git looks a note up by
256+ the hash of the object it annotates, not by the ref's name. Tested.
257+ - `refs/notes/runs` and `refs/notes/runs/<sha>` cannot both exist, so a
258+ repository that used per-commit refs can never move to the working layout
259+ without deleting every one of them first. Tested, and git says so plainly:
260+ `'refs/notes/runs/965790c...' exists; cannot create 'refs/notes/runs'`.
261+
262+ A busy repository would also carry one ref per commit it ever built.
263+
264+ Now `refs/notes/runs`, one ref, tree keyed by the built commit. Several runs of
265+ one commit are separate records, split by `--`, the same shape chapter 13 uses
266+ for comments. `refs/notes/releases/<tag>` moved to `refs/notes/releases` for the
267+ same reason, keyed by the tag object.
268+
269+ Chapter 16 also said a log under 64kb "is inlined" without saying where. It is
270+ an `output` field now, and `log` names a blob when the output is larger.
271+
272+ **27. A run record could not say what was built.** Chapter 24 says the runs
273+ page shows "commit, status, duration, ref, and which machine ran it", and
274+ `runs.html` puts the ref on every row. Chapter 16's record had no ref field.
275+
276+ A commit arrives on a branch and on a proposal, so the commit alone does not
277+ answer it. The record carries `ref` now.
278+
279+ **28. The content security policy blocked the JavaScript the book budgets
280+ for.** Chapter 42.7 set `default-src 'none'` with no `script-src`, which blocks
281+ every script, and closed with "if a future feature needs `script-src`, that
282+ feature is wrong". Chapter 25 budgets 2kb of script for two keyboard shortcuts,
283+ `/` for search and `t` for the file jump, and chapter 34 tells users to press
284+ them.
285+
286+ Both cannot hold. The policy now includes `script-src 'self'`. Inline script is
287+ still blocked, `eval` is still blocked, and nothing loads from another host, so
288+ the rule the policy existed to enforce is intact: `keys.js` is 814 bytes and a
289+ framework cannot arrive through it. `TestScriptBudget` fails the build if it
290+ passes 2kb or if a second script file appears.
291+
292+ **29. Search has no index, on purpose, for now.** Chapter 17 wants an index
293+ built on push and kept in the cache directory. This runs `git grep` over the
294+ repositories the asker may read, and no others.
295+
296+ Chapter 17's warning is that filtering a shared index after ranking leaks the
297+ existence and count of private matches, and calls that the highest-severity
298+ mistake available in the codebase. Not looking at all is the same rule applied
299+ one step earlier, so that class of bug cannot occur here.
300+
301+ The cost is that a query is O(readable repositories). An index has to keep this
302+ property when it arrives.
303+
304+ **30. Copying with alternates makes a copy that a delete can destroy.** Chapter
305+ 21.1 says to copy server-side using git alternates, which is right: it keeps a
306+ 4 GB repository off a home connection. It did not say what happens next.
307+
308+ A copy made with `--shared` borrows the original's objects. Delete the original
309+ and the copy loses the history it never had its own copy of, which turns
310+ "delete my repository" into "delete somebody else's work".
311+
312+ A delete now detaches its dependents first: `git repack -a -d` writes every
313+ borrowed object into the copy and the alternates file goes. A delete that
314+ cannot detach a dependent fails instead of proceeding. Chapter 21.1 says so
315+ now, and `TestCopyAndDetach` deletes the source and checks the copy still has
316+ its history.
317+
318+ **31. A repository could be taken and never put back.** Chapter 40.3 tells a
319+ user to move hosts with `git push --mirror`. Writing the test in chapter 45.1
320+ showed that push being refused, ref by ref:
321+
322+ ```
323+ ! [remote rejected] refs/meta/counter (pre-receive hook declined)
324+ ! [remote rejected] refs/notes/runs (pre-receive hook declined)
325+ ! [remote rejected] refs/proposals/2 (pre-receive hook declined)
326+ ```
327+
328+ The access matrix in chapter 18 says those namespaces are the server's or
329+ nobody's, which is right for a contributor and wrong for the owner restoring
330+ their own repository. Chapter 3's promise is that you can point your clone at
331+ another host and keep working, and half of it was missing.
332+
333+ The matrix now says the namespace owner may write any ref in their own
334+ repository. It grants nothing that was withheld: an owner who wanted to forge a
335+ build result could already push any content they liked.
336+
337+ **32. Allocation could hand out a number already in use.** Chapter 35.5 tells a
338+ user they can open a thread by pushing `refs/notes/threads/<n>` from their
339+ clone. That leaves `refs/meta/counter` behind, and the next proposal took the
340+ same number, putting two conversations in one place. Allocation steps over any
341+ number that already names a thread or a proposal.
342+
343+ Both were found by writing chapter 45.1's test, on its first two runs.
344+
345+ **33. Anyone could take over anyone's proposal.** Chapter 12 says only the
346+ proposal's author and accounts with push access may update its ref. The check
347+ read the commit author, `git log --format=%an`.
348+
349+ A commit's author is whatever the pusher typed into `git config user.name`.
350+ Setting it to `lisa` was enough to force-push over lisa's proposal. Writing
351+ chapter 45.2's hook tests surfaced it: the rejection said "belongs to tester",
352+ which is the name the test harness commits under, not the account that pushed.
353+
354+ The author is now read from the thread's `meta` blob, which records the account
355+ that pushed. Chapter 12 says so, and `TestCommitAuthorIsNotIdentity` performs
356+ the takeover and expects it to fail.
357+
358+ The two also differ in ordinary use: applying somebody's patch and pushing it
359+ is normal, and it should not hand them your proposal.
360+
361+ **34. The chapter 25 budget is not met, and the reason is process spawn.**
362+ Chapter 45.5 says to assert the budget against a large repository rather than a
363+ toy. Against 1000 files and 200 commits, on an ordinary laptop:
364+
365+ | page | before caching | after caching | budget |
366+ |---|---|---|---|
367+ | file tree | 82ms | 38ms | 10ms |
368+ | log with diffs | 56ms | 56ms | 20ms |
369+ | file view with blame | 167ms | 47ms | 20ms |
370+
371+ Payloads are all inside budget: 1kb, 18kb, 1kb against 15kb, 30kb, 40kb.
372+
373+ Caching blame by blob hash and the per-entry log by tree hash, both of which
374+ chapter 25 specifies, took the file view from 167ms to 47ms and the file tree
375+ from 82ms to 38ms. Neither reaches the number.
376+
377+ The floor is process spawn. `git rev-parse HEAD` on a small repository measures
378+ 8ms, almost all of it spawn. A page that runs four git commands has spent 32ms
379+ before rendering anything. Chapter 25 now states this and says the budget is a
380+ budget on git invocations as much as on milliseconds: one or two per page,
381+ reached with `git cat-file --batch` and one `git log --patch` for a whole page,
382+ and libgit2 in-process where that is not enough.
383+
384+ Cutting invocations closed most of it. `git cat-file --batch` answers the hash,
385+ the size and the content in one process instead of three. HEAD and refs are
386+ plain files, so reading them costs nothing where asking git costs 8ms each.
387+ Parsed configuration caches by the commit it came from, which chapter 14 asks
388+ for. The per-entry tree log keys on commit and path rather than tree hash,
389+ because the commit is a file read and resolving the tree is a process.
390+
391+ Each commit's diff is also cached by its own hash. A commit cannot change, so
392+ a push invalidates one entry rather than the page. The commit list comes from
393+ one cheap `git log` with no patch; on a miss the diffs come from a single
394+ `git log --patch` for the range rather than one call per commit.
395+
396+ | page | first measured | now | budget |
397+ |---|---|---|---|
398+ | file tree | 82ms | 10.3ms | 10ms |
399+ | log with diffs | 56ms | 24ms | 20ms |
400+ | file view with blame | 167ms | 10.5ms | 20ms |
401+ | threads list | 104ms at 6 threads | 55ms at 50 | 10ms |
402+ | one thread | not measured | 22ms | 10ms |
403+
404+ **Three pages read git once per row, and none of them were measured.** The
405+ threads list read each thread's meta with a `cat-file`, listed each note tree
406+ with an `ls-tree`, and read every comment blob with another `cat-file`. Fifty
407+ threads with three replies each is roughly three hundred processes. The runs
408+ page did the same over the runs ref and then spent a `git log` per row for the
409+ commit subject. A single thread page did it over one thread's notes.
410+
411+ All three now use `gitx.Batch`, which drives one `cat-file --batch` process for
412+ many objects, and `gitx.TreeEntries`, which reads the raw tree git answers
413+ with. The threads list costs three processes whatever the thread count, the
414+ runs page three, and one thread two. Measured on the live server: threads list
415+ 104ms to 32ms at six threads, runs page 78ms to 32ms at four runs, one thread
416+ 32ms to 21ms.
417+
418+ **The remaining cost is git working, not forge spawning.** `gitx.Run` for a
419+ `rev-parse` measures 7.8ms on this machine and a bare `exec.Command` measures
420+ 9.1ms, so the wrapper adds nothing and the book's 8ms figure holds. Three
421+ processes is 24ms of that 55ms; the rest is git reading fifty trees and a
422+ hundred blobs, which is work no amount of batching removes. Chapter 25's 10ms
423+ is not reachable from a process, and the chapter already names the answer:
424+ libgit2 in-process rather than a looser number.
425+
426+ Getting the threads list to the chapter's stated two processes means dropping
427+ the `for-each-ref` and reading the ref files directly, the way HEAD is read.
428+ That is left alone deliberately: a direct read is wrong inside a worktree,
429+ which is a bug this codebase has already shipped once, and it saves 8ms
430+ against a page that misses by 45.
431+
432+ `TestBudget` is left failing rather than adjusted, because chapter 45.5 asks
433+ for build-failing thresholds and a threshold quietly raised to match the code
434+ measures nothing.
435+
436+ ## Also changed
437+
438+ **SQLite or PostgreSQL.** One setting picks it:
439+
440+ ```toml
441+ [database]
442+ url = "sqlite:///var/lib/barerepo/forge.db"
443+ ```
444+
445+ SQLite is the default and is the right answer for almost every install: the
446+ server-owned data is small, writes are serialised by one process, and the backup
447+ becomes a file copy. Postgres exists for people who already run one. It does not
448+ make forge faster; the hot path is git.
449+
450+ Chapter 41.7.1 covers it. `[paths] db` is gone, replaced by `[database] url`.
451+
452+ **Tests run on SQLite.** It needs no service, so every test gets a fresh empty
453+ database. The Postgres schema is held to the SQLite one by a test that compares
454+ the two definitions column by column and needs no server. Run the full suite
455+ against a real Postgres before a release, not on every commit.
456+
457+ ## The write path was the last one starting a process per row
458+
459+ Reading a thread was batched; writing one was not. `Write` is a compare-and-swap
460+ on the notes ref, and the loser reads again rather than dropping a comment. That
461+ is correct, and under load it was quadratic: twenty writers meant the twentieth
462+ lost nineteen times, and each attempt cost a `ls-tree` plus a `cat-file` per
463+ note. Chapter 45.3's thousand comments took 323 seconds and came within one
464+ retry of the twenty the loop allows, which is where a comment starts being lost
465+ for real rather than in theory.
466+
467+ Two changes. Writers in one process now queue on a striped mutex, so the swap is
468+ contention between processes and not inside one. The read inside the loop uses
469+ `gitx.Batch`, so a thread costs two processes whatever the note count. The same
470+ test now takes 71 seconds, and nothing is dropped.
471+
472+ Serialising in one process leaves the swap untested by the tests named for it,
473+ because they can no longer collide. `TestConcurrentRepliesAcrossProcesses` runs
474+ eight real processes at one thread, which is the case that actually happens: a
475+ hook and the web server writing at once. Deleting the swap's old value makes it
476+ keep one comment of eight, so it is testing what it says it is.
477+
478+ `countComments` was dead and is gone.
479+
480+ The whole suite went from 232 seconds to 66, and three of the five chapter 25
481+ budgets now pass that did not. `TestBudget` still fails on the threads list, one
482+ thread, and the log with diffs, and is still left failing rather than adjusted.
483+
484+ ## Three pages put the wrong thing at the right of the tab row
485+
486+ `repotabs` ended every page with "jump to file `t`". The mockups end four pages
487+ differently: threads with `open · merged · closed · all`, runs with `runners ·
488+ add a runner`, runners with `add a runner`, releases with `newest first`. Only
489+ the code pages get the file jump, which is the only place the `t` shortcut goes
490+ anywhere.
491+
492+ The threads one was not a missing decoration. Chapter 24's thread list says
493+ "Filters are open, merged, closed, all" and forge had no filter at all, so a
494+ repository with three open threads and forty-one closed showed forty-four rows
495+ and no way to narrow them. The filter is a query parameter, the default is all,
496+ and an unknown value shows everything rather than an empty page. Abandoned
497+ counts as closed, because the row names four states and the model has five.
498+
499+ The counts in the header stay counts of every thread. A number that moves with
500+ the filter is the filter read back, and says nothing.
501+
502+ ## Still missing on the thread list row
503+
504+ Chapter 24 says each row shows "whether a ref is attached, diff stats if so,
505+ build status if any, and reply count". Forge shows the ref and the count. The
506+ mockup's `has proposal · +81 -12 · build ok · 2 replies` is two thirds there.
507+
508+ A diffstat per row is a git process per row, which is the thing chapter 25
509+ forbids, so it wants the same treatment the commit diffs got: cache by the pair
510+ of end hashes, which are immutable, and pay only for proposals not seen before.
511+ The build status is already in the runs notes the runs page batches.
512+
513+ ## The log page now opens one diff at a time, against what the book says
514+
515+ Chapter 24's repository log says "the diff **already expanded**", chapter 34.1
516+ says "Each commit shows its diff. Large diffs are collapsed. Press expand", and
517+ docs/BUILD.md repeats it. The page now ships every diff shut, and opening one
518+ shuts the last.
519+
520+ This is the author's call and it overrides the three places above, which should
521+ be amended. The reason it was asked for is visible in what the page actually
522+ did: twenty commits, thirteen of them over the inline threshold, so thirteen
523+ rows said "large diff collapsed" and seven showed a wall of diff. Which state a
524+ row was in depended on a size threshold the reader cannot see, there was no way
525+ to shut a diff once open, and "expand" was not an expand at all, it navigated to
526+ the commit page. Every complaint in that sentence is true whichever default is
527+ chosen.
528+
529+ **No JavaScript was added.** `<details name="log">` is an exclusive group in
530+ HTML: opening one closes the others, with no script at all. Chapter 25 budgets
531+ 2kb of JavaScript for two keyboard shortcuts and this spends none of it. Where a
532+ browser does not know the `name` attribute it ignores it, and the diffs are
533+ still collapsible, just not exclusive, which is the right way for it to fail.
534+
535+ A large diff is still not inlined, because the payload budget is on what is
536+ sent, not on what is displayed, and a shut `<details>` has still sent its
537+ contents. Those rows say "too large to inline" and link to the commit, and they
538+ carry no disclosure triangle, because there is nothing behind them to disclose.
539+
540+ ## Things that should have been links and were not
541+
542+ The month on the keys and profile pages was formatted with `"jan 2006"`. Go's
543+ reference month is `Jan`, so a lowercase one is not a token and was copied out
544+ literally: every key ever added read "added jan 2026" whatever month it was.
545+ Now formatted with the reference layout and lowercased afterwards, which is what
546+ the mockups show.
547+
548+ Then a sweep of every view for values that name something forge can open:
549+
550+ | view | was plain | now opens |
551+ |---|---|---|
552+ | log | the commit hash, the author | the commit, the account |
553+ | commit | the parent hash, the author, each path, the branch | the commit, the account, the file, the log |
554+ | thread | the merged commit, the proposal ref, each author, a line anchor | the commit, a compare, the account, the file at the line |
555+ | thread list | the merged commit, the author | the commit, the account |
556+ | runs, run | the ref, the runner, the subject, the run's hash | a compare, the runner list, the commit |
557+ | releases | the tag, the tagger | the files at the tag, the account |
558+ | compare | each path | the file on the b side |
559+
560+ **A name in a commit or a pushed note is not an account.** It is whatever the
561+ writer set in their local git config, so linking it blindly makes a page full of
562+ 404s. Every name is checked against the accounts table first, in one query per
563+ page rather than one per row, and a name that is not an account stays plain
564+ text. On the live server this is visible: `lisa`, `dave` and `rock` link, and
565+ `donuts-are-good` does not, because only the first three are accounts.
566+
567+ The same rule applies to hashes and paths. A deleted file gets no link, because
568+ the file does not exist at that commit; `parsePatch` now records `Gone` from
569+ git's "deleted file mode" line. A thread with nothing merged gets no merged
570+ link. An anchor whose original is lost gets no file link.
571+
572+ Every link on the log, thread list, thread, runs, releases, commit and file tree
573+ pages was then fetched. None of them 404s.
574+
575+ ## A readme link, and refs you can see before you type them
576+
577+ Two gaps the book does not name, both asked for directly.
578+
579+ **The readme.** Chapter 24 is against a *rendered* readme on the landing page,
580+ because that is the space GitHub spends instead of answering what changed. It is
581+ not against a repository saying it has one. The log bar now carries a `readme`
582+ link when the root tree holds one, under any of the usual names, and it opens
583+ the file view, which shows source with a blame gutter like every other file.
584+ Nothing is rendered on the log page and nothing moved off it.
585+
586+ Finding it costs one tree read, cached by the commit, so it is free after the
587+ first hit. The log page measures 16ms against chapter 25's 20ms with it in.
588+
589+ **The refs.** Chapter 24 says the compare fields are free text and gives the
590+ reason: `master...refs/proposals/47` is a legitimate thing to type and a
591+ dropdown of branches cannot express it. That reasoning holds, and it left a
592+ repository's branches, tags and proposals invisible everywhere in the web
593+ interface. A field whose valid values cannot be discovered is a field only its
594+ author can use.
595+
596+ So the compare page lists what exists, under the form, as links that keep the a
597+ side and swap the b side. It is not a picker: the fields stay free text and the
598+ list is what is there to type. The branch name in the log bar links to that
599+ page, which is the only place a branch name meant anything before.
600+
601+ Notes refs are left out. They are storage, not somewhere to compare against.
602+
603+ ## The thread list row is now the whole row chapter 24 asks for
604+
605+ Chapter 24 says each row shows "whether a ref is attached, diff stats if so,
606+ build status if any, and reply count". Forge showed the ref and the count. Now
607+ it shows all four, in the mockup's order: `has proposal · +2 -0 · build ok ·
608+ 3 replies`, with a failed build in the one colour the mockups use for it.
609+
610+ Neither number costs a process per row. Every run in the repository comes back
611+ in the two processes `run.Recent` already used for the runs page. The diffstat
612+ is keyed on the two end hashes, which are objects and cannot change, so a
613+ proposal is measured once and never again. Resolving a proposal ref to a hash is
614+ a file read.
615+
616+ The threads list measures 41ms against chapter 25's 10ms, up from 35ms, on a
617+ cold cache in a repository built by the test. It was already failing that
618+ threshold for the reason SPEC-NOTES gives above: git reading fifty trees is work
619+ no batching removes.
620+
621+ ## Two bugs from one wrong assumption about HEAD
622+
623+ `gitx.ResolveRef` read a ref file and returned its contents. For every ref under
624+ `refs/` that is a hash. For `HEAD` it is the line `ref: refs/heads/master`,
625+ because HEAD is symbolic.
626+
627+ So `ResolveRef(dir, "HEAD")` returned a ref name where every caller expected a
628+ hash, and returned it with no error, which is the worst shape a wrong answer can
629+ have. It broke two things in one afternoon: the readme page 404'd, because the
630+ name is not a tree, and every proposal's diffstat came back `+0 -0`, because the
631+ name is not a rev.
632+
633+ Fixed in `gitx` rather than at either call site, since the next caller would
634+ have hit it too. `ResolveRef` now follows `ref: ` up to five times and errors on
635+ a ref that points at itself, which is a file somebody wrote by hand.
636+
637+ ## Seventeen templates were never rendered by the test that claims to render them
638+
639+ `TestTemplatesRender` opens "Every template runs here, because a typo otherwise
640+ fails when a user asks for the page." It ran ten of twenty-seven. The other
641+ seventeen, including every account page, the runner setup, search and the inbox,
642+ had nothing rendering them at all.
643+
644+ The test now walks `pages` and fails on any template with no case, so the next
645+ one cannot ship untested. The seventeen have cases.
646+
647+ ## Also
648+
649+ The log rows are uniform: every commit carries a `view commit` link, and the
650+ "too large to inline" wording is gone. A link inside `<summary>` navigates
651+ without toggling the disclosure, which was checked in a browser rather than
652+ assumed.
653+
654+ The rendered readme keeps each control once: `rendered` in the bar, `[source]`
655+ under the prose, `raw` in the footer.
656+
657+ ## The breadcrumb named two places and linked neither
658+
659+ `rock / forge / 26ddf1d` sat at the top of seventeen pages with the account and
660+ the repository as plain text. Both are places, and the reader is one click from
661+ each on every page except the one that made them type the URL.
662+
663+ There is now a `crumbs` template, so a page cannot write the pair by hand and
664+ forget, and a test fails on any template that goes back to doing so. The last
665+ segment stays plain, because it names the page you are already on.
666+
667+ The file tree is the one page where the middle segment moves: at the root the
668+ repository *is* the page, and under a path it is somewhere to go back to, so it
669+ links only in the second case.
670+
671+ ## The one line on the runner page pointed at a 404
672+
673+ BUILD.md calls the runner command "the piece to get exactly right, because it is
674+ the most visible proof of the no-interstitial thesis". The page rendered it
675+ correctly and both halves of it, `/runner.sh` and `/runner.ps1`, returned 404.
676+ So the page that exists to prove one paste is enough handed out a paste that
677+ failed.
678+
679+ The protocol and the `forge runner` subcommand were already built. Only the
680+ script was missing.
681+
682+ **Where the binary comes from.** BUILD.md says "same single binary as the
683+ server, different subcommand", so the server hands out its own executable at
684+ `/runner/binary`, guarded by the runner token, which the script already has and
685+ a stranger does not. No release hosting, no second artifact to keep in step.
686+
687+ **What it does when it cannot help.** A server can only serve the platform it
688+ was built for. The script compares `uname` against the server's and, on a
689+ mismatch, prints the two commands that build and run it instead. A wrong
690+ architecture installed silently is worse than a refusal that says what to do.
691+
692+ Measured on the live server, the real line: downloads 27mb, attaches, and the
693+ runners page shows the machine as idle. That is chapter 15's "the page the user
694+ copied from updates the moment the runner attaches", working.
695+
696+ The book writes the url as the hosted `barerepo.sh`. Serving it from each install's
697+ own `external_url` is what a self-hosted forge can actually do, and it is what
698+ the template already rendered.
699+
700+ ## Reading GitHub workflows, and the book grew a chapter for it
701+
702+ The book had nothing on this. It has chapter 15A now, and BUILD.md has a stage
703+ 6A, because the author asked for the feature and a feature the book does not
704+ describe is a feature nobody can check.
705+
706+ **It is not a contradiction.** Chapter 3 already credits `.github/workflows`
707+ with learning the lesson forges forgot: configuration is a file in the tree. The
708+ objection in this book was never to that file. It was to a forge configurable
709+ only through its own web forms. Reading a workflow honours the same rule
710+ `.barerepo/config` honours.
711+
712+ **What it translates.** Every `run:` step into one shell script, in order, with
713+ `env:` turned into exports and `working-directory` into a subshell so a step's
714+ directory does not leak into the next. `actions/checkout` is answered rather
715+ than run, because forge cloned the repository already. `container:` becomes the
716+ image.
717+
718+ **What it declines, and the rule the feature rests on.** Any other action, any
719+ `if:`, any matrix, any shell forge cannot start, and any `${{ }}` left in a
720+ command. Each one is printed in the push output, naming the step and the reason.
721+ A skipped step is never silent. A build reporting success while quietly running
722+ half of what was asked is worse than no build, and it is the failure every
723+ partial implementation of somebody else's format drifts toward.
724+
725+ **runs-on, against machines that actually exist.** A job now carries the labels
726+ it asked for, and a runner takes the first queued job it *satisfies* rather than
727+ the first queued job, so a build waiting for a machine nobody attached does not
728+ block the ones that could run now. `ubuntu-latest` and its siblings match on the
729+ operating system the runner reported when it attached, `self-hosted` is always
730+ true because every forge runner is, and anything else must be a label the runner
731+ declared.
732+
733+ When nothing fits, the push declines and prints the page that fixes it, which is
734+ chapter 11's rule about typos applied to machines:
735+
736+ ```
737+ unit wants a ubuntu-latest machine and none is attached.
738+ attach one: https://barerepo.example/john/johnbot/runners/new
739+ ```
740+
741+ **Precedence.** `[build] command` wins and the workflow is not read at all. A
742+ repository that answered in forge's own file is not second-guessed, and a
743+ repository with both does not build twice. Both directions are tested.
744+
745+ **The one dependency.** `gopkg.in/yaml.v3`. Hand-rolling a YAML subset for a
746+ format other people control is how a parser becomes a bug farm, and the book's
747+ minimalism is about architecture, not about refusing a parser.
748+
749+ The `jobs` table grew a `labels` column in both dialects, which the portability
750+ test compares column by column and passed.
751+
752+ ## Making a workflow work without editing it, and a silent pass it turned up
753+
754+ The aim the author set is that a repository arrives with the file it already has
755+ and builds. Two things stood between that and the first implementation.
756+
757+ **`${{ }}` was not a decline, it was a break.** The first version left an
758+ expression in the command and reported it. `sh` reads `${{` as a bad
759+ substitution and fails the step outright, so any workflow naming
760+ `${{ github.sha }}` failed at the first step that used one. That is the most
761+ common thing in a real workflow after `run:` itself.
762+
763+ Forge now sets the environment GitHub sets, including `GITHUB_SHA`,
764+ `GITHUB_REF_NAME`, `GITHUB_REPOSITORY`, `GITHUB_WORKFLOW`, `GITHUB_JOB`, `CI`,
765+ and the runner's own `RUNNER_OS`, `RUNNER_ARCH` and `RUNNER_TEMP`, and fills the
766+ expressions that name the same things. A workflow reading either form now works
767+ untouched. This is a substitution and not an evaluator: `secrets`, and anything
768+ else outside the list, is still left alone and still reported.
769+
770+ **A failing step did not fail the build.** The runner passes the command to
771+ `sh -c` with no `set -e`. For chapter 15's one command that is correct, and its
772+ exit code is the command's. For a translated workflow of many steps it is not:
773+ the first step could fail, every later step still ran, and the build reported
774+ the exit code of the last one. Measured: a script whose first line is `false`
775+ exits 0.
776+
777+ That is precisely the green build that means nothing, which is the failure the
778+ whole chapter is written against, and the feature shipped with it. The generated
779+ script now begins with `set -e`, which is also what GitHub does. `[build]
780+ command` is untouched, being one command by definition.
781+
782+ **The setup actions check, they do not install.** `setup-go`, `setup-node`,
783+ `setup-python`, `setup-java` and `setup-dotnet` become a test that the tool is
784+ on the machine, failing with a sentence if it is not, and printing the version
785+ the workflow asked for beside the version the machine has. The version is
786+ printed rather than enforced, since a patch digit should not fail a build.
787+
788+ Not installing is the point. The runner is a machine the user owns, and a build
789+ that quietly puts a toolchain on somebody's laptop is doing something the person
790+ who pasted one line did not agree to. Chapter 15 says a build has access to
791+ whatever the machine has. It does not say a build may change what the machine
792+ has. A test asserts no generated line runs `apt-get`, `brew install`, `npm
793+ install`, `pip install` or a piped shell script.
794+
795+ `actions/cache` is answered as a statement that forge does not cache, so a build
796+ starting from the clone is a stated fact rather than a surprise.
797+
798+ ## A matrix builds a job several times, so forge queues it several times
799+
800+ The first version declined a matrix whole, which meant a repository whose CI is
801+ a matrix got no build at all. That is most repositories that test more than one
802+ version of anything.
803+
804+ The axes are multiplied, `exclude` removes what it names, and each combination
805+ becomes its own job with its own `runs-on`. The combination is substituted into
806+ the command, the image and the machine, and exported as `MATRIX_<AXIS>` so a
807+ step reading the environment works as well as a step reading the expression.
808+
809+ `include` is not applied. It can add keys to a combination and whole
810+ combinations no axis names, and a wrong guess there runs a build the workflow
811+ did not ask for. Forge says so and builds the axes.
812+
813+ **The part that made it usable rather than merely present.** A matrix queues one
814+ commit several times, and `run.Record` had no way to say which build was which:
815+ three combinations produced three records distinguishable only by the machine,
816+ and not at all when two ran on one machine. A matrix build where you cannot see
817+ which combination failed is not worth having.
818+
819+ The job now carries a name, `test (go 1.26, os ubuntu-latest)`, with the axes
820+ named and sorted so the same matrix always produces the same names. The server
821+ already holds the job when the run finishes, so the name reaches the record
822+ without the runner protocol changing at all. The runs page and the run page both
823+ show it.
824+
825+ The `jobs` table gained `name` beside `labels`, in both dialects, which the
826+ portability test compares column by column and passed.
827+
828+ **What this looks like when the machines are not all there.** A matrix over
829+ ubuntu and macos, on a server with only a linux runner attached, queues the
830+ ubuntu half and declines the mac half by name, with the link to attach one. That
831+ is tested end to end, along with each machine taking only its own combination.
832+
833+ ## The runs page said builds were off while builds were running
834+
835+ `BuildOn` was `[build] command != ""`, which was the whole truth until chapter
836+ 15A gave a repository a second way to build. A repository building from
837+ `.github/workflows` showed "builds are off. set [build] command to turn them
838+ on" underneath its own build results.
839+
840+ The page now says what the repository actually builds from, naming the files,
841+ and the off state names both routes rather than only forge's own. Finding out
842+ costs one process and only when `[build] command` is absent, since a repository
843+ that answered in forge's file is not asked a second question.
844+
845+ This is the same shape as the seeded runs that read "builds are off" earlier in
846+ this file: a page holding two facts that contradict each other, where one of
847+ them was written before the other existed.
848+
849+ ## I added two columns without a migration, and every existing install would have broken
850+
851+ The `jobs` table gained `labels` and then `name` for chapter 15A. I added them to
852+ the `CREATE TABLE` in migration 5, which is the one migration this codebase
853+ already says must never be edited: `migrations` is append-only and
854+ `schema_migrations` records how far an install has got.
855+
856+ A fresh database therefore had the columns and every existing one did not. The
857+ first `/runner/poll` after the upgrade answered `no such column: labels`, 500,
858+ forever. Builds would have stopped on every server that had ever run forge
859+ before, and only on those.
860+
861+ **Nothing in the suite could have caught it.** BUILD.md says tests run on SQLite
862+ because it needs no service, so every test gets a fresh empty database. That is
863+ the right call and it makes the whole suite structurally blind to an upgrade. It
864+ was found by pasting the runner line at the development server, whose database
865+ was three days old.
866+
867+ Fixed by putting migration 5 back as it was and adding migration 6, which is
868+ what the mechanism was always for.
869+
870+ Two tests now cover the class:
871+
872+ `TestAnExistingDatabaseUpgradesToEveryLaterVersion` opens a database as an older
873+ forge that knows only migrations 1..n, closes it, reopens with the full list the
874+ way a replaced binary does, and then runs the real query. It does this for every
875+ n, so a column added without a migration fails at whichever version predates it.
876+
877+ `TestMigrationsAreAppendOnly` holds the count. Adding a migration means raising
878+ one number and is meant to be easy; editing an earlier one changes the count in
879+ the other direction and fails with the reason spelled out. Falsified by putting
880+ the original mistake back, which it catches.
881+
882+ ## The runner needed a flag the book says it does not
883+
884+ Appendix E gives `forge runner <token> [--labels a,b]`, chapter 38.2 gives
885+ `forge runner rt_live_7Kq2mXe --labels build,test`, and the runner-setup mockup
886+ gives the same. Forge's own page printed `--server https://...` in the middle of
887+ it, and the binary refused to start without it.
888+
889+ A token that cannot say where it came from forces a second parameter, which is
890+ the interstitial chapter 15 exists to remove.
891+
892+ The runner now records where a token attached, after the attach succeeded so a
893+ wrong address is never the one kept, in `~/.config/forge/servers`. The file maps
894+ a hash of the token to a url and holds no token, at mode 0600. The pasted line
895+ supplies the address the first time and the documented line needs none after.
896+
897+ Measured on the development server: the one-liner attaches, and then
898+ `forge runner <token> --labels build,test` attaches with no address at all.
899+
900+ ## A matrix could show a green thread row over a failed build
901+
902+ The thread list reads the runs for a proposal and stopped at the first one that
903+ named the right ref. That was correct while a proposal had one build. A matrix
904+ gives it several, `run.Recent` returns them newest first, and the newest is not
905+ the one that matters.
906+
907+ So a proposal whose linux build failed at 10:00 and whose mac build passed at
908+ 10:01 said `build ok`. Chapter 19.1 says green is not news; a green that is
909+ hiding a red is worse than not news.
910+
911+ The row now reduces every run for the ref to one status, and any failure is the
912+ status: `1 of 3 builds failed`, in the colour the mockups keep for it. One run
913+ still reads exactly as the mockup does, `build ok · test ok`, because that is
914+ what the build reported and there is nothing to summarise.
915+
916+ The thread page did not have the bug, since chapter 24 puts each build in the
917+ timeline as its own event and it never stopped early. It did drop the job name,
918+ so three builds of one proposal read as three identical rows. They now say which
919+ combination each was.
920+
921+ ## The escape hatch was written and never wired
922+
923+ `internal/webhook` held the deny list, the HMAC signature and the retry, and
924+ nothing in the tree called any of it. A repository could put `[[webhook]]` in
925+ `.barerepo/config` and forge would do nothing with it, without saying so.
926+
927+ Chapter 23.1 calls a webhook the escape hatch, the thing that makes it
928+ acceptable to refuse every integration request forever. An escape hatch that is
929+ present in the source and absent at run time is worse than one that was never
930+ started, because the config file accepts the lines.
931+
932+ **Where it fires from.** The event log, after a cursor, read by the server
933+ process. Both writers of events keep working the way they did: the hook process
934+ records a push and returns, the web process records a comment and answers. A
935+ receiver that takes thirty seconds cannot slow a push, because nothing on the
936+ push path is waiting for it.
937+
938+ A first start puts the cursor at the newest event. A new hook is not a request
939+ for ninety days of history.
940+
941+ **The secret is a name.** `secret_env = "DEPLOY_HOOK_SECRET"` names a value in
942+ the server's environment. A name whose value is not set is a recorded failure
943+ that says which name, not an unsigned delivery to a receiver expecting a
944+ signature. Committing a secret to a public repository stays impossible by
945+ construction, which is the whole reason 23.2 names it rather than holding it.
946+
947+ **Twenty failures in a row stops a hook**, per 23.4, and the config page is the
948+ only place that could report it, so it does: the url, the events it asked for,
949+ and either when it last delivered or the reason it is failing. One delivery
950+ clears the count, or a hook that fails once a week eventually stops for nothing.
951+
952+ Proven on the development server rather than only in tests: a pushed config
953+ naming a hook with an unset secret produced
954+
955+ 1 failure since the last delivery · DEPLOY_HOOK_SECRET is not set on this
956+ server, so nothing would sign the body
957+
958+ ## One dead receiver held up every other repository
959+
960+ The first sender walked one event at a time and each hook in turn, three
961+ attempts with backoff on every one. A receiver that is down costs about six
962+ seconds per event and twenty events before it disables, and nothing else in the
963+ queue moves for those two minutes.
964+
965+ A hook that has already failed now gets one attempt. The three attempts are for
966+ a receiver that is briefly down, and the first failure is the test of that. One
967+ event's hooks go at once, since they are independent of each other.
968+
969+ ## The budget was failing and the fork was most of it
970+
971+ Chapter 25's thresholds are asserted, and two rows had been red: the threads
972+ list at 39ms against 10, and one thread at 12ms.
973+
974+ Measured rather than guessed. Every git process this machine starts costs 6.5 to
975+ 8ms before git does any work, which the file tree page shows exactly: one
976+ `ls-tree`, 6.4ms, and 6.7ms on the clock. The threads list started four
977+ processes.
978+
979+ `git cat-file --batch` is now kept open per repository and pooled, so reading
980+ objects starts nothing. One thread went from 12ms to 400 microseconds.
981+
982+ **I guarded against something that does not happen.** A long-lived reader cannot
983+ see an object that arrives in a new pack, I assumed, so I stat'd every pack
984+ directory and restarted the process when it changed. It was wrong: git rereads
985+ the pack directory every time it answers `missing`, so the kept reader does see
986+ it. The guard is deleted. The test that made me delete it stays, because that
987+ reread is the assumption the entire pool rests on, and a future git that stops
988+ doing it must fail here rather than serve a page with a hole in it.
989+
990+ **And a deadlock that does happen.** The batch wrote every object id before
991+ reading any answer. Past the pipe buffer that is a deadlock: git stops reading
992+ input while it is blocked writing output, and forge stops writing while it is
993+ blocked writing input. Four thousand ids reproduces it. The write moved to its
994+ own goroutine. Both tests were falsified before being kept.
995+
996+ **The threads list itself.** It read every comment blob of every thread to count
997+ replies, and forked `for-each-ref` to find them. Refs are files, which is
998+ chapter 6, so the ref list costs no process now either. A row is cached by the
999+ note commit that wrote it, and a commit is immutable, so the entry never needs
1000+ invalidating. The page reads nothing it has read before.
1001+
1002+ 39ms to 7.4ms. The whole budget passes.
1003+
1004+ ## A url the book hands out, that answered 404
1005+
1006+ Chapter 19.5 lists four feeds, chapter 39.4 tells a reader to paste two of them
1007+ into a feed reader, and appendix C lists all four as routes. Three were served.
1008+ `/<user>/<repo>/threads.atom` fell through the route table to the 404 page.
1009+
1010+ The book prints that url twice, so the failure is not a missing feature, it is a
1011+ promise the running server does not keep. A reader who follows 39.4 gets a feed
1012+ reader with a dead entry in it and no reason given.
1013+
1014+ It now answers, with the same events the threads page shows and nothing else:
1015+ the six thread and proposal kinds from 19.1. A push is not discussion, so the
1016+ repository feed carries it and the threads feed does not. Both are asserted
1017+ against a live server rather than against the query, because the route was the
1018+ part that was missing.
1019+
1020+ A private repository answers 404 on both feeds, since 19.5 says public only and
1021+ existence leaks.
1022+
1023+ **The profile page had the same shape of gap.** `profile.html` puts `atom` in
1024+ the sidebar under the key count, and `/<user>.atom` has worked since the feeds
1025+ went in, but the template never linked it. The link is back. A feed nobody can
1026+ find from the page is a feed that needs the book open next to it.
1027+
1028+ ## The older link was drawn and never given a value
1029+
1030+ `repo-log.html` puts `older` in the footer, and the template has carried
1031+ `{{if .Older}}<a href="{{.Older}}">older</a>{{end}}` since the log was built.
1032+ Nothing ever set `Older`. The condition was false on every page forge has served.
1033+
1034+ So the log showed twenty commits and the twenty-first was unreachable. A
1035+ repository with two hundred commits published a hundred and eighty of them over
1036+ git and none of them over http.
1037+
1038+ The page now starts where `?from=<sha>` says, asks for twenty-one, and links the
1039+ twenty-first as `older`. There is no offset and no cursor to keep, because a
1040+ commit already names its own position in the history. A `from` that is not a
1041+ commit here answers 404 rather than quietly showing the newest page, since a
1042+ stale link that looks like it worked is worse than one that says it did not.
1043+
1044+ ## A tag cost a process, and a hundred tags cost a hundred
1045+
1046+ The releases page called `git notes show` once per tag, inside the loop over
1047+ `for-each-ref`. Measured against chapter 25's ten milliseconds, with a hundred
1048+ tags: **1.785 seconds**, and 29kb over a 15kb payload budget.
1049+
1050+ Three things were wrong and each is worth naming.
1051+
1052+ **The notes.** They are now read the way chapter 16's other note refs are: walk
1053+ the notes tree through the object pool, one level of fanout at a time, skipping
1054+ any subtree that holds no note this page asked for, then read the bodies in one
1055+ batch. No process at all, and only the twenty bodies the page draws.
1056+
1057+ **`for-each-ref`.** Eleven milliseconds on its own for a hundred tags, which is
1058+ the whole budget. Refs are files, per chapter 6, so the tag names and their
1059+ object ids come from the ref files, and the tag objects come from the pool.
1060+
1061+ Batching by ref name still cost 11ms because git resolves each name; batching by
1062+ the object id the ref file already gave costs 7.7ms. Then the sorted list is
1063+ cached under a digest of the ref state, so a page that follows a page with no tag
1064+ pushed between them reads no objects at all. The key is the content, so nothing
1065+ invalidates it.
1066+
1067+ **No end to the list.** Twenty rows, then `older`, the same word in the same
1068+ corner as the log. The releases mockup has two releases and so shows no such
1069+ link; the log mockup does, and one convention for "this list continues" beats
1070+ inventing a second.
1071+
1072+ 1.785s to 7.4ms, 29kb to 6kb. That row of the budget passes.
1073+
1074+ **Still failing, and it was failing before this pass.** The log with diffs takes
1075+ 26ms against 20. Measured at the previous commit, with none of this pass's
1076+ changes and no tags in the repository, it took 24ms. The file tree sits on 10ms
1077+ against 10 and crosses in either direction between runs. Neither is caused by
1078+ anything here, and neither is fixed by anything here.
1079+
1080+ ## The landing page spent nine milliseconds asking whether the repository was empty
1081+
1082+ Chapter 25 gives the log with diffs twenty milliseconds. It took twenty-six, and
1083+ the last pass could not say why. This pass measured the parts instead of reading
1084+ the code, which is the method that worked on the releases page.
1085+
1086+ page 22.9ms
1087+ gitread.Log 12.3ms, of which one fork is 11.5ms
1088+ transport.Open 0.2ms
1089+ readme, config ~0ms
1090+ git --version 8.2ms
1091+
1092+ That last line is the important one. A bare `git --version` costs 8.2ms on this
1093+ machine, so a process is 8ms before git does anything. Two processes is 16ms of
1094+ a 20ms budget.
1095+
1096+ The log page started two. The second was `repo.IsEmpty`, which ran
1097+ `git for-each-ref --count=1` on every repository landing page to answer "does
1098+ this repository have a ref". Refs are files, per chapter 6. `gitx.AnyRef` walks
1099+ `refs/` and stops at the first one, then falls back to `packed-refs`, which is
1100+ where `git gc` moves them. No process.
1101+
1102+ 26ms to 13ms. What remains is the one `git log`, and that one has to be a
1103+ process, because the order of a log is a revision walk and forge is not going to
1104+ reimplement one.
1105+
1106+ ## The file tree ran ls-tree for a listing the object pool already had
1107+
1108+ The same measurement put the file tree on 10.2ms against a 10ms budget, which
1109+ means it failed about half the runs. One `ls-tree --long` fork was the whole of
1110+ it.
1111+
1112+ `--long` is there for blob sizes. Nothing displays them: not the template, not
1113+ the mockup, and `Entry.Size` had one writer and no reader. So the size was the
1114+ only reason to ask git rather than read the tree object, and the size was never
1115+ used.
1116+
1117+ `gitx.TreeRows` reads a raw tree object, keeping the mode, since the mode is the
1118+ only field that says tree or blob. Entry order is git's own tree order, which is
1119+ what ls-tree was printing anyway, so the directories-on-top pass below it still
1120+ sees exactly what it saw.
1121+
1122+ 10.2ms to 0.5ms.
1123+
1124+ ## Rendered diffs are cached now, which the build guide asked for
1125+
1126+ `BUILD.md` says to cache rendered diffs because they are immutable. Forge cached
1127+ the patch text and re-parsed and re-rendered it on every request.
1128+
1129+ The hunk markup was written three times, identically, in `repo-log.html`,
1130+ `repo-commit.html` and `repo-compare.html`. It is one `hunks` block in the layout
1131+ now, and the log renders its commits through it once and keeps the html under the
1132+ commit hash.
1133+
1134+ Worth saying plainly: this was worth 0.7ms of the 10ms I thought it would fix. I
1135+ had assumed the render was the cost and it was not. The measurement above is what
1136+ found the two forks. The cache stays because the guide asks for it by name and
1137+ because it took triplicated markup down to one copy, not because it was the fix.
1138+
1139+ **The whole budget passes**, with margin on every row:
1140+
1141+ file tree 0.5ms 1kb (budget 10ms, 15kb)
1142+ log with diffs 13.1ms 22kb (budget 20ms, 30kb)
1143+ file view with blame 10.3ms 1kb (budget 20ms, 40kb)
1144+ threads list 2.1ms 13kb (budget 10ms, 15kb)
1145+ one thread 0.6ms 1kb (budget 10ms, 15kb)
1146+ releases 7.5ms 6kb (budget 10ms, 15kb)
1147+
1148+ ## Copying was written, and had no way to reach it from the site
1149+
1150+ `repo.Copy` clones with `--shared`, fetches notes and the counter, drops proposal
1151+ refs and installs hooks. `repo.Detach` un-borrows. `repo.Dependents` finds who
1152+ borrows. The delete path already calls Detach before it trashes anything, which
1153+ is the requirement in chapter 21.1 that stops a copy losing its history.
1154+
1155+ `forge copy <src> <dst>` uses all of it. Appendix C lists
1156+ `POST /<user>/<repo>/copy` and nothing answered it. The escape hatch shape again:
1157+ the machinery was complete and one route was missing.
1158+
1159+ **Who gets the button.** Not the owner. Chapter 21.1 exists because chapter 12
1160+ removed the fork, and the thing being restored is taking a project somewhere its
1161+ maintainer will not go. So the control is on the config page for any signed in
1162+ reader who can read the repository. Read access is the whole permission, because
1163+ chapter 12 already removed asking as a step.
1164+
1165+ **Where it goes.** `<you>/<the same name>`, with no field to fill in. The book's
1166+ own example is `forge copy john/johnbot lisa/johnbot`. If you already have a
1167+ repository by that name the page says so and links it, rather than offering a
1168+ button that will fail.
1169+
1170+ **What it says.** The same four sentences the CLI prints, because the terminal
1171+ and the page must not explain the same operation differently: branches, tags,
1172+ history, threads and notes come across, proposal refs do not, there is no link
1173+ back and no badge, and contributing means pushing a proposal.
1174+
1175+ **The plain commands come first**, per chapter 5 rule 2. `git clone --mirror`,
1176+ then a push naming heads, tags and notes, which is exactly the ref set the server
1177+ side copy moves. Forge creates the destination on push, per chapter 11, so the
1178+ plain path needs no visit to `/new`.
1179+
1180+ Proven end to end against a running server: lisa copies john's repository, the
1181+ copy has master and does not have the proposal ref that was pushed to the
1182+ original first, and john deleting the original leaves lisa's history intact. The
1183+ test asserts the original had a proposal ref before the copy, so the assertion
1184+ that none came across cannot pass by accident.
1185+
1186+ ## The last url in appendix C that answered 404
1187+
1188+ `GET /<user>/<repo>/release/<tag>` is in the route table. Chapter 24 has one
1189+ entry for releases and it describes the list. There is no mockup for a single
1190+ release and no paragraph describing one.
1191+
1192+ Building a page the plans do not describe would be inventing product voice, which
1193+ is the mistake that produced a landing page full of made up copy earlier in this
1194+ work. Answering 404 to a url the book prints is the mistake fixed two passes ago.
1195+
1196+ So it opens the list at that tag, using the `?from=` the releases page already
1197+ takes. A release is a tag, a body and some files, and all three are on that row.
1198+ A tag that is not in the repository is a 404, not the newest release wearing the
1199+ wrong name.
1200+
1201+ Every url in appendix C now answers.
1202+
1203+ ## A repository search result printed its own name twice
1204+
1205+ `search.html` gives one thing a box: the matched source line. A code row is the
1206+ kind, the file and line, then the line itself in a box with the match marked.
1207+
1208+ A thread row in the mockup is `johnbot 44 · does this work behind a socks proxy?`
1209+ on one line, with the excerpt underneath in muted text. A repository row is
1210+ `john / johnbot` and its description underneath. Neither has a box.
1211+
1212+ Forge gave every row a box, filled with `Result.Text`. For a thread that put the
1213+ title in a monospace code box. For a repository it put `rock / forge` in a box
1214+ directly under the link that already said `rock / forge`.
1215+
1216+ The box is now the code row's alone. A thread carries its title beside its number
1217+ where the mockup puts it, and a repository carries only its description. The query
1218+ is marked in the heading line as well, since that is now where a thread title
1219+ lives.
1220+
1221+ `TestOnlyACodeSearchResultDrawsABox` renders one of each kind and counts the
1222+ boxes. A `want` list of strings cannot say "and not this".
1223+
1224+ ## The rule about comments was not being checked where comments also live
1225+
1226+ Every comment in this tree is one line. I had been checking `.go` and `.css` with
1227+ an awk one liner and had never looked at `.js`, and the one script in the product
1228+ opened with a two line block.
1229+
1230+ `TestEveryCommentIsOneLine` walks the tree and fails on any run of consecutive
1231+ `//` lines in a `.go`, `.js` or `.css` file, and on a `/* */` that does not close
1232+ on the line it opened. It was falsified before it was kept: a temporary file with
1233+ a two line comment fails it by name and line.
1234+
1235+ ## Search has no index, and chapter 17 opens by asking for one
1236+
1237+ Recorded rather than fixed, because it is the largest thing left and half of it
1238+ would be worse than none.
1239+
1240+ Chapter 17: "One index, one result set". "Index on push, incrementally, from the
1241+ pushed range rather than a full rescan. Index thread comments on note write."
1242+ Appendix E lists `forge doctor --reindex` to rebuild it.
1243+
1244+ There is no index. `readable` opens every repository on the server for every
1245+ query, and `search.Search` then runs `git grep` in each one, plus a thread read.
1246+ Measured on the development server with 29 repositories: **64ms**, against
1247+ chapter 25's ten milliseconds for a page with no diff. It is O(repositories) in
1248+ processes, so it gets worse with exactly the growth a forge wants.
1249+
1250+ What is already right, and must stay right when the index arrives: the read
1251+ filter is applied **before** the search, not after. `readable` builds the target
1252+ set from what the asker may open, so a private match is never ranked and then
1253+ dropped. Chapter 17 calls filtering after ranking a leak of the count of private
1254+ matches, and BUILD.md calls it the highest severity mistake available here.
1255+
1256+ The constraint that decides the design: BUILD.md requires a schema both SQLite
1257+ and PostgreSQL accept, so FTS5 and tsvector are both out. One document table with
1258+ a repository column, filtered in the query, is portable and is one query instead
1259+ of N processes.
1260+
1261+ ## An empty package
1262+
1263+ `internal/web` was an empty directory imported by nothing. Deleted.
1264+
1265+ ## Search has an index now, and the first thing it indexed was the trash
1266+
1267+ Chapter 17 opens with "one index, one result set". Forge had no index. Every
1268+ query opened every repository on the server and ran `git grep` in each one, plus
1269+ a thread read. Measured on the development server with 29 repositories: 64ms,
1270+ against chapter 25's ten for a page with no diff, and O(repositories) in
1271+ processes, so it got worse with exactly the growth a forge wants.
1272+
1273+ **The read filter is the whole design.** Chapter 17 says filter in the query, and
1274+ BUILD.md calls filtering after ranking the highest severity mistake available
1275+ here. So every document carries `public` and a `readers` column holding the
1276+ owner and `[access] push` as `|john|lisa|`, and the query is
1277+
1278+ WHERE (LOWER(body) LIKE ? OR LOWER(title) LIKE ?)
1279+ AND (public = 1 OR readers LIKE ?)
1280+
1281+ The pipes matter: `|john|` never matches inside `|johnson|`. Chapter 18 makes read
1282+ binary, public or the owner or `[access] push`, so the whole rule fits in two
1283+ columns and needs no join.
1284+
1285+ That test was falsified before it was kept. Changing `public = 1` to `1 = 1`
1286+ makes it fail by name, for both an anonymous reader and a signed in stranger.
1287+
1288+ **Why not FTS5 or tsvector.** BUILD.md requires a schema both SQLite and
1289+ PostgreSQL accept, so neither becomes the only one actually tested. Neither full
1290+ text extension is portable. One document table with a LIKE scan is, and a scan of
1291+ one table beats N processes by a wide margin, which is the whole problem being
1292+ solved.
1293+
1294+ **What is indexed.** One row per file at the tip of the default branch, one per
1295+ thread with its comments, one per repository for its name and description. Blobs
1296+ over 512kb and anything holding a NUL byte are skipped: chapter 17 indexes source,
1297+ and one generated file should not become the index.
1298+
1299+ **Incremental, as the chapter asks.** A push takes `git diff --name-only old..new`
1300+ and updates only those paths, deleting the rows for paths the push removed. A
1301+ first push, or a push whose old side is unknown, walks the tree once. Code lives
1302+ at the tip of the default branch, so no other ref changes what a code search
1303+ finds, but every push rewrites the read set and the discussion, because
1304+ visibility arrives in the tree and a proposal push carries notes.
1305+
1306+ **A comment written on the web is a note write and not a push**, so the three
1307+ places httpd writes a note reindex the discussion.
1308+
1309+ **`forge doctor --reindex`** is appendix E's line, and it empties the index first.
1310+ A rebuild that only adds cannot remove a repository that has gone.
1311+
1312+ **Which is how the bug was found.** The first reindex on the development server
1313+ said "indexed 7 repositories" and one of them was
1314+ `trash/1787104067-mark-renamed`. Chapter 44.4 keeps a deleted repository for 30
1315+ days, and the walk was matching every `*.git` directory under the repository root,
1316+ including the ones waiting to be erased. A deleted private repository would have
1317+ had its contents searchable under an account named `trash`.
1318+
1319+ The delete path already dropped its documents, so this was reindex alone. The
1320+ walk skips the trash directory now and the test that proves it was falsified
1321+ first: without the skip it fails with the actual leaked link,
1322+ `trash / 1787130049-john-johnbot / retry.go:3`.
1323+
1324+ **64ms to 7.8ms on the development server.** In the budget, over a thousand
1325+ indexed files, fifty threads and a hundred tags:
1326+
1327+ search 900µs 1kb (budget 10ms, 15kb)
1328+
1329+ Every row of chapter 25 still passes.
1330+
1331+ ## Giving a repository away left the old owner able to search it
1332+
1333+ A transfer moves the directory, the database row and the index rows. It did not
1334+ move the read set. Chapter 18 makes read binary and the owner is half of it, so
1335+ the `readers` column still said `|john|` after john gave the repository to lisa.
1336+
1337+ Two things followed, and both are wrong in opposite directions. Lisa could not
1338+ search the private repository she now owned, which is exactly the failure chapter
1339+ 17 names when it refuses to solve access by indexing only public work. And john
1340+ could still find its contents, in a repository he no longer owned.
1341+
1342+ A rename does not have the problem, because a rename does not change the owner.
1343+ The read set is rewritten from the tree after any move, since a transfer is the
1344+ one change to who may read that no push announces.
1345+
1346+ The test fails on both halves without the fix.
1347+
1348+ ## A query is a string, and LIKE thinks some of it is syntax
1349+
1350+ `%` and `_` are LIKE's wildcards. A search for `100%` was reaching the database as
1351+ `%100%%`, and a search for `read_all` matched `readXall`.
1352+
1353+ They are escaped now, with `ESCAPE '\'`, which both dialects read the same way.
1354+
1355+ The first version of this test did not actually test it. `%100%%` still needs the
1356+ literal `100`, so it matched the one document it should have matched and the test
1357+ passed with the escaping removed. It asks for `c%e` now, a string that appears in
1358+ neither document and which unescaped reaches both through the word "coverage",
1359+ and for `read_all` against a document holding `readXall`. Both halves fail
1360+ without the escaping.
1361+
1362+ Worth writing down as a rule rather than an incident: a test that passes when the
1363+ thing it tests is deleted is not a test. Take the code out and watch it go red.
1364+
1365+ ## One row per file hid the rest of the matches
1366+
1367+ `git grep` returned every matching line, so a symbol used four times in a file was
1368+ four rows. The index holds one row per file, and the first version of the query
1369+ turned that into one row per file on the page as well.
1370+
1371+ That is a loss for the one thing chapter 17 says a forge is usually searched for.
1372+ A file now contributes up to three lines, each with its own number and link, and
1373+ the whole set is still bounded by the page's limit, so a common word cannot fill
1374+ the page from one file.
1375+
1376+ ## The tab strip carried a count that only one page filled in
1377+
1378+ Every mockup draws the tab row as `log · files · threads 3 · runs · config`. The
1379+ number is the point of putting it there: a reader learns there is discussion
1380+ without spending a page load to find out.
1381+
1382+ `openRepoFor` builds that row for every repository page and passed `0`. The
1383+ threads page overwrote it afterwards with the real number, so the count appeared
1384+ on exactly the one page where a reader already had the list in front of them.
1385+
1386+ It is read once in `openRepoFor` now. `thread.OpenCount` goes through the same
1387+ cached row list the threads page uses, so with fifty threads the cost is the file
1388+ tree at 0.5ms to 2ms and the log at 12.9 to 14.5, both well inside chapter 25.
1389+
1390+ The thread page's footer had the same gap. `thread.html` in the mockups says
1391+ `threads · 3 open` and the template said `threads`.
1392+
1393+ ## A profile row was missing the number chapter 24 asks it for
1394+
1395+ Chapter 24, profile: "each with language, size, default branch, and open proposal
1396+ count". `repoMeta` built the first three. The mockup line is
1397+ `go · 4.1mb · master · 3 proposals` and forge drew `go · 4.5mb · master`.
1398+
1399+ An open proposal is an open thread that carries a ref, which is what the threads
1400+ list already means by the word, so the count comes from the same place. A
1401+ repository with none says nothing rather than "0 proposals".
1402+
1403+ ## The bio in profile.html has nowhere to live, and that is correct
1404+
1405+ The mockup puts `writes bots. mostly go.` under the account name. There is no
1406+ such field and there should not be one.
1407+
1408+ Chapter 10's closed list is the complete set of what the server stores outside
1409+ git. It has six items and none of them is a profile. The chapter says outright
1410+ that the list was four items in an earlier draft and grew without being updated,
1411+ and that "a closed list that quietly grows is worse than an open one".
1412+
1413+ A repository description lives in `.barerepo/config`, in the tree, which is why that
1414+ one is drawn. An account has no tree to put a bio in. Left unbuilt on purpose.
1415+
1416+ ## A note to myself about git checkout
1417+
1418+ Falsifying the tab count test meant editing `web.go`, running the test, and
1419+ putting the line back. I put it back with `git checkout internal/httpd/web.go`,
1420+ which threw away every other uncommitted change in that file from the same pass:
1421+ the struct field, the profile count and the `repoMeta` signature. The test went
1422+ red for the wrong reason and I had to write them again from the transcript.
1423+
1424+ Falsify by reversing the exact edit, never by checking the file out.
1425+
1426+ ## The keys page named one credential kind out of four
1427+
1428+ Chapter 24 is explicit about why the closing line exists: "The page names what
1429+ each one can do, because a user about to paste a token into a feed reader
1430+ deserves to know it cannot write."
1431+
1432+ Forge said "a key signs you in and pushes. a git token clones and pushes over
1433+ https." and stopped. The two sentences it dropped are the two the chapter gives a
1434+ reason for. `keys.html` has them both: "a runner token attaches one machine to one
1435+ repository. a feed token reads one feed and can write nothing."
1436+
1437+ All four are named now. The runner sentence also says where a runner token comes
1438+ from, because forge has no `new runner token` button and the mockup does.
1439+
1440+ **That missing button is deliberate.** Chapter 15 puts the token inside the copied
1441+ command, per repository. A button on `/keys` has no repository to scope to, so it
1442+ would have to ask for one, which is the interstitial the whole chapter exists to
1443+ remove. The runners page of a repository is where the token is made.
1444+
1445+ ## A runner token said nothing about the runner
1446+
1447+ Chapter 24 asks a runner token row for "its labels, attached machine, and
1448+ last-seen time". Forge showed the scope and the last-used time.
1449+
1450+ The runners table already carries `token_id`, so the machine that attached with a
1451+ token is one join away. A row now reads
1452+ `runner · rock/forge · m2 · labels build, test · last used 2h`, which is the
1453+ mockup's `john/johnbot · labels build, test · last seen 40m` with the machine
1454+ named as well.
1455+
1456+ ## Runners were never busy and had never run anything
1457+
1458+ Chapter 24, runners: "Attached machines, platform, labels, **run count**, status,
1459+ last seen". The run count was absent and the status had two values where the
1460+ mockup has three.
1461+
1462+ Both come from the jobs table, which already carries `runner_id`, in one grouped
1463+ query: how many jobs each machine has taken, and whether any of them is running
1464+ now. A machine holding a job reads `busy`, which is what `runners.html` shows for
1465+ `lisa-mbp`, and a machine that has run nothing says nothing rather than "0 runs".
1466+
1467+ ## Pages checked this pass and found correct
1468+
1469+ The sweep is worth recording in both directions, or the next pass repeats it.
1470+
1471+ - **inbox** matches `inbox.html` completely, including the horizontal rule at
1472+ `last_visited` from chapter 19.4 and the ninety day line.
1473+ - **commit** correctly has no tab strip, because `repo-commit.html` has none.
1474+ - **compare** matches, including the ref shortcuts under the form.
1475+ - **file tree**, **runs**, **thread**, **releases**, **threads** match.
1476+
1477+ ## Adding a runner had grown the second step chapter 24 forbids
1478+
1479+ Chapter 24 on the add-a-runner page: "Three commands, one per platform, all
1480+ visible at once, each with the token already inside it." Then, in bold: "If this
1481+ page ever grows a second step, something has gone wrong."
1482+
1483+ The page showed one button, `new runner token`. You clicked it and then got the
1484+ commands. That is a second step, on the page the chapter picks out as the
1485+ clearest demonstration of the whole thesis.
1486+
1487+ The token is minted on arrival now. The reason the button existed is real: a
1488+ token is shown once, so a page that mints on every view leaves a dead token per
1489+ view. So a view first revokes this account's runner tokens for this repository
1490+ that are over an hour old and that no machine ever attached with. A reload cannot
1491+ pile them up, and cannot revoke the line the reader copied a moment ago either,
1492+ which is the case the test names.
1493+
1494+ **What I did not build: the auto-refresh.** The mockup's third fact is "this page
1495+ refreshes the moment a runner attaches", and chapter 24 lists it. It cannot be
1496+ done here without breaking something else. A meta refresh on a page that mints a
1497+ token either mints one per tick or revokes the line the reader is in the middle of
1498+ pasting. Forge's page does not claim to refresh, so nothing on it is untrue, and
1499+ the honest fix needs the page to know a runner attached without reloading, which
1500+ is polling, which is javascript this design does not want. Left out on purpose.
1501+
1502+ ## A new repository could not be given a description
1503+
1504+ Chapter 24, new repository: "Shows. Name, description, default branch, visibility,
1505+ create." The form had name, default branch and visibility.
1506+
1507+ The reason it was missing is real: chapter 11 says the server commits nothing, so
1508+ a description has nowhere to go. It lives in `.barerepo/config`, in a tree that does
1509+ not exist yet. Visibility has the same problem and was already solved, by carrying
1510+ the answer to the empty repository page and putting it in the block the reader
1511+ pastes.
1512+
1513+ The description now goes the same way.
1514+
1515+ **The paste block changes shape when there is one.** The mockup writes the config
1516+ with `printf '[repo]\nvisibility = "public"\n'`, and printf reads backslashes, and
1517+ the whole argument is inside single quotes. A description holding a quote or a
1518+ backslash would break the command the reader pastes, silently, on their machine.
1519+
1520+ So a described repository gets a quoted heredoc instead, which passes every
1521+ character through untouched:
1522+
1523+ mkdir -p .forge && cat > .barerepo/config <<'EOF'
1524+ [repo]
1525+ visibility = "public"
1526+ description = "irc bot that refuses to leave"
1527+ EOF
1528+
1529+ A repository with no description keeps the mockup's printf line exactly. The only
1530+ character a quoted heredoc cannot carry is a newline, and a form input cannot hold
1531+ one, but it is stripped anyway rather than trusted.
1532+
1533+ ## The push rejected page was never built, and it is the one that matters most
1534+
1535+ `push-rejected.html` is a mockup, chapter 24 has an entry for it, and BUILD.md
1536+ stage 4 says "this page matters more than it looks, because rejection is where new
1537+ contributors get stuck". There was no template, no route, and no url.
1538+
1539+ The wording had been done. An earlier pass made the hook print the mockup's exact
1540+ sentences so the terminal and the page could not differ. The page they were copied
1541+ from did not exist.
1542+
1543+ Chapter 24 says why it has to: "The hook already printed the reason. The page
1544+ exists because terminals scroll, and because a rejected push is where a new
1545+ contributor decides whether to keep going." And: "The hook prints a URL alongside
1546+ the rejection message. Without that line the page is unreachable, because a
1547+ rejected push happens in a terminal and no browser is involved."
1548+
1549+ **It holds no state.** A rejection is not on chapter 10's closed list and must not
1550+ be, so nothing is written down. The url carries the one fact the server cannot
1551+ recompute, which is the ref you pushed to:
1552+
1553+ http://barerepo.example/john/johnbot/rejected?ref=refs/heads/master
1554+
1555+ Everything else the page reads live: the `[access] push` line out of the tree, and
1556+ who you are out of your session. That means it also stays correct later. Sign in,
1557+ or get added to the list, and the page stops telling you to open a proposal and
1558+ says you may push now.
1559+
1560+ The mockup's `e91b7d · 2m ago` line is not drawn. A time carried in a url is a
1561+ number the reader supplied to themselves, and they already know when they pushed.
1562+
1563+ `repocfg.List` and `repocfg.Who` were moved out of the hook so both the terminal
1564+ and the page render the config line from one function. That was the point of
1565+ copying the wording in the first place.
1566+
1567+ ## A run's footer named the wrong ref, and a rev that is not here drew a page
1568+
1569+ `run.html` puts the triggering ref in the footer, `refs/proposals/46`. Forge put
1570+ the default branch there, which for a proposal build is the one ref the run had
1571+ nothing to do with.
1572+
1573+ Separately, `/john/johnbot/run/8` answered 200 with "nothing has been built at
1574+ this commit". `8` is a valid rev *name* and not a commit in the repository, so the
1575+ page was drawn for something that does not exist. A rev that does not resolve is a
1576+ 404 now. A real commit with no runs still gets the page, because that is a true
1577+ state and a reader may have come looking for it.
1578+
1579+ ## A collapsed diff had no expand link, only a way off the page
1580+
1581+ Chapter 24 on the repository log: "Diffs over a threshold collapse with a size
1582+ label and an expand link." The size label was there. The link said `view commit`
1583+ and went to the commit page.
1584+
1585+ `repo-log.html` reads `3d · large diff collapsed · expand`. Forge read
1586+ `14 files · +302 -288 · view commit`. Both the word and the destination were
1587+ wrong: expand means show it here, and the sha at the front of the row is already
1588+ the link to the commit page.
1589+
1590+ The link is `?expand=<sha>#<short>` now. It reopens the log with that one commit
1591+ open and jumps to it, keeps `?from=` so a reader on the second page stays there,
1592+ and every other row stays collapsed, which is the point of collapsing.
1593+
1594+ The diff itself comes from the cache the log already wrote. **The first version
1595+ read only the cache**, which meant the link silently did nothing whenever the
1596+ cache was cold. The end to end test caught it, because the test harness does not
1597+ set the caches. It falls back to reading the one commit now.
1598+
1599+ ## The releases page crossed its budget again, and the fix was one commit hash
1600+
1601+ Adding the open thread count to every repository page cost about 1.5ms, which put
1602+ releases at 10.8ms against 10. Measured rather than guessed:
1603+
1604+ Releases 2.7ms ReadNotes 4.4ms OpenCount 1.4ms
1605+
1606+ `ReadNotes` was walking the notes tree through the object pool on every view,
1607+ reading about forty objects to draw twenty bodies. Notes live under one ref, and
1608+ that ref is a commit, and a commit is immutable. So the whole object-to-body map
1609+ is read once and cached under the notes commit.
1610+
1611+ The cold pass costs more, because it now reads every note rather than the twenty
1612+ on screen: 11.5ms once, then 1.4ms for every view until somebody pushes a note.
1613+
1614+ Releases 2.6ms ReadNotes 1.4ms OpenCount 1.4ms
1615+
1616+ The releases row is 5.9ms. Every row of chapter 25 passes.
1617+
1618+ The tree walk also lost the filter that kept it to wanted notes, and gained the
1619+ rule it should have had from the start: a tree entry whose accumulated path is a
1620+ full object id is a note, and a shorter one is a fanout directory.
1621+
1622+ ## Every page now has to render the same with no cache at all
1623+
1624+ Last pass the expand link silently did nothing on a cold cache, and the only
1625+ reason that was caught is that the end to end harness happened not to set the
1626+ caches. That is luck, not a test.
1627+
1628+ So the harness sets them now, and every existing test runs the warm path, which
1629+ is the one production takes and the only one where a wrong cached answer can
1630+ appear. Then one test takes them away and fetches thirteen pages twice: the
1631+ profile, the log, the log with a diff expanded, the file tree at two paths, a
1632+ file, a commit, a compare, the threads list, runs, releases, config and a search.
1633+ Every pair must render identically, relative times aside.
1634+
1635+ **An empty cache is not a cold cache.** The first version deleted the cache
1636+ directory, which proved nothing: `cache.Disk` keeps a hot map in memory, and even
1637+ without it the log writes every diff it reads before anything asks for one back.
1638+ The honest test is a cache that retains nothing, which is what a full disk is, so
1639+ the second pass sets them to nil.
1640+
1641+ **And a test needs something to find.** The second version still passed with the
1642+ fix removed, because the repository had two small commits and nothing to collapse,
1643+ so the expand url changed nothing either way. The repository has a seven file
1644+ commit now. With the fallback taken out the test fails by name and by url.
1645+
1646+ That is three versions of one test, two of which proved nothing. Writing the test
1647+ is the easy half.
1648+
1649+ The invariant it holds is the one chapter 10 item 6 states: caches are derived,
1650+ discardable, and rebuildable. If deleting them changes what a reader sees, one of
1651+ them is not a cache.
1652+
1653+ ## Four of the five limits were settings that did nothing
1654+
1655+ `[limits]` in the server config has five keys. One of them, `signup_per_hour_per_ip`,
1656+ is enforced. The other four are values an operator can set and forge never reads:
1657+ `max_blob_mb`, `max_push_mb`, `max_open_proposals`, `artifact_retain_days`.
1658+
1659+ That is worse than not having them. A config key the file accepts and the server
1660+ ignores is a promise the operator has no way to check.
1661+
1662+ **`max_blob_mb` is enforced now**, because chapter 20.2 is the one that cannot
1663+ wait: "Turning LFS off is not enough on its own. Without a limit, a user commits a
1664+ 4 GB video directly into git. That is worse than LFS, because it is in the history
1665+ permanently and every clone pays for it forever."
1666+
1667+ The check runs in pre-receive over the pushed range and not the whole repository,
1668+ which inside a hook is exactly `rev-list --objects <new> --not --all`, since the
1669+ ref has not moved yet so `--all` still holds the old tips. Two processes, and only
1670+ when a limit is set. The ids go through one `cat-file --batch-check`.
1671+
1672+ `rev-list --objects` prints the path beside each blob, which is the whole point:
1673+ chapter 20.2 says "The message must name the file and its size. A rejection that
1674+ says only 'push too large' sends the user hunting." So the rejection reads
1675+
1676+ demo.mov is 2.1mb. the limit is 1mb.
1677+
1678+ large files belong in object storage, with a url or a checksum in the
1679+ repository. the build fetches them.
1680+
1681+ The second half is chapter 20.4, which says to put it in the rejection rather than
1682+ leave the user with a refusal and no direction. Blobs over the limit are sorted
1683+ largest first, and a push with several says how many, so a reader fixes the worst
1684+ one first instead of pushing five more times.
1685+
1686+ The test pushes a two megabyte file against a one megabyte limit and asserts the
1687+ name, the limit, the object storage line, and that the small file in the same
1688+ commit is not blamed. Removing the check fails it.
1689+
1690+ **Still unenforced, and recorded rather than half done:** `max_push_mb`,
1691+ `max_open_proposals` and `artifact_retain_days`. `max_open_proposals` is BUILD.md's
1692+ "rate limit proposal refs per key per repo, rule 5 is an open door", and it belongs
1693+ with the other trap on the same list, expiring unreferenced proposal refs.
1694+
1695+ ## The open door had no doorstop
1696+
1697+ Rule 5 says anyone authenticated can propose. Chapter 27 opens with "Rule 5 is an
1698+ open door and must be defended without closing it", and names the defense: "Cap
1699+ open proposals per account per repository at a small number, ten is plenty."
1700+
1701+ Appendix D's pre-receive pseudocode has the check on the line after the one forge
1702+ already had:
1703+
1704+ if not may_propose(user, config): reject("...")
1705+ if open_proposals(user, repo) >= limits.max_open_proposals: reject("...")
1706+
1707+ Forge had the first and not the second. `max_open_proposals` was one of the four
1708+ `[limits]` keys nothing read.
1709+
1710+ An open proposal is an open thread that carries a ref, which is what the threads
1711+ list already means by the word, so the count comes from the same place. Three
1712+ things the test pins down, because each is a way to get this wrong:
1713+
1714+ - **Per account.** Mark at the cap does not stop john proposing.
1715+ - **Only on opening.** A force-push to `refs/proposals/2` is revising something
1716+ already open, not opening another, and chapter 12 makes that the normal way to
1717+ update a proposal. Capping it would break the mechanism it is protecting.
1718+ - **The message says what to do.** "you have 2 proposals open on john/johnbot. the
1719+ limit is 2. land or close one, then push this again."
1720+
1721+ Removing the check fails the test.
1722+
1723+ ## Still open on the same list
1724+
1725+ Chapter 26 names two more, both about a repository growing without bound, and both
1726+ are config keys that already exist and do nothing:
1727+
1728+ - **Proposal refs.** "Expire proposals with no activity for `[proposals]
1729+ expire_days`, default 180. Delete the ref, retain the thread. The thread is
1730+ small; the ref pins commits." `repocfg.Proposals.ExpireDays` is parsed and
1731+ defaulted and never read.
1732+ - **Proposal revisions.** "Keep the most recent five and the ones with anchored
1733+ comments. Delete the rest on the same expiry schedule."
1734+
1735+ They belong together, in `Server.Sweep`, which already runs on a schedule and
1736+ already drops expired tokens and old events. The anchored comment rule is the
1737+ part that needs care: chapter 43.4 keeps a comment's line visible after the code
1738+ moves, and it does that through the blob hash recorded beside it, so a revision
1739+ holding one of those blobs cannot be deleted.
1740+
1741+ ## A proposal ref pins commits forever, and expire_days did nothing
1742+
1743+ Chapter 26 names three places a repository grows that other forges do not have.
1744+ The first: "Proposal refs. Anyone may create them, so they accumulate. Expire
1745+ proposals with no activity for `[proposals] expire_days`, default 180. Delete the
1746+ ref, retain the thread. The thread is small; the ref pins commits."
1747+
1748+ `repocfg.Proposals.ExpireDays` was parsed, defaulted to 180, and never read.
1749+
1750+ The sweep already runs on a schedule and already drops expired tokens, old events
1751+ and trash past its window, so this went beside them. Per repository, per proposal
1752+ ref: the thread's own last activity decides, since a thread and its proposal are
1753+ one object and the thread is where activity lands. A ref with no thread behind it
1754+ is dated by the commit it points at, read through the object pool rather than a
1755+ process.
1756+
1757+ **A window of zero is expiry switched off, not expiry of everything.** That is
1758+ the kind of default that deletes a whole forge on a config typo, so the test says
1759+ it out loud.
1760+
1761+ **The commits are not gone**, and the test asserts that too. `update-ref -d`
1762+ unpins them and `git gc` collects them later, which is the same order chapter 26
1763+ puts them in: "Run `git gc` per repository on a schedule, not on push. Repack
1764+ after bulk ref deletion, or the pack files retain everything you just deleted."
1765+
1766+ `repo.Walk` came out of this. The reindex command had its own copy of the walk
1767+ that skips the trash directory, and that copy is where the bug two passes ago
1768+ lived, so there is one of them now and both callers use it.
1769+
1770+ **Still open, the other two thirds of chapter 26.** Revisions retain the old tip
1771+ on every force-push under `refs/revisions/<n>/<k>`, and the rule is to keep the
1772+ most recent five and the ones with anchored comments. The retention half is easy;
1773+ the anchored half is not, because chapter 43.4 keeps a comment's line visible
1774+ through the blob hash recorded beside it, so a revision that holds the only copy
1775+ of an anchored blob cannot be deleted without breaking a comment that is still on
1776+ the page. `git gc` per repository on a schedule is also not run.
1777+
1778+ ## The field that says which revision a comment belongs to was never filled in
1779+
1780+ Chapter 26's second growth point: "Proposal revisions. Each force-push retains the
1781+ old tip under `refs/revisions/<n>/<k>`. Keep the most recent five and the ones with
1782+ anchored comments."
1783+
1784+ The first half is arithmetic. The second half needs to know which revision a
1785+ comment is anchored to, and `thread.Comment` has had a `Revision` field, written
1786+ into the note format and parsed back out of it, since threads were built. Nothing
1787+ ever set it. Every comment in every repository says revision 0.
1788+
1789+ So the retention rule had no input, which is presumably why the pruning was never
1790+ written.
1791+
1792+ **A comment now records the revision it was written against.** That number is the
1793+ one the content on screen will take when the next force-push retains it, which is
1794+ the count of existing revision refs plus one. `proposal.CurrentRevision` reads it
1795+ from the ref files, so a comment costs no process to number.
1796+
1797+ `nextRevision` in the hook was forking `for-each-ref` for the same count. It calls
1798+ `CurrentRevision` too now, so a proposal update starts one process fewer.
1799+
1800+ **A comment that will not say pins everything.** Every comment written before this
1801+ pass says revision 0, and chapter 43.4 keeps a comment's line visible through the
1802+ blob recorded beside it, so deleting the revision that holds that blob breaks a
1803+ comment still on the page. When an anchored comment cannot name its revision,
1804+ nothing is pruned for that proposal at all. It is the conservative answer and it
1805+ un-sticks itself as comments are written.
1806+
1807+ **An expired proposal keeps none.** The sweep prunes with a keep of five normally
1808+ and zero for a proposal whose ref it just expired, because the reason the ref went
1809+ is the reason the revisions should go with it. Anchored comments still hold what
1810+ they need, since the thread outlives both.
1811+
1812+ Eight revisions, five newest kept, one comment anchored to revision two: one and
1813+ three are deleted and two survives. Removing the anchored check prunes it and the
1814+ test says so by number.
1815+
1816+ ## The last third of chapter 26, and the sentence that made it urgent
1817+
1818+ "Run `git gc` per repository on a schedule, not on push. Repack after bulk ref
1819+ deletion, or the pack files retain everything you just deleted."
1820+
1821+ The second sentence is about what the last two passes built. Expiring a proposal
1822+ ref and pruning retained revisions frees nothing on their own: the objects stay in
1823+ the pack, so a repository that grew without bound still grows without bound, only
1824+ with a shorter ref list.
1825+
1826+ The sweep collects each repository now, and which form it runs is decided by
1827+ whether it deleted anything there:
1828+
1829+ - **It deleted refs**: a full `git gc`, which repacks. That is chapter 26's
1830+ "repack after bulk ref deletion", run the moment the deletion happens rather
1831+ than left to a threshold that may never trip.
1832+ - **It deleted nothing**: `git gc --auto`, which is git's own scheduled form. It
1833+ costs one process and returns immediately unless git thinks there is work.
1834+
1835+ **No `--prune=now`, deliberately.** Git's default two week grace exists because a
1836+ push in flight writes objects before it writes the ref that reaches them, and
1837+ pruning aggressively deletes them out from under it. So a proposal ref expired an
1838+ hour ago is unpinned now and collected a fortnight later, which is the same order
1839+ chapter 26 states and the reason the expiry test asserts the commit is still
1840+ readable straight afterwards.
1841+
1842+ The test packs a loose repository, checks a pack file appears where there was
1843+ none, and checks the commit a ref still reaches survived it. A repack that loses
1844+ reachable objects is the one way this can be badly wrong, so it is asserted rather
1845+ than assumed. Taking the gc out fails it.
1846+
1847+ **Chapter 26 is complete.** All three growth points it names are handled: proposal
1848+ refs expire, revisions are pruned to five plus the anchored ones, and note trees
1849+ are left alone on purpose, because 26 says the durability of discussion is the
1850+ product.
1851+
1852+ ## The whole push has a limit too, and it costs nothing extra to check
1853+
1854+ `max_push_mb` was the third of the four `[limits]` keys nothing read. Chapter
1855+ 20.2 sets it beside `max_blob_mb`, and `config.go` already carried the reason it
1856+ is loose: "the push most likely to hit it is somebody's first import."
1857+
1858+ Both limits ask about the same objects, so they are one pass of git. The pre
1859+ receive check reads the pushed range once, sums every new object, and picks the
1860+ largest blobs out of the same output. Two processes for both limits, and none at
1861+ all when neither is set.
1862+
1863+ The two rejections are deliberately different, because the fixes are:
1864+
1865+ demo.mov is 2.1mb. the limit is 1mb.
1866+ large files belong in object storage...
1867+
1868+ this push adds 1.4mb of objects. the limit is 1mb.
1869+ push it in parts, or ask whoever runs this forge to raise [limits] max_push_mb.
1870+
1871+ One is the user's mistake and chapter 20.4 says where the file belongs. The other
1872+ may be a perfectly good repository meeting a policy, so it names the setting.
1873+
1874+ **It says "adds ... of objects", not "is".** The number is the uncompressed size
1875+ of what the push adds, which is what forge can measure at pre-receive and what
1876+ decides how much disk it takes. Saying "this push is 1.4mb" of a transfer that
1877+ was four hundred kilobytes on the wire would be a number that does not match
1878+ anything the user can see.
1879+
1880+ The test pushes three files, each under the blob limit and together over the push
1881+ limit, and asserts the rejection does not name a file, because naming one would
1882+ mean the wrong limit fired.
1883+
1884+ ## What is left of the four
1885+
1886+ `artifact_retain_days` is the last one, and it cannot be enforced because there
1887+ is nothing to retain. `[paths] artifacts` is configured and the directory is
1888+ created, and no code writes to it or reads from it. Chapter 22 is unbuilt: a
1889+ release has a body in `refs/notes/releases`, which works, and attached files,
1890+ which do not exist.
1891+
1892+ That is the honest state. Chapter 22.4's rule is already written down for when
1893+ they do: build artifacts expire, release artifacts do not, because a published
1894+ download that disappears breaks other people's installers.
1895+
1896+ ## Release artifacts exist now, and chapter 22 is built
1897+
1898+ `[paths] artifacts` was configured, the directory was created at init, and no code
1899+ ever wrote to it or read from it. A release had a body and no files.
1900+
1901+ **The upload is chapter 22.5 exactly.** "A run can attach its output to a release.
1902+ The job token from chapter 15 carries the permission, scoped to one repository and
1903+ one job." `POST /runner/artifact` takes that token, and it is the job's own token
1904+ and not the runner's long-lived one, which is the difference that makes "one job"
1905+ true. The poll already issued it and labelled it `job <id>`; that label is what
1906+ binds the token to the job, and the test proves a plain git token for the same
1907+ repository is refused.
1908+
1909+ The tag has to be a tag in the repository. Without that check an upload creates a
1910+ directory nobody can ever reach, which is a disk leak with no page to show it.
1911+
1912+ **The build gets what it needs to speak the protocol.** `BAREREPO_URL`, `BAREREPO_REPO`,
1913+ `BAREREPO_JOB` and `BAREREPO_JOB_TOKEN` are in the environment of `[build] command`, so a
1914+ build attaches a file with curl and forge invents no new syntax to describe
1915+ artifacts. The runner setup page lists the endpoint beside the other four and
1916+ shows the command, per chapter 24's rule that the page says what the protocol is
1917+ so anyone can write their own runner.
1918+
1919+ **The write is a rename.** A half finished upload is a dotfile ending in `.part`,
1920+ and `List` skips it, so a reader never sees a truncated binary. A rerun replaces a
1921+ file rather than appending to a list.
1922+
1923+ **The download is an attachment and never a page.** `application/octet-stream`,
1924+ `Content-Disposition: attachment`, `nosniff` and a sandbox policy, which is what
1925+ chapter 42.3 asks of any bytes a stranger uploaded. Read access is checked, so a
1926+ private repository's binaries are not public.
1927+
1928+ **A deleted repository takes its files.** They are not in git, so nothing else
1929+ would have.
1930+
1931+ ## artifact_retain_days stays a setting that does nothing, correctly
1932+
1933+ Chapter 22.4: "Build artifacts expire. Release artifacts do not, because a
1934+ published download that disappears breaks other people's installers."
1935+
1936+ Everything built here is a release artifact, so nothing expires and the key has
1937+ nothing to act on. Build artifacts, the kind that expire, are files a run keeps
1938+ without attaching them to a release, and no mockup shows them: chapter 24's run
1939+ detail entry is "status, exit code, what triggered it, and the complete log as
1940+ plain text", with no artifact row. Inventing that surface to give the key a job
1941+ would be building a page the plans do not describe.
1942+
1943+ So it stays unused on purpose, and this is the note that says why rather than
1944+ leaving the next reader to find a dead key and guess.
1945+
1946+ ## The same bug as last time, in the new feature
1947+
1948+ Auditing the artifact work found the mistake the search index made two passes ago,
1949+ in the same shape: **what changes this state without going through the path I
1950+ built?**
1951+
1952+ Attached files live at `<artifacts>/<owner>/<name>/<tag>/`. A rename changes the
1953+ name and a transfer changes the owner, and `serveRepoMove` moved the directory,
1954+ the database row and the search index, and left every attached file behind. The
1955+ releases page would show none, and the bytes would sit on disk with no page to
1956+ reach them and no delete to collect them, because a delete only removes the path
1957+ the repository has now.
1958+
1959+ `artifact.Move` runs beside `MoveDocs` now. The test renames, checks the file is
1960+ still listed and still downloads, then transfers to another account and checks
1961+ again, because the two halves of the path move separately.
1962+
1963+ **A copy does not take them, and that is right.** Chapter 21.1 lists what copying
1964+ carries: branches, tags, all history, threads and notes. Attached binaries are not
1965+ on that list, and 22.3 already says a mirror does not take them either.
1966+
1967+ ## Two uploads of one name could write into each other
1968+
1969+ `Put` wrote to `.<name>.part` and renamed. Two jobs attaching the same file name
1970+ to the same tag at the same time share that path, so one truncates the other and
1971+ the rename publishes a mixture. `os.CreateTemp` gives each upload its own part
1972+ file now.
1973+
1974+ ## The releases page did twenty directory reads to draw nothing
1975+
1976+ Listing files per release meant a `ReadDir` per row, twenty of them, on
1977+ directories that do not exist for a repository with nothing attached, which is
1978+ almost all of them. The row sat between 5.9 and 12.2ms against a 10ms budget and
1979+ crossed depending on what else the machine was doing.
1980+
1981+ The whole blob store for a repository is read in one pass now: one `ReadDir` of
1982+ the repository's directory, and one more only for a tag that actually has files.
1983+ A repository with nothing attached costs one failed stat.
1984+
1985+ That row has been near its limit for several passes and every small addition
1986+ tipped it. This is the difference between nudging it under and giving it room.
1987+
1988+ ## A rename detached every runner, and six other things
1989+
1990+ The checklist from the last pass turned into a test, and the test found six more
1991+ holes in the same wall.
1992+
1993+ `DB.Move` updated two tables, `repos` and `redirects`. Seven others key on the
1994+ repository path and none of them moved:
1995+
1996+ - **tokens.scope.** A runner token is scoped to `john/johnbot`. After a rename the
1997+ scope still says the old path and every job is queued under the new one, so the
1998+ check in `jobRunner` never matches again. The machine polls forever and builds
1999+ nothing, and the runners page is empty. Renaming a repository silently detached
2000+ every runner attached to it.
2001+ - **runners.repo**, so the page could not list them either.
2002+ - **jobs.repo**, so anything already queued was orphaned.
2003+ - **events.repo** and **participation.repo**, so a transfer left the inbox
2004+ entries with the old owner and gave the new one nothing.
2005+ - **webhooks.repo**, so a failing hook's count reset to zero and the config page
2006+ said it had never delivered.
2007+ - **search_docs.repo**, which was moved separately by the handler, one more place
2008+ to forget.
2009+
2010+ All seven move inside the same transaction now, and the handler's separate call
2011+ is gone. The test writes one row into every table, renames and transfers in one
2012+ move, and asserts each one followed. Before the fix it fails seven times.
2013+
2014+ ## The budget was measuring a repository forge does not keep
2015+
2016+ The releases row had been flaking between 5.9ms and 12.2ms against its ten, and
2017+ the last pass's fix was not the reason it passed.
2018+
2019+ The cause was reading a hundred loose tag ref files. Under any disk load that
2020+ doubles. The search row, one SQLite query, barely moved in the same runs, which
2021+ is what said it was I/O and not the machine.
2022+
2023+ Forge gcs every repository in the sweep now, and gc packs refs. So a repository
2024+ forge has been hosting for an hour reads its refs out of one file, and the
2025+ benchmark was measuring one that had never been swept.
2026+
2027+ The test packs refs before measuring, because that is the state forge maintains,
2028+ and the whole table changed:
2029+
2030+ file tree 0.6ms was 1.9
2031+ threads list 1.0ms was 3.5
2032+ one thread 0.7ms was 1.9
2033+ releases 0.9ms was 5.9
2034+
2035+ Nothing is near its limit now. The honest caveat: a repository between a large tag
2036+ push and the next sweep does have loose refs and is slower, for up to an hour.
2037+ That is a real state and it is bounded by the sweep, which is the reason the sweep
2038+ exists.
2039+
2040+ Worth noticing that garbage collection turned out to be load bearing for page
2041+ speed and not only for disk.
2042+
2043+ ## The delete had the same six holes as the rename
2044+
2045+ The move test made the shape obvious, so the same test was written for the other
2046+ end of a repository's life. `DB.Forget` dropped one row, the ownership row in
2047+ `repos`, and left six tables pointing at a repository that no longer exists.
2048+
2049+ What that looks like to a user:
2050+
2051+ - **The keys page lists a runner token for a repository that is gone.** Its detail
2052+ line names `john/johnbot`, which 404s. There is no way to tell from the page
2053+ that the token is now worthless.
2054+ - **A machine stays attached** to a repository with no page, polling forever.
2055+ - **A queued job** waits for a build that can never run.
2056+ - **The inbox keeps its lines**, each linking to a 404, which is the one thing
2057+ chapter 24's own rule about names says not to do.
2058+ - **A failing webhook's counter** survives, so a repository created later with the
2059+ same name inherits somebody else's failure count. That one is not just untidy,
2060+ it is wrong.
2061+ - **Search still finds it.** The handler was dropping the index separately, so this
2062+ one was covered by accident rather than by the store.
2063+
2064+ All seven deletes are one transaction in `Forget` now, and the handler's separate
2065+ index call is gone, the same consolidation the move got. `ForgetDocs` and
2066+ `MoveDocs` are both deleted: a caller that has to remember a second call is a
2067+ caller that will forget it.
2068+
2069+ **A delete is safe to be this total because there is no restore.** Chapter 44.4
2070+ keeps the git data in trash for thirty days, and that is the recovery path. None
2071+ of these rows are recoverable state: a token is a secret nobody can read back, a
2072+ runner reattaches with one line, a queued job reruns on the next push.
2073+
2074+ The test is the delete half of the move test, sharing the fixture that fills every
2075+ table. Both fail loudly when the code is taken out.
2076+
2077+ ## The old name was freed by the one door that does not go through the transport
2078+
2079+ Chapter 21.2 states it in bold: "**The old name is never freed.** This contradicts
2080+ the instinct to recycle unused names, and it is deliberate."
2081+
2082+ A push to a renamed repository already followed the redirect, because the
2083+ transport looks the redirect up before it considers creating anything. The form at
2084+ `/new` does not go through the transport. It called `repo.Create` directly, and
2085+ `repo.Create` only asks whether a directory is there. The directory moved, so the
2086+ name looked free.
2087+
2088+ Take `john/johnbot` to `john/ircbot`, then make `john/johnbot` again on the form,
2089+ and there are now two truths: a real repository at the old path, and a redirect
2090+ row saying that path is somewhere else. Everything that points at the old name is
2091+ wrong, and chapter 21.2's reason for the rule is the worse half of it, that the
2092+ new repository inherits every mention of the old one.
2093+
2094+ **And `repo.InTrash` had never been called.** Its own comment says "reports a name
2095+ still in its window, where a push fails rather than creates. Appendix D." Nothing
2096+ called it, from either door. So a repository deleted a minute ago handed its name
2097+ straight back out while thirty days of its data sat in the trash under that name.
2098+
2099+ `transport.Claimed` answers both questions in one place and says which it is:
2100+
2101+ john/johnbot is now john/ircbot. the old name is kept forever, so nothing
2102+ that points at it breaks.
2103+
2104+ john/johnbot was deleted. its name is held for 30 days, then it is free.
2105+
2106+ Both doors ask it now, `mayCreate` for a push and `serveNewRepo` for the form.
2107+ The test takes both doors for both cases, and fails on both without it.
2108+
2109+ **The shape, again.** Two ways in, one of them checked. It is the same mistake as
2110+ the delete that dropped one row of seven and the rename that moved two tables of
2111+ nine. The question that keeps finding it: what is the other way this happens?
2112+
2113+ ## A reply pushed from a clone told nobody
2114+
2115+ The two doors again, and this time the one that was silent is the one the whole
2116+ design is about.
2117+
2118+ Chapter 3's claim is that discussion is git notes in your clone. Chapter 19.2 says
2119+ participation is subscription: "Anything in a thread you opened or replied to.
2120+ There is no watch button and no subscribe button."
2121+
2122+ `recordPushes` skipped `refs/notes/` entirely. So a reply written on the web
2123+ recorded `thread.replied` and subscribed its author, and the same reply pushed
2124+ from a clone recorded nothing and subscribed nobody. The owner of the repository
2125+ never heard it. The person who wrote it never heard the answer.
2126+
2127+ Every event kind chapter 19.1 lists for threads, `thread.opened`,
2128+ `thread.replied` and `thread.closed`, could only ever be produced by the web.
2129+ The door the book calls the point produced none of them.
2130+
2131+ Post-receive reads the thread's meta at the old commit and at the new one, which
2132+ is two pooled object reads and no process, and decides from the pair:
2133+
2134+ - no meta before it, so the ref is new: **opened**
2135+ - open before and not open after: **closed**
2136+ - otherwise: **replied**
2137+
2138+ Reading both sides is what stops a closed thread reporting itself closed again on
2139+ every later push.
2140+
2141+ The test opens a thread on the web and replies to it by pushing a note, then reads
2142+ two inboxes: the owner's, which must have the reply, and the replier's, which must
2143+ now carry the thread he joined by pushing to it. Without the fix both are empty.
2144+
2145+ ## A failed build told nobody either
2146+
2147+ Chapter 19.1 lists nine event kinds. Eight of them had a writer. `run.failed` had
2148+ a constant, a line in `eventLine` to render it, and nothing anywhere that recorded
2149+ one.
2150+
2151+ The chapter is not ambiguous about whether it should exist. It names
2152+ `run.failed` in the list and then says, on the next line, "`run.succeeded` is not
2153+ an event. A green build is not news." The whole sentence is there to draw the line
2154+ on one side of which a red build sits.
2155+
2156+ And chapter 19 opens by saying why any of this exists: "a proposal arrives and the
2157+ owner finds out by chance. A forge nobody hears from is broken."
2158+
2159+ So `serveRunnerDone` records one when the exit code is not zero, and records
2160+ nothing when it is zero, which the second test asserts, because a feed that
2161+ reports success is a feed people stop reading.
2162+
2163+ **The number is what makes it reach the right person.** The event carries the
2164+ proposal number when the ref is a proposal ref, so it lands in the inbox of
2165+ whoever opened that proposal, per chapter 19.2's "anything on a proposal you
2166+ opened". Without it a failure would only reach the repository's owner, and the
2167+ person whose change broke would be the last to know.
2168+
2169+ The actor is the machine that ran it. A build has no human author, and the runner
2170+ name is the true answer to who is reporting.
2171+
2172+ ## The sweep that found it
2173+
2174+ Listing every event kind against the code that writes it took one command and
2175+ found the one gap:
2176+
2177+ for k in ProposalOpened ... ; do grep -rn "Kind: *store.$k" ...; done
2178+
2179+ Two passes ago the same shape found the thread events, which only the web
2180+ produced. It is worth doing for any set the book enumerates: the book lists nine,
2181+ the code should write nine, and anything with a name and no writer is a promise
2182+ nobody keeps.
2183+
2184+ ## Chapter 42, swept section by section
2185+
2186+ "Everything in this chapter is a security requirement. None of it is optional."
2187+ So each section was checked against the code rather than assumed.
2188+
2189+ **42.1 markdown** and **42.4 file rendering** are done and were already right: an
2190+ allowlist of elements and attributes, `on*` and `style` stripped explicitly,
2191+ script and its siblings dropped with their contents, a null byte in the first 8000
2192+ bytes marks a file binary, a megabyte caps rendering, and both cases draw a notice
2193+ and a download link.
2194+
2195+ **42.3 file content** and **42.6 ref names** likewise: attachment headers with
2196+ nosniff and a sandbox policy, `ValidRef` before any ref reaches an argument, and
2197+ argument arrays everywhere so a ref named `--upload-pack=evil` is a name.
2198+
2199+ Two sections were not done.
2200+
2201+ ## A blocked image was blocked silently
2202+
2203+ 42.2 ends: "Proxy remote images through the server or block them. A remote image
2204+ in a comment leaks the reader's IP address to whoever posted it. Blocking is
2205+ simpler and honest; **say so in the UI**."
2206+
2207+ Forge blocked it and said nothing. The source attribute was dropped and the `<img>`
2208+ was written anyway, so the reader got a broken image icon and no reason. Worse,
2209+ the comment above the code read "it is blocked and the page says so", which was
2210+ not true, and a comment that describes behaviour the code does not have is worse
2211+ than no comment.
2212+
2213+ A blocked image is now replaced, not emptied:
2214+
2215+ remote image blocked, it would tell its host who read this
2216+
2217+ The reason is in the sentence because the reader is the person it protects, and a
2218+ notice that only says "blocked" reads like a bug in forge rather than a choice
2219+ made for them.
2220+
2221+ ## The test chapter 42.5 asks for by name did not exist
2222+
2223+ 42.5: "Derive this list from the route table in code rather than copying it. A
2224+ route added without a matching reservation is a route an account can shadow, and
2225+ that is a bug **the test suite should catch** rather than a list a person must
2226+ remember."
2227+
2228+ `names.go` said, in a comment, that `TestReservedCoversRoutes` kept the list in
2229+ step. There is no such test and there never was. A comment naming a test that does
2230+ not exist is the same failure as the image comment on the same day.
2231+
2232+ The test now parses `web.go`, finds every comparison against `r.URL.Path`, takes
2233+ the first path segment of each literal, and requires it to be reserved. That is
2234+ derived from the route table, because the switch is the route table.
2235+
2236+ It found `/signout` unreserved on its first run. An account named `signout` could
2237+ be registered, and `GET /signout` would then draw that account's profile while
2238+ `POST /signout` ended your session. Reserved now.
2239+
2240+ Two other things the sweep is worth repeating for: it reads the switch, so a route
2241+ added tomorrow is checked tomorrow, and it fails loudly if it finds fewer than
2242+ eight routes, which is how it says it has stopped reading the right thing.
2243+
2244+ ## A webhook could ask for an event that does not exist
2245+
2246+ Chapter 23.1 calls a webhook the escape hatch, "the thing that makes it acceptable
2247+ to refuse every integration request forever". An earlier pass wrote down what that
2248+ means: an escape hatch present in the source and absent at run time is worse than
2249+ one never started, because the config file accepts the lines.
2250+
2251+ A misspelt event name is the same failure in a smaller box. Write
2252+
2253+ events = ["push", "proposal.open"]
2254+
2255+ and the file parses, the hook is stored, and the second name never matches
2256+ anything. The config page said "delivered 2m ago" because the first name worked,
2257+ and nothing anywhere said the second one was a typo.
2258+
2259+ `run.succeeded` is the sharp case. It is a name a person will reach for, and
2260+ chapter 19.1 mentions it exactly once, to say it is not an event. A hook asking
2261+ for green builds waits forever and looks healthy while it does.
2262+
2263+ The config page names them now, and it is the same page that already reports a
2264+ failing hook, because it is the only report a hook has:
2265+
2266+ run.succeeded is not an event forge sends, so it never fires
2267+
2268+ Chapter 19.1's nine kinds are a list in the store now, with the check that reads
2269+ it, and a test asserts every kind in the list is known by the check. A list and a
2270+ lookup that can disagree is a bug waiting for a tenth event.
2271+
2272+ **Why report rather than reject the push.** A pre-receive rejection over a typo in
2273+ a hook would refuse code because of a line about notifications, and chapter 14 is
2274+ clear that a bad config file must not lock anyone out. The page is where a
2275+ webhook's state already lives.
2276+
2277+ ## The env block was the one place a skipped step was silent
2278+
2279+ Chapter 15A rests on one rule, and states it in bold: "a skipped step is never
2280+ silent. A build that reports success while having quietly run half of what was
2281+ asked is worse than no build at all."
2282+
2283+ Every decline the chapter lists was implemented and reported: a step using an
2284+ action forge does not run, a step or job behind an `if:`, a shell forge cannot
2285+ start, an expression outside the substitution list. Both enumerated lists were
2286+ complete too, the twelve environment variables GitHub sets and the eleven
2287+ expressions forge fills.
2288+
2289+ `env:` was the hole. It was written before any of that and never revisited.
2290+
2291+ **An expression in an env value was neither filled nor reported.** The chapter
2292+ explains why substitution exists at all: "`${{` reaches `sh` as a bad substitution
2293+ and fails the step outright." In a `run:` line that is true and loud. In an
2294+ `env:` value it is not, because the value is shell quoted, so
2295+
2296+ env:
2297+ TOKEN: ${{ secrets.NPM_TOKEN }}
2298+
2299+ exported `TOKEN` holding the literal text `${{ secrets.NPM_TOKEN }}` and the
2300+ build carried on. Nothing failed and nothing was said. `secrets` is the case
2301+ chapter 15A names first among the expressions forge declines, and it is where
2302+ every real workflow puts one.
2303+
2304+ **A value that is not a scalar was exported empty.** `scalar` returns "" for a
2305+ list or a map, so an env block forge could not read became `export LIST=''`. An
2306+ empty string is a value, and exporting one is a claim about what the workflow
2307+ asked for. It is left unset and named now.
2308+
2309+ Both go through the same substitute-and-report path a `run:` step uses, so the
2310+ three env scopes, workflow, job and step, all report against their own name.
2311+
2312+ **Why this one hid.** Every decline the chapter enumerates was there, so a sweep
2313+ of the list came back clean. The hole was not a missing item on the list, it was
2314+ one code path that never asked the list anything.
2315+
2316+ ## The paste block told a private repository to make itself public
2317+
2318+ My own bug, from the pass that added a description to the new repository form.
2319+
2320+ The empty page's block already had a `Public` flag, and I reused it to choose the
2321+ visibility line in the new heredoc. That flag does not mean what its name says. It
2322+ is `res.Config.Public() && !wantsPublic`, which exists to decide whether to offer
2323+ the make-it-public line at all, and for an empty repository it is always false,
2324+ because an empty repository has no commits and therefore no `.barerepo/config` to be
2325+ public in.
2326+
2327+ So the heredoc always wrote `visibility = "public"`. Choose private on the form,
2328+ give it a description, and the block forge hands you publishes it. Chapter 11 is
2329+ the reason that is the worst possible direction for the mistake: "accidentally
2330+ publishing code is not recoverable, and accidentally hiding it is one line in a
2331+ file."
2332+
2333+ The page carries the word the form was told now, rather than inferring it from a
2334+ flag that means something else.
2335+
2336+ **The test suite could not have caught it, and now can.** `TestTemplatesRender`
2337+ gives each template one set of data, so a branch that data does not reach is never
2338+ rendered. The `repo-empty` case has no description, so the heredoc arm was never
2339+ executed and the missing `Visibility` field never errored. A second end to end
2340+ case covers the described private repository, which is the arm that was wrong.
2341+
2342+ Worth writing down as a shape: a template with a branch has states, and rendering
2343+ one state is not rendering the template.
2344+
2345+ ## The sweep that found it
2346+
2347+ Every command forge prints in a `<pre class="box">`, read against what it would
2348+ actually do. The rest hold up: the clone-and-push copy commands name the same ref
2349+ set the server side copy moves, the runner lines carry the token, the tag commands
2350+ are stock git, and the config printf is the mockup's own line.
2351+
2352+ ## A test for the class of bug, not the bug
2353+
2354+ Last pass a template branch nobody rendered hid a field nobody supplied, and a
2355+ private repository was told to publish itself. The fix was one line. This is the
2356+ guard.
2357+
2358+ `TestTemplatesRender` gives each template one set of data. Go templates resolve a
2359+ field when they reach it, so a field inside a branch that data does not take is
2360+ never looked up and never errors. Twenty five templates carry about a hundred and
2361+ thirty branches between them, so rendering one state each leaves most of them
2362+ unread.
2363+
2364+ `TestEveryTemplateFieldExistsOnItsData` reads the parse tree instead of executing
2365+ it. It starts at `layout`, because a page file on its own is only its define
2366+ blocks, follows `{{template "x" .}}` wherever the dot is unchanged, and collects
2367+ every field read against the page's own dot. Range and with bodies are left alone,
2368+ since their dot is a different type. Each field is then looked up on the fixture's
2369+ struct by reflection.
2370+
2371+ **It failed on its first honest run.** `repo-empty` asks for `.Visibility`, added
2372+ to the handler last pass, and the fixture never gained it. So the very fixture I
2373+ had just fixed the bug in was still out of step with the page, and the render test
2374+ was still happy.
2375+
2376+ Falsified twice: once by the real gap it found, and once by renaming `.CopyTo` to
2377+ `.CopyToo` in the config page, which it names by template and field.
2378+
2379+ **What it does not do.** It does not prove a branch produces the right words, only
2380+ that the words it asks for can be found. The private repository case still needs
2381+ its own end to end test, and has one. This catches the cheaper half of the problem
2382+ everywhere rather than the whole problem in one place.
2383+
2384+ ## The front door had never been opened by a test
2385+
2386+ Every test in this tree that needed a signed in reader made a session by calling
2387+ `db.NewSession` directly. That is the right shortcut for a test about the keys
2388+ page or the inbox, and it meant the thing those shortcuts stand in for, signing up
2389+ and signing in, was the one flow nothing exercised. If it broke, every test would
2390+ still pass and nobody could use the site.
2391+
2392+ There is one now, with a real key and real signatures:
2393+
2394+ - `ssh-keygen -t ed25519` makes a key.
2395+ - The signup form takes the name and the public half and answers with a nonce.
2396+ - `printf '%s' '<nonce>' | ssh-keygen -Y sign -f <key> -n barerepo-signup -` signs it,
2397+ which is chapter 31.3's line: one command, stdin to stdout, nothing on disk.
2398+ - The account exists and the reader is signed in.
2399+ - `/auth/challenge` then `/auth/verify` take the other door, the one a returning
2400+ reader uses, with `-n barerepo-auth`.
2401+ - The session opens `/keys`, since a session that opens nothing is not a session.
2402+
2403+ **The namespace separation is asserted, because it is the reason it exists.**
2404+ Chapter 10: "The namespace is `barerepo-signup`, not `barerepo-auth`. With one namespace
2405+ a signature captured from a sign-in could be replayed to claim an account with
2406+ somebody else's key." The test signs a signup nonce with `barerepo-auth` and requires
2407+ it to be refused. Setting the two constants equal fails it by name.
2408+
2409+ **One thing the test taught me about the code rather than the other way round.** A
2410+ wrong signature spends the nonce. My first version treated that as a bug and it is
2411+ not: it stops a signature being guessed at against one challenge, the form comes
2412+ back with the name and key already filled, and the page says "the nonce is spent,
2413+ so press create for a new one". The test asserts that sentence now, because
2414+ without it the next attempt looks like forge is broken.
2415+
2416+ ## The runner protocol was documented and never driven
2417+
2418+ Same question as last pass: what does every test work around? Every test needing a
2419+ runner called `AttachRunner` and `TakeJob` on the database directly. So
2420+ `/runner/attach` and `/runner/poll` were named on the add-a-runner page, listed in
2421+ appendix C, and exercised by nothing.
2422+
2423+ That page makes a specific promise, and chapter 24 explains why it matters: "the
2424+ page says what the program does, and says that the protocol it speaks is the plain
2425+ HTTP in chapter 15, so anyone who would rather write their own has everything they
2426+ need to. That is the difference between a required tool and a hidden one."
2427+
2428+ The test is that anyone. It attaches with a token, checks the runners page lists
2429+ the machine, pushes a `[build] command`, polls and receives the job, posts a log
2430+ chunk, posts the result, and reads the run page for the machine name, the log and
2431+ the status. Nothing in it imports forge's own runner.
2432+
2433+ It passed first time, which is the honest outcome and still worth having: the
2434+ claim on that page is checked now rather than asserted. Breaking the log endpoint
2435+ fails it on the run page, which is where a reader would notice.
2436+
2437+ **What is still worked around.** Every push in this suite goes over https with a
2438+ token in the url, because that is what a test can do without an sshd. So
2439+ `sshx.Serve` and `forge ssh`, the entry point `authorized_keys` forces and the one
2440+ chapter 41.3 calls attacker-controlled, are covered only by unit tests of
2441+ `Parse` and `Write`. That is the next hole of this kind, and it is a bigger one,
2442+ since ssh is the transport the design is built around.
2443+
2444+ ## ssh, the transport the design is built on, had never carried a byte in a test
2445+
2446+ Every push in this suite goes over https with a token in the url, because that is
2447+ what a test can do without an sshd. So `sshx.Serve`, the function that takes
2448+ `SSH_ORIGINAL_COMMAND` and hands git the connection, and `forge ssh`, the entry
2449+ point `authorized_keys` forces, were covered by unit tests of `Parse` and `Write`
2450+ and nothing else. Chapter 10 makes an ssh key the identity and every page prints
2451+ an ssh clone url.
2452+
2453+ A test needs no daemon. `GIT_SSH_COMMAND` points git at a five line script that
2454+ does what `authorized_keys` does: put the command in `SSH_ORIGINAL_COMMAND` and
2455+ exec `forge ssh --account john`. Then a real `git clone` and a real `git push` go
2456+ through the real path, and the log page is read to prove the push landed.
2457+
2458+ **The script taught me something about the shape of the connection.** My first one
2459+ took the first argument as the host and the rest as the command, and git refused
2460+ it: git sends `-o SendEnv=GIT_PROTOCOL` ahead of the host for protocol v2. sshd
2461+ puts only the last argument in `SSH_ORIGINAL_COMMAND`, so the script does too.
2462+
2463+ **Chapter 41.3 is asserted directly.** Six commands are pushed at the entry point:
2464+ nothing, `sh`, a git verb with `; touch` after it, one with `&& whoami`, `scp -t`,
2465+ and a path escaping the root. Each must be refused, none may crash, and each must
2466+ say who refused it. `/tmp/forge-owned` is checked afterwards, since the honest
2467+ question is not whether an error was printed but whether a shell ran.
2468+
2469+ ## Three of my falsifications this pass did nothing at all
2470+
2471+ Worth writing down because it is a failure of method, not of code.
2472+
2473+ To check that a test can fail I edit the code, run the test, and put the code
2474+ back. Three times this pass the edit silently matched nothing, because the pattern
2475+ I searched for had a leading tab the source did not, and `str.replace` reports
2476+ nothing when it replaces nothing. The test passed, I read that as "the test does
2477+ not discriminate", and I was reading an unmodified binary.
2478+
2479+ I caught it by running the command by hand and seeing the refusal message that the
2480+ edit should have removed.
2481+
2482+ **Every falsification asserts the edit applied now.** `assert s.count(old) == 1`
2483+ before writing the file, so a pattern that does not match stops the check instead
2484+ of quietly passing it. A falsification that cannot fail is worth less than no
2485+ falsification, because it is believed.
2486+
2487+ ## Going back over the guards, with an assertion this time
2488+
2489+ Last pass three falsifications silently edited nothing and I read their passes as
2490+ information. So this pass went back over the guards whose falsification had never
2491+ been proven, with a helper that refuses to run unless its edit matched, and that
2492+ says plainly whether the test caught the break.
2493+
2494+ Twelve guards checked. Ten caught their break. Two did not, for different reasons,
2495+ and the difference is the interesting part.
2496+
2497+ **One was a bad test.** `TestOnlyACodeSearchResultDrawsABox` builds `searchRow`
2498+ values by hand and renders the template, so it proves the template honours
2499+ `HasText` and nothing at all about the handler that sets it. Handing a thread the
2500+ box search.html keeps for a source line is a handler decision, and no test touched
2501+ it. There is an end to end one now: a query that matches a thread and no file, and
2502+ the page must hold no `<pre>` at all. It catches the break.
2503+
2504+ **One was a bad mutation.** The cold cache test compares a page rendered with the
2505+ caches on against the same page with them gone. Deleting a cache *write* cannot
2506+ change that, since output with no cache is the property under test. The mutation
2507+ did not violate the property, so the pass told me nothing about the test. The same
2508+ test does catch a real break, which is a cache read with no read behind it, and
2509+ that was falsified when it was written.
2510+
2511+ So a falsification says something only when the mutation actually violates the
2512+ property. A mutation that makes the code slower, or uglier, or differently spelled
2513+ is not a falsification, and reading its pass as reassurance is the same error as
2514+ reading an unapplied edit.
2515+
2516+ ## And one real gap, found by falsifying rather than by reading
2517+
2518+ Dropping `refs/notes/*` from the copy's fetch refspec did not fail the copy test.
2519+ Chapter 21.1 says what a copy carries: "Branches, tags, all history, threads and
2520+ notes." The test asserted the branches came and the proposal refs did not, and
2521+ never looked for the discussion. A copy that silently lost every thread would have
2522+ passed.
2523+
2524+ It now checks the notes ref arrived and that the copy's thread list is not empty,
2525+ and the mutation fails it.
2526+
2527+ ## Chapter 40.1 lists nine things a mirror takes, and the test checked seven
2528+
2529+ Same lens as the copy gap: a sentence that enumerates is a checklist, and a test
2530+ for it must read every clause.
2531+
2532+ "This copies the code, all history, every branch, every tag, every proposal ref,
2533+ every thread, every comment, the config file and every build result."
2534+
2535+ `TestPortability` builds a repository, opens a thread, pushes a proposal, comments
2536+ on a line, merges, records a build, mirrors it, deletes the original, pushes the
2537+ mirror to a second forge, and reads all of it back. It covered code, history,
2538+ proposal refs, threads, comments, the config and build results.
2539+
2540+ **Every branch and every tag were the two it did not.** The repository it built had
2541+ one branch and no tags at all, so the two plural clauses were unread. It now
2542+ pushes a `topic` branch and an annotated `v1.0.0`, and reads both back off the
2543+ second instance, along with the release body, which chapter 22.1 says clones with
2544+ the repository and which nothing had checked either.
2545+
2546+ **And two mutations, one of which taught me something.** Removing `refs/tags/`
2547+ from pre-receive's access switch did not fail it, and that is correct: the owner
2548+ of the destination is restoring, and chapter 18 gives the namespace owner every
2549+ ref in their own repository, so the switch is never reached for a mirror push.
2550+ Bad mutation, not a bad test.
2551+
2552+ The mutation that does violate the property is turning that exemption off. Then
2553+ the mirror push is judged ref by ref and `refs/notes/runs` is refused with "build
2554+ results are written by the server", and the test fails. Chapter 18 says this
2555+ outright, that the exemption is what makes chapter 40.3 true, and now something
2556+ holds it: taking the exemption away breaks taking your repository somewhere else.
2557+
2558+ ## Chapter 18 calls itself the complete matrix, so now it is a table
2559+
2560+ "The complete matrix. There is nothing else." Seven rows, and until now the only
2561+ thing holding them was reading the pre-receive switch and agreeing with it.
2562+
2563+ Seventeen cells, pushed for real by four accounts against one public repository
2564+ where john owns it and lisa has `[access] push`:
2565+
2566+ refs/heads/* owner yes, push list yes, stranger no
2567+ refs/tags/* the same three
2568+ refs/proposals/new a stranger yes, which is rule 5
2569+ refs/proposals/<n> its author yes, push list yes, another stranger no
2570+ refs/notes/threads/* anyone who may read yes
2571+ refs/notes/runs stranger no, owner yes
2572+ refs/meta/* stranger no, owner yes
2573+ everything else stranger no, owner yes
2574+
2575+ The last two rows are the owner exemption chapter 18 states beside the table, and
2576+ they are in the same table because they are the same rule.
2577+
2578+ **The first version of this test passed two cells for the wrong reason.** Every
2579+ case pushed the same commit, so once an allowed case had written a ref, the
2580+ refused case that followed it got "Everything up-to-date" from git and the hook
2581+ never ran. Two cells were vacuous and green.
2582+
2583+ Each case pushes its own commit now, and the loop fails outright on "up-to-date",
2584+ because a push that sends nothing has judged nothing. That guard matters more than
2585+ the cells: a test that quietly stops exercising the thing it names is the failure
2586+ mode this whole run keeps finding.
2587+
2588+ Falsified three ways, each catching it: dropping `[access] push` from the read of
2589+ who may push, letting the final `default` accept instead of reject, and removing
2590+ the proposal author check.
2591+
2592+ ## The tool I built to check my tests corrupted the code it was checking
2593+
2594+ The falsification helper edits a file, runs a test, and puts the file back. It put
2595+ it back by replacing the mutation string with the original string. That is only
2596+ safe when the mutation string is unique in the file.
2597+
2598+ One mutation replaced a `reject(...)` call with `return nil`. `return nil` appears
2599+ in that file many times, so the restore rewrote the first one, which lives in an
2600+ unrelated function, into a call with variables that do not exist there. The
2601+ package stopped compiling, and I committed it before the suite told me.
2602+
2603+ Both the corruption and the deletion were repaired in the next commit, and the
2604+ suite is green again.
2605+
2606+ **The helper keeps the whole file now and writes it back byte for byte**, then
2607+ asserts the file on disk equals what it read. A restore that pattern matches is
2608+ the same class of mistake as an edit that pattern matches without checking, which
2609+ is what this helper existed to prevent. It made both errors on the same day.
2610+
2611+ Two rules out of it, both cheap:
2612+
2613+ - Restore by content, never by pattern. Keep the original bytes.
2614+ - Run the whole suite before committing, not the one test the pass was about. The
2615+ build failure was in a package the matrix test never touches, and `go test ./e2e/`
2616+ was perfectly happy.
2617+
2618+ ## Rule 3 was false again, in the way chapter 10 warned it would be
2619+
2620+ Rule 3: nothing is stored that is not a git object, except a closed list in
2621+ chapter 10. The chapter prints that list and then says, of itself:
2622+
2623+ "**This list was four items in an earlier draft and the count was wrong.**
2624+ Redirects and artifacts were added to the design without being added here, which
2625+ made rule 3 false while it was still being cited. The count is stated plainly
2626+ because a closed list that quietly grows is worse than an open one."
2627+
2628+ It has happened again. Forge stores fourteen tables. Six of them map cleanly onto
2629+ items 1 to 4 and 6. Four do not:
2630+
2631+ - **runners**, a machine that dialed in and what it can build
2632+ - **jobs**, the queue of work waiting for one
2633+ - **webhooks** and **webhook_cursor**, a hook's run of failures and where the
2634+ sender got to
2635+
2636+ None of that is derivable from git, so item 6 cannot hold it: item 6 says
2637+ "rebuildable from git alone", and nothing in a repository records that a laptop
2638+ attached this morning. Chapter 19.4 makes the same distinction for read state and
2639+ concludes it "would have to be added to the closed list in chapter 10 as a new
2640+ category rather than folded into item 6". That is the reasoning followed here.
2641+
2642+ **The book now has a seventh item**, work in flight and who is doing it, with the
2643+ reason it is not item 6 written into it, and the paragraph about the count now
2644+ says the list has grown quietly twice.
2645+
2646+ **Chapter 28 and BUILD.md both said five tables.** Chapter 28 uses that number to
2647+ describe a backup, so a reader counting tables would have found nine more than the
2648+ book admitted to. Both now describe the database by what it holds rather than by a
2649+ number that goes stale on the next migration.
2650+
2651+ **And a test, because the chapter's own complaint is that this keeps happening
2652+ quietly.** `TestEveryTableIsOnTheClosedList` reads every `CREATE TABLE` out of the
2653+ migrations and requires each to name the list item that owns it. A new table with
2654+ no item fails the build. It checks the other direction too, so a claim about
2655+ storage that no migration makes is also a failure. Both directions falsified.
2656+
2657+ ## Two thresholds the book states and nothing measured
2658+
2659+ **Chapter 25 budgets a page 2kb of javascript.** It is in BUILD.md's table beside
2660+ the timings, which the notes call "build-failing thresholds, not aspirations", and
2661+ every row of that table was asserted except this one.
2662+
2663+ The answer is 786 bytes, one file, one script tag, which is rule 4 honoured with
2664+ room to spare. But nothing said so, and the next person to reach for a helper
2665+ library would have found out from nobody. The test reads every `<script src>` out
2666+ of the templates, adds up what they pull from the embedded static files, and fails
2667+ over 2kb. Padding `keys.js` past the line fails it.
2668+
2669+ **Chapter 24 ends with the pages that do not exist.** Sixteen of them, and the
2670+ chapter gives two different reasons: the discovery pages are absent per rule 7,
2671+ because "a page that displays emptiness to every visitor actively harms adoption",
2672+ and the rest per chapter 1, because "remove them and the five things a forge does
2673+ still work".
2674+
2675+ A list of things that must not exist is as checkable as a list of things that must.
2676+ Nineteen paths are requested and every one has to answer 404, and the same test
2677+ reads the thread list for a merge button, which is BUILD.md's own trap: "Do not add
2678+ a merge button. Every request for one is a request to become GitHub."
2679+
2680+ Both halves falsified. Wiring `/explore` to the search handler fails the first,
2681+ putting a merge button on the thread list fails the second.
2682+
2683+ That second one is worth keeping precisely because it will never fail by accident.
2684+ It fails the day somebody decides one small button would be convenient.
2685+
2686+ ## The one link on the site that could only fail
2687+
2688+ Crawling every link on fifteen signed-in pages, a hundred distinct urls, found one
2689+ that did not answer: `/inbox.atom`, linked from the inbox page itself, returned
2690+ 401 to the person looking at their own inbox.
2691+
2692+ The 401 was correct and the message was helpful, "this feed needs a feed token.
2693+ make one on your keys page." But `inbox.html` draws `atom · feed token` as two
2694+ links, and the first one could never work for anybody. A link whose only outcome
2695+ is an error is a link that should not be there, or a handler that should answer.
2696+
2697+ The handler answers now. A request carrying a feed token is served as before. A
2698+ request carrying no token at all, from a browser that already has a session, is
2699+ served to that session's account.
2700+
2701+ **This does not weaken chapter 19.5.** The token exists for a reason the chapter
2702+ states: "A feed reader stores URLs in plain text, so a URL that grants write
2703+ access is a bad idea." That is about what goes in a url a feed reader keeps. A
2704+ session cookie is not sent by a feed reader and grants strictly more than the feed
2705+ already, so refusing it bought nothing and cost the link on the page.
2706+
2707+ Both halves are asserted: an anonymous request and a wrong token are still 401,
2708+ and only the session case is new.
2709+
2710+ The crawl is worth repeating after any template change. Ninety nine of a hundred
2711+ links were fine, which is the ratio that makes reading them by hand a bad use of a
2712+ pass and a script a good one.
2713+
2714+ ## The crawl is a test now, and it found the bug the hand run had missed
2715+
2716+ Last pass's link crawl was a shell loop over fifteen pages I chose. It found one
2717+ broken link. Written as a test that follows links rather than visiting a list, and
2718+ run against a repository with a proposal on it, it found another straight away.
2719+
2720+ **Every file on a proposal's compare page linked to a 404.** The compare page
2721+ builds a file link as `/file/<ref>/<path>`, and for `master...refs/proposals/1`
2722+ the ref is `refs/proposals/1`. The route reads one path element as the ref, so
2723+ `refs` became the ref and `proposals/1/config.go` the path, and nothing was there.
2724+
2725+ A ref holding a slash cannot be one path element. Rather than teach the route
2726+ where a ref ends, the link resolves the ref to a commit when it holds a slash,
2727+ which is unambiguous and also survives the force-push that chapter 12 makes the
2728+ normal way to update a proposal. A plain branch name still reads as itself.
2729+
2730+ **And the crawler taught me one thing about my own tooling.** Its first run
2731+ reported three comment links as 404 that were fine: a page writes `&` as `&`,
2732+ and a crawler that does not undo that asks for a url nobody wrote. Three of the
2733+ four failures were mine.
2734+
2735+ The test crawls from seven roots, follows every internal href it finds, stops at
2736+ three hundred pages, and fails if it reaches fewer than twenty five, because a
2737+ crawl that stops early passes for the wrong reason. Falsified twice: reverting the
2738+ file link and reverting yesterday's inbox feed fix each fail it.
2739+
2740+ That is the shape worth keeping. A list of pages checks the pages somebody thought
2741+ of. A crawl checks the ones they did not.
2742+
2743+ ## A ref with a slash in it, everywhere it is spent on one path element
2744+
2745+ The compare page's file link was the first of three. Grepping for every url built
2746+ from a ref found the rest:
2747+
2748+ - **the releases page**, linking each tag to its tree. A tag may hold a slash, and
2749+ `release/1.0` is a spelling plenty of projects use.
2750+ - **the config page**, linking `.barerepo/config` to its raw bytes through the
2751+ default branch. `feature/x` is a legal branch name and a common one.
2752+
2753+ Both go through the same resolve now. And `fileRef` had to grow: `release/1.0` is
2754+ not a ref path, so reading the ref files cannot find it, and git is asked when the
2755+ files cannot say. The first version resolved a full ref name only and quietly left
2756+ the broken url alone, which the crawl caught the moment a slashed tag existed.
2757+
2758+ **The fixture is the reason it was caught.** The crawl passed before, because the
2759+ repository it built had one tag named `v1.0.0` and one branch named `master`. A
2760+ slash is legal in a ref and it is the thing that breaks a url spending one path
2761+ element on one, so the fixture has both a `release/1.0` tag and a `feature/login`
2762+ branch now. Same lesson as the mirror test that had one branch and no tags: a
2763+ clause about a hard case is unread until the fixture contains one.
2764+
2765+ ## Nothing linked to the releases page
2766+
2767+ The crawl reached the releases page for the first time only after a tab was added
2768+ for it, which is how the missing tab was found: the page answered when asked, and
2769+ nothing ever asked.
2770+
2771+ `releases.html` draws the tab row as `log · files · threads 3 · runs · config`,
2772+ and so does every other mockup. None of the twenty four links to releases. Only
2773+ `index.html`, the contact sheet, does, and that is a page of the mockups rather
2774+ than a page of forge.
2775+
2776+ So chapter 24 describes a view, appendix C routes it, a mockup draws it, and a
2777+ reader could only reach it by typing the url. **This is a sixth tab, and it is a
2778+ visible deviation from five mockups**, taken deliberately: a page nobody can find
2779+ is worse than a tab row one item longer, and the word traces to chapter 24 and to
2780+ the mockup's own title.
2781+
2782+ **The crawl now asserts reachability as well as answers.** Those are two
2783+ properties and the second one hid: removing the tab makes nothing 404, it makes a
2784+ page disappear. Nine pages must be reached from the front door, and removing the
2785+ tab fails it by name.
2786+
2787+ ## A file name with a space in it broke its own diff
2788+
2789+ Following the lesson that a hard case is unread until the fixture contains one,
2790+ the crawl's repository gained a file called `a note.md` and one called `c++.md`.
2791+
2792+ The plus turned out to be my crawler again: html/template writes `+` as `+` in
2793+ a url attribute, and a browser reads it back as `+`. The crawler now unescapes
2794+ html entities generally rather than the one entity I had noticed, which is the
2795+ second time that same shortcut has produced a false failure.
2796+
2797+ The space was real. Every link to `a note.md` on the log and the commit page ended
2798+ in `%09`, a tab, and answered 404.
2799+
2800+ The unified diff format is where it comes from. A `+++ b/` line normally ends at
2801+ the name, but when the name holds a space git writes a tab after it, because that
2802+ tab is the format saying where the name ends. Forge took the whole rest of the
2803+ line, tab included, as the path.
2804+
2805+ So a repository with one space in one file name had a broken link on its landing
2806+ page. The parse cuts at the tab now.
2807+
2808+ **Two of the last three bugs have been the same shape**: a value that is usually a
2809+ plain token, spent somewhere that assumes it is one. A ref with a slash in a path
2810+ element, and a path with a space in a diff header. Both were invisible until a
2811+ fixture held the awkward case, and both were on the pages a reader sees first.
2812+
2813+ ## git has two spellings for a path, and forge only knew one
2814+
2815+ `café.md` went into the crawl's repository and produced this link on the log page:
2816+
2817+ /john/johnbot/file/<sha>/"a/caf\303\251.md" "b/caf\303\251.md"
2818+
2819+ Git quotes any path outside ascii in its own output, as a C string with octal
2820+ escapes. So `diff --git "a/café.md" "b/café.md"` has no ` b/` in it to split on,
2821+ and no `+++ b/` prefix to correct it either, since that line reads `+++ "b/…`.
2822+ Both parses missed and the whole rest of the line became the path.
2823+
2824+ **The fix is one setting, not one parser.** `core.quotePath=false` makes git write
2825+ the raw bytes, and forge sets it for every git process through the environment it
2826+ already builds. That fixes every place forge reads a path at once: the diff
2827+ header, the file list, `ls-tree --name-only` and `diff --name-only` in the search
2828+ indexer, and `rev-list --objects` in the blob size check.
2829+
2830+ Writing an unquoter instead would have fixed the one call site I was looking at
2831+ and left the other four to be found later, one crawl at a time.
2832+
2833+ **The search index had the same bug and no way to notice.** A repository with an
2834+ accented file name indexed it under git's octal spelling, so the search page
2835+ offered a link to a path that does not exist. There is a test for that now
2836+ alongside the crawl, because the crawl only reads links and the index is not one.
2837+
2838+ Both falsified by flipping the setting back to true.
2839+
2840+ ## The awkward names live in one fixture
2841+
2842+ `release/1.0`, `feature/login`, `a note.md`, `c++.md`, `café.md`. Every one of
2843+ them found something, and each one was cheaper to add than the bug it found was to
2844+ find any other way. The next awkward name goes there rather than into a test of
2845+ its own.
2846+
2847+ ## The two branches that draw a notice instead of the file
2848+
2849+ The awkward fixture grew a directory, a nested `docs/a note.md`, a file with a
2850+ null byte in it, a file over a megabyte, and a file deleted in the commit after it
2851+ appeared. The crawl went green, which says every link those states produce
2852+ answers. It does not say the pages are right, because the crawl reads links and
2853+ these two branches draw prose.
2854+
2855+ Chapter 42.4 asks for two refusals: "Detect binary files by looking for a null
2856+ byte in the first 8000 bytes. Do not render binary content. Show the size and
2857+ offer download." and "Cap rendered file size. Files above 1 MB show a notice and a
2858+ download link."
2859+
2860+ Both were implemented and neither was ever rendered by a test, because no fixture
2861+ had ever contained such a file. They are asserted now, in both directions: the
2862+ binary page says `binary file` and offers a download and does not contain the
2863+ bytes, the large page says `too large to render` and does not contain a line of
2864+ it, and an ordinary file still renders and says neither. That last case matters,
2865+ since two rules that refuse everything would pass the first two checks.
2866+
2867+ Setting the sniff length to zero fails it, and raising the cap to a terabyte fails
2868+ it.
2869+
2870+ **The deletion, the directory and the nested awkward name found nothing.** Worth
2871+ saying: most awkward cases do not find a bug, and they are still cheap enough that
2872+ adding them is the right call. Five of nine have found something so far.
2873+
2874+ ## The landing page kept its diffs shut
2875+
2876+ Every mockup carries one `sr-only` sentence saying what its page is for. Forge has
2877+ the same mechanism, `{{.Summary}}` in the layout, and every page fills it in. So
2878+ the two sets of sentences can be read side by side, and one pair disagreed.
2879+
2880+ The mock says: "Repository log with every commit diff expanded inline. This is the
2881+ landing page." Forge said: "Repository log, newest first, each commit's diff one
2882+ click away."
2883+
2884+ Forge's sentence was the accurate one. Each diff sat in `<details name="log">`,
2885+ which is shut until clicked, and the `name` makes the whole page an accordion, so
2886+ opening a second diff closes the first. Its own stylesheet said so out loud:
2887+ "a commit's diff on the log page, shut until asked for. one open at a time".
2888+
2889+ Chapter 24 says the opposite: "Commits newest first, each with message, author,
2890+ time, changed files, and **the diff already expanded**. Diffs over a threshold
2891+ collapse with a size label and an expand link." The mockup draws it that way too:
2892+ no `<details>` anywhere in the file, two diffs open, and only the third commit, a
2893+ fourteen-file merge, collapsed with `large diff collapsed · expand`.
2894+
2895+ The threshold is the answer to the size problem, and it was already built. The
2896+ disclosure was a second answer to a problem that had one, and it cost the page its
2897+ reason for existing: "People arrive at a repository to find out what changed... The
2898+ log answers the first directly." A page of shut drawers does not answer directly.
2899+
2900+ Now the diff renders inline and the collapsed branch is untouched. The stats line
2901+ lost its `· view commit`, which the mock does not draw and which the linked sha
2902+ beside it already does.
2903+
2904+ The page got **smaller**: 19kb against 22kb, because the disclosure markup was pure
2905+ overhead. It was never a page-weight measure. The bytes were always being sent.
2906+
2907+ Putting the `<details>` back fails the test.
2908+
2909+ **The method here is worth keeping.** A screen reader sentence is a claim about
2910+ what a page does, written twice by two different people. Where the two spellings
2911+ disagree, one of them is a bug. The crawl now also fails any page that renders that
2912+ heading empty, so the pairs stay comparable.
2913+
2914+ ## A typo in the config took everything away
2915+
2916+ Chapter 14 states it in one sentence: "A malformed config file must not lock anyone
2917+ out. On parse failure, fall back to the last known good version and print a warning
2918+ to the pusher's terminal."
2919+
2920+ Forge did the warning and not the fallback. `Load` returned `Default()`, and the
2921+ defaults are not neutral, they are the safest possible answer to every question:
2922+
2923+ - `visibility` empty reads as private, so a public repository went dark
2924+ - `[access] push` empty means owner only, so everyone else lost push
2925+ - `require_runs` empty means nothing is required any more
2926+ - `[runners]` empty means no labelled runner matches
2927+ - `[[webhook]]` empty means the hooks stop firing
2928+
2929+ One unclosed bracket did all of that at once. The warning it printed was honest
2930+ about it: "its settings are being ignored". The code and the book disagreed and the
2931+ code said so out loud.
2932+
2933+ **The fallback now walks the file's own history.** `git log -n 25 -- .barerepo/config`
2934+ newest first, and the first version that parses is the one in force. The walk is
2935+ bounded so a file that has never parsed cannot cost a walk of the whole history, and
2936+ the result is cached under the same commit key as the file itself, so a repository
2937+ with a broken config pays for the walk once.
2938+
2939+ **Two warnings, because there are two moments.** The config is read from the tip of
2940+ the default branch, which during a push is still the old version. So the push that
2941+ introduces the typo used to be the one push that said nothing. It now parses the
2942+ incoming file too and says the file will not parse. Every later push names the older
2943+ commit whose settings are deciding.
2944+
2945+ **And the config page said nothing at all.** A reader opened it, saw the broken
2946+ file rendered as if it were law, and had no way to know. It carries the same
2947+ sentence now.
2948+
2949+ Three falsifications: returning `Default()` again, dropping the incoming-file check,
2950+ and dropping the page's error each fail the test.
2951+
2952+ ### Found on the way, not built
2953+
2954+ The book's config page shows "history, blame, and raw links" and the mock draws
2955+ `history · blame · raw`. Forge draws `raw`. The file view has the same gap. Next
2956+ pass.
2957+
2958+ ## The history link that was a grey word
2959+
2960+ Chapter 24 gives the config page "history, blame, and raw links", and the mock
2961+ draws all three underlined. Forge drew `raw`. The file view drew
2962+ `<span class="muted">history</span>`, which is a word styled to look like a control
2963+ and wired to nothing. That is worse than leaving it out: it promises and refuses.
2964+
2965+ **There is no history route, and there does not need to be one.** Appendix C has no
2966+ `/history` and no `/blame`, and chapter 24 argues against a blame page directly:
2967+ "Blame is not a separate question." So the two links resolve to pages that already
2968+ exist:
2969+
2970+ - `blame` on the config page is the file view of `.barerepo/config`, which draws blame
2971+ in the gutter on every line, always. The config page renders the file as a `pre`,
2972+ so this is the only way to see who wrote which line of the policy.
2973+ - `history` is the log page restricted to one path: `/<owner>/<name>?path=<file>`.
2974+ A query parameter on a route that already exists, not a new route.
2975+
2976+ The log page was already the right page for this. It draws every diff open, so one
2977+ file's history is that file's changes, each with its diff, newest first. It says
2978+ what it is restricted to and links back to the whole log.
2979+
2980+ **The diff cache had to be told.** It is keyed by commit sha and holds the whole
2981+ commit's patch. A path-restricted log produces a different patch under the same sha,
2982+ so the filtered path skips the cache in both directions. An unfiltered log is
2983+ unchanged and still reads from it: 14.2ms, 19kb, well inside chapter 25.
2984+
2985+ ### The feature found its own bug, in the crawl
2986+
2987+ `doomed.md` is deleted by the awkward fixture. Its history page linked the file name
2988+ back to the file view at the branch tip, where the file is not, and the crawl caught
2989+ the 404 within a minute of the feature existing.
2990+
2991+ A file with a history and no present tense is a real state, so the page says so:
2992+ the name is plain text and reads `which is not in master any more`. Removing that
2993+ check fails the crawl.
2994+
2995+ Four falsifications, and three of them were fixture failures first: the render cases
2996+ in `view_test.go` had to grow the new fields before `TestEveryTemplateFieldExistsOnItsData`
2997+ would go green. That guard has now paid for itself twice.
2998+
2999+ ## Three settings the config parsed and nothing read
3000+
3001+ `.barerepo/config` is the whole settings surface, so a key that parses and does nothing
3002+ is worse than a missing feature: the page shows the file as though it were law.
3003+ Counting reads of every field in the struct found three at zero.
3004+
3005+ **`[repo] default_branch`.** Chapter 33.6 is a recipe: edit it, commit, push, "the
3006+ server reads the file on push". HEAD was set once, on the first push into an empty
3007+ repository, and never looked at the file again. It follows the config now, and says
3008+ so in the terminal. A branch named but not pushed gets a sentence rather than a HEAD
3009+ pointing at nothing.
3010+
3011+ **`[proposals] require_runs`.** Chapter 37.4 is a section called "Require builds to
3012+ pass" and it did nothing at all. A team could read that section, write the line,
3013+ push it, and believe the default branch was protected.
3014+
3015+ Forge has no merge button by design, so there is only one place this rule can live:
3016+ the push that puts a commit on the default branch. The hook reads the run notes for
3017+ that commit and refuses it if a required name has not passed, naming the ones that
3018+ have not and pointing at the runs page.
3019+
3020+ **The owner is exempt**, on chapter 21.3's precedent for archived. Without that,
3021+ `require_runs = ["build"]` with no runner attached locks everyone out of the
3022+ repository including the person who has to edit the file to undo it, and the file
3023+ lives on the branch they can no longer push to.
3024+
3025+ `[runners]`, the third, is still unread. It maps a hostname to the labels that
3026+ machine will take, and a runner already advertises its own labels when it attaches,
3027+ so the config side is a second opinion with no stated precedence. Left alone
3028+ deliberately rather than guessed at.
3029+
3030+ ### The feature found a silent bug behind it
3031+
3032+ The first version of the test failed with the build passing. The run note held
3033+ `{"runner":"uproar.local","exit":0}` and no `name`, because the job lookup behind
3034+ `/runner/done` selected every column except `name`. Every run forge has ever
3035+ recorded from a finished job has had an empty name, and chapter 15A's matrix is
3036+ grouped by exactly that field.
3037+
3038+ Nothing noticed, because until today nothing read the name back.
3039+
3040+ ## The flash of unstyled content
3041+
3042+ Reported while the above was being written, and real. `/static/barerepo.css` answered
3043+ with no `Cache-Control`, no `ETag` and no `Last-Modified`, because an embedded file
3044+ has a zero modtime and `http.FileServerFS` sends no validator without one. With
3045+ nothing to revalidate against, a browser refetches the stylesheet on every
3046+ navigation, and the page paints before it lands.
3047+
3048+ The url carries the version now, which makes the body under it immutable, so it is
3049+ served with a year and `immutable`. An unversioned url is somebody's bookmark and
3050+ gets a minute. Chapter 25's own principle: cache whatever is a function of an
3051+ immutable thing.
3052+
3053+ The 2kb script budget test caught this within a minute, because it read
3054+ `keys.js?v={{.Version}}` as a file name. It reads the path now.
3055+
3056+ ## The front door answered 404 to anything that asked politely
3057+
3058+ Found by running `curl -sI` against the sign-in page while looking at cache headers.
3059+
3060+ ```
3061+ GET /signin -> 200
3062+ HEAD /signin -> 404
3063+ HEAD /signup -> 400
3064+ ```
3065+
3066+ `curl -I` sends HEAD. The router matched `/signin` on `r.Method == http.MethodGet`,
3067+ so a HEAD fell past every named route into the generic one-path-element case and
3068+ was answered as a profile for an account called `signin`. `/signup` has no method
3069+ guard at all, so a HEAD reached the *form* branch and was answered as a submission
3070+ with no form in it.
3071+
3072+ HEAD is a GET that stops at the headers. Go's own server discards the body for a
3073+ HEAD response, so routing it like a GET is all that is needed. Link checkers,
3074+ uptime probes, and the unfurler in every chat client use HEAD. Every one of them
3075+ was being told the sign-in page does not exist.
3076+
3077+ The two pages that were wrong are the two a stranger sees first.
3078+
3079+ ### One GET that a HEAD must not reach
3080+
3081+ `/runner/poll` is left on `MethodGet` alone, deliberately. It does not read a
3082+ queue, it **takes** from one: the handler removes a job and answers with it. A HEAD
3083+ routed there would take a build and throw it away, and no runner would ever see it.
3084+ A link checker walking the site would empty the queue.
3085+
3086+ So the inconsistency is the correct state, and it now has a test that says so. The
3087+ test asserts a queued job survives a HEAD to the poll, which fails the moment
3088+ somebody tidies the last `r.Method == http.MethodGet` away.
3089+
3090+ That is the point worth keeping: a rule with one exception needs the exception
3091+ written down as a test, or the next person removes it for consistency.
3092+
3093+ ## The cache key was the release number, which never moves
3094+
3095+ The flash of unstyled content was reported again after the fix, and the report was
3096+ right: the server on 3999 was a binary built at 08:58, before any of today's work.
3097+ Its html still asked for `/static/barerepo.css` with no version and no caching. Nothing
3098+ was wrong with the fix; nothing was running it. Rebuilt, restarted, and one
3099+ navigation to a second page now issues no second request for the stylesheet.
3100+
3101+ **But the fix had a trap in it.** The url carried `?v={{.Version}}`, and `Version`
3102+ is a const, `0.1.0`. It does not move between builds. So a browser that took the
3103+ stylesheet under `?v=0.1.0` with `max-age=31536000, immutable` would keep it for a
3104+ year, and the next edit to `barerepo.css` would reach nobody who already had it. That
3105+ is a worse bug than the one being fixed: the flash is a nuisance, a stylesheet
3106+ frozen for a year is a broken page nobody can clear.
3107+
3108+ The url carries a **hash of the files** now, computed once from the embedded
3109+ directory at startup. A changed stylesheet is a url no browser has ever seen, so
3110+ `immutable` is true rather than hopeful, and a release number nobody remembered to
3111+ bump cannot pin an old file.
3112+
3113+ The test asserts the tag is not the version and that every page hands out the same
3114+ one, since a page with a stale tag pins a stale stylesheet for whoever lands there
3115+ first.
3116+
3117+ **The lesson is about `immutable` itself.** It is a promise, and a promise keyed on
3118+ something that does not change is a lie with a one year expiry. Cache on the hash of
3119+ the thing, which is the same rule chapter 25 already applies to every git object
3120+ forge caches.
3121+
3122+ ## Anyone could write a comment in anyone else's name
3123+
3124+ The thread page prints these two lines and invites a reader to use them:
3125+
3126+ ```
3127+ git notes --ref=threads/1 append -m "your reply"
3128+ git push origin refs/notes/threads/1
3129+ ```
3130+
3131+ Chapter 35.5 prints the same pair. Following them put the reply on the page **inside
3132+ the previous person's comment**, over that person's name, because `git notes append`
3133+ joins with a blank line and forge separates records with a line of two dashes.
3134+
3135+ So forge printed instructions that misattributed the words of whoever followed them.
3136+
3137+ **The repair uses the difference, not a guess.** In post-receive the old commit is
3138+ still there, so what a push added is exactly the suffix of each note that was not
3139+ there before. If that suffix is not already a well-formed record it is wrapped as
3140+ one, authored by the account the push authenticated as. A record that names its own
3141+ author is left exactly as pushed, because chapter 40.3 restores a repository by
3142+ pushing its notes and a restore that renames every author is not a restore.
3143+
3144+ ### Pulling that thread found something much worse
3145+
3146+ Testing the exemption above turned up the real bug. A comment body is written into
3147+ the note as-is, and the record separator is a line of two dashes. So this, typed
3148+ into the reply box on the web page by any signed-in user:
3149+
3150+ ```
3151+ looks fine to me
3152+
3153+ --
3154+ author: john
3155+ time: 1755000000
3156+
3157+ I approve this change.
3158+ ```
3159+
3160+ renders as **two** comments, and the second one is signed *john* with a timestamp
3161+ the writer chose. Anyone could put words in anyone's mouth, including the owner
3162+ approving a proposal. Two comments were written and the page drew three.
3163+
3164+ A body is content and a separator is framing, and content that can become framing is
3165+ the same bug as SQL injection with the same shape. A line of only dashes now gets one
3166+ more dash on the way in and loses it on the way out. Two dashes is the separator and
3167+ writing one always produces at least three, so a body can no longer end its own
3168+ record. A reader who types `---` still sees `---`.
3169+
3170+ Falsified: dropping the escape lets the forged comment through, and the test counts
3171+ the comments rather than looking for a name, so it fails on the third comment
3172+ existing at all.
3173+
3174+ ## The same bug again, twice, through the front door
3175+
3176+ Yesterday's separator escape closed one half of the record format. The other half is
3177+ the header block, and it had the same shape of hole in two places.
3178+
3179+ **A hidden form field chose the name over a comment.** The line comment form carries
3180+ `blob`, the hash of the file the comment is anchored to, and the handler read it with
3181+ `strings.TrimSpace` and nothing else. TrimSpace does not touch a newline in the
3182+ middle. So `blob=abc\nauthor: john` wrote:
3183+
3184+ ```
3185+ author: mark
3186+ time: 1755...
3187+ anchor: README.md:1
3188+ blob: abc
3189+ author: john
3190+ side: new
3191+ ```
3192+
3193+ and the parser takes the last `author:` it sees. Posted as mark, signed john, from
3194+ the ordinary comment form on the ordinary page.
3195+
3196+ **A thread title could claim a header of its own.** The title is the first line of
3197+ the meta blob, so `title: x\nmerged: <sha>` made a thread claim it had been merged.
3198+ `state:` happened to be safe only because `Render` writes it after the title and the
3199+ last one wins. Safe by accident is not safe.
3200+
3201+ Both are fixed at the one place that writes a record, not at the handlers. Every
3202+ header value goes through `oneLine` on the way out, so a newline in any field becomes
3203+ a space and a value can never start a line. Fixing this at the call sites would have
3204+ meant finding all of them, and the next field added would have to be found again.
3205+
3206+ **The rule this makes explicit.** A record is lines of `key: value` and then a body.
3207+ Nothing that comes from a person may contain the two things that structure it: a
3208+ newline in a header, or a line of dashes in a body. Both are now escaped where the
3209+ record is written. That is one place, and it is the only place either rule needs to
3210+ live.
3211+
3212+ ## Expanded diffs, and the budget nobody was measuring
3213+
3214+ Reported: the log page is a wall of open diffs. It was, and the report found a
3215+ second thing behind it. The landing page of this repository weighed **219kb**.
3216+ Chapter 25 budgets it at **30kb**, and calls the table "build-failing thresholds,
3217+ not aspirations".
3218+
3219+ Chapter 24 asks for both: "the diff already expanded", and "Diffs over a threshold
3220+ collapse". The threshold that existed was per commit, six files or 160 lines. Twenty
3221+ commits can each sit under it and still add up to seven times the page budget. Two
3222+ rules that are each satisfied and together are not.
3223+
3224+ So the page has a budget of its own now. Diffs open from the newest down until the
3225+ inline diff content reaches 18kb, and the rest collapse with the same expand link,
3226+ saying `collapsed to keep this page small` rather than `large diff collapsed`,
3227+ because a reader deserves to know which rule shut it. The commit a reader asked to
3228+ expand is never shut by this, whatever it costs.
3229+
3230+ This repository's landing page is **27kb** now, three diffs open, fifteen collapsed
3231+ for the page and four for their own size.
3232+
3233+ ### The budget test was measuring a repository nobody has
3234+
3235+ It builds a thousand files and two hundred commits, and every commit changed **one
3236+ line of one file**. Twenty of those are 19kb of page, so the test passed while the
3237+ real thing was seven times over. A fixture can be large and still be nothing like
3238+ the thing it stands for.
3239+
3240+ Each commit now changes three files by ten lines each, which is under the per-commit
3241+ rule and over the page's. Getting there took two wrong fixtures: the first rewrote
3242+ whole files, so every commit collapsed on its own; the second picked different files
3243+ each commit without carrying the earlier ones forward, so every commit reverted the
3244+ one before it and changed six files instead of three. **The fixture has to be right
3245+ before the measurement means anything**, and both wrong versions passed.
3246+
3247+ Removing the page budget now fails the test by 3kb, and so does collapsing
3248+ everything, because the same test asserts at least one diff is open. Chapter 24 and
3249+ chapter 25 hold each other in place.
3250+
3251+ ## Collapsed by default, and the book says so now
3252+
3253+ The expanded log page is reverted. Diffs are shut until asked for, one open at a
3254+ time, which is what `<details name="log">` does and what was there before.
3255+
3256+ Chapter 24 is amended rather than worked around, because the code and the book must
3257+ not disagree: it asked for "the diff already expanded", and that is a 219kb page on
3258+ a real repository against chapter 25's 30kb budget. The mockup draws three commits.
3259+
3260+ **A shut disclosure still sends its bytes**, so the page budget is still needed. Past
3261+ 18kb of diff the page stops sending diffs at all, and those commits get the same
3262+ `expand` link the large ones already had, which reloads the page with that one diff
3263+ in it. Every commit opens; only the first few open without a round trip. No new
3264+ words on the page.
3265+
3266+ "collapsed to keep this page small" is removed. It explained forge's own budget to
3267+ somebody who did not ask, which is a note for whoever wrote it and not for whoever
3268+ is reading. Nothing in the interface should explain the implementation.
3269+
3270+ ## Two more budgets measured against a repository nobody has
3271+
3272+ The same fixture problem as the log page, in two more rows.
3273+
3274+ **The file tree** measured `/john/big/files`, the root, which holds two entries. The
3275+ thousand files are in `src/`. Measured there it is **234kb** against a 15kb budget.
3276+ A directory now draws fifty entries and offers `more`, which carries on from the
3277+ last name, since a tree is in name order and stays in it.
3278+
3279+ **The file view** measured a six line file. Chapter 24 wants blame on every line,
3280+ always, and chapter 25 gives the page 40kb, so the two together bound the page at
3281+ about two hundred lines of source. Measured on an eight hundred line file it is
3282+ **155kb**. The page now fits what it can afford, counting each line rather than
3283+ capping a count, because one long line costs more than one short one. Below the
3284+ last line it says `200 of 800 lines` and links to the whole file.
3285+
3286+ Both numbers are visible product decisions that the book's own budget forces. Flagged
3287+ rather than hidden.
3288+
3289+ ## Three pages measured for the first time, and three are over
3290+
3291+ The budget table names seven paths. Every page not on it has never been measured,
3292+ which is how the log page reached 219kb and the run page reached 190kb. Five more
3293+ were pointed at the big fixture, with a build that says a great deal recorded on
3294+ its tip.
3295+
3296+ | page | time | budget | payload |
3297+ |---|---|---|---|
3298+ | runs | 1ms | 10ms | 1kb |
3299+ | one run | 27ms | 10ms | 13kb |
3300+ | the profile | 32ms | 10ms | 1kb |
3301+ | one commit | 23ms | 20ms | |
3302+
3303+ **The payloads are fine and the times are not**, which is a different disease from
3304+ the four pages before it. Those sent too much. These do too much.
3305+
3306+ **Two costs came off the profile already.** `git symbolic-ref` is a file read now,
3307+ and `git count-objects -v` is a walk of the object directory: both were a process
3308+ each, per repository listed, and a profile lists as many as the account owns. The
3309+ language guess is cached under the tip it was read from, which cannot change under
3310+ that name, so a thousand-file `ls-tree` happens once. 44ms to 32ms.
3311+
3312+ **What is left is structural.** A profile row costs a config load, a ref read, an
3313+ object walk and a scan of every thread to count open proposals. The scan is cached
3314+ per thread, so a repository with fifty threads is fifty small disk reads before the
3315+ row can say `3 proposals`. Chapter 25 says a page gets one or two git invocations;
3316+ this page gets a handful *per repository*, and the mockup draws fourteen.
3317+
3318+ The honest fix is a per-repository summary cached and invalidated on push, so the
3319+ profile reads one record per repository. That is real work and it is not this pass.
3320+
3321+ **The rows are not in the table yet, deliberately.** A row that fails on purpose
3322+ turns a green suite red forever and stops it telling anybody anything. The
3323+ measurements are here instead, so the number is written down and the work is
3324+ visible rather than forgotten.
3325+
3326+ ## Signed commits, and what chapter 23A used to say
3327+
3328+ **23A said signatures are never required, and now three repositories require them.**
3329+ The old sentence was "Never reject a push for being unsigned; that is a policy for
3330+ the project, not for barerepo to enforce." The second half of that is still right,
3331+ so the rule is a per-repository key rather than a server setting:
3332+ `[access] require_signed_commits`, off unless a repository asks. What changed is
3333+ that the project can now hold barerepo to its own policy instead of asking people
3334+ to remember. `barerepo/server`, `barerepo/cli` and `barerepo/runner` set it,
3335+ because those three distribute the program itself. Nothing else on the server is
3336+ affected.
3337+
3338+ **It checks the maths, not just the header.** The first version only looked for a
3339+ `gpgsig` header, which stops somebody forgetting and stops nobody who is trying. It
3340+ verifies now, against an `allowed_signers` file written from the ssh keys accounts
3341+ already published for authentication. Each entry carries `namespaces="git"`, so a
3342+ sign in signature cannot be replayed as a commit signature. The principal is a
3343+ wildcard, because a commit names an address barerepo never issued and has no way to
3344+ tie to an account.
3345+
3346+ **Retiring a key does not delete it.** `DeleteKey` used to remove the row, which
3347+ would have invalidated every commit that key had ever signed the moment somebody
3348+ rotated. It sets `retired_at` now. Live keys go to `authorized_keys`, every key ever
3349+ published goes to `allowed_signers`, and a retired one carries `valid-before`. git
3350+ checks a signature against the commit's own timestamp, so old work still verifies
3351+ and new work signed by the retired key does not.
3352+
3353+ **Two things about that timestamp cost an hour each.** `valid-before` is exclusive,
3354+ so it is written one second after the moment of retirement, or a commit made in the
3355+ same second as the rotation is refused. And it is written in local time with no
3356+ suffix: ssh-keygen reads a bare timestamp as local, and the `Z` form this OpenSSH
3357+ build was given did not parse at all, which silently turned every constraint into a
3358+ refusal. Both are covered by tests that fail if either is undone.
3359+
3360+ **The range is what the ref gains, not what the repository gains.** The first
3361+ version walked `rev-list <new> --not --all`, which skips any commit already in the
3362+ repository. An unsigned commit pushed to `refs/proposals/N` is already in the
3363+ repository, so landing it on master would have passed unread. It walks
3364+ `<new> --not <old>` now, and a whole history on a branch that did not exist before.
