fix(interactions): separate non-answer framing from machine authorship
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) <noreply@anthropic.com>
This commit is contained in:
parent
d524e9a1c2
commit
a7bb79bc62
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue