Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-list): separate what the sort picker lists from what it persists - #6583
Conversation
…rsists The toolbar sort picker keeps a platform-refused field listed while the current sort names it — the only way a user can remove a sort the server refuses outright (#6108). But it rendered and emitted from the same array, so any other edit in the popover re-emitted the refused entry and the host turned it into `persistViewPatch({ sort })`: a personalization PUT storing a column the platform answers `400 INVALID_SORT` for, written by a user who never touched that row. Route every `onSortChange` through one emit boundary that drops what the served projection refuses, and leave `currentSort` whole so the entry stays listed and removable. Same separation #5729 made at the grid seam.
A published behaviour fix in @object-ui/plugin-list, scored patch — `scripts/check-changeset-presence.mjs` demanded it and now passes.
✅ 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
commented
Aug 26, 2026
ACCEPT on the diff. ⏸ Landing held until every check is green — 17 success / 3 skipped / 9 still running, zero failures at The seam, verified from the branch rather than the reportThe narrower seam was chosen and One boundary, and the control proves it is the only one. The hard constraint survived, and is documented where it matters. Those two adjacent lines are the whole card: the same array, one to state unfiltered so the refused entry stays listed and removable, one through the filter so it never reaches a write. And the change is scoped to a served projection — Two corrections to the dispatch order, both from the devA third door existed and my order named only two. And a fix that was declined rather than claimed.
|
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
commented
Aug 26, 2026
Dequeued This PR is untouched by itOn the reviewed head Why it timed outThe repository-wide Actions runner outage recorded in #6588. Measured just now across the 30 most recent workflow runs of any event type:
This PR was enqueued first of three (15:05:11Z), so it aged out first. #6585 (15:07:41Z) and #6586 (15:11:46Z) are still enqueued and will presumably follow.
Why not re-queue immediatelyRe-queueing is normally the right move for a transient dequeue, and this seat did exactly that for #6571 earlier today. It is the wrong move right now: with zero runner throughput, a re-queued PR would sit and time out again, spending the one-re-queue allowance on a certainty and adding load to the exact resource that is starved. ⛔ Not re-queued. ⛔ No push, no re-run, no change to the branch — there is nothing here to fix. ⛔ And explicitly not a case for weakening anything: a queue timeout caused by unavailable runners is not a gate to relax. What this needs: runner throughput to return. The signal this seat is watching for is any run reaching If the outage turns out to be org-side runner concurrency rather than a GitHub incident, that is the lever — and whoever can reach Generated by Claude Code |
Fixes#6455
ListView's toolbar sort picker keeps a platform-refused field listed while the current sort names it. That exception is deliberate and stays: it is the only way a user can REMOVE a sort the server answers400 INVALID_SORTfor. But the picker rendered and emitted from the same array, so any other edit in that popover — a second sort key, a reset to the view's default — re-emitted the refused entry, and the host'sonSortChangeturned it intopersistViewPatch({ sort }): a personalization PUT storing a refused column, written by a user who never touched that row.The seam, and why this one
Intercepted in
packages/plugin-list/src/ListView.tsx, not inpackages/app-shell/src/views/ObjectView.tsx. Three reasons, in order:ObjectViewis a correct consumer of a leaky emit.readObjectSortability(objectDef)for its render leg.ObjectViewwould have to re-derive the same projection to filter the write — a second copy of the judgement thatisPlatformSortableFieldwas consolidated to make singular.ObjectViewhas twopersistViewPatch({ sort })call sites, and the grid seam is already filtered. Filtering there would double-filterObjectGrid's already-clean emit and still leave any future non-ObjectViewhost ofListViewleaking.The shape converges on the grid precedent (#5729):
ObjectGrid'smanualSort/manualOnSortChangepair separates render from emit. This picker was the second door onto the same stored view state.What changed
platformSortability), shared by the render leg and the emit leg — read twice, the two copies could answer differently about the same field on the same render.emitSortChangeboundary that everyonSortChangein the component crosses: the builder'sonChange,handleHeaderSort, andhandleResetSort. A filter spelled three times is a filter that one new door can be added without.currentSortstill takes the array whole, so the refused entry stays listed, removable, and still the order this list asks the server for.undefinedis NO SIGNAL SERVED (an older deployment, an inline/mock data source), not "nothing is sortable" — that branch is unchanged.Net:
packages/plugin-list/src/ListView.tsx+58 −8, new test +258, changeset +32.The leak, reproduced before it was fixed
packages/plugin-list/src/__tests__/ListView.sortPersistLeak.test.tsxmodels the host exactly asObjectViewwrites it (onSortChange→ apersistViewPatch({ sort })spy), so what it reads is the payload that reaches stored view state.Reverse-verification, direction predicted first (predicted: the two fix-direction tests go red, all four controls stay green). The fix was committed first, then the pre-fix expression was restored from the merge-base
12402a9b8, the mutation was confirmed on disk (emitSortChangecount 0, pre-fix spelling count 1, blob hash differing from HEAD's), and the file was restored withgit checkout HEAD --and proven byte-identical (hash-object= HEAD blobf52456576e7b4be7589b388a380510683c99eb49,git diff HEADempty in worktree and index).The two received values are the leak:
[{remote_status,asc},{name,asc}]written by a user who only added a second sort key, and[{remote_status,asc}]written by a reset. The four controls that stayed green through the mutation are the ones the naive fix (filtering the array the picker renders from) would fail:['Name','Amount','Remote Status'];sort: []);[amount, name], in order);[remote_status, name]).RelatedList — measured, out of scope by construction
The card asked for this to be confirmed rather than assumed.
packages/plugin-detail/src/RelatedList.tsx: 84 hits forsort(positive control — the sort surface is very much present), 1onSortChange, and it is an inbound prop handed to the embedded table (onSortChange: handleTableSort), whose handler only callssetSortField/setSortDirection/setCurrentPage. 0persistViewPatch, 0persistView, 0localStorage.RelatedListPropsdeclares no sort callback at all (onRowClick,onRowAction,onToolbarAction— no sort), so no host can persist it: the sort is component state that dies with the component. Nothing to leak, and nothing to file.Gates (exit code captured before any pipe; verdict lines are each gate's own)
Union re-run at final HEAD
2914bd516, clean tree, from the repo root per AGENTS.md §怎么跑测试:pnpm exec vitest run packages/plugin-list/Test Files 49 passed (49)/Tests 696 passed (696)turbo run type-check --filter=@object-ui/plugin-listTasks: 14 successful, 14 totalturbo run lint --filter=@object-ui/plugin-list✖ 422 problems (0 errors, 422 warnings)node scripts/check-changeset-presence.mjs✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjs✅ No changeset declares a major bump.node scripts/check-changeset-overwrite.mjs✅ No pre-existing changeset was modified or deleted.node scripts/check-control-bytes.mjs✅ OK (scanned 5437 tracked text file(s))node scripts/check-lint-coverage.mjs✅ 46/46 packages linted, 0 with outstanding errorsnode scripts/check-type-check-coverage.mjs✅ 41/41 packages compile their testsnode scripts/check-vi-mock-specifiers.mjs✅ OKTwo notes on what those readings do and do not cover:
type-checkistsc --noEmit && tsc -p tsconfig.test.json;--listFileson the second project names bothListView.tsxandListView.sortPersistLeak.test.tsx, so "type-check clean" is a statement about the new file and not a true sentence about a set it excludes.eslint .insidepackages/plugin-list, the exact task CI runs for it),--format jsoncounts 60 files with 0 errors, andeslint.config.jsenables no type-aware linting (noprojectService, noparserOptions.project), so this diff cannot move the verdict of any file it did not touch. The new test's 9 warnings are all@typescript-eslint/no-explicit-any, the same class (and fewer) than its The other two sort-axis consumers still re-derive sortability from field type — ListView's sort picker and RelatedList #6108 sibling — no new warning family. The repo-wide farm is CI's run.Out of scope, untouched
filterPlatformSortableSort's definition inpackages/coreandObjectGrid's use of it (the precedent, not the subject);packages/plugin-view/src/ObjectView.tsx(a different file with the same component name, in flight elsewhere); anything server-side under objectstack-ai/objectstack#10235, which objectui consumes as a published package.Generated by Claude Code
Generated by Claude Code