Skip to content

Commit a0d24bf

Browse files
claude[bot]claude
andauthored
fix(docs-audit): a registration bounds the previous handler window even when its path is a variable (#9571)
Refs #9503 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8d98268 commit a0d24bf

1 file changed

Lines changed: 118 additions & 1 deletion

File tree

scripts/docs-audit/affected-docs.mjs

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,8 +991,16 @@ function parseRegistrarSource(text) {
991991
const lines = maskComments(text).split('\n');
992992
const sites = [];
993993
for (let i = 0; i < lines.length; i++) {
994+
// TWO QUESTIONS, AND THEY ARE NOT THE SAME ONE (#9503). "Does a registration's
995+
// property list start here?" decides the WINDOW BOUNDARY; "what route is it?"
996+
// decides the TAIL. Only the second one needs a literal. A site with no tail was
997+
// always allowed here — 38 of today's 86 literal `path:` lines yield no tail (a
998+
// bare `/api/v1`, a mount prefix) and the loop below already skips them for window
999+
// production while still honouring them as the previous site's `next`. This makes a
1000+
// NON-LITERAL `path:` line behave the same way, which is the whole change.
1001+
if (!/(?:^|[\s{,(])path\s*:/.test(lines[i])) continue;
9941002
const m = lines[i].match(/(?:^|[\s{,(])path\s*:\s*([`'"])(.*?)\1/);
995-
if (m) sites.push({ line: i, tail: routeTailOf(m[2]) });
1003+
sites.push({ line: i, tail: m ? routeTailOf(m[2]) : null });
9961004
}
9971005
const byTail = new Map();
9981006
for (let k = 0; k < sites.length; k++) {
@@ -1524,6 +1532,115 @@ function selfTest() {
15241532
check('bridge', 'a diff touching only the symbol the comment MENTIONS selects no route', 'tails', JSON.stringify([]), JSON.stringify(tailsSelectedBy(new Set(['promoteDraftForPublish']), commentary)));
15251533
check('bridge', 'a diff touching the symbol the handler CALLS still selects the publish route', 'tails', JSON.stringify(['/:type/:name/publish']), JSON.stringify(tailsSelectedBy(new Set(['publishMetaItem']), commentary)));
15261534

1535+
// ---- a registration BOUNDS the previous window even when its path is a variable (#9503) ----
1536+
// The third layer of the same family, and the only one with no measured wrong row on the
1537+
// tree that filed it — read that as the point of the block, not as a reason to skip it.
1538+
//
1539+
// MECHANISM (verified on e7daea169). `rest-server.ts:5661` registers
1540+
// `/:name/state/:field`; the next LITERAL `path:` is 255 lines later at 5916, so the
1541+
// window runs its full 150 lines to 5810 — straight over `path: publishedPath` at 5747,
1542+
// which registers a different route the scan cannot see. 64 lines of the `published`
1543+
// handler sat inside the `state` route's window.
1544+
//
1545+
// MEASURED HARM ON THAT TREE: ZERO identifiers, and the reason is worth writing down.
1546+
// 55 of those 64 lines are the ADR-0033/#8278 commentary, which #9432 masks to blank,
1547+
// and the remaining 9 are `for`/`register`/`method`/`handler`/`try`/`const` boilerplate
1548+
// whose every token already occurs earlier in the same window. Whole-tree before/after:
1549+
// 42 tails → 42, 3320 identifier slots → 3320, zero gained, zero lost. #9432's mask is
1550+
// what is holding this defect down, and it holds it down by ACCIDENT OF COMMENT LENGTH.
1551+
//
1552+
// WHAT IS UNDERNEATH: the same span measured against the foreign handler's own 150 lines
1553+
// carries 18 identifiers the `state` route does not otherwise see — `getMetaItemLayered`,
1554+
// `getPublished`, `resolveProtocol`, `publishedProtocol`, `overlayError`, `resolveExecCtx`
1555+
// … i.e. the `published` route's implementation, including the exact name
1556+
// `rest-route-ledger.ts` binds `/:type/:name/published` to (`meta.getPublished`). Shorten
1557+
// that comment block by a screenful and a `published`-handler diff starts putting the
1558+
// state-machine page on the advisory with a `via` that names a symbol it does implement —
1559+
// for a route it does not. The window's invariant is "this span is ONE route's handler";
1560+
// that invariant is false today and costs nothing today. Both halves are true.
1561+
//
1562+
// ⚠️ PINNED IN BOTH DIRECTIONS, and a third: the foreign handler's symbol must NOT bridge,
1563+
// the site's own symbol MUST still bridge (a boundary rule that over-truncates trades this
1564+
// precision bug for the recall hole the bridge exists to fill), and a `path:` written in a
1565+
// COMMENT must still bound nothing — that last one is new risk this change introduces and
1566+
// #9432 could not have pinned, because before this change a commented `path:` could only
1567+
// mint a phantom TAIL, and now it could also truncate a real window.
1568+
//
1569+
// NOT FIXED HERE, and deliberately: the variable-path route still gets no window of its
1570+
// own, so nothing bridges TO `/:type/:name/published`. Resolving `publishedPath` needs a
1571+
// one-hop binding lookup, and the recall hole it would dent is a small slice of a much
1572+
// larger one — 176 of the 221 client-bound ledger rows have no registrar tail at all
1573+
// today, most of them `plugin-auth` routes that never take a `path:` property. That is a
1574+
// different card with a different measurement; the tails assertion below states the
1575+
// omission as a fact rather than leaving it to be discovered.
1576+
const variablePathRegistrar = [
1577+
'export class RestServer {',
1578+
' private registerStateRoutes() {',
1579+
" for (const objectsSegment of ['objects', 'object']) {",
1580+
' this.routeManager.register({',
1581+
" method: 'GET',",
1582+
' path: `${metaPath}/${objectsSegment}/:name/state/:field`,',
1583+
' handler: async (req, res) => {',
1584+
' // Pre-#4432 this door also registered',
1585+
" // path: '/api/v1/meta/legacy/:name/state',",
1586+
' // and both spellings reach the same primitive.',
1587+
' return this.legalNextStates(req.params);',
1588+
' },',
1589+
' });',
1590+
' }',
1591+
'',
1592+
' // The foreign registration: a real route whose path is a loop variable.',
1593+
' for (const publishedPath of [`${metaPath}/:type/:name/published`]) {',
1594+
' this.routeManager.register({',
1595+
" method: 'GET',",
1596+
' path: publishedPath,',
1597+
' handler: async (req, res) => {',
1598+
' return this.getMetaItemLayered(req.params);',
1599+
' },',
1600+
' });',
1601+
' }',
1602+
' }',
1603+
'}',
1604+
].join('\n');
1605+
const variablePath = parseRegistrarSource(variablePathRegistrar);
1606+
const stateIds = variablePath.get('/:name/state/:field');
1607+
check('parseRegistrarSource', 'the site\'s OWN implementation symbol still lands in its window', 'legalNextStates', true, !!stateIds?.has('legalNextStates'));
1608+
check('parseRegistrarSource', 'a `path:` written in a COMMENT bounds nothing — the boundary rides the same mask the tail does', 'legalNextStates after a commented `path:`', true, !!stateIds?.has('legalNextStates'));
1609+
check('parseRegistrarSource', 'the NEXT route\'s handler symbol is not this route\'s implementation', 'getMetaItemLayered', false, !!stateIds?.has('getMetaItemLayered'));
1610+
check('parseRegistrarSource', 'a variable `path:` bounds a window without claiming a tail — the recall half is untouched, not silently faked', 'tails', JSON.stringify(['/:name/state/:field']), JSON.stringify([...variablePath.keys()]));
1611+
1612+
// The counterfactual: `parseRegistrarSource` verbatim as it stood before this hop —
1613+
// literal `path:` lines are the only sites. Both halves have to be real for the block
1614+
// above to prove anything: the defect must reach the foreign symbol, and it must do so
1615+
// through the boundary and not through the 150-line cap.
1616+
const literalOnlySites = (src) => {
1617+
const ls = maskComments(src).split('\n');
1618+
const sites = [];
1619+
for (let i = 0; i < ls.length; i++) {
1620+
const m = ls[i].match(/(?:^|[\s{,(])path\s*:\s*([`'"])(.*?)\1/);
1621+
if (m) sites.push({ line: i, tail: routeTailOf(m[2]) });
1622+
}
1623+
const byTail = new Map();
1624+
for (let k = 0; k < sites.length; k++) {
1625+
const { line, tail } = sites[k];
1626+
if (!tail) continue;
1627+
const next = k + 1 < sites.length ? sites[k + 1].line : ls.length;
1628+
const end = Math.min(next, line + REGISTRAR_HANDLER_WINDOW, ls.length);
1629+
let ids = byTail.get(tail);
1630+
if (!ids) byTail.set(tail, (ids = new Set()));
1631+
for (let j = line; j < end; j++) for (const id of ls[j].matchAll(/[A-Za-z_$][\w$]*/g)) ids.add(id[0]);
1632+
}
1633+
return byTail;
1634+
};
1635+
const preFix = literalOnlySites(variablePathRegistrar);
1636+
check('parseRegistrarSource', 'counterfactual: the literal-only site scan DID swallow the next route\'s handler whole', 'getMetaItemLayered', true, !!preFix.get('/:name/state/:field')?.has('getMetaItemLayered'));
1637+
check('parseRegistrarSource', 'counterfactual: and the fixture is short enough that the 150-line cap is not what stops it', 'lines under the window', true, variablePathRegistrar.split('\n').length < REGISTRAR_HANDLER_WINDOW);
1638+
1639+
// End to end through the same selection step the bridge runs.
1640+
check('bridge', 'a diff touching only the FOREIGN handler\'s symbol selects no route', 'tails', JSON.stringify([]), JSON.stringify(tailsSelectedBy(new Set(['getMetaItemLayered']), variablePath)));
1641+
check('bridge', 'the pre-fix behaviour, held as the counterfactual: it DID select the state route', 'tails', JSON.stringify(['/:name/state/:field']), JSON.stringify(tailsSelectedBy(new Set(['getMetaItemLayered']), preFix)));
1642+
check('bridge', 'a diff touching the site\'s own symbol still selects its own route', 'tails', JSON.stringify(['/:name/state/:field']), JSON.stringify(tailsSelectedBy(new Set(['legalNextStates']), variablePath)));
1643+
15271644
// ---- the CLI command anchor kind (#9230) ----------------------------------
15281645
// The recall class: a CLI-surface change derives `MetaResync` / a lowercase `resync`,
15291646
// and neither reaches the page that documents the command. The phrase does. Both halves

0 commit comments

Comments
 (0)