mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 22:54:18 +01:00
fix(issues): preserve human handoffs from the active agent run (#16082)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tasks have one owner, and the server records assignment changes. > - An agent can finish its work by handing the task to a human for review. > - The assignment route stops the active run before it saves that handoff. > - When the caller is that run, a slow workspace restore can exceed the stop timeout and prevent the assignment from saving. > - Recovery can then mark the task Blocked while its comment says the handoff succeeded. > - This pull request lets the authenticated legacy run save a human handoff and finish its turn normally. > - Other actors and transfers to executable agents keep the verified stop requirement. ## Linked Issues or Issue Description Refs #13730. Related: #13782, #14700, and #15094. **What happened?** An agent posted its review handoff, then sent `PATCH /api/issues/{id}` to assign the task to its human creator with status `in_review`. The route cancelled that same run before saving the assignment. Workspace restore took 66 seconds. The stop limit is 60 seconds. The assignment never committed. Recovery marked the task Blocked, and it stayed assigned to the agent. **Expected behavior** The authenticated active legacy run can hand its own task to a human and finish normally. The human assignment and requested review status must save without waiting for workspace restore. Native runs, board reassignment, agent API keys, and transfers to another agent still require a verified stop. **Steps to reproduce** 1. Run a legacy agent on its assigned task with a signed run token. 2. Send a task PATCH from that run with a human `assigneeUserId`, a null `assigneeAgentId`, and status `in_review`. 3. Make stopping the sandbox take more than 60 seconds. Before this fix, the handoff fails before the task update. With this fix, the update saves and the run finishes normally. **Paperclip version or commit** Reproduced in hosted execution. The patch is based on master at `56a0bf0d492`. **Deployment mode** Hosted authenticated server with a legacy agent in a remote sandbox. The same route serves local deployments. PR #13782 proposes a broader exemption for all self-handoffs. This focused change limits the exemption to human recipients and legacy runs. It checks the exact active run, agent, company, and task, and it retains the stop requirement before an executable successor can take ownership. PRs #14700 and #15094 address later wake and recovery behavior after a reassignment stop. ## What Changed - Skip the reassignment cancellation only for an authenticated legacy run handing its own task to a human. - Reject later mutations from the outgoing run against its human-owned task, both in route authorization and under the update transaction locks. - Keep assignment permission checks, goal control, and queued human response protection. - Add route regressions for slow stops, active-run lookup, actor and runtime boundaries, recipient types, permission denials, and follow-up edits, completion, and reassignment. - Update the real HTTP low-trust handoff test to prove the run stays alive, its late edit fails, and the human wake receipt remains invalidated. - Add Postgres tests for saved assignment, former-owner rejection, and review state after the outgoing run succeeds or fails. - Document the handoff rule and update the task feature and agent access maps. ## Verification - 328 relevant tests passed across the route, issue service, low-trust HTTP, and legacy recovery suites. The route suite passed all 160 tests, and all 13 low-trust HTTP tests passed after updating the obsolete cancellation expectation. - Removing the production guard makes both slow-shutdown handoff regressions fail with HTTP 500. - `pnpm -r typecheck` passed. - `pnpm build` passed. - `node scripts/check-maps.mjs` passed. - `git diff --check` passed. - All 54 current-head checks passed; 2 optional Storybook checks were skipped. The CI lanes cover the full Vitest suite, typecheck/build, Runner checks, and product E2E checks. - Apex reviewed `c68fbfb356c3d80eb7136babb49940709a3c3459` at 5/5 with no findings; both earlier review threads are resolved. - The complete test lanes passed in CI. The duplicate local full `pnpm test:run` was stopped after CI completed; it did not finish locally. All 328 focused local tests and full local typecheck/build passed. ## Risks - The outgoing legacy turn stays alive after the human takes ownership. Route and transactional ownership checks reject its later mutations of the human-owned task. Read and comment access follows normal policy. Goal control still runs before the handoff. - This exemption does not apply to native runs or another agent as recipient. Those paths still stop the old execution before assignment. - This fix prevents future failed handoffs. It does not infer or replay old assignment requests. - There is no schema change or API shape change. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, and code execution. The session does not expose a more specific runtime model ID or context window size. ## 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e168512f7b
commit
6c198d616e
10 files changed
+299
-10
No files matched your search
@@ -29,6 +29,17 @@ An issue has at most one assignee.
|
||||
|
||||
This is a hard invariant. Paperclip is single-assignee by design.
|
||||
|
||||
A legacy agent run can hand its own task to a human without cancelling itself.
|
||||
The REST request must use the run's signed token and match the current agent,
|
||||
company, task, and active run. The assignment and requested status persist while
|
||||
the current turn finishes normally. This avoids losing the handoff when workspace
|
||||
restoration takes longer than the stop timeout. The former agent no longer owns
|
||||
the task. Its later task mutations are rejected, including under the update
|
||||
transaction's task and run locks. Read and comment access follows normal policy.
|
||||
|
||||
Board-initiated reassignments, agent API keys, native executions, and handoffs to
|
||||
another agent still require the outgoing execution to stop before ownership changes.
|
||||
|
||||
## 3. Status Semantics
|
||||
|
||||
Paperclip issue statuses are not just UI labels. They imply different expectations about ownership and execution.
|
||||
|
||||
@@ -71,7 +71,12 @@ Agent access: `partial` — Native set_task_title/reassign_task/set_dependencies
|
||||
for permitted non-lifecycle fields; REST.
|
||||
|
||||
Policy: Company/task visibility, assignment/change policy, active-run checkout ownership
|
||||
and lifecycle rules.
|
||||
and lifecycle rules. A legacy run can assign its own task to a human without
|
||||
cancelling itself only with its signed run token and matching active run, agent,
|
||||
company and task. The outgoing run cannot mutate its human-owned task afterward,
|
||||
including through a late update already admitted before the handoff. Agent keys,
|
||||
native runs, board reassignment and transfers to
|
||||
another agent retain verified-stop requirements.
|
||||
|
||||
Evidence: `source-only` — [implementation](../../server/src/routes/issues.ts) at the
|
||||
reviewed commit above.
|
||||
@@ -79,7 +84,10 @@ reviewed commit above.
|
||||
Verification: `not-run` — exercise a listed permitted operation with actual agent
|
||||
credentials/run context; inspect its result and applicable persistence/attribution.
|
||||
Assert the relevant denied actor, owner, company or mode from Policy. Review the
|
||||
remaining operations before qualifying the whole capability.
|
||||
remaining operations before qualifying the whole capability. Focused route
|
||||
regressions in [ownership tests](../../server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts)
|
||||
cover a human handoff and stop enforcement for other actors, runtimes and targets;
|
||||
they use mocked services and do not claim live execution proof.
|
||||
|
||||
Gaps: Native fallback blocks status/assignment/dependency policy fields. Dedicated tools
|
||||
differ in scope and supported fields.
|
||||
|
||||
@@ -55,6 +55,13 @@ Manual: Filter by status and assignee, change grouping/sort, open a task, and re
|
||||
|
||||
Automated: [task detail](../../ui/src/pages/IssueDetail.test.tsx) covers component behavior, while [issue service](../../server/src/__tests__/issues-service.test.ts) covers server contracts.
|
||||
|
||||
Manual: For a disposable task owned by a legacy agent, have its current run
|
||||
assign the task to a human with status In review. Verify that assignment and
|
||||
status persist, that the current run finishes without `issue_reassigned`, and
|
||||
that slow workspace restoration does not turn the task Blocked. Try a later edit
|
||||
from the outgoing run and verify rejection. Repeat a board
|
||||
reassignment and confirm that the outgoing run still stops.
|
||||
|
||||
Manual: Add a comment and edit authorized properties. Reload and verify history, actor, and values. Complete a disposable task and inspect its deliverable; reopen through an available action and confirm the next execution follows current policy.
|
||||
|
||||
## Gotchas
|
||||
|
||||
@@ -325,6 +325,7 @@ function createRunContextDb(
|
||||
runAgentOrRows: string | Record<string, unknown>[] = ownerAgentId,
|
||||
runId: string = ownerRunId,
|
||||
chatBindings: Array<{ id: string; companyId: string; issueId: string; state: string }> = [],
|
||||
emptyWakeQueue = false,
|
||||
) {
|
||||
const chatBindingQueries: ReturnType<PgDialect["sqlToQuery"]>[] = [];
|
||||
const runRows = Array.isArray(runAgentOrRows)
|
||||
@@ -382,6 +383,9 @@ function createRunContextDb(
|
||||
transaction: async (callback: (tx: typeof dbStub) => Promise<unknown>) => callback(dbStub),
|
||||
select: vi.fn((selection: Record<string, unknown> = {}) => ({
|
||||
from: vi.fn((table: Parameters<typeof getTableName>[0]) => {
|
||||
if (emptyWakeQueue && getTableName(table) === "agent_wakeup_requests") {
|
||||
return buildQuery({}, false, true);
|
||||
}
|
||||
if (getTableName(table) === "issue_thread_interactions") {
|
||||
return { where: vi.fn(() => ({ limit: vi.fn(async () => []) })) };
|
||||
}
|
||||
@@ -2688,6 +2692,149 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("human self-handoff", () => {
|
||||
const runningRun = {
|
||||
id: ownerRunId,
|
||||
companyId,
|
||||
agentId: ownerAgentId,
|
||||
runtimeMode: "legacy",
|
||||
status: "running",
|
||||
contextSnapshot: { issueId },
|
||||
};
|
||||
const handoff = {
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: "board-user",
|
||||
status: "in_review",
|
||||
};
|
||||
const handoffApp = (actor: Record<string, unknown>) => createApp(actor,
|
||||
createRunContextDb({}, ownerAgentId, ownerRunId, [], true));
|
||||
|
||||
beforeEach(() => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({
|
||||
status: "in_review",
|
||||
executionRunId: ownerRunId,
|
||||
}));
|
||||
mockHeartbeatService.getRun.mockResolvedValue(runningRun);
|
||||
mockHeartbeatService.cancelRun.mockResolvedValue({ ...runningRun, status: "cancelled" });
|
||||
mockAgentService.resolveByReference.mockImplementation(async (_companyId: string, id: string) => ({
|
||||
ambiguous: false,
|
||||
agent: await mockAgentService.getById(id),
|
||||
}));
|
||||
});
|
||||
|
||||
it.each(["in_progress", "in_review"])("persists a human handoff from %s without waiting for its own slow shutdown", async status => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status, executionRunId: ownerRunId }));
|
||||
mockHeartbeatService.cancelRun.mockRejectedValue(new Error("Execution is still stopping; termination has not been verified."));
|
||||
|
||||
const res = await request(handoffApp({ ...ownerActor(), source: "agent_jwt" }))
|
||||
.patch(`/api/issues/${issueId}`).send(handoff);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toMatchObject(handoff);
|
||||
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.update.mock.calls[0]?.[1]).toMatchObject({
|
||||
...handoff,
|
||||
actorRunId: ownerRunId,
|
||||
actorRunStopId: null,
|
||||
});
|
||||
expect(mockLogActivity.mock.calls.find(call => call[1].action === "issue.updated")?.[1]).toMatchObject({
|
||||
action: "issue.updated",
|
||||
runId: ownerRunId,
|
||||
details: handoff,
|
||||
});
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("recognizes the caller when the active run is resolved without an issue execution lock", async () => {
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ status: "in_review" }));
|
||||
mockHeartbeatService.getActiveRunForAgent.mockResolvedValue(runningRun);
|
||||
const res = await request(handoffApp({ ...ownerActor(), source: "agent_jwt" }))
|
||||
.patch(`/api/issues/${issueId}`).send(handoff);
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toMatchObject(handoff);
|
||||
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["edit", "complete", "reclaim"])("rejects the outgoing run's later %s after handing the task to a human", async action => {
|
||||
const app = handoffApp({ ...ownerActor(), source: "agent_jwt" });
|
||||
const handedOff = await request(app).patch(`/api/issues/${issueId}`).send(handoff);
|
||||
expect(handedOff.status, JSON.stringify(handedOff.body)).toBe(200);
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue(handoff));
|
||||
mockIssueService.update.mockClear();
|
||||
|
||||
const res = await request(app).patch(`/api/issues/${issueId}`).send(
|
||||
action === "edit" ? { title: "Late outgoing edit" } :
|
||||
action === "complete" ? { status: "done" } :
|
||||
{ assigneeAgentId: ownerAgentId, assigneeUserId: null },
|
||||
);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(res.body.details).toMatchObject({ code: "issue_handed_off" });
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps a queued human response with its current agent", async () => {
|
||||
const res = await request(createApp({ ...ownerActor(), source: "agent_jwt" }))
|
||||
.patch(`/api/issues/${issueId}`).send(handoff);
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(409);
|
||||
expect(res.body.code).toBe("interaction_response_queued");
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("retains assignment permission checks for a different human recipient", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action !== "tasks:assign",
|
||||
action: input.action,
|
||||
reason: "deny_policy_restricted",
|
||||
explanation: "Assignment requires approval.",
|
||||
}));
|
||||
const res = await request(handoffApp({ ...ownerActor(), source: "agent_jwt" }))
|
||||
.patch(`/api/issues/${issueId}`).send({ ...handoff, assigneeUserId: "another-user" });
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(403);
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["board", "agent_key", "other_run", "native", "other_issue", "other_agent", "other_company"])(
|
||||
"still requires a verified stop for %s reassignment", async scenario => {
|
||||
let actor: Record<string, unknown> = { ...ownerActor(), source: "agent_jwt" };
|
||||
if (scenario === "board") actor = boardActor();
|
||||
if (scenario === "agent_key") actor = ownerActor();
|
||||
const run = {
|
||||
...runningRun,
|
||||
...(scenario === "other_run" ? { id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" } : {}),
|
||||
...(scenario === "native" ? { runtimeMode: "native" } : {}),
|
||||
...(scenario === "other_issue" ? { contextSnapshot: { issueId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" } } : {}),
|
||||
...(scenario === "other_agent" ? { agentId: peerAgentId } : {}),
|
||||
...(scenario === "other_company" ? { companyId: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa" } : {}),
|
||||
};
|
||||
mockHeartbeatService.getRun.mockResolvedValue(run);
|
||||
mockHeartbeatService.cancelRun.mockRejectedValue(new Error("Execution is still stopping; termination has not been verified."));
|
||||
|
||||
const res = await request(handoffApp(actor)).patch(`/api/issues/${issueId}`).send(handoff);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(500);
|
||||
expect(mockHeartbeatService.cancelRun).toHaveBeenCalledWith(run.id, "Cancelled before issue reassignment",
|
||||
expect.objectContaining({ errorCode: "issue_reassigned" }));
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["agent", "unassigned"])("still stops the outgoing run before an %s handoff", async target => {
|
||||
const res = await request(handoffApp({ ...ownerActor(), source: "agent_jwt" }))
|
||||
.patch(`/api/issues/${issueId}`).send({
|
||||
assigneeAgentId: target === "agent" ? peerAgentId : null,
|
||||
assigneeUserId: null,
|
||||
});
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(mockHeartbeatService.cancelRun).toHaveBeenCalledTimes(1);
|
||||
expect(mockHeartbeatService.cancelRun.mock.invocationCallOrder[0])
|
||||
.toBeLessThan(mockIssueService.update.mock.invocationCallOrder[0]!);
|
||||
});
|
||||
});
|
||||
|
||||
describe("task watchdog scope grants", () => {
|
||||
const watchdogRunId = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaab";
|
||||
const watchdogReportIssueId = "cccccccc-cccc-4ccc-8ccc-cccccccccccd";
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
issueAccessGrants,
|
||||
issuePlanDecompositions,
|
||||
issueReadStates,
|
||||
issueRecoveryActions,
|
||||
issueRelations,
|
||||
issueThreadInteractions,
|
||||
issueWorkProducts,
|
||||
@@ -46,6 +47,8 @@ import {
|
||||
readIssueCommentRunLogText,
|
||||
} from "../services/issues.ts";
|
||||
import { getRunLogStore } from "../services/run-log-store.js";
|
||||
import { terminalizeLegacyExecution } from "../services/legacy-execution-recovery.js";
|
||||
import { settleUnrecoverableExecutions } from "../services/execution-recovery-resolution.js";
|
||||
import {
|
||||
WORKSPACE_WORKTREE_REQUIRES_PROJECT_CODE,
|
||||
WORKSPACE_WORKTREE_REQUIRES_PROJECT_MESSAGE,
|
||||
@@ -7048,6 +7051,7 @@ describeEmbeddedPostgres("issueService.assertCheckoutOwner stale checkout adopti
|
||||
await db.delete(goals);
|
||||
await db.delete(agents);
|
||||
await db.delete(instanceSettings);
|
||||
await db.delete(companyMemberships);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
@@ -7136,6 +7140,67 @@ describeEmbeddedPostgres("issueService.assertCheckoutOwner stale checkout adopti
|
||||
return { issueId, assigneeAgentId, actorAgentId, staleRunId, actorRunId };
|
||||
}
|
||||
|
||||
it.each(["succeeded", "failed"])("preserves a human handoff when the outgoing legacy run later %s", async status => {
|
||||
const seeded = await seedOwnershipIssue({ checkoutStatus: "failed" });
|
||||
const original = await svc.getById(seeded.issueId);
|
||||
await db.insert(companyMemberships).values({
|
||||
companyId: original!.companyId,
|
||||
principalType: "user",
|
||||
principalId: "handoff-reviewer",
|
||||
status: "active",
|
||||
});
|
||||
await db.update(heartbeatRuns).set({
|
||||
contextSnapshot: { issueId: seeded.issueId },
|
||||
}).where(eq(heartbeatRuns.id, seeded.actorRunId));
|
||||
await svc.assertCheckoutOwner(seeded.issueId, seeded.actorAgentId, seeded.actorRunId);
|
||||
|
||||
const handoff = await svc.update(seeded.issueId, {
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: "handoff-reviewer",
|
||||
status: "in_review",
|
||||
actorAgentId: seeded.actorAgentId,
|
||||
actorRunId: seeded.actorRunId,
|
||||
});
|
||||
expect(handoff).toMatchObject({
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: "handoff-reviewer",
|
||||
status: "in_review",
|
||||
checkoutRunId: null,
|
||||
executionRunId: null,
|
||||
});
|
||||
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, seeded.actorRunId));
|
||||
expect(run!.status).toBe("running");
|
||||
await expect(svc.assertCheckoutOwner(seeded.issueId, seeded.actorAgentId, seeded.actorRunId))
|
||||
.rejects.toMatchObject({ status: 409 });
|
||||
await expect(svc.update(seeded.issueId, {
|
||||
title: "Late outgoing edit",
|
||||
actorAgentId: seeded.actorAgentId,
|
||||
actorRunId: seeded.actorRunId,
|
||||
})).rejects.toMatchObject({ status: 403, details: { code: "issue_handed_off" } });
|
||||
|
||||
await terminalizeLegacyExecution({
|
||||
db,
|
||||
run: run!,
|
||||
status,
|
||||
fromStatuses: ["running"],
|
||||
reconcileIfNeeded: true,
|
||||
patch: { finishedAt: new Date(), ...(status === "failed" ? { errorCode: "process_lost" } : {}) },
|
||||
});
|
||||
await settleUnrecoverableExecutions(db);
|
||||
await expect(svc.update(seeded.issueId, {
|
||||
status: "done",
|
||||
actorAgentId: seeded.actorAgentId,
|
||||
actorRunId: seeded.actorRunId,
|
||||
})).rejects.toMatchObject({ status: 403, details: { code: "issue_handed_off" } });
|
||||
expect(await svc.getById(seeded.issueId)).toMatchObject({
|
||||
status: "in_review",
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: "handoff-reviewer",
|
||||
});
|
||||
expect(await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, seeded.issueId)))
|
||||
.toHaveLength(0);
|
||||
});
|
||||
|
||||
it("lets the current assignee adopt a stale terminal checkout owner", async () => {
|
||||
const seeded = await seedOwnershipIssue({ checkoutStatus: "failed" });
|
||||
|
||||
|
||||
@@ -977,14 +977,19 @@ describeEmbeddedPostgres(
|
||||
const handoff = await request(directedAgent).patch(`/api/issues/${created.body.id}`)
|
||||
.send({ status: "in_review", assigneeAgentId: null, assigneeUserId: "board-user", comment: "Ready for human review" });
|
||||
expect(handoff.status, JSON.stringify(handoff.body)).toBe(200);
|
||||
const [stopped] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, directRun.id));
|
||||
expect(stopped).toMatchObject({ status: "cancelled", errorCode: "issue_reassigned" });
|
||||
expect(handoff.body).toMatchObject({ status: "in_review", assigneeAgentId: null, assigneeUserId: "board-user" });
|
||||
const [outgoing] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, directRun.id));
|
||||
expect(outgoing).toMatchObject({ status: "running", errorCode: null });
|
||||
const lateEdit = await request(directedAgent).patch(`/api/issues/${created.body.id}`)
|
||||
.send({ description: "Late outgoing edit" });
|
||||
expect([403, 404]).toContain(lateEdit.status);
|
||||
const returned = await request(board).patch(`/api/issues/${created.body.id}`)
|
||||
.send({ status: "backlog", assigneeAgentId: fixture.agents.lowTrust.id, assigneeUserId: null });
|
||||
expect(returned.status, JSON.stringify(returned.body)).toBe(200);
|
||||
// Reassignment back does not revive the cancelled run's human request.
|
||||
const staleRead = await request(directedAgent).get(`/api/issues/${created.body.id}`);
|
||||
expect([403, 404]).toContain(staleRead.status);
|
||||
const [humanRequest] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, wake.id));
|
||||
expect(humanRequest.status).toBe("cancelled");
|
||||
const reassignedRead = await request(directedAgent).get(`/api/issues/${created.body.id}`);
|
||||
expect(reassignedRead.status, JSON.stringify(reassignedRead.body)).toBe(200);
|
||||
|
||||
});
|
||||
|
||||
|
||||
@@ -19,7 +19,9 @@ export async function assertAgentRunWriteAllowed(tx: Db, companyId: string, acto
|
||||
stopId?: string | null;
|
||||
}) {
|
||||
if (!actor.agentId || !actor.runId) return;
|
||||
const [run] = await tx.select({ status: heartbeatRuns.status, resultJson: heartbeatRuns.resultJson })
|
||||
const [run] = await tx.select({ status: heartbeatRuns.status, resultJson: heartbeatRuns.resultJson,
|
||||
runtimeMode: heartbeatRuns.runtimeMode, companyId: heartbeatRuns.companyId,
|
||||
contextSnapshot: heartbeatRuns.contextSnapshot })
|
||||
.from(heartbeatRuns).where(and(eq(heartbeatRuns.id, actor.runId),
|
||||
eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.agentId, actor.agentId)))
|
||||
.for("share");
|
||||
@@ -28,4 +30,5 @@ export async function assertAgentRunWriteAllowed(tx: Db, companyId: string, acto
|
||||
if (agentRunWritesRevoked(run) && !stoppedForThisMutation) {
|
||||
throw forbidden("This run was cancelled", { code: "agent_run_cancelled" });
|
||||
}
|
||||
return run;
|
||||
}
|
||||
@@ -0,0 +1,24 @@
|
||||
import { forbidden } from "./errors.js";
|
||||
|
||||
export function assertLegacyRunTaskOwnership(
|
||||
run: {
|
||||
companyId: string;
|
||||
runtimeMode: string;
|
||||
contextSnapshot?: Record<string, unknown> | null;
|
||||
} | null | undefined,
|
||||
issue: {
|
||||
id: string;
|
||||
companyId: string;
|
||||
assigneeAgentId: string | null;
|
||||
assigneeUserId: string | null;
|
||||
},
|
||||
) {
|
||||
if (run?.runtimeMode === "legacy" && run.companyId === issue.companyId &&
|
||||
run.contextSnapshot?.issueId === issue.id &&
|
||||
issue.assigneeAgentId === null && issue.assigneeUserId) {
|
||||
throw forbidden("This task is assigned to a human; the outgoing run can no longer change it", {
|
||||
code: "issue_handed_off",
|
||||
issueId: issue.id,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
import { parseObject } from "../adapters/utils.js";
|
||||
import { assertLegacyRunTaskOwnership } from "../legacy-run-task-ownership.js";
|
||||
import { enqueueFastResponse } from "../services/fast-responses.js";
|
||||
import { hasRequiredWorkspaceRecovery, LEGACY_WORKSPACE_RECOVERY_SCHEMA } from "../services/workspace-restore-recovery-state.js";
|
||||
import { hasUnverifiedNativeWorkspaceFinalizationOwner } from "../services/native-runtime/native-workspace-finalization-recovery-state.js";
|
||||
@@ -5632,6 +5633,11 @@ export function issueRoutes(
|
||||
res.status(403).json({ error: "Agent authentication required" });
|
||||
return false;
|
||||
}
|
||||
if (req.actor.source === "agent_jwt" && req.actor.runId &&
|
||||
issue.assigneeAgentId === null && issue.assigneeUserId) {
|
||||
const run = await heartbeat.getRun(req.actor.runId);
|
||||
if (run?.agentId === actorAgentId) assertLegacyRunTaskOwnership(run, issue);
|
||||
}
|
||||
// Task-watchdog runs receive a scoped *grant* to mutate issues inside the
|
||||
// watched subtree. This must be evaluated before the base assignee-ownership
|
||||
// boundary below: that boundary denies an agent mutating an issue owned by a
|
||||
@@ -14241,7 +14247,18 @@ export function issueRoutes(
|
||||
agentId: existing.assigneeAgentId,
|
||||
});
|
||||
const runToStopForReassignment = await resolveActiveIssueRun(existing);
|
||||
if (runToStopForReassignment) {
|
||||
const isHumanSelfHandoff =
|
||||
req.actor.type === "agent" &&
|
||||
req.actor.source === "agent_jwt" &&
|
||||
nextAssigneeAgentId === null &&
|
||||
typeof nextAssigneeUserId === "string" &&
|
||||
req.actor.agentId === existing.assigneeAgentId &&
|
||||
runToStopForReassignment?.runtimeMode === "legacy" &&
|
||||
runToStopForReassignment.id === req.actor.runId &&
|
||||
runToStopForReassignment.agentId === req.actor.agentId &&
|
||||
runToStopForReassignment.companyId === existing.companyId &&
|
||||
runToStopForReassignment.contextSnapshot?.issueId === existing.id;
|
||||
if (runToStopForReassignment && !isHumanSelfHandoff) {
|
||||
const cancelled = await heartbeat.cancelRun(
|
||||
runToStopForReassignment.id,
|
||||
"Cancelled before issue reassignment",
|
||||
|
||||
@@ -7,6 +7,7 @@ import { notifyChatPublicationWork } from "./chat-work-notifications.js";
|
||||
import { recordChatHandoff, recordChatCompletion, existingChatCompletionReply, acknowledgeChatCompletionReply } from "./chat-completion-delivery.js";
|
||||
import { mirrorSlackBoardComment, slackBoardReplyBindings } from "./slack-board-messages.js";
|
||||
import { assertAgentRunWriteAllowed } from "../agent-run-cancellation.js";
|
||||
import { assertLegacyRunTaskOwnership } from "../legacy-run-task-ownership.js";
|
||||
import { retryIdempotentDatabaseOperation } from "../database-retry.js";
|
||||
import { externalConversationStateSql, nonIdleSlackIssueCondition, resumeSlackConversation } from "./slack-conversation-state.js";
|
||||
import { documentService } from "./documents.js";
|
||||
@@ -11194,9 +11195,10 @@ export function issueService(db: Db) {
|
||||
if (actorAgentId && actorRunId) {
|
||||
// Recheck under a run lock: a request admitted before Stop must not
|
||||
// commit a late Done after cancellation revoked its credentials.
|
||||
await assertAgentRunWriteAllowed(tx, receiptExisting.companyId, {
|
||||
const actorRun = await assertAgentRunWriteAllowed(tx, receiptExisting.companyId, {
|
||||
agentId: actorAgentId, runId: actorRunId, stopId: actorRunStopId,
|
||||
});
|
||||
assertLegacyRunTaskOwnership(actorRun, receiptExisting);
|
||||
}
|
||||
if (actorAgentId && patch.status === "done") {
|
||||
const [review] = await tx.select({ id: toolActionRequests.id }).from(toolActionRequests).where(and(eq(toolActionRequests.companyId, existing.companyId), eq(toolActionRequests.issueId, id), inArray(toolActionRequests.status, ["pending", "approved", "executing"]))).limit(1);
|
||||
|
||||
Reference in new issue
Block a user