Skip to content

fix(cli): derive a parseable JS identifier for the emitted plugin symbol - #16536

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-15892-create-plugin-identifier-sanitiser
Sep 7, 2026
Merged

fix(cli): derive a parseable JS identifier for the emitted plugin symbol#16536
os-sales merged 2 commits into
mainfrom
claude/issue-15892-create-plugin-identifier-sanitiser

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #15892

os create plugin foo.bar exited 0 having written export const foo.barPlugin: Plugin = { — a property access where a binding name belongs — so the project the scaffolder emitted did not parse.

The ruling this implements

Comment 5564368535 on the card (director seat, decision batch #64, 2026-09-07T02:52:30Z) records the maintainer's reply, verbatim 「同意」, adopting option 2:

os create keeps accepting exactly what npm accepts (the #15816 validator is right). The emitted identifier is derived, not copied: every character legal in an npm name but illegal in a JS identifier folds the way - already does (foo.barfooBarPlugin; _, ~, and a leading digit handled likewise, e.g. a leading digit gets a fixed prefix). This is the completion of the existing toCamelCase, in the same spirit as sanitizeNamespace() one line away — not a third naming philosophy on the emitted manifest: the package name stays exactly what the user typed (#15530 / #15816), only the code identifier is normalised, which is the JS ecosystem's universal convention.

Acceptance is untouched: validateProjectName still admits exactly what npm admits, and the emitted package name, its scope and the project directory stay byte-for-byte what the user typed. Only the code identifier is normalised. The emitted scope question raised on #15530 stays with the maintainer and is not addressed here.

What changed

toCamelCase in packages/cli/src/commands/create.ts folded -x into X and passed every other character straight through. sanitizeIdentifier generalises that one fold: every run of characters illegal in a JS identifier is treated as the separator - already was, and a leading digit takes the a prefix sanitizeNamespace() has always used for the same rule. It is used at all four emission sites the ruling names — src/index.ts twice, README.md twice — and the emitted README now names the derived symbol in prose, so the mapping from package name to exported symbol is stated once where the user reads it.

No reserved-word handling, deliberately: every emission site appends Plugin, so the identifier that lands is never a bare keyword.

Driven verification

Built, then driven through the published entry into a scratch directory, exit code read before any pipe, and each emitted file handed to TypeScript's own parser (ts.createSourceFile + getSyntacticDiagnostics, no lib, no resolution):

name      exit  package.json name               symbol         src/index.ts  README fence
foo.bar   0     @objectstack/plugin-foo.bar     fooBarPlugin   0 diags       0 diags
1foo      0     @objectstack/plugin-1foo        a1fooPlugin    0 diags       0 diags
a_b       0     @objectstack/plugin-a_b         aBPlugin       0 diags       0 diags
my-app    0     @objectstack/plugin-my-app      myAppPlugin    0 diags       0 diags

CANARY (the pre-fix bytes for foo.bar): 1 diagnostic — "',' expected."

my-app is the control in both directions and still yields myAppPlugin. The canary is what makes a zero a reading rather than an inert harness: the same instrument, over the bytes this change replaces, reports the defect — and it reproduces the count the card measured.

Tests

packages/cli/test/create-plugin-identifier-parses.test.ts (unit tier) drives the same instrument over the rendered template for all four names and both placements, and additionally pins:

  • the mapping for each name, because a_b does not discriminate on parseability — a_bPlugin was always legal TypeScript, so it is asserted on the fold instead;
  • that validateProjectName still accepts every one of them, which is this PR's own re-derivation that npm acceptance did not move;
  • that the emitted package name and directory are the input string, unchanged;
  • the emission-site counts, so a new site has to be added here deliberately;
  • the canary above.

A tilde is worth recording: the card lists ~ as a shape that reaches the emission, and at origin/main it does not — validateProjectName's charset is ^[a-z0-9][a-z0-9._-]*$, which refuses it before any write. The sanitiser folds it anyway; the test states which of the two refuses it.

验收备注

Noted, not filed — the PM seat opens the card if it wants one:

  • os generate carries the same class, and has no validator in front of it. Reproduced on this branch, driven: os generate object foo.bar exits 0 and writes const foo.bar: Data.ServiceObject = { in src/objects/foo.bar.object.ts (1 syntactic diagnostic) and export { default as foo.bar } from './foo.bar.object'; in src/objects/index.ts (1 syntactic diagnostic). generate.ts runs no name validation at all, so its accepted set is wider than create's. This PR is scoped to create.ts by the ruling and does not touch it.
  • The two sibling copies of the helper (generate.ts and init.ts) fold [-_]([a-z]), not -([a-z]) — they are not byte-identical to the one this PR replaced, so a future consolidation is a behaviour decision about init and generate, not a lift-and-drop.

Generated by Claude Code

`os create plugin <name>` interpolated the project name straight into an
identifier position (`export const <name>Plugin`), while the shared
`validateProjectName` accepts exactly what npm accepts — a dot, an
underscore and a leading digit included. So `os create plugin foo.bar`
exited 0 having written `export const foo.barPlugin: Plugin = {`, a
property access where a binding name belongs.

Acceptance is unchanged: the emitted package name, its scope and the
emitted directory name stay byte-for-byte what the user typed. Only the
code identifier is normalised. `toCamelCase` folded `-x` into `X` and
passed everything else through; `sanitizeIdentifier` generalises that
fold to every run of non-identifier characters and prefixes a leading
digit with `a`, the rule `sanitizeNamespace()` already uses. `my-app`
still yields `myApp`.

The emitted README now names the derived identifier in prose, so the
mapping from package name to exported symbol is stated once where the
user reads it.

The pin drives TypeScript's own parser over the emitted bytes and
asserts zero syntactic diagnostics, with `my-app` as a control in both
directions and a canary that asserts the pre-fix bytes DO produce a
diagnostic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
@github-actions github-actions Bot added the size/m label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 3 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via os create (command, read off packages/cli/src/commands/create.ts))
  • content/docs/plugins/index.mdx (via os create (command, read off packages/cli/src/commands/create.ts))
  • content/docs/protocol/kernel/index.mdx (via os create (command, read off packages/cli/src/commands/create.ts))
  • content/docs/protocol/kernel/plugin-spec.mdx (via os create (command, read off packages/cli/src/commands/create.ts))
What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 46e626513940806af0e8da5dbe1eeec12eae1c45packageMentionDocs.

Which tree this was computed on

This run read content/docs from 42c24e952c3a3e00ae795c4acb689e99d38a58cc — the merge of head b96b0620df27108306f9dd687fdd0700e4175d86 into base 46e626513940806af0e8da5dbe1eeec12eae1c45, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 42c24e952c3a3e00ae795c4acb689e99d38a58cc && git checkout 42c24e952c3a3e00ae795c4acb689e99d38a58cc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 46e626513940806af0e8da5dbe1eeec12eae1c45 b96b0620df27108306f9dd687fdd0700e4175d86 && git checkout -B drift-repro 46e626513940806af0e8da5dbe1eeec12eae1c45 && git merge --no-ff b96b0620df27108306f9dd687fdd0700e4175d86

node scripts/docs-audit/affected-docs.mjs --json 46e626513940806af0e8da5dbe1eeec12eae1c45

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 46e626513940806af0e8da5dbe1eeec12eae1c45 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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

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

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

  • Test Core (6/6) — 失败步骤: Run this shard's tests

    @objectstack/lint:test:  FAIL  src/system-fields-consumers.test.ts > SYSTEM_FIELDS consumer census (#8999) > the live population > is anchored to a real export, so an empty answer cannot be a false gr
      ↳ 失败原因: @objectstack/lint:test: Error: Test timed out in 5000ms.
    

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

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

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

  • src/system-fields-consumers.test.ts — 24h 窗口内只有本 PR 撞到过,暂不汇总(再有一个不同 PR 撞到就会自动开汇总 issue)。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

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

分诊清单:

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

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

os-sales commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Queue build 34099758788 — standing down. NEW SIGNATURE ⇒ ⛔ not re-queued on this seat's judgement.

domain:cli seat, session session_01YFY46JydE1gMxQG1TqBcMZ, 2026-09-07T08:30Z.

The signature, in full

job          Test Core (6/6)   ·   step: Run this shard's tests
package      @objectstack/lint
file         src/system-fields-consumers.test.ts
test         SYSTEM_FIELDS consumer census (#8999) > the live population
             > is anchored to a real export, so an empty answer cannot be a false green
cause        Error: Test timed out in 5000ms.
run          34099758788

Initial judgement: this is not this PR's regression — and the ground is structural, not a hunch

  1. @objectstack/lint does not depend on @objectstack/cli. Measured: 10 dependencies across dependencies / devDependencies / peerDependencies, none naming cli. This PR's diff is packages/cli/src/commands/create.ts, one new packages/cli/test/ file and a changeset. ⇒ The diff cannot reach that test, even transitively — this is a dependency-direction fact, ⛔ not the weaker "different package" heuristic.
  2. The cause is a TIMEOUT, not an assertion. The queue-triage workflow's own discriminator applies: Test timed out … 多半是负载/时序,不是本 PR 的回归;AssertionError: … 才指向真实的行为改变. A census whose population had changed would fail as an assertion naming the arrived/vanished members; this one ran out of a 5000 ms budget.
  3. The queue runs the full suite where PR-side CI runs the affected subset — which is exactly why a load-sensitive timeout surfaces here and not on the PR.

⛔ Why it is NOT being re-queued anyway

The signature ledger is the sole authority here and it does not carry this file. Open anchors, read at 08:29Z: #16506 (src/email-service.queue-delivery.test.ts) and #14822 (test/run-dev-unbuilt-workspace.e2e.test.ts). system-fields-consumers.test.ts is neither.

New signature ⇒ ⛔ do not re-投. A blind re-queue rebuilds every PR behind this one, and "flake" is not a root cause. ⭐ ⛔ This seat also does not add a ledger row: a suspected new flaky is a proposal, never a self-added entry — only a human upgrades that table.

The triage workflow's own checklist lands in the same place. Item 1 (failing test in a package this PR changed) is no; item 2 (known cross-PR signature) is no aggregate issue yet"24h 窗口内只有本 PR 撞到过", and ⚠️ that count is an explicit lower bound, since the 24h comment ledger was not read to the end. That leaves item 3: possible semantic interaction with the batch ⇒ wait for the PRs ahead to land or fail out, then re-queue once — ⛔ never consecutively.

⚠️ Batch context: #16537 entered the queue 66 seconds after this PR (08:16:33 / 08:17:39) and both are still resolving. That is the batch item 3 names.

What this seat will do

  1. Wait for fix(client)!: bind the auth.* family to the wire shapes better-auth sends #16537 to land or leave the queue.
  2. Then one re-queue of this PR, unmodified — the single sanctioned re-run, spent deliberately rather than reflexively.
  3. If it reddens on the same signature a second time, that is a real second data point: ⛔ no further re-queues, and the finding gets filed for a queue-flake-anchor row as a proposal to the ledger's owner.

⛔ No test will be skipped, quarantined or given a longer timeout to get green. ⛔ No empty commit, no close-and-reopen. The diff is unchanged and stays unchanged.

⚠️ Queue membership itself is NOT MEASURED: the timeline carries added_to_merge_queue and no removed_from_merge_queue, while auto_merge reads empty (which is also what queue ownership looks like). Dequeue is answered by a positive probe, and ref absence is only circumstantial — so this seat is ⛔ not asserting the PR is out, only that it has not merged.


Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 7, 2026
@os-sales
os-sales added this pull request to the merge queue Sep 7, 2026

os-sales commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

The one sanctioned re-queue — spent now, unmodified. ⛔ There is no second.

domain:cli seat, session session_01YFY46JydE1gMxQG1TqBcMZ, 2026-09-07T09:1xZ.

The precondition I named at 08:31Z is met

I said this seat would (1) wait for #16537 to land or leave the queue, then (2) re-queue this PR once, unmodified. Read just now:

PR #16537   state: closed   merged: TRUE   merged_at: 2026-09-07T08:57:41Z
PR #16543   state: closed   merged: TRUE   merged_at: 2026-09-07T08:51Z (same batch window)

⇒ The batch that triage's item 3 named — "可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次" — has resolved by landing, not by timing out. That is the condition, and it is the only one that was outstanding.

Nothing about the diff moved

Head is still b96b0620df27108306f9dd687fdd0700e4175d86, 3 files, +271/−6. ⛔ No empty commit, ⛔ no close-and-reopen, ⛔ no test skipped, quarantined or given a longer timeout. The PR is being re-submitted byte-for-byte as the queue last rejected it, which is what makes this a discriminating second reading rather than a different experiment.

PR-side CI, collapsed latest-per-name before tallying

35 raw check runs ⇒ 33 distinct names: 30 success, 3 skipped, 0 failure, 0 pending. ⚠️ The raw count is not the reading — the endpoint returns every run per name, and two names here carry a second run apiece. Tallying the raw list would have reported failures this PR does not have.

What a second red would mean, and what it would NOT license

If build N+1 reddens on src/system-fields-consumers.test.ts again, that is the second data point the ledger wants:

  • no third re-queue on this seat's judgement;
  • the finding gets written up as a queue-flake-anchor proposal to the ledger's owner — ⛔ this seat does not add a ledger row itself, because a suspected flaky is a proposal and only a human upgrades that table;
  • and the structural ground stays on the record: @objectstack/lint does not depend on @objectstack/cli (10 deps across dependencies / devDependencies / peerDependencies, none naming cli), so this diff cannot reach that test even transitively.

If it reddens on a different signature, that is a new signature again and the same standing-down rule applies to it — not an escalation of this one.

If it goes green, the timeout reading is confirmed and the card's residue gets stripped on merge.


Generated by Claude Code

Merged via the queue into main with commit ce8caba Sep 7, 2026
36 checks passed
@os-sales
os-sales deleted the claude/issue-15892-create-plugin-identifier-sanitiser branch September 7, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os create plugin foo.bar emits export const foo.barPlugin — an npm-legal name that renders un-parseable TypeScript

2 participants