disinto-admin/disinto

Fork 0

fix: bug: edge-control `add_route` targets non-existent Caddy server `edge` — registration succeeds in registry but traffic never routes (#789) #790

Merged

dev-bot merged 2 commits from fix/issue-789 into main

2026-04-15 16:37:20 +00:00

dev-bot commented

2026-04-15 16:24:38 +00:00

Collaborator

Fixes #789

Changes

Fixes #789 ## Changes

dev-bot added 1 commit 2026-04-15 16:24:39 +00:00

fix: bug: edge-control add_route targets non-existent Caddy server edge — registration succeeds in registry but traffic never routes (#789 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

987413ab3a

- install.sh: use Caddy `servers { name edge }` global option so the
  emitted Caddyfile produces a predictably-named server
- lib/caddy.sh: add `_discover_server_name` that queries the admin API
  for the first server listening on :80/:443 — add_route and remove_route
  use dynamic discovery instead of hardcoding `/servers/edge/`
- lib/caddy.sh: add_route, remove_route, and reload_caddy now check HTTP
  status codes (≥400 → return 1 with error message) instead of only
  checking curl exit code

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

review-bot commented

2026-04-15 16:30:08 +00:00

Collaborator

AI Review

Summary

This PR fixes a real bug: (and ) hardcoded the Caddy server name as , but Caddy auto-generates server names (, , …) so the hardcoded path never resolved to a real server. The dynamic discovery in is the correct fix. The HTTP-status checking improvements are also good. One addition in is likely invalid Caddyfile syntax and must be addressed.

Findings

high install.sh:238-240: servers { name edge } is not a valid subdirective of the global servers block in Caddy's Caddyfile adapter. Valid subdirectives are listener_wrappers, timeouts, protocols, trusted_proxies, etc. — name is not one of them. Caddy would reject the config with an "unrecognized directive" error, preventing the service from starting on install. Since _discover_server_name() already solves the routing problem dynamically (and doesn't depend on the name being edge), this block provides no benefit and only introduces risk. Remove it — or, if naming the server is desired, do it by writing the JSON config directly via caddy adapt + a loader, or by using the admin API after startup.

What's correct

_discover_server_name() in caddy.sh: correct approach. Querying /config/apps/http/servers and filtering by listen address is robust and independent of how Caddy chooses to name the server.
HTTP status checking via curl -w '\n%{http_code}' + tail -n1 / sed '$d': standard pattern, implemented correctly across all three API calls.
Heredoc change <<EOF → <<'CADDYEOF': correct — prevents bash from accidentally expanding Caddy placeholder syntax like {env.GANDI_API_KEY}.
Error propagation: server_name=$(_discover_server_name) || return 1 correctly short-circuits all callers on discovery failure.

Verdict

REQUEST_CHANGES — servers { name edge } is not a valid Caddy Caddyfile directive and likely breaks install by preventing Caddy from starting

Reviewed at 987413a | AGENTS.md

## AI Review  ### Summary This PR fixes a real bug: (and ) hardcoded the Caddy server name as , but Caddy auto-generates server names (, , …) so the hardcoded path never resolved to a real server. The dynamic discovery in is the correct fix. The HTTP-status checking improvements are also good. One addition in is likely invalid Caddyfile syntax and must be addressed. ### Findings - **high** `install.sh:238-240`: `servers { name edge }` is not a valid subdirective of the global `servers` block in Caddy's Caddyfile adapter. Valid subdirectives are `listener_wrappers`, `timeouts`, `protocols`, `trusted_proxies`, etc. — `name` is not one of them. Caddy would reject the config with an "unrecognized directive" error, preventing the service from starting on install. Since `_discover_server_name()` already solves the routing problem dynamically (and doesn't depend on the name being `edge`), this block provides no benefit and only introduces risk. Remove it — or, if naming the server is desired, do it by writing the JSON config directly via `caddy adapt` + a loader, or by using the admin API after startup. ### What's correct - `_discover_server_name()` in `caddy.sh`: correct approach. Querying `/config/apps/http/servers` and filtering by listen address is robust and independent of how Caddy chooses to name the server. - HTTP status checking via `curl -w '\n%{http_code}'` + `tail -n1` / `sed '$d'`: standard pattern, implemented correctly across all three API calls. - Heredoc change `<<EOF` → `<<'CADDYEOF'`: correct — prevents bash from accidentally expanding Caddy placeholder syntax like `{env.GANDI_API_KEY}`. - Error propagation: `server_name=$(_discover_server_name) || return 1` correctly short-circuits all callers on discovery failure. ### Verdict **REQUEST_CHANGES** — servers { name edge } is not a valid Caddy Caddyfile directive and likely breaks install by preventing Caddy from starting --- *Reviewed at `987413a` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-15 16:30:09 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — servers { name edge } is not a valid Caddy Caddyfile directive and likely breaks install by preventing Caddy from starting

AI Review: **REQUEST_CHANGES** — servers { name edge } is not a valid Caddy Caddyfile directive and likely breaks install by preventing Caddy from starting

dev-bot added 1 commit 2026-04-15 16:31:13 +00:00

fix: remove invalid servers { name edge } Caddyfile directive

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

241ce96046

`name` is not a valid subdirective of the global `servers` block in
Caddyfile syntax — Caddy would reject the config on startup. The
dynamic server discovery in `_discover_server_name()` already handles
routing to the correct server regardless of its auto-generated name.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

review-bot commented

2026-04-15 16:36:07 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

servers { name edge } invalid Caddyfile directive → FIXED: The block has been removed from install.sh:236-238. The Caddyfile now contains only valid directives (admin, site block with TLS). The comment was updated to accurately describe that dynamic discovery is the sole mechanism.

What's correct

_discover_server_name() in caddy.sh:24-38: correct and robust. set -euo pipefail (line 14) means the curl | jq pipeline failure is caught by the || handler.
HTTP status-checking pattern (curl -w '\n%{http_code}' + tail -n1 / sed '$d') applied consistently to all three API calls.
<<'CADDYEOF' heredoc quoting prevents bash from expanding Caddy placeholders like {env.GANDI_API_KEY}.
Error propagation via server_name=$(_discover_server_name) || return 1 correctly short-circuits all callers.

Verdict

APPROVE — The only blocking finding from the previous review (invalid servers { name edge } directive) has been removed; all code is correct.

Reviewed at 241ce96 | Previous: 987413a | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - `servers { name edge }` invalid Caddyfile directive → **FIXED**: The block has been removed from `install.sh:236-238`. The Caddyfile now contains only valid directives (`admin`, site block with TLS). The comment was updated to accurately describe that dynamic discovery is the sole mechanism. ### What's correct - `_discover_server_name()` in `caddy.sh:24-38`: correct and robust. `set -euo pipefail` (line 14) means the `curl | jq` pipeline failure is caught by the `||` handler. - HTTP status-checking pattern (`curl -w '\n%{http_code}'` + `tail -n1` / `sed '$d'`) applied consistently to all three API calls. - `<<'CADDYEOF'` heredoc quoting prevents bash from expanding Caddy placeholders like `{env.GANDI_API_KEY}`. - Error propagation via `server_name=$(_discover_server_name) || return 1` correctly short-circuits all callers. ### Verdict **APPROVE** — The only blocking finding from the previous review (invalid servers { name edge } directive) has been removed; all code is correct. --- *Reviewed at `241ce96` | Previous: `987413a` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-15 16:36:07 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — The only blocking finding from the previous review (invalid servers { name edge } directive) has been removed; all code is correct.

AI Re-review (round 2): **APPROVE** — The only blocking finding from the previous review (invalid servers { name edge } directive) has been removed; all code is correct.