Merge branch 'fix-agent-archive-non-worktree'

This commit is contained in:
Mohamed Boudra
2026-01-24 23:34:55 +07:00
7 changed files with 338 additions and 50 deletions
@@ -0,0 +1,67 @@
import path from 'node:path';
import type { Locator, Page } from '@playwright/test';
import { test, expect } from './fixtures';
import { ensureHostSelected, gotoHome, setWorkingDirectory } from './helpers/app';
import { createTempGitRepo } from './helpers/workspace';
async function longPress(page: Page, locator: Locator, durationMs = 1100) {
const box = await locator.boundingBox();
if (!box) {
throw new Error('Expected long-press target to have a bounding box.');
}
await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
await page.mouse.down();
await page.waitForTimeout(durationMs);
await page.mouse.up();
}
test('deleting an agent via long-press persists after reload', async ({ page }) => {
const repo = await createTempGitRepo();
const nonce = Math.random().toString(36).slice(2, 10);
const prompt = `delete-agent-persists-${nonce}`;
try {
await gotoHome(page);
await setWorkingDirectory(page, repo.path);
await ensureHostSelected(page);
// Create agent (via message input) so it shows up in the sidebar list.
const input = page.getByRole('textbox', { name: 'Message agent...' });
await expect(input).toBeEditable();
await input.fill(prompt);
await input.press('Enter');
await page.waitForURL(/\/agent\//, { waitUntil: 'commit' });
const match = page.url().match(/\/agent\/([^/]+)\/([^/?#]+)/);
if (!match) {
throw new Error(`Expected /agent/:serverId/:agentId URL, got ${page.url()}`);
}
const serverId = decodeURIComponent(match[1]);
const agentId = decodeURIComponent(match[2]);
// Return home and delete via long-press in the agent list.
await gotoHome(page);
const rowTestId = `agent-row-${serverId}-${agentId}`;
const agentRow = page.getByTestId(rowTestId).first();
await expect(agentRow).toBeVisible({ timeout: 30000 });
await longPress(page, agentRow, 1200);
const deleteButton = page.getByTestId('agent-action-delete').first();
await expect(deleteButton).toBeVisible({ timeout: 10000 });
await deleteButton.click({ force: true });
await expect(page.getByTestId('agent-action-cancel')).toHaveCount(0, { timeout: 10000 });
// Ensure deletion finished before reload (avoids races).
await expect(page.getByTestId(rowTestId)).toHaveCount(0, { timeout: 30000 });
// A full reload should not bring the agent back.
await page.reload();
await expect(page.getByRole('textbox', { name: 'Message agent...' })).toBeVisible();
await expect(page.getByTestId(rowTestId)).toHaveCount(0, { timeout: 30000 });
} finally {
await repo.cleanup();
}
});
@@ -174,6 +174,7 @@ export function AgentList({
]} ]}
onPress={() => handleAgentPress(agent.serverId, agent.id)} onPress={() => handleAgentPress(agent.serverId, agent.id)}
onLongPress={() => handleAgentLongPress(agent)} onLongPress={() => handleAgentLongPress(agent)}
testID={`agent-row-${agent.serverId}-${agent.id}`}
> >
{({ hovered }) => ( {({ hovered }) => (
<View style={styles.agentContent}> <View style={styles.agentContent}>
@@ -278,6 +279,7 @@ export function AgentList({
disabled={!deleteAgent || isActionDaemonUnavailable} disabled={!deleteAgent || isActionDaemonUnavailable}
style={[styles.sheetButton, styles.sheetDeleteButton]} style={[styles.sheetButton, styles.sheetDeleteButton]}
onPress={handleDeleteAgent} onPress={handleDeleteAgent}
testID="agent-action-delete"
> >
<Text <Text
style={[ style={[
@@ -292,6 +294,7 @@ export function AgentList({
<Pressable <Pressable
style={[styles.sheetButton, styles.sheetCancelButton]} style={[styles.sheetButton, styles.sheetCancelButton]}
onPress={handleCloseActionSheet} onPress={handleCloseActionSheet}
testID="agent-action-cancel"
> >
<Text style={styles.sheetCancelText}>Cancel</Text> <Text style={styles.sheetCancelText}>Cancel</Text>
</Pressable> </Pressable>
@@ -2,6 +2,7 @@ import { describe, expect, test, beforeEach, afterEach } from "vitest";
import os from "node:os"; import os from "node:os";
import path from "node:path"; import path from "node:path";
import { mkdtempSync, rmSync } from "node:fs"; import { mkdtempSync, rmSync } from "node:fs";
import { promises as fs } from "node:fs";
import { createTestLogger } from "../../test-utils/test-logger.js"; import { createTestLogger } from "../../test-utils/test-logger.js";
import { AgentStorage } from "./agent-storage.js"; import { AgentStorage } from "./agent-storage.js";
@@ -280,4 +281,67 @@ describe("AgentStorage", () => {
expect(records).toHaveLength(1); expect(records).toHaveLength(1);
expect(records[0]?.internal).toBe(true); expect(records[0]?.internal).toBe(true);
}); });
test("remove deletes all duplicate record files across project directories", async () => {
const agentId = "agent-duplicate";
// Create a valid record file in two different project directories to simulate
// storage migrations/duplication. Only one copy will be referenced in-memory,
// but deletion should remove *all* copies on disk.
const recordA = await (async () => {
await storage.applySnapshot(
createManagedAgent({
id: agentId,
cwd: "/tmp/project-a",
provider: "codex",
})
);
const record = await storage.get(agentId);
expect(record).not.toBeNull();
return record!;
})();
const projectDirB = path.join(storagePath, "tmp-project-b");
await fs.mkdir(projectDirB, { recursive: true });
const duplicatePathB = path.join(projectDirB, `${agentId}.json`);
await fs.writeFile(
duplicatePathB,
JSON.stringify({ ...recordA, cwd: "/tmp/project-b" }, null, 2),
"utf8"
);
// Force a reload so the registry has to discover from disk (and may choose either copy).
const reloaded = new AgentStorage(storagePath, logger);
const before = await reloaded.list();
expect(before.map((r) => r.id)).toContain(agentId);
await reloaded.remove(agentId);
const hasAnyRecordFile = async () => {
try {
const projects = await fs.readdir(storagePath, { withFileTypes: true });
for (const project of projects) {
if (!project.isDirectory()) {
continue;
}
const candidate = path.join(storagePath, project.name, `${agentId}.json`);
try {
await fs.access(candidate);
return true;
} catch {
// not here
}
}
} catch {
// ignore
}
return false;
};
expect(await hasAnyRecordFile()).toBe(false);
const afterReload = new AgentStorage(storagePath, logger);
const after = await afterReload.list();
expect(after.some((r) => r.id === agentId)).toBe(false);
});
}); });
@@ -67,6 +67,8 @@ export type StoredAgentRecord = z.infer<typeof STORED_AGENT_SCHEMA>;
export class AgentStorage { export class AgentStorage {
private cache: Map<string, StoredAgentRecord> = new Map(); private cache: Map<string, StoredAgentRecord> = new Map();
private pathById: Map<string, string> = new Map(); private pathById: Map<string, string> = new Map();
private pendingWrites: Map<string, Promise<void>> = new Map();
private deleting: Set<string> = new Set();
private loaded = false; private loaded = false;
private baseDir: string; private baseDir: string;
private loadPromise: Promise<StoredAgentRecord[]> | null = null; private loadPromise: Promise<StoredAgentRecord[]> | null = null;
@@ -106,33 +108,102 @@ export class AgentStorage {
async upsert(record: StoredAgentRecord): Promise<void> { async upsert(record: StoredAgentRecord): Promise<void> {
await this.load(); await this.load();
const nextPath = this.buildRecordPath(record); const agentId = record.id;
const previousPath = this.pathById.get(record.id); const prev = this.pendingWrites.get(agentId) ?? Promise.resolve();
const next = prev.then(async () => {
await fs.mkdir(path.dirname(nextPath), { recursive: true }); if (this.deleting.has(agentId)) {
await writeFileAtomically(nextPath, JSON.stringify(record, null, 2)); return;
if (previousPath && previousPath !== nextPath) {
try {
await fs.unlink(previousPath);
} catch {
// ignore cleanup errors
} }
}
this.cache.set(record.id, record); const nextPath = this.buildRecordPath(record);
this.pathById.set(record.id, nextPath); const previousPath = this.pathById.get(agentId);
await fs.mkdir(path.dirname(nextPath), { recursive: true });
await writeFileAtomically(nextPath, JSON.stringify(record, null, 2));
if (previousPath && previousPath !== nextPath) {
try {
await fs.unlink(previousPath);
} catch {
// ignore cleanup errors
}
}
this.cache.set(agentId, record);
this.pathById.set(agentId, nextPath);
});
this.pendingWrites.set(
agentId,
next.finally(() => {
if (this.pendingWrites.get(agentId) === next) {
this.pendingWrites.delete(agentId);
}
})
);
await next;
}
beginDelete(agentId: string): void {
this.deleting.add(agentId);
} }
async remove(agentId: string): Promise<void> { async remove(agentId: string): Promise<void> {
await this.load(); await this.load();
this.beginDelete(agentId);
await (this.pendingWrites.get(agentId) ?? Promise.resolve());
const candidates = new Set<string>();
const existingPath = const existingPath =
this.pathById.get(agentId) ?? (await this.findAgentPathById(agentId)); this.pathById.get(agentId) ?? (await this.findAgentPathById(agentId));
if (existingPath) { if (existingPath) {
candidates.add(existingPath);
}
// Remove any stray duplicate record files across project directories.
// This can happen across storage layout migrations or when a record path changes.
try {
const projects = await fs.readdir(this.baseDir, { withFileTypes: true });
for (const project of projects) {
if (!project.isDirectory()) {
continue;
}
const projectDir = path.join(this.baseDir, project.name);
candidates.add(path.join(projectDir, `${agentId}.json`));
// Support one more nesting layer (e.g. provider/version subfolders).
let entries: Array<import("node:fs").Dirent> = [];
try {
entries = await fs.readdir(projectDir, { withFileTypes: true });
} catch {
continue;
}
for (const entry of entries) {
if (!entry.isDirectory()) {
continue;
}
candidates.add(path.join(projectDir, entry.name, `${agentId}.json`));
}
}
} catch {
// ignore scan errors
}
// Support legacy flat layouts: baseDir/<agentId>.json
candidates.add(path.join(this.baseDir, `${agentId}.json`));
for (const filePath of candidates) {
try { try {
await fs.unlink(existingPath); await fs.unlink(filePath);
} catch { } catch (error) {
// ignore removal errors const code = (error as NodeJS.ErrnoException).code;
if (code && code !== "ENOENT") {
this.logger.warn(
{ err: error, agentId, filePath },
"Failed to remove agent record file"
);
}
} }
} }
+37 -26
View File
@@ -78,12 +78,13 @@ import {
WorktreeSetupError, WorktreeSetupError,
type WorktreeConfig, type WorktreeConfig,
type WorktreeSetupCommandResult, type WorktreeSetupCommandResult,
slugify, slugify,
validateBranchSlug, validateBranchSlug,
listPaseoWorktrees, listPaseoWorktrees,
deletePaseoWorktree, deletePaseoWorktree,
isPaseoOwnedWorktreeCwd, isPaseoOwnedWorktreeCwd,
} from "../utils/worktree.js"; resolvePaseoWorktreeRootForCwd,
} from "../utils/worktree.js";
import { import {
getCheckoutDiff, getCheckoutDiff,
getCheckoutStatus, getCheckoutStatus,
@@ -1148,6 +1149,9 @@ export class Session {
`Deleting agent ${agentId} from registry` `Deleting agent ${agentId} from registry`
); );
// Prevent the persistence hook from re-creating the record while we close/delete.
this.agentStorage.beginDelete(agentId);
try { try {
await this.agentManager.closeAgent(agentId); await this.agentManager.closeAgent(agentId);
} catch (error: any) { } catch (error: any) {
@@ -3298,12 +3302,12 @@ export class Session {
} }
} }
private async handlePaseoWorktreeArchiveRequest( private async handlePaseoWorktreeArchiveRequest(
msg: Extract<SessionInboundMessage, { type: "paseo_worktree_archive_request" }> msg: Extract<SessionInboundMessage, { type: "paseo_worktree_archive_request" }>
): Promise<void> { ): Promise<void> {
const { requestId } = msg; const { requestId } = msg;
let targetPath = msg.worktreePath; let targetPath = msg.worktreePath;
let repoRoot = msg.repoRoot ?? null; let repoRoot = msg.repoRoot ?? null;
try { try {
if (!targetPath) { if (!targetPath) {
@@ -3335,16 +3339,23 @@ export class Session {
return; return;
} }
repoRoot = ownership.repoRoot ?? repoRoot ?? null; repoRoot = ownership.repoRoot ?? repoRoot ?? null;
if (!repoRoot) { if (!repoRoot) {
throw new Error("Unable to resolve repo root for worktree"); throw new Error("Unable to resolve repo root for worktree");
} }
const removedAgents = new Set<string>(); const resolvedWorktree = await resolvePaseoWorktreeRootForCwd(targetPath, {
const agents = this.agentManager.listAgents(); paseoHome: this.paseoHome,
for (const agent of agents) { });
if (this.isPathWithinRoot(targetPath, agent.cwd)) { if (resolvedWorktree) {
removedAgents.add(agent.id); targetPath = resolvedWorktree.worktreePath;
}
const removedAgents = new Set<string>();
const agents = this.agentManager.listAgents();
for (const agent of agents) {
if (this.isPathWithinRoot(targetPath, agent.cwd)) {
removedAgents.add(agent.id);
try { try {
await this.agentManager.closeAgent(agent.id); await this.agentManager.closeAgent(agent.id);
} catch { } catch {
@@ -3370,11 +3381,11 @@ export class Session {
} }
} }
await deletePaseoWorktree({ await deletePaseoWorktree({
cwd: repoRoot, cwd: repoRoot,
worktreePath: targetPath, worktreePath: targetPath,
paseoHome: this.paseoHome, paseoHome: this.paseoHome,
}); });
for (const agentId of removedAgents) { for (const agentId of removedAgents) {
this.emit({ this.emit({
@@ -305,6 +305,25 @@ describe("paseo worktree manager", () => {
expect(remaining.map((worktree) => worktree.path)).toEqual([second.worktreePath]); expect(remaining.map((worktree) => worktree.path)).toEqual([second.worktreePath]);
}); });
it("deletes a paseo worktree even when given a subdirectory path", async () => {
const created = await createWorktree({
branchName: "main",
cwd: repoDir,
baseBranch: "main",
worktreeSlug: "alpha",
paseoHome,
});
const nestedDir = join(created.worktreePath, "nested", "dir");
execSync(`mkdir -p ${nestedDir}`);
await deletePaseoWorktree({ cwd: repoDir, worktreePath: nestedDir, paseoHome });
expect(existsSync(created.worktreePath)).toBe(false);
const remaining = await listPaseoWorktrees({ cwd: repoDir, paseoHome });
expect(remaining.some((worktree) => worktree.path === created.worktreePath)).toBe(false);
});
it("ensures .paseo is ignored in .gitignore", async () => { it("ensures .paseo is ignored in .gitignore", async () => {
await ensurePaseoIgnored(repoDir); await ensurePaseoIgnored(repoDir);
await ensurePaseoIgnored(repoDir); await ensurePaseoIgnored(repoDir);
+60 -7
View File
@@ -514,6 +514,57 @@ export async function listPaseoWorktrees({
.filter((entry) => entry.path.startsWith(rootPrefix)); .filter((entry) => entry.path.startsWith(rootPrefix));
} }
export async function resolvePaseoWorktreeRootForCwd(
cwd: string,
options?: { paseoHome?: string }
): Promise<{ repoRoot: string; worktreeRoot: string; worktreePath: string } | null> {
let repoInfo: RepoInfo;
try {
repoInfo = await detectRepoInfo(cwd);
} catch {
return null;
}
const worktreesRoot = await getPaseoWorktreesRoot(repoInfo.path, options?.paseoHome);
const resolvedRoot = normalizePathForOwnership(worktreesRoot) + sep;
let worktreeRoot: string | null = null;
try {
const { stdout } = await execAsync(
"git rev-parse --path-format=absolute --show-toplevel",
{ cwd, env: READ_ONLY_GIT_ENV }
);
const trimmed = stdout.trim();
worktreeRoot = trimmed.length > 0 ? trimmed : null;
} catch {
worktreeRoot = null;
}
if (!worktreeRoot) {
return null;
}
const resolvedWorktreeRoot = normalizePathForOwnership(worktreeRoot);
if (!resolvedWorktreeRoot.startsWith(resolvedRoot)) {
return null;
}
const knownWorktrees = await listPaseoWorktrees({
cwd: repoInfo.path,
paseoHome: options?.paseoHome,
});
const match = knownWorktrees.find((entry) => entry.path === resolvedWorktreeRoot);
if (!match) {
return null;
}
return {
repoRoot: repoInfo.path,
worktreeRoot: worktreesRoot,
worktreePath: match.path,
};
}
export async function deletePaseoWorktree({ export async function deletePaseoWorktree({
cwd, cwd,
worktreePath, worktreePath,
@@ -531,21 +582,23 @@ export async function deletePaseoWorktree({
const repoInfo = await detectRepoInfo(cwd); const repoInfo = await detectRepoInfo(cwd);
const worktreesRoot = await getPaseoWorktreesRoot(repoInfo.path, paseoHome); const worktreesRoot = await getPaseoWorktreesRoot(repoInfo.path, paseoHome);
const targetPath = worktreePath ?? join(worktreesRoot, worktreeSlug!);
const resolvedRoot = normalizePathForOwnership(worktreesRoot) + sep; const resolvedRoot = normalizePathForOwnership(worktreesRoot) + sep;
const resolvedTarget = normalizePathForOwnership(targetPath); const requestedPath = worktreePath ?? join(worktreesRoot, worktreeSlug!);
const resolvedRequested = normalizePathForOwnership(requestedPath);
const resolvedWorktree =
(await resolvePaseoWorktreeRootForCwd(requestedPath, { paseoHome }))?.worktreePath ??
resolvedRequested;
if (!resolvedTarget.startsWith(resolvedRoot)) { if (!resolvedWorktree.startsWith(resolvedRoot)) {
throw new Error("Refusing to delete non-Paseo worktree"); throw new Error("Refusing to delete non-Paseo worktree");
} }
const canonicalTargetPath = normalizePathForOwnership(targetPath); await execAsync(`git worktree remove "${resolvedWorktree}" --force`, {
await execAsync(`git worktree remove "${canonicalTargetPath}" --force`, {
cwd: repoInfo.path, cwd: repoInfo.path,
}); });
if (existsSync(canonicalTargetPath)) { if (existsSync(resolvedWorktree)) {
rmSync(canonicalTargetPath, { recursive: true, force: true }); rmSync(resolvedWorktree, { recursive: true, force: true });
} }
} }