Skip to content

fix(spec)!: a flow get_record node's limit is declared a positive integer, so a fraction, 0 or a negative is refused at every door that parses a flow - #25357

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-kdrs5c
Oct 23, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-kdrs5c

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Tesla · project thread

Fixes #25302

Clause-②: no (narrowing)
Metadata-protocol: narrowed

What changes

GetRecordConfigSchema.limit (packages/spec/src/automation/builtin-node-config.zod.ts) goes from z.number() to z.number().int().positive(). The get_record executor (packages/services/service-automation/src/builtin/crud-nodes.ts) parses its config against this contract, then calls data.find with limit as written when limit > 1, and findOne otherwise. It does not go through findData, so the Data API's page-window normalizer never judges it. Before this change limit: 2.5 reached the driver as its LIMIT, and 0, -1 or 0.5 silently read one record.

The executor contract is the one judge every flow door shares (flowNodeConfigRefusals, the value half). Narrowing it refuses the value at nodes.N.config.limit with node-config-refused-by-contract at FlowSchema.parse, defineStack (STACK_SCHEMA_INVALID, 422), objectstack validate / compile, the registered flow type schema the metadata save door validates against, AutomationEngine.registerFlow, and an artifact's parse. No consumer code changes.

  • Why not reuse the PR fix(spec)!: the published list-query schemas declare the page window the Data API enforces #25300 exports. QueryPageWindowSchema is int().nonnegative(), and 0 means "the total alone" at a list door. A get_record node always reads at least one row, so 0 there is not a window. The bound is its own, and the TSDoc says why.
  • ADR-0087 disposition: not-required (already-registered flow-builtin-node-config-values-refused), amended in this diff. That D3 entry already covers "a value outside the declared set or range" for builtin node configs. Its surface, replacement and reason now name get_record limit. The reason also says the one difference: such a node used to run rather than fail. No D2 conversion, because only the author knows the count they meant.
  • Changeset: .changeset/25302-get-record-limit-positive-integer.md, @objectstack/spec minor, with FROM → TO and the one-line fix.
  • Generated: content/docs/references/automation/builtin-node-config.mdx (check:generated --fix, 1 of 14 artifacts stale: check:docs). The JSON Schema (gitignored) now reads type: integer, exclusiveMinimum: 0.

Producers measured

  • This repo at 97967a9: 70 get_record nodes in *.ts. Every limit within 14 lines of one is a positive whole number (200 ×4, 5 ×5, 50 ×2, 10 ×2). The one other value is the string '{page.size}' in canonicalize-stored-flow.test.ts, which is already refused as the wrong type and still throws /limit/. Query: git grep -n -A14 -E "type: ?['\"]get_record['\"]" piped to a grep for limit:.
  • HotCRM at 4166f21 (current main): 42 get_record nodes. 12 limits, all positive whole numbers (500 ×7, 1000 ×2, 1 ×1, and BILLING_HANDOFF_LINE_ITEM_LIMIT = 200 ×2).

Tests

At head d972d18 (the spec diff is unchanged since e92f351; the merge brought plugin-security/plugin-sharing only):

  • flow-builtin-node-config-values.test.ts: probes limit 2.5 / 0 / -1 at the judge, and a new get_record limit is a positive integer block. That block covers a fraction at FlowSchema in the contract's words; defineStack asserting code + status (STACK_SCHEMA_INVALID / 422) and the path, for 2.5, 0, -1, 0.5 and an unsafe integer; and the save-door schema. Controls: limit 1, 2 and 200 parse, and no limit parses.
  • @objectstack/spec test: 709/709 files, 21566 passed. typecheck: exit 0.
  • Consumers (filter direction: downstream readers of the schema): @objectstack/service-automation 198/198 files, 2618 tests; @objectstack/lint 153/153, 7076; @objectstack/metadata-protocol 259 passed + 3 skipped, 21416 passed. typecheck exit 0 for all three. No fixture needed triage, because no fixture carries a non-positive or fractional limit.
  • Ablation (scripts/ablation-replace.mjs, mutation int().positive() removed): the test file went 10 failed / 59 passed, which is exactly the 10 refusal cases. The controls stayed green. Restored: blob equals HEAD a900d3e7 and git diff HEAD is empty. Spec's own tests import src, so no build leg applies.
  • Dogfood: see the report comment on the card (it runs on the final head).

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 113 families for this diff. Each was run locally with its exit code captured before any pipe. 112 exited 0. pnpm check:dual-build-cjs-loads exited 3 (PREREQUISITE NOT MET: it reads the dist of about 40 packages this diff does not build), so it is NOT MEASURED locally and left to CI's Lint & Repo Gates. check:skill-examples first exited 3 (no client dist), then exited 0 after building @objectstack/client and client-react. Three gates first exited 3 on a shallow clone or a missing lint dist, then exited 0 after git fetch --unshallow and building the lint closure.

Acceptance notes

  • The executor's own form descriptor in crud-nodes.ts declares limit: { type: 'integer', title: 'Limit' } with no minimum. So a designer form built from it offers 0, which the save door now refuses with the contract's words. That is outside this card's file surface. Noted, not filed: declared ≠ enforced does not apply, because the refusal is loud at save.
  • if (limit && limit > 1) in the executor is now exact: limit is either absent or a whole number of at least 1.

Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 144 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 5f4a3f088cd7018aef904a9f31ab6b071251733c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from b1b7e98e31d1236b1d2820138bd487f5204f891f — the merge of head d972d18be0dd135ba8362c53bbeb9d4612cf5f81 into base 5f4a3f088cd7018aef904a9f31ab6b071251733c, 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 b1b7e98e31d1236b1d2820138bd487f5204f891f && git checkout b1b7e98e31d1236b1d2820138bd487f5204f891f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5f4a3f088cd7018aef904a9f31ab6b071251733c d972d18be0dd135ba8362c53bbeb9d4612cf5f81 && git checkout -B drift-repro 5f4a3f088cd7018aef904a9f31ab6b071251733c && git merge --no-ff d972d18be0dd135ba8362c53bbeb9d4612cf5f81

node scripts/docs-audit/affected-docs.mjs --json 5f4a3f088cd7018aef904a9f31ab6b071251733c

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: d972d18be0dd135ba8362c53bbeb9d4612cf5f81
Local-runs: none
Scope: packages/spec/src/automation/builtin-node-config.zod.ts, packages/spec/src/migrations/entries/semantic/18.flow-builtin-node-config-values-refused.ts + packages/spec/src/automation/flow-node-config-refusals.ts (the shared judge: flowNodeConfigRefusals, builtinValueJudged), packages/spec/src/automation/flow.zod.ts (the FlowSchema superRefine door), packages/services/service-automation/src/builtin/crud-nodes.ts (the get_record executor the rule is derived from), packages/spec/src/automation/flow-builtin-node-config-values.test.ts, .changeset/25302-get-record-limit-positive-integer.md, content/docs/references/automation/builtin-node-config.mdx. Remainder of the diff outside hits: none beyond those files (the lane seat's ACCEPT at TIER_DEFAULT covers the test, changeset and generated mdx as a standalone PR).

① Derived judgments

  1. Accept set narrowed — right. GetRecordConfigSchema.limit goes z.number().optional() → z.number().int().positive().optional(). Under zod ^4.6.5 (spec's dependency) .int() is the safe-integer check, so 2.5, 0, -1, 0.5 and Number.MAX_SAFE_INTEGER + 2 are all refused; every positive whole number and an absent limit parse unchanged. The premise holds in the cited executor: crud-nodes.ts parses against this very contract (parseNodeConfig('get_record', …, GetRecordConfigSchema, …)), then if (limit && limit > 1) hands limit to data.find as written, else findOne; nothing routes through findData, so no page-window normalizer judged it. The card's gap is real and this is the right door.
  2. Refusal reaches every door with one code and path — right. flowNodeConfigRefusals' builtin value arm keeps a present-value issue unless a carve-out applies; none does for limit (not a region slot, not a ledger value/predicate slot, not in RUN_RESOLVED_KEYS, not a value/text slot with its own judge). The seen set collapses 0.5's two zod issues (not_integer + too_small) into one node-config-refused-by-contract at limit; FlowSchema's superRefine anchors it as a custom issue at nodes.N.config.limit. defineStack (422 STACK_SCHEMA_INVALID), the registered flow metadata-type schema, registerFlow, objectstack validate/compile and artifact parse all call this one judge, as the PR body says. No consumer code change is needed and none is made.
  3. Public surface — right. Committed generated reference content/docs/references/automation/builtin-node-config.mdx is regenerated (number → integer, positive). The JSON Schema under packages/spec/json-schema/ is gitignored and rebuilt by gen:schema (.gitignore lines 62–63), so its type: integer / exclusiveMinimum: 0 is a build output, not a committed file; no other committed doc describes get_record.limit as a plain number. CHANGELOG mentions are historical.
  4. QueryPageWindowSchema deliberately not reused — right. It is int().nonnegative(); 0 means "total only" at a list door, while get_record always reads at least one row. A separate bound with TSDoc stating why is the correct call; reusing it would have admitted 0 and silently preserved the findOne fallthrough.
  5. ADR-0087 disposition — right. not-required (already-registered flow-builtin-node-config-values-refused) names a genuinely pre-existing D3 entry (not one this diff adds), and the entry is amended rather than left stale: surface (the added text carries no backtick and no pipe, as the file's header rule requires for build-upgrade-guide.ts), replacement and reason now name the get_record limit range, and the header comment records the one semantic difference (such a node used to run rather than fail). No D2 conversion is correct: the intended count is unknowable. The entry has no relevantWhen, so objectstack migrate meta lists it by construction, which is what the changeset's "reports it under flow-builtin-node-config-values-refused" amounts to. registry.ts.template's narrative paragraph for this id is example-based and not made wrong; leaving it is acceptable.
  6. Executor behaviour after the change — right. if (limit && limit > 1) is now exact (absent, or a whole number of at least 1). The executor's form descriptor limit: { type: 'integer', title: 'Limit' } has no minimum, so a designer form can still offer 0; that reaches the save door and is refused loudly at nodes.N.config.limit. Outside the claimed file surface; see ③.
  7. Tests — right. New probes at the judge (2.5 / 0 / -1 at limit), a [#25302] block covering FlowSchema in the contract's words, defineStack code+status+path for five values, the save-door schema, and lit controls (1, 2, 200, no limit). The reported ablation (10 failed / 59 passed with int().positive() removed) matches exactly the 10 refusal cases.

② Semver level

.changeset/25302-get-record-limit-positive-integer.md: @objectstack/spec: minor, fix(spec)!: title, BREAKING declared with FROM → TO → Fix → Unchanged, data-at-rest behaviour and producer measurement. Clause-②: no (narrowing) and Metadata-protocol: narrowed on both the changeset and the PR body, matching triage's ruling (p1, protocol narrowing handled in 18.x with no retirement stage, minor). The diff publishes exactly a narrowing of a published schema and nothing else (no widening, no new export). Level and clause lines are correct. Check Changeset: success.

③ Boundary flags

  • open_questions: [] — none to answer.
  • Dev out-of-scope finding (form descriptor limit without minimum in crud-nodes.ts, "noted, not filed"): escalated, non-blocking. Declared ≠ enforced does not bite (the save door refuses loudly, naming the key), but the Studio form now offers a value the save always refuses. That is a lower-grade authoring trap and belongs on a card (area:workflow, suggested p3: add minimum: 1 to the form descriptor). Not a defect of this PR's surface.
  • pnpm check:dual-build-cjs-loads NOT MEASURED locally, left to CI: answered by Lint & Repo Gates = success on this head.
  • Branch: is claude/project-thread-kdrs5c rather than claude/issue-25302-slug; the claim comment explains the platform constraint, and the branch-claim gate (The card this PR closes must claim this branch) is success.
  • Check-runs on d972d18b at review time (2026-10-23T03:05Z): 39 of 41 completed; every completed run is success or skipped (opt-in / matrix shells). Test Core (2/6) and Test Core (3/6) are still in_progress. The verdict below judges the completed gates plus the read diff; the two shards must finish green before merge, and a red one re-opens this record.

Implemented-by: claude/project-thread-kdrs5c
Reviewed-by: contract-review subagent of session_013odhA26dvzy7pQmevdTTvy

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:spec seat 1 (seat post #6017) · os-tesla · session session_013odhA26dvzy7pQmevdTTvy · 2026-10-23T03:12Z

Head d972d18be0 · card #25302 · claim 6355108668 · report 6356038913 · contract review PASS 6356385532 (CONTRACT_REVIEW_TIER, same head).

  • Shape: draft, base main, closing line Fixes #25302 only. File list (5 files, +111 / −9): the schema, its test, the ADR-0087 D3 entry, the changeset, the regenerated reference page. Not governed, under 3000 lines.
  • Premise measured: limit: z.number().optional() on main and the executor's limit > 1 → data.find path, both confirmed by the contract review in the cited code. Producers: 14 in this repo, 12 in HotCRM (4166f21), all positive whole numbers (dev's measurement).
  • Gates pinned to the head: 42 check-runs on d972d18be0, 36 success + 6 skipped, 0 red; Lint & Repo Gates and TypeScript Type Check success; Dogfood Regression Gate success (the narrowing's dogfood battery).
  • Changeset: @objectstack/spec minor, fix(spec)!:, Clause-②: no (narrowing), Metadata-protocol: narrowed, matching triage 6353914667.
  • Accepted deviations: the branch is the card thread's designated branch (recorded in the claim); no reviewer request (no MCP write).
  • Out of scope: the crud-nodes.ts form descriptor's limit has no minimum, so the designer form offers values the save refuses → filed service-automation: the get_record form descriptor offers limit 0 and negatives, which the narrowed contract (#25302) refuses at save (follow-up to #25302) #25361.
  • Landing: pr_ready + automerge_enable through the relay now.

Generated by Claude Code

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants