mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 06:10:44 +01:00
fix(grok): avoid routine wakes for proactive admission (#16422)
## Thinking Path > - Paperclip lets people manage AI agents and their work. > - Grok Bot can request governed work from its own conversation. > - Managed Runner admission can take longer than the initial MCP response. > - The dispatcher treated a delayed offer as a reason to start another routine. > - The original conversation was already waiting for that same work. > - This pull request keeps admission in its original request and cancels old prepared echoes. > - Users get one task interaction while normal assigned-task wakes remain available. ## Linked Issues or Issue Description Refs #16219 and #16394. The duplicate search found no other proactive Grok correction. **What happened?** A real idle Grok conversation created one Paperclip task and reached accepted completion. During admission, the dispatcher also sent a routine webhook after the offer was ten seconds old. Two acceptance requests and two title writes were recorded. The extra routine can compete with the original conversation. **Expected behavior** Immediate and delayed Bot-initiated admission stay with the original request. The Bot checks the same request ID and arguments until admission succeeds or fails. A new routine must not echo that work back to the Bot. **Steps to reproduce** 1. Ask an idle Grok Bot to create and complete a Paperclip task. 2. Let managed admission return pending. 3. Leave its offered assignment unaccepted for more than ten seconds. 4. Run the dispatcher. It sends another routine webhook for that proactive offer. **Paperclip version or commit** Observed through the actual Grok client on Core11d69d04c7. Both new wake regressions also fail on current master before this correction. ## What Changed - Remove the delayed proactive-admission wake scan. - Fence prepared proactive wakes using the current binding and generation. A dispatcher restart cancels those old intents without sending them. - Tell Grok to keep checking its original queued request. Preserve Dot event instructions and normal assigned-task dispatch. - Add delayed-offer and recovered-intent regressions. Update the agent guide and release evidence plan. ## Verification - Two new network regressions fail before the fix. The corrected Grok and Dot suites pass all 80 checks. - The full focused Grok, Dot, MCP authorization/shutdown and question-delivery selection passes 214 checks across six files. - Repository typecheck and build pass. Token gates, the 2,731-story structural catalog and Storybook build pass. All these checks also pass after the branch was rebased onto master250429942e. - The real-client investigation created one task and one managed run, saved a 100-word report, reached canonical Done and recorded successful lease cleanup. Its exact public source marker was absent from the operator prompt, and no Paperclip proxy operation fetched it. The no-echo condition failed on that source; this PR still requires a deployed real-client repeat. - Public synthetic marker for the deployed repeat: `grok-native-read-8731cd8120b140829fbe476f6974fde8`. - Current-head CI passes with 55 successful checks and two optional Storybook skips. Greptile reviewede6b828af28at 5/5 with no findings. - The first workspace CI attempt hit a 5,000ms timeout in the unchanged snapshot-worker command-preparation test. All four variants passed locally; one targeted remote retry passed. The initial failure is retained and its root cause is not established. ## Risks Grok must keep checking the original request while admission is pending. If that conversation ends, the durable request remains available for reconciliation; Paperclip cannot infer provider idleness from a delay. Prepared intents from older dispatchers are cancelled by the authority guard. The real client must verify the polling path after deployment. No schema migration is needed. ## Model Used OpenAI GPT-6 in Codex, with reasoning, repository editing, shell tools and computer use. The exact serving model ID, snapshot and context window are not exposed by this session. ## 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
6acec4266e
commit
7f9d3d1929
6 files changed
+60
-23
No files matched your search
@@ -243,6 +243,9 @@ describe("durable Dot Runner integration", () => {
|
||||
try {
|
||||
const requests = await Promise.all([1, 2, 3].map(() => f.broker.requestTurn(f.principal, "Create hello and assign it to my owner", requestId)));
|
||||
expect(requests[1]).toEqual(requests[0]); expect(requests[2]).toEqual(requests[0]);
|
||||
expect(requests[0]!.instruction).toContain(provider === "grok_bot"
|
||||
? "Queued admission does not start another Grok routine"
|
||||
: "Events also notify queued work");
|
||||
const intake = await db.select().from(issues).where(eq(issues.id, requests[0]!.issueId));
|
||||
expect(intake).toHaveLength(1);
|
||||
expect(intake[0]).toMatchObject({ assigneeAgentId: f.agent.id, createdByAgentId: f.agent.id, responsibleUserId: f.userId });
|
||||
|
||||
@@ -289,6 +289,28 @@ describe("durable Grok routine qualification", () => {
|
||||
expect((await dotRunnerBroker(db).read(f.principal, work.id)).toolUsageInstructions).toContain("paperclip_grok_operation_status");
|
||||
});
|
||||
|
||||
it.each(["new offer", "prepared before restart"] as const)("does not echo delayed Bot-initiated work: %s", async scenario => {
|
||||
const f = await fixture(); const { work, run } = await admittedWork(f, "offered");
|
||||
await db.update(heartbeatRuns).set({ contextSnapshot: { grokProactiveRequest: {
|
||||
requestId: randomUUID(), bindingId: f.pairing.bindingId, bindingGeneration: 1,
|
||||
} } }).where(eq(heartbeatRuns.id, run.id));
|
||||
await db.update(dotRunnerAssignments).set({ createdAt: new Date(Date.now() - 60_000) }).where(eq(dotRunnerAssignments.id, work.id));
|
||||
const legacy = scenario === "prepared before restart" ? await f.service.reserve({
|
||||
companyId: f.company.id, agentId: f.agent.id, bindingId: f.pairing.bindingId,
|
||||
assignmentId: work.id, kind: "mailbox_updated", sourceEventId: `proactive-admission:${work.id}`,
|
||||
}) : null;
|
||||
f.delivery.mockClear(); const dispatcher = createGrokRoutineDispatcher(db, () => true, f.delivery);
|
||||
await dispatcher.tick(); await dispatcher.tick();
|
||||
expect(f.delivery).not.toHaveBeenCalled();
|
||||
const wakes = await db.select().from(grokBotDispatches).where(eq(grokBotDispatches.assignmentId, work.id));
|
||||
if (legacy) expect(wakes).toEqual([expect.objectContaining({ id: legacy.id, status: "cancelled" })]);
|
||||
else expect(wakes).toEqual([]);
|
||||
// The original conversation still owns and can accept the polled offer.
|
||||
await expect(dotRunnerBroker(db).read(f.principal, work.id)).resolves.toBeDefined();
|
||||
await (await acceptance(f, work))();
|
||||
expect((await db.select().from(dotRunnerAssignments).where(eq(dotRunnerAssignments.id, work.id)))[0]?.status).toBe("accepted");
|
||||
});
|
||||
|
||||
it("cancels a queued wake when its assignment was already accepted", async () => {
|
||||
const f = await fixture(); const { work } = await admittedWork(f, "offered");
|
||||
const queued = await f.service.reserve({ companyId: f.company.id, agentId: f.agent.id,
|
||||
|
||||
@@ -249,6 +249,16 @@ function createBroker(db: Db) {
|
||||
if (options.requireOffered && authority.assignment.status !== "offered") {
|
||||
throw fail("This work was already accepted. Read its feedback and operation receipts without another routine wake.");
|
||||
}
|
||||
if (options.requireOffered && authority.binding.providerKind === "grok_bot") {
|
||||
// A prepared intent from an older dispatcher must not start a second
|
||||
// interaction while the original caller is waiting for admission.
|
||||
const [run] = await db.select({ contextSnapshot: heartbeatRuns.contextSnapshot }).from(heartbeatRuns)
|
||||
.where(and(eq(heartbeatRuns.id, authority.assignment.runId), eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.agentId, agentId)));
|
||||
const proactive = run?.contextSnapshot?.grokProactiveRequest as Record<string, unknown> | undefined;
|
||||
if (proactive?.bindingId === authority.binding.id && proactive.bindingGeneration === authority.binding.generation) {
|
||||
throw fail("This Bot requested the work. Continue polling its original request instead of starting another routine.");
|
||||
}
|
||||
}
|
||||
return authority;
|
||||
},
|
||||
async authorizeBinding(principal: McpPrincipal, companyId: string, bindingId: string) {
|
||||
@@ -390,13 +400,16 @@ function createBroker(db: Db) {
|
||||
if (!assignment) await new Promise(resolve => setTimeout(resolve, 50));
|
||||
} while (!assignment && Date.now() < deadline);
|
||||
return { ...wake, issueId: task.id, identifier: task.identifier, assignment: assignment ?? null,
|
||||
instruction: wake.status === "admission_failed" ? wake.message : "Drain the inbox now. If assignment is present, read and accept it, then use its catalog. If admission is still pending, retry this provider’s request_turn tool with exactly the same requestId and prompt to check this intake; never create a second request. Events also notify queued work. This intake supplies normal run authority; no human needs to create a preliminary task." };
|
||||
instruction: wake.status === "admission_failed" ? wake.message : `Drain the inbox now. If assignment is present, read and accept it, then use its catalog. If admission is still pending, retry this provider’s request_turn tool with exactly the same requestId and prompt to check this intake; never create a second request. ${b.providerKind === "grok_bot" ? "Queued admission does not start another Grok routine. Keep checking this request from the current conversation until it is admitted or fails." : "Events also notify queued work."} This intake supplies normal run authority; no human needs to create a preliminary task.` };
|
||||
},
|
||||
async requestWork(principal: McpPrincipal, issueId: string, requestId: string) {
|
||||
const b = await principalBinding(principal);
|
||||
if (!z.uuid().safeParse(requestId).success) throw fail("Use a stable UUID requestId.");
|
||||
if (!await enabled(b.providerKind) || !(await this.bindingForAgent(b.companyId, b.agentId))?.wakeVerified) throw fail("External agent admission is unavailable.");
|
||||
const key = `dot-work:${b.id}:${b.generation}:${requestId}`;
|
||||
const admissionMessage = b.providerKind === "grok_bot"
|
||||
? "Normal admission determines when this task can run. Keep checking the original request with the same request ID and arguments, then read and accept its inbox assignment. Queued admission does not start another Grok routine."
|
||||
: "Normal admission determines when this task can run. Read the mailbox after its event.";
|
||||
const receipt = async () => (await db.select().from(agentWakeupRequests).where(and(eq(agentWakeupRequests.companyId, b.companyId), eq(agentWakeupRequests.agentId, b.agentId), eq(agentWakeupRequests.idempotencyKey, key))).limit(1))[0];
|
||||
const replay = async (row: typeof agentWakeupRequests.$inferSelect) => {
|
||||
if (row.payload?.issueId !== issueId) throw fail("requestId was reused for another task.");
|
||||
@@ -407,7 +420,7 @@ function createBroker(db: Db) {
|
||||
return { status: "admission_failed", runId: row.runId, admissionStatus: run.status,
|
||||
message: "The previous admission ended without an active assignment. Stop waiting for its mailbox event. Inspect the task's run and use normal task recovery after resolving the failure. This request ID retains its original receipt." };
|
||||
}
|
||||
return { status: "requested", runId: row.runId, message: "Normal admission determines when this task can run. Read the mailbox after its event." };
|
||||
return { status: "requested", runId: row.runId, message: admissionMessage };
|
||||
};
|
||||
// A retry is a receipt read, including after the task yielded or finished.
|
||||
const old = await receipt();
|
||||
@@ -424,7 +437,7 @@ function createBroker(db: Db) {
|
||||
payload: { issueId, dotRequestId: requestId }, contextSnapshot: { issueId, ...(b.providerKind === "grok_bot" ? { grokProactiveRequest: { requestId, bindingId: b.id, bindingGeneration: b.generation } } : {}) }, idempotencyKey: key, requestedByActorType: "agent", requestedByActorId: b.agentId,
|
||||
allowRunCoalescing: false, durableDotRequest: { id: receiptId, companyId: b.companyId, agentId: b.agentId, issueId, requestId, idempotencyKey: key, requestedAt: new Date() } });
|
||||
const reserved = await receipt();
|
||||
return reserved ? replay(reserved) : { status: "requested", runId: run?.id ?? null, message: "Normal admission determines when this task can run. Read the mailbox after its event." };
|
||||
return reserved ? replay(reserved) : { status: "requested", runId: run?.id ?? null, message: admissionMessage };
|
||||
} catch (error) {
|
||||
// A concurrent reservation can lose the PK race on a different task.
|
||||
// Replay only that durable receipt; do not mask unrelated failures.
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { and, asc, eq, isNull, lt, sql } from "drizzle-orm";
|
||||
import { grokBotDispatches, grokBotRoutines, dotAgentBindings, dotRunnerAssignments, heartbeatRuns, type Db } from "@paperclipai/db";
|
||||
import { and, asc, eq } from "drizzle-orm";
|
||||
import { grokBotDispatches, grokBotRoutines, type Db } from "@paperclipai/db";
|
||||
import { grokRoutineService } from "./routine.js";
|
||||
import type { deliverGrokRoutineWake } from "./webhook.js";
|
||||
import { externalBotEnabled } from "../external-bot-authority.js";
|
||||
@@ -14,17 +14,9 @@ export function createGrokRoutineDispatcher(db: Db, isBackgroundWorkEnabled: ()
|
||||
running = true;
|
||||
try {
|
||||
const service = grokRoutineService(db, deliver);
|
||||
// A proactive caller receives an immediate assignment without an echo.
|
||||
// Queued admission gets one delayed wake only while still unaccepted.
|
||||
const proactive = await db.select({ assignment: dotRunnerAssignments }).from(dotRunnerAssignments)
|
||||
.innerJoin(dotAgentBindings, eq(dotAgentBindings.id, dotRunnerAssignments.bindingId))
|
||||
.innerJoin(heartbeatRuns, eq(heartbeatRuns.id, dotRunnerAssignments.runId))
|
||||
.where(and(eq(dotAgentBindings.providerKind, "grok_bot"), isNull(dotAgentBindings.revokedAt), eq(dotRunnerAssignments.status, "offered"),
|
||||
lt(dotRunnerAssignments.createdAt, new Date(Date.now() - 10_000)), sql`${heartbeatRuns.contextSnapshot}->'grokProactiveRequest'->>'bindingId' = ${dotAgentBindings.id}::text`)).limit(10);
|
||||
for (const { assignment } of proactive) {
|
||||
try { await service.reserve({ companyId: assignment.companyId, agentId: assignment.agentId, bindingId: assignment.bindingId,
|
||||
assignmentId: assignment.id, kind: "mailbox_updated", sourceEventId: `proactive-admission:${assignment.id}` }); } catch { /* Current authority is rechecked on reservation and send. */ }
|
||||
}
|
||||
// Bot-initiated callers poll their durable request while admission is
|
||||
// pending. Delay does not prove that the originating conversation ended.
|
||||
// Starting a webhook here would create a competing Grok interaction.
|
||||
// An accepted session already has a routine handling it. Quiet MCP
|
||||
// traffic does not prove that routine ended. Keep feedback and receipts
|
||||
// durable for the Bot to read instead of starting overlapping work.
|
||||
|
||||
Reference in new issue
Block a user