From 1ad9db760614efe02e5f1a90eca353c793b0e14c Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Wed, 28 Oct 2026 09:25:50 +0100 Subject: [PATCH] fix: keep a remote session's fix plan authoritative (release review, #722) #722 stopped handing a remote session's workingDir to the Ralph tracker, which also left isFileAuthoritative false for remote sessions only. That flag is what keeps the output-based completion guessers off, and it is true for every local session with a working directory, so a remote loop could end, and mark every todo done, on a prose line such as "All tasks completed" while a local one cannot. setRemoteWorkingDir() records the path without reading or watching the local disk, so the fix plan stays authoritative as for a local session; the file still reaches the tracker only through fix-plan/read, over ssh. The test pins both, and fails on the previous head. Co-Authored-By: Claude Opus 5.5 --- docs/remote-sessions.md | 9 ++++-- src/ralph-fix-plan-watcher.ts | 14 ++++++++++ src/ralph-tracker.ts | 9 ++++++ src/web/server.ts | 12 ++++++-- test/ralph-fix-plan-remote.test.ts | 44 ++++++++++++++++++++++++------ 5 files changed, 73 insertions(+), 15 deletions(-) diff --git a/docs/remote-sessions.md b/docs/remote-sessions.md index a5db8ef3..1c36a9ff 100644 --- a/docs/remote-sessions.md +++ b/docs/remote-sessions.md @@ -432,9 +432,12 @@ path but the SESSION (`session.remote`): a remote session never falls back to lo records, which are keyed to the session that registered them. **Local watchers skip a remote session.** The Ralph tracker's `@fix_plan.md` auto-load -(and the `fs.watch` behind it) never runs for a remote session: `setupSessionListeners()` -(`server.ts`) does not hand the tracker a remote `workingDir`, because the watcher reads -the LOCAL disk, where that path is missing or a different tree. +(and the `fs.watch` behind it) never runs for a remote session, because the watcher reads +the LOCAL disk, where that path is missing or a different tree: `setupSessionListeners()` +(`server.ts`) calls `setRemoteWorkingDir()` instead, which only records the path. The fix +plan stays authoritative exactly as for a local session, so the tracker's output-based +completion guessers stay off there too; the file reaches the tracker only through +`fix-plan/read`, over ssh. The image watcher skips remote sessions the same way. ## Wake-on-LAN from user input diff --git a/src/ralph-fix-plan-watcher.ts b/src/ralph-fix-plan-watcher.ts index 17396fde..305f3b39 100644 --- a/src/ralph-fix-plan-watcher.ts +++ b/src/ralph-fix-plan-watcher.ts @@ -248,6 +248,20 @@ export class RalphFixPlanWatcher extends EventEmitter { this.startWatchingFixPlan(); } + /** + * Record a REMOTE session's working directory (a path on the remote host) without reading or + * watching it: the local disk holds nothing of it. The fix plan stays authoritative exactly as for a + * local session, so the output-based completion guessers stay off (they are off for every session + * with a working directory); the file reaches the tracker only through an explicit import + * (`POST .../fix-plan/read`, which reads it over ssh). + * @param workingDir - The session's working directory on the remote host + */ + setRemoteWorkingDir(workingDir: string): void { + this.stopWatchingFixPlan(); + this._workingDir = workingDir; + this._fixPlanPath = join(workingDir, '@fix_plan.md'); + } + /** * Load @fix_plan.md from disk if it exists. * Called on initialization and when file changes are detected. diff --git a/src/ralph-tracker.ts b/src/ralph-tracker.ts index 9fc6a7c8..40f25158 100644 --- a/src/ralph-tracker.ts +++ b/src/ralph-tracker.ts @@ -717,6 +717,15 @@ export class RalphTracker extends EventEmitter { this.fixPlanWatcher.setWorkingDir(workingDir); } + /** + * Record a remote session's working directory without touching the local disk (see + * `RalphFixPlanWatcher.setRemoteWorkingDir`): the fix plan stays authoritative, as for a local session. + * @param workingDir - The session's working directory on the remote host + */ + setRemoteWorkingDir(workingDir: string): void { + this.fixPlanWatcher.setRemoteWorkingDir(workingDir); + } + /** * Load @fix_plan.md from disk if it exists. */ diff --git a/src/web/server.ts b/src/web/server.ts index f5b9828f..ddfc296d 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2068,9 +2068,15 @@ export class WebServer extends EventEmitter { // Set working directory for Ralph tracker to auto-load @fix_plan.md (not supported for external CLIs). // A remote session's workingDir is a path on the remote host, and the watcher reads and - // fs.watch()es it on the LOCAL disk, so it is skipped there too. - if (!isExternalCliMode(session.mode) && !session.remote) { - session.ralphTracker.setWorkingDir(session.workingDir); + // fs.watch()es it on the LOCAL disk, so a remote session only records the path: the fix plan stays + // authoritative as for a local session (keeping the output-based completion guessers off), and its + // file is read over ssh only on an explicit import (fix-plan/read). + if (!isExternalCliMode(session.mode)) { + if (session.remote) { + session.ralphTracker.setRemoteWorkingDir(session.workingDir); + } else { + session.ralphTracker.setWorkingDir(session.workingDir); + } } // Start watching for new images in this session's working directory (if enabled globally and per-session). diff --git a/test/ralph-fix-plan-remote.test.ts b/test/ralph-fix-plan-remote.test.ts index 9127ac0f..c499b65e 100644 --- a/test/ralph-fix-plan-remote.test.ts +++ b/test/ralph-fix-plan-remote.test.ts @@ -7,12 +7,13 @@ * so every server start logged * `[RalphFixPlanWatcher] Could not watch @fix_plan.md: Error: ENOENT ...` once per * remote session (and where a same-named local directory did exist, it watched - * the wrong tree). Setting the path also made the tracker treat the unreadable - * file as authoritative (`isFileAuthoritative`), which switched off its - * output-based todo completion for every remote session. - * `setupSessionListeners()` already skips external CLI modes (the tracker is - * not supported there); a remote location is the other case with no local - * file to watch. + * the wrong tree). A remote session therefore only RECORDS the path + * (`setRemoteWorkingDir`): nothing is read or watched locally, and the fix + * plan stays authoritative (`isFileAuthoritative`) exactly as for a local + * session, so the tracker's output-based completion guessers stay off. They + * are off for every session with a working directory; turning them on for + * remote sessions alone would let a prose line such as "All tasks completed" + * mark every todo done and end the loop there and nowhere else. * * Drives the real `WebServer.setupSessionListeners()`, like * discard-partially-built-session.test.ts. @@ -86,16 +87,41 @@ describe('Ralph @fix_plan.md watcher and the session location', () => { it('does not watch a remote session working directory on the local disk', async () => { const session = buildSession(true); const setWorkingDir = vi.spyOn(session.ralphTracker, 'setWorkingDir'); + const setRemoteWorkingDir = vi.spyOn(session.ralphTracker, 'setRemoteWorkingDir'); const log = vi.spyOn(console, 'log').mockImplementation(() => {}); await internals.registerSessionWithLayout(session); await internals.setupSessionListeners(session); expect(setWorkingDir).not.toHaveBeenCalled(); + expect(setRemoteWorkingDir).toHaveBeenCalledWith(REMOTE_PATH); expect(log.mock.calls.some((args) => String(args[0]).includes('Could not watch @fix_plan.md'))).toBe(false); - // A fix-plan path that can never be read must not make the file "authoritative": - // that turns off the tracker's output-based todo completion for the session. - expect(session.ralphTracker.isFileAuthoritative).toBe(false); + // Same authority as a local session: the output-based completion guessers stay off. + expect(session.ralphTracker.isFileAuthoritative).toBe(true); + }); + + it('keeps a remote session todo list out of the prose completion guessers', async () => { + const session = buildSession(true); + vi.spyOn(console, 'log').mockImplementation(() => {}); + + await internals.registerSessionWithLayout(session); + await internals.setupSessionListeners(session); + + const tracker = session.ralphTracker; + tracker.enable(); + tracker.startLoop('ALLDONE'); + const completions: string[] = []; + tracker.on('completionDetected', (phrase: string) => completions.push(phrase)); + tracker.processTerminalData('- [ ] Write the parser\n- [ ] Add tests\n'); + const pending = () => tracker.todos.filter((t) => t.status !== 'completed').length; + expect(pending()).toBe(2); + + // Prose, not the configured completion phrase: it must not end the loop. + tracker.processTerminalData('All tasks completed\n'); + + expect(pending()).toBe(2); + expect(tracker.loopState.active).toBe(true); + expect(completions).toEqual([]); }); it('still watches a local session working directory', async () => {