fix(debuglog): don't crash run when archiving debug logs (#1522)#3329
Open
fantasyRqg wants to merge 1 commit into
Open
fix(debuglog): don't crash run when archiving debug logs (#1522)#3329fantasyRqg wants to merge 1 commit into
fantasyRqg wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Fixes a crash where a successful run could fail at the very end with
java.nio.file.NoSuchFileExceptioninDebugLogStore.finalizeRun()→FileUtils.zipDir()→Files.walk().Root cause:
DebugLogStore.removeOldLogs()runs at startup over the shared~/Library/Logs/maestro/directory and deleted every entry beyond the 6 newest —including the live working directory of another concurrently-running
maestroprocess (a separate invocation, or CLI + Studio). The owning process then tried
to zip its now-deleted directory and crashed. Long runs are most exposed, since
they drop out of the keep-window while still active.
Changes:
DebugLogStore): replaced count-based pruning withpruneLogs()— keep the newest N.ziparchives (completed runs) and reapworking directories only when older than 24h (orphans from crashed runs).
A live run's freshly-modified directory is never deleted, removing the race at
its source. Crashed-run leftovers are still reclaimed.
FileUtils.zipDir: return early with a warning when the sourcedirectory does not exist, closing the check-then-walk window.
finalizeRun: wrap archival in try/catch that warns andcontinues — debug-log cleanup can never crash a completed run.
printStackTrace()inzipDirwith a proper warning.This is a bug fix (Type A); the one extracted helper (
pruneLogs) exists to makethe retention policy unit-testable and is not a broader refactor.
Testing
Unit tests added and run via
./gradlew :maestro-client:test(all green):FileUtilsTest:zipDiron a missing source no longer throws and leaves nooutput file (this test reproduces the original crash as a red test); happy-path
zip/unzip still works.
DebugLogStoreTest:pruneLogskeeps the newest archives, deletes a staleorphan working dir while leaving a fresh (live) dir untouched, and no-ops on a
missing base directory.
Lint:
./gradlew detektpasses.Issues fixed
Fixes #1522