diff --git a/agent/Dockerfile b/agent/Dockerfile index 82c5f5e1..ed017639 100644 --- a/agent/Dockerfile +++ b/agent/Dockerfile @@ -44,6 +44,15 @@ RUN curl -fsSL https://deb.nodesource.com/setup_24.x | bash - && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* /var/cache/apt/archives/* +# Enable Corepack so ``yarn`` / ``pnpm`` resolve out of the box. Many target +# repos (including ABCA itself) drive installs with ``yarn install`` via their +# build command; without this, ``yarn`` is "command not found" (exit 127) and +# the build-verification GATE runs inert — a green build is reported for a repo +# we never actually built (live-caught 2026-06-29 dogfooding ABCA-on-ABCA: the +# agent had to hand-build a ``~/bin/yarn`` shim every run). Corepack ships with +# Node 24; ``enable`` installs the yarn/pnpm shims onto PATH. +RUN corepack enable && corepack prepare yarn@stable --activate || corepack enable + # Install Claude Code CLI (the Python SDK requires this binary) # Then update known vulnerable transitive packages where fixed versions exist. # Pinned 2.1.191 to match the CLI bundled by claude-agent-sdk 0.2.110 (see diff --git a/agent/src/entrypoint.py b/agent/src/entrypoint.py index e2097694..53e14830 100644 --- a/agent/src/entrypoint.py +++ b/agent/src/entrypoint.py @@ -31,7 +31,7 @@ TaskResult, TokenUsage, ) -from pipeline import main, run_task # noqa: F401 +from pipeline import main, run_task, run_task_from_payload # noqa: F401 from post_hooks import ( # noqa: F401 ensure_committed, ensure_pr, diff --git a/agent/src/pipeline.py b/agent/src/pipeline.py index 980e2df0..ddab154d 100644 --- a/agent/src/pipeline.py +++ b/agent/src/pipeline.py @@ -4,6 +4,7 @@ import asyncio import hashlib +import inspect import os import subprocess import sys @@ -1272,6 +1273,112 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None: raise +#: Orchestrator payload keys that map to a differently-named ``run_task`` kwarg. +#: The orchestrator emits ``prompt``/``model_id``; ``run_task`` calls them +#: ``task_description``/``anthropic_model``. Everything else is a 1:1 name match. +_PAYLOAD_KEY_ALIASES = { + "prompt": "task_description", + "model_id": "anthropic_model", +} + +#: ``run_task`` kwargs that must be coerced to ``str`` — the orchestrator may +#: emit them as numbers (issue_number, pr_number) and ``run_task`` types them as +#: strings. ``max_turns`` is coerced to int. Absent keys are left to the +#: ``run_task`` defaults. +_PAYLOAD_STR_KEYS = frozenset({"issue_number", "pr_number"}) + +#: Parameter names ``run_task`` accepts — computed once at import from the REAL +#: signature (not inside the function, so patching ``run_task`` in tests can't +#: shadow it). Any payload key not in this set is ignored, never passed through. +_RUN_TASK_PARAMS = frozenset(inspect.signature(run_task).parameters) + +#: Orchestrator payload keys we KNOW about that ``run_task`` does not (yet) +#: accept as a parameter. Dropping one of these is expected today (e.g. +#: ``base_branch`` is consumed via ``hydrated_context``, not a run_task kwarg; +#: ``github_token_secret_arn`` is resolved before this call), but a key that +#: shows up here AND is silently dropped is exactly the "wired one side of an +#: orchestrator→agent field, forgot the other" no-op that ABCA-487 was — so we +#: WARN when we drop one, making a future contract gap visible instead of silent. +#: Keys not in this set (genuinely foreign) are dropped quietly as before. +_KNOWN_ORCHESTRATOR_KEYS = frozenset( + { + "build_command", + # ``lint_command``'s sibling: neither is a run_task param today (the build/ + # lint commands are consumed via repo config, not passed through here), but + # listing both makes a future "wired build_command, forgot lint_command" + # contract gap WARN instead of drop silently (N4). + "lint_command", + "merge_branches", + "base_branch", + "github_token_secret_arn", + # AgentCore's server.py exports task_started_at as TASK_STARTED_AT, which + # hooks._remaining_maxlifetime_s() uses to clip the Cedar HITL approval-gate + # maxLifetime. The ECS boot path bypasses server.py and does not (yet) set + # that env, so this key is dropped here — a silent AgentCore↔ECS HITL + # divergence (fail-open: the clip returns None, gate uses the task default). + # Listing it makes the drop WARN so the parity gap is visible until the ECS + # strategy sets TASK_STARTED_AT in containerEnv (tracked as a follow-up). + "task_started_at", + } +) + + +def run_task_from_payload(payload: dict) -> dict: + """Invoke :func:`run_task` from a full orchestrator payload dict. + + The ECS compute path (``ecs-strategy.ts``) hands the agent the *entire* + orchestrator payload (via the #502 S3 pointer). Previously the ECS boot + command hand-listed a subset of ``run_task`` kwargs and silently dropped the + rest — most visibly ``channel_source``/``channel_metadata`` (no Linear/Jira + reactions or channel MCP on ECS — ABCA-487), plus ``build_command``, + ``cedar_policies``, ``base_branch``/``merge_branches``, ``attachments``, etc. + + This maps the payload to ``run_task``'s real signature so no field can be + silently dropped again: rename the aliased keys, filter to parameters + ``run_task`` actually accepts (unknown keys are ignored, not passed as + ``**kwargs`` which ``run_task`` doesn't accept), and coerce the str/int + fields the orchestrator may emit as numbers. ``aws_region`` falls back to the + ``AWS_REGION`` env var when the payload omits it (the boot command used to + supply this explicitly). + + Single source of truth + unit-testable, replacing the untestable inline + Python string that already drifted once. + """ + kwargs: dict = {} + for key, value in (payload or {}).items(): + target = _PAYLOAD_KEY_ALIASES.get(key, key) + if target not in _RUN_TASK_PARAMS: + # Not a run_task parameter — ignore. A KNOWN orchestrator key being + # dropped is expected today but worth a breadcrumb: if run_task ever + # grows a matching param, this WARN is where a "forgot to wire it + # through" no-op surfaces (N4 / ABCA-487 class). Foreign keys are + # dropped quietly. + if key in _KNOWN_ORCHESTRATOR_KEYS and value is not None: + log( + "WARN", + f"run_task_from_payload: dropping known orchestrator key '{key}' " + f"(not a run_task parameter) — consumed elsewhere or not yet wired", + ) + continue + if value is None: + continue # let run_task's default apply + if target in _PAYLOAD_STR_KEYS: + value = str(value) + elif target == "max_turns": + # Defensive, matching how every other field is handled: a malformed + # max_turns must not crash the whole boot — drop it and let run_task's + # default apply (with a breadcrumb) rather than raise a ValueError. + try: + value = int(value) + except (TypeError, ValueError): + log("WARN", f"run_task_from_payload: ignoring non-integer max_turns {value!r}") + continue + kwargs[target] = value + + kwargs.setdefault("aws_region", os.environ.get("AWS_REGION", "")) + return run_task(**kwargs) + + def main(): config = get_config() diff --git a/agent/tests/test_run_task_from_payload.py b/agent/tests/test_run_task_from_payload.py new file mode 100644 index 00000000..d6dbabac --- /dev/null +++ b/agent/tests/test_run_task_from_payload.py @@ -0,0 +1,186 @@ +"""Unit tests for pipeline.run_task_from_payload — the ECS payload→run_task map. + +Regression cover for ABCA-487: the ECS boot command used to hand-list a subset +of run_task kwargs and silently dropped channel_source/channel_metadata (no +Linear/Jira reactions on ECS), build_command, cedar_policies, base_branch, etc. +run_task_from_payload maps the WHOLE payload so nothing is dropped again. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from pipeline import _RUN_TASK_PARAMS, run_task_from_payload + + +def _capture(payload: dict) -> dict: + """Run the mapper with run_task replaced by a capturing stub; return kwargs.""" + seen: dict = {} + + def fake_run_task(**kwargs): + seen.update(kwargs) + return {"status": "success"} + + with patch("pipeline.run_task", side_effect=fake_run_task): + run_task_from_payload(payload) + return seen + + +class TestRunTaskFromPayload: + def test_renames_prompt_and_model_id(self): + seen = _capture({"prompt": "do the thing", "model_id": "anthropic.claude-x"}) + assert seen["task_description"] == "do the thing" + assert seen["anthropic_model"] == "anthropic.claude-x" + # The original payload keys must NOT leak through as-is (run_task rejects them). + assert "prompt" not in seen + assert "model_id" not in seen + + def test_forwards_channel_fields_ABCA_487(self): + # THE regression: channel_source/channel_metadata must reach run_task so + # the Linear/Jira reaction + channel MCP fire on ECS. + cm = {"linear_issue_id": "iss-1", "linear_oauth_secret_arn": "arn:sm:...:lin"} + seen = _capture({"channel_source": "linear", "channel_metadata": cm}) + assert seen["channel_source"] == "linear" + assert seen["channel_metadata"] == cm + + def test_forwards_cedar_attachments_trace_user_fields(self): + # HITL guardrails (cedar_policies, approval_*), attachments, trace, and + # user_id are real run_task params here — they must reach run_task on ECS + # (the hand-listed boot command used to drop them). + seen = _capture( + { + "cedar_policies": ["p1", "p2"], + "attachments": [{"filename": "x.png"}], + "trace": True, + "user_id": "user-9", + } + ) + assert seen["cedar_policies"] == ["p1", "p2"] + assert seen["attachments"] == [{"filename": "x.png"}] + assert seen["trace"] is True + assert seen["user_id"] == "user-9" + + def test_drops_payload_keys_that_are_not_yet_run_task_params(self): + # build_command/lint_command (configurable verify, #1) and base_branch/ + # merge_branches (orchestration stacking, #247) are emitted by the + # orchestrator but are NOT run_task parameters on this branch. The mapper + # filters against run_task's REAL signature, so they are dropped rather + # than smuggled through as an invalid kwarg. When those params land, + # they forward automatically with no change here — that's the point of + # keying off inspect.signature instead of a hand-list. + seen = _capture( + { + "repo_url": "org/repo", + "build_command": "npm ci && npm test", + "lint_command": "npm run lint", + "base_branch": "epic-tip", + "merge_branches": ["a", "b"], + } + ) + assert seen["repo_url"] == "org/repo" + for not_yet in ("build_command", "lint_command", "base_branch", "merge_branches"): + assert not_yet not in seen + + def test_coerces_issue_and_pr_number_to_str_and_max_turns_to_int(self): + seen = _capture({"issue_number": 42, "pr_number": 7, "max_turns": "50"}) + assert seen["issue_number"] == "42" + assert seen["pr_number"] == "7" + assert seen["max_turns"] == 50 + assert isinstance(seen["max_turns"], int) + + def test_ignores_unknown_payload_keys(self): + # github_token_secret_arn is on the payload but is NOT a run_task param + # (it's consumed platform-side); passing it as **kwargs would TypeError. + seen = _capture( + { + "repo_url": "org/repo", + "github_token_secret_arn": "arn:...", + "sources": ["x"], + } + ) + assert seen["repo_url"] == "org/repo" + assert "github_token_secret_arn" not in seen + assert "sources" not in seen + + def test_drops_none_values_so_run_task_defaults_apply(self): + seen = _capture({"repo_url": "org/repo", "base_branch": None, "channel_metadata": None}) + assert "base_branch" not in seen + assert "channel_metadata" not in seen + + def test_aws_region_falls_back_to_env(self, monkeypatch): + monkeypatch.setenv("AWS_REGION", "us-east-1") + seen = _capture({"repo_url": "org/repo"}) + assert seen["aws_region"] == "us-east-1" + + def test_explicit_aws_region_in_payload_wins(self, monkeypatch): + monkeypatch.setenv("AWS_REGION", "us-east-1") + seen = _capture({"repo_url": "org/repo", "aws_region": "eu-west-1"}) + assert seen["aws_region"] == "eu-west-1" + + def test_every_forwarded_key_is_a_real_run_task_param(self): + # Guard: whatever the mapper forwards must be accepted by run_task, so a + # future payload key can never smuggle an invalid kwarg through. Compare + # against the module's real param set (run_task is patched in _capture). + accepted = _RUN_TASK_PARAMS + seen = _capture( + { + "prompt": "p", + "model_id": "m", + "repo_url": "r", + "issue_number": 1, + "channel_source": "linear", + "channel_metadata": {"a": "b"}, + "build_command": "b", + "cedar_policies": ["c"], + "base_branch": "x", + "attachments": [{}], + "trace": False, + "user_id": "u", + "pr_number": 3, + "hydrated_context": {"k": "v"}, + "resolved_workflow": {"id": "w"}, + } + ) + assert set(seen).issubset(accepted) + + def test_warns_when_dropping_a_known_orchestrator_key(self): + # N4: a KNOWN orchestrator key that run_task doesn't accept is dropped + # (expected today) but logged, so a future "wired one side, forgot the + # other" contract gap (the ABCA-487 class) is visible, not silent. + logs: list[tuple[str, str]] = [] + with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))): + _capture({"build_command": "mise run build", "repo_url": "r"}) + assert [m for level, m in logs if level == "WARN" and "build_command" in m], ( + "expected a WARN when dropping the known orchestrator key build_command" + ) + + def test_does_NOT_warn_when_dropping_a_foreign_key(self): + # A genuinely-foreign key (not a known orchestrator field) is dropped + # quietly — no log noise for keys we never expected to forward. + logs: list[tuple[str, str]] = [] + with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))): + _capture({"some_future_unrelated_key": "v", "repo_url": "r"}) + assert not [m for level, m in logs if "some_future_unrelated_key" in m] + + def test_warns_when_dropping_task_started_at_HITL_parity(self): + # task_started_at drives the AgentCore HITL maxLifetime clip via + # TASK_STARTED_AT; the ECS path doesn't set it yet, so its drop must WARN + # (surface the AgentCore↔ECS parity gap, not silently fail-open). + logs: list[tuple[str, str]] = [] + with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))): + _capture({"task_started_at": "2026-07-14T00:00:00Z", "repo_url": "r"}) + assert [m for level, m in logs if level == "WARN" and "task_started_at" in m] + + def test_malformed_max_turns_is_dropped_not_raised(self): + # A non-integer max_turns must not crash the boot — it's dropped (run_task + # default applies) with a WARN, matching how every other field defaults. + logs: list[tuple[str, str]] = [] + with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))): + seen = _capture({"repo_url": "r", "max_turns": "not-a-number"}) + assert "max_turns" not in seen + assert [m for level, m in logs if level == "WARN" and "max_turns" in m] + + def test_valid_max_turns_still_coerced_to_int(self): + seen = _capture({"repo_url": "r", "max_turns": "50"}) + assert seen["max_turns"] == 50 + assert isinstance(seen["max_turns"], int) diff --git a/cdk/mise.toml b/cdk/mise.toml index 60332012..8847487d 100644 --- a/cdk/mise.toml +++ b/cdk/mise.toml @@ -40,8 +40,17 @@ run = ["mkdir -p $TMPDIR", "yarn synth:quiet"] description = "cdk deploy (pass args after --)" run = "npx cdk deploy" +# Bootstraps with ComputeTypes=agentcore (the template default). To ALSO enable the +# ECS compute backend you must set the ComputeTypes CFN *parameter* — `cdk bootstrap` +# has no way to pass template parameters (no --parameters flag; --context sets CDK +# construct context, NOT a CFN parameter), so run this task once, then set the +# parameter directly on the CDKToolkit stack: +# aws cloudformation update-stack --stack-name CDKToolkit --use-previous-template \ +# --capabilities CAPABILITY_NAMED_IAM \ +# --parameters ParameterKey=ComputeTypes,ParameterValue=agentcore\,ecs +# # verify: aws cloudformation describe-stacks --stack-name CDKToolkit --query 'Stacks[0].Parameters' [tasks.bootstrap] -description = "Bootstrap CDK with least-privilege policies (pass -- --context ComputeTypes=agentcore,ecs for ECS)" +description = "Bootstrap CDK with least-privilege policies (ComputeTypes=agentcore; for ECS set the ComputeTypes CFN param on CDKToolkit — see comment above the task)" depends = [":install", ":bootstrap:generate"] run = "npx cdk bootstrap --template bootstrap/bootstrap-template.yaml" diff --git a/cdk/src/constructs/ecs-agent-cluster.ts b/cdk/src/constructs/ecs-agent-cluster.ts index a73a8435..8228c861 100644 --- a/cdk/src/constructs/ecs-agent-cluster.ts +++ b/cdk/src/constructs/ecs-agent-cluster.ts @@ -28,6 +28,7 @@ import * as s3 from 'aws-cdk-lib/aws-s3'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; import { NagSuppressions } from 'cdk-nag'; import { Construct } from 'constructs'; +import { AgentMemory } from './agent-memory'; import { AgentSessionRole } from './agent-session-role'; import { resolveBedrockModelIds } from './bedrock-models'; @@ -52,6 +53,23 @@ export interface EcsAgentClusterProps { */ readonly payloadBucket?: s3.IBucket; + /** + * Artifacts bucket for repo-bound artifact workflows (#299 coding/decompose-v1 + * emits its plan JSON here via ``deliver_artifact``). The AgentCore runtime + * gets ``ARTIFACTS_BUCKET_NAME`` in its env; the ECS task needs the SAME env + + * read/write grant or an artifact workflow fails at delivery with + * "ARTIFACTS_BUCKET_NAME is not configured" (live-caught: a :decompose on an + * ecs-configured repo). Read/WRITE because the container DELIVERS the artifact + * (unlike the read-only payload bucket). + * + * NOTE: this wires only ``ARTIFACTS_BUCKET_NAME`` (artifact delivery). It does + * NOT set ``TRACE_ARTIFACTS_BUCKET_NAME`` (telemetry.py reads that for the + * ``--trace`` upload), so ``--trace`` silently skips on ECS today — a separate + * ECS-parity gap, not wired here. + * Omitted in isolated construct tests → no env/grant. + */ + readonly artifactsBucket?: s3.IBucket; + /** * Per-task SessionRole (#209). When provided, tenant-data DynamoDB access * (task/events tables) is NOT granted to the Fargate task role; instead the @@ -62,6 +80,19 @@ export interface EcsAgentClusterProps { * retains the legacy direct grants. */ readonly agentSessionRole?: AgentSessionRole; + + /** + * AgentCore Memory for cross-task learning (F-2 / ABCA-488-class parity). When + * provided, the ECS task role is granted read+write on it so the agent's + * memory writes (write_task_episode / write_repo_learnings → + * ``bedrock-agentcore:CreateEvent``) succeed on the ECS substrate. The + * AgentCore runtime role already gets this via ``agentMemory.grantReadWrite`` + * in agent.ts; without the same grant here, memory writes hit AccessDenied and + * no-op on ECS (logged, non-fatal — memory.py treats an AccessDenied as an + * infra failure), so learning never persists on an ECS-only deployment. + * Omitted in isolated construct tests / memory-less deployments. + */ + readonly agentMemory?: AgentMemory; } /** HTTPS port — the only egress allowed from the agent task ENIs. */ @@ -109,10 +140,26 @@ export class EcsAgentCluster extends Construct { // CDK creates this automatically via taskDefinition, but we need to // grant additional permissions to the task role. - // Fargate task definition + // Fargate task definition. Sized for heavy CI-parity builds (e.g. ABCA's + // own ~2800-test `mise run build` + cdk synth). Sizing history (all + // live-caught dogfooding ABCA-on-ABCA, 2026-06-29): + // - 4 GB / 2 vCPU → OOM-killed even the AgentCore microVM. + // - 32 GB / 8 vCPU → ran ~50 min then OOM-killed (exit 137) at the cap; + // peak working set ~31.6 GB when the root build fans out 4 heavy jobs + // in PARALLEL (agent:quality ‖ cdk:build ‖ cli:build ‖ docs:build), + // each spawning its own worker fleet (jest maxWorkers, pytest, esbuild + // Lambda bundling). 32 GB had no headroom for that concurrent peak. + // - 64 GB / 16 vCPU → still OOM-killed (exit 137) on ABCA-662's baseline + // build: the parallel storm's peak exceeded 64 GB too. The false + // "build_before=broken" that followed is fixed in repo.py, but the build + // itself genuinely needs more RAM. + // - 120 GB / 16 vCPU (current) → the MAX Fargate admits at 16 vCPU (32–120 + // GB in 8 GB steps). If a build OOMs even here, the fix is to cut the + // build's peak parallelism (serialize the mise DAG / cap jest workers), + // not more RAM — there is none. Paired with BUILD_VERIFY_TIMEOUT_S=3600. this.taskDefinition = new ecs.FargateTaskDefinition(this, 'TaskDef', { - cpu: 2048, - memoryLimitMiB: 4096, + cpu: 16384, + memoryLimitMiB: 122880, runtimePlatform: { cpuArchitecture: ecs.CpuArchitecture.ARM64, operatingSystemFamily: ecs.OperatingSystemFamily.LINUX, @@ -133,11 +180,22 @@ export class EcsAgentCluster extends Construct { USER_CONCURRENCY_TABLE_NAME: props.userConcurrencyTable.tableName, LOG_GROUP_NAME: logGroup.logGroupName, GITHUB_TOKEN_SECRET_ARN: props.githubTokenSecret.secretArn, + // Heavy CI-parity builds on this big-box substrate legitimately run + // longer than the 1800s default (ABCA's own `mise run build` is + // ~50 min cold). Raise the post-agent build-verify cap so a slow-but- + // healthy build isn't mis-reported as a timeout (see post_hooks.py + // BUILD_VERIFY_TIMEOUT_S). ECS-only: AgentCore repos keep the default. + BUILD_VERIFY_TIMEOUT_S: '3600', ...(props.memoryId && { MEMORY_ID: props.memoryId }), // #502: the payload bucket name so the orchestrator-issued // AGENT_PAYLOAD_S3_URI can be fetched. (The orchestrator sets the URI // per-task via container override; this is informational parity.) ...(props.payloadBucket && { ECS_PAYLOAD_BUCKET: props.payloadBucket.bucketName }), + // #299 ECS-parity: artifact workflows (coding/decompose-v1) deliver their + // plan JSON to this bucket. The AgentCore runtime has ARTIFACTS_BUCKET_NAME; + // the ECS task needs it too or deliver_artifact raises "ARTIFACTS_BUCKET_NAME + // is not configured" (live-caught on an ecs-repo :decompose). + ...(props.artifactsBucket && { ARTIFACTS_BUCKET_NAME: props.artifactsBucket.bucketName }), // Per-session IAM scoping (#209): when a SessionRole is wired, the // agent assumes it for tenant-data access (see aws_session.py). ...(props.agentSessionRole && { @@ -175,6 +233,58 @@ export class EcsAgentCluster extends Construct { props.payloadBucket.grantRead(taskRole); } + // #299 ECS-parity: coding/decompose-v1 delivers its plan to the artifacts + // bucket via deliver_artifact — but the write goes through the assumed + // SessionRole (deliverers.py -> tenant_client), scoped to + // artifacts/${task_id}/*, exactly like the AgentCore runtime (whose task + // role likewise has NO direct artifacts grant). So the task role needs only + // the ARTIFACTS_BUCKET_NAME env (set above), not a bucket grant. Granting + // whole-bucket read+write here would over-privilege the untrusted-code role + // and break cross-task isolation (a task could read/clobber other tasks' + // artifacts//, traces/, attachments/ on the same bucket). + // (no props.artifactsBucket grant — intentional; see comment) + + // F-2 (ABCA-488-class parity): grant the task role read+write on the + // AgentCore Memory so the agent's cross-task learning writes + // (write_task_episode / write_repo_learnings → bedrock-agentcore:CreateEvent) + // succeed on ECS. The AgentCore runtime role gets this via + // agentMemory.grantReadWrite(runtime) in agent.ts; without the same grant + // here the writes hit AccessDenied and no-op on the ECS substrate (logged, + // non-fatal), so learning never persists on an ECS-only deployment. + if (props.agentMemory) { + props.agentMemory.grantReadWrite(taskRole); + } + + // ABCA-488: per-workspace Linear/Jira OAuth tokens live in Secrets Manager + // under `bgagent-linear-oauth-*` (written by the CLI at setup). For a + // Linear/Jira-channel task the agent resolves that token at startup + // (config.resolve_linear_api_token / resolve_jira_oauth_token) to fire the + // 👀→✅ reaction and drive the channel MCP. The AgentCore runtime role + + // orchestrator/fanout/screenshot roles all have this prefix grant; the ECS + // task role did NOT, so on ECS the token fetch hit AccessDenied and + // reactions/MCP no-op'd — logged by config.py's token resolver, not silent, + // but the channel effect (no 👀→✅, no MCP) is invisible to the user + // (ECS-parity gap, live-caught on ABCA-488). + // GetSecretValue only — the container reads the token; the orchestrator owns + // refresh/PutSecretValue. + taskRole.addToPrincipalPolicy(new iam.PolicyStatement({ + actions: ['secretsmanager:GetSecretValue'], + resources: [ + Stack.of(this).formatArn({ + service: 'secretsmanager', + resource: 'secret', + arnFormat: ArnFormat.COLON_RESOURCE_NAME, + resourceName: 'bgagent-linear-oauth-*', + }), + Stack.of(this).formatArn({ + service: 'secretsmanager', + resource: 'secret', + arnFormat: ArnFormat.COLON_RESOURCE_NAME, + resourceName: 'bgagent-jira-oauth-*', + }), + ], + })); + // Bedrock model invocation — scoped to explicit foundation-model and // cross-region inference-profile ARNs (parity with the AgentCore runtime // grants in agent.ts), NOT a Resource: '*' wildcard. The model set is the @@ -208,6 +318,22 @@ export class EcsAgentCluster extends Construct { resources: bedrockResources, })); + // ECS-parity: a CDK-based target repo's build gate runs `cdk synth`, and a + // stack wired to a concrete env ({account, region}) does a synth-time + // availability-zone context lookup (ec2:DescribeAvailabilityZones). On a + // developer box the gitignored cdk.context.json caches the answer so synth + // is hermetic; the agent clones fresh, so there's no cache and synth fires + // the live lookup. Without this grant the ECS task role hit AccessDenied → + // "Synthesis finished with errors" → a FALSE build-gate failure on code that + // builds fine everywhere else (live-caught on the ABCA fork; same class as + // the ABCA-488 GetSecretValue and F-2 CreateEvent ECS-parity gaps). This is a + // read-only describe with no resource-level scoping in IAM, so Resource:* is + // required (suppressed below); it grants no mutation and no data access. + taskRole.addToPrincipalPolicy(new iam.PolicyStatement({ + actions: ['ec2:DescribeAvailabilityZones'], + resources: ['*'], + })); + // CloudWatch Logs write logGroup.grantWrite(taskRole); @@ -218,7 +344,7 @@ export class EcsAgentCluster extends Construct { NagSuppressions.addResourceSuppressions(this.taskDefinition, [ { id: 'AwsSolutions-IAM5', - reason: 'DynamoDB index/* wildcards from CDK grantReadWriteData (UserConcurrencyTable, and task tables only when no SessionRole is wired); Secrets Manager wildcards from CDK grantRead; CloudWatch Logs wildcards from CDK grantWrite; S3 object/* wildcard from CDK grantRead on the ECS payload bucket (read-only, scoped to that bucket — #502). Bedrock InvokeModel is scoped to explicit model/inference-profile ARNs (no wildcard resource).', + reason: 'DynamoDB index/* wildcards from CDK grantReadWriteData (UserConcurrencyTable, and task tables only when no SessionRole is wired); Secrets Manager wildcards from CDK grantRead (GitHub token) and the bgagent-linear-oauth-*/bgagent-jira-oauth-* prefix grant (ABCA-488 — per-workspace channel OAuth tokens are created by the CLI at setup, name unknown at synth, GetSecretValue only); CloudWatch Logs wildcards from CDK grantWrite; S3 object/* wildcard from CDK grantRead on the ECS payload bucket (read-only, scoped to that bucket — #502). Bedrock InvokeModel is scoped to explicit model/inference-profile ARNs (no wildcard resource). ec2:DescribeAvailabilityZones requires Resource:* (EC2 describe actions have no resource-level scoping) — read-only, no mutation/data access; needed so a CDK target repo\'s `cdk synth` build gate can resolve AZ context on a fresh clone (ECS-parity, no cdk.context.json cache in the container).', }, { id: 'AwsSolutions-ECS2', diff --git a/cdk/src/handlers/shared/strategies/ecs-strategy.ts b/cdk/src/handlers/shared/strategies/ecs-strategy.ts index df70e8a2..a45ef129 100644 --- a/cdk/src/handlers/shared/strategies/ecs-strategy.ts +++ b/cdk/src/handlers/shared/strategies/ecs-strategy.ts @@ -98,8 +98,17 @@ export class EcsComputeStrategy implements ComputeStrategy { blueprintConfig: BlueprintConfig; }): Promise { if (!ECS_CLUSTER_ARN || !ECS_TASK_DEFINITION_ARN || !ECS_SUBNETS || !ECS_SECURITY_GROUP) { + // Config/deploy mismatch: this repo is compute_type=ecs but the stack was + // deployed WITHOUT the ECS substrate (no `--context compute_type=ecs`), so + // the orchestrator has no ECS_* env vars. Name the root cause + remedy so an + // admin doesn't have to reverse-engineer it from a bare env-var list. (The + // CLI `repo onboard --compute-type ecs` guard normally prevents this; a repo + // onboarded before that guard, or edited directly, can still reach here.) throw new Error( - 'ECS compute strategy requires ECS_CLUSTER_ARN, ECS_TASK_DEFINITION_ARN, ECS_SUBNETS, and ECS_SECURITY_GROUP environment variables', + 'This repository is configured compute_type=ecs, but this stack was deployed without the ECS ' + + 'substrate (missing ECS_CLUSTER_ARN/ECS_TASK_DEFINITION_ARN/ECS_SUBNETS/ECS_SECURITY_GROUP). ' + + 'Redeploy the stack with `--context compute_type=ecs` to provision the Fargate substrate, or ' + + 'set this repo to compute_type=agentcore (bgagent repo onboard --compute-type agentcore).', ); } @@ -169,38 +178,28 @@ export class EcsComputeStrategy implements ComputeStrategy { // Override the container command to run a Python one-liner that: // 1. Loads the payload — from S3 (AGENT_PAYLOAD_S3_URI) when set, else the // inline AGENT_PAYLOAD env var (fallback). - // 2. Calls entrypoint.run_task() directly with all fields. + // 2. Calls entrypoint.run_task_from_payload(p), which maps the WHOLE payload + // dict to run_task's signature (rename prompt→task_description / + // model_id→anthropic_model, filter to accepted params, coerce str/int). + // This replaces the old hand-listed kwarg subset that silently dropped + // channel_source/channel_metadata (no Linear/Jira reactions or channel + // MCP on ECS — ABCA-487), build_command, cedar_policies, base_branch/ + // merge_branches, attachments, trace, user_id, etc. Single source of + // truth in the agent, unit-tested (see test_run_task_from_payload). // 3. Exits with code 0 on success, 1 on failure. // This bypasses the uvicorn server entirely — no HTTP, no OTEL noise. const bootCommand = [ 'python', '-c', 'import json, os, sys; ' + 'sys.path.insert(0, "/app/src"); ' - + 'from entrypoint import run_task; ' + + 'from entrypoint import run_task_from_payload; ' + '_uri = os.environ.get("AGENT_PAYLOAD_S3_URI"); ' + 'p = (' + 'json.loads(__import__("boto3").client("s3").get_object(' + 'Bucket=_uri.split("/",3)[2], Key=_uri.split("/",3)[3])["Body"].read()) ' + 'if _uri else json.loads(os.environ["AGENT_PAYLOAD"])' + '); ' - + 'r = run_task(' - + 'repo_url=p.get("repo_url",""), ' - + 'task_description=p.get("prompt",""), ' - + 'issue_number=str(p.get("issue_number","")), ' - + 'github_token=p.get("github_token",""), ' - + 'anthropic_model=p.get("model_id",""), ' - + 'max_turns=int(p.get("max_turns",100)), ' - + 'max_budget_usd=p.get("max_budget_usd"), ' - + 'aws_region=os.environ.get("AWS_REGION",""), ' - + 'task_id=p.get("task_id",""), ' - + 'hydrated_context=p.get("hydrated_context"), ' - + 'system_prompt_overrides=p.get("system_prompt_overrides",""), ' - + 'prompt_version=p.get("prompt_version",""), ' - + 'memory_id=p.get("memory_id",""), ' - + 'resolved_workflow=p.get("resolved_workflow"), ' - + 'branch_name=p.get("branch_name",""), ' - + 'pr_number=str(p.get("pr_number",""))' - + '); ' + + 'r = run_task_from_payload(p); ' + 'sys.exit(0 if r.get("status")=="success" else 1)', ]; diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 1382dce4..aa4c268c 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -22,8 +22,7 @@ import * as bedrock from '@aws-cdk/aws-bedrock-alpha'; import { ArnFormat, AspectPriority, Aspects, Stack, StackProps, RemovalPolicy, CfnOutput, CfnResource, Duration, Fn, Lazy } from 'aws-cdk-lib'; import * as agentcore from 'aws-cdk-lib/aws-bedrockagentcore'; import * as ec2 from 'aws-cdk-lib/aws-ec2'; -// ecr_assets import is only needed when the ECS block below is uncommented -// import * as ecr_assets from 'aws-cdk-lib/aws-ecr-assets'; +import * as ecr_assets from 'aws-cdk-lib/aws-ecr-assets'; import * as iam from 'aws-cdk-lib/aws-iam'; import * as logs from 'aws-cdk-lib/aws-logs'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; @@ -40,8 +39,8 @@ import { Blueprint } from '../constructs/blueprint'; import { CedarWasmLayer } from '../constructs/cedar-wasm-layer'; import { ConcurrencyReconciler } from '../constructs/concurrency-reconciler'; import { DnsFirewall } from '../constructs/dns-firewall'; -// import { EcsAgentCluster } from '../constructs/ecs-agent-cluster'; -// import { EcsPayloadBucket } from '../constructs/ecs-payload-bucket'; +import { EcsAgentCluster } from '../constructs/ecs-agent-cluster'; +import { EcsPayloadBucket } from '../constructs/ecs-payload-bucket'; import { FanOutConsumer } from '../constructs/fanout-consumer'; import { GitHubScreenshotIntegration } from '../constructs/github-screenshot-integration'; import { JiraIntegration } from '../constructs/jira-integration'; @@ -577,38 +576,74 @@ export class AgentStack extends Stack { description: 'Name of the S3 bucket storing --trace trajectory artifacts (design §10.1)', }); - // --- ECS Fargate compute backend (optional) --- - // To enable ECS as an alternative compute backend, uncomment the block below - // and the EcsAgentCluster import at the top of this file. Repos can then use - // compute_type: 'ecs' in their blueprint config to route tasks to ECS Fargate. - // - // const agentImageAsset = new ecr_assets.DockerImageAsset(this, 'AgentImage', { - // directory: repoRoot, - // file: 'agent/Dockerfile', - // platform: ecr_assets.Platform.LINUX_ARM64, - // }); - // - // // #502: ephemeral bucket for ECS task payloads — the orchestrator writes - // // the payload here (it exceeds the 8 KB RunTask containerOverrides limit) - // // and passes only an S3 URI pointer; the container fetches it on boot. - // const ecsPayloadBucket = new EcsPayloadBucket(this, 'EcsPayloadBucket'); - // - // const ecsCluster = new EcsAgentCluster(this, 'EcsAgentCluster', { - // vpc: agentVpc.vpc, - // agentImageAsset, - // taskTable: taskTable.table, - // taskEventsTable: taskEventsTable.table, - // userConcurrencyTable: userConcurrencyTable.table, - // githubTokenSecret, - // memoryId: agentMemory.memory.memoryId, - // // #502: read-only grant so the container can fetch its payload. - // payloadBucket: ecsPayloadBucket.bucket, - // // Per-session IAM scoping (#209): the ECS task role assumes the same - // // SessionRole as the AgentCore runtime for tenant-data access. The - // // construct admits the task role to the trust and injects - // // AGENT_SESSION_ROLE_ARN into the container. - // agentSessionRole, - // }); + // --- ECS Fargate compute backend (CONTEXT-GATED) --- + // K12 (2026-06-29): AgentCore's fixed microVM envelope OOM-kills heavy + // CI-parity builds (ABCA's own ~2800-test `mise run build`). ECS Fargate + // gives a bigger, tunable task (see EcsAgentCluster for the exact vCPU/memory + // sizing + its OOM history — 64 GB was itself OOM-killed, so it runs larger) + // for repos that set ``compute_type: 'ecs'``. GATED on the ``compute_type`` deploy context + // (default 'agentcore') — ECS resources only synthesize when you deploy with + // ``--context compute_type=ecs``, so the default synth (and the + // bootstrap-coverage test that synths with default context) stays + // agentcore-only. Mirrors upstream #164 (gate ECS construct on context). + const computeType = this.node.tryGetContext('compute_type') ?? 'agentcore'; + // #502: ephemeral bucket for ECS task payloads — the orchestrator writes the + // payload here (it exceeds the 8 KB RunTask containerOverrides limit) and + // passes only an S3 URI pointer; the container fetches it on boot, the + // orchestrator deletes it at finalize. Only synthesized under the ecs gate. + const ecsPayloadBucket = computeType === 'ecs' + ? new EcsPayloadBucket(this, 'EcsPayloadBucket') + : undefined; + if (ecsPayloadBucket) { + NagSuppressions.addResourceSuppressions(ecsPayloadBucket.bucket, [ + { + id: 'AwsSolutions-S1', + reason: 'Ephemeral per-task payloads (#502) with a 1-day TTL; writes confined to the orchestrator IAM role by grantPut, reads to the ECS task role by grantRead, both scoped to this bucket. Object deleted at finalize. Object-level audit intentionally omitted — CloudTrail data events / a log bucket are not justified for transient boot payloads.', + }, + ]); + } + const ecsCluster = computeType === 'ecs' + ? new EcsAgentCluster(this, 'EcsAgentCluster', { + vpc: agentVpc.vpc, + agentImageAsset: new ecr_assets.DockerImageAsset(this, 'AgentImage', { + directory: repoRoot, + file: 'agent/Dockerfile', + platform: ecr_assets.Platform.LINUX_ARM64, + }), + taskTable: taskTable.table, + taskEventsTable: taskEventsTable.table, + userConcurrencyTable: userConcurrencyTable.table, + githubTokenSecret, + memoryId: agentMemory.memory.memoryId, + // F-2: grant the ECS task role read+write on AgentCore Memory so the + // agent's cross-task learning writes succeed on ECS (parity with the + // runtime's agentMemory.grantReadWrite below). + agentMemory, + // #502: read-only grant so the container can fetch its payload from S3. + payloadBucket: ecsPayloadBucket!.bucket, + // #299 ECS-parity: the same bucket the runtime uses for ARTIFACTS_BUCKET_NAME — + // coding/decompose-v1 delivers its plan artifact here (read+write grant in + // the construct). Without this, an ecs-repo :decompose fails at delivery. + artifactsBucket: traceArtifactsBucket.bucket, + // Per-session IAM scoping (#209): the ECS task role assumes the same + // SessionRole as the AgentCore runtime for tenant-data access. The + // construct admits the task role to the trust and injects + // AGENT_SESSION_ROLE_ARN into the container. + agentSessionRole, + }) + : undefined; + + // Advertise which compute substrate this deploy actually provisioned, so the + // CLI can refuse to onboard a repo as ``compute_type: ecs`` when the ECS gate + // wasn't on (``--context compute_type=ecs``) — otherwise that mismatch only + // surfaces per-task as "ECS compute strategy requires ECS_CLUSTER_ARN…" at + // runtime. ``ecs`` implies the AgentCore runtime is ALSO available (the ECS + // gate is additive), so an agentcore repo works on either substrate. + new CfnOutput(this, 'ComputeSubstrate', { + value: ecsCluster ? 'ecs' : 'agentcore', + description: 'Compute substrate provisioned by this deploy: "agentcore" (default) or "ecs" ' + + '(deployed with --context compute_type=ecs; adds the Fargate substrate alongside AgentCore).', + }); // --- Task Orchestrator (durable Lambda function) --- const orchestrator = new TaskOrchestrator(this, 'TaskOrchestrator', { @@ -622,19 +657,22 @@ export class AgentStack extends Stack { guardrailId: inputGuardrail.guardrailId, guardrailVersion: inputGuardrail.guardrailVersion, attachmentsBucket: attachmentsBucket.bucket, - // To wire ECS, uncomment the ecsCluster block above and add: - // ecsConfig: { - // clusterArn: ecsCluster.cluster.clusterArn, - // taskDefinitionArn: ecsCluster.taskDefinition.taskDefinitionArn, - // subnets: agentVpc.vpc.selectSubnets({ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }).subnetIds.join(','), - // securityGroup: ecsCluster.securityGroup.securityGroupId, - // containerName: ecsCluster.containerName, - // taskRoleArn: ecsCluster.taskRoleArn, - // executionRoleArn: ecsCluster.executionRoleArn, - // }, - // // #502: pass the payload bucket so the orchestrator writes/deletes the - // // out-of-band payload and the ECS strategy builds the S3 URI pointer. - // ecsPayloadBucket: ecsPayloadBucket.bucket, + // K12: route ``compute_type: 'ecs'`` repos to the Fargate cluster above — + // only when the cluster was synthesized (deploy --context compute_type=ecs). + ...(ecsCluster && { + ecsConfig: { + clusterArn: ecsCluster.cluster.clusterArn, + taskDefinitionArn: ecsCluster.taskDefinition.taskDefinitionArn, + subnets: agentVpc.vpc.selectSubnets({ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }).subnetIds.join(','), + securityGroup: ecsCluster.securityGroup.securityGroupId, + containerName: ecsCluster.containerName, + taskRoleArn: ecsCluster.taskRoleArn, + executionRoleArn: ecsCluster.executionRoleArn, + }, + }), + // #502: pass the payload bucket so the orchestrator writes/deletes the + // out-of-band payload and the ECS strategy builds the S3 URI pointer. + ...(ecsPayloadBucket && { ecsPayloadBucket: ecsPayloadBucket.bucket }), }); // Now that the orchestrator exists, resolve the Lazy used by TaskApi at synth. diff --git a/cdk/test/constructs/ecs-agent-cluster.test.ts b/cdk/test/constructs/ecs-agent-cluster.test.ts index fe0ec378..929e8c89 100644 --- a/cdk/test/constructs/ecs-agent-cluster.test.ts +++ b/cdk/test/constructs/ecs-agent-cluster.test.ts @@ -26,6 +26,7 @@ import * as ecr_assets from 'aws-cdk-lib/aws-ecr-assets'; import * as iam from 'aws-cdk-lib/aws-iam'; import * as s3 from 'aws-cdk-lib/aws-s3'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; +import { AgentMemory } from '../../src/constructs/agent-memory'; import { AgentSessionRole } from '../../src/constructs/agent-session-role'; import { EcsAgentCluster } from '../../src/constructs/ecs-agent-cluster'; @@ -88,10 +89,10 @@ describe('EcsAgentCluster construct', () => { }); }); - test('creates a Fargate task definition with 2 vCPU and 4 GB', () => { + test('creates a Fargate task definition with 16 vCPU and 120 GB (ABCA-662: full parallel mise build OOM\'d at 64 GB → max Fargate RAM)', () => { baseTemplate.hasResourceProperties('AWS::ECS::TaskDefinition', { - Cpu: '2048', - Memory: '4096', + Cpu: '16384', + Memory: '122880', RequiresCompatibilities: ['FARGATE'], RuntimePlatform: { CpuArchitecture: 'ARM64', @@ -155,6 +156,83 @@ describe('EcsAgentCluster construct', () => { }); }); + test('task role can read the per-workspace Linear/Jira OAuth secrets (ABCA-488)', () => { + // REGRESSION: a Linear/Jira-channel task resolves its per-workspace OAuth + // token (bgagent-linear-oauth-) at startup to fire the 👀→✅ reaction + // and drive the channel MCP. Without a prefix grant on the ECS task role the + // fetch hit AccessDenied and reactions/MCP silently no-op'd on ECS (worked on + // AgentCore). Pin a GetSecretValue statement whose resource ARN names the + // bgagent-linear-oauth-* prefix. + const policies = baseTemplate.findResources('AWS::IAM::Policy'); + let hasLinearOauthGrant = false; + for (const p of Object.values(policies)) { + for (const s of p.Properties.PolicyDocument.Statement) { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + if (!actions.includes('secretsmanager:GetSecretValue')) continue; + if (JSON.stringify(s.Resource).includes('bgagent-linear-oauth-')) hasLinearOauthGrant = true; + } + } + expect(hasLinearOauthGrant).toBe(true); + }); + + test('task role gets bedrock-agentcore:CreateEvent on the AgentMemory when wired (F-2 / ABCA-488-class)', () => { + // REGRESSION: the agent's cross-task learning writes (write_task_episode / + // write_repo_learnings) call bedrock-agentcore:CreateEvent on the AgentCore + // Memory. The runtime role gets this via agentMemory.grantReadWrite; the ECS + // task role did NOT, so writes hit AccessDenied and silently no-op'd (WARN) + // on the ECS substrate — learning never persisted on an ECS-only deploy. + // Build a stack WITH an AgentMemory and assert the CreateEvent grant exists, + // scoped to the memory ARN (not a wildcard). + const app = new App(); + const stack = new Stack(app, 'EcsMemStack'); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2 }); + const agentImageAsset = new ecr_assets.DockerImageAsset(stack, 'AgentImage', { + directory: path.join(__dirname, '..', '..', '..', 'agent'), + }); + const mk = (id: string) => + new dynamodb.Table(stack, id, { partitionKey: { name: 'task_id', type: dynamodb.AttributeType.STRING } }); + const userConcurrencyTable = new dynamodb.Table(stack, 'UserConcurrencyTable', { + partitionKey: { name: 'user_id', type: dynamodb.AttributeType.STRING }, + }); + const agentMemory = new AgentMemory(stack, 'AgentMemory'); + new EcsAgentCluster(stack, 'EcsAgentCluster', { + vpc, + agentImageAsset, + taskTable: mk('TaskTable'), + taskEventsTable: mk('TaskEventsTable'), + userConcurrencyTable, + githubTokenSecret: new secretsmanager.Secret(stack, 'GitHubTokenSecret'), + agentMemory, + }); + const template = Template.fromStack(stack); + const policies = template.findResources('AWS::IAM::Policy'); + let hasCreateEvent = false; + for (const [id, p] of Object.entries(policies)) { + if (!id.includes('TaskDefTaskRole')) continue; + for (const s of p.Properties.PolicyDocument.Statement) { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + if (actions.includes('bedrock-agentcore:CreateEvent')) { + hasCreateEvent = true; + // resource must reference the memory ARN, not a bare wildcard + expect(JSON.stringify(s.Resource)).toContain('MemoryArn'); + expect(s.Resource).not.toEqual('*'); + } + } + } + expect(hasCreateEvent).toBe(true); + }); + + test('task role has NO bedrock-agentcore grant when no AgentMemory is wired (isolated default)', () => { + const policies = baseTemplate.findResources('AWS::IAM::Policy'); + for (const [id, p] of Object.entries(policies)) { + if (!id.includes('TaskDefTaskRole')) continue; + for (const s of p.Properties.PolicyDocument.Statement) { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + expect(actions.some((a: string) => a.startsWith('bedrock-agentcore:'))).toBe(false); + } + } + }); + test('task role Bedrock InvokeModel is scoped to explicit model/inference-profile ARNs (no wildcard)', () => { const policies = baseTemplate.findResources('AWS::IAM::Policy'); let bedrockStatement: { Resource: unknown } | undefined; @@ -176,6 +254,26 @@ describe('EcsAgentCluster construct', () => { expect(serialized).toContain('anthropic.claude-haiku-4-5-20251001-v1:0'); }); + test('task role can DescribeAvailabilityZones so a CDK target repo can `cdk synth` on a fresh clone (ECS-parity)', () => { + // REGRESSION: `mise run build` on a CDK-based target repo runs `cdk synth`, + // and a stack wired to a concrete env does a synth-time AZ context lookup + // (ec2:DescribeAvailabilityZones). A dev box caches the answer in the + // gitignored cdk.context.json; the agent clones fresh (no cache) → the live + // lookup fires. Without this grant the ECS task role hit AccessDenied → + // "Synthesis finished with errors" → a FALSE build-gate failure. Pin the + // read-only describe (Resource:* — EC2 describe has no resource scoping). + const policies = baseTemplate.findResources('AWS::IAM::Policy'); + let azStatement: { Resource: unknown } | undefined; + for (const p of Object.values(policies)) { + for (const s of p.Properties.PolicyDocument.Statement) { + const actions = Array.isArray(s.Action) ? s.Action : [s.Action]; + if (actions.includes('ec2:DescribeAvailabilityZones')) azStatement = s; + } + } + expect(azStatement).toBeDefined(); + expect(azStatement!.Resource).toEqual('*'); + }); + test('bedrockModels context override changes the granted model ARNs (#433)', () => { const template = createStack({ bedrockModels: ['anthropic.claude-opus-4-8'] }).template; const policies = template.findResources('AWS::IAM::Policy'); @@ -210,6 +308,9 @@ describe('EcsAgentCluster construct', () => { Match.objectLike({ Name: 'TASK_EVENTS_TABLE_NAME', Value: Match.anyValue() }), Match.objectLike({ Name: 'USER_CONCURRENCY_TABLE_NAME', Value: Match.anyValue() }), Match.objectLike({ Name: 'LOG_GROUP_NAME', Value: Match.anyValue() }), + // K14: ECS big-box substrate raises the build-verify cap so a + // slow-but-healthy CI-parity build isn't mis-flagged as a timeout. + Match.objectLike({ Name: 'BUILD_VERIFY_TIMEOUT_S', Value: '3600' }), ]), }), ]), @@ -410,3 +511,81 @@ describe('EcsAgentCluster payload bucket (#502)', () => { } }); }); + +describe('EcsAgentCluster artifacts bucket (#299 ECS-parity)', () => { + function createWithArtifactsBucket(): Template { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2 }); + const agentImageAsset = new ecr_assets.DockerImageAsset(stack, 'AgentImage', { + directory: path.join(__dirname, '..', '..', '..', 'agent'), + }); + const taskTable = new dynamodb.Table(stack, 'TaskTable', { + partitionKey: { name: 'task_id', type: dynamodb.AttributeType.STRING }, + }); + const taskEventsTable = new dynamodb.Table(stack, 'TaskEventsTable', { + partitionKey: { name: 'task_id', type: dynamodb.AttributeType.STRING }, + sortKey: { name: 'event_id', type: dynamodb.AttributeType.STRING }, + }); + const userConcurrencyTable = new dynamodb.Table(stack, 'UserConcurrencyTable', { + partitionKey: { name: 'user_id', type: dynamodb.AttributeType.STRING }, + }); + const githubTokenSecret = new secretsmanager.Secret(stack, 'GitHubTokenSecret'); + const artifactsBucket = new s3.Bucket(stack, 'ArtifactsBucket'); + + new EcsAgentCluster(stack, 'EcsAgentCluster', { + vpc, + agentImageAsset, + taskTable, + taskEventsTable, + userConcurrencyTable, + githubTokenSecret, + artifactsBucket, + }); + return Template.fromStack(stack); + } + + test('injects ARTIFACTS_BUCKET_NAME into the container env (parity with the AgentCore runtime)', () => { + createWithArtifactsBucket().hasResourceProperties('AWS::ECS::TaskDefinition', { + ContainerDefinitions: Match.arrayWith([ + Match.objectLike({ + Environment: Match.arrayWith([ + Match.objectLike({ Name: 'ARTIFACTS_BUCKET_NAME', Value: Match.anyValue() }), + ]), + }), + ]), + }); + }); + + test('does NOT grant the task role write on the artifacts bucket (the scoped SessionRole owns delivery)', () => { + // #596 review B1: coding/decompose-v1 delivers via the assumed SessionRole + // (scoped to artifacts/${task_id}/*), exactly like the AgentCore runtime — + // whose task role likewise has no direct artifacts grant. A whole-bucket + // grantReadWrite here would over-privilege the untrusted-code role and break + // cross-task isolation. The task role gets only the ARTIFACTS_BUCKET_NAME env. + const template = createWithArtifactsBucket(); + const policies = template.findResources('AWS::IAM::Policy'); + const s3WriteActions = new Set(); + for (const policy of Object.values(policies)) { + for (const stmt of policy.Properties.PolicyDocument.Statement) { + const actions = Array.isArray(stmt.Action) ? stmt.Action : [stmt.Action]; + for (const a of actions) { + // Only true S3 mutations — Put*/Delete*. The read-only payload bucket + // (#502) legitimately grants GetObject*/List* on the task role, so those + // are NOT flagged; what must be absent is any write to any S3 bucket. + if (typeof a === 'string' && /^s3:(Put|Delete)/.test(a)) s3WriteActions.add(a); + } + } + } + expect([...s3WriteActions]).toEqual([]); + }); + + test('omits ARTIFACTS_BUCKET_NAME when no artifacts bucket is provided', () => { + const { template } = createStack(); + const taskDefs = template.findResources('AWS::ECS::TaskDefinition'); + for (const def of Object.values(taskDefs)) { + const env = def.Properties.ContainerDefinitions[0].Environment ?? []; + expect(env.some((e: { Name: string }) => e.Name === 'ARTIFACTS_BUCKET_NAME')).toBe(false); + } + }); +}); diff --git a/cdk/test/handlers/shared/strategies/ecs-strategy.test.ts b/cdk/test/handlers/shared/strategies/ecs-strategy.test.ts index bee9c41a..d3b6a0d0 100644 --- a/cdk/test/handlers/shared/strategies/ecs-strategy.test.ts +++ b/cdk/test/handlers/shared/strategies/ecs-strategy.test.ts @@ -27,6 +27,14 @@ process.env.ECS_TASK_DEFINITION_ARN = TASK_DEF_ARN; process.env.ECS_SUBNETS = 'subnet-aaa,subnet-bbb'; process.env.ECS_SECURITY_GROUP = 'sg-12345'; process.env.ECS_CONTAINER_NAME = 'AgentContainer'; +// The top-of-file import's inline-fallback / no-op tests assume these OPTIONAL +// vars are ABSENT at load time. They are unset in a dev shell but the real ECS +// agent container HAS ECS_PAYLOAD_BUCKET set (#502) — so leaving this to ambient +// env made the build pass locally yet FAIL on ECS ("works local, dies on ECS"). +// The #502 / #299 describe blocks below set these via isolateModules; delete them +// here so the top-of-file import is hermetic regardless of the runner's env. +delete process.env.ECS_PAYLOAD_BUCKET; +delete process.env.ECS_PLANNING_TASK_DEFINITION_ARN; const mockSend = jest.fn(); jest.mock('@aws-sdk/client-ecs', () => ({ @@ -423,6 +431,13 @@ describe('EcsComputeStrategy with ECS_PAYLOAD_BUCKET (S3-pointer path, #502)', ( expect(src).toContain('AGENT_PAYLOAD_S3_URI'); expect(src).toContain('get_object'); expect(src).toContain('AGENT_PAYLOAD'); + // ABCA-487: the boot command maps the WHOLE payload via + // run_task_from_payload (not a hand-listed kwarg subset that dropped + // channel_source/channel_metadata → no Linear reactions on ECS). Assert we + // call the mapper and no longer hand-pick the old prompt/model_id kwargs. + expect(src).toContain('run_task_from_payload(p)'); + expect(src).not.toContain('task_description=p.get'); + expect(src).not.toContain('channel_source'); // never hand-listed; the mapper forwards it }); test('deleteEcsPayload deletes the task payload object', async () => { diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 3b8e6249..7c2610ee 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -69,6 +69,12 @@ describe('AgentStack', () => { }); }); + test('outputs ComputeSubstrate=agentcore on the default (no-gate) deploy', () => { + // The CLI reads this to refuse onboarding a repo as compute_type=ecs on a + // stack that never provisioned the ECS substrate. + template.hasOutput('ComputeSubstrate', { Value: 'agentcore' }); + }); + test('outputs CedarWasmLayerArn', () => { template.hasOutput('CedarWasmLayerArn', {}); }); @@ -502,3 +508,26 @@ describe('AgentStack', () => { }); }); }); + +describe('AgentStack with the ECS substrate gate (--context compute_type=ecs)', () => { + let template: Template; + + beforeAll(() => { + // Deploying with the gate on provisions the Fargate substrate alongside the + // always-present AgentCore runtime; the ComputeSubstrate output flips to 'ecs'. + const app = new App({ context: { compute_type: 'ecs' } }); + const stack = new AgentStack(app, 'TestAgentStackEcs', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + template = Template.fromStack(stack); + }); + + test('provisions an ECS cluster + Fargate task definition', () => { + template.resourceCountIs('AWS::ECS::Cluster', 1); + template.resourceCountIs('AWS::ECS::TaskDefinition', 1); + }); + + test('outputs ComputeSubstrate=ecs so the CLI allows compute_type=ecs onboarding', () => { + template.hasOutput('ComputeSubstrate', { Value: 'ecs' }); + }); +}); diff --git a/cli/src/commands/repo.ts b/cli/src/commands/repo.ts index d037934f..94a58f3d 100644 --- a/cli/src/commands/repo.ts +++ b/cli/src/commands/repo.ts @@ -166,16 +166,32 @@ export function makeRepoCommand(): Command { } const { region, stackName } = resolveOperatorContext(opts); - const [tableName, platformRuntimeArn, platformGithubTokenSecretArn] = await Promise.all([ + const [tableName, platformRuntimeArn, platformGithubTokenSecretArn, computeSubstrate] = await Promise.all([ getStackOutput(region, stackName, 'RepoTableName'), getStackOutput(region, stackName, 'RuntimeArn'), getStackOutput(region, stackName, 'GitHubTokenSecretArn'), + getStackOutput(region, stackName, 'ComputeSubstrate'), ]); if (!tableName) { throw new CliError( `Stack '${stackName}' is missing output 'RepoTableName'. Re-deploy the CDK stack.`, ); } + // Refuse to onboard a repo as compute_type=ecs when the deployed stack did + // NOT provision the ECS substrate — otherwise every task on this repo fails + // at session start with "ECS compute strategy requires ECS_CLUSTER_ARN…". + // Catch it here, at config time, with a fixable message. ComputeSubstrate is + // null on stacks predating this output; treat that as "unknown" and only + // hard-block on an explicit non-ecs value, so onboarding still works against + // an older deploy (the runtime error remains the backstop there). + if (opts.computeType === 'ecs' && computeSubstrate && computeSubstrate !== 'ecs') { + throw new CliError( + `Stack '${stackName}' was deployed without the ECS substrate (ComputeSubstrate=${computeSubstrate}), ` + + 'so a repo onboarded as --compute-type ecs would fail at task start. Redeploy the stack with ' + + '`--context compute_type=ecs` first (adds the Fargate substrate alongside AgentCore), then re-run this — ' + + 'or onboard with --compute-type agentcore.', + ); + } const config = await onboardRepo(region, tableName, repoId, { computeType: opts.computeType, diff --git a/cli/test/commands/repo.test.ts b/cli/test/commands/repo.test.ts index 17a8fd4d..cc115feb 100644 --- a/cli/test/commands/repo.test.ts +++ b/cli/test/commands/repo.test.ts @@ -166,6 +166,59 @@ describe('repo command JSON output', () => { expect(payload.repo.github_token_secret_arn).toContain('****'); }); + test('onboard --compute-type ecs is REFUSED when the stack has no ECS substrate', async () => { + // Per-key outputs: RepoTableName present, ComputeSubstrate=agentcore (deployed + // without --context compute_type=ecs). + getStackOutputMock.mockReset().mockImplementation((_r: string, _s: string, key: string) => + Promise.resolve(key === 'ComputeSubstrate' ? 'agentcore' : 'RepoTable-dev')); + + const cmd = makeRepoCommand(); + await expect(cmd.parseAsync([ + 'node', 'test', 'onboard', 'acme/a', '--region', 'us-east-1', '--compute-type', 'ecs', + ])).rejects.toThrow(/without the ECS substrate|compute_type=ecs/i); + // Must NOT write the repo row when it would be dead-on-arrival. + expect(onboardRepoMock).not.toHaveBeenCalled(); + }); + + test('onboard --compute-type ecs is ALLOWED when the stack provisioned ECS', async () => { + getStackOutputMock.mockReset().mockImplementation((_r: string, _s: string, key: string) => + Promise.resolve(key === 'ComputeSubstrate' ? 'ecs' : 'RepoTable-dev')); + onboardRepoMock.mockResolvedValue({ repo: 'acme/a', status: 'active', compute_type: 'ecs' }); + + const cmd = makeRepoCommand(); + await cmd.parseAsync([ + 'node', 'test', 'onboard', 'acme/a', '--region', 'us-east-1', '--compute-type', 'ecs', + ]); + expect(onboardRepoMock).toHaveBeenCalledWith( + 'us-east-1', 'RepoTable-dev', 'acme/a', expect.objectContaining({ computeType: 'ecs' })); + }); + + test('onboard --compute-type ecs proceeds against an OLDER stack lacking ComputeSubstrate (null → unknown)', async () => { + // Back-compat: pre-output stacks return null for ComputeSubstrate; don't hard-block + // (the runtime error is the backstop there). + getStackOutputMock.mockReset().mockImplementation((_r: string, _s: string, key: string) => + Promise.resolve(key === 'ComputeSubstrate' ? null : 'RepoTable-dev')); + onboardRepoMock.mockResolvedValue({ repo: 'acme/a', status: 'active', compute_type: 'ecs' }); + + const cmd = makeRepoCommand(); + await cmd.parseAsync([ + 'node', 'test', 'onboard', 'acme/a', '--region', 'us-east-1', '--compute-type', 'ecs', + ]); + expect(onboardRepoMock).toHaveBeenCalled(); + }); + + test('onboard --compute-type agentcore is unaffected by ComputeSubstrate', async () => { + getStackOutputMock.mockReset().mockImplementation((_r: string, _s: string, key: string) => + Promise.resolve(key === 'ComputeSubstrate' ? 'agentcore' : 'RepoTable-dev')); + onboardRepoMock.mockResolvedValue({ repo: 'acme/a', status: 'active', compute_type: 'agentcore' }); + + const cmd = makeRepoCommand(); + await cmd.parseAsync([ + 'node', 'test', 'onboard', 'acme/a', '--region', 'us-east-1', '--compute-type', 'agentcore', + ]); + expect(onboardRepoMock).toHaveBeenCalled(); + }); + test('repo offboard --output json redacts the per-repo secret ARN', async () => { offboardRepoMock.mockResolvedValue({ repo: 'acme/a', diff --git a/docs/design/DEPLOYMENT_ROLES.md b/docs/design/DEPLOYMENT_ROLES.md index 170aac0c..9f2f0bac 100644 --- a/docs/design/DEPLOYMENT_ROLES.md +++ b/docs/design/DEPLOYMENT_ROLES.md @@ -33,9 +33,18 @@ These policies are not created or attached manually. The repository generates th ```bash # Regenerate artifacts (policies JSON + template YAML) and bootstrap. -# Default ComputeTypes is "agentcore"; pass ECS via context to also include Compute-ECS: +# ComputeTypes defaults to "agentcore". MISE_EXPERIMENTAL=1 mise //cdk:bootstrap -MISE_EXPERIMENTAL=1 mise //cdk:bootstrap -- --context ComputeTypes=agentcore,ecs + +# To ALSO enable the ECS compute backend (attach IaCRole-ABCA-Compute-ECS), set the +# ComputeTypes CFN *parameter*. `cdk bootstrap` cannot pass template parameters — it +# has no --parameters flag, and --context sets CDK construct context, not a CFN +# parameter — so bootstrap first (above), then update the parameter on CDKToolkit: +aws cloudformation update-stack --stack-name CDKToolkit --use-previous-template \ + --capabilities CAPABILITY_NAMED_IAM \ + --parameters ParameterKey=ComputeTypes,ParameterValue=agentcore\,ecs +# verify it took: +aws cloudformation describe-stacks --stack-name CDKToolkit --query 'Stacks[0].Parameters' ``` Under the hood, `mise //cdk:bootstrap` runs `npx cdk bootstrap --template bootstrap/bootstrap-template.yaml` (see `cdk/mise.toml`). The generated template defines five inline `AWS::IAM::ManagedPolicy` resources that **replace** the default `AdministratorAccess` on the CloudFormation execution role; the `IaCRole-ABCA-Compute-ECS` policy is conditional on the `ComputeTypes` parameter including `ecs`. The policy sources are `cdk/src/bootstrap/policies/{infrastructure,application,observability,compute-agentcore,compute-ecs}.ts`, compiled to `cdk/bootstrap/policies/*.json` by `cdk/scripts/generate-bootstrap-artifacts.ts`. @@ -703,7 +712,7 @@ Bedrock AgentCore runtime/memory operations — a single statement granting `bed ### IaCRole-ABCA-Compute-ECS -When the ECS Fargate compute backend is enabled (bootstrap with `--context ComputeTypes=agentcore,ecs`), the generated template conditionally attaches this policy to the CloudFormation execution role. It is a standalone managed policy, not an addition to `IaCRole-ABCA-Application`. +When the ECS Fargate compute backend is enabled (set the `ComputeTypes` CFN parameter to `agentcore,ecs` on the `CDKToolkit` stack — see the bootstrap snippet near the top of this doc), the generated template conditionally attaches this policy to the CloudFormation execution role. It is a standalone managed policy, not an addition to `IaCRole-ABCA-Application`. ```json { diff --git a/docs/src/content/docs/architecture/Deployment-roles.md b/docs/src/content/docs/architecture/Deployment-roles.md index 94395aa8..a852a9b6 100644 --- a/docs/src/content/docs/architecture/Deployment-roles.md +++ b/docs/src/content/docs/architecture/Deployment-roles.md @@ -37,9 +37,18 @@ These policies are not created or attached manually. The repository generates th ```bash # Regenerate artifacts (policies JSON + template YAML) and bootstrap. -# Default ComputeTypes is "agentcore"; pass ECS via context to also include Compute-ECS: +# ComputeTypes defaults to "agentcore". MISE_EXPERIMENTAL=1 mise //cdk:bootstrap -MISE_EXPERIMENTAL=1 mise //cdk:bootstrap -- --context ComputeTypes=agentcore,ecs + +# To ALSO enable the ECS compute backend (attach IaCRole-ABCA-Compute-ECS), set the +# ComputeTypes CFN *parameter*. `cdk bootstrap` cannot pass template parameters — it +# has no --parameters flag, and --context sets CDK construct context, not a CFN +# parameter — so bootstrap first (above), then update the parameter on CDKToolkit: +aws cloudformation update-stack --stack-name CDKToolkit --use-previous-template \ + --capabilities CAPABILITY_NAMED_IAM \ + --parameters ParameterKey=ComputeTypes,ParameterValue=agentcore\,ecs +# verify it took: +aws cloudformation describe-stacks --stack-name CDKToolkit --query 'Stacks[0].Parameters' ``` Under the hood, `mise //cdk:bootstrap` runs `npx cdk bootstrap --template bootstrap/bootstrap-template.yaml` (see `cdk/mise.toml`). The generated template defines five inline `AWS::IAM::ManagedPolicy` resources that **replace** the default `AdministratorAccess` on the CloudFormation execution role; the `IaCRole-ABCA-Compute-ECS` policy is conditional on the `ComputeTypes` parameter including `ecs`. The policy sources are `cdk/src/bootstrap/policies/{infrastructure,application,observability,compute-agentcore,compute-ecs}.ts`, compiled to `cdk/bootstrap/policies/*.json` by `cdk/scripts/generate-bootstrap-artifacts.ts`. @@ -707,7 +716,7 @@ Bedrock AgentCore runtime/memory operations — a single statement granting `bed ### IaCRole-ABCA-Compute-ECS -When the ECS Fargate compute backend is enabled (bootstrap with `--context ComputeTypes=agentcore,ecs`), the generated template conditionally attaches this policy to the CloudFormation execution role. It is a standalone managed policy, not an addition to `IaCRole-ABCA-Application`. +When the ECS Fargate compute backend is enabled (set the `ComputeTypes` CFN parameter to `agentcore,ecs` on the `CDKToolkit` stack — see the bootstrap snippet near the top of this doc), the generated template conditionally attaches this policy to the CloudFormation execution role. It is a standalone managed policy, not an addition to `IaCRole-ABCA-Application`. ```json {