test: expand adapter-claude and mcp-server coverage, fix transport harness#46
Open
iliassjabali wants to merge 4 commits intomainfrom
Open
test: expand adapter-claude and mcp-server coverage, fix transport harness#46iliassjabali wants to merge 4 commits intomainfrom
iliassjabali wants to merge 4 commits intomainfrom
Conversation
The 18 failing tests in transport.test.ts all share one root cause: the harness spawned `node packages/mcp-server/dist/index.js`, but `dist/` does not exist on a fresh worktree (no pretest build hook). Spawned processes exited with MODULE_NOT_FOUND, stdio reads hung, and 17 tests hit the 5s vitest timeout while the "exits when stdin closes" test got exit code 1 (instead of 0) for the same reason. Switch BIN to spawn `tsx src/index.ts` directly via the package-local tsx binary, removing the build dependency entirely. Also: - Await child exit in afterEach so the next test starts on a clean process table and isn't fighting an orphaned readline interface. - Wait for the "running on stdio" startup banner in the "exits when stdin closes" test before closing stdin, so the readline close handler is wired up before the close event fires (tsx cold-start is ~1-2s slower than node + JS). - Bump per-test timeout to 15s on every transport test to absorb the tsx cold-start cost without affecting other suites. After: 18/18 transport tests pass, including from a fresh worktree (`rm -rf packages/mcp-server/dist && pnpm --filter @agentspec/mcp test`).
Adds 9 new tests covering gaps in the existing 45-test suite (issue #24): - repairYaml() (4 tests): happy path returns Claude's repaired YAML; throws when ANTHROPIC_API_KEY missing; throws when response lacks the agent.yaml field; system prompt mentions AgentSpec v1 schema rules and the user message embeds the broken YAML for context. - generateWithClaude() prompt structure (2 tests): the user message wraps the manifest JSON in <context_manifest> tags AND embeds resolved $file: refs as <context_file> blocks with their actual content. These are the regression-sensitive tests where silent prompt drift breaks generation quality without any failed assertion downstream. - generateWithClaude() ignores non-text content blocks in the response array (e.g. tool_use), parsing only the text block. - generateWithClaude() streaming path surfaces an error thrown mid-stream from the async iterable. - buildContext() silently skips $file: refs that resolve to a directory rather than a file (readFileSync EISDIR fall-through). All 54 tests pass.
Closes the largest gap from issue #24: two registered MCP tools (agentspec_scan, agentspec_generate) had zero direct test files. Also adds a tool-registry snapshot test to catch schema drift, which is the most common silent break for MCP clients (Claude Desktop, Cursor, etc.) when a tool's input schema changes without anyone noticing. New test files: - scan.test.ts (3 tests): mocks spawnCli, asserts CLI args ['scan', '--dir', dir, '--dry-run'], output trimming, error propagation. - generate.test.ts (4 tests): asserts CLI args with and without --out, JSON-wrapped success response, error propagation. - index.test.ts (11 tests): - TOOLS registry snapshot guard — locks the names + input schemas of all 9 registered tools via toMatchInlineSnapshot. Free-form descriptions are stripped from the snapshot so wording tweaks don't churn the diff; only the contract (name, properties, required) is locked. - Per-tool invariants: object schema, properties present, required is an array, description non-empty. - handleRpc dispatch tests covering initialize, tools/list, ping, method-not-found (-32601), invalid-request (-32600), missing tool name (-32602), and tool error wrapping as isError content blocks. To enable importing the entry-point module from a unit test, src/index.ts now guards the startStdio/startHttp call behind an isMainEntry() check (standard ESM "if executed as main" pattern). Without this, importing { TOOLS, handleRpc } in index.test.ts would spawn a readline interface and call process.exit(0) when vitest's stdin closed, surfacing as an unhandled error. 123 mcp-server tests pass.
…operator network errors Adds the error-path scenarios highlighted as missing in #24: - health.test.ts: sidecar returns 200 with malformed JSON (a common failure mode when a reverse proxy serves an HTML error page — res.json() throws SyntaxError). Plus a hung-connection / fetch timeout test, since a silently dropped connection is worse than a refused one (the client waits forever instead of failing fast). - gap.test.ts: control-plane (operator-mode) fetch fails with a network error like ENOTFOUND — the existing tests covered the sidecar-mode equivalent but never exercised fetchFromControlPlane's rejection path. 123 mcp-server tests still pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #24.
Summary
repairYaml()(zero prior coverage), prompt structure (manifest wrapped in<context_manifest>, resolved$file:refs embedded as<context_file>blocks), streaming-error propagation, non-text content blocks, and$file:-as-directory edge case. 45 → 54 tests.scan.test.ts,generate.test.ts) for the previously-untestedagentspec_scanandagentspec_generatetools. Newindex.test.tswith an inline snapshot of the full TOOLS registry (names + input schemas) to catch schema drift, plushandleRpcdispatch coverage. Adds sidecar malformed-JSON, hung-connection (timeout), and operator-mode network-error tests. 102 → 123 tests.node dist/index.jswithout a pretest build hook. Switched totsx src/index.ts, awaited child exit inafterEach, and isolated theexits when stdin closestest from the afterEach SIGTERM race. Fresh-worktree run (rm -rf packages/mcp-server/dist && pnpm --filter @agentspec/mcp test) now passes.mcp-server/src/index.tsso the module can be imported by unit tests without spawning a readline interface.Acceptance criteria (#24)
pnpm testTest plan
pnpm --filter @agentspec/adapter-claude test→ 54 passingpnpm --filter @agentspec/mcp test→ 123 passingpnpm test(workspace) → sdk 285, sidecar 245, cli 436, adapter-claude 54, mcp-server 123, all greenpnpm -r typecheck→ greenrm -rf packages/mcp-server/dist && pnpm --filter @agentspec/mcp test→ 123/123