## Pairs that must not collapse

Every correctness bug this tool has had is one of a small set of pairs
being treated as one thing. They are listed here because the list is
short, it has not changed in a long time, and reading it before touching a
read path is cheaper than rediscovering an entry:

- **absence and ignorance.** "No record says so" and "no record I could see
  says so" are different facts. `pr close` read the author's PDS and not the
  repo owner's, concluded a merged pull was open, and wrote `closed` over the
  merge. A pull somebody else had stacked on read as "not stacked".
- **decided and unknown.** A knot that refuses has done nothing; a knot that
  dies may have merged. Both were reported as "retry later".
- **stale and current.** `stack create` compared a pull's patch shas against
  branch commits that `--add-change-ids` had just rewritten.
- **identity and incidental context.** Two reports of the same chain damage
  differed only by which member the walk started from, and the writer read
  that as new damage.
- **one authority for two questions.** `stack merge` inherited an ownership
  check from the verbs that write member records, and refused the repo owner
  a merge it writes no member for.

The tool already has the abstraction for the first of these — `Evidence`,
`Reach`, `State::Unknown`, `Listing` — and every instance of it has been in
code that did not use one. A value derived from a partial read should carry
its reach; where a function returns a bare `Vec` or a bare enum from a
listing, that is the thing to check.

## The best fix is not to have the partial view

`understack_patches` in `cmd/pr/review.rs` is the one place that sidesteps
"absence and ignorance" entirely rather than reporting it: it walks
`dependentOn` record by record with direct `getRecord` reads, so there is no
listing, no page cap and no index to be behind. A cycle is refused, a
malformed link is refused, and an unreadable member stops the checkout
rather than producing a working tree missing a patch.

Carrying `Evidence` is the right answer when a listing is genuinely needed —
"every pull aimed at this repo" cannot be walked link by link. Where the
question is "what does this record point at", the links are the answer and
no listing should be involved. Ask which kind of question is being asked
before reaching for a listing.

## The rule about the rules

Every entry above was one rule with several copies, or one rule in the wrong
place. So the fix for an instance is not finished when the instance stops
happening — it is finished when there is one copy of the rule, somewhere that
owns it.

Two of these were written twice on the same day. "Absence and ignorance" was
fixed in `pr close` and then found again in `stack view`'s "not stacked".
"Decided and unknown" was fixed in `stack merge` and then implemented a
second time, by hand, in `repo delete`. Both second copies were written by
somebody who had just written the first, which is the strongest evidence
available that noticing a pattern is not the same as removing it.
`knot::decided` is the one definition now, and it takes a tag and a status
rather than an error, because its two callers hold different things and the
rule is about neither.

## Fixing one copy is not fixing the bug

**Every fix starts by grepping for the other implementations.** Not after —
before. This was learned the expensive way, seven times:

- `target_branch_of` was made strict on the stack path while `pr merge` kept
  `unwrap_or("main")`, feeding the same `MergePlan` to the same knot call.
  The doc comment on the strict one even named the bug it was fixing.
- "Newest state wins" had *five* orderings; three decided state and all three
  differed. Two carried doc comments asserting they agreed with each other.
- "No PDS endpoint in the DID document" is one sentence with an
  `Exit::NotFound` owner and six hand-rolled copies that exit `1`.
- The `@handle`-or-DID rule was centralised, and copies kept turning up for
  days afterward.

A subagent sweep found three of these in one pass, and grepping properly
found two orderings the sweep had missed. So: name the rule, `grep -rn` for
every place that decides it, fix them together or write down why one of them
is deliberately different. A fix applied to one copy leaves the codebase
*less* consistent than before, because now the copies disagree.

**Then ask why there were copies.** Grepping is the fix for the instance;
it is not the fix for the pattern, and it had been written down here for a
while before the pattern kept happening anyway. Every rule on that list is a
rule about what a *record* means, and `src/` was organised by verb, so none
of them had anywhere to live: `cmd/` is forty thousand lines, and a rule you
cannot find is a rule you rewrite. `model/` is where they go now — see
docs/module-layout.md. Duplication that keeps coming back is usually a
missing module, not a missing habit, and a rule that has an obvious home
gets found instead of rewritten. `cmd/stack`'s chain reader is the evidence:
it became that home by accident and its rules stopped diverging.

## A mock that ignores who is asking cannot catch asking the wrong one

The mock PDS served every blob to every `did`: `getBlob` looked the CID up in
one global map and never read the `did` parameter. Everything about that is
convenient and it silently voided a whole class of test.

`stack merge` exists so a repo owner can land a *contributor's* stack, and it
read every member's patch blob from the **merging** account's PDS. A real PDS
answers 404 — the blob is in the contributor's repository — so the command
could not do the one thing it is for.
`the_owner_merges_a_contributors_stack_from_their_own_pds` covered exactly
this flow and passed, because in the harness there was no wrong PDS to ask.

The mock now records who uploaded each blob and 404s anyone else, and with
the fix reverted that test fails with the message a maintainer would really
have seen. The rule generalises past blobs: **when a mock collapses an axis
the real service distinguishes — which account, which host, which repo — no
test can fail along that axis**, and a green suite says nothing about it. A
service that answers every question the same way is the empty-service
mistake in a costume: see the next section.

## Three accounts, three hosts, and a repo none of them is

Tangled is multi-account by construction: a record lives in its author's own
PDS, a repo belongs to one account and is contributed to by others, and no
account can read or write another's repository. A fixture that does not model
that cannot fail along it.

The harness used to have two accounts sharing **one** PDS endpoint, so:

- "ask the right account's PDS" was a question with one answer, and
  `stack merge` had been asking its own host for a contributor's patch blob
  for as long as the command existed;
- "write only into your own repository" was unenforced — the mock checked
  that *a* token was present, never that it belonged to the repo being
  written to.

Now every account is served by its own mock on its own port, answering only
for itself; a request naming anyone else gets a 404, and a write whose bearer
is not the repo's owner gets a 403. Both are what the real services do. The
axes are the point: `Scenario::new` gives Alice, Bob and Carol, and
`Scenario::upstream` moves the repo to Carol so the two contributors are
aiming at a repository *neither of them owns* — the case where "the owner's
PDS" and "my PDS" stop being the same host, and every rule that tells them
apart finally has to be right.

Prefer `Scenario::upstream` for anything about ownership, standing, merging
somebody else's work, or which host holds a record. A test written against a
fixture where the acting account owns everything is a test that cannot tell a
correct answer from a lucky one.

## Practices these came out of

- **Mutate before believing a test.** Three tests in this suite passed for
  the wrong reason and were caught by deliberately breaking the code they
  covered. A green test is evidence only once it has been seen to fail.
- **Model a service that fails, not only one that is empty.** An empty answer
  is evidence about the world; a refused request is evidence about nothing.
  `World::bobbin_fails`, `web_fails`, `knot_fails` and
  `Checkout::refuse_pushes` exist for this and each found untested handling.
- **Agreement is not correctness.** `assert_stack_reads_alike` checks that
  atgc and the appview read the *same* chain; two readers agree perfectly
  well on a chain that is wrong. Name the shape as well.

# Testing

`cargo test` runs everything and takes about eight seconds. No spindle is
attached to this repo, so `prek run --all-files` plus `cargo test` locally is
the actual gate, whatever `.tangled/workflows/ci.yml` claims.

**What earns a test.** Three things.

Parsers of other people's data. Every `sh.tangled.*` record, DID document,
Bobbin response and knot redirect is written by software this project does not
control, and the code is permissive about them on purpose, which is the kind of
parsing that fails quietly.

Things that are fiddly out of proportion to their size, like a base64url
decoder or character-safe truncation. Short enough to look obviously right, and
with no natural manual check.

Anything a commit message had to justify, because the paragraph will not
survive the next refactor but the test will.

Glue does not earn one: `main.rs` dispatch, the layout of a `println!`, the
four lines around a request whose parser is already tested. That kind of
coverage costs the same to maintain as a real test, goes red for unrelated
reasons, and creates the impression that a green suite means the tool works.

**Hermetic, non-negotiable.** No network, no OAuth, no session, no reading or
writing the real `~/.config/atgc`, no touching the user's git config, no
assuming a particular Tangled repo exists.

Two traps are specific to this crate. `HOME` cannot be redirected inside the
test process, because that needs `std::env::set_var`, which is `unsafe` in
edition 2024 and the crate forbids unsafe code. So a function that reads `HOME`
itself is not unit-testable; take the directory as a parameter and leave one
untested wrapper that names `$HOME`. And the working directory is process-wide
while cargo runs tests on threads, so `testutil::TempRepo` takes a module-level
mutex and must stay the only thing in the tree that moves the process.

Network access is allowed in exactly one place: a test marked `#[ignore]` whose
message says why. If an `#[ignore]` ever pins a known bug rather than a slow
socket, its message must say so, because "ignored" reads as "unimportant" and a
green suite standing over a broken function is worse than a red one.

**Fixtures for reads, mocks for writes.** Test a parser against real bytes
captured off the real service, never against a mock. A mock encodes what we
believe the service returns, so a test of a reader against it passes exactly
when our belief is self-consistent, which is the thing already known.

A write command asks the opposite question: what atgc *sent*, in what order, as
whom. That is atgc's own behaviour, which no fixture can observe.
`tests/support/` is the environment for it, with mock services on loopback
ports, a session that restores with no network, and the real binary spawned as
a child. Spawning a child is what makes an alternate `HOME` possible at all,
since `Command::env` is not `unsafe`. Reach for it whenever the bug is about
how two commands interact rather than about one function's output: a round
appended to the wrong record, a chain linked to something outside its own
batch, a write made with the wrong account's credentials.

**A model of somebody else's service is a third thing.** Fixtures and mocks
both answer "what did atgc do?". `tests/support/appview.rs` answers "what does
the service that reads these records make of them?", which is a question no
capture and no mock of our own can be asked — a stack can satisfy every rule a
PDS enforces and still be one tangled.org draws wrongly. A model like that is
only worth what its reading of the upstream source is worth, so it names the
file and the date it was read from, and it is written to look like the code it
models rather than like code we would have written.

**Keep this page this short.** Do not add per-feature notes, a list of what is
verified by hand, or an inventory of what is not covered. That kind of writing
goes stale the week after it lands and nothing checks it. This page once ran to
535 lines and named six source files that no longer existed. A fact about one
test belongs in that test's doc comment; how a fixture was captured belongs
beside the code that loads it; a claim that something is unproven belongs at
the claim, in the code, where a reader is standing when it matters.