Repository navigation
fix(daemon): reattach sessions after daemon restart - #9
leeroybrun wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughDaemon now persists session markers to disk, reattaches eligible running Happy processes on startup, adds a session marker registry, changes Changes
Sequence Diagram(s)sequenceDiagram
participant Daemon as Daemon Process
participant Registry as SessionRegistry
participant Doctor as Doctor Module
participant Run as Run Module
Daemon->>Registry: listSessionMarkers()
Registry-->>Daemon: markers (filtered by happyHomeDir)
loop per marker
Daemon->>Doctor: findHappyProcessByPid(pid)
Doctor-->>Daemon: process info or null
alt process valid and type allowed
Daemon->>Run: reattach session (mark reattachedFromDiskMarker)
else invalid or stale
Daemon->>Registry: removeSessionMarker(pid)
end
end
sequenceDiagram
participant Client as Client
participant API as API Handler
participant Run as Run Module
participant Doctor as Doctor Module
participant Registry as SessionRegistry
participant OS as OS
Client->>API: stopSession(sessionId)
API->>Run: await stopSession(sessionId)
alt session reattached
Run->>Doctor: findHappyProcessByPid(pid)
Doctor-->>Run: process info
alt type allowed
Run->>OS: send SIGTERM to pid
OS-->>Run: terminated
Run->>Registry: removeSessionMarker(pid)
else type not allowed
Run-->>API: return false
end
else normal session
Run->>OS: send SIGTERM
OS-->>Run: terminated
Run->>Registry: removeSessionMarker(pid)
end
Run-->>API: return true/false
API-->>Client: Promise<boolean>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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
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 significantly enhances the daemon's resilience and session management capabilities. It introduces a mechanism to persist information about active sessions to disk, allowing the daemon to automatically reattach to these sessions upon restart. This change improves the user experience by maintaining session continuity across daemon lifecycle events. Furthermore, it incorporates several safety measures, such as verifying process types and ignoring cross-daemon reports, to ensure that session reattachment and termination are performed reliably and securely, preventing unintended side effects like killing unrelated processes due to PID reuse. 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 robust mechanism for reattaching to running sessions after a daemon restart, significantly improving its resilience. The core of this change is a new session registry that persists session markers to disk using atomic file operations. The daemon now reads these markers on startup to safely re-attach to valid sessions, with excellent safety checks to prevent killing unrelated processes due to PID reuse. The persistence logic for the daemon's own state has also been hardened with atomic writes and schema validation. Overall, this is a high-quality contribution that substantially improves the daemon's reliability. I have a couple of suggestions to further enhance robustness and code consistency.
| async function writeJsonAtomic(filePath: string, value: unknown): Promise<void> { | ||
| const tmpPath = `${filePath}.tmp`; | ||
| await writeFile(tmpPath, JSON.stringify(value, null, 2), 'utf-8'); | ||
| await rename(tmpPath, filePath); | ||
| } |
There was a problem hiding this comment.
The current implementation of writeJsonAtomic is not fully robust. If the rename operation fails, it can leave behind a temporary .tmp file. This could cause issues in subsequent operations. It's good practice to clean up the temporary file in a catch block to ensure atomicity and a clean state, similar to the pattern used in writeDaemonState in persistence.ts.
async function writeJsonAtomic(filePath: string, value: unknown): Promise<void> {
const tmpPath = `${filePath}.tmp`;
try {
await writeFile(tmpPath, JSON.stringify(value, null, 2), 'utf-8');
await rename(tmpPath, filePath);
} catch (e) {
// Best-effort cleanup to avoid leaving behind an orphaned temporary file on failure.
try {
await unlink(tmpPath);
} catch {
// Ignore cleanup failure
}
throw e;
}
}
|
@coderabbitai @codex please review |
|
To use Codex here, create an environment for this repo. |
|
Tip For best results, initiate chat on the files or code changes. @leeroybrun, hello! I see you've mentioned me, but I don't see a specific request or question. How can I assist you with this PR? I'm here to help with:
Just let me know what you'd like me to do! 😊 🧠 Learnings used |
94de95f to
e4dba9c
Compare
|
@codex @coderabbitai please review |
|
@leeroybrun, I'll review the changes in this PR that implement session reattachment after daemon restart. This includes the new session registry module, async stopSession changes, and the safety mechanisms for handling PID reuse. 🧠 Learnings used✅ Actions performedReview triggered.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll review the changes in this PR. Let me examine the key files to provide a thorough analysis. 🏁 Script executed: Length of output: 3898 🏁 Script executed: Length of output: 39757 🏁 Script executed: Length of output: 25132 🏁 Script executed: Length of output: 779 🏁 Script executed: Length of output: 4564 @leeroybrun, I'll provide a comprehensive review of this PR implementing session reattachment after daemon restart. Overall AssessmentThis is a well-designed implementation with strong safety mechanisms for handling the inherently difficult PID reuse problem. The code demonstrates good defensive programming practices with atomic writes, validation, and multiple layers of safety checks. Key Strengths1. Atomic OperationsBoth // Good: atomic write with cleanup on failure
await writeFile(tmpPath, JSON.stringify(value, null, 2), 'utf-8');
await rename(tmpPath, filePath);2. PID Reuse SafetyThe multi-layered approach is excellent:
3. Stack IsolationFiltering by Issues & Recommendations🔴 Critical: Marker Accumulation on CrashIf the daemon crashes or is forcibly killed (SIGKILL), markers won't be cleaned up and will accumulate over time: // In run.ts onChildExited
const onChildExited = (pid: number) => {
logger.debug(`[DAEMON RUN] Removing exited process PID ${pid} from tracking`);
pidToTrackedSession.delete(pid);
void removeSessionMarker(pid); // ✓ Cleanup happens here
};Problem: If daemon crashes, Recommendation: Add marker age-based cleanup in the reattachment logic: // During reattachment in run.ts
const MARKER_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 7 days
for (const marker of markers) {
const age = Date.now() - marker.createdAt;
if (age > MARKER_MAX_AGE_MS) {
logger.debug(`[DAEMON RUN] Removing stale marker for PID ${marker.pid} (${Math.floor(age / 86400000)} days old)`);
await removeSessionMarker(marker.pid);
continue;
}
// ... existing validation
}🟡 Medium: TOCTOU in stopSessionThere's a theoretical race between checking and killing: // Line ~520 in run.ts
if (session.reattachedFromDiskMarker) {
const proc = await findHappyProcessByPid(pid); // Check
if (!proc || !ALLOWED_HAPPY_SESSION_PROCESS_TYPES.has(proc.type)) {
// ...refuse to kill
}
}
// ... [PID could die and be reused here]
process.kill(pid, 'SIGTERM'); // KillReality Check: This is extremely unlikely in practice (requires PID reuse in <1ms) and the existing check is reasonable. Optional Enhancement: Add a post-kill verification: try {
process.kill(pid, 'SIGTERM');
// Verify we killed the right thing
await new Promise(r => setTimeout(r, 50));
const postKillProc = await findHappyProcessByPid(pid);
if (postKillProc && postKillProc.type !== proc.type) {
logger.error(`[DAEMON RUN] WARNING: Killed PID ${pid} but process type changed! May have hit PID reuse.`);
}
} catch (error) { /* ... */ }🟡 Medium: Process Type Detection HeuristicsThe process type classification in // doctor.ts lines ~20-40
const isHappy = name.includes('happy') ||
name === 'node' && (cmd.includes('happy-cli') || cmd.includes('dist/index.mjs')) ||
// ...Risk: If a user runs Happy from an unusual path or with custom flags, it might not be detected correctly. Recommendation: Consider adding a more robust marker like an environment variable set by all Happy processes: // In spawnHappyCLI and daemon startup
env: {
...process.env,
HAPPY_CLI_PROCESS_MARKER: 'true',
HAPPY_CLI_VERSION: packageJson.version,
...extraEnv
}Then check for this in 🟢 Minor: Webhook Handling for Reattached SessionsLines 245-250 in } else if (existingSession?.reattachedFromDiskMarker) {
// Reattached sessions remain kill-protected (PID reuse safety), but we still keep metadata up to date.
existingSession.startedBy = sessionMetadata.startedBy ?? existingSession.startedBy;
existingSession.happySessionId = sessionId;
existingSession.happySessionMetadataFromLocalWebhook = sessionMetadata;
}Question: Should reattached sessions be downgraded from kill-protected status once they report in via webhook? Since we now have confirmation they're real Happy sessions? Consideration: Keeping them kill-protected is more conservative and safer. Current approach is fine. 🟢 Minor: Error Handling in writeSessionMarkerLines 63-69 in void writeSessionMarker({
pid,
happySessionId: sessionId,
startedBy: sessionMetadata.startedBy ?? 'terminal',
cwd: sessionMetadata.path,
metadata: sessionMetadata,
}).catch((e) => {
logger.debug('[DAEMON RUN] Failed to write session marker', e);
});This is fine, but consider whether marker write failures should be more visible (warn vs debug): }).catch((e) => {
logger.warn('[DAEMON RUN] Failed to persist session marker - session won\'t survive daemon restart', e);
});🟢 Minor: Schema Version in Markers
const DaemonSessionMarkerSchema = z.object({
version: z.literal('1.0.0').default('1.0.0'),
pid: z.number().int().positive(),
// ... rest of schema
});This matches the pattern already established in Code Quality✅ Excellent:
✅ Good:
Testing RecommendationsConsider testing these scenarios:
SummaryThis is a solid implementation ready for merge with minor improvements suggested above. The PID reuse safety mechanisms are well thought out, and the atomic operations ensure data integrity. The most actionable item is adding age-based marker cleanup to prevent accumulation after crashes. Great work on tackling this challenging problem! 🎉 🧠 Learnings used |
![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: e4dba9cb55
ℹ️ 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".
| async function writeJsonAtomic(filePath: string, value: unknown): Promise<void> { | ||
| const tmpPath = `${filePath}.tmp`; | ||
| try { | ||
| await writeFile(tmpPath, JSON.stringify(value, null, 2), 'utf-8'); | ||
| await rename(tmpPath, filePath); | ||
| } catch (e) { |
There was a problem hiding this comment.
On Windows, rename does not overwrite an existing destination file, so writeJsonAtomic will throw whenever a marker already exists (which is the normal update path for active sessions). Because writeSessionMarker is called on every webhook and the error is swallowed at the call site, this results in stale pid-*.json markers that never get refreshed. That breaks the new reattach flow on daemon restart (it will read outdated session IDs/metadata), and in the worst case it can reattach to the wrong session if a PID is reused. Consider unlinking the target before renaming on Windows or using a replace-capable write strategy.
Useful? React with 👍 / 👎.
| const tmpPath = `${configuration.daemonStateFile}.tmp`; | ||
| try { | ||
| writeFileSync(tmpPath, JSON.stringify(state, null, 2), 'utf-8'); | ||
| renameSync(tmpPath, configuration.daemonStateFile); |
There was a problem hiding this comment.
The atomic write now uses renameSync(tmp, daemonStateFile), but on Windows renameSync throws if the destination exists. That means a stale daemon.state.json (e.g., after a crash) will cause the next daemon start to throw on the initial writeDaemonState call, and periodic heartbeats will always fail to update the file. This makes daemon startup brittle on Windows and can leave control commands reading stale state. Consider deleting the target first (or using a Windows-safe atomic replace) before renaming.
Useful? React with 👍 / 👎.
e4dba9c to
de4fc06
Compare
![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/daemon/sessionRegistry.ts:
- Line 14: The Zod schema for startedBy (in TrackedSession / sessionRegistry.ts)
restricts values to z.enum(['daemon','terminal']) but other code (e.g., run.ts
creating TrackedSession and writeSessionMarker) uses additional values like
'happy directly - likely by user from terminal' and 'reattached', causing
validation failures; update the schema by either replacing startedBy:
z.enum([...]).optional() with startedBy: z.string().optional() to accept
arbitrary strings, or add the extra literal values used in run.ts to the z.enum
so the schema matches all actual startedBy values (then run tests / try creating
a TrackedSession to verify writeSessionMarker no longer fails).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/api/apiMachine.tssrc/daemon/controlServer.tssrc/daemon/doctor.tssrc/daemon/run.tssrc/daemon/sessionRegistry.tssrc/daemon/types.tssrc/persistence.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/daemon/types.ts
- src/api/apiMachine.ts
- src/daemon/controlServer.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/daemon/doctor.tssrc/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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/daemon/doctor.tssrc/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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/daemon/doctor.tssrc/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.ts
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
.tsfile extensions for TypeScript files
Files:
src/daemon/doctor.tssrc/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.ts
src/daemon/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/daemon/CLAUDE.md)
When daemon receives 'daemon start' command in src/index.ts, spawn a detached process via spawnHappyCLI(['daemon', 'start-sync'], { detached: true }) that calls startDaemon() from src/daemon/run.ts
Files:
src/daemon/doctor.tssrc/daemon/run.tssrc/daemon/sessionRegistry.ts
src/{auth,persistence}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Store private keys in
~/.handy/access.keywith restricted permissions
Files:
src/persistence.ts
🧠 Learnings (31)
📓 Common learnings
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} : Track spawned Happy processes by storing their PID and happySessionId in daemon.state.json for cleanup purposes; include process information for both daemon-spawned and user-spawned sessions
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
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 stopSession() to find session by happySessionId, send SIGTERM to process, and remove from tracking map on exit event
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 a heartbeat loop in the daemon that runs every 60 seconds (or via HAPPY_DAEMON_HEARTBEAT_INTERVAL) to check for version updates and prune dead sessions
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} : Register RPC handlers for 'spawn-happy-session', 'stop-session', and 'requestShutdown' on the daemon's HTTP server and WebSocket connection
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 version mismatch is detected during heartbeat, spawn a new daemon via spawnHappyCLI(['daemon', 'start']) and wait to be killed by the new daemon instance
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 HTTP endpoint /spawn-session that creates new sessions (used by integration tests) via daemon HTTP server
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
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 HTTP endpoint /stop-session that terminates specific sessions via daemon HTTP server
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/**/*.{ts,tsx} : When daemon receives 'daemon start' command in src/index.ts, spawn a detached process via spawnHappyCLI(['daemon', 'start-sync'], { detached: true }) that calls startDaemon() from src/daemon/run.ts
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} : Write daemon state including PID, HTTP port, version, and start time to daemon.state.json upon successful startup; maintain 'state' and 'stateReason' fields to explain daemon status and preserve file on exit/stop instead of hard deleting
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
📚 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:
src/daemon/doctor.tssrc/daemon/run.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} : Implement 'happy doctor clean' command that calls findRunawayHappyProcesses() to filter likely orphans, then killRunawayHappyProcesses() to send SIGTERM, wait 1 second, and send SIGKILL if still alive
Applied to files:
src/daemon/doctor.tssrc/daemon/run.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} : Track spawned Happy processes by storing their PID and happySessionId in daemon.state.json for cleanup purposes; include process information for both daemon-spawned and user-spawned sessions
Applied to files:
src/daemon/doctor.tssrc/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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} : 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:
src/daemon/doctor.tssrc/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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 stopSession() to find session by happySessionId, send SIGTERM to process, and remove from tracking map on exit event
Applied to files:
src/daemon/doctor.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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} : When version mismatch is detected during heartbeat, spawn a new daemon via spawnHappyCLI(['daemon', 'start']) and wait to be killed by the new daemon instance
Applied to files:
src/daemon/doctor.tssrc/persistence.tssrc/daemon/run.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 a heartbeat loop in the daemon that runs every 60 seconds (or via HAPPY_DAEMON_HEARTBEAT_INTERVAL) to check for version updates and prune dead sessions
Applied to files:
src/daemon/doctor.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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/**/*.{ts,tsx} : When daemon receives 'daemon start' command in src/index.ts, spawn a detached process via spawnHappyCLI(['daemon', 'start-sync'], { detached: true }) that calls startDaemon() from src/daemon/run.ts
Applied to files:
src/daemon/doctor.tssrc/daemon/run.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/daemon/doctor.tssrc/persistence.tssrc/daemon/run.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 machine metadata structure containing host, platform, happyCliVersion, homeDir, and happyHomeDir as static machine information that rarely changes
Applied to files:
src/daemon/doctor.tssrc/persistence.tssrc/daemon/run.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} : Write daemon state including PID, HTTP port, version, and start time to daemon.state.json upon successful startup; maintain 'state' and 'stateReason' fields to explain daemon status and preserve file on exit/stop instead of hard deleting
Applied to files:
src/persistence.tssrc/daemon/run.tssrc/daemon/sessionRegistry.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/persistence.tssrc/daemon/sessionRegistry.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} : If daemon.state.json file is found and corrupted, attempt to upgrade it to the latest version or remove it if write access is available
Applied to files:
src/persistence.tssrc/daemon/sessionRegistry.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/persistence.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} : If daemon.state.json file is not found, assume daemon was never started, was cleaned out by user, or was cleaned by doctor command
Applied to files:
src/persistence.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} : During daemon startup, perform version check by reading daemon.state.json and comparing startedWithCliVersion with configuration.currentCliVersion; if version mismatch detected, call stopDaemon() to kill old daemon before proceeding
Applied to files:
src/persistence.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 cleanupAndShutdown() to clear heartbeat interval, update daemon state to 'shutting-down' on backend, disconnect WebSocket, stop HTTP server, release lock file, and write final daemon state before exit
Applied to files:
src/persistence.tssrc/daemon/run.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/persistence.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} : Use acquireDaemonLock() to create an exclusive lock file with O_EXCL flag to prevent multiple daemon instances from running simultaneously
Applied to files:
src/persistence.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/persistence.tssrc/daemon/run.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/persistence.tssrc/daemon/run.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} : Register RPC handlers for 'spawn-happy-session', 'stop-session', and 'requestShutdown' on the daemon's HTTP server and WebSocket connection
Applied to files:
src/daemon/run.tssrc/daemon/sessionRegistry.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 notifyDaemonSessionStarted() webhook endpoint that receives POST requests to /session-started and creates TrackedSession entries with startedBy identifier
Applied to files:
src/daemon/run.tssrc/daemon/sessionRegistry.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/daemon/run.tssrc/daemon/sessionRegistry.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 HTTP endpoint /spawn-session that creates new sessions (used by integration tests) via daemon HTTP server
Applied to files:
src/daemon/run.tssrc/daemon/sessionRegistry.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/{auth,persistence}.{ts,tsx} : Store private keys in `~/.handy/access.key` with restricted permissions
Applied to files:
src/daemon/run.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 HTTP endpoint /stop-session that terminates specific sessions via daemon HTTP server
Applied to files:
src/daemon/run.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/daemon/run.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} : When receiving mobile app RPC call to stop daemon, immediately update daemon state to 'shutting-down' with shutdownSource and shutdownRequestedAt before performing cleanup
Applied to files:
src/daemon/run.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 HTTP endpoint /list that returns all tracked sessions from the daemon's HTTP server
Applied to files:
src/daemon/sessionRegistry.ts
🧬 Code graph analysis (2)
src/persistence.ts (2)
src/configuration.ts (1)
configuration(86-86)src/ui/logger.ts (1)
logger(235-235)
src/daemon/run.ts (2)
src/daemon/sessionRegistry.ts (3)
listSessionMarkers(101-124)removeSessionMarker(89-99)writeSessionMarker(60-87)src/daemon/doctor.ts (2)
findAllHappyProcesses(14-57)findHappyProcessByPid(59-62)
🔇 Additional comments (18)
src/daemon/doctor.ts (1)
59-62: LGTM - Clean helper function for PID lookup.The implementation correctly delegates to
findAllHappyProcessesand uses nullish coalescing for the fallback. This provides the necessary PID-to-process-type validation for the reattachment safety checks inrun.ts.src/persistence.ts (4)
261-268: LGTM - Well-structured Zod schema for daemon state validation.The schema correctly validates all fields of
DaemonLocallyPersistedStatewith appropriate constraints (positive integers for pid/port, strings for timestamps). This provides runtime safety when reading persisted state.
496-525: LGTM - Robust read with appropriate retry and error handling.The retry logic correctly handles transient filesystem conditions (e.g., catching mid-rename), while early-exiting on unrecoverable errors (corrupt JSON, schema mismatch). The 15ms delay between attempts is reasonable.
530-565: LGTM - Solid atomic write implementation with cross-platform handling.The temp-file-then-rename pattern ensures atomicity. The Windows-specific EEXIST/EPERM handling is necessary since Windows
renamedoesn't atomically overwrite. The cleanup of orphan temp files on failure is a good defensive measure.
574-577: LGTM - Good addition to clean up leftover temp files.This prevents accumulation of orphan
.tmpfiles from previous failed writes or crashes.src/daemon/run.ts (9)
28-29: LGTM - Appropriate process type allowlist for reattachment safety.The Set correctly includes both production (
daemon-spawned-session,user-session) and development (dev-daemon-spawned,dev-session,dev-related) variants to support both environments.
180-217: LGTM - Well-implemented reattachment with layered PID-reuse safety.The logic correctly:
- Validates process liveness via
process.kill(pid, 0)- Validates process type via
findAllHappyProcessescross-reference- Sets
reattachedFromDiskMarker: truefor downstream safety checks- Handles errors non-fatally to avoid blocking daemon startup
One consideration for future improvement: stale markers from crashed daemons could accumulate over time. An age-based cleanup (e.g., markers older than 24h with dead processes) could be added to the heartbeat loop.
223-227: LGTM - Necessary cross-stack isolation.This prevents a daemon from accidentally tracking sessions from a different
happyHomeDir(e.g., dev vs prod instances), which would cause incorrect lifecycle management.
264-269: LGTM - Metadata update while preserving kill-protection.Correctly updates session metadata from the webhook while keeping
reattachedFromDiskMarker: trueto maintain PID-reuse safety for stop operations.
272-280: LGTM - Best-effort marker persistence without blocking webhook.The fire-and-forget pattern with error logging is appropriate. Marker writes are auxiliary and shouldn't fail the webhook handler.
349-349: LGTM - Proper async/await usage.Using
awaitensures the token file is fully written before proceeding, preventing potential race conditions where the spawned Codex process might read an incomplete file.
679-690: LGTM - Critical PID-reuse safety check for reattached sessions.Before sending SIGTERM to a reattached session, this validates the PID still corresponds to a Happy session process. This mitigates the risk of killing an unrelated process that reused the PID.
The inherent TOCTOU window between
findHappyProcessByPidandprocess.killis minimal and acceptable—the alternative (no validation) carries significantly higher risk.
714-714: LGTM - Proper marker cleanup on child exit.Removes the session marker when the child process exits, preventing stale marker accumulation.
795-795: LGTM - Marker cleanup during heartbeat pruning.Ensures markers are cleaned up for dead processes detected during the heartbeat loop, which is especially important for reattached sessions that don't trigger the
exitevent handler.src/daemon/sessionRegistry.ts (4)
29-58: LGTM - Correct async atomic write implementation.The temp-file-then-rename pattern with Windows fallback handling mirrors the synchronous version in
persistence.ts. The cleanup of orphan temp files on failure is appropriate.
60-87: LGTM - Well-structured marker write with timestamp preservation.The logic correctly:
- Preserves
createdAtfrom existing markers for session age tracking- Forces
happyHomeDirfrom configuration (security enforcement)- Uses schema validation before writing
Note: Depends on the
startedByenum fix mentioned earlier to avoid unexpected validation failures.
89-99: LGTM - Clean removal with appropriate error handling.Correctly ignores
ENOENT(marker already gone) while logging other unexpected errors for diagnostics.
101-124: LGTM - Defensive listing with proper isolation and error handling.The implementation correctly:
- Uses
safeParseto handle malformed markers gracefully- Filters by
happyHomeDirfor stack isolation- Logs parse errors for diagnostics without failing the operation
| createdAt: z.number().int().positive(), | ||
| updatedAt: z.number().int().positive(), | ||
| flavor: z.enum(['claude', 'codex', 'gemini']).optional(), | ||
| startedBy: z.enum(['daemon', 'terminal']).optional(), |
There was a problem hiding this comment.
Potential schema mismatch for startedBy values.
The schema restricts startedBy to 'daemon' | 'terminal', but run.ts uses other values like 'happy directly - likely by user from terminal' and 'reattached' when creating TrackedSession entries. When these are passed to writeSessionMarker, the non-matching values will fail Zod validation.
Consider either:
- Making
startedBya plain string:startedBy: z.string().optional() - Extending the enum to include all actual values used
Suggested fix
- startedBy: z.enum(['daemon', 'terminal']).optional(),
+ startedBy: z.string().optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| startedBy: z.enum(['daemon', 'terminal']).optional(), | |
| startedBy: z.string().optional(), |
🤖 Prompt for AI Agents
In @src/daemon/sessionRegistry.ts at line 14, The Zod schema for startedBy (in
TrackedSession / sessionRegistry.ts) restricts values to
z.enum(['daemon','terminal']) but other code (e.g., run.ts creating
TrackedSession and writeSessionMarker) uses additional values like 'happy
directly - likely by user from terminal' and 'reattached', causing validation
failures; update the schema by either replacing startedBy:
z.enum([...]).optional() with startedBy: z.string().optional() to accept
arbitrary strings, or add the extra literal values used in run.ts to the z.enum
so the schema matches all actual startedBy values (then run tests / try creating
a TrackedSession to verify writeSessionMarker no longer fails).
|
@codex please review the new commit |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.