Skip to content

Commit bed765a

Browse files
EhabYEhabY
authored
Add proxy log file cleanup (#736)
- Adds cleanup for proxy log files (`coder-ssh-*.log`) in the proxy log directory - Only triggers when file count exceeds 20 AND oldest files are more than 7 days old - Deletes oldest stale files (by mtime) to bring count back to 20 - Runs on monitor start alongside existing network info file cleanup Fixes #377
1 parent 2f8782c commit bed765a

File tree

2 files changed

+228
-33
lines changed

2 files changed

+228
-33
lines changed

‎src/remote/sshProcess.ts‎

Lines changed: 104 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ export interface SshProcessMonitorOptions {
4040
}
4141

4242
// 1 hour cleanup threshold for old network info files
43-
const CLEANUP_MAX_AGE_MS = 60 * 60 * 1000;
43+
const CLEANUP_NETWORK_MAX_AGE_MS = 60 * 60 * 1000;
44+
// 7 day cleanup threshold for old proxy log files
45+
const CLEANUP_LOG_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000;
46+
// Maximum number of proxy log files to keep during cleanup
47+
const CLEANUP_MAX_LOG_FILES = 20;
4448

4549
/**
4650
* Monitors the SSH process for a Coder workspace connection and displays
@@ -74,49 +78,110 @@ export class SshProcessMonitor implements vscode.Disposable {
7478
private lastStaleSearchTime = 0;
7579

7680
/**
77-
* Cleans up network info files older than the specified age.
81+
* Helper to clean up files in a directory.
82+
* Stats files in parallel, applies selection criteria, then deletes in parallel.
7883
*/
79-
private static async cleanupOldNetworkFiles(
80-
networkInfoPath: string,
81-
maxAgeMs: number,
84+
private static async cleanupFiles(
85+
dir: string,
86+
fileType: string,
8287
logger: Logger,
88+
options: {
89+
filter: (name: string) => boolean;
90+
select: (
91+
files: Array<{ name: string; mtime: number }>,
92+
now: number,
93+
) => Array<{ name: string }>;
94+
},
8395
): Promise<void> {
8496
try {
85-
const files = await fs.readdir(networkInfoPath);
8697
const now = Date.now();
98+
const files = await fs.readdir(dir);
99+
100+
// Gather file stats in parallel
101+
const withStats = await Promise.all(
102+
files.filter(options.filter).map(async (name) => {
103+
try {
104+
const stats = await fs.stat(path.join(dir, name));
105+
return { name, mtime: stats.mtime.getTime() };
106+
} catch (error) {
107+
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
108+
logger.debug(`Failed to stat ${fileType} ${name}`, error);
109+
}
110+
return null;
111+
}
112+
}),
113+
);
87114

88-
const deletedFiles: string[] = [];
89-
for (const file of files) {
90-
if (!file.endsWith(".json")) {
91-
continue;
92-
}
93-
94-
const filePath = path.join(networkInfoPath, file);
95-
try {
96-
const stats = await fs.stat(filePath);
97-
const ageMs = now - stats.mtime.getTime();
115+
const toDelete = options.select(
116+
withStats.filter((f) => f !== null),
117+
now,
118+
);
98119

99-
if (ageMs > maxAgeMs) {
100-
await fs.unlink(filePath);
101-
deletedFiles.push(file);
102-
}
103-
} catch (error) {
104-
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
105-
logger.debug(`Failed to clean up network info file ${file}`, error);
120+
// Delete files in parallel
121+
const results = await Promise.all(
122+
toDelete.map(async (file) => {
123+
try {
124+
await fs.unlink(path.join(dir, file.name));
125+
return file.name;
126+
} catch (error) {
127+
if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
128+
logger.debug(`Failed to delete ${fileType} ${file.name}`, error);
129+
}
130+
return null;
106131
}
107-
}
108-
}
132+
}),
133+
);
109134

135+
const deletedFiles = results.filter((name) => name !== null);
110136
if (deletedFiles.length > 0) {
111137
logger.debug(
112-
`Cleaned up ${deletedFiles.length} old network info file(s): ${deletedFiles.join(", ")}`,
138+
`Cleaned up ${deletedFiles.length} ${fileType}(s): ${deletedFiles.join(", ")}`,
113139
);
114140
}
115141
} catch {
116142
// Directory may not exist yet, ignore
117143
}
118144
}
119145

146+
/**
147+
* Cleans up network info files older than the specified age.
148+
*/
149+
private static async cleanupOldNetworkFiles(
150+
networkInfoPath: string,
151+
maxAgeMs: number,
152+
logger: Logger,
153+
): Promise<void> {
154+
await SshProcessMonitor.cleanupFiles(
155+
networkInfoPath,
156+
"network info file",
157+
logger,
158+
{
159+
filter: (name) => name.endsWith(".json"),
160+
select: (files, now) => files.filter((f) => now - f.mtime > maxAgeMs),
161+
},
162+
);
163+
}
164+
165+
/**
166+
* Cleans up old proxy log files that exceed the retention limit.
167+
* Only deletes files older than maxAgeMs when file count exceeds maxFilesToKeep.
168+
*/
169+
private static async cleanupOldLogFiles(
170+
logDir: string,
171+
maxFilesToKeep: number,
172+
maxAgeMs: number,
173+
logger: Logger,
174+
): Promise<void> {
175+
await SshProcessMonitor.cleanupFiles(logDir, "log file", logger, {
176+
filter: (name) => name.startsWith("coder-ssh") && name.endsWith(".log"),
177+
select: (files, now) =>
178+
files
179+
.toSorted((a, b) => a.mtime - b.mtime) // oldest first
180+
.slice(0, -maxFilesToKeep) // keep the newest maxFilesToKeep
181+
.filter((f) => now - f.mtime > maxAgeMs), // only delete stale files
182+
});
183+
}
184+
120185
private constructor(options: SshProcessMonitorOptions) {
121186
this.options = {
122187
...options,
@@ -142,12 +207,24 @@ export class SshProcessMonitor implements vscode.Disposable {
142207
// Clean up old network info files (non-blocking, fire-and-forget)
143208
SshProcessMonitor.cleanupOldNetworkFiles(
144209
options.networkInfoPath,
145-
CLEANUP_MAX_AGE_MS,
210+
CLEANUP_NETWORK_MAX_AGE_MS,
146211
options.logger,
147212
).catch(() => {
148213
// Ignore cleanup errors - they shouldn't affect monitoring
149214
});
150215

216+
// Clean up old proxy log files (combined: count + age threshold)
217+
if (options.proxyLogDir) {
218+
SshProcessMonitor.cleanupOldLogFiles(
219+
options.proxyLogDir,
220+
CLEANUP_MAX_LOG_FILES,
221+
CLEANUP_LOG_MAX_AGE_MS,
222+
options.logger,
223+
).catch(() => {
224+
// Ignore cleanup errors - they shouldn't affect monitoring
225+
});
226+
}
227+
151228
monitor.searchForProcess().catch((err) => {
152229
options.logger.error("Error in SSH process monitor", err);
153230
});

‎test/unit/remote/sshProcess.test.ts‎

Lines changed: 124 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -424,10 +424,12 @@ describe("SshProcessMonitor", () => {
424424
});
425425

426426
describe("cleanup old network files", () => {
427-
const setOldMtime = (filePath: string) => {
428-
// Default cleanup is 1 hour; set mtime to 2 hours ago to mark as old
429-
const TWO_HOURS_AGO = Date.now() - 2 * 60 * 60 * 1000;
430-
vol.utimesSync(filePath, TWO_HOURS_AGO / 1000, TWO_HOURS_AGO / 1000);
427+
// Network cleanup: 1 hour threshold
428+
const NETWORK_MAX_AGE_MS = 60 * 60 * 1000;
429+
430+
const setMtimeAgo = (filePath: string, ageMs: number) => {
431+
const mtime = (Date.now() - ageMs) / 1000;
432+
vol.utimesSync(filePath, mtime, mtime);
431433
};
432434

433435
it("deletes old .json files but preserves recent and non-.json files", async () => {
@@ -438,8 +440,8 @@ describe("SshProcessMonitor", () => {
438440
"/network/recent.json": "{}",
439441
"/network/old.log": "{}",
440442
});
441-
setOldMtime("/network/old.json");
442-
setOldMtime("/network/old.log");
443+
setMtimeAgo("/network/old.json", NETWORK_MAX_AGE_MS * 2);
444+
setMtimeAgo("/network/old.log", NETWORK_MAX_AGE_MS * 2);
443445

444446
createMonitor({
445447
codeLogDir: "/logs/window1",
@@ -477,6 +479,122 @@ describe("SshProcessMonitor", () => {
477479
});
478480
});
479481

482+
describe("cleanup proxy log files", () => {
483+
// Proxy log cleanup: 7 day threshold, 20 files max
484+
const LOG_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000;
485+
const LOG_MAX_FILES = 20;
486+
487+
const setMtimeAgo = (filePath: string, ageMs: number) => {
488+
const mtime = (Date.now() - ageMs) / 1000;
489+
vol.utimesSync(filePath, mtime, mtime);
490+
};
491+
492+
const logFileName = (i: number) =>
493+
`coder-ssh-${i.toString().padStart(2, "0")}.log`;
494+
495+
const setupTest = (total: number, stale: number): string[] => {
496+
vol.fromJSON({
497+
"/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log":
498+
"-> socksPort 12345 ->",
499+
});
500+
vol.mkdirSync("/proxy-logs", { recursive: true });
501+
502+
const files = Array.from({ length: total }, (_, i) => logFileName(i + 1));
503+
for (const name of files) {
504+
vol.writeFileSync(`/proxy-logs/${name}`, "");
505+
}
506+
for (let i = 0; i < stale; i++) {
507+
setMtimeAgo(`/proxy-logs/${files[i]}`, LOG_MAX_AGE_MS * 2);
508+
}
509+
return files;
510+
};
511+
512+
interface StaleLogTestCase {
513+
total: number;
514+
stale: number;
515+
expected: number;
516+
desc: string;
517+
}
518+
519+
it.each<StaleLogTestCase>([
520+
{ total: 25, stale: 8, expected: 20, desc: "Deletes until limit" },
521+
{ total: 25, stale: 3, expected: 22, desc: "Only deletes stale" },
522+
{ total: 25, stale: 0, expected: 25, desc: "Keeps recent files" },
523+
{ total: 15, stale: 5, expected: 15, desc: "Keeps under limit" },
524+
])(
525+
"$desc: $total files, $stale stale → $expected remaining",
526+
async ({ total, stale, expected }) => {
527+
setupTest(total, stale);
528+
529+
createMonitor({
530+
codeLogDir: "/logs/window1",
531+
proxyLogDir: "/proxy-logs",
532+
});
533+
534+
await vi.waitFor(() => {
535+
expect(vol.readdirSync("/proxy-logs")).toHaveLength(expected);
536+
});
537+
},
538+
);
539+
540+
it("only matches coder-ssh*.log files", async () => {
541+
const files = setupTest(25, 25);
542+
// Add non-matching files
543+
const nonMatchingFiles = [
544+
"other.log",
545+
"coder-ssh-config.json",
546+
"readme.txt",
547+
];
548+
for (const f of nonMatchingFiles) {
549+
const filePath = `/proxy-logs/${f}`;
550+
vol.writeFileSync(filePath, "");
551+
setMtimeAgo(filePath, LOG_MAX_AGE_MS * 2);
552+
}
553+
554+
createMonitor({
555+
codeLogDir: "/logs/window1",
556+
proxyLogDir: "/proxy-logs",
557+
});
558+
559+
await vi.waitFor(() => {
560+
expect(vol.readdirSync("/proxy-logs")).toHaveLength(
561+
LOG_MAX_FILES + nonMatchingFiles.length,
562+
);
563+
});
564+
565+
const remaining = vol.readdirSync("/proxy-logs") as string[];
566+
// Non-matching files preserved
567+
expect(remaining).toContain("other.log");
568+
expect(remaining).toContain("coder-ssh-config.json");
569+
expect(remaining).toContain("readme.txt");
570+
// Oldest matching files deleted
571+
expect(remaining).not.toContain(files[0]);
572+
expect(remaining).toContain(files[24]);
573+
});
574+
575+
it("does not throw when proxy log directory is missing or empty", () => {
576+
vol.fromJSON({
577+
"/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log":
578+
"-> socksPort 12345 ->",
579+
});
580+
vol.mkdirSync("/empty-proxy-logs", { recursive: true });
581+
582+
expect(() =>
583+
createMonitor({
584+
codeLogDir: "/logs/window1",
585+
proxyLogDir: "/nonexistent-proxy-logs",
586+
}),
587+
).not.toThrow();
588+
589+
expect(() =>
590+
createMonitor({
591+
codeLogDir: "/logs/window1",
592+
proxyLogDir: "/empty-proxy-logs",
593+
}),
594+
).not.toThrow();
595+
});
596+
});
597+
480598
describe("missing file retry logic", () => {
481599
beforeEach(() => vi.useFakeTimers());
482600
afterEach(() => vi.useRealTimers());

0 commit comments

Comments
 (0)