From 7c974403660a67d952f9a108d80a8ecba88a9c32 Mon Sep 17 00:00:00 2001 From: Joakim Date: Tue, 6 Jan 2026 22:18:09 +0100 Subject: [PATCH] fix: use PersistentPreRun instead of OnInitialize for proper command isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace cobra.OnInitialize with PersistentPreRun pattern to fix initialization issues with setup and server commands. This follows Cobra best practices and allows subcommands to properly override initialization behavior. Problem: - OnInitialize runs globally before command parsing - os.Args check for 'setup' was fragile and broke with flags - Setup wizard failed on server: 'unable to open database file: no such file or directory' Solution: - Use rootCmd.PersistentPreRun for initialization (inherited by all commands) - setup and server commands override with their own PersistentPreRun - Directory overrides still applied correctly in all cases - Removes fragile os.Args parsing Benefits: - Works regardless of flag order - Follows Cobra's intended design patterns - Only 3 files modified (root.go, setup.go, server.go) - Commands that need custom init (setup/server) simply override - All other commands get automatic initialization - Cleaner, more maintainable code Testing: - ✓ opal setup works without initialization errors - ✓ opal list initializes database correctly - ✓ First-run detection still works - ✓ Directory overrides work with flags in any position - ✓ Server command handles its own initialization --- opal-task/cmd/root.go | 10 ++++------ opal-task/cmd/server.go | 10 ++++++++++ opal-task/cmd/setup.go | 19 +++++++++---------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/opal-task/cmd/root.go b/opal-task/cmd/root.go index fb6be23..7551582 100644 --- a/opal-task/cmd/root.go +++ b/opal-task/cmd/root.go @@ -200,7 +200,10 @@ func init() { rootCmd.PersistentFlags().StringVar(&dataDirFlag, "data-dir", "", "Data directory (default: $XDG_DATA_HOME/opal or ~/.local/share/opal)") - cobra.OnInitialize(initializeApp) + // Use PersistentPreRun for initialization (runs for all subcommands unless overridden) + rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + initializeApp() + } // Add regular subcommands rootCmd.AddCommand(addCmd) @@ -224,11 +227,6 @@ func init() { } func initializeApp() { - // Skip initialization for commands that handle their own setup - if len(os.Args) > 1 && os.Args[1] == "setup" { - return - } - // Set directory overrides from flags if provided if configDirFlag != "" { engine.SetConfigDirOverride(configDirFlag) diff --git a/opal-task/cmd/server.go b/opal-task/cmd/server.go index b4ca70e..228df52 100644 --- a/opal-task/cmd/server.go +++ b/opal-task/cmd/server.go @@ -77,6 +77,16 @@ var serverCmd = &cobra.Command{ Use: "server", Short: "Server management commands", Long: `Commands for running and managing the opal API server`, + PersistentPreRun: func(cmd *cobra.Command, args []string) { + // Override root's PersistentPreRun - server handles its own initialization + // But still apply directory overrides from flags + if configDirFlag != "" { + engine.SetConfigDirOverride(configDirFlag) + } + if dataDirFlag != "" { + engine.SetDataDirOverride(dataDirFlag) + } + }, } var serverStartCmd = &cobra.Command{ diff --git a/opal-task/cmd/setup.go b/opal-task/cmd/setup.go index 4c57ffe..e02bf81 100644 --- a/opal-task/cmd/setup.go +++ b/opal-task/cmd/setup.go @@ -36,7 +36,14 @@ Examples: opal setup --show-systemd # Show systemd service template opal setup --show-env # Show environment file template`, PersistentPreRun: func(cmd *cobra.Command, args []string) { - // Skip normal initialization - setup command handles its own init + // Override root's PersistentPreRun to skip initialization + // But still apply directory overrides from flags + if configDirFlag != "" { + engine.SetConfigDirOverride(configDirFlag) + } + if dataDirFlag != "" { + engine.SetDataDirOverride(dataDirFlag) + } }, Run: runSetup, } @@ -51,15 +58,7 @@ func init() { } func runSetup(cmd *cobra.Command, args []string) { - // Apply directory overrides from parent persistent flags - if cmd.Parent() != nil { - if configDir, _ := cmd.Parent().PersistentFlags().GetString("config-dir"); configDir != "" { - engine.SetConfigDirOverride(configDir) - } - if dataDir, _ := cmd.Parent().PersistentFlags().GetString("data-dir"); dataDir != "" { - engine.SetDataDirOverride(dataDir) - } - } + // Directory overrides are already applied in PersistentPreRun // Handle template flags if showSystemdFlag {