Skip to content

Commit 238eb30

Browse files
isadeksbgagentkrokoko
authored
feat(ecs): make the ECS Fargate compute backend usable (context-gated wiring + ECS-parity fixes) (#596)
* fix(ecs): write task payload to S3, not inline overrides (#502) The ECS compute strategy inlined the full orchestrator payload (incl. the large hydrated_context) into the AGENT_PAYLOAD container-override env var. ECS RunTask caps the TOTAL containerOverrides blob at 8192 bytes, so any real task was rejected before the container started: InvalidParameterException: Container Overrides length must be at most 8192 AgentCore is unaffected — it passes the payload in the InvokeAgentRuntime request body, which has no comparable limit. The bug only surfaces with a realistic hydrated payload, which is why the prior ECS smoke test (a small Rust cargo-check, #494) didn't catch it. Fix — stash the payload out-of-band and pass only a pointer: - New EcsPayloadBucket construct (mirrors TraceArtifactsBucket): BLOCK_ALL, enforceSSL, S3_MANAGED encryption, 1-day lifecycle TTL (payloads are ephemeral — read once at boot). Dedicated bucket so the ECS task role's S3 read is scoped to payloads only and can't touch attachments/traces. - ecs-strategy: when ECS_PAYLOAD_BUCKET is set, PutObject the payload to <task_id>/payload.json and pass AGENT_PAYLOAD_S3_URI in the override; the boot command fetches+parses it via boto3. Inline AGENT_PAYLOAD remains as a fallback (small payloads / no bucket), so nothing regresses. deleteEcsPayload helper removes the object. - orchestrate-task finalize: best-effort deleteEcsPayload for ECS tasks once terminal (the container has long since read it); lifecycle rule is the crash backstop. - EcsAgentCluster: accept payloadBucket, inject ECS_PAYLOAD_BUCKET env, grant the task role READ ONLY (untrusted repo code must not write/delete payloads; the trusted orchestrator owns write+delete). Session-role-aware. - task-orchestrator: ecsPayloadBucket prop → grantPut + grantDelete to the orchestrator; @aws-sdk/client-s3 added to bundling externals. - agent.ts: updated the commented uncomment-to-enable ECS scaffolding to wire the payload bucket. Tests: new bucket construct (TTL/SSL/block-public/autoDelete); strategy S3-write + URI-pointer + inline fallback + deleteEcsPayload (incl. best-effort swallow + no-op without bucket); cluster read-grant + env var + read-only (no put/delete). Full build green. Closes #502 * fix: enable corepack in the agent image so yarn/pnpm resolve Root cause of the exit-127 inert gate: the agent Dockerfile installs node/npm/ mise/uv but NOT yarn, so any repo whose build runs 'yarn install' (incl. ABCA) hit 'yarn: command not found' and the agent hand-built a ~/bin/yarn shim every run. corepack (ships with Node 24) installs the yarn/pnpm shims at image-build time. Systemic — fixes every yarn repo, no per-run shim. Needs redeploy to take effect. (cherry picked from commit 2fd5fca) (cherry picked from commit d5e0e64) * wire ECS/Fargate substrate (context-gated) at 32GB/8vCPU for heavy builds AgentCore's fixed microVM envelope OOM-kills CI-parity builds (live-caught dogfooding ABCA-on-ABCA: the ~2800-test `mise run build` killed the VM at the build-gate step). AgentCore memory isn't tunable, so route repos that set `compute_type: 'ecs'` to a Fargate task instead. - agent.ts: gate EcsAgentCluster + the orchestrator ecsConfig on the `compute_type` deploy context (default 'agentcore'). ECS resources only synthesize under `--context compute_type=ecs`, so default synth (and the bootstrap-coverage test) stays agentcore-only. Mirrors upstream's context-gating of the ECS construct. - ecs-agent-cluster.ts: size the Fargate task at 8 vCPU / 32 GB (was 2/4) — a valid ARM64 combo with headroom for the jest worker fleet + tsc that AgentCore's envelope can't give. Comment documents the live OOM. - test: assert the new 8192/32768 sizing. Verified: default synth has 0 ECS resources; `--context compute_type=ecs` synth emits AWS::ECS::Cluster + TaskDefinition. Full `mise run build` green. (cherry picked from commit 8cb7cfa) (cherry picked from commit 632a36b) * bump ECS Fargate to 64GB/16vCPU + BUILD_VERIFY_TIMEOUT_S=3600 Live-fired the full `mise run build` on the 32GB ECS task (fork dogfood): install OK, build ran ~50 min then OOM-killed at the cap (ECS stoppedReason "OutOfMemoryError: container killed due to memory usage", exit 137; peak working set ~31.6 GB). Root cause: the monorepo build fans out 4 heavy jobs in PARALLEL (agent:quality ‖ cdk:build ‖ cli:build ‖ docs:build), each with its own worker fleet (jest, pytest, esbuild Lambda bundling) — 32 GB had no headroom for that concurrent peak, and the ~50-min wall-clock also blew the 1800s build-gate cap. Fix (keep the full build as the gate, give it room): - ecs-agent-cluster.ts: Fargate 8vCPU/32GB → 16vCPU/64GB (valid ARM64 combo; 16 vCPU admits 32–120 GB). 2× memory headroom for the parallel storm + more cores to shorten wall-clock. Comment records the full sizing history. - ecs-agent-cluster.ts: inject BUILD_VERIFY_TIMEOUT_S=3600 into the container env so a slow-but-healthy CI-parity build isn't mis-flagged as a timeout (post_hooks.py reads it; default 1800 stays for AgentCore repos). - tests: assert 16384/65536 + the new env var. Verified: default synth ECS-free; --context compute_type=ecs synth emits Cpu 16384 / Memory 65536 / BUILD_VERIFY_TIMEOUT_S 3600. Full build green. (cherry picked from commit 11fb3a5) (cherry picked from commit cd7b8eb) * fix(ecs): map full payload to run_task so channel/build/cedar fields aren't dropped (ABCA-487) The ECS boot command hand-listed ~18 run_task kwargs and silently dropped the rest. On ECS this meant channel_source/channel_metadata never reached run_task, so resolve_linear_api_token never ran, LINEAR_API_TOKEN was never set, and the Linear/Jira reaction + channel MCP silently no-op'd — @bgagent tasks on ECS posted NO 👀 reaction or anything (live-caught: ABCA-487 on the fork, RUNNING with mcp_servers:[] and no reaction). Also dropped: build_command/lint_command (build-gate used defaults), cedar_policies + approval_* (HITL guardrails), base_branch/merge_branches (orchestration stacking), attachments, trace, user_id. Same "hand-rolled partial payload copy" root as #502. Fix — single source of truth, unit-testable: - pipeline.run_task_from_payload(p): maps the WHOLE payload dict to run_task's signature — rename prompt→task_description / model_id→anthropic_model, filter to accepted params (unknown keys ignored, not passed as **kwargs), coerce issue_number/pr_number→str + max_turns→int, aws_region falls back to env. _RUN_TASK_PARAMS computed once at import (patch-safe). - entrypoint exports it; ecs-strategy boot command now calls run_task_from_payload(p) instead of the hand-listed kwargs. - tests: 9 agent tests (rename, channel fields ABCA-487, build/cedar/branch, coercion, unknown-key drop, None-drop, aws_region, signature guard) + ecs-strategy asserts the boot command calls the mapper and no longer hand-lists. Full build green: agent + cdk 2852 + cli 571 + docs. Deployed to dev (--context compute_type=ecs; agent image rebuilt). (cherry picked from commit 777ee0e) (cherry picked from commit 1639c27) * fix(ecs): grant task role GetSecretValue on bgagent-{linear,jira}-oauth-* (ABCA-488) A Linear/Jira-channel task resolves its per-workspace OAuth token from Secrets Manager (bgagent-linear-oauth-<slug>) at startup to fire the 👀→✅ reaction and drive the channel MCP. The AgentCore runtime role + orchestrator/fanout/ screenshot/linear-integration roles all have the bgagent-linear-oauth-* prefix grant, but the ECS task role only had GetSecretValue on the GitHub token. So on ECS the token fetch hit AccessDenied → LINEAR_API_TOKEN unset → linear_reactions skipped AND the Linear MCP stayed needs-auth (agent couldn't post comments either). Live-caught dogfooding on the fork (ABCA-488, ECS task 828dab35). Fourth ECS-parity gap after #502 (payload size) + ABCA-487 (dropped channel fields): the AgentCore path had a capability the ECS task role didn't. Fix: grant the ECS task role secretsmanager:GetSecretValue on the bgagent-linear-oauth-* and bgagent-jira-oauth-* prefixes (GetSecretValue only — the container reads; the orchestrator owns refresh/PutSecretValue). Nag reason updated + regression test asserting the prefix grant. Full build green (2853). (cherry picked from commit 5a886bb) (cherry picked from commit 49235d4) * fix(ecs): wire ARTIFACTS_BUCKET_NAME into the Fargate task (ECS-parity, #299) Live-caught: a :decompose on an ecs-configured repo ran on ECS (fix confirmed — no more 'ECS_CLUSTER_ARN missing') but then FAILED at delivery with 'ValueError: deliver_artifact: ARTIFACTS_BUCKET_NAME is not configured'. The AgentCore runtime sets ARTIFACTS_BUCKET_NAME; the EcsAgentCluster task def never got the bucket — an ECS-parity gap (same class as #502/ABCA-487). A repo-bound artifact workflow (coding/decompose-v1) delivers its plan JSON there. - EcsAgentCluster: new optional artifactsBucket prop → ARTIFACTS_BUCKET_NAME in the container env + grantReadWrite on the task role (read+write: the container DELIVERS the artifact, unlike the read-only payload bucket). IAM5 suppression extended to cover the second scoped S3 grant. - agent.ts: pass traceArtifactsBucket.bucket (same bucket the runtime uses). - 3 construct tests (env injected / read+write grant / omitted when absent). 2872 CDK + 571 CLI + 1184 agent green. Held on branch. (cherry picked from commit 7617d72) (cherry picked from commit e0dfc93) * feat(compute): guard compute_type=ecs against a stack without the ECS substrate A repo onboarded as compute_type=ecs on a stack deployed WITHOUT --context compute_type=ecs fails every task at session start (no ECS_* env vars). Catch the config/deploy mismatch early + make the runtime error actionable: - agent.ts: new ComputeSubstrate CfnOutput — 'ecs' when the gate is on, else 'agentcore' (ecs is additive; the AgentCore runtime is always present, so an agentcore repo works on either). - cli repo onboard: when --compute-type ecs, read ComputeSubstrate and refuse with a redeploy message if it's an explicit non-ecs value. Null (older stacks predating the output) is treated as unknown → proceed (runtime backstop). - ecs-strategy: the missing-env error now names the root cause + remedy (redeploy with the gate, or set the repo to agentcore) instead of a bare env-var list. - Tests: CDK output flips with the gate (agentcore default / ecs gated); CLI onboard refuses/allows/back-commpat; ecs construct unchanged. 2875 CDK + 575 CLI + 1184 agent green. Held on branch. (cherry picked from commit cb7c8c6) (cherry picked from commit 9048db0) * fix(cdk): raise BUILD task memory 64→120 GB (max Fargate) — ABCA-662 baseline OOM ABCA-662's pre-agent baseline build was OOM-killed (exit 137) at 64 GB: dogfooding ABCA-on-ABCA, the full parallel `mise run build` (agent:quality ‖ cdk:build ‖ cli:build ‖ docs:build, each with its own worker fleet) peaked past 64 GB. Each build task is memory-ISOLATED (its own Fargate microVM), so limiting concurrency does NOT help a single over-64 GB build — only more per-task RAM or less build parallelism does. 120 GB is the MAX Fargate admits at 16 vCPU (32–120 in 8 GB steps), so this is the clean experiment: if a build still OOMs here, the only remaining lever is cutting the build's peak parallelism (serialize the mise DAG / cap jest --maxWorkers) — there is no more RAM to give. Pairs with the repo.py fix (an OOM-killed baseline is now infra, not 'already broken'), so even if a build does hit the ceiling the verdict stays honest. Tests updated to assert Memory 122880 (both BUILD-def assertions). (cherry picked from commit 25e2bb1) * fix(cdk): make ecs-strategy top-of-file import hermetic vs ambient env The inline-fallback / no-op tests in the top-of-file describe blocks assume the OPTIONAL vars ECS_PAYLOAD_BUCKET and ECS_PLANNING_TASK_DEFINITION_ARN are ABSENT when the module is first imported (it reads them as module-level constants). A dev shell has neither set, so the suite passed locally — but the REAL ECS agent container HAS ECS_PAYLOAD_BUCKET set (#502 payload bucket), so on ECS the const was truthy and the 'no bucket → inline fallback' + 'no-op' assertions failed: FAIL test/handlers/shared/strategies/ecs-strategy.test.ts ● startSession › sends RunTaskCommand with correct params ... ● deleteEcsPayload without ECS_PAYLOAD_BUCKET › no-ops ... This was the residual fork baseline-build exit-1: 2 failed / 3093 passed, only ever reproducible inside the ECS microVM. Surfaced by the live-streaming build log (the buffered summary had hidden it). Fix: delete both optional vars before the top-of-file import so it's hermetic regardless of the runner's environment; the #502/#299 describe blocks already set them via jest.isolateModules. (cherry picked from commit 7a2bde9) (cherry picked from commit 8267c29) * fix(cdk): grant ec2:DescribeAvailabilityZones to ECS agent task role (cdk synth on fresh clone) A CDK-based target repo's build gate runs `cdk synth`. When the stack is wired to a concrete env ({account, region}), CDK does a synth-time availability-zone context lookup (ec2:DescribeAvailabilityZones). A developer box caches the result in the gitignored cdk.context.json so synth is hermetic; the agent clones fresh, so there is no cache and the live lookup fires. The ECS task role lacked the action, so synth failed with: ERROR ...EcsAgentClusterTaskRole... is not authorized to perform: ec2:DescribeAvailabilityZones ... Synthesis finished with errors → a FALSE build-gate failure on code that builds fine everywhere else. Same ECS-parity class as ABCA-488 (GetSecretValue) and F-2 (CreateEvent): a permission the AgentCore path had but the ECS task role didn't. Live-caught on the ABCA fork: baseline `mise run build` passed all 3095 tests then died at `cdk synth -q`, surfaced only by the live-streaming build log. Grant is a read-only describe (Resource:* — EC2 describe actions have no resource-level scoping; IAM5 suppressed, no mutation/data access). +1 regression test pins the statement. (cherry picked from commit 3fc9060) (cherry picked from commit 2bf12f4) * fix(ecs): grant ECS task role AgentCore Memory access (F-2, ABCA-488-class) Live-caught during the fork stress smoke (ABCA-495): the ECS TaskDefTaskRole got AccessDeniedException on bedrock-agentcore:CreateEvent for the AgentMemory resource, so the agent's cross-task learning writes (write_task_episode / write_repo_learnings) silently no-op'd (WARN) on EVERY ECS task — memory never persisted on the fork's ECS-only substrate. Same class as ABCA-488: the AgentCore runtime role gets memory access via agentMemory.grantReadWrite(runtime) in agent.ts, and the orchestrator gets it too, but the ECS task role never did. Fix: EcsAgentCluster gains an optional agentMemory prop; when provided the task role gets agentMemory.grantReadWrite (read actions + CreateEvent), scoped to the memory ARN (synth-verified: no wildcard resource, so the existing IAM5 nag suppression is unaffected). agent.ts passes agentMemory to the ECS cluster alongside the existing memoryId. Omitted in isolated construct tests / memory-less deploys → no grant (asserted). Tests: construct asserts CreateEvent on the task role scoped to MemoryArn when wired, and NO bedrock-agentcore grant when unwired. Full cdk build green (2883). (cherry picked from commit c23d49fbead30ed52860e3975815e3643b22ade0) (cherry picked from commit 44bb71e) * docs(bootstrap): correct the ECS ComputeTypes enable instructions The //cdk:bootstrap task hint and DEPLOYMENT_ROLES.md both said to enable the ECS compute backend with 'mise //cdk:bootstrap -- --context ComputeTypes=agentcore,ecs'. That does NOT work: ComputeTypes is a CloudFormation *template parameter*, and this CDK version's 'cdk bootstrap' has no way to set one — there's no --parameters flag, and --context sets CDK construct context, not a CFN parameter. So the ECS policy (IaCRole-ABCA-Compute-ECS, conditional on ComputeTypes containing 'ecs') was never attached, and a --context compute_type=ecs deploy then failed with AccessDenied on ecs:* — live-caught deploying the substrate to dev. Correct it to the mechanism that actually works: bootstrap normally, then set the parameter directly on the CDKToolkit stack via 'aws cloudformation update-stack ... ParameterKey=ComputeTypes,ParameterValue=agentcore,ecs' (with a describe-stacks verify). Fixed in the mise task description + comment and both DEPLOYMENT_ROLES.md occurrences; Starlight mirror regenerated. Drift gate green. (cherry picked from commit a46683e) * fix(ecs): remove dead ECS_PAYLOAD_OBJECT_KEY_PREFIX export (review B1) The `export const ECS_PAYLOAD_OBJECT_KEY_PREFIX = ''` constant was referenced nowhere — ecsPayloadKey() (ecs-strategy.ts) builds `<task_id>/payload.json` independently and already documents that layout in its own docstring. The dead export was a "prefix" whose value was '' with a docstring describing a key layout it didn't produce, and it raised the knip dead-code count, tripping the ratchet inside `mise run build`. Delete it; ecsPayloadKey remains the single source of truth for the key layout. * docs+feat: address review nits N1–N4 on the ECS substrate Review follow-ups for #596 (fix itself unchanged; B1 dead-export fixed in the base PR #503): - N1: correct the stale "64 GB / 16 vCPU" comment in agent.ts — the task is larger (64 GB was itself OOM-killed per the construct's sizing history). Defer the exact figure to EcsAgentCluster rather than duplicate/drift it. - N2: reword the memory-grant + oauth-grant comments — an AccessDenied on those paths is LOGGED (memory.py treats it as an infra failure; config.py's token resolver logs it), not "silently" no-op'd. Describe the user-visible effect (no persisted learning / no 👀→✅ reaction) rather than pin a log level that drifts. - N3: the ec2:DescribeAvailabilityZones comment already frames it correctly as a FALSE build-gate failure (not a WARN no-op) — no change needed; noted here for completeness. - N4: run_task_from_payload now emits a WARN when it drops a KNOWN orchestrator key (build_command / merge_branches / base_branch / github_token_secret_arn) that run_task doesn't accept — expected today (consumed elsewhere / not yet wired), but makes a future "wired one side, forgot the other" no-op visible instead of silent (the ABCA-487 class). Foreign keys still drop quietly. + 2 tests (warns on a known dropped key; stays quiet on a foreign key). Full cdk build green (2286) + full agent suite green (1194); eslint/ruff clean. * style: ruff-format the N4 payload-key block (fix build-mutation failure) CI's "Fail build on mutation" tripped: I ran `ruff check` (passed) but not `ruff format`, so the `_KNOWN_ORCHESTRATOR_KEYS = frozenset({...})` set literal and the two-line `log(...)` call weren't in ruff's canonical multi-line form. `ruff format` normalizes them; no behavior change (12 payload tests still pass). * fix+docs: WARN on dropped task_started_at (HITL parity) + defensive max_turns + comment nits Follow-up on the #596 re-review (code was already approved-quality; these are the new non-blocking finding + 3 nits): - task_started_at HITL parity: 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 never sets it, so run_task_from_payload dropped it SILENTLY — a fail-open AgentCore↔ECS HITL divergence. Added task_started_at to _KNOWN_ORCHESTRATOR_KEYS so the drop now WARNs (surfaces the gap). Fully wiring TASK_STARTED_AT into the ECS containerEnv is a tracked follow-up; this makes the divergence visible today. - max_turns defensive coercion: a malformed max_turns raised ValueError mid-boot while every other field defaults defensively. Now drops it (run_task default applies) with a WARN, matching the surrounding style. - Nit: artifactsBucket JSDoc no longer claims this bucket is the `--trace` upload target (it wires ARTIFACTS_BUCKET_NAME only; the trace uploader reads TRACE_ARTIFACTS_BUCKET_NAME, unset here — noted as a separate ECS-parity gap). - Nit: dropped `2>/dev/null` from the Dockerfile corepack prepare so the prepare-failed diagnostic isn't hidden (the `|| corepack enable` fallback still guarantees resolvable shims — no regression to the exit-127 inert gate). Tests: +3 (task_started_at WARN, malformed-max_turns dropped-not-raised, valid max_turns still coerced). Full agent suite green (1195); cdk tsc + construct tests green; ruff format + eslint clean. * fix(ecs): address #596 review — drop over-privileged artifacts grant + nits B1 (least-privilege regression): remove props.artifactsBucket.grantReadWrite from the ECS task role. coding/decompose-v1 delivers its plan via 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). The whole-bucket grant over-privileged the untrusted-code role and broke cross-task isolation (a task could read/clobber other tasks' artifacts/<other_id>/, traces/, attachments/). Task role keeps only the ARTIFACTS_BUCKET_NAME env. Correct the inverted 'parity' comment and drop the now-stale artifacts clause from the cdk-nag IAM5 reason. Test: flip 'grants READ+WRITE on artifacts' → asserts the task role has NO S3 Put/Delete action at all (the read-only #502 payload GetObject*/List* grant is not flagged). N4: add lint_command to _KNOWN_ORCHESTRATOR_KEYS (sibling of build_command) so a future 'wired build_command, forgot lint_command' contract gap WARNs instead of dropping silently. (N1/N2/N3 comment nits were already addressed on the branch head; B1 dead-const ECS_PAYLOAD_OBJECT_KEY_PREFIX no longer present. B2 governance handled separately.) * fix(ecs): address #596 re-review nits — stale parity comments + WARN noise + max_turns coercion Scott's fresh re-review (head 86ab249) is Approve-with-nits: B1 (over-privilege) and B2 (governance) resolved; remaining asks are doc-accuracy + WARN-channel hygiene. - N1: rewrite the artifactsBucket JSDoc (ecs-agent-cluster.ts) that still claimed a read/write task-role grant the B1 fix deliberately removed — now states env-only wiring with delivery through the task_id-scoped SessionRole (parity with the AgentCore runtime role, which has no artifacts grant). The grant block ~30 lines below was already corrected in the B1 commit; this was its residual half. - N2: same inverted-comment fix at the agent.ts wiring site — '(read+write grant in the construct)' → env-only + SessionRole delivery. - N3: drop github_token_secret_arn from _KNOWN_ORCHESTRATOR_KEYS. It is ALWAYS present and ALWAYS resolved via the GITHUB_TOKEN_SECRET_ARN env, so listing it fired the known-key WARN on 100% of ECS boots — pure noise diluting the channel meant for genuine future contract gaps. Now falls through as a quiet foreign-key drop; comment records why it must not be re-added. +test asserting no WARN. - N4: max_turns int() coercion accepted a bool (int(True)==1) and truncated a non-integral float (int(3.9)==3), and the comment falsely claimed it 'matches how every other field is handled' (it is the one non-str coercion). Now rejects bools + non-integral floats with a breadcrumb; valid int / int-string / int-float still pass. +test. Reachable only via a corrupt payload (orchestrator emits a real int), so cosmetic — bundled since cheap. - N5: no-op — the Dockerfile corepack line already has no 2>/dev/null. Gates: cdk 2355 tests + eslint + synth green; agent 1268 tests + ruff + ty green. Non-blocking #608 test-seam gaps (finalize deleteEcsPayload, boot-uri parse, entrypoint re-export, zero-ECS-synth assert) remain tracked, not in scope here. --------- Co-authored-by: bgagent <bgagent@noreply.github.com> Co-authored-by: Alain Krok <alkrok@amazon.com>
1 parent 30d7b36 commit 238eb30

15 files changed

Lines changed: 916 additions & 86 deletions

File tree

agent/Dockerfile

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ RUN curl -fsSL https://deb.nodesource.com/setup_24.x | bash - && \
4444
apt-get clean && \
4545
rm -rf /var/lib/apt/lists/* /var/cache/apt/archives/*
4646

47+
# Enable Corepack so ``yarn`` / ``pnpm`` resolve out of the box. Many target
48+
# repos (including ABCA itself) drive installs with ``yarn install`` via their
49+
# build command; without this, ``yarn`` is "command not found" (exit 127) and
50+
# the build-verification GATE runs inert — a green build is reported for a repo
51+
# we never actually built (live-caught 2026-06-29 dogfooding ABCA-on-ABCA: the
52+
# agent had to hand-build a ``~/bin/yarn`` shim every run). Corepack ships with
53+
# Node 24; ``enable`` installs the yarn/pnpm shims onto PATH.
54+
RUN corepack enable && corepack prepare yarn@stable --activate || corepack enable
55+
4756
# Install Claude Code CLI (the Python SDK requires this binary)
4857
# Then update known vulnerable transitive packages where fixed versions exist.
4958
# Pinned 2.1.191 to match the CLI bundled by claude-agent-sdk 0.2.110 (see

agent/src/entrypoint.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
TaskResult,
3232
TokenUsage,
3333
)
34-
from pipeline import main, run_task # noqa: F401
34+
from pipeline import main, run_task, run_task_from_payload # noqa: F401
3535
from post_hooks import ( # noqa: F401
3636
ensure_committed,
3737
ensure_pr,

agent/src/pipeline.py

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import asyncio
66
import hashlib
7+
import inspect
78
import os
89
import sys
910
import time
@@ -1287,6 +1288,129 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
12871288
raise
12881289

12891290

1291+
#: Orchestrator payload keys that map to a differently-named ``run_task`` kwarg.
1292+
#: The orchestrator emits ``prompt``/``model_id``; ``run_task`` calls them
1293+
#: ``task_description``/``anthropic_model``. Everything else is a 1:1 name match.
1294+
_PAYLOAD_KEY_ALIASES = {
1295+
"prompt": "task_description",
1296+
"model_id": "anthropic_model",
1297+
}
1298+
1299+
#: ``run_task`` kwargs that must be coerced to ``str`` — the orchestrator may
1300+
#: emit them as numbers (issue_number, pr_number) and ``run_task`` types them as
1301+
#: strings. ``max_turns`` is coerced to int. Absent keys are left to the
1302+
#: ``run_task`` defaults.
1303+
_PAYLOAD_STR_KEYS = frozenset({"issue_number", "pr_number"})
1304+
1305+
#: Parameter names ``run_task`` accepts — computed once at import from the REAL
1306+
#: signature (not inside the function, so patching ``run_task`` in tests can't
1307+
#: shadow it). Any payload key not in this set is ignored, never passed through.
1308+
_RUN_TASK_PARAMS = frozenset(inspect.signature(run_task).parameters)
1309+
1310+
#: Orchestrator payload keys we KNOW about that ``run_task`` does not (yet)
1311+
#: accept as a parameter. Dropping one of these is expected today (e.g.
1312+
#: ``base_branch`` is consumed via ``hydrated_context``, not a run_task kwarg;
1313+
#: ``github_token_secret_arn`` is resolved before this call), but a key that
1314+
#: shows up here AND is silently dropped is exactly the "wired one side of an
1315+
#: orchestrator→agent field, forgot the other" no-op that ABCA-487 was — so we
1316+
#: WARN when we drop one, making a future contract gap visible instead of silent.
1317+
#: Keys not in this set (genuinely foreign) are dropped quietly as before.
1318+
_KNOWN_ORCHESTRATOR_KEYS = frozenset(
1319+
{
1320+
"build_command",
1321+
# ``lint_command``'s sibling: neither is a run_task param today (the build/
1322+
# lint commands are consumed via repo config, not passed through here), but
1323+
# listing both makes a future "wired build_command, forgot lint_command"
1324+
# contract gap WARN instead of drop silently (N4).
1325+
"lint_command",
1326+
"merge_branches",
1327+
"base_branch",
1328+
# NB: ``github_token_secret_arn`` is deliberately NOT listed (N3). It is
1329+
# ALWAYS in the payload and ALWAYS dropped here (resolved via the
1330+
# ``GITHUB_TOKEN_SECRET_ARN`` env in build_config, never a run_task
1331+
# param), so listing it would fire the known-key WARN on 100% of ECS
1332+
# boots — pure noise that dilutes the channel meant to surface genuine
1333+
# future "wired one side, forgot the other" gaps. It falls through as a
1334+
# quiet foreign-key drop instead.
1335+
# AgentCore's server.py exports task_started_at as TASK_STARTED_AT, which
1336+
# hooks._remaining_maxlifetime_s() uses to clip the Cedar HITL approval-gate
1337+
# maxLifetime. The ECS boot path bypasses server.py and does not (yet) set
1338+
# that env, so this key is dropped here — a silent AgentCore↔ECS HITL
1339+
# divergence (fail-open: the clip returns None, gate uses the task default).
1340+
# Listing it makes the drop WARN so the parity gap is visible until the ECS
1341+
# strategy sets TASK_STARTED_AT in containerEnv (tracked as a follow-up).
1342+
"task_started_at",
1343+
}
1344+
)
1345+
1346+
1347+
def run_task_from_payload(payload: dict) -> dict:
1348+
"""Invoke :func:`run_task` from a full orchestrator payload dict.
1349+
1350+
The ECS compute path (``ecs-strategy.ts``) hands the agent the *entire*
1351+
orchestrator payload (via the #502 S3 pointer). Previously the ECS boot
1352+
command hand-listed a subset of ``run_task`` kwargs and silently dropped the
1353+
rest — most visibly ``channel_source``/``channel_metadata`` (no Linear/Jira
1354+
reactions or channel MCP on ECS — ABCA-487), plus ``build_command``,
1355+
``cedar_policies``, ``base_branch``/``merge_branches``, ``attachments``, etc.
1356+
1357+
This maps the payload to ``run_task``'s real signature so no field can be
1358+
silently dropped again: rename the aliased keys, filter to parameters
1359+
``run_task`` actually accepts (unknown keys are ignored, not passed as
1360+
``**kwargs`` which ``run_task`` doesn't accept), and coerce the str/int
1361+
fields the orchestrator may emit as numbers. ``aws_region`` falls back to the
1362+
``AWS_REGION`` env var when the payload omits it (the boot command used to
1363+
supply this explicitly).
1364+
1365+
Single source of truth + unit-testable, replacing the untestable inline
1366+
Python string that already drifted once.
1367+
"""
1368+
kwargs: dict = {}
1369+
for key, value in (payload or {}).items():
1370+
target = _PAYLOAD_KEY_ALIASES.get(key, key)
1371+
if target not in _RUN_TASK_PARAMS:
1372+
# Not a run_task parameter — ignore. A KNOWN orchestrator key being
1373+
# dropped is expected today but worth a breadcrumb: if run_task ever
1374+
# grows a matching param, this WARN is where a "forgot to wire it
1375+
# through" no-op surfaces (N4 / ABCA-487 class). Foreign keys are
1376+
# dropped quietly.
1377+
if key in _KNOWN_ORCHESTRATOR_KEYS and value is not None:
1378+
log(
1379+
"WARN",
1380+
f"run_task_from_payload: dropping known orchestrator key '{key}' "
1381+
f"(not a run_task parameter) — consumed elsewhere or not yet wired",
1382+
)
1383+
continue
1384+
if value is None:
1385+
continue # let run_task's default apply
1386+
if target in _PAYLOAD_STR_KEYS:
1387+
value = str(value)
1388+
elif target == "max_turns":
1389+
# Defensive: a malformed max_turns must not crash the whole boot —
1390+
# drop it and let run_task's default apply (with a breadcrumb) rather
1391+
# than raise. Unlike the str keys above, this is the one field with a
1392+
# non-str coercion, so it also guards the surprising int() cases the
1393+
# orchestrator never emits but a hand-edited payload might: a bool
1394+
# (``int(True) == 1``) and a non-integral float (``int(3.9) == 3``)
1395+
# would both silently become a bogus turn count (N4).
1396+
if isinstance(value, bool) or not isinstance(value, (int, float, str)):
1397+
log("WARN", f"run_task_from_payload: ignoring non-integer max_turns {value!r}")
1398+
continue
1399+
try:
1400+
coerced = int(value)
1401+
except (TypeError, ValueError):
1402+
log("WARN", f"run_task_from_payload: ignoring non-integer max_turns {value!r}")
1403+
continue
1404+
if isinstance(value, float) and coerced != value:
1405+
log("WARN", f"run_task_from_payload: ignoring non-integral max_turns {value!r}")
1406+
continue
1407+
value = coerced
1408+
kwargs[target] = value
1409+
1410+
kwargs.setdefault("aws_region", os.environ.get("AWS_REGION", ""))
1411+
return run_task(**kwargs)
1412+
1413+
12901414
def main():
12911415
config = get_config()
12921416

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
"""Unit tests for pipeline.run_task_from_payload — the ECS payload→run_task map.
2+
3+
Regression cover for ABCA-487: the ECS boot command used to hand-list a subset
4+
of run_task kwargs and silently dropped channel_source/channel_metadata (no
5+
Linear/Jira reactions on ECS), build_command, cedar_policies, base_branch, etc.
6+
run_task_from_payload maps the WHOLE payload so nothing is dropped again.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
from unittest.mock import patch
12+
13+
from pipeline import _RUN_TASK_PARAMS, run_task_from_payload
14+
15+
16+
def _capture(payload: dict) -> dict:
17+
"""Run the mapper with run_task replaced by a capturing stub; return kwargs."""
18+
seen: dict = {}
19+
20+
def fake_run_task(**kwargs):
21+
seen.update(kwargs)
22+
return {"status": "success"}
23+
24+
with patch("pipeline.run_task", side_effect=fake_run_task):
25+
run_task_from_payload(payload)
26+
return seen
27+
28+
29+
class TestRunTaskFromPayload:
30+
def test_renames_prompt_and_model_id(self):
31+
seen = _capture({"prompt": "do the thing", "model_id": "anthropic.claude-x"})
32+
assert seen["task_description"] == "do the thing"
33+
assert seen["anthropic_model"] == "anthropic.claude-x"
34+
# The original payload keys must NOT leak through as-is (run_task rejects them).
35+
assert "prompt" not in seen
36+
assert "model_id" not in seen
37+
38+
def test_forwards_channel_fields_ABCA_487(self):
39+
# THE regression: channel_source/channel_metadata must reach run_task so
40+
# the Linear/Jira reaction + channel MCP fire on ECS.
41+
cm = {"linear_issue_id": "iss-1", "linear_oauth_secret_arn": "arn:sm:...:lin"}
42+
seen = _capture({"channel_source": "linear", "channel_metadata": cm})
43+
assert seen["channel_source"] == "linear"
44+
assert seen["channel_metadata"] == cm
45+
46+
def test_forwards_cedar_attachments_trace_user_fields(self):
47+
# HITL guardrails (cedar_policies, approval_*), attachments, trace, and
48+
# user_id are real run_task params here — they must reach run_task on ECS
49+
# (the hand-listed boot command used to drop them).
50+
seen = _capture(
51+
{
52+
"cedar_policies": ["p1", "p2"],
53+
"attachments": [{"filename": "x.png"}],
54+
"trace": True,
55+
"user_id": "user-9",
56+
}
57+
)
58+
assert seen["cedar_policies"] == ["p1", "p2"]
59+
assert seen["attachments"] == [{"filename": "x.png"}]
60+
assert seen["trace"] is True
61+
assert seen["user_id"] == "user-9"
62+
63+
def test_drops_payload_keys_that_are_not_yet_run_task_params(self):
64+
# build_command/lint_command (configurable verify, #1) and base_branch/
65+
# merge_branches (orchestration stacking, #247) are emitted by the
66+
# orchestrator but are NOT run_task parameters on this branch. The mapper
67+
# filters against run_task's REAL signature, so they are dropped rather
68+
# than smuggled through as an invalid kwarg. When those params land,
69+
# they forward automatically with no change here — that's the point of
70+
# keying off inspect.signature instead of a hand-list.
71+
seen = _capture(
72+
{
73+
"repo_url": "org/repo",
74+
"build_command": "npm ci && npm test",
75+
"lint_command": "npm run lint",
76+
"base_branch": "epic-tip",
77+
"merge_branches": ["a", "b"],
78+
}
79+
)
80+
assert seen["repo_url"] == "org/repo"
81+
for not_yet in ("build_command", "lint_command", "base_branch", "merge_branches"):
82+
assert not_yet not in seen
83+
84+
def test_coerces_issue_and_pr_number_to_str_and_max_turns_to_int(self):
85+
seen = _capture({"issue_number": 42, "pr_number": 7, "max_turns": "50"})
86+
assert seen["issue_number"] == "42"
87+
assert seen["pr_number"] == "7"
88+
assert seen["max_turns"] == 50
89+
assert isinstance(seen["max_turns"], int)
90+
91+
def test_ignores_unknown_payload_keys(self):
92+
# github_token_secret_arn is on the payload but is NOT a run_task param
93+
# (it's consumed platform-side); passing it as **kwargs would TypeError.
94+
seen = _capture(
95+
{
96+
"repo_url": "org/repo",
97+
"github_token_secret_arn": "arn:...",
98+
"sources": ["x"],
99+
}
100+
)
101+
assert seen["repo_url"] == "org/repo"
102+
assert "github_token_secret_arn" not in seen
103+
assert "sources" not in seen
104+
105+
def test_github_token_secret_arn_dropped_quietly(self):
106+
# N3: github_token_secret_arn is ALWAYS present and ALWAYS resolved via
107+
# the GITHUB_TOKEN_SECRET_ARN env (never a run_task param), so its drop is
108+
# 100% expected and must NOT fire the known-key WARN — that channel is for
109+
# genuine future contract gaps, not this always-dropped key.
110+
logs: list[tuple[str, str]] = []
111+
with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))):
112+
_capture({"github_token_secret_arn": "arn:aws:secretsmanager:...", "repo_url": "r"})
113+
assert not [m for level, m in logs if "github_token_secret_arn" in m], (
114+
"github_token_secret_arn must drop quietly (N3) — it is always resolved via env"
115+
)
116+
117+
def test_drops_none_values_so_run_task_defaults_apply(self):
118+
seen = _capture({"repo_url": "org/repo", "base_branch": None, "channel_metadata": None})
119+
assert "base_branch" not in seen
120+
assert "channel_metadata" not in seen
121+
122+
def test_aws_region_falls_back_to_env(self, monkeypatch):
123+
monkeypatch.setenv("AWS_REGION", "us-east-1")
124+
seen = _capture({"repo_url": "org/repo"})
125+
assert seen["aws_region"] == "us-east-1"
126+
127+
def test_explicit_aws_region_in_payload_wins(self, monkeypatch):
128+
monkeypatch.setenv("AWS_REGION", "us-east-1")
129+
seen = _capture({"repo_url": "org/repo", "aws_region": "eu-west-1"})
130+
assert seen["aws_region"] == "eu-west-1"
131+
132+
def test_every_forwarded_key_is_a_real_run_task_param(self):
133+
# Guard: whatever the mapper forwards must be accepted by run_task, so a
134+
# future payload key can never smuggle an invalid kwarg through. Compare
135+
# against the module's real param set (run_task is patched in _capture).
136+
accepted = _RUN_TASK_PARAMS
137+
seen = _capture(
138+
{
139+
"prompt": "p",
140+
"model_id": "m",
141+
"repo_url": "r",
142+
"issue_number": 1,
143+
"channel_source": "linear",
144+
"channel_metadata": {"a": "b"},
145+
"build_command": "b",
146+
"cedar_policies": ["c"],
147+
"base_branch": "x",
148+
"attachments": [{}],
149+
"trace": False,
150+
"user_id": "u",
151+
"pr_number": 3,
152+
"hydrated_context": {"k": "v"},
153+
"resolved_workflow": {"id": "w"},
154+
}
155+
)
156+
assert set(seen).issubset(accepted)
157+
158+
def test_max_turns_rejects_surprising_inputs(self):
159+
# N4: int() accepts a bool (int(True)==1) and truncates a float
160+
# (int(3.9)==3). The orchestrator always emits a real int, but a corrupt
161+
# / hand-edited payload must not silently become a bogus turn count —
162+
# drop with a breadcrumb and let run_task's default apply.
163+
assert "max_turns" not in _capture({"repo_url": "r", "max_turns": True})
164+
assert "max_turns" not in _capture({"repo_url": "r", "max_turns": 3.9})
165+
# Valid inputs still pass: a real int and an int-valued string / float.
166+
assert _capture({"repo_url": "r", "max_turns": 50})["max_turns"] == 50
167+
assert _capture({"repo_url": "r", "max_turns": "50"})["max_turns"] == 50
168+
assert _capture({"repo_url": "r", "max_turns": 50.0})["max_turns"] == 50
169+
170+
def test_warns_when_dropping_a_known_orchestrator_key(self):
171+
# N4: a KNOWN orchestrator key that run_task doesn't accept is dropped
172+
# (expected today) but logged, so a future "wired one side, forgot the
173+
# other" contract gap (the ABCA-487 class) is visible, not silent.
174+
logs: list[tuple[str, str]] = []
175+
with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))):
176+
_capture({"build_command": "mise run build", "repo_url": "r"})
177+
assert [m for level, m in logs if level == "WARN" and "build_command" in m], (
178+
"expected a WARN when dropping the known orchestrator key build_command"
179+
)
180+
181+
def test_does_NOT_warn_when_dropping_a_foreign_key(self):
182+
# A genuinely-foreign key (not a known orchestrator field) is dropped
183+
# quietly — no log noise for keys we never expected to forward.
184+
logs: list[tuple[str, str]] = []
185+
with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))):
186+
_capture({"some_future_unrelated_key": "v", "repo_url": "r"})
187+
assert not [m for level, m in logs if "some_future_unrelated_key" in m]
188+
189+
def test_warns_when_dropping_task_started_at_HITL_parity(self):
190+
# task_started_at drives the AgentCore HITL maxLifetime clip via
191+
# TASK_STARTED_AT; the ECS path doesn't set it yet, so its drop must WARN
192+
# (surface the AgentCore↔ECS parity gap, not silently fail-open).
193+
logs: list[tuple[str, str]] = []
194+
with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))):
195+
_capture({"task_started_at": "2026-07-14T00:00:00Z", "repo_url": "r"})
196+
assert [m for level, m in logs if level == "WARN" and "task_started_at" in m]
197+
198+
def test_malformed_max_turns_is_dropped_not_raised(self):
199+
# A non-integer max_turns must not crash the boot — it's dropped (run_task
200+
# default applies) with a WARN, matching how every other field defaults.
201+
logs: list[tuple[str, str]] = []
202+
with patch("pipeline.log", side_effect=lambda level, msg, **kw: logs.append((level, msg))):
203+
seen = _capture({"repo_url": "r", "max_turns": "not-a-number"})
204+
assert "max_turns" not in seen
205+
assert [m for level, m in logs if level == "WARN" and "max_turns" in m]
206+
207+
def test_valid_max_turns_still_coerced_to_int(self):
208+
seen = _capture({"repo_url": "r", "max_turns": "50"})
209+
assert seen["max_turns"] == 50
210+
assert isinstance(seen["max_turns"], int)

cdk/mise.toml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,17 @@ run = ["mkdir -p $TMPDIR", "yarn synth:quiet"]
4040
description = "cdk deploy (pass args after --)"
4141
run = "npx cdk deploy"
4242

43+
# Bootstraps with ComputeTypes=agentcore (the template default). To ALSO enable the
44+
# ECS compute backend you must set the ComputeTypes CFN *parameter* — `cdk bootstrap`
45+
# has no way to pass template parameters (no --parameters flag; --context sets CDK
46+
# construct context, NOT a CFN parameter), so run this task once, then set the
47+
# parameter directly on the CDKToolkit stack:
48+
# aws cloudformation update-stack --stack-name CDKToolkit --use-previous-template \
49+
# --capabilities CAPABILITY_NAMED_IAM \
50+
# --parameters ParameterKey=ComputeTypes,ParameterValue=agentcore\,ecs
51+
# # verify: aws cloudformation describe-stacks --stack-name CDKToolkit --query 'Stacks[0].Parameters'
4352
[tasks.bootstrap]
44-
description = "Bootstrap CDK with least-privilege policies (pass -- --context ComputeTypes=agentcore,ecs for ECS)"
53+
description = "Bootstrap CDK with least-privilege policies (ComputeTypes=agentcore; for ECS set the ComputeTypes CFN param on CDKToolkit — see comment above the task)"
4554
depends = [":install", ":bootstrap:generate"]
4655
run = "npx cdk bootstrap --template bootstrap/bootstrap-template.yaml"
4756

0 commit comments

Comments
 (0)