feat(cli): support Cloudflare named tunnels#4134
Conversation
|
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 optional ServiceOptions.cloudflareTunnelToken, implements getTunnelUrl to prefer named-tunnel HTTPS hostnames from cloudflared ingress JSON (fallback to trycloudflare), updates startAll/showStatus to use it and to pass the token via TUNNEL_TOKEN, centralizes isRecord/UnknownRecord, updates tests and docs. ChangesCloudflare Named Tunnel Support
Sequence DiagramsequenceDiagram
participant startAll
participant cloudflared
participant cloudflared_log as cloudflared.log
participant getTunnelUrl
startAll->>cloudflared: spawn (tunnel run with TUNNEL_TOKEN) or (tunnel --url ...)
cloudflared->>cloudflared_log: write ingress/config JSON or quick-tunnel lines
getTunnelUrl->>cloudflared_log: read & parse ingress JSON or trycloudflare URL
getTunnelUrl-->>startAll: return named HTTPS URL or trycloudflare URL
Estimated code review effort: Suggested reviewers:
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-4134.docs.buildwithfern.com/nemoclaw |
PR Review AdvisorFindings: 2 needs attention, 2 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Selective E2E Results — ✅ All requested jobs passedRun: 26340733625
|
Selective E2E Results — ✅ All requested jobs passedRun: 26340856367
|
Selective E2E Results — ❌ Some jobs failedRun: 26340961619
|
Selective E2E Results — ✅ All requested jobs passedRun: 26342326438
|
Selective E2E Results — ✅ All requested jobs passedRun: 26344160470
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/tunnel/services.ts (1)
137-152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrefer the newest tunnel URL from
cloudflared.log.Both parsers return the first match from the top of the log, but this file is append-only while
cloudflaredruns. After a reconnect or config reload,showStatus()andstartAll()can keep surfacing the stale earlier URL instead of the current one.Suggested fix
function extractTryCloudflareUrl(log: string): string | null { + let latestUrl: string | null = null; for (const rawToken of log.split(/\s+/)) { const candidate = rawToken.replace(/^[<("']+|[>),."']+$/g, ""); try { const url = new URL(candidate); if (url.protocol !== "https:") continue; if (url.hostname === "trycloudflare.com" || url.hostname.endsWith(".trycloudflare.com")) { url.hash = ""; - return url.toString(); + latestUrl = url.toString(); } } catch { // Not a URL token. } } - return null; + return latestUrl; } function extractNamedCloudflareUrl(log: string, dashboardPort: number): string | null { + let latestUrl: string | null = null; for (const match of log.matchAll(/config="((?:\\"|[^"])*)"/g)) { const escapedConfig = match[1]; if (!escapedConfig) continue; try { const configText = JSON.parse(`"${escapedConfig}"`) as string; const entries = getConfigIngressEntries(JSON.parse(configText) as unknown); for (const entry of entries) { if (!serviceTargetsDashboard(entry.service, dashboardPort)) continue; const url = formatNamedTunnelUrl(entry.hostname); - if (url) return url; + if (url) latestUrl = url; } } catch { // Fall through to the regex parser below for partial or unusual log lines. } } const port = String(dashboardPort).replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const servicePattern = new RegExp(`\\\\"service\\\\"\\s*:\\s*\\\\"http://localhost:${port}/?\\\\"`, "g"); for (const line of log.split(/\r?\n/)) { for (const serviceMatch of line.matchAll(servicePattern)) { const prefix = line.slice(0, serviceMatch.index ?? 0); let hostname: string | null = null; for (const hostnameMatch of prefix.matchAll(/\\"hostname\\"\s*:\s*\\"([^"\\]+)\\"/g)) { hostname = hostnameMatch[1] ?? null; } if (!hostname) continue; const url = formatNamedTunnelUrl(hostname); - if (url) return url; + if (url) latestUrl = url; } } - return null; + return latestUrl; }Also applies to: 189-229
🤖 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 `@src/lib/tunnel/services.ts` around lines 137 - 152, extractTryCloudflareUrl currently returns the first (oldest) match because it scans the log forward; change it to prefer the newest URL by scanning the log from the end (e.g., split log into tokens or lines and iterate in reverse) and return the first https trycloudflare.com match found; apply the same reverse-scan approach to the other parser(s) in this file (the second tryCloudflare parser around the later block) so showStatus()/startAll() surface the most recent tunnel URL instead of an earlier one.
🧹 Nitpick comments (2)
src/lib/state/sandbox.ts (1)
33-33: Run the state lifecycle E2E suite before merge.Because this file governs backup/restore/snapshot/rebuild persistence paths, run the recommended state E2Es on this branch.
As per coding guidelines "
src/lib/state/sandbox.ts: This file manages sandbox state (backup, restore, rebuild, snapshot). Changes affect data persistence across sandbox lifecycle operations." and E2E recommendations "state-backup-restore-e2e,snapshot-commands-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 `@src/lib/state/sandbox.ts` at line 33, This change touches sandbox state persistence in src/lib/state/sandbox.ts (imports like isRecord/UnknownRecord) so run the full state lifecycle E2E suites: state-backup-restore-e2e, snapshot-commands-e2e, and rebuild-openclaw-e2e; verify backup, restore, snapshot and rebuild scenarios pass, capture any failing case, and fix issues in the sandbox backup/restore/snapshot/rebuild handlers (the functions exported from sandbox.ts that implement those flows) until all three E2E suites succeed before merging.src/lib/shields/timer.ts (1)
13-13: Run the shields lifecycle E2E job before merge.Given this path controls shields timer behavior, run
shields-config-e2efor regression confidence.As per coding guidelines "
src/lib/shields/**: These files control shields down/up, config mutability, audit trail, and auto-restore timer." and "E2E test recommendation:shields-config-e2e— shields lifecycle + config get/set/rotate".🤖 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 `@src/lib/shields/timer.ts` at line 13, This change affects shields timer behavior in src/lib/shields/timer.ts (and related files under src/lib/shields/**), so before merging run the shields-config-e2e job (shields lifecycle + config get/set/rotate) to provide regression confidence; ensure the shields-config-e2e pipeline completes green, attach the job run/results to the PR, and do not merge until the E2E passes.
🤖 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 `@src/lib/tunnel/services.ts`:
- Around line 137-152: extractTryCloudflareUrl currently returns the first
(oldest) match because it scans the log forward; change it to prefer the newest
URL by scanning the log from the end (e.g., split log into tokens or lines and
iterate in reverse) and return the first https trycloudflare.com match found;
apply the same reverse-scan approach to the other parser(s) in this file (the
second tryCloudflare parser around the later block) so showStatus()/startAll()
surface the most recent tunnel URL instead of an earlier one.
---
Nitpick comments:
In `@src/lib/shields/timer.ts`:
- Line 13: This change affects shields timer behavior in
src/lib/shields/timer.ts (and related files under src/lib/shields/**), so before
merging run the shields-config-e2e job (shields lifecycle + config
get/set/rotate) to provide regression confidence; ensure the shields-config-e2e
pipeline completes green, attach the job run/results to the PR, and do not merge
until the E2E passes.
In `@src/lib/state/sandbox.ts`:
- Line 33: This change touches sandbox state persistence in
src/lib/state/sandbox.ts (imports like isRecord/UnknownRecord) so run the full
state lifecycle E2E suites: state-backup-restore-e2e, snapshot-commands-e2e, and
rebuild-openclaw-e2e; verify backup, restore, snapshot and rebuild scenarios
pass, capture any failing case, and fix issues in the sandbox
backup/restore/snapshot/rebuild handlers (the functions exported from sandbox.ts
that implement those flows) until all three E2E suites succeed before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bfa92dcb-41ae-4cd9-81c4-f01311e5f664
📒 Files selected for processing (5)
src/lib/shields/timer.tssrc/lib/skill-install.tssrc/lib/state/sandbox.tssrc/lib/tunnel/services.test.tssrc/lib/tunnel/services.ts
Selective E2E Results — ✅ All requested jobs passedRun: 26374556015
|
## Summary Refresh NemoClaw documentation and generated user skills for the v0.0.50 and v0.0.51 release-prep window. Remove obsolete legacy docs version metadata now that Fern docs no longer use `docs/project.json`, `docs/versions1.json`, or the legacy Sphinx config. ## Source summary - #1757 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document Slack channel allowlisting with `SLACK_ALLOWED_CHANNELS`. - #4134 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document Cloudflare named tunnel support through `CLOUDFLARE_TUNNEL_TOKEN`. - #4186 and #4135 -> `docs/inference/use-local-inference.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document Ollama upgrade and user-local install behavior. - #4185 -> `docs/network-policy/integration-policy-examples.mdx`, `docs/about/release-notes.mdx`: Clarify Jira policy validation probes. - Release cleanup -> `.claude/skills/nemoclaw-contributor-update-docs/SKILL.md`, `docs/CONTRIBUTING.md`, `.github/PULL_REQUEST_TEMPLATE.md`, `scripts/bump-version.ts`: Stop using legacy docs version JSON files and align docs verification on `npm run docs`. ## Changes - Add v0.0.50 and v0.0.51 release notes. - Regenerate NemoClaw user skills from the current Fern docs. - Remove obsolete `docs/conf.py`, `docs/project.json`, and `docs/versions1.json`. - Update docs workflow guidance and PR templates to use `npm run docs` instead of `make docs`. - Remove release-version JSON handling from `scripts/bump-version.ts`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional verification: - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` - `npm run build:cli` - `npx tsc --noEmit --allowSyntheticDefaultImports --module NodeNext --moduleResolution NodeNext --target ES2022 --types node scripts/bump-version.ts` - `ReadLints` on touched docs, skills, template, and script files - Searched for stale `versions1.json`, `project.json`, and `make docs` references Known gaps: - `npm run docs` was not rerun after cleanup because the earlier Fern CLI fetch failed with npm registry `403 Forbidden` in this environment. - A broad `npm run typecheck -- --noEmit` hit an unrelated existing `scripts/dev-tier-selector.js` type error. --- Signed-off-by: Miyoung Cho <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added resource profiling with CPU/RAM configuration controls for sandboxes * Enhanced local Ollama inference with automatic GPU memory-aware model fallback * Added `nemoclaw resources` command to display host hardware inventory * Enabled Cloudflare named tunnel support via environment configuration * **Documentation** * Improved setup guides for local inference, sandbox hardening, and policy validation * Enhanced troubleshooting for messaging delivery and host service routing * Added release notes for v0.0.50 and v0.0.51 * **Chores** * Updated build documentation commands from `make docs` to `npm run docs` <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4262?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Adds Cloudflare named tunnel support to
nemoclaw tunnel startusingCLOUDFLARE_TUNNEL_TOKEN.Named tunnel tokens are passed to
cloudflaredthroughTUNNEL_TOKENinstead of argv, and status/banner URL detection now handles named tunnel hostnames as well as quick tunnel URLs.Changes
src/lib/tunnel/services.tsstartscloudflared tunnel runwhenCLOUDFLARE_TUNNEL_TOKENis set, otherwise preserving quick-tunnel behavior.src/lib/tunnel/services.tsaddsgetTunnelUrl()parsing for named tunnelconfig=log entries and quick tunnel URLs.src/lib/tunnel/services.test.tsadds coverage for named tunnel URL parsing and verifies the token is not placed incloudflaredargv.docs/reference/commands.mdxanddocs/manage-sandboxes/messaging-channels.mdxdocument named tunnel usage fornemoclaw tunnel start.Type of Change
Verification
Targeted checks run locally:
npm run build:cli,npm run typecheck:cli, andnpx vitest run src/lib/tunnel/services.test.tspassed.Full
npx prek run --all-filesandnpm testare left for CI because local hooks/tests are currently hanging on this machine.npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit