From 03ad5c5bea50e3d89e2b8153cbd11c932c6ae298 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 15 May 2026 08:54:55 -0500 Subject: [PATCH] [codex] Add issue document locking (#6009) ## Thinking Path > - Paperclip orchestrates AI-agent companies through company-scoped issues, comments, and issue documents. > - Issue documents are the durable place where plans, handoffs, and other work artifacts are revised over time. > - Some documents need to be preserved as operator-approved snapshots while agents continue working on the same issue. > - Without document locking, a later board or agent write can overwrite the document key that reviewers expected to remain stable. > - This pull request adds board-managed issue document locks and makes agent writes to locked keys create a derived document instead of mutating the locked document. > - The benefit is safer document handoffs: approved or frozen issue documents stay immutable until the board explicitly unlocks them. ## What Changed - Added `locked_at`, `locked_by_agent_id`, and `locked_by_user_id` document fields plus migration `0085_tranquil_the_executioner.sql`. - Added document lock/unlock service behavior, route endpoints, activity events, and locked-document write protections. - Made agent document writes to locked keys create a new derived key such as `plan-2` rather than overwriting the locked document. - Surfaced lock state through shared issue document types, UI API methods, document header lock controls, and activity formatting. - Added server and UI tests for lock/unlock behavior, locked document immutability, and UI action visibility. - Updated `doc/SPEC-implementation.md` with the V1 document lock contract and endpoints. ## Verification - `git rebase public-gh/master` completed cleanly after committing the branch changes. - `git diff --check` passed before commit. - `pnpm run preflight:workspace-links && pnpm exec vitest run server/src/__tests__/documents-service.test.ts server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts ui/src/components/IssueDocumentsSection.test.tsx ui/src/components/IssueContinuationHandoff.test.tsx ui/src/lib/document-revisions.test.ts` passed: 5 files, 32 tests. ## Risks - Medium risk because this changes the document persistence contract and adds a migration. - The migration uses `ADD COLUMN IF NOT EXISTS` and guarded foreign-key creation so it remains safe for users who may have already applied an earlier copy of the migration. - Locked documents intentionally reject board edits/deletes/restores until unlocked; any existing workflows that expected direct overwrite need to unlock first. - Agent writes to locked keys now create derived documents, which may create extra issue documents when agents retry locked writes. ## Model Used - OpenAI Codex coding agent based on GPT-5, with tool use and local code execution in the Paperclip worktree. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/SPEC-implementation.md | 6 + .../0085_tranquil_the_executioner.sql | 8 + packages/db/src/migrations/meta/_journal.json | 7 + packages/db/src/schema/documents.ts | 3 + packages/plugins/sdk/src/testing.ts | 3 + packages/shared/src/types/issue.ts | 3 + .../src/__tests__/documents-service.test.ts | 81 ++++++ ...ue-agent-mutation-ownership-routes.test.ts | 1 + server/src/routes/issues.ts | 94 ++++++ server/src/services/documents.ts | 267 +++++++++++++++++- ui/src/api/issues.ts | 4 + .../IssueContinuationHandoff.test.tsx | 3 + .../components/IssueDocumentsSection.test.tsx | 104 +++++++ ui/src/components/IssueDocumentsSection.tsx | 102 +++++-- ui/src/lib/activity-format.ts | 12 +- ui/src/lib/document-revisions.test.ts | 3 + ui/src/pages/IssueDetail.tsx | 1 + ui/storybook/fixtures/paperclipData.ts | 9 + 18 files changed, 684 insertions(+), 27 deletions(-) create mode 100644 packages/db/src/migrations/0085_tranquil_the_executioner.sql diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 45f2e15b..0f7152f9 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -376,6 +376,10 @@ Operational policy: - `created_by_user_id` uuid/text fk null - `updated_by_agent_id` uuid fk null - `updated_by_user_id` uuid/text fk null + - `locked_at` timestamptz null + - `locked_by_agent_id` uuid fk null + - `locked_by_user_id` uuid/text fk null + - Locked documents are immutable until unlocked. Board operators can lock/unlock; agent writes to a locked key create a new issue document with a derived key instead of overwriting the locked document. - `document_revisions` stores append-only history: - `id` uuid pk - `company_id` uuid fk not null @@ -524,6 +528,8 @@ All endpoints are under `/api` and return JSON. - `GET /issues/:issueId/documents` - `GET /issues/:issueId/documents/:key` - `PUT /issues/:issueId/documents/:key` +- `POST /issues/:issueId/documents/:key/lock` +- `POST /issues/:issueId/documents/:key/unlock` - `GET /issues/:issueId/documents/:key/revisions` - `DELETE /issues/:issueId/documents/:key` - `POST /issues/:issueId/checkout` diff --git a/packages/db/src/migrations/0085_tranquil_the_executioner.sql b/packages/db/src/migrations/0085_tranquil_the_executioner.sql new file mode 100644 index 00000000..5dc6e3cb --- /dev/null +++ b/packages/db/src/migrations/0085_tranquil_the_executioner.sql @@ -0,0 +1,8 @@ +ALTER TABLE "documents" ADD COLUMN IF NOT EXISTS "locked_at" timestamp with time zone;--> statement-breakpoint +ALTER TABLE "documents" ADD COLUMN IF NOT EXISTS "locked_by_agent_id" uuid;--> statement-breakpoint +ALTER TABLE "documents" ADD COLUMN IF NOT EXISTS "locked_by_user_id" text;--> statement-breakpoint +DO $$ BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'documents_locked_by_agent_id_agents_id_fk') THEN + ALTER TABLE "documents" ADD CONSTRAINT "documents_locked_by_agent_id_agents_id_fk" FOREIGN KEY ("locked_by_agent_id") REFERENCES "public"."agents"("id") ON DELETE set null ON UPDATE no action; + END IF; +END $$; diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 9d124c7d..953d0a09 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -596,6 +596,13 @@ "when": 1778355326070, "tag": "0084_issue_recovery_actions", "breakpoints": true + }, + { + "idx": 85, + "version": "7", + "when": 1778787362162, + "tag": "0085_tranquil_the_executioner", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/documents.ts b/packages/db/src/schema/documents.ts index 1dae7e1d..9b56c287 100644 --- a/packages/db/src/schema/documents.ts +++ b/packages/db/src/schema/documents.ts @@ -16,6 +16,9 @@ export const documents = pgTable( createdByUserId: text("created_by_user_id"), updatedByAgentId: uuid("updated_by_agent_id").references(() => agents.id, { onDelete: "set null" }), updatedByUserId: text("updated_by_user_id"), + lockedAt: timestamp("locked_at", { withTimezone: true }), + lockedByAgentId: uuid("locked_by_agent_id").references(() => agents.id, { onDelete: "set null" }), + lockedByUserId: text("locked_by_user_id"), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(), }, diff --git a/packages/plugins/sdk/src/testing.ts b/packages/plugins/sdk/src/testing.ts index ce2e2d40..20f18e3f 100644 --- a/packages/plugins/sdk/src/testing.ts +++ b/packages/plugins/sdk/src/testing.ts @@ -1604,6 +1604,9 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness { createdByUserId: existing?.createdByUserId ?? null, updatedByAgentId: null, updatedByUserId: null, + lockedAt: existing?.lockedAt ?? null, + lockedByAgentId: existing?.lockedByAgentId ?? null, + lockedByUserId: existing?.lockedByUserId ?? null, createdAt: existing?.createdAt ?? now, updatedAt: now, body: input.body, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index 7bbdde36..d822aa0f 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -96,6 +96,9 @@ export interface IssueDocumentSummary { createdByUserId: string | null; updatedByAgentId: string | null; updatedByUserId: string | null; + lockedAt: Date | null; + lockedByAgentId: string | null; + lockedByUserId: string | null; createdAt: Date; updatedAt: Date; } diff --git a/server/src/__tests__/documents-service.test.ts b/server/src/__tests__/documents-service.test.ts index 12658845..44a6d3b1 100644 --- a/server/src/__tests__/documents-service.test.ts +++ b/server/src/__tests__/documents-service.test.ts @@ -112,4 +112,85 @@ describeEmbeddedPostgres("documentService system issue documents", () => { body: "# Handoff", })); }); + + 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, + })); + }); }); diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index a4d1d9f7..407592d5 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -410,6 +410,7 @@ describe("agent issue mutation checkout ownership", () => { key: "plan", createdByAgentId: ownerAgentId, createdByRunId: ownerRunId, + lockedDocumentStrategy: "create_new_document", }), ); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index b0f61244..5de602a1 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -2032,8 +2032,11 @@ export function issueRoutes( createdByAgentId: actor.agentId ?? null, createdByUserId: actor.actorType === "user" ? actor.actorId : null, createdByRunId: actor.runId ?? null, + lockedDocumentStrategy: req.actor.type === "agent" ? "create_new_document" : "conflict", }); const doc = result.document; + const redirectedFromLockedDocument = + "redirectedFromLockedDocument" in result ? result.redirectedFromLockedDocument : null; await issueReferencesSvc.syncDocument(doc.id); const referenceSummaryAfter = await issueReferencesSvc.listIssueReferenceSummary(issue.id); const referenceDiff = issueReferencesSvc.diffIssueReferenceSummary(referenceSummaryBefore, referenceSummaryAfter); @@ -2053,6 +2056,7 @@ export function issueRoutes( title: doc.title, format: doc.format, revisionNumber: doc.latestRevisionNumber, + redirectedFromLockedDocument, ...summarizeIssueReferenceActivityDetails({ addedReferencedIssues: referenceDiff.addedReferencedIssues.map(summarizeIssueRelationForActivity), removedReferencedIssues: referenceDiff.removedReferencedIssues.map(summarizeIssueRelationForActivity), @@ -2086,6 +2090,96 @@ export function issueRoutes( res.status(result.created ? 201 : 200).json(doc); }); + router.post("/issues/:id/documents/:key/lock", async (req, res) => { + const id = req.params.id as string; + const issue = await svc.getById(id); + if (!issue) { + res.status(404).json({ error: "Issue not found" }); + return; + } + assertCompanyAccess(req, issue.companyId); + if (req.actor.type !== "board") { + res.status(403).json({ error: "Board authentication required" }); + return; + } + const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); + if (!keyParsed.success) { + res.status(400).json({ error: "Invalid document key", details: keyParsed.error.issues }); + return; + } + + const actor = getActorInfo(req); + const result = await documentsSvc.lockIssueDocument({ + issueId: issue.id, + key: keyParsed.data, + lockedByAgentId: actor.agentId ?? null, + lockedByUserId: actor.actorType === "user" ? actor.actorId : null, + }); + + if (result.changed) { + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + action: "issue.document_locked", + entityType: "issue", + entityId: issue.id, + details: { + key: result.document.key, + documentId: result.document.id, + title: result.document.title, + lockedAt: result.document.lockedAt, + }, + }); + } + + res.json(result.document); + }); + + router.post("/issues/:id/documents/:key/unlock", async (req, res) => { + const id = req.params.id as string; + const issue = await svc.getById(id); + if (!issue) { + res.status(404).json({ error: "Issue not found" }); + return; + } + assertCompanyAccess(req, issue.companyId); + if (req.actor.type !== "board") { + res.status(403).json({ error: "Board authentication required" }); + return; + } + const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); + if (!keyParsed.success) { + res.status(400).json({ error: "Invalid document key", details: keyParsed.error.issues }); + return; + } + + const actor = getActorInfo(req); + const result = await documentsSvc.unlockIssueDocument(issue.id, keyParsed.data); + + if (result.changed) { + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + action: "issue.document_unlocked", + entityType: "issue", + entityId: issue.id, + details: { + key: result.document.key, + documentId: result.document.id, + title: result.document.title, + }, + }); + } + + res.json(result.document); + }); + router.get("/issues/:id/documents/:key/revisions", async (req, res) => { const id = req.params.id as string; const issue = await svc.getById(id); diff --git a/server/src/services/documents.ts b/server/src/services/documents.ts index 5c5ee747..dc322397 100644 --- a/server/src/services/documents.ts +++ b/server/src/services/documents.ts @@ -17,6 +17,20 @@ function isUniqueViolation(error: unknown): boolean { return !!error && typeof error === "object" && "code" in error && (error as { code?: string }).code === "23505"; } +function nextAvailableDocumentKey(sourceKey: string, existingKeys: string[]) { + const usedKeys = new Set(existingKeys); + for (let index = 2; index < 1000; index += 1) { + const suffix = `-${index}`; + const baseMaxLength = 64 - suffix.length; + const base = sourceKey.slice(0, baseMaxLength).replace(/[-_]+$/g, "") || "document"; + const candidate = `${base}${suffix}`; + if (!usedKeys.has(candidate) && issueDocumentKeySchema.safeParse(candidate).success) { + return candidate; + } + } + throw conflict("Unable to choose a new document key for locked document", { key: sourceKey }); +} + export function extractLegacyPlanBody(description: string | null | undefined) { if (!description) return null; const match = /\s*([\s\S]*?)\s*<\/plan>/i.exec(description); @@ -40,6 +54,9 @@ function mapIssueDocumentRow( createdByUserId: string | null; updatedByAgentId: string | null; updatedByUserId: string | null; + lockedAt: Date | null; + lockedByAgentId: string | null; + lockedByUserId: string | null; createdAt: Date; updatedAt: Date; }, @@ -59,6 +76,9 @@ function mapIssueDocumentRow( createdByUserId: row.createdByUserId, updatedByAgentId: row.updatedByAgentId, updatedByUserId: row.updatedByUserId, + lockedAt: row.lockedAt, + lockedByAgentId: row.lockedByAgentId, + lockedByUserId: row.lockedByUserId, createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -78,6 +98,9 @@ const issueDocumentSelect = { createdByUserId: documents.createdByUserId, updatedByAgentId: documents.updatedByAgentId, updatedByUserId: documents.updatedByUserId, + lockedAt: documents.lockedAt, + lockedByAgentId: documents.lockedByAgentId, + lockedByUserId: documents.lockedByUserId, createdAt: documents.createdAt, updatedAt: documents.updatedAt, }; @@ -179,6 +202,7 @@ export function documentService(db: Db) { createdByAgentId?: string | null; createdByUserId?: string | null; createdByRunId?: string | null; + lockedDocumentStrategy?: "conflict" | "create_new_document"; }) => { const key = normalizeDocumentKey(input.key); const issue = await db @@ -188,8 +212,10 @@ export function documentService(db: Db) { .then((rows) => rows[0] ?? null); if (!issue) throw notFound("Issue not found"); - try { - return await db.transaction(async (tx) => { + const maxAttempts = input.lockedDocumentStrategy === "create_new_document" ? 3 : 1; + for (let attempt = 0; attempt < maxAttempts; attempt += 1) { + try { + return await db.transaction(async (tx) => { const now = new Date(); const existing = await tx .select({ @@ -206,6 +232,9 @@ export function documentService(db: Db) { createdByUserId: documents.createdByUserId, updatedByAgentId: documents.updatedByAgentId, updatedByUserId: documents.updatedByUserId, + lockedAt: documents.lockedAt, + lockedByAgentId: documents.lockedByAgentId, + lockedByUserId: documents.lockedByUserId, createdAt: documents.createdAt, updatedAt: documents.updatedAt, }) @@ -215,6 +244,102 @@ export function documentService(db: Db) { .then((rows) => rows[0] ?? null); if (existing) { + if (existing.lockedAt) { + if (input.lockedDocumentStrategy === "create_new_document") { + const issueDocumentKeys = await tx + .select({ key: issueDocuments.key }) + .from(issueDocuments) + .where(eq(issueDocuments.issueId, issue.id)); + const fallbackKey = nextAvailableDocumentKey(key, issueDocumentKeys.map((row) => row.key)); + + const [document] = await tx + .insert(documents) + .values({ + companyId: issue.companyId, + title: input.title ?? null, + format: input.format, + latestBody: input.body, + latestRevisionId: null, + latestRevisionNumber: 1, + createdByAgentId: input.createdByAgentId ?? null, + createdByUserId: input.createdByUserId ?? null, + updatedByAgentId: input.createdByAgentId ?? null, + updatedByUserId: input.createdByUserId ?? null, + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, + createdAt: now, + updatedAt: now, + }) + .returning(); + + const [revision] = await tx + .insert(documentRevisions) + .values({ + companyId: issue.companyId, + documentId: document.id, + revisionNumber: 1, + title: input.title ?? null, + format: input.format, + body: input.body, + changeSummary: input.changeSummary ?? null, + createdByAgentId: input.createdByAgentId ?? null, + createdByUserId: input.createdByUserId ?? null, + createdByRunId: input.createdByRunId ?? null, + createdAt: now, + }) + .returning(); + + await tx + .update(documents) + .set({ latestRevisionId: revision.id }) + .where(eq(documents.id, document.id)); + + await tx.insert(issueDocuments).values({ + companyId: issue.companyId, + issueId: issue.id, + documentId: document.id, + key: fallbackKey, + createdAt: now, + updatedAt: now, + }); + + return { + created: true as const, + redirectedFromLockedDocument: { + id: existing.id, + key: existing.key, + }, + document: { + id: document.id, + companyId: issue.companyId, + issueId: issue.id, + key: fallbackKey, + title: document.title, + format: document.format, + body: document.latestBody, + latestRevisionId: revision.id, + latestRevisionNumber: 1, + createdByAgentId: document.createdByAgentId, + createdByUserId: document.createdByUserId, + updatedByAgentId: document.updatedByAgentId, + updatedByUserId: document.updatedByUserId, + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, + createdAt: document.createdAt, + updatedAt: document.updatedAt, + }, + }; + } + + throw conflict("Document is locked", { + key: existing.key, + documentId: existing.id, + lockedAt: existing.lockedAt, + }); + } + if (!input.baseRevisionId) { throw conflict("Document update requires baseRevisionId", { currentRevisionId: existing.latestRevisionId, @@ -274,6 +399,9 @@ export function documentService(db: Db) { latestRevisionNumber: nextRevisionNumber, updatedByAgentId: input.createdByAgentId ?? null, updatedByUserId: input.createdByUserId ?? null, + lockedAt: existing.lockedAt, + lockedByAgentId: existing.lockedByAgentId, + lockedByUserId: existing.lockedByUserId, updatedAt: now, }, }; @@ -296,6 +424,9 @@ export function documentService(db: Db) { createdByUserId: input.createdByUserId ?? null, updatedByAgentId: input.createdByAgentId ?? null, updatedByUserId: input.createdByUserId ?? null, + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, createdAt: now, updatedAt: now, }) @@ -348,17 +479,26 @@ export function documentService(db: Db) { createdByUserId: document.createdByUserId, updatedByAgentId: document.updatedByAgentId, updatedByUserId: document.updatedByUserId, + lockedAt: document.lockedAt, + lockedByAgentId: document.lockedByAgentId, + lockedByUserId: document.lockedByUserId, createdAt: document.createdAt, updatedAt: document.updatedAt, }, }; - }); - } catch (error) { - if (isUniqueViolation(error)) { - throw conflict("Document key already exists on this issue", { key }); + }); + } catch (error) { + if (isUniqueViolation(error)) { + if (input.lockedDocumentStrategy === "create_new_document" && attempt < maxAttempts - 1) { + continue; + } + throw conflict("Document key already exists on this issue", { key }); + } + throw error; } - throw error; } + + throw conflict("Unable to choose a new document key for locked document", { key }); }, restoreIssueDocumentRevision: async (input: { @@ -378,6 +518,13 @@ export function documentService(db: Db) { .then((rows) => rows[0] ?? null); if (!existing) throw notFound("Document not found"); + if (existing.lockedAt) { + throw conflict("Document is locked", { + key: existing.key, + documentId: existing.id, + lockedAt: existing.lockedAt, + }); + } const revision = await tx .select({ @@ -455,6 +602,105 @@ export function documentService(db: Db) { }); }, + lockIssueDocument: async (input: { + issueId: string; + key: string; + lockedByAgentId?: string | null; + lockedByUserId?: string | null; + }) => { + const key = normalizeDocumentKey(input.key); + return db.transaction(async (tx) => { + const existing = await tx + .select(issueDocumentSelect) + .from(issueDocuments) + .innerJoin(documents, eq(issueDocuments.documentId, documents.id)) + .where(and(eq(issueDocuments.issueId, input.issueId), eq(issueDocuments.key, key))) + .then((rows) => rows[0] ?? null); + + if (!existing) throw notFound("Document not found"); + if (existing.lockedAt) { + return { + changed: false as const, + document: mapIssueDocumentRow(existing, true), + }; + } + + const now = new Date(); + await tx + .update(documents) + .set({ + lockedAt: now, + lockedByAgentId: input.lockedByAgentId ?? null, + lockedByUserId: input.lockedByUserId ?? null, + updatedAt: now, + }) + .where(eq(documents.id, existing.id)); + + await tx + .update(issueDocuments) + .set({ updatedAt: now }) + .where(eq(issueDocuments.documentId, existing.id)); + + return { + changed: true as const, + document: { + ...mapIssueDocumentRow(existing, true), + lockedAt: now, + lockedByAgentId: input.lockedByAgentId ?? null, + lockedByUserId: input.lockedByUserId ?? null, + updatedAt: now, + }, + }; + }); + }, + + unlockIssueDocument: async (issueId: string, rawKey: string) => { + const key = normalizeDocumentKey(rawKey); + return db.transaction(async (tx) => { + const existing = await tx + .select(issueDocumentSelect) + .from(issueDocuments) + .innerJoin(documents, eq(issueDocuments.documentId, documents.id)) + .where(and(eq(issueDocuments.issueId, issueId), eq(issueDocuments.key, key))) + .then((rows) => rows[0] ?? null); + + if (!existing) throw notFound("Document not found"); + if (!existing.lockedAt) { + return { + changed: false as const, + document: mapIssueDocumentRow(existing, true), + }; + } + + const now = new Date(); + await tx + .update(documents) + .set({ + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, + updatedAt: now, + }) + .where(eq(documents.id, existing.id)); + + await tx + .update(issueDocuments) + .set({ updatedAt: now }) + .where(eq(issueDocuments.documentId, existing.id)); + + return { + changed: true as const, + document: { + ...mapIssueDocumentRow(existing, true), + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, + updatedAt: now, + }, + }; + }); + }, + deleteIssueDocument: async (issueId: string, rawKey: string) => { const key = normalizeDocumentKey(rawKey); return db.transaction(async (tx) => { @@ -466,6 +712,13 @@ export function documentService(db: Db) { .then((rows) => rows[0] ?? null); if (!existing) return null; + if (existing.lockedAt) { + throw conflict("Document is locked", { + key: existing.key, + documentId: existing.id, + lockedAt: existing.lockedAt, + }); + } await tx.delete(issueDocuments).where(eq(issueDocuments.documentId, existing.id)); await tx.delete(documents).where(eq(documents.id, existing.id)); diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index 389699a4..0bf061d6 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -259,6 +259,10 @@ export const issuesApi = { getDocument: (id: string, key: string) => api.get(`/issues/${id}/documents/${encodeURIComponent(key)}`), upsertDocument: (id: string, key: string, data: UpsertIssueDocument) => api.put(`/issues/${id}/documents/${encodeURIComponent(key)}`, data), + lockDocument: (id: string, key: string) => + api.post(`/issues/${id}/documents/${encodeURIComponent(key)}/lock`, {}), + unlockDocument: (id: string, key: string) => + api.post(`/issues/${id}/documents/${encodeURIComponent(key)}/unlock`, {}), listDocumentRevisions: (id: string, key: string) => api.get(`/issues/${id}/documents/${encodeURIComponent(key)}/revisions`), restoreDocumentRevision: (id: string, key: string, revisionId: string) => diff --git a/ui/src/components/IssueContinuationHandoff.test.tsx b/ui/src/components/IssueContinuationHandoff.test.tsx index 80e7a3d5..78278569 100644 --- a/ui/src/components/IssueContinuationHandoff.test.tsx +++ b/ui/src/components/IssueContinuationHandoff.test.tsx @@ -38,6 +38,9 @@ function createHandoffDocument(): IssueDocument { createdByUserId: null, updatedByAgentId: "agent-1", updatedByUserId: null, + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, createdAt: new Date("2026-04-19T12:00:00.000Z"), updatedAt: new Date("2026-04-19T12:05:00.000Z"), }; diff --git a/ui/src/components/IssueDocumentsSection.test.tsx b/ui/src/components/IssueDocumentsSection.test.tsx index 31a6e76a..0ce5cb66 100644 --- a/ui/src/components/IssueDocumentsSection.test.tsx +++ b/ui/src/components/IssueDocumentsSection.test.tsx @@ -15,6 +15,8 @@ const mockIssuesApi = vi.hoisted(() => ({ listDocumentRevisions: vi.fn(), restoreDocumentRevision: vi.fn(), upsertDocument: vi.fn(), + lockDocument: vi.fn(), + unlockDocument: vi.fn(), deleteDocument: vi.fn(), getDocument: vi.fn(), })); @@ -178,6 +180,9 @@ function createIssueDocument(overrides: Partial = {}): IssueDocum createdByUserId: "user-1", updatedByAgentId: null, updatedByUserId: "user-1", + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, createdAt: new Date("2026-03-31T12:00:00.000Z"), updatedAt: new Date("2026-03-31T12:05:00.000Z"), ...overrides, @@ -306,6 +311,105 @@ describe("IssueDocumentsSection", () => { queryClient.clear(); }); + it("locks documents from the document header action", async () => { + const unlockedDocument = createIssueDocument({ + body: "Draftable plan body", + lockedAt: null, + }); + const lockedDocument = createIssueDocument({ + body: "Draftable plan body", + lockedAt: new Date("2026-03-31T12:06:00.000Z"), + lockedByUserId: "user-1", + updatedAt: new Date("2026-03-31T12:06:00.000Z"), + }); + const issue = createIssue(); + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + mutations: { + retry: false, + }, + }, + }); + + mockIssuesApi.listDocuments + .mockResolvedValueOnce([unlockedDocument]) + .mockResolvedValue([lockedDocument]); + mockIssuesApi.lockDocument.mockResolvedValue(lockedDocument); + + await act(async () => { + root.render( + + + , + ); + }); + await flush(); + await flush(); + + const lockButton = container.querySelector('button[title="Lock document"]'); + expect(lockButton).toBeTruthy(); + + await act(async () => { + lockButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flush(); + + expect(mockIssuesApi.lockDocument).toHaveBeenCalledWith("issue-1", "plan"); + expect(container.querySelector('button[title="Unlock document"]')).toBeTruthy(); + + await act(async () => { + root.unmount(); + }); + queryClient.clear(); + }); + + it("hides direct edit and delete actions for locked documents", async () => { + const issue = createIssue(); + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + mutations: { + retry: false, + }, + }, + }); + + mockIssuesApi.listDocuments.mockResolvedValue([ + createIssueDocument({ + body: "Locked plan body", + lockedAt: new Date("2026-03-31T12:06:00.000Z"), + lockedByUserId: "user-1", + }), + ]); + + await act(async () => { + root.render( + + + , + ); + }); + await flush(); + await flush(); + + expect(container.textContent).toContain("Locked plan body"); + expect(container.textContent).not.toContain("Edit document"); + expect(container.textContent).not.toContain("Delete document"); + expect(container.querySelector('button[title="Unlock document"]')).toBeTruthy(); + + await act(async () => { + root.unmount(); + }); + queryClient.clear(); + }); + it("shows the restored document body immediately after a revision restore", async () => { const blankLatestDocument = createIssueDocument({ body: "", diff --git a/ui/src/components/IssueDocumentsSection.tsx b/ui/src/components/IssueDocumentsSection.tsx index 717fbd0a..303123b9 100644 --- a/ui/src/components/IssueDocumentsSection.tsx +++ b/ui/src/components/IssueDocumentsSection.tsx @@ -32,7 +32,7 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; -import { Check, ChevronDown, ChevronRight, Copy, Diff, Download, FilePenLine, FileText, MoreHorizontal, Plus, Trash2, X } from "lucide-react"; +import { Check, ChevronDown, ChevronRight, Copy, Diff, Download, FilePenLine, FileText, Lock, MoreHorizontal, Plus, Trash2, Unlock, X } from "lucide-react"; import { DocumentDiffModal } from "./DocumentDiffModal"; type DraftState = { @@ -91,6 +91,10 @@ function isDocumentConflictError(error: unknown) { return error instanceof ApiError && error.status === 409; } +function isLockedDocumentError(error: unknown) { + return error instanceof ApiError && error.status === 409 && error.message === "Document is locked"; +} + function downloadDocumentFile(key: string, body: string) { const blob = new Blob([body], { type: "text/markdown;charset=utf-8" }); const url = URL.createObjectURL(blob); @@ -128,6 +132,9 @@ function toDocumentSummary(document: IssueDocument) { createdByUserId: document.createdByUserId, updatedByAgentId: document.updatedByAgentId, updatedByUserId: document.updatedByUserId, + lockedAt: document.lockedAt, + lockedByAgentId: document.lockedByAgentId, + lockedByUserId: document.lockedByUserId, createdAt: document.createdAt, updatedAt: document.updatedAt, }; @@ -136,6 +143,7 @@ function toDocumentSummary(document: IssueDocument) { export function IssueDocumentsSection({ issue, canDeleteDocuments, + canManageDocumentLocks = false, feedbackVotes = [], feedbackDataSharingPreference = "prompt", feedbackTermsUrl = null, @@ -146,6 +154,7 @@ export function IssueDocumentsSection({ }: { issue: Issue; canDeleteDocuments: boolean; + canManageDocumentLocks?: boolean; feedbackVotes?: FeedbackVote[]; feedbackDataSharingPreference?: FeedbackDataSharingPreference; feedbackTermsUrl?: string | null; @@ -279,6 +288,22 @@ export function IssueDocumentsSection({ }, }); + const setDocumentLock = useMutation({ + mutationFn: ({ key, locked }: { key: string; locked: boolean }) => + locked ? issuesApi.lockDocument(issue.id, key) : issuesApi.unlockDocument(issue.id, key), + onSuccess: (document) => { + syncDocumentCaches(document); + setDraft((current) => current?.key === document.key ? null : current); + setDocumentConflict((current) => current?.key === document.key ? null : current); + resetAutosaveState(); + setError(null); + invalidateIssueDocuments(); + }, + onError: (err) => { + setError(err instanceof Error ? err.message : "Failed to update document lock"); + }, + }); + const sortedDocuments = useMemo(() => { return (documents ?? []).filter((doc) => !isSystemIssueDocumentKey(doc.key)).sort((a, b) => { if (a.key === "plan" && b.key !== "plan") return -1; @@ -442,6 +467,12 @@ export function IssueDocumentsSection({ } return true; } catch (err) { + if (isLockedDocumentError(err)) { + setError("Document is locked. Unlock it before editing."); + resetAutosaveState(); + invalidateIssueDocuments(); + return false; + } if (isDocumentConflictError(err)) { try { const latestDocument = await issuesApi.getDocument(issue.id, normalizedKey); @@ -563,6 +594,15 @@ export function IssueDocumentsSection({ setError(null); }, [documentConflict, draft, getDocumentRevisions, resetAutosaveState, returnToLatestRevision]); + const toggleDocumentLock = useCallback((doc: IssueDocument, locked: boolean) => { + if (!canManageDocumentLocks || setDocumentLock.isPending) return; + if (locked && (documentConflict?.key === doc.key || documentHasUnsavedChanges(doc, draft))) { + setError("Save or cancel local changes before changing the document lock."); + return; + } + setDocumentLock.mutate({ key: doc.key, locked }); + }, [canManageDocumentLocks, documentConflict, draft, setDocumentLock]); + const handleDraftBlur = async (event: React.FocusEvent) => { if (event.currentTarget.contains(event.relatedTarget as Node | null)) return; if (autosaveDebounceRef.current) { @@ -789,8 +829,9 @@ export function IssueDocumentsSection({
{sortedDocuments.map((doc) => { - const activeDraft = draft?.key === doc.key && !draft.isNew ? draft : null; - const activeConflict = documentConflict?.key === doc.key ? documentConflict : null; + const isLocked = Boolean(doc.lockedAt); + const activeDraft = !isLocked && draft?.key === doc.key && !draft.isNew ? draft : null; + const activeConflict = !isLocked && documentConflict?.key === doc.key ? documentConflict : null; const isFolded = foldedDocumentKeys.includes(doc.key); const rawRevisionHistory = getDocumentRevisions(doc.key); const revisionState = deriveDocumentRevisionState(doc, rawRevisionHistory); @@ -809,6 +850,7 @@ export function IssueDocumentsSection({ const displayedUpdatedAt = selectedHistoricalRevision?.createdAt ?? currentRevision.createdAt; const showTitle = !isPlanKey(doc.key) && !!displayedTitle.trim() && !titlesMatchKey(displayedTitle, doc.key); const canVoteOnDocument = Boolean(doc.latestRevisionId && doc.updatedByAgentId && !doc.updatedByUserId && onVote); + const lockActionPending = setDocumentLock.isPending && setDocumentLock.variables?.key === doc.key; return (
{displayedTitle}

}
+ {canManageDocumentLocks ? ( + + ) : isLocked ? ( + + + + ) : null} - + {!isLocked ? ( + + ) : null}
diff --git a/ui/src/lib/activity-format.ts b/ui/src/lib/activity-format.ts index d1cd2b48..c2490e8b 100644 --- a/ui/src/lib/activity-format.ts +++ b/ui/src/lib/activity-format.ts @@ -32,6 +32,8 @@ const ACTIVITY_ROW_VERBS: Record = { "issue.attachment_removed": "removed attachment from", "issue.document_created": "created document for", "issue.document_updated": "updated document on", + "issue.document_locked": "locked document on", + "issue.document_unlocked": "unlocked document on", "issue.document_deleted": "deleted document from", "issue.monitor_scheduled": "scheduled monitor on", "issue.monitor_triggered": "triggered monitor for", @@ -88,6 +90,8 @@ const ISSUE_ACTIVITY_LABELS: Record = { "issue.attachment_removed": "removed an attachment", "issue.document_created": "created a document", "issue.document_updated": "updated a document", + "issue.document_locked": "locked a document", + "issue.document_unlocked": "unlocked a document", "issue.document_deleted": "deleted a document", "issue.monitor_scheduled": "scheduled a monitor", "issue.monitor_triggered": "triggered a monitor", @@ -333,7 +337,13 @@ export function formatIssueActivityAction( } if ( - (action === "issue.document_created" || action === "issue.document_updated" || action === "issue.document_deleted") && + ( + action === "issue.document_created" || + action === "issue.document_updated" || + action === "issue.document_locked" || + action === "issue.document_unlocked" || + action === "issue.document_deleted" + ) && details ) { const key = typeof details.key === "string" ? details.key : "document"; diff --git a/ui/src/lib/document-revisions.test.ts b/ui/src/lib/document-revisions.test.ts index 53e18713..ac9821d0 100644 --- a/ui/src/lib/document-revisions.test.ts +++ b/ui/src/lib/document-revisions.test.ts @@ -17,6 +17,9 @@ function createDocument(overrides: Partial = {}): IssueDocument { createdByUserId: null, updatedByAgentId: "agent-1", updatedByUserId: null, + lockedAt: null, + lockedByAgentId: null, + lockedByUserId: null, createdAt: new Date("2026-04-10T15:00:00.000Z"), updatedAt: new Date("2026-04-10T16:00:00.000Z"), ...overrides, diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index 95156440..b8e71f2e 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -3709,6 +3709,7 @@ export function IssueDetail() {