Skip to content

Commit 7d3edf6

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-22428-field-existence-every-spelling
Claude-Session: https://claude.ai/code/session_01Bw3y2DWhT9RPnrmDsNqEVG Co-authored-by: Claude <noreply@anthropic.com>
2 parents 071050a + e148ca9 commit 7d3edf6

23 files changed

Lines changed: 987 additions & 66 deletions
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): a hook that destructures a renamed key off `ctx` is no longer refused as not-lowerable (#22459)
6+
7+
Clause-②: no
8+
9+
- **What changed.** The free-identifier scan behind `objectstack lint`'s `hook-body/not-lowerable` rule and `objectstack build`'s hook lowering counted a renamed destructuring key as a free identifier. For `const { event, input, previous: prev } = ctx`, it reported `previous`, a property read off `ctx`, although the only name the pattern binds is `prev`. So `objectstack lint` failed the hook with the advice "reach them through `ctx`", which the hook already did, and `objectstack build` shipped it as a bundled closure instead of a metadata-only body. A non-computed destructuring key is now a key, as the key of `{ key: value }` already was. Nested patterns (`{ a: { b: c } } = ctx`), patterns inside array patterns, the parameters of a callback inside the handler, and `catch` clauses are covered by the same rule.
10+
- **What is still refused.** A destructuring default or a computed key that names something out of scope is still free: `const { key: alias = FALLBACK } = ctx` and `const { [KEY]: v } = ctx` report `FALLBACK` and `KEY`, as before. A genuinely free `previous` that is not read off `ctx` is still refused.
11+
- **What to do.** Nothing. A hook written this way now lowers to a body on the next `objectstack build`, and `objectstack lint` stops reporting it. The un-renamed spelling (`const prev = ctx.previous`) keeps working as before.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/plugin-auth': minor
3+
---
4+
5+
With SSO domain verification on, `POST /api/v1/auth/admin/sso/request-domain-verification` and `POST /api/v1/auth/admin/sso/verify-domain` answer an unknown `providerId` with `404 RESOURCE_NOT_FOUND`, no longer with `400 DOMAIN_VERIFICATION_DISABLED`
6+
7+
Clause-②: yes (widening)
8+
9+
- **Before.** With `OS_SSO_DOMAIN_VERIFICATION` on, a platform admin who sent a `providerId` that does not exist got `400 DOMAIN_VERIFICATION_DISABLED`, "Domain verification is not enabled for this environment (set OS_SSO_DOMAIN_VERIFICATION)": an instruction to turn on a feature that was already on. `@better-auth/sso` answers both an unmounted route (feature off) and an unknown provider (feature on) with a `404` that carries no `code`, and both bridges mapped every such `404` to the feature-off answer.
10+
- **Now.** The two mounts hand each bridge the environment's own setting (`AuthManager.isSsoDomainVerificationEnabled()`). Off: `400 DOMAIN_VERIFICATION_DISABLED`, as before, and the vendor is no longer called. On: a code-less vendor `404` is the provider lookup missing, answered `404 RESOURCE_NOT_FOUND` with a message naming the provider id. The decision never reads the vendor's `404` wording.
11+
- **Unchanged.** The platform-admin gate answers first, on or off (an anonymous caller gets `401`). A missing `providerId` is still `400 INVALID_REQUEST`. With the feature on, an existing provider still gets the vendor's own answer: the DNS record, `NO_PENDING_VERIFICATION`, `DOMAIN_VERIFICATION_FAILED`, a `403` for a provider the caller may not manage.
12+
- **The exported bridges.** `runRequestDomainVerification` and `runVerifyDomain` take an optional third argument, `{ domainVerificationEnabled?: boolean }`. A caller that omits it keeps the earlier mapping: a code-less `404` is answered `400 DOMAIN_VERIFICATION_DISABLED`, byte for byte. A host that mounts these helpers itself passes the environment's setting to get the not-found answer.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
`ISecurityService` declares an optional, feature-detected member that answers the existing ADR-0055 master-detail write check: `checkControlledByParentWrite(object, recordId, context)`.
6+
7+
Clause-②: yes (widening)
8+
9+
- **What it answers.** It answers what the master-detail write check decides for an update of one record by the caller. A by-id write to a `controlled_by_parent` record requires edit access to its master, and the write path checks each master up the chain: the object-level `update` grant, the master's write row-level security, and record sharing. The answer equals what a by-id update of the same record gets from that check, the ADR-0090 D10 delegator leg included. It adds no verdict: it exposes the one the write path already reaches, so a gate on a record's attachments or comments can judge the record as its own update is judged.
10+
- **What it resolves with.** `ControlledByParentWriteOutcome` is a discriminated union. `allow` means the check does not refuse. `deny` names the leg that refused (`ControlledByParentWriteDenialLeg`: `object_permission`, `row_level_security`, `record_sharing` or `master_chain`). `not_applicable` means the object is not `controlled_by_parent`. `unresolvable` names why the check reached no verdict (`ControlledByParentWriteUnresolvedReason`: a missing `master_detail` relation, a record that does not exist, or an empty master reference). A store fault is not an outcome: the member rejects with the engine's own error, so the fault keeps its declared status (a datasource outage is `503`).
11+
- **Optional, and absence is typed.** A security service without the member still satisfies the contract, and an unguarded call does not compile. Absence means no master check is composed for a consumer to ask, which is not a policy that admits.
12+
13+
Nothing an author writes changes, and no runtime behaviour changes. `@objectstack/plugin-security` does not serve the member yet. An implementation typed as `ISecurityService` that serves this name must serve it under the declared signature.

‎.claude/skills/pm-dispatch/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ PM 的工作是循环:选卡 → 认领 → 派发 → 收集 → 复核 → 报
183183
- governed 面统一定义:`docs/adr/**` + `.claude/**`(全量)+ `skills/**` + `docs/NORTH-STAR.md`。
184184
- governed 面同含 `AGENTS.md` + `CLAUDE.md`;`GOVERNED_REPOS` 各仓同治理待遇,执行席恒随落地仓车道。
185185
- 路径面命中 Tier H ⇒ ACCEPT 换终局四件套,混合 diff ⛔ 不按比例判;要拆让 dev 单独开 PR。
186-
- 改动 >5000 行(含生成物)同换终局四件套,⛔ 无 Tier S 例外;读数 = PR additions+deletions。
186+
- 改动 >3000 行(含生成物)同换终局四件套,⛔ 无 Tier S 例外;读数 = PR additions+deletions。
187187
- ① 复核结论照常写在 issue 上;技能面 hunk 须由契约复审档的席复核,档外席先交 skills 席。
188188
- ② PR 留给维护者看得见地悬着;终局两条:人工直合即审核记录;授权批准 ⇒ 席位落地。
189189
- 看得见 = ACCEPT 同笔挂 `needs-user-decision` + 贴终稿「维护者速读」评论;①仍是审核记录。

‎.claude/skills/pm-dispatch/references/landing-operations.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
- 确认 MERGED 要两个读数:每轮同时读队列分支与 `origin/main`。
5656
- 契约复核 PASS 落地的 PR 到窗口时已 ready 且 auto-merge 在挂,见 `contract-review.md`。
5757
- 窗口自身权责不变:跟到 MERGED、踢出处置、落地后对账。
58-
- 转 ready/入队前跑 `check-governed-merges.mjs --pr N`:受管面照两层;>5000 行(含生成物)照 Tier H。
58+
- 转 ready/入队前跑 `check-governed-merges.mjs --pr N`:受管面照两层;>3000 行(含生成物)照 Tier H。
5959
- 受管路径全在 `.claude/**` 者 Tier S:席内达档复核过落地前检三条即转正式入队。
6060
- Tier H(其余受管面)者:四件套留 draft 等人批,⛔ 不翻正式不入队;获授权批准后认领席落地。
6161
- ⛔ 两层不由席位批准;清标即落地同受此闸,漏判会被队列守卫在 merge group 里拒收。

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ Even inside your own worktree, operate defensively:
507507
⛔ **Three classes of PR never enter this path on green alone:** (a) a diff touching any **governed surface**
508508
(**Prime Directive #14**, which names them and holds the current list — **this file and `CLAUDE.md` are on it**,
509509
so re-read it rather than recalling it); (b) the **Version Packages** PR, or any PR whose merge performs a
510-
release (**Prime Directive #15**); (c) a PR whose **changed lines exceed 5,000** (`additions + deletions`,
510+
release (**Prime Directive #15**); (c) a PR whose **changed lines exceed 3,000** (`additions + deletions`,
511511
generated files included) — it lands the way a Tier H surface does: an authorized APPROVED review and then
512512
the owning seat, or a human merge. Read the PR's file list (`get_files`), **its author and its size** before
513513
you arm anything.

‎packages/cli/src/lint/hook-body-lowering.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { lowerCallables } from '../utils/lower-callables.js';
2222

2323
// Module scope — exactly what a lowered body cannot reach.
2424
const SLA_MATRIX: Record<string, number> = { high: 4, low: 48 };
25+
// Module scope, spelled like a `ctx` key — a reference to THIS is free.
26+
const previous = { rank: 1 };
2527

2628
const freeIdentifierHook = {
2729
name: 'case_sla',
@@ -139,6 +141,56 @@ describe('checkHookBodyLowering', () => {
139141
});
140142
});
141143

144+
describe('a hook that destructures a renamed key off `ctx` reaches its value through `ctx`', () => {
145+
// `previous` is a property READ OFF ctx — exactly what this rule's own
146+
// remedy prescribes — and `prev` is the only name the pattern binds. The
147+
// scan used to count the key as a free identifier, so lint refused the
148+
// hook (advising what it already did) and the build bundled it.
149+
const renamedKeyHook = {
150+
name: 'carry_rank',
151+
object: 'task',
152+
events: ['beforeInsert', 'beforeUpdate'],
153+
handler: (ctx: any) => {
154+
const { event, input, previous: prev } = ctx;
155+
if (event === 'beforeUpdate' && prev) input.rank = prev.rank;
156+
},
157+
};
158+
// The control: the same spelling, but read from module scope — not `ctx`.
159+
const freePreviousHook = {
160+
name: 'carry_rank_free',
161+
object: 'task',
162+
events: ['beforeUpdate'],
163+
handler: (ctx: any) => {
164+
const prev = previous;
165+
ctx.input.rank = prev.rank;
166+
},
167+
};
168+
169+
it('lint reports nothing, and the build lowers it to a body', () => {
170+
expect(checkHookBodyLowering({ hooks: [{ ...renamedKeyHook }] })).toEqual([]);
171+
172+
const lowering = lowerCallables({ hooks: [{ ...renamedKeyHook }] });
173+
expect(lowering.bodyExtractionWarnings).toEqual([]);
174+
expect(lowering.bodyExtracted).toBe(1);
175+
const [hook] = lowering.lowered.hooks as Array<{ body?: { source: string } }>;
176+
expect(hook.body?.source).toContain('previous: prev');
177+
});
178+
179+
it('a genuinely free `previous` is still refused, by both', () => {
180+
const issues = checkHookBodyLowering({ hooks: [{ ...freePreviousHook }] });
181+
expect(issues).toHaveLength(1);
182+
expect(issues[0].severity).toBe('error');
183+
expect(issues[0].rule).toBe(NOT_LOWERABLE_RULE);
184+
expect(issues[0].path).toBe('hooks[0].handler');
185+
186+
const lowering = lowerCallables({ hooks: [{ ...freePreviousHook }] });
187+
expect(lowering.bodyExtracted).toBe(0);
188+
expect(lowering.bodyExtractionWarnings.map((w) => [w.kind, w.freeIdentifiers])).toEqual([
189+
['free-identifiers', ['previous']],
190+
]);
191+
});
192+
});
193+
142194
it('keeps the STRUCTURAL refusal a warning — the bundle is its designed answer', () => {
143195
const issues = checkHookBodyLowering({ hooks: [forbiddenTokenHook] });
144196

‎packages/cli/src/utils/detect-free-identifiers.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,65 @@ describe('detectFreeIdentifiers (#1876 — body self-containment)', () => {
7575
}
7676
});
7777

78+
// A renamed destructuring key — `const { previous: prev } = ctx` — reads
79+
// `previous` OFF `ctx`; the only name it introduces is `prev`. The key is a
80+
// property name exactly like the `key` of `{ key: value }`, so counting it as
81+
// a free identifier refused a self-contained handler (and bundled it), with
82+
// advice — "reach them through `ctx`" — describing what it already did.
83+
describe('a destructuring KEY is not a reference; its alias is the binding', () => {
84+
const lowerable: Array<[string, string]> = [
85+
[
86+
'a renamed key off ctx, beside shorthand keys',
87+
"(ctx) => { const { event, input, previous: prev } = ctx; if (event === 'beforeUpdate' && prev) input.n = prev.n; }",
88+
],
89+
['a nested renamed pattern', '(ctx) => { const { a: { b: c } } = ctx; return c; }'],
90+
[
91+
'a defaulted alias whose default is in scope',
92+
'(ctx) => { const d = ctx.fallback; const { key: alias = d } = ctx; return alias; }',
93+
],
94+
[
95+
'a computed key and a default, both in scope',
96+
'(ctx) => { const k = ctx.k; const d = 0; const { [k]: v = d } = ctx; return v; }',
97+
],
98+
// The same element shape reached two other ways.
99+
['an object pattern inside an array pattern', '(ctx) => { const [{ previous: prev }] = ctx.items; return prev; }'],
100+
['a nested callback parameter', '(ctx) => ctx.items.map(({ previous: prev }) => prev.n)'],
101+
['a catch-clause pattern', '(ctx) => { try { ctx.run(); } catch ({ message: msg }) { ctx.log = msg; } }'],
102+
// No key at all: elements are bound by position, so nothing to mistake.
103+
['an array pattern', '(ctx) => { const [first, second] = ctx.items; return first + second; }'],
104+
];
105+
for (const [label, source] of lowerable) {
106+
it(label, () => {
107+
const r = detectFreeIdentifiers(source);
108+
expect(r.unparsed).toBe(false);
109+
expect(r.free).toEqual([]);
110+
});
111+
}
112+
113+
// The controls: what the element still READS is still judged. Only the key
114+
// stopped counting — a default or a computed key that names something out
115+
// of scope is as free as it ever was.
116+
it('a defaulted alias whose default is free is still reported', () => {
117+
const r = detectFreeIdentifiers('(ctx) => { const { key: alias = FALLBACK } = ctx; return alias; }');
118+
expect(r.free).toEqual(['FALLBACK']);
119+
});
120+
121+
it('a shorthand element whose default is free is still reported', () => {
122+
const r = detectFreeIdentifiers('(ctx) => { const { previous = FALLBACK } = ctx; return previous; }');
123+
expect(r.free).toEqual(['FALLBACK']);
124+
});
125+
126+
it('a computed key that is free is still reported', () => {
127+
const r = detectFreeIdentifiers('(ctx) => { const { [KEY]: v } = ctx; return v; }');
128+
expect(r.free).toEqual(['KEY']);
129+
});
130+
131+
it('a genuinely free `previous` (no ctx source) is still reported', () => {
132+
const r = detectFreeIdentifiers('(ctx) => { const prev = previous; ctx.input.n = prev.n; }');
133+
expect(r.free).toEqual(['previous']);
134+
});
135+
});
136+
78137
describe('real compiled `.toString()` shapes', () => {
79138
it('does not flag a self-contained closure', () => {
80139
const handler = (ctx: any) => {
@@ -83,6 +142,15 @@ describe('detectFreeIdentifiers (#1876 — body self-containment)', () => {
83142
};
84143
expect(detectFreeIdentifiers(src(handler)).free).toEqual([]);
85144
});
145+
146+
it('does not flag a renamed destructuring key after the compiler has had it', () => {
147+
const handler = (ctx: any) => {
148+
const { event, input, previous: prev } = ctx;
149+
if (event === 'beforeUpdate' && prev) input.n = prev.n;
150+
};
151+
expect(src(handler)).toContain('previous: prev');
152+
expect(detectFreeIdentifiers(src(handler)).free).toEqual([]);
153+
});
86154
});
87155

88156
it('never invents free vars for non-handler junk (conservative — caller won\'t block)', () => {

‎packages/cli/src/utils/detect-free-identifiers.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,9 @@ function collectBindings(fn: ts.FunctionLikeDeclarationBase): Set<string> {
227227
/**
228228
* Collect identifiers used in VALUE position (potential references). Excludes
229229
* the false-positive sources: property-access member names, non-shorthand
230-
* object/class member keys, and statement labels. Binding names that slip
231-
* through are harmless — they are subtracted via `bindings` downstream.
230+
* object/class member keys, non-computed destructuring keys, and statement
231+
* labels. Binding names that slip through are harmless — they are subtracted
232+
* via `bindings` downstream.
232233
*/
233234
function collectReferences(fn: ts.FunctionLikeDeclarationBase): Set<string> {
234235
const refs = new Set<string>();
@@ -253,6 +254,19 @@ function collectReferences(fn: ts.FunctionLikeDeclarationBase): Set<string> {
253254
walk(node.initializer);
254255
return;
255256
}
257+
if (ts.isBindingElement(node)) {
258+
// `{ key: alias = d } = ctx` — the same rule as `{ key: value }` above:
259+
// `key` names a property READ OFF the source, so it is not a ref (unless
260+
// computed). Visit the binding name (an alias is subtracted via
261+
// `bindings`; a nested pattern recurses here), a computed key's
262+
// expression, and the default — `{ [k]: v = d }` reads `k` and `d`.
263+
if (node.propertyName && ts.isComputedPropertyName(node.propertyName)) {
264+
walk(node.propertyName.expression);
265+
}
266+
walk(node.name);
267+
if (node.initializer) walk(node.initializer);
268+
return;
269+
}
256270
if (ts.isMethodDeclaration(node) || ts.isPropertyDeclaration(node) || ts.isGetAccessor(node) || ts.isSetAccessor(node)) {
257271
if (node.name && ts.isComputedPropertyName(node.name)) walk(node.name.expression);
258272
ts.forEachChild(node, (c) => { if (c !== node.name) walk(c); });

‎packages/plugins/plugin-auth/src/admin-sso-bridge-gate.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,10 @@ const mockCtx = (): PluginContext =>
8787
/**
8888
* Mount the plugin's REAL route registration on a real Hono app (the
8989
* auth-catchall-fallthrough.test.ts harness), with the auth manager reduced to
90-
* the two seams the bridges read: `getApi().getSession` (what the gate judges)
91-
* and `handleRequest` (where the bridge delegates).
90+
* the seams the bridges read: `getApi().getSession` (what the gate judges),
91+
* `handleRequest` (where the bridge delegates) and, after the gate, the two
92+
* domain-verification mounts' `isSsoDomainVerificationEnabled()` [#22463] —
93+
* ON here, as in `makeSsoVendor` below.
9294
*/
9395
async function mountBridges(deps: {
9496
getSession: (headers: Headers) => unknown | Promise<unknown>;
@@ -110,6 +112,7 @@ async function mountBridges(deps: {
110112
getApi: async () => ({
111113
getSession: async ({ headers }: { headers: Headers }) => deps.getSession(headers),
112114
}),
115+
isSsoDomainVerificationEnabled: () => true,
113116
};
114117
(plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx);
115118
return app;

0 commit comments

Comments
 (0)