Repository navigation
Description
What to build
Move the Trusted account rule out of the GitHub adapter into a domain module of its own, behind a small reads interface, so that every edge case of the rule can be tested in memory. What users see does not change. Today the rule is spread through the GitHub adapter's gh calls, and the only way to test it is the gh fake. The rule covers:
- the author check;
- which
ready-for-agentlabelling counts; - for a Ticket, the later of its own and its Spec's trusted labellings;
- untrusted edits since a time;
- an edit whose account GitHub no longer knows;
- the repository owner shortcut.
Four callers use the rule:
- the Pickup run's Ready issue search;
- a Spec run before it starts each Ticket's Run;
- a direct Run's warning;
- Delivery's check before each session.
They keep the same verdicts, messages and failure behaviour.
Acceptance criteria
- A domain
trustmodule owns the rule. Its interface is three checks: issue trust, Ticket trust (Ticket plus its Spec), and the first untrusted edit since an optional time. It also owns theUntrustedIssueandIssueTexttypes and theauthorAssociationtest that session issue hand-over shares. - The module reads GitHub only through a
Readsinterface with four reads:- the author's association;
- the issue's
ready-for-agentlabellings, oldest first, each with an optional actor and a time; - the issue's edits of one
IssueText, each with an optional actor and a time; - whether a login is a collaborator on the repository or a member of the organization that owns it.
- The GitHub adapter implements
Readsand holds only queries, paging, parsing and the membership status reading (204 means yes, 404 means no, anything else is an error). It no longer decides trust. - The rule's order and laziness are kept: the author first, then the latest labelling, then edits, body before title, stopping at the first failure. The owner check is case-insensitive and needs no read. A login once found trusted is not looked up again within one check.
- The four callers call the
trustmodule with the GitHub adapter. The Ready issue search keeps its ownReads::issue_trust, which forwards to the module, so its in-memory tests stay as they are. - Inline unit tests use an in-memory
Readsadapter and cover:- an untrusted author;
- no labelling at all;
- a latest labelling with no actor;
- a latest labelling by an untrusted actor, even after an earlier trusted one;
- a latest trusted labelling that supersedes an earlier untrusted one;
- edits at or before the labelling, which are ignored;
- an edit after it, by a trusted account, by an untrusted account, or by an unknown account;
- a title edit as well as a body edit;
- a Ticket that uses the later of its own and its Spec's trusted labellings, or every edit when neither has one;
- the case-insensitive owner;
- no repeated membership read for the same login;
- a read error, which propagates and never grants trust.
- Integration tests that only vary the rule move to those unit tests. Two kinds stay:
- one test per caller showing the wiring (Pickup passes an untrusted issue over, a Spec run stops before an untrusted Ticket, Delivery stops on an untrusted edit, a direct Run warns and still delivers);
- the tests that cover adapter reading (paging, an unknown editor's null login, a membership failure).
-
docs/code-navigation.md's "Trusted account" row names thetrustmodule as the owner and the GitHub adapter's trust reads as the adapter. -
cargo testpasses, andcargo clippyandcargo fmt --checkare clean.
Blocked by
None (can start immediately)
Decisions
- Settle the rule in a domain module, not the adapter. The rule is security-critical domain policy, and today it is woven through
ghcalls with no unit tests. Moving it concentrates the knowledge in one module (locality) and gives four callers one interface (leverage). - Use a
Readsinterface with two adapters, GitHub and in-memory, rather than gathering every fact up front. Early returns and per-login membership lookups stay lazy: no events or edits are read for an untrusted author, and no title edits are read after a failing body edit. That keeps today'sghtraffic and the order of the checks. The two adapters make this a real seam, not a hypothetical one. - Read edits per
IssueText(body, then title). This keeps the existing short-circuit and keeps the two GraphQL queries apart in the adapter. - The owner shortcut and the trusted-login cache belong to the rule. The membership endpoints and the organization lookup belong to the adapter. The owner comes from the issue URL and is a rule fact. Which endpoints prove membership is a GitHub fact.
- The cache lasts for one top-level check. Today it lasts only within one edit scan. Widening it to the whole check changes no verdict, saves repeated lookups and gives the rule one place to remember trusted logins.
- Keep the GitHub side's file as the adapter's trust reads. Its queries and the membership reading stay there, so history follows the adapter code.
- Keep the Ready issue search's own
Reads::issue_trust. Its in-memory search tests depend on that seam. Only its GitHub implementation changes. - Leave the Pickup run's Claim history out of scope. It reads label history from the GraphQL timeline, while trust reads it from REST events. Unifying those reads is a separate change with its own paging risks.
- Replace tests rather than layer them. Rule variants move to unit tests at the module's interface. Integration tests stay only for each caller's wiring and the adapter's reading.
- No
GLOSSARY.mdor ADR change. "Trusted account" already names the concept, and the move is easy to reverse.
Architecture review
Strength: Strong · Dependencies: mock
Files: src/github/trust.rs, src/github.rs, src/github/session_issue.rs, src/ready.rs, src/run.rs, src/spec_run.rs, src/delivery.rs, docs/code-navigation.md
Before
flowchart LR
P[Ready issue search] --> G
S[Spec run] --> G
R[Run start] --> G
D[Delivery] --> G
subgraph G[GitHub adapter]
direction LR
rule[Trusted account rule] -.-> gh[gh calls]
end
F[gh fake] --> G
classDef leak stroke:#dc2626,stroke-width:2px;
class rule leak
After
flowchart LR
P[Ready issue search] --> T
S[Spec run] --> T
R[Run start] --> T
D[Delivery] --> T
T[Trusted account module] --> Reads((Reads))
Reads --> G[GitHub adapter]
Reads --> M[In-memory adapter]
classDef deep stroke-width:3px;
class T deep
Problem: the Trusted account rule lives inside the GitHub adapter, mixed in with gh calls, so its edge cases can only be tested through the gh fake.
Solution: a domain trust module owns the rule over a four-read Reads interface, with the GitHub adapter and an in-memory adapter behind it.
Wins
- Locality: the trust rule sits in one module
- Tests hit one interface, in memory
- The adapter shrinks to reads and parsing
- Leverage: four callers, one rule
- Integration tests keep only the wiring
Activity