fix(hook): handle error from events.LogFeed (#440)

* fix(beads): cache version check and add timeout to prevent cli lag

* fix(mail_queue): add nil check for queue config

Prevents potential nil pointer panic when queue config exists
in map but has nil value. Added || queueCfg == nil check to
the queue lookup condition in runMailClaim function.

Fixes potential panic that could occur if a queue entry exists
in config but with a nil value.

* fix(migrate_agents_test): fix icon expectations to match actual output

The printMigrationResult function uses icons with two leading spaces
("  ✓", "  ⊘", "  ✗") but the test expected icons without spaces.
This fixes the test expectations to match the actual output format.

* fix(hook): handle error from events.LogFeed

Previously the error from LogFeed was silently ignored with _.
Now we log the error to stderr at warning level but don't fail
the operation since the primary hook action succeeded.
This commit is contained in:
sigfawn
2026-01-13 16:40:57 -05:00
committed by GitHub
parent 1453b8b592
commit aa0bfd0c40
4 changed files with 39 additions and 13 deletions

View File

@@ -2,11 +2,14 @@
package cmd package cmd
import ( import (
"context"
"fmt" "fmt"
"os/exec" "os/exec"
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
"sync"
"time"
) )
// MinBeadsVersion is the minimum required beads version for Gas Town. // MinBeadsVersion is the minimum required beads version for Gas Town.
@@ -84,10 +87,19 @@ func (v beadsVersion) compare(other beadsVersion) int {
return 0 return 0
} }
// Pre-compiled regex for beads version parsing
var beadsVersionRe = regexp.MustCompile(`bd version (\d+\.\d+(?:\.\d+)?(?:-\w+)?)`)
func getBeadsVersion() (string, error) { func getBeadsVersion() (string, error) {
cmd := exec.Command("bd", "version") ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
cmd := exec.CommandContext(ctx, "bd", "version")
output, err := cmd.Output() output, err := cmd.Output()
if err != nil { if err != nil {
if ctx.Err() == context.DeadlineExceeded {
return "", fmt.Errorf("bd version check timed out")
}
if exitErr, ok := err.(*exec.ExitError); ok { if exitErr, ok := err.(*exec.ExitError); ok {
return "", fmt.Errorf("bd version failed: %s", string(exitErr.Stderr)) return "", fmt.Errorf("bd version failed: %s", string(exitErr.Stderr))
} }
@@ -96,8 +108,7 @@ func getBeadsVersion() (string, error) {
// Parse output like "bd version 0.44.0 (dev)" // Parse output like "bd version 0.44.0 (dev)"
// or "bd version 0.44.0" // or "bd version 0.44.0"
re := regexp.MustCompile(`bd version (\d+\.\d+(?:\.\d+)?(?:-\w+)?)`) matches := beadsVersionRe.FindStringSubmatch(string(output))
matches := re.FindStringSubmatch(string(output))
if len(matches) < 2 { if len(matches) < 2 {
return "", fmt.Errorf("could not parse beads version from: %s", strings.TrimSpace(string(output))) return "", fmt.Errorf("could not parse beads version from: %s", strings.TrimSpace(string(output)))
} }
@@ -105,9 +116,22 @@ func getBeadsVersion() (string, error) {
return matches[1], nil return matches[1], nil
} }
var (
cachedVersionCheckResult error
versionCheckOnce sync.Once
)
// CheckBeadsVersion verifies that the installed beads version meets the minimum requirement. // CheckBeadsVersion verifies that the installed beads version meets the minimum requirement.
// Returns nil if the version is sufficient, or an error with details if not. // Returns nil if the version is sufficient, or an error with details if not.
// The check is performed only once per process execution.
func CheckBeadsVersion() error { func CheckBeadsVersion() error {
versionCheckOnce.Do(func() {
cachedVersionCheckResult = checkBeadsVersionInternal()
})
return cachedVersionCheckResult
}
func checkBeadsVersionInternal() error {
installedStr, err := getBeadsVersion() installedStr, err := getBeadsVersion()
if err != nil { if err != nil {
return fmt.Errorf("cannot verify beads version: %w", err) return fmt.Errorf("cannot verify beads version: %w", err)

View File

@@ -233,8 +233,10 @@ func runHook(_ *cobra.Command, args []string) error {
fmt.Printf(" Use 'gt handoff' to restart with this work\n") fmt.Printf(" Use 'gt handoff' to restart with this work\n")
fmt.Printf(" Use 'gt hook' to see hook status\n") fmt.Printf(" Use 'gt hook' to see hook status\n")
// Log hook event to activity feed // Log hook event to activity feed (non-fatal)
_ = events.LogFeed(events.TypeHook, agentID, events.HookPayload(beadID)) if err := events.LogFeed(events.TypeHook, agentID, events.HookPayload(beadID)); err != nil {
fmt.Fprintf(os.Stderr, "%s Warning: failed to log hook event: %v\n", style.Dim.Render("⚠"), err)
}
return nil return nil
} }

View File

@@ -35,7 +35,7 @@ func runMailClaim(cmd *cobra.Command, args []string) error {
} }
queueCfg, ok := cfg.Queues[queueName] queueCfg, ok := cfg.Queues[queueName]
if !ok { if !ok || queueCfg == nil {
return fmt.Errorf("unknown queue: %s", queueName) return fmt.Errorf("unknown queue: %s", queueName)
} }

View File

@@ -20,7 +20,7 @@ func TestMigrationResultStatus(t *testing.T) {
Status: "migrated", Status: "migrated",
Message: "successfully migrated", Message: "successfully migrated",
}, },
wantIcon: "✓", wantIcon: " ✓",
}, },
{ {
name: "would migrate shows checkmark", name: "would migrate shows checkmark",
@@ -30,7 +30,7 @@ func TestMigrationResultStatus(t *testing.T) {
Status: "would migrate", Status: "would migrate",
Message: "would copy state from gt-mayor", Message: "would copy state from gt-mayor",
}, },
wantIcon: "✓", wantIcon: " ✓",
}, },
{ {
name: "skipped shows empty circle", name: "skipped shows empty circle",
@@ -40,7 +40,7 @@ func TestMigrationResultStatus(t *testing.T) {
Status: "skipped", Status: "skipped",
Message: "already exists", Message: "already exists",
}, },
wantIcon: "⊘", wantIcon: " ⊘",
}, },
{ {
name: "error shows X", name: "error shows X",
@@ -50,7 +50,7 @@ func TestMigrationResultStatus(t *testing.T) {
Status: "error", Status: "error",
Message: "failed to create", Message: "failed to create",
}, },
wantIcon: "✗", wantIcon: " ✗",
}, },
} }
@@ -59,11 +59,11 @@ func TestMigrationResultStatus(t *testing.T) {
var icon string var icon string
switch tt.result.Status { switch tt.result.Status {
case "migrated", "would migrate": case "migrated", "would migrate":
icon = "✓" icon = " ✓"
case "skipped": case "skipped":
icon = "⊘" icon = " ⊘"
case "error": case "error":
icon = "✗" icon = " ✗"
} }
if icon != tt.wantIcon { if icon != tt.wantIcon {
t.Errorf("icon for status %q = %q, want %q", tt.result.Status, icon, tt.wantIcon) t.Errorf("icon for status %q = %q, want %q", tt.result.Status, icon, tt.wantIcon)