Uh oh!
There was an error while loading. Please reload this page.
refactor(app-shell): call owd-sharing's isExternalWider from ObjectSettingsPanel, pinned across both surfaces - #5579
Merged
Conversation
…ttingsPanel (objectui#5477) The per-object Settings tab re-declared `OWD_WIDTH` and the ADR-0090 D11 "external must never be wider than internal" comparison inline, while `owd-sharing.ts` — which states its own purpose as "the single home" for the pieces this tab and `PackageOwdOverviewPanel` must agree on — exported `isExternalWider` doing the same thing. The two were verified equivalent before the swap, over the full domain of both dials plus `undefined`, the rejected legacy aliases and prototype-chain keys: 144 pairs, zero disagreements. No author-visible verdict changes. Adds the agreement pin the finding asked for: both surfaces are driven over the full 5x5 cross-product of the values their dials offer, and the Settings tab's rendered warning, the overview's per-row error and `isExternalWider` itself must agree on every pair, with the violating pairs pinned by name so the sweep cannot pass vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
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 21, 2026 14:51
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#5477
ObjectSettingsPanelre-declaredOWD_WIDTHand the ADR-0090 D11 "external must never be wider than internal" comparison inline, whileowd-sharing.ts— which states its own purpose as "the single home" for the pieces this tab andPackageOwdOverviewPanelmust agree on — already exportedisExternalWiderdoing the same thing. This swaps the inline block for the import and adds the agreement pin the finding asked for.The two copies AGREED — this is not a behaviour change
The card warned they might have drifted. They had not, and that was measured rather than read off. Both implementations were transcribed verbatim from
origin/main@ac73c24b0and enumerated over the full input domain — the five values either dial offers, the rejected legacy aliases (read/read_write/full),undefined, and prototype-chain keys (toString,constructor,__proto__) thatinreaches:The module's extra
!!internal/!!externaltruthiness guards are redundant at this call site, which already normalizes both dials to''viatypeof … === 'string' ? … : '', and'' in OWD_WIDTHis false regardless. Over the 25 pairs the dials can actually produce, the same three warn before and after:private → public_read,private → public_read_write,public_read → public_read_writeNothing an author sees changes. The one thing worth a reviewer's eye is argument order: the module takes
(internal, external), the reverse of how the violation reads, so the call isisExternalWider(sharingModel, externalSharingModel). A comment at the call site says so, and the pin below fails if it is ever swapped.PackageOwdOverviewPanelwas already a correct consumerChecked before choosing the shape, per the card's request: it is not a second inline copy.
PackageOwdOverviewPanel.tsx:34importsisExternalWiderfrom./owd-sharing.jsand calls it at line 204. So the Settings tab was the only surface not calling the shared module, and no separate finding is needed.The pin (the substantive half)
ObjectSettingsPanel.owdAgreement.test.tsxdeliberately does not assert that the panel calls a function — that proves nothing about whether the comparison is the same one, since a re-inlined copy calls nothing and a call spy stays green when the arguments are swapped. Instead it asserts behaviour across the boundary the drift would cross, the two surfaces: both are driven over the full 5×5 cross-product of the values their dials offer, each verdict is read off what the surface renders (the Settings tab's amber D11 hint; the overview's per-rowowd-error-*node), and all three legs — surface A, surface B, andisExternalWideritself — must agree on every pair. The violating pairs are additionally pinned by name so the sweep cannot pass vacuously if every leg silently went false.Both halves were verified to actually fail, each mutation confirmed on disk by anchored grep before the run and restored by an
EXIT INT TERMtrap after (tree byte-identical afterwards,git status --porcelainempty both times). No rebuild was needed or claimed: the suite imports these modules by relative source path within the package, not across adist/exportsboundary.>=) intoObjectSettingsPanelObjectSettingsPanel=true PackageOwdOverviewPanel=false isExternalWider=false>→>=)The second is the one worth noting: after convergence there is only one implementation, so a change to it moves all three legs together. That is why the pin carries the named-violations anchor as well as the agreement assertion — the two catch different failure modes.
Gates
All run at
33c983a1a(the final commit; the tree was clean and unchanged from it, both ablations having restored byte-identically). Each verdict is the gate's own printed line, with exit codes captured before any pipe.vitest run packages/app-shell/src/views/studio-design/—Test Files 32 passed (32),Tests 187 passed (187)pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json) —TYPECHECK_EXIT=0, no diagnostics. Run afterpnpm --filter '@object-ui/app-shell^...' build; before the closure existed it reported TS2307 on every workspace sibling, which is the stale-closure trap and not a result.pnpm check:control-bytes—✅ check-control-bytes: OK (scanned 4630 tracked text file(s); skipped 85 binary)check-changeset-presence—✅ 2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major—✅ No changeset declares a 'major' bump.check-changeset-fixed—✅ All workspace packages are in the changeset fixed group.Declared narrowing on
pnpm lint. The repo-wide farm is CI's run. Locally eslint was run over the whole affected package,packages/app-shell: 909 files (population from eslint's own config resolution, count from--format json), 0 errors, and 0 errors / 0 warnings on both changed files (the 2508 package warnings are the pre-existingno-unused-vars/no-explicit-anybaseline, none on files this PR touches). The narrowing excludes nothing: the config usestseslint.configs.recommendedwith noproject/projectService, so linting is per-file and syntax-only and each file's verdict is a pure function of its own bytes plus the shared config. This diff changes two.tsxfiles, both inside app-shell, plus a.mdchangeset that the**/*.{ts,tsx}files glob does not select — so no file outside app-shell can change verdict.Scope
Exactly the convergence, per triage's "security-adjacent file, keep the diff to exactly this convergence": three files —
ObjectSettingsPanel.tsx, its new test, and the changeset.owd-sharing.tsis untouched (read-only reuse target; it needed no change). No drive-by tidying.Generated by Claude Code