test: add REST API E2E auth matrix and coverage tooling

[G4614]

Summary

Split from #817 — test-only changes.

• Add reusable REST API coverage inventory/report tooling
• Add REST E2E auth matrix support for AUTH=api-key and AUTH=oidc
• Add REST CLI smoke/full matrix runner
• Add CLI auth whoami integration test (Rust)
• Refactor existing E2E test fixtures to use shared auth helpers
• Document the reusable runbook in docs/testing/rest-api-e2e.md

Verification

make test:e2e:setup
make test:rest:inventory
make test:rest:e2e AUTH=api-key
make test:rest:cli AUTH=api-key SCOPE=smoke
make test:rest:report

Related

• Parent PR with bug fixes: test: add REST API E2E auth matrix #817

Summary by CodeRabbit

• Tests
  • Updated authentication and WebSocket attach-path matching assertions, removing an unnecessary tenant field for the canonical route while keeping legacy default-prefix behavior.
  • Simplified WebSocket/REST auth test coverage to key success/failure scenarios.
  • Refreshed E2E test setup to use shared auth/credentials fixtures, standardized image defaults (pinned agent-base), and simplified box-id parsing.
  • Added SDK E2E coverage for C and Go, plus Node SDK REST E2E scripts; added/adjusted Go/Node e2e entry flows.
  • Relaxed exec-timeout timing checks and removed the quota-test expected-failure marker.
  • Updated Jest transform settings to handle nanoid during tests.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Test specs for BoxliteWsProxyService are refactored with new auth harness helpers and revised match object assertions. E2E test infrastructure is refactored to use shared auth_context() and credentials_path() helpers instead of profile-based configuration. Existing E2E entry-point tests (C, CLI, Go, Node) and integration tests are updated to use the new auth infrastructure with simplified box-id matching and unconditional runner-journal verification. New E2E test modules and SDK driver implementations are added for C, Go, and Node SDKs covering copy, exec-options, and error-handling scenarios. Jest configuration is updated to support nanoid ESM transformation alongside uuid.

Changes

WebSocket Proxy Auth Test Refactor

Layer / File(s)

Summary

WebSocket routing and auth test updates apps/api/src/boxlite-rest/boxlite-rest-routing.spec.ts, apps/api/src/boxlite-rest/boxlite-ws-proxy.service.spec.ts

Canonical attach-path match expectation drops tenant: undefined; legacy route expectation remains tenant: 'default'. Import ordering adjusted; AuthResult, makeService, and authenticate wrapper replaced by authRequest and buildAuthHarness helpers. Broad matchAttachPath/authenticate suite replaced with four focused async cases covering API-key bearer success, JWT bearer success, invalid JWT rejection (skipping org lookup), and JWT rejection when org membership is removed.

E2E Test Infrastructure Refactoring and SDK Coverage Expansion

Layer / File(s)	Summary
E2E test infrastructure foundation scripts/test/e2e/cases/conftest.py, scripts/test/e2e/fixture_setup.py	conftest.py now imports and uses auth_context() and credentials_path() helpers instead of inline credential loading; the rt session fixture builds REST options from auth context; new session-scoped fixtures e2e_auth and e2e_credentials_path expose auth context to downstream tests; BOXLITE_E2E_IMAGE default changed from alpine:3.23 to pinned agent-base image tag; fixture setup constants updated to register the new image and honor BOXLITE_HOME for credentials path.
E2E entry-point tests refactoring scripts/test/e2e/cases/test_c_entry.py, scripts/test/e2e/cases/test_cli_entry.py, scripts/test/e2e/cases/test_cli_detach_recovery.py, scripts/test/e2e/cases/test_go_entry.py, scripts/test/e2e/cases/test_node_entry.py	C, CLI, Go, and Node entry tests updated to use auth_context() for API-key-only auth gating with simplified box-id regex patterns; CLI tests add _cli_env() helper for BOXLITE_PROFILE setup and replace SDK-build gating with direct pytest.skip; runner-journal verification made unconditional across all entry tests; test environments constructed from ctx.api_key_sdk_env(); CLI auth whoami validation weakened from exact URL match to generic status text check; detach-recovery test removes runner-journal verification setup.
E2E integration tests refactoring scripts/test/e2e/cases/test_quota_enforcement.py, scripts/test/e2e/cases/test_runner_concurrency.py	Quota enforcement and runner concurrency tests refactored to use auth_context() and request_json() helpers for HTTP request construction instead of local credential/profile loading and urllib. pytest.mark.xfail removed from test_memory_above_per_box_limit_returns_4xx, converting it from expected failure to normal assertion. Runner concurrency tests use auth_context()-derived URL construction and authentication headers in three test scenarios.
C SDK E2E coverage and driver implementation scripts/test/e2e/cases/test_c_coverage.py, scripts/test/e2e/sdks/c/e2e_exec.c, scripts/test/e2e/sdks/c/e2e_basic.c	New test_c_coverage.py module gates execution on API-key auth, gcc availability, and prebuilt libboxlite; compiles C driver binaries with fixed include/lib paths; runs compiled binaries with constructed environment and captures stdout/stderr; two tests validate exec/STDOUT callback path and error-typing behavior. New e2e_exec.c implements complete E2E executable: reads configuration from environment, creates Boxlite runtime with dedicated drain thread, creates box, executes echo HELLO-FROM-C, captures stdout via async callbacks with synchronized mutex/condition-variable contexts, records exit status, performs cleanup, and prints status markers. e2e_basic.c default image changed to pinned agent-base tag.
Go SDK E2E coverage and driver implementations scripts/test/e2e/cases/test_go_coverage.py, scripts/test/e2e/sdks/go/e2e_copy.go, scripts/test/e2e/sdks/go/e2e_exec_options.go, scripts/test/e2e/sdks/go/e2e_basic.go	New test_go_coverage.py module gates tests on API-key auth and go toolchain availability; three tests validate exec-options (working directory /tmp and environment variable propagation), copy round-trip (marked xfail strict for stdout-drain race), and error-typing behavior. New e2e_copy.go implements round-trip copy test: writes payload, CopyIn to container, verify via exec cat, CopyOut, verify download, cleanup. New e2e_exec_options.go implements exec with working directory and environment propagation. e2e_basic.go default image changed to pinned agent-base tag.
Node SDK E2E coverage and driver implementations scripts/test/e2e/cases/test_node_coverage.py, scripts/test/e2e/sdks/node/e2e_copy.ts, scripts/test/e2e/sdks/node/e2e_exec.ts, scripts/test/e2e/sdks/node/e2e_basic.ts	New test_node_coverage.py module gates tests on API-key auth, npx availability, and Node SDK NAPI artifacts; three tests validate exec (stdout stream capture), copy (marked xfail for stdout-drain race), and error-typing behavior via npx tsx invocations. New e2e_copy.ts implements round-trip copy test with temp directory management and cleanup via finally block. New e2e_exec.ts implements execution test with stdout streaming capture and non-zero exit code handling. e2e_basic.ts default image changed to pinned agent-base tag.
E2E timeout and Jest configuration updates scripts/test/e2e/cases/test_exec_timeout.py, apps/api/jest.config.ts	Exec timeout tests increased ex.wait() timeout from 15s to 45s and relaxed elapsed-time upper bounds from 10-12s to 30s. Jest configuration updated to allow ts-jest to transform both uuid and nanoid from node_modules during test execution.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

Possibly related PRs

• boxlite-ai/boxlite#678: Both PRs modify the E2E test harness around scripts/test/e2e/cases/conftest.py and auth setup, changing how REST auth context/credentials and images are derived.
• boxlite-ai/boxlite#683: Updates to E2E quota enforcement and runner concurrency tests overlap with the retrieved PR's addition of these test cases to the e2e suite.
• boxlite-ai/boxlite#809: Both PRs update the BoxliteWsProxyService.matchAttachPath websocket attach-path tests and refactor authentication expectations around tenant/JWT/API-key behaviors.

Suggested labels

enhancement, javascript

Suggested reviewers

• law-chain-hot
• DorianZheng

---

🐇 Whiskers twitch with test-suite cheer,
WsProxy auth flows now crystal clear!
E2E infrastructure refreshed anew,
C, Go, and Node drivers debut!
Jest transforms both uuid and nanoid too! 🚀

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.39% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The PR partially addresses objectives from issue #817: establishes E2E test infrastructure with shared auth helpers, updates multiple E2E test files, and adds new coverage tests. However, the PR does not complete core objectives: the OIDC auth matrix support is incomplete, the REST CLI matrix runner for both API-key and OIDC is not finished, and docs/testing/rest-api-e2e.md documentation is absent.	Clarify whether this PR is intended as a prerequisite foundation for #817 or if it should include complete OIDC support, full CLI matrix runner, and documentation. The commit messages suggest spec/fixture fixes rather than advancing the auth matrix goals.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main changes: adding REST API E2E authentication matrix and testing coverage tooling across multiple SDKs and test files.
Out of Scope Changes check	✅ Passed	All changes relate to E2E testing infrastructure: updates to test files, addition of new SDK coverage tests, fixture refactoring to use shared auth helpers, and Jest configuration adjustments for ESM dependencies. No changes appear to be unrelated to the stated E2E auth matrix and testing objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/drop-unused-dockerode

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[G4614]

Closing — see follow-up plan

[cla-assistant]

All committers have signed the CLA.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

scripts/test/e2e/sdks/c/e2e_exec.c (1)

59-59: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

strncpy does not guarantee null-termination when source exceeds buffer.

If err->message is >= 511 bytes, strncpy won't write a null terminator, and err_msg[1..511] are uninitialized. Same pattern at line 97.

Proposed fix

-        if (err->message) strncpy(ctx->err_msg, err->message, sizeof(ctx->err_msg) - 1);
+        if (err->message) {
+            strncpy(ctx->err_msg, err->message, sizeof(ctx->err_msg) - 1);
+            ctx->err_msg[sizeof(ctx->err_msg) - 1] = '\0';
+        }

Apply the same fix at line 97 in on_wait.

🤖 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/sdks/c/e2e_exec.c` at line 59, The strncpy call when copying
err->message to ctx->err_msg does not guarantee null-termination if the source
string is equal to or longer than the buffer size, leaving the buffer without a
proper string terminator. After calling strncpy with sizeof(ctx->err_msg) - 1,
explicitly null-terminate the buffer by setting
ctx->err_msg[sizeof(ctx->err_msg) - 1] = '\0'. Apply this same fix at both
locations: the current line 59 and also at line 97 in the on_wait function where
the same pattern exists.

scripts/test/e2e/fixture_setup.py (1)

7-10: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update stale setup text to match the new image and credential-path behavior.

Line 7 and Line 9 still describe alpine:3.23 and ~/.boxlite, but Line 46 and Line 49 now use the pinned agent-base image and BOXLITE_HOME override. Line 195 also logs a hardcoded path that may be wrong.

Suggested patch

-Configures:
-  1. Required snapshots active (alpine:3.23, ubuntu:22.04)
+Configures:
+  1. Required snapshots active (ghcr.io/boxlite-ai/boxlite-agent-base:20260605-p0-r3, ubuntu:22.04, ubuntu:24.04)
   2. Admin org has non-zero per-box quotas
-  3. `[profiles.p1]` in ~/.boxlite/credentials.toml points at the local API
+  3. `[profiles.p1]` in credentials.toml (at $BOXLITE_HOME or ~/.boxlite) points at the local API
@@
-def ensure_p1_profile(prefix: str):
-    """Write [profiles.p1] into ~/.boxlite/credentials.toml. Preserves
+def ensure_p1_profile(prefix: str):
+    """Write [profiles.p1] into credentials.toml at CRED_PATH. Preserves
     other profiles."""
@@
-    print(f"  ~/.boxlite/credentials.toml: profile p1 → {API_URL} (prefix {prefix})")
+    print(f"  {CRED_PATH}: profile p1/default → {API_URL} (prefix {prefix})")

Also applies to: 46-51, 157-159, 195-195

🤖 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/fixture_setup.py` around lines 7 - 10, Update the docstring
at the beginning of the file (lines 7-10) to accurately reflect the current
implementation behavior. Replace the reference to alpine:3.23 with the correct
pinned agent-base image that is actually being used in the fixture setup.
Replace the hardcoded ~/.boxlite/credentials.toml path reference with the
BOXLITE_HOME environment variable override approach that is shown in the actual
setup code at lines 46-51. Also review line 195 where a path is being logged to
ensure it reflects the current BOXLITE_HOME behavior and is not using a stale
hardcoded path reference.

scripts/test/e2e/cases/test_go_coverage.py (1)

55-58: ⚠️ Potential issue | 🟠 Major

Replace pytest.skip() with pytest.fail() for build failures in _run_go().

On line 58 of scripts/test/e2e/cases/test_go_coverage.py, pytest.skip() inside the CalledProcessError handler masks real compilation errors in the Go SDK driver. If go is available, compile failures should fail the test, not skip it, to catch regressions.

This same pattern exists in multiple test files (test_c_entry.py line 56, test_go_entry.py line 44, and test_c_coverage.py line 72) and should be fixed consistently across all of them.

Proposed fix for test_go_coverage.py

 def _run_go(go_env, src_name: str) -> subprocess.CompletedProcess:
     try:
         bin_path = _build_go(src_name)
     except subprocess.CalledProcessError as e:
-        pytest.skip(f"go build {src_name} failed: {e.stderr[:400]}")
+        pytest.fail(f"go build {src_name} failed: {e.stderr[:400]}")
     return subprocess.run(
         [str(bin_path)], env=go_env, timeout=180,
         capture_output=True, text=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 `@scripts/test/e2e/cases/test_go_coverage.py` around lines 55 - 58, In the
exception handler for subprocess.CalledProcessError when calling
_build_go(src_name) in the try-except block, replace the pytest.skip() call with
pytest.fail() to ensure that Go build failures are properly reported as test
failures rather than being silently skipped. This change should be applied
consistently across all affected test files: test_go_coverage.py (around the
_build_go call), test_c_entry.py (around the corresponding build call),
test_go_entry.py (around the corresponding build call), and test_c_coverage.py
(around the corresponding build call) to ensure that legitimate compilation
errors are caught and reported instead of masked by skip behavior.

scripts/test/e2e/cases/test_cli_entry.py (1)

66-72: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

whoami assertion is too weak and can pass on failure output.

This condition accepts output containing only "boxlite", so messages like “not logged in to boxlite” can incorrectly pass.

Proposed change

 def test_cli_whoami_against_api(cli):
     """`boxlite auth whoami` must return identity + server info."""
     r = run(cli, "auth", "whoami")
     out = r.stdout.lower()
-    assert "logged in" in out or "server" in out or "boxlite" in out, (
-        f"whoami output doesn't look like an auth status: {r.stdout!r}"
-    )
+    assert "not logged in" not in out, f"whoami reported unauthenticated state: {r.stdout!r}"
+    assert "server" in out, f"whoami missing server details: {r.stdout!r}"
+    assert ("logged in" in out or "authenticated" in out), (
+        f"whoami missing authenticated identity signal: {r.stdout!r}"
+    )

🤖 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/test_cli_entry.py` around lines 66 - 72, The assertion
in the test_cli_whoami_against_api function is too permissive because checking
for just "boxlite" in the output can match failure messages like "not logged in
to boxlite". Strengthen the assertion to only accept output that contains
"logged in" or "server", which are the indicators of successful authentication
status. Remove "boxlite" from the OR condition or restructure the assertion to
require more specific keywords that guarantee a successful whoami response
rather than just the presence of the application name.

🧹 Nitpick comments (7)

scripts/test/e2e/sdks/c/e2e_exec.c (2)

182-183: 💤 Low value

Error results from boxlite_execution_on_stdout and boxlite_execution_wait are not checked.

These calls pass &err but don't verify success. If either fails, the test could hang or produce misleading results.

Proposed fix

-    boxlite_execution_on_stdout(execution, on_stdout, &ectx, &err);
-    boxlite_execution_wait(execution, on_wait, &ectx, &err);
+    if (boxlite_execution_on_stdout(execution, on_stdout, &ectx, &err) != Ok)
+        DIE("execution_on_stdout: %d %s", err.code, err.message ? err.message : "");
+    if (boxlite_execution_wait(execution, on_wait, &ectx, &err) != Ok)
+        DIE("execution_wait: %d %s", err.code, err.message ? err.message : "");

🤖 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/sdks/c/e2e_exec.c` around lines 182 - 183, The calls to
boxlite_execution_on_stdout and boxlite_execution_wait both pass an error
pointer but do not check whether these operations succeeded. After each function
call, verify the error status (check if err indicates a failure) and handle the
error appropriately by logging it and skipping or terminating the test
execution. This ensures that if either function fails, the test does not proceed
with potentially invalid state or hang indefinitely.

---

132-132: 💤 Low value

Consider checking pthread_create return value.

If thread creation fails, the drain loop won't run and the program could hang waiting on condition variables. For robustness, check the return and handle failure.

Proposed fix

-    pthread_create(&drain_tid, NULL, drain_loop, &drain_args);
+    if (pthread_create(&drain_tid, NULL, drain_loop, &drain_args) != 0)
+        DIE("pthread_create failed");

🤖 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/sdks/c/e2e_exec.c` at line 132, The pthread_create call for
creating the drain_tid thread does not check the return value for success or
failure. Add a return value check after the pthread_create call to verify it
returns 0, and implement error handling logic to exit or report the error if
thread creation fails (non-zero return value), preventing potential hangs when
the drain_loop function never executes.

scripts/test/e2e/cases/test_cli_entry.py (1)

33-33: ⚡ Quick win

Make box-id extraction deterministic for CLI output.

BOX_ID_RE.search(...) can capture unrelated alphanumeric fragments. Extract from a full line/token intended to be the id.

Proposed change

-BOX_ID_RE = re.compile(r"[A-Za-z0-9]{8,36}")
+BOX_ID_RE = re.compile(r"^[A-Za-z0-9-]{8,36}$")
+
+def _extract_box_id(stdout: str) -> str | None:
+    for line in stdout.splitlines():
+        token = line.strip()
+        if BOX_ID_RE.fullmatch(token):
+            return token
+    return None
@@
-    m = BOX_ID_RE.search(r_run.stdout)
-    assert m, f"`boxlite run -d` did not print a uuid: {r_run.stdout!r}"
-    box_id = m.group(0)
+    box_id = _extract_box_id(r_run.stdout)
+    assert box_id, f"`boxlite run -d` did not print a box id: {r_run.stdout!r}"
@@
-    m = BOX_ID_RE.search(r_run.stdout)
-    assert m
-    box_id = m.group(0)
+    box_id = _extract_box_id(r_run.stdout)
+    assert box_id

Also applies to: 91-93, 123-125

🤖 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/test_cli_entry.py` at line 33, The BOX_ID_RE regex
pattern and its usage with .search() is too broad and can match unrelated
alphanumeric fragments in the CLI output, making the extraction
non-deterministic. Instead of using .search() which finds the pattern anywhere
in a string, refactor the box-id extraction logic to extract from complete lines
or tokens that are specifically intended to contain the box-id. Update the
BOX_ID_RE pattern definition and all its usages (at the initial pattern
definition, at lines 91-93, and at lines 123-125) to ensure the extraction
targets the full intended identifier token rather than searching within
arbitrary text.

scripts/test/e2e/cases/test_c_entry.py (1)

28-28: ⚡ Quick win

Parse BOX_ID from its marker, not from the first alphanumeric match.

Current extraction can grab an unrelated token from stdout. Since this test expects a BOX_ID marker, parse that marker explicitly to avoid flaky cleanup/journal assertions.

Proposed change

-BOX_ID_RE = re.compile(r"[A-Za-z0-9]{8,36}")
+BOX_ID_RE = re.compile(r"^BOX_ID=([A-Za-z0-9-]{8,36})$", re.MULTILINE)
@@
-    m = BOX_ID_RE.search(r.stdout)
+    m = BOX_ID_RE.search(r.stdout)
     assert m, f"C driver did not print BOX_ID: {r.stdout!r}"
-    box_id = m.group(0)
+    box_id = m.group(1)

Also applies to: 78-80

🤖 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/test_c_entry.py` at line 28, The BOX_ID_RE regex
pattern at line 28 is too broad and can match unrelated alphanumeric tokens from
stdout, causing flaky test behavior. Modify the BOX_ID_RE pattern to explicitly
include the BOX_ID marker (e.g., look for "BOX_ID" followed by its value) rather
than matching any arbitrary 8-36 character alphanumeric string. Apply the same
fix to the similar extraction logic at lines 78-80 to ensure consistent and
reliable parsing of the BOX_ID marker.

scripts/test/e2e/cases/test_go_entry.py (1)

21-21: ⚡ Quick win

Prefer marker-based BOX_ID parsing in Go entry test output.

The current first-match regex can capture unrelated stdout content. Use explicit BOX_ID= extraction to stabilize the assertion chain.

Proposed change

-BOX_ID_RE = re.compile(r"[A-Za-z0-9]{8,36}")
+BOX_ID_RE = re.compile(r"^BOX_ID=([A-Za-z0-9-]{8,36})$", re.MULTILINE)
@@
-    m = BOX_ID_RE.search(r.stdout)
+    m = BOX_ID_RE.search(r.stdout)
     assert m, f"go driver did not print BOX_ID: {r.stdout!r}"
-    box_id = m.group(0)
+    box_id = m.group(1)

Also applies to: 68-70

🤖 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/test_go_entry.py` at line 21, The BOX_ID_RE regex
pattern at the BOX_ID_RE definition is too broad and can match unrelated stdout
content, making the assertion unstable. Replace the pattern to explicitly match
the BOX_ID= marker followed by the ID value instead of just matching any 8-36
character alphanumeric string. This will anchor the pattern to the explicit
BOX_ID= prefix, ensuring it only captures the intended output. Apply the same
fix to the related parsing logic in lines 68-70 where this pattern is used.

scripts/test/e2e/cases/test_node_entry.py (1)

26-26: ⚡ Quick win

Parse Node driver BOX_ID via explicit marker capture.

Generic regex search is brittle when stdout contains other long tokens. Marker-based extraction is safer for runner-journal verification.

Proposed change

-BOX_ID_RE = re.compile(r"[A-Za-z0-9]{8,36}")
+BOX_ID_RE = re.compile(r"^BOX_ID=([A-Za-z0-9-]{8,36})$", re.MULTILINE)
@@
-    m = BOX_ID_RE.search(r.stdout)
+    m = BOX_ID_RE.search(r.stdout)
     assert m, f"node driver did not print BOX_ID: {r.stdout!r}"
-    box_id = m.group(0)
+    box_id = m.group(1)

Also applies to: 75-77

🤖 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/test_node_entry.py` at line 26, The BOX_ID_RE regex
pattern is too generic and can match unintended tokens in stdout, making the
extraction brittle. Replace the generic character pattern with an explicit
marker-based capture that matches the actual format used by the Node driver to
output BOX_ID (e.g., by including a specific prefix or delimiter that precedes
the actual BOX_ID value). Apply this same marker-based approach consistently to
all BOX_ID extraction locations, including the additional instances around lines
75-77 referenced in the comment, to ensure robust and reliable runner-journal
verification across all extraction points.

scripts/test/e2e/cases/test_cli_detach_recovery.py (1)

42-42: ⚡ Quick win

Use strict box-id token extraction for detach/recovery flows.

The current regex search can match unrelated text in stdout. A full-token parse avoids flaky detach reuse checks and cleanup calls.

Proposed change

-BOX_ID_RE = re.compile(r"[A-Za-z0-9]{8,36}")
+BOX_ID_RE = re.compile(r"^[A-Za-z0-9-]{8,36}$")
+
+def _extract_box_id(stdout: str) -> str | None:
+    for line in stdout.splitlines():
+        token = line.strip()
+        if BOX_ID_RE.fullmatch(token):
+            return token
+    return None
@@
-    m = BOX_ID_RE.search(r_run.stdout)
-    assert m, f"`boxlite run -d` did not print a uuid: {r_run.stdout!r}"
-    box_id = m.group(0)
+    box_id = _extract_box_id(r_run.stdout)
+    assert box_id, f"`boxlite run -d` did not print a box id: {r_run.stdout!r}"
@@
-    m = BOX_ID_RE.search(r_run.stdout)
-    assert m
-    box_id = m.group(0)
+    box_id = _extract_box_id(r_run.stdout)
+    assert box_id

Also applies to: 74-77, 115-118

🤖 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/test_cli_detach_recovery.py` at line 42, The BOX_ID_RE
regex pattern is too loose and can match unrelated text in stdout, causing flaky
detach and recovery operations. Replace the overly broad regex pattern at the
BOX_ID_RE definition with a stricter pattern that matches the complete box-id
token format using word boundaries or anchors. Then update all usages of
BOX_ID_RE throughout the file (including the regex.search calls around lines
74-77 and 115-118) to perform full-token parsing instead of partial matches,
ensuring only complete box-id tokens are extracted from the stdout output.

🤖 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.

Outside diff comments:
In `@scripts/test/e2e/cases/test_cli_entry.py`:
- Around line 66-72: The assertion in the test_cli_whoami_against_api function
is too permissive because checking for just "boxlite" in the output can match
failure messages like "not logged in to boxlite". Strengthen the assertion to
only accept output that contains "logged in" or "server", which are the
indicators of successful authentication status. Remove "boxlite" from the OR
condition or restructure the assertion to require more specific keywords that
guarantee a successful whoami response rather than just the presence of the
application name.

In `@scripts/test/e2e/cases/test_go_coverage.py`:
- Around line 55-58: In the exception handler for subprocess.CalledProcessError
when calling _build_go(src_name) in the try-except block, replace the
pytest.skip() call with pytest.fail() to ensure that Go build failures are
properly reported as test failures rather than being silently skipped. This
change should be applied consistently across all affected test files:
test_go_coverage.py (around the _build_go call), test_c_entry.py (around the
corresponding build call), test_go_entry.py (around the corresponding build
call), and test_c_coverage.py (around the corresponding build call) to ensure
that legitimate compilation errors are caught and reported instead of masked by
skip behavior.

In `@scripts/test/e2e/fixture_setup.py`:
- Around line 7-10: Update the docstring at the beginning of the file (lines
7-10) to accurately reflect the current implementation behavior. Replace the
reference to alpine:3.23 with the correct pinned agent-base image that is
actually being used in the fixture setup. Replace the hardcoded
~/.boxlite/credentials.toml path reference with the BOXLITE_HOME environment
variable override approach that is shown in the actual setup code at lines
46-51. Also review line 195 where a path is being logged to ensure it reflects
the current BOXLITE_HOME behavior and is not using a stale hardcoded path
reference.

In `@scripts/test/e2e/sdks/c/e2e_exec.c`:
- Line 59: The strncpy call when copying err->message to ctx->err_msg does not
guarantee null-termination if the source string is equal to or longer than the
buffer size, leaving the buffer without a proper string terminator. After
calling strncpy with sizeof(ctx->err_msg) - 1, explicitly null-terminate the
buffer by setting ctx->err_msg[sizeof(ctx->err_msg) - 1] = '\0'. Apply this same
fix at both locations: the current line 59 and also at line 97 in the on_wait
function where the same pattern exists.

---

Nitpick comments:
In `@scripts/test/e2e/cases/test_c_entry.py`:
- Line 28: The BOX_ID_RE regex pattern at line 28 is too broad and can match
unrelated alphanumeric tokens from stdout, causing flaky test behavior. Modify
the BOX_ID_RE pattern to explicitly include the BOX_ID marker (e.g., look for
"BOX_ID" followed by its value) rather than matching any arbitrary 8-36
character alphanumeric string. Apply the same fix to the similar extraction
logic at lines 78-80 to ensure consistent and reliable parsing of the BOX_ID
marker.

In `@scripts/test/e2e/cases/test_cli_detach_recovery.py`:
- Line 42: The BOX_ID_RE regex pattern is too loose and can match unrelated text
in stdout, causing flaky detach and recovery operations. Replace the overly
broad regex pattern at the BOX_ID_RE definition with a stricter pattern that
matches the complete box-id token format using word boundaries or anchors. Then
update all usages of BOX_ID_RE throughout the file (including the regex.search
calls around lines 74-77 and 115-118) to perform full-token parsing instead of
partial matches, ensuring only complete box-id tokens are extracted from the
stdout output.

In `@scripts/test/e2e/cases/test_cli_entry.py`:
- Line 33: The BOX_ID_RE regex pattern and its usage with .search() is too broad
and can match unrelated alphanumeric fragments in the CLI output, making the
extraction non-deterministic. Instead of using .search() which finds the pattern
anywhere in a string, refactor the box-id extraction logic to extract from
complete lines or tokens that are specifically intended to contain the box-id.
Update the BOX_ID_RE pattern definition and all its usages (at the initial
pattern definition, at lines 91-93, and at lines 123-125) to ensure the
extraction targets the full intended identifier token rather than searching
within arbitrary text.

In `@scripts/test/e2e/cases/test_go_entry.py`:
- Line 21: The BOX_ID_RE regex pattern at the BOX_ID_RE definition is too broad
and can match unrelated stdout content, making the assertion unstable. Replace
the pattern to explicitly match the BOX_ID= marker followed by the ID value
instead of just matching any 8-36 character alphanumeric string. This will
anchor the pattern to the explicit BOX_ID= prefix, ensuring it only captures the
intended output. Apply the same fix to the related parsing logic in lines 68-70
where this pattern is used.

In `@scripts/test/e2e/cases/test_node_entry.py`:
- Line 26: The BOX_ID_RE regex pattern is too generic and can match unintended
tokens in stdout, making the extraction brittle. Replace the generic character
pattern with an explicit marker-based capture that matches the actual format
used by the Node driver to output BOX_ID (e.g., by including a specific prefix
or delimiter that precedes the actual BOX_ID value). Apply this same
marker-based approach consistently to all BOX_ID extraction locations, including
the additional instances around lines 75-77 referenced in the comment, to ensure
robust and reliable runner-journal verification across all extraction points.

In `@scripts/test/e2e/sdks/c/e2e_exec.c`:
- Around line 182-183: The calls to boxlite_execution_on_stdout and
boxlite_execution_wait both pass an error pointer but do not check whether these
operations succeeded. After each function call, verify the error status (check
if err indicates a failure) and handle the error appropriately by logging it and
skipping or terminating the test execution. This ensures that if either function
fails, the test does not proceed with potentially invalid state or hang
indefinitely.
- Line 132: The pthread_create call for creating the drain_tid thread does not
check the return value for success or failure. Add a return value check after
the pthread_create call to verify it returns 0, and implement error handling
logic to exit or report the error if thread creation fails (non-zero return
value), preventing potential hangs when the drain_loop function never executes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 764b0f5d-408d-4798-afea-403875a94f3f

📥 Commits

Reviewing files that changed from the base of the PR and between 939f3b6 and a1fbc24.

📒 Files selected for processing (21)

• scripts/test/e2e/cases/conftest.py
• scripts/test/e2e/cases/test_c_coverage.py
• scripts/test/e2e/cases/test_c_entry.py
• scripts/test/e2e/cases/test_cli_detach_recovery.py
• scripts/test/e2e/cases/test_cli_entry.py
• scripts/test/e2e/cases/test_exec_timeout.py
• scripts/test/e2e/cases/test_go_coverage.py
• scripts/test/e2e/cases/test_go_entry.py
• scripts/test/e2e/cases/test_node_coverage.py
• scripts/test/e2e/cases/test_node_entry.py
• scripts/test/e2e/cases/test_quota_enforcement.py
• scripts/test/e2e/cases/test_runner_concurrency.py
• scripts/test/e2e/fixture_setup.py
• scripts/test/e2e/sdks/c/e2e_basic.c
• scripts/test/e2e/sdks/c/e2e_exec.c
• scripts/test/e2e/sdks/go/e2e_basic.go
• scripts/test/e2e/sdks/go/e2e_copy.go
• scripts/test/e2e/sdks/go/e2e_exec_options.go
• scripts/test/e2e/sdks/node/e2e_basic.ts
• scripts/test/e2e/sdks/node/e2e_copy.ts
• scripts/test/e2e/sdks/node/e2e_exec.ts

💤 Files with no reviewable changes (2)

• scripts/test/e2e/cases/test_runner_concurrency.py
• scripts/test/e2e/cases/test_quota_enforcement.py

✅ Files skipped from review due to trivial changes (2)

• scripts/test/e2e/sdks/go/e2e_basic.go
• scripts/test/e2e/sdks/c/e2e_basic.c

[coderabbitai]

🧹 Nitpick comments (1)

scripts/test/e2e/cases/test_go_entry.py (1)

21-21: 💤 Low value

Consider adding word boundaries for more precise matching.

The simplified regex [A-Za-z0-9]{12} will match the first 12 alphanumeric characters of any longer sequence. Adding word boundaries would ensure the match targets a standalone 12-character identifier, aligning with the intent to match exactly server IDs.

♻️ Suggested improvement

-BOX_ID_RE = re.compile(r"[A-Za-z0-9]{12}")
+BOX_ID_RE = re.compile(r"\b[A-Za-z0-9]{12}\b")

🤖 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/test_go_entry.py` at line 21, The BOX_ID_RE regex
pattern lacks word boundaries, which allows it to match any 12 consecutive
alphanumeric characters even when they appear as part of a longer sequence. Add
word boundary anchors to the BOX_ID_RE pattern to ensure it matches only
standalone 12-character server identifiers and not partial matches within longer
strings of characters.

🤖 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 `@scripts/test/e2e/cases/test_go_entry.py`:
- Line 21: The BOX_ID_RE regex pattern lacks word boundaries, which allows it to
match any 12 consecutive alphanumeric characters even when they appear as part
of a longer sequence. Add word boundary anchors to the BOX_ID_RE pattern to
ensure it matches only standalone 12-character server identifiers and not
partial matches within longer strings of characters.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d27d2d7b-193a-43b2-b5ac-5eb0a9f67627

📥 Commits

Reviewing files that changed from the base of the PR and between a1fbc24 and 3309383.

📒 Files selected for processing (5)

• scripts/test/e2e/cases/test_c_entry.py
• scripts/test/e2e/cases/test_cli_detach_recovery.py
• scripts/test/e2e/cases/test_cli_entry.py
• scripts/test/e2e/cases/test_go_entry.py
• scripts/test/e2e/cases/test_node_entry.py

🚧 Files skipped from review as they are similar to previous changes (4)

• scripts/test/e2e/cases/test_node_entry.py
• scripts/test/e2e/cases/test_c_entry.py
• scripts/test/e2e/cases/test_cli_entry.py
• scripts/test/e2e/cases/test_cli_detach_recovery.py

[coderabbitai]

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 `@apps/api/jest.config.ts`:
- Around line 14-16: The comment describing the transformIgnorePatterns
configuration incorrectly states that nanoid v5 ships as ESM-only, when it
actually ships as ESM-primary with CommonJS compatibility support. Update the
comment to accurately distinguish between uuid v14, which is strictly ESM-only,
and nanoid v5, which is ESM-primary but includes CommonJS compatibility options.
The rest of the comment explaining that ts-jest needs to down-level these
modules should 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: fb28e7f4-eaa1-4226-b5af-ed4f4f5d5319

📥 Commits

Reviewing files that changed from the base of the PR and between 3309383 and 42e0df8.

📒 Files selected for processing (3)

• apps/api/jest.config.ts
• apps/api/src/boxlite-rest/boxlite-rest-routing.spec.ts
• apps/api/src/boxlite-rest/boxlite-ws-proxy.service.spec.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• apps/api/src/boxlite-rest/boxlite-ws-proxy.service.spec.ts

[DorianZheng]

LGTM