From c624f4fbb008e797a46c6e4846b50ae3a9596b6c Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 17 Jun 2026 15:36:05 +0530 Subject: [PATCH 1/3] fix(permissions): rename cosmenticFlags to cosmeticFlags (L1) --- internal/permissions/canonicalize.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/permissions/canonicalize.go b/internal/permissions/canonicalize.go index 4dc70b8f..4a0602a4 100644 --- a/internal/permissions/canonicalize.go +++ b/internal/permissions/canonicalize.go @@ -31,7 +31,7 @@ var BannedPrefixes = []string{ } // cosmetic flags that don't affect safety -var cosmenticFlags = map[string]bool{ +var cosmeticFlags = map[string]bool{ "--color": true, "--no-color": true, "-v": true, @@ -142,7 +142,7 @@ func (c *Canonicalizer) canonicalizeSingle(cmd string) string { // Strip cosmetic flags var filtered []string for _, tok := range tokens { - if !cosmenticFlags[tok] { + if !cosmeticFlags[tok] { filtered = append(filtered, tok) } } @@ -324,7 +324,7 @@ func (c *Canonicalizer) GeneratePattern(command string) string { tok := tokens[argIdx] if strings.HasPrefix(tok, "-") { // Skip cosmetic flags from the pattern - if !cosmenticFlags[tok] { + if !cosmeticFlags[tok] { prefix = append(prefix, tok) } argIdx++ From 936d2d36a61fac95eb0ffde6cea32edf17f2ae99 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 17 Jun 2026 15:38:20 +0530 Subject: [PATCH 2/3] fix(engine): use comma-ok form in formatNode to avoid panic on non-Expr nodes (H10) --- internal/engine/semantic_diff.go | 5 ++++- internal/engine/semantic_diff_test.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/engine/semantic_diff.go b/internal/engine/semantic_diff.go index 0cd9ff43..2a092a25 100644 --- a/internal/engine/semantic_diff.go +++ b/internal/engine/semantic_diff.go @@ -967,7 +967,10 @@ func formatNode(fset *token.FileSet, node ast.Node) string { case *ast.Ident: return t.Name default: - return formatFieldType(node.(ast.Expr)) //nolint:errcheck + if expr, ok := node.(ast.Expr); ok { + return formatFieldType(expr) + } + return "unknown" } } diff --git a/internal/engine/semantic_diff_test.go b/internal/engine/semantic_diff_test.go index de5d4089..a301692a 100644 --- a/internal/engine/semantic_diff_test.go +++ b/internal/engine/semantic_diff_test.go @@ -1,6 +1,7 @@ package engine import ( + "go/ast" "strings" "testing" ) @@ -691,3 +692,20 @@ func TestGenerateSummaryNoAPIs(t *testing.T) { t.Error("should not contain Affected APIs when there are none") } } + +// TestFormatNodeNonExprRegression guards H10 — a non-ast.Expr node (e.g. *ast.Comment) +// must not panic; the comma-ok form should fall through to "unknown". +func TestFormatNodeNonExprRegression(t *testing.T) { + // *ast.Comment is ast.Node but not ast.Expr. Pre-fix this panicked + // with "interface conversion: *ast.Comment is not ast.Expr". + defer func() { + if r := recover(); r != nil { + t.Fatalf("formatNode panicked on non-Expr node: %v", r) + } + }() + + got := formatNode(nil, &ast.Comment{Text: "x"}) + if got != "unknown" { + t.Errorf("formatNode(*ast.Comment) = %q, want %q", got, "unknown") + } +} From 6cbb8fa8f7ef6b0b79e71b7b0343d32f01b96841 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 17 Jun 2026 15:44:57 +0530 Subject: [PATCH 3/3] fix(state): resolve .hawk/ paths to home dir to stop cwd-leak (L2) --- internal/engine/integration.go | 13 +++++-- internal/engine/l2_home_paths_test.go | 51 +++++++++++++++++++++++++ internal/snapshot/l2_home_paths_test.go | 50 ++++++++++++++++++++++++ internal/snapshot/snapshot.go | 7 +++- internal/snapshot/snapshot_test.go | 4 +- internal/snapshot/workspace.go | 8 +++- internal/snapshot/workspace_test.go | 9 ++++- 7 files changed, 133 insertions(+), 9 deletions(-) create mode 100644 internal/engine/l2_home_paths_test.go create mode 100644 internal/snapshot/l2_home_paths_test.go diff --git a/internal/engine/integration.go b/internal/engine/integration.go index a7e05fc9..a5966edc 100644 --- a/internal/engine/integration.go +++ b/internal/engine/integration.go @@ -2,11 +2,13 @@ package engine import ( "fmt" + "path/filepath" "strings" "sync" "time" "github.com/GrayCodeAI/hawk/internal/engine/ctxmgr" + "github.com/GrayCodeAI/hawk/internal/home" "github.com/GrayCodeAI/hawk/internal/types" "github.com/GrayCodeAI/tok" ) @@ -189,6 +191,11 @@ type SessionSummary struct { // NewIntegrationPipeline initializes all subsystems and returns a ready-to-use // pipeline orchestrator. func NewIntegrationPipeline() *IntegrationPipeline { + // Resolve the user's home dir once so the learning-pipeline stores do not + // leak into /.hawk/ when hawk is run from inside its own source tree. + // See L2 in docs/plans/fix-critical-and-high-review.md. + homeRoot := home.Dir() + return &IntegrationPipeline{ // Pre-query IntentClassifier: NewIntentClassifier(), @@ -214,9 +221,9 @@ func NewIntegrationPipeline() *IntegrationPipeline { OutputRedactor: NewOutputRedactor(), // Learning - ExperienceStore: NewExperienceStore(".hawk/experience"), - KnowledgeBase: NewKnowledgeBase(".hawk/knowledge"), - FeedbackCollector: NewFeedbackCollector(".hawk/feedback"), + ExperienceStore: NewExperienceStore(filepath.Join(homeRoot, ".hawk", "experience")), + KnowledgeBase: NewKnowledgeBase(filepath.Join(homeRoot, ".hawk", "knowledge")), + FeedbackCollector: NewFeedbackCollector(filepath.Join(homeRoot, ".hawk", "feedback")), SelfAssessor: NewSelfAssessor(), // Session management diff --git a/internal/engine/l2_home_paths_test.go b/internal/engine/l2_home_paths_test.go new file mode 100644 index 00000000..e68f10c8 --- /dev/null +++ b/internal/engine/l2_home_paths_test.go @@ -0,0 +1,51 @@ +package engine + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestL2PipelineStatePathsAreHomeRelative is a regression guard for L2 — +// the three learning-pipeline stores (ExperienceStore, KnowledgeBase, +// FeedbackCollector) created by NewIntegrationPipeline must write to +// ~/.hawk/{experience,knowledge,feedback}/, not to /.hawk/... +// +// Pre-fix, NewIntegrationPipeline passed the literal strings +// ".hawk/experience", ".hawk/knowledge", ".hawk/feedback" to those +// constructors, which leaked into /cmd/.hawk/ when hawk was run +// from its own source tree. +func TestL2PipelineStatePathsAreHomeRelative(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("os.UserHomeDir: %v", err) + } + if home == "" { + t.Fatal("os.UserHomeDir returned empty string") + } + wantPrefix := filepath.Clean(home) + string(filepath.Separator) + + check := func(name, got string) { + t.Helper() + if !filepath.IsAbs(got) { + t.Errorf("%s: path %q is not absolute", name, got) + return + } + if !strings.HasPrefix(got, wantPrefix) && !strings.HasPrefix(got, filepath.Clean(home)) { + t.Errorf("%s: path %q does not start with home dir %q", name, got, home) + } + } + + p := NewIntegrationPipeline() + if p == nil { + t.Fatal("NewIntegrationPipeline returned nil") + } + if p.ExperienceStore == nil || p.KnowledgeBase == nil || p.FeedbackCollector == nil { + t.Fatal("NewIntegrationPipeline left a learning-pipeline store nil") + } + + check("ExperienceStore.Dir", p.ExperienceStore.Dir) + check("KnowledgeBase.Dir", p.KnowledgeBase.Dir) + check("FeedbackCollector.Dir", p.FeedbackCollector.Dir) +} diff --git a/internal/snapshot/l2_home_paths_test.go b/internal/snapshot/l2_home_paths_test.go new file mode 100644 index 00000000..17f16124 --- /dev/null +++ b/internal/snapshot/l2_home_paths_test.go @@ -0,0 +1,50 @@ +package snapshot + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestL2DefaultPathsAreHomeRelative is a regression guard for L2 — when the +// state-store constructors are called with empty/zero args, their default +// paths must be absolute and live under the user's home dir +// (~/.hawk/...), not relative to . Pre-fix, the defaults were strings +// like ".hawk/snapshots" and ".hawk/experience" which leaked into +// /cmd/.hawk/ when hawk was run from its own source tree. +func TestL2DefaultPathsAreHomeRelative(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("os.UserHomeDir: %v", err) + } + if home == "" { + t.Fatal("os.UserHomeDir returned empty string") + } + + // Sanitize HOME so we can compare reliably (filepath.Clean strips + // trailing separators). + wantPrefix := filepath.Clean(home) + string(filepath.Separator) + + check := func(name, got string) { + t.Helper() + if !filepath.IsAbs(got) { + t.Errorf("%s: default path %q is not absolute", name, got) + return + } + // On macOS temp dirs may live under /private/var/... while HOME + // resolves to /var/...; compare both forms. + if !strings.HasPrefix(got, wantPrefix) && !strings.HasPrefix(got, filepath.Clean(home)) { + t.Errorf("%s: default path %q does not start with home dir %q", name, got, home) + } + } + + // NewSnapshotStore("") default + ss := NewSnapshotStore("") + check("NewSnapshotStore", ss.Dir) + + // New() default — shadowDir is now home-relative, not + // relative to projectDir. + tracker := New(t.TempDir()) + check("New(tracker)", tracker.shadowDir) +} diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index 8332f0b4..faebb22d 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -9,6 +9,8 @@ import ( "strings" "sync" "time" + + "github.com/GrayCodeAI/hawk/internal/home" ) // Tracker maintains a shadow git repository that records every file change @@ -36,10 +38,13 @@ type FileDiff struct { } // New creates a Tracker for the given project directory. +// The shadow git repository lives under the user's home dir (~/.hawk/snapshots) +// rather than under projectDir, so that running hawk from inside a Go project +// root no longer creates a nested /cmd/.hawk/ tree at runtime. func New(projectDir string) *Tracker { return &Tracker{ projectDir: projectDir, - shadowDir: filepath.Join(projectDir, ".hawk", "snapshots"), + shadowDir: filepath.Join(home.Dir(), ".hawk", "snapshots"), } } diff --git a/internal/snapshot/snapshot_test.go b/internal/snapshot/snapshot_test.go index 879a0903..51fa3060 100644 --- a/internal/snapshot/snapshot_test.go +++ b/internal/snapshot/snapshot_test.go @@ -51,8 +51,8 @@ func TestTracker_Init(t *testing.T) { t.Fatalf("Second Init failed: %v", err) } - if _, err := os.Stat(filepath.Join(dir, ".hawk", "snapshots", ".git")); err != nil { - t.Error("shadow git repo not initialized") + if _, err := os.Stat(filepath.Join(tracker.shadowDir, ".git")); err != nil { + t.Errorf("shadow git repo not initialized at %s: %v", tracker.shadowDir, err) } } diff --git a/internal/snapshot/workspace.go b/internal/snapshot/workspace.go index 67414a03..b3d2605b 100644 --- a/internal/snapshot/workspace.go +++ b/internal/snapshot/workspace.go @@ -16,6 +16,8 @@ import ( "strings" "sync" "time" + + "github.com/GrayCodeAI/hawk/internal/home" ) // WorkspaceSnapshot captures the full state of a project at a point in time. @@ -68,10 +70,12 @@ var ignoredDirs = map[string]bool{ } // NewSnapshotStore creates a new SnapshotStore with the given directory. -// If dir is empty, defaults to ".hawk/snapshots/". +// If dir is empty, defaults to "~/.hawk/snapshots" (the user's home dir) so +// that state does not leak into /.hawk/ when hawk is run from inside +// a Go project root. func NewSnapshotStore(dir string) *SnapshotStore { if dir == "" { - dir = filepath.Join(".hawk", "snapshots") + dir = filepath.Join(home.Dir(), ".hawk", "snapshots") } return &SnapshotStore{ Dir: dir, diff --git a/internal/snapshot/workspace_test.go b/internal/snapshot/workspace_test.go index ac41ccb7..9cf33114 100644 --- a/internal/snapshot/workspace_test.go +++ b/internal/snapshot/workspace_test.go @@ -602,7 +602,14 @@ func TestRestore_PreservesGitDir(t *testing.T) { func TestNewSnapshotStore_DefaultDir(t *testing.T) { store := NewSnapshotStore("") - expected := filepath.Join(".hawk", "snapshots") + // L2: the default path is now home-relative (~/.hawk/snapshots) so + // state stops leaking into /.hawk/ when hawk is run from + // inside a Go project root. + home, err := os.UserHomeDir() + if err != nil { + t.Fatalf("os.UserHomeDir: %v", err) + } + expected := filepath.Join(home, ".hawk", "snapshots") if store.Dir != expected { t.Errorf("expected default dir %q, got %q", expected, store.Dir) }