Skip to content

Commit 061406d

Browse files
os-zhuangclaude
andauthored
fix(spec): the protection-envelope check was hollow — it skipped 24 of 25 types (#4001) (#4519)
The invariant test added one change ago asserted two things about every registered metadata type: that it does not REJECT the ADR-0010 envelope its loader stamps (the hard-422 case), and that it does not silently lose it. The reject half worked — it found `hook` and `datasource` on its first run. The other half did not. It probed each schema with one generic body and asked whether `_packageId` survived. A type whose required fields that body did not supply failed for unrelated reasons, and the assertion returned early. TWENTY- FOUR of the twenty-five types took that early return. Only `field` was ever really checked, and the suite reported green. That is this campaign's own subject matter — a success signal covering an omission — reproduced inside the instrument built to detect it, one change after the ledger recorded the identical lesson about the strictness gate's non-recursive directory walk. A check that skips is indistinguishable from a check that passes. The declaration side is now STRUCTURAL: it walks the schema, unwrapping lazy/pipe/optional/default and expanding unions, and asks whether any resolved object shape declares the key. That needs no valid instance, so it cannot skip. Two guards keep it honest: - a type the walker cannot resolve is a hard FAILURE, not a pass. The walker going quiet is exactly when this test would otherwise stop covering something. - the debt list carries a reverse pin that fails when an entry is fixed, so the list cannot outlive the debt it tracks. What it found: 8 registered types do not declare the envelope, not 1 — `action`, `book`, `field`, `job`, `mapping`, `page`, `translation`, `validation`. Each loses protection metadata on every round-trip today and becomes a hard 422 the day its schema closes. `job` and `book` are closed here; 6 remain listed. Three occurrences now of one pattern, in three different instruments: the ledger gate's non-recursive walk, `strictObject(` not matching the site count, and this early return. Every one was a measuring tool reporting coverage it did not have. The ledger now states the rule it keeps re-deriving: before trusting a green check, make it go red on something you know is there. Verified: spec 282 files / 7141 tests, `tsc --noEmit` clean, all 8 generated artifacts current (the only regeneration is the two envelope key tables), all 15 `check:*` gates green. Claude-Session: https://claude.ai/code/session_01WnqGjQFQMqd5k81LYV8SCY Co-authored-by: Claude <noreply@anthropic.com>
1 parent c2a1134 commit 061406d

8 files changed

Lines changed: 217 additions & 56 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
The protection-envelope invariant test was hollow — it silently skipped 24 of 25 registered types. Fixed, and it immediately found 8 undeclared envelopes instead of 1.
6+
7+
The check shipped in the previous change asserted two things about every registered metadata type: that it does not *reject* the ADR-0010 envelope its loader stamps (the hard-422 case), and that it does not *strip* it (the silent-loss case). The reject half worked — it found `hook` and `datasource` on its first run.
8+
9+
The strip half did not. It probed each schema with one generic body and asked whether `_packageId` survived; a type whose required fields that body did not satisfy failed for unrelated reasons and the assertion returned early. **24 of the 25 types took that early return.** Only `field` was ever actually checked, and the suite reported green.
10+
11+
That is the campaign's own subject matter — a success signal covering an omission — reproduced inside the instrument built to detect it, one change after the ledger recorded the same lesson about the strictness gate's non-recursive directory walk. A check that skips is indistinguishable from a check that passes.
12+
13+
**The declaration side is now structural.** It walks the schema — unwrapping `lazy` / `pipe` / `optional` / `default` and expanding unions — and asks whether any resolved object shape declares the key. That answer does not require constructing a valid instance, so it cannot skip. Two guards keep it honest: a type whose shape the walker cannot resolve is a hard failure (the walker going quiet is exactly when this test would otherwise stop covering something), and the debt list carries a reverse pin that fails when an entry is fixed, so the list cannot outlive the debt it tracks.
14+
15+
**What it found:** 8 registered types do not declare the envelope, not 1 — `action`, `book`, `field`, `job`, `mapping`, `page`, `translation`, `validation`. `job` and `book` are closed here, leaving 6 on the list. Each is protection metadata lost on every round-trip today, and a hard 422 the day its schema is closed.

content/docs/references/system/book.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ const result = Book.parse(data);
6666
| **order** | `number` | optional | Orders books within the portal |
6767
| **audience** | `'org' \| 'public' \| { permissionSet: string }` | optional | Access audience; defaults to 'org' (inherits package grant) |
6868
| **groups** | `{ key: string; label: string; translations?: Record<string, { label: string }>; order?: number; … }[]` || The spine: ordered sections. Two levels total. |
69+
| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). |
70+
| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. |
71+
| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). |
72+
| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). |
73+
| **_packageId** | `string` | optional | Owning package machine id. |
74+
| **_packageVersion** | `string` | optional | Owning package version. |
75+
| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. |
6976

7077

7178
---

content/docs/references/system/job.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ const result = CronSchedule.parse(data);
6565
| **retryPolicy** | `{ maxRetries?: integer; backoffMs?: integer; backoffMultiplier?: number }` | optional | Retry policy: failed runs (including timeouts) are retried with exponential backoff (delay = backoffMs * backoffMultiplier^(retry-1)) up to maxRetries retries after the initial attempt (#3494). Omit for the legacy single-attempt behavior. |
6666
| **timeout** | `integer` | optional | Per-attempt time limit in milliseconds; an over-limit run is recorded with execution status "timeout" (#3494). The in-flight handler is abandoned, not forcibly cancelled. Omit for no time limit. |
6767
| **enabled** | `boolean` | optional | Whether the job is enabled |
68+
| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). |
69+
| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. |
70+
| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). |
71+
| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). |
72+
| **_packageId** | `string` | optional | Owning package machine id. |
73+
| **_packageVersion** | `string` | optional | Owning package version. |
74+
| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. |
6875

6976

7077
---

docs/audits/2026-07-unknown-key-strictness-ledger.md

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,34 @@ dropped at parse, and nothing failed.
169169

170170
The check separates the two severities, because they are not the same bug:
171171
*rejecting* the envelope is live breakage and is asserted unconditionally with
172-
no exemption list; *stripping* it silently loses protection metadata on
173-
round-trip and is tracked with a debt list (`field` only) — and each entry
174-
there becomes a rejection the day its schema is closed.
172+
no exemption list; *not declaring* it silently loses protection metadata on
173+
round-trip and is tracked with a debt list — and each entry there becomes a
174+
rejection the day its schema is closed.
175+
9. **And then that check turned out to be hollow — one change after this file
176+
recorded the same lesson about the gate above.** Its declaration half probed
177+
each schema with one generic body and asked whether `_packageId` survived. A
178+
type whose required fields that body did not satisfy failed for unrelated
179+
reasons and the assertion returned early, so **24 of 25 registered types took
180+
that early return**. Only `field` was ever really checked, and the suite
181+
reported green.
182+
183+
Rewritten to walk the schema *structurally* — unwrapping `lazy` / `pipe` /
184+
`optional` / `default`, expanding unions — which needs no valid instance and
185+
therefore cannot skip. Two guards keep it honest: a type the walker cannot
186+
resolve is a hard failure (the walker going quiet is precisely when the test
187+
would otherwise stop covering something), and the debt list carries a reverse
188+
pin that fails when an entry is fixed, so the list cannot outlive its debt.
189+
190+
It then found **8** undeclared envelopes rather than 1 — `action`, `book`,
191+
`field`, `job`, `mapping`, `page`, `translation`, `validation`. `job` and
192+
`book` were closed immediately; 6 remain.
193+
194+
Three occurrences now of one pattern, in three different instruments: the
195+
ledger gate's non-recursive directory walk, the strip probe's early return,
196+
and (from the other direction) `strictObject(` not matching the site count.
197+
Each was a measuring tool reporting completeness it did not have. **The rule
198+
this file keeps re-deriving: before trusting a green check, make it go red on
199+
something you know is there.**
175200

176201
This is the empirical argument for the ratchet: the inference "no metadata in
177202
the repo carries unknown keys" was **false three times over**, and only the

packages/spec/authorable-surface.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5682,6 +5682,13 @@
56825682
"system/BatchProgress:status",
56835683
"system/BatchProgress:succeeded",
56845684
"system/BatchProgress:total",
5685+
"system/Book:_lock",
5686+
"system/Book:_lockDocsUrl",
5687+
"system/Book:_lockReason",
5688+
"system/Book:_lockSource",
5689+
"system/Book:_packageId",
5690+
"system/Book:_packageVersion",
5691+
"system/Book:_provenance",
56855692
"system/Book:audience",
56865693
"system/Book:description",
56875694
"system/Book:groups",
@@ -6136,6 +6143,13 @@
61366143
"system/IncidentResponsePolicy:triageDeadlineHours",
61376144
"system/IntervalSchedule:intervalMs",
61386145
"system/IntervalSchedule:type",
6146+
"system/Job:_lock",
6147+
"system/Job:_lockDocsUrl",
6148+
"system/Job:_lockReason",
6149+
"system/Job:_lockSource",
6150+
"system/Job:_packageId",
6151+
"system/Job:_packageVersion",
6152+
"system/Job:_provenance",
61396153
"system/Job:description",
61406154
"system/Job:enabled",
61416155
"system/Job:handler",

packages/spec/src/kernel/metadata-type-schemas.test.ts

Lines changed: 129 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -9,30 +9,40 @@
99
* type, and `getMetaItemLayered` → `saveMetaItem` round-trips a body carrying
1010
* the stamped `_packageId` / `_provenance`. A type whose schema does not declare
1111
* {@link MetadataProtectionFields} therefore mishandles it in one of two ways,
12-
* and the severities are different enough to assert separately:
12+
* and the severities differ enough to assert separately:
1313
*
1414
* - **Rejects it** (the schema is `.strict()`): a hard 422 on the overlay path.
15-
* Live breakage. Asserted unconditionally below — no debt list.
16-
* - **Strips it** (the schema is strip-mode): the envelope is silently dropped
17-
* on every parse, so protection metadata is lost on round-trip. Quieter, and
18-
* it becomes the first case the day that schema is closed.
15+
* Live breakage. Asserted unconditionally — no exemption list.
16+
* - **Does not declare it** (strip mode): the envelope is silently dropped on
17+
* every parse, so protection metadata is lost on round-trip. Quieter, and it
18+
* becomes the first case the day that schema is closed.
1919
*
20-
* ## Why this file exists at all
20+
* ## Why this file exists
2121
*
2222
* The same defect was found four separate times, by four different routes,
23-
* before anyone wrote a check for it:
23+
* before anyone wrote a check for it: `permission` (#4001 Tier-A, as a hard 422
24+
* caught by the dogfood gate), `position` (step 2, by reading), `seed` + `doc`
25+
* (the registered-types batch, while converting), and then `hook` +
26+
* `datasource` — which THIS test found on its first run, both already strict on
27+
* `main` and therefore both in the 422 class.
2428
*
25-
* 1. `permission` (#4001 Tier-A) — surfaced as a hard 422 on the ADR-0094
26-
* overlay path, caught by the dogfood gate.
27-
* 2. `position` (#4001 step 2) — found by reading, as the "known sibling gap".
28-
* 3. `seed` + `doc` (#4001 registered-types batch) — found while converting.
29-
* 4. `hook` + `datasource` — found by THIS test, on its first run. Both had
30-
* gone `.strict()` in the #4001 data step without declaring the envelope,
31-
* so both were in the hard-422 class on `main` at the time.
29+
* ## Why the declaration check is structural, not a parse probe
3230
*
33-
* Finding one defect four times by hand is the signal that the check is
34-
* missing, not that the search worked. Case 4 is the argument in miniature: two
35-
* live bugs that three prior hand-searches had walked past.
31+
* The first version of this file probed with one generic body and asked whether
32+
* `_packageId` survived. It reported green. It was hollow: a type whose required
33+
* fields the generic body did not satisfy failed for unrelated reasons, and the
34+
* assertion returned early — **so 24 of 25 types were silently skipped and only
35+
* `field` was ever really checked.** A check that skips is indistinguishable
36+
* from a check that passes, which is the exact defect this whole campaign is
37+
* about, reproduced in the instrument built to detect it.
38+
*
39+
* So the declaration side now walks the schema structurally — unwrapping
40+
* `lazy` / `pipe` / `optional` / `default` and expanding unions — and asks
41+
* whether any resolved object shape declares the key. That answer does not
42+
* depend on constructing a valid instance, so it cannot skip. And a type whose
43+
* shape cannot be resolved at all is a hard FAILURE rather than a pass: the
44+
* walker not understanding a schema is exactly when this test would otherwise
45+
* go quiet.
3646
*/
3747

3848
import { describe, expect, it } from 'vitest';
@@ -42,12 +52,7 @@ import { listMetadataTypeSchemaTypes, getMetadataTypeSchema } from './metadata-t
4252
/** The ADR-0010 stamp the loader puts on every registered item. */
4353
const STAMP = { _packageId: 'pkg_probe', _provenance: 'package' as const };
4454

45-
/**
46-
* A body that satisfies the required fields of the registered types generously
47-
* enough to reach the unknown-key check. Types it does not fully satisfy fail
48-
* on other grounds, which the assertions below distinguish and ignore — this
49-
* test is only about how the `_`-prefixed envelope is treated.
50-
*/
55+
/** A body generous enough to reach the unknown-key check on most types. */
5156
const PROBE: Record<string, unknown> = {
5257
name: 'probe_item',
5358
label: 'Probe',
@@ -58,6 +63,66 @@ const PROBE: Record<string, unknown> = {
5863
...STAMP,
5964
};
6065

66+
/**
67+
* Registered types that parse the envelope but do not declare it, so it is
68+
* dropped on every round-trip. Every entry is a bug awaiting a
69+
* `...MetadataProtectionFields` spread — not a permanent exemption — and each
70+
* becomes a hard 422 the day its schema is closed. Empty this list; never grow
71+
* it. A NEW registered type belongs in neither list.
72+
*
73+
* The structural walk found 8 of these; the probe it replaced had been hiding 7.
74+
* `job` and `book` were closed in the same pass, leaving 6.
75+
*/
76+
const UNDECLARED_ENVELOPE = new Set<string>([
77+
'action', 'field', 'mapping', 'page', 'translation', 'validation',
78+
]);
79+
80+
/**
81+
* Every object shape reachable from `schema`, unwrapping the wrappers the
82+
* registered types actually use and expanding unions. Returns `[]` only when
83+
* the walker does not understand the schema — which the caller treats as a
84+
* failure, never as a pass.
85+
*/
86+
function objectShapes(schema: unknown, depth = 0): Record<string, unknown>[] {
87+
if (!schema || depth > 12) return [];
88+
const s = schema as { shape?: Record<string, unknown>; _zod?: { def?: Def }; def?: Def };
89+
const def = s._zod?.def ?? s.def;
90+
switch (def?.type) {
91+
case 'object':
92+
return [s.shape ?? def.shape ?? {}];
93+
case 'lazy':
94+
try {
95+
return objectShapes(def.getter?.(), depth + 1);
96+
} catch {
97+
return [];
98+
}
99+
case 'pipe':
100+
return [...objectShapes(def.in, depth + 1), ...objectShapes(def.out, depth + 1)];
101+
case 'union':
102+
return (def.options ?? []).flatMap((o) => objectShapes(o, depth + 1));
103+
case 'optional':
104+
case 'nullable':
105+
case 'default':
106+
case 'prefault':
107+
case 'readonly':
108+
case 'nonoptional':
109+
case 'catch':
110+
return objectShapes(def.innerType, depth + 1);
111+
default:
112+
return [];
113+
}
114+
}
115+
116+
interface Def {
117+
type?: string;
118+
shape?: Record<string, unknown>;
119+
getter?: () => unknown;
120+
in?: unknown;
121+
out?: unknown;
122+
options?: unknown[];
123+
innerType?: unknown;
124+
}
125+
61126
/** `_`-prefixed keys the schema reported as unrecognized, if any. */
62127
function rejectedEnvelopeKeys(type: string): string[] {
63128
const result = getMetadataTypeSchema(type)!.safeParse(PROBE);
@@ -68,27 +133,33 @@ function rejectedEnvelopeKeys(type: string): string[] {
68133
.filter((k) => k.startsWith('_'));
69134
}
70135

71-
/**
72-
* Types that parse the envelope but drop it. Every entry is a bug awaiting a
73-
* `...MetadataProtectionFields` spread, not a permanent exemption — and each
74-
* becomes a hard 422 the day its schema is closed. Empty this list; never grow
75-
* it. A new registered type belongs in neither list.
76-
*/
77-
const STRIPS_ENVELOPE = new Set<string>([
78-
// Registered so a single field can be addressed as a metadata item, but
79-
// authored inside `object.fields`, where the object's own envelope covers the
80-
// package. Closing this one means auditing that nesting, so it is tracked
81-
// rather than bundled into the batch that found it.
82-
'field',
83-
]);
84-
85136
describe('registered metadata types', () => {
86137
const types = listMetadataTypeSchemaTypes();
87138

88139
it('is a non-empty set — guards the derivation returning nothing', () => {
89140
expect(types.length).toBeGreaterThan(15);
90141
});
91142

143+
it('every registered type resolves to a schema', () => {
144+
for (const type of types) {
145+
expect(getMetadataTypeSchema(type), `no schema registered for '${type}'`).toBeDefined();
146+
}
147+
});
148+
149+
/**
150+
* The no-silent-skip guard. If the walker stops understanding a schema shape,
151+
* the declaration assertions below would quietly stop covering that type —
152+
* so that condition fails here first, loudly, with the type named.
153+
*/
154+
it.each(types)('%s resolves to at least one object shape the walker understands', (type) => {
155+
expect(
156+
objectShapes(getMetadataTypeSchema(type)).length,
157+
`the structural walker cannot resolve '${type}' to an object shape, so the `
158+
+ 'envelope assertions below would silently skip it. Teach `objectShapes` the '
159+
+ 'wrapper this schema uses.',
160+
).toBeGreaterThan(0);
161+
});
162+
92163
it.each(types)('%s does not REJECT the protection envelope its loader stamps', (type) => {
93164
expect(
94165
rejectedEnvelopeKeys(type),
@@ -98,25 +169,30 @@ describe('registered metadata types', () => {
98169
).toEqual([]);
99170
});
100171

101-
it.each(types.filter((t) => !STRIPS_ENVELOPE.has(t)))(
102-
'%s does not STRIP the protection envelope',
172+
it.each(types.filter((t) => !UNDECLARED_ENVELOPE.has(t)))(
173+
'%s DECLARES the protection envelope',
103174
(type) => {
104-
const result = getMetadataTypeSchema(type)!.safeParse(PROBE);
105-
// A probe that fails for unrelated reasons (a required field this generic
106-
// body does not supply) tells us nothing about stripping — and the reject
107-
// case is already covered unconditionally above.
108-
if (!result.success) return;
175+
const shapes = objectShapes(getMetadataTypeSchema(type));
109176
expect(
110-
(result.data as Record<string, unknown>)._packageId,
111-
`'${type}' silently drops \`_packageId\` — protection metadata is lost on `
112-
+ 'every round-trip. Add `...MetadataProtectionFields` to its schema.',
113-
).toBe(STAMP._packageId);
177+
shapes.some((shape) => '_packageId' in shape),
178+
`'${type}' does not declare \`_packageId\`, so the envelope its loader stamps is `
179+
+ 'dropped on every parse. Add `...MetadataProtectionFields` to its schema.',
180+
).toBe(true);
114181
},
115182
);
116183

117-
it('every registered type resolves to a schema', () => {
118-
for (const type of types) {
119-
expect(getMetadataTypeSchema(type), `no schema registered for '${type}'`).toBeDefined();
120-
}
121-
});
184+
it.each([...UNDECLARED_ENVELOPE])(
185+
'%s is still on the undeclared-envelope debt list (remove it once fixed)',
186+
(type) => {
187+
// A reverse pin: when someone fixes one of these, this fails and forces the
188+
// list to shrink. Without it the debt list would outlive the debt and start
189+
// exempting types that no longer need exempting.
190+
expect(types).toContain(type);
191+
const shapes = objectShapes(getMetadataTypeSchema(type));
192+
expect(
193+
shapes.some((shape) => '_packageId' in shape),
194+
`'${type}' now declares the envelope — remove it from UNDECLARED_ENVELOPE.`,
195+
).toBe(false);
196+
},
197+
);
122198
});

packages/spec/src/system/book.zod.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { z } from 'zod';
44
import { lazySchema } from '../shared/lazy-schema';
5+
import { MetadataProtectionFields } from '../kernel/metadata-protection.zod';
56

67
/**
78
* Package Documentation Navigation — the `book` element (ADR-0046 §6).
@@ -115,6 +116,13 @@ export const BookSchema = lazySchema(() =>
115116
order: z.number().optional().describe('Orders books within the portal'),
116117
audience: BookAudienceSchema.optional().describe("Access audience; defaults to 'org' (inherits package grant)"),
117118
groups: z.array(BookGroupSchema).describe('The spine: ordered sections. Two levels total.'),
119+
120+
// ADR-0010 — runtime protection envelope (internal — set by the loader).
121+
// `book` is a registered metadata type, so the artifact loader stamps
122+
// `_packageId` / `_provenance` on it like every sibling. Undeclared, they
123+
// were dropped on every parse — protection metadata lost on round-trip, and
124+
// a hard 422 waiting for the day this shape is closed.
125+
...MetadataProtectionFields,
118126
}),
119127
);
120128

0 commit comments

Comments
 (0)