test(e2e): add dashboard remote-bind regression guard for #3259#3410
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new E2E test suite ChangesDashboard Remote Bind E2E Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryPi Semantic E2E AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
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 @.github/workflows/regression-e2e.yaml:
- Around line 39-41: The concurrency group key used in the workflow (the
concurrency: group value: regression-e2e-${{ github.event_name }}-${{ github.ref
}}-${{ inputs.jobs || 'all' }}) is too coarse and can cancel unrelated PR runs;
update the group to include a per-run discriminator such as inputs.pr_number (or
another unique run identifier) so manual runs for different PRs or runs against
the same ref won’t cancel each other — modify the group expression to append ${
{ inputs.pr_number } } (or equivalent) to the existing key used by the
concurrency block.
🪄 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: b895cd5d-fb13-4622-8cd8-bab7418edd2e
📒 Files selected for processing (4)
.github/workflows/e2e-branch-validation.yaml.github/workflows/regression-e2e.yamltest/e2e/brev-e2e.test.tstest/e2e/test-dashboard-remote-bind.sh
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 `@test/e2e/test-dashboard-remote-bind.sh`:
- Around line 41-45: The test currently writes logs to a fixed path
/tmp/nemoclaw-dashboard-remote-bind-connect.log which can collide across runs;
change the test to create a unique temp file (mktemp) and store its path in a
variable, use that variable in the nemoclaw connect redirection (when invoking
with NEMOCLAW_DASHBOARD_BIND and SANDBOX_NAME), and register a cleanup trap to
remove the temp file on exit and to cat it on failure; ensure references to the
temp file variable replace the hardcoded path in both the success/failure
branches.
🪄 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: fecc7ae3-6841-4518-9809-65b98679ff87
📒 Files selected for processing (2)
.github/workflows/regression-e2e.yamltest/e2e/test-dashboard-remote-bind.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/regression-e2e.yaml
# Conflicts: # test/cli.test.ts
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed the current head against current main. This is a CI-only regression guard for #3259, merges cleanly, CodeRabbit's prior comments are addressed, and the remaining risk is limited to the manual regression E2E wiring.
Summary
Adds a failing-test-first regression E2E guard for #3259 in the new
regression-e2e.yamlholding-pen workflow.This guard verifies that an explicitly opted-in remote dashboard bind restarts the OpenShell dashboard forward on all interfaces (
0.0.0.0:<port>), rather than leaving the dashboard localhost-only.Regression holding pen
This is intentionally not added to scheduled
nightly-e2e.yaml. The job lives inregression-e2e.yamlso it can be explicitly dispatched while the fix is in flight and later reviewed/promoted into nightly when stable.Expected red-on-unfixed-code behavior
Until #3259 is fixed, the regression job is expected to fail with:
Once this merges,
/skill:nemoclaw-issue-kickoff #3259can produce the fix PR against this acceptance criterion.Validation
bash -n test/e2e/test-dashboard-remote-bind.shregression-e2e.yaml,e2e-branch-validation.yaml,nightly-e2e.yamlnpm run build:cliRelated: #3259
Summary by CodeRabbit
Tests
CI / Workflows