Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): delete metadata items through the API's delete door, not a tombstone PUT - #6478
Merged
Merged
Conversation
…not a tombstone PUT
`MetadataService.deleteObject` and `deleteMetadataItem` wrote
`{ name, enabled: false, _deleted: true }` through `client.meta.saveItem`, i.e.
`PUT /api/v1/meta/:type/:name`. Measured against the installed
`@objectstack/spec` 17.2.0 via `getMetadataTypeSchema` — the registry the
framework's own `saveMetaItem` resolves a PUT's validator from — 25 of the 26
registered overlay schemas refuse `enabled` and/or `_deleted` BY NAME, `object`
among them, so that call was a hard 422 `INVALID_METADATA` and recorded nothing.
Where the schema is tolerant (`view`) or unregistered the framework stores the
body verbatim and nothing on the platform reads `_deleted`, so there the "soft
delete" was a silent no-op that left the item live carrying two junk keys.
Nothing strips them, and there is no third outcome.
Not a rename, because there is nothing to rename to: `ObjectSchema`'s 42-key
accept set has no on/off flag, and the near-spelling `enable` is
`ObjectCapabilities`, a system-features module object, so `enabled: false` ->
`enable: false` fails on the value where it passes on the name. No wire key was
invented — a metadata soft-delete convention would be a `@objectstack/spec`
contract addition and the platform does not have one.
Both sites now call `client.meta.deleteItem(type, name)`, which issues the same
`DELETE /api/v1/meta/:type/:name` that `MetadataClient.reset` does — the
mechanism `MetadataObjectsPage.handleObjectsChange` and `ResourceEditPage`
already used. That route is generic over `:type` on the same route family and
capability gate as the PUT, so the convergence holds for every category the
generic `deleteMetadataItem` serves, not just `object`. The doc comment claiming
the API had no `deleteItem` was stale: `@objectstack/client` 17.2.0 declares it
on the very client this service already holds.
The latent `enabled?: boolean` on `ObjectMetadataPayload` goes with it
(objectui#4687's shape — `toObjectPayload` never populated it), and so does the
`KNOWN_UNPARSEABLE_KEYS` row, which ratchets in both directions. The ledger is
now empty for the first time, so the self-test's non-vacuity guard moved onto a
fixture: `expect(entries.length).toBeGreaterThan(0)` had inverted into a demand
that some key stay unresolved.
Part of #6238
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4qContributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 26, 2026 04:26
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#6238
MetadataService's two delete methods PUT a hand-written tombstone that the metadata API refuses by name. Both now call the API's own delete door, and the latentenabled?: booleanonObjectMetadataPayloadgoes with them. The card asked for measurement before any fix and put a fork fence on a spec-side soft-delete convention — the measurement is below, and it did not hit the fence.1. What the server actually does with the tombstone PUT — refuse? strip? store?
All three questions have an answer, and it is category-dependent. Oracle:
getMetadataTypeSchemafrom@objectstack/spec/kernel17.2.0 — the registry the framework's ownsaveMetaItemresolves a PUT's validator from (resolveOverlaySchema,metadata-protocol/src/protocol.ts:523), so this is the schema the deployed route would use, type by type.objectincluded:422 INVALID_METADATA. The delete recorded nothing, ever.view(its schema tolerates unknown keys) and the four unregistered kinds (they fall through unvalidated). The framework deliberately persists the request item, notparsed.data— the comment at thesafeParsesite says so in as many words, to keep Studio-only auxiliary fields. And_deletedhas no reader anywhere in the platform:grep -rn "\b_deleted\b" --include=*.ts packages/over the framework returns 0, tests included, against a positive control of 237 forsaveMetaItem. So on exactly the categories that did not 422, the "soft delete" was a silent no-op that left the item live carrying two junk keys.parsed.data) is explicitly not used.Neither outcome deleted or disabled anything. The near-spelling is a trap, as the card said:
enableisObjectCapabilities, a system-features module object, soenabled: false→enable: falsefails on the value where it passes on the name — pinned as its own case, because a key-name gate can never state that.2. Does
client.resetcover it — for every category, not justobject?Yes, and the two are the same request.
MetadataClient.reset(type, name)DELETE /api/v1/meta/:type/:nameclient.meta.deleteItem(type, name)(@objectstack/client17.2.0)DELETE /api/v1/meta/:type/:nameThe REST route is registered generic over
:typewith no allow-list, behind the samemanage_metadatacapability gate and the samecanonicalMetaUrlTypefold as the PUT twin, dispatching to adeleteMetaItemthat is likewise generic. So the scope subtlety resolves in favour of converging both sites::262's object-specific writer and:233's any-categorydeleteMetadataItemreach the same door, and there is no category the generic one serves that the delete route does not. This is asserted, not just argued — one case captures both clients' requests and compares method and URL.MetadataServiceholds anObjectStackAdapter, so its name for that door isclient.meta.deleteItem. The doc comment claiming the API "exposessaveItembut no dedicateddeleteItem" was stale: 17.2.0 declares it on the very client this service already holds.3. The fork fence — not reached, and here is why
⛔ No wire key was invented. There was nothing to invent one for:
ObjectSchema's 42-key accept set has no on/off flag (disabled,active,isActive,deleted,isDeleted,softDeletedall absent — asserted), and the tombstone expressed nothing any reader consumes. A capability that 422s where it is validated and no-ops where it is not is not a capability being lost. So this is objectui#4687's shape for the declaration and a mechanism change for the writers, with no spec surface required.resetsemantics are the overlay's, and that is the governed answer rather than a shortfall: it removes the customization row — which is deletion for an object the designer authored — and restores the artifact for one a package declares, an object you are not allowed to delete. Which of the two an item is, is what the API's owndeletable/resettableverdicts report, not something a client-side flag should decide.Premise check — one claim in the card is a step too strong
The card and triage both call this "the LIVE designer delete path". Measured on
origin/main: the designer's Object Manager already deletes throughclient.reset('object', name)(MetadataObjectsPage.handleObjectsChange), andMetadataService.deleteObject/deleteMetadataItemhave zero in-repo callers —useMetadataServiceis referenced nowhere outside its own definition and the barrel, against a positive control of 81 files foruseAdapter.So the refused keys ship on
MetadataService's published API surface (useMetadataService(), exported from app-shell's barrel), not on an in-repo designer screen. The bug is real and the fix is unchanged — but "live designer path" overstates the reach by one step, and the card's own framing of the declaration as latent and the soft-delete as live is what led there.Structural claim: a measured before/after on the gate
A removal has no new behaviour to pin, so the ledger claim is a before/after with its positive controls in the same output.
ObjectMetadataPayloaddeclared keysenabled objectui#6238 [ObjectSchema])uiOnlykeys reportedFieldSchema/ObjectSchemaaccept setsdesigner-field-key-parity: OKdesigner-field-key-parity: OKThe nine
uiOnlykeys — including the object-levelgroup,sortOrderandrelationships— are still reported afterwards, so theObjectSchemaoracle is still scanning and this is a removal rather than a blinded scanner.⭐ The gate's failure text offers the one edit the ledger forbids. When RV1 below turns it red it prints, ready to act on, "record it in
KNOWN_UNPARSEABLE_KEYSrather than picking one here" — correct advice for a new finding, and exactly wrong for a resolved one, where adding a row weakens a gate that ratchets in both directions. Read, not followed. The row was removed, per objectui#6045's precedent one file over.The ledger is now empty for the first time, which is the ratchet arriving where it was pointed — and that surfaced a real defect in the gate's self-test:
expect(entries.length).toBeGreaterThan(0), written as a non-vacuity guard, had inverted into a demand that some key stay unresolved, failing withexpected 0 to be greater than 0whose obvious remedy is to add a row back. The guard is kept and pointed at the right thing — the per-entry validation now runs against fixtures (one well-formed, two malformed that must throw), so it is non-vacuous whatever the live ledger holds.⭐ Which half the gate cannot see
objectui#6045 found the parity gate stays green on a tree where only the copier is restored, because it reads declarations. That applies here with more force: the tombstone was a literal object, not a declared shape — the gate's own coverage note 1. Reverse verification confirms it, and that is why this change needs a runtime assertion on the actual request bytes, not only a declaration removal.
Reverse verification
Direction predicted before each run. Mutations proven on disk by grepping the injected and the deleted text; restores proven by worktree-blob-hash
==HEAD-blob-hash plus an emptygit diff HEAD, under anEXIT/INT/TERMtrap holding absolute paths.RV1 — restore
enabled?: boolean, ledger row still removed. Predicted RED. → RED,GATE_EXIT=1:ObjectMetadataPayloadreads7 declaredagain. Marker count 0→1; blob27c4cf65→728643ba; restored to27c4cf65,git diff HEADempty.RV2 — restore the tombstone PUT only, declaration still removed. Two predictions.
GATE_EXIT=0,designer-field-key-parity: OK.(b) is a pin that stays green in both directions, and it is reported rather than filed off. It is not a broken pin: the gate is a declaration check and the restored tombstone declares nothing, so green is the gate answering its own question correctly. It is the honest limit the gate's header states, and the reason the wire cases exist — they are the only thing standing between a restored tombstone and CI.
Deleted-text count 1→0 and injected-text count 0→1 in the same run; blob
27c4cf65→cd0a6365; restored to27c4cf65,git diff HEADempty.⭐ Clause ② — public surface, stated either way
No published type changed.
ObjectMetadataPayloadis exported from its own module, but that module is not re-exported bypackages/app-shell/src/index.ts— the package's only entry (exports: { "." → dist/index.d.ts }). Positive control: the barrel does export other./services/*symbols (componentRegistry), so the absence is a measurement rather than a failed grep. No**/src/index.tsin this repo is touched by this diff at all.Behaviour, however, is public and does change — stated because it would be wrong to leave it inferred.
MetadataServiceis reachable through the publisheduseMetadataService()hook. Both method signatures are unchanged ((…) => Promise<void>; the delete receipt is deliberately not surfaced — that would be an unrequested API widening), but the HTTP request they issue changes fromPUT /meta/:type/:namewith a body toDELETE /meta/:type/:name. Declared in the changeset.Verification
All commands run in the foreground through the container's shared verify lock; exit codes captured before any pipe; each verdict quoted from the gate's own output. Gate union run after the final commit, on
c2405ad48, against a clean tree:check:designer-field-key-paritydesigner-field-key-parity: OKcheck:control-bytes✅ check-control-bytes: OK (scanned 5350 tracked text file(s); skipped 85 binary)check:spec-symbols✅ spec symbol derivation: 1311 files scanned against 4959 spec export namescheck:phantom-deps✅ Every in-scope import is declared by the package that publishes it.✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)changeset:check✅ All workspace packages are in the changeset fixed group./✅ No changeset declares a major bump.Test Files 8 passed (8)·Tests 85 passed (85)type-checktsc --noEmit && tsc -p tsconfig.test.json,EXIT=0Lint was not narrowed —
eslint .ran over the whole repo from the root: 3809 files selected by eslint's own config, 0 errors, 11038 warnings (pre-existing;.github/workflows/lint.ymlsets no--max-warnings). Every file this PR touches is0 E; the single warning onMetadataService.tsis the pre-existingconst raw: anyinsaveFields, present onorigin/mainat the same statement.The type-check really covers the new test file — the package's
type-checkistsc --noEmit && tsc -p tsconfig.test.json, and--listFilesconfirms both the new and the edited test file are in that program (1 hit each, positive control 1), so "typecheck clean" is not a claim about a program that excluded them.Dependency closure built before any type-check or ablation (
pnpm --filter '@object-ui/app-shell^...' build,VERDICT command-exit 0), so no reading here is against a staledist.Generated by Claude Code