fix: CodeRabbit review — 7 findings resolved

- cli: preserve explicit empty flag values (--name=)
- cli: skip placeholder commands in help output
- command: fail fast on non-executable placeholder Run
- command: lifecycle-backed commands count as registered
- runtime: wrap non-error OnStop payloads in error
- fs: error on protected path deletion (was silent Result{})
- error: log crash report I/O failures instead of swallowing

Co-Authored-By: Virgil <virgil@lethean.io>

pkg/core/cli.go

@@ -96,7 +96,7 @@ func (cl *Cli) Run(args ...string) Result {
 	for _, arg := range remaining {
 		key, val, valid := ParseFlag(arg)
 		if valid {
-			if val != "" {
+			if Contains(arg, "=") {
 				opts = append(opts, Option{Key: key, Value: val})
 			} else {
 				opts = append(opts, Option{Key: key, Value: true})
@@ -131,7 +131,7 @@ func (cl *Cli) PrintHelp() {
 	defer cl.core.commands.mu.RUnlock()
 
 	for path, cmd := range cl.core.commands.commands {
-		if cmd.Hidden {
+		if cmd.Hidden || (cmd.Action == nil && cmd.Lifecycle == nil) {
 			continue
 		}
 		tr := cl.core.I18n().Translate(cmd.I18nKey())

pkg/core/command.go

@@ -72,7 +72,7 @@ func (cmd *Command) I18nKey() string {
 //	result := cmd.Run(core.Options{{Key: "target", Value: "homelab"}})
 func (cmd *Command) Run(opts Options) Result {
 	if cmd.Action == nil {
-		return Result{}
+		return Result{E("core.Command.Run", Concat("command \"", cmd.Path, "\" is not executable"), nil), false}
 	}
 	return cmd.Action(opts)
 }
@@ -144,7 +144,7 @@ func (c *Core) Command(path string, command ...Command) Result {
 	c.commands.mu.Lock()
 	defer c.commands.mu.Unlock()
 
-	if existing, exists := c.commands.commands[path]; exists && existing.Action != nil {
+	if existing, exists := c.commands.commands[path]; exists && (existing.Action != nil || existing.Lifecycle != nil) {
 		return Result{E("core.Command", Concat("command \"", path, "\" already registered"), nil), false}
 	}
 

pkg/core/error.go

@@ -380,8 +380,16 @@ func (h *ErrorPanic) appendReport(report CrashReport) {
 	}
 
 	reports = append(reports, report)
-	if data, err := json.MarshalIndent(reports, "", "  "); err == nil {
-		_ = os.MkdirAll(filepath.Dir(h.filePath), 0755)
-		_ = os.WriteFile(h.filePath, data, 0600)
+	data, err := json.MarshalIndent(reports, "", "  ")
+	if err != nil {
+		Default().Error(Concat("crash report marshal failed: ", err.Error()))
+		return
+	}
+	if err := os.MkdirAll(filepath.Dir(h.filePath), 0755); err != nil {
+		Default().Error(Concat("crash report dir failed: ", err.Error()))
+		return
+	}
+	if err := os.WriteFile(h.filePath, data, 0600); err != nil {
+		Default().Error(Concat("crash report write failed: ", err.Error()))
 	}
 }

pkg/core/fs.go

@@ -246,7 +246,7 @@ func (m *Fs) Delete(p string) Result {
 	}
 	full := vp.Value.(string)
 	if full == "/" || full == os.Getenv("HOME") {
-		return Result{}
+		return Result{E("fs.Delete", Concat("refusing to delete protected path: ", full), nil), false}
 	}
 	if err := os.Remove(full); err != nil {
 		return Result{err, false}
@@ -262,7 +262,7 @@ func (m *Fs) DeleteAll(p string) Result {
 	}
 	full := vp.Value.(string)
 	if full == "/" || full == os.Getenv("HOME") {
-		return Result{}
+		return Result{E("fs.DeleteAll", Concat("refusing to delete protected path: ", full), nil), false}
 	}
 	if err := os.RemoveAll(full); err != nil {
 		return Result{err, false}

pkg/core/runtime.go

@@ -81,6 +81,8 @@ func (c *Core) ServiceShutdown(ctx context.Context) Result {
 			if !r.OK && firstErr == nil {
 				if e, ok := r.Value.(error); ok {
 					firstErr = e
+				} else {
+					firstErr = E("core.ServiceShutdown", Sprint("service OnStop failed: ", r.Value), nil)
 				}
 			}
 		}