Skip to content

Fix home favorites refresh deadlocks and polish workspace form UX.#117

Merged
tonythethompson merged 9 commits into
masterfrom
cursor/home-form-trust-ux
Jul 22, 2026
Merged

Fix home favorites refresh deadlocks and polish workspace form UX.#117
tonythethompson merged 9 commits into
masterfrom
cursor/home-form-trust-ux

Conversation

@tonythethompson

@tonythethompson tonythethompson commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Defer workspace-trust enforcement behind a shared kill switch, restore favorite move actions on the home menu, and tighten CmdPal form layout for suggestion pills and Open in terminal rows.


Summary by cubic

Defers workspace-trust enforcement behind a shared, JSON-backed kill switch and fixes Home favorites refresh deadlocks. Polishes the launch form with a compact, grid-based suggestion pill layout, a separator before command rows, and a non‑editable Open in terminal row; restores pinned move actions in Home.

  • New Features

    • Workspace trust kill switch via shared/workspace-trust-features.json (embedded in QuickShell.Core, synced into QuickShell.Raycast by scripts/sync-workspace-trust-features.js); default off. While off: imported/ingressed workspaces are trusted, trust UI/actions/badges are hidden in QuickShell.Run/QuickShell.Raycast/CmdPal, import messages omit the trust note, and repositories coerce/persist trusted rows.
    • Suggestion pills: 4-per-row grid; 3 rows collapsed, up to 5 rows expanded; emits only visible pills; hides while scanning; pills appear above command rows with a separator. Open in terminal renders as a non‑editable label with Remove. Companion-args labels/inputs are compact. Removed the in‑form terminal refresh control.
    • Faster terminal discovery using PathExecutableLookup (no where.exe calls).
  • Bug Fixes

    • Home reload deadlock: same‑thread Reload queues; cross‑thread waiters are woken when refresh ends; favorite/pin updates apply reliably. Home context menu shows pinned move commands when visible; move labels standardized.
    • Editor: Open in terminal no longer demotes when LaunchKind is dropped; Save with only Open in terminal works; Cancel/Discard skip merging inputs; Save errors keep the form open with a toast and return to the main form.
    • Trust policy and UI honor the kill switch across QuickShell.Core/QuickShell.Run/QuickShell.Raycast; addressed a CodeQL finding around Path.Combine handling.

Written for commit 0a2e4ec. Summary will update on new commits.

Review in cubic

Defer workspace-trust enforcement behind a shared kill switch, restore favorite move actions on the home menu, and tighten CmdPal form layout for suggestion pills and Open in terminal rows.

Co-authored-by: Cursor <cursoragent@cursor.com>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @tonythethompson, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This change adds a shared workspace-trust kill switch across Core and Raycast, updates trust persistence and UI behavior, refines Adaptive Card suggestion and launch-row rendering, fixes refresh re-entry handling, preserves open-terminal rows, restores home-pin move commands, and expands regression coverage.

Changes

Workspace trust feature gating

Layer / File(s) Summary
Trust configuration and runtime controls
shared/workspace-trust-features.json, QuickShell.Core/Services/WorkspaceTrustFeatures.cs, QuickShell.Raycast/src/lib/security.ts, QuickShell.Raycast/scripts/*
Shared trust configuration is embedded in Core, synchronized into Raycast, and exposed through runtime and test-scoped controls.
Authorization and security persistence
QuickShell.Core/Services/WorkspaceSecurityPolicy.cs, QuickShell.Core/Services/ShortcutRepository.cs, QuickShell.Raycast/src/lib/{security,storage,import-export}.ts
Authorization, imports, loaded layouts, and stored security defaults now depend on whether workspace trust is enabled.
Trust-aware UI and messaging
QuickShell.Run/Main.cs, QuickShell.Raycast/src/open-workspace.tsx, QuickShell/Services/*
Trust indicators, trust actions, Git actions, and import messages are conditionally rendered or described.
Trust lifecycle and disabled-mode tests
QuickShell.Core.Tests/WorkspaceTrust*, QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs, QuickShell.Raycast/src/__tests__/*
Tests isolate trust overrides and cover disabled-mode authorization, coercion, repository loading, shared defaults, and cleanup.
Estimated code review effort: 5 (Critical) ~120 minutes

Form and suggestion-pill rendering

Layer / File(s) Summary
Suggestion-pill visibility and template wiring
QuickShell.Core/Services/{SuggestionPillPresentation,ShortcutFormViewBuilder,ShortcutFormTemplateJson,ShortcutLaunchFormJson.SuggestionPills}.cs, QuickShell.Core.Tests/SuggestionPillPresentationTests.cs
Suggestion-pill visibility accounts for scanning and expansion state, and visible counts flow through template generation and caching.
Launch and companion card layout
QuickShell.Core/Services/{AdaptiveCardFormJson,CompanionAppFormJson,ShortcutLaunchFormJson,ShortcutFormTemplateJson}.cs, QuickShell.Core.Tests/{CompanionAppFormJsonTests,ShortcutLaunchFormJsonTests}.cs
Companion arguments, open-in-terminal rows, refresh controls, separators, and save-error visibility produce updated card JSON.
Open-in-terminal state preservation
QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs, QuickShell.Core.Tests/WorkspaceEditorTests.cs
Input merging preserves open-terminal rows, saving retains null commands, and appended command rows retain ordering.
Form submission and error presentation
QuickShell/Pages/ShortcutForm.cs
Cancel and discard actions bypass input application, while stay-open errors use toast plus keep-open results.
Estimated code review effort: 5 (Critical) ~120 minutes

Refresh and home-pin interactions

Layer / File(s) Summary
Refresh re-entry handling
QuickShell/Pages/QuickShellPage.cs, QuickShell.Core.Tests/QuickShellPageSearchTests.cs
Same-thread refreshes are queued without blocking, refresh state is released before notification, and queued queries are replayed.
Home-pin command behavior
QuickShell/Services/ShortcutContextCommands.cs, QuickShell/Services/ShortcutListItems.cs, QuickShell/Commands/MoveFavoriteShortcutCommand.cs, related tests
Home-pin menus omit page commands while retaining visible pinned move commands and forwarding reorder callbacks.
Interaction support
QuickShell.Run/RunLaunchSuggestionPanel.cs
The suggestion-panel height limit is increased for additional wrapped rows.
Estimated code review effort: 4 (Complex) ~60 minutes

Supporting project updates

Layer / File(s) Summary
Preferences and performance documentation
QuickShell.Raycast/raycast-env.d.ts, docs/performance-audit-2026-07-22.md
The Raycast preference type adds blockDirtyBranchSwitch, and a dated performance audit records findings and follow-up items.
Estimated code review effort: 2 (Simple) ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.21% which is insufficient. The required threshold is 60.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Pipeline Stage Enum Ordering ✅ Passed No SessionWorkflowStage enum, converter, or comparisons appear in the PR diff; the ordering check is not applicable.
Gpu/Cpu Runtime Boundary ✅ Passed No inference/, managed runtime requirements, main.py, or DiarizationProvider/Registry files were changed; the full diff is unrelated to the GPU/CPU boundary.
Managed Host Restart Safety ✅ Passed No modified file references the scoped host-management symbols, and no restart/stop/busy-state paths appear in the PR diff.
Title check ✅ Passed The title covers the main refresh deadlock and form UX work, even though it omits the workspace-trust changes.
Description check ✅ Passed The description is clearly related and summarizes the kill switch, Home fixes, and form layout updates.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/home-form-trust-ux
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch cursor/home-form-trust-ux

Warning

Review ran into problems

🔥 Problems

Linked repositories: Couldn't analyze Trackdubllc/Trackdub - clone failed: Clone operation failed: Cloning into '/home/jailuser/git'...
From https://github.com/Trackdubllc/Trackdub

Errors logged to '/home/jailuser/git/.git/lfs/logs/20260722T124243.791863212.log'.
Use git lfs logs last to view the log.
error: external filter 'git-lfs filter-process' failed
fatal: native/win-x64/libmpv-2.dll: smudge filter lfs failed
Downloading native/win-x64/libmpv-2.dll (100 MB)
Error downloading object: native/win-x64/libmpv-2.dll (d9c2711): Smudge error: Error downloading native/win-x64/libmpv-2.dll (d9c2711bfee3d332c37eaf9e3145831e9a96ddd8aba79bb382dc89a2f9b93b1a): LFS: Client error: https://github-cloud.githubusercontent.com/alambic/media/1086566765/d9/c2/d9c2711bfee3d332c37eaf9e3145831e9a96ddd8aba79bb382dc89a2f9b93b1a?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIA5BA2674WPWWEFGQ5%2F20260722%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20260722T124404Z&X-Amz-Expires=3600&X-Amz-Signature=d9920d882c0832d390a3ebb44844487a29bc283ae4301ee660c480abe0c360db&X-Amz-SignedHeaders=host&actor_id=136622811&key_id=0&repo_id=1215239847&token=1

Errors logged to '/home/jailuser/git/.git/lfs/logs/20260722T124404.440969625.log'.
Use git lfs logs last to view the log.
error: external filter 'git-lfs filter-process' failed
fatal: native/win-x64/libmpv-2.dll: smudge filter lfs failed


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

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Simplifying code... This may take up to 20 minutes.

Comment thread QuickShell/Pages/QuickShellPage.cs Fixed
Comment thread QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs Fixed
Comment thread QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs Fixed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f4773b978a

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Anthony Thompson <github@trackdub.com>
tonythethompson and others added 2 commits July 22, 2026 05:07
…ath.Combine' may silently drop its earlier arguments'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Anthony Thompson <github@trackdub.com>
…ath.Combine' may silently drop its earlier arguments'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Anthony Thompson <github@trackdub.com>
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Request timed out after 900000ms (requestId=518eab04-c7d9-49de-bad9-1ad37cf831ac)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec5a7301d2

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread QuickShell/Pages/QuickShellPage.cs Outdated
tonythethompson and others added 3 commits July 22, 2026 05:29
Open-directory still requires an existing rooted Windows path, so Linux CI
temp dirs correctly stay denied; assert trust is not the blocker there.

Co-authored-by: Cursor <cursoragent@cursor.com>
Wake cross-thread RefreshItems waiters when _refreshInProgress clears, and
render Open in terminal rows as non-editable TextBlock so typed input is not
silently discarded.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@tonythethompson
tonythethompson enabled auto-merge (squash) July 22, 2026 12:36
@tonythethompson
tonythethompson merged commit 9954044 into master Jul 22, 2026
8 of 9 checks passed
@tonythethompson
tonythethompson deleted the cursor/home-form-trust-ux branch July 22, 2026 12:38

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a2e4ecc7b

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (coercedTrust)
{
// Persist so a later re-enable does not revive pre-kill-switch denials.
WriteLayoutAtomic(_layout);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep loaded layout when trust-coercion write fails

When the trust kill switch is disabled and an existing shortcuts.json still contains untrusted security metadata, this load-time coercion now calls WriteLayoutAtomic before EnsureLoaded returns. If that write hits a transient Global\QuickShell_shortcuts_json mutex timeout or an access error, EnsureLoaded treats the already-parsed file as a load failure and RestoreLastGoodLayout() leaves first-start callers with an empty/stale in-memory workspace list; make this persistence best-effort or deferred after applying the loaded layout so a write failure cannot hide workspaces on startup.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
QuickShell.Raycast/src/lib/security.ts (1)

266-280: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Authorization Bypass (CWE-862): Missing Authorization

Reachability: External
● Entry
  QuickShell.Raycast/src/__tests__/storage.test.ts
│
▼
● Hop
  QuickShell.Raycast/src/lib/storage.ts
│
▼
● Hop
  QuickShell.Raycast/src/lib/import-export.ts:111
  mergeImportedData: Never adopt imported branchTargets; they auto-switch on launch.
│
▼
● Sink
  QuickShell.Raycast/src/lib/security.ts

Keep trust enforcement independent of the rollout flag Imported workspaces are written as trusted while isWorkspaceTrustEnabled() is false, and authorize() skips the WorkspaceUntrusted block in that mode. That lets clipboard-supplied JSON reach terminal/launch/companion/url actions without a trust review; keep imported workspaces untrusted and hide only the UI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell.Raycast/src/lib/security.ts` around lines 266 - 280, The trust
check in authorize() must not be gated by isWorkspaceTrustEnabled(). Keep
imported workspaces untrusted regardless of the rollout flag, enforce the
existing WorkspaceUntrusted block for the listed external actions, and limit the
flag’s effect to hiding or disabling the UI rather than authorization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/performance-audit-2026-07-22.md`:
- Around line 9-17: Update the performance evidence in the report around the
terminal discovery and overall measurement claims to include reproducibility
metadata: the exact benchmark command, measured commit, host/OS, sample count,
and whether results are cold or warm. Link or record the supporting artifact and
replace unsupported precision with values traceable to that measurement.

In `@QuickShell.Core/Services/ShortcutRepository.cs`:
- Around line 666-668: Update the ingress persistence paths, including
ImportMergeCore, ImportReplaceCore, and alternate-source restore, so they save
ingress workspace data with trust disabled regardless of the runtime enforcement
kill switch. Do not use WorkspaceTrustFeatures.CreateIngressSecurity() when
constructing persisted ShortcutLayoutEntry values; keep any enforcement
relaxation limited to runtime behavior.

In `@QuickShell.Raycast/src/lib/import-export.ts`:
- Line 164: Update the imported workspace persistence logic in the import/export
flow so the stored workspaceSecurity value always uses isTrusted: false,
regardless of whether workspace trust is currently enabled. Preserve the runtime
override that treats workspaces as trusted when trust is disabled, without
persisting that override.

In `@QuickShell.Raycast/src/lib/security.ts`:
- Around line 44-45: Update the missing-record branch in the security
normalization logic to set the existing changed flag when assigning next[id] =
current. Ensure QuickShellStorage.ensureLoaded() persists synthesized records
while preserving the current behavior for already-present rows.

In `@QuickShell.Raycast/src/open-workspace.tsx`:
- Around line 510-512: Update the failure messages in handleOpenFolder,
handleOpenUrl, and handleClearTargetBranch so they do not mention trusting the
workspace when isWorkspaceTrustEnabled() is false; use trust guidance only when
enabled, otherwise describe the invalid folder, URL, or remaining validation
failure while preserving the existing enabled-mode behavior.

In `@QuickShell/Commands/MoveFavoriteShortcutCommand.cs`:
- Around line 36-43: Localize the result messages returned by
MoveFavoriteShortcutCommand.Invoke instead of keeping the hardcoded English
strings. Add or reuse localized String resources for each move outcome,
including the favorite name and direction where applicable, and format them
consistently with the existing Strings.Command_Move*_Name resources.

In `@QuickShell/Pages/QuickShellPage.cs`:
- Around line 509-523: Extract the duplicated
_refreshInProgress/_refreshThreadId/_refreshQueued teardown and Monitor.PulseAll
logic into a single refresh-completion helper. Replace the success and catch
cleanup in the refresh flow with calls using onlyIfOwnedByCurrentThread: false,
and replace the finally guard cleanup with onlyIfOwnedByCurrentThread: true
while preserving queuedQuery assignment and re-entrancy behavior. Ensure the
thread-ID ownership check remains state-dependent so the CodeQL
constant-condition alert is resolved.

---

Outside diff comments:
In `@QuickShell.Raycast/src/lib/security.ts`:
- Around line 266-280: The trust check in authorize() must not be gated by
isWorkspaceTrustEnabled(). Keep imported workspaces untrusted regardless of the
rollout flag, enforce the existing WorkspaceUntrusted block for the listed
external actions, and limit the flag’s effect to hiding or disabling the UI
rather than authorization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 53c9ab79-cbfb-44f8-abc7-1fe81d650028

📥 Commits

Reviewing files that changed from the base of the PR and between 5644772 and 0a2e4ec.

📒 Files selected for processing (43)
  • QuickShell.Core.Tests/CompanionAppFormJsonTests.cs
  • QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs
  • QuickShell.Core.Tests/QuickShellPageSearchTests.cs
  • QuickShell.Core.Tests/ShortcutContextCommandsPreservationTests.cs
  • QuickShell.Core.Tests/ShortcutContextMenuRegressionTests.cs
  • QuickShell.Core.Tests/ShortcutLaunchFormJsonTests.cs
  • QuickShell.Core.Tests/SuggestionPillPresentationTests.cs
  • QuickShell.Core.Tests/WorkspaceEditorTests.cs
  • QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustCommandTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs
  • QuickShell.Core/QuickShell.Core.csproj
  • QuickShell.Core/Services/AdaptiveCardFormJson.cs
  • QuickShell.Core/Services/CompanionAppFormJson.cs
  • QuickShell.Core/Services/ShortcutFormTemplateJson.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.cs
  • QuickShell.Core/Services/ShortcutRepository.cs
  • QuickShell.Core/Services/SuggestionPillPresentation.cs
  • QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs
  • QuickShell.Core/Services/WorkspaceSecurityPolicy.cs
  • QuickShell.Core/Services/WorkspaceTrustFeatures.cs
  • QuickShell.Raycast/package.json
  • QuickShell.Raycast/raycast-env.d.ts
  • QuickShell.Raycast/scripts/sync-workspace-trust-features.js
  • QuickShell.Raycast/src/__tests__/security.test.ts
  • QuickShell.Raycast/src/__tests__/storage.test.ts
  • QuickShell.Raycast/src/__tests__/trust-import-flow.test.ts
  • QuickShell.Raycast/src/lib/import-export.ts
  • QuickShell.Raycast/src/lib/security.ts
  • QuickShell.Raycast/src/lib/storage.ts
  • QuickShell.Raycast/src/lib/workspace-trust-features.json
  • QuickShell.Raycast/src/open-workspace.tsx
  • QuickShell.Run/Main.cs
  • QuickShell.Run/RunLaunchSuggestionPanel.cs
  • QuickShell/Commands/MoveFavoriteShortcutCommand.cs
  • QuickShell/Pages/QuickShellPage.cs
  • QuickShell/Pages/ShortcutForm.cs
  • QuickShell/Services/ShortcutContextCommands.cs
  • QuickShell/Services/ShortcutFormViewBuilder.cs
  • QuickShell/Services/ShortcutListItems.cs
  • docs/performance-audit-2026-07-22.md
  • shared/workspace-trust-features.json
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Trackdubllc/Trackdub (manual)
  • tonythethompson/QuickShell (manual)
  • tonythethompson/numan (manual)
  • tonythethompson/dependency-chain-substrate (manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: Analyze Raycast TypeScript with CodeQL
  • GitHub Check: Analyze C# with CodeQL
  • GitHub Check: Performance harness (artifacts)
  • GitHub Check: .NET build and test
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{cs,csproj}

📄 CodeRabbit inference engine (AGENTS.md)

Treat analyzer warnings seriously; nullable, implicit usings, .NET analyzers, and StyleCop are enabled project-wide.

Files:

  • QuickShell.Core/QuickShell.Core.csproj
  • QuickShell.Run/RunLaunchSuggestionPanel.cs
  • QuickShell/Commands/MoveFavoriteShortcutCommand.cs
  • QuickShell.Core/Services/WorkspaceTrustFeatures.cs
  • QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs
  • QuickShell.Core/Services/AdaptiveCardFormJson.cs
  • QuickShell/Services/ShortcutFormViewBuilder.cs
  • QuickShell.Core/Services/CompanionAppFormJson.cs
  • QuickShell.Core/Services/WorkspaceSecurityPolicy.cs
  • QuickShell.Core.Tests/CompanionAppFormJsonTests.cs
  • QuickShell.Core/Services/ShortcutFormTemplateJson.cs
  • QuickShell.Core.Tests/WorkspaceTrustCommandTests.cs
  • QuickShell.Run/Main.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs
  • QuickShell.Core.Tests/QuickShellPageSearchTests.cs
  • QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs
  • QuickShell/Services/ShortcutListItems.cs
  • QuickShell.Core.Tests/WorkspaceEditorTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs
  • QuickShell.Core.Tests/ShortcutContextMenuRegressionTests.cs
  • QuickShell/Pages/QuickShellPage.cs
  • QuickShell/Pages/ShortcutForm.cs
  • QuickShell.Core/Services/ShortcutRepository.cs
  • QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs
  • QuickShell.Core.Tests/ShortcutLaunchFormJsonTests.cs
  • QuickShell.Core.Tests/ShortcutContextCommandsPreservationTests.cs
  • QuickShell/Services/ShortcutContextCommands.cs
  • QuickShell.Core.Tests/SuggestionPillPresentationTests.cs
  • QuickShell.Core/Services/SuggestionPillPresentation.cs
**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.cs: Keep namespaces aligned with folders, such as QuickShell.Pages, QuickShell.Commands, and QuickShell.Core.Services.
Define one type per C# file; prefer internal visibility, internal static class for stateless helpers, and internal sealed class for stateful singletons.

Files:

  • QuickShell.Run/RunLaunchSuggestionPanel.cs
  • QuickShell/Commands/MoveFavoriteShortcutCommand.cs
  • QuickShell.Core/Services/WorkspaceTrustFeatures.cs
  • QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs
  • QuickShell.Core/Services/AdaptiveCardFormJson.cs
  • QuickShell/Services/ShortcutFormViewBuilder.cs
  • QuickShell.Core/Services/CompanionAppFormJson.cs
  • QuickShell.Core/Services/WorkspaceSecurityPolicy.cs
  • QuickShell.Core.Tests/CompanionAppFormJsonTests.cs
  • QuickShell.Core/Services/ShortcutFormTemplateJson.cs
  • QuickShell.Core.Tests/WorkspaceTrustCommandTests.cs
  • QuickShell.Run/Main.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs
  • QuickShell.Core.Tests/QuickShellPageSearchTests.cs
  • QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs
  • QuickShell/Services/ShortcutListItems.cs
  • QuickShell.Core.Tests/WorkspaceEditorTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs
  • QuickShell.Core.Tests/ShortcutContextMenuRegressionTests.cs
  • QuickShell/Pages/QuickShellPage.cs
  • QuickShell/Pages/ShortcutForm.cs
  • QuickShell.Core/Services/ShortcutRepository.cs
  • QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs
  • QuickShell.Core.Tests/ShortcutLaunchFormJsonTests.cs
  • QuickShell.Core.Tests/ShortcutContextCommandsPreservationTests.cs
  • QuickShell/Services/ShortcutContextCommands.cs
  • QuickShell.Core.Tests/SuggestionPillPresentationTests.cs
  • QuickShell.Core/Services/SuggestionPillPresentation.cs
**/*.{cs,py}

📄 CodeRabbit inference engine (Custom checks)

Keep SessionWorkflowStage members in strictly ascending order: Foundation < MediaLoaded < Transcribed < Diarized < Translated < TtsGenerated. Comparisons must use enum member names rather than raw integer literals. When adding or renumbering members, provide a legacy-compatible JSON converter for old numeric values; when reordering, verify all inequalities across the solution retain their original semantic meaning.

Files:

  • QuickShell.Run/RunLaunchSuggestionPanel.cs
  • QuickShell/Commands/MoveFavoriteShortcutCommand.cs
  • QuickShell.Core/Services/WorkspaceTrustFeatures.cs
  • QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs
  • QuickShell.Core/Services/AdaptiveCardFormJson.cs
  • QuickShell/Services/ShortcutFormViewBuilder.cs
  • QuickShell.Core/Services/CompanionAppFormJson.cs
  • QuickShell.Core/Services/WorkspaceSecurityPolicy.cs
  • QuickShell.Core.Tests/CompanionAppFormJsonTests.cs
  • QuickShell.Core/Services/ShortcutFormTemplateJson.cs
  • QuickShell.Core.Tests/WorkspaceTrustCommandTests.cs
  • QuickShell.Run/Main.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs
  • QuickShell.Core.Tests/QuickShellPageSearchTests.cs
  • QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs
  • QuickShell/Services/ShortcutListItems.cs
  • QuickShell.Core.Tests/WorkspaceEditorTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs
  • QuickShell.Core.Tests/ShortcutContextMenuRegressionTests.cs
  • QuickShell/Pages/QuickShellPage.cs
  • QuickShell/Pages/ShortcutForm.cs
  • QuickShell.Core/Services/ShortcutRepository.cs
  • QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs
  • QuickShell.Core.Tests/ShortcutLaunchFormJsonTests.cs
  • QuickShell.Core.Tests/ShortcutContextCommandsPreservationTests.cs
  • QuickShell/Services/ShortcutContextCommands.cs
  • QuickShell.Core.Tests/SuggestionPillPresentationTests.cs
  • QuickShell.Core/Services/SuggestionPillPresentation.cs
QuickShell/**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

QuickShell/**/*.cs: Implement CmdPal pages as ICommand implementations so they can be used anywhere commands are used.
Use “workspace” as the UI term, while retaining %LOCALAPPDATA%\QuickShell\shortcuts.json as the on-disk filename and location.

Files:

  • QuickShell/Commands/MoveFavoriteShortcutCommand.cs
  • QuickShell/Services/ShortcutFormViewBuilder.cs
  • QuickShell/Services/ShortcutListItems.cs
  • QuickShell/Pages/QuickShellPage.cs
  • QuickShell/Pages/ShortcutForm.cs
  • QuickShell/Services/ShortcutContextCommands.cs
QuickShell.Core/**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

QuickShell.Core/**/*.cs: Keep QuickShell.Core free of CmdPal SDK dependencies so hosts remain swappable.
Use internal static helpers for pure logic and an interface plus DI registration for swappable dependencies.
Async methods ending in Async must accept CancellationToken cancellationToken = default; synchronous wrappers use .GetAwaiter().GetResult().

Files:

  • QuickShell.Core/Services/WorkspaceTrustFeatures.cs
  • QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs
  • QuickShell.Core/Services/AdaptiveCardFormJson.cs
  • QuickShell.Core/Services/CompanionAppFormJson.cs
  • QuickShell.Core/Services/WorkspaceSecurityPolicy.cs
  • QuickShell.Core/Services/ShortcutFormTemplateJson.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.cs
  • QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs
  • QuickShell.Core/Services/ShortcutRepository.cs
  • QuickShell.Core/Services/SuggestionPillPresentation.cs
QuickShell.Raycast/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for Raycast tests and maintain parity with Core launch behavior, including argument escaping, target resolution, and wt launch plans.

Files:

  • QuickShell.Raycast/raycast-env.d.ts
  • QuickShell.Raycast/src/__tests__/storage.test.ts
  • QuickShell.Raycast/src/__tests__/trust-import-flow.test.ts
  • QuickShell.Raycast/src/lib/import-export.ts
  • QuickShell.Raycast/src/__tests__/security.test.ts
  • QuickShell.Raycast/src/open-workspace.tsx
  • QuickShell.Raycast/src/lib/storage.ts
  • QuickShell.Raycast/src/lib/security.ts
QuickShell.Core.Tests/**/*.cs

📄 CodeRabbit inference engine (AGENTS.md)

QuickShell.Core.Tests/**/*.cs: Use xUnit tests, underscore-separated test method names, and no Moq or FluentAssertions.
Prefer real services and established seams such as LaunchExecutorTestEnvironment, FakeShortcutRepository, and AgentCliCatalog.IsCommandOnPathOverride instead of mocks; group process-wide seams with [Collection].

Files:

  • QuickShell.Core.Tests/CompanionAppFormJsonTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustCommandTests.cs
  • QuickShell.Core.Tests/QuickShellPageSearchTests.cs
  • QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs
  • QuickShell.Core.Tests/WorkspaceEditorTests.cs
  • QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs
  • QuickShell.Core.Tests/ShortcutContextMenuRegressionTests.cs
  • QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs
  • QuickShell.Core.Tests/ShortcutLaunchFormJsonTests.cs
  • QuickShell.Core.Tests/ShortcutContextCommandsPreservationTests.cs
  • QuickShell.Core.Tests/SuggestionPillPresentationTests.cs
QuickShell.Raycast/package.json

📄 CodeRabbit inference engine (AGENTS.md)

Require Node.js version >=22.14.0 and retain the configured Raycast tooling and lifecycle checks.

Files:

  • QuickShell.Raycast/package.json
QuickShell.Core/Services/{ShortcutRepository,IShortcutRepository,AtomicFileWriter,PersistenceVersion}.cs

📄 CodeRabbit inference engine (AGENTS.md)

Never write shortcuts.json outside ShortcutRepository; persist through IAtomicFileWriter.

Files:

  • QuickShell.Core/Services/ShortcutRepository.cs
🪛 ast-grep (0.44.1)
QuickShell.Raycast/scripts/sync-workspace-trust-features.js

[warning] 19-19: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(sharedPath, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)


[warning] 20-20: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(raycastPath, "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

🪛 LanguageTool
docs/performance-audit-2026-07-22.md

[style] ~17-~17: Specify a number, remove phrase, use “a few”, or use “some”
Context: ...e remaining gains look real but uneven: a small number of short, safe changes should buy visible ...

(SMALL_NUMBER_OF)


[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ell.Raycast/src/open-workspace.tsx:95, QuickShell.Raycast/src/lib/storage.ts:185, QuickShell.Ra...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re/Services/ShortcutRepository.cs:171, QuickShell.Core/Services/ShortcutRepository.cs:869, Q...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re/Services/ShortcutRepository.cs:869, QuickShell.Core/Services/ShortcutRepository.cs:1497, ...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e/Services/ShortcutRepository.cs:1497, QuickShell.Core/Services/ShortcutRepository.cs:1536, ...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e/Services/ShortcutRepository.cs:1536, QuickShell.Core/Services/ShortcutRepository.cs:1768` | ...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~76-~76: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...hout making the UI feel inconsistent. - Whether the current repository clone churn is w...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🪛 OpenGrep (1.25.0)
QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs

[WARNING] 126-126: File operation with dynamic path can lead to path traversal. Validate and sanitize file paths against a safe base directory.

(coderabbit.path-traversal.csharp-file-read)

🔍 Remote MCP GitHub Copilot

Additional review context

  • BuildForHomePin() is the home-list context menu for pinned shortcuts; the repo’s spec docs say the pre-fix bug was that it omitted Favorite, Duplicate, Delete, and multi-launch entries, while the full Build() menu on the fallback/search page should stay unchanged. ShortcutListItems.cs was the only production caller surfaced by search, so the new onFavoritesReordered callback has a narrow call surface.

  • BuildSuggestionPillsBlock() had one production caller surfaced in search: ShortcutFormTemplateJson.cs. That means the new visiblePillCount parameter mainly impacts template generation/caching, with no other internal callers found in the search results.

  • The repo also contains a dedicated context-menu-missing-items spec whose test artifacts describe the intended fixed behavior for home-pin menus, including the missing management commands.

🔇 Additional comments (46)
QuickShell.Raycast/raycast-env.d.ts (1)

18-20: LGTM!

docs/performance-audit-2026-07-22.md (1)

1-8: LGTM!

Also applies to: 19-77, 88-102

QuickShell.Core/Services/SuggestionPillPresentation.cs (1)

8-35: LGTM!

Also applies to: 114-121

QuickShell/Services/ShortcutFormViewBuilder.cs (1)

49-63: LGTM!

Also applies to: 86-87

QuickShell.Core/Services/ShortcutLaunchFormJson.SuggestionPills.cs (1)

5-95: LGTM!

Also applies to: 119-136

QuickShell.Core/Services/ShortcutFormTemplateJson.cs (1)

50-67: LGTM!

Also applies to: 101-101

QuickShell.Core.Tests/SuggestionPillPresentationTests.cs (1)

90-133: LGTM!

QuickShell.Core/Services/AdaptiveCardFormJson.cs (1)

377-389: LGTM!

QuickShell.Core/Services/CompanionAppFormJson.cs (1)

126-158: LGTM!

QuickShell.Core/Services/ShortcutLaunchFormJson.cs (1)

59-59: LGTM!

Also applies to: 121-122, 149-154

QuickShell.Core.Tests/CompanionAppFormJsonTests.cs (1)

55-60: LGTM!

QuickShell.Core.Tests/ShortcutLaunchFormJsonTests.cs (1)

46-47: LGTM!

Also applies to: 70-83, 96-113

QuickShell.Core/Services/WorkspaceEditor/WorkspaceEditor.cs (1)

1262-1279: LGTM!

QuickShell.Core.Tests/WorkspaceEditorTests.cs (1)

316-360: LGTM!

QuickShell/Pages/ShortcutForm.cs (5)

66-73: LGTM!


90-103: LGTM!


214-218: LGTM!


248-253: LGTM!


266-267: LGTM!

QuickShell/Pages/QuickShellPage.cs (1)

372-393: 🩺 Stability & Availability

Verify the queue-then-recursive-notify combination can't replay a stale query.

Same-thread re-entry during the build window sets _refreshQueued and captures _query at cleanup time (L515-519/567-571/599). But _refreshInProgress is cleared before RaiseItemsChanged() (L511-514), so a same-thread reentry triggered from the notify itself (the documented GetItems→Drain→Reload path) instead runs as a full nested recursive RefreshItems call, which can update the shared _query/_items again before the outer call's finally replays queuedQuery at L608 — potentially re-running the refresh with an older query than what the nested call already applied, and forcing notifyHost: true even when the original queued call didn't request it.

This is a narrow combination (queue-during-build and notify-triggered recursion in the same call), but given the deadlock this code fixes was already subtle, it's worth adding a regression test for this specific interleaving to confirm the replay can't clobber a newer result.

Also applies to: 606-609

QuickShell.Core.Tests/QuickShellPageSearchTests.cs (1)

170-201: LGTM!

QuickShell.Core.Tests/ShortcutContextCommandsPreservationTests.cs (1)

110-167: LGTM!

Also applies to: 416-416

QuickShell.Core.Tests/ShortcutContextMenuRegressionTests.cs (1)

103-133: LGTM!

QuickShell.Run/RunLaunchSuggestionPanel.cs (1)

51-58: LGTM!

shared/workspace-trust-features.json (1)

1-3: LGTM!

QuickShell.Core/QuickShell.Core.csproj (1)

17-22: LGTM!

QuickShell.Core/Services/WorkspaceSecurityPolicy.cs (1)

188-203: LGTM!

QuickShell.Core/Services/ShortcutRepository.cs (1)

666-669: LGTM!

Also applies to: 737-740, 1244-1257, 1355-1363, 1552-1560, 1895-1902

QuickShell.Run/Main.cs (1)

292-312: LGTM!

Also applies to: 505-508

QuickShell/Services/ShortcutListItems.cs (1)

60-64: LGTM!

Also applies to: 88-94

QuickShell/Services/ShortcutContextCommands.cs (1)

172-194: LGTM!

Also applies to: 439-449

QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs (1)

1-166: LGTM!

QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs (1)

124-130: LGTM!

QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs (1)

7-12: LGTM!

QuickShell.Core.Tests/WorkspaceTrustCommandTests.cs (1)

9-14: LGTM!

QuickShell.Raycast/src/lib/workspace-trust-features.json (1)

1-3: LGTM!

QuickShell.Raycast/scripts/sync-workspace-trust-features.js (1)

1-25: LGTM!

QuickShell.Core/Services/WorkspaceTrustFeatures.cs (1)

1-148: LGTM!

QuickShell.Raycast/src/lib/security.ts (1)

6-24: LGTM!

Also applies to: 27-43, 49-50, 267-267, 283-283

QuickShell.Raycast/package.json (1)

202-205: LGTM!

QuickShell.Raycast/src/lib/import-export.ts (1)

5-5: LGTM!

Also applies to: 161-164

QuickShell.Raycast/src/lib/storage.ts (1)

21-27: LGTM!

Also applies to: 159-162, 550-557, 620-620

QuickShell.Raycast/src/open-workspace.tsx (1)

48-54: LGTM!

Also applies to: 635-635, 681-681, 762-768

QuickShell.Raycast/src/__tests__/security.test.ts (1)

1-23: LGTM!

Also applies to: 313-361

QuickShell.Raycast/src/__tests__/storage.test.ts (1)

1-14: LGTM!

QuickShell.Raycast/src/__tests__/trust-import-flow.test.ts (1)

4-21: LGTM!

Comment on lines +9 to +17
The biggest remaining time sinks are no longer in the root palette itself; they’re concentrated in startup discovery, first-paint list assembly, and form/list enrichment. The clearest single win is terminal discovery on the critical path: the current implementation still shells out through serial `where.exe` lookups, and live measurement showed that the discovery fan-out alone accounts for roughly a second of cold-launch cost.

The second cluster is repeated work during list and form construction. A few call sites already cache the right data, but the surrounding presentation layer still recomputes workspace row state, suggestion pills, and form JSON more often than necessary. That makes the hot paths pay for stable data multiple times.

The third cluster is Raycast storage and health loading. The host already uses background work in some places, but the current load path can still re-parse storage and do per-workspace health work in ways that are fine functionally but expensive at scale.

Launch itself is mostly shaped by correctness constraints rather than simple caching. Health, trust, and git checks still need to stay live at launch time, so the remaining work there is limited to deterministic prep and lower-level discovery. The performance docs were slightly optimistic compared to current measurement, so the repo should treat the current artifact as the source of truth.

Overall, the remaining gains look real but uneven: a small number of short, safe changes should buy visible improvements, while the larger opportunities are architectural and should be validated with the existing harnesses rather than guessed at.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the performance evidence reproducible before treating it as the source of truth.

The report relies on a “live measurement” and a roughly 1.03-second discovery figure, but only names source locations and an artifact path. Record or link the benchmark command, commit, host/OS, sample count, and cold-versus-warm conditions so these prioritization claims do not create false precision.

Also applies to: 78-87

🧰 Tools
🪛 LanguageTool

[style] ~17-~17: Specify a number, remove phrase, use “a few”, or use “some”
Context: ...e remaining gains look real but uneven: a small number of short, safe changes should buy visible ...

(SMALL_NUMBER_OF)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/performance-audit-2026-07-22.md` around lines 9 - 17, Update the
performance evidence in the report around the terminal discovery and overall
measurement claims to include reproducibility metadata: the exact benchmark
command, measured commit, host/OS, sample count, and whether results are cold or
warm. Link or record the supporting artifact and replace unsupported precision
with values traceable to that measurement.

Comment on lines 666 to +668
layout.Add(ShortcutLayoutEntry.FromShortcut(
shortcut,
new WorkspaceSecurityMetadata { IsTrusted = false, Revision = 1 }));
WorkspaceTrustFeatures.CreateIngressSecurity()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file list =="
git ls-files | sed -n '1,120p'

echo
echo "== relevant file outline =="
ast-grep outline QuickShell.Core/Services/ShortcutRepository.cs --view expanded || true

echo
echo "== search for trust-related symbols =="
rg -n "CreateIngressSecurity|WorkspaceTrustFeatures|trust|untrusted|trusted|IngressSecurity|replace-import|alternate-source|import" QuickShell.Core -S || true

Repository: tonythethompson/QuickShell

Length of output: 25685


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the relevant region in ShortcutRepository.cs
sed -n '620,720p' QuickShell.Core/Services/ShortcutRepository.cs | cat -n

echo
echo "== definitions/usages of WorkspaceTrustFeatures =="
rg -n "class WorkspaceTrustFeatures|struct WorkspaceTrustFeatures|enum WorkspaceTrustFeatures|CreateIngressSecurity|IngressSecurity" -S .

echo
echo "== any persistence of trust metadata in shortcut layout =="
rg -n "ShortcutLayoutEntry\.FromShortcut|layout\.Add\(|Trust|IsTrusted|WorkspaceTrust" QuickShell.Core -S

Repository: tonythethompson/QuickShell

Length of output: 14140


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Relevant source slices with line numbers
echo "== QuickShell.Core/Services/ShortcutRepository.cs (around 666-668) =="
sed -n '650,690p' QuickShell.Core/Services/ShortcutRepository.cs | cat -n

echo
echo "== WorkspaceTrustFeatures definition =="
fd -a "WorkspaceTrustFeatures.cs" .
fd -a "*Trust*" QuickShell.Core .

Repository: tonythethompson/QuickShell

Length of output: 2404


🌐 Web query:

"CreateIngressSecurity WorkspaceTrustFeatures QuickShell"

💡 Result:

The terms "CreateIngressSecurity," "WorkspaceTrustFeatures," and "QuickShell" do not refer to a single integrated feature or unified technology. Instead, they relate to distinct concepts across different software ecosystems. 1. Workspace Trust Features (Visual Studio Code): "WorkspaceTrustFeatures" refers to the Workspace Trust security framework in Visual Studio Code (and OpenVSCode Server) [1][2][3]. It is a security mechanism that prevents unintended code execution when opening workspaces from untrusted sources by operating in a "Restricted Mode" [1][2]. The specific code reference createWorkspaceTrustFeatures (or similar variants like createFeaturesElement) exists within the VS Code internal source code, specifically in modules responsible for rendering the workspace trust settings editor [4]. 2. QuickShell: "QuickShell" is an unrelated, distinct open-source toolkit for building desktop shell components (such as status bars, widgets, and lockscreens) using QtQuick and QML [5][6]. It is a desktop environment utility and has no functional connection to VS Code's security model [5][7]. 3. Create Ingress Security: "CreateIngressSecurity" does not appear as a standard programming function or feature in either of the above contexts. The term "ingress" in security generally refers to network security controls, such as Cisco TrustSec's "ingress tagging" [8] or cloud infrastructure "ingress security rules" used in network security groups (e.g., Oracle Cloud) [9]. In summary, there is no technical entity or documented API that combines these three terms. They likely appear together in a search context only if a user or system is coincidentally referencing VS Code internals, a desktop shell, and network configuration separately.

Citations:


🏁 Script executed on all repositories:

#!/bin/bash
set -euo pipefail
rg -n "CreateIngressSecurity|WorkspaceTrustFeatures|IngressSecurity" -S . || true

Length of output: 7261


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path("QuickShell.Core/Services/ShortcutRepository.cs")
text = p.read_text()
for needle in ["CreateIngressSecurity", "FromShortcut(", "alternate-source", "replace-import"]:
    print(f"== {needle} ==")
    for i, line in enumerate(text.splitlines(), 1):
        if needle in line:
            start = max(1, i-8)
            end = min(len(text.splitlines()), i+8)
            for j in range(start, end+1):
                print(f"{j:4}: {text.splitlines()[j-1]}")
            print()
PY

Repository: tonythethompson/QuickShell

Length of output: 8675


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,180p' QuickShell.Core/Services/WorkspaceTrustFeatures.cs | cat -n

echo
echo "== trust coercion tests =="
sed -n '1,180p' QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs | cat -n

echo
echo "== import flow tests =="
sed -n '1,240p' QuickShell.Core.Tests/Flows/FormTrustImportFlowTests.cs | cat -n

Repository: tonythethompson/QuickShell

Length of output: 23757


Authorization Bypass (CWE-863): Incorrect Authorization

Reachability: External
● Entry
  QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs:18
  CreateWorkspace
│
▼
● Hop
  QuickShell.Core.Tests/WorkspaceTrustFeaturesDisabledTests.cs:83
  Repository_load_coerces_untrusted_rows_while_disabled: Best-effort cleanup.
│
▼
● Sink
  QuickShell.Core/Services/ShortcutRepository.cs

Keep ingress workspaces untrusted
ImportMergeCore, ImportReplaceCore, and alternate-source restore all persist WorkspaceTrustFeatures.CreateIngressSecurity(), and that helper writes IsTrusted = !Enabled. With the trust kill switch off, imported workspaces are saved as trusted and will skip the review gate if enforcement is turned back on later. Keep persisted ingress data untrusted; only relax enforcement at runtime.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell.Core/Services/ShortcutRepository.cs` around lines 666 - 668,
Update the ingress persistence paths, including ImportMergeCore,
ImportReplaceCore, and alternate-source restore, so they save ingress workspace
data with trust disabled regardless of the runtime enforcement kill switch. Do
not use WorkspaceTrustFeatures.CreateIngressSecurity() when constructing
persisted ShortcutLayoutEntry values; keep any enforcement relaxation limited to
runtime behavior.

existingSecurity && base.workspaces.some((candidate) => candidate.id === workspace.id)
? { ...existingSecurity }
: { isTrusted: false, revision: 1 },
: { isTrusted: !isWorkspaceTrustEnabled(), revision: 1 },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== target file lines =="
nl -ba QuickShell.Raycast/src/lib/import-export.ts | sed -n '1,260p'

echo
echo "== search trust symbols =="
rg -n "isWorkspaceTrustEnabled|isTrusted|workspace trust|trust" QuickShell.Raycast/src -g '!**/dist/**' -g '!**/build/**'

Repository: tonythethompson/QuickShell

Length of output: 228


🏁 Script executed:

#!/bin/bash
set -euo pipefail
nl -ba QuickShell.Raycast/src/lib/import-export.ts | sed -n '1,260p'
echo '---'
rg -n "isWorkspaceTrustEnabled|isTrusted|workspace trust|trust" QuickShell.Raycast/src -g '!**/dist/**' -g '!**/build/**'

Repository: tonythethompson/QuickShell

Length of output: 204


🔍 Additional research query:

DeepWiki: QuickShell.Raycast import-export trust flow around QuickShell.Raycast/src/lib/import-export.ts and how isTrusted is persisted and later read

💡 Result:

QuickShell.Raycast trust flow

  • exportStoredData() removes workspaceSecurity before serializing, so isTrusted is not exported in the portable payload. It also removes branchTargets.

  • On import, mergeImportedData() rebuilds workspaceSecurity for the returned StoredData:

    • if the workspace ID already exists in base.workspaceSecurity, that entry is copied forward;
    • otherwise a new entry is created with isTrusted: !isWorkspaceTrustEnabled() and revision: 1.
  • The trust state is therefore persisted in StoredData.workspaceSecurity[workspace.id], not in the exported JSON.

  • In the storage code we found, new workspaces are initialized with workspaceSecurity[workspace.id] = { isTrusted: false, revision: 1 }, which is another place where the trust state is written into persisted storage.

  • I wasn’t able to retrieve a separate DeepWiki explanation before the tool limit was reached.


🌐 Web query:

tonythethompson/QuickShell QuickShell.Raycast/src/lib/import-export.ts isTrusted workspace trust

💡 Result:

The query refers to code components found in the QuickShell project by Anthony Thompson (tonythethompson) [1][2], specifically concerning internal modules related to workspace management and workspace trust [3]. QuickShell is a launcher extension (available for Microsoft Command Palette and PowerToys Run) that manages "workspaces" [1][4]. Within such applications, workspace trust is a security mechanism—modeled after concepts like Visual Studio Code's Workspace Trust—designed to prevent unintended execution of code or scripts found within project folders [5][6]. The file path src/lib/import-export.ts in the context of QuickShell.Raycast (a reference to the Raycast-inspired architecture/design influence in the project [7]) typically handles the serialization, import, and export of workspace configurations [3]. The presence of an isTrusted property or check in this module is a standard security implementation to ensure that when a user imports or exports workspace settings, the application verifies the trust level of the workspace before executing or loading associated scripts, environment variables, or commands [8][9][6]. In modern VS Code-like environments, isTrusted is frequently used as a boolean flag to gate sensitive operations (like shell integration or environment activation) to prevent arbitrary code execution by ensuring the environment or command originated from a verified, user-consented source [8][9][10]. If you are encountering this in the source code, it indicates a deliberate security gate meant to verify the integrity of the workspace data before performing actions that could impact system security [5][6].

Citations:


🏁 Script executed (no clone):

#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import urllib.request
url = "https://raw.githubusercontent.com/tonythethompson/QuickShell/main/QuickShell.Raycast/src/lib/import-export.ts"
print(urllib.request.urlopen(url, timeout=20).read().decode("utf-8"))
PY

Length of output: 2411


Authorization Bypass (CWE-863): Incorrect Authorization

Reachability: External
● Entry
  QuickShell.Raycast/src/__tests__/storage.test.ts
│
▼
● Hop
  QuickShell.Raycast/src/lib/storage.ts
│
▼
● Sink
  QuickShell.Raycast/src/lib/import-export.ts

Persist imported workspaces as untrusted Imported workspaces are written with isTrusted: true while workspace trust is disabled, which makes that approval durable in workspaceSecurity. If trust is re-enabled later, those imported workspaces stay approved instead of being re-evaluated. Keep the stored value untrusted and apply the disabled-mode override only at runtime.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell.Raycast/src/lib/import-export.ts` at line 164, Update the imported
workspace persistence logic in the import/export flow so the stored
workspaceSecurity value always uses isTrusted: false, regardless of whether
workspace trust is currently enabled. Preserve the runtime override that treats
workspaces as trusted when trust is disabled, without persisting that override.

Comment on lines +44 to +45
} else if (!next[id]) {
next[id] = current;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Mark synthesized security records as changed.

Adding a missing row mutates security, but leaves changed false. QuickShellStorage.ensureLoaded() therefore skips persistence when records are only missing, so this normalization is repeated after every reload.

Proposed fix
     } else if (!next[id]) {
       next[id] = current;
+      changed = true;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else if (!next[id]) {
next[id] = current;
} else if (!next[id]) {
next[id] = current;
changed = true;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell.Raycast/src/lib/security.ts` around lines 44 - 45, Update the
missing-record branch in the security normalization logic to set the existing
changed flag when assigning next[id] = current. Ensure
QuickShellStorage.ensureLoaded() persists synthesized records while preserving
the current behavior for already-present rows.

Comment on lines +510 to +512
message: isWorkspaceTrustEnabled()
? `${result.imported} imported, ${result.skipped} skipped, ${result.renamed} renamed. Imported workspaces are untrusted until reviewed.`
: `${result.imported} imported, ${result.skipped} skipped, ${result.renamed} renamed.`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make disabled-mode failure messages trust-neutral.

handleOpenFolder, handleOpenUrl, and handleClearTargetBranch still say “Trust this workspace…” when trust is disabled and the actual blocker is an invalid folder or URL. The UI hides the trust action, so the toast directs users to an impossible action. Make those messages conditional or describe only the remaining validation failure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell.Raycast/src/open-workspace.tsx` around lines 510 - 512, Update the
failure messages in handleOpenFolder, handleOpenUrl, and handleClearTargetBranch
so they do not mention trusting the workspace when isWorkspaceTrustEnabled() is
false; use trust guidance only when enabled, otherwise describe the invalid
folder, URL, or remaining validation failure while preserving the existing
enabled-mode behavior.

Comment on lines 36 to 43
Name = move switch
{
FavoriteMoveKind.Up => "Move up",
FavoriteMoveKind.Down => "Move down",
FavoriteMoveKind.ToTop => "Move to top",
FavoriteMoveKind.ToBottom => "Move to bottom",
FavoriteMoveKind.Up => Strings.Command_MoveUp_Name,
FavoriteMoveKind.Down => Strings.Command_MoveDown_Name,
FavoriteMoveKind.ToTop => Strings.Command_MoveToTop_Name,
FavoriteMoveKind.ToBottom => Strings.Command_MoveToBottom_Name,
_ => "Move",
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Localize the Invoke() result messages too.

Name now pulls from Strings.Command_Move*_Name, but the result strings returned from Invoke() (L74-81, e.g. $"Moved '{_name}' up in favorites.") are still hardcoded English. Since the command's display name is now localizable, the follow-up status text is an inconsistency users in other locales will notice.

Also applies to: 74-81

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell/Commands/MoveFavoriteShortcutCommand.cs` around lines 36 - 43,
Localize the result messages returned by MoveFavoriteShortcutCommand.Invoke
instead of keeping the hardcoded English strings. Add or reuse localized String
resources for each move outcome, including the favorite name and direction where
applicable, and format them consistently with the existing
Strings.Command_Move*_Name resources.

Comment on lines +509 to +523
// Release before host notify. RaiseItemsChanged can re-enter GetItems/Reload on
// this thread; leaving _refreshInProgress set through notify deadlocks waiters.
lock (_refreshSync)
{
_refreshInProgress = false;
_refreshThreadId = 0;
if (_refreshQueued)
{
_refreshQueued = false;
queuedQuery = _query;
}

Monitor.PulseAll(_refreshSync);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract the repeated refresh-cleanup block; this also appears to be the CodeQL "constant condition" fix.

The _refreshInProgress/_refreshThreadId/_refreshQueued teardown-and-pulse sequence is duplicated three times (success path, catch path, finally guard). This is delicate synchronization code — any future tweak (e.g. a new flag) risks drifting out of sync across the copies. Worth extracting into a small helper, e.g.:

♻️ Proposed extraction
+    private string? CompleteRefresh(bool onlyIfOwnedByCurrentThread)
+    {
+        lock (_refreshSync)
+        {
+            if (onlyIfOwnedByCurrentThread
+                && !(_refreshInProgress && _refreshThreadId == Environment.CurrentManagedThreadId))
+            {
+                return null;
+            }
+
+            _refreshInProgress = false;
+            _refreshThreadId = 0;
+            string? queuedQuery = null;
+            if (_refreshQueued)
+            {
+                _refreshQueued = false;
+                queuedQuery = _query;
+            }
+
+            Monitor.PulseAll(_refreshSync);
+            return queuedQuery;
+        }
+    }

Then each call site becomes a one-liner, e.g. queuedQuery = CompleteRefresh(onlyIfOwnedByCurrentThread: false); in the try/catch paths and queuedQuery ??= CompleteRefresh(onlyIfOwnedByCurrentThread: true); in finally.

Separately: the past CodeQL alert on this file flagged _refreshInProgress access as an always-true constant condition. The new _refreshThreadId == Environment.CurrentManagedThreadId comparisons make the finally-block guard (L592) state-dependent rather than trivially true, so this looks like the intended fix — worth confirming the alert is now closed.

Also applies to: 562-575, 590-603

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@QuickShell/Pages/QuickShellPage.cs` around lines 509 - 523, Extract the
duplicated _refreshInProgress/_refreshThreadId/_refreshQueued teardown and
Monitor.PulseAll logic into a single refresh-completion helper. Replace the
success and catch cleanup in the refresh flow with calls using
onlyIfOwnedByCurrentThread: false, and replace the finally guard cleanup with
onlyIfOwnedByCurrentThread: true while preserving queuedQuery assignment and
re-entrancy behavior. Ensure the thread-ID ownership check remains
state-dependent so the CodeQL constant-condition alert is resolved.

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.

2 participants