Merged
Conversation
carole-lavillonniere
force-pushed
the
carole/drg-603
branch
4 times, most recently
from
531b7b5 to
1eb8898
Compare
carole-lavillonniere
force-pushed
the
carole/drg-603
branch
from
1eb8898 to
191ca3e
Compare
carole-lavillonniere
force-pushed
the
carole/drg-603
branch
3 times, most recently
from
a6b27f9 to
c40a32d
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
carole-lavillonniere
force-pushed
the
carole/drg-603
branch
from
c40a32d to
e3ed19a
Compare
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (status cmd)
participant Env as Env/Config
participant Runtime as Docker Runtime
participant Container as container.Status
participant AWS as Emulator Client
participant Output as Output/Sink
participant UI as UI/TUI
CLI->>Env: read needed values
CLI->>Runtime: create docker runtime
CLI->>AWS: construct emulator client
alt interactive
CLI->>UI: RunStatus(ctx, Runtime, containers, host, AWS client)
UI->>Container: container.Status(...)
else non-interactive
CLI->>Container: container.Status(...)
end
Container->>Runtime: IsRunning(container)
Container->>Runtime: ContainerStartedAt(container)
Container->>AWS: FetchVersion(host)
Container->>AWS: FetchResources(host)
Container->>Output: EmitInstanceInfo / EmitTable / EmitResourceSummary
Output->>UI: format/display (TUI) or write to stdout (plain)
UI-->>CLI: return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
internal/output/plain_format_test.go (1)
114-227: Please add plain-sink parity coverage for these events.These assertions lock down
FormatEventLine, but the sink path also needs matching cases soNewPlainSinkcannot drift forInstanceInfoEventandTableEvent. I'd extendinternal/output/plain_sink_test.goas part of the same change.Based on learnings "Applies to internal/output/**/.go : When adding a new event type, update all of: internal/output/events.go (event type + Event union constraint + emit helper), internal/output/plain_format.go (line formatting fallback), and tests in internal/output/_test.go for formatter/sink behavior parity"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/output/plain_format_test.go` around lines 114 - 227, Add matching tests in internal/output/plain_sink_test.go to cover InstanceInfoEvent and TableEvent parity with FormatEventLine: update or add test cases that emit InstanceInfoEvent (both full and minimal) and TableEvent (with rows, empty) through NewPlainSink and assert the sink output and success/error behavior mirrors FormatEventLine results (use the same expected strings and wantOK semantics); ensure the tests call the sink's Emit/Write methods used by NewPlainSink and exercise narrow/wide terminal widths similar to TestFormatTableWidth so NewPlainSink cannot drift from FormatEventLine behavior.internal/container/status.go (1)
52-67: Inject the AWS emulator client instead of constructing it inStatus.Building
aws.NewClient(&http.Client{})inside the workflow hard-codes transport setup here and makes this path much harder to unit-test beyond the runtime mock. Bothcmd/status.goandinternal/ui/run_status.goare already the outer wiring layers, so this dependency fits better there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/container/status.go` around lines 52 - 67, The Status code hard-codes aws.NewClient(&http.Client{}) — change it to accept an injected AWS emulator client (use the existing aws client interface/type used by emulatorClient) instead of constructing one inside the switch; replace the inline creation and any direct references to aws.NewClient in the switch (emulatorClient.FetchVersion, emulatorClient.FetchResources) with the injected parameter/field (e.g., pass an aws client into the Status function or container struct), and update the outer wiring in the caller sites (cmd/status.go and internal/ui/run_status.go) to construct the real aws client and pass it in so unit tests can supply a mock client.internal/ui/app.go (1)
147-149: KeepbufferedLinesbounded too.
linesis capped viaappendLine, but these newPendingStop()branches append whole tables and other multi-line output straight intobufferedLines. A largestatustable can now grow UI state until the spinner drains. Please route buffered writes through the same trimming helper or capbufferedLinesseparately.Based on learnings, "Applies to internal/ui/**/*.go : Keep Bubble Tea message/history state bounded (for example, capped line buffer)".
Also applies to: 154-155, 230-232, 242-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/app.go` around lines 147 - 149, The PendingStop branches directly append multi-line output into bufferedLines (via a.spinner.PendingStop()), bypassing the existing capping logic in appendLine and allowing unbounded growth; change these branches to route every new line through the same trimming helper (i.e., call appendLine or a shared trimBufferedLines helper for each blank/line/blank piece) or apply a bounded-cap helper when concatenating tables so bufferedLines is trimmed to the same max length as lines; update all similar spots that directly append to bufferedLines (the other PendingStop/append occurrences) to use the same helper so UI message/history state stays bounded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/container/status.go`:
- Line 43: The call to endpoint.ResolveHost(c.Port, localStackHost) ignores its
error, which can leave host empty and cause misleading later failures; update
the status-checking code (around where ResolveHost is called and the host
variable is used) to capture the returned error, check it immediately, and
return or propagate a clear error when ResolveHost fails so we fail fast before
attempting to contact emulator endpoints or printing status like "is running
()". Ensure you reference ResolveHost and the host variable in your change and
handle the error path consistently with surrounding error handling.
In `@internal/emulator/aws/client.go`:
- Around line 71-73: The code in FetchResources uses bufio.NewScanner(resp.Body)
and then caps line size with scanner.Buffer(buf, 1024*1024), which limits NDJSON
lines to 1 MiB and can fail on large deployments; replace this by either using a
bufio.Reader to stream lines (e.g., read with ReadString('\n') or
ReadBytes('\n') from resp.Body) or raise/remove the ceiling on the scanner
(e.g., call scanner.Buffer(nil, <much larger size>) or allocate a larger buffer)
so that the scanner/reader can handle multi-megabyte NDJSON lines; update the
logic around the scanner variable and resp.Body handling in FetchResources to
use the chosen approach.
In `@internal/output/events.go`:
- Around line 60-75: Add typed helper functions EmitInstanceInfo and EmitTable
that wrap the generic Emit(...) so callers can send InstanceInfoEvent and
TableEvent via strongly-typed helpers; implement them mirroring the pattern used
by existing helpers (e.g., create func EmitInstanceInfo(ctx context.Context, e
InstanceInfoEvent) { Emit(ctx, e) } and func EmitTable(ctx context.Context, t
TableEvent) { Emit(ctx, t) }), and update any plain formatter and tests
(plain_format.go and internal/output/*_test.go) to include formatting/coverage
for InstanceInfoEvent and TableEvent to keep parity with other event types.
In `@internal/output/plain_format.go`:
- Around line 206-228: The current logic only shrinks widths[maxCol] down to a
floor (10) which still lets sum(widths)+overhead exceed totalWidth; change this
to ensure sum(widths)+overhead <= totalWidth by computing the excess =
(sum(widths) + overhead) - totalWidth and then reducing columns until excess <=
0; implement a loop that repeatedly picks the widest available column(s) (using
widths and maxCol selection or sort of indices) and decrements them down to a
reasonable per-column minimum (e.g., minWidth = 1 or 3) rather than a single
10-only floor, so that fixedWidth + sum(widths) is guaranteed not to exceed
totalWidth. Ensure you update widths in-place and stop when excess is
eliminated.
In `@internal/output/terminal.go`:
- Around line 9-15: The terminalWidth function currently queries both
os.Stdout.Fd() and os.Stderr.Fd(), which lets an interactive stderr drive width
when stdout is being redirected; change terminalWidth to consider only stdout
(use os.Stdout.Fd() with term.GetSize) and if stdout is not a TTY treat width as
unbounded/no-truncation (e.g., return a very large width or zero-as-unbounded)
so TableEvent output isn’t truncated when stdout is redirected; update
references in code that call terminalWidth accordingly.
---
Nitpick comments:
In `@internal/container/status.go`:
- Around line 52-67: The Status code hard-codes aws.NewClient(&http.Client{}) —
change it to accept an injected AWS emulator client (use the existing aws client
interface/type used by emulatorClient) instead of constructing one inside the
switch; replace the inline creation and any direct references to aws.NewClient
in the switch (emulatorClient.FetchVersion, emulatorClient.FetchResources) with
the injected parameter/field (e.g., pass an aws client into the Status function
or container struct), and update the outer wiring in the caller sites
(cmd/status.go and internal/ui/run_status.go) to construct the real aws client
and pass it in so unit tests can supply a mock client.
In `@internal/output/plain_format_test.go`:
- Around line 114-227: Add matching tests in internal/output/plain_sink_test.go
to cover InstanceInfoEvent and TableEvent parity with FormatEventLine: update or
add test cases that emit InstanceInfoEvent (both full and minimal) and
TableEvent (with rows, empty) through NewPlainSink and assert the sink output
and success/error behavior mirrors FormatEventLine results (use the same
expected strings and wantOK semantics); ensure the tests call the sink's
Emit/Write methods used by NewPlainSink and exercise narrow/wide terminal widths
similar to TestFormatTableWidth so NewPlainSink cannot drift from
FormatEventLine behavior.
In `@internal/ui/app.go`:
- Around line 147-149: The PendingStop branches directly append multi-line
output into bufferedLines (via a.spinner.PendingStop()), bypassing the existing
capping logic in appendLine and allowing unbounded growth; change these branches
to route every new line through the same trimming helper (i.e., call appendLine
or a shared trimBufferedLines helper for each blank/line/blank piece) or apply a
bounded-cap helper when concatenating tables so bufferedLines is trimmed to the
same max length as lines; update all similar spots that directly append to
bufferedLines (the other PendingStop/append occurrences) to use the same helper
so UI message/history state stays bounded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 397cc057-ef0e-46af-a6b0-105babd2a502
📒 Files selected for processing (23)
CLAUDE.mdcmd/root.gocmd/status.gointernal/auth/mock_token_storage.gointernal/container/status.gointernal/container/status_test.gointernal/emulator/aws/aws.gointernal/emulator/aws/client.gointernal/emulator/aws/client_test.gointernal/emulator/aws/resource_name_test.gointernal/output/events.gointernal/output/plain_format.gointernal/output/plain_format_test.gointernal/output/terminal.gointernal/runtime/docker.gointernal/runtime/mock_runtime.gointernal/runtime/runtime.gointernal/ui/app.gointernal/ui/components/input_prompt.gointernal/ui/components/message.gointernal/ui/run_status.gotest/integration/env/env.gotest/integration/status_test.go
carole-lavillonniere
force-pushed
the
carole/drg-603
branch
from
e3ed19a to
40a4944
Compare
carole-lavillonniere
force-pushed
the
carole/drg-603
branch
from
eeb7fd5 to
62db43c
Compare
carole-lavillonniere
requested review from
anisaoshafi and
silv-io
as code owners
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cmd/status.go (1)
35-35: Consider adding a timeout to the HTTP client.The default
&http.Client{}has no timeout, which could cause the status command to hang indefinitely if LocalStack is unresponsive. A reasonable timeout (e.g., 5-10 seconds) would improve UX for the status check.♻️ Proposed fix
- awsClient := aws.NewClient(&http.Client{}) + awsClient := aws.NewClient(&http.Client{Timeout: 10 * time.Second})And add
"time"to the imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/status.go` at line 35, The http.Client used to create awsClient (aws.NewClient(&http.Client{})) has no timeout and can hang; update the call to pass a client with a timeout (e.g., &http.Client{Timeout: 5 * time.Second}) and add the "time" import, ensuring awsClient is created with the timed client in the status command where aws.NewClient is invoked.internal/container/status.go (1)
67-69: Wrap resource-fetch errors with container context.Returning
fetchErrdirectly loses which emulator/container failed.Suggested improvement
rows, fetchErr = emulatorClient.FetchResources(ctx, host) if fetchErr != nil { output.EmitSpinnerStop(sink) - return fetchErr + return fmt.Errorf("fetching resources for %s (%s): %w", c.DisplayName(), host, fetchErr) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/container/status.go` around lines 67 - 69, Wrap the returned fetchErr with container/emulator context instead of returning it raw: keep the call to output.EmitSpinnerStop(sink), then return a wrapped error using fmt.Errorf("fetch %s: %w", <container identifier>, fetchErr) (use the identifier available in the current scope, e.g., c.ID or container.Name), and add the fmt import if missing; ensure output.EmitSpinnerStop(sink) is still executed before returning the wrapped error.internal/emulator/aws/client_test.go (1)
48-107: Assert the resources endpoint path inFetchResourcestests.These handlers currently accept any path, so a path regression could still pass tests.
Suggested test hardening
- server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/_localstack/resources", r.URL.Path) w.Header().Set("Content-Type", "application/x-ndjson") _, _ = fmt.Fprintln(w, `{"AWS::S3::Bucket": [{"region_name": "us-east-1", "account_id": "000000000000", "id": "my-bucket"}]}`) _, _ = fmt.Fprintln(w, `{"AWS::Lambda::Function": [{"region_name": "us-east-1", "account_id": "000000000000", "id": "my-function"}]}`) }))- server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/_localstack/resources", r.URL.Path) w.Header().Set("Content-Type", "application/x-ndjson") _, _ = fmt.Fprintln(w, `{"AWS::SNS::Topic": [{"region_name": "us-east-1", "account_id": "000000000000", "id": "arn:aws:sns:us-east-1:000000000000:my-topic"}]}`) }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/emulator/aws/client_test.go` around lines 48 - 107, The tests for FetchResources accept any request path, so add an explicit assertion in each httptest handler to ensure the client calls the expected endpoint path (the resources endpoint used by FetchResources) — inside each http.HandlerFunc check r.URL.Path (or r.URL.RequestURI()) against the expected path (e.g., "/resources" or the package constant if one exists) and call t.Fatalf or write a 404 when it doesn't match; update all handlers in client_test.go (the handlers used by the tests that call NewClient() and c.FetchResources(...)) to validate the request path before writing the NDJSON response.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/container/status.go`:
- Around line 26-73: The spinner is started once via
output.EmitSpinnerStart(sink, ...) before iterating containers but stopped
multiple times inside the loop (output.EmitSpinnerStop), causing duplicate stops
and missing spinners on later iterations; change to balance start/stop
per-iteration by moving EmitSpinnerStart(sink, ...) inside the for _, c := range
containers loop (call it at the top of each iteration) and ensure a single
matching EmitSpinnerStop(sink) before each continue/return path in that
iteration (remove duplicate stops on the version-failure path and replace early
stops with the per-iteration stop), referencing EmitSpinnerStart,
EmitSpinnerStop, the loop over containers, and error/return branches like the
IsRunning error check, not-running path, version fetch warning path, and
fetchErr return so every start has exactly one stop.
---
Nitpick comments:
In `@cmd/status.go`:
- Line 35: The http.Client used to create awsClient
(aws.NewClient(&http.Client{})) has no timeout and can hang; update the call to
pass a client with a timeout (e.g., &http.Client{Timeout: 5 * time.Second}) and
add the "time" import, ensuring awsClient is created with the timed client in
the status command where aws.NewClient is invoked.
In `@internal/container/status.go`:
- Around line 67-69: Wrap the returned fetchErr with container/emulator context
instead of returning it raw: keep the call to output.EmitSpinnerStop(sink), then
return a wrapped error using fmt.Errorf("fetch %s: %w", <container identifier>,
fetchErr) (use the identifier available in the current scope, e.g., c.ID or
container.Name), and add the fmt import if missing; ensure
output.EmitSpinnerStop(sink) is still executed before returning the wrapped
error.
In `@internal/emulator/aws/client_test.go`:
- Around line 48-107: The tests for FetchResources accept any request path, so
add an explicit assertion in each httptest handler to ensure the client calls
the expected endpoint path (the resources endpoint used by FetchResources) —
inside each http.HandlerFunc check r.URL.Path (or r.URL.RequestURI()) against
the expected path (e.g., "/resources" or the package constant if one exists) and
call t.Fatalf or write a 404 when it doesn't match; update all handlers in
client_test.go (the handlers used by the tests that call NewClient() and
c.FetchResources(...)) to validate the request path before writing the NDJSON
response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 66bb9b2b-9dba-4b95-a1d4-401a61d93263
📒 Files selected for processing (16)
CLAUDE.mdcmd/status.gointernal/container/status.gointernal/container/status_test.gointernal/emulator/aws/client.gointernal/emulator/aws/client_test.gointernal/emulator/aws/resource_name.gointernal/emulator/aws/resource_name_test.gointernal/output/events.gointernal/output/plain_format.gointernal/output/plain_format_test.gointernal/output/plain_sink_test.gointernal/output/terminal.gointernal/ui/app.gointernal/ui/components/input_prompt.gointernal/ui/run_status.go
✅ Files skipped from review due to trivial changes (1)
- internal/ui/components/input_prompt.go
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/container/status_test.go
- internal/output/terminal.go
- internal/output/plain_format_test.go
- internal/output/events.go
Comment on lines
+26
to
+73
| output.EmitSpinnerStart(sink, "Fetching LocalStack status") | ||
|
|
||
| for _, c := range containers { | ||
| name := c.Name() | ||
| running, err := rt.IsRunning(ctx, name) | ||
| if err != nil { | ||
| output.EmitSpinnerStop(sink) | ||
| return fmt.Errorf("checking %s running: %w", name, err) | ||
| } | ||
| if !running { | ||
| output.EmitSpinnerStop(sink) | ||
| output.EmitError(sink, output.ErrorEvent{ | ||
| Title: fmt.Sprintf("%s is not running", c.DisplayName()), | ||
| Actions: []output.ErrorAction{ | ||
| {Label: "Start LocalStack:", Value: "lstk"}, | ||
| {Label: "See help:", Value: "lstk -h"}, | ||
| }, | ||
| }) | ||
| return output.NewSilentError(fmt.Errorf("%s is not running", name)) | ||
| } | ||
|
|
||
| host, _ := endpoint.ResolveHost(c.Port, localStackHost) | ||
|
|
||
| var uptime time.Duration | ||
| if startedAt, err := rt.ContainerStartedAt(ctx, name); err == nil { | ||
| uptime = time.Since(startedAt) | ||
| } | ||
|
|
||
| var version string | ||
| var rows []aws.Resource | ||
| switch c.Type { | ||
| case config.EmulatorAWS: | ||
| if v, err := emulatorClient.FetchVersion(ctx, host); err != nil { | ||
| output.EmitSpinnerStop(sink) | ||
| output.EmitWarning(sink, fmt.Sprintf("Could not fetch version: %v", err)) | ||
| } else { | ||
| version = v | ||
| } | ||
|
|
||
| var fetchErr error | ||
| rows, fetchErr = emulatorClient.FetchResources(ctx, host) | ||
| if fetchErr != nil { | ||
| output.EmitSpinnerStop(sink) | ||
| return fetchErr | ||
| } | ||
| } | ||
|
|
||
| output.EmitSpinnerStop(sink) |
There was a problem hiding this comment.
Balance spinner start/stop per container iteration.
EmitSpinnerStart runs once before the loop, but EmitSpinnerStop runs in several branch paths inside the loop (including twice on version-failure path). This can produce duplicate stop events and leaves later iterations without an active spinner.
Suggested adjustment
- output.EmitSpinnerStart(sink, "Fetching LocalStack status")
-
for _, c := range containers {
+ output.EmitSpinnerStart(sink, "Fetching LocalStack status")
name := c.Name()
running, err := rt.IsRunning(ctx, name)
if err != nil {
output.EmitSpinnerStop(sink)
return fmt.Errorf("checking %s running: %w", name, err)
@@
case config.EmulatorAWS:
if v, err := emulatorClient.FetchVersion(ctx, host); err != nil {
- output.EmitSpinnerStop(sink)
output.EmitWarning(sink, fmt.Sprintf("Could not fetch version: %v", err))
} else {
version = v
}As per coding guidelines "Any feature/workflow package that produces user-visible progress should accept an output.Sink dependency and emit events through internal/output".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/container/status.go` around lines 26 - 73, The spinner is started
once via output.EmitSpinnerStart(sink, ...) before iterating containers but
stopped multiple times inside the loop (output.EmitSpinnerStop), causing
duplicate stops and missing spinners on later iterations; change to balance
start/stop per-iteration by moving EmitSpinnerStart(sink, ...) inside the for _,
c := range containers loop (call it at the top of each iteration) and ensure a
single matching EmitSpinnerStop(sink) before each continue/return path in that
iteration (remove duplicate stops on the version-failure path and replace early
stops with the per-iteration stop), referencing EmitSpinnerStart,
EmitSpinnerStop, the loop over containers, and error/return branches like the
IsRunning error check, not-running path, version fetch warning path, and
fetchErr return so every start has exactly one stop.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
internal/container/status.go (1)
21-68:⚠️ Potential issue | 🟡 MinorSpinner stop called multiple times within loop iteration.
The spinner lifecycle has an imbalance: when
FetchVersionfails (line 54), the spinner is stopped and a warning is emitted, but execution continues to line 68 where the spinner is stopped again. This produces a duplicate stop event.♻️ Suggested fix: remove the stop on the warning-only path
switch c.Type { case config.EmulatorAWS: if v, err := emulatorClient.FetchVersion(ctx, host); err != nil { - output.EmitSpinnerStop(sink) output.EmitWarning(sink, fmt.Sprintf("Could not fetch version: %v", err)) } else { version = v }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/container/status.go` around lines 21 - 68, The loop is calling output.EmitSpinnerStop(sink) twice when emulatorClient.FetchVersion fails: once inside the FetchVersion error branch and again later before the loop iteration ends; remove the premature stop in the FetchVersion error path (the output.EmitSpinnerStop call that precedes output.EmitWarning in the emulatorClient.FetchVersion error branch) so the spinner is only stopped once per iteration at the existing final output.EmitSpinnerStop(sink) (and keep the existing stop-before-return behavior for fetchErr from emulatorClient.FetchResources and other early returns).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/container/status.go`:
- Around line 21-68: The loop is calling output.EmitSpinnerStop(sink) twice when
emulatorClient.FetchVersion fails: once inside the FetchVersion error branch and
again later before the loop iteration ends; remove the premature stop in the
FetchVersion error path (the output.EmitSpinnerStop call that precedes
output.EmitWarning in the emulatorClient.FetchVersion error branch) so the
spinner is only stopped once per iteration at the existing final
output.EmitSpinnerStop(sink) (and keep the existing stop-before-return behavior
for fetchErr from emulatorClient.FetchResources and other early returns).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1782d0a4-f823-4090-a8d4-b0f94e6cfcb0
📒 Files selected for processing (7)
internal/container/status.gointernal/emulator/aws/client.gointernal/emulator/emulator.gointernal/output/plain_format.gointernal/output/plain_format_test.gointernal/output/plain_sink_test.gointernal/ui/run_status.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/emulator/aws/client.go
This was referenced Mar 16, 2026
Merged
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.
Summary
Adds
lstk statuscommand that shows the status of running LocalStack emulators and their deployed AWS resources./_localstack/resourcesand displays them in a formatted tableInstanceInfoEventandTableEventoutput events that can be used by other commandsWith resources deployed
Emulator not started
No resources deployed