fix: Windows command quoting, atomic JSON/log writes, restore dropped test
Implements items 1-3 of the whole-project review's suggested order (docs/PROJECT_REVIEW_PLAN.md): - Restore TestJobListViewIsCompact, accidentally dropped by 5b0e6fe; drop the redundant TestDefaultConfigUsesDetailedJobList row from TESTS.md and document the two other doc gaps the review found. - Fix quoteLeadingWindowsProgramPath to find the earliest file-extension match at a word boundary instead of the first extension in list order, so a .bat/.cmd command whose argument ends in .exe no longer has its whole command line mistaken for the program path. - Write gosentry.json, jobs.json, and run log files atomically (temp file + rename) so a crash or power loss mid-write can no longer leave a truncated file. Wire Service.Stop() into the app shutdown path so it actually runs, cancelling the run context for in-flight runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+43
-5
@@ -230,7 +230,8 @@ func (s *Store) applyConfigPaths() {
|
||||
}
|
||||
|
||||
func writeJSON(path string, value any) error {
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||
dir := filepath.Dir(path)
|
||||
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||
return err
|
||||
}
|
||||
data, err := json.MarshalIndent(value, "", " ")
|
||||
@@ -240,10 +241,47 @@ func writeJSON(path string, value any) error {
|
||||
// A trailing newline keeps the file friendly to editors and diff tools that
|
||||
// expect text files to end with one.
|
||||
data = append(data, '\n')
|
||||
// WriteFile replaces the full file instead of patching it in place. For small
|
||||
// JSON files this is simpler and prevents stale keys from older versions from
|
||||
// lingering after the schema changes.
|
||||
return os.WriteFile(path, data, 0o644)
|
||||
return writeFileAtomic(dir, path, data, 0o644)
|
||||
}
|
||||
|
||||
// writeFileAtomic writes data to a temp file in dir, syncs it, then renames it
|
||||
// over path. Rename is atomic within a volume on both supported platforms, so
|
||||
// a crash, a power loss, or the process being killed mid-write can never leave
|
||||
// path holding a truncated or empty file the way a direct os.WriteFile could.
|
||||
func writeFileAtomic(dir, path string, data []byte, perm os.FileMode) error {
|
||||
tmp, err := os.CreateTemp(dir, filepath.Base(path)+".tmp*")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
tmpPath := tmp.Name()
|
||||
// Any failure past this point must remove the temp file rather than leave
|
||||
// it behind for the next write to trip over.
|
||||
success := false
|
||||
defer func() {
|
||||
if !success {
|
||||
os.Remove(tmpPath)
|
||||
}
|
||||
}()
|
||||
|
||||
if _, err := tmp.Write(data); err != nil {
|
||||
tmp.Close()
|
||||
return err
|
||||
}
|
||||
if err := tmp.Sync(); err != nil {
|
||||
tmp.Close()
|
||||
return err
|
||||
}
|
||||
if err := tmp.Close(); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.Chmod(tmpPath, perm); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := os.Rename(tmpPath, path); err != nil {
|
||||
return err
|
||||
}
|
||||
success = true
|
||||
return nil
|
||||
}
|
||||
|
||||
func defaultJobs() []domain.Job {
|
||||
|
||||
@@ -430,3 +430,44 @@ func TestJobsJSONDoesNotPersistRuntimeNoise(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestWriteJSONReplacesFileAtomically pins the durability fix: writeJSON must
|
||||
// never truncate the destination in place. It writes through a temp file and
|
||||
// renames over the target, so a reader can never observe a partially written
|
||||
// file, and an existing file survives untouched if the marshal fails first.
|
||||
func TestWriteJSONReplacesFileAtomically(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "gosentry.json")
|
||||
|
||||
original := domain.DefaultConfig()
|
||||
original.LogsDir = "logs-original"
|
||||
if err := writeJSON(path, original); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
updated := domain.DefaultConfig()
|
||||
updated.LogsDir = "logs-updated"
|
||||
if err := writeJSON(path, updated); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var got domain.Config
|
||||
if err := json.Unmarshal(data, &got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.LogsDir != "logs-updated" {
|
||||
t.Fatalf("LogsDir = %q, want %q", got.LogsDir, "logs-updated")
|
||||
}
|
||||
|
||||
entries, err := os.ReadDir(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("expected only the final file in %s, got %v", dir, entries)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user