Repository navigation
Client-side Zod validation errors do not gate Save, while inspector-reported errors do — one class of author-time error is advisory, the other blocking #6980
Description
Found while taking the producer census for #6331 (see PR #6979). Filed unassigned as an observation, for PM triage. Not fixed there — out of that card's scope, and not flow-specific.
The asymmetry
ResourceEditPage.tsx has two sources of author-time error, and only one of them gates Save.
Gated — issues an inspector reports:
// ResourceEditPage.tsx:652
// Blocking author-time issues reported by the scoped inspector (e.g. a CEL
// formula that does not parse) — Save must refuse them rather than publish a
// malformed definition (objectui#4306).
ResourceEditPage.tsx:1999
disabled={saving || (!createMode && !isDirty) || inspectorBlocking > 0}
ResourceEditPage.tsx:1618 if (inspectorBlocking > 0) return; // auto-save
ResourceEditPage.tsx:1642 if (!saving && (createMode || isDirty) && inspectorBlocking === 0) {
Not gated — issues the live client Zod validator reports. validateMetadataDraft runs the same schema the server runs (clientValidation.ts maps flow to FlowSchema, object to ObjectSchema, and so on for every wired type, on both the create and the edit door), debounced 200 ms on every keystroke, into setIssues. That state feeds the banner, the inline field errors and previewDiagnostics — and nothing else. inspectorBlocking is the only term in the Save gate, and doSave itself calls no validator.
So: a draft whose shape the contract refuses is Saveable; a draft whose CEL predicate does not parse is not. Both are author-time errors, both are already detected client-side, and the comment quoted above states the intent for the class as a whole ("Save must refuse them rather than publish a malformed definition").
Why it matters
The consequence is the failure shape #6287 was filed about: a refusal the author meets at save time rather than at a gate. It is milder here than in #6287 — the error IS visible while typing (banner, inline, and for flows an on-canvas badge plus a Problems-panel row, since flow-problems.ts maps a nodes.N.… path onto the offending node) — so this is "the gate is advisory where a sibling gate is blocking", not "the author is told nothing".
Scope of the reading, stated
- Read on
origin/mainat7a5da1414. Traced by reading every use ofinspectorBlockinganddisplayIssuesinResourceEditPage.tsxand confirmingdoSave(line 1294 onward) contains no validation call. - Not measured in a browser, and no server was booted — the "the server then refuses it" half is cited from the schema (
FlowSchema.safeParseon a node missinglabelfails withinvalid_typeatnodes.0.label), not from a run.
What this is NOT asking for
⛔ Not "make every Zod issue block Save" as an obvious fix. There are at least two readings, and choosing between them is triage, not this note:
- The suppression is deliberate and merely undocumented — e.g. so a stale bundled
@objectstack/speccan never wedge Save shut on a draft the running server would accept.clientValidation.tsalready carries a root-cure for exactly that skew (it suppresses "missing required field" for fields the live server marks optional), which is evidence for this reading. - It is a real gap, and the fix is to gate Save on client Zod errors the same way
inspectorBlockinggates it.
If (1), the answer is a comment at the Save gate saying why schema issues are advisory — the absence of which is what made this look like a defect from the outside.
Related: #6287 (the save-time-refusal class), #6331 / PR #6979 (where this was found), objectui#4306 (the card that added the inspector-side gate).
Generated by Claude Code
Activity
claude commentedon Aug 31, 2026
Claiming this card.
claude/issue-6980-save-gate-zod-asymmetryDispatched as a MEASUREMENT task with a conditional deliverable: establish on evidence which of the two readings in the body holds (deliberate-and-undocumented vs. real gap). Shipping the gate change is explicitly out of my line either way.
Generated by Claude Code
Generated by Claude Code
claude commentedon Aug 31, 2026
os-dev-report
{ "issue": 6980, "status": "done", "branch": "claude/issue-6980-save-gate-zod-asymmetry", "pr": "https://github.com/objectstack-ai/objectui/pull/6983", "premise_still_valid": true, "summary": "Verdict: reading (1) — advisory is correct and the missing record is the whole defect — but with a correction the evidence forces: it was NEVER argued at the Save gate, so the deliberateness lives in a module-level principle, not in a decision taken at the gate. The discriminator the card lacked is whether the server backstops the class: inspector CEL faults have no backstop (#4306 measured PUT 200 plus publish makes it live; the runtime authoring gate only advises on an already-SUCCESSFUL save, #4133), while schema issues do (doSave already maps 422 / INVALID_METADATA / INVALID_PAYLOAD back into the same inline issues), so client Zod is a preview of the server's verdict, not the thing keeping the draft out of storage. Gating it adds no protection and adds one risk: the client being stricter than the server. Shipped the card's named reading-(1) deliverable only — a comment at the Save gate (48 added lines, all comments, 0 removed), a pin test asserting both halves against one draft, and an empty-frontmatter changeset. The gate change was NOT implemented.", "evidence_for_the_reading": { "skew_class_real_and_cure_incomplete": "DECISIVE, and measured on bundled @objectstack/spec 17.2.0. The serverSchema root-cure suppresses exactly one class: a TOP-LEVEL issue (path.length === 1) on a field ABSENT from the draft that the server marks optional. Measured issue paths per class: server gains a root key = unrecognized_keys at path.length 0; server gains a nested key = unrecognized_keys at path.length 2 (nodes.0); nested required key = path.length 3 (nodes.0.label); present-but-invalid (format / enum / narrowed union) = field present, so the absent test fails; enum member added server-side = invalid_value, covered only for flow nodes.N.type via a 2-entry hard-coded set. The cure's guard is 'if (path.length !== 1) return true', so five of six classes pass straight through. 14 of the 15 wired root schemas reject an undeclared key, so the most likely forward skew is uncovered on almost every type. Not hypothetical: objectstack#5316 is a LANDED instance — stored view bodies carry the platform's own isPinned / sortOrder and the authoring schema rejected them; clientValidation.ts records 'the client strictly stricter than the server'. Under today's advisory gate that cost a false banner; under a blocking gate it would have dead-bolted Save for every author who had ever pinned a view, on a body the server accepts, with no on-screen editor able to remove isPinned (the pin lives in the view switcher, not the form). That wedge is the hazard blockingReport, ObjectFieldInspector, ConditionBuilder, ConditionalFormattingEditor and PermissionAdvancedFacets are each explicitly built around.", "history": "AGAINST literal reading (1), FOR reading (2) on intent — and it is a clean zero. #4306's body, its 3 comments (two PM rulings plus a dev escalation) and PR #4536's body total 18,601 characters. Ten probe terms — zod, clientValidation, validateMetadataDraft, hasClientValidator, setIssues, 'schema validation', 'spec skew', skew, 422, invalid_metadata — returned 0 hits TOTAL. Five hot-control terms over the same corpus in the same pass returned 64: inspector 53, celPredicateField 4, perm.cel.saveBlocked 4, onLintChange 2, onBlockingIssuesChange 1. Independently: ResourceEditPage.tsx AT #4536's BASE COMMIT (f56541826) already imported validateMetadataDraft and ran it into setIssues at line 442, with a Save gate of 'disabled={saving || (!createMode && !isDirty)}' — no validation term at all. The Zod validator was already there, in the same file, and was never weighed. So the split was not argued; it was not considered.", "load_bearing_today": "MIXED, and stated rather than smoothed over. All three named skew cures are measurably INERT on bundled spec 17.2.0: (a) the root-cure's named instance — report objectName / columns — is closed, ReportSchema now requires only name and label, exactly the required set clientValidation.skew.test.tsx feeds as the server side; (b) FORWARD_COMPAT_FLOW_NODE_TYPES is unreachable, flow node type is an open non-empty string that accepts even zzz_made_up_node_type; (c) AUTHOR_SHAPE_ONLY_TYPES' premise for sharing_rule is stale, SharingRuleSchema now declares all 7 ADR-0010 envelope keys. They are inert because objectui's caret pin resolved to 17.2.0 and framework packages/spec at HEAD also reports 17.2.0 — the two release trains happen to be aligned at this instant. Alignment is a moment, not a property, and 14/15 schemas reject undeclared keys, so the class re-opens on the next server-side authorable key. (b) and (c) are filed as #6982.", "hot_control": "Two built, both returned hits in the same pass with the same instrument. (1) Over the #4306 corpus: 10 probe terms 0 hits, 5 control terms 64 hits. (2) Repo-wide git grep over packages/app-shell/src/views/metadata-admin, docs, AGENTS.md, CLAUDE.md, content/docs for a recorded reason the schema class is advisory: 'Zod.*advisory' 0, 'advisory.*Zod' 0, 'schema issues are advisory' 0, 'does not gate Save' 0; hot controls in the same pass inspectorBlocking 7, objectui#4306 10, '#4306' 22, validateMetadataDraft 107, 'root-cure' 7. The absence of a recorded reason is a measurement, not a search miss.", "search_bound": "objectui origin/main at fd8dacecc; @objectstack/spec resolved 17.2.0 in a fresh worktree install. NO SERVER WAS BOOTED — 'the server refuses schema-invalid drafts' is read from doSave's 422 / INVALID_METADATA / INVALID_PAYLOAD handling and clientValidation.ts's own header, not observed on the wire; 'the server accepts malformed CEL' is cited from #4306's measurement, not re-run. Local history was unusable: the checkout is SHALLOW (44 commits), so git log -S returned 1 spurious commit and all history evidence came from REST instead. NOT COVERED: any repo outside objectui; the framework's /meta/types response beyond the required array the cure reads; e2e or browser behaviour; whether a deployed server's spec version differs from framework HEAD's." }, "tests": "Dependency closure built first: pnpm --filter '@object-ui/app-shell^...' build, wrapper VERDICT command-exit 0. New suite (1 path in, 1 file reported): 'Test Files 1 passed (1)' / 'Tests 2 passed (2)'. Regression (10 paths in, 10 reported): 'Test Files 10 passed (10)' / 'Tests 134 passed (134)' over ResourceEditPage.celGate, .defaultGate and all seven clientValidation.* suites plus the new one. type-check: pnpm --filter @object-ui/app-shell run type-check (tsc --noEmit and tsc -p tsconfig.test.json) exit 0, with the script name echoed so it is not a zero-match silent pass; the main tsconfig EXCLUDES the test glob so that pass says nothing about the new file, and tsconfig.test.json --listFiles confirms it IS in the program (1 hit, celGate control also 1). eslint plain form --format json: new test 0 errors / 0 warnings; ResourceEditPage.tsx 0 errors / 55 warnings, EXACTLY the fd8dacecc baseline — measured by linting the base blob as a probe file in the same directory, not assumed. check-changeset-presence verdict line: '1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s) ... Every one of them has an EMPTY frontmatter — declared as releasing nothing, which is the explicit exemption and a complete answer to this gate.' check-changeset-fixed / check-changeset-no-major / check-control-bytes exit 0 with their own verdict lines ('check-control-bytes: OK (scanned 5803 tracked text file(s); skipped 85 binary).'). ABLATION (reverse verification), direction predicted in writing first: wiring issues.length into the Save gate must turn the pin red. THE FIRST RUN CAUGHT MY OWN TEST — 1 failed / 2, and the 'Save stays enabled' half PASSED under the mutation, because waitFor of toBeEnabled retries and had succeeded on a render taken before the 200ms-debounced issues landed; the mutation was caught only incidentally, by a title assertion in the other test. Rewrote the pin to anchor on the banner (proof the validator ran on THIS draft and rejected it) and then assert Save synchronously, with no retry window. SECOND RUN, same mutation: 2 failed / 2, both on the designed assertion, 'Received element is not enabled' with disabled and title 'Save'. Mutation and restore were both proved on disk rather than from an exit code: target blob 2a3d0722 to a76b75e9 and back to 2a3d0722 (equal to the HEAD blob), old-text occurrences 1 to 0 to 1 and injected-text 0 to 1 to 0, and git diff HEAD empty after restore; the restore leg is 'git checkout HEAD -- ABSOLUTE_PATH' under an EXIT INT TERM trap. No dist leg was needed and none is claimed: the suite imports ./ResourceEditPage by RELATIVE path, i.e. source, so there is no exports-resolved dist in the path and no rebuild could change the reading. Every figure above is at the final commit 38f493109.", "mcp_calls": "0 — every GitHub read and write went through REST (curl); zero MCP GitHub tool calls for the whole run", "open_questions": [ { "question": "The verdict is reading (1) on behaviour, but reading (2) is right that the split was never argued at the gate. Does that correction change the card's disposition — close #6980 on this PR, or keep it open so the maintainer sees that a save-door asymmetry reached main unargued?", "options": [ "A. PM accepts the verdict and closes #6980 on merge. The PR says 'Part of #6980', so closing stays an explicit act rather than a side effect.", "B. Keep #6980 open after merge and route the intent half to the maintainer as a process observation about unargued save-door changes." ], "recommendation": "A, because the card asked for a verdict and named this exact deliverable for reading (1); the intent correction is recorded in the code comment, the PR body and this report, so nothing is lost by closing. B only if the maintainer wants unargued save-door changes tracked as a class." } ], "out_of_scope_findings": [ "filed as #6982: two of clientValidation.ts's three spec-skew cures rest on stale premises on bundled spec 17.2.0 — FORWARD_COMPAT_FLOW_NODE_TYPES is unreachable (flow node type is an open string; even zzz_made_up_node_type is accepted) and AUTHOR_SHAPE_ONLY_TYPES' hold on sharing_rule was measured on 17.0.0-rc.5 whereas SharingRuleSchema now declares all 7 ADR-0010 envelope keys. Filed, not fixed: lifting that opt-out turns a client gate ON for a security-metadata edit door, which is author-facing behaviour and needs a live-server probe. Duplicate check: all 272 open issues scanned (REST listing, 3 pages, local grep); 4 probe terms 0 hits, hot controls in the same pass returned clientValidation 2, ResourceEditPage 6, Save 28." ] }Generated by Claude Code
Generated by Claude Code
docs(app-shell): record why client Zod issues are advisory at the Sav…