Skip to content

docs(skills): a hook that enforces a business rule declares refusals and refuses by code - #33488

Draft
objectstack-fleet[bot] wants to merge 4 commits into
mainfrom
claude/project-thread-g605co
Draft

objectstack-fleet[bot] wants to merge 4 commits into
mainfrom
claude/project-thread-g605co

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #33257

domain:skills#1 · os-project-manager · session_01FEkQ5njvc1ya6PypTHRBe5

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:objectstack-data 技能的 hook 参考文档「Declared refusals」一节,开头改为一条规则:执行业务规则的 hook 声明 refusals 目录、每条规则一个代码、body 抛出该代码;没有目录时随手抛的文字错误会被平台答成 HOOK_ABORTED,只适合意料之外的失败。示例换成跨对象状态守卫「只有在库资产才能借出」(ASSET_NOT_IN_STOCK,409),并写明今天只有沙箱 js body 能声明 refusals,handler 写法(含内联 TS)保存时被拒。rules/validation.md 里指向该文档的税号 hook 示例同步改为声明 TAX_ID_BLOCKED 并抛代码。
  • 为什么改:一次画廊场景实测里,AI 用 hook 正确拦下了「维修中的资产借出」,但抛的是无代码的文字错误,线上得到平台通用的 HOOK_ABORTED,用户与 agent 都拿不到有意义的代码和译文;技能原文的验证规则示例恰好教的就是这种写法。
  • 风险与回滚:纯技能文字,两文件 +47 / -29 行,token 在各自文件内用删除重复内容付清,未抬任何上限;不动 schema、不动运行时;回滚即还原本 PR。
  • 先看:skills/objectstack-data/references/data-hooks.md 的「Declared refusals」一节;再看 skills/objectstack-data/rules/validation.md 的税号 hook 示例。
  • 两分钟验证:git diff origin/main -- skills/,预期新示例声明 refusals: { ASSET_NOT_IN_STOCK: … status: 409 } 并 throw Object.assign(new Error(), { code: 'ASSET_NOT_IN_STOCK' }),税号示例不再有 throw new Error('Tax ID is blocked')。
  • 需要你做的:受管面 Tier H(skills/**):请批准或合并。

What changes

  • skills/objectstack-data/references/data-hooks.md § Declared refusals opens with the rule the card asks for: a hook that enforces a business rule declares a refusals catalogue, one SCREAMING_SNAKE code per rule, and its body throws the code. Without a catalogue, a throw is answered HOOK_ABORTED, the platform's code, which suits a failure nobody planned and not a rule the user can act on.
    • The section's example (two single-object codes, CREDIT_LIMIT / ON_HOLD, as an unchecked fragment) is replaced by the card's case as a full os:check hook: checkout_requires_stock, a beforeInsert on asset_checkout that reads the asset through ctx.api (api.read) and throws ASSET_NOT_IN_STOCK (declared at 409) unless the asset's status is in_stock. It fails closed: a checkout with no asset_id or an unknown id is refused too (probed below); an asset the saver cannot read finds nothing, so it is refused the same way (read, not probed).
    • Which hook forms can declare refusals on main today: only a sandboxed js body; a handler hook, an inline TS function included, is refused (bullet replaces "Refused at save without a js body (e.g. a bare handler)"). Nothing hooks: the refusal catalogue (refusals) cannot translate a handler-authored hook's refusals, and holds one sentence per code — a TS hook has no way to refuse in the user's language #33214 would add is taught.
    • What reaches the wire: the status's registered code (default 400), with the hook's code as declaredCode (bullet replaces "the envelope carries one as declaredCode").
    • The Error Handling intro now says onError gives no code; Best Practices' "give a thrown error a message the caller can act on" becomes "refuse a business rule by a declared code", linking the section.
  • skills/objectstack-data/rules/validation.md § External / Custom Validation → Lifecycle Hooks, the hook rule that points to data-hooks.md: the pointer sentence says the body "throws a declared code" (linked to the section) instead of "throws on failure", and its os:check tax-ID hook declares refusals: { TAX_ID_BLOCKED: { message: 'This tax ID is blocked.' } } and throws that code instead of throw new Error('Tax ID is blocked'), which is exactly the HOOK_ABORTED shape the card measured. This file was outside the order's expected surface; the seat approved it in its thread before the edit (pointer sentence plus the example's two lines).
  • .changeset/33257-skills-hook-refuses-by-declared-code.md: '@objectstack/skills': patch, carrying both declaration lines.

Why

The card's measured build (gallery debut refresh 2026-11-14, card asset_lifecycle, recorded build 2026-11-14-scenario-asset_lifecycle.json, cited and not re-run: a live run needs the maintainer's word, per the order) enforced "only an in-stock asset can be checked out" with a sandboxed hook that declared no refusals; the probe write was refused HOOK_ABORTED (400), a platform code. The skill taught the catalogue only as a mechanism, not as the rule for business refusals, and rules/validation.md taught the free-text throw outright.

Mechanism, read on origin/main b5f2f516:

  • packages/spec/src/data/hook.zod.ts: HookSchema.refusals is a record keyed by a SCREAMING_SNAKE code of HookRefusalSchema (message: plain text, a {{ }} hole refused; status: 400 to 499, absent means 400). refuseUndeclaredLiteralRefusalCode refuses refusals on a hook whose body.language is not js (message from hookRefusalsWithoutSandboxedBody, packages/spec/src/data/hook-refusal-codes.ts:201), and refuses a literally thrown code the catalogue does not declare.
  • packages/runtime/src/sandbox/hook-refusal-catalogue.ts resolveHookRefusal: a body throw carrying a declared code is answered with the entry's message (translated through hooks.HOOK.refusals.CODE.message in the caller's locale) as userMessage, the entry's status (default DEFAULT_HOOK_REFUSAL_STATUS 400) and the code; an undeclared code, or none, becomes a sandbox fault (500); host-relayed errors and crashes pass through.
  • packages/types/src/thrown-http-error.ts resolveThrownHttpError: a code outside the ADR-0112 ledger resolves the wire code from the status (standardErrorCodeForHttpStatus: 400 VALIDATION_ERROR, 409 RESOURCE_CONFLICT) and keeps the thrown code as declaredCode. A sandbox business throw with no code at a 4xx is answered UNDECLARED_HOOK_REFUSAL_CODE = HOOK_ABORTED (ledger row in packages/spec/src/api/error-code-ledger.zod.ts). So HOOK_ABORTED does not come from onError: 'abort' as such: onError: 'abort' (the default) only makes the throw abort the write; the code comes from the throw carrying none.

Acceptance notes

Verification

Head 47af59e9, base origin/main b5f2f516.

Both new examples run in the real runtime (scratch probe, not committed)

A throwaway vitest file in packages/runtime/src/sandbox/, on the harness of overlap-hook-template.integration.test.ts (real ObjectQL, SQLite driver, QuickJSScriptRunner, bindHooksToEngine, answers read off mapDataError), each hook passed through HookSchema.parse first. Run after building runtime's dependency closure under the verify lock (VERDICT command-exit 0); then deleted (git status clean). Result: Test Files 1 passed, Tests 3 passed, exit 0.

  • checkout_requires_stock: an in-stock asset checks out; an in_repair asset, a checkout with no asset_id, and an unknown id each answer 409 {"error":"Only an in-stock asset can be checked out.","code":"RESOURCE_CONFLICT","declaredCode":"ASSET_NOT_IN_STOCK","userMessage":"Only an in-stock asset can be checked out."}; one checkout row stored.
  • crm_tax_id_blocklist_check (new shape): 400 {"error":"This tax ID is blocked.","code":"VALIDATION_ERROR","declaredCode":"TAX_ID_BLOCKED","userMessage":"This tax ID is blocked."}.
  • The old tax shape (free text, no catalogue): 400 {"error":"Tax ID is blocked","code":"HOOK_ABORTED"}, the card's symptom.
  • HookSchema.safeParse of a hook with refusals and a string handler, and with an inline function handler: both success: false, one issue at refusals ("refusals is answered only for a sandboxed body (language: 'js') ...").
Line and token budget (skills/**)
  • skills/objectstack-data/references/data-hooks.md: 884 to 893 lines; tokens 9962 to 10009 (ceiling 10011, headroom 2). Paid in the same file: the protocol-17 history sentence under "An unevaluable condition ABORTS" (its point is restated by the paragraph after it), that paragraph shortened, and the old fragment example.
  • skills/objectstack-data/rules/validation.md: 331 to 330 lines; tokens 2754 to 2760 (ceiling 2761, headroom 1). Paid by deleting the "Date range validation" line from Common Script Patterns, which restates the date_range cross_field example in the same file.
  • Whole package, all skills/*/SKILL.md: 4603 to 4603 lines (no SKILL.md touched). Bundle total (whole shipped tree): 244659 to 244712 tokens.
  • node scripts/check-skills-token-ratchet.mjs: exit 0. node scripts/pm/check-skill-line-ratchet.mjs: exit 0 (neither file is on its register). No ceiling raised.
Derived gates: 45 derived, 44 run green, 1 NOT MEASURED

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 45 families for the three paths. All run on head 47af59e9, exit captured before any pipe, after building @objectstack/lint..., @objectstack/client-react... and @objectstack/runtime^... under the verify lock (VERDICT command-exit 0, 320s):

  • 44 exit 0, among them check-skills-token-ratchet (scan and --self-test), check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check:doc-authoring, check:skill-links, check:skill-identifier-liveness, check:skill-frame-sync, check:corpus-claim-drift, check:nul-bytes, check:pm-skill-id-lint, @objectstack/lint check:doc-formula-expressions, @objectstack/spec check:skill-refs, check:dts-closure, check:lean-entry-closure, check:sourcemap-no-sources-content.
  • NOT MEASURED, exit 3 PREREQUISITE NOT MET: check:dual-build-cjs-loads (reads every package's dist; no package source is touched, so no full build was paid locally; CI builds and runs it).
  • --ran reconciliation: "45 derived famil(ies) accounted for — 44 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)".
  • Extra, not derived: pnpm --filter @objectstack/spec check:skill-examples exit 0, "346 prose examples type-check across 3 surface(s)", which covers the new and the edited os:check blocks.
  • CI families are read from this PR's check-runs, not re-run here.
Tier and governed readings
  • dispatch-gates --tier: MANDATORY contract-review tier, both skills/** paths under clause ① (the published catalog); no one-line exit for this surface.
  • node scripts/pm/check-governed-merges.mjs --branch claude/project-thread-g605co: exit 3, GOVERNED, landing tier H (人合). 2 of 3 paths hit the register (skills/** x2); the changeset is not on it. Size +47 / -29, under the 3000 threshold.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FEkQ5njvc1ya6PypTHRBe5

Round 1 (head 652a4a09, after contract review 6885289276)

  • Delete guard (S1): data-hooks.md Common Patterns "1. Preventing Deletion (sandboxed body)" now declares refusals STRATEGIC_ACCOUNT (400) and OPEN_OPPORTUNITIES (409) and throws each by code. The second message drops the count, because a catalogue message takes no parameters.
  • Wording (N1, N2): "a body's throw is answered HOOK_ABORTED"; onError "gives no code of the hook's own".
  • Paid in place: the "Because the VM only holds a snapshot…" paragraph (restated elsewhere in the file) and "(declared, never produced)" in the ctx.session row.
  • Figures: data-hooks.md 884 → 892 lines, 9962 → 10004 tokens (ceiling 10011); validation.md 2760/2761, unchanged. No ceiling moved.
  • Verification on 652a4a09: 45 derived gates, 44 exit 0, 1 NOT MEASURED (check:dual-build-cjs-loads). Scratch runtime probe, 4 passed: the guard answers 400 VALIDATION_ERROR with declaredCode: STRATEGIC_ACCOUNT, then 409 RESOURCE_CONFLICT with declaredCode: OPEN_OPPORTUNITIES, and deletes an account whose only opportunity is closed.

…red code

Co-Authored-By: Claude <noreply@anthropic.com>
…ap the validation pointer

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 14, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#1 · os-project-manager · session_01FEkQ5pjSPT7ui2EUcunaVb

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 47af59e99e21bac55935a0d514e234b7a32056cf
Local-runs: none
Scope: skills/objectstack-data/references/data-hooks.md, skills/objectstack-data/rules/validation.md + none

The diff holds no code, gate or schema file, so no implementing file is in scope. The changeset is the lane seat's ACCEPT at TIER_DEFAULT. Code read on origin/main to judge the two hits files, not reviewed: packages/spec/src/data/hook.zod.ts (refusals, HookRefusalSchema, refuseUndeclaredLiteralRefusalCode, handler/body jsdoc), packages/spec/src/data/hook-refusal-codes.ts (hookRefusalCodesThrownIn, hookRefusalsWithoutSandboxedBody), packages/runtime/src/sandbox/hook-refusal-catalogue.ts (resolveHookRefusal, DEFAULT_HOOK_REFUSAL_STATUS), packages/types/src/thrown-http-error.ts (resolveThrownHttpError, UNDECLARED_HOOK_REFUSAL_CODE), packages/types/src/data-error-classification.ts (sandboxRefusalCodeFields), packages/types/src/sandbox-business-message.ts, packages/mcp/src/mcp-http-tools.ts (dataDoorErrorResult, refusalAsTheDoorServesIt), packages/qa/scenarios/src/score/scenario-operation-grade.ts (refusalMeets), packages/qa/scenarios/src/score/scenario-run-grade.ts (refusalOf), and issue #33214.

Inputs: card #33257 body and all 3 comments (triage, claim, os-dev-report); PR #33488 body, file list (3 files, +47 / -29) and net diff against main (merge base b5f2f516; main is now b392ff7b, 4 commits ahead, none touching these paths or the cited code); the check-runs on the head.

Check-runs on the head at render time: 46 runs. 24 completed success, 18 completed skipped, 4 in progress (Test Core (1/8), Test Core (2/8), Type Check · consumer gates, Type Check · workspace). None failed. Every lint family has concluded success, including Lint · docs & code guards, Lint · repo & tooling gates, Lint · PM dispatch-gates, Lint · ESLint, Check Changeset, Check PR Size, Governed Surface Queue Guard and PR-body guards. This PASS covers the design. The 4 runs still in progress are the gate verdicts for their families, and the queue reads their conclusions.

① Derived judgments

  • Accept set: unchanged. Neither file is a schema, and the diff touches no packages/ source. Both os:check blocks parse against HookSchema as it stands on main. refusals is a record keyed by SCREAMING_SNAKE code with { message, status? } and status in 400 to 499. Each block has a js body, and each literally thrown code is declared, so refuseUndeclaredLiteralRefusalCode raises nothing. That parse is the check:skill-examples family, which is in a concluded lint run. Right.
  • Public surface: the published @objectstack/skills catalog changes. Two prose files change, with no frontmatter, no SKILL.md and no index or eval file. This is a teaching change only. Right.

② Semver level

  • .changeset/33257-skills-hook-refuses-by-declared-code.md: '@objectstack/skills': patch. A prose-only change to a published skill file is a patch, and @objectstack/skills sits in the fixed group in .changeset/config.json. Right.
  • Clause-②: no, on the PR body and the changeset: no accept set is narrowed or widened, and no runtime behaviour moves. Right. Metadata-protocol: none: right.

③ Boundary flags

  • Dev deviation 1, the widened surface to rules/validation.md, is ESCALATED. The dispatch says the seat approved the widening on card skills (data-hooks): a hook that enforces a business rule declares refusals and refuses by code, not a bare onError abort (HOOK_ABORTED) #33257's comments. It is not there. The card has 3 comments (triage 6880063659, claim 6884900591, os-dev-report 6885205230), and none of them approves it. The PR has no comments or reviews. The only source is the dev's own report: "asked the seat first and the seat approved it in its thread". The claim's File surface: admits "a pointer line in a hook rule that points to it only if now incomplete". The pointer sentence in validation.md fits that. The two example lines and the deleted "Date range validation" line go beyond it. The card's own Ask header does name "the hook rules that point to it", and on the merits the edit is right: the old example taught the exact HOOK_ABORTED shape the card measured. The design judgment is not affected. The seat should put the approval on the card, or cite where it lives, before Tier H.
  • Dev deviation 2, the tier, is ANSWERED. dispatch-gates --tier reads MANDATORY contract review for skills/**. This record is that review.
  • Dev deviation 3, the signature form, is NOTED and not contract-bearing. The dev says the signature went in as a prose line. The PR body now carries both that prose line and a trailing HTML signed: comment, which the relay presumably added.
  • Dev deviation 4, the mechanism refinement, is ANSWERED: it is correct. resolveThrownHttpError answers UNDECLARED_HOOK_REFUSAL_CODE (HOOK_ABORTED) only when no code was declared, the status is 4xx and sandboxBusinessMessage reads an inner sentence. onError: 'abort' only makes the throw abort.
  • open_questions: none.
  • Out-of-scope finding, the ask-item-2 lint proposal: ANSWERED. The card makes item 2 conditional ("Otherwise the prose rule is enough"). Not building it is within the order.
  • Out-of-scope finding, "other skills files not swept": PARTLY WRONG. One unswept free-text business throw sits inside a hits file, data-hooks.md Common Patterns 1. See ④ finding S1.
  • FOLLOW-UP, outside this PR: the card's acceptance premise does not hold end to end on main, and item 3's re-run would still fail.
    • refusalMeets passes an app code at a 4xx only when that code fails ErrorCode. The runner reads error.code off the tool output (refusalOf, scenario-run-grade.ts).
    • The MCP record data tools serve an unregistered declared code demoted, through refusalAsTheDoorServesIt (mcp: the record data tools keep an unregistered declared-4xx code and drop a refusal's userMessage, where /data answers the closed code with declaredCode and serves userMessage (LEAVE_EXHAUSTED / 409) #31407). For example, code: 'RESOURCE_CONFLICT', declaredCode: 'LEAVE_EXHAUSTED' at 409, as pinned in mcp-coded-refusal.test.ts. The PR's own probe reads the same demotion for ASSET_NOT_IN_STOCK.
    • So a live run of the taught hook is observed as RESOURCE_CONFLICT, a registered code, and does not meet VALIDATION_FAILED. Neither does any declared refusal at any status: 400 demotes to VALIDATION_ERROR.
    • The grader's unit tests (mid-tier-run-readings.test.ts) feed code: 'ASSET_NOT_IN_STOCK' by hand, so they never see the demotion.
    • The fix belongs to the qa/scenarios lane: refusalOf or refusalMeets should read declaredCode. It is not a defect of this skill text. The skill's wire sentence matches the platform. A follow-up card is owed.

④ Design judgment

skills/objectstack-data/references/data-hooks.md, judged against card #33257's scenario (asset_lifecycle, check_out_asset_in_repair) and the spec it teaches on main:

  • The opener matches the code. It says a business rule declares a catalogue with one SCREAMING_SNAKE code per rule and the body throws the code (HOOK_REFUSAL_CODE_PATTERN, resolveHookRefusal). It also says a code-less throw is answered HOOK_ABORTED.
  • The new example meets the card's ask: a cross-object state guard, "only an in-stock asset can be checked out", ASSET_NOT_IN_STOCK at 409.
    • It parses: api.read is granted, ctx.api.object(...).findOne({ where: { id } }) is the bridge's read shape, and hookRefusalCodesThrownIn reads the code: property inside the throw.
    • It fails closed. A missing id or an unread row leaves asset falsy, so asset?.status is not in_stock and the hook throws.
    • new Error() yields the inner sentence Error, which isNativeErrorName does not treat as a crash, so the catalogue answers it.
  • The bullet "Only a js body can declare one: a handler hook, inline TS included, is refused at save" is right on main. refuseUndeclaredLiteralRefusalCode refuses whenever body.language is not js. os validate parses the pre-build form, where an inline handler has no body; hooks: the refusal catalogue (refusals) cannot translate a handler-authored hook's refusals, and holds one sentence per code — a TS hook has no way to refuse in the user's language #33214 records this, and nothing hooks: the refusal catalogue (refusals) cannot translate a handler-authored hook's refusals, and holds one sentence per code — a TS hook has no way to refuse in the user's language #33214 would add is taught.
  • The bullet "The wire code is the status's (default 400), with the hook's as declaredCode" is right: DEFAULT_HOOK_REFUSAL_STATUS is 400, and standardErrorCodeForHttpStatus demotes the code into declaredCode.
  • The Best Practices link #declared-refusals-refusals resolves to the section heading.
  • The trimmed protocol-17 history paragraph keeps the operative rule: an unevaluable condition fails the write.

skills/objectstack-data/rules/validation.md:

  • The pointer sentence now links to the section.
  • The tax-ID hook declares TAX_ID_BLOCKED with the default 400 and throws it by code. That is the declared shape, and it agrees with data-hooks.md.

Findings:

  • S1 — should-fix — data-hooks.md, Common Patterns, "1. Preventing Deletion (sandboxed body)".
    • The same file still teaches the shape the card measured, in its copy-paste cookbook: a sandboxed beforeDelete guard with no refusals that refuses two business rules by throw new Error('Cannot delete Strategic accounts') and throw new Error('Cannot delete an account with ' + open + ' open opportunities').
    • The second rule is a cross-object ctx.api count, the card's own pattern. On main, both throws are answered HOOK_ABORTED.
    • The new opener says such a hook "declares a refusals catalogue … and its body throws the code". The file now states the rule and shows the opposite one section earlier, where an author building a guard is most likely to copy from.
    • Fix, on the prose face: declare two codes, for example STRATEGIC_ACCOUNT and OPEN_OPPORTUNITIES, and throw them by code. The second message cannot carry the count, because a catalogue message takes no parameter (hook-refusal-codes.ts; hooks: the refusal catalogue (refusals) cannot translate a handler-authored hook's refusals, and holds one sentence per code — a TS hook has no way to refuse in the user's language #33214 item 2), so it becomes plain text.
    • Tokens are paid in the same file; the headroom is 2. This earns a delta review.
  • N1 — nit — data-hooks.md, Declared refusals opener. "Without one, a throw is answered HOOK_ABORTED" is broader than the code.
    • It holds for a sandboxed body's throw with no code.
    • A body throw that carries its own code, with no catalogue, is answered with the status's code plus declaredCode.
    • A handler hook's bare throw is answered 500 INTERNAL_ERROR (mcp-http-tools.ts, measured note on rowRefusalOf).
    • A tighter reading would be "Without one, a body's code-less throw …".
  • N2 — nit — data-hooks.md, Error Handling intro. It says a throw and onError: 'log' give "neither … a code". The next paragraph says a bare throw is answered HOOK_ABORTED, which is a code. The intent is "no code of the hook's own".
  • N3 — nit — data-hooks.md example scope. checkout_requires_stock binds beforeInsert only, so an update that re-points asset_id on an existing checkout is not guarded. Adding beforeUpdate would need the Object.assign({}, ctx.previous, ctx.input) merge the overlap example uses. A known-limits clause, or leaving it as is, is acceptable for a minimal example.
  • N4 — nit — validation.md, the deleted "Date range validation" line. The remaining date_range example is not an exact restatement of it. The deleted pattern refused only an end strictly before the start. The remaining example also refuses a same-day end. This is an accepted deviation: the token payment is reasonable, and nothing taught is now wrong.

Implemented-by: claude/project-thread-g605co
Reviewed-by: session_01FEkQ5pjSPT7ui2EUcunaVb

VERDICT: PASS

Adopted by the seat 2026-11-14T22:36Z · isolated sub-agent at CONTRACT_REVIEW_TIER · Ruling-ref: Leehom card cmsg_01TenzsRopZj4jMbvjX8uz1XFhuiZmTH8VFgMmHRmpiGAb, option A, tapped 2026-11-14T15:37Z

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#1 · os-project-manager · session_01FEkQ5pjSPT7ui2EUcunaVb

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 652a4a09d7861fd8b9ffcb8c1014d731d60d5663
Local-runs: none
Scope: skills/objectstack-data/references/data-hooks.md, skills/objectstack-data/rules/validation.md + none

This is a delta review. The previous record, comment 6885289276, was a PASS on head 47af59e99e21bac55935a0d514e234b7a32056cf with one should-fix (S1) and four nits (N1 to N4). The delta is two commits on top of it: b0006cc1 (data-hooks.md, +10 / -11) and 652a4a09 (the changeset body, one line). rules/validation.md is unchanged since the previous head. The diff holds no code, gate or schema file, so no implementing file is in scope. The changeset is the lane seat's ACCEPT at TIER_DEFAULT, judged here under ② only.

Code read on origin/main to judge the delta, not reviewed: packages/spec/src/data/hook.zod.ts (refuseUndeclaredLiteralRefusalCode, HookRefusalSchema), packages/spec/src/data/hook-refusal-codes.ts (HOOK_REFUSAL_CODE_PATTERN, hookRefusalCodesThrownIn, HOOK_REFUSAL_MESSAGE_WITHOUT_HOLE), packages/runtime/src/sandbox/hook-refusal-catalogue.ts (resolveHookRefusal, declaredRefusalMessage, DEFAULT_HOOK_REFUSAL_STATUS), packages/runtime/src/sandbox/body-runner.ts (the hook body's catch), packages/runtime/src/sandbox/quickjs-runner.ts (how a thrown code reaches SandboxError), packages/types/src/thrown-http-error.ts (resolveThrownHttpError, UNDECLARED_HOOK_REFUSAL_CODE), packages/spec/scripts/check-skill-examples.ts, and the two workflow jobs that hold the skills gates.

Inputs: comment 6885289276; card #33257 body and all 4 comments (triage 6880063659, claim 6884900591, os-dev-report 6885205230, seat surface-widening record 6885297883); PR #33488 body and file list (3 files, +54 / -37); git diff 47af59e9 652a4a09; the net diff against main where context was needed (merge base b5f2f516; main is now f068cde8, 10 commits ahead, and among the cited paths only packages/qa/scenarios/src/score/scenario-run-grade.ts moved, in a decline-verb regex unrelated to this PR); the check-runs on the head.

Check-runs on the head at render time: 42 runs. 22 completed success, 16 completed skipped, 4 in progress (Test Core (1/8), Test Core (2/8), Type Check · consumer gates, Type Check · workspace). None failed. Concluded success includes Type Check · source gates (it runs check-skills-token-ratchet, so the delta's token payment holds), Lint · docs & code guards, Lint · repo & tooling gates, Lint · PM dispatch-gates, Lint · ESLint, Check Changeset, Check PR Size, Governed Surface Queue Guard and PR-body guards. Type Check · consumer gates runs check:skill-examples, the tsc pass over every os:check block, the rewritten delete guard included; it is still in progress, and its conclusion is that gate's verdict. This PASS covers the design, and the queue reads the 4 open runs.

① Derived judgments

  • Accept set: unchanged. The delta touches one prose file and the changeset body, no schema and no packages/ source. Right.
  • The rewritten os:check block parses against HookSchema as it stands on main, read, not run. refusals declares STRATEGIC_ACCOUNT and OPEN_OPPORTUNITIES, both of which match HOOK_REFUSAL_CODE_PATTERN. Each message is non-empty plain text with no {{ }} hole. OPEN_OPPORTUNITIES takes status 409, inside 400 to 499, and STRATEGIC_ACCOUNT omits status, so it is answered 400. The hook has a js body, so hookRefusalsWithoutSandboxedBody does not fire. hookRefusalCodesThrownIn reads both literal code: properties inside their throw statements, and both are declared, so refuseUndeclaredLiteralRefusalCode raises nothing. Note that check:skill-examples is a tsc pass and does not run this zod refinement, so the parse is a reading. The previous record's line that put the parse in that family was loose; nothing here depends on it.
  • Public surface: the published @objectstack/skills catalog changes in teaching only. Right.

② Semver level

  • .changeset/33257-skills-hook-refuses-by-declared-code.md still reads '@objectstack/skills': patch, Clause-②: no and Metadata-protocol: none. The delta adds one sentence to its body naming the delete-guard change (crm_protect_strategic_accounts declares and throws the two codes, and the second message drops the count). That sentence matches the diff. No accept set narrows or widens, and no runtime behaviour moves. Right.
  • The body's closing claim, "Both files pay for the new lines by deleting restatements in the same file", still holds for the delta's two cuts. See ④.

③ Boundary flags

  • The previous record's ESCALATED deviation 1, the widened surface to rules/validation.md, is ANSWERED. Card comment 6885297883 now records the approval on the card itself and names the widened surface: the pointer sentence, the two example lines and the deleted "Date range validation" line. The PR's diff stays inside that surface, and the delta does not touch validation.md.
  • The delta stays inside the claimed surface: data-hooks.md and the changeset.
  • The FOLLOW-UP from the previous record, the scenario grader reading error.code and not declaredCode behind the MCP demotion, is unchanged by this delta and is not re-judged. The one main commit touching scenario-run-grade.ts since the merge base (d7f76bff, fix(scenarios): the run grader reads a write withheld in a card's own verb ("I didn't lend out AST-0041") as a decline #33453) widens the decline-verb list and does not touch refusalOf.
  • open_questions: none.

④ Design judgment

skills/objectstack-data/references/data-hooks.md, the delta judged against main:

  • Common Patterns 1, "Preventing Deletion": the sandboxed beforeDelete guard now declares refusals and throws Object.assign(new Error(), { code }) for each rule. This is the declared shape the section opener teaches.
    • The body runner answers it on every event, because resolveHookRefusal sits in the one hook-body catch with no event filter.
    • Each refusal is answered with its catalogue message, translated through hooks.HOOK.refusals.CODE.message, at 400 or 409. The wire code is the status's (VALIDATION_ERROR or RESOURCE_CONFLICT), and declaredCode carries the hook's code.
    • The second message no longer carries the open count, which is right: a catalogue message is plain text, and declaredRefusalMessage replaces it whole with no parameters.
    • new Error() gives the inner sentence Error, which sandboxBusinessMessage reads as a business throw, not a crash. That is the same reading the previous record made for the section example.
  • Declared refusals opener: "Without one, a body's throw is answered HOOK_ABORTED". The body's scope removes the handler case, because a handler's bare throw is a 500. See Delta N1 for what remains.
  • Error Handling intro: "neither gives the hook's own code" removes the contradiction with HOOK_ABORTED being a code. See Delta N2 and finding N5.
  • Cut 1, the paragraph "Because the VM only holds a snapshot, mutating ctx.previous / ctx.result is local and thrown away; the only way to change the persisted write is ctx.input.x = … or return { … }". No taught fact is lost. The same file still says the sandbox gets "a JSON snapshot … not live engine objects" (the ctx table lead-in). It says a before* hook changes the write "by assigning ctx.input.x = … or return { x: … }" (the body bullets). And Common Patterns 4 says a body "writes back only ctx.input, and its ctx.result is a snapshot". The paragraph restated those three.
  • Cut 2, "(declared, never produced)" in the ctx.session row. This was history only. The operative fact, "No role list — session.roles was retired in 17.0.0", stays. Common Patterns 4 still warns that ctx.session?.roles?.includes(…) is always undefined.
  • Evals: no eval in skills/objectstack-data/evals becomes false. Neither eval file names the snapshot, mutating ctx.previous or ctx.result, session.roles, or the delete guard. The CREDIT_LIMIT refusals eval still has every fact it needs in the file: the catalogue, a code-only throw, a js body, the translation key path and the save-time refusal without a js body. So does the tax-ID hook eval: a body with api.read that throws when a blocked row is found.
  • rules/validation.md: unchanged since the previous head. The previous judgment stands.

New findings:

  • N5 — nit — data-hooks.md, Error Handling intro and Declared refusals opener, read together. On main, a body throw that carries its own code with no catalogue still reaches the wire with the status's code and the thrown code as declaredCode. The runner copies code onto SandboxError, resolveHookRefusal returns the error untouched when there is no catalogue, and resolveThrownHttpError keeps the spelled code. So "neither gives the hook's own code" in the intro and "a body's throw is answered HOOK_ABORTED" in the opener are both looser than the code. The intro wording is the one the previous record suggested, so the looseness is the reviewer's too. The error points the safe way, because it pushes the author toward the catalogue, which is the contract and the only path with a translated message and a declared status. It is a nit and does not reopen the round. A tighter wording, if the tokens allow: "a body's code-less throw" in the opener.
  • N6 — nit — PR docs(skills): a hook that enforces a business rule declares refusals and refuses by code #33488 body. The body still describes head 47af59e9: "+47 / -29", "Head 47af59e9", line and token counts of 893 lines and 10009 tokens, and payments that do not list the two new cuts. Its What changes section does not mention the delete-guard rewrite. The changeset, which is what publishes, is current. The Tier H approver reads the PR body, so the seat may refresh it. This is not contract-bearing.

Delta

  • S1 (should-fix) — FIXED. The Common Patterns delete guard declares STRATEGIC_ACCOUNT (400) and OPEN_OPPORTUNITIES (409) and throws both by code. The count left the message, as the previous record asked. It parses against HookSchema by reading (see ①). It is paid for inside the same file with no ceiling moved, and Type Check · source gates concluded success.
  • N1 (nit) — PARTLY FIXED. "a body's throw" now excludes the handler hook's 500. A body throw that carries its own code with no catalogue is still not HOOK_ABORTED. That remainder is carried as N5, a nit.
  • N2 (nit) — FIXED as the previous record worded it. "neither gives the hook's own code" no longer contradicts the HOOK_ABORTED sentence. The same wording shares N1's remainder, folded into N5.
  • N3 (nit) — NOT FIXED, accepted. checkout_requires_stock still binds beforeInsert only. The previous record allowed leaving the minimal example as it is.
  • N4 (nit) — NOT FIXED, accepted. validation.md is untouched. The previous record graded the deleted "Date range validation" line an accepted deviation.
  • New: N5 and N6, both nits. Nits alone end the round.

Implemented-by: claude/project-thread-g605co
Reviewed-by: session_01FEkQ5pjSPT7ui2EUcunaVb

VERDICT: PASS

Adopted by the seat 2026-11-14T23:05Z · isolated sub-agent at CONTRACT_REVIEW_TIER · Ruling-ref: Leehom card cmsg_01TenzsRopZj4jMbvjX8uz1XFhuiZmTH8VFgMmHRmpiGAb, option A, tapped 2026-11-14T15:37Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:skills#1 · os-project-manager · session_01FEkQ5pjSPT7ui2EUcunaVb

维护者速读

  • 改了什么:objectstack-data 技能的 references/data-hooks.md 教钩子用声明的 refusals 按代码拒绝(只有 js 的 body 能声明;线上 code 是状态码对应的平台码,钩子自己的码放在 declaredCode);两个新例子和“阻止删除”范例都改成声明码再抛出。rules/validation.md 同步换掉两行旧例子。外加 patch changeset。
  • 为什么改:技能还在教抛自由文本,线上一律回 HOOK_ABORTED,调用方分不清原因(卡 skills (data-hooks): a hook that enforces a business rule declares refusals and refuses by code, not a bare onError abort (HOOK_ABORTED) #33257)。
  • 风险与回滚:只改技能文字,不动 schema 与运行时;token 在原文件内付清,上限未动。回滚即还原本 PR。
  • 先看:data-hooks.md 的 Declared refusals 一节和 Common Patterns 第 1 例。
  • 两分钟验证:git diff origin/main -- skills/。
  • 需要你做的:受管面 Tier H(skills/**),契约复审 PASS 评论 6885289276(含 1 条应修)与增量复审 PASS 评论 6885627321(Head-sha 652a4a09d7861fd8b9ffcb8c1014d731d60d5663)已在案,请批准或直接合并。
  • 席位意见:可以合。首轮应修(同文件“阻止删除”范例还在教旧写法)已改,开发线程在真实运行时探测了 400/409 两个码。复审顺带发现场景评分器读不到 declaredCode,已另立卡 qa(scenarios): refusalMeets never sees a hook's declared refusal; the MCP door serves it as declaredCode beside the status's catalogue code #33497。

席位核对

Verdict: ACCEPT, held for the maintainer · head 652a4a09d7861fd8b9ffcb8c1014d731d60d5663 · card #33257 · report 6885205230 · contract review 6885289276 (PASS with one should-fix on 47af59e9) and delta 6885627321 (PASS on this head, CONTRACT_REVIEW_TIER; Ruling-ref for the in-seat review: Leehom card cmsg_01TenzsRopZj4jMbvjX8uz1XFhuiZmTH8VFgMmHRmpiGAb option A).

  • Shape: draft → main, Fixes #33257. skills/objectstack-data/references/data-hooks.md, skills/objectstack-data/rules/validation.md (surface widening approved by the seat, recorded on the card in 6885297883), one '@objectstack/skills': patch changeset. GOVERNED, Tier H (skills/**). Clause-②: no; Metadata-protocol: none.
  • Contract: only a js body declares refusals, and a handler is refused at save; the wire code is the status's (default 400) with the hook's code as declaredCode; the delete-guard block parses against HookSchema on main (codes match the pattern, no {{ }} in a catalogue message, both thrown codes declared).
  • Token payments: data-hooks.md 9962 → 10004 at ceiling 10011; validation.md 2760 / 2761. No ceiling moved. The two cuts lose no taught fact, and no eval becomes false.
  • Gates: 45 derived, 44 exit 0, 1 NOT MEASURED (check:dual-build-cjs-loads). Check-runs on the head all concluded, none failing (Type Check · consumer gates type-checks every os:check block).
  • Accepted deviations (review nits): a body throw that carries its own code with no catalogue reaches the wire with declaredCode, so "a body's throw is answered HOOK_ABORTED" and "neither gives the hook's own code" say slightly more than the code (the error points the safe way, toward declaring); checkout_requires_stock guards beforeInsert only; the deleted "Date range validation" line was not an exact restatement of the remaining date_range example.
  • Found outside the diff: the scenario grader never sees a hook's declared refusal, because the MCP door serves it as declaredCode. Filed as qa(scenarios): refusalMeets never sees a hook's declared refusal; the MCP door serves it as declaredCode beside the status's catalogue code #33497.
  • Process: the thread ran at the default tier, met by these in-seat reviews.
  • Landing: Tier H. The seat does not flip this PR ready or enqueue it. It needs the maintainer's merge, or an authorised APPROVED review after which the seat lands it.

@objectstack-fleet objectstack-fleet Bot added the pm:awaiting-merge PR only: landing waits on the maintainer's own review and merge (SKILL.md 复核 ②); never on an issue label Nov 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation pm:awaiting-merge PR only: landing waits on the maintainer's own review and merge (SKILL.md 复核 ②); never on an issue size/s tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skills (data-hooks): a hook that enforces a business rule declares refusals and refuses by code, not a bare onError abort (HOOK_ABORTED)

2 participants