Draft
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
📝 WalkthroughWalkthroughIntroduces a new Changes
Sequence DiagramsequenceDiagram
participant User as User
participant CLI as Status Command
participant Runtime as Docker Runtime
participant Config as Config
participant UI as Bubble Tea UI
participant Container as Container Status
participant AWS as AWS Emulator API
participant Sink as Output Sink
User->>CLI: Execute status command
CLI->>Config: Initialize config
Config-->>CLI: config ready
alt Interactive Mode
CLI->>UI: RunStatus(ctx, runtime, containers, host)
UI->>UI: NewApp()
UI->>UI: Start Bubble Tea program
UI->>Container: Start goroutine: Status(ctx, runtime, containers, host, sink)
par Container Status Fetching
Container->>Runtime: IsRunning(ctx, containerName)
Runtime-->>Container: running state
Container->>Runtime: ContainerStartedAt(ctx, containerName)
Runtime-->>Container: startup time
Container->>AWS: FetchVersion(ctx, host)
AWS-->>Container: version string
Container->>AWS: FetchResources(ctx, host)
AWS-->>Container: resources slice
Container->>Sink: Emit InstanceInfoEvent
Container->>Sink: Emit TableEvent
and UI Event Loop
UI->>UI: Process Tea messages
UI->>Sink: Append formatted events to display
end
Container-->>UI: runDoneMsg or runErrMsg
UI-->>User: Rendered status output
else Non-Interactive Mode
CLI->>Container: Status(ctx, runtime, containers, host, plainSink)
Container->>Runtime: IsRunning(ctx, containerName)
Runtime-->>Container: running state
Container->>Runtime: ContainerStartedAt(ctx, containerName)
Runtime-->>Container: startup time
Container->>AWS: FetchVersion(ctx, host)
AWS-->>Container: version string
Container->>AWS: FetchResources(ctx, host)
AWS-->>Container: resources slice
Container->>Sink: Emit events to stdout
Sink-->>User: Formatted status text
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 for PR comments
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
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
1 participant
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