From cac9fe1fb81fc0c100dab64c30ffe5d5f31a8442 Mon Sep 17 00:00:00 2001 From: bigbes Date: Wed, 22 Jul 2026 14:46:55 +0300 Subject: [PATCH] docs: three receive hooks, and the read plane's rev guard The two-hook split was unimplementable. Push options reach pre-receive and post-receive only, so the skip-validation escape hatch was unreachable where the design put it; and pre-receive cannot read the pushed objects at all, because they sit in the quarantine until after it runs. update is therefore the earliest hook that can validate, which the design relied on without saying why. Work splits across three hooks correlated by repository and receive-pack pid. Records that per-ref rejection is not atomic: a push of main plus a proposal branch, with main rejected, still lands the proposal. Settles the read plane's audience, which was anonymous-capable in one section and leak-free in the verification checklist. It is fail-closed. Documents the rev guard: reads take the approved head or a full object name, never a ref, because rev=proposals/42 would otherwise serve unreviewed text as approved through the read plane itself. --- docs/DESIGN.md | 74 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 12 deletions(-) diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 2978a94c7c30b26affca908b9c6ff09e1ab5f1b2..7d8960658b5a3994ab85d84cc2e50acd1433a987 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -246,15 +246,34 @@ necessary if the audience stops being one person. > service code". **Both claims were wrong**, and since this is the mechanism the > whole human write path rests on, the corrected version is spelled out below. -#### Which hook does what - -`post-receive` runs **after** refs have already been updated; its exit status is -ignored. Rejection must therefore happen earlier: - -- **`update`** (per-ref, runs before the ref moves, can reject) — enforces the - refs rule: an agent token may only touch `proposals/*`; the approved branch - accepts fast-forwards from you only, never a force-update. Also where - frontmatter and global-ID-collision validation rejects a bad push. +#### Which hook does what — three, not two + +An earlier draft specified two hooks and put the `skip-validation` escape hatch +in `update`. **That is unimplementable**, for two independent reasons found by +building it: + +1. **Push options are not visible to `update`.** `githooks(5)` documents + `GIT_PUSH_OPTION_COUNT`/`_N` for `pre-receive` and `post-receive` only, and + git observably runs `update` with them unset. The escape hatch was therefore + unreachable where the design put it. +2. **`pre-receive` cannot read the pushed objects.** During `pre-receive` they + live in `GIT_QUARANTINE_PATH`; a process opening the bare repo normally gets + "could not get object info". git migrates the quarantine out *after* + `pre-receive` and *before* the first `update`. So `update` is the earliest + hook at which the daemon can actually read what was pushed — the design's + choice of `update` as the rejecter is correct, but for a reason it never + stated. + +Neither hook can do the whole job, so the work splits across three: + +- **`pre-receive`** — carries the push options (the only place they exist) and + acts as an early liveness check. Cannot validate content: the objects are + quarantined. +- **`update`** (per-ref, before the ref moves, can reject) — enforces the refs + rule: an agent token may only touch `proposals/*`; the approved branch accepts + fast-forwards from you only, never a force-update. Also where frontmatter and + global-ID-collision validation rejects a bad push, recalling the skip flag + `pre-receive` recorded. **Your own pushes are validated too**, with a `--push-option=skip-validation` escape hatch. The failure mode being guarded is not malice, it is a typo: a @@ -264,6 +283,18 @@ ignored. Rejection must therefore happen earlier: can never lock you out of your own repository. - **`post-receive`** (after the fact, cannot reject) — notify the daemon so it rebuilds that **space** at its new revision and updates the index rev stamp. + +The three are correlated by `(repository, receive-pack pid)` plus an exact +ref-update match, so a recycled pid cannot be mistaken for a live push. The new +failure mode this introduces is honest and fail-closed: a daemon restarted +between `pre-receive` and `update` rejects with "the daemon did not see the +pre-receive phase of this push; push again". + +**Per-ref rejection is not atomic.** `update` runs once per ref, so pushing +`main` and `proposals/7` together with `main` rejected still lands +`proposals/7`. Making it all-or-nothing would require deciding in `pre-receive`, +which cannot read the objects. Accepted, and stated here so it is not discovered +during a partial push. Note the unit: a space at a revision, not a set of changed documents. An earlier draft said "reindexes the changed documents", which contradicts the decision to absorb the batch rebuilder — and nothing tracks per-document change, @@ -350,7 +381,15 @@ a volume question, not an architectural one (see the reuse inventory). ## The three planes -### 1. Read plane (anonymous-capable, cached) +### 1. Read plane (authenticated, cached) + +**Audience, settled.** An earlier draft called this plane "anonymous-capable" +while the verification checklist required "anonymous request → no content +leaks". With visibility levels deleted from v1 there is no knob reconciling +those, so the read plane is **fail-closed**: the owner and agent tokens read; +everyone else is redirected to meta login, or gets 401 on the machine formats. +A publicly readable corpus is a one-line change if that is ever wanted, but it +has to be a decision rather than an ambiguity. `GET /~user/space/specs/0007-storage` renders. Content negotiation gives `.md` raw, `.json` metadata+body, `?rev=` pinned to an immutable revision. @@ -361,8 +400,19 @@ extension is a *format selector*, so the document's own address carries **no** extension: `/~user/space/specs/0007-storage` renders, `+ .md` is raw source, `+ .json` is metadata plus body. -**Reads default to the approved revision**, with a visible "draft is 3 changes -ahead" affordance. This is the plane bots consume; it must be boring and +**Reads default to the approved revision.** The rule is enforced in the shared +service layer, not in each surface: a read accepts only the approved-head +default or a **full 40-character object name**. Ref names are refused, because +`?rev=proposals/42` would otherwise make the *read* plane serve unreviewed text +that then flows into agent context as though approved — the precise failure this +service exists to prevent, and reachable by a crafted query string. Abbreviated +shas are refused too: one that is unique today can become ambiguous later, so a +pinned revision would silently stop meaning one thing. + +Reading a proposal branch is a real need for the review UI, so it exists as a +separate, deliberately awkward method rather than a flag — something a caller +asks for by name and a reviewer can grep for, never something a read surface can +be talked into. This is the plane bots consume; it must be boring and pinnable. Serving drafts by default would poison every downstream agent context with unreviewed text — which is the exact failure this whole service exists to prevent.