fix(console): search the Applications page through the row's own label resolver (#4343) - #4346
Merged
Merged
Conversation
…l resolver (#4343) `AppManagementPage`'s filter read `(app.label || '').toLowerCase()`. `label` and `description` are `I18nLabel` in `AppSchema` (`string | Record<string, string>` in @objectstack/spec 17.0.0-rc.6), so a non-string label is spec-legal metadata — and an object is truthy, so the `|| ''` guard never fired and `.toLowerCase()` got the object. The TypeError was thrown inside `filter` during render, taking the page out rather than degrading search, and only once someone typed: `if (!searchQuery) return true` short-circuits the empty case. Both reads now go through the resolver the rows already render with — `appTitle` for the label (the one display-name helper #4307 introduced in this file) and the identical `resolveKeyedI18nLabel(…, t)` call the description paragraph makes — so search matches what the operator can see, and a future widening of the resolver reaches display and search together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
Reverse verification predicted four red cases and produced three. The filter is a chain of `||`, so a query the NAME satisfies short-circuits before the label is read: the map case searched `mapped` against `mapped_app` and passed on the UNFIXED page, never reaching the throwing term. It now searches a query that misses every app, which is the only way that term is evaluated, and the header records the rule so the next case written here does not repeat it. 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
|
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 #4343
Filed out of #4307 / PR #4344, which keyed this page's chrome and introduced the
appTitlehelper the rows name an app by. That card deliberately left this line alone — it is a different defect class — and recorded it unmeasured for triage.(Angle brackets are spaced below —
Record< string, string >— so GitHub's sanitizer does not eat the type unions. The same precaution PR #4344's body took.)Gate
PR #4344 was confirmed merged and its squash confirmed as the tip of
origin/main—734d186a02dc3e2ce74d46c83b7ec3f21bce6112— before this worktree was created; the branch is based on that exact commit. TheresolveKeyedI18nLabel(label, t)seam this PR reuses is the one #4344 established, not a re-derivation.The defect
apps/console/src/pages/system/AppManagementPage.tsx, the search filter:An object is truthy, so
|| ''never fired for one and.toLowerCase()received the object:Thrown inside
filterduring render, so it takes the page out rather than degrading search. And invisible until someone types, becauseif (!searchQuery) return truereturns before either read — the page mounts perfectly with the very metadata that kills it one character later. That asymmetry is now pinned as its own case.Reachability — measured, because it grades the severity
The issue left this open and said so. Measured against the vendored
@objectstack/spec17.0.0-rc.6:app.labelbe a non-string today?AppSchema.label: z.ZodUnion< [z.ZodString, z.ZodRecord< z.ZodString, z.ZodString > ] >—dist/app.zod-CH7IEmsS.d.ts:637.descriptionis the same union, optional.{ en: 'Storefront', 'zh-CN': '店面' })I18nLabelSchema = z.union([z.string(), InlineLocaleMapSchema]), objectstackpackages/spec/src/ui/i18n.zod.ts:173{ key, defaultValue }) spec-legal here?InlineLocaleMapSchemakeys are regex-constrained to BCP-47 tags ordefault, and the rejection message names the retired form verbatim: "not bykey/defaultValue, which was the retired key-reference form (#5055) and resolves to nothing"crm,showcase,todo(examples),setup,account,studio(packages/platform-objects/src/apps/*.app.ts)sys-organization.page.tsand siblings); the spec's own comment calls it "a delivered capability, not a convention the runtime ignores"Grading: reachable through authored metadata today, not live in first-party deployments. A partner or operator authoring
label: { en: 'Storefront', 'zh-CN': '店面' }gets a greendefineAppparse and then loses the Applications page on the first keystroke. So this is not latent-by-construction — nothing rejects the input — it is simply not exercised by the apps we ship. Changeset graded patch accordingly.Note the dialect inversion this measurement turned up, which the issue could not have known: the issue names the keyed form as the trigger, and that form is exactly the one the spec rejects for an app label. The reachable trigger is the map form. It does not change the fix —
.toLowerCase()on any object throws identically, and both dialects are covered — but it does change which case is the realistic one, so both are tested.The fix
Both reads now go through the resolver the rows already render with. No second dialect, no
String(label):appTitleis #4344's own one-display-name-per-row helper (it isresolveKeyedI18nLabel(app?.label, t)with the heading's|| app.namefallback); the description term is the identical call the description paragraph makes twelve lines below. Three consequences, in order of how much they matter:defaultValue, andapp.namewherever the heading itself falls back.I18nLabelnow admits an inline per-locale map — audit every read the compiler cannot see, and give Studio a way to author one #4163 widens the resolver, display and search gain the map form in the same commit instead of this filter being left behind a second time.Point 3 is why the label term routes through
appTitlerather than through a bare resolver call: one helper, one meaning of "this app's name".Same-shape sweep
Swept
apps/console/src/pages/system/for a string method applied directly to anI18nLabel-typed slot:Both hits are in this file and both are fixed. The five sibling pages (
AiPendingActionsPage,ApprovalsInboxPage,AuditLogPage,ProfilePage,SystemHubPage) have no same-shape read. Nothing different-shaped was found that needed filing; the one adjacent gap — that neither path resolves the inline map — is #4163's existing audit, cited rather than duplicated.Tests
AppManagementPage.search.test.tsx(new, 7 cases). #4344's two files are untouched — no edit, and both stay green — which the card required and which is also the honest split: they pin the writes and the keying, not the filter.The pack mock answers in a real language whose words share no substring with the call site's
defaultValue(app.crm.label→Vertrieb, neverCRM). That is what makes each keyed case two-sided: the resolved text matches and the authoringdefaultValuedoes not, which pins thetargument rather than merely pinning that some resolver was called. Droppingtwould resolve the label to a perfectly harmless string and only that case would notice.One mock detail is reproduced rather than invented: i18next returns
''for a null key (translate():if (keys == null) return '';, i18next 26.3.6dist/cjs/i18next.js:544). That is precisely the callresolveKeyedI18nLabelmakes when handed a map with nokey, so the map case describes the real page instead of the mock.Both of the package's
tscinvocations, repo-root vitest (objectui#3378), underflock /tmp/os-heavy-verify.lockwithNODE_OPTIONS=--max-old-space-size=4096.ESLint on the two changed files: 0 errors, 10 warnings — all the pre-existing
no-explicit-anyconvention this page and its sister tests already carry for app records.check:control-bytes: OK (4079 files).Reverse verification — the prediction was wrong once, and that is the useful part
Direction predicted before running: reverting only the page turns the keyed and map cases red by the TypeError, and leaves the plain-string control green.
First run: 3 red, not the 4 predicted.
The miss was real, not a miscount. The filter is a chain of
||, so a query the NAME satisfies short-circuits before the label is ever read — the map case searchedmappedagainstmapped_app, matched on the name term, and passed on the unfixed page having never reached the throwing expression. It was coverage-shaped and empty, the exact failure the repo's rejection-class rule warns about in the other direction.Fixed by handing that case a query no earlier term can answer (
zzz), which is the only way the label term is evaluated at all. Second run, with the corrected case:All four red by the TypeError itself, with the control, the empty-query case and #4344's 21 cases green — that asymmetry is the whole claim. The rule is now recorded in the file header so the next case written here does not repeat it: a case that matches on the name is testing the name.
Taken out with
git checkout origin/main -- < path >and restored withgit checkout < branch > -- < path >; nevergit stash.Generated by Claude Code