Skip to content

Link a PR opened from another checkout to its session - #925

Open
realtonyyoung wants to merge 2 commits into
mainfrom
tonyyoung/ai-2738-link-cross-repo-pr
Open

Link a PR opened from another checkout to its session#925
realtonyyoung wants to merge 2 commits into
mainfrom
tonyyoung/ai-2738-link-cross-repo-pr

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Closes #923 — AI-2738

What & why

A session's PR list is fed by one probe, gh pr view in the launch cwd, so a PR the agent opened from a worktree it made itself — in another repository or the same one — never appears. The Claude session watcher now collects the roots of every checkout the agent mutated outside its launch cwd, probes each on the 60s refresh and once more on the final drain, and posts any GitHub PR it finds to POST /api/sessions/{id}/pull-requests, which already dedupes and never repoints the primary repository.

Where to look

Mutation paths only feed the collector: a checkout the agent merely read must not have whatever PR its branch carries attached. Non-GitHub hosts are skipped because the endpoint rebuilds the remote URL on github.com from owner and repo.

Verification

dotnet run --project test/Capacitor.Cli.Core.Tests.Unit -- --treenode-filter '/*/*/SecondaryRepoRootsTests/*'   → total: 8, failed: 0
dotnet run --project test/Capacitor.Cli.Tests.Unit -- --treenode-filter '/*/*/WatchSecondaryPullRequestTests/*' → total: 6, failed: 0
dotnet publish src/Capacitor.Cli/Capacitor.Cli.csproj -c Release → 0 warnings, no IL2026/IL3050

🤖 Generated with Claude Code

Only checkouts the agent mutated are probed: one it merely read must not have the PR its branch happens to carry attached. Only GitHub PRs are posted, because the server endpoint rebuilds the remote URL on github.com from owner and repo.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 13, 2026

Copy link
Copy Markdown

AI-2738

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Link pull requests from secondary Claude checkouts

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Track secondary checkouts only when Claude mutates files outside the launch repository.
• Probe tracked roots periodically and at shutdown, linking deduplicated GitHub pull requests.
• Cover collection, filtering, retries, and deduplication with focused unit tests.
Diagram

sequenceDiagram
    actor Agent as Claude Agent
    participant Watcher as Session Watcher
    participant Roots as Root Collector
    participant Detect as Repo Detector
    participant API as Session API
    participant State as Link State
    Agent->>Watcher: Mutates checkout
    Watcher->>Roots: Record mutation path
    loop Every 60s and final drain
        Watcher->>Roots: Read tracked roots
        Watcher->>Detect: Probe each checkout
        Detect-->>Watcher: PR metadata
        alt GitHub and unlinked
            Watcher->>API: POST pull request
            API-->>Watcher: Accepted
            Watcher->>State: Cache PR key
        else Unsupported or linked
            Watcher-->>Watcher: Skip candidate
        end
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Probe immediately after each mutation
  • ➕ Could discover newly opened pull requests sooner than the periodic refresh.
  • ➖ Adds repeated repository and GitHub CLI work during active editing.
  • ➖ Most mutations occur before a pull request exists, creating unnecessary probes.
2. Discover links server-side
  • ➕ Centralizes pull-request association and avoids client-side polling.
  • ➖ The server lacks reliable checkout and branch context from local worktrees.
  • ➖ Would require broader repository credentials or additional client telemetry.

Recommendation: Keep the bounded mutation-derived root collector with periodic and final-drain probing. It preserves the crucial read-versus-mutation distinction, limits probing overhead, catches late-session PR creation, and reuses the existing deduplicating session endpoint without changing primary repository ownership.

Files changed (6) +331 / -0

Enhancement (1) +7 / -0
Models.csStore secondary roots and linked PR state +7/-0

Store secondary roots and linked PR state

• Extends watcher state with the secondary checkout collector, successful PR-link keys, and the last secondary probe timestamp.

src/Capacitor.Cli.Core/Models.cs

Bug fix (2) +109 / -0
SecondaryRepoRoots.csCollect mutated secondary checkout roots +34/-0

Collect mutated secondary checkout roots

• Adds a bounded collector that extracts Claude mutation paths, resolves their Git roots, and excludes the primary checkout. Duplicate roots, read-only evidence, unsupported vendors, malformed lines, and unresolved paths are safely ignored.

src/Capacitor.Cli.Core/RepoEvidence/SecondaryRepoRoots.cs

WatchCommand.csProbe and link pull requests from secondary checkouts +75/-0

Probe and link pull requests from secondary checkouts

• Feeds transcript mutations into the collector and probes tracked roots every 60 seconds plus once during final drain. Posts GitHub PR metadata to the session endpoint, caches successful links, and retries failed posts on later passes.

src/Capacitor.Cli/Commands/WatchCommand.cs

Tests (2) +203 / -0
SecondaryRepoRootsTests.csTest secondary checkout root collection +96/-0

Test secondary checkout root collection

• Covers mutation filtering, primary-root exclusion, vendor filtering, capacity limits, malformed input, unresolved paths, deduplication, and sessions without a primary root.

test/Capacitor.Cli.Core.Tests.Unit/RepoEvidence/SecondaryRepoRootsTests.cs

WatchSecondaryPullRequestTests.csTest secondary pull-request linking +107/-0

Test secondary pull-request linking

• Verifies successful-post deduplication, failed-post retries, repeated no-PR probes, non-GitHub filtering, changed PR numbers, and no-op behavior without a collector.

test/Capacitor.Cli.Tests.Unit/WatchSecondaryPullRequestTests.cs

Documentation (1) +12 / -0
CHANGES.mdDocument secondary-checkout PR linking behavior +12/-0

Document secondary-checkout PR linking behavior

• Explains why launch-directory probing misses worktree PRs and records the mutation-only, GitHub-only linking constraints.

docs/CHANGES.md

@qodo-code-review

qodo-code-review Bot commented Sep 13, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Resumed sessions miss earlier PRs ✓ Resolved 🐞 Bug ≡ Correctness
Description
SecondaryRoots is initialized without scanning the transcript history, while DrainNewLines only
feeds it lines after the server's acknowledged frontier. When a watcher restarts after a
secondary-checkout mutation was already acknowledged, neither periodic nor final probing ever sees
that checkout.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R509-511]

+        if (vendor == "claude" && agentId is null) {
+            state.SecondaryRoots = new SecondaryRepoRoots(GitRepository.FindRoot, cwd is null ? null : GitRepository.FindRoot(cwd));
+        }
Relevance

●●● Strong

Restart reconstruction gaps were accepted previously for watcher state after acknowledged transcript
frontiers.

PR-#526
PR-#648

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The only startup prefix scan feeds EvidenceScanner, and normal collection later receives only
newLines. Since WatcherConnect seeds LinesProcessed from the server, acknowledged mutation
lines are skipped after a watcher restart and cannot populate the roots used by either probe.

src/Capacitor.Cli/Commands/WatchCommand.cs[517-529]
src/Capacitor.Cli/Commands/WatchCommand.cs[643-645]
src/Capacitor.Cli/Commands/WatchCommand.cs[2191-2193]
src/Capacitor.Cli.Core/RepoEvidence/SecondaryRepoRoots.cs[14-29]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A resumed Claude watcher collects secondary roots only from newly drained lines. Mutations before the server's acknowledged frontier are therefore permanently absent from the secondary PR probes.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[509-529]
- src/Capacitor.Cli/Commands/WatchCommand.cs[643-645]
- src/Capacitor.Cli/Commands/WatchCommand.cs[2191-2193]

## Recommended Fix
During Claude session-watcher initialization, feed the existing transcript lines into `SecondaryRoots.OnLine` independently of repository-evidence promotion. Scan all available historical lines up to the collector's capacity so watcher restarts preserve secondary-checkout discovery before applying the server resume frontier.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Secondary probes can stall shutdown ✓ Resolved 🐞 Bug ☼ Reliability
Description
The periodic detector has no watcher cancellation token, and the final detector and HTTP post run
sequentially across every root with CancellationToken.None. A slow repository or unreachable API
can exceed the five-second graceful-exit allowance, causing the watcher to be force-killed before
drain completion signaling and child teardown.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R899-902]

+            // A PR is usually opened in the session's last turn, after the previous 60s probe.
+            await LinkSecondaryPullRequestsAsync(state,
+                root => RepositoryDetection.DetectRepositoryAsync(config, root),
+                pr => PostLinkedPullRequestAsync(sessionId, pr, CancellationToken.None));
Relevance

●●● Strong

Recent precedents favor cancellation and deadline-aware shutdown work to prevent slow network
operations blocking teardown.

PR-#173
PR-#247
PR-#551

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Repository detection uses independent per-process timeouts and roots are probed sequentially, while
HTTP posting defaults to a 30-second retry budget. The final call supplies no cancellation, yet
watcher management force-kills the process after only five seconds and drain-complete and child
shutdown happen after this probe.

src/Capacitor.Cli/Commands/WatchCommand.cs[747-751]
src/Capacitor.Cli/Commands/WatchCommand.cs[899-925]
src/Capacitor.Cli/Commands/WatchCommand.cs[3350-3377]
src/Capacitor.Cli/RepositoryDetection.cs[126-137]
src/Capacitor.Cli/RepositoryDetection.cs[297-325]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[90-97]
src/Capacitor.Cli/WatcherManager.cs[281-297]
PR-#187

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Secondary repository detection is not cancellation-aware, and the final probe uses no cancellation or aggregate shutdown budget. Sequential detection and posting can therefore keep the watcher alive beyond its graceful shutdown allowance.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[747-751]
- src/Capacitor.Cli/Commands/WatchCommand.cs[899-902]
- src/Capacitor.Cli/Commands/WatchCommand.cs[3343-3360]
- src/Capacitor.Cli/RepositoryDetection.cs[126-137]

## Recommended Fix
Give `LinkSecondaryPullRequestsAsync` an aggregate cancellation token and time budget, propagate the remaining budget into each repository detection and HTTP post, and stop iterating when either expires. Use a short bounded shutdown token for the final probe so the existing five-second graceful-exit path can still complete.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Final server refusals drop the PR ✓ Resolved 🐞 Bug ☼ Reliability
Description
PostLinkedPullRequestAsync leaves retryStatuses at its false default, so retryable responses
such as throttling or temporary unavailability immediately return failure. When this occurs during
the final probe for a PR opened in the last turn, the watcher exits without another pass and never
links it.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R3375-3377]

+            using var content = new StringContent(payload.ToJsonString(), Encoding.UTF8, "application/json");
+            using var resp    = await client.PostWithRetryAsync(
+                $"{Url}/api/sessions/{Uri.EscapeDataString(sessionId)}/pull-requests", content, ct: ct);
Relevance

●●● Strong

The team accepts reliability fixes ensuring retryable server failures remain retryable, especially
for final delivery attempts.

PR-#190
PR-#247
PR-#526

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The extension method defaults retryStatuses to false and directly returns HTTP responses unless
that flag is enabled. The new final probe is explicitly described as the last chance after a late
PR, so a retryable non-success response leaves no subsequent pass.

src/Capacitor.Cli/Commands/WatchCommand.cs[899-902]
src/Capacitor.Cli/Commands/WatchCommand.cs[3375-3387]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[86-97]
src/Capacitor.Cli.Core/HttpClientExtensions.cs[270-280]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The secondary PR POST retries transport failures but not retryable HTTP status responses. A refusal during the final probe has no later watcher pass to recover it.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[3375-3387]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[86-97]
- src/Capacitor.Cli.Core/HttpClientExtensions.cs[270-280]

## Recommended Fix
Enable retryable-status handling for this idempotent, server-deduplicated POST while supplying the bounded aggregate probe timeout. Preserve immediate handling for permanent client errors and return success only after an accepted response.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Shell-edited checkouts lose their PRs ✗ Dismissed 🐞 Bug ≡ Correctness
Description
SecondaryRepoRoots.OnLine depends exclusively on ExtractClaudePaths, whose mutation
classification excludes Claude's Bash tool and its command field. When the agent modifies files or
creates a worktree through shell commands without a later direct edit tool, that checkout is never
collected or probed.
Code

src/Capacitor.Cli.Core/RepoEvidence/SecondaryRepoRoots.cs[R18-19]

+            foreach (var (path, kind) in RepoEvidencePaths.ExtractClaudePaths(jsonlLine)) {
+                if (kind != RepoEvidenceKind.Mutation) continue;
Relevance

●●● Strong

Accepted history fixes missing Claude transcript path variants; shell mutations are a closely
matching extraction gap.

PR-#648

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Claude's normalizer explicitly supports Bash commands, while ExtractClaudePaths recognizes only
direct edit, notebook, read, glob, and grep tools. The new collector has no other source of mutation
paths, so shell-only mutations cannot add a root.

src/Capacitor.Cli.Core/Harness/Claude/ClaudeActionNormalizer.cs[23-36]
src/Capacitor.Cli.Core/RepoEvidence/RepoEvidenceScanner.cs[107-125]
src/Capacitor.Cli.Core/RepoEvidence/SecondaryRepoRoots.cs[14-29]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Secondary checkout discovery only consumes paths from direct file-edit tools, although Claude can perform checkout mutations through Bash. Shell-only workflows consequently leave no root for PR probing.

## Fix Focus Areas
- src/Capacitor.Cli.Core/RepoEvidence/SecondaryRepoRoots.cs[14-29]
- src/Capacitor.Cli.Core/RepoEvidence/RepoEvidenceScanner.cs[107-125]
- src/Capacitor.Cli.Core/Harness/Claude/ClaudeActionNormalizer.cs[23-36]

## Recommended Fix
Extend the collector with conservative Bash mutation handling using the existing shell-command analysis infrastructure. Record only absolute checkout paths from command segments classified as mutating, retaining the current fail-open behavior and avoiding attachment for read-only shell commands.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Failed links are never retried ✗ Dismissed 🔗 Cross-repo conflict ☼ Reliability
Description
PostLinkedPullRequestAsync treats every successful HTTP status as proof that kcap-server durably
recorded the pull request. When the server’s repository append fails, its handler still returns 202,
so the tuple enters LinkedPullRequests and all later probes skip it.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[3382]

+            return resp.IsSuccessStatusCode;
Relevance

●●● Strong

Accepted precedents require server acknowledgements to reflect durable lifecycle state before
suppressing retries.

PR-#262
PR-#817

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CLI converts any 2xx response into a successful post and then stores the pull-request key,
preventing another post. The server recorder can observe a failed append and return without
recording anything, while the endpoint ignores that outcome and unconditionally returns 202
Accepted.

src/Capacitor.Cli/Commands/WatchCommand.cs[3357-3382]
External repo: kurrent-io/kcap-server, src/Capacitor.Server/Hooks/SessionHookHandlers.cs [1520-1535]
External repo: kurrent-io/kcap-server, src/Capacitor.Api.Public/Sessions/RepositoryDetectionRecorder.cs [71-86]
External repo: kurrent-io/kcap-server, src/Capacitor.Api.Public/Sessions/SessionWriter.Writes.cs [1777-1822]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The CLI interprets the server's 202 response as a durable pull-request link, but the server returns 202 even when its repository event append fails. This causes the CLI to suppress every later retry for that pull request.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[3357-3382]
- /cross_repos/kcap-server/src/Capacitor.Server/Hooks/SessionHookHandlers.cs[1520-1535]
- /cross_repos/kcap-server/src/Capacitor.Api.Public/Sessions/RepositoryDetectionRecorder.cs[71-86]

## Recommended Fix
Make the server recorder return a durable-append outcome and have `HandleLinkPullRequest` return a non-success status when persistence fails. Keep the CLI adding the tuple only after a success response, so transient failures remain eligible for its next probe.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: fb9966e3)
Review mode: ⚖️ Balanced: This introduces cross-cutting watcher state, transcript parsing, repository detection, periodic/final probing, and API posting behavior with meaningful integration and retry risks, but is not broad or defect-dense enough to warrant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs
Comment thread src/Capacitor.Cli.Core/RepoEvidence/SecondaryRepoRoots.cs
Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs Outdated
Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs Outdated
Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs
The final pass must finish inside the 5s the watcher is given before it is killed, with the drain-complete signal still to send. A watcher resumed past the server frontier never drains the lines that named an earlier checkout, so the collector replays the transcript at start.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

Link a session's PR when it is opened in a different repository than the session

1 participant