fix(hooks): fail-closed writes, exact hook identity, /ws privilege gate

Author: florintimbuc

CLAUDE.md

@@ -108,11 +108,15 @@ pixel-agents --help
 
 The default bind address is `127.0.0.1`. Binding to `0.0.0.0` exposes the UI and WebSocket to the local network; do this only on a trusted network.
 
+Open the URL the CLI prints — it carries a `?token=` for this session. Any browser can watch the office without it, but installing or removing hooks (which edits your agent tool's own settings file) is only offered to a session that has the token, so an untokened client on the network cannot approve it. Open the bare address instead and the hooks toggle in Settings is refused, and reports the actual install state rather than appearing to work.
+
+Treat that URL as a secret: the token is a bearer capability, not proof of being local. Whoever holds it can approve the hook install from anywhere the server is reachable — so don't paste the URL into a shared channel, and note that it also lands in your browser history and (unredacted) in the server's own request log.
+
 Pass `--no-terminal` to disable the embedded terminal — watch agents without launching or attaching to them from the browser.
 
 ### Running the extension and standalone together
 
-The extension and standalone CLI can run at the same time. Each server registers under `~/.pixel-agents/servers/`; the Claude hook script sends events to all active registrations. VS Code and standalone keep separate agents, seats, and settings while using the shared office layout.
+The extension and standalone CLI can run at the same time. Each server registers under `~/.pixel-agents/servers/`; the hook script sends events to all active registrations. VS Code and standalone keep separate agents, seats, and settings while using the shared office layout.
 
 Stop a standalone server with **Ctrl+C**. It removes only its own registration.
 

README.md

@@ -34,6 +34,7 @@ import {
   writeLayoutToFile,
 } from '../../server/src/layoutPersistence.js';
 import { PathSet } from '../../server/src/pathKey.js';
+import { CONSENT_INSTALL_MESSAGE } from '../../server/src/providers/hook/claude/consentCopy.js';
 import { claudeProvider, copyHookScript } from '../../server/src/providers/index.js';
 import { PixelAgentsServer } from '../../server/src/server.js';
 import {
@@ -220,37 +221,137 @@ export class PixelAgentsViewProvider implements vscode.WebviewViewProvider {
       });
   }
 
-  /** Install hooks + copy the hook script, surfacing installer errors
-   *  (e.g. an unparseable settings.json) instead of swallowing them. */
-  private installHooksAndScript(port: number | undefined, token: string | undefined): void {
-    claudeProvider
-      .installHooks(port !== undefined ? `http://127.0.0.1:${port}` : '', token ?? '')
-      .then(() => {
-        this.sendOrBuffer({ type: 'hooksStatus', installed: true });
-      })
-      .catch((err: unknown) => {
+  /** Copy the hook script, THEN install the settings.json entries, surfacing
+   *  every failure instead of swallowing it.
+   *
+   *  Script first, deliberately: an entry whose command points at a script that
+   *  is not on disk makes Claude Code spawn a dead `node` for every event, so a
+   *  failed copy must abort the install rather than run alongside it. And
+   *  `hooksStatus: true` is sent ONLY after both steps succeeded — it reports
+   *  actual install state, never intent (core/asyncapi.yaml). */
+  private async installHooksAndScript(
+    port: number | undefined,
+    token: string | undefined,
+  ): Promise<void> {
+    if (!copyHookScript(this.context.extensionPath)) {
+      vscode.window.showErrorMessage(
+        'Pixel Agents: could not install the hook script — hooks not installed.',
+      );
+      await this.reportHooksStatus();
+      return;
+    }
+    try {
+      await claudeProvider.installHooks(
+        port !== undefined ? `http://127.0.0.1:${port}` : '',
+        token ?? '',
+      );
+    } catch (err: unknown) {
+      vscode.window.showErrorMessage(
+        `Pixel Agents: ${err instanceof Error ? err.message : String(err)}`,
+      );
+      await this.reportHooksStatus();
+      return;
+    }
+    this.sendOrBuffer({ type: 'hooksStatus', installed: true });
+  }
+
+  /**
+   * The ordinary Settings-toggle path (the consent gate is separate, below).
+   *
+   * The preference is persisted only AFTER the install/uninstall settled, and
+   * only when the resulting on-disk state agrees with what was asked. Writing
+   * it first is how a user gets stranded: a failed uninstall leaves the entries
+   * on disk and still firing, while a persisted hooks-off makes the next
+   * activation skip the consent/install path entirely — never asked again, and
+   * the checkbox reads "off" so clicking it would install rather than remove.
+   */
+  private async setHooksEnabled(enabled: boolean): Promise<void> {
+    if (enabled) {
+      // An explicit Settings toggle IS the consent to modify settings.json.
+      grantHooksConsent();
+      const serverConfig = this.pixelAgentsServer?.getConfig();
+      await this.installHooksAndScript(serverConfig?.port, serverConfig?.token);
+    } else {
+      try {
+        await claudeProvider.uninstallHooks();
+      } catch (err: unknown) {
         vscode.window.showErrorMessage(
           `Pixel Agents: ${err instanceof Error ? err.message : String(err)}`,
         );
+      }
+    }
+    // Re-derive rather than trust the call: installHooksAndScript already
+    // swallows its own failures into an error message, and either way the file
+    // on disk is the only authority for what is actually firing.
+    let installed: boolean;
+    try {
+      installed = await claudeProvider.areHooksInstalled();
+    } catch {
+      return; // the failure was already surfaced; do not also persist on a guess
+    }
+    if (installed === enabled) {
+      this.adapter.setSetting(GLOBAL_KEY_HOOKS_ENABLED, enabled);
+      this.runtime.hooksEnabled.current = enabled;
+      console.log(`[Pixel Agents] Hooks ${enabled ? 'enabled' : 'disabled'} by user`);
+    }
+    // Report the truth either way: on failure the entries are still on disk and
+    // still firing, and a checkbox stuck "off" over live hooks offers no retry.
+    this.sendOrBuffer({ type: 'hooksStatus', installed });
+  }
+
+  /** Broadcast the ACTUAL install state, re-derived from settings.json.
+   *  Every failure path calls this so the Settings checkbox — which renders
+   *  install state, not the preference — self-corrects instead of keeping the
+   *  optimistic value the click produced. Standalone gets this for free
+   *  (clientMessageHandler re-derives after every toggle); VS Code has no
+   *  equivalent seam, so failure paths report explicitly. */
+  private async reportHooksStatus(): Promise<void> {
+    try {
+      this.sendOrBuffer({
+        type: 'hooksStatus',
+        installed: await claudeProvider.areHooksInstalled(),
       });
-    if (!copyHookScript(this.context.extensionPath)) {
-      console.warn('[Pixel Agents] Hook script not copied, hooks may not fire');
+    } catch {
+      // Never let a status broadcast mask the error already surfaced.
     }
   }
 
   /** First-run consent gate: never touch ~/.claude/settings.json until the
    *  user has approved it once (persisted in config.json, shared with the
-   *  standalone CLI). Hooks already present count as consent granted to a
-   *  pre-consent version. "Not Now" and a plain dismissal both leave
-   *  everything untouched and ask again next startup; only the explicit
-   *  "Don't Ask Again" persists hooks-off. */
+   *  standalone CLI).
+   *
+   *  It asks exactly one population: the one with NOTHING of ours installed.
+   *  With our hooks ALREADY present but no recorded consent — a pre-consent
+   *  version installed them silently — consent is granted here and the install
+   *  runs with no prompt at all. That install is the 14 -> 12 migration, and it
+   *  only ever REDUCES scope: it drops UserPromptSubmit and TaskCreated, the two
+   *  events that forwarded prompt text and were consumed by nothing. Nothing
+   *  this user already had is expanded, so the friction of a prompt buys them
+   *  nothing they do not already have. (This is deliberately NOT the general
+   *  rule: consent for a fresh install is still asked for, in full, below.)
+   *
+   *  A non-modal notification, carrying the FULL disclosure in its message. A
+   *  startup consent gate must not block the workbench, and it does not have to:
+   *  a notification with buttons renders permanently expanded (`canCollapse` is
+   *  `!hasActions`) with no line clamp, and truncates only at 1000 characters —
+   *  which the composed message stays under, pinned by consentCopy.test.ts. The
+   *  facts therefore live in the message itself; a "Details" affordance would
+   *  put the disclosure one click away, which is the gap this gate exists to
+   *  close.
+   *
+   *  The Info toast auto-hides after ~10 s WITHOUT closing the notification: it
+   *  parks in the notification bell with its buttons intact and this promise
+   *  still pending. So an unanswered or dismissed prompt writes nothing and
+   *  asks again next startup; only the explicit "Don't Ask Again" persists
+   *  hooks-off. */
   private async installHooksWithConsent(port: number, token: string): Promise<void> {
     if (!readConfig().hooksConsentGiven) {
       if (await claudeProvider.areHooksInstalled()) {
+        // Already installed and already firing: grant and migrate silently.
         grantHooksConsent();
       } else {
         const choice = await vscode.window.showInformationMessage(
-          "To show your agents in real time, Pixel Agents needs to add its hooks to ~/.claude/settings.json. Your existing settings are kept safe, and you can remove Pixel Agents' hooks at any time.",
+          CONSENT_INSTALL_MESSAGE,
           'Install Hooks',
           'Not Now',
           "Don't Ask Again",
@@ -265,7 +366,7 @@ export class PixelAgentsViewProvider implements vscode.WebviewViewProvider {
         grantHooksConsent();
       }
     }
-    this.installHooksAndScript(port, token);
+    await this.installHooksAndScript(port, token);
   }
 
   resolveWebviewView(webviewView: vscode.WebviewView) {
@@ -344,28 +445,7 @@ export class PixelAgentsViewProvider implements vscode.WebviewViewProvider {
       } else if (message.type === 'setGhostHeadlessAgents') {
         this.adapter.setSetting(GLOBAL_KEY_GHOST_HEADLESS_AGENTS, message.enabled);
       } else if (message.type === 'setHooksEnabled') {
-        const enabled = message.enabled as boolean;
-        this.adapter.setSetting(GLOBAL_KEY_HOOKS_ENABLED, enabled);
-        this.runtime.hooksEnabled.current = enabled;
-        if (enabled) {
-          // An explicit Settings toggle IS the consent to modify settings.json.
-          grantHooksConsent();
-          const serverConfig = this.pixelAgentsServer?.getConfig();
-          this.installHooksAndScript(serverConfig?.port, serverConfig?.token);
-          console.log('[Pixel Agents] Hooks enabled by user');
-        } else {
-          claudeProvider
-            .uninstallHooks()
-            .then(() => {
-              this.sendOrBuffer({ type: 'hooksStatus', installed: false });
-              console.log('[Pixel Agents] Hooks disabled by user');
-            })
-            .catch((err: unknown) => {
-              vscode.window.showErrorMessage(
-                `Pixel Agents: ${err instanceof Error ? err.message : String(err)}`,
-              );
-            });
-        }
+        void this.setHooksEnabled(message.enabled as boolean);
       } else if (message.type === 'setHooksInfoShown') {
         this.adapter.setSetting(GLOBAL_KEY_HOOKS_INFO_SHOWN, true);
       } else if (message.type === 'setShowAreas') {

adapters/vscode/PixelAgentsViewProvider.ts

@@ -173,7 +173,7 @@ This section is auto-generated. Do not edit between the markers; CI fails on dri
 
 <!-- BEGIN:E2E-INVENTORY -->
 
-77 tests total. Generated by `scripts/generate-e2e-inventory.mjs`. Re-run after adding or removing tests.
+84 tests total. Generated by `scripts/generate-e2e-inventory.mjs`. Re-run after adding or removing tests.
 
 ### `@area:spawn` (2 tests)
 
@@ -205,11 +205,17 @@ This section is auto-generated. Do not edit between the markers; CI fails on dri
 - `e2e/claude/hooks-on/lifecycle.spec.ts:877` — rapid /clear then new tool within 500ms lands on the reassigned agent (Hooks ON / lifecycle)
 - `e2e/claude/hooks-on/lifecycle.spec.ts:935` — close via X prevents re-adoption of old JSONL during dismissal cooldown (Hooks ON / lifecycle)
 
-### `@area:cross-cutting` (13 tests)
+### `@area:cross-cutting` (19 tests)
 
 - `e2e/claude/hooks-off/lifecycle.spec.ts:735` — agentToolsClear fires at turn end via turn_duration JSONL record (Hooks OFF / lifecycle)
 - `e2e/claude/hooks-off/lifecycle.spec.ts:800` — heuristic permission timer is cancelled when an agent is closed via overlay (Hooks OFF / lifecycle)
 - `e2e/claude/hooks-off/lifecycle.spec.ts:879` — sub-agent permission bubble fires on stalled non-exempt sub-tool via heuristic timer (Hooks OFF / lifecycle)
+- `e2e/claude/hooks-on/consent.spec.ts:141` — fresh install: the prompt discloses scope and Install writes the hooks (Hooks consent gate)
+- `e2e/claude/hooks-on/consent.spec.ts:182` — Not Now writes nothing and leaves consent ungranted (Hooks consent gate)
+- `e2e/claude/hooks-on/consent.spec.ts:206` — Don't Ask Again writes nothing and persists hooks off (Hooks consent gate)
+- `e2e/claude/hooks-on/consent.spec.ts:240` — a pre-consent 14-event install migrates to 12 with no prompt (Hooks consent gate / pre-consent install)
+- `e2e/claude/hooks-on/consent.spec.ts:284` — Settings toggle removes the migrated hooks and keeps third-party entries (Hooks consent gate / pre-consent install)
+- `e2e/claude/hooks-on/consent.spec.ts:330` — a failed uninstall does not persist hooks-off (Hooks consent gate / toggle-off failure)
 - `e2e/claude/hooks-on/lifecycle.spec.ts:1034` — done sound chime fires on agentStatus waiting (Hooks ON / lifecycle)
 - `e2e/claude/hooks-on/lifecycle.spec.ts:1127` — restored agents skip the matrix spawn animation (Hooks ON / lifecycle)
 - `e2e/claude/hooks-on/lifecycle.spec.ts:1211` — tool status text matches every PreToolUse tool name (Hooks ON / lifecycle)
@@ -240,9 +246,10 @@ This section is auto-generated. Do not edit between the markers; CI fails on dri
 - `e2e/claude/hooks-off/matrix.spec.ts:253` — external inline teammate adopted via JSONL polling (Hooks OFF / matrix)
 - `e2e/claude/hooks-off/matrix.spec.ts:312` — external tmux teammate adopted via JSONL polling (Hooks OFF / matrix)
 
-### `@area:standalone` (8 tests)
+### `@area:standalone` (9 tests)
 
-- `e2e/standalone/hooks.spec.ts:10` — propagates hook-driven lifecycle into the browser UI (Standalone / hooks)
+- `e2e/standalone/hooks.spec.ts:11` — propagates hook-driven lifecycle into the browser UI (Standalone / hooks)
+- `e2e/standalone/hooks.spec.ts:111` — the hooks checkbox reflects install state and its click is the consent grant (Standalone / hooks)
 - `e2e/standalone/multi-server-hooks.spec.ts:31` — extension and standalone both stay hook-driven without cross-contamination (Standalone / multi-server hooks)
 - `e2e/standalone/ui.spec.ts:27` — closeAgent despawns the character (Standalone / UI)
 - `e2e/standalone/ui.spec.ts:61` — Debug View renders JSONL diagnostics in standalone (Standalone / UI)

e2e/README.md

@@ -70,11 +70,14 @@ export const test = base.extend<{
   seedConfig: unknown;
   /** Pre-seed `~/.pixel-agents/layout.json` (must carry a high layoutRevision). */
   seedLayout: unknown;
+  /** Pre-seed `~/.claude/settings.json` (e.g. an existing hook install). */
+  seedClaudeSettings: unknown;
   /** Folder basenames for a multi-root workspace (>1 → multi-root). */
   workspaceFolders: string[];
 }>({
   seedConfig: [undefined, { option: true }],
   seedLayout: [undefined, { option: true }],
+  seedClaudeSettings: [undefined, { option: true }],
   workspaceFolders: [[], { option: true }],
   // Auto-fixture: tag every test with Allure epic + feature derived from its
   // @area: annotation and enclosing describe path. Runs before pixelAgents.
@@ -85,10 +88,15 @@ export const test = base.extend<{
     },
     { auto: true },
   ],
-  pixelAgents: async ({ seedConfig, seedLayout, workspaceFolders }, use, testInfo) => {
+  pixelAgents: async (
+    { seedConfig, seedLayout, seedClaudeSettings, workspaceFolders },
+    use,
+    testInfo,
+  ) => {
     const session = await launchVSCode(testInfo.title, {
       seedConfig,
       seedLayout,
+      seedClaudeSettings,
       workspaceFolders,
     });
     const { window, tmpHome, workspaceDir, mockLogFile } = session;

e2e/fixtures/pixel-agents.ts

@@ -45,6 +45,13 @@ export interface LaunchOptions {
   seedConfig?: unknown;
   /** Pre-seed `~/.pixel-agents/layout.json` (written before the panel loads). */
   seedLayout?: unknown;
+  /**
+   * Pre-seed `~/.claude/settings.json` (written before the extension activates).
+   * For consent specs that need an EXISTING hook install to be present at the
+   * moment the gate runs — the gate reads it during activation, so a test-body
+   * write would be too late.
+   */
+  seedClaudeSettings?: unknown;
 }
 
 /**
@@ -97,6 +104,19 @@ export async function launchVSCode(
   if (opts.seedLayout !== undefined) {
     fs.writeFileSync(path.join(paDir, 'layout.json'), JSON.stringify(opts.seedLayout, null, 2));
   }
+  if (opts.seedClaudeSettings !== undefined) {
+    const claudeHomeDir = path.join(tmpHome, '.claude');
+    fs.mkdirSync(claudeHomeDir, { recursive: true });
+    // A string is written VERBATIM, so a spec can seed a deliberately
+    // unparseable file (JSON.stringify would turn it into a valid quoted
+    // string, i.e. the opposite of what such a test needs).
+    fs.writeFileSync(
+      path.join(claudeHomeDir, 'settings.json'),
+      typeof opts.seedClaudeSettings === 'string'
+        ? opts.seedClaudeSettings
+        : JSON.stringify(opts.seedClaudeSettings, null, 2),
+    );
+  }
 
   // Enable Claude Agent Teams in the test workspace. Real Claude Code reads this
   // env from .claude/settings.local.json on startup; without it, team mode is gated