fix: validate settings before stopping daemon on restart

[jcenters]

Bug

restart_daemon() called stop_daemon before verifying the new settings are valid. If settings.json is broken (e.g. missing channels.enabled), the old session was already dead and start_daemon failed silently — leaving nothing running.

Both restart paths were broken the same way:

Normal (outside tmux):

stop_daemon      # kills live session
sleep 2
start_daemon     # fails if settings bad → nothing running

Inside tmux:

nohup bash tinyclaw.sh __delayed_start &   # fire-and-forget: sleep 2; start_daemon
stop_daemon                                 # kills current session immediately
# if start_daemon fails → nothing running, no way to surface error

Fix

Added a preflight_check() function that runs the settings validation (load_settings + token check + channels check) without starting or writing anything. It is called at the top of restart_daemon before any stop action. If it fails, the restart is aborted with a clear error message — the live session stays up.

• Invalid JSON in settings.json: aborts with the jq parse error and path to fix it
• Missing/empty settings: aborts with prompt to run tinyclaw setup
• No channels configured: aborts with prompt to run tinyclaw setup
• Missing bot token for a channel: aborts identifying which channel, with prompt to run tinyclaw setup

Test plan

• With a valid settings.json, tinyclaw restart works as before
• With invalid JSON in settings.json, tinyclaw restart prints the parse error and exits without killing the running session
• With channels.enabled removed from settings.json, tinyclaw restart aborts with "no channels configured" and the daemon keeps running
• With a missing bot token, tinyclaw restart identifies the affected channel and aborts

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes a race condition in restart_daemon() where stop_daemon() was called before validating the new settings, potentially leaving nothing running if settings.json was broken. The fix introduces a preflight_check() function that validates settings (JSON syntax, channel configuration, and bot tokens) before touching the live session — aborting the restart cleanly if anything is wrong.

Key changes:

• New preflight_check() in lib/daemon.sh mirrors the validation logic in start_daemon() without any start/write side effects
• restart_daemon() now calls preflight_check() as its very first step; failure aborts with a clear message while the live daemon stays up
• Both the normal restart path and the inside-tmux delayed-start path are protected
• The comment on preflight_check incorrectly states "reads only, no side effects" — load_settings() does modify globals (ACTIVE_CHANNELS, WORKSPACE_PATH, _CHANNEL_TOKEN_KEYS, _CHANNEL_TOKEN_VALS), which is benign here but could mislead future callers
• The invalid-JSON error path tells users to "Fix manually" without mentioning that tinyclaw stop && tinyclaw start would trigger the existing jsonrepair auto-repair in start_daemon()

Confidence Score: 4/5

• Safe to merge — the core fix is correct and well-scoped; only minor documentation and UX nits remain.
• The fundamental logic is sound: preflight runs before any destructive action, all three failure modes (invalid JSON, no channels, missing token) are handled, and the happy path is unchanged. The two style-level issues (misleading comment and incomplete error hint for JSON auto-repair) do not affect correctness.
• No files require special attention beyond the style comments on lib/daemon.sh.

Important Files Changed

Filename	Overview
lib/daemon.sh	Adds preflight_check() and wires it into restart_daemon() before any stop action; core logic is sound but has a misleading "no side effects" comment and silently bypasses start_daemon's JSON auto-repair path.

Sequence Diagram

sequenceDiagram
    actor User
    participant restart_daemon
    participant preflight_check
    participant load_settings
    participant stop_daemon
    participant start_daemon

    User->>restart_daemon: tinyclaw restart

    restart_daemon->>preflight_check: preflight_check()
    preflight_check->>load_settings: load_settings()
    load_settings-->>preflight_check: rc=0 (ok) / rc=1 (missing) / rc=2 (bad JSON)

    alt rc=2 (invalid JSON)
        preflight_check-->>restart_daemon: return 1
        restart_daemon-->>User: ❌ Aborted — live session preserved
    else rc≠0 (missing/no config)
        preflight_check-->>restart_daemon: return 1
        restart_daemon-->>User: ❌ Aborted — live session preserved
    else ACTIVE_CHANNELS empty
        preflight_check-->>restart_daemon: return 1
        restart_daemon-->>User: ❌ Aborted — live session preserved
    else token missing for a channel
        preflight_check-->>restart_daemon: return 1
        restart_daemon-->>User: ❌ Aborted — live session preserved
    else all checks pass
        preflight_check-->>restart_daemon: return 0
        restart_daemon->>stop_daemon: stop_daemon()
        stop_daemon-->>restart_daemon: done
        restart_daemon->>start_daemon: start_daemon()
        start_daemon-->>restart_daemon: done
        restart_daemon-->>User: ✅ Daemon restarted
    end

Loading

_{Last reviewed commit: b8169c1}

[greptile-apps]

Misleading "no side effects" comment

The comment claims preflight_check is "reads only, no side effects", but load_settings() (called on line 274) actually modifies several global variables: ACTIVE_CHANNELS, WORKSPACE_PATH, _CHANNEL_TOKEN_KEYS, and _CHANNEL_TOKEN_VALS.

This is currently benign — start_daemon() calls load_settings() again immediately after and reinitialises these globals — but the comment could mislead a future developer into calling preflight_check() from a context where those globals must remain untouched.

Suggested change

	# Returns 0 if ready to start, non-zero otherwise.
	# Safe to call while the daemon is running — reads only, no side effects.
	# Validate settings without starting the daemon.
	# Returns 0 if ready to start, non-zero otherwise.
	# NOTE: calls load_settings() internally, which resets ACTIVE_CHANNELS,
	# WORKSPACE_PATH, and the channel-token arrays as a side effect.
	preflight_check() {

[greptile-apps]

Auto-repair path bypassed and error message is incomplete

start_daemon() (lines 39-66) has an auto-repair step: when load_rc -eq 2, it invokes jsonrepair to attempt to fix the broken JSON before giving up. preflight_check skips that step entirely and tells the user to "Fix manually", which is accurate but incomplete — it doesn't inform the user that tinyclaw stop && tinyclaw start would attempt automatic repair.

Consider either:

1. Mentioning the auto-repair path in the error message:

Suggested change

	if [ $load_rc -eq 2 ]; then
	local jq_err
	jq_err=$(jq empty "$SETTINGS_FILE" 2>&1)
	echo -e "${RED}Restart aborted: settings.json contains invalid JSON${NC}"
	echo -e " ${YELLOW}${jq_err}${NC}"
	echo " Fix manually: $SETTINGS_FILE"
	return 1
	if [ $load_rc -eq 2 ]; then
	local jq_err
	jq_err=$(jq empty "$SETTINGS_FILE" 2>&1)
	echo -e "${RED}Restart aborted: settings.json contains invalid JSON${NC}"
	echo -e " ${YELLOW}${jq_err}${NC}"
	echo " Fix manually: $SETTINGS_FILE"
	echo " Or run 'tinyclaw stop && tinyclaw start' to attempt auto-repair"
	return 1

2. Documenting in the function comment that auto-repair is intentionally skipped to avoid side effects during preflight.