test(automation): reconcile the control-flow designer forms against their Zod (#4045) - #4078
Merged
Merged
Conversation
…heir Zod (#4045) `loop` / `parallel` / `try_catch` each carry two descriptions of the same config — a hand-written `configSchema` on the descriptor (drives the Studio form) and a Zod schema in `control-flow.zod.ts` (validates the value) — and nothing compared them. #4064 pinned the shape of the first; this pins the relationship, so neither side can gain or lose a key without the other noticing. Asserts both directions. A key the Zod accepts but the form omits is #3528's failure inverted: a real config key with no way to author it in Studio, discoverable only by reading the executor. A key the form offers but the Zod rejects means the author fills in a field whose value is then dropped. The two sources are deliberately NOT merged, and that is now measured rather than assumed. Generating the form from the Zod emits 9–17× more schema at +9 levels of depth (loop 597 → 5,537 chars / 25 → 201 keys; parallel 294 → 5,112; try_catch 737 → 10,739) with NO $defs and NO $ref — FlowNodeSchema is inlined in full at every region key, so a loop body would reach the designer as the entire node/edge definition instead of the opaque array a canvas-edited sub-graph needs. A projection pruning ~90% back off leaves three things to maintain instead of two, so "one source" would be a fiction. This corrects the premise #4045 opened with. Since the divergence is legitimate, what is enforced is that it stays DECLARED: a `DELIBERATELY_SHALLOW` ledger names each shallow key with a reason, and every entry must name a key both sides still declare — so it cannot rot into a reference to something renamed away. Compares key sets, not generated shapes, on purpose: generating needs `z.toJSONSchema` and `service-automation` does not depend on `zod` (only spec does). Adding that edge to the package graph to run a test is not worth it when `schema.shape` gives the key set without it, and the key set is where the drift that matters lives. The file says so, so the next reader does not think it was forgotten. Verified to bite in all three directions, not merely to pass: - a Zod key the form omits → `loop: accepted by the Zod but absent from the designer form: expected [ 'brandNewRegionKey' ] to deeply equal []` - a form key the Zod rejects → same test red on the inverse set - a ledger entry pointing at a renamed key → `loop.bodyRenamedAway: not on the form any more` Test plan: service-automation 457 passed (5 new), ESLint clean, and spec's api-surface / authorable-surface / docs gates still pass (the negative controls rebuilt spec, so the generated artifacts were checked back to clean). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoA8AV99Ss1RRkLLAYmDzq
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 6 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 08:53
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
…4091) `affected-docs.mjs` derived changed-package roots from every file under `packages/`, tests included. A test observes behaviour rather than defining it, so it cannot make an implementation-accuracy doc stale — yet every tests-only PR lit up its packages' whole doc set. Three in a row did it (#4064, #4078 and one before), each flagging 6 packages/services docs for a diff with no production code. This is the one place the mapper's deliberate over-inclusion actively hurt: a category that is ALWAYS false teaches the reader to skip the comment, and then it fails on the PR where it is right. - Test files are dropped before the roots are derived: *.test.* / *.spec.* at any depth (covering .integration.test.ts and .conformance.test.ts) plus __tests__ / __mocks__ / __fixtures__. - The narrowing is NOT silent — the excluded count rides the summary line and `testFilesSkipped` in --json. - `--self-test` pins the matcher (the check-error-code-casing / check-route-envelope convention), run by the workflow before it computes the mapping. - The workflow also triggers on the mapper's own paths, so editing it runs its own guard. CI surfaced that gap: on the first push the drift job never ran at all, which meant the self-test had been added but was structurally unreachable by the change most likely to break it. Verified against real history: 46e86ba (tests only) → 0 docs, was 6; 4965bfa (engine.ts + formula) → the same 11 docs as before; --all unchanged at 178. Swapping the anchored match for a naive path.includes('test') fails 7 self-test cases, including control-flow.zod.ts and latest.ts being swallowed as tests.
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.
#4045 step C, region half — after #4050 (step 1), #4059 (3a) and #4064 (step A). Tests only, no production code.
The gap
loop/parallel/try_catcheach carry two descriptions of the same config:configSchema(hand-written literal)control-flow.zod.tsNothing compared them. #4064 pinned the shape of the first; this pins the relationship, so neither side can gain or lose a key without the other noticing.
Both directions matter:
notify.sourcesat unnoticed until fix(automation,spec): graduate notify's nestedsourceinto the conversion layer (#4045) #4050.Why the two sources are not merged — measured, not assumed
I ran the generation on main
53fbf49(options as incontrol-flow.test.ts):loopparalleltry_catchGeneration succeeds — no recursion error — but emits no
$defsand no$ref.FlowNodeSchemais inlined in full at every region key, soloop.bodyarrives as the entire node/edge definition:versus the published
nodes: { type: 'array' }with noitems— opaque, because a loop body is edited on the canvas, not in a property form.Making the generated output usable means a projection that prunes ~90% of it back off (201 → 25 keys), at which point "one source" is a fiction: you maintain the Zod, the projection rules, and a check that the projection still does the right thing — three things instead of two.
This corrects the premise #4045 opened with (and my own first analysis on it), which treated these three as redundant copies awaiting de-duplication. They are a second artifact with a different job, and the gaps are deliberate. Full data in the issue.
So the divergence is declared rather than erased
A
DELIBERATELY_SHALLOWledger names each shallow key with its reason (loop.body,parallel.branches,try_catch.try,try_catch.catch), and every entry must name a key both sides still declare — so it cannot rot into a reference to something renamed away. Same bidirectional discipline as the #4040 expression ledger.One deliberate limitation, stated in the file
This compares key sets, not generated shapes. Generating needs
z.toJSONSchema, andservice-automationdoes not depend onzod— only@objectstack/specdoes. Adding that edge to the package graph to run a test is not worth it whenschema.shapeyields the key set without it, and the key set is where the drift that matters lives. The depth divergence is measured once in the table above, and its form half is pinned byconfig-schemas.test.ts. The test file says all of this, so the next reader does not assume it was forgotten.Verified to bite in all three directions
An assertion that never fails is decoration:
Test plan
@objectstack/service-automationcheck:api-surface/check:authorable-surface/check:docsThe negative controls rebuilt spec, which regenerates
authorable-surface.json— checked back to clean and the gates re-run, so no generated artifact rides along in this diff.Empty changeset — tests only, nothing releases.
Remaining in #4045
screen/ CRUD /http/notifyhave no region keys, so the single-Zod-source route stays viable there; the one thing needing explicit handling isadditionalPropertieson the 7 keyValue slots (z.record()emits{}, not the load-bearingtrue;.meta({ additionalProperties: true })overrides it, and objectui accepts{}anyway perjson-schema-to-fields.ts:456).🤖 Generated with Claude Code
https://claude.ai/code/session_01QoA8AV99Ss1RRkLLAYmDzq
Generated by Claude Code