From 80834eb8b24f41ac30977f0ffa10b15b36a73f31 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 18:05:12 -0400 Subject: [PATCH 1/2] chore(lint): cover tools/ in the lint gate; attach cause in dev-proxy rebuild() (#718) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pnpm lint` (pre-push and CI) linted src/, packages/*/src/ and scripts/ but not tools/, so the dev-proxy supervisor sat outside the gate with two preserve-caught-error findings in BackendManager.rebuild() that nothing reported. - package.json: add "tools/**/*.{js,mjs,cjs}" to `lint`; `lint:fix` now runs the same globs with --fix instead of src/**/*.ts alone, so the hook's "run lint:fix" hint holds for every finding lint reports. tools/ contains only the dev-proxy; experimental probes live in scripts/experiments/, already ignored by eslint.config.js. - dev-proxy.mjs: both rebuild() rethrows attach the caught execSync error as `cause`. The tool handlers serialize err.message alone, so the raw build output never reaches a response (issue #154's invariant). - tests/integration/dev-proxy-startup.test.ts: drive dev_rebuild_and_restart through a failing DEV_PROXY_BUILD_CMD — the payload is { success, error } with a "Build failed:" message, no cause, and the old backend keeps serving. rebuild() had no coverage before. - CONTRIBUTING, AGENTS.md and the adapter guide's checklist state the new glob set (the guide's line already omitted scripts/). Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- CONTRIBUTING.md | 12 +++++---- changelog.d/718.fixed.md | 1 + .../architecture/adapter-development-guide.md | 2 +- package.json | 4 +-- tests/integration/dev-proxy-startup.test.ts | 25 +++++++++++++++++++ tools/dev-proxy/dev-proxy.mjs | 9 ++++--- 7 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 changelog.d/718.fixed.md diff --git a/AGENTS.md b/AGENTS.md index be0f65004..a3ee2974f 100644 --- a/AGENTS.md +++ b/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. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 727a2eb0a..69478537d 100644 --- a/CONTRIBUTING.md +++ b/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 ``` diff --git a/changelog.d/718.fixed.md b/changelog.d/718.fixed.md new file mode 100644 index 000000000..85fea8fa0 --- /dev/null +++ b/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 an integration test that drives `dev_rebuild_and_restart` through a failing build), and `pnpm lint:fix` runs the same globs as `pnpm lint` instead of `src/**/*.ts` only, so the hook's "run lint:fix" hint is true for every finding it reports (#718) diff --git a/docs/architecture/adapter-development-guide.md b/docs/architecture/adapter-development-guide.md index 5d9eee690..45737f8f8 100644 --- a/docs/architecture/adapter-development-guide.md +++ b/docs/architecture/adapter-development-guide.md @@ -489,7 +489,7 @@ The loader: - [ ] Unit and integration tests written under `tests/adapters//` - [ ] 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) diff --git a/package.json b/package.json index f0ded5a6f..649bc3f68 100644 --- a/package.json +++ b/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": "eslint \"src/**/*.ts\" \"packages/*/src/**/*.ts\" \"scripts/**/*.{js,mjs,cjs}\" \"tools/**/*.{js,mjs,cjs}\" --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", diff --git a/tests/integration/dev-proxy-startup.test.ts b/tests/integration/dev-proxy-startup.test.ts index 8dfe5f8db..f6f7440fe 100644 --- a/tests/integration/dev-proxy-startup.test.ts +++ b/tests/integration/dev-proxy-startup.test.ts @@ -156,3 +156,28 @@ 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 message only, keeps the running backend, and attaches no cause chain', async () => { + const { client } = 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]); + + 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 caught execSync error rides on `cause` for + // programmatic consumers and must not be serialized into the response. + 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 rebuild failed before the restart, so the old backend still serves. + 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); + }); +}); diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 362eca1a8..191620307 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/tools/dev-proxy/dev-proxy.mjs @@ -364,15 +364,18 @@ class BackendManager { } 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` + `Build timed out after ${Math.floor(BUILD_TIMEOUT_MS / 1000)}s — the build may still have succeeded, re-run manually to confirm`, + { cause: err } ); } // 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 })}`); + throw new Error(`Build failed: ${sanitizeStderrTail(output, { maxLines: 20, maxChars: 2000 })}`, { cause: err }); } log('Build succeeded'); return sanitizeStderrTail(result, { maxLines: 50, maxChars: 2000 }); From 4aa0f41a2b4afb163fc73c8863f279d904756e21 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 18:35:14 -0400 Subject: [PATCH 2/2] fix(dev-proxy): identify a build timeout by ETIMEDOUT, keep its partial output, and never announce an inventory change for a failed build (#718) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #745 on the catch blocks the lint gate now covers: - `err.killed || err.signal === 'SIGTERM'` misclassified an execSync ENOBUFS kill (1 MiB default buffer) as a timeout — both arrive with signal SIGTERM and execSync never sets `killed` (probed: timeout → code ETIMEDOUT, overflow → code ENOBUFS). Discriminate on ETIMEDOUT and raise maxBuffer to 64 MiB so an over-chatty build is not killed at all. - The timeout message now carries the sanitized output produced before the kill, the way the failure message already did. - A failed build never reaches restart(), so the running backend's tool inventory is unchanged; the handlers used to send tools/list_changed for it anyway. Build and restart are now sequenced by one helper that notifies only for a restart failure (rebuildAndRestart() is gone; a rejected env update no longer notifies either). - lint:fix is `pnpm run lint --fix`: one glob list, no second copy to drift (the drift this issue is about). - Integration tests: the failed-build case asserts no notification is added past the initial start's own; a new hanging-build case asserts the timeout message and its partial output. Both fail on the previous dev-proxy.mjs. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/718.fixed.md | 2 +- package.json | 2 +- tests/integration/dev-proxy-startup.test.ts | 29 ++++- tools/dev-proxy/dev-proxy.mjs | 136 +++++++++++--------- 4 files changed, 99 insertions(+), 70 deletions(-) diff --git a/changelog.d/718.fixed.md b/changelog.d/718.fixed.md index 85fea8fa0..0d1e73ede 100644 --- a/changelog.d/718.fixed.md +++ b/changelog.d/718.fixed.md @@ -1 +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 an integration test that drives `dev_rebuild_and_restart` through a failing build), and `pnpm lint:fix` runs the same globs as `pnpm lint` instead of `src/**/*.ts` only, so the hook's "run lint:fix" hint is true for every finding it reports (#718) +**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) diff --git a/package.json b/package.json index 649bc3f68..094796c5e 100644 --- a/package.json +++ b/package.json @@ -100,7 +100,7 @@ "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}\" \"tools/**/*.{js,mjs,cjs}\"", - "lint:fix": "eslint \"src/**/*.ts\" \"packages/*/src/**/*.ts\" \"scripts/**/*.{js,mjs,cjs}\" \"tools/**/*.{js,mjs,cjs}\" --fix", + "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", diff --git a/tests/integration/dev-proxy-startup.test.ts b/tests/integration/dev-proxy-startup.test.ts index f6f7440fe..ca03784e6 100644 --- a/tests/integration/dev-proxy-startup.test.ts +++ b/tests/integration/dev-proxy-startup.test.ts @@ -158,26 +158,45 @@ describe('dev-proxy initial tool discovery (issue #716)', () => { }); describe('dev-proxy rebuild failure reporting (issue #718)', () => { - it('reports a failed build by message only, keeps the running backend, and attaches no cause chain', async () => { - const { client } = await connect({ + 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 caught execSync error rides on `cause` for - // programmatic consumers and must not be serialized into the response. + // 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 rebuild failed before the restart, so the old backend still serves. + // 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); }); }); diff --git a/tools/dev-proxy/dev-proxy.mjs b/tools/dev-proxy/dev-proxy.mjs index 191620307..32e7d2818 100644 --- a/tools/dev-proxy/dev-proxy.mjs +++ b/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,34 +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`, - { cause: err } - ); - } - // 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. 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 })}`, { cause: err }); + 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 @@ -790,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 { @@ -828,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': {