feat: add a project command menu to the TUI - #2143
Open
notgitika wants to merge 4 commits into
Open
Conversation
`project` was listed in the root TUI menu with no route to match it, so selecting it fell through to the catch-all HelpScreen — which prints the launching command's help and exits, leaving a blank frame and no TUI. Adds the project menu and routes its seven subcommands explicitly. None has a screen yet, so each reports NotImplementedError via Ink's exit(error): the TUI tears down, the terminal is restored, and the error travels the normal CLI path with an exit code.
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
The PR does exactly what it says: it plugs the hole where agentcore project [subcommand] in the TUI used to fall through to the catch-all HelpScreen (which prints help and exits, leaving a blank frame), replacing it with an honest NotImplementedError that tears the TUI down via useApp().exit(error) so renderTuiAt's awaited waitUntilExit() rejects and the CLI's normal error path handles it. The reasoning is captured well in the code comments.
Things I checked:
PROJECT_COMMANDSinsrc/components/Root.tsxmatches the subcommands registered insrc/handlers/project/index.ts(create, add, remove, dev, deploy, status, build).NotImplementedErrorextendsAgentCoreCLIErrorand defaultsexitCodeto 1, so the test assertion onexit code === 1is valid.- The
ttyTestIOextraction tosrc/testing/testIO.tsxis a clean move, and the previous inline copy insrc/tui/tui.test.tsxwas replaced by the import. Defaults (columns=100,rows=40) preserve the original behavior. - Test coverage is appropriate: menu listing, reachability from root, esc back, and one test per subcommand plus a dedicated assertion on the error message and exit code.
A couple of nits I'll mention but don't consider blockers:
PROJECT_COMMANDSduplicates the router's subcommand list. If someone adds a project subcommand later without updating this array, the new command will silently fall through toHelpScreenagain. Fine as an interim, but worth being aware of; if you want to be extra defensive you could derive the list at runtime from the Commander tree the wayRouterScreenalready does, or add a small sanity test that walks the project router and asserts every subcommand has a route.useEffect(() => { exit(...) }, [exit, command])slightly diverges from the sibling pattern inHelpScreen(useEffect(exit, [])). Functionally equivalent sinceexitis stable andcommandis a route prop, but consistency with the neighboring file wouldn't hurt.
No changes required from my side. Ship it.
notgitika
force-pushed
the
feat/project-tui-menu
branch
from
August 28, 2026 21:35
51ef172 to
c1e480b
Compare
Contributor
|
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 #2143 +/- ##
=========================================
Coverage 97.29% 97.29%
=========================================
Files 478 479 +1
Lines 29642 29673 +31
=========================================
+ Hits 28840 28871 +31
Misses 802 802 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
The project router had no default handler, so `agentcore project` printed Commander help and exited 2 instead of opening the menu like every sibling router. Registers renderTui as its default and threads Core through createProjectHandler. Drives the subcommand tests off the compiled router rather than a hardcoded list, so they cover any subcommand added later and fail if one is missing a route in Root. Adds coverage for the bare entrypoint, which the path-mounting screen tests could not catch.
notgitika
marked this pull request as ready for review
August 28, 2026 21:55
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Ink only writes frames incrementally when interactive (`!isInCi && isTTY`), so waiting on rendered menu text passed locally and timed out on every CI platform. Asserts renderTui's TTY guard instead, which reaching at all proves the group routed to the TUI rather than to Commander help.
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes
projectnavigable in the TUI. adds the project menu and routes its seven subcommands, each throwing aNotImplementedErroruntil it gets a screen of its own. First PR toward aproject createwizard.Ran all CI checks. Tested the build locally.
And when I clicked on
createfor instance, I see this errorbun run /dist/index.js projectalso opens up this wizard