Repository navigation
fix(spec): composeStacks refuses a non-array concatenated collection instead of dropping its content - #19794
Conversation
…instead of dropping it Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8
…ncat-fields-refusal
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2f0ef75a5793deb4e27d4026fdb668d10497b6f5 && git checkout 2f0ef75a5793deb4e27d4026fdb668d10497b6f5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fae870352ea59ddfb1c6dfd784bc6552cf158211 0bf2e5564619cd3e4d5b36541c66850205ffa2a5 && git checkout -B drift-repro fae870352ea59ddfb1c6dfd784bc6552cf158211 && git merge --no-ff 0bf2e5564619cd3e4d5b36541c66850205ffa2a5
node scripts/docs-audit/affected-docs.mjs --json fae870352ea59ddfb1c6dfd784bc6552cf158211
|
Contract reviewServed-tier: ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Rendered by an isolated at-tier review subagent (card, ruling, precedent PR, the PR and its check-runs — not the dispatch order), adopted by Generated by Claude Code |
…an ADR-0112 envelope (objectstack-ai#19798) Fixes objectstack-ai#19785 Clause-②: no (narrowing) `defineStack(config, { strict: false })` now refuses a non-array `objects`, and an `objects` array holding a non-object entry, with the ADR-0112 envelope: `StackSchemaInvalidError`, `code: 'STACK_SCHEMA_INVALID'`, `status: 422`, and the zod issue at `path: ['objects']` (non-array) or one per entry at `path: ['objects', index]` with `expected: 'object'` (non-object entry). Before, a number or a string raised a bare `TypeError: config.objects.map is not a function` from `mergeActionsIntoObjects`, and a `null` entry raised one reading `actions` off it. No new error code; the error class stays module-local. ## Refuse or skip: the card's first question **Refuse.** Here is what `DefineStackOptions.strict` declares for `false`: "validation is skipped for maximum flexibility (e.g., when views reference objects provided by other plugins). Use this ONLY when you need to bypass validation for advanced use cases." It promises that validation is skipped: cross-references and schema detail. It does not promise to accept, or quietly skip, a shape the action merge cannot read. No ADR is cited there. So this is not a contract-direction question, and the objectstack-ai#18239 ruling (`5690859601`) applies one door earlier: 「an ADR-0112 envelope error (closed `error.code`, status), ⛔ not a bare `TypeError` and ⛔ not a skip」. ## What changed `packages/spec/src/stack.zod.ts`, inside `mergeActionsIntoObjects` only: 1. **Array guard.** Any `objects` that is present and not an array is refused. `undefined` is the only non-array that is not malformed, because the key is absent. This is the line objectstack-ai#19783 drew for `mergeObjects`. It includes the falsy values `null`, `''`, `0` and `false`, which used to be handed on untouched and then refused one call later by `composeStacks` with this same code. Refusing `5` while passing `0` would repeat the silent-skip row objectstack-ai#19783 removed. The map form (`{ name: { ... } }`) is normalized to an array before the merge and is still accepted. 2. **Non-object entries (rework round 1).** An entry that is not an object (`null`, `7`, a string, an array) is refused with the same envelope: one zod issue per such entry at `['objects', index]`, `expected: 'object'` (read from `z.array(z.looseObject({}))`, re-rooted at `objects`, which is the strict parse's own answer for the shape). The message names each position. Round 0 handed such an entry on untouched. The contract review showed that this widens the accept set under a `(narrowing)` declaration: the card's own `[null, obj]` repro threw before and would have succeeded. It would also let the next consumer, plugin-object registration in `packages/objectql/src/engine.ts` (one loop in one warn-level catch), silently drop every object after the bad entry. Now every row narrows. Measured on the base `3f9e2eaa1c`, against the built `dist`: | `objects` under `strict: false` | before | after | | :-- | :-- | :-- | | `5`, `'abc'` | `TypeError: config.objects.map is not a function`, `code`/`status` undefined | `STACK_SCHEMA_INVALID`, 422 | | `null`, `''`, `0`, `false` | returned untouched | `STACK_SCHEMA_INVALID`, 422 | | `[null, obj]` | `TypeError: Cannot read properties of null (reading 'actions')` | `STACK_SCHEMA_INVALID`, 422, issue at `['objects', 0]` | | `[obj, 7]` | returned with the entry in place | `STACK_SCHEMA_INVALID`, 422, issue at `['objects', 1]` | | map form, array, absent | accepted | accepted (unchanged) | ## Tests - New file: `packages/spec/src/define-stack-non-strict-objects-shape-refusal.test.ts`. It has 6 non-array refusal rows asserting `code` + `status` + the issue (`path`, `invalid_type`, `expected: 'array'`). Two entry refusal rows cover `[null, obj, 7]`, with issues exactly at `['objects', 0]` and `['objects', 2]`, all `invalid_type` / `expected: 'object'`, plus `[null, obj]`. Controls: array, map form and absent `objects` are all accepted with the bound action merged, and the strict door raises the same code for the same shape. - `packages/spec/src/compose-stacks-objects-shape-refusal.test.ts` (objectstack-ai#19783's file): its four falsy rows reached composition through `strict: false`. That door now refuses those shapes itself, so the rows now reach composition as hand-built stacks. They keep the same values and the same assertions. This is a fixture re-route, and it is outside the claim's declared file surface. I am declaring it here; the contract review accepted it. - **Ablation, round 0** (commit `d3959b7b79`): `stack.zod.ts` restored from the base, with the new file run against it: `Tests 7 failed | 4 passed (11)`. The refusal rows failed with `expected undefined to be 'STACK_SCHEMA_INVALID'` and the entry row with a bare `TypeError`. Restored, `git diff HEAD` empty. - **Ablation, round 1** (commit `c5a865c769`): `scripts/ablation-replace.mjs` replaced the entry guard's condition. The anchor went x1 to x0 and the blob `88f36c9846` became `ac5fbf58be`. Result: `Tests 2 failed | 10 passed (12)`. Both entry rows failed with `expected undefined to be 'STACK_SCHEMA_INVALID'` (a bare `TypeError` reached the assertion), and every other row stayed green. Restore: blob back to the HEAD blob `88f36c9846`, `git diff HEAD` empty. - At `c5a865c769` (merge of `origin/main` `fae870352e`): - `pnpm --filter @objectstack/spec build` + `typecheck`: exit 0. The test layer compiles, and `test-typecheck-debt.json` held. - The two targeted files: 37/37. - Full spec suite: `Test Files 552 passed (552)`, `Tests 15690 passed | 1 todo`. - eslint `--no-inline-config` over the 3 touched TS files: 3 files, 0 errors, 0 warnings. Type-aware linting is not enabled in `eslint.config.mjs`, so this diff cannot move the verdict on any untouched file. - `dispatch-gates --commands` derived 82 commands (the same list as round 0). 80 exit 0. 2 are NOT MEASURED with exit 3 (they need the whole-workspace build): `check:dual-build-cjs-loads` and `check:type-check-debt`. `--ran` reconcile: 82 accounted, 0 UNRUN. - **Round 1b, at `ce893bbf31`** (merge of `origin/main` `0b83e01627`, which includes objectstack-ai#19794, then the helper routing): - The targeted files plus objectstack-ai#19794's `compose-stacks-concat-shape-refusal.test.ts`: 162/162. - Spec build + typecheck: exit 0. - Full spec suite: `Test Files 553 passed (553)`, `Tests 15815 passed | 1 todo`. - Ablation of the array guard: `Tests 6 failed | 6 passed (12)` (all 6 non-array rows). Ablation of the entry guard: `Tests 2 failed | 10 passed (12)`. Both restores ended at blob == HEAD `ac2a452f57`, `git diff HEAD` empty. - eslint: 3 files, 0 errors. - Gates: 82 derived, 80 exit 0, the same 2 NOT MEASURED, 0 UNRUN. - `origin/main` has since moved one commit, `de4ed33fd5` (docs(pm), objectstack-ai#19795). It is not merged. ## Acceptance notes - `class: a`, not fixed here, filed by the seat as objectstack-ai#19799. The same function still dereferences the other shapes it reads under `strict: false`: a top-level `actions` of `5` or `'abc'`, or `actions: [null]`, raises a TypeError from `sortActionsByOrder`. - Review item 4 is done at `ce893bbf31`. PR objectstack-ai#19794 landed (`0b83e01627`), so the non-array block now calls its `describeNonArrayCollection('objects', …)` helper, with no hand-copied kind/issues code left. Messages are byte-stable for every input that can reach this door. The helper's one extra branch, `'an object'` for a plain object, is unreachable here because `normalizeStackInput` turns the map form into an array first. - `strict: false` with a `Set` as `objects` still returns without throwing; the normalizer reads it as a keyless map. This was recorded on objectstack-ai#19783 and is not part of this card. --- _Generated by [Claude Code](https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…ection instead of accepting it as empty (objectstack-ai#19811) Fixes objectstack-ai#19796 Clause-②: no (narrowing) `normalizeMetadataCollection` now reads **only a plain object** as the map form of a collection. Before, its map branch tested `typeof value === 'object'`, so a `Set`, `Map` or `Date` went through `Object.entries` (which yields an empty list for them) before the schema parse, and strict `defineStack` accepted the stack with the collection empty. Every authored entry was gone, with no error and no warning. Now a non-plain object reaches the parse unchanged, and the ordinary strict envelope refuses it at the key. Direction: **refuse, not convert.** Converting an iterable of entries would widen the strict door's accept set, and the triage condition (`5790091608`) put that out of scope. The governing sentence is ruling `5690859601` on objectstack-ai#18239: 「A composed artifact is complete or it is refused」. ## Landing site `packages/spec/src/shared/metadata-collection.zod.ts`: a module-local `isPlainObject` guards the map branch. It treats a value as plain when its prototype is `null`, or when its prototype's own prototype is `null`. The second case accepts a plain object from another realm (a `vm` context), whose prototype is that realm's `Object.prototype`. This is the only producer, so no other package is touched. ## Measurement (tsx probe, strict `defineStack({ manifest, permissions: VALUE })`) | VALUE | base `0b83e01627` | this branch | | :--- | :--- | :--- | | `new Set([{ name: 'rep', label: 'Rep' }])` | ACCEPTED, `permissions` is `[]` | `STACK_SCHEMA_INVALID` 422, issue `invalid_type` at `['permissions']`, `expected: 'array'` | | `new Map([['rep', { label: 'Rep' }]])` | ACCEPTED, `[]` | same refusal | | `new Date()` | ACCEPTED, `[]` | same refusal | | a class instance with a `rep` field | read as a map of its own fields (then refused deeper, at `permissions.0.objects`) | same refusal, at the key | | `Object.create(null)` map with `rep` | normalized, as a map | unchanged (normalized) | The refusal message is `permissions: Expected array but received object.`, and it names the key where the value was written. No new error code and no prescription text: see the Acceptance notes. ## Callers of `normalizeMetadataCollection` In spec it is called only through `normalizeStackInput` and `normalizePluginMetadata`, in the same file. `normalizePluginMetadata` has no callers outside `packages/spec`. `normalizeStackInput` has these callers outside the spec package: - **Callers that parse:** `defineStack` (strict), CLI `compile`, `validate`, `lint`, `lint/score`, `utils/scaffold-validate`, `migrate meta`. A non-plain object used to reach the parse as `[]` and pass. Now it is refused at the key. - **`defineStack({ strict: false })`:** the value flows through unparsed. `composeStacks` step 3 then refuses it (landed in objectstack-ai#19794), where before a `[]` composed silently. - **Readers that do not parse:** CLI `info`, `doctor`, `i18n check`, `i18n extract`, the dogfood `build-shaped-artifact` helper. They now see the original `Set` / `Map` instead of `[]`. They guard with `Array.isArray` or count through `resolveStackCollection`, so the content is not counted either before or after. Nothing is widened. ## Tests New `packages/spec/src/shared/metadata-collection-non-plain-object.test.ts`: - Unit tests: `normalizeMetadataCollection` returns a Set, Map, Date or class instance unchanged (same reference). Controls: an object literal, a null-prototype object and a cross-realm plain object are still normalized. - Envelope tests: for every key in `MAP_SUPPORTED_FIELDS` (derived from the list, 21 keys) and each of the four non-plain shapes, strict `defineStack` refuses with `code: 'STACK_SCHEMA_INVALID'` and `status: 422`, plus a zod issue `invalid_type` at `[key]` with `expected: 'array'`. Control: `permissions` written as a plain-object map is accepted with its entry. Runs at head `a4c6b1c5ad`: - targeted: `vitest run --project local` on the new file plus `metadata-collection.test.ts`: 2 files, 138 tests passed. - full spec suite: `pnpm --filter @objectstack/spec exec vitest run --project local`: 519 files, 15309 passed, 1 todo. - `pnpm --filter @objectstack/spec typecheck`: exit 0. `build` then `check:generated`: all 15 generated artifacts up to date. - **Ablation** (fix committed first, through `scripts/ablation-replace.mjs`): the anchor `if (isPlainObject(value)) {` was replaced by `if (typeof value === 'object') {`. The anchor count went 1 to 0, the replacement count 0 to 1, and the blob `f5f0f9d131` became `88a88c8d96`. Result: **88 failed / 50 passed**. The 88 failures are 84 envelope cases (21 keys times 4 shapes) and 4 unit cases, and every control stayed green. Restore: the blob equals HEAD and `git diff HEAD` is empty. The test imports `src/` directly, so no `dist` leg is involved. - Gates: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran` reports 83 derived, 81 run with exit 0, 2 NOT MEASURED. Both are `check:dual-build-cjs-loads` and `check:type-check-debt`, which exited 3 with PREREQUISITE NOT MET: they need the whole workspace built, and CI runs them. `check:doc-formula-expressions` and `check:lean-entry-closure` first exited 3; they went green after building `formula`, `lint` and `objectql`. - eslint, narrowed to the 2 changed files (`--no-inline-config --format json`): 2 files, 0 errors, 0 warnings. `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`), so this diff cannot change the verdict for any untouched file. The full `pnpm lint` run is CI's. ## Changeset `.changeset/19796-map-form-non-plain-object.md`: `minor`, BREAKING, ADR-0087 `not-required (no-migration-prescription)`. A `Set`, `Map` or class instance is not a serializable metadata document, so there is nothing for `migrate meta` to rewrite. ## Acceptance notes - The refusal text is the error map's generic `Expected array but received object.` A Set-specific prescription (for example, naming the constructor) would change `shared/error-map.zod.ts`, which is outside this card's file surface, so it is not done here. The key and the envelope are already exact. - `normalizePluginMetadata`: when the alias value (for example `triggers`) is not an array after normalization, the alias key is deleted without merging. This is true both before and after this change, and for any non-collection value. The function has no callers outside `packages/spec` (consumer: none). This is an observation only, and no card is filed. - A class instance used as a map is no longer read as a map of its own fields. This is part of the same narrowing: only a plain object is the declared map form (`MetadataCollectionInput` is `T[] | Record`). --- _Generated by [Claude Code](https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #19784
Clause-②: no (narrowing)
Implements follow-up C of ruling
5690859601(on #18239, batch #139 item 2, letter B, maintainer 「同意」): 「A composed artifact is complete or it is refused」.composeStacksstep 3 (theconcatpass) now refuses a stack whose value for a concatenated collection key is not an array. It uses the envelope step 2 already raises for a non-arrayobjects(landed in #19783). Before this change the pass kept only the array values, printed a one-timeconsole.warn, and composed an artifact without that stack's grants, seed rows, views and so on.The measurement, per key (the card asked for each key to be re-measured before widening)
Probe at base
3f9e2eaa1c, before the fix: stack A declares the key as[{ name: 'a_item' }]. Stack B is hand-built and declares the same key as a map{ b_item: { name: 'b_item' } }. The probe composes[A, B]and reads whether B's content reaches the composed artifact. It ran undermanifest: 'last'and again undermanifest: 'preserve'.datasources,datasourceMapping,translations,objectExtensions,apps,views,viewItems,pages,dashboards,reports,datasets,actions,flows,jobs,emailTemplates,docs,books,positions,permissions,capabilities,sharingRules,apis,webhooks,agents,tools,skills,hooks,mappings,analyticsCubes,connectors,data,requires,tiers(33)last: absent.preserve: present only inside B's package body, carrying the malformed value, so the top level and the package list disagreepackages,plugins,devPlugins,devLogins(4, excluded from the package body)All 37
concatkeys read YES and none reads ambiguous, so the refusal covers every one. The key list is derived fromCOMPOSE_KEY_DISPOSITIONSin the code and in the test, never transcribed, so a key added to the table is covered the day it lands. A non-object entry inside an array is out of scope, as #18239 already measured: it is concatenated as-is and the content is unchanged. The new test pins this.What changed (
packages/spec/src/stack.zod.ts)undefinedmeans the key is absent and the stack is skipped. Any other non-array value (map, number, string,null,false,Set) throwsStackSchemaInvalidErrorwithcode: 'STACK_SCHEMA_INVALID'andstatus: 422. Itsissuescarries one zod issue withpathrooted at the key,code: 'invalid_type'andexpected: 'array'. The message startscomposeStacks validation failed:, names the stack by manifest id and position, and names the key. For a keydefineStackaccepts in the map form (MAP_SUPPORTED_FIELDS), the message adds the parenthetical theobjectsrefusal carries. No new error code and no new export: the code the strict parse raises for the same authored mistake is reused, soClause-②: noholds.objectsrefusal moved intodescribeNonArrayCollection(key, value), and both raise sites use it. Theobjectsmessage is byte-identical, andcompose-stacks-objects-shape-refusal.test.tsis green unchanged.collectSeedDataObjectErrorsandcollectPermissionGrantObjectErrorswarned and returned on a non-arraydataorpermissions. After this change that branch has no reachable caller.defineStackcalls the collectors only after the strict parse, which rejects the shape, andcomposeStackscalls them in step 3b, after step 3 has refused the shape. The guards stay as silent type guards, so each rule never depends on its caller's order, and the docblocks now say that. The warn calls are gone.warnMalformedCollectionKeybecomeswarnMalformedCollectionEntry. With both of its'value'callers gone, the non-array-value sentence was dead code. The helper now carries only the entry notice (its one caller ismergeObjects, for a non-objectobjectsentry), deduplicated per key, and its printed text is unchanged.Hunks stay out of
mergeActionsIntoObjects(the sibling #19785 edit) and out ofpreservePackageEntriesand the options schema (open PR #19666). The one step-3a comment line touched says "refusal" where it said "warning".origin/main628e55dfa6was merged before opening; no conflicts.Fixture triage (the rule's consumer radius)
compose-stacks-key-loss.test.ts: "warns rather than skipping a collection key that holds a non-array value" pinned exactly the warn-and-drop branch this removes. The case is replaced in place and now asserts the refusal (code+status+ the key in the message).stack-artifact-crossref.test.ts: the two cases "a non-arraypermissions/datacomposes, and the key is warned about exactly once" are replaced with refusal assertions (code+status), and the block header is rewritten. The entry-shape cases in the same block are unchanged and green.test-typecheck-debt.json: re-recorded, because the replaced key-loss case dropped two implicit-any callback parameters (debt 4 to 3 and 3 to 2, shrink only).packages/spec, no test relies on the skip-and-warn text (a grep for the warning's phrasing acrosspackages/**tests found none). The public face is byte-unchanged (check:api-surfacegreen), so no importer owes a test.Tests and evidence (at
0bf2e55646)packages/spec/src/compose-stacks-concat-shape-refusal.test.tshas 125 cases: per key, the refusal (map value), the refusal underpreserve, and the array control that composes both stacks' entries. Onpermissionsanddatait also covers number, string,null,falseandSet, in both positions. It adds the absent-key control, the entry-carried control and the strict-door same-code pin.node scripts/ablation-replace.mjs: the refusal was replaced bycontinue(the anchor hit 1 time and went 1 to 0, the marker went 0 to 1, blob9b46ea3999bbtoed55f104c3e1). Result:Tests 84 failed | 41 passed (125), which is 37 keys times 2 refusal cases plus 10 shape rows red, with all 41 controls green. Restore: blob equal to HEAD andgit diff HEADempty. The test imports./stack.zodfromsrc, so there was nodistleg.pnpm --filter @objectstack/spec test:Test Files 518 passed (518),Tests 15213 passed | 1 todo.pnpm --filter @objectstack/spec typecheck: exit 0, after the debt re-record.pnpm --filter @objectstack/spec check:generated: "All 15 generated artifacts are up to date", with dist built from this tree.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran: "83 derived famil(ies) accounted for — 80 run, 3 NOT-MEASURED". All 80 that ran exit 0.check:dual-build-cjs-loads, reason: needs a fullpnpm build, and that prerequisite was not met here.check:type-check-debt, reason: needs the full./packages/*build closure, and that prerequisite was not met here.check:lean-entry-closure, reason: needs a built@objectstack/objectql; the build hit a lock queue-timeout (99) behind a long-running holder. All three are left to CI.Acceptance notes
normalizeMetadataCollection) reads any object throughObject.entries. As a result, strictdefineStack({ …, permissions: new Set([{…}]) })(or aMap) is accepted withpermissionsequal to[], and the grants are silently gone before the parse ever sees them. It is reported to the seat for filing and not touched here: it sits upstream of composition and is a different seam.nullandfalsecarry no content, but they are refused for parity with theobjectsarm (undefinedalone means absent). The strict parse rejects them too.Generated by Claude Code