diff --git a/README.md b/README.md index b95dcb9e..38c07bfe 100644 --- a/README.md +++ b/README.md @@ -286,6 +286,11 @@ Features adopted from open-source agent projects. All are off by default unless | Live agent status | `GET /v1/agent/status` (daemon) | Machine-readable working/idle/stale per session | | X/Twitter search | `SearchX` tool | Live X search by forwarding a query to an xAI endpoint with server-side search; returns a cited summary. Requires `XAI_API_KEY` (or `GROK_API_KEY`) | | Desktop computer-use | `ComputerUse` tool | snapshot/click/type/scroll/press/screenshot via a pluggable `tool.SetComputerBackend` seam (host wires a native macOS accessibility backend) | +| Token-cheaper file views | `Read` tool `--minify` | Read-only, comment-stripped, whitespace-dense file view (Go via `go/parser`; other languages string-aware; never touches disk) — fewer tokens per read | +| Classified provider hints | `internal/errhint` | Buckets provider errors (Auth/RateLimit/Connectivity/ModelNotFound/ContextOverflow) into a one-line fixable next step | +| Atomic install transactions | `internal/installtxn` | Cross-process staged install/remove with rollback (plugin/skill install paths) | +| Stale-lock reclaim | `internal/lockutil` | Race-correct atomic reclaim of O_EXCL lock files with live-restore | +| Test command discovery | `internal/testrunner` | Auto-detect test/verify commands (Go/npm/bun/pnpm/yarn/pytest/cargo) and parse runner output into structured results | ## Usage diff --git a/internal/errhint/errhint.go b/internal/errhint/errhint.go new file mode 100644 index 00000000..0b2dea91 --- /dev/null +++ b/internal/errhint/errhint.go @@ -0,0 +1,160 @@ +// Package errhint classifies provider/model failures into a few user-actionable +// categories and turns them into a one-line "next step" hint. +// +// Provider errors already arrive with a classified string prefix from the +// provider layer ("auth error:", "rate limit error:", …); lower-level failures +// (DNS, TLS, timeouts, context-length) arrive as raw driver or library messages. +// Classify matches both so an interactive error row and a headless exec +// provider-error path can append one concrete next step instead of dumping an +// identical red blob for every failure mode. +// +// Adopted from Zero (internal/errhint) — classified, fixable provider error hints. +package errhint + +import "strings" + +// Category buckets a provider/model failure into a small set of classes that each +// map to a distinct recovery action. +type Category int + +const ( + // Unknown means the error didn't match any known signature; callers should + // emit no hint rather than guess. + Unknown Category = iota + Auth + RateLimit + Connectivity + ModelNotFound + ContextOverflow +) + +// providerMarkers are the prefixes the provider layer attaches to every +// provider-originated failure. A UI surface's error can also be a *local* failure +// (a tool's "permission denied", a "file does not exist", a config error), so +// Classify only proceeds past this gate for messages that are recognizably from +// the provider — otherwise a broad substring like "does not exist" would attach a +// bogus model hint to an unrelated local error. +var providerMarkers = []string{ + "auth error:", + "rate limit error:", + "provider error:", + "provider request error:", + "provider stream error:", +} + +// Classify buckets err by scanning its message for known signatures. It is a +// deliberately conservative string heuristic. It first gates on a provider-origin +// marker (see providerMarkers) so local failures never draw a provider hint, then +// sub-classifies. Order matters: more specific signatures are tested before +// broader ones (e.g. "context length" as overflow before the generic "timeout" as +// connectivity). +func Classify(err error) Category { + if err == nil { + return Unknown + } + m := strings.ToLower(err.Error()) + if !containsAny(m, providerMarkers...) { + return Unknown + } + switch { + case containsAny(m, "auth error:", "unauthorized", "api key", "api_key", "invalid_api_key", + "authentication", "permission denied", "forbidden") || containsStatusCode(m, "401", "403"): + return Auth + case containsAny(m, "rate limit", "rate_limit", "too many requests", "quota", + "resource_exhausted", "overloaded") || containsStatusCode(m, "429", "529"): + return RateLimit + case containsAny(m, "context length", "context window", "maximum context", "context_length_exceeded", + "too many tokens", "prompt is too long", "reduce the length", "maximum context length"): + return ContextOverflow + case containsAny(m, "model not found", "model_not_found", "does not exist", "unknown model", + "no such model", "unsupported model", "invalid model", "model is not"): + return ModelNotFound + case containsAny(m, "dial tcp", "no such host", "connection refused", "network is unreachable", + "i/o timeout", "context deadline exceeded", "tls handshake", "connection reset", + "unexpected eof", "lookup ", "timeout"): + return Connectivity + default: + return Unknown + } +} + +// TUIHint returns a one-line hint referencing interactive slash commands, or "" +// when the category is Unknown. Meant to sit under the raw error in the live +// error row. +func TUIHint(err error) string { + switch Classify(err) { + case Auth: + return "API key rejected — run /provider to re-check your credentials" + case RateLimit: + return "Rate limited — wait a moment, or switch model with /model" + case Connectivity: + return "Can't reach the provider — run /doctor --connectivity" + case ModelNotFound: + return "Model unavailable — pick another with /model" + case ContextOverflow: + return "Context window full — run /compact to free space" + default: + return "" + } +} + +// CLIHint returns a one-line hint referencing CLI subcommands, or "" when the +// category is Unknown. Meant for the non-interactive exec error path, where slash +// commands don't apply. +func CLIHint(err error) string { + switch Classify(err) { + case Auth: + return "API key rejected — set the provider's API key or re-run provider setup" + case RateLimit: + return "Rate limited — wait a moment, or switch model with --model" + case Connectivity: + return "Can't reach the provider — run `hawk doctor`" + case ModelNotFound: + return "Model unavailable — run `hawk doctor` or pick another with --model" + case ContextOverflow: + return "Context window full — shorten the prompt or start a fresh session" + default: + return "" + } +} + +func containsAny(haystack string, needles ...string) bool { + for _, n := range needles { + if strings.Contains(haystack, n) { + return true + } + } + return false +} + +func containsStatusCode(haystack string, codes ...string) bool { + return HasStatusCode(haystack, codes...) +} + +// HasStatusCode reports whether haystack contains any of the given HTTP status +// codes as a standalone number — not embedded in a longer digit run like +// "completed in 4290ms" or "request id 14015". Exported so other packages can +// gate on a status code without re-implementing the digit-boundary check. +func HasStatusCode(haystack string, codes ...string) bool { + for _, code := range codes { + for from := 0; ; { + rel := strings.Index(haystack[from:], code) + if rel < 0 { + break + } + pos := from + rel + beforeOK := pos == 0 || !isASCIIDigit(haystack[pos-1]) + end := pos + len(code) + afterOK := end >= len(haystack) || !isASCIIDigit(haystack[end]) + if beforeOK && afterOK { + return true + } + from = pos + 1 + } + } + return false +} + +func isASCIIDigit(b byte) bool { + return b >= '0' && b <= '9' +} diff --git a/internal/errhint/errhint_test.go b/internal/errhint/errhint_test.go new file mode 100644 index 00000000..e058feb1 --- /dev/null +++ b/internal/errhint/errhint_test.go @@ -0,0 +1,79 @@ +package errhint + +import ( + "errors" + "testing" +) + +func TestClassifyGatesOnProviderMarker(t *testing.T) { + // A local error must not draw a provider hint even if it contains a keyword. + if got := Classify(errors.New("permission denied")); got != Unknown { + t.Fatalf("local permission denied classified as %v, want Unknown", got) + } + if got := Classify(errors.New("provider error: 401 unauthorized")); got != Auth { + t.Fatalf("classified = %v, want Auth", got) + } +} + +func TestClassifyCategories(t *testing.T) { + cases := []struct { + msg string + want Category + }{ + {"provider error: invalid_api_key", Auth}, + {"auth error: 403 forbidden", Auth}, + {"rate limit error: too many requests", RateLimit}, + {"provider request error: 429", RateLimit}, + {"provider error: 529", RateLimit}, + {"provider error: context length exceeded", ContextOverflow}, + {"provider error: prompt is too long", ContextOverflow}, + {"provider error: model not found", ModelNotFound}, + {"provider error: unsupported model", ModelNotFound}, + {"provider stream error: dial tcp 10.0.0.1:443: i/o timeout", Connectivity}, + {"provider error: connection refused", Connectivity}, + {"some unrelated thing", Unknown}, + {"", Unknown}, + } + for _, tc := range cases { + got := Classify(errors.New(tc.msg)) + if got != tc.want { + t.Errorf("Classify(%q) = %v, want %v", tc.msg, got, tc.want) + } + } +} + +func TestClassifyNil(t *testing.T) { + if got := Classify(nil); got != Unknown { + t.Fatalf("Classify(nil) = %v, want Unknown", got) + } +} + +func TestHints(t *testing.T) { + if TUIHint(errors.New("provider error: invalid api key")) == "" { + t.Fatal("expected a TUI hint for auth") + } + if CLIHint(errors.New("provider error: invalid api key")) == "" { + t.Fatal("expected a CLI hint for auth") + } + if TUIHint(errors.New("local file error")) != "" { + t.Fatal("expected no hint for local error") + } + if CLIHint(errors.New("local file error")) != "" { + t.Fatal("expected no hint for local error") + } +} + +func TestHasStatusCode(t *testing.T) { + if !HasStatusCode("provider error: 401", "401") { + t.Fatal("expected standalone 401 to match") + } + if HasStatusCode("completed in 4290ms", "429") { + t.Fatal("429 embedded in 4290 must not match") + } + if HasStatusCode("request id 14015", "401") { + t.Fatal("401 embedded in 14015 must not match") + } + if !HasStatusCode("provider: 429 too many", "429") { + t.Fatal("expected standalone 429 to match") + } +} diff --git a/internal/installtxn/installtxn.go b/internal/installtxn/installtxn.go new file mode 100644 index 00000000..813dd477 --- /dev/null +++ b/internal/installtxn/installtxn.go @@ -0,0 +1,149 @@ +// Package installtxn provides the cross-process filesystem transaction used by +// plugin and skill installation. Callers stage content before taking the lock, +// then commit the content swap and lockfile update together while holding it. +// +// Adopted from Zero (internal/installtxn) — atomic install with rollback. +package installtxn + +import ( + "errors" + "fmt" + "os" + "path/filepath" +) + +const lockFileName = ".hawk-install.lock" + +// Lock takes the per-install-root cross-process lock. It blocks until any other +// installer or remover using dir has completed. +func Lock(dir string) (func(), error) { + if err := os.MkdirAll(dir, 0o755); err != nil { + return nil, fmt.Errorf("create install dir: %w", err) + } + return lockFile(filepath.Join(dir, lockFileName)) +} + +// StageDir creates an install workspace on the target filesystem. Content must +// be built and validated in the returned stage directory before CommitDir is +// called. cleanup is always safe to call. +func StageDir(dir string) (stage string, cleanup func(), err error) { + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", func() {}, fmt.Errorf("create install dir: %w", err) + } + workspace, err := os.MkdirTemp(dir, ".hawk-install-txn-") + if err != nil { + return "", func() {}, fmt.Errorf("create install staging dir: %w", err) + } + return filepath.Join(workspace, "staged"), func() { cleanupWorkspace(workspace) }, nil +} + +// CommitDir replaces target with staged and runs publish while retaining the +// previous target. If either the swap or publish fails, the previous target is +// restored (or the new target is removed for a first install). +// +// The caller must hold the install-root lock returned by Lock. +func CommitDir(target string, staged string, publish func() error) error { + workspace := filepath.Dir(staged) + backup := filepath.Join(workspace, "previous") + hadPrevious := false + if _, err := os.Stat(target); err == nil { + if err := os.Rename(target, backup); err != nil { + return fmt.Errorf("retain previous install: %w", err) + } + hadPrevious = true + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect previous install: %w", err) + } + + if err := os.Rename(staged, target); err != nil { + if hadPrevious { + if restoreErr := os.Rename(backup, target); restoreErr != nil { + return errors.Join(fmt.Errorf("publish staged install: %w", err), fmt.Errorf("restore previous install: %w", restoreErr)) + } + } + return fmt.Errorf("publish staged install: %w", err) + } + if err := publish(); err != nil { + return rollback(target, backup, hadPrevious, err) + } + if hadPrevious { + _ = os.RemoveAll(backup) + } + cleanupWorkspace(workspace) + return nil +} + +// RemoveDir removes target and runs publish while retaining the target until +// publish succeeds. A publish failure restores the directory. +// +// The caller must hold the install-root lock returned by Lock. +func RemoveDir(target string, publish func() error) error { + workspace, err := os.MkdirTemp(filepath.Dir(target), ".hawk-install-txn-") + if err != nil { + return fmt.Errorf("create removal staging dir: %w", err) + } + defer cleanupWorkspace(workspace) + backup := filepath.Join(workspace, "previous") + if err := os.Rename(target, backup); err != nil { + return fmt.Errorf("retain removed install: %w", err) + } + if err := publish(); err != nil { + if restoreErr := os.Rename(backup, target); restoreErr != nil { + return errors.Join(err, fmt.Errorf("restore removed install: %w", restoreErr)) + } + return err + } + _ = os.RemoveAll(backup) + return nil +} + +func rollback(target string, backup string, hadPrevious bool, cause error) error { + if err := os.RemoveAll(target); err != nil { + return errors.Join(cause, fmt.Errorf("remove failed install: %w", err)) + } + if hadPrevious { + if err := os.Rename(backup, target); err != nil { + return errors.Join(cause, fmt.Errorf("restore previous install: %w", err)) + } + } + return cause +} + +// cleanupWorkspace never removes a retained previous install. If rollback was +// unable to restore it (for example because Windows still has a target file +// open), preserving the workspace is safer than turning a recoverable error +// into data loss. +func cleanupWorkspace(workspace string) { + if _, err := os.Stat(filepath.Join(workspace, "previous")); err == nil { + return + } + _ = os.RemoveAll(workspace) +} + +// WriteFileAtomically publishes data by renaming a complete sibling temporary +// file over path. The caller is responsible for any surrounding transaction +// lock. +func WriteFileAtomically(path string, data []byte, perm os.FileMode) error { + temp, err := os.CreateTemp(filepath.Dir(path), ".hawk-lockfile-") + if err != nil { + return err + } + tempPath := temp.Name() + defer func() { _ = os.Remove(tempPath) }() + if err := temp.Chmod(perm); err != nil { + _ = temp.Close() + return err + } + if _, err := temp.Write(data); err != nil { + _ = temp.Close() + return err + } + if err := temp.Sync(); err != nil { + _ = temp.Close() + return err + } + if err := temp.Close(); err != nil { + return err + } + return replaceFile(tempPath, path) +} diff --git a/internal/installtxn/installtxn_test.go b/internal/installtxn/installtxn_test.go new file mode 100644 index 00000000..931d5b40 --- /dev/null +++ b/internal/installtxn/installtxn_test.go @@ -0,0 +1,127 @@ +package installtxn + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func TestCommitDirRestoresPreviousInstallWhenPublishFails(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "demo") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(target, "version"), []byte("old"), 0o644); err != nil { + t.Fatal(err) + } + staged, cleanup, err := StageDir(root) + if err != nil { + t.Fatal(err) + } + defer cleanup() + if err := os.MkdirAll(staged, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(staged, "version"), []byte("new"), 0o644); err != nil { + t.Fatal(err) + } + + publishErr := errors.New("publish failed") + err = CommitDir(target, staged, func() error { return publishErr }) + if !errors.Is(err, publishErr) { + t.Fatalf("CommitDir error = %v, want publish failure", err) + } + data, err := os.ReadFile(filepath.Join(target, "version")) + if err != nil { + t.Fatalf("read restored install: %v", err) + } + if string(data) != "old" { + t.Fatalf("restored content = %q, want old", data) + } +} + +func TestCommitDirRemovesFirstInstallWhenPublishFails(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "demo") + staged, cleanup, err := StageDir(root) + if err != nil { + t.Fatal(err) + } + defer cleanup() + if err := os.MkdirAll(staged, 0o755); err != nil { + t.Fatal(err) + } + + err = CommitDir(target, staged, func() error { return errors.New("publish failed") }) + if err == nil { + t.Fatal("CommitDir unexpectedly succeeded") + } + if _, statErr := os.Stat(target); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("failed first install remains at target: %v", statErr) + } +} + +func TestCommitDirSuccessPublishesAndCleansBackup(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "demo") + if err := os.MkdirAll(target, 0o755); err != nil { + t.Fatal(err) + } + staged, cleanup, err := StageDir(root) + if err != nil { + t.Fatal(err) + } + defer cleanup() + if err := os.MkdirAll(staged, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(staged, "version"), []byte("new"), 0o644); err != nil { + t.Fatal(err) + } + if err := CommitDir(target, staged, func() error { return nil }); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(filepath.Join(target, "version")) + if string(data) != "new" { + t.Fatalf("target content = %q, want new", data) + } +} + +func TestCleanupWorkspacePreservesRetainedPreviousInstall(t *testing.T) { + workspace := t.TempDir() + previous := filepath.Join(workspace, "previous") + if err := os.MkdirAll(previous, 0o755); err != nil { + t.Fatal(err) + } + + cleanupWorkspace(workspace) + + if _, err := os.Stat(previous); err != nil { + t.Fatalf("cleanup removed retained previous install: %v", err) + } +} + +func TestWriteFileAtomically(t *testing.T) { + path := filepath.Join(t.TempDir(), "data.txt") + if err := WriteFileAtomically(path, []byte("hello"), 0o600); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != "hello" { + t.Fatalf("content = %q, want hello", data) + } +} + +func TestLock(t *testing.T) { + dir := t.TempDir() + unlock, err := Lock(dir) + if err != nil { + t.Fatal(err) + } + unlock() +} diff --git a/internal/installtxn/lock_unix.go b/internal/installtxn/lock_unix.go new file mode 100644 index 00000000..e3917261 --- /dev/null +++ b/internal/installtxn/lock_unix.go @@ -0,0 +1,29 @@ +//go:build !windows + +package installtxn + +import ( + "fmt" + "os" + + "golang.org/x/sys/unix" +) + +func lockFile(path string) (func(), error) { + file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600) + if err != nil { + return nil, fmt.Errorf("open install lock: %w", err) + } + if err := unix.Flock(int(file.Fd()), unix.LOCK_EX); err != nil { + _ = file.Close() + return nil, fmt.Errorf("lock install root: %w", err) + } + return func() { + _ = unix.Flock(int(file.Fd()), unix.LOCK_UN) + _ = file.Close() + }, nil +} + +func replaceFile(source string, target string) error { + return os.Rename(source, target) +} diff --git a/internal/installtxn/lock_windows.go b/internal/installtxn/lock_windows.go new file mode 100644 index 00000000..1132d023 --- /dev/null +++ b/internal/installtxn/lock_windows.go @@ -0,0 +1,31 @@ +//go:build windows + +package installtxn + +import ( + "fmt" + "os" + + "golang.org/x/sys/windows" +) + +func lockFile(path string) (func(), error) { + file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600) + if err != nil { + return nil, fmt.Errorf("open install lock: %w", err) + } + handle := windows.Handle(file.Fd()) + overlapped := new(windows.Overlapped) + if err := windows.LockFileEx(handle, windows.LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, overlapped); err != nil { + _ = file.Close() + return nil, fmt.Errorf("lock install root: %w", err) + } + return func() { + _ = windows.UnlockFileEx(handle, 0, 1, 0, overlapped) + _ = file.Close() + }, nil +} + +func replaceFile(source string, target string) error { + return windows.MoveFileEx(windows.StringToUTF16Ptr(source), windows.StringToUTF16Ptr(target), windows.MOVEFILE_REPLACE_EXISTING|windows.MOVEFILE_WRITE_THROUGH) +} diff --git a/internal/lockutil/lockutil.go b/internal/lockutil/lockutil.go new file mode 100644 index 00000000..a3eb6356 --- /dev/null +++ b/internal/lockutil/lockutil.go @@ -0,0 +1,58 @@ +// Package lockutil provides the platform-specific file primitives behind the +// O_EXCL lock files used by hawk (daemon, hooks, cron, install): a no-overwrite +// restore for locks that were sidelined during a stale-reclaim attempt, and a +// lock file remover with one cross-platform contract (missing files are a no-op; +// Windows retries transient sharing violations). +// +// Adopted from Zero (internal/lockutil) — atomic stale-lock reclaim primitives. +package lockutil + +import ( + "io" + "os" +) + +// restoreByCopy restores reclaimed to path without overwriting an existing +// path, as a fallback for when the platform's primary no-replace primitive +// (hard link on POSIX, MoveFileEx on Windows) fails for a reason other than +// the destination existing. It stages a full copy under a private name next +// to reclaimed and publishes it to path with publish (the same no-replace +// primitive the caller's platform uses for the primary restore), so path +// never appears in a partially-copied state. publish keeps the no-overwrite +// guarantee: a new holder that appeared in the meantime wins and this returns +// os.ErrExist. The copy resets the lock's mtime to now, which only makes the +// restored lock look fresher; that is safe, since it is being handed back to +// a live holder. +func restoreByCopy(reclaimed, path string, publish func(from, to string) error) error { + staged := reclaimed + ".copy" + src, err := os.Open(reclaimed) + if err != nil { + return err + } + dst, err := os.OpenFile(staged, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) + if err != nil { + _ = src.Close() + return err + } + _, err = io.Copy(dst, src) + // Close the source before removing anything below: Go opens files without + // FILE_SHARE_DELETE on Windows, so deleting reclaimed or staged while src + // is open would fail with a sharing violation. + _ = src.Close() + if err != nil { + _ = dst.Close() + _ = os.Remove(staged) + return err + } + if err := dst.Close(); err != nil { + _ = os.Remove(staged) + return err + } + if err := publish(staged, path); err != nil { + _ = os.Remove(staged) + return err + } + _ = os.Remove(staged) + _ = RemoveLockFile(reclaimed) + return nil +} diff --git a/internal/lockutil/lockutil_other.go b/internal/lockutil/lockutil_other.go new file mode 100644 index 00000000..a8d03e05 --- /dev/null +++ b/internal/lockutil/lockutil_other.go @@ -0,0 +1,45 @@ +//go:build !windows + +package lockutil + +import ( + "errors" + "io/fs" + "os" +) + +// RestoreLockFile restores a sidelined lock file on non-Windows platforms. It +// uses os.Link so a competing lock created at path in the meantime wins: the +// link fails with os.ErrExist instead of overwriting it. If the link fails for +// any other reason (hardlink-incapable filesystems such as FAT or some +// FUSE/network mounts, ENOSPC, EPERM), it falls back to an O_EXCL copy rather +// than leaving path missing and the live holder's lock stranded in reclaimed. +// Once the lock is back at path the restore has succeeded, so a failed cleanup +// of the sidelined name is not reported as an error. +func RestoreLockFile(reclaimed, path string) error { + err := os.Link(reclaimed, path) + if err == nil { + _ = RemoveLockFile(reclaimed) + return nil + } + if errors.Is(err, os.ErrExist) { + return err + } + return restoreByCopy(reclaimed, path, os.Link) +} + +// isReclaimContended reports whether a failed rename-aside of a suspected +// stale lock means it was lost to a racer rather than a hard failure. POSIX +// rename has no contention errno (a lost race surfaces only as ENOENT, which +// ReclaimStaleLock already treats as benign), so nothing extra is benign here. +func isReclaimContended(error) bool { return false } + +// RemoveLockFile removes a lock file on non-Windows platforms. Removing an +// already-missing file reports nil, matching the Windows implementation, so +// callers see one cross-platform contract. +func RemoveLockFile(path string) error { + if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + return nil +} diff --git a/internal/lockutil/lockutil_windows.go b/internal/lockutil/lockutil_windows.go new file mode 100644 index 00000000..f4091196 --- /dev/null +++ b/internal/lockutil/lockutil_windows.go @@ -0,0 +1,75 @@ +//go:build windows + +package lockutil + +import ( + "errors" + "os" + "syscall" + "time" + + "golang.org/x/sys/windows" +) + +// RestoreLockFile restores a sidelined lock file on Windows. It moves the file +// with a no-replace MoveFileEx so a competing lock created at path in the +// meantime wins: the move fails with os.ErrExist instead of overwriting it. If +// the move fails for any other reason (ERROR_SHARING_VIOLATION or +// ERROR_ACCESS_DENIED under concurrent access to the file), it falls back to +// an O_EXCL copy rather than leaving path missing and the live holder's lock +// stranded in reclaimed. +func RestoreLockFile(reclaimed, path string) error { + err := moveFileNoReplace(reclaimed, path) + if err == nil || errors.Is(err, os.ErrExist) { + return err + } + return restoreByCopy(reclaimed, path, moveFileNoReplace) +} + +// moveFileNoReplace renames from to to, failing if to already exists. It calls +// MoveFileExW directly via golang.org/x/sys/windows (the standard library's +// syscall package does not export MoveFileEx on this platform) with no flags, +// so an existing destination fails with ERROR_ALREADY_EXISTS, which satisfies +// errors.Is(err, os.ErrExist), instead of overwriting it the way os.Rename does +// on Windows. +func moveFileNoReplace(from, to string) error { + fromPtr, err := windows.UTF16PtrFromString(from) + if err != nil { + return err + } + toPtr, err := windows.UTF16PtrFromString(to) + if err != nil { + return err + } + return windows.MoveFileEx(fromPtr, toPtr, 0) +} + +// isReclaimContended reports whether a failed rename-aside of a suspected +// stale lock means the file was concurrently open or pending deletion (which +// surfaces as ERROR_SHARING_VIOLATION or ERROR_ACCESS_DENIED on Windows) +// rather than a hard failure. +func isReclaimContended(err error) bool { + var errno syscall.Errno + return errors.As(err, &errno) && (errno == windows.ERROR_SHARING_VIOLATION || errno == windows.ERROR_ACCESS_DENIED) +} + +// RemoveLockFile removes a lock file on Windows, retrying on the sharing +// violation or access denied errors that are common under heavy concurrent +// contention. Removing an already-missing file reports nil, matching the +// non-Windows implementation, so callers see one cross-platform contract. +func RemoveLockFile(path string) error { + var err error + for i := 0; i < 15; i++ { + err = os.Remove(path) + if err == nil || os.IsNotExist(err) { + return nil + } + var errno syscall.Errno + if errors.As(err, &errno) && (errno == windows.ERROR_SHARING_VIOLATION || errno == windows.ERROR_ACCESS_DENIED) { + time.Sleep(5 * time.Millisecond) + continue + } + return err + } + return err +} diff --git a/internal/lockutil/reclaim.go b/internal/lockutil/reclaim.go new file mode 100644 index 00000000..2c20778f --- /dev/null +++ b/internal/lockutil/reclaim.go @@ -0,0 +1,59 @@ +package lockutil + +import ( + "errors" + "os" +) + +// restoreLockFile is swappable so tests can force the fail-closed path of +// ReclaimStaleLock, which requires both the fast restore and its no-replace +// fallback (with its own copy fallback) to fail; that cannot be provoked +// portably on a healthy filesystem. +var restoreLockFile = restoreLiveLock + +// restoreLiveLock puts a lock that turned out to be live back at path after +// ReclaimStaleLock moved it aside to inspect it. It first tries a fast, +// replacing rename straight from reclaimed to path: a single syscall, which +// keeps the window during which path does not exist as short as possible. +// RestoreLockFile's no-replace restore is a correctness-preserving fallback +// for when the fast path itself fails. +func restoreLiveLock(reclaimed, path string) error { + if err := os.Rename(reclaimed, path); err == nil { + return nil + } + return RestoreLockFile(reclaimed, path) +} + +// ReclaimStaleLock atomically reclaims a suspected-stale lock file. It renames +// lockPath aside to ".stale." (only one racer can win the +// rename of a given file, so two racers can never both reclaim the same lock), +// then consults isLive on the moved file; if the lock turns out to be live (a +// holder reacquired it in the gap between the caller's stale check and the +// rename) it is restored rather than stolen. The suffix must be unique per +// acquirer attempt. Returns true only when a genuinely stale lock was removed, +// so the caller knows it is safe to retry its exclusive create immediately; on +// a lost race it returns false. A non-nil error means either the rename aside +// failed for a reason that is not contention, or a live holder's lock could not +// be restored, so lockPath may be missing; callers must fail closed instead of +// re-acquiring. The sidelined file is removed on every restore failure. +func ReclaimStaleLock(lockPath, suffix string, isLive func(reclaimedPath string) bool) (bool, error) { + reclaimed := lockPath + ".stale." + suffix + if err := os.Rename(lockPath, reclaimed); err != nil { + if errors.Is(err, os.ErrNotExist) || isReclaimContended(err) { + return false, nil // another racer already moved/removed it, or it vanished + } + return false, err + } + if isLive(reclaimed) { + // Put the live lock back instead of stealing it, and let the caller wait. + if rerr := restoreLockFile(reclaimed, lockPath); rerr != nil { + _ = RemoveLockFile(reclaimed) + if !errors.Is(rerr, os.ErrExist) { + return false, rerr + } + } + return false, nil + } + _ = RemoveLockFile(reclaimed) + return true, nil +} diff --git a/internal/lockutil/reclaim_test.go b/internal/lockutil/reclaim_test.go new file mode 100644 index 00000000..e2ede7f4 --- /dev/null +++ b/internal/lockutil/reclaim_test.go @@ -0,0 +1,101 @@ +package lockutil + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func TestReclaimStaleLockRemovesStale(t *testing.T) { + dir := t.TempDir() + lock := filepath.Join(dir, "demo.lock") + if err := os.WriteFile(lock, []byte("holder"), 0o600); err != nil { + t.Fatal(err) + } + reclaimed, err := ReclaimStaleLock(lock, "abc", func(string) bool { return false }) + if err != nil { + t.Fatal(err) + } + if !reclaimed { + t.Fatal("expected stale lock to be reclaimed") + } + if _, err := os.Stat(lock); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("lock should be gone after reclaim, stat err = %v", err) + } +} + +func TestReclaimStaleLockRestoresLive(t *testing.T) { + dir := t.TempDir() + lock := filepath.Join(dir, "demo.lock") + if err := os.WriteFile(lock, []byte("holder"), 0o600); err != nil { + t.Fatal(err) + } + reclaimed, err := ReclaimStaleLock(lock, "abc", func(string) bool { return true }) + if err != nil { + t.Fatal(err) + } + if reclaimed { + t.Fatal("a live lock must not be reported as reclaimed") + } + data, err := os.ReadFile(lock) + if err != nil { + t.Fatalf("live lock not restored: %v", err) + } + if string(data) != "holder" { + t.Fatalf("restored lock content = %q, want holder", data) + } +} + +func TestReclaimStaleLockMissingIsLostRace(t *testing.T) { + dir := t.TempDir() + lock := filepath.Join(dir, "nope.lock") + reclaimed, err := ReclaimStaleLock(lock, "abc", func(string) bool { return false }) + if err != nil { + t.Fatal(err) + } + if reclaimed { + t.Fatal("missing lock must not be reported as reclaimed") + } +} + +func TestReclaimStaleLockRestoreFailureFailsClosed(t *testing.T) { + dir := t.TempDir() + lock := filepath.Join(dir, "demo.lock") + if err := os.WriteFile(lock, []byte("holder"), 0o600); err != nil { + t.Fatal(err) + } + orig := restoreLockFile + restoreLockFile = func(string, string) error { return errors.New("restore boom") } + defer func() { restoreLockFile = orig }() + + if _, err := ReclaimStaleLock(lock, "abc", func(string) bool { return true }); err == nil { + t.Fatal("expected error when restore fails") + } +} + +func TestRestoreLockFileNoOverwrite(t *testing.T) { + dir := t.TempDir() + reclaimed := filepath.Join(dir, "reclaimed") + path := filepath.Join(dir, "target") + if err := os.WriteFile(reclaimed, []byte("r"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("new-holder"), 0o600); err != nil { + t.Fatal(err) + } + err := RestoreLockFile(reclaimed, path) + if !errors.Is(err, os.ErrExist) { + t.Fatalf("expected os.ErrExist, got %v", err) + } + data, _ := os.ReadFile(path) + if string(data) != "new-holder" { + t.Fatalf("competing holder overwritten: %q", data) + } +} + +func TestRemoveLockFileMissingIsNoop(t *testing.T) { + if err := RemoveLockFile(filepath.Join(t.TempDir(), "missing.lock")); err != nil { + t.Fatalf("expected no-op, got %v", err) + } +} diff --git a/internal/minify/comments.go b/internal/minify/comments.go new file mode 100644 index 00000000..4c732206 --- /dev/null +++ b/internal/minify/comments.go @@ -0,0 +1,189 @@ +package minify + +import "strings" + +// commentStyle describes one language's comment + string lexical rules for the +// string-aware stripper. ONLY languages whose every string form this scanner can +// model exactly appear in commentStyles. Languages with raw-string delimiters +// (C++ R"(...)", C# @"..."), lifetimes (Rust 'a), template literals (JS/TS +// `${...}`), nested block comments (Rust/Swift), or heredocs (shell/Ruby) are +// deliberately omitted — they fall back to whitespace-only minification — so the +// stripper can never mistake code or string content for a comment, nor a comment +// for code. Correctness over coverage. +type commentStyle struct { + name string + line []string // line-comment markers; the rest of the line is dropped + blockOpen string // "" when the language has no block comments + blockClose string + triples []string // raw multi-line string delimiters (Python ''' """, Java text block """) + fStrings bool // Python f-strings: a quote inside {…} does not end the string +} + +// commentStyles maps a file extension to its stripper config. Every entry here is +// covered by golden tests in comments_test.go, including the tricky cases +// (comment chars inside strings, strings inside comments, escapes, triple-quotes, +// f-string same-quote nesting, Java text blocks). +var commentStyles = map[string]commentStyle{ + ".c": {name: "c", line: []string{"//"}, blockOpen: "/*", blockClose: "*/"}, + ".java": {name: "java", line: []string{"//"}, blockOpen: "/*", blockClose: "*/", triples: []string{`"""`}}, + ".css": {name: "css", blockOpen: "/*", blockClose: "*/"}, + ".scss": {name: "scss", line: []string{"//"}, blockOpen: "/*", blockClose: "*/"}, + ".less": {name: "less", line: []string{"//"}, blockOpen: "/*", blockClose: "*/"}, + ".py": {name: "python", line: []string{"#"}, triples: []string{`"""`, `'''`}, fStrings: true}, + ".pyi": {name: "python", line: []string{"#"}, triples: []string{`"""`, `'''`}, fStrings: true}, +} + +// stripComments removes line and block comments from src for one language while +// correctly skipping string, char, triple-quoted, and (Python) f-string literals, +// so a comment marker inside a literal is never stripped and literal content is +// never mistaken for a comment. Delimiters are all ASCII, so scanning byte-wise is +// safe — UTF-8 content is copied verbatim. +func stripComments(src string, style commentStyle) string { + var out strings.Builder + out.Grow(len(src)) + i, n := 0, len(src) + for i < n { + // Raw multi-line strings first: """ must win over a bare " literal. + if d, ok := longestPrefix(src[i:], style.triples); ok { + j := scanDelimited(src, i+len(d), d) + out.WriteString(src[i:j]) + i = j + continue + } + // Block comment: drop through the close marker (or to EOF if unterminated). + if style.blockOpen != "" && strings.HasPrefix(src[i:], style.blockOpen) { + rest := src[i+len(style.blockOpen):] + if end := strings.Index(rest, style.blockClose); end >= 0 { + i += len(style.blockOpen) + end + len(style.blockClose) + } else { + i = n + } + continue + } + // Line comment: drop to the newline, which is emitted on the next iteration. + if _, ok := longestPrefix(src[i:], style.line); ok { + if nl := strings.IndexByte(src[i:], '\n'); nl >= 0 { + i += nl + } else { + i = n + } + continue + } + // String / char literal. + if c := src[i]; c == '"' || c == '\'' { + j := scanString(src, i, style.fStrings) + out.WriteString(src[i:j]) + i = j + continue + } + out.WriteByte(src[i]) + i++ + } + return out.String() +} + +// longestPrefix reports the longest candidate that prefixes s (markers are short +// and few, so a linear scan is fine). +func longestPrefix(s string, candidates []string) (string, bool) { + best := "" + for _, c := range candidates { + if len(c) > len(best) && strings.HasPrefix(s, c) { + best = c + } + } + return best, best != "" +} + +// scanDelimited returns the index just past the closing delim, starting from +// bodyStart (just past the opening delim). A backslash escapes the next byte so a +// \-escaped delimiter does not terminate; an unterminated literal runs to EOF. +func scanDelimited(src string, bodyStart int, delim string) int { + j, n := bodyStart, len(src) + for j < n { + if src[j] == '\\' { + j += 2 + continue + } + if strings.HasPrefix(src[j:], delim) { + return j + len(delim) + } + j++ + } + return n +} + +// scanString returns the index just past a closing quote for the string/char +// literal that opens at i. Backslash escapes the next byte. When fStrings is set +// and the literal is an f-string (an f/F prefix precedes the quote), a quote that +// sits inside a {…} replacement field does NOT close the literal — and a nested +// string inside that field is scanned in turn — so Python 3.12 same-quote-nested +// f-strings are handled correctly. +func scanString(src string, i int, fStrings bool) int { + quote := src[i] + isF := fStrings && hasFStringPrefix(src, i) + j, n := i+1, len(src) + brace := 0 + for j < n { + c := src[j] + if c == '\\' { + j += 2 + continue + } + if isF { + switch c { + case '{': + if j+1 < n && src[j+1] == '{' { // {{ is a literal brace + j += 2 + continue + } + brace++ + j++ + continue + case '}': + if j+1 < n && src[j+1] == '}' { + j += 2 + continue + } + if brace > 0 { + brace-- + } + j++ + continue + case '"', '\'': + if brace > 0 { // a nested string inside the replacement field + j = scanString(src, j, false) + continue + } + } + } + if c == quote && brace == 0 { + return j + 1 + } + j++ + } + return n +} + +// hasFStringPrefix reports whether the quote at i is preceded by a Python string +// prefix containing f/F (e.g. f", rf", Rf"), bounded so a quote that merely +// follows an identifier ending in f is not misread as an f-string. +func hasFStringPrefix(src string, i int) bool { + k := i - 1 + sawF := false + for k >= 0 { + switch src[k] { + case 'f', 'F': + sawF = true + case 'r', 'R', 'b', 'B', 'u', 'U': + default: + goto bounded + } + k-- + } +bounded: + return sawF && (k < 0 || !isIdentByte(src[k])) +} + +func isIdentByte(b byte) bool { + return b == '_' || (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') +} diff --git a/internal/minify/minify.go b/internal/minify/minify.go new file mode 100644 index 00000000..16bb54bf --- /dev/null +++ b/internal/minify/minify.go @@ -0,0 +1,224 @@ +// Package minify produces a denser, token-cheaper VIEW of a source file for +// read-only exploration. It strips comments and redundant whitespace so the +// model can scan or understand code for far fewer tokens than read_file's raw, +// line-numbered output — without ever changing the file on disk. +// +// Correctness over aggression: Go is minified through the real go/parser AST +// (guaranteed valid, comment-free output), and any file it cannot parse — or any +// non-Go file — falls back to a conservative whitespace normalization that can +// never alter code meaning (it strips no comments, since doing that safely needs +// a real parser per language). The transform is therefore incapable of corrupting +// what the model sees: the worst case is "no reduction", never "wrong content". +// +// Adopted from Zero (internal/minify) — a read-only file view for token savings. +package minify + +import ( + "bytes" + "go/ast" + "go/parser" + "go/printer" + "go/scanner" + "go/token" + "path/filepath" + "strings" +) + +// Result is the outcome of minifying one file's bytes. +type Result struct { + Content string // minified (or whitespace-normalized) text; never line-numbered + Language string // strategy taken: "go" or "text" + Applied bool // true only when real comment-stripping minification ran +} + +// File minifies content addressed by path; the extension selects the strategy. +func File(path string, content []byte) Result { + ext := strings.ToLower(filepath.Ext(path)) + if ext == ".go" { + if out, ok := minifyGo(content); ok { + return Result{Content: out, Language: "go", Applied: true} + } + if out, ok := minifyGoFragment(content); ok { + return Result{Content: out, Language: "go-fragment", Applied: true} + } + // An incomplete fragment that cannot be wrapped safely falls through to + // whitespace-only normalization; no source text is guessed or discarded. + } else if style, ok := commentStyles[ext]; ok { + // Strip comments with a string-aware lexer, then collapse the whitespace the + // removed comments left behind. The stripper only handles languages whose + // string forms it models exactly, so it cannot corrupt content. + stripped := stripComments(string(content), style) + return Result{Content: minifyGeneric([]byte(stripped)), Language: style.name, Applied: true} + } + return Result{Content: minifyGeneric(content), Language: "text", Applied: false} +} + +// Fragment minifies a bounded source range without assuming lexical state from +// text outside the range. Go remains parser-validated; other languages use the +// conservative whitespace-only transform because a range may begin inside a +// multiline string or block comment. +func Fragment(path string, content []byte) Result { + if strings.EqualFold(filepath.Ext(path), ".go") { + if out, ok := minifyGo(content); ok { + return Result{Content: out, Language: "go", Applied: true} + } + if out, ok := minifyGoFragment(content); ok { + return Result{Content: out, Language: "go-fragment", Applied: true} + } + } + return Result{Content: minifyGeneric(content), Language: "text", Applied: false} +} + +// ContextualFragment minifies a bounded fragment after checking its starting +// position against the complete source. A Go range that begins inside a string +// or comment must remain verbatim apart from whitespace normalization: parsing +// that fragment by itself would mistake literal text for Go syntax. +func ContextualFragment(path string, source, content []byte, startOffset int) Result { + if strings.EqualFold(filepath.Ext(path), ".go") && goFragmentStartsInsideToken(source, startOffset) { + return Result{Content: minifyGeneric(content), Language: "text", Applied: false} + } + return Fragment(path, content) +} + +func goFragmentStartsInsideToken(source []byte, startOffset int) bool { + if startOffset <= 0 || startOffset >= len(source) { + return false + } + fset := token.NewFileSet() + file := fset.AddFile("", fset.Base(), len(source)) + var lexer scanner.Scanner + lexer.Init(file, source, nil, scanner.ScanComments) + for { + position, kind, literal := lexer.Scan() + if kind == token.EOF { + return false + } + if kind != token.STRING && kind != token.COMMENT { + continue + } + start := file.Offset(position) + if startOffset > start && startOffset < start+len(literal) { + return true + } + } +} + +// minifyGoFragment handles bounded reads that do not contain a package clause. +// It finds the longest complete declaration or statement prefix that the real +// Go parser accepts, prints that prefix without comments, and preserves any +// incomplete tail conservatively. Ranges are intentionally capped: repeatedly +// parsing a large broken file would otherwise turn a cheap read into O(n²) work. +func minifyGoFragment(content []byte) (string, bool) { + lines := strings.Split(strings.ReplaceAll(string(content), "\r\n", "\n"), "\n") + if len(lines) > 1 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + if len(lines) > 512 { + return "", false + } + for end := len(lines); end > 0; end-- { + prefix := strings.Join(lines[:end], "\n") + if compact, ok := parseGoDeclarations(prefix); ok { + return joinCompactPrefix(compact, lines[end:]), true + } + if compact, ok := parseGoStatements(prefix); ok { + return joinCompactPrefix(compact, lines[end:]), true + } + } + return "", false +} + +func parseGoDeclarations(fragment string) (string, bool) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "", "package compact\n"+fragment, parser.SkipObjectResolution) + if err != nil || len(file.Decls) == 0 { + return "", false + } + nodes := make([]ast.Node, len(file.Decls)) + for i, declaration := range file.Decls { + nodes[i] = declaration + } + return printGoNodes(fset, nodes) +} + +func parseGoStatements(fragment string) (string, bool) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "", "package compact\nfunc _(){\n"+fragment+"\n}", parser.SkipObjectResolution) + if err != nil || len(file.Decls) != 1 { + return "", false + } + function, ok := file.Decls[0].(*ast.FuncDecl) + if !ok || function.Body == nil || len(function.Body.List) == 0 { + return "", false + } + nodes := make([]ast.Node, len(function.Body.List)) + for i, statement := range function.Body.List { + nodes[i] = statement + } + return printGoNodes(fset, nodes) +} + +func printGoNodes(fset *token.FileSet, nodes []ast.Node) (string, bool) { + var buf bytes.Buffer + cfg := printer.Config{Mode: printer.TabIndent, Tabwidth: 1} + for i, node := range nodes { + if i > 0 { + buf.WriteByte('\n') + } + if err := cfg.Fprint(&buf, fset, node); err != nil { + return "", false + } + } + return strings.TrimSpace(buf.String()), true +} + +func joinCompactPrefix(compact string, remainder []string) string { + tail := minifyGeneric([]byte(strings.Join(remainder, "\n"))) + if tail == "" { + return compact + } + return compact + "\n" + tail +} + +// minifyGo parses Go WITHOUT comments (omitting parser.ParseComments leaves them +// unattached) and reprints the AST, yielding valid, comment-free, gofmt-shaped +// source with tab indentation (1 char per level). It returns ok=false on any +// parse error so the caller falls back to the raw text — a non-package snippet or +// syntactically invalid file is never mangled. +func minifyGo(content []byte) (string, bool) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "", content, parser.SkipObjectResolution) + if err != nil { + return "", false + } + var buf bytes.Buffer + cfg := printer.Config{Mode: printer.TabIndent, Tabwidth: 1} + if err := cfg.Fprint(&buf, fset, file); err != nil { + return "", false + } + return strings.TrimRight(buf.String(), "\n"), true +} + +// minifyGeneric is the safe fallback for non-Go (and unparsable Go) content: it +// normalizes CRLF, trims trailing whitespace, and collapses runs of blank lines +// to a single blank — removing easy bloat with zero risk to code semantics. It +// deliberately strips NO comments. +func minifyGeneric(content []byte) string { + normalized := strings.ReplaceAll(string(content), "\r\n", "\n") + lines := strings.Split(normalized, "\n") + out := make([]string, 0, len(lines)) + blankRun := 0 + for _, line := range lines { + trimmed := strings.TrimRight(line, " \t") + if trimmed == "" { + blankRun++ + if blankRun > 1 { + continue + } + } else { + blankRun = 0 + } + out = append(out, trimmed) + } + return strings.TrimRight(strings.Join(out, "\n"), "\n") +} diff --git a/internal/minify/minify_test.go b/internal/minify/minify_test.go new file mode 100644 index 00000000..0b88375a --- /dev/null +++ b/internal/minify/minify_test.go @@ -0,0 +1,108 @@ +package minify + +import ( + "strings" + "testing" +) + +func TestFileGoStripsCommentsAndWhitespace(t *testing.T) { + src := []byte(`// header comment +package demo + +// a doc comment +func Add(a, b int) int { + // inline + return a + b +} +`) + res := File("x.go", src) + if !res.Applied { + t.Fatal("expected Go minification to apply") + } + if res.Language != "go" { + t.Fatalf("language = %q", res.Language) + } + if strings.Contains(res.Content, "//") { + t.Fatalf("comments not stripped:\n%s", res.Content) + } + if strings.Contains(res.Content, "header comment") { + t.Fatalf("comment text leaked:\n%s", res.Content) + } + if !strings.Contains(res.Content, "func Add") { + t.Fatalf("code missing:\n%s", res.Content) + } +} + +func TestFileGenericPreservesComments(t *testing.T) { + src := []byte("// keep me\n\n\n\nfoo()\n") + res := File("x.txt", src) + if res.Applied { + t.Fatal("text file should not apply comment stripping") + } + if !strings.Contains(res.Content, "// keep me") { + t.Fatalf("comment should be preserved for text: %q", res.Content) + } + if strings.Contains(res.Content, "\n\n\n") { + t.Fatalf("blank lines not collapsed: %q", res.Content) + } +} + +func TestFilePythonStripsCommentsButKeepsString(t *testing.T) { + src := []byte(`# a comment +s = "http://example.com/#frag" +t = '''not a comment # still here''' +# trailing +print(s) +`) + res := File("x.py", src) + if !res.Applied { + t.Fatal("expected python minification") + } + if strings.Contains(res.Content, "# a comment") || strings.Contains(res.Content, "# trailing") { + t.Fatalf("python comments not stripped:\n%s", res.Content) + } + if !strings.Contains(res.Content, "http://example.com/#frag") { + t.Fatalf("url in string was corrupted:\n%s", res.Content) + } + if !strings.Contains(res.Content, "not a comment # still here") { + t.Fatalf("triple-quoted content corrupted:\n%s", res.Content) + } +} + +func TestFileGoInvalidFallsBackToText(t *testing.T) { + res := File("x.go", []byte("func { not valid go")) + if res.Applied { + t.Fatal("invalid Go must not apply comment stripping") + } + if res.Language != "text" { + t.Fatalf("language = %q", res.Language) + } +} + +func TestFileCRLFNormalized(t *testing.T) { + res := File("x.txt", []byte("a\r\n\r\n\r\nb\r\n")) + if strings.Contains(res.Content, "\r") { + t.Fatalf("CRLF not normalized: %q", res.Content) + } +} + +func TestFragmentGo(t *testing.T) { + src := []byte("// c\nfunc A() {}\nfunc B() {}\n") + res := Fragment("x.go", src) + if !res.Applied { + t.Fatal("expected go fragment minification") + } + if strings.Contains(res.Content, "// c") { + t.Fatalf("comment not stripped in fragment:\n%s", res.Content) + } +} + +func TestContextualFragmentInsideToken(t *testing.T) { + // Fragment starts inside a string literal; it must not be parsed as Go. + source := []byte(`s := "hello world"`) + content := []byte(`o world"`) + res := ContextualFragment("x.go", source, content, 9) + if res.Applied { + t.Fatal("fragment starting inside a token must not apply Go parsing") + } +} diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go new file mode 100644 index 00000000..125b79aa --- /dev/null +++ b/internal/testrunner/testrunner.go @@ -0,0 +1,476 @@ +// Package testrunner auto-discovers test/verify commands from project files and +// parses runner output into structured, machine-readable results. Adopted from +// Zero (internal/testrunner) to upgrade hawk's verify/self-verification loop: +// instead of blindly running one hard-coded command, Detect finds the right +// checks (Go, bun/npm/pnpm/yarn, pytest, cargo) and ParseSummary turns raw +// output into counts and failure locations. +package testrunner + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" +) + +// Kind describes the broad purpose of a detected verification check. +type Kind string + +const ( + KindTest Kind = "test" + KindTypecheck Kind = "typecheck" + KindBuild Kind = "build" + KindLint Kind = "lint" +) + +// Framework identifies the runner family used to execute or parse a check. +type Framework string + +const ( + FrameworkGo Framework = "go" + FrameworkBun Framework = "bun" + FrameworkNode Framework = "node" + FrameworkPytest Framework = "pytest" + FrameworkCargo Framework = "cargo" +) + +// Check is a runnable workspace verification command discovered from project files. +type Check struct { + ID string `json:"id"` + Name string `json:"name"` + Command []string `json:"command"` + Kind Kind `json:"kind"` + Framework Framework `json:"framework"` +} + +// Summary is the structured test result parsed from runner output. +type Summary struct { + Framework Framework `json:"framework"` + Total int `json:"total,omitempty"` + Passed int `json:"passed,omitempty"` + Failed int `json:"failed,omitempty"` + Skipped int `json:"skipped,omitempty"` + Failures []Failure `json:"failures,omitempty"` +} + +// Failure captures the most useful location and message for a failing test. +type Failure struct { + Name string `json:"name"` + File string `json:"file,omitempty"` + Message string `json:"message,omitempty"` +} + +type packageJSON struct { + PackageManager string `json:"packageManager"` + Scripts map[string]string `json:"scripts"` +} + +type packageManager struct { + Name string + IDPrefix string + Framework Framework +} + +var ( + goRunPattern = regexp.MustCompile(`^=== RUN\s+(.+)$`) + goPassPattern = regexp.MustCompile(`^--- PASS:\s+([^\s(]+)`) + goFailPattern = regexp.MustCompile(`^--- FAIL:\s+([^\s(]+)`) + goSkipPattern = regexp.MustCompile(`^--- SKIP:\s+([^\s(]+)`) + goFailureLocation = regexp.MustCompile(`^\s*([^:\s]+\.go:\d+):\s*(.*)$`) + goPackageOK = regexp.MustCompile(`^ok\s+\S+`) + bunPassPattern = regexp.MustCompile(`^\(pass\)\s+(.+?)(?:\s+\[.*)?$`) + bunFailPattern = regexp.MustCompile(`^\(fail\)\s+(.+?)(?:\s+\[.*)?$`) + bunSkipPattern = regexp.MustCompile(`^\(skip\)\s+(.+?)(?:\s+\[.*)?$`) + nodePassPattern = regexp.MustCompile(`^ok\s+\d+(?:\s+-\s+(.+))?$`) + nodeFailPattern = regexp.MustCompile(`^not ok\s+\d+(?:\s+-\s+(.+))?$`) + pytestFailureLine = regexp.MustCompile(`^FAILED\s+(\S+)(?:\s+-\s*(.*))?$`) + cargoTestLine = regexp.MustCompile(`^test\s+(.+)\s+\.\.\.\s+(ok|FAILED|ignored)$`) + summaryCountPrefix = regexp.MustCompile(`(?i)(\d+)\s+(pass|passes|passed|fail|fails|failed|skip|skips|skipped|ignored)\b`) +) + +// Detect discovers common test and verification commands in root. +func Detect(root string) ([]Check, error) { + resolvedRoot, err := resolveRoot(root) + if err != nil { + return nil, err + } + checks := []Check{} + if fileExists(filepath.Join(resolvedRoot, "go.mod")) { + checks = append(checks, Check{ + ID: "go.test", + Name: "Go tests", + Command: []string{"go", "test", "./..."}, + Kind: KindTest, + Framework: FrameworkGo, + }) + } + checks = append(checks, detectPackageChecks(resolvedRoot)...) + if detectsPytest(resolvedRoot) { + checks = append(checks, Check{ + ID: "python.pytest", + Name: "Python pytest", + Command: []string{"python", "-m", "pytest"}, + Kind: KindTest, + Framework: FrameworkPytest, + }) + } + if fileExists(filepath.Join(resolvedRoot, "Cargo.toml")) { + checks = append(checks, Check{ + ID: "cargo.test", + Name: "Cargo tests", + Command: []string{"cargo", "test"}, + Kind: KindTest, + Framework: FrameworkCargo, + }) + } + return checks, nil +} + +// ParseSummary extracts structured test counts and failures from runner output. +func ParseSummary(check Check, stdout string, stderr string) *Summary { + framework := check.Framework + if framework == "" { + framework = inferFramework(check.Command) + } + combined := strings.TrimSpace(strings.Join([]string{stdout, stderr}, "\n")) + if combined == "" { + return nil + } + switch framework { + case FrameworkGo: + return parseGoSummary(combined) + case FrameworkBun: + return parseBunSummary(combined) + case FrameworkPytest: + return parsePytestSummary(combined) + case FrameworkCargo: + return parseCargoSummary(combined) + default: + return parseNodeSummary(combined) + } +} + +func detectPackageChecks(root string) []Check { + pkg, ok := readPackageJSON(filepath.Join(root, "package.json")) + if !ok { + return nil + } + manager := detectPackageManager(root, pkg.PackageManager) + checks := []Check{} + for _, candidate := range []struct { + script string + kind Kind + label string + }{ + {script: "typecheck", kind: KindTypecheck, label: "typecheck"}, + {script: "test", kind: KindTest, label: "tests"}, + {script: "build", kind: KindBuild, label: "build"}, + {script: "lint", kind: KindLint, label: "lint"}, + } { + if strings.TrimSpace(pkg.Scripts[candidate.script]) == "" { + continue + } + checks = append(checks, Check{ + ID: manager.IDPrefix + "." + candidate.script, + Name: titleWord(manager.Name) + " " + candidate.label, + Command: []string{manager.Name, "run", candidate.script}, + Kind: candidate.kind, + Framework: manager.Framework, + }) + } + return checks +} + +func readPackageJSON(path string) (packageJSON, bool) { + data, err := os.ReadFile(path) + if err != nil { + return packageJSON{}, false + } + var pkg packageJSON + if err := json.Unmarshal(data, &pkg); err != nil { + return packageJSON{}, false + } + return pkg, true +} + +func detectPackageManager(root string, declared string) packageManager { + declaredName := strings.ToLower(strings.TrimSpace(strings.Split(declared, "@")[0])) + switch { + case fileExists(filepath.Join(root, "bun.lock")) || fileExists(filepath.Join(root, "bun.lockb")) || declaredName == "bun": + return packageManager{Name: "bun", IDPrefix: "bun", Framework: FrameworkBun} + case fileExists(filepath.Join(root, "pnpm-lock.yaml")) || declaredName == "pnpm": + return packageManager{Name: "pnpm", IDPrefix: "pnpm", Framework: FrameworkNode} + case fileExists(filepath.Join(root, "yarn.lock")) || declaredName == "yarn": + return packageManager{Name: "yarn", IDPrefix: "yarn", Framework: FrameworkNode} + case fileExists(filepath.Join(root, "package-lock.json")) || declaredName == "npm": + return packageManager{Name: "npm", IDPrefix: "npm", Framework: FrameworkNode} + default: + return packageManager{Name: "npm", IDPrefix: "npm", Framework: FrameworkNode} + } +} + +func detectsPytest(root string) bool { + for _, name := range []string{"pytest.ini", "tox.ini"} { + if fileExists(filepath.Join(root, name)) { + return true + } + } + if fileContains(filepath.Join(root, "pyproject.toml"), "pytest") { + return true + } + if fileContains(filepath.Join(root, "setup.cfg"), "pytest") { + return true + } + return false +} + +func parseGoSummary(output string) *Summary { + summary := &Summary{Framework: FrameworkGo} + lines := splitLines(output) + failureByName := map[string]int{} + packagePasses := 0 + sawVerbosePerTestOutput := false + for index, line := range lines { + switch { + case goRunPattern.MatchString(line): + sawVerbosePerTestOutput = true + summary.Total++ + case goPassPattern.MatchString(line): + sawVerbosePerTestOutput = true + summary.Passed++ + case goSkipPattern.MatchString(line): + sawVerbosePerTestOutput = true + summary.Skipped++ + case goFailPattern.MatchString(line): + summary.Failed++ + match := goFailPattern.FindStringSubmatch(line) + name := safeSubmatch(match, 1) + failure := Failure{Name: name} + if index+1 < len(lines) { + if location := goFailureLocation.FindStringSubmatch(lines[index+1]); location != nil { + failure.File = safeSubmatch(location, 1) + failure.Message = safeSubmatch(location, 2) + } + } + failureByName[name] = appendFailure(&summary.Failures, failure) + case goPackageOK.MatchString(line): + packagePasses++ + } + } + if !sawVerbosePerTestOutput { + summary.Passed += packagePasses + } + for index, line := range lines { + location := goFailureLocation.FindStringSubmatch(line) + if location == nil || index == 0 { + continue + } + previous := goFailPattern.FindStringSubmatch(lines[index-1]) + if previous == nil { + continue + } + if failureIndex, ok := failureByName[safeSubmatch(previous, 1)]; ok { + summary.Failures[failureIndex].File = safeSubmatch(location, 1) + summary.Failures[failureIndex].Message = safeSubmatch(location, 2) + } + } + normalizeTotals(summary) + return nilIfEmpty(summary) +} + +func parseBunSummary(output string) *Summary { + summary := &Summary{Framework: FrameworkBun} + for _, line := range splitLines(output) { + switch { + case bunPassPattern.MatchString(line): + summary.Passed++ + case bunSkipPattern.MatchString(line): + summary.Skipped++ + case bunFailPattern.MatchString(line): + summary.Failed++ + match := bunFailPattern.FindStringSubmatch(line) + summary.Failures = append(summary.Failures, Failure{Name: safeSubmatch(match, 1)}) + default: + mergeSummaryCounts(summary, line) + } + } + normalizeTotals(summary) + return nilIfEmpty(summary) +} + +func parseNodeSummary(output string) *Summary { + summary := &Summary{Framework: FrameworkNode} + for _, line := range splitLines(output) { + switch { + case nodePassPattern.MatchString(line): + summary.Passed++ + case nodeFailPattern.MatchString(line): + summary.Failed++ + match := nodeFailPattern.FindStringSubmatch(line) + name := safeSubmatch(match, 1) + if name == "" { + name = "tap failure" + } + summary.Failures = append(summary.Failures, Failure{Name: name}) + default: + mergeSummaryCounts(summary, line) + } + } + normalizeTotals(summary) + return nilIfEmpty(summary) +} + +func parsePytestSummary(output string) *Summary { + summary := &Summary{Framework: FrameworkPytest} + for _, line := range splitLines(output) { + if match := pytestFailureLine.FindStringSubmatch(line); match != nil { + summary.Failures = append(summary.Failures, Failure{Name: safeSubmatch(match, 1), Message: safeSubmatch(match, 2)}) + } + mergeSummaryCounts(summary, line) + } + if summary.Failed == 0 && len(summary.Failures) > 0 { + summary.Failed = len(summary.Failures) + } + normalizeTotals(summary) + return nilIfEmpty(summary) +} + +func parseCargoSummary(output string) *Summary { + summary := &Summary{Framework: FrameworkCargo} + for _, line := range splitLines(output) { + if match := cargoTestLine.FindStringSubmatch(line); match != nil { + name := safeSubmatch(match, 1) + switch safeSubmatch(match, 2) { + case "ok": + summary.Passed++ + case "FAILED": + summary.Failed++ + summary.Failures = append(summary.Failures, Failure{Name: name}) + case "ignored": + summary.Skipped++ + } + continue + } + mergeSummaryCounts(summary, line) + } + normalizeTotals(summary) + return nilIfEmpty(summary) +} + +func mergeSummaryCounts(summary *Summary, line string) { + for _, match := range summaryCountPrefix.FindAllStringSubmatch(line, -1) { + count, err := strconv.Atoi(safeSubmatch(match, 1)) + if err != nil { + continue + } + switch strings.ToLower(safeSubmatch(match, 2)) { + case "pass", "passes", "passed": + summary.Passed = maxInt(summary.Passed, count) + case "fail", "fails", "failed": + summary.Failed = maxInt(summary.Failed, count) + case "skip", "skips", "skipped", "ignored": + summary.Skipped = maxInt(summary.Skipped, count) + } + } +} + +func normalizeTotals(summary *Summary) { + if summary.Total == 0 { + summary.Total = summary.Passed + summary.Failed + summary.Skipped + } +} + +func nilIfEmpty(summary *Summary) *Summary { + if summary.Total == 0 && summary.Passed == 0 && summary.Failed == 0 && summary.Skipped == 0 && len(summary.Failures) == 0 { + return nil + } + return summary +} + +func appendFailure(failures *[]Failure, failure Failure) int { + *failures = append(*failures, failure) + return len(*failures) - 1 +} + +func inferFramework(command []string) Framework { + if len(command) == 0 { + return FrameworkNode + } + switch command[0] { + case "go": + return FrameworkGo + case "bun": + return FrameworkBun + case "python", "python3", "pytest": + return FrameworkPytest + case "cargo": + return FrameworkCargo + default: + return FrameworkNode + } +} + +func splitLines(value string) []string { + raw := strings.Split(value, "\n") + lines := make([]string, 0, len(raw)) + for _, line := range raw { + trimmed := strings.TrimSpace(line) + if trimmed != "" { + lines = append(lines, trimmed) + } + } + return lines +} + +func safeSubmatch(match []string, index int) string { + if index >= len(match) { + return "" + } + return strings.TrimSpace(match[index]) +} + +func titleWord(value string) string { + if value == "" { + return "" + } + return strings.ToUpper(value[:1]) + value[1:] +} + +func fileExists(path string) bool { + info, err := os.Stat(path) + return err == nil && !info.IsDir() +} + +func fileContains(path string, needle string) bool { + data, err := os.ReadFile(path) + return err == nil && strings.Contains(strings.ToLower(string(data)), strings.ToLower(needle)) +} + +func resolveRoot(root string) (string, error) { + if strings.TrimSpace(root) == "" { + var err error + root, err = os.Getwd() + if err != nil { + return "", fmt.Errorf("resolve test root: %w", err) + } + } + absolute, err := filepath.Abs(root) + if err != nil { + return "", fmt.Errorf("resolve test root: %w", err) + } + info, err := os.Stat(absolute) + if err != nil || !info.IsDir() { + return "", fmt.Errorf("test root must be an existing directory: %s", absolute) + } + return filepath.Clean(absolute), nil +} + +func maxInt(left int, right int) int { + if left > right { + return left + } + return right +} diff --git a/internal/testrunner/testrunner_test.go b/internal/testrunner/testrunner_test.go new file mode 100644 index 00000000..a92b20de --- /dev/null +++ b/internal/testrunner/testrunner_test.go @@ -0,0 +1,129 @@ +package testrunner + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDetectGo(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "go.mod"), []byte("module x\n"), 0o644); err != nil { + t.Fatal(err) + } + checks, err := Detect(root) + if err != nil { + t.Fatal(err) + } + found := false + for _, c := range checks { + if c.ID == "go.test" { + found = true + if c.Kind != KindTest || c.Framework != FrameworkGo { + t.Fatalf("go.test check wrong: %+v", c) + } + } + } + if !found { + t.Fatalf("go.test not detected: %+v", checks) + } +} + +func TestDetectNodeScripts(t *testing.T) { + root := t.TempDir() + pkg := `{"scripts":{"test":"vitest run","typecheck":"tsc --noEmit","lint":"eslint ."}}` + if err := os.WriteFile(filepath.Join(root, "package.json"), []byte(pkg), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "package-lock.json"), []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + checks, err := Detect(root) + if err != nil { + t.Fatal(err) + } + var ids []string + for _, c := range checks { + ids = append(ids, c.ID) + } + for _, want := range []string{"npm.test", "npm.typecheck", "npm.lint"} { + if !contains(ids, want) { + t.Fatalf("missing %s in %v", want, ids) + } + } +} + +func TestDetectRejectsNonDir(t *testing.T) { + if _, err := Detect(filepath.Join(t.TempDir(), "missing")); err == nil { + t.Fatal("expected error for missing root") + } +} + +func TestParseGoSummary(t *testing.T) { + out := `=== RUN TestOne +--- PASS: TestOne (0.00s) +=== RUN TestTwo +--- FAIL: TestTwo (0.00s) + foo_test.go:12: expected x got y +ok example.com/pkg +FAIL` + check := Check{Framework: FrameworkGo} + summary := ParseSummary(check, out, "") + if summary == nil { + t.Fatal("expected summary") + } + if summary.Passed != 1 || summary.Failed != 1 { + t.Fatalf("passed=%d failed=%d", summary.Passed, summary.Failed) + } + if len(summary.Failures) != 1 || summary.Failures[0].Name != "TestTwo" { + t.Fatalf("failures = %+v", summary.Failures) + } + if summary.Failures[0].File != "foo_test.go:12" { + t.Fatalf("failure file = %q", summary.Failures[0].File) + } +} + +func TestParseCargoSummary(t *testing.T) { + out := `test add ... ok +test sub ... FAILED +test ignored_test ... ignored +` + check := Check{Framework: FrameworkCargo} + summary := ParseSummary(check, out, "") + if summary == nil { + t.Fatal("expected summary") + } + if summary.Passed != 1 || summary.Failed != 1 || summary.Skipped != 1 { + t.Fatalf("summary = %+v", summary) + } +} + +func TestParseSummaryEmptyIsNil(t *testing.T) { + if ParseSummary(Check{Framework: FrameworkGo}, "", "") != nil { + t.Fatal("expected nil for empty output") + } +} + +func TestParseNodeSummaryCounts(t *testing.T) { + out := `not ok 1 - broke +1..3 +# tests 3 +# pass 2 +# fail 1` + summary := ParseSummary(Check{Framework: FrameworkNode}, out, "") + if summary == nil { + t.Fatal("expected summary") + } + if summary.Failed == 0 || len(summary.Failures) == 0 { + t.Fatalf("summary = %+v", summary) + } +} + +func contains(ss []string, want string) bool { + for _, s := range ss { + if s == want { + return true + } + } + return false +} diff --git a/internal/tool/file_read.go b/internal/tool/file_read.go index c213601f..26c97c6c 100644 --- a/internal/tool/file_read.go +++ b/internal/tool/file_read.go @@ -9,6 +9,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/GrayCodeAI/hawk/internal/minify" ) const maxFileSize = 10 << 20 // 10 MiB @@ -34,6 +36,7 @@ func (FileReadTool) Schema() ToolSchema { "end_line": {Type: "integer", Description: "End line (1-based, inclusive, optional)"}, "offset": {Type: "integer", Description: "Archive-compatible 1-based start line alias"}, "limit": {Type: "integer", Description: "Archive-compatible number of lines to read"}, + "minify": {Type: "boolean", Description: "Return a token-cheaper, comment-stripped view of the file (read-only; never changes disk)"}, }, Required: []string{"path"}, } @@ -54,6 +57,7 @@ func (FileReadTool) Execute(ctx context.Context, input json.RawMessage) (string, EndLine int `json:"end_line"` Offset int `json:"offset"` Limit int `json:"limit"` + Minify bool `json:"minify"` } if err := json.Unmarshal(input, &p); err != nil { return "", err @@ -142,6 +146,25 @@ func (FileReadTool) Execute(ctx context.Context, input json.RawMessage) (string, return BinaryIndicator, nil } data = StripBOM(data) + if p.Minify { + // Token-cheaper read-only view: strips comments and collapses whitespace + // without ever changing the file. Never line-numbered, so the minified + // view is returned whole (ranges still respect the requested slice). + view := data + if startLine > 0 || endLine > 0 { + lines := strings.Split(string(data), "\n") + start := max(1, startLine) - 1 + end := len(lines) + if endLine > 0 { + end = min(endLine, len(lines)) + } + if start >= len(lines) { + return "", fmt.Errorf("start_line %d exceeds file length %d", startLine, len(lines)) + } + view = []byte(strings.Join(lines[start:end], "\n")) + } + return minify.File(resolved, view).Content, nil + } if startLine == 0 && endLine == 0 { return string(data), nil } diff --git a/internal/tool/minify_read_test.go b/internal/tool/minify_read_test.go new file mode 100644 index 00000000..fa7613fe --- /dev/null +++ b/internal/tool/minify_read_test.go @@ -0,0 +1,56 @@ +package tool + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestReadWithMinifyStripsComments(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "demo.go") + src := `// header +package demo + +// doc +func Add(a, b int) int { + // inline + return a + b +} +` + if err := os.WriteFile(path, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + + in, _ := json.Marshal(map[string]interface{}{"path": path, "minify": true}) + out, err := FileReadTool{}.Execute(context.Background(), in) + if err != nil { + t.Fatal(err) + } + if strings.Contains(out, "//") || strings.Contains(out, "header") { + t.Fatalf("minified read still contains comments:\n%s", out) + } + if !strings.Contains(out, "func Add") { + t.Fatalf("minified read lost code:\n%s", out) + } +} + +func TestReadWithoutMinifyKeepsComments(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "demo.go") + src := "// keep\npackage demo\n" + if err := os.WriteFile(path, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + in, _ := json.Marshal(map[string]string{"path": path}) + out, err := FileReadTool{}.Execute(context.Background(), in) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "// keep") { + t.Fatalf("plain read should keep comments:\n%s", out) + } +}