scripts-type-check.test.ts 的 workspace-import 检查按文本 grep,注释里的 import 示例会被判成真 import 并给出错误的修复指引

发现于 #3943 的实现(PR 见下),不在该 PR 处理 —— 那一单的面是 sideEffects 一致性门。这一条是另一个门的判定精度问题。

机制

scripts/__tests__/scripts-type-check.test.ts 的最后一条断言(does not need a workspace build, so it can stay in the cheap half of the job)这样判定「scripts 程序是否 import 了 workspace 包」:

const workspaceImports = parsedProject()
  .fileNames.flatMap((f) => [...fs.readFileSync(f, 'utf8').matchAll(/from\s+'(@object-ui\/[^']+)'/g)])
  .map((m) => m[1]);

读的是文件文本,不是 TS AST。于是任何位置只要出现 from + 空白 + 引号包住的 @object-ui/* 字面量就算命中 —— 包括注释、JSDoc @example、以及讲解用的代码片段。\s 匹配换行,所以跨行的散文也能凑出这个形状。

实测

在 scripts/__tests__/check-package-self-import.test.ts 里写下一行注释(内容是在解释「注释里的包名不是 import 边」这件事本身):

// rewrite of this gate breaks on a documented `import … from '@object-ui/core'`

该断言即变红:

- []
+ [ "@object-ui/core" ]
 ❯ scripts/__tests__/scripts-type-check.test.ts:213:7

为什么值得记一笔:失败信息把人指向不存在的东西

红的本身是「安全方向」(宁可误报不可漏报),真正的成本在诊断词:

tsconfig.scripts.json’s program now imports workspace packages, so it needs their built
declaration files. Move the ci.yml step below the build, or drop the import.

而现场根本没有 import,没有 import 可 drop,也没有任何 .d.ts 需求。读到这条的人会去找一条不存在的 import,或者更糟 —— 真的去动 ci.yml 的步骤顺序,把一个廉价步骤挪到 build 之后,而它的前提其实从未被破坏。误报本身廉价,误报 + 错误的修复指引才是这条的内容。

同一个仓里已经有过这个教训的反面:examples/console-starter/vite.config.ts:50 专门写了一段注释说明它故意不在注释里拼出带引号的 specifier,因为「那个诱饵已经骗过一次扫描」。

可能的处置(供分诊,未实施)

按代价从低到高:

只改诊断词 —— 承认判据是文本匹配,让信息同时给出两个方向:「这是真 import → 移动 ci.yml 步骤 / 删掉它;这是注释或示例 → 改写措辞,别在 scripts/ 里拼出带引号的 specifier」。最小、不改判据,消除误导。
剥掉注释再匹配 —— 与 vitest-config-alias-targets-3944.test.ts 的 withoutCommentLines() 同形状(该文件已因同类原因这么做)。能挡掉整行注释,挡不掉行尾注释与跨行散文。
改用 AST —— parsedProject() 已经产出 fileNames,拿 ts.createSourceFile 遍历 ImportDeclaration/ExportDeclaration 的 moduleSpecifier 即精确判定,顺带覆盖 import type 与 export … from。判据从「像不像」变成「是不是」。

倾向 3(判据与被判定的事实对齐),但 1 单独就能消掉误导,且可独立落地。

退出条件

上述任一处置落地(诊断词至少要不再指向不存在的 import);或
判据改为 AST 后本条自动关闭。

参考位置:scripts/__tests__/scripts-type-check.test.ts:205-214、scripts/__tests__/vitest-config-alias-targets-3944.test.ts(withoutCommentLines() 先例)、examples/console-starter/vite.config.ts:50(同类诱饵的现场注释)。

第二实例:`column-identity.ratchet.test.ts` 同样把注释当代码

本条的盲区不是单个门的判定精度问题,而是文本级门家族的共性 —— 今天在 #3951(PR #4909)上又撞了一次,机制完全同形,只是换了一个门。

现场

PR #4909 把 grid 列的键收敛到声明拼写 name(修 reader,不加容忍别名)。为了让下一个读代码的人别把别名加回来,我在 packages/fields/src/widgets/GridField.tsx 的 GridColumn.name 上写了一句注释,内容是声明「这里故意没有这个别名」。CI 20 个 job 里 19 绿,唯一红的是 Test (shard 2/4):

column-identity.ratchet.test.ts  >  finds no dual read in a file the inventory does not list
+ "fields/src/widgets/GridField.tsx:51 :: * There is deliberately NO `col.field ?? col.name` alias: one spelling, at"

也就是说:一句声明「不存在双读」的注释,被判成了新增双读。 棘轮读数 11 → 12。行为面零缺陷 —— 该文件里没有任何一处 alternation 读列身份,整个 PR 做的恰恰是把双拼写收敛成单拼写。

与本卡的同形之处

column-identity.ratchet.test.ts:194-215 的判据是逐行文本:

src.split('\n').forEach((line, i) => {
  if (!ALTERNATION.test(line)) return;              // 行内有 || 或 ??
  const keys = [...line.matchAll(ACCESS)];          // 行内的 .field / .name / .fieldName 等属性访问
  if (distinct.size < 2) return;                    // 两个以上不同身份键 → 记为一处双读

collectSourceFiles() 已经排除了 node_modules / dist / __tests__ / *.test.tsx / *.d.ts,但没有剥注释,所以判据对「这一行是代码还是散文」完全不设防 —— 与本卡 scripts-type-check.test.ts 用 /from\s+'(@object-ui\/[^']+)'/ 扫文件文本是同一件事。

值得注意的是,这个门自己知道判据是启发式的(文件头写着「The scanner is a line-level heuristic, not a parser」),也正因如此它带了 verdict / why 的 inventory 来做人工分诊。但盲区在于:被误判的一方如果是注释,inventory 机制反而会把人引向错误修法 —— 把 GridField.tsx 加进 inventory 列表,等于在一份「双读清单」上登记一处根本不存在的双读,污染的正是这份清单的可信度。本卡记的「误报 + 错误的修复指引」在这里的具体形态是:正确修法(改注释措辞)和错误修法(加白名单)对读到失败信息的人来说难以区分,而错误修法留下的痕迹是永久的。

处置(#4909 内)

按 PM 指示只改措辞,不动 inventory:把该句改写成不含 alternation 形状的散文(「no tolerant alias bridging the retired spelling to this one」),并在注释里留了一行说明「不要把这条规则重新写成两个键名之间的 alternation 表达式,本仓的棘轮逐行扫描且分不清散文与代码」。本地复跑读数回到基线 11。

对本 finding 作用域的影响

原卡的三档处置(改诊断词 / 剥注释 / 换 AST)是针对 scripts-type-check.test.ts 一个门写的。有了第二实例,建议把作用域从「修这个门」提升为「盘点文本级门家族」:

本仓已知同族:scripts/__tests__/scripts-type-check.test.ts(本卡)、packages/core/src/utils/__tests__/column-identity.ratchet.test.ts(本评论)。
已知做对了的先例有两处,可作为处置模板:scripts/__tests__/vitest-config-alias-targets-3944.test.ts 的 withoutCommentLines(),以及 examples/console-starter/vite.config.ts:50 那段「故意不在注释里拼出诱饵」的现场说明。
两个实例都指向同一条结论:凡是「禁止某个代码形状」的文本级门,都会把讲解该形状的注释判成违例 —— 而讲解它的注释恰恰最可能出现在刚刚修掉它的那个 PR 里(fix(fields): grid columns 收敛到声明拼写 name,spec 合规元数据不再渲染空单元格 (#3951) #4909 就是这样:修掉双读的那次提交,因为写下「这里没有双读」而被双读门拦住)。这使得该家族的误报系统性地集中在正确修法上,不是随机噪声。

因此建议:处置 2/3 至少要覆盖这两个门;若采用 3(AST),column-identity.ratchet.test.ts 因为要判的是表达式形状而非 import 边,成本高于本卡的 import 判定,处置 2(先剥注释行)对它的性价比更高。

Generated by Claude Code

Triage (concentrated finding round): promoted finding → pm:queue, with the widened scope the 2026-08-17 comment established: this is the text-level gate family, not one gate. Scope = both known members (scripts-type-check.test.ts workspace-import check; column-identity.ratchet.test.ts), using the two in-repo precedents as templates. Route suggestion per the evidence on this thread: AST for the import check (disposition 3 — parsedProject() already yields the file list), comment-line stripping for the column-identity ratchet (disposition 2 — expression-shape AST is poor value there), and in both cases fix the failure message so the wrong repair (whitelist/inventory pollution) stops being indistinguishable from the right one.

本评论来自分诊座位 Routine (session session_018euHiF12oV8iSfYdjVioUF).

Generated by Claude Code

Claim: PM loop round 8
Session: session_019b5UBNMtTzKbVtZZGvFuxe
Branch: claude/issue-4902-workspace-import-grep-comments
Worktree: objectui-issue-4902
Domain: domain:devx (@ objectui, seat #5748)
File surface: scripts/__tests__/scripts-type-check.test.ts
Container & model: S, mode:subagent, model: sonnet
Clause-②: no
Serial constraints cleared: in-flight 0 at round open. Co-dispatched: #5174 final batch, #5391 (scripts/vite-objectstack-spec-dist.ts), #5007 (apps/console/src/__tests__/), #5306 (root package.json) — all disjoint.

⚠️ This file changed today. #4926 landed at 10:34Z (PR #5974) and added a new describe block to this exact file — the option-parity assertion against apps/console/tsconfig.node.json. Base off a main containing d8b48f495 and ⛔ do not work from a stale copy; the file you are editing is 100+ lines longer than it was this morning.

The card

The workspace-import check in this file matches by text grep, so an import appearing inside a comment is judged a real import and produces a wrong fix instruction. A false positive that ships a confidently wrong remediation is worse than no check.

Zone 2 — measure first, and mind the direction

⛔ I have not verified a live false positive exists today. Find one, or construct the minimal case that triggers it, before changing the matcher. If no comment in scripts/ currently trips it, this is latent — say so plainly rather than implying a live break, and ⛔ do not manufacture a permanent artificial case to justify the change. (Batch-3's dev handled the same situation correctly with an uncommitted scratch repro; that is the pattern.)
⚠️ Getting stricter must not make it blind. The failure mode you are fixing is over-matching; the failure mode you can easily introduce is under-matching, which is strictly worse — this check exists to catch a real workspace-import violation, and a matcher that skips too much passes vacuously. Pin both directions: a real import in code is still caught, an import inside a // and a /* */ comment is not.
Consider whether a text matcher is the right instrument at all. This file already imports typescript and parses configs; if the honest fix is to read import declarations from the AST rather than patch the regex, that is in scope — but keep it to this check, ⛔ do not restructure the file.

⚠️ Vacuity risk — this file's own convention

The sibling guard scripts/__tests__/vitest-config-alias-targets-3944.test.ts warns throughout that "a zero-hit parse would make every case below vacuous", and #4926 landed an explicit anti-vacuity assertion in this file yesterday. Hold to that standard: whatever you change, assert the matcher still finds something known-present, so a fix that silently matches nothing cannot pass.

Gates

Derive from your diff. At minimum pnpm type-check:scripts, scripts/__tests__ (all 63 files — #5984 added one today), and Lint. Run check-changeset-presence.mjs and follow its verdict; ⛔ no skip-changeset label (#4912/#3724).

⚠️ Restore idiom (objectstack#11648): git checkout <ref> -- <path> writes the index, so a trap restoring with bare git checkout -- <path> reads back the mutation and exits 0. Use git checkout HEAD -- <path>; prove it with git diff HEAD.

Generated by Claude Code

{
  "issue": 4902,
  "status": "done",
  "branch": "claude/issue-4902-workspace-import-grep-comments",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5988",
  "premise_still_valid": true,
  "summary": "Replaced the text-grep regex in scripts-type-check.test.ts's workspace-import check with an AST walk (ts.createSourceFile, walking ImportDeclaration/ExportDeclaration/ImportEqualsDeclaration for real moduleSpecifier text). A comment or JSDoc example mentioning @object-ui/* no longer trips the check; a real import/import type/export-from still does. Added a dedicated describe block pinning both directions plus two non-vacuity assertions that the extractor can find a real edge before trusting the not-caught cases.",
  "tests": "False positive confirmed LATENT, not live: grep -rn \"from '@object-ui\" scripts/ --include=\"*.ts\" returned zero hits, and running the exact old regex against every scripts/ .ts file via a one-off node script also returned zero hits. Reproduced with an uncommitted scratch line (the issue's own repro) appended to scripts/__tests__/check-package-self-import.test.ts: npx vitest run scripts/__tests__/scripts-type-check.test.ts went red (1 failed | 8 passed) on the old matcher; restored with git checkout HEAD -- scripts/__tests__/check-package-self-import.test.ts, proved clean via git diff HEAD (empty); re-applied the same scratch line and confirmed 15 passed | 0 failed on the new matcher; reverted again (git diff HEAD empty). Gates at commit be44950f3: pnpm type-check:scripts exit 0; npx vitest run scripts/__tests__ --maxWorkers=2 -> 'Test Files 62 passed (62)', 'Tests 1674 passed (1674)'; npx eslint scripts/__tests__/scripts-type-check.test.ts --no-inline-config exit 0, no output; node scripts/check-changeset-presence.mjs -> 'No source of a released package changed in this range, so no changeset is owed.' (test file under scripts/, not a released package's src/).",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

os-dev-report

(previous comment's HTML marker was stripped by the sanitizer — reposting with the marker as literal text per instructions)

{
  "issue": 4902,
  "status": "done",
  "branch": "claude/issue-4902-workspace-import-grep-comments",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5988",
  "premise_still_valid": true,
  "summary": "Replaced the text-grep regex in scripts-type-check.test.ts's workspace-import check with an AST walk (ts.createSourceFile, walking ImportDeclaration/ExportDeclaration/ImportEqualsDeclaration for real moduleSpecifier text). A comment or JSDoc example mentioning @object-ui/* no longer trips the check; a real import/import type/export-from still does. Added a dedicated describe block pinning both directions plus two non-vacuity assertions that the extractor can find a real edge before trusting the not-caught cases.",
  "tests": "False positive confirmed LATENT, not live: grep -rn \"from '@object-ui\" scripts/ --include=\"*.ts\" returned zero hits, and running the exact old regex against every scripts/ .ts file via a one-off node script also returned zero hits. Reproduced with an uncommitted scratch line (the issue's own repro) appended to scripts/__tests__/check-package-self-import.test.ts: npx vitest run scripts/__tests__/scripts-type-check.test.ts went red (1 failed | 8 passed) on the old matcher; restored with git checkout HEAD -- scripts/__tests__/check-package-self-import.test.ts, proved clean via git diff HEAD (empty); re-applied the same scratch line and confirmed 15 passed | 0 failed on the new matcher; reverted again (git diff HEAD empty). Gates at commit be44950f3: pnpm type-check:scripts exit 0; npx vitest run scripts/__tests__ --maxWorkers=2 -> 'Test Files 62 passed (62)', 'Tests 1674 passed (1674)'; npx eslint scripts/__tests__/scripts-type-check.test.ts --no-inline-config exit 0, no output; node scripts/check-changeset-presence.mjs -> 'No source of a released package changed in this range, so no changeset is owed.' (test file under scripts/, not a released package's src/).",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

os-dev-report

Patch round per PM review of PR #5988: the docblock claim "the second not at all" (re: export { … } from '…') was factually wrong. Measured the exact old regex (/from\s+'(@object-ui\/[^']+)'/g) against all four import/export forms plus the comment case — it matched static import, import type, export … from, export * from, and the line-comment false positive; it missed only import core = require('@object-ui/core') (ImportEqualsDeclaration, no from token). Corrected the docblock sentence to state that measurement. No behavior change — code, the three found-cases, the three not-found cases, and the non-vacuity ordering are unchanged. Ran only the diff-touched checks per PM instruction (no full gate re-run): pnpm type-check:scripts exit 0; the file's own test (scripts/__tests__/scripts-type-check.test.ts) 15 passed | 0 failed; npx eslint scripts/__tests__/scripts-type-check.test.ts --no-inline-config exit 0. Pushed as commit 3c693b384.

{
  "issue": 4902,
  "status": "done",
  "branch": "claude/issue-4902-workspace-import-grep-comments",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5988",
  "premise_still_valid": true,
  "summary": "Amended PR #5988 per PM review: corrected a factually wrong docblock claim in workspaceImportSpecifiers (it said the old regex missed export-from imports entirely; measurement shows it matched them incidentally, same as import type and static imports, via any 'from ...' token sequence). The regex's real, sole blind spot was import = require() (ImportEqualsDeclaration, no from token), which is what the new AST walk actually adds coverage for. No code or test behavior changed.",
  "tests": "Measured the exact old regex against six cases (static import, import type, export...from, export * from, import=require(), line comment) via a one-off node script: matched all but import=require(). Re-ran only diff-touched checks per PM instruction: pnpm type-check:scripts exit 0; npx vitest run scripts/__tests__/scripts-type-check.test.ts -> 15 passed | 0 failed; npx eslint scripts/__tests__/scripts-type-check.test.ts --no-inline-config exit 0. Amended commit: 3c693b384.",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

ACCEPT — PR #5988 after one patch round. Landing on CI convergence.

Reviewer of record: domain:devx @ objectui seat (#5748), round R8, PM session session_019b5UBNMtTzKbVtZZGvFuxe. Verified against the branch's own file — ⛔ not against the report.

The patch round, and why it was worth one

The first push's code was correct and accepted. What sent it back was a docblock claim that was factually wrong: it said the old regex covered export { … } from '…' "not at all". Measured against the actual old pattern /from\s+'(@object-ui\/[^']+)'/g:

static import:         MATCHED       import type:        MATCHED
export ... from:       MATCHED  ←    export * from:      MATCHED
import = require():    not matched  ←  the real blind spot
line comment:          MATCHED  ←    the false positive being fixed

Amended text now reads correctly — "The regex matched any from '…' token sequence, so it already caught import type { … } and export { … } from '…' incidentally, along with static imports — it did not understand statement kinds, only that shape. What it missed entirely was import core = require('…') (ImportEqualsDeclaration), which has no from token at all." Verified on the branch at 3c693b384.

⚠️ Worth stating why a comment earned a round trip. This lane has now filed several cards whose entire content is a comment confidently asserting something untrue — the CodeQL badge (#5408), tsconfig.scripts.json's parity claim (#4926), the parity gate's stale citation (#5007). Landing a fresh one inside the very file being fixed for a precision defect would have been the same defect, one layer up, delivered by the fix for it. A future reader would have taken it as measured — because it sits in a docblock full of measurements.

The dev re-derived the measurement independently rather than taking my word, which is the right response to a review claim.

The substance, accepted from the first push

AST walk over ImportDeclaration / ExportDeclaration / ImportEqualsDeclaration, replacing the text grep. The check now asks "is this a module edge" rather than "does this text look like one".
Both directions pinned: three found-cases (static, import type, export … from), three not-found (line comment, block-comment @example, a plain string containing the specifier).
⚠️ Non-vacuity ordering, unprompted and correct. The found-cases come first, with the reason written out: "A matcher that always returns [] would make every 'not caught' case beneath it pass for the wrong reason." That is the exact hazard the dispatch warned about — under-matching being strictly worse than over-matching — and it is guarded structurally rather than by intent.
Latent, not live — stated plainly. Zero from '@object-ui hits across scripts/, confirmed by running the old regex over every file. Reproduced with an uncommitted scratch line (red on the old matcher, green on the new), then restored with git checkout HEAD -- <path> and proved with an empty git diff HEAD — the safe idiom from objectstack#11648, applied on its first outing.

Landing

Path fork checked against the actual file list: scripts/__tests__/** is not governed surface. Maintainer's ruling, in-session 2026-08-24, verbatim: 「授权 auto-merge（推荐）」 — every check green, ⛔ not the required subset. ⛔ Only diff-touched checks were re-run for the amendment, as instructed; CI runs the full farm.

Generated by Claude Code

Navigation Menu

Repository navigation

scripts-type-check.test.ts 的 workspace-import 检查按文本 grep,注释里的 import 示例会被判成真 import 并给出错误的修复指引 #4902

机制

实测

为什么值得记一笔:失败信息把人指向不存在的东西

可能的处置(供分诊,未实施)

退出条件

第二实例:`column-identity.ratchet.test.ts` 同样把注释当代码

现场

与本卡的同形之处

处置(#4909 内)

对本 finding 作用域的影响

The card

Zone 2 — measure first, and mind the direction

⚠️ Vacuity risk — this file's own convention

Gates

ACCEPT — PR #5988 after one patch round. Landing on CI convergence.

The patch round, and why it was worth one

The substance, accepted from the first push

Landing

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

scripts-type-check.test.ts 的 workspace-import 检查按文本 grep,注释里的 import 示例会被判成真 import 并给出错误的修复指引 #4902

Description

机制

实测

为什么值得记一笔:失败信息把人指向不存在的东西

可能的处置(供分诊,未实施)

退出条件

Activity

yinlianghui commented on Aug 17, 2026

第二实例:column-identity.ratchet.test.ts 同样把注释当代码

现场

与本卡的同形之处

处置(#4909 内)

对本 finding 作用域的影响

os-support-ai commented on Aug 19, 2026

yinlianghui-tw commented on Aug 24, 2026

The card

Zone 2 — measure first, and mind the direction

⚠️ Vacuity risk — this file's own convention

Gates

yinlianghui-tw commented on Aug 24, 2026

yinlianghui-tw commented on Aug 24, 2026

yinlianghui-tw commented on Aug 24, 2026

yinlianghui-tw commented on Aug 24, 2026

ACCEPT — PR #5988 after one patch round. Landing on CI convergence.

The patch round, and why it was worth one

The substance, accepted from the first push

Landing

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

第二实例:`column-identity.ratchet.test.ts` 同样把注释当代码