ADR-0030: Review ingestion spawns gh, over public allowlisted repositories only

  • Status: proposed
  • Date: 2026-09-05
  • Deciders: Theurian maintainers
  • Requirements: FR-V1, FR-V5, FR-V6, SEC-9, SEC-10, SEC-11, SEC-15, SEC-19, T-3, T-7
  • Decision recorded in #479, the Milestone 8 design-first step for the FR-V GitHub-API arm
  • Situates against ADR-0004 — whose Never Git-tracked (derived) list named "raw GitHub review caches", and which this ADR amends in place: decision 3 withdraws that entry and adds a fourth category, ingested content that is authored upstream and not rebuildable — ADR-0013 (nothing ingested here becomes approved knowledge), ADR-0019 (ingested labels are data, not governance), ADR-0026 (a served review thread is evidence; it gates nothing), ADR-0027 (the pre-landing secret scan this one mirrors), and ADR-0029 (the git-native floor of FR-V, which named this arm as explicitly not its own)

This ADR records a decision and ships no code. The adapter, the environment construction, the caps, the version floor, the evidence files, the scan gate, the serving store, review.search, the capability flag, the T-7 rewrite and the prose-population sweep are all deferred to the slices named in Slicing plan and Compliance. Nothing here modifies SCHEMA_VERSION, INDEX_SCHEMA_VERSION, FINDINGS_SCHEMA_VERSION, any *.schema.json, or any *.py; the diff is confined to docs/.

Two measurement frames, kept apart on purpose.

  1. The measurements that needed the network are quoted, never re-run here — this document's own change performs no network call at all. Two sources, each named where it is used, both on 2026-09-05 against gh 2.86.0:
Source Members
The design consult runs A, B, C (decision 1); the PullRequestReviewThread schema introspection (decision 5); the configuration-key observation, http_unix_socket versus api_host (decision 1); the last-40-merged thread count (Dispositions)
Round one's adversarial review runs D, E, F (decision 1), of which D was re-run by the orchestrator and reproduced; the by-PR-number thread population (Dispositions), also re-run by the orchestrator

They say what was true of that binary and that repository on that date. Slice 1 re-runs A–F as driving tests, which is where they stop being quotations and become controls. 2. Every repository measurement was taken on 2026-09-05 against origin/main @ 1fe3302b, and each names the command that produced it. Where a measurement is a population, the key is stated beside the number so a reader can attack the key and not only the count.

Context

ADR-0029 built the git-native floor of FR-V: Review-Finding: trailers parsed out of local history, landed in their own store, served as review.findings. It bounded itself explicitly — "the GitHub-API arm (threads, inline comments, resolution state, CI results, LLM candidate generation) is broader, needs credentials, and is not this ADR's to design". This ADR is that arm's design.

What exists today, measured. The domain model is built and nothing fills it:

Fact Command (2026-09-05, origin/main @ 1fe3302b) Result
The ReviewProvider GitHub adapter cat packages/theurian-core/src/theurian/infrastructure/github/__init__.py a package docstring that says "Not implemented. This package holds no adapter and no HTTP client"
The capability the server publishes grep -n 'reviewIngestion' packages/theurian-core/src/theurian/mcp/tools.py "reviewIngestion": False (line 1905)
The allowlist key git grep -n 'providers\.review\.repositories' -- packages/theurian-core/src 2 hits, both in security/project_config.py's own docstring prose (:5, :11) — no reader anywhere
The tool names already published as planned the planned-tools table in docs/protocol/mcp-tools.md, lines 352–358 (command and output in Compliance) 7 rows — review.findings shipped, six planned, review.search among them
Places in the shipped package that can start another program the pinned set in tests/unit/test_network_call_sites.py (PROCESS_SPAWN_SITES) 3: cli/context.py, infrastructure/git/trailer_source.py, infrastructure/services/runner.py — none of them reaches a network

T-7 has had the wrong owner twice, and the change that ends that is slice 1 — not this document, which ships nothing and can only name the owner. docs/security/threat-model.md records three controls for SSRF — a scheme allowlist, private-network rejection, and the repository allowlist in .theurian/config.yaml — and none of the three is built. #129 was closed having corrected the entry's wording; the audit then repointed the controls at #368, and review found that

368 builds no fetch path either. The threat model states the lesson in its own

words: "an owner has to be the change that would implement the control, and an epic in the right milestone is not automatically that." #429 now holds the three against whatever first performs an external fetch. Slice 1 is that change for the gh activation context, where one of the three is discharged and one is reduced with a recorded residual; Dispositions states which is which, in which context, and what stays #429's.

What stands in for the controls today is an absence, and the first spawn site deletes it. tests/unit/test_network_call_sites.py pins, by equality against the whole set, that nothing in the shipped package outside three fixed-argument spawn sites and the daemon's own loopback health probe can open a connection. Its process-spawn arm was added precisely because a gh api adapter "would contain no client module at all" — a mutation that replaced a fetch with subprocess.run(["curl", ...]) survived the entire suite with the network enumeration green. That file states the admission checklist this ADR has to satisfy, and it is quoted here rather than paraphrased — the source is a concatenated Python string, joined here into prose with its -- set as an em dash and nothing else changed (test_network_call_sites.py:653-661):

If you added a site, establish before listing it: the argument vector is fixed by the adapter rather than taken from a document or a configuration file; the command cannot be handed a URL or a remote; there is a timeout; and a test goes red when any of those stops holding. If the command is meant to reach the network — the Milestone 7 gh api shape — then the repository allowlist is due in the same change, along with the documents that currently promise nothing fetches: docs/security/threat-model.md (T-7) and the infrastructure/github/ package docstring.

So the design problem is not "how do we call GitHub". It is: the moment a spawn site is admitted, an absence that many documents rest on stops being true, and something has to hold what the absence was holding. Decision 1 is the replacement, stated in the positive.

Decision

1. GitHub is reached by spawning gh api graphql, and the absence control is replaced by a positive invariant

The adapter reaches GitHub by spawning the operator's gh binary as an argument vector. The absence control retires at the commit that adds the site, and the ten clauses below take its place. Each clause is a property, and each owes a test that goes RED when it stops holding — an invariant with no test is what the absence was already better than.

# The rule What it holds Owed test, and its slice
1 Exactly one module may reach GitHub. The spawn site lives in infrastructure/github/, and the pinned spawn-site set grows by exactly that one entry. A second fetch path added later on a page nobody re-reads; the equality pin catches an addition and a removal. The existing PROCESS_SPAWN_SITES equality assertion, extended by one entry, plus a test that no other module in the shipped package names the adapter's spawn helper. Slice 1.
2 The endpoint is the literal graphql. Repository identity travels as typed GraphQL variables, never in the URL position. The gh api <path> form interpolates caller data into a path; the GraphQL form has no path segment an owner or repo name can escape into. It also means no raw URL exists for T-7's scheme allowlist to be needed on. A test that the argument vector's endpoint element equals graphql byte-for-byte, and that no element is derived by string-formatting a repository name. Slice 1.
3 The destination host is pinned by an explicit --hostname github.com. An inherited GH_HOST silently moving the request to another host — measured to move it (run B). A test asserting the flag and its value are present in every spawned vector. Slice 1.
4 The child environment is CONSTRUCTED from a closed enumerated constant (env={...} passed to the spawn), never inherited and never merely scrubbed. The membership is the table below, derived from its own Value column; the one open platform question — a Linux credential-store member — is reconciled there by a slice-1 CI measurement, not left to an implementer's judgement. The measured attack class below: destination and identity taken from inherited environment. A scrub is a blocklist, and a blocklist has to be right about every variable gh and its transport stack read; a constructed environment has to be right about the few Theurian deliberately passes. (i) an equality test — the child's environment mapping equals the expected mapping written out test-side, not imported. Membership is by key and value, and an empty string is a present key, not an absent one: gh treats an empty config-locating variable as absent and falls through (decision 1, member (d)), so the two are not interchangeable and the test pins which one the constant carries. It is a strong assertion: it goes RED for every wrong mapping, including a passed-through parent variable, a missing member and a wrong value, and round two's adversarial review drove it RED five ways. The test-side restatement is what makes it killable, and this is the correction of an inverted claim: a test that reads the production constant moves with it, so mutating the constant changes both sides and the test SURVIVES — measured by round three's adversarial review with toy models, 2026-09-05. The killable forms are a test-side expectation or a source scan, which is the shape PROCESS_SPAWN_SITES already uses in this repository (clause 1). (ii) the run-D-shaped fixture — a config directory carrying http_unix_socket, reached through a forwarded config-locating variable — split in two, because its halves have different requirements. (ii-a) the refusal driver: with the fixture in place the adapter refuses before spawning, so it needs no gh at all and is never skipped — a pre-spawn refusal that never reaches the binary is testable on any machine. (ii-b) the residual demonstration: with the refusal bypassed through a test seam — an injection point the test supplies, never a shipped flag or configuration key (a runtime switch that disables a security check is a second way to reach the exposure) — the request leaves through the socket. That half needs a real gh, so it is skipped where the binary is absent and the skip is reported rather than counted as a pass. Only (ii-b) is skippable, and the driver for the control is not. Slice 1.
5 The gh binary is resolved to an absolute path, and the vector is passed with shell=False. SEC-9 verbatim: "Never build a shell command by string concatenation. git and gh are invoked as argument vectors with shell=False" (requirements-analysis.md:236). An unresolved name would also let the child's PATH choose the executable; clause 4 means whatever PATH the child sees is one this project constructed, and clause 5 means it is not consulted for the executable at all. A test that the first vector element is an absolute path and that shell=True appears nowhere in the module. Slice 1.
6 No --paginate. Every page after the first is requested by handing back a GraphQL cursor in a typed variable, with the vector otherwise unchanged. --paginate exists to follow a next-page reference the response supplies. Exactly what it follows, and how, is behaviour of a binary this design does not pin (clause 8 bounds only its version) — and that is the reason the flag is excluded rather than characterised: a destination the response chooses is the shape T-7 names, and a cursor in a typed variable cannot become one. A test that --paginate is absent from every spawned vector, and a cursor-pagination test over a recorded fixture. Slice 1.
7 A request timeout (SEC-19) and recorded ingest cost bounds: a page cap and a PR-count cap, each a named constant. Exceeding a cap is a reported, graded stop, never a silent truncation and never an unbounded loop. A caller — or a large repository — making the system spend work no recorded limit bounds. The severity table grades exactly that as HIGH, and #26's T-6 concurrency cap is the precedent for how such a bound is recorded: a constant, a test, and prose that names the number. A test per cap that the constant is the value the adapter uses, and a test that a fixture exceeding the cap stops with a report. Slice 1.
8 A gh version floor, expressed as a constant with a test, not as prose. gh is not a Python dependency, so ADR-0014's exact pinning does not reach it; the behaviours clauses 2–6 rely on are flag and config behaviours of a binary the operator upgrades independently. Prose asking for "a recent gh" is not a control. A test that the adapter refuses to spawn below the floor, and that the floor is the constant the refusal message names. Slice 1. Measured against 2.86.0 — the floor is chosen at implementation, not asserted here.
9 gh absent, or present and unauthenticated, is a graded refusal envelope with a remedy — never a traceback. The child's stderr surfaces only inside that envelope, never straight to a log or a caller. The failure the product already has a shape for: requirements-analysis.md:328-329 records Degraded as "a success-with-warnings terminal state, not a failure: a missing gh token must not prevent local knowledge from working." Ingestion is the optional capability; the rest of the product keeps working. The stderr half is the GH_DEBUG=api shape — a debug-verbose child can print request detail, and the environment constant of clause 4 excludes GH_DEBUG, so the two halves close it together. A test for each of the two states asserting a refusal envelope carrying a remedy and no traceback, and a test that child stderr reaches no sink outside the envelope. Slice 1.
10 A per-response byte cap, as a named constant, with a typed refusal when a response exceeds it — and the read shape that makes the cap real: the child's output is read incrementally against the cap, not accumulated first and measured after. An unbounded capture_output of third-party bodies is the same unbounded-work class clause 7 covers for request count, one layer down: a repository's comment bodies are content Theurian does not control. The product already records a bar for a bounded read — MAX_SOURCE_FILE_BYTES (8 MiB, security/paths.py:45), used by ingestion_service.py:180 — so the cap is set beside a recorded number rather than invented. A test that a response past the constant is refused with the typed refusal and not truncated silently, and a test that the read does not buffer the whole response before deciding, spelled so it can fail and so a wrong implementation reddens rather than hangs: a child that emits the cap and then blocks, with the assertion under a bounded wait — an implementation that accumulates first exceeds the deadline and fails; one that reads incrementally refuses at the cap and returns inside it. Slice 1.

The attack class clause 4 exists for, measured

Three runs, quoted from the design consult of 2026-09-05, gh 2.86.0. Runs B and C are the positive controls: without them, a clean run of A would be consistent with the threat not existing at all.

Run Command shape Observed
A GH_HOST=evil.test gh api --hostname github.com graphql … request went to Host: api.github.comthe --hostname pin holds against GH_HOST
B GH_HOST=evil.test gh api graphql … (no pin) error connecting to evil.testthe threat is real, and A's result is a pin doing work rather than a variable being ignored
C HTTPS_PROXY=http://127.0.0.1:9 gh api --hostname github.com graphql … proxyconnect tcp: dial tcp 127.0.0.1:9the pin does not cover this class: the request was routed to loopback with the hostname pin in place

Three further runs, quoted from the adversarial review of 2026-09-05 (same binary, gh 2.86.0); run D was re-run by the orchestrator before this text was written and reproduced. They are recorded at the fidelity the round reported them — each run's shape and its result — and slice 1 re-runs them as clause 4's positive control:

Run Shape Observed
D --hostname github.com pinned, only allowlisted variables set, GH_CONFIG_DIR pointing at an isolated config directory whose config sets http_unix_socket the request went to the unix socket
E the same config reached through HOME instead the same
F the same config reached through XDG_CONFIG_HOME instead the same

Runs B, C and D–F are each a positive control for a different half of clause 4: B shows the GH_HOST threat is real rather than ignored, C shows the --hostname pin does not reach the proxy family, and D–F show it does not reach the config file either. Together they are why clause 4 is construction and not scrubbing of GH_HOST: the destination is decided by at least three independent inputs, and a defence that enumerates what an attacker may not set has to be complete about all of them.

The admission rule, derived from the table's Value column

A rule that contradicts its own table is not a rule:177 below states that standard, and three drafts have now died by it. The first two split the environment by what a variable carries: credentials may pass, destination-movers may not, which admits the GH_TOKEN family — identity from the caller's environment, the second half of the class clause 4 exists to close — and then passed the config-locating variables anyway, which runs D–F show to be destination-bearing. It excluded exactly the variables it admitted. The third replaced it with a single necessity test ("admitted only if gh cannot locate the operator's persisted authentication without it") and then admitted five rows that fail it: NO_COLOR, the three notifier and prompt variables, and PATH.

So the rule is not written over the table and then checked against it. It is derived from the table's Value column, and that column has three forms — which is what the table shows, not two:

Value form Admission ground Rows
forwarded by value — the parent's value crosses into the child unchanged necessity: admitted only if gh cannot locate the operator's persisted authentication, or its platform credential store, without it HOME, GH_CONFIG_DIR, XDG_CONFIG_HOME
set to the literal 1 — a constant carrying no parent data and no value choice a named operational property, stated per row, with its transport reach enumerated beside it NO_COLOR, GH_NO_UPDATE_NOTIFIER, GH_PROMPT_DISABLED, GH_NO_EXTENSION_UPDATE_NOTIFIER
set by Theurian to a fixed value that is not forwarded — a value this project chooses, where the choice itself is the decision the operational property plus a pinned value: the value is fixed in the constant and never derived from the parent's, because a derived value would re-admit the parent's input through a row that reads as a literal PATH

PATH's ground is the third form, and the load-bearing half is the never-derived. An implementer may satisfy "a Theurian-constructed value" by building it from the parent's PATH — filtering it, prepending to it — and every word of the earlier wording would still be honoured while the parent chose the helper binaries again. So this ADR fixes the rule (a fixed literal, never derived from the parent's PATH) and leaves the exact literal to slice 1, which is also what makes clause 4(i)'s test-side expected mapping writable: an unpinned value has no expectation to write.

The grounds are asymmetric because the risks are: a forwarded value is an input the parent controls, so it has to be necessary; a literal carries no input at all, so it has to be useful and bounded; a chosen value is neither, so it needs both the property and the pin. Reach is enumerated for every row either way, in the open — reduced where a check exists, recorded as a residual where none does. The derivation cannot contradict the table, because every row's ground is read off that row's own Value.

The closed enumerated constant. The child environment is exactly the rows below, and the table is the constant — there is no "and similar":

Variable Value Why it is admitted Transport reach
HOME forwarded by value gh locates its config directory and the operator's persisted login through it reaches the config file — reduced by slice 1's pre-spawn refusal for the accidental single-well-formed-file case only, with the divergence class derived below
GH_CONFIG_DIR forwarded by value the same, when the operator sets it explicitly the same
XDG_CONFIG_HOME forwarded by value the same, on the XDG path the same
NO_COLOR 1 machine-readable output none
GH_NO_UPDATE_NOTIFIER 1 set, not merely absent: without it gh performs its own 24-hour release check — an outbound request no argument vector of ours chose (gh 2.86.0's documented environment, read in round two) removes an outbound request
GH_PROMPT_DISABLED 1 a spawned gh must never block on an interactive prompt none
GH_NO_EXTENSION_UPDATE_NOTIFIER 1 same class as the update notifier: a check nobody asked for removes an outbound request
PATH a fixed literal chosen by Theurian, never derived from the parent's (the literal is slice 1's; the never-derived is this ADR's) gh shells out (git, credential helpers); an inherited PATH would let the parent environment choose those binaries it selects which helper binaries gh runs — enumerated here as reach, not claimed to be harmless: a helper this project's own PATH value resolves is one this project chose

Nothing else. GH_TOKEN, GITHUB_TOKEN, GH_ENTERPRISE_TOKEN and GITHUB_ENTERPRISE_TOKEN would have to enter as forwarded by value, and they fail that form's ground: gh finds the operator's persisted login without them. So identity never comes from a caller's environment. Headless environment-token authentication is a recorded non-goal until somebody needs it, at which point it is a decision with its own reasoning rather than a variable quietly added to this table.

One platform question is open, and it is answered by measurement, not by guessing. On Linux the credential store is reached through a session bus (DBUS_SESSION_BUS_ADDRESS, XDG_RUNTIME_DIR), and whether gh can find a stored credential without them is not measured here — this machine is macOS. Slice 1 owes that measurement in CI, on Linux, and the constant gains a platform member only if the measurement says the credential is otherwise unreachable. The rule stays fixed either way: the equality test pins whatever the constant records on that platform, so "fixed membership" is a property of the rule, not a claim that today's eight rows are the final list.

The residual, on the measured storage facts

The premise an earlier draft rested on — the same directory holds hosts.yml, and so the operator's token — is false in gh 2.86.0's default configuration. Two measurements, both from round two and both re-verified:

  • The default credential store is the OS keychain. hosts.yml on this machine carries no oauth_token; a plaintext token in that file is the --insecure-storage fallback, not the default.
  • Redirecting and writing are different acts. Pointing a config-locating variable at a new directory loses the credential too — gh reports itself unauthenticated. Writing a transport override into the operator's real located directory does not: the keychain hands gh the token as usual, and gh hands the authenticated request to the attacker's socket. The credential is captured without ever being read.

So the exposure is not "an attacker who could already read the token". It is: an actor able to write the operator's own gh config directory can redirect an authenticated request and capture what it carries, without holding the credential themselves. Slice 1's pre-spawn refusal of known transport-override keys reduces that exposure — but only some of it. The residual is a class, not a list, and it is derived from one fact: the check's read cannot be gh's read. Every way those two reads can diverge is a member, and four are measured:

The two reads diverge on Member Why the check cannot reach it
when — the check reads at check time, gh at spawn time (a) the race — a writer landing a known key in that window Two reads of a mutable file with nothing holding it still between them
which keys — the check knows a fixed set, gh honours what its version understands (b) an unknown or future key, needing no race at all Clause 8's floor bounds the binary only below: a newer gh may honour a setting this check has never heard of
which bytes parse, and how — both read the same file and disagree about it (c) parser divergence. Measured, adversarial round 4, gh 2.86.0: with http_unix_socket present twice, PyYAML safe_load takes the last occurrence (the check sees no override) while gh dials the first occurrence's socket. A parse error is the same class in the other direction: a naive best-effort check fails open A YAML reader is not gh's reader; agreeing on the bytes is not agreeing on the value
which file — the check reads what os.environ.get suggests, gh resolves one directory by precedence (d) resolution divergence. Measured: gh reads exactly one directory — GH_CONFIG_DIR > $XDG_CONFIG_HOME/gh > $HOME/.config/gh — an empty-string variable falls through to the next, and a relative value resolves against the child's cwd Runs D–F moved one variable at a time and never measured the precedence, so a check that reads all three, or reads them in the wrong order, checks a file gh will not open

Members (c) and (d) need neither timing nor an unknown key: the same file, the same moment, and the check still answers a different question.

So the check is priced honestly, in one sentence: it reduces the accidental, pre-existing, single-well-formed-file case, and nothing above it. That is worth having — a typo'd or inherited http_unix_socket is the case an operator actually meets — and it is not a control against an adversary. The class above is the recorded residual.

The parse-error arm is a decision, not an oversight: the check fails open. An unparseable config is refused by nothing here. Refusing to spawn on any config the check cannot parse would deny the ingest to precisely the operator this check exists to help — someone whose config has a typo — and it would make a YAML reader's strictness a gate on an unrelated capability. The exposure that decision accepts is member (c), recorded above.

What the check's own spec must therefore say, all of it owed to slice 1: it resolves the same precedence chain gh does, treating an empty-string variable as absent and falling through exactly as gh does (measured); and it runs before any binary probe — ahead of clause 8's version read and clause 9's authentication probe — because a probe is itself a spawn, and a check that runs after one has already handed the config a request.

The precondition, named because it is the operator's trust and not Theurian's check. The directory is the operator's own, user-owned by construction; Theurian does not inspect its permissions or ownership, and does not propose to. Which directory it is, is decided by precedence and not by union: exactly one is read, by the order in member (d) — an earlier draft of this paragraph said the reach was "the union of what they resolve", which is not what gh does. (Measured on 2.86.0, http_unix_socket is a real configuration key while api_host is not — which is why both the refusal and member (b) name the class of transport-override settings and not one key.) Clause 4's tests keep member (a) demonstrated rather than argued, in the split clause 4 specifies.

Theurian borrows the operator's ambient GitHub identity, and holds no token. Ingestion runs with whatever scopes the operator's gh login already has, which may include private repositories. That is accepted, because the alternative is Theurian taking custody of a token (rejected in Alternatives considered), and because the control that keeps ingestion off private repositories is decision 2's allowlist and its refusal — not the credential's reach.

2. Scope: public allowlisted repositories only, and the argument is an audience argument

The adapter reads providers.review.repositories before any spawn. The key is already schema-ready: schemas/config/project-config.schema.json types it as an array of strings matching ^[\w.-]+/[\w.-]+$, beside a providers.review.adapter enum of github | none defaulting to none. Its description says "Not in force… Nothing reads it today", and slice 1 is the change that makes both sentences false and rewrites them.

Three refusals, each with a synthetic-input driving test owed to slice 1:

  1. A repository not in the allowlist is refused before the process is spawned. Not filtered after the fetch — the spawn does not happen.
  2. A repository that resolves as private is refused at ingestion, even if it is in the allowlist, and nothing about it is written.
  3. The repository the response describes is checked back against the allowlisted entry. GitHub redirects a renamed owner/repo, so an allowlisted name can resolve to a repository nobody allowlisted. The adapter compares the response's resolved nameWithOwner against the entry it asked for, case-folded — GitHub treats owner and repository names case-insensitively, so a byte-comparison would refuse a correct answer. The repository id is checked only against a previously recorded one: on a first ingest there is nothing to compare it to, and an id read out of the same response it is meant to validate proves nothing. Owed a slice-1 test with a renamed-repository fixture and a case-difference fixture.

The argument for public-only v1 is an audience argument, and it must be written narrowly. The tempting sentence — "a public repository cannot carry embargoed content" — is false as a universal, and this ADR does not make it: a public repository's review threads can discuss anything, including an unpublished vulnerability, and this project's own embargo discipline exists precisely because people are capable of putting such a thing where it does not belong. The true and narrower claim is:

Public-only v1 ingests no advisory-private GitHub surface (private repositories, security advisories, private forks), and every record it ingests was visible to the public repository's audience at the moment it was ingested.

That is the same structural shape ADR-0029 decision 6 records for public main: the protection is structural — the source has no access to the withheld surface — and it is not a claim that the public surface is guaranteed clean. What guards the second half is unchanged from the rest of the product: the secret scan at ingestion (decision 4) and the untrusted-content triple at serve (decision 6).

The subject is theurian review ingest, not .theurian/review/, and the correction is to the sentence rather than to the decision. This quote read "every record it holds" until 2026-09-11, which a reader could take as an inventory of the evidence directory — and that is false, because decision 3 makes those files source rather than derived state and theurian init deliberately does not git-ignore them, so a clone can carry evidence a repository author wrote and theurian review build projects it like any other. The scope this decision sets is unchanged: what an ingestion run may fetch, and from where. What a corpus may contain is threat-model T-24, an accepted residual recorded there with its grade and its upgrade triggers. Every surface that publishes this scope — the capabilities schema, mcp/tools.py, docs/protocol/mcp-tools.md — names the same subject.

The tense is load-bearing, and the residual it names is retention. Decision 3 makes the evidence files durable precisely so an upstream delete does not erase the record — which means "visible to the public audience" is true at ingestion time and can stop being true afterwards, when an author edits or deletes a comment upstream. This ADR does not pretend otherwise. The two cases part there, and only one of them is the residual. An upstream edit does reach Theurian's copy: decision 3's writer rewrites a record whose content changed and ReviewIngestReport counts it updated, so the next theurian review ingest run whose window covers the record brings the edit with it. An upstream delete does not — that is the retention decision itself, and no mechanism here would notice one. The remediation path exists and is manual, and it is the delete case's: delete the evidence file and rebuild the derived store, which is exactly the operation decision 3's files-as-source shape already supports (the store is rebuilt from the files, so removing a file removes the record from every surface). A capability that would make deletion automatic is not designed here and is listed in What this does not close.

The sentence above read "it does not build a propagation path either" until 2026-09-11, and that was wrong for edits. Its colon-clause named only delete, but the sentence it followed had enumerated "edits or deletes", so the generic half read as covering both — and the surfaces that publish this scope spelled it out that way, as "an upstream edit or delete does not reach Theurian's copy". The capabilities schema, mcp/tools.py, docs/protocol/mcp-tools.md and the flag's own pin in tests/integration/test_mcp_tools.py are corrected in the same commit, and they were corrected against the measured behaviour: tests/unit/test_review_evidence_store.py::test_a_refetch_rewrites_a_record_whose_content_changed_upstream has pinned the update since slice 2. As with the it holds correction above, the decision is unchanged — retention is still what decision 3 chooses — and what was wrong was a sentence about how the corpus behaves.

The private-repository arm stays owed, and it keeps a named owner. ADR-0029 assigned it to this arm: a finding marked securityRelated at ingestion time, where advisory state is available, then refused uniformly at serve — the refusal must not distinguish "an embargoed item exists and is withheld" from "no such item exists". Milestone 8 does not build it, and this ADR does not hand it to an unnamed follow-up either: it is owned by #575, the change that adds private-repository ingestion. ADR-0029's owed table and the threat model's embargo-arm sentence are repointed to #575 in the same commit as this ADR; nothing pins them to each other thereafter — a later edit to either can drift, and no test would notice. An owed item whose owner is "a follow-up" is the owner-position defect this document diagnoses twice elsewhere, and it is not repeated here.

The scope is machine-visible, not only prose. system.capabilities today publishes reviewIngestion: false beside reviewFindings: true, and docs/roadmap.md records why those two are separate flags: "the change that reaches GitHub is the one that owes SEC-10's repository allowlist … and an offline trailer read owes none." The same reasoning applies one level down, so the flag alone is not enough: a client that reads reviewIngestion: true and nothing else would conclude that review history is ingested wherever the operator points it. The decision is therefore reviewIngestion: true plus a scope field — proposed shape reviewIngestionScope: "public-allowlisted", a string beside the booleans, which the capability block already does elsewhere (the "knowledgeSearch": "hybrid" entry in mcp/tools.py) — flipped together at the serve slice, with the wire schema change that publishes them landing in slice 3. (That cite read mcp/tools.py:1868 until slice 3 moved the line; it is re-cited by the quoted entry under this document's own Demoted populations rule, which is that a quote survives renumbering and a line number does not.)

The flag's published meaning narrows, and that redefinition is stated here rather than performed quietly. Today reviewIngestion: false is read as this build cannot reach GitHub. From slice 3 the flag means an ingestion call surface exists that a client may call — a narrower promise, because the fetch path will have shipped two slices earlier while the flag was still false. Redefining a published flag without moving what cites it is how a security statement becomes false, so slice 1 moves the sites that record the old meaning.

The population is a key, and what lives here is the key, its frame and the rule that judges a line — not the annotated population. Four rounds tried to keep a member table inside this ADR in bijection with a moving tree, and each round found a mispointed anchor, an unjudged line or a stale frame beside a correct run. The table is therefore demoted (see Demoted populations in Dispositions); what a design record owes is the key and the judgment rule, both of which are stable.

The key, at origin/main @ 1fe3302b:

$ git grep -n "reviewIngestion" 1fe3302b -- . ':!.claude' ':!.theurian' ':!docs/work-logs' | wc -l
      31
$ git grep -l "reviewIngestion" 1fe3302b -- . ':!.claude' ':!.theurian' ':!docs/work-logs' | wc -l
      14

The exclusions, measured rather than assertedgit grep -n "reviewIngestion" 1fe3302b -- <path> for each: .claude/ drops 0, .theurian/ drops 0, docs/adr/0030-* drops 0 at that frame. Only docs/work-logs/ drops anything — 1 line — and it stays excluded because a work log is a dated record of what was believed then. CHANGELOGs are not excluded by path, because that dropped a live member: claim_surfaces.dated_lines — the audit tooling's own answer to "is this line inside a dated section?" — puts five reviewIngestion lines inside dated sections of packages/theurian-core/CHANGELOG.md and plugins/claude-code/CHANGELOG.md:281 outside one. The rule is owner_position_cites's, quoted from its _RELEASE_RECORDS docstring:

[Unreleased] is not one of those entries … It describes the tree a reader has checked out, it is rewritten on every merge, and a dead owner written into it is a live claim in a governed file.

The judgment rule, which is what a slice-1 brief applies to each line. Every line in the key's output is exactly one of:

Class Rule Moves in slice 1
Mover the line states what the flag means — that nothing reaches GitHub, that this is the flag that would, or that no adapter exists yes
Non-mover the line states the flag's value, or names it as a key in a list, or is another record's own recorded prose about its own decision no — the value flips at slice 3
Dated record the line is inside a dated release section no — correcting a release record would falsify it

Two frames, because they differ. Line anchors are 1fe3302b; this branch edits three of those fourteen files, so its own frame differs:

$ comm -12 <(git grep -l "reviewIngestion" 1fe3302b -- . ':!.claude' ':!.theurian' ':!docs/work-logs' \
             | sed 's/^1fe3302b://' | sort) \
           <(git diff --name-only origin/main...HEAD | sort)
docs/architecture/review-knowledge.md
docs/roadmap.md
docs/security/threat-model.md

Slice 1 is not a prose pass: movers include production-source docstrings, two assertion messages, and INGEST_CONFIG_BULLET in tests/unit/test_config_key_call_sites.py, which pins the plugin's ingest.md sentence byte for byte — the page and the constant move together or the test goes RED.

The flag itself does not flip until slice 3, because until then there is nothing for a client to call; the two facts are separated in the documents rather than papered over by moving the flip.

The window that leaves is a bounded residual, recorded rather than argued away. Through slices 1 and 2 the machine-readable answer is reviewIngestion: false while a fetch path ships — the documents will say what is true, but a client that reads only the flag gets the retiring meaning. The alternative was considered: publish the scope field early, in slice 1, so the machine-readable surface moves with the code. It was not taken because the field's honest value in slice 1 is "a fetch path exists that no tool exposes", which is a third meaning for a flag already being redefined once, and it would ship a wire-schema change two slices before the surface it describes. The residual's reach is bounded by what the flag can be used for and by nothing else: no tool is callable, so a client acting on the false loses no capability it could have had — but for those two slices the machine-readable security statement reads false while a fetch path ships, which is a wrong answer to a security question even when no feature is lost by it. That is the cost this residual carries, and it is why the documents move in slice 1 rather than waiting for the flag.

3. Evidence files are the source; SQLite is derived

Normalized evidence records land as structured JSON files under .theurian/review/ — durable, git-trackable, and deliberately not under .theurian/cache/. The SQLite serving store (slice 3) is built from those files and is deletable.

This withdraws a position that was live in the documentation, and the closure is a key on the position's own words. Three rounds enumerated the records carrying it by hand — one, then four, then five — and each missed one; a fourth round found that a key on the path token could not see the served corpus at all. So the key is the position string itself:

$ git grep -n "raw GitHub review caches" -- . ':!docs/adr/0030-*'
.theurian/knowledge/architecture/sqlite-is-a-derived-artifact.01M0D5GTZN1YPYAPJ18TAP7NAD.md:41:- raw GitHub review caches, normalized temporary documents
.theurian/knowledge/architecture/sqlite-is-a-derived-artifact.01M1EVBH96EE1X6Z9KTA5EZ95K.md:41:- raw GitHub review caches, normalized temporary documents
.theurian/knowledge/architecture/sqlite-is-a-derived-artifact.01M1QMNC4YSFVPST9RA2FNHKDJ.md:41:- raw GitHub review caches, normalized temporary documents
docs/adr/0004-sqlite-is-a-derived-artifact.md:41:- raw GitHub review caches, normalized temporary documents
docs/adr/0004-sqlite-is-a-derived-artifact.md:86:> "raw GitHub review caches" entry in the *Never Git-tracked (derived)* list

This ADR is excluded from the key by pathspec, for the reason round three recorded and round four upheld: a document that withdraws a position must quote it, so its own lines match any key about that position and an unfiltered paste can never be current in the file it sits in.

Corpus membership, stated because this is a repo-wide key (the standing rule for any walker here): the served corpus is IN, and it is where three of the five hits are. Those three are sqlite-is-a-derived-artifact twins — one current, two superseded revisions — and they carry the withdrawn line because a governed snapshot moves by re-seed and never by edit. They are #579's: the same re-seed that clears the drift this PR causes brings the amendment across.

So the claim is qualified, not universal. No record outside this ADR and outside the frozen corpus twins #579 re-seeds asserts the position: ADR-0004:41 is the as-authored line the amendment at :86 withdraws in place — the repository retracts by amending, never by rewriting.

Two measured limits of this key, recorded rather than discovered later. It matches one physical line, so a straddled form — the phrase split across a wrap — is invisible to it; and it is invariant under deletion of the amendment: delete ADR-0004:86 and this key still returns the corpus twins and :41. What guards the amendment's presence is not this key but #579's re-seed, which byte-pins the amended body into the corpus, plus test_every_pinned_body_is_byte_identical_to_its_source_anchor_commit comparing that body against its anchor.

The withdrawn wording is quoted above because the reasoning, not the corrected sentence, is what a later reader needs. It is safe for an artifact whose source outlives it. It is not safe here: GitHub review comments are editable and deletable upstream, so a deleted local copy of a comment that has since been deleted upstream is data loss, not a cache miss, and no refetch recovers it. "Rebuildable" would be asserting a property the upstream does not provide.

With files as the source, ADR-0004's real property comes back: deleting the SQLite store is a cache miss, because the store is rebuilt from the evidence files. The contrast with the findings store is worth stating honestly, because the two look alike and are grounded differently — infrastructure/sqlite/findings_schema.py says so in its own docstring: the findings store is safe to delete because "the source of truth is git history … and this file is reconstructed wholesale by replaying the git source." Review evidence has no such replayable source. So:

Artifact Why deleting it is safe Layer (ADR-0010)
Review-finding store (ADR-0029) it is safe: git history is replayable Canonical, projected from git
Review evidence files (this ADR) it is not safe — deleting them is data loss Canonical, carrying a SourceAnchor — but with no replayable source
Review serving store (slice 3) it is safe: rebuilt from the evidence files Index / derived

The middle row is a case ADR-0010's three layers do not have, and it is named rather than forced into one of them. The bytes GitHub returned are Source; what lands on disk is a normalized record Theurian wrote, which is the same operation ADR-0029 places in Canonical — so putting the evidence files in Source would contradict that precedent for an identical act. What makes them unlike every other Canonical artifact is the property in the middle column: findings_schema.py says the findings store is safe to delete because "the source of truth is git history … and this file is reconstructed wholesale by replaying the git source", and review evidence has no such replay. So this is a Canonical record whose own source is not re-readable — the fourth-category case ADR-0004's Milestone 8 amendment records — and it carries a SourceAnchor (FR-S3) naming the upstream object it came from, precisely because that anchor is the only remaining pointer to material Theurian can no longer re-fetch.

Paths are built from ids, never from allowlist strings. An evidence file's path is derived from provider-generated identifiers, or a hash of the repository identity — never by joining the configured owner/repo string into a filesystem path. The reason is measurable today: the schema pattern ^[\w.-]+/[\w.-]+$ accepts ../.., so a joined path escapes the directory while satisfying the config schema. Two controls, both owed:

  • Every write resolves through security/paths.py's containment (resolve_within_root, assert_no_symlink_escape) — the same SEC-7 containment the rest of the product uses, applied to the write path and not only to a served filePath field.
  • The schema pattern is tightened to reject . and .. segments, and that tightening lands in slice 1's allowlist-reader commit — the commit that first makes the key load-bearing is the commit that makes its values safe.

Refetch is a best-effort refresh, never the durability story. A later ingestion run updates what upstream still returns and does not delete what it no longer does; a record that vanished upstream stays in the evidence files, marked with the run that last saw it. Whether a project commits .theurian/review/ is the project's decision — the directory is not written into the ignore block. What lands in it is also subject to the configurable ingestion-time redaction review-knowledge.md's Privacy section describes, which is design and not shipped behaviour: no review ingestion path exists to perform it — the GitHub package holds no adapter, measured in Context — and it is slice 2's, applied at landing, beside the scan gate, since both are decisions about what a file may contain before it is written.

The evidence record's field set, enumerated once

The scan gate (decision 4) and the serving trust table (decision 6) need the same population, so it is written here once. Every field FR-V1 names is listed with who controls its value — which decides both whether the scan reads it and which side of the trust boundary it is served on:

Field Controlled by Trust class Read by the ingestion scan
repository owner/name, PR number, event key, review / thread / comment ids the provider structural no — no free text
thread state, resolution state, isResolved / isOutdated, timestamps, diff side, line numbers the provider structural no
head, fix and merge commit shas; linked issue numbers; CI rollup outcome the provider structural no
participant external_id the provider structural no — see the correction below
comment body, review body, PR title, PR description the author untrusted yes
participant display_name the author untrusted yes
file path as received the author (whoever named the file in the PR) untrusted yes
labels, head branch name, milestone name the author untrusted yes
SourceAnchor (provider, source URI, upstream object id, path) Theurian, at ingestion structural no — it is written here, not received
last-seen-run stamp (which run last observed the record upstream) Theurian, at ingestion structural no

Correction, 2026-09-08: the participant external_id row holds only while a node id exists. The row above is left as it was written; this note is what amends it. infrastructure/github/response.optional_participant reads external_id as node id or login, so an actor GitHub answers with no id — an Actor implementation that is not a Node, or a partly-errored response carrying id: null — arrives with the author's own login in that field. A login is chosen by its owner and can be changed, which is the untrusted side of this very table, and a GitHub login is up to 39 characters of [A-Za-z0-9-], so AKIAIOSFODNN7EXAMPLE is a login somebody can register. Two consequences, both shipped:

  • The scan reads external_id in both redaction states since PR #596 round 1 (the round record, adversarial H-D). Which of the two a given id is cannot be decided from the record — a rule recognising node-id shapes would restate the adapter's mapping in a second place, and would be wrong in the direction that skips a login — so the field is read unconditionally rather than only where it can be author-chosen.
  • Redaction pseudonymises a login-fallback id before the record is written (R-12, decision 3's redaction half). Under providers.review.redactParticipantNames, a participant whose external_id equals its pre-redaction display_name — the adapter's own signature for the fallback — lands under redacted~<sha256 prefix> instead, deterministically, so the record keeps a stable identity and the login never becomes a file. A node-id-shaped id is kept verbatim, which is what makes the identity graph survive the redaction.

What the round measured is why both were needed rather than either: with the field unscanned and unredacted, turning R-12 on turned a block refusal into a landing that published the login under externalId while replacing the displayName R-12 promises to remove.

Three values in the Controlled by column, not two. A record carries fields Theurian itself writes: the SourceAnchor (FR-S3) that names the upstream object, and the stamp decision 3 relies on to say a record survived a refetch. They are neither provider structure nor author content, and calling them either would put Theurian's own writes on the wrong side of a trust boundary — so they are their own row, scanned by nothing because nothing outside this process authored them.

Author-controlled structural-looking metadata is on the untrusted side, and that is the row most easily got wrong. A label, a branch name and a milestone name look like provider structure and are not: anyone who can open a pull request chooses them, so they carry text a person wrote and ride under the same safety triple as a comment body.

This is where ADR-0019 is discharged rather than merely cited: an ingested label is data, and it governs nothing. A record labelled security upstream is not thereby security-related to Theurian; no serving decision, no refusal and no ranking reads a label's value. The marking that would carry weight — securityRelated — is #575's, and it is computed at ingestion from advisory state, never read off an author-chosen label. A design that let a label decide what is withheld would hand the withholding decision to whoever opened the pull request.

4. The secret scan runs at ingestion, per record, like propose accept

security.secretScan (SEC-11) applies at ingestion, per record, before a record becomes a file:

Policy Behaviour at review ingestion
block (default) the flagged record is refused and never written; the run reports the refusal by record identity and does not exit as if it were clean
warn the record lands and every finding is reported
off no scan

This mirrors propose accept, not index build, and the reason is the source's own premise. application/index_secret_scan.py states why the build records rather than refuses:

A landed secret is readable through knowledge.search and knowledge.get the moment theurian migrate apply writes it, before any index exists at all … So a build that refused to publish would deny ranking without un-disclosing anything, and on a project that has never built one it would deny ranking for ever.

That premise is false pre-landing. Review ingestion runs before the content exists anywhere in Theurian, so refusing genuinely un-discloses relative to Theurian's own surfaces: nothing is written, so nothing is served, indexed, ranked or reachable through any tool. The gate that matches is ADR-0027's — validate before you move.

What block costs, stated rather than assumed. Refusal is not free, and the cost is not symmetric with the build's. Upstream may already have edited or deleted the comment by the next run (decision 2's retention residual, in the other direction), so a record refused at ingestion — including on a false positive, and the detector is best-effort entropy heuristics by its own admission — may be evidence Theurian never gets another chance to hold. That is accepted, with the reason named: a landed credential is worse than a gap in the evidence. A gap is visible in the ingest report and recoverable by re-running with warn after a human has looked; a landed secret is a disclosure through every serving surface the moment it is written, and no later policy change un-writes it. Operators who weigh those differently have warn, which is why the policy is a setting and not a constant.

The report names the record (repository, PR number, thread and comment id), not the matched bytes, for the reason index_secret_scan.py gives about pasted reports: a report that quotes the secret is a second copy of it.

The consult disagreed here, and the disagreement is recorded rather than smoothed over. The Codex reader recommended the index build signal-mode shape (land and report, never refuse), reasoning by analogy with the existing scan-at-build control. The watchdog reader checked the premise of that control rather than its shape and found it did not transfer — the "already disclosed" condition is exactly what ingestion does not have. The premise check won, and the analogy lost. It is noted here so a future reader who rediscovers the analogy finds the answer instead of re-deciding it.

5. The domain model bends to what GitHub can answer

The ReviewResolution model as built requires two fields GitHub does not guarantee. Four rows below, of which two change — the fourth, PromotionGate.ci_successful, is listed precisely because Milestone 8 does not touch it. Slice 1 work:

Field Today Becomes Why
resolved_at datetime (required) datetime \| None No resolution timestamp exists on the API object.
resolved_by ReviewParticipant (required) ReviewParticipant \| None resolvedBy is nullable — a thread can be resolved with no participant recorded.
ReviewEvent.ci_successful bool \| None (domain/review.py:61) unchanged None already means unknown, and that is the honest value for a PR with no status rollup.
PromotionGate.ci_successful required bool (domain/review.py:152) not touched by Milestone 8 The gate is candidate-generation machinery, which is out of scope (below). None is unrepresentable there today, so how the gate should treat unknown is a real open question — assigned to the candidate-generation design, not answered here.

Amended in Phase B slice B5, by the tri-state widening (2026-09-18, the branch commit ca6246ae of PR #744). The fourth row's "required bool" no longer holds, and the open question it hands over has been answered. PromotionGate.ci_successful is bool | None, and None does not satisfy the gate — ADR-0033 decision 4. What the row records about Milestone 8 stands: Milestone 8 did not touch the field, and the candidate-generation design settled it exactly where this row assigned it.

The table's two line numbers resolved when this ADR was authored and no longer do. domain/review.py:61 and :152 were both correct at 46d0e869, this ADR's own commit; the module has grown since and moved both fields. They are left as authored rather than corrected into history — what to follow is the field on ReviewEvent and on PromotionGate, not the line.

The measurement, quoted from the design consult (2026-09-05, GraphQL schema introspection). PullRequestReviewThread's fields are:

comments  diffSide  id  isCollapsed  isOutdated  isResolved  line
originalLine  originalStartLine  path  pullRequest  repository  resolvedBy
startDiffSide  startLine  subjectType  viewerCanReply  viewerCanResolve
viewerCanUnresolve

There is no resolution-timestamp field in that list, and resolvedBy is nullable. A required field the provider cannot fill leaves the adapter only bad options: fabricate a value — the ingestion time, or the last comment's time — and every consumer downstream reads as a measurement something nobody measured; or drop the whole resolution record, losing the resolution state the model exists to carry. Making the field optional is the change that keeps unknown expressible, which is the thing neither bad option can express.

This is a breaking change to the domain model, and it costs nothing today. Measured 2026-09-05 against origin/main @ 1fe3302b, with the population key being every occurrence of the symbol anywhere in the shipped package:

$ git grep -n "ReviewResolution" -- packages/theurian-core/src
packages/theurian-core/src/theurian/domain/review.py:98:class ReviewResolution:
packages/theurian-core/src/theurian/domain/review.py:121:    resolution: ReviewResolution | None = None

Two hits, both inside the defining module — the class statement and its own field on ReviewThread. There is no consumer to migrate. The CHANGELOG entry for slice 1 still names it as a breaking change with the old shape and the new one, because "no consumer exists" answers the migration cost and not the question of whether the record is honest.

ReviewEvent.ci_successful's mapping is a rule, not a measured enum. The adapter maps the PR's status rollup to the tri-state as: a definite success → True; a definite failure or error → False; anything else — pending, expected, absent, or a value this version of the adapter does not recognise — → None. The mapping is stated by semantics rather than by enumerating the API's enum members, because this ADR did not measure that enum; slice 1 pins the member list against the schema at implementation time. The load-bearing half is the default: on the ingested record, an unrecognised value becomes unknown, never failed.

That is a statement about the ingested record and not about the gate. PromotionGate.ci_successful is a required bool, and is_satisfied / unmet() read it as a verdict — so an unknown handed to that type has already been flattened into True or False by whoever constructed it. Milestone 8 constructs no gate (candidate generation is out of scope), which is why this ADR does not change the type and does not claim the gate is safe from a shrug. Deciding whether the gate gains a tri-state, refuses construction on unknown, or treats unknown as unmet is the candidate-generation design's, and it is listed in What this does not close.

Amended in Phase B slice B5 (2026-09-18, the branch commit ca6246ae of PR #744). The first of those three was chosen: the gate gains the tri-state, so PromotionGate.ci_successful is bool | None and an unknown is no longer flattened by whoever constructs the gate — it arrives as None and does not satisfy it (ADR-0033 decision 4). This ADR's own position is unchanged: it did not change the type, and the deciding was the candidate-generation design's.

Candidate generation is out of Milestone 8. FR-V2 classification and FR-V3 KnowledgeCandidate generation are the write-path half of Phase B: they are SEC-12-gated, and they raise the model question (ADR-0009) that ingestion does not. They are filed and sequenced, not folded in. FR-V5 is satisfied structurally rather than by a fallback path: no model exists anywhere in the ingest path, so raw ingestion cannot be broken by candidate generation failing.

That sentence is a universal, and its authority is a test that does not exist yet, so it is named here as owed rather than left standing on a reading: slice 2 owes a walk of the ingest path's modules asserting that none reaches an embedding, summarization or reranking provider. The shape to follow is test_no_registered_tool_can_reach_a_canonical_write (tests/integration/test_mcp_tools.py:2357), which walks the built object graph — the registered callables, their __wrapped__ chains and nested code objects — rather than a directory of source files, for ADR-0013's equivalent claim. What that buys, stated honestly: it sees what the running system actually holds, including a path no test exercises. What it does not buy is factory resolution — co_names is a name scan one level down, so a provider obtained through a factory or getattr is invisible to it, as round two demonstrated. The owed test is therefore scoped to what it can hold: no ingest module names a provider module or constructor, plus a runtime assertion over the built ingest pipeline's object graph. Until it lands, "no model exists anywhere in the ingest path" is design intent with a named owner, not a measured property — and even once it lands, a factory-resolved provider is outside its reach.

6. Serving is review.search, under the SEC-15 triple, with its own disclosure round

The serving surface is review.search, taken from the already-published planned table in docs/protocol/mcp-tools.md (7 review.* rows — the grep and its output are in Compliance). No eighth name is invented: a tool name is a wire contract, and the table has been publishing this one as planned.

Every body-derived field is untrusted content (T-3, SEC-15); only structural fields are validated and normalized.

The population is decision 3's field table, not a second list. Its Controlled by column decides the side: every author-controlled field — comment and review bodies, PR title and description, display name, file path as received, and the structural-looking metadata (labels, head branch name, milestone name) — is served under contentClassification: untrusted-knowledge, mayContainInstructions: true, executable: false. Every provider-controlled field — states, timestamps, line numbers, repository, PR number, ids, commit shas, CI outcome — is validated and normalized. Enumerating the fields in one place is what keeps a field from being scanned as content at ingestion and then served as structure.

The file path is deliberately on the untrusted side: it is chosen by whoever authored the pull request. It is served as data and SHALL NOT be used to build a filesystem path — the containment SEC-7 requires is not weakened by a string that arrived over the network, and decision 3 states the same rule for the write path.

The disclosure closure is its own round, and its test is built with the serving change. ADR-0029's closure argument says it in five words — "new surface owes its own" — so review.search inherits nothing from review.findings' round. The closure form this project uses is one query against two corpora: an index that held withheld rows and an index that never did must return identical responses. Because public-only v1 makes real withheld rows absent, the fixture is synthetic — that is not a weaker test, it is the only way to have a withheld row at all in a corpus whose scope excludes them, and a sweep with no withheld row reports its own answer.

Two inherited controls are named so the serve slice does not rediscover them:

  • If review evidence is ever served through a ranked surface, the withheld-row exclusion is a physical purge (threat-model T-17a), not a result-set filter: a fusedScore is priced over FTS5/BM25 collection statistics computed at index-build time, which a filter does not clean and a tombstone does not move. This is ADR-0029's family-4 third instance, and it applies here unchanged.
  • reviewIngestion: true (with its scope field, decision 2) flips at the serve slice, not at ingest, because from that slice the flag means an ingestion call surface exists that a client may call — the narrowed meaning decision 2 records, whose six citing sites are rewritten in slice 1, two slices before the flip.
  • The triple is bound by import, not by spelling. A review.search payload carries theurian.mcp.results.SAFETY (mcp/results.py:44) splatted into the result — imported, never re-typed — so a future edit to the shared constant reaches this surface too. A test that asserts three literal key/value pairs passes on a payload that re-spells them locally and then drifts; the owed test therefore asserts the payload against the imported object, with a companion that the assertion can fail.

Consequences

Positive

  • T-7's repository allowlist stops being owed and starts running. The control is promised in many places and enforced in none. Measured 2026-09-05 at 1fe3302b, key = the literal phrase, population = the repository minus .claude/, docs/work-logs/ and this ADR: git grep -l "repository allowlist" -- . ':!.claude' ':!docs/work-logs' ':!docs/adr/0030-*'8 files, 20 lines. The key's recorded limit: it misses the places that spell it differently — the schema description ("Not in force. Allowlist of owner/repo values…"), the infrastructure/github/ docstring ("Repositories must be allowlisted… owed with the adapter rather than in force") and review-knowledge.md ("the design obligation on the adapter, not current behaviour") — so 8 is a floor, not a census. And T-7 is two entries, not one: docs/security/threat-model.md:6454 and docs/architecture/requirements-analysis.md:1352 each carry their own T-7 row naming the allowlist as owed to #429, so "the T-7 entry" is always plural here and slice 1 rewrites both. Slice 1 is the first change that can carry the control, and both entries are rewritten per control rather than repointed at another epic.
  • No new production dependency and no token custody. The core's runtime dependencies are six (jsonschema, pydantic, python-ulid, pyyaml, referencing, typerpackages/theurian-core/pyproject.toml, measured 2026-09-05). Spawning gh keeps that number, and keeps GitHub credentials in the operator's own credential store.
  • The absence control is replaced by something that keeps working after the first fetch lands. "Nothing can reach out" is a sentence with no successor: the first time it is false it is retired, and whatever it was protecting is unprotected. Ten clauses with their own tests still say something on the day after.
  • Evidence outlives the upstream. Files-as-source means a deleted upstream comment is still in the record, which is the whole point of ingesting review history rather than querying it live.
  • The scope is machine-readable. A client learns "public allowlisted repositories only" from system.capabilities, not from a paragraph.

Negative

  • A spawn site is a hole no name-based scan can fully watch. test_network_call_sites.py records this itself: a program started under a name assembled at runtime survives every structural arm, and a socket watch cannot see into another process. After slice 1, "what does gh do with the vector we hand it" is outside every instrument this suite has.
  • Ingestion inherits the operator's identity and scopes. The credential that runs is not one Theurian issued, bounded, or can revoke.
  • gh is an unpinned external binary. ADR-0014's exact pinning does not reach it; a version floor with a test is a weaker instrument than a lockfile, and the behaviours clauses 2–6 rely on are that binary's.
  • The evidence files are a new durability obligation. They are the source, so losing them is data loss — which is exactly the property that makes them the right place for the data, and exactly the property that means a project must treat the directory as content rather than as scratch.
  • A second serving surface means a second disclosure round. review.search inherits nothing from review.findings' round, and its two-corpora fixture has to be built rather than borrowed.

Neutral

  • The git-native arm (ADR-0029) is untouched. review.findings keeps serving trailers from local history and keeps needing no allowlist; the two arms share the safety triple and the FR-V family, not a source.
  • .theurian/review/ is not automatically committed. The ADR decides only that it is not a cache; whether a given project tracks it is that project's decision.
  • The five planned review.* tools other than review.search stay planned (seven rows: one shipped, six planned, of which review.search is one). This ADR neither builds nor retires them.

What this does not close

Amended in Phase B slice B5 (2026-09-18, the branch commit ca6246ae of PR #744): item 2's unknown-CI clause is closed. PromotionGate.ci_successful is bool | None, unknown is unmet, and ADR-0033 decision 4 carries the reasoning. The rest of item 2 stands — FR-V2 classification and FR-V3 generation are designed and not built, and slice B5 is where they land.

It sits above the list rather than under item 2 because tools/audit/owner_position_cites.py reads an amendment block two blocks below a sentence as retracting it, and under item 2 this block retracted item 1's live #575 ownership cite instead.

  1. Private-repository ingestion, and with it the securityRelated ingestion-time marking plus uniform serve refusal that ADR-0029 assigned to this arm. Owned by #575; not built in Milestone 8, and ADR-0029's owed table names #575 rather than this ADR.
  2. FR-V2 classification and FR-V3 KnowledgeCandidate generation. The write-path half of Phase B, SEC-12-gated, deliberately sequenced after ingestion — and with it how PromotionGate should treat an unknown CI outcome, since its ci_successful is a required bool today (decision 5).
  3. The other two T-7 controls in their raw-URL form — the scheme allowlist and private-network rejection — stay #429's (see Dispositions).
  4. The remaining planned review.* tools: review.getThread, review.findSimilar, review.getDecisions, review.generateKnowledgeCandidate, review.listUnresolved.
  5. GitLab and other providers. The ReviewProvider port exists so that a second provider is an adapter and not a domain change; this ADR designs one adapter and does not claim the port is provider-neutral until a second one exists to test that claim.
  6. FR-V6 Markdown views over review evidence. The requirement scopes them as derived artifacts; this ADR neither designs nor forbids them.
  7. What gh does after the vector is handed over. Clause 8's version floor bounds which binary, not its behaviour; the residual is recorded in decision 1 and in Consequences → Negative.
  8. Propagating an upstream delete into the evidence files. Decision 2 records the retention residual and the manual remediation — delete the file, rebuild the derived store; a capability that notices an upstream deletion and acts on it is not designed here. An upstream edit is not in this item and never was: a refetch rewrites the record and reports it updated. The heading read "an upstream edit or delete" until 2026-09-11, matching the widening decision 2 records above and describing as unbuilt something the write path already does.
  9. Headless environment-token authentication. Excluded from clause 4's constant by decision, not by omission: identity comes from the operator's persisted gh login. Admitting a token variable is a decision with its own reasoning, owed to whoever needs it.

Dispositions this ADR records

Each disposition is a decision with a reason, recorded here so a later session finds an answer rather than an open question.

#429 is narrowed, not closed, and exactly one control is discharged. Slice 1 discharges one of T-7's three controls for the gh activation context, reduces a second with a residual that stays recorded, and leaves all three owed in the raw-URL context that is #429's:

T-7 control On the gh path (slice 1) In the raw-URL context
Repository allowlist Discharged — a live check consulted before the spawn, with a pinning test. The only discharged control on this path n/a
Private-network rejection Split by family, and neither half is discharged as a whole. The proxy family is closed by construction: it is absent from the child by the equality of clause 4's constant, and run C is what shows it would otherwise move the request. The config family is reduced, not closed: the config file is reached by the child through the three forwarded variables (runs D–F, re-run by round two under exactly the enumerated constant — the request still dialled the unix socket), and slice 1 adds a pre-spawn best-effort refusal of known transport-override keys in the file those variables resolve to. What survives is decision 1's divergence class — four measured members, derived there from the fact that the check's read cannot be gh's read (when, which keys, which bytes-parse, which file). The check reduces the accidental, pre-existing, single-well-formed-file case and nothing above it still #429's
Scheme allowlist not applicable — there is no URL in the argument vector to check a scheme on (clause 2) still #429's

What holds this row is clause 4's tests, in the split clause 4 specifies — (ii-a) drives the refusal, (ii-b) demonstrates a residual. Neither is "the control that keeps this discharged", and the rationale is not restated here: it lives in clause 4, and round two's fix drifted precisely because the same reasoning sat in two places.

429's activation context is wider than this ADR: it includes the OpenAPI $ref

fetcher, where a URL taken from an ingested document is the input, and there both the scheme allowlist and a destination check are live checks with something to check. T-7 is therefore rewritten per control in slice 1 — the absence control retired at a named commit, the repository allowlist recorded as discharged with the test that pins it, private-network rejection recorded as reduced with its residual class named (decision 1's four-member divergence class), never as discharged, and the rest still owed with #429 named. The threat model's own lesson is the reason this is spelled out — "an owner has to be the change that would implement the control" — and this entry has had the wrong owner twice.

The fetch-absence prose population moves in slice 1, in the same PR as the first spawn site. Not a later slice, and not a follow-up: on-main claims must never call the fetch absent while a fetch path ships. Measured 2026-09-05 against origin/main @ 1fe3302b, with the key stated so it can be attacked:

$ git grep -l -i -E "reviewIngestion|nothing (here )?can reach out|no external fetch|contacts no repository|never fetche[sd]|first external fetch" -- . ':!.claude' | wc -l
      30

The key is a deliberately wide phrase heuristic over six spellings, and it returns tests, tools and work logs alongside the prose that has to change — the same discipline tools/audit/owner_position_cites.py uses: narrow the population to what a person must read, then have the person read it. The number is a dispatch input, re-measured when slice 1 is briefed, not a claim about how many sentences are wrong. This sequencing is binding.

A second population moves in the same slice, and it needs its own key: the sentences that attribute this unbuilt work to Milestone 7. Milestone 7 is closed and built no fetch path, so every one of them is false today. The two sentences this PR corrected in review-knowledge.md are not members of the key below — that key's pathspec is source, tests and plugin commands, and cannot select anything under docs/. An earlier draft of this paragraph claimed them as members, which was a claim about a population its own key excludes. Round three's reviewers re-ran a widened key over docs/ as well and reported the docs side clean after this PR's corrections; the key below stays narrow because the docs-side movers are already dispatched by the fetch-absence population above.

Two subject-filtered keys were tried and both leaked, in opposite directions. Round one's key is recorded in that round's own comment (issuecomment-5551278216), which reports it as returning 8 lines across 4 files; an earlier draft of this paragraph returned 10 across 7, and neither is a superset of the other, so their counts do not reconcile (8 + 3 ≠ 10 was the arithmetic that gave it away). A subject filter reads one physical line, and these attributions routinely straddle two. So the population is taken unfiltered and classified by hand — the union of both keys and then some:

$ git grep -n "Milestone 7" -- packages/theurian-core/src packages/theurian-core/tests \
    plugins/claude-code/commands | wc -l
      27
$ git grep -l "Milestone 7" -- packages/theurian-core/src packages/theurian-core/tests \
    plugins/claude-code/commands | wc -l
      18

27 lines across 18 files at origin/main @ 1fe3302b (same on this branch — it edits none of them). A person classifies each line into one of three, and the rule is what lives here; the annotated member list is demoted to the slice-1 brief (Demoted populations, below):

Class Rule
Mover the line attributes review collection or a T-7 fetch control to Milestone 7 — false, since that milestone closed and built no fetch path
History the line describes something Milestone 7 actually did — for example tests/unit/test_config_key_call_sites.py:42, "the Milestone 7 diff that added the first reader of .theurian/config.yaml", which is true
Unrelated Milestone 7 named about the write path, the ports register or the corpus seed

The three classes partition the key's output, and the reconciliation identity a re-measurement has to satisfy is: movers + history + unrelated = the key's line count. The counts themselves are the demoted population's, not this record's — the last annotation and its numbers live in the round-5 record on this PR, which also found two members the previous annotation had misjudged (a mover filed as unrelated, a second history line), so the re-measurement judges every line against the rule rather than trusting a prior split.

The unrelated class is Milestone 7 references to the write path, the ports register and the corpus seed — different subjects entirely. Slice 1 dispatches on the union above, re-measured at brief time; the unfiltered key is what makes that re-measurement reproducible, at the cost of a hand pass over 27 lines.

The #368 / #479 boundary, stated so a third owner-position defect cannot happen. #368 (open, phase-b) keeps the six git-native findings-store items ADR-0029's owed table assigns it — including a ranked-search surface over findings. #479 (Milestone 8) owns the GitHub-API arm: events, threads, comments, resolutions. review.search serves review evidence, not findings; review.findings keeps serving findings. Two issues, two sources, two tools, no overlap.

Milestone 8 is a planned epic, so the class-expansion brake does not bind it. Four top-row PRs at full synchronous review weight — this ADR, ingest, land, serve — is the plan, not a class that kept producing siblings. A later session counting PRs in this milestone should read them against the plan, not against the three-siblings budget.

Corpus membership: this PR drifts one anchored twin, and the sentence below is the checker's output rather than an assertion about it. An earlier draft asked the wrong question — its key looked for ADR-0025–0029, the threat model and review-knowledge.md among the anchors, and never asked about the file this PR edits. The right key is the anchored filePath list intersected with this PR's own file set:

$ comm -12 <(git grep -h "filePath:" -- '.theurian/migrations/*.yaml' | awk '{print $2}' | sort -u) \
           <(git diff --name-only origin/main...HEAD | sort)
docs/adr/0004-sqlite-is-a-derived-artifact.md

One member — the ADR this PR amends. The checker agrees, and its output is the claim:

$ uv run --frozen python tools/corpus_drift.py
Corpus drift: drifted -- 1 drifted -- compared 26 anchor(s) across 48 committed migration(s); 0 uncheckable; 22 superseded.
  DRIFT  architecture.sqlite-is-a-derived-artifact: docs/adr/0004-sqlite-is-a-derived-artifact.md now hashes to <digest>, and the corpus pins <digest>

(The two contentSha256 prefixes the checker prints are elided as <digest> above, and only there: they are content hashes, not commits, and tools/audit/sha_anchors.py reads any 7–40 hex characters in governed prose as a commit anchor it must resolve. Everything else in that block is the run's exact output.)

The re-seed follows this PR's merge, and cannot precede it. A twin's sourceAnchors[].commitSha must name a commit that holds the body verbatim, and test_every_pinned_body_is_byte_identical_to_its_source_anchor_commit raises on an anchor "this complete clone does not contain". Measured: all 7 distinct anchor commits in the corpus today are ancestors of origin/main (git merge-base --is-ancestor <sha> origin/main for each, 7/7). The amended ADR-0004 exists only on this branch, so the only commit that could be anchored is one a squash-merge discards — which would turn a working test RED on main the moment this lands. So the re-seed is owed to the change that runs after the squash commit exists, by the standing pattern recorded when the seven twins were re-seeded on 2026-09-05, and it is owned by #579 (open; read on 2026-09-05), sequenced as the first work item after this PR merges. Naming the owner is the point: an owed item described without a number is the stated-absence shape this ADR corrects twice elsewhere. CI runs the checker --advisory, which is why this drift does not block, and why saying so here is the only thing that records it.

tools/corpus_drift.py walks the committed migrations and compares each anchor to its live source, so a new docs/ file with no twin — ADR-0030 itself — is outside its population by construction, the same way ADR-0025 through ADR-0029 already are. Stated because every repo-wide claim in this repository declares which side of the frozen corpus it stands on.

Dogfood ground: thin, bot-authored and mutable — not absent. Two measurements, each with a bounded population, because the first one's window moves and a moving window cannot be re-checked later:

Measurement Population, bounded Result
Design consult, 2026-09-05 (gh api graphql, pullRequests(last: 40, states: MERGED)) a moving window, bounded by naming the merge list it covered rather than a range: the last 40 commits on origin/main at 1fe3302b carry 38 trailing pull-request refs — 440, 446, 448, 460, 466, 467, 470, 471, 474, 475, 478, 482, 486, 487, 488, 489, 490, 492, 498, 500, 501, 504, 513, 514, 518, 519, 524, 525, 534, 536, 541, 545, 552, 554, 556, 557, 560, 563 (git log origin/main --format='%s' -40 \| sed -n 's/.*(\(#[0-9 #]*\))$/\1/p' \| awk '{print $NF}', taking the trailing ref per ADR-0029's rule, so (#520 #525) contributes 525 and not 520; two of the forty commits carry no ref) 0 inline review threads, 0 top-level reviews
Adversarial review, 2026-09-05 (REST pulls / comments), re-run by the orchestrator keyed by PR number, not by a window: #12, #132, #224, #352, #569 11 inline review threads (#352 ×5, #12 ×2, #224 ×2, #132, #569 ×1 — open at measurement, so a floor rather than a pin; 2 as of 2026-09-07) and 5 COMMENTED top-level reviews — every root comment authored by github-advanced-security[bot], one dated 2026-09-05 on the still-open #569

Both figures are dated snapshots, and the thread count moved while this PR was under review: a further bot thread landed on the open #569 six minutes after this PR's round-one fix commit, taking 11 to 12. The number is therefore written as 11 at the round-two measurement, 12 shortly after rather than as a property of the repository — and the movement is not a nuisance, it is the evidence for the fixture decision below.

Re-measured 2026-09-07, and the row's #569 ×1 is annotated rather than corrected. Mend's harness re-ran the by-PR-number population and found #569 carrying 2 inline threads — both authored by github-advanced-security, both dated 2026-09-05 — where the member list reads it as one. The 2026-09-05 figures stay exactly as they were taken: #569 was open when they were, so its member count was a floor from the moment it was written, and a snapshot that a later snapshot exceeds is not a snapshot that was wrong. The second thread is consistent with the one this paragraph already records as arriving mid-round; no comment id was compared across the two runs, so they are recorded as agreeing rather than as identified. Discovering record: PR #595's body. What this adds to the argument below is a second independent observation of the same property — the population moves, on an open PR, between one round and the next — rather than a new number for it.

Re-measured a third time on 2026-09-11, by comment identity, and #569's count is now frozen. gh api graphql against repository(owner:"theurian", name:"theurian").pullRequest(number:569) answers state: MERGED, mergedAt: 2026-09-05T15:55:12Z, and reviewThreads.totalCount: 2 — thread ids PRRT_kwDOTqMAc86fiBod and PRRT_kwDOTqMAc86fiqjB, each rooted by github-advanced-security, created 2026-09-05T09:35:33Z and 2026-09-05T11:13:26Z. Both root comments predate the merge, so this is the same pair the 2026-09-07 run counted, now identified rather than merely agreed with, and the pull request is closed so the count can no longer move. It does not move the 2026-09-05 row, which was a floor taken while the PR was open; it retires the uncertainty in the paragraph above.

The second measurement falsifies the universal an earlier draft of this section drew from the first, which said this repository "cannot exercise thread ingestion at all". None of those five PRs is in the merge list above: #12, #132,

224 and #352 are older than every member, and #569 merged on 2026-09-05

after the window that list covers — it is not among the 38 refs enumerated there. (This sentence read "#569 is not merged at all", which was true when it was written and stopped being true the same day: the merge landed at 15:55Z. The claim it carries is unchanged, because what the argument needs is that #569 is outside the merge list, not that it is unmerged.) The narrow 0/0 reading was true; the universal was not, and it is withdrawn here rather than softened.

What remains true is the ground the fixture decision actually rests on: this project's review rounds happen in agent transcripts and land as commit trailers — which is why ADR-0029's arm exists — so the native GitHub-side population is thin (11–12 threads across the repository's whole history), bot-authored (every root comment from one scanner), and mutable (it grew by one during a single review round, on an open PR). Dogfooding on this repository is worth doing and is not a substitute for a controlled corpus. Recorded so that no completion claim for Milestone 8 overstates what was exercised.

So the serve slice's real-run verification data source is decided now, not at slice 3. Verification runs against a named disposable public fixture repository under the theurian org — for example review-ingestion-fixture — carrying a planted, frozen PR and thread set; it is created when slice 3's verification needs it, allowlisted only in the verification configuration, and never in a shipped default. The ground is the population above, and its movement during this very review is the argument: assertions need data that does not change under them, and the native threads are thin, bot-authored, and demonstrably live — one arrived mid-round. The rejected alternative is recorded: an external third-party public repository, rejected for the same reason one step further out, its content being owned by someone else entirely.

Demoted populations

A design record states decisions; it cannot stay in bijection with a moving tree. Four review rounds each found a correct pasted key standing beside a hand-maintained member list that had gone stale — a mispointed anchor, an unjudged line, a count under two different units. The lists are therefore relocated, not deleted: each one below keeps its key, its frame and its judgment rule here, and its annotated membership lives in the slice-1 assignment brief, where the standing dispatch rule re-measures it at assignment time rather than trusting a figure written weeks earlier.

Demoted population What stays in this ADR New home Re-measured by
Flag-sites annotated population (M/N/D per line, and its composition) the key, both frames, the exclusions with measured drops, the three-class judgment rule slice-1 assignment brief the assignment-brief counting rule (CLAUDE.md, The orchestrator has no reviewer: a "how many places" claim is answered by a search command with its output pasted into the brief); last full annotation: PR #570 comment 5552854865
Fetch-absence prose population the key and its stated width, and the binding sequencing (it moves in the same PR as the first spawn site) slice-1 assignment brief the same rule
Milestone-7 attribution union the unfiltered key, the three-class rule, and the partition identity (movers + history + unrelated = the key's line count) slice-1 assignment brief the same rule
providers.review.repositories promise population the key, its recorded limit (it misses the differently-spelled promises), and the commit-split reason slice-1 assignment brief the same rule
Line-anchored citations converted to quoted sentences the quoted sentence and its file — (it travels with the quote) a quote survives renumbering; a line number does not

Rows that quote their anchor sentence carry no line number, deliberately: this PR's own owner repoint moved two cited line numbers inside its own branch, which is the whole argument.

Slicing plan

Slice Contents Notable ordering
1 — ingest the allowlist reader as its own commit (with the schema-pattern tightening, decision 3); then domain fidelity (decision 5); then the adapter, the ten clause tests, and the document corrections The split is justified by a pinned test, not by disjointness — see the note below, which measures the overlap the earlier draft assumed away
2 — land evidence files under .theurian/review/, the ingestion-time secret-scan gate, the CLI entry point The CLI verb is slice 2's to choose; naming one here would assert a command that does not exist
3 — serve the SQLite serving store, review.search, its wire schema, the capability flag and scope field, and the T-7 / roadmap / mcp-tools.md updates that follow from serving The disclosure round and the two-corpora fixture belong to this slice

Why the allowlist reader is its own commit — the real reason, measured. An earlier draft said the two commits touch different file sets. They do not: of the 15 files naming providers.review.repositories, 11 are also in the fetch-absence population at 1fe3302b (12 on this branch, the twelfth being this ADR, which is a member of both populations it measures):

$ comm -12 <(git grep -l -i -E "reviewIngestion|nothing (here )?can reach out|no external fetch|contacts no repository|never fetche[sd]|first external fetch" -- . ':!.claude' | sort) \
           <(git grep -l 'providers\.review\.repositories' -- . ':!.claude' | sort) | wc -l
      12

The justification is a pinned test, not disjointness: tests/unit/test_config_key_call_sites.py asserts by equality that nothing reads the key, so the reader's commit is the one that must flip that pin and rewrite the sentences resting on it. Bundling it with the adapter would put two independently-revertable claim flips in one commit. Because the file sets overlap, the two commits are sequenced serially in one worktree, never fanned out.

Alternatives considered

Alternative Why rejected
An in-process HTTP client (httpx plus a token from the environment) Three costs at once. It adds a production dependency to a core whose runtime dependency list is six packages, none of them an HTTP client. It puts token custody inside Theurian — reading, holding and possibly logging a credential the operator currently keeps in their own credential store. And it creates a raw-URL surface, which makes all three of SEC-10's controls live checks that must be built and kept correct. The gh api graphql form does not make the SSRF class disappear — run C and runs D–F show the destination moving under a pinned hostname — it removes the input those two checks read: there is no URL in the argument vector to apply a scheme allowlist to, and the private-network reach is reduced by constructing the environment (clause 4) rather than by inspecting a destination, with the config-family residuals recorded (decision 1 derives the four-member divergence class that survives slice 1's pre-spawn refusal). The gh spawn trades a process boundary for that, and buys a smaller reduction than an earlier draft of this row claimed.
Inherit the parent environment and scrub the dangerous variables A scrub is a blocklist over a set the adapter does not control. Run C measured that HTTPS_PROXY moves the request even with --hostname pinned, so the blocklist would have to be complete over destination variables, proxy variables, and whatever the next gh release reads. Constructing the environment inverts the burden onto a set this project chooses and pins.
gh api --paginate Whatever the flag follows, the response chooses it rather than the adapter — and this design pins no version-specific semantics for it (clause 8 bounds the binary's version, not its behaviour). Excluding the flag is cheaper than characterising it: a GraphQL cursor is an opaque string in a typed variable, so the adapter still decides every destination it reaches.
gh api repos/{owner}/{repo}/pulls/... (the REST path form) Repository identity would travel in the URL position, where it is string-interpolated into a path. The GraphQL form carries the same identity as typed variables with no path to escape into, which is what makes clause 2 a checkable property rather than a promise about quoting.
Keep the review data under .theurian/cache/, as review-knowledge.md said before this change Upstream review comments are editable and deletable, so a discarded cache entry for a deleted comment is unrecoverable. Calling that a cache asserts a rebuild property GitHub does not provide.
Scan for secrets at index-build time only, in index build's signal mode (land and report, never refuse) This was the Codex reader's recommendation, and it was rejected on the source's own premise: index_secret_scan.py records rather than refuses because its content is already in the canonical store and already served, so refusal "would deny ranking without un-disclosing anything". Pre-landing, refusal genuinely un-discloses — nothing has been written and nothing served — so the propose accept gate is the matching one.
Make resolved_at required and fill it with the ingestion time or the last comment's time The API object carries no resolution timestamp (decision 5's introspection). A required field filled by the adapter is a fabricated measurement that every downstream consumer reads as real. None is the honest value for a quantity the provider does not record.
Invent a new tool name for review-evidence search review.search has been published as planned in docs/protocol/mcp-tools.md since before this design. A tool name is a wire contract; adding an eighth name would leave a published one orphaned and make clients choose between them.
Verify the serve slice against an external third-party public repository Its content can change or vanish under the assertions, so the harness's ground truth would be owned by someone else. A disposable fixture repository under this project's own org is frozen by construction.
Ingest private repositories in v1, gated by the allowlist alone The allowlist decides which repository is contacted; it says nothing about whether the material is safe to serve. Private ingestion needs the securityRelated marking and the uniform serve refusal ADR-0029 specifies, and building that is a disclosure-class design in its own right — folding it into the first fetch path would ship the transport and the disclosure boundary in one review round.

Compliance

This ADR ships no behaviour, so it has no shipped test to name. Its enforcement at design time is the measurements it cites; its enforcement at implementation time is the tests the slices owe. The names below are the properties an implementation must pin, not files that exist today — the same honest split ADR-0029 states for the same reason.

Measured now, and reproducible from this ADR (2026-09-05, origin/main @ 1fe3302b):

  • The published review.* tool table has 7 rows, one shipped and six planned, which is why serving needs no new name:

console $ grep -n '^| `review\.' docs/protocol/mcp-tools.md 352:| `review.findings` | Shipped | Landed `Review-Finding:` trailers, filtered by reviewer, severity, commit or text | 353:| `review.search` | Planned | Search review history | 354:| `review.getThread` | Planned | One thread with comments and resolution | 355:| `review.findSimilar` | Planned | Threads resembling a described situation | 356:| `review.getDecisions` | Planned | Decisions reached in review | 357:| `review.generateKnowledgeCandidate` | Planned write-intent | Emit a proposal; no approved-state write | 358:| `review.listUnresolved` | Planned | Open threads |

Slice 3 did what this measurement predicted: the review.search row moved from Planned to Shipped in place, and no row was added. The block above is left at its own commit, because a measurement re-taken every time the tree moves stops being a measurement.

  • ReviewResolution has no consumer outside its defining module: two hits, both in domain/review.py (git grep -n "ReviewResolution" -- packages/theurian-core/src).
  • The core's runtime dependency list is 6 packages, none an HTTP client (packages/theurian-core/pyproject.toml).
  • Three populations slice 1 must move, each with its key in Dispositions and each a dispatch input to be re-measured then: the fetch-absence prose (30 files, wide phrase key), the providers.review.repositories key (15 files), and the Milestone-7 attribution (the demoted union above — its numbers live with the annotation, per Demoted populations). The first two overlap in 12 files on this branch (11 at 1fe3302b), which is why the allowlist reader's commit is sequenced serially before the adapter's rather than fanned out — the split is justified by test_config_key_call_sites.py's pinned absence, not by disjointness.
  • This ADR is a member of both prose populations it measures, which is why every one of those counts is stated against a named commit rather than as a property of the repository.
  • The scope claim is a stable property, not a snapshot. "Only paths under docs/" was true when it was written and false by the next fix commit — a branch-scope measurement is invalidated by the commits that answer the review, which is what made it wrong for four rounds. What holds for the whole branch and keeps holding:

console $ git diff origin/main...HEAD --name-only -- ':(glob)packages/*/src/**' 'plugins' 'schemas' | wc -l 0

No shipped source, no plugin, no schema. The branch does touch four non-docs/ files, each non-behavioural and each named here rather than counted: SECURITY.md (a governed record, decision 3); tests/unit/test_phase0_exit_records.py (one docstring word, 2930); tests/unit/test_project_and_traceability.py (a parametrised illustration string, target unchanged — is_derived keys on the first path segment); and tools/audit/owner_position_cites.py (ledger rows and their reasoning, which the audit reads as data).

Quoted, not re-run here — design consult, 2026-09-05, gh 2.86.0: runs A, B and C; the PullRequestReviewThread field list; the observation that http_unix_socket is a configuration key on that version while api_host is not; and the 0/0 thread count over the last-40-merged window. Round one's adversarial review, same date and binary: runs D, E and F, and the by-PR-number thread population (11 threads across #12, #132, #224, #352, #569, plus 5 top-level reviews) — run D and the thread population were re-run by the orchestrator and reproduced. Slice 1 re-runs D–F as driving tests — that is where they become controls instead of quotations. A–C cannot be re-run in a test: each one needs a real outbound request to api.github.com or a hostile host, which no suite here makes. They stay quoted, and they are re-taken by hand when clause 8's version floor moves, since what they measure is a property of the binary.

Owed at implementation, each tied to the slice that discharges it. A slice that lands moves its own items from owed to landed here and names the test that discharges each; an item a slice did not reach stays on the list with the slice that will, and is never quietly dropped.

Slice 1 — ingest (shipped at 7c486588. The discharges below were filled in during slice 3 (#603), which is late: the list stood as owed for two slices while the tests existed, and a list that says "owed" about work that shipped is as wrong as one that says "landed" about work that did not. Paths are packages/theurian-core/tests/, and every name here was resolved against the tree rather than written from the slice's own brief. Three items are not discharged by a test and say so by name.)

  • Landed in slice 1 — ten clause tests, one per row of decision 1's table. Clause 1, the single spawn site: unit/test_network_call_sites.py::test_no_module_outside_the_recorded_spawn_sites_can_start_another_program (the equality pin, grown by one) with ::test_the_process_scan_sees_each_spawning_form_and_no_other as its positive control, and unit/test_gh_argument_vector.py::test_only_the_recorded_modules_name_the_spawn_helper_or_the_class_that_calls_it for the second half of the clause. Clause 2, the literal endpoint with identity in variables: ::test_the_endpoint_element_is_the_literal_graphql and ::test_no_vector_element_is_built_by_formatting_a_repository_name. Clause 3, the host pin: ::test_the_hostname_is_pinned_in_every_vector_that_makes_a_request. Clause 5, the absolute binary and no shell: ::test_the_first_vector_element_is_an_absolute_path and ::test_the_spawn_module_reaches_no_shell. Clause 6, no --paginate: ::test_paginate_is_absent_from_every_vector and ::test_a_second_page_changes_only_the_cursor, with the cursor read itself at integration/test_gh_review_provider.py::test_a_second_page_is_asked_for_with_a_cursor. Clause 7, the timeout and the two caps as named constants: unit/test_gh_argument_vector.py::test_each_recorded_bound_is_the_value_the_prose_names and ::test_every_recorded_limit_has_a_test_side_home, with the graded stops at integration/test_gh_review_provider.py::test_a_limit_past_the_recorded_cap_stops_before_spawning, ::test_a_response_that_never_stops_paging_is_stopped_by_the_page_cap and ::test_a_thread_past_the_comment_cap_is_reported_not_truncated, and the timeout at integration/test_gh_bounded_read.py::test_a_child_that_never_answers_is_stopped_at_the_recorded_timeout. Clause 8, the version floor: integration/test_gh_review_provider.py::test_a_gh_below_the_floor_is_refused_and_the_message_names_the_floor. Clause 9, the two graded envelopes with the child's stderr contained: ::test_no_gh_on_the_path_is_a_graded_refusal_not_a_traceback, ::test_an_unauthenticated_gh_is_a_graded_envelope_carrying_its_own_stderr and ::test_a_failed_query_reports_the_childs_own_words_inside_the_envelope. Clause 10, the per-response byte cap and the read shape that makes it real: integration/test_gh_bounded_read.py::test_a_child_that_overruns_the_cap_is_refused_without_waiting_for_it_to_finish — the bounded wait is what separates an incremental read from an accumulating one — with ::test_a_child_at_the_cap_exactly_is_read_rather_than_refused and ::test_a_cap_refusal_leaves_no_child_behind.
  • Landed in slice 1 — clause 4's tests (i), (ii-a) and (ii-b), exactly as clause 4 specifies them. (i) is unit/test_gh_child_environment.py::test_the_child_environment_is_exactly_the_recorded_mapping, written test-side rather than read from the production constant — which is what makes it killable — with ::test_the_expected_mapping_can_fail as its control and integration/test_gh_review_provider.py::test_the_child_receives_the_constructed_environment_and_nothing_else observing the same mapping on the far side of the process boundary. (ii-a), the refusal driver that needs no gh, is ::test_a_planted_transport_override_refuses_before_any_binary_probe. (ii-b), the residual demonstration through a test seam, is integration/test_gh_transport_residual.py::test_with_the_refusal_bypassed_the_request_leaves_through_the_socket with ::test_without_the_seam_the_same_fixture_reaches_the_socket_zero_times as the control that makes the zero mean something; only (ii-b) skips, and it reports the skip. The rationale lives in clause 4 and is not restated here: round two corrected it in the clause and left this bullet carrying the superseded version byte-unchanged, which is the two-places class this ADR keeps meeting. (An earlier version of this bullet cited "(i)'s companion" as a fourth item; the companion is a property of (i)'s form, not a separate test, so the citation resolved to nothing.)
  • Landed in slice 1 — the allowlist is consulted before the spawn. integration/test_gh_review_provider.py::test_an_unallowlisted_repository_starts_no_process asserts the spawn recorder is empty rather than that the result was filtered; the refusal itself is unit/test_review_allowlist.py::test_an_unlisted_repository_is_refused_with_its_grade, and ::test_an_empty_allowlist_allows_nothing holds the direction an absent list fails in.
  • Landed in slice 1 — a private repository is refused at ingestion. integration/test_gh_review_provider.py::test_a_private_repository_is_refused_at_ingestion, with ::test_a_repository_that_does_not_resolve_as_public_is_refused for the answer that is neither public nor private and ::test_a_private_repository_is_refused_on_the_reviews_read_too for the second read, which would otherwise be a seam the first test never reaches.
  • Landed in slice 1 — a renamed repository is refused. integration/test_gh_review_provider.py::test_a_rename_redirect_is_refused_rather_than_followed, held from the other side by ::test_a_resolved_name_contained_in_the_entry_is_still_a_different_repository and ::test_a_case_difference_is_not_a_rename — without the second, a check that refused every case difference would pass the first while breaking ingestion of a correctly allowlisted repository.
  • Landed in slice 1 — evidence paths are containment-safe before the key is load-bearing. The schema-pattern half is unit/test_review_allowlist.py::test_the_pattern_this_module_enforces_is_the_one_the_schema_publishes and ::test_a_traversal_or_malformed_name_is_refused; the path half is unit/test_project_paths_containment.py::test_every_path_helper_refuses_when_a_committed_symlink_escapes_under_it, whose population is derived rather than listed by ::test_the_containment_sweep_covers_every_path_returning_helper, so ProjectPaths.review joined that sweep by existing.
  • Landed in slice 1 — test_network_call_sites.py's absence claim is retired in the same commit that admits the site. Same pin as clause 1: unit/test_network_call_sites.py::test_no_module_outside_the_recorded_spawn_sites_can_start_another_program, an equality that fails on an addition and a removal, with ::test_the_process_scan_sees_each_spawning_form_and_no_other proving the scan can see one. The file's own admission checklist is prose beside the pinned set and is not separately held.
  • Landed in slice 1 — a pre-spawn refusal of transport-override keys in the precedence-resolved gh config — best effort, reducing only the accidental single-well-formed-file case and leaving decision 1's four-member divergence class, with clause 4's tests as its drivers. Its spec: the same precedence chain gh resolves (empty-string variable treated as absent), and the check runs before any binary probe. unit/test_gh_transport_guard.py::test_a_planted_transport_override_is_refused_before_anything_is_spawned is the refusal; the precedence chain is ::test_gh_config_dir_wins_over_both_others, ::test_an_empty_gh_config_dir_falls_through_exactly_as_gh_does and ::test_the_override_is_seen_through_every_locator_gh_resolves; and ::test_a_configuration_that_moves_nothing_refuses_nothing is what stops the check from being satisfied by refusing everything.
  • Still owed, and not a test obligation — the known-key set is re-taken whenever clause 8's version floor moves. Member (b) is bounded only by what that version of gh understands, so nothing in this suite can discharge it: a test can only hold the set this build looks for, which unit/test_gh_transport_guard.py::test_the_known_key_set_is_what_the_check_looks_for does. The obligation is a hand re-measurement against the new binary, owed by whoever raises the floor constant, and it is recorded here rather than dressed as discharged.
  • Landed in slice 1, prose with one pinned half — every T-7 sentence is rewritten per control, recording the repository allowlist as discharged on the gh path and private-network rejection as reduced with its residual class named (decision 1's four-member divergence class), never as discharged, with #429 still owning the raw-URL context. The only half a test holds is the spawn-site enumeration: unit/test_threat_model_t7_claims.py::test_the_t7_spawn_bullet_names_every_pinned_spawn_site_and_spells_how_many. That the per-control rewrite is faithful is a reading, and no mechanical check reaches it — said here rather than left to be inferred from a name beside it. (The three line-anchored cites this bullet carried — threat-model.md:6454, requirements-analysis.md:1352, roadmap.md:272 — are dropped under this document's own Demoted populations rule: a line number does not survive an edit to the file above it, and the sentences are findable by the control each names.)
  • Landed in slice 1, and discharged by a measurement rather than a test — the three populations move in this PR: the fetch-absence prose, the providers.review.repositories sentences, and the Milestone-7 attributions. Each key was re-run on the branch and its output pasted into PR #583's Populations, re-measured section, which is the record; nothing in the suite re-runs them, and a later drift is caught by the next assignment's re-measurement rather than by a RED.
  • Landed in slice 1, pinned in slice 3 — the sites recording the old reviewIngestion meaning are rewritten — the key, its exclusions and its output are in decision 2 — two slices before the flag flips. Test sites are among the movers, one of them a byte-pinned constant, so the sweep is not a prose pass. The annotated population and its composition live where the brief does (Demoted populations), not in a count here that would go stale beside a correct key. The sweep left nothing holding the result, which is what slice 3 added: unit/test_review_ingestion_flag_claims.py::test_the_four_sites_spell_the_never_meant_sentences_one_way holds the four sites' shared sentences against each other, and ::test_each_record_narrates_the_flag_the_capability_dict_publishes holds each narrating record to the era the live flag value selects, with ::test_the_docstring_checker_demands_the_other_era_when_the_flag_moves as its control. Its population is enumerated, not derived: a ninth narrating record would not redden it, and that bound is recorded in the module's own docstring.
  • Half landed in slice 1 — the domain-model break is recorded. The CHANGELOG half landed: packages/theurian-core/CHANGELOG.md's ### Changed section (the level the file actually uses; this item said #### Changed) carries two BREAKING entries naming the old shape and the new one, for ReviewResolution and for ReviewEvent. The commit half did not. The merged commit 7c486588 records the break as prose inside its body — "(BREAKING: ReviewResolution's field order and optionality — no consumer existed to migrate)" — and carries no BREAKING CHANGE: footer; git log origin/main --format='%B' | grep -cE '^BREAKING CHANGE:' answers 1 across the whole history, and it is not this commit. History is not rewritten for it, so what is owed is the next break on this path taking the trailer, and this item is left here saying so rather than marked landed. No test holds either half.

Slice 2 — land

Four of the five landed in slice 2 and each names the test that discharges it; the fifth was half discharged there and its second half landed in slice 3, named in place below. Paths are packages/theurian-core/tests/.

  • Landed in slice 2 — a flagged record under block never becomes a file. unit/test_review_landing_gate.py::test_under_block_the_flagged_record_never_becomes_a_file plants a synthetic secret and asserts the file is absent; ::test_no_report_line_carries_the_matched_bytes holds the report to identities and a bounded family prefix; and ::test_under_block_a_flagged_record_does_not_stop_another_pull_request_landing holds the refusal to one record. That the run does not read as clean is the CLI's half: integration/test_review_ingest_cli.py::test_block_exits_one_and_the_flagged_unit_is_absent_on_disk.
  • Landed in slice 2 — warn lands and reports; off scans nothing. unit/test_review_landing_gate.py::test_under_warn_the_record_lands_and_every_finding_is_reported and ::test_under_off_the_detector_is_never_called, the second with its own positive control (::test_the_counter_would_have_seen_a_scan_under_the_default_policy) so that "never called" is not satisfied by a detector nothing reaches. The CLI's exit codes are integration/test_review_ingest_cli.py::test_warn_exits_zero_and_still_reports_the_finding and ::test_off_scans_nothing_and_lands_everything.
  • Landed in slice 2 — the scan reads exactly the author-controlled fields of decision 3's table. unit/test_review_landing_gate.py::test_a_secret_planted_in_any_author_controlled_field_refuses_the_record is parametrised one case per field, so a label and a head branch name are driving inputs rather than an argument; the boundary is held from the other side by ::test_a_secret_shaped_value_in_a_structural_field_does_not_refuse and ::test_the_url_is_structural_even_though_a_person_can_choose_a_branch_in_it, without which a gate that scanned every string on the object would pass every positive case.
  • Half landed in slice 2 — evidence files are the source. The write→read-back half is unit/test_review_evidence_store.py::test_every_record_kind_reads_back_as_the_object_that_was_written, and the survival half is ::test_a_record_upstream_no_longer_returns_survives_the_refetch with ::test_a_refetch_rewrites_a_record_whose_content_changed_upstream as the positive control that keeps a store writing nothing at all from passing it. The second half landed in slice 3, which is when it first became expressible — there was no derived store to delete until slice 3 built one: integration/test_review_search_rebuild.py::test_a_deleted_store_rebuilds_to_the_same_answers_from_the_same_evidence throws the store away, rebuilds from the files alone and compares the served answers, with ::test_two_builds_agree_whatever_order_the_evidence_records_arrived_in and ::test_the_served_order_does_not_depend_on_the_order_records_were_written holding the reproduction against the one input a filesystem is free to vary.
  • Landed in slice 2 — FR-V5, made checkable. integration/test_review_ingest_is_model_free.py::test_no_callable_in_the_built_pipeline_reaches_a_model walks the built ingest pipeline's object graph in the shape of test_no_registered_tool_can_reach_a_canonical_write, with ::test_the_walk_reaches_the_pipeline_it_claims_to_inspect proving the walk is not vacuous and three planted-model cases (::test_a_model_planted_on_a_service_method_reddens_the_walk and its two siblings) proving it can go RED. Decision 5's "no model exists anywhere in the ingest path" is therefore a measured property rather than design intent — with the bound that module's docstring states: a provider resolved through a factory one level down is invisible to it.

Slice 3 — serve

Five of the six landed and each names the test that discharges it; the sixth is conditional and its condition is unmet, which is a different state from owed and says so. Paths are packages/theurian-core/tests/.

The slice closed at round five under the fixed-round-budget ruling, with one HIGH open and release-gating (2026-09-11). CRITICAL was zero throughout; #636 — the empty-publish guard keying on read-time facts while the build held the publish-time listing that decides them — was filed rather than fixed and was the first post-merge item. It touched none of the discharges below: no arm of the two-corpora equality, the triple, the flag, SEC-13 or the bound-input battery rests on that guard.

That HIGH is discharged, so the 0.2.0 cut is no longer waiting on it (PR #637). The guard is keyed on at_the_publish — the listing taken inside the write section — together with the withholding outcome, and on nothing the read alone saw, which is what separates a stale build from an operator's deletion: a build that read nothing while the corpus is there at the publish refuses instead of emptying a store a landing had just filled, and a corpus emptied whole inside another build's window publishes the empty store instead of being refused while the store goes on serving records whose files are gone. Both faces are driven through the shipped CLI by integration/test_review_build_empty_publish.py::test_a_build_that_read_nothing_refuses_rather_than_emptying_a_store_a_landing_just_filled and ::test_a_corpus_an_operator_emptied_inside_the_window_is_published_rather_than_refused, which hold the read-to-publish window open with a barrier at the read seam and assert what the read returned rather than relying on a race; the two-process reproduction that filed the issue is #636's own and is not re-run there, which that module states as its cost. The rows nothing else reached are integration/test_review_search_builder.py::test_a_build_that_read_nothing_while_a_landing_filled_the_corpus_refuses and ::test_a_build_that_kept_none_of_what_it_read_publishes_when_the_corpus_is_gone, the withholding arm is ::test_a_build_that_withheld_everything_publishes_whether_the_files_are_there_or_not with ::test_an_all_withheld_build_and_a_purpose_emptied_corpus_are_one_observable. That pairing's "one observable" is three named ones and not every observable, so this record states the bound rather than the phrase: what an all-withheld build and a build whose corpus is gone at the publish may not differ in is whether the build raises, what the store holds, and what the report publishes — every key of it but withheld. The axis is the listing and not an operator's intent, which is the axis the guard reads: a corpus still there while the read found nothing is the refusing input, and whether it never held the records or somebody had just emptied it is not something entries can say. The case standing on that row is integration/test_review_build_empty_publish.py::test_a_build_over_an_emptied_corpus_a_writer_landed_into_refuses_and_leaves_the_store.

Duration is outside that claim and was measured, not argued away: the all-withheld build parses every evidence file and a corpus that is gone has nothing to parse, so the two differ in duration. Two figures were taken in round one, and they are two instruments rather than two points on one curve. 1.75× at 60 records is wall-clock over the shipped CLI, where a process-startup floor of roughly 27 ms sits in both numerator and denominator and dilutes the ratio (security review's instrument); 29.8× at 200 records is the builder method driven directly, with no process to start (adversarial review's harness). Neither number extrapolates to the other's instrument, and no curve is claimed here. It is not a channel: the only parties that can time this build are the ones that ran theurian review build or theurian review ingest in their own working tree — both reach it through review_commands.rebuild_search_store — and either can read .theurian/review/ directly, while no MCP tool reaches the build at all (review.search reads the store a build produced). What would re-grade it is a caller able to provoke a build without reading the corpus, and none exists. The key itself is pinned from both sides by unit/test_review_search_builder_claims.py::test_the_empty_publish_guard_is_keyed_on_the_publish_time_capture and ::test_the_build_records_the_decision_its_guard_makes.

  • Landed in slice 3 — every author-controlled field carries the SEC-15 triple, bound by import. integration/test_review_search_tool.py::test_every_served_record_carries_the_imported_safety_triple compares a served row against theurian.mcp.results.SAFETY itself rather than against three re-spelled literals, with ::test_the_triple_assertion_reddens_on_a_re_spelled_drifted_triple as the companion that proves the check can fail and ::test_the_shaper_follows_the_imported_object_and_not_three_typed_literals holding the shaper to the imported object. The population is decision 3's field table: ::test_every_author_controlled_field_is_either_served_under_the_triple_or_not_served is what reddens for a field added there without a disposition, with ::test_the_corpus_really_plants_each_unserved_field keeping it from passing over fields no fixture carries, and unit/test_schemas.py::test_the_published_review_record_fields_are_the_ones_the_shaper_classifies and ::test_the_published_review_record_safety_triple_is_the_one_the_code_attaches hold the published schema to the same classification.
  • Landed in slice 3 — the two-corpora equality, at both layers. At the store: integration/test_review_search_absence_proof.py::test_every_query_in_the_battery_answers_identically_over_the_two_corpora, with ::test_no_generated_query_separates_a_withholding_store_from_one_that_never_held_the_records as the generated-input arm. At the tool, which is the layer a caller actually reaches: integration/test_review_search_tool_absence_proof.py::test_every_query_in_the_battery_answers_identically_over_the_two_corpora, with ::test_the_battery_really_reaches_the_withheld_records and ::test_the_control_serves_the_withheld_records_through_the_very_same_tool keeping the equality from being satisfied by a battery that reaches nothing, and ::test_no_generated_request_separates_the_two_corpora_at_the_tool. Three channels beyond the field values are held separately, because "every published field" does not cover them: ::test_a_withheld_record_never_costs_a_visible_one_its_slot_in_the_response (displacement), ::test_the_page_boundary_bit_does_not_move_with_a_withheld_record (truncated), and ::test_both_published_copies_of_one_response_carry_the_same_payload — which exists because the SDK sends the payload twice and a comparison that read only one copy was comparing None to None.
  • Conditional, and the condition is unmet — a ranked surface over review evidence ranks the T-17a-purged population, if one is built. None is built: this slice's read has no score, no term weight and no collection statistic, so there is no build-time statistic for a withheld row to price, and that absence is structural rather than asserted — integration/test_review_search_absence_proof.py::test_the_store_schema_carries_no_full_text_index_for_a_withheld_row_to_price recomputes it from the store's own schema, with ::test_no_trace_of_a_withheld_record_survives_in_any_table_or_any_byte_of_the_store holding the physical absence the argument rests on. The item stays on this list for whoever builds a ranked surface; it is not discharged, because a filter does not clean FTS5 collection statistics and nothing here proves it would.
  • Landed in slice 3 — the capability flag and its scope field. integration/test_mcp_tools.py::test_capabilities_report_what_is_and_is_not_built holds both values together, ::test_the_capability_block_holds_exactly_the_flags_that_are_pinned and ::test_the_system_capabilities_response_holds_exactly_the_keys_that_are_pinned are the two equality pins the slice had to move to land the flag, and the wire half is integration/test_wire_contract.py::test_a_real_capabilities_response_validates_against_its_published_schema with ::test_the_capabilities_conformance_check_can_fail. The response schema it validates against is new in this slice. (This bullet cited the flag-value assertion by line number, :1980; the line has moved, and the pins are named here instead — see Demoted populations.)
  • Landed in slice 3 — SEC-13 cross-project isolation over review evidence. integration/test_review_search_tool.py::test_a_review_search_for_one_project_cannot_observe_the_other. The refusals are held to the same standard, since a message that varied with a project's contents would be the same leak one layer over: ::test_a_bad_filter_is_refused_the_same_way_whether_or_not_the_project_resolves, integration/test_review_search_tool_absence_proof.py::test_an_empty_corpus_refuses_like_a_full_one_and_serves_a_well_formed_query, and the provenance arm integration/test_review_search_tool.py::test_a_store_this_installation_did_not_build_is_refused_in_the_words_a_missing_one_gets.
  • Landed in slice 3 — review.search query input is bound, not interpolated. integration/test_review_search_bound_input.py::test_a_sql_statement_in_a_filter_is_a_value_and_alters_nothing and ::test_a_wildcard_in_a_structural_filter_is_a_character_and_not_a_pattern hold the binding; ::test_operator_shaped_search_text_selects_only_the_record_that_spells_it holds operator syntax as text rather than as query structure — the ADR wrote that as FTS5 syntax, and the surface that shipped is a LIKE with no query language at all, so what is driven is the operator vocabulary a caller would reach for against either; and ::test_every_text_field_of_a_query_refuses_a_value_sqlite_cannot_be_handed holds the two byte shapes that cannot cross the boundary. At the tool: integration/test_review_search_tool.py::test_operator_syntax_in_the_query_arrives_at_the_store_as_text.