diff --git a/packages/shared/src/constants.ts b/packages/shared/src/constants.ts index 28e18d3dfd..ec6ddc8893 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 6b17bf735e..189bc05079 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 657100a560..7ee9286da9 100644 --- a/server/src/__tests__/issue-thread-interactions-service.test.ts +++ b/server/src/__tests__/issue-thread-interactions-service.test.ts @@ -3140,6 +3140,246 @@ 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 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({ + 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 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, + 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, + derivedAuthorAgentId: randomUUID(), + }, { + 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(); + 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 9b973d962b..1f93608310 100644 --- a/server/src/services/issue-thread-interactions.ts +++ b/server/src/services/issue-thread-interactions.ts @@ -41,6 +41,8 @@ import type { ConnectionIntentInteraction, CreateIssueThreadInteraction, InteractionResolverGovernance, + IssueCommentAuthorType, + IssueCommentPresentation, IssueReviewPolicy, IssueThreadInteraction, IssueThreadInteractionCanonicalResolverPolicy, @@ -71,6 +73,7 @@ import { connectionIntentResultSchema, createIssueThreadInteractionSchema, legacyIssueThreadInteractionResolverPolicyAlias, + NON_HUMAN_SENTINEL_AUTHOR_USER_IDS, normalizeIssueThreadInteractionResolverPolicy, rejectIssueThreadInteractionSchema, requestCheckboxConfirmationPayloadSchema, @@ -818,6 +821,53 @@ function shouldSupersedeInteractionOnUserComment(interaction: UserCommentSuperse return interaction.payload.supersedeOnUserComment === true; } +// 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" + || Boolean(comment.derivedAuthorAgentId); +} + +// 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); +} + function normalizeCreateInteractionInput( input: CreateIssueThreadInteraction, ): CreateIssueThreadInteraction { @@ -4127,14 +4177,18 @@ export function issueThreadInteractionService( id: string; createdAt: Date | string; authorUserId?: string | null; + authorType?: IssueCommentAuthorType | null; createdByRunId?: string | null; + derivedAuthorAgentId?: 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 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 .select() @@ -4236,14 +4290,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 + // non-answer shapes live in jsonb and are filtered below. isNull(issueComments.createdByRunId), ), ) .orderBy(asc(issueComments.createdAt)), ]); - if (rows.length === 0 || comments.length === 0) return []; + // Only a comment that could be a human answer supersedes a card waiting on one. + const answerComments = comments.filter(commentCanSupersedeDecision); + + if (rows.length === 0 || answerComments.length === 0) return []; const now = new Date(); const expired: IssueThreadInteraction[] = []; @@ -4261,7 +4319,7 @@ export function issueThreadInteractionService( ) as UserCommentSupersedableInteraction; if (!shouldSupersedeInteractionOnUserComment(interaction)) continue; - const supersedingComment = comments.find((comment) => + const supersedingComment = answerComments.find((comment) => isCommentAtOrAfterInteraction({ commentCreatedAt: comment.createdAt, interactionCreatedAt: row.createdAt, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index b3d297bdf4..06307813c9 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -94,6 +94,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"; @@ -216,11 +217,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;