fix(create): use harness-first language when project already exists#1594
fix(create): use harness-first language when project already exists#1594tejaskash wants to merge 1 commit into
Conversation
When a user runs `agentcore create` inside an existing project, the existing-project-error screen now leads with `add harness` and offers `add agent` as the code-based alternative, instead of pointing only at `add agent`. Part of the harness GA cutover (single CLI, harness as a first-class create option). Unconditional — no isPreviewEnabled() branch — matching the post-merge GA state.
|
Claude Security Review: no high-confidence findings. (run) |
Package TarballHow to installgh release download pr-1594-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.20.1.tgz |
Coverage Report
|
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Tiny, focused message change — implementation is correct. My only concern is the ordering dependency you've flagged in the PR description: the PR points users at add harness unconditionally, but on main that command is still preview-gated, and a release from main runs npm run build (not build:preview) so __PREVIEW__ = false and harnessPrimitive ends up undefined (registry.ts:26). A GA user hitting the existing-project-error screen would be told to run a command that isn't registered in their build. Left an inline comment with options.
| <Box marginTop={1} flexDirection="column"> | ||
| <Text> | ||
| Use <Text color="cyan">add agent</Text> to create a new agent in the existing project. | ||
| Use <Text color="cyan">add harness</Text> to add a harness to the existing project. |
There was a problem hiding this comment.
The PR description correctly calls out that this is only safe once add harness is ungated, but the change as written doesn't enforce that — the message renders the same regardless of __PREVIEW__. Today on main:
src/cli/primitives/registry.ts:26—harnessPrimitive = isPreviewEnabled() ? new HarnessPrimitive() : undefinedsrc/cli/cli.ts:131-132only registers commands for primitives inALL_PRIMITIVES, soadd harnessis not registered in GA builds.github/workflows/release.ymlrunsnpm run build(GA,__PREVIEW__ = false) when releasing frommainto npmlatest
So if this PR merges to main and a latest release happens before harness is ungated, GA users will land on the existing-project-error screen, follow the instruction to run agentcore add harness, and hit error: 'harness' is not a valid subcommand.
A few ways to address:
- Gate the message on
isPreviewEnabled()— show the new harness-first wording only in preview builds, keep the existing "Use add agent…" wording in GA. When the harness ungating PR lands and removesisPreviewEnabled()fromregistry.ts, it can also remove this branch and leave just the new message. Most defensive option, makes this PR independently safe to merge. - Hold the PR and merge it as part of (or immediately after) the harness ungating PR, with a release-branch coordination so a
latestbuild never goes out with this message but withoutadd harnessregistered. - Add a guard at registration time so the message is sourced from whether
harnessPrimitiveis defined (e.g. importharnessPrimitiveand conditionally render the harness line). Same effect as option 1 but keyed off the actual registration rather than the build flag.
Option 1 or 3 would let this land independently of release sequencing.
Problem
When a user runs
agentcore createinside an existing project, the existing-project-error screen tells them only:As part of the harness GA cutover, the create experience is harness-first. This message should lead with
add harness.Change
One file —
src/cli/tui/screens/create/CreateScreen.tsx(theexisting-project-errorphase). The message now reads:Unconditional — no
isPreviewEnabled()branch — matching the post-merge GA state where there's a single CLI and harness is a first-class create option. This is consistent with the harness-first voice already used in the create-type picker in the same screen (Harness (recommended)).This is the only place this message exists; the CLI flag path (
validate.ts) only checks folder-name collisions and has no equivalent message.Addresses the "harness-first language" action item (#4) in the Harness GA plan.
add harnessis still preview-gated today:This message is only correct once harness is ungated for all builds — i.e. it must land with or after the
__PREVIEW__/isPreviewEnabledremoval work. It should not ship to thelatestnpm tag before that gating is removed, or a GA build would point users at anadd harnesscommand that doesn't exist in that build.Testing
npm run typecheck✅eslint(pre-commit) ✅ / prettier ✅create.test.ts— 16/16 passcreatefrom inside an existing project.