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.

Activity

  1. added this to the v2.3.0 milestone on Aug 12, 2026
  2. added
    v2Issues and PRs for v2
    choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior change
    on Aug 12, 2026
  3. cliffhall commented on Aug 12, 2026

    @cliffhall
    MemberAuthor

    Triage: Priority Low (total 4)

    • Severity 2 — minor: one stale comment, one test-only cast, one smoke-teardown gap; no user-facing behavior change
    • Urgency 2 — wanted eventually
    • Bonuses: none (no bug label; unassigned; no engagement)

    Board: #28, Status Todo, milestone v2.3.0.

  4. modified the milestones: v2.3.0, v2.4.0 on Aug 16, 2026
  5. self-assigned this
    on Aug 23, 2026
  6. added a commit that references this issue on Aug 26, 2026
    031991c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

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