From cc32f4392011f66f9f131bd6fcae35946f67bfd7 Mon Sep 17 00:00:00 2001 From: KazariAI <166915487+KazariAI@users.noreply.github.com> Date: Thu, 8 Oct 2026 14:12:14 +0800 Subject: [PATCH 1/2] fix(bridge): honor allowEdits in source-file skeleton line helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tsserver remaps definitions through declaration maps (session mapDefinitionInfoLocations -> documentPositionMapper), and that decoder calls getPositionOfLineAndCharacter(..., allowEdits=true) for EVERY mapping in the map, clamping stale coordinates — real packages ship them (unified@11.0.5's index.d.ts.map has generated 846:313 on a 301-char line). The host-side SourceFile skeletons dropped the allowEdits argument, so stock's clamping call took the strict path, hit Debug.assert, and the whole definition request failed: goto definition on an import of such a package resolved to nothing while quickinfo still worked. Forward (line, character, allowEdits?) in all three skeletons (createSharedLightStub, installLazyLineHelpers, patchLineHelpers), matching the stock SourceFileLike contract; non-allowEdits callers keep the strict behavior. Witness triage-gtd-declmap-allowedits drives the pinned unified@11.0.5 reproduction through both engines (pre-fix TNB fails the request with the Debug assert; post-fix lands at unified/lib/index.js like stock), wired wg5. --- .../src/compiler/tsgoBackedSourceFile.ts | 10 +- .../overlay/src/compiler/tsgoChecker.ts | 6 +- tools/ci-witness-groups.mjs | 3 +- tools/triage-gtd-declmap-allowedits.mjs | 129 ++++++++++++++++++ 4 files changed, 142 insertions(+), 6 deletions(-) create mode 100755 tools/triage-gtd-declmap-allowedits.mjs diff --git a/patches/typescript/overlay/src/compiler/tsgoBackedSourceFile.ts b/patches/typescript/overlay/src/compiler/tsgoBackedSourceFile.ts index ccb7e77..17009a5 100644 --- a/patches/typescript/overlay/src/compiler/tsgoBackedSourceFile.ts +++ b/patches/typescript/overlay/src/compiler/tsgoBackedSourceFile.ts @@ -47,8 +47,9 @@ function installLazyLineHelpers(target: any, getText: () => string, cacheKey?: s target.getLineStarts = () => ensureLineStarts(); target.getLineAndCharacterOfPosition = (pos: number) => computeLineAndCharacterOfPosition(ensureLineStarts(), pos); - target.getPositionOfLineAndCharacter = (line: number, ch: number) => - computePositionOfLineAndCharacter(ensureLineStarts(), line, ch); + target.getPositionOfLineAndCharacter = (line: number, ch: number, allowEdits?: true) => + // Stock contract: allowEdits=true clamps (declaration-map decoding passes it). + computePositionOfLineAndCharacter(ensureLineStarts(), line, ch, getText(), allowEdits); if (!target.lineMap) { Object.defineProperty(target, "lineMap", { configurable: true, @@ -138,8 +139,9 @@ function patchLineHelpers(shell: any, realSf: any): void { const ls = realSf.getLineStarts(); shell.getLineStarts = () => ls; shell.getLineAndCharacterOfPosition = (pos: number) => computeLineAndCharacterOfPosition(ls, pos); - shell.getPositionOfLineAndCharacter = (line: number, ch: number) => - computePositionOfLineAndCharacter(ls, line, ch); + shell.getPositionOfLineAndCharacter = (line: number, ch: number, allowEdits?: true) => + // Stock contract: allowEdits=true clamps (declaration-map decoding passes it). + computePositionOfLineAndCharacter(ls, line, ch, realSf.text as string | undefined, allowEdits); } export function getTsgoBackedSourceFile(realSf: any): any | undefined { diff --git a/patches/typescript/overlay/src/compiler/tsgoChecker.ts b/patches/typescript/overlay/src/compiler/tsgoChecker.ts index 67c6bf6..13a0763 100644 --- a/patches/typescript/overlay/src/compiler/tsgoChecker.ts +++ b/patches/typescript/overlay/src/compiler/tsgoChecker.ts @@ -654,7 +654,11 @@ function createSharedLightStub(hostFileName: string, configFilePath: string, met get lineMap() { return lineStartsOf(); }, getLineStarts: () => lineStartsOf(), getLineAndCharacterOfPosition: (position: number) => ts.computeLineAndCharacterOfPosition(lineStartsOf(), position), - getPositionOfLineAndCharacter: (line: number, character: number) => ts.computePositionOfLineAndCharacter(lineStartsOf(), line, character, textOf()), + // Stock SourceFileLike contract (scanner.getPositionOfLineAndCharacter): + // allowEdits=true clamps out-of-range coords — declaration-map decoding + // (documentPositionMapper) relies on it for stale map coordinates. + getPositionOfLineAndCharacter: (line: number, character: number, allowEdits?: true) => + ts.computePositionOfLineAndCharacter(lineStartsOf(), line, character, textOf(), allowEdits), forEachChild: (...args: any[]) => { const sf = full(); return sf?.forEachChild ? sf.forEachChild(...args) : undefined; diff --git a/tools/ci-witness-groups.mjs b/tools/ci-witness-groups.mjs index 0b952cb..38aa058 100644 --- a/tools/ci-witness-groups.mjs +++ b/tools/ci-witness-groups.mjs @@ -48,7 +48,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; -const TOTAL = 92; +const TOTAL = 93; // Witnesses intentionally NOT in the matrix — run on demand (reasons above). const LOCAL_ONLY = [ @@ -182,6 +182,7 @@ const groups = [ 'triage-empty-file-position', // ~2s (issue #72) 'triage-deep-relation-cache', // ~3s (issue #77) 'triage-extra-extension-dts', // ~5s (issue #63) + 'triage-gtd-declmap-allowedits', // ~6s (unified install ~4s + two tsserver sessions) ], }, { diff --git a/tools/triage-gtd-declmap-allowedits.mjs b/tools/triage-gtd-declmap-allowedits.mjs new file mode 100755 index 0000000..eac5f0e --- /dev/null +++ b/tools/triage-gtd-declmap-allowedits.mjs @@ -0,0 +1,129 @@ +#!/usr/bin/env node +/** + * Go-to-definition through a declaration map must not crash the request. + * + * Stock's SourceFileLike contract is getPositionOfLineAndCharacter(line, + * character, allowEdits?) — allowEdits=true clamps out-of-range coordinates + * instead of asserting (scanner.ts). The documentPositionMapper that remaps + * definitions through d.ts.map files (session mapDefinitionInfoLocations → + * tryGetSourcePosition) decodes EVERY mapping in the map with allowEdits=true, + * and real packages ship maps with generated coordinates past the line end + * (unified@11.0.5 index.d.ts.map has one: generated 846:313 on a 301-char + * line). TNB's host-side SourceFile skeletons dropped the allowEdits argument, + * so stock's clamping call took the strict path, hit Debug.assert, and the + * whole `definition` request failed (success:false) — goto definition on an + * import of any such package resolved to nothing while quickinfo still worked. + * + * Fixture: unified@11.0.5 (pinned, installed per run) — the reported + * reproduction. Its index.d.ts.map carries a stale generated coordinate + * (846:313 on a 301-char line); the mapper decodes EVERY mapping with + * allowEdits=true, so decoding against the un-materialized light stub is the + * exact crash path. Dual-engine protocol check: the definition (and + * definitionAndBoundSpan) from the import specifier must succeed and match + * stock, landing inside the package's .js body via the map. + * + * Stock side: STOCK_TSSERVER_PATH (CI), else /tmp/stock-ts-p3/package/lib/tsserver.js. + * + * Usage: node tools/triage-gtd-declmap-allowedits.mjs + * Exit 0 = parity, 1 = regression. + */ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { createRequire } from 'node:module'; +import ChildProcess from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { tnbHarnessEnv, withTsserver } from './tsserver-harness.mjs'; + +const require2 = createRequire(import.meta.url); +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const tnbPath = path.join(repoRoot, 'lib', 'tsserver.js'); +const stockPath = process.env.STOCK_TSSERVER_PATH ?? '/tmp/stock-ts-p3/package/lib/tsserver.js'; + +// ── Fixture: pinned unified@11.0.5, the reported reproduction ── +// unified ships `"exports": "./index.js"` (no types condition) and an +// index.d.ts.map with a stale generated coordinate (846:313 on a 301-char +// line). That resolution shape is what leaves lib/index.d.ts un-materialized +// in the program at definition time, so the session's remap decodes the map +// against the light stub — every synthesized stand-in package materialized +// fully and could not reach the defect. Installed once per run, pinned. +const UNIFIED_VERSION = '11.0.5'; + +function makeFixture() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'tnb-gtd-declmap-')); + const src = path.join(dir, 'src'); + fs.mkdirSync(src, { recursive: true }); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ + name: 'consumer', private: true, type: 'module', + dependencies: { unified: UNIFIED_VERSION }, + })); + fs.writeFileSync(path.join(dir, 'tsconfig.json'), JSON.stringify({ + compilerOptions: { strict: true, noEmit: true, module: 'nodenext', moduleResolution: 'nodenext', types: [], skipLibCheck: true }, + include: ['src'], + })); + const consumer = "import { unified } from 'unified';\nconst p = unified().use(function() { return ''; });\nexport { p };\n"; + fs.writeFileSync(path.join(src, 'index.ts'), consumer); + const install = ChildProcess.spawnSync('npm', [ + 'install', '--no-save', '--ignore-scripts', '--no-audit', '--no-fund', + `unified@${UNIFIED_VERSION}`, + ], { cwd: dir, encoding: 'utf8', timeout: 120_000 }); + if (install.status !== 0 || !fs.existsSync(path.join(dir, 'node_modules', 'unified', 'lib', 'index.d.ts'))) { + console.error(`fixture setup failed: npm install unified@${UNIFIED_VERSION}\n${install.stderr ?? ''}${install.stdout ?? ''}`); + process.exit(2); + } + return { dir, file: path.join(src, 'index.ts'), consumer }; +} + +// Inside the `value` import specifier (line 1, col 10). +const POS = { line: 1, offset: 10 }; +const harnessArgs = ['--disableAutomaticTypingAcquisition', '--suppressDiagnosticEvents']; + +// mkdtemp paths can surface as /var/... in one engine and the realpath +// /private/var/... in the other, and TNB canonicalizes payload paths to +// lowercase on case-insensitive hosts (CI is Linux, where none of this +// applies) — compare case-insensitively relative to the fixture dir. +function normDef(d) { + const f = String(d.file ?? ''); + const i = f.toLowerCase().indexOf(`${fixtureReal.toLowerCase()}/`); + return { + file: i >= 0 ? f.slice(i + fixtureReal.length + 1) : f, + line: d.start?.line, offset: d.start?.offset, + endLine: d.end?.line, endOffset: d.end?.offset, + kind: d.kind, name: d.name, + }; +} + +async function run(label, tsserverPath, fixture) { + return withTsserver({ tsserverPath, args: harnessArgs, env: tnbHarnessEnv() }, async ({ send }) => { + await send('configure', { preferences: {} }); + await send('updateOpen', { + changedFiles: [], closedFiles: [], + openFiles: [{ file: fixture.file, fileContent: fixture.consumer, projectRootPath: fixture.dir }], + }); + const def = await send('definition', { file: fixture.file, ...POS }); + const defBound = await send('definitionAndBoundSpan', { file: fixture.file, ...POS }); + return { + label, + def: { success: def?.success === true, defs: (def?.body ?? []).map(normDef), message: def?.message?.split('\n')[0] }, + defBound: { success: defBound?.success === true, defs: (defBound?.body?.definitions ?? []).map(normDef) }, + }; + }); +} + +const fixture = makeFixture(); +const fixtureReal = fs.realpathSync(fixture.dir); +const tnb = await run('TNB', tnbPath, fixture); +const stock = await run('STOCK', stockPath, fixture); +fs.rmSync(fixture.dir, { recursive: true, force: true }); + +const sig = (r) => JSON.stringify([r.def.success, r.def.defs, r.defBound.success, r.defBound.defs]); +const parity = tnb.def.success && tnb.defBound.success && sig(tnb) === sig(stock) + // The navigation must land in the package body, not fall back to the specifier. + && tnb.def.defs.length > 0 + && tnb.def.defs.every(d => d.file.includes('unified/lib/index.js')); + +console.log(`TNB : ${sig(tnb)}`); +console.log(`STOCK: ${sig(stock)}`); +console.log(`verdict: ${parity ? 'PARITY' : 'DIFF'}`); +if (tnb.def.message) console.log(`TNB definition message: ${tnb.def.message}`); +process.exit(parity ? 0 : 1); From 568b3d7f9956b98d5688c2c815c7e0d3528790dc Mon Sep 17 00:00:00 2001 From: KazariAI <166915487+KazariAI@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:30:14 +0800 Subject: [PATCH 2/2] test(witness): synthesize gtd-declmap-allowedits fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unified@11.0.5 npm install was the witness's only dependency and most of its runtime (~4s of ~6s), and its pass depended on upstream package content staying stale. A synthesized fake-unified package reproduces the three conditions by construction (exports→js resolution leaving the d.ts a light stub, an out-of-range mapping, a sourceMappingURL comment), so the witness needs no npm or network, runs in ~1.5s, and cannot silently degrade if upstream fixes their map. --- tools/ci-witness-groups.mjs | 2 +- tools/triage-gtd-declmap-allowedits.mjs | 76 +++++++++++++------------ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/tools/ci-witness-groups.mjs b/tools/ci-witness-groups.mjs index 38aa058..a5938fc 100644 --- a/tools/ci-witness-groups.mjs +++ b/tools/ci-witness-groups.mjs @@ -182,7 +182,7 @@ const groups = [ 'triage-empty-file-position', // ~2s (issue #72) 'triage-deep-relation-cache', // ~3s (issue #77) 'triage-extra-extension-dts', // ~5s (issue #63) - 'triage-gtd-declmap-allowedits', // ~6s (unified install ~4s + two tsserver sessions) + 'triage-gtd-declmap-allowedits', // ~1.5s (synthesized fixture + two tsserver sessions) ], }, { diff --git a/tools/triage-gtd-declmap-allowedits.mjs b/tools/triage-gtd-declmap-allowedits.mjs index eac5f0e..81030e0 100755 --- a/tools/triage-gtd-declmap-allowedits.mjs +++ b/tools/triage-gtd-declmap-allowedits.mjs @@ -6,21 +6,27 @@ * character, allowEdits?) — allowEdits=true clamps out-of-range coordinates * instead of asserting (scanner.ts). The documentPositionMapper that remaps * definitions through d.ts.map files (session mapDefinitionInfoLocations → - * tryGetSourcePosition) decodes EVERY mapping in the map with allowEdits=true, - * and real packages ship maps with generated coordinates past the line end - * (unified@11.0.5 index.d.ts.map has one: generated 846:313 on a 301-char - * line). TNB's host-side SourceFile skeletons dropped the allowEdits argument, - * so stock's clamping call took the strict path, hit Debug.assert, and the - * whole `definition` request failed (success:false) — goto definition on an - * import of any such package resolved to nothing while quickinfo still worked. + * tryGetSourcePosition) decodes EVERY mapping in the map with allowEdits=true. + * TNB's host-side SourceFile skeletons dropped the allowEdits argument, so + * stock's clamping call took the strict path, hit Debug.assert, and the whole + * `definition` request failed (success:false) — goto definition on an import + * of any such package resolved to nothing while quickinfo still worked. * - * Fixture: unified@11.0.5 (pinned, installed per run) — the reported - * reproduction. Its index.d.ts.map carries a stale generated coordinate - * (846:313 on a 301-char line); the mapper decodes EVERY mapping with - * allowEdits=true, so decoding against the un-materialized light stub is the - * exact crash path. Dual-engine protocol check: the definition (and - * definitionAndBoundSpan) from the import specifier must succeed and match - * stock, landing inside the package's .js body via the map. + * Fixture: synthesized `fake-unified` package (installed in-place by this + * script — no npm, no network). It reproduces the three conditions of the + * reported reproduction (unified@11.0.5, whose index.d.ts.map carries a stale + * generated coordinate 846:313 on a 301-char line): + * 1. `"exports": "./index.js"` with no types condition — the resolution + * shape that leaves lib/index.d.ts un-materialized in the program at + * definition time, so the session's remap decodes the map against the + * light stub. + * 2. index.d.ts.map with a mapping whose generated column (50) is past the + * end of the generated line it names — the allowEdits clamp path. + * 3. `//# sourceMappingURL` in index.d.ts — what routes the session mapper + * through the declaration map. + * Dual-engine protocol check: the definition (and definitionAndBoundSpan) + * from the import specifier must succeed and match stock, landing inside the + * package's .js body via the map. * * Stock side: STOCK_TSSERVER_PATH (CI), else /tmp/stock-ts-p3/package/lib/tsserver.js. * @@ -30,51 +36,47 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { createRequire } from 'node:module'; -import ChildProcess from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { tnbHarnessEnv, withTsserver } from './tsserver-harness.mjs'; -const require2 = createRequire(import.meta.url); const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); const tnbPath = path.join(repoRoot, 'lib', 'tsserver.js'); const stockPath = process.env.STOCK_TSSERVER_PATH ?? '/tmp/stock-ts-p3/package/lib/tsserver.js'; -// ── Fixture: pinned unified@11.0.5, the reported reproduction ── -// unified ships `"exports": "./index.js"` (no types condition) and an -// index.d.ts.map with a stale generated coordinate (846:313 on a 301-char -// line). That resolution shape is what leaves lib/index.d.ts un-materialized -// in the program at definition time, so the session's remap decodes the map -// against the light stub — every synthesized stand-in package materialized -// fully and could not reach the defect. Installed once per run, pinned. -const UNIFIED_VERSION = '11.0.5'; +// index.js has 3 short lines; the map's single mapping sits on generated line +// 3 (the two leading ';' separators) at column 50, past that line's end. +// VLQ for absolute [col 49, source 0, line 0, col 0] is "iDAAA". +const mappings = ';;iDAAA'; function makeFixture() { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'tnb-gtd-declmap-')); const src = path.join(dir, 'src'); + const pkg = path.join(dir, 'node_modules', 'fake-unified'); fs.mkdirSync(src, { recursive: true }); + fs.mkdirSync(pkg, { recursive: true }); fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'consumer', private: true, type: 'module', - dependencies: { unified: UNIFIED_VERSION }, + dependencies: { 'fake-unified': '1.0.0' }, })); fs.writeFileSync(path.join(dir, 'tsconfig.json'), JSON.stringify({ compilerOptions: { strict: true, noEmit: true, module: 'nodenext', moduleResolution: 'nodenext', types: [], skipLibCheck: true }, include: ['src'], })); - const consumer = "import { unified } from 'unified';\nconst p = unified().use(function() { return ''; });\nexport { p };\n"; + fs.writeFileSync(path.join(pkg, 'package.json'), JSON.stringify({ + name: 'fake-unified', version: '1.0.0', exports: './index.js', + })); + fs.writeFileSync(path.join(pkg, 'index.js'), 'export const bar = 1;\n\nexport const baz = 2;\n'); + fs.writeFileSync(path.join(pkg, 'index.d.ts'), + 'export declare function foo(): string;\n//# sourceMappingURL=index.d.ts.map\n'); + fs.writeFileSync(path.join(pkg, 'index.d.ts.map'), JSON.stringify({ + version: 3, file: 'index.js', sources: ['index.js'], names: [], mappings, + })); + const consumer = "import { foo } from 'fake-unified';\nexport const p = foo();\n"; fs.writeFileSync(path.join(src, 'index.ts'), consumer); - const install = ChildProcess.spawnSync('npm', [ - 'install', '--no-save', '--ignore-scripts', '--no-audit', '--no-fund', - `unified@${UNIFIED_VERSION}`, - ], { cwd: dir, encoding: 'utf8', timeout: 120_000 }); - if (install.status !== 0 || !fs.existsSync(path.join(dir, 'node_modules', 'unified', 'lib', 'index.d.ts'))) { - console.error(`fixture setup failed: npm install unified@${UNIFIED_VERSION}\n${install.stderr ?? ''}${install.stdout ?? ''}`); - process.exit(2); - } return { dir, file: path.join(src, 'index.ts'), consumer }; } -// Inside the `value` import specifier (line 1, col 10). +// Inside the `foo` import specifier (line 1, col 10). const POS = { line: 1, offset: 10 }; const harnessArgs = ['--disableAutomaticTypingAcquisition', '--suppressDiagnosticEvents']; @@ -120,7 +122,7 @@ const sig = (r) => JSON.stringify([r.def.success, r.def.defs, r.defBound.success const parity = tnb.def.success && tnb.defBound.success && sig(tnb) === sig(stock) // The navigation must land in the package body, not fall back to the specifier. && tnb.def.defs.length > 0 - && tnb.def.defs.every(d => d.file.includes('unified/lib/index.js')); + && tnb.def.defs.every(d => d.file.includes('fake-unified/index.js')); console.log(`TNB : ${sig(tnb)}`); console.log(`STOCK: ${sig(stock)}`);