Repository navigation
Commit 0b83e01
fix(spec): composeStacks refuses a non-array concatenated collection instead of dropping its content (#19794)
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 #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](https://claude.ai/code/session_01VWsFyWDp8Rjb2Ma6a3Cyo8)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 98d8336 commit 0b83e01
6 files changed
Lines changed: 322 additions & 102 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
Lines changed: 157 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
188 | 188 | | |
189 | 189 | | |
190 | 190 | | |
191 | | - | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
192 | 195 | | |
193 | 196 | | |
194 | 197 | | |
195 | | - | |
196 | | - | |
197 | | - | |
198 | | - | |
199 | | - | |
200 | | - | |
201 | | - | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
202 | 207 | | |
203 | 208 | | |
204 | 209 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
396 | 396 | | |
397 | 397 | | |
398 | 398 | | |
399 | | - | |
400 | | - | |
401 | | - | |
402 | | - | |
403 | | - | |
404 | | - | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
405 | 404 | | |
406 | | - | |
407 | | - | |
408 | | - | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
409 | 409 | | |
410 | | - | |
| 410 | + | |
411 | 411 | | |
412 | 412 | | |
413 | 413 | | |
| |||
428 | 428 | | |
429 | 429 | | |
430 | 430 | | |
431 | | - | |
432 | | - | |
433 | | - | |
434 | | - | |
435 | | - | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
436 | 436 | | |
437 | | - | |
438 | | - | |
439 | | - | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
440 | 440 | | |
441 | 441 | | |
442 | | - | |
443 | | - | |
444 | | - | |
445 | | - | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
446 | 446 | | |
447 | 447 | | |
448 | 448 | | |
| |||
0 commit comments