fix(tasks): refresh open windows when a task is archived headlessly#2514
fix(tasks): refresh open windows when a task is archived headlessly#2514dfox288 wants to merge 1 commit into
Conversation
Greptile SummaryHeadless task archives (from automations or inbound MCP) previously left open renderer windows showing stale, still-active tasks because the optimistic store mutation only ran on the renderer-initiated path. This PR closes the gap by emitting a
Confidence Score: 4/5Safe to merge; the fix is well-scoped and the idempotency guard protects existing GUI-initiated archives from being double-processed. The core emission and subscription logic is correct and consistent with every other IPC event pattern in this codebase. The two non-blocking observations are: a missing test for the archive-op-throws path (an error there would let events.emit fire for a task that is still alive), and a brief intermediate MobX state between the two runInAction blocks in the event handler. Neither is currently reachable as a runtime defect given the surrounding guards, but both are worth addressing before the pattern gets copied elsewhere. The test file would benefit from an additional case covering the failure path. The renderer store handler in task-manager.ts is the only place where the two-runInAction pattern is worth a second look.
|
| Filename | Overview |
|---|---|
| apps/emdash-desktop/src/shared/core/tasks/taskEvents.ts | Adds taskArchivedChannel event definition with { taskId, projectId } payload; follows existing channel pattern exactly. |
| apps/emdash-desktop/src/main/core/tasks/task-service.ts | Emits taskArchivedChannel after awaited archive operation; event is correctly gated behind the await so failures suppress the emission. |
| apps/emdash-desktop/src/renderer/features/tasks/stores/task-manager.ts | Adds taskArchivedChannel subscription with idempotency guard; two separate runInAction blocks create a brief intermediate observable state, and the unsubscribe is correctly wired in dispose(). |
| apps/emdash-desktop/src/main/core/tasks/task-service.test.ts | New unit tests verify emit order and correct payload; missing coverage for the error path (archive op throws → emit must not fire). |
Sequence Diagram
sequenceDiagram
participant A as Automation / MCP
participant TS as TaskService (main)
participant IPC as IPC events
participant TM as TaskManagerStore (renderer)
A->>TS: archiveTask(projectId, taskId)
TS->>TS: await archiveTask op (DB)
TS->>TS: callHookBackground task:archived
TS->>IPC: "emit taskArchivedChannel {taskId, projectId}"
IPC-->>TM: task:archived event
TM->>TM: guard: projectId match?
TM->>TM: "guard: isRegistered && !archivedAt?"
TM->>TM: "runInAction → task.data.archivedAt = now"
TM->>TM: _releaseTaskRegistries(taskId)
TM->>TM: runInAction → transitionToDryUnprovisioned
Note over TM: GUI-initiated path sets archivedAt optimistically<br/>before the RPC, so archivedAt guard short-circuits above
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/emdash-desktop/src/main/core/tasks/task-service.test.ts:42-67
**Missing test: event must NOT be emitted when archive operation throws**
The second test confirms ordering when both steps succeed, but there is no coverage for the failure case. If `archiveTask` throws (e.g. DB constraint violation), `events.emit` must stay silent — an emitted event would cause every open renderer window to hide the task even though it is still alive in the database. A three-line test that rejects the mock and then asserts `mockEmit` was not called would close this gap.
### Issue 2 of 2
apps/emdash-desktop/src/renderer/features/tasks/stores/task-manager.ts:157-168
**Brief MobX intermediate state between the two `runInAction` blocks**
After the first `runInAction` completes (`archivedAt` set), MobX flushes reactions. At that point the task is observable as "archived" (`archivedAt` is truthy) but `transitionToDryUnprovisioned` has not been called yet, so its store state (registered/provisioned) is unchanged. Any computed or reaction that reads both `archivedAt` and the task's phase will observe an inconsistent snapshot during the synchronous `_releaseTaskRegistries` call that sits between the two blocks. The GUI-initiated path has this same gap, but there an `await` makes it inevitable; here everything is synchronous, so the two `runInAction` calls (with `_releaseTaskRegistries` moved before them) can be collapsed into one to eliminate the window entirely.
Reviews (1): Last reviewed commit: "fix(tasks): refresh open windows when a ..." | Re-trigger Greptile
11b4cc7 to
45b6a00
Compare
The renderer only updated its task store optimistically inside its own archive flow, so archives initiated from the main process (e.g. the automations runtime) left a stale sidebar until a hard reload. taskService.archiveTask now emits a task:archived event and the task manager store mirrors the archive mutation on receipt, guarded idempotent so renderer-initiated archives are not double-processed.
45b6a00 to
d460a69
Compare
|
Thanks for the review. Both points addressed in the latest push:
|
Mirrors the changes made to PR generalaction#2514: collapse the headless archive handler into a single MobX action (no intermediate observable state), and add a test asserting no task:archived event is emitted when the archive operation throws.
Description
Archiving a task only refreshed the sidebar when the archive was initiated from the renderer. The store updated itself optimistically inside its own
archiveTaskflow, so any archive initiated from the main process — e.g. the automations runtime tearing a task down — left the open window showing a stale, still-active task until a hard reload.Fix:
taskService.archiveTasknow emits atask:archivedevent after the archive operation completes, and the task manager store subscribes and mirrors the same archive mutation it already performs for GUI-initiated archives (setarchivedAt, release the conversation/terminal registries, transition the task store to dry-unprovisioned). The handler is guarded idempotent (archivedAtalready set ⇒ no-op), so renderer-initiated archives that already updated optimistically are not double-processed.Implementation notes:
taskArchivedChannel(task:archived) inshared/core/tasks/taskEvents.ts, carrying{ taskId, projectId }.taskService.archiveTask.projectIdand is a no-op for unknown/unregistered/already-archived tasks.Related issues
None — found while building on top of the archive flow.
Testing
pnpm run typecheck— clean (on top of currentmain)pnpm run format:check— cleanpnpm run lint— clean on touched filespnpm exec vitest run src/main/core/tasks/task-service.test.ts— new test passes (asserts the op runs and the event is emitted, in that order)The renderer handler mirrors the existing optimistic mutation and was validated manually; there is no pre-existing unit-test harness for the task-manager MobX store, so I did not add one for that half.