mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(recovery): count a current response-wake wait as a visible next action in I7 (#20994)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The ownership reconciler checks invariant I7: an agent-assigned `in_progress` issue must have a visible next action. A live run, a pending wake, an open recovery action or a saved monitor deadline counts. > - #20935 added a parked wait. When a native agent yields `response_wake` and no response source is admitted, the arbiter commits `response_wake_waiting_for_input`. The task stays `in_progress`, and the run shows "Waiting for a reply". > - I7 did not know this wait. A task that really waits for the board was reported as `no_owner_no_wake`. > - This pull request makes I7 count that wait as a visible next action while it is still current. I7 still reports the issue when the wait is stale. > - The benefit is that a genuinely parked task is not reported as a silent stall, and a stale wait is still reported. ## Linked Issues or Issue Description **What happened?** The ownership reconciler reported `I7_visible_next_action` / `no_owner_no_wake` for an issue whose latest applied decision was `response_wake_waiting_for_input`. That is the visible "Waiting for a reply" state from #20935. **Expected behavior** A current, committed response-wake wait is a visible next action. The issue is reported again when a user comment arrives after the waiting run started, or when that decision is no longer the issue's latest decision. **Steps to reproduce** 1. Commit a native run whose arbiter decision is `response_wake_waiting_for_input`, and make it the issue's latest decision. 2. Wait past the 5-minute next-action grace window. 3. Run an ownership reconciliation pass. Before this change, I7 reports the issue. **Additional context** Follow-up to #20989 and #20935. The conditions match `hasCommittedNativeResponseWakeWait`, which recovery already uses for the same wait. Refs #20935 Refs #20989 ## What Changed - `server/src/services/recovery/ownership-invariant-report.ts`: `checkVisibleNextAction` excludes an issue with a parked response-wake wait. The wait must meet all of these conditions: - The issue's `lastStatusDecisionId` is an applied `response_wake_waiting_for_input` decision. - Its native finalization is `committed`. - Its run succeeded and belongs to the current agent assignee. - The issue has no user assignee and is not hidden. - No user comment arrived after that run started. - New test in `server/src/__tests__/ownership-reconciliation-report.test.ts`: - A current wait is not reported. - A later user comment makes I7 report the issue. - A decision that is no longer current also makes I7 report it. ## Verification - Red on master: `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts -t "response-wake wait"`. I7 reported the waiting issue. - Green: the full `ownership-reconciliation-report.test.ts` and `environment-pending-cleanup-queue-owner.test.ts` pass (43 tests). `npx tsc --noEmit -p server` passes. - Restart-chaos harness, for reviewers: the harness U1/R1 grading should treat a sample as waiting, not ownerless, when the issue's latest run shows the execution projection phase `waiting_for_answer` ("Waiting for a reply"). It should still fail when a user comment is newer than that run. The harness reads I7 through `tests/restart-chaos/invariants.ts`, so I7 changes with this PR. The U1 sampler does not read the projection yet, and this PR does not change it. ## Risks - Low risk. The change is in a report-only check. It writes nothing and changes no admission, status or recovery path. - If the conditions are too wide, a stalled task is hidden. The conditions are the same as recovery's durable-wait check, and the test proves that a stale wait is still reported. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, local 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 - [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
c4927e1ec2
commit
ad2d123dad
2 files changed
+61
No files matched your search
@@ -10,6 +10,7 @@ import {
|
||||
agents,
|
||||
budgetPolicies,
|
||||
companies,
|
||||
completionContracts,
|
||||
createDb,
|
||||
environmentLeases,
|
||||
environments,
|
||||
@@ -19,7 +20,10 @@ import {
|
||||
issueComments,
|
||||
issues,
|
||||
nativeRunFinalizations,
|
||||
nativeRunResults,
|
||||
sandboxCreationOperations,
|
||||
statusDecisions,
|
||||
workAssessments,
|
||||
workspaceOperations,
|
||||
} from "@paperclipai/db";
|
||||
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
@@ -959,6 +963,41 @@ describe("report-only ownership reconciliation", () => {
|
||||
expect(report.violations.filter(violation => violation.rule === "I7_visible_next_action")).toEqual([]);
|
||||
});
|
||||
|
||||
it("treats a current committed response-wake wait as a next action until a user answers it", async () => {
|
||||
const issueId = randomUUID(), runId = randomUUID(), contractId = randomUUID(), resultId = randomUUID();
|
||||
const assessmentId = randomUUID(), decisionId = randomUUID(), startedAt = minutesAgo(40);
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Waiting for the board", status: "in_progress",
|
||||
assigneeAgentId: agentId, updatedAt: minutesAgo(30) });
|
||||
await db.insert(completionContracts).values({ id: contractId, companyId, issueId, revision: 1,
|
||||
schemaVersion: "paperclip.completion-contract.v1", policyVersion: "test", risk: "standard",
|
||||
completionAuthority: "server_arbiter", incompleteCriteriaPolicy: "preserve_non_terminal",
|
||||
contractJson: { revision: "test", objective: "Wait", criteria: [{ id: "objective", requirement: "Wait" }] },
|
||||
canonicalSha256: contractId, createdByActorType: "system", createdByActorId: "test" });
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, runtimeMode: "native", status: "succeeded",
|
||||
nativeIssueId: issueId, nativeSessionId: runId, completionContractId: contractId, completionContractSha256: contractId,
|
||||
contextSnapshot: { issueId }, startedAt, finishedAt: minutesAgo(35) });
|
||||
await db.insert(nativeRunResults).values({ id: resultId, companyId, issueId, runId, completionContractId: contractId,
|
||||
serverFingerprint: "test", schemaStatus: "accepted", resultJson: {}, canonicalSha256: resultId });
|
||||
await db.insert(workAssessments).values({ id: assessmentId, companyId, issueId, runId, contractId, resultId,
|
||||
triggerKind: "turn_finished", triggerActorCompanyId: companyId, priorIssueStatus: "in_progress", priorStatusVersion: 1,
|
||||
policyVersion: "test", assessmentJson: {}, inputDigest: assessmentId });
|
||||
await db.insert(statusDecisions).values({ id: decisionId, companyId, issueId, runId, assessmentId, decisionVersion: 1,
|
||||
policyVersion: "test", fromStatus: "in_progress", toStatus: "in_progress", reasonCode: "response_wake_waiting_for_input",
|
||||
decisionJson: {}, decisionDigest: decisionId, applicationState: "applied", appliedAt: minutesAgo(35) });
|
||||
await db.insert(nativeRunFinalizations).values({ companyId, issueId, runId, phase: "committed", resultId, assessmentId, decisionId });
|
||||
await db.update(issues).set({ lastStatusDecisionId: decisionId, updatedAt: minutesAgo(30) }).where(eq(issues.id, issueId));
|
||||
const visible = async () => (await runOwnershipReconciliationPass(db, { state: createOwnershipReportState(), record: false }))
|
||||
.violations.filter(violation => violation.rule === "I7_visible_next_action").map(violation => violation.recordId);
|
||||
|
||||
expect(await visible()).toEqual([]);
|
||||
await db.insert(issueComments).values({ companyId, issueId, authorType: "user", authorUserId: "board",
|
||||
body: "Go ahead.", createdAt: minutesAgo(20) });
|
||||
expect(await visible()).toEqual([issueId]);
|
||||
await db.delete(issueComments).where(eq(issueComments.issueId, issueId));
|
||||
await db.update(issues).set({ lastStatusDecisionId: null, updatedAt: minutesAgo(30) }).where(eq(issues.id, issueId));
|
||||
expect(await visible()).toEqual([issueId]);
|
||||
});
|
||||
|
||||
it("returns the end of the next-action grace window for an issue still inside it", async () => {
|
||||
const updatedAt = minutesAgo(2);
|
||||
await db.insert(issues).values({ companyId, title: "Fresh", status: "in_progress", assigneeAgentId: agentId, updatedAt });
|
||||
|
||||
@@ -2,10 +2,14 @@ import { and, asc, eq, inArray, isNotNull, isNull, lt, notExists, or, sql, type
|
||||
import {
|
||||
agentWakeupRequests,
|
||||
heartbeatRuns,
|
||||
issueComments,
|
||||
issueRecoveryActions,
|
||||
issues,
|
||||
nativeRunFinalizations,
|
||||
statusDecisions,
|
||||
type Db,
|
||||
} from "@paperclipai/db";
|
||||
import { NATIVE_RESPONSE_WAKE_WAITING_REASON } from "../native-runtime/status-arbiter.js";
|
||||
import { taskClosedByAgentSql, TEMPORARY_REFUSAL_TRIGGERS } from "../work-intent-outbox.js";
|
||||
import { deliveredQueuedCommentIds } from "../queued-comment-delivery.js";
|
||||
import { logger } from "../../middleware/logger.js";
|
||||
@@ -579,6 +583,23 @@ async function checkExactlyOnceIntents(db: Db, state: OwnershipReportState, now:
|
||||
return violations;
|
||||
}
|
||||
|
||||
function parkedResponseWakeWait(db: Db) {
|
||||
return db.select({ one: sql`1` }).from(statusDecisions)
|
||||
.innerJoin(nativeRunFinalizations, and(eq(nativeRunFinalizations.runId, statusDecisions.runId),
|
||||
eq(nativeRunFinalizations.companyId, statusDecisions.companyId), eq(nativeRunFinalizations.issueId, statusDecisions.issueId),
|
||||
eq(nativeRunFinalizations.decisionId, statusDecisions.id), eq(nativeRunFinalizations.phase, "committed")))
|
||||
.innerJoin(heartbeatRuns, and(eq(heartbeatRuns.id, statusDecisions.runId), eq(heartbeatRuns.companyId, statusDecisions.companyId),
|
||||
eq(heartbeatRuns.nativeIssueId, statusDecisions.issueId), eq(heartbeatRuns.agentId, issues.assigneeAgentId),
|
||||
eq(heartbeatRuns.runtimeMode, "native"), eq(heartbeatRuns.status, "succeeded"), isNotNull(heartbeatRuns.startedAt)))
|
||||
.where(and(eq(statusDecisions.id, issues.lastStatusDecisionId), eq(statusDecisions.companyId, issues.companyId),
|
||||
eq(statusDecisions.issueId, issues.id), eq(statusDecisions.applicationState, "applied"),
|
||||
eq(statusDecisions.reasonCode, NATIVE_RESPONSE_WAKE_WAITING_REASON), isNull(issues.assigneeUserId), isNull(issues.hiddenAt),
|
||||
notExists(db.select({ one: sql`1` }).from(issueComments).where(and(
|
||||
eq(issueComments.companyId, issues.companyId), eq(issueComments.issueId, issues.id),
|
||||
eq(issueComments.authorType, "user"), isNull(issueComments.createdByRunId), isNull(issueComments.deletedAt),
|
||||
sql`${issueComments.createdAt} > ${heartbeatRuns.startedAt}`)))));
|
||||
}
|
||||
|
||||
/** An agent-assigned in-progress issue with no live run, no pending wake and
|
||||
* no open recovery action has no next action (invariant 7). Issues still in
|
||||
* their grace window return the time they become checkable. */
|
||||
@@ -600,6 +621,7 @@ async function checkVisibleNextAction(db: Db, now: Date, limit: number): Promise
|
||||
notExists(db.select({ one: sql`1` }).from(issueRecoveryActions).where(and(
|
||||
eq(issueRecoveryActions.companyId, issues.companyId), eq(issueRecoveryActions.sourceIssueId, issues.id),
|
||||
isNull(issueRecoveryActions.resolvedAt)))),
|
||||
notExists(parkedResponseWakeWait(db)),
|
||||
)).orderBy(asc(issues.updatedAt), asc(issues.id)).limit(limit).offset(offset);
|
||||
rows.push(...page);
|
||||
if (page.length < limit) break;
|
||||
|
||||
Reference in new issue
Block a user