Fix errexit start lie, scan HTTPS/IPv6 misses, failed-state visibility, and clone hangs

Author: djadmin

bin/devkit

@@ -34,6 +34,10 @@ CADDYFILE="$DEVKIT_HOME/Caddyfile"
 DASHBOARD="$DEVKIT_HOME/dashboard.html"
 PIDS_DIR="$DEVKIT_HOME/pids"
 LOGS_DIR="$DEVKIT_HOME/logs"
+# Per-app last-start outcome markers. A start that fails drops a marker here so `list`
+# (and the app/MCP that read it) can show a distinct "failed" state with a pointer to the
+# logs, instead of a plain "stopped" that's indistinguishable from never-started.
+STATUS_DIR="$DEVKIT_HOME/state"
 LOCK_DIR="$DEVKIT_HOME/.registry.lock"
 # Stamp recording the registry fingerprint the generated config was last built from.
 # Lets every command cheaply detect an out-of-band apps.json edit and self-heal.
@@ -73,7 +77,7 @@ else
 fi
 
 ensure_registry_dir() {
-  mkdir -p "$DEVKIT_HOME" "$PIDS_DIR" "$LOGS_DIR" 2>/dev/null || true
+  mkdir -p "$DEVKIT_HOME" "$PIDS_DIR" "$LOGS_DIR" "$STATUS_DIR" 2>/dev/null || true
 }
 
 # One-time move of an old-style registry into the new default location (~/.devkit).
@@ -252,6 +256,22 @@ assert_unique() {
 
 pid_file() { echo "$PIDS_DIR/$1.pid"; }
 log_file() { echo "$LOGS_DIR/$1.log"; }
+fail_file() { echo "$STATUS_DIR/$1.fail"; }
+
+# Record / clear / read the outcome of the most recent `devkit start <name>`. The marker
+# file holds "<epoch>\t<reason>" so a UI can show when it failed and why, and always point
+# at the log. Best-effort: a read-only registry dir must never make start itself fail.
+mark_start_failed() {
+  local name="$1" reason="${2:-start failed}"
+  mkdir -p "$STATUS_DIR" 2>/dev/null || return 0
+  printf '%s\t%s\n' "$(date +%s 2>/dev/null || echo 0)" "$reason" > "$(fail_file "$name")" 2>/dev/null || true
+}
+clear_start_failed() { rm -f "$(fail_file "$1")" 2>/dev/null || true; }
+start_failed_reason() {
+  local f; f=$(fail_file "$1")
+  [[ -f "$f" ]] || return 1
+  cut -f2- < "$f" 2>/dev/null
+}
 app_port() { jq -r --arg n "$1" '.apps[] | select(.name==$n) | .port // empty' "$APPS_JSON"; }
 
 canonical_path() {
@@ -289,7 +309,7 @@ wait_for_pid_exit() {
   local i=0
   while kill -0 "$pid" 2>/dev/null && (( i < attempts )); do
     sleep 0.1
-    (( i++ ))
+    i=$((i + 1))
   done
   ! kill -0 "$pid" 2>/dev/null
 }
@@ -304,7 +324,7 @@ wait_for_port_state() {
       [[ "$desired" == "free" ]] && return 0
     fi
     sleep 0.1
-    (( i++ ))
+    i=$((i + 1))
   done
 
   if port_is_listening "$port"; then
@@ -410,6 +430,26 @@ is_running() {
   [[ -n "$port" && -n "$path" && -n "$(listener_pids_for_app "$port" "$path" "$logfile")" ]]
 }
 
+# Single source of truth for an app's display state, shared by `list` (text + --json) so
+# the human and machine views can never disagree. Returns one of:
+#   running  — a matching listener is up
+#   external — not lifecycle-managed by devkit (we don't claim to know)
+#   failed   — last `devkit start` failed and it's still not up (points the user at logs)
+#   stopped  — managed, down, no recorded failure (identical to never-started)
+app_state() {
+  local name="$1" managed="$2"
+  [[ "$managed" == "pm2" ]] && managed="devkit"
+  case "$managed" in
+    external) echo "external"; return 0 ;;
+    devkit)
+      if is_running "$name"; then echo "running"; return 0; fi
+      [[ -f "$(fail_file "$name")" ]] && { echo "failed"; return 0; }
+      echo "stopped"
+      ;;
+    *) echo "stopped" ;;
+  esac
+}
+
 # ---------- commands ----------
 
 cmd_register() {
@@ -510,6 +550,7 @@ cmd_rename() {
   [[ -f "$(log_file "$old")" ]] && mv "$(log_file "$old")" "$(log_file "$new")" 2>/dev/null || true
   if want_has "$old"; then ensure_supervisor_dirs; mv "$WANTS_DIR/$old" "$WANTS_DIR/$new" 2>/dev/null || true; fi
   [[ -f "$SUPERVISOR_STATE_DIR/$old" ]] && mv "$SUPERVISOR_STATE_DIR/$old" "$SUPERVISOR_STATE_DIR/$new" 2>/dev/null || true
+  [[ -f "$(fail_file "$old")" ]] && mv "$(fail_file "$old")" "$(fail_file "$new")" 2>/dev/null || true
 
   local new_host="$new.$(tld)"
   local tmp; tmp=$(mktemp)
@@ -609,6 +650,7 @@ cmd_remove() {
     _stop_by_name "$name" "$port" "$path" || die "$name did not stop cleanly before remove"
   fi
   want_clear "$name"
+  clear_start_failed "$name"
   rm -f "$(pid_file "$name")" "$(log_file "$name")"
   local tmp; tmp=$(mktemp)
   jq --arg n "$name" '.apps |= map(select(.name != $n))' "$APPS_JSON" > "$tmp"
@@ -619,17 +661,48 @@ cmd_remove() {
 
 cmd_list() {
   ensure_registry
+  local json_mode=false
+  [[ "${1:-}" == "--json" ]] && json_mode=true
+
+  if $json_mode; then
+    # Machine-readable view for the app / MCP, so they consume devkit's own state instead
+    # of re-deriving it (and drifting). One JSON object per app, including the failed-start
+    # reason and the log path so a UI can link straight to the dead end's cause.
+    local objs=()
+    local name port hostname path managed
+    # Fields are \x1f-separated (not \t): tab is IFS-whitespace, so an empty middle field
+    # — e.g. a path-less external app — would collapse and shift every later column.
+    while IFS=$'\037' read -r name port hostname path managed; do
+      local state reason
+      state=$(app_state "$name" "$managed")
+      reason=""
+      [[ "$state" == "failed" ]] && reason=$(start_failed_reason "$name" || true)
+      objs+=("$(jq -n \
+        --arg name "$name" --argjson port "${port:-0}" --arg host "$hostname" \
+        --arg url "$(url_for "$hostname")" --arg state "$state" \
+        --arg path "$path" --arg managed "$managed" \
+        --arg reason "$reason" --arg log "$(log_file "$name")" \
+        '{name:$name, port:$port, hostname:$host, url:$url, state:$state,
+          path:(if $path=="" then null else $path end), managedBy:$managed,
+          failReason:(if $reason=="" then null else $reason end),
+          logFile:(if $state=="failed" then $log else null end)}')")
+    done < <(jq -r '.apps[] | [.name, (.port|tostring), .hostname, (.path // ""), .managedBy] | join("\u001f")' "$APPS_JSON")
+    if (( ${#objs[@]} == 0 )); then printf '[]\n'; else printf '%s\n' "${objs[@]}" | jq -s '.'; fi
+    return 0
+  fi
+
   printf "%-18s %-7s %-32s %-9s %s\n" NAME PORT URL STATE PATH
-  while IFS=$'\t' read -r name port hostname path managed; do
-    local state="stopped"
-    [[ "$managed" == "pm2" ]] && managed="devkit"
-    if [[ "$managed" == "devkit" ]]; then
-      is_running "$name" && state="running" || state="stopped"
-    elif [[ "$managed" == "external" ]]; then
-      state="external"
-    fi
+  local failed_any=0
+  local name port hostname path managed
+  while IFS=$'\037' read -r name port hostname path managed; do
+    local state; state=$(app_state "$name" "$managed")
+    [[ "$state" == "failed" ]] && failed_any=1
     printf "%-18s %-7s %-32s %-9s %s\n" "$name" "$port" "$(url_for "$hostname")" "$state" "${path:-—}"
-  done < <(jq -r '.apps[] | [.name, (.port|tostring), .hostname, (.path // ""), .managedBy] | @tsv' "$APPS_JSON")
+  done < <(jq -r '.apps[] | [.name, (.port|tostring), .hostname, (.path // ""), .managedBy] | join("\u001f")' "$APPS_JSON")
+  if (( failed_any )); then
+    echo
+    echo "  one or more apps failed their last start — inspect with: devkit logs <name>"
+  fi
 }
 
 cmd_show() {
@@ -698,13 +771,28 @@ scan_probe_to_file() {
   local hdr body code ctype scheme=http head title is_html=0 cmd cwd
 
   hdr=$(mktemp); body=$(mktemp)
-  code=$(curl -s -L --max-redirs 3 --connect-timeout "$SCAN_CONNECT_TIMEOUT" -m "$SCAN_TIMEOUT" \
-         -o "$body" -D "$hdr" -w '%{http_code}' "http://127.0.0.1:$port/" 2>/dev/null) || code=000
-  if [[ "$code" == 000 ]]; then
-    code=$(curl -sk -L --max-redirs 3 --connect-timeout "$SCAN_CONNECT_TIMEOUT" -m "$SCAN_TIMEOUT" \
-           -o "$body" -D "$hdr" -w '%{http_code}' "https://127.0.0.1:$port/" 2>/dev/null) || code=000
-    scheme=https
-  fi
+  # Probe HTTP and HTTPS on both IPv4 (127.0.0.1) and IPv6 ([::1]), and trust curl's
+  # reported %{http_code} rather than its exit status. Two reasons this matters:
+  #   * Some servers (notably a self-signed TLS handshake teardown) make curl exit
+  #     non-zero AFTER returning a valid status line. The old `|| code=000` clobbered
+  #     that real 200 with 000 and the app was silently dropped.
+  #   * Servers that bind ::1 ONLY (Vite v6's default) are invisible to a 127.0.0.1-only
+  #     probe even though lsof already enumerated them — enumerated, then dropped.
+  # `|| true` lives INSIDE the command substitution so a non-zero curl can't abort this
+  # backgrounded probe under `set -e`, while the captured http_code is preserved.
+  local _scheme _host _c
+  local _args=()
+  code=000
+  for _scheme in http https; do
+    for _host in 127.0.0.1 '[::1]'; do
+      _args=(-s -L --max-redirs 3 --connect-timeout "$SCAN_CONNECT_TIMEOUT" -m "$SCAN_TIMEOUT"
+             -o "$body" -D "$hdr" -w '%{http_code}')
+      [[ "$_scheme" == https ]] && _args+=(-k)
+      _c=$(curl "${_args[@]}" "$_scheme://$_host:$port/" 2>/dev/null || true)
+      [[ -n "$_c" ]] || _c=000
+      if [[ "$_c" != 000 ]]; then code="$_c"; scheme="$_scheme"; break 2; fi
+    done
+  done
   if [[ "$code" == 000 ]]; then rm -f "$hdr" "$body"; return 0; fi
 
   # `|| true`: under `set -euo pipefail` a grep with no match exits 1, and because this
@@ -890,9 +978,11 @@ cmd_start() {
   port=$(jq -r --arg n "$name" '.apps[] | select(.name==$n) | .port' "$APPS_JSON")
 
   if [[ -z "$path" || ! -d "$path" ]]; then
+    mark_start_failed "$name" "path missing or not a directory: $path"
     echo "devkit: $name — path missing or not a directory: $path" >&2; return 1
   fi
   if [[ -z "$cmd" || "$cmd" == "null" ]]; then
+    mark_start_failed "$name" "no startCmd registered"
     echo "devkit: $name — no startCmd registered" >&2; return 1
   fi
 
@@ -901,10 +991,12 @@ cmd_start() {
 
   # Stop any existing tracked process before starting a new one
   _stop_by_name "$name" "$port" "$path_for_app" || {
+    mark_start_failed "$name" "failed to stop cleanly before restart"
     echo "devkit: $name — failed to stop cleanly before restart" >&2
     return 1
   }
   if ! wait_for_port_state "$port" free 20; then
+    mark_start_failed "$name" "port $port is still in use by another process"
     echo "devkit: $name — port $port is still in use by another process" >&2
     return 1
   fi
@@ -935,7 +1027,7 @@ cmd_start() {
     pid=$(cat "$pf" 2>/dev/null || echo "")
     [[ -n "$pid" ]] && break
     sleep 0.1
-    (( i++ ))
+    i=$((i + 1))
   done
 
   # Wait for the app to bind its port, but fail fast if the launched process exits first
@@ -947,12 +1039,17 @@ cmd_start() {
     if [[ -n "$(listener_pids_on_port "$port" 2>/dev/null)" ]]; then bound=true; break; fi
     if [[ -n "$pid" ]] && ! kill -0 "$pid" 2>/dev/null; then break; fi
     sleep 0.1
-    (( waited++ ))
+    waited=$((waited + 1))
   done
   if ! $bound; then
     _stop_by_name "$name" "$port" "$path_for_app" >/dev/null 2>&1 || true
     local hint="check logs: devkit logs $name"
-    [[ -n "$pid" ]] && ! kill -0 "$pid" 2>/dev/null && hint="the start command exited immediately — $hint"
+    local reason="failed to bind port $port"
+    if [[ -n "$pid" ]] && ! kill -0 "$pid" 2>/dev/null; then
+      hint="the start command exited immediately — $hint"
+      reason="the start command exited immediately before binding port $port"
+    fi
+    mark_start_failed "$name" "$reason"
     echo "devkit: $name failed to bind port $port — $hint" >&2
     return 1
   fi
@@ -964,13 +1061,15 @@ cmd_start() {
     echo "$pid" > "$pf"
   elif [[ -z "$pid" ]] || ! kill -0 "$pid" 2>/dev/null; then
     rm -f "$pf"
+    mark_start_failed "$name" "started listening on $port but no stable listener PID was found"
     echo "devkit: $name started listening on $port, but no stable listener PID was found" >&2
     return 1
   fi
 
   # Record desired state = up and clear any crash-backoff (this start is healthy).
   want_set "$name"
   rm -f "$SUPERVISOR_STATE_DIR/$name" 2>/dev/null || true
+  clear_start_failed "$name"   # last start succeeded — drop any stale "failed" marker
 
   echo "started $name (pid $pid) → $(url_for "$(jq -r --arg n "$name" '.apps[] | select(.name==$n) | .hostname' "$APPS_JSON")")"
 }
@@ -982,6 +1081,7 @@ cmd_stop() {
   # Desired state = down. Clear it first so the supervisor won't race to restart what
   # the user just asked to stop.
   want_clear "$name"
+  clear_start_failed "$name"   # an explicit stop is not a failed-start state
   is_running "$name" || { echo "devkit: $name is not running"; return 0; }
   local port path
   port=$(app_port "$name")
@@ -1044,7 +1144,7 @@ cmd_start_all() {
   local i=0 running=0
   for name in "${names[@]}"; do
     ( cmd_start "$name" >"$workdir/$i.out" 2>&1; echo $? >"$workdir/$i.rc" ) &
-    (( i++ )); (( running++ ))
+    i=$((i + 1)); running=$((running + 1))
     if (( running >= conc )); then wait; running=0; fi
   done
   wait
@@ -1053,8 +1153,8 @@ cmd_start_all() {
   for name in "${names[@]}"; do
     [[ -s "$workdir/$idx.out" ]] && cat "$workdir/$idx.out"
     local rc; rc=$(cat "$workdir/$idx.rc" 2>/dev/null || echo 1)
-    if [[ "$rc" == 0 ]]; then (( started++ )); else (( failed++ )) || true; fi
-    (( idx++ ))
+    if [[ "$rc" == 0 ]]; then started=$((started + 1)); else failed=$((failed + 1)); fi
+    idx=$((idx + 1))
   done
   rm -rf "$workdir"
   local msg="started $started app(s)"
@@ -1075,10 +1175,10 @@ cmd_stop_all() {
       path=$(app_path "$name")
       if _stop_by_name "$name" "$port" "$path"; then
         echo "stopped $name"
-        (( stopped++ ))
+        stopped=$((stopped + 1))
       else
         echo "devkit: failed to stop $name" >&2
-        (( failed++ )) || true
+        failed=$((failed + 1))
       fi
     fi
   done < <(jq -r '.apps[] | select(.managedBy=="devkit") | .name' "$APPS_JSON")
@@ -1104,15 +1204,15 @@ cmd_restart_all() {
   local i=0 running=0
   for name in "${names[@]}"; do
     ( cmd_restart "$name" >"$workdir/$i.out" 2>&1; echo $? >"$workdir/$i.rc" ) &
-    (( i++ )); (( running++ ))
+    i=$((i + 1)); running=$((running + 1))
     if (( running >= conc )); then wait; running=0; fi
   done
   wait
 
   local idx=0
   for name in "${names[@]}"; do
     [[ -s "$workdir/$idx.out" ]] && cat "$workdir/$idx.out"
-    (( idx++ ))
+    idx=$((idx + 1))
   done
   rm -rf "$workdir"
 }
@@ -1164,7 +1264,7 @@ render_dashboard() {
 
   local rows=""
   local link_attrs='target="_blank" rel="noopener"'
-  while IFS=$'\t' read -r name hostname port managed path repo claude; do
+  while IFS=$'\037' read -r name hostname port managed path repo claude; do
     local url; url=$(url_for "$hostname")
     local repo_cell="—"
     [[ -n "$repo" && "$repo" != "null" ]] && repo_cell="<a href=\"$repo\" $link_attrs>repo</a>"
@@ -1184,7 +1284,7 @@ render_dashboard() {
           <td><code class=\"copy\" title=\"click to copy\">devkit edit $name</code></td>
         </tr>
 "
-  done < <(jq -r '.apps[] | [.name, .hostname, (.port|tostring), .managedBy, (.path // ""), (.repo // ""), (.claudeMd // "")] | @tsv' "$APPS_JSON")
+  done < <(jq -r '.apps[] | [.name, .hostname, (.port|tostring), .managedBy, (.path // ""), (.repo // ""), (.claudeMd // "")] | join("\u001f")' "$APPS_JSON")
 
   # Atomic write (temp-in-dir + rename), same rationale as render_caddyfile.
   local tmp; tmp=$(mktemp "$DASHBOARD.XXXXXX" 2>/dev/null) || return 1
@@ -1431,7 +1531,16 @@ cmd_clone() {
     return 0
   fi
   mkdir -p "$(dirname "$path")"
-  git clone "$repo" "$path"
+  # Never let clone hang a non-interactive caller (an agent, a script, clone-all). A
+  # private HTTPS repo would otherwise prompt for a username/password on the TTY, and SSH
+  # would prompt for a passphrase or unknown-host confirmation — both block forever with
+  # no terminal. Disable the prompts so git fails fast with a clear next action instead.
+  if ! GIT_TERMINAL_PROMPT=0 GIT_SSH_COMMAND="${GIT_SSH_COMMAND:-ssh -oBatchMode=yes}" \
+       git clone "$repo" "$path"; then
+    die "clone failed for $name ($repo)
+  If the repo is private, set up access first (e.g. 'gh auth login' or an SSH key), then: devkit clone $name"
+  fi
+  echo "cloned $name → $path"
 }
 
 cmd_clone_all() {
@@ -1710,7 +1819,8 @@ Registry:
   devkit rename <old> <new>
   devkit update <name> [--port N] [--cmd C] [--path P] [--desc D] [--managed-by M]
   devkit remove <name>
-  devkit list
+  devkit list                 # NAME/PORT/URL/STATE/PATH (STATE: running|stopped|failed|external)
+  devkit list --json          # machine-readable state for the menu bar app / agents
   devkit show <name>
   devkit scan                 # find running web apps not yet registered
   devkit scan --json          # machine-readable output (for the menu bar app / agents)
@@ -1764,7 +1874,7 @@ case "${1:-help}" in
   rename)      shift; cmd_rename "$@" ;;
   update)      shift; cmd_update "$@" ;;
   remove)      shift; cmd_remove "$@" ;;
-  list)        cmd_list ;;
+  list)        shift; cmd_list "$@" ;;
   show)        shift; cmd_show "$@" ;;
   scan)        shift; cmd_scan "$@" ;;
   reload)      cmd_reload ;;