fix(tui): esc from a deep-linked harness wizard returns to the menu - #2234
Conversation
`agentcore harness create` with no flags opens the TUI directly on the wizard, so the wizard is the only history entry. Its first-step back was navigate(-1), which had nothing to pop, and esc did nothing. The wizard now takes an explicit onExit: create returns to the harness menu, update returns to the update picker.
Walk the compiled command tree, mount every screen-backed command through the real route table, press esc, and assert the header belongs to a menu above it. This is the counterpart to the command-line-only walk in CliOnlyScreen.test, so a screen added later is covered without a new test. It catches both the policy picker redirect loop (aws#2225) and the deep-linked wizard. compiledRootCommand() is extracted from the harness so tests can enumerate the tree the same way the app does.
agentcore-devx-automation
Bot
added
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice fix. Replacing navigate(-1) with an explicit onExit target correctly handles the case where agentcore harness create / agentcore harness update <id> seed the memory history with a single entry (making the previous navigate(-1) a no-op). Moving the navigation decision to the parent screen keeps HarnessWizard route-agnostic, and the new tree-walking Root.test.tsx guards against this class of regression going forward for any command with a screen.
A couple of things I considered but don't think need to change:
- For
HarnessUpdateScreen,onExitnow always goes to the picker (/agentcore/harness/update), even when the user arrived from/agentcore/harness/get/:idvia the "update" action — where a workingnavigate(-1)would have popped back to the get screen. This is a consistency trade-off (same destination regardless of entry point) and it matches the create screen's behavior, so it seems intentional. CliOnlyScreen.test.tsxstill has its own localcompiledRoot()helper that duplicates the newly-exportedcompiledRootCommand. Could be deduplicated in a follow-up but not required here.
Nit (feel free to ignore): the .catch(() => {}) on waitFor in Root.test.tsx is a nice touch — the subsequent expect(menus).toContain(...) produces a much better failure message than a bare timeout — a one-line comment saying so would help the next reader.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2234 +/- ##
============================================
+ Coverage 96.97% 97.00% +0.03%
============================================
Files 559 559
Lines 38612 38624 +12
============================================
+ Hits 37445 37469 +24
+ Misses 1167 1155 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The update wizard is only reached from the update picker or the detail screen's update action, never as the first history entry, so a pop returns to whichever the user came from. Only the create wizard can be deep-linked and needs an explicit target. Note the same invariant at EndpointWizard's pop, which the route walk cannot reach because every route into it passes a picker first.
Drop the two inline root compiles now that the harness exports one, and record that the walk accepts any ancestor menu, so it catches a no-op or a loop but not an escape that jumps too far up.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
4 participants
Description
Bug.
agentcore harness createrun with no flags opens the TUI directly on the create wizard, so the wizard is the only history entry. The wizard's first-step back wasnavigate(-1), which had nothing to pop, soescdid nothing. Same symptom as #2225 (the policy picker redirect loop), different mechanism. Reproduced against the compiled binary in a pty:agentcore harness create→esc→ screen unchanged.Fix.
HarnessWizardtakes an explicitonExit, so each caller states where the first step'sescgoes.harness createnavigates to the harness menu, since it can be the first history entry.harness updatekeeps a history pop: it is only ever reached from the update picker or the detail screen's update action, and returns to whichever the user came from, as before.EndpointWizardkeeps its pop, which is safe while every route into it passes a picker first; a comment records that.Test. A route-table walk in
src/components/Root.test.tsx: for every command with a screen (102 today), mount it through the realRoot, pressesc, and assert the header line belongs to a menu above it. This is the counterpart to the command-line-only walk inCliOnlyScreen.test.tsx, so a screen added later is covered without a new test. With this PR's three source files reverted to the branch point, the walk fails on exactly one case,agentcore harness create. The policy picker loop is already fixed on this base by #2225; before that landed, the walk failed on it too. The walk accepts any ancestor menu, so it catches a no-op or a loop but not an escape that jumps further up than it should.compiledRootCommand()is extracted from the test harness and the two inline copies in existing tests now use it.Related Issue
No tracked issue. Found while bug-bashing the
refactorbranch TUI.Documentation PR
None needed.
Type of Change
Testing
How have you tested the change?
bun test(3099 pass, 0 fail); this branch has no separatetest:unit/test:integscriptsbun run typecheckbun run lint:checksrc/assets/is untouched, so no snapshot update was neededagentcore harness create→escnow lands on the harness menuChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.