[brinehart]

Closes #50

Summary

GetManyClients interpolated the entire ManyClientsRequest message into the
ILike pattern ($"%{request}%"), so every search compared client names against
the protobuf debug string of the request and returned nothing. The filter now uses
request.SearchTerm.

Intentional deviation from the spec's one-liner

The spec (.kiro/specs/must-fix-50/spec.md) prescribed a one-line change keeping
EF.Functions.ILike and testing GetManyClients under the EF Core InMemory
provider. That test path is impossible: EF.Functions.ILike is the Npgsql-only
NpgsqlDbFunctionsExtensions.ILike. I verified it throws under both the InMemory
provider (ILike ... is not supported, query switched to client-evaluation) and the
Sqlite provider (Translation of NpgsqlDbFunctionsExtensions.ILike failed). Only a
real PostgreSQL provider translates it, and AGENTS.md states the test suite is
self-contained and needs no PostgreSQL.

Per the orchestrator's decision (Option B, scoped), the production filter was changed
from the Npgsql-only EF.Functions.ILike to a provider-agnostic, case-insensitive
predicate:

var searchTerm = request.SearchTerm.ToLower();
predicate.And(c => c.Name.ToLower().Contains(searchTerm));

This is the faithful, testable equivalent of ILike: Npgsql translates
ToLower()/Contains() to SQL LOWER()/strpos, so behavior on PostgreSQL is
unchanged, while the InMemory provider the test suite uses can evaluate it in memory.
Reviewer: expect the predicate form to differ from the spec's one-liner — this is the
deliberate reason.

Empty / null search term

Guarded with !string.IsNullOrEmpty(request.SearchTerm) so ToLower() is never called
on null, and a null or empty term skips the filter entirely → all clients of the
requested type are returned. Matches the spec's stated behavior.

Tests (EF Core InMemory, per spec)

Added ClientsGrpcServiceGetManyClientsTests:

• matching term returns the right clients,
• case-insensitive match,
• non-matching term returns none,
• empty term returns all,
• null term returns all.

A small TestServerCallContext provides a no-op ServerCallContext. The test csproj
adds a build target aliasing the transitive MicroZen.OAuth2 assembly, because it
compiles the same client.proto as MicroZen.Api and would otherwise make the
MicroZen.Grpc.V1.* types ambiguous (CS0433) in test code.

Scope

Search-term filter only. Paging is untouched (that is #51 on a separate branch).
Conflicts at epic-merge time are expected; the orchestrator rebases.

CI gate (run locally from the worktree)

• npx nx affected -t lint test build → green (lint clean, 16/16 tests pass, build ok)
• dotnet test core/csharp/MicroZen.Core.sln → 16/16 pass

Pre-existing format debt (not introduced here): npx nx format:check fails on the
epic's own tracked spec markdown files (.kiro/specs/must-fix-{45,47,50,51,79}/spec.md)
that were committed unformatted on the base branch. I verified this failure exists on
epic/must-have-fixes before this change. I deliberately did not reformat the sibling
specs here to avoid pulling other issues' files into #50's PR and colliding with their
branches at merge time. This affects every PR into the epic equally and is an epic-wide
cleanup for the orchestrator to decide.

[nx-cloud]

View your CI Pipeline Execution ↗ for commit 3cc6a03

Command	Status	Duration	Result
nx affected -t lint test build	✅ Succeeded	39s	View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗

---

☁️ Nx Cloud last updated this comment at 2026-11-02 03:52:08 UTC

[brinehart]

Review — Must Fix #50 (client search filters on search term)

Reviewed against .kiro/specs/must-fix-50/spec.md, judging on the orchestrator-approved provider-agnostic predicate (not literal EF.Functions.ILike).

Correctness — all checks pass:

• ✅ Filter uses the search term, not the whole request. $"%{request}%" (which stringified the protobuf debug message) is replaced by c.Name.ToLower().Contains(request.SearchTerm.ToLower()). Root-cause fix for #50.
• ✅ Case-insensitive + match-all on null/empty. Guard changed to !string.IsNullOrEmpty(request.SearchTerm); null and empty skip the predicate → all clients of the type. Faithful (and slightly safer) realization of the spec's stated behavior — also avoids .ToLower() on null.
• ✅ Provider-agnostic translation. ToLower()/Contains() translates to LOWER()/strpos on Npgsql and evaluates under the EF Core InMemory provider — the point of the approved deviation. PostgreSQL behavior unchanged vs ILike.
• ✅ Test coverage. Five xUnit tests: matching term, case-insensitivity, non-matching term, empty term (all), null term (all). Matches the spec test plan and adds the empty-string case.
• ✅ Scope held to search. Production diff touches only the search-filter if-block; paging (page/skip/NextUrl, #51) is untouched.
• ✅ CI green. Both dotnet and main checks pass.

Non-blocking notes (no action required for #50):

• The TestServerCallContext helper and the AliasOAuth2GrpcTypes MSBuild target (resolving the CS0433 ambiguity between the two assemblies that compile client.proto) are reasonable, well-commented test-only scaffolding.
• The format:check debt on sibling spec markdown is pre-existing on the base branch and correctly left as an epic-wide cleanup.

The fix satisfies the spec intent and checks are green.

[brinehart]

ALL-CLEAR — #50 fully satisfies the spec intent (search by SearchTerm, case-insensitive, null/empty = match all, paging untouched), tests cover match/case-insensitive/non-match/empty/null, and CI is green (dotnet + main).