@@ -1,225 +0,0 @@
-{
- "project": "async-app-result-removal",
- "description": "Remove Result from AsyncApp return types by moving app-alive checks to executor",
- "phases": [
- {
- "id": 1,
- "name": "Add Trampoline Check Infrastructure (macOS First)",
- "status": "completed",
- "tasks": [
- {
- "id": "1.1",
- "description": "Add MainThreadWeak<T> newtype with unsafe Send+Sync impls to platform.rs",
- "file": "crates/gpui/src/platform.rs",
- "status": "completed"
- },
- {
- "id": "1.2",
- "description": "Update RunnableMeta to use Option<MainThreadWeak<AppCell>> for app field",
- "file": "crates/gpui/src/platform.rs",
- "status": "completed"
- },
- {
- "id": "1.3",
- "description": "Update trampoline in Mac dispatcher to check app via unsafe upgrade() before run()",
- "file": "crates/gpui/src/platform/mac/dispatcher.rs",
- "status": "completed"
- },
- {
- "id": "1.4",
- "description": "Update tick() in Test dispatcher to check app via unsafe upgrade() before run()",
- "file": "crates/gpui/src/platform/test/dispatcher.rs",
- "status": "completed"
- },
- {
- "id": "1.5",
- "description": "Add ForegroundExecutor::spawn_with_app that accepts Weak<AppCell>",
- "file": "crates/gpui/src/executor.rs",
- "status": "completed"
- },
- {
- "id": "1.6",
- "description": "Update AsyncApp::spawn to pass self.app wrapped in MainThreadWeak",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "completed"
- },
- {
- "id": "1.7",
- "description": "Update AsyncWindowContext::spawn to pass app wrapped in MainThreadWeak",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "completed"
- },
- {
- "id": "1.8",
- "description": "Write unit tests verifying task cancellation when app is dropped (moved to executor.rs to access pub(crate) items)",
- "file": "crates/gpui/src/executor.rs",
- "status": "completed",
- "notes": "Tests moved from tests/task_cancellation.rs to executor.rs as #[cfg(test)] module because TestAppContext holds strong Rc<AppCell> preventing proper app-drop testing"
- },
- {
- "id": "1.9",
- "description": "Write unit test for nested tasks both cancelling cleanly",
- "file": "crates/gpui/src/executor.rs",
- "status": "completed"
- },
- {
- "id": "1.10",
- "description": "Create async_cancellation.rs example demonstrating behavior",
- "file": "crates/gpui/examples/async_cancellation.rs",
- "status": "completed"
- }
- ]
- },
- {
- "id": 2,
- "name": "Extend to Other Platforms",
- "status": "not_started",
- "tasks": [
- {
- "id": "2.1",
- "description": "Update trampoline function in Linux dispatcher to check app status before run()",
- "file": "crates/gpui/src/platform/linux/dispatcher.rs",
- "status": "not_started"
- },
- {
- "id": "2.2",
- "description": "Update trampoline function in Windows dispatcher to check app status before run()",
- "file": "crates/gpui/src/platform/windows/dispatcher.rs",
- "status": "not_started"
- }
- ]
- },
- {
- "id": 3,
- "name": "Update AsyncApp API & Remove AppContext::Result",
- "status": "not_started",
- "tasks": [
- {
- "id": "3.1",
- "description": "Rename update() -> Result<R> to try_update() -> Option<R>",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.2",
- "description": "Add new update() -> R that panics if app is gone",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.3",
- "description": "Apply try_/non-try pattern to read_entity and update_entity",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.4",
- "description": "Apply try_/non-try pattern to read_global and update_global",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.5",
- "description": "Apply try_/non-try pattern to read_window and update_window",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.6",
- "description": "Apply try_/non-try pattern to new, reserve_entity, insert_entity",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.7",
- "description": "Apply try_/non-try pattern to refresh, open_window, subscribe, has_global",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.8",
- "description": "Apply same changes to AsyncWindowContext",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.9",
- "description": "Remove type Result<T> associated type from AppContext trait",
- "file": "crates/gpui/src/gpui.rs",
- "status": "not_started"
- },
- {
- "id": "3.10",
- "description": "Update all AppContext trait method signatures to return R directly",
- "file": "crates/gpui/src/gpui.rs",
- "status": "not_started"
- },
- {
- "id": "3.11",
- "description": "Remove Flatten trait from gpui.rs",
- "file": "crates/gpui/src/gpui.rs",
- "status": "not_started"
- },
- {
- "id": "3.12",
- "description": "Update WeakEntity::update to remove Flatten bounds",
- "file": "crates/gpui/src/app/entity_map.rs",
- "status": "not_started"
- },
- {
- "id": "3.13",
- "description": "Update WeakEntity::read_with to remove Flatten bounds",
- "file": "crates/gpui/src/app/entity_map.rs",
- "status": "not_started"
- },
- {
- "id": "3.14",
- "description": "Update WeakEntity::update_in to remove Flatten bounds",
- "file": "crates/gpui/src/app/entity_map.rs",
- "status": "not_started"
- },
- {
- "id": "3.15",
- "description": "Update ExampleContext in eval crate to match new AppContext signature",
- "file": "crates/eval/src/example.rs",
- "status": "not_started"
- },
- {
- "id": "3.16",
- "description": "Update AsyncApp documentation",
- "file": "crates/gpui/src/app/async_context.rs",
- "status": "not_started"
- },
- {
- "id": "3.17",
- "description": "Run cargo test -p gpui",
- "file": null,
- "status": "not_started"
- },
- {
- "id": "3.18",
- "description": "Run ./script/clippy",
- "file": null,
- "status": "not_started"
- },
- {
- "id": "3.19",
- "description": "Verify async_cancellation example runs without panics",
- "file": "crates/gpui/examples/async_cancellation.rs",
- "status": "not_started"
- },
- {
- "id": "3.20",
- "description": "Manual test: close windows with pending tasks, verify no panics",
- "file": null,
- "status": "not_started"
- }
- ]
- }
- ],
- "future_work": {
- "brief": "async-app-result-removal-migration.md",
- "description": "Codebase migration to remove error handling from ~500+ callsites",
- "phases": ["Audit Cross-Boundary Awaits", "Codebase Migration", "Testing and Cleanup"]
- }
-}
@@ -1,279 +0,0 @@
-# Project: Remove Result from AsyncApp Return Types
-
-## Problem
-
-`AsyncApp` holds a `Weak<AppCell>` to avoid leaking the app through cyclic references. Every method must upgrade this weak pointer and return `Result<T>` to handle the case where the app was dropped. This pushes error handling complexity to every callsite throughout the codebase.
-
-```rust
-// Current: Every call requires error handling
-cx.update(|app| { ... })?
-cx.update(|app| { ... }).ok();
-cx.update(|app| { ... }).unwrap();
-```
-
-## Solution
-
-Move the app-alive check into the executor's task execution path. Before running any task that needs the app, check if the app is still alive. If not, cancel the task by dropping its runnable. This guarantees that any code inside a foreground task only executes when the app is alive, allowing us to remove the `Result` wrapper.
-
-## Technical Background
-
-### How async-task Works
-
-The `async-task` crate's `spawn()` returns a `(Runnable, Task)` pair:
-- `Runnable`: Executes the future via `runnable.run()`
-- `Task`: A handle to await the future's output
-- Dropping a `Runnable` cancels the task
-- The `schedule` closure determines where/when the runnable is dispatched
-
-### Current Executor Architecture
-
-Tasks are spawned with metadata and a schedule closure:
-```rust
-async_task::Builder::new()
- .metadata(RunnableMeta { location })
- .spawn(
- move |_| future,
- move |runnable| {
- dispatcher.dispatch(RunnableVariant::Meta(runnable), label, priority)
- },
- )
-```
-
-The dispatcher queues the runnable, and a trampoline function executes it:
-```rust
-extern "C" fn trampoline(runnable: *mut c_void) {
- let task = unsafe { Runnable::<RunnableMeta>::from_raw(...) };
- task.run(); // <-- We add the check BEFORE this
-}
-```
-
-### Why This Is Safe
-
-1. **Foreground tasks run on the main thread.** The App also lives on the main thread. While a task runs synchronously, nothing else on the main thread can run, so the App cannot be dropped mid-execution.
-
-2. **The check happens before each poll.** Between await points, the task yields. When rescheduled, the trampoline checks the app status again. If dead, the task is cancelled before user code runs.
-
-3. **Recursive cancellation handles nested awaits.** If an outer foreground task awaits an inner foreground task and the app dies:
- - Inner task scheduled → trampoline sees app dead → cancels (drops runnable)
- - Outer task woken → scheduled → trampoline sees app dead → cancels
- - Neither task's code runs after app death → no panic
-
-4. **Background tasks cannot hold AsyncApp.** `AsyncApp` contains `Weak<Rc<...>>` which is `!Send`, so it cannot be moved into a `Send` future on a background thread.
-
-### The One Edge Case
-
-A background task (no app check) awaiting a foreground task (has app check):
-- Foreground task cancelled when app dies
-- Background task's await sees cancellation → panic
-
-Solution: Provide `try_update()` returning `Option<T>` for these cases.
-
-## API Changes
-
-### AsyncApp Methods
-
-Rename existing methods and add non-Result versions:
-
-| Old Signature | New Signature |
-|--------------|---------------|
-| `fn update(...) -> Result<R>` | `fn update(...) -> R` (panics if app gone) |
-| (new) | `fn try_update(...) -> Option<R>` (returns None if app gone) |
-
-Apply the same pattern to:
-- `update`
-- `read_entity` / `update_entity`
-- `read_global` / `update_global`
-- `read_window` / `update_window`
-- `new` / `reserve_entity` / `insert_entity`
-- `refresh`
-- `open_window`
-- `subscribe`
-- `has_global`
-
-### AppContext Trait
-
-Remove the `Result<T>` associated type entirely. All context types will now return values directly:
-
-```rust
-// Before
-pub trait AppContext {
- type Result<T>;
- fn new<T>(...) -> Self::Result<Entity<T>>;
- fn update_entity<T, R>(...) -> Self::Result<R>;
- // etc.
-}
-
-impl AppContext for AsyncApp {
- type Result<T> = Result<T>;
- // ...
-}
-
-// After
-pub trait AppContext {
- fn new<T>(...) -> Entity<T>;
- fn update_entity<T, R>(...) -> R;
- // etc.
-}
-
-impl AppContext for AsyncApp {
- // All methods return values directly, panicking if app is gone
- // ...
-}
-```
-
-### Remove Flatten Trait
-
-The `Flatten` trait in `gpui.rs` exists solely to handle `Result<Result<T>>` when `WeakEntity` (returns `Result`) is used with `AsyncApp` (also returns `Result`). With this change:
-- `AsyncApp` methods return `T` directly
-- `WeakEntity::update` etc. return `Result<T>` (only for the weak upgrade)
-- No `Result<Result<T>>` case exists, so `Flatten` can be removed
-
-### RunnableMeta
-
-Add optional app weak pointer using a `MainThreadWeak<T>` newtype that unsafely implements `Send + Sync`:
-
-```rust
-/// Weak reference that can cross thread boundaries but must only be accessed on the main thread.
-/// SAFETY: Only create, access (upgrade), and drop on the main thread.
-pub struct MainThreadWeak<T>(Weak<T>);
-unsafe impl<T> Send for MainThreadWeak<T> {}
-unsafe impl<T> Sync for MainThreadWeak<T> {}
-
-pub struct RunnableMeta {
- pub location: &'static Location<'static>,
- pub app: Option<MainThreadWeak<AppCell>>, // NEW
-}
-```
-
-**Safety invariants:**
-- `app` is `Some` only for foreground tasks (spawned via `ForegroundExecutor::spawn_with_app`)
-- `ForegroundExecutor` is `!Send`, so spawn only happens on main thread
-- Trampolines run on main thread, so `upgrade()` is main-thread only
-- Background tasks always use `app: None`
-
-## Implementation Phases
-
-### Phase 1: Add Trampoline Check Infrastructure (macOS + Test)
-
-Start with macOS and Test dispatcher to validate the approach.
-
-1. Add `MainThreadWeak<T>` newtype to `crates/gpui/src/platform.rs` with unsafe `Send + Sync` impls
-2. Update `RunnableMeta` to use `Option<MainThreadWeak<AppCell>>` for app field
-3. Update Mac dispatcher trampoline to check `unsafe { app.upgrade() }` before `run()`:
- - `crates/gpui/src/platform/mac/dispatcher.rs`
-4. Update Test dispatcher `tick()` to check `unsafe { app.upgrade() }` before `run()`:
- - `crates/gpui/src/platform/test/dispatcher.rs`
-5. Add `ForegroundExecutor::spawn_with_app` that wraps `Weak<AppCell>` in `MainThreadWeak`
-6. Update `AsyncApp::spawn` and `AsyncWindowContext::spawn` to use `spawn4. Modify spawn paths in `AsyncApp` to populate the app weak pointer in metadata
-5. Write tests to validate cancellation behavior on macOS
-
-### Phase 2: Extend to Other Platforms
-
-After validating on macOS:
-
-1. Update trampoline function in Linux dispatcher:
- - `crates/gpui/src/platform/linux/dispatcher.rs`
-2. Update trampoline function in Windows dispatcher:
- - `crates/gpui/src/platform/windows/dispatcher.rs`
-
-### Phase 3: Update AsyncApp API & Remove AppContext::Result
-
-1. Rename `update() -> Result<R>` to `try_update() -> Option<R>`
-2. Add new `update() -> R` that panics if app is gone
-3. Apply same pattern to all fallible methods
-4. Remove `type Result<T>` from `AppContext` trait
-5. Update all trait method signatures to return `R` directly
-6. Remove `Flatten` trait from `gpui.rs`
-7. Update `WeakEntity::update`, `WeakEntity::read_with`, etc. to remove `Flatten` bounds
-
-## Files to Modify
-
-### Core Changes
-- `crates/gpui/src/app/async_context.rs` - AsyncApp implementation
-- `crates/gpui/src/executor.rs` - RunnableMeta, spawn functions
-- `crates/gpui/src/platform.rs` - RunnableVariant if needed
-- `crates/gpui/src/gpui.rs` - AppContext trait (remove Result type), remove Flatten trait
-- `crates/gpui/src/app/entity_map.rs` - Update WeakEntity to remove Flatten usage
-
-### Dispatcher Changes
-- `crates/gpui/src/platform/mac/dispatcher.rs` - trampoline function (Phase 1)
-- `crates/gpui/src/platform/test/dispatcher.rs` - trampoline function (Phase 1)
-- `crates/gpui/src/platform/linux/dispatcher.rs` - trampoline function (Phase 2)
-- `crates/gpui/src/platform/windows/dispatcher.rs` - trampoline function (Phase 2)
-
-### External Crate Changes
-- `crates/eval/src/example.rs` - ExampleContext's AppContext impl (update to remove Result type)
-
-### Realtime Tasks (No Changes Needed)
-Realtime/audio tasks use a separate code path with their own thread and channel. They don't use `AsyncApp` and won't be affected.
-
-## Testing Strategy
-
-### Unit Tests (`crates/gpui/src/executor.rs`)
-
-**`test_task_cancelled_when_app_dropped`**
-Verifies the core mechanism: tasks don't execute after the app is gone.
-
-Test spec:
-```
-spawn task that sets flag on completion
-run_until_parked (task starts, blocks on channel)
-quit app
-assert flag was never set
-```
-
-**`test_nested_tasks_both_cancel`**
-Validates recursive cancellation: awaiting a cancelled task doesn't panic when both have app checks.
-
-Test spec:
-```
-spawn outer task that awaits inner task
-both tasks set flags if they run after quit
-quit app
-assert neither flag was set (no panic from awaiting cancelled inner)
-```
-
-**`test_try_update_returns_none_when_app_gone`**
-Confirms the fallback API works for edge cases like background-awaits-foreground.
-
-Test spec:
-```
-get async_cx, quit app
-call try_update
-assert returns None
-```
-
-### GPUI Example (`crates/gpui/examples/async_cancellation.rs`)
-
-Interactive demo for manual validation of cancellation behavior.
-```
-window with "Spawn Task" button
-each task runs 10-second timer, updating status each second
-closing window cancels pending tasks (no panic, no zombie tasks)
-```
-
-Run with: `cargo run -p gpui --example async_cancellation`
-
-### Integration Tests (`crates/gpui/src/app/async_context.rs`)
-
-**`test_spawn_executes_when_app_alive`**
-Basic spawn/await still works.
-
-**`test_update_entity_works`**
-Entity operations via AsyncApp still work.
-
-### Validation Checklist
-
-1. `cargo test -p gpui --features test-support` passes
-2. `cargo run -p gpui --example async_cancellation` works correctly
-3. `cargo clippy -p gpui` passes (ignore pre-existing warnings)
-4. Manual test: close windows with pending tasks, verify no panics
-
-
-## Future Work (Separate Brief)
-
-The following phases will be addressed in a separate brief after this work is validated:
-
-1. **Audit Cross-Boundary Awaits** - Search for patterns where background code awaits foreground tasks and migrate to `try_update()`
-2. **Codebase Migration** - Update all ~500+ callsites to remove `.unwrap()`, `?`, `.ok()` from `update()` calls
-3. **Cleanup** - Remove dead error handling code, update documentation