Skip to content

fix(spec): refuse decision mode beside a non-empty conditions list (#20168) - #20279

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20168-decision-conditions-mode-refused
Sep 27, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-20168-decision-conditions-mode-refused

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #20168
Clause-②: no

This PR carries ruling 5856786357 into packages/spec: batch #227 item 4, letter A, put in force by 5856865124. A decision node that declares a NON-EMPTY conditions list together with mode is now refused at authoring. The refusal names the ruled way out: drop mode (a conditions list is first-match on its own), or move the branches onto the edges and keep mode. mode belongs to the edge-branched decision alone.

Dispatched by the domain:spec seat 4 PM, session session_01CiCTczDo7tGhafXjf61dUJ. The claim is comment 5857419814. Branch base e46218674; every reading below is at head ae8185032 unless it says otherwise.

What changes

  1. DecisionConfigSchema (packages/spec/src/automation/schemaless-node-config.zod.ts) gains a .superRefine. When mode is authored beside a non-empty conditions array, it adds one custom issue at ['mode']. The message comes from a new helper, decisionModeWithConditionsRefusal(), which sits beside the existing decisionModePrescription() and echoes the value the same way. Both members are refused alike, because it is the key that has no reader here, not the value.
  2. Out of scope, by the ruling: mode beside an empty list, mode with conditions absent, and a list without mode. All three parse exactly as before. A mode outside the closed pair still gets its own value refusal first. A base-type issue aborts the object's refinement, so the author sees one issue, never two.
  3. The mode .describe() and docblocks now state the refusal. content/docs/references/automation/schemaless-node-config.mdx is regenerated with gen:docs, because check:generated proved only check:docs stale.
  4. dropped-refinements.baseline.json gains exactly the row the build's ratchet printed: automation/DecisionConfig, with one root site (the empty path). No arm of the projection list in shared/refinement-projection.ts fits "this key forbids that one when the list is non-empty". That rule is value-conditional, and dependent-required / banned-keys are presence-only. Adding an arm would be a public-contract decision, so the one route the ruling allows was taken. The header totals were recounted from the body: 211 → 212 schemas and 604 → 605 sites. The built json-schema/automation/DecisionConfig.json carries x-dropped-refinements: [{ at: '', type: 'object', count: 1 }].
  5. The changeset .changeset/20168-decision-mode-beside-conditions-refused.md bumps @objectstack/spec as patch, with Clause-②: no.

Timing and grade: measured at landing, not assumed (ruling item 2)

reading value source
npm latest @objectstack/spec 17.4.0 npm view @objectstack/spec dist-tags, 2026-09-27T16:12Z, and again at 18:03Z
mode in the published contract absent: json-schema/automation/DecisionConfig.json in the 17.4.0 tarball declares conditions only, with additionalProperties: false. In dist/automation/index.js, the control string first true expression wins has 2 hits and the test string edge-branched decision has 0 npm pack @objectstack/spec@17.4.0
Version Packages PR #17076 open, merged: false REST, 16:12Z and 18:03Z
pending changeset for mode .changeset/19867-decision-config-mode.md is still in .changeset/, so it is unconsumed tree at ae8185032

⇒ Unreleased. Per ruling item 2 this is patch, Clause-②: no, and no ADR-0087 entry. mode reaches its first release together with this refusal, so no published accept set narrows. Against the published 17.4.0 contract, the release still only widens. The same reading and its source are written into the changeset.

Which doors parse DecisionConfigSchema today (PM mechanism assumption 3, measured)

  • Direct parse: yes, through the export and through the SCHEMALESS_NODE_CONFIG_SCHEMAS.decision handle (one object). Both are pinned.
  • Flow registration: no. validateNodeConfigKeys (engine.ts) skips a node whose descriptor publishes no configSchema (if (!schema) continue;), and decision publishes none by design.
  • FlowSchema / defineFlow: no. FlowNodeSchema.config is a z.record(z.string(), z.unknown()), and the node-level config pass parses only an end node (parseEndNodeConfig).
  • os validate: no. lint-flow-patterns.ts reads config.conditions ad hoc (labels, emptiness) and never parses the schema. git grep on DecisionConfigSchema|SCHEMALESS_NODE_CONFIG_SCHEMAS|getSchemalessNodeConfigJsonSchemas outside packages/spec finds three places. metadata-protocol/src/reference-sites.ts is a JSON-projection walk, where a refinement projects byte-identically. service-automation's config-expression-ledger.test.ts reads the projection. config-expression-ledger.test.ts:325 mentions the schema in a comment only.
  • Published JSON Schema: it cannot state the rule. The rule is declared dropped instead, in the ledger and on the artifact, as item 4 above describes.

⇒ No door that answers in the ADR-0112 envelope (a code and a status) parses this schema yet. The envelope arrives with the registration-time reader that #15429 adds, which is the ruling's item 3. This PR pins the parse door, the by-node-type registry handle that reader will look up, and the per-parse objectStackErrorMap a validator may pass. Mechanism assumption 1 held (:471 / :486 / :206 on e46218674). So did assumption 2: the projection drops the refinement, and the ledger row is registered.

For #15429's acceptance list (the domain:services seat)

The refusal that the registration reader must surface:

  • issue code: 'custom', path: ['mode']; exactly one issue for a config with a legal mode beside a non-empty conditions.
  • message first sentence, verbatim (the value is echoed): `mode: 'inclusive'` is not valid on a decision that declares a `conditions` list — `mode` belongs to the edge-branched decision alone.
  • the two remedies in the same message: Either delete `mode` and keep the list … move the branches onto the out-edges (a `condition` on each branch edge, `isDefault: true` on the fallback), delete `conditions`, and keep `mode`.
  • the message carries no tracker number.
  • to leave alone: { conditions: [], mode }, { mode }, and { conditions: [...] } without mode.

Pins, and the ablation

packages/spec/src/automation/schemaless-node-config.test.ts:

  • The old pin "…and alongside a branch list, which the key does not forbid" asserted the accept-both shape. It is replaced by the ruled semantics.
  • A new describe block covers:
    • { conditions: [one], mode: 'inclusive' | 'exclusive' } and the same with a two-entry list: refused, one custom issue at ['mode'], ruled first sentence and both remedies.
    • The same refusal through SCHEMALESS_NODE_CONFIG_SCHEMAS.decision, and under objectStackErrorMap.
    • An illegal value beside a list: the value refusal only.
    • Controls, each a full safeParse success that round-trips: { conditions: [], mode } for both members, { mode } with conditions absent for both members, and a list without mode. Also a test that following either remedy parses.

Ablation (one-shot, run from the committed state; no permanent test file). The test imports the schema by relative path, so the source is what is resolved and no dist/ is in the path. node scripts/ablation-replace.mjs replaced the refinement's if (...) guard with if (false):

  • mutation: anchor 1 → 0, blob 70f2ae5d5b01 → 1de6a6511010;
  • run: 7 failed / 43 passed (50). All six refusal pins went red, plus following either remedy parses, whose first assertion is the refusal. The controls and the value-refusal-first test stayed green. That is the expected direction, and it was observed;
  • restore: blob back to 70f2ae5d5b01 == HEAD blob, git diff HEAD empty.

Flipped-semantics sweep (card clause)

No fixture, example, doc or skill authors conditions + mode together. The sweep grepped for mode: 'inclusive'|'exclusive' and the double-quoted forms:

  • examples/**, skills/** and content/docs/**: 0 hits. The six files carrying type: 'decision' were checked for any mode:, and the only two hits are a screen node's mode: 'create' and a comment.
  • packages/services, packages/lint, packages/cli, packages/metadata, packages/metadata-protocol: 0 hits.
  • /home/user/hotcrm at 2f7b2326 (read-only): 0 hits across its 14 decision-bearing files. The control isDefault hits.

objectui was not checked out in this container, so its designer form is NOT MEASURED here. objectui#10750 stays the coordination card (ruling item 4).

Verification at ae8185032

command result
pnpm --filter @objectstack/spec build exit 0; the dropped-refinement ratchet passes with the new row. Without the row it printed + automation/DecisionConfig (1 site(s)) and exited 1
pnpm --filter @objectstack/spec test exit 0: 549 files, 16159 passed, 2 todo
pnpm --filter @objectstack/spec typecheck exit 0 (tsc, check:scripts-typecheck, check:test-typecheck)
pnpm --filter @objectstack/spec check:generated 14/15 current plus check:docs stale. After gen:docs, check:docs gives exit 0, "226 generated files in sync"
consumer closure turbo run build --only (service-automation / lint / metadata-protocol closures, plus client and client-react, excluding spec) exit 0
@objectstack/service-automation vitest exit 0: 146 files, 1757 passed
@objectstack/lint vitest exit 0: 110 files, 4258 passed
@objectstack/metadata-protocol vitest exit 0: 189 passed, 3 skipped files; 2715 passed, 19 skipped
dispatch-gates --commands → each run → --ran 107 derived, 105 exit 0, 2 NOT MEASURED, 0 unrun
eslint (--no-inline-config --format json) on the 2 touched .ts files exit 0, 2 files, 0 errors, 0 warnings
check:nul-bytes plus a control-byte self-scan of the 4 hand-edited files exit 0; 0 matches
  • NOT MEASURED, declared. check:dual-build-cjs-loads and check:type-check-debt both exited 3 (PREREQUISITE NOT MET): they need every workspace package built, which is 86 packages without dist here, and lint.yml's own prerequisite is a full turbo build of all packages. CI runs both on the PR.
  • Consumer sweep direction. I ran the direct importers of the schema family found by git grep (upstream of nothing; downstream of spec), plus @objectstack/lint as the dispatch named it. I did not run all of ...@objectstack/spec: the public types are byte-unchanged (check:api-surface exit 0 with no regeneration; z.input/z.infer are unaffected by a refinement), so only the parse accept set of this one schema narrows.
  • eslint narrowing. The population is the two .ts files. The .json, .md and .mdx files match no eslint config object. The count comes from the JSON output. It is invariant for untouched files, because eslint.config.mjs never enables type-aware linting (no parserOptions.project).

Acceptance notes


Generated by Claude Code

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/spec/dropped-refinements.baseline.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/dropped-refinements.baseline.json) — pages documenting those are invisible to this run
  • 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 — 136 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 17bd31877109b7cc692e7e54c4fe39f82a5c32d5 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 17bd31877109b7cc692e7e54c4fe39f82a5c32d5

⚠️ 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: ae8185032a188422643476a3861d6d9e9a8d2c2f

Read: card #20168 body and all 6 comments (premise refresh 5852968135, ruling 5856786357, hold 5856799823, confirmation 5856865124, claim 5857419814, dev report 5858409074); PR #20279 object, body, 5-file list, 4 commits, full diff vs merge-base e462186, thread (1 advisory bot comment, 0 reviews); check-runs on the head at 18:33Z and 19:08Z; at the head: schemaless-node-config.zod.ts, its test, strict-object.ts, lazy-schema.ts, refinement-projection.ts, dropped-refinements.baseline.json + scripts/dropped-refinements.test.ts + build-schemas.ts, engine.ts validateNodeConfigKeys, logic-nodes.ts, flow.zod.ts, reference-sites.ts, the regenerated mdx, both changesets, the changeset-gate headers and pr-automation.yml WHICH LEVEL prose; PR #20251 diff; PR #17076 state; npm dist-tags and the 17.4.0 tarball. Ran (detached worktree at the head, pnpm install --frozen-lockfile, every build/test through os-verify-lock.sh): target test + a 17-case probe (51/51), four ablations with proven restores, OS_SKIP_DTS=1 spec build, check:docs, a merge-tree of the two ledger PRs, a ledger recount at base/head/joint. NOT MEASURED: the objectui designer form (sibling not checked out, objectui#10750); full pnpm test/typecheck, check:dual-build-cjs-loads and check:type-check-debt locally (CI green at the head, not re-run); no derived gate family re-run.

① Derived judgments

  • (a) The refusal — holds. Probed at the head on the resolved zod 4.6.1 (the ledger's zod: "4.4.3" field is pre-existing and untouched). { conditions: [one], mode: 'inclusive' }, the same with 'exclusive', and a two-entry list each yield exactly one issue: code: 'custom', path: ['mode'], message beginning mode: 'inclusive'` is not valid on a decision that declares a `conditions` list — `mode` belongs to the edge-branched decision alone. The value is echoed, the reason is stated (first-match on its own), and both remedies are named verbatim: "Either delete mode and keep the list, or move the branches onto the out-edges (a condition on each branch edge, isDefault: true on the fallback), delete conditions, and keep mode." No tracker number in the runtime string (the test pins not.toMatch(/#\d{3,5}/)). An ILLEGAL mode beside a list — 'all', 5, null — gets exactly one invalid_value issue at ['mode'] carrying decisionModePrescription(), never the custom issue too: in zod 4 runChecks skips the object's checks once util.aborted(payload) is true, and the enum issue carries no continue: true. The same single issue comes through SCHEMALESS_NODE_CONFIG_SCHEMAS.decision (probe C3: the handle === the export) and under objectStackErrorMap.
  • (b) The controls — hold. { conditions: [], mode } (both members), { mode } with conditions absent, and { conditions: [non-empty] } without mode all safeParse to success and round-trip (parse(parse(x)) stable; mode has no .default()). Explicit undefined on either key is stripped and accepted. "Non-empty" is Array.isArray(conditions) && length > 0 evaluated on the parsed value, and the refinement never runs over an aborted base parse, so there is no confusing second issue: conditions: [undefined] → only invalid_type at ['conditions', 0]; [{ label: 1 }] → only its two invalid_type issues; conditions: 'x' / null → only invalid_type at ['conditions']; an unknown key beside the pair → only unrecognized_keys (strictObject marks it terminal); a bad element plus an illegal mode → three base issues, no custom one.
  • (c) The pins — load-bearing. Baseline at the head: 50/50 on the target file. Ablation A (the dev's: guard → if (false)), re-run through scripts/ablation-replace.mjs: 7 failed / 43 passed — the six refusal pins plus "following either remedy parses"; restore proven, blob 70f2ae5d5b01 == HEAD, git diff HEAD empty. That mutation is right for the refusal pins but cannot show the controls bite, so three more were run, each restored the same way: C (path: ['mode'] → []) 6 red — the location is pinned; D (length > 0 → > 1) 4 red (both one-entry pins, the handle pin, the error-map pin) with the multi-entry pins green — the boundary sits at one entry; B′ (length > 0 → >= 0, an over-broad guard) exactly 2 red, the two empty-list controls, 48 green — the controls are load-bearing. (A first B attempt was refused by the tool because its replacement was a substring of the anchor; it produced no reading and is not counted.)
  • (d) The by-node-type handle and every door — verified. Tree-wide git grep at the head for DecisionConfigSchema|SCHEMALESS_NODE_CONFIG_SCHEMAS|getSchemalessNodeConfigJsonSchemas|schemaless-node-config outside spec's own source/tests and generated references finds only packages/metadata-protocol/src/reference-sites.ts:480 (a z.toJSONSchema projection walk, where a refinement projects byte-identically; not a validation door) and service-automation/src/builtin/config-expression-ledger.test.ts (the projection, for ledger reconciliation). No .omit, .extend, .pick, .partial, .merge or .strip on the schema or the .decision handle anywhere; the only .shape read is spec's own test at :473, a key-set read that the .superRefine() clone preserves (probe C4). Registration: engine.ts:9225-9226 reads this.actionDescriptors.get(node.type)?.configSchema and if (!schema) continue;; the decision descriptor at logic-nodes.ts:22-28 publishes no configSchema (the configSchema: at :132 belongs to assignment), so registration skips it as the dev says. FlowNodeSchema.config is z.record(z.string(), z.unknown()) (flow.zod.ts:488) and only parseEndNodeConfig descends (:289, :439). No package reads json-schema/automation/DecisionConfig.json for validation (0 hits outside spec); that file is gitignored, built, and carries x-dropped-refinements. Conclusion: every door that parses DecisionConfigSchema today goes through the one refined object; the JSON projection cannot carry the rule and declares that.
  • (e) The ledger — correct, by the allowed route. Body recount: base 211 schemas / 604 sites = header; head 212 / 605 = header; exactly one new row, automation/DecisionConfig: { sites: [""] }, in sorted position; pinned by packages/spec/scripts/dropped-refinements.test.ts:369-371 against entries. The closed projection list (required-one-of, non-blank-string, dependent-required, banned-keys, banned-key-pattern) has no arm for a value-conditional exclusion and its own docblock says an if/then-shaped rule "stays dropped and annotated", so the ledger row is the ruling's item-1 route. Built at the head (OS_SKIP_DTS=1 pnpm --filter @objectstack/spec build, exit 0): the ratchet prints "605 refinement site(s) across 212 published schema(s) … all declared"; json-schema/automation/DecisionConfig.json has properties: [conditions, mode], additionalProperties: false, x-dropped-refinements: [{ at: '', type: 'object', count: 1 }]; check:docs exit 0, "226 generated files in sync". PR feat(spec)!: retire currencyConfig.precision — a currency's decimal places are its currency's (ADR-0049) #20251 (head 71ea994d8, base e46218674, open draft) edits the same two header lines (211→210, 604→591) and removes 12 site lines plus the data/CurrencyConfig row (13 sites) in body hunks disjoint from fix(spec): refuse decision mode beside a non-empty conditions list (#20168) #20279's insertion. git merge-tree --write-tree of the two heads: CONFLICT on packages/spec/dropped-refinements.baseline.json — textual, on the header; the bodies merge. Joint totals once both land: 211 schemas / 592 sites. The ledger is neither merge=os-regen-managed nor in NOT_DRIVER_MANAGED, so the queue's server-side merge will reject whichever PR is second; that lander must git merge origin/main, resolve the header by hand to 211 / 592, and rebuild — the PR body's contention note says this, and the test pins it.
  • (f) First-party authors — none refused. examples/**, packages/services/service-automation/**, skills/**, content/docs/**, packages/lint|cli|metadata|metadata-protocol|plugins|runtime: 0 hits for mode: 'inclusive'|'exclusive' in either quoting; of the 22 files there carrying type: 'decision', the only mode: keys are two screen nodes (validate-translation-references.test.ts:2057 mode: 'edit', content/docs/automation/flows.mdx:508 mode: 'create'). /home/user/hotcrm at 2f7b2326e8f5 (read-only): 14 decision-bearing files, 0 mode: keys, 0 inclusive|exclusive; control isDefault hits in 14 files. Nothing to migrate.

② Semver level

Re-measured 2026-09-27T18:33Z: npm dist-tags for @objectstack/spec = { latest: 17.4.0, rc: 17.0.0-rc.6 }; npm pack @objectstack/spec@17.4.0 → json-schema/automation/DecisionConfig.json declares conditions only with additionalProperties: false (no mode); dist/automation/index.js has 2 hits for "first true expression wins" and 0 for "edge-branched decision". PR #17076 chore: version packages: open, merged: false at 18:32Z and again at 19:08Z. .changeset/19867-decision-config-mode.md is still in the tree at the head, unconsumed. ⇒ mode is unreleased; the refusal narrows no published accept set. The changeset grades @objectstack/spec: patch, Clause-②: no, no BREAKING banner, no ADR-0087 marker — exactly ruling parameter 2. Against the gates' stated rules (headers read, families not run): check-changeset-no-major's level axis stands down when the declaration reads no (not-declared, exit 0); the WHICH LEVEL prose says a fix( that changes no public surface stays patch, and against the published 17.4.0 contract this diff changes none (the release only widens, via #19867's minor, which also sets the lockstep bump); check-adr-0087-registration judges only a changeset that declares breaking, and none does. CI's Check Changeset job: all steps success. Grade consistent with the measured release state.

The pending #19867 bullet "A conditions list is unaffected" will be compiled into the same version's CHANGELOG as this PR's entry and becomes false the moment that release ships both. This PR's own changeset is enough for every gate and for the ruling, and the dev recorded the one-clause amendment in Acceptance notes. But packages/*/CHANGELOG.md is release-owned and can only be fixed afterwards by a dedicated docs-only PR, whereas an unconsumed .changeset/*.md is ordinary input that this PR — the only PR that changes the fact the bullet states — may correct now. Judged non-blocking; the cleaner landing appends " — and mode beside a non-empty list is refused" to that bullet in this PR before merge, as a deliberate correction.

③ Boundary flags

Blocking: none
Non-blocking: (1) Correct the #19867 pending-changeset bullet in this PR as above, or hand the recorded amendment to the release compiler explicitly. (2) Ledger contention with #20251: header-line conflict; joint totals 211 / 592; the second lander recounts by hand (the ledger is hand-edited and driver-excluded), and the test pins it. (3) objectui#10750: the designer form on a conditions-list node is NOT MEASURED here; the PR body names it as the coordination card (ruling item 4). (4) The #15429 acceptance list is named in the body with code custom, path ['mode'], the verbatim first sentence, both remedies and the leave-alone shapes — satisfied; it and objectui#10750 sit under their own headings rather than under "## Acceptance notes", cosmetic. (5) Pre-existing, not this PR's: the ledger header's zod: "4.4.3" reading while packages/spec resolves 4.6.1. (6) The Docs Drift Check comment is advisory (the ledger yields no anchor). Closing keywords: the body closes exactly #20168 (Fixes #20168, once; no other closing keyword; the title carries (#20168) only as a reference). All four commits end with the model-free Claude-Session + Co-authored-by: Claude trailer pair; no model identifier in the body, changeset, code or comments.

CI at this head: 35 check-runs, all completed: 33 success, 2 skipped (Console Pin Gate; Packed-tarball smoke, opt-in). All seven required contexts success: Lint & Repo Gates, TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL), Governed Surface Queue Guard. Read at 18:33Z and unchanged at 19:08Z. The PR is a draft on objectstack-ai/objectstack (not a fork), mergeable: true, no auto-merge armed.

Implemented-by: claude/issue-20168-decision-conditions-mode-refused
Reviewed-by: session_01CiCTczDo7tGhafXjf61dUJ

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review September 27, 2026 19:14
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Sep 27, 2026
Merged via the queue into main with commit 733822c Sep 27, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-20168-decision-conditions-mode-refused branch September 27, 2026 19:35
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 28, 2026
…ull — at all three doors (objectstack-ai#19961) (objectstack-ai#20315)

Fixes objectstack-ai#19961

Clause-②: no (narrowing)

A `decision` branch with no `expression` (the key absent, or
`expression: null`) is now refused at all three doors:
`FlowSchema.parse`, `AutomationEngine.registerFlow` and `objectstack
validate`. It goes through the same walk, the same function and the same
lead sentence that already refuse a blank branch predicate (objectstack-ai#17493 / PR
objectstack-ai#19960).

## What was wrong, measured on `origin/main` `a9fb83ef`

`DecisionConditionSchema` declares a branch `{ label, expression }` with
`expression` a required `z.string()`. Nothing parses a decision node's
open `config` against that schema. The expression ledger's resolver also
skipped an absent value as "not authored". So the build accepted a
branch that the run refuses.

| branch | `FlowSchema.parse` | `registerFlow` | `objectstack validate
--json` |
|:--|:--|:--|:--|
| `{ label: 'y' }` (the card's shape) | accepted | registered | `valid:
true`, exit 0 |
| `{ label: 'y', expression: null }` | accepted | registered | `valid:
true`, exit 0 |
| `{ label: 'y', condition: 'true' }` (the edge's spelling) | accepted |
registered | `valid: true`, exit 0 |
| `{ label: 'y', expression: ' ' }` (control, objectstack-ai#19960) | refused,
`custom` at `nodes.1.config.conditions.0.expression` | refused, same
issue | `valid: false`, exit 1, same path |
| `{ label: 'y', expression: 'true' }` (control) | accepted | registered
| `valid: true`, exit 0 |

What the run did with it: `evaluateCondition({ dialect: 'cel', source:
undefined })` and `source: null` both throw `condition evaluation error:
A structural condition …`. On the real decision executor, a run that
reaches such a branch ends `success: false` at the branch (pinned
below).

## The fix: one walk, one judge

- `packages/spec/src/automation/flow-node-expression-paths.ts`
- `FlowNodeExpressionPath` gains `required?: true`. It is set on
`decision` `conditions[].expression` and on nothing else.
- For a `required` predicate slot, `resolveFlowNodeExpressions` now
emits the absent or `null` value on a branch that exists. It still skips
a decision with no `conditions`, an empty list, and an absent screen
`visibleWhen`.
- `predicateSlotRefusal(undefined | null)` now has its own detail
sentence and prescription, under the unchanged
`PREDICATE_SLOT_STRING_REFUSAL` lead.
- `packages/spec/src/automation/flow.zod.ts`: the `FlowSchema`
predicate-slot refinement now admits the absent or `null` value of a
`required` slot, next to strings. Every other non-string keeps its
objectstack-ai#15572 scope, so it is still not refused at this door.
- `packages/lint/src/validate-expressions.ts`: `checkDeclaredPredicate`
dropped its `raw == null` early return. Whether an absent value is a
finding is the resolver's call. The early return answered "valid" for
the exact value `FlowSchema.parse` refuses, for any caller of
`validateStackExpressions` that does not parse first. This site is
outside the claim's file surface. The measurement put the third door's
refusal there (ablation B below).
- `engine.ts`: no change. Its ledger pass already calls
`predicateSlotRefusal` on everything the resolver emits, and
`registerFlow` parses first, so the parse answers first. That is the
same two-layer shape the blank has.

**The route choice (Zone 2 item 3).** I chose (a), the predicate-slot
walk treating an absent `expression` as a refused slot. I did not choose
(b), parsing each branch against `DecisionConditionSchema`. Reasons, per
axis:

- Business need, measured: the only named producer is objectui's
`rowsToList`, which writes `{ label }`. The absent key is the whole
defect.
- Long-term design: (a) keeps one judge (`predicateSlotRefusal`) and one
walk for the three doors. (b) would be a second judge with Zod's own
messages, a different prescription at each door, and a key-set closure
on branches that nobody ruled.
- Guarding AI authors: both refuse loudly. (a) also names the
`condition` alias mistake in the same prescription.
- No scope growth: (a) touches one ledger entry. (b) would narrow a much
wider accept set, including unknown branch keys and the whole branch
shape.

`DecisionConditionSchema` and the fenced `DecisionConfigSchema` / `mode`
region are untouched. objectstack-ai#20168's PR objectstack-ai#20279 landed while this was in
flight, and this branch is merged over it (`7534fd7e`). Its refusal
lives in `DecisionConfigSchema`, which no door parses a node's config
against, so it and this walk do not meet. Its suite is green here (in
the `src/automation` run below).

**Prescription wording.** Triage (`5811370954`) says PR objectstack-ai#19960's
decision-branch prescription is "删掉这个分支" (delete the branch). The landed
objectstack-ai#19960 text says something else: write the predicate, or `expression:
'false'` to keep what the blank ran, and ⚠️ **not** by dropping a
decision's only branch. That clause is pinned by
`predicate-slot-blank.test.ts`. This PR follows the landed wording. It
drops the "keep what ran" half, because an absent predicate never ran:
it failed the run at the branch. So `'false'` is offered as "keep the
branch and its label, never take it", and nothing is claimed to be
preserved.

### The refusal text, quoted (`predicateSlotRefusal(undefined)`, byte
for byte what all three doors print)

```text
A predicate slot holds BARE CEL TEXT that states a rule — it is declared `z.string()` — so an expression envelope, any other non-string, or a string that is blank after trimming is not authorable there. Found nothing — the key is absent where the slot is required: a decision branch is `{ label, expression }` and its `expression` is not optional, so a branch without one states no rule. Write the predicate the branch was meant to test (e.g. `record.rating >= 4`); a predicate written under another key — `condition` is the edge's spelling — belongs in `expression`. There is no run to keep: the executor evaluates every branch it reaches, and a branch with no `expression` failed the run there. To keep the branch and its label but never take it, write `expression: 'false'`. Not by dropping a decision's only branch: the node then routes by its out-edges alone, and the out-edge that branch labelled is no longer held back.
```

For `null`, `Found nothing — the key is absent` reads `Found` followed
by the code-spelled `null`. The lead sentence
(`PREDICATE_SLOT_STRING_REFUSAL`) is unchanged, byte for byte.

**After, measured on `e702ebd4` (the real CLI door, spec rebuilt; no
file of this diff changed after that).** `{ label: 'y' }`, `expression:
null` and `condition: 'true'` all give `objectstack validate --json`
`valid: false`, exit 1, one `custom` error at
`flows.0.nodes.1.config.conditions.0.expression`. The absent and alias
messages are byte-identical. `registerFlow` refuses the same three with
a `custom` issue at `nodes.1.config.conditions.0.expression`.
`expression: 'true'` still validates and registers. The blank keeps its
own message.

## Pins: one table per door, the same five rows

Every refused row asserts the issue `code`, the `path`, and the full
message equal to the spec's own `predicateSlotRefusal(value).message`.

-
`packages/spec/src/automation/flow-decision-branch-expression-absent.test.ts`:
`FlowSchema.parse`.
- The five rows: absent, `null`, `condition` alias, blank control, real
accept control.
  - Branch index 1 is anchored. The ADR-0031 region body is anchored.
- Controls: a decision with no `conditions` or `[]` still parses; an
absent screen `visibleWhen` still parses.
-
`packages/services/service-automation/src/decision-branch-expression-absent.test.ts`:
`registerFlow`.
  - The same table. `getFlow` is `null` after each refusal.
  - Region body.
- On the real decision executor: the absent branch failed the run
(`success: false`, `condition evaluation error`, ran `['start']`);
`expression: 'false'` routes to the fallback.
- `packages/lint/src/validate-expressions.test.ts` `describe('a decision
branch with no expression (objectstack-ai#19961)')`: `validateStackExpressions`, with
the same table, the exact `where` string, branch index 1, and controls.
- `packages/spec/src/automation/flow-node-expression-paths.test.ts`:
- The resolver emits `undefined` or `null` for the decision slot and
skips everything else.
- `predicateSlotRefusal(undefined | null)` prescription clauses are
pinned by name.
- The `required` set is pinned to exactly
`decision.conditions[].expression (predicate)`, because the absent arm's
wording is decision-specific.
-
`packages/services/service-automation/src/builtin/config-expression-ledger.test.ts`:
the reconciliation ratchet now reads each channel's JSON-Schema
`required` list. It asserts that the ledger's `required` flags equal the
channel's, in both directions, over the `predicate` role. It derives,
not assumes, that `visibleWhen` is optional. It asserts that `required`
is never set on another role. The channels do require `loop.collection`
/ `map.collection`, but no door refuses their absence (reported to the
seat as an out-of-scope finding).

**Pin sweep.** One published pin flipped:
`decision-predicate-envelope.test.ts` asserted
`decisionFlow('str_absent', undefined)` registers. It was re-judged in
place, and the reason is written beside it. It now asserts the throw
carries `PREDICATE_SLOT_STRING_REFUSAL` and `Found nothing — the key is
absent where the slot is required`.

Repo sweep for other branches without an `expression`: a
bracket-balanced scan of every `.ts` / `.json` / `.yaml` file that
mentions both `decision` and `conditions` found only this PR's own
fixtures. A grep of helper-built branches (`{ label: …, expression }`
shorthand) found 4 sites, all in suites run below. No other package's
test builds a `decision` with `conditions`.

## Ablation: the pins can fail

Both ablations were run on committed state through
`scripts/ablation-replace.mjs`, which wraps the change, verifies it on
disk and restores it with a trap. Both proved restore by blob hash equal
to HEAD and an empty `git diff HEAD`.

- **A: the `required` flag neutralised.** `required: true,` was replaced
by a spread that is `{}` unless a `globalThis` flag named
`ABLATION_19961` is set.
- `ablation-dist-preflight.mjs @objectstack/spec ABLATION_19961` found
the marker present in 20 built files.
  - Red, in the expected direction:
- spec: 7 failed (the absent, `null` and alias rows, index 1, region,
the `required`-set pin, the resolver pin);
- service-automation: 6 failed (the three rows, region, the re-judged
envelope pin, the ratchet);
    - lint: 4 failed.
  - The blank and real controls stayed green at every door.
- Restore leg: rebuild, `--absent` marker gone from all 222 built files,
whole-tree `git status` clean. spec 52/52, service-automation 34/34 and
lint 344/344 green.
- The first attempt was a no-op and its reading was discarded: my
replacement was not valid TypeScript, so the transform failed and the
build never ran.
- **B: the lint early return put back** (`if (raw == null) return {
refused: false };`): lint showed 4 failed (absent, `null`, alias, index
1), and the blank and real rows stayed green. Restored by blob hash. The
first attempt was refused by the tool before running anything, because
the anchor matched its own replacement.

## Producer census (Zone 2 item 4): authored count 0

- `examples/**` at `e702ebd4`: 3 flows carry `decision` nodes (app-crm
`convert-lead`, app-showcase `needs_exec` / `triage`, app-todo
`check_recurring`). All of them branch on out-edges and declare no
`conditions`, so 0 branches lack an `expression`.
- `packages/**` non-test: no default flow carries a `decision` node. The
`content/docs/automation/flows.mdx` examples: 3 `conditions` lists, all
with `expression`.
- cloud `origin/main` `96eb092f`: 0 `decision` nodes.
`service-ai-studio`'s authoring whitelist names `decision` as an
authorable node type, so AI-authored flows now meet this refusal.
- objectui at the pin `f8a9d0fb` (`.objectui-sha`):
`FlowObjectListField` `rowsToList` still drops a blank cell, so a branch
row with an empty expression cell is written as `{ label }`. That is the
known writer. Triage accepted that its save now fails loudly, so it is
not fixed here.

## ADR-0087 and changeset

- New semantic entry `flow-decision-branch-expression-absent-refused`
(major 18) and a regenerated `registry.ts`.
- There is no D2 conversion: the platform cannot know the rule the
author left out, and `'false'` would change behaviour rather than keep
it.
- The changeset
`.changeset/19961-decision-branch-expression-absent-refused.md`:
`@objectstack/spec` and `@objectstack/lint` `minor`, BREAKING, with the
FROM → TO table.
- `service-automation` gets no changeset: its diff is test files only,
and those are not in `files[]`.

## Verification (final head `7534fd7e`, which is `origin/main`
`6a6a17b6` merged, objectstack-ai#20279 included)

- Tests (`os-verify-lock`, spec rebuilt on this head):
- spec `src/automation` + `src/migrations`: 1005/1005, including
objectstack-ai#20279's `schemaless-node-config.test.ts`.
  - service-automation: the 4 predicate-slot / ledger files, 50/50.
  - lint `validate-expressions.test.ts`: 344/344.
- Full suites, run on the first merge head `266cd043`: spec 16855 passed
(583 files), service-automation 1767/1767, lint 4269/4269.
- Typecheck, including the test layers, on `266cd043`: spec, lint and
service-automation all exit 0. No file of this diff changed after that.
- Gates: `dispatch-gates.mjs --commands` re-derived on `7534fd7e` gives
90 families. 89 ran with exit 0. `dispatch-gates --ran` answers "90
derived famil(ies) accounted for — 89 run, 1 NOT-MEASURED".
- NOT MEASURED: `check:type-check-debt`. Its `--re-measure` runs a
whole-tree `turbo run build --filter=./packages/*` outside
`os-verify-lock`. This diff touches no DEBT-ledger package.
- On earlier heads, two gates needed their prerequisites built first,
and both then exited 0. `check:dual-build-cjs-loads` answered
PREREQUISITE NOT MET because 12 unrelated packages were unbuilt.
`check:dts-closure` went red on local state: 6 packages lost their
`.d.ts` to my own interrupted `--re-measure` build. That is not this
diff.
- `eslint --no-inline-config --format json` over the 12 changed `.ts`
files on `7534fd7e`: 12 files, 0 errors, 0 warnings.
- Population: `eslint.config.mjs` `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']`, and no file here is ignored.
- Invariance: the config never enables type-aware linting (no
`parserOptions.project`), so this diff cannot move a verdict on an
untouched file.
- Declared narrowing: after the second and third `origin/main` merges, I
re-ran the targeted suites above and every gate, not the full package
suites. The incoming commits touch other surfaces (rls, date comparands,
report charts, cli generate, pm scripts, and objectstack-ai#20279's
`DecisionConfigSchema` `mode`), not the flow predicate walk.

## Acceptance notes (observed, not filed)

- `service-automation` `engine.ts` `evaluateCondition` still has an
inline comment saying the empty-source arm is where "a `decision` node
whose `conditions[]` entry has no `expression`" lands and answers
`false`. Since objectstack-ai#16038 the shape gate throws first, and since this PR the
shape cannot register. The comment is stale; no behaviour follows from
it. Carrier: whoever next edits `evaluateCondition`, else none.

---

_Generated by [Claude
Code](https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
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

Development

Successfully merging this pull request may close these issues.

[Decision] A decision node that declares both a conditions list and mode: refuse it at authoring, give it a meaning, or leave it inert

2 participants