Repository navigation
feat(lint)!: relationship/master-detail-required refuses the three unsafe master-reference shapes at error on a controlled_by_parent object - #22109
Conversation
…safe master-reference shapes at error on a controlled_by_parent object Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
…p-2 pin; add the v18 migration entry Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
…required-lint-error Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
…int's error tier and its reach Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
…ng test slices rule bodies by export) Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
…ginal thread no longer resolves) Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848
📓 Docs Drift CheckThis PR changes 2 package(s): 11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 6 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 139 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 34256403b5ff388bdf117502267d320ea30c5aa6 && git checkout 34256403b5ff388bdf117502267d320ea30c5aa6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin aa71c4d9d146861fc09d63363e4c8fb8286719fb bee9c1e2525ec655c1ee87c51f91081d2cf08c06 && git checkout -B drift-repro aa71c4d9d146861fc09d63363e4c8fb8286719fb && git merge --no-ff bee9c1e2525ec655c1ee87c51f91081d2cf08c06
node scripts/docs-audit/affected-docs.mjs --json aa71c4d9d146861fc09d63363e4c8fb8286719fb
|
Contract reviewServed-tier: Isolated at-tier review of PR #22109 (card #9139) against Check-runs on this head, read 2026-10-07T17:23:57Z: 38 runs — 29 ① Derived judgmentsEvery accept-set and public-surface change the diff implies, each named right or wrong:
② Semver level
③ Boundary flagsDev report
Implemented-by: VERDICT: PASS |
…er-detail-required (objectstack-ai#22370) Fixes objectstack-ai#22111 Clause-②: no The published `objectstack-data` skill's lint-rules table said `relationship/master-detail-required` is a `warning` everywhere. Since the lint change that landed with the parent card (objectstack-ai#9139, PR objectstack-ai#22109), that is false on one class of object. This PR changes that one table row so both cells are true in both cases, in the table's own terse style. ## The row Before (`skills/objectstack-data/references/lint-rules.md:13`): ``` | `relationship/master-detail-required` | warning | a `master_detail` that isn't `required` (a detail can't exist without its master) | ``` After: ``` | `relationship/master-detail-required` | warning; error under `controlled_by_parent` | a `master_detail` that isn't `required` — or, under `controlled_by_parent`, is `readonly`/`system` | ``` ## What the rule does on `main` (verified in code, each fact at its line) - `packages/lint/src/data-model-rules.ts:256` — one rule id, `relationship/master-detail-required`, on both tiers. - `:863` — the tier is chosen by `obj.sharingModel === 'controlled_by_parent'`. - `:324-325`, `:344` — on a `controlled_by_parent` object the finding is `severity: 'error'` whenever `required !== true`, or `readonly: true`, or `system: true` (so `required: true` + `readonly: true` and `required: true` + `system: true` are refused there). - `:866-868` — on every other object only `required !== true` fires, at `severity: 'warning'`, unchanged. - Pinned by `packages/lint/src/data-model-rules.master-detail-required.test.ts:52-57` (the three unsafe shapes at `error`) and `:111-131` (`private` / `public_read` / `public_read_write` / unset: still a `warning`; `readonly`/`system` draw nothing there). ## Budget — the file sat at its token ceiling `scripts/check-skills-token-ratchet.mjs` holds this file at 970 tokens and it was at 970/970 (headroom 0), so a longer row cannot land without deleting content in the same file. The payment is the intro clause "not just naming/labels but the relationship/master-detail/roll-up patterns", which duplicated the table's own heading "Data-model rules (in addition to naming/label/i18n)"; the row also drops its parenthetical rationale. Re-wrapped so the file keeps its line count. | reading | before | after | |---|---|---| | lines | 53 | 53 (net 0) | | bytes | 3880 | 3856 | | tokens (`ceil(bytes/4)`, ceiling 970) | 970 (headroom 0) | 964 (headroom 6) | | the row, bytes | 136 | 191 | `scripts/pm/check-skill-line-ratchet.mjs` does not cover the published `skills/` root (its header says so by design), so the token ratchet is the only ratchet on this file; it stays green and its ceiling row is untouched. ## Scope check inside the skill The only other sentence in `skills/objectstack-data/**` that speaks to this rule's severity is `rules/relationships.md:16` — "Forced only under `controlled_by_parent`; else lint-warned". That is true as written (the builder forces `required` under `controlled_by_parent`; the lint warning is the other case), so it is left alone. ## Changeset `skills/**` is in no released package's `files[]` (no `package.json` under `packages/` names it; the catalog ships through `npx skills add` from this repo), so this diff publishes nothing from any released package: no `.changeset/*.md`, and the `skip-changeset` label is the repo's skip form. ## Gates Derived from the merge base at `6dc260aae` with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (24 families), each run with its exit code captured before any pipe, then reconciled with `--ran`: `24 derived, 24 run, 0 NOT-MEASURED, 0 UNRUN`. - 23 of 24 exit 0 on the first pass. `pnpm --filter @objectstack/lint run check:doc-formula-expressions` exited 3 (PREREQUISITE NOT MET: `@objectstack/formula` and `@objectstack/lint` not built); after the gate's own fix line, `pnpm exec turbo run build --filter=@objectstack/formula --filter=@objectstack/lint`, under the verify lock (`VERDICT command-exit 0`, held 161s, waited 0s), it exits 0 — "22 record-scoped formula example(s) across 471 files / 1384 TS blocks judged clean". - `node scripts/check-skills-token-ratchet.mjs` exit 0 — "skills/objectstack-data/references/lint-rules.md is 964 tokens (ceiling 970; headroom 6)"; its `--self-test` exit 0. - `pnpm --filter @objectstack/spec run check:skill-refs` exit 0 (it generates `references/_index.md` from the Zod map, not from this file, so nothing to regenerate). - `pnpm check:nul-bytes` exit 0; a control-byte scan over the edited file finds none. - Artifact-roster block (the 52 families the derivation scores silent for every card): 47 exit 0. Three are PR-context guards that refuse without `PR_NUMBER`/`PR_BODY` — `check:partof-closing-keyword` re-run with this body exits 0; `check-closing-target-claim` and `check-single-claim-paths` are run against the PR once it exists, result in the report comment on the card. Two need a whole-tree `dist` (`check:dts-closure`, `check:published-readme-exports`: exit 3, PREREQUISITE NOT MET) — NOT MEASURED locally; a skills-only diff cannot move either roster and CI runs both. - `pnpm lint` (repo-level eslint) was not run locally; the diff is one `.md` file. CI owns that run. ## Acceptance notes - The card's landing text assumed a line ratchet on `skills/**`; there is none — the binding gate was the token ratchet at zero headroom, which is why a one-cell change carries an intro-clause deletion. - Governed surface, Tier H (`skills/**`): this PR stays draft; landing waits for an authorized approval and the seat's contract-tier review of the skills hunk. ## 维护者速读(草稿) - **改了什么:** 已发布的 `objectstack-data` 技能里,lint 规则表中 `relationship/master-detail-required` 这一行。原来只写 `warning`;现在写明:默认 `warning`,在 `controlled_by_parent` 对象上是 `error`,且在那里 `readonly`/`system` 的主从引用也会被拒。 - **为什么改:** `os lint` 已经按两档执行(PR objectstack-ai#22109 落地)。技能文本还在教"到处都只是建议",按它写出的 `controlled_by_parent` 明细对象会被 `os lint` 拒绝。文件已顶到 token 上限(970/970),所以多出来的字用删掉一句与表头重复的引言来付,行数不变。 - **风险与代价(含回滚):** 纯文本改动,不碰代码、不发包、无 changeset。回滚即还原这一个文件的一次提交。 - **席位意见:** - **你要做的:** 看一眼第 13 行这一行表述是否认可,认可就给一个批准。 --- _Generated by [Claude Code](https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848)_ Co-authored-by: objectstack-fleet[bot] <noreply@anthropic.com>
Fixes #9139
Clause-②: no
What this does
relationship/master-detail-required(R2 inpackages/lint/src/data-model-rules.ts) now has two tiers under one rule id, as the maintainer ruling of 2026-08-16 (Direction 1, scheduled for the v18 boundary) orders.sharingModel: 'controlled_by_parent', everymaster_detailfield is refused aterrorin each of the three unsafe shapes:requiredabsent (orfalse);required: true+readonly: true;required: true+system: true. One finding per field, located at the first defect (.required,.readonlyor.system); thefixnames every edit the field needs.warningfor amaster_detailwithoutrequired: true, with the same message and fix, and silence on the two flagged shapes.Before this change the predicate was
required !== trueatwarningon every object. Two of the three shapes drew no finding at any severity, as the census note on #9139 measured. A one-line severity flip would have closed one shape and left two open, so the predicate covers all three. Each shape is pinned on its own.Runtime is untouched, as the card's scope requires:
resolveCbpRelation's fallbacks and the security gate stay as they are.The mechanism hypotheses, measured
data-model-rules.ts:742,warning,required !== true, every objectb04a5295f. Before-probe: CBP and non-CBP alike gavewarningfor absent /false, and nothing for+readonly/+system.:817pin filters toSECURITY_CBP_NO_RELATION, so promoting R2 leaves it green untouchedresolveCbpRelation) is silent. That stays true, because the runtime still resolves step 2. The pin is reversed, not removed: see the pin section below.os lintand the eval rubric onlylintDataModelis called fromlintConfig(os lint, exit 1 on any error) and fromscoreMetadata(the generation rubric:validrequires zero errors). It is not anAUTHORING_RULESentry:authoring-rule-wiring.test.tspinslintDataModelinDIRECT_CALL_RATCHETas lint-only. Soos build,os validateand the metadata save door do not run it.check:i18n-coveragespawnsos lintbut tolerates a non-zero exit. Registration was not widened; see the open question in the report.forceCbpMasterDetailRequired(object.zod.ts) forces an omittedrequiredtotrueunder CBP and refuses an explicitfalse. It skips the array field form and never inspectsreadonly/system. So throughObjectSchema.createonly shapes 2 and 3 reach the lint, and both pass the builder untouched. Shape 1 reaches the lint from anything not built throughcreate: a plain object literal, the array form, or raw parse of stored metadata. Pinned with realObjectSchema.createobjects.entries/semantic/, andgen:migration-registrysorts the registry by id. The new entry iscbp-master-detail-required-lint-error. Whichever of this PR, #22094 and #21974 lands later mergesmainand re-runs the generator. Per the entries README's measured table, a driver-less merge conflicts only when two new ids are adjacent in sort order. Whether that holds against those two PRs' ids is NOT MEASURED.The step-2 pin: reversed, not removed
validate-security-posture.test.ts"stays silent on step 2: ANY master_detail (not marked required)" pinned the step-2 shape as supported. The ruling retires that reading as a deliberate contract narrowing. Under H2 the pin's assertion is about the runtime mirror, and that half stays true, so the pin now asserts both halves on one stack:resolveCbpRelationstill resolves a non-requiredmaster_detail, and metadata at rest keeps loading;relationship/master-detail-requiredaterrorfromlintDataModel.The pin's title and comment state the narrowing. The pin is kept rather than deleted, so a later change that re-tolerates the shape at authoring time, or drops the runtime tolerance, turns it red.
Census (H5), at
b04a5295fplus this changeEvery
*.object.tsoutside tests and fixtures (111 files, 0 import failures), plus the CLI's golden eval corpus and the multi-package example's two sub-stacks. Each corpus was linted with its own controls appended to its own array: one positive control per unsafe shape and a clean negative control.controlled_by_parentNo stored in-tree metadata turns red.
v18 upgrade-checklist line (for the v18 release notes)
The changeset's Remedy section carries this line verbatim. The step-18 semantic entry
cbp-master-detail-required-lint-errorcarries the same prescription foros migrate meta. This PR does not touchcontent/docs/releases/**.Release grading
.changeset/pre.jsonis absent onorigin/main: read atb04a5295f(2026-10-07T15:37Z) and again atbafb58bb0before this push. So the changeset grades@objectstack/lintand@objectstack/specatminor, with the BREAKING banner,Clause-②: no (narrowing)in its body, and the ADR-0087 dispositionregistered cbp-master-detail-required-lint-error.Files
packages/lint/src/data-model-rules.ts: R2's CBP tier (cbpMasterReferenceFinding), placed above the first exported rule.authoring-rule-wiring.test.tsreads a rule body as the source text up to the nextexport, so an error-emitting helper placed belowlintLegacyOrganizationCompositeswas read as that advisory rule emittingerror. That was measured red once, then fixed by moving the helper.packages/lint/src/data-model-rules.master-detail-required.test.ts(new): 19 cases. Each unsafe shape aterrorwith its path; two clean controls; one finding for several defects; everymaster_detailfield; the array field form; a lookup ignored; non-CBP unchanged across 4 sharing models plus the two flagged shapes; and realObjectSchema.createobjects.packages/lint/src/validate-security-posture.test.ts: the step-2 pin, reversed as described above.packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.tsand the regeneratedregistry.ts. The entry carries no tracker id and no call spellings.spec-changes.jsonand the upgrade guide do not project step 18 yet, andcheck:generatedreports all 15 artifacts current.packages/cli/test/score.test.ts: fixture triage. The "warning" fixture of "suggestions cost less than warnings cost less than errors" was a CBP object, so it now measured the error weight. Probe:counts.errors1,valid: false, with R2 aterror. Its assertions still passed, but for a different reason. The fixture moves tosharingModel: 'private'and two assertions pin it to the warning tier.content/docs/protocol/kernel/error-handling.mdx: this change made one sentence false ("does not report thereadonly,system… shapes at all"). It now namesos lint's error tier and its reach..changeset/9139-cbp-master-detail-required-error.md.Verification (final head
bee9c1e25)All readings below were taken at
bee9c1e25, after the last commit. Each exit code was captured before any pipe.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackgave 113 commands. All 113 ran atbee9c1e25: 112 exit 0, 1 exit 3. The exit 3 isnode scripts/check-plugin-teardown-shape.mjs --self-testrefusing its own prerequisite, because its positive control is pinned to commit621a4876outside this shallow clone. That is NOT MEASURED, not a finding; CI runs it on a full checkout. Reconciled with--ran: "113 derived famil(ies) accounted for — 112 run, 1 NOT-MEASURED, 0 UNRUN". Gate lines quoted from the run:check-adr-0087-registration: "1 declared-breaking changeset(s), each carrying an ADR-0087 disposition … [BREAKING+bang+clause-②-narrowing] registered cbp-master-detail-required-lint-error (new here: …)"check:migration-registry: "registry.ts is current (382 semantic, 247 retired-key, 218 retired-def)"check-issue-citations: "every citation this change adds resolves"check:nul-bytes: "OK … no raw ASCII control bytes"check:doc-authoring: "17816 customer-facing string(s) … clean"@objectstack/lint:pnpm --filter @objectstack/lint testpassed 122 files / 5665 tests.pnpm --filter @objectstack/lint typecheckwas green; "check:test-typecheck: OK".@objectstack/spec:vitest run src/migrations scripts/build-migration-registry-entry.test.ts scripts/step18-rationale-merge.test.tspassed 5 files / 191 tests.pnpm --filter @objectstack/spec check:generatedwas "All 15 generated artifacts are up to date". That run was on top of3f6d9ca97(the registry regeneration);packages/spechas not changed since, and its per-artifact gates are in the 113 above.@objectstack/cli(consumer that reads R2 findings):vitest run --project unitpassed 259 files / 3795 tests.pnpm --filter @objectstack/cli typecheckwas green. Theintegrationtier is declared to CI;migrate-meta-engine-guidance.test.ts, which holds every semantic entry's printed prose, lives there.eslint --no-inline-config --format jsonover the 6 touched.tsfiles counted 6 files, 0 errors and 0 warnings.--print-configresolves all 6 inside the config. The.md/.mdxfiles answer "File ignored because no matching configuration was supplied". The config enables no type-aware linting (parserOptionsis{ ecmaVersion, sourceType }, with noproject), so this diff cannot move a verdict on an untouched file. The repo-widepnpm lintis CI's run.Reverse verification. With
data-model-rules.tsreverted tob04a5295fand the tests run from the committed state, 10 failed / 142 passed. The failures: all 4 CBP shape cases, several-defects, every-field, array form, the 2ObjectSchema.createrefusals, and the reversed step-2 pin. The controls and every non-CBP case stayed green, as expected. The file was restored withgit checkout HEAD -- …, and the restore was proven by blob hash equal to HEAD's and an emptygit diff HEAD.Acceptance notes (not changed here)
skills/objectstack-data/references/lint-rules.md:13lists this rule's severity aswarning. That is now true only offcontrolled_by_parent.skills/**is a Tier H governed surface outside this claim's file surface, so it is left for the seat (see the report's open question).packages/plugins/plugin-security/src/security-plugin.ts, the paragraph above the freeze note: "Direction 1 … has NOT landed … nothing warns on the way past". It says itself that it "goes stale when lint: promoterelationship/master-detail-requiredfrom warning to error, scoped tocontrolled_by_parent— ruled for the v18 boundary (Direction 1 of #8772) #9139 lands". The dispatch fences the runtime package, so it is left for the seat.object.zod.tsdocblock offorceCbpMasterDetailRequired, and the sibling entrycbp-master-detail-required-forced, say the lint "stayswarninguntil v18". That is still accurate as a schedule, so neither was edited.showcase_field_zoo.f_master_detail) was already fixed onmain(required: true). The global warning count is now 0.Generated by Claude Code