Skip to content

Commit 5b1b1aa

Browse files
authored
fix: fingerprint the built daemon's real import graph (#1545) (#1771)
* fix: fingerprint the built daemon's real import graph (#1545) The static-import regex in computeDaemonCodeSignature required whitespace around `from` and after `import`/`export`. The built daemon entry is a minified bundle (tsdown/rolldown `minify: true`), whose real import statements have zero whitespace (e.g. `import{o as e}from"../foo.js"`), so the regex never matched any dependency edge and the graph walk silently degraded to fingerprinting only the entry file (`graph:1`) instead of its true ~50-file runtime graph. That defeats the safety net the signature exists for: a daemon started from one build can look identical to a client running a materially different build as long as the entry file's own size/mtime happen to coincide, and the "code-signature mismatch" takeover this check drives was one of the mechanisms observed in #1545's worktree dev-daemon multiplication. * refactor: fingerprint import specifiers by shape, not by import grammar Replaces the two keyword-anchored regexes (one for static import/export/from clauses, one for dynamic import()) with a single pattern that matches any quoted, relative-path-shaped string literal. The prior approach had to track the exact whitespace/keyword shape bundlers emit around a specifier, which is exactly what broke for minified output in the immediately preceding commit — formatted source, minified builds, static imports, re-exports, and dynamic imports all put the same thing in the same place (a relative string literal), so matching on that directly is both simpler and immune to future formatting differences. A same-shaped string that isn't really an import is safe to over-match: it just fails to resolve to a file and gets dropped. Verified the file count is unchanged against this repo's real build (dist: graph:50, src: graph:741, both identical to the keyword-anchored fix).
1 parent b59b4e5 commit 5b1b1aa

2 files changed

Lines changed: 84 additions & 15 deletions

File tree

src/daemon/code-signature.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,18 @@ import fs from 'node:fs';
33
import path from 'node:path';
44
import { findProjectRoot } from '../utils/version.ts';
55

6-
const STATIC_IMPORT_RE =
7-
/(?:^|[^\w$.])(?:import|export)\s+(?:type\s+)?(?:[^'"`]*?\s+from\s+)?['"]([^'"]+)['"]/gm;
8-
const DYNAMIC_IMPORT_RE = /import\(\s*['"]([^'"]+)['"]\s*\)/gm;
6+
// Any quoted, relative-path-shaped string literal is treated as a module
7+
// specifier, rather than matching the `import`/`export`/`from` grammar
8+
// around it: bundlers only ever emit relative string literals for real
9+
// specifiers, but the surrounding syntax varies too much to track reliably —
10+
// formatted source spaces `import { x } from './y'` out, a minified build
11+
// (tsdown/rolldown `minify: true`) squashes it to `import{x}from"./y"`, and a
12+
// keyword-anchored regex tuned for one silently stops matching the other
13+
// (#1545: the daemon's own built entry fingerprinted to just itself, since
14+
// nothing downstream of it ever matched). A literal that isn't really an
15+
// import (e.g. one that shows up inside a comment) simply fails to resolve
16+
// to a file below and gets dropped, so over-matching here is harmless.
17+
const RELATIVE_SPECIFIER_RE = /(['"])(\.\.?\/[^'"]*)\1/g;
918
const RESOLVABLE_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs'] as const;
1019

1120
export function resolveDaemonCodeSignature(): string {
@@ -55,20 +64,12 @@ export function computeDaemonCodeSignature(
5564

5665
function collectRelativeImportSpecifiers(content: string): string[] {
5766
const specifiers = new Set<string>();
58-
collectImportMatches(content, STATIC_IMPORT_RE, specifiers);
59-
collectImportMatches(content, DYNAMIC_IMPORT_RE, specifiers);
60-
return [...specifiers];
61-
}
62-
63-
function collectImportMatches(content: string, pattern: RegExp, specifiers: Set<string>): void {
64-
pattern.lastIndex = 0;
67+
RELATIVE_SPECIFIER_RE.lastIndex = 0;
6568
let match: RegExpExecArray | null = null;
66-
while ((match = pattern.exec(content)) !== null) {
67-
const specifier = match[1]?.trim();
68-
if (specifier?.startsWith('.')) {
69-
specifiers.add(specifier);
70-
}
69+
while ((match = RELATIVE_SPECIFIER_RE.exec(content)) !== null) {
70+
specifiers.add(match[2]!);
7171
}
72+
return [...specifiers];
7273
}
7374

7475
function resolveRelativeImportPath(fromPath: string, specifier: string): string | null {

src/utils/__tests__/daemon-client.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1776,6 +1776,74 @@ test('computeDaemonCodeSignature fingerprints the daemon runtime import graph',
17761776
}
17771777
});
17781778

1779+
test('computeDaemonCodeSignature walks minified bundler output (#1545)', () => {
1780+
// The published/built daemon entry is a minified bundle (tsdown/rolldown
1781+
// `minify: true`): real import/export statements have zero whitespace
1782+
// around `from` or even after the keyword itself, e.g.
1783+
// `import{o as e}from"./dep.js"`. computeDaemonCodeSignature detects a
1784+
// dependency edge from the quoted relative-path string alone (not from the
1785+
// import/export/from grammar around it), so every one of these shapes —
1786+
// named, re-export-all, and bare side-effect imports — resolves the same
1787+
// way regardless of formatting, and a same-shaped string that isn't really
1788+
// an import (the `importantValue` literal below) is simply ignored because
1789+
// it doesn't resolve to a file.
1790+
const root = mkdtempForTestSync('agent-device-daemon-signature-minified-');
1791+
try {
1792+
const daemonEntryPath = path.join(root, 'daemon.js');
1793+
const depPath = path.join(root, 'dep.js');
1794+
const reExportPath = path.join(root, 're-export.js');
1795+
const sideEffectPath = path.join(root, 'side-effect.js');
1796+
fs.writeFileSync(
1797+
daemonEntryPath,
1798+
[
1799+
`import{a as x}from"./dep.js";`,
1800+
`export*from"./re-export.js";`,
1801+
`import"./side-effect.js";`,
1802+
`const importantValue="not-an-import";`,
1803+
`export{x};`,
1804+
].join(''),
1805+
'utf8',
1806+
);
1807+
fs.writeFileSync(depPath, `export const a=1;`, 'utf8');
1808+
fs.writeFileSync(reExportPath, `export const b=1;`, 'utf8');
1809+
fs.writeFileSync(sideEffectPath, `globalThis.sideEffect=1;`, 'utf8');
1810+
1811+
const signature = computeDaemonCodeSignature(daemonEntryPath, root);
1812+
assert.match(signature, /^graph:4:[0-9a-f]{40}$/);
1813+
1814+
// A same-length rewrite can land in the same size+mtime bucket on a fast
1815+
// filesystem (the fingerprint isn't a content hash); pad the size so this
1816+
// assertion isn't racing the clock.
1817+
fs.writeFileSync(depPath, `export const a=200;`, 'utf8');
1818+
const changed = computeDaemonCodeSignature(daemonEntryPath, root);
1819+
assert.notEqual(changed, signature);
1820+
} finally {
1821+
fs.rmSync(root, { recursive: true, force: true });
1822+
}
1823+
});
1824+
1825+
test('computeDaemonCodeSignature ignores a relative-path-shaped string that is not an import', () => {
1826+
// A quoted `./`-looking string that never resolves to a real file (e.g.
1827+
// one embedded in a comment or an unrelated string literal) is a candidate
1828+
// the regex necessarily can't rule out by syntax alone — it must be
1829+
// dropped by resolution instead. This is what keeps the "match any quoted
1830+
// relative-path string" strategy safe.
1831+
const root = mkdtempForTestSync('agent-device-daemon-signature-non-import-');
1832+
try {
1833+
const daemonEntryPath = path.join(root, 'daemon.js');
1834+
fs.writeFileSync(
1835+
daemonEntryPath,
1836+
`// example: \`import x from "./does-not-exist.js"\`\nexport const noop=1;`,
1837+
'utf8',
1838+
);
1839+
1840+
const signature = computeDaemonCodeSignature(daemonEntryPath, root);
1841+
assert.match(signature, /^graph:1:[0-9a-f]{40}$/);
1842+
} finally {
1843+
fs.rmSync(root, { recursive: true, force: true });
1844+
}
1845+
});
1846+
17791847
test('stopDaemonProcessForTakeover terminates a matching daemon process', async (t) => {
17801848
const root = mkdtempForTestSync('agent-device-daemon-test-');
17811849
const daemonDir = path.join(root, 'agent-device', 'dist', 'src', 'internal');

0 commit comments

Comments
 (0)