Remove dead ApiKeyService.ValidateKeyAsync usage-recording path (#1409)

[Chris0Jeky]

What

Removes the dead ApiKeyService.ValidateKeyAsync usage-recording path and its supporting domain helper.

Removed

• ApiKeyService.ValidateKeyAsync(string, CancellationToken) (Application) — plus the #1409 dead-path / dual-writer warning comment on it.
• ApiKey.RecordUsage() (Domain) — its only production caller was ValidateKeyAsync; zero production callers remain after that removal.
• ApiKeyTests.RecordUsage_UpdatesLastUsedAt (Domain.Tests) — the only test exercising RecordUsage().

Why

ValidateKeyAsync had zero callers and was a dual-writer trap. MCP authentication is performed by ApiKeyMiddleware, which records usage directly via ExecuteUpdateAsync (#1402/#1406; owner IsActive folded into the initial ApiKeys lookup in #1404/PR #1412). If anyone had wired ValidateKeyAsync into that pipeline it would double-write LastUsedAt/UpdatedAt on every authenticated request. Recorded decision (#1409): remove it rather than wire it.

ApiKey.RecordUsage() followed it out: a grep after removing ValidateKeyAsync showed no remaining production callers (only the one domain unit test, which is removed here). The LastUsedAt property itself is untouched — the middleware still persists it via ExecuteUpdateAsync.

ApiKeyService implements no interface (concrete class registered in DI), so there was no interface member to remove. Surviving methods (CreateKeyAsync, ListKeysAsync, RevokeKeyAsync, GenerateKey, HashKey) are unchanged.

Compiler-proof

dotnet build backend/Taskdeck.sln -c Release -m:1 is clean (0 errors) after removing both members. A clean build with no unresolved references is proof that neither ValidateKeyAsync nor RecordUsage() had any remaining caller in production or test code.

Verification

• Build: dotnet build backend/Taskdeck.sln -c Release -m:1 — 0 errors (11 pre-existing unrelated warnings).
• Tests: dotnet test backend/Taskdeck.sln -c Release -m:1 --filter "FullyQualifiedName~ApiKey" — all green:
  • Domain.Tests: 10 passed (was 11; the removed RecordUsage test accounts for the delta).
  • Api.Tests (ApiKey middleware / MCP auth): 68 passed.
  • Cli.Tests (ApiKey command handler): 12 passed.
  • Total: 90 passed, 0 failed, 0 skipped.

Scope note

No cascade beyond this slice — no other members became unreferenced.

Closes #1409

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Chris0Jeky]

Adversarial Code Review

Reviewed the full diff (3 files, deletions only) against auth/authz, dual-writer, dead-code, layer-purity, and test-coverage lenses.

CRITICAL

• None.

HIGH

• None. Auth path is unaffected: MCP authentication runs entirely through ApiKeyMiddleware (ExecuteUpdateAsync for usage recording). ValidateKeyAsync had zero callers, so removing it cannot bypass or fail-open any auth check. The compiler (clean Release build, 0 errors) is the proof of zero remaining references.

MEDIUM

• None. ApiKey.RecordUsage() removal is safe: its only production caller was ValidateKeyAsync. The LastUsedAt property is retained and is not orphaned — it is still written by the middleware's direct ExecuteUpdateAsync and hydrated by EF materialization, and still read by consumers/tests. _unitOfWork remains used by CreateKeyAsync/RevokeKeyAsync. No cascade: no other member became unreferenced.

LOW

• None. No stale doc/autodoc references to the removed members (the similarly named ILlmQuotaService.RecordUsageAsync is a distinct method, untouched). Retained tests still cover surviving behavior, including Constructor_ValidInput_CreatesEntity asserting LastUsedAt is null on a fresh entity.

Bot Comments Addressed

• gemini-code-assist[bot]: sunset notice only (consumer Gemini Code Assist ceased). Not actionable.

Summary

0 CRITICAL / 0 HIGH / 0 MEDIUM / 0 LOW. Deletion-only dead-code removal with the compiler as the dead-code proof. Verification: build clean (0 errors); --filter FullyQualifiedName~ApiKey → Domain 10, Api 68, Cli 12 = 90 passed, 0 failed, 0 skipped. Not merge-blocking. No fixes required.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[Chris0Jeky]

Coordinator — second independent review lens (blast radius / non-static references): MERGE-SAFE, 0 findings

Lens 2 attempted to refute the removal on four axes; all failed to produce a finding:

1. Non-static references: whole-repo sweeps for ValidateKeyAsync and word-boundary RecordUsage across code, docs, autodoc/, scripts, frontend, and config found only the three removed sites (definition, single caller, one domain test). The distinct ILlmQuotaService.RecordUsageAsync (LLM token quota) is unrelated and untouched. No IApiKeyService interface exists, so no contract member dangles.
2. Behavioral contract: ApiKeyMiddleware.UpdateLastUsedAsync writes both LastUsedAt and UpdatedAt via ExecuteUpdateAsync — fully replicating what RecordUsage() did, on the live auth path. No auditing/telemetry regression.
3. Single-writer rationale: the middleware independently hashes, looks up, and stamps usage; the removed service path had zero callers (proven by grep, independently reproduced) — no call path is left without usage recording.
4. Domain coherence: LastUsedAt retained; no surviving test or invariant depends on an entity-level usage-recording method.

One precision note on the PR description (informational, not a defect): the clean Release build alone proves deadness for RecordUsage() and the removed test, but not for a public method on a non-interface class — ValidateKeyAsync's deadness is proven by the zero-caller grep, which lens 2 reproduced independently.

Gate status: both lenses MERGE-SAFE with zero findings → no fix round required. Merge follows CI green on 688abdfd + bot window + final feedback-by-content sweep.