Skip to content

lint: reach handler-authored hooks with the hook write-set rules, and record which intakes reach them - #16564

Merged
hotlong merged 4 commits into
mainfrom
claude/issue-16095-hook-rules-reach-handler-authored-hooks
Sep 7, 2026
Merged

lint: reach handler-authored hooks with the hook write-set rules, and record which intakes reach them#16564
hotlong merged 4 commits into
mainfrom
claude/issue-16095-hook-rules-reach-handler-authored-hooks

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #16095

Clause-②: yes

Every rule in the hook-body-* / hook-api-update-readonly-* family opens on body.language === 'js'. A hook authored as an inline handler function carries no body, so on the stack os lint handed the rule registry, the whole family returned before reading anything. os build never had that gap: it runs lowerCallables before its Zod parse and judges the lowered stack. lintConfig now hands the registry's parsed tier that same lowered view, so the pre-flight refuses what the build would refuse.

Nothing here changes what os build accepts, and validate-action-body-writes.ts is untouched.

Read this first: the review tier this PR is missing

The mandated contract-review tier was measurably unavailable for this card, so it ran one step down under the documented quota-exhaustion exemption. The needs:contract-review label is the compensation, not a formality: this PR changes what os lint refuses, and the severity/rollout decision behind that belongs to a reviewer running at the proper tier. It should not be flipped ready, queued or merged while the label stands. Everything below is written for a reviewer who was not here.

Premise check against origin/main

The card's premise holds, with two corrections a reviewer should not re-derive:

  • The door line numbers in the card and the triage comment are stale. Card/triage say 740 / 254 / 294; on origin/main the three doors are validate-hook-body-writes.ts:729, validate-readonly-hook-writes.ts:291, validate-action-body-writes.ts:283. Re-read and confirmed.
  • The population figures are not reproducible here. The card's 39-of-39 handler-authored hooks were measured on the reference CRM's pinned @objectstack/lint@17.3.0, which is outside this repository. Re-derived locally instead, and the local shape is different — see the numbers below. The reference app's ratio is not verified by this PR and should not be read as if it were.

Which intakes reach the family, enumerated with a control on every leg

The doors are the call sites of runAuthoringRules, and there are more of them than there are commands. A reachability claim measured through one entry point is a claim about that entry point, so every leg below carries the body-authored control — the identical statement written as an explicit body — which must fire wherever the handler leg is silent for the silence to mean "door", not "rule".

intake lowers before parse handler-authored body-authored control verdict
os build, union run yes judged fires reached, always was
os build, per-package run yes judged fires reached, always was
os lint now yes judged fires reached as of this PR
runScaffoldAuthoringRules (os init / dev) yes 1 finding 1 finding reached, always was — newly pinned
os validate no 0 findings fires not reached
direct library call caller's choice 0 unlowered, 1 lowered fires caller-dependent

Two of those rows are the reason this PR has a second commit. The reach annotation first landed naming three os * commands; os build actually enters twice, and runScaffoldAuthoringRules is a fourth intake that lowers before it parses and has therefore reached this family all along. A ledger that omits a reached door mis-states coverage in the same direction the card is about, so both rule headers now enumerate every measured intake and the scaffold door has a pin with its control beside it.

The os validate row is a measurement, not a decision. Closing it would change what os validate refuses, which is its own call and is not made here.

Finding delta — the blast-radius measurement

Measured over this repository's own four example apps (examples/app-crm, app-showcase, app-todo, app-multi-package) by loading each real config through the real os lint path, once with this change and once with the call site reverted to its pre-change form:

findings warning suggestion error
before 121 93 28 0
after 121 93 28 0

Row for row identical. No finding added, none removed, no severity moved. No configuration that passes today starts failing, so the error/exit-1 escalation that would have made severity a maintainer decision does not occur on this corpus.

Local population, re-derived: 6 hooks, of which 2 are handler-authored (app-crm 1, app-todo 1) and 4 are body-authored (app-showcase). The parsed tier previously saw 4 of those 6 carrying a js body; it now sees 6. So the reach genuinely moved by 2 hooks and still produced 0 new findings. Separately, 125 of 157 actions are body-authored, consistent with the triage note that the action-body door reaches its intake.

The zero is a real zero, not a dead instrument. Positive control, run on the real app-crm configuration with only the hook's handler body swapped for a write to that app's real readonly: true field crm_opportunity.approval_status:

  • pre-change code: 0 family findings — blind
  • this PR: 1 finding, hook-api-update-readonly-field, at error
  • the shipped hook, unmodified: 0 on both sides

The two real handler-authored hooks mutate ctx.input.data directly and never call ctx.api, which is the honest reason the family has nothing to say about them.

Scope limit a reviewer should weigh. This delta is zero on this repository. The reference CRM is out of this session's scope and its 39-hook population could not be measured here, so the real-world blast radius of the error-severity member is not established by this PR. If the severity or a warning-first rollout is in question, that decision needs the reference app's numbers, which this PR does not supply.

Evidence

  • Red-first. The 16 new tests: all 16 pass on this branch; with the call site reverted, 4 fail and the 12 controls stay green. The controls holding in both legs is what makes the 4 readable as reach rather than as rule changes. The revert was applied and undone as a scripted mutation with the on-disk change proven by anchor counts before and after, and the restore proven byte-identical to HEAD by blob hash, not by an exit code.
  • pnpm --filter @objectstack/cli exec vitest run --project unit — 182 files, 2477 passed, 6 expected-fail.
  • pnpm --filter @objectstack/cli exec vitest run --project integration — 32 files, 284 passed. Run locally because this PR adds an integration-tier file (the e2e door suite spawns the CLI).
  • pnpm --filter @objectstack/lint test — 100 files, 3486 passed.
  • pnpm --filter @objectstack/lint --filter @objectstack/cli run typecheck — clean, test layer included, no new ledger debt.
  • pnpm lint (whole repo, eslint . --no-inline-config) — clean. Run in full, so no narrowing argument is needed.
  • Gates: all 86 derived families run and reconcilednode scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, then --ran, which reports 86 derived famil(ies) accounted for — 86 run, 0 NOT-MEASURED. One family, check:type-check-debt, first returned exit 3 PREREQUISITE NOT MET after tsc hit an OOM under a 4096 MB ceiling; that is the not-measured class rather than a finding, and it is green on re-run with a raised heap.
  • All figures above were taken at 33ade701be, after the final merge of origin/main.

Deliberately not done

  • os build acceptance is untouched. Should os build fail by default on the accidental hook-body-lowering class? — deferred until the new lint rule has produced a real population number #13838 is not addressed here and stays open; it is waiting on a population number, and the local figures above are offered to it rather than merged into it.
  • validate-action-body-writes.ts is untouched. Its identical-looking door reaches its intake, so it is not a defect and was not "fixed" in passing.
  • os validate is left not reaching the family. Recorded as a measurement in both rule headers; changing it is a separate decision.
  • No assertion was weakened or deleted. The second commit only adds a pin and extends two header ledgers.
  • The annotation fallback the triage comment mandates is satisfied either way: all five ids carry their scope in the two rule headers, so a rule that cannot see an authoring shape does not read as coverage.

Changeset

.changeset/lint-hook-rules-reach-handler-authored-hooks.md@objectstack/cli minor (rule reachability changes), @objectstack/lint patch (header ledger only, behaviour unchanged). No skip-changeset: this publishes from a package.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8


Generated by Claude Code

… rules reach handler-authored hooks

`os lint` judged the un-lowered normalized stack, so every rule in the
`hook-body-*` / `hook-api-update-readonly-*` family returned before reading a
hook authored as an inline `handler` function; `os build` lowers first and
never had the gap. `lintConfig` now runs `lowerCallables` on the same input
and hands the lowered view to the `parsed` tier only, leaving the
function-reading rules and the caller's stack untouched.

Doors measured with a control per leg (lint / build / validate), the two
validators carry the reach ledger in their headers, docs and changeset added.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
…n the scaffold door

The reach annotation landed naming three `os *` commands. The doors are the
call sites of `runAuthoringRules`, and there are more of them than there are
commands: `os build` enters twice (the union run and the per-package run) and
`runScaffoldAuthoringRules` — which `os init` / `dev` drive over a rendered
template — is a fourth intake that lowers before it parses and has therefore
reached this family all along. A ledger that omits a reached door mis-states
coverage in the same direction the card is about.

Both rule headers now enumerate every measured intake, each recorded with the
body-authored control that makes its verdict readable, and the scaffold door
gains a pin with that control beside it. The pin asserts `schemaError` is null
before reading the verdict, so a stack that stops parsing cannot read as "no
finding". No rule logic changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cli, @objectstack/lint, touching 3 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/lint/src/validate-hook-body-writes.ts, packages/lint/src/validate-readonly-hook-writes.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

20 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 554a16037eedb448a9ddc6d70434dc45208bcfca.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/lint/src/validate-hook-body-writes.ts, packages/lint/src/validate-readonly-hook-writes.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 25 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 554a16037eedb448a9ddc6d70434dc45208bcfcapackageMentionDocs.

Which tree this was computed on

This run read content/docs from 7f687078cb6143025a12c82c1c02d88eb53b406a — the merge of head e4e0a8ef488d106a79d73aeaa9a064d922ece9a3 into base 554a16037eedb448a9ddc6d70434dc45208bcfca, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7f687078cb6143025a12c82c1c02d88eb53b406a && git checkout 7f687078cb6143025a12c82c1c02d88eb53b406a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 554a16037eedb448a9ddc6d70434dc45208bcfca e4e0a8ef488d106a79d73aeaa9a064d922ece9a3 && git checkout -B drift-repro 554a16037eedb448a9ddc6d70434dc45208bcfca && git merge --no-ff e4e0a8ef488d106a79d73aeaa9a064d922ece9a3

node scripts/docs-audit/affected-docs.mjs --json 554a16037eedb448a9ddc6d70434dc45208bcfca

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 554a16037eedb448a9ddc6d70434dc45208bcfca → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…claration (#16095)

The Check Changeset gate refuses patch on a package whose src moved under a
clause-② declaration (maintainer ruling 2026-09-04, batch #35, #15294); ruled
by the contract review on #16095 (comment 5571527379).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XesLUWmuhjuRwmU618AZ1M

hotlong commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Contract review PASS at CONTRACT_REVIEW_TIER#16095 comment 5571527379 (director seat, summon #17, session_01XesLUWmuhjuRwmU618AZ1M); its two mechanical conditions landed on head e4e0a8ef (@objectstack/lint graded minor, Clause-②: yes line in this body — dev report 5571633964). needs:contract-review stripped from this PR in the same stroke (the card never carried it), read back. Landing pre-check: NOT governed (0 of 8 paths); all 37 check runs on e4e0a8ef success/skipped, Check Changeset green ⇒ flipping ready and arming auto-merge now. One cosmetic residue left for the record: the body's ## Changeset paragraph still says @objectstack/lint patch in prose; the frontmatter (what the gate and the release read) says minor.


Generated by Claude Code

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 size/l tests tooling

Projects

None yet

2 participants