[Kyber5323]

What

S5 of circus run #377: the WorkflowRun Workspace release is recorded as its own durable fact after the terminal status, and a Workspace that never confirms deletion is given up on a bounded window instead of keeping the run unsettled forever.

Release states and terminal intent (migration 0055_workflow_run_workspace_release.sql). workflow_runs gains workspace_release_state (pending, releasing, released, abandoned, not_required), workspace_release_error, and workspace_released_at. The backfill records only evidence: a terminal run with no Workspace is not_required; one whose Workspace already carries deleted_at is released with that instant; everything else stays pending. CHECK constraints make released require an instant and abandoned require a non-blank reason, so a row cannot claim a deletion that was never observed. When the run reaches succeeded, failed, or cancelled, the same transaction that writes the terminal status declares the intent: no Workspace settles not_required; an already-deleted Workspace settles released; otherwise the row moves pending → releasing and dispatches the delete operation.

Delete completion writes released. The external-effect completion hook now resolves workflow-run Workspaces too (it previously only read issue_run_id). A successful delete_workspace settles the run released with the completion instant in the same transaction; a completion for a non-terminal run is refused, matching the rule that only terminal runs release. released is never inferred from a dispatch or a request — only from an observed deletion.

Recovery and bounded give-up. A periodic pass heals pending/releasing rows whose Workspace was deleted through another observed path (taking workspaces.deleted_at as the instant) and re-declares deletes for terminal runs that still have a live Workspace and no live delete operation. Give-up reuses the IssueRun D8 condition through one shared predicate: a live sandbox whose Node has been silent past issue_runs.delivery_unreachable_after. Such a run moves releasing → abandoned with reason workspace_unavailable, its unfinished delete operation is marked failed/node_unavailable, and the Workspace row is kept — the run no longer pretends the Workspace is gone, and no longer retries forever. With the window unset (≤ 0) the pass never gives up, so recovery keeps retrying.

Contract

• No public API or OpenAPI change; the release fact is internal scheduling state.
• Migration 0055 is additive to the sequence; both migration READMEs are updated. The upgrade path preserves existing rows and workspace rows.
• main moved past the test(scheduled-task): pin deadline boundaries and the timeout projection #183 freeze: cloud#81 merged as a9c5f20 (occupying 0053/0054) and cloud#118 merged as 4128d86. This branch is rebased onto 4128d86, and its migration is 0055 (immediately after 0054); the test constants and both migration READMEs are synced.
• Rebased onto main 4128d86 (which includes the S4 merge 9814cac and cloud#118's node-unknown window change), so this PR carries only the S5 commit. Migrations through 0054 are already on main; this slice uses 0055.
• The root ADR remains approved with the revised D1 ②/D4 (ora-space/specs#165, merge commit 911c976c); its transition to implemented belongs to the specs run that follows the remaining landing-order steps, not to this slice.
• task test:race is required for this change because it alters a persistence lifecycle and the scheduler's concurrent passes.

Tests

• internal/core/workflow_run_release_db_test.go: TestWorkflowRunCleanupRecordsTheReleaseIntent, TestWorkflowRunDeleteCompletionRecordsReleased, TestWorkflowRunDeleteCompletionRefusesANonTerminalRun, TestWorkflowRunWithoutWorkspaceRecordsNotRequired, TestWorkflowRunGiveUpAbandonsAnUnreachableWorkspace, TestWorkflowRunReleaseHoldsWithinTheWindowAndWithoutOne, TestWorkflowRunReleaseDoesNotAbandonATerminatedSandbox, TestWorkflowRunReleaseHealsAWorkspaceDeletedByAnotherPath on real PostgreSQL.
• integration/migration_upgrade_path_test.go: TestMigration0055WorkflowRunWorkspaceReleaseAppliesFreshAndUpgrades — fresh database, upgrade from 0054 with one run of each release shape, evidence-only backfill, workspace rows untouched, repeated application, and CHECK refusals.
• Existing workflow scheduler and workspace lifecycle tests were adapted to the new terminal-transaction release fact (one additional version bump where the fact lands).

Local gates with REQUIRE_POSTGRES=1 on the rebased head 609d5b3: checkformat, go build ./..., golangci-lint (0 issues), the full ./internal/core suite (372s), and TestMigration0055WorkflowRunWorkspaceReleaseAppliesFreshAndUpgrades + TestRevisionUpgradePreservesPublishedWorkflowSchema are green. On the pre-rebase tree, task format:check, task lint, task build, go test ./internal/core -run TestWorkflow, the full ./internal/core suite, and the full ./integration suite were also green. Under -race on that tree, ./internal/core and every other package were green with no race reports; ./integration was green when given a 30m timeout — the default 10m per-binary timer expires first on this machine, and the timed-out run showed the timer panic, not an assertion failure or a race report.

Refs ora-space/circus#377

🤖 Generated with Claude Code

[zhangshilong123]

Request changes: three stale references left by the 0053 → 0055 renumbering

I reviewed head 8ae7cf77 against main a9c5f20 (after #183 unfroze main and took 0053/0054). The S5 behaviour itself reads correctly; what is out of date is the paperwork around the renumber. All three are one-line fixes.

1. Migration file count is wrong in both READMEs. internal/core/migrations/README.md:9 and README.en.md:9 now say 56, but the branch contains 57 SQL files: main already carried 56 after #183 (0053_retire_tenant_level_clones.sql, 0054_git_delivery_refs.sql), and this PR adds 0055_workflow_run_workspace_release.sql. Verified with git ls-tree -r pr-119 internal/core/migrations | grep -c '\.sql$' → 57. (Main's own count was already stale at 54; this PR touches the line, so it should land the true number.)
2. Stale migration number in a source comment. internal/core/agent_run_release.go:281 says "(settleWorkflowRunWorkspaceDeleted, 0053)" — the migration is 0055 now.
3. PR description still describes the pre-renumber state. The What/Contract sections name migration 0053, quote head 70a5d58 and the freeze notice. It should name 0055, the current head, and note that main was unfrozen and 0053/0054 were taken by #183.

What I verified and consider sound

• 0055: additive columns and the two derived-state CHECKs; backfill records only evidence (not_required/released with the Workspace's own deleted_at/releasing/pending), never stamps the migration clock, and leaves version untouched. workspaces.workflow_run_id is UNIQUE (0039), so the self-join backfill is deterministic. The upgrade test covers fresh, upgrade from 0054 with one row per release shape, repeated application, untouched Workspace rows and all three CHECK refusals.
• Writers: the terminal transaction declares the intent (releasing/not_required) and the delete; the delete-completion hook writes released in the same transaction that soft-deletes the Workspace and fails closed for a non-terminal run; give-up fails the unfinished operation failed/node_unavailable, writes abandoned with access to the Workspace row and no release instant; replay and repeated reconciliation are no-ops. control.go now resolves the XOR binding (0039's workspace_run_binding_exclusive) and still settles create_workspace only for IssueRuns.
• Bound: runWorkspaceReleaseGivenUp is the extracted shared predicate, so the WorkflowRun pass and IssueRun D8 cannot drift; unconfigured window fails closed; a terminated sandbox is not abandoned; the hold-within-window and second-pass cases are pinned on real PostgreSQL.
• Docs: the new paragraph in docs/workflows/workflows{,.en}.md matches the implementation, including the 30-minute default.
• ADR status: specs main 911c976c carries the revised D1 ②/D4, and the PR correctly leaves the root ADR approved.

CI check is still running on 8ae7cf77; please push the fixes and request the re-review once the new head's CI is green.

[zhangshilong123]

Request changes: one rebase left before merge (content approved)

The three items from the previous review are fixed at f89057e: both migration READMEs now say 57 (and the branch really has 57 SQL files), agent_run_release.go:281 cites 0055, and the PR description is on 0055/a9c5f20 with the #183 renumbering recorded. I re-read the S5 diff at this head and found nothing else; the behaviour review from the previous round still stands.

The remaining item is the rebase the owner asked for after main moved again:

• main is now 4128d86, which merged cloud#118 ("honor node unknown window after disconnect") and changed the very predicate this PR delegates to (runWorkspaceNodeUnknown, dropped connection_state='connected').
• The green check does not cover that combination. It ran 03:05:28Z–03:18:54Z, and its pull_request merge ref was built when the run started, before #118 merged at 03:11:33Z; the check-run API shows a single run for f89057e, so main@4128d86 + S5 has never been built or tested.
• I do not see a semantic conflict: S5 calls the shared runWorkspaceReleaseGivenUp/runWorkspaceNodeUnknown rather than re-implementing the judgement, and the release tests age last_seen_at, which both the old and the new definition classify as unknown. But the run's instruction is to re-run CI on a head caught up with the then-current main, and the merged tree is what should be green before the owner merges.

Please rebase f89057e onto 4128d86 (still one commit, migration stays 0055), push, and request the re-review once the new head's check is green. If the owner decides the merge ref is sufficient, this is the only open point.

[zhangshilong123]

Approve

Rebased review of 609d5b3 (parent 4128d86, single commit).

• The patch is unchanged by the rebase. git diff a9c5f20..f89057e and git diff 4128d86..609d5b3 differ only in hunk header offsets; every changed line is identical, so the S5 behaviour reviewed (and the 57-count / 0055 fixes) is exactly what lands.
• The head is on the current main. 609d5b3^ is 4128d86, which carries cloud#118's runWorkspaceNodeUnknown change. The green check on this SHA (03:31:49Z–03:43:01Z) therefore tested the tree that merges into main, which was the open point of the previous review. S5 only calls the shared predicate, and its release tests classify by last_seen_at, so the two changes compose as the author described.
• Migration state is consistent: 0055 follows 0054, both migration READMEs say 57 and the branch has 57 SQL files, the commit message and PR description both name 0055, and the PR carries only the S5 commit.
• The PR description records the root ADR as approved with the revised D1 ②/D4 (specs#165, 911c976c), and the ADR transition stays out of this slice.

The release transition, the bounded give-up and the no-false-release rule were verified in the previous round and are unchanged. Merge remains the owner's call.