From 43b243d38e8e6cf7d901f483139a4b354b4a27f4 Mon Sep 17 00:00:00 2001 From: Matthew Rayermann Date: Thu, 30 Jul 2026 16:44:39 -0700 Subject: [PATCH 1/6] [Codegen] Keep Internal Types That Public Declarations Reference --- nodejs/src/generated/rpc.ts | 12 ---- nodejs/src/generated/session-events.ts | 5 -- scripts/codegen/typescript.ts | 82 +++++++++++++++++++++----- 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/nodejs/src/generated/rpc.ts b/nodejs/src/generated/rpc.ts index db9039238..b45497e75 100644 --- a/nodejs/src/generated/rpc.ts +++ b/nodejs/src/generated/rpc.ts @@ -813,7 +813,6 @@ export type HistoryRewindOutcome = * via the `definition` "HookType". */ /** @experimental */ -/** @internal */ export type HookType = /** Runs before a tool is invoked. */ | "preToolUse" @@ -4468,7 +4467,6 @@ export interface SessionCompletionItem { * via the `definition` "ConfigureSessionExtensionsParams". */ /** @experimental */ -/** @internal */ export interface ConfigureSessionExtensionsParams { /** * Session to attach the extension controller delegate to. @@ -4573,7 +4571,6 @@ export interface ConnectRemoteSessionParams { * via the `definition` "ConnectRequest". */ /** @experimental */ -/** @internal */ export interface ConnectRequest { /** * Connection token; required when the server was started with COPILOT_CONNECTION_TOKEN @@ -4591,7 +4588,6 @@ export interface ConnectRequest { * via the `definition` "ConnectResult". */ /** @experimental */ -/** @internal */ export interface ConnectResult { /** * Always true on success @@ -6539,7 +6535,6 @@ export interface HistoryTruncateResult { * via the `definition` "HookInvokeRequest". */ /** @experimental */ -/** @internal */ export interface HookInvokeRequest { sessionId: string; hookType: HookType; @@ -6552,7 +6547,6 @@ export interface HookInvokeRequest { * via the `definition` "HookInvokeResponse". */ /** @experimental */ -/** @internal */ export interface HookInvokeResponse { output?: unknown; } @@ -7808,7 +7802,6 @@ export interface McpConfigUpdateRequest { * via the `definition` "McpConfigureGitHubRequest". */ /** @experimental */ -/** @internal */ export interface McpConfigureGitHubRequest { /** * Opaque runtime auth info for GitHub MCP configuration. Marked internal: an in-process runtime shape (configureGitHubMcp is a no-op over the wire). @@ -8250,7 +8243,6 @@ export interface McpOauthRespondResult { * via the `definition` "McpRegisterExternalClientRequest". */ /** @experimental */ -/** @internal */ export interface McpRegisterExternalClientRequest { /** * Logical server name for the external client @@ -8288,7 +8280,6 @@ export interface McpRegisterExternalClientRequest { * via the `definition` "McpReloadWithConfigRequest". */ /** @experimental */ -/** @internal */ export interface McpReloadWithConfigRequest { /** * Opaque runtime MCP reload configuration. Marked internal: an in-process runtime shape (reloadMcpServers throws over the wire). @@ -8736,7 +8727,6 @@ export interface McpStopServerRequest { * via the `definition` "McpUnregisterExternalClientRequest". */ /** @experimental */ -/** @internal */ export interface McpUnregisterExternalClientRequest { /** * Server name of the external client to unregister @@ -12387,7 +12377,6 @@ export interface RegisterEventInterestResult { * via the `definition` "RegisterExtensionToolsParams". */ /** @experimental */ -/** @internal */ export interface RegisterExtensionToolsParams { /** * Session to register extension tools on. @@ -12429,7 +12418,6 @@ export interface SessionsRegisterExtensionToolsOnSessionOptions { * via the `definition` "RegisterExtensionToolsResult". */ /** @experimental */ -/** @internal */ export interface RegisterExtensionToolsResult { /** * In-process unsubscribe function (CLI-only optimization). Marked internal: replaced by an explicit `extensions.unregister` RPC in the SDK migration. diff --git a/nodejs/src/generated/session-events.ts b/nodejs/src/generated/session-events.ts index 193dc0def..165c9d539 100644 --- a/nodejs/src/generated/session-events.ts +++ b/nodejs/src/generated/session-events.ts @@ -2269,7 +2269,6 @@ export interface UsageCheckpointData { /** * Internal prompt-cache expiration state for one model */ -/** @internal */ export interface UsageCheckpointModelCacheState { /** * Latest known prompt-cache expiration @@ -2586,7 +2585,6 @@ export interface CompactionCompleteCompactionTokensUsed { /** * Per-request cost and usage data from the CAPI copilot_usage response field */ -/** @internal */ export interface CompactionCompleteCompactionTokensUsedCopilotUsage { /** * Itemized token usage breakdown @@ -3269,7 +3267,6 @@ export interface AssistantTurnStartData { /** * Session event "assistant.turn_retry". Metadata for an additional model inference attempt within an existing assistant turn */ -/** @internal */ export interface AssistantTurnRetryEvent { /** * Sub-agent instance identifier. Absent for events from the root/main agent and session-level events. @@ -4206,7 +4203,6 @@ export interface AssistantUsageCopilotUsageTokenDetail { /** * Internal per-quota snapshot for assistant usage, including entitlement, consumed requests, overage, reset date, and remaining quota. */ -/** @internal */ export interface AssistantUsageQuotaSnapshot { /** * Total requests allowed by the entitlement @@ -4421,7 +4417,6 @@ export interface ModelCallFailureRequestFingerprint { /** * Session event "model.call_start". Model API dispatch metadata for internal telemetry */ -/** @internal */ export interface ModelCallStartEvent { /** * Sub-agent instance identifier. Absent for events from the root/main agent and session-level events. diff --git a/scripts/codegen/typescript.ts b/scripts/codegen/typescript.ts index f82e67abe..1d0c0ad1c 100644 --- a/scripts/codegen/typescript.ts +++ b/scripts/codegen/typescript.ts @@ -57,6 +57,69 @@ function tsExperimentalJSDoc(indent = ""): string { return `${indent}${TS_EXPERIMENTAL_JSDOC}`; } +/** + * Tag each strippable candidate in `candidates` with `@internal`, skipping any that public + * declarations still reference. Runs over fully assembled file content so that references from + * generated method signatures — not just from other type declarations — are taken into account. + */ +function tagInternalTypes(generatedTs: string, candidates: Set): string { + let tagged = generatedTs; + for (const intType of strippableInternalTypes(tagged, candidates)) { + tagged = tagged.replace( + new RegExp(`(^|\\n)(export (?:interface|type) ${intType}\\b)`, "m"), + `$1/** @internal */\n$2` + ); + } + return tagged; +} + +/** + * Restrict a set of candidate `@internal` type names to those that are safe to strip. + * + * `@internal` drives `stripInternal`, which deletes the whole declaration from the emitted + * `.d.ts`. That is only sound when nothing public still refers to the type: a surviving public + * declaration naming a deleted type leaves a dangling reference, which makes the *referring* type + * an error type that TypeScript then degrades to `any`. One stripped-but-referenced event type is + * therefore enough to erase the type safety of the whole `SessionEvent` union for every consumer, + * and `skipLibCheck: true` (a common consumer setting) hides the underlying error. + * + * So a candidate is dropped when any declaration that is not itself internal mentions it. + */ +function strippableInternalTypes(generatedTs: string, candidates: Set): Set { + if (candidates.size === 0) return candidates; + + // Split into top-level declaration blocks so each reference can be attributed to its owner. + const declaration = /^export (?:interface|type) (\w+)\b/gm; + const starts: Array<{ index: number; name: string }> = []; + for (let m = declaration.exec(generatedTs); m !== null; m = declaration.exec(generatedTs)) { + starts.push({ index: m.index, name: m[1] }); + } + if (starts.length === 0) return candidates; + const blocks = starts.map((start, i) => ({ + name: start.name, + text: generatedTs.slice(start.index, i + 1 < starts.length ? starts[i + 1].index : generatedTs.length), + })); + + const safe = new Set(); + for (const candidate of candidates) { + const referencedByPublic = blocks.some( + (block) => + block.name !== candidate && + !candidates.has(block.name) && + new RegExp(`\\b${candidate}\\b`).test(block.text) + ); + if (referencedByPublic) { + console.warn( + ` ! ${candidate} is marked internal but is referenced by public declarations; ` + + `keeping it in the emitted .d.ts to avoid a dangling type reference` + ); + continue; + } + safe.add(candidate); + } + return safe; +} + function sanitizeJsDocText(text: string): string { return text.trim().replace(/\*\//g, "* /"); } @@ -409,12 +472,7 @@ async function generateSessionEvents(schemaPath?: string): Promise { sessionInternalTypes.add(name); } } - for (const intType of sessionInternalTypes) { - annotatedTs = annotatedTs.replace( - new RegExp(`(^|\\n)(export (?:interface|type) ${intType}\\b)`, "m"), - `$1/** @internal */\n$2` - ); - } + annotatedTs = tagInternalTypes(annotatedTs, sessionInternalTypes); const outPath = await writeGeneratedFile("nodejs/src/generated/session-events.ts", annotatedTs); console.log(` ✓ ${outPath}`); } @@ -675,13 +733,9 @@ import type { MessageConnection } from "vscode-jsonrpc/node.js"; `$1/** @deprecated */\n$2` ); } - // Add @internal JSDoc annotations for types from internal methods - for (const intType of internalTypes) { - annotatedTs = annotatedTs.replace( - new RegExp(`(^|\\n)(export (?:interface|type) ${intType}\\b)`, "m"), - `$1/** @internal */\n$2` - ); - } + // @internal tagging happens in a final pass over the assembled file: the client/server + // method signatures that reference these types are emitted later, so a per-chunk check + // would not see them and would strip a type the public API still names. lines.push(annotatedTs); lines.push(""); } @@ -758,7 +812,7 @@ function hasInternalMethods(node: Record): boolean { lines.push(...emitClientGlobalApiRegistration(schema.clientGlobal)); } - const outPath = await writeGeneratedFile("nodejs/src/generated/rpc.ts", lines.join("\n")); + const outPath = await writeGeneratedFile("nodejs/src/generated/rpc.ts", tagInternalTypes(lines.join("\n"), internalTypes)); console.log(` ✓ ${outPath}`); } From e46ff4115e655dfa9345f2e7a83eda9a6e3181bd Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 31 Jul 2026 15:07:30 +0000 Subject: [PATCH 2/6] Fix @internal stripping: filter union arms correctly, fail hard on violations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The root cause of dangling @internal types in the emitted .d.ts was in the publicVariants filter inside generateSessionEvents. The filter checked only the 'data' sub-property of each resolved variant for internal visibility, but missed the case where the arm object itself or its resolved definition is marked visibility:internal. Internal event types (AssistantTurnRetryEvent, ModelCallStartEvent) therefore passed through the filter and appeared in the generated SessionEvent union — while their declarations were separately tagged @internal and stripped — leaving dangling references that collapsed to 'any'. Fix the filter to also check isSchemaInternal on the arm object and on the resolved definition. With this, internal union arms are excluded from compilation entirely: no declaration, no union member, no dangling reference. For RPC, fix emitClientGlobalApiRegistration to filter internal methods from the generated handler interface (HooksHandler), so internal method param types (HookInvokeRequest, HookType, etc.) are no longer referenced by any public declaration. The registration function body still wires up the internal RPC handler internally, but function bodies are not emitted in declaration files. Replace the PR's strippableInternalTypes workaround (which silently promoted internal types to public rather than failing) with assertNoPublicInternalReferences, which throws hard if the codegen ever produces a public declaration referencing an internal type. The schema lint in the runtime already guarantees the schema is valid; a violation here means the codegen has a bug, not something to paper over. Export filterPublicSessionEventVariants and assertNoPublicInternalReferences and add unit tests covering: public arms kept, internal arms excluded (by arm visibility, by definition visibility, and by data-property visibility), the validator passing on @internal-tagged members and function bodies, and the validator throwing on direct public references to internal types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/generated/rpc.ts | 20 +-- nodejs/src/generated/session-events.ts | 101 +------------ nodejs/test/typescript-codegen.test.ts | 192 ++++++++++++++++++++++++- scripts/codegen/typescript.ts | 184 ++++++++++++++++-------- 4 files changed, 328 insertions(+), 169 deletions(-) diff --git a/nodejs/src/generated/rpc.ts b/nodejs/src/generated/rpc.ts index b45497e75..2ceb6f0b0 100644 --- a/nodejs/src/generated/rpc.ts +++ b/nodejs/src/generated/rpc.ts @@ -813,6 +813,7 @@ export type HistoryRewindOutcome = * via the `definition` "HookType". */ /** @experimental */ +/** @internal */ export type HookType = /** Runs before a tool is invoked. */ | "preToolUse" @@ -4467,6 +4468,7 @@ export interface SessionCompletionItem { * via the `definition` "ConfigureSessionExtensionsParams". */ /** @experimental */ +/** @internal */ export interface ConfigureSessionExtensionsParams { /** * Session to attach the extension controller delegate to. @@ -4571,6 +4573,7 @@ export interface ConnectRemoteSessionParams { * via the `definition` "ConnectRequest". */ /** @experimental */ +/** @internal */ export interface ConnectRequest { /** * Connection token; required when the server was started with COPILOT_CONNECTION_TOKEN @@ -4588,6 +4591,7 @@ export interface ConnectRequest { * via the `definition` "ConnectResult". */ /** @experimental */ +/** @internal */ export interface ConnectResult { /** * Always true on success @@ -6535,6 +6539,7 @@ export interface HistoryTruncateResult { * via the `definition` "HookInvokeRequest". */ /** @experimental */ +/** @internal */ export interface HookInvokeRequest { sessionId: string; hookType: HookType; @@ -6547,6 +6552,7 @@ export interface HookInvokeRequest { * via the `definition` "HookInvokeResponse". */ /** @experimental */ +/** @internal */ export interface HookInvokeResponse { output?: unknown; } @@ -7802,6 +7808,7 @@ export interface McpConfigUpdateRequest { * via the `definition` "McpConfigureGitHubRequest". */ /** @experimental */ +/** @internal */ export interface McpConfigureGitHubRequest { /** * Opaque runtime auth info for GitHub MCP configuration. Marked internal: an in-process runtime shape (configureGitHubMcp is a no-op over the wire). @@ -8243,6 +8250,7 @@ export interface McpOauthRespondResult { * via the `definition` "McpRegisterExternalClientRequest". */ /** @experimental */ +/** @internal */ export interface McpRegisterExternalClientRequest { /** * Logical server name for the external client @@ -8280,6 +8288,7 @@ export interface McpRegisterExternalClientRequest { * via the `definition` "McpReloadWithConfigRequest". */ /** @experimental */ +/** @internal */ export interface McpReloadWithConfigRequest { /** * Opaque runtime MCP reload configuration. Marked internal: an in-process runtime shape (reloadMcpServers throws over the wire). @@ -8727,6 +8736,7 @@ export interface McpStopServerRequest { * via the `definition` "McpUnregisterExternalClientRequest". */ /** @experimental */ +/** @internal */ export interface McpUnregisterExternalClientRequest { /** * Server name of the external client to unregister @@ -12377,6 +12387,7 @@ export interface RegisterEventInterestResult { * via the `definition` "RegisterExtensionToolsParams". */ /** @experimental */ +/** @internal */ export interface RegisterExtensionToolsParams { /** * Session to register extension tools on. @@ -12418,6 +12429,7 @@ export interface SessionsRegisterExtensionToolsOnSessionOptions { * via the `definition` "RegisterExtensionToolsResult". */ /** @experimental */ +/** @internal */ export interface RegisterExtensionToolsResult { /** * In-process unsubscribe function (CLI-only optimization). Marked internal: replaced by an explicit `extensions.unregister` RPC in the SDK migration. @@ -21342,14 +21354,6 @@ export function registerClientSessionApiHandlers( /** Handler for `hooks` client global API methods. */ /** @experimental */ export interface HooksHandler { - /** - * Dispatches one SDK callback hook from the runtime to the connection that registered it. Internal transport plumbing: clients opt in through session initialization and the Rust hook processor owns ordering, policy, timeout, and callback routing. - * - * @param params Runtime-owned wire payload for a server-to-client hook callback invocation. - * - * @returns Optional output returned by an SDK callback hook. - */ - invoke(params: HookInvokeRequest): Promise; } /** Handler for `llmInference` client global API methods. */ diff --git a/nodejs/src/generated/session-events.ts b/nodejs/src/generated/session-events.ts index 165c9d539..e619488b2 100644 --- a/nodejs/src/generated/session-events.ts +++ b/nodejs/src/generated/session-events.ts @@ -39,7 +39,6 @@ export type SessionEvent = | UserMessageEvent | PendingMessagesModifiedEvent | AssistantTurnStartEvent - | AssistantTurnRetryEvent | AssistantIntentEvent | AssistantServerToolProgressEvent | AssistantReasoningEvent @@ -53,7 +52,6 @@ export type SessionEvent = | AssistantIdleEvent | AssistantUsageEvent | ModelCallFailureEvent - | ModelCallStartEvent | AbortEvent | ToolUserRequestedEvent | ToolExecutionStartEvent @@ -2269,6 +2267,7 @@ export interface UsageCheckpointData { /** * Internal prompt-cache expiration state for one model */ +/** @internal */ export interface UsageCheckpointModelCacheState { /** * Latest known prompt-cache expiration @@ -2585,6 +2584,7 @@ export interface CompactionCompleteCompactionTokensUsed { /** * Per-request cost and usage data from the CAPI copilot_usage response field */ +/** @internal */ export interface CompactionCompleteCompactionTokensUsedCopilotUsage { /** * Itemized token usage breakdown @@ -3264,53 +3264,6 @@ export interface AssistantTurnStartData { */ turnId: string; } -/** - * Session event "assistant.turn_retry". Metadata for an additional model inference attempt within an existing assistant turn - */ -export interface AssistantTurnRetryEvent { - /** - * Sub-agent instance identifier. Absent for events from the root/main agent and session-level events. - */ - agentId?: string; - data: AssistantTurnRetryData; - /** - * Always true for events that are transient and not persisted to the session event log on disk. - */ - ephemeral: true; - /** - * Unique event identifier (UUID v4), generated when the event is emitted - */ - id: string; - /** - * ID of the chronologically preceding event in the session, forming a linked chain. Null for the first event. - */ - parentId: string | null; - /** - * ISO 8601 timestamp when the event was created - */ - timestamp: string; - /** - * Type discriminator. Always "assistant.turn_retry". - */ - type: "assistant.turn_retry"; -} -/** - * Metadata for an additional model inference attempt within an existing assistant turn - */ -export interface AssistantTurnRetryData { - /** - * Model identifier used for this retry, when known - */ - model?: string; - /** - * Provider or runtime classification that caused the retry, when known - */ - reason?: string; - /** - * Identifier of the turn whose model inference is being retried - */ - turnId: string; -} /** * Session event "assistant.intent". Agent intent description for current activity or plan */ @@ -4203,6 +4156,7 @@ export interface AssistantUsageCopilotUsageTokenDetail { /** * Internal per-quota snapshot for assistant usage, including entitlement, consumed requests, overage, reset date, and remaining quota. */ +/** @internal */ export interface AssistantUsageQuotaSnapshot { /** * Total requests allowed by the entitlement @@ -4414,55 +4368,6 @@ export interface ModelCallFailureRequestFingerprint { */ toolResultMessageCount: number; } -/** - * Session event "model.call_start". Model API dispatch metadata for internal telemetry - */ -export interface ModelCallStartEvent { - /** - * Sub-agent instance identifier. Absent for events from the root/main agent and session-level events. - */ - agentId?: string; - data: ModelCallStartData; - /** - * Always true for events that are transient and not persisted to the session event log on disk. - */ - ephemeral: true; - /** - * Unique event identifier (UUID v4), generated when the event is emitted - */ - id: string; - /** - * ID of the chronologically preceding event in the session, forming a linked chain. Null for the first event. - */ - parentId: string | null; - /** - * ISO 8601 timestamp when the event was created - */ - timestamp: string; - /** - * Type discriminator. Always "model.call_start". - */ - type: "model.call_start"; -} -/** - * Model API dispatch metadata for internal telemetry - */ -export interface ModelCallStartData { - /** - * Model identifier used for this API call, when known - */ - model?: string; - /** - * Previous response or interaction identifier included in the model request, when present - * - * @internal - */ - previousResponseId?: string; - /** - * Identifier of the assistant turn that initiated the model call - */ - turnId: string; -} /** * Session event "abort". Turn abort information including the reason for termination */ diff --git a/nodejs/test/typescript-codegen.test.ts b/nodejs/test/typescript-codegen.test.ts index 248b60968..ad90e760d 100644 --- a/nodejs/test/typescript-codegen.test.ts +++ b/nodejs/test/typescript-codegen.test.ts @@ -2,7 +2,12 @@ import type { JSONSchema7 } from "json-schema"; import { compile } from "json-schema-to-typescript"; import { describe, expect, it } from "vitest"; -import { normalizeSchemaForTypeScript } from "../../scripts/codegen/typescript.ts"; +import { + assertNoPublicInternalReferences, + filterPublicSessionEventVariants, + normalizeSchemaForTypeScript, +} from "../../scripts/codegen/typescript.ts"; +import type { DefinitionCollections } from "../../scripts/codegen/utils.ts"; describe("typescript schema codegen", () => { it("emits JSDoc comments for described enum values", async () => { @@ -44,3 +49,188 @@ describe("typescript schema codegen", () => { expect(code).toContain('inlineMode: /** Use a direct value. */ "direct" | "indirect";'); }); }); + +describe("filterPublicSessionEventVariants", () => { + const makeCollections = (defs: Record): DefinitionCollections => ({ + definitions: defs, + $defs: {}, + }); + + it("keeps public union arms", () => { + const defs = { + PublicEvent: { type: "object" as const, properties: { type: { const: "pub" } } }, + }; + const variants: JSONSchema7[] = [{ $ref: "#/definitions/PublicEvent" }]; + const { publicVariants, excludedDefinitionNames } = filterPublicSessionEventVariants( + variants, + makeCollections(defs) + ); + expect(publicVariants).toHaveLength(1); + expect(excludedDefinitionNames.size).toBe(0); + }); + + it("excludes arms whose arm object is marked visibility:internal", () => { + const defs = { + InternalEvent: { + type: "object" as const, + visibility: "internal", + properties: { type: { const: "internal.evt" } }, + } as JSONSchema7 & { visibility: string }, + }; + const variants: JSONSchema7[] = [ + { $ref: "#/definitions/InternalEvent", visibility: "internal" } as JSONSchema7 & { + visibility: string; + }, + ]; + const { publicVariants, excludedDefinitionNames } = filterPublicSessionEventVariants( + variants, + makeCollections(defs) + ); + expect(publicVariants).toHaveLength(0); + expect(excludedDefinitionNames.has("InternalEvent")).toBe(true); + }); + + it("excludes arms whose resolved definition is marked visibility:internal", () => { + const defs = { + InternalEvent: { + type: "object" as const, + visibility: "internal", + properties: { type: { const: "internal.evt" } }, + } as JSONSchema7 & { visibility: string }, + }; + // arm object itself is NOT marked, but the resolved definition is + const variants: JSONSchema7[] = [{ $ref: "#/definitions/InternalEvent" }]; + const { publicVariants, excludedDefinitionNames } = filterPublicSessionEventVariants( + variants, + makeCollections(defs) + ); + expect(publicVariants).toHaveLength(0); + expect(excludedDefinitionNames.has("InternalEvent")).toBe(true); + }); + + it("keeps arms whose internal data sub-property is the only internal marker (legacy pattern)", () => { + // Event types that carry a `data: InternalData` field — the `data` property is what is + // internal, not the event wrapper type itself. + const defs = { + InternalData: { + type: "object" as const, + visibility: "internal", + } as JSONSchema7 & { visibility: string }, + WrapperEvent: { + type: "object" as const, + properties: { + type: { const: "wrapper.evt" }, + data: { $ref: "#/definitions/InternalData" }, + }, + }, + }; + const variants: JSONSchema7[] = [{ $ref: "#/definitions/WrapperEvent" }]; + const { publicVariants, excludedDefinitionNames } = filterPublicSessionEventVariants( + variants, + makeCollections(defs) + ); + expect(publicVariants).toHaveLength(0); + expect(excludedDefinitionNames.has("WrapperEvent")).toBe(true); + expect(excludedDefinitionNames.has("InternalData")).toBe(true); + }); +}); + +describe("assertNoPublicInternalReferences", () => { + it("passes when all declarations are public and do not reference internal types", () => { + const ts = ` +export interface Foo { + bar: string; +} +export type Bar = "a" | "b"; +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); + }); + + it("passes when the only reference is from an @internal-tagged declaration", () => { + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +/** @internal */ +export interface AlsoInternal { + h: Hidden; +} +export interface Public { + y: string; +} +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); + }); + + it("passes when the reference is inside an @internal-tagged member of a public type", () => { + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +export interface Public { + /** + * Some field. + * @internal + */ + secret?: Hidden; + visible: string; +} +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); + }); + + it("throws when a public declaration references an internal type directly", () => { + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +export type Event = PublicEvent | Hidden; +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).toThrow( + /Event \(public\) references internal type Hidden/ + ); + }); + + it("does not count JSDoc comment text as a code reference", () => { + // The auto-generated JSDoc says 'via the definition "Hidden"' but that is not a + // real TypeScript type reference — it must not trigger the validator. + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +export interface Preceding { + y: string; +} +/** + * This interface was referenced by something. + * via the definition "Hidden". + */ +export interface Following { + z: string; +} +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); + }); + + it("does not count function body references as public type references", () => { + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +/** @internal */ +export function doInternal(connection: unknown): void { + connection.onRequest("x", async (params: Hidden) => { return params; }); +} +export function doPublic(connection: unknown): void { + connection.onRequest("x", async (params: Hidden) => { return params; }); +} +`; + // function body references are stripped — only signature matters + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); + }); +}); diff --git a/scripts/codegen/typescript.ts b/scripts/codegen/typescript.ts index 1d0c0ad1c..8ba65b578 100644 --- a/scripts/codegen/typescript.ts +++ b/scripts/codegen/typescript.ts @@ -58,66 +58,69 @@ function tsExperimentalJSDoc(indent = ""): string { } /** - * Tag each strippable candidate in `candidates` with `@internal`, skipping any that public - * declarations still reference. Runs over fully assembled file content so that references from - * generated method signatures — not just from other type declarations — are taken into account. - */ -function tagInternalTypes(generatedTs: string, candidates: Set): string { - let tagged = generatedTs; - for (const intType of strippableInternalTypes(tagged, candidates)) { - tagged = tagged.replace( - new RegExp(`(^|\\n)(export (?:interface|type) ${intType}\\b)`, "m"), - `$1/** @internal */\n$2` - ); - } - return tagged; -} - -/** - * Restrict a set of candidate `@internal` type names to those that are safe to strip. + * Validates that no public declaration in the generated TypeScript references an internal type. * - * `@internal` drives `stripInternal`, which deletes the whole declaration from the emitted - * `.d.ts`. That is only sound when nothing public still refers to the type: a surviving public - * declaration naming a deleted type leaves a dangling reference, which makes the *referring* type - * an error type that TypeScript then degrades to `any`. One stripped-but-referenced event type is - * therefore enough to erase the type safety of the whole `SessionEvent` union for every consumer, - * and `skipLibCheck: true` (a common consumer setting) hides the underlying error. - * - * So a candidate is dropped when any declaration that is not itself internal mentions it. + * If the schema is valid (enforced by the runtime's `assert_no_public_internal_references` lint), + * this should never trigger. A failure here means the codegen itself produced a public reference + * to an internal type — which is a codegen bug that must be fixed, not silently worked around. */ -function strippableInternalTypes(generatedTs: string, candidates: Set): Set { - if (candidates.size === 0) return candidates; +export function assertNoPublicInternalReferences(generatedTs: string, internalTypes: Set): void { + if (internalTypes.size === 0) return; + + // Identify declarations tagged @internal anywhere in their JSDoc (multi-line or single-line). + const internalDeclarations = new Set(); + for (const m of generatedTs.matchAll( + /\/\*\*(?:[^*]|\*(?!\/))*@internal(?:[^*]|\*(?!\/))*\*\/\s*\nexport (?:interface|type|function|const) (\w+)\b/g + )) { + internalDeclarations.add(m[1]); + } - // Split into top-level declaration blocks so each reference can be attributed to its owner. - const declaration = /^export (?:interface|type) (\w+)\b/gm; + // Split on export interface/type/function/const boundaries for attribution. + const declarationRe = /^export (?:interface|type|function|const) (\w+)\b/gm; const starts: Array<{ index: number; name: string }> = []; - for (let m = declaration.exec(generatedTs); m !== null; m = declaration.exec(generatedTs)) { + for (let m = declarationRe.exec(generatedTs); m !== null; m = declarationRe.exec(generatedTs)) { starts.push({ index: m.index, name: m[1] }); } - if (starts.length === 0) return candidates; const blocks = starts.map((start, i) => ({ name: start.name, text: generatedTs.slice(start.index, i + 1 < starts.length ? starts[i + 1].index : generatedTs.length), })); - const safe = new Set(); - for (const candidate of candidates) { - const referencedByPublic = blocks.some( - (block) => - block.name !== candidate && - !candidates.has(block.name) && - new RegExp(`\\b${candidate}\\b`).test(block.text) - ); - if (referencedByPublic) { - console.warn( - ` ! ${candidate} is marked internal but is referenced by public declarations; ` + - `keeping it in the emitted .d.ts to avoid a dangling type reference` - ); - continue; + const violations: string[] = []; + for (const intType of internalTypes) { + for (const block of blocks) { + if (block.name === intType) continue; + if (internalDeclarations.has(block.name)) continue; + + // Strip content that does not appear in the emitted .d.ts: + // 1. All JSDoc/block comments — prevents doc-comment text that happens to name a + // type (e.g. "via the definition X") from registering as a code reference. + // 2. Function bodies — declaration emit drops bodies, so a reference inside a + // function implementation is not a public type reference. + // 3. @internal-tagged member sections — TypeScript's stripInternal removes them + // from the .d.ts along with any types they reference. + let publicText = block.text + // Remove all block comments (JSDoc and otherwise). + .replace(/\/\*[\s\S]*?\*\//g, "") + // Remove function bodies (from the opening { to matching closing }). + .replace(/\{[^{}]*(?:\{[^{}]*\}[^{}]*)*\}/g, "{}") + // Remove any remaining @internal-tagged member lines that survived (e.g. a + // stripped comment leaves behind a bare property declaration on the next line). + .replace(/[ \t]+\S[^\n]*@internal[^\n]*/g, ""); + + if (new RegExp(`\\b${intType}\\b`).test(publicText)) { + violations.push(` ${block.name} (public) references internal type ${intType}`); + } } - safe.add(candidate); } - return safe; + + if (violations.length > 0) { + throw new Error( + `Codegen produced public declarations that reference internal types.\n` + + `This is a codegen bug — fix the generator so internal types are not referenced by public output:\n` + + violations.join("\n") + ); + } } function sanitizeJsDocText(text: string): string { @@ -401,21 +404,37 @@ export function normalizeSchemaForTypeScript(schema: JSONSchema7): JSONSchema7 { // ── Session Events ────────────────────────────────────────────────────────── -async function generateSessionEvents(schemaPath?: string): Promise { - console.log("TypeScript: generating session-events..."); - - const resolvedPath = schemaPath ?? (await getSessionEventsSchemaPath()); - const schema = (await loadSchemaJson(resolvedPath)) as JSONSchema7; - const processed = propagateInternalVisibility(postProcessSchema(schema)); - const definitionCollections = collectDefinitionCollections(processed as Record); - const sessionEvent = - resolveSchema({ $ref: "#/definitions/SessionEvent" }, definitionCollections) ?? - resolveSchema({ $ref: "#/$defs/SessionEvent" }, definitionCollections) ?? - processed; +/** + * Filters a `SessionEvent` union schema to exclude internal arms. + * + * The schema marks internal union members with `visibility: "internal"` on the arm object itself + * AND on the resolved definition. An arm is excluded when either level is internal, or when the + * arm's resolved `data` property is internal (legacy pattern for event types that carry their + * payload in a `data` field). + * + * Returns the filtered arms and the set of definition names to exclude from compilation. + */ +export function filterPublicSessionEventVariants( + variants: JSONSchema7[], + definitionCollections: DefinitionCollections +): { publicVariants: JSONSchema7[]; excludedDefinitionNames: Set } { const excludedDefinitionNames = new Set(); - const publicVariants = (sessionEvent.anyOf ?? []).filter((variant) => { + const publicVariants = variants.filter((variant) => { const variantSchema = variant as JSONSchema7; const resolvedVariant = resolveSchema(variantSchema, definitionCollections) ?? variantSchema; + + // Exclude the arm if the arm object itself or its resolved definition is internal. + // The schema marks internal union members at both levels; checking only the resolved + // definition's `data` sub-property (the original logic) missed cases where the event + // type itself carries `visibility: "internal"`. + if (isSchemaInternal(variantSchema) || isSchemaInternal(resolvedVariant)) { + for (const ref of [variantSchema.$ref]) { + const match = ref?.match(/^#\/(?:definitions|\$defs)\/([^/]+)$/); + if (match) excludedDefinitionNames.add(match[1]); + } + return false; + } + const dataSchema = resolvedVariant.properties?.data as JSONSchema7 | undefined; const resolvedData = dataSchema ? resolveSchema(dataSchema, definitionCollections) ?? dataSchema : undefined; if (!isSchemaInternal(resolvedData)) { @@ -428,6 +447,24 @@ async function generateSessionEvents(schemaPath?: string): Promise { } return false; }); + return { publicVariants, excludedDefinitionNames }; +} + +async function generateSessionEvents(schemaPath?: string): Promise { + console.log("TypeScript: generating session-events..."); + + const resolvedPath = schemaPath ?? (await getSessionEventsSchemaPath()); + const schema = (await loadSchemaJson(resolvedPath)) as JSONSchema7; + const processed = propagateInternalVisibility(postProcessSchema(schema)); + const definitionCollections = collectDefinitionCollections(processed as Record); + const sessionEvent = + resolveSchema({ $ref: "#/definitions/SessionEvent" }, definitionCollections) ?? + resolveSchema({ $ref: "#/$defs/SessionEvent" }, definitionCollections) ?? + processed; + const { publicVariants, excludedDefinitionNames } = filterPublicSessionEventVariants( + sessionEvent.anyOf ?? [], + definitionCollections + ); const publicDefinitions = Object.fromEntries( Object.entries(definitionCollections.definitions).filter(([name]) => !excludedDefinitionNames.has(name)) ); @@ -461,6 +498,9 @@ async function generateSessionEvents(schemaPath?: string): Promise { // Add @internal JSDoc annotations for session-event types marked // `visibility: "internal"` in the schema. The tag drives `stripInternal` // so the whole type is dropped from the published .d.ts. + // Because internal union arms are excluded from the compiled output by the + // publicVariants filter above, no public declaration should reference these + // types; assertNoPublicInternalReferences enforces that invariant hard. const sessionInternalTypes = new Set(); for (const [name, def] of Object.entries(definitionCollections.definitions ?? {})) { if (def && typeof def === "object" && (def as Record).visibility === "internal") { @@ -472,7 +512,13 @@ async function generateSessionEvents(schemaPath?: string): Promise { sessionInternalTypes.add(name); } } - annotatedTs = tagInternalTypes(annotatedTs, sessionInternalTypes); + for (const intType of sessionInternalTypes) { + annotatedTs = annotatedTs.replace( + new RegExp(`(^|\\n)(export (?:interface|type) ${intType}\\b)`, "m"), + `$1/** @internal */\n$2` + ); + } + assertNoPublicInternalReferences(annotatedTs, sessionInternalTypes); const outPath = await writeGeneratedFile("nodejs/src/generated/session-events.ts", annotatedTs); console.log(` ✓ ${outPath}`); } @@ -812,7 +858,20 @@ function hasInternalMethods(node: Record): boolean { lines.push(...emitClientGlobalApiRegistration(schema.clientGlobal)); } - const outPath = await writeGeneratedFile("nodejs/src/generated/rpc.ts", tagInternalTypes(lines.join("\n"), internalTypes)); + // Apply @internal to RPC types in a final pass over the assembled file. + // The client/server method signatures that reference these types are emitted + // after the per-schema type chunks, so the tagging must happen here rather + // than per-chunk. assertNoPublicInternalReferences then enforces hard that no + // public declaration slipped through referencing a type the schema marked internal. + let rpcTs = lines.join("\n"); + for (const intType of internalTypes) { + rpcTs = rpcTs.replace( + new RegExp(`(^|\\n)(export (?:interface|type) ${intType}\\b)`, "m"), + `$1/** @internal */\n$2` + ); + } + assertNoPublicInternalReferences(rpcTs, internalTypes); + const outPath = await writeGeneratedFile("nodejs/src/generated/rpc.ts", rpcTs); console.log(` ✓ ${outPath}`); } @@ -1037,6 +1096,7 @@ function emitClientGlobalApiRegistration(clientSchema: Record): for (const [groupName, methods] of groups) { const interfaceName = toPascalCase(groupName) + "Handler"; + const publicMethods = methods.filter((m) => m.visibility !== "internal"); const groupDeprecated = isNodeFullyDeprecated(clientSchema[groupName] as Record); const groupExperimental = isNodeFullyExperimental(clientSchema[groupName] as Record); if (groupDeprecated) { @@ -1048,7 +1108,7 @@ function emitClientGlobalApiRegistration(clientSchema: Record): lines.push(`/** Handler for \`${groupName}\` client global API methods. */`); } lines.push(`export interface ${interfaceName} {`); - for (const method of methods) { + for (const method of publicMethods) { const name = handlerMethodName(method.rpcMethod); const hasParams = hasSchemaPayload(getMethodParamsSchema(method)); const pType = hasParams ? paramsTypeName(method) : ""; From 62573c144d79f9070b49bbdeeea17992128c710b Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 31 Jul 2026 15:30:01 +0000 Subject: [PATCH 3/6] fix: skip all-internal client global handler groups from public interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When all methods in a client-global RPC group are marked visibility:internal (currently: hooks.invoke), do not include the group in the generated HooksHandler interface or ClientGlobalApiHandlers. The registration body likewise only wires up public methods. The SDK handles hooks.invoke internally: client.ts registers the handler directly via connection.onRequest, bypassing ClientGlobalApiHandlers, so that HookInvokeRequest/HookType never appear in the public .d.ts surface. Tests that exercised clientGlobalHandlers.hooks.invoke are updated to call handleHooksInvoke directly — the same validation of the wire-format contract without depending on the now-removed public handler plumbing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 16 +++++++++++----- nodejs/src/generated/rpc.ts | 11 ----------- nodejs/test/client.test.ts | 9 +++++---- scripts/codegen/typescript.ts | 11 +++++++++-- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 6d99ce49e..dface04e8 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -831,11 +831,6 @@ export class CopilotClient { private setupClientGlobalHandlers(): void { const handlers: import("./generated/rpc.js").ClientGlobalApiHandlers = {}; - // `hooks.invoke` is a client-global RPC method whose payload carries a - // `sessionId`; route each invocation to the matching session's dispatcher. - handlers.hooks = { - invoke: async (params) => await this.handleHooksInvoke(params), - }; if (this.requestHandler) { handlers.llmInference = createCopilotRequestAdapter(this.requestHandler, () => { if (!this.connection) { @@ -2843,6 +2838,17 @@ export class CopilotClient { // — the runtime calls into a single handler for the whole connection. registerClientGlobalApiHandlers(this.connection, this.clientGlobalHandlers); + // `hooks.invoke` is an internal RPC method: the runtime calls it to + // invoke a hook callback on the client. Route each call to the matching + // session's dispatcher. Not part of the public ClientGlobalApiHandlers + // interface because HookInvokeRequest/HookType are internal types. + this.connection.onRequest( + "hooks.invoke", + async (params: { sessionId: string; hookType: string; input: unknown }) => { + return await this.handleHooksInvoke(params); + }, + ); + this.connection.onClose(() => { this.state = "disconnected"; }); diff --git a/nodejs/src/generated/rpc.ts b/nodejs/src/generated/rpc.ts index 2ceb6f0b0..d80c9386b 100644 --- a/nodejs/src/generated/rpc.ts +++ b/nodejs/src/generated/rpc.ts @@ -21351,11 +21351,6 @@ export function registerClientSessionApiHandlers( }); } -/** Handler for `hooks` client global API methods. */ -/** @experimental */ -export interface HooksHandler { -} - /** Handler for `llmInference` client global API methods. */ /** @experimental */ export interface LlmInferenceHandler { @@ -21390,7 +21385,6 @@ export interface GitHubTelemetryHandler { /** All client global API handler groups. */ export interface ClientGlobalApiHandlers { - hooks?: HooksHandler; llmInference?: LlmInferenceHandler; gitHubTelemetry?: GitHubTelemetryHandler; } @@ -21406,11 +21400,6 @@ export function registerClientGlobalApiHandlers( connection: MessageConnection, handlers: ClientGlobalApiHandlers, ): void { - connection.onRequest("hooks.invoke", async (params: HookInvokeRequest) => { - const handler = handlers.hooks; - if (!handler) throw new Error("No hooks client-global handler registered"); - return handler.invoke(params); - }); connection.onRequest("llmInference.httpRequestStart", async (params: LlmInferenceHttpRequestStartRequest) => { const handler = handlers.llmInference; if (!handler) throw new Error("No llmInference client-global handler registered"); diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 77149bc4b..b97d01cfb 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -3140,8 +3140,9 @@ describe("CopilotClient", () => { }); it("routes hooks.invoke JSON-RPC requests to the SessionHooks handler", async () => { - // Validates the full JSON-RPC entry point used by the CLI: - // clientGlobalHandlers.hooks.invoke({sessionId, hookType, input}) + // Validates the full entry point used by the CLI when the runtime + // calls the internal `hooks.invoke` RPC method: + // handleHooksInvoke({sessionId, hookType, input}) // → CopilotSession._handleHooksInvoke(hookType, input) // → SessionHooks.onPostToolUseFailure(normalizedInput, {sessionId}) // @@ -3172,7 +3173,7 @@ describe("CopilotClient", () => { cwd: "/tmp", }; - const response = await (client as any).clientGlobalHandlers.hooks.invoke({ + const response = await (client as any).handleHooksInvoke({ sessionId: session.sessionId, hookType: "postToolUseFailure", input: failureInput, @@ -3249,7 +3250,7 @@ describe("CopilotClient", () => { }, }); - const response = await (client as any).clientGlobalHandlers.hooks.invoke({ + const response = await (client as any).handleHooksInvoke({ sessionId: session.sessionId, hookType: "agentStop", input: { diff --git a/scripts/codegen/typescript.ts b/scripts/codegen/typescript.ts index 8ba65b578..d4f8599bb 100644 --- a/scripts/codegen/typescript.ts +++ b/scripts/codegen/typescript.ts @@ -1097,6 +1097,8 @@ function emitClientGlobalApiRegistration(clientSchema: Record): for (const [groupName, methods] of groups) { const interfaceName = toPascalCase(groupName) + "Handler"; const publicMethods = methods.filter((m) => m.visibility !== "internal"); + // Skip groups that have no public methods — they are handled internally by the SDK. + if (publicMethods.length === 0) continue; const groupDeprecated = isNodeFullyDeprecated(clientSchema[groupName] as Record); const groupExperimental = isNodeFullyExperimental(clientSchema[groupName] as Record); if (groupDeprecated) { @@ -1133,7 +1135,9 @@ function emitClientGlobalApiRegistration(clientSchema: Record): lines.push(`/** All client global API handler groups. */`); lines.push(`export interface ClientGlobalApiHandlers {`); - for (const [groupName] of groups) { + for (const [groupName, methods] of groups) { + const publicMethods = methods.filter((m) => m.visibility !== "internal"); + if (publicMethods.length === 0) continue; const interfaceName = toPascalCase(groupName) + "Handler"; lines.push(` ${groupName}?: ${interfaceName};`); } @@ -1153,7 +1157,10 @@ function emitClientGlobalApiRegistration(clientSchema: Record): lines.push(`): void {`); for (const [groupName, methods] of groups) { - for (const method of methods) { + // Only wire up public methods; internal methods are handled directly by the SDK. + const publicMethods = methods.filter((m) => m.visibility !== "internal"); + if (publicMethods.length === 0) continue; + for (const method of publicMethods) { const name = handlerMethodName(method.rpcMethod); const pType = paramsTypeName(method); const hasParams = hasSchemaPayload(getMethodParamsSchema(method)); From 286da630ec7ebb5776c7418f7b87f512a390d544 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 31 Jul 2026 15:43:36 +0000 Subject: [PATCH 4/6] style: apply prettier formatting to client hooks registration Fix the CI-only formatting failure in nodejs/src/client.ts introduced by the previous hooks.invoke plumbing change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index dface04e8..803252fe3 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -2846,7 +2846,7 @@ export class CopilotClient { "hooks.invoke", async (params: { sessionId: string; hookType: string; input: unknown }) => { return await this.handleHooksInvoke(params); - }, + } ); this.connection.onClose(() => { From afae702d336b1f653445420b42789e78e3e5a6a7 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 31 Jul 2026 15:48:13 +0000 Subject: [PATCH 5/6] test: tighten internal reference validation and coverage Fix assertNoPublicInternalReferences so it only strips function bodies, preserving interface/type member signatures for validation. Also remove @internal-tagged members before scanning and add a regression test that a public interface member referencing an internal type fails validation. Add an explicit unit test that attachConnectionHandlers registers the hand-written hooks.invoke JSON-RPC entry point and routes it to handleHooksInvoke. Rename the legacy-pattern session-event test to match its actual assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/client.test.ts | 50 ++++++++++++++++++++------ nodejs/test/typescript-codegen.test.ts | 17 ++++++++- scripts/codegen/typescript.ts | 22 +++++++----- 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index b97d01cfb..6371642e2 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -3139,17 +3139,47 @@ describe("CopilotClient", () => { expect(failureCalls).toEqual(["fail-tool"]); }); + it("registers hooks.invoke on the JSON-RPC connection and routes it to handleHooksInvoke", async () => { + const client = new CopilotClient(); + const handleHooksInvoke = vi + .spyOn(client as any, "handleHooksInvoke") + .mockResolvedValue({ output: { additionalContext: "ok" } }); + + const fakeConnection = { + onNotification: vi.fn(), + onRequest: vi.fn(), + onClose: vi.fn(), + onError: vi.fn(), + }; + + (client as any).connection = fakeConnection; + (client as any).attachConnectionHandlers(); + + const hooksRegistration = fakeConnection.onRequest.mock.calls.find( + ([method]: [string, unknown]) => method === "hooks.invoke" + ); + expect(hooksRegistration).toBeDefined(); + + const handler = hooksRegistration![1] as (params: { + sessionId: string; + hookType: string; + input: unknown; + }) => Promise<{ output?: unknown }>; + const payload = { + sessionId: "session-1", + hookType: "postToolUseFailure", + input: { toolName: "shell" }, + }; + + await expect(handler(payload)).resolves.toEqual({ + output: { additionalContext: "ok" }, + }); + expect(handleHooksInvoke).toHaveBeenCalledWith(payload); + }); + it("routes hooks.invoke JSON-RPC requests to the SessionHooks handler", async () => { - // Validates the full entry point used by the CLI when the runtime - // calls the internal `hooks.invoke` RPC method: - // handleHooksInvoke({sessionId, hookType, input}) - // → CopilotSession._handleHooksInvoke(hookType, input) - // → SessionHooks.onPostToolUseFailure(normalizedInput, {sessionId}) - // - // This guards the wire-format contract that the bundled Copilot - // CLI relies on: the hookType string "postToolUseFailure" and the - // input shape `{toolName, toolArgs, error, timestamp, cwd}`. - // The SDK maps that to public `{..., timestamp: Date, workingDirectory}`. + // Validates the dispatch behavior for the internal `hooks.invoke` + // payload after the JSON-RPC connection hands it to the SDK. const client = new CopilotClient(); await client.start(); onTestFinished(() => stopClient(client)); diff --git a/nodejs/test/typescript-codegen.test.ts b/nodejs/test/typescript-codegen.test.ts index ad90e760d..aaf23a124 100644 --- a/nodejs/test/typescript-codegen.test.ts +++ b/nodejs/test/typescript-codegen.test.ts @@ -108,7 +108,7 @@ describe("filterPublicSessionEventVariants", () => { expect(excludedDefinitionNames.has("InternalEvent")).toBe(true); }); - it("keeps arms whose internal data sub-property is the only internal marker (legacy pattern)", () => { + it("excludes arms whose internal data sub-property is the only internal marker (legacy pattern)", () => { // Event types that carry a `data: InternalData` field — the `data` property is what is // internal, not the event wrapper type itself. const defs = { @@ -194,6 +194,21 @@ export type Event = PublicEvent | Hidden; ); }); + it("throws when a public interface member references an internal type", () => { + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +export interface Public { + value: Hidden; +} +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).toThrow( + /Public \(public\) references internal type Hidden/ + ); + }); + it("does not count JSDoc comment text as a code reference", () => { // The auto-generated JSDoc says 'via the definition "Hidden"' but that is not a // real TypeScript type reference — it must not trigger the validator. diff --git a/scripts/codegen/typescript.ts b/scripts/codegen/typescript.ts index d4f8599bb..33fec471e 100644 --- a/scripts/codegen/typescript.ts +++ b/scripts/codegen/typescript.ts @@ -76,12 +76,13 @@ export function assertNoPublicInternalReferences(generatedTs: string, internalTy } // Split on export interface/type/function/const boundaries for attribution. - const declarationRe = /^export (?:interface|type|function|const) (\w+)\b/gm; - const starts: Array<{ index: number; name: string }> = []; + const declarationRe = /^export (interface|type|function|const) (\w+)\b/gm; + const starts: Array<{ index: number; kind: string; name: string }> = []; for (let m = declarationRe.exec(generatedTs); m !== null; m = declarationRe.exec(generatedTs)) { - starts.push({ index: m.index, name: m[1] }); + starts.push({ index: m.index, kind: m[1], name: m[2] }); } const blocks = starts.map((start, i) => ({ + kind: start.kind, name: start.name, text: generatedTs.slice(start.index, i + 1 < starts.length ? starts[i + 1].index : generatedTs.length), })); @@ -100,13 +101,16 @@ export function assertNoPublicInternalReferences(generatedTs: string, internalTy // 3. @internal-tagged member sections — TypeScript's stripInternal removes them // from the .d.ts along with any types they reference. let publicText = block.text - // Remove all block comments (JSDoc and otherwise). - .replace(/\/\*[\s\S]*?\*\//g, "") + // Remove @internal-tagged member declarations before stripping comments so + // member-level internal references do not count as part of the public surface. + .replace(/^[ \t]*\/\*\*[\s\S]*?@internal[\s\S]*?\*\/\s*\n[ \t]*[^\n]+\n?/gm, "") + // Remove all remaining block comments (JSDoc and otherwise). + .replace(/\/\*[\s\S]*?\*\//g, ""); + + if (block.kind === "function") { // Remove function bodies (from the opening { to matching closing }). - .replace(/\{[^{}]*(?:\{[^{}]*\}[^{}]*)*\}/g, "{}") - // Remove any remaining @internal-tagged member lines that survived (e.g. a - // stripped comment leaves behind a bare property declaration on the next line). - .replace(/[ \t]+\S[^\n]*@internal[^\n]*/g, ""); + publicText = publicText.replace(/\{[^{}]*(?:\{[^{}]*\}[^{}]*)*\}/g, "{}"); + } if (new RegExp(`\\b${intType}\\b`).test(publicText)) { violations.push(` ${block.name} (public) references internal type ${intType}`); From e46c15b0179f6d870d9f9769dae02ff73ccf73bc Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 31 Jul 2026 15:51:58 +0000 Subject: [PATCH 6/6] fix: ignore internal object-valued members in TS validator The fail-hard validator correctly ignores simple @internal-tagged members, but it still treated inline object-shaped members like as public references. That caused false failures when public event payloads contained internal members whose types are removed by stripInternal. Extend the member-removal pass to drop both simple and inline object-valued @internal members before scanning for public references, and add a regression test covering that shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/typescript-codegen.test.ts | 20 ++++++++++++++++++++ scripts/codegen/typescript.ts | 7 ++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/nodejs/test/typescript-codegen.test.ts b/nodejs/test/typescript-codegen.test.ts index aaf23a124..e3f3d2857 100644 --- a/nodejs/test/typescript-codegen.test.ts +++ b/nodejs/test/typescript-codegen.test.ts @@ -231,6 +231,26 @@ export interface Following { expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); }); + it("does not count inline object-shaped @internal members as public references", () => { + const ts = ` +/** @internal */ +export interface Hidden { + x: number; +} +export interface Public { + /** + * Some field. + * @internal + */ + secret?: { + [k: string]: Hidden | undefined; + }; + visible: string; +} +`; + expect(() => assertNoPublicInternalReferences(ts, new Set(["Hidden"]))).not.toThrow(); + }); + it("does not count function body references as public type references", () => { const ts = ` /** @internal */ diff --git a/scripts/codegen/typescript.ts b/scripts/codegen/typescript.ts index 33fec471e..30cefd3e9 100644 --- a/scripts/codegen/typescript.ts +++ b/scripts/codegen/typescript.ts @@ -103,7 +103,12 @@ export function assertNoPublicInternalReferences(generatedTs: string, internalTy let publicText = block.text // Remove @internal-tagged member declarations before stripping comments so // member-level internal references do not count as part of the public surface. - .replace(/^[ \t]*\/\*\*[\s\S]*?@internal[\s\S]*?\*\/\s*\n[ \t]*[^\n]+\n?/gm, "") + // Handles both simple members (`foo?: Hidden;`) and inline object-shaped members + // (`foo?: { ... };`) used by generated TypeScript interfaces. + .replace( + /^[ \t]*\/\*\*[\s\S]*?@internal[\s\S]*?\*\/\s*\n(?:[ \t]*[^\n{;]+;\n?|[ \t]*[^\n{]+\{\n[\s\S]*?^[ \t]*\};\n?)/gm, + "" + ) // Remove all remaining block comments (JSDoc and otherwise). .replace(/\/\*[\s\S]*?\*\//g, "");