From 4061a2761ed0cbb55ea630e39089449db52aeb6e Mon Sep 17 00:00:00 2001 From: simosmik Date: Mon, 9 Mar 2026 07:39:00 +0000 Subject: [PATCH] fix(plugins): async shutdown and asset/RPC fixes Await stopPluginServer/stopAllPlugins in signal handlers and route handlers so process exit and state transitions wait for clean plugin shutdown instead of racing ahead. Validate asset paths are regular files before streaming to prevent directory traversal returning unexpected content; add a stream error handler to avoid unhandled crashes on read failures. Fix RPC proxy body detection to use the content-length header instead of Object.keys, so falsy but valid JSON payloads (null, false, 0, {}) are forwarded correctly to plugin servers. Track in-flight start operations via a startingPlugins map to prevent duplicate concurrent plugin starts. --- server/index.js | 8 ++--- server/routes/plugins.js | 30 +++++++++++++++---- server/utils/plugin-process-manager.js | 21 ++++++++++--- src/components/git-panel/view/GitPanel.tsx | 1 + .../git-panel/view/changes/ChangesView.tsx | 3 ++ .../git-panel/view/changes/CommitComposer.tsx | 21 +++++++++++-- 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/server/index.js b/server/index.js index 66012460..e745bf11 100755 --- a/server/index.js +++ b/server/index.js @@ -2545,12 +2545,12 @@ async function startServer() { }); // Clean up plugin processes on shutdown - const shutdownPlugins = () => { - stopAllPlugins(); + const shutdownPlugins = async () => { + await stopAllPlugins(); process.exit(0); }; - process.on('SIGTERM', shutdownPlugins); - process.on('SIGINT', shutdownPlugins); + process.on('SIGTERM', () => void shutdownPlugins()); + process.on('SIGINT', () => void shutdownPlugins()); } catch (error) { console.error('[ERROR] Failed to start server:', error); process.exit(1); diff --git a/server/routes/plugins.js b/server/routes/plugins.js index 2455e526..e7ab7e81 100644 --- a/server/routes/plugins.js +++ b/server/routes/plugins.js @@ -64,9 +64,26 @@ router.get('/:name/assets/*', (req, res) => { return res.status(404).json({ error: 'Asset not found' }); } + try { + const stat = fs.statSync(resolvedPath); + if (!stat.isFile()) { + return res.status(404).json({ error: 'Asset not found' }); + } + } catch { + return res.status(404).json({ error: 'Asset not found' }); + } + const contentType = mime.lookup(resolvedPath) || 'application/octet-stream'; res.setHeader('Content-Type', contentType); - fs.createReadStream(resolvedPath).pipe(res); + const stream = fs.createReadStream(resolvedPath); + stream.on('error', () => { + if (!res.headersSent) { + res.status(500).json({ error: 'Failed to read asset' }); + } else { + res.end(); + } + }); + stream.pipe(res); }); // PUT /:name/enable — Toggle plugin enabled/disabled (starts/stops server if applicable) @@ -99,7 +116,7 @@ router.put('/:name/enable', async (req, res) => { } } } else if (!enabled && isPluginRunning(plugin.name)) { - stopPluginServer(plugin.name); + await stopPluginServer(plugin.name); } } @@ -153,7 +170,7 @@ router.post('/:name/update', async (req, res) => { const wasRunning = isPluginRunning(pluginName); if (wasRunning) { - stopPluginServer(pluginName); + await stopPluginServer(pluginName); } const manifest = await updatePluginFromGit(pluginName); @@ -238,8 +255,11 @@ router.all('/:name/rpc/*', async (req, res) => { res.status(502).json({ error: 'Plugin server error', details: err.message }); }); - // Forward body (already parsed by express JSON middleware, so re-stringify) - if (req.body && Object.keys(req.body).length > 0) { + // Forward body (already parsed by express JSON middleware, so re-stringify). + // Check content-length to detect whether a body was actually sent, since + // req.body can be falsy for valid payloads like 0, false, null, or {}. + const hasBody = req.headers['content-length'] && parseInt(req.headers['content-length'], 10) > 0; + if (hasBody && req.body !== undefined) { const bodyStr = JSON.stringify(req.body); proxyReq.setHeader('content-length', Buffer.byteLength(bodyStr)); proxyReq.write(bodyStr); diff --git a/server/utils/plugin-process-manager.js b/server/utils/plugin-process-manager.js index 56731714..d5fa493e 100644 --- a/server/utils/plugin-process-manager.js +++ b/server/utils/plugin-process-manager.js @@ -4,6 +4,8 @@ import { scanPlugins, getPluginsConfig, getPluginDir } from './plugin-loader.js' // Map const runningPlugins = new Map(); +// Map> — in-flight start operations +const startingPlugins = new Map(); /** * Start a plugin's server subprocess. @@ -11,10 +13,16 @@ const runningPlugins = new Map(); * to stdout within 10 seconds. */ export function startPluginServer(name, pluginDir, serverEntry) { - return new Promise((resolve, reject) => { - if (runningPlugins.has(name)) { - return resolve(runningPlugins.get(name).port); - } + if (runningPlugins.has(name)) { + return Promise.resolve(runningPlugins.get(name).port); + } + + // Coalesce concurrent starts for the same plugin + if (startingPlugins.has(name)) { + return startingPlugins.get(name); + } + + const startPromise = new Promise((resolve, reject) => { const serverPath = path.join(pluginDir, serverEntry); @@ -88,7 +96,12 @@ export function startPluginServer(name, pluginDir, serverEntry) { reject(new Error(`Plugin server exited with code ${code} before reporting ready`)); } }); + }).finally(() => { + startingPlugins.delete(name); }); + + startingPlugins.set(name, startPromise); + return startPromise; } /** diff --git a/src/components/git-panel/view/GitPanel.tsx b/src/components/git-panel/view/GitPanel.tsx index 7f071c0e..f3dd3070 100644 --- a/src/components/git-panel/view/GitPanel.tsx +++ b/src/components/git-panel/view/GitPanel.tsx @@ -108,6 +108,7 @@ export default function GitPanel({ selectedProject, isMobile = false, onFileOpen {activeView === 'changes' && ( (); + type CommitComposerProps = { isMobile: boolean; + projectPath: string; selectedFileCount: number; isHidden: boolean; onCommit: (message: string) => Promise; @@ -14,13 +18,26 @@ type CommitComposerProps = { export default function CommitComposer({ isMobile, + projectPath, selectedFileCount, isHidden, onCommit, onGenerateMessage, onRequestConfirmation, }: CommitComposerProps) { - const [commitMessage, setCommitMessage] = useState(''); + const [commitMessage, setCommitMessageState] = useState(() => commitMessageCache.get(projectPath) ?? ''); + + const setCommitMessage = useCallback( + (msg: string) => { + setCommitMessageState(msg); + if (msg) { + commitMessageCache.set(projectPath, msg); + } else { + commitMessageCache.delete(projectPath); + } + }, + [projectPath], + ); const [isCommitting, setIsCommitting] = useState(false); const [isGeneratingMessage, setIsGeneratingMessage] = useState(false); const [isCollapsed, setIsCollapsed] = useState(isMobile);