Skip to content

feat(setup): add session end lifecycle for OpenCode, Gemini, and Codex #527

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.

Sign up for GitHub

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

Open
daniel-baf wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat(setup): add session end lifecycle for OpenCode, Gemini, and Codex #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c745112
feat(setup): add session end lifecycle for OpenCode, Gemini, and Code…
daniel-baf Jun 20, 2026
ef02c59
fix(opencode): avoid ending unpersisted sessions
daniel-baf Jun 23, 2026
c775989
Merge branch 'main' into feat/session-end-lifecycle
daniel-baf Jun 30, 2026
a568354
Merge branch 'main' into feat/session-end-lifecycle
daniel-baf Jul 20, 2026
49097aa
fix(setup): keep session-close steps as a contiguous numbered list
daniel-baf Jul 20, 2026
6e52fd5
fix(opencode): require confirmed HTTP success for session tracking
daniel-baf Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions internal/setup/plugins/opencode/engram.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ Also search memory PROACTIVELY when:
### SESSION CLOSE PROTOCOL (mandatory)

Before ending a session or saying "done" / "that's it", you MUST:
1. Call \`mem_session_summary\` with this structure:
1. Call \`mem_session_summary\` with the structure below.
2. Call \`mem_session_end\` with the active session id and an optional summary of work completed.

\`mem_session_summary\` structure:

## Goal
[What we were working on this session]
Expand Down Expand Up @@ -133,6 +136,9 @@ async function engramFetch(
headers: opts.body ? { "Content-Type": "application/json" } : undefined,
body: opts.body ? JSON.stringify(opts.body) : undefined,
})
// Treat non-2xx as failure so callers can avoid false-positive side effects
// (e.g. marking a session known when POST /sessions did not succeed).
if (!res.ok) return null
return await res.json()
Comment on lines +139 to 142

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle empty JSON responses gracefully.

If the API returns a successful response (e.g., 204 No Content or 200 OK with an empty body), res.json() will throw a SyntaxError. This error is caught by the blanket catch block, causing the function to incorrectly return null and treat the successful request as a failure. Both files contain the exact same vulnerable fetch implementation.

  • internal/setup/plugins/opencode/engram.ts#L139-L142: Parse the response text first to safely handle empty bodies.
  • plugin/opencode/engram.ts#L139-L142: Parse the response text first to safely handle empty bodies.
🛠️ Proposed fix 
-    if (!res.ok) return null
-    return await res.json()
+    if (!res.ok) return null
+    const text = await res.text()
+    return text ? JSON.parse(text) : {}
📝 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.

Suggested change
// Treat non-2xx as failure so callers can avoid false-positive side effects
// (e.g. marking a session known when POST /sessions did not succeed).
if (!res.ok) return null
return await res.json()
// Treat non-2xx as failure so callers can avoid false-positive side effects
// (e.g. marking a session known when POST /sessions did not succeed).
if (!res.ok) return null
const text = await res.text()
return text ? JSON.parse(text) : {}
📍 Affects 2 files
  • internal/setup/plugins/opencode/engram.ts#L139-L142 (this comment)
  • plugin/opencode/engram.ts#L139-L142
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/setup/plugins/opencode/engram.ts` around lines 139 - 142, Update the
fetch response handling in both
internal/setup/plugins/opencode/engram.ts#L139-L142 and
plugin/opencode/engram.ts#L139-L142: after the existing res.ok check, read the
response text and return an empty-safe result when the body is blank; otherwise
parse and return the JSON. Preserve the existing failure return for non-2xx
responses and avoid treating successful empty responses as errors.

} catch {
// Engram server not running — silently fail
Expand Down Expand Up @@ -225,15 +231,18 @@ export const Engram: Plugin = async (ctx) => {
if (!sessionId || knownSessions.has(sessionId)) return
// Do not register sub-agent sessions in Engram (issue #116).
if (subAgentSessions.has(sessionId)) return
knownSessions.add(sessionId)
await engramFetch("/sessions", {
const created = await engramFetch("/sessions", {
method: "POST",
body: {
id: sessionId,
project,
directory: ctx.directory,
},
})
// Only mark known after confirmed create so failed requests can retry.
if (created != null) {
knownSessions.add(sessionId)
}
}

// Try to start engram server if not running
Expand Down Expand Up @@ -316,8 +325,24 @@ export const Engram: Plugin = async (ctx) => {
const info = (event.properties as any)?.info
const sessionId = info?.id
if (sessionId) {
if (knownSessions.has(sessionId)) {
// session.deleted fires once — retry once inline, and only clear
// knownSessions after a confirmed /end (do not pretend it closed).
let ended = await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
if (ended == null) {
ended = await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
}
if (ended != null) {
knownSessions.delete(sessionId)
}
Comment on lines +329 to +343

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Remove the inline retry loop to prevent memory leaks and keep the adapter thin.

Because session.deleted fires only once, conditionally deleting from knownSessions permanently leaks the session ID in memory if the endpoint fails, since there is no background retry mechanism. Furthermore, inline retry loops violate the path instructions for plugin/** which mandate that adapters stay thin and contain no business logic. Both files share this identical flaw.

  • internal/setup/plugins/opencode/engram.ts#L329-L343: Remove the inline retry loop and unconditionally delete the session from knownSessions.
  • plugin/opencode/engram.ts#L329-L343: Remove the inline retry loop and unconditionally delete the session from knownSessions.
♻️ Proposed fix 
-            // session.deleted fires once — retry once inline, and only clear
-            // knownSessions after a confirmed /end (do not pretend it closed).
-            let ended = await engramFetch(
-              `/sessions/${encodeURIComponent(sessionId)}/end`,
-              { method: "POST" }
-            )
-            if (ended == null) {
-              ended = await engramFetch(
-                `/sessions/${encodeURIComponent(sessionId)}/end`,
-                { method: "POST" }
-              )
-            }
-            if (ended != null) {
-              knownSessions.delete(sessionId)
-            }
+            await engramFetch(
+              `/sessions/${encodeURIComponent(sessionId)}/end`,
+              { method: "POST" }
+            )
+            knownSessions.delete(sessionId)
📝 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.

Suggested change
// session.deleted fires once — retry once inline, and only clear
// knownSessions after a confirmed /end (do not pretend it closed).
let ended = await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
if (ended == null) {
ended = await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
}
if (ended != null) {
knownSessions.delete(sessionId)
}
await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
knownSessions.delete(sessionId)
📍 Affects 2 files
  • internal/setup/plugins/opencode/engram.ts#L329-L343 (this comment)
  • plugin/opencode/engram.ts#L329-L343
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/setup/plugins/opencode/engram.ts` around lines 329 - 343, In
internal/setup/plugins/opencode/engram.ts lines 329-343 and
plugin/opencode/engram.ts lines 329-343, remove the inline retry and
response-dependent condition around the session end request, then
unconditionally delete sessionId from knownSessions after issuing the single
/end request. Keep both adapter implementations thin and behaviorally identical.

Sources: Coding guidelines, Path instructions

}
toolCounts.delete(sessionId)
knownSessions.delete(sessionId)
subAgentSessions.delete(sessionId)
lastNudgeTime.delete(sessionId)
}
Expand Down
5 changes: 4 additions & 1 deletion internal/setup/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,10 @@ Also search memory PROACTIVELY when:
### SESSION CLOSE PROTOCOL (mandatory)

Before ending a session or saying "done" / "listo" / "that's it", you MUST:
1. Call mem_session_summary with this structure:
1. Call mem_session_summary with the structure below.
2. Call mem_session_end with the active session id and an optional summary of work completed.

mem_session_summary structure:

## Goal
[What we were working on this session]
Expand Down
5 changes: 4 additions & 1 deletion plugin/codex/skills/memory/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ Also search memory PROACTIVELY when:
## SESSION CLOSE PROTOCOL (mandatory)

Before ending a session or saying "done" / "that's it", you MUST:
1. Call `mem_session_summary` with this structure:
1. Call `mem_session_summary` with the structure below.
2. Call `mem_session_end` with the active session id and an optional summary of work completed.

`mem_session_summary` structure:

## Goal
[What we were working on this session]
Expand Down
33 changes: 29 additions & 4 deletions plugin/opencode/engram.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ Also search memory PROACTIVELY when:
### SESSION CLOSE PROTOCOL (mandatory)

Before ending a session or saying "done" / "that's it", you MUST:
1. Call \`mem_session_summary\` with this structure:
1. Call \`mem_session_summary\` with the structure below.
2. Call \`mem_session_end\` with the active session id and an optional summary of work completed.

\`mem_session_summary\` structure:

## Goal
[What we were working on this session]
Expand Down Expand Up @@ -133,6 +136,9 @@ async function engramFetch(
headers: opts.body ? { "Content-Type": "application/json" } : undefined,
body: opts.body ? JSON.stringify(opts.body) : undefined,
})
// Treat non-2xx as failure so callers can avoid false-positive side effects
// (e.g. marking a session known when POST /sessions did not succeed).
if (!res.ok) return null
return await res.json()
} catch {
// Engram server not running — silently fail
Expand Down Expand Up @@ -225,15 +231,18 @@ export const Engram: Plugin = async (ctx) => {
if (!sessionId || knownSessions.has(sessionId)) return
// Do not register sub-agent sessions in Engram (issue #116).
if (subAgentSessions.has(sessionId)) return
knownSessions.add(sessionId)
await engramFetch("/sessions", {
const created = await engramFetch("/sessions", {
method: "POST",
body: {
id: sessionId,
project,
directory: ctx.directory,
},
})
// Only mark known after confirmed create so failed requests can retry.
if (created != null) {
knownSessions.add(sessionId)
}
}

// Try to start engram server if not running
Expand Down Expand Up @@ -316,8 +325,24 @@ export const Engram: Plugin = async (ctx) => {
const info = (event.properties as any)?.info
const sessionId = info?.id
if (sessionId) {
if (knownSessions.has(sessionId)) {
// session.deleted fires once — retry once inline, and only clear
// knownSessions after a confirmed /end (do not pretend it closed).
let ended = await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
if (ended == null) {
ended = await engramFetch(
`/sessions/${encodeURIComponent(sessionId)}/end`,
{ method: "POST" }
)
}
if (ended != null) {
knownSessions.delete(sessionId)
}
}
toolCounts.delete(sessionId)
knownSessions.delete(sessionId)
subAgentSessions.delete(sessionId)
lastNudgeTime.delete(sessionId)
}
Expand Down