Repository navigation
Improve desktop rail feedback for selection, launches and pull requests - #950
Merged
Merged
Conversation
PR Summary by QodoImprove rail feedback for launches, selection, and pull requests
AI Description
Diagram
High-Level Assessment
Files changed (46)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+66
to
+81
| async Task ReadAsync(string session) { | ||
| try { | ||
| var ct = _cancel.Token; | ||
| var capability = await _source.DiscoverAsync(false, ct).ConfigureAwait(false); | ||
| if (capability.Kind != PullRequestCapabilityKind.Supported) return; | ||
| var links = await _source.ListAsync(session, ct).ConfigureAwait(false); | ||
| if (links.Kind != PullRequestReadKind.Ready || links.Data is null) { | ||
| if (links.Kind is PullRequestReadKind.SubjectUnavailable or PullRequestReadKind.SignedOut || links.AccessFailure is "invalid" or "denied") Set(session, PullRequestTone.None); | ||
| return; | ||
| } | ||
| var tones = new List<PullRequestTone>(); | ||
| var denied = false; | ||
| foreach (var link in links.Data.Items) { | ||
| var read = await _source.OverviewAsync(session, PullRequestWire.Subject(link), ct).ConfigureAwait(false); | ||
| if (read.Kind is PullRequestReadKind.Ready or PullRequestReadKind.Stale && read.Data is not null && read.AccessFailure is null) | ||
| tones.Add(PullRequestTones.From(read.Data)); |
There was a problem hiding this comment.
9. Local pull requests never colour rail 🐞 Bug ≡ Correctness
PullRequestToneCache invokes the shared source without supplying the reader registry's per-session repository and branch context. When server-linked PR reads are unavailable, the registry can only use its local provider fallback after DescribeSession has populated that context, so those locally discoverable pull requests remain absent from the rail.
Agent Prompt
## Issue description
The new background PR reader bypasses the session-description step required for local provider discovery and fallback.
## Fix Focus Areas
- src/Capacitor.App/Services/PullRequestToneCache.cs[33-45]
- src/Capacitor.App/Services/PullRequestToneCache.cs[66-81]
- src/Capacitor.App/ViewModels/PullRequestContextViewModel.Reads.cs[25-33]
- src/Capacitor.Cli.Core/PullRequests/Readers/PullRequestReaderRegistry.cs[30-64]
## Recommended Fix
Pass the reader-context interface and enough per-row repository and branch metadata into the cache, then call `DescribeSession` before discovery/listing. Preserve that metadata on the directory row or expose it through a suitable directory/context API so local provider discovery receives the same inputs as the workspace reader.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Avalonia's TextBox and SelectableTextBlock select everything on the third click in their own class handler, so the replacement runs on the tunnel route and marks the press handled before that handler sees it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
Hover already paints the raised surface brush, so a selection that used the same brush was indistinguishable from the pointer resting on a row. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
The agent instance only exists once the runtime handshake ends, so the daemon now reports in-flight launches and their stage over the local status snapshot, and the app adds its own placeholder the moment the server accepts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
The rail reads PR state through a cache over the same per-session links and overview reads the workspace uses; conflicts stay uncoloured until the server sends a mergeable field, which the client already accepts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
A placeholder expired only when some other change recomputed the rows, and a same-id row on the other lane could settle a launch it never belonged to. The tone cache now honours the reader's reveal gate, clears on a denial or a signed-out discovery, and skips local rows while the daemon reports another server. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
realtonyyoung
force-pushed
the
desktop-rail-feedback
branch
from
September 15, 2026 13:59
73e9baa to
4f4fa60
Compare
Collaborator
Author
|
Qodo findings, addressed in the latest push:
|
Thanks for the update. Based on your summary, the latest push addresses findings 1–7, 9, and 11 in the review index, including the lane-aware launch tracking, placeholder expiry, PR access/denial handling, and rail filtering. The remaining items are:
The rationale for retaining tones between refreshes is also clear: |
…tone The server accepts a launch as a dashed Guid while the daemon publishes the N form, and only a local row may retire a local launch's stand-in. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui
The expiry timer's callback runs on a pool thread, where an unhandled ObjectDisposedException from the disposed timer would take the process down. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A daemon that dies mid-handshake never sends the snapshot that would retire its pending entry, so the rail and workspace showed "Starting" for good. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Collaborator
Author
|
NO FINDINGS |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No tracker issue on either side: the four changes were requested directly.
What & why
Four desktop fixes. The open session row was indistinguishable from a hovered one, so it now carries an accent edge, its own background and a heavier title. A launch showed nothing until the daemon finished the runtime handshake; the daemon now reports in-flight launches and their stage over the local status snapshot, and the app adds a placeholder row the moment the server accepts, so the rail row and a stage line in the workspace appear at once. A triple click selected the whole text box; one application-wide tunnel handler selects the line under the pointer instead. The worktree's branch glyph and the PR card's labels share one colour vocabulary: red for closed or failing checks, green ready, muted grey draft, pulsing grey while checks run, purple merged, the warning colour for conflicts.
Where to look
Conflicts need a
mergeablefield the server does not send yet; the client reads it as null until it does. The tone cache reads every listed session's server-linked PRs every two minutes through the reader's own reveal gate, but keeps a tone past the 30-second window: it is one colour per worktree, not the protected content the window guards, and re-reading each PR every 30 s would multiply GitHub reads. The gh fallback needs the work-context repository the rail rows lack, so local-only PRs stay uncoloured. Markdown bodies keep MarkView's own selection, which takes no click count.Verification
The triple-click UI tests fail with the handler uninstalled (
SelectedTextreads the double-click word), so they pin the behaviour rather than Avalonia's default.🤖 Generated with Claude Code
https://claude.ai/code/session_01XWRNeNnUJeSbRrQQFwuSui