Skip to content

fix: retry DSH GitHub checkout cleanup on Windows - #124

Merged
Mr-Lucky merged 1 commit into
mainfrom
fix/dsh-windows-ebusy-cleanup
Sep 2, 2026
Merged

fix: retry DSH GitHub checkout cleanup on Windows#124
Mr-Lucky merged 1 commit into
mainfrom
fix/dsh-windows-ebusy-cleanup

Conversation

@Mr-Lucky

@Mr-Lucky Mr-Lucky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix transient Windows EBUSY failures when cleaning up temporary DSH GitHub checkouts.

  • Retry recursive temporary-directory removal with linear backoff.
  • Wait for in-flight acquisition budget checks before starting cleanup.
  • Preserve successful scan results when cleanup still fails after retries.
  • Surface remaining cleanup failures as operational warnings in JSON, Markdown, and HTML reports.
  • Preserve the original scan or acquisition error when cleanup also fails.

Type

  • Bug fix
  • New feature / detection rule
  • Refactoring
  • Documentation

Testing

  • npm run build passes
  • npm test passes (601 tests)
  • Manually tested the change

Related Issues

Closes #

@Mr-Lucky
Mr-Lucky merged commit 52ab3d4 into main Sep 2, 2026
4 checks passed
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

AgentGuard PR Review

I found one actionable regression introduced by this patch.

  1. severity: mediumsrc/dsh/source.ts (runWithDshCleanup, resolveDshSource catch block)
    • What can go wrong: Cleanup errors are now converted into a warning and the original source-failure path is rethrown. However, when resolveDshSource() fails after creating tempRoot, the cleanup warning is computed via runWithDshCleanup(async () => undefined, ...), which swallows the actual cleanup error details and only reports a generic warning string. More importantly, if cleanup itself fails during a fetch error, the thrown error message can now change from the prior stable "Failed to fetch GitHub repository: ..." to a combined message that may leak filesystem/runtime details into logs and user-visible output. This is a correctness and information-disclosure regression for security-sensitive fetch failures.
    • Fix suggestion: Keep the original fetch error message unchanged and attach cleanup failure as a separate non-user-facing field/log entry. If a warning is needed, propagate a structured cleanup error object rather than appending it to the thrown message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants