Repository navigation
feat(export): allow harness exports outside a project - #2542
aidandaly24 wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
I did not find any blocking issues. A few observations worth noting (non-blocking):
-
Partial-state cleanup after project creation (edge case). In
src/core/project/manager.tsx(around L928–L1114), onceyield* this.create({ name: projectName })succeeds, a subsequent failure inside the export body only runscleanupAgentDir()+envFile.rollback(). The newly-created project directory itself is left on disk. In practice, the steps aftercreate()(template render → spec parse → write) are unlikely to fail — invalid inputs and service failures are all caught beforehand, and the tests cover those pre-creation paths well — so this is a rare case. If you want to tighten it, you could either (a) track whether the project was created in this call andrm -rfthe destination on failure, or (b) run project creation last after you have a validated plan in hand. Up to you. -
AgentCoreRegionSchemais a strict enum. If a future/unsupported region appears in the source ARN,AwsDeploymentTargetsSchema.parse(...)at L914 will throw after theGetHarnessfetch has already happened. That's beforethis.create(...), so there's no partial state, but the user sees a validation failure after a successful service call. Probably fine sinceGetHarnesswould also fail in a region AgentCore doesn't support, but worth noting.
The test suite looks solid — real temp dirs, mocking kept at the harness SDK boundary, and good coverage of the "did we create a project when we shouldn't have" cases.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
e34157c to
31ecb98
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2542 +/- ##
=========================================
Coverage 97.44% 97.45%
=========================================
Files 644 644
Lines 47688 47759 +71
=========================================
+ Hits 46471 46543 +72
+ Misses 1217 1216 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Allow
agentcore export harness --arnoutside a project by creating a project through the existing bootstrap flow. Add--project-name, deployable default names, safe destination refusal and prefetch before creation. Newly created projects default deployment targets to the source ARN's account and region; existing project targets remain unchanged.Scope: eight files only. No IAM capture, preservation schema/assets, backend or CDK capability changes. The existing export semantics and vended
@aws/agentcore-cdk1.0.0-rc.3remain unchanged.Separate preservation work: #2532 and its CDK companion https://github.com/aws/agentcore-l3-cdk-constructs/pull/415. All three remain drafts; this PR does not require that unpublished capability.
Related Issue
N/A. The user explicitly requested no new issues.
Documentation PR
Included here: Harness configuration guide and generated command reference.
Type of Change
Testing
Prior scoped TEST-LOCAL gate at
6e1a8d52a60a41d92e3664e2d0ad79bb509607be: 211 focused tests passed; typecheck, lint, format, secrets and build passed. No checks were rerun during SHIP.Supplemental full suite: 3889 pass / 101 fail / 3 errors in unchanged environment/IO paths; not a full pass. No fresh AWS validation for this split. Historical results are not claimed as live validation of this head; resource-preservation validation limits belong to #2532.
Temporary combined gate with #2532: 336 focused tests and type/lint/format/secrets/build passed after manually resolving four conflicts: the Harness guide,
harness.test.ts,harness.tsand projecttypes.ts. Resolution was only in a temporary clone. Future integration needs manual resolution and validation; automatic merge readiness is not claimed.bun test(focused gate above; full-suite failures disclosed)bun run test:e2e, or explained why they are not applicable (no fresh live run in this split)bun run typecheckbun run lint:checkbun run format:checkbun run buildsrc/assets/, I updated affected snapshots withbun test <test-file> --update-snapshotsand committed them (assets unchanged)Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.