fix(ecs): write task payload to S3 instead of 8 KB containerOverrides (#502)

[isadeks]

Summary

Fixes #502 — the ECS/Fargate compute strategy rejected every real task with:

InvalidParameterException: Container Overrides length must be at most 8192

Root cause: ecs-strategy.ts inlined the entire 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 the call failed before the container started. AgentCore is unaffected — it passes the payload in the InvokeAgentRuntime request body, which has no comparable limit. The earlier ECS smoke test (#494, a small Rust cargo check) had a payload that fit under 8 KB, so it didn't surface this.

Fix — pass a pointer, not the payload

Piece	Change
EcsPayloadBucket (new)	Dedicated S3 bucket, mirrors TraceArtifactsBucket: BLOCK_ALL + enforceSSL + S3_MANAGED, 1-day lifecycle TTL. Dedicated (not co-tenant with attachments/traces) so the task role's read is scoped to payloads only.
ecs-strategy	PutObject payload → <task_id>/payload.json; pass AGENT_PAYLOAD_S3_URI in the override; boot command fetches via boto3. Inline AGENT_PAYLOAD kept as fallback (small payloads / no bucket) — no regression. deleteEcsPayload helper.
orchestrate-task finalize	Best-effort deleteEcsPayload for ECS tasks on terminal; the 1-day TTL is the crash backstop.
EcsAgentCluster	Accept payloadBucket, inject ECS_PAYLOAD_BUCKET, grant task role read-only (untrusted repo code must not write/delete; the trusted orchestrator owns write+delete). Session-role-aware.
task-orchestrator	ecsPayloadBucket prop → grantPut + grantDelete; @aws-sdk/client-s3 added to bundling externals.
agent.ts	Updated the commented uncomment-to-enable ECS scaffolding to wire the payload bucket.

Security stance

• ECS task role: read-only on the payload bucket (the container runs untrusted repo code).
• Orchestrator: write + delete (trusted).
• Bucket is private (BLOCK_ALL), TLS-enforced, encrypted; payloads expire in 1 day.

Testing

• New ecs-payload-bucket.test.ts: TTL=1d, SSL-only, block-public, autoDelete.
• ecs-strategy.test.ts: S3 write + AGENT_PAYLOAD_S3_URI pointer (no inline blob); inline fallback when no bucket; boot command fetch-from-S3-with-fallback; deleteEcsPayload delete + best-effort swallow + no-op without bucket.
• ecs-agent-cluster.test.ts: ECS_PAYLOAD_BUCKET env, task role read-only (asserts no s3:Put*/s3:Delete*), omitted when no bucket.
• Full mise run build green (cdk tests + agent + cli + docs + synth + lint).

Live verification (dev, ECS wired)

Deployed to a dev stack with --context compute_type=ecs and fired a real fork task:

• compute_type=ecs, session_id = a real ECS task ARN → RunTask succeeded (the prior tasks died here with InvalidParameterException).
• Payload object written to S3: payload.json = 8455 bytes — above the 8192-byte containerOverrides cap, i.e. exactly the payload that would have failed inline.
• Container log: Using hydrated context from orchestrator, then cloned/branched/ran the build — the agent received and parsed the full payload via the S3 pointer.
• Payload bucket live with the 1-day TTL; orchestrator has s3:PutObject/DeleteObject; ECS task role read-only.

Notes / scope

• ECS remains disabled by default on main (the agent.ts wiring is the existing commented uncomment-to-enable scaffolding). This PR fixes the latent strategy bug + plumbing; it does not flip ECS on. Complementary to fix: two bugs that prevent ECS Fargate from working #494.

Closes #502

🤖 Generated with Claude Code

[scottschreckengaust]

Review — Principal Architect (Stack A: ECS substrate, PR 1/4 · reviewed bottom-up)

Base verified against fresh origin/main. Bootstrap ADR-002 coverage independently verified for the stack. All mandatory agents run (see the stack-tip review on #596 for the full agent table).

Verdict: Approve ✅

Clean solution to the 8 KB RunTask containerOverrides limit. A dedicated ephemeral payload bucket (1-day TTL, BLOCK_ALL, enforceSSL, S3_MANAGED, abort-incomplete-MPU) with a read-only task-role grant — and an explicit test asserting the task role gets s3:GetObject* but never s3:Put*/s3:Delete*. The orchestrator gets write+delete; deleteEcsPayload is best-effort with the lifecycle rule as the backstop. Stack wiring in agent.ts stays commented out here, so this PR synthesizes no new resources on its own — the deploy-time surface only appears in #596 (where I verified bootstrap coverage: the auto-named bucket falls under arn:aws:s3:::backgroundagent-dev-*, no AccessDenied gap).

Governance: fully compliant — branch fix/502-ecs-payload-s3-pointer, issue #502 carries the approved label.

Non-blocking (but fix it in the stack — it bites once wired)

• Dead, self-contradicting export. cdk/src/constructs/ecs-payload-bucket.ts — export const ECS_PAYLOAD_OBJECT_KEY_PREFIX = '' is referenced nowhere (verified: only its own declaration; ecsPayloadKey() builds the key independently, and the test imports ECS_PAYLOAD_TTL_DAYS, not this). It's a "prefix" whose value is '' with a docstring describing a key layout it doesn't produce. Under the root knip.json (project: ["src/**/*.ts!"], production-only) it raises the knip count 78→79, and scripts/check-deadcode-ratchet.mjs process.exit(1)s above baseline. The CI "Dead-code detection" job is continue-on-error / advisory (won't fail the check), but the same ratchet runs inside mise run build, which this repo requires green before commit — so it breaks the required build. Introduced here, reachable once building. Fix: delete the constant + its doc block; fold the key-layout note into ecsPayloadKey()'s docstring (the real source of truth). Flagged as a blocking item on #596 too, since that's where the wiring lands — fix once, anywhere in the stack.

Tests

ecs-payload-bucket.test.ts is comprehensive (encryption, TLS-only policy, TTL, abort-MPU, DESTROY/autoDelete + custom-props override). ecs-strategy.test.ts covers the S3-pointer path (via jest.isolateModules), the inline fallback, and deleteEcsPayload delete + swallow-errors + no-op-without-bucket. One stack-level test-seam gap (verified in the #596 review): deleteEcsPayload is tested in isolation but nothing asserts the orchestrator's finalize step calls it only for compute_type==='ecs', and task-orchestrator.test.ts asserts neither the ECS_PAYLOAD_BUCKET env nor the grantDelete. Non-blocking follow-up.

Bootstrap synth-coverage: N/A for this PR (no resources synthesized until #596 flips the gate).

_{🤖 Generated with Claude Code}

[isadeks]

Addressed the dead-export nit in ce93b62: deleted ECS_PAYLOAD_OBJECT_KEY_PREFIX (the '' "prefix" whose docstring described a layout it didn't produce); ecsPayloadKey() in ecs-strategy.ts remains the single source of truth for <task_id>/payload.json. This clears the knip finding this PR introduced.

One thing worth flagging that the review's delta-based check didn't surface: the dead-code ratchet is already failing on main itself (count 81 vs committed baseline 78 — three exports drifted unused via other merges), independent of this stack. So mise run build is red on main today regardless of this PR. This stack no longer adds to the count, but it can't get the ratchet green on its own. Happy to file a separate cleanup issue to remove main's 3 drifted exports (or re-baseline) — didn't want to fold an unrelated main cleanup into this stack. Lmk your preference.

[isadeks]

Filed #607 for the pre-existing dead-code ratchet failure on main (count 81 > baseline 78) — it's a repo-wide mise run build breakage independent of this stack, so tracking + fixing it separately rather than in this PR.

[scottschreckengaust]

Verdict: ✅ Approve

Clean, well-scoped bug fix for #502 (RunTask rejecting every ECS task at the 8 KB containerOverrides cap). Stashes the payload in a dedicated S3 bucket and passes an AGENT_PAYLOAD_S3_URI pointer, with an inline fallback for small payloads. ECS stays inert on main — the agent.ts scaffolding remains commented — so this fixes the latent strategy bug and plumbing without flipping the backend on. Governance is clean: #502 carries approved, branch fix/502-ecs-payload-s3-pointer conforms, CI green, MERGEABLE.

Vision alignment

Advances bounded blast radius: dedicated bucket (BLOCK_ALL + enforceSSL + S3_MANAGED), 1-day TTL, task role read-only (untrusted repo code can't delete other tasks' payloads), orchestrator owns write+delete. Correctly separated trust boundaries.

Bootstrap coverage — not required (verified)

EcsPayloadBucket is an AWS::S3::Bucket with a CDK-generated backgroundagent-dev-* name, already covered by observability.ts S3ApplicationBuckets (CreateBucket / PutBucketPolicy / PutLifecycleConfiguration / PutEncryptionConfiguration / PutBucketPublicAccessBlock all present) and resource-action-map.ts:93. It introduces no new CFN type, and it's ECS-gated so it never reaches synth-coverage.test.ts (default-context synth). No BOOTSTRAP_VERSION bump or golden-baseline change needed. Same pattern as the existing TraceArtifactsBucket/AttachmentsBucket.

Non-blocking nits (follow-ups; several already tracked in #608)

1. ecs-strategy.ts large-payload branch — when ECS_PAYLOAD_BUCKET is unset and the payload exceeds INLINE_PAYLOAD_WARN_BYTES, it logs a WARN then proceeds into the inline path, where RunTask still throws the raw InvalidParameterException. The actionable cause isn't attached to the task's terminal error. This state is unreachable via the supported deploy (bucket + cluster are co-gated in #596), so it's latent — but consider failing fast with the cause+remedy instead of WARN-then-proceed if inline stays a supported mode.
2. Test seams (non-blocking): the large-payload WARN branch and the orchestrate-task.ts finalize deleteEcsPayload call (fires only for compute_type==='ecs') are untested; the boot-command S3-fetch one-liner is asserted only as a string. Tracked in #608.

Review agents run

• code-reviewer, silent-failure-hunter, pr-test-analyzer, comment-analyzer ✅ — no blocking defects. deleteEcsPayload best-effort swallow is correct (post-terminal, logged, 1-day lifecycle backstop). S3 URI parse (_uri.split("/",3)[2]/[3]) is correct for s3://bucket/key and injection-safe (task_id is a ULID).
• Bootstrap ADR-002 coverage ✅ (see above).
• type-design-analyzer ⏭️ — no new exported types of substance (props interfaces consumed in-file).

_{🤖 Reviewed with Claude Code}