Optimize command palette search - #4043
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCommandPaletteSearch moves fuzzy matching to prepared token/candidate representations, adds bounded top‑N search and a Rust Nucleo FFI index with a Swift dynamic wrapper. ContentView forwards result limits and uses the Nucleo index for previews. Tests, CI, scripts, and Xcode wiring added for build, run, and benchmarks. ChangesPrepared token fuzzy matching refactor
Nucleo-backed FFI index and integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (5 errors, 1 warning, 1 inconclusive)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR optimizes Cmd-P command palette search by precomputing normalized candidate representations (
Confidence Score: 5/5Safe to merge; the three previously-identified blocking issues (synchronous full-corpus FFI on main actor, main-actor index build, and @observableobject render model) are all resolved in this revision. The Nucleo index build runs in a cancellable Task.detached with a generation guard, the synchronous seed path fires only on cold-start when no results are visible, and CommandPaletteOverlayRenderModel is correctly @observable. No new correctness bugs were found across the FFI ABI, scoring logic, min-heap implementations, or the typo-fallback merge path. CommandPaletteSearch.swift has dead overloads and an unexplained penalty constant change; CommandPaletteOverlay.swift scheduleCommandListUpdate tasks all execute as noted in a prior review thread. Important Files Changed
Reviews (19): Last reviewed commit: "Remove stale command palette label helpe..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/CommandPalette/CommandPaletteSearch.swift (1)
617-670: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAddress SwiftLint
discouraged_optional_collectionwarnings on the new match-indices functions.
stitchedWordPrefixMatchIndices,subsequenceMatchIndices, andinitialismMatchIndicesall returnSet<Int>?and trigger SwiftLint warnings at lines 620, 918, and 943. Each implementation guarantees at least one inserted index on success, so an emptySet<Int>is a safe sentinel for "no match" and removes the optional wrapper. Callers inmatchCharacterIndices(lines 361, 366, 372) become a simpleisEmptycheck.♻️ Proposed refactor (illustrative)
- private static func stitchedWordPrefixMatchIndices( - token: PreparedToken, - candidate: PreparedCandidateText - ) -> Set<Int>? { + private static func stitchedWordPrefixMatchIndices( + token: PreparedToken, + candidate: PreparedCandidateText + ) -> Set<Int> { let tokenChars = token.characters let candidateChars = candidate.characters - guard tokenChars.count >= 4 else { return nil } + guard tokenChars.count >= 4 else { return [] } let segments = candidate.wordSegments - guard segments.count >= 2 else { return nil } + guard segments.count >= 2 else { return [] } ... - if !foundMatch { return nil } + if !foundMatch { return [] } } - guard usedWords >= 2 else { return nil } + guard usedWords >= 2 else { return [] } return matchedIndices }And at the call sites:
- if let stitched = stitchedWordPrefixMatchIndices(token: token, candidate: preparedCandidate) { - matched.formUnion(stitched) - continue - } + let stitched = stitchedWordPrefixMatchIndices(token: token, candidate: preparedCandidate) + if !stitched.isEmpty { + matched.formUnion(stitched) + continue + }Apply the same pattern to
subsequenceMatchIndicesandinitialismMatchIndices. If you prefer to keep the optional semantics, add a narrowly scoped// swiftlint:disable:next discouraged_optional_collectionon each signature with a comment explaining the nil-vs-empty distinction.As per coding guidelines: "Apply the cmux custom Swift lint rules in
.github/review-bot-rules/" and "Only disable noisy style/formatting rules; don't broaden disabled rules to hide substantive issues."Also applies to: 918-941, 943-969
🤖 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 `@Sources/CommandPalette/CommandPaletteSearch.swift` around lines 617 - 670, The three match-index functions (stitchedWordPrefixMatchIndices, subsequenceMatchIndices, initialismMatchIndices) currently return Set<Int>? which triggers SwiftLint discouraged_optional_collection; change each signature to return a non-optional Set<Int> and use an empty set to represent "no match" instead of nil, ensuring all code paths that previously returned nil now return Set() and that successful paths return a non-empty set. Update the callers in matchCharacterIndices to treat results via isEmpty checks (replace nil checks with isEmpty) and remove optional unwrapping; keep all existing semantics (a successful match still produces >=1 index) and do not add broad lint disables.
🤖 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.
Outside diff comments:
In `@Sources/CommandPalette/CommandPaletteSearch.swift`:
- Around line 617-670: The three match-index functions
(stitchedWordPrefixMatchIndices, subsequenceMatchIndices,
initialismMatchIndices) currently return Set<Int>? which triggers SwiftLint
discouraged_optional_collection; change each signature to return a non-optional
Set<Int> and use an empty set to represent "no match" instead of nil, ensuring
all code paths that previously returned nil now return Set() and that successful
paths return a non-empty set. Update the callers in matchCharacterIndices to
treat results via isEmpty checks (replace nil checks with isEmpty) and remove
optional unwrapping; keep all existing semantics (a successful match still
produces >=1 index) and do not add broad lint disables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce20d9c0-3cd4-4c9e-b7a7-87b1c0b94a00
📒 Files selected for processing (2)
Sources/CommandPalette/CommandPaletteSearch.swiftcmuxTests/CommandPaletteSearchEngineTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/CommandPalette/CommandPaletteSearch.swift (2)
1160-1233: 🧹 Nitpick | 🔵 Trivial | 🏗️ Heavy liftConsider a partial sort when
resultLimitis provided.
scoredEntriesis reserved and sorted at fullentries.count, then only the topoutputCountare materialized. For the large-workspace benchmark fixture this PR targets,entries.countis large andresultLimitis small, so a full O(n log n) sort dominates the post-scoring work even though only the top‑N are ever inspected. Switching the bounded path to a partial sort / bounded heap (track the lowest score in a min‑heap of sizeresultLimit, sorted once at the end) keeps the existing tie‑breakers (scoredesc,rankasc,titlelocalized case‑insensitive asc) while cutting the comparison count substantially on the hot path. The unbounded path can keepArray.sortas is.This is purely an additional optimization on top of the prepared‑candidate work and shouldn’t affect ranking semantics or test expectations, so it’s well aligned with the PR’s performance focus.
🤖 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 `@Sources/CommandPalette/CommandPaletteSearch.swift` around lines 1160 - 1233, The full sort of scoredEntries even when resultLimit is small causes unnecessary O(n log n) work; replace the bounded path (when resultLimit != nil -> outputCount) with a fixed-size min-heap (or selection algorithm) that keeps the top N ScoredEntry items by the existing comparator (score desc, rank asc, title localizedCaseInsensitive asc) during the scan, while preserving the existing cancellation checks (shouldCancelSearch) and using the same scoring (weightedScore + historyBoost); after the scan, extract the heap into an array and sort it once with the same comparator before building the results (CommandPaletteSearchCorpusResult), and keep the current full Array.sort behavior for the unbounded path so semantics and tie‑breakers (ScoredEntry, scoredEntries, resultLimit/outputCount, shouldCancelSearch) are unchanged.
137-1276: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit this file: extract
CommandPaletteFuzzyMatcherto keep the file under the length budget.The post-change file is 1,276 physical lines, well past the 800-line ceiling, and this PR adds materially more than 250 lines to a file that was already past 800. The matcher (
CommandPaletteFuzzyMatcher, ~140–1059) and the engine/corpus types (CommandPaletteSearchCorpusEntry,CommandPaletteSearchCorpusResult,CommandPaletteSearchEngine, ~1061–1275) are two cohesive but distinct responsibilities (low-level fuzzy matching primitives vs. corpus scoring / ranking / result shaping) and currently sit in the same file only by convention. MovingCommandPaletteFuzzyMatcher(withWordSegment,ASCIIScalarMask,PreparedToken,PreparedCandidateText,PreparedQuery, and the private helpers) into a sibling file such asSources/CommandPalette/CommandPaletteFuzzyMatcher.swiftis a clean, behavior-preserving extraction that brings this file back well under the threshold and satisfies the documented extraction exception (move mixed responsibilities, reduce total line count by >200 lines). No API surface change is required since the matcher already exposes all needed entry points statically.As per coding guidelines: "do not accept more than 250 lines added to an existing production Swift file that is already over 800 lines, unless an extraction exception is met by removing/moving mixed responsibilities and decreasing total line count by more than 200 lines" and "use 500 lines as the tracked-file reference threshold from
.github/swift-file-length-budget.tsv".🤖 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 `@Sources/CommandPalette/CommandPaletteSearch.swift` around lines 137 - 1276, Extract the entire CommandPaletteFuzzyMatcher enum (including nested types WordSegment, ASCIIScalarMask, PreparedToken, PreparedCandidateText, PreparedQuery, SingleEditWordPrefixMatch, SingleEditWordPrefixEditKind and all private helpers) into a new sibling source file (e.g., CommandPaletteFuzzyMatcher.swift), preserving all declarations, access levels, and static APIs exactly as-is so callers like CommandPaletteSearchEngine, CommandPaletteSearchCorpusEntry, and CommandPaletteSearchCorpusResult keep working; remove the original enum from the current file so the remaining file contains only the corpus/engine types, ensure the new file is compiled in the same target (no API changes), and run the build to fix any visibility/import issues.
🤖 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 `@Sources/ContentView.swift`:
- Around line 5329-5338: The current logic in ContentView that sets
previewCandidateCommandIDs to [] when commandPaletteVisibleResultsScope and/or
fingerprint don't match causes the UI to briefly clear visible results; instead,
compute previewCandidateCommandIDs from commandPaletteVisibleResults whenever
that array is non-empty (i.e. prefer visible results over fingerprint match),
and only fall back to [] when there are no visible results; update the block
around commandPaletteVisibleResultsScope/commandPaletteVisibleResultsFingerprint
so PreviewCandidateCommandIDs is derived from
Self.commandPalettePreviewCandidateCommandIDs(resultIDs:
commandPaletteVisibleResults.map(\.id), limit:
Self.commandPaletteVisiblePreviewCandidateLimit) whenever
commandPaletteVisibleResults.count > 0, keeping
scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh:) behavior
untouched.
---
Outside diff comments:
In `@Sources/CommandPalette/CommandPaletteSearch.swift`:
- Around line 1160-1233: The full sort of scoredEntries even when resultLimit is
small causes unnecessary O(n log n) work; replace the bounded path (when
resultLimit != nil -> outputCount) with a fixed-size min-heap (or selection
algorithm) that keeps the top N ScoredEntry items by the existing comparator
(score desc, rank asc, title localizedCaseInsensitive asc) during the scan,
while preserving the existing cancellation checks (shouldCancelSearch) and using
the same scoring (weightedScore + historyBoost); after the scan, extract the
heap into an array and sort it once with the same comparator before building the
results (CommandPaletteSearchCorpusResult), and keep the current full Array.sort
behavior for the unbounded path so semantics and tie‑breakers (ScoredEntry,
scoredEntries, resultLimit/outputCount, shouldCancelSearch) are unchanged.
- Around line 137-1276: Extract the entire CommandPaletteFuzzyMatcher enum
(including nested types WordSegment, ASCIIScalarMask, PreparedToken,
PreparedCandidateText, PreparedQuery, SingleEditWordPrefixMatch,
SingleEditWordPrefixEditKind and all private helpers) into a new sibling source
file (e.g., CommandPaletteFuzzyMatcher.swift), preserving all declarations,
access levels, and static APIs exactly as-is so callers like
CommandPaletteSearchEngine, CommandPaletteSearchCorpusEntry, and
CommandPaletteSearchCorpusResult keep working; remove the original enum from the
current file so the remaining file contains only the corpus/engine types, ensure
the new file is compiled in the same target (no API changes), and run the build
to fix any visibility/import issues.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dee379b0-0e86-403c-9ad5-13a8f300c14e
📒 Files selected for processing (3)
Sources/CommandPalette/CommandPaletteSearch.swiftSources/ContentView.swiftcmuxTests/CommandPaletteSearchEngineTests.swift
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Sources/ContentView.swift (1)
5301-5303:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon’t apply an empty switcher preview over existing results.
When the scope/fingerprint no longer matches,
previewCandidateCommandIDsbecomes empty. The switcher preview path then resolves to[], and Lines 5483-5491 still replace the visible list with that empty preview while the full search is pending, causing a transient blank palette during rapid query changes.Suggested fix
let previewCandidateCommandIDs: [String] if commandPaletteVisibleResultsScope == scope, commandPaletteVisibleResultsFingerprint == fingerprint { previewCandidateCommandIDs = Self.commandPalettePreviewCandidateCommandIDs( resultIDs: commandPaletteVisibleResults.map(\.id), limit: Self.commandPaletteVisiblePreviewCandidateLimit ) } else { previewCandidateCommandIDs = [] } + let shouldApplyPreviewResults = scope == .commands || !previewCandidateCommandIDs.isEmpty isCommandPaletteSearchPending = true commandPaletteSearchTask = Task.detached(priority: .userInitiated) { - let previewMatches = Self.commandPalettePreviewSearchMatches( - scope: scope, - searchCorpus: searchCorpus, - candidateCommandIDs: previewCandidateCommandIDs, - searchCorpusByID: searchCorpusByID, - query: matchingQuery, - usageHistory: usageHistory, - queryIsEmpty: queryIsEmpty, - historyTimestamp: historyTimestamp, - resultLimit: visiblePreviewResultLimit - ) + let previewMatches = shouldApplyPreviewResults + ? Self.commandPalettePreviewSearchMatches( + scope: scope, + searchCorpus: searchCorpus, + candidateCommandIDs: previewCandidateCommandIDs, + searchCorpusByID: searchCorpusByID, + query: matchingQuery, + usageHistory: usageHistory, + queryIsEmpty: queryIsEmpty, + historyTimestamp: historyTimestamp, + resultLimit: visiblePreviewResultLimit + ) + : [] guard !Task.isCancelled else { return } await MainActor.run { + guard shouldApplyPreviewResults else { return } let currentScope = Self.commandPaletteListScope(for: commandPaletteQuery) let currentMatchingQuery = Self.commandPaletteQueryForMatching( query: commandPaletteQuery, scope: currentScope )Based on learnings: do not modify the sync-seeding behavior of
scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh:); keep fixes confined to the preview path.Also applies to: 5440-5491
🤖 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 `@Sources/ContentView.swift` around lines 5301 - 5303, The switcher preview path currently returns [] when previewCandidateCommandIDs is empty, which then replaces the visible list with a blank preview during rapid queries; change the preview logic so that if previewCandidateCommandIDs.isEmpty you do not apply/return an empty preview (e.g., return nil or a sentinel to indicate “no preview”) and leave the existing visible results intact instead of replacing them. Locate the code that builds/applies the switcher preview (references to previewCandidateCommandIDs and the preview application logic around the switcher preview path) and add a guard that skips replacing current results when the preview is empty; do not modify scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh:).
🤖 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 `@Sources/ContentView.swift`:
- Around line 616-671: The Command Palette render types and model were added
directly into ContentView.swift and must be extracted into their own files; move
the CommandPaletteRenderTrailingLabelStyle, CommandPaletteRenderTrailingLabel,
CommandPaletteRenderResultRow, CommandPaletteCommandListRenderState, and the
CommandPaletteOverlayRenderModel declarations (and the
CommandPaletteCommandListRowsView) into new dedicated Swift files under Sources
(e.g., CommandPaletteRenderTypes.swift and
CommandPaletteOverlayRenderModel.swift) keeping their exact type names and
access levels, update any imports/visibility so ContentView still references
them, remove their definitions from ContentView.swift, and ensure the module
compiles by fixing any fileprivate/internal references and updating usages of
CommandPaletteCommandListRenderState.empty and scheduleCommandListUpdate(_:).
- Around line 3918-3963: The scrollPosition is using state.scrollTargetIndex
(Int?) but rows are keyed by row.id (String), so scrolling-by-index fails to
resolve targets; change the scroll target to use the row IDs instead: introduce
or use a String? like state.scrollTargetId and make the Binding passed to
scrollPosition(id:) return state.scrollTargetId (set should ignore passive reads
as before), update any selection-follow logic that currently writes
state.scrollTargetIndex (e.g., where selectedIndex is changed) to write the
corresponding row.id (state.rows[selectedIndex].id) instead, and keep the
ForEach keyed by row.id so scrollPosition targets match the actual row IDs.
---
Duplicate comments:
In `@Sources/ContentView.swift`:
- Around line 5301-5303: The switcher preview path currently returns [] when
previewCandidateCommandIDs is empty, which then replaces the visible list with a
blank preview during rapid queries; change the preview logic so that if
previewCandidateCommandIDs.isEmpty you do not apply/return an empty preview
(e.g., return nil or a sentinel to indicate “no preview”) and leave the existing
visible results intact instead of replacing them. Locate the code that
builds/applies the switcher preview (references to previewCandidateCommandIDs
and the preview application logic around the switcher preview path) and add a
guard that skips replacing current results when the preview is empty; do not
modify scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh:).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7643c0b1-e18b-45e2-bb69-55124c53a373
📒 Files selected for processing (2)
Sources/ContentView.swiftcmuxTests/CommandPaletteSearchEngineTests.swift
* feat: improve markdown viewer
* fix: address markdown review feedback
* fix: clean up markdown review issues
* fix: address markdown review feedback
* fix: address latest markdown review
* Fix #3807: bring notification CLI to panel parity (#3811)
* Prove notification CLI parity is missing
Add behavior-level coverage for the missing notification socket and CLI actions before implementing them. The tests drive V2 notification action methods, CLI subcommands, extended list fields, and the UI open-notification flow so CI can show the pre-fix gap.
Constraint: Regression tests must be committed before the implementation for issue #3807
Constraint: Local Swift tests are not run in this repo; verification is through CI
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Not-tested: Swift/XCUITest execution; intentionally deferred to CI
* Make notification actions scriptable from the CLI
The notifications panel already owned the behavior for dismissing, marking read, opening, and jumping to unread rows. This wires the socket and CLI to those existing store/AppDelegate paths, extends list output with panel metadata, and documents the new command surface without introducing a second notification action model.
Constraint: Existing Notifications page behavior must remain the source of truth
Constraint: Direct xcodebuild and local Swift/XCUITest runs are forbidden in this workspace
Rejected: Duplicate CLI-side focus or read-state logic | would diverge from AppDelegate.openNotification and TerminalNotificationStore semantics
Confidence: medium
Scope-risk: moderate
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Tested: python3 -m json.tool Resources/Localizable.xcstrings
Not-tested: Swift unit/UI execution; deferred to CI per repo policy
* Make notification CI compile under strict concurrency
The notification parity implementation introduced a shared formatter on a main-actor type and a test helper that crossed actor boundaries through escaping closures. This keeps the socket behavior unchanged while making date formatting local to the main-actor call path and keeping the XCTest socket request helper from capturing actor-isolated state in the background request closure.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Run xcodebuild locally to confirm | user explicitly warned direct xcodebuild can deadlock the machine
Confidence: medium
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make notification parity actions atomic and exact
Review feedback exposed two behavior risks in the notification parity path: bulk dismissal was implemented as client-side list-and-loop work, and jump-to-unread could report one unread notification while opening a later valid one. The server now owns already-read dismissal as a single V2 action, the jump helper returns the notification it actually opened, and event/list parsing details match those server semantics.
Constraint: Notification action logic must reuse the existing store and AppDelegate paths
Rejected: Split TerminalController into new controllers in this PR | broad refactor is unrelated to issue #3807 and would obscure the parity fix
Confidence: medium
Scope-risk: moderate
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace instructions
* Expose app test symbols to CLI notification coverage
The integration regression exercises real app-backed notification state through the CLI harness, so the test target must import the built app module the same way the socket action tests do.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with xcodebuild locally | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make notification list and dismiss payloads stable
Review caught two small contract hazards in the new notification RPC surface: the dismiss result used mixed JSON types, and a pipe in the appended tab title could shift the legacy list parser. The server now reports dismissals as counts consistently and escapes only the new trailing list field, with the CLI decoding it after structural parsing.
Constraint: The V1 list response remains pipe-delimited for backward compatibility, so only newly appended trailing fields can be encoded without changing old parser behavior
Rejected: Replace list_notifications with JSON-only output | existing V1 parsers depend on the line format
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Return async notification socket responses from tests
CircleCI caught a compile-only issue in the async V2 test helper: the continuation result was awaited but not returned from the method that promises a response dictionary. Returning the continuation value restores the helper contract without changing the exercised socket behavior.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with local xcodebuild | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Avoid blocking notification CLI integration sockets
The integration regression has to create AppKit-backed notification state on the main actor, but running the CLI subprocess synchronously there can block the socket handler's main-actor store mutations. The test now awaits subprocess work on a background queue and verifies read state through the same CLI list path.
Constraint: Socket notification handlers intentionally hop to the main actor for store and AppDelegate work
Rejected: Run the CLI synchronously from the main actor | it can deadlock the in-process socket server during tests
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Harden notification CLI response contracts
The notification parity path now distinguishes new list-notification trailer fields from old pipe-heavy bodies, rejects surface-only mark-read selectors, and returns post-open read state by marking through the shared store path after a successful focus action.
Constraint: list_notifications remains a legacy pipe-delimited protocol, so the new trailer needs its own discriminator while older body parsing keeps joining payload[6...]
Rejected: Add a third list_notifications sentinel field | the issue asked for exactly the two appended fields
Rejected: Rely on AppDelegate delayed mark-read for socket JSON | CLI callers need the open response and subsequent list output to reflect the explicit action
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI per workspace policy
* Make CLI integration helper capture explicit self
CircleCI's Swift compile step requires explicit self when the background subprocess closure calls the test helper method. This keeps the deadlock fix intact while satisfying Swift capture rules.
Constraint: Local Swift tests and direct xcodebuild are explicitly forbidden in this workspace
Rejected: Verify with local xcodebuild | user explicitly forbids direct xcodebuild
Confidence: high
Scope-risk: narrow
Tested: git diff --check
Tested: jq empty Resources/Localizable.xcstrings
Not-tested: Swift compilation and XCTest execution; deferred to CI
* Make CLI presentation flags order-independent
Users naturally put presentation flags next to the command they are inspecting. Normalize --json and --id-format after command selection so docs examples such as cmux list-notifications --json produce JSON while still preserving literal flag-looking values for command options.
Constraint: Do not run reload.sh before CI is green; never run bare xcodebuild.
Rejected: Documentation-only correction | leaves copy-pasted command behavior surprising.
Rejected: Notification-only --json handling | repeats the same parser split for future JSON-capable commands.
Confidence: medium
Scope-risk: moderate
Directive: Keep presentation flag parsing centralized; update commandOptionsWithValues when adding value-taking command options.
Tested: git diff --check
Not-tested: Local XCTest/build per repo policy; CI will verify.
* Fix notification surface selector error message
* Make notification open mark read synchronously
* Fix CLI presentation flag parsing
* Test notification mark-read missing id
* Reject missing notification mark-read ids
* fix: polish markdown viewer dogfood
* Fix shared WebView task manager attribution
Fixes shared WebContent resource attribution in task manager rows and adds regression coverage.
* Prevent display-link crash from terminal portal layout reentry (#3885)
* Pin portal sync deferral during SwiftUI host callbacks
The silent-exit crash path points at SwiftUI/AppKit layout recursion reaching a CATransaction display-link flush. This test locks the intended invariant: geometry callbacks originating from the SwiftUI NSViewRepresentable host must defer portal reconciliation instead of forcing immediate AppKit layout, even while an interactive resize is active.
Constraint: Local tests are intentionally not run in this repository; CI owns regression proof.\nRejected: Assert on source text or unified-log contents | timing-dependent and not executable through the policy seam.\nConfidence: medium\nScope-risk: narrow\nDirective: Keep immediate portal flushing owned by external AppKit resize observers, not SwiftUI host callbacks.\nTested: Not run locally per repository policy and user instruction.\nNot-tested: Full multi-session crash reproduction is timing-dependent and not deterministic.
* Prevent portal layout reentry from terminal host callbacks
The terminal NSViewRepresentable host was allowed to bind into the window portal and synchronously flush AppKit layout from update/layout callbacks. That made SwiftUI's own host layout and the external terminal portal both believe they could drive geometry in the same render turn, matching the NSHostingView reentrant-layout warnings reported before the CATransaction display-link abort.\n\nThe portal now treats SwiftUI host callbacks as state capture only: they register the binding and schedule the portal owner to reconcile geometry after the current render turn. Immediate geometry flushing remains available to the portal's external AppKit resize observers, which are outside SwiftUI body/layout evaluation. The launch path also records clean exits and posts a one-time TerminalNotificationStore breadcrumb when a newer ghostty Breakpad envelope is found after an unclean exit.
Constraint: Local tests and app builds are not run before CI for this branch; verification is delegated to CI, then the required tagged reload.\nRejected: Wrap the CATransaction/display-link exception in @try/@catch | it would hide AppKit's abort symptom without removing the reentrant layout owner split.\nRejected: Keep immediate sync during interactive SwiftUI host callbacks | still allows layoutSubtreeIfNeeded inside the representable layout/update path.\nConfidence: medium\nScope-risk: moderate\nDirective: SwiftUI/AppKit host callbacks must not force terminal portal layout synchronously; add external observer paths for any future immediate resize flushing.\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local XCTest/build/repro per repository and user instructions; 75-minute multi-session crash reproduction is not deterministic.
* Record clean-exit breadcrumb after teardown
The crash breadcrumb should compare Breakpad envelopes against the last successfully completed termination path. Recording the timestamp after teardown avoids marking an exit clean before session persistence, process cleanup, and notification cleanup have finished.
Constraint: This is a follow-up correctness tweak before CI settled.\nRejected: Keep the timestamp at the start of applicationWillTerminate | a crash during termination cleanup could suppress the next-launch breadcrumb.\nConfidence: high\nScope-risk: narrow\nDirective: Only update the clean-exit timestamp after teardown work that must complete for a clean quit.\nTested: git diff --check; jq empty Resources/Localizable.xcstrings\nNot-tested: Local tests/builds per repository and user instructions.
* Defer first portal install from SwiftUI callbacks
A deferred SwiftUI host bind could still create the WindowTerminalPortal for the first time, and the initializer previously installed the host with an immediate layout flush. Threading the same deferral flag through portal creation keeps the invariant complete: representable update/layout callbacks never synchronously flush terminal portal layout, even on first bind.
Constraint: Must preserve immediate install behavior for non-SwiftUI external portal callers
Rejected: Assume portals already exist before host callbacks | first terminal bind in a new window can create one
Confidence: high
Scope-risk: narrow
Directive: Any future portal creation path from SwiftUI callbacks must carry deferred synchronization through initialization
Tested: git diff --check; jq empty Resources/Localizable.xcstrings
Not-tested: Local tests/builds per repository and user instructions
* Move crash breadcrumb work out of launch hot path
The crash breadcrumb scanner is pure Foundation logic, so it now lives in its own source file and performs the crash-directory scan from a detached utility task. App launch only schedules the check once, then posts the existing localized notification after the background scan returns to the main actor. The terminal host geometry policy is also simplified to make the always-deferred invariant explicit at the call site.
Constraint: Local build/tests are intentionally skipped until CI completes per issue instructions
Rejected: Keep synchronous directory enumeration in AppDelegate.configure | startup should not block on crash artifact I/O
Rejected: Preserve the dead immediate portal sync branch | host callbacks are never allowed to force layout synchronously
Confidence: high
Scope-risk: narrow
Directive: Do not reintroduce synchronous crash-directory scans or immediate portal layout flushes from SwiftUI host callbacks
Tested: git diff --check; jq empty Resources/Localizable.xcstrings; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local build/tests per repository and user instructions
* Tighten portal geometry regression seam
* Fix crash breadcrumb actor isolation
* Mark crash breadcrumb async helper nonisolated
* Make crash breadcrumb scan concurrent
* fix: remove inert crash breadcrumb cooldown key
* fix: use renamed crash breadcrumb annotation
* fix: own crash breadcrumb scan task
* Hide sidebar descriptions in title-only mode (#4040)
* Hide sidebar descriptions in title-only mode
* Add sidebar description visibility toggle
* Let sidebar titles use trailing slack
* Float sidebar shortcut hints over rows
* Remove unused sidebar width helper
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add Pi agent icon
PR: https://github.com/manaflow-ai/cmux/pull/4057
* Keep Claude Running after clear (#3631)
* Prove Claude clear hook loses running status
The regression exercises the Claude hook handler directly with a SessionStart payload whose source is clear, then sends a late Stop for the previous session. The current handler does not promote the clear boundary to Running or reject stale Stop mutations, so this test is expected to fail before the lifecycle fix.
Constraint: Tests must exercise the hook handler directly rather than driving the full app.
Confidence: high
Scope-risk: narrow
Directive: Keep this regression on the direct CLI hook path; do not replace it with source-text assertions.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Prove Claude clear should own running status
Add hook-level regression coverage for Claude Code /clear. The tests drive the bundled CLI against a mock cmux socket so CI proves SessionStart(source=clear) must set the visible Running status and a prior session Stop must not clobber that fresh lifecycle.
Constraint: Local tests are intentionally not run; CI owns test execution for this task.
Confidence: high
Scope-risk: narrow
Tested: Not run locally per instruction
Not-tested: Full app UI repro under CI
* Ignore stale Claude hook events after clear
Claude /clear starts a fresh hook lifecycle but the sidebar status key is shared at the workspace level. Persist the active Claude session per workspace, promote clear SessionStart to a visible Running state, and ignore teardown or notification mutations from sessions that are no longer current.
Constraint: Claude Code emits /clear as SessionStart(source=clear).
Rejected: Only set Running on clear SessionStart | late Stop from the previous lifecycle could still clobber the new status.
Confidence: high
Scope-risk: narrow
Directive: Future Claude hook status mutations must respect the active workspace session before touching claude_code.
Tested: Not run locally per instruction
Not-tested: Full app UI after CI
* Prove Claude clear hook loses running status
The regression exercises the Claude hook handler directly with a SessionStart payload whose source is clear, then sends a late Stop for the previous session. The current handler does not promote the clear boundary to Running or reject stale Stop mutations, so this test is expected to fail before the lifecycle fix.
Constraint: Tests must exercise the hook handler directly rather than driving the full app.
Confidence: high
Scope-risk: narrow
Directive: Keep this regression on the direct CLI hook path; do not replace it with source-text assertions.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Keep Claude clear sessions authoritative
Claude /clear arrives as a SessionStart source=clear event, but the hook store only remembered routing data. This change makes the store also own the active session for each workspace, promotes clear starts to Running, and ignores visible Stop/Notification/SessionEnd mutations when their session no longer matches the active workspace session.
Constraint: Claude Code documents SessionStart source=clear for /clear lifecycle boundaries.
Rejected: Only set Running on source=clear | old Stop and SessionEnd events could still clobber the new session afterward.
Confidence: high
Scope-risk: narrow
Directive: Visible Claude hook mutations must pass active-session ownership checks before changing sidebar status or notifications.
Tested: Not run locally per repository policy.
Not-tested: CI red/green proof pending on GitHub Actions.
* Allow Claude sessions to advance turns
A visible hook mutation can only be rejected as stale after the active session boundary is known. Keeping a completed turn id active after Stop made the next prompt in the same Claude session look stale before it could promote its own turn.
The Stop path now refreshes the active session with no turn id once the completed turn has been accepted, preserving stale-session protection across /clear while allowing normal multi-turn prompts to re-enter Running. The regression now exercises that two-turn path before the clear boundary.
Constraint: Do not run local tests; CI owns verification for this branch.
Rejected: Ignore turn id in all current-session checks | would weaken stale turn filtering for late Stop and Notification events.
Confidence: high
Scope-risk: narrow
Directive: Do not persist a completed turn id past Stop without proving the next prompt-submit can promote a new turn.
Tested: git diff --check
Not-tested: Local unit/regression tests per project policy
* Remove duplicate Claude clear helper
The branch integration accidentally kept two isClaudeClearSessionStart definitions in CLI/cmux.swift. The duplicate version referenced a non-existent parsedInput.source property, so Swift would reject the file before CI could exercise the hook lifecycle tests.
Keep the existing implementation that reads source from the parsed hook object and delete only the duplicate helper.
Constraint: Fix review-reported compile blocker without changing lifecycle behavior.
Rejected: Add source to ClaudeHookParsedInput | unnecessary for this compile fix and broader than the failing duplicate.
Confidence: high
Scope-risk: narrow
Directive: Keep exactly one Claude clear source helper unless the parsed input model is intentionally extended.
Tested: git diff --check; rg confirms a single isClaudeClearSessionStart definition and no parsedInput.source reference.
Not-tested: Local tests per project policy
* Restore Claude hook compileability after lifecycle merge
The active-session merge left behind the old source-property helper alongside the newer compact-payload helper. Removing the duplicate keeps source=clear detection on the JSON payload and avoids both the redeclaration and missing-property errors.
Constraint: Do not run local tests; CI owns verification for this branch
Rejected: Reintroduce ClaudeHookParsedInput.source | duplicates state already retained in the compact hook payload
Confidence: high
Scope-risk: narrow
Tested: git diff --check; rg verified a single isClaudeClearSessionStart definition and no parsedInput.source references
Not-tested: Local compile/test execution per task policy
* Normalize Claude active-session cleanup keys
The active-session map is keyed by normalized workspace id, so cleanup after consuming a session must use the same normalized key. Otherwise a record containing incidental whitespace could leave a stale active-session entry behind.
Constraint: Address reviewer-reported lifecycle cleanup edge case without changing visible hook behavior.
Rejected: Store raw workspace ids as active map keys | inconsistent with existing upsert normalization and lookup guards.
Confidence: high
Scope-risk: narrow
Directive: Any activeSessionsByWorkspace lookup should use the normalized workspace key, matching insertion.
Tested: git diff --check
Not-tested: Local tests per project policy
* Harden CI Zig downloads against transient upstream failures
CircleCI and the activation workflow both depend on ziglang.org tarballs during remote macOS setup. A transient 500 from that host failed the debug build before any project code compiled, so the install path now retries downloads and avoids unnecessary Homebrew update/cleanup churn on CircleCI.
Constraint: CI must be made green remotely without running local tests or local xcodebuild.
Rejected: Push an empty commit to rerun CI | would leave the same upstream download flake unchanged.
Confidence: high
Scope-risk: narrow
Directive: Keep Zig installer retries in remote CI setup paths; failures here happen before project build logic runs.
Tested: git diff --check
Not-tested: Local tests and local builds not run per repository/user policy.
* Cover stale Claude session-end lifecycle
Greptile identified that stale SessionEnd exercises a different clear-state path than stale Stop. The existing active-session guard already blocks the mutation, so the regression now drives that hook directly and asserts the active /clear session keeps its status, PID, and notifications intact.
Constraint: Do not run local tests; CI is the verification source for this branch.
Rejected: Add another Swift integration test | the existing Python hook harness already executes the CLI handler through the socket path used by CI.
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repo policy.
Not-tested: Local test execution.
* Stop activation CI from stalling on Zig downloads
The activation workflow was cancelled before build because repeated ziglang.org transfers crawled for the full job timeout. Prefer Homebrew's pinned zig@0.15 bottle on macOS runners, then keep the direct tarball path as a bounded fallback with connection, total-time, and minimum-speed limits.
Constraint: The failed check never reached project build or tests; the only failing surface was CI tool bootstrap.
Rejected: Increase the job timeout | it would hide the bootstrap failure and delay feedback.
Confidence: medium
Scope-risk: narrow
Directive: Keep activation workflow tool bootstrap bounded so performance CI reaches the benchmark or fails quickly.
Tested: git diff --check
Not-tested: Local workflow execution, per repository and user instruction.
* Gate Claude session-end cleanup on the consumed workspace
Greptile noted that session-end computed the current-session guard from the fallback workspace while clearing the consumed session's workspace. Move the guard next to the mutation target so stale cleanup and visible cleanup always evaluate the same workspace identity.
Constraint: This is follow-up review hardening on the existing active-session model.
Rejected: Collapse activeSessionsByWorkspace into sessions in this PR | per-workspace current-session lookup is the intended lifecycle boundary for stale event gating.
Confidence: high
Scope-risk: narrow
Directive: SessionEnd visible cleanup must be gated against the workspace being cleared, not an earlier fallback lookup.
Tested: git diff --check
Not-tested: Local test execution, per repository and user instruction.
* Gate Claude session-end cleanup on consumed workspace
A late SessionEnd can be resolved through fallback surface lookup, so the workspace used to find a record is not always the workspace whose visible state would be cleared. Move the staleness check after consume() and evaluate it against the consumed session's workspace, where the clear_status and notification cleanup actually run.
Constraint: Hook events are delivered by short-lived CLI processes and must tolerate stale or partial Claude payloads
Rejected: Keep the pre-consume fallback workspace guard | it can validate one workspace while clearing another
Confidence: high
Scope-risk: narrow
Tested: Added Swift integration regression for stale SessionEnd fallback cleanup
Not-tested: Local tests not run per repository policy
* Make stale Claude session-end test consume the seeded session
Greptile caught that the Swift regression used an unknown session id, so the hook returned before reaching the intended consumed-session visibility guard. Use the seeded stale session id in the SessionEnd payload so the test exercises consume(), then verifies that stale visible cleanup is blocked.
Constraint: Address high-priority review feedback without running local tests.
Rejected: Leave Python-only coverage | the Swift regression would continue passing for the wrong reason.
Confidence: high
Scope-risk: narrow
Directive: Stale session-end regressions should consume a known stale session before asserting visible cleanup is skipped.
Tested: git diff --check
Not-tested: Local tests per repository and user instruction
* Cover fallback Claude session-end consumption
The stale SessionEnd regression should prove the handler consumed the stale session through fallback lookup before deciding whether visible state may be cleared. Use an unknown late session id and assert the seeded stale session is removed from the store, so the test cannot pass without exercising the consumed-workspace guard.
Constraint: Review feedback flagged the prior Swift regression as able to pass without covering the intended guard
Rejected: Use the stored stale session id directly | that only covers the ordinary mapped-session stale path
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repository policy; CI will run the Swift regression
Not-tested: Local XCTest execution
* Keep Claude clear ownership panel-scoped
The clear SessionStart path now owns the active Claude boundary only for explicit clear events, so late startup/resume SessionStart events from the previous session cannot reclaim the workspace before their stale Stop or SessionEnd arrives. The same clear path now passes the resolved surface id into status updates so split panels receive the Running state in the intended pane.
Constraint: Local test execution is disallowed by repository policy; CI remains the behavioral gate.
Rejected: Let every SessionStart mark active | late non-clear events can overwrite the clear boundary.
Rejected: Drop fail-open isCurrent behavior | transient store errors should not hide legitimate current status updates.
Confidence: medium
Scope-risk: moderate
Directive: Do not let non-clear Claude SessionStart events replace an explicit /clear active boundary without adding event ordering metadata.
Tested: python3 -m py_compile tests/test_claude_hook_clear_running_status.py; git diff --check; conflict marker scan.
Not-tested: Local Swift/unit/UI test execution per repo policy.
* Protect Claude clear state from stale session cleanup
Late hook events can arrive after an explicit /clear boundary. The SessionStart handler now skips PID registration when the event is stale against the active session, and SessionEnd consumption preserves the active session when the incoming turn id is older than the stored active turn.
Constraint: Local test execution is disallowed by repository policy; CI remains the behavioral gate.
Rejected: Clear active ownership before checking currentness | same-session stale SessionEnd would fail open and apply cleanup.
Rejected: Keep prefix-only command assertions | option ordering changes would make the regression test brittle.
Confidence: medium
Scope-risk: moderate
Directive: Do not consume a Claude session or replace its PID from a hook event that is stale relative to active session/turn state.
Tested: python3 -m py_compile tests/test_claude_hook_clear_running_status.py; git diff --check; conflict marker scan.
Not-tested: Local Swift/unit/UI test execution per repo policy.
* Allow new Claude sessions to replace stopped owners
A stopped Claude turn must keep the same session eligible for the next turn, but it must not strand the workspace if that process exits before session-end. Mark stopped ownership as replaceable and let only session activation paths use that escape hatch; clear-session ownership remains non-replaceable so stale pre-clear events stay blocked. Session-end fallback cleanup now checks the consumed session id so missing input session ids do not fail open.\n\nConstraint: /clear SessionStart must continue to suppress stale pre-clear startup, stop, and session-end events\nRejected: Clear active ownership on Stop | breaks same-session multi-turn prompt-submit currentness\nConfidence: medium\nScope-risk: narrow\nTested: git diff --check; conflict-marker scan\nNot-tested: local XCTest per repo policy; CI will run cmux unit regressions
* test: cover codex hooks TOML features section
* fix: harden hook config and auth token cache
* test: cover sign-out browser auth loading cleanup
* fix: cancel browser auth on sign-out
* test: cover codex config read failures
* fix: fail closed on codex config read errors
* test: cover auth sign-out callback race
* fix: discard auth callback after sign-out
* fix: keep claude subcommands out of hook injection
* fix: dismiss stale sparkle update replies
* fix: redact auth logs while preserving observability
* fix: reset update checks and guard sign-out races
* fix: preserve update metadata from driver state
* fix: await auth token assertions before comparing
* fix: address wrapper and auth review feedback
* fix: address session and config review feedback
* ci: retrigger pending checks
* fix: clear stale ime and auth token state
* fix: clean legacy codex hooks config
* test: cover legacy codex hooks markers
* fix: clean empty codex features table
* fix: preserve browser key reentry dispatch
* Fix new-workspace caller window routing (#4042)
* test: cover new workspace caller routing
* fix: route new workspace to caller context
* Add iMessage workspace insertion regression test
* Reset Kitty keyboard mode at shell prompt boundaries (#3870)
* Prove stale Kitty keyboard state leaks CSI-u key bytes
The regression exercises a hosted Ghostty terminal, leaves Kitty keyboard protocol enabled as a crashed TUI would, mirrors the clear-history socket handler clear_screen path, and captures raw PTY stdin bytes for a plain c key. Current behavior should encode CSI-u instead of a single ASCII byte, making the test fail until the protocol state is reset at the shell prompt boundary.
Constraint: Regression must cover PTY input bytes, not source text shape
Confidence: high
Scope-risk: narrow
Directive: Keep this test on the real ghostty_surface_key path; sendText alone bypasses the keyboard encoder
Tested: Manual current-main repro captured c9;1:3uc9;1:3uc9;1:3u after Kitty enable plus clear-history
Not-tested: Local unit execution deferred to CI per requested red/green workflow
* Reset stale Kitty keyboard mode at prompt boundaries
A crashed TUI can leave Ghostty's Kitty keyboard protocol stack pushed after clear-history, causing normal shell input to be encoded as CSI-u. Resetting the stack from the shell prompt hook makes the prompt boundary the ownership point for returning interactive shells to plain byte input.
Constraint: Fix must run through shell integration because Ghostty keeps protocol state inside the terminal surface
Rejected: Reset only in clear-history socket path | stale state also leaks after any crashed TUI returns to prompt
Confidence: high
Scope-risk: narrow
Directive: Keep prompt-boundary reset paired across bash and zsh integrations
Tested: Not run locally per repository testing policy; regression added in prior commit
Not-tested: CI not yet completed
* Clarify Kitty reset fixture failures
The stale keyboard regression now fails at the fixture boundary if the zsh integration does not emit the expected reset bytes, instead of falling through to a misleading PTY byte mismatch.
Constraint: Repository policy forbids local test execution for this PR loop
Confidence: high
Scope-risk: narrow
Tested: Not run locally per repository policy; CI will run the affected XCTest
Not-tested: Local XCTest execution
* chore: retrigger Vercel checks
* Add Kitty reset shell hook coverage
* Reset all terminal keyboard protocols at prompt
* Fix terminal keyboard reset test expectations
* Clarify disabling agent session auto-resume for #3640 (#3991)
* docs: explain disabling agent auto resume
* docs: name auto resume config path
* Fix stale restored agent resume state
* Harden restored agent hook liveness
* refactor: share restored agent normalization
* Honor iMessage workspace ordering and previews
* Add workspace cwd inheritance setting (#3921)
* feat: add workspace cwd inheritance setting
* fix: align workspace cwd setting key naming
* fix: apply workspace cwd setting to detached creation
* fix: expose workspace cwd inheritance setting
* fix: route pane break through detached workspace creation
* fix: keep pane break response pane ids non-null
* fix: distinguish pane break resolution errors
* Approve installed Codex hooks (#4035)
* Approve installed Codex hooks
* Address Codex hook trust review feedback
* Avoid tomllib in Codex hook tests
* Align Codex hook trust test mirror
* Harden Codex hook trust ownership
* Preserve legacy Codex hook cleanup
* Fix workspace unit test after merge
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Revert "Approve installed Codex hooks (#4035)" (#4074)
This reverts commit e4546b7675a4ffa5a41a5429218b60f4e7644e21.
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Fix workspace unit test transfer resume state (#4076)
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Clarify in cmux --help that reload-config covers Ghostty config too (#4060)
* Clarify cmux help that reload-config covers Ghostty config too
`cmux reload-config` reloads BOTH ~/.config/cmux/cmux.json and Ghostty
config (~/.config/ghostty/config) and refreshes terminals in place, but
the help/docs only mentioned cmux.json. Agents (and humans) reading the
help would think they had to restart cmux after editing Ghostty config.
- cmux --help "Agent Help" now tells agents where Ghostty config lives
and that reload-config picks it up live.
- cmux docs settings, cmux settings path, cmux config --help now list
~/.config/ghostty/config as a related (not cmux-owned) location and
describe reload-config's actual scope.
- cmux schema sidebarAppearance.tintOpacity description now notes it's
sidebar-only, and points to Ghostty background-opacity / blur for
terminal transparency.
- skills/cmux/SKILL.md mirrors the wording.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Align ghostty_config JSON shape across CLI surfaces
Cursor Bugbot and Greptile both flagged that `cmux settings path --json`
emitted `ghostty_config` as a string while `cmux docs settings --json`
emitted it as an object {path, note}. An agent reading both outputs with
the same key expectation would have to special-case the type.
Standardize on the object shape with `path` and `note` in both, matching
docsPayload. This is the agent-discoverability path the PR is trying to
make reliable, so making the shape consistent is on-purpose.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Use canonical Ghostty config key background-blur (not background-blur-radius)
CodeRabbit flagged that `background-blur-radius` is outdated. Verified
against the Ghostty submodule at ghostty/src/config/Config.zig: line 70
declares `background-blur-radius` as a compatibilityRenamed alias for
`background-blur`. The current canonical key is `background-blur` and it
accepts the same integer value (e.g. `background-blur = 20`).
Update the two docs that introduced the alias name:
- skills/cmux/SKILL.md
- web/data/cmux.schema.json (sidebarAppearance.tintOpacity description)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add nucleo FFI command palette benchmark
* Open right sidebar tools as panes (#4065)
* Open right sidebar tools as panes
* Remove unreachable sidebar pane commands
* Fix sidebar pane unit test build
* Address sidebar pane review feedback
* Fix vault pane focus tracking
* Address right sidebar pane review followups
* Use modern sidebar pane flash observer
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Save crash diagnostics under cmux state (#4077)
* Save crash diagnostics under cmux state
* fix: key GhosttyKit artifacts by crash path
* fix: pin cmux crash GhosttyKit archive
* fix: mark crash breadcrumb scan concurrent
* fix: keep crash scan compatible with Xcode 16
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Menubar global search P1 (#3908)
* Ship local global search as a remappable menubar flow
Phase 1 needs browser and markdown value without terminal scrollback, so the index owns durable FTS5 upserts while AppDelegate keeps one navigation path from palette rows to focused panels. The global shortcut is wired through the existing shortcut settings instead of a standalone Carbon shim so Settings and cmux.json remain authoritative.
Constraint: Phase 1 excludes Ghostty terminal scrollback capture
Constraint: User required no local test execution and no reload before CI is green
Rejected: Hardcoded GlobalSearchHotkey shim | violates KeyboardShortcutSettings policy
Confidence: medium
Scope-risk: broad
Directive: Keep future terminal capture feeding SearchIndex documents through GlobalSearchCoordinator rather than adding another palette/navigation path
Tested: git diff --check; python3 -m json.tool Resources/Localizable.xcstrings; python3 -m json.tool web/data/cmux.schema.json; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local unit/UI tests and tagged app launch per task constraints
* Remove duplicate search refresh work
Cursor review pointed out that the menubar toggle and palette onAppear both refreshed the live index. Keeping the refresh in the palette lifecycle avoids resetting browser debounce tasks while still indexing each time the palette opens, and removing the unused workspace delete API keeps the storage surface honest.
Constraint: Review feedback came from PR #3908 after the first CI pass started
Rejected: Keep both refresh calls | causes avoidable debounce cancellation and slower browser result availability
Confidence: high
Scope-risk: narrow
Directive: Add workspace-level deletion only when a real workspace teardown caller is wired
Tested: git diff --check; python3 -m json.tool Resources/Localizable.xcstrings; python3 -m json.tool web/data/cmux.schema.json; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj
Not-tested: Local tests/build per task constraints
* Fix global search stale index entries
* Address global search review feedback
* Fix search query token mapping compile error
* Fix global search CI failures
* Fix stale unavailable markdown search entries
* Cancel stale markdown search captures
* Cancel global search refresh on dismiss
* Fix global search review followups
* Address global search post-CI feedback
* Preserve markdown panel title search on read failure
* Address global search lifecycle feedback
* Address global search review lifecycle feedback
* Refine global search capture ownership
* fix: address global search review blockers
* feat: show open panels in global search
* fix: cover right sidebar tool panel in search
* Fix cmuxTests: rename restorableAgentAutoResumePending to restorableAgentResumeState (#4068)
* fix: handle repeated assistant imessage completions
* Open supported files in cmux on cmd-click (#4041)
* Open supported files in cmux on cmd-click
* Add cmd-click file preview verification script
* Reuse right pane for cmd-click file previews
* Keep cmd-click UI test terminal after preview focus
* Accept numeric cmd-click test payload values
* Capture cmd-click UI test window snapshots
* Add file-type-aware external open actions
* Address supported file routing review feedback
* Fix external open menu sendability warnings
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Suppress native proxy icon on cmux main windows (#3973)
* Add proxy icon regression test
* Suppress native proxy icon on cmux windows
* chore: retrigger preview deployments
* Refine native proxy icon suppression
* fix: keep titlebar folder icon aligned
* fix: restore titlebar folder icon leading offset
* test: cover folder icon frame replacement
* fix: resync folder icon on frame replacement
* test: cover folder icon ancestor movement
* fix: track folder icon ancestor movement
* fix: address folder icon review feedback
* test: stabilize folder icon ancestor sync
* fix: handle sidebar tool panels in global search
---------
Co-authored-by: austinpower1258 <austinwang115@gmail.com>
* Close browser panels when pages request window close (#4070)
* Add browser self-close restore regression test
* Close browser panels from WebKit close callbacks
* fix: index right sidebar tool panels as titles
* fix: keep web close callback synchronous
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add notification policy hooks
Merged https://github.com/manaflow-ai/cmux/pull/4054
* Fix sidebar unread badge after re-marking notifications (#4084)
* Add sidebar unread notification regression test
* Keep sidebar notification badge live during menu freeze
* Cover sidebar presentation fallback cases
* Limit Cloud VMs by active provider state (#4046)
* test: cover active vm limit with paused freestyle vms
* fix: enforce cloud vm limits by active state
* fix: parallelize cloud vm status refresh
* chore: update web security dependencies
* fix: handle right sidebar tools in global search
* fix: guard cloud vm status refresh races
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Fix Settings search synonyms (#4082)
* Add settings search synonym regressions
* Fix settings search synonyms
* Add shortcut bindings anchor regression
* Cover clickable PR settings search alias
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add tagged debug CLI helper (#4092)
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add unread defer shortcut (#4086)
* Add unread defer shortcut
* fix: address unread defer feedback
* fix: keep manual unread jump explicit
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Document notification hooks default off
Document that notification hooks are off by default and style the config key in localized docs.
* Approve installed Codex hooks after dogfood (#4075)
* Reapply Codex hook approval changes for dogfood
* Notify on Codex plan input requests
* Handle Codex plan question transcript items
* Address Codex hook review feedback
* Recover malformed Codex hook trust blocks
* Avoid duplicate consecutive cmux hook reinserts
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Document session restore behavior
Adds session restore docs, blog, README updates, and review cleanup.
* Use nucleo for command palette search
* Skip unrestorable Claude startup sessions
PR: https://github.com/manaflow-ai/cmux/pull/4079
* Revert "Suppress native proxy icon on cmux main windows" (#4099)
* Revert "Suppress native proxy icon on cmux main windows (#3973)"
This reverts commit 5048440ff44b4edbe328b65403724ff79476b2e6.
* Fix titlebar proxy icon without detached panel sync
* Apply proxy icon override to fallback config
* Restore key regain redraw invariant
* Tune nucleo palette initialism ranking
* Fix terminal portal resize lag (#4102)
* Fix Korean 2-Set terminal arrows (#4095)
* Add Korean 2-Set arrow IME regression
* Restore Korean 2-Set arrow forwarding
* Restore Zhuyin IME command routing
* Address IME review feedback
* Fix Korean IME regression test compile
* Address IME review follow-ups
* Address Bopomofo preedit review feedback
* Avoid idle Zhuyin key suppression
* Remove dead text input wrapper
* Address IME suppression review feedback
* Fix palette stitched highlight precedence
* Add Codex Teams subagent panes
* Open markdown files in preview panels from cmux open (#4085)
* fix: open markdown files in preview panels
* fix: preserve markdown viewer transparency
* fix: mute markdown open-with header button
* fix: align markdown header controls
* fix: align file header controls
* fix: address markdown review feedback
* test: cover opaque text editor alpha
* fix: align header open fallback
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Add task manager sorting and program aggregates (#4066)
* Add task manager sorting and program aggregates
* Add sorting to cmux top output
* Add flat TSV cmux top output
* Add coding agent task manager totals
* Make task manager program totals payload-backed
* Recognize agent launcher process names in task manager
* Fix task manager test build helpers
* Recognize Claude versioned launcher processes
* Show loading state before task manager sample
* Recognize versioned agent process names
* Use agent totals for task manager hierarchy icons
* fix: address task manager review feedback
* fix: address follow-up task manager review
* fix: address final task manager review
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Keep manual unread sticky until terminal interaction (#4104)
* test: cover sticky manual unread state
* fix: keep manual unread sticky until terminal input
* fix: show workspace manual unread pane ring
* fix: sync manual unread badge on focus changes
* fix: stabilize manual unread representative fallback
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: austinpower1258 <austinwang115@gmail.com>
* Bump version to 0.64.5 (#4107)
* Fix Pi Vault icon and JSONL titles (#4120)
* test: cover Pi JSONL content block titles
* fix: parse Pi JSONL text blocks for Vault titles
* fix: align Pi JSONL title role handling
* fix: require typed text blocks for Pi titles
* Improve Cloud VM error guidance (#4094)
* Improve Cloud VM error guidance
* Address final Cloud VM review feedback
* Narrow Cloud VM sanitizer env var block
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Keep selected workspace visible after sidebar reorders
* Add sidebar scroll regression for workspace move to top
* Reveal selected workspace after sidebar reorders
* Handle right sidebar tool panels in global search
* Test workspace move to top visibility only
* Refine sidebar reorder scroll trigger
* Add move-to-top notification regression
* Skip no-op move-to-top notifications
* Assert no-op move-to-top keeps order
* Require matching manager for reorder scroll
* Scroll selected workspace on index shifts
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Use transparent backgrounds for file preview panels (#4088)
* Handle right sidebar tools in global search browse hits
* Use transparent backgrounds for file preview panels
* Cover panel theme background preservation
* Fix PDF file preview open menu
* Make file open menu button compact
* Fix PDF open menu chrome button
* Fix PDF open-with chrome click target
* Address file preview chrome review feedback
* Fix PDF background cache invalidation
---------
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
* Inherit stdin in backgrounded ssh inside startup wrapper (#4135)
* Add regression test for SSH startup wrapper dropping stdin
After PR #3786 backgrounded the ssh invocation inside the startup
wrapper for signal/reconnect handling, `cmux ssh <host>` sessions
stopped forwarding keystrokes from the surface PTY to the remote
shell. Output flowed back (the remote prompt rendered in cmux UI),
but anything typed never reached zsh on the other side, which sat
blocked in `do_poll` on the remote pts.
Root cause: POSIX sh redirects stdin of an async command (`&`) to
/dev/null when job control is off — the default for the `/bin/sh -c …`
that runs the startup wrapper. Without an explicit `<&0` on the
`&`'d ssh line, the local PTY stdin is silently dropped.
This commit adds the failing regression test. The fix follows in
the next commit so CI can prove the test catches the bug.
* Inherit stdin in backgrounded ssh inside startup wrapper
PR #3786 wrapped the ssh invocation in `while :; do … & wait` to enable
SIGHUP/INT/TERM trap handling and reconnect-on-exit-255. Backgrounding
ssh, however, drops its stdin: POSIX sh redirects fd 0 of any async
command to /dev/null when job control is off, which is the default for
the `/bin/sh -c …` host that runs this wrapper.
As a result, output from the remote still flowed back to the surface
PTY (ssh's stdout/stderr stayed wired) but the user's keystrokes never
reached the remote — they hit ttysNNN on the Mac side, kernel echoed
them locally, but ssh's stdin was /dev/null so nothing was forwarded.
zsh on the far side sat in `do_poll` forever.
Explicit `<&0` on both the `command` line and the `( … )` shell-snippet
form overrides the POSIX default and re-attaches the wrapper's own
stdin to the backgrounded ssh process.
Verified with the regression test added in the previous commit, plus
manual repro on Darwin:
/bin/sh -c 'cat & wait' # cat's fd 0 → /dev/null (bug)
/bin/sh -c 'cat <&0 & wait' # cat's fd 0 → parent stdin (fix)
* Add docs search
Adds localized Pagefind docs search with heading anchors and section-aware results.
* Fix Swift interpolation escape in SSH stdin regression test (#4154)
`testSSHStartupForwardsStdinToBackgroundedSSH` used `\"<empty>\"` inside a
`\(...)` string interpolation. Swift parses `\"` as the end of the outer
literal, breaking compilation of cmuxTests on main:
Cannot find ')' to match opening '(' in string interpolation
Unterminated string literal
That kept `ci/circleci: macos-unit-tests` red on main after #4135 even
though `macos-debug-build` and `macos-release-build` still passed (the
test target was the only thing affected). The runtime fix (`<&0` on the
backgrounded ssh) is unchanged and was verified end-to-end on a cloud Mac.
Inside an interpolation, the string is already a Swift expression and
literal quotes are unescaped; drop the four backslashes.
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add command palette settings toggles
Adds command palette toggles for boolean Settings rows, including iMessage Mode.
* Ensure Rust toolchain is installed for nucleo FFI builds
* Document nucleo FFI thread and ABI assumptions
* Reuse nucleo index in preview search test helper
* Install Rust for activation perf builds
---------
Co-authored-by: Tobi Lutke <tobi@shopify.com>
Co-authored-by: Austin Wang <austinwang115@gmail.com>
Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Kevin Peng <48529172+kays0x@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@cmuxTests/CommandPaletteNucleoFFITests.swift`:
- Around line 1-929: The test file exceeds the 800-line guideline; extract the
bulky helper types and fixtures into separate test-support files: move the
dlopen wrappers NucleoLibrary and NucleoIndex into a new support file (e.g.,
CommandPaletteNucleoFFIHelpers) and move fixture builders makeOpenFolderEntries,
makeInitialismWorkspaceEntries, makeEdgeCasePaletteEntries, and
makeLargeWorkspaceSwitcherEntries (and their helpers like searchCorpus,
optimizedResults overloads if needed) into another support file (e.g.,
CommandPaletteTestFixtures); update the test class to import/use those types
(NucleoLibrary, NucleoIndex, and fixture functions) so the
CommandPaletteNucleoFFITests.swift file falls under 800 lines while preserving
existing public/internal visibility and test behavior.
In `@Sources/CommandPalette/CommandPaletteNucleoSearch.swift`:
- Around line 263-283: The boosts construction is needlessly using map { …
}.flatMap { $0.isEmpty ? nil : $0 }; replace it with a straightforward local
computation: iterate over entries to build values and track hasNonZeroBoost
(same loop body currently inside the map), then set boosts to hasNonZeroBoost ?
values : nil. Update the variable named boosts (used when calling
library.search(index: pointer, query: query, ...)) so it is either nil or the
[Int32] array without the extra map/flatMap indirection.
- Around line 302-311: Replace the direct conversion
Int(rawMatch.score.rounded()) with a defensive clamping initializer so
FFI-provided NaN/Inf/out-of-range values cannot trap; update the construction of
CommandPaletteNucleoSearchResult where score is set (the expression using
rawMatch.score) to use a clamped Int conversion (e.g., Int(clamping:
rawMatch.score.rounded())) consistent with other uses like Int32(clamping:), so
the score safely bounds to Int.
In `@Sources/ContentView.swift`:
- Around line 616-671: Move the command-palette helper types and model into a
new Swift source file (e.g.,
Sources/CommandPalette/CommandPaletteOverlay.swift): extract
CommandPaletteRenderTrailingLabelStyle, CommandPaletteRenderTrailingLabel,
CommandPaletteRenderResultRow, CommandPaletteCommandListRenderState, and
CommandPaletteOverlayRenderModel (and the CommandPaletteCommandListRowsView
referenced in the review) out of ContentView.swift and paste them into the new
file, preserving the `@MainActor` and ObservableObject annotations and all
implementations; remove the originals from ContentView.swift. Update access
levels from file-private/private to internal (or adjust to the narrower
appropriate visibility) so ContentView and other consumers can still reference
them, and add any necessary imports at the top of the new file. Finally, run a
build to fix any visibility or symbol-reference issues and update any
tests/imports that referenced the old file-scoped symbols.
- Around line 5498-5508: After updating the preview scroll target, force a
resync so the overlay sees the new scroll anchor: after calling
updateCommandPaletteScrollTarget(resultCount:previewResults.count,
animated:false) in the preview branch, bump the commandPaletteResultsRevision
(e.g. increment commandPaletteResultsRevision or call the helper used elsewhere
to update that revision) before calling
syncCommandPaletteDebugStateForObservedWindow() so the published
commandPaletteOverlayRenderModel snapshot includes the updated scroll target.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5bcec795-b71a-4bdd-b5e5-cb344dbb5c87
⛔ Files ignored due to path filters (1)
Native/CommandPaletteNucleoFFI/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.circleci/config.yml.github/workflows/ci-macos-compat.yml.github/workflows/ci.yml.github/workflows/nightly.yml.github/workflows/perf-activation.yml.github/workflows/release.ymlGhosttyTabs.xcodeproj/project.pbxprojNative/CommandPaletteNucleoFFI/.gitignoreNative/CommandPaletteNucleoFFI/Cargo.tomlNative/CommandPaletteNucleoFFI/src/lib.rsSources/CommandPalette/CommandPaletteNucleoSearch.swiftSources/CommandPalette/CommandPaletteSearch.swiftSources/ContentView.swiftcmuxTests/CommandPaletteNucleoFFITests.swiftcmuxTests/CommandPaletteSearchEngineTests.swiftcmuxTests/WorkspaceManualUnreadTests.swiftscripts/build-command-palette-nucleo-ffi.shscripts/install-rust-ci.shscripts/test-command-palette-nucleo-ffi.sh
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/release.yml">
<violation number="1" location=".github/workflows/release.yml:123">
P2: Checking only `command -v rustup` is insufficient; ensure an active/default toolchain exists before `rustup target add` to avoid release job failures on uninitialized rustup installs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bc097d1. Configure here.
| span_count: usize, | ||
| ) -> *mut CmuxNucleoIndex { | ||
| if blob_ptr.is_null() || spans_ptr.is_null() { | ||
| return std::ptr::null_mut(); |
There was a problem hiding this comment.
Empty index creation rejected incorrectly
Low Severity
cmux_nucleo_index_create returns null when blob_ptr or spans_ptr is null, even when blob_len or span_count is zero. Swift buffer pointers are null for empty arrays, so creating CommandPaletteNucleoSearchIndex with an empty corpus fails unexpectedly and silently falls back away from native behavior.
Reviewed by Cursor Bugbot for commit bc097d1. Configure here.
This was referenced Jul 8, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Verification:
Notes:
Note
Medium Risk
Adds a Rust FFI dylib to the macOS build and rewires command-palette searching and rendering, which could affect ranking/selection behavior and CI/build reliability across architectures.
Overview
Command Palette search is reworked for performance and UI stability. A new Rust
nucleo-backed search index is added viaNative/CommandPaletteNucleoFFIand dynamically loaded from Swift (CommandPaletteNucleoSearch.swift), with Swift fallback when the dylib isn’t available.The Swift matcher/search pipeline is optimized by precomputing per-entry normalized text, character/word-segment data, and ASCII masks, plus adding optional
resultLimitand more efficient top-N selection inCommandPaletteSearchEngine/CommandPaletteFuzzyMatcher. Search orchestration is centralized inCommandPaletteSearchOrchestrator, including preview-vs-final result handling, history/extra boosts, and a targeted Swift single-edit fallback when Rust results likely miss typo matches.The command palette overlay UI is decoupled into
CommandPaletteOverlaywith a render model that schedules list updates byresultsVersion, replacing inline row rendering/hover state inContentViewand improving scroll/selection syncing. CI/build workflows now install Rust (CircleCI + GitHub Actions) and the Xcode project gains a build phase to compile/sign a universallibcmux_command_palette_nucleo_ffi.dylib; release workflow also ensuresrustuptargets. Separately, notification clearing now also clears panel-unread badges for affected tabs when marking all read/clearing all.Reviewed by Cursor Bugbot for commit bc097d1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Switches Cmd‑P search to a Rust
nucleobackend via a small FFI layer, keeps a shared in‑memory index, and makes typing and previews much faster. Indexing now runs off the main actor and small cold searches stay synchronous for smoother input.New Features
nucleodylib is missing; CI/release/perf builds install Rust, with scripts to build/test the FFI.Bug Fixes
nucleosemantics aligned with Swift: normalized highlight precedence (including stitched highlights), alias authority, and a narrowed typo‑tolerance fallback; preserve normalized matches.Written for commit bc097d1. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Refactor
New Features
Tests
Chores