diff --git a/apps/desktop/src/ipc/contract.ts b/apps/desktop/src/ipc/contract.ts index ca5f65180..827616a08 100644 --- a/apps/desktop/src/ipc/contract.ts +++ b/apps/desktop/src/ipc/contract.ts @@ -3006,7 +3006,11 @@ export interface LegacyImportRequest { export type LegacyImportResult = Result<{ data: BootstrapPayload; imported: number }>; -/** `created: false` means an existing project at that path was selected instead of inserted. */ +/** + * `created: false` means an existing project at that path was selected instead + * of inserted: one already on the rail, or a removed one put back on it with + * its tickets and Sessions (VC-822). + */ export type ProjectCreateResult = Result<{ project: Project; created: boolean }>; export type ProjectUpdateResult = Result<{ project: Project }>; diff --git a/apps/desktop/src/main/data-ipc.test.ts b/apps/desktop/src/main/data-ipc.test.ts index 62c9819c3..75801bc5e 100644 --- a/apps/desktop/src/main/data-ipc.test.ts +++ b/apps/desktop/src/main/data-ipc.test.ts @@ -457,15 +457,21 @@ describe("volli:database", () => { * path, and that bargain is only honest if something refuses it earlier, where a * person is present to be told. */ -// VC-796: the channel projects the host's `project.remove`. The cascade takes -// every Session of the project, so the live work they hold stops first, while -// the row is still there, and every window (a second one included) is told. +// VC-796: the channel projects the host's `project.remove`. The live work its +// Sessions hold stops first, while the project is still on the rail, and +// every window (a second one included) is told. VC-822: the project is then +// detached, not deleted, so adding its folder again brings its board back. describe("project removal", () => { + function onRail(projectId: string): boolean { + const bootstrap = invoke("volli:data-bootstrap"); + return bootstrap.ok && bootstrap.data.projects.some(({ id }) => id === projectId); + } + function liveWork(projectId: string, stillOpen: readonly string[] = []) { const stopped: string[] = []; const stop = (step: string) => { - // Each stop runs while the project, and so its Sessions, still exist. - expect(getProjectById(ctx.db, projectId)).toBeDefined(); + // Each stop runs while the project is still on the rail. + expect(onRail(projectId)).toBe(true); stopped.push(step); }; const live: ProjectLiveWork = { @@ -481,7 +487,7 @@ describe("project removal", () => { return { live, stopped }; } - it("stops the project's live work before deleting it, then tells every window", async () => { + it("stops the project's live work before detaching it, then tells every window", async () => { const projectId = createProject(); const { live, stopped } = liveWork(projectId); registerDataIpcHandlers( @@ -496,12 +502,62 @@ describe("project removal", () => { ).resolves.toEqual({ ok: true }); expect(stopped).toEqual(["bindings", "shells", "terminals", "tabs"]); - expect(getProjectById(ctx.db, projectId)).toBeUndefined(); + expect(onRail(projectId)).toBe(false); // Untargeted, so a window that still holds the project re-reads the list // without it; the fan-out reaches every window, a second one included. await expectDataChanged({ entity: "tickets" }); }); + // The defect (VC-822): the removal used to delete the row, and the cascades + // took the board and every Session with it. A re-add found nothing. + it("brings the board back when the same folder is added again", async () => { + const path = freshProjectDir(); + const nothingRunning: ProjectLiveWork = { + releaseBindings: async () => ({ stillOpen: [] }), + disposeShells: () => undefined, + killTerminals: () => undefined, + closeBrowserTabs: () => undefined, + mcp: null, + }; + registerDataIpcHandlers( + { ok: true, db: ctx.db }, + { + sessionEngine: fixtureSessionEngine, + projectLiveWork: nothingRunning, + detectBaseBranch: async () => "main", + }, + ); + const added = await invoke>("volli:project-create", { + path, + name: "Kept", + }); + if (!added.ok) throw new Error(added.error); + const ticket = createTicket(added.project.id); + await expect( + invoke>("volli:project-remove", added.project.id), + ).resolves.toEqual({ ok: true }); + + // Off every window's rail, its tickets with it. + const hidden = invoke("volli:data-bootstrap"); + expect(hidden.ok && hidden.data.projects).toEqual([]); + expect(hidden.ok && Object.keys(hidden.data.ticketsByProject)).toEqual([]); + flushDataChangedForTest(); + dataChangedSends.length = 0; + + await expect( + invoke>("volli:project-create", { path, name: "Kept" }), + ).resolves.toMatchObject({ ok: true, created: false, project: { id: added.project.id } }); + + const back = invoke("volli:data-bootstrap"); + expect(back.ok && back.data.projects.map(({ id }) => id)).toEqual([added.project.id]); + expect(back.ok && back.data.ticketsByProject[added.project.id]).toEqual([ + expect.objectContaining({ id: ticket.id, ticketNumber: ticket.ticketNumber }), + ]); + // Every window re-reads it: one that seeded an empty board on the add + // must not keep showing it empty. + await expectDataChanged({ projectId: added.project.id, entity: "tickets" }); + }); + it("keeps the project, and says why, when one of its Sessions will not stop", async () => { const projectId = createProject(); const { live, stopped } = liveWork(projectId, ["session-1"]); @@ -521,7 +577,7 @@ describe("project removal", () => { }); expect(stopped).toEqual(["bindings"]); - expect(getProjectById(ctx.db, projectId)).toBeDefined(); + expect(onRail(projectId)).toBe(true); expectNoDataChange(); }); }); diff --git a/apps/desktop/src/main/data-ipc.ts b/apps/desktop/src/main/data-ipc.ts index 15cae43cf..d66400896 100644 --- a/apps/desktop/src/main/data-ipc.ts +++ b/apps/desktop/src/main/data-ipc.ts @@ -288,12 +288,10 @@ function buildBootstrapPayload(db: Database.Database): BootstrapPayload { ticketsByProject[project.id] = []; labelsByProject[project.id] = []; } - for (const ticket of listAllTickets(db)) { - (ticketsByProject[ticket.projectId] ??= []).push(ticket); - } - for (const label of listAllLabels(db)) { - (labelsByProject[label.projectId] ??= []).push(label); - } + // Only the projects on the rail: a removed project is detached, not deleted + // (VC-822), and its tickets and labels stay out of every window with it. + for (const ticket of listAllTickets(db)) ticketsByProject[ticket.projectId]?.push(ticket); + for (const label of listAllLabels(db)) labelsByProject[label.projectId]?.push(label); return { projects, ticketsByProject, labelsByProject, appState }; } @@ -555,8 +553,9 @@ export function registerDataIpcHandlers( "volli:legacy-import": (request: LegacyImportRequest): LegacyImportResult => { // Idempotent-safe: only import into a genuinely empty projects // table; a second call (e.g. a relaunch racing the renderer) just - // hands back the current state instead of re-importing over it. - if (countProjects(db) > 0) { + // hands back the current state instead of re-importing over it. A + // removed project still holds its row (VC-822), so it counts here. + if (countProjects(db, "all") > 0) { return { ok: true, data: buildBootstrapPayload(db), imported: 0 }; } const legacyProjects = sanitizeLegacyProjects(request.projects); @@ -594,8 +593,17 @@ export function registerDataIpcHandlers( // The rules live in host-core (VC-623), shared with the operator's // `volli project add` on a headless host: one validation, every door. + // A registration is announced (VC-822): adding a removed project's folder + // again brings back its board, which a window only reads on a change. "volli:project-create": (input: ProjectCreateInput): Promise => - createProject({ db, detectBaseBranch: options.detectBaseBranch }, input), + createProject( + { + db, + detectBaseBranch: options.detectBaseBranch, + onCreated: (project) => broadcastDataChanged({ projectId: project.id }), + }, + input, + ), /** * Whether one project's registered folder is still there (VC-430) — the @@ -637,8 +645,9 @@ export function registerDataIpcHandlers( /** * The window's projection of the host's removal (VC-796): the project's - * live work stopped, then the delete, its feed ended and every window - * told. The renderer keeps only its own views' forgetting. + * live work stopped, then the project detached with its tickets and + * Sessions kept (VC-822), its feed ended and every window told. The + * renderer keeps only its own views' forgetting. */ "volli:project-remove": async (id: string): Promise => { await board("project.remove", { projectId: id }); diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index 2a8c9d165..ec7361c6a 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -1532,7 +1532,7 @@ const appStartup = app.whenReady().then(async () => { automations: runtimeAutomations, busyWorktreeSites, interruptTicketSessions: interruptTicketSessionsAnnounced, - // What removing a project stops before its delete (VC-796): every door + // What removing a project stops before it detaches it (VC-796): every door // that removes one, the window's `volli:project-remove` included. projectLiveWork: hostProjectLiveWork({ runtime: () => sessionRuntime, diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index ba6af8aeb..f90ef8035 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -568,7 +568,10 @@ const api = { projects: { pickFolder: (): Promise => invoke("volli:pick-project-folder"), syncRoots: (paths: string[]): Promise => invoke("volli:sync-project-roots", paths), - /** Creates a project row, or (`created: false`) returns the existing one already tracked at `path`. */ + /** + * Creates a project row, or (`created: false`) returns the existing one + * tracked at `path`, re-attaching it if it was removed (VC-822). + */ create: (input: ProjectCreateInput): Promise => invoke("volli:project-create", input), /** Updates the project's pinned automation base branch and/or worktree setup command. */ diff --git a/apps/desktop/src/renderer/src/components/rail/remove-project-dialog.tsx b/apps/desktop/src/renderer/src/components/rail/remove-project-dialog.tsx index 0ecc98914..1a9215aaf 100644 --- a/apps/desktop/src/renderer/src/components/rail/remove-project-dialog.tsx +++ b/apps/desktop/src/renderer/src/components/rail/remove-project-dialog.tsx @@ -86,7 +86,9 @@ export function RemoveProjectDialog({ project, open, onOpenChange }: RemoveProje Remove {project.name} from Volli? - The folder on disk is untouched. + {/* Removal detaches the project (VC-822): nothing is deleted. */} + The folder and its tickets and Sessions are kept. Add the folder again to bring them + back. {busyLines.map((line) => ( {line} diff --git a/apps/desktop/src/renderer/src/stores/projects.test.ts b/apps/desktop/src/renderer/src/stores/projects.test.ts index b33519e08..72d5ebb94 100644 --- a/apps/desktop/src/renderer/src/stores/projects.test.ts +++ b/apps/desktop/src/renderer/src/stores/projects.test.ts @@ -495,7 +495,7 @@ describe("removeProject", () => { }); it("forgets the removed project's terminal sessions, leaving their PTYs to the host", async () => { - // The host's `project.remove` ended the PTYs before its delete (VC-796), + // The host's `project.remove` ended the PTYs before its detach (VC-796), // for every window; the store only forgets this window's views of them, // with no terminal view mounted to do it as an unmount side effect. // Keeps `appState` in the stub too: zustand's `persist` middleware writes diff --git a/apps/desktop/src/renderer/src/stores/projects.ts b/apps/desktop/src/renderer/src/stores/projects.ts index b61f04736..03a9e89b4 100644 --- a/apps/desktop/src/renderer/src/stores/projects.ts +++ b/apps/desktop/src/renderer/src/stores/projects.ts @@ -665,9 +665,10 @@ export function createProjectsStore( if (!result) return; localProjectIds.delete(id); - // The host stopped the project's live work before it deleted it (VC-796): + // The host stopped the project's live work before it detached it (VC-796): // its Sessions' bindings, shells, terminals and tabs, for every window - // and whichever door removed it. What is left here is this window's own + // and whichever door removed it. Its tickets and Sessions are kept for + // a re-add of the folder (VC-822). What is left here is this window's own // views of it. `forgetProjectSessions` disposes the terminal engines // and drops their records (the PTYs behind them are already gone), the // archived tickets' included, whether or not a view is mounted. diff --git a/apps/hostd/src/host-protocol.test.ts b/apps/hostd/src/host-protocol.test.ts index 631d309de..85181fbd0 100644 --- a/apps/hostd/src/host-protocol.test.ts +++ b/apps/hostd/src/host-protocol.test.ts @@ -17,6 +17,7 @@ import { hostResourceWorkspace, isLiteralLoopback, parseListen, + servedWorkspace, sessionWorkspace, } from "./host-protocol"; @@ -140,6 +141,15 @@ describe("hostd's board feed and resource port (VC-565)", () => { ]); }); + // VC-822: a removed project is detached, not deleted; no Workspace door + // serves it until its folder is added again. + it("serves a Workspace only while its project is on the rail", () => { + expect(servedWorkspace(db, PROJECT)).toEqual({ id: PROJECT, epoch: 4 }); + db.prepare("UPDATE projects SET removed_at = 9 WHERE id = ?").run(PROJECT); + expect(servedWorkspace(db, PROJECT)).toBeNull(); + expect(servedWorkspace(db, "no-such-project")).toBeNull(); + }); + it("answers a Session from its ledger and every board kind from the board", async () => { const resolve = hostResourceWorkspace(db, { getSession: async () => null as never, diff --git a/apps/hostd/src/host-protocol.ts b/apps/hostd/src/host-protocol.ts index f66fc952b..d0bc30780 100644 --- a/apps/hostd/src/host-protocol.ts +++ b/apps/hostd/src/host-protocol.ts @@ -34,7 +34,7 @@ import { createBoardChangeFeed, type BoardChangeFeed, } from "@volli/host-core/board"; -import { getProjectById, getTicketRow, listProjects, prepared } from "@volli/host-core/db"; +import { getTicketRow, isProjectAttached, listProjects, prepared } from "@volli/host-core/db"; import { admittedHandlers, ROUTER_POLICY, type HostHandlerMap } from "@volli/host-core/handlers"; import { HOST_CONNECTION_ONLY_FEATURES, @@ -146,12 +146,16 @@ export function parseListen(value: string): HostProtocolBind | string { return { host, port }; } -/** The highest epoch this host records for a project, or null when it has no such project. */ +/** + * The highest epoch this host records for a project, or null when it has no + * such project on its rail: a removed project is detached, not deleted + * (VC-822), and no Workspace door serves it. + */ export function servedWorkspace( db: Database.Database, workspaceId: string, ): ServedWorkspace | null { - if (getProjectById(db, workspaceId) === undefined) return null; + if (!isProjectAttached(db, workspaceId)) return null; // An aggregate answers one row, with a null epoch when there is none. const { epoch } = prepared<[string], { epoch: number | null }>( db, diff --git a/packages/host-core/src/agent-dispatch/project-verbs.ts b/packages/host-core/src/agent-dispatch/project-verbs.ts index c57b8395e..b65189b1e 100644 --- a/packages/host-core/src/agent-dispatch/project-verbs.ts +++ b/packages/host-core/src/agent-dispatch/project-verbs.ts @@ -62,6 +62,7 @@ export async function projectAddVerb( if (dryRun) { const target = { kind: "project" as const, id: plan.project.id, label: plan.project.name }; // A folder already tracked is answered, not written: say so in the preview. + // A removed one is re-attached (VC-822), which the preview names too. const preview = plan.kind === "existing" ? dryRunResponse(request, target, { @@ -70,7 +71,20 @@ export async function projectAddVerb( `${plan.project.path} is already the project ${plan.project.name}; nothing changes.`, ], }) - : dryRunResponse(request, { ...target, id: null }); + : plan.kind === "reattached" + ? dryRunResponse(request, target, { + durableWrites: [ + { + resource: "project", + operation: "update", + summary: "Put the removed Project row back on the rail.", + }, + ], + humanVisibleEffects: [ + `${plan.project.path} was the removed project ${plan.project.name}; it comes back under that name, with its tickets and Sessions.`, + ], + }) + : dryRunResponse(request, { ...target, id: null }); return preview!; } return { diff --git a/packages/host-core/src/automation-services.headless.test.ts b/packages/host-core/src/automation-services.headless.test.ts index af830cf29..88df14b59 100644 --- a/packages/host-core/src/automation-services.headless.test.ts +++ b/packages/host-core/src/automation-services.headless.test.ts @@ -4,7 +4,7 @@ import { afterEach, expect, it, vi } from "vite-plus/test"; import { nextScheduleOccurrence } from "@volli/shared"; import { createHostAutomations, type AutomationSessionPorts } from "./automation-services"; import { openTestDb, testProject, type TestDb } from "./db/test-helpers"; -import { insertProject } from "./db/projects-repo"; +import { detachProject, insertProject } from "./db/projects-repo"; import { listSkippedOccurrencesForProject } from "./db/automations-repo"; let ctx: TestDb | undefined; @@ -76,3 +76,61 @@ it("keeps CRUD and the unattended schedule alive without a window or attention a await automations.settled(); } }); + +// VC-822: removal detaches a project and keeps its Automations, so its schedule +// still comes due. The real Run door refuses it, and the skip says why. +it("records a removed project's due schedule as a skip that names the removal", async () => { + vi.useFakeTimers(); + const now = Date.UTC(2026, 9, 4, 12); + vi.setSystemTime(now); + ctx = openTestDb(); + const project = testProject(); + insertProject(ctx.db, project); + const log = { error: vi.fn() }; + const automations = createHostAutomations({ db: ctx.db, events: { publish: vi.fn() }, log }); + const schedule = { preset: "hourly" as const, minute: 30, timeZone: "UTC" }; + const created = await automations.service.create({ + commandId: randomUUID(), + projectId: project.id, + name: "Removed sweep", + instructions: "Check the project", + trigger: { kind: "schedule", schedule }, + runtime: { kind: "tier", tier: "fast" }, + }); + if (!created.ok) throw new Error(created.error); + await automations.service.setEnabled({ + commandId: randomUUID(), + automationId: created.automation.id, + enabled: true, + }); + detachProject(ctx.db, project.id, now); + + const create = vi.fn(); + automations.start(() => ({ + sessions: { create, attach: vi.fn() }, + promptSupply: vi.fn(), + deliverInstructions: vi.fn(), + reportInstructionDeliveryFailure: vi.fn(), + readSessionActivity: vi.fn(), + })); + try { + const due = nextScheduleOccurrence({ schedule, staggerKey: created.automation.id, after: now }); + await vi.advanceTimersByTimeAsync(due - now); + await vi.waitFor(() => + expect(listSkippedOccurrencesForProject(ctx!.db, project.id)).toMatchObject([ + { + reason: { + kind: "run-refused", + code: "PROJECT_REMOVED", + error: "The project was removed from Volli.", + }, + }, + ]), + ); + expect(create).not.toHaveBeenCalled(); + expect(log.error).not.toHaveBeenCalled(); + } finally { + automations.stop(); + await automations.settled(); + } +}); diff --git a/packages/host-core/src/automation-services.test.ts b/packages/host-core/src/automation-services.test.ts index 1e516a6e7..2d1d15b9d 100644 --- a/packages/host-core/src/automation-services.test.ts +++ b/packages/host-core/src/automation-services.test.ts @@ -23,7 +23,7 @@ import { readScheduleCursors, rebaseScheduleCursor, } from "./automations/schedule-cursor"; -import { getProjectById } from "./db/projects-repo"; +import { getProjectById, isProjectAttached, isProjectRemoved } from "./db/projects-repo"; import { getTicket, getTicketRow } from "./db/tickets-repo"; import { getAutomation, @@ -183,10 +183,12 @@ describe("construction", () => { it("reads the service's projections from the same database", () => { const deps = fixture().serviceDeps(); - vi.mocked(getProjectById).mockReturnValueOnce({ id: "p1" } as never); + // A removed project is detached, not deleted (VC-822): it is no project + // an Automation can be written in. + vi.mocked(isProjectAttached).mockReturnValueOnce(true).mockReturnValueOnce(false); expect(deps.findProject("p1")).toBe(true); expect(deps.findProject("p2")).toBe(false); - expect(getProjectById).toHaveBeenCalledWith(db, "p2"); + expect(isProjectAttached).toHaveBeenCalledWith(db, "p2"); deps.findAutomation("a1"); expect(getAutomation).toHaveBeenCalledWith(db, "a1"); deps.listAutomationsForProject("p1"); @@ -359,9 +361,15 @@ describe("runner wiring", () => { expect(getAutomationRun).toHaveBeenCalledWith(db, "r1"); deps.findTicket("t1"); expect(getTicket).toHaveBeenCalledWith(db, "t1"); - vi.mocked(getProjectById).mockReturnValueOnce({ id: "p1" } as never); + // Nor one a Run starts in (VC-822). + vi.mocked(isProjectAttached).mockReturnValueOnce(true).mockReturnValueOnce(false); expect(deps.findProject("p1")).toBe(true); expect(deps.findProject("p2")).toBe(false); + expect(isProjectAttached).toHaveBeenLastCalledWith(db, "p2"); + // And a removed one is told apart, so its schedule's skip says so. + vi.mocked(isProjectRemoved).mockReturnValueOnce(true); + expect(deps.projectRemoved!("p2")).toBe(true); + expect(isProjectRemoved).toHaveBeenLastCalledWith(db, "p2"); deps.listRunsForTicket("t1"); expect(listRunsForTicket).toHaveBeenCalledWith(db, "t1"); const query = { automationId: "a1", projectId: "p1" }; @@ -426,6 +434,7 @@ describe("pending armed Run wiring", () => { const row = { archived_at: null, project_id: "p1" } as never; const ticket = { projectId: "p1", status: "doing", ticketNumber: 7 } as never; const project = { ticketPrefix: "VC" } as never; + vi.mocked(isProjectAttached).mockReturnValue(true); expect(readTicket("t1")).toBeUndefined(); vi.mocked(getTicketRow).mockReturnValueOnce({ archived_at: 1, project_id: "p1" } as never); expect(readTicket("t1")).toBeUndefined(); @@ -436,6 +445,12 @@ describe("pending armed Run wiring", () => { vi.mocked(getTicket).mockReturnValueOnce(ticket); expect(readTicket("t1")).toBeUndefined(); expect(getProjectById).toHaveBeenLastCalledWith(db, "p1"); + // A removed project's armed column starts nothing (VC-822): its ticket + // reads as gone, so the countdown is dropped rather than run. + vi.mocked(isProjectAttached).mockReturnValueOnce(false); + vi.mocked(getTicketRow).mockReturnValueOnce(row); + expect(readTicket("t1")).toBeUndefined(); + expect(isProjectAttached).toHaveBeenLastCalledWith(db, "p1"); vi.mocked(getTicketRow).mockReturnValueOnce(row); vi.mocked(getTicket).mockReturnValueOnce(ticket); vi.mocked(getProjectById).mockReturnValueOnce(project); diff --git a/packages/host-core/src/automation-services.ts b/packages/host-core/src/automation-services.ts index 26013ae56..2c5a56802 100644 --- a/packages/host-core/src/automation-services.ts +++ b/packages/host-core/src/automation-services.ts @@ -13,7 +13,7 @@ import type Database from "better-sqlite3"; import { displayTicketId } from "@volli/shared"; import type { Logger } from "./log/logger"; import { hostLogger } from "./log/root"; -import { getProjectById } from "./db/projects-repo"; +import { getProjectById, isProjectAttached, isProjectRemoved } from "./db/projects-repo"; import { getTicket, getTicketRow } from "./db/tickets-repo"; import { getAutomation, @@ -139,7 +139,9 @@ export function createHostAutomations(input: HostAutomationsInput): HostAutomati const service = createAutomationService({ engine, - findProject: (projectId) => getProjectById(db, projectId) !== undefined, + // A removed project is detached, not deleted (VC-822): its Automations + // stay, but none of them is edited, scheduled or run until it is back. + findProject: (projectId) => isProjectAttached(db, projectId), findAutomation: (automationId) => getAutomation(db, automationId), listAutomationsForProject: (projectId) => listAutomationsForProject(db, projectId), runsForTicket: (ticketId) => listRunsForTicket(db, ticketId), @@ -168,7 +170,8 @@ export function createHostAutomations(input: HostAutomationsInput): HostAutomati findAutomation: (automationId) => getAutomation(db, automationId), findRun: (runId) => getAutomationRun(db, runId), findTicket: (ticketId) => getTicket(db, ticketId), - findProject: (projectId) => getProjectById(db, projectId) !== undefined, + findProject: (projectId) => isProjectAttached(db, projectId), + projectRemoved: (projectId) => isProjectRemoved(db, projectId), listRunsForTicket: (ticketId) => listRunsForTicket(db, ticketId), listPossiblyLiveProjectRunsForAutomation: (query) => listPossiblyLiveProjectRunsForAutomation(db, query), @@ -203,6 +206,8 @@ export function createHostAutomations(input: HostAutomationsInput): HostAutomati readTicket: (ticketId) => { const row = getTicketRow(db, ticketId); if (row === undefined || row.archived_at !== null) return undefined; + // An armed column in a removed project starts nothing (VC-822). + if (!isProjectAttached(db, row.project_id)) return undefined; const ticket = getTicket(db, ticketId); const project = getProjectById(db, row.project_id); if (ticket === undefined || project === undefined) return undefined; diff --git a/packages/host-core/src/automations/run.test.ts b/packages/host-core/src/automations/run.test.ts index 8a6bea226..c96a53553 100644 --- a/packages/host-core/src/automations/run.test.ts +++ b/packages/host-core/src/automations/run.test.ts @@ -1284,6 +1284,38 @@ describe("createAutomationRunner", () => { expect(h.creates).toEqual([]); }); + // VC-822: removal detaches a project and keeps its Automations, so a + // schedule still comes due there. Its skip names the removal, not a + // project that was never there. + it("refuses a Run in a removed project as removed, ticket or project scope", async () => { + const h = harness({ projectRemoved: () => true }); + const automation = await savedAutomation(h); + const removed = { + ok: false, + code: "PROJECT_REMOVED", + error: "The project was removed from Volli.", + }; + + await expect( + h.runner.runForProject({ + commandId: randomUUID(), + automationId: automation.id, + projectId: h.projectId, + attendance: "unattended", + }), + ).resolves.toMatchObject(removed); + await expect( + h.runner.run({ + commandId: randomUUID(), + target: { kind: "automation", automationId: automation.id }, + ticketId: h.ticketId, + modelOverride: null, + attendance: "attended", + }), + ).resolves.toMatchObject(removed); + expect(h.creates).toEqual([]); + }); + it("refuses an Automation owned by another project", async () => { const h = harness(); const created = await h.engine.create({ diff --git a/packages/host-core/src/automations/run.ts b/packages/host-core/src/automations/run.ts index f2017151f..fda0eb495 100644 --- a/packages/host-core/src/automations/run.ts +++ b/packages/host-core/src/automations/run.ts @@ -98,8 +98,14 @@ export interface AutomationRunnerPorts { /** The frozen Run projection that owns a pending delivery's launch title. */ findRun(runId: string): AutomationRun | undefined; findTicket(ticketId: string): AutomationRunTicket | undefined; - /** Whether a Project scope names a project this host actually has. */ + /** Whether a Project scope names a project this host has on its rail. */ findProject(projectId: string): boolean; + /** + * Whether the project was removed (VC-822): detached, its Automations kept. + * A Run there is refused as `PROJECT_REMOVED` rather than as not found, so + * the skip a schedule records says what happened. Absent: never removed. + */ + projectRemoved?(projectId: string): boolean; listRunsForTicket(ticketId: string): readonly AutomationRun[]; /** * One Automation's Runs in one project that could still be live — the @@ -260,6 +266,9 @@ export interface AutomationRunner { } /** One refusal shape, so the IPC layer never has to parse a bare string. */ +/** Read after "Skipped — " in a schedule's history, so it starts as a sentence. */ +const PROJECT_REMOVED_REFUSAL = "The project was removed from Volli."; + function refuse(code: AutomationRunRefusalCode, error: string): AutomationRunRefusal { return { ok: false, code, error }; } @@ -665,6 +674,9 @@ export function createAutomationRunner(deps: AutomationRunnerPorts): AutomationR const ticket = deps.findTicket(scope.ticketId); if (ticket === undefined) return refuse("TICKET_NOT_FOUND", "The requested Ticket was not found."); + if (deps.projectRemoved?.(ticket.projectId) === true) { + return refuse("PROJECT_REMOVED", PROJECT_REMOVED_REFUSAL); + } if ( automation !== null && automation.projectId !== null && @@ -686,6 +698,9 @@ export function createAutomationRunner(deps: AutomationRunnerPorts): AutomationR priorRuns: deps.listRunsForTicket(ticket.id), }; } + if (deps.projectRemoved?.(scope.projectId) === true) { + return refuse("PROJECT_REMOVED", PROJECT_REMOVED_REFUSAL); + } if (!deps.findProject(scope.projectId)) { return refuse("PROJECT_NOT_FOUND", "The requested project was not found."); } diff --git a/packages/host-core/src/board/commands.test.ts b/packages/host-core/src/board/commands.test.ts index 6711a75b1..8c7166d3b 100644 --- a/packages/host-core/src/board/commands.test.ts +++ b/packages/host-core/src/board/commands.test.ts @@ -25,7 +25,7 @@ import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from "vite import { getComment } from "../db/comments-repo"; import { listTicketEvents } from "../db/events-repo"; import { listTicketLabels } from "../db/labels-repo"; -import { insertProject } from "../db/projects-repo"; +import { detachProject, insertProject } from "../db/projects-repo"; import { openTestDb, testProject, type TestDb } from "../db/test-helpers"; import { getTicket, getTicketRow, listTicketsByProject } from "../db/tickets-repo"; import { ADMITTED, admittedHandlers, type HandlerPolicy } from "../handlers/handler-map"; @@ -282,6 +282,47 @@ describe("reads", () => { expect(await map["board.ticketBody"]({ ticketId: "t-1" }, USER)).toEqual({ body: "the body" }); }); + // VC-822: a removed project is detached, not deleted. Its board is not read + // or written until its folder is added again; a write it already took + // still replays. + it("reads and writes nothing in a removed project, but replays a write it took", async () => { + const map = handlers(); + seed("t-1"); + const input = { + projectId: PROJECT, + commandId: "c-before", + status: "todo" as const, + title: "A", + }; + const before = await map["board.createTicket"](input, USER); + detachProject(ctx.db, PROJECT, 7); + const receipts = counts().board_command_receipts; + batches.length = 0; + const removed = "This project was removed from Volli. Add its folder again to change it."; + + await expect(async () => map["board.snapshot"]({ projectId: PROJECT }, USER)).rejects.toThrow( + "Unknown project", + ); + await expect(async () => map["board.roster"]({ projectId: PROJECT }, USER)).rejects.toThrow( + "Unknown project", + ); + await expect(async () => + map["board.createTicket"]({ ...input, commandId: "c-after" }, USER), + ).rejects.toThrow(removed); + await expect(async () => + map["board.updateTicket"]({ ticketId: "t-1", title: "Renamed" }, USER), + ).rejects.toThrow(removed); + await expect(async () => + map["board.updateProject"]({ projectId: PROJECT, baseBranch: "main" }, USER), + ).rejects.toThrow(removed); + await expectReplay(before, () => map["board.createTicket"](input, USER)); + + expect(getTicket(ctx.db, "t-1")?.title).not.toBe("Renamed"); + expect(listTicketsByProject(ctx.db, PROJECT)).toHaveLength(2); + expect(counts().board_command_receipts).toBe(receipts); + expect(batches).toEqual([]); + }); + it("refuses an unknown project or ticket rather than answering empty", async () => { const map = handlers(); await expect(async () => map["board.snapshot"]({ projectId: "nope" }, USER)).rejects.toThrow( diff --git a/packages/host-core/src/board/commands.ts b/packages/host-core/src/board/commands.ts index c3e546606..32ce09a62 100644 --- a/packages/host-core/src/board/commands.ts +++ b/packages/host-core/src/board/commands.ts @@ -58,6 +58,8 @@ import { listTicketEvents, listTicketStatusEntries } from "../db/events-repo"; import { getLabel, listLabelsByProject, listTicketLabels, setLabelColor } from "../db/labels-repo"; import { getProjectById, + isProjectAttached, + isProjectRemoved, updateProjectBaseBranch, updateProjectSessionDefaults, updateProjectSetupCommand, @@ -286,6 +288,8 @@ export interface BoardCommandOptions { const BOARD_UNAVAILABLE = "The board is unavailable: the database did not open"; const SIGNALS_UNAVAILABLE = "Ticket signals are unavailable on this host"; +/** Why a removed project's board takes no write (VC-822). */ +const PROJECT_REMOVED = "This project was removed from Volli. Add its folder again to change it."; /** * Tickets whose worktree is being materialized right now (VC-98): a @@ -401,6 +405,12 @@ export function createBoardHandlers(options: BoardCommandOptions): BoardHandlerS }; } } + // A removed project takes no write (VC-822): it is detached, not deleted, + // and nothing in it changes until its folder is added again. A replay above + // still answers the write it already took. + if (spec.workspaceId !== null && isProjectRemoved(db, spec.workspaceId)) { + throw new Error(PROJECT_REMOVED); + } // A resource that is gone refuses inside `write`, with today's message. const workspaceId = spec.workspaceId ?? ""; const transact = (): Value => @@ -549,10 +559,13 @@ export function createBoardHandlers(options: BoardCommandOptions): BoardHandlerS return { "ticket.move": (input, call) => movedWithFeed(board(), input, call, undefined), + // A removed project (VC-822) is no board to read: refused like an unknown one. "board.snapshot": ({ projectId }) => { const db = board(); const project = getProjectById(db, projectId); - if (project === undefined) throw new Error("Unknown project"); + if (project === undefined || isProjectRemoved(db, projectId)) { + throw new Error("Unknown project"); + } return { project, tickets: listTicketsByProject(db, projectId), @@ -565,7 +578,7 @@ export function createBoardHandlers(options: BoardCommandOptions): BoardHandlerS // roster would clear a live slice off a board (VC-387). "board.roster": ({ projectId }) => { const db = board(); - if (getProjectById(db, projectId) === undefined) throw new Error("Unknown project"); + if (!isProjectAttached(db, projectId)) throw new Error("Unknown project"); return { tickets: listTicketRosterByProject(db, projectId), labels: listLabelsByProject(db, projectId), diff --git a/packages/host-core/src/db/export.test.ts b/packages/host-core/src/db/export.test.ts index 8553fc370..377bc8407 100644 --- a/packages/host-core/src/db/export.test.ts +++ b/packages/host-core/src/db/export.test.ts @@ -288,6 +288,9 @@ describe("buildExportDocument — populated db", () => { rowVersion: 4, createdAt: project.createdAt, updatedAt: 62, + // On the rail: removal detaches a project rather than deleting it + // (migration 064, VC-822). + removedAt: null, }, ]); diff --git a/packages/host-core/src/db/export.ts b/packages/host-core/src/db/export.ts index cb690d45e..bd5305080 100644 --- a/packages/host-core/src/db/export.ts +++ b/packages/host-core/src/db/export.ts @@ -127,6 +127,12 @@ export interface ExportProject { rowVersion: number; createdAt: number; updatedAt: number; + /** + * When the person removed the project (migration 064), or `null` while it + * is on the rail. A removed project is detached, not deleted (VC-822), so + * the export carries it with everything it held. + */ + removedAt: number | null; } export interface ExportTicket { @@ -419,6 +425,7 @@ interface ProjectRow { row_version: number; created_at: number; updated_at: number; + removed_at: number | null; } /** @@ -460,6 +467,7 @@ function exportProjects(db: Database.Database): ExportProject[] { rowVersion: row.row_version, createdAt: row.created_at, updatedAt: row.updated_at, + removedAt: row.removed_at, })); } diff --git a/packages/host-core/src/db/migrations-authority-collision.test.ts b/packages/host-core/src/db/migrations-authority-collision.test.ts index aef43ce8c..b3d1bf28e 100644 --- a/packages/host-core/src/db/migrations-authority-collision.test.ts +++ b/packages/host-core/src/db/migrations-authority-collision.test.ts @@ -82,8 +82,8 @@ function schema(table: string): unknown[] { .all(table); } -function expectConverged(): void { - expect(db.pragma("user_version", { simple: true })).toBe(LATEST_SCHEMA_VERSION); +function expectConverged(version: number = LATEST_SCHEMA_VERSION): void { + expect(db.pragma("user_version", { simple: true })).toBe(version); expect((db.pragma("table_info(projects)") as { name: string }[]).map((c) => c.name)).toContain( "decision_model", ); @@ -123,11 +123,21 @@ describe("053 collision: decision models and remembered authority approvals", () const project = db.prepare("SELECT * FROM projects").all(); const session = db.prepare("SELECT * FROM sessions").all(); const projectSchema = schema("projects"); - expect(migrate(db, dbPath)).toBe(true); - expectConverged(); + // The repair itself, judged where it ends: 054. + expect(migrate(db, dbPath, { toVersion: 54 })).toBe(true); + expectConverged(54); expect(db.prepare("SELECT * FROM projects").all()).toEqual(project); expect(db.prepare("SELECT * FROM sessions").all()).toEqual(session); expect(schema("projects")).toEqual(projectSchema); + // And on to the head, where a later expand-only migration only adds a + // column: 064's `removed_at` (VC-822), NULL for every project on the rail. + expect(migrate(db, dbPath)).toBe(true); + expectConverged(); + expect(project).toHaveLength(1); + expect(db.prepare("SELECT * FROM projects").all()).toEqual([ + { ...(project[0] as object), removed_at: null }, + ]); + expect(db.prepare("SELECT * FROM sessions").all()).toEqual(session); }); it("repairs a branch-lineage database stamped 53 while preserving approval and decision history and schemas", () => { @@ -158,6 +168,8 @@ describe("053 collision: decision models and remembered authority approvals", () expect(db.prepare("SELECT * FROM projects").get()).toEqual({ ...project!, decision_model: null, + // Migration 064 (VC-822): on the rail. + removed_at: null, }); expect(db.prepare("SELECT * FROM sessions").all()).toEqual(session); expect(db.prepare("SELECT * FROM authority_approvals").all()).toEqual(approvals); diff --git a/packages/host-core/src/db/migrations.lock.json b/packages/host-core/src/db/migrations.lock.json index 12b60a875..5c248d708 100644 --- a/packages/host-core/src/db/migrations.lock.json +++ b/packages/host-core/src/db/migrations.lock.json @@ -62,5 +62,6 @@ "61": "66d193a9d03063fe386080ed1d1ad243e157bd51015ee4488d57c9903e399c94", "62": "363ba1553656c5c6fe5f44d63c46744cab83a18bbe722d7dfba02dc85250bd1b", "63": "65a707dd40e37965f3635a992872e40ae92745374b0ddc8fca6ae59e489f6330", + "64": "00e39d63c66fd4a477eaf78696dd0f377aa20df7652e6da348d332b0e3a56091", "format": 1 } diff --git a/packages/host-core/src/db/migrations.ts b/packages/host-core/src/db/migrations.ts index b6346bf1d..7b01ba406 100644 --- a/packages/host-core/src/db/migrations.ts +++ b/packages/host-core/src/db/migrations.ts @@ -17,6 +17,10 @@ import { BOARD_COMMAND_RECEIPTS_MIGRATION } from "./board-receipts-migration"; import { CLOUD_IDENTITY_MIGRATION } from "./cloud-identity-migration"; import { HOST_NOTICE_OUTBOX_MIGRATION } from "./host-notice-outbox-migration"; +import { + applyProjectRemovedColumn, + PROJECT_REMOVED_COLUMN_MIGRATION, +} from "./project-removed-migration"; import { SESSION_FOLLOW_UP_MIGRATION } from "./session-follow-up-migration"; import { applySpawnLedgerParkedColumn, @@ -2809,6 +2813,12 @@ export const MIGRATIONS: readonly Migration[] = [ sql: SPAWN_LEDGER_PARKED_COLUMN_MIGRATION, apply: applySpawnLedgerParkedColumn, }, + { + version: 64, + name: "projects.removed_at — removing a project detaches it instead of deleting (VC-822)", + sql: PROJECT_REMOVED_COLUMN_MIGRATION, + apply: applyProjectRemovedColumn, + }, ]; /** diff --git a/packages/host-core/src/db/project-removed-migration.ts b/packages/host-core/src/db/project-removed-migration.ts new file mode 100644 index 000000000..ccd4e1f2c --- /dev/null +++ b/packages/host-core/src/db/project-removed-migration.ts @@ -0,0 +1,30 @@ +import type Database from "better-sqlite3"; + +/** + * VC-822: expand-only. `projects.removed_at` — when the person removed the + * project from Volli, or NULL while it is on the rail. + * + * Removing a project used to delete its row, and the foreign-key cascades + * took every ticket, comment and Session of the project with it. Removal now + * detaches the project instead: the row and everything that hangs off it + * stay, every listing hides it, and adding the same folder again re-attaches + * it with its board and Session history intact. + * + * A nullable column with no default: every existing row reads as "on the + * rail". An older build names its columns in every INSERT and does not read + * this one, so it shows a detached project as an ordinary one: nothing is + * lost, the project only reappears on that build's rail. + */ +export const PROJECT_REMOVED_COLUMN_MIGRATION = ` +ALTER TABLE projects ADD COLUMN removed_at INTEGER; +`; + +/** + * Probe-gated like 053's column addition: `ADD COLUMN` throws on a column that + * is already there, and a lineage can be re-offered a version it already ran. + */ +export function applyProjectRemovedColumn(db: Database.Database): void { + const columns = db.pragma("table_info(projects)") as { name: string }[]; + if (columns.some(({ name }) => name === "removed_at")) return; + db.exec(PROJECT_REMOVED_COLUMN_MIGRATION); +} diff --git a/packages/host-core/src/db/projects-repo.test.ts b/packages/host-core/src/db/projects-repo.test.ts index 6c7d47a22..545fd401f 100644 --- a/packages/host-core/src/db/projects-repo.test.ts +++ b/packages/host-core/src/db/projects-repo.test.ts @@ -5,10 +5,18 @@ import { testProject, testSession, testTicket, openTestDb } from "./test-helpers import type { TestDb } from "./test-helpers"; import { DEFAULT_AUTHORITY_POLICY } from "@volli/shared"; import { + countProjects, deleteProject, + detachProject, + findProjectByPath, + findRemovedProjectByPrefix, getProjectAuthorityPolicy, getProjectById, insertProject, + isProjectAttached, + isProjectRemoved, + listProjects, + reattachProject, updateProjectAuthorityPolicy, updateProjectSkillModes, updateProjectSessionDefaults, @@ -72,6 +80,66 @@ describe("deleteProject", () => { }); }); +/** + * VC-822: removing a project detaches it (migration 064). Every listing stops + * returning it while its row, and everything hanging off it, stays. + */ +describe("detachProject / reattachProject", () => { + function rowOf(id: string) { + return ctx.db + .prepare("SELECT removed_at, sort_order, row_version, updated_at FROM projects WHERE id = ?") + .get(id); + } + + it("hides a removed project from every listing, but not from an id or path lookup", () => { + ctx = openTestDb(); + insertProject(ctx.db, testProject({ id: "a", path: "/a", sortOrder: 0, ticketPrefix: "AA" })); + insertProject(ctx.db, testProject({ id: "b", path: "/b", sortOrder: 1, ticketPrefix: "BB" })); + insertTicket(ctx.db, testTicket("a", { id: "t1" })); + + expect(detachProject(ctx.db, "a", 50)).toBe(true); + + expect(listProjects(ctx.db).map(({ id }) => id)).toEqual(["b"]); + expect(countProjects(ctx.db)).toBe(1); + expect(countProjects(ctx.db, "all")).toBe(2); + expect(isProjectAttached(ctx.db, "a")).toBe(false); + expect(isProjectAttached(ctx.db, "b")).toBe(true); + expect(isProjectAttached(ctx.db, "missing")).toBe(false); + // Removed is its own answer: an unknown id is not a removed project. + expect(isProjectRemoved(ctx.db, "a")).toBe(true); + expect(isProjectRemoved(ctx.db, "b")).toBe(false); + expect(isProjectRemoved(ctx.db, "missing")).toBe(false); + // It keeps its prefix for its return; one on the rail is not "removed". + expect(findRemovedProjectByPrefix(ctx.db, "AA")).toMatchObject({ id: "a" }); + expect(findRemovedProjectByPrefix(ctx.db, "BB")).toBeUndefined(); + expect(getProjectById(ctx.db, "a")).toMatchObject({ id: "a" }); + expect(findProjectByPath(ctx.db, "/a")).toMatchObject({ id: "a" }); + expect(rowOf("a")).toEqual({ removed_at: 50, sort_order: 0, row_version: 2, updated_at: 50 }); + // Nothing cascaded. + expect(ctx.db.prepare("SELECT id FROM tickets WHERE project_id = 'a'").all()).toEqual([ + { id: "t1" }, + ]); + }); + + it("detaches once, and re-attaches only a removed project, at the end of the rail", () => { + ctx = openTestDb(); + insertProject(ctx.db, testProject({ id: "a", path: "/a", sortOrder: 0 })); + insertProject(ctx.db, testProject({ id: "b", path: "/b", sortOrder: 1 })); + + expect(reattachProject(ctx.db, "a", 60)).toBe(false); + expect(detachProject(ctx.db, "a", 50)).toBe(true); + expect(detachProject(ctx.db, "a", 55)).toBe(false); + expect(detachProject(ctx.db, "missing", 55)).toBe(false); + expect(rowOf("a")).toMatchObject({ removed_at: 50 }); + + expect(reattachProject(ctx.db, "a", 60)).toBe(true); + expect(reattachProject(ctx.db, "missing", 60)).toBe(false); + + expect(rowOf("a")).toEqual({ removed_at: null, sort_order: 2, row_version: 3, updated_at: 60 }); + expect(listProjects(ctx.db).map(({ id }) => id)).toEqual(["b", "a"]); + }); +}); + describe("per-project agent configuration (migration 023)", () => { it("reads a fresh project as inheriting its active defaults", () => { ctx = openTestDb(); diff --git a/packages/host-core/src/db/projects-repo.ts b/packages/host-core/src/db/projects-repo.ts index c96d44d98..4c802679a 100644 --- a/packages/host-core/src/db/projects-repo.ts +++ b/packages/host-core/src/db/projects-repo.ts @@ -66,6 +66,13 @@ interface ProjectRow { * read it. */ next_ticket_number: number; + /** + * Migration 064 — when the person removed the project, or NULL while it is + * on the rail (VC-822). Like `next_ticket_number`, not on the domain + * `Project`: a detached project is one no listing returns, so nothing that + * holds a `Project` has to ask. + */ + removed_at: number | null; } /** @@ -174,23 +181,101 @@ function readDecisionModelColumn(value: string | null): DecisionModelSetting | n return parseDecisionModelSetting(parseJsonColumn(value)) ?? NO_DECISION_MODEL; } -/** Every project, ordered by rail position. */ +/** + * Every project on the rail, ordered by rail position. A removed project is + * detached, not deleted (VC-822), and no listing returns it: this is the one + * read every rail, workspace list, worktree allow-list and CLI door builds on. + */ export function listProjects(db: Database.Database): Project[] { - const rows = prepared<[], ProjectRow>(db, "SELECT * FROM projects ORDER BY sort_order").all(); + const rows = prepared<[], ProjectRow>( + db, + "SELECT * FROM projects WHERE removed_at IS NULL ORDER BY sort_order", + ).all(); return rows.map(mapProject); } -export function countProjects(db: Database.Database): number { - const row = prepared<[], { count: number }>(db, "SELECT COUNT(*) as count FROM projects").get(); +/** + * How many projects are on the rail, or with `"all"` how many rows the table + * holds, detached ones included: an import that must only ever fill a + * genuinely empty table asks the second question. + */ +export function countProjects( + db: Database.Database, + scope: "attached" | "all" = "attached", +): number { + const row = prepared<[], { count: number }>( + db, + scope === "all" + ? "SELECT COUNT(*) as count FROM projects" + : "SELECT COUNT(*) as count FROM projects WHERE removed_at IS NULL", + ).get(); return row?.count ?? 0; } +/** + * Whether `id` names a project on the rail: false for an unknown id and for + * a removed (detached) one. What starts work in a project asks this, so a + * removed project's Automations, armed columns and Sessions stay still. + */ +export function isProjectAttached(db: Database.Database, id: string): boolean { + return ( + prepared<[string], { id: string }>( + db, + "SELECT id FROM projects WHERE id = ? AND removed_at IS NULL", + ).get(id) !== undefined + ); +} + +/** + * Whether `id` names a project that was removed: its row is there, detached. + * An unknown id answers false, so a door can keep its own wording for "no such + * project" and say "removed" only when that is what happened. + */ +export function isProjectRemoved(db: Database.Database, id: string): boolean { + return ( + prepared<[string], { id: string }>( + db, + "SELECT id FROM projects WHERE id = ? AND removed_at IS NOT NULL", + ).get(id) !== undefined + ); +} + +/** + * The project tracked at `path`, on the rail or removed: the path stays the + * row's (`UNIQUE`), so adding the folder again finds the removed project and + * re-attaches it rather than registering a second one. + * + * The match is the exact stored string. A spelling of the same folder that + * differs (another letter case on a case-insensitive volume, a symlink to it) + * is a different path here, and registers a second project, as it always has. + */ export function findProjectByPath(db: Database.Database, path: string): Project | undefined { const row = prepared<[string], ProjectRow>(db, "SELECT * FROM projects WHERE path = ?").get(path); return row ? mapProject(row) : undefined; } -/** One project by id — used by the artifacts IPC handlers to resolve a ticket's project path. */ +/** + * The removed project that holds `ticketPrefix`, if one does. A removed + * project keeps its prefix for its return (VC-822): a new project may not take + * it, or adding the old folder again would meet a prefix on the rail it can + * never get back, since a prefix cannot be changed. + */ +export function findRemovedProjectByPrefix( + db: Database.Database, + ticketPrefix: string, +): Project | undefined { + const row = prepared<[string], ProjectRow>( + db, + "SELECT * FROM projects WHERE ticket_prefix = ? AND removed_at IS NOT NULL ORDER BY sort_order", + ).get(ticketPrefix); + return row ? mapProject(row) : undefined; +} + +/** + * One project by id, removed or not — used by the artifacts IPC handlers to + * resolve a ticket's project path. An id lookup is not a listing: a removed + * project's Sessions and tickets still name it. + */ export function getProjectById(db: Database.Database, id: string): Project | undefined { const row = prepared<[string], ProjectRow>(db, "SELECT * FROM projects WHERE id = ?").get(id); return row ? mapProject(row) : undefined; @@ -534,6 +619,39 @@ export function deleteProject(db: Database.Database, id: string): void { prepared(db, "DELETE FROM projects WHERE id = ?").run(id); } +/** + * Removes a project from the rail without deleting it (VC-822): the row, its + * tickets, Sessions and their history all stay, and every listing stops + * returning it. Answers whether this call detached it; an unknown or already + * removed project is left as it is. + */ +export function detachProject(db: Database.Database, id: string, now: number): boolean { + return ( + prepared( + db, + `UPDATE projects + SET removed_at = ?, row_version = row_version + 1, updated_at = ? + WHERE id = ? AND removed_at IS NULL`, + ).run(now, now, id).changes > 0 + ); +} + +/** + * Puts a removed project back on the rail, at its end, with everything it + * held when it was removed. Answers whether this call re-attached it; an + * unknown project, or one already on the rail, is left as it is. + */ +export function reattachProject(db: Database.Database, id: string, now: number): boolean { + return ( + prepared( + db, + `UPDATE projects + SET removed_at = NULL, sort_order = ?, row_version = row_version + 1, updated_at = ? + WHERE id = ? AND removed_at IS NOT NULL`, + ).run(nextSortOrder(db), now, id).changes > 0 + ); +} + /** * Rewrites `sort_order` to `0..n-1` following `orderedIds`; unknown ids are * silently no-ops. Wrapped in a transaction so the N row updates commit diff --git a/packages/host-core/src/entries/db.ts b/packages/host-core/src/entries/db.ts index e2b6327c5..5792029d3 100644 --- a/packages/host-core/src/entries/db.ts +++ b/packages/host-core/src/entries/db.ts @@ -58,6 +58,7 @@ export { deleteProject, getProjectById, insertProject, + isProjectAttached, listProjects, reorderProjects, updateProjectAppearance, diff --git a/packages/host-core/src/handlers/host-handlers.test.ts b/packages/host-core/src/handlers/host-handlers.test.ts index 2d793cb2a..e19e3e08f 100644 --- a/packages/host-core/src/handlers/host-handlers.test.ts +++ b/packages/host-core/src/handlers/host-handlers.test.ts @@ -799,6 +799,10 @@ describe("desktop-only commands", () => { expect(stopped).toEqual(["bindings", "shells", "terminals", "tabs"]); expect(listProjects(ctx.db).map(({ id }) => id)).toEqual(["second"]); + // Detached at the host's clock, not deleted (VC-822). + expect(ctx.db.prepare("SELECT removed_at FROM projects WHERE id = ?").get(PROJECT)).toEqual({ + removed_at: 50, + }); expect(boardFeed.size).toBe(0); expect(publish.mock.calls).toEqual([["data-changed", {}]]); }); diff --git a/packages/host-core/src/handlers/host-handlers.ts b/packages/host-core/src/handlers/host-handlers.ts index 8efd209b1..d8181e61a 100644 --- a/packages/host-core/src/handlers/host-handlers.ts +++ b/packages/host-core/src/handlers/host-handlers.ts @@ -290,7 +290,7 @@ export interface HostHandlerSignatures extends BoardHandlerSignatures { { shellId: string } >; readonly "project.reorder": HostHandler<{ orderedIds: readonly string[] }, null>; - /** Stops a project's live work, then deletes it (VC-796): `../project-remove`. */ + /** Stops a project's live work, then detaches it (VC-796, VC-822): `../project-remove`. */ readonly "project.remove": HostHandler<{ projectId: string }, null>; readonly "worktree.trimSettings": HostHandler; /** @@ -501,9 +501,9 @@ export interface HostHandlerOptions { readonly piSessionsDirectory?: string; /** * The live work this host holds for a project's Sessions, which removing - * the project stops before its delete (VC-796). Absent or null: a host that - * cannot stop it, where `project.remove` answers unavailable rather than - * deleting Sessions out from under their work. + * the project stops before it detaches it (VC-796, VC-822). Absent or null: + * a host that cannot stop it, where `project.remove` answers unavailable + * rather than hiding Sessions that are still working. */ readonly projectLiveWork?: ProjectLiveWork | null; /** @@ -852,6 +852,7 @@ function hostHandlerEntries( live: present(options.projectLiveWork ?? null, PROJECT_REMOVAL_UNAVAILABLE), disposeFeed: (id) => feed.dispose(id), announce: () => feed.untapped(() => ports.events.publish("data-changed", {})), + now, }, projectId, ); diff --git a/packages/host-core/src/mcp/settings.test.ts b/packages/host-core/src/mcp/settings.test.ts index 3800899a3..007e1ed10 100644 --- a/packages/host-core/src/mcp/settings.test.ts +++ b/packages/host-core/src/mcp/settings.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; import { MCP_CONNECTION_TIMEOUT_MS, MCP_ERROR_MAX_CHARS } from "@volli/shared"; import type { McpProtocolClient } from "./discovery"; -import { deleteProject, insertProject } from "../db/projects-repo"; +import { detachProject, insertProject } from "../db/projects-repo"; import { MemoryMcpCredentialStore } from "./credential-store"; import { openTestDb, testProject, type TestDb } from "../db/test-helpers"; import { McpSettingsService, MCP_CANCELLED_MESSAGE } from "./settings"; @@ -255,10 +255,12 @@ describe("McpSettingsService", () => { }); }); -// VC-796: a removed project's servers go with its row (ON DELETE CASCADE), so -// their credentials have to go by name, and only once the row is gone. +// VC-796, VC-822: a removed project is detached, not deleted, so its server +// rows stay; but no Settings screen shows them any more, and a credential +// nobody can see is one nobody would think to revoke. They go by name, once +// the project is off the rail. describe("McpSettingsService on a project's removal", () => { - it("names the project's servers, then forgets only the credentials whose row is gone", async () => { + it("names the project's servers, then forgets the credentials of every one off the rail", async () => { const credentials = new MemoryMcpCredentialStore(); const settings = new McpSettingsService({ db: ctx.db, @@ -279,17 +281,22 @@ describe("McpSettingsService on a project's removal", () => { }); credentials.update("server-1", () => ({ secrets: { TOKEN: "one" } })); credentials.update("server-2", () => ({ secrets: { TOKEN: "two" } })); + // A record whose server row is gone altogether is no one's either. + credentials.update("server-gone", () => ({ secrets: { TOKEN: "gone" } })); const serverIds = settings.disconnectProject("p1"); expect(serverIds).toEqual(["server-1"]); - // Before the delete commits, nothing is forgotten. + // Before the removal commits, nothing is forgotten. settings.forgetRemovedServers(serverIds); expect(credentials.read("server-1")).toEqual({ secrets: { TOKEN: "one" } }); - deleteProject(ctx.db, "p1"); - settings.forgetRemovedServers([...serverIds, "server-2"]); + detachProject(ctx.db, "p1", 200); + settings.forgetRemovedServers([...serverIds, "server-2", "server-gone"]); expect(credentials.read("server-1")).toBeUndefined(); + expect(credentials.read("server-gone")).toBeUndefined(); expect(credentials.read("server-2")).toEqual({ secrets: { TOKEN: "two" } }); + // The server itself is kept for the project's return, without its secrets. + expect(settings.list("p1").map(({ id }) => id)).toEqual(["server-1"]); }); }); diff --git a/packages/host-core/src/mcp/settings.ts b/packages/host-core/src/mcp/settings.ts index 68c086f50..84d4f31a0 100644 --- a/packages/host-core/src/mcp/settings.ts +++ b/packages/host-core/src/mcp/settings.ts @@ -23,7 +23,7 @@ import { putMcpServer, selectedMcpToolDefinitions, } from "../db/mcp-servers-repo"; -import { getProjectById } from "../db/projects-repo"; +import { getProjectById, isProjectAttached } from "../db/projects-repo"; import { createMcpProtocolClientOpener } from "./client"; import { MemoryMcpCredentialStore, type McpCredentialStore } from "./credential-store"; import { @@ -611,7 +611,7 @@ export class McpSettingsService { /** * Before a project is removed (VC-796): ends every sign-in still in flight * for its servers, and names them so {@link forgetRemovedServers} can drop - * their credentials once the cascade has taken the rows. A Session's own + * their credentials once the project is detached. A Session's own * connections are its attachment's, and close when its binding is released. */ disconnectProject(projectId: string): readonly string[] { @@ -622,12 +622,17 @@ export class McpSettingsService { /** * After a project is removed: every credential its servers held, for the - * reason {@link remove} drops one server's. A server whose row is still - * there (the removal did not commit) keeps its own. + * reason {@link remove} drops one server's. A removed project is detached, + * not deleted (VC-822), so its server rows stay, but no Settings screen + * shows them any more. A server whose project is still on the rail (the + * removal did not commit) keeps its own. */ forgetRemovedServers(serverIds: readonly string[]): void { for (const serverId of serverIds) { - if (getMcpServer(this.#db, serverId) === undefined) this.#store.delete(serverId); + const server = getMcpServer(this.#db, serverId); + if (server === undefined || !isProjectAttached(this.#db, server.projectId)) { + this.#store.delete(serverId); + } } } diff --git a/packages/host-core/src/operator-door.test.ts b/packages/host-core/src/operator-door.test.ts index 1f89ad1ac..0bc8ac001 100644 --- a/packages/host-core/src/operator-door.test.ts +++ b/packages/host-core/src/operator-door.test.ts @@ -25,8 +25,10 @@ import { createAgentCommandService } from "./agent-commands"; import { testHostHandlers } from "./testing/host-handlers"; import type { OperatorWriteRecord } from "./agent-dispatch/context"; import { + detachProject, findProjectByPath, insertProject, + isProjectAttached, listProjects, updateProjectAuthorityPolicy, } from "./db/projects-repo"; @@ -246,6 +248,36 @@ describe("project add", () => { }); }); + // VC-822: removal detaches a project, so the folder's own project comes back. + it("previews, then re-attaches, a removed project's folder with its tickets", async () => { + const { run, operator } = scenario(); + const path = folder("Echo"); + insertProject( + ctx.db, + testProject({ id: "project-echo", name: "Echo", path, ticketPrefix: "EC" }), + ); + insertTicket(ctx.db, testTicket("project-echo", { id: "ticket-echo", ticketNumber: 3 })); + detachProject(ctx.db, "project-echo", 50); + + const preview = await run("project.add", { id: path, dryRun: true }, operator); + expect(preview).toMatchObject({ + ok: true, + data: { + target: { id: "project-echo", label: "Echo" }, + durableWrites: [{ resource: "project", operation: "update" }], + }, + }); + expect(isProjectAttached(ctx.db, "project-echo")).toBe(false); + + expect(await run("project.add", { id: path }, operator)).toMatchObject({ + ok: true, + data: { created: false, project: { id: "project-echo", prefix: "EC" } }, + }); + expect(listTicketsByProject(ctx.db, "project-echo").map(({ id }) => id)).toEqual([ + "ticket-echo", + ]); + }); + it("refuses what the app refuses, with the app's sentences", async () => { const { run, operator } = scenario(); const parent = folder("x"); diff --git a/packages/host-core/src/project-create.test.ts b/packages/host-core/src/project-create.test.ts index b1bf55d2e..80ab0cdcb 100644 --- a/packages/host-core/src/project-create.test.ts +++ b/packages/host-core/src/project-create.test.ts @@ -8,8 +8,16 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; -import { findProjectByPath, insertProject, listProjects } from "./db/projects-repo"; -import { openTestDb, testProject } from "./db/test-helpers"; +import { + detachProject, + findProjectByPath, + insertProject, + isProjectAttached, + listProjects, + listProjectSessionIds, +} from "./db/projects-repo"; +import { insertTicket, listAllTickets } from "./db/tickets-repo"; +import { openTestDb, testProject, testTicket } from "./db/test-helpers"; import type { TestDb } from "./db/test-helpers"; import { createProject, planProjectCreate } from "./project-create"; @@ -148,3 +156,124 @@ describe("planProjectCreate", () => { expect(listProjects(ctx.db)).toEqual([]); }); }); + +/** + * VC-822: removing a project detaches it, so adding its folder again finds the + * same row and puts it back on the rail with everything it held. + */ +describe("re-adding a removed project's folder", () => { + function removedProject(path: string): void { + insertProject( + ctx.db, + testProject({ id: "old", name: "Old", path, ticketPrefix: "OL", baseBranch: "trunk" }), + ); + insertTicket(ctx.db, testTicket("old", { id: "t1", ticketNumber: 4 })); + ctx.db.prepare("UPDATE projects SET next_ticket_number = 5 WHERE id = 'old'").run(); + ctx.db + .prepare( + "INSERT INTO sessions (id, project_id, ticket_id, title, created_at) VALUES ('s1', 'old', 't1', 's1', 1)", + ) + .run(); + insertProject(ctx.db, testProject({ id: "other", path: "/other", ticketPrefix: "OT" })); + detachProject(ctx.db, "old", 3); + } + + it("re-attaches the same project, its tickets, Sessions and counter included", async () => { + const path = dir("old"); + removedProject(path); + const detect = vi.fn(async () => "main"); + const onCreated = vi.fn(); + const ports = { db: ctx.db, detectBaseBranch: detect, now: () => 9, onCreated }; + + const outcome = await createProject(ports, { path, name: "Renamed" }); + + // The row it always was: its own id, name, prefix and base branch, at the + // end of the rail. Nothing is detected again. + expect(outcome).toMatchObject({ + ok: true, + created: false, + project: { id: "old", name: "Old", ticketPrefix: "OL", baseBranch: "trunk", updatedAt: 9 }, + }); + expect(listProjects(ctx.db).map(({ id }) => id)).toEqual(["other", "old"]); + expect(detect).not.toHaveBeenCalled(); + expect(listAllTickets(ctx.db).map(({ id }) => id)).toEqual(["t1"]); + expect(listProjectSessionIds(ctx.db, "old")).toEqual(["s1"]); + expect( + ctx.db.prepare("SELECT next_ticket_number, removed_at FROM projects WHERE id = 'old'").get(), + ).toEqual({ next_ticket_number: 5, removed_at: null }); + // Announced once, as a registration: every window re-reads its board. + expect(onCreated).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ id: "old" })); + + await createProject(ports, { path, name: "Renamed" }); + expect(onCreated).toHaveBeenCalledOnce(); + }); + + it("previews the re-attach without writing it", async () => { + const path = dir("old"); + removedProject(path); + + const plan = await planProjectCreate({ db: ctx.db }, { path, name: "Old" }, { write: false }); + + expect(plan).toMatchObject({ kind: "reattached", project: { id: "old" } }); + expect(isProjectAttached(ctx.db, "old")).toBe(false); + // A door that announces nothing still re-attaches it. + expect(await createProject({ db: ctx.db }, { path, name: "Old" })).toMatchObject({ + ok: true, + created: false, + }); + expect(isProjectAttached(ctx.db, "old")).toBe(true); + }); + + it("re-attaches once when the folder is added twice at the same time", async () => { + const path = dir("old"); + removedProject(path); + const onCreated = vi.fn(); + const ports = { db: ctx.db, onCreated }; + + const outcomes = await Promise.all([ + createProject(ports, { path, name: "Old" }), + createProject(ports, { path, name: "Old" }), + ]); + + expect(outcomes).toMatchObject([ + { ok: true, created: false, project: { id: "old" } }, + { ok: true, created: false, project: { id: "old" } }, + ]); + expect(onCreated).toHaveBeenCalledOnce(); + }); + + // Prefixes cannot be changed, so a new folder taking a removed project's + // prefix would leave the old folder impossible to add again. + it("refuses a new project the prefix a removed one keeps for its return", async () => { + removedProject(dir("old")); + + expect(await createProject({ db: ctx.db }, { path: dir("olive"), name: "Old Lane" })).toEqual({ + ok: false, + error: 'Ticket prefix "OL" is used by Old, which was removed. Add its folder again first.', + }); + expect(listProjects(ctx.db).map(({ id }) => id)).toEqual(["other"]); + }); + + it("refuses while a project on the rail holds its ticket prefix, or its folder is gone", async () => { + const path = dir("old"); + removedProject(path); + // An older build, which does not see the removal, could add a project with + // the prefix: two projects on the rail with one would make every display id + // ambiguous. + insertProject( + ctx.db, + testProject({ id: "new", name: "New", path: "/new", ticketPrefix: "OL" }), + ); + + expect(await createProject({ db: ctx.db }, { path, name: "Old" })).toEqual({ + ok: false, + error: 'Ticket prefix "OL" is already used by New.', + }); + rmSync(path, { recursive: true }); + expect(await createProject({ db: ctx.db }, { path, name: "Old" })).toEqual({ + ok: false, + error: "Project path does not exist", + }); + expect(isProjectAttached(ctx.db, "old")).toBe(false); + }); +}); diff --git a/packages/host-core/src/project-create.ts b/packages/host-core/src/project-create.ts index 56906330e..dcce439f7 100644 --- a/packages/host-core/src/project-create.ts +++ b/packages/host-core/src/project-create.ts @@ -8,6 +8,11 @@ * existing directory, the base branch is detected, and the ticket prefix the * name derives must be unique across the workspace. * + * A folder whose project was removed is re-attached rather than registered + * again (VC-822): removal only detaches a project, so its tickets, Sessions, + * ticket counter and Automations come back with it. A removed project keeps + * its prefix for that return: a new project may not take it. + * * The `stat` and the branch detection are awaited, never blocking: desktop * runs this on Electron main, and a folder on an unmounted volume is where a * synchronous read freezes the window. @@ -22,9 +27,13 @@ import type { Project } from "@volli/shared"; import { countProjects, findProjectByPath, + findRemovedProjectByPrefix, + getProjectById, insertProject, + isProjectAttached, listProjects, nextSortOrder, + reattachProject, } from "./db/projects-repo"; import { detectProjectBaseBranchAsync } from "./project-base-branch"; @@ -45,17 +54,22 @@ export interface ProjectCreatePorts { readonly detectBaseBranch?: (path: string) => Promise; readonly now?: () => number; readonly newId?: () => string; - /** Announces a newly committed registration once; never an existing row or a preview. */ + /** + * Announces a committed registration once: a new row, or a removed project + * re-attached. Never a project already on the rail, and never a preview. + */ readonly onCreated?: (project: Project) => void; } /** * What a registration decided: the project already tracked at the path, a - * refusal, or the new row — inserted, unless this was a preview. + * refusal, the removed project at the path put back on the rail, or the new + * row. A preview writes neither of the last two. */ export type ProjectCreatePlan = | { readonly kind: "existing"; readonly project: Project } | { readonly kind: "refused"; readonly error: string } + | { readonly kind: "reattached"; readonly project: Project } | { readonly kind: "new"; readonly project: Project }; /** @@ -73,7 +87,8 @@ export async function planProjectCreate( ): Promise { const { db } = ports; const existing = findProjectByPath(db, input.path); - if (existing) return { kind: "existing", project: existing }; + if (existing && isProjectAttached(db, existing.id)) + return { kind: "existing", project: existing }; let stats; try { stats = await stat(input.path); @@ -81,6 +96,7 @@ export async function planProjectCreate( return { kind: "refused", error: "Project path does not exist" }; } if (!stats.isDirectory()) return { kind: "refused", error: "Project path is not a directory" }; + if (existing) return reattach(ports, existing.id, options); const baseBranch = await (ports.detectBaseBranch ?? detectProjectBaseBranchAsync)(input.path); // Detection yields to other requests. Re-read mutable project state only // after it returns, then validate and insert without another await. @@ -89,6 +105,15 @@ export async function planProjectCreate( const ticketPrefix = derivePrefix(input.name); const prefixValidation = validateUniquePrefix(ticketPrefix, listProjects(db)); if (!prefixValidation.ok) return { kind: "refused", error: prefixValidation.error }; + // A removed project keeps its prefix (VC-822): taking it here would leave + // that project's folder impossible to add again. + const removedHolder = findRemovedProjectByPrefix(db, ticketPrefix); + if (removedHolder !== undefined) { + return { + kind: "refused", + error: `Ticket prefix "${ticketPrefix}" is used by ${removedHolder.name}, which was removed. Add its folder again first.`, + }; + } const now = (ports.now ?? Date.now)(); const project: Project = { id: (ports.newId ?? randomUUID)(), @@ -108,6 +133,32 @@ export async function planProjectCreate( return { kind: "new", project }; } +/** + * The removed project at the folder, put back on the rail with its own id, + * base branch and ticket counter. Asked again after the `stat` yielded, with + * no await from here to the write: a concurrent add may have re-attached it + * already. Its ticket prefix has to still be its own on the rail. A new + * project cannot take a removed one's, but an older build, which does not see + * the removal, can; two projects on the rail with one prefix would make every + * display id ambiguous. + */ +function reattach( + ports: ProjectCreatePorts, + removedId: string, + options: { readonly write: boolean }, +): ProjectCreatePlan { + const { db } = ports; + const current = getProjectById(db, removedId)!; + if (isProjectAttached(db, removedId)) return { kind: "existing", project: current }; + const prefixValidation = validateUniquePrefix(current.ticketPrefix, listProjects(db), removedId); + if (!prefixValidation.ok) return { kind: "refused", error: prefixValidation.error }; + if (!options.write) return { kind: "reattached", project: current }; + reattachProject(db, removedId, (ports.now ?? Date.now)()); + const project = getProjectById(db, removedId)!; + ports.onCreated?.(project); + return { kind: "reattached", project }; +} + /** Creates the project at `path`, or answers with the one already tracked there. */ export async function createProject( ports: ProjectCreatePorts, diff --git a/packages/host-core/src/project-relink.test.ts b/packages/host-core/src/project-relink.test.ts index 63d0d55ad..b301cff19 100644 --- a/packages/host-core/src/project-relink.test.ts +++ b/packages/host-core/src/project-relink.test.ts @@ -14,7 +14,7 @@ import { afterEach, describe, expect, it } from "vite-plus/test"; import type { AgentRequest } from "@volli/shared"; -import { insertProject, getProjectById, listProjects } from "./db/projects-repo"; +import { detachProject, insertProject, getProjectById, listProjects } from "./db/projects-repo"; import { projectForCreate } from "./agent-dispatch/resolution"; import { insertTicket, listTicketsByProject, listWorktreePathsByProject } from "./db/tickets-repo"; import { openTestDb, testProject, testTicket } from "./db/test-helpers"; @@ -96,6 +96,25 @@ describe("relinkProject", () => { expect(getProjectById(ctx.db, "p2")?.path).toBe("/Users/me/atlas"); }); + // VC-822: a removed project is detached, not deleted, and keeps its path for + // its return. Relinking onto it used to fail on the path's UNIQUE constraint. + it("refuses the folder of a removed project, naming it, and changes nothing", async () => { + ctx = openTestDb(); + insertProject(ctx.db, testProject({ id: "p1", name: "Volli", path: "/Users/me/volli" })); + insertProject(ctx.db, testProject({ id: "p2", name: "Old", path: "/Users/me/old" })); + detachProject(ctx.db, "p2", 5); + + const outcome = await relinkProject(deps(), { projectId: "p1", path: "/Users/me/old" }); + + expect(outcome).toEqual({ + ok: false, + refusal: "claimed", + error: "This folder belongs to Old, which was removed. Add it again instead.", + }); + expect(getProjectById(ctx.db, "p1")?.path).toBe("/Users/me/volli"); + expect(getProjectById(ctx.db, "p2")?.path).toBe("/Users/me/old"); + }); + // macOS is case-insensitive by default, so these two paths name ONE folder. // The strings differ, so only the disk can say they are the same — and it is // the disk this asks. Without it, two rows end up on one checkout and every diff --git a/packages/host-core/src/project-relink.ts b/packages/host-core/src/project-relink.ts index f080c2c4a..5d0a177ae 100644 --- a/packages/host-core/src/project-relink.ts +++ b/packages/host-core/src/project-relink.ts @@ -39,7 +39,13 @@ import type { ProjectRelinkSubject, } from "@volli/shared"; -import { getProjectById, listProjects, updateProjectPath } from "./db/projects-repo"; +import { + findProjectByPath, + getProjectById, + isProjectRemoved, + listProjects, + updateProjectPath, +} from "./db/projects-repo"; import { runGitCapturingAsync } from "./worktree/git"; import { listWorktreeSitesByProject, updateTicketFields } from "./db/tickets-repo"; import { busySiteWithin } from "./worktree/activity"; @@ -176,6 +182,16 @@ export async function relinkProject( if (!validation.ok) { return { ok: false, error: validation.error, refusal: validation.refusal }; } + // A removed project keeps its folder's path for its return (VC-822): the + // folder is that project's to be added again, not this one's to take over. + const holder = findProjectByPath(deps.db, validation.path); + if (holder !== undefined && isProjectRemoved(deps.db, holder.id)) { + return { + ok: false, + refusal: "claimed", + error: `This folder belongs to ${holder.name}, which was removed. Add it again instead.`, + }; + } const relinked = updateProjectPath(deps.db, project.id, validation.path, now()); // `getProjectById` answered for this id a moment ago and nothing deletes a diff --git a/packages/host-core/src/project-remove.test.ts b/packages/host-core/src/project-remove.test.ts index a20de3dc8..b636455f1 100644 --- a/packages/host-core/src/project-remove.test.ts +++ b/packages/host-core/src/project-remove.test.ts @@ -1,16 +1,24 @@ /** - * Removing a project (VC-796): its live work stops BEFORE the cascade takes - * its Sessions, and the windows hear about it after. + * Removing a project (VC-796, VC-822): its live work stops while it is still + * on the rail, then it is DETACHED, keeping its tickets and Sessions, and the + * windows hear about it after. * * Every port records the step it was called at together with what the - * database still held then, so "before the delete" is read off the database - * rather than inferred from call order alone. + * database held then, so "before the detach" is read off the database rather + * than inferred from call order alone. */ import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test"; import type { OpenNativeBinding } from "@volli/session-engine"; -import { getProjectById, insertProject, listProjectSessionIds } from "./db/projects-repo"; -import { openTestDb, testProject, type TestDb } from "./db/test-helpers"; +import { + getProjectById, + insertProject, + isProjectAttached, + listProjects, + listProjectSessionIds, +} from "./db/projects-repo"; +import { insertTicket } from "./db/tickets-repo"; +import { openTestDb, testProject, testTicket, type TestDb } from "./db/test-helpers"; import { hostProjectLiveWork, ProjectRemovalRefused, @@ -39,10 +47,18 @@ function insertSession(id: string, projectId: string): void { .run(id, projectId, id); } +function removedAt(projectId: string): number | null { + return ( + ctx.db.prepare("SELECT removed_at FROM projects WHERE id = ?").get(projectId) as { + removed_at: number | null; + } + ).removed_at; +} + interface Step { step: string; - /** Whether the project row was still there when the step ran. */ - projectRow: boolean; + /** Whether the project was still on the rail when the step ran. */ + onRail: boolean; /** The project's Session rows when the step ran. */ sessions: readonly string[]; arg?: unknown; @@ -53,7 +69,7 @@ function recorder(overrides: Partial = {}) { const note = (step: string, arg?: unknown) => { steps.push({ step, - projectRow: getProjectById(ctx.db, "p1") !== undefined, + onRail: isProjectAttached(ctx.db, "p1"), sessions: listProjectSessionIds(ctx.db, "p1"), ...(arg === undefined ? {} : { arg }), }); @@ -80,6 +96,7 @@ function recorder(overrides: Partial = {}) { live, disposeFeed: (projectId) => note("disposeFeed", projectId), announce: () => note("announce"), + now: () => 42, }; return { steps, ports }; } @@ -88,34 +105,58 @@ function seed(): void { ctx = openTestDb(); insertProject(ctx.db, testProject({ id: "p1", path: "/repo/one" })); insertProject(ctx.db, testProject({ id: "p2", path: "/repo/two" })); + insertTicket(ctx.db, testTicket("p1", { id: "t-live", ticketNumber: 1 })); + insertTicket(ctx.db, testTicket("p1", { id: "t-archived", ticketNumber: 2 })); + ctx.db.prepare("UPDATE tickets SET archived_at = 5 WHERE id = 't-archived'").run(); insertSession("s-board", "p1"); insertSession("s-ticket", "p1"); insertSession("s-other", "p2"); } describe("removeProject", () => { - it("stops every Session's live work while its rows still exist, then deletes, then tells", async () => { + it("stops every Session's live work while the project is on the rail, then detaches it", async () => { const { steps, ports } = recorder(); await removeProject(ports, "p1"); const both = ["s-board", "s-ticket"]; expect(steps).toEqual([ - { step: "releaseBindings", projectRow: true, sessions: both, arg: both }, - { step: "disposeShells", projectRow: true, sessions: both, arg: both }, - { step: "killTerminals", projectRow: true, sessions: both, arg: both }, - { step: "closeBrowserTabs", projectRow: true, sessions: both, arg: "p1" }, - { step: "mcp.disconnect", projectRow: true, sessions: both, arg: "p1" }, - // The cascade has run by now: the row and its Sessions are gone. - { step: "mcp.forget", projectRow: false, sessions: [], arg: ["server-1"] }, - { step: "disposeFeed", projectRow: false, sessions: [], arg: "p1" }, - { step: "announce", projectRow: false, sessions: [] }, + { step: "releaseBindings", onRail: true, sessions: both, arg: both }, + { step: "disposeShells", onRail: true, sessions: both, arg: both }, + { step: "killTerminals", onRail: true, sessions: both, arg: both }, + { step: "closeBrowserTabs", onRail: true, sessions: both, arg: "p1" }, + { step: "mcp.disconnect", onRail: true, sessions: both, arg: "p1" }, + // Detached by now, and nothing cascaded: its Sessions are all still there. + { step: "mcp.forget", onRail: false, sessions: both, arg: ["server-1"] }, + { step: "disposeFeed", onRail: false, sessions: both, arg: "p1" }, + { step: "announce", onRail: false, sessions: both }, ]); - expect(getProjectById(ctx.db, "p2")).toBeDefined(); + expect(isProjectAttached(ctx.db, "p2")).toBe(true); expect(listProjectSessionIds(ctx.db, "p2")).toEqual(["s-other"]); }); - it("refuses, stopping and deleting nothing else, when a binding will not close", async () => { + // The defect (VC-822): removal used to delete the row, and the cascades took + // every ticket, live or archived, and every Session with its history. + it("keeps the project's tickets and Sessions, and only hides it from the rail", async () => { + const { ports } = recorder(); + + await removeProject(ports, "p1"); + + expect(listProjects(ctx.db).map(({ id }) => id)).toEqual(["p2"]); + expect(getProjectById(ctx.db, "p1")).toMatchObject({ id: "p1", path: "/repo/one" }); + expect(removedAt("p1")).toBe(42); + expect( + ctx.db + .prepare("SELECT id, archived_at FROM tickets WHERE project_id = 'p1' ORDER BY id") + .all(), + ).toEqual([ + { id: "t-archived", archived_at: 5 }, + { id: "t-live", archived_at: null }, + ]); + expect(listProjectSessionIds(ctx.db, "p1")).toEqual(["s-board", "s-ticket"]); + }); + + it("refuses, stopping and detaching nothing else, when a binding will not close", async () => { const { steps, ports } = recorder({ releaseBindings: async () => ({ stillOpen: ["s-ticket"] }), }); @@ -127,7 +168,7 @@ describe("removeProject", () => { "A Session in this project is still running, so nothing was removed. Stop it and try again.", ); expect(steps).toEqual([]); - expect(getProjectById(ctx.db, "p1")).toBeDefined(); + expect(isProjectAttached(ctx.db, "p1")).toBe(true); expect(listProjectSessionIds(ctx.db, "p1").toSorted()).toEqual(["s-board", "s-ticket"]); }); @@ -147,13 +188,15 @@ describe("removeProject", () => { ); await expect(refusal).rejects.toMatchObject({ stillOpen: ["s-late", "s-later"] }); expect(steps).toEqual([]); - expect(getProjectById(ctx.db, "p1")).toBeDefined(); + expect(isProjectAttached(ctx.db, "p1")).toBe(true); }); - it("removes a project on a host that keeps no MCP settings", async () => { + it("removes a project on a host that keeps no MCP settings, stamped by the wall clock", async () => { const { steps, ports } = recorder({ mcp: null }); + const { now: _clock, ...unclocked } = ports; + const before = Date.now(); - await removeProject(ports, "p1"); + await removeProject(unclocked, "p1"); expect(steps.map(({ step }) => step)).toEqual([ "releaseBindings", @@ -163,16 +206,22 @@ describe("removeProject", () => { "disposeFeed", "announce", ]); - expect(getProjectById(ctx.db, "p1")).toBeUndefined(); + expect(isProjectAttached(ctx.db, "p1")).toBe(false); + expect(removedAt("p1")).toBeGreaterThanOrEqual(before); }); - it("does nothing for an id no project has", async () => { + it("does nothing for an id no project on the rail has", async () => { const { steps, ports } = recorder(); await removeProject(ports, "missing"); + await removeProject(ports, "p1"); + steps.length = 0; + // Removed already: there is nothing left to stop, and nothing to tell. + await removeProject({ ...ports, now: () => 99 }, "p1"); expect(steps).toEqual([]); - expect(getProjectById(ctx.db, "p1")).toBeDefined(); + expect(removedAt("p1")).toBe(42); + expect(isProjectAttached(ctx.db, "p2")).toBe(true); }); }); @@ -332,6 +381,7 @@ describe("hostProjectLiveWork", () => { "mcp.forget:server-1", ]); expect(disposed).toEqual(["p1"]); - expect(getProjectById(ctx.db, "p1")).toBeUndefined(); + expect(isProjectAttached(ctx.db, "p1")).toBe(false); + expect(listProjectSessionIds(ctx.db, "p1")).toEqual(["s-board", "s-ticket"]); }); }); diff --git a/packages/host-core/src/project-remove.ts b/packages/host-core/src/project-remove.ts index daf0fc808..3fdb583a9 100644 --- a/packages/host-core/src/project-remove.ts +++ b/packages/host-core/src/project-remove.ts @@ -1,34 +1,35 @@ /** - * Removing a project from the host (VC-796). + * Removing a project from the host (VC-796, VC-822). * - * The delete is one statement, and it cascades: every Session of the project - * and its whole event history go with the row (`sessions.project_id` and - * `session_events.session_id` are `ON DELETE CASCADE`). So the work those - * Sessions are doing has to stop FIRST. A structured turn left running past - * the delete keeps spending and keeps acting in the folder while every append - * it makes fails the foreign key; a shell, a terminal or a tab left open is a - * process or a page nothing in the product names any more. + * Removal DETACHES the project: the row stays with a `removed_at` stamp, and + * so do its tickets, comments, Automations and every Session with its whole + * history. Every listing stops returning it, nothing starts new work in it, + * and adding the same folder again re-attaches it (`project-create.ts`). The + * foreign-key cascades are unchanged; removal no longer reaches them. + * + * The work its Sessions are doing still has to stop: a structured turn left + * running would keep spending and acting in a folder the person just took off + * the rail, and a shell, a terminal or a tab left open is a process or a page + * nothing in the product names any more. * * The order is the whole module, and each step is a port a test can watch: * * 1. stop the live work rooted in the project: its Sessions' structured * bindings, background shells and terminals, its Browser Tabs, and the MCP * sign-ins still in flight for its servers; - * 2. delete the row; - * 3. forget the credentials the cascaded MCP servers held; + * 2. detach the row; + * 3. forget the credentials its MCP servers held: no screen shows a removed + * project's servers, so nobody would think to revoke them. A re-attached + * project's servers ask to sign in again; * 4. end the project's board feed; * 5. tell every window, so a second one drops the project too. * * A binding that will not close refuses the removal before anything else is - * stopped or deleted: deleting a Session's history under a turn that is still - * running is the defect, and the person can stop that Session and try again. - * - * The cascade itself is unchanged here: what a removal keeps is a separate - * decision. + * stopped or detached: the person can stop that Session and try again. */ import type Database from "better-sqlite3"; -import { deleteProject, getProjectById, listProjectSessionIds } from "./db/projects-repo"; +import { detachProject, isProjectAttached, listProjectSessionIds } from "./db/projects-repo"; import { releaseAgentSessions, type AgentSiteRuntime } from "./worktree/agent-sites"; /** What a release left bound, by Session id. */ @@ -53,7 +54,7 @@ export interface ProjectLiveWork { closeBrowserTabs(projectId: string): void; /** * The project's MCP servers: `disconnect` ends their in-flight sign-ins and - * names them; `forget` drops their stored credentials after the delete. + * names them; `forget` drops their stored credentials once it is detached. */ readonly mcp: { disconnect(projectId: string): readonly string[]; @@ -68,6 +69,8 @@ export interface ProjectRemovalPorts { disposeFeed(projectId: string): void; /** Tells every window the project list changed. */ announce(): void; + /** Stamps `removed_at`; defaults to `Date.now`. */ + readonly now?: () => number; } /** @@ -86,18 +89,19 @@ export class ProjectRemovalRefused extends Error { } /** - * Removes one project, its live work stopped first. An id no project has is a - * no-op: there is nothing to stop and nothing to tell. + * Removes one project from the rail, its live work stopped first. An id no + * project on the rail has (unknown, or already removed) is a no-op: there is + * nothing to stop and nothing to tell. */ export async function removeProject(ports: ProjectRemovalPorts, projectId: string): Promise { - if (getProjectById(ports.db, projectId) === undefined) return; + if (!isProjectAttached(ports.db, projectId)) return; const sessionIds = new Set(listProjectSessionIds(ports.db, projectId)); const { stillOpen } = await ports.live.releaseBindings(sessionIds); if (stillOpen.length > 0) throw new ProjectRemovalRefused(stillOpen); // The release awaited: a Session started meanwhile was not released, and - // the cascade would take its history from under it. From here to the - // delete nothing awaits, so this is the last such gap. + // would be left running in a project nobody can see. From here to the + // detach nothing awaits, so this is the last such gap. const started = listProjectSessionIds(ports.db, projectId).filter((id) => !sessionIds.has(id)); if (started.length > 0) throw new ProjectRemovalRefused(started); ports.live.disposeShells(sessionIds); @@ -105,7 +109,7 @@ export async function removeProject(ports: ProjectRemovalPorts, projectId: strin ports.live.closeBrowserTabs(projectId); const serverIds = ports.live.mcp?.disconnect(projectId) ?? []; - deleteProject(ports.db, projectId); + detachProject(ports.db, projectId, (ports.now ?? Date.now)()); ports.live.mcp?.forget(serverIds); ports.disposeFeed(projectId); diff --git a/packages/host-core/src/pty/manager.ts b/packages/host-core/src/pty/manager.ts index 7c5de0761..ac81c37e8 100644 --- a/packages/host-core/src/pty/manager.ts +++ b/packages/host-core/src/pty/manager.ts @@ -1646,8 +1646,8 @@ export class PtyManager { /** * Kills the live sessions among `sessionIds`, whichever window holds them, * and names the ones it killed: a removed project's terminals (VC-796), - * which the host ends before the delete takes their Sessions' rows. An id - * with no live session is skipped. + * which the host ends before it detaches the project (VC-822). An id with + * no live session is skipped. */ killSessions(sessionIds: ReadonlySet): string[] { const killed = Array.from(this.sessions.keys()).filter((sessionId) => diff --git a/packages/host-core/src/pty/scope.test.ts b/packages/host-core/src/pty/scope.test.ts index 0ab0daf75..96d5c7712 100644 --- a/packages/host-core/src/pty/scope.test.ts +++ b/packages/host-core/src/pty/scope.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it } from "vite-plus/test"; import { createSessionEngine } from "@volli/session-engine"; import { getHarnessAdapter, roleImpliedByTicket } from "@volli/shared"; -import { insertProject } from "../db/projects-repo"; +import { detachProject, insertProject } from "../db/projects-repo"; import { openTestDb, testProject, testTicket } from "../db/test-helpers"; import type { TestDb } from "../db/test-helpers"; import { insertTicket } from "../db/tickets-repo"; @@ -148,6 +148,39 @@ describe("resolveScope", () => { expect(result).toMatchObject({ ok: true, scope: { title: "Terminal 2", ticketId: null } }); }); + // VC-822: a removed project keeps its Sessions for a re-add, and opens no + // terminal until then, ticket or Board. + it("refuses a terminal in a removed project, and still opens one for an unknown project", async () => { + const { project, ticket, control } = setup(); + detachProject(ctx.db, project.id, 5); + const resolve = (workspaceId: string, ticketId?: string) => + resolveScope( + ctx.db, + control, + { + workspaceId, + cwd: project.path, + cols: 80, + rows: 24, + ...(ticketId === undefined ? {} : { ticket: { ticketId } }), + }, + "", + () => null, + (id) => getHarnessAdapter(id), + ); + const removed = { + ok: false, + error: "This project was removed from Volli. Add its folder again to use it.", + }; + + await expect(resolve(project.id)).resolves.toEqual(removed); + await expect(resolve(project.id, ticket.id)).resolves.toEqual(removed); + await expect(resolve("no-such-project")).resolves.toMatchObject({ + ok: true, + scope: { artifactsRoot: null }, + }); + }); + it("rejects unknown, mismatched, open, and shell history instead of manufacturing a resume", async () => { const { project, ticket, control } = setup(); const other = testTicket(project.id, { id: "other", usesWorktree: false }); diff --git a/packages/host-core/src/pty/scope.ts b/packages/host-core/src/pty/scope.ts index 8b2af4f7e..250ab5090 100644 --- a/packages/host-core/src/pty/scope.ts +++ b/packages/host-core/src/pty/scope.ts @@ -19,7 +19,7 @@ import type { HarnessWrapperLookup, } from "@volli/shared"; import { materializeBlobs } from "../blob-materialize"; -import { getProjectById } from "../db/projects-repo"; +import { getProjectById, isProjectRemoved } from "../db/projects-repo"; import { getTicketSessionContext } from "../db/tickets-repo"; import { terminalSessionRecord } from "../session-control"; @@ -83,11 +83,16 @@ export interface SessionScope { /** A resolved scope, or the one failure: a ticket request naming a ticket that does not exist. */ export type ScopeResolution = { ok: true; scope: SessionScope } | { ok: false; error: string }; +/** Why a terminal does not open in a removed project (VC-822). */ +const PROJECT_REMOVED = "This project was removed from Volli. Add its folder again to use it."; + /** * Resolves a request to its session scope from the db: a ticket session * (VOLLI_TICKET env, MAIN-repo-root cwd, the ticket's harness, `Session N` * title) or a Board Session (default harness, `Terminal N`). - * The only failure is a ticket request naming a ticket that does not exist. + * A ticket request naming a ticket that does not exist fails, and so does a + * request in a removed project (VC-822): it keeps its Sessions for a re-add, + * and nothing runs in it until then. * * `wrapperFor` and `adapterFor` travel together for the same reason: both * answers belong to main's harness runtime, and a launch line built from one @@ -107,6 +112,7 @@ export async function resolveScope( if (request.ticket !== undefined) { const ctx = getTicketSessionContext(db, request.ticket.ticketId); if (ctx === undefined) return { ok: false, error: "Unknown ticket" }; + if (isProjectRemoved(db, ctx.projectId)) return { ok: false, error: PROJECT_REMOVED }; const displayId = displayTicketId(ctx.ticketPrefix, ctx.ticketNumber); const kickoff = request.ticket.kickoff; const resume = request.ticket.resume; @@ -261,7 +267,9 @@ export async function resolveScope( } // Board Session: resolve the project's MAIN path so VOLLI_ARTIFACTS_DIR is // injected the same way a ticket session gets it (decision #9). A project - // that can't be resolved still spawns (no artifacts env) rather than failing. + // that can't be resolved still spawns (no artifacts env) rather than failing; + // a removed one does not. + if (isProjectRemoved(db, request.workspaceId)) return { ok: false, error: PROJECT_REMOVED }; const project = getProjectById(db, request.workspaceId); const sessionCount = await sessionEngine.countSessions({ projectId: request.workspaceId, diff --git a/packages/host-core/src/session-runtime/location.test.ts b/packages/host-core/src/session-runtime/location.test.ts index ce821b728..b9be21ec1 100644 --- a/packages/host-core/src/session-runtime/location.test.ts +++ b/packages/host-core/src/session-runtime/location.test.ts @@ -4,7 +4,7 @@ import { join } from "node:path"; import { afterEach, describe, expect, it, vi } from "vite-plus/test"; import { roleImpliedByTicket } from "@volli/shared"; import type { Session } from "@volli/shared"; -import { insertProject } from "../db/projects-repo"; +import { detachProject, insertProject, reattachProject } from "../db/projects-repo"; import { insertTicket } from "../db/tickets-repo"; import { openTestDb, testProject, testTicket, type TestDb } from "../db/test-helpers"; @@ -356,6 +356,38 @@ describe("desktop Session location resolver", () => { expect(ensureWorktree).not.toHaveBeenCalled(); }); + // VC-822: removal detaches a project and keeps its Sessions, so nothing may + // start one there: not a new binding, and not a turn on a binding that + // outlived the removal. Reading where it ran starts nothing and still works. + it("refuses to start or reaffirm a Session of a removed project, until it is re-added", async () => { + testDb = openTestDb(); + const root = scratch(); + const project = testProject({ id: "project-1", path: root }); + insertProject(testDb.db, project); + const resolver = createSessionLocationResolver(testDb.db, ports, options); + const session: Session = { + id: "project-session", + projectId: project.id, + ticketId: null, + role: "project", + parentSessionId: null, + title: null, + createdAt: 0, + }; + const removed = + "This Session's project was removed from Volli. Add its folder again to use it."; + + detachProject(testDb.db, project.id, 1); + + await expect(resolver.prepare(session)).rejects.toThrow(removed); + await expect(resolver.reaffirm(session, root)).rejects.toThrow(removed); + await expect(resolver.resolve(session)).resolves.toMatchObject({ directory: root }); + + reattachProject(testDb.db, project.id, 2); + await expect(resolver.prepare(session)).resolves.toMatchObject({ directory: root }); + await expect(resolver.reaffirm(session, root)).resolves.toBeUndefined(); + }); + // VC-284. An agent start is the other side of the deletion // lease: a Session can still be bound to a directory this app now calls an // orphan (deleting a ticket only clears `sessions.ticket_id`), so "an agent is diff --git a/packages/host-core/src/session-runtime/location.ts b/packages/host-core/src/session-runtime/location.ts index 47cd3e5a3..dbcdbb426 100644 --- a/packages/host-core/src/session-runtime/location.ts +++ b/packages/host-core/src/session-runtime/location.ts @@ -5,13 +5,17 @@ import type { SessionLocation, SessionLocationResolver } from "@volli/session-en import type { Session, SessionExecutionVenue } from "@volli/shared"; import type { HostEventBus } from "../ports"; -import { getProjectById } from "../db/projects-repo"; +import { getProjectById, isProjectAttached } from "../db/projects-repo"; import { getTicket } from "../db/tickets-repo"; import { acquireWorktreeStartLease, ensure, UNDER_DELETION_REFUSAL } from "../worktree"; import { worktreeDeps } from "../worktree-runtime"; const LOCAL: SessionLocation["venue"] = { id: "local", kind: "local" }; +/** Why a Session of a removed project does not start (VC-822). */ +const PROJECT_REMOVED_REFUSAL = + "This Session's project was removed from Volli. Add its folder again to use it."; + /** * Resolves immutable Session identity to its trusted project/worktree directory. * @@ -72,6 +76,7 @@ export function createSessionLocationResolver( const prepare = async (session: Session): Promise => { const { project, ticket } = site(session); + if (!isProjectAttached(db, project.id)) throw new Error(PROJECT_REMOVED_REFUSAL); if (ticket === null || !ticket.usesWorktree) { return withStartLease(project.path, async () => ({ directory: project.path, venue })); } @@ -113,6 +118,7 @@ export function createSessionLocationResolver( prepare, async reaffirm(session, directory) { + if (!isProjectAttached(db, session.projectId)) throw new Error(PROJECT_REMOVED_REFUSAL); // A turn is about to write into this directory, so it is a start like any // other, and the lease is taken before the question rather than after it: // a directory a cleanup is removing right now must never be reaffirmed as diff --git a/packages/host-core/src/worktree/containers.test.ts b/packages/host-core/src/worktree/containers.test.ts index 7c7870fa2..7a8c98c55 100644 --- a/packages/host-core/src/worktree/containers.test.ts +++ b/packages/host-core/src/worktree/containers.test.ts @@ -1,7 +1,7 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test"; -import { insertProject } from "../db/projects-repo"; +import { detachProject, insertProject } from "../db/projects-repo"; import { openTestDb, testProject, type TestDb } from "../db/test-helpers"; import { isOwnedWorktreePath, @@ -49,6 +49,22 @@ describe("ownedContainers", () => { { projectId: "bbbbbbbb-2222", path: "/home/me/.volli/worktrees/beta-bbbbbbbb" }, ]); }); + + // VC-822: a removed project's checkouts are kept for its return, so no + // destructive worktree path may reach its container. + it("leaves out a removed project's container", () => { + insertProject(ctx.db, testProject({ id: "aaaaaaaa-1111", path: "/repos/alpha" })); + insertProject(ctx.db, testProject({ id: "bbbbbbbb-2222", path: "/repos/beta" })); + detachProject(ctx.db, "aaaaaaaa-1111", 5); + + const containers = ownedContainers(ctx.db, "/home/me"); + expect(containers).toEqual([ + { projectId: "bbbbbbbb-2222", path: "/home/me/.volli/worktrees/beta-bbbbbbbb" }, + ]); + expect( + isOwnedWorktreePath(containers, "/home/me/.volli/worktrees/alpha-aaaaaaaa/VC-1-kept"), + ).toBe(false); + }); }); describe("isOwnedWorktreePath", () => { diff --git a/packages/session-rpc/src/desktop-router.ts b/packages/session-rpc/src/desktop-router.ts index d8a9e4dc6..a03fc08d8 100644 --- a/packages/session-rpc/src/desktop-router.ts +++ b/packages/session-rpc/src/desktop-router.ts @@ -465,7 +465,8 @@ export function createDesktopRouter() { .mutation(({ ctx, input }) => ctx.handlers["project.reorder"](input, ctx.call)), /** * `volli:project-remove` projects the same handler (VC-796): the - * project's live work stopped, then the delete, then every window told. + * project's live work stopped, then the project detached (VC-822), + * then every window told. */ remove: workspaceProcedure( "project.remove", diff --git a/packages/shared/src/automation.ts b/packages/shared/src/automation.ts index 705dc51af..a23aaec2c 100644 --- a/packages/shared/src/automation.ts +++ b/packages/shared/src/automation.ts @@ -405,6 +405,12 @@ export type AutomationRunRefusalCode = | "TICKET_NOT_FOUND" /** A Run that names the Project as its Target, and no such project (VC-130). */ | "PROJECT_NOT_FOUND" + /** + * A Run in a project the person removed (VC-822). Removal detaches the + * project and keeps its Automations, so a schedule still comes due there; + * it is refused with this, and recorded as a skip, until the project is back. + */ + | "PROJECT_REMOVED" | "RUN_IN_FLIGHT" | "MODEL_REQUIRED" /** diff --git a/packages/shared/src/desktop-entries.ts b/packages/shared/src/desktop-entries.ts index d40ffbcaa..aceea8887 100644 --- a/packages/shared/src/desktop-entries.ts +++ b/packages/shared/src/desktop-entries.ts @@ -390,13 +390,14 @@ export const DESKTOP_ENTRIES = [ summary: "Give keyboard focus back to a shown Browser page whose shortcut did nothing.", }, // VC-796: the host's removal, which `volli:project-remove` projects. It - // stops the project's live work before the delete its cascade makes. + // stops the project's live work, then detaches the project (VC-822): its + // tickets and Sessions are kept, and adding the folder again restores them. { key: "project.remove", compatibility: "host-command", placement: "workspace", idempotency: "natural", - summary: "Remove a project from Volli: stop its Sessions' live work, then delete it.", + summary: "Remove a project from Volli: stop its Sessions' live work, then detach it.", }, ] as const satisfies readonly DesktopEntryDeclaration[];