Skip to content

feat: link and synchronize thread pull requests (A2) - #652

Merged
Tryanks merged 13 commits into
mainfrom
feat/linked-pull-requests
Oct 8, 2026
Merged

Tryanks merged 13 commits into
mainfrom
feat/linked-pull-requests

Conversation

@Tryanks

@Tryanks Tryanks commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

SessionMeta.pull_requests            (the only copy on the wire; children's metas reach clients through the index)
  ├─ host summary sweep  → shared snapshots, native stack siblings (unsettled threads only)
  ├─ branch discovery    → created links; never restores a tombstone
  ├─ /pull-requests MCP  → link / unlink / list, bound to the caller's token
  └─ client              → sidebar badge, header badge, "Pull requests 3" tab (last, after Tasks), phone chip + sheet

unlink (any link)  → { key, source: "dismissed" }   discovery / stack expansion: no-op
explicit link      → fresh link, unwatched          tombstones shown only as rows of a stack topology

launch: caps().mcp_servers && registration  ──► tcode_pull_requests registered
dispatch: same registration present         ──► LINKING_INSTRUCTIONS + turn   (a native `/command` turn: no block)
                                                recorded as visible context, labelled by content

Refs #532 and #535. Deferred: the PR rule in automatic settlement (every remaining link terminal, latest terminal link at or after the last human message, auto_settle_on_merge, immediate re-evaluation after a confirmed merge) and the merge-settle settings row land in a small follow-up after A2 and B1 (#526) merge. This PR does not change settlement, auto-archive, sidebar sections or lifecycle fields.

Review corrections in this revision:

  • No McpServersRegistered event, no pull_request_tools dispatch barrier, no provider-kind match in runtime, no Cursor/ACP production change: registration and the instruction prefix use preview's gate (MCP capability plus a registration present). The registration is revoked explicitly at preview's revoke sites.
  • Every unlink leaves a key-only dismissed tombstone (stack_dismissed renamed). The in-memory discovered map is removed.
  • Typed ChecksState, ReviewDecision, Mergeability in core; the service maps GitHub's values to them; the UI matches the enums.
  • Removed: Command::RefreshPullRequests, SessionStatus.pull_requests, current_pull_request, ThreadPullRequestLink::position(), watch (Source control: pull request watch that wakes the agent (A3) #639 defines it), the interactive parameters, the 50 / 50 ms head path and the unused HeadRequest fields.
  • Discovery triggers during a pass re-run only their threads, each with the refresh its triggers asked for. A gh pr merge|close / glab mr merge|close command (word-boundary match on the raw text, as upstream) refreshes only OPEN links. Each group's pause is checked once, one flag schedules the 10 ms requested sweep, and runtime no longer repeats the service's github.com stack check.
  • The turn disclosure is labelled "Pull request instructions", "Orchestrate Skill", or both.
  • UI: no Preview tab in the diff panel's strip (the shell mounts Preview); open_tab_for(session, tab) replaces the copied opener; compact mode follows window_state.compact; phone line 2 reads source · author · head → base · Layer · diff, with the author truncating first; the phone entry is a chip in a row above the composer; the sheet has its natural height; the empty state has an in-body "Link pull request…" button; a disabled host shows the missing-credential notice with the Source Control link; the stack caption carries stack_tooltip.
  • The link dialog names the repository when it is known. GitStatus.github_repository (resolved as gh does) carries it; without it the dialog shows the URL-only description.
  • Topology-only rows open and copy layer URLs synthesized from the key (Repository::url(number)), not URLs the service supplies.

Evidence

  • Before: the corrections above were open review findings on d25bc7f9.
    After: checks on the final commit eac2caf7:
cargo fmt --all --check                                         [no output; exit 0]
cargo clippy --workspace --all-targets --locked -- -D warnings  Finished (no warnings)
cargo nextest run --workspace --locked                          Summary [57.863s] 1018 tests run: 1018 passed, 13 skipped
cargo machete                                                   didn't find any unused dependencies

Screenshots: not retaken. In this session screencapture cannot capture the display (could not create image from display, even full-screen), and the accessibility observer cannot attach to the window. The images below are from d25bc7f9, before these corrections. They still show the Preview tab in the strip, the old tab count, and the old pill and sheet. Desktop and phone visuals of this revision have not been inspected.

Desktop before
Desktop at d25bc7f9 (stale)
Phone before
Phone at d25bc7f9 (stale)

Wire once: index_snapshot lists every non-archived meta in AppState.sessions, and create_child_session upserts the child there, so dispatched children's pull_requests reach clients through the index. SessionStatus does not duplicate them.

Tests (CONTRIBUTING step that settled each):

  • rate_limit_keeps_requests_due_until_host_pause_expires. Step 1: it waited a real 1 s Retry-After on a wall-clock timer. It now gets a 3600 s Retry-After and asserts a retry_at an hour out. It then moves the runtime's recorded group deadline into the past and gives the runtime a client whose transport pause has elapsed. The transport's pause ledger is private and has no clock, and no production seam was added. Nothing depends on elapsed time.
  • shared_sync_changes_only_observations_and_honors_terminal_cadence. Step 2: the settled-exclusion assertion only checked an empty list after unlink. It now has a real case: a settled thread with an unsynced link nobody shares is never read. The closed read now arrives through the real path (an agent gh pr close command, then turn completion) instead of the removed refresh command. The explicit unlink step moved to the tombstone tests.
  • native_stack_sync_preserves_dismissals_and_explicit_restore (was …_has_no_watch). Step 2: the watch assertions went with the field. The tombstone literal is now "source":"dismissed". The unsettle case now expands on the next read of an open shared layer (a real path) instead of the removed refresh command.
  • mcp_child_linking_is_bound_to_its_token_and_rejects_a_thread_override. Step 2: the acknowledgement-event part went with the machinery. After unlink, the final assertion is that the parent has no links and the child has no visible link (a tombstone remains).
  • skill_mentions_use_the_providers_native_invocation: restored to main's version.
  • linked_pr_summaries_coalesce_and_retry_only_missing_graphql_aliases. Step 2: the contains("stackEntry") query-text assertion is dropped. Typed signal values are asserted.
  • head_queries_filter_fork_owner_and_keep_branch_names_in_variables. Step 3, moved to the owning boundary: it now drives PullRequests::branch (the production caller), because by_head and HeadRequest are private.
  • New: registered_tools_prefix_each_turn_except_a_native_command (plain / /compact / converted $review through dispatch_next_queued, with the recorded disclosure split). New: an_unlinked_discovered_pull_request_stays_unlinked_across_new_refs_and_restarts. It fails when unlink drops the link instead of tombstoning it: the PR is relinked after a new remote ref. New: merge_or_close_detection_matches_words_in_the_raw_command.
  • Mechanical: eight dispatch_next_pending() calls in tests.rs pass false for the new gate argument. parse_status's literal fixture gains github_repository: None.
  • Test declarations (#[test]/#[gpui::test]): main 1016 → d25bc7f9 1025 → eac2caf7 1028. No test deleted in this revision.

Rate-limit wording: the spec's "GitHub rate limit reached. Status resumes %{ago}." is restored with humanize_ago. Because humanize_ago renders past times ("5m ago"), the notice reads "Status resumes 10m ago" for a future retry_at. This needs a wording or helper decision.

Platform limits: macOS-only local run. Windows/Linux and iOS/Android/Web builds are left to CI. The 13 ignored tests were not run. No live GitHub request was made in this revision.

Merge Danger

Door: two-way

Blast Radius: threads

Adds persisted PR metadata (SessionMeta.pull_requests, empty by default for old metas), GitStatus.github_repository, and unreleased wire commands (LinkPullRequest, UnlinkPullRequest, PullRequestLinked) under protocol version 10. Background sync and discovery workers run on the host. Rollback stops them and leaves the stored links inert. Automatic settlement stays with B1 and the deferred follow-up.

Tryanks added 11 commits October 8, 2026 18:54
- Every unlink leaves a key-only dismissed tombstone; discovery and stack
  expansion never restore it, an explicit link does. The in-memory
  discovered map is gone.
- PR tools register under preview's gate (MCP capability plus a
  registration) and turns carry the instructions by the same check; the
  McpServersRegistered acknowledgement and its barrier are removed. A
  native /command turn carries no block. Registrations are revoked at
  preview's sites.
- Discovery triggers during a pass re-run only their threads with their
  refresh flag; a merge/close command refreshes only open links and is
  matched on word boundaries (gh pr, glab mr).
- One pause check per group, one scheduled flag for the 10 ms requested
  sweep, no duplicated host check.
- Typed checks, review decision and mergeability; no watch field, no
  current_pull_request or position(); no RefreshPullRequests command; link
  state travels only in SessionMeta; GitStatus names the GitHub
  repository a bare number refers to.
- Head lookups drop the interactive path and unused request fields.
- Read links from the session index; label the turn disclosure by its
  content; Pull requests is the last tab with a muted count; no Preview
  tab in the diff panel; one open_tab_for.
- Typed signals, folded credential notices, spec rate-limit wording,
  stack tooltip, unused keys removed.
- Compact mode follows the window state; phone chip and natural-height
  sheet; line 2 order with readable source and branches; in-body empty
  state button; the dialog names the repository when known.
@Tryanks
Tryanks marked this pull request as ready for review October 8, 2026 13:13
…ests

# Conflicts:
#	crates/protocol/src/lib.rs
#	crates/runtime/src/app/lifecycle.rs
#	crates/ui/src/shell.rs
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