mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 14:19:00 +01:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Task documents keep a revision history, and agents read and restore
revisions through the API
> - The exact-revision read returns one revision, but it does not say if
that revision is still current
> - An agent read the revision that it restored earlier by its ID and
reported it as "latest", but another person had saved a newer edit
> - This pull request adds the document's current revision to the
exact-revision read
> - The benefit is that a pinned read cannot pass as a read of the
current document
## Linked Issues or Issue Description
**What happened**
In a chat eval, the agent restored a plan revision. Then another person
saved a new revision. The user asked the agent to read the current plan.
The agent called `GET
/api/issues/{id}/documents/plan/revisions/{revisionId}` with the ID of
its own restored revision. The response did not show that a newer
revision existed. The agent reported the old revision as the latest and
said that the read confirmed this.
**Expected behavior**
A read of one exact revision must show if that revision is still
current, and which revision is current. Then the reader can see a newer
edit and read the current document.
**Steps to reproduce**
1. Save two revisions of a task plan.
2. Call `GET
/api/issues/{id}/documents/plan/revisions/{firstRevisionId}`.
3. See that the response gives no sign that revision 2 is the current
revision.
## What Changed
- `getIssueDocumentRevision` now also returns `latestRevisionId`,
`latestRevisionNumber` and `isLatest`.
- Shared type `ExactDocumentRevision` describes this response. The UI
client uses it.
- The OpenAPI description of the exact-revision read names these fields
and points to `GET /api/issues/{id}/documents/{key}` for the current
text.
- The service test checks `isLatest` for an older and a current
revision.
## Verification
- `cd server && pnpm exec vitest run
src/__tests__/documents-service.test.ts
src/__tests__/openapi-routes.test.ts` (42 passed).
- `npx tsc --noEmit -p packages/shared` passes.
## Risks
- Low risk. The change only adds fields to a read response. No caller
removes or renames a field. No migration.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code
execution.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
273 lines
11 KiB
TypeScript
273 lines
11 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
|
import {
|
|
agents,
|
|
companies,
|
|
createDb,
|
|
documentRevisions,
|
|
documents,
|
|
heartbeatRuns,
|
|
issueDocuments,
|
|
issues,
|
|
} from "@paperclipai/db";
|
|
import { ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY } from "@paperclipai/shared";
|
|
import {
|
|
getEmbeddedPostgresTestSupport,
|
|
startEmbeddedPostgresTestDatabase,
|
|
} from "./helpers/embedded-postgres.js";
|
|
import { documentService } from "../services/documents.js";
|
|
|
|
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
|
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
|
|
|
if (!embeddedPostgresSupport.supported) {
|
|
console.warn(
|
|
`Skipping embedded Postgres document service tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
|
);
|
|
}
|
|
|
|
describeEmbeddedPostgres("documentService system issue documents", () => {
|
|
let db!: ReturnType<typeof createDb>;
|
|
let svc!: ReturnType<typeof documentService>;
|
|
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
|
|
|
beforeAll(async () => {
|
|
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-documents-service-");
|
|
db = createDb(tempDb.connectionString);
|
|
svc = documentService(db);
|
|
}, 20_000);
|
|
|
|
afterEach(async () => {
|
|
await db.delete(documentRevisions);
|
|
await db.delete(issueDocuments);
|
|
await db.delete(documents);
|
|
await db.delete(issues);
|
|
await db.delete(heartbeatRuns);
|
|
await db.delete(agents);
|
|
await db.delete(companies);
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await tempDb?.cleanup();
|
|
});
|
|
|
|
async function createIssueWithDocuments() {
|
|
const companyId = randomUUID();
|
|
const issueId = randomUUID();
|
|
|
|
await db.insert(companies).values({
|
|
id: companyId,
|
|
name: "Paperclip",
|
|
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
|
requireBoardApprovalForNewAgents: false,
|
|
});
|
|
|
|
await db.insert(issues).values({
|
|
id: issueId,
|
|
companyId,
|
|
identifier: "PAP-1600",
|
|
title: "System document filtering",
|
|
description: "Validate document filtering",
|
|
status: "in_progress",
|
|
priority: "medium",
|
|
});
|
|
|
|
await svc.upsertIssueDocument({
|
|
issueId,
|
|
key: "plan",
|
|
title: "Plan",
|
|
format: "markdown",
|
|
body: "# Plan",
|
|
createdByUserId: "board-user",
|
|
});
|
|
await svc.upsertIssueDocument({
|
|
issueId,
|
|
key: ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY,
|
|
title: "Continuation Summary",
|
|
format: "markdown",
|
|
body: "# Handoff",
|
|
});
|
|
|
|
return { issueId, companyId };
|
|
}
|
|
|
|
it("filters continuation summaries from default document lists and issue payload summaries", async () => {
|
|
const { issueId } = await createIssueWithDocuments();
|
|
|
|
const defaultDocuments = await svc.listIssueDocuments(issueId);
|
|
expect(defaultDocuments.map((doc) => doc.key)).toEqual(["plan"]);
|
|
|
|
const payload = await svc.getIssueDocumentPayload({ id: issueId, description: null });
|
|
expect(payload.planDocument?.key).toBe("plan");
|
|
expect(payload.documentSummaries.map((doc) => doc.key)).toEqual(["plan"]);
|
|
});
|
|
|
|
it("reads one pinned revision and never substitutes a newer or unrelated document", async () => {
|
|
const { issueId } = await createIssueWithDocuments();
|
|
const original = await svc.getIssueDocumentByKey(issueId, "plan");
|
|
await svc.upsertIssueDocument({ issueId, key: "plan", format: "markdown", body: "# Newer plan", baseRevisionId: original!.latestRevisionId });
|
|
const pinned = await svc.getIssueDocumentRevision(issueId, "plan", original!.latestRevisionId!);
|
|
expect(pinned?.body).toBe("# Plan");
|
|
expect(pinned?.id).toBe(original!.latestRevisionId);
|
|
const current = await svc.getIssueDocumentByKey(issueId, "plan");
|
|
expect(pinned).toMatchObject({ isLatest: false, latestRevisionId: current!.latestRevisionId, latestRevisionNumber: 2 });
|
|
expect(await svc.getIssueDocumentRevision(issueId, "plan", current!.latestRevisionId!)).toMatchObject({ isLatest: true, revisionNumber: 2, body: "# Newer plan" });
|
|
expect(await svc.getIssueDocumentRevision(issueId, "other-document", original!.latestRevisionId!)).toBeNull();
|
|
expect(await svc.getIssueDocumentRevision(randomUUID(), "plan", original!.latestRevisionId!)).toBeNull();
|
|
expect(await svc.getIssueDocumentRevision(issueId, "plan", randomUUID())).toBeNull();
|
|
});
|
|
|
|
it("keeps system documents available for includeSystem and direct fetch callers", async () => {
|
|
const { issueId } = await createIssueWithDocuments();
|
|
|
|
const debugDocuments = await svc.listIssueDocuments(issueId, { includeSystem: true });
|
|
expect(debugDocuments.map((doc) => doc.key)).toEqual([
|
|
ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY,
|
|
"plan",
|
|
]);
|
|
|
|
const directHandoff = await svc.getIssueDocumentByKey(issueId, ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY);
|
|
expect(directHandoff).toEqual(expect.objectContaining({
|
|
key: ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY,
|
|
body: "# Handoff",
|
|
}));
|
|
});
|
|
|
|
it("returns each actual writer run in revision history without attributing human revisions to a run", async () => {
|
|
const { issueId, companyId } = await createIssueWithDocuments();
|
|
const agentId = randomUUID(), firstRunId = randomUUID(), secondRunId = randomUUID();
|
|
await db.insert(agents).values({ id: agentId, companyId, name: "Receipt writer", role: "engineer", adapterType: "process" });
|
|
await db.insert(heartbeatRuns).values([firstRunId, secondRunId].map(id => ({ id, companyId, agentId,
|
|
status: "succeeded", invocationSource: "assignment", triggerDetail: "system", contextSnapshot: { issueId } })));
|
|
const first = await svc.upsertIssueDocument({ issueId, key: "receipt", title: "Receipt", format: "markdown",
|
|
body: '{"total":6}', createdByAgentId: agentId, createdByRunId: firstRunId });
|
|
const second = await svc.upsertIssueDocument({ issueId, key: "receipt", title: "Receipt", format: "markdown",
|
|
body: '{"total":6,"currency":"USD"}', createdByAgentId: agentId, createdByRunId: secondRunId,
|
|
baseRevisionId: first.document.latestRevisionId });
|
|
expect(await svc.listIssueDocumentRevisions(issueId, "receipt")).toMatchObject([
|
|
{ id: second.document.latestRevisionId, revisionNumber: 2, createdByAgentId: agentId, createdByRunId: secondRunId,
|
|
body: '{"total":6,"currency":"USD"}' },
|
|
{ id: first.document.latestRevisionId, revisionNumber: 1, createdByAgentId: agentId, createdByRunId: firstRunId,
|
|
body: '{"total":6}' },
|
|
]);
|
|
expect(await svc.listIssueDocumentRevisions(issueId, "plan")).toMatchObject([{ createdByUserId: "board-user", createdByRunId: null }]);
|
|
expect(await svc.listIssueDocumentRevisions(randomUUID(), "receipt")).toEqual([]);
|
|
});
|
|
|
|
it("explains the revision guard and rejects missing or stale update revisions without changing the document", async () => {
|
|
const { issueId } = await createIssueWithDocuments();
|
|
const current = (await svc.getIssueDocumentByKey(issueId, "plan"))!;
|
|
const update = {
|
|
issueId,
|
|
key: "plan",
|
|
title: "Plan",
|
|
format: "markdown" as const,
|
|
body: "# Revised plan",
|
|
};
|
|
|
|
await expect(svc.upsertIssueDocument(update)).rejects.toMatchObject({
|
|
status: 409,
|
|
message: expect.stringContaining("set baseRevisionId to that latestRevisionId"),
|
|
details: { currentRevisionId: current.latestRevisionId },
|
|
});
|
|
expect(await svc.getIssueDocumentByKey(issueId, "plan")).toMatchObject({
|
|
body: current.body,
|
|
latestRevisionId: current.latestRevisionId,
|
|
});
|
|
|
|
const saved = await svc.upsertIssueDocument({ ...update, baseRevisionId: current.latestRevisionId });
|
|
expect(saved.document.body).toBe(update.body);
|
|
expect(saved.document.latestRevisionNumber).toBe(current.latestRevisionNumber + 1);
|
|
await expect(svc.upsertIssueDocument({
|
|
...update,
|
|
body: "# Stale replacement",
|
|
baseRevisionId: current.latestRevisionId,
|
|
})).rejects.toMatchObject({ status: 409, message: "Document was updated by someone else" });
|
|
expect(await svc.getIssueDocumentByKey(issueId, "plan")).toMatchObject({
|
|
body: saved.document.body,
|
|
latestRevisionId: saved.document.latestRevisionId,
|
|
});
|
|
});
|
|
|
|
it("locks and unlocks issue documents", async () => {
|
|
const { issueId } = await createIssueWithDocuments();
|
|
|
|
const locked = await svc.lockIssueDocument({
|
|
issueId,
|
|
key: "plan",
|
|
lockedByUserId: "board-user",
|
|
});
|
|
|
|
expect(locked.changed).toBe(true);
|
|
expect(locked.document.lockedAt).toBeInstanceOf(Date);
|
|
expect(locked.document.lockedByUserId).toBe("board-user");
|
|
|
|
await expect(svc.upsertIssueDocument({
|
|
issueId,
|
|
key: "plan",
|
|
title: "Plan",
|
|
format: "markdown",
|
|
body: "# Updated plan",
|
|
baseRevisionId: locked.document.latestRevisionId,
|
|
createdByUserId: "board-user",
|
|
})).rejects.toMatchObject({
|
|
status: 409,
|
|
message: "Document is locked",
|
|
});
|
|
|
|
const unlocked = await svc.unlockIssueDocument(issueId, "plan");
|
|
expect(unlocked.changed).toBe(true);
|
|
expect(unlocked.document.lockedAt).toBeNull();
|
|
|
|
const updated = await svc.upsertIssueDocument({
|
|
issueId,
|
|
key: "plan",
|
|
title: "Plan",
|
|
format: "markdown",
|
|
body: "# Updated plan",
|
|
baseRevisionId: unlocked.document.latestRevisionId,
|
|
createdByUserId: "board-user",
|
|
});
|
|
|
|
expect(updated.created).toBe(false);
|
|
expect(updated.document.body).toBe("# Updated plan");
|
|
});
|
|
|
|
it("creates a new document instead of updating a locked document when requested", async () => {
|
|
const { issueId } = await createIssueWithDocuments();
|
|
const locked = await svc.lockIssueDocument({
|
|
issueId,
|
|
key: "plan",
|
|
lockedByUserId: "board-user",
|
|
});
|
|
|
|
const fallback = await svc.upsertIssueDocument({
|
|
issueId,
|
|
key: "plan",
|
|
title: "Plan",
|
|
format: "markdown",
|
|
body: "# Agent replacement plan",
|
|
baseRevisionId: locked.document.latestRevisionId,
|
|
lockedDocumentStrategy: "create_new_document",
|
|
});
|
|
|
|
expect(fallback.created).toBe(true);
|
|
expect(fallback.document.key).toBe("plan-2");
|
|
expect(fallback.document.body).toBe("# Agent replacement plan");
|
|
expect("redirectedFromLockedDocument" in fallback ? fallback.redirectedFromLockedDocument : null)
|
|
.toEqual({ id: locked.document.id, key: "plan" });
|
|
|
|
const originalPlan = await svc.getIssueDocumentByKey(issueId, "plan");
|
|
expect(originalPlan).toEqual(expect.objectContaining({
|
|
body: "# Plan",
|
|
lockedAt: expect.any(Date),
|
|
}));
|
|
|
|
const newPlan = await svc.getIssueDocumentByKey(issueId, "plan-2");
|
|
expect(newPlan).toEqual(expect.objectContaining({
|
|
body: "# Agent replacement plan",
|
|
lockedAt: null,
|
|
}));
|
|
});
|
|
});
|