Sign up
View stargazersSign up

fix(web): scope command palette results to their environment#22099

closed2
main
GitHub fork: fix/fleet-palette-identity-20261020
opened11d agobySsaphid
5 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Connected environments restored from the same state can share project and thread IDs. The command palette then renders duplicate rows, marks both as current, and can open the wrong environment; project titles and relative-path roots also come from the last matching ID.

Use the existing environment-qualified project map and qualify thread row identity and current-thread checks by environment. Web and desktop share this palette; mobile has a separate navigation UI. No wire or provider changes.

Historical client evidence from the original implementation: verified the base failure and candidate in the real dev client using disposable same-ID environment fixtures: base showed two remote/current rows; candidate showed separate local/remote rows and ArrowDown + Enter navigated to the remote environment. The focused regression also failed on base. The original 42 palette tests, web typecheck, and targeted lint passed. Independent Claude review found no blocking defect. The relative-path root change is source-traced, without a separate filesystem browse runtime test.

Current integration (27ca2d3, 2026-10-23): main extracted palette lookups and thread items into separate modules. The environment-qualified identities, project lookup and current-thread checks now follow those modules. All 18 retained palette tests, web typecheck, scoped lint and formatting pass; required hosted checks pass on this commit. The captures below predate this integration; no new client session was run.

Before: Before

After: After

Implemented by GPT-6.1-Sol through the Codex harness; independently reviewed by Claude Sonnet 5.5.

Checks

24 successful · 14 skipped
All checks passed
27ca2d3

Added 1 commit

  • fix(web): scope command palette results to their environmentfba5520
MmacroscopeappBot11d ago
Synced from GitHub

Approvability

Verdict: Approved at 27ca2d3

Macroscope's review found this PR approvable — This is a focused command-palette bug fix that qualifies thread and project identity by environment, preventing duplicate results, incorrect current-thread indicators, and navigation to the wrong environment. The change is localized and includes regression coverage for shared IDs.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot11d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: abc56447-4c0a-4283-82fa-109bbaf13e0a
📥 Commits

Reviewing files that changed from the base of the PR and between a3ca1a57d65093c3f19d83f2c2b6d63663a380e1 and fba5520160849f7f014bcd28513b3ad2ea752d4b.

📒 Files selected for processing: 3
  • apps/web/src/components/CommandPalette.logic.test.ts
  • apps/web/src/components/CommandPalette.logic.ts
  • apps/web/src/components/CommandPalette.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The command palette now uses environment-qualified project lookups and thread values. Current-thread matching checks both the thread ID and environment ID. Tests cover duplicate thread IDs across environments and related search and display behavior.

Changes: Environment-aware command palette

Layer / File(s)Summary
Environment-aware project lookup
apps/web/src/components/CommandPalette.tsx		Project metadata and current-project resolution now use both the environment ID and project ID.
Environment-qualified thread actions
apps/web/src/components/CommandPalette.logic.ts, apps/web/src/components/CommandPalette.tsx, apps/web/src/components/CommandPalette.logic.test.ts		Thread action values include the environment ID. Current-thread matching checks both environment and thread IDs. Tests cover duplicate IDs across environments, selection, project context, and search behavior.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: ⚪ Minimal · up to fba55

The palette changes appear ready to merge after normal checks; no actionable merge-blocking issue remains in the supplied evidence.

Security architecture risk: 🔵 Low · up to fba55

The change improves separation between connected environments without changing permissions. A narrow identifier-encoding edge case remains: unusual stored identifiers could still make different results share an identity, although end-to-end reachability has not been established.

Retained concerns

  • Low · security · inferred: The new row identity is ambiguous over identifiers permitted by the schemas: environment/thread pairs ("a:b", "c") and ("a", "b:c") both produce "thread:a:b:c", whereas their previous thread-only row values differed. If both references reach the palette, highlighted-result lookup can select the wrong environment. Fresh environment IDs are UUIDs, but persisted IDs are accepted without delimiter validation. Existing scoped-key encoding has the same assumption, so production reachability and incremental exposure remain unresolved.
Security review details

Security Blast Radius

  • inferred — The unresolved collision would affect result selection in a client already presenting both environments. A hostile-state scenario requires influence over a connected environment's identity and thread data; ordinary fresh environment creation uses UUIDs. No broader server privilege or cross-user exposure was established.

Security Findings and Attack Paths

  • inferred — For delimiter-colliding row values, keyboard execution resolves the first enabled matching item and invokes its retained reference. This supports a conditional wrong-environment selection path, not a verified authentication or authorization bypass. Whether both references can coexist in production palette input remains unresolved.

Trust Boundaries and Controls

  • observed — Filesystem browsing retains the selected environment, its filesystem-read scope check, and connection-phase checks. The corrected project root is used only when its environment matches the browse environment.

Hardening Proposals

  • proposed — Use an unambiguous tuple encoding for composite selection identities, or explicitly enforce a delimiter-free environment-ID contract at ingestion. This would remove reliance on the existing implicit delimiter assumption.
Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main change: scoping command palette results by environment.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Description checkPassedThe description explains the problem, the implementation, scope boundaries, verification results, screenshots, limitations, and agent usage. It does not use the required section headings and does not …
Full details: Description check

Explanation

The description explains the problem, the implementation, scope boundaries, verification results, screenshots, limitations, and agent usage. It does not use the required section headings and does not provide an explicit triaged issue or maintainer approval comment, but the PR link and focused-fix context provide sufficient scope information.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Added 1 commit

  • fix(web): retain environment-scoped palette results through main refactor27ca2d3