chore: use shared FormRadio component in endpointWizard - #2197
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice, focused refactor extracting the radio-list rendering into a reusable FormRadioGroup. The behavior in VersionStep is preserved (key handling, focus semantics, sort order, "latest" default in create mode), tests were updated to assert the new bordered layout, and no other call sites still rely on the removed detail field.
A couple of small observations, none of which I think block merging:
FormRadioGrouplives insrc/components/while the other reusable UI primitives (spinner, stepper, dialog, key-hint, etc.) live insrc/components/ui/and typically have a colocated.test.tsx. Might be worth moving it underui/and adding a lightweight unit test in a follow-up, especially since it's meant to be reused.- In the loaded state the wizard now shows both a bold
name("choose a harness version") and a mutedhelpText("which harness version should this endpoint serve?"), while the pending/error/empty branches still render just the original muted<Question>. The header text visibly changes as the versions query resolves. If that's not intentional, consider either droppingname(pass an empty string, or extendFormRadioGroupto make it optional) or rendering an equivalent bold header in the other branches too.
Neither is a correctness issue — feel free to address in a follow-up.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2197 +/- ##
============================================
- Coverage 97.09% 97.08% -0.02%
============================================
Files 544 544
Lines 37834 37839 +5
============================================
- Hits 36736 36735 -1
- Misses 1098 1104 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
notgitika
left a comment
There was a problem hiding this comment.
The PR itself LGTM but I would maybe try to address this portion that the harness reviewer added above
In the loaded state the wizard now shows both a bold name ("choose a harness version") and a muted helpText ("which harness version should this endpoint serve?"), while the pending/error/empty branches still render just the original muted . The header text visibly changes as the versions query resolves. If that's not intentional, consider either dropping name (pass an empty string, or extend FormRadioGroup to make it optional) or rendering an equivalent bold header in the other branches too.
44fe8d9 to
5bd6779
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Description
EndpointWizard.tsxwas added before theFormRadioGroupshared component was created, as until now was using its own radio button logic.This PR updates endpoint wizard to use the shared component, aligning it to the style and handling the rest of the TUI uses.
Before:


After:
Type of Change
Testing
How have you tested the change?
bun run test(2854 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.