fix: remove obsolete sidecar staging from worktree bootstrap#2561
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughDocumentation across architecture guides, setup instructions, and bootstrap scripts is revised to reflect the transition from an external ChangesCore Server Model Documentation Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@gitbooks/developing/architecture/tauri-shell.md`:
- Line 13: Update the broken link fragment: replace the anchor "`#commands`" used
in the sentence "Expose a small, explicit set of Tauri commands (see
[Commands](`#commands`))" with the actual heading ID
"`#tauri-ipc-commands-app-src-tauri`" so the link points to the existing
"Commands" section (i.e., change the link target from "`#commands`" to
"`#tauri-ipc-commands-app-src-tauri`").
🪄 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
Run ID: 01d3f4b3-fbcb-4bbe-8446-265de4ebf639
📒 Files selected for processing (5)
gitbooks/developing/architecture/tauri-shell.mdgitbooks/developing/architecture/tauri-shell.zh-CN.mdgitbooks/developing/getting-set-up.mdgitbooks/developing/getting-set-up.zh-CN.mdscripts/worktree-bootstrap.sh
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gitbooks/developing/architecture/tauri-shell.md (1)
189-191:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate CoreProcessHandle description to reflect embedded in-process model.
This section still describes the old sidecar model ("Resolves the
openhuman-coreexecutable (staged underbinaries/...)"), which contradicts the embedded in-process architecture documented elsewhere (lines 14, 20, 67). Line 20 explicitly states "local builds no longer need a stagedopenhuman-core-*sidecar underapp/src-tauri/binaries/".📝 Suggested rewrite to align with embedded model
### `CoreProcessHandle` (`core_process.rs`) -- Resolves the **`openhuman-core`** executable (staged under `binaries/` or `PATH` / dev layout). -- Starts or attaches to the core process and exposes its RPC URL (`OPENHUMAN_CORE_RPC_URL`). +- Manages the embedded core server task lifecycle within the Tauri process. +- Exposes the core's RPC URL (`OPENHUMAN_CORE_RPC_URL`) and per-process bearer token. - Used during app setup in `lib.rs` (`app.manage(core_handle)`).🤖 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 `@gitbooks/developing/architecture/tauri-shell.md` around lines 189 - 191, Update the CoreProcessHandle description to reflect the embedded in-process architecture rather than a sidecar executable: replace the text that says it "resolves the openhuman-core executable (staged under binaries/ or PATH / dev layout)" and "starts or attaches to the core process" with wording that CoreProcessHandle initializes and manages an embedded in-process OpenHuman core instance, exposes its RPC endpoint (OPENHUMAN_CORE_RPC_URL) to the app, and is registered during app setup (app.manage(core_handle)); ensure the description mentions the embedded model and remove any references to staging a separate openhuman-core-* sidecar.
🤖 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.
Outside diff comments:
In `@gitbooks/developing/architecture/tauri-shell.md`:
- Around line 189-191: Update the CoreProcessHandle description to reflect the
embedded in-process architecture rather than a sidecar executable: replace the
text that says it "resolves the openhuman-core executable (staged under
binaries/ or PATH / dev layout)" and "starts or attaches to the core process"
with wording that CoreProcessHandle initializes and manages an embedded
in-process OpenHuman core instance, exposes its RPC endpoint
(OPENHUMAN_CORE_RPC_URL) to the app, and is registered during app setup
(app.manage(core_handle)); ensure the description mentions the embedded model
and remove any references to staging a separate openhuman-core-* sidecar.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4f37974-3097-4917-971a-cc34322a52cd
📒 Files selected for processing (1)
gitbooks/developing/architecture/tauri-shell.md
|
very valid! great PR |
2 participants
Summary
scripts/worktree-bootstrap.sh.openhuman-core-aarch64-apple-darwinpath during fresh worktree bootstrap.core:stagehook.Problem
scripts/worktree-bootstrap.shstill assumed the old sidecar model and checked for a staged Apple Silicon core binary before runningcargo buildandpnpm core:stage. The current app links the core in-process andcore:stageis a no-op, so the bootstrap step is stale and misleading, especially for non-arm64 macOS contributors.Solution
Remove the sidecar build/staging block from the worktree bootstrap script and align the English/zh-CN setup and Tauri architecture docs with the in-process core lifecycle.
Submission Checklist
## Related: N/A - no feature matrix IDs affected.Closes #NNNin the## Relatedsection: N/A - this avoids one stale hardcoded architecture path but does not claim to fully resolve the broader Intel CEF issue.Impact
Fresh worktree bootstrap no longer performs an unnecessary core build or checks an Apple-Silicon-specific sidecar path. This should make local setup less confusing and avoids reinforcing stale sidecar guidance.
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/worktree-bootstrap-sidecar-staging2c0c0699f33d222c68f5fc2a6cc5f7a8f9fd2914Validation Run
pnpm --filter openhuman-app format:check: not run; fresh clone has nonode_modulesand this PR only changes shell/docs.pnpm typecheck: not run; no TypeScript source changed.bash -n scripts/worktree-bootstrap.shgit diff --checkValidation Blocked
command:pnpm --filter openhuman-app format:checkerror:dependencies are not installed in this fresh clone (node_modulesabsent).impact:formatting/typecheck gates should be unaffected by shell/docs-only changes; shell syntax and whitespace checks passed locally.Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit