Skip to content

fix(spec): composeStacks refuses a non-array concatenated collection instead of dropping its content - #19794

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-19784-concat-fields-refusal
Sep 23, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-19784-concat-fields-refusal

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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」. composeStacks step 3 (the concat pass) 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-array objects (landed in #19783). Before this change the pass kept only the array values, printed a one-time console.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 under manifest: 'last' and again under manifest: 'preserve'.

keys composed top-level collection anywhere in the artifact reading
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) B absent, A present, one warn last: absent. preserve: present only inside B's package body, carrying the malformed value, so the top level and the package list disagree YES: content changes
packages, plugins, devPlugins, devLogins (4, excluded from the package body) B absent, A present, one warn absent in both modes YES: content changes

All 37 concat keys read YES and none reads ambiguous, so the refusal covers every one. The key list is derived from COMPOSE_KEY_DISPOSITIONS in 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)

  1. Step 3 (the ruled change). For each concat key and each stack: undefined means the key is absent and the stack is skipped. Any other non-array value (map, number, string, null, false, Set) throws StackSchemaInvalidError with code: 'STACK_SCHEMA_INVALID' and status: 422. Its issues carries one zod issue with path rooted at the key, code: 'invalid_type' and expected: 'array'. The message starts composeStacks validation failed:, names the stack by manifest id and position, and names the key. For a key defineStack accepts in the map form (MAP_SUPPORTED_FIELDS), the message adds the parenthetical the objects refusal carries. No new error code and no new export: the code the strict parse raises for the same authored mistake is reused, so Clause-②: no holds.
  2. Shared helper. The value-description and zod-issue half of the objects refusal moved into describeNonArrayCollection(key, value), and both raise sites use it. The objects message is byte-identical, and compose-stacks-objects-shape-refusal.test.ts is green unchanged.
  3. What happens to fix(spec): resolve permission-set and seed object references against the release artifact, not the single stack #18212's step-3b collectors. collectSeedDataObjectErrors and collectPermissionGrantObjectErrors warned and returned on a non-array data or permissions. After this change that branch has no reachable caller. defineStack calls the collectors only after the strict parse, which rejects the shape, and composeStacks calls 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.
  4. warnMalformedCollectionKey becomes warnMalformedCollectionEntry. 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 is mergeObjects, for a non-object objects entry), deduplicated per key, and its printed text is unchanged.

Hunks stay out of mergeActionsIntoObjects (the sibling #19785 edit) and out of preservePackageEntries and the options schema (open PR #19666). The one step-3a comment line touched says "refusal" where it said "warning". origin/main 628e55dfa6 was 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-array permissions / data composes, 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).
  • Outside packages/spec, no test relies on the skip-and-warn text (a grep for the warning's phrasing across packages/** tests found none). The public face is byte-unchanged (check:api-surface green), so no importer owes a test.

Tests and evidence (at 0bf2e55646)

  • New packages/spec/src/compose-stacks-concat-shape-refusal.test.ts has 125 cases: per key, the refusal (map value), the refusal under preserve, and the array control that composes both stacks' entries. On permissions and data it also covers number, string, null, false and Set, in both positions. It adds the absent-key control, the entry-carried control and the strict-door same-code pin.
  • Ablation, run with node scripts/ablation-replace.mjs: the refusal was replaced by continue (the anchor hit 1 time and went 1 to 0, the marker went 0 to 1, blob 9b46ea3999bb to ed55f104c3e1). 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 and git diff HEAD empty. The test imports ./stack.zod from src, so there was no dist leg.
  • 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.
    • NOT MEASURED: check:dual-build-cjs-loads, reason: needs a full pnpm build, and that prerequisite was not met here.
    • NOT MEASURED: check:type-check-debt, reason: needs the full ./packages/* build closure, and that prerequisite was not met here.
    • NOT MEASURED: 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

  • Out-of-scope finding (class a, not fixed here): the map-form normalizer (normalizeMetadataCollection) reads any object through Object.entries. As a result, strict defineStack({ …, permissions: new Set([{…}]) }) (or a Map) is accepted with permissions equal 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.
  • null and false carry no content, but they are refused for parity with the objects arm (undefined alone means absent). The strict parse rejects them too.

Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 11 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/spec/test-typecheck-debt.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/getting-started/examples.mdx (via composeStacks (symbol, a top-level function))
  • content/docs/getting-started/glossary.mdx (via composeStacks (symbol, a top-level function))

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

  • content/docs/releases/v17/17-3.mdx (via composeStacks (symbol, a top-level function))
  • content/docs/releases/v17/17-4.mdx (via composeStacks (symbol, a top-level function))
  • content/docs/releases/v17/index.mdx (via composeStacks (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
  • 1 changed file(s) yielded no anchor (packages/spec/test-typecheck-debt.json) — pages documenting those are invisible to this run
  • 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 136 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 fae870352ea59ddfb1c6dfd784bc6552cf158211 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2f0ef75a5793deb4e27d4026fdb668d10497b6f5 — the merge of head 0bf2e5564619cd3e4d5b36541c66850205ffa2a5 into base fae870352ea59ddfb1c6dfd784bc6552cf158211, 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 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

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 0bf2e5564619cd3e4d5b36541c66850205ffa2a5

① Derived judgments

  • Per-key measurement — real, derived, complete; reproduced independently: in an isolated worktree at merge-base 628e55dfa6 with the base stack.zod.ts, iterating COMPOSE_KEY_DISPOSITIONS rows with rule concat at runtime — 37 keys × (last, preserve) = 74 rows. Base: every row composes with B's content absent from the composed top level (under preserve, it survives only inside B's package body for 33 keys; absent everywhere for packages, plugins, devPlugins, devLogins). Head: all 74 throw STACK_SCHEMA_INVALID / 422, one invalid_type issue at the key, expected: 'array'. No key where skipping is not content loss ⇒ no over-reach.
  • undefined the only pass-through (:4540, and :4045 for objects) — seat re-read; null / false refused; every concat key is z.array(…).optional(), so the strict parse refuses the same set (pinned).
  • Same envelope as fix(spec): composeStacks refuses a non-array objects with an ADR-0112 envelope #19783, ⛔ no new code, no export change (only an import of the already-exported MAP_SUPPORTED_FIELDS); the map-form hint only for MAP_SUPPORTED_FIELDS keys.
  • Step-3b collectors' non-array branches unreachable — TRUE: two call sites only (validateCrossReferences after the strict parse succeeds; step 3b after step 3 now throws) — seat re-read :2614 / :2642 / :4427-4428.
  • warnMalformedCollectionKey → warnMalformedCollectionEntry: printed text byte-identical, old name 0 references (seat re-read); describeNonArrayCollection extraction leaves the objects message byte-identical (base vs head cmp equal for a plain object and a Set).
  • No accept-set widening anywhere. Flipped pins re-judged to assert the refusal (code + status + key), ⛔ not deleted; test-typecheck-debt.json shrink only.

② Semver level

minor + BREAKING + adr-0087: not-required (no-migration-prescription) + Clause-②: no (narrowing) — the #19783 pattern; prose true. Nit: the table's first row says non-map shapes were measured per key, but only on permissions / data (the map shape on all 37); the refusal path is shape-agnostic, so the claim holds by construction.

③ Boundary flags

Implemented-by: claude/issue-19784-concat-fields-refusal
Reviewed-by: session_01VWsFyWDp8Rjb2Ma6a3Cyo8

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 domain:spec seat 4 after re-reading three of its readings on the head. Checks at review time: 32 success, 3 path-filter / opt-in skips, 0 failed.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review September 23, 2026 06:17
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit 0b83e01 Sep 23, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-19784-concat-fields-refusal branch September 23, 2026 06:40
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…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>
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…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>
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

1 participant