feat(dashboard): pin a nested sub-item to the left nav rail

[chenmingwei23]

Problem / Motivation

The left nav rail is a fixed list. Everything under Agent Capabilities - Steering
files, Skills, Hooks, Prompts, Knowledge, Crews, Agent Templates, Connections,
Workflows - is reachable only by opening that page first and then picking a tab,
so a destination someone uses every day sits two clicks deep while the rail
itself cannot be changed.

Issue #8500 asks for four things. This PR ships ONE of them: pinning. See
"Deliberately not built" below.

Why it matters

The cost is paid on every visit, by exactly the people who use a sub-item most:
the deeper an item is, the more the fixed layout mismatches real usage. There is
currently no way to express "I live in Steering files" to the dashboard.

What changed (motivation -> approach -> change)

Goal. Mark a sub-item, it appears at top level; unmark it, it leaves. Persist
that per user, and change nothing for anyone who does not use it.

The two questions that decided the design.

Where does per-user dashboard layout state already live? It already exists and
is a near-exact match, so this PR introduces no new persistence mechanism.
website/src/lib/appNavHidden.ts is the sidebar pin-persistence contract for the
Apps group: a localStorage key, one module owning read/write, and a same-tab
window event because a localStorage write does not fire storage in its own tab.
lib/navPinned.ts mirrors it key for key. Worth recording for whoever reads
#8500 next: the codebase has NO identity-keyed per-user preference store. Layout
preferences (mc-app-nav-hidden, mc-app-nav-order, mc-nav) live in the
browser; only appearance and onboarding (theme_mode, language, onboarded)
are server-backed through the dashboard section of config.json. So
"per-user" in this codebase means per-browser for layout, and this PR follows the
layout precedent rather than inventing a third category.

What is a "sub-item"? Nothing in the nav data model represented nesting. The
rail is a flat list of Surface rows; "Steering files" was a SidePanelLayout
tab, and "Discover" is hardcoded JSX. Rather than invent a parallel catalog with
its own labels and icons, each promotable tab is now registered as a REAL
surface carrying a new optional Surface.pinnable. It stays in
getBuiltinSurfaces(), so every registry-wide invariant - labelKey coverage
included - keeps covering it for free, and a promoted row gets the rail's
ordinary label, icon and active handling. pinnable keeps it off the rail until
the user promotes it, which is the same conditional-inclusion job appOnly,
hiddenFromNav and previewFlag already do on that interface.

What was built.

• Surface.pinnable plus a named getPinnableSurfaces() getter. Named rather
  than inlined per call site for the reason the neighbouring
  getAdvertisedSurfaces() docstring already gives: a call site that forgets the
  predicate leaks a row silently.
• The nine Agent Capabilities tabs registered as pinnable surfaces, reusing the
  catalog keys CapabilitiesPage already renders (pages.capabilitiesPage.*_label)
  rather than minting nav.* twins. One destination, one name, and zero new
  i18n catalog entries.
• lib/navPinned.ts: mc-nav-pinned, read/toggle/subscribe/hook, mirroring
  appNavHidden.
• PinSurfaceButton, rendered from a page's headerRight. It derives its own
  subject from the URL instead of taking a prop, which is what keeps
  SidePanelLayout completely untouched - the four other pages built on that
  shared component are byte-for-byte unaffected.
• One expression in App.tsx drops unpinned pinnable rows, placed in the single
  derivation that file designates as the only correct place to filter.

Deliberately inverted vs the module it mirrors. appNavHidden stores the
HIDDEN set; this stores the PINNED set. Both are shaped so an id absent from
storage means "leave this row as it shipped". App rows default visible, so their
absent-means-default set is the hidden one. A promoted sub-item defaults to
off-rail, so here it is the pinned one. Storing hidden ids would require seeding
every sub-item as hidden on first run and would put every future sub-item on
every existing user's rail unasked.

Default is the empty set, so installing this changes nothing until a user pins
something. That is asserted, not asserted-in-prose: see the first rail test.

The default tab could not be pinned, and a test of mine said that was correct

Review caught this, and it is the sharper of the two findings because the miss was
not an ABSENT test -- it was a test asserting the defect. SidePanelLayout expresses
"the first tab" as the ABSENCE of ?tab= on desktop: it deletes the param when you
select that tab (SidePanelLayout.tsx:295) and its URL-sync effect never writes it
back (:369). So /capabilities lands on its first tab with no param,
PinSurfaceButton matched on the raw param alone, found no surface and returned
null -- the control was missing on the most prominent sub-item, the one the feature
most needs to cover.

My own test renders nothing on the host panel with no tab named asserted exactly
that absence, and its comment shows I had the fact and drew the wrong conclusion
from it: "SidePanelLayout shows its first tab BEFORE WRITING the param, so there is
no sub-item to attribute a pin to YET". I read a permanent desktop convention as a
transient startup state and then wrote the misreading down as intent. The case is
corrected in place rather than deleted, and its comment now records the wrong
premise, because a reader who trusted the old rationale would reintroduce the bug.

The fix: the host passes its own first tab (defaultTab={tabs[0]?.key}) and the
control resolves a missing param to it -- derived, not hardcoded, from the same
array SidePanelLayout computes first from, so the two cannot drift. The
complement is pinned too: with no default supplied the control stays inert rather
than guessing a tab, so the fallback must come from the host.

No screenshot covered the param-less URL either, so this was invisible to the
evidence as well as to the suite. There is a scene for it now.

The dead-key guard caught an incomplete subtraction

Frontend Tests (3) went red on src/i18n/deadKeys.test.ts: 30 keys referenced
nowhere against a baseline of 29. Deleting the hooks palette row removed the only
call site of components.commandPalette.providers.pagesProvider.hooks and left the
key in the catalogs -- exactly the "copy deleted without its key" case that test
names. The key is deleted from the 13 catalogs that carried it, returning the count
to 29. The baseline was NOT raised: banking a new dead key to green a gate is the
thing the guard exists to prevent. Reinstating the key reproduces the CI failure
exactly, which is how I know the diagnosis instead of assuming it.

Stale prose is a defect, and four pieces of it shipped

Review found that the artifacts contradicted the fixes, and every instance was
mine. PinSurfaceButton's own docstring still said the control "is absent until a
tab is actually selected"; the Known-gaps list still named three gaps that were
closed; the Screenshots section still said "not captured"; and the builtins.tsx
glyph rationale still cited the palette /hooks entry this PR deletes. I had
corrected the TEST that asserted the old behaviour and left the prose asserting it,
which is the same defect in a different file -- and it is the argument I made myself
one round earlier, that a reader who trusts a stale rationale reintroduces the bug.
All four are corrected in place, with the wrong premise recorded rather than
quietly overwritten.

Correcting the glyph rationale changed the code, not just the comment. hooks had
been moved from its tab's Webhook glyph to Zap BECAUSE the palette's standalone
/hooks entry drew a Webhook. Deleting that entry voided the reason, and the
webhooks surface is hiddenFromNav so it has no rail row -- nothing owns Webhook
on the rail. The file's own rule ("the tab's own glyph EXCEPT where it is already
spoken for on the rail") then says keep the tab's, so hooks has its Webhook back
and one of the three glyph divergences is gone. Note this one is NOT test-pinned:
mutating the glyph reddens nothing, and I am reporting that rather than implying a
guard exists.

The at-cap label, on the fourth pass

I deferred this three times on the grounds that it costs a new key in every
translated catalog. That cost was real -- catalogParity.test.ts requires every key
in all 11 non-English catalogs in the same commit, and an English-only key reddens
it in 11 places, which I measured before writing anything. What I had not checked is
that the repo's own translation driver is built for precisely this and explicitly
does NOT call a model: "sending it is the caller's job (an agent, a contributor,
whatever)". The blocker I kept citing was a step I had never attempted.

So the control no longer lies while disabled. components.pinSurfaceButton.pin_limit_reached
is added to en.manual.json and all 11 translated catalogs, composed from each
locale's OWN existing vocabulary rather than invented: each one reuses the verb and
the noun that locale's existing pin_to_sidebar string already uses, so for example
the Chinese entry keeps the verb that string uses for pinning instead of the
different verb meaning "fix in place", and the German, Japanese and Russian entries
each reuse their own established pin verb. Shaped after the sibling precedent components.bottomTerminalPanel.maximum_terminals
("Maximum {{n}} terminals"), same role and same {{n}} contract. en-XA.json was
REGENERATED with npm run i18n:pseudo, never hand-edited, as that file requires.

Declared plainly: the 11 translations were authored in this change and have not had
a native-speaker pass. The placeholder is preserved in every one and parity passes,
but the wording deserves a reviewer who reads the language. Dropping one locale's
value reddens parity, naming the key.

Declined: moving the control into SidePanelLayout

Review is right that every future host must remember defaultTab={tabs[0]?.key} or
the control silently vanishes on that panel's landing tab, and right that
SidePanelLayout already owns tabs[0]. But moving the control (or the fallback)
into that component changes a five-consumer shared component for the benefit of
hosts that do not exist yet, and this PR's whole shape is built on not touching it.
Review's own framing -- "when a second host adopts pinnable tabs" -- names the right
trigger, and that is the moment to do it. Until then the seam is one line in one
host, and the inert-without-a-default behaviour is test-pinned so an omission
degrades to "no control" rather than to a wrong pin.

Screenshots

Captured by website/scripts/capture-nav-pin-subitems.mjs, committed with the
change. It drives the REAL SPA with the backend stubbed -- the same way
capture-leftnav-footer-border.mjs does, because the subject is the rail
App.tsx renders and an isolated entry would only prove something about a
re-created rail. Every scene ASSERTS its claim against the DOM before the frame
is taken and fails the run otherwise, so a broken stub cannot ship as a picture
of a working feature. That is not a hypothetical: the first attempt at this
capture produced four plausible-looking PNGs of an app that had rendered no rail
at all, and the assertions are what caught it.

Rail with nothing pinned -- a capability tab is NOT on the rail until you pin it:

Steering files pinned -- it joins the Main group, above the Apps section:

The promoted row lights while its own tab is showing:

Collapsed icon-only rail -- the promoted row keeps a glyph distinct from Discover's compass below it:

Light theme:

The control offers to pin the tab you are looking at:

Once pinned, the same control offers to unpin:

At the five-pin cap a sixth tab cannot be pinned, and the control is disabled:

The landing view of /capabilities carries no ?tab= at all, and still offers the control:

Nothing pinned, and the tab is still searchable in the command palette -- the
effect the description declares, which no frame showed until now:

A regression these frames caught, which no assertion would have.
SidePanelLayout renders headerRight inside a flex items-center justify-between container. Passing a Fragment therefore made the pin control and
the existing Apply & Restart button two SEPARATE flex items, and
justify-between stranded the pin in the middle of the header, ~250px from the
action it belongs beside. Before this change headerRight was a single node, so
the defect arrived with the pin control -- mine, not the layout's. Fixed by
grouping the two into one element. aria-pressed and disabled were correct the
whole time; only looking at the frame showed it.

PinSurfaceButton also gained data-testid="pin-surface-button" so the harness
can address it without locating by the label it is asserting -- the first version
matched button[aria-pressed] and silently found a different toggle, which made
one scene pass for the wrong reason.

Command palette: a declared consequence of using the registry

Registering the tabs as real surfaces means they are ADVERTISED surfaces, and
collectPages() in the Search Everywhere provider reads the same
getAdvertisedSurfaces() list (pagesProvider.ts:161). So eight capability
tabs become searchable in the palette whether or not the user has pinned them.
That is intended rather than incidental -- the registry's own docstring defines an
advertised surface as one offered everywhere a destination can be found, and such
a tab IS always reachable inside its host panel; only its rail row is opt-in --
but the first version of this description did not name it, so it is named here.

Two things fall out of it, both handled:

• The palette's header comment claimed to read "the very same
  getAdvertisedSurfaces() list App.tsx renders the left rail from". This
  change makes that false, because the rail additionally drops an unpinned
  pinnable surface. Corrected in place rather than left standing beside a
  correction.
• The knowledge EXTRA_PAGES row carried the route /capabilities?tab=knowledge,
  byte-identical to the new capabilities-knowledge surface. collectPages()
  dedupes by exact route with the registry loop running first, so that row became
  unreachable dead code. Deleted, along with its orphaned title-key row and a
  now-unused icon import. Ownership of the entry moved to the registry, and the
  palette test now pins the move: dropping the surface from its fixture takes the
  count to 0.

SUBTRACTION TAKEN, after I declined it twice. The hooks EXTRA_PAGES row is gone
too, with its title key and a then-unused icon import. My earlier reason for
keeping it -- that unlike knowledge its route DIFFERS, so it is live code rather
than dead -- was true but not sufficient: both routes render the same HooksPage
under the identical title "Hooks", and this file's own webhooks comment sets the
standard that adjacent hooks results must differ in BOTH title and icon. The
standard is the file's, not the reviewers', and this change is what broke it.

The test that used Hooks as its exemplar of "routed-but-not-in-rail EXTRA_PAGES"
was neither deleted nor loosened: it now uses Developer, the same KIND of entry
(a real route with no rail surface), so the property it exists for is unchanged and
only the specimen moved. Mutation confirms it still bites -- deleting the
developer row reddens it. mc-agents is deferred rather than dropped: its route
redirects to /capabilities root rather than to a tab, making it a near-twin
rather than a duplicate.

A second regression the screenshots caught. On a promoted tab the rail lit TWO
rows at once -- the promoted row and its host "Agent Capabilities" -- because the
host's active test is a prefix match on /capabilities while the promoted row's is
an exact ?tab= match, so both passed. Two rows claiming "you are here" answer the
question with neither. The host now yields to the promoted row that owns the tab.
Note what missed it: this suite already asserted the promoted row IS lit, which a
two-lit rail satisfies. The new test counts lit rows instead, and the capture
harness now counts across the whole rail rather than checking one row -- its
lit > 0 was the same weakness in the same shape.

Overflow: a real consequence, measured

The rail is three blocks. The top block (App.tsx, brand + Main rows + Apps
header) is shrink-0 and does NOT scroll; the only scroll region is the middle
flex-1 min-h-0 overflow-y-auto, which holds the dynamic app list; the bottom
block is shrink-0. Promoted rows join the Main group in the non-scrolling top
block, so they take height from the app list's scroll area rather than becoming
scrollable themselves.

Handled with a cap: NAV_PINNED_LIMIT = 5, enforced on write AND again on read
so a hand-edited or previously-larger stored value cannot render an unbounded
rail. This is the same defence the existing APPS_NAV_LIMIT = 6 provides for the
app list. Reaching the cap REFUSES the new pin rather than evicting an older one,
because silently dropping a row the user pinned earlier is worse than a refused
click. Residual, stated rather than hidden: on a very short viewport the app
list's scroll area gets shorter. That is true of adding any Main-group row and is
not specific to this change; the cap is what bounds it.

Deliberately not built (issue #8500 asks for four things)

• Hide / collapse unused sections (the other half of the issue title). Not
  built. Note the machinery is closer than it looks: mc-app-nav-hidden already
  hides rows, but only ever filters the Apps group, so extending it to static
  surfaces is its own change.
• Drag-reorder of the rail. Not built. It exists for the Apps group
  (mc-app-nav-order, dnd-kit) and is a separate item.
• Expandable sub-listings under each root section. Not built. Promoted rows
  join the Main group; they are not nested under their host.
• Count badges per section. Not built.
• Hosts other than Agent Capabilities. Only that panel's tabs are registered
  as pinnable. Settings, Developer, Schedule and Webhooks also use
  SidePanelLayout and would each need their tabs registered; the seam is now
  there for them and costs one entry each.

Tests

31 new assertions across four files, each mutation-verified (below).

• lib/navPinned.test.ts (13) - storage contract: absent / malformed /
  non-array / tampered values all degrade to "rail as it shipped" and never
  throw; toggle round-trips; the cap refuses a 6th pin WITHOUT evicting an
  earlier one (per-item pairing, not just a size check); the cap also truncates
  on read; unpinning still works at the cap; every persisted write dispatches the
  sync event and a REFUSED pin dispatches nothing; the stored array is sorted.
• test/App.navPinnedRail.test.tsx (7) - real rail, real derivation: with
  nothing pinned NO pinnable surface has a row (asserted across EVERY pinnable
  surface, not one sample - this is the regression guard for the whole feature);
  a pinned id gets a row while an unpinned sibling does not; the filter is live
  on the same-tab event in both directions; the ordinary rail is set-identical
  before and after a pin; and a promoted row paints as current for its own tab
  but not for a sibling tab.
• components/PinSurfaceButton.test.tsx (10) - inert on an unrelated page, on a
  bare host URL, and on an unregistered tab; toggles and reports aria-pressed;
  the two states do not announce identically; disabled at the cap when unpinned;
  still live at the cap when already pinned, so the cap is escapable.
• i18n/navLabels.test.tsx (+1) - closes a vacuity I found in the existing
  guard. That file rejects a raw key only by its nav. prefix, so for a
  surface whose labelKey lives in another namespace an unresolved key is truthy
  and does not match /^nav\./ - it would sail through. The new case asserts
  each pinnable label resolves to something different from the key itself, in
  every SUPPORTED_CODES language.

Rows are located by the data-onboarding-nav navId, never by the label under
assertion, so a test cannot pass by finding a sibling that renders the same
string, and the assertions are not coupled to catalog wording.

Mutation verification - 9 mutations, 9 DISTINCT reddened sets, each applied
from a uniqueness-asserted anchor, each read from the runner's own failure line,
each reverted and the tree confirmed clean afterwards:

mutation	reddened
rail filter clause removed	3 rail tests
write cap neutered	2 (refusal + no-op event)
read cap (slice) removed	1 (truncation)
non-string filter removed	1 (tampered array)
change event not dispatched	1 (propagation)
cap gate ignores already-pinned	1 (cap escapable)
subject match ignores the tab	6 button tests
active: query branch made unreachable	2 (both active cases)
active: tab equality made inert	1 (sibling only)

The last two are the same property from opposite sides and redden DIFFERENT
sets, which is what says the two observables are independently pinned rather
than one of them being decorative. That the nine sets all differ is also what
rules out a harness artefact.

A tenth mutation was attempted with a multi-line anchor, silently matched
nothing, and is reported here as a missing measurement rather than a pass; it was
re-anchored to a single line and is the last row above.

Also run green, unchanged: surfaces.test.tsx (31), previewSurfaces.test.tsx
(27), App.appNavHiddenFilter.test.tsx (5), App.test.tsx (104),
CapabilitiesPage.knowledgeTab (2), CapabilitiesPage.tabLanguage (1),
MembersPage.test.tsx (37). tsc --noEmit clean; eslint clean on every
changed file.

Manual verification

Performed - see the Screenshots section, ten committed frames. Every claim
above rests on the automated coverage and the mutation table, and the load-
bearing behaviours (default-unchanged rail, live toggle, active state, cap
refusal) are each pinned by a mutation-verified test rather than by a click-
through.

Screenshots / video

Captured. Ten frames under temp-screenshots/nav-pin-subitems/, produced by
website/scripts/capture-nav-pin-subitems.mjs (committed) and shown in the
Screenshots section above. An earlier revision of this PR said "not captured, and
I am not waiving the requirement", which was true then and became false without
this section being updated -- exactly the stale-artifact problem review flagged
elsewhere in this description. The no-screenshots label was never applied: it is
maintainer-scoped, and applying it would skip the gate rather than satisfy it.

To see it: open Agent Capabilities, pick any tab, and use the pin control left of
the Restart button in the page header. The row appears in the rail immediately
(no reload). Unpin from the same control. With nothing pinned the rail is
byte-identical to today.

Related Issues

Refs #8500

Refs, not Closes, deliberately: #8500 asks for four things and this ships
one. Closing it would credit this PR with the hide/collapse, reorder,
sub-listing and badge work that is not here.

Pattern harvest

Rule candidate: review-prompt
Pattern: an invariant test that rejects a raw i18n key by matching one namespace
prefix (/^nav\./) is vacuous for any key outside that namespace - it must
compare the resolved string against the key itself instead. Found live in
navLabels.test.tsx while adding surfaces whose labelKey is a
pages.capabilitiesPage.* key; fixed in this PR for the new surfaces.

Checklist

• At most two commits (one), Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) - N/A, no doc surface describes the
  rail's contents
• No secrets, credentials, or internal references in the diff

Notes for review

• Shared file with sibling work. website/src/App.tsx is the file this
  shares with Collapsible chat input and pinned message for more reading space #8453 (collapsible chat input) and Header: keep CPU/MEM/DSK session stats visible when width is tight #8287 (header stats when width
  is tight), both in flight. Measured at the time of writing: neither had an open
  PR, and none of the 17 open PRs on this account touched App.tsx,
  SidePanelLayout, surfaces/ or appNavHidden. My change is confined to the
  nav rail - one filter expression, one active helper, one carried-through
  field - and touches no chat composer or topbar code.
• i18n key reuse, declared. PinSurfaceButton uses
  pages.libraryPage.pin_to_sidebar / unpin_from_sidebar. Same grammatical
  role (the accessible label of a control that adds or removes a sidebar row) and
  the same {{name}} interpolation contract, already translated in every
  catalog. The alternative - a components.sidePanelLayout.* twin - means one
  sentence duplicated across 14 files, which is the drift hazard the reuse
  avoids. Happy to move both keys to a shared namespace if reviewers prefer it;
  that is a rename, not a translation.
• Known gaps, none silent. Only one of the original four survives. The two
  "Hooks" palette entries are gone (subtraction taken), the at-cap control now
  states its limit instead of promising a pin, and the bare /capabilities URL now
  resolves to the host's first tab. What remains: rail rows carry no aria-current
  anywhere on main, so a promoted row's active state is class-only and the tests
  assert the class. Exposing aria-current across the rail is a pre-existing gap
  and out of scope here.

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence reconciled — every control and state appears in the ten screenshots, the blind read ran, and there is no hard-swap transition needing a recording. Final review:

UX-Verdict: CONCERNS

Pin control works and reads correctly, but its at-cap state looks live and eats the click, and the pinned row's icon contradicts its own tab.

Watch

• At-cap pin (shot header-3-at-cap) is visually near-identical to the live unpinned pin (header-1); the blind reader took it for live: "clicking pins this page → yes". Clicking a disabled button yields silence, and "Maximum 5 sidebar pins - unpin one first" lives only in a hover tooltip — task-gating info demoted to hover. Rare state × total silence × every time at cap. Fix: keep the control clickable at cap and show the limit message at the click point.
• Pinned "Steering" rail row uses a scroll glyph while its own tab shows a compass in the same viewport (rail-5), and compass also means "Discover" in that rail — reader: "if these are the same page, the two different icons are confusing." Fix: change the steering/crews tab glyphs in CapabilitiesPage to match the new surface icons (ScrollText, Bot) so one destination keeps one icon.
• The pin→rail link is only inferable: reader rated the pin button "a guess" and noted "nothing on screen ever says the word 'pin'… the link between the thumbtack and the menu entry is inferred, not shown." Live row-appearance and the tooltip mitigate; acceptable, but it is the feature's main path resting on hover text.
• Pinning the crews tab puts a row literally labeled "Agents" in the same rail as "Agent Capabilities"; the reader already "cannot tell how 'Agents' and 'Agent Capabilities' relate." Pre-existing label, but this PR is what makes them adjacent.

Suggestions

• components.pinSurfaceButton.pin_limit_reached: replace " - " with " — " to match sibling strings (e.g. already_attached).

[UX-REVIEWED] 1e1074e

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Registry-backed pinning that reuses the existing layout-persistence and surface-registry seams, defaults to no change, and declines generalization until a second host exists — sound and proportionate.

Suggestions

• CAPABILITY_SUB_ITEMS in builtins.tsx and the tabs array in CapabilitiesPage.tsx are now two hand-maintained catalogs of the same nine tab keys with no invariant tying them; a future tab rename/removal in one leaves a registered pinnable surface (palette entry + pinned rail row) navigating to a tab that no longer exists, silently falling back. A cheap membership test (pinnable routes' ?tab= values ⊆ the page's tab keys) — or deriving both from one shared key list — would close the drift the same way this PR's other invariants are pinned.

[DESIGN-REVIEWED] 1e1074e

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. The change is a self-contained opt-in feature; the mechanisms it claims to mirror (appNavHidden.ts, APPS_NAV_LIMIT, the committed temp-screenshots/ convention with 1080 files) all check out, and consumer counts for the new surface are non-zero. One small zero-harm guard is worth subtracting. Final review:

First-Principles-Verdict: PASS

Every item traces to issue #8500 or a constraint in the repo; the only unearned surface is a two-line read-side cap that guards nothing reachable.

What this change ships

Intent: let a user promote an Agent Capabilities tab they use daily onto the left nav rail — an ADDITION (one of #8500's four asks, by declaration).

1. Pin control in the Capabilities header pins/unpins the tab shown — justified
2. Pinned tab becomes a top-level rail row, per-browser (mc-nav-pinned), empty by default — justified
3. Rail unchanged for anyone who pins nothing — justified, pinned by test
4. Five-pin cap; at cap the control disables and states the limit (new i18n key ×13 catalogs) — justified (non-scrolling shrink-0 Main block; APPS_NAV_LIMIT precedent at App.tsx:1815)
5. Promoted row lights on its tab; host row yields so exactly one row is current — justified
6. Param-less /capabilities landing view still offers the control — justified fix
7. All nine capability tabs now searchable in the command palette (seven newly) — declared, deliberate consequence of registry registration
8. Palette "Hooks" retargets from /hooks to the capabilities tab (same HooksPage) — justified move (the file's own title-uniqueness invariant forbids the twin)
9. Capture script + nine PNGs committed — convention (1080 files under temp-screenshots/)
10. Feature-map doc updated — mandated same-commit doc rule

Subtractions

• Drop the .slice(0, NAV_PINNED_LIMIT) in readNavPinned (navPinned.ts:70) and its test. The rail is already bounded: App.tsx:1787 renders a pinned id only if a registered pinnable surface carries it (9 exist, grepped CAPABILITY_SUB_ITEMS), and the write path enforces the cap. The only route past it is the user hand-editing their own localStorage — not a boundary this codebase names — and "written under a higher earlier cap" is future-proofing; today no earlier cap exists.

[FIRST-PRINCIPLES-REVIEWED] 1e1074e

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a — this comment is updated in place on each push.

Review details

No blocking issues; one advisory active-state gap.

FINDING — website/src/App.tsx:416 — promotedTabOwner/navRowActive have no default-tab fallback, so on the param-less /capabilities landing (which desktop SidePanelLayout permanently uses for the first tab, crews) new URLSearchParams(location.search).get('tab') is null, null === 'crews' fails, and a pinned first-tab row stays dark while the host row lights — the same "row never paints active while you stand on it" the PR fixes for every other tab, and inconsistent with PinSurfaceButton's own defaultTab handling. → Fix: in navRowActive/promotedTabOwner, treat a missing tab param on /capabilities as the first tab when comparing, mirroring the button's params.get('tab') ?? defaultTab.

[OPUS-REVIEWED] 1e1074e

Verdict parsed from the review's SHA-scoped output markers for commit 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings (all downgraded on adjudication)

GPT 5.6 flagged blocking issues on 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a; Opus 4.8 adjudication downgraded every one of them to advisory. Adjudication downgraded all 1 blocking finding(s) to advisory: the remedy each one requires is disproportionate to the harm it prevents. Read them as advice, not as merge conditions.

This comment is updated in place on each push.

Review details

BLOCKING -- website/src/components/PinSurfaceButton.tsx:53 -- The mobile root pins Crews although no sub-item is open
const activeTab = params.get('tab') ?? defaultTab ?? null
390px /capabilities -> mobile root renders the header control -> clicking it unexpectedly pins Crews.
Anchor: narrow-viewport-required
Fix: Apply defaultTab only on desktop; keep the parameterless mobile root inert.

FINDING -- website/src/App.tsx:2852 -- On desktop /capabilities, "get('tab') === wanted" leaves pinned Crews inactive while Crews is displayed -> Fix: resolve a missing tab to the host’s default before active-row comparison.

[GPT-REVIEWED] 1e1074e
[BLOCK-MERGE-DOWNGRADED] 1e1074e

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I have enough to rule. Verified facts:

• On mobile root of /capabilities, headerRight (dock header) renders — SidePanelLayout.tsx:412, inside the !mobileTab root branch (SidePanelLayout.tsx:403-404); mobileTab is null when no ?tab= param (SidePanelLayout.tsx:216).
• activeTab falls back to defaultTab with no param — PinSurfaceButton.tsx:53 — so the control matches the Crews surface and clicking pins it (PinSurfaceButton.tsx:80).
• The action is a pure toggle: clicking again removes the pin — toggleNavPinned deletes on re-click (navPinned.ts:82-86), and the change is visible (the rail re-renders and the control flips to "unpin", PinSurfaceButton.tsx:60-74).
• The component is deliberately viewport-agnostic; its docstring (PinSurfaceButton.tsx:1-23) makes URL+defaultTab its sole inputs to keep it out of SidePanelLayout. The proposed fix threads a useIsMobile branch into it, adding a permanent cross-cutting dependency and branch to a component designed to have none.

Harm is LOW: an unintended pin on a narrow viewport, immediately visible and reversible by the same one-click toggle, self-correcting. The remedy's permanent maintenance load (viewport coupling in a component built to avoid it) exceeds that trivially recoverable mispin.

LOW — conditions: mobile root renders control (SidePanelLayout.tsx:412), defaultTab fallback (PinSurfaceButton.tsx:53), pins on click (PinSurfaceButton.tsx:80); recovery: same toggle unpins (navPinned.ts:84-86), visible.
Real fix: add useIsMobile branch to a deliberately viewport-agnostic component — permanent coupling cost exceeds a visible, one-click-reversible mispin.

[ADJUDICATION] 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a total=1 uphold=0 downgrade=1
DOWNGRADE F1 website/src/components/PinSurfaceButton.tsx:53 reason=disproportionate-remedy
[GPT-ADJUDICATED] 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a: <one-sentence reason>

[chenmingwei23]

@-

[chenmingwei23]

Round 2, on fb4830759.

First: the empty description was my error and it is fixed. All three reviewers
correctly reported the body as the literal @-. I updated it with
gh api -f body=@-, where -f takes a literal and only -F reads @, so I
wrote the two characters instead of the file. The API returned the PR number and I
read that as success when it only meant the field had been written. The full
description is restored and verified by reading it back, not by the return value.
Everything items 3-8 of the First Principles list called "declared only by code
comments" is in it, including the cap, the storage key, the glyph rationale and
the palette consequence.

The hooks palette duplicate: keeping it, with three measurements behind the
decision rather than the "not our surface" reason I gave last round.

1. It is NOT the same act as the knowledge removal, which is why the strategy
   looks half-applied. knowledge's EXTRA_PAGES route was BYTE-IDENTICAL to the
   new surface's, so exact-route dedup made it unreachable dead code -- I verified
   that by mutation (reintroducing the row produces no second entry). /hooks is a
   different route from /capabilities?tab=hooks: it is live, reachable, and
   App.tsx routes it. Deleting it removes a real destination rather than dead
   code.
2. pagesProvider.test.ts:87 uses that very entry as the EXEMPLAR of
   "routed-but-not-in-rail EXTRA_PAGES" -- the class the hand-list exists for. The
   standalone page has no rail surface, so the palette is its only discovery path.
3. "Two same-named destinations with no way to tell them apart" is not what the UI
   does. CommandPalette.tsx:762-789 renders each row's subtitle, and
   pagesProvider.ts:166,179 set it to the route -- so the two rows read
   /hooks and /capabilities?tab=hooks. The redundancy is real; the
   indistinguishability is not.

On the retitle alternative: I searched the catalogs for a reusable
distinguishing title at the same role and there is none. pages.hooksPage.hooks
is the same word, and gov_scope_script_hooks ("Script hooks") is a governance
scope name, so using it as a page title would be the cross-role key reuse
i18n-catalog.md forbids. A retitle therefore costs a new key in 12 translated
catalogs, which is a translation per locale rather than a lookup.

So the fork is a maintainer's, and both arms are costed: drop the standalone
/hooks palette row and accept that the standalone route loses its only discovery
path, or retitle it and accept one new string in 12 locales. I am not picking
between those inside this PR.

mc-agents is deferred rather than dropped: it redirects to /capabilities root,
not to a tab, so it is not the same destination as capabilities-crews -- a
near-twin, as the review says, rather than a duplicate.

Still open and not waived: no screenshots, so the blind read has not run. That
is a real gap in this PR, not a disagreement. I have not applied the
maintainer-scoped no-screenshots label.

[chenmingwei23]

Board is green on 1e1074ec1a96f9cb063b001c2ba390cb1eb64c8a and stable across two
reads: 64 lanes, 0 pending, 0 failing, PR Readiness success, and all five bot
verdicts bound to this head with no [BLOCK-MERGE]. A human approval is the only
gate left; I am not merging.

Recording the four UX items still open, none of which I am changing on a green
board, with why:

1. At-cap still reads as live. I fixed the half where the label lied -- at the
   cap it now says "Maximum 5 sidebar pins - unpin one first" instead of promising
   the pin -- but UX is right that the message lives in a tooltip and the disabled
   pin looks like the live one. Their newer suggestion is a different design:
   keep the control clickable and surface the limit at the click point. That is a
   new interaction, not a tightening of this one, and disabled-plus-tooltip is the
   convention this codebase already uses. Worth doing; worth a maintainer choosing
   it rather than me widening scope after the board went green.

2. The promoted Steering row wears a scroll glyph while its own tab shows a
   compass. Real, and visible in one frame (rail-5). It exists because Discover
   already owns the compass on the rail and the collapsed rail is icon-only, so the
   rail row had to differ from something. UX proposes the better resolution I had
   not considered: change the TAB glyphs to match the new surface icons, so one
   destination keeps one icon everywhere. I did not take it because it changes the
   appearance of a tab strip users already know, for tabs beyond the ones this
   change promotes -- a judgement about an existing surface rather than a
   consequence of this one. Note the third divergence is already gone: hooks got
   its Webhook glyph back once deleting the palette twin removed the collision.

3. The pin-to-rail link is inferable rather than shown. UX rates this
   acceptable and so do I; recording it because the feature's main path does rest
   on hover text.

4. Pinning the crews tab puts a row labelled "Agents" beside "Agent
   Capabilities". The label is pre-existing, but this change is what makes the two
   adjacent, so the confusion is fairly attributed here. A rename is a product
   decision about an existing surface.

Scope, restated plainly: this ships PINNING only -- one of the four asks in #8500.
Hiding or collapsing unused sections, expandable per-section sub-listings, and count
badges are NOT built, which is why the trailer is Refs and not Closes. The
finding I think outlives the feature is in the description: there is no
identity-keyed per-user preference store in this codebase, so "per-user and
persistent" as the issue words it is not fully satisfiable today -- what shipped is
per-browser, declared as such.