Repository navigation
fix(project): align user-facing copy style - #2622
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
The PR is a user-facing copy consistency pass, but it misses a few spots that are emitted into the same output streams as messages this PR just lowercased. The remaining capitals will show up right next to the lowercased ones at runtime, which is exactly the inconsistency the PR is trying to fix.
Issues
-
Built projectleft capitalized —src/handlers/project/build/index.ts:16return `Built project '${project.name}'`;
This is the summary line printed by the build handler, parallel to the
deployed project/removed projectlines insrc/handlers/project/deploy/index.ts:38-42that this PR did lowercase. Right nowproject.test.ts:1118still asserts"Built project 'MyAgent'"andbuild/index.test.ts:72,84assert"Built project 'orders'"— inconsistent with the deploy handler’s new wording. Please also lowercaseBuilt project→built projectand update the three build test assertions. -
CDK credential-provider step messages left capitalized —
src/core/project/backends/cdk/credentials.ts:170, 267, 305yield { type: "step", message: `Removing credential provider '${provider}'` }; yield { type: "step", message: `Preparing credential provider '${provider}'` }; yield { type: "step", message: `Removing credential provider '${provider}' this deploy created` };
These step events are interleaved with the ones in
cdk.ts(which were lowercased), so a single deploy now emits a mix like:✓ synthesizing CloudFormation templates ✓ Preparing credential provider 'openai-key' ✓ deploying AgentCore-example-default-0You can see the mismatch cleanly in
cdk.test.tsafter this PR, e.g. line 831/940:messages.indexOf("removing stack ...").toBeLessThan(messages.indexOf("Removing credential provider 'wallet'"))
Please lowercase those three messages (and update the matching assertions in
cdk.test.ts).
Optional, in the same spirit
If this pass is meant to be exhaustive, there are a few other step messages still starting with a capital that will show up alongside the lowercased ones in other flows: src/core/policy.tsx:51,64,77,102, src/cliVersionManager/manager.ts:102, and src/handlers/project/migrate/index.ts:115. Up to you whether those are in scope — flagging only so a follow-up isn’t needed.
Everything else in the diff looks like a straightforward and consistent rename.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2622 +/- ##
============================================
- Coverage 96.61% 96.61% -0.01%
============================================
Files 702 702
Lines 58347 58346 -1
============================================
- Hits 56373 56371 -2
- Misses 1974 1975 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jariy17
left a comment
There was a problem hiding this comment.
Pretty good, look at the failing unit tests and the suggestions from harness reviewer.
79b5a4d to
773962b
Compare
|
Addressed the two actionable Harness findings in 773962b: lowercased the build success message and credential-provider progress messages, with their assertions updated. I left the optional policy/update/migrate copy suggestions out of scope for this PR. I also investigated the macOS timeout. The reported online-eval test passes 20/20 in isolation; a normal local full-suite run later stalled on a different untouched test under host load, so this is suite/runner contention rather than a defect in that test. I did not add a timeout workaround. Fresh CI is running on the rebased branch. |
|
Claude Security Review: no high-confidence findings. (run) |
Summary
dev,deploy,status, andharness--modeflag from thedevdescriptionThe root menu descriptions now read:
dev: develop and test project agentsdeploy: build and deploy project resources to an AWS targetstatus: inspect deployed project resources for an AWS targetharness: manage config-defined agents and their versionsContext
This addresses cosmetic feedback from the CLI v1 bug bash. User-facing task labels and short descriptions follow the lowercase microcopy style, while full diagnostic error sentences remain unchanged.
Related Issue
Bug-bash feedback; no GitHub issue was filed.
Documentation PR
Not applicable; this changes CLI and TUI copy only.
Type of Change
Testing
bun test— 4,687 tests passedbun run typecheckbun run lint:checkbun run format:checkbun run buildChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.