Network mode without vibe kanban#5
Open
koudis wants to merge 7 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR implements host network vs bridge mode support for container networking, refactors agent module architecture around a unified HostInfo identity consolidation, and improves Docker image cleanup procedures. The ChangesHost Network Mode Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.kiro/steering/cli-flags.md (1)
121-126:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
--host-network-offto the CLI-3 start-only summary list.The summary currently omits a newly enforced START-only flag, which can mislead users skimming this section.
Suggested doc patch
-- `--agents`, `--port`, `--ssh-key`, `--rebuild`, `--no-update-known-hosts` are only valid in START mode (CLI-3) +- `--agents`, `--port`, `--ssh-key`, `--rebuild`, `--no-update-known-hosts`, `--host-network-off` are only valid in START mode (CLI-3)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.kiro/steering/cli-flags.md around lines 121 - 126, The START-only flags summary (CLI-3) currently lists `--agents`, `--port`, `--ssh-key`, `--rebuild`, and `--no-update-known-hosts`; add `--host-network-off` to that same bullet so the doc reflects the newly enforced START-only flag. Update the line containing "— `--agents`, `--port`, `--ssh-key`, `--rebuild`, `--no-update-known-hosts` are only valid in START mode (CLI-3)" to include `--host-network-off` alongside the other flags and ensure formatting/commas match the existing list.
🧹 Nitpick comments (1)
internal/cmd/purge_test.go (1)
145-167: ⚡ Quick winAssert that dangling prune was invoked.
The mock tracks
pruned, but this test doesn’t assert it. Add arequire.True(t, mock.pruned, ...)check so regressions in the prune step are caught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cmd/purge_test.go` around lines 145 - 167, The test currently verifies removal order via cmd.RunPurgeWith and mock.removedImageIDs but omits asserting that the dangling prune was invoked; add an assertion after RunPurgeWith such as require.True(t, mock.pruned, "dangling prune should have been invoked") to fail on regressions—place it near the existing require.Len(t, mock.removedImageIDs, 3) / immediately after RunPurgeWith in the test that references RunPurgeWith, mock.removedImageIDs and mock.pruned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.kiro/specs/bootstrap-ai-coding/design-vibekanban.md:
- Line 693: The markdown has two fenced code blocks missing language tags
(MD040) around the Dockerfile snippets; locate the fences containing the lines
"FROM ubuntu:26.04" and "FROM bac-base:latest" and change the opening
triple-backtick markers from ``` to ```dockerfile so both blocks are marked as
Dockerfile code blocks and stop triggering the lint warning.
In @.kiro/specs/bootstrap-ai-coding/tasks.md:
- Line 132: The MD052 error is caused by nested-backtick markdown in the example
line that mentions rapid.StringMatching and the regex; fix it by replacing the
nested backticks so the function name rapid.StringMatching remains a code span
and the regex is displayed as a quoted string (not wrapped with backticks inside
another code span), e.g., show rapid.StringMatching with the pattern expressed
as a quoted regex string to eliminate nested backticks.
In `@internal/cmd/purge.go`:
- Around line 84-88: The dangling image prune is too broad: update the
danglingFilter used before calling api.ImagesPrune so it only targets
BAC-managed artifacts by adding the same label filter used in purgeFilter (i.e.,
add the label filter key/value for bac.managed=true to danglingFilter) so
api.ImagesPrune(ctx, danglingFilter) only removes images with dangling=true and
label=bac.managed=true; edit the danglingFilter construction (symbol:
danglingFilter) before the call to api.ImagesPrune to include that label filter.
In `@internal/cmd/root.go`:
- Around line 526-535: The current guard that compares persisted host network
mode omits needInstance and can incorrectly block runs that already require an
instance rebuild; update the conditional that wraps the dd.ReadHostNetworkOff()
check to also require !needInstance (i.e., execute the persistedOff vs
hostNetworkOff comparison only when !needBase && !flagRebuild && !needInstance),
leaving the inside logic (calling dd.ReadHostNetworkOff(), comparing
persistedOff to hostNetworkOff, printing the "run with --rebuild" message and
returning) unchanged so the network-mode warning only applies when reusing the
existing instance image.
In `@internal/docker/integration_test.go`:
- Around line 653-654: Replace the hardcoded SSH port usage (sshPort := 22222)
in the integration tests with a dynamic free port lookup; call findFreePort() at
each test setup where sshPort is defined (including occurrences around sshPort
in the blocks referenced) and assign its result to sshPort, then use that
variable where the SSH listener/dialer expects a port so each test gets a unique
free port and avoids collisions.
In `@internal/docker/runner_network_test.go`:
- Line 39: The test handler uses fixed-index slicing on r.URL.Path
(r.URL.Path[len(r.URL.Path)-18:] == "/containers/create") which can panic for
paths shorter than 18 chars; change the check to use
strings.HasSuffix(r.URL.Path, "/containers/create") and add the strings import
so the POST branch safely matches suffixes without risking a negative index
panic (update the handler in runner_network_test.go where r.Method and
r.URL.Path are checked).
---
Outside diff comments:
In @.kiro/steering/cli-flags.md:
- Around line 121-126: The START-only flags summary (CLI-3) currently lists
`--agents`, `--port`, `--ssh-key`, `--rebuild`, and `--no-update-known-hosts`;
add `--host-network-off` to that same bullet so the doc reflects the newly
enforced START-only flag. Update the line containing "— `--agents`, `--port`,
`--ssh-key`, `--rebuild`, `--no-update-known-hosts` are only valid in START mode
(CLI-3)" to include `--host-network-off` alongside the other flags and ensure
formatting/commas match the existing list.
---
Nitpick comments:
In `@internal/cmd/purge_test.go`:
- Around line 145-167: The test currently verifies removal order via
cmd.RunPurgeWith and mock.removedImageIDs but omits asserting that the dangling
prune was invoked; add an assertion after RunPurgeWith such as require.True(t,
mock.pruned, "dangling prune should have been invoked") to fail on
regressions—place it near the existing require.Len(t, mock.removedImageIDs, 3) /
immediately after RunPurgeWith in the test that references RunPurgeWith,
mock.removedImageIDs and mock.pruned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 411d7649-9da8-4689-822b-ae38e12c4f68
📒 Files selected for processing (36)
.kiro/specs/bootstrap-ai-coding/design-build-resources.md.kiro/specs/bootstrap-ai-coding/design-components.md.kiro/specs/bootstrap-ai-coding/design-data-models.md.kiro/specs/bootstrap-ai-coding/design-docker.md.kiro/specs/bootstrap-ai-coding/design-properties.md.kiro/specs/bootstrap-ai-coding/design-vibekanban.md.kiro/specs/bootstrap-ai-coding/design.md.kiro/specs/bootstrap-ai-coding/requirements-agents.md.kiro/specs/bootstrap-ai-coding/requirements-cli-combinations.md.kiro/specs/bootstrap-ai-coding/requirements-core.md.kiro/specs/bootstrap-ai-coding/requirements-two-layer-image.md.kiro/specs/bootstrap-ai-coding/requirements.md.kiro/specs/bootstrap-ai-coding/tasks.md.kiro/steering/agent-module.md.kiro/steering/cli-flags.md.kiro/steering/constants.md.kiro/steering/product.md.kiro/steering/structure.mdinternal/agents/augment/integration_test.gointernal/agents/buildresources/buildresources.gointernal/agents/buildresources/integration_test.gointernal/agents/claude/integration_test.gointernal/cmd/purge.gointernal/cmd/purge_test.gointernal/cmd/root.gointernal/cmd/root_test.gointernal/cmd/stop.gointernal/datadir/datadir.gointernal/datadir/datadir_test.gointernal/docker/builder.gointernal/docker/builder_test.gointernal/docker/client.gointernal/docker/export_test.gointernal/docker/integration_test.gointernal/docker/runner.gointernal/docker/runner_network_test.go
Comment on lines
+84
to
+88
| danglingFilter := filters.NewArgs() | ||
| danglingFilter.Add("dangling", "true") | ||
| if _, err := api.ImagesPrune(ctx, danglingFilter); err != nil { | ||
| fmt.Fprintf(os.Stderr, "warning: pruning dangling images: %v\n", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify prune filter scope in purge paths and compare against TL-7 wording.
rg -n -C2 'ImagesPrune|dangling|label' internal/cmd/purge.go internal/cmd/root.go
rg -n -C2 'TL-7|bac-managed|Instance_Images|Base_Image' .kiro/specs/bootstrap-ai-coding/requirements-two-layer-image.mdRepository: koudis/bootstrap-ai-coding
Length of output: 10766
Scope dangling image prune to BAC-managed artifacts.
The dangling prune at lines 84–86 uses only dangling=true, which can delete unrelated dangling images on the host. Requirement TL-7 explicitly requires removal of "all bac-managed images" only. Add the bac.managed=true label filter to match the pattern established in purgeFilter() and align with requirement scope.
Proposed fix
danglingFilter := filters.NewArgs()
danglingFilter.Add("dangling", "true")
+ danglingFilter.Add("label", "bac.managed=true")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/cmd/purge.go` around lines 84 - 88, The dangling image prune is too
broad: update the danglingFilter used before calling api.ImagesPrune so it only
targets BAC-managed artifacts by adding the same label filter used in
purgeFilter (i.e., add the label filter key/value for bac.managed=true to
danglingFilter) so api.ImagesPrune(ctx, danglingFilter) only removes images with
dangling=true and label=bac.managed=true; edit the danglingFilter construction
(symbol: danglingFilter) before the call to api.ImagesPrune to include that
label filter.
Comment on lines
+526
to
+535
| if !needBase && !flagRebuild { | ||
| persistedOff, err := dd.ReadHostNetworkOff() | ||
| if err != nil { | ||
| return fmt.Errorf("reading persisted host network mode: %w", err) | ||
| } | ||
| if persistedOff != hostNetworkOff { | ||
| fmt.Println("Network mode changed — run with --rebuild to update the image.") | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Only enforce network-mode mismatch when reusing the existing instance image.
At Line 526, the guard omits needInstance. This can block valid runs that already require an instance rebuild and incorrectly asks for --rebuild.
Proposed fix
- if !needBase && !flagRebuild {
+ if !needBase && !needInstance && !flagRebuild {
persistedOff, err := dd.ReadHostNetworkOff()
if err != nil {
return fmt.Errorf("reading persisted host network mode: %w", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if !needBase && !flagRebuild { | |
| persistedOff, err := dd.ReadHostNetworkOff() | |
| if err != nil { | |
| return fmt.Errorf("reading persisted host network mode: %w", err) | |
| } | |
| if persistedOff != hostNetworkOff { | |
| fmt.Println("Network mode changed — run with --rebuild to update the image.") | |
| return nil | |
| } | |
| } | |
| if !needBase && !needInstance && !flagRebuild { | |
| persistedOff, err := dd.ReadHostNetworkOff() | |
| if err != nil { | |
| return fmt.Errorf("reading persisted host network mode: %w", err) | |
| } | |
| if persistedOff != hostNetworkOff { | |
| fmt.Println("Network mode changed — run with --rebuild to update the image.") | |
| return nil | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/cmd/root.go` around lines 526 - 535, The current guard that compares
persisted host network mode omits needInstance and can incorrectly block runs
that already require an instance rebuild; update the conditional that wraps the
dd.ReadHostNetworkOff() check to also require !needInstance (i.e., execute the
persistedOff vs hostNetworkOff comparison only when !needBase && !flagRebuild &&
!needInstance), leaving the inside logic (calling dd.ReadHostNetworkOff(),
comparing persistedOff to hostNetworkOff, printing the "run with --rebuild"
message and returning) unchanged so the network-mode warning only applies when
reusing the existing instance image.
Comment on lines
+653
to
+654
| sshPort := 22222 | ||
|
|
There was a problem hiding this comment.
Avoid fixed SSH ports in integration tests to reduce flakes.
Hardcoded ports can collide with host processes and cause non-deterministic failures. Use findFreePort() per test instead.
Suggested change
- // Use a high port to avoid conflicts
- sshPort := 22222
+ sshPort, err := findFreePort()
+ require.NoError(t, err, "finding free SSH port")- // Use a high port for SSH to avoid conflicts
- sshPort := 22224
+ sshPort, err := findFreePort()
+ require.NoError(t, err, "finding free SSH port")- // Use a different high port to avoid conflicts with host network test
- sshPort := 22223
+ sshPort, err := findFreePort()
+ require.NoError(t, err, "finding free SSH port")Also applies to: 765-766, 877-878
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/docker/integration_test.go` around lines 653 - 654, Replace the
hardcoded SSH port usage (sshPort := 22222) in the integration tests with a
dynamic free port lookup; call findFreePort() at each test setup where sshPort
is defined (including occurrences around sshPort in the blocks referenced) and
assign its result to sshPort, then use that variable where the SSH
listener/dialer expects a port so each test gets a unique free port and avoids
collisions.
| mux := http.NewServeMux() | ||
| mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { | ||
| // Match the containers/create endpoint (path includes version prefix) | ||
| if r.Method == http.MethodPost && r.URL.Path[len(r.URL.Path)-18:] == "/containers/create" { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n internal/docker/runner_network_test.go | sed -n '35,45p'Repository: koudis/bootstrap-ai-coding
Length of output: 610
🏁 Script executed:
python3 << 'EOF'
# Verify Go string slicing behavior with negative indices
# In Go, negative indices in slicing cause a panic
test_cases = [
(5, 18), # "hello"[5-18:] = "hello"[-13:]
(10, 18), # path of 10 chars
(17, 18), # path of 17 chars (just under)
(18, 18), # path of exactly 18 chars
(25, 18), # path of 25 chars
]
for path_len, suffix_len in test_cases:
computed_idx = path_len - suffix_len
if computed_idx < 0:
print(f"Path length {path_len}: Index {computed_idx} (NEGATIVE - will panic in Go)")
elif computed_idx == 0:
print(f"Path length {path_len}: Index {computed_idx} (valid - equivalent to full path)")
else:
print(f"Path length {path_len}: Index {computed_idx} (valid)")
EOFRepository: koudis/bootstrap-ai-coding
Length of output: 325
🏁 Script executed:
head -20 internal/docker/runner_network_test.goRepository: koudis/bootstrap-ai-coding
Length of output: 544
Avoid potential panic from negative slice index on short paths.
Line 39 uses fixed-index slicing r.URL.Path[len(r.URL.Path)-18:] on POST path matching. Any path shorter than 18 characters will produce a negative index, causing a panic at runtime. Common test paths like /v1.40/containers (18 chars) or shorter variants will trigger this immediately.
Use strings.HasSuffix() instead:
Proposed fix
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
+ "strings"
"testing"
@@
- if r.Method == http.MethodPost && r.URL.Path[len(r.URL.Path)-18:] == "/containers/create" {
+ if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/containers/create") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if r.Method == http.MethodPost && r.URL.Path[len(r.URL.Path)-18:] == "/containers/create" { | |
| import ( | |
| "context" | |
| "encoding/json" | |
| "fmt" | |
| "net/http" | |
| "net/http/httptest" | |
| "strings" | |
| "testing" | |
| ) | |
| ... | |
| if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/containers/create") { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/docker/runner_network_test.go` at line 39, The test handler uses
fixed-index slicing on r.URL.Path (r.URL.Path[len(r.URL.Path)-18:] ==
"/containers/create") which can panic for paths shorter than 18 chars; change
the check to use strings.HasSuffix(r.URL.Path, "/containers/create") and add the
strings import so the POST branch safely matches suffixes without risking a
negative index panic (update the handler in runner_network_test.go where
r.Method and r.URL.Path are checked).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.kiro/specs/bootstrap-ai-coding/requirements-core.md (2)
34-34: 💤 Low valueMinor style improvement: Add subject to sentence.
The phrase "Can be overridden per invocation via
--port" is missing a subject. Consider: "The SSH_Port can be overridden per invocation via--port" or "This port can be overridden per invocation via--port".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.kiro/specs/bootstrap-ai-coding/requirements-core.md at line 34, The sentence fragment "Can be overridden per invocation via `--port`" lacks a subject; update the requirements text so the subject is explicit (e.g., "The SSH_Port can be overridden per invocation via `--port`" or "This port can be overridden per invocation via `--port`") in the paragraph that documents SSH_Port, SSH_Server, Host, Container and Tool_Data_Dir so the final sentence reads as a complete sentence referring clearly to SSH_Port.
459-461: 💤 Low valueOptional: Vary sentence structure to reduce repetition.
Three consecutive acceptance criteria (AC 10, 11, 12) begin with "THE", which is flagged by static analysis. While this format is consistent with the rest of the requirements document, consider varying the sentence structure for improved readability.
For example, AC 11 could be reworded: "In both modes, the
HostBindIPconstant (127.0.0.1) SHALL ensure SSH is only accessible from localhost—used as the sshd bind address in host mode and as the Docker port binding IP in bridge mode."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.kiro/specs/bootstrap-ai-coding/requirements-core.md around lines 459 - 461, Rewrite the three acceptance criteria to vary sentence openings while preserving meaning: update AC 10 to state that WaitForSSH SHALL connect to 127.0.0.1:<SSH_Port> to verify SSH readiness in both modes; rephrase AC 11 to explain that HostBindIP (`127.0.0.1`) SHALL ensure SSH is only accessible from localhost by being the sshd bind address in host mode and the Docker port binding IP in bridge mode; and reword AC 12 to state that the --host-network-off flag SHALL be valid only in START mode and is subject to the CLI-3 constraint. Ensure the symbols WaitForSSH, HostBindIP, --host-network-off, START, and CLI-3 remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.kiro/specs/bootstrap-ai-coding/requirements-core.md:
- Line 463: Replace the undefined term "Instance_Image" with the established
term "Container_Image" in the requirement sentence (the occurrence of
Instance_Image at line 463); ensure the phrase now reads that the CLI shall
require `--rebuild` to regenerate the Container_Image with the correct
sshd_config, and scan nearby requirements in the same section for any other
stray "Instance_Image" occurrences and update them to "Container_Image" for
consistency with the Glossary.
---
Nitpick comments:
In @.kiro/specs/bootstrap-ai-coding/requirements-core.md:
- Line 34: The sentence fragment "Can be overridden per invocation via `--port`"
lacks a subject; update the requirements text so the subject is explicit (e.g.,
"The SSH_Port can be overridden per invocation via `--port`" or "This port can
be overridden per invocation via `--port`") in the paragraph that documents
SSH_Port, SSH_Server, Host, Container and Tool_Data_Dir so the final sentence
reads as a complete sentence referring clearly to SSH_Port.
- Around line 459-461: Rewrite the three acceptance criteria to vary sentence
openings while preserving meaning: update AC 10 to state that WaitForSSH SHALL
connect to 127.0.0.1:<SSH_Port> to verify SSH readiness in both modes; rephrase
AC 11 to explain that HostBindIP (`127.0.0.1`) SHALL ensure SSH is only
accessible from localhost by being the sshd bind address in host mode and the
Docker port binding IP in bridge mode; and reword AC 12 to state that the
--host-network-off flag SHALL be valid only in START mode and is subject to the
CLI-3 constraint. Ensure the symbols WaitForSSH, HostBindIP, --host-network-off,
START, and CLI-3 remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 45ffb065-6789-4b27-9050-110af17d6b78
📒 Files selected for processing (7)
.kiro/specs/bootstrap-ai-coding/design-architecture.md.kiro/specs/bootstrap-ai-coding/design.md.kiro/specs/bootstrap-ai-coding/requirements-agents.md.kiro/specs/bootstrap-ai-coding/requirements-core.md.kiro/specs/bootstrap-ai-coding/requirements.md.kiro/specs/bootstrap-ai-coding/tasks.mdinternal/constants/constants.go
✅ Files skipped from review due to trivial changes (5)
- .kiro/specs/bootstrap-ai-coding/tasks.md
- .kiro/specs/bootstrap-ai-coding/design-architecture.md
- .kiro/specs/bootstrap-ai-coding/design.md
- internal/constants/constants.go
- .kiro/specs/bootstrap-ai-coding/requirements.md
| 11. THE `HostBindIP` constant (`127.0.0.1`) SHALL be used as the sshd bind address in host mode and as the Docker port binding IP in bridge mode, ensuring SSH is only accessible from localhost in both cases. | ||
| 12. THE `--host-network-off` flag SHALL only be valid in START mode; it is a START-only flag subject to the CLI-3 constraint. | ||
| 13. THE `--host-network-off` value SHALL influence the Instance_Image build: when absent (host mode), sshd_config includes `Port <SSH_Port>` and `ListenAddress 127.0.0.1`; when set (bridge mode), these directives are omitted. | ||
| 14. WHEN `--host-network-off` is changed between invocations for the same project (e.g. added or removed), THE CLI SHALL require `--rebuild` to regenerate the Instance_Image with the correct sshd_config. IF the network mode has changed and `--rebuild` is not set, THE CLI SHALL print a message instructing the user to run with `--rebuild` and exit with a zero exit code. |
There was a problem hiding this comment.
Undefined term: "Instance_Image"
Line 463 uses the term "Instance_Image" which is not defined in the Glossary. Based on the context and the rest of the document, this should likely be "Container_Image" to maintain consistency with the established terminology.
📝 Proposed fix
-14. WHEN `--host-network-off` is changed between invocations for the same project (e.g. added or removed), THE CLI SHALL require `--rebuild` to regenerate the Instance_Image with the correct sshd_config. IF the network mode has changed and `--rebuild` is not set, THE CLI SHALL print a message instructing the user to run with `--rebuild` and exit with a zero exit code.
+14. WHEN `--host-network-off` is changed between invocations for the same project (e.g. added or removed), THE CLI SHALL require `--rebuild` to regenerate the Container_Image with the correct sshd_config. IF the network mode has changed and `--rebuild` is not set, THE CLI SHALL print a message instructing the user to run with `--rebuild` and exit with a zero exit code.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 14. WHEN `--host-network-off` is changed between invocations for the same project (e.g. added or removed), THE CLI SHALL require `--rebuild` to regenerate the Instance_Image with the correct sshd_config. IF the network mode has changed and `--rebuild` is not set, THE CLI SHALL print a message instructing the user to run with `--rebuild` and exit with a zero exit code. | |
| 14. WHEN `--host-network-off` is changed between invocations for the same project (e.g. added or removed), THE CLI SHALL require `--rebuild` to regenerate the Container_Image with the correct sshd_config. IF the network mode has changed and `--rebuild` is not set, THE CLI SHALL print a message instructing the user to run with `--rebuild` and exit with a zero exit code. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.kiro/specs/bootstrap-ai-coding/requirements-core.md at line 463, Replace
the undefined term "Instance_Image" with the established term "Container_Image"
in the requirement sentence (the occurrence of Instance_Image at line 463);
ensure the phrase now reads that the CLI shall require `--rebuild` to regenerate
the Container_Image with the correct sshd_config, and scan nearby requirements
in the same section for any other stray "Instance_Image" occurrences and update
them to "Container_Image" for consistency with the Glossary.
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 by CodeRabbit
New Features
--host-network-offflag to control container networking mode (host network enabled by default; use flag for bridge mode).build-resourcesas a default enabled agent alongside existing agents.Bug Fixes
Documentation
127.0.0.1:<port>; bridge mode uses Docker port mapping.--host-network-offis incompatible with--stop-and-removeand--purge.--rebuild.--ssh-keyflag precedence over default key paths.