Skip to content

fix: add SSH remote support for wizard file operations#567

Open
rothnic wants to merge 6 commits intoRunMaestro:mainfrom
rothnic:fix/wizard-ssh-remote-support
Open

fix: add SSH remote support for wizard file operations#567
rothnic wants to merge 6 commits intoRunMaestro:mainfrom
rothnic:fix/wizard-ssh-remote-support

Conversation

@rothnic
Copy link

@rothnic rothnic commented Mar 13, 2026

Summary

Adds SSH remote support for wizard file operations to fix issues when generating Auto Run documents on remote hosts.

Changes

  • App.tsx: Added SerializableWizardState import and wrapped openWizard with resume state check
  • phaseGenerator.ts: Added sshRemoteId parameter extraction and passing to watchFolder and writeDoc calls
  • PreparingPlanScreen.tsx: Added sshRemoteId extraction from state.sessionSshRemoteConfig
  • inlineWizardDocumentGeneration.ts: Added sshRemoteId parameter to watchFolder call

Fixes

  • Wizard resume not triggering when clicking Wizard button with saved state
  • File operations failing on remote hosts due to missing SSH remote ID

Testing

All existing tests pass. The changes ensure SSH remote configuration is properly passed through the wizard document generation flow.

Related

Replaces and consolidates PR #548 (which had messy commit history with debug logging).

Summary by CodeRabbit

  • New Features

    • Wizards can resume from your previous step and progress after restart, preserving workflow state.
    • SSH remote support added for document generation, file operations, and real-time watching, enabling secure remote workflows.
  • Bug Fixes

    • Improved error handling when detecting resume state to avoid interruptions and ensure fallback to the normal wizard flow.

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a wrapper openWizardModal in App that reads window.maestro.settings for a resumable wizard state, logs errors via captureException, and falls back to the base modal opener. Separately, passes an SSH remote id through wizard document generation (watchFolder, saveDocuments, writeDoc).

Changes

Cohort / File(s) Summary
Wizard Resume State Management
src/renderer/App.tsx
Added openWizardModal wrapper that reads window.maestro.settings.wizardResumeState, casts to SerializableWizardState, opens wizardResume modal when resumable, captures exceptions via captureException, and falls back to the base _baseOpenWizardModal. Exported SerializableWizardState type imported.
Preparing Plan — SSH context
src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
Compute sshRemoteId from state.sessionSshRemoteConfig and pass it into phaseGenerator.saveDocuments and generateDocuments options; updated callback dependencies.
Phase generator — SSH support
src/renderer/components/Wizard/services/phaseGenerator.ts
Added sshRemoteId handling: pass through to watchFolder, readFile, autorun.writeDoc, and include in save paths. Public saveDocuments signature extended with optional sshRemoteId.
Inline wizard document streaming
src/renderer/services/inlineWizardDocumentGeneration.ts
Pass sshRemoteId into watchFolder and into downstream saveDocument calls; use nullish handling for remote id extraction. Public watchFolder updated to accept sshRemoteId.

Sequence Diagram

sequenceDiagram
    participant App as App (Maestro)
    participant Settings as window.maestro.settings
    participant ModalStore as ModalStore
    participant useWizard as useWizard()

    App->>Settings: read wizardResumeState
    Settings-->>App: return resume data

    alt resumeState present & resumable step
        App->>ModalStore: openModal('wizardResume', SerializableWizardState)
        ModalStore-->>App: modal opened
    else no applicable resume state or error
        App->>useWizard: call _baseOpenWizardModal()
        useWizard-->>App: wizard modal opened
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and concisely summarizes the main change: adding SSH remote support for wizard file operations, which is the primary focus across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR adds SSH remote session support to two wizard document-generation code paths (phaseGenerator.ts and inlineWizardDocumentGeneration.ts) and fixes wizard resume state not triggering when the Wizard button is clicked (App.tsx). The approach — extracting sshRemoteId from the session's SSH config and threading it through watchFolder, readFile, and writeDoc calls — is consistent with how the rest of the codebase handles remote file operations.

Key findings:

  • Bug — incomplete SSH fix in inlineWizardDocumentGeneration.ts: watchFolder was correctly updated to pass sshRemoteId (line 846), but the readFile call inside the onFileChanged → readWithRetry handler at line 874 still omits sshRemoteId. On remote SSH sessions, every in-flight file read triggered by the watcher will target the local filesystem and silently fail. The equivalent call in phaseGenerator.ts (updated in this same PR) correctly passes sshRemoteId and can serve as the reference fix.
  • Style — misplaced import in App.tsx: import { captureException } from './utils/sentry' is placed after const lazy-load declarations (line 50), violating the convention that all import statements appear before any code.

Confidence Score: 3/5

  • Not safe to merge as-is — the inline wizard document generation path has an incomplete SSH fix that will cause silent read failures on remote sessions.
  • Three of the four files are implemented correctly and cleanly. However, inlineWizardDocumentGeneration.ts has a definite omission: watchFolder passes sshRemoteId but the subsequent readFile call inside the same file-change callback does not, making the real-time file-streaming feature broken for SSH remote sessions. This is the exact scenario the PR claims to fix, so it represents a direct regression for a stated goal of the change.
  • src/renderer/services/inlineWizardDocumentGeneration.ts — the readFile call at line 874 is missing sshRemoteId.

Important Files Changed

Filename Overview
src/renderer/services/inlineWizardDocumentGeneration.ts SSH remote support partially applied: watchFolder and saveDocument correctly pass sshRemoteId, but the readFile call inside the file-change event handler at line 874 is missing sshRemoteId, causing all in-flight file reads to target the local filesystem on remote sessions.
src/renderer/App.tsx Adds wizard resume-state check that gates openWizard behind a settings read; logic is sound and uses defensive validation, but the new captureException import is placed after const lazy declarations (style issue).
src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx Correctly extracts sshRemoteId from state.sessionSshRemoteConfig and passes it to phaseGenerator.saveDocuments; dependency array updated accordingly.
src/renderer/components/Wizard/services/phaseGenerator.ts Both watchFolder and readFile calls are consistently updated to pass sshRemoteId; saveDocuments signature extended cleanly.

Sequence Diagram

sequenceDiagram
    participant User
    participant App as App.tsx
    participant Settings as window.maestro.settings
    participant ModalStore as ModalStore
    participant WizardScreen as PreparingPlanScreen
    participant PhaseGen as phaseGenerator.ts
    participant InlineGen as inlineWizardDocGen.ts
    participant FS as window.maestro.fs
    participant AutoRun as window.maestro.autorun

    User->>App: Click Wizard button
    App->>Settings: get('wizardResumeState')
    Settings-->>App: saved state or null
    alt Has resumable saved state
        App->>ModalStore: openModal('wizardResume', {state})
    else No saved state
        App->>App: _baseOpenWizardModal()
    end

    WizardScreen->>WizardScreen: Extract sshRemoteId from state.sessionSshRemoteConfig
    WizardScreen->>PhaseGen: saveDocuments(path, docs, cb, 'Initiation', sshRemoteId)
    PhaseGen->>AutoRun: watchFolder(autoRunPath, sshRemoteId)
    PhaseGen->>AutoRun: writeDoc(path, filename, content, sshRemoteId)
    AutoRun-->>PhaseGen: file change event
    PhaseGen->>FS: readFile(fullPath, sshRemoteId) ✅

    Note over InlineGen: inlineWizardDocGen flow
    InlineGen->>AutoRun: watchFolder(subfolderPath, sshRemoteId) ✅
    AutoRun-->>InlineGen: file change event
    InlineGen->>FS: readFile(fullPath) ❌ missing sshRemoteId
    InlineGen->>AutoRun: saveDocument → writeDoc(path, filename, content, sshRemoteId) ✅
Loading

Comments Outside Diff (2)

  1. src/renderer/services/inlineWizardDocumentGeneration.ts, line 874 (link)

    Missing sshRemoteId in readFile call inside file-change handler

    The watchFolder call on line 846 was correctly updated to pass sshRemoteId, so the file watcher will detect events on the remote SSH host. However, when an event fires and readWithRetry tries to read the newly-created file, it calls readFile(fullPath) without the SSH remote ID. This means every file-read attempt inside this watcher will hit the local filesystem rather than the remote host, silently failing (or reading stale/wrong data) for any remote session.

    The parallel fix in phaseGenerator.ts (line 1025 of the same PR) correctly passes sshRemoteId:

    const content = await window.maestro.fs.readFile(fullPath, sshRemoteId);
    

    The same correction is needed here:

  2. src/renderer/App.tsx, line 50 (link)

    import statement placed after variable declarations

    import { captureException } from './utils/sentry'; is placed after several const lazy-load declarations (e.g. const DirectorNotesModal = lazy(...)). ES module import statements are meant to be at the top of the file before any code. While bundlers like Webpack/Vite hoist them at build time, placing them mid-file can confuse linters (e.g. ESLint's import/first rule), IDEs, and other developers. Move this import alongside the other utility imports at the top of the file.

Last reviewed commit: 88bfa49

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx (1)

771-782: ⚠️ Potential issue | 🔴 Critical

sshRemoteId is computed but not forwarded to saveDocuments.

Line 771 prepares the remote ID, but the saveDocuments invocation omits the new 5th argument, so remote writes still run without SSH context in this flow.

🐛 Proposed fix
							const saveResult = await phaseGenerator.saveDocuments(
								state.directoryPath,
								genResult.documents,
								(file) => {
									// Add file to the created files list as it's saved
									addCreatedFile(file);
								},
-								'Initiation' // Save in Initiation subfolder
+								'Initiation', // Save in Initiation subfolder
+								sshRemoteId
							);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx` around lines
771 - 782, The variable sshRemoteId is computed from
state.sessionSshRemoteConfig but never passed into phaseGenerator.saveDocuments,
so remote writes run without SSH context; update the call to
phaseGenerator.saveDocuments (the invocation that currently passes
state.directoryPath, genResult.documents, the progress callback addCreatedFile,
and 'Initiation') to include sshRemoteId as the fifth argument so saveDocuments
receives the SSH remote id; locate the call to phaseGenerator.saveDocuments in
PreparingPlanScreen.tsx and append sshRemoteId to its parameter list.
🧹 Nitpick comments (1)
docs/releases.md (1)

201-201: Normalize product spelling and hyphenation consistency.

Use GitHub (Line 201), and keep one style for pre-release wording (pre-release vs prerelease) consistently across the page (Line 266).

Also applies to: 266-266

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` at line 201, Replace the inconsistent product spelling and
hyphenation: change "Github Worktree" to "GitHub Worktree" (the phrase currently
"Github Worktree was added") and normalize all occurrences of the pre-release
wording to a single style (use "pre-release" consistently instead of
"prerelease") throughout the document, searching for variants like
"pre-release", "prerelease", and "pre release" and updating them to
"pre-release".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/releases.md`:
- Line 101: Fix the typo in the user-facing sentence that currently reads
"Launch from file preview or from the FIle tree panel" by changing "FIle" to
"File" in the "🗄️ Document Graphs." line of docs/releases.md so the sentence
becomes "Launch from file preview or from the File tree panel."
- Line 107: Replace the incorrect heading "Smaller Changes in 014.x" with the
correctly formatted version label "Smaller Changes in 0.14.x" in the
docs/releases.md heading line so it matches the project's versioning style;
locate the heading string "# Smaller Changes in 014.x" and update it to "#
Smaller Changes in 0.14.x".
- Line 20: Several release-section headings (e.g., the heading "Major 0.15.x
Additions") are using top-level or inconsistent hash depth which breaks
hierarchy and TOC; update those subsection headings under each "vX.Y.x" section
to a consistent third-level header (use "###") so they nest correctly. Search
for headings that jump back to "#" or "##" inside version blocks (including the
ones noted like the "Major 0.15.x Additions" heading and the other similar
section headings) and change their leading hashes to "###" to maintain
consistent heading depth across the release notes.

In `@src/renderer/App.tsx`:
- Around line 354-373: The catch in openWizardModal currently only logs errors
and falls back, which hides failures from production monitoring; update the
catch to capture the exception with Sentry (e.g. call Sentry.captureException(e)
or Sentry.withScope to add context such as { handler: 'openWizardModal', key:
'wizardResumeState' }) before the console.error, ensuring Sentry is
imported/available, then continue with the existing fallback to
_baseOpenWizardModal(); reference openWizardModal, window.maestro.settings.get,
and useModalStore.getState().openModal when adding context.

---

Outside diff comments:
In `@src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx`:
- Around line 771-782: The variable sshRemoteId is computed from
state.sessionSshRemoteConfig but never passed into phaseGenerator.saveDocuments,
so remote writes run without SSH context; update the call to
phaseGenerator.saveDocuments (the invocation that currently passes
state.directoryPath, genResult.documents, the progress callback addCreatedFile,
and 'Initiation') to include sshRemoteId as the fifth argument so saveDocuments
receives the SSH remote id; locate the call to phaseGenerator.saveDocuments in
PreparingPlanScreen.tsx and append sshRemoteId to its parameter list.

---

Nitpick comments:
In `@docs/releases.md`:
- Line 201: Replace the inconsistent product spelling and hyphenation: change
"Github Worktree" to "GitHub Worktree" (the phrase currently "Github Worktree
was added") and normalize all occurrences of the pre-release wording to a single
style (use "pre-release" consistently instead of "prerelease") throughout the
document, searching for variants like "pre-release", "prerelease", and "pre
release" and updating them to "pre-release".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1d40375-a27b-47bc-ac63-7e2002621d63

📥 Commits

Reviewing files that changed from the base of the PR and between 9ff6839 and 69c00f5.

📒 Files selected for processing (5)
  • docs/releases.md
  • src/renderer/App.tsx
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
  • src/renderer/components/Wizard/services/phaseGenerator.ts
  • src/renderer/services/inlineWizardDocumentGeneration.ts

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/renderer/App.tsx`:
- Around line 360-369: The code opens the 'wizardResume' modal by casting a
loosely-validated saved object to SerializableWizardState based only on presence
of currentStep; tighten validation by checking that saved.currentStep is one of
the known resumable steps (e.g. an allowlist like ['step-a','step-b',...]) and
that required SerializableWizardState fields exist/types before calling
useModalStore.getState().openModal('wizardResume', { state: saved as
SerializableWizardState });; update the guard around saved/currentStep to
validate against the allowlist (and any other mandatory keys) and only then cast
and pass saved to openModal.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2a8ba2a8-f3b4-4187-a95d-635c621c22c0

📥 Commits

Reviewing files that changed from the base of the PR and between 69c00f5 and 921a768.

📒 Files selected for processing (3)
  • src/renderer/App.tsx
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
  • src/renderer/services/inlineWizardDocumentGeneration.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
  • src/renderer/services/inlineWizardDocumentGeneration.ts

@rothnic rothnic force-pushed the fix/wizard-ssh-remote-support branch from 45e94f7 to c14d109 Compare March 13, 2026 11:55
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (3)
docs/releases.md (3)

20-20: ⚠️ Potential issue | 🟡 Minor

Keep subsection heading depth consistent within each version section.

Line 20 and the other referenced lines still jump back to #/## under ## vX.Y.x sections, which breaks document hierarchy and TOC nesting. Use ### for these subsection headers.

Also applies to: 32-32, 107-107, 170-170, 174-174, 178-178, 203-203, 295-295, 303-303, 313-313, 318-318, 324-324

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` at line 20, Several subsection headings under version
sections use top-level/second-level headers (e.g., the line "# Major 0.15.x
Additions" and other headers listed) which breaks hierarchy and TOC nesting;
update those subsection headers inside each "## vX.Y.x" section to use
third-level headings (###) instead of "#" or "##" so their depth is consistent
across the document and the TOC nests correctly; locate the offending headings
by searching for "Major 0.15.x Additions" and other repeated
top-level/second-level subsection titles and convert them to "###" headings.

107-107: ⚠️ Potential issue | 🟡 Minor

Correct version label formatting in the heading.

Line 107 uses 014.x; this should be 0.14.x for consistency with the rest of the changelog.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` at line 107, Update the malformed changelog heading
"Smaller Changes in 014.x" to use the correct semantic version format by
replacing "014.x" with "0.14.x" so the heading reads "Smaller Changes in
0.14.x"; locate the heading string "Smaller Changes in 014.x" in
docs/releases.md and make this single-word change to restore consistency with
other release headings.

101-101: ⚠️ Potential issue | 🟡 Minor

Fix typo in user-facing release text.

Line 101 still says FIle tree panel; this should be File tree panel.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/releases.md` at line 101, Fix the typo in the release note sentence that
currently reads "Launch from file preview or from the FIle tree panel" by
changing "FIle tree panel" to "File tree panel"; locate the sentence in
docs/releases.md (the line containing "Launch from file preview or from the FIle
tree panel") and update the capitalization to "File" so the user-facing text
reads "Launch from file preview or from the File tree panel."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/releases.md`:
- Line 201: The release note line starting with "Github Worktree support was
added." uses incorrect casing; update that occurrence to "GitHub Worktree
support was added." (replace "Github" with the official "GitHub") so the product
name uses correct casing in the release notes text.

In `@src/renderer/components/Wizard/services/phaseGenerator.ts`:
- Around line 979-987: The SSH remote id (sshRemoteId) is passed into
watchFolder but not used in subsequent read/fallback code paths; update the
file-reading calls to include the remote context so SSH sessions see
remote-generated docs: pass sshRemoteId into the fs.readFile invocation(s)
(referencing fs.readFile) and thread the sshRemoteId through the disk fallback
by updating readDocumentsFromDisk to accept and use a remoteId parameter
(referencing readDocumentsFromDisk), or call a remote-aware read API, ensuring
all places that read files use the sshRemoteId when
config.sshRemoteConfig?.enabled.

In `@src/renderer/services/inlineWizardDocumentGeneration.ts`:
- Around line 846-847: The SSH remote id (sshRemoteId) is not passed into the
fallback disk-read paths causing documentsFromWatcher to be empty; thread
sshRemoteId into the disk read functions so remote reads use the same context:
update the callsites that perform fs.readFile and the readDocumentsFromDisk
invocation to accept and forward sshRemoteId, and modify readDocumentsFromDisk
(and any helper that does the actual fs.readFile) to use sshRemoteId when
resolving/reading files so watchFolder/saveDocument and the fallback path use
the same remote context.

---

Duplicate comments:
In `@docs/releases.md`:
- Line 20: Several subsection headings under version sections use
top-level/second-level headers (e.g., the line "# Major 0.15.x Additions" and
other headers listed) which breaks hierarchy and TOC nesting; update those
subsection headers inside each "## vX.Y.x" section to use third-level headings
(###) instead of "#" or "##" so their depth is consistent across the document
and the TOC nests correctly; locate the offending headings by searching for
"Major 0.15.x Additions" and other repeated top-level/second-level subsection
titles and convert them to "###" headings.
- Line 107: Update the malformed changelog heading "Smaller Changes in 014.x" to
use the correct semantic version format by replacing "014.x" with "0.14.x" so
the heading reads "Smaller Changes in 0.14.x"; locate the heading string
"Smaller Changes in 014.x" in docs/releases.md and make this single-word change
to restore consistency with other release headings.
- Line 101: Fix the typo in the release note sentence that currently reads
"Launch from file preview or from the FIle tree panel" by changing "FIle tree
panel" to "File tree panel"; locate the sentence in docs/releases.md (the line
containing "Launch from file preview or from the FIle tree panel") and update
the capitalization to "File" so the user-facing text reads "Launch from file
preview or from the File tree panel."

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 477cdf73-476e-41bb-a716-da4f409efd26

📥 Commits

Reviewing files that changed from the base of the PR and between 45e94f7 and c14d109.

📒 Files selected for processing (5)
  • docs/releases.md
  • src/renderer/App.tsx
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
  • src/renderer/components/Wizard/services/phaseGenerator.ts
  • src/renderer/services/inlineWizardDocumentGeneration.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx

rothnic added 4 commits March 13, 2026 07:43
…cion

Addresses Greptile review comments:

- PreparingPlanScreen.tsx: Pass sshRemoteId as 5th parameter to phaseGenerator.saveDocuments()

- inlineWizardDocumentGeneration.ts: Use ?? undefined at extraction time, remove || undefined at call sites
- Add Sentry captureException to openWizardModal error handler (App.tsx)

- Greptile fixes already committed: sshRemoteId parameter passing and ?? undefined pattern
- Add explicit validation that currentStep is a known resumable step

- Check typeof saved.currentStep === 'string' before includes() check

- Validates against known steps: directory-selection, conversation, preparing-plan, phase-review
@rothnic rothnic force-pushed the fix/wizard-ssh-remote-support branch from c14d109 to bc501e3 Compare March 13, 2026 12:47
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/renderer/components/Wizard/services/phaseGenerator.ts (1)

1025-1025: ⚠️ Potential issue | 🟠 Major

File read inside watcher callback lacks SSH remote context.

The fs.readFile(fullPath) call at Line 1025 doesn't pass sshRemoteId. When the watcher detects file creation on a remote session, this read will fail or read from the wrong location. The sshRemoteId is available in the outer closure scope (defined at lines 980-982) and should be passed here.

🔧 Proposed fix
-												const content = await window.maestro.fs.readFile(fullPath);
+												const content = await window.maestro.fs.readFile(fullPath, sshRemoteId);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/Wizard/services/phaseGenerator.ts` at line 1025, The
file read inside the watcher callback uses window.maestro.fs.readFile(fullPath)
without the remote context; update the call to include the outer-scope
sshRemoteId so remote reads succeed (e.g., window.maestro.fs.readFile(fullPath,
sshRemoteId) or the API-specific options object containing sshRemoteId),
ensuring you modify the read in the watcher callback that references fullPath so
it forwards sshRemoteId from the closure.
src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx (1)

830-839: ⚠️ Potential issue | 🟡 Minor

Missing state.sessionSshRemoteConfig in dependency array.

The startGeneration callback reads state.sessionSshRemoteConfig at lines 729 and 771, but it's not included in the dependency array. If the SSH config changes during the wizard flow, the callback would use stale values.

🛠️ Proposed fix
 	}, [
 		state.selectedAgent,
 		state.directoryPath,
 		state.agentName,
 		state.conversationHistory,
+		state.sessionSshRemoteConfig,
 		setGeneratingDocuments,
 		setGeneratedDocuments,
 		setGenerationError,
 		nextStep,
 	]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx` around lines
830 - 839, The startGeneration callback (used in PreparingPlanScreen) reads
state.sessionSshRemoteConfig but the useCallback dependency array at the end of
the file omits it; add state.sessionSshRemoteConfig to the dependency array that
currently includes state.selectedAgent, state.directoryPath, state.agentName,
state.conversationHistory, setGeneratingDocuments, setGeneratedDocuments,
setGenerationError, and nextStep so the callback captures updates to the SSH
config and avoids using stale values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx`:
- Around line 830-839: The startGeneration callback (used in
PreparingPlanScreen) reads state.sessionSshRemoteConfig but the useCallback
dependency array at the end of the file omits it; add
state.sessionSshRemoteConfig to the dependency array that currently includes
state.selectedAgent, state.directoryPath, state.agentName,
state.conversationHistory, setGeneratingDocuments, setGeneratedDocuments,
setGenerationError, and nextStep so the callback captures updates to the SSH
config and avoids using stale values.

In `@src/renderer/components/Wizard/services/phaseGenerator.ts`:
- Line 1025: The file read inside the watcher callback uses
window.maestro.fs.readFile(fullPath) without the remote context; update the call
to include the outer-scope sshRemoteId so remote reads succeed (e.g.,
window.maestro.fs.readFile(fullPath, sshRemoteId) or the API-specific options
object containing sshRemoteId), ensuring you modify the read in the watcher
callback that references fullPath so it forwards sshRemoteId from the closure.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7587a77a-fa4d-45b7-b36e-9d3866def94e

📥 Commits

Reviewing files that changed from the base of the PR and between c14d109 and bc501e3.

📒 Files selected for processing (4)
  • src/renderer/App.tsx
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
  • src/renderer/components/Wizard/services/phaseGenerator.ts
  • src/renderer/services/inlineWizardDocumentGeneration.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/renderer/services/inlineWizardDocumentGeneration.ts

- phaseGenerator.ts: Pass sshRemoteId to fs.readFile in watcher callback (line 1025)

- PreparingPlanScreen.tsx: Add state.sessionSshRemoteConfig to useCallback dependency array
@rothnic
Copy link
Author

rothnic commented Mar 13, 2026

Addressed all feedback from coderabbit and greptile, this should be good for review.

@chr1syy
Copy link
Collaborator

chr1syy commented Mar 13, 2026

@coderabbitai re-review
@greptileai re-review

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

@chr1syy Sure, I'll re-review the PR now!


🧠 Learnings used
Learnt from: nolanmclark
Repo: RunMaestro/Maestro PR: 566
File: src/main/storage/copilot-session-storage.ts:297-299
Timestamp: 2026-03-13T02:46:43.536Z
Learning: In RunMaestro/Maestro, all session storage implementations (claude-session-storage.ts, codex-session-storage.ts, opencode-session-storage.ts, factory-droid-session-storage.ts, copilot-session-storage.ts) gate SSH remote operations using `if (sshConfig)` object truthiness checks, NOT `sshConfig?.enabled === true`. Do not flag this pattern as incorrect in future reviews.
✅ Actions performed

Full review triggered.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/renderer/services/inlineWizardDocumentGeneration.ts (1)

846-847: ⚠️ Potential issue | 🟠 Major

Remote watcher is wired, but remote fallback reads are still local.

watchFolder(..., sshRemoteId) is correct, but fallback paths still miss remote context: Line 874 uses readFile(fullPath) (no SSH ID) and Line 1099 calls readDocumentsFromDisk(subfolderPath) without SSH context. In remote sessions, this can still return zero docs on fallback.

🔧 Suggested fix
- const content = await window.maestro.fs.readFile(fullPath);
+ const content = await window.maestro.fs.readFile(fullPath, sshRemoteId);

- const diskDocs = await readDocumentsFromDisk(subfolderPath);
+ const diskDocs = await readDocumentsFromDisk(subfolderPath, sshRemoteId);

- async function readDocumentsFromDisk(autoRunFolderPath: string): Promise<ParsedDocument[]> {
+ async function readDocumentsFromDisk(
+   autoRunFolderPath: string,
+   sshRemoteId?: string
+ ): Promise<ParsedDocument[]> {

- const listResult = await window.maestro.autorun.listDocs(autoRunFolderPath);
+ const listResult = await window.maestro.autorun.listDocs(autoRunFolderPath, sshRemoteId);

- const readResult = await window.maestro.autorun.readDoc(autoRunFolderPath, fileBaseName);
+ const readResult = await window.maestro.autorun.readDoc(
+   autoRunFolderPath,
+   fileBaseName,
+   sshRemoteId
+ );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/services/inlineWizardDocumentGeneration.ts` around lines 846 -
847, The remote watch call uses watchFolder(subfolderPath, sshRemoteId) but the
fallback disk reads still use local-only functions; update the fallback reads to
pass the SSH context so remote sessions read from the remote filesystem: change
calls that use readFile(fullPath) to the SSH-aware variant (or add sshRemoteId
parameter) and update readDocumentsFromDisk(subfolderPath) to accept and forward
sshRemoteId (or call a readDocumentsFromDiskRemote(subfolderPath, sshRemoteId))
so both fallback branches use sshRemoteId consistently; ensure any helper
functions invoked by watchResult propagate sshRemoteId as well.
src/renderer/components/Wizard/services/phaseGenerator.ts (1)

979-987: ⚠️ Potential issue | 🟠 Major

SSH support is still incomplete in the disk fallback path.

The watcher now gets sshRemoteId, but Line 709 still calls readDocumentsFromDisk(config.directoryPath) without remote context, and the helper at Line 1160 reads via local listDocs/readDoc. If watcher events are unavailable for remote (expected), fallback can still miss generated docs.

🔧 Suggested fix
- const diskDocs = await this.readDocumentsFromDisk(config.directoryPath);
+ const diskDocs = await this.readDocumentsFromDisk(config.directoryPath, sshRemoteId);

- private async readDocumentsFromDisk(directoryPath: string): Promise<ParsedDocument[]> {
+ private async readDocumentsFromDisk(
+   directoryPath: string,
+   sshRemoteId?: string
+ ): Promise<ParsedDocument[]> {

- const listResult = await window.maestro.autorun.listDocs(autoRunPath);
+ const listResult = await window.maestro.autorun.listDocs(autoRunPath, sshRemoteId);

- const readResult = await window.maestro.autorun.readDoc(autoRunPath, fileBaseName);
+ const readResult = await window.maestro.autorun.readDoc(
+   autoRunPath,
+   fileBaseName,
+   sshRemoteId
+ );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/components/Wizard/services/phaseGenerator.ts` around lines 979 -
987, The disk-fallback path must include the remote context: update the call
site that currently invokes readDocumentsFromDisk(config.directoryPath) so it
also passes sshRemoteId (the same value computed for
window.maestro.autorun.watchFolder), and update the helper that reads files (the
helper calling listDocs/readDoc) to use the remote-aware APIs (or accept
sshRemoteId and call the remote variants) so remote document listing/reading
uses sshRemoteId instead of only local filesystem functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/renderer/components/Wizard/services/phaseGenerator.ts`:
- Around line 979-987: The disk-fallback path must include the remote context:
update the call site that currently invokes
readDocumentsFromDisk(config.directoryPath) so it also passes sshRemoteId (the
same value computed for window.maestro.autorun.watchFolder), and update the
helper that reads files (the helper calling listDocs/readDoc) to use the
remote-aware APIs (or accept sshRemoteId and call the remote variants) so remote
document listing/reading uses sshRemoteId instead of only local filesystem
functions.

In `@src/renderer/services/inlineWizardDocumentGeneration.ts`:
- Around line 846-847: The remote watch call uses watchFolder(subfolderPath,
sshRemoteId) but the fallback disk reads still use local-only functions; update
the fallback reads to pass the SSH context so remote sessions read from the
remote filesystem: change calls that use readFile(fullPath) to the SSH-aware
variant (or add sshRemoteId parameter) and update
readDocumentsFromDisk(subfolderPath) to accept and forward sshRemoteId (or call
a readDocumentsFromDiskRemote(subfolderPath, sshRemoteId)) so both fallback
branches use sshRemoteId consistently; ensure any helper functions invoked by
watchResult propagate sshRemoteId as well.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0bab768f-73f6-4285-812a-d6b2e69d0411

📥 Commits

Reviewing files that changed from the base of the PR and between 9ff6839 and 88bfa49.

📒 Files selected for processing (4)
  • src/renderer/App.tsx
  • src/renderer/components/Wizard/screens/PreparingPlanScreen.tsx
  • src/renderer/components/Wizard/services/phaseGenerator.ts
  • src/renderer/services/inlineWizardDocumentGeneration.ts

Addresses CodeRabbit review comments:

- phaseGenerator.ts: Pass sshRemoteId through readDocumentsFromDisk, listDocs, and readDoc calls

- inlineWizardDocumentGeneration.ts: Pass sshRemoteId through readFile, readDocumentsFromDisk, listDocs, and readDoc calls

- Ensures SSH context is preserved in all file operations for remote sessions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants