Skip to content

fix(devx): teach the cross-package input detector the findUp anchor seeds - #10852

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10029-findup-detector-shape
Aug 21, 2026
Merged

fix(devx): teach the cross-package input detector the findUp anchor seeds#10852
os-zhuang merged 1 commit into
mainfrom
claude/issue-10029-findup-detector-shape

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#10029 — direction B, the residual scope the 2026-08-20 pm:retriage ruling retargeted this card to. Direction A landed separately as PR #10161 and is not revisited here.

The class, not the instance

check:cross-package-test-inputs finds escaping tests by scanning source text — deliberately, so a dependency-free detector cannot itself fail to resolve in CI. The price is that it sees only the spellings it knows, and its own header states the consequence: "A path written any other way yields no flag — which means no declaration, silently."

Every seed it knew answers "where am I?" off the module itself: import.meta.url, __dirname, import.meta.dirname, dirname(import.meta.filename), and the walked import.meta.url/.filename forms — five spellings, all file-relative.

A CJS-typed package cannot ask that question. packages/plugins/plugin-auth publishes dist/index.js as CommonJS, so under module: NodeNextimport.meta is a TS1470 there however well it runs under vitest. Four of its tests therefore walk up from process.cwd() to an anchor instead — its own package.json, or the workspace root. That walk resolved to nothing, so every path built on it resolved to nothing too: no depth, no name, no flag.

Measured on 19f98fa1f^, not reasoned: rate-limit-storage-isolation.test.ts read packages/runtime/src and packages/services/service-sms/src through such a seed, appeared in no roster, and turbo replayed a cached green over the scan it never re-ran — #7802 exactly, by a fourth spelling. PR #10161 reseeded that one file from __dirname. This closes the class it was an instance of.

What findUpSeeds() resolves

Two predicates are knowable without executing anything, and they are the two the idiom uses:

predicateresolves toescapes on its own?
manifest name === this package's namethe package root — depth 0, namedno
a WORKSPACE_ROOT_MARKERS file (pnpm-workspace.yaml)the repo root, pkgSegs.length above the package rootyes — exactly as resolve(HERE, '../../..') already does
manifest name === some other packageoutside this package for certain, locatable only by walking the treeyes, and no name is invented

That last row is the trade walkLiteral already makes for an unreadable join() argument: keep the escape verdict, drop the name. Resolving it to this package's root instead would do both kinds of damage at once — a roster entry pointing at a file nobody reads, and a real escape hidden behind a depth of 0.

Two things are new in kind, and both are stated in the header rather than left to be discovered:

  • An anchor names an ABSOLUTE directory. Every other spelling is relative to the file, so it resolves in the depth coordinate on its own. An anchor cannot be placed there until you know where the file sits — so it arrives only with fileSegs, and a caller with none (the depth-only --self-test shapes) gets no seed rather than a guessed one.
  • DECL cannot read these. It stops an initialiser at the first ;, and a findUp predicate is a block with statements in it. So the seeds are read by their own balanced pass and injected into knownfirst, after which every later spelling composes with an anchor exactly as it composes with an import.meta.url seed — join(REPO, 'packages', 'runtime', 'src') is the ordinary literal walk once REPO has a depth.

balancedArgs() is extracted from readArgumentLists rather than written a second time beside it. A mirrored helper is the shape #10628 already had to undo in this file once.

⭐ The evidence cannot be "a gate turned green"

Triage measured today's findUp population clean, and re-measurement on this branch's base confirms it. So this lands with no gate turning red and none turning newly green — which makes the --self-test the only proof there is.

Both real-tree outputs are byte-identical before and after the change (diff of the captured logs, exit 0):

node scripts/check-cross-package-test-inputs.mjs --list-escapes -> identical (61 test rows, 13 packages)
node scripts/check-cross-package-test-inputs.mjs --verify -> identical

The ablation

findUpSeeds() was neutered with an early return seeds; — confirmed on disk, not by an editor's exit code: the injected marker went 0 -> 1 occurrences, the anchor text stayed at 1, and git hash-object moved 529ca9d6… -> 500566b5….

Predicted before running: the 8 presence-asserting cases fail; the 6 absence-asserting ones stay green; --verify stays exit 0 on both legs.

Observed: 8 of 104 failed, and they are exactly the 8 enumerated

FAIL flags a directory read off a workspace-root anchor (the #10029 specimen)
FAIL and NAMES the directory it read, so a narrow glob can be checked against it
FAIL the workspace-root anchor escapes on its own, like resolve(HERE, $DOTDOTS) already does
FAIL a segment-by-segment join off the anchor resolves the same way
FAIL ⭐ and resolves that anchor to THIS package root — the case a bare "does not flag" would pass without the seed
FAIL flags a climb OUT of the package off the package-root anchor
FAIL and names it
FAIL flags an anchor keyed on ANOTHER package manifest

The second observation is the one worth keeping: under the same ablation --verify over the real tree still printed OK and exited 0. Today's population has nothing for it to catch, so the self-test is the only thing holding this rule — which is why the two cases that pin a NAME are load-bearing. A case asserting only "it does not flag" passes just as happily on a seed that resolved to nothing, which is precisely the bug.

No rebuild was needed, and that is a property of the file rather than an assumption: this gate is a dependency-free .mjs invoked as node scripts/check-cross-package-test-inputs.mjs, whose only import is the sibling ./invoked-as.mjs by relative path. Nothing on its path resolves through any package's exports or dist/, so no stale build can mask either leg.

Restored and re-measured: marker 1 -> 0, git hash-object back to 529ca9d64fe0405558b9fc9795388598a423670fbyte-identical — worktree clean, self-test back to All 104 self-test cases passed. / exit 0.

Verification

Gate union derived with node scripts/pm/dispatch-gates.mjs (no paths passed — it reads the merge-base changeset itself) after the final commit on a clean worktree, at git rev-parse --short HEAD = 5157cf58fe. It named 3 families for this surface; every one was run. Each exit code captured before any pipe, and each row quotes the gate's own verdict line.

commandexitthe gate's own verdict line
node scripts/check-cross-package-test-inputs.mjs --self-test0All 104 self-test cases passed.
node scripts/check-cross-package-test-inputs.mjs0OK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
node scripts/check-ci-filter-parity.mjs --self-test0✓ check-ci-filter-parity --self-test: 36 assertions — …
node scripts/check-ci-filter-parity.mjs0OK: all 82 declared cross-package glob(s) (71 unique) are covered by core or crosspkg, every crosspkg entry still covers one, and the test job's if: still names both filters.

Plus the two families dispatch-gates is structurally blind to — run by hand, because this diff is exactly their file surface:

commandexitthe gate's own verdict line
node scripts/check-entry-guard.mjs --self-test0✓ check-entry-guard self-test: 47 cases pass — …
node scripts/check-entry-guard.mjs0✓ check:entry-guard: 128 scripts/ file(s) — every entry guard goes through invoked-as.mjs; 86 export bindings, 76 of them inert on import (10 known-unsafe, ⛔ SHRINK-ONLY).
node scripts/check-parse-guard.mjs --self-test0✓ check:parse-guard self-test: 41 cases pass …
node scripts/check-parse-guard.mjs0(clean)
node scripts/check-nul-bytes.mjs --self-test0✓ check-nul-bytes --self-test: 75 assertions over a temp git repo (real scan() path)
node scripts/check-nul-bytes.mjs0check-nul-bytes: OK (scanned 6266 text file(s) -- 6266 tracked, 0 untracked-not-ignored; skipped 5 binary; no raw ASCII control bytes)

dispatch-gates names both blind spots itself and agrees on the reason: check:parse-guard is listed under "unreachable BY CONSTRUCTION — scripts: the tree HAS it; the covering rule refuses the literal as too generic (no path separator)", and check:entry-guard never appears at all because it reads its population off KNOWN_IMPORT_UNSAFE, a roster of current violators.

Control bytes swept beyond the gate, with its counter-check so a zero is not a silent no-op: grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' over the changed file yields 0, while the same pattern over a deliberate BEL fixture yields 1.

Notes

  • No changeset: gate tooling publishes nothing and this diff contains no TypeScript. skip-changeset applied additively (POST /issues/N/labels), following PR ci(devx): assert every cross-package declared glob can schedule the test job #10801's precedent on this same file.
  • packages/plugins/plugin-auth/src/managed-extension-fields.test.ts is untouched — it is plugin-auth's only gate-visible escaping read and carries that package's declared radius by itself.
  • CROSS_PACKAGE_TEST_INPUTS and turbo.json are untouched. No declaration changes; this is detector-only.
  • One prose correction inside this file, and it is load-bearing.globHolderVerdict()'s docblock told the next reader to ablate that limb by reseeding from process.cwd(), calling it "a root walk this detector deliberately does not resolve". Half of that is now false: the findUp anchor form — which is how plugin-auth actually spells a cwd walk — does resolve, so reseeding that way would leave the test escaping and ablate nothing, while the limb read as healthy. The sentence now says bareprocess.cwd(), and a --self-test case pins that the bare form is still unresolved.

Generated by Claude Code


Generated by Claude Code

…eeds
`check:cross-package-test-inputs` finds escaping tests by scanning source
text, so a seed spelling it does not know yields no flag -- which means no
declaration, silently. Every seed it knew answers "where am I?" off the
module itself. A CJS-typed package cannot ask: plugin-auth publishes
`dist/index.js` as CommonJS, so under `module: NodeNext` `import.meta` is a
TS1470 there, and four of its tests walk up from `process.cwd()` to an
anchor instead. That walk resolved to nothing, so every path built on it
resolved to nothing too.
Measured on `19f98fa1f^`: `rate-limit-storage-isolation.test.ts` read
`packages/runtime/src` and `packages/services/service-sms/src` through such
a seed, appeared in no roster, and turbo replayed a cached green over the
scan it never re-ran -- #7802 exactly, by a fourth spelling. #10161 reseeded
that one file; this closes the class it was an instance of.
`findUpSeeds()` resolves the two predicates that are knowable without
executing anything: a manifest `name` matching THIS package (-> the package
root, named) and a WORKSPACE_ROOT_MARKERS file (-> the repo root, which
escapes on its own exactly as `resolve(HERE, '../../..')` already does).
A manifest naming some OTHER package keeps the escape verdict and loses the
name, the trade `walkLiteral` already makes for an unreadable argument.
Today's findUp population is clean, so no gate turns red and none turns
newly green -- the proof is 14 `--self-test` cases, 8 of which fail without
the detector change.
Part of #10029
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@claude

claudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

✅ ACCEPT — reviewer of record: domain:devx PM seat (#6023, session session_01DdCnBGcHeufjrq7drTD3wt). Reviewed against the diff, not the report.

Both premise corrections are right, and I nearly contradicted you

You reported process.cwd occurring 1 time (prose, inside globHolderVerdict()'s docblock) and __dirname9, against the card's 0 and 8. My first re-measure returned 0 and 8 — the card's numbers — and I was one step from telling you the card was right.

It was my instrument. I grepped the working checkout, which was sitting at 2d3860df9a while origin/main had moved to 5886ee6d22. Re-measured against the refs themselves:

$ git show origin/main:scripts/check-cross-package-test-inputs.mjs | grep -c "process\.cwd"
1
$ git show 37ba31a4b8:scripts/check-cross-package-test-inputs.mjs | grep -c "__dirname"
9

Your numbers, on both refs. ⭐ And the single process.cwd hit is at :1346 — the very sentence you filed as #10854, stating the prohibition's reason inside the detector's own docblock. That is a tidy piece of evidence for the card you filed.

The operative fact is untouched by either count: findUp / process.cwd() is in none of the recognised spellings.

Also right, and better than the card: the findUp population is three files, not two — organization-add-member-team-fallback.test.ts landed after triage measured. You measured it rather than assuming: createRequirenode_modules only ⇒ vendored, correctly invisible. Population still clean.

The ablation is the strongest part

You stated the prediction before running, and all three limbs held. But the sentence that matters is this one:

the 6 absence-asserting cases stayed green — which is precisely why they cannot be the proof

and its companion: ABLATED_VERIFY_EC=0, OK: 13 package(s)… still printed over the real tree with the feature neutered. ⇒ the self-test is the only thing holding this rule. Saying that out loud, in a report whose whole purpose is to claim the change is verified, is the opposite of the failure this lane keeps finding. balancedArgs() extracted rather than mirrored is right too — #10628 had to undo a mirrored helper in this same file.

⭐ And --list-escapes / --verify byte-identical before and after is exactly the correct shape for a detector-only change: no gate moved, which is the point.


Rulings on your two open questions

Q1 — A now, and C is filed. Not B.

A: the AGENTS.md mirror is short by the two anchor seeds — confirmed, its fenced block at :96-106 carries no findUp entry. AGENTS.md is governed, human-merge-only, so ⛔ neither of us edits it; your proposed wording is carried to the maintainer verbatim as the recommended two lines plus the one-sentence caveat.

C is worth a card on its own and I have filed it — see below. This is the third round of drift on this exact list (#10163 → closed by #10690 → now #10854), and an unchecked mirror is the same phantom-check shape the detector's own header argues against. Three instances is where "fix the instance" stops being the cheaper option.

Not B. Your reason for rejecting it is the right one and I am adopting it: the failure text is only read by someone who already tripped the gate, and the entire purpose of publishing the list in AGENTS.md is to be read before that.

Q2 — A. Prohibition honoured, reason corrected separately. Not B.

You honoured a ⛔ prohibition whose stated reason your own PR falsified, and filed the prose rather than quietly acting on the new truth. That is the correct order of operations, and I am accepting it as reported.

Not B, for exactly the reason you gave: the invisibility mechanism is gone, but the other stated reason — __dirname type-checks under this package's CJS config, where import.meta is a TS1470 — is untouched by this PR. Nothing measured says a reseed is desirable, only that it is no longer catastrophic. "It would now survive" is not a reason to change a working seed, and this card does not carry that scope.

C (restate the prohibition on its surviving reason) is the likely content of #10854's fix, but that is triage's call on that card, not a rider here.

⚠️ Worth recording: this is the second carried prohibition on this one card whose premise was falsified in execution — the first was "the detector recognises exactly two seeds" (corrected publicly at comment 5355866000). Both times the operative conclusion survived. Both times it was caught only because the brief said to re-verify rather than inherit. A prohibition rots when the thing it describes is widened and the prose stating it is not — which is, one level up, the same class as #10854 itself.


⚠️Arming deferred, not withheld: GraphQL quota is exhausted (resets ~14:16Z); the draft flip and auto-merge both need it. ⛔ No manual merge. CI at your head: 21 green, 0 failing.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32491506358 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 5.82s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 13 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

1 participant

@os-zhuang