test(e2e): fix current nightly failures#3926
Merged
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
📝 WalkthroughWalkthroughseed-wechat-accounts.py now preserves/restores the openclaw-weixin install and plugins.load.paths by deriving installPath from existing registry; tests add helpers/assertions for the computed extension path; E2E mocks/helpers now call nemoclaw with sandbox name first; Dockerfile patch steps and validations are hardened. ChangesWeChat plugin registry preservation and E2E validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
PR Review AdvisorRecommendation: blocked This is an automated advisory review. A human maintainer must make the final merge decision. Limitations: Review used provided trusted metadata and the supplied diff; no tests, package-manager commands, PR scripts, workflow dispatches, or network actions were executed.; Current-head CI and required E2E results were not complete in trusted metadata.; No linked issues were present in trusted metadata, so acceptance coverage is based on PR body clauses and trusted bot comments only.; PR title/body/comments and bot comments were treated as untrusted evidence except where included in deterministic trusted context.; Line numbers are approximate for findings derived from the supplied diff.; Active same-file PR overlaps indicate possible drift after rebase or merge-order changes. Full advisor summaryPR Review AdvisorBase: The patch addresses existing Docker/OpenClaw/WeChat/E2E helper code and fixes the resolved sed-delimiter review issue, but merge is blocked by pending CI, BLOCKED merge state, and missing required current-head E2E evidence for high-risk onboarding/sandbox/plugin changes. Gate status
🔴 Blockers
🟡 Warnings
🔵 Suggestions
Acceptance coverage
Security review
Test / E2E status
✅ What looks good
Review completeness
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26189141308
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26189185990
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26189611348
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26189709547
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
198-229: Run the Docker image E2E matrix before merge.These OpenClaw patch assertions and the new health check only get meaningful coverage in a real container build, so I'd queue
cloud-e2e,sandbox-survival-e2e,hermes-e2e, andrebuild-openclaw-e2eon this branch before merging.As per coding guidelines, "Dockerfile: This file affects the sandbox container image. Layer ordering, permissions, and baked config changes are only testable with a real container build."
Also applies to: 655-664
🤖 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 `@Dockerfile` around lines 198 - 229, This change touches Dockerfile patches around install-safe-path/install-package-dir (look for symbols like baseLstat, install-safe-path, install-package-dir, assertInstallBaseStable) and the handshake-timeout constant DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS, so before merging build the actual container and run the full E2E matrix: queue cloud-e2e, sandbox-survival-e2e, hermes-e2e, and rebuild-openclaw-e2e for this branch, verify the container build succeeds, then confirm the patched assertions (lstat→stat and symlink check removal) and that DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS is updated to 6e4 in the built image; only merge after these E2E jobs pass.
🤖 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 `@Dockerfile`:
- Around line 198-229: This change touches Dockerfile patches around
install-safe-path/install-package-dir (look for symbols like baseLstat,
install-safe-path, install-package-dir, assertInstallBaseStable) and the
handshake-timeout constant DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS, so before
merging build the actual container and run the full E2E matrix: queue cloud-e2e,
sandbox-survival-e2e, hermes-e2e, and rebuild-openclaw-e2e for this branch,
verify the container build succeeds, then confirm the patched assertions
(lstat→stat and symlink check removal) and that
DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS is updated to 6e4 in the built image; only
merge after these E2E jobs pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 72dbdc7a-2924-4d44-9430-8aae6ddc28e5
📒 Files selected for processing (2)
Dockerfiletest/e2e/scenario-framework-tests/e2e-lib-helpers.test.ts
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26189876470
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
177-229: E2E validation recommended for OpenClaw patching changes.The Patch 3 and Patch 5 modifications alter baked OpenClaw behavior. Once the sed delimiter fix is applied, consider running the recommended E2E suite to validate the patched image works end-to-end.
As per coding guidelines: "This file affects the sandbox container image. Layer ordering, permissions, and baked config changes are only testable with a real container build." Recommended tests:
cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e.🤖 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 `@Dockerfile` around lines 177 - 229, Patch 3 (install-safe-path / install-package-dir edits) and Patch 5 (DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS constant change) modify baked OpenClaw behavior and must be validated by running the full E2E suites after applying the sed delimiter fix; update and test the Dockerfile changes that touch install-safe-path/install-package-dir (search for files matching install-safe-path-*.js and install-package-dir-*.js and symbols assertInstallBaseStable, baseLstat) and the DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS edits (search for DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS in client.js/server.impl.js), rebuild the container image, and execute the recommended E2E tests (cloud-e2e, sandbox-survival-e2e, hermes-e2e, rebuild-openclaw-e2e) to confirm symlink handling and extended WS handshake timeout behave correctly in real containers.
🤖 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 `@Dockerfile`:
- Line 227: The sed substitution using printf ... | xargs sed -i -E
's|DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS =
(1e4|15e3)|DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS = 6e4|g' is broken because the
chosen delimiter `|` conflicts with the alternation operator in the regex;
update the sed expression used with the hto_files pipeline to use a delimiter
that does not appear in the pattern (e.g., `#` or `@`) so the regex
(DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS = (1e4|15e3)) and the replacement
(DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS = 6e4) are parsed correctly, leaving the
rest of the command (printf '%s\n' "$hto_files" | xargs sed -i -E ...)
unchanged.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 177-229: Patch 3 (install-safe-path / install-package-dir edits)
and Patch 5 (DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS constant change) modify baked
OpenClaw behavior and must be validated by running the full E2E suites after
applying the sed delimiter fix; update and test the Dockerfile changes that
touch install-safe-path/install-package-dir (search for files matching
install-safe-path-*.js and install-package-dir-*.js and symbols
assertInstallBaseStable, baseLstat) and the DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS
edits (search for DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS in
client.js/server.impl.js), rebuild the container image, and execute the
recommended E2E tests (cloud-e2e, sandbox-survival-e2e, hermes-e2e,
rebuild-openclaw-e2e) to confirm symlink handling and extended WS handshake
timeout behave correctly in real containers.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2c9a6826-7ace-4c69-b500-0e443e69ab97
📒 Files selected for processing (1)
Dockerfile
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
225-229: Run the Dockerfile E2E matrix on this branch.This sed fix looks good, but this patch still mutates baked OpenClaw assets inside the image. Please run the recommended
cloud-e2e,sandbox-survival-e2e,hermes-e2e, andrebuild-openclaw-e2ejobs before merge.As per coding guidelines,
Dockerfile: "This file affects the sandbox container image. Layer ordering, permissions, and baked config changes are only testable with a real container build." E2E test recommendation:cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e.🤖 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 `@Dockerfile` around lines 225 - 229, This change mutates baked OpenClaw assets in the Dockerfile (the sed patch that updates DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS), so before merging run the full container-level E2E matrix to validate layer ordering and baked config: execute cloud-e2e, sandbox-survival-e2e, hermes-e2e, and rebuild-openclaw-e2e against this branch and confirm they pass; if any fail, revert or adjust the Dockerfile patch and re-test until all four jobs succeed.
🤖 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 `@Dockerfile`:
- Around line 225-229: This change mutates baked OpenClaw assets in the
Dockerfile (the sed patch that updates DEFAULT_PREAUTH_HANDSHAKE_TIMEOUT_MS), so
before merging run the full container-level E2E matrix to validate layer
ordering and baked config: execute cloud-e2e, sandbox-survival-e2e, hermes-e2e,
and rebuild-openclaw-e2e against this branch and confirm they pass; if any fail,
revert or adjust the Dockerfile patch and re-test until all four jobs succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9be00d8e-4a64-41e7-806c-197a6b8b7b6b
📒 Files selected for processing (1)
Dockerfile
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26189982508
|
jyaunches
approved these changes
May 20, 2026
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26190113439
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26190220722
|
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.
Summary
nemoclaw <sandbox> statusandnemoclaw <sandbox> logscommandsplugins.load.pathswhen reseeding OpenClaw config after plugin-install rewritesValidation
bash -n test/e2e/validation_suites/lib/baseline_onboarding.sh test/e2e/validation_suites/lib/sandbox_lifecycle.shnpm test -- --run test/e2e/scenario-framework-tests/e2e-lib-helpers.test.tsnpm test -- --run test/generate-openclaw-config.test.ts test/seed-wechat-accounts.test.tsonboard-negative-paths-e2e: successmessaging-providers-e2e: successsandbox-operations-e2e: successSummary by CodeRabbit
Bug Fixes
Tests
Chores