Skip to content

Commit 147a0d1

Browse files
feat(spec)!: refuse a duplicate name inside one stack’s functions array (#22902) (#22931)
Fixes #22902 Clause-②: no (narrowing) > **Held out of the merge queue** by #22949 (the queue-wide suite-duration ceiling red, live since `1bd612bd6`; not this PR's). Accepted, reviewed at tier and green; the seat re-arms it when #22949 resolves. ## What this does `defineStack`'s cross-reference aggregate gains `collectDuplicateFunctionNameErrors`, a sibling of `collectDuplicateActionKeyErrors` in `packages/spec/src/stack.zod.ts`. When two or more entries of one stack's `functions` **array** author the same `name`, the stack is refused. There is one finding per colliding name, and it names the name and every index: ```text ✗ Function 'kc_a' is declared twice in the `functions` array: stack.functions[0] and stack.functions[1]. Functions are registered and resolved by name, so only one of these entries can own 'kc_a' and the others would be silently renamed or dropped. Rename one of them, or merge them into a single entry. ``` The refusal uses the existing envelope: `StackCrossReferenceError`, `code: 'STACK_CROSS_REFERENCE_INVALID'`, `status: 422`, one `issues` entry per finding. It runs before the object-count early return, so an object-less stack is refused too. Every entry counts, byte-identical twins included. Unchanged, per the triage fences: - The map form. It cannot carry a duplicate, and the collector never visits it. - The build. `objectstack build` and `lowerCallables` are not touched, so a suffix stays an internal ref. - The runtime. The cross-stack case stays `composeStacks`' own refusal (`STACK_COMPOSE_FUNCTION_CONFLICT`). Landing point: `packages/spec/src/stack.zod.ts`, as the order expected. ## Where the refusal sits, measured (PM assumption 3) `validateCrossReferences` runs inside `defineStack` in strict mode, which is the default. `objectstack validate`, `objectstack build` and `objectstack serve` all load the config module through `loadConfig`, which runs the author's `defineStack(...)` call. A refusal raised there therefore reaches all three doors, before each door's own parse. So the sibling placement refuses at the contract and at every public door. No second placement is needed. `os validate`'s own parse reads the stack after `lowerCallables`, and by then the names are already suffixed (`kc_a`, `kc_a__2`). A schema-level refinement would never see the duplicate there. That is one more reason to place the check in the aggregate and not in the `functions` schema. ## Before and after, through the real CLI Fixture: `defineStack({ manifest, objects, functions: [{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }] })`. Before: `main` at `7dbb89dc5`, after #22901 merged. After: this branch. | door | before (`7dbb89dc5`) | after (this branch) | |:--|:--|:--| | `os validate --json` | exit 0, `valid: true` | exit 1, `valid: false`, `code: STACK_CROSS_REFERENCE_INVALID`, the line above | | `os build` then `loadArtifactBundle` | exit 0. Artifact `[{"name":"kc_a","handler":"kc_a"},{"name":"kc_a__2","handler":"kc_a__2"}]`. Loaded: `kc_a` answers `first`, `kc_a__2` answers `second` | exit 2, the line above, no artifact | | source boot (`os serve --no-server`, no build) | boots with no notice. A probe in `onEnable` reads `resolveFunction('kc_a')`, which answers `second`. `first` is dropped, and `kc_a__2` is unregistered | exit 1 at `Loading objectstack.config.ts...`, the line above, kernel never boots | The card's two "Not measured" rows are now measured. The source boot ran the opposite entry from the build: `kc_a` ran `first` when built and `second` when booted from source. Three entries under one name plus two under another, `[kc_a, kc_b, kc_a, kc_b, kc_a]`: before, the artifact shipped `kc_a`, `kc_b`, `kc_a__2`, `kc_b__2` and `kc_a__3` at exit 0. After, the stack is refused with 2 issues: `stack.functions[0], stack.functions[2] and stack.functions[4]` for `kc_a`, and `stack.functions[1] and stack.functions[3]` for `kc_b`. **CONTROL.** Two distinct names (`kc_a`, `kc_b`), and the map form `{ kc_a: first, kc_b: { handler: second, effect: 'writes' } }`. Both give `os validate` exit 0 `valid: true` and `os build` exit 0, and both load callable. The control rows of the before and after runs are byte-identical. **The four example apps.** `os validate --json` on `examples/app-crm`, `app-todo`, `app-showcase` and `app-multi-package` exits 0 with `valid: true`. Each output is byte-identical to the run with the collector call removed (the ablated build below), apart from the `duration` value. Sizes: 11513, 8478, 75343 and 3665 bytes. ## Census: array-of-named-entries members of the stack schema Method: every `STACK_DEFINITION_KEYS` member whose entries carry a string `name`. Corpus keys: take the `defineStack` output of `examples/app-showcase` (with bound actions de-echoed), duplicate the key's first entry, and re-run `defineStack`. The corpus rows were also run through `os validate --json` on a fixture that does exactly that. Keys the examples do not author: hand-written minimal entries, through `defineStack` only. **Refused at the contract (same stack):** | key | how | |:--|:--| | `functions` (array form) | this PR | | `actions` | `collectDuplicateActionKeyErrors`, by scope-qualified key | | `apis` | by METHOD + path (`applyApiEndpointGates`). Two endpoints sharing a `name` on different paths are **accepted** | | `apps` | only through the single-app rule on a `manifest.type: 'app'` package (a count, not a name). On a `module` package, two apps sharing a name are **accepted** | **No same-stack uniqueness refusal on `name`.** `defineStack` accepts two entries with one name. Where marked, `os validate --json` was also measured: exit 0, `valid: true`, and no new finding names the duplicate (the one new finding per fixture is the fixture-path SDUI-manifest info line). | key | duplicated name | `os validate` | |:--|:--|:--| | `objects` | `showcase_account` | exit 0 (its only new findings are `field-no-consumers` on the copy's fields) | | `datasources` | `showcase_external` | exit 0 | | `pages` | `showcase_capability_map` | exit 0 | | `dashboards` | `showcase_chart_gallery` | exit 0 | | `reports` | `showcase_hours_by_status` | exit 0 | | `datasets` | `showcase_task_metrics` | exit 0 | | `flows` | `showcase_task_completed` | exit 0 | | `jobs` | `showcase_health_sweep` | exit 0 | | `emailTemplates` | `showcase_task_done_email` | exit 0 | | `books` | `showcase_manual` | exit 0 | | `positions` | `contributor` | exit 0 | | `permissions` | `showcase_contributor` | exit 0 | | `capabilities` | `showcase.export_data` | exit 0 | | `sharingRules` | `share_new_inquiries_with_field_ops` | exit 0 | | `webhooks` | `showcase_task_changed` | exit 0 | | `hooks` | `showcase_normalize_task_title` | exit 0 | | `mappings` | `showcase_inquiry_feed` | exit 0 | | `analyticsCubes` | `showcase_delivery` | exit 0 | | `connectors` | `showcase_status_api` | exit 0 | | `plugins` | `com.objectstack.connector.openapi` | exit 0 | | `picklists`, `docs`, `agents`, `tools`, `skills`, `views` (a named view) | hand-written `cen_*` entries | `defineStack` only | | `apis` by name on different paths, `apps` on a `module` | (above) | `defineStack` only | Not named collections, so out of the census: `packages` (wrapper over `manifest`), `translations` (keyed by locale), `objectExtensions` and `picklistExtensions` (keyed by `extend`, additive), `data` (keyed by `object`), `datasourceMapping`, `devLogins` (keyed by `email`), `devPlugins`, `requires` and `tiers` (strings). `viewItems` is `z.never`. What happens downstream to each sibling (last-wins, merge, both kept) was **not measured**. This census measures only the contract door. Per the triage fence this PR fixes `functions` only. The siblings go to one batch card, which the seat files. ## Tests New pin file: `packages/spec/src/stack-duplicate-function-name.test.ts`, 10 cases. Each refusal case asserts the ADR-0112 envelope (`code`, `status: 422`, the `issues` count, the header) and the named subjects of each finding: the name, and exactly the indices it lists. It does not assert a bare `toThrow()`. - Refused: two entries; 3+2 entries; byte-identical twins; an object-less stack; the lowered string-ref form repeating a name; one aggregate with a duplicate action key (2 issues). - Kept: skipped under `strict: false`, like every cross-reference check. - CONTROL: distinct names accepted in order, with each entry's `effect`; the map form, keys unchanged; and the suffixed refs a build emits (`kc_a`, `kc_a__2`), accepted as two names. Ablation 1 (source; the suite imports `./stack.zod` directly). `scripts/ablation-replace.mjs` deleted `errors.push(...collectDuplicateFunctionNameErrors(config));`: anchor 1 → 0, blob `a3a6790a16c0` → `a8e4e3300fe3`. The run went **6 failed / 4 passed**: the six refusal cases are red, and the three CONTROLs and `strict: false` stay green, which is the predicted direction. Restored: blob == HEAD, `git diff HEAD` empty. Ablation 2 (dist, through the CLI). The same deletion went through `--hold`, then `OS_SKIP_DTS=1` spec build. `ablation-dist-preflight --absent` reported the marker absent from all 98 built files. `os validate --json` on the duplicate fixture then answered exit 0, `valid: true`. Restore leg: `git checkout HEAD`, blob == HEAD, full spec build, preflight marker present in 4 built files with the tree clean. After that, `os validate` answered exit 1 with `STACK_CROSS_REFERENCE_INVALID`. Package readings at `6d555670a` (spec unchanged by the later `main` merge): - `pnpm --filter @objectstack/spec test`: 649 files, **19404 passed** | 1 todo, exit 0. - `pnpm --filter @objectstack/spec typecheck`: exit 0. This covers `tsc --noEmit`, `check:scripts-typecheck` and `check:test-typecheck` (52 files / 246 errors / 135 pinned signatures held, the new test file adding none). - `@objectstack/cli` unit tier: `src/utils/lower-callables.test.ts`, `test/lower-callables.test.ts` and `src/utils/boot-preparation-parity.test.ts`. 3 files, **95 passed**, exit 0. - `test/build-callable-names.test.ts` lives in the `integration` tier and is declared to CI. Its array-form fixtures author distinct names under `strict: false`, so this check cannot reach them. - `@objectstack/runtime`: `artifact-function-declarations`, `bundle-function-entries`, `artifact-collections` and `app-plugin`. 4 files, **68 passed**, exit 0. - `pnpm --filter @objectstack/spec build`, then `check:generated`: all 14 generated artifacts up to date, no file changed. Fixture triage (PM assumption 5). A repo-wide grep of array-form `functions` covered `packages/cli` (`lower-callables` ×2, `build-callable-names`, `lint-hook-rules-reach-handler-hooks`), `packages/runtime` (`app-plugin`, `artifact-collections`, `artifact-function-declarations`, `bundle-function-entries`), `packages/objectql`, `packages/spec` and `examples/**`. No hit authors one name twice. The two example apps that declare `functions` use the map form. ## ADR-0087 and the changeset `.changeset/22902-stack-functions-array-duplicate-name-refused.md`: `@objectstack/spec` `minor`, `Clause-②: no (narrowing)`, a **BREAKING** banner, FROM → TO and the one-line fix. Because the body carries a migration prescription, `check:adr-0087-registration` refuses `not-required (no-migration-prescription)`. Measured on a detached probe commit: "contradicts the changeset's own body, which carries a migration prescription". So the disposition is `registered`, with the D3 semantic entry `packages/spec/src/migrations/entries/semantic/18.stack-functions-array-duplicate-name-refused.ts` (protocol 18) and no D2 conversion. There is no `STEP18_RATIONALE` fragment (114 fragments for 346 step-18 entries, so one is not required) and no `relevantWhen`. ## Gates Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths) on `70c36c115`, which is this PR's head. That is 90 commands. Each one ran with its exit code captured before any pipe, then everything was reconciled with `--ran`. Result: **90 derived, 89 run, 1 NOT-MEASURED, 0 UNRUN**. - 87 exited 0 on the first pass. These include `check-adr-0087-registration` (1 declared-breaking changeset, `registered`, new here), `check-changeset-no-major` (no major; pre-mode lockstep evidenced), `check:nul-bytes`, `check:doc-authoring`, `check:issue-citations`, and every spec `check:*` the derivation named (`check:api-surface` "unchanged", `check:authorable-surface`, `check:docs`, `check:liveness`, `check:spec-changes`, `check:upgrade-guide` and the rest). Also `check:type-check-coverage` and `check:type-check-debt`, the latter re-measured in 754 s with no entry above its record. - `check-plugin-teardown-shape.mjs --self-test` exited 3 on the first pass. Its pinned fixture commit `621a487607` was outside this shallow clone. After `git fetch --depth=1` of that one commit, it exits 0 (48 cases). - `check:dts-closure` exited 1, from local build state and not from this diff. The 58 packages outside `@objectstack/spec` were built with `OS_SKIP_DTS=1`, so they carry no `.d.ts`. `@objectstack/spec` is not on its list. **NOT MEASURED**, reason: no full DTS build of the closure locally. CI builds it. - `check:dual-build-cjs-loads` exited 3, `PREREQUISITE NOT MET`: 8 packages outside the built closure have no `dist/`. **NOT MEASURED**, reason: as reported. - Not run locally, CI's: the integration tier of `@objectstack/cli`, the whole-workspace type-check lanes, and the path-scheduled CI jobs the derivation lists outside its 90. ## Acceptance notes - **Boundary, shared with every cross-reference check.** `defineStack(..., { strict: false })` skips the check. `os validate` and `os build` then accept the duplicate: their own parse reads the lowered, already-suffixed names. This is the same reach as the duplicate action-key refusal, and it is pinned as kept, not widened here. - **PM assumption 4 re-measured.** On current `main` (after #22901), both entries are callable after a build, and the second ships under `kc_a__2`. That is the card's "PR #22901 branch" reading, and it is now the `main` reading. - **Grade.** Pre mode (`next`) is open, so `check:changeset-no-major` admits either `minor` or `major`. The order named `minor`, and the triage comment called it "a major's change". `minor` follows the order and the launch-window banner precedent, and in the fixed group it moves no version number. - **Doors, measured.** The refusal surfaces at load on all three doors, with door-specific exits: validate 1, build 2, serve 1. --- _Generated by [Claude Code](https://claude.ai/code/session_019G5ZnRrTMiwcHG6iZM19zf)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 74c4b34 commit 147a0d1

4 files changed

Lines changed: 334 additions & 2 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
feat(spec)!: `defineStack` refuses two entries of one stack's `functions` array that author the same `name`, naming the name and every index (#22902)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: registered stack-functions-array-duplicate-name-refused -->
10+
11+
**BREAKING**: an accept-set narrowing on a published authoring surface, shipped as `minor` under the launch-window convention for breaking changes.
12+
13+
Functions are one flat namespace resolved by name: a hook's `handler`, an action's `target` and a flow `script` node's `config.function` all look a function up by its name. The `functions` **array** form could author one name twice, and nothing refused it. Measured on `main` at this change's base with `functions: [{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }]`:
14+
15+
- `objectstack validate` exited 0 (`valid: true`);
16+
- `objectstack build` exited 0 and shipped `[{ name: 'kc_a' }, { name: 'kc_a__2' }]`: the second entry went out under a suffixed internal ref the author never wrote and nothing references, and `kc_a` ran `first`;
17+
- a source boot (`objectstack serve`, no build) registered `kc_a` as `second` and dropped `first`, with no notice.
18+
19+
So the same stack ran different code under `kc_a` depending on whether it was built. `composeStacks` already refused one name defined by two stacks (`STACK_COMPOSE_FUNCTION_CONFLICT`); this is the one-stack case.
20+
21+
**What is refused.** `defineStack`'s cross-reference pass refuses the stack with its existing envelope (`STACK_CROSS_REFERENCE_INVALID`, status 422, `defineStack cross-reference validation failed (N issues):`), one finding per colliding name, naming the name and every index:
22+
23+
```text
24+
✗ Function 'kc_a' is declared twice in the `functions` array: stack.functions[0] and stack.functions[1]. …
25+
```
26+
27+
It runs in an object-less stack too, and every entry counts, byte-identical twins included. `objectstack validate`, `objectstack build` and a source boot all load the config through `defineStack`, so all three refuse it at load, printing that line: `objectstack validate --json` answers `valid: false` with `code: 'STACK_CROSS_REFERENCE_INVALID'` (exit 1), `objectstack build` exits 2, and `objectstack serve` exits 1 before the kernel boots. Like every cross-reference check it is skipped under `defineStack(…, { strict: false })`.
28+
29+
**Unchanged.** The map form, which cannot author one name twice; an array whose names are distinct; the suffixed refs a build emits for an internal slot (`kc_a__2` beside `kc_a` is two names); and the cross-stack refusal in `composeStacks`.
30+
31+
## FROM → TO
32+
33+
| you wrote | write instead |
34+
|:--|:--|
35+
| `functions: [{ name: 'sync', handler: a }, { name: 'sync', handler: b }]` | `functions: [{ name: 'sync', handler: a }, { name: 'sync_backup', handler: b }]`, and point each hook, action or script node that meant `b` at `sync_backup` |
36+
| two entries that are the same function under one name | one entry: delete the other |
37+
38+
**The one-line fix: give each `functions` array entry its own `name`, or merge the duplicates into one entry.**
39+
40+
No D2 conversion rewrites it: which entry keeps the name, and what every caller of the other one should name instead, is the author's call. Its D3 record is the semantic entry `stack-functions-array-duplicate-name-refused` (protocol 18).
41+
42+
**Who is affected, measured.** On `main` at this change's base, no example app, fixture or test authors one name twice in a `functions` array: the four example apps author the map form or none, and every array-form fixture under `packages/` names distinct entries. `objectstack validate --json` on `examples/app-crm`, `app-todo`, `app-showcase` and `app-multi-package` answers byte-for-byte as it did without the check, apart from the `duration` value. Deployed metadata and other repositories were not measured.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import type { SemanticMigration } from '../../types.js';
4+
5+
// #22902. An accept-set narrowing of the stack aggregate: two entries of one
6+
// stack's `functions` ARRAY that author the same `name` are refused by the
7+
// cross-reference pass, the one-stack sibling of the duplicate action-key
8+
// refusal and of `composeStacks`' cross-stack function-name refusal. No D2
9+
// conversion: which entry should own the name, and what the other one is
10+
// renamed to (with every hook, action and script node that reads it), is the
11+
// author's call.
12+
export const entry: SemanticMigration = {
13+
id: 'stack-functions-array-duplicate-name-refused',
14+
// No backticks in `surface` — build-upgrade-guide renders it inside a code
15+
// span already, and a nested backtick would close it.
16+
surface:
17+
'stack.functions in the ARRAY form — two or more entries that author the same name, e.g. '
18+
+ "functions: [{ name: 'sync', handler: a }, { name: 'sync', handler: b }]",
19+
replacement:
20+
'One entry per name. Rename one of the colliding entries (and every hook handler, action target '
21+
+ 'and flow script node that should call it), or merge them into a single entry and delete the '
22+
+ 'other. The map form, which cannot author one name twice, is unchanged, and so is an array whose '
23+
+ 'names are distinct.',
24+
reason:
25+
'Functions are one flat namespace resolved by name: a hook handler, an action target and a flow '
26+
+ 'script node all look a function up by its name, and the engine registers each under that name. '
27+
+ 'The array form could author one name twice and nothing refused it, so the two entries diverged '
28+
+ 'silently and differently on each path. objectstack build exited 0 and shipped the second entry '
29+
+ 'under a suffixed internal ref the author never wrote and nothing references, so the name ran the '
30+
+ 'FIRST entry; a source boot registered the name as the SECOND entry and dropped the first, with no '
31+
+ 'notice. The same stack therefore ran different code under one name depending on whether it was '
32+
+ 'built. composeStacks already refused the same name across two stacks; the stack aggregate now '
33+
+ 'refuses it inside one stack, in the same cross-reference envelope as a duplicate action key. '
34+
+ 'Not convertible: which entry keeps the name is a fact only the author holds.',
35+
acceptanceCriteria:
36+
'os validate and os build load the stack without a STACK_CROSS_REFERENCE_INVALID refusal. A stack '
37+
+ 'that still repeats a name is refused at load, with one finding per colliding name that names the '
38+
+ 'name and every index (stack.functions[i] and stack.functions[j]). After renaming, check that '
39+
+ 'each hook, action and script node that calls the renamed function now names the entry it '
40+
+ 'means, rather than assuming the old resolution was the intended one.',
41+
};
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/**
2+
* `defineStack` refuses two entries of one stack's `functions` ARRAY that
3+
* author the same `name` — and only those.
4+
*
5+
* Functions are one flat namespace resolved by name (a hook's `handler`, an
6+
* action's `target`, a flow `script` node's `config.function`), so one name
7+
* can own one handler. Measured on main `7dbb89dc5` before the check existed,
8+
* with `[{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }]`:
9+
*
10+
* ```
11+
* os validate --json : exit 0 valid: true
12+
* os build → loadArtifactBundle : exit 0 [{ name: 'kc_a' }, { name: 'kc_a__2' }] kc_a → first
13+
* source boot (os serve, no build) : boots kc_a → second, first dropped, no notice
14+
* ```
15+
*
16+
* The map form cannot author one name twice (an object key is unique) and is
17+
* never visited; the cross-stack case is `composeStacks`' own refusal
18+
* (`STACK_COMPOSE_FUNCTION_CONFLICT`). This is the one-stack sibling of the
19+
* duplicate action-key check, raised in the same cross-reference aggregate.
20+
*
21+
* Refusals assert the ADR-0112 envelope (`code` + `status`) and the named
22+
* subjects of each finding — the colliding name and every index — rather than
23+
* a bare `toThrow()`, which a refusal for any other reason would also satisfy.
24+
* Every refusal fixture differs from an accepted CONTROL by one name.
25+
*/
26+
import { describe, it, expect } from 'vitest';
27+
import { defineStack } from './stack.zod';
28+
29+
const manifest = {
30+
id: 'com.example.dupfn',
31+
name: 'duplicate-function-name-test',
32+
version: '1.0.0',
33+
type: 'app' as const,
34+
};
35+
36+
// `as const` on the field type is load-bearing (see stack.test.ts).
37+
const probeItem = { name: 'probe_item', label: 'Probe Item', fields: { title: { type: 'text' as const } } };
38+
39+
const first = () => 'first';
40+
const second = () => 'second';
41+
const third = () => 'third';
42+
43+
interface Refusal {
44+
name?: string;
45+
code?: string;
46+
status?: number;
47+
issues?: readonly string[];
48+
message: string;
49+
}
50+
51+
/** The thrown refusal, or `null` when the stack is accepted. */
52+
function refusal(config: Parameters<typeof defineStack>[0]): Refusal | null {
53+
try {
54+
defineStack(config);
55+
return null;
56+
} catch (e) {
57+
return e as Refusal;
58+
}
59+
}
60+
61+
function expectCrossReferenceEnvelope(r: Refusal | null, issueCount: number): asserts r is Refusal {
62+
expect(r).not.toBeNull();
63+
expect(r!.code).toBe('STACK_CROSS_REFERENCE_INVALID');
64+
expect(r!.status).toBe(422);
65+
expect(r!.issues).toHaveLength(issueCount);
66+
expect(r!.message).toContain(
67+
`defineStack cross-reference validation failed (${issueCount} issue${issueCount === 1 ? '' : 's'}):`,
68+
);
69+
}
70+
71+
/** The one finding naming `name`, which must name exactly `indices`. */
72+
function findingFor(r: Refusal, name: string, indices: number[]): string {
73+
const found = (r.issues ?? []).filter((issue) => issue.startsWith(`Function '${name}' `));
74+
expect(found, `exactly one finding for '${name}'`).toHaveLength(1);
75+
const finding = found[0];
76+
for (const i of indices) expect(finding).toContain(`stack.functions[${i}]`);
77+
const named = [...finding.matchAll(/stack\.functions\[(\d+)\]/g)].map((m) => Number(m[1]));
78+
expect(named, `the indices '${name}' names`).toEqual(indices);
79+
// The finding reaches the message too, as one `✗` line.
80+
expect(r.message).toContain(` ✗ ${finding}`);
81+
return finding;
82+
}
83+
84+
describe('defineStack - duplicate name in one stack\'s `functions` array', () => {
85+
it('refuses two entries authoring one name, naming the name and both indices', () => {
86+
const r = refusal({
87+
manifest,
88+
objects: [probeItem],
89+
functions: [{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }],
90+
});
91+
expectCrossReferenceEnvelope(r, 1);
92+
expect(r.name).toBe('StackCrossReferenceError');
93+
const finding = findingFor(r, 'kc_a', [0, 1]);
94+
expect(finding).toContain('declared twice');
95+
});
96+
97+
it('refuses one finding per colliding name, each listing every index, and leaves a unique name alone', () => {
98+
const r = refusal({
99+
manifest,
100+
objects: [probeItem],
101+
functions: [
102+
{ name: 'kc_a', handler: first },
103+
{ name: 'kc_b', handler: second },
104+
{ name: 'kc_a', handler: third },
105+
{ name: 'kc_b', handler: second },
106+
{ name: 'kc_a', handler: first },
107+
{ name: 'kc_c', handler: third },
108+
],
109+
});
110+
expectCrossReferenceEnvelope(r, 2);
111+
expect(findingFor(r, 'kc_a', [0, 2, 4])).toContain('declared 3 times');
112+
expect(findingFor(r, 'kc_b', [1, 3])).toContain('declared twice');
113+
expect(r.issues!.some((issue) => issue.includes("'kc_c'"))).toBe(false);
114+
});
115+
116+
it('refuses byte-identical twins — two registrations under one name whatever they hold', () => {
117+
const entry = { name: 'kc_twin', handler: first, effect: 'writes' as const };
118+
const r = refusal({ manifest, objects: [probeItem], functions: [entry, { ...entry }] });
119+
expectCrossReferenceEnvelope(r, 1);
120+
findingFor(r, 'kc_twin', [0, 1]);
121+
});
122+
123+
it('refuses in an object-less stack — the check needs no object to resolve against', () => {
124+
const r = refusal({
125+
manifest,
126+
functions: [{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }],
127+
});
128+
expectCrossReferenceEnvelope(r, 1);
129+
findingFor(r, 'kc_a', [0, 1]);
130+
});
131+
132+
it('refuses the lowered (string-ref) array form too, when it repeats a name', () => {
133+
const r = refusal({
134+
manifest,
135+
objects: [probeItem],
136+
functions: [{ name: 'kc_a', handler: 'kc_a' }, { name: 'kc_a', handler: 'kc_a' }],
137+
});
138+
expectCrossReferenceEnvelope(r, 1);
139+
findingFor(r, 'kc_a', [0, 1]);
140+
});
141+
142+
it('joins the existing aggregate: one envelope with the duplicate action-key finding', () => {
143+
const act = (name: string) => ({ name, label: name, type: 'script' as const, target: 'noop' });
144+
const r = refusal({
145+
manifest,
146+
objects: [probeItem],
147+
actions: [act('dup_r'), act('dup_r')],
148+
functions: [{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }],
149+
});
150+
expectCrossReferenceEnvelope(r, 2);
151+
findingFor(r, 'kc_a', [0, 1]);
152+
expect(r.issues!.some((issue) => issue.startsWith("Action key 'global:dup_r' is declared twice"))).toBe(true);
153+
});
154+
155+
it('is skipped under `strict: false`, like every other cross-reference check', () => {
156+
expect(() => defineStack(
157+
{ manifest, objects: [probeItem], functions: [{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }] },
158+
{ strict: false },
159+
)).not.toThrow();
160+
});
161+
});
162+
163+
describe('defineStack - CONTROL: distinct names and the map form are accepted unchanged', () => {
164+
it('accepts two distinct names in the array form, in order, with what each entry declared', () => {
165+
const out = defineStack({
166+
manifest,
167+
objects: [probeItem],
168+
functions: [{ name: 'kc_a', handler: first }, { name: 'kc_b', handler: second, effect: 'writes' }],
169+
});
170+
const fns = out.functions as Array<{ name: string; handler: unknown; effect?: string }>;
171+
expect(fns.map((e) => e.name)).toEqual(['kc_a', 'kc_b']);
172+
expect(fns.map((e) => typeof e.handler)).toEqual(['function', 'function']);
173+
expect(fns.map((e) => e.effect)).toEqual([undefined, 'writes']);
174+
});
175+
176+
it('accepts the map form, keys unchanged', () => {
177+
const out = defineStack({
178+
manifest,
179+
objects: [probeItem],
180+
functions: { kc_a: first, kc_b: { handler: second, effect: 'writes' } },
181+
});
182+
expect(Object.keys(out.functions as Record<string, unknown>)).toEqual(['kc_a', 'kc_b']);
183+
});
184+
185+
it('accepts the suffixed internal refs a build emits — distinct names, not a duplicate', () => {
186+
const out = defineStack({
187+
manifest,
188+
objects: [probeItem],
189+
functions: [{ name: 'kc_a', handler: 'kc_a' }, { name: 'kc_a__2', handler: 'kc_a__2' }],
190+
});
191+
expect((out.functions as Array<{ name: string }>).map((e) => e.name)).toEqual(['kc_a', 'kc_a__2']);
192+
});
193+
});

‎packages/spec/src/stack.zod.ts‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2229,6 +2229,59 @@ function collectDuplicateActionKeyErrors(config: ObjectStackDefinition): string[
22292229
return errors;
22302230
}
22312231

2232+
/**
2233+
* Same-stack duplicate `functions` names in the ARRAY form (#22902) — one
2234+
* refusal line per colliding name, naming the name and every index.
2235+
*
2236+
* Functions live in one flat namespace resolved by name: a hook's `handler`,
2237+
* an action's `target` and a flow `script` node's `config.function` all look a
2238+
* function up by its name, and the engine registers each under that name
2239+
* (`registerFunction` is a plain `Map.set`). The map form cannot author one
2240+
* name twice — an object key is unique — so it is never visited. The array
2241+
* form can, and nothing refused it, so the two entries diverged silently and
2242+
* differently on each path. Measured on main `7dbb89dc5` with
2243+
* `[{ name: 'kc_a', handler: first }, { name: 'kc_a', handler: second }]`:
2244+
*
2245+
* - `objectstack build` exited 0 and shipped
2246+
* `[{ name: 'kc_a' }, { name: 'kc_a__2' }]`: `lowerCallables` mints refs in
2247+
* one namespace, so the second entry took a suffixed internal ref the author
2248+
* never wrote and nothing references — `kc_a` ran `first`;
2249+
* - a source boot (no build) registered `kc_a` as `second` and dropped
2250+
* `first`, with no notice (`collectBundleFunctionEntries` keys by name, last
2251+
* wins).
2252+
*
2253+
* So the same stack ran different code under `kc_a` depending on whether it
2254+
* was built. {@link composeStacks} already refuses the cross-stack case
2255+
* (`composeFunctions`, `STACK_COMPOSE_FUNCTION_CONFLICT`); this is its
2256+
* one-stack sibling, in the aggregate beside the action-key check. Every entry
2257+
* counts, byte-identical twins included: two entries are two registrations
2258+
* under one name whatever they hold, and only one can own it.
2259+
*/
2260+
function collectDuplicateFunctionNameErrors(config: ObjectStackDefinition): string[] {
2261+
const functions = config.functions;
2262+
if (!Array.isArray(functions)) return [];
2263+
2264+
const indicesByName = new Map<string, number[]>();
2265+
for (const [i, entry] of functions.entries()) {
2266+
const list = indicesByName.get(entry.name) ?? [];
2267+
list.push(i);
2268+
indicesByName.set(entry.name, list);
2269+
}
2270+
2271+
const errors: string[] = [];
2272+
for (const [name, indices] of indicesByName) {
2273+
if (indices.length < 2) continue;
2274+
errors.push(
2275+
`Function '${name}' is declared ${indices.length === 2 ? 'twice' : `${indices.length} times`} ` +
2276+
`in the \`functions\` array: ${joinDeclarationOrigins(indices.map((i) => `stack.functions[${i}]`))}. ` +
2277+
`Functions are registered and resolved by name, so only one of these entries can own ` +
2278+
`'${name}' and the others would be silently renamed or dropped. Rename one of them, or ` +
2279+
`merge them into a single entry.`,
2280+
);
2281+
}
2282+
return errors;
2283+
}
2284+
22322285
/**
22332286
* [ADR-0112 · #15963] The envelope every refusal `defineStack` raises shares:
22342287
* `status: 422` — an unprocessable authored entity, not a server fault, the
@@ -2303,8 +2356,8 @@ abstract class StackRefusalError<TIssue = string> extends Error {
23032356
* machine-readable in {@link StackCrossReferenceError.issues}, one entry per
23042357
* finding, which is the structured form of what was previously only
23052358
* newline-joined prose. The family is also WIDER than "undefined object": the
2306-
* same aggregate carries the duplicate-action-key, global-`update`-action and
2307-
* mapping `javascript`-transform findings, so a
2359+
* same aggregate carries the duplicate-action-key, duplicate-function-name,
2360+
* global-`update`-action and mapping `javascript`-transform findings, so a
23082361
* `…_UNDEFINED_OBJECT` spelling would be false for those.
23092362
*
23102363
* ⭐ There are TWO raise sites since #18202, one per PASS, and they share this
@@ -2945,6 +2998,9 @@ function validateCrossReferences(
29452998
// stack that declares two globals under one name collides on exactly the
29462999
// same runtime key, and this check needs no object to resolve against.
29473000
errors.push(...collectDuplicateActionKeyErrors(config));
3001+
// Same placement, same reason: two `functions` array entries under one name
3002+
// collide in the one function namespace whether or not there are objects.
3003+
errors.push(...collectDuplicateFunctionNameErrors(config));
29483004
// Same placement, same reason: a global `operation: 'update'` action is
29493005
// wrong whether or not the stack declares any objects.
29503006
errors.push(...collectGlobalUpdateActionErrors(config));

0 commit comments

Comments
 (0)