Backfill Cmd+K command palette to native-menu parity (Phase 1) (#2679)#672
Merged
Conversation
* docs(open-knowledge): spec command palette / menu parity registry Add a spec for a single command registry as the source of truth for command identity, so every native-menu action is reachable from the Cmd+K palette and a conformance ratchet keeps the two surfaces from drifting. Internal spec only (specs/ is not mirrored). * docs(open-knowledge): revise cmdk parity spec after adversarial review Fold in an independent 3-reviewer adversarial pass. Corrections: remove a fabricated command (toggle-linter); use the real 32-member OkMenuAction union and real menu labels; add the APP_RESERVED bucket for leaf-less ids and bespoke leaves; fix the inventory (report-bug is PRESENT, add the three extra view toggles); re-scope so Phase 1 is backfill + a conformance ratchet (registry/lingui-in-core/bus-refactor demoted to deferred Phase 2, which cannot compile msg in core); fix the web dispatch story; add acceptance criteria; define the 7-to-4 target projection. * docs(open-knowledge): fold keyboard-shortcut parity into cmdk spec Keyboard shortcuts are the fourth fragmented id-space for the same commands, and menu.ts hand-types accelerators without importing the shortcut registry, so menu-vs-shortcut accelerator drift is live and unguarded. Add a staged plan (13.1): Level 0 in Phase 1 (palette rows show accelerators via formatShortcut; Ratchet D guards accelerator parity), Levels 1-2 in Phase 2. Adds Ratchet D and D9; refines Q5/Q6. * docs(open-knowledge): add agent (MCP-tool) surface as Phase 3 to cmdk spec Fifth command surface: MCP tools in packages/server/src/mcp/tools (registry in index.ts). Distinct external contract, cannot render from the command registry, but overlapping mutations already share the api-extension HTTP handler spine (move->rename-path, write->agent-write-md) guarded by attribution-sweep. Integrate via a capability-parity map + ratchet keyed on those handlers (exposure parity, not implementation). Phase 3 / parallel track, gated on a product decision (Q10). Adds 13.2, D10, Q10, a 3 non-goal. * docs(open-knowledge): add menu de-dup + settings-to-agents to cmdk spec Menu de-dup (13.3): "Check for updates..." is emitted twice and shows on macOS in both the App menu and Help menu (App-menu leaf is isMac-gated, Help-menu leaf is unconditional). Phase 1 fix = gate Help on !isMac (mirror the Settings platform-XOR); Phase 2 = single-declaration placement + Ratchet B flags undeclared same-platform multi-placement. Settings-to-agents (13.4): toggle-shaped settings (theme, spell-check) are commands and ride the registry + 13.2 agent parity; general config-as-data is a sibling config-contract (guarded config.set beyond the read-only config MCP tool), not this registry. Adds D11, D12, Q11. * feat(open-knowledge): menu-action bus, view-menu-state store, two bridge invokes, updates menu de-dup Foundational layer for Cmd+K / menu parity (Phase 1): - local-menu-action-bus: renderer-local fan-out with a ref-counted single bridge.onMenuAction forwarder so both surfaces converge on one dispatch path. - view-menu-state-store: renderer mirror of the push-only View-menu snapshot so the palette can render state-reflecting toggle labels and gate kill-terminal. - Export OkMenuAction + OkEditorViewMenuStateSnapshot for the bus/store/ratchet. - Two new bridge invokes: mcpWiring.reconfigure() and spellcheck.toggle(), each delegating to the existing main-side body (reconfigureMcpWiringNow extracted). - Gate the Help 'Check for updates' entry on !isMac so macOS shows it once (App menu), mirroring the Settings platform-XOR. * refactor(open-knowledge): migrate the nine menu-action subscribers onto the local bus All nine bridge.onMenuAction subscribers (FileSidebar, EditorArea, EditorPane, TerminalSessionsHost, ProjectSwitcher, CreateProjectMenuTrigger, ReportBugMenuTrigger, NavigatorApp, DocumentContext) now attach to subscribeLocalMenuAction instead. The bus's ref-counted forwarder becomes the only bridge.onMenuAction listener, so a real menu click fires each handler once (no double-fire) and host-agnostic commands (view/panel/tree toggles) are now reachable on the web host. The four view-menu-state producers also mirror their snapshot into the renderer store so the palette can read it. Drop the now-unused bridge prop from ReportBugMenuTrigger. * feat(open-knowledge): backfill the Cmd+K command / menu parity gap Adds the ~24 §14 ADD commands to the Cmd+K palette, each dispatched through the smallest existing seam: the local menu-action bus for id-backed commands (view/tree toggles, terminal, worktree, contextual file ops), a bridge invoke for the three main-side commands (check for updates, set up integrations, spell check), and a direct renderer call for GitHub. Rows are search-only so the empty-open state is unchanged; view toggles show state-reflecting labels from the renderer view-menu-state store; contextual rows gate on the active editor target via the 7-to-4 projection; rows with a shortcut show the accelerator. Shared labels route through MENU_LABELS so the palette and native menu read identically, guarded by the extended label-parity test. * test(open-knowledge): drive migrated menu-action subscribers via the bus Updates the per-subscriber DOM tests to fire actions through emitLocalMenuAction (the same fan-out a real menu click reaches via the bus forwarder) instead of a captured bridge.onMenuAction callback. Also hardens the bus forwarder to guard a truthy-but-thin window.okDesktop (partial stub or session-only host) that lacks onMenuAction, so it never throws. * test(open-knowledge): command/menu parity ratchet + acceptance coverage Installs the durable 'nothing is missed' guarantee (Ratchets A-D) plus the Phase-1 acceptance criteria: - Ratchet A: OK_MENU_ACTIONS runtime array with a compile-time drift guard against the OkMenuAction union. - Ratchet B: every OkMenuAction id and every buildMenuTemplate leaf (both platform branches) is classified palette-command / OS-role / app-reserved; untracked is empty. - Ratchet C + AC1/AC3/AC5-AC8: each backfilled palette row renders under its enabling context and fires its declared dispatch (bus emit / bridge invoke / external URL); web-parity subset, state-reflecting toggle labels, contextual gating, and search-only empty-open. - Ratchet D: menu accelerators agree with the keyboard-shortcut registry. - FR3/AC2: a source scan asserts the bus forwarder is the only bridge.onMenuAction listener (no subscriber double-fires). - Bus unit tests for subscribe/emit/unsubscribe. * chore(open-knowledge): commit i18n .po catalogs + satisfy knip - Add the extracted en/pseudo .po source catalogs for the backfill strings. - Ratchet derives the menu-item type from buildMenuTemplate (drop the direct electron import the app package does not depend on). - Un-export the view-menu-state store's internal get/subscribe (used only via useViewMenuState). * chore(open-knowledge): relocate parity ratchet to unit tier + comment discipline Move the command/menu parity ratchet from tests/integration (which mandates markdown @covers-* tags that do not apply to a menu ratchet) to src/lib so it runs in the unit tier. Strip acceptance-criterion / functional-requirement id tokens from comments (they live in the PR body); the test titles keep them. * fix(open-knowledge): keep bridge mirror + IPC ratchets in sync with new invokes - Mirror mcpWiring.reconfigure() + spellcheck.toggle() into the core OkDesktopBridge copy so the 3-way drift catcher (M1 smoke) stays green. - Bump the RequestChannels count ratchet cap 83 -> 85 for the two new command invoke channels. - Update the menu unit test to assert the de-duplicated Check-for-updates (App-menu only on macOS, not also in Help). - Land the comment-discipline fixes on the parity tests. * test(open-knowledge): real-cmdk e2e smoke for the palette backfill Closes the DOM tests' mocked-cmdk fidelity gap: drives the real palette on the web host — a backfilled toggle is search-only on empty open, renders under a query with a state-reflecting label, and activating it round-trips through the bus to FileSidebar (the re-queried label flips). Registered in the CI e2e list. * chore(open-knowledge): changeset for the Cmd+K command / menu parity backfill * fix(open-knowledge): converge Cmd+K parity review findings - Import afterEach in CommandPalette.dom.test so the backfill suite registers (a missing import silently skipped ~29 tests). - Wire Ratchet C completeness: share the palette-command id classification via a test-helper and cross-check it against the rendered id-backed rows, so a classified-but-unrendered command turns the DOM suite red. - Gate Expand all / Collapse all palette rows on canExpandAll/canCollapseAll, matching the native menu's smart-hide. - Add bus tests for single-inbound dispatch, ref-counted forwarder teardown, and the partial-bridge guard. - Harden the double-fire guard to a repo-wide scan asserting a single production onMenuAction call site. - Cover doc-panel/terminal toggle labels and kill-terminal gating; make the asset-target gating test deterministic. - Correct the emitLocalMenuAction JSDoc and the mcp-wiring channel comment; strip spec-section citations from source comments. * chore(open-knowledge): raise main bundle budget for Cmd+K backfill The Cmd+K menu-parity backfill adds ~24 palette commands and their icons to the eagerly-loaded CommandPalette, growing the main index bundle by ~1.3 kB gzipped (477.35 kB). Raise the main-bundle size-limit 476 to 480 kB — a proportionate bump for real feature surface, consistent with prior budget raises as the app grew. The other two budgets (all-chunks 3.31/3.35 MB, CSS 54.57/55 kB) are unchanged and still green. * fix(open-knowledge): isolate menu bus subscriber faults Converge the claude review on the Cmd+K parity PR: - Wrap each local-menu-action-bus subscriber dispatch in try/catch with a console breadcrumb, plus a behavioral test, so one throwing handler cannot starve later subscribers (native menu clicks and palette commands share this single dispatch path). - Refresh the ipc-channels.ts header count narrative to 85 in lock-step with the ratchet test, appending the 83 to 85 bump rationale. - Add a unit suite for the view-menu-state store covering the partial-merge contract, subscriber notification, snapshot stability, and the test reset. - Document why EditorPane's store mirror deliberately sits behind both restore gates, unlike the unconditional sibling mirrors. * chore(open-knowledge): apply re-review suggestions on Cmd+K parity - Share the repository URL as OPEN_KNOWLEDGE_GITHUB_URL in core so the native Help menu and the palette command cannot drift (the parity ratchets check labels, not URLs). - Cover the openExternalUrl window.open fallback on the web host. - Cover the folder target kind in the AC7 contextual gating test. * test(open-knowledge): make ProblemsPanel lingui macro mocks race-proof Both lingui macro specifiers alias to one shim module in the vitest dom config, so this file's two mock.module registrations raced for a single resolved module id and only one factory survived. The specifier-shaped split (t on core/macro, components on react/macro) lost useLingui when the core factory won, failing all 14 tests. Both registrations now return the same superset object so either race winner carries every export. The race is latent (15 sibling dom test files share the pattern); this branch's regenerated lingui catalogs shifted registration timing enough to flip this one file. Full dom tier verified locally: 218 files, 2064 tests green. * test(open-knowledge): widen ReportBugDialog lazy-mount wait for CI The renderDialog helper waits for the lazy-loaded dialog body with findByRole's 1s default timeout. The file's first render pays the chunk's cold transform and import cost, which can exceed that deadline on a contended CI runner: the dom tier failed twice in a row on exactly this wait (empty body, no chunk-load error in the log) while passing locally and on the two prior CI runs. A generous explicit deadline only lengthens the failure path. GitOrigin-RevId: 3c124b0df9e7cfd46a6b1d0034ca651c3666f26e
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.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.