Repository navigation
docs(spec): name the mechanism each of two docblocks describes - #24290
Conversation
The ActionParam options docblock's visibleWhen bullet quoted objectui's inline param branch as `options: param.options`; at the console pin it reads `options: authoredOptions` (resolveOptionLabels over the authored list). The cbp-master-detail-required-defaulted conversion's docblock said an update nulling the master was refused before by the security gate; that gate reads the stored row's FK on a by-id update, and the refusal is the record validator's ADR-0113 check, which only `required: true` arms. Comment text only, plus a patch changeset for @objectstack/spec. Claude-Session: https://claude.ai/code/session_019oFY6Hi35J7atvgVFnmoaK Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019oFY6Hi35J7atvgVFnmoaK Co-authored-by: Claude <noreply@anthropic.com>
…cblock-mechanisms
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 143 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 202ce7da85f1d0ce50b8396acda2d73f82ff4f4c && git checkout 202ce7da85f1d0ce50b8396acda2d73f82ff4f4c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a8733f1c77681137d25abb1c02b5c889b71c01e c239951c267fcc596fba3d871bba712ef20b687c && git checkout -B drift-repro 7a8733f1c77681137d25abb1c02b5c889b71c01e && git merge --no-ff c239951c267fcc596fba3d871bba712ef20b687c
node scripts/docs-audit/affected-docs.mjs --json 7a8733f1c77681137d25abb1c02b5c889b71c01e
|
席位核对 (seat check) · PR #24290 ·
|
| item | reading |
|---|---|
| Shape | Draft, base main. The body's line 1 is Fixes #24187, line 2 is Clause-②: no, and the body has no other closing keyword. Assignee zhuangjianguo = the card's. 3 files, +36/-5: the changeset, conversions/registry.ts, ui/action.zod.ts. No content/docs/releases/. |
| Premises, re-measured by the seat | Item 1, objectui at the pin 5e15928b4631: resolveActionParam (:538); authoredOptions = resolveOptionLabels(param.options, ctx.locale) (:565); options: authoredOptions (:574); resolveOptionLabels (:246) returns options untouched when everyLabelResolved, and otherwise spreads only the map-label entries. Item 2, on origin/main: assertControlledByParentWrite reads opCtx.data[rel.fk] on operation === 'insert', and row[rel.fk] from the stored row otherwise. validateRecordInScope's update branch pushes required_cleared only under def.required && isEmptyForRequired(…). Both new sentences match. |
| Comment-only | The diff's two packages/spec/src hunks change only * docblock lines. No key, export, accepted value or message text moves. |
| Changeset, sentence by sentence | .changeset/24187-docblock-mechanisms.md: '@objectstack/spec': patch and Clause-②: no, both right for a prose-only fix to a file that ships (src/**/*.zod.ts is in files[]). Its four sentences each match the pin reading above: "no longer quotes … options: param.options"; "reads options: authoredOptions, where … resolveOptionLabels(param.options, ctx.locale)"; "the authored array itself … otherwise only the entries with an inline per-locale label re-spread"; "No key, schema, export, accepted value or message text changes". |
| Gates (check-runs on this head, read now) | 38 runs: 19 success, 6 skipped, 13 in progress, 0 failure. Not converged, so it does not land yet. |
| Accepted deviations | (1) One git fetch --depth=1 of the pinned commit 621a4876… for check-plugin-teardown-shape --self-test. (2) One main merge before the PR opened. (3) Two declared NOT MEASURED narrowings (check:dual-build-cjs-loads, check:dev-prereqs); CI's Lint & Repo Gates runs the first. |
| Out of scope | The PR's three Acceptance notes stay notes. The object.zod.ts forceCbpMasterDetailRequired "Today only the security gate…" sentence is auxiliary-face drift that does not mislead: the same docblock states the parse refusal. The packages/lint prose names only the insert leg, which is true. Each line: Acceptance notes. |
| Landing | After the PASS on this head and a fully green head: pr_ready then automerge_enable, through the relay. Not a governed surface; under 3,000 changed lines. |
Generated by Claude Code
Contract reviewServed-tier: Isolated read-only review, rendered 2026-10-18T09:12Z as a subagent of the seat session named in ① Derived judgmentsNet diff: 3 files, +36 / −5, identical on GitHub and locally. The 27 changed lines under
② Semver level
Tier: the schema face ( ③ Boundary flags
Check-runs on Implemented-by: VERDICT: PASS Generated by Claude Code |
|
Contract review adopted, plus one correction to the seat's ACCEPT ·
Generated by Claude Code |
Fixes #24187
Clause-②: no
What changes
Two docblocks in
packages/specnamed the wrong mechanism. This PR rewrites both and adds a prose-onlypatchchangeset. Nothing else changes.packages/spec/src/ui/action.zod.ts, thevisibleWhenbullet of theActionParamoptionsdocblock. It quoted objectui's inline param branch asoptions: param.options. At the console pin, that branch readsoptions: authoredOptions. The bullet now quotes that, names the pin, and says whatauthoredOptionsis. Its point is unchanged: an inline param's options reach the widget as authored.packages/spec/src/conversions/registry.ts, the docblock of thecbp-master-detail-required-defaultedconversion. It said that two writes the row could not legally take ("an insert without its master, an update nulling it") were "refused before as well, by the security gate". The insert half is true and stays. The update half was wrong in two ways. The security gate never reads the incoming FK on an update. So before the conversion, that update was not refused at all. The sentence now names both seams by symbol anchor. It says the update was accepted before, and that its refusal now comes from the record validator's ADR-0113 check, which onlyrequired: truearms..changeset/24187-docblock-mechanisms.md:'@objectstack/spec': patch, prose only, withClause-②: no.No key, schema, export, accepted value,
guidancestring or message text changes. All 27 changed lines inpackages/specare*comment lines (see the mechanical check under Verification).Item 1: what the pin reads
.objectui-sha=5e15928b4631a9dfd0d9bdd7282d1b746cdbaa68. I read every file below withgit show PIN:PATHfrom the local objectui object store. The objectui tree was not checked out or edited.packages/fields/src/resolveActionParams.tsat the pin::538declaresexport function resolveActionParam(, singular, so the docblock's name is right.resolveActionParams(plural,:873) is the list wrapper.:565isconst authoredOptions = resolveOptionLabels(param.options, ctx.locale);.:574(the inline branch, noparam.field) and:634(field-backed, but the field is not in the metadata) both readoptions: authoredOptions,.resolveOptionLabels(:246) returns the authored array itself wheneveryLabelResolved(options)holds, that is, when everylabelis already a string. This is an identity fast path. Its comment says objectuifbc23e094'stoBepin depends on it. Otherwise it maps the list. An entry with a stringlabelcomes back as it is. An entry whoselabelis an inline per-locale map is re-spread as{ ...option, label: resolveInlineI18nLabel(option.label, locale) ?? option.value }.So
visibleWhen, and every other key, survives on both paths. Only alabelwritten as a map is resolved. That is the bullet's point, so the bullet keeps the word VERBATIM, and its parenthesis now says exactly what happens tolabel. The map path can be reached from spec-valid metadata, because the spec's optionlabelisI18nLabelSchema(the optionstrictObjectinaction.zod.ts).I re-read the bullet's other links at the same pin:
packages/app-shell/src/views/ActionParamDialog.tsx:536re-spreads each option to localiselabel.packages/app-shell/src/utils/paramToField.ts:102passesoptions: param.optionson.SelectField,MultiSelectField,RadioFieldandCheckboxesFieldeach referenceuseCascadingOptions.resolveCascadingOptionsis exported frompackages/core/src/evaluator/optionRules.ts:217.resolveActionParams.ts,ActionParamDialog.tsxandparamToField.tsare byte-identical between this pin and the previous pinf8267df93aba, which the docblock's next paragraph names.git diff --statbetween the two pins over those three paths prints nothing. So that paragraph's:650-:651and:515readings still hold.Item 2: the two seams, read on this branch's base
93b4acb57packages/plugins/plugin-security/src/security-plugin.ts#assertControlledByParentWrite.operation === 'insert'it takes the master id from the incoming body,opCtx.data[rel.fk].readRowById(object, targetId, { isSystem: true })) and takesrow[rel.fk]. It never reads the incoming body's FK there.MasterReferenceMissingError(422). The one exception is an insert whererel.omissionRefusedByValidationholds (master_detail+required, and neitherreadonlynorsystem). There the gate stands down and leaves the refusal to the validator.packages/objectql/src/validation/record-validator.ts#validateRecordInScope, which the exportedvalidateRecordcalls.def.required && isEmptyForRequired(def, value) && def.type !== 'autonumber'. It pushes arequirederror with message keyrequired_cleared.required, the check does not fire."Before" in the docblock means the runtime before protocol 18. Then, a row with
requiredomitted derived its master through the second tier. I re-read both seams at76815b901^, the parent of the #23179 merge that retired that tier. All four readings are the same there:masterId = data … [rel.fk]on the insert leg;masterId = row[rel.fk]otherwise;def?.type === 'master_detail' && !!def?.required && !def?.readonly && !def?.system;def.required && isEmptyForRequired(…)update check.So before the conversion:
This matches contract review
6142098559on PR #23179 (items 5 and A). I read the card's summary, "refused by the record validator's ADR-0113 check, whichrequired: truearms", in the same sense: the refusal exists only once the conversion writesrequired: true. So the new sentence does not say the update "was refused before". It says:masterFK IN (accessible master ids)never matches;required: truearms that check.That is the same refusal the builder's force gives an authored object. So the paragraph's "Lossless" claim still holds, as the contract review judged. The filter is spelled the way
src/migrations/registry.tsand the lint rule already spell it.Changeset
@objectstack/spec'sfilesarray shipssrc/**/*.zod.ts, sosrc/ui/action.zod.tsgoes into the tarball as written. After the build, the new bullet is also in thedistbundles: 28distfiles carryoptions: authoredOptions, and 0 carryoptions: param.options.src/conversions/registry.tsis not infiles, and its new text reaches nodistfile: 0 files carryThe update nulling it was not. So the changeset describes item 1 only. It is apatchwithClause-②: no, following PR #24072.Verification
All readings below are on the final head
c239951c2(the merge ofmainat1e001c7df) unless marked otherwise. Build and test ran throughscripts/pm/os-verify-lock.sh, and every exit code was captured before any pipe.pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec check:generated: lock verdictcommand-exit 0. The output wasAll 14 generated artifacts are up to date. The comment-only edit regenerates nothing, and no artefact was touched.pnpm --filter @objectstack/spec exec vitest run --project local --maxWorkers=2:Test Files 694 passed (694),Tests 21027 passed | 1 todo (21028). On the pre-merge headfe8a88886it was 693 files and 21002 tests, all passing.pnpm --filter @objectstack/spec typecheck: exit 0, endingcheck:test-typecheck: OK.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 83 commands from the 3 changed paths, against merge base1e001c7df. 82 exit 0.--ranreconciliation:83 derived, 82 run, 1 NOT-MEASURED, 0 UNRUN.check:spec-docblock-symbol-anchors:4971 anchors across 2121 spec sources resolve — 12 symbol (12 declaration, 0 literal).check:nul-bytes: OK.check-adr-0087-registration:this PR adds no declared-breaking changeset.check-empty-changesetandcheck-changeset-no-majorare green.NOT MEASURED:
check:dual-build-cjs-loads(exit 3,PREREQUISITE NOT MET). It reads the built output of every workspace package, and 92 had nodist. I did not run a whole-repo build locally; that is a declared narrowing. CI'sLint & Repo Gatesruns this gate.On the pre-merge head, three more commands exited 3 at first. Each was re-run after its prerequisite was met:
check-plugin-teardown-shape.mjs --self-testneeded a pinned commit missing from the shallow clone (see Deviations);check:doc-formula-expressionsandcheck:lean-entry-closureneeded@objectstack/formula,@objectstack/lintand@objectstack/objectqlbuilt.On the final head, all three exit 0 in the first pass, after those three packages were rebuilt on the merged tree.
The symbol-anchor gate judges both new anchors (a one-off reverse check on
fe8a88886; the merge did not touchregistry.ts).scripts/ablation-replace.mjs. Each time the gate went red with exit 1:[unresolved-symbol] packages/spec/src/conversions/registry.ts:14475for#validateRecordInScope, and:14467for#assertControlledByParentWrite.5dacc0aa3dc2),git diff HEADis empty, andgit statusis clean.Comment-only check:
git diff -U0 origin/main...HEAD -- packages/spechas 27 changed lines. None starts with anything other than*. A control-byte scan of the three changed files finds nothing.Local-only family
node scripts/check-dev-prereqs.mjs: NOT MEASURED. It exits 1 with "The workspace is not built" (56 of 70 packages have nodist), so it never reached the spec-dist freshness check. Spec's owndistfreshness is covered bycheck:generated's declaration stamp match on this head.No ablation: the diff changes no tested behaviour.
Not run locally, left to CI:
test:repo(Test Core),pnpm lint, the type-check lanes, and the familiesdispatch-gateslists outside its runnable set.Acceptance notes
packages/spec/src/data/object.zod.ts, in theforceCbpMasterDetailRequireddocblock: the "Why" paragraph says "Today only the security gate (assertControlledByParentWrite) closes that shape". Since protocol 18, the parse also refuses that shape (refuseControlledByParentWithoutOneMaster). The same docblock says so two paragraphs later, so a reader is not misled. It is present-tense drift, and outside this card's file surface. Carrier: none.required: truemaster, which is the 2026-08-15 ruling behindomissionRefusedByValidation. The envelope is then400 VALIDATION_FAILEDwithfields[], where it was the gate's422 MISSING_REQUIRED_FIELD. The docblock's sentence is about "before", so it stays true, and I did not widen it to describe the insert's current envelope. This is an observation, not a defect. Carrier: none.assertControlledByParentWrite:packages/lint/src/data-model-rules.ts,packages/lint/src/rule-explanations.tsand that sameobject.zod.tsdocblock. Each speaks of the insert leg only. None repeats the wrong update-half claim.Deviations
git fetch --depth=1 origin 621a487607881c66b2899b7e3477115229a156b4. This adds one object to the shared store.check-plugin-teardown-shape.mjs --self-testrefused with exit 3 because its pinned positive-control commit was missing from the shallow clone. After the fetch it exits 0.mainwas merged once before this PR opened (c239951c2), as AGENTS.md section 10 asks. The two incoming commits (feat(metadata-protocol): publishPackageDrafts takes an opt-in backfill of new fields' literal defaults, written as the caller #24269, feat(spec): the bound record blocks accept an authored emptyState, drawn only when their dataSource binding answers no row #24270) touch none of the three files here. Spec was rebuilt and re-verified on the merged head.Generated by Claude Code