Skip to content

fix(service-automation): evaluation refuses a malformed condition shape with registration's own refusal - #16438

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-16038-evaluate-condition-shape-refusal
Sep 7, 2026
Merged

fix(service-automation): evaluation refuses a malformed condition shape with registration's own refusal#16438
os-warren merged 1 commit into
mainfrom
claude/issue-16038-evaluate-condition-shape-refusal

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #16038

Implements the maintainer's ruling of 2026-09-06 (decision batch #57, comment 5559820263), option A: evaluateCondition refuses a malformed condition shape with the same STRUCTURAL_CONDITION_SHAPE_REFUSAL registration already raises, so evaluation and registration share one refusal.

Verified at 59d785982 — the final head, and the commit every number below was measured on.

The enumeration came first, and it found two arms the reproduction does not name

The ruling's first execution note is to enumerate every sibling site before patching the reported one. There is exactly one unguarded read in evaluateCondition — the exprStr derivation — but it has three distinct measured failure arms, and a fix written at the reported line reaches only two of them:

Arm Input Behaviour on 3e7ef9c23, driven
A — raw TypeError envelope with a present, non-string source: { source: 1 }, { dialect: 'cel', source: 1 }, { dialect: 'cel', source: {} }, { dialect: 'template', source: 1 } TypeError: exprStr.trim is not a function — naming no flow, no node, no expression
B — silent false neither text nor envelope-shaped: 42, true, ['a'], {}, { dialect: 'cel' } returns false off the empty-source arm, nothing logged
C — silent false, one statement earlier malformed envelope under a non-predicate dialect: { dialect: 'cron', source: 1 } returns false at the dialect pre-check, never reaching the derivation

Arm C is why the guard is the method's first statement rather than a patch at exprStr. All three are refused by structuralConditionRefusal at registration today, so all three are the ruling's population.

Every helper the method delegates to — templateHoles, celScope, refuseUnresolvedTemplateHole, refuseUnresolvedCelOperand, compareValues — is handed exprStr and nothing else, so one guard above the derivation closes the whole delegation tree.

The sibling value path is NOT a site — measured, not assumed

evaluateValueEnvelope already derives its verdict from valueEnvelopeRefusals, the same call registerFlow makes (#15137). Driven before the fix, { source: 1 }, { dialect: 'cel', source: 1 }, { dialect: 'cel', source: {} }, { ast, source: 1 }, { dialect: 'cel' }, 42, ['a'] and {} each threw an attributed error leading with ASSIGNMENT_VALUE_ENVELOPE_REFUSAL or a located CEL fault. Zero raw TypeErrors. It is already this shape one door over, with its own shared constructor. Nothing moved there.

One residual defect found there is deliberately not fixed here and is filed separately: evaluateValueEnvelope(null) / (undefined) throws TypeError: Cannot read properties of null (reading 'source'). It is a different refusal vocabulary the ruling does not name, and it is unreachable from the only production call site.

No caller depended on the TypeError

Repo-wide, every occurrence of is not a function on this path is prose recording the pre-fix symptom — never an assertion, never a catch that branches. structural-condition-shape.test.ts asserts the opposite direction already. The two engine-internal callers (the start gate at engine.ts and the edge gate) call it bare, so a throw propagates to execute()'s catch and is recorded as a loud flow failure — ADR-0032 §1c's prescribed handling. The screen-field caller passes String(...) and cannot reach the site. The ruling lands as written.

logic-nodes.ts:73cond.expression is not guaranteed a string

conditions is an unchecked as Array cast taken off node.config, an open z.record that DecisionConditionSchema is never parsed against. The only thing making it a string is registration's predicateSlotRefusal gate (#15572) — a producer-side gate. Measured: registering such a flow throws today, so authored metadata cannot reach it; a flow stored before that gate landed, or a direct caller of this public method on an exported class, still can. That asymmetry is exactly what this PR closes.

One constructor, not two envelopes

The guard calls structuralConditionRefusal — the identical import registerFlow already uses at the same file. No second hand-written envelope was introduced, and no file under packages/spec is touched: the constructor was already exported and already imported by engine.ts.

Tests — red before green, per site

structural-condition-shape.test.ts gains 18 tests: one per refused shape across all three arms, three property tests, and five CONTROL tests. The prediction was written before the run and matched exactly.

The strongest of the three property tests walks one shared population through both doors and asserts both refuse with the same published sentence — the machine-checkable form of "the two reject sets are one set". Two envelopes that drifted apart would fail there while every per-site row stayed green.

Ablation, from the committed state, with both legs proven on disk:

  • mutation: guard-call marker 1 to 0, sentinel 0 to 1, blob 213c3d65 to 804490eb; result 13 failed | 18 passed — the same 13, so the mutation demonstrably reached the subject. No rebuild is involved: the suite imports ./engine.js relative source, not a package exports entry.
  • restore: git checkout HEAD -- ABSOLUTE_PATH (never the bare form, which takes from a polluted index), then blob equals the HEAD blob 213c3d65 with both hashes non-empty, sentinel back to 0, guard-call back to 1, and git diff HEAD zero bytes.

Controls pinned, so the guard cannot over-reach

Bare CEL text and both envelope spellings still evaluate; an ast-only envelope still answers false (that population is #15430/#15807's, deliberately untouched); a well-formed cron envelope still answers false rather than being refused; absent, null, empty and whitespace-only conditions are still "not authored"; and a malformed string still earns the brace trap or the §1c CEL fault, never the shape refusal.

Changeset

Included, minor, @objectstack/service-automation only. Required because this is shipped runtime source in a published package and the change is user-observable: a stored flow carrying a refused shape previously ran (silently false, or faulted unattributed) and now fails loudly. minor rather than patch matches the sibling #15662 bump for the same class — a previously-accepted population is now refused. packages/spec and packages/lint are unversioned here because neither is modified.

Verification

  • structural-condition-shape.test.ts — 31/31.
  • @objectstack/service-automation full suite — Test Files 124 passed (124), Tests 1461 passed (1461).
  • typecheck and check:test-typecheck — green. The latter compiles the test layer under tsconfig.test.json, so the new tests are actually type-checked rather than excluded.
  • Downstream consumers (prefix filter, i.e. packages that consume this one) connector-mcp 23/23, trigger-record-change 101/101, trigger-schedule 57/57, plugin-approvals 690/690. An initial connector-mcp failure was an unbuilt-dist collection error, not a finding; it passes once the prerequisite is met.
  • Gate families derived mechanically at the final head with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the four artifact-roster families whose roster sits under a directory this diff touches: 60/60 green. Three initially returned PREREQUISITE NOT MET (unbuilt monorepo dist) — recorded as not-measured, then re-run to a real reading after turbo run build over the packages closure, exactly as lint.yml does.

Local scope was the affected package plus its consumers and the derived families; the repo-wide scans remain CI's run.


Generated by Claude Code

…pe with registration's own refusal

`evaluateCondition` derived `exprStr` with a `typeof` guard covering only the
bare-string arm, so an envelope whose `source` was present and not a string
became that value and `.trim()` threw a bare `TypeError` naming no flow, no
node and no expression. Two sibling arms shared the same unguarded read: a
value that is neither text nor envelope-shaped read as an EMPTY condition and
answered a silent `false` — on the same key a start node's trigger gate is read
from — and a malformed envelope under a non-predicate dialect answered `false`
one statement earlier still, at the dialect check.

#15662 closed this reject set at the producer; the evaluator was left
disagreeing with it in a different vocabulary. Per the maintainer's ruling
(decision batch #57, option A) evaluation now calls `structuralConditionRefusal`
— the SAME constructor `registerFlow` calls, not a second hand-written envelope
that could drift — as the method's first statement, above the dialect check so
all three arms are covered.

Controls pinned alongside: bare CEL text and both envelope spellings still
evaluate; an `ast`-only envelope still answers `false` (that population is
#15430/#15807's); a well-formed `cron` envelope still answers `false` rather
than being refused; absent/`null`/empty/whitespace conditions are still "not
authored"; and a malformed STRING still earns the brace trap or the §1c CEL
fault, never the shape refusal.

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

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-automation, touching 2 documentable anchor(s).

2 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17.mdx (via AutomationEngine (symbol, a top-level class), evaluateCondition (symbol, a method of class AutomationEngine))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

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

Coarse fallback — 5 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 c14ebfeb29d8d3bc0ae9b148132a78b86dec94bcpackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json c14ebfeb29d8d3bc0ae9b148132a78b86dec94bc

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

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 7, 2026
@os-warren
os-warren marked this pull request as ready for review September 7, 2026 01:37
@os-warren
os-warren enabled auto-merge September 7, 2026 01:37
@os-warren
os-warren added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 5a91387 Sep 7, 2026
35 checks passed
@os-warren
os-warren deleted the claude/issue-16038-evaluate-condition-shape-refusal branch September 7, 2026 02:50
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