fix(acme): remove stray paren breaking ACME completion task (v1.8.5, Issue #35)

The background order-completion task (complete_pending_acme_orders, 60s
cycle) failed on EVERY cycle since v1.8.0 with:

    [ACME-COMPLETE] Error in completion task: syntax error at or near ")"

Root cause: the bounded DNS-01 retry OR-arm added to the atomic order-claim
query in v1.8.0 (13b65d9) carried one extra closing parenthesis, making the
whole SELECT invalid PostgreSQL. The claim is the task's first statement, so
the generic except swallowed it each minute and NO background ACME work ever
ran on v1.8.0-v1.8.4:

- orders were never claimed for finalize -> download -> save (http-01 too);
- advance_dns01_order never ran, so the DNS-01 TXT record was never
  published - DNS-01 with an automated provider (e.g. Cloudflare) could
  never validate (exactly the report in Issue #35);
- wizard-staged orders never left wizard_staged (same try block);
- retry_invalid_dns01 / reconcile_dns01_cleanup never executed;
- hourly-created renewal orders could never complete in the background.

Fix: drop the stray ')' (one line). Query semantics are unchanged.

Why the suite missed it: the unit tests mock asyncpg, so raw SQL never
reaches a real parser. Added a regression test that AST-scans the ACME
modules' SQL string literals (comments/quoted literals stripped) and fails
on unbalanced parentheses - it is red on the pre-fix tree and would have
caught the v1.8.0 regression at commit time. Scanned all six ACME modules:
this query was the only unbalanced SQL.

Verification: full backend suite in docker green (1062 passed, 151
skipped); the fixed query EXPLAINs cleanly on postgres:15; live localtest
run shows zero completion-task errors and a seeded pending order is claimed
("[ACME-COMPLETE] Claimed 1 order(s)"). Backend-only, no schema/API/agent
changes; fully backward compatible.

Reported-by: @tkkost (GitHub Issue #35)

README.md

@@ -2415,6 +2415,8 @@ Developed with ❤️ for the HAProxy community
 
 ## Release Notes
 
+- **v1.8.5** (2026-07-03) — **ACME completion-task SQL fix** (Issue #35 follow-up): the background order-completion task (`complete_pending_acme_orders`, runs every 60s) died on **every cycle** with `syntax error at or near ")"` — an extra closing parenthesis introduced in v1.8.0's bounded DNS-01 retry claim query. Because that query is the task's first database call, **no background ACME work ran at all from v1.8.0 through v1.8.4**: orders were never claimed for finalize/download, the DNS-01 TXT record was never published (so DNS-01 with an automated provider such as Cloudflare could never validate), Site Wizard staged orders never left `wizard_staged`, and DNS-01 retry/TXT-cleanup never executed. The stray parenthesis is removed and a regression test now scans all ACME modules' SQL for unbalanced parentheses (the unit suite mocks the database, which is why a raw-SQL syntax error could slip through). One-line backend query fix; no schema, API, or agent changes — fully backward compatible.
+- **v1.8.4** (2026-06-27) — **Agent installer self-kill fix** (Issue #31): the Linux/macOS agent installer could abort during "pre-installation cleanup" (terminal showed `Killing processes matching: haproxy-agent` then `Killed`) when the install script's own filename contained "haproxy-agent". The cleanup killed processes by matching the bare string "haproxy-agent" against full command lines, which also matched the running installer (and a `sudo`/PAM ancestor the self-exclusion did not cover), so the installer terminated itself. Cleanup now targets only the installed agent (the `$INSTALL_DIR/haproxy-agent` binary and the agent service), never the bare string, and the UI now names the downloaded scripts `install-agent-<platform>.sh` / `uninstall-agent-<platform>.sh`. Installer-only change; the running agent and its privilege model (it runs as root for HAProxy reload, config writes, keepalived, and self-upgrade) are unchanged.
 - **v1.8.3** (2026-06-25) — **Agent heartbeat JSON fix** (Issue #31): a self-hosted agent could fail every heartbeat with `HTTP 400 Invalid JSON: Expecting property name enclosed in double quotes` when the system-info block it collects came back empty on an unusual host, leaving a stray comma in the hand-built heartbeat JSON. The agent script now substitutes a valid placeholder when that block is empty so it can no longer emit a stray comma, and the backend heartbeat endpoint now parses valid payloads as-is and, only when a body fails to parse, tolerates that specific malformed pattern (a leading or doubled comma) so an already-deployed agent recovers on its next heartbeat after this build is deployed. Backend + agent-script only; healthy agents of every version are byte-for-byte unaffected.
 - **v1.8.2** (2026-06-25) — **ACME nonce fix** (Issue #35 follow-up): the ACME client now scopes the anti-replay nonce **per certificate authority** so a nonce issued by one CA is never sent to another. This fixes ZeroSSL/Google account registration failing with `malformed: The Replay Nonce could not be base64url-decoded` (the client previously shared one nonce across CAs and only auto-retried on `badNonce`). Account registration now always uses a fresh nonce from the target CA, and the retry covers this case too. Backend-only; HTTP-01 and Let's Encrypt are unaffected.
 - **v1.8.1** (2026-06-24) — **ACME DNS-01 fixes** (Issue #35 follow-up): Cloudflare API tokens are now sanitized so a pasted token with quotes/spaces no longer fails with "Invalid request headers"; ZeroSSL/Google **External Account Binding (EAB)** can be entered per-account in the register dialog and EAB-required failures show a clear message; and **Apply Management** now categorizes cluster ACME enable/disable changes under their own "ACME Challenge Routing" section and **Apply/Reject All** correctly process them (previously "Rejected 0 HA/VIP change(s)"), consistent with every other entity. Fully backward compatible.

backend/main.py

@@ -9,7 +9,7 @@ import asyncio
 from datetime import datetime, timedelta
 
 # Build/deploy marker for the v1.8.x (Issue #35, DNS-01) rollout — ensures the pipeline ships this commit's image.
-_version_info = {"version": "1.8.3", "releaseName": "Agent heartbeat JSON fix", "releaseDate": "2026-06-25"}
+_version_info = {"version": "1.8.4", "releaseName": "Agent installer self-kill fix", "releaseDate": "2026-06-27"}
 for _vpath in ["/app/version.json", os.path.join(os.path.dirname(__file__), "..", "version.json")]:
     try:
         with open(_vpath) as _vf:
@@ -318,7 +318,6 @@ async def complete_pending_acme_orders():
                                     OR dns01_last_attempt_at < NOW() - (
                                         (CASE COALESCE(dns01_attempts, 0) WHEN 0 THEN 15 WHEN 1 THEN 30 ELSE 60 END)
                                         || ' minutes')::INTERVAL
-                                    )
                                 )
                             )
                         )

backend/tests/test_agent_script_heartbeat_json.py

@@ -44,3 +44,18 @@ def test_b2_guard_precedes_every_fragment_system_info_use():
     for name, script in (("linux", LINUX), ("macos", MACOS)):
         assert script.count("    $system_info,") >= 1, f"{name}: fragment heartbeat form unexpectedly gone"
         assert script.count(_B2_GUARD) == 2, f"{name}: each fragment call site must carry the guard"
+
+
+def test_cleanup_does_not_self_kill_via_bare_haproxy_agent_pattern():
+    # Issue #31 (v1.8.4): the pre-installation cleanup kills processes by pgrep -f "$pattern". A bare
+    # "haproxy-agent" pattern also matches the installer's OWN path (install-haproxy-agent-*.sh) and a
+    # sudo/PAM ancestor, so the installer killed itself. The kill loop must target ONLY the installed
+    # agent (binary path + service/label), never the bare string.
+    for name, script in (("linux", LINUX), ("macos", MACOS)):
+        assert 'for pattern in "haproxy-agent"' not in script, (
+            f"{name}: pre-install cleanup uses the bare 'haproxy-agent' kill pattern -> self-kill (issue #31)"
+        )
+        # The narrowed, installer-safe pattern must be present (binary path via $INSTALL_DIR).
+        assert 'for pattern in "$INSTALL_DIR/haproxy-agent"' in script, (
+            f"{name}: cleanup must match the installed binary path, not a bare substring"
+        )

backend/tests/test_dns01.py

@@ -110,3 +110,111 @@ def test_nonce_scoped_per_directory():
     assert got == "NONCE_A"                                            # returns THIS CA's nonce
     assert svc._nonce_by_dir.get("https://a.example/dir") is None      # consumed (single-use)
     assert svc._nonce_by_dir.get("https://b.example/dir") == "NONCE_B" # the other CA is untouched
+
+
+def _sql_paren_depth(sql: str):
+    """Parenthesis depth of a SQL string, counting only OUTSIDE '...' literals (with ''
+    escapes), `--` line comments and /* */ block comments. Single-pass state machine so a
+    `--` inside a literal or a `'` inside a comment cannot corrupt the count. Dollar-quoted
+    strings are out of scope (not used in this codebase). Returns (final_depth, min_depth).
+    """
+    depth = 0
+    min_depth = 0
+    state = "normal"
+    i, n = 0, len(sql)
+    while i < n:
+        ch = sql[i]
+        nxt = sql[i + 1] if i + 1 < n else ""
+        if state == "normal":
+            if ch == "'":
+                state = "string"
+            elif ch == "-" and nxt == "-":
+                state = "line_comment"
+                i += 1
+            elif ch == "/" and nxt == "*":
+                state = "block_comment"
+                i += 1
+            elif ch == "(":
+                depth += 1
+            elif ch == ")":
+                depth -= 1
+                min_depth = min(min_depth, depth)
+        elif state == "string":
+            if ch == "'":
+                if nxt == "'":
+                    i += 1  # escaped '' stays inside the literal
+                else:
+                    state = "normal"
+        elif state == "line_comment":
+            if ch == "\n":
+                state = "normal"
+        else:  # block_comment
+            if ch == "*" and nxt == "/":
+                state = "normal"
+                i += 1
+        i += 1
+    return depth, min_depth
+
+
+def test_acme_sql_parentheses_balanced():
+    """Issue #35 v1.8.5: the completion task's order-claim query shipped (v1.8.0-v1.8.4) with an
+    extra closing parenthesis, so EVERY 60s cycle died with `syntax error at or near ")"` and no
+    background ACME work (claim/finalize/download, DNS-01 publish, wizard-staged promotion,
+    retry, TXT cleanup) ever ran. The suite never caught it because the DB layer is mocked and
+    raw SQL never reaches a real parser. This guard scans the ACME modules' SQL string literals
+    for unbalanced parentheses.
+
+    Guard scope is deliberately conservative to avoid false positives on production changes:
+    keyword matching is case-sensitive (SQL is uppercase in this codebase; prose in docstrings
+    is not) and f-string fragments are excluded (they split at `{`, so a fragment may be
+    legitimately unbalanced).
+    """
+    import ast
+    import re
+
+    backend_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+    modules = [
+        "main.py",
+        os.path.join("services", "dns01_orchestrator.py"),
+        os.path.join("services", "acme_service.py"),
+        os.path.join("services", "letsencrypt_service.py"),
+        os.path.join("routers", "letsencrypt.py"),
+        os.path.join("routers", "acme_diagnostics.py"),
+    ]
+    problems = []
+    for rel in modules:
+        with open(os.path.join(backend_dir, rel), encoding="utf-8") as fh:
+            tree = ast.parse(fh.read())
+        fstring_parts = {
+            id(const)
+            for joined in ast.walk(tree) if isinstance(joined, ast.JoinedStr)
+            for const in ast.walk(joined) if isinstance(const, ast.Constant)
+        }
+        for node in ast.walk(tree):
+            if not (isinstance(node, ast.Constant) and isinstance(node.value, str)):
+                continue
+            if id(node) in fstring_parts:
+                continue
+            sql = node.value
+            if not re.search(r"\b(SELECT|INSERT|UPDATE|DELETE)\b", sql):
+                continue
+            if not re.search(r"\b(FROM|INTO|SET|WHERE)\b", sql):
+                continue
+            depth, min_depth = _sql_paren_depth(sql)
+            if depth != 0 or min_depth < 0:
+                problems.append(f"{rel}:{node.lineno} (paren depth {depth:+d}, min {min_depth})")
+    assert not problems, f"Unbalanced parentheses in SQL literal(s): {problems}"
+
+
+def test_sql_paren_depth_scanner():
+    # The guard's scanner itself: parens in literals/comments must not count; '' escapes and
+    # block comments handled; an extra ')' is reported via min_depth even if a later '(' would
+    # re-balance the total.
+    assert _sql_paren_depth("SELECT (1)") == (0, 0)
+    assert _sql_paren_depth("SELECT (1))") == (-1, -1)                       # the v1.8.0 bug shape
+    assert _sql_paren_depth("SELECT ')' , '((' FROM t") == (0, 0)            # literals ignored
+    assert _sql_paren_depth("SELECT 'it''s ))' FROM t") == (0, 0)            # '' escape stays inside
+    assert _sql_paren_depth("SELECT 1 -- comment ) (\nFROM t") == (0, 0)     # line comment ignored
+    assert _sql_paren_depth("SELECT 1 /* ) */ FROM t") == (0, 0)             # block comment ignored
+    assert _sql_paren_depth("SELECT 'a--b' AND (x=1\n)") == (0, 0)           # -- inside literal is data
+    assert _sql_paren_depth("WHERE x) AND (y") == (0, -1)                    # net 0 but went negative

backend/utils/agent_scripts/linux_install.sh

@@ -470,7 +470,12 @@ safe_remove() {
 [[ "$QUIET_MODE" != "true" ]] && echo "Terminating existing HAProxy Agent processes..."
 KILLED_COUNT=0
 INSTALLER_PID=$$
-for pattern in "haproxy-agent" "/usr/local/bin/haproxy-agent" "haproxy-agent.service"; do
+# issue #31: match ONLY the installed agent (binary path + service), never the bare string
+# "haproxy-agent". With pgrep -f, that bare string can also match the installer's OWN command line
+# or a sudo/PAM ancestor (which the $$/$PPID guard does not fully cover), making the cleanup kill
+# the installer itself ("Killed", install aborts). The systemd service is also stopped below; the
+# "$INSTALL_DIR/haproxy-agent" path still catches any running daemon.
+for pattern in "$INSTALL_DIR/haproxy-agent" "haproxy-agent.service"; do
     PIDS=$(pgrep -f "$pattern" 2>/dev/null || true)
     if [[ -n "$PIDS" ]]; then
         FILTERED=""

backend/utils/agent_scripts/macos_install.sh

@@ -318,7 +318,11 @@ safe_remove() {
 [[ "$QUIET_MODE" != "true" ]] && echo "Terminating existing HAProxy Agent processes..."
 KILLED_COUNT=0
 INSTALLER_PID=$$
-for pattern in "haproxy-agent" "/usr/local/bin/haproxy-agent" "com.haproxy.agent"; do
+# issue #31: match ONLY the installed agent (binary path + LaunchDaemon label), never the bare
+# string "haproxy-agent". With pgrep -f, that bare string can also match the installer's OWN command
+# line or a sudo ancestor (which the $$/$PPID guard does not fully cover), making the cleanup kill
+# the installer itself. The "$INSTALL_DIR/haproxy-agent" path still catches any running daemon.
+for pattern in "$INSTALL_DIR/haproxy-agent" "com.haproxy.agent"; do
     PIDS=$(pgrep -f "$pattern" 2>/dev/null || true)
     if [[ -n "$PIDS" ]]; then
         FILTERED=""

frontend/package.json

@@ -1,6 +1,6 @@
 {
   "name": "haproxy-openmanager-frontend",
-  "version": "1.8.3",
+  "version": "1.8.5",
   "description": "HAProxy Load Balancer Management UI",
   "license": "AGPL-3.0-or-later",
   "dependencies": {

frontend/src/components/AgentManagement.js

@@ -2378,7 +2378,7 @@ const AgentManagement = () => {
                         const element = document.createElement('a');
                         const file = new Blob([installScript], { type: 'text/plain' });
                         element.href = URL.createObjectURL(file);
-                        element.download = `install-haproxy-agent-${selectedPlatform}.sh`;
+                        element.download = `install-agent-${selectedPlatform}.sh`;
                         document.body.appendChild(element);
                         element.click();
                         document.body.removeChild(element);
@@ -2516,7 +2516,7 @@ const AgentManagement = () => {
                                     const element = document.createElement('a');
                                     const file = new Blob([uninstallScript], { type: 'text/plain' });
                                     element.href = URL.createObjectURL(file);
-                                    element.download = `uninstall-haproxy-agent-${selectedPlatform}.sh`;
+                                    element.download = `uninstall-agent-${selectedPlatform}.sh`;
                                     document.body.appendChild(element);
                                     element.click();
                                     document.body.removeChild(element);
@@ -3147,7 +3147,7 @@ const AgentManagement = () => {
                           const element = document.createElement('a');
                           const file = new Blob([deleteUninstallScript], { type: 'text/plain' });
                           element.href = URL.createObjectURL(file);
-                          element.download = `uninstall-haproxy-agent-${agentToDelete.platform || 'linux'}.sh`;
+                          element.download = `uninstall-agent-${agentToDelete.platform || 'linux'}.sh`;
                           document.body.appendChild(element);
                           element.click();
                           document.body.removeChild(element);

version.json

@@ -1,5 +1,5 @@
 {
-  "version": "1.8.3",
-  "releaseName": "Agent heartbeat JSON fix",
-  "releaseDate": "2026-06-25"
+  "version": "1.8.5",
+  "releaseName": "ACME completion-task SQL fix",
+  "releaseDate": "2026-07-03"
 }