A cloud-connection bind failure reads the locale whichever clock noticed it (#5054) - #5659
Merged
Merged
Conversation
…the wire English (#5054) One device-approval expiry could be noticed by the panel's own `expires_in` clock (rendering `cloudConnection.errors.expired`, translated in ten packs) or by the server (HTTP 400, rendering the producer's English sentence verbatim). One condition, two languages, decided by which clock got there first. `getJson` now carries `declaredCode`/`code` across its throw, and a closed map turns the two user-causable RFC 8628 spellings into console copy — `expired_token` -> the existing key, `access_denied` -> a new key in all ten packs. Unrecognized codes keep rendering the wire `message`.
`tRef.current = t` in the render body is what `react-hooks/refs` is for, and it was a lint finding this branch introduced. The effect form keeps the helper's identity stable (which is what stops `refreshStatus` re-running the mount effect on every state update) without writing a ref during render. `failureText` takes `unknown` rather than `any` for the same reason: the branch should not add a warning to a file it is fixing. Runtime is unchanged -- `message` when it is a string, `String(err)` otherwise, exactly as before.
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
|
os-sales
marked this pull request as ready for review
August 22, 2026 07:30
This was referenced Aug 22, 2026
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
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 #5054
One abandoned device approval can be noticed by either of two clocks, and the Cloud Connection panel had a different answer for each. This makes both answers the same string, in the user's language.
The dispatch fenced
packages/app-shell/**as OUT (held by #5344 / #5025) and, in the same claim comment, granted this card "the cloud-connection bind-failure message sites plus the locale packs they resolve through". Those two sentences cannot both be honoured literally: the panel that renders a bind failure ispackages/app-shell/src/console/cloud-connection/CloudConnectionPanel.tsx, and there is no other renderer — a fence read literally makes this card unimplementable rather than merely narrower.I verified the fence's own premise rather than assuming it:
packages/app-shell/src/views/metadata-admin/inspectors/ViewColumnInspector.tsxpackages/app-shell/src/views/ObjectView.tsxBoth are under
src/views/**; this PR touches onlysrc/console/cloud-connection/**pluspackages/i18n/src/locales/**. Disjoint at file level, so no git conflict is possible with either holder.I implemented on the reading that makes both sentences satisfiable (the fence excludes the held regions, the grant names my region) — but I am flagging rather than deciding. This is a draft PR: if the PM meant the fence literally, hold it and I will re-run once #5344 / #5025 land. Nothing here is merged.
What each side rendered BEFORE the fix
Measured on
7e811687a, both paths, before any edit:/bind/poll→ HTTP 400 →getJsonthrows →catchrenderserr.messageDevice authorization failed: expired_token— English, on all ten localesexpires_indeadlinepoll()'stickguardcloudConnection.errors.expired— translated, on all ten localesThe
SYMMETRYcase states it in one diff, from the pre-fix run:Producer confirmed at source, not from the card: objectstack
packages/cloud-connection/src/cloud-connection-plugin.tsserves the terminal exit as 400 with{ code: 'DEVICE_CODE_FAILED', declaredCode: <RFC 8628 spelling>, message: 'Device authorization failed: <spelling>' }.codeis the same for both outcomes — the discriminating spelling is only indeclaredCode.The fix
Adjudicated disposition (Option A restricted + B fallback), implemented as ruled:
getJsonnow carries the envelope'sdeclaredCodeandcodeacross its throw. A bareErrordropped them, so the message was the only thing that survived — that is the whole mechanism of the asymmetry. Attached rather than subclassed (extendingErroris brittle under a downlevel target).declaredCodefirst thencode:expired_token→ the existingcloudConnection.errors.expired,access_denied→ a newcloudConnection.errors.accessDeniedadded to all ten packs. Thet()arguments are string literals socheck:i18n-keysresolves them; aRecord<code, key>read ast(key)is a dynamic key and that gate goes blind to it.poll()'s terminal 2xx branch too, so the asymmetry cannot simply move to a third reader.failureText, so no future call site can reintroduce the split.invalid_grant, anything upstream invents next — still renders the wiremessage.Neither key takes interpolation, so interpolation parity is vacuous here and
check:i18n-keysconfirms it (0 with an unreadable option set).Both sides get a case, and each case gets a mutation leg
CloudConnectionPanel.bindErrorLocale.test.tsx(new, 6 cases). Directions were written into the file header before running. Pre-fix:SERVER expiredRED ·SERVER deniedRED ·SYMMETRYRED ·CLIENTGREEN ·CONTROLGREEN — the two green ones green on purpose (the side already correct, and the B-fallback), which is why each has its own leg below rather than being counted as silent evidence.All four legs were re-measured against the shipped commit
01576cb96; each mutation was proved on disk in both directions before its reading was accepted (a pristine-count assertion, then a post-mutation assertion that the anchor text left and the replacement arrived), and each ran under anEXIT INT TERMtrap that restores the tree.git status --porcelainwas empty after every leg.translateFailureCode(tRef.current, e?.declaredCode, e?.code)→(…, undefined, undefined)4 failed | 6 passedt('cloudConnection.errors.expired')→ a hard-coded English literal2 failed | 8 passedaccess_denied→invalid_grantin the map2 failed | 8 passedgetJson's non-2xx guard disabled, so a 400 reachespoll()'s terminal branch1 failed | 9 passedThe route leg is the informative one for the design: with the 400 delivered to the other reader,
SERVER expiredandSERVER deniedstay green, which is the direct evidence that the map sits at both readers rather than at one.One leg was attempted and did not run: the first
map-overreachattempt passed a two-line replacement, andgrep -Ftreats an embedded newline as multiple patterns, so the pristine-count precondition read 1 instead of 0 and the runner aborted withABORT: anchor not pristine — this leg did NOT run. It was re-done single-line. Recording it because a no-op ablation that is quietly retried until something lands is the same defect one level up.Fixture triage on the sibling suite
CloudConnectionPanel.bindError.test.tsx's CONTROL case asserted the pre-fix wire English — the exact symptom this card retires. It was not deleted and its fixture was not touched (it is still the producer's verbatim envelope): the assertion moved to the post-fix key, and theDEVICE_CODE_FAILEDnegative stays, since "no machine code reaches a human" is that file's own subject.What that case lost is real and is called out in its comment: the wire English used to discriminate the two readers, and post-fix both readers answer
expired_tokenidentically. The measurable discriminator is replaced by the newROUTEcase, which is the one fixture the two readers still answer differently — a 400 carrying noerrorobject at all rendersHTTP 400fromgetJson's last arm, wherepoll()'s terminal branch would rendercloudConnection.errors.bindFailed. Its own mutation leg above measures it failing.Clause ② — nothing reachable from a published entry was added or widened
Measured from the built entries, not from a grep of a barrel (
export *propagates without naming).@object-ui/app-shell—exportsmap has exactly two entries (.,./styles.css), unchanged.dist/was cleared together withtsconfig.tsbuildinfobefore each build (compositetscskips emit if the build info survives), then the whole emitted type surface was hashed base vs HEAD:.d.tsfiles byte-identical (sha256 manifest diff, empty);.entrydist/index.d.ts→8c886251f0e086ab56a99236ce5f899c41b8ad33b027d453531c11136a10dbc3on both sides;import './console/cloud-connection/CloudConnectionPanel.js';— no named export, unchanged frommain.translateFailureCode,ApiFailureandfailureTextare module-local and appear nowhere in it.@object-ui/i18n— two published entries. The.entrydist/index.d.tsis byte-identical. The./locales/*subpath entries changed by exactly the key that was added and nothing else:accessDeniedlocales/{ar,de,en,es,fr,ja,ko,pt,ru,zh}.d.tslocales/index.d.tsNo export added, no signature widened, no resolver changed what it accepts. That is the "adding a locale key" case, which the dispatch states is not a widening — and it is stated here as a measurement rather than as a claim.
Verification
Union re-run after the final commit, at
01576cb96. Each gate's own verdict line, never a bare$?(exit status captured from a redirect, before any pipe):check:i18n-keys→ EXIT=0 — "Every in-scope call-site key resolves against the en pack (2919 keys), every literal inline defaultValue matches the value the pack serves, every call site passes exactly the arguments that value has holes for, and no call site carries a literal fallback beside itself."check:i18n-drift→ EXIT=0 — "No en value changed in this range." (0 en value(s) changed (1 key(s) added, 0 removed …)) — an addition, not a drift.check:control-bytes→ EXIT=0 — "✅ check-control-bytes: OK (scanned 4705 tracked text file(s); skipped 85 binary)."check-changeset-presence.mjs→ EXIT=0 — "✅ 13 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)."check-changeset-no-major.mjs→ EXIT=0 — "✅ No changeset declares amajorbump."check-changeset-fixed.mjs→ EXIT=0 — "✅ All workspace packages are in the changeset fixed group."type-check(@object-ui/app-shell,@object-ui/i18n) → EXIT=0, both packages echoingtsc --noEmit && tsc -p tsconfig.test.jsonand reportingDone— the echo is the guard against a zero-match--filterexiting 0 without running anything (this repo spells the scripttype-check, with the hyphen).vitestfrom the repo root,pnpm exec vitest run packages/app-shell/src/console/cloud-connection/ packages/i18n/src→ EXIT=0, 52 files / 888 tests passed, including all 6 new cases, the 4 re-triaged sibling cases andcloudConnection-locale-parity(identical key set in all ten packs, non-empty translated leaf at every path).check:i18n-dead-keys(report-only, wired into no workflow) → the new key does not appear in the candidate list.Lint: a declared narrowing, with its three measurements
turbo run lintcovers 47 of 47 workspace packages. I ran 2 of 47 in full — the two this diff touches — and the narrowing is a measurement, not an omission:lintscript iseslint ., run unmodified.--format json:@object-ui/app-shell= 922 files linted (0 errors),@object-ui/i18n= 75 files linted (0 errors).eslint.config.jsis the single config in the repo (no per-packageeslint.config.*exists) and it declares neitherprojectServicenorparserOptions.project— type-aware linting is not enabled, so every file's verdict is computed from that file plus the config alone. A diff confined toapp-shellandi18nsources therefore cannot move the verdict of any file in the other 45 packages.Net lint findings on the changed file are identical to
main, measured by rule:Both new/changed test files: 0 errors, 0 warnings. Getting there took a correction worth naming — the first shape wrote
tRef.current = tin the render body, whichreact-hooks/refsflags, and typed the helper(err: any), which added a sixthno-explicit-any. Both are fixed in01576cb96: the ref syncs in an effect, and the helper takesunknown. Runtime is unchanged (messagewhen it is a string,String(err)otherwise, exactly as before) and all four legs were re-measured against that final shape.One implementation hazard, recorded
Routing
tintofailureText'suseCallbackdependency list maderefreshStatusunstable, which re-ran the mountuseEffecton every state update — an infinite render loop that timed out all nine cases in this directory at 15s. Several translation providers hand back a freshteach render, sotmust not reach a dependency list the mount effect transitively depends on. The latest-ref (synced in an effect) keeps the helper's identity stable. The mechanism is written into the code at the ref, not just here.Not touched
content/docs/releases/(the changeset is this PR's release-notes input) · noskills/**path is in this diff, so the published-skills line-count readings do not apply · no.claude/**,AGENTS.md,CLAUDE.mdordocs/adr/**· no producer-side change was asked of objectstack (Option C stays recorded as a future maintainer call only).CI has not converged at the time of writing — per the dispatch contract the report is delivered at draft-PR time, and gate status is whatever CI says.
Generated by Claude Code