mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 16:35:48 +01:00
fix(github): keep one main PR review comment and reconcile stopped work (#16901)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub connections send review requests to an assigned agent. > - A PR needs one main comment that shows its current review state. > - The response ledger used one comment per incoming request. Pushes and follow-up messages created more placeholders. > - Thread subscriptions also accepted requests addressed to external bots. Recovery could leave an acknowledged request without a final notice. > - This pull request shares the main PR comment, filters other recipients, and reconciles stopped work. ## Linked Issues or Issue Description Related: #16874 showed the repeated placeholders. #15416 introduced the per-request response path. The prior summary path used one PR-level marker. **What happened?** Repeated pushes created new main PR comments. A subscribed `@greptile review` message also woke the Paperclip bot. If execution stopped before staging a milestone, its comment could keep claiming that work was active. **Expected behavior** One bot keeps one main comment per PR. Progress and review results update it. A message addressed to another recipient does not wake the bot unless it also explicitly mentions this bot. Stopped work gets an accurate notice. **Steps to reproduce** 1. Accept two review requests in one PR conversation. 2. Add `@greptile review` to that subscribed conversation. 3. Fail the assigned run before it stages a milestone. Run channel maintenance. ## What Changed - Add a durable main PR response target. Keep per-request receipts and the existing publication lease. - Recover the target from App-owned provider markers. Adopt older placeholders and point verified duplicates to the current review. - Use a queued notice before a PR run starts. Preserve version ordering and final responses. - Filter leading mentions addressed to another recipient. Explicit mentions of this bot take precedence. Incidental mentions keep normal thread behavior. - Reconcile acknowledged requests after terminal execution. Show paused state for saved requests on a blocked task. Preserve newer work and confirmed or uncertain tool delivery. - Keep a request open when its stale acknowledgement is suppressed. Close placeholders after replies published elsewhere. Retire old coalesced placeholders before replacing their receipts. Resolve native request identity when legacy wake fields are absent. - Update guidance, connection documentation, and feature/access maps. ## Verification - Red: all three behavioral regressions failed on the original code. A second review created a second comment; an external-bot request was processed; stopped execution retained its working placeholder. - Green: the same three cases passed after the fixes. - Review regressions also failed before their repairs: a delayed acknowledgement closed a new request; normal and stored formal-review receipts left a live placeholder; coalesced completion lost a duplicate receipt; native terminal work without legacy wake fields stayed queued. All 13 combined regression and safety cases passed after repair. - The same legacy receipt-loss bug was also reproduced in the superseded-acknowledgement branch. Its four affected cases passed after adding retirement before that receipt replacement. - A further review regression reproduced paused requests rejecting resumed progress and skipping stopped-run cleanup. Four cases failed before repair; all eight resume and neighboring safety cases passed afterward. Both paused-notice branches keep the request open. - Repeated paused maintenance reproduced two needless GitHub edits. The writer now skips an identical complete comment body. Nine affected cases passed, covering repeated maintenance, resumption, stopped work, final assessment preservation, and main-comment reuse. - Safety cases cover lost receipts, legacy adoption, newer progress, published assessments, provider-verified mention aliases, and old replies that must not complete new requests. - `pnpm -r typecheck`, `pnpm build`, map validation, and module boundaries passed. The server TypeScript check passed after the final repair. The broader local GitHub workflow and policy run passed 188 tests. Full CI passed on the final commit (`554979d79285b68bcbf90cd639e3d2d053f4ea4b`), including all three chat shards, server/workspace/Runner suites, typecheck, build, and E2E gates. Greptile is 5/5 with no open review findings. One serialized job received a runner shutdown signal mid-test; its single retry passed. - Preserved failure evidence: the first CI run exposed an overbroad recipient filter and an old queued-text assertion. The next commit passed all three CI chat shards. Greptile then found the receipt transitions covered by the new regressions above. Local full-suite runs were stopped when those defects required source changes; no clean full local pass is claimed. - Tests use real PostgreSQL and controlled provider responses. They do not establish a live GitHub App rollout. The original instance needs the fixed server before its comments change. ## Risks - Leading mentions now select a recipient even when that account has no Paperclip connection. A leading human mention also selects that person. Mid-message incidental mentions retain normal subscription behavior. - Existing comment adoption edits only comments proven by this endpoint's request ledger or prior summary marker and App identity. Deleted and replaced comments remain protected. - Failure cleanup uses current authority and does not replace newer progress or unresolved provider writes. It does not restart failed execution. - No schema migration is required. ## Model Used OpenAI Codex, GPT-6. The exact serving model ID and context-window size are not exposed in this session. The model used reasoning, tools, code execution, and browser inspection. ## 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
16689de291
commit
3b8324a233
10 files changed
+544
-71
No files matched your search
+20
-10
@@ -241,7 +241,7 @@ recovery of the existing App instead of a second creation. Direct webhook and
|
||||
manual existing-App recovery remain available. Connecting does not prove that
|
||||
an agent runtime can execute a review.
|
||||
|
||||
### One editable response per request
|
||||
### One main response per pull request
|
||||
|
||||
A pull request task can retain both its main discussion and inline finding
|
||||
threads. Bot tool discovery lists each endpoint/catalog tool once across those
|
||||
@@ -249,10 +249,13 @@ threads. Publishing an inline finding therefore does not duplicate the tool
|
||||
catalog on the task's next run. Both conversation mappings remain available for
|
||||
authorized follow-up replies.
|
||||
|
||||
After accepting authorized GitHub work, Paperclip posts **Working on this…**
|
||||
before starting the agent. Issue and PR discussion comments use the same GitHub
|
||||
comment API; inline-review conversations receive an inline reply. Description
|
||||
mentions and automatic tasks also receive a working comment. No new eyes
|
||||
The main PR conversation shares one App-owned comment across pushes, mentions,
|
||||
and repeat reviews. A new request updates that comment to **Review queued…**;
|
||||
agent progress and the final assessment update the same comment. Its identity
|
||||
survives restarts and reconnects. Existing request comments can be adopted without
|
||||
creating another main comment. Issue conversations and inline-review threads
|
||||
retain one editable response per request. Description mentions and automatic
|
||||
tasks also receive an acknowledgement. No new eyes
|
||||
reaction is added; cleanup still handles receipts created by older versions.
|
||||
|
||||
The agent's `update_comment` tool edits the current request's working comment.
|
||||
@@ -275,7 +278,11 @@ App-owned markers recover uncertain creation without posting duplicates.
|
||||
Deleted or no-longer-owned comments are not recreated or edited. Coalesced runs
|
||||
settle a deleted earlier working comment without blocking the current response,
|
||||
review findings, or check publication. Failed runs
|
||||
update the same comment when no final reply is confirmed or unresolved. A run
|
||||
update the same comment when no final reply is confirmed or unresolved. Maintenance
|
||||
also reconciles acknowledged requests when recovery stops before a publication
|
||||
is staged. Requests saved on a blocked task report that work is paused and stay
|
||||
open for resumed progress or stopped-run cleanup. An older
|
||||
run cannot replace a newer run's response. A run
|
||||
that ends without a final reply clears a remaining working state honestly.
|
||||
Repository restrictions, person authorization and governed tool checks remain
|
||||
in effect for edits. Existing disabled or quarantined tools stay disabled.
|
||||
@@ -290,14 +297,17 @@ Private images use the exact unchanged source's authenticated GitHub rendering;
|
||||
repository/thread identity and body hashes remain bound across restarts. Signed
|
||||
image URLs and credentials are never added to durable attachment descriptors.
|
||||
|
||||
A manual message naming another connected GitHub bot in the same company does
|
||||
not wake this bot through its thread subscription. Paperclip filters that
|
||||
A manual message beginning with another recipient's @mention does not wake this
|
||||
bot through its thread subscription, including Apps outside Paperclip such as
|
||||
`@greptile review`. Mentions of other connected GitHub bots are also filtered
|
||||
elsewhere in the body. Paperclip filters that
|
||||
delivery before creating task work or adding an acknowledgement. Explicitly
|
||||
mentioning both bots allows both to receive the request, subject to their normal
|
||||
authorization and repository checks.
|
||||
|
||||
Unaddressed follow-ups, human mentions and unknown handles retain the existing
|
||||
subscription behavior. The routing rule does not change automatic issue or PR
|
||||
Unaddressed follow-ups and incidental human or unknown mentions retain the existing
|
||||
subscription behavior. Explicitly mentioning this bot admits a request even when
|
||||
another recipient is named. The routing rule does not change automatic issue or PR
|
||||
event policies. Archived bots and identities from other companies or providers
|
||||
are excluded from the routing lookup.
|
||||
|
||||
|
||||
@@ -255,16 +255,20 @@ discussion, it uses `comment`. For a review, `submit_review` publishes the
|
||||
assessment summary and updates the check. It should not add another comment
|
||||
just to announce that the review is complete.
|
||||
|
||||
Paperclip posts one “Working on this…” comment when it accepts a request. The
|
||||
agent can periodically edit that comment with `update_comment` as it makes
|
||||
Paperclip keeps one main comment per bot per PR, beginning with “Review queued…”.
|
||||
Pushes and repeat requests reuse it. The agent can periodically edit it with `update_comment` as it makes
|
||||
progress. Its final `comment` or review summary replaces the same comment;
|
||||
inline-only reviews close it with a link to the finding. Coalesced requests have
|
||||
their existing working comments updated to point to the response. New requests
|
||||
inline-only reviews close it with a link to the finding. Coalesced PR requests
|
||||
share the main comment; issue and inline threads keep per-request responses. New requests
|
||||
no longer create eyes reactions.
|
||||
|
||||
The runner's final text stays inside the Paperclip task. If a run ends without
|
||||
a tool reply, Paperclip updates the working comment with a safe notice and a
|
||||
link to the task. A real question can link to its answer form in Paperclip.
|
||||
Maintenance clears stale working states after recovery stops, and requests saved
|
||||
while the task is blocked show a paused notice. A leading mention addressed to
|
||||
another recipient, such as `@greptile review`, does not wake this bot unless the
|
||||
message also explicitly mentions it.
|
||||
A confirmed reply suppresses fallback notices. Pending or uncertain tool
|
||||
delivery prevents a duplicate reply while its receipt is resolved. A check
|
||||
alone does not count as a conversation reply.
|
||||
|
||||
@@ -110,6 +110,16 @@ not task completion. Source:
|
||||
PostgreSQL-backed provider fixtures; this entry does not claim live provider
|
||||
qualification for the new retry path.
|
||||
|
||||
GitHub main-response identity (2026-10-19): PR requests share a company-, App-,
|
||||
repository- and PR-bound comment. Agents still supply no comment destination.
|
||||
Current sender, assignee, repository and publication authority are checked before
|
||||
edits, including maintenance failure notices. Leading mentions for another
|
||||
recipient are filtered before task work; explicit mentions of this bot still
|
||||
follow normal access checks. Provider-fixture coverage is not live GitHub proof.
|
||||
Native terminal reconciliation resolves the accepted run identity through the
|
||||
existing chat provenance resolver; an older run's reply cannot complete a newer
|
||||
request merely because both requests share the same PR comment.
|
||||
|
||||
Speko browser sessions use `POST /api/companies/{companyId}/voice-sessions`,
|
||||
with GET inspection/notification and POST `/{sessionId}/end` operations below
|
||||
that collection. These routes require board identity; ordinary agent credentials
|
||||
|
||||
@@ -40,6 +40,16 @@ Setup is covered in [connection setup](./connection-setup.md).
|
||||
|
||||
Automated: [channel integration](../../server/src/__tests__/chat-channels.integration.test.ts) uses provider fixtures; it does not prove a live Slack, Discord, Teams, Telegram, GitHub, email, or Photon delivery.
|
||||
|
||||
GitHub review comment regression (2026-10-19): repeated PR review requests reuse
|
||||
one main App comment, including across restart and reconnect. Test a leading
|
||||
request for an external bot (`@greptile review`) and verify it creates no wake or
|
||||
acknowledgement. Stop execution before a milestone is staged, run maintenance,
|
||||
and verify the existing comment reports the stop; saved requests on a blocked
|
||||
task show paused state. Preserve newer progress and confirmed or uncertain replies.
|
||||
Repeat terminal reconciliation with native identity and no legacy wake fields.
|
||||
Check delayed acknowledgements, check-only assessments followed by formal reviews,
|
||||
and combined requests with separate pre-upgrade placeholders.
|
||||
|
||||
Manual: For each supported provider being changed, send a unique message, verify the intended agent/company receives it, and inspect the actual external reply in the original thread. Repeat with an unlinked/unauthorized sender, duplicate inbound delivery, and one supported attachment. Record provider-specific results separately.
|
||||
|
||||
For Slack, enable **Require at-mention** and **Allow direct messages**. Send a
|
||||
|
||||
@@ -2480,7 +2480,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
await send(); await send();
|
||||
const rows = await db.select().from(chatDeliveries).where(and(eq(chatDeliveries.endpointId, f.endpoint.id), eq(chatDeliveries.providerEventId, `${threadId}:41902`)));
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0]).toMatchObject({ state: "filtered", principalId: null, redactedError: "GitHub message explicitly mentions a different connected bot" });
|
||||
expect(rows[0]).toMatchObject({ state: "filtered", principalId: null, redactedError: "GitHub message explicitly addresses a different recipient" });
|
||||
expect(rows[0].normalizedEvent).toHaveProperty("filtering.contentRetained", false);
|
||||
expect(JSON.stringify(rows[0].normalizedEvent)).not.toContain("tell me a joke");
|
||||
expect(await readIssueCommentsByIssueId(f.conversation.issueId!)).toHaveLength(1);
|
||||
@@ -2488,11 +2488,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
expect(f.runtime.endpoints.get(f.endpoint.id)?.reactions).toEqual([]);
|
||||
expect(f.responseWrites).toHaveLength(1);
|
||||
expect(await f.service.listActivity(f.endpoint.id)).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ status: "filtered", detail: "GitHub message explicitly mentions a different connected bot" }),
|
||||
expect.objectContaining({ status: "filtered", detail: "GitHub message explicitly addresses a different recipient" }),
|
||||
]));
|
||||
});
|
||||
it.each(["Thanks, continue", "@octocat thanks", "@unknown-bot help", "@gonzo-other help", "x@gonzo help"])(
|
||||
"retains subscribed GitHub follow-ups without another exact connected bot mention: %s", async text => {
|
||||
it.each(["Thanks, continue", "Thanks @octocat", "Please ask @unknown-bot for context", "Check @gonzo-other's advice", "x@gonzo help"])(
|
||||
"retains subscribed GitHub follow-ups with incidental human or unknown mentions: %s", async text => {
|
||||
const f = await subscribedGitHubFixture();
|
||||
await addPeerGitHubBot(f);
|
||||
const wakeCount = f.wakeup.mock.calls.length;
|
||||
@@ -2518,7 +2518,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
await db.update(chatEndpoints).set({ botUsername: "gonzo[bot]", status: kind === "archived" ? "archived" : "active" }).where(eq(chatEndpoints.id, peer.id));
|
||||
const wakeCount = f.wakeup.mock.calls.length;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread: f.thread,
|
||||
trigger: "subscribed_message", message: makeMessage({ id: "41902", text: "@gonzo help", userId: "42", userName: "octocat" }) });
|
||||
trigger: "subscribed_message", message: makeMessage({ id: "41902", text: "Please consider @gonzo's advice", userId: "42", userName: "octocat" }) });
|
||||
expect(await readIssueCommentsByIssueId(f.conversation.issueId!)).toHaveLength(2);
|
||||
expect(f.wakeup).toHaveBeenCalledTimes(wakeCount + 1);
|
||||
});
|
||||
@@ -2599,6 +2599,267 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
)))[0];
|
||||
return { ...f, service, head, assessment, provider, action, inline, checks, writes };
|
||||
}
|
||||
it("reuses one main GitHub PR comment across review requests", async () => {
|
||||
const f = await publicationRetryFixture();
|
||||
await f.service.execute(f.session, "submit_review", f.assessment, "first-review");
|
||||
const originalId = [...f.responseComments.keys()][0];
|
||||
// Restart after a provider write whose shared receipt was lost. The
|
||||
// App-owned marker, rather than process memory, must recover the target.
|
||||
await db.update(chatActions).set({ result: null }).where(and(eq(chatActions.endpointId, f.endpoint.id),
|
||||
eq(chatActions.kind, "github_pr_response_comment")));
|
||||
await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
const [next] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.assignedAgentId,
|
||||
status: "running", runtimeMode: "native", contextSnapshot: await chatWakeContext({
|
||||
endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802",
|
||||
}) }).returning();
|
||||
const session = { ...f.session, runId: next.id };
|
||||
await f.service.execute(session, "begin_review", { reviewedCommit: f.head });
|
||||
await f.service.execute(session, "submit_review", { ...f.assessment, summary: "Second assessment" }, "second-review");
|
||||
expect([...f.responseComments.keys()]).toEqual([originalId]);
|
||||
expect(f.responseComments.get(originalId)?.body).toContain("Second assessment");
|
||||
expect(f.responseWrites.filter(write => write.method === "POST")).toHaveLength(1);
|
||||
});
|
||||
it("ignores a review request addressed to an external GitHub bot", async () => {
|
||||
const f = await subscribedGitHubFixture("github:paperclipai/paperclip:419");
|
||||
const wakeCount = f.wakeup.mock.calls.length;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread: f.thread,
|
||||
trigger: "subscribed_message", message: makeMessage({ id: "41902", text: "@greptile review", userId: "42", userName: "octocat" }) });
|
||||
const [delivery] = await db.select().from(chatDeliveries).where(and(eq(chatDeliveries.endpointId, f.endpoint.id),
|
||||
eq(chatDeliveries.providerEventId, `${f.thread.id}:41902`)));
|
||||
expect(delivery.state).toBe("filtered");
|
||||
expect(f.wakeup).toHaveBeenCalledTimes(wakeCount);
|
||||
expect(await readIssueCommentsByIssueId(f.conversation.issueId!)).toHaveLength(1);
|
||||
expect(f.responseWrites).toHaveLength(1);
|
||||
});
|
||||
it("retains a provider-verified GitHub mention when its plain text uses an alias", async () => {
|
||||
const f = await subscribedGitHubFixture("github:paperclipai/paperclip:419");
|
||||
const wakeCount = f.wakeup.mock.calls.length;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread: f.thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41902", text: "@rendered-alias review", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
expect(f.wakeup).toHaveBeenCalledTimes(wakeCount + 1);
|
||||
expect(await readIssueCommentsByIssueId(f.conversation.issueId!)).toHaveLength(2);
|
||||
});
|
||||
it("reconciles a GitHub working comment when execution stops without a publication", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
await db.update(heartbeatRuns).set({ status: "failed", errorCode: "execution_finalization_deadline_exceeded" })
|
||||
.where(eq(heartbeatRuns.id, f.run.id));
|
||||
await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, f.conversation.issueId!));
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
expect([...f.responseComments.values()][0].body).toContain("stopped before completing");
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
expect([...f.responseComments.values()][0].body).toContain("paused");
|
||||
});
|
||||
it("preserves newer GitHub PR progress when an older run fails", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
const [next] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.assignedAgentId,
|
||||
status: "running", runtimeMode: "native", contextSnapshot: await chatWakeContext({
|
||||
endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802",
|
||||
}) }).returning();
|
||||
await githubChatReviewService(db, f.providerFetch).execute({ ...f.session, runId: next.id }, "update_comment",
|
||||
{ body: "Reviewing the new request", idempotencyKey: "new-progress" });
|
||||
await expect(githubResponseCommentService(db, f.providerFetch).failRun({ ...f.session, endpointId: f.endpoint.id,
|
||||
body: "Old run failed" })).rejects.toMatchObject({ status: 403 });
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
expect([...f.responseComments.values()][0].body).toContain("Reviewing the new request");
|
||||
});
|
||||
it("adopts an existing GitHub PR placeholder and retires earlier duplicate placeholders", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
const original = [...f.responseComments.values()][0];
|
||||
original.body = original.body.replace(/<!-- paperclip-pr-response:[^>]+ -->\n/, "");
|
||||
await db.delete(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id), eq(chatActions.kind, "github_pr_response_comment")));
|
||||
// Reconcile a pre-upgrade acknowledgement before its run completes.
|
||||
const [first] = await db.select().from(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id), eq(chatActions.kind, "github_response_comment")));
|
||||
await db.update(chatActions).set({ status: "received" }).where(eq(chatActions.id, first.id));
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
expect(original.body).toContain("paperclip-pr-response:");
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
const [second] = await db.select().from(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id),
|
||||
eq(chatActions.kind, "github_response_comment"), sql`${chatActions.id} <> ${first.id}`));
|
||||
// Simulate the second request's persisted pre-upgrade provider receipt.
|
||||
const duplicateId = 799999;
|
||||
f.responseComments.set(duplicateId, { ...original, id: duplicateId,
|
||||
body: original.body.replace(/<!-- paperclip-pr-response:[^>]+ -->\n/, ""),
|
||||
html_url: `${original.html_url}-legacy`,
|
||||
});
|
||||
await db.update(chatActions).set({ status: "received", result: { ...second.result, id: String(duplicateId) } }).where(eq(chatActions.id, second.id));
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect(f.responseComments.get(duplicateId)?.body).toContain("current review");
|
||||
expect(f.responseComments.get(duplicateId)?.body).not.toContain("queued");
|
||||
expect(f.responseWrites.filter(write => write.method === "POST")).toHaveLength(1);
|
||||
await githubChatReviewService(db, f.providerFetch).execute(f.session, "comment", { body: "Final result", idempotencyKey: "legacy-final" });
|
||||
expect(original.body).toContain("Final result");
|
||||
});
|
||||
it("keeps a newer GitHub request open when its acknowledgement follows older progress", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
const [link] = await db.select().from(chatMessageLinks).where(and(eq(chatMessageLinks.endpointId, f.endpoint.id), eq(chatMessageLinks.providerMessageId, "41802")));
|
||||
await db.update(chatActions).set({ status: "received", result: null }).where(and(eq(chatActions.deliveryId, link.deliveryId!), eq(chatActions.kind, "github_response_comment")));
|
||||
await githubChatReviewService(db, f.providerFetch).execute(f.session, "update_comment", { body: "Older run progress", idempotencyKey: "old-progress" });
|
||||
await githubResponseCommentService(db, f.providerFetch).tryAcknowledge(link.deliveryId!);
|
||||
const [response] = await db.select().from(chatActions).where(and(eq(chatActions.deliveryId, link.deliveryId!), eq(chatActions.kind, "github_response_comment")));
|
||||
expect(response.result?.final).toBe(false);
|
||||
const [next] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.assignedAgentId,
|
||||
status: "running", runtimeMode: "native", contextSnapshot: await chatWakeContext({
|
||||
endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802",
|
||||
}) }).returning();
|
||||
const result = await githubChatReviewService(db, f.providerFetch).execute({ ...f.session, runId: next.id }, "update_comment",
|
||||
{ body: "Newer request progress", idempotencyKey: "new-progress" });
|
||||
expect(result.status).toBe("processed");
|
||||
expect([...f.responseComments.values()][0].body).toContain("Newer request progress");
|
||||
});
|
||||
it.each([false, true])("closes a GitHub placeholder after a formal review (legacy receipt: %s)", async legacy => {
|
||||
const f = await publicationRetryFixture();
|
||||
const formal = { id: 991, html_url: "https://github.com/paperclipai/paperclip/pull/418#pullrequestreview-991" };
|
||||
if (legacy) {
|
||||
await db.insert(chatActions).values({ companyId: f.companyId, endpointId: f.endpoint.id, conversationId: f.conversation.id,
|
||||
kind: "github_review_publication", providerActionId: randomUUID(), status: "processed",
|
||||
payload: { operation: "formal_review", session: f.session }, result: { id: String(formal.id), url: formal.html_url } });
|
||||
} else {
|
||||
const config = await f.management.configuration(f.endpoint.id, "owner-user");
|
||||
await f.management.saveConfiguration(f.endpoint.id, { expectedRevision: config.revision,
|
||||
configuration: { ...config.configuration, defaults: { ...config.configuration.defaults,
|
||||
allowApprove: true, publishSummary: false, publishInline: false } } }, "owner-user");
|
||||
f.setSupplementalProviderFetch(async (input, init) => String(input).includes("/pulls/418/reviews")
|
||||
? Response.json(init?.method === "POST" ? formal : []) : f.provider(input, init));
|
||||
await f.service.execute(f.session, "submit_review", f.assessment, "score-only");
|
||||
await f.service.execute(f.session, "formal_review", { event: "APPROVE", reviewedCommit: f.head, body: "Reviewed", idempotencyKey: "formal-only" });
|
||||
}
|
||||
await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
const body = [...f.responseComments.values()][0].body;
|
||||
expect(body).not.toMatch(/Review queued|Working on this/);
|
||||
expect(body).toContain("paperclip-response-state:final");
|
||||
});
|
||||
it("retires a legacy coalesced GitHub placeholder before replacing its receipt", async () => {
|
||||
const f = await publicationRetryFixture();
|
||||
const original = { ...[...f.responseComments.values()][0] };
|
||||
const [first] = await db.select().from(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id), eq(chatActions.kind, "github_response_comment")));
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
const duplicateId = 799999;
|
||||
f.responseComments.set(duplicateId, { ...original, id: duplicateId,
|
||||
body: original.body.replace(/<!-- paperclip-pr-response:[^>]+ -->\n/, ""),
|
||||
html_url: "https://github.com/paperclipai/paperclip/pull/418#issuecomment-799999" });
|
||||
await db.update(chatActions).set({ result: { ...first.result, id: String(duplicateId) } }).where(eq(chatActions.id, first.id));
|
||||
const links = await db.select().from(chatMessageLinks).where(and(eq(chatMessageLinks.endpointId, f.endpoint.id), eq(chatMessageLinks.direction, "inbound")));
|
||||
const latest = await chatWakeContext({ endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802" });
|
||||
await db.update(heartbeatRuns).set({ contextSnapshot: { ...latest, wakeCommentIds: links.map(link => link.commentId) } }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await f.service.execute(f.session, "begin_review", { reviewedCommit: f.head });
|
||||
await f.service.execute(f.session, "submit_review", f.assessment, "coalesced-upgrade");
|
||||
expect(f.responseComments.get(duplicateId)?.body).toContain("current review");
|
||||
expect(f.responseComments.get(duplicateId)?.body).not.toMatch(/Review queued|Working on this/);
|
||||
expect([...f.responseComments.values()].some(comment => comment.body.includes("Advisory review"))).toBe(true);
|
||||
});
|
||||
it("does not handle a blocked new GitHub request with an older run's final reply", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
await githubChatReviewService(db, f.providerFetch).execute(f.session, "comment", { body: "Earlier result", idempotencyKey: "earlier-result" });
|
||||
await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, f.conversation.issueId!));
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "New request", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
const body = [...f.responseComments.values()][0].body;
|
||||
expect(body).toContain("paused");
|
||||
expect(body).not.toContain("has been handled");
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
});
|
||||
it.each([["failed", false], ["failed", true], ["queued", false], ["queued", true]] as const)(
|
||||
"resumes a paused GitHub request and reconciles stopped execution (prior run: %s, progress: %s)", async (status, progress) => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
await db.update(heartbeatRuns).set({ status }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, f.conversation.issueId!));
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "New request", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
const responses = githubResponseCommentService(db, f.providerFetch);
|
||||
await responses.processPending();
|
||||
expect([...f.responseComments.values()][0].body).toContain("paused");
|
||||
const pausedWrites = f.responseWrites.length;
|
||||
await responses.processPending();
|
||||
await responses.processPending();
|
||||
expect(f.responseWrites).toHaveLength(pausedWrites);
|
||||
await db.update(issues).set({ status: "in_progress" }).where(eq(issues.id, f.conversation.issueId!));
|
||||
const [next] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.assignedAgentId,
|
||||
status: "running", runtimeMode: "native", contextSnapshot: await chatWakeContext({
|
||||
endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802",
|
||||
}) }).returning();
|
||||
const session = { ...f.session, runId: next.id };
|
||||
if (progress) {
|
||||
expect(await githubChatReviewService(db, f.providerFetch).execute(session, "update_comment",
|
||||
{ body: "Resumed request progress", idempotencyKey: "resumed-progress" })).toMatchObject({ status: "processed" });
|
||||
expect([...f.responseComments.values()][0].body).toContain("Resumed request progress");
|
||||
}
|
||||
await db.update(heartbeatRuns).set({ status: "failed" }).where(eq(heartbeatRuns.id, next.id));
|
||||
await responses.failRun({ ...session, endpointId: f.endpoint.id, body: "Resumed work stopped" });
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
expect([...f.responseComments.values()][0].body).toContain("Resumed work stopped");
|
||||
});
|
||||
it("preserves a final GitHub assessment while settling an incomplete request receipt", async () => {
|
||||
const f = await publicationRetryFixture();
|
||||
await f.service.execute(f.session, "submit_review", f.assessment, "published-assessment");
|
||||
const before = [...f.responseComments.values()][0].body;
|
||||
const [response] = await db.select().from(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id), eq(chatActions.kind, "github_response_comment")));
|
||||
await db.update(chatActions).set({ result: { ...response.result, final: false } }).where(eq(chatActions.id, response.id));
|
||||
await db.update(heartbeatRuns).set({ status: "succeeded", updatedAt: new Date() }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect([...f.responseComments.values()][0].body).toBe(before);
|
||||
const [settled] = await db.select().from(chatActions).where(eq(chatActions.id, response.id));
|
||||
expect(settled.result?.final).toBe(true);
|
||||
});
|
||||
it("reconciles stopped GitHub work from native identity without legacy wake fields", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
await initializeRunIdentity(db, { companyId: f.companyId, runId: f.run.id, issueId: f.conversation.issueId!,
|
||||
responsibleUserId: "owner-user", messageIds: [String(f.run.contextSnapshot?.wakeCommentId)], cause: "instruction" });
|
||||
await db.update(heartbeatRuns).set({ nativeIssueId: f.conversation.issueId, contextSnapshot: {}, status: "failed", updatedAt: new Date() })
|
||||
.where(eq(heartbeatRuns.id, f.run.id));
|
||||
await db.update(issues).set({ status: "in_progress" }).where(eq(issues.id, f.conversation.issueId!));
|
||||
await githubResponseCommentService(db, f.providerFetch).processPending();
|
||||
expect([...f.responseComments.values()][0].body).toContain("stopped before completing");
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
});
|
||||
it("retires a legacy GitHub placeholder when its acknowledgement is superseded", async () => {
|
||||
const f = await toolReplyFixture("github:paperclipai/paperclip:418");
|
||||
const original = { ...[...f.responseComments.values()][0] };
|
||||
const [first] = await db.select().from(chatActions).where(and(eq(chatActions.endpointId, f.endpoint.id), eq(chatActions.kind, "github_response_comment")));
|
||||
const duplicateId = 799999;
|
||||
f.responseComments.set(duplicateId, { ...original, id: duplicateId,
|
||||
body: original.body.replace(/<!-- paperclip-pr-response:[^>]+ -->\n/, ""),
|
||||
html_url: "https://github.com/paperclipai/paperclip/pull/418#issuecomment-799999" });
|
||||
await db.update(chatActions).set({ status: "received", result: { ...first.result, id: String(duplicateId) } }).where(eq(chatActions.id, first.id));
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review again", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
const links = await db.select().from(chatMessageLinks).where(and(eq(chatMessageLinks.endpointId, f.endpoint.id), eq(chatMessageLinks.direction, "inbound")));
|
||||
const latest = await chatWakeContext({ endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802" });
|
||||
await db.update(heartbeatRuns).set({ contextSnapshot: { ...latest, wakeCommentIds: links.map(link => link.commentId) } }).where(eq(heartbeatRuns.id, f.run.id));
|
||||
await githubChatReviewService(db, f.providerFetch).execute(f.session, "update_comment", { body: "Newer progress", idempotencyKey: "latest-progress" });
|
||||
await githubResponseCommentService(db, f.providerFetch).tryAcknowledge(first.deliveryId!);
|
||||
expect(f.responseComments.get(duplicateId)?.body).toContain("current review");
|
||||
expect([...f.responseComments.values()].some(comment => comment.body.includes("Newer progress"))).toBe(true);
|
||||
const [settled] = await db.select().from(chatActions).where(eq(chatActions.id, first.id));
|
||||
expect(settled.result?.final).toBe(false);
|
||||
});
|
||||
it.each([false, true])("recovers an approved queued GitHub review without another approval (revoked: %s)", async revoked => {
|
||||
const f = await publicationRetryFixture();
|
||||
let unavailable = true;
|
||||
@@ -3193,7 +3454,8 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
const f = await toolReplyFixture(threadId);
|
||||
const service = githubChatReviewService(db, f.providerFetch);
|
||||
expect(f.responseWrites).toHaveLength(1);
|
||||
expect(f.responseWrites[0]).toMatchObject({ method: "POST", body: expect.stringContaining("Working on this") });
|
||||
expect(f.responseWrites[0]).toMatchObject({ method: "POST", body: expect.stringContaining(
|
||||
threadId === "github:paperclipai/paperclip:418" ? "Review queued" : "Working on this") });
|
||||
expect(f.runtime.endpoints.get(f.endpoint.id)?.reactions).toEqual([]);
|
||||
const progress = await service.execute(f.session, "update_comment", { body: "Checking the request", idempotencyKey: "progress-1" });
|
||||
expect(progress.status).toBe("processed");
|
||||
@@ -3319,7 +3581,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
const thread = makeThread({ channelId: "paperclipai/paperclip", id: f.conversation.externalThreadId }).thread;
|
||||
await deliverMessage({ callbacks: f.callbacks, endpointId: f.endpoint.id, provider: "github", thread,
|
||||
trigger: "mention", message: makeMessage({ id: "41802", text: "Review this too", userId: "42", userName: "octocat", mentioned: true }) });
|
||||
await expect.poll(async () => { await githubResponseCommentService(db, f.providerFetch).processPending(); return f.responseComments.size; }, { timeout: 10_000 }).toBe(2);
|
||||
await expect.poll(async () => { await githubResponseCommentService(db, f.providerFetch).processPending(); return f.responseWrites.length; }, { timeout: 10_000 }).toBe(2);
|
||||
f.responseComments.delete([...f.responseComments.keys()][0]);
|
||||
const links = await db.select().from(chatMessageLinks).where(and(eq(chatMessageLinks.endpointId, f.endpoint.id), eq(chatMessageLinks.direction, "inbound")));
|
||||
const latest = await chatWakeContext({ endpointId: f.endpoint.id, issueId: f.conversation.issueId, provider: "github", providerMessageId: "41802" });
|
||||
@@ -3353,13 +3615,15 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
findings: [{ key: "note", path: "src/math.ts", line: 1, side: "RIGHT", severity: "error", category: "correctness", body: "Example finding" }],
|
||||
}, "inline-only");
|
||||
expect(inlineWrites).toHaveLength(1);
|
||||
expect(f.responseWrites.map(write => write.method)).toEqual(deletedSibling ? ["POST", "POST", "PATCH"] : ["POST", "PATCH"]);
|
||||
expect(f.responseComments.size).toBe(1);
|
||||
expect(f.responseWrites.map(write => write.method)).toEqual(["POST", "PATCH"]);
|
||||
expect(f.responseComments.size).toBe(deletedSibling ? 0 : 1);
|
||||
const comment = [...f.responseComments.values()][0];
|
||||
expect(comment.body).toContain("Review complete.");
|
||||
expect(comment.body).toContain("https://github.com/paperclipai/paperclip/pull/418");
|
||||
expect(comment.body).not.toContain("Disabled summary");
|
||||
expect(comment.body).toContain("paperclip-response-state:final");
|
||||
if (!deletedSibling) {
|
||||
expect(comment.body).toContain("Review complete.");
|
||||
expect(comment.body).toContain("https://github.com/paperclipai/paperclip/pull/418");
|
||||
expect(comment.body).not.toContain("Disabled summary");
|
||||
expect(comment.body).toContain("paperclip-response-state:final");
|
||||
}
|
||||
await expect(githubRunReplyState(db, { ...f.session, endpointId: f.endpoint.id })).resolves.toBe("confirmed");
|
||||
});
|
||||
it.each(["during_summary", "after_summary"] as const)("keeps partial review delivery authoritative when the head changes %s", async phase => {
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { and, asc, eq } from "drizzle-orm";
|
||||
import { chatActions, chatConversations, chatDeliveries, chatEndpoints, type Db } from "@paperclipai/db";
|
||||
import { forbidden, HttpError } from "../../../errors.js";
|
||||
|
||||
export type GitHubResponseSource = {
|
||||
endpoint: typeof chatEndpoints.$inferSelect;
|
||||
conversation: typeof chatConversations.$inferSelect;
|
||||
delivery: typeof chatDeliveries.$inferSelect;
|
||||
number: number;
|
||||
replyId: string | null;
|
||||
repositoryId?: string;
|
||||
};
|
||||
|
||||
export const isMainPullResponse = (source: GitHubResponseSource) =>
|
||||
!source.replyId && /^github:[^:]+:[1-9][0-9]*$/.test(source.conversation.externalThreadId);
|
||||
|
||||
export const requestResponseMarker = (source: GitHubResponseSource) => `<!-- paperclip-response:${createHash("sha256")
|
||||
.update(`${source.endpoint.companyId}:${source.endpoint.id}:${source.delivery.id}`).digest("hex")} -->`;
|
||||
|
||||
export const pullResponseMarker = (source: GitHubResponseSource) => `<!-- paperclip-pr-response:${createHash("sha256")
|
||||
.update(`${source.endpoint.companyId}:${source.endpoint.id}:${source.conversation.externalThreadId.toLowerCase()}`).digest("hex")} -->`;
|
||||
|
||||
/** The per-request ledger remains intact; only the main PR's publication target
|
||||
* is shared. Callers serialize mutations with the repository publication lease. */
|
||||
export async function stagePullResponse(db: Db, source: GitHubResponseSource) {
|
||||
const key = `github_pr_response:${source.conversation.externalThreadId.toLowerCase()}`;
|
||||
const [inserted] = await db.insert(chatActions).values({
|
||||
companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
conversationId: source.conversation.id, kind: "github_pr_response_comment", providerActionId: key,
|
||||
payload: { botExternalId: source.endpoint.botExternalId, agentId: source.endpoint.assignedAgentId }, status: "processed",
|
||||
}).onConflictDoNothing().returning();
|
||||
const target = inserted ?? (await db.select().from(chatActions).where(and(
|
||||
eq(chatActions.companyId, source.endpoint.companyId), eq(chatActions.endpointId, source.endpoint.id),
|
||||
eq(chatActions.providerActionId, key),
|
||||
)))[0];
|
||||
if (!target || target.conversationId !== source.conversation.id ||
|
||||
target.payload.botExternalId !== source.endpoint.botExternalId || target.payload.agentId !== source.endpoint.assignedAgentId)
|
||||
throw forbidden("GitHub main response binding changed");
|
||||
return target;
|
||||
}
|
||||
|
||||
/** Adopt only comments identified by this conversation's durable request ledger
|
||||
* or the previous PR summary marker, never an arbitrary comment by the App. */
|
||||
export async function legacyPullResponseMarkers(db: Db, source: GitHubResponseSource) {
|
||||
const rows = await db.select({ deliveryId: chatActions.deliveryId }).from(chatActions).where(and(
|
||||
eq(chatActions.companyId, source.endpoint.companyId), eq(chatActions.endpointId, source.endpoint.id),
|
||||
eq(chatActions.conversationId, source.conversation.id), eq(chatActions.kind, "github_response_comment"),
|
||||
)).orderBy(asc(chatActions.createdAt));
|
||||
// Per-request markers include the company and endpoint, and remain provable
|
||||
// after a receipt was lost between the provider write and the local commit.
|
||||
const summary = source.repositoryId ? [`<!-- paperclip-review:${createHash("sha256")
|
||||
.update(JSON.stringify(`${source.endpoint.id}:${source.repositoryId}:${source.number}`)).digest("hex")} -->`] : [];
|
||||
return [...summary, ...rows.filter(row => row.deliveryId).map(row => requestResponseMarker({ ...source, delivery: { ...source.delivery, id: row.deliveryId! } }))];
|
||||
}
|
||||
|
||||
/** Retire the proven old placeholder before its only receipt is replaced. */
|
||||
export async function retireLegacyResponsePlaceholder(
|
||||
source: GitHubResponseSource,
|
||||
result: typeof chatActions.$inferSelect["result"],
|
||||
current: { id: string; url: string },
|
||||
api: { request<T>(path: string, options?: { method?: string; body?: unknown }): Promise<T> },
|
||||
assertCurrent: () => Promise<void>,
|
||||
) {
|
||||
if (!result?.id || String(result.id) === current.id || result.final === true) return;
|
||||
if (!/^[1-9][0-9]*$/.test(String(result.id))) throw forbidden("Invalid GitHub response receipt");
|
||||
const item = source.replyId ? "/pulls/comments" : "/issues/comments";
|
||||
try {
|
||||
await assertCurrent();
|
||||
const old = await api.request<{ id: number; body?: string; user?: { login?: string } }>(`${item}/${result.id}`);
|
||||
if (old.user?.login !== source.endpoint.botUsername || !old.body?.includes(requestResponseMarker(source)))
|
||||
throw forbidden("GitHub response is no longer owned by this App");
|
||||
await assertCurrent();
|
||||
await api.request(`${item}/${old.id}`, { method: "PATCH", body: {
|
||||
body: `See the [current review](${current.url}).\n\n${requestResponseMarker(source)}\n<!-- paperclip-response-state:final -->`,
|
||||
} });
|
||||
} catch (error) {
|
||||
const details = error instanceof HttpError ? error.details as { providerStatus?: number } | undefined : undefined;
|
||||
if (details?.providerStatus !== 404) throw error;
|
||||
}
|
||||
}
|
||||
@@ -9,7 +9,7 @@ import type { EndpointRow, DbTransaction, DbOrTransaction, InboundRuntimeContext
|
||||
import { notifyChatDeliveryWork } from "../../chat-work-notifications.js";
|
||||
|
||||
import {
|
||||
githubMessageAddressesAnotherBot,
|
||||
githubMessageAddressesAnotherRecipient,
|
||||
} from "../../chat-github-events.js";
|
||||
|
||||
import {
|
||||
@@ -173,15 +173,16 @@ export function createInboundAdmission(db: Db, options: InboundAdmissionOptions)
|
||||
message.isMention !== true;
|
||||
// GitHub sends the same comment to every installed App. Subscribed
|
||||
// conversations are a fallback for unaddressed replies, not permission
|
||||
// to wake every bot when the person names a different connected App.
|
||||
// to wake every bot when the person addresses a different recipient.
|
||||
// Recheck inside durable admission so recovered deliveries follow the
|
||||
// same company-scoped routing decision before any task or reaction.
|
||||
const githubAddressedElsewhere =
|
||||
endpointAccepting &&
|
||||
endpoint.provider === "github" &&
|
||||
!addressed &&
|
||||
!githubAutomatic &&
|
||||
!githubIssue &&
|
||||
(await githubMessageAddressesAnotherBot(tx, {
|
||||
(await githubMessageAddressesAnotherRecipient(tx, {
|
||||
id: endpoint.id,
|
||||
companyId: endpoint.companyId,
|
||||
botUsername: currentEndpoint.botUsername,
|
||||
@@ -201,7 +202,7 @@ export function createInboundAdmission(db: Db, options: InboundAdmissionOptions)
|
||||
? "Connection activation changed before admission"
|
||||
: "Connection is not active"
|
||||
: githubAddressedElsewhere
|
||||
? "GitHub message explicitly mentions a different connected bot"
|
||||
? "GitHub message explicitly addresses a different recipient"
|
||||
: mentionRequired
|
||||
? "Message did not @mention the bot"
|
||||
: endpoint.provider === "telegram" && !thread.isDM && !addressed
|
||||
|
||||
@@ -42,14 +42,16 @@ export function githubBodyMentionsBot(body: string, username: string | null): bo
|
||||
|
||||
/**
|
||||
* A thread subscription must not turn a request for another connected bot
|
||||
* into work for this bot. Human mentions still follow ordinary thread policy,
|
||||
* and explicitly mentioning this bot (including alongside another) wins. */
|
||||
export async function githubMessageAddressesAnotherBot(
|
||||
* into work for this bot. A leading mention explicitly selects a recipient,
|
||||
* including Apps outside Paperclip. Incidental human mentions still follow
|
||||
* thread policy, and explicitly mentioning this bot always wins. */
|
||||
export async function githubMessageAddressesAnotherRecipient(
|
||||
db: Db | Parameters<Parameters<Db["transaction"]>[0]>[0],
|
||||
endpoint: Pick<typeof chatEndpoints.$inferSelect, "id" | "companyId" | "botUsername">,
|
||||
body: string,
|
||||
): Promise<boolean> {
|
||||
if (!body.includes("@") || githubBodyMentionsBot(body, endpoint.botUsername)) return false;
|
||||
if (/^\s*@[a-z\d](?:[a-z\d-]{0,38})(?:\[bot\])?(?![\w-])/i.test(body)) return true;
|
||||
const others = await db
|
||||
.select({ username: chatEndpoints.botUsername })
|
||||
.from(chatEndpoints)
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { and, asc, desc, eq, inArray, or, sql } from "drizzle-orm";
|
||||
import {
|
||||
chatActions, chatConversations, chatDeliveries, chatEndpoints,
|
||||
@@ -17,30 +16,27 @@ import { githubRunReplyState } from "./chat-run-publications.js";
|
||||
import { logActivity } from "./activity-log.js";
|
||||
import { boundRunIssueId } from "./run-task-binding.js";
|
||||
import { chatRunSourceMessageId } from "./chat-channels/run-origin.js";
|
||||
import { isMainPullResponse, legacyPullResponseMarkers, pullResponseMarker, requestResponseMarker,
|
||||
retireLegacyResponsePlaceholder, stagePullResponse, type GitHubResponseSource } from "./chat-channels/github/response-target.js";
|
||||
|
||||
type Transaction = Parameters<Parameters<Db["transaction"]>[0]>[0];
|
||||
type Api = { request<T>(path: string, options?: { method?: string; body?: unknown }): Promise<T> };
|
||||
type Lease = { commit<T>(write: (tx: Transaction) => Promise<T>): Promise<T> };
|
||||
type Source = {
|
||||
endpoint: typeof chatEndpoints.$inferSelect;
|
||||
conversation: typeof chatConversations.$inferSelect;
|
||||
delivery: typeof chatDeliveries.$inferSelect;
|
||||
number: number;
|
||||
replyId: string | null;
|
||||
};
|
||||
type Source = GitHubResponseSource;
|
||||
type Comment = { id: number; body?: string; html_url: string; user?: { login?: string } };
|
||||
const generation = (endpoint: Source["endpoint"]) => (endpoint.setup as { runtimeGeneration?: number }).runtimeGeneration ?? 0;
|
||||
const actionKey = (deliveryId: string) => `github_response:${deliveryId}`;
|
||||
const responseMarker = (source: Source) => `<!-- paperclip-response:${createHash("sha256")
|
||||
.update(`${source.endpoint.companyId}:${source.endpoint.id}:${source.delivery.id}`).digest("hex")} -->`;
|
||||
const responseMarker = (source: Source) => isMainPullResponse(source) ? pullResponseMarker(source) : requestResponseMarker(source);
|
||||
|
||||
async function findResponseComment(source: Source, api: Api): Promise<Comment | null> {
|
||||
async function findResponseComment(source: Source, api: Api, legacyMarkers: string[] = []): Promise<Comment | null> {
|
||||
const list = source.replyId ? `/pulls/${source.number}/comments` : `/issues/${source.number}/comments`;
|
||||
let legacy: Comment | null = null;
|
||||
for (let page = 1; page <= 100; page++) {
|
||||
const rows = await api.request<Comment[]>(`${list}?per_page=100&page=${page}`);
|
||||
const found = rows.find(row => row.body?.includes(responseMarker(source)) && row.user?.login === source.endpoint.botUsername);
|
||||
if (found) return found;
|
||||
if (rows.length < 100) return null;
|
||||
legacy ??= rows.find(row => legacyMarkers.some(marker => row.body?.includes(marker)) && row.user?.login === source.endpoint.botUsername) ?? null;
|
||||
if (rows.length < 100) return legacy;
|
||||
}
|
||||
throw conflict("GitHub history is too large to safely recover the response");
|
||||
}
|
||||
@@ -71,13 +67,26 @@ async function stageResponse(db: Db, source: Source) {
|
||||
|
||||
export async function writeGitHubResponseComment(
|
||||
db: Db, source: Source, api: Api, lease: Lease,
|
||||
input: { body: string; final: boolean; versionAt?: Date; acknowledgement?: boolean },
|
||||
input: { body: string; final: boolean; versionAt?: Date; acknowledgement?: boolean; preserveFinal?: boolean },
|
||||
assertCurrent: () => Promise<void>,
|
||||
) {
|
||||
await assertCurrent();
|
||||
const response = await stageResponse(db, source);
|
||||
const shared = isMainPullResponse(source);
|
||||
const target = shared ? await stagePullResponse(db, source) : response;
|
||||
const versionAt = input.versionAt ?? source.delivery.createdAt;
|
||||
if (target.result?.id && Date.parse(String(target.result.versionAt)) > versionAt.getTime()) {
|
||||
if (!input.acknowledgement) throw conflict("A newer GitHub response has already been published");
|
||||
if (shared) await retireLegacyResponsePlaceholder(source, response.result,
|
||||
{ id: String(target.result.id), url: String(target.result.url) }, api, assertCurrent);
|
||||
await lease.commit(async tx => { await tx.update(chatActions).set({ status: "processed", result: {
|
||||
...response.result, id: target.result!.id, url: target.result!.url,
|
||||
final: response.result?.final === true, code: "acknowledgement_superseded", versionAt: versionAt.toISOString(),
|
||||
}, updatedAt: new Date() }).where(eq(chatActions.id, response.id)); });
|
||||
return { id: String(target.result.id), url: String(target.result.url) };
|
||||
}
|
||||
// A delayed acknowledgement or progress retry must never replace a result.
|
||||
if (response.result?.id && (input.acknowledgement || (response.result.final === true && !input.final) ||
|
||||
if (response.result?.id && (!shared || response.result.id === target.result?.id) && (input.acknowledgement || (response.result.final === true && !input.final) ||
|
||||
(input.versionAt && Date.parse(String(response.result.versionAt)) > input.versionAt.getTime()))) {
|
||||
if (!input.acknowledgement && response.result.final === true && !input.final)
|
||||
throw conflict("This GitHub response is already final");
|
||||
@@ -88,30 +97,40 @@ export async function writeGitHubResponseComment(
|
||||
const item = source.replyId ? "/pulls/comments" : "/issues/comments";
|
||||
await assertCurrent();
|
||||
let previous: Comment | null = null;
|
||||
if (response.result?.id) {
|
||||
if (target.result?.id) {
|
||||
// Never recreate a deleted comment or edit an arbitrary caller-selected ID.
|
||||
if (!/^[1-9][0-9]*$/.test(String(response.result.id))) throw forbidden("Invalid GitHub response receipt");
|
||||
previous = await api.request<Comment>(`${item}/${response.result.id}`);
|
||||
if (!/^[1-9][0-9]*$/.test(String(target.result.id))) throw forbidden("Invalid GitHub response receipt");
|
||||
previous = await api.request<Comment>(`${item}/${target.result.id}`);
|
||||
if (!previous.body?.includes(marker) || previous.user?.login !== source.endpoint.botUsername)
|
||||
throw forbidden("GitHub response is no longer owned by this App");
|
||||
} else {
|
||||
previous = await findResponseComment(source, api);
|
||||
previous = await findResponseComment(source, api, shared ? await legacyPullResponseMarkers(db, source) : []);
|
||||
}
|
||||
await assertCurrent();
|
||||
const alreadyFinal = previous?.body?.includes("<!-- paperclip-response-state:final -->") === true;
|
||||
const body = `${projectSafeChatPublicationText(input.body)}\n\n${marker}${input.final ? "\n<!-- paperclip-response-state:final -->" : ""}`;
|
||||
const posted = previous && (input.acknowledgement || (alreadyFinal && !input.final)) ? previous : await api.request<Comment>(
|
||||
const providerVersion = previous?.body?.match(/<!-- paperclip-response-version:([^ ]+) -->/)?.[1];
|
||||
if (providerVersion && Date.parse(providerVersion) > versionAt.getTime()) throw conflict("A newer GitHub response has already been published");
|
||||
const sameRequest = !shared || previous?.body?.includes(requestResponseMarker(source)) === true;
|
||||
const alreadyFinal = sameRequest && previous?.body?.includes("<!-- paperclip-response-state:final -->") === true;
|
||||
const body = `${projectSafeChatPublicationText(input.body)}\n\n${marker}${shared ? `\n${requestResponseMarker(source)}` : ""}\n<!-- paperclip-response-version:${versionAt.toISOString()} -->${input.final ? "\n<!-- paperclip-response-state:final -->" : ""}`;
|
||||
const preserved = previous && (previous.body === body || (input.acknowledgement && sameRequest) || (alreadyFinal && !input.final) ||
|
||||
(input.preserveFinal && previous.body?.includes("<!-- paperclip-response-state:final -->"))) ? previous : null;
|
||||
const posted = preserved && (!shared || preserved.body?.includes(marker)) ? preserved : await api.request<Comment>(
|
||||
previous ? `${item}/${previous.id}` : source.replyId
|
||||
? `/pulls/${source.number}/comments/${source.replyId}/replies` : list,
|
||||
{ method: previous ? "PATCH" : "POST", body: { body } },
|
||||
{ method: previous ? "PATCH" : "POST", body: { body: preserved ? `${preserved.body}\n${marker}` : body } },
|
||||
);
|
||||
if (!Number.isSafeInteger(posted.id) || posted.id <= 0 || !posted.html_url)
|
||||
if (!posted || !Number.isSafeInteger(posted.id) || posted.id <= 0 || !posted.html_url)
|
||||
throw conflict("GitHub response delivery was not confirmed");
|
||||
if (shared) await retireLegacyResponsePlaceholder(source, response.result,
|
||||
{ id: String(posted.id), url: posted.html_url }, api, assertCurrent);
|
||||
await assertCurrent();
|
||||
const receipt = { id: String(posted.id), url: posted.html_url };
|
||||
await lease.commit(async tx => {
|
||||
if (shared) await tx.update(chatActions).set({ result: {
|
||||
...receipt, final: input.final || alreadyFinal, versionAt: versionAt.toISOString(), deliveryId: source.delivery.id,
|
||||
}, updatedAt: new Date() }).where(eq(chatActions.id, target.id));
|
||||
await tx.update(chatActions).set({ status: "processed", result: {
|
||||
...receipt, final: input.final || alreadyFinal, versionAt: (input.versionAt ?? response.createdAt).toISOString(),
|
||||
...receipt, final: input.final || alreadyFinal, versionAt: versionAt.toISOString(),
|
||||
}, updatedAt: new Date() }).where(eq(chatActions.id, response.id));
|
||||
await logActivity(tx as unknown as Db, { companyId: source.endpoint.companyId,
|
||||
actorType: "system", actorId: "github-response", action: "chat_github.response_updated",
|
||||
@@ -125,7 +144,7 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
async function context(deliveryId: string) {
|
||||
const [row] = await db.select({ endpoint: chatEndpoints, conversation: chatConversations,
|
||||
delivery: chatDeliveries, resource: chatEndpointResources, config: chatGitHubConfigurations,
|
||||
assignee: issues.assigneeAgentId }).from(chatMessageLinks)
|
||||
assignee: issues.assigneeAgentId, issueStatus: issues.status, sourceMessageId: chatMessageLinks.commentId }).from(chatMessageLinks)
|
||||
.innerJoin(chatDeliveries, and(eq(chatDeliveries.id, chatMessageLinks.deliveryId), eq(chatDeliveries.companyId, chatMessageLinks.companyId)))
|
||||
.innerJoin(chatConversations, and(eq(chatConversations.id, chatMessageLinks.conversationId), eq(chatConversations.companyId, chatMessageLinks.companyId)))
|
||||
.innerJoin(chatEndpoints, and(eq(chatEndpoints.id, chatConversations.endpointId), eq(chatEndpoints.companyId, chatConversations.companyId)))
|
||||
@@ -164,9 +183,13 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
};
|
||||
await assertCurrent();
|
||||
let onlyExisting = false;
|
||||
let finalVersionAt: Date | undefined;
|
||||
let requestRunning = false;
|
||||
let requestPaused = false;
|
||||
let preserveFinal = false;
|
||||
if (finalBody === undefined) {
|
||||
const existing = await stageResponse(db, source);
|
||||
const [run] = await db.select().from(heartbeatRuns).innerJoin(chatMessageLinks, and(
|
||||
const [requestRun] = await db.select().from(heartbeatRuns).innerJoin(chatMessageLinks, and(
|
||||
eq(chatMessageLinks.companyId, source.endpoint.companyId), eq(chatMessageLinks.endpointId, source.endpoint.id),
|
||||
eq(chatMessageLinks.deliveryId, deliveryId), eq(chatMessageLinks.direction, "inbound"),
|
||||
or(sql`${chatMessageLinks.commentId}::text = ${heartbeatRuns.contextSnapshot}->>'wakeCommentId'`,
|
||||
@@ -174,15 +197,42 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
)).where(and(eq(heartbeatRuns.companyId, source.endpoint.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId),
|
||||
sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${source.conversation.issueId}`))
|
||||
.orderBy(desc(heartbeatRuns.createdAt)).limit(1);
|
||||
if (run && ["succeeded", "failed", "interrupted", "timed_out", "cancelled"].includes(run.heartbeat_runs.status)) {
|
||||
const reply = await githubRunReplyState(db, { companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
issueId: source.conversation.issueId!, runId: run.heartbeat_runs.id });
|
||||
const [latestRun] = await db.select().from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, source.endpoint.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId),
|
||||
sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${source.conversation.issueId}`,
|
||||
)).orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)).limit(1);
|
||||
const originMessageId = latestRun?.activeIdentityContextId ? await chatRunSourceMessageId(db, latestRun,
|
||||
{ allowLegacyWakeComment: true, provider: "github" }) : null;
|
||||
const ownsLatestRequest = !!latestRun && (requestRun?.heartbeat_runs.id === latestRun.id ||
|
||||
(!!source.sourceMessageId && originMessageId === source.sourceMessageId));
|
||||
requestRunning = ownsLatestRequest && latestRun?.status === "running";
|
||||
const terminal = latestRun && ["succeeded", "failed", "interrupted", "timed_out", "cancelled"].includes(latestRun.status);
|
||||
const run = terminal && (ownsLatestRequest || source.issueStatus === "blocked") ? latestRun : null;
|
||||
if (run) {
|
||||
const reply = ownsLatestRequest ? await githubRunReplyState(db, {
|
||||
companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
issueId: source.conversation.issueId!, runId: run.id }) : "none";
|
||||
if (reply === "unsettled") throw conflict("GitHub reply is still being resolved");
|
||||
if (existing.result?.final === true) return existing.result.id ? { id: String(existing.result.id), url: String(existing.result.url) } : undefined;
|
||||
preserveFinal = reply === "confirmed";
|
||||
onlyExisting = reply === "confirmed" && !existing.result?.id;
|
||||
requestPaused = source.issueStatus === "blocked" && !ownsLatestRequest;
|
||||
finalBody = reply === "confirmed" ? "This request has been handled. See the response in this conversation."
|
||||
: run.heartbeat_runs.status === "succeeded" ? "This turn ended without publishing a final response. See the Paperclip task for details."
|
||||
: requestPaused ? "Work is paused. Your request is saved. See the Paperclip task for details."
|
||||
: run.status === "succeeded" ? "This turn ended without publishing a final response. See the Paperclip task for details."
|
||||
: "This turn stopped before completing. See the Paperclip task for details.";
|
||||
} else if (existing.result?.id) return { id: String(existing.result.id), url: String(existing.result.url) };
|
||||
finalVersionAt = new Date(Math.max(run.updatedAt.getTime(), source.delivery.createdAt.getTime()));
|
||||
} else if (source.issueStatus === "blocked" && latestRun?.status !== "running") {
|
||||
requestPaused = true;
|
||||
finalBody = "Work is paused. Your request is saved. See the Paperclip task for details.";
|
||||
finalVersionAt = source.delivery.createdAt;
|
||||
} else if (existing.result?.id && (!isMainPullResponse(source) ||
|
||||
(await stagePullResponse(db, source)).result?.id === existing.result.id)) {
|
||||
// Rotate no-op maintenance checks so a page of newer queued asks
|
||||
// cannot hide other conversations that need terminal reconciliation.
|
||||
await lease.commit(async tx => { await tx.update(chatActions).set({ updatedAt: new Date() }).where(eq(chatActions.id, existing.id)); });
|
||||
return { id: String(existing.result.id), url: String(existing.result.url) };
|
||||
}
|
||||
if (finalBody === undefined) {
|
||||
const [fast] = await db.select({ request: fastResponseRequests, publicationState: chatPublications.state })
|
||||
.from(fastResponseRequests).leftJoin(chatPublications, and(
|
||||
@@ -216,8 +266,9 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
return;
|
||||
}
|
||||
return writeGitHubResponseComment(db, source, api, lease,
|
||||
{ body: finalBody ?? "Working on this…", final: finalBody !== undefined,
|
||||
acknowledgement: finalBody === undefined, ...(finalBody !== undefined ? { versionAt: new Date() } : {}) }, assertCurrent);
|
||||
{ body: finalBody ?? (isMainPullResponse(source) && !requestRunning ? "Review queued…" : "Working on this…"), final: finalBody !== undefined && !requestPaused,
|
||||
acknowledgement: finalBody === undefined, preserveFinal,
|
||||
...(finalBody !== undefined ? { versionAt: finalVersionAt ?? new Date() } : {}) }, assertCurrent);
|
||||
});
|
||||
}
|
||||
async function requestsForRun(input: { companyId: string; endpointId: string; issueId: string; runId: string }) {
|
||||
@@ -245,6 +296,16 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
throw forbidden("Coalesced GitHub response belongs to another conversation");
|
||||
const [response] = await db.select().from(chatActions).where(and(eq(chatActions.deliveryId, link.deliveryId), eq(chatActions.kind, "github_response_comment")));
|
||||
if (!response || response.result?.final === true) continue;
|
||||
if (isMainPullResponse(primary) && !input.closePrimary) {
|
||||
// Every coalesced request shares the published main comment. Settle
|
||||
// receipts without replacing the assessment with a link to itself.
|
||||
await retireLegacyResponsePlaceholder(sibling, response.result, receipt, api,
|
||||
async () => { await assertCurrent(); await context(link.deliveryId!); });
|
||||
await lease.commit(async tx => { await tx.update(chatActions).set({ status: "processed",
|
||||
result: { ...response.result, ...receipt, final: true }, updatedAt: new Date(),
|
||||
}).where(eq(chatActions.id, response.id)); });
|
||||
continue;
|
||||
}
|
||||
// Never create a sibling placeholder just to close it. Mark a failed
|
||||
// acknowledgement settled so its maintenance retry cannot post later.
|
||||
if (!response.result?.id && !(await findResponseComment(sibling, api))) {
|
||||
@@ -290,6 +351,11 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
source.endpoint.id !== input.endpointId || source.conversation.issueId !== input.issueId ||
|
||||
boundRunIssueId(current) !== input.issueId)
|
||||
throw forbidden("GitHub failure authority changed");
|
||||
const [latest] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, input.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId),
|
||||
sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${input.issueId}`,
|
||||
)).orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)).limit(1);
|
||||
if (latest?.id !== run.id) throw forbidden("A newer GitHub run owns the response");
|
||||
if ((await responseForRequest())?.result?.final === true) throw forbidden("GitHub request already has a final response");
|
||||
if (await githubRunReplyState(db, input) !== "none") throw conflict("GitHub reply is still being resolved");
|
||||
};
|
||||
@@ -312,20 +378,44 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
or(eq(chatActions.status, "received"), and(eq(chatActions.status, "failed"), sql`${chatActions.result}->>'retryable' = 'true'`,
|
||||
sql`(${chatActions.result}->>'retryAt')::timestamptz <= now()`)))).orderBy(asc(chatActions.createdAt)).limit(limit);
|
||||
for (const row of rows) if (row.deliveryId) await tryAcknowledge(row.deliveryId);
|
||||
return rows.length;
|
||||
// Recovery can stop execution before a milestone publication is staged.
|
||||
// Revisit acknowledged requests with terminal execution, even after a
|
||||
// restart; successful acknowledgement is not successful task completion.
|
||||
const stopped = await db.select({ deliveryId: chatActions.deliveryId }).from(chatActions)
|
||||
.innerJoin(chatConversations, and(eq(chatConversations.id, chatActions.conversationId), eq(chatConversations.companyId, chatActions.companyId)))
|
||||
.innerJoin(chatEndpoints, and(eq(chatEndpoints.id, chatActions.endpointId), eq(chatEndpoints.companyId, chatActions.companyId)))
|
||||
.innerJoin(issues, and(eq(issues.id, chatConversations.issueId), eq(issues.companyId, chatActions.companyId)))
|
||||
.where(and(eq(chatActions.kind, "github_response_comment"), eq(chatActions.status, "processed"),
|
||||
sql`coalesce(${chatActions.result}->>'final', 'false') <> 'true'`,
|
||||
sql`exists (select 1 from ${heartbeatRuns} r where r.company_id = ${chatActions.companyId}
|
||||
and r.agent_id = ${chatEndpoints.assignedAgentId}
|
||||
and coalesce(r.native_issue_id::text, r.context_snapshot->>'issueId') = ${chatConversations.issueId}::text
|
||||
and r.status in ('succeeded', 'failed', 'interrupted', 'timed_out', 'cancelled')
|
||||
and not exists (select 1 from ${heartbeatRuns} newer where newer.company_id = r.company_id
|
||||
and newer.agent_id = r.agent_id
|
||||
and coalesce(newer.native_issue_id::text, newer.context_snapshot->>'issueId') = ${chatConversations.issueId}::text
|
||||
and (newer.created_at, newer.id) > (r.created_at, r.id))
|
||||
and (${issues.status} = 'blocked' or r.active_identity_context_id is not null or exists (select 1 from ${chatMessageLinks} l
|
||||
where l.company_id = ${chatActions.companyId} and l.endpoint_id = ${chatActions.endpointId}
|
||||
and l.delivery_id = ${chatActions.deliveryId} and l.direction = 'inbound'
|
||||
and (l.comment_id::text = r.context_snapshot->>'wakeCommentId'
|
||||
or coalesce(r.context_snapshot->'wakeCommentIds', '[]'::jsonb) ? l.comment_id::text))))`,
|
||||
)).orderBy(asc(chatActions.updatedAt)).limit(limit);
|
||||
for (const row of stopped) if (row.deliveryId) await tryAcknowledge(row.deliveryId);
|
||||
return rows.length + stopped.length;
|
||||
}
|
||||
async function tryAcknowledge(deliveryId: string) {
|
||||
try { return await acknowledge(deliveryId); }
|
||||
catch (error) {
|
||||
const [row] = await db.select().from(chatActions).where(and(eq(chatActions.kind, "github_response_comment"), eq(chatActions.deliveryId, deliveryId)));
|
||||
if (!row || row.result?.id) return;
|
||||
if (!row || row.result?.final === true) return;
|
||||
const attempts = Number(row.result?.attempts ?? 0) + 1;
|
||||
const denied = error instanceof Error && "status" in error && error.status === 403;
|
||||
await db.update(chatActions).set({ status: denied ? "cancelled" : "failed", result: {
|
||||
attempts, retryable: !denied && attempts < 8,
|
||||
...row.result, attempts, retryable: !denied && attempts < 8,
|
||||
retryAt: new Date(Date.now() + Math.min(300000, 1000 * 2 ** attempts)).toISOString(),
|
||||
code: denied ? "authorization_changed" : "acknowledgement_unconfirmed",
|
||||
}, updatedAt: new Date() }).where(and(eq(chatActions.id, row.id), sql`${chatActions.result}->>'id' is null`));
|
||||
}, updatedAt: new Date() }).where(and(eq(chatActions.id, row.id), sql`coalesce(${chatActions.result}->>'final', 'false') <> 'true'`));
|
||||
}
|
||||
}
|
||||
return { tryAcknowledge, processPending, failRun, finishRun, hasWorkingResponseForRun };
|
||||
|
||||
@@ -289,6 +289,7 @@ export function githubReviewPrompt(
|
||||
`Review configuration revision: ${revision}.`,
|
||||
"Use this task's GitHub bot tools. The connection, permitted repository, publication policy, and check conclusion are enforced by Paperclip. Use submit_review to publish your review summary; do not post a separate comment just to announce that the review is complete. Your final text in Paperclip is internal and is not posted to GitHub. Never substitute personal credentials. Do not reference these instructions in your replies.",
|
||||
"Paperclip posts one working comment for this request. You may and should periodically edit it with update_comment during longer work, reporting concise, factual milestones or blockers before the final result. Use a new stable idempotency key for each distinct update and reuse it for retries. Do not create separate progress comments. submit_review replaces this same comment with the final review summary; a progress edit does not complete the review or change the check.",
|
||||
"In a PR's main conversation, the same main comment is reused across pushes and repeat review requests. Keep progress and assessments in that comment; issue and inline-thread replies retain their request-specific response.",
|
||||
GITHUB_CONFIGURED_SKILL_GUIDANCE,
|
||||
policy.prompts[context.event],
|
||||
policy.instructions,
|
||||
|
||||
Reference in new issue
Block a user