Independent exact-head review of PR #224. The branch is already published (no origin/T-1657/bugfix-project-lookup-hides-fetch-failures..HEAD delta), so this applies the pre-push workflow to the requested immutable PR range: base ae8a7169e891c7dd0aaf28f840f814bd72fb7569 through head 576a19a1c006443dfb13795390431aa98d0843f3.
INTERNAL_ERROR for ProjectLookupError.storageFailure, while genuine name misses and ambiguity retain the established successful empty-array contract.INTERNAL_ERROR; a stale selected project remains PROJECT_NOT_FOUND.30918542481 succeeded on this exact SHA; GitHub returned zero review threads; local lint, fast tests, and both-platform build passed.Ready
No correctness, regression, security, or maintainability finding was identified. The exact rebased T-1657 implementation and tests are byte-identical to previously reviewed d72e5a541814253636982238cd5c1e885f9d2da3; git range-diff also reports the compilation remediation as equivalent. The only old-versus-new difference is an unrelated changelog baseline entry. Exact-head make lint, make test-quick, and make build all passed.
dfd43a2 T-1657: Add project lookup failure regressions 30c993c T-1657: Propagate project lookup storage failures 576a19a Fix T-1657 project filter resolution compilation Transit normally needs to find a project before creating or filtering work. A missing project is a normal outcome, but a database read failure is different: retrying may work. This change stops two paths from treating a read failure as if the project were simply absent. Shortcuts now returns an internal error instead of an empty list, and the visual Add Task shortcut tells the user that the database could not be read.
The tests deliberately make the project read fail. They verify the correct error reaches JSON, MCP, and visual interfaces and that no task or milestone was added.
ProjectService.findProject is the error boundary: it returns a typed Result<Project, ProjectLookupError>. T-1770's repository-wide rule is that untyped persistence failures map to INTERNAL_ERROR, not a domain validation result. Query Milestones performs typed preflight resolution into ProjectFilterResolution; it preserves projectId precedence and folds only notFound/ambiguous into its historical valid-empty query result. Add Task maps only storageFailure to the new visual error case before it calls TaskService.createTask.
The rebase repairs a Swift type-system defect without changing T-1657 semantics: the reviewed version replaces invalid Result<UUID?, String> transport with a purpose-specific enum whose .error(IntentError) preserves delayed JSON serialization. Direct comparison proves six T-1657 code/test/report blobs equal between d72e5a5 and 576a19a; git range-diff d72^...d72 ae8...576 marks the remediating commit equal. The only comparison delta is an unrelated T-1628 changelog baseline line. There is no new fetch/mutation path, no widened API, and no changed project-filter precedence.
Transit/Transit/Intents/QueryMilestonesIntent.swift
Why it matters. Prevents a database fetch failure from being reported as a successful empty result, while preserving valid-empty behavior for missing or ambiguous names.
What to look at. QueryMilestonesIntent.execute; resolveValidatedProjectFilter; resolveProjectFilter
Transit/Transit/Intents/Visual/AddTaskIntent.swift
Why it matters. Avoids misleading a user into changing a valid selection when the actual problem is an unreadable database.
What to look at. AddTaskIntent.execute project lookup switch
Transit/Transit/Intents/Visual/VisualIntentError.swift
Why it matters. Keeps visual Shortcuts behavior aligned with JSON App Intent and T-1770 persistence-failure semantics.
What to look at. VisualIntentError.storageFailure and LocalizedError mappings
Transit/TransitTests/ProjectLookupStorageFailureSurfaceTests.swift
Why it matters. Proves JSON, MCP, and visual clients expose the intended contracts and that failed creation never inserts a model.
What to look at. ProjectLookupStorageFailureSurfaceTests (four serialized tests)
ee8cfbc (T-1770) is an ancestor of exact head. Its convention applies directly: ProjectLookupError.storageFailure maps to INTERNAL_ERROR, rather than a not-found, ambiguity, or valid-empty outcome.
Name-based notFound and ambiguous remain [], because this query historically treats them as no match. The new preflight escalates only storageFailure. A syntactically valid projectId still takes precedence without an existence lookup, preserving previous filtering semantics.
The previously reviewed compilation fix removes invalid Result<UUID?, String> usage: Swift requires a Result failure type to conform to Error. The enum cleanly represents unfiltered, resolved, valid-no-match, and typed-error states without serializing JSON prematurely.
Click to expand.
Proven: each T-1657 code, test, and report blob is identical in d72e5a541814253636982238cd5c1e885f9d2da3 and 576a19a1c006443dfb13795390431aa98d0843f3. git range-diff reports d72e5a5 = 576a19a. Comparing the two heads yields only an unrelated T-1628 changelog-line difference.
11 direct invocations across 10 logical paths: JSON Create Task, Create Milestone, Query Tasks, Query Milestones, shared JSON milestone-name resolver (used by Update Milestone), visual Add Task, MCP create_task, MCP query_tasks (ID and name branches), MCP create_milestone, and MCP query_milestones. JSON/MCP mutation and task-query callers use IntentHelpers.mapProjectLookupError: notFound → PROJECT_NOT_FOUND/tool error, ambiguous → AMBIGUOUS_PROJECT/tool error, storageFailure → INTERNAL_ERROR/tool error, noIdentifier → INVALID_INPUT/tool error. Query Milestones intentionally maps only storageFailure to INTERNAL_ERROR and preserves notFound/ambiguous as []. Visual Add Task maps storageFailure to INTERNAL_ERROR and any non-storage ID miss to PROJECT_NOT_FOUND; UUID lookup cannot be ambiguous.
Every creation caller resolves its project before calling TaskService or MilestoneService creation. The shared JSON milestone resolver runs before Update Milestone validates or applies changes. Query callers do not mutate. The new serialized suite uses a throwing ModelFetching seam and asserts exact errors plus empty task/milestone stores; VisualIntentErrorTests covers the new external code. Exact head passed make lint, make test-quick, and make build.
The owner’s prior PR review overview for exact d72e5a5 records lint success, make test-quick success with 1,724 checks and zero failures, exact-head diff review, and zero unresolved threads: issuecomment-5170090372. Current GitHub CI run 30918542481 completed SUCCESS on exact 576a19a; the PR GraphQL reviewThreads query returned an empty node list.