-
Notifications
You must be signed in to change notification settings - Fork 287
feat: surface playbook STATUS.json in Auto Run UI #732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,7 @@ import chokidar, { FSWatcher } from 'chokidar'; | |||||||||||||||||||||||||||||||||||||||
| import Store from 'electron-store'; | ||||||||||||||||||||||||||||||||||||||||
| import { logger } from '../../utils/logger'; | ||||||||||||||||||||||||||||||||||||||||
| import { createIpcHandler, CreateHandlerOptions } from '../../utils/ipcHandler'; | ||||||||||||||||||||||||||||||||||||||||
| import { SshRemoteConfig } from '../../../shared/types'; | ||||||||||||||||||||||||||||||||||||||||
| import { SshRemoteConfig, PlaybookStatus } from '../../../shared/types'; | ||||||||||||||||||||||||||||||||||||||||
| import { MaestroSettings } from './persistence'; | ||||||||||||||||||||||||||||||||||||||||
| import { isWebContentsAvailable } from '../../utils/safe-send'; | ||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -55,6 +55,10 @@ function getSshRemoteById( | |||||||||||||||||||||||||||||||||||||||
| const autoRunWatchers = new Map<string, FSWatcher>(); | ||||||||||||||||||||||||||||||||||||||||
| let autoRunWatchDebounceTimer: NodeJS.Timeout | null = null; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Playbook STATUS.json watchers (keyed by project path) | ||||||||||||||||||||||||||||||||||||||||
| const statusWatchers = new Map<string, FSWatcher>(); | ||||||||||||||||||||||||||||||||||||||||
| let statusWatchDebounceTimer: NodeJS.Timeout | null = null; | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Replace the singleton with a per-path map: // Replace line 60 with:
const statusWatchDebounceTimers = new Map<string, NodeJS.Timeout>();Then in const handleStatusChange = async () => {
const existing = statusWatchDebounceTimers.get(projectPath);
if (existing) clearTimeout(existing);
const timer = setTimeout(async () => {
statusWatchDebounceTimers.delete(projectPath);
try {
const content = await fs.readFile(statusFilePath, 'utf-8');
const status = JSON.parse(content) as PlaybookStatus;
const mainWindow = getMainWindow();
if (isWebContentsAvailable(mainWindow)) {
mainWindow.webContents.send('autorun:statusChanged', { projectPath, status });
}
} catch (err) {
logger.warn(`${LOG_CONTEXT} Failed to read STATUS.json: ${err}`, LOG_CONTEXT);
}
}, 500);
statusWatchDebounceTimers.set(projectPath, timer);
};Also clear pending timers in |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Tree node interface for autorun directory scanning. | ||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1270,13 +1274,117 @@ export function registerAutorunHandlers( | |||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Watch for .maestro/STATUS.json changes in a project directory | ||||||||||||||||||||||||||||||||||||||||
| // When the file changes, read and parse it, then emit the parsed status to the renderer | ||||||||||||||||||||||||||||||||||||||||
| ipcMain.handle( | ||||||||||||||||||||||||||||||||||||||||
| 'autorun:watchStatus', | ||||||||||||||||||||||||||||||||||||||||
| createIpcHandler(handlerOpts('watchStatus'), async (projectPath: string) => { | ||||||||||||||||||||||||||||||||||||||||
| // Stop any existing status watcher for this path | ||||||||||||||||||||||||||||||||||||||||
| if (statusWatchers.has(projectPath)) { | ||||||||||||||||||||||||||||||||||||||||
| statusWatchers.get(projectPath)?.close(); | ||||||||||||||||||||||||||||||||||||||||
| statusWatchers.delete(projectPath); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const statusFilePath = path.join(projectPath, '.maestro', 'STATUS.json'); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Read initial status if file exists | ||||||||||||||||||||||||||||||||||||||||
| let initialStatus: PlaybookStatus | null = null; | ||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||
| const content = await fs.readFile(statusFilePath, 'utf-8'); | ||||||||||||||||||||||||||||||||||||||||
| initialStatus = JSON.parse(content) as PlaybookStatus; | ||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||
| // File doesn't exist yet — that's fine | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1292
to
+1297
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t swallow non-ENOENT errors while loading initial STATUS.json. This catch currently hides JSON parse and filesystem permission errors as if the file simply does not exist, which makes failures hard to diagnose and can keep stale/empty UI state. 💡 Suggested fix (handle expected errors, rethrow unexpected ones) let initialStatus: PlaybookStatus | null = null;
try {
const content = await fs.readFile(statusFilePath, 'utf-8');
initialStatus = JSON.parse(content) as PlaybookStatus;
- } catch {
- // File doesn't exist yet — that's fine
+ } catch (error) {
+ const err = error as NodeJS.ErrnoException;
+ if (err.code === 'ENOENT') {
+ // File doesn't exist yet — expected
+ } else if (error instanceof SyntaxError) {
+ logger.warn(`${LOG_CONTEXT} Invalid STATUS.json format: ${statusFilePath}`, LOG_CONTEXT);
+ } else {
+ throw error;
+ }
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Watch the .maestro directory for STATUS.json changes | ||||||||||||||||||||||||||||||||||||||||
| const maestroDir = path.join(projectPath, '.maestro'); | ||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||
| await fs.stat(maestroDir); | ||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||
| await fs.mkdir(maestroDir, { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1299
to
+1305
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This block unconditionally creates the chokidar supports watching a non-existent file path — it will detect the file when it is eventually created. The
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const watcher = chokidar.watch(statusFilePath, { | ||||||||||||||||||||||||||||||||||||||||
| persistent: true, | ||||||||||||||||||||||||||||||||||||||||
| ignoreInitial: true, | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const handleStatusChange = async () => { | ||||||||||||||||||||||||||||||||||||||||
| if (statusWatchDebounceTimer) { | ||||||||||||||||||||||||||||||||||||||||
| clearTimeout(statusWatchDebounceTimer); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| statusWatchDebounceTimer = setTimeout(async () => { | ||||||||||||||||||||||||||||||||||||||||
| statusWatchDebounceTimer = null; | ||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||
| const content = await fs.readFile(statusFilePath, 'utf-8'); | ||||||||||||||||||||||||||||||||||||||||
| const status = JSON.parse(content) as PlaybookStatus; | ||||||||||||||||||||||||||||||||||||||||
| const mainWindow = getMainWindow(); | ||||||||||||||||||||||||||||||||||||||||
| if (isWebContentsAvailable(mainWindow)) { | ||||||||||||||||||||||||||||||||||||||||
| mainWindow.webContents.send('autorun:statusChanged', { | ||||||||||||||||||||||||||||||||||||||||
| projectPath, | ||||||||||||||||||||||||||||||||||||||||
| status, | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||
| logger.warn(`${LOG_CONTEXT} Failed to read STATUS.json: ${err}`, LOG_CONTEXT); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| }, 500); | ||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| watcher.on('add', handleStatusChange); | ||||||||||||||||||||||||||||||||||||||||
| watcher.on('change', handleStatusChange); | ||||||||||||||||||||||||||||||||||||||||
| watcher.on('unlink', () => { | ||||||||||||||||||||||||||||||||||||||||
| // File was deleted — clear status in renderer | ||||||||||||||||||||||||||||||||||||||||
| const mainWindow = getMainWindow(); | ||||||||||||||||||||||||||||||||||||||||
| if (isWebContentsAvailable(mainWindow)) { | ||||||||||||||||||||||||||||||||||||||||
| mainWindow.webContents.send('autorun:statusChanged', { | ||||||||||||||||||||||||||||||||||||||||
| projectPath, | ||||||||||||||||||||||||||||||||||||||||
| status: null, | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| watcher.on('error', (error) => { | ||||||||||||||||||||||||||||||||||||||||
| logger.error( | ||||||||||||||||||||||||||||||||||||||||
| `${LOG_CONTEXT} STATUS.json watcher error for ${projectPath}`, | ||||||||||||||||||||||||||||||||||||||||
| LOG_CONTEXT, | ||||||||||||||||||||||||||||||||||||||||
| error | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| statusWatchers.set(projectPath, watcher); | ||||||||||||||||||||||||||||||||||||||||
| logger.info(`Started watching STATUS.json in: ${projectPath}`, LOG_CONTEXT); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return { status: initialStatus }; | ||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Stop watching STATUS.json for a project | ||||||||||||||||||||||||||||||||||||||||
| ipcMain.handle( | ||||||||||||||||||||||||||||||||||||||||
| 'autorun:unwatchStatus', | ||||||||||||||||||||||||||||||||||||||||
| createIpcHandler(handlerOpts('unwatchStatus', false), async (projectPath: string) => { | ||||||||||||||||||||||||||||||||||||||||
| if (statusWatchers.has(projectPath)) { | ||||||||||||||||||||||||||||||||||||||||
| statusWatchers.get(projectPath)?.close(); | ||||||||||||||||||||||||||||||||||||||||
| statusWatchers.delete(projectPath); | ||||||||||||||||||||||||||||||||||||||||
| logger.info(`Stopped watching STATUS.json in: ${projectPath}`, LOG_CONTEXT); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| return {}; | ||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // Clean up all watchers on app quit | ||||||||||||||||||||||||||||||||||||||||
| app.on('before-quit', () => { | ||||||||||||||||||||||||||||||||||||||||
| for (const [folderPath, watcher] of autoRunWatchers) { | ||||||||||||||||||||||||||||||||||||||||
| watcher.close(); | ||||||||||||||||||||||||||||||||||||||||
| logger.info(`Cleaned up Auto Run watcher for: ${folderPath}`, LOG_CONTEXT); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| autoRunWatchers.clear(); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| for (const [projectPath, watcher] of statusWatchers) { | ||||||||||||||||||||||||||||||||||||||||
| watcher.close(); | ||||||||||||||||||||||||||||||||||||||||
| logger.info(`Cleaned up STATUS.json watcher for: ${projectPath}`, LOG_CONTEXT); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| statusWatchers.clear(); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| logger.debug(`${LOG_CONTEXT} Auto Run IPC handlers registered`); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1288,3 +1396,10 @@ export function registerAutorunHandlers( | |||||||||||||||||||||||||||||||||||||||
| export function getAutoRunWatcherCount(): number { | ||||||||||||||||||||||||||||||||||||||||
| return autoRunWatchers.size; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * Get the current number of active STATUS.json watchers (for testing/debugging) | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| export function getStatusWatcherCount(): number { | ||||||||||||||||||||||||||||||||||||||||
| return statusWatchers.size; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -839,6 +839,32 @@ export function useBatchProcessor({ | |
| // State machine: INITIALIZING -> RUNNING (initialization complete) | ||
| dispatch({ type: 'SET_RUNNING', sessionId }); | ||
|
|
||
| // Start watching .maestro/STATUS.json for playbook status updates | ||
| const statusProjectPath = effectiveCwd; | ||
| let statusCleanup: (() => void) | null = null; | ||
| try { | ||
| const { status: initialStatus } = | ||
| await window.maestro.autorun.watchStatus(statusProjectPath); | ||
| if (initialStatus) { | ||
| dispatch({ | ||
| type: 'UPDATE_PLAYBOOK_STATUS', | ||
| sessionId, | ||
| status: initialStatus, | ||
| }); | ||
| } | ||
| statusCleanup = window.maestro.autorun.onStatusChanged((data) => { | ||
| if (data.projectPath === statusProjectPath) { | ||
| dispatch({ | ||
| type: 'UPDATE_PLAYBOOK_STATUS', | ||
| sessionId, | ||
| status: data.status ?? undefined, | ||
| }); | ||
| } | ||
| }); | ||
| } catch { | ||
| // STATUS.json watching is optional — don't fail the batch | ||
| } | ||
|
|
||
|
Comment on lines
+842
to
+867
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make STATUS watcher lifecycle non-leaky and non-silent. At Line 864 and Line 1762, errors are swallowed silently, and cleanup is only on the normal completion path. If any awaited operation throws before cleanup, the status listener/watch can remain active for that session. Please move watcher teardown into a Suggested direction+// import { captureException } from '../../../utils/sentry';
let statusCleanup: (() => void) | null = null;
const statusProjectPath = effectiveCwd;
-try {
+try {
const { status: initialStatus } = await window.maestro.autorun.watchStatus(statusProjectPath);
...
statusCleanup = window.maestro.autorun.onStatusChanged(...);
-} catch {
- // STATUS.json watching is optional — don't fail the batch
+} catch (error) {
+ // expected-recoverable path: watcher unavailable
+ // captureException(error, { context: { sessionId, statusProjectPath, phase: 'watchStatus:init' } });
}
+try {
+ // existing batch execution body
+} finally {
+ try {
+ statusCleanup?.();
+ } catch (error) {
+ // captureException(error, { context: { sessionId, statusProjectPath, phase: 'watchStatus:unsubscribe' } });
+ }
+
+ try {
+ await window.maestro.autorun.unwatchStatus(statusProjectPath);
+ } catch (error) {
+ // captureException(error, { context: { sessionId, statusProjectPath, phase: 'watchStatus:unwatch' } });
+ }
+}As per coding guidelines, Also applies to: 1756-1765 🤖 Prompt for AI Agents |
||
| // Prevent system sleep while Auto Run is active | ||
| window.maestro.power.addReason(`autorun:${sessionId}`); | ||
|
|
||
|
|
@@ -1727,6 +1753,16 @@ export function useBatchProcessor({ | |
| } | ||
| } | ||
|
|
||
| // Clean up STATUS.json watcher | ||
| if (statusCleanup) { | ||
| statusCleanup(); | ||
| } | ||
| try { | ||
| await window.maestro.autorun.unwatchStatus(statusProjectPath); | ||
| } catch { | ||
| // Ignore cleanup errors | ||
| } | ||
|
|
||
| // Critical: Always flush debounced updates and dispatch COMPLETE_BATCH to clean up state. | ||
| // These operations are safe regardless of mount state - React handles reducer dispatches gracefully, | ||
| // and broadcasts are external calls that don't affect React state. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use per-project debounce timers to avoid cross-project status loss.
A single module-level debounce timer means one project’s status update can cancel another project’s pending update. This can drop or misorder events when multiple runs/watchers are active.
💡 Suggested fix (isolate debounce state by project)
Also applies to: 1313-1317, 1366-1369, 1383-1387
🤖 Prompt for AI Agents