-
Notifications
You must be signed in to change notification settings - Fork 195
End all sessions when the opencode process is killed #462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,14 +4,67 @@ | |||||||||||||||||||||||||||||||||||||||||||||
| // Requires Bun runtime (used by OpenCode's plugin system for loading ESM plugins). | ||||||||||||||||||||||||||||||||||||||||||||||
| import type { Plugin } from "@opencode-ai/plugin" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| export const EntirePlugin: Plugin = async ({ $, directory }) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| // 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. | ||||||||||||||||||||||||||||||||||||||||||||||
| function spawnCleanupWatcher(entireCmd: string) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const script = ` | ||||||||||||||||||||||||||||||||||||||||||||||
| const { execFileSync } = require("child_process"); | ||||||||||||||||||||||||||||||||||||||||||||||
| const cmd = ${JSON.stringify(entireCmd)}; | ||||||||||||||||||||||||||||||||||||||||||||||
| const sessions = new Set(); | ||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+27
|
||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| process.stdin.on("end", () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| for (const id of sessions) { | ||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||
| execFileSync(cmd, ["hooks", "opencode", "session-end"], { | ||||||||||||||||||||||||||||||||||||||||||||||
| input: JSON.stringify({ session_id: id }), | ||||||||||||||||||||||||||||||||||||||||||||||
| timeout: 1000, | ||||||||||||||||||||||||||||||||||||||||||||||
gtrrz-victor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| } catch {} | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| `; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||
| const child = Bun.spawn(["node", "-e", script], { | ||||||||||||||||||||||||||||||||||||||||||||||
| stdin: "pipe", | ||||||||||||||||||||||||||||||||||||||||||||||
| stdout: "ignore", | ||||||||||||||||||||||||||||||||||||||||||||||
| stderr: "ignore", | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+45
to
+46
|
||||||||||||||||||||||||||||||||||||||||||||||
| stderr: "ignore", | |
| }); | |
| stderr: "ignore", | |
| detached: true, | |
| }); | |
| // Allow the watcher process to run independently and avoid zombie processes. | |
| child.unref(); |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
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.
| 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
AI
Feb 23, 2026
There was a problem hiding this comment.
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
AI
Feb 23, 2026
There was a problem hiding this comment.
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.
| 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
AI
Feb 23, 2026
There was a problem hiding this comment.
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
AI
Feb 23, 2026
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.