fix(metadata-protocol): a flow save that skipped canonicalization says so (#4580) - #4582
Merged
Merged
Conversation
…s so (#4580) `saveMetaItem` canonicalizes flow bodies before the schema gate (#4542). When the canonicalizer throws — it is stricter than the gate: strict parse, cycle detection, control-flow region validation — the save falls back to the raw body so a WIP draft with a temporary cycle stays saveable. That fallback is correct and unchanged here. It was also completely silent. Three of the four postures at this seam announce themselves: a clean pass heals the row, a refused rename fails with 409 FLOW_CONVERSION_CONFLICT naming the token, and a host with no automation service is reported by `os migrate meta --stored`. The throw-fallback said nothing — so a save that skipped canonicalization was indistinguishable from one that healed the row, and a body that is BOTH a legacy dialect and unparseable re-persisted verbatim. That is the #4542 symptom arriving silently, against a boot warning that tells the author re-saving is the remedy. The fallback now warns, naming the flow and the canonicalizer's own error, deduped once per flow per process (the `convertStoredItem` pattern — Studio autosaves the same draft repeatedly and a WIP cycle throws on every write). This aligns the write seam with ADR-0087 D2's "loud" posture. No behavior change: the body still saves, the gate stays the arbiter, and `registerFlow` still refuses to arm a malformed flow. Refusing the save in publish mode was considered and rejected — publish is the default mode, so it would silently tighten validation for every existing caller, and it can only be enforced where an automation service exists, making the same body saveable on a control-plane host and a 422 on an automation host. Tests: 5 new cases in protocol.save-flow-canonicalization.test.ts — warns with the flow name and reason; deduped across repeat saves; silent on the clean, conflict, and no-service paths. Full metadata-protocol suite green (206). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDmJ5ASMSzcw1q17vh32MG
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4580. Follow-up to #4542 / #4548.
What
saveMetaItemcanonicalizes flow bodies before the schema gate (#4548). Four postures live at that seam — three announce themselves, one said nothing:--storedreports canonical409 FLOW_CONVERSION_CONFLICTnaming token + pathconsole.warn, deduped per flowos migrate meta --storedreports itThe fallback itself is correct and is unchanged by this PR:
canonicalizeStoredFlowis stricter than the gate (strict parse, cycle detection, control-flow regions), so a WIP draft with a temporary cycle must stay saveable — in publish mode as well as draft. What was wrong is that it was invisible.Why it mattered
A save that skipped canonicalization was indistinguishable from one that healed the row. A body that is both a legacy dialect and unparseable by the strict canonicalizer re-persisted verbatim — the exact #4542 symptom, arriving silently, while the boot warning for legacy stored rows tells the author that re-saving is the remedy. The author found out only by running
os migrate meta --storedand seeing the rowfailed.It was also the one quiet link in a chain that is otherwise loud by design (ADR-0087 D2): conversions emit notices,
convertStoredItemwarns once per row on read,migrateStoredMetadatareportsfailedwith the message.What changed
One
catchblock inpackages/metadata-protocol/src/protocol.tsnow warns with the flow name and the canonicalizer's own error, plus aflowCanonicalizeFallbackWarneddedupe set alongside the existingstoredConversionWarned— Studio autosaves the same draft repeatedly and a WIP cycle throws on every write, so an undeduped warning would be a spam loop.No behavior change: the body still saves, the schema gate stays the arbiter,
registerFlowstill refuses to arm a malformed flow.Decision carried forward from #4548
Refusing the save in publish mode was reconsidered here and rejected again, for a reason worth recording:
publishis the default mode, so refusing would silently tighten validation for every existing caller — a breaking change for bodies that save fine today.resolveFlowCanonicalizer()returnsundefinedand there is no strict parse to run — so the same body would save on one host and 422 on another. That is unlike the 409, which reflects genuine environment state (a token owned by a live executor); a 422 should depend on the body alone.Observability was the actual gap, and this closes it.
Tests
5 new cases in
protocol.save-flow-canonicalization.test.ts(16 total in the file):--storedremedyAssertions filter warnings by message rather than counting all
console.warncalls, so unrelated one-shot protocol warnings (e.g. #3770's) can't make them order-dependent.Full
metadata-protocolsuite green (206 tests, was 201). Typecheck and eslint clean on both changed files.Riders
Changeset (
.changeset/save-flow-canonicalization-fallback-warning.md). Nocontent/docs/releases/changes.🤖 Generated with Claude Code
https://claude.ai/code/session_01NDmJ5ASMSzcw1q17vh32MG
Generated by Claude Code