fix: retry DSH GitHub checkout cleanup on Windows

src/dsh/scan.ts

@@ -17,7 +17,7 @@ import { getDshScannerMetadata } from './metadata.js';
 import { inspectRegularFileWithinRoot } from '../scanner/safe-file.js';
 import { walkDirectoryWithCoverage } from '../scanner/file-walker.js';
 import { addFindingContext, calculateReviewPriority, runtimeSurfaceTags } from './finding-context.js';
-import { resolveDshSource } from './source.js';
+import { resolveDshSource, runWithDshCleanup } from './source.js';
 import type {
   DshCapabilityProfile,
   DshFinding,
@@ -157,7 +157,7 @@ export async function scanDshPlugin(
   options: ScanDshPluginOptions = {},
 ): Promise<DshPluginScanReport> {
   const source = await resolveDshSource(input, options);
-  try {
+  const { value: report, cleanupWarning } = await runWithDshCleanup(async (): Promise<DshPluginScanReport> => {
     const directory = await walkDirectoryWithCoverage(source.rootDir, { includeGeneratedRuntime: true });
     const detection = await detectDshPlugin(source.rootDir, directory.files);
     const capabilityProfile = await buildCapabilityProfile(source.rootDir, detection, directory.files);
@@ -304,9 +304,9 @@ export async function scanDshPlugin(
         cordisParseErrors: detection.cordis.parseErrors,
       },
     };
-  } finally {
-    await source.cleanup();
-  }
+  }, source.cleanup);
+  if (cleanupWarning) report.diagnostics.cleanupWarning = cleanupWarning;
+  return report;
 }
 
 export { DSH_RULES, RULES as DSH_SCAN_RULES };

src/dsh/source.ts

@@ -9,6 +9,20 @@ const GITHUB_REPO = /^https:\/\/github\.com\/([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+
 export const MAX_GITHUB_ACQUISITION_BYTES = 256 * 1024 * 1024;
 export const MAX_GITHUB_OBJECTS = 100_000;
 const ACQUISITION_POLL_MS = 100;
+const CLEANUP_MAX_RETRIES = 8;
+const CLEANUP_RETRY_DELAY_MS = 100;
+
+export async function removeDshTempRoot(
+  tempRoot: string,
+  remove: typeof rm = rm,
+): Promise<void> {
+  await remove(tempRoot, {
+    recursive: true,
+    force: true,
+    maxRetries: CLEANUP_MAX_RETRIES,
+    retryDelay: CLEANUP_RETRY_DELAY_MS,
+  });
+}
 
 async function directoryBytesWithinBudget(rootDir: string, maxBytes: number): Promise<number> {
   const pending = [rootDir];
@@ -46,30 +60,53 @@ export async function assertDshAcquisitionByteBudget(
   }
 }
 
+interface KillableGitChild {
+  readonly killed: boolean;
+  kill(signal?: NodeJS.Signals | number): boolean;
+}
+
+export function startDshAcquisitionMonitor(
+  child: KillableGitChild,
+  checkBudget: () => Promise<void>,
+  pollMs = ACQUISITION_POLL_MS,
+): { stop(): Promise<Error | undefined> } {
+  let budgetError: Error | undefined;
+  let activeCheck: Promise<void> | undefined;
+  const monitor = setInterval(() => {
+    if (activeCheck || child.killed) return;
+    activeCheck = checkBudget().catch(error => {
+      budgetError = error as Error;
+      child.kill('SIGKILL');
+    }).finally(() => {
+      activeCheck = undefined;
+    });
+  }, pollMs);
+  return {
+    async stop(): Promise<Error | undefined> {
+      clearInterval(monitor);
+      await activeCheck;
+      return budgetError;
+    },
+  };
+}
+
 function execBoundedGit(args: string[], rootDir: string, timeout: number): Promise<{ stdout: string; stderr: string }> {
   return new Promise((resolvePromise, reject) => {
-    let budgetError: Error | undefined;
-    let checking = false;
     const child = execFile('git', args, {
       timeout,
       maxBuffer: 4 * 1024 * 1024,
       env: { ...process.env, GIT_TERMINAL_PROMPT: '0' },
     }, (error, stdout, stderr) => {
-      clearInterval(monitor);
-      if (budgetError) reject(budgetError);
-      else if (error) reject(error);
-      else resolvePromise({ stdout, stderr });
+      void monitor.stop().then(budgetError => {
+        if (budgetError) reject(budgetError);
+        else if (error) reject(error);
+        else resolvePromise({ stdout, stderr });
+      }, reject);
     });
-    const monitor = setInterval(() => {
-      if (checking || child.killed) return;
-      checking = true;
-      void assertDshAcquisitionByteBudget(rootDir).catch(error => {
-        budgetError = error as Error;
-        child.kill('SIGKILL');
-      }).finally(() => {
-        checking = false;
-      });
-    }, ACQUISITION_POLL_MS);
+    const monitor = startDshAcquisitionMonitor(
+      child,
+      () => assertDshAcquisitionByteBudget(rootDir),
+    );
   });
 }
 
@@ -111,6 +148,45 @@ export interface ResolveDshSourceOptions {
   ref?: string;
 }
 
+export interface DshCleanupResult<T> {
+  value: T;
+  cleanupWarning?: string;
+}
+
+function describeCleanupFailure(error: unknown): string {
+  const code = (error as NodeJS.ErrnoException).code;
+  return `Temporary GitHub checkout cleanup failed${code ? ` (${code})` : ''}; files may remain in the OS temporary directory`;
+}
+
+export async function runWithDshCleanup<T>(
+  operation: () => Promise<T>,
+  cleanup: () => Promise<void>,
+): Promise<DshCleanupResult<T>> {
+  let value!: T;
+  let operationFailed = false;
+  let operationError: unknown;
+  try {
+    value = await operation();
+  } catch (error) {
+    operationFailed = true;
+    operationError = error;
+  }
+  let cleanupWarning: string | undefined;
+  try {
+    await cleanup();
+  } catch (error) {
+    cleanupWarning = describeCleanupFailure(error);
+  }
+  if (operationFailed) {
+    if (cleanupWarning) {
+      const message = operationError instanceof Error ? operationError.message : String(operationError);
+      throw new Error(`${message}; ${cleanupWarning}`, { cause: operationError });
+    }
+    throw operationError;
+  }
+  return { value, cleanupWarning };
+}
+
 async function gitMetadata(rootDir: string): Promise<{ revision?: string; lastCommitAt?: string }> {
   try {
     const [{ stdout: revision }, { stdout: lastCommitAt }] = await Promise.all([
@@ -240,11 +316,15 @@ export async function resolveDshSource(
         repositoryUrl,
         requestedRef,
         ...metadata,
-        cleanup: () => rm(tempRoot, { recursive: true, force: true }),
+        cleanup: () => removeDshTempRoot(tempRoot),
       };
     } catch (error) {
-      await rm(tempRoot, { recursive: true, force: true });
-      throw new Error(`Failed to fetch GitHub repository: ${(error as Error).message}`);
+      const { cleanupWarning } = await runWithDshCleanup(
+        async () => undefined,
+        () => removeDshTempRoot(tempRoot),
+      );
+      const fetchMessage = `Failed to fetch GitHub repository: ${(error as Error).message}`;
+      throw new Error(cleanupWarning ? `${fetchMessage}; ${cleanupWarning}` : fetchMessage, { cause: error });
     }
   }
 

src/dsh/types.ts

@@ -188,5 +188,7 @@ export interface DshPluginScanReport {
   diagnostics: {
     packageParseError?: string;
     cordisParseErrors: Array<{ file: string; message: string }>;
+    /** Operational warning only; does not affect the security risk result. */
+    cleanupWarning?: string;
   };
 }

src/reports/dsh-report.ts

@@ -61,6 +61,9 @@ export function renderDshMarkdown(report: DshPluginScanReport): string {
   const runtimeSurfaceRisk = report.runtimeSurfaceRiskLevel ?? report.riskLevel;
   const runtimeSurfaceRecommendation = report.runtimeSurfaceRecommendation ?? report.installRecommendation;
   const scanCoverage = report.scanCoverage;
+  const cleanupWarning = report.diagnostics.cleanupWarning
+    ? `- Cleanup warning: ${markdownEscape(report.diagnostics.cleanupWarning)}\n`
+    : '';
   const capabilities = Object.entries(report.capabilityProfile)
     .map(([key, enabled]) => `| ${CAPABILITY_LABELS[key as keyof typeof CAPABILITY_LABELS]} | ${enabled ? 'Yes' : 'No'} |`)
     .join('\n');
@@ -137,7 +140,7 @@ ${findings}
 - Artifact hash: ${report.identity.artifactHash ?? 'Unknown'}
 - Scanned at: ${report.scannedAt}
 - Files scanned: ${report.filesScanned}
-- Scan coverage: ${scanCoverage
+${cleanupWarning}- Scan coverage: ${scanCoverage
     ? `${scanCoverage.complete ? 'complete' : 'INCOMPLETE'} (${scanCoverage.scanned}/${scanCoverage.discovered}; skipped ${scanCoverage.skipped}: file limit ${scanCoverage.skippedByReason.fileLimit}, oversized ${scanCoverage.skippedByReason.oversized}, unreadable ${scanCoverage.skippedByReason.unreadable})`
     : 'Unavailable in legacy schema-v1 report'}
 
@@ -155,6 +158,9 @@ export function renderDshHtml(report: DshPluginScanReport): string {
   const runtimeSurfaceRisk = report.runtimeSurfaceRiskLevel ?? report.riskLevel;
   const runtimeSurfaceRecommendation = report.runtimeSurfaceRecommendation ?? report.installRecommendation;
   const scanCoverage = report.scanCoverage;
+  const cleanupWarning = report.diagnostics.cleanupWarning
+    ? `<dt>Cleanup warning</dt><dd>${htmlEscape(report.diagnostics.cleanupWarning)}</dd>`
+    : '';
   const scanCoverageText = scanCoverage
     ? `${scanCoverage.complete ? 'Complete' : 'INCOMPLETE'} — ${scanCoverage.scanned}/${scanCoverage.discovered} scanned; ${scanCoverage.skipped} skipped (file limit ${scanCoverage.skippedByReason.fileLimit}, oversized ${scanCoverage.skippedByReason.oversized}, unreadable ${scanCoverage.skippedByReason.unreadable})`
     : 'Unavailable in legacy schema-v1 report';
@@ -213,7 +219,7 @@ export function renderDshHtml(report: DshPluginScanReport): string {
   <div class="grid">
     <section style="grid-column:1/-1"><h2>Key findings</h2>${findings}</section>
     <section class="recommendation"><h2>Install recommendation</h2><strong>${htmlEscape(RECOMMENDATIONS[report.installRecommendation])}</strong><p>Runtime surface: ${htmlEscape(RECOMMENDATIONS[runtimeSurfaceRecommendation])}</p>${report.harmlessMismatch ? '<p>Looks harmless, but requests elevated capabilities.</p>' : ''}</section>
-    <section><h2>Artifact</h2><dl><dt>Repository</dt><dd>${htmlEscape(report.project.repositoryUrl ?? 'Local directory')}</dd><dt>Requested ref</dt><dd>${htmlEscape(report.source.requestedRef ?? (report.source.kind === 'github' ? 'Default branch HEAD' : 'Not applicable'))}</dd><dt>Resolved revision</dt><dd>${htmlEscape(report.source.revision ?? 'Unknown')}</dd><dt>Last commit</dt><dd>${htmlEscape(report.source.lastCommitAt ?? 'Unknown')}</dd><dt>Files scanned</dt><dd>${report.filesScanned}</dd><dt>Scan coverage</dt><dd>${htmlEscape(scanCoverageText)}</dd><dt>Scanned</dt><dd>${htmlEscape(report.scannedAt)}</dd><dt>Hash</dt><dd>${htmlEscape(report.identity.artifactHash ?? 'Unknown')}</dd></dl></section>
+    <section><h2>Artifact</h2><dl><dt>Repository</dt><dd>${htmlEscape(report.project.repositoryUrl ?? 'Local directory')}</dd><dt>Requested ref</dt><dd>${htmlEscape(report.source.requestedRef ?? (report.source.kind === 'github' ? 'Default branch HEAD' : 'Not applicable'))}</dd><dt>Resolved revision</dt><dd>${htmlEscape(report.source.revision ?? 'Unknown')}</dd><dt>Last commit</dt><dd>${htmlEscape(report.source.lastCommitAt ?? 'Unknown')}</dd><dt>Files scanned</dt><dd>${report.filesScanned}</dd>${cleanupWarning}<dt>Scan coverage</dt><dd>${htmlEscape(scanCoverageText)}</dd><dt>Scanned</dt><dd>${htmlEscape(report.scannedAt)}</dd><dt>Hash</dt><dd>${htmlEscape(report.identity.artifactHash ?? 'Unknown')}</dd></dl></section>
   </div>
   <footer>Static analysis can miss runtime-loaded behavior and cannot prove that a plugin is safe.</footer>
 </main></body></html>`;

src/tests/dsh-source.test.ts

@@ -0,0 +1,105 @@
+import assert from 'node:assert/strict';
+import { describe, it } from 'node:test';
+import { access, mkdtemp, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import {
+  removeDshTempRoot,
+  runWithDshCleanup,
+  startDshAcquisitionMonitor,
+} from '../dsh/source.js';
+import { scanDshPlugin } from '../dsh/scan.js';
+import { renderDshHtml, renderDshMarkdown } from '../reports/dsh-report.js';
+
+function busyError(): NodeJS.ErrnoException {
+  const error = new Error('resource busy or locked') as NodeJS.ErrnoException;
+  error.code = 'EBUSY';
+  return error;
+}
+
+describe('DSH GitHub source cleanup', () => {
+  it('enables retries when requesting recursive temporary directory removal', async () => {
+    let receivedOptions: Parameters<typeof rm>[1];
+    await removeDshTempRoot('temporary-checkout', async (_path, options) => {
+      receivedOptions = options;
+    });
+
+    assert.equal(receivedOptions!.recursive, true);
+    assert.equal(receivedOptions!.force, true);
+    assert.ok((receivedOptions!.maxRetries ?? 0) > 0);
+    assert.ok((receivedOptions!.retryDelay ?? 0) > 0);
+  });
+
+  it('waits for an in-flight acquisition budget check before stopping', async () => {
+    let markStarted!: () => void;
+    const started = new Promise<void>(resolve => { markStarted = resolve; });
+    let releaseCheck!: () => void;
+    const release = new Promise<void>(resolve => { releaseCheck = resolve; });
+    const child = { killed: false, kill: () => true };
+    const monitor = startDshAcquisitionMonitor(child, async () => {
+      markStarted();
+      await release;
+    }, 1);
+    await started;
+
+    let stopped = false;
+    const stopping = monitor.stop().then(error => {
+      stopped = true;
+      return error;
+    });
+    await new Promise<void>(resolve => setImmediate(resolve));
+    assert.equal(stopped, false);
+
+    releaseCheck();
+    assert.equal(await stopping, undefined);
+    assert.equal(stopped, true);
+  });
+
+  it('removes a temporary GitHub checkout tree', async () => {
+    const tempRoot = await mkdtemp(join(tmpdir(), 'agentguard-dsh-cleanup-test-'));
+    await writeFile(join(tempRoot, 'pack-file.pack'), 'pack');
+
+    await removeDshTempRoot(tempRoot);
+
+    await assert.rejects(() => access(tempRoot), { code: 'ENOENT' });
+  });
+
+  it('returns a completed operation when temporary cleanup remains busy', async () => {
+    const result = await runWithDshCleanup(
+      async () => ({ report: 'complete' }),
+      async () => { throw busyError(); },
+    );
+
+    assert.deepEqual(result.value, { report: 'complete' });
+    assert.match(result.cleanupWarning ?? '', /EBUSY/);
+  });
+
+  it('preserves the operation failure when temporary cleanup also fails', async () => {
+    await assert.rejects(
+      () => runWithDshCleanup(
+        async () => { throw new Error('scan failed'); },
+        async () => { throw busyError(); },
+      ),
+      error => {
+        assert.match((error as Error).message, /^scan failed/);
+        assert.match((error as Error).message, /cleanup failed \(EBUSY\)/);
+        return true;
+      },
+    );
+  });
+
+  it('renders a non-fatal temporary cleanup warning in scan reports', async () => {
+    const root = await mkdtemp(join(tmpdir(), 'agentguard-dsh-cleanup-report-test-'));
+    try {
+      await writeFile(join(root, 'package.json'), JSON.stringify({ name: 'cleanup-report-test' }));
+      const report = await scanDshPlugin(root);
+      report.diagnostics.cleanupWarning = 'Temporary GitHub checkout cleanup failed (EBUSY)';
+
+      assert.match(renderDshMarkdown(report), /Cleanup warning:.*EBUSY/);
+      assert.match(renderDshHtml(report), /Cleanup warning.*EBUSY/);
+      assert.equal(report.riskLevel, 'low');
+    } finally {
+      await rm(root, { recursive: true, force: true });
+    }
+  });
+});