Skip to content

Comments

End all sessions when the opencode process is killed#462

Open
gtrrz-victor wants to merge 1 commit intosoph/opencode-refactorfrom
gtrrz-victor/opencode-detect-process-exit
Open

End all sessions when the opencode process is killed#462
gtrrz-victor wants to merge 1 commit intosoph/opencode-refactorfrom
gtrrz-victor/opencode-detect-process-exit

Conversation

@gtrrz-victor
Copy link
Contributor

Entire-Checkpoint: d4c504e45799

…killed so we can end up the sessions

Entire-Checkpoint: d4c504e45799
Copilot AI review requested due to automatic review settings February 23, 2026 02:30
@gtrrz-victor gtrrz-victor requested a review from a team as a code owner February 23, 2026 02:30
@cursor
Copy link

cursor bot commented Feb 23, 2026

PR Summary

Medium Risk
Introduces a new background child process and IPC that triggers session cleanup on parent exit, which could affect reliability or performance if the watcher misbehaves.

Overview
Ensures OpenCode sessions are properly closed even if the OpenCode process is abruptly terminated.

The plugin now spawns a background "cleanup watcher" child process and tracks active session IDs; when the parent exits and the stdin pipe closes, the watcher invokes entire hooks opencode session-end for any remaining tracked sessions. Session lifecycle handling is updated to track on session.created and untrack on session.deleted.

Written by Cursor Bugbot for commit 570bc11. Configure here.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a cleanup mechanism to ensure OpenCode sessions are properly ended when the OpenCode process is killed unexpectedly. The solution spawns a child Node.js process that monitors the parent via stdin pipe - when the parent exits (and the pipe closes), the child calls entire hooks opencode session-end for all tracked sessions.

Changes:

  • Implements a process-based cleanup watcher using Bun.spawn and a child Node.js process
  • Tracks active sessions via IPC messages over stdin/stdout
  • Automatically cleans up orphaned sessions when the parent process exits

Comment on lines +15 to +27
let buf = "";
process.stdin.setEncoding("utf8");
process.stdin.on("data", (chunk) => {
buf += chunk;
let i;
while ((i = buf.indexOf("\\n")) !== -1) {
try {
const msg = JSON.parse(buf.slice(0, i));
if (msg.type === "track") sessions.add(msg.id);
else if (msg.type === "untrack") sessions.delete(msg.id);
} catch {}
buf = buf.slice(i + 1);
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The child process's stdin handlers accumulate data in buf without any size limit. A malicious or buggy parent could send an unbounded amount of data before closing stdin, causing the child process to consume excessive memory. Consider adding a maximum buffer size and discarding or rejecting data once the limit is reached.

Copilot uses AI. Check for mistakes.
Comment on lines 94 to 156
@@ -99,6 +153,7 @@ export const EntirePlugin: Plugin = async ({ $, directory }) => {
await callHook("session-end", {
session_id: session.id,
})
watcher.untrack(session.id)
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a potential race condition between session-start/session-end events and the cleanup watcher. If a session-end event occurs normally (via session.deleted) and untrack() is called at line 156, but the parent process is killed before the untrack message is processed by the child, the child may still attempt to clean up the already-ended session. While the try-catch at line 31 will suppress errors, this could result in unnecessary processing or duplicate session-end calls. Consider adding idempotency checks on the Go side to handle duplicate session-end calls gracefully.

Copilot uses AI. Check for mistakes.
// Track current session ID for message events (which don't include sessionID)
let currentSessionID: string | null = null
// In-memory store for message metadata (role, tokens, etc.)
const messageStore = new Map<string, any>()
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If OpenCode is restarted while sessions are still active, the cleanup watcher is spawned fresh without knowledge of pre-existing sessions. If the OpenCode process is then killed, those pre-existing sessions will not be cleaned up because they were never tracked by the watcher. Consider implementing a mechanism to discover and track existing sessions when the plugin initializes, or document this limitation.

Suggested change
const messageStore = new Map<string, any>()
const messageStore = new Map<string, any>()
// NOTE: The cleanup watcher only tracks sessions reported during the lifetime
// of this plugin instance. If OpenCode is restarted while sessions are still
// active, those pre-existing sessions will not be known to this watcher and
// may not receive automatic session-end cleanup if the process then exits.

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +46
stderr: "ignore",
});
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The child process spawned by Bun.spawn will become a zombie process when the parent exits, as there is no mechanism to reap it. The child process will terminate when stdin closes, but it will remain as a zombie until a parent process reaps it (calls wait). Consider using the detached: true option in Bun.spawn and calling child.unref() to allow the child to run independently and avoid zombie processes.

Suggested change
stderr: "ignore",
});
stderr: "ignore",
detached: true,
});
// Allow the watcher process to run independently and avoid zombie processes.
child.unref();

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +54
try { child.stdin.write(JSON.stringify({ type, id }) + "\n"); child.stdin.flush(); } catch { }
};
return {
track: (id: string) => send("track", id),
untrack: (id: string) => send("untrack", id),
};
} catch {
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The write to child.stdin at line 48 could fail if the child process has already exited or if the pipe is broken. While the try-catch suppresses the error, this means that track/untrack calls could silently fail, leading to sessions not being cleaned up. Consider logging these failures or implementing a retry mechanism for critical operations like tracking new sessions.

Suggested change
try { child.stdin.write(JSON.stringify({ type, id }) + "\n"); child.stdin.flush(); } catch { }
};
return {
track: (id: string) => send("track", id),
untrack: (id: string) => send("untrack", id),
};
} catch {
try {
child.stdin.write(JSON.stringify({ type, id }) + "\n");
child.stdin.flush();
} catch (err) {
// Log (but do not rethrow) so failed tracking doesn't remain silent
console.error("[entire-plugin] failed to send session tracking message to cleanup watcher:", err);
}
};
return {
track: (id: string) => send("track", id),
untrack: (id: string) => send("untrack", id),
};
} catch (err) {
// If the watcher process cannot be started, log and fall back to no-op tracking
console.error("[entire-plugin] failed to spawn cleanup watcher process:", err);

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +9
// Spawns a child process connected to the parent via stdin pipe.
// When the parent exits (including SIGKILL), the pipe closes and the child
// calls `entire hooks opencode session-end` for every tracked session.
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states that the child process handles SIGKILL, but this is incorrect. SIGKILL cannot be caught or handled by any process - it immediately terminates the process without allowing any cleanup. What actually happens is that when the parent process is terminated (by any signal including SIGKILL), the pipe closes, which triggers the 'end' event on stdin. The comment should be corrected to accurately describe this mechanism.

Copilot uses AI. Check for mistakes.
Comment on lines +41 to +56
try {
const child = Bun.spawn(["node", "-e", script], {
stdin: "pipe",
stdout: "ignore",
stderr: "ignore",
});
const send = (type: string, id: string) => {
try { child.stdin.write(JSON.stringify({ type, id }) + "\n"); child.stdin.flush(); } catch { }
};
return {
track: (id: string) => send("track", id),
untrack: (id: string) => send("untrack", id),
};
} catch {
return { track: () => { }, untrack: () => { } };
}
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code assumes that the 'node' command is available in the system PATH, but there is no validation or graceful degradation if Node.js is not installed. While the outer try-catch at line 41 will catch spawn failures and return no-op functions, this could lead to silent failures where sessions are never cleaned up. Consider adding a comment explaining this fallback behavior, or implementing a one-time check/warning to notify users if node is not available.

Copilot uses AI. Check for mistakes.
Comment on lines 93 to +94
currentSessionID = session.id
watcher.track(session.id)
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The watcher tracks sessions when session.created fires, but if the track() call fails silently (e.g., child process already exited), and then the OpenCode process is killed before a normal session-end event, the session will be orphaned and not cleaned up. Consider adding validation or logging to detect when the watcher fails to initialize or track sessions, so users are aware of potential cleanup issues.

Copilot uses AI. Check for mistakes.
@gtrrz-victor gtrrz-victor changed the base branch from gtrrz-victor/opencode-refactor to soph/opencode-refactor February 23, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant