Repository navigation
fix(plugin-detail): declare code optional in isConcurrentUpdateError's narrowed type - #6474
Merged
os-support-ai merged 1 commit intoAug 26, 2026
Conversation
…r's narrowed type The exported predicate matches on two limbs — `code === 'CONCURRENT_UPDATE'` (the wire shape) or `name === 'ConcurrentUpdateError'` (the deliberate cross-realm discriminator for a host that bundles the adapter twice, so `instanceof` fails). Its narrowed type declared `code: 'CONCURRENT_UPDATE'` as a required literal, so for exactly the case the second limb exists to serve it handed the caller a property the value does not have: `err.code === 'CONCURRENT_UPDATE'` type-checked and was `undefined` at runtime, with the compiler agreeing it could not happen. `code` is now optional, stating only what both limbs guarantee. The two-limb runtime check is byte-identical — it is deliberate and documented, and the diff carries nothing but the return type and the comment above it. No runtime symptom existed to fix: the sole consumer (`InlineEditSaveBar`'s conflict builder) reads only the optional `currentVersion` / `currentRecord` fields. Pinned by `ConcurrentUpdateDialog.narrowedCode-6421.test.tsx`, which states the invariant as an assignability relation — the value the runtime accepts through the `name` limb must be assignable to the type the predicate returns — over a named witness interface rather than an object literal, so excess-property checking cannot make the pin pass for the wrong reason. It also adds the runtime coverage of both limbs that this package had none of (the sibling copies in `data-objectstack` and `plugin-form` each have theirs). 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
|
os-support-ai
marked this pull request as ready for review
August 26, 2026 02:51
os-support-ai
deleted the
claude/issue-6421-concurrent-update-code-optional
branch
August 26, 2026 03:04
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 #6421
isConcurrentUpdateErrorno longer promises acodeits runtime check knowingly accepts values without.What moved
The exported predicate in
packages/plugin-detail/src/ConcurrentUpdateDialog.tsxmatches on two limbs —code === 'CONCURRENT_UPDATE'(the wire shape) orname === 'ConcurrentUpdateError'(the deliberate cross-realm discriminator, for a host that bundles the adapter twice soinstanceoffails). Its narrowed type declaredcode: 'CONCURRENT_UPDATE'as a required literal, so for exactly the case the second limb exists to serve it handed the caller a property the value does not have.One character in the return type, plus the comment that now explains why:
export function isConcurrentUpdateError(err: unknown): err is { - code: 'CONCURRENT_UPDATE'; + code?: 'CONCURRENT_UPDATE';The two-limb runtime check is byte-identical. Per the triage ruling, only the narrowed type moves. The three body lines do not appear in the diff at all, and the other two copies of this predicate (
data-objectstacknarrows to the class;plugin-form'soccSavereturns bareboolean) are untouched.Premise re-derived on
origin/main— one contradiction to recordVerified rather than taken on faith, each with a positive control:
codeConcurrentUpdateDialog.tsx:224onorigin/main:code: 'CONCURRENT_UPDATE';namelimb admits objects lackingcodereturn e.code === 'CONCURRENT_UPDATE' || e.name === 'ConcurrentUpdateError';InlineEditSaveBar.tsx:183; itsbuildConflict(draft, err)takeserr: anyand reads onlycurrentVersion/currentRecordContradiction: the claim comment's file face named
packages/plugin-detail/src/index.tsxas "the predicate and its narrowed type". It is not there.index.tsx:74-76is a re-export block; the definition lives inConcurrentUpdateDialog.tsx:223, which is where the issue body itself points. This PR does not touchindex.tsx— the export list is unchanged.Public surface (clause ②) — stated, not inferred
packages/plugin-detail/src/index.tsxis not in the diff. The builtdist/index.d.ts:49still readsexport { ConcurrentUpdateDialog, isConcurrentUpdateError, } from './ConcurrentUpdateDialog.js';.dist/ConcurrentUpdateDialog.d.ts:53-58now carriescode?: 'CONCURRENT_UPDATE';— the annotation reaches the published surface verbatim.unknown, so the set of values the predicate takes is identical. What shrinks is what it promises. Calling this a widening would be backwards.code.err.codeis now'CONCURRENT_UPDATE' | undefined, soconst c: string = err.codecompiled before and will not now. Could a consumer have relied on the stronger promise? Yes — and that reliance was already wrong. On a name-only error the value isundefinedat runtime; the old declaration was precisely the reason the compiler could not say so. The change converts a silentundefinedinto a compile error. No such consumer exists in this repo (onlyInlineEditSaveBar, viaerr: any).minorwith the breaking semantics spelled out in the changeset, per AGENTS.md's no-majorrule for the fixed group.The pin, and why it is not blind
packages/plugin-detail/src/__tests__/ConcurrentUpdateDialog.narrowedCode-6421.test.tsx. Every assertion is labelled DISAGREES (red before, green after) or CONTROL (passes both ways, on purpose, naming the future wrong shape it guards).nameset, nocode) rather than an object literal. That is load-bearing: excess-property checking fires only on fresh literals, so a literal would have gone red in both directions — onname, which is on neither version of the type — and measured nothing aboutcode.message: stringon the witness is also load-bearing. The new narrowed type is all-optional, i.e. a weak type, and TypeScript rejects a source sharing no property name with a weak type;messageis what keeps the pin a measurement ofcoderather than an accident of weak-type detection.as anyin the file, and the one cast present (nameOnly as { code?: unknown }) is a runtime read asserting the witness really is code-less._PredicateStillNarrowsfails if the predicate stops being an exported type predicate (Narrowedwould collapse tonever, which is assignable to everything and would satisfy every other pin silently);_CodeIsStillTheWireLiteralfails if someone "fixes" this by widening tocode?: string; athrowinside the narrowing test fires if thenamelimb ever stops accepting a code-less error.dist/.tsc -p tsconfig.test.json --listFilesputspackages/plugin-detail/src/ConcurrentUpdateDialog.tsx(source, line 1424 of 1686) and the pin file (line 1465) in the program, with noplugin-detail/distentry at all. The dependency closure was built first, and dependencies do resolve through their built.d.ts(packages/components/dist/...is in the same listing).Reverse verification
Direction predicted before the run: putting the required
codeback turnstsc -p tsconfig.test.jsonred at exactly the three DISAGREES pins, whiletsc --noEmit(which excludes tests) and vitest both stay green.Mutation proven on disk by grepping both the injected and the deleted text — never a diffstat, never an editor's exit code:
injected_required_form_count=1,deleted_optional_form_count=0.Lines 114 / 172 / 179 are
_CodeReadsAsMaybeUndefined,_CodeIsOptional,_NameOnlyErrorIsAssignableToTheNarrowedType— the three DISAGREES pins, and only those. The controls at 150 / 156 / 163 stayed green, which is what makes the red a measurement ofcode's optionality rather than of the file failing to compile.MUT_VITEST_EXIT=0is the other half of the result and worth stating plainly: the whole runtime suite passes against the defect. A runtime-only pin could not have caught this, which is why the instrument here is a type pin.Restore proven by state, not by exit code: worktree blob
7c30e8c69c1feb3d0d07a695b2a6ed86b587155cequals theHEADblob,git diff HEADempty,git statusclean. AnEXIT/INT/TERMtrap held absolute paths resolved fromgit rev-parse --show-toplevel, and the restore named the ref —git checkout HEAD -- ABSPATH— rather than the baregit checkout -- ABSPATH, which reads the index and would have handed the mutation straight back.Gates — verdict lines, exit codes captured before any pipe
All run on the final commit,
2bd48ad6d:pnpm --filter @object-ui/plugin-detail type-checkFINAL_TYPECHECK_EXIT=0, echoing> tsc --noEmit && tsc -p tsconfig.test.json(not a zero-match no-op)pnpm exec vitest runover the affected filesTest Files 5 passed (5)/Tests 76 passed (76)pnpm --filter '@object-ui/plugin-detail^...' buildos-verify-lock: VERDICT command-exit 0pnpm --filter @object-ui/plugin-detail buildBUILD_DETAIL_EXIT=0node 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.mjs0grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over both edited files: no matchTest scope is the measured affected set, not a guess: the change is a type annotation plus a comment, so the only tests that can observe it are the three that import the module (
ConcurrentUpdateDialog.narrowedCode-6421,InlineEditSaveBar,InlineEditSaveBar.savingNoProviderFallback) and the two that read its source text as a path string (i18n/residue-namespaces-3546,i18n/fallback-placeholder-spelling-3512). A full-package run was attempted first and was killed by the container's ~10-minute foreground cap.Lint, narrowed and measured (counts from eslint's own
--format json):react-refresh/only-export-componentson theexport function isConcurrentUpdateErrorline, pre-existing: the rule reads the file's export list, which this diff does not change (no export added, moved or removed; the line number shifted only because the doc comment grew).errorCount0, eslint exits 0).eslint.config.jsextendstseslint.configs.recommended, notrecommendedTypeChecked, and noparserOptions.project/projectServiceappears anywhere in it, so every file is linted from its own AST alone. A type annotation in one file cannot change any verdict in a file whose bytes did not change. The third changed file is a.mdchangeset, which is not in eslint's population at all (files: ['**/*.{ts,tsx}']).The repo-wide farm (
turbo run lint, the fullcheck:*set) is CI's run and is left to it.Found from #6375.
Generated by Claude Code