|
| 1 | +# ADR-0183 - The suite rule runs on the commit, not the working tree |
| 2 | + |
| 3 | +- **Date:** 2026-10-19 |
| 4 | +- **Status:** accepted |
| 5 | +- **Area:** `research-kit/lib/gate.mjs` (`runSuiteOnCommit`, `suiteBreach`) |
| 6 | +- **Supersedes in part:** ADR-0120, which ran "the checkout's `bin/selftest.mjs`": the runner is |
| 7 | + now the commit's own, run in a scratch repository built from the index being committed. The |
| 8 | + rest of ADR-0120 and ADR-0142 (scope, result file, budget, overrides) stands. |
| 9 | +- **Fires:** ADR-0024's deferred trigger. Its rejected "materialise the whole index" said: "If a |
| 10 | + future check needs a path outside `research/`, this scope widens with it." ADR-0120 was that |
| 11 | + check, and the scope did not widen. |
| 12 | +- **Freeze (ADR-0117):** a bug fix. The check judged the wrong bytes; no command, flag, config |
| 13 | + key or check is added. |
| 14 | + |
| 15 | +## Context |
| 16 | + |
| 17 | +ADR-0024 decided that the commit gate judges the index - the bytes that become the commit - |
| 18 | +and every other caller judges the working tree. `materializeIndex` reproduces the index's |
| 19 | +`research/` in a scratch directory, and the corpus verdict runs over that. |
| 20 | + |
| 21 | +ADR-0120 added the suite rule on 2026-10-01: in the kit's own checkout, a commit staging a path |
| 22 | +under `research-kit/` needs a green suite. `runSuiteHere(root)` spawned |
| 23 | +`<root>/research-kit/bin/selftest.mjs` with `root` as its cwd. That is the working tree: the |
| 24 | +runner, the tests and the library they load all came from disk, not from the index. |
| 25 | + |
| 26 | +Reproduced on 2026-10-19 with the shipped `suiteBreach`, on a repository holding the kit's four |
| 27 | +markers and a stand-in runner whose colour is in its own bytes: |
| 28 | + |
| 29 | +- **False block.** Staged: a green runner. Unstaged: a red edit to it. The gate blocked: "the |
| 30 | + suite is red: 1 failed". An unstaged work-in-progress test, a half-made edit or a README count |
| 31 | + bumped ahead blocks a green commit, and on the operator's PC each run costs 25-40 minutes. |
| 32 | +- **False pass.** Staged: a red runner. Fixed on disk only. The gate allowed it, and the commit |
| 33 | + carried the red file. A partial commit (`git commit <paths>`, `git commit -a`), whose index is |
| 34 | + a temporary `GIT_INDEX_FILE`, was judged by the working tree as well. |
| 35 | + |
| 36 | +The issue sweep that found this worked around it by committing kit changes only with the working |
| 37 | +tree equal to the index - a rule kept in a script, which is where ADR-0120 said such rules fail. |
| 38 | + |
| 39 | +## Decision |
| 40 | + |
| 41 | +The suite runs in a scratch repository built from the index being committed |
| 42 | +(`runSuiteOnCommit`, the default run of `suiteBreach`): |
| 43 | + |
| 44 | +1. In the hook's environment - so a partial commit's temporary `GIT_INDEX_FILE` is the index |
| 45 | + read - `git ls-files --stage -z` lists the index's entries, `git rev-parse --git-path objects` |
| 46 | + names the object store, `--show-object-format` its format, and |
| 47 | + `git rev-parse --verify -q HEAD^{commit}` the parent (none on a first commit). An entry added |
| 48 | + with `git add -N` (intent-to-add) is listed by `ls-files` as the empty blob and left out of |
| 49 | + the commit, so the paths the commit holds are read with `git diff-index --cached |
| 50 | + --ita-invisible-in-index` against the empty tree, and only those entries are kept. The index |
| 51 | + is only read. An index with unmerged entries blocks: `git commit` refuses one before its hook |
| 52 | + runs, and `checkout-index` would skip the conflicted paths without an error. |
| 53 | +2. Under `tempBase()`, in a folder named `research-kit-commit-` and six characters, addressed by |
| 54 | + its long name (Windows hands a temp folder over as an 8.3 short name - `RUNNER~1` on CI - and |
| 55 | + git reports the long one, so repo-hygiene's tests would not know their repository and would |
| 56 | + return early), with the repository-location variables stripped (`GIT_DIR`, `GIT_INDEX_FILE` |
| 57 | + and kin, the list ADR-0120 strips from the suite): `git init` without templates and in the |
| 58 | + commit's object format (the machine's default may differ, and a sha256 scratch refuses a sha1 |
| 59 | + index); the real object store, as an absolute path, in `objects/info/alternates`; |
| 60 | + `git update-index -z --index-info` with the entries; `git checkout-index --all --force -u` |
| 61 | + with `core.longpaths`; and `git update-ref HEAD <parent>` when there is one. |
| 62 | +3. The runner is the scratch's `research-kit/bin/selftest.mjs`, with the scratch - or the |
| 63 | + project's place in it, for a project below the top level (#309) - as its cwd. The result-file |
| 64 | + contract, the stripped environment and the budget are ADR-0120's and ADR-0142's. |
| 65 | +4. After the run the index is listed again. A plain `git commit` holds no lock on the index |
| 66 | + while its hook runs and commits what the index holds when the hook returns, so a `git add` |
| 67 | + during the run would reach the commit untested: a changed index blocks, green or not. |
| 68 | +5. The scratch is removed afterwards. A gate killed mid-run (Ctrl-C, the hook's watchdog) never |
| 69 | + reaches that step, so each run first removes the folders of step 2's name that hold a `.git` |
| 70 | + and are more than a day old - its own leftovers, never a scratch a gate in another checkout |
| 71 | + is still using. |
| 72 | + |
| 73 | +The scratch is a repository of its own, at the real HEAD, holding the commit: its |
| 74 | +`git rev-parse --show-toplevel` is itself, so repo-hygiene's tests run instead of returning early, |
| 75 | +and its `git status` shows what the commit stages and nothing else. |
| 76 | + |
| 77 | +A scratch that cannot be built is a block that names the failed step and git's first line of |
| 78 | +error, as an index the gate cannot read blocks (ADR-0024). It never falls back to the working |
| 79 | +tree: that fallback is the defect. Where the project has no `.git` there is no index, and the |
| 80 | +files there are judged, as before: a fixture or a copy. The one hook that meets this is a |
| 81 | +repository whose `.git` lives elsewhere (`--git-dir`, `core.worktree`), where the suite still |
| 82 | +runs in place; the kit's checkouts do not use that layout. |
| 83 | + |
| 84 | +Measured on 2026-10-19 on this repository's tree (1743 tracked files, Windows 11): the scratch |
| 85 | +built in 9.3 to 10.8 s over three runs and was removed in under a second. The full suite, run in |
| 86 | +such a scratch on this change before review: 2052 passed, 0 failed, 16 unsupported, in 1653 s - |
| 87 | +inside the 1409 to 1825 s measured in the working tree (ADR-0142). Built under a 110-character |
| 88 | +temp root without `core.longpaths`, 98 captures failed with "Filename too long" (the longest |
| 89 | +tracked path is 172 characters) - so the scratch sits directly under `tempBase()`, sets |
| 90 | +`core.longpaths`, and a checkout that fails blocks. |
| 91 | + |
| 92 | +## Alternatives rejected |
| 93 | + |
| 94 | +- **Materialise `research-kit/` only, as `materializeIndex` does `research/`.** The tests read |
| 95 | + outside it (the README count, the docs, the ADR index), and without a `.git` repo-hygiene's |
| 96 | + tests return early - a silent green. |
| 97 | +- **A plain `checkout-index` copy, no repository.** The same silent skip. |
| 98 | +- **`git write-tree`, then `read-tree` in the scratch.** One step shorter, and it leaves |
| 99 | + intent-to-add entries out by itself, but `write-tree` may write the cache-tree back |
| 100 | + into the index being committed: the gate would modify the commit's index to form an opinion |
| 101 | + of it. `ls-files` only reads. |
| 102 | +- **`git stash --keep-index`, or any other step through the working tree.** It mutates the |
| 103 | + operator's files; ADR-0024 rejected it outright. |
| 104 | +- **Refuse, or warn, when the index and the working tree differ.** ADR-0024 rejected both: the |
| 105 | + divergence is a normal workflow, and a gate that forbids it is removed. |
| 106 | +- **Pass `GIT_INDEX_FILE` through to the suite.** The suite builds scratch repositories of its |
| 107 | + own, and every one of their `git add`s would act on the commit's index (ADR-0120). |
| 108 | +- **`git worktree add --detach`.** It writes the real repository's administrative files from a |
| 109 | + hook, leaves them behind when interrupted, and checks out HEAD, not the index, so the index |
| 110 | + would still have to be read in. |
| 111 | +- **Fall back to the working tree when the scratch cannot be built.** A gate that answers a |
| 112 | + question it could not ask is the defect this decision removes. |
| 113 | +- **Remove a leftover scratch on a signal.** `spawnSync` blocks the gate's event loop for the |
| 114 | + whole run, so no handler runs while the suite does, and the watchdog's kill cannot be caught. |
| 115 | + |
| 116 | +## Consequences |
| 117 | + |
| 118 | +- A commit owing the suite pays for the scratch (about 10 s on this tree) before the run. The |
| 119 | + build is not counted in the suite's budget; it fits in the rest of the gate's allowance |
| 120 | + (`GATE_REST_S`, 120 s, ADR-0142). |
| 121 | +- Untracked files no longer reach the suite. A test that passed only because of a file nobody |
| 122 | + committed now fails at the commit, as it would on CI. |
| 123 | +- Staging while a commit's suite runs blocks that commit; commit again. |
| 124 | +- A run stopped at its budget can leave its scratch when a child still holds a file (Windows), |
| 125 | + as a stopped runner already leaves its own (ADR-0142), and a killed gate leaves all of it - |
| 126 | + a copy of the checkout. The next run removes either after a day. |
| 127 | +- An intent-to-add path is not in the tested tree, as it is not in the commit. `git status` and |
| 128 | + `diff-files` were not used to find one: they report it as added, or as a rename or a deletion |
| 129 | + once its file is gone, and `diff-index` without `--ita-invisible-in-index` lists it as held. |
| 130 | +- The corpus verdict is unchanged: `materializeIndex` still reproduces `research/` only. The |
| 131 | + whole-index scratch is built only when the suite is owed. |
| 132 | +- The edit gate, and `bin/selftest.mjs` run by hand, still judge the working tree - ADR-0024's |
| 133 | + "every other caller". |
| 134 | +- The hook runs the deployed kit's gate (ADR-0120): until `~/.agents/research-kit` carries this |
| 135 | + `lib/gate.mjs`, the deployed gate still runs the working tree, and kit changes are committed |
| 136 | + with the working tree equal to the index. |
0 commit comments