Skip to content

refactor: extract the pull request host boundary (A7 step 1) - #676

Merged
Tryanks merged 5 commits into
mainfrom
feat/host-trait
Oct 9, 2026
Merged

Tryanks merged 5 commits into
mainfrom
feat/host-trait

Conversation

@Tryanks

@Tryanks Tryanks commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

Step 1 of #643 (A7 of #535): the host trait extracted from the finished GitHub code as a pure refactor. No second host.

runtime (pull_requests, pull_request_watch, pull_request_stacks, github settings glue)
   │  Arc<dyn Forge>, ForgeError{kind, description}, Summary, Discovered, Fingerprint, Tails,
   │  MergeSubmission, StackRebase
   ▼
services::forge            trait Forge (terms(key), reads, writes, sync, watch; defaults for
                           fingerprints and native stack writes) + neutral types + connect()
   ▲  impl Forge for GitHub (services/github/forge.rs): existing GitHub code, moved behind the trait
services::github           GitHubApi, credentials, reads, actions, stacks, watch (unchanged API)

core::pull_request         HostTerms + HOSTS = [GITHUB]; linking_instructions(), host_names() and
                           merges_or_closes() aggregate over HOSTS (also read by pull-request-mcp)
protocol                   PullRequestCapabilities (conversation + action state), opaque diff cursor,
                           reviewer id, label id
ui                         capabilities decide what is offered; permissions decide the rest

Part of #643

Evidence

  • Before: runtime imported GitHubApi, GitHubError, PullRequests, PullRequestReads, repository, stack_actions, pull_request_watch and matched GitHubError variants in pull_requests.rs, pull_request_watch.rs, pull_request_stacks.rs (host study §7.1).
    After: grep -rn "services::github\|GitHubApi\|GitHubError" crates/runtime/src crates/ui/src crates/core/src outside test files: no matches.
  • With HOSTS = [GITHUB], the linking block is byte-identical to origin/main's LINKING_INSTRUCTIONS, and the MCP description still reads "without asking GitHub." (checked with a throwaway comparison against origin/main, not committed).
  • strip_linking_instructions now strips any <pull_request_linking>…</pull_request_linking> block, not only the current text, so older persisted turns get the instructions label too.
  • cargo fmt --all --check: clean
  • cargo clippy --workspace --all-targets --locked -- -D warnings: clean
  • cargo nextest run --workspace --locked --no-fail-fast: 1089 tests run: 1089 passed, 13 skipped. An earlier fail-fast run had one failure in tcode-traverse::transport an_invitation_that_finds_no_path_pairs_at_a_typed_address, which passed alone and in the full rerun. That test uses a fake host and depends only on protocol and client, none of which this change touches.
  • cargo machete: no unused dependencies
  • No screenshots: nothing visible changes, because GitHub has every capability the UI checks. iOS, Android and Web run in CI only.

Tests touched, none of them with a changed assertion or expected value:

  • merge_or_close_detection_matches_words_in_the_raw_command moved from runtime to core with the same cases, where merges_or_closes now lives (step 3: moved to the owning boundary).
  • runtime pull_requests_tests.rs and pull_request_watch_tests.rs: setup only. The fixture GitHubApi is wrapped in GitHub::new, and the swapped summary service became a swapped forge. The instructions expected by one test are linking_instructions(). In tests.rs, state.github became state.forge built with forge::connect.
  • services pull_request_reads.rs: the two PullRequestReviewer literals gain id (GitHub's id is the login).
  • ui detail.rs paging test: the page number became the opaque cursor "2", and the conversation literal carries capabilities.
  • New: github_errors_keep_their_meaning_across_the_boundary, a neutral error round trip covering the rate-limit retry time and a disabled host.

Merge Danger

Door: two-way
Blast Radius: pull-requests
Wire changes, noted above PROTOCOL_VERSION: Files{cursor} and next_cursor replace page numbers, reviewers and labels carry ids, and the conversation and action state carry capabilities. A client and host from different commits on main disagree on these shapes, which is allowed between releases. The release feed now has its own anonymous GitHub transport. Only the release check used the anonymous ledger scope, so the release check sees no difference. pull-request-mcp now depends on tcode-core for the hosts' terms.

services::forge holds the Forge trait, a neutral ForgeError, capabilities
and the host-neutral types the runtime needs; GitHub implements it in
services::github::forge, its code moved rather than rewritten. The runtime
no longer imports GitHub types or matches GitHub errors. Linking
instructions, tool descriptions, the URL hint and merge/close detection are
parameterised by the host's terms. Diff pages are opaque cursors, and
reviewers and labels carry opaque ids.
Reply, resolve, reactions, the request-changes verdict, draft, reopen,
revert, auto-merge and update-branch are offered only where the host has
them; what the account may do still decides the rest.
…g capability

Forge::terms takes the pull request's key; the linking block, the tool
descriptions and merge/close detection read every host's terms from core,
so nothing in the runtime holds one host's words. Capability flags nothing
reads are dropped, and so are the fingerprint and media flags: a host
without them answers through the trait's default and its own media read.
Native stack writes have trait defaults that refuse.
@Tryanks
Tryanks marked this pull request as ready for review October 9, 2026 09:16
# Conflicts:
#	crates/protocol/src/lib.rs
@Tryanks
Tryanks enabled auto-merge (squash) October 9, 2026 09:19
# Conflicts:
#	crates/protocol/src/lib.rs
@Tryanks
Tryanks merged commit 0dc44b9 into main Oct 9, 2026
7 checks passed
@Tryanks
Tryanks deleted the feat/host-trait branch October 9, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant