Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
540 changes: 411 additions & 129 deletions cdk/src/constructs/ecs-agent-cluster.ts

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions cdk/src/constructs/task-orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ export interface TaskOrchestratorProps {
readonly containerName: string;
readonly taskRoleArn: string;
readonly executionRoleArn: string;
/**
* The smaller read-only PLANNING task def (see
* docs/design/ECS_RIGHTSIZED_PLANNING.md). The ECS strategy selects it for
* read-only workflows so planning doesn't run on the larger build box.
*
* Required, like its siblings, so the all-or-nothing constraint stays visible
* at the type level: the strategy reads this ARN from an env var, and an
* ecsConfig that omitted it would compile but leave the planning def defined
* and permanently unreachable.
*
* Needs no extra IAM — it shares the build def's task and execution roles,
* and the `ecs:RunTask` grant below is scoped by `ecs:cluster` rather than by
* task-definition ARN, so it already covers every def in the cluster.
*/
readonly planningTaskDefinitionArn: string;
};

/**
Expand Down Expand Up @@ -272,6 +287,11 @@ export class TaskOrchestrator extends Construct {
ECS_SUBNETS: props.ecsConfig.subnets,
ECS_SECURITY_GROUP: props.ecsConfig.securityGroup,
ECS_CONTAINER_NAME: props.ecsConfig.containerName,
// Read-only workflows route here instead of the build def. Without this
// var the strategy's `readOnly && ECS_PLANNING_TASK_DEFINITION_ARN`
// guard is always falsy, so the planning def would be synthesized and
// never used.
ECS_PLANNING_TASK_DEFINITION_ARN: props.ecsConfig.planningTaskDefinitionArn,
}),
// #502: bucket the orchestrator writes the ECS payload to (and deletes
// from at finalize); the ECS strategy reads this to build the S3 URI.
Expand Down
47 changes: 24 additions & 23 deletions cdk/src/handlers/orchestrate-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
return loadTask(taskId);
});

// Correlation envelope (#245): stamp {task_id, user_id, repo} on every
// lifecycle log line so admission→terminal transitions join to TaskEvents
// and the agent trace without hand-adding fields per call. `repo` is
// undefined for repo-less workflows (#248 Phase 3).
// Correlation envelope: stamp {task_id, user_id, repo} on every lifecycle log
// line so admission→terminal transitions join to TaskEvents and the agent
// trace without hand-adding fields per call. `repo` is undefined for repo-less
// workflows (those with no target repository).
const { log, correlation } = envelopeFor(task);

// Step 1b: Load blueprint config (per-repo overrides)
Expand Down Expand Up @@ -169,13 +169,16 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
userId: task.user_id,
payload,
blueprintConfig,
// A read-only workflow (e.g. planning that only clones and reads the
// repo) runs on the smaller ECS planning task def. Ignored by AgentCore.
readOnly: workflowIsReadOnly(task.resolved_workflow?.id ?? 'coding/new-task-v1'),
};
// Transient-error AUTO-RETRY (once), extracted to startSessionWithRetry so
// the four branches are unit-tested (#599 B2). The retry-event emit is
// best-effort and guarded there (#599 B1): a TaskEvents PutItem fault can't
// abort or mis-attribute the retry. The correlation envelope is threaded so
// the session_start_retry event carries the same trace context as
// session_started below (#245).
// its branches are unit-tested. The retry-event emit is best-effort and
// guarded there: a TaskEvents PutItem fault can't abort or mis-attribute
// the retry. The correlation envelope is threaded so the
// session_start_retry event carries the same trace context as
// session_started below.
const { handle, autoRetried: retried } = await startSessionWithRetry(
strategy,
startInput,
Expand Down Expand Up @@ -213,16 +216,14 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
return handle;
} catch (err) {
// Carry the auto-retry fact into error_message: the `[auto-retried]` suffix
// is persisted verbatim (the classifier ignores it — it does not affect
// classification). It is a breadcrumb for a FORTHCOMING failure renderer to
// detect and surface "I already tried again" to the channel — no consumer
// renders it yet on this branch (retryGuidance() in error-classifier.ts is
// the intended copy source; it ships ahead of its consumer).
// is persisted verbatim (the error classifier ignores it — it does not
// affect classification). It is a breadcrumb for a failure renderer to
// detect and surface "I already tried again" to the channel.
//
// Stamp on BOTH paths a retry ran (#599 N1): branch 3 sets `autoRetried`
// above then a LATER step throws; branch 4 (transient-then-transient) throws
// FROM startSessionWithRetry before `autoRetried` is assigned, so the local
// is still false — read the fact off the thrown error via isAutoRetried().
// Stamp on BOTH paths where a retry ran: one path sets `autoRetried` above
// then a LATER step throws; the transient-then-transient path throws FROM
// startSessionWithRetry before `autoRetried` is assigned, so the local is
// still false — read the fact off the thrown error via isAutoRetried().
// Without this, a double-transient failure was told "reply to retry" instead
// of "I already retried" — the exact confusion the marker exists to prevent.
const retriedNote = (autoRetried || isAutoRetried(err)) ? ' [auto-retried]' : '';
Expand Down Expand Up @@ -330,11 +331,11 @@ const durableHandler: DurableExecutionHandler<OrchestrateTaskEvent, void> = asyn
// Step 6: Finalize — update terminal status, emit events, release concurrency
await context.step('finalize', async () => {
await finalizeTask(taskId, finalPollState, task.user_id);
// #502: the task is terminal — the container has long since read its
// payload, so delete the ephemeral S3 payload object now. Best-effort
// (deleteEcsPayload swallows errors) and a no-op for AgentCore tasks /
// deployments without a payload bucket; the bucket's 1-day lifecycle rule
// is the backstop if this delete or the whole step never runs.
// The task is terminal — the container has long since read its payload, so
// delete the ephemeral S3 payload object now. Best-effort (deleteEcsPayload
// swallows errors) and a no-op for AgentCore tasks / deployments without a
// payload bucket; the bucket's 1-day lifecycle rule is the backstop if this
// delete or the whole step never runs.
if (blueprintConfig.compute_type === 'ecs') {
await deleteEcsPayload(taskId);
}
Expand Down
9 changes: 9 additions & 0 deletions cdk/src/handlers/shared/compute-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@ export interface ComputeStrategy {
userId: string;
payload: Record<string, unknown>;
blueprintConfig: BlueprintConfig;
/**
* #299 ECS_RIGHTSIZED_PLANNING: true for a read-only workflow (e.g.
* coding/decompose-v1) that clones + reads + emits an artifact but never
* builds. The ECS strategy uses it to pick the smaller planning task def
* instead of the larger build def. AgentCore ignores it (its microVM is a
* single fixed size). Optional so callers/tests that omit it default to the
* build def (never worse than today).
*/
readOnly?: boolean;
}): Promise<SessionHandle>;
pollSession(handle: SessionHandle): Promise<SessionStatus>;
stopSession(handle: SessionHandle): Promise<void>;
Expand Down
Loading
Loading