mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
feat: environment delete with agent reassignment and consented sandbox destroy (#12053)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Environments define where agent runs execute: local, SSH, or provider sandboxes > - Operators can create and edit environments, but the UI has no way to delete one > - The server already exposes `DELETE /environments/:id` and a delete-blast-radius preflight, but no UI consumes them, and a delete blocked by reusable sandbox leases gives the operator no path forward > - This pull request adds the delete flow to the environment configuration page: a preflight-driven modal that reassigns dependent agents, names the workspaces that hold blocking sandbox leases, and can destroy those sandboxes with explicit consent > - The benefit is that operators can retire stale environments from the UI without database surgery, and dependent agents move to a chosen replacement instead of silently falling back ## Linked Issues or Issue Description Refs #8554 Refs #11124 **Subsystem affected** Environments (server routes, environment runtime service, and the environment settings UI). **Problem or motivation** The environment configuration page has no delete control. The server delete endpoint exists, but nothing in the UI calls it. When reusable sandbox leases block a delete, the 409 error names no owner, so the operator cannot find the blocking workspace. Agents that use the environment as their default lose it silently through the FK `on delete set null`. **Proposed solution** Add a delete button with a confirmation modal on the environment edit page. The modal reads the delete-blast-radius preflight. It offers a dropdown to reassign dependent agents to another environment before the delete. It lists each workspace that holds a blocking reusable sandbox lease, with a link. When those leases are the only blocker, the confirm button destroys the sandboxes inline (`?destroyReusableSandboxLeases=true`) and then deletes. A failed teardown falls back to `pending_cleanup` for the sweep, so no sandbox is orphaned. ## What Changed - `ui/src/pages/CompanyEnvironments.tsx`: delete button on the edit page header, confirmation modal with agent reassignment select, lease-holder list, impact notes, and a consent-labeled destroy-and-delete action - `ui/src/api/environments.ts`: `deleteBlastRadius` and `remove` client methods; `remove` takes an optional `destroyReusableSandboxLeases` flag - `server/src/routes/environments.ts`: `DELETE /environments/:id` accepts `?destroyReusableSandboxLeases=true`; it destroys the environment's reusable sandbox leases first, but only when those leases are the sole delete blocker, then re-checks the blast radius before it deletes - `server/src/services/environment-runtime.ts`: new `destroyReusableSandboxLeasesForEnvironment` — destroys every reusable sandbox lease an environment still owns while the environment config (provider credentials) is still available - `server/src/services/environments.ts`: the delete blast radius now returns `reusableSandboxLeaseHolders` (lease id, workspace, issue) so clients can name what blocks a delete - `packages/shared/src/types/environment.ts`: `EnvironmentDeleteReusableLeaseHolder` type on the blast radius - Tests: route gating for the consent flag (destroy runs, mixed-blocker rejection, surviving-lease rejection), runtime destroy scoped to an environment, blast-radius holder join, and UI tests for the reassignment flow, holder links, and the consent button ## Verification - `npx vitest run server/src/__tests__/environment-routes.test.ts server/src/__tests__/environment-service.test.ts server/src/__tests__/environment-runtime.test.ts ui/src/pages/CompanyEnvironments.test.tsx` - Manual: open Settings → Environments → edit an environment. The trash icon opens the modal. With agents on the environment, pick a reassignment target and confirm; agents move and the environment deletes. With reusable sandbox leases, the modal names the holding workspaces and the confirm button reads "Destroy N sandboxes and delete". ## Risks - The consented path destroys provider sandboxes. It runs only when reusable leases are the sole blocker, so a delete that would still be rejected never destroys anything. A failed teardown routes to `pending_cleanup` and the delete stays blocked until the sweep resolves it. - Agent reassignment issues one PATCH per agent from the client. A mid-sequence failure leaves some agents reassigned; the reassignments are valid on their own and the UI refreshes to the actual state. - Hard blockers (managed local, instance default, pending cleanup) keep the existing 409 behavior and disable the confirm button. ## Model Used - Claude (Anthropic) — Fable 5, model id `claude-fable-5`, extended thinking, agentic tool use via Claude Code CLI. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
1 parent
627eef7cbd
commit
c62bb4b16b
12 files changed
+1107
-9
No files matched your search
@@ -1,7 +1,7 @@
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { and, eq, inArray } from "drizzle-orm";
|
||||
import { and, eq, inArray, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { companySecrets, companySecretVersions, environmentLeases } from "@paperclipai/db";
|
||||
import { companySecrets, companySecretVersions, environmentLeases, heartbeatRuns } from "@paperclipai/db";
|
||||
import type {
|
||||
Environment,
|
||||
EnvironmentLease,
|
||||
@@ -3405,6 +3405,131 @@ export function environmentRuntimeService(
|
||||
return destroyed;
|
||||
},
|
||||
|
||||
/**
|
||||
* Destroy every reusable sandbox lease still owned by one environment, so a
|
||||
* consented environment delete can proceed. This must run while the
|
||||
* environment row still exists: the driver resolves provider credentials
|
||||
* from the environment config, and after the delete the normal destroy path
|
||||
* has no context left. A per-lease failure is contained — the driver routes
|
||||
* a failed teardown to `pending_cleanup` for the sweep, and an unexpected
|
||||
* throw leaves the lease in place — so the caller re-checks the blast
|
||||
* radius instead of trusting these counts for the delete decision.
|
||||
*/
|
||||
async destroyReusableSandboxLeasesForEnvironment(input: {
|
||||
environmentId: string;
|
||||
failureReason?: string;
|
||||
}): Promise<{ destroyed: number; failed: number; skippedLiveRun: number }> {
|
||||
const environment = await environmentsSvc.getById(input.environmentId);
|
||||
if (!environment) return { destroyed: 0, failed: 0, skippedLiveRun: 0 };
|
||||
const leaseRows = await db
|
||||
.select()
|
||||
.from(environmentLeases)
|
||||
.where(
|
||||
and(
|
||||
eq(environmentLeases.environmentId, input.environmentId),
|
||||
eq(environmentLeases.leasePolicy, "reuse_by_environment"),
|
||||
inArray(environmentLeases.status, ["active", "released", "retained"]),
|
||||
),
|
||||
);
|
||||
|
||||
// A lease whose holding run is still in flight keeps its sandbox: the
|
||||
// consented delete must not tear a live run's environment out from under
|
||||
// it. The skipped lease keeps blocking the delete, so the caller's
|
||||
// blast-radius re-check rejects and the operator retries after the run
|
||||
// finishes. A lease pointing at a finished run — or at no run — is a
|
||||
// stale reservation and destroys normally.
|
||||
const holdingRunIds = leaseRows
|
||||
.map((row) => row.heartbeatRunId)
|
||||
.filter((runId): runId is string => Boolean(runId));
|
||||
const liveRunIds = new Set<string>();
|
||||
if (holdingRunIds.length > 0) {
|
||||
const liveRuns = await db
|
||||
.select({ id: heartbeatRuns.id })
|
||||
.from(heartbeatRuns)
|
||||
.where(
|
||||
and(
|
||||
inArray(heartbeatRuns.id, holdingRunIds),
|
||||
inArray(heartbeatRuns.status, ["queued", "scheduled_retry", "running"]),
|
||||
),
|
||||
);
|
||||
for (const run of liveRuns) liveRunIds.add(run.id);
|
||||
}
|
||||
|
||||
let destroyed = 0;
|
||||
let failed = 0;
|
||||
let skippedLiveRun = 0;
|
||||
const failureReason = input.failureReason ?? "environment_delete_requested";
|
||||
const now = new Date();
|
||||
for (const leaseRow of leaseRows) {
|
||||
if (leaseRow.heartbeatRunId && liveRunIds.has(leaseRow.heartbeatRunId)) {
|
||||
skippedLiveRun += 1;
|
||||
continue;
|
||||
}
|
||||
// Claim the row BEFORE the provider call, mirroring the inline-teardown
|
||||
// invariant used elsewhere in this file: no provider destroy without a
|
||||
// durable `pending_cleanup` reference already on disk. The claim is one
|
||||
// conditional UPDATE, so it is the fence against a racing resume: a
|
||||
// resume that re-activates the lease first makes the status predicate
|
||||
// (or the run-liveness predicate) fail and the claim loses — the live
|
||||
// run keeps its sandbox. A claim that wins parks the lease where the
|
||||
// cleanup sweep owns it, so a crash or thrown destroy after this point
|
||||
// is recovered by the sweep's idempotent teardown, and a double write
|
||||
// failure cannot strand the lease in a reusable status.
|
||||
const claimedRow = await db
|
||||
.update(environmentLeases)
|
||||
.set({
|
||||
status: "pending_cleanup",
|
||||
failureReason,
|
||||
cleanupStatus: "failed",
|
||||
releasedAt: now,
|
||||
lastUsedAt: now,
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(
|
||||
and(
|
||||
eq(environmentLeases.id, leaseRow.id),
|
||||
inArray(environmentLeases.status, ["active", "released", "retained"]),
|
||||
sql`NOT EXISTS (
|
||||
SELECT 1 FROM ${heartbeatRuns}
|
||||
WHERE ${heartbeatRuns.id} = ${environmentLeases.heartbeatRunId}
|
||||
AND ${heartbeatRuns.status} IN ('queued', 'scheduled_retry', 'running')
|
||||
)`,
|
||||
),
|
||||
)
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!claimedRow) {
|
||||
// Lost to a racing resume or a concurrent terminal transition — the
|
||||
// lease is no longer ours to destroy.
|
||||
skippedLiveRun += 1;
|
||||
continue;
|
||||
}
|
||||
const leaseSnapshot = toEnvironmentLeaseSnapshot(claimedRow);
|
||||
try {
|
||||
const driver = getDriver(getLeaseDriverKey(leaseSnapshot, environment));
|
||||
if (!driver?.destroyRunLease) {
|
||||
// No driver available: the claim already parked the lease for the
|
||||
// sweep, which retries once the driver's plugin is back.
|
||||
failed += 1;
|
||||
continue;
|
||||
}
|
||||
const lease = await driver.destroyRunLease({
|
||||
environment,
|
||||
lease: leaseSnapshot,
|
||||
failureReason,
|
||||
});
|
||||
if (lease && lease.status !== "pending_cleanup") destroyed += 1;
|
||||
else failed += 1;
|
||||
} catch {
|
||||
// The claim above already parked the lease in `pending_cleanup`, so
|
||||
// the sweep owns the retry; its teardown is idempotent, so a destroy
|
||||
// that reached the provider before the throw resolves as success.
|
||||
failed += 1;
|
||||
}
|
||||
}
|
||||
return { destroyed, failed, skippedLiveRun };
|
||||
},
|
||||
|
||||
async resumeRunLease(input: EnvironmentDriverLeaseInput): Promise<PluginEnvironmentLease | EnvironmentLease | null> {
|
||||
const driver = requireDriverKey(getLeaseDriverKey(input.lease, input.environment));
|
||||
if (!driver.resumeRunLease) {
|
||||
|
||||
@@ -1230,8 +1230,17 @@ export function environmentService(db: Db) {
|
||||
),
|
||||
),
|
||||
db
|
||||
.select({ count: sql<number>`count(*)::int` })
|
||||
.select({
|
||||
leaseId: environmentLeases.id,
|
||||
executionWorkspaceId: environmentLeases.executionWorkspaceId,
|
||||
executionWorkspaceName: executionWorkspaces.name,
|
||||
issueId: environmentLeases.issueId,
|
||||
issueIdentifier: issues.identifier,
|
||||
issueTitle: issues.title,
|
||||
})
|
||||
.from(environmentLeases)
|
||||
.leftJoin(executionWorkspaces, eq(environmentLeases.executionWorkspaceId, executionWorkspaces.id))
|
||||
.leftJoin(issues, eq(environmentLeases.issueId, issues.id))
|
||||
.where(
|
||||
and(
|
||||
eq(environmentLeases.environmentId, id),
|
||||
@@ -1253,7 +1262,15 @@ export function environmentService(db: Db) {
|
||||
const isManagedLocal = environment.driver === "local";
|
||||
const isInstanceDefault = countFromRows(instanceDefaultRows) > 0;
|
||||
const pendingCleanupLeaseCount = countFromRows(pendingCleanupLeaseRows);
|
||||
const reusableSandboxLeaseCount = countFromRows(reusableSandboxLeaseRows);
|
||||
const reusableSandboxLeaseHolders = reusableSandboxLeaseRows.map((row) => ({
|
||||
leaseId: row.leaseId,
|
||||
executionWorkspaceId: row.executionWorkspaceId,
|
||||
executionWorkspaceName: row.executionWorkspaceName,
|
||||
issueId: row.issueId,
|
||||
issueIdentifier: row.issueIdentifier,
|
||||
issueTitle: row.issueTitle,
|
||||
}));
|
||||
const reusableSandboxLeaseCount = reusableSandboxLeaseHolders.length;
|
||||
const deleteBlockedReasons: EnvironmentDeleteBlockedReason[] = [];
|
||||
if (isManagedLocal) deleteBlockedReasons.push("managed_local");
|
||||
if (isInstanceDefault) deleteBlockedReasons.push("instance_default");
|
||||
@@ -1273,6 +1290,7 @@ export function environmentService(db: Db) {
|
||||
deleteBlockedReasons,
|
||||
pendingCleanupLeaseCount,
|
||||
reusableSandboxLeaseCount,
|
||||
reusableSandboxLeaseHolders,
|
||||
staticReferences: {
|
||||
isManagedLocal,
|
||||
isInstanceDefault,
|
||||
|
||||
Reference in new issue
Block a user