refactor(util): address review feedback on writeAtomically

- Add optional onCleanupError callback so callers can log orphaned
  temp files; cliCredentialManager passes its logger to restore the
  warn-on-cleanup behavior that existed before the extraction.
- Document that writeAtomically requires the parent directory to
  exist.
- Use suffix "temp" so the call matches the doc example and the
  cliManager convention.
- Tighten the json writer's midstream-error test to assert the
  destination survives and no temp file is left behind.
- Fix the asyncIterable helper's docstring: it exercises async
  iteration, not stream backpressure.

Author: EhabY

src/core/cliCredentialManager.ts

@@ -266,8 +266,11 @@ export class CliCredentialManager {
 		content: string,
 	): Promise<void> {
 		await fs.mkdir(path.dirname(filePath), { recursive: true });
-		await writeAtomically(filePath, (tempPath) =>
-			fs.writeFile(tempPath, content, { mode: 0o600 }),
+		await writeAtomically(
+			filePath,
+			(tempPath) => fs.writeFile(tempPath, content, { mode: 0o600 }),
+			(rmErr, tempPath) =>
+				this.logger.warn("Failed to delete temp file", tempPath, rmErr),
 		);
 	}
 }

src/util/fs.ts

@@ -58,19 +58,26 @@ export function tempFilePath(basePath: string, suffix: string): string {
  * Writes `outputPath` atomically: `write` builds the content at a sibling
  * temp path, then we rename onto the destination. A failure mid-write never
  * touches the destination; the temp file is best-effort cleaned up.
+ *
+ * The parent directory of `outputPath` must already exist; if it doesn't,
+ * the `write` callback will surface an ENOENT on the temp path. Provide
+ * `onCleanupError` to be notified if the post-failure temp-file removal
+ * itself fails (e.g. for Windows-lock diagnostics); the original error is
+ * always rethrown regardless.
  */
 export async function writeAtomically<T>(
 	outputPath: string,
 	write: (tempPath: string) => Promise<T>,
+	onCleanupError?: (err: unknown, tempPath: string) => void,
 ): Promise<T> {
-	const tempPath = tempFilePath(outputPath, "tmp");
+	const tempPath = tempFilePath(outputPath, "temp");
 	try {
 		const result = await write(tempPath);
 		await renameWithRetry(fs.rename, tempPath, outputPath);
 		return result;
 	} catch (err) {
-		await fs.rm(tempPath, { force: true }).catch(() => {
-			// Surface the original failure, not the cleanup failure.
+		await fs.rm(tempPath, { force: true }).catch((rmErr) => {
+			onCleanupError?.(rmErr, tempPath);
 		});
 		throw err;
 	}

test/mocks/asyncIterable.ts

@@ -1,6 +1,6 @@
 /**
  * Wraps a sync array as an `AsyncIterable` that yields one item per microtask,
- * so consumers exercise the same backpressure path they would in production.
+ * so consumers exercise the same async iteration path they would in production.
  */
 export async function* asyncIterable<T>(
 	values: readonly T[],

test/unit/telemetry/export/writers/json.test.ts

@@ -44,13 +44,17 @@ describe("writeJsonArrayExport", () => {
 		expect(readOut()).toEqual([]);
 	});
 
-	it("propagates midstream errors", async () => {
+	it("leaves the destination untouched on midstream errors", async () => {
+		vol.writeFileSync(OUT, "previous");
 		const failing = (async function* () {
 			yield makeEvent();
 			await Promise.resolve();
 			throw new Error("boom");
 		})();
 
 		await expect(writeJsonArrayExport(OUT, failing)).rejects.toThrow(/boom/);
+
+		expect(vol.readFileSync(OUT, "utf8")).toBe("previous");
+		expect(vol.readdirSync("/exports")).toEqual(["telemetry.json"]);
 	});
 });