Repository navigation
fix(api): coordinate workspace deletion with queued and running turns - #424
Open
shuntianyifang wants to merge 13 commits into
Open
shuntianyifang wants to merge 13 commits into
shuntianyifang wants to merge 13 commits into
Conversation
shuntianyifang
marked this pull request as ready for review
September 29, 2026 09:43
adrian-lorenzo
requested changes
Sep 29, 2026
adrian-lorenzo
left a comment
Member
There was a problem hiding this comment.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current validation (2026-10-20, 56b89bd)
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.pnpm verifypassed 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.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.Previous validation (2026-10-16, c003d77)
b052a79(v0.13.26 dependency fixes and v0.13.27 development-branch support) in3a9c871, without conflicts. Addedc003d77to extend the development-branch workspace regression through service-level permanent deletion and rejected restoration after deletion.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.pnpm verifypassed 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.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.Previous validation (2026-10-07, 4d1d78f)
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.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.pnpm verifystill exits nonzero at dependency audit. Today's report is 3 unignored findings: criticalshell-quote(GHSA-pqg4-j6r4-53mv), highsharp(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.Previous validation (2026-10-05, e47d916)
446d1eeinf20e295without conflicts. Added two regression cases ine47d916for 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.sleepinginstead ofdeletingwhen 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.pnpm verifystill 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 arehttp-cache-semantics(fixed in 4.2.1) andbraces(fixed in 3.0.4), with docs-toolchain paths in this report. Lockfile and audit policy match upstream main. No audit exceptions were added.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_errorattention for an already deleted workspace.This change coordinates deletion with turn claims under the story lock and row locks. Deletion returns
409 workspace_busywhile 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)
a37390fwithout conflicts, including pricing, Claude Code 2.1.284, OAuth consent and dependency updates. All 571 verification source files match this commit byte-for-byte.pnpm verifyis 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.Suspension/deletion review follow-up
ad66388coordinates 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.deletingand 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 withsleepinginstead ofdeleting.pnpm test:criticalpassed 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.Main synchronization
e1f1043into the PR as07ebf31without conflicts. The merge brings only README/reference documentation and documentation-test changes; runtime code is unchanged.Startup/deletion race follow-up
pnpm test:criticalsuite passed without skips. API typecheck/build, repository lint and guards also passed.Validation
pnpm verifypassed on Linux (Alpine / Node 24.18.1 / pnpm 11.20.0) using the standard Composepostgres:16-alpineimage, PostgreSQL 16.15.d77c3de(normalizing line endings); no local-font changes were included.next/font/googledownloads worked through the existing host proxy.TURBO_ENV_MODE=loosewas used for this verification invocation to passHTTP_PROXY/HTTPS_PROXYthrough Turbo. No source or dependency changes were required.Additional validation
6a339ddand included in the existing gated workspace runtime test file.Production runner validation
de28bb7, the completerunnertarget built successfully for Linux amd64, including the checksum-pinned Go tools, compiler/module audit, Chromium and locked agent CLIs.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.Remaining verification
c003d77with 3 upstream-configured ignored high findings; see the current validation section. Historical failures below are superseded.action_requiredfor the fork and require maintainer approval. No remote green checks are claimed; this PR is awaiting re-review.