Skip to content

Commit 3653b81

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-19785-merge-actions-shape-refusal
2 parents c5a865c + 0b83e01 commit 3653b81

7 files changed

Lines changed: 323 additions & 103 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
fix(spec): `composeStacks` refuses a stack whose value for a concatenated collection (`permissions`, `data`, `views`, …) is not an array, with the ADR-0112 envelope
6+
7+
**BREAKING** — `composeStacks`, a public root export, now refuses a class of input it used to compose with that stack's content silently missing.
8+
9+
Step 3 of `composeStacks` concatenates every collection key the composer declares `concat` (`permissions`, `data`, `apps`, `views`, `flows`, `agents`, `packages`, … — every `'concat'` row of `COMPOSE_KEY_DISPOSITIONS`). It kept only the array values and announced the rest with a one-time `console.warn`. The strict `defineStack` parse already rejects a non-array value for any of these keys, so the reachable population is an input that bypassed it — a hand-built stack object, or `defineStack(config, { strict: false })`. Measured before this change, per key, composing a well-formed stack with one whose value for the key is a map:
10+
11+
| the second stack's value | before | after |
12+
| :--- | :--- | :--- |
13+
| a map, a number, a string, `null`, `false`, a `Set` | composed; the composed collection lacks every entry of that stack (e.g. its permission-set grants, its seed rows), one `console.warn` | refused, `STACK_SCHEMA_INVALID`, `status: 422` |
14+
| the same, under `manifest: 'preserve'` | composed; the top-level collection lacks the entries, while that stack's package body still carries the malformed value — the artifact disagrees with itself | refused, `STACK_SCHEMA_INVALID`, `status: 422` |
15+
16+
A composed artifact is complete or it is refused, so no non-array value is skipped. An absent key (`undefined`) is not malformed and composes as before. The refusal is the one `composeStacks` already raises for a non-array `objects`: the code the strict parse raises for the same authored mistake, the zod issue on `issues` (`path` rooted at the key, `expected: 'array'`), and a message naming the stack by manifest id and position and the key. For a key `defineStack` accepts in the map form, the message says so. A non-object entry inside an array is still concatenated as-is — the entry is carried, not lost.
17+
18+
The one-line fix: author the key as an array, or pass the stack through strict `defineStack` (which normalizes the map form and rejects every other shape where it is written).
19+
20+
No code is added to the ADR-0112 ledger and no export changes: `STACK_SCHEMA_INVALID` is already registered under `@objectstack/spec`, and the error class stays module-local.
21+
22+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable moves: no spec key, Zod schema, export, config field or stored metadata shape is added, removed, renamed or re-spelled. The strict defineStack parse already refused every input this refuses, so an authored stack that passed its schema composes exactly as before; what narrows is the runtime behaviour of composeStacks on inputs that bypassed that parse, and objectstack migrate meta has no document to rewrite for it. -->
23+
24+
Clause-②: no (narrowing)

‎.claude/skills/pm-dispatch/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ PM 的工作是循环:选卡 → 认领 → 派发 → 收集 → 复核 → 报
263263
- 表未覆盖的包首次分诊时归类并走 PR 更新本表;新增或退役 `domain:*` 必须同批改本表。
264264
- `domain:*` 只由分诊席产出,例外仅在飞卡衍生 sub-issue 承父卡域;无标签的卡 ⛔ 不得认领。
265265
- 跨域例外路径:拆不动的跨域单 PR 由分诊席指定一个车道 PM 认领,认领评论申报文件面。
266-
- 派发前在飞检查:读本车道 `pm:dispatched` 卡认领评论申报的文件面,本地求交、不另发查询。
266+
- 派发前在飞检查:读 `pm:dispatched` 卡认领评论申报的文件面,本地求交、不另发查询。
267267
- 跨域例外路径的范围加上该单文件面触及各域的在飞单;相交 ⇒ 排该 PR 之后串行或让行。
268268
- 跨域相交只经三条越界通道:跨域例外路径、认领席跟到底的越界面、接手无主阻塞项。
269269
- 合并队列是共享串行资源:车道买到的是并行编写,不是并行落地。
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `composeStacks` step 3 (the `concat` pass) refuses a stack whose value for a
5+
* concatenated collection key is not an array (#19784 — ruling C, the
6+
* follow-up the #18239 ruling routed here).
7+
*
8+
* ## What was wrong
9+
*
10+
* Step 3 kept only the array values it found for each `concat` key and
11+
* announced the rest with a one-time `console.warn`. So a stack reaching
12+
* composition with `permissions: { … }` or `data: 42` (a hand-built stack
13+
* object, or `defineStack(config, { strict: false })`) composed "successfully"
14+
* into an artifact that simply lacked that stack's grants or seed rows — and
15+
* the same for every other concatenated key. Under `manifest: 'preserve'` the
16+
* dropped value survived only inside that stack's package body, so the
17+
* artifact's top level and its package list disagreed.
18+
*
19+
* ## What is pinned
20+
*
21+
* The ruling: 「A composed artifact is complete or it is refused」. Every key the
22+
* composer concatenates (derived from `COMPOSE_KEY_DISPOSITIONS`, never
23+
* transcribed — a key added to the table is covered the day it lands) refuses a
24+
* non-array value with step 2's envelope for the same defect on `objects`:
25+
* `STACK_SCHEMA_INVALID`, `status: 422`, one zod issue rooted at the key.
26+
*
27+
* Every refusal has its CONTROL: the same composition with the key authored as
28+
* an array is accepted with both stacks' content, so no refusal can satisfy
29+
* the assertions for the wrong reason.
30+
*/
31+
import { describe, it, expect, vi, afterEach } from 'vitest';
32+
import { composeStacks, defineStack, COMPOSE_KEY_DISPOSITIONS, type ObjectStackDefinition } from './stack.zod';
33+
34+
type Envelope = Error & {
35+
code?: string;
36+
status?: number;
37+
issues?: ReadonlyArray<{ code?: string; path?: readonly PropertyKey[]; expected?: string }>;
38+
};
39+
40+
/** The thrown value, or `null` when the composition is accepted. */
41+
function refusal(fn: () => unknown): Envelope | null {
42+
try {
43+
fn();
44+
return null;
45+
} catch (e) {
46+
return e as Envelope;
47+
}
48+
}
49+
50+
const mf = (id: string) => ({ id, name: id.split('.').pop()!, version: '1.0.0', type: 'app' as const });
51+
52+
/** A stack object nobody parsed. */
53+
const handBuilt = (overrides: Record<string, unknown>) => overrides as unknown as ObjectStackDefinition;
54+
55+
/** A stack through `defineStack`'s `strict: false` door, which skips the parse. */
56+
const unparsed = (overrides: Record<string, unknown>) => defineStack(overrides as never, { strict: false });
57+
58+
const CONCAT_KEYS = Object.entries(COMPOSE_KEY_DISPOSITIONS)
59+
.filter(([, rule]) => rule === 'concat')
60+
.map(([key]) => key);
61+
62+
const B1 = "'com.example.b' (stack #1)";
63+
64+
afterEach(() => {
65+
vi.restoreAllMocks();
66+
});
67+
68+
describe('#19784 — composeStacks refuses a non-array concatenated collection', () => {
69+
it('covers every concat key the composer declares (the table is the census, not a transcription)', () => {
70+
expect(CONCAT_KEYS).toContain('permissions');
71+
expect(CONCAT_KEYS).toContain('data');
72+
expect(CONCAT_KEYS.length).toBeGreaterThanOrEqual(30);
73+
});
74+
75+
for (const key of CONCAT_KEYS) {
76+
describe(`'${key}'`, () => {
77+
const a = () => handBuilt({ manifest: mf('com.example.a'), [key]: [{ name: 'a_item' }] });
78+
const b = (value: unknown) => handBuilt({ manifest: mf('com.example.b'), [key]: value });
79+
80+
it('a map-shaped value is refused with STACK_SCHEMA_INVALID / 422 — never skipped', () => {
81+
const refused = refusal(() => composeStacks([a(), b({ b_item: { name: 'b_item' } })]));
82+
expect(refused).toBeInstanceOf(Error);
83+
expect(refused?.code).toBe('STACK_SCHEMA_INVALID');
84+
expect(refused?.status).toBe(422);
85+
expect(refused?.issues).toHaveLength(1);
86+
expect(refused?.issues?.[0]?.path).toEqual([key]);
87+
expect(refused?.issues?.[0]?.code).toBe('invalid_type');
88+
expect(refused?.issues?.[0]?.expected).toBe('array');
89+
expect(refused?.message.startsWith(`composeStacks validation failed: ${B1} declares '${key}'`)).toBe(true);
90+
});
91+
92+
it("is refused under `manifest: 'preserve'` too", () => {
93+
const refused = refusal(() =>
94+
composeStacks([a(), b({ b_item: { name: 'b_item' } })], { manifest: 'preserve' }),
95+
);
96+
expect(refused?.code).toBe('STACK_SCHEMA_INVALID');
97+
expect(refused?.status).toBe(422);
98+
});
99+
100+
it('the control — the same key authored as an array composes the entries of BOTH stacks', () => {
101+
const composed = composeStacks([a(), b([{ name: 'b_item' }])]) as Record<string, unknown>;
102+
expect(composed[key]).toEqual([{ name: 'a_item' }, { name: 'b_item' }]);
103+
});
104+
});
105+
}
106+
107+
/** The value shapes, on the two keys the ruling's measurement named. */
108+
// A Set rides a hand-built stack only: `defineStack`'s map-form normalizer
109+
// reads any object through `Object.entries`, so a Set entering through the
110+
// `strict: false` door arrives here already flattened to `[]` — upstream of
111+
// composition, and not this pass's finding.
112+
const shapes: Array<{ label: string; value: () => unknown; door: typeof handBuilt }> = [
113+
{ label: 'a number, through `strict: false`', value: () => 42, door: unparsed },
114+
{ label: 'a string, through `strict: false`', value: () => 'not-an-array', door: unparsed },
115+
{ label: 'null, through `strict: false`', value: () => null, door: unparsed },
116+
{ label: 'false, through `strict: false`', value: () => false, door: unparsed },
117+
{ label: 'a Set of entries, on a hand-built stack', value: () => new Set([{ name: 'b_item' }]), door: handBuilt },
118+
];
119+
for (const key of ['permissions', 'data']) {
120+
for (const shape of shapes) {
121+
it(`'${key}' as ${shape.label} is refused, whichever position it holds`, () => {
122+
const good = () => handBuilt({ manifest: mf('com.example.a'), [key]: [{ name: 'a_item' }] });
123+
const bad = () => shape.door({ manifest: mf('com.example.b'), [key]: shape.value() });
124+
for (const run of [() => composeStacks([good(), bad()]), () => composeStacks([bad(), good()])]) {
125+
const refused = refusal(run);
126+
expect(refused?.code).toBe('STACK_SCHEMA_INVALID');
127+
expect(refused?.status).toBe(422);
128+
expect(refused?.issues?.[0]?.path).toEqual([key]);
129+
}
130+
});
131+
}
132+
}
133+
134+
it('an ABSENT key is not a malformed one — composed from the stacks that declare it, silently', () => {
135+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
136+
const composed = composeStacks([
137+
handBuilt({ manifest: mf('com.example.a'), permissions: [{ name: 'a_item' }] }),
138+
handBuilt({ manifest: mf('com.example.b'), permissions: undefined }),
139+
]);
140+
expect(composed.permissions).toEqual([{ name: 'a_item' }]);
141+
expect(warn).not.toHaveBeenCalled();
142+
});
143+
144+
it('a non-object ENTRY inside an array is carried as-is — the composed content is unchanged', () => {
145+
const composed = composeStacks([
146+
handBuilt({ manifest: mf('com.example.a'), permissions: [{ name: 'a_item' }] }),
147+
handBuilt({ manifest: mf('com.example.b'), permissions: [null, { name: 'b_item' }] }),
148+
]);
149+
expect(composed.permissions).toEqual([{ name: 'a_item' }, null, { name: 'b_item' }]);
150+
});
151+
152+
it('the strict door raises the SAME code for the same defect — one dialect for one authored mistake', () => {
153+
const refused = refusal(() => defineStack({ manifest: mf('com.example.b'), permissions: 42 } as never));
154+
expect(refused?.code).toBe('STACK_SCHEMA_INVALID');
155+
expect(refused?.status).toBe(422);
156+
});
157+
});

‎packages/spec/src/compose-stacks-key-loss.test.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,17 +188,22 @@ describe('#5005 rule 3 — a key with no declared rule warns', () => {
188188
expect(warnSpy.mock.calls.map((c) => String(c[0])).some((w) => w.includes("'futureList'"))).toBe(true);
189189
});
190190

191-
it('warns rather than skipping a collection key that holds a non-array value', () => {
191+
it('refuses — never skips — a collection key that holds a non-array value (#19784)', () => {
192+
// Skipping it (the #5005 warn-and-drop) composed an artifact that silently
193+
// lacked stack B's views; the full per-key census lives in
194+
// `compose-stacks-concat-shape-refusal.test.ts`.
192195
const a = raw({ manifest: manifestA, views: [{ name: 'v1' }] });
193196
const b = raw({ manifest: manifestB, views: 'not-an-array' });
194197

195-
const composed = composeStacks([a, b]);
196-
expect(composed.views).toHaveLength(1);
197-
expect(
198-
warnSpy.mock.calls
199-
.map((c) => String(c[0]))
200-
.some((w) => w.includes('composeStacks') && w.includes("'views'") && w.includes('cannot be composed')),
201-
).toBe(true);
198+
let thrown: (Error & { code?: string; status?: number }) | undefined;
199+
try {
200+
composeStacks([a, b]);
201+
} catch (e) {
202+
thrown = e as Error & { code?: string; status?: number };
203+
}
204+
expect(thrown?.code).toBe('STACK_SCHEMA_INVALID');
205+
expect(thrown?.status).toBe(422);
206+
expect(thrown?.message).toContain("'views'");
202207
});
203208

204209
it('does NOT warn for keys the composer has a declared rule for', () => {

‎packages/spec/src/stack-artifact-crossref.test.ts‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -396,18 +396,18 @@ describe('#18202 — an input that bypassed the strict parse IS checked at compo
396396
* The shape guard the two collectors carry (#18202 rework).
397397
*
398398
* Because the artifact pass reads `permissions` / `data` off inputs the strict
399-
* parse never saw, those keys can be a non-array, and an entry can be `null` or
400-
* a scalar. `composeStacks`'s step-3 concat pass already refuses to drop such a
401-
* key without a word (#5005); the two collectors must not turn the same input
402-
* into a bare `TypeError` with no `code` and no `status`, which is exactly what
403-
* this pass did before the guards existed. Every case below composes on
404-
* `origin/main`, so a throw here is a regression, not a stricter contract.
399+
* parse never saw, an entry can be `null` or a scalar; the two collectors must
400+
* not turn such an input into a bare `TypeError` with no `code` and no
401+
* `status`, which is exactly what this pass did before the guards existed.
402+
* Every entry-shape case below composes, so a throw there is a regression, not
403+
* a stricter contract.
405404
*
406-
* ⚠️ `warnMalformedCollectionKey` deduplicates per key for the lifetime of the
407-
* module, so each key is asserted in exactly ONE test and the count assertion
408-
* (`toBe(1)`) is what proves the two passes do not both speak.
405+
* A non-array VALUE for either key is a different finding: since #19784
406+
* `composeStacks`'s step-3 concat pass REFUSES it with the ADR-0112 envelope
407+
* before this pass runs — skipping it composed an artifact without that
408+
* stack's grants or seed rows — so those two cases assert the refusal.
409409
*/
410-
describe('#18202 — a malformed collection on an unparsed input is skipped, never a bare TypeError', () => {
410+
describe('#18202 — a malformed collection on an unparsed input is skipped or refused, never a bare TypeError', () => {
411411
/** Collect `console.warn` for one call, restoring the real one afterwards. */
412412
function warningsDuring(run: () => unknown): { warnings: string[]; thrown: Envelope | null } {
413413
const warnings: string[] = [];
@@ -428,21 +428,21 @@ describe('#18202 — a malformed collection on an unparsed input is skipped, nev
428428
const composeWith = (stack: ReturnType<typeof defineStack>) => () =>
429429
composeStacks([serviceStack(), stack], { manifest: 'preserve' });
430430

431-
it('a non-array `permissions` composes, and the key is warned about exactly once', () => {
432-
// Map format, which `permissions` does not support — the shape a
433-
// hand-built stack most plausibly carries. It is NOT iterable, which is
434-
// what makes this the case that distinguishes the guard: a string value
435-
// would iterate its characters and never throw either way.
431+
it('a non-array `permissions` is refused by the concat pass, never a bare TypeError (#19784)', () => {
432+
// Map format on a hand-built stack (only `defineStack` normalizes it). It
433+
// is NOT iterable, which is what makes this the case that distinguishes a
434+
// guard from a bare `TypeError`: a string value would iterate its
435+
// characters and never throw either way.
436436
const mapShaped = { sales_rep: { label: 'Sales Rep', objects: { [NOWHERE]: { allowRead: true } } } };
437-
const { warnings, thrown } = warningsDuring(composeWith(malformed({ permissions: mapShaped })));
438-
expect(thrown).toBeNull();
439-
expect(warnings.filter((w) => w.includes("top-level key 'permissions'"))).toHaveLength(1);
437+
const { thrown } = warningsDuring(composeWith(malformed({ permissions: mapShaped })));
438+
expect(thrown?.code).toBe('STACK_SCHEMA_INVALID');
439+
expect(thrown?.status).toBe(422);
440440
});
441441

442-
it('a non-array `data` composes, and the key is warned about exactly once', () => {
443-
const { warnings, thrown } = warningsDuring(composeWith(malformed({ data: 42 })));
444-
expect(thrown).toBeNull();
445-
expect(warnings.filter((w) => w.includes("top-level key 'data'"))).toHaveLength(1);
442+
it('a non-array `data` is refused by the concat pass, never a bare TypeError (#19784)', () => {
443+
const { thrown } = warningsDuring(composeWith(malformed({ data: 42 })));
444+
expect(thrown?.code).toBe('STACK_SCHEMA_INVALID');
445+
expect(thrown?.status).toBe(422);
446446
});
447447

448448
it('a null entry inside `permissions` is skipped, not dereferenced', () => {

0 commit comments

Comments
 (0)