diff --git a/internal/httpapi/phase3.go b/internal/httpapi/phase3.go index 7bb483d..fe1e538 100644 --- a/internal/httpapi/phase3.go +++ b/internal/httpapi/phase3.go @@ -69,6 +69,14 @@ func initSSH() { } } +// sshExecTimeout bounds how long a single remote command may run. Without +// this, a hung remote command (e.g. a piped install script stuck retrying +// DNS against a misconfigured gateway) blocks the executing goroutine +// forever: the execution never leaves 'approved'/'running', the operator +// sees an unkillable spinner, and get_execution_status has nothing new to +// report. Generous enough for a real apt/docker install; not infinite. +const sshExecTimeout = 10 * time.Minute + func sshExec(ctx context.Context, host, user, command string) (string, error) { initSSH() if len(_sshKey) == 0 { @@ -103,19 +111,44 @@ func sshExec(ctx context.Context, host, user, command string) (string, error) { } defer session.Close() - out, err := session.CombinedOutput(command) - text := strings.TrimSpace(string(out)) - // A non-zero exit MUST surface as an error. The previous guard only - // errored when there was no output, so a `pct create` that printed - // "CT 132 already exists" and exited non-zero was reported as success — - // the execution was marked completed though nothing was provisioned. - if err != nil { - if text != "" { - return text, fmt.Errorf("%w: %s", err, text) - } - return text, fmt.Errorf("exec: %w", err) + type result struct { + out []byte + err error + } + done := make(chan result, 1) + go func() { + out, err := session.CombinedOutput(command) + done <- result{out, err} + }() + + select { + case r := <-done: + text := strings.TrimSpace(string(r.out)) + // A non-zero exit MUST surface as an error. The previous guard only + // errored when there was no output, so a `pct create` that printed + // "CT 132 already exists" and exited non-zero was reported as + // success — the execution was marked completed though nothing was + // provisioned. + if r.err != nil { + if text != "" { + return text, fmt.Errorf("%w: %s", r.err, text) + } + return text, fmt.Errorf("exec: %w", r.err) + } + return text, nil + case <-time.After(sshExecTimeout): + // Close the session/client to hang up the remote side; the + // goroutine above will eventually exit once that unblocks + // CombinedOutput, but we don't wait for it — the caller needs an + // answer now, not an indefinite hang. + session.Close() + client.Close() + return "", fmt.Errorf("timed out after %s waiting for command to finish on %s", sshExecTimeout, host) + case <-ctx.Done(): + session.Close() + client.Close() + return "", ctx.Err() } - return text, nil } func resolveHostSSH(ctx context.Context, pool *db.Pool, entitySlug string) (string, string, error) { @@ -526,11 +559,19 @@ func provisionScript(pkgs []string, postInstall string) string { b.WriteString("export LANG=C.UTF-8 LC_ALL=C.UTF-8 DEBIAN_FRONTEND=noninteractive\n") b.WriteString("probe=deb.debian.org\n") b.WriteString("ok=0\n") - b.WriteString("for i in $(seq 1 30); do if getent hosts \"$probe\" >/dev/null 2>&1; then ok=1; break; fi; sleep 2; done\n") + // `timeout 3` on every getent call is load-bearing, not cosmetic: when + // the network is truly unreachable (e.g. a wrong gateway), a plain + // `getent hosts` doesn't fail fast — it can hang far longer than the + // resolver's nominal timeout because packets are just dropped, not + // rejected. Without a hard per-attempt cap, this loop's "~90s" budget + // was fiction — one run hung 17+ minutes on a bad gateway before the Go + // side finally got a hard sshExec timeout to fall back on. Capping each + // attempt makes the wall-clock budget real. + b.WriteString("for i in $(seq 1 30); do if timeout 3 getent hosts \"$probe\" >/dev/null 2>&1; then ok=1; break; fi; sleep 2; done\n") // Self-heal: if the assigned resolver can't resolve, fall back to public DNS. b.WriteString("if [ \"$ok\" != 1 ]; then printf 'nameserver 1.1.1.1\\nnameserver 8.8.8.8\\n' > /etc/resolv.conf; ") - b.WriteString("for i in $(seq 1 15); do if getent hosts \"$probe\" >/dev/null 2>&1; then ok=1; break; fi; sleep 2; done; fi\n") - b.WriteString("if [ \"$ok\" != 1 ]; then echo 'ERROR: container has no DNS/connectivity after ~90s'; exit 1; fi\n") + b.WriteString("for i in $(seq 1 15); do if timeout 3 getent hosts \"$probe\" >/dev/null 2>&1; then ok=1; break; fi; sleep 2; done; fi\n") + b.WriteString("if [ \"$ok\" != 1 ]; then echo 'ERROR: container has no DNS/connectivity after ~2min — check the LXC net0 gateway/IP are correct for this subnet'; exit 1; fi\n") b.WriteString("set -e\n") if len(pkgs) > 0 { b.WriteString("export DEBIAN_FRONTEND=noninteractive\n") diff --git a/internal/mcp/server.go b/internal/mcp/server.go index c4fe008..6d6a25a 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -1048,6 +1048,12 @@ func initSSH() { } } +// sshExecTimeout bounds how long a single remote command may run — see the +// matching constant/comment in httpapi/phase3.go. Without it, a hung remote +// command (piped install script stuck retrying DNS, etc.) blocks this +// goroutine forever with no way for the caller to ever get an answer. +const sshExecTimeout = 10 * time.Minute + func sshExec(ctx context.Context, host, user, command string) (string, error) { initSSH() if len(sshKey) == 0 { @@ -1082,11 +1088,40 @@ func sshExec(ctx context.Context, host, user, command string) (string, error) { } defer session.Close() - out, err := session.CombinedOutput(command) - if err != nil && out == nil { - return "", fmt.Errorf("exec: %w", err) + type result struct { + out []byte + err error + } + done := make(chan result, 1) + go func() { + out, err := session.CombinedOutput(command) + done <- result{out, err} + }() + + select { + case r := <-done: + text := strings.TrimSpace(string(r.out)) + // A non-zero exit MUST surface as an error — matching the fix + // applied to httpapi's sshExec (this copy still had the original + // bug: only erroring when there was no output at all, so a command + // that failed but printed something was silently reported as + // success). + if r.err != nil { + if text != "" { + return text, fmt.Errorf("%w: %s", r.err, text) + } + return text, fmt.Errorf("exec: %w", r.err) + } + return text, nil + case <-time.After(sshExecTimeout): + session.Close() + client.Close() + return "", fmt.Errorf("timed out after %s waiting for command to finish on %s", sshExecTimeout, host) + case <-ctx.Done(): + session.Close() + client.Close() + return "", ctx.Err() } - return strings.TrimSpace(string(out)), nil } func resolveHost(ctx context.Context, pool *db.Pool, entitySlug string) (hostIP string, sshUser string, err error) { diff --git a/web/src/lib/components/InlineApproval.svelte b/web/src/lib/components/InlineApproval.svelte index 01c1c86..0052abf 100644 --- a/web/src/lib/components/InlineApproval.svelte +++ b/web/src/lib/components/InlineApproval.svelte @@ -13,7 +13,7 @@ // Per-execution UI phase, keyed by executionId. A resolved phase hides the // action buttons permanently so the banner clears after a click and can // never re-POST /decision. - type Phase = 'deciding' | 'running' | 'completed' | 'failed' | 'denied' + type Phase = 'deciding' | 'running' | 'completed' | 'failed' | 'denied' | 'stalled' const phase = new SvelteMap() // Latest execution row (for status/result display), keyed by executionId. const exec = new SvelteMap() @@ -32,10 +32,40 @@ return typeof v === 'string' ? v.trim() : '' } + function elapsedSeconds(e: Execution | undefined): number | null { + if (!e?.created_at) return null + return Math.max(0, Math.round((now - new Date(e.created_at).getTime()) / 1000)) + } + + function fmtDuration(s: number): string { + if (s < 60) return `${s}s` + const m = Math.floor(s / 60) + return `${m}m ${s % 60}s` + } + + // Live clock for the elapsed-time display on running cards. Tied to + // component lifecycle via $effect so the interval is guaranteed cleared on + // unmount — a bare setInterval field here would leak a 1Hz timer for the + // lifetime of the page every time this component was mounted. + let now = $state(Date.now()) + $effect(() => { + const t = setInterval(() => { now = Date.now() }, 1000) + return () => clearInterval(t) + }) + + // Backend commands are hard-capped at 10 minutes (internal sshExec + // timeout) before the execution is force-finalized as failed — so polling + // must outlast that with margin, or the UI gives up and goes stale before + // the backend ever resolves. Poll for 14 minutes; anything still running + // past that is a genuine anomaly worth surfacing distinctly rather than + // silently going quiet. + const POLL_CEILING_MS = 14 * 60 * 1000 + // Poll the execution until it reaches a terminal state, so the operator sees // provisioning progress and the final outcome without leaving the chat. async function track(id: string) { - for (let i = 0; i < 150; i++) { // ~6min ceiling at 2.5s + const deadline = Date.now() + POLL_CEILING_MS + while (Date.now() < deadline) { const e = await getExecution(id) if (e) { exec.set(id, e) @@ -45,8 +75,10 @@ } await new Promise((r) => setTimeout(r, 2500)) } - // Timed out waiting — leave whatever we last saw, mark running-stalled. - if (!TERMINAL.has(exec.get(id)?.status ?? '')) phase.set(id, 'running') + // Genuinely outlasted the backend's own hard timeout — this means + // something is wrong beyond a slow command (e.g. the API is down). + // Say so explicitly instead of freezing on "running" with no signal. + if (!TERMINAL.has(exec.get(id)?.status ?? '')) phase.set(id, 'stalled') } async function decide(approval: PendingApproval, decision: 'approve' | 'deny') { @@ -125,23 +157,60 @@ Denied. {:else if p === 'running' || p === 'deciding'} -
- - {p === 'deciding' ? 'Submitting approval…' : `Running on ${approval.target}… (this can take a minute)`} + {@const secs = elapsedSeconds(e)} +
+
+ + + {#if p === 'deciding'} + Submitting approval… + {:else} + Running on {approval.target}{secs !== null ? ` — ${fmtDuration(secs)} elapsed` : '…'} + {/if} + +
+ {#if p === 'running' && approval.command} + {approval.command} + {/if} + {#if p === 'running'} + + Execution {approval.executionId.slice(0, 8)} — long installs can take several minutes; this + will resolve on its own (capped at 10 min) or you can check the Operations page for live output. + + {/if} +
+ {:else if p === 'stalled'} +
+
+ + No update from the server in over 14 minutes. + +
+ + The command itself is capped at 10 minutes server-side, so this is unusual — the API may be unreachable. + Execution {approval.executionId}. Check the Operations page directly. +
{:else if approval.destructive} -
- - - DESTRUCTIVE — {approval.action} on {approval.target}. Type - "I confirm" in chat, or use the button. - - - +
+
+ + + DESTRUCTIVE — {approval.action} on {approval.target}. Type + "I confirm" in chat, or use the button. + + + +
+ {#if approval.command} + {approval.command} + {/if}
{:else}
diff --git a/web/src/lib/stores/chat.ts b/web/src/lib/stores/chat.ts index 1dae6de..3320dce 100644 --- a/web/src/lib/stores/chat.ts +++ b/web/src/lib/stores/chat.ts @@ -7,6 +7,8 @@ export interface PendingApproval { action: string target: string destructive: boolean + command?: string + purpose?: string } export interface ChatMessage { @@ -40,7 +42,9 @@ function extractApprovals(tools: ToolCallResult[]): PendingApproval[] { executionId: m[1], action: t.args?.action ?? t.args?.purpose ?? t.name ?? 'unknown', target: t.args?.target ?? 'unknown', - destructive: /\bDESTRUCTIVE\b/.test(text) + destructive: /\bDESTRUCTIVE\b/.test(text), + command: t.args?.command, + purpose: t.args?.purpose }) } }