Skip to content

fix(types,rest): a /data write's NOT NULL refusal names the field by its label - #24759

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24715-not-null-field-label
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-24715-not-null-field-label

Conversation

@objectstack-fleet

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

Copy link
Copy Markdown
Contributor

Fixes #24715
Clause-②: yes (widening: three additive exports, notNullRefusalSite, relabelNotNullRefusal and the type NotNullRefusalSite, consumed by @objectstack/rest's /data doors; no accept set, status or code moves)
Basis, per package: @objectstack/types widens its published surface by exactly those three names, which reach its built entry through export * (packages/types/src/index.ts:98). Measured on the built dist/index.d.ts export list, origin/main 580fd4ed3 against this branch: 122 → 125 names, three added, none removed. Its changeset is therefore minor, Clause-②: yes (widening). @objectstack/rest changes message text only: the refusal's status, code and fields[] keys and codes do not move, and its new module is not on its entry. Its changeset stays patch, Clause-②: no.

What changes

A field declared storage: { notNull: true } without required is ADR-0113's engine-populated column. The record validator judges required only, so a write with no value for it reaches the driver and the database refuses it. mapDataError classifies that refusal from the driver's text, which names the column and nothing else.

  • @objectstack/types. The NOT NULL branch stays field-map-free (per triage, no field map is threaded into it). It now brands the body it builds. The caller that holds the field map uses notNullRefusalSite(answer) to recognise that answer, and no lookalike, and gets back the column and the table where the dialect names one (SQLite, Postgres). It calls relabelNotNullRefusal(answer, { message, declaresNotNull }) to swap the sentence and, for a declared storage.notNull field, the hint. The branch's own hint no longer adds "or declare the field required". That hint is what every door that does not relabel still serves, so it must hold for both ways in: a declared NOT NULL column and drift.
  • @objectstack/rest, POST /data/:object and PATCH /data/:object/:id. The catch relabels a NOT NULL refusal (not-null-refusal-label.ts). The field is named the way the record validator names its own required refusal. The validator's builder is not on @objectstack/objectql's public entry, and rest does not depend on the engine at run time, so no new export in packages/objectql was needed or made. Every piece that builder composes is a @objectstack/spec/system export, and this door reuses them:
    • the catalog's required sentence through renderValidationMessage, with the validation.field.required override honoured;
    • the label through t(objectFieldLabelKey(...)), then the declared label, then the API name;
    • the locale: the request's ExecutionContext.locale (the one the engine's validator was given), negotiated by resolveBundleLocale against the i18n service's locales, as the engine does.
  • One extra read, only on this path. The door reads the object by name, through the read the exposure gate already makes on every /data request. That read is now one private method, objectItemReader, shared by both callers. The exposure gate's behaviour is unchanged.
  • Unchanged. status, code, fields[0].field, fields[0].code, object and the key set, on the body and on the field.
  • Left alone, on purpose. A column the object does not declare. A refusal the driver attributes to another table (a hook's write to a second object). Any failed read. Each keeps the classification's answer.

Before and after, at both /data doors

Measured on a real store: SqlDriver on better-sqlite3 :memory:, the real ObjectQL and ObjectStackProtocolImplementation. The field is unit_price (label "Sales Price"), declared storage: { notNull: true } with no required.

RestServer door, before (origin/main 7e7e605de). PATCH /api/v1/data/OBJECT/ID with { "unit_price": null }, and POST with no value, both answered:

{"status":400,"body":{"error":"unit_price is required","code":"VALIDATION_FAILED","fields":[{"field":"unit_price","code":"required","message":"unit_price is required"}],"hint":"The database column for 'unit_price' requires a value: provide it, or declare the field `required` in the object metadata. If the object declares neither `required` nor `storage: { notNull: true }` for 'unit_price', the physical schema has drifted from metadata instead: run 'os migrate' to reconcile (or reset the dev database).","object":"line_item_probe"}}

RestServer door, after (this branch). PATCH and POST both answer:

{"status":400,"body":{"error":"Sales Price is required","code":"VALIDATION_FAILED","fields":[{"field":"unit_price","code":"required","message":"Sales Price is required"}],"hint":"The field 'unit_price' is declared `storage: { notNull: true }`, so its database column requires a value: send one, or omit the field when a default value or a hook populates it (an update that omits it keeps the stored value).","object":"line_item_24715"}}
  • A zh-CN caller with a zh-CN bundle reads "error":"销售价格不能为空". That is the validator's own zh-CN required template; a bare zh negotiates to the same text.
  • Drift control (drift_col, declared with neither required nor storage.notNull over a NOT NULL column): "error":"Drift Column is required", and the classification's hint, which leads with "requires a value: provide one" and keeps the conditional os migrate sentence.
  • Undeclared-column control (gone_col): "error":"gone_col is required", with the same hint.

Dispatcher /data door (packages/runtime), before = after. Its premise does not hold: this door never answered the VALIDATION_FAILED / required envelope.

  • domains/data.ts does not catch, so HttpDispatcher.dispatch('PATCH', '/data/OBJECT/ID', { unit_price: null }) lets the raw SqliteError out of dispatch(). That error carries code: SQLITE_CONSTRAINT_NOTNULL, no status, and the message update [statement and bound values redacted] - NOT NULL constraint failed: OBJECT.unit_price.
  • declaredEnvelopeForThrow declines it. resolveThrownHttpError answers 500 INTERNAL_ERROR, and looksLikeInternalErrorLeak is true for the message.
  • So @objectstack/hono's catch-all answers 500 {"success":false,"error":{"code":"INTERNAL_ERROR","message":"Internal server error"}}, and dispatcher-plugin's exit answers the same status and code.
  • Naming a field there means changing that door's status and code, which this card holds off. packages/runtime is untouched. The report carries this as an open question and a finding.

Pins

  • packages/rest/src/data-not-null-field-label-24715.test.ts: both doors on the real driver.
    • PATCH and POST assert error, fields[0].message and the hint's kind: it names the field and storage: { notNull: true }, and names neither required nor drift. They also assert the unchanged envelope and key sets, and that the row did not move.
    • The zh-CN case is checked against the validator's own message for a required sibling field, on the same request shape, and the bare zh case is checked too.
    • The drift control and the undeclared-column control.
    • The sentence is asserted as renderValidationMessage's required sentence for the label, never as a literal.
  • packages/rest/src/not-null-refusal-label.test.ts: the helper's declines, each with the source it must not ask: another table, another answer, a failed read, an undeclared column.
  • packages/types/src/data-error-classification.not-null-label.test.ts:
    • the site per dialect (SQLite, Postgres with and without the relation, MySQL);
    • the validator's own required refusal not recognised, and a copied body not recognised;
    • the hint holding for both cases;
    • the relabel moving text only.

Ablations

One per change, each through node scripts/ablation-replace.mjs: the anchor hit once, landed on disk, and was restored. Each restore was proven by blob (git hash-object equal to the HEAD blob, and git diff HEAD empty). Each leg went red, the expected direction. The subjects resolve from source here (relative imports), so no dist build sits between mutation and run.

  1. Hint. The classification's first sentence was put back to "provide it, or declare the field required". Result: data-error-classification.not-null-label.test.ts went 1 failed / 9 passed, and the hint case is the one that failed. Blob 61ebd27afc24 → 2f324ae3d717 → 61ebd27afc24.
  2. Brand. The line that records the branch's body was deleted. Result: 6 failed / 4 passed. The four site cases and both relabel cases went red; the validator lookalike, copy and pass-through cases stayed green. Blob 61ebd27afc24 → b3d98ac5a64e → 61ebd27afc24.
  3. Door relabel. labelNotNullRefusal was made to decline every answer. Result: 6 failed / 6 passed. PATCH, POST, zh-CN, zh, the drift label and the helper's CONTROL went red. The fixture case, the undeclared-column control and the four declines stayed green. Blob f2a76857df43 → e2383610aa51 → f2a76857df43.

Census of the old-text pins

  • The hint. git grep over every package's tests for the hint's wording found one pin this change made false: packages/rest/src/data-not-null-hint-20963.test.ts, which asserted that the hint's first sentence contains `required`. It now asserts the opposite. Its harness registers no stored-metadata family, so the door's object read fails there and the classification's answer goes out unchanged. That makes it the fail-safe control for the read, and its docblock now says so.
  • The X is required text. The other tests that carry it either call mapDataError directly, where the sentence is unchanged (rest.test.ts, rest-unclassified-fault-status.test.ts, rest-unique-violation-dialects.test.ts), or assert the record validator's own messages, or an import row's report. None drives a labelled declared field through the relabelled doors. None changed.

Tests and gates

The test suites, typecheck and the first lint ran at ee57acee6. The one later commit, 055d54575, edits .changeset/24715-not-null-field-label-types.md alone (minor, Clause-②: yes (widening)). The derived gates and lint were re-run at 055d54575.

  • Full suites, every project. Each part's exit was recorded before any pipe.
    • @objectstack/types: local 29 files, 774 passed, exit 0; repo 1 file, 11 passed, exit 0.
    • @objectstack/rest: local 304 files, 6124 passed / 326 skipped, exit 0; repo 5 files, 280 passed / 18 skipped, exit 0.
  • Typecheck. pnpm --filter @objectstack/types typecheck and pnpm --filter @objectstack/rest typecheck both exit 0, including rest's check:test-typecheck (0 debt). tsc --listFiles confirms that each package's program compiles the new test files.
  • Derived gates. node scripts/pm/dispatch-gates.mjs --commands derived 67 commands, and all 67 were run with their exit codes recorded.
    • At ee57acee6: pnpm check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET, no dist/). It exited 0 after a full turbo run build. dispatch-gates --ran then printed: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.
    • Re-run at 055d54575 (the same 67 derived): all 67 exit 0, the changeset readers included (check-changeset-no-major.mjs, check-adr-0087-registration.mjs, check-empty-changeset.mjs, check:changeset-gate-self-tests, check:pm-changeset-deadline-census, check:published-files). dispatch-gates --ran printed: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN.
    • Check Changeset's level axis, run locally as check-changeset-no-major.mjs --event against this PR's payload: it passes with the body declaring no, and it passes with this body declaring yes (widening) ("the declared widening is accounted for").
  • Lint. pnpm lint (eslint . --no-inline-config, the whole repository, the lane's full union) exits 0 with no findings, at ee57acee6 and again at 055d54575.

Acceptance notes

  • Other doors, same family, not relabelled here. The /data/:object/batch and /batch rows, import rows, the public form submit, and data actions still name the column when the database refuses NOT NULL. Those that carry the classification's hint now carry it without the required prescription (an import row carries no hint). None was measured with a labelled field, so none is filed here.
  • The dispatcher /data door. See above. A driver refusal leaves dispatch() raw and is answered 500.
  • The update sentence. The relabel uses the catalog's required sentence for both create and update, per the card. The validator words its own update refusal of a required field with required_cleared ("is required and cannot be cleared"); that sentence was not adopted.

Generated by Claude Code

…its label (WIP)

The REST /data create and update doors relabel the NOT NULL refusal the
classification builds from the driver's text: the field's label in the
caller's locale, and for a field declared storage.notNull a hint that
prescribes neither required nor os migrate. The classification's own hint
stops prescribing required.

Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T
Co-authored-by: Claude <noreply@anthropic.com>
The door pins run on a real driver: PATCH and create name the field by its
label (and in zh-CN by its translated label, in the validator's own catalog
sentence), a declared storage.notNull column gets a hint without required
or os migrate, and the drift and undeclared-column controls keep today's
answer. The classification's half and the label helper's declines are unit
pinned. The 20963 hint pin drops the required prescription it asserted.

Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/rest, @objectstack/types, touching 23 documentable anchor(s).

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

  • content/docs/getting-started/common-patterns.mdx (via unit_price (literal, a string literal in a comment on a changed line))
  • content/docs/protocol/kernel/i18n-standard.mdx (via fieldLabel (symbol, a top-level function))
  • content/docs/protocol/objectui/layout-dsl.mdx (via unit_price (literal, a string literal in a comment on a changed line))
  • content/docs/ui/forms.mdx (via /forms/:slug (route, bridged from symbol resolveI18nService — its route source's handler names it))
  • content/docs/ui/public-data-collection.mdx (via /forms/:slug (route, bridged from symbol resolveI18nService — its route source's handler names it))

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

  • content/docs/releases/implementation-status.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v12.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v16.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v17/17-3.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via RestServer (symbol, a top-level class))
  • content/docs/releases/v17/17-6.mdx (via /forms/:slug (route, bridged from symbol resolveI18nService — its route source's handler names it))
  • content/docs/releases/v17/17-7.mdx (via /forms/:slug (route, bridged from symbol resolveI18nService — its route source's handler names it))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: enforceApiAccess (9 routes)
  • 1 anchor(s) matched too much of the corpus to be a work list: VALIDATION_FAILED (literal, 31 pages)
  • 4 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 — 20 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 580fd4ed3c98a1752dfb0041b45cc713644fef7e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 933ab24e9ceadb6dadedb02aeb7f902fd3e0050e — the merge of head 055d54575682dc859c8b245ad882b24febb616cf into base 580fd4ed3c98a1752dfb0041b45cc713644fef7e, 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 933ab24e9ceadb6dadedb02aeb7f902fd3e0050e && git checkout 933ab24e9ceadb6dadedb02aeb7f902fd3e0050e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 580fd4ed3c98a1752dfb0041b45cc713644fef7e 055d54575682dc859c8b245ad882b24febb616cf && git checkout -B drift-repro 580fd4ed3c98a1752dfb0041b45cc713644fef7e && git merge --no-ff 055d54575682dc859c8b245ad882b24febb616cf

node scripts/docs-audit/affected-docs.mjs --json 580fd4ed3c98a1752dfb0041b45cc713644fef7e

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

The NOT NULL relabel adds three exports to @objectstack/types' entry
(notNullRefusalSite, relabelNotNullRefusal, type NotNullRefusalSite),
consumed by @objectstack/rest's /data doors: an additive public-surface
widening. No accept set, status or code moves.

Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 — ACCEPT (the lane seat's review of record for a Clause-②: yes declaration)

domain:cli seat 1 (seat post #6024) · zhuangjianguo · session session_01SYEJqDL3NXxrJRBVqaMu9T · 2026-10-20T05:29Z · on head 055d54575 · #24715 (p2; claim 6282809559, amended Clause-② on the card; reports 6283722892 and 6284097182). This is the seat's own review at the default tier, read against the diff on GitHub, not the reports. For a Clause-②: yes declaration with no published schema and no governed rules text, it is the review owed.

  1. What moved: 10 files, +874 / −50 at round 0. Round 1 changes one changeset line.
    • @objectstack/types: the NOT NULL branch of classifyDataError stays field-map-free, as triage required.
      • It parses the column, and the table where the dialect names one (SQLite, Postgres, MySQL), into NotNullRefusalSite.
      • It brands the body it builds through a module-private WeakMap, so no key reaches the wire.
      • Its hint now leads with "requires a value: provide one", names drift only conditionally, and no longer prescribes required.
      • notNullRefusalSite(answer) recognises only that branch's own body, not a validator lookalike or a copied body. relabelNotNullRefusal replaces error and fields[0].message, and the hint for a declared storage.notNull field.
    • @objectstack/rest: POST /data/:object and PATCH /data/:object/:id hand mapDataError's answer to a private labelNotNullRefusal.
  2. The envelope does not move: status, code, fields[0].field (the column key), fields[0].code (required), object and the key sets are pinned on the real driver. The row is unmoved. Drift keeps the drift hint.
  3. Pins and ablations:
    • data-not-null-field-label-24715.test.ts (7, real SqlDriver + better-sqlite3): PATCH and POST answer the label. A zh-CN caller reads 销售价格不能为空, checked against the validator's own zh-CN required message for a sibling field.
    • not-null-refusal-label.test.ts (5) and the types site and relabel pin (10).
    • Three ablations each went red as predicted, and each restore was proven by blob.
    • One old pin made false, data-not-null-hint-20963.test.ts, is inverted. It now serves as the failed-read control.
  4. Clause-②: yes (the seat's amended declaration, its review):
    • @objectstack/types adds exactly notNullRefusalSite, relabelNotNullRefusal and the type NotNullRefusalSite, through export *. The built dist/index.d.ts goes from 122 to 125 names, none removed, measured by the dev against origin/main 580fd4ed3.
    • It is additive, first-party plumbing consumed by rest. No accept set, status or code moves.
    • The minor changeset follows AGENTS.md's floor for Clause-②: yes. @objectstack/rest stays patch / Clause-②: no: its new module is not on its entry.
    • The seat applied the PR body's 3 body_edits (read back identical). Check Changeset's local twin passes on the yes (widening) body.
  5. Scope kept: packages/runtime and packages/objectql are untouched. The dev measured that the dispatcher /data twin answers 500 INTERNAL_ERROR for the same refusal, so a label there would move that door's status, which was held off. Filed as A database NOT NULL refusal answers differently per door: the dispatcher /data twin answers 500 INTERNAL_ERROR, and REST batch/import/form/action rows name the column key (RestServer POST/PATCH /data answer 400 with the label after #24715) #24768, with the engine-side translation recorded as the proposed direction. That card also carries the other REST doors that keep the column key (batch and import rows, form submit, data actions).
  6. Gates:
    • dispatch-gates: 67 derived and 67 run, 0 NOT-MEASURED, at both heads. pnpm lint exited 0.
    • types: 774 passed (local), 11 (repo). rest: 6,124 passed (local), 280 (repo). Typecheck exited 0 for both.
  7. Findings: A database NOT NULL refusal answers differently per door: the dispatcher /data twin answers 500 INTERNAL_ERROR, and REST batch/import/form/action rows name the column key (RestServer POST/PATCH /data answer 400 with the label after #24715) #24768, filed by this seat.
  8. Landing: ready plus auto-merge through the queue once every check of 055d54575 is green or a rostered skip. The body edit may re-run Check Changeset. Fixes #24715 closes the card on the merge.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants