fix: critical issues in wisp hook system
Code review fixes: 1. CRITICAL: Move polecat check to start of runSling - Previously wrote wisp THEN failed, leaving orphan - Now fails fast before any file operations 2. CRITICAL: Sanitize slashes in agent IDs for filenames - Agent IDs like 'gastown/crew/joe' were creating subdirs - Now converts '/' to '--' for safe filenames - Added sanitizeAgentID/unsanitizeAgentID helpers 3. MODERATE: Use git root instead of WorkDir in prime.go - Hooks are written to clone root, not cwd - Added getGitRoot() helper for consistency 4. MODERATE: Fix silent error swallowing - Now logs non-ErrNoHook errors when reading hooks - Warns if bead doesn't exist before burning hook - Preserves hook if bead is missing for debugging
This commit is contained in:
@@ -1169,15 +1169,21 @@ func checkSlungWork(ctx RoleContext) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for hook file in the clone root
|
// Get the git clone root (hooks are stored at clone root, not cwd)
|
||||||
cloneRoot := ctx.WorkDir
|
cloneRoot, err := getGitRoot()
|
||||||
|
if err != nil {
|
||||||
|
// Not in a git repo - can't have hooks
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
sw, err := wisp.ReadHook(cloneRoot, agentID)
|
sw, err := wisp.ReadHook(cloneRoot, agentID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if errors.Is(err, wisp.ErrNoHook) {
|
if errors.Is(err, wisp.ErrNoHook) {
|
||||||
// No hook - normal case, nothing to do
|
// No hook - normal case, nothing to do
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
// Other error - log but continue
|
// Log other errors (permission, corruption) but continue
|
||||||
|
fmt.Printf("%s Warning: error reading hook: %v\n", style.Dim.Render("⚠"), err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1196,14 +1202,15 @@ func checkSlungWork(ctx RoleContext) {
|
|||||||
fmt.Printf(" Slung at: %s\n", sw.CreatedAt.Format("2006-01-02 15:04:05"))
|
fmt.Printf(" Slung at: %s\n", sw.CreatedAt.Format("2006-01-02 15:04:05"))
|
||||||
fmt.Println()
|
fmt.Println()
|
||||||
|
|
||||||
// Show the bead details
|
// Show the bead details - verify it exists
|
||||||
fmt.Println("**Bead details:**")
|
fmt.Println("**Bead details:**")
|
||||||
cmd := exec.Command("bd", "show", sw.BeadID)
|
cmd := exec.Command("bd", "show", sw.BeadID)
|
||||||
cmd.Dir = cloneRoot
|
cmd.Dir = cloneRoot
|
||||||
var stdout bytes.Buffer
|
var stdout bytes.Buffer
|
||||||
cmd.Stdout = &stdout
|
cmd.Stdout = &stdout
|
||||||
cmd.Stderr = nil
|
cmd.Stderr = nil
|
||||||
if cmd.Run() == nil {
|
beadExists := cmd.Run() == nil
|
||||||
|
if beadExists {
|
||||||
// Show first 20 lines of bead details
|
// Show first 20 lines of bead details
|
||||||
lines := strings.Split(stdout.String(), "\n")
|
lines := strings.Split(stdout.String(), "\n")
|
||||||
maxLines := 20
|
maxLines := 20
|
||||||
@@ -1214,6 +1221,13 @@ func checkSlungWork(ctx RoleContext) {
|
|||||||
for _, line := range lines {
|
for _, line := range lines {
|
||||||
fmt.Printf(" %s\n", line)
|
fmt.Printf(" %s\n", line)
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
fmt.Printf(" %s Bead %s not found! It may have been deleted.\n",
|
||||||
|
style.Bold.Render("⚠ WARNING:"), sw.BeadID)
|
||||||
|
fmt.Println(" The hook will NOT be burned. Investigate this issue.")
|
||||||
|
fmt.Println()
|
||||||
|
// Don't burn - leave hook for debugging
|
||||||
|
return
|
||||||
}
|
}
|
||||||
fmt.Println()
|
fmt.Println()
|
||||||
|
|
||||||
@@ -1222,12 +1236,22 @@ func checkSlungWork(ctx RoleContext) {
|
|||||||
fmt.Println(" Begin working on this bead immediately. No human input needed.")
|
fmt.Println(" Begin working on this bead immediately. No human input needed.")
|
||||||
fmt.Println()
|
fmt.Println()
|
||||||
|
|
||||||
// Burn the hook now that it's been read
|
// Burn the hook now that it's been read and verified
|
||||||
if err := wisp.BurnHook(cloneRoot, agentID); err != nil {
|
if err := wisp.BurnHook(cloneRoot, agentID); err != nil {
|
||||||
fmt.Printf("%s Warning: could not burn hook: %v\n", style.Dim.Render("⚠"), err)
|
fmt.Printf("%s Warning: could not burn hook: %v\n", style.Dim.Render("⚠"), err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// getGitRoot returns the root of the current git repository.
|
||||||
|
func getGitRoot() (string, error) {
|
||||||
|
cmd := exec.Command("git", "rev-parse", "--show-toplevel")
|
||||||
|
out, err := cmd.Output()
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return strings.TrimSpace(string(out)), nil
|
||||||
|
}
|
||||||
|
|
||||||
// getAgentIdentity returns the agent identity string for hook lookup.
|
// getAgentIdentity returns the agent identity string for hook lookup.
|
||||||
func getAgentIdentity(ctx RoleContext) string {
|
func getAgentIdentity(ctx RoleContext) string {
|
||||||
switch ctx.Role {
|
switch ctx.Role {
|
||||||
|
|||||||
@@ -50,6 +50,11 @@ func init() {
|
|||||||
func runSling(cmd *cobra.Command, args []string) error {
|
func runSling(cmd *cobra.Command, args []string) error {
|
||||||
beadID := args[0]
|
beadID := args[0]
|
||||||
|
|
||||||
|
// Polecats cannot sling - check early before writing anything
|
||||||
|
if polecatName := os.Getenv("GT_POLECAT"); polecatName != "" {
|
||||||
|
return fmt.Errorf("polecats cannot sling (use gt done for handoff)")
|
||||||
|
}
|
||||||
|
|
||||||
// Verify the bead exists
|
// Verify the bead exists
|
||||||
if err := verifyBeadExists(beadID); err != nil {
|
if err := verifyBeadExists(beadID); err != nil {
|
||||||
return err
|
return err
|
||||||
@@ -166,13 +171,6 @@ func detectCloneRoot() (string, error) {
|
|||||||
|
|
||||||
// triggerHandoff restarts the agent session.
|
// triggerHandoff restarts the agent session.
|
||||||
func triggerHandoff(agentID, beadID string) error {
|
func triggerHandoff(agentID, beadID string) error {
|
||||||
// Check if we're a polecat
|
|
||||||
if polecatName := os.Getenv("GT_POLECAT"); polecatName != "" {
|
|
||||||
fmt.Printf("%s Polecat detected - cannot sling (use gt done instead)\n",
|
|
||||||
style.Bold.Render("⚠"))
|
|
||||||
return fmt.Errorf("polecats cannot sling - use gt done for handoff")
|
|
||||||
}
|
|
||||||
|
|
||||||
// Must be in tmux
|
// Must be in tmux
|
||||||
if !tmux.IsInsideTmux() {
|
if !tmux.IsInsideTmux() {
|
||||||
return fmt.Errorf("not running in tmux - cannot restart")
|
return fmt.Errorf("not running in tmux - cannot restart")
|
||||||
|
|||||||
@@ -136,6 +136,7 @@ func HasHook(root, agent string) bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ListHooks returns a list of agents with active hooks.
|
// ListHooks returns a list of agents with active hooks.
|
||||||
|
// Agent IDs are returned in their original form (with slashes).
|
||||||
func ListHooks(root string) ([]string, error) {
|
func ListHooks(root string) ([]string, error) {
|
||||||
dir := filepath.Join(root, WispDir)
|
dir := filepath.Join(root, WispDir)
|
||||||
entries, err := os.ReadDir(dir)
|
entries, err := os.ReadDir(dir)
|
||||||
@@ -152,7 +153,8 @@ func ListHooks(root string) ([]string, error) {
|
|||||||
if len(name) > len(HookPrefix)+len(HookSuffix) &&
|
if len(name) > len(HookPrefix)+len(HookSuffix) &&
|
||||||
name[:len(HookPrefix)] == HookPrefix &&
|
name[:len(HookPrefix)] == HookPrefix &&
|
||||||
name[len(name)-len(HookSuffix):] == HookSuffix {
|
name[len(name)-len(HookSuffix):] == HookSuffix {
|
||||||
agent := name[len(HookPrefix) : len(name)-len(HookSuffix)]
|
sanitized := name[len(HookPrefix) : len(name)-len(HookSuffix)]
|
||||||
|
agent := unsanitizeAgentID(sanitized)
|
||||||
agents = append(agents, agent)
|
agents = append(agents, agent)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,6 +9,7 @@
|
|||||||
package wisp
|
package wisp
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -126,6 +127,23 @@ func NewPatrolCycle(formula, createdBy string) *PatrolCycle {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// HookFilename returns the filename for an agent's hook file.
|
// HookFilename returns the filename for an agent's hook file.
|
||||||
|
// Agent IDs containing slashes (e.g., "gastown/crew/joe") are sanitized
|
||||||
|
// by replacing "/" with "--" to create valid filenames.
|
||||||
func HookFilename(agent string) string {
|
func HookFilename(agent string) string {
|
||||||
return HookPrefix + agent + HookSuffix
|
// Sanitize agent ID: replace path separators with double-dash
|
||||||
|
// This is reversible and avoids creating subdirectories
|
||||||
|
sanitized := sanitizeAgentID(agent)
|
||||||
|
return HookPrefix + sanitized + HookSuffix
|
||||||
|
}
|
||||||
|
|
||||||
|
// sanitizeAgentID converts an agent ID to a safe filename component.
|
||||||
|
// "gastown/crew/joe" -> "gastown--crew--joe"
|
||||||
|
func sanitizeAgentID(agent string) string {
|
||||||
|
return strings.ReplaceAll(agent, "/", "--")
|
||||||
|
}
|
||||||
|
|
||||||
|
// unsanitizeAgentID converts a sanitized filename back to an agent ID.
|
||||||
|
// "gastown--crew--joe" -> "gastown/crew/joe"
|
||||||
|
func unsanitizeAgentID(sanitized string) string {
|
||||||
|
return strings.ReplaceAll(sanitized, "--", "/")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user