fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record<string, unknown>): Promise<{ id: string }>;
update(object: string, id: string, data: Record<string, unknown>): Promise<void>;
delete(object: string, id: string): Promise<void>;
find(object: string, query: Record<string, unknown>): Promise<Array<Record<string, unknown>>>;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise<Array<Record<string, unknown>>>;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) - #15118

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter
Sep 4, 2026
Merged

fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175)#15118
hotlong merged 2 commits into
mainfrom
claude/issue-14175-action-engine-facade-find-filter

Conversation

@claude

@claudeclaudeBot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#14175

Clause ②: yes — path limb: packages/spec/src/** (ui/action-params.zod.ts is a Clause ② SUSPECT surface per dispatch-gates --tier, no path-derived mandate); content limb yes: a published type narrows — ActionEngineFacade.find's second parameter moves from an open string-keyed record to FilterCondition, so what a caller may pass changes at compile time. needs:contract-review hung on this PR and on #14175 in the same stroke. Draft; not to be flipped ready by this seat.

Remedy 1 of the card only (type the parameter, document it). Remedy 2 (the runtime throwing on an envelope) is the domain:cli lane's and is not taken; remedy 3 (the facade as a shipped test double) is a card of its own and is not taken.

What changed

  • packages/spec/src/ui/action-params.zod.tsActionEngineFacade.find(object, query) with an open record becomes find(object, filter: FilterCondition), the published QueryAST.where type (data/filter.zod.ts), renamed queryfilter. The member's doc comment states: it is a FILTER (the where half), not an ObjectQL envelope; the runtime — buildActionEngineFacade's find arm, packages/runtime/src/action-execution.ts:1183-1187 on 369da918, read-only — wraps a non-empty filter as { where: filter } and passes an EMPTY filter ({}) through unwrapped (the unfiltered read); an envelope becomes { where: { where: … } }, matches nothing and returns [] without error; and exactly what the type refuses and does not refuse. The facade docblock gains one sentence pointing at it. insert / update / delete and ActionHandlerContext untouched.
  • packages/spec/src/ui/action-params.test.ts — four pins, in the compiled test program (tsc -p tsconfig.test.json --listFiles lists the file): the exported type-level pin FindFilterIsFilterCondition (strict Eq between the declared second slot of find, read via Parameters, and FilterCondition; measured: the same assertion against the old open record is a TS2344); a positive control (implicit equality, operators, $and / $or / $not, and the empty filter); a compile-time refusal pin under @ts-expect-error for what the type refuses; and a MEASURED-GAP pin recording that the exact envelope mistake still compiles (below).
  • content/docs/ui/actions.mdx — a callout under the registered-handler example carrying the filter-not-envelope sentence and both limbs. It is the only hand-written page showing the facade; git grep -n "engine.find" origin/main -- content/docs hits are DataEngine / ObjectQL engine.find (envelope-taking), not ctx.engine, and were left alone.
  • .changeset/action-engine-facade-find-filter.md@objectstack/specpatch. Precedent: [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 (.changeset/action-param-excess-keys-compile.md, a compile-layer-only narrowing "shipped as patch because no working code changes meaning"); the ActionHandlerContext.session narrowing in 17.0.0 rode a minor changeset only because it landed together with the new ActionSessionSchema. Non-breaking, so no ADR-0087 marker is owed (check-adr-0087-registration agrees). The contract review judges the level.

Measurements the card asked for

FilterCondition IS the shape the runtime's where accepts.QuerySchema.where is FilterConditionSchema.optional() (data/query.zod.ts:522); EngineQueryOptionsSchema.where — what ql.find(object, options) takes — is a union of an open string-keyed record and FilterConditionSchema (data/data-engine.zod.ts:98), so FilterCondition is the named member of that union and the narrowest published filter type the slot admits; the facade's old open record was the other member. It admits every shape handlers legitimately pass (field-keyed equality, operators, $and / $or / $not, {}) — the positive control compiles.

⚠️ The compile-time bar is PARTIAL — FilterCondition admits where as a key. Measured with a scratch tsc program before the edit: FilterCondition's string index signature admits any per key, so { where: { position_code: 'qa_lead' } } — the exact mistake — compiles, and so does the nested { where: { where: … } }. What the type DOES refuse: a primitive (TS2322), $and / $or that are not arrays ({ $and: 'active' }, { $or: { active: true } }), and $not that is not a filter ({ $not: 'archived' }) — the three logical-operator cases and the primitive are the @ts-expect-error pins (an array under $not is admitted and is not pinned). So the reporting app's mistake becomes a documented mistake, not a compile error; the doc comment is the contract of record, and the MEASURED-GAP pin records the admission so that a later narrowing updates the sentence together with the type. Not widened to force the bar (per the dispatch); the where-refusing intersection that would close it is raised below for the contract review.

Before / after (angle brackets spelled as entities so the body sanitizer leaves them alone)

Before (369da918):

/**
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
*/
export interface ActionEngineFacade {
insert(object: string, data: Record&lt;string, unknown&gt;): Promise&lt;{ id: string }&gt;;
update(object: string, id: string, data: Record&lt;string, unknown&gt;): Promise&lt;void&gt;;
delete(object: string, id: string): Promise&lt;void&gt;;
find(object: string, query: Record&lt;string, unknown&gt;): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

After (this PR; the member doc comment abbreviated to its load-bearing sentences — the file carries the full text):

/**
* … (unchanged three lines) …
*
* `find` is the one member whose argument shape the signature alone never
* settled: it takes a bare FILTER — the `where` half of a query — and never
* an ObjectQL query envelope; read its doc comment before writing a handler
* or a test double against it (#14175).
*/
export interface ActionEngineFacade {
insert(…): … (unchanged)
update(…): … (unchanged)
delete(…): … (unchanged)
/**
* Read the rows of `object` that match `filter`.
*
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
* {@link FilterCondition} that `QueryAST.where` carries … It is NOT the
* query ENVELOPE (`{ where, fields, orderBy, limit }`) … The runtime builds
* the envelope itself: `buildActionEngineFacade`'s `find` arm
* (`packages/runtime/src/action-execution.ts`, `:1183` on `369da918`) wraps
* a non-empty filter as `{ where: filter }` and passes an EMPTY filter
* (`{}`) through unwrapped — the unfiltered read.
*
* Two consequences, both silent (#14175): [envelope → `{ where: { where: … } }` → `[]`,
* no error; `{}` skips the wrap, so a mixed handler looks partially alive]
*
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
* mistyped logical operator … It does NOT refuse `{ where: … }` … so the
* envelope mistake still compiles, and this doc comment, not the type, is
* the contract of record. Both halves are pinned in `action-params.test.ts`.
*/
find(object: string, filter: FilterCondition): Promise&lt;Array&lt;Record&lt;string, unknown&gt;&gt;&gt;;
}

Consumer direction (READ and typecheck only — no edits outside packages/spec)

  • Importers of ActionEngineFacade / ActionHandlerContext / ActionHandler outside packages/spec (excluding dist, CHANGELOGs, docs): zero by git grep. packages/runtime names the facade only in comments, and buildActionEngineFacade is declared (…): any around an unannotated object literal, so the narrowed member cannot reach it; packages/objectql mentions ActionHandler in one comment.
  • After pnpm --filter @objectstack/spec build: pnpm turbo run typecheck --filter=@objectstack/runtime --filter=@objectstack/objectql --concurrency=2 — the prefix direction, i.e. these two CONSUMERS — 31 tasks successful (29 cached closure builds + the 2 typechecks), exit 0. The reroute condition (the runtime's facade construction no longer compiling) did not fire.
  • Where a handler-side copy of the facade lives — examples/app-todo/src/actions/task.handlers.ts declares its own ActionContext with the old query spelling and an array-taking delete — is out of scope here and filed as ActionEngineFacade.delete declares id: string while the runtime facade accepts string | string[] and examples/app-todo relies on the array form through a hand-rolled context type #15117 (the neighbouring delete member's declared-vs-produced gap; find's copy is mentioned there).

Reverse verification (one leg, from the committed state 81e79ab3)

Prediction written before the run: the type-level pin turns red (TS2344); the three logical-operator refusal pins turn red (TS2578 — the directive goes unused because the open record admits them); the primitive refusal, the positive control and the measured-gap pin stay green. Mutation: filter: FilterCondition reverted to the open record plus the then-dead import type removed, in action-params.zod.ts; confirmed on disk by anchored counts (removed text 0, injected text 1, import line 0) and blob f6cd8405 vs HEAD dd75f9a9. tsc -p tsconfig.test.json: 265 errors against the 261-error ledger baseline — exactly action-params.test.ts(416,51): TS2344 and TS2578 at 446 / 448 / 450; nothing else moved. No build leg was owed: the test program compiles the pin against src through a relative import, not through dist. Restore: git checkout HEAD -- the absolute path, under an EXIT/INT/TERM trap; proven by git hash-object equal to the HEAD: blob dd75f9a9… and by an empty git diff HEAD / empty git status --porcelain. Side effect worth knowing: the restore refreshed the file's mtime, so the dist-reading spec gates reported "dist OLDER than src" until one more pnpm --filter @objectstack/spec build; the readings below are from after it.

Verification record — head 81e79ab3 (each exit captured before any pipe; verdict lines quoted from the gates)

  • pnpm --filter @objectstack/spec build through scripts/pm/os-verify-lock.shVERDICT command-exit 0, check-dts-emitted: 34/34 declared declaration file(s) present (run before the union and again after the reverse leg).
  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/ui/action-params.test.tsTest Files 1 passed (1) · Tests 29 passed (29) (25 existing + 4 new), exit 0.
  • pnpm --filter @objectstack/spec typecheck (tsc --noEmit + check:scripts-typecheck + check:test-typecheck) — exit 0; check:test-typecheck: OK — 54 file(s) / 261 error(s) / 145 pinned signature(s) — the shrink-only ledger is unchanged, so the new test contributes zero errors; --listFiles lists src/ui/action-params.test.ts and src/ui/action-params.zod.ts.
  • pnpm --filter @objectstack/spec check:generated✓ All 15 generated artifacts are up to date. (nothing regenerated: the interface member's docblock does not render on references/ui/action-params.mdx, and api-surface/ records that ActionEngineFacade exists, not its member signatures; check:react-declaration-parity "cannot run here" as always).
  • check:api-surfacepublic API surface + factory signatures unchanged ✓; check:docs229 generated files in sync with packages/spec; check:strictness-ledgerdocs/audits/2026-07-unknown-key-strictness-ledger.counts.md is current — 439 site(s) measured (no new object site, nothing to update); check:export-origins5242 exports across 17 entry points resolve exactly as recorded.
  • eslint --no-inline-config over the two edited TypeScript files — exit 0, no output.
  • Census: node scripts/check-system-context-census.mjs (+ --self-test) — OK — 106 elevation read sites in 20 packages across 45 files, all anchored; 140 anchors resolve; nothing in action-params.zod.ts is anchored, so no --fix was owed.
  • Consumer-direction typecheck — above.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set derived from the merge base) at 81e79ab3 → 89 commands, all run; every one exit 0 except the following, which are NOT MEASURED by the gates' own text (exit 3, PREREQUISITE NOT MET), not red: scripts/check-test-completeness.mjs (grades a saved turbo run test log; none exists locally), pnpm check:dual-build-cjs-loads (38 packages have no dist/), pnpm check:type-check-debt (--re-measure needs 4 unbuilt workspace deps of the ledgered packages; none of the ledgered packages imports the narrowed type). check:skill-examples first reported the client-SDK surface unmeasured (no client-react dist); after turbo run build --filter=@objectstack/client-react --filter=@objectstack/client it reads ✅ 257 prose examples type-check across 3 surface(s), exit 0. pnpm lint (repo-wide eslint) is CI's run; the local reading is the two-file eslint above.
  • Control-byte scan (grep -naP over the four edited files) clean; pnpm check:nul-bytesOK (scanned 8224 text file(s) … no raw ASCII control bytes).

For the contract review

  1. Close the compile-time bar fully? Typing the slot as FilterCondition intersected with { where?: never } would make the exact { where: … } mistake a TS2322 at the call site (measured shape: any intersected with never is never). where is not a reserved field name anywhere in packages/spec today (grep), so this is a new vocabulary claim ("no object has a field named where") rather than a re-statement of an existing one — which is why it is raised here and not shipped. Recommendation: land this PR as is (the dispatch's shape), and decide the intersection as its own small follow-up if the review wants the full bar.
  2. Changeset level: patch per the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent; minor if the review reads a parameter-type narrowing on a published interface as a public-surface move regardless of behaviour.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/ui/actions.mdx(via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 128 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 97a22639b44ae04693c9999f13d8a9aa985ed810packageMentionDocs.

Which tree this was computed on

This run read content/docs from 073756227d292a523b4c5547e38305b9b05562fa — the merge of head 81e79ab343a6b3dbd78e14c483d929af8724564d into base 97a22639b44ae04693c9999f13d8a9aa985ed810, 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 073756227d292a523b4c5547e38305b9b05562fa && git checkout 073756227d292a523b4c5547e38305b9b05562fa
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97a22639b44ae04693c9999f13d8a9aa985ed810 81e79ab343a6b3dbd78e14c483d929af8724564d && git checkout -B drift-repro 97a22639b44ae04693c9999f13d8a9aa985ed810 && git merge --no-ff 81e79ab343a6b3dbd78e14c483d929af8724564d
node scripts/docs-audit/affected-docs.mjs --json 97a22639b44ae04693c9999f13d8a9aa985ed810

⚠️ 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 97a22639b44ae04693c9999f13d8a9aa985ed810 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Contract review (Clause ②) — PASS · ACCEPT

Reviewer of record: the domain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Tier fuse: get_session read at 2026-09-04T01:28Z — both model fields equal CONTRACT_REVIEW_TIER. Gate: check-clause2-carriers.mjs --pair 15118 exit 0 at 01:29Z (the Clause-②: declaration is in the dev's own claim comment 5533620652; both carriers hung). Report comment 5534017289 (00:41Z) read against GitHub and origin/main, not against its own prose.

Head and window (readings at 2026-09-04T01:32Z)

  • Report head 81e79ab3 = PR head.sha. Trial merge onto origin/mainf594e70d: clean. Governed test: 0 of 4 changed files. No content/docs/releases/ edits.
  • CI on 81e79ab3: 38 checks — 33 success, 5 skipped, 0 red, 0 running. All-green: the landing window opens in this stroke.

The narrowing, verified in the diff

  • What changes:ActionEngineFacade.find(object, query: Record<string, unknown>)find(object, filter: FilterCondition) (packages/spec/src/ui/action-params.zod.ts), a type-only import of FilterCondition from data/filter.zod.ts, a doc comment stating filter-not-envelope, the runtime wrap (cited by file, line and commit: action-execution.ts:1183 on 369da918), the silent-[] consequence and the {}-unwrapped limb; one sentence on the facade docblock. insert / update / delete and ActionHandlerContext untouched. Remedy 1 only, as dispatched.
  • What the type refuses / admits, pinned in action-params.test.ts: the type-level Eq pin reads the slot off the interface (Parameters<ActionEngineFacade['find']>[1]), so a re-widening reds in the compiled test program; @ts-expect-error pins for a primitive and mistyped $and / $or / $not; positive controls incl. {}; and a MEASURED-GAP pin recording that { where: … } still compiles (the string index signature). The partial bar is stated in the doc comment, the changeset and the docs callout — not hidden.
  • Consumers on origin/mainf594e70d (01:31Z):ActionEngineFacade has zero importers outside packages/spec (hits are a generated reference page, ADR-0096 prose, a test docblock, a changelog line); the runtime constructs the facade untyped. examples/app-todo/src/actions/task.handlers.ts:84 / :113 already pass bare filters ({ status: 'completed' }, {}); the { where: { id } } call sites in examples/** tests are the three-argument ObjectQL engine, not the facade. Consumer-direction typecheck (@objectstack/runtime + @objectstack/objectql, prefix filter after building the closure): exit 0 — the dispatch's reroute condition did not fire.
  • Docs-drift advisory (bot comment on this PR): one hand-written page, content/docs/ui/actions.mdx — edited (the callout).

Report checklist

  • Reverse verification: one leg from the committed head — the slot reverted to the open record, confirmed by anchored counts and blob hash; tsc -p tsconfig.test.json 265 vs the 261-error baseline, exactly TS2344 at the type pin and TS2578 at the three logical-operator pins, nothing else; restore proven by git hash-object = HEAD blob and empty git diff HEAD / porcelain. Direction red, as predicted in writing beforehand. Accepted.
  • Gate readings at 81e79ab3: spec build (check-dts-emitted 34/34), pins 29/29, spec typecheck + check:test-typecheck ledger unchanged (a first draft's unused alias was caught by that ledger and fixed), check:generated all 15 up to date, api-surface unchanged, eslint 0, census OK (no anchor in the file), nul-bytes OK; dispatch-gates --commands 89 run, NOT MEASURED by the gates' own words: test-completeness (saved log), dual-build CJS loads (no dist), type-check debt (unbuilt deps); check:skill-examples green after building client-react / client.
  • Changeset:@objectstack/spec is published, changeset present — patch, precedent .changeset/action-param-excess-keys-compile.md ([finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615, a compile-layer-only narrowing shipped as patch). No ADR-0087 marker owed (no authorable metadata shape moves).
  • Scope: 4 files, all inside the claim's declared surface; no packages/runtime edits (the wrap is cited read-only).
  • Closing keyword, two reads (01:29Z): first line Fixes #14175; no other closing keyword beside a card number (#15117 appears as "filed as"). Fixes is correct — remedy 1 is the card's whole ask as triaged.
  • MCP-call accounting: 0. Head pinning: report head = PR head; the local gate readings are live.
  • Housekeeping noted, not blocking: two wip-titled commits without trailers on the branch — the squash merge composes the landing message from the PR; the body carries the attribution.

Seat rulings on the report's open questions (open to the maintainer's veto)

  1. Close the compile-time bar fully?A — land as is. Intersecting the slot with a never-typed where key asserts a vocabulary fact the spec declares nowhere (no object may have a field named where); that is a contract claim for triage, not a rider. This seat files it as a finding at landing, citing the MEASURED-GAP pin.
  2. Changeset level:A — patch. Nothing changes in what the facade accepts or returns at runtime; an implementation typed with the old open record still satisfies the interface (method-parameter bivariance); zero importers outside spec; the [finding] tsc does not police unknown keys on plugin action-param literals — the only enforcement is the ActionParamSchema strict parse at module load #12615 precedent is the repo's own spelling for a compile-layer-only narrowing.

Out-of-scope group (sweep criterion: neighbouring facade members whose declared type differs from what the runtime facade produces or accepts)

Landing

All-green on 81e79ab3 now: provenance → ready → squash auto-merge → added_to_merge_queue in this stroke. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.

needs:contract-review is cleared on this PR and on #14175 in this stroke (read-modify-write, both read back). If the head moves after this clear, the gate's C3 reading re-hangs it and the delta is reviewed before it is cleared again.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 4, 2026 01:32
@claude

claudeBot commented Sep 4, 2026

Copy link
Copy Markdown
ContributorAuthor

Landing provenancedomain:spec PM seat, session session_0174WZTU6XcFcS7g2kykC53i (seat post #6017). Contract review (Clause ②) PASS · ACCEPT: comment 5534393939 (01:32Z); needs:contract-review cleared on both carriers 01:32Z.

Landing window readings, all on head 81e79ab3 at 2026-09-04T01:33Z:

  • CI: 38 checks — 33 success, 5 skipped, 0 red, 0 running (read at 01:29Z; the required floor Lint & Repo Gates, Type Check · workspace / · consumer gates / · debt ledger, Check Changeset, Build Core, Temporal Conformance (live PG + MySQL) all success on this head).
  • Trial merge onto origin/mainf594e70d: clean (01:32Z).
  • check-clause2-carriers.mjs --pair 15118: exit 0 at 01:29Z (head unchanged since the clear — no C3). Governed surface: 0 of 4 changed files. No content/docs/releases/ edits.
  • Closing keyword, two reads: first line Fixes #14175; no other closing keyword beside a card number in the body.

Sequence: ready → squash auto-merge → added_to_merge_queue. On MERGED: strip pm:dispatched from #14175, probe action-params.zod.ts on origin/main, landing note on the card, the where-key finding filed.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueSep 4, 2026
Merged via the queue into main with commit f794e4eSep 4, 2026
43 checks passed
@hotlong
hotlong deleted the claude/issue-14175-action-engine-facade-find-filter branch September 4, 2026 01:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@hotlong@claude