fix(app-shell): a failed package-list refresh is not a deletion (objectui#7821) - #7879
Merged
os-sam merged 1 commit intoSep 6, 2026
Merged
Conversation
…ctui#7821)
`onManageChanged` refreshed the package list into a local `list` initialised to
`[]` and swallowed the rejection under a comment reading "keep the stale list".
That is true of the `pkgs` state — which is simply not written — and false of
the local, which stayed `[]`. So after a failed `GET /api/v1/packages` the
`!list.some((p) => p.id === managedId)` check three lines down was
unconditionally true, the code took the branch labelled `// Deleted`, and when
the managed package was the one under the editor it navigated away with
`list[0]` undefined — to `/home`. One transient 503 evicted the author from the
Studio editor, silently, while the package was still there.
The local now starts as `null` ("the refresh told us nothing"), and only a list
that actually came back, without the managed package in it, is read as a
deletion. A failure draws no inference: no navigation. It is reported instead
through the posture this surface already carries — `formatMetadataError` on the
shared `studio-package-list` sonner id, and recorded so the switcher reads
`failed` rather than presenting a stale list as current.
Still a `.catch` and still no retry: one 503 must not take the Studio down, and
no retry policy has been ruled on.
Behavioural pins in StudioDesignSurface.packageDeletionInference.test.tsx cover
all three legs: no navigation on failure, the failure still reported, and a real
deletion navigating exactly as before (to a surviving sibling, or `/home`).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Contributor
✅ 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
|
This was referenced Sep 6, 2026
os-sam
marked this pull request as ready for review
September 6, 2026 01:34
os-sam
deleted the
claude/issue-7821-onmanagechanged-false-deletion-eviction
branch
September 6, 2026 01:47
os-sam
pushed a commit
that referenced
this pull request
Sep 6, 2026
…tui#7881)
`fetchFullPackage` — the `PackageSwitcher` helper behind "Package info &
settings" — fetched `/api/v1/packages` and went straight to `res.json()`,
never reading `res.ok`. The platform answers a failed read in the ADR-0112
envelope, `{ success: false, error: { code, message } }`, and that envelope
parses cleanly through the reader below it: `root` becomes the error object,
which is neither an array nor carries `packages`, so the list fell to `[]` and
`.find()` to `null`. Nothing threw, so `openManage`'s `catch` — the one that
toasts `formatMetadataError` — never ran, and the two lines after it still
fired: `setManage(null)` then `setManageOpen(true)`.
`PackageDetailSheet` renders `null` for a null package, so during an outage the
author clicked the menu item and got silence: no sheet, no toast, no
explanation — and `manageOpen` stuck true with no rendered sheet to close it.
Third variant of the objectui#7368 family after objectui#7821 (PR #7879): not a
lost toast and not an inverted decision, but a failure laundered into a
successful-looking empty result. An empty list is a completely legitimate
success answer, which is exactly why it must never be the value a failure
produces.
Measured before deciding what to read. `GET /api/v1/packages` is served by the
direct-mount registrar, which mounts first in the production stack and is
pinned at zero hand-written bodies, so every failure leaves through the shared
`sendError` / `sendThrownError`: 401 UNAUTHENTICATED, 403 FORBIDDEN, 503
SERVICE_UNAVAILABLE and 500 INTERNAL_ERROR, all one shape. The envelope's own
`success` is therefore not a second bit here — `sendOk` writes `true` on every
2xx and the error writers `false` on every non-2xx, which is `!res.ok`
restated. So `res.ok` is the decision and the envelope is read for the words;
in the 5xx band the platform withholds the producer's prose for the generic
`Internal server error`, leaving `error.code` as the only discriminating word,
so the code travels with the message. A non-JSON error body — the one other
reachable shape, a proxy's HTML 502/504 — names the status instead of the JSON
syntax error the author used to be shown.
Reported through this surface's existing posture, not a second one:
`formatMetadataError` on the shared `studio-package-list` sonner id, so one
outage across this surface's four callers of that endpoint is one toast rather
than four. Deliberately not also recorded in `pkgsErr`: that slot is the
switcher list's own state, written exactly where `pkgs` is, and this callback
never writes `pkgs`.
And the sheet no longer opens on a `null` package at all — this card's
user-visible deliverable. A successful list that does not contain the package
(deleted or uninstalled elsewhere) now says so.
Nine behavioural pins in StudioDesignSurface.packageLookupFailure.test.tsx.
Three of them are negative controls that stay green with the fix reverted, so
the other six are provably not restating an existing assertion — and a "fix"
that merely stopped opening the sheet cannot pass the file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
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 #7821
The defect, re-verified on today's
main(384715bb1)onManageChanged— the callback the Studio'sPackageDetailSheetfires after everypackage lifecycle action (disable / duplicate / delete / publish / manifest edit) —
refreshed the package list into a local
list, initialised to[], and swallowedthe rejection under a comment reading
/* keep the stale list */.That comment is true of the
pkgsstate, which is simply not written. It is falseof the local, which stayed
[]. So after a failedGET /api/v1/packages:!list.some((p) => p.id === managedId)three lines down was unconditionally true,// Deleted,list[0]undefined — i.e. to/home.One transient 503, network blip or auth expiry threw the author out of the Studio
editor, with no toast and no confirmation, while the package was still there. Every
link of that chain was re-read on today's
mainbefore the fix (PR #7822 landed in thissame file at 00:30:12Z); all four still held.
This is the sibling of #7368 escalated: not a swallowed failure, but a swallowed failure
that then decides the opposite of the truth.
The fix
The local now starts as
null— "the refresh told us nothing" — and only a list thatactually came back, without the managed package in it, is read as a deletion. A
failure draws no inference at all: no navigation.
It is reported instead, through the posture this surface already carries (#7368,
reused rather than duplicated):
formatMetadataErroron the sharedstudio-package-listsonner id, and recorded into the existing
pkgsErrslot so the switcher readspkgListState === 'failed'rather than presenting a now-stale list as current. Asuccessful refresh clears that slot, since the names on screen are then genuinely fresh.
Still a
.catchand still no retry — one 503 must not take the Studio down (#7368'sruling), and no retry policy (count, backoff, what to show after giving up) has been
ruled on. No new error state machine.
Evidence — behavioural, five pins
packages/app-shell/src/views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsxdrives the real switcher (trigger, "Package info & settings", lifecycle
onChanged) andvaries only what the refresh does:
/studio/app.b2r4/interfaces,/homenever mounts.studio-package-listsonner id, the trigger readsfailed, and the top bar is stilla working trigger (never a throw).
/home, unchanged.loaded.Pins 3 and 4 are the ones that stop this fix degrading into "never navigate", which
would strand the author on a package that no longer exists.
Ablation (implementation committed first, restore trap armed only after): reverting
the distinction —
let list: PkgEntry[] = []plus the bare!list.some(...)guard,keeping the reporting — with the mutation proven on disk before running (blob hash
781e0c89to65393017; anchor counts flipped1/1/0/0to0/0/1/1). Result:Pin 2 fell with it — the eviction unmounts the switcher, so the trigger cannot be found
— which is the eviction's own signature. Pins 3, 4 and 5 stayed green, so pin 1 is
not merely restating them. No rebuild was needed: the pins import
./StudioDesignSurfacerelatively, inside the package, so nothing resolves throughdist/or package exports. Restoration proven by state:git diff HEADempty, diskhash equal to the HEAD blob,
git statusclean, anchors back to1/1/0.What was run (on
a95ee67a9, the head of this branch)pnpm exec vitest run packages/app-shell/src/views/studio-design/ apps/console/src/components/StudioRoute.test.tsxTest Files 50 passed (50)·Tests 276 passed (276)pnpm --filter @object-ui/app-shell run type-check(tsc --noEmit && tsc -p tsconfig.test.json)--listFilesconfirms both changed files are in the programpnpm --filter @object-ui/app-shell run lintnode scripts/check-changeset-presence.mjs✅ 1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)pnpm changeset:check·check:vi-mock-specifiers·check:vi-mock-inherit·check:control-bytes·check:i18n-keys·check:lint-coverage·check:type-check-coverage·check:unreferenced-sources·check:i18n-driftDeclared narrowing: repo-wide
pnpm lint/pnpm testwere left to CI.turbo run lintruns each package's own
eslint .; the only package this diff touches is@object-ui/app-shell, whose full population (1078 files, counted from eslint's own--format jsonoutput, not from a hand-picked list) was linted here.eslint.config.jsdeclares no
project/projectService, i.e. no type-aware linting, so this diff cannotmove a verdict in a file it does not touch.
Scope
packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx(theonManageChangedsite only), one new test file beside it, one changeset. No new export,no signature change, no gate change.
Related, deliberately left alone: #7373 owns whether the recovery destination should
be a hard-coded
/homerather than the declared landing page. This PR only stops theredirect firing on a false premise; the destination it fires to on a real deletion is
byte-for-byte what it was.
Generated by Claude Code