diff --git a/review-enrichment/src/render-helpers.ts b/review-enrichment/src/render-helpers.ts index e4b2f31ec9..64d79d5f43 100644 --- a/review-enrichment/src/render-helpers.ts +++ b/review-enrichment/src/render-helpers.ts @@ -25,15 +25,25 @@ export interface AnalyzerRenderHelpers { bytesLabel(value: number | null): string; } +// safeCodeSpan/promptText sit at the analyzer-output boundary: their signature promises `string`, but analyzer +// findings are loosely-typed data flowing through `as never` casts (registry.ts render()), not values this module +// controls -- a descriptor whose finding shape drifts (or a future config-driven table) can hand either helper a +// non-string. Coercing defensively here mirrors bytesLabel's own `value: number | null` guard below: never let a +// render helper throw into the brief pipeline (GITTENSORY-15, `value.replace is not a function`). +function asRenderableString(value: string): string { + return typeof value === "string" ? value : String(value ?? ""); +} + export function safeCodeSpan(value: string): string { - return `\`${value.replace( + const safe = asRenderableString(value); + return `\`${safe.replace( CODE_SPAN_UNSAFE, (char) => CODE_SPAN_REPLACEMENTS[char] ?? "\ufffd", )}\``; } export function promptText(value: string): string { - return value + return asRenderableString(value) .replace(/[\u0000-\u001f\u007f]/g, " ") .replace(/\\/g, "\\\\") .replace(/`/g, "\\`") diff --git a/review-enrichment/test/render-helpers.test.ts b/review-enrichment/test/render-helpers.test.ts new file mode 100644 index 0000000000..cbf29141fb --- /dev/null +++ b/review-enrichment/test/render-helpers.test.ts @@ -0,0 +1,58 @@ +// Units for the shared analyzer-render helpers (render-helpers.ts). Own file (not render-contract.test.ts or +// terminology.test.ts) so it can cover the helpers directly as well as the boundary they sit behind. Regression +// coverage for GITTENSORY-15: "TypeError: value.replace is not a function" thrown from safeCodeSpan when a +// terminology finding's `term`/`suggestion` reached render as a non-string. safeCodeSpan/promptText are typed +// `(value: string): string`, but they receive values from less-trusted analyzer/descriptor output (routed through +// `as never` casts in render.ts/registry.ts), the same boundary bytesLabel already defends with its +// `value: number | null` guard -- so both now defensively coerce instead of trusting the type signature. +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { safeCodeSpan, promptText } from "../dist/render-helpers.js"; +import { renderBrief } from "../dist/render.js"; + +test("safeCodeSpan: wraps a normal string in backticks unchanged", () => { + assert.equal(safeCodeSpan("src/net.ts:12"), "`src/net.ts:12`"); +}); + +test("safeCodeSpan: escapes backticks and control characters in a real string", () => { + assert.equal(safeCodeSpan("a`b\nc"), "`aˋb␤c`"); +}); + +test("safeCodeSpan: coerces a non-string value instead of throwing (GITTENSORY-15 regression)", () => { + assert.equal(safeCodeSpan(undefined), "``"); + assert.equal(safeCodeSpan(null), "``"); + assert.equal(safeCodeSpan(42), "`42`"); + assert.equal(safeCodeSpan(["master", "slave"]), "`master,slave`"); + assert.equal(safeCodeSpan({ toString: () => "obj" }), "`obj`"); +}); + +test("promptText: escapes markdown-sensitive characters in a real string", () => { + assert.equal(promptText("a*b_c"), "a\\*b\\_c"); +}); + +test("promptText: coerces a non-string value instead of throwing (GITTENSORY-15 regression)", () => { + assert.equal(promptText(undefined), ""); + assert.equal(promptText(null), ""); + assert.equal(promptText(7), "7"); +}); + +test("renderBrief: terminology descriptor render survives a non-string term/suggestion without throwing", () => { + // Mirrors the exact GITTENSORY-15 stack trace: registry.ts's "terminology" descriptor template calls + // safeCodeSpan(item.term) and safeCodeSpan(item.suggestion) directly (no template-literal coercion), unlike + // the `${item.file}:${item.line}` interpolation just before it. Simulates a malformed finding (e.g. from a + // future config-driven term table or an upstream payload drift) reaching the render path unchanged. + assert.doesNotThrow(() => { + // Cast through `any` (not `@ts-expect-error`) since test/**/*.ts is outside tsconfig.json's "src"-only + // include and is run via --experimental-strip-types (no type-check on test files) -- an unused + // `@ts-expect-error` here would be inert, not a compile guard. + const malformedFinding = { file: "src/net.ts", line: 1, term: undefined, suggestion: 42 }; + const { promptSection } = renderBrief({ + terminology: [malformedFinding], + }); + assert.match(promptSection, /Non-inclusive terminology/); + assert.match(promptSection, /src\/net\.ts:1/); + assert.match(promptSection, /``/); // the coerced-empty term renders as an empty code span + assert.match(promptSection, /`42`/); // the coerced-number suggestion renders as `42` + }); +});