Skip to content

fix: validate settings before stopping daemon on restart #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jcenters wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: validate settings before stopping daemon on restart #219

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions lib/daemon.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,54 @@ stop_daemon() {
log "Daemon stopped"
}

# Validate settings without starting the daemon.
# Returns 0 if ready to start, non-zero otherwise.
# Safe to call while the daemon is running — reads only, no side effects.
Comment on lines +271 to +272
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() {

preflight_check() {
load_settings
local load_rc=$?

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
Comment on lines +277 to +283
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
  1. Documenting in the function comment that auto-repair is intentionally skipped to avoid side effects during preflight.

elif [ $load_rc -ne 0 ]; then
echo -e "${RED}Restart aborted: settings.json is missing or has no configuration${NC}"
echo " Run 'tinyclaw setup' to reconfigure"
return 1
fi

if [ ${#ACTIVE_CHANNELS[@]} -eq 0 ]; then
echo -e "${RED}Restart aborted: no channels configured in settings.json${NC}"
echo " Run 'tinyclaw setup' to reconfigure"
return 1
fi

for ch in "${ACTIVE_CHANNELS[@]}"; do
local token_key
token_key="$(channel_token_key "$ch")"
if [ -n "$token_key" ] && [ -z "$(get_channel_token "$ch")" ]; then
echo -e "${RED}Restart aborted: $(channel_display "$ch") bot token is missing${NC}"
echo " Run 'tinyclaw setup' to reconfigure"
return 1
fi
done

return 0
}

# Restart daemon safely even when called from inside TinyClaw's tmux session
restart_daemon() {
# Validate settings before touching the live session.
# If preflight fails, the running daemon stays up and we bail out.
if ! preflight_check; then
echo -e "${YELLOW}Daemon restart cancelled — live session preserved${NC}"
return 1
fi

if session_exists && [ -n "${TMUX:-}" ]; then
local current_session
current_session=$(tmux display-message -p '#S' 2>/dev/null || true)
Expand Down