fix: reliably show plans in the Plan pane and restore sticky plan confirmation CTAs (#10930)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat-style tasks show an agent's plan in a dedicated "Plan" pane, and a plan confirmation lets the user accept or request changes to that plan > - When an agent asked for confirmation but never actually published the plan document (it only wrote the plan in a comment or a question), the Plan pane rendered empty, and the confirmation call-to-action that used to sit pinned at the bottom of the pane had disappeared > - A user asked to confirm a plan they cannot see, with no visible CTA, is stuck — the feature silently fails > - This pull request closes the gap on both sides: it prevents plan confirmations that don't point at a real, latest plan revision, it teaches agents to publish the plan document before confirming, and it restores the sticky confirmation action bar and an explanatory empty state so the pane never goes silently blank > - The benefit is that when a plan is expected, it reliably shows up in the right pane with reachable accept/revise actions ## Linked Issues or Issue Description <!-- No public GitHub issue exists; describing in-PR per the bug template. --> **Bug report** - **What happened:** A task in planning mode could present a plan confirmation while the Plan pane stayed empty (no plan document rendered), and the plan-card confirmation CTAs that were previously pinned to the bottom of the Plan pane no longer appeared. - **Expected behavior:** When a plan is expected, the plan document appears in the Plan pane; when a plan is genuinely missing, the pane explains why rather than showing nothing; and the accept/request-changes CTAs stay visible and reachable while the plan scrolls. - **Steps to reproduce:** Put a task in planning mode with the chat-style task view enabled, have an agent create a plan confirmation without first publishing the `plan` document, and open the Plan tab — the pane is blank and the confirmation actions are missing. - **Deployment mode:** Local dev and self-hosted; UI + server. Related PR (not a duplicate): #9609 "Pin pending confirmations by composer" pins confirmations in a different surface (the composer); this PR restores the Plans-pane action bar and the server/agent guarantees behind it. ## What Changed - **Server:** Reject a `request_confirmation` whose target is a plan document unless a plan document exists and the target points at its *latest* revision, so a confirmation can never reference a plan the pane cannot render (`readPlanTarget` is now exported for reuse). - **Agent instructions:** The CEO and default agent instruction bundles now spell out a plan-publish contract — publish the `plan` document, re-`GET` it and capture `latestRevisionId`, then create the confirmation targeting that revision; never present a plan only in a thread comment or via `ask_user_questions`. - **UI — sticky CTAs:** Restore the plan confirmation action bar pinned to the bottom of the Plans tab so accept/revise stay reachable while the plan scrolls. - **UI — diagnostics:** Keep the Plan tab visible whenever an issue is in planning mode (even before a plan document exists) and show an empty state explaining why the pane is empty instead of rendering nothing. - **UI — annotations:** Add a `panelPlacement="inline"` mode so the plan-document annotation panel renders in document flow instead of as a floating side panel when hosted in the narrow task properties pane. ## Verification - `pnpm check:token-gates` → 3/3 CLEAN - `pnpm typecheck` → clean (all packages) - UI: `pnpm --filter @paperclipai/ui exec vitest run src/components/issue-properties/IssuePlanConfirmationActionBar.test.tsx src/components/IssueProperties.test.tsx src/components/IssueDocumentAnnotations.test.tsx` → 69 passed - Server: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/issue-thread-interaction-routes.test.ts src/__tests__/agent-skills-routes.test.ts` → 60 passed - Manual: with a planning-mode task, the Plan tab stays visible, shows the plan document (or a diagnostic empty state), and the confirmation CTAs stay pinned at the bottom. Visual note: snapshot baselines are intentionally not updated — per `doc/design/DECISION-SHEET.md` "Per-change snapshot verification demoted to dormant (Jul 13 2026)". The `storybook-visual` label is intentionally not added. ## Risks Low-to-moderate. The server change adds a validation gate on plan-document confirmations: an interaction that targets a stale or nonexistent plan revision is now rejected with a 422 instead of being created. This is the intended guarantee, but any caller that relied on creating such confirmations will now need to publish the plan document first (which the updated agent instructions cover). UI changes are additive to the Plans tab and gated by the existing chat-style-task experimental flag. ## Model Used Claude Opus 4.8 (Anthropic), model id `claude-opus-4-8`, extended thinking enabled, with tool use (file editing, shell, test 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 - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
scotttong
GitHub
parent
dc71fef6bf
commit
f950952de7
|
|
@ -1035,6 +1035,13 @@ describe.sequential("agent skill routes", () => {
|
|||
}),
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(mockAgentInstructionsService.materializeManagedBundle).toHaveBeenCalledWith(
|
||||
expect.any(Object),
|
||||
expect.objectContaining({
|
||||
"AGENTS.md": expect.stringMatching(/PUT \/issues\/\{id\}\/documents\/plan[\s\S]*Re-`GET \/documents\/plan`, assert it returns `200`[\s\S]*latestRevisionId[\s\S]*target=\{ type: 'issue_document', key: 'plan', revisionId: latestRevisionId \}[\s\S]*Never present a plan only in a thread comment or through `ask_user_questions`/),
|
||||
}),
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(mockAgentInstructionsService.materializeManagedBundle).toHaveBeenCalledWith(
|
||||
expect.any(Object),
|
||||
expect.objectContaining({
|
||||
|
|
|
|||
|
|
@ -1043,6 +1043,47 @@ describe.sequential("issue thread interaction routes", () => {
|
|||
expect(mockInteractionService.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("forwards plan-document confirmations to the interaction service for revision validation", async () => {
|
||||
const app = await createApp();
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions")
|
||||
.send({
|
||||
kind: "request_confirmation",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Approve the plan?",
|
||||
target: {
|
||||
type: "issue_document",
|
||||
issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||
documentId: "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb",
|
||||
key: "plan",
|
||||
revisionId: "cccccccc-cccc-4ccc-8ccc-cccccccccccc",
|
||||
revisionNumber: 1,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// The route delegates plan-target validation to the service, which rejects a
|
||||
// stale/missing revision atomically inside its insert transaction
|
||||
// (assertRequestConfirmationTargetIsCurrent). The route must pass the target
|
||||
// through unchanged rather than pre-checking it non-atomically.
|
||||
expect(res.status).toBe(201);
|
||||
expect(mockInteractionService.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" }),
|
||||
expect.objectContaining({
|
||||
kind: "request_confirmation",
|
||||
payload: expect.objectContaining({
|
||||
target: expect.objectContaining({
|
||||
key: "plan",
|
||||
revisionId: "cccccccc-cccc-4ccc-8ccc-cccccccccccc",
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("accepts request checkbox confirmations with selected option ids and wakes the assignee", async () => {
|
||||
mockInteractionService.acceptInteraction.mockResolvedValueOnce({
|
||||
interaction: {
|
||||
|
|
|
|||
|
|
@ -2524,6 +2524,130 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("rejects creating a plan confirmation against a stale document revision and accepts the current one", async () => {
|
||||
const companyId = randomUUID();
|
||||
const goalId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
const documentId = randomUUID();
|
||||
const revisionId = randomUUID();
|
||||
const nextRevisionId = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: false });
|
||||
await db.insert(goals).values({
|
||||
id: goalId,
|
||||
companyId,
|
||||
title: "Stale plan confirmation",
|
||||
level: "task",
|
||||
status: "active",
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
goalId,
|
||||
title: "Parent issue",
|
||||
status: "in_progress",
|
||||
priority: "medium",
|
||||
});
|
||||
// Document is already at revision 2 — revision 1 is stale.
|
||||
await db.insert(documents).values({
|
||||
id: documentId,
|
||||
companyId,
|
||||
title: "Plan",
|
||||
format: "markdown",
|
||||
latestBody: "v2",
|
||||
latestRevisionId: nextRevisionId,
|
||||
latestRevisionNumber: 2,
|
||||
});
|
||||
await db.insert(issueDocuments).values({
|
||||
companyId,
|
||||
issueId,
|
||||
documentId,
|
||||
key: "plan",
|
||||
});
|
||||
await db.insert(documentRevisions).values([
|
||||
{
|
||||
id: revisionId,
|
||||
companyId,
|
||||
documentId,
|
||||
revisionNumber: 1,
|
||||
title: "Plan",
|
||||
format: "markdown",
|
||||
body: "v1",
|
||||
},
|
||||
{
|
||||
id: nextRevisionId,
|
||||
companyId,
|
||||
documentId,
|
||||
revisionNumber: 2,
|
||||
title: "Plan",
|
||||
format: "markdown",
|
||||
body: "v2",
|
||||
},
|
||||
]);
|
||||
|
||||
const staleTarget = {
|
||||
type: "issue_document" as const,
|
||||
issueId,
|
||||
documentId,
|
||||
key: "plan",
|
||||
revisionId,
|
||||
revisionNumber: 1,
|
||||
};
|
||||
|
||||
// The revision check runs inside the create transaction (locking the
|
||||
// document row), so a target pointing at an older revision is rejected
|
||||
// atomically with the would-be insert rather than by a racy pre-check.
|
||||
await expect(interactionsSvc.create({
|
||||
id: issueId,
|
||||
companyId,
|
||||
}, {
|
||||
kind: "request_confirmation",
|
||||
continuationPolicy: "wake_assignee",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Apply the plan document?",
|
||||
target: staleTarget,
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
})).rejects.toMatchObject({
|
||||
status: 422,
|
||||
message: expect.stringContaining("current issue document revision"),
|
||||
});
|
||||
|
||||
const noRows = await db
|
||||
.select({ id: issueThreadInteractions.id })
|
||||
.from(issueThreadInteractions)
|
||||
.where(eq(issueThreadInteractions.issueId, issueId));
|
||||
expect(noRows).toHaveLength(0);
|
||||
|
||||
const created = await interactionsSvc.create({
|
||||
id: issueId,
|
||||
companyId,
|
||||
}, {
|
||||
kind: "request_confirmation",
|
||||
continuationPolicy: "wake_assignee",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Apply the plan document?",
|
||||
target: {
|
||||
...staleTarget,
|
||||
revisionId: nextRevisionId,
|
||||
revisionNumber: 2,
|
||||
},
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
});
|
||||
expect(created).toMatchObject({ status: "pending", kind: "request_confirmation" });
|
||||
});
|
||||
|
||||
it("preserves resolved request_item_verdicts items when the watched issue document revision changes", async () => {
|
||||
const companyId = randomUUID();
|
||||
const goalId = randomUUID();
|
||||
|
|
|
|||
|
|
@ -34,7 +34,12 @@ You MUST delegate work rather than doing it yourself. When a task is assigned to
|
|||
- If the board asks you to do something and you're unsure who should own it, default to the CTO for technical work.
|
||||
- Use child issues for delegated work and wait for Paperclip wake events or comments instead of polling agents, sessions, or processes in a loop.
|
||||
- Create child issues directly when ownership and scope are clear. Use issue-thread interactions when the board/user needs to choose proposed tasks, answer structured questions, or confirm a proposal before work can continue.
|
||||
- Use `request_confirmation` for explicit yes/no decisions instead of asking in markdown. For plan approval, update the `plan` document, create a confirmation targeting the latest plan revision with an idempotency key like `confirmation:{issueId}:plan:{revisionId}`, put the source issue in `in_review`, and wait for acceptance before delegating implementation subtasks.
|
||||
- Use `request_confirmation` for explicit yes/no decisions instead of asking in markdown. Before presenting a plan for review, you MUST complete this publish contract:
|
||||
1. `PUT /issues/{id}/documents/plan` with `{ format: 'markdown', body, changeSummary }`.
|
||||
2. Re-`GET /documents/plan`, assert it returns `200`, and capture its `latestRevisionId`.
|
||||
3. Only then create `request_confirmation` with `target={ type: 'issue_document', key: 'plan', revisionId: latestRevisionId }` and `idempotencyKey=confirmation:{issueId}:plan:{revisionId}`.
|
||||
4. Put the source issue in `in_review` and wait for acceptance before delegating implementation subtasks.
|
||||
Never present a plan only in a thread comment or through `ask_user_questions`; comments are supporting context and questions are for gathering input, not plan review.
|
||||
- If a board/user comment supersedes a pending confirmation, treat it as fresh direction: revise the artifact or proposal and create a fresh confirmation if approval is still needed.
|
||||
- Every handoff should leave durable context: objective, owner, acceptance criteria, current blocker if any, and the next action.
|
||||
- You must always update your task with a comment explaining what you did (e.g., who you delegated to and why).
|
||||
|
|
|
|||
|
|
@ -11,7 +11,12 @@ You are an agent at Paperclip company.
|
|||
- Final disposition checklist: mark `done` when complete and verified; use `in_review` only with a real reviewer, approval, interaction, or monitor path; use `blocked` only with first-class blockers or a named unblock owner/action; create delegated follow-up issues with blockers when another agent owns the next step; keep `in_progress` only when a live continuation path exists.
|
||||
- Use child issues for parallel or long delegated work instead of polling agents, sessions, or processes.
|
||||
- Create child issues directly when you know what needs to be done. If the board/user needs to choose suggested tasks, answer structured questions, or confirm a proposal first, create an issue-thread interaction on the current issue with `POST /api/issues/{issueId}/interactions` using `kind: "suggest_tasks"`, `kind: "ask_user_questions"`, or `kind: "request_confirmation"`.
|
||||
- Use `request_confirmation` instead of asking for yes/no decisions in markdown. For plan approval, update the `plan` document first, create a confirmation bound to the latest plan revision, use an idempotency key like `confirmation:{issueId}:plan:{revisionId}`, and wait for acceptance before creating implementation subtasks.
|
||||
- Use `request_confirmation` instead of asking for yes/no decisions in markdown. Before presenting a plan for review, you MUST complete this publish contract:
|
||||
1. `PUT /issues/{id}/documents/plan` with `{ format: 'markdown', body, changeSummary }`.
|
||||
2. Re-`GET /documents/plan`, assert it returns `200`, and capture its `latestRevisionId`.
|
||||
3. Only then create `request_confirmation` with `target={ type: 'issue_document', key: 'plan', revisionId: latestRevisionId }` and `idempotencyKey=confirmation:{issueId}:plan:{revisionId}`.
|
||||
4. Wait for acceptance before creating implementation subtasks.
|
||||
Never present a plan only in a thread comment or through `ask_user_questions`; comments are supporting context and questions are for gathering input, not plan review.
|
||||
- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `true`, so a later board/user comment invalidates the pending request. Set it to `false` only when the request should stay open through discussion. If you wake up from a superseding comment, revise the artifact, question set, or proposal and create a fresh interaction if input is still needed.
|
||||
- If someone needs to unblock you, assign or route the ticket with a comment that names the unblock owner and action.
|
||||
- Respect budget, pause/cancel, approval gates, and company boundaries.
|
||||
|
|
|
|||
|
|
@ -10042,6 +10042,12 @@ export function issueRoutes(
|
|||
throw unprocessable("payload.toolAction is server-owned metadata and cannot be supplied when creating an interaction");
|
||||
}
|
||||
|
||||
// Plan-document confirmation targets are validated authoritatively inside
|
||||
// issueThreadInteractionService.create, which re-reads the plan document's
|
||||
// latest revision and rejects a stale/missing target under the same insert
|
||||
// transaction (see assertRequestConfirmationTargetIsCurrent). We deliberately
|
||||
// do not pre-check the revision here: a separate route-level read would be
|
||||
// non-atomic with the insert and only duplicate the service gate.
|
||||
const interaction = await issueThreadInteractionService(db).create(issue, {
|
||||
...req.body,
|
||||
sourceRunId: req.actor.type === "agent" ? agentSourceRunId : req.body.sourceRunId ?? null,
|
||||
|
|
|
|||
|
|
@ -1009,10 +1009,14 @@ async function getIssueDocumentTargetSnapshot(db: Db | any, args: {
|
|||
companyId: string;
|
||||
issueId: string;
|
||||
target: RequestConfirmationTarget;
|
||||
// When true, take a FOR UPDATE row lock on the joined document so a concurrent
|
||||
// revision publish (which updates documents.latestRevisionId) must serialize
|
||||
// behind the caller's transaction. Only meaningful inside a transaction.
|
||||
lockForUpdate?: boolean;
|
||||
}) {
|
||||
if (args.target.type !== "issue_document") return null;
|
||||
const targetIssueId = args.target.issueId ?? args.issueId;
|
||||
const row = await db
|
||||
const query = db
|
||||
.select({
|
||||
issueId: issueDocuments.issueId,
|
||||
documentId: issueDocuments.documentId,
|
||||
|
|
@ -1026,7 +1030,8 @@ async function getIssueDocumentTargetSnapshot(db: Db | any, args: {
|
|||
eq(issueDocuments.companyId, args.companyId),
|
||||
eq(issueDocuments.issueId, targetIssueId),
|
||||
eq(issueDocuments.key, args.target.key),
|
||||
))
|
||||
));
|
||||
const row = await (args.lockForUpdate ? query.for("update", { of: documents }) : query)
|
||||
.then((rows: Array<{
|
||||
issueId: string;
|
||||
documentId: string;
|
||||
|
|
@ -1080,6 +1085,10 @@ async function assertRequestConfirmationTargetIsCurrent(db: Db | any, args: {
|
|||
companyId: string;
|
||||
issueId: string;
|
||||
target?: RequestConfirmationTarget | null;
|
||||
// Forwarded to getIssueDocumentTargetSnapshot; pass true when validating
|
||||
// inside the create transaction so the revision read locks the document row
|
||||
// and stays atomic with the interaction insert.
|
||||
lockForUpdate?: boolean;
|
||||
}) {
|
||||
if (!args.target) return;
|
||||
if (args.target.type !== "issue_document") return;
|
||||
|
|
@ -1087,6 +1096,7 @@ async function assertRequestConfirmationTargetIsCurrent(db: Db | any, args: {
|
|||
companyId: args.companyId,
|
||||
issueId: args.issueId,
|
||||
target: args.target,
|
||||
lockForUpdate: args.lockForUpdate,
|
||||
});
|
||||
if (!snapshot || snapshot.latestRevisionId !== args.target.revisionId) {
|
||||
throw unprocessable("request_confirmation target must reference the current issue document revision");
|
||||
|
|
@ -1812,17 +1822,10 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
|
|||
}
|
||||
}
|
||||
|
||||
if (
|
||||
const requiresCurrentTarget =
|
||||
data.kind === "request_confirmation"
|
||||
|| data.kind === "request_checkbox_confirmation"
|
||||
|| data.kind === "request_item_verdicts"
|
||||
) {
|
||||
await assertRequestConfirmationTargetIsCurrent(db, {
|
||||
companyId: issue.companyId,
|
||||
issueId: issue.id,
|
||||
target: data.payload.target ?? null,
|
||||
});
|
||||
}
|
||||
|| data.kind === "request_item_verdicts";
|
||||
|
||||
let created: IssueThreadInteractionRow;
|
||||
let superseded: IssueThreadInteractionRow[] = [];
|
||||
|
|
@ -1841,6 +1844,19 @@ export function issueThreadInteractionService(db: Db, opts: IssueThreadInteracti
|
|||
if (!issueRow || isTerminalIssueStatus(issueRow.status)) {
|
||||
throw conflict("Cannot create an interaction on a closed issue");
|
||||
}
|
||||
// Validate the plan/document confirmation target inside the same
|
||||
// transaction (locking the document row) so the latest-revision check
|
||||
// is atomic with the insert below. A concurrent revision publish can no
|
||||
// longer slip between the check and the insert to leave a confirmation
|
||||
// pointing at a stale revision.
|
||||
if (requiresCurrentTarget) {
|
||||
await assertRequestConfirmationTargetIsCurrent(tx, {
|
||||
companyId: issue.companyId,
|
||||
issueId: issue.id,
|
||||
target: data.payload.target ?? null,
|
||||
lockForUpdate: true,
|
||||
});
|
||||
}
|
||||
const [row] = await tx
|
||||
.insert(issueThreadInteractions)
|
||||
.values({
|
||||
|
|
|
|||
|
|
@ -58,6 +58,8 @@ export interface AnnotationPanelProps {
|
|||
isMobile?: boolean;
|
||||
/** Desktop panel width calculated by the document frame. */
|
||||
desktopWidth?: number;
|
||||
/** Render as a full-width card in a constrained host instead of a floating side panel. */
|
||||
inline?: boolean;
|
||||
className?: string;
|
||||
/** Resolve `<authorAgentId>` to a display name. */
|
||||
agentMap?: ReadonlyMap<string, Pick<Agent, "id" | "name"> & Partial<Pick<Agent, "icon">>>;
|
||||
|
|
@ -92,7 +94,8 @@ export function DocumentAnnotationPanel(props: AnnotationPanelProps) {
|
|||
aria-label={`Annotations for ${props.documentKey.toUpperCase()}, revision ${props.documentRevisionNumber}`}
|
||||
data-testid="document-annotation-panel"
|
||||
className={cn(
|
||||
"isolate flex h-full max-h-(--sz-80vh) w-(--sz-360px) shrink-0 flex-col overflow-hidden rounded-none border border-border bg-popover text-popover-foreground shadow-xl",
|
||||
"isolate flex h-full max-h-(--sz-80vh) shrink-0 flex-col overflow-hidden rounded-none border border-border bg-popover text-popover-foreground shadow-xl",
|
||||
props.inline ? "w-full" : "w-(--sz-360px)",
|
||||
props.className,
|
||||
)}
|
||||
style={props.desktopWidth ? { width: props.desktopWidth, maxWidth: props.desktopWidth } : undefined}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { useState } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { createRoot as createReactRoot } from "react-dom/client";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import type {
|
||||
DocumentAnnotationThreadWithComments,
|
||||
|
|
@ -138,10 +138,47 @@ async function act(callback: () => void | Promise<void>) {
|
|||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
}
|
||||
|
||||
// Track every root so afterEach can unmount it. Tests mount into a throwaway
|
||||
// container but never unmount, so without this the panel's window scroll/resize
|
||||
// listeners and react-query subscriptions from earlier tests stay live and can
|
||||
// recompute positioning against a detached host — an order-dependent flake that
|
||||
// only surfaced under CI's fuller suite run.
|
||||
const activeRoots: Array<ReturnType<typeof createReactRoot>> = [];
|
||||
function createRoot(node: Parameters<typeof createReactRoot>[0]) {
|
||||
const root = createReactRoot(node);
|
||||
activeRoots.push(root);
|
||||
return root;
|
||||
}
|
||||
|
||||
async function unmountActiveRoots() {
|
||||
if (activeRoots.length === 0) return;
|
||||
const roots = activeRoots.splice(0);
|
||||
await act(() => {
|
||||
for (const root of roots) root.unmount();
|
||||
});
|
||||
}
|
||||
|
||||
async function flush() {
|
||||
await act(() => {});
|
||||
}
|
||||
|
||||
// Poll an assertion across React flushes until it passes or times out. The panel
|
||||
// positions itself and loads threads through effects + react-query, so a fixed
|
||||
// number of flushes can race on a loaded machine (CI). Waiting on the assertion
|
||||
// itself is deterministic regardless of how many turns the settle takes.
|
||||
async function waitFor(assertion: () => void, { timeout = 2000 }: { timeout?: number } = {}) {
|
||||
const start = Date.now();
|
||||
for (;;) {
|
||||
try {
|
||||
assertion();
|
||||
return;
|
||||
} catch (error) {
|
||||
if (Date.now() - start > timeout) throw error;
|
||||
await flush();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function setTextareaValue(textarea: HTMLTextAreaElement, value: string) {
|
||||
const setter = Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")?.set;
|
||||
setter?.call(textarea, value);
|
||||
|
|
@ -249,6 +286,7 @@ function Harness({
|
|||
historicalPreview = false,
|
||||
locationHash = "",
|
||||
initialPanelOpen = false,
|
||||
panelPlacement,
|
||||
}: {
|
||||
doc: IssueDocument;
|
||||
draftDirty?: boolean;
|
||||
|
|
@ -256,6 +294,7 @@ function Harness({
|
|||
historicalPreview?: boolean;
|
||||
locationHash?: string;
|
||||
initialPanelOpen?: boolean;
|
||||
panelPlacement?: "floating" | "inline";
|
||||
}) {
|
||||
const [open, setOpen] = useState(initialPanelOpen);
|
||||
return (
|
||||
|
|
@ -276,6 +315,7 @@ function Harness({
|
|||
locationHash={locationHash}
|
||||
panelOpen={open}
|
||||
onPanelOpenChange={setOpen}
|
||||
panelPlacement={panelPlacement}
|
||||
>
|
||||
<p>Body content</p>
|
||||
</IssueDocumentAnnotations>
|
||||
|
|
@ -292,7 +332,8 @@ describe("IssueDocumentAnnotations", () => {
|
|||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
afterEach(async () => {
|
||||
await unmountActiveRoots();
|
||||
container.remove();
|
||||
});
|
||||
|
||||
|
|
@ -329,6 +370,30 @@ describe("IssueDocumentAnnotations", () => {
|
|||
expect(anchor?.className).toContain("z-(--z-60)");
|
||||
});
|
||||
|
||||
it("stacks an inline panel below the document instead of floating over its host", async () => {
|
||||
mockAnnotationsApi.list.mockResolvedValue([makeThread()]);
|
||||
const root = createRoot(container);
|
||||
const queryClient = makeQueryClient();
|
||||
const doc = makeDoc();
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<Harness doc={doc} initialPanelOpen panelPlacement="inline" />
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
const inlinePanel = container.querySelector('[data-testid="document-annotation-panel-inline"]');
|
||||
const floatingAnchor = container.querySelector('[data-testid="document-annotation-panel-anchor"]');
|
||||
const panel = container.querySelector('[data-testid="document-annotation-panel"]');
|
||||
expect(inlinePanel).not.toBeNull();
|
||||
expect(floatingAnchor).toBeNull();
|
||||
expect(panel?.className).toContain("w-full");
|
||||
});
|
||||
|
||||
it("keeps the desktop annotation panel inside the issue content area when properties are visible", async () => {
|
||||
mockAnnotationsApi.list.mockResolvedValue([makeThread()]);
|
||||
const originalGetBoundingClientRect = HTMLElement.prototype.getBoundingClientRect;
|
||||
|
|
@ -370,17 +435,16 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
const anchor = container.querySelector('[data-testid="document-annotation-panel-anchor"]') as HTMLElement | null;
|
||||
const panel = container.querySelector('[data-testid="document-annotation-panel"]') as HTMLElement | null;
|
||||
expect(anchor).not.toBeNull();
|
||||
expect(panel).not.toBeNull();
|
||||
expect(anchor!.style.left).toBe("524px");
|
||||
expect(anchor!.style.width).toBe("360px");
|
||||
expect(panel!.style.width).toBe("360px");
|
||||
expect(parseFloat(anchor!.style.left) + parseFloat(anchor!.style.width)).toBeLessThanOrEqual(884);
|
||||
await waitFor(() => {
|
||||
const anchor = container.querySelector('[data-testid="document-annotation-panel-anchor"]') as HTMLElement | null;
|
||||
const panel = container.querySelector('[data-testid="document-annotation-panel"]') as HTMLElement | null;
|
||||
expect(anchor).not.toBeNull();
|
||||
expect(panel).not.toBeNull();
|
||||
expect(anchor!.style.left).toBe("524px");
|
||||
expect(anchor!.style.width).toBe("360px");
|
||||
expect(panel!.style.width).toBe("360px");
|
||||
expect(parseFloat(anchor!.style.left) + parseFloat(anchor!.style.width)).toBeLessThanOrEqual(884);
|
||||
});
|
||||
} finally {
|
||||
rectSpy.mockRestore();
|
||||
}
|
||||
|
|
@ -427,15 +491,14 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
const anchor = container.querySelector('[data-testid="document-annotation-panel-anchor"]') as HTMLElement | null;
|
||||
expect(anchor).not.toBeNull();
|
||||
// The document body ends at 640; the panel should clear it with a margin
|
||||
// rather than sitting flush against the document's right edge.
|
||||
expect(parseFloat(anchor!.style.left)).toBeGreaterThan(640);
|
||||
expect(anchor!.style.left).toBe("664px");
|
||||
await waitFor(() => {
|
||||
const anchor = container.querySelector('[data-testid="document-annotation-panel-anchor"]') as HTMLElement | null;
|
||||
expect(anchor).not.toBeNull();
|
||||
// The document body ends at 640; the panel should clear it with a margin
|
||||
// rather than sitting flush against the document's right edge.
|
||||
expect(parseFloat(anchor!.style.left)).toBeGreaterThan(640);
|
||||
expect(anchor!.style.left).toBe("664px");
|
||||
});
|
||||
} finally {
|
||||
rectSpy.mockRestore();
|
||||
}
|
||||
|
|
@ -454,13 +517,12 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
const panel = container.querySelector('[data-testid="document-annotation-panel"]');
|
||||
expect(panel).not.toBeNull();
|
||||
const focusedThread = container.querySelector('[data-thread-id="thread-99"][data-focused]');
|
||||
expect(focusedThread).not.toBeNull();
|
||||
await waitFor(() => {
|
||||
const panel = container.querySelector('[data-testid="document-annotation-panel"]');
|
||||
expect(panel).not.toBeNull();
|
||||
const focusedThread = container.querySelector('[data-thread-id="thread-99"][data-focused]');
|
||||
expect(focusedThread).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("shows a disabled reason in the panel when the draft is dirty", async () => {
|
||||
|
|
@ -476,14 +538,13 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
const reason = container.querySelector(
|
||||
'[data-testid="document-annotation-disabled-reason"]',
|
||||
);
|
||||
expect(reason).not.toBeNull();
|
||||
expect(reason!.textContent).toMatch(/draft/i);
|
||||
await waitFor(() => {
|
||||
const reason = container.querySelector(
|
||||
'[data-testid="document-annotation-disabled-reason"]',
|
||||
);
|
||||
expect(reason).not.toBeNull();
|
||||
expect(reason!.textContent).toMatch(/draft/i);
|
||||
});
|
||||
});
|
||||
|
||||
it("shows open and resolved threads together in a single list (no filter tabs)", async () => {
|
||||
|
|
@ -503,12 +564,11 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
// Open + resolved both render without any filter interaction.
|
||||
expect(container.querySelector('[data-thread-id="open-1"]')).not.toBeNull();
|
||||
expect(container.querySelector('[data-thread-id="resolved-1"]')).not.toBeNull();
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector('[data-thread-id="open-1"]')).not.toBeNull();
|
||||
expect(container.querySelector('[data-thread-id="resolved-1"]')).not.toBeNull();
|
||||
});
|
||||
// Orphaned threads can't be anchored in the doc, so they stay hidden.
|
||||
expect(container.querySelector('[data-thread-id="orphan-1"]')).toBeNull();
|
||||
|
||||
|
|
@ -537,12 +597,11 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
const order = Array.from(container.querySelectorAll("[data-thread-id]"))
|
||||
.map((el) => el.getAttribute("data-thread-id"));
|
||||
expect(order).toEqual(["thread-early", "thread-mid", "thread-late"]);
|
||||
await waitFor(() => {
|
||||
const order = Array.from(container.querySelectorAll("[data-thread-id]"))
|
||||
.map((el) => el.getAttribute("data-thread-id"));
|
||||
expect(order).toEqual(["thread-early", "thread-mid", "thread-late"]);
|
||||
});
|
||||
});
|
||||
|
||||
it("renders author name + role from agent and user maps", async () => {
|
||||
|
|
@ -615,12 +674,11 @@ describe("IssueDocumentAnnotations", () => {
|
|||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
// Click the open thread to expand it.
|
||||
await waitFor(() => {
|
||||
expect(container.querySelector('[data-thread-id="open-1"]')).not.toBeNull();
|
||||
});
|
||||
const threadCard = container.querySelector('[data-thread-id="open-1"]') as HTMLElement | null;
|
||||
expect(threadCard).not.toBeNull();
|
||||
await act(async () => threadCard!.click());
|
||||
await flush();
|
||||
|
||||
|
|
|
|||
|
|
@ -42,6 +42,8 @@ export interface IssueDocumentAnnotationsProps {
|
|||
/** Controlled panel state. Caller owns this so the count chip can live in the doc header. */
|
||||
panelOpen: boolean;
|
||||
onPanelOpenChange: (open: boolean) => void;
|
||||
/** Keep the panel in document flow for narrow hosts such as the task properties pane. */
|
||||
panelPlacement?: "floating" | "inline";
|
||||
agentMap?: ReadonlyMap<string, Pick<Agent, "id" | "name"> & Partial<Pick<Agent, "icon">>>;
|
||||
userProfileMap?: ReadonlyMap<string, CompanyUserProfile>;
|
||||
/** Seed which thread is focused on mount. Used by Storybook/screenshot harness. */
|
||||
|
|
@ -66,6 +68,7 @@ export function IssueDocumentAnnotations({
|
|||
locationHash,
|
||||
panelOpen,
|
||||
onPanelOpenChange,
|
||||
panelPlacement = "floating",
|
||||
agentMap,
|
||||
userProfileMap,
|
||||
defaultFocusedThreadId,
|
||||
|
|
@ -104,7 +107,7 @@ export function IssueDocumentAnnotations({
|
|||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
if (!panelOpen || isMobile || typeof window === "undefined") {
|
||||
if (!panelOpen || panelPlacement === "inline" || isMobile || typeof window === "undefined") {
|
||||
setDesktopPanelFrame(null);
|
||||
return;
|
||||
}
|
||||
|
|
@ -169,7 +172,7 @@ export function IssueDocumentAnnotations({
|
|||
window.removeEventListener("scroll", updatePanelFrame, true);
|
||||
resizeObserver?.disconnect();
|
||||
};
|
||||
}, [doc.key, isMobile, panelOpen]);
|
||||
}, [doc.key, isMobile, panelOpen, panelPlacement]);
|
||||
|
||||
const annotationsQuery = useQuery({
|
||||
queryKey: target?.kind === "routine"
|
||||
|
|
@ -284,7 +287,7 @@ export function IssueDocumentAnnotations({
|
|||
);
|
||||
|
||||
const fallbackDesktopPanelFrame = useMemo(() => {
|
||||
if (!panelOpen || isMobile || desktopPanelFrame || typeof window === "undefined") return null;
|
||||
if (!panelOpen || panelPlacement === "inline" || isMobile || desktopPanelFrame || typeof window === "undefined") return null;
|
||||
const width = Math.min(
|
||||
DESKTOP_ANNOTATION_PANEL_WIDTH,
|
||||
Math.max(
|
||||
|
|
@ -304,7 +307,7 @@ export function IssueDocumentAnnotations({
|
|||
),
|
||||
width,
|
||||
};
|
||||
}, [desktopPanelFrame, isMobile, panelOpen]);
|
||||
}, [desktopPanelFrame, isMobile, panelOpen, panelPlacement]);
|
||||
const renderedDesktopPanelFrame = desktopPanelFrame ?? fallbackDesktopPanelFrame;
|
||||
|
||||
const annotationPanel = panelOpen ? (
|
||||
|
|
@ -338,6 +341,7 @@ export function IssueDocumentAnnotations({
|
|||
newCommentDisabled={newCommentDisabled}
|
||||
newCommentDisabledReason={newCommentDisabledReason}
|
||||
isMobile={isMobile}
|
||||
inline={panelPlacement === "inline"}
|
||||
desktopWidth={renderedDesktopPanelFrame?.width}
|
||||
agentMap={agentMap}
|
||||
userProfileMap={userProfileMap}
|
||||
|
|
@ -374,6 +378,11 @@ export function IssueDocumentAnnotations({
|
|||
/>
|
||||
) : null}
|
||||
</section>
|
||||
{panelOpen && panelPlacement === "inline" && !isMobile ? (
|
||||
<div className="mt-3" data-testid="document-annotation-panel-inline">
|
||||
{annotationPanel}
|
||||
</div>
|
||||
) : null}
|
||||
{panelOpen && !isMobile && renderedDesktopPanelFrame ? (
|
||||
<div
|
||||
data-testid="document-annotation-panel-anchor"
|
||||
|
|
|
|||
|
|
@ -33,6 +33,10 @@ const mockExecutionWorkspacesApi = vi.hoisted(() => ({
|
|||
|
||||
const mockIssuesApi = vi.hoisted(() => ({
|
||||
list: vi.fn(),
|
||||
getDocument: vi.fn(),
|
||||
listAcceptedPlanDecompositions: vi.fn(),
|
||||
listAttachments: vi.fn(),
|
||||
listInteractions: vi.fn(),
|
||||
listLabels: vi.fn(),
|
||||
createLabel: vi.fn(),
|
||||
upsertWatchdog: vi.fn(),
|
||||
|
|
@ -136,6 +140,7 @@ vi.mock("./AgentIconPicker", () => ({
|
|||
vi.mock("@/lib/router", () => ({
|
||||
Link: ({ children, to, ...props }: { children: ReactNode; to: string } & ComponentProps<"a">) => <a href={to} {...props}>{children}</a>,
|
||||
useCaseHref: () => (caseId: string) => `/cases/${caseId}`,
|
||||
useLocation: () => ({ hash: "", pathname: "/", search: "", state: null, key: "test" }),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/ui/separator", () => ({
|
||||
|
|
@ -435,6 +440,10 @@ describe("IssueProperties", () => {
|
|||
mockProjectsApi.list.mockResolvedValue([]);
|
||||
mockExecutionWorkspacesApi.controlRuntimeCommands.mockReset();
|
||||
mockIssuesApi.list.mockResolvedValue([]);
|
||||
mockIssuesApi.getDocument.mockResolvedValue(null);
|
||||
mockIssuesApi.listAcceptedPlanDecompositions.mockResolvedValue([]);
|
||||
mockIssuesApi.listAttachments.mockResolvedValue([]);
|
||||
mockIssuesApi.listInteractions.mockResolvedValue([]);
|
||||
mockIssuesApi.listLabels.mockResolvedValue([]);
|
||||
mockIssuesApi.createLabel.mockResolvedValue(createLabel({
|
||||
id: "label-new",
|
||||
|
|
@ -468,6 +477,43 @@ describe("IssueProperties", () => {
|
|||
document.body.innerHTML = "";
|
||||
});
|
||||
|
||||
it("keeps the Plan tab visible for a planning-mode issue without a plan document", async () => {
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({
|
||||
enableTaskWatchdogs: false,
|
||||
enableTaskChatRedesign: true,
|
||||
});
|
||||
mockIssuesApi.listInteractions.mockResolvedValue([
|
||||
{
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
payload: { target: { type: "issue_document", key: "plan" } },
|
||||
},
|
||||
]);
|
||||
const root = renderProperties(container, {
|
||||
issue: createIssue({ workMode: "planning" }),
|
||||
childIssues: [],
|
||||
onUpdate: vi.fn(),
|
||||
inline: true,
|
||||
});
|
||||
|
||||
await waitForAssertion(() => {
|
||||
expect(Array.from(container.querySelectorAll("button")).some((button) => button.textContent === "Plan")).toBe(true);
|
||||
});
|
||||
|
||||
const planTab = Array.from(container.querySelectorAll("button")).find((button) => button.textContent === "Plan");
|
||||
await act(async () => {
|
||||
// Radix Tabs triggers select on mousedown (button 0), not on click.
|
||||
planTab!.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, button: 0 }));
|
||||
});
|
||||
|
||||
await waitForAssertion(() => {
|
||||
expect(container.textContent).toContain("This task is in plan mode but no plan document has been written yet.");
|
||||
expect(container.textContent).toContain("A plan confirmation is pending, but the plan document it should confirm is missing.");
|
||||
});
|
||||
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
it("shows assignee and originating without responsible wording", async () => {
|
||||
mockAgentsApi.list.mockResolvedValue([{ id: "agent-1", name: "CodexCoder", status: "active", adapterType: "codex_local" }]);
|
||||
const root = renderProperties(container, {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,117 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { MemoryRouter } from "react-router-dom";
|
||||
import type { Issue, RequestConfirmationInteraction } from "@paperclipai/shared";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { queryKeys } from "@/lib/queryKeys";
|
||||
import { IssuePropertiesPlansTab } from "./IssuePropertiesPlansTab";
|
||||
import { IssuePlanConfirmationActionBar } from "./IssuePlanConfirmationActionBar";
|
||||
|
||||
const mockIssuesApi = vi.hoisted(() => ({
|
||||
listInteractions: vi.fn(),
|
||||
listAcceptedPlanDecompositions: vi.fn(),
|
||||
acceptInteraction: vi.fn(),
|
||||
rejectInteraction: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/api/issues", () => ({ issuesApi: mockIssuesApi }));
|
||||
vi.mock("@/hooks/useIssuePlanDocument", () => ({
|
||||
useIssuePlanDocument: () => ({ data: undefined, isLoading: false }),
|
||||
}));
|
||||
vi.mock("../PropertiesPanel", () => ({
|
||||
PROPERTIES_PANE_FOOTER_SLOT_ID: "properties-pane-footer-slot",
|
||||
}));
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
const issue = {
|
||||
id: "issue-1",
|
||||
identifier: "PAP-1",
|
||||
} as Issue;
|
||||
|
||||
const confirmation = {
|
||||
id: "confirmation-1",
|
||||
companyId: "company-1",
|
||||
issueId: issue.id,
|
||||
kind: "request_confirmation",
|
||||
status: "pending",
|
||||
continuationPolicy: "wake_assignee",
|
||||
resolverPolicy: "board_only",
|
||||
requestedResolverPolicy: "board_only",
|
||||
effectiveResolverPolicy: "board_only",
|
||||
createdAt: "2026-08-05T00:00:00.000Z",
|
||||
updatedAt: "2026-08-05T00:00:00.000Z",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Approve this plan?",
|
||||
acceptLabel: "Approve plan",
|
||||
rejectLabel: "Request changes",
|
||||
rejectRequiresReason: true,
|
||||
allowDeclineReason: true,
|
||||
target: { type: "issue_document", key: "plan", revisionId: "rev-1" },
|
||||
},
|
||||
} satisfies RequestConfirmationInteraction;
|
||||
|
||||
let root: ReturnType<typeof createRoot> | null = null;
|
||||
let container: HTMLDivElement | null = null;
|
||||
let client: QueryClient | null = null;
|
||||
|
||||
function render(element: React.ReactElement) {
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity } } });
|
||||
client.setQueryData(queryKeys.issues.interactions(issue.id), [confirmation]);
|
||||
client.setQueryData(queryKeys.issues.acceptedPlanDecompositions(issue.id), []);
|
||||
act(() => root?.render(
|
||||
<MemoryRouter>
|
||||
<QueryClientProvider client={client!}>{element}</QueryClientProvider>
|
||||
</MemoryRouter>,
|
||||
));
|
||||
return container;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
if (root) act(() => root?.unmount());
|
||||
root = null;
|
||||
container?.remove();
|
||||
container = null;
|
||||
client?.clear();
|
||||
client = null;
|
||||
vi.unstubAllGlobals();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("IssuePlanConfirmationActionBar", () => {
|
||||
it("renders a pending plan confirmation even before its plan document exists", () => {
|
||||
const rendered = render(<IssuePropertiesPlansTab issue={issue} />);
|
||||
|
||||
expect(rendered.querySelector('[data-testid="plan-pane-action-bar"]')).not.toBeNull();
|
||||
expect(rendered.textContent).toContain("Approve plan");
|
||||
expect(rendered.textContent).toContain("Request changes");
|
||||
});
|
||||
|
||||
it("moves into a footer slot that mounts on the next paint", () => {
|
||||
let resolveNextPaint: FrameRequestCallback | undefined;
|
||||
vi.stubGlobal("requestAnimationFrame", vi.fn((callback: FrameRequestCallback) => {
|
||||
resolveNextPaint = callback;
|
||||
return 1;
|
||||
}));
|
||||
vi.stubGlobal("cancelAnimationFrame", vi.fn());
|
||||
|
||||
const rendered = render(<IssuePlanConfirmationActionBar issue={issue} />);
|
||||
const footer = document.createElement("div");
|
||||
footer.id = "properties-pane-footer-slot";
|
||||
document.body.appendChild(footer);
|
||||
|
||||
act(() => resolveNextPaint?.(0));
|
||||
|
||||
expect(rendered.querySelector('[data-testid="plan-pane-action-bar"]')).toBeNull();
|
||||
expect(footer.querySelector('[data-testid="plan-pane-action-bar"]')).not.toBeNull();
|
||||
footer.remove();
|
||||
});
|
||||
});
|
||||
|
|
@ -51,8 +51,18 @@ export function IssuePlanConfirmationActionBar({ issue, inline }: IssuePlanConfi
|
|||
setFooterSlot(null);
|
||||
return;
|
||||
}
|
||||
setFooterSlot(document.getElementById(PROPERTIES_PANE_FOOTER_SLOT_ID));
|
||||
}, [inline]);
|
||||
|
||||
const resolveFooterSlot = () => {
|
||||
setFooterSlot(document.getElementById(PROPERTIES_PANE_FOOTER_SLOT_ID));
|
||||
};
|
||||
|
||||
// The properties pane and this action bar can mount in either order. Check
|
||||
// once synchronously, then again on the next paint so the footer slot is
|
||||
// found when it is mounted later in the same commit.
|
||||
resolveFooterSlot();
|
||||
const frame = requestAnimationFrame(resolveFooterSlot);
|
||||
return () => cancelAnimationFrame(frame);
|
||||
}, [confirmation?.id, inline]);
|
||||
|
||||
const [rejecting, setRejecting] = useState(false);
|
||||
const [rejectReason, setRejectReason] = useState("");
|
||||
|
|
@ -113,7 +123,9 @@ export function IssuePlanConfirmationActionBar({ issue, inline }: IssuePlanConfi
|
|||
onChange={(event) => setRejectReason(event.target.value)}
|
||||
placeholder={
|
||||
confirmation.payload.declineReasonPlaceholder
|
||||
?? "Optional: what would you like revised?"
|
||||
?? (confirmation.payload.acceptLabel === "Approve plan"
|
||||
? "Optional: what would you like revised?"
|
||||
: "Optional: tell the agent what you'd change.")
|
||||
}
|
||||
aria-invalid={rejectAttempted && reasonInvalid}
|
||||
className={cn(
|
||||
|
|
|
|||
|
|
@ -186,8 +186,9 @@ export function IssueProperties({
|
|||
}
|
||||
setPaneHeaderSlot(document.getElementById(PROPERTIES_PANE_HEADER_SLOT_ID));
|
||||
}, [taskChatRedesignEnabled, inline]);
|
||||
// Plan/Artifacts only earn a tab when they have content; with neither, the
|
||||
// header bar shows a plain "Properties" title instead of a one-tab strip.
|
||||
// Plan earns a tab as soon as an issue is in planning mode, even before the
|
||||
// plan document arrives. This keeps an expected plan surface visible and
|
||||
// lets its diagnostic empty state explain what is missing.
|
||||
// Same query keys as the tab bodies, so these share their cached fetches.
|
||||
const { data: paneTabPlanDocument } = useIssuePlanDocument(
|
||||
taskChatRedesignEnabled ? issue.id : null,
|
||||
|
|
@ -202,7 +203,10 @@ export function IssueProperties({
|
|||
queryFn: () => issuesApi.listAttachments(issue.id),
|
||||
enabled: taskChatRedesignEnabled,
|
||||
});
|
||||
const hasPlanTab = Boolean(paneTabPlanDocument) || (paneTabAcceptedPlans?.length ?? 0) > 0;
|
||||
const hasPlanTab =
|
||||
Boolean(paneTabPlanDocument)
|
||||
|| (paneTabAcceptedPlans?.length ?? 0) > 0
|
||||
|| issue.workMode === "planning";
|
||||
const hasArtifactsTab = (paneTabAttachments?.length ?? 0) > 0;
|
||||
const [paneTab, setPaneTab] = useState("properties");
|
||||
const [assigneeOpen, setAssigneeOpen] = useState(false);
|
||||
|
|
|
|||
|
|
@ -1,10 +1,13 @@
|
|||
import { useState } from "react";
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
import type { Issue } from "@paperclipai/shared";
|
||||
import type { Issue, IssueThreadInteraction } from "@paperclipai/shared";
|
||||
import { issuesApi } from "@/api/issues";
|
||||
import { queryKeys } from "@/lib/queryKeys";
|
||||
import { IssuePlanDecompositionsSection } from "@/components/IssuePlanDecompositionsSection";
|
||||
import { MarkdownBody } from "@/components/MarkdownBody";
|
||||
import { DocumentAnnotationsCountChip, IssueDocumentAnnotations } from "@/components/IssueDocumentAnnotations";
|
||||
import { useIssuePlanDocument } from "@/hooks/useIssuePlanDocument";
|
||||
import { useLocation } from "@/lib/router";
|
||||
import { IssuePlanConfirmationActionBar } from "./IssuePlanConfirmationActionBar";
|
||||
|
||||
interface IssuePropertiesPlansTabProps {
|
||||
|
|
@ -14,6 +17,16 @@ interface IssuePropertiesPlansTabProps {
|
|||
inline?: boolean;
|
||||
}
|
||||
|
||||
function hasPendingPlanConfirmation(interactions: IssueThreadInteraction[] | undefined): boolean {
|
||||
return (interactions ?? []).some(
|
||||
(interaction) =>
|
||||
interaction.kind === "request_confirmation"
|
||||
&& interaction.status === "pending"
|
||||
&& interaction.payload.target?.type === "issue_document"
|
||||
&& interaction.payload.target.key === "plan",
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Plans tab of the redesigned properties pane (flag: enableTaskChatRedesign).
|
||||
*
|
||||
|
|
@ -25,19 +38,42 @@ interface IssuePropertiesPlansTabProps {
|
|||
*/
|
||||
export function IssuePropertiesPlansTab({ issue, inline }: IssuePropertiesPlansTabProps) {
|
||||
const { data: planDocument, isLoading: planDocumentLoading } = useIssuePlanDocument(issue.id);
|
||||
const location = useLocation();
|
||||
const [annotationPanelOpen, setAnnotationPanelOpen] = useState(false);
|
||||
const { data } = useQuery({
|
||||
queryKey: queryKeys.issues.acceptedPlanDecompositions(issue.id),
|
||||
queryFn: () => issuesApi.listAcceptedPlanDecompositions(issue.id),
|
||||
});
|
||||
const { data: interactions } = useQuery({
|
||||
queryKey: queryKeys.issues.interactions(issue.id),
|
||||
queryFn: () => issuesApi.listInteractions(issue.id),
|
||||
});
|
||||
const hasPlans = (data?.length ?? 0) > 0;
|
||||
const pendingPlanConfirmation = hasPendingPlanConfirmation(interactions);
|
||||
|
||||
if (!planDocument && !hasPlans) {
|
||||
return (
|
||||
<div className="px-1 py-6 text-sm text-muted-foreground">
|
||||
{planDocumentLoading
|
||||
? "Loading plan…"
|
||||
: "No plan yet. The plan document, accepted plans, and their revisions will appear here."}
|
||||
</div>
|
||||
<>
|
||||
{/* This is deliberately outside the plan-document gate: an interaction
|
||||
can arrive before its plan document query resolves or persists. */}
|
||||
<IssuePlanConfirmationActionBar issue={issue} inline={inline} />
|
||||
<div className="px-1 py-6 text-sm text-muted-foreground">
|
||||
{planDocumentLoading ? (
|
||||
"Loading plan…"
|
||||
) : issue.workMode === "planning" ? (
|
||||
<div className="space-y-2">
|
||||
<p>This task is in plan mode but no plan document has been written yet.</p>
|
||||
{pendingPlanConfirmation ? (
|
||||
<p className="text-amber-foreground">
|
||||
A plan confirmation is pending, but the plan document it should confirm is missing.
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
) : (
|
||||
"No plan yet. The plan document, accepted plans, and their revisions will appear here."
|
||||
)}
|
||||
</div>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -45,18 +81,41 @@ export function IssuePropertiesPlansTab({ issue, inline }: IssuePropertiesPlansT
|
|||
<div className="space-y-4 py-2">
|
||||
{/* Pending plan confirmation: its CTAs pin to the pane's footer slot so
|
||||
they stay visible while the plan scrolls. */}
|
||||
{planDocument ? <IssuePlanConfirmationActionBar issue={issue} inline={inline} /> : null}
|
||||
<IssuePlanConfirmationActionBar issue={issue} inline={inline} />
|
||||
{planDocument ? (
|
||||
<section data-testid="issue-plan-document" className="space-y-2">
|
||||
<div className="text-xs text-muted-foreground">
|
||||
<div className="flex items-center gap-1 text-xs text-muted-foreground">
|
||||
{`Revision ${planDocument.latestRevisionNumber ?? 1} · updated ${new Date(planDocument.updatedAt).toLocaleString([], {
|
||||
month: "short",
|
||||
day: "numeric",
|
||||
hour: "numeric",
|
||||
minute: "2-digit",
|
||||
})}`}
|
||||
<DocumentAnnotationsCountChip
|
||||
issueId={issue.id}
|
||||
docKey="plan"
|
||||
panelOpen={annotationPanelOpen}
|
||||
onToggle={() => setAnnotationPanelOpen((open) => !open)}
|
||||
/>
|
||||
</div>
|
||||
<MarkdownBody>{planDocument.body}</MarkdownBody>
|
||||
<IssueDocumentAnnotations
|
||||
issueId={issue.id}
|
||||
doc={{
|
||||
key: "plan",
|
||||
latestRevisionId: planDocument.latestRevisionId,
|
||||
latestRevisionNumber: planDocument.latestRevisionNumber,
|
||||
}}
|
||||
bodyMarkdown={planDocument.body}
|
||||
draftDirty={false}
|
||||
draftConflicted={false}
|
||||
historicalPreview={false}
|
||||
locationHash={location.hash}
|
||||
panelOpen={annotationPanelOpen}
|
||||
onPanelOpenChange={setAnnotationPanelOpen}
|
||||
panelPlacement="inline"
|
||||
>
|
||||
<MarkdownBody>{planDocument.body}</MarkdownBody>
|
||||
</IssueDocumentAnnotations>
|
||||
</section>
|
||||
) : null}
|
||||
{hasPlans ? (
|
||||
|
|
|
|||
Loading…
Reference in New Issue