Skip to content

Three small cleanups flagged in the v2.2.0 merge review #2000

Description

@cliffhall

Three independent small items Copilot raised while reviewing the v2.2.0 milestone merge (#1993), each verified. Grouped because none is worth its own card; each is a few lines.

1. clients/web/.npmignore's comment now contradicts the root manifest

#1934 added clients/web/static to the root package.json "files" allowlist, but the explanatory comment in clients/web/.npmignore:11-15 still says the allowlist "restricts publishing to those two directories" (build/ and dist/) and that everything else under clients/web "stays out regardless". That is now false, and it is exactly the kind of packaging claim someone will rely on — the file exists because a packaging contract was misread once already.

Update the comment to name static and say why it ships (read from disk at runtime by sandbox-controller.ts).

2. Unjustified as unknown as Response in a test

clients/web/src/test/core/react/useServers.test.tsx:1016 returns { ok: true, body } as unknown as Response from a stub fetchFn. AGENTS.md prohibits an unjustified double cast: it must either carry an inline comment explaining why no better option exists, or be replaced.

Preferred: build a real Response over a real ReadableStream whose second read blocks, so the double is structurally type-checked. Failing that, add the justification comment.

3. scripts/smoke-web-app.mjs can orphan the test server on a readiness timeout

startMcpServer() (scripts/smoke-web-app.mjs:164-196) returns { child, url } only once the announcement line matches. On the 30-second timeout — a child that is alive but never announces — it throws without ever handing the handle back, so mcpServer is unassigned, the caller's shutdown() cannot stop it, and process.exit(1) leaves the server orphaned, possibly holding its port for a later run.

Make the child reachable before the wait (assign it via an out-param, or have the caller own the spawn), so teardown covers the timeout path. The other early-exit paths (spawnError, exited) are fine — the child is already gone there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior changev2Issues and PRs for v2

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions