fix(console): an unloadable app list is UNKNOWN, not "no default app"; wire the Applications page's writes (#4233) - #4300
Merged
Conversation
…; wire the Applications page's writes (#4233) Half A — the `/` landing resolved a path from an app list it had failed to load. `MetadataProvider.ensureType` catches a failed `GET /meta/app` and resolves `[]`, so `loading` goes false, nothing rejects, and the error arrives wearing exactly the shape of the answer: `resolveLandingPath([])` falls through to `/home` and reports "this deployment has no default app" about a deployment it never managed to ask. `Navigate … replace` then fossilizes it — a reload re-enters at `/home`, and a dead session gets `?redirect=%2Fhome` captured and honored after sign-in. `/` now resolves a landing only from a list that is an ANSWER, gated on the metadata context's own per-type status (`getTypeStatus('app')`) — one source of truth, no second dialect of loading/auth state. The landing POLICY is untouched: every `resolveLandingPath` rule, the empty-list fallthrough included, still answers exactly as before when the list genuinely loaded. While the list is unknown the console holds at `/` and re-asks once, so a transient failure heals and an outage settles on a screen that is not a claim about which apps exist. The originally reported pre-auth link is already closed by #4042 (which wrapped `/` in ProtectedRoute); that door, and the deep-link `?redirect=` capture that must keep working, are pinned here for the first time. Half B — Set as default, Disable, the bulk toggle and Delete each showed a success toast having issued no request, then called `refresh()`, which re-rendered unchanged server state underneath the confirmation. Their TODO's premise was measured against @objectstack/client 17.0.0-rc.6 and is false: the write surface exists (`meta.saveItem` / `meta.deleteItem`, gated on `manage_metadata`) and is already how app schemas are persisted elsewhere in this console. All four handlers now await a real mutation and report success only afterwards; a refusal surfaces the server's message. Set-as-default demotes the outgoing default first so the landing cannot depend on list order, and the bulk toggle counts what actually landed. 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
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 12:25
This was referenced Aug 11, 2026
Merged
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 #4233. Touches the landing resolver #4048 last changed, and finishes the entry-path work #4042 started.
Premise check first — one of the two halves had partly expired
Rule: verify the card against
origin/mainbefore implementing. Half A's stated chain has four links, and link 3 turned out to be about a build that is no longermain.QA ran vendored console
09987b68(2026-08-09). At that commit the root route was:— no auth guard above the resolver, so an unauthenticated visitor really did run the whole resolution against a list emptied by a 401,
Navigated to/home, and the guard on/homethen captured that already-wrong path into?redirect=%2Fhome. #4042 (commit41d602274, 2026-08-10 — one day after that build, one day before this card was filed) wrapped/inProtectedRoute. On currentmainan unauthenticated visitor never mounts the resolver at all, andLoginRedirectdeclines to capture the bare/(redirect !== '/'), soLoginPagefalls back to/and the resolver re-runs post-auth against a real list.So the cold-sign-in reproduction is already green on
main— not because of anything in this PR. The card's re-verification atbb68488checkedRootLandingRedirect.tsx(correctly: unchanged) and carried link 3 forward from the observation, which was made on the older bundle.What has not expired is the principle, and the defect it names is still live. The guard proves a session resolved; it does not prove
GET /meta/appsucceeded. Behind a valid session — server restart, 5xx, a request racing a session refresh —ensureTypecatches and resolves[],loadinggoes false, nothing rejects, and the error arrives wearing exactly the shape of the answer.resolveLandingPath([])then reports "this deployment has no default app" about a deployment it never managed to ask, andNavigate … replacemakes it stick:/is rewritten to/home, a reload re-enters at/home, and if the session does turn out to be dead the guard captures/homeinto?redirect=after all. Same defect, narrower door.Half A — mechanism: option 1, because option 2 is already satisfied
The ruling offered two mechanisms. Measuring the code decided it:
?redirect=) is already true onmainand needs no change —/is guarded, and the bare/is explicitly not captured. ChangingLoginRedirecthere would also have meant editing the console auth-guard module whiletype: 'form'action fired from a record opens an empty CREATE form —/forms/:nameignores the?recordId=ActionRunner forwards #4278 is in flight, for no behavioural gain. It is instead pinned in both polarities so it cannot silently regress: a legitimate deep link (/apps/crm_app/record/1, query string included) is still captured and honored; the bare/still is not.RootLandingRedirectproduces no conclusion from a list that is not an answer.The distinguishing fact already existed on the metadata context, so no second dialect of loading/auth state was added:
getTypeStatus('app')—MetadataProvider's own per-type status (idle | loading | ready | error), which the provider already sets toerrorinensureType's catch. One source of truth, read where the decision is made.resolveLandingPathis unchanged. Its rules were never wrong — including[] ⇒ /home, which is right when the emptiness is real. The new predicate gates whether the policy runs at all, so #4048's Setup-only pins and every other policy pin stay green untouched.Because a wrong conclusion is never produced, the
?redirect=%2Fhomeamplification is closed at its source rather than patched at the capture site."No conclusion" is made recoverable rather than terminal: the component holds at
/on the loading fallback and re-asks the metadata layer once (bounded by a ref, not by effect-dep identity), pastERROR_RETRY_COOLDOWN_MSso the re-ask is not answered from the failed cache entry. A transient failure heals with no user action; a real outage settles on a screen that is at least not a claim about which apps exist — which is what/homewas, since the launcher reads the same failed list.Reverse verification — predicted, run, and the prediction corrected
Predicted: reverting the gate turns the two conclusion cases red, everything else green.
Measured: 11 passed / 3 failed — three red, not two. The extra one is
re-asks the metadata layer exactly once, and the mechanism is worth recording rather than papering over: without the gate the componentNavigates away on its first render, unmounts, and the effect cleanup clears the pending timer, so the re-ask never fires. The retry is not a second behaviour bolted on beside the gate — it is only reachable because the gate keeps the component mounted. One seam, three pins.Everything else stayed green, which is the property that matters: the cold-sign-in and deep-link cases cannot go red for this change's reason, so they remain independent evidence that the pre-auth door is still shut.
Half B — the API exists, so the controls are wired
Measured before deciding, per the ruling.
@objectstack/client17.0.0-rc.6 exposesmeta.saveItemandmeta.deleteItem; the objectstack route ledger carriesPUT /api/v1/meta/:type/:nameandDELETE /api/v1/meta/:type/:name, both gated onmanage_metadata(ADR-0066 D1).useNavigationSyncin@object-ui/app-shellhas been persisting app schemas throughmeta.saveItem('app', …)all along. The TODO's premise is false — this was never blocked on the backend — so the disabled-with-a-notice branch does not apply and the handlers are wired.Each handler now awaits a real mutation and reports success only afterwards; a refusal surfaces the server's own message instead of a lie. Two details that are judgment, not transcription:
resolveLandingPathtakes the firstisDefaultmatch, so two holders is not a cosmetic inconsistency — it makes the landing depend on the order the server happens to list apps in. Demote-then-promote also means a mid-way failure leaves the deployment with no default (rule 3,/home) rather than two.The causal note from the card: because these controls never wrote, an operator could not set
isDefaultfrom the console at all — the app had to be republished through metadata — so half A had no in-product workaround. Half B is what restores one.Reverse verification
Restoring
origin/main's stub handlers wholesale: 12 of 12 red, in both directions — theissues a real …cases fail on the missing client call, and the error-path cases fail because a stub cannot fail and so reports success where the server refused. That pair is precisely what the stubs were able to satisfy before, which is why no assertion here rests on a toast appearing.Verification
pnpm exec vitest run apps/console/— 39 files, 421 tests, all passing (26 of them new).tsc --noEmitandtsc -b tsconfig.node.json --forceinapps/console— both exit 0, after building the dependency closure (pnpm --filter '@object-ui/console^...' build).eslinton all five changed files — 0 errors. Remaining warnings are the file's pre-existinganyparameters; thereact-hooks/refswarning an earlier draft introduced was removed by dropping the ref.check-changeset-presence.mjsgreen,minor, nevermajor).Scope
apps/consoleonly — nopackages/*touched, and none ofFormPage.tsx/createdRecordPath.ts/ the #4279 form-route modules, so nothing overlaps #4278. No new user-facing strings, so no i18n resource changes were needed.Generated by Claude Code