Skip to content

fix(cli)!: report the named export the config default export already declares, instead of dropping it silently (#18419) - #18647

Merged
os-support-ai merged 3 commits into
mainfrom
claude/issue-18419-named-export-key-collision-silent-drop
Sep 17, 2026
Merged

os-support-ai merged 3 commits into
mainfrom
claude/issue-18419-named-export-key-collision-silent-drop

Conversation

@os-support-ai

@os-support-ai os-support-ai commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Fixes #18419

What was wrong

objectstack.config.ts is loaded as a module: loadConfig() takes the default export as the base and merges every named export onto it as a top-level stack key. A named export whose name the default export already carries was skipped by

if (key === 'default' || key in merged) continue;   // the silent skip

so the authored value reached no artifact, no parse and no log at any level, while os build exited 0.

Triage's two binding constraints, carried verbatim:

⭐ 那个循环是故意的(注释写明要保住 onEnable / functions 这类具名导出);缺陷只在 key in merged 那一臂:撞名解析成默认,⛔ 无声。⇒ ⛔ 修法不是删循环。

✅ 填报席给了三行对照,其中 A 行是发火对照(export const ProbeNamedExport ⇒ 响亮失败、parse 拒、点名该键)⇒ 证明同一条通道有能力响亮,静默是这一臂特有的。⛔ 本席不重跑。

The loop is untouched. What changed is the one arm, and what it does when it fires.

The card's three rows, re-measured on origin/main 32be735

Row A was taken as given as the card's positive control — it is not rebuilt here, it is used: it rides in the same harness as rows B and C so a repaired loader is distinguishable from one that simply warns about everything. All three reproduced, plus two shapes the sweep added.

row authored before after
A (control) export const ProbeNamedExport = [1,2,3] loud — parse refuses, names the key unchanged
B export const objects = [], default carries no objects merged, accepted unchanged
C export const objects = [row], default carries objects: [] silent, exit 0, row gone reported on stderr, shadowedNamedExports: ['objects']
C2 same collision on functions silent, the handler gone reported, names functions
D export const toString = [1,2,3] silent, exit 0 refused by name — rejoins row A

Premise falsified: key in merged is not one silent arm, it is two

Sweeping loadConfig() for other silent shapes (the dispatch asked for this, carrying a control that can fail) turned up row D inside the very same expression. key in merged walks the prototype chain, so every Object.prototype member — toString, valueOf, constructor, hasOwnProperty, propertyIsEnumerable, toLocaleString, isPrototypeOf — answered true for a default export that carries no such key at all. Such an export was skipped by the collision arm and therefore never reached the strict parse that refuses an undeclared stack key by name: row A's loud refusal, silently turned off by the spelling of the key.

This is not an optional extra. Without it the new diagnostic would lie — it would report toString as "shadowed by the default export" when the default declares nothing of the sort. Object.prototype.hasOwnProperty.call(merged, key) is what makes the report truthful and closes the hole in one move.

Everything else in loadConfig() was swept and is loud: mod.default || mod with a falsy default hands the whole namespace to the strict parse, which refuses the default key by name.

The decision: advisory, not refusal — read off the tree

The choice was made from this package's own repairs of this class, not from taste:

A refusal would also have to live in loadConfig() to reach every face, and two of those faces exist precisely to read a config the current schema is unhappy with: os doctor diagnoses broken projects, and os migrate meta is entitled to read past a rejection via authoredSourcePlugin — a loader-level throw is not something its authoredSource option can shim away. Refusing there would close the upgrade path against exactly the legacy configs a collision is most likely to sit in.

Where it renders: stderr, from the loader. loadConfig() is handed no --json flag (this file's resolveConfigPath header states the same fact for the same reason) and twelve commands call it, so printing from the loader is what puts the finding on all twelve faces instead of the two that handle named exports today. stderr is what keeps a --json run's stdout a single parseable document — the shape refuseConfig already established in this file, minus the throw. printWarningToStderr is the warning-severity counterpart of the existing printErrorToStderr, added for that one reason.

Accept-set measurement (the dispatch asked for the number and the pathspec)

Pathspec swept: git ls-files '*objectstack.config.ts' '*objectstack.config.js' '*objectstack.config.mjs'10 tracked config files.

  • 3 carry a named export at all (examples/app-crm, examples/app-showcase, examples/app-todo), each exporting onEnable. That non-zero count is the sweep's own positive control: the detector finds named exports where they exist, so the collision count below is a reading and not a dead search.
  • 0 of the 10 would newly fail. None of the three writes onEnable inside its defineStack({ … }) call, so none collides; and none exports an Object.prototype name.

Rows C and C2 move no accept-set member — the build still exits 0 with the same artifact, it merely also says what it dropped. Row D is the one narrowing, and it narrows toward the documented rule rather than away from it.

Clause-②: no (narrowing)

Re-declared from the measured diff, not inherited from the claim comment. The #16349 ruling made this clause directional: widening the accept set or the public surface triggers the contract-review tier; pulling code back to the declared contract does not.

Three readings:

  1. New exportsshadowedNamedExportWarning and printWarningToStderr are added, plus the LoadedConfig.shadowedNamedExports field. None is on a published entry point. packages/cli's exports map is exactly ., ./console, ./hook-body, ./package.json; none of the three entry sources carries an export * or re-exports utils/config.js / utils/format.js (grepped — the only matches are prose in comments). The module ships inside the tarball under dist/, but no published entry point names these symbols, so the public surface does not grow.
  2. ERROR_CODE_LEDGER / StandardErrorCode — 0 hits across the diff, against a positive control on the same instrument that returns 4. No code is minted: the advisory is text on stderr and carries neither code nor httpStatus, so errorCodeFields() still contributes nothing here — the standing prohibition in the ConfigRefusalError header is respected.
  3. Accept-set changes — one, and it is a narrowing: row D. A config exporting an Object.prototype name used to build green and now gets the same named refusal every other undeclared helper export has always got. That is the rule this file's own header states, so the movement is toward the declared contract, not away from it. Rows C and C2 — the card's actual subject — move no accept-set member at all.

The mechanical limb is NOT MEASURED, and that is reported rather than read as clean. check-widening-tells.mjs --declaration no over this diff exits 0 while saying so in its own words: 5 changed files, 0 judged, 5 NOT MEASURED — "no declared surface covers it", because the tells are defined over packages/spec schema sources, closed sets, the api-surface listings and the registries, and this diff touches none of those. So its exit 0 is evidence about no surface at all, and the reading above is the semantic one.

--pair exit code is recorded in the report comment on #18419.

⇒ The measured reading is no, so needs:contract-review is not hung — that label stays the seat's. The fix was not softened to reach that answer: the one accept-set movement in the diff is kept, declared, and carried in the changeset under a **BREAKING** banner with an ADR-0087 disposition.

Documentation falsification sweep

Predicate written down before reading anything. A page is falsified iff (a) it shows a config this change now REFUSES — a default export plus a named export named for an Object.prototype member; or (b) it states, present tense, that a named export whose key the default already carries is merged/wins/honoured; or (c) it documents the silent drop as intended. NOT falsified by merely naming loadConfig, by non-colliding named exports (onEnable / functions — that loop is deliberate and untouched), or by a default export alone.

Swept by the INPUT shape rather than the symbol, over content/docs, skills/, examples/, apps/docs/ and every tracked config.

Controls, both directions: a token known present returned 5 files (export const onEnable); a nonsense token returned zero. What the sweep could not reach: the sibling objectui repo (out of this repository) and any page describing the shape in wording that shares no token with either arm — though arm (a) is a source regex over every tracked file, so it is wording-independent.

Verification

Everything below is measured on the merged head b04bfbefd (this branch with origin/main merged in, rebuilt per AGENTS.md §9).

  • Derived gates: 90 families, 90 run, 0 NOT-MEASURED, 0 UNRUN, all exit 0. Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on this tree and reconciled with --ran carrying COMMAND :: exit CODE for every one.
  • pnpm --filter @objectstack/cli typecheck — exit 0.
  • pnpm --filter @objectstack/cli exec vitest run --project unit — exit 0, 212 files / 3026 tests. The integration tier is declared to CI: the diff touches no spawn entry, no bin/, no test/helpers/serve-process.ts and no driver/kernel boot path. The new pin's tier is measured, not asserted — vitest list --project unit finds its 7 tests, and a known spawn e2e is absent from that list as the control.
  • pnpm lint — exit 0, repo-wide, not narrowed. (dispatch-gates does not name this family; it was run anyway.)

One gate was red before the merge and is green after, and the difference is not mine. pnpm check:cross-package-test-inputs failed on packages/cli/test/init-created-files-summary.e2e.test.ts — a file this diff never touches — for descending packages/spec/dist/. A control on pristine origin/main plus an unrelated one-line packages/cli edit reproduced it identically (the naive control was vacuous: that worktree had no packages/spec/dist to walk, so the row could not fire). origin/main then landed #18641, which judges a walk root against the index rather than the working tree — exactly this case. Merged in, the gate is exit 0. No finding to file: it was a real defect, in the gate, already fixed upstream.

Acceptance notes

Noted, not filed — observations from the sweep, no card:

  • loadConfig()'s if (!baseConfig) throw new Error('No default export found in …') is unreachable. baseConfig is mod.default || mod, and mod is always a non-null object from bundleRequire, so the guard can only fire on a falsy mod. The real "no default export" path falls through to the namespace-as-stack branch and is refused by the strict parse instead. Dead code, not a defect; left alone. Successor who will meet it: whoever next touches this unwrap — the same reader the #18419 comment right above it now addresses.
  • The docs table's first row (not a declared stack key → "the build fails, naming the key") was false for prototype-chain names before this change and is true for every spelling after it. Corrected as part of the same row rewrite rather than filed.

Authored by Claude Code, session session_01DvvamiacK328idtBYJBxV3.


Generated by Claude Code

… declares, instead of dropping it silently

`loadConfig()` merges every named export of `objectstack.config.ts` onto the
default-exported stack as a top-level key. A name the default already carried
was skipped by `if (key === 'default' || key in merged) continue` — the build
exited 0, the artifact carried the default's value, and nothing was written at
any level, so authored content vanished on the success path.

The drop itself is correct and stays: one key, one value. What changes is that
it is now REPORTED — on stderr, from the loader, so all twelve commands that
load a config carry it and a `--json` run's stdout stays a single parseable
document — and recorded structurally on `LoadedConfig.shadowedNamedExports`.
Advisory, never fatal: the stack that comes out is valid, it is merely missing
what the shadowed export carried. That disposition is this package's own for
the class (#3786's undeclared authoring keys, #4095's orphaned runtime
members), not a fresh judgement.

Sweeping the arm for other silent shapes found a second one in the same
expression: `key in merged` walks the PROTOTYPE chain, so `Object.prototype`'s
members answered true for a default export carrying no such key. An
`export const toString = …` was skipped by the collision arm and therefore
never reached the strict parse that refuses an undeclared stack key by name —
the loud refusal, silently turned off by the spelling of the key. The test is
now `Object.prototype.hasOwnProperty.call`, which both closes that hole and is
what makes the new diagnostic truthful: without it the loader would report
`toString` as shadowed by a default export that declares nothing of the sort.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 17, 2026
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/automation/hook-bodies.mdx (via loadConfig (symbol, a top-level function))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-2.mdx (via loadConfig (symbol, a top-level function))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 — 24 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 84ad2e1394f77e7f117b3422590b791a068c2bc3packageMentionDocs.

Which tree this was computed on

This run read content/docs from 283248dacd7bd32e575fa269d07b71ecb3dc9d46 — the merge of head b04bfbefd51c44f1bde7b3d36712bfd1081c0c72 into base 84ad2e1394f77e7f117b3422590b791a068c2bc3, 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 283248dacd7bd32e575fa269d07b71ecb3dc9d46 && git checkout 283248dacd7bd32e575fa269d07b71ecb3dc9d46
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 84ad2e1394f77e7f117b3422590b791a068c2bc3 b04bfbefd51c44f1bde7b3d36712bfd1081c0c72 && git checkout -B drift-repro 84ad2e1394f77e7f117b3422590b791a068c2bc3 && git merge --no-ff b04bfbefd51c44f1bde7b3d36712bfd1081c0c72

node scripts/docs-audit/affected-docs.mjs --json 84ad2e1394f77e7f117b3422590b791a068c2bc3

⚠️ 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 84ad2e1394f77e7f117b3422590b791a068c2bc3 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… not silent

`your-first-project.mdx` stated the shape by its INPUTS and never named
`loadConfig`, so the docs drift check could not see it: its callout said a
named export whose key the default export already carries "is dropped
**silently**, and the build still succeeds". The first half is still true and
the second half is unchanged — what is no longer true is the word the sentence
turns on.

Found by sweeping for the input shape (a config pairing a default export with a
colliding named export) rather than for the symbol, which is the only arm that
reaches a page written this way.

Claude-Session: https://claude.ai/code/session_01DvvamiacK328idtBYJBxV3
Co-authored-by: Claude <noreply@anthropic.com>
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

2 participants