feat(coder): guard session delete against worktree work loss
Deleting a BooChat session CASCADE-wipes its session_worktrees row, which would silently orphan uncommitted/unpushed/unmerged work in the worktree. Add a pre-DELETE gate: the server reads session_worktrees from the shared DB first (no row = chat-only session = delete immediately, zero round-trip), and for worktree-backed sessions calls a new BooCoder endpoint that runs git on the host (only the host systemd service can see /tmp/booworktrees). checkWorktreeWorkAtRisk reports dirty/unpushed/unmerged via the audited hostExec+shellEscape path; default branch is detected from refs/remotes/origin/HEAD (not the worktree's own branch), never hardcoded. Any at-risk worktree returns 409 with per-worktree RiskReport[]; force=true bypasses the check entirely. Fail-closed: coder unreachable/errored also blocks (force still escapes). The sidebar renders a block dialog distinguishing work-at-risk (Commit/Stash/Force) from couldn't-verify (Cancel/Force only); stash uses -u and re-blocks on remaining commits with an explanatory message. Commit never auto-commits — it routes the user to the session. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -151,8 +151,17 @@ export const api = {
|
||||
method: 'PATCH',
|
||||
body: JSON.stringify(body),
|
||||
}),
|
||||
remove: (id: string) =>
|
||||
request<void>(`/api/sessions/${id}`, { method: 'DELETE' }),
|
||||
// force=true bypasses the server-side worktree work-loss guard. A blocked
|
||||
// delete throws ApiError(409) whose body carries { error, reports }.
|
||||
remove: (id: string, force = false) =>
|
||||
request<void>(`/api/sessions/${id}${force ? '?force=true' : ''}`, { method: 'DELETE' }),
|
||||
// Stash the session's worktree (uncommitted changes) on the host, via the
|
||||
// BooCoder proxy. Recoverable escape from the work-at-risk dialog.
|
||||
worktreeStash: (id: string) =>
|
||||
request<{ results: { worktreePath: string; stashed: boolean; error?: string }[] }>(
|
||||
`/api/coder/sessions/${id}/worktree-stash`,
|
||||
{ method: 'POST' },
|
||||
),
|
||||
archive: (id: string) =>
|
||||
request<void>(`/api/sessions/${id}/archive`, { method: 'POST' }),
|
||||
unarchive: (id: string) =>
|
||||
|
||||
@@ -34,6 +34,19 @@ export interface AvailableProject {
|
||||
|
||||
export type SessionStatus = 'open' | 'archived';
|
||||
|
||||
// Session-delete work-loss guard. Mirror of WorktreeRiskReport in
|
||||
// apps/server/src/types/api.ts — edit both copies together. Arrives as the
|
||||
// `reports` field of the 409 body when a delete is blocked.
|
||||
export interface WorktreeRiskReport {
|
||||
worktreePath: string;
|
||||
branch: string;
|
||||
dirty: boolean;
|
||||
unpushed: number; // commits ahead of upstream, or -1 if no upstream
|
||||
unmerged: number; // commits not in the project default branch
|
||||
atRisk: boolean;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
export interface Session {
|
||||
id: string;
|
||||
project_id: string;
|
||||
|
||||
@@ -19,12 +19,12 @@ import {
|
||||
DialogDescription,
|
||||
} from '@/components/ui/dialog';
|
||||
import { AddProjectModal } from './AddProjectModal';
|
||||
import { api } from '@/api/client';
|
||||
import { api, ApiError } from '@/api/client';
|
||||
import { useSidebar } from '@/hooks/useSidebar';
|
||||
import { useSidebarDrawer } from '@/hooks/useSidebarDrawer';
|
||||
import { useViewport } from '@/hooks/useViewport';
|
||||
import { usePullToRefresh } from '@/hooks/usePullToRefresh';
|
||||
import type { SidebarProject } from '@/api/types';
|
||||
import type { SidebarProject, WorktreeRiskReport } from '@/api/types';
|
||||
import { giteaUrlFor } from '@/lib/projectUrls';
|
||||
import { isCoderSessionName } from '@/lib/coder-session';
|
||||
import { cn } from '@/lib/utils';
|
||||
@@ -110,6 +110,16 @@ export function ProjectSidebar() {
|
||||
const [renamingProject, setRenamingProject] = useState<string | null>(null);
|
||||
const [renameProjectValue, setRenameProjectValue] = useState('');
|
||||
const [archiveProjectConfirm, setArchiveProjectConfirm] = useState<{ id: string; name: string } | null>(null);
|
||||
// Work-at-risk dialog: shown when a delete is blocked (409) because the
|
||||
// session's worktree holds uncommitted/unpushed/unmerged work.
|
||||
const [riskState, setRiskState] = useState<{
|
||||
sessionId: string;
|
||||
projectId: string;
|
||||
name: string;
|
||||
message: string;
|
||||
reports: WorktreeRiskReport[];
|
||||
} | null>(null);
|
||||
const [riskBusy, setRiskBusy] = useState(false);
|
||||
const navigate = useNavigate();
|
||||
const location = useLocation();
|
||||
const lastToastedError = useRef<string | null>(null);
|
||||
@@ -174,16 +184,81 @@ export function ProjectSidebar() {
|
||||
}
|
||||
}
|
||||
|
||||
async function handleDeleteSession(sessionId: string, projectId: string) {
|
||||
async function handleDeleteSession(
|
||||
sessionId: string,
|
||||
projectId: string,
|
||||
name: string,
|
||||
force = false,
|
||||
) {
|
||||
try {
|
||||
await api.sessions.remove(sessionId);
|
||||
await api.sessions.remove(sessionId, force);
|
||||
// Server publishes session_deleted via WS; useUserEvents delivers it.
|
||||
setRiskState(null);
|
||||
if (activeSession === sessionId) navigate(`/project/${projectId}`);
|
||||
} catch (err) {
|
||||
// 409 => the server's work-loss guard blocked the delete. Open the
|
||||
// work-at-risk dialog with the per-worktree reports instead of toasting.
|
||||
if (
|
||||
err instanceof ApiError &&
|
||||
err.status === 409 &&
|
||||
err.body && typeof err.body === 'object' && 'reports' in err.body
|
||||
) {
|
||||
const body = err.body as { error?: string; reports?: WorktreeRiskReport[] };
|
||||
setRiskState({
|
||||
sessionId,
|
||||
projectId,
|
||||
name,
|
||||
message: body.error ?? 'This session has work at risk.',
|
||||
reports: body.reports ?? [],
|
||||
});
|
||||
return;
|
||||
}
|
||||
toast.error(err instanceof Error ? err.message : 'failed to delete session');
|
||||
}
|
||||
}
|
||||
|
||||
// Stash the worktree's uncommitted changes (recoverable), then re-attempt the
|
||||
// delete. If unpushed/unmerged commits remain, the retry 409s again and the
|
||||
// dialog re-renders with the narrowed risk.
|
||||
async function handleStashAndRetry() {
|
||||
if (!riskState || riskBusy) return;
|
||||
setRiskBusy(true);
|
||||
try {
|
||||
const { results } = await api.sessions.worktreeStash(riskState.sessionId);
|
||||
const failed = results.find((r) => r.error);
|
||||
if (failed) {
|
||||
toast.error(`stash failed: ${failed.error}`);
|
||||
return;
|
||||
}
|
||||
await handleDeleteSession(riskState.sessionId, riskState.projectId, riskState.name, false);
|
||||
} catch (err) {
|
||||
toast.error(err instanceof Error ? err.message : 'stash failed');
|
||||
} finally {
|
||||
setRiskBusy(false);
|
||||
}
|
||||
}
|
||||
|
||||
// Explicit, destructive override — deletes despite work at risk.
|
||||
async function handleForceDelete() {
|
||||
if (!riskState || riskBusy) return;
|
||||
setRiskBusy(true);
|
||||
try {
|
||||
await handleDeleteSession(riskState.sessionId, riskState.projectId, riskState.name, true);
|
||||
} finally {
|
||||
setRiskBusy(false);
|
||||
}
|
||||
}
|
||||
|
||||
// Route the user to commit it themselves — never auto-commit. Opens the
|
||||
// session workspace where they can use a terminal or agent pane.
|
||||
function handleGoCommit() {
|
||||
if (!riskState) return;
|
||||
const sessionId = riskState.sessionId;
|
||||
setRiskState(null);
|
||||
navigate(`/session/${sessionId}`);
|
||||
toast.info('Open a terminal or agent in this session, commit and push your work, then delete again.');
|
||||
}
|
||||
|
||||
async function handleRenameSession(sessionId: string) {
|
||||
const trimmed = renameValue.trim();
|
||||
setRenamingSession(null);
|
||||
@@ -216,6 +291,20 @@ export function ProjectSidebar() {
|
||||
)
|
||||
: 'w-60 shrink-0 border-r bg-sidebar text-sidebar-foreground flex flex-col h-screen';
|
||||
|
||||
// Work-at-risk dialog framing. The server returns 409 in two distinct
|
||||
// situations: (1) work genuinely at risk (reports has ≥1 atRisk entry), or
|
||||
// (2) it couldn't verify (BooCoder down/errored → reports is empty). These
|
||||
// are different user stories — "your work is in danger" vs "the checker is
|
||||
// offline" — so the dialog must not show one generic message for both.
|
||||
const atRiskReports = riskState?.reports.filter((r) => r.atRisk) ?? [];
|
||||
const verifyFailed = riskState !== null && atRiskReports.length === 0;
|
||||
const anyDirty = atRiskReports.some((r) => r.dirty);
|
||||
// Commit-based risk (unpushed/unmerged) that stash can NOT clear. When this is
|
||||
// all that remains (e.g. after a stash cleared the dirty changes), the dialog
|
||||
// explains why it re-blocked and hides the Stash button so it doesn't look
|
||||
// like stash "didn't work".
|
||||
const anyCommits = atRiskReports.some((r) => r.unpushed !== 0 || r.unmerged > 0);
|
||||
|
||||
return (
|
||||
<aside className={asideCls}>
|
||||
<div className="px-4 py-3 border-b flex items-center justify-between">
|
||||
@@ -499,7 +588,7 @@ export function ProjectSidebar() {
|
||||
const projectId = projects.find((p) =>
|
||||
p.recent_sessions.some((s) => s.id === deleteConfirm.id)
|
||||
)?.id;
|
||||
if (projectId) void handleDeleteSession(deleteConfirm.id, projectId);
|
||||
if (projectId) void handleDeleteSession(deleteConfirm.id, projectId, deleteConfirm.name);
|
||||
}
|
||||
setDeleteConfirm(null);
|
||||
}}
|
||||
@@ -509,6 +598,77 @@ export function ProjectSidebar() {
|
||||
</div>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
|
||||
<Dialog open={riskState !== null} onOpenChange={(open) => { if (!open && !riskBusy) setRiskState(null); }}>
|
||||
<DialogContent>
|
||||
<DialogHeader>
|
||||
<DialogTitle>
|
||||
{verifyFailed ? 'Could not verify worktree safety' : 'This session has work at risk'}
|
||||
</DialogTitle>
|
||||
<DialogDescription>
|
||||
{verifyFailed ? (
|
||||
<>
|
||||
{riskState?.message ?? 'The worktree safety check is unavailable.'} Your work may be
|
||||
fine, but it couldn't be checked — only force-delete if you're sure.
|
||||
</>
|
||||
) : anyDirty && anyCommits ? (
|
||||
<>
|
||||
Deleting {riskState ? `"${riskState.name}"` : 'this session'} would orphan uncommitted
|
||||
changes <em>and</em> commits that aren't pushed or merged. Stash clears the
|
||||
changes (recoverable), but the commits will still block — push them or force-delete.
|
||||
</>
|
||||
) : anyDirty ? (
|
||||
<>
|
||||
Deleting {riskState ? `"${riskState.name}"` : 'this session'} would orphan uncommitted
|
||||
changes in its worktree. Stash them (recoverable), commit them, or force-delete.
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
Deleting {riskState ? `"${riskState.name}"` : 'this session'} would orphan commits that
|
||||
aren't pushed or merged. Stashing won't recover these — push them, or
|
||||
force-delete.
|
||||
</>
|
||||
)}
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
{!verifyFailed && (
|
||||
<div className="flex flex-col gap-2 py-1 text-sm">
|
||||
{atRiskReports.map((r) => (
|
||||
<div key={r.worktreePath} className="rounded border border-border/60 px-3 py-2">
|
||||
<div className="font-mono text-xs text-muted-foreground truncate" title={r.worktreePath}>
|
||||
{r.branch || r.worktreePath}
|
||||
</div>
|
||||
<ul className="mt-1 list-disc pl-5 text-foreground/90">
|
||||
{r.error && <li className="text-destructive">git error: {r.error}</li>}
|
||||
{r.dirty && <li>uncommitted changes</li>}
|
||||
{r.unpushed === -1 && <li>local-only branch (no upstream)</li>}
|
||||
{r.unpushed > 0 && <li>{r.unpushed} unpushed commit{r.unpushed === 1 ? '' : 's'}</li>}
|
||||
{r.unmerged > 0 && <li>{r.unmerged} unmerged commit{r.unmerged === 1 ? '' : 's'}</li>}
|
||||
</ul>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
<div className="flex flex-wrap gap-2 justify-end pt-2">
|
||||
<Button variant="outline" disabled={riskBusy} onClick={() => setRiskState(null)}>
|
||||
Cancel
|
||||
</Button>
|
||||
{!verifyFailed && (
|
||||
<Button variant="outline" disabled={riskBusy} onClick={handleGoCommit}>
|
||||
Commit…
|
||||
</Button>
|
||||
)}
|
||||
{!verifyFailed && anyDirty && (
|
||||
<Button variant="outline" disabled={riskBusy} onClick={() => void handleStashAndRetry()}>
|
||||
Stash & delete
|
||||
</Button>
|
||||
)}
|
||||
<Button variant="destructive" disabled={riskBusy} onClick={() => void handleForceDelete()}>
|
||||
Force delete
|
||||
</Button>
|
||||
</div>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
</aside>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user