An Auto-dark-mode user reported being flashed by a white window every
evening, then switching to a dark palette by hand. That is two defects,
and only one of them is the missing feature.
The flash fired even for someone who had already saved a dark palette.
`index.html` hardcoded `data-theme="white"` while the code that reads the
stored theme, `initializeTheme()`, only runs after the app bundle's
dynamic imports resolve. So every launch painted white first. A
synchronous inline script now resolves the theme before any stylesheet is
parsed, and Electron seeds `backgroundColor` from a cached appearance so
the window is not white before the renderer's first frame either.
Following the system is a switch in Settings -> General rather than a
seventh palette. The OS only reports dark/light while the app ships six
palettes, so each ground carries its own preference: the picker splits
into "use in light mode" (the four paper grounds) and "use in dark mode"
(the two ink ones), and a pick lands in the preference for its own ground.
Choosing ink-blue at noon is therefore remembered for that night rather
than fighting the OS. Detection goes through `prefers-color-scheme`
because the same renderer runs under Electron, the Tauri shell and the
browser entry, and the media query is the only signal all three share.
New installs follow the system; existing ones keep their fixed palette
until they opt in, so an update never silently repaints someone's app.
`nativeTheme.themeSource` is deliberately left alone, which is the part
most likely to be "fixed" later. Pinning it to the user's palette would
make context menus and the macOS frame agree with the app, but it is a
process-wide override of `prefers-color-scheme` — the very signal this
feature reads. Re-enabling the switch would then resolve against the
pinned value instead of the real OS setting, and the override also leaks
into the preview WebContentsView, forcing third-party pages to the app's
theme. A test fails on any assignment to it.
The OS-flip listener reads the preferences from storage rather than from
its own store. The pet and trace windows run the same bootstrap with
their own store instance over one shared localStorage, so after the main
window turns the switch off their in-memory copy still says "on" — acting
on it wrote the user's choice straight back out. A `storage` listener
catches the other windows up.
`settingsStore.theme` is gone. It was a copy that only refreshed on an
explicit `setTheme`, so an OS flip left the Settings picker highlighting
a palette that was no longer on screen. uiStore owns the theme; the copy
had no remaining readers.
The 「纸·墨·印」 rename reaches the new keys too: `light` -> `warm-classic`
now migrates for the per-ground preferences, not just the applied theme,
so the palette daytime returns to is not silently reset.
Guards, each verified by breaking what it protects: the inline script is
extracted from `index.html` and run verbatim against `resolveAppliedTheme`
over every stored combination — including dirty values, which are
reachable because it runs before the persistence migrations, and
cross-ground values like a dark palette stored as the light preference;
the three copies of the palette grounds (CSS `--cc-bg`, `index.html`,
main process) are pinned to each other and to `THEME_MODES`, so a seventh
palette cannot ship without a pre-paint color; the two grounds are proven
to cover every palette at compile time; and the IPC payload is held to a
literal 6-digit hex because `setBackgroundColor` also accepts
`#AARRGGBB`, where a translucent window means click-through and overlay
spoofing.
Four gaps against the official Grok CLI surface, found by comparing this
adapter with sub2api's and each confirmed by a probe against the live
endpoint.
The first is not a parity gap but a hard failure. A named tool_choice was
translated into Chat Completions syntax, `{type:'function',function:{name}}`,
which the Responses endpoint refuses:
422 data did not match any variant of untagged enum ModelToolChoice
so every request where the caller pins a tool failed outright. Responses
names the function inline. Only the Responses transform changes; the Chat
transform keeps the nested form, which is correct there.
The rest:
- A tool_choice sent with no tools, or one whose target the BatchTool
filter removed, is dropped rather than left selecting a tool the upstream
never receives — and a tool list that filters down to empty is omitted
instead of sent as `[]`.
- `prompt_cache_key` and the `x-grok-conv-id` header carry one identity per
conversation, reusing resolvePromptCacheKey (the Claude Code session id)
so a turn stops re-billing the whole prefix. Both survive the 401 refresh
retry: header assembly is folded into one helper so the first attempt and
the retry cannot drift apart.
- `x-grok-client-mode: interactive` matches what the official client sends.
Probes, in order: baseline 200; the three new fields together 200; the
inline tool_choice 200; the nested form 422.
Also lands the adapter-level regression test for the mid-stream socket
reset fixed in the previous commit — it drives a real reset through
buildGrokFetch and the SSE transform, so it needs both changes present.
A turn on the Grok provider died on `API Error: The socket connection was
closed unexpectedly` with partial text already on screen. That message is
Bun's, raised while reading a response body the peer reset: a raw TCP server
that sends chunked SSE headers plus one event and then terminates the socket
reproduces it exactly, as `Error{code:'ECONNRESET'}`.
The reset itself is the network — a stale pooled keep-alive socket, a
proxy/NAT dropping a reused connection, an edge resetting mid-flight — not
the payload. Ruled out against the live endpoint: twelve tools with a 21KB
skill tool_result, an 8-minute 11MB 47k-event generation, and a pooled
connection reused after 130s idle all complete normally.
What was broken is that nothing retried it. The error lands during stream
*consumption*, where all three recovery paths miss it:
- `withRetry` only wraps stream creation, so consumption never reaches it.
- `isRetryableStreamError` requires an APIError carrying `"type":"api_error"`
inside a 200 SSE body. A dead socket is a bare Error with neither.
- The desktop injects `CLAUDE_CODE_DISABLE_NONSTREAMING_FALLBACK=1`, so the
one remaining branch rethrows and ends the turn.
`isRetryableStreamTransportError` therefore walks the cause chain for a
transport code (ECONNRESET/ECONNABORTED/EPIPE/UND_ERR_SOCKET/
ERR_STREAM_PREMATURE_CLOSE) instead of keying on the error type, and
queryModel re-establishes the stream through the existing withStreamRetry
budget.
SSE has no resume, so the retry is a full re-send and is gated on the same
side-effect boundary the watchdog retry uses: StreamAssistantCommitBuffer
holds completed thinking/text unpublished, so they are discarded with the
attempt, but once a tool_use block completes or server-side tool activity
starts the disconnect surfaces to the user instead (#766 / inc-4258).
Keep-alive is disabled only when nothing arrived at all — no `message_start`
means the socket was already dead when the request went out, which is the
stale-pool signature, while a connection that broke later leaves the pool
trusted.
This preempts the non-streaming fallback for these disconnects, the same
trade the existing transient-error branch already makes.
Covered by the guard matrix (each transport code, bare and wrapped in
APIConnectionError; tool boundary crossed; watchdog abort; user abort;
non-transport error) and by withStreamRetry recovering a transport-flavoured
RetriableStreamError — the first one whose payload is not an SDK error
object — including the exhaustion branch that has to render it.
The redesigned chat header stacked a non-truncating 22px title above its
metadata row inside px-9 pt-6 pb-4, so a long session title wrapped onto
two lines and pushed the transcript down. Truncate the title on one line
at 15px, move the metadata onto that same row, and tighten the padding to
py-2: measured 123.5px -> 35.75px on a real session at a 1600px viewport,
with the full title kept in a hover tooltip. The wide and workspace-open
layouts now share one set of styles instead of drifting apart, and the
metadata separator is marked aria-hidden since it is pure decoration.
`TaskUpdate` treats `deleted` as a delete action, not a status — the CLI
tool unlinks the task file and returns early. The activity panel read it as
a status, and `normalizeTaskStatus` has no case for it, so it fell through
to `pending`. The row then survived `mergeTaskRowsById`, which only
overwrites rows the live list still knows about. A task the server had
already deleted stayed pinned to the panel as pending, badge included.
Reproduced end to end against a real provider (MiniMax-M3) on a throwaway
Node project: the model reached for `TaskUpdate{taskId:'1',status:'deleted'}`
on its own, `~/.claude/tasks/<session>/1.json` was gone afterwards, and
`GET /api/tasks/lists/:id` returned only the two survivors — while the panel
still rendered three rows with a badge of 1.
Three places needed it:
- Parsing `TaskUpdate` now removes the row instead of restating it. This
also stops the panel inventing a `Task #N` row when the deletion has no
matching `TaskCreate` in the same turn.
- Deletions are collected across turns. A task created in one turn is often
deleted in a later one, and per-turn parsing left the earlier row behind,
which also skewed the "Earlier tasks" roll-up to `1 of 2 / stopped`.
- `liveTasks` is filtered too. The task list only refreshes once the
`tool_result` lands, so between the deletion and that round trip it still
reports the deleted task.
`normalizeTaskStatus` keeps its `pending` fallback — `deleted` is now caught
upstream, and folding a delete action into the status enum is what caused
this in the first place.
Verified by replaying both real transcripts through the old and the new
model side by side: the single-turn session goes from 3 rows/badge 1 to
2 rows/badge 0, and the two-turn session from `Task #1 pending` plus a
`stopped` history row to just the one completed task. Four regression tests
cover same-turn deletion, deletion without a matching create, cross-turn
deletion, and a stale live list. `check:desktop` green at 3051 tests.
Project scope reads and writes disagreed about which file owns a server.
getMcpConfigsByScope('project') walks from the cwd up to the filesystem root
and inherits every .mcp.json it finds, so a server declared in an ancestor
directory shows up for a nested project -- with canRemove: true, because the
DTO derives that from the scope alone. removeMcpConfig('project') only ever
read the cwd's own .mcp.json, so it threw a plain Error for exactly those
servers, and deleteServer let it fall through to the errorHandler's 500 catch.
That is the reported pair of symptoms: the delete does nothing and the toast
says "An unexpected error occurred". Windows hits it easily, since dropping
.mcp.json in the home directory (mistaking it for global config) makes every
project below inherit servers that none of them can remove.
removeMcpConfig now resolves the file that actually declares the server, using
the same nearest-first order the reader uses, and edits that file. The edit is
applied to the raw parsed JSON rather than to a rebuilt McpJsonConfig: the old
rebuild ran through parseMcpConfigFromFilePath with expandVars: true, so
rewriting the file baked resolved ${VAR} values into it and dropped every
top-level key outside mcpServers, since the zod schema strips them. Lookup
matches on the raw object too, because validation is all-or-nothing per file --
one malformed entry would otherwise make every server in that file unremovable,
and would leave lookup and rewrite working off different views.
addMcpConfig had the same rebuild, so adding any project-scoped server rewrote
its siblings the same way. Measured against a live server with MY_MCP_TOKEN
set, "Bearer ${MY_MCP_TOKEN}" came back as "Bearer super-secret-value" -- a
plaintext credential written into a file that is normally committed. It now
inserts into the raw object as well, and its duplicate-name check reads the raw
file so a name sitting in an otherwise-invalid .mcp.json is still reported as a
conflict instead of being silently overwritten.
Two smaller corrections follow from the same asymmetry. deleteServer reports
why a removal failed (400 with the underlying message) instead of an opaque
500, and rejects unknown or non-editable scopes with a reason. configLocation
pointed at join(getCwd(), '.mcp.json') for every project-scoped server, naming
a file that does not exist for inherited ones; it now names the declaring file,
which is what tells a user where the server came from.
getProjectMcpConfigsFromCwd() has no callers left. Kept rather than deleted --
it is vendored upstream code and removing it widens future sync conflicts --
with its comment corrected, since it claimed to serve add/remove.
Verified over real HTTP against a server with an isolated CLAUDE_CONFIG_DIR,
not only through handleMcpApi: before the change the delete returned 500 with
the generic message and left the file untouched; after it returns 200, removes
the entry from the parent file, creates no .mcp.json in the cwd, and preserves
both $schema and the unexpanded ${MY_MCP_TOKEN} of a sibling server. The add
path was confirmed the same way. check:server: 226 files, 2405 tests, 0 failed.
One continuous thinking block arrived in the chat split across several
`已思考` bubbles, with nothing rendered between them. The report blamed
the model — DeepSeek — but the provider is not involved: the trace holds
a single well-formed thinking block, and reopening the session restores
it, because history is rebuilt from the transcript's own blocks.
The split comes from three pieces that are each correct alone. A
background (async) agent's tool activity is re-emitted as a normal
`tool_use_complete` / `tool_result` carrying `parentToolUseId`
(`handler.ts`, added in 1c554dc30 so background subagents stop showing
"no tool activity"). Those land at the end of `messages`. `MessageList`
then folds them into the parent agent card rather than rendering them
inline. But the merge test for a streamed thinking chunk asked whether
the *array tail* was thinking — and the tail was now a child tool call,
so every chunk after one started a new bubble. The split points are the
moments background activity arrived, which is why they look arbitrary
and why the gaps between bubbles are empty.
`findStreamMergeTargetIndex` skips those bubbled-child messages and
merges against the last real main-stream message instead. The same tail
test backed `appendAssistantTextMessage`, so a reply could be chopped
the same way; both now share the helper.
Reproducing needs thinking mode, a `run_in_background` agent, and child
activity landing mid-thought — rare enough that most sessions never see
it, and near-certain for a slow reasoning model made to wait on agents.
The screenshot shows exactly that: four subagents running, and the
mangled thought reads "The agents are still running. Let me wait a bit."
Covered by three tests: thinking stays one block across interleaved
child activity, a reply stays one block, and the main agent's own tool
call still opens a new block. Verified against the reproduction before
and after. Desktop suite 3050 tests green, tsc and build clean.
(cherry picked from commit 7d8a22f9465b8f987193dda7af5cae3858c481fc)
Independent QA marked three checks `not run` because they needed a human
at the keyboard. Each covers behavior this refactor changed, so they are
now assertions instead of intentions.
`A11Y-01` — a full keyboard tab-order audit. These components back
roughly 500 controls, so a decoration that picks up a tab stop or a
control that loses one would spread everywhere before anyone noticed.
`keyboard.test.tsx` fixes the property they must all share: interactive
components expose an exact number of tab stops, decorative ones expose
none, `disabled` and `loading` leave the sequence, a segmented control
stays one stop at any size, and every reachable control shows a focus
indicator.
That last check has two valid shapes, which the first version got wrong:
most controls style themselves, but `Switch` hides its native checkbox
under `peer sr-only` and paints the ring on the track beside it. Checking
only the focused node reported a missing indicator on a control that
draws one.
`ATT-02` — the image lightbox. Its arrows were icon-only with hardcoded
English before this branch; the existing suite only counted overlay
registrations. Now covers naming, wrap-around in both directions, arrow
keys, that the arrows disappear for a single image, and that keys stop
firing once closed.
`SEARCH-02` — stepping through find-in-page matches. `FindInPageModal`
had no `useTranslation` at all; its three controls are icon-only, so a
missing name leaves them unreachable by screen reader. Now covers naming,
the disabled state before a query matches, forward and backward wrap, and
close.
`CHAT-05` (queued message edit/delete) needed nothing — `ChatInput.test.tsx`
already covers it; QA had only skipped it by hand.
Each guard was verified by breaking the thing it protects: a `tabIndex`
on `Badge` fails the decorative check, and reverting the focus fix fails
the indicator check. Changed-lines coverage 91.32%.
Independent QA on 3264db10 flagged changed-lines coverage at 86.17%
(5539/6428), under the 90% gate. Two causes, handled differently.
`desktop/src/dev/` joins `mocks/` and `types/` in the coverage exclusions.
It holds the component gallery — 260 of the 889 uncovered lines, and by
far the largest single contributor. Vite never bundles it (the build
input is `index.html` alone), and unit-testing a page whose whole job is
rendering every primitive would assert that the primitives render, which
their own tests already do. Excluding it is a scope correction, not a
threshold adjustment.
The rest are three components this branch touched that had no test file
at all. They now have one each, covering the behavior that changed:
- `BackgroundTasksBar` — drawer open/close including Escape, the running
vs finished split, dismissed-key filtering, and that clearing reports
every finished key while keeping the drawer open if work continues.
- `TeamStatusBar` — the progress bar's `aria-valuenow`, lead exclusion
from both list and count, and that it greens on "nothing running"
rather than on 100%: one completed plus one errored is done at 50%,
which is why `tone="auto"` would have been wrong here.
- `MarketSkillDetail` — skeleton semantics, retry, install/uninstall by
`installState`, and the disabled+spinner state mid-install.
Changed-lines coverage: 91.06% (5610/6161).
The QA report's second finding, `check:impact` blocking on a missing
`allow-cli-core-change` label, is an artifact of the branch trailing
main. `check:impact` diffs against `main`, so main's own newer commits —
9 files under `src/` — are counted as this branch's. Against the merge
base the same evaluator returns `areas: desktop, blocked: false`. No code
change here; the branch needs a rebase before it can pass that lane.
`components/AGENTS.md` is the reference: what to use for a given need,
where a new component goes, and the style / i18n / a11y / test rules.
`desktop/AGENTS.md` now routes here — the line it replaces ("reuse the
existing desktop design system") named nothing to look up and so was not
an executable instruction for a person or a model.
`docs/component-library-plan.md` keeps the audit evidence and a record of
what actually shipped, including where the plan was wrong.
Two rules earned their own sections because the library broke them
itself:
- Overriding a component's utility with `className` does not reliably
win. Tailwind sorts same-utility arbitrary values by value and takes
the last, regardless of the order they were passed. `hoverTone="danger"`
was a no-op with exactly the two tones it was built for, and its test
passed because it only asserted the red class was *present*, never that
the neutral one was gone. Assert that a class prefix appears exactly
once, and prove it by reverting the fix.
- The library must not compose user-visible English. `SearchField` built
`Clear ${label}`, which made adopting it an i18n regression for every
caller that had already translated its clear button. The first repair —
falling back to `label` — was worse: the input and its clear button
then shared an accessible name and `getByLabelText` matched both.
Reuse went from 22% to 75% (524 component uses against 179 remaining
native buttons). The remainder are elements that should not be
components: `role="tab"`, `menuitem`, `option`, `treeitem`, `gridcell`,
whole-row and whole-card click targets, drag handles, and the OS
titlebar.
Renders every `components/ui` primitive under each of the three themes at
`/gallery.html`, reachable under `bun run dev` with no backend.
Unit tests assert structure and ARIA. They cannot tell whether a token
resolves to a readable color, whether an overlay lands above the thing it
is meant to cover, or whether an entrance animation actually plays. Three
defects surfaced here that had a full green suite behind them:
- `Button` and `IconButton` swallowed refs, so `Tooltip` had nothing to
measure and `Dropdown` could not return focus. Console warning only.
- The light theme's warning badge sat at 2.66:1.
- `Dropdown`'s Escape closed the dialog behind it as well.
Vite's build input is `index.html` alone, so this never enters the
production bundle.
`mockPaths.test.ts` asserts every `vi.mock` specifier resolves. A mock
pointing at a moved module is not an error — Vitest registers the factory
against a specifier nothing imports, the real module loads, and the test
keeps passing with its isolation quietly gone. That is what happened to
`RepositoryLaunchControls.test.tsx`, which kept mocking `./DirectoryPicker`
after the component moved to `composite/`, so the real picker and its API
calls rendered in a test that believed they were stubbed.
Suite updates for markup that legitimately changed shape:
- Dropdown entries are `role="option"` now, not buttons. `<button>` is
invalid inside a `role="listbox"` and left the dropdown without arrow
keys; the assertions follow the corrected semantics.
- `PetSettings`' toggles report `role="switch"`, the ARIA role for a
toggle. They were bare checkboxes while `McpSettings`' equivalent was
already a switch — the two are now consistent.
- `pages.test.tsx` drops its references to the two deleted mock pages.
Native buttons in `pages/`: 88 -> 33.
An earlier pass over this directory converted only 15 sites and reported
the blocker precisely: `Button` offered h-5/h-6/h-9/h-10 while nearly
every page button is h-8. Adding `base` unblocked 17 conversions on its
own. `IconButton`'s `bordered` and `hoverTone` unblocked the trace row
actions, whose treatment is transparent-with-a-border and whose delete
must not sit red at rest.
- `ComputerUseSettings` moves 21 stock palette classes onto token pairs,
retiring 11 alpha modifiers with them. Its allowlist entry is gone
rather than reduced.
- `ActivitySettings`' hand-rolled `createPortal` dialog becomes `Modal`.
- `TraceList`'s `RowAction` and `TraceSession`'s `IconAction` helpers are
deleted in favor of `IconButton`.
Three dead files removed:
- `pages/NewTaskModal.tsx` — zero importers; the live one is
`components/tasks/NewTaskModal.tsx`. It was also the largest remaining
holder of the deprecated `--color-primary`.
- `pages/ToolInspection.tsx` and `pages/AgentTeams.tsx` — neither appears
among the nine pages `ContentRouter` and `AppShell` import. Only
`pages.test.tsx` referenced them.
`Settings.tsx` keeps two stock palette classes. They sit inside a
hardcoded white QR box that scanners need for contrast; theme tokens go
light-on-white under the dark theme. Converted, reverted, and commented.
Left native (33): settings nav rows, the dnd drag handle, tree nodes,
`role="tab"` and `role="gridcell"` elements, and the theme/language
pickers — `SegmentedControl` emits radio semantics while two tests pin
`role="button"` with `aria-pressed`, which is an a11y decision rather
than a styling one.
Native buttons across these directories: 42 -> ~30.
- `browser/` moves off hardcoded Chinese onto `useTranslation()`.
- `AgentManager`'s `MetaPill` becomes `Badge bordered` across 11 render
sites; its tool search becomes `SearchField`.
- `trace/` filter chips become `SegmentedControl`, which adds arrow-key
navigation and a `radiogroup` they never had.
- `PermissionModeSelector` had a hardcoded DOM id rendered in two places.
`useId()` alone was not enough — a nested duplicate `role="menu"`
wrapper still emitted it twice, so that came out too.
- `ReasoningEffortPopover` defaulted its `ariaLabel` to a Chinese
literal, announcing Chinese in every locale to any caller that omitted
it.
- `DoctorPanel`'s status label was bare `--color-warning` text, an AA
failure; `Badge tone="warning"` uses the on-container pair.
- `TeamStatusBar`'s progress bar becomes `Progress`, retiring the twin of
the one in `SessionTaskBar` — they matched down to the comment.
- `PetApp`'s error banner leaves the stock rose palette for error tokens.
`Progress` here uses an explicit tone rather than `tone="auto"`: this bar
turns green when nothing is *running*, which is not the same as 100% (one
done plus one errored is green at 50%).
`ModelSelector` and `PermissionModeSelector` get only the safe changes —
outside-click and icon buttons. Their selection logic is untouched; both
couple five to seven stores and sit on the highest-frequency path in the
app.
Left native: tree nodes, `role="menuitem"` rows, model option rows,
heatmap cells, and the pet sprite controls.
Native buttons across the five directories: 37 -> 14. This is where the
badges and empty states were densest.
`ConfirmPopover` moves in from `shared/` to `tasks/` — one caller.
- `InstallStateBadge` and `SecurityBadge` become their status maps and
nothing else, which is what `Badge` exists for. Both also had
`success` text on a `success-container` fill; the tone pair fixes a
contrast failure neither had noticed.
- Long-content badges (hook matchers, package specifiers, file paths)
needed `wrap` — badges are single-line by default and these overflowed.
- `TaskRow`'s hand-rolled `mousedown` becomes two `useDismissable` calls,
one per overlay. Escape now closes them, which it did not before.
- `MarketHome`'s error banner keeps its layout but drops
`border-[…]/35` + `bg-[…]/25` for solid tokens and gains `role="alert"`.
Those alpha modifiers meant the banner had no panel at all on Safari 15.
One real regression fixed: `FilterBar`'s filter chips were dead. When
`Dropdown` started cloning its trigger to attach ref, aria state and its
own `onClick`, `FilterTrigger` — a plain function component forwarding
none of it — silently swallowed them, and clicking a chip did nothing.
It now forwards. `FilterBar.test.tsx` is the regression anchor; verified
it fails with the spread removed.
Left native (14): `role="tab"` and `role="option"` elements, whole-card
and whole-row click targets, and `DayOfWeekPicker`'s circular day
toggles.
Native buttons across the three directories: 81 -> 49.
`Toast` and `UpdateChecker` move in from `shared/` — one caller each,
both application singletons that belong with the shell.
`Sidebar` was the hard case. Its 21 icon buttons hover to
`--color-sidebar-item-hover`, which differs from `--color-surface-hover`
in all three themes (it is a translucent white tuned for the sidebar's
gradient). Overriding via className means two arbitrary `hover:bg-[…]`
values competing, with the winner decided by stylesheet order — so
`IconButton` grew a `surface="sidebar"` instead.
Both local `ToolbarIconButton` adapters are deleted; `pressed` was the
only thing they still added, and it brings `aria-pressed` they never had.
`ProjectHeaderMenu` gains `forwardRef` so its four menus can share one
`useDismissable`. Their triggers are click toggles, so `triggerRef` is
what keeps a second click from reopening what it just closed.
Three retry buttons drop five `/N` alpha modifiers on the way.
Left native (49): menu items, tab items, tree nodes, file rows, the OS
titlebar controls, `WorkspaceDiffSurface`'s cells, and the two sidebar
collapse toggles — their `sidebar-toggle-button` class carries an active
transform and drives the chevron animation, not just size and radius.
`TitleBar.tsx` is deleted: zero importers. The audit listed its settings
button among the unnamed controls, but it never rendered.
Native buttons in `chat/`: 79 -> 47.
`ProjectContextChip` and `RepositoryLaunchControls` move in from
`shared/` — one caller each and two, so they belong beside the composer
rather than in a directory named for common ground.
Notable conversions:
- `ChatInput` and `EmptySession` carried 59 byte-identical lines of
outside-click handling, four effects each, `diff` output empty. Both
now call `useDismissable`.
- `StreamingIndicator`'s retry banner moves off the stock amber palette
onto the warning token pair. Its `dark:` variants disappear with it —
they only ever covered one of the three themes.
- The composer's edit/delete pair: delete rests muted and reddens only on
hover, since a delete icon that is red at rest reads as an error state.
- Two badge maps lose their `/N` alpha fills, which Safari 15 WebViews
drop outright.
The run button's arrow now trails its label, matching the same control
on `EmptySession`. The two were mirror images of each other — caught by
opening the app, not by any test. `git show HEAD~5` confirms this
predates the refactor.
Left native (47): `role="option"` rows, tab strips, full-bleed disclosure
headers with no radius, the session inspector's private
`--color-inspector-*` palette, and the composer's plus/submit buttons —
`ChatInput.test.tsx` pins those to 44px, and submit carries a dual fill
(error container while stopping, brand gradient otherwise) that no
variant expresses.
`MessageList` keeps its structure untouched; its virtualization and
`content-visibility` paths are not worth risking for a button.
48 new keys across all five locales (en / zh / zh-TW / jp / kr), and the
call sites switched to `useTranslation()`.
Two classes of problem:
- Buttons with no accessible name at all: the tab-strip scroll arrows and
the terminal folder-browse control. They needed keys before they could
be named, which is why earlier passes left them unnamed.
- Hardcoded literals reaching users: English in `WindowControls`,
`MermaidRenderer`, `ImageGalleryModal`, `CodeViewer`, `FindInPageModal`
(which had no `useTranslation` at all), and Chinese in all of
`browser/`. A hardcoded Chinese `aria-label` is the same defect as a
hardcoded English one — it just fails for a different audience.
Also repoints two keys borrowed from the wrong namespace, and drops the
`scheduledPage.col*` family (5 keys x 5 locales) left orphaned when the
table layout became cards.
`TranslationKey` derives from `en.ts`, so a missing key in any locale is
a compile error rather than a runtime blank — the key-set parity test
covers the rest.
Components that legitimately reach into stores or the API — the thing
`ui/` forbids — but that several features share. `DirectoryPicker` (5
callers) and `OpenWithMenu` (3) move here from `shared/` and `common/`.
The entry price is one rule, and `contract.test.ts` enforces it: two or
more callers. `shared/` had no such rule and ended up 35% single-caller
components, each really a private part of one feature sitting in a
directory that advertised itself as common ground.
`OpenWithMenu` also switches to `useDismissable`, which was ported out of
it in the first place — it held the most complete overlay behavior in the
codebase. It keeps `mousedown` explicitly rather than taking the hook's
`pointerdown` default, because two call sites document that contract and
three tests assert `fireEvent.mouseDown`; flipping it belongs with those
files.
24 primitives, of which 7 move in from `shared/` unchanged in behavior
and 17 are new. What they replace, by count from the audit:
- `Button` — 362 native buttons vs 102 uses of the old one, a 22% reuse
rate. It was bypassed for a reason: no focus ring, no icon-only mode,
and none of its three sizes pinned a height, which is the direct cause
of 18 distinct button heights. Sizes now pin `h-*` reproducing what the
old padding rendered, so adopting it moves nothing.
- `IconButton` — the largest single category: 76 icon-only buttons across
40 files, in 15 radii and 12 sizes, with a focus ring on 41% and ten
with no accessible name at all. `label` is required rather than
optional-with-a-fallback, because a fallback lets unnamed buttons keep
shipping.
- `Spinner` — resurrects the dead `shared/Spinner.tsx` (zero importers)
whose SVG path was character-identical to a private copy inside
`shared/Button.tsx`, two files apart.
- `Badge` / `StatusDot` — 45 pills and 41 dots. `market/InstallStateBadge`
and `market/SecurityBadge` had character-identical shells differing
only in their status map.
- `EmptyState` / `ErrorState` / `LoadingState` — 22 + 12 + 19 sites, plus
five local components sharing three names with incompatible props
(`{icon, text}` vs `{title, body}`).
- Input family — of 76 native inputs, `focus-visible:` appeared zero
times, `disabled:` styling zero times, `aria-invalid` once, and three
had no visible focus at all. Ids now come from `useId()`; deriving them
from label text collides for any two Chinese labels, since the
space-stripping is a no-op there.
- `SegmentedControl` — 10 implementations with 6 active-state treatments,
five of them inside `pages/Settings.tsx` alone. None of the four
`role="tab"` sites had a `tablist` parent or arrow keys.
- `Dropdown` — keeps its API but gains listbox semantics, arrow keys and
focus return. Its trigger is now cloned to carry the ref and aria
state, so it must reach a real DOM element; a dev-only assertion warns
when a custom trigger swallows the props, which is otherwise silent.
- `Skeleton` / `Card` / `Progress` / `Tooltip` / `Switch` / `Checkbox`.
`contract.test.ts` encodes what makes this directory different from
`shared/`, which failed precisely because its name carried no rule: no
store or api imports, no imports from sibling feature directories, no
hardcoded hex, no Tailwind radii (`rounded-lg` is 8px while
`--radius-lg` is 12px — same name, different value).
First of a series that extracts a component library out of the desktop
UI. Intermediate commits in this series do not build on their own — the
directory move and the call-site updates cannot be separated per file.
Theme layer fixes, all previously invisible to any test:
- `dark:` compiled to `prefers-color-scheme` and so followed the OS, not
`<html data-theme>`. With the OS in light mode the variant never fired,
which is why the streaming retry banner rendered near-white under the
dark theme. Bound it with `@custom-variant`.
- Six tokens were referenced but never defined, leaving borders falling
back to `currentColor` and panels transparent. Five were renamed to the
tokens they meant; `--color-on-primary-container` was genuinely missing
and is now defined per theme.
- Status colors now come in pairs: `--color-<tone>-container` with a
matching `--color-on-<tone>-container`. Using the accent as its own
foreground measured 2.66:1 in the light theme, well under AA.
- One ordered z-index scale replaces 17 ad-hoc values. Two orderings are
load-bearing: dropdowns sit above dialogs (a modal blocks the page, so
an open dropdown belongs to the topmost dialog — this is why
DirectoryPicker had to hardcode `zIndex: 9999`), and toasts sit above
sheets (otherwise feedback raised from a sheet is invisible).
- `animate-in slide-in-from-*` came from `tailwindcss-animate`, removed
in the shadcn rollback. Toast and Dropdown kept the classes and lost
their entrance animation entirely; real keyframes now back them.
- The embedded terminal hardcoded 20 hex values, bypassing the per-theme
`--color-terminal-*` tokens that were already defined. `terminalTheme.ts`
resolves them for xterm, which cannot read CSS variables itself.
Behavior:
- `useDismissable` replaces 21 hand-rolled outside-click effects, split
across `mousedown` (14), `click` (3) and `pointerdown` (3). It defaults
to `pointerdown`: `mousedown` is unreliable for touch, which is the
shape of the "tapping outside doesn't close the menu" reports on H5.
- `useAnchoredPosition` merges four viewport calculations whose margins
had drifted to 8, 8, 12 and 0, only one of which flipped.
- `chat/clipboard.ts` moves to `lib/` — it had a base primitive
(`CopyButton`) depending on a feature directory.
Guards, because conventions in prose do not hold:
- `tokenUsage.test.ts` — every `var(--token)` resolves, and no raw
z-index escapes the scale. A typo'd token throws no error; it just
renders transparent.
- `contrast.test.ts` — computes real ratios for 3 themes x 6 tones and
requires AA. Verified by reverting the fix and watching it reproduce
2.66:1 exactly.
- `paletteEscapes.test.ts` — a ratchet on stock Tailwind palette classes,
which do not follow `data-theme` and which `tokenUsage` cannot see
because they are not `var()` references.
Desktop denials answered the CLI with `interrupt: true`, which
permissionPromptToolResultToPermissionDecision turns into
toolUseContext.abortController.abort(). query.ts then hits its post-tool abort
check, yields the interruption marker and returns `aborted_tools` -- the denial
was written to the transcript, but no follow-up request was ever made. The
model never learned it had been denied and never got to reply, so a rejected
tool ended the turn in silence.
#1051 named the root cause directly, and it was not a missing abort: the denial
text was 'User denied via UI', a debug string the model cannot act on, where
upstream sends REJECT_MESSAGE ("... STOP what you are doing and wait for the
user to tell you how to proceed."). Aborting suppressed the reported symptom --
the model could not go looking for a workaround because it was never asked
again -- while leaving in place a denial it still could not read. Send the copy
the CLI itself uses and the instruction lands where it belongs.
ExitPlanMode carries the opposite meaning: rejecting a plan is "keep planning",
so it must not receive REJECT_MESSAGE's STOP. It now gets copy saying the plan
was rejected and should be revised, in place of that same debug string.
Deliberately not PLAN_REJECTION_PREFIX -- its contract is the prefix followed
by the full plan text, and both renderers already read the plan from the tool
input (the CLI's renderToolUseRejectedMessage, the desktop's
extractPlanPreview), so reusing it would only echo the model's own plan back
into context. Plan feedback gets the same framing instead of being passed
through bare, where "add rollback steps" reads as a go-ahead.
REJECT_MESSAGE and REJECT_MESSAGE_WITH_REASON_PREFIX move to the
dependency-free constants/messages.ts so the server can share the CLI's exact
wording without pulling utils/messages.ts and its analytics deps into the
sidecar. utils/messages.ts re-exports them, leaving existing callers untouched.
Verified end to end against a mock upstream driving the real server and a real
CLI subprocess: the model is asked a second time, that request carries the
denial as tool_result, and the model replies. Restoring `interrupt: true` leaves
the upstream with a single request and no closing reply, which is the reported
symptom. A denied Write reaches the filesystem in neither case.
permissionPromptToolResultToPermissionDecision had no coverage at the point
where it decides to abort; it does now.
The mascot sits at the bottom of a mostly transparent 384x400 window, so
clamping a drag against the mascot rather than against the whole window
deliberately asks for a negative y that pushes the padding above it off-screen.
macOS runs a visible window's frame through
-[NSWindow constrainFrameRect:toScreen:], which rewrites any y above the work
area back down to its top edge -- silently. The request was refused, the
controller never noticed, and the mascot stranded a padding-height below the
menu bar: 208px of dead band on this display, with
~/.claude/cc-haha/pet-window.json recording y=-175 for a window that never
left y=33.
Measured on Electron 42.7.0 / Darwin 25.4: only the top edge is constrained.
Off-screen x and off-screen bottom y are kept verbatim, which is why the other
three edges always worked. Nothing else escapes it -- not window level (all
eight, up to screen-saver), movable, frame, transparent, panel type, and not
positioning the window while hidden, since showInactive re-runs the constraint.
enableLargerThanScreen is the one switch that skips the method, and it leaves
size, getContentBounds and ordinary moves untouched. clampPetWindowPosition is
already a complete four-edge bound, so opting out makes it the only clamp
rather than removing one.
Restoring then exposed a second problem the constraint had been hiding. A saved
position deliberately leaves the padding off-screen, but the window is created
before the renderer reports any region, so getPetWindowBounds clamped it against
the whole window and opened a padding-height lower -- a jump of zero while the
window was pinned, 208px once it is not. Persist the mascot box next to the
position so the first frame lands where the drag left it. State written without
one still restores the way it always did, and an empty box is dropped rather
than trusted.
The fake window accepted every y, so no top-edge assertion could fail and the
suite stayed green through all of this -- the same blind spot that let the
Windows DIP bugs hide. It now applies the constraint the way AppKit does, only
while visible and only when the constructor options did not opt out, so the
assertions prove the fix rather than the platform. Reverting either half turns
the new cases red.
c2774fc1 routed every pasted file through its native path so non-image
files could be attached, and `pathToComposerAttachment` labels all of
them `type: 'file'` with no preview source. AttachmentGallery only draws
an <img> when `previewUrl` or `data` is set, so a pasted image rendered
as the generic file card -- until the tab was reopened, when the same
attachment came back from the transcript's inlined image block with
base64 `data` and displayed correctly. Live and restored display had
drifted apart.
Re-inlining the bytes is not the fix: ba28e444 made desktop attachments
path-only after a 12-file send inflated the payload to 50MB, and that
still holds. attachmentImages resolves an image attachment's source
instead -- previewUrl, then data, then an absolute path streamed back
through /api/filesystem/file, the endpoint InlineImageGallery already
uses for model-produced images. The websocket payload stays path-only.
The extension set deliberately matches ConversationService's
shouldInlineImageAttachment rather than everything that endpoint can
serve: promoting .svg or .avif to `type: 'image'` would make the server
inline a media type the API rejects. Those keep the file card, as does
an image whose preview cannot load -- a moved file, or a path outside
the roots the local server may read.
What reaches the model is unchanged; the server already inlined pasted
images by extension, so this `type` never decided that.
SKILL.md opens with a YAML frontmatter block. Passing it straight to the
markdown renderer made the closing `---` a setext heading marker, so the
whole block rendered as one giant bold <h2> above the document.
Split the block out before rendering and lay it out as structured rows.
Where it goes differs by context:
- The detail sidebar, next to the market metadata. `slug`, `license` and
`allowed-tools` are the same kind of data as `author` and `category`,
just from a different source, and it is reference material -- it should
never precede the document that explains what the skill does. Short
scalars mirror the meta rows above them; arrays and long text stack,
because 300px cannot do both on one line.
- The file preview, as the file's header block. There the frontmatter
genuinely is the top of the file being read, so removing it would
misrepresent the file; it is styled as a header rather than a card so
it does not compete with the body.
This also covers SkillHub, whose `description` is the raw SKILL.md -- so
the overview tab hit the same wall of bold text -- and puts ClawHub's
already-parsed `descriptionFrontmatter` to use for the first time. Local
installed skills previously flattened frontmatter into sidebar rows as
comma-joined strings; they now share the same panel.
The parser is a deliberately small YAML subset (scalars, inline and block
sequences, block scalars) rather than a new dependency, since skill
frontmatter is flat by convention. It never throws: a malformed or
unterminated block returns the document untouched, and nested mappings
are kept as raw YAML so nothing is silently dropped.
Separately, the files tab was capped at a hard-coded 520px, leaving the
bottom of the page empty. `min-h-full` on the flex column was not enough:
a container sized by min-height keeps `height: auto`, so `flex-1`'s
`flex-basis: 0%` cannot resolve and the panel falls back to content
height -- measured 1215px of content inside a 720px viewport. A definite
`lg:h-full` makes the panel claim the leftover space. Below lg the page
scrolls as before.
Walked through in a browser at three widths in both themes. check:desktop
is green: 227 files, 2562 passing.
4c3b08c0 made `isLocalTrustedRequest` return early on
`localAccessTokenConfigured`, so once the desktop shell injects
CC_HAHA_LOCAL_ACCESS_TOKEN the loopback address stops being evidence of
anything -- only the token grants trust. The system browser never has it.
`shell.open`ing the Grok OAuth success page landed on
{"error":"Unauthorized","message":"Missing H5 access token"} instead of
the success HTML, and /preview-fs links, /local-file links and plain curl
went the same way. With H5 access off it was worse: 403 "H5 access is
disabled", which no H5 token can clear.
The classifier had no way to tell the two apart. A scenario matrix over
the policy functions returned the identical verdict for "system browser
opens the OAuth page" and "malicious site fetches 127.0.0.1" -- the rule
bought its safety by refusing every request that is not the app itself.
What that early return actually added over the checks below it was one
case: a reverse proxy on this machine perfectly mimicking loopback. A LAN
client is already rejected on clientAddress, a cross-site fetch on Origin,
and a tunnel forwarding its original Host on isLoopbackHost(url.hostname).
Reaching the remaining case means running a process on the user's box,
where ~/.claude is readable anyway. Loopback goes back to being trusted on
its own.
The boundary worth keeping is narrower than the whole surface, so
requiresLocalAccessCredential pins it to /api/h5-access: another program
on this machine must not be able to publish the user's sessions to the
network. /api/h5-access/verify stays open -- checking a token the phone
already holds is not a control-plane change -- and a server with no token
configured does not lock itself out.
Restoring loopback trust re-exposed an older hole that the blanket rule
had been covering: `if (!origin) return true` also accepts cross-site
subresource loads, since <img src="http://127.0.0.1:3456/api/...">
carries no Origin. Fetch Metadata separates them -- a real navigation
sends Sec-Fetch-Mode: navigate, a subresource does not -- and clients
that send no Fetch Metadata at all (curl, adapters, the CLI subprocess)
are not a browser CSRF vector, so they stay trusted. This is net new
protection; it existed neither before 4c3b08c0 nor after.
desktopRuntime resolved the token inside a Promise.all with getServerUrl,
so an IPC failure took the whole startup down with it. It degrades to
null now, which is the same principle: the token raises what the shell
may do, it is not a precondition for running.
Verified end to end against a server started with the token injected: the
OAuth success page returns 200 and its HTML, /api/status, /api/sessions
and /api/adapters return 200 tokenless, while the control plane, an <img>
subresource, a cross-site fetch and a proxy hop all return 403.
check:server is 224 files / 2379 passing, desktop 225 files / 2522.
pluginStore applies a plugin change, reloads the runtime, and resolves
either way -- a failed reload is recorded in `refreshWarning` and the
action still returns its success message. Nothing reads that field, so
"disabled the plugin but the runtime never reloaded" reached the user as
a plain success toast, indistinguishable from a clean run.
The mutation failing still throws and still surfaces, so this was not
fully silent; the half where the change lands but the reload does not
was. PluginDetail now reads `refreshWarning` after the call -- the store
clears it when a mutation starts, so anything present belongs to that
action -- and reports a warning naming the reload failure.
Kept in the component rather than the store: no store in this codebase
reaches into useUIStore, and adding the first such call to fix a toast
is not the place to introduce that.
5b4e224f deleted these four as part of the migration cleanup; e9b53f68
rolled back its UI half and brought them along. They were already dead
at c2774fc1 -- the pre-shadcn baseline -- so this restores a cleanup
rather than removing anything the rollback needed.
Nothing imports them: neither a relative nor an @/ import of any of the
four appears anywhere under desktop/src, tsc is clean, and the suite
still reports 2520 passing. The other files the same revert restored
stay: shared/Button (23 importers), shared/Input (7), shared/Dropdown
(4) and pages/NewTaskModal (3) are all live under the reverted UI.
b64069a3 removed the installed-skills overview and its nine
market.installedSkills.* / market.section.installed keys from all five
locales. e9b53f68, its immediate child, reverted the locale files
wholesale -- and because the overview (8061b9b2) predates the shadcn
migration those keys were inside the range being restored, so they came
straight back.
The component that read them is gone, and nothing else references them:
`git grep -E "market\.installedSkills|market\.section\.installed"`
outside the locale files returns nothing. Every locale kept the same
key count, so a parity check has no way to notice.
Re-applies b64069a3's deletion, and adds a test that fails if the keys
reappear -- the previous deletion was undone by a mechanical revert, so
a guard is worth more here than the removal itself.
`preview-agent.js` is a tracked build artifact, bundled from
desktop/src/preview-agent/ by build:preview-agent and shipped as a
packaged resource. The rollback (e9b53f68) deleted that source's
theme.ts and trimmed editBubble.ts, but never rebuilt the artifact --
it appears in neither revert's file list, so the committed blob was
last produced at ad597ffe.
The result is a bundle built from source that no longer exists: it
still carries all 12 --cc-haha-* custom properties the deleted theme.ts
defined, plus the shadcn-era brand hex, and lacks the literal styling
the current source emits. It is internally consistent so nothing
crashes -- the packaged app just renders styling that diverges from
what the repository says it should.
Rebuilt from the current source. Verified byte-stable across repeated
builds, so this does not reintroduce artifact drift.
The shadcn migration (ad597ffe) and its follow-up (5b4e224f) were
mega-commits: only ~16% of the insertions were UI work. Rolling either
one back wholesale also deletes ~3400 lines of unrelated server,
Electron, and store hardening -- and because each of those changes
shipped with its own tests, the suite stays green while the hardening
silently disappears.
This rolls back the UI layer only.
Reverted (back to c2774fc1):
- desktop/src/components, pages, features, theme, i18n
- components/ui/**, components.json, src/lib/utils.ts (cn helper)
- radix-ui, class-variance-authority, tailwind-merge deps
- src-tauri/src/lib.rs hardening: dead code, nothing compiles it
(no @tauri-apps dep, no cargo invocation anywhere, and
scripts/pr/release-workflow.test.ts asserts Tauri's absence)
Kept (identical to main):
- src/** -- WhatsApp authDir escape allowing arbitrary recursive
directory removal, computer-use tier bypass granting every
pre-authorized app full tier, plugin uninstall keepData inversion
that deleted data when asked to keep it, plugin project-scope
writing to the sidecar cwd, diagnostics share leaking provider
config, memory API TOCTOU plus atomic writes
- desktop/electron/** -- PTY leak on renderer destruction, IPC
boundary validation rejecting NaN and unbounded input
- desktop/src/stores/** -- request-id race guards
- desktop/src/api, lib/desktopHost -- kept in step with the stores
Follows main's b64069a3 in dropping the installed-skills overview.
Updated two memorySettings assertions to expect the optimistic-lock
fields the retained memory store sends.
check:desktop pass (lint clean, 225 files / 2519 passed / 1 skipped,
build ok). check:server pass (224 files / 2375 passed / 0 failed).
The overview added in #1098 duplicated Settings -> Skills: same
useSkillStore, same fetchSkills/fetchSkillDetail, and it even reused the
settings.skills.* translation keys. Its only new logic collapsed the five
skill sources into a personal/system pair, while "is it installed?" was
already answered by InstallStateBadge on each market card. In exchange it
pushed the actual catalog below the fold behind a 93-row list.
Also reverts f82b596f, which existed solely to give that overview a
working detail view; with the entry point gone it was dead code.
Restores desktop/src/pages/Market.tsx and MarketHome to their pre-#1098
shape (verified: empty diff against f82b596f^ and 6b7e90ae^1), and drops
the nine market.installedSkills.* keys from all five locales.
A skill name is a single slash command, but three code paths disagreed on
which file it pointed at once the same name existed in more than one root.
The CLI loader deduped by `${scopeKey}\0${name}`, so it only collapsed a
collision inside one scope. Across scopes both entries survived, and the
slash command menu has no dedup of its own -- typing `/` listed two identical
rows, and only the first was ever executable.
The desktop menu builds its list separately, in listSkillSlashCommands, where
a plain `byName.set()` made the last writer win. Its input runs user roots
before project roots, so the project copy took the name. The kernel resolves
the same call through findCommand, which takes the first match and therefore
picked the user copy. A user with `deploy` in both ~/.agents/skills and the
repository's .claude/skills read the project description in the menu and ran
the .agents body on click.
Both paths now share outranksClaimedSkill: `.claude` outranks `.agents` in
every scope, including across them, and any other pairing leaves the incumbent
in place so the existing root order still decides. This repository is the
Claude Code CLI, so a skill installed for us stays authoritative over one
another client dropped into the shared .agents space. Plugin and legacy
/commands/ entries carry no flavor and neither displace nor get displaced on
flavor alone -- which also settles a second disagreement, where the desktop
list let a plugin skill take a name the CLI gave to the user's own.
The --bare branch loads only --add-dir roots and had its own scoped helper;
it now runs the same rule, so two added directories holding one name collapse
like everywhere else.
Behavior change: a name repeated across scopes used to produce two entries,
and a regression test pinned that. Two identical rows give the reader no way
to tell them apart while only one can run, so the pair now collapses; the
test is replaced by the cross-scope cases that assert the surviving copy.
Verified end to end in a sandbox HOME plus project: for same-scope,
cross-flavor, and cross-scope-same-flavor collisions, the desktop menu, the
CLI menu, and findCommand all land on one file and agree on which.
Clicking an installed skill in the Skills Market only wrote
skillStore.selectedSkill, but the page renders details exclusively
off marketStore.selectedId, so the click had no visible effect.
Render the shared SkillDetail when an installed selection exists,
mirroring the Settings -> Skills wiring: hidden (not unmounted)
market home to preserve search/collapse state, and the same
active-project context guard that drops stale details.
Follow-up to #1098.
The resolveElectronExecutable case asserted against the real
node_modules/electron/dist, which bun never downloads without the
electron postinstall, so it failed on macOS/Linux dev machines.
Point the tests at a self-built fixture desktop root instead and
extend coverage to the darwin/linux candidates and the missing-
executable error path.
Follow-up to #1096.
Follow-ups to 9ef170bb, where the discovery path itself is sound but three
surfaces around it are not.
The desktop badge showed the same tooltip for both scopes, and that tooltip
names ~/.agents/skills. A skill that ships inside a checked-out repository was
therefore described as one the user had installed in their own home directory
-- the least trusted source presented as the most trusted. Project-scope
skills now get their own copy, in all five locales.
The doctor listed both .agents targets unconditionally, so a user who set
disableAgentSkillsDirectory (or the env var) still saw them reported, with an
entry count if the directory had contents. That reads as "these skills are in
use" when the loader is ignoring them.
Skill directory names went into the catalog the model sees verbatim, one
`- name: description` line per skill, with no validation. A name containing a
line break writes entries of its own -- and .agents/skills is a directory
other clients and repositories write into, so the name is not necessarily the
user's. Names carrying control characters are now skipped. Deliberately
narrower than the Agent Skills spec's lowercase-and-hyphens rule: existing
.claude/skills directories predate it, and silently dropping one of those
would be the worse failure.
Also collapses the two spellings of the scoped dedup key -- the main loader
joined with a raw NUL byte embedded in the source, the --add-dir path with a
space -- into one helper, so the comment claiming both apply the same rule is
true and the invisible byte is gone.
Adds coverage for the two scopes 9ef170bb claimed but left untested: the
hot-reload watcher (no test file existed for it at all) and on-demand monorepo
discovery. Both were already correct; nothing was holding them that way.
8a3ea62b taught dragging to clamp against the mascot rather than the whole
window, so a saved edge position deliberately leaves the transparent padding
off-screen. Recreating the window re-clamps that position against the whole
window again, and the reposition-on-restore step that undoes it was gated on
darwin -- so on Windows and Linux the mascot walked inwards by the padding
width on every hide/show, not just on restart. The gate only existed because
visibleDragRegion used to be darwin-only; the region now arrives everywhere,
so the repositioning runs everywhere.
e24d59fe restated the recorded window size on every drag tick to stop
setPosition from growing the window, but sampled that size from getBounds().
Chromium converts window rects between physical pixels and DIP with ceil in
both directions, so the sampled value has already been rounded up once and
restating it rounds up again: stable within a drag, a pixel per drag across
them. The window is created non-resizable at the nominal size, so restating
the constants is idempotent instead -- and heals a window that already drifted.
Both call sites now go through movePetWindow.
petWindowContentExtent fell back to the nominal constants when
getContentBounds returned an empty rect, which silently reinstates the exact
clamp it exists to avoid. Fall through to the live window box first.
The fake window in the tests returned one object for both getBounds and
getContentBounds, and modelled setBounds as inherently size-neutral -- so
neither "clamp to the content box" nor "size-neutral drag" was actually
proven: reverting either left all 33 tests green. It now models the ceil/ceil
DIP round trip and carries a distinct content box, and covers the previously
untested paths: cross-platform restore, multi-region shapes, the region
normalization bounds, and the getContentBounds fallbacks.
- Prevent EmptySession crash when settings have not loaded: fall back
to the default permission-mode display and an empty model list, and
stop undefined models responses from poisoning the settings store
- Sync Tauri terminal hardening with Electron: owner binding, input
limits, window-destroy session cleanup, malformed config fallback
- Fix Sheet/MobileBottomSheet z-order so dialogs render above sheets
- Migrate ConfirmPopover to the shadcn Button; remove dead legacy
shared form components and unused mockup pages
- Keep underscores readable in shared diagnostics, redact provider
models only on secret-scanner hits, clean stale WhatsApp login
staging dirs, and add recovery hints for corrupt computer-use config
- Token-ize find-in-page highlight colors, restore the indeterminate
progress slide animation, and tidy a11y/displayName details
Skills installed by Codex, Cursor, Gemini CLI, or opencode live under
`.agents/skills/` -- the convention documented at agentskills.io -- so users
had to keep a duplicate copy in `.claude/skills/` before cc-haha could see
them. The spec only constrains SKILL.md itself, which we already parse, so
this is purely a directory-discovery gap.
Scan both conventions at every scope: user (`~/.agents/skills`), project
(every level up to the repo root), `--add-dir` roots, on-demand monorepo
discovery, and the hot-reload watcher. skillRoots.ts is the single source of
truth for those paths; the REST API now reports which convention a skill came
from and the desktop list badges the `.agents` ones.
`.claude` is always ordered first, so first-wins dedup keeps a user's own
skills authoritative rather than letting an externally installed same-named
skill shadow them. Name collisions are collapsed per scope, which covers
copy-based sync -- symlink-based sync was already handled by the existing
realpath dedup. Collisions across different scopes, such as a user skill and
a project skill sharing a name, keep their existing behavior, and the shadowed
file's identity is recorded so overlapping roots (CLAUDE_CONFIG_DIR pointing
inside the project) cannot reintroduce it under the other scope.
getProjectDirsUpToHome is untouched -- it is shared by commands, agents,
output-styles, and workflows. Only its directory walk is extracted so the
skill resolver reuses the same submodule/worktree stop boundary. Writes stay
where they were: market installs and /skillify still target
`~/.claude/skills`.
`.agents` is anchored at $HOME rather than CLAUDE_CONFIG_DIR, since it is a
space shared with other tools that read $HOME. Note that Bun snapshots
os.homedir() at startup while Node re-reads $HOME per call, so the resolver
reads the env var explicitly.
Opt out with `disableAgentSkillsDirectory` in settings.json or
CLAUDE_CODE_DISABLE_AGENT_SKILLS_DIR=1; no `.agents` root is produced at all
then, so there is not even a failed stat.
(cherry picked from commit ba9338f85d6beaba237753e42e823c87b59633d4)
setInteractiveRegions only recorded visibleDragRegion inside the darwin
branch, so on Windows and Linux it stayed null and dragging fell back to
clamping against the whole PET_WINDOW_WIDTH x PET_WINDOW_HEIGHT window. That
window is mostly transparent padding around a mascot anchored bottom center,
so the mascot stopped short of every display edge by the width of that
padding and could not be dragged into a corner. Record the region on all
platforms; the reposition-on-restore step stays darwin-only.
Normalizing the region also clamped against the nominal constants. Renderer
regions are measured against the live viewport, so clamp against the real
content box instead -- the same petWindowContentExtent the shape clamp
already uses -- and let the shape clamp reuse that one normalizer.
The Windows shape clamp used the nominal PET_WINDOW_HEIGHT constant. Renderer
regions are measured against the live viewport, so once the content area was
taller than that constant the mascot -- which sits flush with the viewport
bottom -- got sliced off from below, while the task badge kept its own
unclamped rect and stayed visible. Clamp the shape to the real content box.
Dragging also moved the window with setPosition, which Windows resolves as
getSize() + setBounds(); that DIP round trip grows the window a pixel at a
time on fractional display scaling, and the drag timer fires every 16ms.
Restate the recorded size on every tick so dragging stays size-neutral.
