Skip to content

Commit d00e871

Browse files
committed
telegram: spawn+exit restart, singleton lock, typing indicator fix, update docs
- Replace syscall.Exec with spawn+exit: SIGHUP spawns child, parent exits No more binary overwrite races, stale HTTP/2 connections, or session loops - Singleton lock via ~/.odek/telegram.pid kills stale instances on startup Prevents 409 Conflict errors from dual Telegram polling - Typing indicator: fire-and-forget SendChatAction per tick Hanging HTTP call can't block the ticker loop anymore - Remove restartRequested atomic.Bool (no longer needed) - Update docs: TELEGRAM.md + CHEATSHEET.md with restart, lock, plan commands - Tests: spawn/marker tests replace tryReexec/atomic tests
1 parent 3f02b0f commit d00e871

4 files changed

Lines changed: 151 additions & 214 deletions

File tree

cmd/odek/telegram.go

Lines changed: 71 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"strconv"
1111
"strings"
1212
"sync"
13-
"sync/atomic"
1413
"syscall"
1514
"time"
1615

@@ -132,28 +131,20 @@ func telegramCmd(args []string) error {
132131
return "", nil
133132
}
134133

135-
// restartRequested is set atomically when a /restart command is received.
136-
// Checked after the update loop exits to decide between restart and exit.
137-
var restartRequested atomic.Bool
138-
139134
handler.OnCommand = func(chatID int64, cmdName string, argsStr string) (string, error) {
140135
cmd := telegram.FindCommand(cmdName)
141136
if cmd == nil {
142137
return fmt.Sprintf("Unknown command: /%s", cmdName), nil
143138
}
144139

145-
// Handle /restart — send confirmation directly, then trigger SIGHUP.
140+
// Handle /restart — send confirmation, then signal SIGHUP.
141+
// The signal handler spawns a child and exits.
146142
if cmdName == "restart" {
147-
// Send the restart message directly via the bot to ensure it's
148-
// delivered before the process re-execs.
149143
if _, err := bot.SendMessage(chatID,
150144
"🔄 *Restarting...*\n\nThe bot will restart momentarily. This may take a few seconds.",
151145
nil); err != nil {
152146
handlerLog.Error("send restart message failed", "chat_id", chatID, "error", err)
153147
}
154-
// Signal SIGHUP to self — the signal handler will cancel the
155-
// context, stopping the poller, and the main loop will re-exec.
156-
restartRequested.Store(true)
157148
syscall.Kill(os.Getpid(), syscall.SIGHUP)
158149
return "", nil
159150
}
@@ -373,20 +364,31 @@ func telegramCmd(args []string) error {
373364
ctx, cancel := context.WithCancel(context.Background())
374365
defer cancel()
375366

376-
// 15. Handle SIGINT/SIGTERM/SIGHUP for graceful shutdown and restart.
377-
// SIGHUP triggers a full process restart (used by /restart command).
367+
// 15. Handle SIGINT/SIGTERM/SIGHUP.
368+
// SIGHUP spawns a child process then exits (used by /restart and
369+
// agent-triggered restarts). The child's acquireLock kills this
370+
// process if it's still alive. SIGINT/SIGTERM do a graceful shutdown.
378371
sigCh := make(chan os.Signal, 1)
379372
signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP)
380373
go func() {
381374
sig := <-sigCh
382375
if sig == syscall.SIGHUP {
383-
fmt.Fprintf(os.Stderr, "\nodek telegram: restart requested...\n")
384-
} else {
385-
fmt.Fprintf(os.Stderr, "\nodek telegram: shutting down...\n")
376+
fmt.Fprintf(os.Stderr, "\nodek telegram: restart requested — spawning child...\n")
377+
writeRestartMarker()
378+
if err := spawnChild(); err != nil {
379+
fmt.Fprintf(os.Stderr, "odek telegram: spawn failed: %v\n", err)
380+
}
381+
os.Exit(0)
386382
}
383+
fmt.Fprintf(os.Stderr, "\nodek telegram: shutting down...\n")
387384
cancel()
388385
}()
389386

387+
// 15b. Check for restart marker from previous instance and notify.
388+
if chatID, ok := readRestartMarker(); ok && chatID != 0 {
389+
bot.SendMessage(chatID, "🔄 *Bot restarted*\n\nA new instance is now running. Use `/new` if you experience any context issues.", &telegram.SendOpts{ParseMode: "Markdown"})
390+
}
391+
390392
// 16. Start polling in a background goroutine.
391393
updates := make(chan telegram.Update, 100)
392394
go poller.Start(ctx, updates)
@@ -396,37 +398,71 @@ func telegramCmd(args []string) error {
396398
handler.HandleUpdate(upd)
397399
}
398400

399-
// 18. If restart was requested (via /restart command), re-exec the binary.
400-
// This preserves the exact same arguments so the bot comes back with
401-
// the same configuration. If syscall.Exec fails, fall through to exit.
402-
if restartRequested.Load() {
403-
return tryReexec()
401+
// 18. Clean exit.
402+
return nil
403+
}
404+
405+
// ── Restart (spawn + exit) ─────────────────────────────────────────────
406+
//
407+
// Instead of syscall.Exec (binary race, stale HTTP/2 connections, session
408+
// loops), we spawn a child process and exit. The child's acquireLock kills
409+
// the old process if it's still alive.
410+
411+
// restartMarkerPath returns the path to the restart marker file.
412+
func restartMarkerPath() (string, error) {
413+
home, err := os.UserHomeDir()
414+
if err != nil {
415+
return "", err
404416
}
417+
return filepath.Join(home, ".odek", "restart.json"), nil
418+
}
405419

406-
return nil
420+
// writeRestartMarker writes a marker so the next instance knows a restart
421+
// just happened. Currently writes an empty marker (global restart).
422+
func writeRestartMarker() error {
423+
path, err := restartMarkerPath()
424+
if err != nil {
425+
return err
426+
}
427+
return os.WriteFile(path, []byte("{}\n"), 0644)
407428
}
408429

409-
// execFunc is the system call used to replace the current process image.
410-
// Swapped in tests to avoid replacing the test process.
411-
var execFunc func(argv0 string, argv []string, envv []string) error = syscall.Exec
430+
// readRestartMarker reads and removes the restart marker. Returns true
431+
// if a marker existed, along with the chat_id (0 if none specified).
432+
func readRestartMarker() (int64, bool) {
433+
path, err := restartMarkerPath()
434+
if err != nil {
435+
return 0, false
436+
}
437+
data, err := os.ReadFile(path)
438+
if err != nil {
439+
return 0, false
440+
}
441+
os.Remove(path)
442+
// Future: parse chat_id from JSON. For now, just signal restart happened.
443+
_ = data
444+
return 0, true
445+
}
412446

413-
// tryReexec replaces the current process with the same binary and arguments.
414-
// On success, it never returns (the new process takes over). On failure, it
415-
// logs the error and returns it so the caller can fall through to graceful exit.
416-
func tryReexec() error {
447+
// spawnChild starts a new odek telegram process detached from the parent.
448+
func spawnChild() error {
417449
exe, err := os.Executable()
418450
if err != nil {
419-
return fmt.Errorf("re-exec: cannot find executable: %w", err)
451+
return fmt.Errorf("executable: %w", err)
420452
}
453+
// Copy args (same as current process).
421454
argv := make([]string, len(os.Args))
422455
copy(argv, os.Args)
423456
argv[0] = exe
424-
fmt.Fprintf(os.Stderr, "odek telegram: re-executing %s %v...\n", exe, os.Args[1:])
425-
if err := execFunc(exe, argv, os.Environ()); err != nil {
426-
fmt.Fprintf(os.Stderr, "odek telegram: restart failed: %v\n", err)
427-
return err
457+
458+
attr := &os.ProcAttr{
459+
Env: os.Environ(),
460+
// Detach: nil files so child gets its own stdin/stdout/stderr.
461+
// The child process will be reparented to init when we exit.
462+
Files: []*os.File{nil, nil, nil},
428463
}
429-
return nil
464+
_, err = os.StartProcess(exe, argv, attr)
465+
return err
430466
}
431467

432468
// handleChatMessage processes a user message from Telegram in a background

0 commit comments

Comments
 (0)