fix(service-settings): enforce the declared step grid on the settings write path and the env path (#6199) - #6501
Merged
Conversation
`step` 是 `SpecifierSchema` 五个值约束里的第五个,也是最后一个只声明不执行的。 #5932(PR #6201)补齐 `min`/`max`/`minLength`/`maxLength` 之后,`step` 仍是零读取点。 判定取自 schema 自己的写法:`step` 与 `min`/`max` 声明在同一段 "numeric bounds and step" 注释之下,即它是按「界」被作者写下的。另一种读法(纯 UI 步进)经核查 不成立:落地时 `step` 在本仓库与 objectui 中没有任何消费者。 - `step` 挂进 `DeclaredBounds` 与 `firstRangeViolation`,按构造同时到达写入路径 与 env 路径(`effectiveEnvOverride` 这一个判定点)。 - 越界发码表现有的 `invalid_value`(ADR-0114);`rest-server.ts` 早已把 Zod 的 `not_multiple_of` 映射到同一成员。packages/spec 未改动。 - 锚点取声明的 `min`,未声明时取 0(HTML step-base 约定)。 - 容差 `|value - nearest| <= max(|value|, |anchor|, |step|) * 1e-9`;精确取模在 二进制浮点下会拒掉 `0.7`(`0.7/0.1 === 6.999999999999999`)。 - 非正 / 非有限的 `step` 声明不记录网格,永不拒写。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01USNUyHEr7uaU6MoEWXitei
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-project-manager
marked this pull request as ready for review
August 8, 2026 04:06
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6199
stepis the fifth ofSpecifierSchema's value constraints, and the last one that was declared but never executed. After #5932 (PR #6201) landedmin/max/minLength/maxLength,stepstill had zero read points inpackages/services/service-settings/src/: superRefine does not check it, the write path does not read it, the env path does not read it.Premise check (against origin/main, not the issue's snapshot)
packages/spec/src/system/settings-manifest.zod.ts:237still declares onlystep: z.number().optional().stepunderpackages/services/service-settings/src/is a declaration site, not a read:manifests/ai.manifest.ts:189(temperature).objectui: the settings specifier'sstephas no consumer at all — no renderer writes it onto aninputattribute, no service reads it. Premise holds.Why this is a value constraint and not a pure UI hint
The issue offered two readings. The one that settles it is the schema's own:
stepis declared under the same "numeric bounds and step" doc comment asmin/max, so it was authored as a bound, and #5932's ruling — a declared bound binds — transfers with it.The competing reading (it is only the
input[type=number]arrow increment and never says other values are illegal) does not survive contact:stephas no consumer whatsoever. Under that reading the key would be expressing presentation for a renderer that does not exist. That is the ADR-0049 hole, not a UI affordance.Implementation (same shape as #5932 — the family's fifth member)
stepridesDeclaredBoundsandfirstRangeViolation, so it reaches both doors by construction — the write path (validatePatch) and the env path (effectiveEnvOverride, the ONE decision point). It cannot become the next key that is enforced on one door only, which is exactly what env 来源的 settings 值绕过 manifest 的 options 表校验 —— #5094 在写入 API 上堵住的洞,在 OS_* 覆盖这一侧原样敞开 #5204 exists to prevent.invalid_value(ADR-0114: "rejected for a reason no other member names"). NoFieldErrorCodemember names a grid, and the catalog is closed on purpose;packages/rest/src/rest-server.tsalready maps Zod'snot_multiple_ofonto that same member, which is this exact condition arriving from the other direction. ⛔packages/specis untouched.validatePatchemits at most oneFieldErrorper key, so the ordering decides what the author is told.temperature: 40.05reportsmax_value, notinvalid_value— "it misses the 0.1 grid" is true and useless, and would bury that the value is twenty times the declared maximum.Anchor convention
A value must lie on
min + k * step; when nominis declared the anchor falls back to0. That is the HTML step-base convention, and the only reading under which the declaration means what it says:min: 1, step: 2names the odd numbers. Anchoring at 0 regardless would invert the specifier entirely — it would accept exactly the values the author excluded. Nothing in the repo declares a different base; the only othermultipleOf-shaped rule anywhere (Zod's, mapped inrest-server.ts) is anchored at 0, which is this same convention with nomindeclared.constraintcarries bothstepand (when declared)min, so a client can rebuild the grid.Tolerance rule
Exact modulo is wrong. Under binary floating point
0.7 / 0.1 === 6.999999999999999and1.2 / 0.1 === 11.999999999999998, and both of those are values the console's own slider emits. What makes the trap dangerous is that it fires on some values of a grid and not others: on that same 0.1 grid2 / 0.1and0.2 / 0.1happen to land exactly, so a naive check looks correct until an author picks the wrong temperature.Why relative. An absolute slack scales wrongly with the operands:
1e-9would be a third of a step wide atstep: 1e-6, and tighter than one ULP atmax: 1048576(ai.max_tokens).Why
1e-9. It sits deliberately between the two errors it must separate. A double carries about2.2e-16of relative precision, so a handful of arithmetic steps accumulate about1e-15— six orders of magnitude below this bound. A genuine off-grid value misses by a fraction of a step:0.15on a0.1grid misses by0.05, which is3e-1relative — eight orders of magnitude above it. Nothing real lands in the gap.Compared in the value domain, not the multiplier domain.
|k - round(k)|would measure the error as a fraction of a step, so its meaning would change with the grid's fineness — while the floating-point error is a property of the numbers.The direction of the residual doubt is deliberate. This gate tightens a path that accepted everything yesterday, so where the arithmetic genuinely cannot tell (a magnitude at which the grid is finer than the double's own spacing) it accepts. Rejecting a legitimate write is the expensive mistake; letting one absurd-magnitude value through is not.
Defensive posture for a non-positive
stepstep: 0(anchor + k * 0is a single point), a negative spacing and a non-finite value all record no grid, the same disposition an option-bearing specifier with no table already gets: nothing to enforce, unchanged behaviour, never a refused write. This matches #5204's registration posture — registration REPORTS, it never refuses — and there is nothing to report here: a manifest declaring an impossible grid refuses no write and misconfigures no deployment; it merely fails to constrain, which is exactly where every specifier without astepalready sits. A window declared beside a bad step is unaffected (min: 0, max: 10, step: 0still enforces the window).Known consequence, accepted at ruling time
The repo's only
stepdeclaration isai.manifest.ts'stemperature(min: 0, max: 2, step: 0.1). Under enforcement0.15is refused. That is the declaration binding as written, not a defect of this gate. Whether that declaration itself is right (if0.15should stay legal, the manifest should declare a finerstepor none) is the manifest owner's question — this PR does not touch it.Tests
pnpm --filter @objectstack/service-settings test— 286 passed (15 files), 17 of them new:code+constraint+value+SETTINGS_VALIDATION); decimal multiples accepted (including0.7,1.2,0.1 + 0.2); off-grid neighbours refused; min-anchored grid (odd numbers); zero-anchor fallback; window before grid; TOUCH gate; numeric strings; uncomparable values left alone; non-positivestepdeclares no grid; encrypted specifier never echoes the value; the real ai manifest binding.locked/ cascade / log wording); an on-grid override still wins and locks; reported at registration and reported once; a rejected override pins nothing.envelope.conformance.test.ts, 1 case: drives the real route and asserts HTTP 400 +SETTINGS_VALIDATION+FieldErrorSchemaparses +invalid_value+constraint. Both halves of the ADR-0112 envelope (codeANDstatus) are covered —SettingsValidationErrorcarries no status of its own, so the 400 is only observable at the route layer.Reverse verification (direction predicted first: RED; result matches). Deleting the new
stepbranch fromfirstRangeViolationturns 12 of the 17 red and leaves 5 green. The 12 reds are exactly the refusal-class cases. The 5 that stay green are exactly the ones asserting the gate must not fire (decimal multiples accepted, window before grid, uncomparable values left alone, non-positive step declares no grid, on-grid env override still wins) — those were green before the gate existed and should be.Also run:
pnpm lint(clean);turbo run typecheck(120 tasks, all green);turbo run build;@objectstack/service-sms70 passed;@objectstack/plugin-email302 passed.Gates enumerated one by one from
.github/workflows/lint.yml(not from memory) — all PASS:check:slot-lookup,query-options-erasure,nul-bytes,doc-authoring,docs-audit-scope,role-word,quick-reference-counts,adr-anchors,org-identifier,authz-resolver,service-providers,route-envelope,error-code-casing,wildcard-fallthrough,meta-type-normalized,init-service-contract,durability-log-level,startup-registry-verdict,objectui-changeset,release-notes,release-body,node-version,workflow-status-functions,shard-attestation,published-files,engine-double-contract,resume-authority-declared,merge-driver,spec-parsed-alias,type-check-coverage,driver-conformance,stall-guard,skill-frame-sync,skill-compatibility,type-check-debt,i18n,i18n-coverage.File surface
packages/services/service-settings/src/settings-service.ts—DeclaredBoundsgainsstep,RangeViolationgains thestepkind and theinvalid_valuecode, plusisOnStepGridand the two call sites' prose.packages/services/service-settings/src/settings-service.test.ts— 16 new cases in two describes.packages/services/service-settings/src/envelope.conformance.test.ts— 1 new route-level envelope case..changeset/settings-declared-step-grid-enforced.md— patch,@objectstack/service-settings, following settings 写入路径只执行 required / options / pattern —— 声明的 min / max / minLength / maxLength 一条都不校验(auth.password_min_length 可被写成 1) #5932's changeset precedent for a write-path behaviour tightening.Related: #5932 / PR #6201 (the family's
min/max/minLength/maxLengthhalf), #5131 (optionson the write path), #5204 (env half aligned with the write half).🤖 Generated with Claude Code
https://claude.ai/code/session_01USNUyHEr7uaU6MoEWXitei
Generated by Claude Code