[Sanilas-ai]

Fixes #1804.

serve-instrument exited when an instrument directory was deleted or renamed while it ran, or when it got a URL with a malformed percent-escape. In both cases a promise nobody handled was rejected, and Node turned that into a process exit. In --all mode that took down every instrument.

Changes

packages/serve-instrument/src/server.tsx:

• updateEncodedBundle now does the readdir/readFile calls inside its try. Its catch sets encodedBundle = null, so a deleted instrument answers 404 (503 in single mode) and doesn't keep serving its last bundle, as the triage comment pointed out.
• Each InstrumentLoader watcher has an 'error' listener. It logs the error and clears the bundle, so the next request rebuilds. This covers the Windows EPERM-on-delete case.
• Server's request callback catches a rejected handle, logs it and answers 500.
• All three page branches render first and call writeHead afterwards. Every await in both handle methods now runs before writeHead, so the .catch can always send a 500.
• A URIError from decodeURIComponent is caught where it is thrown and answers 400.
• The three copies of the error-logging code are now one logFailure helper, so the non-Error branch is covered once.

Docs:

• packages/serve-instrument/AGENTS.md has a new trap about the never-reject-unhandled / render-before-writeHead rule, and the test notes now describe the new mock.
• .agents/docs/architecture/testing-strategy.md: fixed the serve-instrument row. The mock lives in src/__tests__/server.test.ts, and Server.stop() does close the watchers. build: upgrade dependencies held back by breaking changes #1660 merged with the old text still there.

Verification

• src/__tests__/server.test.ts: the node:fs mock now records each watcher's target and callback and returns an EventEmitter with close. There are 4 new tests:
  • deleting forms/BAR and firing its callback: FOO still answers 200 and BAR answers 404
  • /forms/%E0%A4%A answers 400 and / still answers 200
  • a client.js read rejects once: the response is 500, the error is logged, and the next request answers 200
  • a watcher 'error': the error is logged and the next request calls bundle again
• With main's server.tsx, all 4 new tests fail (unhandled ENOENT / URIError rejections and timeouts). With this change, all 46 tests in the serve-instrument project pass, and server.tsx, cli.ts, client.tsx and root.tsx are at 100% statements/branches/functions/lines.
• pnpm --filter @opendatacapture/serve-instrument lint (tsc + eslint) passes.
• Manual check on Linux with the built dist/cli.js --all and two forms. After rm -rf forms/BAR the server stays up: FOO answers 200, BAR answers 404, /forms/%E0%A4%A answers 400 and / answers 200.

What I couldn't run:

• No e2e test: serve-instrument isn't part of the stack under testing/, as the issue says.
• I didn't run the full root pnpm test:coverage / pnpm test:e2e. My machine's CPU has no AVX, so MongoDB (and mongodb-memory-server in the api tests) dies with SIGILL. CI will run the full suite.

[joshunrau]

Thanks for this. It's a careful fix, and it follows the plan on #1804 closely, including the stale-bundle and watcher 'error' points. I ran your four new tests against main's server.tsx, and each one fails with the crash the issue describes. When I removed either encodedBundle = null, or moved the single-mode writeHead back above the render, a different test caught each change.

One change before merge:

1. Please also test the 500 path in --all mode. The render-before-writeHead change is tested only in single mode. I moved res.writeHead(200) back above await this.renderPage(...) on the --all index page and on the instrument page (server.tsx lines 253-255 and 276-283), and all 27 server tests still passed. With that order, a failed render makes the .catch call writeHead(500) after the head has already been written. That throws ERR_HTTP_HEADERS_SENT inside the .catch and the process exits. That is the crash this PR fixes, in the mode where it takes down every instrument. To cover it, add an --all counterpart to should answer 500 and keep serving when handling a request fails. After a first successful request (so the instrument bundle is already cached, as in the single-mode test), make the client.js read reject once on / and once on /forms/<name>. Assert a 500, then a 200 on the next request.

Reviewed at commit 26389e3.