Repository navigation
Commit 6ee8c61
fix(service-settings)!: a json setting persists a JSON object or array, and the console sends one (#31279)
<!-- claude-projects-attribution: {"github_login":"marchtian"} -->
_Requested by **田騏** · [project
thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSi59ATBeDy7jECXRUFiXAgGB)_
Fixes #31241
Clause-②: yes (narrowing) — the settings save door and the env door
start refusing a `json` value that is not a JSON object or array, which
they accept today; ADR-0007:123 already declares `json` persists an
object, so the narrowing enforces the declared contract and widens
nothing
Metadata-protocol: none
Skills-docs: updated here —
`content/docs/protocol/kernel/config-resolution.mdx` (the env coercion
paragraph said an unparseable value is used as the raw string; a `json`
key now rejects it). `skills/**` names the three lifecycle keys but
never spells how a `json` value is sent, so nothing it teaches changes.
> **BREAKING** accept-set narrowing at the settings doors
(`@objectstack/service-settings`). `PUT /api/settings/:namespace`
refuses a string, number or boolean under a `json` key with `400
SETTINGS_VALIDATION` and one `invalid_type` field error, and stores
nothing; an `OS_*` override for a `json` key whose text does not parse
to an object or array is not in force (one `error` line, the key
resolves from the next layer). **Migration:** send the parsed value, not
its text: `{ "quotas": { "sys_audit_log": 1000 } }`, not `{ "quotas":
"{\"sys_audit_log\": 1000}" }`. Values already stored as JSON text:
quota text is read as the object it holds, with one warning;
retention-override text is not applied until an administrator saves the
Data Lifecycle page once: the page opens with that text already marked
as an unsaved change and sends it as an object, so one Save with no edit
puts it in force (one error names the key and the fix until then).
ADR-0087 disposition: `not-required (no-migration-prescription)`, one
marker, in `.changeset/31241-json-setting-persists-object.md`.
## 维护者速读
- **改了什么:** 「数据生命周期」设置页的三个 JSON
框(保留期覆盖、行数配额、按类默认配额)以前保存的是一段文字,平台读不懂,配额和覆盖全部静默失效。现在控制台保存时先把文字解析成对象再发送,JSON
写错会在该行提示并拦下保存;服务端保存口和环境变量口也不再接受文字。以前存成文字的 JSON 值,打开设置页时会自动标成 1
处未保存并在该行提示,管理员直接点保存就改存为对象。
- **为什么改:**
管理员看到「已保存」,以为平台在执行配额和保留期,实际上什么都没执行,也没有任何日志,直到存储或合规出问题才发现(#31241,p2)。
- **风险与回滚:** 以前存成文字的配额升级后开始告警,每条打一次 warn;保留期覆盖的旧文字不会生效,只报一次
error,需要管理员打开「数据生命周期」页点一次保存才生效(页面打开就显示未保存,不用改任何内容),所以不会因为旧值意外删数据。打开含旧文字的设置页时离开页面会先确认,因为它被算作未保存的更改。直接调
API 把 JSON 当文字发送的脚本会收到 400。回滚方式是 revert。
- **先看:** `packages/objectql/src/lifecycle/lifecycle-service.ts` 的
`governanceMap`(旧文字的读取与告警);`packages/services/service-settings/src/settings-service.ts`
的 `jsonShapeMiss` 与它在 `validatePatch`、`effectiveEnvOverride`
的两处调用;`ui/apps/console/src/pages/settings/SettingsView.tsx` 的
`outgoingSettingsPatch` 与 `storedJsonTextDraft`(加载时把已存文字放进 draft)。
- **两分钟验证:** `cd packages/services/service-settings && pnpm exec vitest
run src/settings-json-shape-31241.test.ts`,预期 8 个全过;其中第一个用控制台的旧请求体,得到
400 和 `invalid_type`,什么都不存。
- **需要你做的:** 无,席位落地。
## What changes
- **`@objectstack/service-settings`: one shape judgment for both doors
(`jsonShapeMiss`).** A `json` key takes a plain JSON object or an array;
`null` is a clear. `validatePatch` refuses anything else (a string, a
blank string included, a number, a boolean) with one `FieldError` per
key: `code: 'invalid_type'`, `constraint: { type: 'json' }`, a message
that names what to send and never the value. It judges only the keys the
save touches, like every other family, so a string already stored under
another key does not block an unrelated save. `effectiveEnvOverride`
rejects an override that did not parse to an object or array through the
existing `reportRejectedEnvOverride` (one `error` line per variable and
value, carrying the value's shape, never its content).
- **`@objectstack/objectql`: values already stored as text
(`LifecycleService.governanceMap`).** `loadGovernance` reads
`retention_overrides`, `quotas` and `quota_defaults` through it, at the
global read and at each organization's read. For `quotas` and
`quota_defaults`, text that parses to a plain object is applied as that
object, with one `warn` per stored value naming the key (and the
organization, for a tenant-scope row) and the fix: save the page again.
They only raise alerts and never delete. For `retention_overrides`
(`LEGACY_TEXT_NOT_APPLIED_KEYS`), stored text is **not** applied: the
declared windows stay in force and one `error` per stored value names
the key (and the organization) and the fix, open Setup → Data Lifecycle,
check the value and save the page once. The reaper deletes on these
windows, so a window that was never in force must not start deleting
rows on upgrade without an administrator acting (seat review
6713609079). Anything else that is not a plain object applies nothing,
as before, and is now said once at `error`. **Choice:** a reader repair,
not a migration. These three keys are the only `json` specifiers in
`packages/` and `LifecycleService` is their only reader, so the repair
needs no rewrite of `sys_setting` / `sys_platform_setting` rows, and the
settings read API keeps returning what is stored. Saving the page once
stores the object, and the warning or error stops.
- **`@object-ui/console`: the field sends the value, never the text, and
offers a value stored as text for one save (`SettingsView`).** At Save,
each `json` key in the draft is parsed: valid JSON is sent as the object
or array, blank text as `null`. Text that does not parse blocks Save on
that row through the existing pre-save path (no request, the row marked,
focus on it) with the widget's own translated sentence
`fields.object.invalidJson`. Only a key in the draft is judged. At load,
`storedJsonTextDraft` puts every `json` key whose stored value is a
string, and is not locked, into the draft as it stands, so the page
opens with the save bar (one unsaved change per such key) and the row
carries a note, `console.settingsView.jsonStoredAsText` (new key, all 11
packs; zh-Hant regenerated from zh-Hans). One Save with no edit sends
the parsed object. Stored text that does not parse is seeded too and
stops Save on its row until it is fixed or cleared. A locked row (env
override) is neither seeded nor noted. Seeding also arms the existing
leave guard, since the page holds an unsaved change.
- **Docs:** `config-resolution.mdx` states the `json` exception to "the
raw string is used".
- **Changeset:** `.changeset/31241-json-setting-persists-object.md`,
`patch` for all three packages, with both declaration lines, the
BREAKING section, its migration and the ADR-0087 marker.
## Tests
-
`packages/services/service-settings/src/settings-json-shape-31241.test.ts`
(8), over real `ObjectQL`, the platform setting objects, the real route
and the shipped `lifecycleSettingsManifest`: the console's old body is
refused (400, `invalid_type` on both keys, nothing stored); a number, a
boolean and a blank string are refused; an object and an array are
stored as themselves; a clear falls back to the default; a stored string
does not block an unrelated save; env JSON text of an object is in
force, while unparseable text and a scalar are not, each with one
`error` line.
-
`packages/objectql/src/lifecycle/lifecycle-service.governance-map-text.test.ts`
(6): an object control alerts with no log; quotas stored as text alert
`quota-exceeded` and warn once over two sweeps; `quota_defaults` as text
applies; a global `retention_overrides` as text moves the reap cutoff to
90d; an organization's `retention_overrides` as text runs that
organization's pass on its 2y window and the global pass excludes it;
text that is not a JSON object applies nothing and errors once.
-
`ui/apps/console/src/pages/settings/__tests__/SettingsView.jsonSendsObject-31241.test.tsx`
(7): the patch carries the object, an array and a clear; text that does
not parse blocks Save; a locked stored string is not seeded and does not
block an unrelated save; a stored string shows the save bar and the note
on load; Save with no edit PUTs `{ quotas: { sys_audit_log: 1000 } }`
and the bar and note go; stored text that does not parse is seeded and
stops Save on its row.
## Acceptance notes
- **Measured `retention_overrides` this time.** Global and tenant scope
both, through a sweep (the lifecycle test above), not code-read.
- **A JSON value that parses but is not an object or array** (for
example `42`) is not blocked by the console. It is sent parsed and the
server refuses it inline with its own sentence, which is English. The
server already refuses it, so the page does not add a second refusal.
- **The claim's file surface** did not name
`content/docs/protocol/kernel/config-resolution.mdx` (the env paragraph
became false with this change) or the 11 locale packs
`ui/packages/i18n/src/locales/*.ts` (the new note key, round 2), so both
are changed here; the seat amends the claim.
- **The changeset carries the ADR-0087 marker.** This body states the
disposition in prose, because a body sent to GitHub carries no HTML
comment other than the attribution line.
## Verification
Ablations, each through `scripts/ablation-replace.mjs` (mutation landed,
test red, restored blob equal to HEAD and `git diff HEAD` empty), on
head `4246cad1` (the code under them is unchanged since, except the
A3/A4 file's error line now calls its logger channel directly):
| # | Mutation | Red |
|:--|:--|:--|
| A1 | `validatePatch`: `jsonShapeMiss(type, value)` → `null` | 2/8: the
old console body answered `expected 200 to be 400`; the scalar case too
|
| A2 | env door: `jsonShapeMiss(...)` → `null` | 2/8: `expected
'{"sys_audit_log": 1000' to deeply equal {}`, `expected 5 to deeply
equal {}` |
| A3 | `governanceMap`: `isPlainRecord(parsed)` → `false && …` | 4/6:
both quota cases `expected [] to deeply equal [quota-exceeded]`, both
retention cases |
| A4 | `loadGovernance`: the map keys bypass `governanceMap` | 5/6, the
error case included (`called 0 times`) |
| A5 | console: `outgoingSettingsPatch(...)` → `draft` | 3/4: `expected
{ quotas: '[1, 2]', … }` and the text cases |
| A6 | console: the parse refusal disabled | 1/4: `expected null to be
truthy` (no alert on the row) |
| A7 | console: load seeds `{}` instead of `storedJsonTextDraft(p)` |
3/7: the three stored-text cases (no save bar, no request, no row alert)
|
| A8 | console: `!resolved.locked` dropped from `isStoredJsonText` |
1/7: the locked case (save bar drawn on load, `expected … to be null`) |
| A9 | console: the note condition set to `false` | 1/7: the load case
(no note on the row) |
Local runs (shared container, under `os-verify-lock`; package suites
before the `origin/main` merge, the three new files and `src/lifecycle/`
re-run on the final head `e4d9a02a`, green):
`@objectstack/service-settings` test 72 files / 1379 tests and
typecheck, exit 0; `@objectstack/objectql` test 565 files / 11431 tests,
exit 0, typecheck and `src/lifecycle/` 6 files / 150 tests, exit 0;
console `pages/settings/` 39 files / 317 tests, exit 0. Console
`type-check` NOT MEASURED locally (it needs every `ui/` package built;
CI's Type Check UI carries it); `tsc --noEmit` on the console shows no
error in the two changed files beyond the unbuilt-module family. Round 2
on `29159a9b`: console `pages/settings/` 39 files / 320 tests,
`ui/packages/i18n` 1804 tests, the seven `check:i18n*` scripts, all exit
0.
**Gates, on head `29159a9b`:** `node scripts/pm/dispatch-gates.mjs
--commands --repo objectstack-ai/objectstack` derived 101 commands; each
ran with stdin from /dev/null, all `:: exit 0`. `--ran`: "101 derived,
101 run, 0 NOT-MEASURED". Round 1 on `e4d9a02a`: 100 derived, 100 run, 0
NOT-MEASURED, 0 UNRUN. The first round (before the merge) caught one
real red, `check:logger-receiver-detach` on the new error line in
`governanceMap` (`logger.error ?? logger.warn` read into a local), fixed
in `748e6b27`; its `check:dts-closure` and `check:dual-build-cjs-loads`
reds were a closure build running alongside and a missing `dist/` (exit
3), both green on the final head. `pnpm lint`: exit 0. `ui` eslint on
the two changed console files: 0 errors (4 warnings, all on lines this
diff does not touch).
**Dogfood (narrowing arm), full battery on the final head `e4d9a02a`:**
closure built with `pnpm --filter '@objectstack/dogfood^...' build`,
then `pnpm --filter @objectstack/dogfood test` under the lock, no shard
and no path: `Test Files 435 passed | 3 skipped (438)`, `Tests 3609
passed | 41 skipped (3650)`, `VERDICT command-exit 0`. Not re-run for
round 2: it changes only the console, the locale packs and the
changeset, none of which the battery loads.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- signed: domain:server#2 · marchtian ·
session_017ihL2fBJ6xsLWJsnNJGMMx -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 604a2b3 commit 6ee8c61
19 files changed
Lines changed: 1032 additions & 7 deletions
File tree
- .changeset
- content/docs/protocol/kernel
- packages
- objectql/src/lifecycle
- services/service-settings/src
- ui
- apps/console/src/pages/settings
- __tests__
- packages/i18n/src/locales
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
261 | 261 | | |
262 | 262 | | |
263 | 263 | | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
264 | 269 | | |
265 | 270 | | |
266 | 271 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
0 commit comments