|
| 1 | +# Task 1254: iOS Restore Guard And Gateway Thread Type Consistency |
| 2 | + |
| 3 | +## Problem |
| 4 | + |
| 5 | +Build 108 includes `3557c4c3` (`Fix iOS home cold-start restore`). That patch |
| 6 | +made the iOS persisted last-opened restore path reject resolved workflow-run |
| 7 | +destinations and clear polluted restore defaults. |
| 8 | + |
| 9 | +The pending-intent theory is rejected. `pendingThreadId` is queued only by |
| 10 | +URL/widget route handling before the gateway is ready, so it is deferred user |
| 11 | +navigation, not automatic restore. Cold-start deep links to workflow runs must |
| 12 | +continue to open workflow runs, just like warm links. |
| 13 | + |
| 14 | +The current iOS restore path is also narrower than the second draft claimed: |
| 15 | +iOS cold-start restore resolves missing recent-list entries through |
| 16 | +`/api/threads/{id}`, and the app test below confirms that a workflow summary |
| 17 | +fetched from that endpoint is rejected and clears polluted restore state. iOS |
| 18 | +does not currently decode the `thread` or `session` summary blocks returned by |
| 19 | +`/api/threads/history`. |
| 20 | + |
| 21 | +There is still a real gateway contract bug worth fixing: gateway history detail |
| 22 | +summaries can report a stored workflow-run thread as `chat`. That is independent |
| 23 | +contract hygiene for clients and tools that do consume the history envelope, not |
| 24 | +a proven iOS cold-start root cause. |
| 25 | + |
| 26 | +## Evidence |
| 27 | + |
| 28 | +Rejected iOS candidate: |
| 29 | + |
| 30 | +- `queuePendingThreadLink` is reached through `openMobileRouteFromLink` and |
| 31 | + `queuePendingMobileRoute`, which are triggered from `.onOpenURL`; |
| 32 | +- the pending route is in memory and not a persisted automatic restore source; |
| 33 | +- blocking it would break explicit workflow-run links during cold start. |
| 34 | + |
| 35 | +Confirmed iOS restore guard: |
| 36 | + |
| 37 | +- `GaryxMobileModel.restoreLastOpenedThread(id:)` consults recent in-memory |
| 38 | + summaries, then refreshed recent threads, then `client().getThread(threadId:)` |
| 39 | + (`/api/threads/{id}`); |
| 40 | +- every resolved summary goes through the existing restoration policy before the |
| 41 | + app opens a destination; |
| 42 | +- `GaryxThreadTranscript` does not decode `/api/threads/history` `thread` or |
| 43 | + `session` blocks, so that endpoint is not a current iOS restore input. |
| 44 | + |
| 45 | +Confirmed gateway contract split: |
| 46 | + |
| 47 | +- list summaries derive `thread_type` from stored `thread_kind` with a `chat` |
| 48 | + fallback; |
| 49 | +- `/api/threads/{id}` metadata currently clones raw metadata and does not |
| 50 | + normalize `thread_type`; |
| 51 | +- `/api/threads/history` derives both `thread_type` and `session_type` from |
| 52 | + `infer_thread_type(thread_id)`, so a normal workflow-run id with stored |
| 53 | + `thread_kind: workflow_run` comes back as `chat`; |
| 54 | +- the same fallback is inconsistent for legacy ids without `thread_kind`: |
| 55 | + metadata/list treat them as `chat`, while history currently treats |
| 56 | + `cron::...` as `cron`. |
| 57 | + |
| 58 | +Added deterministic regression coverage: |
| 59 | + |
| 60 | +- App coverage: |
| 61 | + `GaryxLastOpenedWorkflowRestoreTests.testColdLaunchRestoreRejectsWorkflowRunFetchedAfterRecentListOmission` |
| 62 | + uses the captured workflow fixture, starts with empty in-memory `threads`, |
| 63 | + stubs empty recent/pins plus `/api/threads/{id}`, and verifies restore lands on |
| 64 | + home and clears polluted defaults. This passes today and documents that the |
| 65 | + direct iOS getThread fallback is not the current hole. |
| 66 | +- Gateway red test: |
| 67 | + `api::tests::test_thread_history_detail_preserves_workflow_thread_type` seeds a |
| 68 | + transcript-backed thread with `thread_kind: workflow_run`, calls |
| 69 | + `/api/threads/history?thread_id=...`, and currently fails because |
| 70 | + `thread.thread_type == "chat"` instead of `"workflow_run"`. |
| 71 | +- Gateway fallback red test: |
| 72 | + `api::tests::test_thread_history_detail_defaults_missing_thread_kind_to_chat` |
| 73 | + seeds a `cron::...` shaped legacy id without `thread_kind` and verifies history |
| 74 | + defaults to `chat`, matching metadata/list behavior. |
| 75 | +- Metadata red tests: |
| 76 | + `routes::tests::thread_metadata_preserves_workflow_thread_type` and |
| 77 | + `routes::tests::thread_metadata_defaults_missing_thread_kind_to_chat` verify the |
| 78 | + single-thread metadata response uses the same type derivation as history and |
| 79 | + list summaries. |
| 80 | + |
| 81 | +## Design |
| 82 | + |
| 83 | +Introduce one gateway helper for thread-summary type derivation: |
| 84 | + |
| 85 | +```rust |
| 86 | +thread_kind_from_value(data).unwrap_or_else(|| "chat".to_owned()) |
| 87 | +``` |
| 88 | + |
| 89 | +Use that helper in all JSON summary surfaces that build from a raw thread record: |
| 90 | + |
| 91 | +- `routes::thread_summary(...)`, used by list/create/update style summaries; |
| 92 | +- `routes::thread_metadata_response(...)`, used by `/api/threads/{id}`; |
| 93 | +- `api::summarize_thread(...)`, used by `/api/threads/history` `thread` and |
| 94 | + `session` envelopes. |
| 95 | + |
| 96 | +The helper deliberately has no id-pattern fallback. If old thread metadata lacks |
| 97 | +`thread_kind`, every raw-record summary surface reports `chat`. That matches the |
| 98 | +existing list fallback and eliminates the history-vs-metadata split for |
| 99 | +`cron::...` or group-shaped legacy ids without stored kind data. |
| 100 | + |
| 101 | +The fix is server-side because `thread_kind` is gateway/router state, and |
| 102 | +clients should not guess workflow identity from ids or workflow metadata side |
| 103 | +channels. |
| 104 | + |
| 105 | +## Why This Is Not A Patch |
| 106 | + |
| 107 | +The fix does not add an iOS-only `if workflow_run` guard and does not special |
| 108 | +case one cold-start branch. It makes the gateway surfaces that summarize the |
| 109 | +same raw thread record share the same type derivation and fallback. |
| 110 | + |
| 111 | +The iOS policy remains: |
| 112 | + |
| 113 | +- automatic persisted restore may open chat destinations only; |
| 114 | +- workflow-run and unresolved destinations clear polluted restore state and land |
| 115 | + on home; |
| 116 | +- direct user opens, including cold-start deep links/widgets, may open workflow |
| 117 | + runs. |
| 118 | + |
| 119 | +This design does not claim that `/api/threads/history` is the current iOS |
| 120 | +cold-start failure input. It preserves the existing iOS restore fix with app |
| 121 | +coverage and fixes an adjacent gateway contract bug that could mislead history |
| 122 | +consumers. |
| 123 | + |
| 124 | +## Validation Plan |
| 125 | + |
| 126 | +Before implementation: |
| 127 | + |
| 128 | +- Red gateway test: |
| 129 | + `cargo test -p garyx-gateway test_thread_history_detail_preserves_workflow_thread_type -- --nocapture` |
| 130 | + fails with `left: String("chat")`, `right: "workflow_run"`. |
| 131 | +- Red gateway fallback test: |
| 132 | + `cargo test -p garyx-gateway test_thread_history_detail_defaults_missing_thread_kind_to_chat -- --nocapture` |
| 133 | + fails because history reports `cron` for a `cron::...` id without |
| 134 | + `thread_kind`. |
| 135 | +- Red metadata tests: |
| 136 | + `cargo test -p garyx-gateway thread_metadata_preserves_workflow_thread_type -- --nocapture` |
| 137 | + and |
| 138 | + `cargo test -p garyx-gateway thread_metadata_defaults_missing_thread_kind_to_chat -- --nocapture` |
| 139 | + fail because metadata does not normalize `thread_type`. |
| 140 | +- App getThread fallback coverage: |
| 141 | + `xcodebuild test -project mobile/garyx-mobile/GaryxMobile.xcodeproj -scheme GaryxMobile -destination 'id=<simulator>' -only-testing:GaryxMobileTests/GaryxLastOpenedWorkflowRestoreTests/testColdLaunchRestoreRejectsWorkflowRunFetchedAfterRecentListOmission CODE_SIGNING_ALLOWED=NO` |
| 142 | + passes and confirms that branch is already protected. |
| 143 | + |
| 144 | +After implementation: |
| 145 | + |
| 146 | +- Green gateway regressions above. |
| 147 | +- Green relevant gateway history tests: |
| 148 | + `cargo test -p garyx-gateway thread_history_detail -- --nocapture`. |
| 149 | +- Green route metadata summary tests: |
| 150 | + `cargo test -p garyx-gateway thread_metadata_ -- --nocapture`. |
| 151 | +- Green iOS restore tests: |
| 152 | + `xcodebuild test -project mobile/garyx-mobile/GaryxMobile.xcodeproj -scheme GaryxMobile -destination 'id=<simulator>' -only-testing:GaryxMobileTests/GaryxLastOpenedWorkflowRestoreTests CODE_SIGNING_ALLOWED=NO`. |
| 153 | +- Green SwiftPM Core restore policy tests: |
| 154 | + `swift test --package-path mobile/garyx-mobile --filter GaryxLastOpenedThreadRestorationPolicyTests`. |
| 155 | +- Real app-target compile: |
| 156 | + `xcodebuild build -project mobile/garyx-mobile/GaryxMobile.xcodeproj -scheme GaryxMobile -destination 'id=<simulator>' CODE_SIGNING_ALLOWED=NO`. |
0 commit comments