Skip to content

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

Draft
os-bill wants to merge 9 commits into
mainfrom
claude/issue-15807-flow-edge-condition-evaluated
Draft

feat(spec)!: FlowEdgeSchema.condition is an evaluated slot — composes EvaluatedExpressionInputSchema; structuralConditionRefusal drops the ast-only admission (#15807)#17267
os-bill wants to merge 9 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.tscondition: 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 testTest Files 471 passed (471) · Tests 13236 passed (13236) on both heads.
  • pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2127 passed · 1504 passed on both heads.
  • pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2103 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.tsSTALE 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 a256f18962970878a0215109ddb1c4f0357d105apackageMentionDocs.

Which tree this was computed on

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

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

⚠️ 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 a256f18962970878a0215109ddb1c4f0357d105a → 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_TIERVerdict: 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-62applyConversionsToStoredItem is deliberately not applied to 'flow' ("flows canonicalize at registerFlow instead"). The read path never re-validates a flow row; the door is registerFlowcanonicalizeStoredFlowFlowSchema.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:596no-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 sourceEvaluatedExpressionSchema, 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

  • F3packages/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.
  • F4objectstack 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 — cccf75fcb432e61114

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_TIERVerdict: 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 432e6111registered 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 stringsplugin.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_TIERVerdict: 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 sourcecustom 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

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

Labels

Projects

None yet

2 participants