[nborges-aws]

Description

This change addresses failures in the archive-lifecycle e2e tests. Previously, batch evaluation test could timeout before reporting its status or ID. This created a chain of misleading failures in tests dependent on that ID.

This PR updates the batch evaluation call to omit --wait, instead returning immediately, and polling its status in a separate call. The immediate return lets us capture ID early, and unblocks the following code. Additionally, updated logic to wait for a recommendation to reach terminal state before archiving, and skip archive checks when the batch eval ID is not available.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Other (please describe):

Testing

How have you tested the change?

• I ran bun test
• I ran the relevant end-to-end tests with bun run test:e2e, or explained why they are not applicable
• I ran bun run typecheck
• I ran bun run lint:check
• I ran bun run format:check
• I ran bun run build
• If I modified src/assets/, I updated affected snapshots with bun test <test-file> --update-snapshots and committed them

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[github-actions]

Package Tarball

aws-agentcore-0.31.1.tgz

How to install

gh release download pr-2712-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.31.1.tgz

[agentcore-devx-automation]

AgentCore Harness Review

Verdict: Changes requested

The approach (start the job without --wait, capture the ID immediately, poll view <type> <id> --json with a bounded deadline) is the right fix for the "timed out before reporting status/ID" failure mode, and the ctx.skip() gating matches the existing pattern in e2e-tests/insights-lifecycle.test.ts. Two things should be adjusted before merge, though — one of them reintroduces the exact failure the PR is trying to eliminate.

1. The 8-minute wait can still be cut off by the Vitest test timeout (no diagnostics)

e2e-tests/archive-lifecycle.test.ts — "waits for batch evaluation to finish and captures the ID" keeps a 600 000 ms test timeout, but the body is now retry(..., 6, 15000) plus waitForJob(..., 480000):

• worst successful retry path: 5 failures × (~5–15 s call + 15 s backoff) + final call ≈ 130–150 s
• then up to 480 s of polling → ~610–630 s > the 600 s test timeout

The run batch-evaluation start is known to need several retries because of log-group/trace eventual consistency (the analogous step in insights-lifecycle.test.ts uses retry(18, 10000)), so this isn't an unlikely tail case. When Vitest kills the test at 600 s, you lose the carefully built waitForJob error message (last status / resultsFetched / error) and are back to a bare "Test timed out in 600000ms".

The same arithmetic applies to "archives the recommendation with --json flag" (line ~345): 480 000 ms of polling plus the archive call inside a 600 000 ms budget, where the archive call alone previously had a 120 000 ms budget.

Options:

• Raise the per-test timeouts so they comfortably exceed retry budget + jobWaitTimeoutMs (e.g. 900 000–1 200 000 ms for both tests).
• Lower jobWaitTimeoutMs (e.g. 300 000 ms) so retries + wait stay well under 600 s.
• Start the waitForJob deadline clock before the retry loop and derive both from a single budget constant that is strictly less than the test timeout.

2. Terminal-status lists are hand-rolled and have drifted from the engine's

waitForJob is called with ['COMPLETED','COMPLETED_WITH_ERRORS','FAILED','STOPPED','CANCELLED'] for batch evaluations and ['COMPLETED','FAILED'] for recommendations. The engine's source of truth (src/cli/operations/jobs/shared/constants.ts, TERMINAL_STATUSES) also includes NOT_FOUND for both types and SUCCEEDED / DELETING for recommendations. If a record lands in any of those states (e.g. the refresh GET 404s and the record is persisted as NOT_FOUND), the engine considers it settled and will never move again, but the test will spin for the full 8 minutes and then fail with a misleading timeout error.

Fix: import TERMINAL_STATUSES (or isTerminal) from src/cli/operations/jobs and index it by type instead of duplicating the lists — this file already imports from src/. Or, at minimum, add NOT_FOUND (and the defensive recommendation statuses) to the inline lists.

Minor notes (non-blocking)

• Gating the batch-eval archive on batchEvaluationReady (only set after the COMPLETED|COMPLETED_WITH_ERRORS|STOPPED assertion) means a FAILED evaluation now skips the archive step entirely, so the service-side job is never deleted and afterAll doesn't clean it up. Previously any captured ID got archived. Gating on batchEvaluationId while keeping the status assertion in the wait test would preserve cleanup, since FAILED is still terminal and archivable.
• Dropping --wait here leaves run batch-evaluation --wait without e2e coverage (--wait is now only exercised for insights and recommendation). The shared waitForTerminal path is still covered, so this is probably acceptable — just calling it out.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	41.75%	16052 / 38446
🔵	Statements	41%	17125 / 41767
🔵	Functions	35.72%	2764 / 7736
🔵	Branches	34.78%	10738 / 30871

Generated in workflow #4596 for commit 84a97f0 by the Vitest Coverage Report Action

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[agentcore-devx-automation]

AgentCore Harness Review

Verdict: Changes requested

Restructuring this test (start without --wait, capture the ID immediately, then poll view <type> <id> --json) is the right fix, and the two blocking items from the previous review are addressed: TERMINAL_STATUSES/NOT_FOUND_STATUS are now imported from the engine instead of hand-rolled, and batchEvaluationArchivable is set before the status assertion so a FAILED/CANCELLED evaluation still gets archived. Remaining concerns are about the wait budget and cleanup on failure.

1. jobWaitTimeoutMs = 300000 is likely shorter than a real batch evaluation (archive-lifecycle.test.ts:31, test timeout at :188)

The failure this PR is fixing came from --wait being inside the retry(6, 15000) loop within a 600 000 ms test (#2499), i.e. the batch evaluation did not reach a terminal state within roughly 7 minutes of available budget. The new structure gives the job only 300 000 ms (5 min) — less wall-clock time for the job itself than the arrangement that was already timing out. For comparison, e2e-tests/insights-lifecycle.test.ts:394 allots 1 500 000 ms for the analogous run insights --wait job (insights goes through the same batch-evaluation API).

If the evaluation genuinely needs 6–10 minutes, this test still fails — just with a better message — and now also skips the archive step (see #2). Options:

• Raise the per-test timeout for "waits for batch evaluation to finish and captures the ID" (and the recommendation archive test) to something comparable to insights-lifecycle (e.g. 1 200 000–1 500 000 ms) and raise jobWaitTimeoutMs to fill most of it (retry budget is ~165 s worst case: 5 × 15 s backoff + 6 start calls).
• Derive the wait deadline from a single budget constant: start the clock before the retry loop and poll until testTimeout - slack, so the wait always consumes whatever the retries did not.
• If the goal is only to exercise archive, drop the terminal-status requirement for batch-evaluation and archive whatever state the job is in (only viable if DELETE /evaluations/batch-evaluate/{id} is accepted for a running job — the handler comment says recommendation archive is the cancel, but batch-evaluation has a separate Stop, so this needs verifying).

2. A failed/timed-out wait now leaks the service-side job

batchEvaluationArchivable is only set after waitForJob returns (line 185), and the recommendation archive calls waitForJob before archive (lines 345–348). If the wait times out or throws, the archive never runs, so the batch evaluation / recommendation is never deleted on the service, and afterAll (lines 76–81) only tears down the CDK stack — it does not clean up job records. Every CI run that hits the timeout leaves a job behind.

Suggest a best-effort cleanup in afterAll: if batchEvaluationId/recommendationId were captured and the corresponding *Archived flag is false, fire archive <type> --id <id> --json and ignore the result.

3. A single transient view failure aborts the whole wait (lines 92–95)

waitForJob throws as soon as view exits non-zero. engine.get() can fail for transient reasons (region detection, credential/STS blip, throttling), and one such blip now ends the wait and skips the archive instead of retrying — the opposite of the flake-reduction goal this PR is after. Consider tolerating non-zero exits inside the loop (record lastError/stdout, delay, continue) and only failing at the deadline, with the last observed failure in the message. Keep the hard throw for the persisted record.error case (line 101) — that one is genuinely terminal, since the engine only writes error after refresh retries are exhausted and isTerminal() then stops refreshing.