Skip to content

feat(core): pure row-to-definition mappers for the ADR-0131 upgrade ceremony (C7b build-ahead, PR 1 of 2) - #22959

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-22937-core-row-definition-mappers
Oct 12, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/issue-22937-core-row-definition-mappers

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Part of #22937
Clause-②: yes
Part of #15211

PR 1 of 2 of the C7b build-ahead card: E5's pure row → definition mappers, in core (seat answer 6110777865, E5 → A; its note on #15211, 6110790598). PR 2 (os migrate organization-ownership --apply) is a separate PR from main and is not in this diff. No closing keyword here: PR 2 completes #22937.

What changes

Two pure functions in @objectstack/core, beside convertPositionBindingRows in packages/core/src/security/, each in a new file with its own pins, exported from security/index.ts (and so from the package's . entry):

  • permissionSetDefinitionFromRow(row) (permission-set-row-definition.ts): one sys_permission_set row into the permission-set definition it carries. It reproduces permissionSetBodyFromRow (plugin-security/src/permission-set-projection.ts): the JSON-text facet columns become the camelCase body keys; label falls back to the name only when the column is null or absent (an empty-string label is kept); a facet column that is null, absent, an empty string, JSON null or unparseable text answers its default; adminScope travels only when the column is truthy and reads to a truthy value; only the definition columns are read, and the body is then filtered to the keys PermissionSetSchema declares (read from the schema's shape, never transcribed).
  • positionDefinitionFromRow(row) (position-row-definition.ts): one sys_position row into { name, label, description, delegatable }. It reproduces the mapping the row-only position backfill (position-environment-backfill.ts) applies, which is positionBodyFromRow in plugin-security/src/position-write-through.ts.
  • Row types PermissionSetRowForDefinition and PositionRowForDefinition (the columns read, plus an index signature: other columns are accepted and ignored). Both functions return a string-keyed record of unknown values (TypeScript's Record of string to unknown), the type the plugin's copies return, so 7a/7b can switch to them without a cast.

Helpers copied into core (new files only): parseMaybeJson and the spec-key filter (permissionSpecBodyKeys / pickSpecDeclaredKeys, folded into the function) from permission-set-projection.ts; positionBodyFromRow is itself the position mapping. Core already depends on @objectstack/spec and already imports values from @objectstack/spec/security, so no dependency is added. Nothing in packages/plugins/plugin-security/** is edited; #15204's 7a/7b switch to these functions and delete their copies.

No behaviour changes. Nothing calls the new functions yet, and neither runs at boot.

Equivalence with the plugin-security originals (one-time measurement)

Core's tests cannot import plugin-security without a new dependency (core is plugin-security's dependency), so the permanent pins assert the documented outputs on hand-written fixtures, and byte-for-behaviour equivalence was measured once with a scratch script (not committed). It imports the originals and the core functions from source with tsx, and compares them over a generated population of rows with util.isDeepStrictEqual (which tells a missing key from an undefined one), plus key order and JSON.stringify, plus thrown errors:

  • permission sets: 60,244 rows (exhaustive over name × label × description × admin_scope; every facet column over 20 values × 20 bases; 50,000 seeded random rows over every column at once, with state columns; null, undefined, {}; the test's own fixture). 0 differing.
  • positions: 20,737 rows (exhaustive over name × label × description × delegatable, each with and without the row-state columns; the test's own fixture). 0 differing.
  • Positive control: the same harness against mutated variants (an empty label falling back to the name; delegatable: 'yes' reading true) reports 7,174 and 1,152 differing rows, so the comparison can fail.

Run at head e975c410bc (and once before, at e5c8727332), same counts. The final head d477e5fbef is a wording-only commit: changeset text, one docblock line and one test comment, so no code line differs from e975c410bc.

Pins

  • permission-set-row-definition.test.ts (12 tests): full stored row → exact body in key order; every emitted key is declared on PermissionSetSchema and none is filtered away (a spec rename reddens this); a well-formed row's body passes PermissionSetSchema.safeParse; no state, provenance or identity column travels; already-parsed facets kept; the defaults for absent, null, empty, JSON-null and unparseable facets; fresh defaults per call; the label and description rules; the adminScope rule; a null/undefined row; purity (frozen row, same answer twice).
  • position-row-definition.test.ts (9 tests): the four keys in order; every key declared on PositionSchema and a well-formed body passes its safeParse; no row-state column travels; the label, description and delegatable rules; two rows of one name serialize equal exactly when their definitions agree (the backfill's agreement check reads that); the name is not judged; purity.

Ablation (committed state, restored and proven)

Both legs through scripts/ablation-replace.mjs (anchor hit 1 → 0, replacement 0 → 1, blob changed), inside a script with an absolute-path trap restore; the subject is imported by relative path, so no dist/ is involved.

  • permissionSetDefinitionFromRow: label: row?.label ?? row?.name, → label: row?.label,. 2 of 12 fail (the label rule; the defaults row, whose expected label is the name). Restored: blob f4e53ca6a2cb equals HEAD, git diff HEAD empty.
  • positionDefinitionFromRow: row.delegatable === '1' → row.delegatable === 'one'. 1 of 9 fails (the delegatable rule). Restored: blob b998eca94d1a equals HEAD, git diff HEAD empty.

Direction observed: red, as expected.

Tests and gates (head e975c410bc; the final head d477e5fbef changes wording only)

  • Gate union. node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at e975c410bc (merge base 51b43a8dc, 6 paths, +483/-0) derives 63 commands, the same 63 derived at dispatch on 1bd612bd68. All 63 were run at e975c410bc, each exit code captured before any pipe: 63 x exit 0. --ran reconciliation: 63 derived, 63 run, 0 NOT-MEASURED (derived from the recorded codes), 0 UNRUN. Readings of note: check:dts-closure 72 built packages swept, 171/171 declaration files present; check:dual-build-cjs-loads 107 require entry points across 66 packages load (core's two declared-unreachable declaration splits are the existing ledger entries); check:lean-entry-closure @objectstack/objectql/core admitted set held exactly (15 packages, both conditions); check:type-check-debt 26 raw errors, none above record; check:published-files, check:test-source-alias, check:cross-package-test-inputs, check:issue-citations, check:doc-authoring, check:nul-bytes green.
  • pnpm --filter @objectstack/core test at e975c410bc: 96 files, 2,402 tests passed. Both new files are in the local project's population (vitest list --project local names them).
  • pnpm --filter @objectstack/core typecheck at e975c410bc: exit 0. The test layer's ledger holds at 4 files / 4 errors, none in the new files, which tsconfig.test.json's program includes (--listFiles).
  • Build. pnpm turbo run build --filter=@objectstack/objectql (core and its closure included), 14 tasks. Core's CJS and ESM builds both export the two functions, and both .d.ts files declare them.
  • Lint, narrowed and proven. eslint over the 5 changed TypeScript files, --no-inline-config --format json: 5 files, 0 errors, 0 warnings. Population: all 5 sit inside eslint.config.mjs (an ignored file reports as a warning; none did). Invariance: the config never enables type-aware linting (no parserOptions.project, no typed rules), and its four local plugins are per-file AST rules whose only file read is the query-options baseline JSON, which this diff does not touch, so no untouched file's verdict can move. The full pnpm lint is CI's.
  • Consumers. Two functions and two types are added; no existing export changes shape, so no import site is affected.

Acceptance notes


Generated by Claude Code

…eremony

permissionSetDefinitionFromRow and positionDefinitionFromRow turn one
sys_permission_set / sys_position row into the definition it carries,
beside convertPositionBindingRows. Pure; no caller yet, never at boot.

Claude-Session: https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX
Co-authored-by: Claude <[email protected]>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 12, 2026
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/core, touching 13 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/authorization.mdx (via system_permissions (symbol, a field of interface PermissionSetRowForDefinition), tab_permissions (symbol, a field of interface PermissionSetRowForDefinition))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-1.mdx (via system_permissions (symbol, a field of interface PermissionSetRowForDefinition), tab_permissions (symbol, a field of interface PermissionSetRowForDefinition))

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
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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 — 27 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 6a40a09ed952f83b1f765d47cd76aee4a41f6672 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from e7057d7a693986a4651a5bd5cbbb5831130a914e — the merge of head 74b7c06c6b4422f23533aff3a65bf2841a48d13f into base 6a40a09ed952f83b1f765d47cd76aee4a41f6672, 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 e7057d7a693986a4651a5bd5cbbb5831130a914e && git checkout e7057d7a693986a4651a5bd5cbbb5831130a914e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6a40a09ed952f83b1f765d47cd76aee4a41f6672 74b7c06c6b4422f23533aff3a65bf2841a48d13f && git checkout -B drift-repro 6a40a09ed952f83b1f765d47cd76aee4a41f6672 && git merge --no-ff 74b7c06c6b4422f23533aff3a65bf2841a48d13f

node scripts/docs-audit/affected-docs.mjs --json 6a40a09ed952f83b1f765d47cd76aee4a41f6672

⚠️ 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 6a40a09ed952f83b1f765d47cd76aee4a41f6672 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…code applies them

The changeset's permission-set and position bullets now state each rule's
condition (label fallback, description, admin scope, facet defaults) and
separate reading only definition columns from the schema-key filter; the
ceremony is to apply the mappers (not yet built). Comment-only edits in the
position mapper's docblock and pin.

Claude-Session: https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX
Co-authored-by: Claude <[email protected]>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

CI red, not this PR's · domain:engine seat 2 (#20966) · os-steve · session_01ADzJtzYTLUfgrRZHxkagkX · 2026-10-12T05:53Z.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: e975c410bc26a1eae2f3f322cbc50e5b6c913ae6
Local-runs: none

PR #22959 (card #22937, PR 1 of 2) at the head above, read against origin/main 51b43a8dc45fef9d86229509ab6b2679ffa9d501, which is also the merge base: six files, +483/−0, additions only. Inputs: the card body and both of its comments (claim 6117549106, os-dev-report 6118339780); #15211's body with the seat answer 6110777865 (E5 → A) and its note 6110790598; the PR body, its one comment, its file list and the three-dot diff; the check-runs on the head; and, on origin/main, the plugin-security originals, core's security/index.ts and position-binding-conversion.ts, PermissionSetSchema, PositionSchema, and the two export * sites.

① Derived judgments

Surface added — right. security/index.ts gains two export blocks directly after the convertPositionBindingRows block: permissionSetDefinitionFromRow with type PermissionSetRowForDefinition, and positionDefinitionFromRow with type PositionRowForDefinition. Core's root src/index.ts runs export * from './security/index.js', so all four land on @objectstack/core's published . entry; ./logger is untouched. @objectstack/runtime (src/index.ts) and @objectstack/plugin-hono-server (src/adapter.ts) each run export * from '@objectstack/core' and so re-export the two functions and two types. Collision: none — git grep of the four names on origin/main returns nothing, on the head they occur only under packages/core/src/security/ and in the changeset, and neither re-exporting package declares a local of those names, so the star re-export is unambiguous.

Equivalence with the originals — right, judged by reading the pairs side by side.

  • permissionSetDefinitionFromRow against permissionSetBodyFromRow (permission-set-projection.ts). The candidate object is the same literal in the same order: name; label: row?.label ?? row?.name; a conditional description spread on row?.description != null (an empty string travels, null and absent do not); objects, fields, systemPermissions, rowLevelSecurity, tabPermissions through parseMaybeJson with the same defaults ({}, {}, [], [], {}); then a conditional adminScope spread. adminScope is computed first in both and travels only when the column is truthy AND parses to a truthy value, so '', 'null', 'false', '0', unparseable text and null all leave it absent. parseMaybeJson is copied verbatim: a non-string answers itself, or the fallback when nullish; '' is parsed as JSON null; a parsed null and a throw both answer the fallback; the default is a fresh object per call. The spec-key filter is the plugin's permissionSpecBodyKeys plus pickSpecDeclaredKeys folded into the function: the same Object.keys(PermissionSetSchema.shape ?? {}), memoised on first call rather than at import (the schema is a lazySchema proxy, the plugin's own reason), and the same Object.entries loop, so key order is insertion order. strictObject returns a closedObject built from the z.object(...).strict() def, so .shape is present, and the plugin's production code already depends on that read. The nine emitted keys (name, label, description, objects, fields, systemPermissions, rowLevelSecurity, tabPermissions, adminScope) are all declared on PermissionSetSchema, so the filter removes nothing today and the pin reddens on a rename. A null or undefined row answers { name: undefined, label: undefined, objects: {}, fields: {}, systemPermissions: [], rowLevelSecurity: [], tabPermissions: {} } in both. The differences are type-level only: the original takes and returns any; the copy takes PermissionSetRowForDefinition | null | undefined (the named optional columns plus a string index signature, all unknown) and returns a string-keyed record of unknown values. The PR body's "the type the plugin's copies return" is exact for positionBodyFromRow and loose for permissionSetBodyFromRow, which returns any; no caller-visible consequence for the 7a switch-over.
  • positionDefinitionFromRow against positionBodyFromRow (position-write-through.ts, which position-environment-backfill.ts imports and applies — the dev located the mapping correctly where the card's pointer named the backfill file). Identical statement for statement: name as stored; label when a non-empty string, else the name; description only when a non-empty string (so null, '', absent and a non-string are all absent); delegatable always present and true for exactly true, 1, '1', 'true'. Key order name, label, description, delegatable. Both dereference row.name directly, so a null or undefined row throws in both; the copy's parameter type refuses null at compile time, which is honest about that. The asymmetry with the permission-set function, which tolerates a null row, is inherited from the originals, not introduced.

Purity and no behaviour change — right. Neither function performs I/O, touches an engine, or mutates its input (the pins freeze the row); the only module state is the memoised key set, a constant. Nothing calls either function: the four names occur nowhere outside the two new modules, their tests, the barrel and the changeset. Neither runs at boot. No existing export changes shape.

No validation, no name judgement — right, as the docblocks say. Neither function calls safeParse, tests METADATA_ITEM_NAME_PATTERN, or reads PositionSchema.shape.name; metadataDoorAcceptsPositionName stays in plugin-security. The position pin asserts a capitalised name still answers a body; the permission-set pins assert an off-contract facet still answers a body and a well-formed row's body passes PermissionSetSchema.safeParse.

No dependency added; lean closure held — right. packages/core/package.json is not in the diff. The only new runtime import is PermissionSetSchema from @objectstack/spec/security, a module core already loads values from (api-key.ts, effective-object-permissions.ts, resolve-authz-context.ts, deployment-org-scoping-entitlement.ts); the proxy is not materialised until the first call, so core's load does not grow. position-row-definition.ts imports nothing. The position test imports PositionSchema from @objectstack/spec/identity — core's first import of that subpath, test-only, inside the already-declared workspace package. check:lean-entry-closure asserts a package SET, and no package enters it.

packages/plugins/plugin-security/** untouched — right. The file list is .changeset/22937-core-row-definition-mappers.md, packages/core/src/security/index.ts, the two new modules and their two tests. Nothing under content/docs/permissions/, content/docs/releases/ or any CHANGELOG.md.

Changeset, sentence by sentence against the code. Frontmatter "@objectstack/core": minor — right (②). Title — right. Clause-②: yes (widening) — right (②).

  • "What is new … the upgrade ceremony (os migrate) applies them to save Setup-authored rows through the metadata door. ⛔ Neither runs at boot." The exports, the home beside convertPositionBindingRows and "never at boot" are right. "applies them" is present tense while PR 2 is unbuilt; the final bullet, "nothing calls them yet", corrects it inside the same text, and origin/main already describes convertPositionBindingRows the same way in a published migration entry (18.position-permission-sets-declared.ts) with no caller either. Follows house precedent; "is to apply" would be tighter.
  • "The permission-set mapping … admin_scope → adminScope when set." The five facet renames are right; "when set" compresses "a truthy column that parses to a truthy value", which the docblock spells out. "label falls back to the name." Right for null and absent; the condition is unstated here while the position sentence states its own, and the two mappings differ on '' (kept for a permission set, replaced by the name for a position). "A facet that is absent, empty or unparseable answers its default." Right, with null and JSON null unlisted; the docblock lists them (commit dd1e7d44424c added them there and not here). "The body keeps only the keys PermissionSetSchema declares, so active, managed_by, customized and the timestamps never travel." The first clause is right; the "so" is not the mechanism — those columns never enter the candidate, and the filter removes nothing today. The stated outcome is true.
  • "The position mapping … a null or blank description is absent". "Blank" does not describe the rule the code applies. The test is typeof row.description === 'string' && row.description !== '': only the empty string is dropped, a whitespace-only description travels, and a non-string one is dropped. The word is copied from the plugin original's own docblock, where it carries the same imprecision; in a CHANGELOG line "empty" is the accurate word. The label and delegatable clauses and "active and is_default are row state and never travel" are right.
  • "No behaviour changes … the same output as … permissionSetBodyFromRow and positionBodyFromRow, and nothing calls them yet. Neither validates …" Right, by the side-by-side read above.

Net: no sentence misstates the exported surface or its semver; four sentences are looser than the code, and one of them ("blank") is strictly false for a whitespace-only description. These are wording corrections to one file; the seat may ask for them before arming, and a new head then needs a fresh record on it.

② Semver level

@objectstack/core minor — right. Two value exports and two type exports are added to a published entry and nothing is removed or narrowed: a purely additive widening, which the ruling check-changeset-no-major.mjs mechanises prices at least minor, on the one package whose src/** moved. @objectstack/runtime and @objectstack/plugin-hono-server widen transitively through export *; their src/** did not move, so the gate asks nothing of them, and their bump arrives through changesets' internal-dependency propagation at version time. Check Changeset concluded success on this head.

Clause-② — one value, written three ways: PR body line 2 yes, changeset yes (widening), claim yes. The arm is optional (clause2-line.mjs: an absent arm is "the overwhelming live shape"), carries no breaking signal, and check-adr-0087-registration.mjs pins both spellings as a non-breaking widening in its own self-tests. The dev's deviation note ("not a conflict") is right. The value is true: the published surface widens.

ADR-0087 marker — none owed. The gate judges only a changeset that declares a breaking change; a minor carrying Clause-②: yes or yes (widening) is, by the gate's own rule, not breaking; no spec key moves; no registry entry exists to cite. The dev's run of the gate exited 0; CI's copy rides in Lint & Repo Gates, pending at the final read.

③ Boundary flags

  • No closing keyword. Right for PR 1 of 2: Part of #22937 and Part of #15211; PR 2 completes the card. Part-of PR must not also close its card and The card this PR closes must claim this branch both concluded success; the newest Claim: on the card names this branch and this session.
  • Backfill decisions left in plugin-security (metadataDoorAcceptsPositionName; the rows-of-one-name-must-agree check). Inside the card's scope: E5 → A moves "the row-to-definition mapping", and the card forbids plugin-security edits. Both decisions decide what to WRITE, which is PR 2's per-name call; the door's own refusal covers the grammar meanwhile. The core mapping's key order is pinned (the serialisation-equality test), so an agreement check over its output stays meaningful. Carrier as the dev names it: PR 2 and 7b. Answered; no escalation.
  • S8 overlap in security/index.ts. On origin/main the isRowActive export is line 245; this PR inserts after line 280, the close of the position-binding-conversion.js block, with 35 untouched lines between — distinct hunks, so a text merge applies cleanly in either landing order, and neither new file imports row-active.ts. Answered.
  • Uncommitted equivalence harness. Acceptable: core cannot import plugin-security without inverting the dependency, plugin-security is closed to this card, and the measurement is evidence to weigh, not to repeat. Equivalence stands here on the side-by-side read in ①; the permanent proof is 7a/7b deleting the plugin copies. The committed pins assert the documented outputs and carry the one spec-rename tripwire that matters.
  • Commit trailers and footer. The two authored commits end with the model-free pair AGENTS.md prescribes; the PR body ends with the session-URL footer under a rule line, the body form. The harness reminder's model-named trailer yields to AGENTS.md by its own precedence sentence; right. The merge commit, which is the head, carries git's default message and no trailer pair — a squash landing replaces it, and the pre-push hook refuses a model identifier, not an absence. Noted; no action.
  • First gate derivation in the shared checkout. A read-only dispatch-gates.mjs derivation that wrote nothing (exit 2), then re-derived from the worktree; Prime Directive 11 governs edits, and none occurred. Reported as a deviation, which is the right handling; not verifiable from the inputs beyond the report, and harmless by construction. Answered.
  • Other deviations and the report's lists. Battery 1's timeouts and dist-less gates are superseded by the final 63/63 union at this head — accepted on the dev's own reconciliation; CI's required contexts are the verdicts here. One merge of origin/main before opening (docs and workflows only); the merge base equals origin/main's tip, so the three-dot diff is a clean delta. open_questions: empty. out_of_scope_findings: the three answered above, each noted rather than filed — right under Prime Directive 10, since none is a defect, a contract violation or an authoring trap.
  • Docs drift advisory (PR comment 6118319857). content/docs/permissions/authorization.mdx is listed through a column-name coincidence (system_permissions and tab_permissions as fields of the new interface). No behaviour changes; no page owes an edit.
  • Check-runs on the head, at the final read. Completed, every one success or skipped: Build Core; Dogfood Regression Gate and its three shards; Temporal Conformance (live PG + MySQL); Governed Surface Queue Guard; TypeScript Type Check and its four sub-jobs; Check Changeset; Check PR Size; Dogfood Verify CLI; the two claim checks and the Part-of check; Flag docs affected by code changes. Still in progress: Lint & Repo Gates, and Test Core (1/6) through Test Core (6/6) — both required contexts. Their conclusions are the gate verdicts; this record does not vouch for them, and the seat arms only once each reads success.
  • The PR is a draft, as the card asked; flipping it is the seat's act, after the two pending contexts conclude.

Implemented-by: claude/issue-22937-core-row-definition-mappers
Reviewed-by: session_01ADzJtzYTLUfgrRZHxkagkX

VERDICT: PASS

Adopted by domain:engine#2 (session_01ADzJtzYTLUfgrRZHxkagkX) at 2026-10-12T05:55Z. The record came from an isolated at-tier reviewer. Its transcript shows read-only tool use: REST reads, a fetch into a review ref and git show reads, and its only write was its own scratch record. Following the record's finding, the seat asked for the changeset corrections before arming: the false "blank" sentence and the four loose ones. The dev made them, with no code change, in commit d477e5fbef (patch round 1, report 6118594623). The seat corrected the matching PR-body line. This PASS covers e975c410bc. A delta record on d477e5fbef follows before the PR is armed. The Test Core (6/6) red on e975c410bc is the timing-drift step alone, filed as #22966.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: d477e5fbef0f89677a44b55eb93ec1e170497987
Local-runs: none

Delta record for PR #22959 (card #22937, PR 1 of 2). The branch gained exactly one commit since the PASS record on e975c410bc26a1eae2f3f322cbc50e5b6c913ae6: d477e5fbef, parent e975c410bc, no merge of main. Inputs: the PR body (the seat's edited permissionSetDefinitionFromRow bullet included), its three comments (the prior record with the seat's adoption note, the seat's CI comment naming #22966, the docs-drift advisory), its file list; the card's three comments, patch-round report 6118594623 among them; the delta git diff e975c410bc refs/review/pr-22959 and the three-dot diff against origin/main (merge base 51b43a8dc, unchanged; six files, +483/−0); the check-runs on the head; on the head, permission-set-row-definition.ts and position-row-definition.ts; on origin/main, permissionSetBodyFromRow and parseMaybeJson in permission-set-projection.ts and positionBodyFromRow in position-write-through.ts.

① Derived judgments

The delta changes no code — right. Three files, +5/−5. .changeset/22937-core-row-definition-mappers.md: three bullets rewritten. position-row-definition.ts: one line inside the module docblock (the block opens at line 3 and closes at line 42; the edit is line 26). position-row-definition.test.ts: one line comment at line 88. Blob identity across the two heads: permission-set-row-definition.ts 987eabe8d0 on both, security/index.ts 397a792ba1 on both; position-row-definition.ts moves from e768032fef to 2f23f68e0a by that one docblock line. No statement, type, export or test assertion moves. The two functions, the two row types, the barrel's two export blocks and the 21 pins are byte-for-byte what the prior record judged, so no accept set and no public surface it judged has changed; its equivalence, purity, no-dependency and no-collision findings carry over unaltered.

Each rewritten sentence, read against the code on the head and against the plugin originals on origin/main.

  • Changeset, "What is new": "The upgrade ceremony (os migrate) is to apply them, to save Setup-authored rows through the metadata door." Right. Nothing calls either function (the four names occur only in the two modules, their tests, the barrel and the changeset); PR 2 carries the --apply. This is the wording the prior record called tighter. The two module docblocks and the barrel comment keep their present tense ("converts it: it reads the row, calls this function"; "applied by the ceremony"); they were not rewritten, the prior record read them without objection, and origin/main describes convertPositionBindingRows the same way. Not owed.
  • Changeset, "The permission-set mapping", adminScope: "only when the column is truthy and reads to a truthy value." Right. const adminScope = row?.admin_scope ? parseMaybeJson(row.admin_scope, undefined) : undefined then ...(adminScope ? { adminScope } : {}): a falsy column (null, '', absent) and a truthy column whose parse is falsy ('0', 'false', 'null', '""', unparseable text) both leave the key absent; an already-parsed object is truthy and travels. Identical in permissionSetBodyFromRow.
  • Same bullet, label: "falls back to the name only when the column is null or absent; an empty-string label is kept (unlike the position rule below)." Right. label: row?.label ?? row?.name is nullish coalescing: '' is kept; the position rule replaces '' with the name. Identical in the original.
  • Same bullet, facet defaults: "A facet column that is null, absent, an empty string, JSON null or unparseable text answers its default; a facet a driver already parsed is kept as it is." Right, clause by clause against parseMaybeJson: a non-string answers itself or, when nullish, the fallback (so null and absent answer the default and a parsed object is kept); '' is parsed as the text null; a parsed null and a thrown parse both answer the fallback. The copy is verbatim from the original.
  • Same bullet, read-then-filter: "Only the definition columns are read, so active, managed_by, customized and the timestamps never enter the body. The body is then filtered to the keys PermissionSetSchema declares." Right, and the mechanism the prior record said the old "so" misstated is now the one the code applies: the candidate literal reads exactly name, label, description, object_permissions, field_permissions, system_permissions, row_level_security, tab_permissions and admin_scope; the Object.entries loop then keeps the keys in permissionSpecBodyKeys(), read from PermissionSetSchema.shape.
  • Changeset, "The position mapping", label: "falls back to the name unless it is a non-empty string (so '' is replaced by the name)." Right: typeof row.label === 'string' && row.label !== '' ? row.label : row.name.
  • Same bullet, description: "travels only when it is a non-empty string: null, '', an absent column and a non-string are absent, and a whitespace-only string travels as stored." Right: if (typeof row.description === 'string' && row.description !== '') body.description = row.description. The strictly false "blank" of the prior record is gone, and the whitespace-only case is now stated the way the code decides it. Identical in positionBodyFromRow.
  • Same bullet, delegatable and "active and is_default are row state and never travel": unchanged text, still right.
  • Docblock, position-row-definition.ts line 26: "a null or empty column is absent, never null." Right; it illustrates the full rule stated on the line above it ("only when it is a non-empty string"), and null (not a string) and '' are exactly the two cases named.
  • Test comment, position-row-definition.test.ts line 88: "An empty description is the same definition as a null one." Right: the assertion below it serializes description: '' against description: null, and both omit the key.
  • PR body, the edited permissionSetDefinitionFromRow bullet: the text is the replacement the dev gave in report 6118594623, verbatim. Each clause holds by the reads above: the label condition, the five-case facet default list, the adminScope condition, "only the definition columns are read, and the body is then filtered to the keys PermissionSetSchema declares (read from the schema's shape, never transcribed)". The dev's claims about the rest of the body also hold: the position bullet states no description rule, and no body line says the ceremony applies the mappers today.

No sentence the prior record called right became wrong. The prior record's six wording findings (tense, adminScope "when set", the unstated label condition, the facet list missing null and JSON null, the "so" mechanism, "blank") are each replaced by a sentence that is true to the code; the sentences it called right (the five facet renames, delegatable, the row-state clause, "No behaviour changes", the frontmatter and title) are untouched. The one original docblock imprecision the prior record traced ("blank" in positionBodyFromRow) stays in plugin-security, which this card forbids editing; the core copy no longer inherits it.

② Semver level

Unchanged. The changeset frontmatter is still "@objectstack/core": minor and the body still carries Clause-②: yes (widening); the delta touches neither line. security/index.ts is blob-identical across the two heads, so the published . entry widens by the same two value exports and two type exports and nothing else; @objectstack/runtime and @objectstack/plugin-hono-server re-export them through export * as before. No ADR-0087 marker is owed: no changeset declares a breaking change and no spec key moves; the dev's run of check-adr-0087-registration.mjs on this head reports one non-breaking changeset, and check-changeset-no-major.mjs and check-empty-changeset.mjs exit 0. Check Changeset concluded success on this head, on both of its runs. CI's copy of the changeset gates rides in Lint & Repo Gates, in progress at the final read.

③ Boundary flags

Every deviations and out_of_scope_findings entry in report 6118594623, answered:

  • PR-body write declined under the role file. The dev made no PR-body write and handed the seat the exact replacement line; the seat wrote it, and the body now carries that text verbatim (read above). The outcome is the one the round needed, under the stricter of the two instructions; the role file itself is not among this record's inputs and is not re-read here. Answered; no escalation.
  • Two corrections beyond the four asked. (a) The adminScope condition: this sentence was among the prior record's own loose findings, so it is inside the record's scope, not beyond it; the new wording is right. (b) The pin comment "blank" to "empty": the same word the prior record called false, corrected at its second occurrence; right by the assertion it labels. Both are wording only, inside the PR's own new files, within the card's file surface (tests beside the code; one changeset); packages/plugins/plugin-security/** stays untouched. Answered.
  • Worktree recreated from the branch; no merge of main. origin/main has moved past the merge base since the prior record (tip da9c2bf2f at the final read; merge base still 51b43a8dc), the PR reads mergeable, and the queue rebuilds the entry onto current main. The three-dot diff is unaffected. No action owed.
  • "No conflict between the four corrections and the code." Confirmed by the reads in ①.
  • out_of_scope_findings: the body line for the seat. Written; verified. One residual wording staleness the delta creates and nobody has edited: the body's "Equivalence" and "Tests and gates" sections still say "final head e975c410bc". The readings were taken there, and because no code line moved (two of the three core blobs are identical, the third differs by one docblock line) they carry over to this head's code; only the phrase "final head" now names the previous head. The seat may amend it or leave it; not a verdict matter.
  • Narrowed re-verification this round. The dev re-ran the three changeset gates, eslint on the two edited position files, check:nul-bytes, the two pin files, core's typecheck and core's build at d477e5fbef, not the 63-command union. Proportionate to a comment-only delta: the only gate inputs that moved are the changeset text and two comment lines, and CI's required contexts are the verdicts here.
  • Commit trailers. d477e5fbef ends with the model-free pair AGENTS.md prescribes (Claude-Session: and the bare Co-authored-by: Claude); author and committer are unchanged from the earlier commits. Right.
  • Docs drift advisory (PR comment 6118319857, re-run on the merge of this head): the same column-name coincidence on PermissionSetRowForDefinition's fields; no behaviour changes; no page owes an edit.
  • Check-runs on the head, at the final read. Completed success: Build Core; Check Changeset (both runs); Check Documentation Links; Check PR Size; Dogfood Verify CLI; Flag docs affected by code changes; Governed Surface Queue Guard; Type Check · source gates, Type Check · debt ledger, Type Check · consumer gates; the two claim checks and the Part-of check (both runs); filter; Auto Label. Skipped: Build Docs, Console Pin Gate, Packed-tarball smoke (opt-in), and the second runs of Auto Label and Check PR Size. Still in progress: Lint & Repo Gates; Type Check · workspace (so the TypeScript Type Check context has not concluded); Test Core (1/6) through Test Core (6/6); Dogfood Regression Gate (1/3) through (3/3); Temporal Conformance (live PG + MySQL). Five required contexts are therefore pending; their conclusions are the gate verdicts, this record does not vouch for them, and the seat arms only once each reads success. On Test Core (6/6) specifically: if it concludes failure on the "Check this shard's timing drift" step alone, that is the red ci: PR-time Test Core (6/6) reds on shard-timing-drift at 1.60–1.62x on three PRs that change low-graph packages (core, metadata-*) — runtime, service-analytics, rest and client run 1.4–2.4x their refreshed weights #22966 carries and not a verdict on this diff, and the seat's CI comment already says the PR then waits on ci: PR-time Test Core (6/6) reds on shard-timing-drift at 1.60–1.62x on three PRs that change low-graph packages (core, metadata-*) — runtime, service-analytics, rest and client run 1.4–2.4x their refreshed weights #22966 unarmed; a red on any other step, or on any other job, is a red.
  • The PR is still a draft; flipping it is the seat's act, after the pending contexts conclude.

Implemented-by: claude/issue-22937-core-row-definition-mappers
Reviewed-by: session_01ADzJtzYTLUfgrRZHxkagkX

VERDICT: PASS

Adopted by domain:engine#2 (session_01ADzJtzYTLUfgrRZHxkagkX) at 2026-10-12T06:16Z. The record came from an isolated at-tier reviewer. Its transcript shows read-only tool use: REST reads, a fetch into a review ref and object reads, and its only write was its own scratch record. CI on this head has now concluded: 48 runs, 40 success, 7 skipped, 1 failure. All six Test Core shards are success, the drift step included. So are Lint & Repo Gates, TypeScript Type Check, the three Dogfood Regression Gate shards and Temporal Conformance. The one red is the Test Core aggregator at the step "Check the per-package suite-duration ceilings", over one package: @objectstack/cloud-connection: 174.50s, over its 163.89s ceiling (job 114686711754). That is #22949's signature. The same package has crossed that ceiling on PRs that change only packages/spec or packages/lint, and on main's hourly run. This diff adds two functions nothing calls, so it cannot slow that suite. It is not this PR's red, and no fix exists yet because a ceiling rises only by a ruling. The seat cannot re-run a job, and the PR is not armed red.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 12, 2026 09:12
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 12, 2026
Merged via the queue into main with commit e1deaef Oct 12, 2026
35 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22937-core-row-definition-mappers branch October 12, 2026 09:38
github-merge-queue Bot pushed a commit that referenced this pull request Oct 13, 2026
…row-only position backfill (C3 stage 7b) (#23302)

Refs #22975
Clause-②: no

C3 (#15204) stage 7b: delete the position write-through and the row-only
position backfill. Stage 8 left both unreferenced, so this is dead-code
deletion. It is the parallel half of 7a; 8-post follows once both have
landed. #22975 stays open.

## What this changes

Stage 8 (#23222) stopped registering the `sys_position` row object, the
`sys_position` data-door write-through and the row-only position
backfill. Their modules stayed in the tree with no caller. This PR
deletes them.

- **Deleted:** `position-write-through.ts` and
`position-environment-backfill.ts`. Measured on `main` before this PR:
no file imports `position-environment-backfill`, and only
`catalog-reference-report.ts` imports `position-write-through` (for
`POSITION_OBJECT` and `metadataDoorAcceptsPositionName`). The backfill
is deleted rather than switched to core's `positionDefinitionFromRow`.
That mapper (#22959) stays in core for the upgrade ceremony; no boot
path calls it.
- **`PositionNameConflictError`** goes with the write-through, its only
thrower (ruling Q4 → A). It was never exported from the package's entry.
- **The catalog reference report** (`catalog-reference-report.ts`) drops
categories 4 and 5:
  - category 4: a position row with no definition;
  - category 5: a position name that several rows carry.
  
It also drops the `posture` input, which only those two categories read.
No composition registers `sys_position` after stage 8, so those
categories could never read a row. Legacy position rows are the upgrade
ceremony's to convert and report (ADR-0131 D10). The report still lists,
per organization:
  - every assignment whose position the catalog does not resolve;
  - every grant whose permission set the catalog does not resolve;
  - every grant that names no permission set.
  
  The report is not exported. Seat answer Q1 → A.
- **Generated, regenerated with the repo's own tools:**
  - row 11b leaves `content/docs/permissions/system-context.mdx`;
- the write-through's `UNTYPED_RECEIVERS` row leaves
`scripts/tenant-audit-census.mjs`;
- the tenant-audit census page and counts are regenerated with `node
scripts/tenant-audit-census.mjs --write`, and the prose figures the gate
names are updated (228 → 225 sites).

No export of `@objectstack/plugin-security`'s entry changes. The
changeset is a `patch`, so no ADR-0087 marker is needed.

## Acceptance notes

- **Overlap with 7a.** 7a edits the same census, system-context and
`security-plugin.ts` regions. Whichever PR lands second merges `main`,
re-runs `node scripts/tenant-audit-census.mjs --write` and updates the
census prose figures.
- **Not here:**
  - the comment sweep and the remaining catalog gates (8-post);
- the catalog reference report's sixth class (8-post, director ruling
#23229 → B).

## Verification

On head `8c7ee7f06a` (`main` `026391d5d8` merged in), unless noted:

- `plugin-security` unit suite: 188/188 files, 3,854 passed (45
skipped). Typecheck: exit 0. eslint on the changed files: exit 0.
- `check:tenant-audit-census` and `check:system-context-census`: exit 0.
- The derived gate sweep (`dispatch-gates --commands`): 114 derived and
114 run, all exit 0; `--ran` reconciles with 0 NOT-MEASURED and 0 UNRUN.
This was measured on `05dee10c89`, the head before the last merge of
`main`, whose five commits touch none of these files.
- The three symbol-anchor sweeps (adr, scripts, spec-docblock): exit 0.
- `check:dual-build-cjs-loads` on a full build (74/74 tasks): exit 0.
- `check:platform-checklist`: exit 1 with 8 reds. The same 8 reds appear
on `main` (they are pre-existing).
- Dogfood: 254 files passed (1 skipped), 2,106 tests passed.
- CLI integration tier (`--project integration`): 119/119 files, 1,036
tests passed.
- objectui pinned sibling: no export of `@objectstack/plugin-security`'s
entry changes, so nothing the console imports moves. CI's Console Pin
Gate covers it.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01287UbjvrigpWwNfGDmdwpS)_

Co-authored-by: Claude <[email protected]>
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

Development

Successfully merging this pull request may close these issues.

2 participants