Version Control nav group, sidebar parity, ⌘K palette#24
Merged
Conversation
…arity Three changes that go together: 1. Connections/Policies sidebars now share the same top-level structure: Google, Cloud, LLM API, HTTP Proxy, MCP Proxy, Version Control, Slack. Previously Connections collapsed HTTP+MCP into one "Proxy" tab while Policies kept them split — they're now consistent. 2. New "Version Control" sub-group in both sidebars, parent links to ?type=version_control / ?scope=version_control, expandable children link directly to GitHub and GitLab. The group is collapsible like the existing Google/Cloud sub-groups in Policies. 3. handleConnections + handlePolicies recognise the synthetic "version_control" filter value and include both github and gitlab connectors/policies in the result. The catalog already groups GitHub + GitLab cards under a "Version Control" h3 header (their ConnectorMeta.Category), so the in-page section was always present — this just makes the sidebar match. Also rewrites the GitHub card's PAT-vs-App explanation panel: the inner grid-cols-2 layout squeezed both halves when the parent card sat in a 3-column grid at lg. Each option now stacks vertically as a single-paragraph description so it reads cleanly in the narrow card. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a fuzzy-search modal triggered by Ctrl+K (or ⌘+K on macOS) that
lists every top-level page and every Connections/Policies sub-tab. A
sidebar chip shows the keybinding ("⌘K" on Mac, "Ctrl+K" elsewhere).
UX details:
- Scoring: exact-prefix > substring-in-label > substring-in-keywords >
subsequence-fuzzy. Each item carries an optional keywords field for
common aliases (e.g. "Connections / Google" matches "gmail drive").
- ↑/↓ navigate; Enter opens; Esc / backdrop-click closes; Ctrl+K
toggles.
- innerHTML rendering uses an inline HTML-escape on every interpolated
value (defense-in-depth even though items come from a static list);
annotated with `// xss-safe:` so the templates-XSS lint passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the admin UI navigation to align Connections and Policies sidebar structure, adds a Version Control grouping (GitHub + GitLab), and introduces a Ctrl/Cmd+K command palette for fast page navigation. It also adjusts backend filtering to support the new synthetic version_control grouping and tweaks GitHub connection copy/layout for better responsiveness.
Changes:
- Add Version Control collapsible subgroup in sidebar for both Connections and Policies, plus command palette UI + fuzzy search index.
- Update server-side filtering in
handleConnectionsandhandlePoliciesto supporttype/scope=version_controlgrouping (GitHub + GitLab). - Rework GitHub connection “PAT vs App” explanation panel layout to avoid grid squeeze on large breakpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| internal/web/templates/nav.html | Adds Version Control sidebar subgroup and implements the Ctrl/Cmd+K command palette and its navigation index/handlers. |
| internal/web/templates/connections.html | Updates GitHub connection help panel layout/copy for better responsive behavior. |
| internal/web/server.go | Adds synthetic version_control filtering logic for connections and policies views. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Five threads from Copilot's first review of 67e4210: 1. Ctrl+K stole focus from open form inputs (e.g. connection alias / token fields). The toggle-open shortcut now bails out when the event target is an editable element AND the palette is closed; the toggle-close path still fires unconditionally as an escape hatch. 2. CMDK_ITEMS pointed at /policies?scope=google and ?scope=cloud, which are sidebar group labels rather than real leaf scopes — landing on them shows the "No operations picker" warning. Removed both; sub-scopes (gmail, drive, calendar, aws-*, hyperstack, etc.) carry the relevant keywords so fuzzy search still finds them. 3+4. Version Control collapsibles in both Connections and Policies sidebars nested an <a> inside a <button>, which is invalid HTML (interactive-in-interactive). Restructured each as sibling elements — an <a> for the navigation target plus a separate <button> for the chevron toggle — sharing the same flex container so the active / hover styling still tracks the whole row. 5. Added internal/web/policies_version_control_test.go: TestPoliciesPage_VersionControlScope pins three properties of the synthetic scope filter: ?scope=version_control includes github + gitlab + unscoped policies, ?scope=github excludes gitlab, and ?scope=slack excludes both VCS policies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 2 surfaced three genuinely-new issues on top of stale repeats of round-1 threads: - nav.html: the command-palette chip button at the top of the sidebar was missing type="button". The toggle-sub buttons I added in round 1 already have it; this brings the chip in line so the nav remains safe if it's ever rendered inside a <form>. - policy_ops_picker.html: previously fell back to the amber "No operations picker for scope X" card for github/gitlab/version_control, which made the rule builder useless for these new sidebar tabs. Added typed pickers for github + gitlab (Read / Write / escape-hatch groups mirroring each connector's Operations() set), and a friendly "pick a connector" landing card for the synthetic version_control scope that links operators to the per-connector tabs. - TestConnectionsPage_VersionControlTab (new file): asserts the /connections?type=version_control filter includes both GitHub + GitLab catalog cards and existing github/gitlab connections, and excludes connections of other types. Mirrors the policies test from round 1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single substantive change: the two new version_control tests each had their own copy-paste GET helper. Promoted the canonical "GET with operator session cookie" helper to server_status_test.go (matching the existing authedPost), and both new tests now call it directly. Avoids the drift Copilot flagged. The chip-button type="button" comment Copilot reposted is already in place since 99448cf — it's reviewing stale state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three threads from Copilot's review of 99448cf: 1. policies.html / handlePolicyCreate: the create-policy form on the ?scope=version_control tab would have persisted scope="version_control" into the saved policy config. The synthetic filter only pulls github + gitlab + unscoped policies, so a policy stamped with scope=version_control would orphan from both the github and gitlab tabs while showing up only under the synthetic group. Template: hide the create form on this scope and show a "browse-only view" card with links to ?scope=github / ?scope=gitlab. Server: reject POST /policies/create when policy_config.scope is "version_control" (defence-in-depth against a hand-crafted POST or stale browser tab). 2. nav.html cmdk overlay: added role="dialog", aria-modal="true", aria-labelledby pointing at a screen-reader-only <h2> title, plus role="listbox" on the results <ul> and aria-controls on the input. The chevron SVG inside the input gained aria-hidden="true" so it doesn't confuse screen readers. 3. nav.html cmdk overlay: closing the palette didn't restore focus to the element that triggered it (focus would land at <body>). openCmdK now captures document.activeElement; closeCmdK restores it, guarding against the captured node being removed from the DOM in the interim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- connections_version_control_test.go: the previous version_control catalog assertion was a soft probe — both "PAT or GitHub App" and "personal-access-token" had an explicit `continue` fallback so a regression that removed the catalog cards while leaving existing connections listed would have passed. Tightened to require: "PAT or GitHub App" (GitHub Meta().Description) AND "merge requests" (GitLab Meta().Description) AND both connection display names; no soft-fallbacks. - server.go handlePolicyCreate: comment referred to "pScope", a variable name that exists in handlePolicies but not here. Reworded to describe the list-filter (handlePolicies) by name and avoid the dangling identifier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- nav.html cmdk: completed the listbox ARIA widget. The parent ul already had role="listbox"; each rendered result item now carries role="option" + aria-selected="true|false" tracking the highlighted row. Screen readers can now announce the selected item correctly. - Test files: gofmt'd two helper-call sites in each of policies_version_control_test.go and connections_version_control_test.go to add the missing space after the comma between the second and third arguments. Behavior unchanged; lines pass `gofmt -s -l` cleanly now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
handlePolicies' version_control filter previously matched any policy whose stored scope == "version_control" via the generic "pScope == scope" branch. That contradicted the handlePolicyCreate comment which describes the scope as strictly github + gitlab + unscoped. Reworded the filter to make the synthetic scope a closed set: a policy literally stamped scope=version_control is excluded. Creation of such a policy is already blocked at handlePolicyCreate; this hardens the filter against a hand-crafted DB row that slipped past creation. Test: TestPoliciesPage_VersionControlScope now seeds a vcs-stamped policy as a negative control and asserts it does NOT appear under ?scope=version_control. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Focus trap: added a Tab / Shift+Tab handler inside the modal so focus can't wander to the underlying page. With aria-modal="true" set, browsers do not enforce trapping; the page is hidden behind the backdrop but still keyboard-reachable. Collects focusable descendants of the dialog and wraps Tab from the last → first, Shift+Tab from the first → last. - "No matches" empty-state row now carries role="option", aria-selected="false", aria-disabled="true" so the parent role="listbox" composite stays valid for assistive tech. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Both Version Control sidebar disclosure buttons (Connections and Policies) now declare aria-controls pointing at their submenu ul and start with aria-expanded="false". - toggleSub() keeps every button[aria-controls=id] aria-expanded in sync with the submenu's visibility — so screen-reader announcements match the on-screen state, including the auto-expand on page load (which calls toggleSub once per active sub-item). - Command palette Enter handler now bails out when the active element is a result anchor (.cmdk-item). The user can tab-walk to a specific row and press Enter without being redirected to whatever the arrow-key cursor points at. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- handleConnections version_control filter previously matched against `cat`, which is overridden from config.category for http_proxy connections. A user (or attacker who reached the connections table) could set http_proxy config.category="github" and have it appear under the version_control tab even though it's not actually a GitHub connector. Match against the immutable c.ConnectorType instead, mirroring the comment's intent. - TestConnectionsPage_VersionControlTab: simplified the github + gitlab seed configs to empty maps (the filter test doesn't depend on each connector's typed schema, and the prior payload was wrong enough that connection-creation was passing by accident). Added a new negative control: a fake-vcs-proxy http_proxy with config.category="github" must NOT slip into the version_control tab. The hardened filter excludes it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bf1c0d8 added a fake-vcs-proxy http_proxy negative-control seed but didn't register the httpproxy connector with the test registry, so Connections.Add returned "unknown connector type: http_proxy" and the test failed in CI. Register httpproxy in the test setup so the fake-vcs seed succeeds and the regression is actually verified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pressing ArrowUp/ArrowDown while focus sits on a tab-walked result anchor would rerender #cmdk-results via innerHTML, remove the focused node, and drop focus to <body>. Pull focus back to the search input *before* the rerender so there's no transient body-focus state and subsequent arrow presses continue to work from the input. The two keyboard idioms (arrow + Enter from the input; Tab + Enter on a specific row) are now cleanly mutually exclusive. Also coalesced the two near-identical ArrowDown / ArrowUp branches into a single block — duplicate scrollIntoView / renderCmdK boilerplate is the same path in both directions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- server_status_test.go: corrected the path reference in getRequest's doc comment from "web/internal slack_test.go" to "internal/web/slack_test.go" and clarified the package distinction. - nav.html: the search-icon <svg> inside the command-palette chip button is purely decorative — added aria-hidden="true" so screen readers don't announce it as an unlabelled graphic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
Test plan
🤖 Generated with Claude Code