Skip to content

fix(mobile): satisfy native source lint

MacroscopeApp / Macroscope - UI Consistency failed Sep 1, 2026 in 13m 15s

UI Consistency: 2 issues found

Two consistency issues in the new thread.copyReference wiring, both in apps/web/src/components/CommandPalette.tsx:

  • Divergent PR resolution (lines 606-620): the palette hand-rolls gitStatus.refName === branch instead of the shared resolveDisplayedThreadPr (+ useLinkedThreadPullRequest, change-request snapshot) that ChatView and Sidebar use. For a local-checkout thread with a retained terminal PR snapshot, ChatView's shortcut copies the PR URL while the palette labels the item "Copy thread ID" and copies the ID.
  • Shortcut gated on list membership (lines 2248-2258): the handler only runs when a matching item is in displayedGroups, so mod+shift+c is inert when the query filters the item out or during browse/add-project steps; ChatView's global handler is disabled while the palette is open, so nothing happens at all.

The ChatView additions themselves reuse the existing displayed-PR resolver and follow the established writeTextToClipboard + toastManager/stackedThreadToast conventions.

Details

Note

Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.

Files reviewed against the include patterns (apps/web/src/**/*.{ts,tsx,css}): apps/web/src/components/ChatView.tsx, apps/web/src/components/CommandPalette.tsx, apps/web/src/keybindings.test.ts. Non-web files (mobile native module, shared threadReference.ts, contracts, docs) were used as context only.

Checks performed:

  • Compared the new copy-reference PR resolution in CommandPalette against resolveDisplayedThreadPr / resolveDisplayedThreadPrProvider in ThreadStatusIndicators.tsx and the existing call sites in ChatView.tsx (line 4596) and Sidebar.tsx (line 818), including snapshot retention and linked-PR live status semantics.
  • Verified environment routing: the new vcsEnvironment.status query passes activeThread.environmentId explicitly (no active-environment fallback), so the environment-scoping rule is satisfied.
  • Verified toast conventions: success via toastManager.add({ type: "success" }) and failures via stackedThreadToast, matching BranchToolbarBranchSelector, Sidebar, and useThreadActionMenu; wording ("Thread ID copied", "Failed to copy thread ID") matches existing strings.
  • Verified palette item shape (kind, searchTerms, icon with ITEM_ICON_CLASS, shortcutCommand, close-on-run via executeItem) and that LinkIcon, useProjects, useEnvironmentQuery, writeTextToClipboard are all already imported.
  • Traced keyboard handling for double-dispatch: ChatView's window handler returns early when isCommandPaletteOpen(), which is what makes the palette's list-membership gate observable as a dead shortcut.
  • Confirmed no import-order lint rule exists in the repo lint config, so the new import placement is not a finding.

No Tailwind, CSS, theme, or scroll/mask behavior was changed by this PR, so those rule groups were not applicable.


🔍 Agent activity — 28 tool calls

💭 I'm checking ChatView.tsx and CommandPalette.tsx for the in-scope changes. I notice the copy handler's success path adds a toast directly while the error path wraps it in stackedThreadToast — that inconsistency is worth flagging. The environment routing in CommandPalette looks fine, but I want to check whether the "thread.copyReference" shortcut is handled in both CommandPalette and ChatView, potentially causing double execution — though the palette does stop propagation on its own keydown handler. I also want to verify conventions around stackedThreadToast and writeTextToClipboard usage elsewhere in the repo, and note that the import placement for writeTextToClipboard seems slightly out of order but likely harmless. Let me dig into the actual code to confirm these.

1. git_tools — stackedThreadToast

2. git_tools — writeTextToClipboard

💭 I'm checking how similar copy-to-clipboard patterns work elsewhere, like the branch name copy with toast in BranchToolbarBranchSelector, and looking into how writeTextToClipboard handles failures or empty input.

3. browse_code — apps/web/src/hooks/useCopyToClipboard.ts

4. browse_code — apps/web/src/components/BranchToolbarBranchSelector.tsx

💭 The new ChatView code follows the existing established pattern exactly, which is good. Now I'm looking more closely at the CommandPalette additions, checking whether activeThread is available in OpenCommandPaletteDialog and whether the palette already computes project/cwd/git status elsewhere, to avoid duplicating existing logic. I'm also examining the keydown handling for thread.copyReference to see if there's a generic pattern to reuse, and checking the "Copy PR link" action item and its shortcut wiring. The vcs status RPC correctly uses activeThread.environmentId, so I want to pull up CommandPalette around those lines to verify everything lines up.

5. browse_code — apps/web/src/components/CommandPalette.tsx

💭 I want to check whether the palette action item's run function closes the dialog after executing, particularly for the copy action, and compare consistency with executeItem and other action items to see if they all call setOpen(false).

6. browse_code — apps/web/src/components/CommandPalette.tsx

7. browse_code — apps/web/src/components/CommandPalette.tsx

💭 I'm noticing the keydown handler duplicates the thread-jump block structure, which is a minor style issue rather than a strong finding. More significant is a potential gap in the palette's handleKeyDown: if the copy item isn't in displayedGroups due to filtering or scoping, the event isn't prevented and could fall through to the global handler. Since ChatView's global keydown listener might also fire while the palette is open, I want to check whether there's a guard checking for command palette open state, so I should look at ChatView around lines 5320-5360 where the keydown handler is set up.

8. browse_code — apps/web/src/components/ChatView.tsx

💭 Good, so the command palette check prevents double-execution there. Now I'm checking ChatView's use of activeThreadPr?.url — unlike CommandPalette, which verifies the git status branch matches the active thread before using the detected PR, ChatView doesn't seem to do that same scoping check, so I need to look at what activeThreadPr actually represents.

9. git_tools — activeThreadPr|linkedThreadPullRequest

10. browse_code — apps/web/src/components/ChatView.tsx

💭 I'm noticing that ChatView relies on the shared resolver for a thread's displayed PR, but CommandPalette instead reimplements its own separate resolution logic with a different vcs status query. That's a duplication problem—two different code paths could end up showing different PR links for the same thread, so I want to look closer at the shared resolver to understand the inconsistency.

11. git_tools — resolveDisplayedThreadPr

12. git_tools — refName ===

13. browse_code — apps/web/src/components/ThreadStatusIndicators.tsx

💭 I'm checking whether resolveDisplayedThreadPr correctly returns the live linked PR when present, versus falling back to the snapshot. I need to verify that linkedThreadPullRequest actually exposes a url field on its type, since both ChatView and CommandPalette rely on linkedPullRequest?.url for the thread shell. I'm also noticing a consistency concern: the Sidebar has an established pattern for computing the displayed PR per thread that accounts for cached snapshots, but CommandPalette seems to duplicate a weaker, standalone version of that resolution—skipping snapshot retention and adding its own vcs status query rather than reusing whatever shared hook the Sidebar uses (something like useThreadGitStatus). This could cause the palette to show "Copy thread ID" in cases where the rest of the UI shows a cached PR chip, so I want to check the Sidebar's implementation more closely.

14. browse_code — apps/web/src/components/Sidebar.tsx

15. git_tools — linkedPullRequest?: |interface ThreadLinkedPullRequest|ThreadLinkedPullRequest =

16. browse_code — packages/contracts/src/orchestration.ts

💭 I'm noting that the CommandPalette duplicates PR resolution logic instead of reusing the shared resolver and hook that ChatView and Sidebar rely on, which creates inconsistent gating conditions for when the status query runs. The bigger issue is that the palette skips terminal PR snapshot retention, so for a local thread whose checkout moved branches, the palette could offer "Copy thread ID" while ChatView's shortcut copies the PR link for the same thread — a genuine divergence between two implementations of the same command introduced in this PR. I want to check whether the linked-PR case (not just the merged/terminal case) is also affected, since both surfaces seem to pull from linkedPullRequest.url there and might actually match. There's also verbatim duplication of the cwd/project/status/detected-url block across ChatView, CommandPalette, and mobile despite the shared resolveThreadReferenceCopyTarget helper existing, plus a duplicated thread-jump block inside handleKeyDown. More interesting is a possible silent-failure edge case: if the copy item is filtered out of displayedGroups, Cmd+Shift+C does nothing and doesn't preventDefault, though this may just mirror the existing thread-jump pattern and be intentional rather than a new bug. I also notice the new clipboard import breaks alphabetical import order relative to the existing hook import, which could trip lint/CI if the repo enforces import sorting. Checking a few other spots: the copy handler's own error toast overlaps with the palette's generic error toast but isn't harmful, the palette description shows the full PR URL which matches existing patterns, and the item doesn't set keepOpen so it closes before copying like other actions. I'm now verifying whether LinkIcon is actually imported in CommandPalette, since it's used but I don't see a corresponding import added in the diff.

17. git_tools — LinkIcon

18. git_tools — useEnvironmentQuery|useProjects

💭 With all imports confirmed, I check whether a shared hook for resolving the displayed thread PR already exists—something combining the environment query with resolution logic. It only lives in ChatView and Sidebar currently, so there's no shared hook to reuse; the cleanest fix would be having the palette reuse resolveDisplayedThreadPr and useLinkedThreadPullRequest rather than rolling its own ref-based approach. Now I want to compare ChatView's gitStatusQuery gating against the palette's gating to pin down exactly where they diverge.

19. git_tools — gitStatusQuery|changeRequestSnapshotByKey

💭 I've confirmed ChatView uses the canonical thread resolution logic with snapshot and live PR status, while CommandPalette implements a weaker, divergent version. I'm identifying specific mismatches: for a local checkout on a mismatched branch with a cached terminal PR snapshot, ChatView copies the PR URL but the palette copies the thread ID instead—and there's another divergence around how linked PRs are handled. There's also a minor gating difference in when the palette queries status compared to Sidebar's behavior, and I'm noting a duplicate status query that's likely deduplicated by the shared cache anyway, so it's not a real issue. The core finding is that CommandPalette should derive PR info from the shared resolver instead of duplicating logic, since I want to keep this report focused on concrete violations rather than minor stylistic differences.

20. browse_code — Does the repo enforce import ordering/sorting in apps/web (eslint plugin import order, oxlint, biome organize imports, prettier plugin)? Show relevant lint config files and rules.

💭 No import ordering rule here, so I'll skip that check. I'm weighing whether the palette shortcut and ChatView producing different labels/values for the same command counts as a genuine regression introduced by the changed lines, keeping in mind the instruction to prefer the smallest durable fix and only flag concrete violations. I also want to verify whether the palette's activeThread from useHandleNewThread carries linkedPullRequest as part of the ThreadShell type—mobile added this in threadDetailToShell, so I need to check if the web-side shell type includes the same field in client-runtime.

21. browse_code — In packages/client-runtime (or wherever ThreadShell is defined), does the ThreadShell type include linkedPullRequest? Also what does useHandleNewThread() return as activeThread (type)? Show the type definitions.

💭 I notice mergeEnvironmentThread doesn't copy linkedPullRequest from the shell, but since ChatView reads from the same merged source, the behavior is actually consistent. I'll skip verifying the mobile threadDetailToShell change since that's out of scope here. Checking the palette's status query, the same staleness caveat applies but matches ChatView's behavior, so it's not a real discrepancy—just a minor perf concern about an extra RPC firing on palette open. Looking at the keybinding resolution, mod+shift+c resolves fine in the palette context since terminal focus isn't a factor when the palette input is active. I should check whether there's already an existing "Copy thread ID" or "Copy PR link" item in the thread action menu, since the new command might duplicate existing functionality or use inconsistent wording compared to it.

22. git_tools — Copy PR link|Copy pull request|Copy thread ID|copy-pr-link|copy thread id

💭 I notice the existing "Copy Thread ID" label uses title case in the legacy sidebar, while the new palette item says "Copy thread ID" — I should check the success toast in useThreadActionMenu to keep the wording consistent with "Thread ID copied."

23. browse_code — apps/web/src/components/Sidebar.tsx

24. browse_code — apps/web/src/hooks/useThreadActionMenu.ts

💭 Toast wording matches existing conventions, that's fine. The real issue is CommandPalette builds its own weaker thread PR resolution instead of reusing resolveDisplayedThreadPr alongside useLinkedThreadPullRequest and threadChangeRequestSnapshotsAtom, which is what ChatView and Sidebar rely on — meaning thread.copyReference could resolve to different targets depending on which surface handles it. I'm tracing a concrete scenario: a local thread checked out on a different branch than the thread's own branch, with a cached merged-PR snapshot. ChatView would correctly copy the PR URL via the retained snapshot, while the palette's branch-mismatch check nulls out the detected PR, falling back to labeling the action "Copy thread ID" and copying the wrong thing entirely — a real, visible divergence in both behavior and label. I check the inverse and the linked-PR case, which both resolve consistently since they share linkedPullRequest.url, before moving to a third scenario involving the palette's own logic. Given this, I'll flag one inline comment on the CommandPalette lines with the added logic, making sure the line numbers align with the diff's right side for the review tool. ChatView needs no comment since it correctly reuses the shared resolver. There's a minor duplicated keydown block elsewhere I'll skip mentioning since it's just a style preference, though I want to note one real behavioral nuance about the copy shortcut only working while the item is visible in the filtered results. I realize the shortcut goes inert when the user types a filtering query that hides the target item, since ChatView's handler stays disabled while the palette is open. That's not a regression since the shortcut is new, but it's still a concrete inconsistency worth a light second inline comment, phrased carefully to avoid adding noise.

25. git_tools — const displayedGroups

26. git_tools — displayedGroups

💭 I've confirmed that displayedGroups is empty during browse/clone flows since it's just the query-filtered results, meaning the copy shortcut does nothing in those states—and since ChatView's global handler also bails when the palette is open, mod+shift+c is completely dead there. That gating makes sense for thread-jump commands since they target listed threads, but thread.copyReference acts on the active thread regardless of the list, so it shouldn't depend on list state at all. The fix is to call copyActiveThreadReference directly when the command resolves and a target exists, bypassing the displayedGroups search entirely—closing the palette first to match executeItem's existing behavior, then invoking the copy function which already handles its own errors so no additional catch is needed. For the second comment on lines 606-620, I'm double-checking that it's not self-contained enough for an inline diff suggestion since it depends on several imports—resolveDisplayedThreadPr, useLinkedThreadPullRequest, threadChangeRequestSnapshotsAtom, useAtomValue, and scopedThreadKey—so I'll need to describe the fix in prose rather than as a diff, and I want to verify where these hooks and atoms actually come from in ChatView's imports before finalizing that.

27. git_tools — useLinkedThreadPullRequest|threadChangeRequestSnapshotsAtom

💭 I'm drafting the review as a failure with findings, flagging that CommandPalette resolves the thread's PR with an ad-hoc comparison instead of using the shared resolveDisplayedThreadPr helper that ChatView and Sidebar rely on, creating a consistency risk worth calling out inline. For the second comment, I'm building a suggested diff that adds the thread.copyReference command handling with the standard preventDefault/stopPropagation/close-panel pattern, matching the file's existing indentation. Once both comments are ready, I'll post the review.

28. modify_pr — Both findings are in apps/web/src/components/CommandPalette.tsx; the ChatView side of the shortcut reuses the existing shared resolver and looks consistent with the surrounding toast/clipboard conventions.


Agent Credits: 331 credits

Token usage: 84 uncached input · 2,220,199 cache-read input · 223,950 cache-write input · 25,423 output
Agent Credits may also include non-token charges from external tools such as web research.