fix(matching): return typed ownership errors consistently with SD

[dkrotx]

fix(matching): return typed ownership errors consistently with SD

What changed?
Standardized task list ownership-guard behavior in matching so SD-onboarded and non-onboarded paths return consistent typed ownership errors where applicable.
Also added unit-test for errIfShardOwnershipLost.
This is part of production-readiness for SD (#6862)

Why?
Previously, ownership checks could return different error shapes depending on whether shard distributor onboarding was enabled.
In SD mode, one branch returned an untyped wrapped error while non-SD mode returned TaskListNotOwnedByHostError.
This made error handling inconsistent and could cause ownership-related metrics/logs to be missed.
This change aligns behavior and adds dedicated tests to protect expected ownership semantics with and without shard-distributor.

How did you test it?

• go test -v ./service/matching/handler TestErrIfShardOwnershipLost
• go test -v ./service/matching/handler/...

Potential risks

• Matching ownership guard behavior is sensitive and sits in task-list manager acquisition flow.
• Error typing/shape changes can affect downstream error classification or alerting expectations.
  • at the same time, the behaviour is now aligned with the previous
    (ringpop) case

Release notes
N/A (internal correctness/consistency fix).

Documentation Changes
N/A.

---

Reviewer Validation

PR Description Quality (check these before reviewing code):

• "What changed" provides a clear 1-2 line summary
  • Project Issue is linked
• "Why" explains the full motivation with sufficient context
• Testing is documented:
  • Unit test commands are included (with exact go test invocation)
  • Integration test setup/commands included (if integration tests were run)
  • Canary testing details included (if canary was mentioned)
• Potential risks section is thoughtfully filled out (or legitimately N/A)
• Release notes included if this completes a user-facing feature
• Documentation needs are addressed (or noted if uncertain)

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Clean refactor that standardizes ownership error types across SD and ringpop paths. Previous finding about missing SD happy path test has been resolved with the new test case at line 464. No new issues found.

✅ 2 resolved

✅ Quality: Unused currentHostIdentity() method added

📄 service/matching/handler/engine.go:1535
The new currentHostIdentity() method (lines 1535-1542) is not called anywhere in the codebase. While it appears to be future-proofing, adding dead code without an immediate consumer increases maintenance surface. Consider either adding it in the PR where it's actually needed, or adding a comment explaining its intended future use to prevent it from being flagged/removed by linters or future contributors.

✅ Quality: Missing test case for SD happy path (non-nil shard process)

📄 service/matching/handler/engine_test.go:407
The new TestErrIfShardOwnershipLost covers 6 scenarios but is missing the SD happy path: when IsOnboardedToSD() returns true, GetShardProcess returns a non-nil shard process with no error, and the function should return nil. This would exercise line 1511 (return nil) in engine.go.

Without this test case, you don't verify that the SD-onboarded path correctly returns no error when the shard is still owned. While the logic is straightforward, adding this case would complete the branch coverage matrix.

Rules ✅ All requirements met

Repository Rules

✅ PR Description Quality Standards: PR description comprehensively addresses all required sections including intent, implementation details, test coverage, and risk assessment

_{1 rule not applicable. Show all rules by commenting gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}