fix(ui): let a producer-marked refusal reach the drag-write surfaces - #6376
Merged
os-support-ai merged 2 commits intoAug 25, 2026
Merged
Conversation
The kanban card-move toast, the calendar reschedule toast and the OCC conflict dialog each substituted a generic string for a refusal the PRODUCER had marked as user-facing (`userMessage`, objectstack#9934), so a user was told "Save failed" where the author had written a sentence for them. All three now read the marking through the shared `declaredUserMessage` reader, which knows both places the adapter boundary parks it: the typed member on `ConcurrentUpdateError` and the details bag on `DataApiValidationError`. Nothing unmarked reaches the user - the reader answers null for it and every existing substitution stands, so objectstack#3821's protection holds by construction. The two toasts substitute; the conflict dialog augments. Its description also explains what the destructive "Overwrite" button does, which is affordance copy this surface owns rather than a refusal message, so the marking leads and that paragraph stays. Applies the objectui#5210 ruling, already implemented for the console form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
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 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 #5902
Applies the objectui#5210 ruling — a refusal the producer marked as user-facing must reach the user rather than being replaced by a generic string — to the drag-write surfaces. Declared-≠-enforced restoration: no accept set widens, no new key is accepted, and every existing substitution still governs every unmarked refusal.
All measurements below are on
e5fb2be3b, the head this PR pushes.1. The census, re-derived — it is three, not two
The card named two surfaces. Re-derived on this branch's base (
9180fd139):packages/plugin-kanban/src/ObjectKanban.tsxtoast.error(...)after a failed persistpackages/plugin-calendar/src/ObjectCalendar.tsxtoast.error(...)after a failed drag-writepackages/plugin-form/src/occSave.tsxSurface 3 confirmed independently:
occSave.tsxalready reached into the conflict error forcurrentVersionand had nouserMessagereader, so a 409 an author had marked showed the same canned sentence as every other 409. It did not already read the marking, so there is no fork to report here.Dependency direction checked before importing. All three packages already declare
@object-ui/reactindependencies, so the shared reader is reachable from each and nothing had to be duplicated. All three also already declare@object-ui/data-objectstackindevDependencies, which is what lets the pins build fixtures through the real boundary.packages/app-shell/src/views/RecordAttachmentsPanel.tsxbranches onisPermissionErrortoo but selects an empty-state kind rather than rendering server text — a different question, excluded by the card, and untouched.2. The fix
One reader everywhere:
declaredUserMessagefrom@object-ui/react. Nothing is duck-typed at any surface — the marking's landing spot is asymmetric (ConcurrentUpdateError.userMessageis a typed readonly member;DataApiValidationError's lands inerr.details.userMessage), and that reader is the one place that knows both.The two toasts substitute, exactly as
form.tsxdoes:The conflict dialog augments — a deliberate per-surface deviation, called out for review. Its description does two different jobs in one paragraph: it says why the write was refused, and it explains what the destructive
Overwritebutton will do.userMessageis a refusal message, not affordance copy this surface owns, so evicting the paragraph would leaveOverwriteunexplained on the one surface where the choice is irreversible. The marking leads, in its own right; the paragraph stays. Both halves are pinned, so the choice is visible rather than implied — flip it by changing one assertion if review disagrees.No new i18n key on any surface, so no locale pack moves.
3. Per-surface coverage: which error shapes, and both arms
15 new tests. Every fixture is built wire-shaped and pushed through the real
normaliseClientError— the same boundary a realdataSource.updatefailure crosses — following the pattern atpackages/react/src/utils/error-message.normalisation-boundary.test.ts. Hand-rolling the post-boundary shape would have pinned each surface against the test file's own idea of where the marking lives, which is precisely the asymmetry worth pinning.CONCURRENT_UPDATEVALIDATION_FAILEDThe 409 and 400 rows are the two shapes that land the marking in different places, so a surface pinned on one only would be half-fixed. The 403 column pins the contract's status-agnosticism: a marked 403 must beat the
errors.unauthorizedsubstitution, which is the branch the ruling was originally reported on.The marked arms assert the marked text itself, not that a toast appeared — the fixture's
userMessageis deliberately unlike every generic string these surfaces can produce (Save failed,You are not authorized to perform this action., and the raw server text), so the assertion cannot pass on the pre-fix path.The unmarked arms are the other half. Without them a fix that simply printed
String(error)would pass every marked arm, and that is a different defect — leaking raw server diagnostics to end users, which is what objectstack#3821's substitution exists to prevent. The unmarked-403 arm asserts both that the localized string is used and thatinsufficient privileges to update task t1does not appear.For
occSavethe second shape is covered as what that seam actually owes:saveWithOccrethrows everything that is not a conflict, so the pin asserts the rejection arrives at the caller unchanged (toBe, identity) withdeclaredUserMessagestill finding the marking — it cannot quietly swallow or re-wrap it on the way past.4.
rejectedMoveRollback— nothing movedpackages/plugin-kanban/src/ObjectKanban.rejectedMoveRollback.test.tsxis untouched; zero assertions changed, and it does not appear in this PR's diff.Its fixture (
invalidTransition()— a 400invalid_transition) carries nouserMessage, sodeclaredUserMessageanswersnulland the toast text is unchanged. Itsexpect(toast.error).toHaveBeenCalledWith('Invalid status transition')therefore still pins theextractWriteErrorMessagearm, and the rollback behaviour it guards is not weakened in any way. It ran green in both the red and the green runs below (5/5).5. Red before, green after — predicted first
Predictions were recorded before the pre-fix run: 7 of the 15 new tests should fail, being the 3 marked arms on each toast surface plus the marked-409 dialog arm; all 6 unmarked arms and all 5
rejectedMoveRollbacktests green on both sides.Pre-fix (
9180fd139+ pins only):Test Files 3 failed | 1 passed (4)·Tests 7 failed | 13 passed (20)All seven failures matched the predicted identity exactly — no prediction was wrong in either count or identity:
Post-fix:
Test Files 5 passed (5)·Tests 28 passed (28)(the 15 new +rejectedMoveRollback5 + the pre-existingoccSave.test.tsx8).6. Ablation — are the unmarked arms load-bearing?
The red run above already establishes that the marked arms gate the fix. What it cannot show is whether the negative arms do any work, so they were ablated separately, after the fix was committed (so restoration had a safe anchor).
Mutation: in
ObjectKanban.tsx, keep the marking read but drop theisPermissionErrorsubstitution — i.e. the plausible wrong fix that leaks raw server text.Predicted: exactly 1 failure, and specifically
keeps the localized substitution for an UNMARKED 403; all three marked arms and the other two unmarked arms stay green.Actual: exactly that.
Mutation confirmed on disk before measuring — injected marker present ×1, removed anchor text
errors.unauthorizedpresent ×0, and the mutated blob hash differing from theHEADblob — so this was not a no-op reading. The script carried atrap … EXIT INT TERMwith absolute paths. Restore leg proven by bytes, not by an exit code:git hash-objectreturned2c99db839a55bcbbb25e6599fb8140f441967ab6, identical to theHEADblob, withgit diff HEADempty anderrors.unauthorizedback to ×1.7. Gates
Derived from the CI job step lists under
.github/workflows/(ci.yml,lint.yml, and the standalone gate workflows). Each row quotes the gate's own verdict line; exit codes were captured before any pipe.check-changeset-presence.mjs✅ 6 source file(s) of 3 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major.mjs✅ No changeset declares a major bump.check:control-bytes✅ check-control-bytes: OK (scanned 5248 tracked text file(s); skipped 85 binary).check:vi-mock-specifiers✅ check-vi-mock-specifiers: OK (… 443 carry a mock …)check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.check:self-import✅ No package names itself inside its own src/.check:i18n-keysEvery in-scope call-site key resolves against the en pack (2824 keys) …check:i18n-driftNo en value changed in this range.type-check:coverage✅ type-check coverage: 45/46 …·✅ test type-check coverage: 41/41 packages compile their testslint:coverage✅ lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total).check:entry-guard✓ check:entry-guard: 47 scripts/ file(s) — no entry guard outside the baselinecheck:spec-symbols✅ spec symbol derivation: 1306 files scanned against 4959 spec export namescheck:esm-specifiers--specifiers-only: the load leg (which builds) was not run.(specifier leg only)check:pre-install-import-graph✅ check-pre-install-import-graph: OKcheck:shell-escape-residue✅ check-shell-escape-residue: OK (4/4 root(s) resolved …)turbo run type-check(3 pkgs)Tasks: 17 successful, 17 totalturbo run lint(3 pkgs)Tasks: 4 successful, 4 totalvitest run packages/plugin-{kanban,calendar,form}/Test Files 97 passed (97)·Tests 862 passed (862)vitest run scripts/__tests__/Test Files 79 passed (79)·Tests 2279 passed (2279)NOT MEASURED locally — recorded as such rather than as green, because each says in its own words that it measured nothing:
check:eager-closure❌ No eager-closure report at apps/console/dist/eager-closure.json … This is a broken gauge, not a passing budget.Needs a consolevite build; CI builds it.check:readme-exports❌ check-readme-exports: the population COLLAPSED -- this run proves nothing— 23 of 40 packages unbuilt in this worktree; CI installs and builds.check:published-distBuild finished in 409s.then SIGTERM). Not implicated: nopackage.jsonor build config changed.half-state-patrolis a scheduled board sweep triggered byscripts/pm/check-half-states.mjs(untouched) — N/A, not a gate this diff owes. The docs, shadcn, spec-floors and skills-paths workflows match no path in this diff.Type-check actually covered the new tests. These packages run
tsc --noEmit && tsc -p tsconfig.test.json, and that second project is the one that compiles tests. Verified rather than assumed —--listFilesreports each new test file present in its package's program (1 hit each), so the green above is not the "compiled, but excluded your tests" reading.Repo-wide
pnpm lintwas narrowed, and the narrowing is measured. Ranturbo run lintscoped to the three packages — CI's own command, over each package in full rather than only the changed files — plus a--format jsonrun over the diff. Three pieces of evidence that the narrowing excluded nothing: (1) the population comes from eslint's own config resolution, not a guess about which files count; (2)--format jsonreportsfiles linted: 6 | errors: 0 | warnings: 104; (3) linting here is not type-aware —eslint.config.jsdeclares noparserOptions.projectand noprojectService— so no rule reads cross-file type information and this diff cannot move the verdict on any file it did not touch. The 104 warnings are pre-existingno-explicit-any/react-hooks/*findings on the two large existing components (ObjectCalendar.tsx68,ObjectKanban.tsx33,occSave.tsx3); all three new test files are 0 errors / 0 warnings, and the source edits add noanyand no hook.CI still runs the full farm exactly once regardless.
8. Out of scope
Filed as #6375 (unassigned,
finding):normaliseClientError's firstCONCURRENT_UPDATEguard is subsumed by the line under it, so itshttpStatus === 409half decides nothing. Recorded rather than edited —packages/data-objectstackis scoped out of this card, and this PR does not touch it.Generated by Claude Code