From 95bf90bd2c71d38a9c188274fc01effcf1380236 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:17:36 -0600 Subject: [PATCH] fix(chat): post a conversation turn's answer while an approval holds the task (#20179) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People talk to agents in chat. The agent's final message becomes the chat reply. > - A chat turn can end while the task stays held. For example, the agent resubmits a hire approval and the approval is pending. > - The finalizer keeps the task in review for the approval (`governed_response_waiting`). That decision is not `conversation_turn_finished`. > - Run completion then treats the yielded turn as a control-plane wait. It records `presentationDecision: none` and posts no comment. > - The person never sees the agent's answer. The chat shows nothing after the request. > - This pull request counts a committed conversation turn as answered unless its own pending question or confirmation card presents it. > - The benefit is that the person always gets the reply, whatever state the task keeps. ## Linked Issues or Issue Description Refs #19971 (post the full final reply). That fix covers the reply text. This fix covers replies that were never posted. **What happened** In a chat, the person asked the CEO agent to change a proposed hire to a cheaper model and send it back for approval. The agent resubmitted the approval correctly. It called `paperclip_finish`, and Paperclip accepted the report with "task is still waiting for approval". The run succeeded and the task moved to `in_review`. The run event `run.presentation.resolved` recorded `commentAction: none` (reason `yielded_control_plane_wait` on master). The agent's final message never reached the chat. **Expected behavior** A chat turn's accepted final answer is always posted to the conversation. An approval, an execution stage or a dependency blocker can hold the task. It must not hide the reply. Only a question or confirmation card that the turn itself left pending presents the turn instead of its prose. **Steps to reproduce** 1. Open an Agent Chat with an agent. Link a hire approval in `revision_requested` to the chat task. 2. Ask the agent to change the hire and resubmit it. 3. The agent resubmits the approval and ends the turn with a yielded result and a response wake. 4. See that the chat shows no reply. The run is `succeeded` and its presentation decision is `none`. ## What Changed - `server/src/services/native-runtime/native-governance.ts`: move the pending-card query of `pendingNativeGovernance()` into `pendingNativeInteraction()`, with no change in behavior. Add `conversationTurnAnswered()`. It returns false only when a pending thread interaction (the same predicate as status arbitration) gates the turn. It checks cards on their own, so a pending execution stage does not hide a card. - `server/src/services/heartbeat/run-completion.ts`: a successful conversation run whose status decision committed now passes `conversationTurnFinished: true` to the presentation resolver, unless `conversationTurnAnswered()` is false. The existing `conversation_turn_finished` path is unchanged. - `server/src/services/heartbeat/run-completion.test.ts`: add a regression table with a real database. A pending approval, a revision-requested approval, a pending execution stage and a dependency blocker now post the answer. A finished turn still posts. The turn's own question card still owns the response, also when an execution stage is pending. - `doc/execution-semantics.md`: state the rule. Other finish outcomes I checked: - `done` or `needs_review` with a gate (`governed_gate_pending`): the result is not yielded, so the reply was already posted. - `blocked` (`paperclip_block`, waiting on a person or another task): not yielded, so the reply was already posted. - Yielded with a dependency blocker (`durable_dependency_blocker_bound`), or with `record_finalization_error` (`prior_status_preserved_no_live_path`, `monitor_wait_authority_lost`): the reply was also dropped. This fix covers them, because the rule does not depend on the reason code. ## Verification - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts`: 118 passed. Before the fix, 4 new cases failed (both approval states, execution stage, dependency blocker). The finished-turn and question-card cases passed both before and after. - `cd server && npx vitest run src/__tests__/heartbeat-run-summary.test.ts src/services/heartbeat/run-comment-selection.test.ts src/__tests__/slack-conversation-lifecycle.test.ts src/services/native-runtime/agent-chat-response-recovery.test.ts src/__tests__/chat-run-milestone-governed-work.test.ts src/__tests__/agent-conversations.test.ts`: all passed. - `tsc --noEmit -p server`: no errors in the changed files. Unrelated errors in this worktree come from an unbuilt local Runner package. - An end-to-end chat eval run on the case branch is in progress. I will add its result here. ## Risks - Low risk. The change only affects successful Agent Chat runs with a committed status decision. Ordinary tasks and external chat (Slack, GitHub) keep their own paths. External chat returns before this check. - A turn that yields while an approval is pending now posts its prose. The approval stays pending and the task status does not change. - One extra small read (the task row and pending gates) runs after a conversation turn that is not `conversation_turn_finished`. It runs after the provider turn ends, so it does not add time before the first token. - Previous-release state: no persisted shape changes. The fix reads `finalizationPhase` and `finalizationReasonCode`, which the previous release already writes. A run in flight at the deploy completes on the new code and posts its reply. A run that completed before the deploy keeps its recorded decision; nothing re-presents it. - Known gap, unchanged: background presentation recovery for conversations still requires `conversation_turn_finished` with a `done` result. If the server restarts between the status commit and the live presentation, a held yielded turn is not re-posted. That is a separate follow-up. > 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 - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use (shell, file edits, test runs) in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- doc/execution-semantics.md | 7 +++ .../services/heartbeat/run-completion.test.ts | 63 ++++++++++++++++++- .../src/services/heartbeat/run-completion.ts | 12 +++- .../native-runtime/native-governance.ts | 60 +++++++++++++----- 4 files changed, 123 insertions(+), 19 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 6541f4088b1..2478035988a 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -2241,6 +2241,13 @@ apply. Stream closure without a turn terminal is not proof of success. Event replay uses the existing source receipts and never repeats provider work merely to recover recorded output. +In an Agent Chat conversation, a successful turn whose status decision committed +posts its final answer to the conversation even when the task stays held: a +pending or revision-requested approval, a pending execution stage, or a +dependency blocker. A held task does not present the turn. Only a pending +thread interaction (a question or confirmation card) presents the turn in place +of its prose. + A delegated-task completion callback can finish native execution before its accepted final reply is saved. The completion outbox must retain that callback's queued target and attempt while the existing presentation selector finds pending diff --git a/server/src/services/heartbeat/run-completion.test.ts b/server/src/services/heartbeat/run-completion.test.ts index 6ff985ed53d..42c34facc34 100644 --- a/server/src/services/heartbeat/run-completion.test.ts +++ b/server/src/services/heartbeat/run-completion.test.ts @@ -16,7 +16,7 @@ import { createHash, randomUUID } from "node:crypto"; import { appendHeartbeatRunEvent } from "../heartbeat-run-events.js"; import { createNativeRunLogCoverageVerifier, formatNativeRunLogEvent } from "../native-runtime/native-run-log-export.js"; import type { Db } from "@paperclipai/db"; -import { activityLog, agents, agentWakeupRequests, budgetReservations, caseHandoffs, cases, companies, completionContracts, costEvents, createDb, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, heartbeatRuns, heartbeatRunEvents, issueComments, issueQuestionResponseDeliveries, issueRecoveryActions, issues, nativeRunFinalizations } from "@paperclipai/db"; +import { activityLog, agents, agentWakeupRequests, approvals, issueApprovals, budgetReservations, caseHandoffs, cases, companies, completionContracts, costEvents, createDb, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, heartbeatRuns, heartbeatRunEvents, issueComments, issueQuestionResponseDeliveries, issueRecoveryActions, issues, nativeRunFinalizations } from "@paperclipai/db"; import { and, eq, sql } from "drizzle-orm"; import { afterAll, beforeAll, beforeEach, describe, expect, it, onTestFinished, vi } from "vitest"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js"; @@ -469,6 +469,67 @@ describe.skipIf(!support.supported && !process.env.PAPERCLIP_TEST_DATABASE_URL)( expect(f.deps.finalizeAgentStatus).toHaveBeenCalledWith(f.agent.id, "succeeded", null, expect.any(Object)); }); + // A chat turn can finish while its task keeps waiting: a resubmitted hire + // approval, an execution stage or a dependency blocker holds the status. + // The accepted final answer must still reach the conversation. Only a card + // the run itself left pending in the thread presents the turn instead. + it.each([ + { hold: "pending approval", reasonCode: "governed_response_waiting", replies: true }, + { hold: "approval sent back for revision", reasonCode: "governed_response_waiting", replies: true }, + { hold: "execution stage", reasonCode: "governed_response_waiting", replies: true }, + { hold: "dependency blocker", reasonCode: "durable_dependency_blocker_bound", replies: true }, + { hold: "finished turn", reasonCode: "conversation_turn_finished", replies: true }, + { hold: "own question card", reasonCode: "governed_response_waiting", replies: false }, + { hold: "execution stage and own question card", reasonCode: "governed_response_waiting", replies: false }, + ] as const)("posts a waiting conversation turn's accepted answer: $hold", async ({ hold, reasonCode, replies }) => { + const f = await fixture({ runtimeMode: "native" }, `CNV${randomUUID().slice(0, 6)}`); + const [issue] = await db.insert(issues).values({ companyId: f.run.companyId, title: "Chat with CEO", + status: "in_progress", assigneeAgentId: f.agent.id, responsibleUserId: "local-board", + conversationAgentId: f.agent.id, conversationUserId: "local-board", conversationState: "active", + ...(hold.startsWith("execution stage") ? { executionState: { status: "pending" } } : {}), + }).returning(); + await db.update(heartbeatRuns).set({ nativeIssueId: issue.id, responsibleUserId: "local-board", + contextSnapshot: { issueId: issue.id, conversationSessionGeneration: 0 } }).where(eq(heartbeatRuns.id, f.run.id)); + if (hold.includes("approval")) { + const [approval] = await db.insert(approvals).values({ companyId: f.run.companyId, type: "hire_agent", + status: hold === "pending approval" ? "pending" : "revision_requested", payload: { name: "Researcher" } }).returning(); + await db.insert(issueApprovals).values({ companyId: f.run.companyId, issueId: issue.id, approvalId: approval.id }); + } + if (hold.endsWith("own question card")) { + await issueThreadInteractionService(db).create(issue, { kind: "ask_user_questions", sourceRunId: f.run.id, + continuationPolicy: "wake_assignee", payload: { version: 1, questions: [{ id: "model", prompt: "Which model?", + selectionMode: "single", required: true, options: [{ id: "flash", label: "Flash" }] }] } }, + { agentId: f.agent.id, runId: f.run.id }); + } + const answer = "Resubmitted the hire with the cheaper model. [Review the pending approval](/CNV/approvals/a1)."; + f.input.issueId = issue.id; + f.input.issueRef = issue; + f.input.issueContext = await createHeartbeatRunPreparation(db).getIssueExecutionContext(f.run.companyId, issue.id); + // The finalizer's saved shape, unchanged from the previous release. + f.input.adapterResult.resultJson = { + nativeResult: { schema: "paperclip.run_result.v1", reportedWorkDisposition: "yielded", + summary: "Waiting for the approval decision.", + continuation: { kind: "response_wake", summary: "Wait for the decision.", idempotencyKey: "hire-wait" } }, + finalizationPhase: "committed", finalizationReasonCode: reasonCode, + }; + f.deps.findLatestCompletedFinalAgentMessage.mockResolvedValue({ seq: 1, text: answer, sourceEventId: "final-event", channel: "final", + reasonCode: "latest_non_empty_completed_final_agent_message" }); + f.deps.issuesSvc.addComment.mockImplementation(async (issueId, body) => { + const [comment] = await db.insert(issueComments).values({ companyId: f.run.companyId, issueId, + authorAgentId: f.agent.id, authorType: "agent", createdByRunId: f.run.id, body }).returning(); + return { ...comment, authorType: "agent" as const }; + }); + + await f.completion.completeRun(f.input); + + const posted = (await db.select().from(issueComments).where(eq(issueComments.issueId, issue.id))) + .filter(comment => comment.createdByRunId === f.run.id); + expect(posted.map(comment => comment.body)).toEqual(replies ? [answer] : []); + expect((await read(f.run.id)).resultJson).toMatchObject({ presentationDecision: replies + ? { chosenSource: "final_agent_message", commentAction: "create", commentId: posted[0]?.id } + : { chosenSource: "none", commentAction: "none" } }); + }, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS); + it("settles the accepted native exact rate-card price without replacing it with zero", async () => { const f = await fixture({ runtimeMode: "native" }); const runnerInstanceId = randomUUID(); diff --git a/server/src/services/heartbeat/run-completion.ts b/server/src/services/heartbeat/run-completion.ts index 2a24bbe3006..425e681f025 100644 --- a/server/src/services/heartbeat/run-completion.ts +++ b/server/src/services/heartbeat/run-completion.ts @@ -45,6 +45,7 @@ import { isUnresolvedWorkspaceBaseRefError, readUnresolvedWorkspaceBaseRefDiagno import { runnerGoalService } from "../runner-goals.js"; import { resolveChatRunPresentationAuthorizationReason } from "../chat-run-publications.js"; import { hasChatRunOwnedProviderInteraction } from "../chat-interaction-arbitration.js"; +import { conversationTurnAnswered } from "../native-runtime/native-governance.js"; import { readAcceptedTaskContinuationSummary } from "../heartbeat-run-summary.js"; import { SANDBOX_PROVIDER_PLUGIN_NOT_READY_REASON } from "../recovery/stranded-notice.js"; import { MAX_TURN_CONTINUATION_RETRY_REASON, isResolvedInteractionContinuationWakeContext } from "../../modules/run-dispatch/index.js"; @@ -845,8 +846,15 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC resultJson: persistedResultJson, interactionOwnsResponse: Boolean(issueId && readAcceptedTaskContinuationSummary(persistedResultJson) && await hasChatRunOwnedProviderInteraction(db, { companyId: livenessRun.companyId, issueId, runId: livenessRun.id })), - conversationTurnFinished: isConversation(issueContext) && livenessRun.status === "succeeded" - && persistedResultJson?.finalizationReasonCode === "conversation_turn_finished", + conversationTurnFinished: Boolean(issueId) && isConversation(issueContext) && livenessRun.status === "succeeded" + && (persistedResultJson?.finalizationReasonCode === "conversation_turn_finished" + // A turn left waiting on an approval, an execution stage or a + // blocker still answered the person. Only its own question card + // presents it instead. + || (persistedResultJson?.finalizationPhase === "committed" + && await measureRunPhase("response.resolve", () => conversationTurnAnswered({ + db, companyId: livenessRun.companyId, issueId: issueId!, runId: livenessRun.id, + })))), existingComment: existingRunComment, finalAgentMessage, preferFinalResponseOverExistingComment: diff --git a/server/src/services/native-runtime/native-governance.ts b/server/src/services/native-runtime/native-governance.ts index dd8f2a27835..84b7c5c5b80 100644 --- a/server/src/services/native-runtime/native-governance.ts +++ b/server/src/services/native-runtime/native-governance.ts @@ -24,6 +24,32 @@ export async function lockNativeGovernanceSources(input: { db: Db; companyId: st eq(approvals.companyId, input.companyId))).for("share", { noWait: true }); } +/** The pending thread interaction (question or confirmation card) that gates this run, if any. */ +async function pendingNativeInteraction(input: { + db: Db; + companyId: string; + issueId: string; + runId: string; + ignoredInteractionIds?: string[]; +}): Promise<{ id: string } | null> { + const [issue] = await input.db.select({ conversationAgentId: issues.conversationAgentId, conversationUserId: issues.conversationUserId }) + .from(issues).where(and(eq(issues.id, input.issueId), eq(issues.companyId, input.companyId))); + return input.db + .select({ id: issueThreadInteractions.id }) + .from(issueThreadInteractions) + .where( + and( + eq(issueThreadInteractions.companyId, input.companyId), + eq(issueThreadInteractions.issueId, input.issueId), + eq(issueThreadInteractions.status, "pending"), + ...(input.ignoredInteractionIds?.length ? [notInArray(issueThreadInteractions.id, input.ignoredInteractionIds)] : []), + activeIssueInteractionCondition({ runId: input.runId, conversationMode: isConversation(issue) }), + ), + ) + .limit(1) + .then((rows) => rows[0] ?? null); +} + export async function pendingNativeGovernance(input: { db: Db; companyId: string; @@ -37,23 +63,8 @@ export async function pendingNativeGovernance(input: { if (record(input.executionState).status === "pending") { return { kind: "execution_stage", id: input.runId }; } - const [issue] = await input.db.select({ conversationAgentId: issues.conversationAgentId, conversationUserId: issues.conversationUserId }) - .from(issues).where(and(eq(issues.id, input.issueId), eq(issues.companyId, input.companyId))); const [pendingInteraction, pendingApproval] = await Promise.all([ - input.db - .select({ id: issueThreadInteractions.id }) - .from(issueThreadInteractions) - .where( - and( - eq(issueThreadInteractions.companyId, input.companyId), - eq(issueThreadInteractions.issueId, input.issueId), - eq(issueThreadInteractions.status, "pending"), - ...(input.ignoredInteractionIds?.length ? [notInArray(issueThreadInteractions.id, input.ignoredInteractionIds)] : []), - activeIssueInteractionCondition({ runId: input.runId, conversationMode: isConversation(issue) }), - ), - ) - .limit(1) - .then((rows) => rows[0] ?? null), + pendingNativeInteraction(input), input.db .select({ id: approvals.id }) .from(issueApprovals) @@ -75,3 +86,20 @@ export async function pendingNativeGovernance(input: { return null; } + +/** + * A committed conversation turn has answered the person, whatever status its + * task keeps. An approval or an execution stage can hold the task, but neither + * presents that answer in the conversation. Only a pending thread interaction + * (a question or confirmation card) is the turn's visible response. + */ +export async function conversationTurnAnswered(input: { + db: Db; + companyId: string; + issueId: string; + runId: string; +}): Promise { + // Check cards on their own: an execution stage outranks a card as the + // status gate, but it does not present the turn. + return !(await pendingNativeInteraction(input)); +}