Workspace cleanup - #787
Merged
Merged
Conversation
lazabogdan
force-pushed
the
ralphx/ralphx/agent-f2757b67
branch
from
July 18, 2026 00:17
6f40f13 to
0c3a007
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
lazabogdan
marked this pull request as ready for review
July 18, 2026 18:02
lazabogdan
enabled auto-merge (squash)
July 18, 2026 18:02
lazabogdan
disabled auto-merge
July 18, 2026 18:18
lazabogdan
enabled auto-merge (squash)
July 18, 2026 18:26
lazabogdan
disabled auto-merge
July 18, 2026 18:35
lazabogdan
enabled auto-merge (squash)
July 18, 2026 18:42
lazabogdan
disabled auto-merge
July 18, 2026 19:11
lazabogdan
disabled auto-merge
July 19, 2026 01:20
lazabogdan
enabled auto-merge (squash)
July 19, 2026 01:29
lazabogdan
disabled auto-merge
July 19, 2026 01:57
lazabogdan
enabled auto-merge (squash)
July 19, 2026 01:58
lazabogdan
disabled auto-merge
July 19, 2026 02:31
lazabogdan
enabled auto-merge (squash)
July 19, 2026 02:37
lazabogdan
disabled auto-merge
July 19, 2026 02:41
lazabogdan
enabled auto-merge (squash)
July 19, 2026 02:44
lazabogdan
disabled auto-merge
July 19, 2026 02:59
lazabogdan
enabled auto-merge (squash)
July 19, 2026 03:01
lazabogdan
disabled auto-merge
July 19, 2026 03:13
lazabogdan
enabled auto-merge (squash)
July 19, 2026 03:15
lazabogdan
disabled auto-merge
July 19, 2026 04:06
lazabogdan
enabled auto-merge (squash)
July 19, 2026 04:07
reefagent
pushed a commit
that referenced
this pull request
Jul 22, 2026
Main's #787 workspace-cleanup rewrite made terminal cleanup bail with branch_not_ralphx_owned for any branch not matching the ralphx/<slug>/ naming, skipping worktree removal entirely. Strict ClickUp ticket workspaces use the provider's branch convention (e.g. eng-42_ticket_ada), so after that change merged into this branch the strict startup cleanup and next-cycle prepare paths could no longer release their worktrees. Add a strict-managed cleanup variant that removes the terminal worktree while preserving the canonical ticket branch, and route the strict prepare and startup paths through it. Generic (non-strict) cleanup is unchanged and still preserves non-RalphX branches by skipping removal. Fixes the two Rust lib tests failing on PR #774 (Shard 2/4): - later_conversation_prepares_next_generation_only_after_clean_terminal_release - startup_terminal_workspace_cleanup_removes_worktree_but_preserves_strict_ticket_branch
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.
Summary
User Impact
Technical Context
archive_agent_conversation_for_statenow returns a terminal workspace outcome that includes runtime shutdown status, cleanup claim state, and local cleanup result.archiveConversationin the frontend now parses a structured archive response with nestedconversationandcleanupfields, and the React action layer maps cleanup states to warning toasts.Risks / Follow-Ups
View full plan
Goal
Implement one reliable terminal-workspace cleanup lifecycle that runs during the current app process for merged PRs, remotely closed PRs, explicit PR closure, and conversation archive. “Cleaned up” means the verified RalphX-owned local artifact target is force-removed, including uncommitted and ignored build/test artifacts, and the verified RalphX-owned local branch for that target is force-deleted. For direct workspace PRs this target is the agent workspace worktree/branch; for linked ideation plan PRs it is the linked plan-branch worktree/branch, not an unrelated conversation row path.
Assumptions:
Repository Evidence
src-tauri/src/application/services/pr_merge_poller.rs.pr_merge_poller.rsand has no cross-entry single-flight guard.src-tauri/src/application/git_artifact_cleanup.rsvalidates the expected workspace path, refuses the project root, removes only clean worktrees, and deletes branches only for merged/content-equivalent cases.GitService::delete_worktreealready uses double-force removal plus a path-safe fallback, andGitService::delete_branch(..., true)already performsgit branch -D; no new raw Git subprocess path is needed.archive_agent_conversation_for_stateandclose_agent_workspace_pr_for_stateupdate terminal state but do not invoke local cleanup.RunningAgentInfo.worktree_pathprovides the established evidence needed to replace the global pause with candidate-local busy checks.Architecture
1. Extract one terminal cleanup coordinator
Mechanically extract the existing
terminalize_agent_workspace_after_prandcleanup_terminal_agent_workspace_after_prbehavior fromapplication/services/pr_merge_poller.rsintoapplication/agent_workspace_terminal_cleanup.rs. Register it throughapplication/mod.rs; do not leave a forwarding copy or parallel implementation.The coordinator accepts a typed terminal cause:
MergedPrClosedPrArchivedConversationIt returns a typed outcome separating:
claimed,already_in_progress,already_cleaned);cleaned,failed_unsafe,failed_operational).Runtime shutdown failure remains blocking. Filesystem cleanup failure becomes durable and retryable; it must not reactivate the workspace or let another workflow continue as if the PR were open.
2. Establish terminal authority before destructive effects
Each entry path must persist or verify terminal authority before cleanup:
publication_pr_status=merged|closed.closed.status=archived; whether the remote PR remains open is controlled by the existing checkbox.After authority is persisted, the coordinator stops the target conversation runtime, fails any still-active run with the terminal reason, verifies no active run remains, kills lingering processes whose CWD is the verified worktree using the established worktree-process cleanup seam, then performs local deletion.
Do not delete first and attempt to persist terminal state afterward.
3. Add durable single-flight cleanup state
Extend
AgentConversationWorkspaceRepositorywith an atomic claim operation backed by SQLite compare-and-set and mirrored by the memory repository:pending,failed, or stalecleaning.local_cleanup_status='cleaning'andlocal_cleanup_checked_at=now; exactly one caller succeeds.cleaned.failed_unsafeorfailed_operational.cleaningorcleanedperform no Git operations and cannot overwrite the terminal marker withbranch_missing.Reuse the existing columns; no schema migration is required. Update terminal-candidate queries so stale
cleaning, both failure states, and legacyunsafe|target_ref_missing|workspace_dirtymarkers are retryable. Treat an already-missing verified worktree and branch as successfulcleaned, making crash recovery idempotent.4. Resolve and validate the exact local artifact target
Add a typed cleanup target resolver before any destructive Git operation:
DirectAgentWorkspacefor direct edit/review workspace PRs, backed by the conversation workspace worktree and expected agent-conversation branch.LinkedPlanBranchfor ideation/plan PRs backed by a linkedPlanBranch, using the plan branch worktree and plan branch name.Do not route a linked plan-branch PR through the direct agent-workspace branch guard; that can either skip the real plan branch or delete the wrong local target.
Refactor
git_artifact_cleanup.rsto expose force-owned terminal cleanup for both target kinds:git check-ref-format.After these checks, skip the current dirty-worktree and merged/content-equivalence gates, call the existing forced worktree deletion, and force-delete the local branch when present. Missing worktree/branch components count as idempotent success. Any path/ownership mismatch fails closed and is persisted as
failed_unsafe.Keep non-terminal plan-branch cleanup and unknown-orphan cleanup on their existing conservative containment policy.
5. Wire every terminal entry path
Route these callers through the coordinator:
Each caller supplies or reloads enough state for the coordinator to resolve the cleanup target as direct workspace vs linked plan branch. Remove caller-local cleanup decisions such as “delete branch only when merged” or “skip ideation plan targets.” The typed cause is for state/audit semantics; the selected product policy force-deletes the verified local branch for all terminal causes after the correct target is resolved.
6. Make fallback cleanup progress during the same process
Keep the periodic terminal sweep as crash/error recovery, but replace the global “any agent running” pause with a candidate-local check:
RunningAgentInfo.worktree_path;Add a periodic orphan cleanup loop with a runtime-configured cadence. For each orphan candidate:
Do not couple orphan cleanup to the terminal cleanup CAS because there is no workspace row to claim.
Affected Files
Backend application flow
src-tauri/src/application/agent_workspace_terminal_cleanup.rs— mechanically extracted coordinator, typed causes/outcomes, runtime shutdown, CAS claim, process teardown, and cleanup orchestration.src-tauri/src/application/agent_workspace_terminal_cleanup_tests.rs— production-entry behavioral tests; no inline test module.src-tauri/src/application/mod.rs— module/test registration.src-tauri/src/application/services/pr_merge_poller.rsand its test surface — replace embedded terminalization with coordinator calls; mechanically move only the relevant existing tests/helpers.src-tauri/src/application/agent_workspace_external_pr_reconciliation.rssrc-tauri/src/application/agent_workspace_pr_supervision_recovery.rssrc-tauri/src/application/automation/merged_run_finalizer.rssrc-tauri/src/application/agent_conversation_archive.rs— archive and explicit-close integration, typed archive result, and direct-vs-linked-plan cleanup target resolution.src-tauri/src/application/git_artifact_cleanup.rsandgit_artifact_cleanup_tests.rs— force-owned terminal policy for direct workspace and linked plan-branch targets, plus path/branch guards.src-tauri/src/application/orphan_worktree_cleanup.rsandorphan_worktree_cleanup_tests.rs— candidate-local busy guard and periodic runner.src-tauri/src/application/startup_pipeline.rs— start the periodic orphan runner alongside terminal recovery.src-tauri/src/infrastructure/agents/claude/agent_config/runtime_config.rs, its tests, andconfig/ralphx.yaml— orphan cleanup interval.Persistence
src-tauri/crates/ralphx-domain/src/repositories/agent_conversation_workspace_repository.rsand tests — atomic claim/finalize contract.src-tauri/src/infrastructure/sqlite/sqlite_agent_conversation_workspace_repo.rsand tests — SQLite CAS and retry-candidate query.src-tauri/src/infrastructure/memory/memory_agent_conversation_workspace_repo.rsand tests — behaviorally equivalent test implementation.IPC and frontend
src-tauri/src/commands/unified_chat_commands/**— return an archive result that distinguishes logical archive success from cleanup outcome.frontend/src/api/chat.ts— zod snake_case response schema and camelCase transform for the archive result.frontend/src/components/agents/ArchiveConversationDialog.tsx— replace restart wording with an explicit irreversible local deletion warning covering uncommitted and ignored artifacts.frontend/src/components/agents/BulkArchiveConversationControls.tsx,bulkConversationArchive.ts, anduseAgentConversationActions.ts— allow bulk archive to use archive-as-local-terminal-authority withclosePullRequest=false, and show equivalent destructive warning/copy.Data / State
No migration is planned because
local_cleanup_statusandlocal_cleanup_checked_atare nullable text/timestamp columns.State progression:
terminal authority persisted -> cleaning (CAS) -> cleaned | failed_unsafe | failed_operationalThe CAS is keyed by the retained conversation workspace row even when the resolved local artifact target is a linked plan branch.
Recovery:
cleaning-> eligible for retry after the configured terminal retry window;failed_operational-> retry periodically;failed_unsafe-> retry only after metadata changes or the retry window, while continuing to fail closed;workspace_dirty|unsafe|target_ref_missing-> eligible under the new force-owned policy;cleaned.The logical workspace row remains for history/UI. Cleanup must not delete conversation or workspace records.
Agent And MCP Surface
UI / UX
cleaned, show the normal archive success behavior.closePullRequest=false, warn that local workspace artifacts will be deleted, leave remote PRs open by default, and summarize cleanup failures without restoring successfully archived rows.Progression Scenarios
Constraints
remove_dir_allor direct Git command in orchestration code.DbConnection::run/transactional SQLite paths; CAS must be atomic.Avoid
cleanedwithbranch_missing.Proof Obligations
cleanedoutcome.Decisions
Testing Strategy
Write failing tests before production changes:
Backend behavioral tests
cleaned, notbranch_missing.cleaningandfailed_operationalcandidates retry; already-missing artifacts converge to cleaned.Frontend tests
Validation commands
cargo nextest run --manifest-path src-tauri/Cargo.toml --lib --features test-utilsfor the broader backend library surface.rustfmt --edition 2021 --checkon every touched Rust leaf file only.python3 scripts/check-layering.py.cd frontend && npm run test:run -- <touched test files>.npm runscripts.HEAD, then run the stateful false-success lens: stale claim, read failure, duplicate event, event ordering, and path sink checks.Generated by RalphX