Fix service shutdown deadlock and log directory naming mismatch

[Copilot]

Fix Plan: Graceful Shutdown Deadlock & Log Directory Mismatch ✅

Issue 1: Graceful Shutdown Deadlock ✅

• Review existing code - handler.go already has httpServer field and Shutdown() method
• Review existing code - program.go already calls srv.Shutdown() in Stop()
• Fix run() in program.go to handle http.ErrServerClosed as normal shutdown (not fatal)
• Add net/http import to program.go
• Verify shutdown flow: Stop() closes s.quit safely after server shutdown completes
• Add and refine comments to document the shutdown flow precisely

Issue 2: Log Directory Naming Mismatch ✅

• Add LogServiceName variable to cmd/BasculaServicio/main.go (renamed from ServiceName)
• Update daemon.New() to accept logServiceName parameter
• Store LogServiceName in Service struct with comprehensive documentation
• Use injected LogServiceName in Init() for logging setup instead of s.env.ServiceName
• Update Taskfile.yml to inject correct service names via main.LogServiceName
• Update config.go fallback defaults to match naming convention

Code Review Feedback ✅

• Rename ServiceName → LogServiceName to clarify it's for logging, not Windows SCM service name
• Align naming with Taskfile variables (SVC_LOG_NAME_*)
• Clarify shutdown flow with improved comments explaining how s.quit is closed
• Refine comment precision based on code review

Testing & Verification ✅

• Build and verify compilation succeeds
• Multiple code reviews - all issues resolved
• Run security scan - no vulnerabilities found
• Address all code review feedback

Original prompt

Context

The poster-tuis TUI installer (https://github.com/adcondev/poster-tuis) manages the lifecycle of both scale-daemon and ticket-daemon Windows services. During testing, two critical issues were identified:

1. Scale daemon hangs on stop/uninstall — the service process never exits when sc stop is sent, leaving it in STOP_PENDING state until force-killed from Task Manager. The ticket-daemon stops cleanly with the exact same TUI code.
2. TUI cannot open log files/folders — the log directory path the TUI expects doesn't match the path the daemon actually writes to.

---

Issue 1: Graceful Shutdown Deadlock (CRITICAL)

Root Cause

The Stop() method in internal/daemon/program.go deadlocks because:

1. Stop() calls s.wg.Wait() which waits for the run() goroutine to finish
2. run() is blocked on <-s.quit — waiting for the quit channel to close
3. The quit channel is only closed when ListenAndServe() returns with an error (line ~122 in run())
4. ListenAndServe() in internal/server/handler.go uses the raw http.ListenAndServe() function which has no way to be shut down gracefully — there's no stored *http.Server reference
5. So ListenAndServe() never returns → <-s.quit blocks forever → run() never exits → wg.Wait() deadlocks → Stop() hangs → Windows SCM times out

The deadlock chain:

SCM sends Stop signal
  → Stop() calls s.cancel() ✓ (context cancelled)
  → Stop() calls s.reader.Stop() ✓ (serial reader exits)
  → Stop() calls s.wg.Wait() ← BLOCKS HERE
    → run() blocked on <-s.quit ← BLOCKS HERE
      → s.quit only closed on ListenAndServe() error
        → http.ListenAndServe() has NO shutdown method ← ROOT CAUSE

Why ticket-daemon works

The ticket-daemon's Stop() does it correctly:

• Stores an *http.Server and calls httpServer.Shutdown(ctx) — gracefully shuts down HTTP server
• Calls wsServer.Shutdown() — closes all WebSocket clients
• Calls close(p.quit) — explicitly unblocks run()
• Then calls p.wg.Wait() — now completes because run() can exit

Required Fix

In internal/server/handler.go:

• Add an httpServer *http.Server field to the Server struct
• In ListenAndServe(), create an *http.Server{Addr: ..., Handler: mux} and store it before calling s.httpServer.ListenAndServe()
• Add a new Shutdown(ctx context.Context) error method that calls s.httpServer.Shutdown(ctx) if non-nil
• Handle http.ErrServerClosed gracefully in run() (not as a fatal error — it's the normal shutdown path)

In internal/daemon/program.go Stop() method:

• After cancelling context and stopping reader, call s.srv.Shutdown(ctx) with a 10-second timeout to gracefully close the HTTP server
• Explicitly call close(s.quit) to unblock run()
• Then call s.wg.Wait() which will now complete
• Update run() to check for http.ErrServerClosed and not treat it as a fatal error

---

Issue 2: Log Directory Naming Mismatch

What the Taskfile defines and TUI expects

The Taskfile naming convention is R2k_<<Daemon>><<Type>>_<<Env>>. The Taskfile injects main.ServiceName via ldflags:

• Scale Local: -X main.ServiceName=R2k_BasculaServicio_Local
• Scale Remote: -X main.ServiceName=R2k_BasculaServicio_Remote

The TUI installer (poster-tuis) constructs log paths using the Windows service registry name (RegistryName), which follows this convention:

C:\ProgramData\R2k_BasculaServicio_Local\R2k_BasculaServicio_Local.log
C:\ProgramData\R2k_BasculaServicio_Remote\R2k_BasculaServicio_Remote.log

What the daemon actually does

The ServiceName variable from main package is injected but never forwarded to the logging setup. Instead, internal/config/config.go has hardcoded ServiceName values in the Environments map:

var Environments = map[string]Environment{
    "remote": {
        ServiceName: "BasculaServicio_Remoto",  // ← Hardcoded, missing R2k_ prefix, uses "Remoto" instead of "Remote"
    },
    "local": {
        ServiceName: "BasculaServicio_Local",   // ← Hardcoded, missing R2k_ prefix
    },
}

The logging setup in internal/logging/logging.go uses this ServiceName for the log directory:

logDir := filepath.Join(os.Getenv("PROGRAMDATA"), serviceName)
mgr.FilePath = filepath.Join(logDir, serviceName+".log")

So the actual log files end up at:

C:\ProgramData\BasculaServicio_Remoto\BasculaServicio_Remoto.log   (remote)
C:\ProgramData\BasculaServicio_Local\BasculaServicio_Local.log     (local)

But the TUI and the desired convention expect:

C:\ProgramData\R2k_BasculaServicio_Remote\R2k_BasculaServicio_Remote.log
C:\ProgramData\R2k_BasculaServicio_Local\R2k_BasculaServicio_Local.log

Required Fix — Make ServiceName injectable from ldflags

The main.ServiceName variable (which IS correctly injected by the Taskfile as R2k_BasculaServicio_Local / R2k_BasculaServicio_Remote) needs to be forwarded through to the loggin...

This pull request was created from Copilot chat.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[Copilot]

Pull request overview

Fixes two service integration issues: shutdown hangs during sc stop and log folder naming not matching what the TUI installer expects.

Changes:

• Threads an ldflags-injected ServiceName from main → daemon.Service → logging.Setup() to align ProgramData log directories with the Windows service registry name.
• Treats http.ErrServerClosed as an expected case during shutdown rather than a fatal server error.
• Updates remote environment default service name and Taskfile build flags to use the ..._Remote naming convention.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
internal/daemon/program.go	Accepts injected service name for logging and adjusts HTTP server error handling on shutdown.
internal/config/config.go	Updates remote default ServiceName to match expected registry/log directory naming.
cmd/BasculaServicio/main.go	Adds ServiceName build-time variable and passes it into daemon.New().
Taskfile.yml	Injects log-oriented service name via ldflags for local/remote builds.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

run() previously used the ListenAndServe() return path to close s.quit. With this change, an expected graceful shutdown (http.ErrServerClosed) will no longer close s.quit, which can reintroduce the stop deadlock if the main run() loop is blocked on <-s.quit (as described in the PR context). A concrete fix is to ensure s.quit is closed on the normal stop path (e.g., in Stop() after calling server shutdown), or close s.quit when ListenAndServe() exits regardless of error type while guarding with a sync.Once to prevent double-close panics if Stop() also closes it.

[Copilot]

The shutdown flow is correct. Stop() calls Shutdown() which causes ListenAndServe() to return http.ErrServerClosed, then Stop() closes s.quit safely (lines 165-170) using select to prevent double-close. I've added clarifying comments to document this flow. Fixed in 455fc46.

[Copilot]

The new ServiceName variable is described (in internal/daemon/program.go) as “used for logging” and may differ from the actual Windows service ID/display name. Consider renaming this to something more specific (e.g., LogServiceName or RegistryServiceName) to avoid confusion with the SCM service name, and keep the naming consistent with the Taskfile vars (SVC_LOG_NAME_*).

[Copilot]

Renamed ServiceName → LogServiceName to clarify it's used for log directory naming, not the Windows SCM service name. This aligns with the Taskfile's SVC_LOG_NAME_* variable naming. Fixed in 0ecee0c.

[adcondev]

@copilot apply changes based on the comments in this thread