Skip to content

Keep PR context readable while the app is unfocused - #884

Merged
alexeyzimarev merged 2 commits into
mainfrom
pr-panel-unfocused-fixes
Sep 11, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
pr-panel-unfocused-fixes

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Closes #883 — AI-2700

What & why

The PR card and reader masked every protected field the moment another app took keyboard focus, because the window fed the PR context a foreground flag that required it to be active. Foreground is now the window being on screen, visible and not minimized, so the reader stays readable beside another window and keeps its access lease renewed. The pane's single refresh reloads the work item and the pull requests together; the card has no refresh of its own. The open button beside the work item key opens the item's page in the web UI on the profile server, since a Linear-keyed issue link arrives without a tracker URL. The external-link icon beside a label sits on the glyphs' visual centre, 1px above the line-box centre.

Where to look

MainWindow.UpdateActivityVisibility is the one predicate; hiding to the tray and minimizing still mask. The new smoke test deactivates the headless window through the platform interface map, because that hook is internal to Avalonia.

Verification

  • dotnet run --project test/Capacitor.App.Tests.Unit: 1684 passed, 0 failed.
  • Against the focus-keyed predicate the new window test fails at pullRequests.CanReveal right after focus loss.
  • dotnet build src/Capacitor.App --no-incremental: 0 warnings.
  • The icon offset was measured from the report's screenshot; the sandbox cannot launch the app, so it wants an eyeball check.

🤖 Generated with Claude Code

The access lease must keep renewing while another app holds focus, so
foreground means the window is on screen: visible and not minimized.
The work-item card opens the item's page on the profile server because
a Linear-keyed issue link arrives without a tracker URL.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-11T09:23:52.889181Z 52172ac PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Keep PR context readable when the app loses focus

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

Grey Divider

AI Description

• Keeps protected PR context readable and leased while visible but unfocused.
• Unifies pane refreshes and opens served work items on the profile server.
• Adds regression coverage and aligns external-link icons beside labels.
Diagram

graph TD
  WS["Window State"] --> VP["Visibility Predicate"] --> PR["PR Context"] --> AL["Readable Lease"]
  WP["Work Pane"] --> RF["Unified Refresh"] --> PR
  WP --> OW["Open Work Item"] --> PS[("Profile Server")]
Loading
High-Level Assessment

The chosen approach is appropriate: a single window-level visibility predicate keeps masking and lease behavior consistent, while the work pane owns the combined refresh and delegates URL construction to the existing action service. Retaining focus-based foreground detection would reproduce the bug, and opening tracker URLs would not work reliably for Linear-keyed items lacking tracker links.

Files changed (16) +222 / -44

Enhancement (9) +57 / -27
AgentActionService.csAdd profile-server work-item URL handling +11/-5

Add profile-server work-item URL handling

• Generalizes browser opening around a reusable path-based helper. Adds an escaped work-item route that always targets the signed-in profile server and preserves existing notification behavior.

src/Capacitor.App/Services/AgentActionService.cs

PullRequestContextViewModel.csExpose coordinated pull-request refresh +3/-1

Expose coordinated pull-request refresh

• Routes the command through a public Refresh method so the work-context pane can trigger the same manual rediscovery and reload behavior.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.cs

WorkContextViewModel.Projections.csTrack whether the served work item can open +12/-5

Track whether the served work item can open

• Wraps the primary work-item ID so changes update command availability. Ensures absorbed items use the server-provided ID and clearing the card disables opening.

src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs

WorkContextViewModel.csCoordinate pane refresh and work-item opening +12/-2

Coordinate pane refresh and work-item opening

• Makes the pane refresh reload linked pull requests alongside work context. Adds an availability-aware command that opens the currently served work-item ID.

src/Capacitor.App/ViewModels/WorkContextViewModel.cs

WorkspaceViewModel.csWire work-item opening into the workspace +1/-1

Wire work-item opening into the workspace

• Injects AgentActionService.OpenWorkItemInWeb into the work-context view model.

src/Capacitor.App/ViewModels/WorkspaceViewModel.cs

PullRequestCard.axamlRemove redundant PR-card refresh action +8/-10

Remove redundant PR-card refresh action

• Removes the card-level refresh button in favor of the pane's unified refresh. Simplifies action visibility and applies label-aware alignment to the provider's external-link icon.

src/Capacitor.App/Views/PullRequestCard.axaml

PullRequestReader.axamlAlign the reader external-link icon +4/-1

Align the reader external-link icon

• Centers the provider label and applies the corrected visual offset to its adjacent external-link glyph.

src/Capacitor.App/Views/PullRequestReader.axaml

PullRequestStyles.axamlAdd label-adjacent icon alignment style +4/-0

Add label-adjacent icon alignment style

• Introduces a one-pixel upward transform for external-link icons displayed beside text labels.

src/Capacitor.App/Views/PullRequestStyles.axaml

WorkContextView.axamlOpen canonical work-item web pages +2/-2

Open canonical work-item web pages

• Replaces the conditional tracker issue action with an always-present command for opening the served work item in the application web UI.

src/Capacitor.App/Views/WorkContextView.axaml

Bug fix (1) +6 / -5
MainWindow.axaml.csBase PR foreground state on screen visibility +6/-5

Base PR foreground state on screen visibility

• Stops treating keyboard focus changes as PR foreground transitions. Pull-request context remains available while the visible, non-minimized window is unfocused, but still masks when minimized or hidden.

src/Capacitor.App/Views/MainWindow.axaml.cs

Refactor (1) +1 / -2
PullRequestContextViewModel.Presentation.csRemove obsolete card refresh tooltip projection +1/-2

Remove obsolete card refresh tooltip projection

• Removes the pull-request-specific refresh tooltip after refresh ownership moves to the containing work-context pane.

src/Capacitor.App/ViewModels/PullRequestContextViewModel.Presentation.cs

Tests (4) +155 / -9
AgentActionServiceTests.csTest profile-server work-item links +28/-0

Test profile-server work-item links

• Verifies work-item IDs are escaped and opened against the profile server rather than the daemon snapshot server. Also covers the signed-out notification path.

test/Capacitor.App.Tests.Unit/AgentActionServiceTests.cs

MainWindowSmokeTests.csTest PR readability across focus loss +64/-0

Test PR readability across focus loss

• Adds a headless window regression test that deactivates the platform window through Avalonia's interface map. Confirms focus loss preserves PR access while minimizing or hiding masks it.

test/Capacitor.App.Tests.Unit/MainWindowSmokeTests.cs

WorkContextViewModelTests.csTest unified refresh and work-item opening +62/-8

Test unified refresh and work-item opening

• Covers coordinated work-context and pull-request refreshes, served-ID selection, command enablement, and the revised sidebar open action. Updates existing view assertions for the new behavior.

test/Capacitor.App.Tests.Unit/WorkContextViewModelTests.cs

WorkContextViewSmokeTests.csUpdate smoke tests for the work-item action +1/-1

Update smoke tests for the work-item action

• Updates work-context rendering assertions to expect the new profile web action regardless of inline issue presentation.

test/Capacitor.App.Tests.Unit/WorkContextViewSmokeTests.cs

Documentation (1) +3 / -1
CHANGES.mdDocument screen-based PR context protection +3/-1

Document screen-based PR context protection

• Clarifies that protected content is masked only when the workspace is hidden or minimized. Documents that losing keyboard focus preserves readability and lease renewal.

docs/CHANGES.md

@qodo-code-review

qodo-code-review Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Invalid work items open wrong pages ✓ Resolved 🐞 Bug ≡ Correctness
Description
PrimaryId enables OpenWorkItemCommand for every non-null assignment ID without applying the
work-item ID validation used by the HTTP client. When an assignment contains an empty,
whitespace-only, . or .. ID, the card exposes the action and passes that value into a normalized
browser path such as /work-items/ or /work-items/...
Code

src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs[R24-26]

+        set {
+            _primaryId = value;
+            _canOpenWorkItem.OnNext(value is not null && _openWorkItem is not null);
Relevance

●●● Strong

Clear correctness bug: browser actions must reject invalid IDs like the HTTP client; similar
defensive fixes are accepted.

PR-#779
PR-#790

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new setter enables opening solely from non-nullness, and ApplyReady can assign the raw ID from
the assignment response when no item was served. The repository's HTTP client explicitly runs those
same IDs through ValidWorkItemId, whose implementation trims and rejects empty values and dot
segments, but the new browser action escapes and concatenates the unvalidated value directly.

src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs[22-27]
src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs[198-220]
src/Capacitor.Cli.Core/WorkItems/WorkContextClient.cs[18-26]
src/Capacitor.Cli.Core/WorkItems/WorkContextIds.cs[3-12]
src/Capacitor.App/Services/AgentActionService.cs[145-159]

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 new work-item browser command treats every non-null assignment ID as openable, although the existing API client rejects empty, whitespace-only, `.` and `..` IDs. Such values enable the button and can produce an invalid or normalized browser route.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs[22-27]
- src/Capacitor.App/Services/AgentActionService.cs[145-159]

## Recommended Fix
Canonicalize IDs with `WorkContextIds.ValidWorkItemId` before storing them in `PrimaryId` or enabling the command, and defensively apply the same validation in `OpenWorkItemInWeb`. Disable the command or notify and return when validation fails, and add coverage for empty, whitespace-only, `.` and `..` IDs.

ⓘ 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: 01e241aa)
Review mode: ⚖️ Balanced: This is a behavior-changing, cross-cutting UI and ViewModel update involving protected-content visibility, lease renewal, refresh coordination, URL construction, and multiple platform-sensitive paths, but not clearly dense enough to warrant redundant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.App/ViewModels/WorkContextViewModel.Projections.cs Outdated
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.

Desktop PR panel: content masked when the app is unfocused, plus card refresh, icon and work-item button fixes

1 participant