fix(resolver): FamilyResolver — alias inside mutex + UPPER case-normalization#129
Merged
Conversation
…Resolver The shared FamilyResolver acquired the per-key mutex, ran find-or-create, released, then ran resolveAlias() OUTSIDE the lock. Two parallel resolve() calls on the same family name could split if an alias was installed between the create and the alias-lookup — one returned the alias target, the other the pre-alias id. This is the same race PR #127 fixed for PersonResolver and CompanyResolver in commit 8fe6fd0; applying the same fix here so SponsorFamilyResolver and UnderwriterFamilyResolver benefit. Adds regression tests in both family-resolver test files that wrap aliasRepo.resolve with an in-flight counter and assert maxInFlight === 1 under two parallel resolves on the same key (pre-fix code drives it to 2).
…a FamilyResolver.test.ts The shared normalizeFamilyName introduced in commit 960d707 silently flipped the family-name fold from UPPER (the pre-refactor SponsorFamilyResolver convention) to lower. CanonicalSponsorFamilyRepo.findByResolverAndName runs an exact-match storage query with no case fold, so any existing UPPER canonical_sponsor_family.normalized_name rows became unreachable, and operator-installed aliases keyed on those canonical ids would silently orphan. Reverts the fold to UPPER and pins it via a new dedicated FamilyResolver.test.ts that covers UPPER output, case-insensitivity, internal-whitespace collapse, and empty/whitespace-only input. Also updates three test fixtures (CanonicalSponsorFamilyRepo, CanonicalUnderwriterFamilyRepo, underwriterFamily command tests) that hardcoded lowercase normalized_name literals so they match the live convention.
3 tasks
PR #129 pinned FamilyResolver normalization to UPPER but two callers were missed: - Form_S_1.storage.underwriters.test.ts: findByResolverAndName lookup still used "goldman sachs", returning undefined and crashing the next line's family! dereference. Updated to "GOLDMAN SACHS" to match the three other test files already flipped in PR #129. - SponsorFamilyResolver.normalizeSponsorFamilyName JSDoc still described lower-casing; the function delegates to normalizeFamilyName which now UPPER-cases. Updated the doc to reflect actual behavior and the canonical_sponsor_family.normalized_name column it matches against. UnderwriterFamilyResolver was checked and has no equivalent stale doc. Co-authored-by: Claude <noreply@anthropic.com>
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
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.
Summary
Two HIGH-priority fixes for the shared
FamilyResolverpoweringSponsorFamilyResolverandUnderwriterFamilyResolver.Alias lookup serialised inside the per-key mutex. Pre-fix code acquired the per-key mutex for find-or-create, released it, then ran
resolveAlias()OUTSIDE the lock. Two parallelresolve()calls on the same family name could split if an alias was installed between the create and the alias lookup — one returned the alias target, the other the pre-alias id. This is the same race PR fix: DRSLTR dispatch, SPAC sponsor span verification, resolver test + RFC-9112 Content-Length #127 fixed forPersonResolver/CompanyResolverin commit8fe6fd0; applying the same fix here so the family resolvers benefit. New regression tests in both family-resolver test files wrapaliasRepo.resolvewith an in-flight counter and assertmaxInFlight === 1under two parallel resolves on the same key (pre-fix code drives it to 2).normalizeFamilyNamereverted to UPPER. The shared helper introduced in commit960d707silently flipped the family-name fold from UPPER (the pre-refactorSponsorFamilyResolverconvention) to lower.CanonicalSponsorFamilyRepo.findByResolverAndNameruns an exact-match storagequerywith no case fold, so any existing UPPERcanonical_sponsor_family.normalized_namerows became unreachable, and operator-installed aliases keyed on those canonical ids would silently orphan. Reverts to UPPER and pins it via a new dedicatedFamilyResolver.test.tscovering UPPER output, case-insensitivity, internal-whitespace collapse, and empty/whitespace-only input. Three test fixtures (CanonicalSponsorFamilyRepo,CanonicalUnderwriterFamilyRepo,underwriterFamilycommand tests) that hardcoded lowercasenormalized_nameliterals are updated to match.Test plan
bun test src/resolver/FamilyResolver.test.ts— new file pinning the UPPER case convention.bun test src/resolver/SponsorFamilyResolver.test.ts— adds the alias-in-mutex regression test.bun test src/resolver/UnderwriterFamilyResolver.test.ts— adds the alias-in-mutex regression test.bun test src/resolver/PersonResolver.test.ts— sibling resolver, unchanged but exercised to confirm no regression.bun test src/resolver/CompanyResolver.test.ts— sibling resolver, unchanged but exercised to confirm no regression.bun test src/storage/canonical/CanonicalSponsorFamilyRepo.test.ts— fixture flipped to UPPER.bun test src/storage/canonical/CanonicalUnderwriterFamilyRepo.test.ts— fixture flipped to UPPER.bun test src/commands/sponsorFamily.test.ts— usesnormalizeSponsorFamilyName()dynamically; passes after the case flip.bun test src/commands/underwriterFamily.test.ts— three lowercasenormalized_nameliterals flipped to UPPER.bun test src/resolver/ src/storage/canonical/final sanity sweep: 116 tests pass, 0 fail.bun run build-types— TypeScript strict type-check clean.Generated by Claude Code