ci(deploy): isolate non-release runner runtime caches#724
Draft
G4614 wants to merge 28 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an always-on Tokyo e2e GitHub Actions workflow, an OIDC IAM provisioning script, API/runner build-and-deploy steps, pytest e2e execution with diagnostics, README and fixture updates, and a Dockerfile build-arg for configurable Nx builds. ChangesTokyo Always-on E2E Stack
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant AWS as AWS(ECR/ECS/SSM/S3/ALB)
participant API as API(ECS Service / ALB)
participant Runner as Runner(EC2 boxlite-runner)
participant Pytests as Pytests
GitHubActions->>AWS: obtain OIDC id-token & assume role
GitHubActions->>AWS: resolve ECS cluster, ALB DNS/target-group, runner instance, S3 bucket
GitHubActions->>AWS: build & push API image to ECR (apps/api/Dockerfile.source)
GitHubActions->>AWS: build runner binary and upload to S3 (builds/<sha>/boxlite-runner)
GitHubActions->>AWS: register new ECS task definition with new image -> update service (rolling deploy)
AWS->>API: deploy new task -> ALB health checks
GitHubActions->>Runner: send SSM AWS-RunShellScript to download binary, replace executable, restart systemd
GitHubActions->>AWS: ECS Exec into API task to run psql quota init
GitHubActions->>Pytests: install SDK, write credentials (from SSM), run pytest via ALB DNS
Pytests->>API: e2e requests via ALB DNS
GitHubActions->>AWS: on failure, fetch CloudWatch logs and runner journal via SSM
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
G4614
force-pushed
the
chore/e2e-required-merge-gate
branch
from
0cdf723 to
49a95e2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/e2e-cloud.yml:
- Around line 42-56: Add a top-level workflow permissions block to restrict the
default GITHUB_TOKEN to the minimum (e.g., permissions: contents: read) so jobs
do not inherit broad repo access; then explicitly opt individual jobs (like the
e2e job that currently needs id-token) into additional scopes by adding
id-token: write under that job's permissions. Locate the workflow's
concurrency/group declaration (e2e-cloud-shared) and the e2e job definition to
apply the changes so default token scope is locked down and only required
permissions are granted per-job.
- Around line 91-101: The workflow's change filters are too narrow (the filters:
relevant block only matches apps/api/src/** and sdks/python/src/**) while the
job actually builds the full apps/api Docker context and the entire sdks/python
package; update the changes filter to include the full contexts (e.g.,
apps/api/** and sdks/python/**) or explicitly add Dockerfile, package*.json,
pyproject.toml and other top-level files (e.g., apps/api/Dockerfile,
apps/api/package*.json, sdks/python/pyproject.toml) so edits that affect what
gets built will trigger the e2e workflow; apply the same widening to the other
similar filters referenced in the file.
In `@scripts/test/e2e/cases/conftest.py`:
- Around line 116-118: The environment check for BOXLITE_E2E_SKIP_PATH_VERIFY is
currently treating any non-empty value as true; change the conditional to
explicitly test for canonical truthy values (e.g., lowercased value in
("1","true","yes")) so that explicit disables like "0" or "false" do not skip
verification; update the if statement around the yield/return in conftest.py to
read the env var, normalize it (lower()), and compare against the accepted true
tokens (or use a standard parser like distutils.util.strtobool) before deciding
to yield and return.
🪄 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: a9da2708-7499-4191-886f-7cadda734569
📒 Files selected for processing (5)
.github/workflows/README.md.github/workflows/e2e-cloud.yml.github/workflows/e2e-stack.ymlscripts/ci/setup-e2e-cloud-oidc.shscripts/test/e2e/cases/conftest.py
💤 Files with no reviewable changes (1)
- .github/workflows/e2e-stack.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/test/e2e/cases/conftest.py (1)
116-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse explicit truthy parsing for
BOXLITE_E2E_SKIP_PATH_VERIFY.Line 116 currently enables skip for any non-empty value, so
0/falsestill bypass verification. That contradicts the docstring’s “=1” contract.Suggested minimal fix
- if os.environ.get("BOXLITE_E2E_SKIP_PATH_VERIFY"): + if os.environ.get("BOXLITE_E2E_SKIP_PATH_VERIFY", "").strip().lower() in {"1", "true", "yes"}: yield return🤖 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 `@scripts/test/e2e/cases/conftest.py` around lines 116 - 118, The environment-var check currently treats any non-empty BOXLITE_E2E_SKIP_PATH_VERIFY as truthy; change it to explicitly parse the intended truthy value (per docstring) by testing for "1" (e.g., os.environ.get("BOXLITE_E2E_SKIP_PATH_VERIFY") == "1") so only "1" skips path verification; update the conditional around the yield/return in conftest.py that references BOXLITE_E2E_SKIP_PATH_VERIFY accordingly.
🧹 Nitpick comments (1)
.github/workflows/e2e-cloud.yml (1)
87-88: ⚡ Quick winConsider pinning security-critical actions to commit SHAs.
Actions like
aws-actions/configure-aws-credentials,actions/checkout, andaws-actions/amazon-ecr-loginare security-sensitive (they handle credentials or code checkout). Pinning to commit SHAs instead of version tags prevents supply chain compromise if an action's repository is attacked.At minimum, pin the credential-handling actions:
# Example for configure-aws-credentials - uses: aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502 # v4.0.2Also applies to: 115-115, 118-118, 207-207, 229-229, 233-233, 464-464
🤖 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/e2e-cloud.yml around lines 87 - 88, The workflow uses version tags for security-sensitive actions (e.g., actions/checkout, dorny/paths-filter, aws-actions/configure-aws-credentials, aws-actions/amazon-ecr-login) which is vulnerable to supply-chain attacks; update each uses: entry for these actions to pin to a specific commit SHA instead of a tag (replace e.g. actions/checkout@v5 with the corresponding commit SHA for that release), doing this for all occurrences flagged (including the configure-aws-credentials and amazon-ecr-login usages) so the workflow references immutable commits.Source: Linters/SAST tools
🤖 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.
Duplicate comments:
In `@scripts/test/e2e/cases/conftest.py`:
- Around line 116-118: The environment-var check currently treats any non-empty
BOXLITE_E2E_SKIP_PATH_VERIFY as truthy; change it to explicitly parse the
intended truthy value (per docstring) by testing for "1" (e.g.,
os.environ.get("BOXLITE_E2E_SKIP_PATH_VERIFY") == "1") so only "1" skips path
verification; update the conditional around the yield/return in conftest.py that
references BOXLITE_E2E_SKIP_PATH_VERIFY accordingly.
---
Nitpick comments:
In @.github/workflows/e2e-cloud.yml:
- Around line 87-88: The workflow uses version tags for security-sensitive
actions (e.g., actions/checkout, dorny/paths-filter,
aws-actions/configure-aws-credentials, aws-actions/amazon-ecr-login) which is
vulnerable to supply-chain attacks; update each uses: entry for these actions to
pin to a specific commit SHA instead of a tag (replace e.g. actions/checkout@v5
with the corresponding commit SHA for that release), doing this for all
occurrences flagged (including the configure-aws-credentials and
amazon-ecr-login usages) so the workflow references immutable commits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 777a5470-4e26-49d6-a116-7147a3b7fed9
📒 Files selected for processing (5)
.github/workflows/README.md.github/workflows/e2e-cloud.yml.github/workflows/e2e-stack.ymlscripts/ci/setup-e2e-cloud-oidc.shscripts/test/e2e/cases/conftest.py
💤 Files with no reviewable changes (1)
- .github/workflows/e2e-stack.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/README.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/Dockerfile (1)
37-37: ⚡ Quick winUse
CACHE_BUSTinstead of a hardcoded epoch token.
ARG CACHE_BUST=1is declared on Line 16, but Line 37 hardcodesecho 1777389170; this makes the arg ineffective and obscures cache invalidation intent.Proposed change
-RUN echo 1777389170 && yarn nx build api --configuration=${NX_BUILD_CONFIG} --nxBail=true +RUN echo ${CACHE_BUST} && yarn nx build api --configuration=${NX_BUILD_CONFIG} --nxBail=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 `@apps/api/Dockerfile` at line 37, The RUN line currently echoes a hardcoded epoch token which ignores the declared ARG CACHE_BUST; update the RUN command in the Dockerfile (the line invoking echo and yarn nx build api --configuration=${NX_BUILD_CONFIG} --nxBail=true) to echo the CACHE_BUST build arg instead (use $CACHE_BUST or ${CACHE_BUST}) so the declared ARG CACHE_BUST takes effect for cache invalidation, and ensure ARG CACHE_BUST is present earlier in the Dockerfile.
🤖 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 `@apps/api/Dockerfile`:
- Line 37: The RUN line currently echoes a hardcoded epoch token which ignores
the declared ARG CACHE_BUST; update the RUN command in the Dockerfile (the line
invoking echo and yarn nx build api --configuration=${NX_BUILD_CONFIG}
--nxBail=true) to echo the CACHE_BUST build arg instead (use $CACHE_BUST or
${CACHE_BUST}) so the declared ARG CACHE_BUST takes effect for cache
invalidation, and ensure ARG CACHE_BUST is present earlier in the Dockerfile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e7f4cbe3-0a49-48c1-9444-cbff14532232
📒 Files selected for processing (2)
.github/workflows/e2e-cloud.ymlapps/api/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/e2e-cloud.yml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/e2e-cloud.yml (2)
154-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't take
TargetGroups[0]from the ALB.This block asserts there is one matching load balancer, but it never asserts there is one attached target group. If another TG gets attached later, the deploy health gate can watch the wrong backend and either pass too early or fail a healthy rollout.
Suggested hardening
- API_LB_TG_ARN=$(aws elbv2 describe-load-balancers \ - --query "LoadBalancers[?starts_with(LoadBalancerName,'ApiLoadBalancer-')]|[0].LoadBalancerArn" \ - --output text \ - | xargs -I{} aws elbv2 describe-target-groups --load-balancer-arn {} \ - --query "TargetGroups[0].TargetGroupArn" --output text) + API_LB_ARN=$(aws elbv2 describe-load-balancers \ + --query "LoadBalancers[?starts_with(LoadBalancerName,'ApiLoadBalancer-')]|[0].LoadBalancerArn" \ + --output text) + API_LB_TG_COUNT=$(aws elbv2 describe-target-groups \ + --load-balancer-arn "$API_LB_ARN" \ + --query "length(TargetGroups)" --output text) + assert_one "ApiLoadBalancer-* target group" "$API_LB_TG_COUNT" + API_LB_TG_ARN=$(aws elbv2 describe-target-groups \ + --load-balancer-arn "$API_LB_ARN" \ + --query "TargetGroups[0].TargetGroupArn" --output text)🤖 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/e2e-cloud.yml around lines 154 - 166, The script currently reads TargetGroups[0] for API_LB_TG_ARN which can pick the wrong TG if multiple exist; update the logic to query and count target groups attached to the ALB (use aws elbv2 describe-target-groups --load-balancer-arn {} with a length(...) query and store/check a TG_COUNT variable), call assert_one (or reuse existing assert_one) to ensure exactly one target group is attached, and only then retrieve the single TargetGroupArn into API_LB_TG_ARN (instead of unguarded TargetGroups[0]); ensure you reference LB_COUNT, API_LB_DNS, API_LB_TG_ARN, and the describe-target-groups call when making these changes.
42-47:⚠️ Potential issue | 🟠 MajorBind
workflow_dispatchruns to the trustede2e-cloudenvironment and avoid ambiguous ALB target-group selection.
jobs.e2ein.github/workflows/e2e-cloud.ymlhas noenvironment:;scripts/ci/setup-e2e-cloud-oidc.shtrust policy only allows...:ref:refs/heads/main,...:pull_request, or...:environment:e2e-cloud, soworkflow_dispatchon non-mainbranches won’t be able to assume the AWS role via OIDC.Suggested fix
e2e: name: E2E suite (Tokyo) needs: changes + environment: e2e-cloud # Skip on PRs that don't touch relevant paths. workflow_dispatch # always runs (operator explicitly asked for it). if: needs.changes.outputs.relevant == 'true' || github.event_name == 'workflow_dispatch'
- In
.github/workflows/e2e-cloud.yml(“Resolve stack resources”),api_lb_tg_arnis set usingTargetGroups[0].TargetGroupArnwithout asserting that exactly one target group is returned for the matched ALB; this can silently bind to the wrong target group. Add aTargetGroupscount/uniqueness assertion (or filter deterministically) before using index0.🤖 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/e2e-cloud.yml around lines 42 - 47, Add environment: e2e-cloud to the jobs.e2e definition so workflow_dispatch runs are bound to the trusted e2e-cloud environment (matching the OIDC trust conditions in scripts/ci/setup-e2e-cloud-oidc.sh); and in the “Resolve stack resources” step where api_lb_tg_arn is derived from TargetGroups[0].TargetGroupArn, assert/filter that the ALB returns exactly one matching TargetGroup (or deterministically filter by Name/Tags) before using index 0 to avoid silently selecting the wrong target group.
🧹 Nitpick comments (1)
apps/api/Dockerfile.source (1)
26-63: ⚡ Quick winRun container as non-root user.
The container runs as root by default, which violates the principle of least privilege. Even in an e2e test environment, following security best practices reduces attack surface. The official Node.js image includes a pre-configured
nodeuser (UID 1000).🔒 Proposed fix
Add the USER directive after installing dependencies but before copying application code:
COPY apps/nx.json apps/tsconfig.base.json apps/NOTICE ./ # tsx auto-discovers tsconfig.json in cwd — symlink so the path-alias # definitions in apps/tsconfig.base.json (the `@boxlite-ai/*` mappings) # resolve at import time without needing an explicit --tsconfig flag. RUN ln -sf tsconfig.base.json tsconfig.json ENV NX_DAEMON=false +USER node + COPY apps/api/ apps/api/🤖 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 `@apps/api/Dockerfile.source` around lines 26 - 63, The Dockerfile runs as root; update the boxlite-source stage to switch to the unprivileged node user after installing dependencies but before copying application files: create/ensure the /boxlite workdir is owned by node (chown -R node:node /boxlite) and add a USER node (UID 1000) directive before the COPY apps/api/ ... lines so subsequent COPY, HEALTHCHECK and ENTRYPOINT (yarn tsx apps/api/src/main.ts) run as the non-root node user; keep the existing RUN npm/yarn install steps as root then switch to node.Source: Linters/SAST tools
🤖 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 `@apps/api/Dockerfile.source`:
- Line 26: Set a non-root runtime user and make the application directory
writable to fix Trivy DS-0002: in the Dockerfile.source ensure you create or
verify the /boxlite directory, chown it to the existing node user (e.g., chown
-R node:node /boxlite), and then add USER node before the ENTRYPOINT so the
container runs as the node user rather than root; reference the Dockerfile's
ENTRYPOINT, the /boxlite path, and the USER directive when applying the changes.
---
Outside diff comments:
In @.github/workflows/e2e-cloud.yml:
- Around line 154-166: The script currently reads TargetGroups[0] for
API_LB_TG_ARN which can pick the wrong TG if multiple exist; update the logic to
query and count target groups attached to the ALB (use aws elbv2
describe-target-groups --load-balancer-arn {} with a length(...) query and
store/check a TG_COUNT variable), call assert_one (or reuse existing assert_one)
to ensure exactly one target group is attached, and only then retrieve the
single TargetGroupArn into API_LB_TG_ARN (instead of unguarded TargetGroups[0]);
ensure you reference LB_COUNT, API_LB_DNS, API_LB_TG_ARN, and the
describe-target-groups call when making these changes.
- Around line 42-47: Add environment: e2e-cloud to the jobs.e2e definition so
workflow_dispatch runs are bound to the trusted e2e-cloud environment (matching
the OIDC trust conditions in scripts/ci/setup-e2e-cloud-oidc.sh); and in the
“Resolve stack resources” step where api_lb_tg_arn is derived from
TargetGroups[0].TargetGroupArn, assert/filter that the ALB returns exactly one
matching TargetGroup (or deterministically filter by Name/Tags) before using
index 0 to avoid silently selecting the wrong target group.
---
Nitpick comments:
In `@apps/api/Dockerfile.source`:
- Around line 26-63: The Dockerfile runs as root; update the boxlite-source
stage to switch to the unprivileged node user after installing dependencies but
before copying application files: create/ensure the /boxlite workdir is owned by
node (chown -R node:node /boxlite) and add a USER node (UID 1000) directive
before the COPY apps/api/ ... lines so subsequent COPY, HEALTHCHECK and
ENTRYPOINT (yarn tsx apps/api/src/main.ts) run as the non-root node user; keep
the existing RUN npm/yarn install steps as root then switch to node.
🪄 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: 094c7c9a-9051-4c60-ac33-d16a8ef0a9a2
📒 Files selected for processing (2)
.github/workflows/e2e-cloud.ymlapps/api/Dockerfile.source
G4614
added a commit
that referenced
this pull request
Jun 11, 2026
Hide the AWS account ID + region + role ARN behind repo Variables — same pattern as .github/workflows/e2e-test.yml uses for AWS_ACCOUNT_ID. Previous version had them hardcoded because the original PR (#680) was a fork-PR and GitHub strips the `vars` context on fork-PR runs. PR #724 is now a same-repo PR (branch lives on boxlite-ai/boxlite), so the fork-PR workaround is no longer needed. Reads from: vars.AWS_E2E_CLOUD_REGION = ap-northeast-1 vars.AWS_E2E_CLOUD_ROLE_ARN = arn:aws:iam::<account>:role/boxlite-e2e-cloud-github-actions vars.AWS_ACCOUNT_ID (consumed by setup-e2e-cloud-oidc.sh) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
law-chain-hot
previously approved these changes
Jun 11, 2026
law-chain-hot
left a comment
law-chain-hot
left a comment
Contributor
There was a problem hiding this comment.
Approved to land the e2e-cloud workflow and run it from the updated branch.
G4614
force-pushed
the
chore/e2e-required-merge-gate
branch
2 times, most recently
from
c07f7cd to
8b66ae1
Compare
2 tasks
G4614
added a commit
that referenced
this pull request
Jun 12, 2026
Merging the substantive bits of #745 and #746 here so PR #724's deploy-app-services CI can actually push an otel-collector image: * otel-collector/builder-config.yaml: fix module paths after #730 moved everything under apps/ (was: ../../../api-client-go etc., now: ../../../apps/api-client-go). The OCB build was failing with `filepath does not exist: /boxlite/otel-collector/exporter`. (#745) * otel-collector/Dockerfile: apk add python3 make g++ so node-gyp can compile any musl-incompatible native add-on during workspace `yarn install`. Kept even after dropping dockerode below — small safety net in case future deps reintroduce a native build. (#745) * apps/package.json: drop unused dockerode + @types/dockerode. They were inherited from the Daytona import in #460 but never imported in source. Drops the dockerode -> docker-modem -> ssh2 -> cpu-features chain; cpu-features was the optional native addon that forced the toolchain workaround in the first place. (#746) * apps/yarn.lock: regenerated (-161 lines). * apps/api-client-go/api/openapi.yaml: regenerate to catch up with main's existing `assignedRoleIds.default: []` drift so the api-client-drift PR check passes.
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
G4614
added a commit
that referenced
this pull request
Jun 13, 2026
waitForToolboxReady was introduced in fc88aa0 (2026-06-05, "feat: add agent-ready runtime catalog", which then landed on main via PR #715 on 2026-06-10). It HTTP-polls http://127.0.0.1:<hostPort>/version expecting a daemon on guest TCP port 2280 — that interface dates from the Daytona-daemon era and was never reimplemented after the Rust guest agent rewrite landed in dbb11ec (2026-04-01). The new agent binds **vsock://2695** for gRPC + notifies the host via vsock://2696; nothing inside the VM listens on TCP:2280, so libkrun's port-forward accepts the SYN and immediately reset-by-peer's, and every CREATE_BOX times out 30 s in. Production data from a Tokyo runner: in 24 h, 490 CREATE_BOX events, 0 toolbox-ready successes, 181 toolbox-ready failures. The exec path that fires immediately afterward (via the same vsock gRPC channel) **always succeeds** — confirming the box VM is healthy, the readiness check itself is the bug. Remove the dead probe: drop waitForToolboxReady from client.go's Create and Start, drop the function + its TCP/HTTP imports, drop the toolboxReadyTimeout field, drop the ToolboxReadyTimeout/ DaemonStartTimeoutSec config plumbing in main.go + config.go, drop the two now-unreachable tests. Box readiness is now signalled by bx.Start(ctx) returning (which itself blocks on the vsock notification from the guest). Branched off chore/e2e-required-merge-gate (PR #724) so the e2e-cloud stack picks this up next dispatch.
DorianZheng
pushed a commit
that referenced
this pull request
Jun 15, 2026
**Draft.** Targets `chore/e2e-required-merge-gate` (PR #724) so the e2e-cloud stack picks this up on the next dispatch. `waitForToolboxReady` was introduced in `fc88aa0b` ("feat: add agent-ready runtime catalog") and landed on `main` via PR #715 on 2026-06-10. It HTTP-polls `http://127.0.0.1:<hostPort>/version` expecting a daemon on guest TCP port 2280 — the Daytona-daemon interface that was never reimplemented after the Rust guest agent rewrite in `dbb11ec8` (2026-04-01). The new agent binds **vsock://2695** for gRPC + notifies the host via vsock://2696; nothing in the VM listens on TCP:2280, so libkrun's port-forward accepts the SYN and the guest end immediately RST's, timing out every `CREATE_BOX` 30 s in. Production data from the live Tokyo runner: ``` $ sudo journalctl -u boxlite-runner --since "24 hours ago" | grep -E "CREATE_BOX|toolbox is ready|toolbox not ready" CREATE_BOX events: 490 toolbox-ready ok: 0 toolbox-ready fail: 181 ``` The exec request that fires immediately after CREATE_BOX (over the same vsock gRPC channel) **always succeeds** — confirming the box VM and the guest agent are healthy. The readiness probe itself is the bug. ## Test plan | | Before | After | |---|---|---| | `make test:integration:*` (PyO3 / FFI path, bypasses the runner) | ✓ pass | ✓ pass (unchanged) | | `go test ./pkg/boxlite/` | 4 tests, 2 of them assert against the dead HTTP probe | 2 tests (the dead two are deleted) | | `go build ./...` + `go vet ./...` | ✓ | ✓ | | Live Tokyo: `CREATE_BOX` for `docker.io/library/alpine:3.23` via e2e-cloud-test | always `box toolbox not ready after 30s` | expected to succeed (next dispatch on this branch) | | `journalctl -u boxlite-runner` post-deploy | `toolbox not ready` per CREATE_BOX | no `toolbox` lines at all (probe gone) | ## Iteration plan (no full e2e-cloud needed) 1. Dispatch `Deploy Runner Binary` on this branch — rebuilds libboxlite.a + the runner Go binary and pushes via SSH+SCP. 2. Dispatch `Deploy API` on this branch (no-op if no API changes, just to make sure the LB/health side is fresh). 3. Dispatch `E2E test (Tokyo)` standalone — runs the pytest suite against the freshly-deployed runner. That triple is ~30-40 min total vs running the full `e2e-cloud.yml` umbrella. ## Files * `apps/runner/pkg/boxlite/client.go` — drop the two `waitForToolboxReady` call sites + the `toolboxReadyTimeout` field + plumbing. * `apps/runner/pkg/boxlite/toolbox_ports.go` — delete `waitForToolboxReady`, drop now-unused `io / net/http / time` imports. * `apps/runner/pkg/boxlite/toolbox_ports_test.go` — delete the two tests that asserted against the dead probe. * `apps/runner/cmd/runner/main.go` — drop `ToolboxReadyTimeout: time.Duration(cfg.DaemonStartTimeoutSec) * time.Second`. * `apps/runner/cmd/runner/config/config.go` — drop `DaemonStartTimeoutSec` env var.
G4614
force-pushed
the
chore/e2e-required-merge-gate
branch
from
31f0de0 to
e77d9e8
Compare
G4614
added a commit
that referenced
this pull request
Jun 15, 2026
Hide the AWS account ID + region + role ARN behind repo Variables — same pattern as .github/workflows/e2e-test.yml uses for AWS_ACCOUNT_ID. Previous version had them hardcoded because the original PR (#680) was a fork-PR and GitHub strips the `vars` context on fork-PR runs. PR #724 is now a same-repo PR (branch lives on boxlite-ai/boxlite), so the fork-PR workaround is no longer needed. Reads from: vars.AWS_E2E_CLOUD_REGION = ap-northeast-1 vars.AWS_E2E_CLOUD_ROLE_ARN = arn:aws:iam::<account>:role/boxlite-e2e-cloud-github-actions vars.AWS_ACCOUNT_ID (consumed by setup-e2e-cloud-oidc.sh) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G4614
added a commit
that referenced
this pull request
Jun 15, 2026
Merging the substantive bits of #745 and #746 here so PR #724's deploy-app-services CI can actually push an otel-collector image: * otel-collector/builder-config.yaml: fix module paths after #730 moved everything under apps/ (was: ../../../api-client-go etc., now: ../../../apps/api-client-go). The OCB build was failing with `filepath does not exist: /boxlite/otel-collector/exporter`. (#745) * otel-collector/Dockerfile: apk add python3 make g++ so node-gyp can compile any musl-incompatible native add-on during workspace `yarn install`. Kept even after dropping dockerode below — small safety net in case future deps reintroduce a native build. (#745) * apps/package.json: drop unused dockerode + @types/dockerode. They were inherited from the Daytona import in #460 but never imported in source. Drops the dockerode -> docker-modem -> ssh2 -> cpu-features chain; cpu-features was the optional native addon that forced the toolchain workaround in the first place. (#746) * apps/yarn.lock: regenerated (-161 lines). * apps/api-client-go/api/openapi.yaml: regenerate to catch up with main's existing `assignedRoleIds.default: []` drift so the api-client-drift PR check passes.
Stripped-down e2e suite that runs pytest against the dev cloud stack without building or deploying the API or runner. C/Node/CLI artifacts are optional inputs; Python SDK is built from the checkout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Build Python, Node (napi-rs), C (libboxlite), and CLI from the checkout using BOXLITE_DEPS_STUB=1. All REST e2e tests run without skips — no pre-built artifact inputs needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Push to chore/e2e-required-merge-gate fires the dev e2e suite automatically. Rebase onto main → push → tests run. api_url defaults to dev API when triggered by push (no inputs). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The dev e2e suite only needs REST API access — no AWS credentials required. Uses BOXLITE_DEV_API_KEY repo secret instead of OIDC + SSM parameter fetch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
C and Go SDK REST tests exercise FFI bindings over HTTP — not a real user path. Remove their build steps and --ignore them in pytest to cut build time. Python, Node, and CLI cover the actual user surface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
napi build --js native/boxlite.js fails with "Failed to write js binding file" when native/ doesn't exist. Create it first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The hand-rolled napi command had wrong --js path. Use the package.json script which is the tested build path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Node tests failed because only napi native bindings were built but not the TypeScript dist/ (package.json main points to dist/index.js). Add `npm run build` after `build:native`. Exec-timeout tests time out on the single-node dev runner — skip them since they test runner-side kill semantics, not the REST API surface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
test_node_entry asserts runner journalctl contains the box ID, which is unreachable from GHA. test_node_coverage already exercises the Node SDK REST surface (exec, copy, errors) without journal checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Move the maintained e2e-dev.yml into this CI PR (it previously lagged the copy on chore/e2e-comprehensive-tests): run test_path_verification (no journal dependency — it proves the API→Runner chain via response headers), keep test_exec_timeout ignored with the accurate guest-side root cause now tracked in #910. The comprehensive-tests PR drops its copy so the workflow lives in one place. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Jul 3, 2026
Node cases shell out to the tsx driver; without -s pytest captures the subprocess stdout and the log shows only PASSED. -s surfaces each case's BOX_ID / marker / OK inline (the test-side echo lives in the comprehensive test PR). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 tasks
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
3 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.
Add runner build and rollout scripts while making non-release runner builds use dev sequence runtime cache keys, uploading local artifacts for rollout, and keeping verified versioned runner binaries for rollback.
Test plan: