feat(project): wire dev handler - #1966
Conversation
GitHub Actions
Failure
github-actions
Bot
added
agentcore-harness-reviewing
736c095 to
3d6dd24
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #1966 +/- ##
============================================
+ Coverage 96.91% 96.94% +0.02%
============================================
Files 356 359 +3
Lines 20136 20361 +225
============================================
+ Hits 19515 19739 +224
- Misses 621 622 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3d6dd24 to
d003ce7
Compare
d003ce7 to
3b72b9c
Compare
3b72b9c to
03f5223
Compare
03f5223 to
b7c2b24
Compare
ac10a56 to
17d2047
Compare
126d77b to
c55046e
Compare
c55046e to
37312c9
Compare
37312c9 to
73ab887
Compare
Hweinstock
left a comment
Hweinstock
left a comment
There was a problem hiding this comment.
only blocking comment is the direct process kill.
| }); | ||
|
|
||
| await expect(collect(runner.run(input(root, runtime({ buildContextPath }))))).rejects.toThrow( | ||
| "container build context must be within the project root", |
There was a problem hiding this comment.
should we validate the error types in these tests?
| if ( | ||
| relativeContext === ".." || | ||
| relativeContext.startsWith(`..${sep}`) || | ||
| isAbsolute(relativeContext) |
There was a problem hiding this comment.
in what cases does relative return an absolute path?
| ); | ||
| const hasAwsConfig = existsSync(this.awsDirectory); | ||
| if (!hasAwsCredentials && !hasAwsConfig) { | ||
| throw new InputValidationError( |
There was a problem hiding this comment.
do you think it would be useful to distinguish the missing credentials case from the invalid inputs case in telemetry with a separate error type here?
| }; | ||
|
|
||
| function portInUse(port: number, suffix = ""): InputValidationError { | ||
| return new InputValidationError( |
There was a problem hiding this comment.
would it be simpler to make this function its own error type?
| } | ||
|
|
||
| export class RuntimeInvokeInterruptedError extends AgentCoreCLIError { | ||
| export class CommandInterruptedError extends AgentCoreCLIError { |
There was a problem hiding this comment.
I think this is doing something similar to https://github.com/aws/agentcore-cli/pull/1986/changes (haven't fully reviewed that one yet)
| if (name) { | ||
| const runtime = project.runtimes.find((candidate) => candidate.name === name); | ||
| if (runtime) return runtime; | ||
| throw new InputValidationError( |
There was a problem hiding this comment.
should this be a resourceNotFoundError or something?
| handle: async () => { | ||
| throw new NotImplementedError("agentcore project dev is not implemented yet"); | ||
| flags: [ | ||
| flag("agent", "runtime to run", z.string().optional()), |
There was a problem hiding this comment.
would using the flag agent imply it works for runtime and harness?
| }, | ||
| loadDevEnvironment, | ||
| checkPort, | ||
| forceExit: () => process.exit(130), |
There was a problem hiding this comment.
I think we want to avoid killing the process directly so that we can get telemetry here.
| }, | ||
| "main": "./dist/index.js", | ||
| "engines": { | ||
| "node": ">=20.12.0" |
There was a problem hiding this comment.
why exactly is this needed? is there a component bun is missing that we need here?
3 participants
Summary
agentcore project devruntime selection, port resolution, runner dispatch, structured output, and signal handlingwithProjectfor project discovery and actionable missing-project guidance.env.localvalues directly; containers forward host AWS variables, mount~/.awsread-only, and fail early when neither credential source existsStacks on #1962.
Verification
bun test(1107 pass)bun run typecheckbun run lint:checkbun run format:checkbun run buildproject dev, received healthy/ping, interrupted with exit 130, and verified child cleanup and port releaseproject createguidance