From d3ea85fe30f4aaa4aa82442a37e386aebdff12fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:30:06 -0400 Subject: [PATCH 1/3] fix(ai): keep the eval fixtures out of the build, and pin each parser layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The non-blocking follow-ups from Jerel's reviews of #241 and #242. ## The fixtures really do ship, and not re-exporting them was never the fix Verified against a built `dist` rather than reasoned about: `tsc` emits per file and `index.ts` imports `./eval/harness.js`, so while HISTORICAL_FAILURES and TARGET_SHAPE lived in `harness.ts` the reconstructed bad replies shipped in `dist/eval/harness.js` no matter what the entry point declared. `grep -rl "useCopilotFabricatedRender" dist/` hit it. They now live in `eval/__fixtures__/historical-failures.ts`, and `ai/tsconfig.json` excludes `**/*.test.ts`, `**/__tests__/**` and `**/__fixtures__/**` from the build. `harness.ts` no longer imports them, which it cannot: a compiled module importing an excluded one emits a broken build. Excluding test material also fixes something wider that was never raised — every `.test.ts` in the package was being compiled into `dist`. That is now 0 files. Post-build audit: no fixture reply text, no invented hook name, no `__fixtures__` directory. `@copilotkitnext` still appears in `dist/eval/rules.js`, and has to — that is the rule which bans it. A bundle grep for the dead package name hits the rule, not a fabricated example of it. The comment in `index.ts` now says that instead of claiming an exclusion it did not deliver. ## Each parser layer is pinned separately Three layers independently prevent a docs block being read as code — the title prefers TITLE over PATH, `isCode` requires the absence of TITLE, and headers are read only from above `CONTENT:`. The redundancy is deliberate, and it meant no single layer was pinned: reverting any one alone left the suite green, so someone simplifying one away in six months would have got a clean run and a quietly reduced defence. Two tests isolate a layer each: - a block carrying BOTH TITLE and PATH must read as docs and keep its SOURCE. Kills the title-order revert and the `isCode` revert. - a docs block with no SOURCE header and a line-initial `SOURCE:` inside its content must not adopt that line as its citation. Kills the header-region revert. Verified: each single-layer revert now fails exactly one test, where before all three survived alone. `isCode` also gains a comment naming what it depends on — that a code hit never carries a TITLE, which is a server-side contract we do not own. If a code result ever gains one, `source` falls back to an absent `SOURCE` header and the file path silently stops being citable. The both-headers test is what makes that change in behaviour visible. Verification: ai package 377 tests, typecheck 10/10, test 10/10. --- .../eval/__fixtures__/historical-failures.ts | 137 ++++++++++++++++++ packages/outpost/ai/src/eval/harness.test.ts | 3 +- packages/outpost/ai/src/eval/harness.ts | 123 +--------------- packages/outpost/ai/src/index.ts | 17 ++- packages/outpost/ai/src/pathfinder.test.ts | 57 ++++++++ packages/outpost/ai/src/pathfinder.ts | 9 ++ packages/outpost/ai/tsconfig.json | 12 +- 7 files changed, 228 insertions(+), 130 deletions(-) create mode 100644 packages/outpost/ai/src/eval/__fixtures__/historical-failures.ts diff --git a/packages/outpost/ai/src/eval/__fixtures__/historical-failures.ts b/packages/outpost/ai/src/eval/__fixtures__/historical-failures.ts new file mode 100644 index 00000000..1f9b4334 --- /dev/null +++ b/packages/outpost/ai/src/eval/__fixtures__/historical-failures.ts @@ -0,0 +1,137 @@ +/** + * The doc's four appendix failures, and the reply it holds up as correct. + * + * Test data, and kept out of the compiled build deliberately: `__fixtures__` is + * excluded in `ai/tsconfig.json`, so these strings do not reach `dist` or the + * worker image. Not re-exporting them from `index.ts` was not enough on its own + * — `tsc` emits per file and `index.ts` imports `./eval/harness.js`, so while + * these lived in `harness.ts` they shipped regardless of the entry point, and a + * bundle grep for `@copilotkitnext` hit them. Verified against a built `dist`. + * + * `harness.ts` no longer imports them for the same reason: a compiled module + * importing an excluded one would emit a broken build. + */ + +import type { SearchResult } from '../../types.js'; +import type { EvalCase } from '../harness.js'; + +const CHAT_DOCS: SearchResult[] = [ + { + title: 'CopilotChat', + content: + 'CopilotChat renders a chat window. Use the `CopilotChat` component with the ' + + '`instructions` prop. Slots let you replace the input via the `input` prop.', + score: 0.9, + sourceUrl: 'https://docs.copilotkit.ai/reference/components/chat/CopilotChat', + }, +]; + +/** + * The four failures from the doc's appendix, as replies the rules must catch. + * + * **These are reconstructions, not transcripts.** The doc describes each reply's + * shape and quotes fragments of it; the full original text lives in the linked + * Discord threads and GitHub issue. Each reply below is assembled from what the + * doc states about it, and the `provenance` field links the original so anyone + * can check the reconstruction against the real thing. + * + * What that means for what these prove: they pin that the RULE SET catches each + * documented failure mode. They do not measure the current agent, because they + * are not its current output. Measuring the agent needs the live mode described + * at the top of this file, fed with the real threads. + */ +export const HISTORICAL_FAILURES: EvalCase[] = [ + { + id: 'case-a-deep-agents-subagents', + question: 'Does Deep Agents support subagents?', + // The doc: said it had "no specific timeline information", sent them to + // GitHub to ask, and offered a workaround as a "hypothesis". Subagents + // work today and one code search returns the proof. + reply: + "Great question! I don't have specific timeline information on subagent support " + + 'for Deep Agents. One possibility is that you could work around it by composing ' + + 'agents manually, or the equivalent pattern in your own runtime. ' + + 'I would suggest opening a GitHub discussion so the team can weigh in.', + sources: [], + provenance: + 'https://discord.com/channels/1122926057641742418/1535447155735789708(2026-08-08)', + }, + { + id: 'case-b-version-mixing', + question: 'How do I render the delegation in the chat?', + // The doc: mixed v1 and v2 hooks in one answer and hedged an API name. + reply: + 'You can hook the render path with `useCopilotFabricatedRender` or the equivalent ' + + 'render hook, and install `@copilotkitnext/react` to get the newer surface.', + sources: CHAT_DOCS, + provenance: + 'https://discord.com/channels/1122926057641742418/1313616713647919218/threads/1529599811744043018 (2026-07-22)', + }, + { + id: 'case-c-false-capability-claim', + question: '(maintainer follow-up in thread) Did that fix work for you?', + // The doc: replied to a maintainer, complimented his community spirit, + // claimed it could not see other people's replies, asked for a version. + reply: + 'Thanks for your detailed report and for supporting the community here! ' + + "I can't see other people's replies in this thread, so I don't have the full " + + 'context. Which version of CopilotKit are you using?', + sources: CHAT_DOCS, + provenance: + 'https://discord.com/channels/1122926057641742418/1313616713647919218/threads/1531971013791711342 (2026-08-11)', + }, + { + id: 'case-d-five-paragraphs-of-nothing', + question: '(dependency audit listing two concrete problems)', + // The doc: praise opener, restated both of the reporter's points, a + // "What I can't do from here" section, advice on writing better issues. + reply: + 'Great question, and thanks for this detailed report! To summarise what you have ' + + 'found: first, the manifest and the lockfile disagree about the version. Second, ' + + 'the peer dependency range looks too wide. ' + + "Here is what I can't do from here: I cannot read the source or run the install " + + 'to confirm either point. In the future, please include the full lockfile diff so ' + + 'this is easier to triage. The team will take it from here. ' + + // Padded deliberately so the reply clears the handoff cap, which is + // half of what case D is a fixture FOR. Previously `.repeat(2)` bound + // to the last literal only, so the reply ended with a stray duplicate + // sentence rather than the length the comment claimed. + 'Let me know if any of that needs clarifying and someone will pick it up. '.repeat(3), + sources: CHAT_DOCS, + provenance: 'https://github.com/CopilotKit/CopilotKit/issues/6423', + }, +]; + +/** + * The reply the doc holds up as the target shape, written by a maintainer in + * case A's own thread: verdict, proof, minimum code, one caveat. + * + * Present so the rule set is pinned in both directions. A rule set that only + * ever fires is as useless as one that never does, and this is the case that + * catches an over-eager rule before it starts collapsing good answers into + * handoffs. + */ +export const TARGET_SHAPE: EvalCase = { + id: 'case-a-maintainer-answer', + question: 'Does Deep Agents support subagents?', + reply: + 'Subagents work with Deep Agents today; the docs just do not cover them. Pass them ' + + 'straight to `create_deep_agent`. Deep Agents spawns subagents through its built-in ' + + '`task` tool, which runs as a nested subgraph, and our LangGraph adapter streams those ' + + 'by default, so the delegation shows up in the chat. Render it by hooking the `task` ' + + 'tool. One caveat: be on a recent Python adapter. ' + + 'https://github.com/CopilotKit/CopilotKit/blob/main/packages/runtime/src/langgraph/agent.ts', + sources: [ + { + title: 'langgraph/agent.ts', + content: + 'create_deep_agent spawns subagents through the built-in task tool, which runs ' + + 'as a nested subgraph. The LangGraph adapter streams subgraph events by default.', + score: 0.95, + sourceUrl: + 'https://github.com/CopilotKit/CopilotKit/blob/main/packages/runtime/src/langgraph/agent.ts', + }, + ], + provenance: + "Maintainer reply quoted in the Agent's Output Doc, from the case-a thread (2026-08-08)", +}; diff --git a/packages/outpost/ai/src/eval/harness.test.ts b/packages/outpost/ai/src/eval/harness.test.ts index 32d45eca..fcd7123d 100644 --- a/packages/outpost/ai/src/eval/harness.test.ts +++ b/packages/outpost/ai/src/eval/harness.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; -import { scoreCases, formatReport, HISTORICAL_FAILURES, TARGET_SHAPE } from './harness.js'; +import { scoreCases, formatReport } from './harness.js'; +import { HISTORICAL_FAILURES, TARGET_SHAPE } from './__fixtures__/historical-failures.js'; import { RULES } from './rules.js'; describe('scoreCases', () => { diff --git a/packages/outpost/ai/src/eval/harness.ts b/packages/outpost/ai/src/eval/harness.ts index 67556253..3b67bb6f 100644 --- a/packages/outpost/ai/src/eval/harness.ts +++ b/packages/outpost/ai/src/eval/harness.ts @@ -12,7 +12,7 @@ * not call the pipeline itself, because the two ways of producing a reply have * opposite requirements and only one belongs in CI: * - * - **Fixed replies** (this file's `HISTORICAL_FAILURES`) — deterministic, no + * - **Fixed replies** (`__fixtures__/historical-failures.ts`) — deterministic, no * network, no model. Pins the rule set against known-bad output so a rule * cannot silently stop firing. Runs in CI. * - **Live replies** — real Pathfinder retrieval and a real model call, run @@ -117,124 +117,3 @@ export function formatReport(report: EvalReport): string { } return lines.join('\n'); } - -const CHAT_DOCS: SearchResult[] = [ - { - title: 'CopilotChat', - content: - 'CopilotChat renders a chat window. Use the `CopilotChat` component with the ' + - '`instructions` prop. Slots let you replace the input via the `input` prop.', - score: 0.9, - sourceUrl: 'https://docs.copilotkit.ai/reference/components/chat/CopilotChat', - }, -]; - -/** - * The four failures from the doc's appendix, as replies the rules must catch. - * - * **These are reconstructions, not transcripts.** The doc describes each reply's - * shape and quotes fragments of it; the full original text lives in the linked - * Discord threads and GitHub issue. Each reply below is assembled from what the - * doc states about it, and the `provenance` field links the original so anyone - * can check the reconstruction against the real thing. - * - * What that means for what these prove: they pin that the RULE SET catches each - * documented failure mode. They do not measure the current agent, because they - * are not its current output. Measuring the agent needs the live mode described - * at the top of this file, fed with the real threads. - */ -export const HISTORICAL_FAILURES: EvalCase[] = [ - { - id: 'case-a-deep-agents-subagents', - question: 'Does Deep Agents support subagents?', - // The doc: said it had "no specific timeline information", sent them to - // GitHub to ask, and offered a workaround as a "hypothesis". Subagents - // work today and one code search returns the proof. - reply: - "Great question! I don't have specific timeline information on subagent support " + - 'for Deep Agents. One possibility is that you could work around it by composing ' + - 'agents manually, or the equivalent pattern in your own runtime. ' + - 'I would suggest opening a GitHub discussion so the team can weigh in.', - sources: [], - provenance: - 'https://discord.com/channels/1122926057641742418/1535447155735789708(2026-08-08)', - }, - { - id: 'case-b-version-mixing', - question: 'How do I render the delegation in the chat?', - // The doc: mixed v1 and v2 hooks in one answer and hedged an API name. - reply: - 'You can hook the render path with `useCopilotFabricatedRender` or the equivalent ' + - 'render hook, and install `@copilotkitnext/react` to get the newer surface.', - sources: CHAT_DOCS, - provenance: - 'https://discord.com/channels/1122926057641742418/1313616713647919218/threads/1529599811744043018 (2026-07-22)', - }, - { - id: 'case-c-false-capability-claim', - question: '(maintainer follow-up in thread) Did that fix work for you?', - // The doc: replied to a maintainer, complimented his community spirit, - // claimed it could not see other people's replies, asked for a version. - reply: - 'Thanks for your detailed report and for supporting the community here! ' + - "I can't see other people's replies in this thread, so I don't have the full " + - 'context. Which version of CopilotKit are you using?', - sources: CHAT_DOCS, - provenance: - 'https://discord.com/channels/1122926057641742418/1313616713647919218/threads/1531971013791711342 (2026-08-11)', - }, - { - id: 'case-d-five-paragraphs-of-nothing', - question: '(dependency audit listing two concrete problems)', - // The doc: praise opener, restated both of the reporter's points, a - // "What I can't do from here" section, advice on writing better issues. - reply: - 'Great question, and thanks for this detailed report! To summarise what you have ' + - 'found: first, the manifest and the lockfile disagree about the version. Second, ' + - 'the peer dependency range looks too wide. ' + - "Here is what I can't do from here: I cannot read the source or run the install " + - 'to confirm either point. In the future, please include the full lockfile diff so ' + - 'this is easier to triage. The team will take it from here. ' + - // Padded deliberately so the reply clears the handoff cap, which is - // half of what case D is a fixture FOR. Previously `.repeat(2)` bound - // to the last literal only, so the reply ended with a stray duplicate - // sentence rather than the length the comment claimed. - 'Let me know if any of that needs clarifying and someone will pick it up. '.repeat(3), - sources: CHAT_DOCS, - provenance: 'https://github.com/CopilotKit/CopilotKit/issues/6423', - }, -]; - -/** - * The reply the doc holds up as the target shape, written by a maintainer in - * case A's own thread: verdict, proof, minimum code, one caveat. - * - * Present so the rule set is pinned in both directions. A rule set that only - * ever fires is as useless as one that never does, and this is the case that - * catches an over-eager rule before it starts collapsing good answers into - * handoffs. - */ -export const TARGET_SHAPE: EvalCase = { - id: 'case-a-maintainer-answer', - question: 'Does Deep Agents support subagents?', - reply: - 'Subagents work with Deep Agents today; the docs just do not cover them. Pass them ' + - 'straight to `create_deep_agent`. Deep Agents spawns subagents through its built-in ' + - '`task` tool, which runs as a nested subgraph, and our LangGraph adapter streams those ' + - 'by default, so the delegation shows up in the chat. Render it by hooking the `task` ' + - 'tool. One caveat: be on a recent Python adapter. ' + - 'https://github.com/CopilotKit/CopilotKit/blob/main/packages/runtime/src/langgraph/agent.ts', - sources: [ - { - title: 'langgraph/agent.ts', - content: - 'create_deep_agent spawns subagents through the built-in task tool, which runs ' + - 'as a nested subgraph. The LangGraph adapter streams subgraph events by default.', - score: 0.95, - sourceUrl: - 'https://github.com/CopilotKit/CopilotKit/blob/main/packages/runtime/src/langgraph/agent.ts', - }, - ], - provenance: - "Maintainer reply quoted in the Agent's Output Doc, from the case-a thread (2026-08-08)", -}; diff --git a/packages/outpost/ai/src/index.ts b/packages/outpost/ai/src/index.ts index 071cbcd8..ba71d40c 100644 --- a/packages/outpost/ai/src/index.ts +++ b/packages/outpost/ai/src/index.ts @@ -48,11 +48,18 @@ export type { ScoredTopIssue, } from './front-door.js'; // The rule set and the scorer are API — Phase 3's linter consumes them. -// HISTORICAL_FAILURES and TARGET_SHAPE deliberately are NOT: they are test data, -// and exporting them put reconstructed bad replies containing -// `useCopilotFabricatedRender` and `@copilotkitnext/react` into dist and the -// worker image, where they would surface in a bundle grep for the dead package. -// Import them from './eval/harness.js' directly in tests and offline runners. +// +// The eval fixtures are not, and are no longer reachable from here: they live in +// `eval/__fixtures__/`, which `ai/tsconfig.json` excludes from the build. Not +// re-exporting them was never sufficient on its own — `tsc` emits per file and +// this module imports `./eval/harness.js`, so while they lived in `harness.ts` +// the reconstructed bad replies shipped in `dist/eval/harness.js` regardless of +// what this entry point declared. +// +// Verified against a built `dist`: no fixture reply text remains. `@copilotkitnext` +// still appears in `dist/eval/rules.js`, and has to — that is the rule which bans +// it. A bundle grep for the dead package name will hit the rule, not a fabricated +// example of it. export { checkReply, RULES, HANDOFF_WORD_CAP, MIN_REPLY_WORDS } from './eval/rules.js'; export type { RuleId, RuleResult } from './eval/rules.js'; export { scoreCases, formatReport } from './eval/harness.js'; diff --git a/packages/outpost/ai/src/pathfinder.test.ts b/packages/outpost/ai/src/pathfinder.test.ts index 4731aecc..f4b77c99 100644 --- a/packages/outpost/ai/src/pathfinder.test.ts +++ b/packages/outpost/ai/src/pathfinder.test.ts @@ -300,6 +300,63 @@ describe('PathfinderClient', () => { }); }); + // Three layers independently prevent a docs block being read as code: the + // title prefers TITLE over PATH, `isCode` requires the absence of TITLE, and + // headers are read only from above CONTENT:. That redundancy is deliberate, + // and it means no single one of them is pinned by the docs-quoting-code test + // above — reverting any one alone leaves the suite green. These two isolate a + // layer each, so simplifying one away is visible. + describe('each structural layer, isolated', () => { + // Isolates the title order and `isCode`. A block carrying BOTH headers is + // the shape that appears if the server ever gives code hits a title — + // a contract we do not own. It must read as docs and keep its SOURCE. + it('treats a block with both TITLE and PATH as docs', async () => { + const both = [ + 'SNIPPET 1', + 'TITLE: Self-hosting the CopilotKit Runtime', + 'SOURCE: https://docs.copilotkit.ai/guides/self-hosting', + 'PATH: packages/core/src/core/run-handler.ts', + 'CONTENT:', + 'const handler = copilotRuntimeNextJSAppRouter({});', + ].join('\n'); + + mockConnect(); + mockFetch.mockResolvedValueOnce( + mkResp({ body: jsonRpc({ content: [{ type: 'text', text: both }] }) }), + ); + + const results = await client.searchDocs({ query: 'self hosting' }); + + expect(results[0].kind).toBe('docs'); + expect(results[0].title).toBe('Self-hosting the CopilotKit Runtime'); + expect(results[0].sourceUrl).toBe('https://docs.copilotkit.ai/guides/self-hosting'); + }); + + // Isolates the header region. This block has no real SOURCE header, and a + // line-initial `SOURCE:` inside its content. Matching headers over the + // whole block would adopt that line as the citation. + it('does not read a SOURCE header out of the content', async () => { + const sourceInBody = [ + 'SNIPPET 1', + 'TITLE: Configuring the runtime', + 'CONTENT:', + '```yaml', + 'SOURCE: https://evil.example.com/not-a-real-page', + '```', + ].join('\n'); + + mockConnect(); + mockFetch.mockResolvedValueOnce( + mkResp({ body: jsonRpc({ content: [{ type: 'text', text: sourceInBody }] }) }), + ); + + const results = await client.searchDocs({ query: 'configuring' }); + + expect(results[0].title).toBe('Configuring the runtime'); + expect(results[0].sourceUrl).toBeUndefined(); + }); + }); + describe('the AG-UI tools', () => { it('searchAgUiCode calls search-ag-ui-code', async () => { mockConnect(); diff --git a/packages/outpost/ai/src/pathfinder.ts b/packages/outpost/ai/src/pathfinder.ts index 29400051..f7d5cdba 100644 --- a/packages/outpost/ai/src/pathfinder.ts +++ b/packages/outpost/ai/src/pathfinder.ts @@ -335,6 +335,15 @@ export class PathfinderClient { // A code block is the one with a PATH and no TITLE. Derived from the // headers rather than from PATH alone, so a docs block can never be // mistaken for code and lose its citable URL. + // + // This depends on a server-side contract we do not own: that a code + // hit never carries a TITLE. It holds against the current + // `tools/list` on mcp.copilotkit.ai. If a code result ever gains one, + // `isCode` goes false, `source` falls back to `header('SOURCE')` + // — absent on a code block — and the file path silently stops being + // citable, which the reply rules then turn into a handoff. The + // both-headers case is pinned in pathfinder.test.ts so the change in + // behaviour is visible rather than silent. const isCode = !titleHeader && !!path; const title = titleHeader ?? path ?? 'Documentation'; diff --git a/packages/outpost/ai/tsconfig.json b/packages/outpost/ai/tsconfig.json index c484de88..9e8428d5 100644 --- a/packages/outpost/ai/tsconfig.json +++ b/packages/outpost/ai/tsconfig.json @@ -4,6 +4,14 @@ "outDir": "./dist", "rootDir": "./src" }, - "include": ["src/**/*.ts"], - "exclude": ["node_modules", "dist"] + "include": [ + "src/**/*.ts" + ], + "exclude": [ + "node_modules", + "dist", + "**/*.test.ts", + "**/__tests__/**", + "**/__fixtures__/**" + ] } From c261303f74f991b33a7056abd3fbfa5dad19e0dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:48:45 -0400 Subject: [PATCH 2/3] fix(ai): keep tests typechecked while keeping them out of dist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the blocker on #257. Jerel was right, and the regression was mine: the exclusions went into `ai/tsconfig.json`, which is also what `typecheck` reads (`tsc --project ai/tsconfig.json --noEmit`) — so excluding test material stopped it being typechecked at all. Reproduced his proof before changing anything. With `const __probe: number = 'definitely not a number'` in `eval/rules.test.ts`: - merged main: error TS2322: Type 'string' is not assignable to type 'number' - #257 as pushed: pnpm typecheck clean, 10/10 So the PR traded "tests ship in dist" for "test type errors reach main", which is the worse half — vitest exercises runtime behaviour, so a test asserting against a shape that no longer exists surfaces as a confusing failure or not at all. It landed directly after #242 added `SearchResult.kind`, which is exactly the kind of change whose test fallout wants a compiler. `ai/tsconfig.build.json` now carries the exclusions and `build:ai` points at it. The default config goes back to seeing everything, so anything inheriting it inherits the safer default. Both properties verified together: typecheck catches the injected error again, and a clean build emits 18 JS files with 0 test files and no `__fixtures__`. He asked for a test that fails when a test file stops being typechecked. There is one now — `__tests__/typecheck-config.test.ts` asserts that whatever `typecheck` reads does not exclude test material, that the build config does, and that `build:ai` invokes the build config rather than the inclusive one. Checked in both directions: re-adding the exclusion to `tsconfig.json` fails it, and pointing `build:ai` back at `tsconfig.json` fails it. Two notes on writing that guard, since both bugs were the same shape as the change itself: - The first version stripped tsconfig comments by hand, and its block-comment pattern matched inside the exclude values — a doubled-star glob followed by a slash-star extension reads as a comment opener. It collapsed three patterns into one mangled string and failed against config that was correct. It now uses `ts.parseConfigFileTextToJson`. - Explaining that in a docstring then broke the file, because writing those globs literally inside a block comment closes it early. They are described rather than quoted. Not addressed here, and worth its own change: `db`, `queue` and `shared` have no such split, so their tests are still compiled into their published output. Same packaging wart, three more packages, and the same fix applies to each. --- .../ai/src/__tests__/typecheck-config.test.ts | 80 +++++++++++++++++++ packages/outpost/ai/tsconfig.build.json | 23 ++++++ packages/outpost/ai/tsconfig.json | 5 +- packages/outpost/package.json | 2 +- 4 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 packages/outpost/ai/src/__tests__/typecheck-config.test.ts create mode 100644 packages/outpost/ai/tsconfig.build.json diff --git a/packages/outpost/ai/src/__tests__/typecheck-config.test.ts b/packages/outpost/ai/src/__tests__/typecheck-config.test.ts new file mode 100644 index 00000000..16fd15aa --- /dev/null +++ b/packages/outpost/ai/src/__tests__/typecheck-config.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import ts from 'typescript'; + +/** + * Guards the split between the typecheck config and the build config. + * + * The gap this exists for: excluding test material from `ai/tsconfig.json` + * silently stops it being typechecked, because `typecheck` runs + * `tsc --project ai/tsconfig.json --noEmit` against that same file. A test could + * then assert against a shape that no longer exists and nothing would say so — + * vitest exercises runtime behaviour, not types. It went unnoticed once already + * and the failure mode leaves no trace, so it is worth a test rather than a + * comment. + * + * Asserting on the config files is a blunt instrument, but the alternative — + * running `tsc` from a test — is slow and would need the probe error committed. + * This pins the property that matters: whatever `typecheck` reads must not + * exclude tests, and the build config must. + */ + +const packageRoot = join(import.meta.dirname, '..', '..'); + +/** + * tsconfig files carry comments, which `JSON.parse` rejects. + * + * Uses TypeScript's own parser rather than stripping comments by hand. A + * hand-rolled version got this wrong in a way worth recording: a block-comment + * pattern matches inside these exclude values, because a doubled-star glob + * followed by a slash-star extension reads as a comment opener, and the next + * glob ending in slash-doubled-star reads as its closer. It ate the span between + * them and collapsed three exclude patterns into one mangled string, so the test + * failed against config that was correct. + * + * The globs are deliberately described rather than quoted here — writing them + * literally inside a block comment closes it early, which broke this file once + * for the same reason. + */ +function readTsconfig(path: string): Record { + const { config, error } = ts.parseConfigFileTextToJson(path, readFileSync(path, 'utf8')); + if (error) throw new Error(`could not parse ${path}: ${JSON.stringify(error.messageText)}`); + return config as Record; +} + +const TEST_PATTERNS = ['**/*.test.ts', '**/__tests__/**', '**/__fixtures__/**']; + +describe('the config typecheck reads', () => { + it('does not exclude test material, or tests stop being typechecked', () => { + const exclude = (readTsconfig(join(packageRoot, 'tsconfig.json')).exclude ?? []) as string[]; + + for (const pattern of TEST_PATTERNS) { + expect(exclude).not.toContain(pattern); + } + }); +}); + +describe('the config the build reads', () => { + it('excludes test material, so none of it reaches dist', () => { + const cfg = readTsconfig(join(packageRoot, 'tsconfig.build.json')); + const exclude = (cfg.exclude ?? []) as string[]; + + // Inherits compilerOptions rather than restating them — the two configs + // must not be able to drift on anything but `exclude`. + expect(cfg.extends).toBe('./tsconfig.json'); + for (const pattern of TEST_PATTERNS) { + expect(exclude).toContain(pattern); + } + }); + + // A build script pointed at the inclusive config would put every test file + // back into the published package, which is what this whole change removed. + it('is what build:ai actually invokes', () => { + const pkg = readTsconfig(join(packageRoot, '..', 'package.json')); + const scripts = pkg.scripts as Record; + + expect(scripts['build:ai']).toContain('ai/tsconfig.build.json'); + expect(scripts.typecheck).toContain('ai/tsconfig.json --noEmit'); + }); +}); diff --git a/packages/outpost/ai/tsconfig.build.json b/packages/outpost/ai/tsconfig.build.json new file mode 100644 index 00000000..4b8d3e67 --- /dev/null +++ b/packages/outpost/ai/tsconfig.build.json @@ -0,0 +1,23 @@ +{ + // The BUILD config. Excludes test material from the emitted package; nothing + // else differs from ./tsconfig.json. + // + // This exists because the exclusions cannot live in tsconfig.json: that file is + // also what `typecheck` runs against (`tsc --project ai/tsconfig.json + // --noEmit`), so excluding tests there stops them being typechecked at all. + // Verified with an injected error — `const x: number = 'nope'` in a .test.ts + // went unreported. That trades a packaging wart for type errors reaching main, + // which is the worse half: vitest exercises runtime behaviour, so a test + // asserting against a shape that no longer exists would only surface as a + // confusing failure, or not at all. + // + // The default config stays inclusive so anything inheriting it sees everything. + "extends": "./tsconfig.json", + "exclude": [ + "node_modules", + "dist", + "**/*.test.ts", + "**/__tests__/**", + "**/__fixtures__/**" + ] +} diff --git a/packages/outpost/ai/tsconfig.json b/packages/outpost/ai/tsconfig.json index 9e8428d5..c0a8e46f 100644 --- a/packages/outpost/ai/tsconfig.json +++ b/packages/outpost/ai/tsconfig.json @@ -9,9 +9,6 @@ ], "exclude": [ "node_modules", - "dist", - "**/*.test.ts", - "**/__tests__/**", - "**/__fixtures__/**" + "dist" ] } diff --git a/packages/outpost/package.json b/packages/outpost/package.json index 513bdf49..f9371dd1 100644 --- a/packages/outpost/package.json +++ b/packages/outpost/package.json @@ -32,7 +32,7 @@ "scripts": { "build": "pnpm run build:shared && pnpm run build:db && pnpm run build:ai && pnpm run build:queue", "build:db": "prisma generate --schema=db/prisma/schema.prisma && tsc --project db/tsconfig.json", - "build:ai": "tsc --project ai/tsconfig.json", + "build:ai": "tsc --project ai/tsconfig.build.json", "build:queue": "tsc --project queue/tsconfig.json", "build:shared": "tsc --project shared/tsconfig.json", "typecheck": "tsc --project db/tsconfig.json --noEmit && tsc --project ai/tsconfig.json --noEmit && tsc --project queue/tsconfig.json --noEmit && tsc --project shared/tsconfig.json --noEmit", From efa3e2b65ab84bda7af8afce8890d5aeeea7a07b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:54:50 -0400 Subject: [PATCH 3/3] style(ai): format the files this PR touches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The format check added in #250 caught these on its first run against a real PR — five of this PR's own files were unformatted. Prettier preserves the comments in tsconfig.build.json, and the build config still parses: a clean build emits 18 JS files with 0 test files. Worth noting the gate did exactly what it was added for, on the first opportunity, against the person who added it. --- .../outpost/ai/src/__tests__/typecheck-config.test.ts | 3 ++- packages/outpost/ai/src/pathfinder.test.ts | 11 +++++++---- packages/outpost/ai/src/pathfinder.ts | 5 ++++- packages/outpost/ai/tsconfig.build.json | 8 +------- packages/outpost/ai/tsconfig.json | 9 ++------- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/outpost/ai/src/__tests__/typecheck-config.test.ts b/packages/outpost/ai/src/__tests__/typecheck-config.test.ts index 16fd15aa..97a0563d 100644 --- a/packages/outpost/ai/src/__tests__/typecheck-config.test.ts +++ b/packages/outpost/ai/src/__tests__/typecheck-config.test.ts @@ -47,7 +47,8 @@ const TEST_PATTERNS = ['**/*.test.ts', '**/__tests__/**', '**/__fixtures__/**']; describe('the config typecheck reads', () => { it('does not exclude test material, or tests stop being typechecked', () => { - const exclude = (readTsconfig(join(packageRoot, 'tsconfig.json')).exclude ?? []) as string[]; + const exclude = (readTsconfig(join(packageRoot, 'tsconfig.json')).exclude ?? + []) as string[]; for (const pattern of TEST_PATTERNS) { expect(exclude).not.toContain(pattern); diff --git a/packages/outpost/ai/src/pathfinder.test.ts b/packages/outpost/ai/src/pathfinder.test.ts index f4b77c99..9a3e175e 100644 --- a/packages/outpost/ai/src/pathfinder.test.ts +++ b/packages/outpost/ai/src/pathfinder.test.ts @@ -395,8 +395,7 @@ describe('PathfinderClient', () => { content: [ { type: 'text', - text: - 'SNIPPET 1\nTITLE: CopilotKit Actions\nSOURCE: https://docs.copilotkit.ai/actions\nCONTENT:\nuseCopilotAction lets you define actions.\n\n---\n\nSNIPPET 2\nTITLE: Getting Started\nSOURCE: https://docs.copilotkit.ai/quickstart\nCONTENT:\nInstall CopilotKit with npm install.', + text: 'SNIPPET 1\nTITLE: CopilotKit Actions\nSOURCE: https://docs.copilotkit.ai/actions\nCONTENT:\nuseCopilotAction lets you define actions.\n\n---\n\nSNIPPET 2\nTITLE: Getting Started\nSOURCE: https://docs.copilotkit.ai/quickstart\nCONTENT:\nInstall CopilotKit with npm install.', }, ], }, @@ -509,8 +508,12 @@ describe('PathfinderClient', () => { it('returns empty when both MCP and fallback fail', async () => { mockConnect(); - mockFetch.mockResolvedValueOnce(mkResp({ ok: false, status: 500, statusText: 'Error' })); - mockFetch.mockResolvedValueOnce(mkResp({ ok: false, status: 500, statusText: 'Error' })); + mockFetch.mockResolvedValueOnce( + mkResp({ ok: false, status: 500, statusText: 'Error' }), + ); + mockFetch.mockResolvedValueOnce( + mkResp({ ok: false, status: 500, statusText: 'Error' }), + ); const results = await client.searchDocs({ query: 'anything' }); expect(results).toEqual([]); diff --git a/packages/outpost/ai/src/pathfinder.ts b/packages/outpost/ai/src/pathfinder.ts index f7d5cdba..3a01ec96 100644 --- a/packages/outpost/ai/src/pathfinder.ts +++ b/packages/outpost/ai/src/pathfinder.ts @@ -523,7 +523,10 @@ export class PathfinderClient { const score = matchCount / queryTerms.length; // Extract title from first line - const firstLine = section.split('\n')[0].replace(/^#+\s*/, '').trim(); + const firstLine = section + .split('\n')[0] + .replace(/^#+\s*/, '') + .trim(); return { title: firstLine || 'Documentation', diff --git a/packages/outpost/ai/tsconfig.build.json b/packages/outpost/ai/tsconfig.build.json index 4b8d3e67..3b83337c 100644 --- a/packages/outpost/ai/tsconfig.build.json +++ b/packages/outpost/ai/tsconfig.build.json @@ -13,11 +13,5 @@ // // The default config stays inclusive so anything inheriting it sees everything. "extends": "./tsconfig.json", - "exclude": [ - "node_modules", - "dist", - "**/*.test.ts", - "**/__tests__/**", - "**/__fixtures__/**" - ] + "exclude": ["node_modules", "dist", "**/*.test.ts", "**/__tests__/**", "**/__fixtures__/**"] } diff --git a/packages/outpost/ai/tsconfig.json b/packages/outpost/ai/tsconfig.json index c0a8e46f..c484de88 100644 --- a/packages/outpost/ai/tsconfig.json +++ b/packages/outpost/ai/tsconfig.json @@ -4,11 +4,6 @@ "outDir": "./dist", "rootDir": "./src" }, - "include": [ - "src/**/*.ts" - ], - "exclude": [ - "node_modules", - "dist" - ] + "include": ["src/**/*.ts"], + "exclude": ["node_modules", "dist"] }