-
Notifications
You must be signed in to change notification settings - Fork 1
Remove retention limit support #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| //go:build !windows | ||
|
|
||
| package histree | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "syscall" | ||
| ) | ||
|
|
||
| func platformHasSufficientDiskSpace(dir string) (bool, error) { | ||
| var stat syscall.Statfs_t | ||
| if err := syscall.Statfs(dir, &stat); err != nil { | ||
| return false, fmt.Errorf("failed to stat filesystem: %w", err) | ||
| } | ||
|
|
||
| return stat.Bavail > 0, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||||||
| //go:build windows | ||||||||||
|
|
||||||||||
| package histree | ||||||||||
|
|
||||||||||
| import ( | ||||||||||
| "fmt" | ||||||||||
| "syscall" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
|
||||||||||
| // platformHasSufficientDiskSpace checks available disk space on Windows using the | |
| // GetDiskFreeSpaceEx system call and returns true if any free space is available. |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,11 @@ package histree | |||||||||||||
|
|
||||||||||||||
| import ( | ||||||||||||||
| "database/sql" | ||||||||||||||
| "errors" | ||||||||||||||
| "fmt" | ||||||||||||||
| "os" | ||||||||||||||
| "path/filepath" | ||||||||||||||
| "sync" | ||||||||||||||
| "time" | ||||||||||||||
|
|
||||||||||||||
| _ "github.com/mattn/go-sqlite3" | ||||||||||||||
|
|
@@ -39,10 +43,20 @@ type HistoryEntry struct { | |||||||||||||
| // DB represents a histree database connection | ||||||||||||||
| type DB struct { | ||||||||||||||
| *sql.DB | ||||||||||||||
| path string | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // OpenDB initializes and returns a new database connection | ||||||||||||||
| func OpenDB(dbPath string) (*DB, error) { | ||||||||||||||
| dir := filepath.Dir(dbPath) | ||||||||||||||
| if dir == "" { | ||||||||||||||
| dir = "." | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if err := ensureDirExists(dir); err != nil { | ||||||||||||||
| return nil, err | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| db, err := sql.Open("sqlite3", dbPath) | ||||||||||||||
| if err != nil { | ||||||||||||||
| return nil, fmt.Errorf("failed to open database: %w", err) | ||||||||||||||
|
|
@@ -58,14 +72,23 @@ func OpenDB(dbPath string) (*DB, error) { | |||||||||||||
| return nil, err | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return &DB{db}, nil | ||||||||||||||
| return &DB{DB: db, path: dbPath}, nil | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Close closes the database connection | ||||||||||||||
| func (db *DB) Close() error { | ||||||||||||||
| return db.DB.Close() | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // ErrInsufficientDiskSpace indicates that no additional history entries can be recorded | ||||||||||||||
| // because the underlying filesystem has no free space available. | ||||||||||||||
| var ErrInsufficientDiskSpace = errors.New("insufficient disk space for history entry") | ||||||||||||||
|
|
||||||||||||||
| var ( | ||||||||||||||
| diskSpaceCheckerMu sync.RWMutex | ||||||||||||||
| diskSpaceCheckerFn = hasSufficientDiskSpace | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| func setPragmas(db *sql.DB) error { | ||||||||||||||
| _, err := db.Exec(` | ||||||||||||||
| PRAGMA journal_mode = WAL; | ||||||||||||||
|
|
@@ -137,20 +160,90 @@ func createIndexes(tx *sql.Tx) error { | |||||||||||||
|
|
||||||||||||||
| // AddEntry adds a new command history entry to the database | ||||||||||||||
| func (db *DB) AddEntry(entry *HistoryEntry) error { | ||||||||||||||
| _, err := db.Exec( | ||||||||||||||
| hasSpace, err := checkDiskSpace(db.path) | ||||||||||||||
|
||||||||||||||
| if err != nil { | ||||||||||||||
| return fmt.Errorf("failed to check disk space: %w", err) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| if !hasSpace { | ||||||||||||||
| return fmt.Errorf("%w: unable to record command history entry in %s", ErrInsufficientDiskSpace, db.path) | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+163
to
+170
|
||||||||||||||
|
|
||||||||||||||
| if _, err := db.Exec( | ||||||||||||||
| "INSERT INTO history (command, directory, timestamp, exit_code, hostname, process_id) VALUES (?, ?, ?, ?, ?, ?)", | ||||||||||||||
| entry.Command, | ||||||||||||||
| entry.Directory, | ||||||||||||||
| entry.Timestamp, | ||||||||||||||
| entry.ExitCode, | ||||||||||||||
| entry.Hostname, | ||||||||||||||
| entry.ProcessID, | ||||||||||||||
| ) | ||||||||||||||
| if err != nil { | ||||||||||||||
| ); err != nil { | ||||||||||||||
| return fmt.Errorf("failed to insert entry: %w", err) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| return nil | ||||||||||||||
| } | ||||||||||||||
|
||||||||||||||
| } | |
| } |
Copilot
AI
Nov 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exported function SetDiskSpaceChecker and the package-level variables diskSpaceCheckerMu and diskSpaceCheckerFn are documented, but the internal helper function checkDiskSpace lacks documentation. Consider adding a comment explaining its purpose as a thread-safe wrapper around the configurable disk space checker.
| } | |
| } | |
| // checkDiskSpace is a thread-safe wrapper around the configurable disk space checker. | |
| // It acquires a read lock to safely access the current disk space checker function. |
Copilot
AI
Nov 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function lacks documentation. Consider adding a comment explaining that it's the default implementation for checking disk space that validates the directory exists before delegating to the platform-specific implementation.
| // hasSufficientDiskSpace is the default implementation for checking disk space. | |
| // It validates that the directory containing the database exists and is a directory | |
| // before delegating to the platform-specific implementation. |
Copilot
AI
Nov 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function lacks documentation. Consider adding a comment explaining that it creates the directory and all necessary parent directories if they don't exist, or validates that the path is a directory if it already exists.
| // ensureDirExists checks if the given path exists and is a directory. | |
| // If the directory does not exist, it creates the directory and all necessary parent directories. | |
| // If the path exists but is not a directory, it returns an error. | |
| // Returns nil if the directory exists or is successfully created, otherwise returns an error. |
Copilot
AI
Nov 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ensureDirExists function automatically creates directories with os.MkdirAll when they don't exist. This side effect in a disk space checking function (called from AddEntry) is unexpected and could lead to unintended directory creation. Consider either documenting this behavior clearly or separating directory creation from the disk space check to make the behavior more explicit and predictable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function lacks documentation. Consider adding a comment explaining that it checks available disk space on Unix-like systems using the Statfs system call and returns true if any available blocks exist.