transit PR #240 author @ArjenSchwarz head → base T-1862/bugfix-display-id-queries-hide-storage-lookup-failures → main commits 1 files 6 touched lines +306 / -22 unresolved 1 CR-level comment CI 1 check passed

PR overview: #240 — Fix T-1862: Preserve display-ID lookup failures

PR #240 by @ArjenSchwarz · merging T-1862/bugfix-display-id-queries-hide-storage-lookup-failuresmain · view on GitHub

At a glance

  • Single-record task and milestone queries no longer report a storage read failure as a successful empty result.

  • The implementation preserves established duplicate-ID wording and uses existing injected-fetch seams for deterministic regressions.

  • Review feedback has no blocking issue for this PR’s declared query scope, and the sole CI check has passed.

Verdict

Ready

The scoped display-ID query fix is sound: only typed absence returns [], while duplicate identifiers and unexpected storage failures remain distinguishable. GitHub has no unresolved diff threads or non-approved review bodies. One CR-level comment identifies the same error-mapping pattern in out-of-scope mutation resolvers and explicitly suggests a follow-up rather than blocking this narrow query-path fix. The sole claude-review check completed successfully. No merge was attempted.

Author's PR description

Shown verbatim — the markdown the author wrote, unmodified.

@ArjenSchwarz · 2026-08-05
## Summary
- preserve non-not-found storage failures from single-record task and milestone display-ID lookups
- keep valid missing-record arrays and duplicate-ID contracts unchanged
- add exact injected-seam regressions across JSON App Intents and MCP `query_milestones`

## Validation
- `make lint`
- `make test-quick`
- `make build-ios`

Bugfix report: `specs/bugfixes/display-id-queries-hide-storage-lookup-failures/report.md`

Commits

Three-level explanation

What changed

A missing record and an unreadable database previously produced the same empty answer. The fix keeps an empty result only for a confirmed missing record and reports a problem when the read fails.

Why it matters

Automations can now distinguish missing data from an unavailable store, avoiding unsafe recovery actions.

Key concepts

A typed not-found error is an expected business result; a fetch failure is an infrastructure error and should not masquerade as success.

Changes overview

QueryTasksIntent, QueryMilestonesIntent, and MCP milestone lookup now catch typed not-found cases before generic errors. The generic path returns source-appropriate INTERNAL_ERROR or tool errors.

Implementation

The milestone App Intent isolates its direct lookup mapping in a helper. Serialized Swift Testing injects a failing ModelFetching implementation and asserts exact no-match, duplicate, and storage-failure contracts.

Trade-off

The patch reuses existing service error types and test seams, preserving duplicate contracts without expanding the persistence API.

Technical deep dive

findByDisplayID already exposes three outcomes: typed absence, duplicate-ID corruption, and a propagated fetch failure. The adapters collapsed the third into the first. Revised catch ordering restores a total source-specific mapping without altering SwiftData semantics.

Architecture impact

The change reinforces the adapter convention of classifying domain errors before preserving unknown infrastructure failures.

Potential issue

The current CR-level comment accurately identifies analogous catch-all behavior in mutation resolvers; it is outside this query-only PR scope but should be tracked as a separate follow-up.

Important changes — detailed

QueryTasksIntent: classify direct task lookup failures

Transit/Transit/Intents/QueryTasksIntent.swift

Why it matters. Correctness: prevents a storage failure from impersonating a valid no-match task query.

What to look at. displayId lookup catch ordering

Takeaway. Reserve a successful empty collection for verified domain absence, not a generic thrown error.
Rationale. TaskService already distinguishes taskNotFound, duplicateDisplayID, and propagated fetch failures.

QueryMilestonesIntent: isolate direct milestone lookup mapping

Transit/Transit/Intents/QueryMilestonesIntent.swift

Why it matters. Correctness and maintainability: makes each milestone direct-lookup outcome explicit in one helper.

What to look at. lookupMilestoneByDisplayID(_:json:milestoneService:resolvedProjectId:)

Takeaway. A narrow transport helper makes error contracts auditable without changing a service API.
Rationale. The report requires preserving malformed-ID and duplicate behavior while surfacing unexpected fetch errors.

MCP query_milestones: surface unexpected store errors

Transit/Transit/MCP/MCPToolHandler.swift

Why it matters. API correctness: MCP callers receive a tool error instead of an incorrect successful empty array.

What to look at. lookupMilestoneByDisplayId(_:args:projectFilter:)

Takeaway. Keep source-specific error shapes while using the same domain-outcome classification.
Rationale. Duplicate identifiers already return MCP errors; unexpected lookup failures now remain distinct too.

Regression suite: inject deterministic display-ID fetch failures

Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift

Why it matters. Regression protection: tests all three outcomes across App Intents and the affected MCP tool.

What to look at. DisplayIDLookupStorageFailureTests

Takeaway. Use the existing fetch abstraction to deterministically exercise persistence failures without corrupting a real store.
Rationale. Existing ModelFetching and MCP test-environment seams cover the paths without a new production hook.

Key decisions

Make typed not-found the only route to a successful empty result

The adapter catches taskNotFound or milestoneNotFound before the generic branch. Duplicate IDs retain established corruption errors; unexpected fetch failures remain errors.

Reuse existing service and test seams

No service API or production persistence hook is added. The implementation relies on existing propagated fetcher errors and the injected ModelFetching test seam.

Unresolved comments

Open review threads and PR-level comments still awaiting a response.

discussion @claude · 2026-08-05 · view on GitHub
## Review

### Overview
Fixes T-1862: the single-record `displayId` lookup paths in `QueryTasksIntent`, `QueryMilestonesIntent`, and MCP `query_milestones` were collapsing *any* non-duplicate error (including genuine storage/fetch failures) into a successful empty-array `[]` response, making a transient storage failure indistinguishable from a verified "not found." The fix inserts an explicit `catch .taskNotFound` / `catch .milestoneNotFound` branch before the generic catch, so only a real typed not-found collapses to `[]`; everything else now surfaces as `INTERNAL_ERROR` (App Intents) or a tool error (MCP).

### Correctness
- Verified `TaskService.findByDisplayID` and `MilestoneService.findByDisplayID` (`Transit/Transit/Services/TaskService.swift:317`, `Transit/Transit/Services/MilestoneService.swift:260`) throw the typed not-found case before `duplicateDisplayID`, so the new catch ordering is sound and exhaustive for all three outcomes (not-found / duplicate / anything else).
- The error message format (`"Failed to look up task/milestone: \(error)"`) matches existing conventions used elsewhere in `IntentHelpers.swift` (lines 170, 261, 283, 326, 485) and in MCP's own `resolveTaskArgument`/duplicate-branch comments — good consistency.
- `QueryMilestonesIntent`'s extraction of `lookupMilestoneByDisplayID` into a helper is a clean, low-risk refactor that also makes the fix testable in isolation.

### Test coverage
`DisplayIDLookupStorageFailureTests.swift` is thorough: it exercises not-found, duplicate, and injected `ModelFetching` storage-failure outcomes across `QueryTasksIntent`, `QueryMilestonesIntent`, and the MCP `query_milestones` tool, using the existing `ModelFetching`/`MCPTestHelpers.makeEnv(milestoneServiceFetcher:)` seams rather than a new production hook. Assertions on exact error payloads (not just "is an error") are a good touch — they'll catch accidental message drift.

### Potential gap — same bug class left open in sibling code paths
While verifying the fix, I found two other MCP call sites in `MCPToolHandler.swift` with the **identical** collapsing-error pattern this PR fixes, that weren't touched:

- `resolveMilestone(from:)` (`MCPToolHandler.swift:1403`) — used by `update_milestone` and `delete_milestone` (lines 918, 1065). Its generic `catch` maps *any* non-duplicate error, including a storage failure, to `"No milestone with displayId \(displayId)"` (line 1412) — a false not-found.
- `resolveTaskArgument(_:)` (`MCPToolHandler.swift:1280`) — used by three task-mutation handlers (lines 494, 1113, 1215). Its generic `catch` maps *any* non-duplicate/non-invalid-identifier error to `"Provide either displayId (integer) or taskId (UUID string)"` — a real infrastructure failure reported as a user-input problem.

Both are arguably worse than the query-path bug this PR fixes, since callers of `update_milestone`/`delete_milestone`/task-mutation tools could take corrective action (e.g., re-creating a "missing" record) based on a false not-found triggered by a transient storage error. Since the PR title and CHANGELOG entry are scoped specifically to the query paths, this may be intentionally out of scope (worth a follow-up ticket) rather than a blocker — but given it's the same bug class in the same file, it seemed worth flagging here rather than in a separate pass.

### Minor
- `CHANGELOG.md` and the bugfix report are clear and accurately describe the change; no issues there.
- No security or performance concerns — this is purely error-classification logic on an already-fetched single record.

Overall: solid, well-tested, minimal-diff fix for the stated scope. Only substantive suggestion is considering a follow-up for `resolveMilestone`/`resolveTaskArgument`.

Per-file diffs

Click to expand.

CHANGELOG.md Modified +1 / -0
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex e95cb60..00cb77e 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ## [Unreleased] +- T-1862: Single-record `displayId` queries now reserve successful `[]` results for explicit task/milestone not-found outcomes. `QueryTasksIntent` and `QueryMilestonesIntent` surface other direct-lookup failures as source-specific `INTERNAL_ERROR` payloads, while MCP `query_milestones` returns the matching tool error; duplicate-ID error contracts remain unchanged. Deterministic injected-fetch regressions assert exact not-found, duplicate, and storage-failure behavior across the affected JSON and MCP paths. - T-1824: MCP `query_milestones` and `QueryMilestonesIntent` now resolve every well-formed `projectId` through `ProjectService` before filtering. A missing project returns the established project-not-found error and an unreadable project store retains its exact storage error, for both full-list and `displayId` queries; malformed UUID validation and `projectId`-over-name precedence are unchanged. Cross-surface regressions cover those contracts. - T-1821: MCP Settings now reconciles a committed port change—including focus loss or Settings closure—through the existing serialized listener lifecycle. A debounced coordinator keeps only the latest enabled change, cancels pending work when MCP is disabled, and flushes pending work on view teardown; invalid ports retain the lifecycle's existing stop/error behavior. The setup command now follows the server's active listener port rather than a persisted draft, and focused state plus live loopback regressions cover coalescing, disabled-server, presentation, and focus-loss replacement behavior. - T-2018: Task editing requires the selected project UUID to resolve to a live project model before Save. When a project disappears while the editor is open, Save remains disabled and both iOS and macOS show an inline, VoiceOver-labelled recovery message explaining that the user must choose an available project to re-enable Save. The task-edit applier still fails closed before any mutation when a changed project has no resolved model, while unchanged project fields still accept nil and project-move milestone clearing/validation remain intact.
Transit/Transit/Intents/QueryMilestonesIntent.swift Modified +34 / -17
diff --git a/Transit/Transit/Intents/QueryMilestonesIntent.swift b/Transit/Transit/Intents/QueryMilestonesIntent.swiftindex 4693244..e94baf2 100644--- a/Transit/Transit/Intents/QueryMilestonesIntent.swift+++ b/Transit/Transit/Intents/QueryMilestonesIntent.swift@@ -79,23 +79,12 @@ struct QueryMilestonesIntent: AppIntent {         // Single-milestone lookup by displayId. Remaining filters still apply conjunctively —         // a milestone that does not satisfy them is filtered out, mirroring QueryTasksIntent [T-963].         if json["displayId"] != nil {-            // Route through IntentHelpers.parseIntValue so JSON booleans (delivered as-            // NSNumber wrapping CFBoolean) are rejected rather than silently coerced to-            // 1/0 and targeting M-1/M-0. A plain `as? Int` would accept them. [T-1280]-            guard let displayId = IntentHelpers.parseIntValue(json["displayId"]) else {-                return IntentError.invalidInput(hint: "displayId must be an integer").json-            }--            let milestone: Milestone-            do {-                milestone = try milestoneService.findByDisplayID(displayId)-            } catch MilestoneService.Error.duplicateDisplayID {-                return IntentHelpers.mapMilestoneError(.duplicateDisplayID).json-            } catch {-                return IntentHelpers.encodeJSONArray([])-            }-            let filtered = applyFilters(json, to: [milestone], resolvedProjectId: resolvedProjectId)-            return IntentHelpers.encodeJSONArray(filtered.map { milestoneToDict($0, detailed: true) })+            return lookupMilestoneByDisplayID(+                json["displayId"],+                json: json,+                milestoneService: milestoneService,+                resolvedProjectId: resolvedProjectId+            )         }          // Fetch all milestones and filter in-memory. Surface storage fetch failures as@@ -115,6 +104,34 @@ struct QueryMilestonesIntent: AppIntent {      // MARK: - Private Helpers +    @MainActor+    private static func lookupMilestoneByDisplayID(+        _ rawDisplayID: Any?,+        json: [String: Any],+        milestoneService: MilestoneService,+        resolvedProjectId: UUID?+    ) -> String {+        // Route through IntentHelpers.parseIntValue so JSON booleans (delivered as+        // NSNumber wrapping CFBoolean) are rejected rather than silently coerced to+        // 1/0 and targeting M-1/M-0. A plain `as? Int` would accept them. [T-1280]+        guard let displayId = IntentHelpers.parseIntValue(rawDisplayID) else {+            return IntentError.invalidInput(hint: "displayId must be an integer").json+        }++        let milestone: Milestone+        do {+            milestone = try milestoneService.findByDisplayID(displayId)+        } catch MilestoneService.Error.milestoneNotFound {+            return IntentHelpers.encodeJSONArray([])+        } catch MilestoneService.Error.duplicateDisplayID {+            return IntentHelpers.mapMilestoneError(.duplicateDisplayID).json+        } catch {+            return IntentError.internalError(hint: "Failed to look up milestone: \(error)").json+        }+        let filtered = applyFilters(json, to: [milestone], resolvedProjectId: resolvedProjectId)+        return IntentHelpers.encodeJSONArray(filtered.map { milestoneToDict($0, detailed: true) })+    }+     private static func parseInput(_ input: String) -> [String: Any]? {         let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines)         if trimmed.isEmpty {
Transit/Transit/Intents/QueryTasksIntent.swift Modified +6 / -4
diff --git a/Transit/Transit/Intents/QueryTasksIntent.swift b/Transit/Transit/Intents/QueryTasksIntent.swiftindex 23f3dec..830a4e2 100644--- a/Transit/Transit/Intents/QueryTasksIntent.swift+++ b/Transit/Transit/Intents/QueryTasksIntent.swift@@ -118,9 +118,9 @@ struct QueryTasksIntent: AppIntent {             return intentError.json         } -        // Single-task lookup by displayId. Surface CloudKit duplicate-id corruption-        // as an INTERNAL_ERROR instead of letting `try?` collapse it into an empty-        // "not found" result.+        // Single-task lookup by displayId. Only a genuine no-match is an empty+        // result; CloudKit duplicate-id corruption and storage failures must remain+        // distinguishable from a valid "not found" response. [T-1862]         if let displayId = filters.displayId {             do {                 let task = try taskService.findByDisplayID(displayId)@@ -129,12 +129,14 @@ struct QueryTasksIntent: AppIntent {                 return IntentHelpers.encodeJSONArray(filtered.map {                     IntentHelpers.taskToDict($0, formatter: formatter, detailed: true)                 })+            } catch TaskService.Error.taskNotFound {+                return IntentHelpers.encodeJSONArray([])             } catch TaskService.Error.duplicateDisplayID {                 return IntentError.internalError(                     hint: "Duplicate task identifier for displayId \(displayId)"                 ).json             } catch {-                return IntentHelpers.encodeJSONArray([])+                return IntentError.internalError(hint: "Failed to look up task: \(error)").json             }         } 
Transit/Transit/MCP/MCPToolHandler.swift Modified +3 / -1
diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex 7dc4362..c183bfd 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -863,10 +863,12 @@ extension MCPToolHandler {             let formatter = ISO8601DateFormatter()             let dict = milestoneToDict(milestone, formatter: formatter, detailed: true)             return textResult(IntentHelpers.encodeJSONArray([dict]))+        } catch MilestoneService.Error.milestoneNotFound {+            return textResult(IntentHelpers.encodeJSONArray([]))         } catch MilestoneService.Error.duplicateDisplayID {             return errorResult("Duplicate milestone identifier detected for displayId \(displayId)")         } catch {-            return textResult(IntentHelpers.encodeJSONArray([]))+            return errorResult("Failed to look up milestone: \(error)")         }     } 
Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift Added +148 / -0
diff --git a/Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift b/Transit/TransitTests/DisplayIDLookupStorageFailureTests.swiftnew file mode 100644index 0000000..76d15ca--- /dev/null+++ b/Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift@@ -0,0 +1,148 @@+#if os(macOS)+import Foundation+import SwiftData+import Testing+@testable import Transit++/// T-1862: single-record display-ID queries must preserve the three lookup+/// outcomes across their JSON and MCP adapters: not-found is `[]`, duplicate+/// IDs remain explicit errors, and storage failures remain INTERNAL_ERROR/tool+/// errors rather than false successful empty arrays.+@MainActor @Suite(.serialized)+struct DisplayIDLookupStorageFailureTests {++    private struct FetchFailure: Swift.Error, CustomStringConvertible {+        var description: String { "simulated display-ID lookup fetch failure" }+    }++    private struct FailingLookupFetcher: ModelFetching {+        func fetch<T: PersistentModel>(_ descriptor: FetchDescriptor<T>) throws -> [T] {+            throw FetchFailure()+        }+    }++    private func allocator() -> DisplayIDAllocator {+        DisplayIDAllocator(store: InMemoryCounterStore())+    }++    private func intentArray(_ response: String) throws -> [[String: Any]] {+        let data = try #require(response.data(using: .utf8))+        return try #require(try JSONSerialization.jsonObject(with: data) as? [[String: Any]])+    }++    private func expectIntentInternalError(_ response: String, hint: String) throws {+        let data = try #require(response.data(using: .utf8))+        let payload = try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+        #expect(Set(payload.keys) == Set(["error", "hint"]))+        #expect(payload["error"] as? String == "INTERNAL_ERROR")+        #expect(payload["hint"] as? String == hint)+    }++    @Test func queryTasksDisplayIdClassifiesNotFoundDuplicateAndStorageFailure() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let projectService = ProjectService(modelContext: context)+        let milestoneService = MilestoneService(modelContext: context, displayIDAllocator: allocator())+        let normalTaskService = TaskService(modelContext: context, displayIDAllocator: allocator())++        let missing = QueryTasksIntent.execute(+            input: #"{"displayId":42}"#,+            projectService: projectService,+            taskService: normalTaskService,+            milestoneService: milestoneService+        )+        #expect(try intentArray(missing).isEmpty)++        let project = Project(name: "Transit", description: "", gitRepo: nil, colorHex: "#000000")+        let first = TransitTask(name: "First", type: .feature, project: project, displayID: .permanent(42))+        let second = TransitTask(name: "Second", type: .feature, project: project, displayID: .permanent(42))+        StatusEngine.initializeNewTask(first)+        StatusEngine.initializeNewTask(second)+        context.insert(project)+        context.insert(first)+        context.insert(second)++        let duplicate = QueryTasksIntent.execute(+            input: #"{"displayId":42}"#,+            projectService: projectService,+            taskService: normalTaskService,+            milestoneService: milestoneService+        )+        try expectIntentInternalError(+            duplicate,+            hint: "Duplicate task identifier for displayId 42"+        )++        let failingTaskService = TaskService(+            modelContext: context,+            displayIDAllocator: allocator(),+            fetcher: FailingLookupFetcher()+        )+        let storageFailure = QueryTasksIntent.execute(+            input: #"{"displayId":43}"#,+            projectService: projectService,+            taskService: failingTaskService,+            milestoneService: milestoneService+        )+        try expectIntentInternalError(+            storageFailure,+            hint: "Failed to look up task: simulated display-ID lookup fetch failure"+        )+    }++    @Test func queryMilestonesDisplayIdClassifiesOutcomesAcrossIntentAndMCP() async throws {+        let normal = try MCPTestHelpers.makeEnv()+        let missingArguments: [String: Any] = ["displayId": 42]++        let missingMCP = await normal.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_milestones", arguments: missingArguments+        ))+        #expect(try MCPTestHelpers.decodeArrayResult(missingMCP).isEmpty)+        let missingIntent = QueryMilestonesIntent.execute(+            input: #"{"displayId":42}"#,+            milestoneService: normal.milestoneService,+            projectService: normal.projectService+        )+        #expect(try intentArray(missingIntent).isEmpty)++        let project = MCPTestHelpers.makeProject(in: normal.context, name: "Transit")+        let first = Milestone(name: "First", description: nil, project: project, displayID: .permanent(42))+        let second = Milestone(name: "Second", description: nil, project: project, displayID: .permanent(42))+        normal.context.insert(first)+        normal.context.insert(second)++        let duplicateMCP = await normal.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_milestones", arguments: missingArguments+        ))+        #expect(try MCPTestHelpers.isError(duplicateMCP))+        #expect(try MCPTestHelpers.errorText(duplicateMCP)+            == "Duplicate milestone identifier detected for displayId 42")+        let duplicateIntent = QueryMilestonesIntent.execute(+            input: #"{"displayId":42}"#,+            milestoneService: normal.milestoneService,+            projectService: normal.projectService+        )+        try expectIntentInternalError(+            duplicateIntent,+            hint: "A duplicate milestone identifier was detected"+        )++        let failing = try MCPTestHelpers.makeEnv(milestoneServiceFetcher: FailingLookupFetcher())+        let storageFailureMCP = await failing.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_milestones", arguments: missingArguments+        ))+        #expect(try MCPTestHelpers.isError(storageFailureMCP))+        #expect(try MCPTestHelpers.errorText(storageFailureMCP)+            == "Failed to look up milestone: simulated display-ID lookup fetch failure")+        let storageFailureIntent = QueryMilestonesIntent.execute(+            input: #"{"displayId":42}"#,+            milestoneService: failing.milestoneService,+            projectService: failing.projectService+        )+        try expectIntentInternalError(+            storageFailureIntent,+            hint: "Failed to look up milestone: simulated display-ID lookup fetch failure"+        )+    }+}+#endif
specs/bugfixes/display-id-queries-hide-storage-lookup-failures/report.md Added +114 / -0
diff --git a/specs/bugfixes/display-id-queries-hide-storage-lookup-failures/report.md b/specs/bugfixes/display-id-queries-hide-storage-lookup-failures/report.mdnew file mode 100644index 0000000..d88b531--- /dev/null+++ b/specs/bugfixes/display-id-queries-hide-storage-lookup-failures/report.md@@ -0,0 +1,114 @@+# Bugfix Report: Display-ID Queries Hide Storage Lookup Failures++**Date:** 2026-08-05  +**Status:** Fixed  +**Ticket:** T-1862++## Description of the Issue++The single-record `displayId` paths in `QueryTasksIntent`, `QueryMilestonesIntent`, and MCP `query_milestones` treated every lookup error other than a duplicate identifier as a valid empty result (`[]`). A caller therefore could not distinguish a truly absent task or milestone from an unreadable SwiftData store.++**Reproduction steps:**+1. Inject a `ModelFetching` implementation that throws into `TaskService` or `MilestoneService`.+2. Query a single task or milestone using a `displayId`.+3. Observe a successful empty array instead of `INTERNAL_ERROR` (App Intents) or an MCP tool error.++**Impact:** Automations could incorrectly act on a transient or persistent storage failure as though the target record did not exist, potentially taking unsafe follow-up actions.++## Investigation Summary++### Phase 1 — Initial overview++Expected behavior is three distinct outcomes: explicit service not-found errors return `[]`; duplicate display IDs keep their existing corruption error; every other lookup failure is surfaced as an internal/tool error. T-1657 and T-1675 establish that infrastructure failures must remain distinct from valid no-match results and preserve source-specific lookup text.++The focused test suite was red before the implementation:++```text+DisplayIDLookupStorageFailureTests.queryTasksDisplayIdClassifiesNotFoundDuplicateAndStorageFailure()+DisplayIDLookupStorageFailureTests.queryMilestonesDisplayIdClassifiesOutcomesAcrossIntentAndMCP()+```++### Phase 2 — Systematic inspection++- `TaskService.findByDisplayID` and `MilestoneService.findByDisplayID` already distinguish `taskNotFound` / `milestoneNotFound`, `duplicateDisplayID`, and thrown SwiftData errors through their injected `ModelFetching` seams.+- The full-table query paths already map fetch failures to `INTERNAL_ERROR` or MCP tool errors.+- `QueryTasksIntent` caught duplicates explicitly but mapped all remaining errors, including storage failures, to `[]`.+- `QueryMilestonesIntent` and MCP `lookupMilestoneByDisplayId` had the same catch-all-empty behavior.+- Existing MCP test setup already exposes `MilestoneService`'s injected fetcher, matching T-1675, so no new production seam was necessary.++### Phase 3 — Root cause analysis++1. Why did unreadable storage appear as no match? The display-ID adapters used a generic catch that returned `[]`.+2. Why did the generic catch include infrastructure failures? It did not first match the service's explicit not-found case.+3. Why is that incorrect? `[]` is a successful query result and must be reserved for a verified missing record or a filter mismatch.+4. Why was the defect limited to these paths? The adjacent full-table and MCP task display-ID paths already preserve unexpected fetch failures.+5. Why did it persist? Regression tests covered ordinary no-match and duplicate behavior but not a failing direct-lookup seam across these adapters.++**Root cause:** three single-record display-ID adapters collapsed a typed absence result and an untyped storage failure into the same successful empty-array response.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/Intents/QueryTasksIntent.swift` — catches only `TaskService.Error.taskNotFound` as `[]`; unexpected lookup failures now return `INTERNAL_ERROR` with `Failed to look up task: <error>`.+- `Transit/Transit/Intents/QueryMilestonesIntent.swift` — catches only `MilestoneService.Error.milestoneNotFound` as `[]`; unexpected lookup failures now return `INTERNAL_ERROR` with `Failed to look up milestone: <error>`.+- `Transit/Transit/MCP/MCPToolHandler.swift` — MCP `query_milestones` now returns `Failed to look up milestone: <error>` for unexpected single-record lookup failures.+- `Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift` — adds deterministic, exact-contract tests for not-found, duplicate, and injected storage-failure outcomes across the affected JSON and MCP surfaces.++**Approach rationale:** This is the smallest change that reuses existing service seams and error contracts. It follows T-1657/T-1675 by catching typed domain outcomes first and preserving unexpected storage errors without changing the service APIs or pre-existing duplicate wording.++**Alternatives considered:**+- Return `[]` for all lookup failures — rejected because it makes store failures indistinguishable from a valid no-match.+- Introduce a new service error case — rejected because raw fetch errors already flow through the injected seam and established callers classify them as internal errors.+- Change MCP `query_tasks` — rejected because its display-ID path already returns a tool error for unexpected lookup failures and is outside this ticket's scope.++## Regression Test++**Test file:** `Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift`++**Tests:**+- `queryTasksDisplayIdClassifiesNotFoundDuplicateAndStorageFailure`+- `queryMilestonesDisplayIdClassifiesOutcomesAcrossIntentAndMCP`++**What it verifies:** Exact no-match arrays and duplicate errors remain unchanged, while deterministic `ModelFetching` failures produce the source-specific `INTERNAL_ERROR` payload or MCP tool error rather than a false successful `[]`.++**Focused run command:**++```bash+xcodebuild test -project Transit/Transit.xcodeproj -scheme Transit \+  -destination 'platform=macOS' \+  -only-testing:TransitTests/DisplayIDLookupStorageFailureTests+```++## Affected Files++| File | Change |+|---|---|+| `Transit/Transit/Intents/QueryTasksIntent.swift` | Separates task not-found from unexpected lookup failure. |+| `Transit/Transit/Intents/QueryMilestonesIntent.swift` | Separates milestone not-found from unexpected lookup failure. |+| `Transit/Transit/MCP/MCPToolHandler.swift` | Returns a lookup tool error for an unexpected milestone lookup failure. |+| `Transit/TransitTests/DisplayIDLookupStorageFailureTests.swift` | Exact cross-surface regression coverage. |+| `CHANGELOG.md` | Records the corrected public automation contract. |++## Verification++**Automated:**+- [x] Focused regression suite failed before the production change.+- [x] Focused regression suite passed after the production change.+- [x] Full macOS unit suite passes (`make test-quick`).+- [x] SwiftLint and the SwiftData ownership guard pass (`make lint`).+- [x] iOS Simulator build passes (`make build-ios`).++**Manual verification:** Not required. The injected `ModelFetching` seam deterministically exercises the failing storage branch without relying on a corrupted production store.++## Prevention++- Reserve successful `[]` results for an explicit typed not-found outcome or post-lookup filter mismatch.+- Catch domain errors before generic infrastructure failures and preserve a source-specific error message for the latter.+- Add an injected-seam regression whenever a single-record query adapter adds or changes error mapping.++## Related++- T-1862+- T-1657 — project lookup storage failures remain distinct from no match.+- T-1675 — scoped milestone lookup failures preserve exact source-specific errors.+- T-1770 — generic persistence failures map to `INTERNAL_ERROR`.

Things to double-check

No merge was performed

claude-review completed successfully. This report records readiness only; the PR remains open and unmerged.

Consider a follow-up for mutation lookup resolvers

The CR-level comment is accurate: resolveMilestone(from:) and resolveTaskArgument(_:) still map unexpected service failures to misleading messages. It is outside this query-only PR’s declared scope, so it is a follow-up suggestion rather than a release blocker.