mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
fix(heartbeat): deliver a person's answer that waited behind a turn that closed the task (#20996)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - An agent can ask the person a question on its task, and the person
answers it in the task chat
> - The person can answer while the agent's turn is still running. The
answer then waits for that turn to finish
> - If that turn completes the task, the release step cancels every
waiting wake for the assignee on the done task. The person's own queued
messages reopen the task, but the answer was dropped and no turn
received it
> - A person who answers an agent's question expects the agent to get
the answer. Losing it without a trace is a silent failure
> - This pull request treats an answer saved before the task was
completed as owed input. It reopens the task and delivers the answer
once
> - The benefit is that a person's answer is never dropped because the
agent finished its turn first
## Linked Issues or Issue Description
**What happened?**
The paid chat eval `steering.approval-delivery` showed it. The person
answered the agent's naming question while the agent's turn was running.
The turn completed the task. The answer's own wake was then cancelled
with "Deferred execution wake no longer applies to a terminal task". No
turn received the answer.
**Steps to reproduce**
1. Start a turn for the assignee on a task.
2. While the turn runs, answer the agent's question on that task.
3. Let the turn mark the task done.
4. The answer's wake is cancelled, and no run receives it.
**Expected behavior**
The answer reaches the assignee exactly once. An answer saved after the
task was completed is still dropped, as before.
Refs #20981, #19331
## What Changed
- `wake-queue/application/use-cases.ts`: before cancelling a deferred
assignee wake on a done task, check whether it carries a person's owed
answer. If it does, reopen the task to `in_progress` and promote the
wake. An answer continues the assignee's own execution, so the existing
continuation status gate applies unchanged.
- `wake-queue/adapters/postgres.ts`: add `owedInteractionAnswer`. It
applies only to an undelivered response that a person resolved, that is
not a tool action, and that was saved at or before the task's
`completedAt`. It returns `blocked` when unresolved blockers forbid
`in_progress`. `reopenIssue` takes an optional target status.
- The blocked case cancels the answer wake with an explicit reason and
does not reopen the task, so the release step does not fail. A
regression test reproduces the earlier throw (fix for the Greptile P1).
- `heartbeat-answer-behind-settling-run.test.ts`: an end-to-end
heartbeat test. It is red on master (the answer wake is cancelled and no
run is bound to it) and green here (one run is bound and succeeds). A
control test shows that an answer saved after completion is still
dropped.
- `use-cases.test.ts`: unit cases for an owed answer, a not-owed answer,
a cancelled task, and an agent-requested wake.
- Eval `steering.approval-delivery` oracle v7: also accept the Acorn
document when the worker's settling turn saved it after the answer was
saved. The eval still requires exactly one later turn bound to the
answer. Calibration tests cover the retained shape, reworded text, a
pre-answer save, the wrong name, another agent, a stale revision, and a
missing bound turn.
## Verification
- `cd server && npx vitest run
src/__tests__/heartbeat-answer-behind-settling-run.test.ts
src/modules/wake-queue
src/__tests__/heartbeat-blocked-question-answer.test.ts` (351 passed)
- Red check: with the `server/src/modules` changes reverted, the new
delivery test fails.
- `cd server && npx vitest run
src/__tests__/heartbeat-comment-wake-batching.test.ts
src/__tests__/question-response-delivery.test.ts` passed. In
`issue-queued-comments-routes.test.ts` all 138 tests passed, but a
cleanup hook timed out locally. CI covers this file.
- `npx vitest run --config tests/runner-e2e/vitest.config.ts
tests/runner-e2e/chat-catalog-domains/runtime-approval-queue.test.ts
tests/runner-e2e/chat-catalog-consistency.test.ts` (58 passed)
- `tsc --noEmit` passes for `server` and `tests/runner-e2e`.
- Paid single-case run 43136466619 on 8a4eaab2c0 passed: one turn was
bound to the answer, there were 5 runs within the bound, and the task
was done. The later blocker commit adds only a blocker read on that
path.
## Risks
- Behavior change: a done task reopens when a person's answer was saved
before completion and no run received it. In that case the agent gets
one more turn. Answers saved after completion, cancelled tasks,
tool-action outcomes, and agent-requested wakes do not change.
- If the agent already read the answer by itself, the extra turn sees
the finished work and completes again. That is one extra turn, and the
answer is not repeated.
- Known gap: the answer is still dropped when the task has unresolved
blockers. This happens when a blocker reopens during the turn. Such a
task cannot move to `in_progress`, and an answer turn cannot run on any
other status. In this case the release step cancels the answer wake with
the reason "Queued answer cannot continue a task with unresolved
blockers", keeps the task done, and continues to the next wake. This is
not a regression, because master dropped the answer in every case. But
it does not meet the goal that a person's answer is never lost. It is a
separate follow-up.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
5626a0299c
commit
2a1620f143
8 files changed
+295
-9
No files matched your search
@@ -34,6 +34,7 @@ import {
|
||||
nativeRunFinalizations,
|
||||
} from "@paperclipai/db";
|
||||
import { hasConversationContinuationPolicy } from "../../../services/conversation-continuation.js";
|
||||
import { readQueuedInteractionResponse } from "../../../services/queued-interaction-response.js";
|
||||
import { legacyExecutionNeedsReconciliationWithEvidence } from "../../../services/legacy-execution-recovery.js";
|
||||
import {
|
||||
authorizeFailedChatRunRetryWake,
|
||||
@@ -450,6 +451,18 @@ function buildTransaction(tx: Db, deps: WakeQueuePostgresAdapterDeps, db: Db, ru
|
||||
};
|
||||
},
|
||||
|
||||
async owedInteractionAnswer({ companyId, issueId, payload }) {
|
||||
const response = await readQueuedInteractionResponse(tx as unknown as Db, companyId, issueId, payload);
|
||||
if (!response || response.toolActionOutcome || !response.comment.authorUserId) return null;
|
||||
const [task] = await tx.select({ completedAt: issues.completedAt }).from(issues)
|
||||
.where(and(eq(issues.companyId, companyId), eq(issues.id, issueId)));
|
||||
const answeredAt = new Date(response.comment.createdAt);
|
||||
if (!task || (task.completedAt && answeredAt.getTime() > task.completedAt.getTime())) return null;
|
||||
// in_progress is refused while blockers are unresolved, and reopening would throw inside this drain.
|
||||
const readiness = await issuesSvc.getDependencyReadiness(issueId, tx);
|
||||
return readiness.unresolvedBlockerIssueIds.length > 0 ? "blocked" : "deliver";
|
||||
},
|
||||
|
||||
async getCommentSelfAuthorship({ companyId, issueId, finishingRunId, commentIds }) {
|
||||
const rows = await tx
|
||||
.select({ createdByRunId: issueComments.createdByRunId })
|
||||
@@ -500,8 +513,8 @@ function buildTransaction(tx: Db, deps: WakeQueuePostgresAdapterDeps, db: Db, ru
|
||||
return isCompletedOnboardingHandoffWake(tx, input);
|
||||
},
|
||||
|
||||
async reopenIssue({ companyId, issueId }) {
|
||||
const updated = await issuesSvc.updateForCompany(issueId, companyId, { status: "todo", executionState: null }, tx);
|
||||
async reopenIssue({ companyId, issueId, status = "todo" }) {
|
||||
const updated = await issuesSvc.updateForCompany(issueId, companyId, { status, executionState: null }, tx);
|
||||
return updated ? toIssueSnapshot(updated as unknown as IssueRow) : null;
|
||||
},
|
||||
|
||||
|
||||
@@ -165,6 +165,12 @@ export interface WakeQueueTransaction {
|
||||
reason: string | null;
|
||||
releasePolicy: unknown;
|
||||
}>;
|
||||
/**
|
||||
* Whether the wake carries a person's saved answer to an interaction on this task that no run has received yet and
|
||||
* that was saved before the task was completed. Such an answer is owed input, like the person's own queued message.
|
||||
* "deliver" when the task can run now; "blocked" when unresolved blockers keep it from in_progress; null otherwise.
|
||||
*/
|
||||
owedInteractionAnswer(input: { companyId: string; issueId: string; payload: Record<string, unknown> }): Promise<"deliver" | "blocked" | null>;
|
||||
getCommentSelfAuthorship(input: {
|
||||
companyId: string;
|
||||
issueId: string;
|
||||
@@ -179,7 +185,8 @@ export interface WakeQueueTransaction {
|
||||
wakeAgentId: string;
|
||||
commentIds: string[];
|
||||
}): Promise<boolean>;
|
||||
reopenIssue(input: { companyId: string; issueId: string; runId: string }): Promise<IssueSnapshot | null>;
|
||||
/** Reopens to todo, or straight to in_progress for an owed answer, which continues the assignee's own execution. */
|
||||
reopenIssue(input: { companyId: string; issueId: string; runId: string; status?: "todo" | "in_progress" }): Promise<IssueSnapshot | null>;
|
||||
/** Verifies a Done onboarding parent's completion wake against its own completed children. */
|
||||
isCompletedOnboardingHandoffWake(input: { companyId: string; issueId: string; agentId: string;
|
||||
reason: string | null; contextSnapshot: Record<string, unknown> }): Promise<boolean>;
|
||||
|
||||
@@ -115,6 +115,7 @@ function createFakeTransaction(overrides: Partial<WakeQueueTransaction> = {}): W
|
||||
releasePolicy: null,
|
||||
})),
|
||||
getCommentSelfAuthorship: vi.fn(async () => ({ allSelfAuthored: false })),
|
||||
owedInteractionAnswer: vi.fn(async (): Promise<"deliver" | "blocked" | null> => null),
|
||||
isCompletedDelegationMention: vi.fn(async () => false),
|
||||
reopenIssue: vi.fn(async () => null),
|
||||
claimDeferredWakeForPromotion: vi.fn(async () => true),
|
||||
@@ -594,6 +595,54 @@ describe("releaseIssueExecution", () => {
|
||||
expect(result.outcome.kind).toBe("promoted");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["owed answer on a done task", "done", "user", "deliver", "promoted"],
|
||||
["answer already delivered or saved after completion", "done", "user", null, "released"],
|
||||
["answer on a cancelled task", "cancelled", "user", "deliver", "released"],
|
||||
["agent-requested interaction wake", "done", "agent", "deliver", "released"],
|
||||
] as const)("handles a person's answer queued behind a turn that closed the task: %s", async (_label, status, actor, owed, outcome) => {
|
||||
const queue = [wakeCandidate({ agentId: ISSUE.assigneeAgentId!, requestedByActorType: actor,
|
||||
payload: { issueId: ISSUE.id, mutation: "interaction", interactionId: "11111111-1111-4111-8111-111111111111", interactionStatus: "answered" } })];
|
||||
const transaction = createFakeTransaction({
|
||||
findNextDeferredWake: vi.fn(async () => queue.shift() ?? null),
|
||||
owedInteractionAnswer: vi.fn(async () => owed),
|
||||
reopenIssue: vi.fn(async () => ({ ...ISSUE, status: "in_progress" })),
|
||||
});
|
||||
const release = createReleaseIssueExecution({
|
||||
issueLock: createFakeIssueLock(createFakeHost(), transaction, { ...ISSUE, status }),
|
||||
recovery: createFakeRecovery(),
|
||||
});
|
||||
const result = await release({ companyId: RUN.companyId, runId: RUN.id, now: new Date() });
|
||||
expect(result.outcome.kind).toBe(outcome);
|
||||
if (outcome === "promoted") {
|
||||
expect(transaction.reopenIssue).toHaveBeenCalledWith(expect.objectContaining({ status: "in_progress" }));
|
||||
expect(transaction.cancelDeferredWake).not.toHaveBeenCalled();
|
||||
} else {
|
||||
expect(transaction.reopenIssue).not.toHaveBeenCalled();
|
||||
expect(transaction.cancelDeferredWake).toHaveBeenCalledWith(expect.objectContaining({ reason: "Deferred execution wake no longer applies to a terminal task" }));
|
||||
}
|
||||
});
|
||||
|
||||
it("cancels an owed answer without reopening or failing the drain while blockers forbid in_progress", async () => {
|
||||
const queue = [wakeCandidate({ id: "answer", agentId: ISSUE.assigneeAgentId!, requestedByActorType: "user",
|
||||
payload: { issueId: ISSUE.id, mutation: "interaction", interactionId: "11111111-1111-4111-8111-111111111111", interactionStatus: "answered" } }),
|
||||
wakeCandidate({ id: "next", agentId: ISSUE.assigneeAgentId!, requestedByActorType: "agent" })];
|
||||
const transaction = createFakeTransaction({
|
||||
findNextDeferredWake: vi.fn(async () => queue.shift() ?? null),
|
||||
owedInteractionAnswer: vi.fn(async () => "blocked" as const),
|
||||
});
|
||||
const release = createReleaseIssueExecution({
|
||||
issueLock: createFakeIssueLock(createFakeHost(), transaction, { ...ISSUE, status: "done" }),
|
||||
recovery: createFakeRecovery(),
|
||||
});
|
||||
const result = await release({ companyId: RUN.companyId, runId: RUN.id, now: new Date() });
|
||||
expect(result.outcome.kind).toBe("released");
|
||||
expect(transaction.reopenIssue).not.toHaveBeenCalled();
|
||||
expect(transaction.cancelDeferredWake).toHaveBeenCalledWith(expect.objectContaining({ wakeId: "answer", reason: "Queued answer cannot continue a task with unresolved blockers" }));
|
||||
// The drain continues past the answer to the next queued wake.
|
||||
expect(transaction.cancelDeferredWake).toHaveBeenCalledWith(expect.objectContaining({ wakeId: "next" }));
|
||||
});
|
||||
|
||||
it.each(["done_live", "cancelled_live", "done_missing", "done_self", "done_no_resume", "done_untracked_comment"])("handles explicit agent feedback after completion: %s", async (scenario) => {
|
||||
const commentIds = ["accepted-agent-feedback"];
|
||||
const queue = [wakeCandidate({
|
||||
|
||||
@@ -374,6 +374,31 @@ async function promoteDeferredWake(
|
||||
workingCandidate.queuedCommentIds.length > 0));
|
||||
}
|
||||
|
||||
// A person's answer saved while the task was still open is owed input. The
|
||||
// worker may have finished its turn without reading it; closing the task
|
||||
// must not silently drop it. It reopens the task straight to in_progress,
|
||||
// because an answer continues the assignee's own execution. While
|
||||
// unresolved blockers forbid in_progress, an answer continuation cannot run
|
||||
// (it requires in_progress), so the task stays done and the wake is
|
||||
// cancelled with that reason; it never fails or stalls this drain.
|
||||
let reopenForAnswer = false;
|
||||
if (
|
||||
!shouldReopen &&
|
||||
currentIssue.status === "done" &&
|
||||
workingCandidate.requestedByActorType === "user" &&
|
||||
workingCandidate.agentId === currentIssue.assigneeAgentId
|
||||
) {
|
||||
const owed = await ports.transaction.owedInteractionAnswer({ companyId: run.companyId, issueId: currentIssue.id, payload: workingCandidate.payload });
|
||||
if (owed === "blocked") {
|
||||
await ports.transaction.cancelDeferredWake({
|
||||
companyId: run.companyId, wakeId: workingCandidate.id, agentId: workingCandidate.agentId,
|
||||
reason: "Queued answer cannot continue a task with unresolved blockers", now: input.now,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
if (owed === "deliver") shouldReopen = reopenForAnswer = true;
|
||||
}
|
||||
|
||||
// Agent continuations can outlive the work they addressed. Live,
|
||||
// non-self feedback with an explicit resume intent must still be read
|
||||
// after completion; it cannot revive a cancelled task. Other stale
|
||||
@@ -412,6 +437,7 @@ async function promoteDeferredWake(
|
||||
companyId: run.companyId,
|
||||
issueId: currentIssue.id,
|
||||
runId: run.id,
|
||||
...(reopenForAnswer ? { status: "in_progress" as const } : {}),
|
||||
});
|
||||
if (reopened) {
|
||||
postCommitEffects.push({
|
||||
|
||||
Reference in new issue
Block a user