From 30c358a2f8aa2ad77b98481ad81a43d613a79b60 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Thu, 25 Jun 2026 12:08:21 +0300 Subject: [PATCH] test(review): add the 4 new test-coverage points from PR #185 re-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-review's blocking/structural points (lease leak, dup-id guard test, body-before-title test, CHANGELOG, pg18, shared jsonb decoder) were already addressed in commit 24264ef; this adds the 4 genuinely-new coverage requests: - pt 6: `scrollToReference(id, index?)` exercised against a live editor DOM — selects the index-th `sup[data-footnote-ref][data-id]` occurrence, falls back to the first for out-of-range, returns false for an empty id (scrollIntoView stubbed). (#168) - pt 7: export `backlinkLabel` and pin the base-26 carry boundary (25->z, 26->aa, 27->ab, 51->az, 52->ba). (#168) - pt 8: integration fail-open — a PRESENT-but-corrupt tool_allowlist (jsonb string scalar holding non-array JSON) reads back as null ("no restriction"), covering normalizeRow's degrade branch. (#159 #172/#173) - pt 9: getFootnoteRefCount cache invalidation — adding a `[^a]` reference bumps the cached count 2 -> 3. (#168) Verified: editor-ext footnote 23; client structure 7 + tsc; server int 8. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../footnote/footnote-definition-view.tsx | 2 +- .../footnote-views.structure.test.tsx | 17 ++- .../ai-mcp-server-repo.int-spec.ts | 19 ++++ .../src/lib/footnote/footnote.test.ts | 105 ++++++++++++++++++ 4 files changed, 141 insertions(+), 2 deletions(-) diff --git a/apps/client/src/features/editor/components/footnote/footnote-definition-view.tsx b/apps/client/src/features/editor/components/footnote/footnote-definition-view.tsx index 7f6cc7b3..b8fe182f 100644 --- a/apps/client/src/features/editor/components/footnote/footnote-definition-view.tsx +++ b/apps/client/src/features/editor/components/footnote/footnote-definition-view.tsx @@ -7,7 +7,7 @@ import classes from "./footnote.module.css"; * A 0-based backlink index -> its lowercase letter label (0 -> "a", 25 -> "z", * 26 -> "aa", ...), matching the Pandoc/Wikipedia "↩ a b c" convention. */ -function backlinkLabel(index: number): string { +export function backlinkLabel(index: number): string { let out = ""; let x = index; while (x >= 0) { diff --git a/apps/client/src/features/editor/components/footnote/footnote-views.structure.test.tsx b/apps/client/src/features/editor/components/footnote/footnote-views.structure.test.tsx index e6cd46a6..bfffac90 100644 --- a/apps/client/src/features/editor/components/footnote/footnote-views.structure.test.tsx +++ b/apps/client/src/features/editor/components/footnote/footnote-views.structure.test.tsx @@ -75,7 +75,9 @@ vi.mock("@/features/editor/components/code-block/mermaid-view.tsx", () => ({ })); import FootnotesListView from "./footnotes-list-view"; -import FootnoteDefinitionView from "./footnote-definition-view"; +import FootnoteDefinitionView, { + backlinkLabel, +} from "./footnote-definition-view"; import CodeBlockView from "../code-block/code-block-view"; // Minimal NodeViewProps stub: definition view only touches node.attrs.id and @@ -214,3 +216,16 @@ describe("#168 footnote definition multi-backlinks", () => { ); }); }); + +// #185 re-review pt 7: backlinkLabel is base-26 (a..z, then aa…). The component +// tests only cover a,b,c (index 0-2); pin the >= 26 carry boundary. +describe("backlinkLabel base-26 boundary (#168)", () => { + it("maps 0->a, 25->z, 26->aa, 27->ab, 51->az, 52->ba", () => { + expect(backlinkLabel(0)).toBe("a"); + expect(backlinkLabel(25)).toBe("z"); + expect(backlinkLabel(26)).toBe("aa"); + expect(backlinkLabel(27)).toBe("ab"); + expect(backlinkLabel(51)).toBe("az"); + expect(backlinkLabel(52)).toBe("ba"); + }); +}); diff --git a/apps/server/test/integration/ai-mcp-server-repo.int-spec.ts b/apps/server/test/integration/ai-mcp-server-repo.int-spec.ts index 0730f46d..2e181791 100644 --- a/apps/server/test/integration/ai-mcp-server-repo.int-spec.ts +++ b/apps/server/test/integration/ai-mcp-server-repo.int-spec.ts @@ -91,6 +91,25 @@ describe('AiMcpServerRepo tool_allowlist jsonb round-trip [integration]', () => const healed = enabled.find((r) => r.id === id); expect(healed?.toolAllowlist).toEqual(['alpha', 'beta']); }); + + it('FAIL-OPEN: a present-but-corrupt tool_allowlist reads back as null (no restriction)', async () => { + // #185 re-review pt 8: normalizeRow's fail-open branch — the column is + // PRESENT but does not parse into a string[] (here a jsonb string scalar + // holding non-array JSON). The read must degrade to `null` ("no restriction"), + // not crash. (A warn is logged with the server id; not asserted here.) + const id = randomUUID(); + await sql` + INSERT INTO ai_mcp_servers (id, workspace_id, name, transport, url, tool_allowlist) + VALUES ( + ${id}, ${ws}, ${`srv-${id}`}, 'http', 'https://example.com/mcp', + to_jsonb(${'{"not":"an array"}'}::text) + ) + `.execute(db); + // Sanity: the column is present (a jsonb string scalar), not SQL NULL. + expect(await jsonbTypeof(id)).toBe('string'); + // ...yet the read degrades to null (fail-open). + expect((await repo.findById(id, ws))?.toolAllowlist).toBeNull(); + }); }); /** diff --git a/packages/editor-ext/src/lib/footnote/footnote.test.ts b/packages/editor-ext/src/lib/footnote/footnote.test.ts index 11c868f6..d539d832 100644 --- a/packages/editor-ext/src/lib/footnote/footnote.test.ts +++ b/packages/editor-ext/src/lib/footnote/footnote.test.ts @@ -162,6 +162,111 @@ describe('getFootnoteRefCount (cached, live editor)', () => { expect(getFootnoteRefCount(editor.state, 'nope')).toBe(0); editor.destroy(); }); + + // #185 re-review pt 9: the cached count must update on a doc change (mirror of + // the number-cache invalidation test) — add another `[^a]` reference and the + // count goes 2 -> 3. + it('recomputes the cached ref count when a reference is added', () => { + const editor = makeEditor({ + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'a' } }, + { type: 'text', text: ' and ' }, + { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'a' } }, + ], + }, + { + type: FOOTNOTES_LIST_NAME, + content: [ + { + type: FOOTNOTE_DEFINITION_NAME, + attrs: { id: 'a' }, + content: [{ type: 'paragraph' }], + }, + ], + }, + ], + }); + expect(getFootnoteRefCount(editor.state, 'a')).toBe(2); + + // Insert a THIRD reference to `a` at the start of the first paragraph. + const refType = editor.schema.nodes[FOOTNOTE_REFERENCE_NAME]; + editor.view.dispatch( + editor.state.tr.insert(1, refType.create({ id: 'a' })), + ); + + expect(getFootnoteRefCount(editor.state, 'a')).toBe(3); + editor.destroy(); + }); +}); + +// #185 re-review pt 6: scrollToReference picks the index-th occurrence among the +// reused references, falls back to the first for an out-of-range index, and is a +// no-op (false) for an empty id. Runs the REAL command against the editor's DOM +// (scrollIntoView is stubbed — jsdom does not implement it). +describe('scrollToReference command (occurrence selection + fallback)', () => { + it('selects the index-th occurrence, falls back to the first, false for empty id', () => { + const scrolled: Element[] = []; + const original = (Element.prototype as any).scrollIntoView; + (Element.prototype as any).scrollIntoView = function () { + scrolled.push(this as Element); + }; + try { + const editor = makeEditor({ + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'a' } }, + { type: 'text', text: ' x ' }, + { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'a' } }, + { type: 'text', text: ' y ' }, + { type: FOOTNOTE_REFERENCE_NAME, attrs: { id: 'a' } }, + ], + }, + { + type: FOOTNOTES_LIST_NAME, + content: [ + { + type: FOOTNOTE_DEFINITION_NAME, + attrs: { id: 'a' }, + content: [{ type: 'paragraph' }], + }, + ], + }, + ], + }); + const sups = editor.view.dom.querySelectorAll( + 'sup[data-footnote-ref][data-id="a"]', + ); + expect(sups.length).toBe(3); + + // index 1 -> the SECOND occurrence. + expect(editor.commands.scrollToReference('a', 1)).toBe(true); + expect(scrolled[scrolled.length - 1]).toBe(sups[1]); + + // out-of-range index -> falls back to the FIRST occurrence. + expect(editor.commands.scrollToReference('a', 99)).toBe(true); + expect(scrolled[scrolled.length - 1]).toBe(sups[0]); + + // default index (0) -> first. + expect(editor.commands.scrollToReference('a')).toBe(true); + expect(scrolled[scrolled.length - 1]).toBe(sups[0]); + + // empty id -> false, no scroll. + const before = scrolled.length; + expect(editor.commands.scrollToReference('')).toBe(false); + expect(scrolled.length).toBe(before); + + editor.destroy(); + } finally { + (Element.prototype as any).scrollIntoView = original; + } + }); }); describe('setFootnote command', () => {