feat(tui): guide first-time users to project creation - #2433
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Small, focused change. The Alert component and optionHints/banner additions to RouterScreen are cleanly separated, and RootScreen's use of useQuery matches the existing pattern in ProjectGate/useProject. Tests exercise real routing and only override projectManager.resolve (the correct I/O seam), asserting on rendered frame content rather than internals — no excessive mocking. The leftArrow glyph is added in both the unicode and ASCII tables, so tests remain deterministic across terminals. This is a UI hint on top of an existing screen, so no new telemetry seems warranted.
Nothing blocking — LGTM.
Minor observations (non-blocking, take or leave):
RootScreenusesqueryKey: ["project-detected", from]whileuseProjectuses["project", from], so project resolution runs twice when the user drills into a project screen. Sharing the same key (or exposinguseProjectin a non-throwing variant) would avoid the duplicate FS read, but atgcTime: 0the cost is negligible.- If
projectManager.resolverejects, the banner is silently suppressed. Acceptable for a hint, just noting it.
|
Claude Security Review: no high-confidence findings. (run) |
8d9faf0 to
d66d792
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 #2433 +/- ##
=========================================
Coverage 97.24% 97.24%
=========================================
Files 617 620 +3
Lines 43848 43920 +72
=========================================
+ Hits 42639 42711 +72
Misses 1209 1209 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d66d792 to
a1c0c36
Compare
|
Claude Security Review: no high-confidence findings. (run) |
| this.handler(handler); | ||
| this.projectCommandNames.add(handler.name()); | ||
| } | ||
| return this; |
There was a problem hiding this comment.
Hmm I don't think this logic should be in the router. The router is supposed to be general.
There was a problem hiding this comment.
The previous impl should be fine:
createProjectHandlers(core, io).forEach((handler) => {
root.handler(handler);
});
We just need to filter the project commands in the router, which we can do since we have the context there.
d518480 to
5cb49ff
Compare
|
Claude Security Review: no high-confidence findings. (run) |
* feat(tui): grey out project commands when no project is detected * refactor(tui): share one project-detected lookup between the root banner and the menu
5cb49ff to
044cf0c
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
|
||
| let exitMessage: string | undefined; | ||
| let handoff: string[] | undefined; | ||
| const projectDetected = |
There was a problem hiding this comment.
Could we limit this project check to the root menu? right now, a malformed agentcore.json can also prevent unrelated commands like agentcore runtime list from opening
PR also includes changes from @tejaskash PR #2437
Description
Add UI hints to TUI landing page, guiding users to project creation if one cannot be auto-deteced.
← start herehint besidecreate. The root menu listscreate, the resource commands (gateway,harness,identity,memory,payment,runtime), andfeedback,config, andupdate. Project commands andevalare hidden.create.Alertand left-arrow glyph support, plus an ASCII fallbackType of Change
Testing
How have you tested the change?
bun run test(3658 pass, 0 fail)npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.