BitSquare/Codeman
2
0
Fork 0
mirror of https://github.com/Ark0N/Codeman.git synced 2026-10-12 02:09:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(uploads): list upload dirs bounded and async, and keep uploads inside the data dir collected

Browse Source 
The hourly sweep called lstatSync and realpathSync on every live session's working directory, so a linked case on a mount that stopped answering blocked the event loop 30 s after boot and then every hour; the old sweep was fully async. uploadDirs() now probes the workspace with probePathKind() first and skips an unknown path without touching it, then uses fs.promises for the lstat and realpath. The sweep keeps the probe's stall cap; cleanupSession(), acting on one path at the user's request, passes pastCap and removes the directories with fs.rm.

Refusing an upload dir that sits strictly inside the data dir protected nothing (it only ever holds uploads, and a link is already excluded by lstat) while the route kept writing there, so uploads under a workspace like the ~/.codeman/app that install.sh clones were never swept and never removed. Only the two cases that matter stay refused: the upload dir being the data dir, or containing it.

Also: the ignore file's take-back after a failed write no longer replaces the error that caused it with its own, and the shared-dir test's header no longer names the fixed port it stopped using.
This commit is contained in:
JD
2026-10-09 20:05:21 -04:00
parent f12b5ac88b
commit ba36cc36d4
7 changed files with 105 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/paste-image-dir-shared.test.ts
+12 -1
View File
@@ -10,7 +10,7 @@
* closes sessions unattended, which turns that from an occasional loss into a
* routine one.
*
* Port: 3188
* Port: 0 (ephemeral, read from `server.boundPort`)
*/
import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
@@ -18,6 +18,12 @@ import { join } from 'node:path';
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { pasteImageDirInUseByOtherSession } from '../src/web/paste-image-gc.js';
import { probePathKind } from '../src/utils/bounded-path-probe.js';
vi.mock('../src/utils/bounded-path-probe.js', async (importOriginal) => {
const real = await importOriginal<typeof import('../src/utils/bounded-path-probe.js')>();
return { ...real, probePathKind: vi.fn(real.probePathKind) };
});
describe('pasteImageDirInUseByOtherSession', () => {
const none = new Set<string>();
@@ -148,9 +154,14 @@ describe('deleting a session that shares its working directory', () => {
expect(existsSync(join(uploadDir, 'paste-1.png'))).toBe(true);
expect(existsSync(join(legacyDir, 'paste-0.png'))).toBe(true);
// The delete acts on one path at the user's request, so its probe goes past
// the bulk stall cap (#516); the hourly sweep keeps the cap. Cleared first:
// the create path probes the same workspace past the cap too.
vi.mocked(probePathKind).mockClear();
expect((await remove(second)).status).toBe(200);
expect(existsSync(uploadDir)).toBe(false);
expect(existsSync(legacyDir)).toBe(false);
expect(probePathKind).toHaveBeenCalledWith(workingDir, { pastCap: true });
});
it('does not follow a planted symlink at the upload dir into another case when the last session closes', async () => {
test/paste-image-gc.test.ts
+41 -16
View File
@@ -2,15 +2,22 @@
* @fileoverview The hourly upload sweep reads BOTH upload dirs of a live session:
* `.codeman-uploads` and the `.claude-images` a case in use before the move still
* carries. Only `paste-*` regular files past the age cap go. `uploadDirs()` never
* lists a planted symlink, a directory that is, contains or sits inside the data
* dir, or anything for a remote session, since the sweep and the delete cleanup
* both act on what it returns.
* lists a planted symlink, a directory that is or contains the data dir, anything
* under a workspace whose bounded probe did not answer, or anything for a remote
* session, since the sweep and the delete cleanup both act on what it returns.
*/
import { existsSync, lstatSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, utimesSync, writeFileSync } from 'node:fs';
import fsp from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { afterEach, describe, expect, it } from 'vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { LEGACY_UPLOADS_DIR, UPLOADS_DIR, sweepPasteImagesOnce, uploadDirs } from '../src/web/paste-image-gc.js';
import { probePathKind } from '../src/utils/bounded-path-probe.js';
vi.mock('../src/utils/bounded-path-probe.js', async (importOriginal) => {
const real = await importOriginal<typeof import('../src/utils/bounded-path-probe.js')>();
return { ...real, probePathKind: vi.fn(real.probePathKind) };
});
const DAY_MS = 24 * 60 * 60 * 1000;
const sessionsOf = (workingDir: string, remote?: unknown) => ({
@@ -31,7 +38,7 @@ describe('sweepPasteImagesOnce', () => {
const legacy = join(workingDir, LEGACY_UPLOADS_DIR);
mkdirSync(current);
mkdirSync(legacy);
expect(uploadDirs({ workingDir })).toEqual([current, legacy]);
expect(await uploadDirs({ workingDir })).toEqual([current, legacy]);
const old = (now - 8 * DAY_MS) / 1000;
const fresh = (now - 1 * DAY_MS) / 1000;
const plant = (dir: string, name: string, mtime: number) => {
@@ -69,7 +76,7 @@ describe('sweepPasteImagesOnce', () => {
// age out the other case's uploads through it.
symlinkSync(join(other, UPLOADS_DIR), join(workingDir, UPLOADS_DIR));
expect(uploadDirs({ workingDir })).toEqual([]);
expect(await uploadDirs({ workingDir })).toEqual([]);
expect(await sweepPasteImagesOnce(sessionsOf(workingDir), now)).toEqual({ scanned: 0, deleted: 0 });
expect(existsSync(join(other, UPLOADS_DIR, 'paste-9-zz.png'))).toBe(true);
});
@@ -84,7 +91,7 @@ describe('uploadDirs', () => {
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
});
it('lists the upload dir of a home workspace, which sits beside the data dir, not inside it', () => {
it('lists the upload dir of a home workspace, which sits beside the data dir, not inside it', async () => {
// The home-as-workspace case the flat name exists for: `<home>/.codeman-uploads`
// is a sibling of `<home>/.codeman`, so a prefix test without the separator would
// wrongly refuse it.
@@ -93,10 +100,10 @@ describe('uploadDirs', () => {
process.env.CODEMAN_DATA_DIR = join(home, '.codeman');
const dir = join(home, UPLOADS_DIR);
mkdirSync(dir);
expect(uploadDirs({ workingDir: home })).toEqual([dir]);
expect(await uploadDirs({ workingDir: home })).toEqual([dir]);
});
it('refuses an upload dir that is the data dir, contains it, or sits inside it', () => {
it('refuses an upload dir that is the data dir or contains it, and lists one inside it', async () => {
const root = mkdtempSync(join(tmpdir(), 'codeman-gc-data-'));
const parent = mkdtempSync(join(tmpdir(), 'codeman-gc-link-'));
dirs.push(root, parent);
@@ -104,16 +111,34 @@ describe('uploadDirs', () => {
mkdirSync(join(uploads, 'state'), { recursive: true });
// `CODEMAN_INSTANCE=uploads` on a home workspace: the upload dir IS the data dir.
process.env.CODEMAN_DATA_DIR = uploads;
expect(uploadDirs({ workingDir: root })).toEqual([]);
expect(await uploadDirs({ workingDir: root })).toEqual([]);
// A data dir pointed inside the upload dir: the recursive delete would take it.
process.env.CODEMAN_DATA_DIR = join(uploads, 'state');
expect(uploadDirs({ workingDir: root })).toEqual([]);
// An upload dir inside the data dir, directly and through a symlinked working
// directory (the upload dir itself is real there).
expect(await uploadDirs({ workingDir: root })).toEqual([]);
// An upload dir strictly inside the data dir only ever holds uploads (install.sh
// clones the app to ~/.codeman/app), so it is listed, directly and through a
// symlinked working directory.
process.env.CODEMAN_DATA_DIR = root;
expect(uploadDirs({ workingDir: root })).toEqual([]);
expect(await uploadDirs({ workingDir: root })).toEqual([uploads]);
symlinkSync(root, join(parent, 'ws'));
expect(uploadDirs({ workingDir: join(parent, 'ws') })).toEqual([]);
expect(await uploadDirs({ workingDir: join(parent, 'ws') })).toEqual([join(parent, 'ws', UPLOADS_DIR)]);
});
it('skips a workspace whose bounded probe does not answer, without touching it', async () => {
// A linked case on a mount that stopped answering reads `unknown` (#516): the
// sweep must not lstat under it, which would hold a threadpool worker.
const workingDir = mkdtempSync(join(tmpdir(), 'codeman-gc-stalled-'));
dirs.push(workingDir);
mkdirSync(join(workingDir, UPLOADS_DIR));
vi.mocked(probePathKind).mockResolvedValueOnce('unknown');
const lstat = vi.spyOn(fsp, 'lstat');
try {
expect(await uploadDirs({ workingDir })).toEqual([]);
expect(lstat).not.toHaveBeenCalled();
} finally {
lstat.mockRestore();
}
expect(probePathKind).toHaveBeenCalledWith(workingDir, {});
});
it('lists nothing for a remote session, whose workingDir is the remote path', async () => {
@@ -125,7 +150,7 @@ describe('uploadDirs', () => {
const old = (Date.now() - 8 * DAY_MS) / 1000;
utimesSync(join(workingDir, UPLOADS_DIR, 'paste-1-aa.png'), old, old);
const remote = { hostId: 'gpu-box' };
expect(uploadDirs({ workingDir, remote })).toEqual([]);
expect(await uploadDirs({ workingDir, remote })).toEqual([]);
expect(await sweepPasteImagesOnce(sessionsOf(workingDir, remote))).toEqual({ scanned: 0, deleted: 0 });
expect(existsSync(join(workingDir, UPLOADS_DIR, 'paste-1-aa.png'))).toBe(true);
});