diff --git a/cdk/src/constructs/iteration-heartbeat.ts b/cdk/src/constructs/iteration-heartbeat.ts index 4c50a6cb6..9b9f7d7b3 100644 --- a/cdk/src/constructs/iteration-heartbeat.ts +++ b/cdk/src/constructs/iteration-heartbeat.ts @@ -52,14 +52,13 @@ export interface IterationHeartbeatProps { * Mid-run liveness heartbeat (scheduled). * * A scheduled Lambda that finds RUNNING comment-triggered iteration tasks and - * EDITS the existing maturing Linear reply in place to show liveness ("πŸ”„ + * EDITS the existing maturing Linear/Jira comment in place to show liveness ("πŸ”„ * Working β€” updating PR #N… _8m elapsed_"), so a long run isn't a silent black * box between πŸ‘€ and the terminal βœ…/❌ (observed in practice as a 22-min silence). * - * The construct owns only the Lambda + schedule + TaskTable read. The Linear - * workspace-registry env + per-workspace OAuth ``GetSecretValue`` grant are - * wired by the stack after instantiation (mirrors OrchestrationReconciler), - * since they belong to the LinearIntegration construct. + * The construct owns only the Lambda + schedule + TaskTable read. Surface + * registry environments and OAuth grants are wired by the stack after + * instantiation, since they belong to their integration constructs. */ export class IterationHeartbeat extends Construct { public readonly fn: lambda.NodejsFunction; @@ -86,7 +85,7 @@ export class IterationHeartbeat extends Construct { }); // Read-only on the TaskTable (StatusIndex query). No write β€” a heartbeat - // never mutates task state; it only edits a Linear comment. + // never mutates task state; it only edits a surface comment. props.taskTable.grantReadData(this.fn); const schedule = props.schedule ?? Duration.minutes(DEFAULT_SCHEDULE_MINUTES); @@ -103,7 +102,7 @@ export class IterationHeartbeat extends Construct { { id: 'AwsSolutions-IAM5', reason: 'DynamoDB index/* wildcard generated by CDK grantReadData; ' - + 'per-workspace linear-oauth secret prefix grant added by the stack', + + 'scoped per-surface OAuth secret prefix grants added by the stack', }, ], true); } diff --git a/cdk/src/handlers/fanout-task-events.ts b/cdk/src/handlers/fanout-task-events.ts index a04f4e69a..dcd93c4c1 100644 --- a/cdk/src/handlers/fanout-task-events.ts +++ b/cdk/src/handlers/fanout-task-events.ts @@ -55,9 +55,9 @@ import { claimTerminalReply, releaseReplyClaim, terminalReplyClaimed } from './s import { buildAdfDocument, postIssueCommentAdf, - type AdfParagraph, - type AdfTextRun, + updateIssueCommentAdf, } from './shared/jira-feedback'; +import { renderJiraFinalStatusComment } from './shared/jira-status-comment'; import { EMOJI_FAILURE, EMOJI_NEEDS_INPUT, EMOJI_SUCCESS, postIssueComment, swapCommentReaction, upsertThreadedReply } from './shared/linear-feedback'; import { logger } from './shared/logger'; import { coerceNumericOrNull } from './shared/numeric'; @@ -206,10 +206,11 @@ export const CHANNEL_DEFAULTS: Record> // now emits it as a distinct terminal event (``orchestrator.ts``); the // Slack + email defaults already subscribe to it. // - // Jira has no comment-edit API (same as Linear), so this is post-once: - // idempotency across partial-batch retries rides on the - // ``jira_final_comment_event_id`` marker. The agent-side start comment - // ("πŸ€– ABCA picked up this issue…") stays for in-flight progress. + // Ordinary Jira tasks receive a post-once terminal comment, with idempotency + // carried by ``jira_final_comment_event_id``. Comment-triggered iterations + // instead edit their stored status comment under a terminal-writer claim. + // The agent-side start comment ("πŸ€– ABCA picked up this issue…") stays for + // ordinary tasks' in-flight progress. jira: new Set([ ...TERMINAL_EVENT_TYPES, 'task_timed_out', @@ -691,10 +692,9 @@ async function saveLinearPrCommentState(taskId: string, eventId: string): Promis } /** - * Persist the post-once marker after a successful Jira final-status comment - * (see ``dispatchToJira``). The Jira analogue of ``saveLinearCommentState`` β€” - * Jira has no comment-edit API, so the marker is what makes the post - * idempotent across partial-batch retries. + * Persist the post-once marker after a successful ordinary Jira final-status + * comment (see ``dispatchToJira``). Iteration comments are edited in place and + * use the shared terminal-reply claim instead. */ async function saveJiraCommentState(taskId: string, eventId: string): Promise { await saveDispatchMarker({ @@ -1546,93 +1546,7 @@ async function sumIterationCostForIssue( }); } -/** - * Render the Jira final-status comment as ADF paragraphs. Mirrors - * ``renderLinearFinalStatusComment`` framing β€” the difference is the output - * shape (ADF runs vs Markdown string), because Jira REST v3 comments require - * Atlassian Document Format, not Markdown. - * - * Three outcomes based on ``(eventType, prUrl)``: - * - * 1. ``task_completed`` β†’ βœ… "Task completed" - * 2. any non-completed terminal event WITH PR β†’ ⚠️ "Shipped a PR but stopped early" - * 3. any non-completed terminal event NO PR β†’ ❌ "Task " + classifier title - * - * The PR URL is rendered on the βœ… success path too β€” not just the ⚠️ path β€” - * because the agent's own "PR opened" comment is not guaranteed to have fired - * (an agent that skipped that step), so the platform comment must always carry - * the link or it can be lost entirely. - * ``renderLinearFinalStatusComment`` does the same for Linear. - * - * Missing metric values render as ``β€”``. The result is a list of ADF - * paragraphs (blank lines are empty paragraphs β€” ADF text nodes do not - * honor ``\n``), fed to ``buildAdfDocument``. - */ -export function renderJiraFinalStatusComment(args: { - eventType: string; - prUrl: string | null; - costUsd: number | null; - turns: number | null; - maxTurns: number | null; - durationS: number | null; - taskId: string; - errorTitle: string | null; -}): ReadonlyArray { - const isCompleted = args.eventType === 'task_completed'; - const shippedDespiteFailure = !isCompleted && args.prUrl != null; - - // Header runs. Bold scope mirrors Linear's Markdown: the ⚠️ frame bolds - // only through the reason and leaves the trailing "review and decide…" - // advice unbolded, so it's a two-run paragraph. The βœ… / ❌ frames are a - // single bold run. - let headerRuns: AdfTextRun[]; - if (isCompleted) { - headerRuns = [{ text: 'βœ… Task completed', strong: true }]; - } else if (shippedDespiteFailure) { - const reason = args.errorTitle ? ` β€” ${args.errorTitle}` : ''; - headerRuns = [ - { text: `⚠️ Shipped a PR but stopped early${reason}`, strong: true }, - { text: ' β€” review and decide if more work is needed' }, - ]; - } else { - // Humanize the event subtype for the header: strip the ``task_`` prefix - // and turn underscores into spaces so ``task_timed_out`` reads "Task - // timed out" rather than the raw "Task timed_out". Jira is the only - // channel routing ``task_timed_out`` through this renderer, so this - // multi-word subtype is a case the copied-from-Linear code never hit. - const subtype = args.eventType.replace(/^task_/, '').replace(/_/g, ' '); - const reason = args.errorTitle ? `: ${args.errorTitle}` : ''; - headerRuns = [{ text: `❌ Task ${subtype}${reason}`, strong: true }]; - } - - const costStr = args.costUsd != null ? `$${args.costUsd.toFixed(2)}` : 'β€”'; - const turnsStr = args.turns != null - ? `${args.turns}${args.maxTurns != null ? ` / ${args.maxTurns}` : ''}` - : 'β€”'; - const durationStr = args.durationS != null - ? formatDuration(args.durationS) - : 'β€”'; - - const paragraphs: AdfParagraph[] = [ - headerRuns, - [{ text: `cost: ${costStr} β€’ turns: ${turnsStr} β€’ duration: ${durationStr}` }], - ]; - // Render the PR link whenever one exists β€” on both the βœ… success path and - // the ⚠️ shipped-but-stopped path β€” because the agent's own "PR opened" - // comment is not guaranteed to have fired, so this is the only guaranteed - // PR-link surface. The URL run carries an ``href`` so it renders as a - // clickable hyperlink β€” ADF does not auto-linkify a bare URL in a plain - // text node the way Linear's Markdown does, so without this the requester - // would have to copy-paste it. - if (args.prUrl) { - paragraphs.push([ - { text: 'PR: ' }, - { text: args.prUrl, href: args.prUrl }, - ]); - } - paragraphs.push([{ text: `task ${args.taskId}`, em: true }]); - return paragraphs; -} +export { renderJiraFinalStatusComment }; /** * Jira dispatcher β€” posts a deterministic final-status comment when a @@ -1697,11 +1611,18 @@ async function dispatchToJira(event: FanOutEvent): Promise { return; } - // Idempotency across partial-batch retries: Jira has no comment edit API, - // so a re-run (e.g. a sibling channel's infra rejection pushed the whole - // stream record into batchItemFailures) would post a duplicate. The - // marker is persisted after the first successful post below. - if (task.jira_final_comment_event_id) { + const iterationReplyId = task.channel_metadata?.iteration_reply_comment_id; + const isIteration = Boolean(task.channel_metadata?.trigger_comment_id); + const isOrchestratedIteration = + task.channel_metadata?.orchestration_iteration === 'true'; + // The reconciler owns an orchestrated iteration's terminal comment because it + // must settle that comment before continuing the dependent restack cascade. + if (isOrchestratedIteration) return; + + // Ordinary Jira terminal comments are created once. A re-run (for example, + // because a sibling channel rejected the record) would otherwise duplicate + // that comment, so persist a marker after the first successful post below. + if (!isIteration && task.jira_final_comment_event_id) { logger.info('[fanout/jira] final comment already posted β€” skipping (idempotent retry)', { event: 'fanout.jira.already_posted', task_id: task.task_id, @@ -1746,6 +1667,55 @@ async function dispatchToJira(event: FanOutEvent): Promise { errorTitle: classification?.title ?? null, }); + if (isIteration && iterationReplyId) { + const tableName = process.env.TASK_TABLE_NAME; + if (!tableName) return; + const claim = await claimTerminalReply( + ddb, + tableName, + task.task_id, + event.timestamp, + ); + if (!claim.won) return; + + const updateResult = await updateIssueCommentAdf( + { cloudId, registryTableName }, + issueKey, + iterationReplyId, + buildAdfDocument(paragraphs), + ); + if (updateResult.ok) { + logger.info('[fanout/jira] iteration comment matured in place', { + event: 'fanout.jira.iteration_matured', + task_id: task.task_id, + jira_issue_key: issueKey, + comment_id: iterationReplyId, + }); + return; + } + + const release = await releaseReplyClaim( + ddb, + tableName, + task.task_id, + claim.stamp, + ); + logger.warn('[fanout/jira] iteration comment update failed (non-fatal)', { + event: 'fanout.jira.iteration_update_failed', + task_id: task.task_id, + jira_issue_key: issueKey, + comment_id: iterationReplyId, + retryable: updateResult.retryable, + release, + }); + if (updateResult.retryable && release !== 'exhausted') { + throw new Error( + `[fanout/jira] transient Jira iteration update failure for task ${task.task_id}`, + ); + } + return; + } + const postResult = await postIssueCommentAdf( { cloudId, registryTableName }, issueKey, diff --git a/cdk/src/handlers/iteration-heartbeat-sweep.ts b/cdk/src/handlers/iteration-heartbeat-sweep.ts index 7a6d56df1..883a96837 100644 --- a/cdk/src/handlers/iteration-heartbeat-sweep.ts +++ b/cdk/src/handlers/iteration-heartbeat-sweep.ts @@ -35,16 +35,22 @@ * (reconciler) later overwrites the working line with βœ…/❌ as today. */ -import { DynamoDBClient, QueryCommand } from '@aws-sdk/client-dynamodb'; +import { DynamoDBClient, GetItemCommand, QueryCommand } from '@aws-sdk/client-dynamodb'; import { planHeartbeat, type HeartbeatTaskView } from './shared/iteration-heartbeat'; import { logger } from './shared/logger'; -import { makeLinearChannel } from './shared/orchestration-channel-linear'; +import { + channelForSource, + type ChannelRegistryTables, +} from './shared/orchestration-channel-factory'; import { makeClient } from './shared/ua'; const ddb = makeClient(DynamoDBClient); const TASK_TABLE = process.env.TASK_TABLE_NAME!; const STATUS_INDEX = process.env.TASK_STATUS_INDEX_NAME ?? 'StatusIndex'; -const WORKSPACE_REGISTRY_TABLE = process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME; +const CHANNEL_REGISTRY_TABLES: ChannelRegistryTables = { + linear: process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME, + jira: process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME, +}; /** Hard cap on tasks edited per sweep β€” a backstop against an unexpected flood. */ const MAX_EDITS_PER_SWEEP = 50; @@ -61,6 +67,7 @@ function toView(img: DdbMap): HeartbeatTaskView { ...(img.created_at?.S !== undefined && { createdAt: img.created_at.S }), ...(img.channel_source?.S !== undefined && { channelSource: img.channel_source.S }), ...(cm.linear_workspace_id?.S !== undefined && { linearWorkspaceId: cm.linear_workspace_id.S }), + ...(cm.jira_cloud_id?.S !== undefined && { jiraCloudId: cm.jira_cloud_id.S }), ...(cm.iteration_reply_comment_id?.S !== undefined && { iterationReplyCommentId: cm.iteration_reply_comment_id.S }), ...(cm.trigger_comment_id?.S !== undefined && { triggerCommentId: cm.trigger_comment_id.S }), // The issue the reply lives on. The orchestration path stamps @@ -68,8 +75,9 @@ function toView(img: DdbMap): HeartbeatTaskView { // the STANDALONE path stamps only ``linear_issue_id`` (the reply is on that // same issue). Fall back to it so standalone iterations get a heartbeat β€” // the reconciler's reply path uses the same precedence. - ...((cm.trigger_comment_issue_id?.S ?? cm.linear_issue_id?.S) !== undefined && { - triggerCommentIssueId: cm.trigger_comment_issue_id?.S ?? cm.linear_issue_id?.S, + ...((cm.trigger_comment_issue_id?.S ?? cm.linear_issue_id?.S ?? cm.jira_issue_key?.S) !== undefined && { + triggerCommentIssueId: + cm.trigger_comment_issue_id?.S ?? cm.linear_issue_id?.S ?? cm.jira_issue_key?.S, }), isIteration: cm.orchestration_iteration?.S === 'true', ...(prNumberRaw !== undefined && { prNumber: Number(prNumberRaw) }), @@ -77,6 +85,25 @@ function toView(img: DdbMap): HeartbeatTaskView { }; } +/** Strongly-consistent terminal guard immediately before a cosmetic edit. */ +async function terminalReplyAlreadyClaimed(taskId: string): Promise { + try { + const result = await ddb.send(new GetItemCommand({ + TableName: TASK_TABLE, + Key: { task_id: { S: taskId } }, + ProjectionExpression: 'ack_replied_at', + ConsistentRead: true, + })); + return Boolean(result.Item?.ack_replied_at?.S); + } catch (err) { + logger.warn('Heartbeat sweep: terminal-claim check failed (non-fatal)', { + task_id: taskId, + error: err instanceof Error ? err.message : String(err), + }); + return false; + } +} + /** Query every RUNNING task via the StatusIndex GSI (paginated). */ async function loadRunningTasks(): Promise { const items: DdbMap[] = []; @@ -102,8 +129,8 @@ async function loadRunningTasks(): Promise { * never wedge or alarm). */ export async function handler(): Promise { - if (!WORKSPACE_REGISTRY_TABLE) { - logger.info('Heartbeat sweep skipped β€” no Linear workspace registry configured'); + if (!CHANNEL_REGISTRY_TABLES.linear && !CHANNEL_REGISTRY_TABLES.jira) { + logger.info('Heartbeat sweep skipped β€” no channel credentials registry configured'); return; } @@ -129,29 +156,35 @@ export async function handler(): Promise { edited_cap: MAX_EDITS_PER_SWEEP, }); - // The reply edit goes through the surface-agnostic channel; only the surface - // this sweep reads from (a Linear iteration reply) picks the adapter. - const channel = makeLinearChannel(WORKSPACE_REGISTRY_TABLE); - let edited = 0; for (const plan of plans.slice(0, MAX_EDITS_PER_SWEEP)) { try { - await channel.upsertThreadedReply?.( - { issueId: plan.issueId, credentialsRef: plan.linearWorkspaceId }, - { commentId: plan.parentCommentId }, - plan.body, - { commentId: plan.replyId }, - { - // Keep any already-landed deploy-preview block (a heartbeat must never - // clobber the screenshot the webhook may have appended). - preservePreview: true, - // A liveness tick is the LEAST important writer of this reply: if the - // task has already settled, saying "working" again would un-settle it - // in the reader's eyes. - skipIfSettled: true, - }, - ); - edited += 1; + if (await terminalReplyAlreadyClaimed(plan.taskId)) continue; + const channel = channelForSource(plan.channelSource, CHANNEL_REGISTRY_TABLES); + if (!channel) continue; + const issue = { issueId: plan.issueId, credentialsRef: plan.credentialsRef }; + const ref = plan.channelSource === 'linear' && plan.parentCommentId + ? await channel.upsertThreadedReply?.( + issue, + { commentId: plan.parentCommentId }, + plan.body, + { commentId: plan.replyId }, + { + // Keep any already-landed deploy-preview block (a heartbeat must never + // clobber the screenshot the webhook may have appended). + preservePreview: true, + // A liveness tick is the LEAST important writer of this reply: if the + // task has already settled, saying "working" again would un-settle it + // in the reader's eyes. + skipIfSettled: true, + }, + ) + : await channel.upsertComment( + issue, + plan.body, + { commentId: plan.replyId }, + ); + if (ref) edited += 1; } catch (err) { logger.warn('Heartbeat sweep: reply edit failed (non-fatal)', { task_id: plan.taskId, diff --git a/cdk/src/handlers/jira-webhook-processor.ts b/cdk/src/handlers/jira-webhook-processor.ts index cee4dfe6e..016bb655c 100644 --- a/cdk/src/handlers/jira-webhook-processor.ts +++ b/cdk/src/handlers/jira-webhook-processor.ts @@ -20,7 +20,7 @@ import * as crypto from 'crypto'; import { BedrockRuntimeClient, ApplyGuardrailCommand } from '@aws-sdk/client-bedrock-runtime'; import { S3Client } from '@aws-sdk/client-s3'; -import { GetCommand, ScanCommand } from '@aws-sdk/lib-dynamodb'; +import { GetCommand, ScanCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb'; import { ulid } from 'ulid'; import type { ScreeningConfig } from './shared/attachment-screening'; import { @@ -28,6 +28,7 @@ import { parseCommentTrigger, } from './shared/comment-trigger'; import { createTaskCore } from './shared/create-task-core'; +import { renderMaturingReply } from './shared/iteration-reply'; import { extractDescriptionMarkdown } from './shared/jira-adf'; import { cleanupPreScreenedAttachments, @@ -937,6 +938,7 @@ async function handleCommentTrigger( ); const idempotencyKey = buildCommentIdempotencyKey(cloudId, issue.key, comment.id); const requestId = crypto.randomUUID(); + const taskId = ulid(); const result = await createTaskCore( { repo: priorTask.repo, @@ -949,6 +951,7 @@ async function handleCommentTrigger( channelSource: 'jira', channelMetadata, idempotencyKey, + taskId, }, requestId, ); @@ -981,12 +984,29 @@ async function handleCommentTrigger( return; } - await safeReportIssueFailure( - issue.key, - cloudId, - `πŸ‘€ ABCA accepted this follow-up and is updating PR #${prNumber}.`, - ); + try { + const reply = await makeJiraChannel(WORKSPACE_REGISTRY_TABLE).postComment( + { issueId: issue.key, credentialsRef: cloudId }, + renderMaturingReply({ state: 'on_it' }), + ); + if (reply?.commentId) { + await ddb.send(new UpdateCommand({ + TableName: TASK_TABLE, + Key: { task_id: taskId }, + UpdateExpression: 'SET channel_metadata.iteration_reply_comment_id = :comment_id', + ConditionExpression: 'attribute_exists(task_id)', + ExpressionAttributeValues: { ':comment_id': reply.commentId }, + })); + } + } catch (err) { + logger.warn('Jira iteration acknowledgement failed (non-fatal)', { + task_id: taskId, + issue_key: issue.key, + error: err instanceof Error ? err.message : String(err), + }); + } logger.info('Jira comment-triggered PR iteration task created', { + task_id: taskId, issue_key: issue.key, comment_id: comment.id, prior_task_id: priorTask.task_id, @@ -1014,6 +1034,8 @@ function buildIterationChannelMetadata( jira_site_url: siteUrl, jira_trigger_comment_id: commentId, jira_prior_task_id: priorTask.task_id, + trigger_comment_id: commentId, + trigger_comment_issue_id: issue.key, }; const projectKey = issue.fields?.project?.key ?? previous.jira_project_key; @@ -1028,8 +1050,6 @@ function buildIterationChannelMetadata( metadata.orchestration_id = previous.orchestration_id; metadata.orchestration_sub_issue_id = previous.orchestration_sub_issue_id; metadata.orchestration_iteration = 'true'; - metadata.trigger_comment_id = commentId; - metadata.trigger_comment_issue_id = issue.key; } return metadata; } diff --git a/cdk/src/handlers/orchestration-reconciler.ts b/cdk/src/handlers/orchestration-reconciler.ts index 8b5a082ae..20307de6a 100644 --- a/cdk/src/handlers/orchestration-reconciler.ts +++ b/cdk/src/handlers/orchestration-reconciler.ts @@ -44,10 +44,12 @@ import { } from '@aws-sdk/lib-dynamodb'; import type { DynamoDBBatchResponse, DynamoDBRecord, DynamoDBStreamEvent } from 'aws-lambda'; import { createTaskCore } from './shared/create-task-core'; +import { classifyError } from './shared/error-classifier'; import { renderFailureReply, renderPanelFailureReason } from './shared/failure-reply'; import { sumIterationCostForIssue } from './shared/iteration-cost'; import { isNoChangeIteration, renderMaturingReply } from './shared/iteration-reply'; import { claimTerminalReply, releaseReplyClaim } from './shared/iteration-reply-claim'; +import { renderJiraFinalStatusText } from './shared/jira-status-comment'; import { logger } from './shared/logger'; import type { Channel, IssueRef } from './shared/orchestration-channel'; import { channelForSource, type ChannelRegistryTables } from './shared/orchestration-channel-factory'; @@ -187,6 +189,10 @@ interface TerminalTaskEvent { readonly costUsd?: number; /** This iteration's wall-clock seconds β€” folded into the reply. */ readonly durationS?: number; + /** Agent turns consumed by this iteration. */ + readonly turns?: number; + /** Configured turn cap for this iteration. */ + readonly maxTurns?: number; } /** @@ -229,6 +235,10 @@ export function parseTerminalTaskRecord(record: DynamoDBRecord): TerminalTaskEve : (img.cost_usd?.S !== undefined ? Number(img.cost_usd.S) : undefined); const durationS = img.duration_s?.N !== undefined ? Number(img.duration_s.N) : (img.duration_s?.S !== undefined ? Number(img.duration_s.S) : undefined); + const turns = img.turns_attempted?.N !== undefined ? Number(img.turns_attempted.N) + : (img.turns_attempted?.S !== undefined ? Number(img.turns_attempted.S) : undefined); + const maxTurns = img.max_turns?.N !== undefined ? Number(img.max_turns.N) + : (img.max_turns?.S !== undefined ? Number(img.max_turns.S) : undefined); // Cascade marker: an iteration/restack task names the node it acted on // via channel_metadata. A restack task also carries @@ -264,6 +274,8 @@ export function parseTerminalTaskRecord(record: DynamoDBRecord): TerminalTaskEve ...(iterationReplyId !== undefined && { iterationReplyId }), ...(costUsd !== undefined && Number.isFinite(costUsd) && { costUsd }), ...(durationS !== undefined && Number.isFinite(durationS) && { durationS }), + ...(turns !== undefined && Number.isFinite(turns) && { turns }), + ...(maxTurns !== undefined && Number.isFinite(maxTurns) && { maxTurns }), }; } @@ -1143,7 +1155,7 @@ async function replyToIterationComment( // 'updated' (real edit) state folds the thumbnail in β€” a question didn't change UI. const isUpdated = !isNoChangeIteration(evt.codeChanged); const shot = isUpdated ? await reloadIterationScreenshot(evt.taskId) : { screenshotUrl: null, deployUrl: null }; - const body = succeeded + const linearBody = succeeded ? renderMaturingReply({ state: isNoChangeIteration(evt.codeChanged) ? 'answered' : 'updated', prNumber, @@ -1171,16 +1183,39 @@ async function replyToIterationComment( // threaded reply for older tasks that captured no reply id. // preservePreview: converge with the screenshot webhook's async `[preview]` // append so this terminal re-render doesn't clobber it. - const reply = await channel.upsertThreadedReply?.( - issueRef(replyIssueId, workspaceId), - { commentId }, - body, - evt.iterationReplyId ? { commentId: evt.iterationReplyId } : undefined, - // repairIfOverwritten: a progress render delivered at the same moment can land - // on top of this outcome, and the surface has no conditional update to prevent - // it β€” so re-assert the outcome if that happened. - { preservePreview: true, repairIfOverwritten: true }, - ); + const target = issueRef(replyIssueId, workspaceId); + const existing = evt.iterationReplyId + ? { commentId: evt.iterationReplyId } + : undefined; + const reply = channel.kind === 'jira' + ? await channel.upsertComment( + target, + renderJiraFinalStatusText({ + eventType: succeeded + ? 'task_completed' + : (evt.status === TaskStatus.COMPLETED + ? 'task_failed' + : `task_${evt.status.toLowerCase()}`), + prUrl, + costUsd: evt.costUsd ?? null, + turns: evt.turns ?? null, + maxTurns: evt.maxTurns ?? null, + durationS: evt.durationS ?? null, + taskId: evt.taskId, + errorTitle: classifyError(evt.errorMessage)?.title ?? null, + }), + existing, + ) + : await channel.upsertThreadedReply?.( + target, + { commentId }, + linearBody, + existing, + // repairIfOverwritten: a progress render delivered at the same moment can land + // on top of this outcome, and the surface has no conditional update to prevent + // it β€” so re-assert the outcome if that happened. + { preservePreview: true, repairIfOverwritten: true }, + ); // A surface that cannot mature a reply at all (the capability is optional) // legitimately returns undefined; only an attempted-and-failed reply β€” null β€” // means the outcome went unsaid. diff --git a/cdk/src/handlers/shared/iteration-heartbeat.ts b/cdk/src/handlers/shared/iteration-heartbeat.ts index a59c50cae..5bdd64542 100644 --- a/cdk/src/handlers/shared/iteration-heartbeat.ts +++ b/cdk/src/handlers/shared/iteration-heartbeat.ts @@ -52,10 +52,12 @@ export interface HeartbeatTaskView { readonly status: string; /** ISO timestamp the task was created (drives elapsed). */ readonly createdAt?: string; - /** Trigger channel β€” only 'linear' is wired for the reply edit. */ + /** Trigger channel. Linear and Jira support maturing iteration comments. */ readonly channelSource?: string; /** Linear workspace id (for the per-workspace OAuth token). */ readonly linearWorkspaceId?: string; + /** Jira cloud id (for the per-tenant OAuth token). */ + readonly jiraCloudId?: string; /** The maturing reply comment id stamped at trigger time. */ readonly iterationReplyCommentId?: string; /** The human comment that triggered the iteration (reply parent). */ @@ -91,9 +93,11 @@ export interface HeartbeatTaskView { /** What the sweep should do for one task. */ export interface HeartbeatPlan { readonly taskId: string; - readonly linearWorkspaceId: string; + readonly channelSource: 'linear' | 'jira'; + readonly credentialsRef: string; readonly issueId: string; - readonly parentCommentId: string; + /** Linear thread root. Jira's maturing comment is top-level. */ + readonly parentCommentId?: string; readonly replyId: string; readonly body: string; readonly elapsedS: number; @@ -108,12 +112,13 @@ function parseIso(ts: string | undefined): number | null { /** * Decide whether to heartbeat ONE task, and render the new reply body. Returns - * null when the task is not eligible (not a RUNNING linear iteration with a + * null when the task is not eligible (not a RUNNING Linear/Jira iteration with a * reply to edit, or not yet past the elapsed floor). Pure β€” ``nowMs`` injected. */ export function planHeartbeat(task: HeartbeatTaskView, nowMs: number): HeartbeatPlan | null { if (task.status !== 'RUNNING') return null; - if ((task.channelSource ?? 'linear') !== 'linear') return null; + const channelSource = task.channelSource ?? 'linear'; + if (channelSource !== 'linear' && channelSource !== 'jira') return null; // Eligibility = "this task has a maturing Linear reply to keep alive". That's // exactly the set of comment-triggered iterations β€” BOTH orchestration and @@ -121,8 +126,20 @@ export function planHeartbeat(task: HeartbeatTaskView, nowMs: number): Heartbeat // but still has the reply, and the black-box case observed was standalone). So we // key on the reply-routing fields, NOT ``isIteration``. A first-run / non-PR // task has no ``iteration_reply_comment_id`` and is correctly skipped. - const { linearWorkspaceId, iterationReplyCommentId, triggerCommentId, triggerCommentIssueId } = task; - if (!linearWorkspaceId || !iterationReplyCommentId || !triggerCommentId || !triggerCommentIssueId) { + const { + linearWorkspaceId, + jiraCloudId, + iterationReplyCommentId, + triggerCommentId, + triggerCommentIssueId, + } = task; + const credentialsRef = channelSource === 'jira' ? jiraCloudId : linearWorkspaceId; + if ( + !credentialsRef + || !iterationReplyCommentId + || !triggerCommentIssueId + || (channelSource === 'linear' && !triggerCommentId) + ) { return null; } @@ -141,9 +158,10 @@ export function planHeartbeat(task: HeartbeatTaskView, nowMs: number): Heartbeat return { taskId: task.taskId, - linearWorkspaceId, + channelSource, + credentialsRef, issueId: triggerCommentIssueId, - parentCommentId: triggerCommentId, + ...(triggerCommentId && { parentCommentId: triggerCommentId }), replyId: iterationReplyCommentId, body, elapsedS, diff --git a/cdk/src/handlers/shared/jira-app-actor.ts b/cdk/src/handlers/shared/jira-app-actor.ts index a17552fa3..e7923629d 100644 --- a/cdk/src/handlers/shared/jira-app-actor.ts +++ b/cdk/src/handlers/shared/jira-app-actor.ts @@ -27,6 +27,7 @@ export const JIRA_APP_ACTOR_MIN_SECRET_LENGTH = sharedConstants.jira_app_actor.m const PROXY_ERROR_CODES = new Set([ 'cloud_id_required', 'invalid_comment_request', + 'invalid_update_comment_request', 'invalid_issue_key', 'invalid_json', 'invalid_payload', @@ -46,9 +47,10 @@ export interface JiraAppActorConfig { export interface JiraAppActorRequest { readonly version: 1; - readonly operation: 'comment' | 'get_transitions' | 'transition' | 'identity'; + readonly operation: 'comment' | 'update_comment' | 'get_transitions' | 'transition' | 'identity'; readonly cloud_id: string; readonly issue_key?: string; + readonly comment_id?: string; readonly body?: Record; readonly transition_id?: string; } diff --git a/cdk/src/handlers/shared/jira-feedback.ts b/cdk/src/handlers/shared/jira-feedback.ts index 90f556f73..2ac707e25 100644 --- a/cdk/src/handlers/shared/jira-feedback.ts +++ b/cdk/src/handlers/shared/jira-feedback.ts @@ -133,11 +133,15 @@ export function buildAdfDocument(paragraphs: ReadonlyArray): Recor * (network error, request timeout, HTTP 5xx/429) β€” where a Lambda retry * may genuinely succeed β€” from terminal ones (bad issue id, revoked * credential, malformed request) where it cannot. The best-effort - * boolean-returning {@link postIssueComment} collapses this to - * ``ok``/``!ok``; the fan-out dispatcher branches on ``retryable`` to - * decide whether to escalate to the partial-batch retry path (#573). + * {@link postIssueComment} returns the created comment ID or null; the fan-out + * dispatcher uses this classified result to decide whether to escalate to the + * partial-batch retry path (#573). */ export type JiraPostResult = + | { readonly ok: true; readonly commentId: string } + | { readonly ok: false; readonly retryable: boolean }; + +export type JiraUpdateResult = | { readonly ok: true } | { readonly ok: false; readonly retryable: boolean }; @@ -148,29 +152,31 @@ export type JiraPostResult = * classified caller ({@link postCommentWithResult}) can tell a transient * 5xx/429/network blip from a terminal 4xx. */ -type PostOutcome = - | { readonly kind: 'ok' } +type WriteOutcome = + | { readonly kind: 'ok'; readonly responseBody: string } | { readonly kind: 'auth' } | { readonly kind: 'error'; readonly retryable: boolean }; -async function postComment( +async function writeComment( accessToken: string, cloudId: string, issueIdOrKey: string, body: Record, -): Promise { + commentId?: string, +): Promise { // The 3LO token (audience=api.atlassian.com) is only valid against the // gateway base scoped by cloudId β€” see JIRA_API_BASE. Posting to the raw // site host (`*.atlassian.net`) would 401. Both path segments are // URL-encoded for defense-in-depth: cloudId is registry-sourced (a stored // tenant UUID), but encoding it keeps a malformed/compromised row from // injecting extra path segments into the gateway URL. - const url = `${JIRA_API_BASE}/${encodeURIComponent(cloudId)}/rest/api/3/issue/${encodeURIComponent(issueIdOrKey)}/comment`; + const commentPath = commentId ? `/comment/${encodeURIComponent(commentId)}` : '/comment'; + const url = `${JIRA_API_BASE}/${encodeURIComponent(cloudId)}/rest/api/3/issue/${encodeURIComponent(issueIdOrKey)}${commentPath}`; const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); try { const resp = await fetch(url, { - method: 'POST', + method: commentId ? 'PUT' : 'POST', headers: { 'Authorization': `Bearer ${accessToken}`, 'Content-Type': 'application/json', @@ -179,7 +185,8 @@ async function postComment( body: JSON.stringify({ body }), signal: controller.signal, }); - if (resp.ok) return { kind: 'ok' }; + const responseBody = await resp.text(); + if (resp.ok) return { kind: 'ok', responseBody }; // 401/403 are recoverable via a forced refresh: the stored access token // may be dead despite a not-yet-reached `expires_at` (server-side // revocation, scope re-issue, or a value cached past its out-of-band @@ -255,9 +262,9 @@ export async function postIssueComment( ctx: JiraFeedbackContext, issueIdOrKey: string, body: string, -): Promise { +): Promise { const result = await postCommentWithResult(ctx, issueIdOrKey, toAdfDocument(body)); - return result.ok; + return result.ok ? result.commentId : null; } /** @@ -295,13 +302,16 @@ async function postCommentWithResult( issue_key: issueIdOrKey, body, }); - return appResult.ok - ? { ok: true } - : { ok: false, retryable: appResult.retryable }; + return createdCommentResult(appResult); } - const outcome = await postComment(resolved.accessToken, ctx.cloudId, issueIdOrKey, body); - if (outcome.kind === 'ok') return { ok: true }; + const outcome = await writeComment(resolved.accessToken, ctx.cloudId, issueIdOrKey, body); + if (outcome.kind === 'ok') { + return createdCommentResult({ + ok: true, + body: outcome.responseBody, + }); + } if (outcome.kind === 'error') return { ok: false, retryable: outcome.retryable }; // outcome.kind === 'auth': the stored access token was rejected. Force a @@ -321,9 +331,7 @@ async function postCommentWithResult( issue_key: issueIdOrKey, body, }); - return appResult.ok - ? { ok: true } - : { ok: false, retryable: appResult.retryable }; + return createdCommentResult(appResult); } // If the refresh handed back the same access token, the retry can only // reproduce the 401 β€” skip the redundant network call. @@ -334,8 +342,13 @@ async function postCommentWithResult( }); return { ok: false, retryable: false }; } - const retryOutcome = await postComment(refreshed.accessToken, ctx.cloudId, issueIdOrKey, body); - if (retryOutcome.kind === 'ok') return { ok: true }; + const retryOutcome = await writeComment(refreshed.accessToken, ctx.cloudId, issueIdOrKey, body); + if (retryOutcome.kind === 'ok') { + return createdCommentResult({ + ok: true, + body: retryOutcome.responseBody, + }); + } // A second auth rejection means the credential is genuinely unusable β€” // terminal. A transient error on the retry stays retryable so the // dispatcher can escalate for a Lambda retry. @@ -343,6 +356,119 @@ async function postCommentWithResult( return { ok: false, retryable: false }; } +function createdCommentResult( + result: { readonly ok: true; readonly body: string } + | { readonly ok: false; readonly retryable: boolean }, +): JiraPostResult { + if (!result.ok) return { ok: false, retryable: result.retryable }; + try { + const value = JSON.parse(result.body) as { id?: unknown }; + const commentId = typeof value.id === 'string' + ? value.id + : (typeof value.id === 'number' ? String(value.id) : ''); + if (commentId) return { ok: true, commentId }; + } catch { + // Handled by the common missing-id warning below. + } + logger.warn('Jira comment create succeeded without a usable comment id'); + return { ok: false, retryable: false }; +} + +/** + * Update an existing Jira comment in place. Returns true on success and false + * on any failure. Like comment creation, this is advisory and never throws. + */ +export async function updateIssueComment( + ctx: JiraFeedbackContext, + issueIdOrKey: string, + commentId: string, + body: string, +): Promise { + const result = await updateIssueCommentAdf( + ctx, + issueIdOrKey, + commentId, + toAdfDocument(body), + ); + return result.ok; +} + +/** Update an existing Jira comment with a pre-built ADF document. */ +export async function updateIssueCommentAdf( + ctx: JiraFeedbackContext, + issueIdOrKey: string, + commentId: string, + body: Record, +): Promise { + const resolved = await resolveTenantAuth(ctx); + if (!resolved) return { ok: false, retryable: false }; + + if (resolved.kind === 'app') { + const appResult = await requestJiraAppActor(resolved.appActor, { + version: 1, + operation: 'update_comment', + cloud_id: ctx.cloudId, + issue_key: issueIdOrKey, + comment_id: commentId, + body, + }); + return appResult.ok + ? { ok: true } + : { ok: false, retryable: appResult.retryable }; + } + + const outcome = await writeComment( + resolved.accessToken, + ctx.cloudId, + issueIdOrKey, + body, + commentId, + ); + if (outcome.kind === 'ok') return { ok: true }; + if (outcome.kind === 'error') return { ok: false, retryable: outcome.retryable }; + + logger.info('Jira feedback got auth rejection β€” forcing token refresh and retrying once', { + jira_cloud_id: ctx.cloudId, + issue_id_or_key: issueIdOrKey, + comment_id: commentId, + }); + const refreshed = await resolveTenantAuth(ctx, true); + if (!refreshed) return { ok: false, retryable: false }; + if (refreshed.kind === 'app') { + const appResult = await requestJiraAppActor(refreshed.appActor, { + version: 1, + operation: 'update_comment', + cloud_id: ctx.cloudId, + issue_key: issueIdOrKey, + comment_id: commentId, + body, + }); + return appResult.ok + ? { ok: true } + : { ok: false, retryable: appResult.retryable }; + } + if (refreshed.accessToken === resolved.accessToken) { + logger.warn('Jira feedback refresh returned an unchanged token β€” not retrying', { + jira_cloud_id: ctx.cloudId, + issue_id_or_key: issueIdOrKey, + comment_id: commentId, + }); + return { ok: false, retryable: false }; + } + const retryOutcome = await writeComment( + refreshed.accessToken, + ctx.cloudId, + issueIdOrKey, + body, + commentId, + ); + if (retryOutcome.kind === 'ok') return { ok: true }; + if (retryOutcome.kind === 'error') { + return { ok: false, retryable: retryOutcome.retryable }; + } + return { ok: false, retryable: false }; +} + /** * Post a feedback comment with the failure marker (❌) folded into the * message text. Mirrors `linear-feedback.reportIssueFailure` semantics diff --git a/cdk/src/handlers/shared/jira-status-comment.ts b/cdk/src/handlers/shared/jira-status-comment.ts new file mode 100644 index 000000000..86f265b1f --- /dev/null +++ b/cdk/src/handlers/shared/jira-status-comment.ts @@ -0,0 +1,92 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import type { AdfParagraph, AdfTextRun } from './jira-feedback'; + +export interface JiraFinalStatusInput { + readonly eventType: string; + readonly prUrl: string | null; + readonly costUsd: number | null; + readonly turns: number | null; + readonly maxTurns: number | null; + readonly durationS: number | null; + readonly taskId: string; + readonly errorTitle: string | null; +} + +function formatDuration(seconds: number): string { + const rounded = Math.max(0, Math.round(seconds)); + const minutes = Math.floor(rounded / 60); + const remainingSeconds = rounded % 60; + if (minutes === 0) return `${remainingSeconds}s`; + return remainingSeconds === 0 + ? `${minutes}m` + : `${minutes}m ${remainingSeconds}s`; +} + +/** Render a Jira terminal status as ADF paragraphs. */ +export function renderJiraFinalStatusComment( + args: JiraFinalStatusInput, +): ReadonlyArray { + const isCompleted = args.eventType === 'task_completed'; + const shippedDespiteFailure = !isCompleted && args.prUrl != null; + + let headerRuns: AdfTextRun[]; + if (isCompleted) { + headerRuns = [{ text: 'βœ… Task completed', strong: true }]; + } else if (shippedDespiteFailure) { + const reason = args.errorTitle ? ` β€” ${args.errorTitle}` : ''; + headerRuns = [ + { text: `⚠️ Shipped a PR but stopped early${reason}`, strong: true }, + { text: ' β€” review and decide if more work is needed' }, + ]; + } else { + const subtype = args.eventType.replace(/^task_/, '').replace(/_/g, ' '); + const reason = args.errorTitle ? `: ${args.errorTitle}` : ''; + headerRuns = [{ text: `❌ Task ${subtype}${reason}`, strong: true }]; + } + + const costStr = args.costUsd != null ? `$${args.costUsd.toFixed(2)}` : 'β€”'; + const turnsStr = args.turns != null + ? `${args.turns}${args.maxTurns != null ? ` / ${args.maxTurns}` : ''}` + : 'β€”'; + const durationStr = args.durationS != null + ? formatDuration(args.durationS) + : 'β€”'; + + const paragraphs: AdfParagraph[] = [ + headerRuns, + [{ text: `cost: ${costStr} β€’ turns: ${turnsStr} β€’ duration: ${durationStr}` }], + ]; + if (args.prUrl) { + paragraphs.push([ + { text: 'PR: ' }, + { text: args.prUrl, href: args.prUrl }, + ]); + } + paragraphs.push([{ text: `task ${args.taskId}`, em: true }]); + return paragraphs; +} + +/** Plain-text equivalent for the channel adapter's string comment contract. */ +export function renderJiraFinalStatusText(args: JiraFinalStatusInput): string { + return renderJiraFinalStatusComment(args) + .map((paragraph) => paragraph.map((run) => run.text).join('')) + .join('\n'); +} diff --git a/cdk/src/handlers/shared/orchestration-channel-jira.ts b/cdk/src/handlers/shared/orchestration-channel-jira.ts index 2103f0881..5b48b8245 100644 --- a/cdk/src/handlers/shared/orchestration-channel-jira.ts +++ b/cdk/src/handlers/shared/orchestration-channel-jira.ts @@ -39,6 +39,7 @@ import { postIssueComment, reportIssueFailure, + updateIssueComment, type JiraFeedbackContext, } from './jira-feedback'; import { type Channel, type IssueRef } from './orchestration-channel'; @@ -58,19 +59,22 @@ export function makeJiraChannel(registryTableName: string): Channel { kind: 'jira', async postComment(issue, body) { - const ok = await postIssueComment(ctxFor(issue), issue.issueId, body); - // The Jira helper doesn't return the new comment id, so an edit-in-place - // isn't possible yet (see upsertComment); report success/failure only. - return ok ? { commentId: '' } : null; + const commentId = await postIssueComment(ctxFor(issue), issue.issueId, body); + return commentId ? { commentId } : null; }, - async upsertComment(issue, body) { - // Jira has no comment-update helper wired today, so a repeated "upsert" - // posts a fresh comment rather than editing in place. Behaviourally safe - // (the reviewer sees the latest state); a true edit-in-place needs a Jira - // update-comment call and a returned comment id β€” tracked as a follow-up. - const ok = await postIssueComment(ctxFor(issue), issue.issueId, body); - return ok ? { commentId: '' } : null; + async upsertComment(issue, body, existing) { + if (!existing?.commentId) { + const commentId = await postIssueComment(ctxFor(issue), issue.issueId, body); + return commentId ? { commentId } : null; + } + const ok = await updateIssueComment( + ctxFor(issue), + issue.issueId, + existing.commentId, + body, + ); + return ok ? existing : null; }, async reportFailure(issue, message) { diff --git a/cdk/src/handlers/shared/types.ts b/cdk/src/handlers/shared/types.ts index bb3243e07..549204d47 100644 --- a/cdk/src/handlers/shared/types.ts +++ b/cdk/src/handlers/shared/types.ts @@ -229,13 +229,11 @@ export interface TaskRecord { */ readonly linear_pr_comment_event_id?: string; /** - * Event ID of the terminal event whose Jira final-status comment was - * successfully posted (fan-out plane). Jira has no comment edit API, - * so the dispatcher is post-once: this marker makes the post - * idempotent across partial-batch Lambda retries (a sibling channel's - * infra rejection re-runs every dispatcher for the record). The Jira - * analogue of ``linear_final_comment_event_id``. Absent until the - * first successful post. + * Event ID of the terminal event whose ordinary Jira final-status comment + * was successfully posted (fan-out plane). This marker makes that create + * idempotent across partial-batch Lambda retries. Comment-triggered + * iterations edit their stored status comment instead and do not use this + * marker. Absent until the first successful ordinary-task post. */ readonly jira_final_comment_event_id?: string; readonly attachments?: AttachmentRecord[]; diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 93a500646..5a1cc685f 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -1082,12 +1082,12 @@ export class AgentStack extends Stack { })); // Mid-run liveness heartbeat. A scheduled sweep edits the maturing - // Linear reply of RUNNING comment-triggered iterations to show elapsed time + // Linear/Jira comment of RUNNING comment-triggered iterations to show elapsed time // ("πŸ”„ Working … _8m elapsed_") so a long run isn't a silent black box // (observed in practice: a run went 22 minutes with no visible output). - // Needs the workspace registry + per-workspace - // linear-oauth secret read to resolve the outbound token (same as the - // reconciler's reply path). Read-only on the TaskTable. + // Needs each surface registry and scoped OAuth-secret access to resolve + // outbound credentials (same as the reconciler's reply path). Read-only on + // the TaskTable. const iterationHeartbeat = new IterationHeartbeat(this, 'IterationHeartbeat', { taskTable: taskTable.table, }); @@ -1144,6 +1144,26 @@ export class AgentStack extends Stack { attachmentsBucket: attachmentsBucket.bucket, }); + // Add Jira to the channel-neutral heartbeat sweep. Token resolution can + // refresh an expiring OAuth bundle, so this trusted Lambda needs scoped + // Get+Put on the per-tenant secret prefix as well as registry-table read. + jiraIntegration.workspaceRegistryTable.grantReadData(iterationHeartbeat.fn); + iterationHeartbeat.fn.addEnvironment( + 'JIRA_WORKSPACE_REGISTRY_TABLE_NAME', + jiraIntegration.workspaceRegistryTable.tableName, + ); + iterationHeartbeat.fn.addToRolePolicy(new iam.PolicyStatement({ + actions: ['secretsmanager:GetSecretValue', 'secretsmanager:PutSecretValue'], + resources: [ + Stack.of(this).formatArn({ + service: 'secretsmanager', + resource: 'secret', + arnFormat: ArnFormat.COLON_RESOURCE_NAME, + resourceName: 'bgagent-jira-oauth-*', + }), + ], + })); + // Agent runtime reads the per-tenant Jira OAuth token directly from // Secrets Manager. The CLI (`bgagent jira setup`) creates // `bgagent-jira-oauth-` secrets at install time; the secret diff --git a/cdk/test/handlers/fanout-task-events.test.ts b/cdk/test/handlers/fanout-task-events.test.ts index 8125a12d6..5c8ce5286 100644 --- a/cdk/test/handlers/fanout-task-events.test.ts +++ b/cdk/test/handlers/fanout-task-events.test.ts @@ -142,6 +142,7 @@ jest.mock('../../src/handlers/shared/linear-feedback', () => ({ // so tests can flatten them back to text (see `adfText`) instead of walking // real ADF nodes. Default ``{ ok: true }`` drives the happy path. const mockPostIssueCommentAdf: jest.Mock = jest.fn().mockResolvedValue({ ok: true }); +const mockUpdateIssueCommentAdf: jest.Mock = jest.fn().mockResolvedValue({ ok: true }); const mockBuildAdfDocument: jest.Mock = jest.fn( (paragraphs: ReadonlyArray>) => ({ _adf: paragraphs }), ); @@ -151,6 +152,12 @@ jest.mock('../../src/handlers/shared/jira-feedback', () => ({ issueKey: string, body: unknown, ) => mockPostIssueCommentAdf(ctx, issueKey, body), + updateIssueCommentAdf: ( + ctx: { cloudId: string; registryTableName: string }, + issueKey: string, + commentId: string, + body: unknown, + ) => mockUpdateIssueCommentAdf(ctx, issueKey, commentId, body), buildAdfDocument: (paragraphs: ReadonlyArray>) => mockBuildAdfDocument(paragraphs), })); @@ -2212,6 +2219,7 @@ describe('fanout-task-events: Jira dispatcher', () => { beforeEach(() => { mockDdbSend.mockReset().mockResolvedValue({ Item: undefined }); mockPostIssueCommentAdf.mockReset().mockResolvedValue({ ok: true }); + mockUpdateIssueCommentAdf.mockReset().mockResolvedValue({ ok: true }); mockBuildAdfDocument.mockClear(); // Keep the sibling dispatchers quiet so they don't reject the batch. mockDispatchSlackEvent.mockReset().mockResolvedValue(undefined); @@ -2297,6 +2305,144 @@ describe('fanout-task-events: Jira dispatcher', () => { expect(adfText(body)).toContain('❌'); }); + test('standalone iteration matures its stored comment instead of posting a new one', async () => { + mockGet({ + ...TASK_RECORD_JIRA, + channel_metadata: { + ...TASK_RECORD_JIRA.channel_metadata, + trigger_comment_id: 'human-comment-1', + trigger_comment_issue_id: 'KAN-42', + iteration_reply_comment_id: 'status-comment-1', + }, + }); + + await handler({ Records: [mkEvent('task_completed', 't-jira')] }); + + expect(mockUpdateIssueCommentAdf).toHaveBeenCalledTimes(1); + const [ctx, issueKey, commentId, body] = mockUpdateIssueCommentAdf.mock.calls[0]; + expect(ctx).toEqual({ + cloudId: 'cloud-uuid-acme', + registryTableName: 'JiraWorkspaceRegistry', + }); + expect(issueKey).toBe('KAN-42'); + expect(commentId).toBe('status-comment-1'); + expect(adfText(body)).toContain('cost: $0.55 β€’ turns: 27 / 100 β€’ duration: 3m 41s'); + expect(adfText(body)).toContain('task t-jira'); + expect(mockPostIssueCommentAdf).not.toHaveBeenCalled(); + }); + + test('orchestrated iteration leaves terminal maturation to the reconciler', async () => { + mockGet({ + ...TASK_RECORD_JIRA, + channel_metadata: { + ...TASK_RECORD_JIRA.channel_metadata, + trigger_comment_id: 'human-comment-1', + iteration_reply_comment_id: 'status-comment-1', + orchestration_iteration: 'true', + }, + }); + + await handler({ Records: [mkEvent('task_completed', 't-jira')] }); + + expect(mockUpdateIssueCommentAdf).not.toHaveBeenCalled(); + expect(mockPostIssueCommentAdf).not.toHaveBeenCalled(); + }); + + test('redelivered standalone iteration loses the terminal claim and does not edit twice', async () => { + const task = { + ...TASK_RECORD_JIRA, + channel_metadata: { + ...TASK_RECORD_JIRA.channel_metadata, + trigger_comment_id: 'human-comment-1', + iteration_reply_comment_id: 'status-comment-1', + }, + }; + mockDdbSend.mockReset().mockImplementation((cmd: { _type?: string }) => { + if (cmd?._type === 'Get') return Promise.resolve({ Item: task }); + if (cmd?._type === 'Update') { + return Promise.reject(Object.assign(new Error('claimed'), { + name: 'ConditionalCheckFailedException', + })); + } + return Promise.resolve({}); + }); + + await handler({ Records: [mkEvent('task_completed', 't-jira')] }); + + expect(mockUpdateIssueCommentAdf).not.toHaveBeenCalled(); + }); + + test('failed standalone iteration update releases its terminal claim', async () => { + const task = { + ...TASK_RECORD_JIRA, + channel_metadata: { + ...TASK_RECORD_JIRA.channel_metadata, + trigger_comment_id: 'human-comment-1', + iteration_reply_comment_id: 'status-comment-1', + }, + }; + mockGet(task); + mockUpdateIssueCommentAdf.mockResolvedValue({ + ok: false, + retryable: false, + }); + + const result = await handler({ + Records: [mkEvent('task_completed', 't-jira')], + }); + + expect(result).toEqual({ batchItemFailures: [] }); + const claims = mockDdbSend.mock.calls + .map(([command]) => command as { + _type?: string; + input?: { + UpdateExpression?: string; + ExpressionAttributeValues?: Record; + }; + }) + .filter(command => + command._type === 'Update' + && /ack_replied_at/.test(command.input?.UpdateExpression ?? ''), + ); + expect(claims[0].input?.UpdateExpression).toBe('SET ack_replied_at = :now'); + expect(claims[1].input?.UpdateExpression).toContain('REMOVE ack_replied_at'); + expect(claims[1].input?.ExpressionAttributeValues?.[':ours']) + .toBe(claims[0].input?.ExpressionAttributeValues?.[':now']); + }); + + test('transient standalone iteration update failure releases the claim and retries the record', async () => { + const task = { + ...TASK_RECORD_JIRA, + channel_metadata: { + ...TASK_RECORD_JIRA.channel_metadata, + trigger_comment_id: 'human-comment-1', + iteration_reply_comment_id: 'status-comment-1', + }, + }; + mockGet(task); + mockUpdateIssueCommentAdf.mockResolvedValue({ + ok: false, + retryable: true, + }); + const record = mkEvent('task_completed', 't-jira'); + + const result = await handler({ Records: [record] }); + + expect(result.batchItemFailures).toEqual([ + { itemIdentifier: record.eventID }, + ]); + const releases = mockDdbSend.mock.calls + .map(([command]) => command as { + _type?: string; + input?: { UpdateExpression?: string }; + }) + .filter(command => + command._type === 'Update' + && /REMOVE ack_replied_at/.test(command.input?.UpdateExpression ?? ''), + ); + expect(releases).toHaveLength(1); + }); + test('non-Jira task short-circuits β€” postIssueCommentAdf never called', async () => { mockGet({ ...TASK_RECORD_JIRA, channel_source: 'github' }); diff --git a/cdk/test/handlers/iteration-heartbeat-sweep.test.ts b/cdk/test/handlers/iteration-heartbeat-sweep.test.ts index 8c847790c..ff5d1b9c0 100644 --- a/cdk/test/handlers/iteration-heartbeat-sweep.test.ts +++ b/cdk/test/handlers/iteration-heartbeat-sweep.test.ts @@ -25,6 +25,7 @@ const ddbSend = jest.fn(); jest.mock('@aws-sdk/client-dynamodb', () => ({ DynamoDBClient: jest.fn(() => ({ send: ddbSend })), QueryCommand: jest.fn((input: unknown) => ({ _type: 'Query', input })), + GetItemCommand: jest.fn((input: unknown) => ({ _type: 'GetItem', input })), })); // Mock at the per-surface helper, NOT the channel: the real Linear adapter runs, @@ -34,6 +35,11 @@ jest.mock('../../src/handlers/shared/linear-feedback', () => ({ upsertThreadedReply: (...args: unknown[]) => upsertThreadedReplyMock(...args), })); +const updateIssueCommentMock = jest.fn(); +jest.mock('../../src/handlers/shared/jira-feedback', () => ({ + updateIssueComment: (...args: unknown[]) => updateIssueCommentMock(...args), +})); + jest.mock('../../src/handlers/shared/logger', () => ({ logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })); @@ -41,6 +47,7 @@ jest.mock('../../src/handlers/shared/logger', () => ({ const REGISTRY = 'LinearWorkspaceRegistry'; process.env.TASK_TABLE_NAME = 'TaskTable'; process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = REGISTRY; +process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraWorkspaceRegistry'; // The sweep compares each task's created_at against the wall clock, so pin now. const NOW = Date.parse('2026-06-29T13:30:00Z'); @@ -68,11 +75,34 @@ function runningTask(overrides: { taskId?: string; createdAt?: string } = {}) { }; } +function runningJiraTask() { + return { + task_id: { S: 'task-jira' }, + status: { S: 'RUNNING' }, + created_at: { S: '2026-06-29T13:20:00Z' }, + channel_source: { S: 'jira' }, + pr_number: { N: '42' }, + channel_metadata: { + M: { + jira_cloud_id: { S: 'cloud-1' }, + jira_issue_key: { S: 'ENG-42' }, + iteration_reply_comment_id: { S: 'jira-reply-1' }, + trigger_comment_id: { S: 'jira-trigger-1' }, + }, + }, + }; +} + beforeEach(() => { jest.clearAllMocks(); jest.spyOn(Date, 'now').mockReturnValue(NOW); upsertThreadedReplyMock.mockResolvedValue('reply-1'); - ddbSend.mockResolvedValue({ Items: [runningTask()] }); + updateIssueCommentMock.mockResolvedValue(true); + ddbSend.mockImplementation((command: { _type?: string }) => ( + command._type === 'Query' + ? Promise.resolve({ Items: [runningTask()] }) + : Promise.resolve({}) + )); }); afterEach(() => jest.restoreAllMocks()); @@ -109,10 +139,43 @@ describe('iteration heartbeat sweep', () => { expect(upsertThreadedReplyMock).not.toHaveBeenCalled(); }); - test('one task\'s edit failure does not stop the rest of the sweep', async () => { - ddbSend.mockResolvedValue({ - Items: [runningTask({ taskId: 'task-1' }), runningTask({ taskId: 'task-2' })], + test('edits a Jira iteration status comment through the Jira adapter', async () => { + ddbSend.mockImplementation((command: { _type?: string }) => ( + command._type === 'Query' + ? Promise.resolve({ Items: [runningJiraTask()] }) + : Promise.resolve({}) + )); + + await handler(); + + expect(updateIssueCommentMock).toHaveBeenCalledWith( + { cloudId: 'cloud-1', registryTableName: 'JiraWorkspaceRegistry' }, + 'ENG-42', + 'jira-reply-1', + expect.stringContaining('πŸ”„ Working'), + ); + expect(upsertThreadedReplyMock).not.toHaveBeenCalled(); + }); + + test('a terminal claim prevents a late Jira heartbeat from regressing the comment', async () => { + ddbSend.mockImplementation((command: { _type?: string }) => { + if (command._type === 'Query') return Promise.resolve({ Items: [runningJiraTask()] }); + return Promise.resolve({ Item: { ack_replied_at: { S: '2026-06-29T13:29:59Z' } } }); }); + + await handler(); + + expect(updateIssueCommentMock).not.toHaveBeenCalled(); + }); + + test('one task\'s edit failure does not stop the rest of the sweep', async () => { + ddbSend.mockImplementation((command: { _type?: string }) => ( + command._type === 'Query' + ? Promise.resolve({ + Items: [runningTask({ taskId: 'task-1' }), runningTask({ taskId: 'task-2' })], + }) + : Promise.resolve({}) + )); upsertThreadedReplyMock.mockRejectedValueOnce(new Error('surface hiccup')); await expect(handler()).resolves.toBeUndefined(); @@ -120,7 +183,11 @@ describe('iteration heartbeat sweep', () => { }); test('a query failure is swallowed β€” a cosmetic sweep never throws', async () => { - ddbSend.mockRejectedValue(new Error('throttled')); + ddbSend.mockImplementation((command: { _type?: string }) => ( + command._type === 'Query' + ? Promise.reject(new Error('throttled')) + : Promise.resolve({}) + )); await expect(handler()).resolves.toBeUndefined(); expect(upsertThreadedReplyMock).not.toHaveBeenCalled(); }); diff --git a/cdk/test/handlers/jira-webhook-processor.test.ts b/cdk/test/handlers/jira-webhook-processor.test.ts index b71a11570..c91e883cc 100644 --- a/cdk/test/handlers/jira-webhook-processor.test.ts +++ b/cdk/test/handlers/jira-webhook-processor.test.ts @@ -23,6 +23,7 @@ jest.mock('@aws-sdk/lib-dynamodb', () => ({ DynamoDBDocumentClient: { from: jest.fn(() => ({ send: ddbSend })) }, GetCommand: jest.fn((input: unknown) => ({ _type: 'Get', input })), ScanCommand: jest.fn((input: unknown) => ({ _type: 'Scan', input })), + UpdateCommand: jest.fn((input: unknown) => ({ _type: 'Update', input })), })); const createTaskCoreMock = jest.fn(); @@ -31,8 +32,10 @@ jest.mock('../../src/handlers/shared/create-task-core', () => ({ })); const reportIssueFailureMock = jest.fn(); +const postIssueCommentMock = jest.fn(); jest.mock('../../src/handlers/shared/jira-feedback', () => ({ reportIssueFailure: (...args: unknown[]) => reportIssueFailureMock(...args), + postIssueComment: (...args: unknown[]) => postIssueCommentMock(...args), })); const resolveJiraOauthTokenMock = jest.fn(); @@ -160,6 +163,8 @@ describe('jira-webhook-processor handler', () => { createTaskCoreMock.mockReset(); reportIssueFailureMock.mockReset(); reportIssueFailureMock.mockResolvedValue(undefined); + postIssueCommentMock.mockReset(); + postIssueCommentMock.mockResolvedValue('ack-comment-1'); resolveJiraOauthTokenMock.mockReset(); // Default: tenant IS resolvable. Drop-path tests override per-case // with `.mockResolvedValueOnce(null)`. @@ -248,18 +253,32 @@ describe('jira-webhook-processor handler', () => { jira_status_on_pr: 'Code Review', jira_trigger_comment_id: 'comment-1', jira_prior_task_id: 'prior-task', + trigger_comment_id: 'comment-1', + trigger_comment_issue_id: 'ENG-42', jira_oauth_secret_arn: 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent-jira-oauth-cloud-1', }); - expect(reportIssueFailureMock).toHaveBeenCalledWith( + expect(postIssueCommentMock).toHaveBeenCalledWith( expect.anything(), 'ENG-42', - 'πŸ‘€ ABCA accepted this follow-up and is updating PR #42.', + 'πŸ‘€ On it β€” reading the PR…', ); + const ackUpdate = ddbSend.mock.calls + .map(([command]) => command) + .find((command) => command?._type === 'Update'); + expect(ackUpdate.input).toMatchObject({ + TableName: 'Tasks', + UpdateExpression: 'SET channel_metadata.iteration_reply_comment_id = :comment_id', + ExpressionAttributeValues: { ':comment_id': 'ack-comment-1' }, + }); // Comment triggers route from the prior task, not the current project - // mapping or label state. The only DDB Get is author attribution. - expect(ddbSend.mock.calls).toHaveLength(1); - expect(ddbSend.mock.calls[0][0].input.Key) + // mapping or label state. The only DDB Get is author attribution; the + // additional write stores the maturing acknowledgement id. + const gets = ddbSend.mock.calls + .map(([command]) => command) + .filter((command) => command?._type === 'Get'); + expect(gets).toHaveLength(1); + expect(gets[0].input.Key) .toEqual({ jira_identity: 'cloud-1#reviewer-1' }); }); @@ -286,6 +305,7 @@ describe('jira-webhook-processor handler', () => { trigger_comment_id: 'comment-1', trigger_comment_issue_id: 'ENG-42', }); + expect(postIssueCommentMock).toHaveBeenCalledTimes(1); }); test('ADF mention node creates a PR iteration', async () => { @@ -446,6 +466,7 @@ describe('jira-webhook-processor handler', () => { expect(createTaskCoreMock).toHaveBeenCalledTimes(1); expect(reportIssueFailureMock).not.toHaveBeenCalled(); + expect(postIssueCommentMock).not.toHaveBeenCalled(); }); test('task admission failure is reported instead of acknowledged', async () => { diff --git a/cdk/test/handlers/orchestration-reconciler.test.ts b/cdk/test/handlers/orchestration-reconciler.test.ts index 63d5433cb..cdd577535 100644 --- a/cdk/test/handlers/orchestration-reconciler.test.ts +++ b/cdk/test/handlers/orchestration-reconciler.test.ts @@ -68,6 +68,13 @@ jest.mock('../../src/handlers/shared/linear-feedback', () => ({ EMOJI_NEEDS_INPUT: 'question', })); +const jiraPostIssueCommentMock = jest.fn(); +const jiraUpdateIssueCommentMock = jest.fn(); +jest.mock('../../src/handlers/shared/jira-feedback', () => ({ + postIssueComment: (...args: unknown[]) => jiraPostIssueCommentMock(...args), + updateIssueComment: (...args: unknown[]) => jiraUpdateIssueCommentMock(...args), +})); + jest.mock('../../src/handlers/shared/logger', () => ({ logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, })); @@ -77,6 +84,7 @@ process.env.TASK_TABLE_NAME = 'TaskTable'; // Cascade surfacing: the cascade posts Linear comments only when the // workspace registry is configured. Set it so the surfacing path is exercised. process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = 'WorkspaceRegistry'; +process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraWorkspaceRegistry'; process.env.ARTIFACTS_BUCKET_NAME = 'ArtifactsBucket'; import { TERMINAL_STATUSES } from '../../src/constructs/task-status'; @@ -101,6 +109,11 @@ function taskRecord(fields: { error_message?: string; // Whether the agent actually edited code; false marks a question/answer run. code_changed?: boolean; + iteration_reply_comment_id?: string; + cost_usd?: number; + duration_s?: number; + turns_attempted?: number; + max_turns?: number; // Stream sequence number (itemIdentifier for partial-batch reporting). sequenceNumber?: string; }): DynamoDBRecord { @@ -109,6 +122,10 @@ function taskRecord(fields: { if (fields.status) img.status = { S: fields.status }; if (fields.build_passed !== undefined) img.build_passed = { BOOL: fields.build_passed }; if (fields.code_changed !== undefined) img.code_changed = { BOOL: fields.code_changed }; + if (fields.cost_usd !== undefined) img.cost_usd = { N: String(fields.cost_usd) }; + if (fields.duration_s !== undefined) img.duration_s = { N: String(fields.duration_s) }; + if (fields.turns_attempted !== undefined) img.turns_attempted = { N: String(fields.turns_attempted) }; + if (fields.max_turns !== undefined) img.max_turns = { N: String(fields.max_turns) }; if (fields.error_message) img.error_message = { S: fields.error_message }; // PRODUCTION SHAPE: createTaskCore persists orchestration_id INSIDE the // nested channel_metadata MAP, not as a top-level attribute. The stream @@ -125,6 +142,9 @@ function taskRecord(fields: { if (fields.orchestration_iteration) cm.orchestration_iteration = { S: 'true' }; if (fields.trigger_comment_id) cm.trigger_comment_id = { S: fields.trigger_comment_id }; if (fields.trigger_comment_issue_id) cm.trigger_comment_issue_id = { S: fields.trigger_comment_issue_id }; + if (fields.iteration_reply_comment_id) { + cm.iteration_reply_comment_id = { S: fields.iteration_reply_comment_id }; + } if (Object.keys(cm).length > 0) img.channel_metadata = { M: cm }; return { eventName: fields.eventName ?? 'MODIFY', @@ -738,19 +758,22 @@ describe('feedback surface is chosen from the orchestration row, not assumed', ( expect(upsertStatusCommentMock).toHaveBeenCalled(); }); - test('a row whose surface has no configured registry skips feedback instead of posting to Linear', async () => { - // The Jira tenant registry is unset in this handler's env, so a Jira-sourced - // orchestration has no adapter. It must stay silent β€” NOT fall through and - // address the Linear workspace, which is a different tenant's data. + test('a Jira row drives the Jira adapter instead of posting to Linear', async () => { + jiraPostIssueCommentMock.mockResolvedValue('jira-panel-1'); mockOrchestration({ subIssueId: 'A', children: [{ sub_issue_id: 'A', child_status: 'released' }], - meta: { channel_source: 'jira' }, + meta: { + channel_source: 'jira', + parent_issue_ref: 'KAN-1', + credentials_ref: 'cloud-1', + }, }); await handler(completed()); expect(upsertStatusCommentMock).not.toHaveBeenCalled(); expect(swapIssueReactionMock).not.toHaveBeenCalled(); expect(transitionIssueStateMock).not.toHaveBeenCalled(); + expect(jiraPostIssueCommentMock).toHaveBeenCalled(); }); }); @@ -1121,6 +1144,8 @@ describe('orchestration-reconciler handler β€” the iteration ack reply', () => { transitionIssueStateMock.mockReset().mockResolvedValue(true); replyToCommentMock.mockReset().mockResolvedValue('reply-1'); upsertThreadedReplyMock.mockReset().mockResolvedValue('reply-1'); + jiraPostIssueCommentMock.mockReset().mockResolvedValue('jira-reply-1'); + jiraUpdateIssueCommentMock.mockReset().mockResolvedValue(true); }); /** An iteration event carrying the human comment id that triggered it. */ @@ -1158,6 +1183,159 @@ describe('orchestration-reconciler handler β€” the iteration ack reply', () => { expect(transitionIssueStateMock).toHaveBeenCalledWith(expect.anything(), 'A', 'started', ['In Review'], false); }); + test('a Jira orchestration iteration matures the stored top-level comment with metrics', async () => { + mockCascade( + [{ + sub_issue_id: 'KAN-2', + child_status: 'succeeded', + child_task_id: 'task-A', + child_branch_name: 'branch-A', + }], + { + channel_source: 'jira', + parent_issue_ref: 'KAN-1', + credentials_ref: 'cloud-1', + }, + ); + + await handler({ + Records: [taskRecord({ + task_id: 'iter-task-1', + status: 'COMPLETED', + orchestration_id: 'orch_1', + orchestration_sub_issue_id: 'KAN-2', + orchestration_iteration: true, + trigger_comment_id: 'human-cmt-1', + trigger_comment_issue_id: 'KAN-2', + iteration_reply_comment_id: 'jira-status-1', + cost_usd: 1.25, + duration_s: 125, + turns_attempted: 12, + max_turns: 100, + })], + } as never); + + const statusUpdate = jiraUpdateIssueCommentMock.mock.calls.find( + (([, , commentId]) => commentId === 'jira-status-1'), + ); + expect(statusUpdate).toBeDefined(); + const [ctx, issueKey, commentId, body] = statusUpdate!; + expect(ctx).toEqual({ + cloudId: 'cloud-1', + registryTableName: 'JiraWorkspaceRegistry', + }); + expect(issueKey).toBe('KAN-2'); + expect(commentId).toBe('jira-status-1'); + expect(body).toContain('cost: $1.25 β€’ turns: 12 / 100 β€’ duration: 2m 5s'); + expect(body).toContain('task iter-task-1'); + expect(upsertThreadedReplyMock).not.toHaveBeenCalled(); + }); + + test('a failed Jira terminal update releases the claim for redelivery', async () => { + mockCascade( + [{ + sub_issue_id: 'KAN-2', + child_status: 'succeeded', + child_task_id: 'task-A', + child_branch_name: 'branch-A', + }], + { + channel_source: 'jira', + parent_issue_ref: 'KAN-1', + credentials_ref: 'cloud-1', + }, + ); + jiraUpdateIssueCommentMock.mockImplementation( + (_ctx, _issueKey, commentId) => Promise.resolve(commentId !== 'jira-status-1'), + ); + + await handler({ + Records: [taskRecord({ + task_id: 'iter-task-1', + status: 'COMPLETED', + orchestration_id: 'orch_1', + orchestration_sub_issue_id: 'KAN-2', + orchestration_iteration: true, + trigger_comment_id: 'human-cmt-1', + trigger_comment_issue_id: 'KAN-2', + iteration_reply_comment_id: 'jira-status-1', + })], + } as never); + + const claims = ddbSend.mock.calls + .map(([command]) => command as { + _type?: string; + input?: { + UpdateExpression?: string; + ExpressionAttributeValues?: Record; + }; + }) + .filter(command => + command._type === 'Update' + && /ack_replied_at/.test(command.input?.UpdateExpression ?? ''), + ); + expect(claims[0].input?.UpdateExpression).toBe('SET ack_replied_at = :now'); + expect(claims[1].input?.UpdateExpression).toContain('REMOVE ack_replied_at'); + expect(claims[1].input?.ExpressionAttributeValues?.[':ours']) + .toBe(claims[0].input?.ExpressionAttributeValues?.[':now']); + }); + + test('redelivered Jira iteration matures its status comment once', async () => { + mockCascade( + [{ + sub_issue_id: 'KAN-2', + child_status: 'succeeded', + child_task_id: 'task-A', + child_branch_name: 'branch-A', + }], + { + channel_source: 'jira', + parent_issue_ref: 'KAN-1', + credentials_ref: 'cloud-1', + }, + ); + let ackClaims = 0; + const base = ddbSend.getMockImplementation()!; + ddbSend.mockImplementation(async (command: { + _type: string; + input: Record; + }) => { + if ( + command._type === 'Update' + && String(command.input.UpdateExpression).includes('ack_replied_at') + ) { + ackClaims += 1; + if (ackClaims > 1) { + throw Object.assign(new Error('claimed'), { + name: 'ConditionalCheckFailedException', + }); + } + return {}; + } + return base(command); + }); + const event = { + Records: [taskRecord({ + task_id: 'iter-task-1', + status: 'COMPLETED', + orchestration_id: 'orch_1', + orchestration_sub_issue_id: 'KAN-2', + orchestration_iteration: true, + trigger_comment_id: 'human-cmt-1', + trigger_comment_issue_id: 'KAN-2', + iteration_reply_comment_id: 'jira-status-1', + })], + } as never; + + await handler(event); + await handler(event); + + const statusUpdates = jiraUpdateIssueCommentMock.mock.calls.filter( + (([, , commentId]) => commentId === 'jira-status-1'), + ); + expect(statusUpdates).toHaveLength(1); + }); + test('a no-change iteration (a question) settles πŸ’¬ and leaves the sub-issue state alone', async () => { // An answer is neither a success-edit nor a failure. βœ… would imply "PR // updated, merge-worthy" and advancing the sub-issue would claim work landed diff --git a/cdk/test/handlers/shared/iteration-heartbeat.test.ts b/cdk/test/handlers/shared/iteration-heartbeat.test.ts index 45d22871a..dc613c638 100644 --- a/cdk/test/handlers/shared/iteration-heartbeat.test.ts +++ b/cdk/test/handlers/shared/iteration-heartbeat.test.ts @@ -42,6 +42,8 @@ describe('planHeartbeat β€” eligibility', () => { const plan = planHeartbeat(task(), NOW); expect(plan).not.toBeNull(); expect(plan!.taskId).toBe('t-1'); + expect(plan!.channelSource).toBe('linear'); + expect(plan!.credentialsRef).toBe('ws-1'); expect(plan!.replyId).toBe('reply-1'); expect(plan!.parentCommentId).toBe('cmt-1'); expect(plan!.issueId).toBe('issue-1'); @@ -74,7 +76,24 @@ describe('planHeartbeat β€” eligibility', () => { expect(planHeartbeat(task({ iterationReplyCommentId: undefined }), NOW)).toBeNull(); }); - test('non-linear channel β†’ no plan (reply edit only wired for linear)', () => { + test('a Jira iteration uses the tenant and top-level status comment', () => { + const plan = planHeartbeat(task({ + channelSource: 'jira', + linearWorkspaceId: undefined, + jiraCloudId: 'cloud-1', + triggerCommentId: undefined, + triggerCommentIssueId: 'ENG-42', + }), NOW); + expect(plan).toMatchObject({ + channelSource: 'jira', + credentialsRef: 'cloud-1', + issueId: 'ENG-42', + replyId: 'reply-1', + }); + expect(plan).not.toHaveProperty('parentCommentId'); + }); + + test('unsupported channel β†’ no plan', () => { expect(planHeartbeat(task({ channelSource: 'slack' }), NOW)).toBeNull(); }); @@ -88,6 +107,11 @@ describe('planHeartbeat β€” eligibility', () => { expect(planHeartbeat(task({ triggerCommentId: undefined }), NOW)).toBeNull(); expect(planHeartbeat(task({ triggerCommentIssueId: undefined }), NOW)).toBeNull(); expect(planHeartbeat(task({ linearWorkspaceId: undefined }), NOW)).toBeNull(); + expect(planHeartbeat(task({ + channelSource: 'jira', + linearWorkspaceId: undefined, + jiraCloudId: undefined, + }), NOW)).toBeNull(); }); test('unparseable / missing created_at β†’ no plan', () => { diff --git a/cdk/test/handlers/shared/jira-feedback.test.ts b/cdk/test/handlers/shared/jira-feedback.test.ts index b375a3fe4..81a82c5db 100644 --- a/cdk/test/handlers/shared/jira-feedback.test.ts +++ b/cdk/test/handlers/shared/jira-feedback.test.ts @@ -27,6 +27,7 @@ import { postIssueComment, postIssueCommentAdf, reportIssueFailure, + updateIssueComment, } from '../../../src/handlers/shared/jira-feedback'; const CTX = { cloudId: 'cloud-uuid-1', registryTableName: 'JiraWorkspaceRegistry' }; @@ -34,12 +35,12 @@ const CTX = { cloudId: 'cloud-uuid-1', registryTableName: 'JiraWorkspaceRegistry // ``fetch`` is the global transport; each test installs its own mock. const originalFetch = global.fetch; -function mockResponse(status: number): Response { +function mockResponse(status: number, body = '{"id":"10001"}'): Response { return { ok: status >= 200 && status < 300, status, json: async () => ({}), - text: async () => '', + text: async () => body, } as unknown as Response; } @@ -75,9 +76,9 @@ describe('jira-feedback: postIssueComment', () => { const fetchMock = jest.fn().mockResolvedValue(mockResponse(201)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(true); + expect(commentId).toBe('10001'); expect(fetchMock).toHaveBeenCalledTimes(1); const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; expect(url).toBe('https://install.webtrigger.atlassian.app/public/trigger-id'); @@ -105,7 +106,7 @@ describe('jira-feedback: postIssueComment', () => { }); global.fetch = jest.fn().mockResolvedValue(mockResponse(403)) as unknown as typeof fetch; - await expect(postIssueComment(CTX, 'ENG-42', 'hello')).resolves.toBe(false); + await expect(postIssueComment(CTX, 'ENG-42', 'hello')).resolves.toBeNull(); expect(resolveJiraOauthTokenMock).toHaveBeenCalledTimes(1); expect(global.fetch).toHaveBeenCalledTimes(1); }); @@ -114,9 +115,9 @@ describe('jira-feedback: postIssueComment', () => { const fetchMock = jest.fn().mockResolvedValue(mockResponse(201)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(true); + expect(commentId).toBe('10001'); expect(fetchMock).toHaveBeenCalledTimes(1); const [url, init] = fetchMock.mock.calls[0]; @@ -151,9 +152,9 @@ describe('jira-feedback: postIssueComment', () => { const fetchMock = jest.fn(); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); expect(fetchMock).not.toHaveBeenCalled(); }); @@ -164,9 +165,9 @@ describe('jira-feedback: postIssueComment', () => { const fetchMock = jest.fn(); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); expect(fetchMock).not.toHaveBeenCalled(); }); @@ -179,9 +180,9 @@ describe('jira-feedback: postIssueComment', () => { const fetchMock = jest.fn().mockResolvedValueOnce(mockResponse(401)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); // Only the first POST happened; the retry never got a token. expect(fetchMock).toHaveBeenCalledTimes(1); expect(resolveJiraOauthTokenMock).toHaveBeenCalledTimes(2); @@ -193,9 +194,9 @@ describe('jira-feedback: postIssueComment', () => { const fetchMock = jest.fn().mockResolvedValue(mockResponse(500)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); // 5xx is terminal β€” no forced-refresh retry, no second POST. expect(fetchMock).toHaveBeenCalledTimes(1); expect(resolveJiraOauthTokenMock).toHaveBeenCalledTimes(1); @@ -233,9 +234,9 @@ describe('jira-feedback: 401 β†’ forced refresh β†’ retry (issue #370)', () => { .mockResolvedValueOnce(mockResponse(201)); // retry: fresh token accepted global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(true); + expect(commentId).toBe('10001'); // Two POSTs, and the second carried the refreshed bearer token. expect(fetchMock).toHaveBeenCalledTimes(2); const firstHeaders = (fetchMock.mock.calls[0][1] as RequestInit).headers as Record; @@ -258,9 +259,9 @@ describe('jira-feedback: 401 β†’ forced refresh β†’ retry (issue #370)', () => { .mockResolvedValueOnce(mockResponse(201)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(true); + expect(commentId).toBe('10001'); expect(fetchMock).toHaveBeenCalledTimes(2); }); @@ -271,9 +272,9 @@ describe('jira-feedback: 401 β†’ forced refresh β†’ retry (issue #370)', () => { const fetchMock = jest.fn().mockResolvedValue(mockResponse(401)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); expect(fetchMock).toHaveBeenCalledTimes(1); // no retry with an unchanged token expect(resolveJiraOauthTokenMock).toHaveBeenCalledTimes(2); }); @@ -285,9 +286,9 @@ describe('jira-feedback: 401 β†’ forced refresh β†’ retry (issue #370)', () => { const fetchMock = jest.fn().mockResolvedValueOnce(mockResponse(401)); global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); expect(fetchMock).toHaveBeenCalledTimes(1); }); @@ -301,9 +302,9 @@ describe('jira-feedback: 401 β†’ forced refresh β†’ retry (issue #370)', () => { .mockResolvedValueOnce(mockResponse(401)); // fresh also rejected β†’ give up global.fetch = fetchMock as unknown as typeof fetch; - const ok = await postIssueComment(CTX, 'ENG-42', 'hello'); + const commentId = await postIssueComment(CTX, 'ENG-42', 'hello'); - expect(ok).toBe(false); + expect(commentId).toBeNull(); // Exactly two POSTs β€” the retry is bounded at one attempt. expect(fetchMock).toHaveBeenCalledTimes(2); expect(resolveJiraOauthTokenMock).toHaveBeenCalledTimes(2); @@ -382,7 +383,7 @@ describe('jira-feedback: postIssueCommentAdf (classified result, #573)', () => { const result = await postIssueCommentAdf(CTX, 'ENG-42', ADF); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, commentId: '10001' }); const init = fetchMock.mock.calls[0][1] as RequestInit; expect(JSON.parse(init.body as string)).toEqual({ body: ADF }); }); @@ -435,7 +436,7 @@ describe('jira-feedback: postIssueCommentAdf (classified result, #573)', () => { const result = await postIssueCommentAdf(CTX, 'ENG-42', ADF); - expect(result).toEqual({ ok: true }); + expect(result).toEqual({ ok: true, commentId: '10001' }); expect(fetchMock).toHaveBeenCalledTimes(2); }); @@ -472,3 +473,83 @@ describe('jira-feedback: postIssueCommentAdf (classified result, #573)', () => { expect(result).toEqual({ ok: false, retryable: true }); }); }); + +describe('jira-feedback: updateIssueComment', () => { + test('updates the requested comment with ADF via OAuth', async () => { + const fetchMock = jest.fn().mockResolvedValue(mockResponse(200)); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(updateIssueComment(CTX, 'ENG-42', '10001', 'working')).resolves.toBe(true); + + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe( + 'https://api.atlassian.com/ex/jira/cloud-uuid-1/rest/api/3/issue/ENG-42/comment/10001', + ); + expect(init.method).toBe('PUT'); + expect(JSON.parse(init.body as string)).toEqual({ + body: { + type: 'doc', + version: 1, + content: [{ + type: 'paragraph', + content: [{ type: 'text', text: 'working' }], + }], + }, + }); + }); + + test('uses the Forge update_comment operation when app auth is configured', async () => { + resolveJiraOauthTokenMock.mockResolvedValueOnce({ + kind: 'app', + appActor: { + proxyUrl: 'https://install.webtrigger.atlassian.app/public/trigger-id', + sharedSecret: 's'.repeat(64), + }, + siteUrl: 'https://acme.atlassian.net', + oauthSecretArn: 'arn:secret:acme', + }); + const fetchMock = jest.fn().mockResolvedValue(mockResponse(200)); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(updateIssueComment(CTX, 'ENG-42', '10001', 'done')).resolves.toBe(true); + + expect(JSON.parse(fetchMock.mock.calls[0][1].body as string)).toMatchObject({ + operation: 'update_comment', + issue_key: 'ENG-42', + comment_id: '10001', + }); + }); + + test('forces one OAuth refresh on 401 and retries the PUT', async () => { + resolveJiraOauthTokenMock + .mockReset() + .mockResolvedValueOnce({ accessToken: 'stale', scope: '', siteUrl: '', oauthSecretArn: 'x' }) + .mockResolvedValueOnce({ accessToken: 'fresh', scope: '', siteUrl: '', oauthSecretArn: 'x' }); + const fetchMock = jest.fn() + .mockResolvedValueOnce(mockResponse(401)) + .mockResolvedValueOnce(mockResponse(200)); + global.fetch = fetchMock as unknown as typeof fetch; + + await expect(updateIssueComment(CTX, 'ENG-42', '10001', 'done')).resolves.toBe(true); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect((fetchMock.mock.calls[0][1].headers as Record).Authorization) + .toBe('Bearer stale'); + expect((fetchMock.mock.calls[1][1].headers as Record).Authorization) + .toBe('Bearer fresh'); + expect(resolveJiraOauthTokenMock.mock.calls[1][2]).toEqual({ forceRefresh: true }); + }); + + test('returns false for a missing comment and never throws on a resolver failure', async () => { + global.fetch = jest.fn().mockResolvedValue(mockResponse(404)) as unknown as typeof fetch; + await expect(updateIssueComment(CTX, 'ENG-42', '99999', 'done')).resolves.toBe(false); + + resolveJiraOauthTokenMock.mockReset().mockRejectedValueOnce(new Error('registry down')); + await expect(updateIssueComment(CTX, 'ENG-42', '10001', 'done')).resolves.toBe(false); + }); + + test('rejects a create response that omitted the Jira comment id', async () => { + global.fetch = jest.fn().mockResolvedValue(mockResponse(201, '{}')) as unknown as typeof fetch; + await expect(postIssueComment(CTX, 'ENG-42', 'hello')).resolves.toBeNull(); + }); +}); diff --git a/cdk/test/handlers/shared/orchestration-channel.test.ts b/cdk/test/handlers/shared/orchestration-channel.test.ts index 36942f94a..d7e8e13a2 100644 --- a/cdk/test/handlers/shared/orchestration-channel.test.ts +++ b/cdk/test/handlers/shared/orchestration-channel.test.ts @@ -60,9 +60,11 @@ jest.mock('../../../src/handlers/shared/linear-subissue-fetch', () => ({ const jiraPostIssueComment = jest.fn(); const jiraReportIssueFailure = jest.fn(); +const jiraUpdateIssueComment = jest.fn(); jest.mock('../../../src/handlers/shared/jira-feedback', () => ({ postIssueComment: (...a: unknown[]) => jiraPostIssueComment(...a), reportIssueFailure: (...a: unknown[]) => jiraReportIssueFailure(...a), + updateIssueComment: (...a: unknown[]) => jiraUpdateIssueComment(...a), })); import { type IssueRef } from '../../../src/handlers/shared/orchestration-channel'; @@ -231,15 +233,38 @@ describe('Jira channel adapter (capability-limited surface)', () => { test('kind is jira', () => expect(ch.kind).toBe('jira')); test('postComment builds the JiraFeedbackContext (cloudId) from the issue', async () => { - jiraPostIssueComment.mockResolvedValue(true); + jiraPostIssueComment.mockResolvedValue('cmt-42'); const res = await ch.postComment(jiraIssue, 'hello'); - expect(res).not.toBeNull(); + expect(res).toEqual({ commentId: 'cmt-42' }); const [ctx, id, body] = jiraPostIssueComment.mock.calls[0]; expect(ctx).toEqual({ cloudId: 'cloud-1', registryTableName: 'JiraRegistry' }); expect(id).toBe('ABC-1'); expect(body).toBe('hello'); }); + test('upsertComment edits an existing Jira comment instead of posting another', async () => { + jiraUpdateIssueComment.mockResolvedValue(true); + + const res = await ch.upsertComment(jiraIssue, 'working', { commentId: 'cmt-42' }); + + expect(res).toEqual({ commentId: 'cmt-42' }); + expect(jiraUpdateIssueComment).toHaveBeenCalledWith( + { cloudId: 'cloud-1', registryTableName: 'JiraRegistry' }, + 'ABC-1', + 'cmt-42', + 'working', + ); + expect(jiraPostIssueComment).not.toHaveBeenCalled(); + }); + + test('upsertComment creates when there is no existing Jira comment', async () => { + jiraPostIssueComment.mockResolvedValue('cmt-new'); + await expect(ch.upsertComment(jiraIssue, 'on it')).resolves.toEqual({ + commentId: 'cmt-new', + }); + expect(jiraUpdateIssueComment).not.toHaveBeenCalled(); + }); + test('reportFailure routes to the Jira failure helper', async () => { await ch.reportFailure(jiraIssue, '❌ nope'); expect(jiraReportIssueFailure).toHaveBeenCalledWith( diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 82cb22dfc..7d848a712 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -500,6 +500,24 @@ describe('AgentStack', () => { expect(vars.JIRA_WORKSPACE_REGISTRY_TABLE_NAME).toBeDefined(); }); + test('the iteration heartbeat can reach BOTH surfaces and refresh Jira OAuth', () => { + const fns = template.findResources('AWS::Lambda::Function'); + const heartbeat = Object.entries(fns).find(([id]) => id.startsWith('IterationHeartbeat')); + expect(heartbeat).toBeDefined(); + const vars = (heartbeat![1] as { Properties?: { Environment?: { Variables?: Record } } }) + .Properties?.Environment?.Variables ?? {}; + expect(vars.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME).toBeDefined(); + expect(vars.JIRA_WORKSPACE_REGISTRY_TABLE_NAME).toBeDefined(); + + const policies = template.findResources('AWS::IAM::Policy'); + const heartbeatPolicies = Object.entries(policies) + .filter(([logicalId]) => logicalId.startsWith('IterationHeartbeat')); + const asJson = JSON.stringify(heartbeatPolicies.map(([, policy]) => policy)); + expect(asJson).toContain('bgagent-jira-oauth-*'); + expect(asJson).toContain('secretsmanager:GetSecretValue'); + expect(asJson).toContain('secretsmanager:PutSecretValue'); + }); + test('the orchestration reconciler cannot read S3 objects at all', () => { // The trace/artifacts bucket holds full agent trajectories under // traces// β€” tool input and output, authorized per-user by the presign diff --git a/docs/guides/JIRA_SETUP_GUIDE.md b/docs/guides/JIRA_SETUP_GUIDE.md index e01a7128e..0c4996ccc 100644 --- a/docs/guides/JIRA_SETUP_GUIDE.md +++ b/docs/guides/JIRA_SETUP_GUIDE.md @@ -56,11 +56,17 @@ runner picks task with channel_source="jira" Outbound terminal status (Platform β†’ Jira) β€” Forge app actor, deterministic: ``` -task reaches a terminal event (completed / failed / cancelled / +ordinary task reaches a terminal event (completed / failed / cancelled / stranded / timed out) β†’ TaskEventsTable DynamoDB Stream β†’ fan-out Lambda's dispatchToJira resolves the same Forge proxy and posts ONE app-authored final-status comment with cost, turns, duration, task id, and the PR link + +@bgagent iteration is admitted β†’ JiraWebhookProcessor posts ONE + app-authored status comment and stores its comment id + β†’ heartbeat edits that comment with elapsed time while the task runs + β†’ fan-out (standalone) or reconciler (orchestrated child) edits that + same comment with the terminal outcome and metrics ``` Outbound board transitions (Agent β†’ Jira) β€” Forge app actor: @@ -75,17 +81,19 @@ the originating issue as it works β€” the same signal Linear-origin tasks alread give. See [Board transitions](#board-transitions) below for the resolution order and the permission it requires. -The **start** comment is posted by the agent. The **terminal** comment is -posted by the platform's fan-out plane, not the agent β€” so it always includes -cost / turns / duration and fires even when the agent crashes before -completing (max-turns, OOM). The final comment frames three outcomes: +For an ordinary task, the **start** comment is posted by the agent and the +**terminal** comment is posted by the platform's fan-out plane. For an +`@bgagent` iteration, the processor immediately posts one status comment; the +heartbeat and terminal owner edit that same comment in place. Terminal feedback +therefore includes cost / turns / duration even when the agent crashes before +completing (max-turns, OOM). The final state frames three outcomes: - βœ… **Task completed** β€” with the PR link when one was opened. - ⚠️ **Shipped a PR but stopped early** β€” the PR link plus the reason it stopped (e.g. "Hit max-turns cap"), so you can review and decide. - ❌ **Task failed / cancelled / timed out** β€” with a short classifier reason. -Comments are advisory and best-effort: network/auth failures are logged and swallowed (the agent path has an auth circuit-breaker; the platform path classifies transient failures as retryable and retries the record), never gating the task itself. Jira has no comment-edit API, so the terminal comment is posted exactly once (a per-task marker guards against duplicate posts on stream retries). +Comments are advisory and best-effort: network/auth failures are logged and swallowed (the agent path has an auth circuit-breaker; the platform path classifies transient failures as retryable and retries the record), never gating the task itself. Ordinary terminal comments use a per-task post-once marker. Iteration terminal writers use a per-task claim before updating the stored comment ID, and the heartbeat checks that claim before writing, so retries do not duplicate the comment or regress a terminal outcome back to running. **Identity selection rule.** A complete Forge app configuration always wins for every outbound path. If that configured proxy, signature, permission, or Jira API call fails, ABCA logs the failure and skips the advisory write; it does **not** retry as the 3LO user. Tenants with no Forge configuration retain the old 3LO writer as an explicit migration fallback, with a warning. @@ -97,7 +105,7 @@ Comments are advisory and best-effort: network/auth failures are logged and swal > actor through `api.asApp().requestJira(...)`. See > [ADR-015](../decisions/ADR-015-jira-integration.md). -Inbound admission (webhook β†’ task) is Jira-specific and has no DynamoDB Streams consumer of its own. The **terminal** status comment, however, is delivered by the shared fan-out plane's DynamoDB Streams consumer (`dispatchToJira`) β€” the same platform-side surface that posts Linear final-status comments β€” so it behaves identically to Linear for terminal outcomes. +Inbound admission (webhook β†’ task) is Jira-specific and has no DynamoDB Streams consumer of its own. Ordinary **terminal** status comments are delivered by the shared fan-out plane's DynamoDB Streams consumer (`dispatchToJira`). For comment-triggered iterations, fan-out matures standalone status comments while the orchestration reconciler matures child-iteration comments before restacking dependents. ## Setup walkthrough @@ -149,7 +157,7 @@ Paste that same secret value back at the `Webhook signing secret:` prompt. ABCA ### 4. Install the dedicated outbound app -The repository includes a narrow Forge app under `integrations/jira-forge-app`. Its web trigger accepts only four signed operations: identity probe, comment, read transitions, and perform transition. It does not expose a general Jira REST proxy. +The repository includes a narrow Forge app under `integrations/jira-forge-app`. Its web trigger accepts only five signed operations: identity probe, create comment, update comment, read transitions, and perform transition. It does not expose a general Jira REST proxy. Run the login in an interactive terminal. Forge asks for your Atlassian email and the Forge CLI scoped token from the prerequisites; the Jira 3LO access token is not a Forge CLI credential. On the first registration, Forge also asks you to create or select a **Developer Space**. @@ -276,7 +284,7 @@ The teammate needs their own ABCA account first (Cognito user + configured CLI). Add the trigger label (`bgagent` by default) to a Jira issue in a mapped project. The agent should start within ~30 seconds, comment on the issue as it works, and post a PR link when ready. The issue **summary** plus the **description** (converted from Atlassian Document Format to markdown), the issue's **recent comments**, and any supported **file attachments** become the task context β€” see [Issue context: attachments and comments](#issue-context-attachments-and-comments). -After the PR exists, add a Jira comment such as `@bgagent update the README too`. ABCA should acknowledge the request on the issue and update the existing PR. +After the PR exists, add a Jira comment such as `@bgagent update the README too`. ABCA should create one acknowledgement status comment, update it with elapsed time during a long run, update the existing PR, and finally replace the same comment with the terminal outcome and metrics. The progress comment author and transition actor should be the `bgagent` app. The task owner shown by `bgagent list`, audit records, concurrency accounting, and cost attribution should remain the linked human who triggered the Jira event. @@ -313,7 +321,9 @@ ABCA resolves the Jira tenant and issue key to the newest prior task that actual When the comment author has linked their Jira and ABCA accounts, the iteration is attributed to that user. Otherwise, ABCA falls back to the original task owner so a useful reviewer request is not dropped. Comments without the mention, app-authored comments, and ABCA's own generated status comments are no-ops. -The acknowledgement is immediate after task admission. The existing platform fan-out path posts the terminal outcome and cost comment when the iteration finishes. Comment redelivery is idempotent: the webhook receiver deduplicates by Jira comment ID, and task creation uses a deterministic idempotency key as a second guard. +The acknowledgement is immediate after task admission and its Jira comment ID is stored on the iteration task. Eligible long-running iterations edit that comment with elapsed time; they do not add heartbeat comments. When the iteration finishes, fan-out owns the terminal edit for a standalone iteration and the orchestration reconciler owns it for a child iteration so dependent restacking remains ordered. Both replace the same comment with the outcome, cost, turns, duration, task ID, and PR link when available. + +Comment redelivery is idempotent: the webhook receiver deduplicates by Jira comment ID, task creation uses a deterministic idempotency key as a second guard, and terminal writers claim the stored status comment before editing it. A heartbeat checks the terminal claim immediately before its cosmetic edit, preventing an overlapping sweep from replacing a completed outcome with a running message. ## Authored subtask orchestration diff --git a/docs/src/content/docs/using/Jira-setup-guide.md b/docs/src/content/docs/using/Jira-setup-guide.md index bf322c826..8ef1d5d03 100644 --- a/docs/src/content/docs/using/Jira-setup-guide.md +++ b/docs/src/content/docs/using/Jira-setup-guide.md @@ -60,11 +60,17 @@ runner picks task with channel_source="jira" Outbound terminal status (Platform β†’ Jira) β€” Forge app actor, deterministic: ``` -task reaches a terminal event (completed / failed / cancelled / +ordinary task reaches a terminal event (completed / failed / cancelled / stranded / timed out) β†’ TaskEventsTable DynamoDB Stream β†’ fan-out Lambda's dispatchToJira resolves the same Forge proxy and posts ONE app-authored final-status comment with cost, turns, duration, task id, and the PR link + +@bgagent iteration is admitted β†’ JiraWebhookProcessor posts ONE + app-authored status comment and stores its comment id + β†’ heartbeat edits that comment with elapsed time while the task runs + β†’ fan-out (standalone) or reconciler (orchestrated child) edits that + same comment with the terminal outcome and metrics ``` Outbound board transitions (Agent β†’ Jira) β€” Forge app actor: @@ -79,17 +85,19 @@ the originating issue as it works β€” the same signal Linear-origin tasks alread give. See [Board transitions](#board-transitions) below for the resolution order and the permission it requires. -The **start** comment is posted by the agent. The **terminal** comment is -posted by the platform's fan-out plane, not the agent β€” so it always includes -cost / turns / duration and fires even when the agent crashes before -completing (max-turns, OOM). The final comment frames three outcomes: +For an ordinary task, the **start** comment is posted by the agent and the +**terminal** comment is posted by the platform's fan-out plane. For an +`@bgagent` iteration, the processor immediately posts one status comment; the +heartbeat and terminal owner edit that same comment in place. Terminal feedback +therefore includes cost / turns / duration even when the agent crashes before +completing (max-turns, OOM). The final state frames three outcomes: - βœ… **Task completed** β€” with the PR link when one was opened. - ⚠️ **Shipped a PR but stopped early** β€” the PR link plus the reason it stopped (e.g. "Hit max-turns cap"), so you can review and decide. - ❌ **Task failed / cancelled / timed out** β€” with a short classifier reason. -Comments are advisory and best-effort: network/auth failures are logged and swallowed (the agent path has an auth circuit-breaker; the platform path classifies transient failures as retryable and retries the record), never gating the task itself. Jira has no comment-edit API, so the terminal comment is posted exactly once (a per-task marker guards against duplicate posts on stream retries). +Comments are advisory and best-effort: network/auth failures are logged and swallowed (the agent path has an auth circuit-breaker; the platform path classifies transient failures as retryable and retries the record), never gating the task itself. Ordinary terminal comments use a per-task post-once marker. Iteration terminal writers use a per-task claim before updating the stored comment ID, and the heartbeat checks that claim before writing, so retries do not duplicate the comment or regress a terminal outcome back to running. **Identity selection rule.** A complete Forge app configuration always wins for every outbound path. If that configured proxy, signature, permission, or Jira API call fails, ABCA logs the failure and skips the advisory write; it does **not** retry as the 3LO user. Tenants with no Forge configuration retain the old 3LO writer as an explicit migration fallback, with a warning. @@ -101,7 +109,7 @@ Comments are advisory and best-effort: network/auth failures are logged and swal > actor through `api.asApp().requestJira(...)`. See > [ADR-015](/sample-autonomous-cloud-coding-agents/architecture/adr-015-jira-integration). -Inbound admission (webhook β†’ task) is Jira-specific and has no DynamoDB Streams consumer of its own. The **terminal** status comment, however, is delivered by the shared fan-out plane's DynamoDB Streams consumer (`dispatchToJira`) β€” the same platform-side surface that posts Linear final-status comments β€” so it behaves identically to Linear for terminal outcomes. +Inbound admission (webhook β†’ task) is Jira-specific and has no DynamoDB Streams consumer of its own. Ordinary **terminal** status comments are delivered by the shared fan-out plane's DynamoDB Streams consumer (`dispatchToJira`). For comment-triggered iterations, fan-out matures standalone status comments while the orchestration reconciler matures child-iteration comments before restacking dependents. ## Setup walkthrough @@ -153,7 +161,7 @@ Paste that same secret value back at the `Webhook signing secret:` prompt. ABCA ### 4. Install the dedicated outbound app -The repository includes a narrow Forge app under `integrations/jira-forge-app`. Its web trigger accepts only four signed operations: identity probe, comment, read transitions, and perform transition. It does not expose a general Jira REST proxy. +The repository includes a narrow Forge app under `integrations/jira-forge-app`. Its web trigger accepts only five signed operations: identity probe, create comment, update comment, read transitions, and perform transition. It does not expose a general Jira REST proxy. Run the login in an interactive terminal. Forge asks for your Atlassian email and the Forge CLI scoped token from the prerequisites; the Jira 3LO access token is not a Forge CLI credential. On the first registration, Forge also asks you to create or select a **Developer Space**. @@ -280,7 +288,7 @@ The teammate needs their own ABCA account first (Cognito user + configured CLI). Add the trigger label (`bgagent` by default) to a Jira issue in a mapped project. The agent should start within ~30 seconds, comment on the issue as it works, and post a PR link when ready. The issue **summary** plus the **description** (converted from Atlassian Document Format to markdown), the issue's **recent comments**, and any supported **file attachments** become the task context β€” see [Issue context: attachments and comments](#issue-context-attachments-and-comments). -After the PR exists, add a Jira comment such as `@bgagent update the README too`. ABCA should acknowledge the request on the issue and update the existing PR. +After the PR exists, add a Jira comment such as `@bgagent update the README too`. ABCA should create one acknowledgement status comment, update it with elapsed time during a long run, update the existing PR, and finally replace the same comment with the terminal outcome and metrics. The progress comment author and transition actor should be the `bgagent` app. The task owner shown by `bgagent list`, audit records, concurrency accounting, and cost attribution should remain the linked human who triggered the Jira event. @@ -317,7 +325,9 @@ ABCA resolves the Jira tenant and issue key to the newest prior task that actual When the comment author has linked their Jira and ABCA accounts, the iteration is attributed to that user. Otherwise, ABCA falls back to the original task owner so a useful reviewer request is not dropped. Comments without the mention, app-authored comments, and ABCA's own generated status comments are no-ops. -The acknowledgement is immediate after task admission. The existing platform fan-out path posts the terminal outcome and cost comment when the iteration finishes. Comment redelivery is idempotent: the webhook receiver deduplicates by Jira comment ID, and task creation uses a deterministic idempotency key as a second guard. +The acknowledgement is immediate after task admission and its Jira comment ID is stored on the iteration task. Eligible long-running iterations edit that comment with elapsed time; they do not add heartbeat comments. When the iteration finishes, fan-out owns the terminal edit for a standalone iteration and the orchestration reconciler owns it for a child iteration so dependent restacking remains ordered. Both replace the same comment with the outcome, cost, turns, duration, task ID, and PR link when available. + +Comment redelivery is idempotent: the webhook receiver deduplicates by Jira comment ID, task creation uses a deterministic idempotency key as a second guard, and terminal writers claim the stored status comment before editing it. A heartbeat checks the terminal claim immediately before its cosmetic edit, preventing an overlapping sweep from replacing a completed outcome with a running message. ## Authored subtask orchestration diff --git a/integrations/jira-forge-app/src/proxy.js b/integrations/jira-forge-app/src/proxy.js index cdb11c247..aee7df997 100644 --- a/integrations/jira-forge-app/src/proxy.js +++ b/integrations/jira-forge-app/src/proxy.js @@ -24,6 +24,7 @@ const MAX_BODY_BYTES = 256 * 1024; const MAX_CLOCK_SKEW_SECONDS = 5 * 60; const APP_ACTOR_MIN_SECRET_LENGTH = 32; const ISSUE_KEY_RE = /^[A-Za-z][A-Za-z0-9_]*-\d+$/; +const COMMENT_ID_RE = /^\d+$/; const TRANSITION_ID_RE = /^[A-Za-z0-9_-]{1,128}$/; function response(statusCode, body = '') { @@ -162,6 +163,27 @@ export function createProxyHandler({ body: JSON.stringify({ body: payload.body }), }, )); + case 'update_comment': + if ( + !ISSUE_KEY_RE.test(payload.issue_key ?? '') + || !COMMENT_ID_RE.test(payload.comment_id ?? '') + || !payload.body + || typeof payload.body !== 'object' + || Array.isArray(payload.body) + ) { + return response(400, { error: 'invalid_update_comment_request' }); + } + return jiraResponse(await requestJira( + route`/rest/api/3/issue/${payload.issue_key}/comment/${payload.comment_id}`, + { + method: 'PUT', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ body: payload.body }), + }, + )); case 'get_transitions': if (!ISSUE_KEY_RE.test(payload.issue_key ?? '')) { return response(400, { error: 'invalid_issue_key' }); diff --git a/integrations/jira-forge-app/test/proxy.test.js b/integrations/jira-forge-app/test/proxy.test.js index 928ff36fe..be0f096db 100644 --- a/integrations/jira-forge-app/test/proxy.test.js +++ b/integrations/jira-forge-app/test/proxy.test.js @@ -89,6 +89,28 @@ test('posts comments through Jira as the app actor', async () => { }); }); +test('updates an allowlisted Jira comment as the app actor', async () => { + const calls = []; + const invoke = handler(async (...args) => { + calls.push(args); + return jiraResponse(200, '{"id":"10001"}'); + }); + const body = { type: 'doc', version: 1, content: [] }; + const result = await invoke(event({ + version: 1, + operation: 'update_comment', + cloud_id: 'cloud-1', + issue_key: 'ENG-42', + comment_id: '10001', + body, + })); + + assert.equal(result.statusCode, 200); + assert.equal(calls[0][0], '/rest/api/3/issue/ENG-42/comment/10001'); + assert.equal(calls[0][1].method, 'PUT'); + assert.deepEqual(JSON.parse(calls[0][1].body), { body }); +}); + test('rejects invalid and stale signatures before Jira is called', async () => { let calls = 0; const invoke = handler(async () => { @@ -181,6 +203,46 @@ test('rejects traversal issue keys and missing comment bodies', async () => { assert.equal(calls, 0); }); +test('rejects unsafe or missing update-comment fields', async () => { + let calls = 0; + const invoke = handler(async () => { + calls += 1; + return jiraResponse(200, '{}'); + }); + const cases = [ + { + version: 1, + operation: 'update_comment', + cloud_id: 'cloud-1', + issue_key: '../../evil-1', + comment_id: '10001', + body: { type: 'doc' }, + }, + { + version: 1, + operation: 'update_comment', + cloud_id: 'cloud-1', + issue_key: 'ENG-42', + comment_id: '../10001', + body: { type: 'doc' }, + }, + { + version: 1, + operation: 'update_comment', + cloud_id: 'cloud-1', + issue_key: 'ENG-42', + comment_id: '10001', + }, + ]; + + for (const payload of cases) { + const result = await invoke(event(payload)); + assert.equal(result.statusCode, 400); + assert.match(result.body, /invalid_update_comment_request/); + } + assert.equal(calls, 0); +}); + test('rejects oversized bodies before signature verification', async () => { let calls = 0; const invoke = handler(async () => {