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] 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,