Repository navigation
Commit 9282578
fix(spec): composeStacks' action-key collision pass skips malformed actions, so step 7 refuses them with the envelope (#19903)
Fixes #19816
Clause-②: no
## What changed
`composeStacks` step 6, the cross-stack action-key collision pass
(`collectComposedActionKeyCollisions` in
`packages/spec/src/stack.zod.ts`), now reads only what is shaped like an
action:
- a top-level `actions` entry that is not an object (`isRecord` fails)
is skipped;
- a composed object's own `actions` that is not an array is skipped, and
so is a non-object entry inside one.
It throws nothing new. Every skipped input is still in the composed
artifact: the entry is in the `actions` that step 3 concatenated, and
the object is the same composed object. So step 7's existing guard in
`mergeActionsIntoObjects` refuses each one with the
`STACK_SCHEMA_INVALID` / `status: 422` envelope, at the composed
artifact's own zod path. The stale in-function comment ("the concat pass
drops it (and warns)") now says what step 3 has done since it gained its
guard: it refuses a non-array `actions`.
Landing site: only that function's region, plus one new test file and a
changeset. Step 7's guard and its message are read, not edited. No other
region of `stack.zod.ts` changes. No other package changes.
## Route: (a), skip and let step 7 refuse. Not (b), throw here
I measured before choosing: with the collision pass removed in-tree (a
wrapped `ablation-replace` mutation, restored and proven byte-equal to
HEAD), step 7 alone answered **every** malformed row below with the
envelope. So route (a) leaves one refusal, with one wording, for one
condition. Route (b) would add a second copy of step 7's message in this
pass for the same shapes. The fixed tree's answers are byte-for-byte
what the bypass probe printed for every malformed row. The only row that
differs is the real-collision control, which the bypass could not see by
construction.
(a) also fixes a misdiagnosis that (b) would have to handle separately.
Measured on `origin/main`: two stacks that each carried a non-object
entry the pass *could* read (`actions: ['x']`) were keyed
`global:undefined` and refused as `STACK_COMPOSE_ACTION_KEY_COLLISION`,
which is the wrong code for a schema defect. They are now
`STACK_SCHEMA_INVALID`.
Trade-off, stated rather than hidden: the index in a top-level issue
path is the entry's index in the **composed** `actions`, for example
`['actions', 1]` for stack B's first entry after stack A's one action.
It is not the input stack's local index. That is the coordinate step 7
has always used for this artifact. An object's own `actions` is named by
the object (`object 'b_item''s 'actions'`).
## Readings (hand-built stacks through `composeStacks`,
`packages/spec/src` via `tsx`)
Stack A is valid (one object, one global action). Stack B carries the
malformed shape.
| B's malformed shape | `origin/main` `1f89ba0d70` | step 6 bypassed
(same tree) | this PR |
| :-- | :-- | :-- | :-- |
| top-level `actions: [null]` | `TypeError` reading `objectName`,
code/status `undefined` | `STACK_SCHEMA_INVALID` 422 at `['actions', 1]`
| same as bypass |
| top-level `actions: [undefined]` | `TypeError` reading `objectName` |
`STACK_SCHEMA_INVALID` 422 at `['actions', 1]` | same |
| top-level `[valid, null]` | `TypeError` reading `objectName` |
`STACK_SCHEMA_INVALID` 422 at `['actions', 2]` | same |
| top-level `['x']` / `[5]` / `[[]]` | `STACK_SCHEMA_INVALID` 422
(already reached step 7) | same | same |
| `['x']` in **both** stacks | `STACK_COMPOSE_ACTION_KEY_COLLISION` on
`global:undefined` | `STACK_SCHEMA_INVALID` 422 at `['actions', 0]`,
`['actions', 1]` | same as bypass |
| object `actions: 5` / `'abc'` / `{}` / `true` | `TypeError:
(obj.actions ?? []).entries is not a function` | `STACK_SCHEMA_INVALID`
422 at `['objects', 1, 'actions']` | same |
| object `actions: null` / a `Map` | `STACK_SCHEMA_INVALID` 422 (already
reached step 7) | same | same |
| object `actions: [null]` / `[undefined]` | `TypeError` reading `name`
| `STACK_SCHEMA_INVALID` 422 at `['objects', 1, 'actions', 0]` | same |
| object `actions: 5`, `objectConflict: 'override'` | `TypeError …
entries is not a function` | `STACK_SCHEMA_INVALID` 422 | same |
| object `actions: [null]`, `objectConflict: 'merge'` | `TypeError`
reading `name` | `STACK_SCHEMA_INVALID` 422 | same |
| CONTROL: valid pair | accepted | accepted | accepted |
| CONTROL: real `global:a_home` collision |
`STACK_COMPOSE_ACTION_KEY_COLLISION` | accepted (pass removed) |
`STACK_COMPOSE_ACTION_KEY_COLLISION` |
## Tests
New file
`packages/spec/src/compose-stacks-action-collision-shape.test.ts`, 19
cases. Each malformed shape above goes through `composeStacks`. The
assertions: not a `TypeError`; `code` `STACK_SCHEMA_INVALID`; `status`
422; the exact issue paths with `invalid_type` / `expected`; and the
message names the location (`'actions'` plus `#index`, or `object
'b_item'`). The message wording itself is not pinned. The controls: a
valid pair composes; a real cross-stack collision is still
`STACK_COMPOSE_ACTION_KEY_COLLISION`; and the walk goes **on** past a
skipped entry. A collision declared right after a `null` entry is still
found at the declaring stack's own site (`stack.actions[1]`,
`objects['b_item'].actions[1]`).
**Firing control** (fix committed first, then `origin/main`'s
`stack.zod.ts` restored into the working tree only, then restored from
HEAD, with the blob hash proven equal to
`HEAD:packages/spec/src/stack.zod.ts` and `git diff HEAD` empty). On
today's file the new file goes **13 failed / 6 passed**. The 13 red are
every row that crashed (the assertion reads `expected TypeError: Cannot
read properties of null… to not be an instance of TypeError`), the
`global:undefined` misdiagnosis, and both walk-continues controls. The 6
green are the rows that already reached step 7 on `main` (`'x'`, `5`,
`[]` top-level, and `'x'` embedded) plus the valid-pair and
real-collision controls. Spec's vitest imports `./stack.zod` from `src`,
so no rebuild sits between the mutation and the run.
### Runs, all at this branch's head `91bc309849` (the final commit;
every figure below is from a run after it)
Every build and test ran through `scripts/pm/os-verify-lock.sh`, so its
wall-clock figures come from a shared box.
- `@objectstack/spec` build: exit 0. `check-dts-emitted` 34/34.
`authorable-surface.base.json` was not touched and `git status` stayed
clean.
- `@objectstack/spec` suite, `--project local`: **527 files / 15520
passed** (1 todo). `--project repo`: **35 files / 602 passed**.
`typecheck` (`tsc --noEmit`, the scripts program,
`check:test-typecheck`): exit 0. The new test file is inside the test
program (`tsc -p tsconfig.test.json --listFilesOnly` counts 1), and
`check:test-typecheck` reports no unledgered file.
- Downstream suites, meaning the **consumers** of `@objectstack/spec`.
These are the packages whose tests call `composeStacks` (found with `git
grep composeStacks` over `packages/**/*.test.ts`), not the whole
`...@objectstack/spec` consumer closure. They ran after a full `turbo
run build --filter='./packages/*' --filter='./packages/*/*'` (72/72
tasks):
- `@objectstack/metadata`: 53 files / 792 passed
- `@objectstack/plugin-security`: 120 files / 2289 passed
- `@objectstack/plugin-dev`: 8 files / 76 passed
- `@objectstack/runtime`: `--project local` 272 files / 3800 passed (1
skipped); `--project repo` 2 files / 69 passed
- `@objectstack/cli`: `--project unit` 222 files / 3141 passed. The 11
per-PR files that call `composeStacks`, across both tiers: 11 files /
118 passed. **Declared narrowing:** the rest of the `integration` tier
is left to CI's `pnpm test`. The two `*.e2e.test.ts` files that call
`composeStacks` (`build-multi-package-artifact`,
`build-package-docs-attachment`) are in the nightly `OS_TEST_TIERS`
tier, outside the per-PR population, and were not run here.
- `@objectstack/dogfood`: the one file that calls `composeStacks`
(`multi-package-artifact.dogfood.test.ts`), 1 file / 9 passed.
**Declared narrowing:** the rest of the dogfood suite belongs to CI's
Dogfood Regression Gate.
- Gates: `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derived 82 commands at
`91bc309849`, and all 82 ran with each exit code captured before any
pipe. Four exited 3 (PREREQUISITE NOT MET) on the first pass:
`check:doc-formula-expressions`, `check:dual-build-cjs-loads`,
`check:lean-entry-closure` and `check:type-check-debt`. All four were
rerun after the full build and exited 0. `--ran` reports: `82 derived
famil(ies) accounted for — 82 run, 0 NOT-MEASURED (a DERIVED zero — all
82 recorded an exit code and none of them is 3)`.
- ESLint, a proven narrowing rather than the repo-wide run (that belongs
to CI): `eslint --no-inline-config --format json` over the 2 changed
`.ts` files gives 2 files, 0 errors, 0 warnings. Neither file is
ignored, so both are in the population `eslint.config.mjs` lints. That
config never enables type-aware linting (no `parserOptions.project`, as
its own comment states), so this diff cannot move a verdict on any
untouched file.
## Changeset
`.changeset/19816-compose-action-collision-shape.md`:
`'@objectstack/spec': patch`. Derivation: `stack.zod.ts` ships in
`@objectstack/spec`'s `files[]` twice, as `dist` and as
`src/**/*.zod.ts`, so the fix publishes runtime bytes. It is a bug fix
in a released package, so it takes `patch`. `Clause-②: no` without an
arm: every input the pass now skips was a refusal before (a `TypeError`,
or the wrong-coded collision) and is a refusal now (the envelope). An
input that passed before contained no non-object entry in any array this
pass walks, because step 7 refuses those, so it behaves identically.
Skipping can only remove keys the pass would count, never add one, so no
acceptance is newly refused and no refusal is newly accepted.
## Acceptance notes
- **Step 7's wording for an `undefined` entry** reads `#1 (a undefined)`
(`a ${typeof entry}`). It is cosmetic and lives in
`mergeActionsIntoObjects`, which this card fences off, so it is
untouched here and not filed.
- **A record entry without a string `name`** (`{}`) still keys
`global:undefined` in this pass: two stacks each carrying `actions:
[{}]` are refused as `STACK_COMPOSE_ACTION_KEY_COLLISION`, and a single
one composes. `composeStacks` does not schema-validate action records
(step 7 guards shape only). Skipping name-less records here would turn
that refusal into an acceptance, a widening, so the line stays where the
card drew it: non-object entries and non-array `actions`. Noted, not
filed.
- **A `Symbol` `name`** still throws a bare `TypeError` (`Cannot convert
a Symbol value to a string`) from this pass's key template. JSON or YAML
cannot produce it, and type-checked TS cannot either. Noted, not filed.
- **Ordering**: a real collision that sits beside a malformed entry is
refused as the collision first, and step 7 refuses the malformed entry
once the collision is fixed. Both refusals are envelopes. The pin is the
walk-continues control above.
---
_Generated by [Claude
Code](https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent f7a3495 commit 9282578
3 files changed
Lines changed: 232 additions & 3 deletions
File tree
- .changeset
- packages/spec/src
| 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 | + | |
| 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 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4381 | 4381 | | |
4382 | 4382 | | |
4383 | 4383 | | |
| 4384 | + | |
| 4385 | + | |
| 4386 | + | |
| 4387 | + | |
| 4388 | + | |
| 4389 | + | |
| 4390 | + | |
| 4391 | + | |
| 4392 | + | |
| 4393 | + | |
4384 | 4394 | | |
4385 | 4395 | | |
4386 | 4396 | | |
| |||
4401 | 4411 | | |
4402 | 4412 | | |
4403 | 4413 | | |
4404 | | - | |
4405 | | - | |
| 4414 | + | |
| 4415 | + | |
4406 | 4416 | | |
4407 | 4417 | | |
4408 | 4418 | | |
| 4419 | + | |
| 4420 | + | |
4409 | 4421 | | |
4410 | 4422 | | |
4411 | 4423 | | |
| |||
4422 | 4434 | | |
4423 | 4435 | | |
4424 | 4436 | | |
4425 | | - | |
| 4437 | + | |
| 4438 | + | |
| 4439 | + | |
| 4440 | + | |
| 4441 | + | |
| 4442 | + | |
4426 | 4443 | | |
4427 | 4444 | | |
4428 | 4445 | | |
| |||
0 commit comments