Merge pull request #649 from paperclipai/fix/issue-runs-heartbeat-regressions
Fix issue run lookup and heartbeat run summaries
This commit is contained in:
@@ -2,13 +2,14 @@ import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
copyGitHooksToWorktreeGitDir,
|
||||
copySeededSecretsKey,
|
||||
rebindWorkspaceCwd,
|
||||
resolveGitWorktreeAddArgs,
|
||||
resolveWorktreeMakeTargetPath,
|
||||
worktreeInitCommand,
|
||||
worktreeMakeCommand,
|
||||
} from "../commands/worktree.js";
|
||||
import {
|
||||
@@ -203,7 +204,11 @@ describe("worktree helpers", () => {
|
||||
|
||||
it("copies the source local_encrypted secrets key into the seeded worktree instance", () => {
|
||||
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-worktree-secrets-"));
|
||||
const originalInlineMasterKey = process.env.PAPERCLIP_SECRETS_MASTER_KEY;
|
||||
const originalKeyFile = process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE;
|
||||
try {
|
||||
delete process.env.PAPERCLIP_SECRETS_MASTER_KEY;
|
||||
delete process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE;
|
||||
const sourceConfigPath = path.join(tempRoot, "source", "config.json");
|
||||
const sourceKeyPath = path.join(tempRoot, "source", "secrets", "master.key");
|
||||
const targetKeyPath = path.join(tempRoot, "target", "secrets", "master.key");
|
||||
@@ -222,6 +227,16 @@ describe("worktree helpers", () => {
|
||||
|
||||
expect(fs.readFileSync(targetKeyPath, "utf8")).toBe("source-master-key");
|
||||
} finally {
|
||||
if (originalInlineMasterKey === undefined) {
|
||||
delete process.env.PAPERCLIP_SECRETS_MASTER_KEY;
|
||||
} else {
|
||||
process.env.PAPERCLIP_SECRETS_MASTER_KEY = originalInlineMasterKey;
|
||||
}
|
||||
if (originalKeyFile === undefined) {
|
||||
delete process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE;
|
||||
} else {
|
||||
process.env.PAPERCLIP_SECRETS_MASTER_KEY_FILE = originalKeyFile;
|
||||
}
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
@@ -247,6 +262,36 @@ describe("worktree helpers", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("persists the current agent jwt secret into the worktree env file", async () => {
|
||||
const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-worktree-jwt-"));
|
||||
const repoRoot = path.join(tempRoot, "repo");
|
||||
const originalCwd = process.cwd();
|
||||
const originalJwtSecret = process.env.PAPERCLIP_AGENT_JWT_SECRET;
|
||||
|
||||
try {
|
||||
fs.mkdirSync(repoRoot, { recursive: true });
|
||||
process.env.PAPERCLIP_AGENT_JWT_SECRET = "worktree-shared-secret";
|
||||
process.chdir(repoRoot);
|
||||
|
||||
await worktreeInitCommand({
|
||||
seed: false,
|
||||
fromConfig: path.join(tempRoot, "missing", "config.json"),
|
||||
home: path.join(tempRoot, ".paperclip-worktrees"),
|
||||
});
|
||||
|
||||
const envPath = path.join(repoRoot, ".paperclip", ".env");
|
||||
expect(fs.readFileSync(envPath, "utf8")).toContain("PAPERCLIP_AGENT_JWT_SECRET=worktree-shared-secret");
|
||||
} finally {
|
||||
process.chdir(originalCwd);
|
||||
if (originalJwtSecret === undefined) {
|
||||
delete process.env.PAPERCLIP_AGENT_JWT_SECRET;
|
||||
} else {
|
||||
process.env.PAPERCLIP_AGENT_JWT_SECRET = originalJwtSecret;
|
||||
}
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rebinds same-repo workspace paths onto the current worktree root", () => {
|
||||
expect(
|
||||
rebindWorkspaceCwd({
|
||||
@@ -329,7 +374,7 @@ describe("worktree helpers", () => {
|
||||
const fakeHome = path.join(tempRoot, "home");
|
||||
const worktreePath = path.join(fakeHome, "paperclip-make-test");
|
||||
const originalCwd = process.cwd();
|
||||
const originalHome = process.env.HOME;
|
||||
const homedirSpy = vi.spyOn(os, "homedir").mockReturnValue(fakeHome);
|
||||
|
||||
try {
|
||||
fs.mkdirSync(repoRoot, { recursive: true });
|
||||
@@ -341,7 +386,6 @@ describe("worktree helpers", () => {
|
||||
execFileSync("git", ["add", "README.md"], { cwd: repoRoot, stdio: "ignore" });
|
||||
execFileSync("git", ["commit", "-m", "Initial commit"], { cwd: repoRoot, stdio: "ignore" });
|
||||
|
||||
process.env.HOME = fakeHome;
|
||||
process.chdir(repoRoot);
|
||||
|
||||
await worktreeMakeCommand("paperclip-make-test", {
|
||||
@@ -354,12 +398,8 @@ describe("worktree helpers", () => {
|
||||
expect(fs.existsSync(path.join(worktreePath, ".paperclip", ".env"))).toBe(true);
|
||||
} finally {
|
||||
process.chdir(originalCwd);
|
||||
if (originalHome === undefined) {
|
||||
delete process.env.HOME;
|
||||
} else {
|
||||
process.env.HOME = originalHome;
|
||||
}
|
||||
homedirSpy.mockRestore();
|
||||
fs.rmSync(tempRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
}, 20_000);
|
||||
});
|
||||
|
||||
@@ -651,7 +651,17 @@ async function runWorktreeInit(opts: WorktreeInitOptions): Promise<void> {
|
||||
});
|
||||
|
||||
writeConfig(targetConfig, paths.configPath);
|
||||
mergePaperclipEnvEntries(buildWorktreeEnvEntries(paths), paths.envPath);
|
||||
const sourceEnvEntries = readPaperclipEnvEntries(resolvePaperclipEnvFile(sourceConfigPath));
|
||||
const existingAgentJwtSecret =
|
||||
nonEmpty(sourceEnvEntries.PAPERCLIP_AGENT_JWT_SECRET) ??
|
||||
nonEmpty(process.env.PAPERCLIP_AGENT_JWT_SECRET);
|
||||
mergePaperclipEnvEntries(
|
||||
{
|
||||
...buildWorktreeEnvEntries(paths),
|
||||
...(existingAgentJwtSecret ? { PAPERCLIP_AGENT_JWT_SECRET: existingAgentJwtSecret } : {}),
|
||||
},
|
||||
paths.envPath,
|
||||
);
|
||||
ensureAgentJwtSecret(paths.configPath);
|
||||
loadPaperclipEnvFile(paths.configPath);
|
||||
const copiedGitHooks = copyGitHooksToWorktreeGitDir(cwd);
|
||||
|
||||
70
server/src/__tests__/activity-routes.test.ts
Normal file
70
server/src/__tests__/activity-routes.test.ts
Normal file
@@ -0,0 +1,70 @@
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { activityRoutes } from "../routes/activity.js";
|
||||
|
||||
const mockActivityService = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
forIssue: vi.fn(),
|
||||
runsForIssue: vi.fn(),
|
||||
issuesForRun: vi.fn(),
|
||||
create: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockIssueService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
getByIdentifier: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../services/activity.js", () => ({
|
||||
activityService: () => mockActivityService,
|
||||
}));
|
||||
|
||||
vi.mock("../services/index.js", () => ({
|
||||
issueService: () => mockIssueService,
|
||||
}));
|
||||
|
||||
function createApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
(req as any).actor = {
|
||||
type: "board",
|
||||
userId: "user-1",
|
||||
companyIds: ["company-1"],
|
||||
source: "session",
|
||||
isInstanceAdmin: false,
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", activityRoutes({} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
describe("activity routes", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("resolves issue identifiers before loading runs", async () => {
|
||||
mockIssueService.getByIdentifier.mockResolvedValue({
|
||||
id: "issue-uuid-1",
|
||||
companyId: "company-1",
|
||||
});
|
||||
mockActivityService.runsForIssue.mockResolvedValue([
|
||||
{
|
||||
runId: "run-1",
|
||||
},
|
||||
]);
|
||||
|
||||
const res = await request(createApp()).get("/api/issues/PAP-475/runs");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PAP-475");
|
||||
expect(mockIssueService.getById).not.toHaveBeenCalled();
|
||||
expect(mockActivityService.runsForIssue).toHaveBeenCalledWith("company-1", "issue-uuid-1");
|
||||
expect(res.body).toEqual([{ runId: "run-1" }]);
|
||||
});
|
||||
});
|
||||
33
server/src/__tests__/heartbeat-run-summary.test.ts
Normal file
33
server/src/__tests__/heartbeat-run-summary.test.ts
Normal file
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { summarizeHeartbeatRunResultJson } from "../services/heartbeat-run-summary.js";
|
||||
|
||||
describe("summarizeHeartbeatRunResultJson", () => {
|
||||
it("truncates text fields and preserves cost aliases", () => {
|
||||
const summary = summarizeHeartbeatRunResultJson({
|
||||
summary: "a".repeat(600),
|
||||
result: "ok",
|
||||
message: "done",
|
||||
error: "failed",
|
||||
total_cost_usd: 1.23,
|
||||
cost_usd: 0.45,
|
||||
costUsd: 0.67,
|
||||
nested: { ignored: true },
|
||||
});
|
||||
|
||||
expect(summary).toEqual({
|
||||
summary: "a".repeat(500),
|
||||
result: "ok",
|
||||
message: "done",
|
||||
error: "failed",
|
||||
total_cost_usd: 1.23,
|
||||
cost_usd: 0.45,
|
||||
costUsd: 0.67,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns null for non-object and irrelevant payloads", () => {
|
||||
expect(summarizeHeartbeatRunResultJson(null)).toBeNull();
|
||||
expect(summarizeHeartbeatRunResultJson(["nope"] as unknown as Record<string, unknown>)).toBeNull();
|
||||
expect(summarizeHeartbeatRunResultJson({ nested: { only: "ignored" } })).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -52,5 +52,5 @@ describe("privateHostnameGuard", () => {
|
||||
const res = await request(app).get("/dashboard").set("Host", "dotta-macbook-pro:3100");
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.text).toContain("please run pnpm paperclipai allowed-hostname dotta-macbook-pro");
|
||||
});
|
||||
}, 20_000);
|
||||
});
|
||||
|
||||
@@ -22,6 +22,13 @@ export function activityRoutes(db: Db) {
|
||||
const svc = activityService(db);
|
||||
const issueSvc = issueService(db);
|
||||
|
||||
async function resolveIssueByRef(rawId: string) {
|
||||
if (/^[A-Z]+-\d+$/i.test(rawId)) {
|
||||
return issueSvc.getByIdentifier(rawId);
|
||||
}
|
||||
return issueSvc.getById(rawId);
|
||||
}
|
||||
|
||||
router.get("/companies/:companyId/activity", async (req, res) => {
|
||||
const companyId = req.params.companyId as string;
|
||||
assertCompanyAccess(req, companyId);
|
||||
@@ -47,42 +54,27 @@ export function activityRoutes(db: Db) {
|
||||
res.status(201).json(event);
|
||||
});
|
||||
|
||||
// Resolve issue identifiers (e.g. "PAP-39") to UUIDs
|
||||
router.param("id", async (req, res, next, rawId) => {
|
||||
try {
|
||||
if (/^[A-Z]+-\d+$/i.test(rawId)) {
|
||||
const issue = await issueSvc.getByIdentifier(rawId);
|
||||
if (issue) {
|
||||
req.params.id = issue.id;
|
||||
}
|
||||
}
|
||||
next();
|
||||
} catch (err) {
|
||||
next(err);
|
||||
}
|
||||
});
|
||||
|
||||
router.get("/issues/:id/activity", async (req, res) => {
|
||||
const id = req.params.id as string;
|
||||
const issue = await issueSvc.getById(id);
|
||||
const rawId = req.params.id as string;
|
||||
const issue = await resolveIssueByRef(rawId);
|
||||
if (!issue) {
|
||||
res.status(404).json({ error: "Issue not found" });
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
const result = await svc.forIssue(id);
|
||||
const result = await svc.forIssue(issue.id);
|
||||
res.json(result);
|
||||
});
|
||||
|
||||
router.get("/issues/:id/runs", async (req, res) => {
|
||||
const id = req.params.id as string;
|
||||
const issue = await issueSvc.getById(id);
|
||||
const rawId = req.params.id as string;
|
||||
const issue = await resolveIssueByRef(rawId);
|
||||
if (!issue) {
|
||||
res.status(404).json({ error: "Issue not found" });
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
const result = await svc.runsForIssue(issue.companyId, id);
|
||||
const result = await svc.runsForIssue(issue.companyId, issue.id);
|
||||
res.json(result);
|
||||
});
|
||||
|
||||
|
||||
35
server/src/services/heartbeat-run-summary.ts
Normal file
35
server/src/services/heartbeat-run-summary.ts
Normal file
@@ -0,0 +1,35 @@
|
||||
function truncateSummaryText(value: unknown, maxLength = 500) {
|
||||
if (typeof value !== "string") return null;
|
||||
return value.length > maxLength ? value.slice(0, maxLength) : value;
|
||||
}
|
||||
|
||||
function readNumericField(record: Record<string, unknown>, key: string) {
|
||||
return key in record ? record[key] ?? null : undefined;
|
||||
}
|
||||
|
||||
export function summarizeHeartbeatRunResultJson(
|
||||
resultJson: Record<string, unknown> | null | undefined,
|
||||
): Record<string, unknown> | null {
|
||||
if (!resultJson || typeof resultJson !== "object" || Array.isArray(resultJson)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const summary: Record<string, unknown> = {};
|
||||
const textFields = ["summary", "result", "message", "error"] as const;
|
||||
for (const key of textFields) {
|
||||
const value = truncateSummaryText(resultJson[key]);
|
||||
if (value !== null) {
|
||||
summary[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
const numericFieldAliases = ["total_cost_usd", "cost_usd", "costUsd"] as const;
|
||||
for (const key of numericFieldAliases) {
|
||||
const value = readNumericField(resultJson, key);
|
||||
if (value !== undefined && value !== null) {
|
||||
summary[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
return Object.keys(summary).length > 0 ? summary : null;
|
||||
}
|
||||
@@ -24,6 +24,7 @@ import { createLocalAgentJwt } from "../agent-auth-jwt.js";
|
||||
import { parseObject, asBoolean, asNumber, appendWithCap, MAX_EXCERPT_BYTES } from "../adapters/utils.js";
|
||||
import { secretService } from "./secrets.js";
|
||||
import { resolveDefaultAgentWorkspaceDir } from "../home-paths.js";
|
||||
import { summarizeHeartbeatRunResultJson } from "./heartbeat-run-summary.js";
|
||||
import {
|
||||
buildWorkspaceReadyComment,
|
||||
ensureRuntimeServicesForRun,
|
||||
@@ -46,38 +47,6 @@ const DEFERRED_WAKE_CONTEXT_KEY = "_paperclipWakeContext";
|
||||
const startLocksByAgent = new Map<string, Promise<void>>();
|
||||
const REPO_ONLY_CWD_SENTINEL = "/__paperclip_repo_only__";
|
||||
|
||||
const summarizedHeartbeatRunResultJson = sql<Record<string, unknown> | null>`
|
||||
CASE
|
||||
WHEN ${heartbeatRuns.resultJson} IS NULL THEN NULL
|
||||
ELSE NULLIF(
|
||||
jsonb_strip_nulls(
|
||||
jsonb_build_object(
|
||||
'summary', CASE
|
||||
WHEN ${heartbeatRuns.resultJson} ->> 'summary' IS NULL THEN NULL
|
||||
ELSE left(${heartbeatRuns.resultJson} ->> 'summary', 500)
|
||||
END,
|
||||
'result', CASE
|
||||
WHEN ${heartbeatRuns.resultJson} ->> 'result' IS NULL THEN NULL
|
||||
ELSE left(${heartbeatRuns.resultJson} ->> 'result', 500)
|
||||
END,
|
||||
'message', CASE
|
||||
WHEN ${heartbeatRuns.resultJson} ->> 'message' IS NULL THEN NULL
|
||||
ELSE left(${heartbeatRuns.resultJson} ->> 'message', 500)
|
||||
END,
|
||||
'error', CASE
|
||||
WHEN ${heartbeatRuns.resultJson} ->> 'error' IS NULL THEN NULL
|
||||
ELSE left(${heartbeatRuns.resultJson} ->> 'error', 500)
|
||||
END,
|
||||
'total_cost_usd', ${heartbeatRuns.resultJson} -> 'total_cost_usd',
|
||||
'cost_usd', ${heartbeatRuns.resultJson} -> 'cost_usd',
|
||||
'costUsd', ${heartbeatRuns.resultJson} -> 'costUsd'
|
||||
)
|
||||
),
|
||||
'{}'::jsonb
|
||||
)
|
||||
END
|
||||
`;
|
||||
|
||||
const heartbeatRunListColumns = {
|
||||
id: heartbeatRuns.id,
|
||||
companyId: heartbeatRuns.companyId,
|
||||
@@ -92,7 +61,7 @@ const heartbeatRunListColumns = {
|
||||
exitCode: heartbeatRuns.exitCode,
|
||||
signal: heartbeatRuns.signal,
|
||||
usageJson: heartbeatRuns.usageJson,
|
||||
resultJson: summarizedHeartbeatRunResultJson.as("resultJson"),
|
||||
resultJson: heartbeatRuns.resultJson,
|
||||
sessionIdBefore: heartbeatRuns.sessionIdBefore,
|
||||
sessionIdAfter: heartbeatRuns.sessionIdAfter,
|
||||
logStore: heartbeatRuns.logStore,
|
||||
@@ -2336,10 +2305,11 @@ export function heartbeatService(db: Db) {
|
||||
)
|
||||
.orderBy(desc(heartbeatRuns.createdAt));
|
||||
|
||||
if (limit) {
|
||||
return query.limit(limit);
|
||||
}
|
||||
return query;
|
||||
const rows = limit ? await query.limit(limit) : await query;
|
||||
return rows.map((row) => ({
|
||||
...row,
|
||||
resultJson: summarizeHeartbeatRunResultJson(row.resultJson),
|
||||
}));
|
||||
},
|
||||
|
||||
getRun,
|
||||
|
||||
@@ -132,7 +132,7 @@ export function InlineEditor({
|
||||
return (
|
||||
<DisplayTag
|
||||
className={cn(
|
||||
"cursor-pointer rounded hover:bg-accent/50 transition-colors",
|
||||
"cursor-pointer rounded hover:bg-accent/50 transition-colors overflow-hidden",
|
||||
pad,
|
||||
!value && "text-muted-foreground italic",
|
||||
className
|
||||
|
||||
@@ -117,7 +117,7 @@ export function MarkdownBody({ children, className }: MarkdownBodyProps) {
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
"paperclip-markdown prose prose-sm max-w-none prose-pre:whitespace-pre-wrap prose-pre:break-words prose-code:break-all",
|
||||
"paperclip-markdown prose prose-sm max-w-none break-words overflow-hidden prose-pre:whitespace-pre-wrap prose-pre:break-words prose-code:break-all",
|
||||
theme === "dark" && "prose-invert",
|
||||
className,
|
||||
)}
|
||||
|
||||
Reference in New Issue
Block a user