Deepen the Trusted account rule behind its own module

[JacobStephens2]

Summary

The Trusted account rule moves out of the GitHub adapter and into a domain trust module. The module reads GitHub only through a four-read Reads interface. What users see does not change.

 src/
+├── trust.rs           # the rule: issue_trust, ticket_trust, untrusted_edit,
+│                      #   UntrustedIssue, IssueText, trusted_association,
+│                      #   Reads, in-memory unit tests
 └── github/
-    └── trust.rs       # the rule woven through gh calls
+    └── trust.rs       # impl trust::Reads for GitHub: queries, paging,
+                       #   parsing, membership status (204 / 404 / error)

issue_trust(reads, issue)                  one Check, trusted logins cached for it
  author_association        → Author
  ready_labellings().last   → ReadyLabel   (no actor, or not owner/member)
  edits(Body), then edits(Title), each edit read as reached → Edited(text)
    an edit at or before the labelling is skipped; an unknown actor is untrusted
ticket_trust(reads, ticket, spec)          edits since the later trusted labelling, or every edit
untrusted_edit(reads, issue, since)

The four callers now call the module with &GitHub::new():

• Ready issue search: GitHubAndOrigin's own Reads::issue_trust forwards to the module.
• Spec run: start_ticket.
• Direct Run: the warning.
• Delivery: no_untrusted_edit_since.

docs/code-navigation.md points at the new owner.

Seams under test:

• trust's three checks, over an in-memory Reads.
• Each caller's wiring, plus the GitHub adapter's reading, through the gh fake.

Evidence

The integration tests that only varied the rule are replaced by inline unit tests at src/trust.rs. These cover every case the issue lists:

• untrusted author;
• no labelling;
• a labelling with no actor;
• an untrusted labeller after a trusted one, and the reverse;
• edits at or before the labelling;
• trusted, untrusted and unknown editors;
• title edits;
• Ticket since-times;
• the case-insensitive owner;
• no repeated membership read;
• a read error that propagates.

Integration tests kept or added:

• Wiring:
  • Pickup passes over an untrusted author.
  • A Spec run stops before an untrusted Ticket, and starts a Ticket edited before its Spec's labelling.
  • Delivery stops on an untrusted edit.
  • A direct Run warns and still delivers.
• Adapter reading:
  • Label events filtered and membership endpoints read.
  • An unknown editor's null login, for body and title.
  • A ready labelling with a null actor.
  • Edit history read past its first page (new).
  • A failed membership lookup.

The review's C1 finding came with a test: an_untrusted_edit_is_found_before_a_later_edit_with_no_time.

• Before (first commit, edits parsed up front): test an_untrusted_edit_is_found_before_a_later_edit_with_no_time ... FAILED
  After (each edit read as the rule reaches it): ... ok

• Final: cargo test: 50 test binaries with no failures, including 1,089 unit tests (2 ignored). cargo fmt --check and cargo clippy --all-targets are clean.

Merge Danger

Door: two-way

The rule's verdicts, messages and gh traffic are unchanged, except that the trusted-login cache now lasts a whole check rather than one edit scan. This saves repeat membership reads, as the issue decided.

Blast Radius: security-gate

If the move were wrong, an untrusted issue could start work, or trusted work could stop. The unit tests pin each verdict, and each caller keeps a wiring test.

Unaddressed findings

Standards

• 1. Read-order and read-count assertions in src/trust.rs tests. Declined. The issue requires them: "The rule's order and laziness are kept … A login once found trusted is not looked up again within one check", and the unit-test list includes "no repeated membership read for the same login". The reads are gh API calls, so they are part of the interface the tests check.
• 2. Action as a Mysterious Name. Declined. It is the issue's "each with an optional actor and a time", shared by labellings and edits. The doc comment at src/trust.rs (/// A ready-for-agent labelling or an edit of an issue: the account that made it, if GitHub still knows it, and when.) says what it holds. No GLOSSARY term fits better.
• 3. trusted: Vec<(String, String)> keyed by repository. Declined. The reviewer rated it low priority. Keying by repository keeps the cache sound if a Ticket and its Spec were ever in different repositories, at the cost of one comparison.
• 4–7, 10, 11. No action was suggested.
• 9. The adapter test is named for how it reads. Declined. It is an adapter-reading test (ready_labels_are_read_from_label_events_and_their_actors_trusted_by_membership), where how GitHub is read is the behaviour under test.

The rest of finding 8 was addressed: a null-actor labelling is now read through the adapter, and the restored Spec run test sends an edit dated before the labelling through the adapter.

Spec

• C1, the remaining part. With since = None, which only ticket_trust uses when neither the Ticket nor its Spec has a trusted labelling, an edit whose time is missing now fails the check. Main never read times in that case. Declined. The issue's Reads gives "the issue's edits of one IssueText, each with an optional actor and a time", and GitHub's schema makes editedAt and createdAt non-null. The case the finding tested is fixed (see Evidence).

Security

None. The Security review traced the moved rule in src/trust.rs and the adapter reads in src/github/trust.rs against main. It found no boundary the change opens: the order of checks, the owner shortcut, how an unknown actor is handled, the per-repository cache of trusted logins, and failing closed on errors all match main.

Security review outcome

No unaddressed introduced findings.

Review coverage

Both reviewers read all 14 changed files at c05ed62, and no generated output was excluded. The fix commit ab6fac9 (written after the review) changes src/trust.rs, src/github/trust.rs, tests/pickup_run.rs and tests/spec_run.rs. The review did not read that commit's changes.

Closes #862

🤖 Generated with Claude Code

Built with claude · claude-opus-5-5 · medium