From c5ac3efe9fb4eae236956fe735d87b6b3ce1b33f Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 1 Aug 2026 09:39:12 +0800 Subject: [PATCH] =?UTF-8?q?fix(wave):=20composite-review=20findings=20?= =?UTF-8?q?=E2=80=94=20mask=20wikilink=20interiors=20from=20the=20bare-pat?= =?UTF-8?q?h=20scan;=20future-safe=20extractor=20watermark?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cross-PR findings from the wave-i hostile review (Codex xhigh): 1. The #3560-ungated bare-path pass scanned inside [[...]] spans, so a dir-qualified wikilink's lowercase prefix (its parent page) became a spurious 'markdown' edge whenever the parent existed. Wikilink spans are now masked with equal-length blanks before pass 2; discriminating test added (fails without the mask). 2. LINK_EXTRACTOR_VERSION_TS was midnight today with a strict-< staleness predicate, so same-day stamps from pre-wave code read as fresh and never re-extracted. Bumped to 2026-08-02T00:00:00Z. Plus two comment corrections from both reviewers: upgrade.ts's X1 hook rationale (stale after #3085) and PGLiteEngine.transaction's tx-engine db-proxy hazard for #3613's searchVector wrapper. --- src/commands/upgrade.ts | 11 ++++++----- src/core/link-extraction.ts | 14 ++++++++++++-- src/core/pglite-engine.ts | 5 +++++ test/link-extraction.test.ts | 25 +++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/commands/upgrade.ts b/src/commands/upgrade.ts index 313ab4120..e9a4a9c10 100644 --- a/src/commands/upgrade.ts +++ b/src/commands/upgrade.ts @@ -397,11 +397,12 @@ export async function runPostUpgrade(args: string[] = []): Promise { } // v0.28.5 (X1): explicitly apply pending schema migrations. - // apply-migrations runs orchestrator migrations and only WARNs about - // schema-version drift (apply-migrations.ts:296-302). Without this hook, - // `gbrain upgrade` leaves wedged brains wedged — the user has to read - // the WARN and run `gbrain init --migrate-only` themselves. We've shipped - // 11 wedge incidents asking users to read warnings; close the loop here. + // Since #3085, apply-migrations --yes applies schema-version drift itself + // (it previously only WARNed), so the in-process call above may have + // already run these — runMigrations is idempotent, making this hook a + // harmless second pass. It stays because it also covers paths where the + // preflight was skipped. We've shipped 11 wedge incidents asking users to + // read warnings; keep the loop closed here. // A1's hasPendingMigrations probe in connectEngine is belt-and-suspenders // for any path that bypasses upgrade (autopilot, direct CLI on stale brain). try { diff --git a/src/core/link-extraction.ts b/src/core/link-extraction.ts index 90bce0512..0d8830358 100644 --- a/src/core/link-extraction.ts +++ b/src/core/link-extraction.ts @@ -35,7 +35,10 @@ import { slugifyPath } from './sync.ts'; // whitelist no longer drops markdown links / bare-slug refs / slash-shaped // wikilinks in non-whitelisted directories). Pages stamped by earlier sweeps // are re-flagged so the next --stale sweep re-extracts under both fixes. -export const LINK_EXTRACTOR_VERSION_TS = '2026-08-01T00:00:00Z'; +// The watermark is the day AFTER the wave ships: the staleness predicate is +// strict `<`, so a same-day stamp written by pre-wave code would otherwise +// read as fresh and never re-extract. +export const LINK_EXTRACTOR_VERSION_TS = '2026-08-02T00:00:00Z'; // ─── Entity references ────────────────────────────────────────── @@ -589,7 +592,14 @@ export async function extractPageLinks( // that happens to look like a path (`on/off`, `com/foo/bar` inside a URL) // is dropped by the callers' page-existence checks, never persisted. // Code blocks are stripped first — slugs in code samples are not real refs. - const strippedContent = stripCodeBlocks(content); + // Wikilink spans are masked too (equal-length blanks, so match indices stay + // valid for excerpt()): the wikilink pass above owns `[[...]]` interiors, + // and without the mask a dir-qualified wikilink like + // `[[llm-wiki/entities/AI 3.0]]` leaves its lowercase prefix + // `llm-wiki/entities` as a bare-path match — a spurious edge to the parent + // page whenever that page exists. + const strippedContent = stripCodeBlocks(content) + .replace(/\[\[[^\]]*\]\]/g, (s) => ' '.repeat(s.length)); const bareRe = new RegExp( `\\b(${ANY_DIR_SEGMENT}\\/[a-z0-9][a-z0-9/-]*[a-z0-9])\\b`, 'g', diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index 8a784b695..db7d2df8a 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -972,6 +972,11 @@ export class PGLiteEngine implements BrainEngine { return fn(conn); } + // NOTE: the tx-engine handed to `fn` proxies `db` to a PGLite Transaction, + // which has query/sql/exec but NO .transaction — so engine methods that + // open their own transaction (searchVector since #3613) will throw if + // called on the tx-engine. No current callback does; keep it that way or + // add pass-through nesting first. async transaction(fn: (engine: BrainEngine) => Promise): Promise { return this.db.transaction(async (tx) => { const txEngine = Object.create(this) as PGLiteEngine; diff --git a/test/link-extraction.test.ts b/test/link-extraction.test.ts index c7bb5bc30..664c809fa 100644 --- a/test/link-extraction.test.ts +++ b/test/link-extraction.test.ts @@ -615,6 +615,31 @@ describe('extractPageLinks', () => { expect(resolved.map(c => c.targetSlug)).toEqual(['vault/llm-wiki/entities/ai-3.0']); }); + test('wikilink interiors are masked from the bare-path pass (no parent-page edge)', async () => { + // Codex wave-i finding: `[[llm-wiki/entities/AI 3.0]]` leaves its + // lowercase prefix `llm-wiki/entities` as a bare-path match if the + // scanner sees wikilink interiors — a spurious 'markdown' edge to the + // PARENT page whenever it exists. The mask blanks `[[...]]` spans before + // pass 2; the wikilink pass owns those interiors. + const resolver: SlugResolver = { + resolve: async () => null, + resolveBasenameMatches: async (name) => + name === 'ai-3.0' ? ['llm-wiki/entities/ai-3.0'] : [], + }; + const { candidates } = await extractPageLinks( + 'llm-wiki/notes/roadmap', + 'See [[llm-wiki/entities/AI 3.0]] for the model. Also see ops/runbook.', + {}, 'concept', resolver, + ); + // The parent-prefix must NOT appear from the wikilink interior... + expect(candidates.map(c => c.targetSlug)).not.toContain('llm-wiki/entities'); + // ...while a genuine bare path in prose still produces its candidate, + expect(candidates.map(c => c.targetSlug)).toContain('ops/runbook'); + // and the wikilink itself still resolves through its own pass. + expect(candidates.filter(c => c.linkSource === 'wikilink-resolved') + .map(c => c.targetSlug)).toEqual(['llm-wiki/entities/ai-3.0']); + }); + test('opts.skipFrontmatter suppresses the frontmatter pass', async () => { // Real resolver shape that WOULD resolve frontmatter source: too, // but skipFrontmatter blocks the path entirely.