feat(compute): Lambda MicroVMs P2 — smoke parity foundations (#645)

[dreamorosi]

Implements Phase P2 of ADR-021 on the lambda-microvm backend. Follows #689 (P1). Honest scope note: live run 2 PASSED on 2026-08-07 — two tasks completed clone→change→commit→push→PR in 12 turns / $0.279 / 153 s, with the 45 s heartbeat cadence observed. The row is not fully closed: the run needed one live IAM workaround, and what remains is a clean re-run on a re-bootstrapped account with the source fixes and no workarounds; the deferred P3 empirical items (suspend TTL >1h and SUSPENDED-vs-quota) remain tracked on #645.

Platform config travels with the task, not the image

MicroVM env vars are image-version-frozen, and external images can't receive stack env at all — so deployment identifiers (table names, secret ARNs, session-role ARN; 13-key allowlist, 4 required) now ride the /run envelope as a platform_config block. The agent installs only allowlisted keys into its environment before any credential/pipeline initialization and fails closed on unknown keys (env installation from a network payload = injection surface). The allowlist lives once in contracts/constants.json; both the agent and the CDK strategy derive from it and check-constants-sync forbids literal re-declarations. Canonical wire shapes are documented in ADR-021 §3.

Snapshot credential hygiene (real defect found and fixed)

/ready was spinning up the CloudWatch debug writer, which resolved a credential chain and pinned the build-time region into boto3.DEFAULT_SESSION — state the image snapshot would replay into every MicroVM. Build hooks and pre-install /run logging are now stdout-only; a poisoned-seam test suite (every AWS/credential entry point armed to throw) plus a subprocess regression test lock the property, mutation-checked.

Full hook set + IAM parity + dual-signal liveness

The image now declares exactly what the agent serves: ready+validate (image hooks; /validate makes zero AWS calls — it runs under the build role) and run+terminate (runtime; /terminate returns 200 for any body and never writes terminal task status — the orchestrator owns terminal state). The execution role gains feature-based runtime parity (GitHub PAT + channel-OAuth secrets, scoped Bedrock, Memory grantReadWrite, AZ describe — still no direct DynamoDB; tenant data flows through the SessionRole). Heartbeat-staleness liveness now covers lambda-microvm (exhaustive per-backend switch; agentcore byte-identical, ECS excluded, RUNNING-scoped so P3 suspends stay immune) — closing the hung-pipeline-behind-healthy-substrate blind spot.

Docs

COMPUTE.md gains the Lambda MicroVMs column + explicit classic-Lambda distinction (the #645 acceptance criterion) with live-verified values only; ORCHESTRATOR.md gains the dual-signal liveness + lifecycle sections; SECURITY.md covers all three compute roles; ADR-021 amended in place (proposed). cli/README.md, USER_GUIDE.md, and API_CONTRACT.md now document the HEARTBEAT column and its list-level liveness semantics.

Operators must re-bootstrap before deploying this backend. The compute-lambda-microvm policy gains a MicrovmPassRoles statement, shipped in bootstrap policy bundle 1.6.0. A CDKToolkit stack bootstrapped at 1.5.0 or earlier fails the CDK-managed MicroVM image deploy with a caller-side iam:PassRole AccessDenied on the build role — an IAM error that reads like a code bug. Check CDKToolkit's BootstrapPolicyVersion output and follow the procedure in DEPLOYMENT_GUIDE.md §"Lambda MicroVMs backend (experimental)". (1.6.0 rather than 1.4.0 because #629 and #246 took 1.4.0 and 1.5.0 on main while this branch was open.)

Verification: cdk 4 269 / cli 768 / agent 1 755 (83.6% cov); tsc, eslint, ruff/ty/vulture clean; drift-prevention green; docs sync idempotent + astro check clean; bootstrap regeneration a zero diff; gitleaks clean (the retired 999988887777 fixture is suppressed path-scoped in .gitleaks.toml, so the per-PR range scan stays green across a rebase).

security:sast (semgrep) is not part of the per-PR gate — security-pr.yml runs gitleaks, osv-scanner and zizmor only, and SAST runs on the weekly security.yml schedule. semgrep is unavailable in this environment, so the new error-handling code was hand-checked against .semgrep/silent-success-masking.yaml; a workflow_dispatch of security.yml on this branch is cheap if you want the full suite pre-merge.

Refs #645

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 99.69574% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1b17c28). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
agent/src/server.py	97.60%	6 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #733   +/-   ##
=======================================
  Coverage        ?   92.28%           
=======================================
  Files           ?      318           
  Lines           ?    89576           
  Branches        ?     9934           
=======================================
  Hits            ?    82662           
  Misses          ?     6914           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dreamorosi]

Pre-review pass (self-commissioned, per .abca/commands/review_pr.md) + housekeeping since the smoke run, for reviewer context:

Pre-review found 6 blockers; all fixed in 38ecc66. The two worth highlighting: the no-platform_config /run branch violated this PR's own pre-install stdout-only EARS rule (_warn_cw → CW thread → pinned boto3.DEFAULT_SESSION — the P2 build-hook defect one phase later), and the seam-guard test structurally couldn't catch it (disarmed on the vacuous no-config early-return — asserting what the code does, not what it should). Also: the diagnostics-only claude probe could still kill a task on TimeoutExpired (now non-fatal), the /ready budget invariant moved from a hardcoded test literal into contracts/constants.json with ordering invariants, and SECURITY.md now states the MicroVM trust-condition delta explicitly.

The verification runbooks are now committed (docs/verification/, ~4,100 lines, redacted: IAM unique-IDs, account IDs, emails, VM endpoints) — every IAM relaxation this PR ships cites evidence that is now reviewable in-repo, and ADR-021 §4 additionally inlines the load-bearing pieces (two-arm PassRole experiments, verbatim denials, the contaminated-control chronology).

One PR-body correction (pre-review caught it): the body said security:sast runs in CI — it actually runs on the weekly schedule, not on PRs (security-pr.yml is gitleaks/osv/zizmor only). The new error-handling code was hand-checked against .semgrep/silent-success-masking.yaml; a workflow_dispatch of security.yml on this branch pre-merge is cheap if you want the full SAST suite first.

08b3091 — merged current main (#544 admission queue; 13 overlapping files, 3 textual conflicts, no semantic re-seating needed — PollState/pollTaskStatus were untouched by #544; both TaskDetail additions coexist in types-sync-exact order; Queue: and Heartbeat: render in different blocks and are mutually exclusive in practice). Full matrix green post-merge (cdk 3885 / cli 750 / agent 1622).

Also filed #736 so the "re-tighten IAM when AWS exposes usable condition keys" revisit has a handle.

@krokoko this is the fourth main-merge this PR has absorbed (#695, #711-#718, #345+#704, #544) — grateful for a prompt look when you have one, or auto-merge-on-approval if that's easier.

[scottschreckengaust]

Verdict: Request changes

Strong architecture, security posture, and test discipline — CI is green (4/4), the bootstrap bundle verified clean by execution, and platform_config is a genuine security improvement over image-frozen env. Gated on seven items, all small and local. Four of them sit on the diagnosis path, and one hits the failure mode this backend hits most often.

The unifying theme: this PR is excellent at failing closed, and weaker at explaining why it failed. Rejections are correct, distinctly coded, and tested — then the code either discards the reason, routes it to a classifier that renames it, or documents a mechanism that isn't real.

---

Blocking

B1. stateReason is discarded, so the most common failure gets a fabricated cause

cdk/src/handlers/shared/strategies/lambda-microvm-strategy.ts:691-744

pollSession reads result.state and ignores stateReason. Your own runbook (docs/verification/645-p2-smoke-runbook.md:771) records what that field says on the dominant runtime failure:

state       = TERMINATED
stateReason = Run lifecycle hook returned HTTP status 400. Please check your hook endpoint...

TERMINATED → {status:'completed'} has no error slot, so orchestrator.ts:451 builds detail = "substrate state completed" and the operator gets:

MicroVM substrate terminated before the agent wrote a terminal status: substrate state completed

…with a remedy naming "session duration cap, host fault, or an external terminate" — none of which happened. Every distinct wire code /run carefully emits (MICROVM_RUN_PLATFORM_CONFIG_INVALID, TASK_RECORD_INCOMPLETE) is thrown away one layer up. Per this repo's own standard, a plausible-but-wrong result is a defect.

Fix: thread stateReason into the reconcile detail (minimum: logger.warn it plus append to the message).

B2. The new required-key throw is classified TRANSIENT and auto-retried

cdk/src/handlers/shared/strategies/lambda-microvm-strategy.ts:282-291

I ran the real classifier regexes. This message carries no MICROVM_ERROR_MARKER, so it falls to the /Session start failed/i catch-all (error-classifier.ts:320) → retryable: true, remedy "Check AgentCore Runtime or ECS cluster health." A hand-edited-Lambda-environment fault, which retrying provably cannot fix, gets retried and misattributed to the wrong substrate. This is exactly what the marker section's own comment calls "mandatory, not decorative." Tests assert the message text thoroughly but never its classification.

Fix: wrapMicrovmError('platform config', …) or a classifier entry — plus a test asserting classification, not just message.

B3. orchestrator.ts:82-85 documents an invariant the code does not have — in the direction that breaks ECS

"The agent writes that timestamp UNCONDITIONALLY on every substrate… so this predicate decides only whether the ORCHESTRATOR acts on it."

Verified false. pipeline.py:919 writes once; the only periodic writer is _heartbeat_worker (server.py:272, 45 s), started solely from _run_task_background — reachable only via /invocations or the MicroVM /run hook. ECS bypasses uvicorn entirely (ecs-strategy.ts:245: "This bypasses the uvicorn server entirely").

So ecs => false is a hard correctness constraint, not the tuning preference the comment frames it as. Anyone consolidating those "two independently-tuned kill paths" would flip it to true and fail every ECS task after ~6 minutes (grace 120 s + stale 240 s). The comment invites the change that breaks the backend.

Fix: lead the ecs bullet with the hard constraint — "ECS never starts _heartbeat_worker; enabling this fails every ECS task after ~6 min."

B4. The boto3.DEFAULT_SESSION region-pinning claim is factually wrong

agent/src/server.py:1084-1088, 1105-1107, 1122-1123

Tested in this repo's own venv (botocore 1.43.42), controlled for ambient AWS_PROFILE/~/.aws/config:

BUILD  session.region_name: us-west-2 | logs client: us-west-2
AFTER  session.region_name: eu-west-1 | logs client: eu-west-1
creds frozen to build role: AKIAbuild

A pre-existing DEFAULT_SESSION freezes credentials only. Region and AWS_SDK_UA_APP_ID re-resolve per client, because botocore's EnvironmentProvider holds a live os.environ reference.

Worth fixing rather than shrugging at, because the credential half is real and is the stronger argument: build-role credentials baked into a snapshot every MicroVM restores from is a security property; a stale region is a bug. This claim is the sole justification for three helpers and a deliberately awkward branch at server.py:1839 — a maintainer who tests the region claim, finds it false, and concludes the discipline was cargo-cult would delete helpers whose real rationale is sound.

Fix: narrow all three comments to credentials.

B5. GITHUB_TOKEN_SECRET_ARN redirect reads another workspace's OAuth token

agent/src/server.py:1074 + cdk/src/constructs/lambda-microvm-compute.ts:1168-1184

_install_platform_config validates keys rigorously and values not at all — no ARN-shape, account, partition, or region check before os.environ[env_name] = value. Most keys are contained by IAM (a foreign agent_session_role_arn AccessDenies → SessionScopingError → fails closed; tables/buckets are LeadingKeys/prefix-scoped). One is not:

The execution role holds GetSecretValue on bgagent-linear-oauth-* / bgagent-jira-oauth-*. config.py:55-65 fetches whatever ARN GITHUB_TOKEN_SECRET_ARN names and caches the raw SecretString into os.environ["GITHUB_TOKEN"], from which shell.py passes the environment to every repo subprocess — i.e. into the model's tool surface. So a /run payload naming another workspace's channel-OAuth secret succeeds: allowlisted key, unvalidated value, matching grant.

The prefix grant is at ECS parity. The asymmetry that makes it reachable is new: on ECS the ARN arrives as deploy-time container env; here it arrives in a network payload. The comment at server.py:874 ("secrets are still fetched at /run time … using the ARNs delivered here") is precisely where the ARN needs to stop being free-form.

Fix: pin *_secret_arn / *_role_arn values to the MicroVM's own partition/account/region before install, rejecting with the existing …_INVALID code. A per-key regex table beside env_by_key keeps it contract-sourced.

Related, and worth recording: NO_INGRESS reachability was never negatively verified. 645-p2-smoke-runbook.md:745-749 notes a NO_INGRESS VM still returns a public <vm-id>.lambda-microvm.<region>.on.aws hostname and warns that "endpoint exists" isn't evidence of reachability — but nobody probed it. /run has no application-layer auth (no Depends/HTTPBearer; lambda:CreateMicrovmAuthToken granted to no role, by design per sub-decision 3), so the entire posture rests on that untested inference. A bounded probe belongs in the P3 runbook.

B6. A real AWS account ID is committed

cdk/test/bootstrap/policies.test.ts:518-519 — 704224321915

Verified new in this PR (absent from main), against a repo-wide convention of 123456789012 (8 uses in the same tree). Both 2,000-line runbooks in this PR correctly redact to <account> — the runbooks were scrubbed, the test wasn't. Not a secret, but this is a public sample repo. The test asserts prefix-glob survival across CFN's 64-char truncation, which depends only on the role-name segment, so the placeholder loses nothing.

B7. The iam:PassRole map entries are inert, and the comment names a guard that doesn't exist

cdk/src/bootstrap/resource-action-map.ts:89-100

"listing the action here is what makes its removal a test failure instead of a redeploy failure."

It doesn't. collectBootstrapAllowActions() collects action strings only, discarding Resource and Condition — and infrastructure.ts's IAMPassRole already contributes bare iam:PassRole to every bundle. The requirement is therefore satisfied by the conditioned statement, which is exactly the one P2r2-F9 proved is denied on this path. Empirically confirmed: deleting MicrovmPassRoles leaves the check green (missing for MicrovmImage: []). Compounding it, synth-coverage.test.ts synthesizes only the default context, where AWS::Lambda::MicrovmImage is never emitted, so these entries are never consulted.

Not an open hole — the new policies.test.ts SID assertions do catch removal. But it's a security-relevant comment misdescribing which mechanism protects the fix.

Fix: point the comment at policies.test.ts, or make the map condition-aware.

---

Non-blocking

1. Null byte → 500 instead of a structured 400. server.py:1074. Verified: os.environ['X']='a\x00b' raises ValueError, which escapes _install_platform_config (raises only _PlatformConfigError) and bypasses the handler's except at :1814. Newlines are accepted silently (log injection into your structured lines). Keys get regex validation; values don't. Untested.
2. MicroVM poll failures never escalate. orchestrate-task.ts:405-415 warns forever; the ECS sibling 40 lines up fails at 3 consecutive failures. A permanent fault (missing lambda:GetMicrovm, Region gap) is indistinguishable from a hiccup → ~1020 identical warns and a full 8.5 h billed reservation — the cost posture this PR's own heartbeat work exists to prevent.
3. Payload retention regresses ECS parity. deleteEcsPayload is called at finalize (orchestrate-task.ts:458); MicroVM has no equivalent and relies solely on MICROVM_PAYLOAD_TTL_DAYS = 1. With bucket-wide grantRead and <taskId>/payload.json keys, any running MicroVM can read another task's hydrated prompt/issue thread — window widens from minutes to ~24 h.
4. ADR-021's own EARS requirement is unimplemented. :52 requires CSPRNG reseeding on /run; :378 calls missing it "a silent security defect." No random.seed/os.urandom anywhere in agent/src. Exposure is small (only progress_writer.py:75's getrandbits(80) ULID, a sort key under a task_id partition — a collision needs same task and millisecond). Implement the one-liner or amend the ADR.
5. Unreachable FINALIZING arm. finalizeTask acts on sessionUnhealthy for RUNNING or FINALIZING, but pollTaskStatus can only set it for RUNNING — dead for all three backends.
6. _debug_cw_failures is incremented, never read. Three docstrings describe an alarm operators can watch; no metric, alarm, or /ping exposure exists. Pre-existing — but B4's rationale leans on it ("would poison the signal"), and there's no signal to poison.
7. /validate secret detection reports into a void. server.py:1629. warnings: ["secret_env_present_in_snapshot:GITHUB_TOKEN"] rides a 200 nothing parses. One _build_hook_log line would land it in the build log group. Report-only is the right call; discarding it isn't.
8. _READY_WARMUP_TOTAL_BUDGET_SECONDS: float = 240.0 evades the drift regex (\d+\b fails on 240.0), and check-constants-sync.ts gained ~140 lines with no test file anywhere.
9. Producer misattributed. server.py:879 sends readers to orchestrator.ts for the platform_config producer; it's buildMicrovmPlatformConfig in the strategy (grep finds 0 hits in orchestrator.ts).
10. agent_heartbeat_at on toTaskDetail but not toTaskSummary — bgagent list still can't show liveness. Worth a deliberate decision given the PR's own framing.
11. logs:CreateLogGroup on the execution role (:1482) — the group is pre-created at :1018, so this is a create right the runtime never uses on the role that runs untrusted repo code. Splitting build/runtime grants costs three lines.
12. PR description understates the artifacts. The body says the smoke run "executes against a live account after this lands," while ADR-021:403 and the synth warning both record run 2 passing on 2026-08-07 (2 PRs, 12 turns, $0.279, heartbeat observed). The artifacts are more accurate than the description — worth reconciling so the honest scope note is trusted.

---

Vision alignment — strongly aligned

• Bounded blast radius — platform_config removes the image snapshot as a source of deployment truth. Rejecting the whole block on an unknown key rather than filtering correctly treats unrecognized keys as LD_PRELOAD/AWS_ENDPOINT_URL injection, not a compatibility gap. I verified the 13-key allowlist contains no process-hijacking variable.
• Bounded cost — heartbeat liveness on lambda-microvm closes a real 8-hour-reservation burn; live evidence shows a hung guest sits in RUNNING indefinitely with no stateReason.
• Reviewable outcomes — the unconditional, non-suppressible synth warning telling operators to keep production on agentcore/ecs is exactly right, as is freezing the warning id across phases.
• Tenet trades documented — the dropped iam:PassedToService / aws:SourceAccount conditions are evidenced by a controlled two-arm experiment with per-role compensating controls in ADR-021 §4. The ADR retracts its own earlier false negative and diagnoses the contaminated control that caused it. That's the standard.

Documentation — complete

• Mirror in sync (verified): ran node scripts/sync-starlight.mjs → exit 0, git status clean.
• COMPUTE.md adds the Lambda MicroVMs column and the explicit classic-Lambda distinction (COMPUTE.md:24) — the #645 acceptance criterion, satisfied.
• ORCHESTRATOR.md, SECURITY.md (all three roles), DEPLOYMENT_ROLES.md golden baseline, contracts/constants.md, ADR-021 amended as proposed.
• Gap: the mandatory re-bootstrap to ≥1.4.0 appears in the ADR, the synth warning, and the packaging script — but no docs/guides/ file mentions lambda-microvm at all. An operator following a guide won't learn they must re-bootstrap.

Tests & CI

CI 4/4 green. cdk 3,825 / agent 1,590 (82.8%).

Bootstrap synth-coverage: PASS, verified by running it. Reframing worth noting: the constructs pre-existed this PR, so there are no new CFN resource types — the addition is one MicrovmPassRoles statement. A context-enabled synth showed UNMAPPED TYPES: [], MISSING ACTIONS: {}. Artifact regeneration was byte-identical (no hand-editing). 1.3.0 → 1.4.0 is the correct minor bump. test/bootstrap: 113/113. The ARN-truncation risk is pinned by a test, not just a comment — ConnectorOperatorRole truncates to …ComputeConnectorOp-, still inside the glob, and the modeled BuildRoleF0 truncation reproduces the live ARN quoted in the comment.

Rule #366 (synth perf): not violated — no test re-enables bundling; new synths are beforeAll-cached.

The poisoned-seam suite is real, not theater. It sets LOG_GROUP_NAME before asserting silence (without which every assertion passes trivially), and install_phase_done flips only on a non-empty install list — the difference between a real test and a hole, since _install_platform_config(None) returning [] would otherwise disarm the guard across the exact legacy path where the bug lived. Mutation claims spot-checked and held: reverting _build_hook_log→_debug_cw, platform_client→boto3.client, and heartbeatLivenessApplies→=== 'agentcore' are each caught.

Real gaps: no \x00/\n value test; /terminate's active: None branch untested despite a comment arguing it's load-bearing; and the heartbeat→TerminateMicrovm composition is unasserted for any backend — fakeContext ignores waitStrategy, so the billing outcome the change exists for isn't covered end-to-end. The runbook admits the switch "never got a long-enough RUNNING window to exercise."

Review agents run

All seven dispatched and completed: code-reviewer, silent-failure-hunter, type-design-analyzer, pr-test-analyzer, comment-analyzer, plus security/IAM and bootstrap-coverage reviews. None omitted.

Two agents disagreed on B4's boto3.DEFAULT_SESSION behavior, so I settled it by experiment rather than vote — and my first five measurements all agreed with the wrong answer because ambient AWS_PROFILE/~/.aws/config silently confounded them, S3 masks region behind a us-east-1 fallback, and this botocore reads AWS_DEFAULT_REGION rather than AWS_REGION. Same shape as the contaminated control ADR-021 documents and retracts. Flagging because it means any reviewer re-checking B4 needs a clean-room env to see it.

Human heuristics

• Proportionality — concern. server.py 1,892 lines, lambda-microvm-compute.ts 1,534, ~4,100 lines of in-tree runbooks. Most length is load-bearing evidence (measured values, the probe that established them, the wrong value replaced) — genuinely unrecoverable once lost. But two ~30-line docstrings document an "unreachable in practice today" branch twice (server.py:1550, 1600), and phase-status prose is narrated in three files needing hand-updates per phase.
• Coherence — pass. Correct routing. New backend reuses ComputeStrategy unchanged (verified: empty diff on compute-strategy.ts) — no optional member forced on the other two. Divergences from ecs-strategy (NotFound→completed) are explained by substrate behavior, not convenience.
• Clarity — concern. Names are good; the exhaustive never switch is mutation-verified compile-breaking. But B1/B2 hide real causes behind plausible defaults (AI004), and B3/B4/B7 are comments asserting mechanisms that don't exist — the highest-cost comment defect, since maintainers act on them.
• Appropriateness — mostly pass. Verified against real AWS behavior, not self-written mocks (AI001) — the 4,096-vs-documented-16,384 runHookPayload cap with its verbatim ValidationException is exactly the grounding I want, as is the shell-parsing test that replaced a prose "keep these in step" comment which had already failed once. Two exceptions: the inner/outer platform_config precedence tests pin a branch the producer's own contract makes unobservable, and contracts/constants.json derivation is rightly preferred over hand-copied literals.

---

Genuinely good work — the live-evidence discipline, the self-retracting ADR, and the poisoned-seam suite are all above the bar for this repo. The gate is narrow: make the failures say what actually happened (B1, B2), fix three comments that would mislead a maintainer into breaking ECS or deleting sound credential hygiene (B3, B4, B7), pin ARN values (B5), and scrub the account ID (B6).

[scottschreckengaust]

Inline follow-up to my review above — the same findings anchored to specific lines, with committable suggestion blocks where the fix is unambiguous.

Directly committable via the GitHub UI: B6 (account ID), B3 (heartbeat docstring), B4 (credential-vs-region wording), B7 (map-entry comment), and the control-character nit.

Code sketches rather than one-click suggestions — because they touch a type or span non-adjacent lines: B1 (stateReason needs an optional reason?: string on SessionStatus plus the orchestrator.ts:451 detail string), B2 (wrapMicrovmError — verify the escaped apostrophe survives your lint), and B5 (ARN pinning).

B1 is anchored slightly above pollSession because that function falls outside the diff hunks; the comment names the real line range.

Everything asserted here I verified by running it — the classifier regexes, the botocore region/credential behavior in this repo's own venv, the os.environ null-byte ValueError, and the bootstrap action-collection check with MicrovmPassRoles deleted.

[ayushtr-aws]

Verdict: Request changes

Second-pass review (principal-architect lens + the mandatory pr-review-toolkit agents, run against the checked-out branch). I re-verified @scottschreckengaust's earlier review against current HEAD and found some new issues; this comment does not restate B1–B7 — it confirms they're still live and adds three findings that review didn't cover.

Excellent, evidence-disciplined work overall — platform_config as deployment-truth-off-the-snapshot, the poisoned-seam suite, and the self-retracting ADR are all above this repo's bar. The gate is the same theme the first review named: this PR fails closed correctly, then loses or misreports why it failed.

---

Prior blockers are all still open

The earlier review landed on 08b3091, which is still HEAD — no fix commits since — so B1–B7 remain unaddressed. I independently reproduced every one; flagging that they still gate:

• B1 pollSession (lambda-microvm-strategy.ts:696) discards stateReason → dominant failure renders "substrate state completed".
• B2 required-key throw (lambda-microvm-strategy.ts:283) skips wrapMicrovmError — with a correction to the earlier framing (see below).
• B3 orchestrator.ts:84-86 — confirmed false; ecs => false is a hard correctness constraint (see N-follow-up below).
• B4 boto3.DEFAULT_SESSION freezes credentials only, not region.
• B5 _install_platform_config validates key allowlist + value type, never value content → cross-workspace OAuth-secret ARN redirect.
• B6 real account ID 704224321915 at policies.test.ts:518-519.
• B7 resource-action-map.ts:96-97 — the iam:PassRole map entry is inert; the real guard is policies.test.ts.

---

New blocking-grade findings (not in the prior review)

N1 — S3 payload parse errors are misclassified as non-retryable 400. server.py:1246-1248: on the dominant pointer path, json.loads(body) (JSONDecodeError ⊂ ValueError) and the non-object guard raise bare ValueError, which hits /run's except ValueError → 400 MICROVM_RUN_PAYLOAD_INVALID ("retrying an identical body cannot help") before the except Exception → 500 …_UNREADABLE ("retrying CAN help"). A truncated / racing / half-written S3 object is retryable, and the operator is told the orchestrator built a bad envelope — it didn't; the S3 object was bad. Only the pre-fetch URI-shape ValueError at :1239 correctly belongs in the 400 branch, which is exactly why a blanket except ValueError is the wrong discriminator.
Fix: raise a dedicated non-ValueError (e.g. PayloadFetchError) for post-fetch content problems so they fall through to the 500 branch. Note the function-level ValueError on a non-object body is covered (test_server.py:1461), but no test drives that unreadable body through the /run handler to assert its classification — the handler-level S3-failure test (test_server.py:1464) mocks _fetch to raise RuntimeError, so the JSONDecodeError/non-object → 400 misroute is never exercised end-to-end.

N2 — the no-platform_config path silently skips the required-key check, so tenant isolation can be silently OFF. server.py:1025-1026: _install_platform_config(None) returns [] before the required-key validation at :1061. agent_session_role_arn is a required key precisely because aws_session downgrades to ambient compute-role credentials with tenant scoping silently off when AGENT_SESSION_ROLE_ARN is unset, and ADR-021 sub-decision 3 forbids baking that ARN into the snapshot. The docstring itself says the image and orchestrator "deploy on independent cadences" — so a pre-P2 orchestrator (no platform_config sibling) launching a P2 image produces a task that runs to completion with per-tenant isolation disabled, traced only by a stdout-only _pre_config_log line that never reaches the task's CloudWatch group. This is the sharpest failure the required-key list exists to prevent, and the None path routes around it.
Fix: on the None path, still enforce MICROVM_PLATFORM_CONFIG_REQUIRED_KEYS against the effective environment (payload-or-baked) and reject …_INCOMPLETE when a required key is neither delivered nor present; at minimum escalate the no-config branch beyond a stdout breadcrumb so a silently-unscoped run is auditable.

N3 — comment/code contradiction on the build-role Logs grant. server.py:1102-1104 states "The build role has no Logs grant, so the write can only FAIL" — but lambda-microvm-compute.ts:1094 calls grantMicrovmLogWrites(this.buildRole), granting scoped logs:CreateLogGroup/CreateLogStream/PutLogEvents on the /aws/lambda-microvms/* namespace. The accurate statement is that the grant is scoped to the service namespace, so a write to any other LOG_GROUP_NAME (e.g. a baked APPLICATION_LOGS group) fails. Reason #2 in that same docstring (boto3.DEFAULT_SESSION freezing the build session into the snapshot) is accurate and load-bearing, so the code behavior is fine — only the stated IAM fact is wrong, and it's the kind a maintainer would act on.

---

Follow-ups reinforcing the prior review

• B2 — correction to the earlier framing (I traced the classifier paths). The earlier review said the missing-key error is "classified TRANSIENT and auto-retried." The auto-retry half doesn't hold: startSessionWithRetry classifies the raw error (session-start-retry.ts:116, deliberately, per #599 — the /Session start failed/i wrapper is itself a transient pattern), and the raw "Cannot start a lambda-microvm session: …" matches no transient pattern → falls to UNKNOWN→USER → thrown immediately, not retried. But the wrong-remedy half is real: failTask persists "Session start failed: <raw>", and the operator/channel-facing failure-reply.ts:150 re-classifies that prefixed string, which now matches the /Session start failed/i TRANSIENT catch-all → the user is told "Check AgentCore Runtime or ECS cluster health / quota exhausted" for a hand-edited-orchestrator-env fault on the MicroVM path. So B2 still stands as a real defect (misattributed cause + errorClass: TRANSIENT), just via the failure-reply classification rather than an actual retry. wrapMicrovmError('platform config', …) still fixes it; the fix and its blocking status are unchanged.

• B3 is a hard correctness constraint, restated for emphasis. pipeline.py:919 writes the heartbeat once; the periodic 45s refresh that staleness detection depends on is _heartbeat_worker (server.py:272), started only inside _run_task_background — the AgentCore /invocations and MicroVM /run paths. ECS launches run_task_from_payload directly (ecs-strategy.ts:246), bypassing server.py, so it has no periodic writer. A maintainer flipping ecs to true on the strength of the "predicate only decides whether the orchestrator acts" comment would fail every ECS task ~240s in. Please lead the ecs bullet with the constraint.

Non-blocking nits (concur with the prior review; spot-verified)

1. Null-byte value → uncaught ValueError → 500 + partial env install (fail-closed contract broken); newlines accepted silently (log injection). server.py:1074. Untested.
2. MicroVM payload has no deleteEcsPayload equivalent — relies solely on MICROVM_PAYLOAD_TTL_DAYS = 1; parity regression vs ECS (orchestrate-task.ts:458), widening the cross-task read window from minutes to ~24h given bucket-wide grantRead.
3. ADR-021's CSPRNG-reseed EARS requirement is unimplemented (no random.seed/os.urandom in agent/src; only progress_writer.py:75). Implement the one-liner or amend the ADR.
4. Unreachable FINALIZING arm in the sessionUnhealthy gate (orchestrator.ts:990) — pollTaskStatus only sets it for RUNNING.
5. Heartbeat-stale failure message hard-codes "container" (finalizeTask), now emitted for a MicroVM substrate that has none — parameterize by computeType.
6. formatTaskDetail heartbeat age calls Date.now() directly (cli/src/format.ts), non-deterministic under test, unlike its formatStatusSnapshot sibling which threads an injected now.

Docs

Mirror verified in sync; COMPUTE.md's Lambda MicroVMs column + classic-Lambda distinction satisfies the #645 criterion. One gap stands from the prior review: the mandatory re-bootstrap to bundle ≥1.4.0 appears in the ADR, the synth warning, and the packaging script, but no docs/guides/ file mentions lambda-microvm — an operator following a guide won't learn they must re-bootstrap.

Tests & CI

CI 4/4 green. Bootstrap synth-coverage PASS / no new CFN types (constructs pre-existed; 1.3.0→1.4.0 is the correct minor bump). Rule #366 not violated (bundling stays off; new synths beforeAll-cached; the two per-test synths carry explicit constructor-throw exemption comments). Real gaps: no \x00/\n value test; /terminate's active:None branch is never actually exercised (the best-effort-failure test assigns active before the patched raise); and the heartbeat → TerminateMicrovm composition is unasserted end-to-end for any backend.

Review agents run

All five pr-review-toolkit agents dispatched against the checked-out branch and completed: code-reviewer (no ≥80-confidence blockers; confirmed type-sync of agent_heartbeat_at across cdk/cli, UA attribution #319 on the S3 fetch, bootstrap regen consistency, contract sourcing), silent-failure-hunter (N1, N2), comment-analyzer (N3, confirmed B3/B7), pr-test-analyzer (test gaps above). type-design-analyzer omitted — the diff adds no substantive new types (only optional reason? / agent_heartbeat_at fields on existing records, verified type-synced). Security/IAM + bootstrap judgment applied by hand.

---

Net: the seven prior blockers stand, and I'd add N1 (retryability inversion on the dominant payload path) and N2 (silent tenant-isolation-off on version skew) as blocking-grade, plus N3 as a must-fix misleading comment. The live-evidence discipline throughout is genuinely strong — the gate is narrow and local.

[scottschreckengaust]

Heads-up: a 6-issue model-configuration stack is about to take flight and overlaps this PR

@dreamorosi — flagging an incoming file-level overlap so it doesn't surprise you at rebase time. No action needed from you right now, and nothing here asks this PR to change direction; we're treating it as normal first-in-merge conflict resolution.

The stack

Tracking issues #740 (docs) and #741 (Opus 5 + global Bedrock endpoint), broken into six children:

#	What	Touches files this PR touches?
#742	canonical model-config docs + fix stale defaults + drift test	agent/README.md ✔
#743	run.sh stops overriding the platform default	no
#744	grant Claude Opus 5 (additive IAM + allowlist)	bedrock-models.ts ✔ (entries only)
#745	flip platform default → us.anthropic.claude-opus-5	no
#746	new bedrockGeoRegion context key, default us	bedrock-models.ts, stacks/agent.ts ✔
#747	flip geo → global	no

Where the real overlap is

#746 vs this PR is the meaningful one. This PR introduces:

export const DEFAULT_HAIKU_MODEL_ID = 'anthropic.claude-haiku-4-5-20251001-v1:0';
export const DEFAULT_HAIKU_INFERENCE_PROFILE_ID = `us.${DEFAULT_HAIKU_MODEL_ID}`;

and reads it at cdk/src/stacks/agent.ts:393 in place of the previous hardcoded literal. That's genuinely the right direction — it removes a two-place hardcode that #740 flagged as a drift risk. 👍

#746 needs the same value to become geo-parameterized (us. → whatever bedrockGeoRegion resolves to), because the platform is moving to the global. cross-region profile. So #746 will want to convert your constant into a derived value rather than replace it. Also relevant: this PR's lambda-microvm platform_config carries its own anthropicDefaultHaikuModel, which means #746 has three grant sites to thread rather than the two its body currently assumes — I've annotated that on #746.

#744 and #745 shouldn't affect you. They add entries to DEFAULT_BEDROCK_MODEL_IDS / WORKFLOW_MODEL_ALLOWLIST and change default-value strings; they don't touch the haiku constants or the geo plumbing.

Verified context that may be useful for this PR

Checked against live Bedrock (us-east-1) while planning the stack:

• us.anthropic.claude-opus-5 and global.anthropic.claude-opus-5 are both ACTIVE and invocable; the bare anthropic.claude-opus-5 returns ValidationException: ... on-demand throughput isn't supported — consistent with the comment rationale in this PR for why bare ids can't be used.
• global.anthropic.claude-haiku-4-5-20251001-v1:0 also exists and is invocable, so a global.-prefixed haiku profile is available if this PR ever wants it.
• Global and us. profile ARNs are identical in shape (regional + account-qualified), so a geo switch is a prefix change only — no ARN-shape handling needed.
• The pinned toolchain (claude-agent-sdk==0.2.110, bundled CLI 2.1.191) passes both us.- and global.-prefixed Opus 5 through successfully, including with a global.-prefixed ANTHROPIC_DEFAULT_HAIKU_MODEL. No SDK/CLI bump needed for either.

If you'd like to avoid the rebase entirely

Optional and entirely your call: if this PR took the geo prefix from a resolver rather than hardcoding us. in the template literal, #746 would have nothing to re-open. Given this PR is already +10,575/−430, I'd understand preferring to land as-is and let #746 rebase onto it — which is the default assumption unless you say otherwise.

[dreamorosi]

19 findings fixed; 3 answered with code/live-run evidence.
+100 regression tests; CDK, CLI, agent, docs, type-sync, drift-prevention, bootstrap regeneration, and lint gates are green.
Fix wave: baa365fc.

@ayushtr-aws — second-pass findings

N1 — S3 payload parse errors

Fixed. agent/src/server.py now raises a dedicated non-ValueError _PayloadFetchError for post-fetch content problems, so truncated/non-JSON/non-object S3 payloads reach the retryable 500 MICROVM_RUN_PAYLOAD_UNREADABLE branch. The pre-fetch URI-shape check remains a non-retryable 400. I also converted the nested agent_payload type check, which is the same fetched-object failure class. Tests assert the exception is not a ValueError and drive the handler-level 500 classification.

N2 — no-platform_config path

Fixed the audit half, with one correction to the impact statement.

The mechanism is exactly as described: _install_platform_config(None) returned before the required-key check, and aws_session.get_session() silently takes the ambient branch with _scoped = False when AGENT_SESSION_ROLE_ARN is unset. The skew is reachable — the pre-P2 strategy sends no platform_config.

However, “runs to completion with per-tenant isolation disabled” does not hold on a CDK-managed deployment. imageEnvironmentVariables defaults to {} on both main and this branch, so TASK_TABLE_NAME is absent too, and the MicroVM execution role has zero direct DynamoDB grants. The unscoped run cannot reach tenant data; it dies on the first table write. The path therefore fails closed through IAM plus absent table names — just obscurely, with a stdout-only breadcrumb.

The no-config branch now checks the effective environment for every required value. A genuine legacy image that bakes those values still runs; a pre-P2 orchestrator launching a P2 image returns 400 MICROVM_RUN_PLATFORM_CONFIG_INCOMPLETE with an attributable log line and a missing_env list. The new rejection path is covered by the hostile pre-install seam guard, so it makes no CloudWatch/credential call before refusing the run.

N3 — build-role Logs comment

Fixed. _build_hook_log now says the build role's Logs grant is scoped to /aws/lambda-microvms/*, so a write to another baked LOG_GROUP_NAME fails; the credential-memoization reason is marked load-bearing.

Nits and test gaps

• Control characters / partial install: fixed before any environment mutation; NUL/newline/carriage-return return structured 400 and install nothing.
• MicroVM payload retention: fixed with finalize-time deleteMicrovmPayload; the one-day lifecycle remains the backstop.
• CSPRNG EARS requirement: ADR-021 amended and moved to P3 scope with the measured exposure: the sole random consumer is progress_writer.py's ULID sort key under a task_id partition; os.urandom/secrets are unaffected and no credential derives from random.
• Unreachable FINALIZING arm: kept defensively, documented with the exact reachability gap, and pinned by tests proving sessionUnhealthy remains RUNNING-scoped.
• Heartbeat copy says “container”: fixed with compute-specific “MicroVM” / “container” / generic “runtime” wording, including the classifier copy.
• Date.now() inside detail formatting: fixed; detail and list formatting now accept an injectable now.
• /terminate active: None branch: directly exercised by making the thread-registry read fail; the prior _debug_cw-failure test is now documented as not reaching that branch.
• Heartbeat → TerminateMicrovm composition: now covered end to end with a durable-context harness that honors waitStrategy; a stale heartbeat against a healthy substrate stops on the first iteration, finalizes FAILED, and calls TerminateMicrovm.

@scottschreckengaust — non-blocking findings and documentation

MicroVM poll failures never escalate — answered with evidence

Not changed, though the cost math is right: a permanent fault can produce roughly 1,020 warnings over the 8.5-hour window.

The catch already records the deliberate deferral in orchestrate-task.ts: adding the ECS-style counter introduces PollState fields that P3's suspend policy must reshape in the same area. Adding and pinning an intermediate state shape now would immediately create rework for P3. The exposure is bounded at both ends: MAX_POLL_ATTEMPTS caps the window, finalize always calls stopSession, and the heartbeat-staleness path now terminates early for the cost-relevant “healthy VM, hung guest” case — covered by the new end-to-end heartbeat → finalize → TerminateMicrovm test.

logs:CreateLogGroup on the execution role — you were right

I initially expected to push back using the construct docstring, then checked that claim against both live runbooks. Across P1, P2 run 1, and P2 run 2, exactly one /aws/lambda-microvms/* group ever existed: the CloudFormation-created /aws/lambda-microvms/<image-name> group. Both build and guest-runtime lines landed there. The P2 post-run inventory records no groups after stack deletion, and its “service-vended outside CloudFormation” list names only AgentCore and classic Lambda groups — not per-MicroVM groups.

The grant is therefore split in baa365fc: the build role keeps logs:CreateLogGroup (service-documented, and the P1 failed-build diagnosis demonstrates why build logs are load-bearing); the execution role, which runs untrusted repository code, now has only CreateLogStream / PutLogEvents. The docstring carries the runbook evidence and a re-verify note for the pending clean run, including the exact AccessDenied logs:CreateLogGroup symptom that would require widening it again. Bootstrap regeneration remains byte-identical because this is a stack role, not a bootstrap policy.

Drift-regex 240.0 claim — answered with evidence; valid halves fixed

The specific claim does not reproduce:

1. No _READY_WARMUP_TOTAL_BUDGET_SECONDS: float = 240.0 line exists. The code is contract-sourced: _READY_WARMUP_TOTAL_BUDGET_SECONDS: int = _HOOK_BUDGETS["warmup_total_budget_seconds"]; there are zero 240.0 occurrences across the consumers.
2. The actual regex already catches an unannotated NAME = 240.0: \d+ consumes 240, and \b is satisfied before .. The narrower hole was an explicitly annotated NAME: float = 240.0, because the annotation group accepted only int.

That real hole is fixed by widening the annotation group to (?:int|float). The valid test-gap half is also fixed: new cdk/test/scripts/check-constants-sync.test.ts has 23 spawn-based tests against a throwaway repository, including both float forms, scalar/collection/string/TypeScript literal rejection, false-positive protection for contract-sourced assignments, hook-budget invariants, and the new ARN-pinning contract.

Remaining non-blockers

• Payload retention: finalize-time MicroVM delete added, with same-task-key and best-effort tests.
• ADR CSPRNG requirement: re-scoped to P3 with the exposure analysis recorded at all four relevant ADR sites; mirrors regenerated.
• FINALIZING arm: retained and documented defensively; RUNNING-only reachability is now test-pinned.
• _debug_cw_failures: confirmed pre-existing, incremented and never read. Follow-up filed as chore(agent): expose or remove the _debug_cw_failures counter — incremented, never read #810: chore(agent): expose or remove the _debug_cw_failures counter — incremented, never read #810
• /validate secret warning: now logged through _build_hook_log on both 200/503 paths, names only.
• Producer attribution: corrected to buildMicrovmPlatformConfig in the strategy.
• Heartbeat on task summary: added to both API/CLI TaskSummary types, toTaskSummary, and a HEARTBEAT column in bgagent list; type-sync passes.
• PR description: reconciled to the completed run-2 evidence; what remains is the clean no-workaround run after re-bootstrap.
• Operator docs: Deployment guide now covers the experimental backend, not-for-production posture, supported-Region check, and mandatory re-bootstrap to bundle ≥1.4.0; User guide's compute_type list updated; mirrors are idempotent.
• Account ID: scrubbed to the repository placeholder; the live role-name truncation evidence remains intact.

Validation

• mise run build — exit 0
• CDK: 184 suites / 3,939 tests
• CLI: 56 suites / 758 tests
• Agent: 1,640 tests / 83.23% coverage
• mise run drift-prevention — exit 0
• mise //docs:sync — idempotent
• mise //cdk:bootstrap:generate — zero diff
• CDK + CLI eslint --fix — zero mutation
• committed-range secrets scan — clean

Model-config overlap (#740–#747)

@scottschreckengaust thanks for the heads-up and the live Bedrock checks. Landing this PR as-is and letting #746 rebase, per your default assumption. The geo resolver belongs with the change that needs it rather than speculatively here. Two details for #746: this backend's platform_config.anthropic_default_haiku_model is the third propagation site, and that block is contract-sourced on both sides with check-constants-sync forbidding literal re-declaration — the geo prefix should become a derived value in the contract consumer rather than a strategy-local find-and-replace.

[dreamorosi]

Fifth main merge absorbed in 7bdfb728: 18 commits, including the #764/#768 model-config stack. The Haiku profile now derives through upstream's resolveBedrockGeoRegion, closing the "third grant site" from comment 5259759127 as a derivation.

Bootstrap is now 1.6.0 because upstream took 1.4.0 and 1.5.0. This corrects earlier replies that said 1.4.0. The current PR body contains no remaining 1.4.0 references.

The merge also exposed a pre-existing ECS gap: it never delivers ANTHROPIC_DEFAULT_HAIKU_MODEL, so non-US deployments retain the us. fallback. The haikuInferenceProfileId JSDoc records it, and follow-up #811 is filed: #811

[dreamorosi]

Second self-commissioned pre-review of the response delta found 8 blockers: 6 comment/doc accuracy issues and 2 behavioural defects.
The behavioural fixes widen platform-config rejection to all C0 controls + DEL, and classify guest /run 4xx failures as non-retryable CONFIG faults instead of inviting a retry loop.
All 8 are fixed in 8050d088, with +14 tests.
The PR body is refreshed with current counts, the mandatory re-bootstrap ≥1.6.0 warning, and the corrected weekly-cron-only security:sast statement.
Follow-up: registry MCP asset portability on lambda-microvm is tracked in #818.

[isadeks]

Approving on verification

I independently verified baa365fc against all ten blocking findings from the two prior reviews — by execution rather than by reading the replies, and mutation-testing every fix that claims a test pins it.

Genuinely closed, and pinned:

• B1 (stateReason discarded) — rendered the runbook's verbatim hook-400 value end-to-end; three separate mutations each fail the suite. The Success. normalization is exact-equality, so it cannot swallow a useful reason.
• B2 (required-key throw misclassified) — extracted and ran the real classifier regexes against the actual Session start failed:-prefixed string that reaches failure-reply.ts: CONFIG / SERVICE / retryable: false, entry correctly ordered above the catch-all, and the remedy names neither AgentCore nor ECS. Reverting wrapMicrovmError reproduces the original misclassification exactly. Adopting @ayushtr-aws's corrected framing over the auto-retry framing was the right call.
• B6 (account ID) — zero hits repo-wide including the ~4,100 lines of in-tree runbooks; the new .gitleaksignore entries suppress a fake 999988887777 fixture, not a real ID.
• B7 (inert PassRole map entries) — confirmed by experiment: deleting MicrovmPassRoles leaves the map check green with missing: [], and fails policies.test.ts (7 tests) plus bootstrap-template.test.ts:93. The corrected comment names the real guards accurately.
• N2 fix — the no-config path now rejects on the effective environment while a genuinely legacy baked image still runs; both halves verified.
• Eleven nits, including the check-constants-sync test file that previously did not exist.

The boto3.DEFAULT_SESSION result reproduced cleanly on my side too (botocore 1.43.78, clean-room per @scottschreckengaust's warning — all AWS_* purged, config to /dev/null, logs not S3): region us-west-2→eu-west-1, UA build-app→runtime-app, credentials memoized to AKIAbuild. The narrowing to credentials is correct, and the credential half is the load-bearing one.

Follow-ups filed as #817, to be picked up across the remaining PRs in the ADR-021 stack: four findings closed in code but not in their docs, two defects introduced by the fix commit, and one missing test. Deferring is reasonable because lambda-microvm is experimental and carries a non-suppressible synth warning keeping production on agentcore/ecs — nothing there sits on a production path.

Two items are worth naming here rather than leaving buried in the issue, because they are near-misses on things this PR did deliberately well:

1. The appended stateReason hijacks an earlier classifier pattern (#817 item 1). The regional-availability entry (error-classifier.ts:208) matches "microvm" within 60 chars of "unavailable", and it sits above the substrate entry — so …substrate state completed (MicroVM host unavailable.) classifies as "Lambda MicroVMs is not available in this Region" with retryable flipped true→false. A transient host fault is reported as non-retryable in a Region where a VM demonstrably just ran. Re-checked at 8050d088: still present — the new hook-4xx entry sits above the substrate entry but below the regional one. #817 carries a verified one-line lookahead fix (4 hijacks reclassify, 4 real Region errors still match, 229/229 pass) plus the reorder alternative. Both are stopgaps; the durable fix is to keep stateReason as display text and never feed AWS-owned free text to the classifier. Worth noting the ordering reasoning in this PR is otherwise careful and correct — the collision is with a sibling above, while both ORDERING: comments reason about the catch-all below.

2. deleteMicrovmPayload has no s3:DeleteObject grant (#817 item 2). task-orchestrator.ts:528 has grantPut only where the ECS sibling at :517-518 has both; confirmed by synthesizing the real --context compute_type=lambda-microvm stack. Every delete AccessDenieds and is swallowed as best-effort, so the ~24h window stays open while three comments state the delete is deliberately absent. Heads-up that this one fails CI on contact: task-orchestrator.test.ts:816 and agent.test.ts:1190 both assert not.toContain('s3:DeleteObject').

One correction to the record, offered because the reasoning is now near a code comment: the reply that an unscoped run "dies on the first table write" does not hold. agent/src/task_state.py:43-58 returns None when TASK_TABLE_NAME is unset and every writer early-returns — with all four required vars popped, write_submitted/write_running/write_terminal all return normally with no AWS call. @scottschreckengaust's original framing was right. Harmless, because the N2 fix makes the scenario unreachable — flagging only so the table write is not later mistaken for the safety net and the real guard at server.py:2090 removed.

The live-evidence discipline, the self-retracting ADR, and the poisoned-seam suite are all above the bar for this repo, and the review response held up under adversarial checking better than most. Nice work @dreamorosi.

Note for @scottschreckengaust and @ayushtr-aws: your reviews are still recorded against 08b30917, several commits back, so the PR remains blocked pending your re-review — the substance of B1–B7 and N1–N3 is addressed as detailed above.

review has been addressed