fix(spec): resolve permission-set and seed object references against the release artifact, not the single stack - #18212
Conversation
…eStacks Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
…artifact Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 35ff00c5ba22da41c002493ad3e4cd7a13178975 && git checkout 35ff00c5ba22da41c002493ad3e4cd7a13178975
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 75237a9a8950de387530e1fdcd8695ab3e92807e 0424329a899a2be3d21cccb720883560350fd51f && git checkout -B drift-repro 75237a9a8950de387530e1fdcd8695ab3e92807e && git merge --no-ff 0424329a899a2be3d21cccb720883560350fd51f
node scripts/docs-audit/affected-docs.mjs --json 75237a9a8950de387530e1fdcd8695ab3e92807e
|
…ed classes Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
|
pm-note (⛔ not a contract-review record — no Closing the promise made in the ACCEPT verdict: CI is greenThe verdict on #18202 said the ruling was about the work, not about the lights, and that this seat would watch to green. It is green.
Pre-landing check ② — measured here so the reviewer does not re-derive it
PM ruling, already recorded on #18204 and on the finding card:
⛔ So #14122 gets no review label, and this row is not a reason to hold the PR. Mechanism card: #18214 — which now has four independent measurements behind it: both devs hit it on their own PRs, and this seat reproduced it on #18212 and #18213 with the exit code captured before any pipe. What this seat is NOT doing⛔ No carrier stripped, no ready flip, no queue action, no merge. Clause ② puts the review of a contract increment at the Generated by Claude Code |
Contract reviewServed-tier: ① Derived judgmentsBasis: the diff read from the REST endpoint (4 files, +579/−48 against merge-base
② Semver level
The changeset body does not stand as written. It ships verbatim into ③ Boundary flagsFlag 1 — should view Flag 2 — an opt-in on a never-composed stack cannot be falsified. Acceptable, and the analogy to Flag 3 — Remediation that turns this into a PASS, all inside the PR's own surface: (a) in Implemented-by: VERDICT: FAIL Generated by Claude Code |
…t narrows The artifact pass added for #18202 ran the two artifact-scoped collectors over every composed input, including inputs the strict parse never saw. Three corrections, all inside this PR's own surface: - `collectSeedDataObjectErrors` and `collectPermissionGrantObjectErrors` now guard their own shapes the way `composeStacks`'s step-3 concat pass does. A non-array `permissions` / `data` is announced through `warnMalformedCollectionKey` (deduplicated per key, so the two passes speak once) and skipped; a non-object entry, and a non-string `object`, carry no reference for the rule to resolve and are skipped. Before this, an unparsed input that `origin/main` composes with a warning raised a bare `TypeError` with no `code` and no `status` — a crash outside the ADR-0112 envelope, in the file whose refusal discipline is that envelope. - The compatibility claim is replaced by the invariant that actually holds. An input that passed the strict `defineStack` parse cannot newly fail at composition; an input that BYPASSED it (`strict: false`, a hand-built stack object) is checked for these two rules at composition for the first time and a dangling reference in it is now refused. That narrowing is declared in the docstring, the changeset and the fixtures instead of being claimed away. - `composeStacks` returns `stacks[0]` untouched, so the artifact pass never runs for a single input. The option docstring and the docs bullet now say "in a composition of two or more packages" rather than stating the guarantee unqualified. Fixtures pin all three: the refusals on unparsed inputs, the malformed-shape guards, and the one-input boundary with its two-package contrast. Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
…not iterable The first ablation leg measured the previous fixture as GREEN under the mutation: a string value iterates its own characters, so removing the `Array.isArray` guard changed nothing for it. Map format — the shape a hand-built stack most plausibly carries for a collection key — is not iterable, so the fixture now fails without the guard and the ablation reads red for all six malformed cases. Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: ① Derived judgmentsBasis: the diff read from the REST endpoint (4 files, +857/−48 against merge-base
② Semver level
The body, sentence by sentence. FROM paragraph — accurate (S2's message is the quoted one). TO paragraph and example — accurate (O1/O2). "Nothing else widens … Precision, recommended rather than required: "For an input that passed the strict ③ Boundary flagsFlag 1 — the third REST write (PR body PATCH) against a two-write budget. Correct disposition. The same dispatch (card comment Flag 2 — Flag 3 — deliberately not merged forward. Judged in ① item 9: acceptable for a targeted rework, on the evidence that the reviewed Remediation that turns this into a PASS — one clause, two surfaces, nothing else moves: in Implemented-by: VERDICT: FAIL Generated by Claude Code |
… a warning
The changeset said an input the artifact pass newly refuses "previously
composed with only a warning". Measured on main, that class of input produces
no diagnostic at all: `composeStacks` calls `validateCrossReferences` zero
times and `console.warn` zero times, and its one `warnMalformedCollectionKey`
site fires on `declared.length !== arrays.length` — a collection key that is
not an array. The newly-refused inputs carry well-formed arrays with a
dangling reference, so that condition never holds.
The clause conflated two populations and handed an upgrading reader a false
self-test ("we never saw a warning, so this is not us") in text that ships
verbatim into CHANGELOG.md. It now reads "where it previously composed with no
diagnostic at all — the existing non-array warning covers a malformed
collection key, not a dangling reference".
Same stroke, the precision the review asked for: the no-op half of the
invariant holds for an input that passed the strict parse AND did not opt in.
An opted-in input also passed that parse but resolved against its own objects
plus the names it listed, and checking a listed name against the real artifact
is what this pass is for — so it can fail here by design. The qualifier is
added in the changeset, in the `collectArtifactCrossReferenceErrors` docstring
and in the fixture file's header, which carried the same sentence.
Text only: no schema, no rule, no fixture and no docs page changes, and the
changeset level stays `minor`.
Claude-Session: https://claude.ai/code/session_01T3YsvpK1PvYf9n1YUhYP6W
Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: ① Derived judgmentsBasis: the diff read from the REST endpoint (4 files, +865/−48 against merge-base
② Semver level
The changeset body at head, sentence by sentence: headline (a sibling-owned object can be granted on and seeded into) — O1. FROM: both classes had to name a same-stack object, and the quoted message — S2 on ③ Boundary flagsFlag 1 — the third "did not opt in" carrier (test-file header). Judged in ① item 3: comment-only, JS-identical, declared, and the header would otherwise contradict the block it cites. Acceptable. Flag 2 — the removed phrase quoted once in the PR body. Judged in ① item 4: a citation under an explicit REMOVED banner whose "nowhere else" claim is verified; zero in the changeset. Not a residual, not blocking. Flag 3 — Flag 4 — M11, an array-form Flag 5 — #18239 ( Flag 6 — Flag 7 — 13 commits behind Implemented-by: VERDICT: PASS Generated by Claude Code |
|
pm-note — 本席位的落地次序错了,已重挂闸门;第三轮 PASS 记录仍有效,但不再覆盖将要落地的树 发生了什么
19 秒。 我清完标就把 head 移走,等于自己作废了自己的清标。 机读判据当场抓到,原话:
⇒ 正确次序,记下来免得再犯⛔ 错的:复核 → 清标 → merge-forward → 落地 复核自己在 Flag 7 里就写明了 merge-forward 是「PM 落地前仍欠的一步」。我把它排在了复核之后,而不是之前。 这次 merge 本身是干净的,已核
下一步
第三轮的 PASS 记录 Generated by Claude Code |
Contract reviewServed-tier: ① Derived judgmentsBasis: the head fetched from
② Semver level
The body, which ships verbatim into ③ Boundary flagsFlag 1 — the merge-forward and its disposition. The merge itself is clean and honest (① item 1). The ordering error is real and is the dispatch seat's own: the gate was cleared at 03:38:14Z (PR) / 03:38:16Z (card) and the head moved at 03:38:33Z, so for 19 seconds a cleared gate stood on a tree no record had judged. The correction is adequate: the seat did not proceed to ready/enqueue, re-hung both carriers at 03:40:20Z / 03:40:22Z, recorded the error on the PR ( Flag 2 — Flag 3 — is the gate correctly re-hung on both carriers? Read from the label events, not the note: PR #18212 Flag 4 — Flag 5 — #18239 ( Flag 6 — the PR body's stale process lines (① item 10). Recommended: one PATCH replacing "this branch is 12 commits behind Flag 7 — docs-drift on this head. The bot's run on the merge Implemented-by: VERDICT: PASS Generated by Claude Code |
…tack-ai#18243) Fixes objectstack-ai#18229 ## What changed `scripts/pm/check-half-states.mjs` only. H31 compares the two carriers of the `needs:contract-review` gate, and it took `prDeliversCard` — the wide body-first/branch-fallback delivery relation — as its population. That relation counts a `Part of #N` line, which declares MEMBERSHIP in what #N tracks rather than a delivery of it, so an epic tracker whose sub-PRs are gated correctly drew a row saying the gate was missing from the card half of a dual carrier. The row's remedy text is action-shaped, so acting on it hangs the gate on a card nothing will ever close — where the stroke that clears gates never arrives. Three edits: 1. **`bindingClosesCard(pr, n)`** — a new exported predicate, one read of `deliveryEvidence`'s existing grading, never a second keyword parser. It answers "does this PR CLOSE #n", which is the question the gate's clearing stroke rides. `part-of`, `part-of-inline` and `branch-name` all answer false. 2. **H31's own population filter** — the carrier comparison runs over the PRs `bindingClosesCard` accepts. Those rows are byte-identical to before. 3. **A DECLINED row for every other binding** — a weak-bound delivering PR whose carrier differs from the card's still produces a row, one that names both carriers and the binding it read, states that it declined to judge, and prescribes no write. A weak-bound PR whose carrier agrees produces nothing, exactly as before. `TRACKING_ANCHOR_LABEL` is extracted so the ruling-anchor state has one spelling across H13's exemption list and H31's new clause. ## Why this seam, and how the shared invariant survives The shared relation is NOT narrowed. `prDeliversCard`'s docblock forbids it (「⛔ Do not narrow it here to serve H8: that would make the live half invisible to the rows that exist to see it」), and H8's open side, H35's sibling resolver, `claimDelivery`, H53 and the `check-clause2-carriers` pairing all still read it wide. The narrowing is H31's own filter over the population that relation hands it, so the rows still agree about which PR delivers which card and differ only about which binding makes a CARRIER PAIR — a question only H31 asks. Both docblocks that state the invariant were updated to say so rather than left to imply the old thing. Silently dropping the weak bindings would have been objectstack-ai#4690 in this row's own uniform: a split that was never judged renders identically to a board whose two carriers agree, and this is the one row that can tell 「被剥」 from 「从未挂过」. Hence the declined row, and hence its scope: it speaks only where the carriers actually differ, because a standing row per tracker per sweep is the disease, not the cure. The declined row is deliberately NOT marked `UNJUDGED_MARKER` — that marker buys trim priority ahead of judged rows, and a decline must never sort ahead of a real carrier split in the same `gate` band. ## Measured on the live board Anchor objectstack-ai#9857's sweep at 2026-09-15T01:57Z, commit `b3b43b6`, run 34919049964, carries exactly one H31 row: objectstack-ai#14122 (`tracking`), naming open PR objectstack-ai#18212 (draft, via a `Part of` declaration). Replayed offline against the 9 open PRs and the 9 open cards those PRs name (the exact H31 candidate set — a card no open PR names can never reach the comparison): | card | `tracking` | delivering evidence | row before | row after | |---|---|---|---|---| | objectstack-ai#18122 | no | objectstack-ai#18238 closing-keyword | none | none | | objectstack-ai#17502 | no | objectstack-ai#18231 closing-keyword | none | none | | objectstack-ai#17598 | no | objectstack-ai#18230 closing-keyword | none | none | | objectstack-ai#15410 | no | objectstack-ai#18227 closing-keyword | none | none | | objectstack-ai#18202 | no | objectstack-ai#18212 closing-keyword | none | none | | objectstack-ai#14122 | **yes** | objectstack-ai#18212 **part-of** | action-shaped finding | **DECLINED row** | | objectstack-ai#17396 | no | objectstack-ai#18198 closing-keyword | none | none | | objectstack-ai#17356 | no | objectstack-ai#18131 closing-keyword | none | none | | objectstack-ai#17097 | no | objectstack-ai#18096 closing-keyword | none | none | One row changes, and it is the expected one. Every other delivering binding on the live board is a closing keyword, so nothing else moves. ## Tests `pnpm check:pm-half-states` (= `node scripts/pm/check-half-states.mjs --self-test`): **4042 cases pass** before, **4075 cases pass** after, exit 0 — 33 new cases. The pin and the control both land, per the card's own remedies: - pin: the `Part of`-only tracker still produces a row (⛔ not a silent skip), the row says `DECLINES to judge`, names the binding it read, explains why a ruling anchor can never clear a gate, and prescribes NOTHING; it is asserted to be neither the old action-shaped sentence nor a LOUD row nor an UNJUDGED-ranked one. - control: a closing-keyword binding whose card lacks the gate — the row STANDS, with the unchanged action-shaped sentence. - plus: agreement on a weak binding stays clean both ways; a weak-bound card without `tracking` still reports the split but without the ruling-anchor clause; an adjudicable split outranks a decline and names only the closing-bound PR. **Ablation** (one-off, restored; run from the committed fix). Mutating `bindingClosesCard` back to the pre-fix wide population (`deliveryEvidence(pr, n) !== null`) turns the self-test RED: ``` == anchor counts BEFORE == old-text occurrences: 1 / new-text occurrences: 0 mutation written == anchor counts AFTER == old-text occurrences: 0 / new-text occurrences: 1 HEAD blob=dca4e1dbff5d6a46d10c5ac53a527b888e5f0811 MUT blob=906ac10ab7837ef5dc746f1bb3ccda8740e88c75 VERDICT ablation-exit=1 ✗ check-half-states self-test: 11 of 4075 case(s) failed. == restore leg == REST blob=dca4e1dbff5d6a46d10c5ac53a527b888e5f0811 old-text occurrences after restore: 1 / new-text occurrences after restore: 0 git diff HEAD bytes: 0 ``` The mutation is proved on disk by the blob hash moving off the HEAD blob and by the two anchor occurrence counts flipping; the restore leg is proved by the blob hash returning to the HEAD blob and by an empty `git diff HEAD`, not by an exit code. There is no build step and no `dist/` for a repo-root `.mjs`, so the rebuild half of the ablation preflight does not apply here. **Gates** — derived with no paths, at head `e0a4efa`: ``` node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack -> 41 command(s) node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran RECORD_FILE -> Run reconciliation — 41 derived, 41 run, 0 NOT-MEASURED, 0 UNRUN. EXIT CODES — all 41 accounted famil(ies) carry one, so the NOT-MEASURED count above is DERIVED from them. ``` All 41 exited 0, each captured by redirect before any pipe. The gate script's own wiring is inside that set: `pnpm check:pm-half-states` is the package.json step lint.yml runs, and it is the script's `--self-test`. **Lint** — a declared narrowing, not the farm. `eslint scripts/pm/check-half-states.mjs --no-inline-config --format json` at head `e0a4efa`: **1 file linted, 0 errors, 0 warnings**, exit 0. The population is read from eslint's own config, not guessed — an `ESLint` instance over `git ls-files` reports `tracked=8674 in-scope=6755 ignored=1919 no-rules=0`. Invariance: this repo runs one `eslint.config.mjs` which enables type-aware linting for no file at all (every `parserOptions` in it is exactly `{ ecmaVersion: 'latest', sourceType: 'module' }`; zero `project`/`projectService` hits; the config says so itself at its `QUERY_OPTIONS_TEST_GLOBS` note), and that config is not in this diff — so no rule's verdict on a file this PR did not touch can move because of it. The repo-wide `pnpm lint` run is CI's. `skip-changeset`: `scripts/pm/**` is PM tooling and ships in no package's `files[]`, so nothing published moves. ## Acceptance notes - `scripts/pm/check-half-states.mjs` is non-governed PM tooling, so this is in-seat review. `.github/workflows/half-state-patrol.yml` and anchor objectstack-ai#9857 are untouched: the workflow runs the script from `main`, so the fix reaches the anchor at the first scheduled sweep after landing. - objectstack-ai#18214 is the sibling shape one file over — the clause-② dual carrier that `scripts/pm/check-clause2-carriers.mjs --pair` demands on an epic tracker reached through a `Part of` line. It is out of scope here and remains open. `bindingClosesCard` is exported for it: that file already imports `deliveryEvidence` and `prDeliversCard` from this one, so the fix there is an import rather than a second copy of the predicate. - noted, not filed: the sibling install of this script in `objectui` (`scripts/pm/check-half-states.mjs`) has already drifted from this copy (different md5 at `b3b43b6`), while this file's header describes the pair as copied VERBATIM. No gate can hold two repos equal, so the wording is an aspiration rather than a claim — carrier: the next dev dispatched on a cross-repo patrol card. --- _Generated by [Claude Code](https://claude.ai/code/session_01HZfg2AwVX191qCizp88gQr)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
… that has one (objectstack-ai#18251) Fixes objectstack-ai#18174 `check-clause2-carriers --pair` read a review of record's `Served-tier:` line only beside a gate clear, so `references/contract-review.md` :29's 「无此行不成裁决」 was enforced on one path of two. ## The defect, measured One spelling, two answers, same script, same day: | pair | gate hung? | the record's `Served-tier:` | `--pair` | |---|---|---|---| | objectstack PR objectstack-ai#18157 / card objectstack-ai#17991 (record 5661052272) | no — `Clause-②` never declared `yes` | a stamp control, then prose, then the constant — unreadable to this file's own reader | **0** | | objectui PR objectstack-ai#9486 / card objectstack-ai#9191 (record 5662548425) | hung and cleared | the same spelling | **4**, C7 | The seam: `reviewOfRecord` answered `not-owed` before reading a thread unless the pair was in C6's completed state (`needsRecordRead` → `gateBindingState(pair).state === 'completed'`), and `c7ServedTierBelow` returned `null` for anything but `found`. Measured on this branch's base `b3b43b6`: a non-gated pair carrying an unreadable record, one carrying the live shape, and one carrying no record at all were indistinguishable — `pairRows` empty for all three, `reviewOfRecord` `not-owed` for all three. ## The change - **`locateReviewOfRecord(pair)`** — the gate-independent locator: the same imported H51 heading / head-sha recognition, the same `Reviewed-by:` third fact, the same newest-governs choice, with no reference to the gate. No recognition moved. - **`reviewOfRecord(pair)`** — that locator under C6's population gate. C6's row, its 「not-owed」 scope, its shape sentence and its remedy are unmoved, and both rows still read ONE comment chosen once. - **C7** and the **C6-RECORD note** read the locator. C7's accept set (one token, the constant's NAME), its exactness, its refusal of a missing line and its remedy are untouched — widened in POPULATION only, in the direction its own rule text names. Its opening sentence is now gate-aware: it names a clear only where a clear rides on the record. The note's citation half stays C6's, because the act it names is C6's. - **`gather`'s `landingReads`** (was `readFiles`) buys the PR thread for every pair on the `--pair` path, where the ruling puts this reading (「清标前」); the sweep's population is unchanged, for the budget reason C5's identical split already records. **`locatedRecordUnjudged`** is that path's own objectstack-ai#4690 half, so an unread thread is UNJUDGED and never clean. - **Consequence pinned, not left to be discovered:** with the PR thread in hand, `verdictThreadRows` hands C4 a thread it previously saw only on completed pairs, so a verdict on a non-gated or still-hung pair is now judged for the independence pair it declares. That direction only adds verdicts to a reading whose newest-governs rule already lets a later independent verdict displace an older self-review. Pinned in both directions; the sweep's blindness there is restated as the limit that survives. ## Evidence **Reproduction on the real historical pair, replayed offline.** Both PRs are merged, so `--pair` cannot form the pair from the live board; the document carries the fetched `/pulls/18157`, `objectstack-ai/issues/17991`, both comment threads and the file list, and nothing else. - before (`origin/main` `b3b43b6`): `--pair 18157 --pair-json …` → exit **0** - after: exit **4** — C7 naming comment 5661052272, the unreadable line quoted back, and its stamp control read as NOT total (the 「回退证据」 half) **Live control**, PR objectstack-ai#18243 / card objectstack-ai#18229 — open, non-gated, `Clause-②: no`, record 5673963268 in the template's shape: exit **0** before and after. After, the run also prints the C6-RECORD note naming that record and saying the pair owes no clear (4 reads → 5). **The sweep is unmoved, deterministically:** the same document in sweep mode produces byte-identical `--json` output before and after. A live gated pair (objectstack-ai#18212) reads exit 4 with the same rows on both. **Self-test:** 629 → 658 cases, `pnpm check:pm-clause2-carriers` exit 0. A new battery (`objectstack-ai#18174: …`, 28 cases) carries the measured non-gated spelling beside the live one; the roster floor moves 22 → 23 and C7's own floor 42 → 43. Four of C7's population pins were re-triaged rather than left green for the wrong reason: each now spells a pair whose threads were READ and carry no record, instead of passing because no thread was ever fetched. **Reverse verification**, from the committed fix: `c7ServedTierBelow`'s locator call mutated back to `reviewOfRecord`. On-disk proof — injected marker grep = 1, deleted anchor grep = 0, blob hash `5a79776b` → `c4757e7a`. Self-test under the ablation: **5 of 658 red** (the measured pair, its two sentence pins, the still-hung pair and the 重挂 pair). Restored with `git checkout HEAD --`: `git diff HEAD` empty and the blob back at `5a79776b`. Direction: turns red, as predicted. **Gates**, union re-run at `7af15586` (`git rev-parse --short HEAD`), working tree clean: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` with no paths derives **35** families; all 35 ran in the foreground with exits captured by redirect, all **exit 0**; `--ran` reconciles **35 derived / 35 run / 0 NOT-MEASURED / 0 UNRUN**, every exit code recorded. Beyond the derivation: `pnpm lint` (repo-wide, `eslint . --no-inline-config`) exit 0, and the two siblings that import this module — `check-governed-queue-guard.mjs --self-test` (238 cases) and `check-half-states.mjs --self-test` (4042 cases) — exit 0. `skip-changeset`: `scripts/pm/**` is repo tooling that no package `files[]` ships. ## Acceptance notes - noted, not filed: in a SWEEP a record on a non-gated pair stays invisible, because the sweep buys no PR thread for it. That is a declared limit, written into this file's budget paragraph beside C5's identical one, not a defect — and 承接者: nobody today; it becomes a card only if a seat ever prices a per-pair sweep thread. - noted, not filed: an existing pre-fetched `--pair-json` document that omits the `comments` entry for the PR now reads UNJUDGED (exit 2) on a pair that owes no record, where it read 0 before. That is this change's own documented owing, restated in the doc-shape paragraph, not a separate finding. 承接者: the MCP-only seats that hand this file a document — the file's own refusal text names the key to add. - The C4 reach on the landing path widens as a consequence of the bought thread (see above). In scope, documented and pinned in both directions. --- _Generated by [Claude Code](https://claude.ai/code/session_01HZfg2AwVX191qCizp88gQr)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #18202
Part of #14122 — the epic tracking the one-artifact/N-packages family. That tracker stays open.
Rework after a contract-review FAIL
The isolated contract review on head
b0072379returned FAIL and the bolded compatibilityclaim this body used to carry was false. Three corrections landed on this branch; nothing else
moved, and the changeset level stays
minor(the review judged the level correct and the bodywrong).
Array.isArrayguards and 0 malformed warnings, while step 3 of the samecomposeStacksbody carries 3 and 1 — so a malformedpermissions/dataon an unparsed input raised a bareTypeError(nocode, nostatus, outside the ADR-0112 envelope) wheremaincomposes with a warningwarnMalformedCollectionKeyand skipped, and a non-object entry — or a non-stringobject— carries no reference for the rule to resolve and is skippedfor (const stack of stacks)over every input, whiledefineStackreturns atif (!strict)before cross-reference validation ever runs — sostrict: falseand hand-built inputs are newly refusedif (stacks.length === 1) return stacks[0];is unchanged, so a one-input composition never reaches the pass, and the docs bullet and option docstring overstated the guaranteeRound 2 of the rework — one clause, two surfaces
The second contract review re-measured all three findings above and recorded them resolved
(36 probes; the guards hold with no
TypeError, thestrict: falserefusals are pinned by fivepositive fixtures, the one-input qualifier is fenced, and it confirmed the false-green fixture
correction). It failed on one sentence. The changeset and this body described the newly-refused input's prior
fate with the phrase
previously composed with only a warning— quoted here as the text that wasREMOVED, and appearing nowhere else in this PR.
That was false. On
mainthis class of input produces no diagnostic at all —composeStackscalls
validateCrossReferenceszero times andconsole.warnzero times, and its onewarnMalformedCollectionKeysite fires ondeclared.length !== arrays.length, i.e. a collectionkey that is not an array. The newly-refused inputs carry well-formed arrays holding a dangling
reference, so that condition never holds. The clause conflated two populations and handed an
upgrading reader a false self-test — "we never saw a warning, so this is not us" — in text that
ships verbatim into
CHANGELOG.md.Corrected in the changeset and here. Taken in the same stroke, the precision the review recommended:
the no-op half of the invariant holds for an input that passed the strict parse and did not opt
in, which is now said in the changeset, in the
collectArtifactCrossReferenceErrorsdocstring andin the fixture file's header, which carried the same sentence. Text only — no schema, no rule, no
fixture and no docs page moved, and the level stays
minor.What changed
defineStackvalidated cross-references per stack, so "defined inobjects" and "defined in the artifact" were the same question. In an ADR-0130 artifact they are two questions, and the two accepted records could not both hold once thetype: apppackage owns objects of its own:type: apppackage, because a set is authored per ROLE and ADR-0086 D3 gives it exactly one owning package.So the app package's sets necessarily grant on its modules' objects, and every such grant was refused. Measured downstream on
objectstack-ai/hotcrmclaude/issue-1907-sales-app-service-module(be11c07) as 18 grants across 7 sets;data[].objectwas refused the same way, which is why that branch had to push its seed rows into the module.This PR gives the two reference classes that legitimately cross a resolution scope of their own, and moves their refusal to the pass that owns that scope.
DefineStackOptions.artifactObjectspermissions[].objectsanddata[].objectresolve against the stack's own objects plus these names. Nothing else in the function moves.The measurement that chose the mechanism
The dispatch proposed gating the widening on the stack declaring non-empty
manifest.dependencies. Measured against the reporter's own repro, that gate does nothing for this defect, and the finding is worth recording because the card's title carries the same framing:src/salesapp.objectstack.hotcrmtype: app, nodependenciescrm_case— this is the failing stacksrc/serviceapp.objectstack.hotcrm.servicetype: module,dependencies: { 'app.objectstack.hotcrm': '^3.1.0' }The dependency edge runs module to app, which is the only direction ADR-0116's topological order admits: the module registers after the package it extends. The app therefore cannot name its modules as dependencies without inverting that order. Resolving against the referencing package's declared dependency closure would refuse exactly the shape the addendum makes mandatory.
⇒ The resolution scope is the artifact, which is what ADR-0130 D1 says it is — not the dependency closure, and not "anything, checked later".
The two classes that cross, and the ones that must not
Every object-name reference class the check covers, with the per-class ruling:
permissions[].objectsdata[].object(seed)hooks[].objectnavigationobjectNamerequiresObjectremains the declared per-item escape)data.object,mappings[].targetObject,actions[].objectName(both positions)The first two blocks of the new fixture assert that
artifactObjectsdoes not widen the hook and navigation refusals even when the name is listed.The refusal MOVED —
composeStacksis the named passcomposeStacksdid not re-run any cross-reference check at all (it only checked composed action-key collisions), so deferring without adding one would have opened a hole. It now re-checks the two artifact-scoped rules over the composed object set:code—STACK_CROSS_REFERENCE_INVALID— and the samestatus: 422;composeStacks artifact cross-reference validation failed (N issues):— because only the pass differs.A name
artifactObjectsclaims and no package in the artifact defines is therefore still refused, loudly, at a named pass — in a composition of two or more packages. That fixture is an acceptance criterion of the card, and it isArtifactPassin the new test file.What it can and cannot newly refuse
An earlier revision of this body claimed the pass "cannot newly refuse a composition that passes today". That is false, and the contract review measured it twice. The invariant that actually holds is two statements, not one:
defineStackparse AND did not opt in cannot newly fail here. It does not read the option (recording the option on the returned stack would put a composition-time concern onObjectStackDefinitionSchema, i.e. on every artifact already built). It re-runs the two rules for every input, which is a no-op for such an input — its references already resolved against its own objects, and those are a subset of the composed set. The qualifier is load-bearing: an input that did opt in also passed the strict parse, but it resolved against its own objects plus the names it listed, and checking a listed name against the real artifact is precisely what this pass is for — so an opted-in input can fail here by design. That is theArtifactPassfixture, not a gap.defineStack(config, { strict: false })returns beforevalidateCrossReferencesruns at all, and a hand-built stack object never enters it. Such an input carrying a danglingpermissions[].objectskey ordata[].objectis now refused at composition where it previously composed with no diagnostic at all — the existing non-array warning covers a malformed collection key, not a dangling reference.The second is a real narrowing of what
composeStacksaccepts. It is the Prime Directive #12 direction — reject off-spec input loudly — and it is now declared: the changeset states it,collectArtifactCrossReferenceErrors's docstring states it, and the fixture pins it rather than leaving it to be rediscovered as a regression.What the narrowing is not is a licence to crash. Both collectors guard their own shapes, so an unparsed input carrying a non-array
permissions/data, anullentry, or a non-stringobjectis warned about and skipped the way step 3's concat pass already treats it — never a bareTypeErroroutside the ADR-0112 envelope.Inputs declaring no objects are still skipped, inheriting
validateCrossReferences's ownobjectNames.size === 0leniency verbatim; that leniency is why hotcrm#1449's measurement never saw the defect, and it is pinned as untouched in both directions.Known boundary — this pass runs only for two or more packages.
composeStacksreturnsstacks[0]untouched for a single input, so a one-package composition never reaches the pass, and neither does a stack that is never composed. That is the population whoseartifactObjectsclaim stands unverified — the same shape asstrict: false, for the same reason. Every reader-facing statement of the guarantee now carries the qualifier.os buildcomposes.Compatibility — #14122 §6
Nothing in this PR touches the load path, the artifact schema or any authored key.
defineStackis unchanged for a stack that does not passartifactObjects: the widened scope is literally the sameSetobject as the narrow one when the option is absent. The three existing cross-reference pin suites pass unchanged, andcheck:authorable-surface/check:api-surfaceare both green without regeneration.composeStacksis where the acceptance set does move, for exactly one population — inputs that bypassed the strict parse — and that is stated above, in the changeset and in the docstring rather than claimed away. §6's own subject (a stored single-manifestartifact replayed through the load path) is untouched by it: the artifact pass runs at composition time, on the inputs, and never at load.Verification
Re-run in a worktree off
origin/maind4554d4, at0c21631c— the rework head. Every exit codebelow was redirected to a file and captured before any pipe.
pnpm --filter @objectstack/spec testpnpm --filter @objectstack/spec typecheckcheck:test-typecheck)vitest run src/stack-artifact-crossref.test.tspnpm --filter @objectstack/spec check:generatednode scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 106; each run singly with its exit code captured before any pipe, then--ranreconciledpnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' --concurrency=2pnpm lint(eslint . --no-inline-config) — the whole union, not a narrowing0c21631cSeven of the 106 first returned a PREREQUISITE reading — six
exit 3(PREREQUISITE NOT MET) andcheck:skill-examplesexit 1whose body says "packages/client-react/dist holds no .d.tsdeclarations — build first, then re-run". Those are NOT-MEASURED, not reds: all seven were re-run
after the closure build and all seven exit 0. They are
check:dual-build-cjs-loads,check:lean-entry-closure,check:type-check-debt,check:docs-transcript-drift,@objectstack/lint check:doc-formula-expressions,@objectstack/lint check:doc-security-postureand
@objectstack/spec check:skill-examples.Carried from the pre-rework head
b0072379, and why it is still valid: the consumer sweep(
turbo run typecheck --filter='...@objectstack/spec', the DOWNSTREAM direction, 139/139) was notre-run. The rework changes two internal function bodies, docstrings, a fixture, a changeset and a
docs page — no exported type, no published
exportsentry.check:api-surfaceis green with noregeneration owed and
check:generatedreports all 15 artifacts up to date, so the publisheddeclaration surface is byte-unchanged and no import site can have moved.
Known staleness, declared: this branch is 12 commits behind
origin/mainanddispatch-gates.mjssays so in its own words — six of the files its derivation reads changed acrossthat range. The 106 is therefore this tree's derivation, not
origin/main's. The branch isdeliberately not merged forward: it is a targeted rework of an open PR that a reviewer is about to
re-read against its recorded FAIL, and CI runs the full farm on the merge result anyway.
Reverse verification (one-off, not committed tests)
Two legs, each committed first, then mutated, then restored under a
trapwhose restore path isabsolute and spelled
git checkout HEAD --. Both restores are proven by blob identity and an emptygit diff HEAD, never by an exit code. No build step is involved: these tests import./stack.zodby relative path, not through the package
exports.Leg 1 — the shape guards. Prediction, written before the run: removing the two guards turns the
malformed-input block red with bare
TypeErrors. Mutation proven on disk —Array.isArrayinsidethe two collectors 2 to 0,
warnMalformedCollectionKeycall sites 3 to 1, blob8debc710vs HEAD690c80dd. Observed: 5 failed / 25 passed, every failure a bareTypeError(
permissions is not iterable,datasets is not iterable,Cannot read properties of null,name.startsWith is not a function). The sixth case in that block stays green under the mutationand is honestly a control, not a guard pin:
objects: nullon a permission entry was alreadycovered by the pre-existing
!grants || typeof grants !== 'object'line.One correction worth recording, because the first attempt of this leg measured a false green: the
non-array
permissionsfixture originally used a string, and a string is iterable, so removingthe guard changed nothing for it. It now uses map format — not iterable — and fails without the
guard. The fixture was fixed and the leg re-run from the committed state.
Leg 2 — the one-input boundary. Prediction: deleting
if (stacks.length === 1) return stacks[0];turns the two "accepts a one-input composition" fixtures red and leaves the rest green, proving those
fixtures actually observe the early return rather than merely asserting an accept. Mutation proven on
disk (occurrences 1 to 0, blob
e7c7b384vs HEAD690c80dd). Observed: exactly 2 failed / 28passed, both in the one-input block. Restored,
git status --porcelainempty.docs-drift-checkadvisory — decided, not deferredThe advisory flagged five pages. Decisions, so a reviewer does not re-derive them:
content/docs/getting-started/glossary.mdx— namescomposeStacks()once, in passing ("the authoring form … or composes one withcomposeStacks()"). Nothing it states is falsified. No edit.content/docs/getting-started/examples.mdx— its "rules worth knowing before you split a product this way" list is the page a reader lands on for exactly this shape, and nothing already there becomes untrue. But the list would have been silently missing the rule this PR creates, so it gains one bullet naming the two artifact-scoped classes, theartifactObjectsspelling, and the hook exception. That is the documentation half of the option, not scope creep.content/docs/releases/v17/*(three pages) — release-owned under AGENTS.md's Documentation Guardrails, and read-only in a code PR. Not edited here, and none of them was found to be wrong: they describe shipped behaviour this PR does not change.Note the advisory was computed on the merge commit
318def6e, not on this PR's head, so a local re-derivation can legitimately differ.Acceptance notes
check-plugin-teardown-shape.mjs --self-testexits 1 on a shallow checkout — it pins its positive control to a commit the clone cannot reach and says so rather than passing. Fetching that one commit clears it (47 cases pass). Not a finding, and not this PR's to change.PREREQUISITE NOT MET) until the package closure is built:check:dual-build-cjs-loads,check:lean-entry-closure,check:type-check-debt,@objectstack/lint check:doc-formula-expressions. All four are green after the closure build. Working as designed; recorded so the exit-3 readings in this PR's history are not read as reds.pnpm check:role-wordcaught it in the same local run and it now says position. Recorded because the word is ratcheted repo-wide and the gate is easy to meet only if you know it exists.StackCrossReferenceErrordocstring said "there is exactly ONE raise site". With the artifact pass there are two, so the paragraph now names both and says why they share onecode: the code names the rule family (what a machine matches), the header names the pass (what a reader needs).composeStacksstep 2 raises a bareTypeErrorfor a non-arrayobjectson an unparsed input, and always has.mergeObjectsiteratesstack.objectswith no guard; probed against the builtdist,objectsas a map andobjectsas a number both giveTypeError: stack.objects is not iterable,codeundefined,statusundefined.mergeObjectsis not touched by this diff (git diff d4554d4f5..HEAD -- packages/spec/src/stack.zod.tshas no hit on it), so this is a pre-existingmaindefect of the same family as the one this rework fixes, not a regression. It is reported for filing rather than fixed here: the rework dispatch is explicitly scoped to the three review findings, and widening it would be the second time this PR's scope moved. Dedupe words:composeStacks,mergeObjects, non-arrayobjects, bareTypeError, ADR-0112 envelope.isPlatformObjectNamecallsname.startsWithwith no string guard. No live call site can reach it with a non-string now that the seed-data collector filters ontypeof === 'string'(the other caller passesObject.keysoutput), so this is noted, not filed.warnMalformedCollectionKeycall added inside each collector can never be the FIRST to warn for its key in today's code: step 3 ofcomposeStacksruns before step 3b and warns for the same key, and the reporter deduplicates per key for the module's lifetime. It is kept anyway so the rule cannot become the silent one if a future caller reaches it first, and the fixture asserts the observable consequence — exactly ONE warning per key, not two.Follow-ups this PR does not take
data.object,mappings[].targetObjectandactions[].objectNamestay stack-scoped. Whether a co-owning package may carry a view over, an import mapping into, or an action bound to a sibling's object is a contract question no record answers, and the repro does not need it. Flagged for thedomain:specseat rather than decided here.Errorenvelope gap on these refusals is finding(spec):defineStack's cross-reference refusals are bareErrors — no ADR-0112code/status— so five REFUSED item classes in the ADR-0130 matrix are distinguishable only by message text #14552's, deliberately untouched — and the class this PR moves keeps its envelope shape: both passes raiseStackCrossReferenceError,codeSTACK_CROSS_REFERENCE_INVALID,status422,issuesone entry per finding.defineStack's per-stack cross-reference validation refusespermissions[].objectsanddata[].objectthat name an object owned by ANOTHER PACKAGE OF THE SAME ARTIFACT — so ADR-0130's co-owned artifact and the "permission sets stay whole in the app package" addendum cannot both hold once the app package owns objects #18202 rather than only in a review thread.artifactObjectsthreaded intocreateHotCrmAppStackand a local@objectstack/speclink; the sister checkout is read-only here and that branch belongs to another card, so this PR relies on the fixture, which reproduces the two-package shape at its smallest.packages/cli,packages/lintandexamples/**are untouched — the sibling dispatch on cli:os build/os devper-package author-time rules refuse an action param's record-pickerreferenceto a dependency's object (object-reference-unknown), while the composed pass accepts it and ADR-0130 R1 accepts the field-level equivalent #18204 owns them this round.Generated by Claude Code