feat(factory): auto-start relayfile local mount on factory start (pear#364)#366
Conversation
…r#364) - Add `relayfile-binary.ts`: resolves the mount binary (env override → repo-root bin/ → dev relayfile/dist fallbacks) and `checkMountStaleness` (workspace mismatch, stale timestamp, dead pid → stale=true with reason) - Add `local-mount-preflight.ts`: `ensureLocalMount(workspaceId, startDir)` spawns the binary when state.json is missing/unparseable; warns to stderr when mount is stale; auth errors surface as actionable thrown messages - Wire `ensureLocalMount` into `fleet.ts` `factory start` handler before `factory.start()`, injectable via `FleetCliDeps` for test override Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Review limit reached
More reviews will be available in 44 minutes and 49 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a local-mount preflight system to the factory SDK. A new ChangesLocal Mount Preflight
Sequence Diagram(s)sequenceDiagram
participant FleetCLI as fleet.ts runFactoryCommand
participant ELM as ensureLocalMount
participant IMP as isMountStatePresent
participant SM as spawnMount
participant WSF as waitForStateFile
participant CMS as checkMountStaleness
FleetCLI->>ELM: workspaceId, cwd
ELM->>IMP: stateFilePath
IMP-->>ELM: present: boolean
alt state file absent
ELM->>SM: resolvedBinary, args
SM-->>ELM: resolve / reject(auth error / exit code)
ELM->>WSF: stateFilePath, 10s deadline
WSF-->>ELM: resolve / timeout error
else state file present
ELM->>CMS: stateFilePath, workspaceId
CMS-->>ELM: stale, reason, pid
alt stale
ELM->>ELM: write warning to process.stderr
end
end
ELM-->>FleetCLI: void
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fbdc2e119
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| throw new Error(NOT_FOUND_ERROR) | ||
| } | ||
|
|
||
| const pearRoot = findPearRoot(__dirname) |
There was a problem hiding this comment.
When RELAYFILE_MOUNT_BIN is not set, this starts discovery from the module directory and only accepts an ancestor containing factory.config.json as the Pear root. In the normal repo/app layout the installed binary is under bin/relayfile-mount, while factory.config.json is a user factory config and is not an ancestor of packages/factory-sdk/src/mount, so pearRoot becomes null and factory start fails with “relayfile-mount binary not found” before it can auto-start the mount. Use a package/repo marker or pass the launcher/repo root instead.
Useful? React with 👍 / 👎.
| while (Date.now() < deadline) { | ||
| try { | ||
| await readFile(stateFilePath, 'utf8') | ||
| return | ||
| } catch { | ||
| // not yet present | ||
| } | ||
| await sleep(200) | ||
| } |
There was a problem hiding this comment.
If relayfile-mount start --background exits 0 but the daemon never creates .integrations/.relay/state.json within this timeout, the loop simply falls through and ensureLocalMount returns success. In that slow-start/wrong-cwd/daemon-crash case factory.start runs with no local mount and no warning, so writeback can be silently lost; throw or at least warn on timeout instead of treating absence as success.
Useful? React with 👍 / 👎.
| const lastReconcileAt = typeof parsed.lastReconcileAt === 'string' | ||
| ? Date.parse(parsed.lastReconcileAt) | ||
| : NaN | ||
| if (!Number.isFinite(lastReconcileAt)) { | ||
| return { stale: true, reason: 'last reconcile timestamp is missing', pid } |
There was a problem hiding this comment.
The existing Relayfile mount state reader in src/main/integration-mounts.ts checks lastSuccessfulReconcileAt from .relay/state.json; a healthy mount state with that field but no lastReconcileAt reaches this branch and is reported stale with “last reconcile timestamp is missing”. That makes every factory start warn that writeback may not propagate even though the mount is fresh, hiding the real stale-mount signal; parse lastSuccessfulReconcileAt here as well.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/factory-sdk/src/mount/relayfile-binary.test.ts (1)
37-48: ⚡ Quick winAdd a resolver test for repo-root
bincandidate onwin32.Current tests don’t cover the platform-specific repo-root candidate path, so the Windows
.exeregression can slip through. A smallvi.spyOn(process, 'platform', 'get')-style platform-branch test (or equivalent abstraction seam) would lock this behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/factory-sdk/src/mount/relayfile-binary.test.ts` around lines 37 - 48, Add a new test case in the describe block for resolveRelayfileMountBinary that covers the platform-specific repo-root candidate path for Windows. Use vi.spyOn(process, 'platform', 'get') to mock the platform as 'win32', and then verify that resolveRelayfileMountBinary() correctly resolves to the repo-root bin candidate with the appropriate .exe extension handling. This test should run alongside the existing RELAYFILE_MOUNT_BIN test to ensure Windows-specific behavior is covered and prevent .exe-related regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/factory-sdk/src/mount/local-mount-preflight.ts`:
- Around line 70-81: The waitForStateFile function has two issues: it silently
falls through without throwing an error when the timeout deadline is reached
(after the while loop ends), allowing ensureLocalMount to continue even when
mount startup failed; and it returns successfully upon reading any file without
validating that the file contents are actually valid/well-formed state data. Fix
this by throwing an error when Date.now() exceeds the deadline, and by
validating the contents of the successfully read file (such as checking if it
can be parsed as valid state data) before returning, so that malformed state
files are rejected.
In `@packages/factory-sdk/src/mount/relayfile-binary.ts`:
- Around line 69-71: The hardcoded binary name `relayfile-mount` on line 70 does
not include the platform-specific executable extension, causing Windows systems
to fail discovering the valid `relayfile-mount.exe` binary in the `bin/`
directory. Replace the hardcoded string with a platform-aware binary name
construction that appends `.exe` on Windows and uses the plain name on other
platforms. This should be applied to the repo-root candidate path being joined
with `pearRoot`, 'bin', and the binary name.
---
Nitpick comments:
In `@packages/factory-sdk/src/mount/relayfile-binary.test.ts`:
- Around line 37-48: Add a new test case in the describe block for
resolveRelayfileMountBinary that covers the platform-specific repo-root
candidate path for Windows. Use vi.spyOn(process, 'platform', 'get') to mock the
platform as 'win32', and then verify that resolveRelayfileMountBinary()
correctly resolves to the repo-root bin candidate with the appropriate .exe
extension handling. This test should run alongside the existing
RELAYFILE_MOUNT_BIN test to ensure Windows-specific behavior is covered and
prevent .exe-related regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a5ccec4e-f50c-4de9-a091-8959a6314f00
📒 Files selected for processing (4)
packages/factory-sdk/src/cli/fleet.tspackages/factory-sdk/src/mount/local-mount-preflight.tspackages/factory-sdk/src/mount/relayfile-binary.test.tspackages/factory-sdk/src/mount/relayfile-binary.ts
|
This bundles the binary? We can't assume local installation or the existence of a binary by default |
|
Addressed in b26f5ae. Short answer on the SDK/binary question:
Also fixed the CI failure by injecting the preflight in daemon-signal tests, fixed Windows |
|
pr-reviewer could not complete review for #366 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #366 in AgentWorkforce/pear. |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. pr-reviewer could not complete review for #366 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #366 in AgentWorkforce/pear. |
2 similar comments
|
pr-reviewer could not complete review for #366 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #366 in AgentWorkforce/pear. |
1 participant
Summary
relayfile-binary.ts: resolves the mount binary (env override → repo-rootbin/relayfile-mount→ devrelayfile/distfallbacks with platform/arch mapping);checkMountStaleness(stateFilePath, workspaceId)checks workspace mismatch, stalelastReconcileAt(>15 min), and dead pidlocal-mount-preflight.ts:ensureLocalMount(workspaceId, startDir)— spawns the mount binary when.integrations/.relay/state.jsonis missing or unparseable; warns to stderr when stale (no auto-restart); auth errors throw actionable messages pointing torelayfile workspace joinfleet.ts: callsensureLocalMountat the top of thefactory starthandler (before signal handlers); injectable viaFleetCliDeps.ensureLocalMountfor test overrideTest plan
npx tsc -p packages/factory-sdk/tsconfig.json --noEmitpassesnpx vitest run packages/factory-sdk/src/mount/relayfile-binary.test.tspassesfactory startwith no state file → mount binary is spawnedfactory startwith stale state file → warning logged to stderr, factory continuesfactory startwith auth error from binary → throws with actionable message🤖 Generated with Claude Code