Repository navigation
fix: tmux parsing/targeting + review feedback - #3
Conversation
Validation: - Verified TMUX/TMUX_PANE values inside an isolated tmux server (custom socket) - Verified kill-window syntax and new-session -t semantics via tmux manpage Changes: - Correct TMUX env parsing (socket_path + server_pid) and prefer TMUX_PANE for pane id - Avoid invalid -t injection for commands that already specify a target; do not treat new-session as targetable - Fix kill-window invocation to use -t session:window - Align Machine.metadata typing with runtime (nullable) - Make socket send behavior consistent when disconnected; tighten misc scripts/tests
|
Warning Rate limit exceeded@leeroybrun has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request refactors environment detection and signal handling across development utilities, test infrastructure, and core runner scripts. Key changes include streamlined environment variable initialization, refined platform detection logic, enhanced error handling for incomplete invocations, and significant restructuring of tmux environment parsing and command execution patterns. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. 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 |
Summary of ChangesHello @leeroybrun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the core functionality related to Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 number of valuable fixes and improvements. The corrections to tmux environment variable parsing, command targeting, and kill-window invocation are particularly great, making the tmux integration much more robust. The refactoring in apiSession.ts to centralize the socket connection check is a good step towards cleaner code, and the added argument validation in scripts improves their reliability.
I've included a few suggestions in src/api/apiSession.ts to slightly improve efficiency by performing the socket connection check before encrypting message payloads. This avoids unnecessary work when the socket is disconnected.
Overall, this is a solid set of changes that addresses several important issues.
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); | ||
|
|
||
| // Check if socket is connected before sending | ||
| if (!this.socket.connected) { | ||
| logger.debug('[API] Socket not connected, cannot send message. Message will be lost:', { type: body.type }); | ||
| // TODO: Consider implementing message queue or HTTP fallback for reliability | ||
| } | ||
| if (!this.canSend('Codex message', { type: body?.type })) return; |
There was a problem hiding this comment.
For efficiency, it's better to check if the socket is connected before performing the encryption. This avoids unnecessary computation if the message can't be sent anyway.
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); | |
| // Check if socket is connected before sending | |
| if (!this.socket.connected) { | |
| logger.debug('[API] Socket not connected, cannot send message. Message will be lost:', { type: body.type }); | |
| // TODO: Consider implementing message queue or HTTP fallback for reliability | |
| } | |
| if (!this.canSend('Codex message', { type: body?.type })) return; | |
| // Check if socket is connected before sending | |
| if (!this.canSend('Codex message', { type: body?.type })) return; | |
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); |
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); | ||
|
|
||
| // Check if socket is connected before sending | ||
| if (!this.canSend(`${agentType} message`, { agentType, type: body?.type })) return; |
There was a problem hiding this comment.
For efficiency, it's better to check if the socket is connected before performing the encryption. This avoids unnecessary computation if the message can't be sent anyway.
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); | |
| // Check if socket is connected before sending | |
| if (!this.canSend(`${agentType} message`, { agentType, type: body?.type })) return; | |
| // Check if socket is connected before sending | |
| if (!this.canSend(`${agentType} message`, { agentType, type: body?.type })) return; | |
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); |
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); | ||
|
|
||
| // Check if socket is connected before sending | ||
| if (!this.canSend('session event', { eventType: event.type })) return; |
There was a problem hiding this comment.
For efficiency, it's better to check if the socket is connected before performing the encryption. This avoids unnecessary computation if the message can't be sent anyway.
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); | |
| // Check if socket is connected before sending | |
| if (!this.canSend('session event', { eventType: event.type })) return; | |
| // Check if socket is connected before sending | |
| if (!this.canSend('session event', { eventType: event.type })) return; | |
| const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); |
![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
🤖 Fix all issues with AI agents
In @src/claude/runClaude.ts:
- Around line 132-150: The abortOnSignal handler and abortController are
declared inside the try block so abortOnSignal is out of scope in the finally
block; move the declarations for abortController and abortOnSignal to the
surrounding scope (declare abortController and define abortOnSignal before the
try) so that the finally block can call process.removeListener('SIGINT',
abortOnSignal) and process.removeListener('SIGTERM', abortOnSignal) and still
pass abortController.signal into claudeLocal; keep the same calls to
process.once and the existing claudeLocal invocation.
🧹 Nitpick comments (5)
scripts/claude_version_utils.test.ts (1)
35-38: Update test name to reflect the new behavior.The expectation change is correct, but the test name "should detect npm with different scoped packages" is now misleading. The test verifies that non-Claude-Code npm packages are not classified as 'npm' source.
✏️ Suggested rename
- it('should detect npm with different scoped packages', () => { + it('should NOT detect npm for non-Claude-Code scoped packages', () => { const result = detectSourceFromPath('C:/Users/test/AppData/Roaming/npm/node_modules/@babel/core/cli.js'); expect(result).toBe('PATH'); });src/utils/tmux.ts (1)
95-107: Consider removing unused interface fields.
TmuxSessionInfostill declaresenv_sessionandenv_window(lines 103-104), butgetSessionInfono longer populates them sinceTmuxEnvironmentno longer includes session/window. These fields will always beundefined.♻️ Suggested cleanup
export interface TmuxSessionInfo { target_session: string; session: string; window: string; pane: string; socket_path?: string; tmux_active: boolean; current_session?: string; - env_session?: string; - env_window?: string; env_pane?: string; available_sessions: string[]; }Also applies to: 695-702
src/api/apiSession.ts (3)
246-254: Consider moving the guard before encryption for consistency.The
canSendcheck at Line 249 is placed after encryption at Line 246. This differs fromsendClaudeSessionMessagewhere the guard precedes encryption. Moving the guard before encryption avoids unnecessary crypto work when disconnected.♻️ Suggested reordering
sendCodexMessage(body: any) { + // Check if socket is connected before sending + if (!this.canSend('Codex message', { type: body?.type })) return; + let content = { role: 'agent', content: { type: 'codex', data: body // This wraps the entire Claude message }, meta: { sentFrom: 'cli' } }; const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); - - // Check if socket is connected before sending - if (!this.canSend('Codex message', { type: body?.type })) return; - + this.socket.emit('message', { sid: this.sessionId, message: encrypted }); }
276-286: Same inconsistency: guard placed after encryption.For consistency with
sendClaudeSessionMessage, consider moving thecanSendcheck (Lines 280-281) before the encryption at Line 278.♻️ Suggested reordering
sendAgentMessage(agentType: 'gemini' | 'codex' | 'claude' | 'opencode', body: any) { + // Check if socket is connected before sending + if (!this.canSend(`${agentType} message`, { agentType, type: body?.type })) return; + let content = { role: 'agent', content: { type: agentType, data: body }, meta: { sentFrom: 'cli' } }; logger.debug(`[SOCKET] Sending ${agentType} message:`, { type: body.type, hasMessage: !!body.message }); const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); - - // Check if socket is connected before sending - if (!this.canSend(`${agentType} message`, { agentType, type: body?.type })) return; - + this.socket.emit('message', { sid: this.sessionId, message: encrypted }); }
306-314: Same inconsistency: guard placed after encryption.For consistency, consider moving the
canSendcheck (Lines 308-309) before the encryption at Line 306.♻️ Suggested reordering
sendSessionEvent(event: { type: 'switch', mode: 'local' | 'remote' } | { type: 'message', message: string } | { type: 'permission-mode-changed', mode: 'default' | 'acceptEdits' | 'bypassPermissions' | 'plan' } | { type: 'ready' }, id?: string) { + // Check if socket is connected before sending + if (!this.canSend('session event', { eventType: event.type })) return; + let content = { role: 'agent', content: { id: id ?? randomUUID(), type: 'event', data: event } }; const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); - - // Check if socket is connected before sending - if (!this.canSend('session event', { eventType: event.type })) return; - + this.socket.emit('message', { sid: this.sessionId, message: encrypted }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
bin/happy-dev.mjsdocs/bug-fix-plan-2025-01-15-athundt.mdscripts/claude_version_utils.cjsscripts/claude_version_utils.test.tsscripts/env-wrapper.cjsscripts/ripgrep_launcher.cjsscripts/test-continue-fix.shsrc/api/api.test.tssrc/api/apiSession.tssrc/api/types.tssrc/claude/claudeLocal.tssrc/claude/runClaude.tssrc/utils/tmux.test.tssrc/utils/tmux.ts
💤 Files with no reviewable changes (1)
- docs/bug-fix-plan-2025-01-15-athundt.md
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: Use descriptive variable and function names that clearly convey intent
Write clear, concise comments explaining the 'why' behind complex logic
Keep functions small and focused on a single responsibility
Use consistent code formatting and adhere to the project's style guide
Files:
src/claude/runClaude.tsscripts/claude_version_utils.test.tssrc/api/types.tssrc/utils/tmux.test.tssrc/api/api.test.tssrc/api/apiSession.tssrc/utils/tmux.tssrc/claude/claudeLocal.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: No untyped code - use strict typing in TypeScript with explicit parameter and return types
Use comprehensive JSDoc comments - each file includes header comments explaining responsibilities
ALL imports must be at the top of the file - NEVER import modules mid-code
Use named exports preferred, with occasional default exports for main functions
Avoid creating stupid small functions, getters, or setters - minimize unnecessary function decomposition
Avoid excessive use ofifstatements - especially if you can avoid control flow changes with a better design
Files:
src/claude/runClaude.tsscripts/claude_version_utils.test.tssrc/api/types.tssrc/utils/tmux.test.tssrc/api/api.test.tssrc/api/apiSession.tssrc/utils/tmux.tssrc/claude/claudeLocal.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Use@/alias for src imports, e.g.,import { logger } from '@/ui/logger'
Use graceful error handling with proper error messages, try-catch blocks with specific error logging, and abort controllers for cancellable operations
All debugging must be through file logs to avoid disturbing Claude sessions - console output only for user-facing messages
Handle large JSON objects with truncation in logs to avoid excessive output
Minimize use of classes - prefer functional approaches in TypeScript
Files:
src/claude/runClaude.tssrc/api/types.tssrc/utils/tmux.test.tssrc/api/api.test.tssrc/api/apiSession.tssrc/utils/tmux.tssrc/claude/claudeLocal.ts
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
.tsfile extensions for TypeScript files
Files:
src/claude/runClaude.tssrc/api/types.tssrc/utils/tmux.test.tssrc/api/api.test.tssrc/api/apiSession.tssrc/utils/tmux.tssrc/claude/claudeLocal.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Write unit tests using Vitest with no mocking - tests should make real API calls
Colocate test files with source files using.test.tsnaming convention
Use descriptive test names and proper async handling in tests
Files:
scripts/claude_version_utils.test.tssrc/utils/tmux.test.tssrc/api/api.test.ts
src/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/api/**/*.{ts,tsx}: Implement end-to-end encryption for all communications using TweetNaCl
Use Socket.IO for real-time messaging in API sessions
Implement optimistic concurrency control for state updates in sessions
Implement session isolation through unique session IDs
Files:
src/api/types.tssrc/api/api.test.tssrc/api/apiSession.ts
src/api/types.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Zod schemas for type-safe API communication in types files
Files:
src/api/types.ts
src/api/apiSession.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Register RPC handlers for remote procedure calls in session clients
Files:
src/api/apiSession.ts
🧠 Learnings (19)
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to src/claude/watcher.{ts,tsx} : Implement file system watcher for Claude session files in interactive mode
Applied to files:
src/claude/runClaude.tsscripts/claude_version_utils.test.tsscripts/claude_version_utils.cjssrc/claude/claudeLocal.tsscripts/test-continue-fix.sh
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to src/**/*.{ts,tsx} : Use graceful error handling with proper error messages, try-catch blocks with specific error logging, and abort controllers for cancellable operations
Applied to files:
src/claude/runClaude.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : On daemon shutdown, resolve the shutdown promise when OS signals (SIGINT/SIGTERM) are received, HTTP /stop endpoint is called, RPC requestShutdown is invoked, or uncaught exceptions occur
Applied to files:
src/claude/runClaude.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : In stopDaemon() from controlClient.ts, read daemon.state.json and attempt graceful shutdown via HTTP POST to /stop endpoint, falling back to process.kill(pid, 'SIGKILL') if HTTP fails
Applied to files:
src/claude/runClaude.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : Ensure caffeinate process is started by the daemon (not individual sessions) and tracked in daemon.state.json to prevent it from becoming a runaway process
Applied to files:
src/claude/runClaude.ts
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to **/*.test.{ts,tsx} : Use descriptive test names and proper async handling in tests
Applied to files:
scripts/claude_version_utils.test.tssrc/utils/tmux.test.tsscripts/test-continue-fix.sh
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : Implement machine metadata structure containing host, platform, happyCliVersion, homeDir, and happyHomeDir as static machine information that rarely changes
Applied to files:
src/api/types.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : Emit 'machine-update-metadata' WebSocket events when machine metadata changes (rare updates like version changes), including expectedVersion for version mismatch handling
Applied to files:
src/api/types.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : On daemon startup, POST to /v1/machines REST endpoint with encrypted metadata and daemonState, sending machineId, and receive metadataVersion and daemonStateVersion from server response
Applied to files:
src/api/types.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : Encrypt all daemon state and metadata separately with TweetNaCl before transmitting via WebSocket; use base64 encoding for encrypted payloads
Applied to files:
src/api/types.tssrc/api/apiSession.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : Implement daemon state structure containing status ('running' | 'shutting-down' | 'offline'), pid, httpPort, startedAt, shutdownRequestedAt, and shutdownSource as dynamic daemon runtime state
Applied to files:
src/api/types.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/src/index.ts : In CLI startup phase, check if machine ID exists in settings; if not, create ID locally only without creating machine on server (daemon's responsibility on startup)
Applied to files:
src/api/types.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : Emit 'machine-update-state' WebSocket events whenever daemon status changes, including expectedVersion for optimistic locking and handle version-mismatch responses by syncing to current server version
Applied to files:
src/api/types.tssrc/api/apiSession.ts
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to **/*.test.{ts,tsx} : Write unit tests using Vitest with no mocking - tests should make real API calls
Applied to files:
src/api/api.test.ts
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to src/api/**/*.{ts,tsx} : Use Socket.IO for real-time messaging in API sessions
Applied to files:
src/api/apiSession.ts
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to src/api/**/*.{ts,tsx} : Implement session isolation through unique session IDs
Applied to files:
src/api/apiSession.ts
📚 Learning: 2026-01-12T11:37:46.296Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:37:46.296Z
Learning: Applies to src/api/**/*.{ts,tsx} : Implement optimistic concurrency control for state updates in sessions
Applied to files:
src/api/apiSession.ts
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/doctor/**/*.{ts,tsx} : In 'happy doctor' command, use 'ps aux | grep' to find all Happy processes and categorize by command args: daemon, daemon-spawned, user-session, doctor; match production patterns 'happy.mjs', 'happy-coder', 'dist/index.mjs' and development pattern 'tsx.*src/index.ts'
Applied to files:
bin/happy-dev.mjs
📚 Learning: 2026-01-12T11:38:16.567Z
Learnt from: CR
Repo: leeroybrun/happy-cli PR: 0
File: src/daemon/CLAUDE.md:0-0
Timestamp: 2026-01-12T11:38:16.567Z
Learning: Applies to src/daemon/**/daemon/**/*.{ts,tsx} : When daemon receives RPC 'spawn-happy-session' from backend, create directory if needed, spawn detached Happy process with flags '--happy-starting-mode remote --started-by daemon', add to pidToTrackedSession map, and set up 10-second awaiter for session webhook
Applied to files:
bin/happy-dev.mjs
🧬 Code graph analysis (3)
src/claude/runClaude.ts (1)
src/claude/claudeLocal.ts (1)
claudeLocal(17-328)
scripts/env-wrapper.cjs (1)
scripts/claude_version_utils.cjs (1)
command(60-60)
src/api/apiSession.ts (2)
src/ui/logger.ts (1)
logger(235-235)scripts/session_hook_forwarder.cjs (1)
body(26-26)
🪛 GitHub Actions: Smoke Test
src/claude/runClaude.ts
[error] 149-149: Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'? (during 'yarn build' / tsc --noEmit)
🪛 GitHub Actions: TypeScript typecheck
src/claude/runClaude.ts
[error] 149-149: Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'? (Command: yarn typecheck; tsc --noEmit)
🪛 GitHub Check: smoke-test-linux (20)
src/claude/runClaude.ts
[failure] 150-150:
Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'?
[failure] 149-149:
Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'?
🪛 GitHub Check: smoke-test-windows (20)
src/claude/runClaude.ts
[failure] 150-150:
Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'?
[failure] 149-149:
Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'?
🪛 GitHub Check: typecheck
src/claude/runClaude.ts
[failure] 150-150:
Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'?
[failure] 149-149:
Cannot find name 'abortOnSignal'. Did you mean 'AbortSignal'?
🔇 Additional comments (19)
scripts/claude_version_utils.cjs (1)
134-136: LGTM! Stricter Windows npm detection prevents false positives.The added path segment requirements (
@anthropic-ai,claude-code, and exclusion of.claude-code-) correctly narrow the npm source classification to only match actual Claude Code installations, avoiding false positive detection of other npm packages like@babel/core.scripts/ripgrep_launcher.cjs (1)
41-54: LGTM! Correct stdio configuration for capturing command output.The change to
stdio: ['ignore', 'pipe', 'ignore']properly configuresexecFileSyncto capture stdout while ignoring stdin and stderr. Combined withencoding: 'utf8', this ensures thewhich/whereoutput is returned as a string for parsing.src/claude/claudeLocal.ts (1)
37-37: LGTM!Good consistency fix - the
-rshort flag is now recognized inhasResumeFlag, aligning with theextractFlagcall at line 90 which already handles both--resumeand-r.scripts/test-continue-fix.sh (1)
17-17: LGTM!Good defensive addition. The guard prevents a confusing
ENOENTerror fromreaddirSyncby failing early with a clear, actionable error message when the project directory doesn't exist.Also applies to: 25-28
scripts/env-wrapper.cjs (1)
54-61: LGTM!Good guard to enforce the required command argument. This prevents proceeding with an undefined command, which would otherwise cause confusing failures downstream when spawning the subprocess.
bin/happy-dev.mjs (1)
12-14: LGTM!Moving environment variable initialization to the top-level ensures
HAPPY_HOME_DIRandHAPPY_VARIANTare consistently set before the re-exec vs direct-import branching. This eliminates duplication and guarantees the subprocess inherits the correct environment viaenv: process.envon line 27.src/utils/tmux.test.ts (4)
288-312: LGTM!The test helper correctly manages
TMUX_PANElifecycle: captures original state, conditionally sets or deletes the variable, and properly restores it in the finally block. This ensures tests don't leak environment state.
322-332: LGTM!Expected fields correctly updated to
socket_pathandserver_pidto match the newTmuxEnvironmentinterface. The tests validate the corrected parsing logic that extracts these values from the TMUX environment variable format.Also applies to: 350-361
334-348: LGTM!Good coverage for malformed TMUX values. Returning
nullfor non-numericserver_pidis the correct behavior since the second component must be a valid PID.
412-422: LGTM!This test validates the critical fix:
TMUX_PANEtakes precedence over the third TMUX component when present. The expected pane value'%0'(fromTMUX_PANE) correctly differs from'0'(the TMUX third component).src/utils/tmux.ts (5)
79-86: LGTM!The updated
TmuxEnvironmentinterface correctly reflects the TMUX environment variable format:socket_path,server_pid,pane_id. Usingnumberforserver_pidis appropriate since it's a process ID.
348-350: LGTM!Correct removal of
new-sessionfrom auto-targeting. The comment clarifies thatnew-session -ttargets a session group, not a session/window target—different semantics that would cause incorrect behavior if auto-targeted.
379-397: LGTM!The rewritten parsing logic correctly:
- Requires exactly 3+ comma-separated components
- Validates
server_pidis numeric via regex before parsing- Prefers
TMUX_PANEwhen set, falling back to the third TMUX component- Returns
nullon any validation failure instead of partial/incorrect dataThis aligns with the PR objective of correcting TMUX environment parsing.
444-450: LGTM!Good fix to prevent double
-tinjection. ThehasExplicitTargetcheck ensures auto-targeting is skipped when the command already specifies a target, addressing the reported issue of invalid target injection.
884-885: LGTM!The
killWindowfix correctly delegates toexecuteTmuxCommandwith separatesessionandwindowparameters. This ensures the proper-t session:windowsyntax is built, fixing the kill-window invocation issue mentioned in the PR objectives.src/api/api.test.ts (1)
221-238: LGTM!The spy initialization is now consistent with the pattern used in other tests in this file. The test correctly validates that non-connection errors are re-thrown with proper wrapping and that the offline warning is not shown.
src/api/apiSession.ts (2)
30-36: Good centralization of the socket connectivity guard.The helper provides consistent logging and a clean abstraction for connection checks. Based on learnings, this aligns with the requirement to use Socket.IO for real-time messaging in API sessions.
203-212: LGTM - Guard is correctly placed before encryption.The
canSendcheck at Line 206 is placed after logging but before the encryption operation at Line 208, avoiding unnecessary encryption work when the socket is disconnected.src/api/types.ts (1)
217-225: Type alignment with runtime behavior is correct.Making
metadatanullable aligns withdaemonState(already nullable on Line 223) and reflects actual runtime scenarios where metadata may not yet be populated. Verification confirms consumers inapiMachine.tsproperly handle the null case through handler functions. Zod schemas are implemented as required.
- Fix abortOnSignal scoping in offline mode so finally cleanup compiles - Avoid unnecessary encryption/UUID work when socket is disconnected (Gemini/CodeRabbit feedback on PR #3)
- Remove unused env_session/env_window fields from TmuxSessionInfo - Rename misleading claude_version_utils npm test (CodeRabbit/Gemini nitpicks on PR #3)
Validation:
Verified TMUX/TMUX_PANE values inside an isolated tmux server (custom socket)
Verified kill-window syntax and new-session -t semantics via tmux manpage
Changes:
Correct TMUX env parsing (socket_path + server_pid) and prefer TMUX_PANE for pane id
Avoid invalid -t injection for commands that already specify a target; do not treat new-session as targetable
Fix kill-window invocation to use -t session:window
Align Machine.metadata typing with runtime (nullable)
Make socket send behavior consistent when disconnected; tighten misc scripts/tests
Issues reported by CodeRabbit in #1
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.