fix(data-objectstack): deleteView removes every home the view has — draft-only, published, and pairs (#4479) - #4562
Merged
Conversation
) A view has two homes -- the pending per-item draft (DELETE /api/v1/meta/view/:name?state=draft) and the published overlay (DELETE /api/v1/meta/view/:name). deleteView addressed only the second, unqualified, so deleting a draft-only view fired at the published overlay, the server answered 200 reset:false "nothing to delete", the draft survived and the tab was back after reload. Not the mechanical mirror of #4139: a draft-first-ONLY delete would discard just the draft on a published+draft pair, silently downgrading Delete view into Discard draft (an operation that already exists, discardRuntimeDraft). persistRuntimeMetadata stages every runtime edit as a draft, so pairs are routine. Both homes are now deleted, draft first, so a mid-operation fault leaves the published overlay intact and the delete cleanly retryable. Two blind calls, no probe: measured against the framework's deleteMetaItem, a missing home answers 200 reset:false, never 404. One transport: both halves go through MetadataClient.reset, which issues the byte-identical request for the published half and collapses two error shapes into one. Receipt widened additively with optional per-home outcomes; deleted is true only when no home remains and at least one held a row. A published-half failure after the draft was discarded throws with the partial state on the error's outcome rather than rounding to true. invalidateViewKeys moves into a finally so it fires once on every outcome including the throw. Red-first: 11 new pins red against unfixed code, 4 must-not-change assertions green throughout; reverse-verified by restoring the unqualified delete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)
Auto-merge armed (squash) — landing verified per the merge-queue discipline. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 13, 2026 09:39
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 13, 2026
…comes (#4564) (#4569) PR #4562 (#4479) widened the ObjectStack adapter's deleteView to return DeleteViewResult { deleted, draft?, published? }, but the shared interface still declared the narrow Promise of { deleted: boolean }. Nothing failed to compile — a wider return is assignable to a narrower declaration — so the adapter satisfied the interface while every consumer reaching it THROUGH DataSource was handed a type with the per-home outcomes already discarded. DeleteViewResult and ViewHomeDeleteOutcome move to packages/types/src/data.ts beside the interface that returns them, and deleteView?'s declared return widens to Promise of DeleteViewResult (optionality and parameters unchanged). data-objectstack imports them for its own use and re-exports both names, so every existing importer keeps compiling and now resolves to the same declaration the shared contract speaks. Census before the move: zero importers of either name outside the declaring file, PR #4562's own suite included. Pinned by deleteViewContract.types.test.ts. The pins are compile-time because the defect is: vitest erases types, so the consumer read below was green against the narrow declaration too — measured, 7/7 green with the fix reverted, while tsc returned 14 diagnostics. A live NarrowLegacyResult control keeps the discrimination honest. Note that `satisfies` and `extends` assertions are green in BOTH worlds for the same assignability reason the gap exploited; only the type IDENTITY assertions discriminate. Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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 #4479
The defect
A saved view has two possible homes — the pending per-item draft (
DELETE /api/v1/meta/view/:name?state=draft) and the published overlay (DELETE /api/v1/meta/view/:name).deleteViewaddressed only the second, unqualified. Deleting a draft-only view therefore fired the delete at the published overlay, the server answeredthe draft survived untouched, the tab was still present after reload, and the receipt reported
{ deleted: false }with nothing surfacing the refusal.Premise re-verified on current
origin/mainbefore implementing: still valid, unchanged.Why the mirror of #4139 is not mechanical
updateViewprobes the draft first and writes back to whichever home the read resolved. Copying that here would be wrong on a published + pending draft pair: a draft-first-only delete discards the draft and leaves the published row still serving the view. That is not Delete view, it is Discard draft — a deliberately different operation that already exists (discardRuntimeDraft, documented as "the published overlay is untouched").persistRuntimeMetadatastages every runtime edit as a draft, so "publish a view, then edit it" makes pairs routine.The asymmetry, stated cleanly: for an update, one home is the right home; for a delete, "remove this view" is satisfied only when no home is left serving it.
The four ruling points, as implemented
1. Delete BOTH homes. Both are deleted on every call.
2. Draft first; receipt widened additively. Draft first because a fault between the two calls then leaves the published overlay intact — the view is still served and the delete is cleanly retryable. The reverse order would strand a draft-only view, which is this card's original bug shape. The receipt gains optional
draft/publishedoutcomes alongside the existingdeleted;deletedis true only when no home is left serving the view and at least one actually held a row (a view that existed in neither home still answersfalse, unchanged). Error propagation matchesupdateView's measured convention — surface the fault, never degrade — so a published-half failure after the draft was discarded throws, carrying the partial state on the error'soutcome. "Draft gone, overlay left" is exactly what the old{ deleted: boolean }could not express, and it is never rounded up totrue.3. No-row answers pinned first, then blind chosen. Measured verbatim from the framework's
deleteMetaItem(packages/metadata-protocol/src/protocol.ts, read-only sibling), and pinned as test fixtures:DELETE ?state=draft200 {"success":true,"reset":false,"message":"No pending draft for view/x."}DELETE(active)200 {"success":true,"reset":false,"message":"No view 'x' found — nothing to delete."}DELETE ?state=draft200 {"success":true,"reset":true,"seq":N,"message":"Draft discarded — view/x. [seq=N]"}DELETE(active)200 {"success":true,"reset":true,"seq":N,"message":"Deleted view 'x' — it no longer exists. [seq=N]"}Both no-row answers are a 200 carrying
reset:false, never a 404 — benign. So the ruling's condition for blind-two-calls is met and no probe is added.updateViewneeds its probe for a different reason (its read must resolve the row the merge writes back to), which has no counterpart for a delete.4. One transport, one error contract — measurement pointed at the metadataClient.
MetadataClient.reset(type, name)with nostateissuesDELETE {baseUrl}/api/v1/meta/view/:name, the byte-identical requestclient.meta.deleteItemwas issuing; this adapter configures noenvironmentId, so no header or scoping diverges. Both halves therefore route throughMetadataClient, and cross-transport normalization is unnecessary.metadata-client.tsneeded no change — the published-state delete path already existed. One residual difference is recorded honestly in the report: the SDK honours a discovery-supplied metadata route whileMetadataClienthardcodes/api/v1/meta. That divergence is pre-existing and repo-wide (updateView's draft half andlistViewsalready rely on the hardcoded prefix); this change does not widen it.Red-first evidence
Predicted split written down before running against unfixed code, then confirmed exactly: 11 red, 4 green. The 4 that stayed green are precisely the must-not-change assertions.
receipt.deletedis trueAssertionError: expected false to be trueexpected [ 'published' ] to include 'draft'expected undefined to be trueexpected [ 'published' ] to deeply equal [ 'draft', 'published' ]deletedis falsepromise resolved "{ deleted: true }" instead of rejectingReverse-verified by taking the fix out (
git diffto a patch +git checkout origin/main -- …, nevergit stash) and re-running:Tests 11 failed | 474 passed (485), all 11 in the new file, everything else green. Restored and confirmed byte-identical bysha256sum -c. Note thatviewCacheInvalidation.pin.test.tspasses under both the fixed and unfixed code, which is what proves the harness extension there did not change what that pin asserts.Must-not-change pins held
discardRuntimeDraftstays a different operation — pinned by driving the samereset(..., { state: 'draft' })primitive it uses and asserting it touches only the draft home whiledeleteViewtouches both. Driven through the primitive rather than imported, because app-shell is downstream of this package.invalidateViewKeysfires exactly one ordered pair per call on every outcome, now including the throw path (moved into afinally; objectui#4363's asymmetry decides it — an unnecessary invalidation costs one refetch, a missed one costs the full 5-minute TTL).find(), fix(console): an unauthorized app says so — access denial no longer masquerades as publishing (#4252) #4521probeAppAccess, permission-matrix-edit-loop: the field-level half of the permission matrix is dead for every object (MetadataClient.get() does not unwrap the envelope) #4271get()-unwrap pins all green in the full suite.Verification
pnpm exec vitest run --maxWorkers=2 packages/data-objectstack/— 35 files, 485 tests passedpnpm --filter '@object-ui/data-objectstack' type-check— clean--filter '...@object-ui/data-objectstack'= consumers, not dependencies) — 32 packages green, includingapp-shell,console,plugin-view,plugin-designer.d.tsdiff measured on a clean rebuild (dist/andtsconfig.tsbuildinforemoved between builds) — additive only: two new exported types, anddeleteView's return widens from an inline{ deleted: boolean }toDeleteViewResult, which still carriesdeleted: boolean. Type reverse-verified both ways against the rebuilt declaration: readingreceipt.draft?.removedcompiles, and a typo of it is rejected withTS2551.no-explicit-any;--max-warningsis deliberately unset repo-wide perlint.yml)check:control-bytesOK,check:phantom-depsOK, plus a manual control-byte self-scan of every touched fileConsumer census
One real call site:
packages/app-shell/src/views/ObjectView.tsxhandleDeleteView, which awaitsdeleteViewand does not read the receipt (it catches and toasts on failure).packages/types/src/data.tsdeclaresdeleteView?returning the narrow{ deleted: boolean }; the adapter's wider return is assignable to it, so the canary did not demand an edit and none was made — noted in the report as an observation, since a consumer reaching the adapter through that interface still sees onlydeleted.Surface
packages/data-objectstack/src/index.ts,packages/data-objectstack/src/deleteView.homes.test.ts(new),packages/data-objectstack/src/viewCacheInvalidation.pin.test.ts(harness models DELETE; assertions verbatim), one changeset. No consumer edits, nothing in #4527-phase-2's write set, nothing in plugin-gantt / CelPredicateField / plugin-designer / plugin-grid / plugin-kanban /content/docs/releases/.Changeset:
@object-ui/data-objectstackminor — entry-reachable receipt widening plus a published-behavior move, the same grading objectui#4271'sget()unwrap and objectui#4495'sfind()resolve to reject took.Generated by Claude Code