refactor: improve RunCommand error semantics to distinguish execution failures from non-zero exits

[harp-intel]

Summary

• Refactors RunCommand and RunCommandEx to return nil error when a command executes successfully but returns a non-zero exit code
• The error return is now reserved for actual execution failures (binary not found, permission denied, etc.)
• Updates all call sites to check err and exitCode separately with distinct error handling

Motivation

Previously, RunCommand returned a non-nil error both when:

1. A command failed to execute (binary not found, permission denied)
2. A command executed successfully but returned a non-zero exit code

This was confusing because many commands use non-zero exit codes for normal conditions:

• grep returns 1 when no matches found
• ps -p returns 1 when process doesn't exist
• which returns 1 when command not found

Changes

Core change in internal/target/helpers.go:

// Before: err was non-nil for non-zero exit codes
// After: err is only non-nil for execution failures
if errors.As(err, &exitError) {
    exitCode = exitError.ExitCode()
    err = nil  // Command executed, just had non-zero exit
}

Updated call sites pattern:

stdout, stderr, exitCode, err := t.RunCommand(cmd)
if err != nil {
    // Actual execution failure
    return fmt.Errorf("failed to execute command: %v", err)
}
if exitCode != 0 {
    // Command ran but returned non-zero
    return fmt.Errorf("command returned exit code %d: %s", exitCode, stderr)
}

[Copilot]

Pull request overview

This PR refactors the RunCommand and RunCommandEx functions to return nil error when a command executes successfully but returns a non-zero exit code, reserving the error return for actual execution failures (e.g., binary not found, permission denied). This change improves error semantics by distinguishing between execution failures and non-zero exits.

Changes:

• Modified core RunCommand/RunCommandEx functions to set err = nil when a command executes but returns a non-zero exit code
• Updated all call sites to check err and exitCode separately with distinct error handling
• Improved error messages to differentiate between execution failures and non-zero exit codes

Reviewed changes

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

Show a summary per file

File	Description
internal/target/helpers.go	Core change: sets err = nil for non-zero exit codes, only returning errors for actual execution failures
internal/workflow/targets.go	Updates call sites to handle execution errors and non-zero exit codes separately
internal/workflow/signals.go	Refactors signal handling to distinguish between execution failures and non-zero exits
internal/target/remote_target.go	Updates remote target operations to check both error and exit code
internal/target/local_target.go	Updates local target privilege checks to verify both error and exit code
internal/script/script.go	Separates execution failure logging from non-zero exit code handling
cmd/metrics/process.go	Improves error messages and separates execution vs exit code checks
cmd/metrics/nmi_watchdog.go	Updates sysctl command calls to check exit codes separately
cmd/metrics/metadata.go	Separates error handling for command execution failures vs non-zero exits