Repository navigation
feat(service-settings): warn when a settings value misses the format its type declares (18.x half, 「甲」) - #25001
Conversation
…format ADR-0007 declares a format for the email, url, phone and color specifier types. Both doors now judge it through one shared helper (firstFormatMiss) and, under the standing rule 甲, report a miss instead of refusing it: a save is still accepted and stored, an OS_* override is still in force, and one warn line names namespace, key and type without ever printing the value. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
Both live readers (the mail transport's address check and the branding read) trim before use, so surrounding whitespace is not a format miss. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 9 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 7bf9c4a4fef534c827c6e1affc560661faa152b0 && git checkout 7bf9c4a4fef534c827c6e1affc560661faa152b0
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7532ea3c0893a08a518739aebfe7c0751211f53d b3b236b772ec04d79c6083e8f25ce67258d42b3c && git checkout -B drift-repro 7532ea3c0893a08a518739aebfe7c0751211f53d && git merge --no-ff b3b236b772ec04d79c6083e8f25ce67258d42b3c
node scripts/docs-audit/affected-docs.mjs --json 7532ea3c0893a08a518739aebfe7c0751211f53d
|
席位核对ACCEPT · #24882 (its 18.x half) ·
|
Part of #24882. This PR is the 18.x warning half only; refusing these values is the next major's half, which the
domain:servicesseat files when this lands.Clause-②: no (warning only: every save and every
OS_*override accepted before is still accepted, with the same value stored or served, the same status and the same errors)What changes
ADR-0007's specifier table declares a value format for four types:
email/url/phonetake "format validation", andcolortakes a "hex string". Nothing judged it.PUT /api/settings/brandingstored a Logo URL ofnot a urland answered 200, and anOS_*override was served unchecked.packages/services/service-settings/src/settings-service.tsnow judges that format at both doors through one shared helper,firstFormatMiss(type, value). The helper returns a verdict that carries the ADR-0114 code the refusal will use (invalid_email/invalid_url/invalid_phone/invalid_format). Both call sites report a miss and keep the value:validatePatch). The format branch runs after every refusal family, so the errors a refused patch carries are exactly what they were. A miss is collected, never pushed toerrors. When the patch as a whole is accepted, onewarnline is logged per key, naming the namespace, key, type, the expected format and the value's length.effectiveEnvOverride). The format is judged last, only on an override that every refusal family has already let through. A miss draws onewarnline, deduplicated per (variable, value) like the existing rejection report. The override stays in force: it is served withsource: 'env'and it still locks the key.auditEnvOverridesalso walks format-typed keys, so the line arrives at boot rather than on a first read.The grammars are the platform's existing ones for the same words, not new dialects:
emailplugin-email, the live reader ofmail.from_email. A value without a warning is one the transport can send from.no-reply@localhostwarns.urlscheme://…, a root-relative/path,data:, orblob:urlfield grammar. Not http(s)-only: the reader ofbranding.logo_url(RuntimeConfigPlugin.logoUrl) declares "absolute or relative URL", and storage hands back/api/v1/storage/files/…for an upload.phone+ ( ) - ., at least 5 charactersphonefield grammar. No shipped manifest authors aphonespecifier (company.phoneistext).color#rgb,#rgba,#rrggbb,#rrggbbaa#rrggbb, and the reader serves it as a browser theme colour.Empty values, blank values and non-string values are not judged, matching how
firstPatternMisstreats them. Surrounding whitespace is ignored because both live readers trim before use.「甲」 reading
The standing rule (#6015, comment 6292005854), verbatim: "No conversion can carry it: 18.x ships only the loud half (a warning, an explain line, a reported event). The refusal is filed for 19.0."
The seat's derivation for this card is comment 6302777692:
email/url/phone/colorsave narrows what the settings door accepts today.sys_setting(runtime state). AnOS_*override is a deployment environment value. No ADR-0087 conversion can rewrite either.This PR implements exactly that:
FieldError, no400and no new409anywhere.The pins below assert all of this through the real route. The refusal half reads
miss.codefrom the verdict, so turning the warning into aFieldErrorchanges the two call sites and nothing else.Where the warning reaches. Measured at
7adfaec2a:PUTanswer is{ values }only.setManyreturns a record ofResolvedSettingValuekeyed by setting, which has no advisory member.settings:changedevent (SettingsChangeEventSchema) and the two audit sinks carry no advisory member either.So the server log, through the service's existing
logger.warn(wired fromctx.logger;console.warnwithout one), is the only surface reachable without a new response member. Whether a log-only warning is loud enough for an admin saving in Setup is put to the seat as a decision in the report. The console half (constraint validation on Save) is objectui's, beside objectstack-ai/objectui#13239.Tests
New file
packages/services/service-settings/src/settings-format-warning.test.ts, with 31 cases. The save door is driven through the real route (registerSettingsRoutesover a realSettingsService, assettings-routes.test.tsdoes). Status starts at 0, so200is asserted rather than defaulted. The env door is driven throughget,getNamespaceand thePUTlock.branding.logo_url,company.website,branding.accent_color,company.primary_contact_emailandmail.from_email, plus a syntheticphonekey.source: 'env',locked: true, a save still answers 409). It draws exactly one warning, at boot, which re-reads do not repeat.emailkey at both doors: the value and its fragments never appear in the warning.maxLengthcarries only themax_lengthFieldError. A batch refused for one key stores nothing and warns about nothing.data:andhttp:logos, 3/4/6/8-digit hex, a padded email) draw no warning. Empty and blank values draw no warning.SettingsManifestSchema.Readings on the final head
b3b236b77, each throughscripts/pm/os-verify-lock.sh(VERDICT command-exit 0):pnpm --filter @objectstack/service-settings run typecheck: exit 0. The program includes the new test (tsc --listFiles, 57 test files).pnpm --filter @objectstack/service-settings exec vitest run --maxWorkers=2: 57 files, 960 tests passed.pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2overauth-settings-ordering.pin,auth-settings-seeded-boot.pinandauth-settings-refusal-isolation: 3 files, 13 tests passed. These importservice-settingsfrom source.Ablations
Each leg ran on
b3b236b77with the implementation committed. The mutation went throughnode scripts/ablation-replace.mjsin WRAP mode: the anchor hit 1 time and fell to 0, and the blob changed on disk. A trap-armed wrapper then ran the format pins and restored the file. The restore is proven with blob == HEAD (f3caa344f9a9) and an emptygit diff HEAD. The tests import the service from source, so nodist/is involved.return null)expected [] to have a length of 1 but got +0)errors.push+continue)expected 400 to be 200), and the precedence pin ([ 'support_phone', 'docs_url' ]vs[ 'docs_url' ])return null)expected null to be …,expected [] to have a length of 1)expected … not to contain …)Gates
Gate commands were derived from this worktree with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(no paths) onb3b236b77: 67 commands. Each ran locally, with its exit code captured before any pipe. The readings are in the os-dev report on the card. CI owns the families outside that list.Acceptance notes
plugin-emailand this file. The url and phone grammars are spelled in two places each, and the record validator's and rule validator's phone grammars already differ. No shared predicate exists inpackages/spec, and nothing pins the copies equal. Owner: none.logo_urlis stored as given and dropped by the branding reader. That is a type constraint, not a format, so it is left alone here exactly aspatternleaves it. It was not measured at a public door. Owner: none.company.phoneis authored astype: 'text'with an "E.164 recommended" description, so no phone format applies to the one phone-like shipped key. The manifests are outside this card. Owner: none.Generated by Claude Code