From 8d5b21fa6f7f067c8dc6428eb57271ec91da5937 Mon Sep 17 00:00:00 2001 From: david-hummingbot Date: Mon, 7 Sep 2026 22:26:46 +0800 Subject: [PATCH 1/3] Find the API node when TAILSCALE_HOSTNAME names the desk, not a number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tailnet_api_peers` matched `hummingbot-api` plus a numeric suffix, because that is the only suffix Tailscale itself appends when a name is taken. But TAILSCALE_HOSTNAME is a setting, and naming the API per desk -- `hummingbot-api-cornell`, `hummingbot-api-eu` -- is the ordinary way to run more than one of these. For those, the search found nothing. "Nothing" is the branch that warns ! No hummingbot-api node is visible on this tailnet yet. → Deploy it on that machine first (its setup joins the tailnet), or → enter the name/address it is reachable at. about a node sitting in plain sight in `tailscale status`, and then asks the operator to type the name it had just declined to recognise. Driven through the wizard against a tailnet whose only API node was `hummingbot-api-cornell`, the same run now reports ✓ Found it on your tailnet: hummingbot-api-cornell and asks nothing. Widening the suffix cannot make the earlier silent-wrong- node failure worse: more matches means the operator is shown the list and picks, which is the one outcome that is never wrong. --- setup-environment.sh | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/setup-environment.sh b/setup-environment.sh index c932003bd..cdb4ffeb8 100755 --- a/setup-environment.sh +++ b/setup-environment.sh @@ -421,13 +421,21 @@ tailnet_node_name() { } # Peers on this tailnet whose name looks like a hummingbot-api node, newline -# separated. Matches the requested name and the -1/-2 suffixes Tailscale adds, -# so the machine we are looking for is found under whichever one it got. +# separated. +# +# The suffix is deliberately `-.+` and not `-[0-9]+`. Tailscale's own +# de-duplication only ever appends numbers, but TAILSCALE_HOSTNAME is a +# setting, and a deployment that names its API per desk -- hummingbot-api-cornell, +# hummingbot-api-eu -- is the normal way to run more than one. Matching only +# numbers found nothing for those, and "nothing" is the branch that warns +# "deploy it on that machine first" about a node sitting right there in +# `tailscale status`, then asks the operator to type the name it just declined +# to recognise. tailnet_api_peers() { local want="${1:-hummingbot-api}" tailscale status --peers 2>/dev/null \ | awk '{print $2}' \ - | grep -E "^${want}(-[0-9]+)?$" || true + | grep -E "^${want}(-.+)?$" || true } # Make sure a tailscaled is running, then `tailscale up`. From 0ed0ab8d524ff1a125900191e43306c7fe694671 Mon Sep 17 00:00:00 2001 From: david-hummingbot Date: Tue, 29 Sep 2026 12:06:29 +0700 Subject: [PATCH 2/3] Confirm a suffixed lone match, and stop calling deliberate names collisions Two things the widened suffix in this branch exposed. Greptile's point first: widening `-[0-9]+` to `-.+` widens what can reach the count==1 branch, and that branch selects silently. A lone `hummingbot-api-staging`, a node someone rebuilt, another desk's API -- each is alone on a tailnet where the intended one has not joined yet, and each is now accepted and written into config.yml with only a "Found it" to show for it. The wrong node is very likely running the same software, so it answers, and the mistake arrives as "401 Incorrect username or password" against a password that was never wrong -- the exact failure #231 set out to stop. Being the only match is not evidence of being the right match. So count==1 splits. The unsuffixed `hummingbot-api` is still taken silently: it is the name hummingbot-api's own setup asks for, and there is nothing to choose between. A suffixed lone match is shown and confirmed, pre-filled, so Enter accepts it when it is right -- which is the common case and the whole point of the search. It costs a keystroke and it means no name the operator never chose lands in config.yml unseen. Second, the pick-one message asserted "Tailscale adds -1, -2 ... when a name is taken, so these are different machines. Pick the one you just deployed." Running several APIs on purpose -- hummingbot-api-1, hummingbot-api-2 alongside the default -- is an ordinary deployment, and there those suffixes are chosen names, not collision artifacts. The old wording told that operator their own naming was an accident, and "the one you just deployed" is the wrong instruction anyway when Condor is being pointed at an instance that has been up for weeks. It now covers both origins and asks which node this Condor should talk to. Plus the assertions over the helper this branch described but never committed: 16 of them, sourcing the real function out of the script with a stubbed `tailscale` rather than restating the regex in Python (a copy of the regex asserts only that the copy matches itself). Four fail against the old `-[0-9]+`. Full suite 5109 passed, 23 skipped. Co-Authored-By: Claude Opus 5 --- setup-environment.sh | 32 ++++++- tests/test_setup_tailnet_peers.py | 142 ++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 tests/test_setup_tailnet_peers.py diff --git a/setup-environment.sh b/setup-environment.sh index cdb4ffeb8..bd62017b9 100755 --- a/setup-environment.sh +++ b/setup-environment.sh @@ -1172,16 +1172,42 @@ if [ -z "${DEPLOY_HUMMINGBOT_API:-}" ] || [ "$finish_remote_api" = true ]; then ts_hostname="" _ts_candidates="$(tailnet_api_peers hummingbot-api)" _ts_count="$(printf '%s' "$_ts_candidates" | grep -c . || true)" - if [ "${_ts_count:-0}" -eq 1 ]; then + if [ "${_ts_count:-0}" -eq 1 ] && [ "$_ts_candidates" = "hummingbot-api" ]; then + # The unsuffixed name, and the only match. Nothing to choose + # between, and it is the name hummingbot-api's own setup asks + # for, so this is the node in every ordinary install. ts_hostname="$_ts_candidates" msg_ok "Found it on your tailnet: $ts_hostname" + elif [ "${_ts_count:-0}" -eq 1 ]; then + # One match, but under a suffix -- hummingbot-api-1, or a name + # somebody chose. Being alone is not evidence it is the right + # one: a stale node, a colleague's staging box, or another + # desk's API is just as alone, and accepting it silently writes + # it into config.yml where the next sign of trouble is a 401 + # against a machine the operator never meant to reach. Shown + # and confirmed instead, with Enter as the answer when it is + # right, which is the common case. + msg_ok "Found one hummingbot-api node on your tailnet: $_ts_candidates" + msg_info "That is not the default name, so it is worth a look before it" + msg_info "goes in config.yml. Press Enter to use it, or type another." + prompt_visible "hummingbot-api host" "$_ts_candidates" "ts_hostname" + ts_hostname="${ts_hostname:-$_ts_candidates}" elif [ "${_ts_count:-0}" -gt 1 ]; then msg_warn "More than one hummingbot-api node on this tailnet:" printf '%s\n' "$_ts_candidates" | while IFS= read -r _c; do [ -n "$_c" ] && echo " • $_c" done - msg_info "Tailscale adds -1, -2 ... when a name is taken, so these are" - msg_info "different machines. Pick the one you just deployed." + # Deliberately does NOT say "Tailscale adds -1, -2 when a name + # is taken, so pick the one you just deployed". Running several + # APIs on purpose -- hummingbot-api-1, hummingbot-api-2 -- is a + # normal deployment, and there those suffixes are chosen names, + # not collision artifacts. The old wording told such an operator + # their own naming was an accident, and "the one you just + # deployed" is the wrong instruction anyway when Condor is being + # pointed at an instance that has been up for weeks. + msg_info "These are separate machines -- deployments named on purpose, or" + msg_info "names Tailscale suffixed because one was already taken." + msg_info "Pick the one this Condor should talk to." prompt_required_visible "Which node is your hummingbot-api?" "ts_hostname" "Name cannot be empty" else msg_warn "No hummingbot-api node is visible on this tailnet yet." diff --git a/tests/test_setup_tailnet_peers.py b/tests/test_setup_tailnet_peers.py new file mode 100644 index 000000000..45791244a --- /dev/null +++ b/tests/test_setup_tailnet_peers.py @@ -0,0 +1,142 @@ +"""`tailnet_api_peers` in setup-environment.sh, exercised as shell. + +The helper decides which tailnet node the wizard writes into config.yml as +the hummingbot-api host, and getting it wrong is not a visible failure: the +wrong node is very likely running the same software, so it answers, and the +mistake surfaces as "401 Incorrect username or password" against a password +that was never wrong. + +There is no bats in this repo and no reason to add one for a single awk/grep +pipeline, so the function is sourced out of the script with a `tailscale` +stub ahead of it on PATH. That runs the real line, not a Python +reimplementation of it -- a copy of the regex in a test asserts only that the +copy matches itself. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +SETUP = Path(__file__).resolve().parent.parent / "setup-environment.sh" + + +def run_peers(tmp_path: Path, status_output: str, want: str = "hummingbot-api") -> list[str]: + """Return `tailnet_api_peers ` over a stubbed `tailscale status`.""" + stub_dir = tmp_path / "bin" + stub_dir.mkdir(exist_ok=True) + stub = stub_dir / "tailscale" + peers = tmp_path / "peers.txt" + peers.write_text(status_output) + stub.write_text(f'#!/bin/sh\ncat "{peers}"\n') + stub.chmod(0o755) + + # Pull just the function out: sourcing the whole script would run an + # installer. Everything from its definition to the closing brace. + src = SETUP.read_text() + start = src.index("tailnet_api_peers() {") + end = src.index("\n}\n", start) + len("\n}\n") + func = src[start:end] + + proc = subprocess.run( + ["bash", "-c", f'PATH="{stub_dir}:$PATH"\n{func}\ntailnet_api_peers "{want}"'], + capture_output=True, + text=True, + timeout=30, + ) + assert proc.returncode == 0, proc.stderr + return [ln for ln in proc.stdout.splitlines() if ln.strip()] + + +def status(*names: str) -> str: + return "".join( + f"100.64.0.{i + 1}\t{n}\tdavid@\tlinux\t-\n" for i, n in enumerate(names) + ) + + +# ── the suffixes Tailscale itself assigns ────────────────────────────── + + +def test_finds_the_unsuffixed_name(tmp_path): + assert run_peers(tmp_path, status("hummingbot-api")) == ["hummingbot-api"] + + +def test_finds_numeric_collision_suffixes(tmp_path): + out = run_peers(tmp_path, status("hummingbot-api-1", "hummingbot-api-2")) + assert out == ["hummingbot-api-1", "hummingbot-api-2"] + + +# ── the suffixes an operator assigns, which is what this PR is about ─── + + +@pytest.mark.parametrize("name", ["hummingbot-api-cornell", "hummingbot-api-eu"]) +def test_finds_desk_named_nodes(tmp_path, name): + """The regression: TAILSCALE_HOSTNAME is a setting, not only a collision.""" + assert run_peers(tmp_path, status(name)) == [name] + + +def test_deliberate_numeric_fleet_all_found(tmp_path): + """A planned multi-instance deployment, the case that motivated the reword.""" + out = run_peers( + tmp_path, status("hummingbot-api", "hummingbot-api-1", "hummingbot-api-2") + ) + assert out == ["hummingbot-api", "hummingbot-api-1", "hummingbot-api-2"] + assert len(out) > 1, "must reach the pick-one branch, never auto-select" + + +# ── names that must NOT be claimed ───────────────────────────────────── + + +@pytest.mark.parametrize( + "name", + [ + "condor", + "condor-hackathon", + "condor-hackathon-server", + "condor1", + "hbapi", + "my-hummingbot-api", # suffix match, not prefix -- anchored ^ + ], +) +def test_ignores_unrelated_nodes(tmp_path, name): + assert run_peers(tmp_path, status(name)) == [] + + +def test_ignores_the_prefix_without_a_separator(tmp_path): + """`hummingbot-apis` is a different word, not a suffixed hummingbot-api.""" + assert run_peers(tmp_path, status("hummingbot-apis")) == [] + + +def test_picks_only_the_api_out_of_a_mixed_tailnet(tmp_path): + out = run_peers( + tmp_path, + status( + "condor-hackathon", + "hummingbot-api-cornell", + "condor1", + "someones-laptop", + ), + ) + assert out == ["hummingbot-api-cornell"] + + +# ── shape of the output ──────────────────────────────────────────────── + + +def test_empty_tailnet_is_empty_not_an_error(tmp_path): + """The no-match branch must be reachable: `|| true` keeps grep's 1 quiet.""" + assert run_peers(tmp_path, "") == [] + + +def test_reads_the_name_column_not_the_address(tmp_path): + out = run_peers(tmp_path, status("hummingbot-api")) + assert out == ["hummingbot-api"] + assert not any(c.startswith("100.") for c in out) + + +def test_custom_want_is_honoured(tmp_path): + """The parameter is real, even though today's only caller hardcodes it.""" + out = run_peers(tmp_path, status("condor", "condor-hackathon"), want="condor") + assert out == ["condor", "condor-hackathon"] From 9b0d1c7f86942e2e951a942462cc1f5e41670326 Mon Sep 17 00:00:00 2001 From: david-hummingbot Date: Tue, 29 Sep 2026 12:31:26 +0700 Subject: [PATCH 3/3] Run black over the new test file `black --check .` is a CI gate and the file I added in the previous commit had not been through it. Formatting only; the 16 assertions are unchanged and still pass. Co-Authored-By: Claude Opus 5 --- tests/test_setup_tailnet_peers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_setup_tailnet_peers.py b/tests/test_setup_tailnet_peers.py index 45791244a..d399369ed 100644 --- a/tests/test_setup_tailnet_peers.py +++ b/tests/test_setup_tailnet_peers.py @@ -23,7 +23,9 @@ SETUP = Path(__file__).resolve().parent.parent / "setup-environment.sh" -def run_peers(tmp_path: Path, status_output: str, want: str = "hummingbot-api") -> list[str]: +def run_peers( + tmp_path: Path, status_output: str, want: str = "hummingbot-api" +) -> list[str]: """Return `tailnet_api_peers ` over a stubbed `tailscale status`.""" stub_dir = tmp_path / "bin" stub_dir.mkdir(exist_ok=True)