Skip to content

Commit 158cc2a

Browse files
fix(service-settings): a select, radio or multiselect setting refuses every value when its option source answers an empty list (#31569)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X1nhftM2hZHHvqk2GLFyVTN)_ domain:server#3 · os-project-manager · session_01FEkQ5nBSzc6ktJyjdFcDb3 Fixes #31550 Clause-②: no (narrowing) (a select / radio / multiselect value is refused when its registered option source answered an empty list; it was accepted at both doors before) Metadata-protocol: none ## 维护者速读 - 改了什么:select / radio / multiselect 设置的值,按该键注册的选项来源判定;来源答了「空列表」时,以前保存门与 `OS_*` 环境变量门都跳过校验、任何值都放行;现在两道门都拒绝(保存 `invalid_option`「… offers no value to choose from right now.」,环境变量记一条 error 且不生效),与 #31422 对 map 键的规则一致。顺带:环境变量拒绝日志的去重改按稳定序列化,同一变量下两份不同的坏 map 各报一次(以前所有 map 都被当成 `[object Object]`,第二份被吞)。 - 为什么改:#31539 契约复审的 Finding 1 与一条观察。 - 风险与回滚:收窄,只影响「已注册来源答了空列表」的 select 族键。启动窗口:`localization.locale` 的来源在 i18n 列表为空时答的是「无答案」(`undefined`),不是空列表,所以走声明选项;环境变量门在启动期间(`holdOptionSourceVerdicts` 到 `kernel:bootstrapped` 封存)对来源判定一律先放行并在封存时重判(有对照 pin)。没有来源、或来源答不出时,仍按声明选项判定。service-settings 全量测试全绿。回滚即还原本 PR。 - 先看:`packages/services/service-settings/src/settings-service.ts` 的 `validatePatch` select 分支、`effectiveEnvOverride` select 分支、`reportRejectedEnvOverride` 的 `dedupeAt`;新 pin `settings-select-empty-source-31550.test.ts`。 - 两分钟验证:`pnpm --filter @objectstack/service-settings exec vitest run src/settings-select-empty-source-31550.test.ts`,预期 9 条全绿;其中 5 条在 main 上是红的。 - 需要你做的:无,席位落地(收窄,落地前席位做契约复审)。 ## What changes - **`validatePatch`, the option-bearing branch** (`service-settings`): it reads `sourced` from `optionTableOf` (#31539) and judges the value whenever a source answered (`sourced || allowed.length > 0`). The `allowed.length > 0` leniency stays for the declared table alone (a hand-built manifest with no options). An empty sourced table refuses with `invalid_option` and the sentence "`<Label>` offers no value to choose from right now." (the value echoed unless the key is encrypted), constraint `{ allowed: '' }`. - **`effectiveEnvOverride`, the option family**: the same rule. The refusal's detail reads "Allowed values: none (the running system offers no value right now)." The boot hold is untouched: a sourced refusal while held is taken in force and judged again at the seal. - **`reportRejectedEnvOverride`**: the "said once" key is `envName=` plus `stableStringify(value)` for an object or array (the file's own order-independent serialiser), `String(value)` for a scalar as before. Every map used to key as `[object Object]`. ## How the boot window is covered (`localization.locale`) - The key with real reach is `localization.locale`. Its source (`localizationLocaleOptionSource`) answers `undefined`, not an empty list, when the i18n service reports no locale, so the declared four apply and nothing here changes for it while the list is empty. - The `OS_*` door is held from `SettingsServicePlugin.init` (`holdOptionSourceVerdicts`) to `kernel:bootstrapped` (`sealOptionSources`). While held, a sourced key's refusal (an empty answer included) is taken in force, unreported and remembered; at the seal it is judged against the list as it then stands, said once if still refused, and a change is emitted. Pinned: an override held over an empty list stays in force when the list fills before the seal, and is judged and said once when it is still empty at the seal. - The save door has no hold. A save during the boot window against a source that answers an empty list would be refused; no shipped source does that (the locale source answers `undefined`), and a save comes over HTTP after the boot. ## Tests New `packages/services/service-settings/src/settings-select-empty-source-31550.test.ts` (9 rows), over a synthetic `pick_lab` manifest with two sourced keys (`select`, `multiselect`) and one unsourced `select`: - Save door: an empty answer refuses a select and a multiselect value (`invalid_option`, nothing stored). Controls: the value saves once the source offers it; a source that cannot answer leaves the declared options; an unsourced key is judged against its declared options. - Env door: an `OS_PICK_LAB_CHANNEL` override over an empty answer is not in force (one error line, the default reads). Controls: the boot hold above; a source that cannot answer leaves the declared options. - Dedupe: two different bad `OS_LIFECYCLE_QUOTAS` maps are each said once; the same map with its keys reordered is not said again. ## Acceptance notes - **Responsibility.** Finding 1 and an observation of the at-tier contract review of PR #31539 (6722607950), noted in #31422's os-dev-report (6722247790). - **Narrowing, stated.** A select-family value under a registered source that answers an empty list, saved or in `OS_*`, was accepted and is refused now. No value could be one the running system offers, so nothing that worked stops working. - **Worktree.** The claim names `objectstack-issue-31550`; this ran in the chain's existing worktree `objectstack-issue-31317` on the same claimed branch, cut from fresh `origin/main` after #31539 merged. ## Verification Head `1999c4c5` (base `4d165acd`, fresh `origin/main`, which holds #31539's merge `dc8479e0`). <details><summary>Red first, fix, suites</summary> - The new pin on main's source: Tests 5 failed | 4 passed (9): both save rows (`the save was accepted`), the env row, the hold row's still-empty half (`expected 'env' to be 'default'`) and the dedupe row (`expected "vi.fn()" to be called 2 times, but got 1 times`); the four controls green. With the fix: 9 passed. - `@objectstack/service-settings` vitest: Test Files 74 passed (74), Tests 1416 passed (1416). - `typecheck` for `@objectstack/service-settings`: exit 0. `eslint --no-inline-config packages/services/service-settings`: exit 0. </details> <details><summary>Ablation (ablation-replace, each restored to HEAD)</summary> - Save door: `if (sourced || allowed.length > 0) {` in `validatePatch` made to ignore `sourced` (`(sourced && String("ABL31550").length < 0) || …`), blob 2c3e7c6 to 6301804190b0: Tests 2 failed | 7 passed (9), the two save rows. - Env door: the same in `effectiveEnvOverride`, blob to fb851f1300fa: Tests 2 failed | 7 passed (9), the env row and the hold row. - Dedupe: the `stableStringify` arm made never-taken, blob to 9274e3133eb6: Tests 1 failed | 8 passed (9), the dedupe row. - Each restored: blob == HEAD (2c3e7c6), `git diff HEAD` empty. The suite reads its own `src/`, so no dist leg. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `1999c4c5`: 66 commands, each run with `timeout 1500` and stdin from `/dev/null`, exit codes captured before any pipe, after a `@objectstack/runtime` closure and `@objectstack/service-settings` build. All 66 exit 0 on the first run (the changeset carries the `adr-0087` line #31539 needed). `dispatch-gates --ran`: 66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN. Not run locally (CI): `test:repo`, dogfood shards, type-check lanes. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5nBSzc6ktJyjdFcDb3)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6b83ec2 commit 158cc2a

3 files changed

Lines changed: 231 additions & 7 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/service-settings': patch
3+
---
4+
5+
fix(service-settings): a select, radio or multiselect setting refuses every value when its option source answers an empty list
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) no key of any schema is added, removed or re-shaped and no stored row is read differently: the narrowing is one runtime refusal at the settings save door and the OS_* env door, for a select-family key whose registered option source answers an empty list, and its remedy is a value the source offers once it offers one, so there is nothing for `objectstack migrate meta` to rewrite. A value already stored is not re-judged. -->
11+
12+
A select, radio or multiselect key whose options come from a registered option source (`registerOptionSource`) was judged against that list only when the list had entries. When the source answered an empty list, any value was saved and any `OS_*` override was put in force. Now no value is admitted then: the save door refuses it with `invalid_option` ("… offers no value to choose from right now."), and the env door logs the override as not in force, as #31422 made the `map` family judge its keys. This narrows what is accepted. During the boot, while the option verdicts are held (`holdOptionSourceVerdicts`, sealed at `kernel:bootstrapped`), an override is still taken in force and judged at the seal, so a list the boot is still filling refuses nothing early. A key with no source, or whose source cannot answer, is judged against its declared options as before. Also, the env door's "said once" record now keys a structured value by a stable serialisation, so a second, different bad map under one `OS_*` variable is reported too (before, every map counted as the same value).
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#31550] A select / radio / multiselect key whose registered option source
5+
* ANSWERED an empty list admits no value, at both doors: the save
6+
* (`validatePatch`) and the `OS_*` override (`effectiveEnvOverride`). The map
7+
* family has judged its keys this way since #31422; the select family kept its
8+
* own `allowed.length > 0` gate, so an empty answer admitted any value.
9+
*
10+
* Unchanged, and pinned as controls: a key with no source (or whose source
11+
* answered nothing, `undefined`) is judged against its declared `options`; a
12+
* key that declares none has nothing to enforce. During the boot hold
13+
* (`holdOptionSourceVerdicts`) a sourced verdict is taken in force and judged
14+
* again at the seal, so an empty list the boot is still filling refuses
15+
* nothing early.
16+
*
17+
* Riding along: an env override's refusal is said once per value, and the
18+
* value is keyed by a stable serialisation, so two DIFFERENT bad maps under one
19+
* `OS_*` variable are each said once (keyed by `String(value)`, every map was
20+
* `[object Object]`, and the second was silenced).
21+
*/
22+
23+
import { describe, it, expect, vi } from 'vitest';
24+
import type { SettingsManifest } from '@objectstack/spec/system';
25+
import {
26+
lifecycleObjectKeySource,
27+
lifecycleSettingsManifest,
28+
type LifecycleKeyRegistryLike,
29+
} from '@objectstack/objectql';
30+
import { SettingsService } from './settings-service.js';
31+
32+
/** Synthetic: a sourced select, a multiselect, and an unsourced select with declared options. */
33+
const pickLabManifest = {
34+
namespace: 'pick_lab',
35+
version: 1,
36+
label: 'Pick Lab',
37+
scope: 'global',
38+
readPermission: 'setup.access',
39+
writePermission: 'setup.write',
40+
specifiers: [
41+
{ type: 'select', key: 'channel', label: 'Channel', required: false, default: 'none',
42+
options: [{ value: 'none', label: 'None' }, { value: 'email', label: 'Email' }] },
43+
{ type: 'multiselect', key: 'regions', label: 'Regions', required: false,
44+
options: [{ value: 'eu', label: 'EU' }] },
45+
{ type: 'select', key: 'tier', label: 'Tier', required: false, default: 'basic',
46+
options: [{ value: 'basic', label: 'Basic' }, { value: 'pro', label: 'Pro' }] },
47+
],
48+
} as unknown as SettingsManifest;
49+
50+
/** A source whose list the case sets; `undefined` = it could not answer. */
51+
function listSource(state: { values: string[] | undefined }) {
52+
return () => {
53+
if (state.values === undefined) return undefined;
54+
const values = state.values;
55+
return { options: values.map((value) => ({ value, label: value })), admits: (v: string) => values.includes(v) };
56+
};
57+
}
58+
59+
function service(env: Record<string, string>, state: { values: string[] | undefined }, opts: { hold?: boolean } = {}) {
60+
const error = vi.fn();
61+
const svc = new SettingsService({ env, logger: { warn: vi.fn(), error } as never });
62+
if (opts.hold) svc.holdOptionSourceVerdicts();
63+
svc.registerOptionSource('pick_lab', 'channel', listSource(state));
64+
svc.registerOptionSource('pick_lab', 'regions', listSource(state));
65+
svc.registerManifest(pickLabManifest);
66+
return { svc, error };
67+
}
68+
69+
/** The `FieldError`s a refused save carries. */
70+
async function refusal(p: Promise<unknown>): Promise<Array<Record<string, unknown>>> {
71+
const err: any = await p.then(() => undefined, (e) => e);
72+
expect(err, 'the save was accepted').toBeDefined();
73+
expect(err.code).toBe('SETTINGS_VALIDATION');
74+
return (err.errors ?? err.fields ?? err.details?.fields) as Array<Record<string, unknown>>;
75+
}
76+
77+
describe('[#31550] a source that answered an empty list admits no value — the save door', () => {
78+
it('refuses a select value: invalid_option, nothing stored', async () => {
79+
const { svc } = service({}, { values: [] });
80+
81+
const fields = await refusal(svc.setMany('pick_lab', { channel: 'email' }));
82+
83+
expect(fields).toHaveLength(1);
84+
expect(fields[0]).toMatchObject({ field: 'channel', code: 'invalid_option' });
85+
expect(fields[0].message).toBe("Channel offers no value to choose from right now. Received 'email'.");
86+
expect((await svc.get('pick_lab', 'channel')).value).toBe('none');
87+
});
88+
89+
it('refuses a multiselect value the same way', async () => {
90+
const { svc } = service({}, { values: [] });
91+
92+
const fields = await refusal(svc.setMany('pick_lab', { regions: ['eu'] }));
93+
94+
expect(fields[0]).toMatchObject({ field: 'regions', code: 'invalid_option' });
95+
});
96+
97+
it('CONTROL: once the source answers the value, it saves', async () => {
98+
const state = { values: [] as string[] | undefined };
99+
const { svc } = service({}, state);
100+
state.values = ['none', 'email'];
101+
102+
await svc.setMany('pick_lab', { channel: 'email' });
103+
104+
expect((await svc.get('pick_lab', 'channel')).value).toBe('email');
105+
});
106+
107+
it('CONTROL: a source that could not answer leaves the declared options, judged as ever', async () => {
108+
const { svc } = service({}, { values: undefined });
109+
110+
await svc.setMany('pick_lab', { channel: 'email' });
111+
const fields = await refusal(svc.setMany('pick_lab', { channel: 'sms' }));
112+
113+
expect((await svc.get('pick_lab', 'channel')).value).toBe('email');
114+
expect(fields[0]).toMatchObject({ field: 'channel', code: 'invalid_option', message: "Channel must be one of: none, email. Received 'sms'." });
115+
});
116+
117+
it('CONTROL: an unsourced key is judged against its declared options', async () => {
118+
const { svc } = service({}, { values: [] });
119+
120+
await svc.setMany('pick_lab', { tier: 'pro' });
121+
const fields = await refusal(svc.setMany('pick_lab', { tier: 'gold' }));
122+
123+
expect(fields[0]).toMatchObject({ field: 'tier', code: 'invalid_option' });
124+
});
125+
});
126+
127+
describe('[#31550] a source that answered an empty list admits no value — the env door', () => {
128+
it('an OS_* select override is not in force: one error line, the next layer reads', async () => {
129+
const { svc, error } = service({ OS_PICK_LAB_CHANNEL: 'email' }, { values: [] });
130+
131+
const got = await svc.get('pick_lab', 'channel');
132+
await svc.get('pick_lab', 'channel');
133+
134+
expect(got).toMatchObject({ value: 'none', source: 'default' });
135+
expect(error).toHaveBeenCalledTimes(1);
136+
expect(String(error.mock.calls[0][0])).toContain('OS_PICK_LAB_CHANNEL');
137+
expect(String(error.mock.calls[0][0])).toContain('IGNORED');
138+
expect(String(error.mock.calls[0][0])).toContain('Allowed values: none (the running system offers no value right now).');
139+
});
140+
141+
it('CONTROL: while the boot holds the verdicts the override is in force; at the seal it is judged against the list as it then stands', async () => {
142+
const state = { values: [] as string[] | undefined };
143+
const held = service({ OS_PICK_LAB_CHANNEL: 'email' }, state, { hold: true });
144+
145+
expect(await held.svc.get('pick_lab', 'channel')).toMatchObject({ value: 'email', source: 'env' });
146+
expect(held.error).not.toHaveBeenCalled();
147+
// The list fills before the seal: the override stays in force, nothing is said.
148+
state.values = ['none', 'email'];
149+
held.svc.sealOptionSources();
150+
expect(await held.svc.get('pick_lab', 'channel')).toMatchObject({ value: 'email', source: 'env' });
151+
expect(held.error).not.toHaveBeenCalled();
152+
153+
// A list still empty at the seal: judged then, said once.
154+
const stillEmpty = service({ OS_PICK_LAB_CHANNEL: 'email' }, { values: [] }, { hold: true });
155+
expect((await stillEmpty.svc.get('pick_lab', 'channel')).source).toBe('env');
156+
stillEmpty.svc.sealOptionSources();
157+
expect((await stillEmpty.svc.get('pick_lab', 'channel')).source).toBe('default');
158+
expect(stillEmpty.error).toHaveBeenCalledTimes(1);
159+
});
160+
161+
it('CONTROL: a source that could not answer leaves the declared options at the env door too', async () => {
162+
const { svc, error } = service({ OS_PICK_LAB_CHANNEL: 'email' }, { values: undefined });
163+
164+
expect(await svc.get('pick_lab', 'channel')).toMatchObject({ value: 'email', source: 'env' });
165+
expect(error).not.toHaveBeenCalled();
166+
});
167+
});
168+
169+
describe('[#31550] an env override refusal is said once per value, two different maps included', () => {
170+
const REGISTRY: LifecycleKeyRegistryLike = {
171+
getAllObjects: () => [{ name: 'sys_audit_log', label: 'Audit log', lifecycle: { class: 'audit' } }],
172+
};
173+
174+
it('two different bad maps under one OS_* variable are each said once', async () => {
175+
const env: Record<string, string> = { OS_LIFECYCLE_QUOTAS: '{"sys_audit_logs": 1000}' };
176+
const error = vi.fn();
177+
const svc = new SettingsService({ env, logger: { warn: vi.fn(), error } as never });
178+
svc.registerOptionSource('lifecycle', 'quotas', lifecycleObjectKeySource(() => REGISTRY));
179+
svc.registerManifest(lifecycleSettingsManifest);
180+
181+
await svc.get('lifecycle', 'quotas');
182+
await svc.get('lifecycle', 'quotas');
183+
expect(error).toHaveBeenCalledTimes(1);
184+
185+
env.OS_LIFECYCLE_QUOTAS = '{"sys_job_runs": 1000}';
186+
await svc.get('lifecycle', 'quotas');
187+
await svc.get('lifecycle', 'quotas');
188+
expect(error).toHaveBeenCalledTimes(2);
189+
expect(String(error.mock.calls[1][0])).toContain("'sys_job_runs' is not a key this setting admits.");
190+
191+
// The same map, its keys in another order, is the same value: not said again.
192+
env.OS_LIFECYCLE_QUOTAS = '{"sys_audit_logs": 1000, "sys_job_runs": 5}';
193+
await svc.get('lifecycle', 'quotas');
194+
env.OS_LIFECYCLE_QUOTAS = '{"sys_job_runs": 5, "sys_audit_logs": 1000}';
195+
await svc.get('lifecycle', 'quotas');
196+
expect(error).toHaveBeenCalledTimes(3);
197+
});
198+
});

‎packages/services/service-settings/src/settings-service.ts‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,13 +1528,19 @@ export class SettingsService {
15281528
// A key with no declared table has nothing to enforce — unchanged
15291529
// behaviour (#5131's exhaustive-options semantics, untouched when no
15301530
// domain is declared).
1531-
const { allowed, judge } = this.optionTableOf(namespace, key, reg.optionTables.get(key) ?? []);
1532-
if (allowed.length > 0) {
1531+
// [#31550] …and a source that ANSWERED is the table even when it
1532+
// answered no options, as the map family judges its keys (#31422): the
1533+
// `allowed.length` gate is the declared fallback's alone. The boot hold
1534+
// below covers a list the boot is still filling.
1535+
const { allowed, sourced, judge } = this.optionTableOf(namespace, key, reg.optionTables.get(key) ?? []);
1536+
if (sourced || allowed.length > 0) {
15331537
const rejected = judge(value);
15341538
if (rejected && !this.holdsOptionSourceVerdict(namespace, key)) {
15351539
this.reportRejectedEnvOverride(reg, namespace, key, envName, rejected.value, {
15361540
what: 'is not a declared option for',
1537-
detail: `Allowed values: ${allowed.join(', ')}.`,
1541+
detail: allowed.length > 0
1542+
? `Allowed values: ${allowed.join(', ')}.`
1543+
: 'Allowed values: none (the running system offers no value right now).',
15381544
fix: 'one of the allowed values',
15391545
});
15401546
return null;
@@ -1703,7 +1709,10 @@ export class SettingsService {
17031709
*/
17041710
rejection: { what: string; detail: string; fix: string; shape?: string },
17051711
): void {
1706-
const dedupeAt = `${envName}=${String(offending)}`;
1712+
// [#31550] Keyed by a stable serialisation of a structured value: by
1713+
// `String(offending)` every map was `[object Object]`, so a second,
1714+
// different bad map under the same variable was never said.
1715+
const dedupeAt = `${envName}=${offending !== null && typeof offending === 'object' ? stableStringify(offending) : String(offending)}`;
17071716
if (this.reportedEnvOverrides.has(dedupeAt)) return;
17081717
this.reportedEnvOverrides.add(dedupeAt);
17091718

@@ -3420,13 +3429,16 @@ export class SettingsService {
34203429
if (!empty && OPTION_BEARING_TYPES.has(type) && !domain) {
34213430
// The table a registered option source serves, else the declared one
34223431
// (see `optionTableOf`).
3423-
const { allowed, judge } = this.optionTableOf(namespace, key, declaredOptionValues(spec.options));
3432+
const { allowed, sourced, judge } = this.optionTableOf(namespace, key, declaredOptionValues(spec.options));
34243433
// A manifest with no option table cannot say what is legal. The spec
34253434
// refuses that shape at parse time, but `registerManifest` takes
34263435
// manifests as given (no Zod pass), so skip rather than reject every
34273436
// write to a hand-built manifest — same leniency the unparseable
34283437
// `visible` and invalid `pattern` branches already take.
3429-
if (allowed.length > 0) {
3438+
// [#31550] That leniency is the declared table's alone: a registered
3439+
// source that ANSWERED is the table even when it answered no options,
3440+
// and then no value is one of them (as the map family, #31422).
3441+
if (sourced || allowed.length > 0) {
34303442
// Shared with the env path (#5204) — see `firstRejectedOption` for the
34313443
// array/scalar handling and why the result is wrapped.
34323444
const rejected = judge(value);
@@ -3440,7 +3452,9 @@ export class SettingsService {
34403452
errors.push({
34413453
field: key,
34423454
code: 'invalid_option',
3443-
message: `${label} must be one of: ${allowed.join(', ')}.${got}`,
3455+
message: allowed.length > 0
3456+
? `${label} must be one of: ${allowed.join(', ')}.${got}`
3457+
: `${label} offers no value to choose from right now.${got}`,
34443458
label,
34453459
// The allowed set as a discrete constraint, so a client composes
34463460
// its own sentence instead of parsing ours (`FieldError.

0 commit comments

Comments
 (0)