chore(lint): cover tools/ in the lint gate; attach cause in dev-proxy rebuild() (#718)

AGENTS.md

@@ -13,7 +13,7 @@
 - `pnpm dev` launches the TypeScript entry point via `ts-node` for quick feedback.
 - `pnpm test` performs a full build, ensures Docker images are ready, and runs Vitest across all suites.
 - Targeted runs: `pnpm test:unit`, `pnpm test:integration`, `pnpm test:e2e`, `pnpm test:coverage`.
-- `pnpm lint` runs ESLint over `src/**/*.ts`, `packages/*/src/**/*.ts`, and `scripts/**/*.{js,mjs,cjs}`. `pnpm lint:fix` is narrower — it is `eslint src/**/*.ts --fix` — so it does **not** auto-fix everything `pnpm lint` reports; findings under `packages/` and `scripts/` have to be fixed by hand.
+- `pnpm lint` runs ESLint over `src/**/*.ts`, `packages/*/src/**/*.ts`, `scripts/**/*.{js,mjs,cjs}`, and `tools/**/*.{js,mjs,cjs}` (the dev-proxy supervisor is production tooling; experimental probes go in `scripts/experiments/`, which is ignored). `pnpm lint:fix` runs the same globs with `--fix`.
 - `pnpm typecheck` type-checks the shipped sources via `tsconfig.typecheck.json` (no build needed; the root `tsc -p tsconfig.json` checks nothing). `pnpm typecheck:tests` runs the per-file ratchet over the test trees against `tests/typecheck-baseline.json`; `pnpm typecheck:tests:update` re-records that baseline and `pnpm typecheck:tests:raw` emits unfiltered `tsc` output. `pnpm typecheck:all` runs both, and is the exact command pre-push and CI run.
 - **Run the whole dev-loop gate before pushing.** It is lint -> baseline committed -> `typecheck:all` -> clean build -> `test:unit` + `test:integration`, and both `.husky/pre-push` and CI enforce it; `typecheck:all` is the step most likely to block a push. It is documented once, canonically, in [CONTRIBUTING.md — Dev-Loop Gate](CONTRIBUTING.md#dev-loop-gate). Follow that section rather than any restatement of it.
 

CONTRIBUTING.md

@@ -106,9 +106,11 @@ no longer resolve, and counts that fell behind the code), `pnpm run changelog:ch
 (test-only changes are exempt automatically; label a genuine no-op PR `no-changelog`). Running
 those four before you push saves a CI round-trip.
 
-1. **Lint** — `pnpm run lint`. ESLint over `src/**/*.ts`, `packages/*/src/**/*.ts`, and
-   `scripts/**/*.{js,mjs,cjs}`. Note that `pnpm run lint:fix` is only `eslint src/**/*.ts --fix`,
-   so it does not auto-fix findings under `packages/` or `scripts/`.
+1. **Lint** — `pnpm run lint`. ESLint over `src/**/*.ts`, `packages/*/src/**/*.ts`,
+   `scripts/**/*.{js,mjs,cjs}`, and `tools/**/*.{js,mjs,cjs}` (the dev-proxy supervisor lives
+   under `tools/`, and it starts, stops and proxies the backend — it is production tooling, not a
+   probe script). `pnpm run lint:fix` runs the same globs with `--fix`. Experimental probes belong
+   in `scripts/experiments/`, which `eslint.config.js` ignores.
 
 2. **A committed `tests/typecheck-baseline.json`** — pre-push refuses to go further while that
    file is modified but uncommitted. The ratchet validates your working tree locally but the
@@ -177,10 +179,10 @@ We use ESLint to maintain consistent code style. There is no Prettier setup in t
 ### Setup
 
 ```bash
-# Run ESLint (src/**, packages/*/src/**, scripts/**)
+# Run ESLint (src/**, packages/*/src/**, scripts/**, tools/**)
 npm run lint
 
-# Fix auto-fixable issues — note this covers src/**/*.ts only
+# Fix auto-fixable issues over the same globs
 npm run lint:fix
 ```
 

changelog.d/718.fixed.md

@@ -0,0 +1 @@
+**The lint gate covers the dev-proxy** — `pnpm lint` (and so the pre-push hook and CI) linted `src/`, `packages/*/src/` and `scripts/` but not `tools/`, leaving the dev-proxy supervisor — the process that starts, stops and proxies the backend — outside the gate; it carried two `preserve-caught-error` findings in `BackendManager.rebuild()` that nothing reported. `tools/**/*.{js,mjs,cjs}` is now in the `lint` globs, the two rethrows attach the caught `execSync` error as `cause` (the tool response still serializes the sanitized message alone, now locked by integration tests that drive `dev_rebuild_and_restart` through a failing and a hanging build), and `pnpm lint:fix` is derived from `pnpm lint` instead of carrying its own `src/**/*.ts`-only glob, so the hook's "run lint:fix" hint is true for every finding it reports. Reviewing those catch blocks also fixed what they said: a build killed for exceeding execSync's 1 MiB output buffer was reported as a timeout (both arrive as `SIGTERM`; the timeout is now identified by `ETIMEDOUT` and the buffer raised to 64 MiB), a timed-out build now shows the output it had produced instead of only "re-run manually", and a failed build no longer sends the client a spurious `tools/list_changed` — the build fails before any restart, so the inventory it was told to re-fetch had not changed (#718)

docs/architecture/adapter-development-guide.md

@@ -496,7 +496,7 @@ The loader:
 - [ ] Unit and integration tests written under `tests/adapters/<language>/`
 - [ ] The new policy passes `tests/unit/shared/adapter-policy-contract.test.ts` — the cross-policy contract that runs against the real policies via `getPolicyForLanguage`. Its pinned capability table is a deliberate duplicate of what the policies declare, so a new language means editing that table on purpose
 - [ ] `pnpm install` run to link workspace
-- [ ] `pnpm run lint` clean (`src/**` and `packages/*/src/**` are linted)
+- [ ] `pnpm run lint` clean (`src/**`, `packages/*/src/**`, `scripts/**` and `tools/**` are linted)
 - [ ] `pnpm run typecheck:all` clean — `typecheck` (shipped sources, must be zero errors) plus `typecheck:tests` (the per-file ratchet). Both are gated by `.husky/pre-push` and by CI's lint job
 - [ ] `tests/typecheck-baseline.json` committed if the ratchet moved. The ratchet fails in *both* directions: a count going up means new type errors to fix; a count going down (or a removed test file) means the baseline is stale — run `pnpm run typecheck:tests:update` and commit the result in the same PR
 - [ ] TypeScript builds to `dist/` (ESM)

package.json

@@ -99,8 +99,8 @@
     "typecheck:tests:update": "node scripts/typecheck-tests-ratchet.mjs --update",
     "typecheck:tests:raw": "tsc -p tsconfig.spec.json --pretty false",
     "typecheck:all": "pnpm run typecheck && pnpm run typecheck:tests",
-    "lint": "eslint \"src/**/*.ts\" \"packages/*/src/**/*.ts\" \"scripts/**/*.{js,mjs,cjs}\"",
-    "lint:fix": "eslint src/**/*.ts --fix",
+    "lint": "eslint \"src/**/*.ts\" \"packages/*/src/**/*.ts\" \"scripts/**/*.{js,mjs,cjs}\" \"tools/**/*.{js,mjs,cjs}\"",
+    "lint:fix": "pnpm run lint --fix",
     "check:personal-paths": "node scripts/check-personal-paths.cjs",
     "check:all-personal-paths": "node scripts/check-all-personal-paths.cjs",
     "check:docs": "node scripts/check-docs.mjs",

tests/integration/dev-proxy-startup.test.ts

@@ -156,3 +156,47 @@ describe('dev-proxy initial tool discovery (issue #716)', () => {
     expect((await restart).result?.isError).not.toBe(true);
   });
 });
+
+describe('dev-proxy rebuild failure reporting (issue #718)', () => {
+  it('reports a failed build by its sanitized message alone and keeps the running backend', async () => {
+    const { client, notifications } = await connect({
+      DEV_PROXY_BUILD_CMD: `"${process.execPath}" -e "console.error('boom: build exploded'); process.exit(1)"`,
+    });
+    expect((await client.listTools()).tools.map(tool => tool.name)).toEqual(['fixture_tool', ...devTools]);
+    // The successful initial start announces its inventory once; nothing below may add to that.
+    await expect.poll(() => notifications).toHaveLength(1);
+
+    const failed = await client.callTool({ name: 'dev_rebuild_and_restart', arguments: {} });
+    expect(failed.isError).toBe(true);
+    const payload = JSON.parse((failed.content as Array<{ text: string }>)[0]?.text ?? '{}');
+    // The build's own output reaches the caller only through the sanitized
+    // message (issue #154): the payload is the message and nothing else.
+    expect(Object.keys(payload).sort()).toEqual(['error', 'success']);
+    expect(payload.success).toBe(false);
+    expect(payload.error).toMatch(/^Build failed: /);
+    expect(payload.error).toContain('boom: build exploded');
+
+    // The build failed before the restart, so the old backend still serves —
+    // and the inventory it serves did not change, so nothing was announced.
+    expect((await client.listTools()).tools.map(tool => tool.name)).toEqual(['fixture_tool', ...devTools]);
+    const forwarded = await client.callTool({ name: 'fixture_tool', arguments: {} });
+    expect(forwarded.isError, JSON.stringify(forwarded)).not.toBe(true);
+    expect(notifications).toHaveLength(1);
+  });
+
+  it('reports a timed-out build as a timeout, with the output it had produced', async () => {
+    const { client, notifications } = await connect({
+      DEV_PROXY_BUILD_TIMEOUT_MS: '1000',
+      DEV_PROXY_BUILD_CMD: `"${process.execPath}" -e "console.log('vendoring step 3 of 9'); setTimeout(() => {}, 20000)"`,
+    });
+    expect((await client.listTools()).tools.map(tool => tool.name)).toEqual(['fixture_tool', ...devTools]);
+    await expect.poll(() => notifications).toHaveLength(1);
+
+    const failed = await client.callTool({ name: 'dev_rebuild_and_restart', arguments: {} });
+    expect(failed.isError).toBe(true);
+    const payload = JSON.parse((failed.content as Array<{ text: string }>)[0]?.text ?? '{}');
+    expect(payload.error).toMatch(/^Build timed out after 1s/);
+    expect(payload.error).toContain('vendoring step 3 of 9');
+    expect(notifications).toHaveLength(1);
+  });
+});

tools/dev-proxy/dev-proxy.mjs

@@ -75,6 +75,10 @@ const BACKEND_CMD = process.env.DEV_PROXY_BACKEND_CMD || null;
 
 const parsedTimeout = parseInt(process.env.DEV_PROXY_BUILD_TIMEOUT_MS || '', 10);
 const BUILD_TIMEOUT_MS = Number.isFinite(parsedTimeout) && parsedTimeout > 0 ? parsedTimeout : 120000;
+// Well above any tsc/esbuild transcript. execSync's 1 MiB default kills an
+// over-chatty build with the same SIGTERM a timeout uses (ENOBUFS), which
+// used to be reported as a timeout.
+const BUILD_MAX_BUFFER_BYTES = 64 * 1024 * 1024;
 
 // How long a request will wait for an in-flight backend start or restart.
 // Deliberately NOT tied to HEALTH_POLL_TIMEOUT_MS: that bounds how patient the
@@ -359,31 +363,32 @@ class BackendManager {
         encoding: 'utf-8',
         stdio: ['ignore', 'pipe', 'pipe'],
         timeout: BUILD_TIMEOUT_MS,
+        maxBuffer: BUILD_MAX_BUFFER_BYTES,
         env: { ...process.env },
       });
     } catch (err) {
-      if (err.killed || err.signal === 'SIGTERM') {
-        throw new Error(
-          `Build timed out after ${Math.floor(BUILD_TIMEOUT_MS / 1000)}s — the build may still have succeeded, re-run manually to confirm`
-        );
-      }
-
       // execSync's error message embeds raw build stderr — sanitize before it
       // reaches tool responses via err.message (issue #154). Include stdout
-      // too: build tools (tsc via npm) print their diagnostics there.
+      // too: build tools (tsc via npm) print their diagnostics there. The raw
+      // execSync error rides on `cause` for programmatic consumers; the tool
+      // handlers serialize `message` alone, so it never reaches a response.
       const output = [err.stdout, err.stderr].filter(Boolean).join('\n') || err.message || String(err);
-      throw new Error(`Build failed: ${sanitizeStderrTail(output, { maxLines: 20, maxChars: 2000 })}`);
+      const tail = sanitizeStderrTail(output, { maxLines: 20, maxChars: 2000 });
+      // Only the timeout is a kill by this proxy. `err.killed` is never set by
+      // execSync, and signal SIGTERM alone does not identify it: a buffer
+      // overflow (ENOBUFS) arrives the same way.
+      if (err.code === 'ETIMEDOUT') {
+        throw new Error(
+          `Build timed out after ${Math.floor(BUILD_TIMEOUT_MS / 1000)}s — the build may still have succeeded, re-run manually to confirm. Output before the timeout:\n${tail}`,
+          { cause: err }
+        );
+      }
+      throw new Error(`Build failed: ${tail}`, { cause: err });
     }
     log('Build succeeded');
     return sanitizeStderrTail(result, { maxLines: 50, maxChars: 2000 });
   }
 
-  async rebuildAndRestart() {
-    const buildOutput = this.rebuild();
-    await this.restart();
-    return buildOutput;
-  }
-
   /**
    * Resolve once no lifecycle operation is in flight, so a request that lands
    * during the initial start or a restart sees the backend it is about to get
@@ -787,32 +792,65 @@ async function notifyToolListChanged(server) {
   }
 }
 
+/** The failed dev-tool response: the message alone (issue #154). */
+function devToolFailure(err) {
+  return {
+    content: [{ type: 'text', text: JSON.stringify({ success: false, error: err.message }, null, 2) }],
+    isError: true,
+  };
+}
+
+/**
+ * Build, then restart. A failed build never reaches restart(): the running
+ * backend — and so the client's tool inventory — is untouched, so no
+ * tools/list_changed is sent for it. Only a restart can change the inventory.
+ */
+async function rebuildThenRestart(backend, server) {
+  let buildOutput;
+  try {
+    buildOutput = backend.rebuild();
+  } catch (err) {
+    return devToolFailure(err);
+  }
+  try {
+    await backend.restart();
+    await server.sendToolListChanged();
+    return {
+      content: [
+        {
+          type: 'text',
+          text: JSON.stringify(
+            {
+              success: true,
+              action: 'rebuild_and_restart',
+              buildOutput,
+              status: backend.getStatus(),
+            },
+            null,
+            2
+          ),
+        },
+      ],
+    };
+  } catch (err) {
+    await notifyToolListChanged(server);
+    return devToolFailure(err);
+  }
+}
+
 async function handleDevTool(backend, server, name, args) {
   switch (name) {
     case 'dev_restart_debugger': {
+      // A rejected env update changes nothing either: no notification.
       try {
         backend.applyEnvUpdate(args);
-        if (args?.rebuild) {
-          const buildOutput = await backend.rebuildAndRestart();
-          await server.sendToolListChanged();
-          return {
-            content: [
-              {
-                type: 'text',
-                text: JSON.stringify(
-                  {
-                    success: true,
-                    action: 'rebuild_and_restart',
-                    buildOutput,
-                    status: backend.getStatus(),
-                  },
-                  null,
-                  2
-                ),
-              },
-            ],
-          };
-        }
+      } catch (err) {
+        return devToolFailure(err);
+      }
+      if (args?.rebuild) {
+        return rebuildThenRestart(backend, server);
+      }
+      try {
         await backend.restart();
         await server.sendToolListChanged();
         return {
@@ -825,42 +863,17 @@ async function handleDevTool(backend, server, name, args) {
         };
       } catch (err) {
         await notifyToolListChanged(server);
-        return {
-          content: [{ type: 'text', text: JSON.stringify({ success: false, error: err.message }, null, 2) }],
-          isError: true,
-        };
+        return devToolFailure(err);
       }
     }
 
     case 'dev_rebuild_and_restart': {
       try {
         backend.applyEnvUpdate(args);
-        const buildOutput = await backend.rebuildAndRestart();
-        await server.sendToolListChanged();
-        return {
-          content: [
-            {
-              type: 'text',
-              text: JSON.stringify(
-                {
-                  success: true,
-                  action: 'rebuild_and_restart',
-                  buildOutput,
-                  status: backend.getStatus(),
-                },
-                null,
-                2
-              ),
-            },
-          ],
-        };
       } catch (err) {
-        await notifyToolListChanged(server);
-        return {
-          content: [{ type: 'text', text: JSON.stringify({ success: false, error: err.message }, null, 2) }],
-          isError: true,
-        };
+        return devToolFailure(err);
       }
+      return rebuildThenRestart(backend, server);
     }
 
     case 'dev_server_status': {