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;