[codex] Improve issue thread review flow (#4381)

## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies
> - Issue detail is where operators coordinate review, approvals, and
follow-up work with active runs
> - That thread UI needs to surface blockers, descendants, review
handoffs, and reply ergonomics clearly enough for humans to guide agent
work
> - Several small gaps in the issue-thread flow were making review and
navigation clunkier than necessary
> - This pull request improves the reply composer, descendant/blocker
presentation, interaction folding, and review-request handoff plumbing
together as one cohesive issue-thread workflow slice
> - The benefit is a cleaner operator review loop without changing the
broader task model

## What Changed

- restored and refined the floating reply composer behavior in the issue
thread
- folded expired confirmation interactions and improved post-submit
thread scrolling behavior
- surfaced descendant issue context and inline blocker/paused-assignee
notices on the issue detail view
- tightened large-board first paint behavior in `IssuesList`
- added loose review-request handoffs through the issue
execution-policy/update path and covered them with tests

## Verification

- `pnpm vitest run ui/src/pages/IssueDetail.test.tsx`
- `pnpm vitest run server/src/__tests__/issues-service.test.ts
server/src/__tests__/issue-execution-policy.test.ts`
- `pnpm exec vitest run --project @paperclipai/ui
ui/src/components/IssueChatThread.test.tsx
ui/src/components/IssueProperties.test.tsx
ui/src/components/IssuesList.test.tsx ui/src/lib/issue-tree.test.ts
ui/src/api/issues.test.ts`
- `pnpm exec vitest run --project @paperclipai/adapter-utils
packages/adapter-utils/src/server-utils.test.ts`
- `pnpm exec vitest run --project @paperclipai/server
server/src/__tests__/issue-comment-reopen-routes.test.ts -t "coerces
executor handoff patches into workflow-controlled review wakes|wakes the
return assignee with execution_changes_requested"`
- `pnpm exec vitest run --project @paperclipai/server
server/src/__tests__/issue-execution-policy.test.ts
server/src/__tests__/issues-service.test.ts`

## Visual Evidence

- UI layout changes are covered by the focused issue-thread component
and issue-detail tests listed above. Browser screenshots were not
attachable from this automated greploop environment, so reviewers should
use the running preview for final visual confirmation.

## Risks

- Moderate UI-flow risk: these changes touch the issue detail experience
in multiple spots, so regressions would most likely show up as
thread-layout quirks or incorrect review-handoff behavior

> 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-based coding agent with tool use and code execution
in the Codex CLI environment

## 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 or documented the visual verification path
- [ ] 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:
Dotta 2026-04-24 08:02:45 -05:00 committed by GitHub
parent 35a9dc37b0
commit 7ad225a198
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 1046 additions and 44 deletions

View file

@ -795,6 +795,9 @@ describe("issue comment reopen routes", () => {
status: "in_review",
assigneeAgentId: null,
assigneeUserId: "local-board",
reviewRequest: {
instructions: "Please verify the fix against the reproduction steps and note any residual risk.",
},
});
expect(res.status).toBe(200);
@ -811,6 +814,9 @@ describe("issue comment reopen routes", () => {
type: "agent",
agentId: "22222222-2222-4222-8222-222222222222",
},
reviewRequest: {
instructions: "Please verify the fix against the reproduction steps and note any residual risk.",
},
});
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"33333333-3333-4333-8333-333333333333",
@ -821,6 +827,9 @@ describe("issue comment reopen routes", () => {
executionStage: expect.objectContaining({
wakeRole: "reviewer",
stageType: "review",
reviewRequest: {
instructions: "Please verify the fix against the reproduction steps and note any residual risk.",
},
allowedActions: ["approve", "request_changes"],
}),
}),

View file

@ -171,6 +171,75 @@ describe("issue execution policy transitions", () => {
expect(result.decision).toBeUndefined();
});
it("carries loose review instructions on the pending handoff", () => {
const reviewInstructions = [
"Please focus on whether the migration path is reversible.",
"",
"- Check failure handling",
"- Call out any unclear operator instructions",
].join("\n");
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_progress",
assigneeAgentId: coderAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: null,
},
policy,
requestedStatus: "done",
requestedAssigneePatch: {},
actor: { agentId: coderAgentId },
commentBody: "Implemented the migration",
reviewRequest: { instructions: reviewInstructions },
});
expect(result.patch.executionState).toMatchObject({
status: "pending",
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
reviewRequest: { instructions: reviewInstructions },
});
});
it("clears loose review instructions with explicit null during a stage transition", () => {
const reviewStageId = policy.stages[0].id;
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_progress",
assigneeAgentId: coderAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: reviewStageId,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
reviewRequest: { instructions: "Old review request" },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
},
policy,
requestedStatus: "in_review",
requestedAssigneePatch: {},
actor: { agentId: coderAgentId },
commentBody: "Ready for review",
reviewRequest: null,
});
expect(result.patch.executionState).toMatchObject({
status: "pending",
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
reviewRequest: null,
});
});
it("reviewer approves → advances to approval stage", () => {
const reviewStageId = policy.stages[0].id;
const result = applyIssueExecutionPolicyTransition({
@ -214,6 +283,44 @@ describe("issue execution policy transitions", () => {
});
});
it("lets a reviewer provide loose instructions for the next approval stage", () => {
const reviewStageId = policy.stages[0].id;
const approvalInstructions = "Please decide whether this is ready to ship, with any launch caveats.";
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: reviewStageId,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
reviewRequest: { instructions: "Review the implementation details." },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
},
policy,
requestedStatus: "done",
requestedAssigneePatch: {},
actor: { agentId: qaAgentId },
commentBody: "QA signoff complete",
reviewRequest: { instructions: approvalInstructions },
});
expect(result.patch.executionState).toMatchObject({
status: "pending",
currentStageType: "approval",
currentParticipant: { type: "user", userId: ctoUserId },
reviewRequest: { instructions: approvalInstructions },
});
});
it("approver approves → marks completed (allows done)", () => {
const reviewStageId = policy.stages[0].id;
const approvalStageId = policy.stages[1].id;

View file

@ -355,6 +355,110 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
expect(result.map((issue) => issue.id)).toEqual([commentMatchId, descriptionMatchId]);
});
it("filters issue lists to the full descendant tree for a root issue", async () => {
const companyId = randomUUID();
const rootId = randomUUID();
const childId = randomUUID();
const grandchildId = randomUUID();
const siblingId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
requireBoardApprovalForNewAgents: false,
});
await db.insert(issues).values([
{
id: rootId,
companyId,
title: "Root",
status: "todo",
priority: "medium",
},
{
id: childId,
companyId,
parentId: rootId,
title: "Child",
status: "todo",
priority: "medium",
},
{
id: grandchildId,
companyId,
parentId: childId,
title: "Grandchild",
status: "todo",
priority: "medium",
},
{
id: siblingId,
companyId,
title: "Sibling",
status: "todo",
priority: "medium",
},
]);
const result = await svc.list(companyId, { descendantOf: rootId });
expect(new Set(result.map((issue) => issue.id))).toEqual(new Set([childId, grandchildId]));
});
it("combines descendant filtering with search", async () => {
const companyId = randomUUID();
const rootId = randomUUID();
const childId = randomUUID();
const grandchildId = randomUUID();
const outsideMatchId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
requireBoardApprovalForNewAgents: false,
});
await db.insert(issues).values([
{
id: rootId,
companyId,
title: "Root",
status: "todo",
priority: "medium",
},
{
id: childId,
companyId,
parentId: rootId,
title: "Relevant parent",
status: "todo",
priority: "medium",
},
{
id: grandchildId,
companyId,
parentId: childId,
title: "Needle grandchild",
status: "todo",
priority: "medium",
},
{
id: outsideMatchId,
companyId,
title: "Needle outside",
status: "todo",
priority: "medium",
},
]);
const result = await svc.list(companyId, { descendantOf: rootId, q: "needle" });
expect(result.map((issue) => issue.id)).toEqual([grandchildId]);
});
it("accepts issue identifiers through getById", async () => {
const companyId = randomUUID();
const issueId = randomUUID();

View file

@ -101,6 +101,7 @@ type ExecutionStageWakeContext = {
stageType: ParsedExecutionState["currentStageType"];
currentParticipant: ParsedExecutionState["currentParticipant"];
returnAssignee: ParsedExecutionState["returnAssignee"];
reviewRequest: ParsedExecutionState["reviewRequest"];
lastDecisionOutcome: ParsedExecutionState["lastDecisionOutcome"];
allowedActions: string[];
};
@ -124,6 +125,7 @@ function buildExecutionStageWakeContext(input: {
stageType: input.state.currentStageType,
currentParticipant: input.state.currentParticipant,
returnAssignee: input.state.returnAssignee,
reviewRequest: input.state.reviewRequest ?? null,
lastDecisionOutcome: input.state.lastDecisionOutcome,
allowedActions: input.allowedActions,
};
@ -831,6 +833,7 @@ export function issueRoutes(
workspaceId: req.query.workspaceId as string | undefined,
executionWorkspaceId: req.query.executionWorkspaceId as string | undefined,
parentId: req.query.parentId as string | undefined,
descendantOf: req.query.descendantOf as string | undefined,
labelId: req.query.labelId as string | undefined,
originKind: req.query.originKind as string | undefined,
originId: req.query.originId as string | undefined,
@ -1787,6 +1790,7 @@ export function issueRoutes(
: null;
const {
comment: commentBody,
reviewRequest,
reopen: reopenRequested,
interrupt: interruptRequested,
hiddenAt: hiddenAtRaw,
@ -1813,7 +1817,8 @@ export function issueRoutes(
: false;
let interruptedRunId: string | null = null;
const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(existing);
const isAgentWorkUpdate = req.actor.type === "agent" && Object.keys(updateFields).length > 0;
const isAgentWorkUpdate =
req.actor.type === "agent" && (Object.keys(updateFields).length > 0 || reviewRequest !== undefined);
if (closedExecutionWorkspace && (commentBody || isAgentWorkUpdate)) {
respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace);
@ -1887,6 +1892,7 @@ export function issueRoutes(
userId: actor.actorType === "user" ? actor.actorId : null,
},
commentBody,
reviewRequest: reviewRequest === undefined ? undefined : reviewRequest,
});
const decisionId = transition.decision ? randomUUID() : null;
if (decisionId) {
@ -1900,6 +1906,20 @@ export function issueRoutes(
};
}
Object.assign(updateFields, transition.patch);
if (reviewRequest !== undefined && transition.patch.executionState === undefined) {
const existingExecutionState = parseIssueExecutionState(existing.executionState);
if (!existingExecutionState || existingExecutionState.status !== "pending") {
if (reviewRequest !== null) {
res.status(422).json({ error: "reviewRequest requires an active review or approval stage" });
return;
}
} else {
updateFields.executionState = {
...existingExecutionState,
reviewRequest,
};
}
}
const nextAssigneeAgentId =
updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null);

View file

@ -31,6 +31,7 @@ type TransitionInput = {
requestedAssigneePatch: RequestedAssigneePatch;
actor: ActorLike;
commentBody?: string | null;
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
};
type TransitionResult = {
@ -168,6 +169,7 @@ function buildCompletedState(previous: IssueExecutionState | null, currentStage:
currentStageType: null,
currentParticipant: null,
returnAssignee: previous?.returnAssignee ?? null,
reviewRequest: null,
completedStageIds,
lastDecisionId: previous?.lastDecisionId ?? null,
lastDecisionOutcome: "approved",
@ -186,6 +188,7 @@ function buildStateWithCompletedStages(input: {
currentStageType: input.previous?.currentStageType ?? null,
currentParticipant: input.previous?.currentParticipant ?? null,
returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee,
reviewRequest: input.previous?.reviewRequest ?? null,
completedStageIds: input.completedStageIds,
lastDecisionId: input.previous?.lastDecisionId ?? null,
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
@ -204,6 +207,7 @@ function buildSkippedStageCompletedState(input: {
currentStageType: null,
currentParticipant: null,
returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee,
reviewRequest: null,
completedStageIds: input.completedStageIds,
lastDecisionId: input.previous?.lastDecisionId ?? null,
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
@ -216,6 +220,7 @@ function buildPendingState(input: {
stageIndex: number;
participant: IssueExecutionStagePrincipal;
returnAssignee: IssueExecutionStagePrincipal | null;
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
}): IssueExecutionState {
return {
status: PENDING_STATUS,
@ -224,6 +229,7 @@ function buildPendingState(input: {
currentStageType: input.stage.type,
currentParticipant: input.participant,
returnAssignee: input.returnAssignee,
reviewRequest: input.reviewRequest ?? null,
completedStageIds: input.previous?.completedStageIds ?? [],
lastDecisionId: input.previous?.lastDecisionId ?? null,
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
@ -236,6 +242,7 @@ function buildChangesRequestedState(previous: IssueExecutionState, currentStage:
status: CHANGES_REQUESTED_STATUS,
currentStageId: currentStage.id,
currentStageType: currentStage.type,
reviewRequest: null,
lastDecisionOutcome: "changes_requested",
};
}
@ -247,6 +254,7 @@ function buildPendingStagePatch(input: {
stage: IssueExecutionStage;
participant: IssueExecutionStagePrincipal;
returnAssignee: IssueExecutionStagePrincipal | null;
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
}) {
input.patch.status = "in_review";
Object.assign(input.patch, patchForPrincipal(input.participant));
@ -256,6 +264,7 @@ function buildPendingStagePatch(input: {
stageIndex: input.policy.stages.findIndex((candidate) => candidate.id === input.stage.id),
participant: input.participant,
returnAssignee: input.returnAssignee,
reviewRequest: input.reviewRequest,
});
}
@ -295,6 +304,9 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
const currentStage = input.policy ? findStageById(input.policy, existingState?.currentStageId) : null;
const requestedStatus = input.requestedStatus;
const activeStage = currentStage && existingState?.status === PENDING_STATUS ? currentStage : null;
const effectiveReviewRequest = input.reviewRequest === undefined
? existingState?.reviewRequest ?? null
: input.reviewRequest;
if (!input.policy) {
if (existingState) {
@ -359,6 +371,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
stage: activeStage,
participant,
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
reviewRequest: effectiveReviewRequest,
});
return {
patch,
@ -405,6 +418,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
stage: nextStage,
participant,
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
reviewRequest: input.reviewRequest ?? null,
});
return {
patch,
@ -461,6 +475,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
stage: activeStage,
participant: currentParticipant,
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
reviewRequest: effectiveReviewRequest,
});
return {
patch,
@ -538,6 +553,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
stage: pendingStage,
participant,
returnAssignee,
reviewRequest: input.reviewRequest ?? null,
});
return {
patch,

View file

@ -106,6 +106,7 @@ export interface IssueFilters {
workspaceId?: string;
executionWorkspaceId?: string;
parentId?: string;
descendantOf?: string;
labelId?: string;
originKind?: string;
originId?: string;
@ -1396,6 +1397,24 @@ export function issueService(db: Db) {
AND ${issueComments.body} ILIKE ${containsPattern} ESCAPE '\\'
)
`;
if (filters?.descendantOf) {
conditions.push(sql<boolean>`
${issues.id} IN (
WITH RECURSIVE descendants(id) AS (
SELECT ${issues.id}
FROM ${issues}
WHERE ${issues.companyId} = ${companyId}
AND ${issues.parentId} = ${filters.descendantOf}
UNION
SELECT ${issues.id}
FROM ${issues}
JOIN descendants ON ${issues.parentId} = descendants.id
WHERE ${issues.companyId} = ${companyId}
)
SELECT id FROM descendants
)
`);
}
if (filters?.status) {
const statuses = filters.status.split(",").map((s) => s.trim());
conditions.push(statuses.length === 1 ? eq(issues.status, statuses[0]) : inArray(issues.status, statuses));