fix(files): handle reveal launcher failures (#2490)
This commit is contained in:
@@ -382,6 +382,7 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
path,
|
path,
|
||||||
fsPromises,
|
fsPromises,
|
||||||
spawn,
|
spawn,
|
||||||
|
platform = process.platform,
|
||||||
crypto,
|
crypto,
|
||||||
normalizeDirectoryPath,
|
normalizeDirectoryPath,
|
||||||
resolveProjectDirectory,
|
resolveProjectDirectory,
|
||||||
@@ -393,6 +394,27 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
realpath: fsPromises.realpath.bind(fsPromises),
|
realpath: fsPromises.realpath.bind(fsPromises),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const spawnDetached = (command, args) => new Promise((resolve, reject) => {
|
||||||
|
let child;
|
||||||
|
try {
|
||||||
|
child = spawn(command, args, { windowsHide: true, stdio: 'ignore', detached: true });
|
||||||
|
} catch (error) {
|
||||||
|
reject(new Error('Failed to launch file browser', { cause: error }));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const onError = (error) => {
|
||||||
|
child.removeListener('spawn', onSpawn);
|
||||||
|
reject(new Error('Failed to launch file browser', { cause: error }));
|
||||||
|
};
|
||||||
|
const onSpawn = () => {
|
||||||
|
child.removeListener('error', onError);
|
||||||
|
child.unref();
|
||||||
|
resolve();
|
||||||
|
};
|
||||||
|
child.once('error', onError);
|
||||||
|
child.once('spawn', onSpawn);
|
||||||
|
});
|
||||||
|
|
||||||
const execJobs = new Map();
|
const execJobs = new Map();
|
||||||
const commandTimeoutMs = createCommandTimeoutMs();
|
const commandTimeoutMs = createCommandTimeoutMs();
|
||||||
const gitReadCacheTtlMs = createGitReadCacheTtlMs();
|
const gitReadCacheTtlMs = createGitReadCacheTtlMs();
|
||||||
@@ -1134,13 +1156,12 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
const resolved = path.resolve(targetPath.trim());
|
const resolved = path.resolve(targetPath.trim());
|
||||||
await fsPromises.access(resolved);
|
await fsPromises.access(resolved);
|
||||||
|
|
||||||
const platform = process.platform;
|
|
||||||
if (platform === 'darwin') {
|
if (platform === 'darwin') {
|
||||||
const stat = await fsPromises.stat(resolved);
|
const stat = await fsPromises.stat(resolved);
|
||||||
if (stat.isDirectory()) {
|
if (stat.isDirectory()) {
|
||||||
spawn('open', [resolved], { windowsHide: true, stdio: 'ignore', detached: true }).unref();
|
await spawnDetached('open', [resolved]);
|
||||||
} else {
|
} else {
|
||||||
spawn('open', ['-R', resolved], { windowsHide: true, stdio: 'ignore', detached: true }).unref();
|
await spawnDetached('open', ['-R', resolved]);
|
||||||
}
|
}
|
||||||
} else if (platform === 'win32') {
|
} else if (platform === 'win32') {
|
||||||
const stat = await fsPromises.stat(resolved);
|
const stat = await fsPromises.stat(resolved);
|
||||||
@@ -1164,7 +1185,7 @@ export const registerFsRoutes = (app, dependencies) => {
|
|||||||
} else {
|
} else {
|
||||||
const stat = await fsPromises.stat(resolved);
|
const stat = await fsPromises.stat(resolved);
|
||||||
const dir = stat.isDirectory() ? resolved : path.dirname(resolved);
|
const dir = stat.isDirectory() ? resolved : path.dirname(resolved);
|
||||||
spawn('xdg-open', [dir], { windowsHide: true, stdio: 'ignore', detached: true }).unref();
|
await spawnDetached('xdg-open', [dir]);
|
||||||
}
|
}
|
||||||
|
|
||||||
return res.json({ success: true, path: resolved });
|
return res.json({ success: true, path: resolved });
|
||||||
|
|||||||
@@ -200,6 +200,27 @@ const registerMkdir = (fsPromises) => {
|
|||||||
return getRoute('POST', '/api/fs/mkdir');
|
return getRoute('POST', '/api/fs/mkdir');
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const registerReveal = ({ fsPromises, spawn, platform = 'linux' }) => {
|
||||||
|
const { app, getRoute } = createRouteRegistry();
|
||||||
|
registerFsRoutes(app, {
|
||||||
|
os: { homedir: () => '/home/user' },
|
||||||
|
path: path.posix,
|
||||||
|
fsPromises: {
|
||||||
|
realpath: async (targetPath) => targetPath,
|
||||||
|
...fsPromises,
|
||||||
|
},
|
||||||
|
spawn,
|
||||||
|
platform,
|
||||||
|
crypto: { randomUUID: () => 'job-0' },
|
||||||
|
normalizeDirectoryPath: (p) => p,
|
||||||
|
resolveProjectDirectory: async () => ({ directory: '/repo' }),
|
||||||
|
buildAugmentedPath: () => '/usr/bin',
|
||||||
|
resolveGitBinaryForSpawn: () => 'git',
|
||||||
|
openchamberUserConfigRoot: '/home/user/.config',
|
||||||
|
});
|
||||||
|
return getRoute('POST', '/api/fs/reveal');
|
||||||
|
};
|
||||||
|
|
||||||
const callExec = async (handler, body) => {
|
const callExec = async (handler, body) => {
|
||||||
const res = createMockResponse();
|
const res = createMockResponse();
|
||||||
await handler({ body }, res);
|
await handler({ body }, res);
|
||||||
@@ -230,6 +251,12 @@ const callMkdir = async (handler, body) => {
|
|||||||
return res;
|
return res;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const callReveal = async (handler, body) => {
|
||||||
|
const res = createMockResponse();
|
||||||
|
await handler({ body }, res);
|
||||||
|
return res;
|
||||||
|
};
|
||||||
|
|
||||||
describe('fs write', () => {
|
describe('fs write', () => {
|
||||||
it('does not rewrite a file when content is unchanged', async () => {
|
it('does not rewrite a file when content is unchanged', async () => {
|
||||||
const fsPromises = {
|
const fsPromises = {
|
||||||
@@ -431,6 +458,77 @@ describe('fs read', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('fs reveal', () => {
|
||||||
|
it.each([
|
||||||
|
['linux', 'xdg-open', ['/repo']],
|
||||||
|
['darwin', 'open', ['-R', '/repo/file.txt']],
|
||||||
|
])('returns a controlled error when the %s launcher is unavailable', async (platform, command, args) => {
|
||||||
|
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||||
|
const child = new EventEmitter();
|
||||||
|
child.unref = vi.fn();
|
||||||
|
const spawn = vi.fn(() => {
|
||||||
|
queueMicrotask(() => child.emit('error', Object.assign(new Error('not found'), { code: 'ENOENT' })));
|
||||||
|
return child;
|
||||||
|
});
|
||||||
|
const handler = registerReveal({
|
||||||
|
fsPromises: {
|
||||||
|
access: vi.fn(async () => undefined),
|
||||||
|
stat: vi.fn(async () => ({ isDirectory: () => false })),
|
||||||
|
},
|
||||||
|
spawn,
|
||||||
|
platform,
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await callReveal(handler, { path: '/repo/file.txt' });
|
||||||
|
|
||||||
|
expect(res.statusCode).toBe(500);
|
||||||
|
expect(res.body).toEqual({ error: 'Failed to launch file browser' });
|
||||||
|
expect(spawn).toHaveBeenCalledWith(command, args, { windowsHide: true, stdio: 'ignore', detached: true });
|
||||||
|
expect(child.unref).not.toHaveBeenCalled();
|
||||||
|
error.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('unrefs a detached launcher only after it spawns successfully', async () => {
|
||||||
|
const child = new EventEmitter();
|
||||||
|
child.unref = vi.fn();
|
||||||
|
const spawn = vi.fn(() => {
|
||||||
|
queueMicrotask(() => child.emit('spawn'));
|
||||||
|
return child;
|
||||||
|
});
|
||||||
|
const handler = registerReveal({
|
||||||
|
fsPromises: {
|
||||||
|
access: vi.fn(async () => undefined),
|
||||||
|
stat: vi.fn(async () => ({ isDirectory: () => false })),
|
||||||
|
},
|
||||||
|
spawn,
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await callReveal(handler, { path: '/repo/file.txt' });
|
||||||
|
|
||||||
|
expect(res.body).toEqual({ success: true, path: '/repo/file.txt' });
|
||||||
|
expect(child.unref).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns a controlled error when the launcher throws synchronously', async () => {
|
||||||
|
const error = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||||
|
const spawnError = Object.assign(new Error('not found'), { code: 'ENOENT' });
|
||||||
|
const handler = registerReveal({
|
||||||
|
fsPromises: {
|
||||||
|
access: vi.fn(async () => undefined),
|
||||||
|
stat: vi.fn(async () => ({ isDirectory: () => false })),
|
||||||
|
},
|
||||||
|
spawn: vi.fn(() => { throw spawnError; }),
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await callReveal(handler, { path: '/repo/file.txt' });
|
||||||
|
|
||||||
|
expect(res.statusCode).toBe(500);
|
||||||
|
expect(res.body).toEqual({ error: 'Failed to launch file browser' });
|
||||||
|
expect(error).toHaveBeenCalledWith('Failed to reveal path:', expect.objectContaining({ cause: spawnError }));
|
||||||
|
error.mockRestore();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('fs exec git-read cache', () => {
|
describe('fs exec git-read cache', () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
delete process.env.OPENCHAMBER_GIT_READ_CACHE_TTL_MS;
|
delete process.env.OPENCHAMBER_GIT_READ_CACHE_TTL_MS;
|
||||||
|
|||||||
Reference in New Issue
Block a user