fix(milpacs): route errors to NotFound vs Internal

[SyniRon]

Summary

Fixes the long-standing MilpacsService error-routing bug: every datastore error mapped to codes.NotFound (HTTP 404). DB outages, query timeouts, and genuine missing-record lookups were indistinguishable to consumers.

Two distinct bug shapes get fixed in one bundled PR:

• Pattern A — single-row finders (5 datastore funcs, 4 RPCs, 5 call sites). Datastore caught gorm.ErrRecordNotFound and wrapped with fmt.Errorf without %w, destroying the sentinel. Now the datastore returns result.Error directly; handler adds errors.Is(err, gorm.ErrRecordNotFound) → codes.NotFound, everything else → codes.Internal. Mirrors TicketsService.
• Pattern B — roster finders (3 datastore funcs, 3 RPCs). Datastore used Find() without capturing result.Error, so DB outages produced empty slices with no error — handlers returned HTTP 200 {Profiles: {}}. Handler's if err != nil → 404 branch was dead code. Now the datastore captures result.Error and wraps with %w; handler maps any error to codes.Internal.

All &proto.X{} error returns flipped to nil (gRPC ignores the message on error; matches the tickets convention).

13 new unit tests via a shared fakeDatastore lifted out of tickets_test.go into fake_datastore_test.go.

No proto / OpenAPI / interface changes. No release cut.

Files

• datastores/mysql.go — Pattern A unwrap on 5 single-row finders; Pattern B result.Error capture on 3 roster finders
• servers/grpc/grpc.go — handler updates on 7 RPCs (GetProfile username + user_id, GetUserViaKeycloakId, GetUserViaDiscordId, GetGamertagProfile, GetRoster, GetLiteRoster, GetS1UniformsRoster)
• servers/grpc/fake_datastore_test.go — new shared test stub
• servers/grpc/milpacs_test.go — new, 13 unit tests
• servers/grpc/tickets_test.go — fakeDatastore block removed (moved to the new shared file)

Already-correct RPCs untouched

SearchByPosition, GetAllRanks, GetPositionGroups, GetAwol — all four already route errors correctly via the Internal pattern. Listed for reviewer context, no code change.

Out of scope (tracked as followup chore in the vault, blocked by this PR)

• errors.New("cannot request null roster type") on 3 roster handlers (should be codes.InvalidArgument).
• Empty Keycloak/Discord ID Warn.Println(...) warn-but-continue (should early-return InvalidArgument).
• Three fmt.Errorf("error generating profile") outliers without %w (FindProfileByKeycloakID, FindProfileByDiscordID, S1-uniforms inline loop).

Test plan

• go test ./... — 13 new milpacs tests + existing suite green
• go vet ./... && go build ./... — clean
• Local-stack smoke: known user Jarvis.A → 200, bogus user → 404, combat roster → 200, post-docker stop/docker start cycle recovery → 200
• Insomnia 61/61 — verified manually
• Reviewer to confirm

Coverage gap acknowledged

The Pattern-B unit tests stub the datastore method and verify the handler's mapping; they do not directly verify the datastore-side result.Error capture. The planned end-to-end docker stop xenforo-db → expect 500 smoke can't isolate that path because the auth middleware (ValidateApiKey) also queries xenforo-db, so requests 401 before reaching the milpacs handlers. The datastore-side capture is verified by code inspection only. Reviewing the diff is the path of record.

🤖 Generated with Claude Code