Skip to content

fix: never overwrite user settings - consent-gated, merge-safe hook installer - #378

Merged
pablodelucca merged 23 commits into
mainfrom
fix/hook-installer-safety
Aug 15, 2026
Merged

fix: never overwrite user settings - consent-gated, merge-safe hook installer#378
pablodelucca merged 23 commits into
mainfrom
fix/hook-installer-safety

Conversation

@pablodelucca

@pablodelucca pablodelucca commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Closes #377

Context

A Marketplace review (2026-08-04) reported Pixel Agents replacing a user's entire ~/.claude/settings.json — permission rules included — with 14 copies of its own hook. Root cause confirmed: readClaudeSettings() treated an existing-but-unparseable file as an empty config, and the subsequent install rewrote the file from {}. A hand-edited trailing comma, a BOM, or a torn read while Claude Code writes the same file was enough to trigger it. This PR closes that hole and hardens every path that touches the user's settings.

Changes

Installer safety

  • Abort (with a surfaced error) when settings.json exists but cannot be parsed — never write on a failed read, on install and uninstall. Failed uninstalls no longer claim success.
  • One-time settings.json.backup before our first-ever modification, never overwritten afterwards.
  • Guarded read-modify-write: re-read raw content just before the atomic rename, redo the mutation on fresh content if the file changed underneath, retry torn reads before concluding the file is corrupt.
  • Tightened hook identity: a command is ours only if it references claude-hook.js inside a .pixel-agents directory (a third-party hook that happens to be named claude-hook.js survives), and cleanup strips individual commands, not whole entries (a user-merged entry keeps its third-party hooks).

Consent

  • First-run consent gate on both surfaces before anything touches settings.json: VS Code notification with Install Hooks / Not Now / Don't Ask Again; CLI [Y/n/never] prompt on a TTY, skip-with-log on non-interactive runs. "Not Now"/dismiss persist nothing and ask again next startup; only the explicit permanent option turns hooks off.
  • Consent is shared across surfaces (hooksConsentGiven in ~/.pixel-agents/config.json); hooks already installed count as consent from a pre-consent version, so existing users see no prompt.
  • New hooksStatus protocol message: the webview now reflects the actual install state instead of the hooksEnabled preference — no more "Instant Detection Active" while consent is pending.

Uninstall

  • New vscode:uninstall script (dist/uninstall.js): removes our hook entries and factory-resets hooks config when the extension is uninstalled, so nothing keeps running behind the user's back and a future reinstall starts from the first-run experience.

Testing

  • 12 new unit tests: byte-for-byte no-touch on malformed/BOM'd files, backup semantics, merge preservation, lookalike-hook survival, per-command filtering, consent config round-trips.
  • Manually verified end-to-end on macOS (standalone CLI, isolated HOME) and Windows 11 (real VS Code first-install, all three prompt outcomes, uninstall cleanup) — four bugs found in manual testing are fixed in this branch.
  • Full suites green locally: 382 server, 52 webview, package contract, both tsc projects, lint.

🤖 Generated with Claude Code

pablodelucca and others added 15 commits August 5, 2026 13:28
readClaudeSettings treated an existing-but-unparseable ~/.claude/settings.json
as an empty config, so installHooks would rewrite the file with only the Pixel
Agents hook entries — silently erasing the user's permission rules and every
other setting (Marketplace review, 2026-08-04). An unreadable file now throws
before any write; areHooksInstalled reports false and uninstallHooks aborts
with a log instead of rewriting. Both hosts surface the error to the user.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Before Pixel Agents' first-ever write to ~/.claude/settings.json, copy it to
settings.json.backup. The backup is never overwritten afterwards, so it always
preserves the pre-Pixel-Agents state — recoverable even if a future installer
bug corrupts the live file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude Code writes ~/.claude/settings.json too, so the installer's
read-modify-write raced it two ways: a mid-write read parses like a corrupt
file, and a write based on a stale read silently drops the other writer's
change. Both mutations (install/uninstall) now run through a guarded cycle
that re-reads the raw file just before writing — redoing the mutation on
fresh content if it changed — and retries torn reads before concluding the
file is truly unparseable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Locks in the abort-on-failed-read contract: malformed and BOM-prefixed
settings.json must reject installHooks and stay byte-for-byte unchanged
(the file-replacement scenario from the Marketplace review), and the happy
path must preserve unrelated keys (permissions, model) and third-party
hook entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uninstalling the extension previously left the Pixel Agents hooks live in
~/.claude/settings.json — fourteen events still invoking a script for an app
the user had deleted. A vscode:uninstall script (bundled to dist/uninstall.js,
plain Node, no vscode module) now removes our entries via the same guarded
uninstallHooks path. The shared hook script under ~/.pixel-agents/hooks/ is
left for the standalone CLI, which re-adds its own entries on next run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Neither surface touches ~/.claude/settings.json anymore until the user has
approved it once (hooksConsentGiven in config.json, shared by both surfaces —
consent is per-human, not per-adapter). VS Code shows a notification with
Install Hooks / Not Now; the standalone CLI asks on an interactive terminal
and skips installation on non-TTY runs. An explicit hooks toggle in either UI
grants consent, hooks already present count as consent from a pre-consent
version, and declining turns the hooks setting off. E2E seeds consent so the
startup-install specs keep exercising the real installer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Since hook installation became consent-gated, first run can legitimately end
with hooks off and nothing installed — but the first-run tooltip still
announced 'Instant Detection Active'. Show it only when hooks are enabled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Toggling hooks off on an unparseable settings.json logged the parse error
followed by 'Hooks uninstalled (user toggle)' — uninstallHooks swallowed the
failure, so callers could not tell nothing was removed. Uninstall now rejects
like install does, callers log success only on success, and each operation
carries accurate wording: install appends 'hooks not installed', uninstall
appends 'hook entries left in place'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consent belongs to the installation it was granted to. The uninstall script
now resets hooksConsentGiven after removing the hook entries, so a future
install asks again instead of silently modifying settings.json on the back of
a stale approval.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The uninstall script only revoked consent, but a persisted hooks-off (from a
decline or a toggle) survived it — and a disabled hooks setting makes startup
skip the entire consent/install flow with zero output, so the next install
never prompts and never says why. Uninstall now resets hooksConsentGiven,
hooksEnabled, and hooksInfoShown to factory state in both namespaces, and the
CLI logs a line when hooks are disabled instead of staying silent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…name

Two ways the old marker could clear a hook that wasn't ours: identity was the
bare substring 'claude-hook.js', a generic name another Claude tool could
plausibly pick for its own hook script; and the check judged whole entries, so
an entry a user had hand-merged our command into lost its third-party hooks
along with ours. Identity now requires the script name AND our .pixel-agents
directory in the same command, and cleanup strips only our commands from an
entry, keeping the entry alive when anything else remains.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The spawned dist/cli.js has no TTY, so the first-run consent gate now
(correctly) skips hook installation there — the test only passed before
against a stale pre-consent bundle. Seed consent in its isolated HOME so it
keeps exercising the real install path, mirroring the e2e fixtures.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The webview only knew the hooksEnabled preference, which defaults to true —
so while the first-run consent prompt was still pending it announced 'Instant
Detection Active' with nothing installed (caught in manual Windows testing).
New hooksStatus ServerMessage carries the real install state: sent after the
connect handshake and re-sent when install/uninstall completes, so the
tooltip appears at the moment consent is granted and hooks actually land.
Tooltip now gates on it instead of the preference.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
'Not Now' now matches a plain dismissal — nothing persisted, ask again next
startup — and only the explicit 'Don't Ask Again' (or 'never' in the CLI
prompt, previously any 'n') turns the hooks setting off for good. New body
copy leads with the benefit and says plainly that existing settings are kept
safe and Pixel Agents' hooks are removable any time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

@florintimbuc florintimbuc changed the title Never overwrite user settings: consent-gated, merge-safe hook installer fix: never overwrite user settings - consent-gated, merge-safe hook installer Aug 5, 2026
pablodelucca and others added 6 commits August 5, 2026 17:00
…visories

Two independent CI failures on the PR:

- verify-npm-package spawns the packed CLI without a TTY, where the first-run
  consent gate now (correctly) skips hook installation — seed consent in the
  smoke HOME so the script keeps exercising the real install path, mirroring
  server/__tests__/cli.test.ts.
- npm audit gained fresh upstream advisories unrelated to this branch:
  brace-expansion and fast-uri (high, fixed in-range via lockfile) and the
  ajv@<=6.12.6 ReDoS under the dev-only AsyncAPI RAML parser chain (moderate,
  pinned to the patched ajv 6.15.0 via nested overrides — same major, so the
  parser API is untouched). asyncapi validate/generate verified drift-free.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(hooks): skip the settings.json backup when replacing our own install's output

The one-time .pixel-agents.backup exists to preserve the user's
pre-Pixel-Agents file. In a home where our install CREATED settings.json,
the first later write (uninstall or a reinstall over the emptied file)
backed up our own entries — a copy that restores nothing user-authored
while posing as the user's original.

writeClaudeSettings now skips the backup when the content being replaced
is entirely of our own writing (settingsHoldOnlyOurHooks): only a `hooks`
key, every entry exactly the shape we write, every command ours — plus
the {} / {"hooks":{}} shapes our own uninstall leaves behind, which hold
nothing a backup could restore. Any user-authored content — an extra
top-level key, a non-empty matcher, an unknown field, a foreign command —
still takes the backup before the first write that replaces it, now
capturing that content instead of our own. The skip reads expectedRaw,
the same content the pre-rename verify pins, so a stale decision aborts
before the rename and the retry re-decides on fresh content: a commit
never outruns a warranted backup.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(consent): smoother disclosure wording

Wording only, no semantic change: "This adds…", the backup is saved
"next to them", and the --host caveat reads as a note rather than an
aside. Both surfaces render the same copy, so the tests pinning the
disclosure facts are unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* feat(consent): move the first-run hooks ask in-app on both surfaces

The consent prompt is no longer a native VS Code modal or a CLI TTY
question. The server sends hooksConsentRequest (headline + disclosure
from consentCopy.ts, so the client renders the server's exact terms)
during the webviewReady handshake, and the webview's new ConsentModal
answers with hooksConsentResponse { install | notNow | never }.

- Standalone: request rides the tokened /ws handshake only — an
  untokened spectator is never asked, since its answer would be ignored.
- Fail-closed on exact matches: only 'install' grants consent and
  installs, only 'never' persists hooks-off (without touching
  settings.json), and notNow plus every unrecognized value writes
  nothing. Escape sends no message at all — the ask fires again next
  open. No close x, no Cancel synonym, backdrop clicks are inert.
- Silent-grant migration for pre-consent installs is unchanged; the
  Settings checkbox remains the documented route after a decline.
- Hooks now install only once the office is first opened by someone
  who can approve — no more install at bare activation.

Tests translated pin-for-pin: the CLI prompt suite moved to
clientMessageHandler (junk-choice table, ordering, privilege boundary),
httpServerWs gained the tokened/untokened handshake pins, the VS Code
consent spec drives the in-app dialog, and standalone gained a consent
suite (seedHooksConsent fixture opt-out) covering the token boundary
and the checkbox route.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(consent): make the first-run ask diegetic — a greeter and its bubble

The consent dialog is no longer a centered modal over the office. A char_0
greeter spawns near the office's bottom-left corner and ConsentBubble is its
speech bubble: anchored up-right of its head, tail squares stepping down to
it, the camera drifting to center character + bubble together.

- The greeter is not an agent: FSM-frozen, invisible to hit-testing and to
  palette diversity, never seated (even across a layout rebuild). It lives
  exactly as long as the bubble is mounted, so every close path (a button,
  Escape, a hooksStatus that moots the ask) despawns it via the matrix effect.
- Camera: consentCameraTarget is fed per frame by the overlay; an explicit
  cameraFollowId outranks it and a manual pan/wheel cancels it until the next
  spawn. Offsets are capped against the viewport so a narrow panel can't shove
  the greeter off-screen.
- Copy: the headline is now a pure welcome ("Welcome to Pixel Agents!") and
  every disclosure fact lives in the shared block — consentCopy.test.ts pins
  the split, and the VS Code-modal-specific rationale is gone with the modal.
- Backdrop clicks were inert; clicks on the live office around the bubble are
  inert for the same reason — a stray click is not an answer.

tsconfig.node.json gains the DOM lib so the node-runner unit tests can
type-check OfficeState (matrixEffect pulls in CanvasRenderingContext2D).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(office): move the consent greeter out of the agent map

The greeter lived in OfficeState.characters — the map that means
"agents" — and was filtered back out by hand with five isGreeter guards.
Two consumers missed theirs: both saveAgentSeats payload builders, so the
greeter's palette/seat row was persisted to ~/.pixel-agents state on any
agentCreated that landed while the ask was up.

It now lives in OfficeState.greeter, its own field. Seat assignment,
palette diversity, the wander FSM, hit-testing and seat persistence
iterate `characters` and exclude it structurally — every guard is
deleted, and the render feed (getCharacters()) is the one seam where it
joins the agents. The two copy-pasted seat payload loops collapse into
officeState.getPersistableSeats(). isGreeter survives only as a snapshot
marker for e2e.

Supporting extractions, each deleting a smaller duplication:

- matrixEffectState.ts: startMatrixEffect / advanceMatrixEffect. The
  spawn/despawn three-field reset was hand-rolled at eight sites, and
  the state half of the effect no longer lives with its Canvas renderer.
- defaultZoom moves from office/toolUtils.ts to its only consumer
  (useEditorActions). It reads devicePixelRatio, and a viewport concern
  in the tool-taxonomy module dragged the DOM into OfficeState's module
  graph — which is what had forced DOM libs onto tsconfig.node.json.
  That lib widening is reverted; the node-runner test graph can no
  longer reference DOM globals that don't exist at runtime.

consentGreeter.test.ts pins the structure: the agent map never holds
the greeter, the render feed does, the persisted seat payload cannot
contain it, and palette diversity ignores it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(consent): one consent policy, shared by both surfaces

The ask-or-not predicate and the choice→action rule were written once
per surface, and the copies had already diverged: VS Code's Install
button granted consent and installed directly, skipping setHooksEnabled's
re-derive-then-persist step. Two windows could interleave — one clicks
Don't Ask Again (persists hooks-off), the other's still-open dialog
clicks Install — leaving hooks installed and firing while the persisted
preference said off, so the next activation ran heuristic timers against
live hooks.

consentGate.ts now owns the policy: hooksConsentRequest() decides
whether a webviewReady handshake carries the ask (installed / consent
recorded / preference off / unprivileged are each a reason not to), and
consentActionFor() maps an answer to install | persistOff | none,
fail-closed on exact matches. Both surfaces call these; neither carries
a copy to drift.

The VS Code fix itself is one line: Install routes through
setHooksEnabled(true) — the same grant, install, re-derive, persist
sequence as the Settings toggle, which the old comment claimed to
mirror. Don't Ask Again now also reports the derived install state,
matching standalone.

consentGate.test.ts pins the policy directly (ask conditions, junk-choice
table); the per-surface behavior stays pinned where it was
(clientMessageHandler.test.ts, consent.spec.ts, standalone/hooks.spec.ts
— all still green untouched).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(webview): one world→screen projection, testable bubble geometry

The map-centering + pan offset formula lived in the renderer, in
ToolOverlay, and (a third copy) in ConsentBubble — three chances to round
differently and put an overlay a pixel off the sprite it labels.
projection.ts now owns it: mapOffset() for the renderer's device-pixel
frame, overlayProjection() for the DOM overlays' CSS-pixel frame. DOM-free
by design (dpr is a parameter), so state and math modules can import it
without dragging window into their graph.

ConsentBubble's geometry — the clamped anchor, the tail squares chasing a
clamped bubble, the camera-offset caps — moves to
consentBubbleGeometry.ts as one pure function of the measured frame. That
was the half most likely to be wrong on a narrow VS Code side panel and
the only half with zero coverage; consentBubbleGeometry.test.ts now pins
anchoring, edge clamping, tail attachment under clamping, the unmeasured
first frame, and both camera caps. The component drops to measuring and
rendering, and the camera target is fed from the same function the render
positions with, so the drift and the drawn bubble cannot disagree.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(hooks): derive settingsHoldOnlyOurHooks from the writer

The backup-skip predicate was a second hand-written description of the
shape makeHookEntry() writes, and a twin like that drifts: the next field
added to our entries would make the predicate read our own fresh output
as user content, silently reviving the backup-of-our-own-file bug for
every install after it — with no test failing, since both copies would
need updating together.

The fields are now compared against makeHookEntry() itself. Two are
exempt because PAST installs legitimately differ in them: command (the
script path moves between homes and versions — identity stays isOurHook,
the same suffix rule as everywhere else) and timeout. Everything else,
present or added later, must match the writer's value or the ordinary
backup-before-write rule applies. Behavior today is identical — every
pinned case in claudeHookInstaller.test.ts passes unchanged — and the
fresh-home install→uninstall test now doubles as the drift pin, since it
pushes real writer output through the predicate.

Deliberately NOT the strip-and-check alternative (run uninstall's removal
on a clone, check nothing survives): that reads a user-edited matcher or
an unknown field on our entry as ours and skips a backup that could still
restore user-authored bytes. Strictness kept, drift deleted.

Also documents why the skip re-parses expectedRaw: the object
mutateClaudeSettings parsed from it has already been mutated into what we
are about to WRITE, and the decision must be about what we are REPLACING.

CLAUDE.md updated for the whole series: consentGate, the greeter's
structural exclusion, projection.ts, matrixEffectState.ts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(consent): read a consent answer as an absolute state command

consentActionFor(choice, installed) is now state-aware: 'never' over a
landed install takes the full Settings toggle-off path (uninstall, persist
hooks-off only once the disk agrees) and 'notNow' over a landed install
reverts it entirely (uninstall + the new revokeHooksConsent(), preference
untouched, so the ask genuinely returns). With nothing installed every
choice behaves exactly as before, and junk still writes nothing — an
unreadable settings file degrades to installed=false, so no choice ever
uninstalls on a guess.

Groundwork for the Intro, whose closing step lets the user walk Back and
revise an already-sent answer; replaying one-shot semantics there would
recreate the stranding bug (live entries behind a persisted hooks-off).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(intro): wrap the first-run ask in a four-step greeter tour

ConsentBubble becomes IntroBubble: Welcome → Powered by Claude Code
(install command + link) → hooks consent → You're all set (Discord),
with step dots and a top-right ✕ on every step. Back is ghost,
Continue/Not Now default, Install Hooks and Let's Go accent. The consent
step still renders the server's hooksConsentRequest verbatim; its
headline is retoned as that step's title, with the welcome moving to the
webview-owned opening step and every disclosure fact kept.

A consent-step click sends immediately and the tour advances; the App
snapshots the request and, once a choice was sent from this tour, ignores
consentRequest going null — so the closing step survives the hooksStatus
its own install broadcasts, while an unanswered tour still closes when a
cross-window install moots the ask. The ✕ and Escape abort without
sending, so the whole Intro returns on the next open. Back from the
closing step re-opens the consent step for a real change of mind, riding
the absolute-state answer semantics.

E2E: helpers/intro.ts walks the steps; the consent specs now pin the
paging, the ✕, the closing step surviving its own install, and the
Back-then-Don't-Ask-Again undo of a landed install.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: record the Intro — CLAUDE.md, glossary terms, ADR 0001

CLAUDE.md's consent bullet now describes the tour (steps, immediate send,
the moot gate, absolute-state revision, the ✕/Escape abort). CONTEXT.md
gains a First Run section defining Intro and Greeter. ADR 0001 — the
repo's first — records why consent choices send immediately and why a
revised choice undoes a landed install.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(hooks): compare settings.json against the writer structurally

settingsHoldOnlyOurHooks reached the writer's fields through a double
cast (`makeHookEntry() as unknown as Record<string, unknown>`) and
compared them with JSON.stringify. Both go: Object.entries into a Map
needs no cast, and `has` distinguishes "the writer has no such field" —
an unknown field on the entry, so not ours — from "its value is
undefined", which index access conflates.

The stringify comparison was a trap rather than a live bug, and the new
test says so rather than implying otherwise: it compares SERIALIZATIONS,
so it is key-order sensitive, but every field it reaches today is a
scalar (`hooks` is compared separately, element by element) and scalars
serialize identically in any order. It arms itself the day makeHookEntry()
grows a field with an object value — at which point a reordered copy of
our own output reads as user content, reviving the backup-of-our-own-file
bug for every install after it. deepEqual compares values, and key order
is not part of a value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(consent): read a revision against everything the last answer left

Three defects in the Intro's consent path, plus the duplication that let
the first one hide.

`notNow` could not undo a FAILED install. `install` records the grant
BEFORE it writes, so an install that then threw — the unparseable
settings.json of issue #377, exactly the population this gate exists for —
leaves a grant with nothing on disk, and the grant ALONE is what retires
the ask. Keying the revert off `installed` read that as "nothing to undo"
and did nothing: the user was never asked again. consentActionFor now
takes { installed, consentGiven }, and a revert with nothing installed
skips the uninstall entirely and revokes unconditionally — that is a
config.json write of our own, so no settings.json read can fail it.

Two answers could race. Revision is the whole POINT of absolute-state
semantics, so two answers in quick succession are designed-for, not an
edge case; both surfaces dispatch without awaiting and each re-reads the
disk to decide what its answer means. A revision that observed the first
answer's install mid-flight read `installed: false` and degraded into its
no-op variant, leaving hooks installed against the user's final answer.
applyConsentChoice serializes per process — the test fails without the
queue, with hooks left on disk.

The closing step congratulated unconditionally. It now reports the
OUTCOME: hooksStatusSeq exists because a failed install re-reports the
same `false` the webview already held, so the value never changes and
only the ARRIVAL of the message can settle the verdict. The step reads
"Hooks couldn't be installed" and points at Settings.

And the reason the first defect could hide: consentGate centralized the
POLICY, but the five-arm switch that carries it out stayed duplicated
per surface — the same drift the module's own header warns about, one
layer down, with the same thing on the other side of it. consentExecutor
owns the actions and their ORDER; each surface supplies only its effects
(error modal vs console, workspace setting vs store adapter). Standalone's
persistOff silently gained the hooksStatus b7d71cd claimed it already had.

Also renames what shipped: the bubble is IntroBubble, so the geometry
module and the greeter API say so (CONTEXT.md's Intro/Greeter), the
getConsentGreeter passthrough over a public field is gone, the bubble's
z-index is a named constant recording why it sits BELOW the modal stack,
and the thrice-declared { headline, disclosure } is core's
HooksConsentRequest.

E2E: a new case seeds an unparseable settings.json and pins both halves —
the failure is reported, and Not Now brings the ask back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(hooks): replace the bespoke deepEqual with util.isDeepStrictEqual

ab684e9 replaced the stringify comparison with deepEqual because key
order is not part of a value. The argument was right and the platform
already ships its conclusion: node:util's isDeepStrictEqual makes the
same judgement over parsed-JSON values, so the twenty-line bespoke
helper goes. Every field matchesWriter reaches today is a scalar
(matcher '', type 'command' — hooks is compared element by element), so
the deep comparison is entirely future-proofing for the day
makeHookEntry() grows an object-valued field; the platform can carry
that weight instead of code we review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(consent): make the effects' never-reject contract explicit

The seam claimed both rules and guaranteed neither. ConsentEffects
documented areHooksInstalled as "never throws" while both surfaces
forward the provider promise untouched — the never-throws was really the
executor's own catch. applyConsentChoice promised "the rejection still
reaches this call's own caller" while both call sites void it, so any
effect rejection became an unhandled-rejection crash log. And the two
surfaces diverged exactly where the contract was silent: VS Code's
setHooksEnabled swallows every failure internally, standalone's
applyHooksPreference had no catch at all.

One rule now, stated on the interface: every effect surfaces its own
failure the surface's way and resolves — except areHooksInstalled, which
may reject, because only the call site knows which way a given decision
fails closed (false when deciding whether to uninstall, true when
deciding whether a removal landed). applyHooksPreference wraps its body
(it is both the toggle dispatch's fire-and-forget and the consent
executor's setHooksEnabled effect), the standalone uninstall effect
wraps the host callback whose contract is unstated, and the executor
keeps a catch-and-log backstop for a broken effect — which also lets the
queue chain collapse to a plain assignment, since the chained promise
can no longer reject.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(server): split the consent flow suite into consentFlow.test.ts

clientMessageHandler.test.ts had grown to 1,022 lines bundling four
unrelated suites — areas/carpet ordering, the hooks consent flow,
webviewReady ordering, seat-palette sync — with the consent flow the
suite most likely to keep growing. It moves verbatim (it was already
self-contained: own home, own helpers, own describe) into
consentFlow.test.ts, leaving both files under 600 lines with one topic
each. Cross-references in consentGate.test.ts, cli.test.ts and
verify-npm-package.mjs follow it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(intro): pure-reducer tour state, one installFailed verdict, glossary names

The Intro's trickiest client logic — which asks survive being mooted,
when a hooksStatus arrival is this tour's install verdict and when it is
noise, what a revised choice resets — lived inline in the App as two
refs, two states and two effects, e2e-testable only. It is now a pure
reducer (introTourState.ts, four events, Node-runner tested in
introTour.test.ts) that useIntroTour wires to React and the transport;
the App is back to composition. Two files because the transport module
connects eagerly at import, and the reducer must not drag that into the
test graph.

The extraction also collapses a split brain: the App armed the verdict
wait with choice === 'install' while IntroBubble re-derived
sentChoice === 'install' && installOutcome === false off its own
duplicate of the sent choice. "Only an install has an outcome" now lives
once, in the reducer; the bubble takes a single installFailed boolean
and loses its sentChoice state and the InstallOutcome type.

And the constants say what the glossary says (CONTEXT.md: Intro,
Greeter; "consent modal" is the term to avoid): the eleven CONSENT_*
geometry/camera/greeter constants become INTRO_BUBBLE_*, INTRO_TAIL_*,
INTRO_CAMERA_* and GREETER_* — they size and aim the bubble on all four
steps, not the consent step. Rename and extraction ride together because
they interlock in IntroBubble.tsx's imports; CLAUDE.md follows all of it
(tree, consent bullet, test table, the long-stale __tests__ count).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(consent): provider-agnostic hooks consent with per-provider storage and wire

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Florin Timbuc <florin@sowild.design>
pablodelucca and others added 2 commits August 15, 2026 21:20
The WHAT fact now notes that existing settings are kept, and the DATA
fact leads with "Everything stays local" (listening only on 127.0.0.1)
— still naming --host as the one explicit exception, so the default is
emphasized without promising what a --host start can break. The
settings-survival pin in consentCopy.test.ts tracks the new casing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clicking Install Hooks advanced to the closing step immediately, which
rendered "You're all set!" for the whole server round-trip before a
failed install corrected it to "Hooks couldn't be installed" — a flash
of the wrong verdict. A neutral "Installing hooks..." closing title was
tried next and flashed on every success instead.

Now the consent step holds: all four of its buttons disable, Install
reads "Installing...", and the tour advances only when THIS tour's
hooksStatus arrives (useIntroTour's new installPending — the reducer's
armed verdict wait, which install alone arms). The closing step
therefore only ever renders WITH its verdict. Declines still advance
immediately (no outcome to wait on) and the x/Escape abort path is
untouched. Side effect: the failed-install e2e's "never shows success"
assertion stops racing the flash and becomes deterministic.

Also: welcome and Claude Code step copy tweaks, and the closing step's
Discord link is centered, text-base, with more vertical margin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pablodelucca
pablodelucca merged commit cf3ef72 into main Aug 15, 2026
15 checks passed
@pablodelucca
pablodelucca deleted the fix/hook-installer-safety branch August 15, 2026 20:37
@pablodelucca pablodelucca mentioned this pull request Aug 15, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Clobbers agent settings files

2 participants