Skip to content

fix(tasks): teardown archived and cold workspaces#2887

Open
rasitakyol wants to merge 2 commits into
generalaction:mainfrom
rasitakyol:agent/fix-archived-task-teardown
Open

fix(tasks): teardown archived and cold workspaces#2887
rasitakyol wants to merge 2 commits into
generalaction:mainfrom
rasitakyol:agent/fix-archived-task-teardown

Conversation

@rasitakyol

Copy link
Copy Markdown
Contributor

Description

Archive and cold-delete paths now run the same persisted resource teardown as live task deletion. The change separates lifecycle teardown from provider destruction, records each completed phase in the task row, and resumes only unfinished phases after restart.

This also makes provisioning clear stale teardown markers before acquiring workspace resources, keeps BYOI and missing-transport failures fail-closed, and guarantees registered workspace resources are released even when a teardown hook throws.

The schema change is additive and includes migration metadata, pre-migration/baseline/empty fixtures, migration coverage, and updated worktree lifecycle documentation.

Related issues

Fixes #2886.

Testing

  • pnpm --filter @emdash/emdash-desktop test (Node: 288 files / 2026 tests; main-db: 26 / 161; migrations: 8 / 21; browser: 3 / 37 with CI=1 --maxWorkers=1; scripts: 1 / 14)
  • Focused task/workspace teardown suites (15 files / 74 tests)
  • Legacy DB port suites (2 files / 10 tests)
  • pnpm --filter @emdash/emdash-desktop format:check
  • pnpm --filter @emdash/emdash-desktop lint
  • pnpm --filter @emdash/emdash-desktop typecheck
  • git diff --check

The repository-wide test target also reaches two pre-existing runtime session-manager assertion failures; both reproduce on upstream main at f6249ae56. The complete desktop test matrix above passes.

Screenshot/Recording (if applicable)

Not applicable; this changes background task lifecycle handling and persistence.

Checklist
  • I kept this PR small and focused
  • I ran a self-review before opening this PR
  • I ran the relevant local checks or explained why not
  • I updated docs when behavior or setup changed
  • I added or updated tests when behavior changed, or explained why not
  • I only added comments where the logic is not obvious
  • I used Conventional Commits for commit
    messages and, when possible, the PR title

@rasitakyol
rasitakyol marked this pull request as ready for review July 16, 2026 08:14
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends persisted resource teardown to archived and cold workspaces. The main changes are:

  • Adds durable lifecycle and provider-destroy completion markers.
  • Reopens stored workspaces for archive and delete cleanup.
  • Separates lifecycle teardown from provider destruction.
  • Clears stale teardown state before provisioning.
  • Adds migration fixtures and lifecycle coverage.

Confidence Score: 4/5

Concurrent provisioning and teardown can invalidate the workspace being set up.

  • Teardown calls are serialized with each other, but not with provisioning.
  • A delete or archive during setup can destroy newly acquired workspace resources.
  • Migration and cold-workspace teardown behavior otherwise appear consistent.

apps/emdash-desktop/src/main/core/tasks/operations/beginTaskProvisioning.ts, apps/emdash-desktop/src/main/core/tasks/task-service.ts, and apps/emdash-desktop/src/main/core/tasks/operations/teardownTaskResources.ts

Important Files Changed

Filename Overview
apps/emdash-desktop/src/main/core/tasks/operations/teardownTaskResources.ts Adds phase-aware live and cold teardown with in-flight teardown coalescing.
apps/emdash-desktop/src/main/core/tasks/operations/beginTaskProvisioning.ts Clears persisted and in-memory teardown state without excluding concurrent teardown.
apps/emdash-desktop/src/main/core/workspaces/teardown-stored-workspace.ts Reconstructs cold workspaces in teardown-only mode and fails closed for unavailable transports.
apps/emdash-desktop/src/main/core/workspaces/workspace-registry.ts Adds distinct archive and provider-destroy hooks with guaranteed resource release.
apps/emdash-desktop/src/main/db/schema.ts Adds nullable lifecycle and provider teardown timestamps to task rows.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant P as Provision
    participant DB as Task row
    participant W as Workspace setup
    participant D as Delete or archive
    participant T as Teardown
    P->>DB: Clear completion markers
    P->>W: Acquire and configure workspace
    D->>DB: Read cleared markers
    D->>T: Start cold teardown
    T->>W: Destroy workspace resources
    W-->>P: Setup fails or persists invalid state
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant P as Provision
    participant DB as Task row
    participant W as Workspace setup
    participant D as Delete or archive
    participant T as Teardown
    P->>DB: Clear completion markers
    P->>W: Acquire and configure workspace
    D->>DB: Read cleared markers
    D->>T: Start cold teardown
    T->>W: Destroy workspace resources
    W-->>P: Setup fails or persists invalid state
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/emdash-desktop/src/main/core/tasks/operations/beginTaskProvisioning.ts:11-15
**Provision Races With Teardown**

When delete or archive starts after these markers are cleared but before workspace setup finishes, teardown treats the task as cold and can destroy the workspace being provisioned. Setup then continues against released runtime resources or a removed worktree, leaving the task partially provisioned or failing with provider and filesystem errors.

Reviews (1): Last reviewed commit: "fix(tasks): teardown archived and cold w..." | Re-trigger Greptile

Comment on lines +11 to +15
await db
.update(tasks)
.set({ lifecycleTeardownAt: null, providerDestroyAt: null })
.where(eq(tasks.id, taskId));
clearTaskResourceTeardown(taskId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Provision Races With Teardown

When delete or archive starts after these markers are cleared but before workspace setup finishes, teardown treats the task as cold and can destroy the workspace being provisioned. Setup then continues against released runtime resources or a removed worktree, leaving the task partially provisioned or failing with provider and filesystem errors.

Context Used: AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/emdash-desktop/src/main/core/tasks/operations/beginTaskProvisioning.ts
Line: 11-15

Comment:
**Provision Races With Teardown**

When delete or archive starts after these markers are cleared but before workspace setup finishes, teardown treats the task as cold and can destroy the workspace being provisioned. Setup then continues against released runtime resources or a removed worktree, leaving the task partially provisioned or failing with provider and filesystem errors.

**Context Used:** AGENTS.md ([source](https://app.greptile.com/emdash/github/generalaction/emdash/-/custom-context?memory=c9624c9f-4bb0-42b3-aa19-4ea456b59255))

How can I resolve this? If you propose a fix, please make it concise.

@rasitakyol

Copy link
Copy Markdown
Contributor Author

Addressed the provisioning/teardown race in add38c969.

  • Added a task-scoped lifecycle mutex around provisioning, archive, delete, direct teardown, and restore.
  • Batch deletion now uses the same serialized path.
  • A queued provisioning attempt re-reads the task under the lock and refuses an already archived task.
  • Failed provisioning releases the lock so a later cleanup or retry can proceed.

Validation:

  • Desktop Node suite: 288 files / 2030 tests
  • Focused lifecycle suites: 6 files / 35 tests
  • Relevant main-db suite: 10 tests
  • Format, lint, typecheck, and git diff --check

The original thread remains anchored to beginTaskProvisioning, while the serialization boundary is implemented in TaskService.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: Archived tasks never run the teardown script

1 participant