diff --git a/cdk/src/constructs/iteration-heartbeat.ts b/cdk/src/constructs/iteration-heartbeat.ts new file mode 100644 index 000000000..4c50a6cb6 --- /dev/null +++ b/cdk/src/constructs/iteration-heartbeat.ts @@ -0,0 +1,110 @@ +/** + * 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 * as path from 'path'; +import { Duration } from 'aws-cdk-lib'; +import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; +import * as events from 'aws-cdk-lib/aws-events'; +import * as targets from 'aws-cdk-lib/aws-events-targets'; +import { Runtime, Architecture } from 'aws-cdk-lib/aws-lambda'; +import * as lambda from 'aws-cdk-lib/aws-lambda-nodejs'; +import { NagSuppressions } from 'cdk-nag'; +import { Construct } from 'constructs'; +import { TaskTable } from './task-table'; + +/** Sweep Lambda timeout (minutes) β€” bounded reply edits, well under a sweep interval. */ +const SWEEP_TIMEOUT_MINUTES = 2; + +/** Default heartbeat sweep interval (minutes). Short enough that a long run shows liveness quickly. */ +const DEFAULT_SCHEDULE_MINUTES = 2; + +/** Sweep Lambda memory (MB). */ +const SWEEP_MEMORY_MB = 256; + +/** Properties for the IterationHeartbeat construct. */ +export interface IterationHeartbeatProps { + /** TaskTable (has the StatusIndex GSI the sweep queries for RUNNING tasks). */ + readonly taskTable: dynamodb.ITable; + /** + * How often to sweep RUNNING iterations and refresh their maturing reply. + * @default Duration.minutes(2) + */ + readonly schedule?: Duration; +} + +/** + * 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 ("πŸ”„ + * 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. + */ +export class IterationHeartbeat extends Construct { + public readonly fn: lambda.NodejsFunction; + + constructor(scope: Construct, id: string, props: IterationHeartbeatProps) { + super(scope, id); + + const handlersDir = path.join(__dirname, '..', 'handlers'); + + this.fn = new lambda.NodejsFunction(this, 'SweepFn', { + entry: path.join(handlersDir, 'iteration-heartbeat-sweep.ts'), + handler: 'handler', + runtime: Runtime.NODEJS_24_X, + architecture: Architecture.ARM_64, + timeout: Duration.minutes(SWEEP_TIMEOUT_MINUTES), + memorySize: SWEEP_MEMORY_MB, + environment: { + TASK_TABLE_NAME: props.taskTable.tableName, + TASK_STATUS_INDEX_NAME: TaskTable.STATUS_INDEX, + }, + bundling: { + externalModules: ['@aws-sdk/*'], + }, + }); + + // Read-only on the TaskTable (StatusIndex query). No write β€” a heartbeat + // never mutates task state; it only edits a Linear comment. + props.taskTable.grantReadData(this.fn); + + const schedule = props.schedule ?? Duration.minutes(DEFAULT_SCHEDULE_MINUTES); + const rule = new events.Rule(this, 'HeartbeatSchedule', { + schedule: events.Schedule.rate(schedule), + }); + rule.addTarget(new targets.LambdaFunction(this.fn)); + + NagSuppressions.addResourceSuppressions(this.fn, [ + { + id: 'AwsSolutions-IAM4', + reason: 'AWSLambdaBasicExecutionRole is required for CloudWatch Logs access', + }, + { + id: 'AwsSolutions-IAM5', + reason: 'DynamoDB index/* wildcard generated by CDK grantReadData; ' + + 'per-workspace linear-oauth secret prefix grant added by the stack', + }, + ], true); + } +} diff --git a/cdk/src/handlers/iteration-heartbeat-sweep.ts b/cdk/src/handlers/iteration-heartbeat-sweep.ts new file mode 100644 index 000000000..9d7e1de55 --- /dev/null +++ b/cdk/src/handlers/iteration-heartbeat-sweep.ts @@ -0,0 +1,168 @@ +/** + * 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. + */ + +/** + * Mid-run liveness heartbeat sweep (scheduled). + * + * Observed in practice: a comment-triggered iteration ran 22 min showing only + * "πŸ€– Starting on this issue" then a terminal ❌ β€” a silent black box. This + * scheduled Lambda runs every couple of minutes, finds RUNNING comment-triggered + * iteration tasks, and EDITS THE EXISTING maturing reply in place to show + * liveness ("πŸ”„ Working β€” updating PR #N… _8m elapsed_"). It never posts a new + * comment (the user's "don't clutter the Linear UI" constraint) β€” it reuses the + * one reply comment id the trigger-time ack stamped on the task. + * + * Eligibility + body are decided by the pure {@link planHeartbeat}; this handler + * owns only the I/O: a ``StatusIndex`` query for RUNNING tasks, the field + * extraction off each record's ``channel_metadata``, and the best-effort reply + * edit. Idempotent: editing to the same body is a no-op; the terminal settle + * (reconciler) later overwrites the working line with βœ…/❌ as today. + */ + +import { DynamoDBClient, 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'; + +const ddb = new 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; + +/** Hard cap on tasks edited per sweep β€” a backstop against an unexpected flood. */ +const MAX_EDITS_PER_SWEEP = 50; + +interface DdbMap { [k: string]: { S?: string; N?: string; BOOL?: boolean; M?: DdbMap } } + +/** Map a RUNNING task's DDB image β†’ the heartbeat view (channel_metadata is nested). */ +function toView(img: DdbMap): HeartbeatTaskView { + const cm = img.channel_metadata?.M ?? {}; + const prNumberRaw = img.pr_number?.N; + return { + taskId: img.task_id?.S ?? '', + status: img.status?.S ?? '', + ...(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.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 + // ``trigger_comment_issue_id`` (the parent epic, for a routed comment); + // 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, + }), + isIteration: cm.orchestration_iteration?.S === 'true', + ...(prNumberRaw !== undefined && { prNumber: Number(prNumberRaw) }), + ...(img.pr_url?.S !== undefined && { prUrl: img.pr_url.S }), + }; +} + +/** Query every RUNNING task via the StatusIndex GSI (paginated). */ +async function loadRunningTasks(): Promise { + const items: DdbMap[] = []; + let lastKey: Record | undefined; + do { + const resp = await ddb.send(new QueryCommand({ + TableName: TASK_TABLE, + IndexName: STATUS_INDEX, + KeyConditionExpression: '#s = :running', + ExpressionAttributeNames: { '#s': 'status' }, + ExpressionAttributeValues: { ':running': { S: 'RUNNING' } }, + ExclusiveStartKey: lastKey as Record | undefined, + })); + items.push(...((resp.Items ?? []) as unknown as DdbMap[])); + lastKey = resp.LastEvaluatedKey; + } while (lastKey); + return items; +} + +/** + * Scheduled entrypoint. Best-effort throughout: a single task's edit failure is + * logged and skipped; the sweep never throws (a heartbeat is cosmetic β€” it must + * never wedge or alarm). + */ +export async function handler(): Promise { + if (!WORKSPACE_REGISTRY_TABLE) { + logger.info('Heartbeat sweep skipped β€” no Linear workspace registry configured'); + return; + } + + const nowMs = Date.now(); + let running: DdbMap[]; + try { + running = await loadRunningTasks(); + } catch (err) { + logger.warn('Heartbeat sweep: StatusIndex query failed (non-fatal)', { + error: err instanceof Error ? err.message : String(err), + }); + return; + } + + const plans = running + .map(toView) + .map((v) => planHeartbeat(v, nowMs)) + .filter((p): p is NonNullable => p !== null); + + logger.info('Heartbeat sweep', { + running_count: running.length, + eligible: plans.length, + 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; + } catch (err) { + logger.warn('Heartbeat sweep: reply edit failed (non-fatal)', { + task_id: plan.taskId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + + if (plans.length > MAX_EDITS_PER_SWEEP) { + logger.warn('Heartbeat sweep: capped β€” some eligible tasks not edited this round', { + eligible: plans.length, cap: MAX_EDITS_PER_SWEEP, + }); + } + logger.info('Heartbeat sweep complete', { edited }); +} diff --git a/cdk/src/handlers/shared/clarify-resume.ts b/cdk/src/handlers/shared/clarify-resume.ts new file mode 100644 index 000000000..8afd0b2a3 --- /dev/null +++ b/cdk/src/handlers/shared/clarify-resume.ts @@ -0,0 +1,120 @@ +/** + * 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. + */ + +/** + * Clarify-before-spend RESUME. + * + * A ``coding/new-task-v1`` run can HOLD to ask a clarifying question instead of + * guessing on a vague issue: it makes no commit, opens no PR, and persists the + * question as ``answer_text`` with ``code_changed=false``. The fanout dispatcher + * surfaces that as a πŸ’¬ question comment on the Linear issue. + * + * The gap this closes: when the user REPLIES to that question (``@bgagent ``), the standalone comment path found a task with no PR and silently + * no-op'd β€” the answer was dropped and the task never resumed. Now we recognise + * the clarify-HOLD shape and re-dispatch a fresh ``new-task-v1`` carrying the + * original ask, the question the agent posed, and the user's answer, so the run + * continues with the missing information. + * + * Pure + no I/O so the predicate and the resume-prompt assembly are + * unit-testable; the processor does the DDB read + ``createTaskCore`` dispatch. + */ + +/** The workflow a clarify-HOLD can only originate from (a brand-new task run). */ +export const NEW_TASK_WORKFLOW_ID = 'coding/new-task-v1'; + +/** + * The subset of a persisted TaskRecord needed to recognise a clarify-HOLD and + * reconstruct a resume. Read from the BASE table by ``task_id`` (the + * ``LinearIssueIndex`` GSI does not project ``code_changed`` / ``answer_text`` / + * ``task_description`` / the workflow pin β€” only ``pr_url`` and friends). + */ +export interface ClarifyHoldRow { + readonly resolved_workflow?: { readonly id?: string } | null; + readonly workflow_ref?: string; + readonly code_changed?: boolean; + readonly answer_text?: string; + readonly task_description?: string; + readonly pr_url?: string; + readonly pr_number?: number; +} + +/** True when this task ran ``coding/new-task-v1`` (via pin or the raw ref). */ +function isNewTaskWorkflow(row: ClarifyHoldRow): boolean { + const pinned = row.resolved_workflow?.id; + if (typeof pinned === 'string' && pinned === NEW_TASK_WORKFLOW_ID) return true; + // Fallback for rows written before the pin, or where only the raw ref exists. + // ``workflow_ref`` may be a bare ``coding/new-task-v1`` or carry a version. + return typeof row.workflow_ref === 'string' && row.workflow_ref.startsWith(NEW_TASK_WORKFLOW_ID); +} + +/** + * Recognise the clarify-HOLD shape precisely, so a resume never misfires on: + * - a running task (``code_changed`` unset until terminal), + * - an ordinary no-change PR iteration (has ``pr_url`` + is ``pr-iteration-v1``), + * - a completed task that shipped a PR (``pr_url`` present), + * - a plain failure (no ``answer_text``). + * + * The distinguishing signature is: a ``new-task-v1`` that finished with + * ``code_changed===false``, a non-empty ``answer_text`` (the question), and NO + * PR. See {@link ClarifyHoldRow}. + */ +export function isClarifyHold(row: ClarifyHoldRow | null | undefined): row is ClarifyHoldRow { + if (!row) return false; + if (row.code_changed !== false) return false; + if (typeof row.answer_text !== 'string' || row.answer_text.trim() === '') return false; + if (typeof row.pr_url === 'string' && row.pr_url.trim() !== '') return false; + if (typeof row.pr_number === 'number') return false; + return isNewTaskWorkflow(row); +} + +/** + * Assemble the resume task description: the original ask, the question the agent + * posed, and the user's answer β€” so the fresh run has the context that was + * missing the first time. Order matters: original intent first, then the + * clarifying exchange, so the agent reads it as "do the original thing, now with + * this detail resolved". + * + * ``question`` is the held ``answer_text``; ``answer`` is the user's reply + * (already stripped of the ``@bgagent`` mention by the comment parser). A blank + * original (older rows) degrades to just the exchange. + */ +export function buildClarifyResumeDescription( + originalDescription: string | undefined, + question: string | undefined, + answer: string, +): string { + const parts: string[] = []; + const orig = (originalDescription ?? '').trim(); + if (orig) parts.push(orig); + const q = (question ?? '').trim(); + const a = answer.trim(); + const exchange: string[] = []; + if (q) exchange.push(`You asked: ${q}`); + exchange.push(`The reviewer answered: ${a}`); + parts.push( + [ + '---', + 'This continues an earlier run that paused to ask a clarifying question.', + ...exchange, + 'Proceed with the original request using this answer.', + ].join('\n'), + ); + return parts.join('\n\n'); +} diff --git a/cdk/src/handlers/shared/failure-reply.ts b/cdk/src/handlers/shared/failure-reply.ts new file mode 100644 index 000000000..ff65ad963 --- /dev/null +++ b/cdk/src/handlers/shared/failure-reply.ts @@ -0,0 +1,218 @@ +/** + * 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. + */ + +/** + * "Failure is a conversation": renders the threaded ❌ reply the agent posts + * beneath a human's ``@bgagent`` comment when the requested iteration does not + * land cleanly. Two distinct shapes: + * + * - BUILD/TEST failure (the agent ran and opened/updated a PR, but the build + * or tests are red): a sanitized ONE-LINE reason pointing at the agent's + * CloudWatch build log. We deliberately do NOT dump the raw build output β€” + * it's untrusted repo code. The pointer is CloudWatch (by task id), NOT the + * PR's GitHub checks: the agent runs the configured build (``mise run + * build``) INSIDE the microVM, so that output lives in CloudWatch, and the + * target repo may have no GitHub CI at all. (Reported in practice as + * "sub-issues pass, parent build fails" and "saw nothing in the PR" β€” the + * old "see the PR's checks" copy pointed at the wrong, often-empty surface.) + * + * - AGENT-ITSELF failure (the agent crashed / timed out / hit a cap before a + * clean terminal): the classified one-line title + a TRUNCATED excerpt of + * the raw error, plus a pointer to the full CloudWatch logs by task id. + * + * Both always end by inviting a reply β€” the failure reply is answerable, so + * the user replies ``@bgagent `` and the comment trigger re-runs the + * iteration on the same PR. Pure and deterministic; no I/O. + */ + +import { classifyError, retryGuidance } from './error-classifier'; +import type { TaskStatusType } from '../../constructs/task-status'; + +/** Max chars of the raw agent error surfaced inline (the rest is in CloudWatch). */ +const EXCERPT_MAX = 200; + +export interface FailureReplyInput { + /** Terminal task status. */ + readonly status: TaskStatusType | string; + /** Whether the post-change build/tests passed. false β‡’ build/test failure. */ + readonly buildPassed?: boolean | null; + /** Raw agent error_message, if any (drives the agent-failure classification). */ + readonly errorMessage?: string | null; + /** Task id β€” surfaced so the user can find the run in CloudWatch. */ + readonly taskId: string; +} + +/** + * The agent pipeline's signature for "the AGENT finished fine, but the build + * verification GATE failed" (a build/test regression). Verified against the + * live pipeline: it gates this to ``status=FAILED`` with + * ``error_message="Task did not succeed (agent_status='success', build_ok=False)"`` + * and leaves the separate ``build_passed`` attribute null β€” so the previous + * ``COMPLETED && build_passed===false`` check NEVER matched a real regression + * and every build failure fell through to the (wrong) agent-crash copy. We + * key off the real persisted signal instead. See + * ``agent/src/pipeline.py`` ``_resolve_overall_task_status`` / + * ``_apply_post_hook_gates``. + */ +const BUILD_GATE_FAILED_RE = /agent_status=['"]?(success|end_turn)['"]?.*build_ok\s*=\s*(False|timeout)/i; + +/** + * The agent finished cleanly but the build gate failed because the build + * VERIFICATION TIMED OUT (exceeded ``BUILD_VERIFY_TIMEOUT_S`` and was killed) β€” + * a different diagnosis from a genuine red build. ``agent/src/pipeline.py`` + * ``_resolve_overall_task_status`` emits ``build_ok=timeout`` for this case so + * we render "build timed out" rather than the misleading "build/tests failed" + * (the build didn't fail β€” it didn't finish in time; the fix is a faster build + * or a higher cap, not a code change). + */ +const BUILD_GATE_TIMEOUT_RE = /agent_status=['"]?(success|end_turn)['"]?.*build_ok\s*=\s*timeout/i; + +/** + * True when the failure is a BUILD/TEST failure (the agent completed and a PR + * exists, but the verification gate is red) vs an agent-itself failure + * (crash / cap / timeout). Two shapes are accepted: + * - the live gating shape: ``error_message`` says ``agent_status='success' … + * build_ok=False`` (the agent succeeded; only the build gate failed), OR + * ``build_ok=timeout`` (the build gate timed out β€” also a build-side, not + * agent-crash, failure); OR + * - the explicit field shape: a terminal task with ``build_passed === false`` + * and no crash error_message (defensive β€” e.g. an informational-gate path + * that surfaces build_passed directly). + */ +function isBuildFailure(input: Pick): boolean { + if (input.errorMessage && BUILD_GATE_FAILED_RE.test(input.errorMessage)) { + return true; + } + return input.buildPassed === false && !input.errorMessage; +} + +/** True when the build gate failed specifically because it TIMED OUT (a subset of build failures). */ +function isBuildTimeout(input: Pick): boolean { + return !!input.errorMessage && BUILD_GATE_TIMEOUT_RE.test(input.errorMessage); +} + +/** Collapse whitespace + clip to EXCERPT_MAX chars with an ellipsis. Strips the + * internal `[auto-retried]` marker (it drives the guidance, not user-facing text). */ +function excerpt(raw: string): string { + const oneLine = raw.replace(/\s*\[auto-retried\]\s*/gi, ' ').replace(/\s+/g, ' ').trim(); + return oneLine.length > EXCERPT_MAX ? `${oneLine.slice(0, EXCERPT_MAX)}…` : oneLine; +} + +/** + * Render the ❌ failure reply body. Best-effort, never throws. + */ +export function renderFailureReply(input: FailureReplyInput): string { + if (isBuildTimeout(input)) { + // Build verification TIMED OUT β€” a distinct diagnosis from a red build: + // the build didn't fail, it didn't finish within the time limit. Say so, + // so the user fixes the right thing (a slow build / a higher cap), not + // their code. Still answerable β€” a reply re-runs it. + return ( + '❌ I made the change, but the build/tests didn\'t finish in time (timed ' + + `out) β€” see the build log in CloudWatch for task \`${input.taskId}\`. ` + + "Reply with guidance and I'll try again." + ); + } + if (isBuildFailure(input)) { + // Build/test failure β€” one line. Point at the agent's CloudWatch build log + // (by task id), NOT the PR's GitHub checks: the agent ran the configured + // build inside the microVM, so that's where the failing output is, and the + // repo may have no GitHub CI. No raw output dump (untrusted repo code). + return ( + "❌ I made the change, but the build/tests didn't pass β€” see the build " + + `log in CloudWatch for task \`${input.taskId}\`. Reply with guidance ` + + "and I'll try again." + ); + } + + // Agent-itself failure: classified title + truncated excerpt + CloudWatch + + // a category-aware NEXT STEP. The old copy always ended "Reply with guidance + // and I'll try again" β€” misleading for an infra/deploy-race failure (the user's + // guidance is irrelevant; it's a plain retry) and for a not-retryable auth/ + // config/guardrail failure (a retry won't help; an admin or an edit is needed). + // retryGuidance answers the user's real question β€” "retry, or tell my admin?" + const classification = classifyError(input.errorMessage); + const title = classification?.title ?? "the task didn't complete"; + const detail = input.errorMessage ? ` ${excerpt(input.errorMessage)}` : ''; + // The orchestrator stamps `[auto-retried]` on a session-start error that already + // auto-retried once (transient) β€” so the guidance says "I tried again, still + // failed" instead of "reply to retry". + const autoRetried = wasAutoRetried(input.errorMessage); + const nextStep = classification + ? retryGuidance(classification, autoRetried) + : 'Reply here with any extra guidance and I\'ll try again.'; + return ( + `❌ ${title} β€”${detail} see CloudWatch for task \`${input.taskId}\`. ` + + nextStep + ); +} + +/** True when the orchestrator marked this failure as already auto-retried once. */ +function wasAutoRetried(errorMessage?: string | null): boolean { + return !!errorMessage && /\[auto-retried\]/i.test(errorMessage); +} + +/** + * Compose the SHORT one-line failure reason shown as a sub-line under a ❌ row + * on the parent epic panel. The panel path + * (``reconcileTerminalChild β†’ refreshPanelAndSettle``) is where a failed node β€” + * crucially the SYNTHETIC integration node, which has no Linear sub-issue and + * therefore no comment-iteration reply β€” would otherwise surface as a bare + * "❌ … β€” failed" with no reason and no pointer. This gives the user the one + * thing they need to debug: WHAT failed (build vs agent-crash) and WHERE to + * read it (CloudWatch by task id). + * + * Same security stance as {@link renderFailureReply}: classified reason + task + * id only, never the raw (untrusted) build output. Returns null when there's no + * task id to point at (nothing actionable to render). ``isIntegration`` tailors + * the build-failure wording to name the merge β€” the integration node's failure + * is specifically "the combined build after merging the sub-issue branches", + * which is the exact failure mode reported and the panel must make legible. + */ +export function renderPanelFailureReason(input: { + readonly buildPassed?: boolean | null; + readonly errorMessage?: string | null; + readonly taskId?: string; + readonly isIntegration?: boolean; +}): string | null { + if (!input.taskId) return null; + if (isBuildTimeout(input)) { + // Distinct from a red build: the build didn't finish within the time limit. + const what = input.isIntegration + ? 'Combined build timed out after merging the sub-issue branches' + : 'Build/tests timed out'; + return `${what} β€” see the build log in CloudWatch for task \`${input.taskId}\`.`; + } + if (isBuildFailure(input)) { + const what = input.isIntegration + ? 'Combined build failed after merging the sub-issue branches' + : 'Build/tests failed'; + return `${what} β€” see the build log in CloudWatch for task \`${input.taskId}\`.`; + } + const classification = classifyError(input.errorMessage); + const title = classification?.title ?? "the task didn't complete"; + // Append the category-aware next step so a reader of the epic panel (where this + // sub-line lives) knows whether to just retry or escalate β€” the exact "can I + // retry, or is this an admin thing?" gap the epic rollup left open on the + // "Agent session failed to start" (deploy-race) rows. + const nextStep = classification + ? ` ${retryGuidance(classification, wasAutoRetried(input.errorMessage))}` + : ''; + return `${title} β€” see CloudWatch for task \`${input.taskId}\`.${nextStep}`; +} diff --git a/cdk/src/handlers/shared/iteration-heartbeat.ts b/cdk/src/handlers/shared/iteration-heartbeat.ts new file mode 100644 index 000000000..a59c50cae --- /dev/null +++ b/cdk/src/handlers/shared/iteration-heartbeat.ts @@ -0,0 +1,151 @@ +/** + * 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. + */ + +/** + * Mid-run liveness heartbeat (pure core). + * + * Observed in practice: a comment-triggered iteration ran for 22 minutes showing + * only "πŸ€– Starting on this issue", then a terminal ❌ β€” a total black box in + * between. The platform already has the data to do better: an + * iteration task carries its maturing-reply comment id in + * ``channel_metadata.iteration_reply_comment_id`` and stamps ``created_at``; the + * agent bumps ``agent_heartbeat_at`` every 45s while RUNNING. + * + * This module decides, for ONE RUNNING iteration task, whether a scheduled sweep + * should edit its maturing reply to show liveness (elapsed + an optional progress + * note) β€” and what the new body is. It edits the SAME existing reply comment in + * place (no new comments β€” the user's explicit "don't clutter the Linear UI" + * constraint), reusing the iteration reply's ``working`` state. + * + * Pure + deterministic (``now`` injected); all I/O lives in the sweep handler. + */ + +import { renderMaturingReply } from './iteration-reply'; + +/** + * Below this elapsed floor a RUNNING iteration is NOT heartbeat-updated β€” a task + * that just started doesn't need a liveness nudge, and editing too early would + * fight the trigger-time ack / pr_created edit. Matches the renderer's own + * elapsed floor so the suffix is meaningful when we do edit. + */ +export const HEARTBEAT_MIN_ELAPSED_S = 90; + +/** The fields the sweep reads off a RUNNING task's record (already DDB-unmarshalled). */ +export interface HeartbeatTaskView { + readonly taskId: string; + readonly status: string; + /** ISO timestamp the task was created (drives elapsed). */ + readonly createdAt?: string; + /** Trigger channel β€” only 'linear' is wired for the reply edit. */ + readonly channelSource?: string; + /** Linear workspace id (for the per-workspace OAuth token). */ + readonly linearWorkspaceId?: string; + /** The maturing reply comment id stamped at trigger time. */ + readonly iterationReplyCommentId?: string; + /** The human comment that triggered the iteration (reply parent). */ + readonly triggerCommentId?: string; + /** The issue the trigger comment lives on (parent epic or sub-issue). */ + readonly triggerCommentIssueId?: string; + /** + * Whether this task carries the orchestration-iteration marker. NOT used for + * eligibility β€” both orchestration AND standalone @bgagent iterations have a + * maturing reply, and a STANDALONE iteration deliberately omits this marker + * (see where the webhook processor dispatches a standalone comment iteration). + * Eligibility keys on the reply fields below, so standalone iterations are + * covered too. Kept on the view for logging/diagnostics only. + */ + readonly isIteration?: boolean; + /** PR number, when known (makes the working line name the PR). */ + readonly prNumber?: number | null; + /** PR url, when known (clickable PR reference). */ + readonly prUrl?: string | null; + /** + * Latest agent progress note (sanitized milestone detail). + * + * RESERVED β€” nothing populates this yet. The sweep's ``toView`` does not set it + * and there is no persisted progress-note attribute to read, so in production + * the heartbeat currently shows elapsed time only. Kept because the render path + * is written and tested; wiring it needs an agent-side progress write, which + * belongs with that work rather than here. Do not document it as a shipped + * capability until a producer exists. + */ + readonly latestProgressNote?: string; +} + +/** What the sweep should do for one task. */ +export interface HeartbeatPlan { + readonly taskId: string; + readonly linearWorkspaceId: string; + readonly issueId: string; + readonly parentCommentId: string; + readonly replyId: string; + readonly body: string; + readonly elapsedS: number; +} + +/** Parse an ISO timestamp to epoch ms, or null if unusable. */ +function parseIso(ts: string | undefined): number | null { + if (!ts) return null; + const ms = Date.parse(ts); + return Number.isFinite(ms) ? ms : 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 + * 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; + + // Eligibility = "this task has a maturing Linear reply to keep alive". That's + // exactly the set of comment-triggered iterations β€” BOTH orchestration and + // STANDALONE @bgagent iterations (the latter omits the orchestration marker + // 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) { + return null; + } + + const startedMs = parseIso(task.createdAt); + if (startedMs === null) return null; + const elapsedS = Math.max(0, Math.round((nowMs - startedMs) / 1000)); + if (elapsedS < HEARTBEAT_MIN_ELAPSED_S) return null; + + const body = renderMaturingReply({ + state: 'working', + ...(task.prNumber != null && { prNumber: task.prNumber }), + ...(task.prUrl != null && { prUrl: task.prUrl }), + elapsedS, + ...(task.latestProgressNote ? { progressNote: task.latestProgressNote } : {}), + }); + + return { + taskId: task.taskId, + linearWorkspaceId, + issueId: triggerCommentIssueId, + parentCommentId: triggerCommentId, + replyId: iterationReplyCommentId, + body, + elapsedS, + }; +} diff --git a/cdk/src/handlers/shared/iteration-reply-claim.ts b/cdk/src/handlers/shared/iteration-reply-claim.ts new file mode 100644 index 000000000..aa8a8d060 --- /dev/null +++ b/cdk/src/handlers/shared/iteration-reply-claim.ts @@ -0,0 +1,262 @@ +/** + * 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. + */ + +/** + * The once-only claim on a comment-iteration's terminal reply. + * + * One maturing reply per iteration matures πŸ‘€ β†’ πŸ”„ β†’ βœ…/πŸ’¬/❌, and three + * independent writers edit it: the terminal settle (the reconciler for an + * orchestration iteration, the fan-out dispatcher for a standalone one), the + * progress milestone, and the liveness heartbeat. Their stream records are + * redelivered β€” the cascade source's was observed arriving 3Γ— live β€” so the + * terminal writers coordinate through a single conditional attribute on the + * iteration task's own record: whoever writes ``ack_replied_at`` first owns the + * reply, and the losers skip. + * + * That protocol lived as copy-pasted conditional expressions in each writer, + * where one of them dropping a step went unnoticed: a terminal writer claimed, + * its edit failed, and the claim stayed taken β€” so no redelivery could retry it + * AND the progress writers read the claim as "already settled" and stood down + * too, freezing the reply at "πŸ‘€ On it" with no outcome ever shown. Keeping the + * three operations together makes the claim/release pairing visible. + */ + +import { type DynamoDBDocumentClient, GetCommand, UpdateCommand } from '@aws-sdk/lib-dynamodb'; +import { logger } from './logger'; + +/** Outcome of trying to become the one writer of a task's terminal reply. */ +export type ReplyClaimOutcome = + /** This caller owns the reply and must post it (or release the claim). */ + | { readonly won: true; readonly stamp: string } + /** + * Another caller already owns it (a redelivery of the same event), or the + * claim write itself failed. Either way this caller must not reply: on a lost + * race a reply would duplicate, and on an error we cannot know whether the + * write landed, so replying could duplicate too. + */ + | { readonly won: false }; + +/** + * Claim the right to write a task's terminal reply, exactly once. + * + * ``stamp`` is recorded so {@link releaseReplyClaim} can prove the claim it + * removes is still the one this caller made. + */ +export async function claimTerminalReply( + ddb: DynamoDBDocumentClient, + tableName: string, + taskId: string, + stamp: string, +): Promise { + try { + await ddb.send(new UpdateCommand({ + TableName: tableName, + Key: { task_id: taskId }, + UpdateExpression: 'SET ack_replied_at = :now', + ConditionExpression: 'attribute_not_exists(ack_replied_at)', + ExpressionAttributeValues: { ':now': stamp }, + })); + return { won: true, stamp }; + } catch (err) { + if ((err as { name?: string })?.name !== 'ConditionalCheckFailedException') { + logger.warn('Terminal-reply claim write failed β€” not replying', { + task_id: taskId, + error: err instanceof Error ? err.message : String(err), + }); + } + return { won: false }; + } +} + +/** + * How many times a failed terminal reply may be re-attempted. + * + * Bounded, and the bound is load-bearing rather than cautious. Releasing the + * claim is itself a write to the task record, and the reconciler consumes that + * table's stream β€” so a release re-wakes the very handler that performed it. When + * the reply cannot ever succeed (its comment was deleted, the issue is gone) an + * unbounded release therefore spins: release β†’ stream event β†’ retry β†’ fail β†’ + * release. Observed live at ~900 iterations in six minutes before this bound + * existed. Small on purpose: a reply that fails three times is not failing for a + * reason another attempt fixes. + */ +export const MAX_REPLY_ATTEMPTS = 3; + +/** What happened when a failed reply tried to hand its claim back. */ +export type ReplyReleaseOutcome = + /** The claim is free again; a redelivery may re-attempt the reply. */ + | 'released' + /** + * The retry budget is spent, so the claim was deliberately KEPT β€” no further + * attempt will be made. The caller must still convey the outcome some other + * way (the reaction on the trigger comment), because the reply itself is now + * never going to say it. + */ + | 'exhausted' + /** The claim is no longer this caller's, so another delivery owns the reply. */ + | 'not_ours'; + +/** + * Give the claim back after failing to write the reply, so a later delivery of + * the same event can try again β€” up to {@link MAX_REPLY_ATTEMPTS}. + * + * Without any release a failed reply is permanent in two ways: no redelivery may + * re-attempt it, and the progress and heartbeat writers β€” which read this same + * attribute as "an outcome has landed" β€” also stand down, leaving the reply + * stuck on its last progress text. Without a BOUND on the release, a + * never-succeeding reply spins instead (see {@link MAX_REPLY_ATTEMPTS}). Both + * failure modes are worse than one unanswered reply, so the release is bounded + * and the attempt count lives on the record next to the claim. + * + * Conditional on the exact stamp this caller wrote. A blind delete would, in the + * interleaving where this release is delayed past another delivery's successful + * claim-and-reply, strip that writer's claim and let a third delivery reply + * again β€” turning one lost reply into a duplicated one. + * + * Re-attempting is safe for the usual case, an EDIT of an existing reply, which + * converges on the same body. Where there was no reply id to edit and the reply + * had to be created, a create whose response was lost in transit could be + * created twice β€” accepted, because a duplicated reply is noise whereas a + * missing one leaves the human's request looking unanswered. + */ +export async function releaseReplyClaim( + ddb: DynamoDBDocumentClient, + tableName: string, + taskId: string, + stamp: string, +): Promise { + try { + // Count the attempt in the SAME write that frees the claim, so the budget + // can't be lost between the two (which would restore the unbounded spin). + const res = await ddb.send(new UpdateCommand({ + TableName: tableName, + Key: { task_id: taskId }, + UpdateExpression: 'REMOVE ack_replied_at SET ack_reply_attempts = if_not_exists(ack_reply_attempts, :zero) + :one', + ConditionExpression: + 'ack_replied_at = :ours AND (attribute_not_exists(ack_reply_attempts) OR ack_reply_attempts < :max)', + ExpressionAttributeValues: { + ':ours': stamp, ':zero': 0, ':one': 1, ':max': MAX_REPLY_ATTEMPTS, + }, + ReturnValues: 'UPDATED_NEW', + })); + logger.info('Released the terminal-reply claim after a failed reply β€” a retry may re-attempt', { + task_id: taskId, + attempt: (res.Attributes as { ack_reply_attempts?: number } | undefined)?.ack_reply_attempts, + max_attempts: MAX_REPLY_ATTEMPTS, + }); + return 'released'; + } catch (err) { + // Losing the release leaves the reply un-retryable, which is the bug this + // exists to prevent β€” so log loudly. Never throw: the caller is on a + // best-effort feedback path. + const conditional = (err as { name?: string })?.name === 'ConditionalCheckFailedException'; + if (conditional) { + // Two causes, and they need different handling by the caller: either the + // budget is spent (the claim stays, so nothing retries and the outcome must + // be conveyed another way) or the claim is no longer ours (another delivery + // owns the reply and nothing is stuck). Distinguish by re-reading. + const spent = await attemptsExhausted(ddb, tableName, taskId); + if (spent) { + logger.error('Giving up on a terminal reply after repeated failures β€” settling without it', { + event: 'iteration_reply.attempts_exhausted', + task_id: taskId, + max_attempts: MAX_REPLY_ATTEMPTS, + }); + return 'exhausted'; + } + logger.info('Terminal-reply claim is no longer ours β€” another delivery owns the reply', { + task_id: taskId, + }); + return 'not_ours'; + } + logger.warn('Could not release the terminal-reply claim', { + event: 'iteration_reply.claim_release_failed', + task_id: taskId, + claim_no_longer_ours: conditional, + ...(conditional ? {} : { error: err instanceof Error ? err.message : String(err) }), + }); + // An infra failure (throttle, AccessDenied) leaves the claim held. Reported as + // exhausted so the caller settles the comment rather than assuming a retry + // that may never come. + return 'exhausted'; + } +} + +/** + * Has this task's reply used up its retry budget? Read strongly-consistent: the + * increment it is checking was written moments ago by a sibling invocation. + * + * Treats an unreadable record as exhausted β€” the caller then settles the comment + * instead of leaving a request looking unanswered while it waits for a retry it + * cannot confirm. + */ +async function attemptsExhausted( + ddb: DynamoDBDocumentClient, + tableName: string, + taskId: string, +): Promise { + try { + const res = await ddb.send(new GetCommand({ + TableName: tableName, + Key: { task_id: taskId }, + ProjectionExpression: 'ack_reply_attempts', + ConsistentRead: true, + })); + const attempts = (res.Item as { ack_reply_attempts?: number } | undefined)?.ack_reply_attempts ?? 0; + return attempts >= MAX_REPLY_ATTEMPTS; + } catch { + return true; + } +} + +/** + * Has a terminal reply already been claimed for this task? + * + * For the PROGRESS writers, which must not render "working" over an outcome. + * Read strongly-consistent, because the whole point is to observe a write + * another Lambda may have made moments ago β€” an eventually-consistent read is + * precisely how a stale progress edit slips past. + * + * Fails OPEN (false) on a read error: a missed progress edit is cosmetic, while + * suppressing progress on a task that never settled would leave the reply frozen + * at "On it". Note this marker is stamped just BEFORE the reply it announces is + * rendered, so it is necessary but not sufficient β€” the surface also compares + * the current body (see the ``skipIfSettled`` reply option). + */ +export async function terminalReplyClaimed( + ddb: DynamoDBDocumentClient, + tableName: string, + taskId: string, +): Promise { + try { + const res = await ddb.send(new GetCommand({ + TableName: tableName, + Key: { task_id: taskId }, + ProjectionExpression: 'ack_replied_at', + ConsistentRead: true, + })); + return Boolean((res.Item as { ack_replied_at?: string } | undefined)?.ack_replied_at); + } catch (err) { + logger.warn('Could not check whether the terminal reply already landed', { + task_id: taskId, + error: err instanceof Error ? err.message : String(err), + }); + return false; + } +} diff --git a/cdk/src/handlers/shared/trigger-label.ts b/cdk/src/handlers/shared/trigger-label.ts new file mode 100644 index 000000000..b734f7107 --- /dev/null +++ b/cdk/src/handlers/shared/trigger-label.ts @@ -0,0 +1,51 @@ +/** + * 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. + */ + +/** + * Trigger-label helpers. + * + * A project's ``label_filter`` (default ``bgagent``) names the Linear label that + * starts a task. Kept pure β€” no I/O, no Linear or AWS types β€” so the label + * decision is unit-testable on its own; the webhook processor does the I/O of + * resolving the filter from the project mapping. + */ + +/** Normalise a label name for comparison: trim + lower-case. */ +function norm(name: string | undefined | null): string { + return (name ?? '').trim().toLowerCase(); +} + +/** The base trigger label when a project doesn't override ``label_filter``. */ +export const DEFAULT_LABEL_FILTER = 'bgagent'; + +/** + * Suffix (after ``:``) that requests the one-time explainer of what the trigger + * labels do, and creates NO task. + */ +export const HELP_SUFFIX = 'help'; + +/** True when the ``:help`` explainer label is present (any case). */ +export function hasHelpLabel( + labelNames: readonly (string | undefined | null)[], + labelFilter: string = DEFAULT_LABEL_FILTER, +): boolean { + const base = norm(labelFilter) || DEFAULT_LABEL_FILTER; + const help = `${base}:${HELP_SUFFIX}`; + return labelNames.some((n) => norm(n) === help); +} diff --git a/cdk/test/constructs/iteration-heartbeat.test.ts b/cdk/test/constructs/iteration-heartbeat.test.ts new file mode 100644 index 000000000..dc9758b47 --- /dev/null +++ b/cdk/test/constructs/iteration-heartbeat.test.ts @@ -0,0 +1,99 @@ +/** + * 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 { App, Duration, Stack } from 'aws-cdk-lib'; +import { Match, Template } from 'aws-cdk-lib/assertions'; +import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; +import { IterationHeartbeat } from '../../src/constructs/iteration-heartbeat'; + +function synth(props?: { schedule?: Duration }) { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + const taskTable = new dynamodb.Table(stack, 'TaskTable', { + partitionKey: { name: 'task_id', type: dynamodb.AttributeType.STRING }, + }); + new IterationHeartbeat(stack, 'IterationHeartbeat', { taskTable, ...props }); + return Template.fromStack(stack); +} + +describe('IterationHeartbeat', () => { + test('creates a scheduled Lambda that runs every 2 minutes by default', () => { + const template = synth(); + template.resourceCountIs('AWS::Events::Rule', 1); + template.hasResourceProperties('AWS::Events::Rule', { + ScheduleExpression: 'rate(2 minutes)', + State: 'ENABLED', + }); + }); + + test('honours a caller-supplied schedule', () => { + synth({ schedule: Duration.minutes(5) }).hasResourceProperties('AWS::Events::Rule', { + ScheduleExpression: 'rate(5 minutes)', + }); + }); + + test('the rule targets the sweep function', () => { + const template = synth(); + const fns = template.findResources('AWS::Lambda::Function'); + const sweep = Object.keys(fns).find((id) => id.startsWith('IterationHeartbeatSweepFn')); + expect(sweep).toBeDefined(); + template.hasResourceProperties('AWS::Events::Rule', { + Targets: Match.arrayWith([Match.objectLike({ Arn: { 'Fn::GetAtt': [sweep, 'Arn'] } })]), + }); + }); + + test('surfaces the task table + its status index to the handler', () => { + // The sweep queries the StatusIndex GSI for RUNNING tasks; without both env + // vars it has nothing to sweep and silently no-ops. + const template = synth(); + const fns = template.findResources('AWS::Lambda::Function'); + const sweep = Object.values(fns).find((f) => { + const vars = (f as { Properties?: { Environment?: { Variables?: Record } } }) + .Properties?.Environment?.Variables ?? {}; + return 'TASK_STATUS_INDEX_NAME' in vars; + }); + expect(sweep).toBeDefined(); + const vars = (sweep as { Properties: { Environment: { Variables: Record } } }) + .Properties.Environment.Variables; + expect(vars.TASK_TABLE_NAME).toBeDefined(); + expect(vars.TASK_STATUS_INDEX_NAME).toBeDefined(); + }); + + test('is granted READ on the task table and never write', () => { + // The construct's own comment claims read-only. The sweep edits a Linear + // comment, not a task row, so a write grant here would be unexplained + // privilege on the platform's most sensitive table. + const template = synth(); + const policies = template.findResources('AWS::IAM::Policy'); + const actions = JSON.stringify(Object.values(policies) + .flatMap((p) => (p as { Properties: { PolicyDocument: { Statement: unknown[] } } }) + .Properties.PolicyDocument.Statement)); + expect(actions).toContain('dynamodb:Query'); + for (const write of ['dynamodb:PutItem', 'dynamodb:UpdateItem', 'dynamodb:DeleteItem', 'dynamodb:BatchWriteItem']) { + expect(actions).not.toContain(write); + } + }); + + test('runs on ARM with a bounded timeout', () => { + synth().hasResourceProperties('AWS::Lambda::Function', { + Architectures: ['arm64'], + Timeout: 120, + }); + }); +}); diff --git a/cdk/test/handlers/iteration-heartbeat-sweep.test.ts b/cdk/test/handlers/iteration-heartbeat-sweep.test.ts new file mode 100644 index 000000000..8c847790c --- /dev/null +++ b/cdk/test/handlers/iteration-heartbeat-sweep.test.ts @@ -0,0 +1,127 @@ +/** + * 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. + */ + +// The sweep's eligibility + body rendering are covered by the pure planner's own +// tests; these cover the handler's I/O wiring β€” that a plan reaches the channel +// as the right issue/parent/reply triple, and that failures stay non-fatal. + +const ddbSend = jest.fn(); +jest.mock('@aws-sdk/client-dynamodb', () => ({ + DynamoDBClient: jest.fn(() => ({ send: ddbSend })), + QueryCommand: jest.fn((input: unknown) => ({ _type: 'Query', input })), +})); + +// Mock at the per-surface helper, NOT the channel: the real Linear adapter runs, +// so the test exercises the actual channel-op β†’ surface-call mapping. +const upsertThreadedReplyMock = jest.fn(); +jest.mock('../../src/handlers/shared/linear-feedback', () => ({ + upsertThreadedReply: (...args: unknown[]) => upsertThreadedReplyMock(...args), +})); + +jest.mock('../../src/handlers/shared/logger', () => ({ + logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, +})); + +const REGISTRY = 'LinearWorkspaceRegistry'; +process.env.TASK_TABLE_NAME = 'TaskTable'; +process.env.LINEAR_WORKSPACE_REGISTRY_TABLE_NAME = REGISTRY; + +// The sweep compares each task's created_at against the wall clock, so pin now. +const NOW = Date.parse('2026-06-29T13:30:00Z'); + +// Imported after the env + mocks above: the handler reads them at module load. +import { handler } from '../../src/handlers/iteration-heartbeat-sweep'; + +/** A RUNNING iteration task image, as the StatusIndex projects it. */ +function runningTask(overrides: { taskId?: string; createdAt?: string } = {}) { + return { + task_id: { S: overrides.taskId ?? 'task-1' }, + status: { S: 'RUNNING' }, + created_at: { S: overrides.createdAt ?? '2026-06-29T13:20:00Z' }, // 10 min in + channel_source: { S: 'linear' }, + pr_number: { N: '42' }, + channel_metadata: { + M: { + linear_workspace_id: { S: 'ws-1' }, + iteration_reply_comment_id: { S: 'reply-1' }, + trigger_comment_id: { S: 'cmt-1' }, + trigger_comment_issue_id: { S: 'issue-1' }, + orchestration_iteration: { S: 'true' }, + }, + }, + }; +} + +beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Date, 'now').mockReturnValue(NOW); + upsertThreadedReplyMock.mockResolvedValue('reply-1'); + ddbSend.mockResolvedValue({ Items: [runningTask()] }); +}); + +afterEach(() => jest.restoreAllMocks()); + +describe('iteration heartbeat sweep', () => { + test('edits the maturing reply through the channel, preserving a landed preview link', async () => { + await handler(); + + expect(upsertThreadedReplyMock).toHaveBeenCalledTimes(1); + const [ctx, issueId, parentCommentId, body, existingReplyId, options] = upsertThreadedReplyMock.mock.calls[0]; + // The channel builds the per-workspace context from the task's own workspace. + expect(ctx).toEqual({ linearWorkspaceId: 'ws-1', registryTableName: REGISTRY }); + // The reply is addressed to the issue the trigger comment lives on, threaded + // under that comment, editing the reply captured at trigger time. + expect(issueId).toBe('issue-1'); + expect(parentCommentId).toBe('cmt-1'); + expect(existingReplyId).toBe('reply-1'); + expect(body).toContain('πŸ”„ Working'); + // A separate writer appends the deploy preview to this same reply, so the + // heartbeat must carry it over rather than overwrite it β€” AND, as the least + // important writer of the three, it must yield entirely once the reply has + // settled rather than re-render "working" over an outcome. + // A liveness tick is progress, never an outcome, so it never asks for the + // terminal writer's restore-if-overwritten behaviour. + expect(options).toEqual({ preservePreview: true, skipIfSettled: true, repairIfOverwritten: false }); + }); + + test('a task with no reply to mature is skipped, not edited', async () => { + const noReply = runningTask(); + delete (noReply.channel_metadata.M as Record).iteration_reply_comment_id; + ddbSend.mockResolvedValue({ Items: [noReply] }); + + await handler(); + 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' })], + }); + upsertThreadedReplyMock.mockRejectedValueOnce(new Error('surface hiccup')); + + await expect(handler()).resolves.toBeUndefined(); + expect(upsertThreadedReplyMock).toHaveBeenCalledTimes(2); + }); + + test('a query failure is swallowed β€” a cosmetic sweep never throws', async () => { + ddbSend.mockRejectedValue(new Error('throttled')); + await expect(handler()).resolves.toBeUndefined(); + expect(upsertThreadedReplyMock).not.toHaveBeenCalled(); + }); +}); diff --git a/cdk/test/handlers/iteration-reply-claim.test.ts b/cdk/test/handlers/iteration-reply-claim.test.ts new file mode 100644 index 000000000..5c3a2be1e --- /dev/null +++ b/cdk/test/handlers/iteration-reply-claim.test.ts @@ -0,0 +1,215 @@ +/** + * 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. + */ + +// The conditional-write semantics of the once-only reply claim, on their own. +// Both handlers that use it are tested end-to-end elsewhere; this pins the +// expressions themselves, because getting a condition subtly wrong here is +// invisible in a mocked handler test but decides whether a human's request ends +// up answered once, twice, or never. + +import { + claimTerminalReply, + MAX_REPLY_ATTEMPTS, + releaseReplyClaim, + terminalReplyClaimed, +} from '../../src/handlers/shared/iteration-reply-claim'; +import { logger } from '../../src/handlers/shared/logger'; + +const send = jest.fn(); +const ddb = { send } as unknown as Parameters[0]; + +const TABLE = 'TaskTable'; +const TASK = 'iter-task-1'; +const STAMP = '2026-07-26T01:14:04.301Z'; + +/** The rejection DynamoDB raises when a ConditionExpression is not satisfied. */ +function conditionalFailure(): Error { + const err = new Error('The conditional request failed'); + (err as { name?: string }).name = 'ConditionalCheckFailedException'; + return err; +} + +/** The command input of the Nth send call. */ +function inputOf(call: number): Record { + return (send.mock.calls[call][0] as { input: Record }).input; +} + +// Spied rather than module-mocked (the repo's idiom) so the assertions read the +// same logger the code under test writes to. +let warnSpy: jest.SpyInstance; +let infoSpy: jest.SpyInstance; + +beforeEach(() => { + jest.clearAllMocks(); + send.mockResolvedValue({}); + warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => undefined); + infoSpy = jest.spyOn(logger, 'info').mockImplementation(() => undefined); +}); + +afterEach(() => jest.restoreAllMocks()); + +/** Structured fields of the Nth warn call. */ +function warnFields(call = 0): Record { + return warnSpy.mock.calls[call][1] as Record; +} + +describe('claimTerminalReply', () => { + test('the first caller wins and gets its stamp back for a later release', async () => { + const outcome = await claimTerminalReply(ddb, TABLE, TASK, STAMP); + + expect(outcome).toEqual({ won: true, stamp: STAMP }); + expect(inputOf(0)).toMatchObject({ + TableName: TABLE, + Key: { task_id: TASK }, + UpdateExpression: 'SET ack_replied_at = :now', + // Only-if-absent is what makes this once-only across redeliveries. + ConditionExpression: 'attribute_not_exists(ack_replied_at)', + ExpressionAttributeValues: { ':now': STAMP }, + }); + }); + + test('a caller that loses the race does NOT win, and stays quiet about it', async () => { + send.mockRejectedValueOnce(conditionalFailure()); + + expect(await claimTerminalReply(ddb, TABLE, TASK, STAMP)).toEqual({ won: false }); + // Losing is the expected outcome for every redelivery, so it must not warn β€” + // otherwise the logs cry wolf on healthy runs. + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('a claim write that ERRORS does not win either β€” replying could duplicate', async () => { + // The write may or may not have landed, so the safe read is "not mine". + send.mockRejectedValueOnce(new Error('ProvisionedThroughputExceeded')); + + expect(await claimTerminalReply(ddb, TABLE, TASK, STAMP)).toEqual({ won: false }); + expect(warnSpy).toHaveBeenCalled(); + }); +}); + +describe('releaseReplyClaim', () => { + test('frees the claim and counts the attempt in ONE write', async () => { + send.mockResolvedValueOnce({ Attributes: { ack_reply_attempts: 1 } }); + + expect(await releaseReplyClaim(ddb, TABLE, TASK, STAMP)).toBe('released'); + + const input = inputOf(0); + expect(input.UpdateExpression).toContain('REMOVE ack_replied_at'); + // Counting in the same write is what keeps the budget honest: a separate + // increment could be lost between the two and restore the unbounded spin. + expect(input.UpdateExpression).toContain('ack_reply_attempts'); + // A blind REMOVE would, when this release is delayed past another delivery's + // successful claim-and-reply, strip that writer's claim and let a third + // delivery reply again β€” one lost reply becoming a duplicated one. + expect(input.ConditionExpression).toContain('ack_replied_at = :ours'); + // And the budget is enforced by the condition, not by a later read. + expect(input.ConditionExpression).toContain('ack_reply_attempts < :max'); + expect((input.ExpressionAttributeValues as Record)[':max']).toBe(MAX_REPLY_ATTEMPTS); + }); + + test('a spent budget reports EXHAUSTED, so the caller settles instead of waiting', async () => { + // The live failure this bound prevents: releasing the claim writes to the task + // record, and the reconciler consumes that record's stream β€” so a release + // re-wakes the handler that performed it. With a reply that can never succeed + // (its comment was deleted) that spun ~900 times in six minutes. + send + .mockRejectedValueOnce(conditionalFailure()) + .mockResolvedValueOnce({ Item: { ack_reply_attempts: MAX_REPLY_ATTEMPTS } }); + + expect(await releaseReplyClaim(ddb, TABLE, TASK, STAMP)).toBe('exhausted'); + }); + + test('a claim that is no longer OURS is distinguished from a spent budget', async () => { + // Both surface as a failed condition but need opposite handling: another + // delivery owning the reply means nothing is stuck, so the caller must NOT + // settle over it. + send + .mockRejectedValueOnce(conditionalFailure()) + .mockResolvedValueOnce({ Item: { ack_reply_attempts: 1 } }); + + expect(await releaseReplyClaim(ddb, TABLE, TASK, STAMP)).toBe('not_ours'); + }); + + test('the budget check reads strongly-consistent β€” the increment is seconds old', async () => { + send + .mockRejectedValueOnce(conditionalFailure()) + .mockResolvedValueOnce({ Item: { ack_reply_attempts: 1 } }); + + await releaseReplyClaim(ddb, TABLE, TASK, STAMP); + expect(inputOf(1)).toMatchObject({ ProjectionExpression: 'ack_reply_attempts', ConsistentRead: true }); + }); + + test('an unreadable budget counts as exhausted rather than looping', async () => { + send + .mockRejectedValueOnce(conditionalFailure()) + .mockRejectedValueOnce(new Error('throttled')); + + expect(await releaseReplyClaim(ddb, TABLE, TASK, STAMP)).toBe('exhausted'); + }); + + test('an infra failure reports exhausted β€” the claim is still held', async () => { + // AccessDenied or a throttle leaves the claim in place, so promising a retry + // would leave the request looking unanswered indefinitely. + send.mockRejectedValueOnce(new Error('AccessDeniedException')); + + expect(await releaseReplyClaim(ddb, TABLE, TASK, STAMP)).toBe('exhausted'); + expect(warnFields()).toMatchObject({ event: 'iteration_reply.claim_release_failed' }); + }); + + test('a successful release is announced with the attempt number', async () => { + send.mockResolvedValueOnce({ Attributes: { ack_reply_attempts: 2 } }); + await releaseReplyClaim(ddb, TABLE, TASK, STAMP); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining('Released'), + expect.objectContaining({ task_id: TASK, attempt: 2, max_attempts: MAX_REPLY_ATTEMPTS }), + ); + }); + + test('never throws β€” the caller is on a best-effort feedback path', async () => { + send.mockRejectedValueOnce(new Error('boom')); + await expect(releaseReplyClaim(ddb, TABLE, TASK, STAMP)).resolves.toBe('exhausted'); + }); +}); + +describe('terminalReplyClaimed', () => { + test('reads strongly-consistent, because the write may be seconds old', async () => { + send.mockResolvedValueOnce({ Item: { ack_replied_at: STAMP } }); + + expect(await terminalReplyClaimed(ddb, TABLE, TASK)).toBe(true); + expect(inputOf(0)).toMatchObject({ + Key: { task_id: TASK }, + ProjectionExpression: 'ack_replied_at', + // An eventually-consistent read is exactly how a stale progress edit slips + // past a settle that has already happened. + ConsistentRead: true, + }); + }); + + test('an unclaimed or missing task reads as not settled', async () => { + send.mockResolvedValueOnce({ Item: {} }); + expect(await terminalReplyClaimed(ddb, TABLE, TASK)).toBe(false); + + send.mockResolvedValueOnce({}); + expect(await terminalReplyClaimed(ddb, TABLE, TASK)).toBe(false); + }); + + test('fails OPEN on a read error β€” a missed progress edit beats a frozen reply', async () => { + send.mockRejectedValueOnce(new Error('throttled')); + expect(await terminalReplyClaimed(ddb, TABLE, TASK)).toBe(false); + }); +}); diff --git a/cdk/test/handlers/shared/clarify-resume.test.ts b/cdk/test/handlers/shared/clarify-resume.test.ts new file mode 100644 index 000000000..1243fa03f --- /dev/null +++ b/cdk/test/handlers/shared/clarify-resume.test.ts @@ -0,0 +1,106 @@ +/** + * 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 { + buildClarifyResumeDescription, + isClarifyHold, + type ClarifyHoldRow, +} from '../../../src/handlers/shared/clarify-resume'; + +/** A canonical clarify-HOLD row: new-task-v1 that paused with a question. */ +function hold(overrides: Partial = {}): ClarifyHoldRow { + return { + resolved_workflow: { id: 'coding/new-task-v1' }, + code_changed: false, + answer_text: 'Which environment should this deploy to β€” staging or prod?', + task_description: 'Wire up the deploy button.', + ...overrides, + }; +} + +describe('isClarifyHold', () => { + test('recognises the canonical clarify-hold (new-task-v1, no code, question, no PR)', () => { + expect(isClarifyHold(hold())).toBe(true); + }); + + test('accepts a raw workflow_ref (versioned or bare) when the pin is absent', () => { + expect(isClarifyHold(hold({ resolved_workflow: null, workflow_ref: 'coding/new-task-v1' }))).toBe(true); + expect(isClarifyHold(hold({ resolved_workflow: null, workflow_ref: 'coding/new-task-v1@3' }))).toBe(true); + }); + + test('rejects a running task (code_changed unset until terminal)', () => { + expect(isClarifyHold(hold({ code_changed: undefined }))).toBe(false); + }); + + test('rejects a task that actually shipped code (code_changed=true)', () => { + expect(isClarifyHold(hold({ code_changed: true }))).toBe(false); + }); + + test('rejects a no-op PR ITERATION (has a PR β€” the reconciler/fanout owns that reply)', () => { + // A pr-iteration-v1 that answered without changing code shares + // code_changed=false + answer_text, but it has a PR and a different + // workflow β€” must NOT be treated as a resumable clarify-hold. + expect(isClarifyHold(hold({ + resolved_workflow: { id: 'coding/pr-iteration-v1' }, + pr_url: 'https://github.com/o/r/pull/7', + pr_number: 7, + }))).toBe(false); + // Even a new-task-v1 row with a PR is not a hold (it shipped something). + expect(isClarifyHold(hold({ pr_url: 'https://github.com/o/r/pull/9' }))).toBe(false); + expect(isClarifyHold(hold({ pr_number: 9 }))).toBe(false); + }); + + test('rejects a plain failure / completion with no question text', () => { + expect(isClarifyHold(hold({ answer_text: undefined }))).toBe(false); + expect(isClarifyHold(hold({ answer_text: ' ' }))).toBe(false); + }); + + test('rejects null / undefined rows', () => { + expect(isClarifyHold(null)).toBe(false); + expect(isClarifyHold(undefined)).toBe(false); + }); +}); + +describe('buildClarifyResumeDescription', () => { + test('carries the original ask, the question, and the answer in order', () => { + const md = buildClarifyResumeDescription( + 'Wire up the deploy button.', + 'Which environment β€” staging or prod?', + 'staging', + ); + // Original intent first so the agent reads "do the original thing, now resolved". + expect(md.indexOf('Wire up the deploy button.')).toBeLessThan(md.indexOf('You asked:')); + expect(md).toContain('You asked: Which environment β€” staging or prod?'); + expect(md).toContain('The reviewer answered: staging'); + expect(md).toMatch(/proceed with the original request/i); + }); + + test('degrades to just the exchange when the original description is blank', () => { + const md = buildClarifyResumeDescription(undefined, 'Q?', 'A'); + expect(md).toContain('You asked: Q?'); + expect(md).toContain('The reviewer answered: A'); + }); + + test('still includes the answer when the held question is missing', () => { + const md = buildClarifyResumeDescription('Do the thing.', undefined, 'yes, all of it'); + expect(md).toContain('Do the thing.'); + expect(md).not.toContain('You asked:'); + expect(md).toContain('The reviewer answered: yes, all of it'); + }); +}); diff --git a/cdk/test/handlers/shared/failure-reply.test.ts b/cdk/test/handlers/shared/failure-reply.test.ts new file mode 100644 index 000000000..cdfcd011b --- /dev/null +++ b/cdk/test/handlers/shared/failure-reply.test.ts @@ -0,0 +1,227 @@ +/** + * 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 { TaskStatus } from '../../../src/constructs/task-status'; +import { renderFailureReply, renderPanelFailureReason } from '../../../src/handlers/shared/failure-reply'; + +describe('renderFailureReply β€” failure is a conversation', () => { + describe('build/test failure β€” the real gating shape, as verified live', () => { + // A build/test regression persists as + // status=FAILED, build_passed=null, error_message="Task did not succeed + // (agent_status='success', build_ok=False)". The agent finished fine; only + // the build gate failed. (The previous COMPLETED+build_passed===false + // assumption NEVER occurs live β€” that bug shipped to dev and was only caught + // by deliberately forcing a regression.) + const body = renderFailureReply({ + status: TaskStatus.FAILED, + buildPassed: null, + errorMessage: "Task did not succeed (agent_status='success', build_ok=False)", + taskId: 't1', + }); + + // The agent runs the configured build INSIDE the + // microVM, so the failing output is in CloudWatch β€” NOT the PR's GitHub + // checks (the repo may have no CI). The old "see the PR's checks" copy + // pointed the user at an empty surface. Point at the build log by task id. + test('points at the CloudWatch build log by task id, not the PR checks', () => { + expect(body).toMatch(/^❌/); + expect(body).toMatch(/build\/tests didn't pass/i); + expect(body).toMatch(/build log in CloudWatch for task `t1`/); + expect(body).not.toMatch(/PR's checks/i); + }); + + test('invites a reply (the retry seam)', () => { + expect(body).toMatch(/reply with guidance/i); + }); + + test('also matches the end_turn variant of the gating message', () => { + const b = renderFailureReply({ + status: TaskStatus.FAILED, + errorMessage: "Task did not succeed (agent_status='end_turn', build_ok=False)", + taskId: 't1b', + }); + expect(b).toMatch(/build\/tests didn't pass/i); + expect(b).toMatch(/build log in CloudWatch for task `t1b`/); + }); + + test('defensive: explicit build_passed=false with no error_message still reads as build failure', () => { + const b = renderFailureReply({ status: TaskStatus.FAILED, buildPassed: false, taskId: 't1c' }); + expect(b).toMatch(/build\/tests didn't pass/i); + expect(b).toMatch(/build log in CloudWatch for task `t1c`/); + }); + }); + + describe('build TIMEOUT β€” distinct from a red build', () => { + // The agent finished cleanly but the build verify exceeded its wall-clock + // limit and was killed β†’ error_message carries build_ok=timeout. This is a + // different diagnosis (slow build / cap too low), NOT "your code is broken". + const body = renderFailureReply({ + status: TaskStatus.FAILED, + errorMessage: "Task did not succeed (agent_status='success', build_ok=timeout)", + taskId: 't-to', + }); + + test('reads as "timed out / didn\'t finish in time", not "didn\'t pass"', () => { + expect(body).toMatch(/^❌/); + expect(body).toMatch(/didn't finish in time|timed out/i); + expect(body).not.toMatch(/didn't pass/i); + expect(body).toMatch(/build log in CloudWatch for task `t-to`/); + }); + + test('still invites a reply (retry seam)', () => { + expect(body).toMatch(/reply with guidance/i); + }); + + test('a timeout is NOT misread as an agent crash (no classified-title path)', () => { + // It must take the build branch, not the classifyError/CloudWatch-crash branch. + expect(body).not.toMatch(/Unexpected error|didn't complete/i); + }); + }); + + describe('agent-itself failure (crash / cap / timeout before a clean terminal)', () => { + test('max-turns crash β†’ classified title + CloudWatch task id + retry invite', () => { + const body = renderFailureReply({ + status: TaskStatus.FAILED, + errorMessage: 'Task did not succeed: agent_status="error_max_turns"', + taskId: 'task-xyz', + }); + expect(body).toMatch(/^❌/); + expect(body).toMatch(/Exceeded max turns/i); // classified title + expect(body).toMatch(/CloudWatch for task `task-xyz`/); + // TIMEOUT + retryable β†’ a plain reply-to-retry next step. + expect(body).toMatch(/reply here with any extra guidance/i); + }); + + test('infra/compute failure β†’ "temporary, retry, else contact admin" next step (not a generic guidance ask)', () => { + // A coding child hit a transient compute failure. The user's question was + // "do I retry, or tell my admin?" β€” the reply must answer it. + const body = renderFailureReply({ + status: TaskStatus.FAILED, + errorMessage: 'Session start failed: InvalidParameterException: TaskDefinition is inactive', + taskId: 'task-infra', + }); + expect(body).toMatch(/temporary infrastructure issue|mid-update|compute environment/i); + expect(body).toMatch(/reply here to try again/i); + expect(body).toMatch(/contact your ABCA admin/i); + // It must NOT ask for "guidance" β€” the user's input is irrelevant to an infra retry. + expect(body).not.toMatch(/extra guidance/i); + }); + + test('not-retryable auth failure β†’ says a retry won\'t help + names the admin', () => { + const body = renderFailureReply({ + status: TaskStatus.FAILED, + errorMessage: 'INSUFFICIENT_GITHUB_REPO_PERMISSIONS', + taskId: 'task-auth', + }); + expect(body).toMatch(/won'?t fix this|won'?t help/i); + expect(body).toMatch(/admin/i); + }); + + test('truncates a long raw error to an excerpt with an ellipsis', () => { + const longErr = 'boom '.repeat(200); // 1000 chars + const body = renderFailureReply({ status: TaskStatus.FAILED, errorMessage: longErr, taskId: 't2' }); + expect(body).toContain('…'); + // The reply stays compact β€” nowhere near the 1000-char raw error. + expect(body.length).toBeLessThan(400); + }); + + test('unclassifiable error β†’ generic fallback title, still points at CloudWatch', () => { + const body = renderFailureReply({ status: TaskStatus.FAILED, errorMessage: 'weird thing', taskId: 't3' }); + // UNKNOWN_CLASSIFICATION title is "Unexpected error". + expect(body).toMatch(/Unexpected error/i); + expect(body).toMatch(/CloudWatch for task `t3`/); + }); + + test('no error_message at all β†’ still a coherent agent-failure reply', () => { + const body = renderFailureReply({ status: TaskStatus.FAILED, taskId: 't4' }); + expect(body).toMatch(/^❌/); + expect(body).toMatch(/CloudWatch for task `t4`/); + }); + + test('a genuine agent crash (agent_status=error_*) reads as agent failure, NOT build', () => { + // An agent crash mid-execution β€” distinct from the build-gate-failed + // shape (which carries agent_status='success'). Must get the CloudWatch + // pointer, not the softer "PR's checks" build copy. + const body = renderFailureReply({ + status: TaskStatus.FAILED, + errorMessage: 'Task did not succeed (agent_status=\'error_during_execution\', build_ok=False)', + taskId: 't5', + }); + expect(body).toMatch(/CloudWatch for task `t5`/); + expect(body).not.toMatch(/PR's checks/i); + }); + }); +}); + +describe('renderPanelFailureReason β€” the failed-node sub-line on the epic panel', () => { + test('integration-node build failure names the combined merge + points at CloudWatch', () => { + const reason = renderPanelFailureReason({ + errorMessage: "Task did not succeed (agent_status='success', build_ok=False)", + taskId: 't-int', + isIntegration: true, + }); + expect(reason).toMatch(/combined build failed after merging the sub-issue branches/i); + expect(reason).toMatch(/build log in CloudWatch for task `t-int`/); + // No raw build output leaks into the panel. + expect(reason).not.toMatch(/build_ok/i); + }); + + test('a regular sub-issue build failure reads generically (no merge wording)', () => { + const reason = renderPanelFailureReason({ buildPassed: false, taskId: 't-leaf' }); + expect(reason).toMatch(/^Build\/tests failed/); + expect(reason).not.toMatch(/merging/i); + expect(reason).toMatch(/CloudWatch for task `t-leaf`/); + }); + + test('an agent crash surfaces the classified title + CloudWatch pointer', () => { + const reason = renderPanelFailureReason({ + errorMessage: 'Task did not succeed: agent_status="error_max_turns"', + taskId: 't-crash', + isIntegration: true, + }); + expect(reason).toMatch(/Exceeded max turns/i); + expect(reason).toMatch(/CloudWatch for task `t-crash`/); + // An agent crash is NOT a build failure β€” no combined-build wording. + expect(reason).not.toMatch(/combined build/i); + }); + + test('null when there is no task id to point at (nothing actionable to render)', () => { + expect(renderPanelFailureReason({ buildPassed: false })).toBeNull(); + }); + + test('a build TIMEOUT on the integration node reads "timed out", not "failed"', () => { + const reason = renderPanelFailureReason({ + errorMessage: "Task did not succeed (agent_status='success', build_ok=timeout)", + taskId: 't-int-to', + isIntegration: true, + }); + expect(reason).toMatch(/Combined build timed out after merging the sub-issue branches/i); + expect(reason).not.toMatch(/failed/i); + expect(reason).toMatch(/CloudWatch for task `t-int-to`/); + }); + + test('a build TIMEOUT on a regular leaf reads "Build/tests timed out"', () => { + const reason = renderPanelFailureReason({ + errorMessage: "Task did not succeed (agent_status='end_turn', build_ok=timeout)", + taskId: 't-leaf-to', + }); + expect(reason).toMatch(/^Build\/tests timed out/); + expect(reason).not.toMatch(/failed/i); + }); +}); diff --git a/cdk/test/handlers/shared/iteration-heartbeat.test.ts b/cdk/test/handlers/shared/iteration-heartbeat.test.ts new file mode 100644 index 000000000..45d22871a --- /dev/null +++ b/cdk/test/handlers/shared/iteration-heartbeat.test.ts @@ -0,0 +1,115 @@ +/** + * 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 { planHeartbeat, type HeartbeatTaskView } from '../../../src/handlers/shared/iteration-heartbeat'; + +const NOW = Date.parse('2026-06-29T13:30:00Z'); + +function task(overrides: Partial = {}): HeartbeatTaskView { + return { + taskId: 't-1', + status: 'RUNNING', + createdAt: '2026-06-29T13:20:00Z', // 10 min before NOW + channelSource: 'linear', + linearWorkspaceId: 'ws-1', + iterationReplyCommentId: 'reply-1', + triggerCommentId: 'cmt-1', + triggerCommentIssueId: 'issue-1', + isIteration: true, + prNumber: 42, + ...overrides, + }; +} + +describe('planHeartbeat β€” eligibility', () => { + test('a long-running linear iteration with a reply β†’ a plan', () => { + const plan = planHeartbeat(task(), NOW); + expect(plan).not.toBeNull(); + expect(plan!.taskId).toBe('t-1'); + expect(plan!.replyId).toBe('reply-1'); + expect(plan!.parentCommentId).toBe('cmt-1'); + expect(plan!.issueId).toBe('issue-1'); + expect(plan!.elapsedS).toBe(600); + expect(plan!.body).toContain('πŸ”„ Working β€” updating PR #42…'); + expect(plan!.body).toContain('10m elapsed'); + }); + + test('not RUNNING β†’ no plan', () => { + expect(planHeartbeat(task({ status: 'COMPLETED' }), NOW)).toBeNull(); + expect(planHeartbeat(task({ status: 'HYDRATING' }), NOW)).toBeNull(); + }); + + test('a STANDALONE iteration (no orchestration marker) is STILL eligible β€” keys on the reply, not isIteration', () => { + // The black-box case observed in practice was a standalone @bgagent + // iteration, which omits orchestration_iteration but still has a maturing + // reply. It must get a heartbeat: eligibility is the reply-routing fields, + // not isIteration. + // (The first deploy returned eligible:0 for exactly this case + // because the standalone path stamps linear_issue_id, not + // trigger_comment_issue_id β€” the sweep's toView now falls back to it, so by + // the time we reach planHeartbeat the issue id is populated either way.) + const plan = planHeartbeat(task({ isIteration: false }), NOW); + expect(plan).not.toBeNull(); + expect(plan!.replyId).toBe('reply-1'); + expect(plan!.issueId).toBe('issue-1'); + }); + + test('a task with NO maturing reply (first run / non-PR) β†’ no plan', () => { + expect(planHeartbeat(task({ iterationReplyCommentId: undefined }), NOW)).toBeNull(); + }); + + test('non-linear channel β†’ no plan (reply edit only wired for linear)', () => { + expect(planHeartbeat(task({ channelSource: 'slack' }), NOW)).toBeNull(); + }); + + test('below the elapsed floor β†’ no plan (fresh task, no nudge)', () => { + // 30s elapsed < 90s floor + expect(planHeartbeat(task({ createdAt: '2026-06-29T13:29:30Z' }), NOW)).toBeNull(); + }); + + test('missing any reply-routing field β†’ no plan (cannot edit)', () => { + expect(planHeartbeat(task({ iterationReplyCommentId: undefined }), NOW)).toBeNull(); + expect(planHeartbeat(task({ triggerCommentId: undefined }), NOW)).toBeNull(); + expect(planHeartbeat(task({ triggerCommentIssueId: undefined }), NOW)).toBeNull(); + expect(planHeartbeat(task({ linearWorkspaceId: undefined }), NOW)).toBeNull(); + }); + + test('unparseable / missing created_at β†’ no plan', () => { + expect(planHeartbeat(task({ createdAt: undefined }), NOW)).toBeNull(); + expect(planHeartbeat(task({ createdAt: 'not-a-date' }), NOW)).toBeNull(); + }); +}); + +describe('planHeartbeat β€” body content', () => { + test('a progress note is folded into the working line', () => { + const plan = planHeartbeat(task({ latestProgressNote: 'running build verification' }), NOW); + expect(plan!.body).toContain('10m elapsed Β· running build verification'); + }); + + test('no PR number β†’ generic working line still carries elapsed', () => { + const plan = planHeartbeat(task({ prNumber: null }), NOW); + expect(plan!.body).toContain('πŸ”„ Working…'); + expect(plan!.body).toContain('10m elapsed'); + }); + + test('a PR url makes the reference clickable', () => { + const plan = planHeartbeat(task({ prUrl: 'https://gh/pull/42' }), NOW); + expect(plan!.body).toContain('[PR #42](https://gh/pull/42)'); + }); +}); diff --git a/cdk/test/handlers/shared/trigger-label.test.ts b/cdk/test/handlers/shared/trigger-label.test.ts new file mode 100644 index 000000000..d7f3af911 --- /dev/null +++ b/cdk/test/handlers/shared/trigger-label.test.ts @@ -0,0 +1,52 @@ +/** + * 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 { hasHelpLabel, DEFAULT_LABEL_FILTER } from '../../../src/handlers/shared/trigger-label'; + +describe('DEFAULT_LABEL_FILTER', () => { + test('is the documented default a project inherits without a label_filter', () => { + // Load-bearing: the project-mapping table treats an absent label_filter as + // this value, and the rollup renders it in operator-facing copy. Changing it + // silently stops every project that never set one explicitly. + expect(DEFAULT_LABEL_FILTER).toBe('bgagent'); + }); +}); + +describe('hasHelpLabel', () => { + test('detects the base:help label, case-insensitive', () => { + expect(hasHelpLabel(['bgagent:help'])).toBe(true); + expect(hasHelpLabel(['BGAgent:Help'])).toBe(true); + expect(hasHelpLabel(['something', 'bgagent:help', 'other'])).toBe(true); + }); + + test('respects a custom label filter', () => { + expect(hasHelpLabel(['ship:help'], 'ship')).toBe(true); + expect(hasHelpLabel(['bgagent:help'], 'ship')).toBe(false); + }); + + test('is false for the trigger label and for lookalikes', () => { + // The help label must not fire on the plain trigger (that would explain the + // labels instead of doing the work), nor on a name that merely contains + // "help". + expect(hasHelpLabel(['bgagent'])).toBe(false); + expect(hasHelpLabel(['helpful', 'bghelp'])).toBe(false); + expect(hasHelpLabel([])).toBe(false); + expect(hasHelpLabel([undefined, null])).toBe(false); + }); +});