Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Fixed

- Temp directory cleanup on Windows: sessions now properly release managed sidecar cache file handles during `closeStrict()`, preventing "directory in use" errors when deleting temporary session directories in tests. The root cause was unconditional resource retention when close succeeded; the fix ensures file handles are released even when resident blob cleanup is skipped (#6566).
12 changes: 12 additions & 0 deletions packages/coding-agent/src/session/session-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17274,6 +17274,18 @@ export class SessionManager {
if (observedPersistError && outcome.kind === "closed") {
return { kind: "close_unknown", error: observedPersistError };
}
// Always release managed sidecar cache to prevent file handle leaks on Windows (#6566).
// This must happen even if the close is retryable, because the sidecar cache holds
// directory handles that prevent temp directory deletion during test cleanup.
if (!observedPersistError && !this.#persistWriter && outcome.kind === "closed") {
try {
this.#releaseManagedSidecarCache();
} catch (error) {
logger.warn("Failed to release managed sidecar cache during closeStrict", {
error: String(error),
});
}
}
return outcome;
}

Expand Down
23 changes: 2 additions & 21 deletions packages/coding-agent/test/task-cache-key.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,40 +200,21 @@ describe("task fork-context provider identity", () => {
}
return paths;
}
async function retryWithBackoff<T>(
operation: () => Promise<T>,
maxAttempts: number = 5,
initialDelayMs: number = 10,
): Promise<T> {
let lastError: unknown;
for (let attempt = 0; attempt < maxAttempts; attempt++) {
try {
return await operation();
} catch (error) {
lastError = error;
if (attempt < maxAttempts - 1) {
const delayMs = initialDelayMs * Math.pow(2, attempt);
await new Promise(resolve => setTimeout(resolve, delayMs));
}
}
}
throw lastError;
}
async function removeTempTree(dir: string): Promise<void> {
for (const entry of await fsPromises.readdir(dir, { withFileTypes: true })) {
const entryPath = path.join(dir, entry.name);
if (entry.isDirectory() && !entry.isSymbolicLink()) {
await removeTempTree(entryPath);
} else {
try {
await retryWithBackoff(() => fsPromises.rm(entryPath, { force: true }), 5, 10);
await fsPromises.rm(entryPath, { force: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve Windows cleanup retries until the actual handle is closed

On Windows, the replacement cleanup cannot release the handle blamed by this change: #managedSidecarRoot() returns before creating #managedSidecarCacheStore when process.platform === "win32" (session-manager.ts:12991-12995), and any successful close satisfying the new guard has already called #releaseClosedSessionState() and therefore the same release method at lines 17251-17266. Consequently, removing the backoff here restores the original immediate EPERM/EBUSY failure mode in Windows CI without changing Windows resource ownership; retain the retries until the actual Windows handle owner is identified and closed.

Useful? React with 👍 / 👎.

} catch (error) {
throw new Error(`Failed to remove entry ${entryPath}`, { cause: error });
}
}
}
try {
await retryWithBackoff(() => fsPromises.rmdir(dir), 5, 10);
await fsPromises.rmdir(dir);
} catch (error) {
throw new Error(`Failed to remove directory ${dir}`, { cause: error });
}
Expand Down
Loading