fix: fix infinite loop between source type and add another KB TUI screens#1699
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
Package TarballHow to installgh release download pr-1699-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.22.0.tgz |
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
The fix eliminates the infinite loop cleanly — my trace of the new state machine agrees with the description, and I don't see any new cycle introduced.
One thing worth adding before merging: a regression test for the loop scenario. The existing AddKnowledgeBaseScreen.test.tsx already drives the wizard with ink-testing-library (see walkToGatewayStep), so a test that reproduces "add one source → land on add-another → Esc → Esc" and asserts the screen doesn't oscillate would be inexpensive to add and would prevent this exact regression from returning. Without it, someone could easily re-introduce the loop by tweaking one of these onExit/onCancel handlers.
Minor observation (non-blocking, pre-existing): the description TextInput doesn't pass initialValue={description}, so on any Esc-round-trip that returns to the description step the field renders empty, and hitting Enter clears the previously entered description. This PR widens the set of paths that reach the description step a second time (Esc from data-source-type now works even when sources exist), so users are more likely to hit it. Worth a follow-up — probably as simple as initialValue={description} on the description input, or gating setDescription(value) on non-empty input.
Coverage Report
|
Description
Fix a bug which caused TUI screen to infinitely loop between "source type" and "add another?" when atleast one source was configured on a KB.
The previous cycle was caused by:
The solution in this PR updates the logic in the following ways:
New flow is as follows:
Related Issue
Closes #
Documentation PR
Type of Change
Testing
How have you tested the change?
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.