Repository navigation
feat(dev): support harness dev and Inspector chat - #68
Hweinstock wants to merge 12 commits into
Conversation
d339f71 to
81e1603
Compare
| io.stderr.write(`${message}\n`); | ||
| } | ||
|
|
||
| const devFlags = [ |
There was a problem hiding this comment.
lets inline these below in the handler.
| { group: "Runtime Options:" }, | ||
| ), | ||
| flag("traces", "disable local OTEL trace collection", z.boolean().default(true), { | ||
| group: "Runtime Options:", |
There was a problem hiding this comment.
lets make the group string a constant for both harness and runtime.
| config.io.stderr.write("Shutting down…\n"); | ||
| controller.abort(new UserCancellationError()); | ||
| }; | ||
| if (isHarnessDev(ctx.require(ProjectKey), flags.agent)) { |
There was a problem hiding this comment.
I think we can simplify the control flow. Lets have a function resolveAgentSelection that returns a { type: 'harness', harnesses: [...]} | { type: 'runtime', runtimes: [...]} to drive this flow. then both flows here shouldn't have to re-derive the agents they need to run on.
| ctx: Context, | ||
| flags: DevFlags, | ||
| ): Promise<void> { | ||
| const unsupported = |
There was a problem hiding this comment.
lets define a list of unsupportedFlags and use that to verify. avoid ambiguous variable names like unsupported.
| json, | ||
| ); | ||
| }, | ||
| const target = flags.target ?? DEFAULT_TARGET_NAME; |
There was a problem hiding this comment.
this is deploymentTarget.
| await createDeployProjectHandler(config).handle(ctx, { target, yes: flags.yes }, {}); | ||
| } | ||
| if (flags.mode === "headless") { | ||
| if (flags["skip-deploy"] || ctx.require(JsonKey)) { |
There was a problem hiding this comment.
avoid double nesting conditions. Combine nested ifs when possible to make it more readable.
| : project.spec.harnesses, | ||
| }, | ||
| }; | ||
| const controller = new AbortController(); |
There was a problem hiding this comment.
lets use withUserCancellation Here and update it to handle the additional signal. We can do the same for runtime.
| if (flags.traces && runtimes.some((runtime) => runtime.instrumentation?.enableOtel ?? true)) { | ||
| const tracesDirectory = join(project.rootPath, "agentcore", ".cli", "traces", "otlp"); | ||
| let tracePersistErrorReported = false; | ||
| collector = await config.startTraceCollector({ |
There was a problem hiding this comment.
there may be some opportunities to break this function up into smaller digestible pieces. the collector logic for example seems like a good candidate.
| controller.abort(new UserCancellationError()); | ||
| }; | ||
| if (isHarnessDev(ctx.require(ProjectKey), flags.agent)) { | ||
| await runHarnessDev(config, ctx, flags); |
There was a problem hiding this comment.
i think we can validate the flags on this level within the if statement. I.e. if runtime flags are passed, fail before calling harness dev, and vice versa for harness only flags.
| } | ||
| } | ||
|
|
||
| async function* transformHarnessSse( |
There was a problem hiding this comment.
is there logic we can re-use from the harness invoke path?
| } | ||
| } | ||
|
|
||
| export function harnessStreamError(event: InvokeHarnessStreamOutput) { |
There was a problem hiding this comment.
if this function isn't used outside the file, lets not export it.
| async function runHarnessDev( | ||
| config: DevProjectHandlerConfig, | ||
| ctx: Context, | ||
| flags: DevFlags, |
There was a problem hiding this comment.
rather than adding a type for dev flags and passing them all down, can we pass them down in an options object?
| } | ||
|
|
||
| async function startRuntimeTraceCollector( | ||
| config: DevProjectHandlerConfig, |
There was a problem hiding this comment.
for any function taking 3+ parameters, have it take a props object with nested fields.
| @@ -1,5 +1,12 @@ | |||
| import { describe, expect, test } from "bun:test"; | |||
There was a problem hiding this comment.
are all these tests necessary? there are most test changes than service code. Then tests only need to ensure we cover the cases outlined in the spec with as few tests as possible (no need to double cover).
| name: "dev", | ||
| description: "run the project locally for development", | ||
| description: | ||
| "test changes made to project resources. For Runtime, this is done with a local server. For Harness, this is an alias for deploy.", |
There was a problem hiding this comment.
lets change this comment to make it more concise, and mention that there is agent inspector as well. Keep the same info, just make it more concise and add the agent inspector bit.
95a7fc0 to
50e0116
Compare
1 participant
Description
agentcore devsupports harnesses: deploy and print invoke guidance by default, or open Agent Inspector with--mode browser.--skip-deployreuses deployed harnesses.Solution
Related Issue
Related to aws#1376 (closed) and prior work aws#2525.
Documentation PR
Concise README guidance; command reference generated on release.
Type of Change
Testing
Verification
Evidence
Recorded during the earlier feature verification. Long deployment waits are hidden; the TUI clip ends after dev prints invoke guidance.
CLI browser mode: deploy, serve Inspector, and invoke the harness through Inspector.
TUI default headless mode: select dev, deploy, and print invoke guidance.
CLI VHS tape
TUI VHS tape
Harness AWS E2E log — 15 passed, including cleanup
Earlier runtime AWS E2E log — 38 passed, including cleanup
Harness E2E verified commit
81e16038cusing the compiled binary. Local paths are redacted; all test results and timings are retained.Follow-up compiled-binary smoke — skip-deploy and SIGTERM cleanup
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Notes
--agentlimits the harnesses shown in Inspector.