From d524e9a1c2a4805168e870b085b96b341c6022c6 Mon Sep 17 00:00:00 2001 From: anatol Date: Thu, 10 Sep 2026 03:05:42 +0300 Subject: [PATCH 1/3] fix(interactions): do not supersede decision cards on board-token automation comments Automation that authenticates with a board or user token has no run context. Its comments carry a non-null authorUserId and a null createdByRunId, so the supersede guard treats them as a human answer and expires a pending card. Honour the two machine markers the schema already has: authorType "system" and presentation.kind "system_notice". The live and the historical supersede paths share one helper. --- .../issue-thread-interactions-service.test.ts | 126 ++++++++++++++++++ .../src/services/issue-thread-interactions.ts | 37 ++++- 2 files changed, 157 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index 657100a560..b407afc370 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -3140,6 +3140,132 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { expect(rows[0]?.status).toBe("pending"); }); + it("does not supersede request confirmations for board-token automation comments", async () => { + // Automation that authenticates with a board/user token has no run context, so + // createdByRunId is null and the comment is indistinguishable from a human's unless + // the caller marks it. Both machine markers must be honoured. + const { companyId, issueId } = await seedConfirmationIssue("Board-token automation supersede exclusion"); + + const created = await interactionsSvc.create({ + id: issueId, + companyId, + }, { + kind: "request_confirmation", + payload: { + version: 1, + prompt: "Proceed with the current draft?", + }, + }, { + userId: "local-board", + }); + const afterCreated = new Date(new Date(created.createdAt).getTime() + 1_000); + + await expect(interactionsSvc.expireRequestConfirmationsSupersededByComment({ + id: issueId, + companyId, + }, { + id: randomUUID(), + createdAt: afterCreated, + authorUserId: "local-board", + createdByRunId: null, + presentation: { + kind: "system_notice", + tone: "info", + detailsDefaultOpen: false, + }, + }, { + userId: "local-board", + })).resolves.toHaveLength(0); + + await expect(interactionsSvc.expireRequestConfirmationsSupersededByComment({ + id: issueId, + companyId, + }, { + id: randomUUID(), + createdAt: afterCreated, + authorUserId: "local-board", + authorType: "system", + createdByRunId: null, + }, { + userId: "local-board", + })).resolves.toHaveLength(0); + + const rows = await db.select().from(issueThreadInteractions); + expect(rows).toHaveLength(1); + expect(rows[0]?.status).toBe("pending"); + }); + + it("does not repair historical confirmations from system-notice comments, but still repairs from human ones", async () => { + const { companyId, issueId } = await seedConfirmationIssue("Historical system-notice exclusion"); + const humanCommentId = randomUUID(); + const createdAt = new Date("2026-05-18T12:00:00.000Z"); + + const created = await interactionsSvc.create({ + id: issueId, + companyId, + }, { + kind: "request_confirmation", + payload: { + version: 1, + prompt: "Proceed with the current draft?", + }, + }, { + userId: "local-board", + }); + await db + .update(issueThreadInteractions) + .set({ createdAt, updatedAt: createdAt }) + .where(eq(issueThreadInteractions.id, created.id)); + + await db.insert(issueComments).values({ + id: randomUUID(), + companyId, + issueId, + authorUserId: "local-board", + authorType: "user", + presentation: { + kind: "system_notice", + tone: "info", + detailsDefaultOpen: false, + }, + body: "Automated pipeline check: no open PR for this card yet.", + createdAt: new Date("2026-05-18T12:01:00.000Z"), + updatedAt: new Date("2026-05-18T12:01:00.000Z"), + }); + + await expect(interactionsSvc.expireRequestConfirmationsSupersededByHistoricalComments({ + id: issueId, + companyId, + })).resolves.toEqual([]); + + // The sweep is filtered, not disabled: a real human comment still supersedes. + await db.insert(issueComments).values({ + id: humanCommentId, + companyId, + issueId, + authorUserId: "local-board", + authorType: "user", + body: "Please revise this first.", + createdAt: new Date("2026-05-18T12:02:00.000Z"), + updatedAt: new Date("2026-05-18T12:02:00.000Z"), + }); + + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByHistoricalComments({ + id: issueId, + companyId, + }); + expect(expired).toHaveLength(1); + expect(expired[0]).toMatchObject({ + id: created.id, + status: "expired", + result: { + version: 1, + outcome: "superseded_by_comment", + commentId: humanCommentId, + }, + }); + }); + it("repairs historical request confirmations superseded by later user comments idempotently", async () => { const { companyId, issueId } = await seedConfirmationIssue("Historical comment supersede"); const commentId = randomUUID(); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 8e2fea0a81..78af6d61f7 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -39,6 +39,8 @@ import type { ConnectionIntentInteraction, CreateIssueThreadInteraction, InteractionResolverGovernance, + IssueCommentAuthorType, + IssueCommentPresentation, IssueReviewPolicy, IssueThreadInteraction, IssueThreadInteractionCanonicalResolverPolicy, @@ -816,6 +818,23 @@ function shouldSupersedeInteractionOnUserComment(interaction: UserCommentSuperse return interaction.payload.supersedeOnUserComment === true; } +// `createdByRunId` catches machine comments that come from a heartbeat run, but not +// every machine posts from one. Local automation — pipeline monitors, cron scripts, +// gateway hooks — authenticates with a board/user token and carries no run context, so +// its comments are byte-identical to a human's at this decision point. A comment the +// caller marked as a system notice is machine-authored by construction and must never +// stand in for a human answer: otherwise an automated "no PR yet, your move" nudge +// expires the very question the work is waiting on, and the next nudge asks again. +function isMachineAuthoredComment(comment: { + authorType?: IssueCommentAuthorType | null; + createdByRunId?: string | null; + presentation?: IssueCommentPresentation | null; +}) { + if (comment.createdByRunId) return true; + if (comment.authorType === "system") return true; + return comment.presentation?.kind === "system_notice"; +} + function normalizeCreateInteractionInput( input: CreateIssueThreadInteraction, ): CreateIssueThreadInteraction { @@ -4109,14 +4128,16 @@ export function issueThreadInteractionService( id: string; createdAt: Date | string; authorUserId?: string | null; + authorType?: IssueCommentAuthorType | null; createdByRunId?: string | null; + presentation?: IssueCommentPresentation | null; }, actor: InteractionActor, ) => { if (!comment.authorUserId) return []; - // Local-CLI adapters post under user auth, so authorUserId can't tell a human from a - // machine; createdByRunId can. Only genuine human comments (no run context) supersede. - if (comment.createdByRunId) return []; + // Local-CLI adapters and board-token automation both post under user auth, so + // authorUserId can't tell a human from a machine. Only genuine human comments do. + if (isMachineAuthoredComment(comment)) return []; const rows = await db .select() @@ -4218,14 +4239,18 @@ export function issueThreadInteractionService( eq(issueComments.companyId, issue.companyId), eq(issueComments.issueId, issue.id), isNotNull(issueComments.authorUserId), - // Only genuine human comments supersede; machine-originated ones carry createdByRunId. + // Cheap SQL prefilter for the common machine case; the remaining + // machine-authored shapes live in jsonb and are filtered below. isNull(issueComments.createdByRunId), ), ) .orderBy(asc(issueComments.createdAt)), ]); - if (rows.length === 0 || comments.length === 0) return []; + // Only genuine human comments supersede a card that is waiting on a human. + const humanComments = comments.filter((comment) => !isMachineAuthoredComment(comment)); + + if (rows.length === 0 || humanComments.length === 0) return []; const now = new Date(); const expired: IssueThreadInteraction[] = []; @@ -4243,7 +4268,7 @@ export function issueThreadInteractionService( ) as UserCommentSupersedableInteraction; if (!shouldSupersedeInteractionOnUserComment(interaction)) continue; - const supersedingComment = comments.find((comment) => + const supersedingComment = humanComments.find((comment) => isCommentAtOrAfterInteraction({ commentCreatedAt: comment.createdAt, interactionCreatedAt: row.createdAt, From a7bb79bc62db8f9a4082e959fb67a6e1adcba1fa Mon Sep 17 00:00:00 2001 From: anatol-zeon <265601443+anatol-zeon@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:12:30 +0300 Subject: [PATCH 2/3] fix(interactions): separate non-answer framing from machine authorship MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile is right that `presentation.kind` is caller-supplied and cannot prove authorship, so folding it into `isMachineAuthoredComment` overstated what the signal means. Split the classifier into the two questions it was conflating: - `isMachineAuthoredComment` — authenticated provenance only (`createdByRunId`, `authorType: "system"`). - `isNonAnswerComment` — declared intent: a comment framed as a status notice is not an answer to a pending question, whoever wrote it. `commentCanSupersedeDecision` requires both to be false. Behaviour for the board-token automation case is unchanged — that path has no authenticated provenance to read, which is the bug this PR fixes — but a human who posts a system notice now falls under a rule that is stated as intended rather than as an authorship inference, and they keep every way of answering: resolve the card, or comment normally. Covered by a new regression test. Co-Authored-By: Claude Opus 5 (1M context) --- .../issue-thread-interactions-service.test.ts | 71 ++++++++++++++++++- .../src/services/issue-thread-interactions.ts | 52 +++++++++----- 2 files changed, 105 insertions(+), 18 deletions(-) diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index b407afc370..058d738d7e 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -3143,7 +3143,8 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { it("does not supersede request confirmations for board-token automation comments", async () => { // Automation that authenticates with a board/user token has no run context, so // createdByRunId is null and the comment is indistinguishable from a human's unless - // the caller marks it. Both machine markers must be honoured. + // the caller marks it. Both markers must be honoured: the system-notice framing says + // the comment is not an answer, and authorType says the author is not a human. const { companyId, issueId } = await seedConfirmationIssue("Board-token automation supersede exclusion"); const created = await interactionsSvc.create({ @@ -3195,6 +3196,74 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { expect(rows[0]?.status).toBe("pending"); }); + it("treats a system notice as a non-answer even when a human posted it, and still supersedes on their next plain comment", async () => { + // `presentation` is caller-supplied, so it cannot prove authorship — and it does not + // have to. A comment framed as a status notice is not an answer to a pending question + // whoever wrote it, so it must not consume the card. The person keeps every way of + // answering: resolving the card, or simply commenting normally. + const { companyId, issueId } = await seedConfirmationIssue("Human-authored system notice is not an answer"); + + const created = await interactionsSvc.create({ + id: issueId, + companyId, + }, { + kind: "request_confirmation", + payload: { + version: 1, + prompt: "Proceed with the current draft?", + }, + }, { + userId: "local-board", + }); + const afterCreated = new Date(new Date(created.createdAt).getTime() + 1_000); + + await expect(interactionsSvc.expireRequestConfirmationsSupersededByComment({ + id: issueId, + companyId, + }, { + id: randomUUID(), + createdAt: afterCreated, + authorUserId: "local-board", + authorType: "user", + createdByRunId: null, + presentation: { + kind: "system_notice", + tone: "info", + detailsDefaultOpen: false, + }, + }, { + userId: "local-board", + })).resolves.toHaveLength(0); + + const stillPending = await db.select().from(issueThreadInteractions); + expect(stillPending).toHaveLength(1); + expect(stillPending[0]?.status).toBe("pending"); + + const plainCommentId = randomUUID(); + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({ + id: issueId, + companyId, + }, { + id: plainCommentId, + createdAt: new Date(afterCreated.getTime() + 1_000), + authorUserId: "local-board", + authorType: "user", + createdByRunId: null, + }, { + userId: "local-board", + }); + expect(expired).toHaveLength(1); + expect(expired[0]).toMatchObject({ + id: created.id, + status: "expired", + result: { + version: 1, + outcome: "superseded_by_comment", + commentId: plainCommentId, + }, + }); + }); + it("does not repair historical confirmations from system-notice comments, but still repairs from human ones", async () => { const { companyId, issueId } = await seedConfirmationIssue("Historical system-notice exclusion"); const humanCommentId = randomUUID(); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 78af6d61f7..69c2d3d9a7 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -818,21 +818,38 @@ function shouldSupersedeInteractionOnUserComment(interaction: UserCommentSuperse return interaction.payload.supersedeOnUserComment === true; } -// `createdByRunId` catches machine comments that come from a heartbeat run, but not -// every machine posts from one. Local automation — pipeline monitors, cron scripts, -// gateway hooks — authenticates with a board/user token and carries no run context, so -// its comments are byte-identical to a human's at this decision point. A comment the -// caller marked as a system notice is machine-authored by construction and must never -// stand in for a human answer: otherwise an automated "no PR yet, your move" nudge -// expires the very question the work is waiting on, and the next nudge asks again. +// Authenticated provenance: the comment demonstrably came from a machine. `createdByRunId` +// covers anything posted from a heartbeat run; `authorType: "system"` covers the internal +// producers (pipeline monitors, watchdogs, recovery) that insert notices directly. function isMachineAuthoredComment(comment: { authorType?: IssueCommentAuthorType | null; createdByRunId?: string | null; +}) { + return Boolean(comment.createdByRunId) || comment.authorType === "system"; +} + +// Declared intent: the comment is a status notice, not an answer to anything. That is a +// separate question from who wrote it, and deliberately so — `presentation` is caller- +// supplied and cannot prove authorship. It does not need to. Local automation that +// authenticates with a board/user token has no run context and is forced to +// `authorType: "user"`, so at this decision point its comments are byte-identical to a +// human's, and the notice framing is the only thing the caller told us. Reading that as +// "not an answer" holds either way: an automated "no PR yet, your move" nudge must not +// expire the very question the work is waiting on, and a person who deliberately posts a +// system notice has not answered the question either. Answering stays available and +// explicit — resolve the card. +function isNonAnswerComment(comment: { presentation?: IssueCommentPresentation | null }) { + return comment.presentation?.kind === "system_notice"; +} + +// A pending decision card waits on a human answer, so only a comment that could be one may +// consume it. +function commentCanSupersedeDecision(comment: { + authorType?: IssueCommentAuthorType | null; + createdByRunId?: string | null; presentation?: IssueCommentPresentation | null; }) { - if (comment.createdByRunId) return true; - if (comment.authorType === "system") return true; - return comment.presentation?.kind === "system_notice"; + return !isMachineAuthoredComment(comment) && !isNonAnswerComment(comment); } function normalizeCreateInteractionInput( @@ -4136,8 +4153,9 @@ export function issueThreadInteractionService( ) => { if (!comment.authorUserId) return []; // Local-CLI adapters and board-token automation both post under user auth, so - // authorUserId can't tell a human from a machine. Only genuine human comments do. - if (isMachineAuthoredComment(comment)) return []; + // authorUserId can't tell a human from a machine, and a status notice is not an + // answer whoever wrote it. Only a comment that could be a human answer supersedes. + if (!commentCanSupersedeDecision(comment)) return []; const rows = await db .select() @@ -4240,17 +4258,17 @@ export function issueThreadInteractionService( eq(issueComments.issueId, issue.id), isNotNull(issueComments.authorUserId), // Cheap SQL prefilter for the common machine case; the remaining - // machine-authored shapes live in jsonb and are filtered below. + // non-answer shapes live in jsonb and are filtered below. isNull(issueComments.createdByRunId), ), ) .orderBy(asc(issueComments.createdAt)), ]); - // Only genuine human comments supersede a card that is waiting on a human. - const humanComments = comments.filter((comment) => !isMachineAuthoredComment(comment)); + // Only a comment that could be a human answer supersedes a card waiting on one. + const answerComments = comments.filter(commentCanSupersedeDecision); - if (rows.length === 0 || humanComments.length === 0) return []; + if (rows.length === 0 || answerComments.length === 0) return []; const now = new Date(); const expired: IssueThreadInteraction[] = []; @@ -4268,7 +4286,7 @@ export function issueThreadInteractionService( ) as UserCommentSupersedableInteraction; if (!shouldSupersedeInteractionOnUserComment(interaction)) continue; - const supersedingComment = humanComments.find((comment) => + const supersedingComment = answerComments.find((comment) => isCommentAtOrAfterInteraction({ commentCreatedAt: comment.createdAt, interactionCreatedAt: row.createdAt, From c481262320ced37c26d15488c1ae32d0e00fe3a9 Mon Sep 17 00:00:00 2001 From: anatol-zeon <265601443+anatol-zeon@users.noreply.github.com> Date: Thu, 10 Sep 2026 03:18:40 +0300 Subject: [PATCH 3/3] fix(interactions): gate the system-notice signal on server-derived provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile and Superagent both landed on the same objection, and the security framing is the sharper one: `presentation` is caller-supplied, so letting it alone suppress expiry lets a genuine user hold their own decision card open indefinitely. Two changes close that without reopening the original bug. Add `derivedAuthorAgentId` to machine provenance. The server writes it from run logs and no caller can set it, and the tier exists precisely because agents post under the `local-board` sentinel — the case this guard has to catch. Honour the `system_notice` marker only from a non-human sentinel author. That id comes from authentication, never from a request body, so the pair (sentinel author + notice framing) is evidence a real signup cannot manufacture. Board-token automation still has no run context and is still forced to `authorType: "user"`, so it stays excluded and SBK-167's regression stays fixed; a genuine user's marked comment now supersedes as it always did. `NON_HUMAN_SENTINEL_AUTHOR_USER_IDS` moves to shared constants so both services read one definition instead of two. Tests: a genuine user cannot suppress expiry with the marker, and a server-derived agent attribution does not supersede while the next plain board comment still does. Co-Authored-By: Claude Opus 5 (1M context) --- packages/shared/src/constants.ts | 7 ++ packages/shared/src/index.ts | 1 + .../issue-thread-interactions-service.test.ts | 67 ++++++++++++++++--- .../src/services/issue-thread-interactions.ts | 51 +++++++++----- server/src/services/issues.ts | 6 +- 5 files changed, 98 insertions(+), 34 deletions(-) diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index ed8aabe7d8..baf784c2b2 100644 --- a/packages/shared/src/constants.ts +++ b/packages/shared/src/constants.ts @@ -232,6 +232,13 @@ export type SummarySlotStatus = (typeof SUMMARY_SLOT_STATUSES)[number]; export const ISSUE_COMMENT_AUTHOR_TYPES = ["user", "agent", "system"] as const; export type IssueCommentAuthorType = (typeof ISSUE_COMMENT_AUTHOR_TYPES)[number]; +// Author sentinels that agents and local automation post under. `local-board` is also +// materialized as a row in the `user` table (it is the implicit board admin), so a plain +// "exists in the user table" check does not identify it. The id is assigned by +// authentication and never read from a request body, which makes it safe to treat as +// provenance. Genuine human users — real signups with their own ids — are never in here. +export const NON_HUMAN_SENTINEL_AUTHOR_USER_IDS = new Set(["local-board"]); + export const ISSUE_COMMENT_PRESENTATION_KINDS = ["message", "system_notice"] as const; export type IssueCommentPresentationKind = (typeof ISSUE_COMMENT_PRESENTATION_KINDS)[number]; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 9d546d584d..5dd98c71ac 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -401,6 +401,7 @@ export { SUMMARY_SLOT_KEYS, SUMMARY_SLOT_STATUSES, ISSUE_COMMENT_AUTHOR_TYPES, + NON_HUMAN_SENTINEL_AUTHOR_USER_IDS, ISSUE_COMMENT_METADATA_ROW_TYPES, ISSUE_COMMENT_PRESENTATION_KINDS, ISSUE_COMMENT_PRESENTATION_TONES, diff --git a/server/src/__tests__/issue-thread-interactions-service.test.ts b/server/src/__tests__/issue-thread-interactions-service.test.ts index 058d738d7e..7ee9286da9 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -3196,12 +3196,61 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { expect(rows[0]?.status).toBe("pending"); }); - it("treats a system notice as a non-answer even when a human posted it, and still supersedes on their next plain comment", async () => { - // `presentation` is caller-supplied, so it cannot prove authorship — and it does not - // have to. A comment framed as a status notice is not an answer to a pending question - // whoever wrote it, so it must not consume the card. The person keeps every way of - // answering: resolving the card, or simply commenting normally. - const { companyId, issueId } = await seedConfirmationIssue("Human-authored system notice is not an answer"); + it("does not let a genuine user suppress expiry with a system-notice marker", async () => { + // `presentation` is caller-supplied, so it is only honoured from a non-human sentinel + // author. A real signup that marks its comment as a system notice must not be able to + // hold its own pending card open indefinitely — that comment still supersedes. + const { companyId, issueId } = await seedConfirmationIssue("Genuine user cannot suppress expiry"); + + const created = await interactionsSvc.create({ + id: issueId, + companyId, + }, { + kind: "request_confirmation", + payload: { + version: 1, + prompt: "Proceed with the current draft?", + }, + }, { + userId: "local-board", + }); + + const markedCommentId = randomUUID(); + const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({ + id: issueId, + companyId, + }, { + id: markedCommentId, + createdAt: new Date(new Date(created.createdAt).getTime() + 1_000), + authorUserId: "genuine-signup-user", + authorType: "user", + createdByRunId: null, + presentation: { + kind: "system_notice", + tone: "info", + detailsDefaultOpen: false, + }, + }, { + userId: "genuine-signup-user", + }); + + expect(expired).toHaveLength(1); + expect(expired[0]).toMatchObject({ + id: created.id, + status: "expired", + result: { + version: 1, + outcome: "superseded_by_comment", + commentId: markedCommentId, + }, + }); + }); + + it("does not supersede on a server-derived agent attribution, and still supersedes on the next plain board comment", async () => { + // `derivedAuthorAgentId` is written by the server from run logs, not by the caller. It + // exists because agents post under the `local-board` sentinel, which is exactly the + // case this guard has to catch. + const { companyId, issueId } = await seedConfirmationIssue("Derived agent attribution is machine-authored"); const created = await interactionsSvc.create({ id: issueId, @@ -3226,11 +3275,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => { authorUserId: "local-board", authorType: "user", createdByRunId: null, - presentation: { - kind: "system_notice", - tone: "info", - detailsDefaultOpen: false, - }, + derivedAuthorAgentId: randomUUID(), }, { userId: "local-board", })).resolves.toHaveLength(0); diff --git a/server/src/services/issue-thread-interactions.ts b/server/src/services/issue-thread-interactions.ts index 69c2d3d9a7..7f3d650ecd 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -71,6 +71,7 @@ import { connectionIntentResultSchema, createIssueThreadInteractionSchema, legacyIssueThreadInteractionResolverPolicyAlias, + NON_HUMAN_SENTINEL_AUTHOR_USER_IDS, normalizeIssueThreadInteractionResolverPolicy, rejectIssueThreadInteractionSchema, requestCheckboxConfirmationPayloadSchema, @@ -818,35 +819,48 @@ function shouldSupersedeInteractionOnUserComment(interaction: UserCommentSuperse return interaction.payload.supersedeOnUserComment === true; } -// Authenticated provenance: the comment demonstrably came from a machine. `createdByRunId` -// covers anything posted from a heartbeat run; `authorType: "system"` covers the internal -// producers (pipeline monitors, watchdogs, recovery) that insert notices directly. +// Server-derived provenance: the comment demonstrably came from a machine, on evidence no +// caller can forge. `createdByRunId` covers anything posted from a heartbeat run; +// `authorType: "system"` covers the internal producers (pipeline monitors, watchdogs, +// recovery) that insert notices directly; `derivedAuthorAgentId` covers the comments the +// server later reattributed to an agent from run logs — the tier that exists precisely +// because agents post under the `local-board` sentinel. function isMachineAuthoredComment(comment: { authorType?: IssueCommentAuthorType | null; createdByRunId?: string | null; + derivedAuthorAgentId?: string | null; }) { - return Boolean(comment.createdByRunId) || comment.authorType === "system"; + return Boolean(comment.createdByRunId) + || comment.authorType === "system" + || Boolean(comment.derivedAuthorAgentId); } -// Declared intent: the comment is a status notice, not an answer to anything. That is a -// separate question from who wrote it, and deliberately so — `presentation` is caller- -// supplied and cannot prove authorship. It does not need to. Local automation that -// authenticates with a board/user token has no run context and is forced to -// `authorType: "user"`, so at this decision point its comments are byte-identical to a -// human's, and the notice framing is the only thing the caller told us. Reading that as -// "not an answer" holds either way: an automated "no PR yet, your move" nudge must not -// expire the very question the work is waiting on, and a person who deliberately posts a -// system notice has not answered the question either. Answering stays available and -// explicit — resolve the card. -function isNonAnswerComment(comment: { presentation?: IssueCommentPresentation | null }) { - return comment.presentation?.kind === "system_notice"; +// Declared intent: the comment is a status notice, not an answer to a pending question. +// `presentation` is caller-supplied, so on its own it proves nothing — a genuine user could +// set it and stall their own card indefinitely. It is therefore only honoured from a +// non-human sentinel author, which is assigned by authentication and never read from a +// request body. That combination is what local automation looks like and what a real signup +// cannot produce: automation on a board token has no run context and is forced to +// `authorType: "user"` by assertIssueCommentAuthorTypeAllowed, so the sentinel id plus the +// notice framing is the only evidence left that its "no PR yet, your move" nudge is not an +// answer to the very question the work is waiting on. +function isNonAnswerComment(comment: { + authorUserId?: string | null; + presentation?: IssueCommentPresentation | null; +}) { + if (comment.presentation?.kind !== "system_notice") return false; + const authorUserId = comment.authorUserId; + if (!authorUserId) return false; + return NON_HUMAN_SENTINEL_AUTHOR_USER_IDS.has(authorUserId); } // A pending decision card waits on a human answer, so only a comment that could be one may // consume it. function commentCanSupersedeDecision(comment: { + authorUserId?: string | null; authorType?: IssueCommentAuthorType | null; createdByRunId?: string | null; + derivedAuthorAgentId?: string | null; presentation?: IssueCommentPresentation | null; }) { return !isMachineAuthoredComment(comment) && !isNonAnswerComment(comment); @@ -4147,14 +4161,15 @@ export function issueThreadInteractionService( authorUserId?: string | null; authorType?: IssueCommentAuthorType | null; createdByRunId?: string | null; + derivedAuthorAgentId?: string | null; presentation?: IssueCommentPresentation | null; }, actor: InteractionActor, ) => { if (!comment.authorUserId) return []; // Local-CLI adapters and board-token automation both post under user auth, so - // authorUserId can't tell a human from a machine, and a status notice is not an - // answer whoever wrote it. Only a comment that could be a human answer supersedes. + // authorUserId alone can't tell a human from a machine. Only a comment that could be + // a human answer supersedes. if (!commentCanSupersedeDecision(comment)) return []; const rows = await db diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index e51af10035..72efa31de8 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -93,6 +93,7 @@ import { issueCommentMetadataSchema, issueCommentPresentationSchema, isUuidLike, + NON_HUMAN_SENTINEL_AUTHOR_USER_IDS, normalizeIssueIdentifier as normalizeIssueReferenceIdentifier, } from "@paperclipai/shared"; import { conflict, HttpError, notFound, unprocessable } from "../errors.js"; @@ -215,11 +216,6 @@ const ISSUE_LIST_RELATED_QUERY_CHUNK_SIZE = 500; export const MAX_CHILD_ISSUES_CREATED_BY_HELPER = 25; const MAX_CHILD_COMPLETION_SUMMARIES = 20; const CHILD_COMPLETION_SUMMARY_BODY_MAX_CHARS = 500; -// Non-human author sentinels that agents post under. These ARE eligible for -// agent-attribution derivation even though `local-board` is also materialized -// as a row in the `user` table (it is the implicit board admin). Genuine human -// users — real signups with their own ids — are never reattributed. -const NON_HUMAN_SENTINEL_AUTHOR_USER_IDS = new Set(["local-board"]); const ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES = 2_000_000; const ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES = 256_000; const ISSUE_COMMENT_RUN_LOG_DERIVATION_END_SLACK_MS = 60_000;