mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
Show workspace changes and stale notices in issue threads (#5356)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - The issue thread is the operator's durable audit trail for what changed and why > - Workspace changes and stale disposition notices need to be visible in that same timeline without noisy or misleading rendering > - The local branch already contained backend activity details, timeline conversion, and UI rendering work for those events > - This pull request isolates the issue-thread activity work into a standalone branch against `origin/master` > - The benefit is a focused audit-trail PR that can merge independently of the sidebar/operator UI polish branch ## What Changed - Adds readable workspace-change activity details to issue update activity events. - Surfaces workspace-change events in issue chat/timeline rendering. - Makes the existing issue comment migration idempotent. - Folds and renders stale disposition notices inline so they match activity-log styling and spacing. - Adds focused route, timeline, and issue-thread system notice coverage. ## Verification - `pnpm install --frozen-lockfile` - `pnpm exec vitest run server/src/__tests__/issue-activity-events-routes.test.ts ui/src/lib/issue-timeline-events.test.ts ui/src/components/IssueChatThreadSystemNotice.test.tsx` — 3 files passed, 22 tests passed. - Confirmed the PR changes 9 files and does not include `pnpm-lock.yaml` or `.github/workflows/*`. - `pnpm exec vitest run server/src/__tests__/issue-closed-workspace-routes.test.ts` — 1 file passed, 4 tests passed. - `pnpm exec vitest run server/src/__tests__/issue-activity-events-routes.test.ts ui/src/lib/issue-timeline-events.test.ts ui/src/components/IssueChatThreadSystemNotice.test.tsx server/src/services/recovery/successful-run-handoff.test.ts packages/shared/src/validators/issue.test.ts` — 5 files passed, 54 tests passed. - `pnpm --filter @paperclipai/shared typecheck && pnpm --filter @paperclipai/server typecheck && pnpm --filter @paperclipai/ui typecheck`. - `pnpm --filter @paperclipai/ui typecheck` after adding the Storybook screenshot fixture. - Captured Storybook screenshots for the new UI rendering paths: - Collapsed stale notice + workspace-change row: `docs/pr-screenshots/pr-5356/issue-thread-notices-collapsed.png` - Expanded stale notice details: `docs/pr-screenshots/pr-5356/issue-thread-notices-expanded.png` ### Screenshots Collapsed stale notice with workspace-change row:  Expanded stale notice details:  ## Risks - Moderate risk: this touches issue activity serialization and issue-thread rendering, both of which are central operator surfaces. - Migration risk is low: the only migration change makes an existing migration idempotent. - No new migrations are introduced, so there is no cross-PR migration ordering requirement. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5 coding agent, shell/tool-use enabled, used to split the existing branch, verify the isolated PR branch, and create this PR. ## 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 - [x] 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 <noreply@paperclip.ing>
This commit is contained in:
parent
4103978578
commit
d0e9cc76f2
17 changed files with 852 additions and 36 deletions
|
|
@ -1,5 +1,6 @@
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { getTableName } from "drizzle-orm";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { normalizeIssueExecutionPolicy } from "../services/issue-execution-policy.ts";
|
||||
|
||||
|
|
@ -266,6 +267,76 @@ describe("issue activity event routes", () => {
|
|||
});
|
||||
}, 15_000);
|
||||
|
||||
it("logs readable workspace change activity details for issue updates", async () => {
|
||||
const previousProjectWorkspaceId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa";
|
||||
const nextExecutionWorkspaceId = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb";
|
||||
const issue = {
|
||||
...makeIssue(),
|
||||
projectId: "cccccccc-cccc-4ccc-8ccc-cccccccccccc",
|
||||
projectWorkspaceId: previousProjectWorkspaceId,
|
||||
executionWorkspaceId: null,
|
||||
executionWorkspacePreference: "shared_workspace",
|
||||
executionWorkspaceSettings: { mode: "shared_workspace" },
|
||||
};
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
|
||||
...issue,
|
||||
...patch,
|
||||
updatedAt: new Date(),
|
||||
}));
|
||||
|
||||
const dbMock = {
|
||||
select: vi.fn(() => ({
|
||||
from: (table: unknown) => ({
|
||||
where: async () => {
|
||||
const tableName = getTableName(table as Parameters<typeof getTableName>[0]);
|
||||
if (tableName === "project_workspaces") {
|
||||
return [{ id: previousProjectWorkspaceId, name: "Main workspace" }];
|
||||
}
|
||||
if (tableName === "execution_workspaces") {
|
||||
return [{ id: nextExecutionWorkspaceId, name: "Feature workspace" }];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
}),
|
||||
})),
|
||||
};
|
||||
|
||||
const res = await request(await createApp(dbMock))
|
||||
.patch(`/api/issues/${issue.id}`)
|
||||
.send({ executionWorkspaceId: nextExecutionWorkspaceId });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
await vi.waitFor(() => {
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({
|
||||
action: "issue.updated",
|
||||
details: expect.objectContaining({
|
||||
executionWorkspaceId: nextExecutionWorkspaceId,
|
||||
workspaceChange: {
|
||||
from: {
|
||||
label: "Main workspace",
|
||||
projectWorkspaceId: previousProjectWorkspaceId,
|
||||
executionWorkspaceId: null,
|
||||
mode: "shared_workspace",
|
||||
},
|
||||
to: {
|
||||
label: "Feature workspace",
|
||||
projectWorkspaceId: previousProjectWorkspaceId,
|
||||
executionWorkspaceId: nextExecutionWorkspaceId,
|
||||
mode: "shared_workspace",
|
||||
},
|
||||
},
|
||||
_previous: expect.objectContaining({
|
||||
executionWorkspaceId: null,
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("logs successful_run_handoff_resolved when an in_progress issue transitions to done with a pending required handoff", async () => {
|
||||
const issue = { ...makeIssue(), status: "in_progress" };
|
||||
mockIssueService.getById.mockResolvedValue(issue);
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import multer from "multer";
|
|||
import { z } from "zod";
|
||||
import { and, desc, eq, inArray } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { activityLog, issueExecutionDecisions } from "@paperclipai/db";
|
||||
import { activityLog, executionWorkspaces, issueExecutionDecisions, projectWorkspaces } from "@paperclipai/db";
|
||||
import {
|
||||
addIssueCommentSchema,
|
||||
acceptIssueThreadInteractionSchema,
|
||||
|
|
@ -96,6 +96,7 @@ import {
|
|||
redactIssueMonitorExternalRef,
|
||||
setIssueExecutionPolicyMonitorScheduledBy,
|
||||
} from "../services/issue-execution-policy.js";
|
||||
import { parseIssueExecutionWorkspaceSettings } from "../services/execution-workspace-policy.js";
|
||||
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
|
||||
|
||||
const MAX_ISSUE_COMMENT_LIMIT = 500;
|
||||
|
|
@ -142,10 +143,148 @@ const SUCCESSFUL_RUN_HANDOFF_ACTIONS = [
|
|||
"issue.successful_run_handoff_escalated",
|
||||
] as const;
|
||||
|
||||
const ISSUE_WORKSPACE_AUDIT_FIELDS = new Set([
|
||||
"projectWorkspaceId",
|
||||
"executionWorkspaceId",
|
||||
"executionWorkspacePreference",
|
||||
"executionWorkspaceSettings",
|
||||
]);
|
||||
|
||||
function readNonEmptyString(value: unknown): string | null {
|
||||
return typeof value === "string" && value.trim().length > 0 ? value.trim() : null;
|
||||
}
|
||||
|
||||
function hasIssueWorkspaceAuditChange(previous: Record<string, unknown>) {
|
||||
return Object.keys(previous).some((key) => ISSUE_WORKSPACE_AUDIT_FIELDS.has(key));
|
||||
}
|
||||
|
||||
function labelIssueWorkspaceMode(mode: string | null) {
|
||||
switch (mode) {
|
||||
case "shared_workspace":
|
||||
return "Project default";
|
||||
case "isolated_workspace":
|
||||
return "New isolated workspace";
|
||||
case "operator_branch":
|
||||
return "Operator branch";
|
||||
case "reuse_existing":
|
||||
return "Reuse existing workspace";
|
||||
case "agent_default":
|
||||
return "Agent default";
|
||||
case "inherit":
|
||||
return "Inherited workspace";
|
||||
default:
|
||||
return "No workspace";
|
||||
}
|
||||
}
|
||||
|
||||
type IssueWorkspaceAuditInput = {
|
||||
projectWorkspaceId?: string | null;
|
||||
executionWorkspaceId?: string | null;
|
||||
executionWorkspacePreference?: string | null;
|
||||
executionWorkspaceSettings?: unknown;
|
||||
};
|
||||
|
||||
type WorkspaceNameMaps = {
|
||||
projectWorkspaceNames: Map<string, string>;
|
||||
executionWorkspaceNames: Map<string, string>;
|
||||
};
|
||||
|
||||
function emptyWorkspaceNameMaps(): WorkspaceNameMaps {
|
||||
return {
|
||||
projectWorkspaceNames: new Map(),
|
||||
executionWorkspaceNames: new Map(),
|
||||
};
|
||||
}
|
||||
|
||||
function summarizeIssueWorkspaceForActivity(
|
||||
issue: IssueWorkspaceAuditInput,
|
||||
names: WorkspaceNameMaps,
|
||||
) {
|
||||
const settings = parseIssueExecutionWorkspaceSettings(issue.executionWorkspaceSettings);
|
||||
const mode = settings?.mode ?? issue.executionWorkspacePreference ?? null;
|
||||
const executionWorkspaceId = issue.executionWorkspaceId ?? null;
|
||||
const projectWorkspaceId = issue.projectWorkspaceId ?? null;
|
||||
|
||||
const label = (() => {
|
||||
if (executionWorkspaceId) {
|
||||
return names.executionWorkspaceNames.get(executionWorkspaceId) ?? `Workspace ${executionWorkspaceId.slice(0, 8)}`;
|
||||
}
|
||||
if (projectWorkspaceId) {
|
||||
return names.projectWorkspaceNames.get(projectWorkspaceId) ?? `Workspace ${projectWorkspaceId.slice(0, 8)}`;
|
||||
}
|
||||
return labelIssueWorkspaceMode(mode);
|
||||
})();
|
||||
|
||||
return {
|
||||
label,
|
||||
projectWorkspaceId,
|
||||
executionWorkspaceId,
|
||||
mode,
|
||||
};
|
||||
}
|
||||
|
||||
async function buildIssueWorkspaceChangeActivityDetails(
|
||||
db: Db,
|
||||
companyId: string,
|
||||
previousIssue: IssueWorkspaceAuditInput,
|
||||
nextIssue: IssueWorkspaceAuditInput,
|
||||
) {
|
||||
const projectWorkspaceIds = [
|
||||
previousIssue.projectWorkspaceId,
|
||||
nextIssue.projectWorkspaceId,
|
||||
].filter((value): value is string => typeof value === "string" && value.length > 0);
|
||||
const executionWorkspaceIds = [
|
||||
previousIssue.executionWorkspaceId,
|
||||
nextIssue.executionWorkspaceId,
|
||||
].filter((value): value is string => typeof value === "string" && value.length > 0);
|
||||
|
||||
const [projectRows, executionRows] = await Promise.all([
|
||||
projectWorkspaceIds.length > 0
|
||||
? db
|
||||
.select({ id: projectWorkspaces.id, name: projectWorkspaces.name })
|
||||
.from(projectWorkspaces)
|
||||
.where(and(eq(projectWorkspaces.companyId, companyId), inArray(projectWorkspaces.id, projectWorkspaceIds)))
|
||||
: Promise.resolve([]),
|
||||
executionWorkspaceIds.length > 0
|
||||
? db
|
||||
.select({ id: executionWorkspaces.id, name: executionWorkspaces.name })
|
||||
.from(executionWorkspaces)
|
||||
.where(and(eq(executionWorkspaces.companyId, companyId), inArray(executionWorkspaces.id, executionWorkspaceIds)))
|
||||
: Promise.resolve([]),
|
||||
]);
|
||||
|
||||
const names: WorkspaceNameMaps = {
|
||||
projectWorkspaceNames: new Map(projectRows.map((row) => [row.id, row.name])),
|
||||
executionWorkspaceNames: new Map(executionRows.map((row) => [row.id, row.name])),
|
||||
};
|
||||
|
||||
return {
|
||||
from: summarizeIssueWorkspaceForActivity(previousIssue, names),
|
||||
to: summarizeIssueWorkspaceForActivity(nextIssue, names),
|
||||
};
|
||||
}
|
||||
|
||||
function hasExecutionParticipant(value: unknown) {
|
||||
const state = parseIssueExecutionState(value);
|
||||
if (!state || state.status !== "pending") return false;
|
||||
const participant = state.currentParticipant;
|
||||
if (!participant) return false;
|
||||
if (participant.type === "agent") return Boolean(participant.agentId);
|
||||
if (participant.type === "user") return Boolean(participant.userId);
|
||||
return false;
|
||||
}
|
||||
|
||||
function hasScheduledMonitor(input: {
|
||||
existingMonitorNextCheckAt?: Date | null;
|
||||
patchMonitorNextCheckAt?: unknown;
|
||||
executionPolicy?: unknown;
|
||||
}) {
|
||||
if (input.patchMonitorNextCheckAt instanceof Date && !Number.isNaN(input.patchMonitorNextCheckAt.getTime())) return true;
|
||||
if (input.patchMonitorNextCheckAt === undefined && input.existingMonitorNextCheckAt) return true;
|
||||
const policy = normalizeIssueExecutionPolicy(input.executionPolicy ?? null);
|
||||
return Boolean(policy?.monitor?.nextCheckAt);
|
||||
}
|
||||
|
||||
function successfulRunHandoffStateFromActivity(row: {
|
||||
action: string;
|
||||
agentId: string | null;
|
||||
|
|
@ -236,27 +375,6 @@ const INVALID_AGENT_IN_REVIEW_DISPOSITION_MESSAGE =
|
|||
"link or request a pending approval, assign a human reviewer with assigneeUserId, set a typed executionState.currentParticipant through an execution policy, " +
|
||||
"or schedule an issue monitor for an external review/check. After creating one of those review paths, retry the status update.";
|
||||
|
||||
function hasExecutionParticipant(value: unknown) {
|
||||
const state = parseIssueExecutionState(value);
|
||||
if (!state || state.status !== "pending") return false;
|
||||
const participant = state.currentParticipant;
|
||||
if (!participant) return false;
|
||||
if (participant.type === "agent") return Boolean(participant.agentId);
|
||||
if (participant.type === "user") return Boolean(participant.userId);
|
||||
return false;
|
||||
}
|
||||
|
||||
function hasScheduledMonitor(input: {
|
||||
existingMonitorNextCheckAt?: Date | null;
|
||||
patchMonitorNextCheckAt?: unknown;
|
||||
executionPolicy?: unknown;
|
||||
}) {
|
||||
if (input.patchMonitorNextCheckAt instanceof Date && !Number.isNaN(input.patchMonitorNextCheckAt.getTime())) return true;
|
||||
if (input.patchMonitorNextCheckAt === undefined && input.existingMonitorNextCheckAt) return true;
|
||||
const policy = normalizeIssueExecutionPolicy(input.executionPolicy ?? null);
|
||||
return Boolean(policy?.monitor?.nextCheckAt);
|
||||
}
|
||||
|
||||
function executionPrincipalsEqual(
|
||||
left: ParsedExecutionState["currentParticipant"] | null,
|
||||
right: ParsedExecutionState["currentParticipant"] | null,
|
||||
|
|
@ -2673,6 +2791,19 @@ export function issueRoutes(
|
|||
}
|
||||
|
||||
const hasFieldChanges = Object.keys(previous).length > 0;
|
||||
let workspaceChange = null;
|
||||
if (hasIssueWorkspaceAuditChange(previous)) {
|
||||
try {
|
||||
workspaceChange = await buildIssueWorkspaceChangeActivityDetails(db, issue.companyId, existing, issue);
|
||||
} catch (err) {
|
||||
logger.warn({ err, issueId: issue.id }, "failed to enrich issue workspace change activity details");
|
||||
const fallbackNames = emptyWorkspaceNameMaps();
|
||||
workspaceChange = {
|
||||
from: summarizeIssueWorkspaceForActivity(existing, fallbackNames),
|
||||
to: summarizeIssueWorkspaceForActivity(issue, fallbackNames),
|
||||
};
|
||||
}
|
||||
}
|
||||
const reopened =
|
||||
commentBody &&
|
||||
effectiveMoveToTodoRequested &&
|
||||
|
|
@ -2697,6 +2828,7 @@ export function issueRoutes(
|
|||
...(reopened ? { reopened: true, reopenedFrom: reopenFromStatus } : {}),
|
||||
...(interruptedRunId ? { interruptedRunId } : {}),
|
||||
...(cancelledStatusRunId ? { cancelledStatusRunId } : {}),
|
||||
...(workspaceChange ? { workspaceChange } : {}),
|
||||
_previous: hasFieldChanges ? previous : undefined,
|
||||
...summarizeIssueReferenceActivityDetails(
|
||||
updateReferenceDiff
|
||||
|
|
|
|||
|
|
@ -218,6 +218,7 @@ describe("successful run handoff decision", () => {
|
|||
title: "Missing issue disposition",
|
||||
detailsDefaultOpen: false,
|
||||
});
|
||||
expect(notice.metadata.sourceRunId).toBe("22222222-2222-4222-8222-222222222222");
|
||||
expect(notice.metadata.sections).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
title: "Required action",
|
||||
|
|
@ -267,6 +268,7 @@ describe("successful run handoff decision", () => {
|
|||
tone: "danger",
|
||||
detailsDefaultOpen: false,
|
||||
});
|
||||
expect(notice.metadata.sourceRunId).toBe("22222222-2222-4222-8222-222222222222");
|
||||
expect(notice.metadata.sections).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
title: "Recovery owner",
|
||||
|
|
|
|||
|
|
@ -146,6 +146,7 @@ export function buildSuccessfulRunHandoffRequiredNotice(input: {
|
|||
}),
|
||||
metadata: {
|
||||
version: 1,
|
||||
sourceRunId: input.run.id,
|
||||
sections: [
|
||||
{
|
||||
title: "Required action",
|
||||
|
|
@ -193,6 +194,7 @@ export function buildSuccessfulRunHandoffExhaustedNotice(input: {
|
|||
}),
|
||||
metadata: {
|
||||
version: 1,
|
||||
sourceRunId: input.sourceRun?.id ?? null,
|
||||
sections: [
|
||||
{
|
||||
title: "Recovery owner",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue