Skip to content

test(cli): mask comments before scanning serve.ts in two contract/pin scans - #11002

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10514-serve-scan-comment-mask
Aug 22, 2026
Merged

test(cli): mask comments before scanning serve.ts in two contract/pin scans#11002
os-elon merged 1 commit into
mainfrom
claude/issue-10514-serve-scan-comment-mask

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10514

What changed

Both packages/cli scans that read serve.ts with no comment/code separator now scan maskComments(SERVE_SOURCE) (scripts/js-comment-mask.mjs) instead of raw text:

  • serve-email-config-parity.contract.test.tskeysReadFromConfigEmail() now matches cfgEmail\.\w+ over masked source.
  • serve-multi-node-cap-advisory.pin.test.ts — the four checkMultiNodeAllowed/MultiNodeGateVerdict shape assertions now match over masked source. interfaceFields()'s own narrower, brace-matched strip (used only for the producer/consumer verdict-shape comparison) is untouched — the issue calls this out explicitly as a different, narrower mechanism and out of scope here.

FIRST ACTION — re-verification duty (release condition on this card)

Re-measured on origin/main at branch point 047ac86ee9 (before touching either scan), per the triage comment's explicit condition:

  • Subject 1 (email keys): raw / naive-two-regex-stripped / masked key sets for cfgEmail.* are identical: ["apiKey","appName","defaultFrom","defaultTemplateContext","options","persist","provider","queueDelivery","retries"] in all three.
  • Subject 2 (multi-node shape): all four regex match verdicts (not.toMatch zero-arg, toMatch non-empty-arg, toMatch replicas call, toMatch typed dynamic import) are identical raw vs. masked — all false/true/true/true respectively.

No verdict moved, despite serve.ts having changed materially since the card was written (#10919 module-scope importFromHost, #10948Serve.importConfigPlugin, #10956 capability-loop comment rewrite). This was confirmed two independent ways: a standalone script against the raw file text, and — after landing the fix — the full test suite with the production routing lines reverted back to raw (13/13 tests green, see "Reverse-verification" below).

Fresh answer to the open question #10909's dev never returned

Yes — pre-fix, editing comment prose in serve.ts could move both scans' results.#10956 merging green is not evidence either way (nobody added the adversarial comment); established here directly via the vacuity-proof tests (see below), which demonstrate on synthetic sources shaped exactly like the real resolver/call site that a comment containing the pattern each scan keys on flips the verdict when read raw. Post-fix, it cannot — masking blanks the comment before either scan runs.

CROSS_PACKAGE_TEST_INPUTS registration — already present

Confirmed live: scripts/js-comment-mask.mjs and scripts/js-comment-mask.d.mts are already in CROSS_PACKAGE_TEST_INPUTS['@objectstack/cli'] (scripts/check-cross-package-test-inputs.mjs) and in turbo.json's @objectstack/cli#test inputs — landed with #10453/PR #10513. No registration work was needed; this PR is the clean two-site maskComments routing the issue predicted for that case.

Vacuity proof — the hardening actually hardens

Three new tests, each showing the RAW leg produce the wrong verdict (what these scans would have done pre-#10514) next to the MASKED leg producing the right one, on synthetic sources rather than waiting for serve.ts to regress:

  • serve-email-config-parity.contract.test.ts — a comment naming an undeclared key (cfgEmail.bogusKey) is read as a real key raw, ignored masked ("declares every key" direction).
  • Same file — a comment naming a declared-but-unread key (cfgEmail.persist) is read as a real read raw, correctly unread masked — the worse direction the issue calls out: this is the exact spec/cli: EmailServiceConfig.persist 声明了但没有载体 —— config.email.persist 永远到不了 EmailServicePlugin(#5307 的反向面,ADR-0049) #5447DECLARED_BUT_UNREAD shape the file's own docblock says was deleted for good.
  • serve-multi-node-cap-advisory.pin.test.ts — a reverted zero-arg call with a trailing comment quoting the old argued call satisfies the "good shape present" assertion raw (hiding the regression); masked, it correctly does not.

Reverse-verification (both legs)

Leg A — revert only the production routing (the MASKED_SERVE_SOURCE = maskComments(SERVE_SOURCE) default / four expect(MASKED_SERVE_SOURCE) call sites back to raw SERVE_SOURCE), full targeted suite: predicted GREEN, confirmed GREEN (13/13) — consistent with the re-measured identity above: today's serve.ts has no adversarial comment, so raw and masked agree everywhere the real assertions look. Restored, confirmed GREEN again (byte-identical to HEAD via git checkout HEAD -- <path>).

Leg B — bypass masking inside the new vacuity-proof tests' "post-fix" leg only (to prove those tests are not vacuous): predicted RED, confirmed RED — all 3 vacuity tests failed exactly as predicted (expected [ 'bogusKey', 'provider' ] to deeply equal [ 'provider' ], expected [ 'persist', 'provider' ] to not include 'persist', expected '...' not to match /checkMultiNodeAllowed\(\s*[^)\s]/). Restored, confirmed GREEN again (13/13).

Collateral gate fix (same PR, caused by this diff)

Adding the maskComments import gave serve-email-config-parity.contract.test.ts its first real escaping read, which newly exposed a pre-existing prose mention of packages/spec/src/system/email-config.test.ts (a doc cross-reference, never actually read by this file) to check:cross-package-test-inputs's literal collector — check:cross-package-test-inputs went red. Reworded the comment to name that file without a single quoted repo-relative literal, following the precedent already set in this same directory (serve-multi-node-cap-advisory.pin.test.ts's own comment about the identical gate). Confirmed green after.

Verification

  • Dependency closure: pnpm --filter '@objectstack/cli^...' build — green.
  • pnpm --filter @objectstack/cli typecheck — green (tsc --noEmit, 0 errors).
  • pnpm --filter @objectstack/cli test (full package, both via pnpm test -- and directly via vitest run on the two changed files) — 152 test files / 1682 tests green package-wide; 13/13 on the two changed files verbose.
  • Gate union at final commit (ebd9be7da6, via node scripts/pm/dispatch-gates.mjs, no paths passed — derived from the real diff):
    • pnpm check:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
    • pnpm check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new...
    • pnpm check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scanned...
    • pnpm check:type-source-resolutioncheck-type-source-resolution OK — 76 packages with a tsconfig.json scanned...
    • node scripts/check-ci-filter-parity.mjsOK: all 82 declared cross-package glob(s) (71 unique) are covered...
    • node scripts/check-plugin-teardown-shape.mjs✓ check:plugin-teardown-shape: 61 Plugin implementation(s)... 0 known-unreached, SHRINK-ONLY, baseline fully burned down.
    • node scripts/docs-audit/check-affected-docs.mjs — self-test + main run, no FAIL.
    • pnpm check:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s)... none new.
    • pnpm check:type-check-coveragecheck-type-check-coverage: OK — 64/77 workspace packages type-checked...
    • pnpm check:type-check-debt (full pnpm exec turbo run build --filter=./packages/* --filter=./packages/*/* closure built first, 70/70 tasks) — check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 203.5s, 1908 raw tsc error(s) total, none above its recorded number.@objectstack/cli's TEST_DEBT entry (146) is unaffected — both new test files sit in the regularly-typechecked src tree, not the hidden test/ tree the ledger tracks. (Unrelated pre-existing informational note in this run: @objectstack/plugin-auth TEST_DEBT could be lowered by 12 — untouched by this PR, tracked by the gate's own linked issue [finding][devx] check:type-check-debt 的 ledger 余量会让新写的 pin 变哑:mongodb 曾有 33 条余量吞掉一次真实回退,另有 5 条目前带 4–19 余量 #6376.)
    • pnpm check:engine-double-contractOK — 377 pinned, 133 in the DEBT ledger, 2 exempt.
    • pnpm check:where-matcher✓ where-matcher conformance holds: 276 matcher(s) discovered, 276 answer... correctly or refuse it loudly.
  • All quoted verdict lines are read from the gates' own output (never a bare $?); every exit code captured before any pipe (cmd > log 2>&1; EXIT=$?).

Changeset — noted conflict, resolved per AGENTS.md

The dispatch brief said a changeset is required. This diff touches only two *.test.ts files under packages/cli/src/commands/ — no runtime/user-facing change. Per AGENTS.md's own convention for this repo (a real skip-changeset label mechanism, applied by the PR author, not CI), this qualifies as tests-only and takes the label instead of a changeset. Applying skip-changeset on this PR now and will read the label list back once it settles.


Generated by Claude Code

… scans (#10514)
Two `packages/cli` scans read `serve.ts` raw, with no comment/code separator
at all, so prose counted as code -- the FABRICATES direction
`scripts/js-comment-mask.mjs`'s own header calls the worse one. Neither scan
was wrong today, but both were one ordinary comment away from being wrong:
- `serve-email-config-parity.contract.test.ts`'s `keysReadFromConfigEmail()`
matched `cfgEmail\.\w+` over raw source, so a comment naming an undeclared
key would have fabricated a false "undeclared key" red, and -- the worse
direction -- a comment naming a declared-but-unread key would have
silently restored the exact `DECLARED_BUT_UNREAD` exemption this file's
own docblock says was deleted for good after #5447/#5470.
- `serve-multi-node-cap-advisory.pin.test.ts`'s four shape assertions
matched regexes over raw source, so a reverted call could hide behind a
trailing comment quoting the shape it replaced.
Both now scan `maskComments(SERVE_SOURCE)` instead. `interfaceFields()`'s own
narrower, brace-matched strip is untouched -- out of scope per the issue.
Re-verified per the card's release condition: on origin/main at branch point
(047ac86), the raw / naive-stripped / masked key sets for subject 1 and
the raw / masked match verdicts for subject 2's four assertions are all
identical -- no verdict moved by #10453/#10919/#10948/#10956's changes to
serve.ts since the card was written. `CROSS_PACKAGE_TEST_INPUTS['@objectstack/cli']`
and the turbo.json `@objectstack/cli#test` input already carry
`scripts/js-comment-mask.mjs`/`.d.mts` (landed with #10453/PR #10513), so no
registration work was needed -- just the two-site routing.
Adds vacuity-proof tests demonstrating the FABRICATES shapes on synthetic
sources: a comment containing the exact pattern each scan keys on is shown
producing the wrong verdict when read raw, and the right one once masked.
Fresh answer to the open question #10909's dev never returned: yes, editing
comment prose in serve.ts could move both scans' results pre-fix (demonstrated
above); post-fix it cannot.
Side effect caught by `check:cross-package-test-inputs`: adding the
`maskComments` import gave `serve-email-config-parity.contract.test.ts` its
first real escaping read, which newly exposed a PRE-EXISTING prose mention of
`packages/spec/src/system/email-config.test.ts` (a doc cross-reference, never
actually read) to the gate's literal collector. Reworded to name it without a
single quoted repo-relative literal, following the precedent already set in
this directory's `serve-multi-node-cap-advisory.pin.test.ts` for the identical
gate.
Fixes#10514
@os-elonos-elon added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/m labels Aug 22, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

@os-elon
os-elon marked this pull request as ready for review August 22, 2026 04:44
@os-elon
os-elon added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit 5359a9bAug 22, 2026
38 checks passed
@os-elon
os-elon deleted the claude/issue-10514-serve-scan-comment-mask branch August 22, 2026 05:28
@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

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

    ✗ Build failed in 5.81s
    

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

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

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

历史信号:

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

分诊清单:

  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

skip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-elon@claude