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

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