-
Notifications
You must be signed in to change notification settings - Fork 35
fix(status): show 'No target configured' instead of empty '(target: )' in status header #1171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,7 +113,8 @@ | |
|
|
||
| render( | ||
| <Text> | ||
| AgentCore Status - {result.runtimeId} (target: {result.targetName}) | ||
| AgentCore Status - {result.runtimeId} (target:{' '} | ||
| {result.targetName && result.targetName.length > 0 ? result.targetName : 'No target configured'}) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That's not wrong, but it contradicts the PR description, which frames this as fixing "the same latent bug for that code path." There's no latent bug here right now — the reproducer in #985 ( Options:
|
||
| {runtimeStatus ? ` - ${runtimeStatus}` : ''} | ||
| </Text> | ||
| ); | ||
|
|
@@ -299,7 +300,7 @@ | |
| }); | ||
| }; | ||
|
|
||
| function ResourceEntry({ entry, showRuntime }: { entry: ResourceStatusEntry; showRuntime?: boolean }) { | ||
| return ( | ||
| <Text> | ||
| {' '} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test verifies the
handleProjectStatuscontract (emptytargetNamewhen no targets are configured), but the code change in this PR is on the runtime-status path that callshandleRuntimeLookup. So the test doesn't actually exercise the render line you modified.It's a fine regression guard for the default-path fix that landed in #1068, but if the goal is to lock in the behavior that this PR changes, consider adding a test for
handleRuntimeLookupor a rendering test for the runtime-status header — otherwise nothing in the test suite prevents a future regression of line 113–114.Also, minor: this test is inside
describe('handleProjectStatus — live enrichment', ...)but has nothing to do with live enrichment. Consider moving it to a more appropriate describe block (e.g. the existinghandleProjectStatusblock that covers target resolution).