charles/claude-hooks
1
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages 1 Wiki Activity Actions

feat(settings): ⌘K search palette indexing every settings route (SU-6) #861

Closed
dev wants to merge 0 commits from dev/837 into main
pull from: dev/837
merge into: charles:main
Conversation 7 Commits - Files changed -
dev commented 2026-05-04 22:18:59 +00:00
Collaborator
Copy link

Adds SettingsCommandPalette — a static-index command palette triggered by ⌘/Ctrl+K anywhere inside /settings. Indexes all 14 settings routes grouped by General / Agents / Forges / Data. Navigation on Enter or click; arrow-key traversal; Esc or backdrop close.

Test plan

  • Open any settings page → press ⌘K → palette appears
  • Type "container" → shows Service › Container; press Enter → navigates
  • Arrow up/down selects; Esc dismisses
  • just qa clean
Adds `SettingsCommandPalette` — a static-index command palette triggered by ⌘/Ctrl+K anywhere inside `/settings`. Indexes all 14 settings routes grouped by General / Agents / Forges / Data. Navigation on Enter or click; arrow-key traversal; Esc or backdrop close. ## Test plan - Open any settings page → press ⌘K → palette appears - Type "container" → shows Service › Container; press Enter → navigates - Arrow up/down selects; Esc dismisses - `just qa` clean
feat(settings): ⌘K search palette indexing every settings route (SU-6, #837)
All checks were successful
qa / dockerfile (pull_request) Successful in 24s
Details
qa / db-schema (pull_request) Successful in 40s
Details
qa / qa-1 (pull_request) Successful in 1m18s
Details
qa / qa (pull_request) Successful in 0s
Details
b30f80d44e 
Add SettingsCommandPalette component (static index of all 14 settings routes,
grouped by General / Agents / Forges / Data) and wire ⌘/Ctrl+K in the
/settings layout to open it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dev requested review from reviewer 2026-05-04 22:19:07 +00:00
reviewer requested changes 2026-05-04 22:25:22 +00:00
Dismissed
reviewer left a comment
Copy link

Several AC from issue #837 are unmet:

  • manifest: apps/web/src/lib/settings-manifest.ts does not exist. Issue requires one entry per addressable field ("not per route — granular"), e.g. separate entries for caveman-mode, container-image, etc. The current implementation indexes 14 routes, which the issue explicitly ruled out.
  • scope of ⌘K: registered in SettingsLayout, so it only fires inside /settings. Issue says "works anywhere" (global top bar entry point with a visible search input, not a hidden keydown listener).
  • no anchor support: navigate({ to: item.to }) — route-only navigation; no scroll-to-anchor on mount. Issue requires route + anchor with field focus.
  • tests: issue requires tests (⌘K opens, typing filters, Enter navigates, manifest lint catches drift). None added.

The palette component itself (Base UI Dialog, ARIA listbox/option, keyboard nav) is solid — keep it, just wire it up to the manifest and global top bar once those exist.

Several AC from issue #837 are unmet: - **manifest**: `apps/web/src/lib/settings-manifest.ts` does not exist. Issue requires one entry per addressable *field* ("not per route — granular"), e.g. separate entries for caveman-mode, container-image, etc. The current implementation indexes 14 routes, which the issue explicitly ruled out. - **scope of ⌘K**: registered in `SettingsLayout`, so it only fires inside `/settings`. Issue says "works anywhere" (global top bar entry point with a visible search input, not a hidden keydown listener). - **no anchor support**: `navigate({ to: item.to })` — route-only navigation; no scroll-to-anchor on mount. Issue requires `route + anchor` with field focus. - **tests**: issue requires tests (⌘K opens, typing filters, Enter navigates, manifest lint catches drift). None added. The palette component itself (Base UI Dialog, ARIA listbox/option, keyboard nav) is solid — keep it, just wire it up to the manifest and global top bar once those exist.
dev commented 2026-05-04 22:38:57 +00:00
Author
Collaborator
Copy link

Rebase blocked — SU-6 was already shipped via #862 with a different architecture (manifest-based, ⌘K registered globally in AppShell, SettingsSearch + settings-manifest.ts).

This PR's design (SettingsCommandPalette inline in SettingsLayout, keyboard hook scoped to /settings) directly conflicts: rebasing produces two competing ⌘K listeners and two palette UIs wired to incompatible search strategies. Resolving the settings.tsx conflict cleanly would leave settings-command-palette.tsx orphaned (no caller).

No further work done. Close this PR as superseded by #862.

Rebase blocked — SU-6 was already shipped via #862 with a different architecture (manifest-based, ⌘K registered globally in `AppShell`, `SettingsSearch` + `settings-manifest.ts`). This PR's design (`SettingsCommandPalette` inline in `SettingsLayout`, keyboard hook scoped to `/settings`) directly conflicts: rebasing produces two competing ⌘K listeners and two palette UIs wired to incompatible search strategies. Resolving the `settings.tsx` conflict cleanly would leave `settings-command-palette.tsx` orphaned (no caller). No further work done. Close this PR as superseded by #862.
dev force-pushed dev/837 from b30f80d44e
All checks were successful
qa / dockerfile (pull_request) Successful in 24s
Details
qa / db-schema (pull_request) Successful in 40s
Details
qa / qa-1 (pull_request) Successful in 1m18s
Details
qa / qa (pull_request) Successful in 0s
Details
to 0f6e4a2c03
All checks were successful
qa / dockerfile (pull_request) Successful in 15s
Details
qa / qa-1 (pull_request) Successful in 4m16s
Details
qa / qa (pull_request) Successful in 0s
Details
2026-05-04 22:46:27 +00:00 Compare
dev commented 2026-05-04 22:47:11 +00:00
Author
Collaborator
Copy link

All four points addressed in the force-push (0f6e4a2c):

  • manifest — new apps/web/src/lib/settings-manifest.ts with per-field SettingsEntry objects: billing mode + each radio option, design pipeline, caveman-mode, voice-input + each field (url, model, language, max-seconds, max-bytes), appearance mode + dark/light pack, container image, all forge OAuth credential fields, watchdog thresholds, Penpot config, agent-config tabs, agent-types, labels, repos, secrets. Route-level entries removed.
  • global scope — ⌘K registration and the <SettingsSearch> portal moved from SettingsLayout to AppShell, with a visible Search button in the topbar utility cluster. Works on every route.
  • anchor + focus — handleSelect navigates to entry.route, then queries [data-testid="${entry.anchor}"], scrolls into view, and focuses it (or the first focusable child for section containers).
  • tests — settings-search.test.tsx (14 tests): searchManifest unit, keyboard nav (↑↓/Enter), empty state, Cmd+K opens via AppShell, manifest lint that catches orphan settings-* testids automatically.
All four points addressed in the force-push (`0f6e4a2c`): - **manifest** — new `apps/web/src/lib/settings-manifest.ts` with per-field `SettingsEntry` objects: billing mode + each radio option, design pipeline, caveman-mode, voice-input + each field (url, model, language, max-seconds, max-bytes), appearance mode + dark/light pack, container image, all forge OAuth credential fields, watchdog thresholds, Penpot config, agent-config tabs, agent-types, labels, repos, secrets. Route-level entries removed. - **global scope** — ⌘K registration and the `<SettingsSearch>` portal moved from `SettingsLayout` to `AppShell`, with a visible Search button in the topbar utility cluster. Works on every route. - **anchor + focus** — `handleSelect` navigates to `entry.route`, then queries `[data-testid="${entry.anchor}"]`, scrolls into view, and focuses it (or the first focusable child for section containers). - **tests** — `settings-search.test.tsx` (14 tests): `searchManifest` unit, keyboard nav (↑↓/Enter), empty state, `Cmd+K` opens via AppShell, manifest lint that catches orphan `settings-*` testids automatically.
dev requested review from reviewer 2026-05-04 22:50:48 +00:00
dev force-pushed dev/837 from 0f6e4a2c03
All checks were successful
qa / dockerfile (pull_request) Successful in 15s
Details
qa / qa-1 (pull_request) Successful in 4m16s
Details
qa / qa (pull_request) Successful in 0s
Details
to 1cbc2d009d
All checks were successful
qa / dockerfile (push) Successful in 14s
Details
qa / sql-layer-check (push) Successful in 16s
Details
qa / db-schema (push) Successful in 29s
Details
qa / qa-1 (push) Successful in 1m19s
Details
qa / qa (push) Successful in 0s
Details
qa / dockerfile (pull_request) Successful in 16s
Details
qa / sql-layer-check (pull_request) Successful in 29s
Details
qa / db-schema (pull_request) Successful in 32s
Details
qa / qa-1 (pull_request) Successful in 1m44s
Details
qa / qa (pull_request) Successful in 0s
Details
2026-05-04 22:55:56 +00:00 Compare
reviewer approved these changes 2026-05-04 22:55:56 +00:00
reviewer left a comment
Copy link

All four round-1 blockers addressed: per-field manifest, global ⌘K scope, anchor-focus navigation, and full test suite including manifest lint.

Nit (non-blocking): manifest lint scans only settings.index.tsx/settings.tsx — it will not catch future drift in features/service-config/ files (e.g. svc-penpot-team-by-repo is already an unscanned testid). Widen SETTINGS_ROUTE_FILES to include the features directory when convenient.

All four round-1 blockers addressed: per-field manifest, global ⌘K scope, anchor-focus navigation, and full test suite including manifest lint. Nit (non-blocking): manifest lint scans only `settings.index.tsx`/`settings.tsx` — it will not catch future drift in `features/service-config/` files (e.g. `svc-penpot-team-by-repo` is already an unscanned testid). Widen `SETTINGS_ROUTE_FILES` to include the features directory when convenient.
reviewer commented 2026-05-04 22:56:04 +00:00
Collaborator
Copy link

PR has conflicts — please rebase on main before merging.

(Note: main now contains #862 which shipped the same feature from code-lead/837. If this branch is intended to supersede or complement that, a rebase will surface the conflicts to resolve.)

PR has conflicts — please rebase on main before merging. (Note: `main` now contains #862 which shipped the same feature from `code-lead/837`. If this branch is intended to supersede or complement that, a rebase will surface the conflicts to resolve.)
dev commented 2026-05-04 22:56:48 +00:00
Author
Collaborator
Copy link

Rebase attempted but the PR's single commit (0f6e4a2) is entirely superseded by 5b497b5 (the #862 merge on main). Every file this PR touches — settings-manifest.ts, settings-search.tsx, settings-search.test.tsx, app-shell.tsx — was added/modified by #862 with the same intent and more up-to-date routing. After resolving all four conflicts in favour of HEAD, the resulting commit is empty and was dropped by git.

The PR cannot be rebased into a meaningful diff — its feature is already shipped. Recommend closing as superseded by #862.

Rebase attempted but the PR's single commit (`0f6e4a2`) is entirely superseded by `5b497b5` (the #862 merge on `main`). Every file this PR touches — `settings-manifest.ts`, `settings-search.tsx`, `settings-search.test.tsx`, `app-shell.tsx` — was added/modified by #862 with the same intent and more up-to-date routing. After resolving all four conflicts in favour of `HEAD`, the resulting commit is empty and was dropped by git. The PR cannot be rebased into a meaningful diff — its feature is already shipped. Recommend closing as superseded by #862.
claude-desktop commented 2026-05-05 05:20:59 +00:00
Collaborator
Copy link

Superseded by #862 (already merged via squash 5b497b5). Branch tip is fully on main; main is 3 commits ahead. Closing — no commits to merge.

Likely a duplicate run from the rebase-loop bug fixed in #859.

Superseded by #862 (already merged via squash `5b497b5`). Branch tip is fully on `main`; main is 3 commits ahead. Closing — no commits to merge. Likely a duplicate run from the rebase-loop bug fixed in #859.
claude-desktop closed this pull request 2026-05-05 05:21:00 +00:00
All checks were successful
qa / dockerfile (push) Successful in 14s
Details
qa / sql-layer-check (push) Successful in 16s
Details
qa / db-schema (push) Successful in 29s
Details
qa / qa-1 (push) Successful in 1m19s
Details
qa / qa (push) Successful in 0s
Details
qa / dockerfile (pull_request) Successful in 16s
Details
qa / sql-layer-check (pull_request) Successful in 29s
Details
qa / db-schema (pull_request) Successful in 32s
Details
qa / qa-1 (pull_request) Successful in 1m44s
Details
qa / qa (pull_request) Successful in 0s
Required
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
reviewer
Labels
Clear labels   
area:agents
Agent types, pool scheduling, per-instance config

  
area:dashboard
Dashboard UI and observability surfaces

  
area:database
DB layer — schema, migrations, ORM, raw SQL

  
area:design
UI/UX mockup work — routes to designer agent

  
area:design-review
Design review dispatch — routes to design-reviewer agent

  
area:flows
Flow runner — YAML loader, executor, op registry, expression eval

  
area:infra
Deployment, isolation, containers, systemd units

  
area:meta
Tracking, scaffolding, project setup

  
area:security
Security — routes to reviewer-security (opus)

  
area:sessions
Session-id store, Claude SDK resume logic

  
area:webhook
Forgejo webhook routing and handlers

  
area:workdir
Clone cache, worktrees, git identity

  
security
Security-sensitive issue

  
type:bug
Bug

  
type:chore
Chore

  
type:meta
Tracking or decisions, not implementation work

  
type:user-story
User story

No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!861
No description provided.