diff --git a/src/server/responses/code-mode-collab-surface.ts b/src/server/responses/code-mode-collab-surface.ts index 63f13dff9a7..0515d60ad96 100644 --- a/src/server/responses/code-mode-collab-surface.ts +++ b/src/server/responses/code-mode-collab-surface.ts @@ -77,13 +77,100 @@ interface RenderedTool { name: string; raw: string } /** The declaration fence codex-rs opens: three backticks and "ts". */ const DECLARATION_FENCE_SHAPE: Fence = { char: "`", length: 3 }; -/** A complete declaration as render_code_mode_sample emits it, single- or multi-line. */ -function isCompleteDeclaration(body: readonly string[], name: string): boolean { +const DECLARATION_PREFIX = "declare const tools: { "; +const PROMISE_SEPARATOR = "): Promise<"; +const DECLARATION_SUFFIX = ">; };"; +// Bounds keep a caller-controlled description linear work: a rendered listing is a heading, a +// description, and a small declaration per tool, so anything this large cannot be the codex-rs +// shape this parser exists to recognize. +const MAX_DESCRIPTION_LENGTH = 1 << 20; +const MAX_DECLARATION_LENGTH = 64 * 1024; + +const isSpace = (ch: string | undefined): boolean => ch === undefined || /\s/.test(ch); + +/** + * The declared tool name when body is a complete declaration as render_code_mode_sample emits it, + * single- or multi-line, else undefined. Fixed boundaries plus one forward scan that balances the + * parameter list's brackets and the return type's generics: no backtracking, so a fence body full + * of candidate separators stays linear work. + */ +function completeDeclarationName(body: readonly string[]): string | undefined { const text = body.join("\n"); - const prefix = "declare const tools: { " + name + "("; - if (!text.startsWith(prefix)) return false; + if (text.length > MAX_DECLARATION_LENGTH) return undefined; + if (!text.startsWith(DECLARATION_PREFIX) || !text.endsWith(DECLARATION_SUFFIX)) return undefined; + const signature = text.slice(DECLARATION_PREFIX.length, -DECLARATION_SUFFIX.length); + const openParen = signature.indexOf("("); + const name = openParen > 0 ? signature.slice(0, openParen) : ""; + if (!/^[A-Za-z_$][\w$]*$/.test(name)) return undefined; // A named, typed parameter (`args` for functions, `input` for freeform) and a non-empty return type. - return /^(?:args|input): \S[\s\S]*\): Promise<[^>\s][\s\S]*>; \};$/.test(text.slice(prefix.length)); + const rest = signature.slice(openParen + 1); + const typed = rest.startsWith("args: ") ? 6 : rest.startsWith("input: ") ? 7 : -1; + if (typed < 0 || isSpace(rest[typed])) return undefined; + // Quoted text and comments are opaque to delimiter matching. Each skip advances + // past its text once; an unterminated region cannot contain a closing declaration. + const isOpaqueTypeText = (at: number): boolean => { + const ch = rest[at]; + return ch === '"' || ch === "'" || ch === "`" + || ch === "/" && (rest[at + 1] === "/" || rest[at + 1] === "*"); + }; + const skipTypeText = (at: number): number | undefined => { + const quote = rest[at]!; + if (quote === "/" && rest[at + 1] === "/") { + let i = at + 2; + while (i < rest.length && rest[i] !== "\n" && rest[i] !== "\r" + && rest[i] !== "\u2028" && rest[i] !== "\u2029") i += 1; + return i < rest.length ? i : undefined; + } + if (quote === "/" && rest[at + 1] === "*") { + const end = rest.indexOf("*/", at + 2); + return end < 0 ? undefined : end + 1; + } + let i = at + 1; + while (i < rest.length) { + const ch = rest[i]!; + if (ch === "\\") i += 2; + else if (ch === quote) return i; + else i += 1; + } + return undefined; + }; + // The parameter list ends at the ')' that balances the opening '(': a ')' nested inside the + // parameter's own type (e.g. `args: (a: string) => Promise`) cannot pose as the separator. + let depth = 1; + let close = -1; + for (let at = typed; at < rest.length; at += 1) { + const ch = rest[at]!; + if (isOpaqueTypeText(at)) { + const end = skipTypeText(at); + if (end === undefined) return undefined; + at = end; + continue; + } + if (ch === "(" || ch === "[" || ch === "{") depth += 1; + else if (ch === ")" || ch === "]" || ch === "}") depth -= 1; + if (depth === 0) { close = at; break; } + } + if (close < 0 || !rest.startsWith(PROMISE_SEPARATOR, close)) return undefined; + const head = rest[close + PROMISE_SEPARATOR.length]; + if (head === ">" || isSpace(head)) return undefined; + // The return type runs to the stripped suffix, which already supplied exactly one '>' for the + // Promise's '<': generic depth must stay positive to the end and land at exactly 1 — a '>' that + // reaches zero ('Promise; garbage') is surplus text, a depth left above 1 + // ('Promise' arrow never counts. + let generic = 1; + for (let at = close + PROMISE_SEPARATOR.length; at < rest.length; at += 1) { + const ch = rest[at]!; + if (isOpaqueTypeText(at)) { + const end = skipTypeText(at); + if (end === undefined) return undefined; + at = end; + continue; + } + if (ch === "<") generic += 1; + else if (ch === ">" && rest[at - 1] !== "=") generic -= 1; + if (generic === 0) return undefined; + } + return generic === 1 ? name : undefined; } /** codex-rs normalize_code_mode_identifier: invalid identifier characters become `_`. */ @@ -114,9 +201,21 @@ function renderedHeading(match: RegExpExecArray): RenderedTool | undefined { * alone. Lines inside a quoted fence can neither open a section nor declare a tool. */ function renderedNestedTools(description: string): RenderedTool[] { + if (description.length > MAX_DESCRIPTION_LENGTH) return []; const lines = description.split("\n"); const rendered: RenderedTool[] = []; const pending: RenderedTool[] = []; + // Ascending indexes into `pending` per name, so a declaration finds its owner in one lookup + // instead of rescanning every pending heading. + const pendingByName = new Map(); + const dropPendingTo = (length: number): void => { + while (pending.length > length) { + const removed = pending.pop()!; + const indexes = pendingByName.get(removed.name)!; + indexes.pop(); + if (indexes.length === 0) pendingByName.delete(removed.name); + } + }; let fence: Fence | undefined; for (let i = 0; i < lines.length; i += 1) { const line = lines[i]!; @@ -127,7 +226,11 @@ function renderedNestedTools(description: string): RenderedTool[] { const match = TOOL_HEADING.exec(line); if (match) { const heading = renderedHeading(match); - if (heading) pending.push(heading); + if (heading) { + const indexes = pendingByName.get(heading.name); + if (indexes) indexes.push(pending.length); else pendingByName.set(heading.name, [pending.length]); + pending.push(heading); + } continue; } if (line === DECLARATION_LEAD && lines[i + 1] === DECLARATION_FENCE) { @@ -137,10 +240,11 @@ function renderedNestedTools(description: string): RenderedTool[] { const body = lines.slice(i + 2, close); // The outermost pending heading with the declared name owns it, so a same-name heading quoted // inside the tool's own description cannot take over the genuine section. - const owner = pending.findIndex(tool => isCompleteDeclaration(body, tool.name)); + const declared = completeDeclarationName(body); + const owner = declared === undefined ? -1 : pendingByName.get(declared)?.[0] ?? -1; if (owner >= 0) { rendered.push(pending[owner]!); - pending.length = owner; + dropPendingTo(owner); } i = close; continue; diff --git a/structure/subagents.md b/structure/subagents.md index 4948e676f29..23233beb0a2 100644 --- a/structure/subagents.md +++ b/structure/subagents.md @@ -448,6 +448,7 @@ neither prove nor contradict a native surface. The two checks are asymmetric on purpose: evidence needs a rendered section, while the contradiction check reads every heading and declaration line, quoted or not, so text copied into the listing can push a turn toward `null` but cannot hide another surface. +The parse uses bounded linear scans: descriptions over `1 << 20` string code units carry no nested evidence, declarations over `64 * 1024` code units are not recognized, and quoted text and comments are skipped while matching delimiters; return generics must close fully. Nested evidence never produces `v2`. When the model supports tool search (every OpenCodex catalog row), codex-rs defers the V1 tools and leaves them out of the description. Its own `` item exists only on V2 and it pins the version per session, so neither the saved diff --git a/tests/codex-integration/code-mode-collab-surface.test.ts b/tests/codex-integration/code-mode-collab-surface.test.ts index 0f5dfebc0b1..2644ae0cffa 100644 --- a/tests/codex-integration/code-mode-collab-surface.test.ts +++ b/tests/codex-integration/code-mode-collab-surface.test.ts @@ -220,11 +220,31 @@ describe("code mode without V1 evidence stays null", () => { test.each([ ["an abbreviated parameter list", "declare const tools: { multi_agent_v1__spawn_agent(...): Promise; };"], ["an empty return type", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise<>; };"], + ["surplus text after the return type", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise; garbage>; };"], + ["an unbalanced parameter type", "declare const tools: { multi_agent_v1__spawn_agent(args: {: Promise; };"], + ["a return type missing its generic closer", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise; };"], + ["an unterminated quoted return type", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise<\"unterminated>; };"], + ["an unterminated block comment", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise; };"], + ["a line comment consuming the closing suffix", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise; };"], ])("%s is not a rendered declaration", (_label, declaration) => { const snippet = [heading("multi_agent_v1__spawn_agent"), "exec tool declaration:", FENCE + "ts", declaration, FENCE].join("\n"); expect(surfaceOf(codeModeTools(execDescription(nested("docs__lookup", snippet))))).toBeNull(); }); + test.each([ + ["a nested generic return type", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise>; };"], + ["a function-typed parameter", "declare const tools: { multi_agent_v1__spawn_agent(args: (task: string) => void): Promise; };"], + ["a quoted bracket inside the parameter type", "declare const tools: { multi_agent_v1__spawn_agent(args: { choice: \")\" }): Promise; };"], + ["an apostrophe in a field line comment", "declare const tools: { multi_agent_v1__spawn_agent(args: {\n // The agent's task.\n message: string;\n}): Promise; };"], + ["an unmatched bracket in a field line comment", "declare const tools: { multi_agent_v1__spawn_agent(args: {\n // Optional unmatched bracket ) in prose.\n message: string;\n}): Promise; };"], + ["delimiters in a block comment", "declare const tools: { multi_agent_v1__spawn_agent(args: { /* ) < ' */ message: string }): Promise; };"], + ["a comment in the return type", "declare const tools: { multi_agent_v1__spawn_agent(args: {}): Promise; };"], + ["comment markers inside quoted type text", "declare const tools: { multi_agent_v1__spawn_agent(args: { marker: \"/* not a comment */ //\" }): Promise<\"// quoted < )\">; };"], + ])("%s still completes the declaration", (_label, declaration) => { + const snippet = [heading("multi_agent_v1__spawn_agent"), "exec tool declaration:", FENCE + "ts", declaration, FENCE].join("\n"); + expect(surfaceOf(codeModeTools(execDescription(nested("docs__lookup", snippet))))).toBe("v1"); + }); + test("a raw-name suffix that does not normalize to its heading is not a heading", () => { // Placed in the namespace description, ahead of the genuine heading it would otherwise outrank. const namespace = "## docs\n" + heading("docs__lookup", "multi_agent_v1__spawn_agent"); @@ -346,3 +366,39 @@ describe("a caller-defined exec contributes no nested evidence", () => { expect(surfaceOf([{ type: "custom", name: "exec", description: foreign }])).toBeNull(); }); }); + +describe("an adversarial declaration body stays linear work", () => { + // Regression: the declaration check must not backtrack per candidate Promise separator, and the + // owner lookup must not re-validate the body once per pending heading. On the pre-fix regex this + // ~393 KiB body starved the event loop for seconds well inside the default test timeout. + const separators = "): Promise { + const section = [heading("multi_agent_v1__spawn_agent"), listing].join("\n"); + expect(surfaceOf(codeModeTools(execDescription(section)))).toBeNull(); + }); + + test("many pending same-name headings do not multiply the parse", () => { + const headings = Array.from({ length: 256 }, () => heading("multi_agent_v1__spawn_agent")).join("\n"); + expect(surfaceOf(codeModeTools(execDescription(headings, listing)))).toBeNull(); + }); + + test("a description beyond the nested-listing bound contributes no evidence", () => { + const padded = SPAWN_DESCRIPTION + "\n" + "padding ".repeat(200_000); + expect(surfaceOf(codeModeTools(execDescription(nested("multi_agent_v1__spawn_agent", padded))))).toBeNull(); + }); + + test("comment-heavy declarations below the bound retain their evidence", () => { + const comments = "/* ): Promise<' */".repeat(2048); + const declaration = `declare const tools: { multi_agent_v1__spawn_agent(args: { ${comments} message: string }): Promise; };`; + expect(declaration.length).toBeLessThan(64 * 1024); + const snippet = [heading("multi_agent_v1__spawn_agent"), "exec tool declaration:", FENCE + "ts", declaration, FENCE].join("\n"); + expect(surfaceOf(codeModeTools(execDescription(nested("docs__lookup", snippet))))).toBe("v1"); + }); +});