fix(desktop): improve terminal startup reliability with agent hook wrappers#1659
Closed
fix(desktop): improve terminal startup reliability with agent hook wrappers#1659
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR refactors agent hooks initialization from synchronous to asynchronous, restructures setup logic to execute directory creation and script generation in parallel phases instead of sequentially, and replaces direct file-writing methods with pure content getter functions. Consumers are updated to handle the new async flow. Changes
Sequence DiagramsequenceDiagram
participant Main as Main Process
participant DaemonMgr as Daemon Manager
participant AgentSetup as ensureAgentHooks
participant FSPhase1 as Phase 1:<br/>Dir Creation
participant FSPhase2 as Phase 2:<br/>Notify + Settings
participant FSPhase3 as Phase 3:<br/>Scripts + Wrappers
Main->>DaemonMgr: doCreateOrAttach()
DaemonMgr->>DaemonMgr: Check if daemon exists
alt No daemon exists
DaemonMgr->>DaemonMgr: waitForAgentHooks()
DaemonMgr->>AgentSetup: ensureAgentHooks()
par Parallel Phase 1
AgentSetup->>FSPhase1: Create BIN_DIR, HOOKS_DIR,<br/>ZSH_DIR, BASH_DIR, etc.
and
AgentSetup->>AgentSetup: cleanupGlobalOpenCodePlugin()
end
par Parallel Phase 2
AgentSetup->>FSPhase2: Queue notify script creation
and
AgentSetup->>FSPhase2: Queue Claude settings
end
par Parallel Phase 3
AgentSetup->>FSPhase3: Create all wrappers<br/>(claude, codex, cursor, etc.)
and
AgentSetup->>FSPhase3: Create hooks<br/>(Cursor, Gemini, Copilot)
and
AgentSetup->>FSPhase3: Create plugins
end
AgentSetup-->>DaemonMgr: Promise resolves
end
DaemonMgr->>DaemonMgr: Create shell env
DaemonMgr->>DaemonMgr: Attach daemon
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/main/terminal-host/session.ts (1)
14-14: Consider using the path aliasmain/lib/agent-setupinstead of a relative path.The daemon-manager.ts imports from
"main/lib/agent-setup"(alias), while this file uses a relative path. If the terminal-host files intentionally avoid aliases (all existing imports here are relative), this is fine — but it's inconsistent with the convention in other main-process files. As per coding guidelines: "Use alias as defined intsconfig.jsonwhen possible."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/terminal-host/session.ts` at line 14, The import in session.ts uses a relative path for getShellArgs (imported as getAgentShellArgs) which is inconsistent with other main-process files; update the import to use the path alias main/lib/agent-setup (matching daemon-manager.ts) so the symbol getShellArgs (getAgentShellArgs) is imported via the alias and conforms to tsconfig.json aliasing conventions.apps/desktop/src/main/lib/agent-setup/shell-wrappers.ts (1)
6-9: Path constants are duplicated between module-levelconsts and exported getter functions.Lines 6–9 define
ZSH_PROFILE,ZSH_RC,ZSH_LOGIN, andBASH_RCFILEas module-level constants, while lines 42–56 define getter functions that recompute the samepath.join(...)expressions. The getters should return the existing constants to avoid drift.♻️ Proposed fix — return existing constants from getters
export function getZshProfilePath(): string { - return path.join(ZSH_DIR, ".zprofile"); + return ZSH_PROFILE; } export function getZshRcPath(): string { - return path.join(ZSH_DIR, ".zshrc"); + return ZSH_RC; } export function getZshLoginPath(): string { - return path.join(ZSH_DIR, ".zlogin"); + return ZSH_LOGIN; } export function getBashRcfilePath(): string { - return path.join(BASH_DIR, "rcfile"); + return BASH_RCFILE; }Also applies to: 42-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/shell-wrappers.ts` around lines 6 - 9, The module defines path constants ZSH_PROFILE, ZSH_RC, ZSH_LOGIN, and BASH_RCFILE but the exported getter functions later recompute the same path.join(...) expressions; update those exported getters to simply return the existing constants (ZSH_PROFILE, ZSH_RC, ZSH_LOGIN, BASH_RCFILE) instead of recalculating to avoid drift and duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/main/lib/agent-setup/shell-wrappers.ts`:
- Around line 6-9: The module defines path constants ZSH_PROFILE, ZSH_RC,
ZSH_LOGIN, and BASH_RCFILE but the exported getter functions later recompute the
same path.join(...) expressions; update those exported getters to simply return
the existing constants (ZSH_PROFILE, ZSH_RC, ZSH_LOGIN, BASH_RCFILE) instead of
recalculating to avoid drift and duplication.
In `@apps/desktop/src/main/terminal-host/session.ts`:
- Line 14: The import in session.ts uses a relative path for getShellArgs
(imported as getAgentShellArgs) which is inconsistent with other main-process
files; update the import to use the path alias main/lib/agent-setup (matching
daemon-manager.ts) so the symbol getShellArgs (getAgentShellArgs) is imported
via the alias and conforms to tsconfig.json aliasing conventions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ensureAgentHooks()warmup.zshrc/.zloginsplit, add wrapper-env fallback behavior, and remove test-only writer exportsValidation
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Performance