Don't persist the run-summary.md file beyond its own run
#503
Conversation
This was something we missed from scala-steward-org#502 - because the `run-summary.md` file is saved by scala-steward-org/scala-steward#3071 to the Scala Steward workspace, and almost the entire workspace is persisted to cache by Scala Steward's GitHub Action, the report would be persisted until the _next_ run of the GitHub Action, and concievably, if the next run went wrong somehow, the incorrect _old_ summary could get reported against that subsequent run, which would be bad. The fix here is to delete the summary at the `saveWorkspaceCache()` step, which occurs after the summary has been transferred to the `GITHUB_STEP_SUMMARY` file by calling `core.summary()`.
|
A snapshot release has been created as You can test it out with: uses: scala-steward-org/scala-steward-action@snapshots/503It will be automatically recreated on any change to this PR. |
|
There was a problem hiding this comment.
Great catch @rtyley! I fixed the failing lint & test so we can release this as soon as possible. Thank you very much!
|
There is also some cleanup happening before every run in Scala Steward: https://github.com/scala-steward-org/scala-steward/blob/49b1a5f2abb80c8a4a01517827c873b580004fca/modules/core/src/main/scala/org/scalasteward/core/application/StewardAlg.scala#L95 |
I think that makes sense, I'll raise a PR for that. |
|
🤔 If we remove it there it won't be available on the action, right? |
|
I think it should be since Scala Steward would only delete an old summary before it runs and the action picks up the current summary after a run has finished. |
To ensure that its workspace is clean, and that unintended files are not held over from previous runs, Scala Steward already erases the `repos` folder before starting its run. With new `run-summary.md` file introduced by scala-steward-org#3071, we again want to be careful that the summary file from a previous run isn't picked up in a subsequent run. scala-steward-org/scala-steward-action#503 addressed this for the particular concern of GitHub Actions, by ensuring that the file wasn't persisted to cache, but @fthomas pointed out that this additional point at the start of Scala Steward's code was also an appropriate place for removing any run-specific files: scala-steward-org/scala-steward-action#503 (comment)
To ensure that its workspace is clean, and that unintended files are not held over from previous runs, Scala Steward already erases the `repos` folder before starting its run. With new `run-summary.md` file introduced by scala-steward-org#3071, we again want to be careful that the summary file from a previous run isn't picked up in a subsequent run. scala-steward-org/scala-steward-action#503 addressed this for the particular concern of GitHub Actions, by ensuring that the file wasn't persisted to cache, but @fthomas pointed out that this additional point at the start of Scala Steward's code was also an appropriate place for removing any run-specific files: scala-steward-org/scala-steward-action#503 (comment)
Oh, sorry, you're right, my brain read |
PR raised for clearing-out run-specific files like |
To ensure that its workspace is clean, and that unintended files are not held over from previous runs, Scala Steward already erases the `repos` folder before starting its run. With new `run-summary.md` file introduced by #3071, we again want to be careful that the summary file from a previous run isn't picked up in a subsequent run. scala-steward-org/scala-steward-action#503 addressed this for the particular concern of GitHub Actions, by ensuring that the file wasn't persisted to cache, but @fthomas pointed out that this additional point at the start of Scala Steward's code was also an appropriate place for removing any run-specific files: scala-steward-org/scala-steward-action#503 (comment)
This was something we missed from #502 - because the
run-summary.mdfile (introduced by scala-steward-org/scala-steward#3071) is saved to the Scala Steward workspace, and almost the entire workspace is persisted to cache by Scala Steward's GitHub Action, the report would be persisted until the next run of the GitHub Action.Conceivably, if the next run went wrong somehow and didn't overwrite the old file, the old summary could get reported against the subsequent run, which would be bad.
The fix here is to delete the summary at the
saveWorkspaceCache()step, which occurs after the summary has been transferred to theGITHUB_STEP_SUMMARYfile by callingcore.summary().