Commit d3923ad
fix(agent): stop scoped-session S3 hang in baseline build + reap hung suites + early ACK (#616)
* 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(errors): classify a build/verify command TIMEOUT (live-caught ABCA-667, was 'Unexpected error')
Replication finding: a live fork coding child (ABCA-667) failed with
'TimeoutExpired: Command [mise run build] timed out after 600 seconds' — the full
~2800-test fork build exceeded the 600s cap. That's the pre-agent verify-build
subprocess timeout, which lands raw in error_message and fell through to
'Unexpected error' + generic guidance — exactly the transient-vs-real confusion
this rework targets. It's not a crash and not broken code: the build ran too long.
Add a TIMEOUT classification for /
→ 'Build/tests didn't finish in time', user class,
retryable, remedy names the real fix (retry / raise BUILD_VERIFY_TIMEOUT_S or move
to the larger ECS build box). Ordered before the generic poll-timeout pattern.
Test uses the exact ABCA-667 shape. 60 classifier tests green.
(cherry picked from commit 6394a59)
(cherry picked from commit b9c1a94)
* fix(agent): ensure claude-code native binary is placed at image build (Exec format error)
claude-code@2.1.191 uses a shim (bin/claude.exe) + platform-optional-dependency
install model: its postinstall (install.cjs) is supposed to replace the shim
with the platform-native ELF binary. When that postinstall silently falls back
(e.g. it hits EACCES overwriting the shim under npm's root de-privileging), the
error-shim stays on PATH and every agent run dies at the run_agent step with
'OSError: [Errno 8] Exec format error: \'claude\'' — even though the native
arm64 binary is present in the image as an optional dep, just never wired up.
Live-caught on ABCA-659's retry: all 3 ECS runs failed this way on a freshly
rebuilt image (the prior working image ran the older single-binary 2.1.142).
Re-run install.cjs explicitly after the npm install so the native binary is
placed at build time, and hard-verify with 'claude --version' so a still-broken
shim fails the image build instead of every task at runtime.
(cherry picked from commit 7cb4a87)
(cherry picked from commit 7de778b)
* fix(agent): log stdout on run_cmd failure so build-gate failures are debuggable
The post-agent build gate logged only stderr on a non-zero exit. But build/test
tooling (jest, tsc, the mise task DAG) writes the ACTUAL failing-task error to
STDOUT, not stderr — stderr often carries only the runner's plan echo. So a red
`mise run build` surfaced every task STARTING in CloudWatch but never WHICH one
failed or why (ABCA-662: a 7s exit-1 with zero captured cause, indistinguishable
from a phantom, and the reason Linear showed a build-fail GitHub CI passed).
run_cmd now also tails the last 20 lines of stdout on failure (redacted — repo
build output is untrusted). 4 tests pin: stdout surfaced, tailed-not-headed,
redacted, and NOT dumped on success.
(cherry picked from commit 67d84db)
(cherry picked from commit bb578fd)
* fix(agent): surface the FAILING line from a parallel build DAG, not just a tail
The stdout-on-failure logging (67d84db) tailed the last 20 lines — enough for a
serial command, but NOT for `mise run build`, which runs 4 packages in PARALLEL
and interleaves their output. The failing task's error lands in the MIDDLE while
the tail captures whatever finished LAST (e.g. a PASSING package's coverage
table) — so the log showed a green coverage table on a red build and named
nothing (ABCA-662: three verification runs where the actual failing sub-task was
never visible in CloudWatch).
run_cmd now scans the WHOLE stdout for failure-signature lines (FAIL/✕/●/error
TS/ELIFECYCLE/coverage-threshold/mise 'no task'/'task failed'/traceback/…),
filters benign noise ('0 errors', '--no-error'), and logs those FIRST, then a
trailing tail for context — deduped, capped at 40 lines. Falls back to the tail
when nothing matches (unknown tool). This is the tooling gap that made the whole
ABCA-662 build investigation slow: every failure was diagnosable only by luck or
local repro. 5 tests incl. the mid-DAG-failure + coverage-threshold cases.
(cherry picked from commit 3fe99c9)
(cherry picked from commit aaa4f18)
* fix(agent): bound the aws_session concurrency test's Barrier + joins (ROOT of the ECS flaky hang)
test_concurrent_first_call_builds_once used a bare threading.Barrier(20).wait()
+ unbounded t.join(). Barrier(N).wait() blocks FOREVER unless all N threads
arrive; under ECS container memory pressure a worker thread can be reaped (or
thread creation throttled) before reaching the barrier, so every survivor hangs
on wait() and the main thread hangs in join() — stalling the whole
`mise run build` until the 3600s ceiling. THIS is the ECS-only flaky hang chased
across ABCA-684/686/688 (pytest-timeout signal-method is the backstop; this is
the root cause). Bounded the barrier wait (timeout=30 → BrokenBarrierError) and
the joins (timeout=60), so a missing thread fails the test fast+loud instead of
deadlocking. Stress-ran 20x locally: no hang.
The test_attachments name pytest-timeout stamped earlier was a red herring — the
tracker reports whichever test was last seen, not the barrier-blocked one.
(cherry picked from commit 7ff91bf)
(cherry picked from commit 85d9881)
* fix(build): add pytest per-test timeout so a hung agent test fails loud
Agent pytest can hang on the ECS build box (an env-specific test blocking on
network/subprocess/Bedrock with no timeout of its own), stalling the pre-agent
baseline until the 3600s BUILD_VERIFY_TIMEOUT_S guard — turning one flaky test
into an un-diagnosable ~60-min stall (ABCA-684/686). Add pytest-timeout with a
120s per-test cap so the offending test fails LOUDLY with a dumped traceback and
the suite moves on.
Agent-side only: the jest --maxWorkers cap from the same source commit is
Mac-local build-box tuning (core-relative worker count), deliberately NOT
brought to main — CI containers have a different core/RAM profile.
(cherry-picked agent hunks of 5ea9670 from linear-vercel)
(cherry picked from commit bfcd280)
* fix(agent): pytest-timeout method thread→signal so a hung test is actually ABORTED
The thread method only PRINTS the hung test's stack at the 120s cap — it cannot
interrupt a test blocked in a C-level/socket syscall. Proven live on ABCA-688:
test_attachments dumped its stack at 120s, then the build kept hanging ~55 min
until the platform's 3600s build-verify ceiling finally killed it. pytest runs
single-threaded in the main thread here, so SIGALRM (method=signal) interrupts
even a syscall-blocked test and fails it in 120s as intended. Full agent suite
still 1315 passed (4.5s), plugin active.
(cherry picked from commit 803c44b)
(cherry picked from commit 1d20679)
* 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(agent): stop scoped-session S3 hang in the baseline build + reap hung suites + early ACK (#615)
On the ECS substrate the pre-agent baseline build (mise run build in the
cloned repo) hangs silently for 40+ min. Root cause: the ECS agent task def
sets AGENT_SESSION_ROLE_ARN, so aws_session resolves a *scoped* session and
tenant_client returns session.client(...), which BYPASSES a
@patch("boto3.client") mock. test_attachments then makes a REAL S3 get_object
that blocks forever on the ECS network (no egress) in a socket read SIGALRM
cannot interrupt.
Extends #598's 'fail hung tests loudly' theme (barrier bound + pytest-timeout)
with the scoped-session root cause it doesn't cover:
1. conftest _clean_env: add AGENT_SESSION_ROLE_ARN to the scrub list AND reset
the aws_session cache each test, so every test resolves the unscoped path
where boto3.client mocks intercept. The scrub is required in addition to the
reset — a cold get_session() re-resolves scoped while the var is still set.
+ regression test (TestConftestScrubsScopingEnv).
2. conftest hang watchdog: an independent daemon-thread reaper (dump all stacks
+ os._exit(1) at 600s). faulthandler.dump_traceback_later(exit=True) does NOT
work — pytest's faulthandler_timeout re-arms faulthandler's single timer
per-test WITHOUT exit=True, so a session exit timer is cancelled and a hang
only dumps, never exits. The daemon timer pytest cannot clobber; a blocked
socket read releases the GIL so it runs.
3. pipeline early ACK: move token resolve + 👀 (react_task_started) + Jira start
comment to BEFORE setup_repo() so a large-repo task shows immediate feedback
during the multi-minute baseline instead of looking dead. configure_channel_mcp
stays after the clone (needs repo dir). Side benefit: a setup-phase failure
now has a 👀 for the existing crash handler to swap to ❌.
Diagnosed + deployed + live-verified on the linear-vercel line (dev, ECS): the
suite that hung 55 min runs test_attachments in ~1s and completes in ~12s; the
👀 appears ~3s after task start instead of after the baseline. Full agent gate
green (1201 tests under AGENT_SESSION_ROLE_ARN set).
Closes #615.
* 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(agent): address #616 review — real conftest-scrub guard + watchdog cancel + comment
B1 (false regression guard): the TestConftestScrubsScopingEnv tests lived in
test_aws_session.py, whose OWN autouse _reset fixture does
monkeypatch.delenv(SESSION_ROLE_ARN_ENV) — so they passed even with the conftest
scrub line removed (they asserted what the local fixture guarantees, not what the
fix does). Moved both into a new fixture-free module tests/test_conftest_env_scrub.py
that poisons AGENT_SESSION_ROLE_ARN at import time (before any fixture runs) so
only conftest's _clean_env can clear it. Verified bidirectional: passes with the
scrub, BOTH tests fail when the _AGENT_ENV_VARS line is removed.
N2 (watchdog false-positive): add pytest_sessionfinish → _hang_watchdog.cancel()
so a legitimately slow-but-passing suite landing near the 600s deadline isn't
os._exit-killed into a bewildering red. Timer.cancel() is a no-op if it already
fired on a true hang.
N3 (comment rot): the watchdog rationale said 'per-test cap is 300s'; the actual
pytest-timeout is 120s. Reframed 600s as a session backstop sized between the
longest healthy run and the build ceiling.
(N4 early-ACK ordering test + N5 comment de-dup are non-blocking follow-ups.)
Agent gate green (1201 tests under AGENT_SESSION_ROLE_ARN set).
* 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.
* test(agent): address #616 review N4 + N5 — pin early-ACK ordering + de-dup hang comment
Follow-up nits from Scott's review; the blocking B1 + N2/N3 were already fixed in
1ebf6f9. These clear the two non-blocking items so nothing's left open.
- N4: add TestEarlyAckOrdering (test_pipeline.py) pinning the two invariants the
early-ACK reorder sells but nothing asserted: (a) the token-resolve + 👀
react_task_started + 'starting' comment all fire BEFORE the (minutes-long)
setup_repo baseline — captured via a shared parent mock's call order; (b) a
setup_repo failure still swaps the 👀 to ❌ — react_task_finished called with
success=False and the reaction id captured pre-setup. Guards against a future
reorder silently regressing either half.
- N5: de-duplicate the ECS-hang narrative. The authoritative block now lives in
the _clean_env docstring; the inline _AGENT_ENV_VARS comment on
AGENT_SESSION_ROLE_ARN is shrunk to a pointer to it (the top-of-file watchdog
comment + test_aws_session.py's NOTE were already pointers, not copies).
Agent gate green under AGENT_SESSION_ROLE_ARN set (the hang-repro condition):
1203 tests, no hang.
---------
Co-authored-by: bgagent <bgagent@noreply.github.com>
Co-authored-by: Alain Krok <alkrok@amazon.com>1 parent 0ee7227 commit d3923ad
5 files changed
Lines changed: 334 additions & 21 deletions
File tree
- agent
- src
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
842 | 842 | | |
843 | 843 | | |
844 | 844 | | |
845 | | - | |
846 | | - | |
847 | | - | |
848 | | - | |
849 | | - | |
850 | | - | |
851 | | - | |
852 | | - | |
853 | | - | |
854 | | - | |
855 | | - | |
856 | | - | |
857 | | - | |
858 | | - | |
859 | | - | |
860 | | - | |
861 | | - | |
| 845 | + | |
| 846 | + | |
| 847 | + | |
| 848 | + | |
| 849 | + | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
862 | 857 | | |
863 | 858 | | |
864 | 859 | | |
865 | 860 | | |
866 | | - | |
867 | 861 | | |
868 | 862 | | |
869 | 863 | | |
| |||
883 | 877 | | |
884 | 878 | | |
885 | 879 | | |
886 | | - | |
887 | | - | |
| 880 | + | |
| 881 | + | |
| 882 | + | |
| 883 | + | |
888 | 884 | | |
889 | 885 | | |
890 | 886 | | |
891 | 887 | | |
892 | 888 | | |
| 889 | + | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
| 893 | + | |
| 894 | + | |
| 895 | + | |
| 896 | + | |
| 897 | + | |
| 898 | + | |
| 899 | + | |
| 900 | + | |
| 901 | + | |
| 902 | + | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
893 | 912 | | |
894 | 913 | | |
895 | 914 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
3 | 7 | | |
4 | 8 | | |
5 | 9 | | |
6 | 10 | | |
7 | 11 | | |
8 | 12 | | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
9 | 71 | | |
10 | 72 | | |
11 | 73 | | |
| |||
94 | 156 | | |
95 | 157 | | |
96 | 158 | | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
97 | 163 | | |
98 | 164 | | |
99 | 165 | | |
100 | 166 | | |
101 | 167 | | |
102 | | - | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
103 | 184 | | |
104 | 185 | | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
82 | 91 | | |
83 | 92 | | |
84 | 93 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
0 commit comments