Skip to content

fix(api): coordinate workspace deletion with queued and running turns - #424

Open
shuntianyifang wants to merge 13 commits into
theam:mainfrom
shuntianyifang:fix/workspace-deletion-turns
Open

shuntianyifang wants to merge 13 commits into
theam:mainfrom
shuntianyifang:fix/workspace-deletion-turns

Conversation

@shuntianyifang

@shuntianyifang shuntianyifang commented Sep 28, 2026 •

Copy link
Copy Markdown

Current validation (2026-10-20, 56b89bd)

  • Merged upstream main b9f1bac (v0.13.28/v0.13.29 runner security updates) without conflicts. All 584 tracked Linux verification files match this commit byte-for-byte; worktree clean.
  • Full pnpm verify passed on Linux / Node 24 / pnpm 11.28.2 / PostgreSQL 16: lint, typecheck, clean full build, 877 critical tests and 336 remaining tests (1,213 total), unused-reference checks, guards and all-severity dependency audit. No normal-suite skips. Audit initially retried a registry connection timeout, then passed with no unignored findings and 3 upstream-configured ignored high findings. No new audit exceptions were added.
  • Official v0.13.29 runner E2E passed: 13/13, no skips (70.82 seconds). Tested immutable image ghcr.io/theam/facility/runner@sha256:867396e83af4e446dfa86f50b44621378f351b1e2d8d557255fce3d7fadc5457. Coverage includes real queued/running-turn deletion, nested Docker/Compose, authenticated preview and browser artifacts, volume reattachment and backup/restore, and development-branch compute replacement followed by service-level deletion/restore denial.
  • Release image checks confirmed containerd 2.3.6, runc 1.4.3 built with Go 1.26.9, Chromium 155.0.8059.79 and libexpat1 2.9.0-1~deb13u1. Preview gateway SHA-256 matches repository source. This validates the official published image, not a local source rebuild or all architectures/Vercel.
  • Historical validation sections below are superseded by these current results. Upstream Actions approval and re-review still require maintainer action; no remote-green claim.

Previous validation (2026-10-16, c003d77)

  • Merged upstream main b052a79 (v0.13.26 dependency fixes and v0.13.27 development-branch support) in 3a9c871, without conflicts. Added c003d77 to extend the development-branch workspace regression through service-level permanent deletion and rejected restoration after deletion.
  • The regression creates a story branch from develop, preserves story commits and dirty files when the base advances and compute is replaced, then deletes compute/storage through StoryWorkspaceService and confirms the destroyed state and restore denial. Fixture configuration and post-replacement references match service requirements. Runs with both deterministic fake runtime and real Docker.
  • Full pnpm verify passed on Linux / Node 24 / pnpm 11.28.2 / PostgreSQL 16: lint, typecheck, clean full build, 877 critical tests, 335 remaining tests (1,212 total), unused-reference checks, guards and all-severity dependency audit. No normal-suite skips. Audit currently reports no unignored findings and 3 upstream-configured ignored high findings; this is policy compliance, not a claim of zero vulnerabilities. No new audit exceptions were added by this follow-up.
  • Full workspace E2E passed: 13/13, no skips, including the development-branch restore/delete regression with real Docker. Used the previously validated official runner digest ghcr.io/theam/facility/runner@sha256:9aad7af6b96b36c31f2e20806c19fea08e066af9d9aa412a961b4c3c5c1032a0; runner source, CLI lockfile and root package-manager declaration are unchanged by this upstream update. No v0.13.27 image promotion or new image-scan success is claimed.
  • All 584 tracked Linux verification source files match this commit byte-for-byte. Prior sections below describe earlier commits and are superseded by these current results. Upstream re-review and Actions approval still require maintainer action; no remote-green claim.

Previous validation (2026-10-07, 4d1d78f)

  • Merged upstream main 38ec556 (v0.13.25 / fix(images): refresh runner and distro security floors #449) without conflicts. Frozen install and supply-chain checks passed with pnpm 11.28.2; all 577 verification source files match this commit byte-for-byte.
  • Lint, typecheck (14 tasks), clean full build (8 tasks), 847 critical tests, 319 remaining tests, unused-reference checks and guards passed. No normal-suite skips.
  • Official production runner v0.13.25 passed all 12 workspace E2E tests, no skips (3 real Docker tests plus 9 deterministic journey tests). Tested the immutable release digest ghcr.io/theam/facility/runner@sha256:9aad7af6b96b36c31f2e20806c19fea08e066af9d9aa412a961b4c3c5c1032a0. Chromium 154.0.8037.92, Claude Code 2.1.284, Codex 0.144.6 and pinned pnpm 11.28.2 were checked; preview gateway hash matches source. A local source build was stopped during slow distro downloads once the official release image was available; local rebuild success is not claimed.
  • Full pnpm verify still exits nonzero at dependency audit. Today's report is 3 unignored findings: critical shell-quote (GHSA-pqg4-j6r4-53mv), high sharp (GHSA-wq5f-xc86-pv6w), and high @modelcontextprotocol/sdk (GHSA-6qxp-vccf-f47h). It separately reports 3 ignored high findings under upstream policy. Manifests, lockfile and audit policy match main; this PR adds no exceptions beyond the upstream merge. Earlier upstream CI success does not replace today's audit result.
  • Earlier validation sections below are historical. Re-review and upstream Actions approval still require maintainer action; no remote-green claim.

Previous validation (2026-10-05, e47d916)

  • Merged upstream main 446d1ee in f20e295 without conflicts. Added two regression cases in e47d916 for source-issue closure racing workspace deletion: suspension already in flight, and closure after deletion starts or completes. Both run through the GitHub mirror webhook path with a deterministic workspace runtime and real PostgreSQL 16.
  • The in-flight regression fails with sleeping instead of deleting when the issue reconciliation story lock is temporarily removed. Production source was restored and all 40 workspace lifecycle tests passed. Existing lifecycle protection handles the new upstream reconciliation path; no additional production-code change was needed.
  • Linux verification: lint, typecheck (14 tasks), clean build (8 tasks), 847 critical tests, 305 remaining tests, unused-reference checks and guards passed. All 571 tracked verification source files match this commit; no normal-suite skips.
  • Full production-runner workspace E2E passed: 12/12, no skips (3 real Docker tests and 9 deterministic journey tests). Reused the previously validated Linux amd64 runner because its source and CLI lockfile are unchanged by this upstream merge.
  • Full pnpm verify still exits nonzero at dependency audit. The prior unignored findings are gone; the current report is 4 high findings, including 2 existing ignored findings. The 2 unignored findings are http-cache-semantics (fixed in 4.2.1) and braces (fixed in 3.0.4), with docs-toolchain paths in this report. Lockfile and audit policy match upstream main. No audit exceptions were added.
  • Results below are historical evidence from earlier commits. Upstream Actions and re-review still require maintainer action; no remote-green claim.

Problem and change

Deleting a story workspace could leave queued turns claimable or destroy compute while a turn was running. Later dispatches could then create permanent turn_error attention for an already deleted workspace.

This change coordinates deletion with turn claims under the story lock and row locks. Deletion returns 409 workspace_busy while a turn is running, cancels queued turns in the transaction that marks the workspace deleting, and prevents new work or queued successors from starting during or after deletion. Late dispatch failures no longer create attention for deleted workspaces. Provider cleanup failures remain retryable. Startup now reacquires the same story lock, re-reads workspace state, and holds the lock through provider create/wake and success or failure recording, preventing a concurrent delete from being undone by a late startup.

The operator guide explains the running-turn rejection and retry behavior, including using a fresh idempotency key after a cached 409.

Closes #414.

Previous upstream synchronization (9fb9b9a)

  • Merged upstream main a37390f without conflicts, including pricing, Claude Code 2.1.284, OAuth consent and dependency updates. All 571 verification source files match this commit byte-for-byte.
  • Lint, typecheck, clean full build, all 840 critical tests and 305 remaining tests, unused-reference checks and guards passed on Linux / Node 24 / PostgreSQL 16. No test skips or weakened assertions.
  • Full pnpm verify is not green: its final dependency audit now reports 17 findings (8 moderate, 9 high, including 2 existing ignored findings). Dependency manifests, lockfile and audit policy match upstream main; this PR adds no audit exceptions. Earlier successful audit results below are historical.
  • Rebuilt the updated Linux amd64 production runner and passed all 12 workspace E2E tests (3 real Docker, 9 deterministic journey tests), no skips. Claude Code 2.1.284 and Codex 0.144.6 execute correctly; the preview script hash matches source. Build-only proxy/cache settings used lockfile-verified CLI tarballs; runtime source and dependency versions were unchanged.

Suspension/deletion review follow-up

  • ad66388 coordinates ordinary suspension with deletion using the shared story transaction lock, including the fresh workspace read, provider call, sleeping-state writeback and suspended event. An already deleting/destroyed workspace is left untouched.
  • Added two deterministic PostgreSQL integration tests: pause suspension while deletion starts and hold provider destruction open to verify the state stays deleting and new turn admission is rejected; also verify suspension never calls the provider after deletion begins or finishes. The first test fails on the previous implementation with sleeping instead of deleting.
  • Full pnpm test:critical passed without skips on a fresh PostgreSQL 16 database, including all 63 relevant lifecycle/dispatcher/unit/HTTP cases. API typecheck/build, lint and both guards passed. The production-runner workspace E2E passed again: 12/12, no skips.
  • The initial parallel focused run hit a dispatcher timeout, and the initial full run on the long-lived database failed a project-list assertion. The unchanged tests passed using the repository's per-file CI isolation and a fresh database; no assertions or timeouts were weakened.

Main synchronization

  • Merged upstream e1f1043 into the PR as 07ebf31 without conflicts. The merge brings only README/reference documentation and documentation-test changes; runtime code is unchanged.
  • At the merged head: 11 documentation tests, the documentation production build, 61 focused API regressions, repository lint and both guards passed. No tests were skipped. Full repository verification and production runner E2E remain the earlier evidence documented below; they were not repeated for this documentation-only merge.

Startup/deletion race follow-up

  • Three deterministic PostgreSQL 16 regressions cover deletion between admission and wake, deletion during an in-flight wake, and a failed wake racing deletion. All three fail against the previous PR head and pass with the fix.
  • Focused lifecycle, dispatcher, unit and HTTP suites: 61 passed; the full pnpm test:critical suite passed without skips. API typecheck/build, repository lint and guards also passed.
  • Both real Docker deletion tests passed again (the unrelated third test was excluded by the name filter).
  • The full repository verification and image results below describe earlier commits; this follow-up does not claim those full checks were rerun.

Validation

  • Full pnpm verify passed on Linux (Alpine / Node 24.18.1 / pnpm 11.20.0) using the standard Compose postgres:16-alpine image, PostgreSQL 16.15.
  • The verification checkout was matched against all 570 tracked files at PR commit d77c3de (normalizing line endings); no local-font changes were included.
  • Linux critical suite: 802 tests passed with no skips, including database, CLI and API tests.
  • Remaining uncached suite: 293 tests passed with no skips. Total: 1,095 tests (the focused tests below are included in that total).
  • Full dispatcher, workspace lifecycle, deletion unit tests and HTTP integration tests: 58 passed.
  • Six new HTTP tests cover successful deletion/replay, running-turn rejection, provider cleanup retry, malformed requests, invalid/expired/revoked/read-only credentials, and cross-project/cross-tenant access.
  • Concurrency regression covers a dispatcher claim racing deletion. Three new lifecycle regressions were also checked against the old implementation and failed as expected.
  • Clean full repository build: 8/8 tasks passed with no cache hits. Repository typecheck: 14/14 tasks passed with no cache hits. Lint, unused-reference checks and guards passed; migration and diff whitespace checks also passed.
  • Original next/font/google downloads worked through the existing host proxy. TURBO_ENV_MODE=loose was used for this verification invocation to pass HTTP_PROXY/HTTPS_PROXY through Turbo. No source or dependency changes were required.

Additional validation

  • Terraform 1.11.3: fmt, init (backend disabled), validate and the AWS mock-provider test passed; no cloud resources were created.
  • Two new Docker-backed deletion regressions passed on PostgreSQL 16: reject running-turn deletion without destroying compute/storage, and cancel queued work while deleting the actual container, volume and network. These are committed in 6a339dd and included in the existing gated workspace runtime test file.
  • These two tests used a small fixture derived from the same pinned Docker-in-Docker base, not a completed production runner image build.
  • The complete workspace test command also passed: 12/12 tests, no skips, using an isolated fixture image with real Docker-in-Docker, Chromium, the repository preview gateway and Debian static BusyBox. This covers nested Compose, screenshot/DOM/trace, cancellation, volume reattachment and backup/restore. Chromium required software rendering flags under WSL; this remains fixture validation, not proof of the exact production runner build.
  • API and Web self-host images built successfully with temporary proxy transport settings (host build networking and Node native environment proxy support). An isolated Compose smoke test passed migrations, API readiness, worker startup, and Web login HTTP 200. It used the fixture runner and loopback ports; no live external credentials.

Production runner validation

  • At de28bb7, the complete runner target built successfully for Linux amd64, including the checksum-pinned Go tools, compiler/module audit, Chromium and locked agent CLIs.
  • Full pnpm test:e2e-workspace: 12 passed, no skips, on PostgreSQL 16.15. Three tests use the real production runner (queued/running deletion and the persistent Docker workspace loop); nine cover the reference story journey with deterministic fakes.
  • The real runner loop covers nested Compose, authenticated preview, Chromium screenshot/DOM/trace, cancellation, compute replacement with volume reattachment, and backup/restore.
  • All 570 tracked verification files matched the PR's Git contents byte-for-byte. A temporary build-only Dockerfile added proxy/cache settings, limited Go parallelism and split the existing dependency stage for scheduling; source versions, installed tools and audit checks were unchanged. No test assertions or browser flags were changed.

Remaining verification

  • Previous font-download and PostgreSQL 16 verification blockers are resolved. The earlier exit 139 did not recur in the successful full verification run. WSL cores identify Turbo 2.10.2 as the original native crash; the Node wrappers propagated SIGSEGV to themselves. The binary is stripped, so the exact internal cause remains unknown.
  • Current dependency audit passed at c003d77 with 3 upstream-configured ignored high findings; see the current validation section. Historical failures below are superseded.
  • The Linux amd64 production runner build and local workspace E2E are now verified. The separate Vercel target/live Vercel lifecycle and other image architectures were not verified in this follow-up. Local results do not replace upstream CI approval and execution.
  • Upstream GitHub Actions are action_required for the fork and require maintainer approval. No remote green checks are claimed; this PR is awaiting re-review.

@shuntianyifang
shuntianyifang marked this pull request as ready for review September 29, 2026 09:43

@adrian-lorenzo adrian-lorenzo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for covering these races! There’s one left: an in-flight suspension can change deleting back to sleeping, allowing a new turn to queue while deletion is still running. Please coordinate suspension with deletion and add a regression test for that sequence.

This branch has not been deployed

No deployments
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.

deleteWorkspace leaves active and queued turns in limbo, generating spurious turn_error attention items

2 participants