Repository navigation
fix(app-shell,data-objectstack): route every view write through one cache invalidation seam - #4398
Merged
Merged
Conversation
…ache invalidation seam
`ObjectStackAdapter` caches two view-shaped reads — `getView` under
`view:{object}:{name}` and `listViewOverrides` under `view-overrides:{object}` —
with `MetadataCache`'s default 5-minute TTL. objectui#4363 fixed the adapter's
own four write paths, but the console's real create-a-view flow is a SECOND
writer into the same `/meta/view/:name` rows and invalidated nothing:
`handleViewCreate` → `createRuntimeMetadata` → `metadataClient.save`, and
Publish → `RuntimeDraftBar` → `publishRuntimeMetadata` → `metadataClient.publish`.
Publish is the sharp end: it promotes an invisible draft into a published row,
which is exactly the world `listViewOverrides` enumerates, and nothing dropped
the key. It does not self-heal — `loadViewOverrides` treats a RESOLVED map as
authoritative and deliberately does not re-probe per view (objectui#3774) — so
the per-view `getView` fallback that would have masked it is by design
unreachable.
`ObjectStackAdapter.invalidateViewKeys(objectName, viewName)` is now the ONE
place that knows the key set. The adapter's four write paths call it instead of
restating the pair (their eight pins pass unchanged, which is the refactor's
acceptance evidence), app-shell's ADR-0034 persistence module calls it for
`view` saves/creates/publishes/discards, and `MetadataService.saveMetadataItem`
calls it when the category is `view`. `deleteMetadataItem` is documented as
structurally unable to derive an object binding rather than wired.
Fixes #4373
…w-invalidation-seam
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. This landing frees app-shell's metadata-admin surface (#4387 dispatches) and data-objectstack. Generated by Claude Code |
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 #4373
Implements the PM ruling recorded in the triage comment: one exported invalidation seam, routed by every writer (option B), rather than a fifth restatement of the key list.
The defect
ObjectStackAdaptercaches two view-shaped reads —getViewunderview:{object}:{name}andlistViewOverridesunderview-overrides:{object}— withMetadataCache's default 5-minute TTL. #4363 fixed the adapter's own four write paths, but the console's real create-a-view flow is a second writer into the same/meta/view/:namerows and invalidated nothing:ObjectView.handleViewCreatetocreateRuntimeMetadatatometadataClient.saveRuntimeDraftBartopublishRuntimeMetadatatometadataClient.publishPublish is the sharp end. A create lands an invisible per-item draft, and
listViewOverridesenumerates published rows, so the map is still honest there. Publish promotes the row into exactly the world the map describes — and nothing dropped the key. It does not self-heal:loadViewOverridestreats a RESOLVED map as authoritative and deliberately does not re-probe per view (#3774, correct — re-probing reinstates the 404 flurry the batch read exists to remove), so the per-viewgetViewfallback that would have masked a stale map is by design unreachable.The seam
The ONE place that knows the key set. Public because the writers are not all in the class.
viewNameis taken as-is — a runtime-created view is namedaccount.mine, but a source-declared one is not qualified, so splitting on.would be a second, silently-wrong identity rule (AGENTS.md #0.1).The rule is uniform per write, not per branch: draft-addressed writes over-invalidate on purpose. An unnecessary invalidation costs one refetch; a missed one costs up to the 5-minute TTL, and "which half am I in?" is exactly the question that got this key set forgotten twice.
Routing evidence
updateViewConfigcreateViewupdateView(draft half)updateView(published half)deleteViewpersistRuntimeMetadata/createRuntimeMetadata/publishRuntimeMetadata/discardRuntimeDrafttype === 'view'MetadataService.saveMetadataItemcategory === 'view'The four adapter paths' 8 pins in
viewCacheInvalidation.pin.test.tsare byte-for-byte unchanged and pass — that is the ruling's acceptance evidence that routing them through a seam changed nothing observable.The object name is handed down from the call site (
ObjectView,ObjectDataPage,ViewConfigPaneltoRuntimeDraftBar), never parsed out of the row name.MetadataServicederives it from the body via the exportedviewItemObjectName— the same accessorlistViewOverridesnarrows those rows by, not a fourth private copy.MetadataService sibling — measured, not assumed
saveMetadataItem: wired. No in-repo caller passes'view'today, but the class is public API (useMetadataService, exported from app-shell's barrel), so "unreachable" would have been an unmeasured claim about consumers we do not own.deleteMetadataItem: documented as structurally unable, not wired. Both keys are object-scoped; this signature has no object parameter and the tombstone body it writes ({ name, enabled, _deleted }) carries no object binding either — there is nothing to derive one from. The doc comment records what to do if a'view'caller ever appears.Pre-fix red (reverse verification)
Directions were predicted before each run. Five reversals:
publishRuntimeMetadatagetItemsstuck at 1 call, map never learnsaccount.mine,getViewkey still populated. The 3 controls (report type / no adapter / no objectName) stayed greencreateRuntimeMetadatadeleteViewdeleteView invalidates both keys), structural pin 3 red (counts 3 not 2;invalidateLines4 not 2)category === 'view'branchnode:fsin the new pin test@types/nodeReversal 3 is the one that matters. It is the measured proof that the behavioural pins cannot see a restatement: re-inlining the key pair into a write path emits identical keys in identical order, so all 8 stay green while the seam quietly stops being the single source of truth. That blind spot is why the new guard is structural — it asserts each key template appears exactly twice in
index.ts(its reader, plus the seam), and that no app-shell file interpolates either key at all.Honest correction to my own prediction: I called reversal 1 for 2 red and it produced 3. The third,
an invalidation failure does not fail the publish that succeeded, observes the seam call through itsconsole.warn, so it is a positive publish case — I had mis-binned it as a control. Direction correct, count off by one.Verification
pnpm exec vitest run packages/data-objectstack/ packages/app-shell/ --maxWorkers=2— 383 files, 3787 passed, 1 skippedtype-checkboth packages (3 tsc invocations incl.tsconfig.test.json) — cleanturbo run type-check --concurrency=2— 78/78eslintboth packages — 0 errors (warnings pre-existingno-explicit-any)check-control-bytesOK; changeset presence OK; no-major OK (patchfor both packages)Recovered after host restart
A host restart killed the previous agent for this issue mid-task. Recorded for review:
Inherited and verified (not rewritten): the worktree, and one
wip: baseline for reverse verificationcommit holding the whole implementation — adapter seam, four routed paths, app-shell wiring,MetadataService, three test files, changeset. Audited against the ruling's four points before trusting it; all four were already satisfied.Inherited dirty file, kept:
viewInvalidationSeam.pin.test.tswas uncommitted, mid-switch fromnode:fstots.sys. Diffed against the commit and kept as forward progress rather than discarded as a revert artifact — reversal 5 above confirms it was a genuine fix in flight (data-objectstackcarries no@types/node, and the siblingcloud-surface-retired-4152.pin.test.tsrecords the samets.sysconvention).Redone from scratch by me: all five reverse verifications (the previous agent died mid-reversal, so none of its results were trusted), the entire verification ladder above, the
origin/mainmerge, and the history — thewipcommit was soft-reset into one clean commit with a real message. Nothing had been pushed, so no remote state needed reconciling.Generated by Claude Code