async-app-result-removal-migration.md

  1# Project: AsyncApp Result Removal - Codebase Migration
  2
  3## Prerequisites
  4
  5This brief depends on the completion of `async-app-result-removal.md`, which:
  6- Adds trampoline check infrastructure in dispatchers
  7- Updates `AsyncApp` API with `try_update()` / `update()` pattern
  8- Removes `AppContext::Result<T>` associated type
  9- Removes `Flatten` trait
 10
 11## Overview
 12
 13This phase migrates the entire codebase to use the new `AsyncApp` API. Since foreground tasks are now automatically cancelled when the app dies, most callsites can remove their error handling.
 14
 15## Phase 1: Audit Cross-Boundary Awaits
 16
 17### Goal
 18
 19Identify places where background tasks await foreground tasks, as these are the edge cases that need `try_update()`.
 20
 21### Search Patterns
 22
 23```
 24# Background tasks awaiting foreground work
 25background_executor().spawn(...).await
 26
 27# Channels receiving from foreground tasks
 28receiver.recv().await  # where sender is in foreground
 29
 30# Task<T> held across thread boundaries
 31let task: Task<T> = cx.spawn(...)  # then moved to background
 32```
 33
 34### Known Patterns to Check
 35
 361. **cx.spawn(...).await from background** - If a background task stores an `AsyncApp` and calls `spawn`, then awaits the result
 372. **Channel patterns** - Foreground sends via channel, background receives
 383. **Task handles passed to background** - A `Task<T>` created in foreground, awaited in background
 39
 40### Action Items
 41
 42- [ ] Search for `background_spawn` calls that contain awaits on foreground work
 43- [ ] Audit `Task<T>` usage to find cross-boundary cases
 44- [ ] Create list of callsites requiring `try_update()` instead of `update()`
 45- [ ] Document any patterns that cannot be safely migrated
 46
 47## Phase 2: Codebase Migration
 48
 49### Migration Strategy
 50
 511. **Automated pass**: Use search-and-replace for simple patterns
 522. **Manual review**: Handle complex cases requiring `try_update()`
 53
 54### Simple Patterns (Automated)
 55
 56```rust
 57// Before
 58cx.update(|app| { ... })?
 59cx.update(|app| { ... }).ok();
 60cx.update(|app| { ... }).unwrap();
 61
 62// After
 63cx.update(|app| { ... })
 64```
 65
 66### Complex Patterns (Manual)
 67
 68```rust
 69// Background awaiting foreground - use try_update
 70cx.background_spawn(async move {
 71    // Before
 72    let result = task.await?;
 73    
 74    // After - if task could be cancelled
 75    let Some(result) = cx.try_update(|app| { ... }) else {
 76        return; // or handle gracefully
 77    };
 78});
 79```
 80
 81### Crate Priority Order
 82
 83Migrate in dependency order to catch issues early:
 84
 851. `crates/gpui` - Core framework
 862. `crates/language` - Language support
 873. `crates/project` - Project management
 884. `crates/editor` - Editor core
 895. `crates/workspace` - Workspace management
 906. `crates/agent` and `crates/agent_ui` - AI agent
 917. Remaining crates alphabetically
 92
 93### Per-Crate Checklist
 94
 95For each crate:
 96- [ ] Find all `AsyncApp` / `AsyncWindowContext` usage
 97- [ ] Categorize: simple removal vs. needs `try_update()`
 98- [ ] Apply changes
 99- [ ] Run `cargo test -p <crate>` 
100- [ ] Run `./script/clippy`
101
102## Phase 3: Testing & Cleanup
103
104### Remove Dead Code
105
106After migration, search for and remove:
107
108```rust
109// Dead imports
110use anyhow::{Context, Result};  // if only used for AsyncApp errors
111
112// Dead error handling
113.context("app was released")
114.context("window was closed")
115
116// Unused Result type aliases
117type Result<T> = anyhow::Result<T>;  // if only used for AsyncApp
118```
119
120### Update Documentation
121
122- [ ] Update `AsyncApp` rustdoc to explain new semantics
123- [ ] Update GPUI documentation/examples
124- [ ] Add migration guide for external users (if any)
125
126### New Tests
127
128Add tests to prevent regression:
129
130- [ ] Test: `update()` works when app is alive
131- [ ] Test: `try_update()` returns `None` when app is gone
132- [ ] Test: Tasks are cancelled (not panicking) when app dies
133- [ ] Test: Nested tasks both cancel cleanly
134
135### Final Validation
136
137- [ ] `cargo test` (full suite)
138- [ ] `./script/clippy` 
139- [ ] Manual testing: heavy async workloads
140- [ ] Manual testing: rapid window open/close
141- [ ] Manual testing: quit app with pending tasks
142
143## Estimated Scope
144
145Based on grep analysis:
146- ~500+ callsites using `AsyncApp::update()` or similar
147- ~50 crates with potential changes
148- Most changes are mechanical `.unwrap()` / `?` removal
149- ~10-20 complex cases requiring `try_update()`
150
151## Risk Mitigation
152
153### Rollback Plan
154
155If issues are discovered post-merge:
1561. Revert migration commits (codebase changes)
1572. Keep infrastructure changes (they're backwards compatible)
1583. Re-evaluate edge cases
159
160### Incremental Rollout
161
162Consider migrating in stages:
1631. First: `crates/gpui` only - validate core behavior
1642. Second: High-traffic crates (`editor`, `workspace`, `project`)
1653. Third: Remaining crates
166
167## Files Reference
168
169Key files for finding callsites:
170- `crates/gpui/src/app/async_context.rs` - `AsyncApp` definition
171- Search: `update(|`, `update_entity(`, `read_entity(`
172- Search: `.unwrap()`, `.ok()`, `?` following async context calls