fix(scripts): support named Cloudflare tunnels via CLOUDFLARE_TUNNEL_TOKEN#1369
Open
MauroDruwel wants to merge 25 commits intoNVIDIA:mainfrom
Open
fix(scripts): support named Cloudflare tunnels via CLOUDFLARE_TUNNEL_TOKEN#1369MauroDruwel wants to merge 25 commits intoNVIDIA:mainfrom
MauroDruwel wants to merge 25 commits intoNVIDIA:mainfrom
Conversation
…on issues on Linux
…_TOKEN When CLOUDFLARE_TUNNEL_TOKEN is set, start-services.sh runs 'cloudflared tunnel run --token' (named tunnel) instead of the default quick-tunnel. CLOUDFLARE_TUNNEL_HOSTNAME, if set, is used for display in the banner and 'nemoclaw status' output. - Refactor get_tunnel_url() helper to centralise URL resolution - Skip the trycloudflare.com URL-wait loop for named tunnels - Add 6 unit tests in test/service-env.test.js - Add docs/deployment/set-up-cloudflare-tunnel.md how-to guide - Update commands reference and nav
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR adds Cloudflare named-tunnel support: docs for configuring named tunnels and custom domains, code to detect/choose quick vs named tunnel URLs, script changes to start cloudflared using a token when provided, and tests covering tunnel URL extraction and behavior. Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant NemoClaw as NemoClaw Start
participant Cloudflared as cloudflared Process
participant CFZeroTrust as Cloudflare Zero Trust
participant Dashboard as Dashboard Port
User->>NemoClaw: nemoclaw start (with CLOUDFLARE_TUNNEL_TOKEN)
activate NemoClaw
NemoClaw->>NemoClaw: startAll() checks CLOUDFLARE_TUNNEL_TOKEN
NemoClaw->>Cloudflared: cloudflared tunnel run --token TOKEN
activate Cloudflared
Cloudflared->>CFZeroTrust: Register named tunnel & ingress routes
activate CFZeroTrust
CFZeroTrust-->>Cloudflared: Tunnel established (writes config to cloudflared.log)
deactivate CFZeroTrust
Cloudflared->>Dashboard: Forward https://{agent.domain} → localhost:DASHBOARD_PORT
Note over NemoClaw: Polling: get_tunnel_url() reads cloudflared.log
NemoClaw->>NemoClaw: getTunnelUrl() parses log for https://{hostname}
NemoClaw-->>User: Display Public URL: https://{agent.domain}
deactivate NemoClaw
User->>Dashboard: Access via custom domain
activate Dashboard
Dashboard-->>User: Dashboard UI
deactivate Dashboard
deactivate Cloudflared
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
wscurran
added
the
status: needs-info
Named tunnel hostname is configured in the Cloudflare Zero Trust dashboard — there is no need for a local env var to mirror it. The banner and status output simply omit the public URL for named tunnels. Removes all references across script, tests, and docs. https://claude.ai/code/session_01CrfL1YwX5to1mtKciY7LuV
Named tunnels log 'hostname=<domain>' when cloudflared loads ingress rules from the Cloudflare dashboard. Parse that line to display the public URL in the banner and 'nemoclaw status' — same as quick tunnels. Re-enable the URL wait loop for named tunnels. Add 3 tests covering unquoted hostname, quoted hostname, and no-log-file. https://claude.ai/code/session_01CrfL1YwX5to1mtKciY7LuV
Claude/summarize changes e2sr w
Resolved conflict in .agents/skills/nemoclaw-deploy-remote/SKILL.md by keeping the Cloudflare named tunnel documentation from this branch and adopting the correct step number (Step 11) from upstream. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MauroDruwel
changed the title
`|| return` propagated the non-zero exit of the failed `[ -f ]` test, causing get_tunnel_url to exit 1 in pipefail scripts and breaking the test assertion. Use `|| return 0` so the function always exits cleanly when no log file is present. Also fix unused variable and missing fenced-code language to satisfy ESLint and markdownlint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
docs/deployment/set-up-cloudflare-tunnel.md (3)
54-58: Consider restructuring field descriptions to avoid colons.Lines 55–58 use colons after field names to introduce explanations.
The coding guidelines restrict colons to introducing lists.
Consider rephrasing to avoid colons or use a different structure (e.g., bullet text without colons, or a table).As per coding guidelines: "Colons should only introduce a list. Flag colons used as general punctuation between clauses."
Alternative phrasing
2. Set the fields: - For **Subdomain**, enter the subdomain you want, e.g. `agent`. - For **Domain**, select the domain managed in your Cloudflare account, e.g. `mycompany.com`. - For **Service type**, select `HTTP`. - For **URL**, enter `localhost:18789` (or the value of `DASHBOARD_PORT` if you changed it).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/set-up-cloudflare-tunnel.md` around lines 54 - 58, Rephrase the four bullet items so they don't use colons as general punctuation; replace "Subdomain:", "Domain:", "Service type:", and "URL:" with phrasing like "For **Subdomain**, enter …", "For **Domain**, select …", "For **Service type**, select …", and "For **URL**, enter … (or the value of `DASHBOARD_PORT` if you changed it)"; update the lines that mention Subdomain, Domain, Service type, and URL accordingly to follow the colon rule.
151-151: Consider renaming to "Next Steps."The coding guidelines recommend a "Next Steps" section at the bottom of how-to pages.
This page uses "Related Topics" instead.As per coding guidelines: "A 'Next Steps' section at the bottom links to related pages."
Proposed change
-## Related Topics +## Next Steps🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/set-up-cloudflare-tunnel.md` at line 151, Rename the bottom section header "## Related Topics" to "## Next Steps" and update any accompanying text to match the coding guideline tone; specifically replace the header string "Related Topics" in the markdown (the existing line "## Related Topics") with "## Next Steps" and ensure the links under that section still point to the same related pages and read as recommended next-step guidance.
100-101: Replace em dash with a period or comma.The em dash on line 101 is used in place of a period, semicolon, or comma.
As per coding guidelines: "em dashes used instead of commas/periods should be flagged."
Proposed fix
-You can also verify from the Cloudflare dashboard under **Networks → Tunnels** — the tunnel -status should change to **Healthy** within a few seconds of starting. +You can also verify from the Cloudflare dashboard under **Networks → Tunnels**. +The tunnel status should change to **Healthy** within a few seconds of starting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/set-up-cloudflare-tunnel.md` around lines 100 - 101, The sentence containing "Networks → Tunnels — the tunnel status" uses an em dash incorrectly; replace the em dash with the appropriate punctuation (period or comma) so the line reads e.g. "Networks → Tunnels. The tunnel status should change to Healthy within a few seconds of starting." or "Networks → Tunnels, the tunnel status should change to Healthy within a few seconds of starting." Update that occurrence in the docs file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/deployment/set-up-cloudflare-tunnel.md`:
- Line 41: Several section headings in this document are prefixed with "Step X:"
which violates the no-numbering guideline; remove the "Step 1:", "Step 2:",
"Step 3:", "Step 4:", and "Step 5:" prefixes from the section titles so they
read as plain descriptive headings (e.g., change "Step 1: Create a Named Tunnel
in Cloudflare" to "Create a Named Tunnel in Cloudflare") — update the
corresponding heading strings in the file where the titles appear (the headings
containing "Create a Named Tunnel in Cloudflare" and the other step-prefixed
headings) to the same descriptive text without the "Step N:" prefix.
- Line 21: The H1 heading text does not match the frontmatter title.page value;
update the H1 at the top of the document so it exactly matches title.page ("Set
Up a Cloudflare Tunnel for NemoClaw") — locate the existing H1 "Set Up a
Cloudflare Tunnel" and replace it with the full frontmatter value to ensure the
H1 and title.page are identical.
- Line 61: Change the sentence "The full public URL will be
`https://agent.mycompany.com`." to present tense by rephrasing it to "The full
public URL is `https://agent.mycompany.com`." so the documentation describes
current behavior (update the line in docs/deployment/set-up-cloudflare-tunnel.md
accordingly).
- Line 47: The line containing "Copy the **tunnel token** displayed on the next
screen. It begins with `eyJ…`." should be split so each sentence is on its own
line; replace that single-line entry with two separate lines: one with "Copy the
**tunnel token** displayed on the next screen." and a second line with "It
begins with `eyJ…`." to follow the one-sentence-per-line guideline.
- Around line 25-27: The paragraph describing the quick tunnel for `nemoclaw
start` has multiple sentences on one line and uses an em dash instead of a
period; edit the text so each sentence is on its own line (split the sentence
that begins "This is convenient for trying things out..." onto a new line) and
replace the em dash before "the URL changes every time" with a period so the
sentence reads as a separate sentence explaining it's unsuitable for production
use.
- Line 39: The markdown line containing "NemoClaw installed and a sandbox
running. Follow the [Quickstart](../get-started/quickstart.md)." has two
sentences on one line; edit that paragraph so each sentence is on its own line
(first line: "NemoClaw installed and a sandbox running." and second line:
"Follow the [Quickstart](../get-started/quickstart.md).") to satisfy the
one-sentence-per-line guideline.
- Around line 129-131: Split the two sentences in the paragraph into separate
lines so each sentence is on its own line; specifically locate the sentence pair
"This stops `cloudflared` and any other auxiliary services." and "The Cloudflare
tunnel itself remains configured in the dashboard and reconnects automatically
the next time you run `nemoclaw start` with the same token." and break them into
two lines (one sentence per line) to follow the "one sentence per line"
guideline.
- Around line 69-71: Split the two sentences about CLOUDFLARE_TUNNEL_TOKEN and
nemoclaw start into separate lines so each sentence occupies its own line, and
replace the em dash with an appropriate period or semicolon; specifically,
ensure the sentence mentioning "CLOUDFLARE_TUNNEL_TOKEN switches nemoclaw start
from quick-tunnel mode to named-tunnel mode." is on its own line and the
follow-up sentence "The hostname and routing are configured in the Cloudflare
dashboard above — no local variable is needed." is rewritten without the em dash
(e.g., using a period: "The hostname and routing are configured in the
Cloudflare dashboard above. No local variable is needed.") while preserving the
original meaning.
In `@scripts/start-services.sh`:
- Around line 103-110: The logic currently gates named-tunnel URL extraction on
the CLOUDFLARE_TUNNEL_TOKEN env var, which prevents detecting an already-running
named tunnel when the token isn't exported; update the blocks that set/echo the
host (the host="$(grep -o 'hostname=...'" extraction and the grep for
trycloudflare URLs) so they first attempt to parse the named-host entry from
"$PIDDIR/cloudflared.log" (the hostname=... sed pipeline) and if that yields
nothing then fall back to the quick-tunnel grep
('https://[a-z0-9-]*\.trycloudflare\.com'); remove the dependency on
CLOUDFLARE_TUNNEL_TOKEN and apply the same change to the other occurrences
mentioned (the blocks around lines 125-129, 190-196, 209-210) so named-host
detection runs regardless of the env var and only then falls back to the
trycloudflare URL.
In `@test/service-env.test.js`:
- Around line 141-212: The tests duplicate the get_tunnel_url logic rather than
invoking the real implementation, so change each case to source the real
function from scripts/start-services.sh and call that function (e.g. run bash -c
'source scripts/start-services.sh; CLOUDFLARE_TUNNEL_TOKEN=... PIDDIR=...;
get_tunnel_url') instead of redefining get_tunnel_url inline; if
start-services.sh does not expose get_tunnel_url in a test-safe way, extract
that parser into a small helper script or make start-services.sh export a
get_tunnel_url function, then update the tests to source the file and call
get_tunnel_url, ensuring temporary PIDDIR setup/teardown remains the same.
---
Nitpick comments:
In `@docs/deployment/set-up-cloudflare-tunnel.md`:
- Around line 54-58: Rephrase the four bullet items so they don't use colons as
general punctuation; replace "Subdomain:", "Domain:", "Service type:", and
"URL:" with phrasing like "For **Subdomain**, enter …", "For **Domain**, select
…", "For **Service type**, select …", and "For **URL**, enter … (or the value of
`DASHBOARD_PORT` if you changed it)"; update the lines that mention Subdomain,
Domain, Service type, and URL accordingly to follow the colon rule.
- Line 151: Rename the bottom section header "## Related Topics" to "## Next
Steps" and update any accompanying text to match the coding guideline tone;
specifically replace the header string "Related Topics" in the markdown (the
existing line "## Related Topics") with "## Next Steps" and ensure the links
under that section still point to the same related pages and read as recommended
next-step guidance.
- Around line 100-101: The sentence containing "Networks → Tunnels — the tunnel
status" uses an em dash incorrectly; replace the em dash with the appropriate
punctuation (period or comma) so the line reads e.g. "Networks → Tunnels. The
tunnel status should change to Healthy within a few seconds of starting." or
"Networks → Tunnels, the tunnel status should change to Healthy within a few
seconds of starting." Update that occurrence in the docs file.
🪄 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: Pro
Run ID: b5b19414-64ff-490f-b234-ea2914aebd73
📒 Files selected for processing (9)
.agents/skills/nemoclaw-deploy-remote/SKILL.md.agents/skills/nemoclaw-reference/references/commands.mddocs/deployment/deploy-to-remote-gpu.mddocs/deployment/set-up-cloudflare-tunnel.mddocs/deployment/set-up-telegram-bridge.mddocs/index.mddocs/reference/commands.mdscripts/start-services.shtest/service-env.test.js
- scripts/start-services.sh: remove CLOUDFLARE_TUNNEL_TOKEN dependency from get_tunnel_url; detect named tunnel by hostname= log entry first, fall back to *.trycloudflare.com for quick tunnels - docs/deployment/set-up-cloudflare-tunnel.md: fix H1 to match frontmatter, remove numbered step prefixes from headings, one sentence per line, replace em dashes, present tense, rephrase bullet colons, rename Related Topics to Next Steps - test/service-env.test.js: update inline get_tunnel_url to match new implementation, remove unnecessary CLOUDFLARE_TUNNEL_TOKEN from detection tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
test/service-env.test.js (1)
141-208:⚠️ Potential issue | 🟠 MajorParser tests still duplicate
get_tunnel_url()implementation.These tests reimplement the shell function inline instead of exercising the real implementation from
scripts/start-services.sh, so they can drift from production behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/service-env.test.js` around lines 141 - 208, The tests duplicate the get_tunnel_url shell function instead of exercising the real implementation; update each test to source the production script and call its get_tunnel_url function (e.g., run bash -c "source scripts/start-services.sh; PIDDIR=...; mkdir -p \"$PIDDIR\"; get_tunnel_url") rather than defining get_tunnel_url inline, ensuring PIDDIR is set the same way the real script expects and cleaning up the temp directory afterwards so the behavior matches the production implementation.docs/deployment/set-up-cloudflare-tunnel.md (1)
108-110:⚠️ Potential issue | 🟡 MinorKeep exactly one full sentence per source line.
These ranges split single sentences across multiple lines. The style guide requires one complete sentence per line for readable diffs.
As per coding guidelines: "One sentence per line in source (makes diffs readable)."
Also applies to: 133-134, 143-144, 147-148, 151-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/set-up-cloudflare-tunnel.md` around lines 108 - 110, The paragraph starting with "To avoid exporting the variables on every shell session, add them to your shell profile or a `.env` file that is sourced before running `nemoclaw start`:" is split across multiple source lines; collapse it so each physical line contains exactly one full sentence (one sentence per line). Do the same for the other split sentences referenced (the blocks around lines 133-134, 143-144, 147-148, and 151-152) so every sentence is self-contained on a single line; locate those sentences by their text in the document and reflow them accordingly to follow the "one sentence per line" style rule.
🧹 Nitpick comments (2)
docs/deployment/set-up-cloudflare-tunnel.md (2)
8-8: Use canonical product casing in frontmatter tags.
"openclaw"should use the canonical casing ("OpenClaw").As per coding guidelines: "OpenClaw | Incorrect: openclaw (in prose), Openclaw."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/set-up-cloudflare-tunnel.md` at line 8, Update the frontmatter tags to use the canonical product casing: change the "tags" array entry "openclaw" to "OpenClaw" so the tags line (the frontmatter key "tags") uses the correct casing per guidelines.
36-37: Add a one-sentence intro immediately after each H2 heading.Several H2 sections start directly with a list or code block. New docs pages in this repo require each H2/H3 to begin with an introductory sentence.
As per coding guidelines: "Sections use H2 and H3, each starting with an introductory sentence."
Also applies to: 43-44, 66-67, 76-77, 95-96, 126-127, 154-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/deployment/set-up-cloudflare-tunnel.md` around lines 36 - 37, Several H2/H3 headings in this document (for example the H2 "Prerequisites") jump straight into lists or code blocks; add a single concise introductory sentence immediately after each H2 and H3 heading in this file describing what the section covers (e.g., "Prerequisites: a brief list of tools and access needed to follow this guide."), ensure the sentence is one line, descriptive and placed before any lists or code blocks, and apply the same change to all H2/H3 headings throughout the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/deployment/set-up-cloudflare-tunnel.md`:
- Line 140: Update the markdown link labeled "[cloudflared releases page]" so it
points to the official Cloudflare documentation for cloudflared instead of the
GitHub releases page; find the link text "[cloudflared releases page]" in
docs/deployment/set-up-cloudflare-tunnel.md and replace its URL with the
official Cloudflare docs URL (e.g., Cloudflare's cloudflared installation or
downloads documentation) to comply with the "no third-party code repository"
guideline.
In `@test/service-env.test.js`:
- Around line 125-130: The test currently sets CLOUDFLARE_TUNNEL_TOKEN to an
empty string instead of truly unsetting it; update the test that uses execSync
and getTokenLine so the child process runs without the CLOUDFLARE_TUNNEL_TOKEN
key in the env (e.g., clone process.env and delete/omit CLOUDFLARE_TUNNEL_TOKEN
before passing to execSync) rather than setting it to ""; ensure the
expect(result).toBe("") assertion remains to validate the absent-variable
behavior.
---
Duplicate comments:
In `@docs/deployment/set-up-cloudflare-tunnel.md`:
- Around line 108-110: The paragraph starting with "To avoid exporting the
variables on every shell session, add them to your shell profile or a `.env`
file that is sourced before running `nemoclaw start`:" is split across multiple
source lines; collapse it so each physical line contains exactly one full
sentence (one sentence per line). Do the same for the other split sentences
referenced (the blocks around lines 133-134, 143-144, 147-148, and 151-152) so
every sentence is self-contained on a single line; locate those sentences by
their text in the document and reflow them accordingly to follow the "one
sentence per line" style rule.
In `@test/service-env.test.js`:
- Around line 141-208: The tests duplicate the get_tunnel_url shell function
instead of exercising the real implementation; update each test to source the
production script and call its get_tunnel_url function (e.g., run bash -c
"source scripts/start-services.sh; PIDDIR=...; mkdir -p \"$PIDDIR\";
get_tunnel_url") rather than defining get_tunnel_url inline, ensuring PIDDIR is
set the same way the real script expects and cleaning up the temp directory
afterwards so the behavior matches the production implementation.
---
Nitpick comments:
In `@docs/deployment/set-up-cloudflare-tunnel.md`:
- Line 8: Update the frontmatter tags to use the canonical product casing:
change the "tags" array entry "openclaw" to "OpenClaw" so the tags line (the
frontmatter key "tags") uses the correct casing per guidelines.
- Around line 36-37: Several H2/H3 headings in this document (for example the H2
"Prerequisites") jump straight into lists or code blocks; add a single concise
introductory sentence immediately after each H2 and H3 heading in this file
describing what the section covers (e.g., "Prerequisites: a brief list of tools
and access needed to follow this guide."), ensure the sentence is one line,
descriptive and placed before any lists or code blocks, and apply the same
change to all H2/H3 headings throughout the document.
🪄 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: Pro
Run ID: 5be4565e-9f5e-49b7-bfb2-962d6744cc1a
📒 Files selected for processing (3)
docs/deployment/set-up-cloudflare-tunnel.mdscripts/start-services.shtest/service-env.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/start-services.sh
- scripts/start-services.sh: add BASH_SOURCE guard around dispatch so the script can be safely sourced in tests; use BASH_SOURCE[0] for SCRIPT_DIR resolution so it works correctly when sourced - docs/deployment/set-up-cloudflare-tunnel.md: one sentence per line throughout, fix tag casing (openclaw -> OpenClaw), replace GitHub releases link with official Cloudflare docs, add introductory sentence after each H2 heading - test/service-env.test.js: source get_tunnel_url from the real script instead of inlining it; unset CLOUDFLARE_TUNNEL_TOKEN instead of setting to empty string; move scriptPath to outer describe scope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/service-env.test.js`:
- Around line 143-175: Replace the brittle deterministic PIDDIR usage in the
three get_tunnel_url tests with mkdtempSync-created temp dirs and ensure each
test cleans up the temp directory in a try/finally block; specifically, change
the first test to set piddir = mkdtempSync(join(tmpdir(), "test-nonexistent-")),
and for the two named-tunnel tests wrap writeFileSync/execSync/unlinkSync in
try/finally where the finally removes the temp directory (rmdirSync or rmSync)
so piddir is always removed after calling the script (the tests reference
piddir, scriptPath and invoke get_tunnel_url via execSync).
🪄 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: Pro
Run ID: 32e2c976-e62d-4143-9c29-26c1ff17dbe2
📒 Files selected for processing (3)
docs/deployment/set-up-cloudflare-tunnel.mdscripts/start-services.shtest/service-env.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/start-services.sh
Use mkdtempSync for all three get_tunnel_url tests and wrap each in
try/finally with execFileSync("rm", ["-rf", piddir]) cleanup, matching
the pattern used throughout the rest of the test file. Quotes scriptPath
and piddir in the bash -c invocation to handle paths with spaces.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Named tunnels via token log the hostname inside a JSON-encoded config= line rather than as a plain hostname= field. Match the ingress entry whose service targets DASHBOARD_PORT to correctly handle multi-route tunnel configs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@wscurran I entered all needed information and tested the functionality now, so the label can prob be removed :)) |
Contributor
|
I attempted to port this branch across the JS→TS migration and merge the latest Please start with: git fetch origin
git merge origin/main
npx tsx scripts/ts-migration-assist.ts --base origin/main --write
npm run build:cli
npm run typecheck:cli
npm run lint
npm test |
…vices.ts - Resolved 8 merge conflicts from the JS→TS migration in upstream main: skill files (deleted as autogenerated), docs deployment links, toctree, commands.md env-var table, start-services.sh SCRIPT_DIR lines, and test rename - Ported CLOUDFLARE_TUNNEL_TOKEN named-tunnel support from start-services.sh to src/lib/services.ts (now the canonical implementation since nemoclaw start calls startAll() from compiled TS, not the bash script directly) - Fixed getTunnelUrl() to correctly select the hostname matching DASHBOARD_PORT in multi-route named-tunnel configs (matches start-services.sh get_tunnel_url fix) - Hardened cloudflared tests with shims that output a trycloudflare URL to avoid the 15-second wait loop on machines where cloudflared is installed - Regenerated agent skills from updated docs per CLAUDE.md instructions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The bash get_tunnel_url was duplicating the URL-detection logic from
src/lib/services.ts and had a bug: cloudflared emits pretty-printed JSON
with a space after each comma ("hostname": "...", "service": "...") which
the grep pattern did not account for, so named-tunnel URLs were never shown.
- scripts/start-services.sh: replace the grep/sed implementation with a
thin Node shim that calls getTunnelUrl() from dist/lib/services.js —
single source of truth, no duplicate parsing logic
- scripts/start-services.sh: add SCRIPT_DIR so the shim can locate dist/
- src/lib/services.ts: export getTunnelUrl so it can be tested and called
from the bash shim
- src/lib/services.test.ts: add five direct unit tests for getTunnelUrl,
including the space-after-comma case that was previously untested
- test/service-env.test.ts: add test for space-after-comma format; update
multi-route test to use real cloudflared log format
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai summary |
Contributor
✅ Actions performedSummary regeneration triggered. |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
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)
.agents/skills/nemoclaw-user-deploy-remote/SKILL.md (1)
3-322:⚠️ Potential issue | 🟠 MajorDo not edit generated skill files directly.
This file appears to be a generated artifact under
.agents/skills/...and should be regenerated fromdocs/sources instead of being edited directly. Please move these content changes to the correspondingdocs/source and re-run the skill generation flow.As per coding guidelines, "
.agents/skills/nemoclaw-user-*/*.md: Never edit.agents/skills/nemoclaw-user-*/*.mddirectly; regenerate skills fromdocs/source".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-user-deploy-remote/SKILL.md around lines 3 - 322, This file is a generated skill under .agents/skills/nemoclaw-user-deploy-remote/SKILL.md and must not be edited directly; move your content changes into the canonical docs source (the docs/ source that generates the nemoclaw-user-deploy-remote skill), update the source there (not the generated .agents copy), then run the skill generation flow to regenerate SKILL.md; reference the generated artifact by its path (.agents/skills/nemoclaw-user-deploy-remote/SKILL.md) and update the corresponding docs source for the NemoClaw remote deploy doc before running the skill generator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.agents/skills/nemoclaw-user-deploy-remote/SKILL.md:
- Around line 3-322: This file is a generated skill under
.agents/skills/nemoclaw-user-deploy-remote/SKILL.md and must not be edited
directly; move your content changes into the canonical docs source (the docs/
source that generates the nemoclaw-user-deploy-remote skill), update the source
there (not the generated .agents copy), then run the skill generation flow to
regenerate SKILL.md; reference the generated artifact by its path
(.agents/skills/nemoclaw-user-deploy-remote/SKILL.md) and update the
corresponding docs source for the NemoClaw remote deploy doc before running the
skill generator.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c199d88-1b23-4d10-b51d-b228f5f375f1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.agents/skills/nemoclaw-user-deploy-remote/SKILL.md.agents/skills/nemoclaw-user-reference/references/commands.mddocs/deployment/deploy-to-remote-gpu.mddocs/deployment/set-up-telegram-bridge.mddocs/index.mddocs/reference/commands.mdpackage.jsonscripts/start-services.shsrc/lib/services.test.tssrc/lib/services.tstest/cli.test.ts
✅ Files skipped from review due to trivial changes (8)
- docs/index.md
- docs/deployment/deploy-to-remote-gpu.md
- docs/deployment/set-up-telegram-bridge.md
- .agents/skills/nemoclaw-user-reference/references/commands.md
- package.json
- docs/reference/commands.md
- test/cli.test.ts
- src/lib/services.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/services.ts
- scripts/start-services.sh
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
The banner example showed a Telegram line that was removed when messaging was moved to native OpenClaw channels. Updated to show the current Messaging line. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add two tests that run start-services.sh with a shim cloudflared binary that records its argv in the log file. Verifies that CLOUDFLARE_TUNNEL_TOKEN set → `tunnel run --token TOKEN` and unset → `tunnel --url http://localhost:PORT`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
wscurran
added
NemoClaw CLI
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
4 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
Adds support for named Cloudflare tunnels as a stable alternative to the default quick tunnel.
When
CLOUDFLARE_TUNNEL_TOKENis set,nemoclaw startrunscloudflared tunnel run --tokeninstead of the ephemeral*.trycloudflare.comquick tunnel, and reads the tunnel hostname from cloudflared logs to display the public URL in the banner andnemoclaw status.Related Issue
Changes
src/lib/services.ts: canonicalgetTunnelUrl()implementation — parses named-tunnel hostname from cloudflared's JSON-escapedconfig=log line (handles multi-route tunnels and cloudflared's pretty-printed JSON with spaces after commas); falls back to*.trycloudflare.comregex for quick tunnels. Exported so it can be called from the bash shim and tested directly.scripts/start-services.sh:get_tunnel_url()now delegates todist/lib/services.jsvia a thin Node shim — no duplicate parsing logic. Falls back to a plain grep for quick tunnels if Node/dist is unavailable. AddsSCRIPT_DIRto locate the dist.scripts/start-services.sh: whenCLOUDFLARE_TUNNEL_TOKENis set, runcloudflared tunnel run --token(named tunnel) instead ofcloudflared tunnel --url(quick tunnel).docs/deployment/set-up-cloudflare-tunnel.md: new how-to page covering tunnel creation, hostname configuration, environment variables, and troubleshooting.docs/deployment/set-up-telegram-bridge.md,docs/deployment/deploy-to-remote-gpu.md,docs/reference/commands.md: cross-references andnemoclaw startenvironment variable table updated.docs/index.md: new page added to the deployment toctree.src/lib/services.test.ts: five direct unit tests forgetTunnelUrl, including named tunnel with/without spaces after commas, multi-route tunnel, quick tunnel, and missing log file.test/service-env.test.ts: added tests for real cloudflared log format (space after comma); updated multi-route test fixture to match real output.Type of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Doc Changes
After screenshots
Signed-off-by: Mauro Druwel mauro.druwel@gmail.com
Summary by CodeRabbit
New Features
Documentation
Tests