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.
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.
`new URL('..', import.meta.url).pathname` yields a leading-slash path like
/E:/... on Windows, so Bun.spawn failed with ENOENT. Resolve desktopRoot via
fileURLToPath, start Vite through its Node API instead of spawning `bun run
dev`, and resolve the Electron binary directly per platform.
Recovered from the discarded wip/capabilities snapshot (cb1208c9) as a
standalone fix.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
