feat(ci): add devsy ci command for devcontainer CI#603
Conversation
✅ Deploy Preview for devsydev canceled.
|
📝 WalkthroughWalkthroughAdds a ChangesCI command
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CIJob
participant CICmd
participant RunHeadless
participant Devcontainer
participant Workspace
CIJob->>CICmd: invoke devsy ci
CICmd->>RunHeadless: resolve and start workspace
RunHeadless->>Devcontainer: start headlessly
CICmd->>Devcontainer: run command with environment
CICmd->>Workspace: delete workspace unless kept
CICmd-->>CIJob: return command or teardown result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/pr-ci.ymlTraceback (most recent call last): e2e/tests/ci/testdata/ci/.devcontainer/devcontainer.jsonTraceback (most recent call last): e2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.jsonTraceback (most recent call last): 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 |
✅ Deploy Preview for images-devsy-sh canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/workspace/build.go (1)
218-225: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClose the temporary file before returning its path.
os.CreateTempleavesfopen, but callers receive no handle to close it. This leaks the descriptor and can make the cleanup removal fail on Windows.Proposed fix
func NewTempSSHConfig() (string, func(), error) { f, err := os.CreateTemp("", config.BinaryName+"ssh.config") if err != nil { return "", nil, err } path := f.Name() + if err := f.Close(); err != nil { + _ = os.Remove(path) + return "", nil, err + } return path, func() { _ = os.Remove(path) }, 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 `@cmd/workspace/build.go` around lines 218 - 225, Update NewTempSSHConfig to close the file returned by os.CreateTemp before returning the path and cleanup function. Handle any close error consistently, while preserving the existing cleanup behavior and successful path return.
🧹 Nitpick comments (2)
e2e/tests/ci/ci.go (2)
40-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the exact exit code
3is propagated.Checking only
HaveOccurred()allowsdevsy cito replace the container status with any generic exit code while this core requirement still passes.🤖 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 `@e2e/tests/ci/ci.go` around lines 40 - 48, The test case “should propagate a non-zero exit code from the run command” currently checks only that an error occurred; assert that the returned error from f.ExecCommandCapture contains the exact exit code 3, preserving the existing command and setup.
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDistinguish workspace absence from workspace-list failures.
FindWorkspacereturns an error both when the workspace is missing and when listing workspaces fails. These assertions can therefore pass without teardown succeeding. Assert a dedicated not-found error or list successfully and verify the workspace ID is absent.Also applies to: 50-52
🤖 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 `@e2e/tests/ci/ci.go` around lines 35 - 37, Update the teardown assertions around FindWorkspace to distinguish an absent workspace from failures listing workspaces. After CI completion, either assert the dedicated not-found error, or ensure workspace listing succeeds and explicitly verify the torn-down workspace ID is absent; apply the same correction to both assertion sites.
🤖 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 `@cmd/ci/ci.go`:
- Around line 231-235: Update the cleanup closure around teardown and
cmd.teardown so it does not pass the cancelled execution context after signal
handling. When teardown is required, create a fresh bounded context for cleanup,
pass it to cmd.teardown, and ensure its cancellation is released after cleanup.
- Line 194: Update the command logging around the devcontainer execution to
avoid exposing raw arguments: log only the executable from cmd.RunCmd, or move
the full command to an explicitly sensitive debug-level log. Preserve the
existing execution behavior and the informational log context.
- Around line 232-237: Update the cleanup callback and its caller in the CI flow
so teardown failures are returned rather than only logged. Change teardown to
propagate its deletion error, combine it with the primary container command
result, and ensure the final CI status is unsuccessful when ephemeral workspace
cleanup fails while still running cleanupSSH.
In `@cmd/workspace/up/up.go`:
- Around line 499-505: Update WithSignals and the watchForForceShutdown setup so
the force-shutdown listener is registered before the first signal can cancel the
NotifyContext. Ensure a second signal arriving immediately after cancellation is
observed by the force-shutdown path and triggers os.Exit, while preserving the
existing cleanup coordination through done and stop.
---
Outside diff comments:
In `@cmd/workspace/build.go`:
- Around line 218-225: Update NewTempSSHConfig to close the file returned by
os.CreateTemp before returning the path and cleanup function. Handle any close
error consistently, while preserving the existing cleanup behavior and
successful path return.
---
Nitpick comments:
In `@e2e/tests/ci/ci.go`:
- Around line 40-48: The test case “should propagate a non-zero exit code from
the run command” currently checks only that an error occurred; assert that the
returned error from f.ExecCommandCapture contains the exact exit code 3,
preserving the existing command and setup.
- Around line 35-37: Update the teardown assertions around FindWorkspace to
distinguish an absent workspace from failures listing workspaces. After CI
completion, either assert the dedicated not-found error, or ensure workspace
listing succeeds and explicitly verify the torn-down workspace ID is absent;
apply the same correction to both assertion sites.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffc2f360-802a-40dd-86e3-635141cdd6bb
📒 Files selected for processing (14)
.github/workflows/pr-ci.ymlcmd/ci/ci.gocmd/ci/ci_test.gocmd/root.gocmd/workspace/build.gocmd/workspace/up/agent.gocmd/workspace/up/up.goe2e/e2e_suite_test.goe2e/tests/ci/ci.goe2e/tests/ci/testdata/ci/.devcontainer/devcontainer.jsonpkg/devcontainer/config/result.gopkg/devcontainer/config/result_test.gosites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdxsites/docs-devsy-sh/sidebars.js
| return fmt.Errorf("start devcontainer: %w", err) | ||
| } | ||
|
|
||
| log.Infof("running command in devcontainer: %s", strings.Join(cmd.RunCmd, " ")) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid logging raw command arguments.
CI commands may include tokens or passwords as arguments; logging the full joined command exposes them in persistent build logs. Log only the executable or emit full arguments at an explicitly sensitive debug level.
🤖 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 `@cmd/ci/ci.go` at line 194, Update the command logging around the devcontainer
execution to avoid exposing raw arguments: log only the executable from
cmd.RunCmd, or move the full command to an explicitly sensitive debug-level log.
Preserve the existing execution behavior and the informational log context.
| func WithSignals(ctx context.Context) (context.Context, func()) { | ||
| ctx, cancel := context.WithCancel(ctx) | ||
| signals := make(chan os.Signal, 1) | ||
| signal.Notify(signals, os.Interrupt, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGQUIT) | ||
| ctx, stop := signal.NotifyContext(ctx, shutdownSignals...) | ||
|
|
||
| // done lets cleanup unblock goroutines parked on <-signals; signal.Stop | ||
| // alone wouldn't wake them. | ||
| // done lets the force-shutdown watcher exit once cleanup runs; stop() | ||
| // alone wouldn't wake it. | ||
| done := make(chan struct{}) | ||
| go watchForForceShutdown(ctx, done) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Register the force-shutdown listener before the second signal can be missed.
After the first cancellation, there’s a gap before the second watcher is registered. A second signal in that window can be delivered to the original NotifyContext channel and never reach the os.Exit path, leaving cleanup stuck.
🤖 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 `@cmd/workspace/up/up.go` around lines 499 - 505, Update WithSignals and the
watchForForceShutdown setup so the force-shutdown listener is registered before
the first signal can cancel the NotifyContext. Ensure a second signal arriving
immediately after cancellation is observed by the force-shutdown path and
triggers os.Exit, while preserving the existing cleanup coordination through
done and stop.
340c7f0 to
438c7c4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx (2)
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using "afterward" instead of "afterwards".
In American English, "afterward" is typically preferred over "afterwards".
💡 Proposed fix
`devsy ci` builds an ephemeral devcontainer, runs a command inside it, and tears -the workspace down afterwards. It lets you verify — on every push or pull +the workspace down afterward. It lets you verify — on every push or pull🤖 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 `@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx` around lines 8 - 9, In the introductory description of the `devsy ci` command, replace “afterwards” with the American English spelling “afterward” while preserving the surrounding wording.Source: Linters/SAST tools
100-103: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueRecommend pinning the CLI version for reproducible builds.
Downloading from
latestcan introduce unexpected breaking changes into a user's CI pipeline. Consider adding a comment to recommend pinning to a specific version (e.g.,v1.2.3).💡 Proposed change
- name: Install Devsy run: | + # Note: For reproducible CI builds, replace 'latest' with a specific release version (e.g., v1.2.3) curl -fsSL https://github.com/devsy-org/devsy/releases/latest/download/devsy-linux-amd64 -o /usr/local/bin/devsy chmod +x /usr/local/bin/devsy🤖 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 `@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx` around lines 100 - 103, Add a concise comment above the Devsy download command in the CI documentation recommending that users replace the latest release URL with a specific pinned version, such as v1.2.3, for reproducible builds. Keep the existing installation commands 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.
Nitpick comments:
In
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`:
- Around line 8-9: In the introductory description of the `devsy ci` command,
replace “afterwards” with the American English spelling “afterward” while
preserving the surrounding wording.
- Around line 100-103: Add a concise comment above the Devsy download command in
the CI documentation recommending that users replace the latest release URL with
a specific pinned version, such as v1.2.3, for reproducible builds. Keep the
existing installation commands unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae9d3dbd-dd31-4b63-8b54-ce9a005515bc
📒 Files selected for processing (12)
.github/workflows/pr-ci.ymlcmd/ci/ci.gocmd/ci/ci_test.gocmd/root.gocmd/workspace/build.gocmd/workspace/up/up.goe2e/e2e_suite_test.goe2e/tests/ci/ci.goe2e/tests/ci/testdata/ci/.devcontainer/devcontainer.jsone2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.jsonsites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdxsites/docs-devsy-sh/sidebars.js
🚧 Files skipped from review as they are similar to previous changes (7)
- e2e/tests/ci/testdata/ci/.devcontainer/devcontainer.json
- cmd/root.go
- e2e/e2e_suite_test.go
- sites/docs-devsy-sh/sidebars.js
- cmd/workspace/build.go
- cmd/ci/ci.go
- cmd/workspace/up/up.go
7c45aa2 to
0a55cf4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-ci.yml (1)
25-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable checkout credential persistence.
These jobs execute repository-controlled tests and commands while
actions/checkoutstores the GitHub token in.git/config. Addpersist-credentials: falseto every checkout; provide an explicit least-privilege token only to steps that require GitHub access.Proposed fix
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: falseApply this to each checkout listed above.
Also applies to: 44-44, 52-52, 92-92, 127-127, 164-164, 427-428
🤖 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 @.github/workflows/pr-ci.yml at line 25, Update every actions/checkout invocation in the workflow, including the referenced locations, to set persist-credentials to false. For any later step that requires GitHub access, pass an explicit least-privilege token rather than relying on checkout’s persisted credentials.Source: Linters/SAST tools
🧹 Nitpick comments (1)
sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx (1)
99-103: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin CI tooling and verify downloaded binaries.
The examples use mutable
actions/checkout@v4and/releases/latest/download/devsy-linux-amd64, so CI behavior can change without a documentation or PR change. Pin a tested Devsy version, verify its checksum, and prefer a pinned checkout SHA.Also applies to: 121-122
🤖 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 `@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx` around lines 99 - 103, Update the CI example’s actions/checkout reference to a tested immutable commit SHA and replace the mutable Devsy latest-download URL with a specific tested release version. Add checksum verification for the downloaded devsy-linux-amd64 binary before making it executable, preserving the existing installation flow.
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`:
- Around line 24-25: Update the workspace cleanup description in the lifecycle
steps to state that teardown is skipped when either --keep is passed or the
workspace already existed, while preserving the existing command-failure
behavior.
- Around line 117-118: Update the GitLab DinD example’s DOCKER_HOST
configuration to use a consistent connection mode: either configure TLS by
sharing /certs/client and setting DOCKER_TLS_CERTDIR, or change DOCKER_HOST to
the non-TLS Docker port and disable TLS certificate generation. Ensure the
surrounding DinD service and job variables match the selected mode.
---
Outside diff comments:
In @.github/workflows/pr-ci.yml:
- Line 25: Update every actions/checkout invocation in the workflow, including
the referenced locations, to set persist-credentials to false. For any later
step that requires GitHub access, pass an explicit least-privilege token rather
than relying on checkout’s persisted credentials.
---
Nitpick comments:
In
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`:
- Around line 99-103: Update the CI example’s actions/checkout reference to a
tested immutable commit SHA and replace the mutable Devsy latest-download URL
with a specific tested release version. Add checksum verification for the
downloaded devsy-linux-amd64 binary before making it executable, preserving the
existing installation flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7e9baae-8b6d-41de-a7ec-eecf451fa1ee
📒 Files selected for processing (12)
.github/workflows/pr-ci.ymlcmd/ci/ci.gocmd/ci/ci_test.gocmd/root.gocmd/workspace/build.gocmd/workspace/up/up.goe2e/e2e_suite_test.goe2e/tests/ci/ci.goe2e/tests/ci/testdata/ci/.devcontainer/devcontainer.jsone2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.jsonsites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdxsites/docs-devsy-sh/sidebars.js
🚧 Files skipped from review as they are similar to previous changes (10)
- sites/docs-devsy-sh/sidebars.js
- e2e/tests/ci/testdata/ci/.devcontainer/devcontainer.json
- e2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.json
- e2e/e2e_suite_test.go
- cmd/root.go
- e2e/tests/ci/ci.go
- cmd/workspace/build.go
- cmd/ci/ci_test.go
- cmd/ci/ci.go
- cmd/workspace/up/up.go
| 4. Deletes the workspace — always, even when the command fails — unless | ||
| `--keep` is passed. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the existing-workspace teardown exception.
The lifecycle contract also preserves a workspace that already existed. Update this to say cleanup is skipped when --keep is passed or when the workspace already existed.
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`
around lines 24 - 25, Update the workspace cleanup description in the lifecycle
steps to state that teardown is skipped when either --keep is passed or the
workspace already existed, while preserving the existing command-failure
behavior.
| variables: | ||
| DOCKER_HOST: tcp://docker:2376 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration\.mdx$|continuous-integration\.mdx$' || true
echo "== file excerpt =="
if [ -f "sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx" ]; then
nl -ba sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx | sed -n '100,135p'
fi
echo "== related DinD / Docker host / TLS mentions =="
rg -n "DOCKER_HOST|DOCKER_TLS_CERTDIR|/certs/client|2376|2375|docker:2376|din|DinD|services.*docker" sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx sites/docs-devsy-sh || trueRepository: devsy-org/devsy
Length of output: 308
🌐 Web query:
GitLab CI Docker-in-Docker DOCKER_HOST tcp://docker:2376 /certs/client DOCKER_TLS_CERTDIR TLS certificates
💡 Result:
When using Docker-in-Docker (DinD) in GitLab CI/CD, the DOCKER_HOST and DOCKER_TLS_CERTDIR variables are used to establish a secure connection between your job container and the Docker daemon running in the DinD service [1][2]. To configure DinD with TLS: 1. DOCKER_HOST: Set this to tcp://docker:2376. The 'docker' hostname refers to the alias of the service container, and port 2376 is the standard port for Docker daemon over TLS [1][3][2]. 2. DOCKER_TLS_CERTDIR: Set this to /certs. This instructs the Docker client and daemon where to store and locate the TLS certificates [1][4]. The DinD service automatically generates certificates on boot and creates /certs/client for communication, provided it is properly mounted as a volume in your runner's config.toml [1][2]. 3. Runner Configuration: Ensure your GitLab Runner is registered with privileged mode enabled and includes the /certs/client directory in its volumes configuration [2]. If you prefer to disable TLS to simplify the configuration (e.g., in trusted environments), you can set DOCKER_HOST to tcp://docker:2375 and DOCKER_TLS_CERTDIR to an empty string [1][4]. In this case, the daemon will listen on port 2375 without TLS [1].
Citations:
- 1: https://docs.gitlab.com/ci/docker/docker_in_docker/
- 2: https://docs.gitlab.com/17.11/ci/docker/using_docker_build/
- 3: https://docs.gitlab.com/ci/docker/docker_layer_caching/
- 4: https://docs.gitlab.com/18.6/ci/docker/using_docker_build/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt (using awk) =="
if [ -f "sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx" ]; then
awk '{printf "%6d %s\n", NR, $0}' sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx | sed -n '90,135p'
fi
echo "== related DinD / Docker host / TLS mentions =="
rg -n "DOCKER_HOST|DOCKER_TLS_CERTDIR|/certs/client|2376|2375|docker:2376|din|DinD|services.*docker" sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx sites/docs-devsy-sh || true
echo "== search devsy commands for cert context =="
rg -n "DOCKER_TLS_CERTDIR|/certs/client|certs/client|DOCKER_HOST|2376|2375" . --glob '!node_modules' --glob '!dist' --glob '!build' || trueRepository: devsy-org/devsy
Length of output: 45860
Make the GitLab DinD example connect to Docker.
Port 2376 requires TLS certificates shared with the job, but the example only points DOCKER_HOST at docker:2376 and does not configure /certs/client or DOCKER_TLS_CERTDIR. Use the TLS setup consistently or switch to the simpler non-TLS mode.
Simple non-TLS configuration
variables:
- DOCKER_HOST: tcp://docker:2376
+ DOCKER_HOST: tcp://docker:2375
+ DOCKER_TLS_CERTDIR: ""📝 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.
| variables: | |
| DOCKER_HOST: tcp://docker:2376 | |
| variables: | |
| DOCKER_HOST: tcp://docker:2375 | |
| DOCKER_TLS_CERTDIR: "" |
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`
around lines 117 - 118, Update the GitLab DinD example’s DOCKER_HOST
configuration to use a consistent connection mode: either configure TLS by
sharing /certs/client and setting DOCKER_TLS_CERTDIR, or change DOCKER_HOST to
the non-TLS Docker port and disable TLS certificate generation. Ensure the
surrounding DinD service and job variables match the selected mode.
d873000 to
56f8ff2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`:
- Around line 108-109: Update the documented Devsy CLI installation commands to
use a pinned concrete release or immutable asset URL instead of
releases/latest/download, then verify the downloaded binary against its
published checksum or attestation before chmod and installation. Keep the Linux
amd64 installation flow intact while ensuring verification fails before the
binary is executed or installed.
- Around line 108-109: Update the Devsy installation commands in the CI
documentation to install the downloaded binary through a writable location,
preferably using passwordless sudo with install for /usr/local/bin. Preserve
executable permissions and ensure the resulting devsy command remains available
on PATH.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c42d51ec-b342-4538-8f33-514ed7ba2d37
📒 Files selected for processing (15)
.github/workflows/pr-ci.ymlcmd/ci/ci.gocmd/ci/ci_test.gocmd/root.gocmd/workspace/build.gocmd/workspace/up/up.gocmd/workspace/up/up_validate.goe2e/e2e_suite_test.goe2e/tests/ci/ci.goe2e/tests/ci/testdata/ci/.devcontainer/devcontainer.jsone2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.jsonpkg/devcontainer/source.gopkg/devcontainer/source_test.gosites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdxsites/docs-devsy-sh/sidebars.js
🚧 Files skipped from review as they are similar to previous changes (11)
- e2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.json
- e2e/tests/ci/testdata/ci/.devcontainer/devcontainer.json
- e2e/e2e_suite_test.go
- cmd/root.go
- sites/docs-devsy-sh/sidebars.js
- cmd/ci/ci_test.go
- .github/workflows/pr-ci.yml
- e2e/tests/ci/ci.go
- cmd/workspace/build.go
- cmd/ci/ci.go
- cmd/workspace/up/up.go
| curl -fsSL https://github.com/devsy-org/devsy/releases/latest/download/devsy-linux-amd64 -o /usr/local/bin/devsy | ||
| chmod +x /usr/local/bin/devsy |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Pin and verify the CLI release before executing it.
releases/latest/download is mutable, and the workflow performs no checksum or attestation verification. Pin a concrete release or immutable asset, then verify its published checksum or attestation before installation. GitHub documents immutable releases and artifact attestations as supply-chain protections. (docs.github.com)
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`
around lines 108 - 109, Update the documented Devsy CLI installation commands to
use a pinned concrete release or immutable asset URL instead of
releases/latest/download, then verify the downloaded binary against its
published checksum or attestation before chmod and installation. Keep the Linux
amd64 installation flow intact while ensuring verification fails before the
binary is executed or installed.
Source: MCP tools
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Install the binary through a privileged or user-writable path.
The example writes directly to /usr/local/bin, which may not be writable by the GitHub runner user. Use sudo install or install into a user-writable directory and add it to PATH. GitHub-hosted Linux runners provide passwordless sudo for privileged installs. (docs.github.com)
Proposed fix
- curl -fsSL https://github.com/devsy-org/devsy/releases/latest/download/devsy-linux-amd64 -o /usr/local/bin/devsy
- chmod +x /usr/local/bin/devsy
+ curl -fsSL https://github.com/devsy-org/devsy/releases/latest/download/devsy-linux-amd64 -o /tmp/devsy
+ sudo install -m 0755 /tmp/devsy /usr/local/bin/devsy📝 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.
| curl -fsSL https://github.com/devsy-org/devsy/releases/latest/download/devsy-linux-amd64 -o /usr/local/bin/devsy | |
| chmod +x /usr/local/bin/devsy | |
| curl -fsSL https://github.com/devsy-org/devsy/releases/latest/download/devsy-linux-amd64 -o /tmp/devsy | |
| sudo install -m 0755 /tmp/devsy /usr/local/bin/devsy |
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`
around lines 108 - 109, Update the Devsy installation commands in the CI
documentation to install the downloaded binary through a writable location,
preferably using passwordless sudo with install for /usr/local/bin. Preserve
executable permissions and ensure the resulting devsy command remains available
on PATH.
Source: MCP tools
Add a top-level `devsy ci [flags] [source] -- <cmd>` command that builds an ephemeral devcontainer, runs a command inside it, and tears the workspace down afterwards. A non-zero exit from the command propagates as the exit code of `devsy ci`, making it a CI gate on any runner (GitHub Actions, GitLab, Jenkins). - Build+start via a headless `up` (no IDE, no host SSH config), then run the command through the exec path so it runs as the devcontainer's remote user. Pre-build/push stays the responsibility of `devsy build`. - Ephemeral by default: the workspace is always torn down (even on failure) unless --keep is passed or it already existed. SIGINT/SIGTERM is converted to context cancellation so a cancelled CI job still tears down; teardown runs on its own bounded context and its failure is surfaced. - Flags: --run-cmd (shell string via sh -c), --remote-env, --keep, --devcontainer, --cache-from, --workspace-env(-file), --init-env, --secrets-file, --feature-secrets-file, --platform, git/no-cache/machine, registered via the pkg/flags builder. - Extract shared devcontainer source resolution into devcontainer.ResolveSourceSpec, used by both up and ci. - Tests: cmd/ci unit tests, an e2e suite (build/run/teardown, exit-code, --keep, --remote-env, --workspace-env, --run-cmd, --secrets-file, --cache-from) wired into the pr-ci matrix, and docs with GitHub/GitLab examples.
56f8ff2 to
4cca9df
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`:
- Around line 8-10: Update the introductory description of devsy ci to say “a
workspace” and use the US-English spelling “afterward,” preserving the rest of
the explanation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 507cea66-719c-4fb3-b50c-005775d649a1
📒 Files selected for processing (16)
.github/workflows/pr-ci.ymlcmd/ci/ci.gocmd/ci/ci_test.gocmd/root.gocmd/workspace/build.gocmd/workspace/up/up.gocmd/workspace/up/up_devcontainer_source_test.gocmd/workspace/up/up_validate.goe2e/e2e_suite_test.goe2e/tests/ci/ci.goe2e/tests/ci/testdata/ci/.devcontainer/devcontainer.jsone2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.jsonpkg/devcontainer/source.gopkg/devcontainer/source_test.gosites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdxsites/docs-devsy-sh/sidebars.js
💤 Files with no reviewable changes (1)
- cmd/workspace/up/up_devcontainer_source_test.go
🚧 Files skipped from review as they are similar to previous changes (14)
- e2e/tests/ci/testdata/ci/.devcontainer/devcontainer.json
- e2e/tests/ci/testdata/secrets/.devcontainer/devcontainer.json
- e2e/e2e_suite_test.go
- cmd/root.go
- .github/workflows/pr-ci.yml
- cmd/workspace/build.go
- sites/docs-devsy-sh/sidebars.js
- cmd/workspace/up/up_validate.go
- pkg/devcontainer/source.go
- e2e/tests/ci/ci.go
- cmd/ci/ci.go
- cmd/workspace/up/up.go
- pkg/devcontainer/source_test.go
- cmd/ci/ci_test.go
| `devsy ci` builds an ephemeral devcontainer, runs a command inside it, and tears | ||
| the workspace down afterwards. It lets you verify that your `devcontainer.json` | ||
| still builds. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the introduction’s grammar.
Use “a workspace” and the US-English spelling “afterward”.
Proposed fix
-`devsy ci` builds an ephemeral devcontainer, runs a command inside it, and tears
-the workspace down afterwards. It lets you verify that your `devcontainer.json`
+`devsy ci` builds an ephemeral devcontainer, runs a command inside it, and tears
+the workspace down afterward. It lets you verify that your `devcontainer.json`
still builds.
### How it works
-1. Resolves an workspace from the current directory (or a given source).
+1. Resolves a workspace from the current directory (or a given source).Also applies to: 14-14
🧰 Tools
🪛 LanguageTool
[locale-violation] ~9-~9: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...inside it, and tears the workspace down afterwards. It lets you verify that your `devconta...
(AFTERWARDS_US)
🤖 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
`@sites/docs-devsy-sh/pages/developing-in-workspaces/continuous-integration.mdx`
around lines 8 - 10, Update the introductory description of devsy ci to say “a
workspace” and use the US-English spelling “afterward,” preserving the rest of
the explanation.
Source: Linters/SAST tools
1 participant
Summary
Adds a top-level
devsy ci [flags] [source] -- <cmd>command that builds an ephemeral devcontainer, runs a command inside it, and tears the workspace down afterwards.A non-zero exit from the command propagates as the exit code of
devsy ci, so a failing test or build fails the CI job. The workspace is always torn down (even on failure) unless--keepis passed or the workspace already existed.Design
devcontainers/citreats running a CI command and pre-building/pushing an image as separate concerns, and so does Devsy:upbuilds and starts the container, then the command runs via the existing exec path (as the devcontainer'sremoteUser, in the workspace folder). This is whatdevsy cidoes.devsy build --repository --push. Kept out ofcito avoid redundant builds.Implementation notes:
up.RunHeadless(...)entry point: no IDE launch, no host SSH config mutation.newTempSSHConfigwas exported (NewTempSSHConfig) socireuses it.Usage
Tests
go build ./...,go vet, and the fullcmdtest suite pass (12 new tests incmd/ci, golangci-lint clean).Notes / follow-ups
action.yml+ scripts) and dogfooding it in this repo'sdevcontainer.ymlare intentionally not in this PR; they depend on a published release tag and will follow separately.Summary by CodeRabbit
devsy cito run a command in an ephemeral devcontainer workspace with automatic teardown and optional retention via--keep, including--run-cmd, repeatable--remote-env, env/secrets injection, devcontainer selection, and--cache-from.devsy ciwith examples.devsy ciparsing/flags and expanded E2E CI coverage for teardown, failure exit codes,--keep, and injection options.