From a698a7bc10137baf7d25bd5722e1d6e0343387c1 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 9 Jun 2026 22:04:45 +0000 Subject: [PATCH] Address file viewer review followups --- server/src/__tests__/file-resources.test.ts | 49 +++++++++++++++++++ .../src/__tests__/workspace-runtime.test.ts | 10 ++-- .../src/services/workspace-file-resources.ts | 39 ++++++++++----- server/src/services/workspace-runtime.ts | 9 ++-- 4 files changed, 88 insertions(+), 19 deletions(-) diff --git a/server/src/__tests__/file-resources.test.ts b/server/src/__tests__/file-resources.test.ts index 6c1e148494a..1f0516aab75 100644 --- a/server/src/__tests__/file-resources.test.ts +++ b/server/src/__tests__/file-resources.test.ts @@ -1,8 +1,10 @@ import express from "express"; import type { Request } from "express"; +import { execFile } from "node:child_process"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { promisify } from "node:util"; import request from "supertest"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { activityLog, agents, companies, createDb, executionWorkspaces, goals, issues, projects, projectWorkspaces, type Db } from "@paperclipai/db"; @@ -25,6 +27,7 @@ import { const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; +const execFileAsync = promisify(execFile); type TestGraph = { companyId: string; @@ -774,6 +777,52 @@ describeEmbeddedPostgres("workspace file resources", () => { expect(JSON.stringify(listRead?.details)).not.toContain("new.ts"); }); + it("applies list offsets to search, recent, and changed modes", async () => { + const { projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + await execFileAsync("git", ["-C", projectRoot, "init"]); + await Promise.all([ + fs.writeFile(path.join(projectRoot, "alpha.ts"), "export const alpha = true;\n", "utf8"), + fs.writeFile(path.join(projectRoot, "beta.ts"), "export const beta = true;\n", "utf8"), + fs.writeFile(path.join(projectRoot, "gamma.ts"), "export const gamma = true;\n", "utf8"), + ]); + const older = new Date("2026-01-01T00:00:00.000Z"); + const middle = new Date("2026-01-02T00:00:00.000Z"); + const newer = new Date("2026-01-03T00:00:00.000Z"); + await fs.utimes(path.join(projectRoot, "alpha.ts"), older, older); + await fs.utimes(path.join(projectRoot, "beta.ts"), middle, middle); + await fs.utimes(path.join(projectRoot, "gamma.ts"), newer, newer); + + const app = createApp(db, { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }); + + const searchPage = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/list`) + .query({ workspace: "project", q: ".ts", limit: 1, offset: 1 }); + expect(searchPage.status).toBe(200); + expect(searchPage.body.items.map((item: { relativePath: string }) => item.relativePath)).toEqual(["beta.ts"]); + expect(searchPage.body.truncated).toBe(true); + + const recentPage = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/list`) + .query({ workspace: "project", mode: "recent", limit: 1, offset: 1 }); + expect(recentPage.status).toBe(200); + expect(recentPage.body.items.map((item: { relativePath: string }) => item.relativePath)).toEqual(["beta.ts"]); + expect(recentPage.body.truncated).toBe(true); + + const changedPage = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/list`) + .query({ workspace: "project", mode: "changed", limit: 1, offset: 1 }); + expect(changedPage.status).toBe(200); + expect(changedPage.body.items.map((item: { relativePath: string }) => item.relativePath)).toEqual(["beta.ts"]); + expect(changedPage.body.truncated).toBe(true); + }); + it("rejects overlong list searches and redacts the raw query from denial audit details", async () => { const { projectRoot, executionRoot } = await makeWorkspace(); const graph = await seedGraph(db, { projectRoot, executionRoot }); diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index c14c03bbe16..51bad01f46a 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -3087,7 +3087,7 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => { await expect(fetch(service!.url!)).rejects.toThrow(); }); - it("replaces unhealthy adopted Paperclip dev services on a reused auto port", async () => { + it("does not reuse a stopped auto-port service port while another process owns it", async () => { const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-unhealthy-adopt-")); const paperclipHome = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-runtime-home-")); process.env.PAPERCLIP_HOME = paperclipHome; @@ -3278,9 +3278,11 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => { expect(services).toHaveLength(1); expect(services[0]?.reused).toBe(false); expect(services[0]?.id).toBe(stoppedServiceId); - expect(services[0]?.port).toBe(stalePort); - await expect(fetch(healthUrl)).resolves.toMatchObject({ ok: true }); - expect(await readLocalServicePortOwner(stalePort!)).not.toBe(staleProcess.pid); + expect(services[0]?.port).not.toBe(stalePort); + expect(services[0]?.url).not.toBe(rootUrl); + await expect(fetch(services[0]!.url!)).resolves.toMatchObject({ ok: true }); + await expect(fetch(healthUrl)).resolves.toMatchObject({ ok: false, status: 503 }); + expect(await readLocalServicePortOwner(stalePort!)).toBe(staleProcess.pid); } finally { leasedRunIds.delete(runId); await releaseRuntimeServicesForRun(runId); diff --git a/server/src/services/workspace-file-resources.ts b/server/src/services/workspace-file-resources.ts index ca44c5a083a..805f3a1328c 100644 --- a/server/src/services/workspace-file-resources.ts +++ b/server/src/services/workspace-file-resources.ts @@ -751,12 +751,14 @@ async function enumerateWorkspaceFiles(input: { mode: WorkspaceFileListMode; normalizedQuery: string | null; limit: number; + offset: number; }) { const dirs: Array<{ realPath: string; relativePath: string; depth: number }> = [ { realPath: input.startReal ?? input.rootReal, relativePath: input.startRelativePath ?? "", depth: 0 }, ]; const items: WorkspaceFileListItem[] = []; let scannedCount = 0; + let matchedCount = 0; let truncated = false; let hitScanCap = false; @@ -804,20 +806,32 @@ async function enumerateWorkspaceFiles(input: { relativePath, normalizedQuery: input.normalizedQuery, }); - if (item) items.push(item); + if (!item) continue; - if (input.mode !== "recent" && items.length >= input.limit) { + if (input.mode === "recent") { + items.push(item); + continue; + } + + if (matchedCount < input.offset) { + matchedCount += 1; + continue; + } + if (items.length >= input.limit) { truncated = true; break; } + matchedCount += 1; + items.push(item); } if (hitScanCap || (truncated && input.mode !== "recent")) break; } if (input.mode === "recent") { items.sort((a, b) => (b.modifiedAt ?? "").localeCompare(a.modifiedAt ?? "") || a.displayPath.localeCompare(b.displayPath)); - truncated = truncated || items.length > input.limit; - return { items: items.slice(0, input.limit), scannedCount, truncated }; + const end = input.offset + input.limit; + truncated = truncated || items.length > end; + return { items: items.slice(input.offset, end), scannedCount, truncated }; } return { items, scannedCount, truncated }; @@ -847,6 +861,7 @@ async function listChangedWorkspaceFiles(input: { rootReal: string; normalizedQuery: string | null; limit: number; + offset: number; }) { let stdout: string; try { @@ -861,7 +876,7 @@ async function listChangedWorkspaceFiles(input: { } const { paths, hitScanCap } = parseGitStatusPaths(stdout); - const items: WorkspaceFileListItem[] = []; + const matchedItems: WorkspaceFileListItem[] = []; let scannedCount = 0; for (const filePath of paths) { scannedCount += 1; @@ -871,14 +886,14 @@ async function listChangedWorkspaceFiles(input: { relativePath: filePath, normalizedQuery: input.normalizedQuery, }); - if (item) items.push(item); - if (items.length >= input.limit) break; + if (item) matchedItems.push(item); } - items.sort((a, b) => a.displayPath.localeCompare(b.displayPath)); + matchedItems.sort((a, b) => a.displayPath.localeCompare(b.displayPath)); + const end = input.offset + input.limit; return { - items, + items: matchedItems.slice(input.offset, end), scannedCount, - truncated: hitScanCap || paths.length > scannedCount || items.length > input.limit, + truncated: hitScanCap || matchedItems.length > end, }; } @@ -1166,7 +1181,7 @@ export function workspaceFileResourceService(db: Db) { } if (mode === "changed") { - const changed = await listChangedWorkspaceFiles({ candidate, rootReal, normalizedQuery, limit }); + const changed = await listChangedWorkspaceFiles({ candidate, rootReal, normalizedQuery, limit, offset }); if ("unavailableReason" in changed) { const reason = changed.unavailableReason ?? "changed_unavailable"; firstUnavailable ??= { candidate, reason }; @@ -1216,6 +1231,7 @@ export function workspaceFileResourceService(db: Db) { mode, normalizedQuery, limit, + offset, }); return availableFileList({ selector, @@ -1265,6 +1281,7 @@ export function workspaceFileResourceService(db: Db) { mode, normalizedQuery, limit, + offset, }); return availableFileList({ selector, diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index 90bcdcc5953..dc63868aa07 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -2157,10 +2157,11 @@ async function startLocalRuntimeService(input: { companyId: input.agent.companyId, reuseKey: input.reuseKey, }); - const reusableStoppedPort = - asString(portConfig.type, "") === "auto" && stoppedReuseCandidate?.port - ? stoppedReuseCandidate.port - : null; + let reusableStoppedPort: number | null = null; + if (asString(portConfig.type, "") === "auto" && stoppedReuseCandidate?.port) { + const ownerPid = await readLocalServicePortOwner(stoppedReuseCandidate.port); + reusableStoppedPort = ownerPid ? null : stoppedReuseCandidate.port; + } const port = asString(portConfig.type, "") === "auto" ? (reusableStoppedPort ?? await allocatePort())