fix(doctor): recognize gt prime --hook as valid session hook config (#14)
The SessionHookCheck was incorrectly flagging 'gt prime --hook' as invalid, only accepting 'session-start.sh' wrapper. The --hook flag properly handles session_id passthrough via stdin JSON, making it a valid alternative. Changes: - Update usesSessionStartScript to accept --hook flag - Add containsFlag helper to prevent false positives (e.g., --hookup) - Update error messages and fix hints to suggest both options - Add comprehensive tests including edge cases Tests cover: - Bare gt prime (fails) - gt prime --hook (passes) - gt prime --hookup (fails - not a valid flag) - gt prime --verbose --hook (passes - flag order doesn't matter) - session-start.sh (passes) - Mixed valid/invalid hooks in same file - Town-level and rig-level settings
This commit is contained in:
committed by
Steve Yegge
parent
c8c765a239
commit
b3407759d2
@@ -267,8 +267,9 @@ func (c *SettingsCheck) findRigs(townRoot string) []string {
|
|||||||
return findAllRigs(townRoot)
|
return findAllRigs(townRoot)
|
||||||
}
|
}
|
||||||
|
|
||||||
// SessionHookCheck verifies settings.json files use session-start.sh for proper
|
// SessionHookCheck verifies settings.json files use proper session_id passthrough.
|
||||||
// session_id passthrough. Without this wrapper, gt seance cannot discover sessions.
|
// Valid options: session-start.sh wrapper OR 'gt prime --hook'.
|
||||||
|
// Without proper config, gt seance cannot discover sessions.
|
||||||
type SessionHookCheck struct {
|
type SessionHookCheck struct {
|
||||||
BaseCheck
|
BaseCheck
|
||||||
}
|
}
|
||||||
@@ -278,12 +279,12 @@ func NewSessionHookCheck() *SessionHookCheck {
|
|||||||
return &SessionHookCheck{
|
return &SessionHookCheck{
|
||||||
BaseCheck: BaseCheck{
|
BaseCheck: BaseCheck{
|
||||||
CheckName: "session-hooks",
|
CheckName: "session-hooks",
|
||||||
CheckDescription: "Check that settings.json hooks use session-start.sh",
|
CheckDescription: "Check that settings.json hooks use session-start.sh or --hook flag",
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run checks if all settings.json files use session-start.sh wrapper.
|
// Run checks if all settings.json files use session-start.sh or --hook flag.
|
||||||
func (c *SessionHookCheck) Run(ctx *CheckContext) *CheckResult {
|
func (c *SessionHookCheck) Run(ctx *CheckContext) *CheckResult {
|
||||||
var issues []string
|
var issues []string
|
||||||
var checked int
|
var checked int
|
||||||
@@ -307,7 +308,7 @@ func (c *SessionHookCheck) Run(ctx *CheckContext) *CheckResult {
|
|||||||
return &CheckResult{
|
return &CheckResult{
|
||||||
Name: c.Name(),
|
Name: c.Name(),
|
||||||
Status: StatusOK,
|
Status: StatusOK,
|
||||||
Message: fmt.Sprintf("All %d settings.json file(s) use session-start.sh", checked),
|
Message: fmt.Sprintf("All %d settings.json file(s) use proper session_id passthrough", checked),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -316,7 +317,7 @@ func (c *SessionHookCheck) Run(ctx *CheckContext) *CheckResult {
|
|||||||
Status: StatusWarning,
|
Status: StatusWarning,
|
||||||
Message: fmt.Sprintf("%d hook issue(s) found across settings.json files", len(issues)),
|
Message: fmt.Sprintf("%d hook issue(s) found across settings.json files", len(issues)),
|
||||||
Details: issues,
|
Details: issues,
|
||||||
FixHint: "Update SessionStart/PreCompact hooks to use 'bash ~/.claude/hooks/session-start.sh' for session_id passthrough",
|
FixHint: "Update hooks to use 'gt prime --hook' or 'bash ~/.claude/hooks/session-start.sh' for session_id passthrough",
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -334,22 +335,22 @@ func (c *SessionHookCheck) checkSettingsFile(path string) []string {
|
|||||||
// Check for SessionStart hooks
|
// Check for SessionStart hooks
|
||||||
if strings.Contains(content, "SessionStart") {
|
if strings.Contains(content, "SessionStart") {
|
||||||
if !c.usesSessionStartScript(content, "SessionStart") {
|
if !c.usesSessionStartScript(content, "SessionStart") {
|
||||||
problems = append(problems, "SessionStart uses bare 'gt prime' (missing session_id passthrough)")
|
problems = append(problems, "SessionStart uses bare 'gt prime' - add --hook flag or use session-start.sh")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for PreCompact hooks
|
// Check for PreCompact hooks
|
||||||
if strings.Contains(content, "PreCompact") {
|
if strings.Contains(content, "PreCompact") {
|
||||||
if !c.usesSessionStartScript(content, "PreCompact") {
|
if !c.usesSessionStartScript(content, "PreCompact") {
|
||||||
problems = append(problems, "PreCompact uses bare 'gt prime' (missing session_id passthrough)")
|
problems = append(problems, "PreCompact uses bare 'gt prime' - add --hook flag or use session-start.sh")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return problems
|
return problems
|
||||||
}
|
}
|
||||||
|
|
||||||
// usesSessionStartScript checks if the hook configuration uses session-start.sh.
|
// usesSessionStartScript checks if the hook configuration handles session_id properly.
|
||||||
// Returns true if the hook is properly configured or if no hook is configured.
|
// Valid: session-start.sh wrapper OR 'gt prime --hook'. Returns true if properly configured.
|
||||||
func (c *SessionHookCheck) usesSessionStartScript(content, hookType string) bool {
|
func (c *SessionHookCheck) usesSessionStartScript(content, hookType string) bool {
|
||||||
// Find the hook section - look for the hook type followed by its configuration
|
// Find the hook section - look for the hook type followed by its configuration
|
||||||
// This is a simple heuristic - we look for "gt prime" without session-start.sh
|
// This is a simple heuristic - we look for "gt prime" without session-start.sh
|
||||||
@@ -382,10 +383,15 @@ func (c *SessionHookCheck) usesSessionStartScript(content, hookType string) bool
|
|||||||
return true // Uses the wrapper script
|
return true // Uses the wrapper script
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if it uses bare 'gt prime' without the wrapper
|
// Check if it uses 'gt prime --hook' which handles session_id via stdin
|
||||||
// Patterns to detect: "gt prime", "'gt prime'", "gt prime\""
|
|
||||||
if strings.Contains(section, "gt prime") {
|
if strings.Contains(section, "gt prime") {
|
||||||
return false // Uses bare gt prime without session-start.sh
|
// gt prime --hook is valid - it reads session_id from stdin JSON
|
||||||
|
// Must match --hook as complete flag, not substring (e.g., --hookup)
|
||||||
|
if containsFlag(section, "--hook") {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
// Bare 'gt prime' without --hook doesn't get session_id
|
||||||
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
// No gt prime or session-start.sh found - might be a different hook configuration
|
// No gt prime or session-start.sh found - might be a different hook configuration
|
||||||
@@ -504,3 +510,16 @@ func findAllRigs(townRoot string) []string {
|
|||||||
|
|
||||||
return rigs
|
return rigs
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func containsFlag(s, flag string) bool {
|
||||||
|
idx := strings.Index(s, flag)
|
||||||
|
if idx == -1 {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
end := idx + len(flag)
|
||||||
|
if end >= len(s) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
next := s[end]
|
||||||
|
return next == '"' || next == ' ' || next == '\'' || next == '\n' || next == '\t'
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,226 @@
|
|||||||
|
package doctor
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestSessionHookCheck_UsesSessionStartScript(t *testing.T) {
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
content string
|
||||||
|
hookType string
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "bare gt prime fails",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "gt prime --hook passes",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --hook"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "session-start.sh passes",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "bash ~/.claude/hooks/session-start.sh"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "no SessionStart hook passes",
|
||||||
|
content: `{"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "gt handoff"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "PreCompact with --hook passes",
|
||||||
|
content: `{"hooks": {"PreCompact": [{"hooks": [{"type": "command", "command": "gt prime --hook"}]}]}}`,
|
||||||
|
hookType: "PreCompact",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "PreCompact bare gt prime fails",
|
||||||
|
content: `{"hooks": {"PreCompact": [{"hooks": [{"type": "command", "command": "gt prime"}]}]}}`,
|
||||||
|
hookType: "PreCompact",
|
||||||
|
want: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "gt prime --hook with extra flags passes",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --hook --verbose"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "gt prime with --hook not first still passes",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --verbose --hook"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "gt prime with other flags but no --hook fails",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --verbose"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "both session-start.sh and gt prime passes (session-start.sh wins)",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "bash session-start.sh && gt prime"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "gt prime --hookup is NOT valid (false positive check)",
|
||||||
|
content: `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --hookup"}]}]}}`,
|
||||||
|
hookType: "SessionStart",
|
||||||
|
want: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
got := check.usesSessionStartScript(tt.content, tt.hookType)
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("usesSessionStartScript() = %v, want %v", got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSessionHookCheck_Run(t *testing.T) {
|
||||||
|
t.Run("bare gt prime warns", func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
claudeDir := filepath.Join(tmpDir, ".claude")
|
||||||
|
if err := os.MkdirAll(claudeDir, 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
settings := `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime"}]}]}}`
|
||||||
|
if err := os.WriteFile(filepath.Join(claudeDir, "settings.json"), []byte(settings), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
ctx := &CheckContext{TownRoot: tmpDir}
|
||||||
|
result := check.Run(ctx)
|
||||||
|
|
||||||
|
if result.Status != StatusWarning {
|
||||||
|
t.Errorf("expected StatusWarning, got %v", result.Status)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("gt prime --hook passes", func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
claudeDir := filepath.Join(tmpDir, ".claude")
|
||||||
|
if err := os.MkdirAll(claudeDir, 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
settings := `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --hook"}]}]}}`
|
||||||
|
if err := os.WriteFile(filepath.Join(claudeDir, "settings.json"), []byte(settings), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
ctx := &CheckContext{TownRoot: tmpDir}
|
||||||
|
result := check.Run(ctx)
|
||||||
|
|
||||||
|
if result.Status != StatusOK {
|
||||||
|
t.Errorf("expected StatusOK, got %v: %v", result.Status, result.Details)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("rig-level settings with --hook passes", func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
rigDir := filepath.Join(tmpDir, "myrig")
|
||||||
|
if err := os.MkdirAll(filepath.Join(rigDir, "crew"), 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
claudeDir := filepath.Join(rigDir, ".claude")
|
||||||
|
if err := os.MkdirAll(claudeDir, 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
settings := `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --hook"}]}]}}`
|
||||||
|
if err := os.WriteFile(filepath.Join(claudeDir, "settings.json"), []byte(settings), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
ctx := &CheckContext{TownRoot: tmpDir}
|
||||||
|
result := check.Run(ctx)
|
||||||
|
|
||||||
|
if result.Status != StatusOK {
|
||||||
|
t.Errorf("expected StatusOK for rig-level settings, got %v: %v", result.Status, result.Details)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("rig-level bare gt prime warns", func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
rigDir := filepath.Join(tmpDir, "myrig")
|
||||||
|
if err := os.MkdirAll(filepath.Join(rigDir, "polecats"), 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
claudeDir := filepath.Join(rigDir, ".claude")
|
||||||
|
if err := os.MkdirAll(claudeDir, 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
settings := `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime"}]}]}}`
|
||||||
|
if err := os.WriteFile(filepath.Join(claudeDir, "settings.json"), []byte(settings), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
ctx := &CheckContext{TownRoot: tmpDir}
|
||||||
|
result := check.Run(ctx)
|
||||||
|
|
||||||
|
if result.Status != StatusWarning {
|
||||||
|
t.Errorf("expected StatusWarning for rig-level bare gt prime, got %v", result.Status)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("mixed valid and invalid hooks warns", func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
claudeDir := filepath.Join(tmpDir, ".claude")
|
||||||
|
if err := os.MkdirAll(claudeDir, 0755); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
settings := `{"hooks": {"SessionStart": [{"hooks": [{"type": "command", "command": "gt prime --hook"}]}], "PreCompact": [{"hooks": [{"type": "command", "command": "gt prime"}]}]}}`
|
||||||
|
if err := os.WriteFile(filepath.Join(claudeDir, "settings.json"), []byte(settings), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
ctx := &CheckContext{TownRoot: tmpDir}
|
||||||
|
result := check.Run(ctx)
|
||||||
|
|
||||||
|
if result.Status != StatusWarning {
|
||||||
|
t.Errorf("expected StatusWarning when PreCompact is invalid, got %v", result.Status)
|
||||||
|
}
|
||||||
|
if len(result.Details) != 1 {
|
||||||
|
t.Errorf("expected 1 issue (PreCompact), got %d: %v", len(result.Details), result.Details)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("no settings files returns OK", func(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
|
||||||
|
check := NewSessionHookCheck()
|
||||||
|
ctx := &CheckContext{TownRoot: tmpDir}
|
||||||
|
result := check.Run(ctx)
|
||||||
|
|
||||||
|
if result.Status != StatusOK {
|
||||||
|
t.Errorf("expected StatusOK when no settings files, got %v", result.Status)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user