Skip to content

feat(spec)!: FlowEdgeSchema.condition is an evaluated slot — composes EvaluatedExpressionInputSchema; structuralConditionRefusal drops the ast-only admission (#15807) - #17267

Merged
os-bill merged 10 commits into
mainfrom
claude/issue-15807-flow-edge-condition-evaluated
Sep 10, 2026
Merged

os-bill merged 10 commits into
mainfrom
claude/issue-15807-flow-edge-condition-evaluated

Conversation

@os-bill

@os-bill os-bill commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #15807

Clause-②: yes

What this does

FlowEdgeSchema.condition was typed ExpressionInputSchema.optional() — the persistence contract, source OR ast. AutomationEngine.evaluateCondition reads it as expression.source ?? '', so an ast-only envelope authored on an edge parsed, registered, passed objectstack validate, and then landed in the empty-source arm and answered a silent false: a branch that quietly never fired (measured on #15430, comment 5550509137). A whitespace-only source was the same seam through the other key. The silence is the defect; typing the key is the fix.

  • packages/spec/src/shared/expression.zod.ts — new EvaluatedExpressionInputSchema (+ type EvaluatedExpressionInput), the sibling of ExpressionInputSchema for an evaluated slot: the bare-string shorthand still normalizes to { dialect: 'cel', source } but must be non-blank after trimming; the envelope arm composes EvaluatedExpressionSchema (source required, non-blank) instead of ExpressionSchema. The union's error map answers EVALUATED_EXPRESSION_SOURCE_REQUIRED exactly when the refusal is the evaluated-slot rule (a blank string, an object with no string source) and yields to zod's default otherwise (a number, a dialect outside the enum) — measured, not assumed; the docblock records why both spellings land as invalid_union at the slot while a blank source inside an envelope lands as custom at source.
  • packages/spec/src/automation/flow.zod.ts — condition: EvaluatedExpressionInputSchema.optional(); the .describe() names the rule.
  • packages/spec/src/automation/flow-node-expression-paths.ts — the service-automation: refuse a structural flow condition that is neither CEL text nor an expression #15792 admission, revisited (below).
  • packages/spec/src/index.ts — the two new exports. Regenerated: api-surface/, export-origins/, declaration-map/, json-schema.manifest/shared.json, content/docs/references/** (the reference table for the edge now reads source: string, required).
  • ⛔ ExpressionSchema / ExpressionInputSchema are NOT narrowed. ⛔ start.config.condition is not typed here (no schema stands in front of the start node's open config).

How the #15792 admission was disposed of: removed

structuralConditionRefusal admitted an envelope through typeof rec.source === 'string' || rec.ast !== undefined. The ast clause existed only because the spec still admitted the shape at edge.condition (#15430 comment 5550509137 says so in as many words). With the edge schema closed, keeping it would have left the refusal deliberately holed for a shape the schema no longer admits on one surface and the engine cannot run on either — cel-engine.ts refuses AST-only evaluation, and evaluateCondition never reads ast.

So the clause is gone: an authorable envelope is one carrying a string source (an ast beside it is fine). STRUCTURAL_CONDITION_SHAPE_REFUSAL now reads "an expression envelope carrying a string source" and says why; the Found … detail names the ast-only case specifically. Consequence on config.condition (a start node's trigger gate, a decision node's predicate — an open record, so the structural pass is the only producer-side gate there): an ast-only envelope is refused at registerFlow, reported as a located error by objectstack validate, and refused by evaluateCondition with the same sentence, instead of answering a silent false. This is the one function both slots share, so the card's "revisit in the same PR" necessarily reaches that slot; the change is stated in the changeset. The whitespace-only STRING ruling on config.condition (#15662: consistent false on both sides) is untouched. packages/services/service-automation/src/engine.ts and packages/lint/src/validate-expressions.ts changed in comments only — the behaviour arrives through the spec dependency.

Premise re-derived on origin/main at ae19f5edb (BASE), before editing

BASE edge ast-only       ACCEPT  {"condition":{"dialect":"cel","ast":{"kind":"const","value":true}}, …}
BASE edge blank source   ACCEPT  {"condition":{"dialect":"cel","source":"   "}, …}
BASE edge blank string   ACCEPT  {"condition":{"dialect":"cel","source":"   "}, …}
CTRL ExpressionInput ast-only / blank   ACCEPT / ACCEPT

Controls read, not counted: ExpressionInputSchema in flow.zod.ts = the import line plus :567, the live condition key; EvaluatedExpressionSchema in expression.zod.ts = the export, its two type aliases and its docblock, i.e. the live schema, not a tombstone.

Consumer readings, pinned

door ast-only edge blank source edge (envelope / bare string) ast-only config.condition
FlowEdgeSchema / FlowSchema (spec) invalid_union at edges.0.condition, the published sentence custom at condition.source / invalid_union at condition n/a (open record)
registerFlow (service-automation) thrown at FlowSchema.parse, sentence + path same thrown by the structural pass, STRUCTURAL_CONDITION_SHAPE_REFUSAL + "Found an object carrying an ast but no string source"
evaluateCondition (service-automation) never reached for an edge never reached throws the same sentence — no silent false
objectstack validate (lint, runAuthoringRules on the raw path) structural error at edge 'e1' (schema tier) structural error at node 'start' / node 'branch'
objectstack validate (cli, step 2 = ObjectStackDefinitionSchema.safeParse) invalid_union at flows.0.edges.0.condition same shape n/a

The CLI reading is pinned at the spec level on the very call packages/cli/src/commands/validate.ts makes (ObjectStackDefinitionSchema.safeParse, then formatZodErrors); the CLI e2e layer is declared to CI.

Tests and gates

Pre-merge on 451c15e03 (branch base ae19f5edb), then again on the final head 64769f0a3 (after bash scripts/pm/os-regen-merge.sh brought 6 commits from origin/main; check:generated "All 15 generated artifacts are up to date"), all under scripts/pm/os-verify-lock.sh, verdicts read from its VERDICT command-exit line:

  • pnpm --filter @objectstack/spec test — Test Files 471 passed (471) · Tests 13236 passed (13236) on both heads.
  • pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 — 127 passed · 1504 passed on both heads.
  • pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 — 103 passed · 3681 passed pre-merge, 103 passed · 3707 passed on the final head (main brought lint tests).
  • pnpm --filter … typecheck for spec, service-automation, lint on 64769f0a3 — exit 0, check:test-typecheck: OK for all three.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 107 families on 64769f0a3; all 107 run with exit codes written to disk first; --ran reconciles 107 derived, 107 run, 0 NOT-MEASURED, 0 UNRUN. 104 exit 0. check:skill-examples was exit 1 for a missing client-react dist, re-run green after building the client closures. NOT MEASURED (PREREQUISITE NOT MET, exit 3, both need every package's dist/): check:dual-build-cjs-loads, check:type-check-debt — declared to CI's Lint & Repo Gates.
  • Changeset gates on the branch: check-adr-0087-registration ✓ ([BREAKING+bang] not-required (no-migration-prescription)), check-changeset-no-major ✓, check-empty-changeset ✓.

After the first CI run — one red that was mine, one that was not

  • Mine, fixed on cccf75fcb. Dogfood Regression Gate (3/3) on 64769f0a3 failed in packages/qa/dogfood/test/expression-conformance.test.ts — STALE covers — surface no longer in source: automation/flow.zod.ts:FlowEdgeSchema.condition. The ADR-0058 D7 ledger discovers expression surfaces by a roster of input-schema names, and its own docblock says a new narrowed alias belongs in that roster on the commit that introduces it (Narrow the settings-manifest visible declaration to the grammar the save-time evaluator actually implements (#7169 alignment half, measured 1-vs-93) #7327's shape). EvaluatedExpressionInputSchema is now listed; the ledger test is green locally (Tests 5 passed (5), after building @objectstack/verify) and on CI. That CI failure is the red control for the roster row.
  • Not mine, flaky. Test Core (4/6) on cccf75fcb failed once in @objectstack/driver-turso src/turso-driver-timeout.test.ts:122 ("a stalled remote fails the operation within the configured window, as TIMEOUT / 504" — expected 0 to be greater than 0 on remote.requests()). No file in this PR is under packages/drivers/, the same shard passed on 64769f0a3 with the identical tree outside packages/qa/dogfood/test/, and a single rerun-failed-jobs passed. Reported on the card, not "fixed" here.
  • Final head cccf75fcb: gates re-derived (same 107 families), all 107 re-run with exit codes to disk, --ran reconciles 107 run, 0 NOT-MEASURED, 0 UNRUN; same two PREREQUISITE NOT MET rows (check:dual-build-cjs-loads, check:type-check-debt) as above. CI, newest run per check name: every required context green — Lint & Repo Gates, TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL) all success.

Clause ② — measured on the finished diff

node scripts/pm/check-widening-tells.mjs --declaration no --diff pr.diff → exit 4, 7 tells: T3 ×4 (two new rows on the root and shared published entry points: EvaluatedExpressionInputSchema, EvaluatedExpressionInput) and T2 ×3 (the new union itself, read as a closed set gaining members). With --declaration yes → exit 0. So the measured direction is yes — the public surface grows by one schema and one type. It is NOT yes for the reason the claim feared: nothing becomes newly acceptable anywhere; the edge slot's accept set only shrinks, and the T2 hits are the new schema's own two arms, not a member added to an existing set. The claim's provisional yes stands; the seat need not move it.

Changeset

.changeset/flow-edge-condition-evaluated-slot.md — @objectstack/spec: minor (the launch-window convention; major is refused by check-changeset-no-major), a **BREAKING** banner in the accept-set sense, and an ADR-0087 not-required (no-migration-prescription) disposition: no key is renamed, retired or re-typed; both refused spellings never evaluated on any release; the census re-run for edges over examples/ packages/ content/ skills/ at ae19f5edb found 0 ast-only and 0 blank-string edge conditions against a lit control of 1 (the record.amount < 500 example in flow.zod.ts).

验收备注

  • noted, not filed: the whitespace-only STRING on config.condition is still admitted by structuralConditionRefusal (a consistent false on both sides, service-automation: evaluateCondition answers a silent false for a non-string predicate, and a non-string config.condition registers clean #15662's ruling), while the edge now refuses it at parse. Two slots, two rulings, both stated in code; a one-rule flip is a ruling, not a lane's call. 承接者:无.
  • noted, not filed: EvaluatedExpressionSchema's docblock promises "exactly one issue, at source" for every refused shape; through the input union the ast-only and blank-string spellings surface as invalid_union at the slot (both arms abort). Recorded in EvaluatedExpressionInputSchema's docblock and pinned; the bare schema's promise still holds for direct callers. 承接者:无.
  • noted, not filed: packages/lint/src/validate-org-axis-red-lines.ts expressionText reads rec.ast for SharingRule.condition — that slot is the persistence contract and is scanned, not evaluated; correct as is. 承接者:无.
  • check:dual-build-cjs-loads and check:type-check-debt cannot be measured without a full package build; CI owns that run.

Generated by Claude Code


Generated by Claude Code

…Schema; structuralConditionRefusal drops the ast-only admission (#15807)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
… json-schema manifest and docs for EvaluatedExpressionInputSchema

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/lint, @objectstack/service-automation, @objectstack/spec, touching 11 documentable anchor(s). ⚠️ 7 changed file(s) yielded no anchor (packages/spec/api-surface/root.json, packages/spec/api-surface/shared.json, packages/spec/declaration-map/shared.json, …), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/api/error-catalog.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/api/index.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/concepts/metadata-lifecycle.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/data-modeling/drivers.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/data-modeling/objects.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/deployment/cli.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/deployment/environment-variables.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/deployment/validating-metadata.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/kernel/cluster.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/kernel/contracts/metadata-service.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/kernel/services-checklist.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/permissions/authorization.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/permissions/permission-sets.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/plugins/packages.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))

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

  • content/docs/releases/implementation-status.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class), validateStackExpressions (symbol, a top-level function), sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/releases/v17/17-0.mdx (via AutomationEngine (symbol, a top-level class), FlowEdgeSchema (symbol, a top-level const), sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/releases/v17/17-3.mdx (via sys_metadata (literal, a string literal in semantic; a string literal in surface))
  • content/docs/releases/v17/17-4.mdx (via STRUCTURAL_CONDITION_SHAPE_REFUSAL (symbol, a top-level const object))

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
  • 7 changed file(s) yielded no anchor (packages/spec/api-surface/root.json, packages/spec/api-surface/shared.json, packages/spec/declaration-map/shared.json, …) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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 — 134 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 8c9bd8f51fe56bd32fa90a23aefda20c45ec496e → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 8c9bd8f51fe56bd32fa90a23aefda20c45ec496e

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

…surface discovery roster so FlowEdgeSchema.condition stays discovered (#15807)

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

os-bill commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Contract review at CONTRACT_REVIEW_TIER — Verdict: REWORK (adopted by the domain:spec seat)

Head reviewed cccf75fcbaebe2f4181dafd46bafbbb30c4ef69c · card #15807 · merge-base 5a95b0e93 · adopted 2026-09-10T02:47Z.

The code is sound. Both must-fix findings are on the changeset and the migration registry, none on code. ⛔ Adopted verbatim, ⛔ not softened: a REWORK is not downgraded because the diff is good.

  • Tier fuse — ⛔ a subagent's claim about its own tier is never a reading: 114 harness-stamped "model":"claude-fable-5-1", zero residue, over an 883 KB transcript, with a lit control ("role":"assistant" = 98, "role":"user" = 68).
  • ⚠️ Independence, stated at the strength the evidence supports: the reviewer reported its own session id and it is this seat's own — a subagent inherits the dispatching session's id. So this is a seat-side at-tier review, ⛔ not the cross-seat independence a director-seat review has. The reviewer volunteered that itself.

F1 — must-fix — the stored-flow blast radius is unstated

⭐ This is the finding that turns a good PR into a REWORK, and it is bigger than the edge.

Measured: packages/spec/src/conversions/stored.ts:57-62 — applyConversionsToStoredItem is deliberately not applied to 'flow' ("flows canonicalize at registerFlow instead"). The read path never re-validates a flow row; the door is registerFlow → canonicalizeStoredFlow → FlowSchema.parse. At boot, packages/services/service-automation/src/plugin.ts:949-963, :1947-1956 and :1992-2000 each wrap registerFlow in try/catch, emit ctx.logger.warn('[Automation] failed to register flow', …) and continue.

So for a sys_metadata flow whose edge carries { dialect:'cel', ast } or a blank source:

before this diff after
that edge answers a silent false —
the rest of the flow trigger armed, everything else runs ⛔ the whole flow is skipped at boot, trigger never armed
what the operator sees nothing one warn line

The changeset's remedy sentence — "author a source, which the refusal itself prescribes" — is true for an author at objectstack validate / POST /flows, and false for a stored row, whose only refusal is that warn. Fix: one paragraph saying so.

F2 — must-fix — no-migration-prescription is the wrong disposition, by ADR-0087's own words and by this lane's own precedent

  • docs/adr/0087…md:596 — no-migration-prescription "is refused by a body that carries a migration prescription". The changeset's "What an author does with a refused edge condition" paragraph is a prescription in everything but the FROM → TO label. ⚠️ check-adr-0087-registration passes (exit 0 locally and in CI) because it detects a FROM/TO block, not the substance — and feat(spec)!: an evaluated expression slot requires a non-blank source — EvaluatedExpressionSchema, composed by the assignment value envelope (#15430) #15810 on this same lane once had to reword a label to get past it, which is the tell that the gate's letter and the ADR's meaning have separated.
  • D2 is genuinely unavailable (no lossless mapping: ast-only has no source; dropping a blank condition flips the edge from never-fires to always-fires). That is an argument for a D3 semantic entry, which exists for exactly "non-lossless change; structured TODO" — ⛔ not for the exemption.
  • ⭐ The precedent is three days old and on this lane: 233222e2e (driver-memory analytics silently accepts an unparseable dateRange and matches EVERY row — and the platform's own documented spelling 'Last 7 days' is one of them #16041, 2026-09-07) closed timeDimensions[].dateRange's string arm — an accept-set narrowing with no D2 conversion possible — and registered a semantic entry anyway, its reason saying in as many words "⚠️ No D2 conversion and no stored-metadata rewrite". That PR's value was query-time, explicitly "not a sys_metadata shape". This one is a sys_metadata shape. The weaker case registered; the stronger one is claiming the exemption.
  • The in-repo census (0/0, control lit) is a repo reading. The changeset uses it to conclude migrate meta "has nothing to rewrite" — conflating nothing to rewrite mechanically (true, D2 is impossible) with nothing to notify (false, exactly the F1 case).

Fix: one semantic entry in packages/spec/src/migrations/registry.ts in the #16041 shape, marker flipped to adr-0087: registered <id>, projections regenerated.

⛔ This seat does not take the alternative on offer. The reviewer noted the seat could instead rule that launch-window narrowings with a zero repo census keep no-migration-prescription. That ruling would contradict #16041 and would be a decision about how ADR-0087's categories apply — ⛔ not a seat's call, and ⛔ not one to make implicitly inside a changeset. Registering the entry is the conforming action and the one the precedent already took.

⚠️ One claim inside the verdict that does NOT check out — corrected rather than propagated

F2 says packages/spec/src/migrations/registry.ts "was named a held single-writer path on this lane (#15430 claim 5549282927)". Measured, both legs fail:

  • scripts/check-single-claim-paths.mjs SINGLE_CLAIM_PATHS declares exactly one path — .objectui-sha — and the file's own self-test pins the list at ≤ 5 entries precisely so it stays a declaration. migrations/registry.ts is not on it, which is why the No other open PR may claim the same single-writer path check is green on every PR here.
  • Claim 5549282927's file face names packages/spec/src/automation/builtin-node-config.zod.ts and a sibling EvaluatedExpressionSchema — ⛔ not the registry. And spec/formula: ExpressionSchema accepts an ast-only envelope that no engine can evaluate — it validates, it registers, it faults at run time #15430 itself is pm:blocked, unassigned, untouched since 2026-09-05, so nothing is actively holding anything.

⭐ The action the finding prescribes is still right, and I have the contention measured independently: PR #17146 (awaiting the maintainer's hand) and PR #17257 (green, held) both touch packages/spec/src/migrations/registry.ts. It is generated and ⛔ not routed to merge=os-regen, so a clash is an ordinary visible conflict, not a silent drop. The rework order carries the sequencing; the seat owns landing order.

I adopt verdicts verbatim — but a factual claim inside one that I can falsify gets corrected here, not carried forward.

Non-blocking, folded into the same round where cheap

  • F3 — packages/spec/src/shared/expression.zod.ts:147-152 still promises "exactly one issue, at source, for every refused shape". True for direct callers, false once composed through the new union, where ast-only and blank-bare-string surface as invalid_union at the slot. The new docblock records it; the old one promises the opposite. One sentence. In the round.
  • F4 — objectstack validate refuses an ast-only edge with EVALUATED_EXPRESSION_SOURCE_REQUIRED (CLI schema step) and with STRUCTURAL_CONDITION_SHAPE_REFUSAL (lint's raw-input path). Both prescribe source. Recorded, no fix now.
  • F5 / F6 — see the ACCEPT list.

What the review confirmed, so it is on the record

Ruling executed and the three doors agree; the removal of #15792's rec.ast !== undefined admission is in scope by the card's own text ("Revisit #15792's admission in the same PR") — the dispatch's fence was on typing the open record, which the diff does not do, so ⛔ the fence is not crossed. Clause ② yes reproduced independently (7 tells read one by one: T3 ×4 are the two new published rows, T2 ×3 are the new union's own members, ⛔ not members added to an existing set). Level and banner coherent with the launch-window convention. ADR grep across the naming ADRs: no accepted ADR reversed — ADR-0032 :95/:102 writes the IR as {dialect, source, ast?}, which this diff is consistent with, and the "M9.2 makes ast required" promise lives only in a docblock, in no ADR (grep -l "M9\.[12]" docs/adr → 0). Scope exact at 23 files, nothing unauthorised. The dogfood roster re-registration is real, with its own red-then-green as the control. Both NOT-MEASURED gates are owned by green CI jobs — ⛔ not gaps.

⚠️ And a correction to my own vocabulary: I have been calling #17121 "the known turso flake" in briefs. The reviewer is right that this overstates a ledger status it does not have — #17121 is an open p1 with its real-defect hypothesis unresolved and no queue-flake-anchor. The causation reading here still holds (this diff touches 0 files under packages/drivers/, the failing signature is byte-identical, the rerun is green), but ⛔ "known flake" is not a status I get to assert.

File at ACCEPT (⛔ not folded into this PR)

what 承接者
config.condition's whitespace-only string is still a silent false on both doors while the edge now refuses the same blank at parse — one shared structuralConditionRefusal, two rulings. Needs a ruling, ⛔ not a lane's call domain:services seat (the #15662/#15792 lineage)
objectui FlowDesignerEdge.condition imports ExpressionInput so it cannot describe a spec-rejected condition — after this PR the mirror is wider than the server again, the exact drift its docblock claims to close objectui / domain:ui seat, at pin-bump time, with Blocked-by:
The "M9.2 — ast becomes required in build output" promise lives only in a docblock and is now load-bearing for two repos' pins. Decide it in an ADR (an 0032 amendment) or drop it domain:spec seat
#17121 — a second cross-PR hit of the signature (already dispatched, ⛔ not re-filed) domain:engine

⛔ needs:contract-review stays on the PR and the card. ⛔ The PR stays draft. The rework round follows.


Generated by Claude Code

…edge condition, and state the stored-flow blast radius (#15807)

Contract-review REWORK on this branch. No code change: the schema, the three
doors, the pins and the roster are untouched.

F1 — the changeset's remedy sentence was true for an AUTHOR and false for a
row already in `sys_metadata`. `applyConversionsToStoredItem` is deliberately
not applied to `flow` (conversions/stored.ts; same skip in the database
loader's `rowToData`), so the read path never re-validates a stored flow; the
door is `registerFlow` -> `canonicalizeStoredFlow` -> `FlowSchema.parse`, and
all three boot paths in service-automation's plugin wrap it in try/catch, warn
and continue. A stored flow carrying a refused edge condition therefore stops
being registered ENTIRELY -- trigger never armed, whole flow dark, announced by
one warn line. Stated as its own paragraph.

F2 — `no-migration-prescription` is refused by a body carrying a migration
prescription (ADR-0087), and the "What an author does" paragraph is one. D2 is
genuinely impossible (an `ast`-only envelope has no `source` to derive; dropping
a blank condition flips the edge from never-fires to always-fires), which is the
argument FOR a D3 semantic TODO rather than for the exemption. Registered in the
shape of the three-day-old precedent on this lane, #16041's
`analytics-time-dimension-date-range-vocabulary-closed`, which also had no D2
available and registered anyway. Marker flipped to `registered`.

The entry is authored as a FILE under migrations/entries/semantic/ and the
registry regions regenerated -- those regions are generated (#7297/#6957) and a
hand edit between the markers is reverted by the next gen run. Both projections
correctly show NO diff: they fold only up to PROTOCOL_MAJOR (17) and this is a
major-18 entry -- controlled by the precedent entry being absent from them too,
against a lit control of a major-17 id that is present.

F3 — `EvaluatedExpressionSchema`'s "exactly one issue, at `source`, for every
refused shape" is true only for a direct caller; composed through
`EvaluatedExpressionInputSchema` the `ast`-only and blank-bare-string shapes
surface as one `invalid_union` at the slot. One sentence scoping the promise.

Level unchanged and re-derived, not inherited: `@objectstack/spec` `minor` plus
the **BREAKING** banner. The disposition marker records HOW a break reaches the
upgrade channels; it is not an input to the semver level.

Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
Co-authored-by: Claude <noreply@anthropic.com>

os-bill commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Rework round landed — cccf75fcb → 432e61114

Addressing the REWORK verdict in comment 5611806589. No code change: schema, the three doors, the pins and the roster are untouched, as the verdict directed.

F1 — new paragraph in the changeset: a flow already stored in sys_metadata with a refused edge condition is no longer registered at boot, so the whole flow goes dark, not just the edge. Each leg re-verified here rather than inherited — including a runtime probe (ast-only → one invalid_union at edges.0.condition; non-blank source → zero issues on edges.*, the lit control). One correction to the suggested wording: the three boot sites emit three different warn strings ([Automation] failed to register flow, ... flow re-sync: ..., ... cold-boot flow bind: ...), so all three are named.

F2 — registered the D3 semantic entry flow-edge-condition-evaluated-slot-source-required, matching #16041's analytics-time-dimension-date-range-vocabulary-closed (same five fields, same no-backticks-in-surface discipline, same explicit "no D2 possible" sentence in reason). Marker flipped to registered. Authored as a new file under migrations/entries/semantic/ — those registry regions are generated, so a hand edit between the markers would have been reverted.

Both projections correctly show no diff: they fold only to PROTOCOL_MAJOR (17) and this is major 18. Controlled — the precedent entry is absent from them too, against a lit control of a major-17 id that is present. Nothing hand-added.

F3 — one sentence scoping EvaluatedExpressionSchema's "exactly one issue at source" promise to direct callers.

Level re-derived, unchanged: @objectstack/spec minor + BREAKING. The disposition marker records how a break reaches the upgrade channels; it is not an input to the semver level.

Merged origin/main (9c52b1bd3) before writing the entry — no conflict, registry untouched, so neither #17146 nor #17257 had landed. One merge commit plus one work commit; no rebase, no amend, no force-push.

Gates green locally: check:migration-registry, check-adr-0087-registration (post-commit — it reads committed blobs, so an uncommitted marker flip greens on the old bytes), check:spec-changes, check:upgrade-guide, check:generated, plus 208 spec tests and typecheck. check-governed-merges --test over all 25 paths: NOT governed. check-clause2-carriers --pair 17267: exit 0, both carriers agree.

PR stays draft; needs:contract-review untouched on both PR and card. Full report on the card: #15807.


Generated by Claude Code

os-bill commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Delta review at CONTRACT_REVIEW_TIER — Verdict: PASS WITH FINDINGS (adopted). ⛔ No must-fix; one nit is being fixed anyway, and here is why

Head 432e6111462bcfcd52b239ff66550ce75972b02f · delta cccf75fcb → 432e61114 (merge f17c3a435, combined diff empty ⇒ clean merge with no hand resolution, + one work commit, 4 files, +169/−1) · adopted 2026-09-10T03:28Z.

  • Tier fuse — 85 harness-stamped "model":"claude-fable-5-1", zero residue, over 650 KB, control lit (assistant = 72, user = 47).
  • ⚠️ Seat-side at-tier review, ⛔ not cross-seat independence — the reviewer reported this seat's own session id and said so itself.

The two must-fix findings are closed on the tree

  • F1 — the stored-flow blast radius is now stated. The dev's extra measurement is confirmed: the read-path skip is enforced at the call site, not merely documented — packages/metadata/src/loaders/database-loader.ts:824 returns the payload before applyConversionsToStoredItem when singular === 'flow'. Door chain confirmed end to end.
    ⭐ And a completeness argument, which is what makes this trustworthy rather than merely present: the only other registerFlow call sites are packages/runtime/src/domains/automation.ts:1684 (POST), :1976 (clone), :2589 (PUT) — HTTP author doors that answer a 400. There is no fourth stored-row door. ⇒ "three boot paths" is the whole stored-row story, not a sample.
  • F2 — the D3 entry matches the driver-memory analytics silently accepts an unparseable dateRange and matches EVERY row — and the platform's own documented spelling 'Last 7 days' is one of them #16041 precedent field for field, and ⭐ the disposition flip was proven on committed bytes with a control: the same script at two heads printed two dispositions — --head 432e6111 → registered flow-edge-condition-evaluated-slot-source-required, --head f17c3a435 (the pre-flip merge) → not-required (no-migration-prescription). The not-interchangeable warning is present verbatim (authoring a source = never-fires until authored; deleting the key inverts the edge to always-fires), repeated as "do not default to removal" in acceptanceCriteria. Registry insertion-only, generated region byte-equal to the entry modulo indent, in sorted position.
  • F3 folded, accurate against the union's pins. Projections zero with the control read (major-17 action-descriptor-is-async-retired lit in both files; both major-18 ids absent from both). Scope exact — the schema, the three doors, the pins and the dogfood roster untouched. Level re-derived, minor + **BREAKING** stands.

⛔ A correction to the standing verdict, which I adopted

Its F1 said the three boot sites each emit ctx.logger.warn('[Automation] failed to register flow', …). They emit three different strings — plugin.ts:960 (start()), :1951 ([Automation] flow re-sync: …, from the metadata:reloaded hook), :1997 ([Automation] cold-boot flow bind: …, the kernel:ready bind). An operator grepping the one I passed on would have missed two of three paths. The dev caught it, the rework names all three and maps each to its moment, and the reviewer re-confirmed it. Recorded so the record is right, ⛔ not smoothed over.

New finding F1 — non-blocking, and ⛔ I am fixing it anyway

.changeset/flow-edge-condition-evaluated-slot.md:96-99, mirrored in the entry's acceptanceCriteria, tells the operator to "Run objectstack validate on the exported stack". ⚠️ There is no export verb in the CLI. I verified this myself rather than adopting it: git grep for objectstack export|meta export|exported stack across packages/cli/src and content/docs returns nothing but this changeset, with a lit control (loadConfig = 4 hits in validate.ts) proving the grep speaks; packages/cli/src/commands/meta/ offers get|list|register|delete|resync. validate consumes a config file, not a database.

⇒ For the population this paragraph is written for — a Studio-authored row that exists only in sys_metadata — the instruction points at a command that does not exist. The route that does exist is the warn line itself, which already names the edge: describeThrownForLog (thrown-cause-diagnostics.ts:160-185) reshapes the ZodError into issues[].path, rendered edges[0].condition by formatIssuePath.

Why a non-blocking nit is getting a round: this text ships to consumers in CHANGELOG.md, and the same phrase is in the ledger entry an upgrading agent greps. It is the same standard I applied to the "Director ruling" misattribution on the sibling PR — ⛔ shipping prose that sends its reader nowhere is not a wording preference. The fix is one clause plus a gen:migration-registry re-run; the reviewer offered changeset-only, and ⛔ I am not taking that, because leaving the false locator in the ledger keeps it in front of the reader who most needs it.

F2 (path spelling: edges.N.condition vs flows.N.edges.N.condition at the CLI door) is substring-true and harmless — folded into the same round since the file is open.

Landability at this head

check reading
① at-tier verdict for THIS head this document — but the F1 round moves the head, so a short delta re-check follows
② --pair 17267 exit 0
③ every check green, newest-run-per-name ⛔ not yet — 5–7 still running at 03:13Z (two independent reads), 0 red

⛔ needs:contract-review stays on the PR and the card. ⛔ PR stays draft. ⚠️ #17121 is an open p1 with an unresolved hypothesis and no queue-flake-anchor — ⛔ not a "known flake"; if Test Core (4/6) reds on that signature it is a causation question for this seat, ⛔ not a content finding.

At ACCEPT — unchanged, nothing new

config.condition's whitespace-only string (承接者 domain:services); objectui's FlowDesignerEdge.condition mirror, now wider than the server (承接者 objectui / domain:ui, at pin-bump, with Blocked-by:); the "M9.2 — ast required in build output" promise that lives only in a docblock (承接者 domain:spec); #17121 (already dispatched). Plus the PR body's doubled attribution footer — a platform append, ⛔ not to be "fixed" by re-sending a body that already carries one.


Generated by Claude Code

….path`, not a CLI verb that does not exist (#15807)

The stored-flow paragraph is written for the operator whose row exists only in
`sys_metadata` — a Studio-authored flow with no config file. It told that reader
to "Run `objectstack validate` on the exported stack". There is no export verb
in the CLI: oclif discovers commands from `dist/commands/**`, which holds no
`export`; `packages/cli/src/commands/meta/` offers `get|list|register|delete|
resync`; and `validate` consumes a config file (`validate.ts` `loadConfig`), not
a database. `git grep 'objectstack export|meta export|exported stack'` over
`packages/cli/src` and `content/docs` returns only the module-export sense at
`utils/config.ts:352`, against a lit control (`loadConfig` = 4 in `validate.ts`,
`objectstack validate` = 8 doc files, `objectstack export` = 0).

The route that does exist is the warn line the paragraph already names: all
three boot sites spread `describeThrownForLog(e)` into the log meta, which
reshapes the ZodError into `issues[].path` and renders it through
`formatIssuePath` — numeric segments become `[i]`, so the offending edge reads
`edges[N].condition`. `objectstack validate` is kept only where it is true: for
a stack authored in config files.

Folds in the path-spelling nit at the same time. Each spelling now matches the
door it describes: the CLI prints `path.join('.')` over an
`ObjectStackDefinitionSchema` parse, so its reading is
`flows.N.edges.N.condition` (pinned, `flow-edge-condition-evaluated.test.ts`),
while the warn renders `edges[N].condition`.

Both carriers move — the changeset and the D3 entry's `acceptanceCriteria` —
because leaving the false locator in the ledger keeps it in front of the reader
who most needs it. `gen:migration-registry` re-run; the generated region is the
entry's text modulo indent. No code, schema, door, pin or roster changes, and
the not-interchangeable warning is untouched.

Level re-derived and unchanged: `@objectstack/spec` `minor` + **BREAKING** +
`adr-0087: registered flow-edge-condition-evaluated-slot-source-required`.
Editing prose inside a `reason`/`acceptanceCriteria` string and a changeset
paragraph moves nothing published and changes no break.

Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
Co-authored-by: Claude <noreply@anthropic.com>

os-bill commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Delta review at CONTRACT_REVIEW_TIER — Verdict: PASS WITH FINDINGS (adopted). ⛔ No further round, and the reason is a distinction worth stating

Head 496cdfa47347b7bc7ed21ad3c17a0dbcbae0041f · delta 432e61114 → 496cdfa47 (one commit, 3 files, +27/−15) · adopted 2026-09-10T03:47Z. No must-fix.

  • Tier fuse — 89 harness-stamped "model":"claude-fable-5-1", zero residue, control lit (assistant = 74, user = 46).
  • ⚠️ Seat-side at-tier review, ⛔ not cross-seat independence — the reviewer reported this seat's own session id and said so.

⛔ Why this PR does NOT get a fourth round, when the last two findings did

I ordered rounds on this PR for the "Director ruling" misattribution and for the "exported stack" imperative. Both were false statements shipping to consumers — one misattributed whose decision it was, the other told an operator to run a command that does not exist.

F1 and F2 here are not false. F1's claim ("the warn names the offending edge as edges[N].condition") is exact for two of the three refused spellings and, for the third, lands at edges[N].condition.**source** — which still names the edge index, so the locator claim holds. F2 is a redundancy the reviewer explicitly judged non-misleading. ⇒ ⛔ Ordering another round here would be churn, and it would blur the standard I have been applying: a round buys the removal of a falsehood, ⛔ not the polishing of an imprecision. Both are recorded below for whoever next opens those files.

What the review measured, beyond reading the code

⭐ A runtime probe at the head, not a code read: ast-only → invalid_union at edges[1].condition; blank bare string → same; blank envelope source → custom at edges[1].condition.source; control non-blank source → 0 issues. And it followed the value all the way to the operator: FlowSchema.parse inside canonicalizeStoredFlow has no try/catch, registerFlow calls it bare, and core/src/logger.ts:337-375 JSON-stringifies the meta onto the same physical line in all three formats. ⇒ The warn line really is a locator, in the log an operator actually reads.

⭐ The narrowing holds on measurement, and the reviewer went further than confirming it: a data-export sense of "stack" does not exist in this repo — a grep for export (your|the|a) stack|exported stack|export metadata|metadata export over content/docs + packages/cli/src returns only config.ts:352 (module sense), the only --export in the CLI docs is os secret orphans --export (a secrets backup), and export default defineStack appears in 32 docs files as the documented authoring idiom. It also found the nearest hand route — os meta get flow <name> --format json against a live server, hand-wrapped and fed to validate by explicit path — and confirms that is a workaround, not a verb, so the entry's "no CLI verb" clause stands as written.

⭐ The staleness I asked it to price, priced properly rather than waved through: it ran main's check-tier-file-adoption.mjs from inside the head tree (exit 0; the PR adds no e2e/live test file), ran main's check-widening-tells.mjs on the merge-base..head diff (--declaration yes exit 0, --declaration no exit 4 with the same 7 tells the PR body lists ⇒ the Clause ② reading does not move), and noted CI already runs on the merge commit. ⇒ The 5-commit staleness hides nothing for this file set.

Everything else re-derived clean: registry regeneration self-test + --check exit 0 and the entry body 68/68 lines identical to its generated block; projections 0 with the control read in both files (⭐ and it checked that acceptanceCriteria is a projected field — 87 Done when: lines in the guide — so the zero is meaningful, not vacuous); ADR-0087 disposition proven post-commit; level unchanged; the not-interchangeable warning and expression.zod.ts byte-identical.

Findings — both non-blocking, recorded ⛔ not actioned

  • F1 — entry :72-73 / changeset :97-98: add "(or edges[N].condition.source when the envelope's source is blank)" if a round ever opens on these files. ⛔ Not worth one alone.
  • F2 — entry :57: "exported stacks" beside "defineStack({ flows }) sources" names one artifact twice in the module sense. True, and not misleading given :66-68. Optional.

Landability at this head

# pre-check reading
① at-tier verdict for THIS head PASS WITH FINDINGS, no must-fix
② --pair 17267 exit 0, re-derived by the reviewer at 03:43Z
③ every check green, newest-run-per-name 31 success / 2 skipped / 0 red, 1 still running (Lint & Repo Gates) at 03:46Z

⇒ Landable the moment ③ converges. ⚠️ Test Core (4/6) — the shard carrying #17121's signature — is green on this head; #17121 remains an open p1 with fix PR #17282 unmerged, so it is a queue-ejection risk on the merge-queue full-suite run, ⛔ not a finding here.

⛔ But it is not being enqueued yet, and the reason has just doubled in cost

This PR touches packages/spec/src/migrations/registry.ts too (measured: it is in the 25-path diff). So it falls under the same hold as PR #17257: PR #17146 is a governed PR sitting with the maintainer for a hand merge and shares that file.

⇒ Two PASSed, green, landable PRs are now held on one human action, where an hour ago it was one. That changes the arithmetic, and the seat's recommendation with it — stated on #17146 rather than decided quietly here.


Generated by Claude Code

@os-bill
os-bill marked this pull request as ready for review September 10, 2026 05:52
@os-bill
os-bill added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 53ec0b1 Sep 10, 2026
39 checks passed
@os-bill
os-bill deleted the claude/issue-15807-flow-edge-condition-evaluated branch September 10, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants