Add factory runbook docs, npm scripts, and pear factory passthrough#322
Conversation
The factory-sdk autonomous loop (issue #321) had no operator-facing docs and no convenient entrypoint — the only invocation was the verbose `node packages/factory-sdk/bin/fleet.mjs factory … --config <cfg>`, and the `pear` binary (an Electron launcher) had no `factory` verb. - bin/pear.mjs: intercept `pear factory <action> …` and forward verbatim to the factory-sdk `fleet` CLI in a plain Node process. Deliberately does NOT launch Electron — the daemon/reaper are headless by design. Exit codes and signals propagate. - package.json: add factory / factory:start / factory:run-once / factory:reap / factory:status npm scripts. - packages/factory-sdk/README.md: quick start, command table, the 2-process production model (daemon + same-`--config` reaper coupling), gh-auth precondition, minimal config + annotated fields, and fixture mode. No behavior change: mergePolicy still defaults to `never`; the autonomous merge-flip remains pending per #321. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 46 minutes and 11 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 (1)
📝 WalkthroughWalkthrough
Changesfactory CLI passthrough and documentation
Mount-root invariant incident logging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a headless passthrough for pear factory commands in bin/pear.mjs to execute the factory-sdk CLI directly in a plain Node process, bypassing Electron. It also adds corresponding helper scripts to package.json and comprehensive documentation in packages/factory-sdk/README.md. The reviewer suggested consolidating the child process spawning logic in bin/pear.mjs to eliminate duplicated event handlers for the spawned processes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (args[0] === 'factory') { | ||
| const fleetBin = join(appRoot, 'packages', 'factory-sdk', 'bin', 'fleet.mjs') | ||
| if (!existsSync(fleetBin)) { | ||
| console.error(`factory-sdk CLI not found (missing ${fleetBin}).`) | ||
| process.exit(1) | ||
| } | ||
| const child = spawn(process.execPath, [fleetBin, ...args], { stdio: 'inherit' }) | ||
| child.on('close', (code, signal) => { | ||
| if (signal) { | ||
| const signalNumber = typeof os.constants.signals[signal] === 'number' ? os.constants.signals[signal] : 0 | ||
| process.exit(128 + signalNumber) | ||
| } | ||
| process.exit(code ?? 0) | ||
| }) | ||
| child.on('error', (error) => { | ||
| console.error('Failed to launch factory CLI:', error instanceof Error ? error.message : String(error)) | ||
| process.exit(1) | ||
| }) | ||
| } else { | ||
| if (!existsSync(appMain)) { | ||
| console.error(`Pear is not built yet — run \`npm run build\` first (missing ${appMain}).`) | ||
| process.exit(1) | ||
| } | ||
|
|
||
| // In a Node context, requiring electron yields the path to its executable. | ||
| const electronPath = require('electron') | ||
| // In a Node context, requiring electron yields the path to its executable. | ||
| const electronPath = require('electron') | ||
|
|
||
| const child = spawn(electronPath, [appMain, ...process.argv.slice(2)], { | ||
| stdio: 'inherit' | ||
| }) | ||
| const child = spawn(electronPath, [appMain, ...args], { | ||
| stdio: 'inherit' | ||
| }) | ||
|
|
||
| child.on('close', (code, signal) => { | ||
| // A signal-terminated child reports code === null; surface that as a failure | ||
| // (128 + signal number, per shell convention) so calling scripts can detect it. | ||
| if (signal) { | ||
| const signalNumber = typeof os.constants.signals[signal] === 'number' ? os.constants.signals[signal] : 0 | ||
| process.exit(128 + signalNumber) | ||
| } | ||
| process.exit(code ?? 0) | ||
| }) | ||
| child.on('error', (error) => { | ||
| console.error('Failed to launch Pear:', error instanceof Error ? error.message : String(error)) | ||
| process.exit(1) | ||
| }) | ||
| child.on('close', (code, signal) => { | ||
| // A signal-terminated child reports code === null; surface that as a failure | ||
| // (128 + signal number, per shell convention) so calling scripts can detect it. | ||
| if (signal) { | ||
| const signalNumber = typeof os.constants.signals[signal] === 'number' ? os.constants.signals[signal] : 0 | ||
| process.exit(128 + signalNumber) | ||
| } | ||
| process.exit(code ?? 0) | ||
| }) | ||
| child.on('error', (error) => { | ||
| console.error('Failed to launch Pear:', error instanceof Error ? error.message : String(error)) | ||
| process.exit(1) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The event handlers for the spawned child process (close and error) are duplicated between the factory passthrough branch and the standard Electron launch branch. We can consolidate the spawning logic by determining the target binary and arguments first, and then registering the event handlers once on the spawned child process. This significantly reduces code duplication and improves maintainability.
const isFactory = args[0] === 'factory'
let child
if (isFactory) {
const fleetBin = join(appRoot, 'packages', 'factory-sdk', 'bin', 'fleet.mjs')
if (!existsSync(fleetBin)) {
console.error(`factory-sdk CLI not found (missing ${fleetBin}).`)
process.exit(1)
}
child = spawn(process.execPath, [fleetBin, ...args], { stdio: 'inherit' })
} else {
if (!existsSync(appMain)) {
console.error(`Pear is not built yet — run \`npm run build\` first (missing ${appMain}).`)
process.exit(1)
}
// In a Node context, requiring electron yields the path to its executable.
const electronPath = require('electron')
child = spawn(electronPath, [appMain, ...args], { stdio: 'inherit' })
}
child.on('close', (code, signal) => {
// A signal-terminated child reports code === null; surface that as a failure
// (128 + signal number, per shell convention) so calling scripts can detect it.
if (signal) {
const signalNumber = typeof os.constants.signals[signal] === 'number' ? os.constants.signals[signal] : 0
process.exit(128 + signalNumber)
}
process.exit(code ?? 0)
})
child.on('error', (error) => {
console.error(`Failed to launch ${isFactory ? 'factory CLI' : 'Pear'}:`, error instanceof Error ? error.message : String(error))
process.exit(1)
})![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![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: 2620175371
ℹ️ 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".
| // factory daemon/reaper run in a plain Node process. Everything after `factory` | ||
| // is forwarded verbatim, so `pear factory start --mode live --config <cfg>` | ||
| // behaves identically to `node packages/factory-sdk/bin/fleet.mjs factory …`. | ||
| if (args[0] === 'factory') { |
There was a problem hiding this comment.
This branch only runs when bin/pear.mjs is the entrypoint, but the macOS user-installed pear command is generated in src/main/cli-install.ts as a shell shim that execs app.getPath('exe'), so pear factory ... from the packaged app still launches Electron instead of this Node launcher. In that installed-CLI context the main process only scans argv for open in src/main/cli.ts, so the documented production pear factory start/reap-orphans commands won't reach fleet; the installer needs to invoke this passthrough or Electron needs equivalent factory handling before advertising pear factory.
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
🤖 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 `@package.json`:
- Line 27: The npm script `factory:status` in package.json invokes `factory
loop-status`, but the README documents `status` and `loop-status` as distinct
commands, creating confusion between the script name and its actual behavior.
Either rename the `factory:status` script to `factory:loop-status` to accurately
reflect what it executes, or change the script to invoke `factory status`
instead to align with the documented command semantics. Choose whichever option
matches your intended command architecture.
In `@packages/factory-sdk/README.md`:
- Line 75: The documentation line at packages/factory-sdk/README.md line 75 uses
`node bin/fleet.mjs` without specifying the working directory context, while
earlier examples in the README use the full path `node
packages/factory-sdk/bin/fleet.mjs`. To prevent copy/paste errors and confusion,
either explicitly clarify the working directory requirement (e.g., "from the
factory-sdk package directory") for the relative path example, or normalize all
examples throughout the README to use a consistent path style (either all
relative or all absolute paths) with clear context about where commands should
be executed.
🪄 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: fe103d5a-c5f5-4013-b17d-ab93c446fda1
📒 Files selected for processing (3)
bin/pear.mjspackage.jsonpackages/factory-sdk/README.md
| "factory:start": "node packages/factory-sdk/bin/fleet.mjs factory start --mode live", | ||
| "factory:run-once": "node packages/factory-sdk/bin/fleet.mjs factory run-once", | ||
| "factory:reap": "node packages/factory-sdk/bin/fleet.mjs factory reap-orphans", | ||
| "factory:status": "node packages/factory-sdk/bin/fleet.mjs factory loop-status", |
There was a problem hiding this comment.
Align factory:status with documented command semantics.
factory:status currently invokes factory loop-status, while the README defines status and loop-status as different commands. Either rename this script to factory:loop-status or point it to factory status to avoid operator confusion.
🤖 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 `@package.json` at line 27, The npm script `factory:status` in package.json
invokes `factory loop-status`, but the README documents `status` and
`loop-status` as distinct commands, creating confusion between the script name
and its actual behavior. Either rename the `factory:status` script to
`factory:loop-status` to accurately reflect what it executes, or change the
script to invoke `factory status` instead to align with the documented command
semantics. Choose whichever option matches your intended command architecture.
…lity Lead with the no-install forms (`npm run factory:*` / `node bin/fleet.mjs`), then document `npm link` as the standard way operators get the `pear` command — it is the normal local-Node-CLI install, not a workaround. Clarify that the packaged .app's "Install 'pear' command in PATH" installs the GUI launcher for `pear open` only; the factory-sdk is not bundled into the app, so `pear factory …` is operator-from-repo only. Avoids overstating that `pear factory` works out of the box. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
khaliqgant
force-pushed
the
docs/factory-runbook-and-pear-passthrough
branch
from
a0d2c0a to
824302a
Compare
khaliqgant
added
the
no-agent-relay-review
|
Implemented the PR-scope fix in bin/pear.mjs: the factory and Electron branches now choose the child process target first, then share the Addressed comments
Advisory Notes
Verification run: I’m not printing |
The config section only had a 4-line minimal example + a field list. Add: - a complete annotated config (subscription, repo routing, safety, models, loop) - where each value comes from (workspaceId, clonePaths, stateIds + the AR-team default UUIDs in src/constants/linear.ts) - the repo-routing precedence (byLabel → byProject → keywordRules → default) - the safety gate explained — title `[factory-e2e]` prefix AND team key AR — so it's clear why run-once can pull issues yet dispatch none Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
khaliqgant
force-pushed
the
docs/factory-runbook-and-pear-passthrough
branch
from
4ac8b15 to
2413885
Compare
|
Implemented fixes for the PR review findings. Changed:
Addressed comments
Advisory NotesNone. Local validation run:
I did not run the macOS |
1 participant
Why
The factory-sdk autonomous loop (#321) was built and validated, but had no operator-facing docs and no convenient entrypoint. The only way to run it was the verbose
node packages/factory-sdk/bin/fleet.mjs factory … --config <cfg>, andpear factory …didn't work becausepearis the Electron app launcher — a different binary from the factory'sfleetCLI.What
pear factory <action> …passthrough (bin/pear.mjs): intercepts thefactoryverb and forwards verbatim to the factory-sdkfleetCLI in a plain Node process. Deliberately does not launch Electron — the daemon/reaper are headless by design. Exit codes and signals propagate.package.json):factory,factory:start,factory:run-once,factory:reap,factory:status.packages/factory-sdk/README.md): quick start, full command table, the 2-process production model (daemon + same---configreaper coupling), thegh-auth precondition, a minimal config with annotated fields, and fixture mode.All three invocations are now equivalent:
```bash
pear factory start --mode live --config ./factory.config.json
npm run factory:start -- --config ./factory.config.json
node packages/factory-sdk/bin/fleet.mjs factory start --mode live --config ./factory.config.json
```
Safety / scope
No behavior change.
mergePolicystill defaults tonever; the autonomous merge-flip remains pending per #321. This is purely ergonomics + docs.Verification
node --check bin/pear.mjspasses.pear factory run-oncewith no--config→ fails fast with the expected config error (proves the passthrough reachesfleet'smain).pear factory run-once --config <fixture> --dry-run→ runs a full offline cycle and prints an IterationReport.npm run factory:status -- --config <fixture>→ returns heartbeat liveness JSON.🤖 Generated with Claude Code