feat(setup): add session end lifecycle for OpenCode, Gemini, and Codex#527
Open
daniel-baf wants to merge 1 commit into
Open
feat(setup): add session end lifecycle for OpenCode, Gemini, and Codex#527daniel-baf wants to merge 1 commit into
daniel-baf wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughOn ChangesSession End Lifecycle
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@internal/setup/setup.go`:
- Line 200: The instruction at line 200 that reads "Call mem_session_end" is
incomplete and does not specify the required parameters. Update this instruction
to clarify that mem_session_end requires an explicit id parameter (the session
identifier to close) and optionally accepts a summary parameter describing the
work completed. The updated instruction should read "Call mem_session_end with
the active session id and an optional summary of work completed" to provide
clear guidance on the parameters needed.
In `@plugin/opencode/engram.ts`:
- Around line 319-321: The `engramFetch` call to POST
`/sessions/{sessionId}/end` is being made for all deleted sessions, including
sub-agent sessions that were never actually created since `ensureSession`
early-exits for those. Add a condition before the `engramFetch` call that skips
the POST request for sub-agent sessions or sessions that were never persisted,
checking whether the session is a sub-agent session type or verifying that the
session was actually persisted before attempting to call the end endpoint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0c98b8f8-f108-4b94-939c-4a304934ae99
📒 Files selected for processing (4)
internal/setup/plugins/opencode/engram.tsinternal/setup/setup.goplugin/codex/skills/memory/SKILL.mdplugin/opencode/engram.ts
| - path/to/file — [what it does or what changed] | ||
|
|
||
| This is NOT optional. If you skip this, the next session starts blind. | ||
| 2. Call mem_session_end. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect tool definitions/usage for mem_session_end parameter requirements
rg -n -C4 'mem_session_end'
# 2) Check whether wrappers auto-inject session id before tool dispatch
rg -n -C5 'activeSessionId|sessionId|params.id|callMemoryTool|executeMemoryTool'
# 3) Locate server-side /sessions/{id}/end handler contract (body requirements, validation)
rg -n -C5 '/sessions/.*/end|session.*end' --type go --type tsRepository: Gentleman-Programming/engram
Length of output: 50385
Clarify mem_session_end requires explicit session identifier and optional summary.
At line 200, "Call mem_session_end" is incomplete. The tool requires an explicit id parameter (the session identifier to close) and optionally accepts a summary describing what was accomplished. Update the instruction to read: "Call mem_session_end with the active session id and an optional summary of work completed."
🤖 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/setup.go` at line 200, The instruction at line 200 that reads
"Call mem_session_end" is incomplete and does not specify the required
parameters. Update this instruction to clarify that mem_session_end requires an
explicit id parameter (the session identifier to close) and optionally accepts a
summary parameter describing the work completed. The updated instruction should
read "Call mem_session_end with the active session id and an optional summary of
work completed" to provide clear guidance on the parameters needed.
Comment on lines
+319
to
+321
| await engramFetch(`/sessions/${encodeURIComponent(sessionId)}/end`, { | ||
| method: "POST", | ||
| }) |
There was a problem hiding this comment.
Skip /end for sub-agent sessions that were never persisted.
At Line 319, the handler posts /sessions/{id}/end for every deleted session, but sub-agent sessions are intentionally not created (ensureSession early-exits for subAgentSessions). This introduces avoidable network calls/noisy failures on Task-heavy runs.
Suggested patch
const info = (event.properties as any)?.info
const sessionId = info?.id
if (sessionId) {
- await engramFetch(`/sessions/${encodeURIComponent(sessionId)}/end`, {
- method: "POST",
- })
+ if (!subAgentSessions.has(sessionId)) {
+ await engramFetch(`/sessions/${encodeURIComponent(sessionId)}/end`, {
+ method: "POST",
+ })
+ }
toolCounts.delete(sessionId)
knownSessions.delete(sessionId)
subAgentSessions.delete(sessionId)
lastNudgeTime.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
| await engramFetch(`/sessions/${encodeURIComponent(sessionId)}/end`, { | |
| method: "POST", | |
| }) | |
| if (!subAgentSessions.has(sessionId)) { | |
| await engramFetch(`/sessions/${encodeURIComponent(sessionId)}/end`, { | |
| method: "POST", | |
| }) | |
| } |
🤖 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 `@plugin/opencode/engram.ts` around lines 319 - 321, The `engramFetch` call to
POST `/sessions/{sessionId}/end` is being made for all deleted sessions,
including sub-agent sessions that were never actually created since
`ensureSession` early-exits for those. Add a condition before the `engramFetch`
call that skips the POST request for sub-agent sessions or sessions that were
never persisted, checking whether the session is a sub-agent session type or
verifying that the session was actually persisted before attempting to call the
end endpoint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #48
PR Type
type:bug— Bug fixtype:feature— New featuretype:docs— Documentation onlytype:refactor— Code refactoring (no behavior change)type:chore— Maintenance, dependencies, toolingtype:breaking-change— Breaking changeSummary
ended_atis set) by calling thePOST /sessions/{id}/endAPI endpoint.session.deletedevent handler.mem_session_endat the end of a session.mem_session_endat the end of a session.Changes
plugin/opencode/engram.ts/sessions/{id}/endonsession.deletedinternal/setup/plugins/opencode/engram.tsinternal/setup/setup.gomemoryProtocolMarkdowninstructions withmem_session_endplugin/codex/skills/memory/SKILL.mdmem_session_endTest Plan
go test ./...go test -tags e2e ./internal/server/...Contributor Checklist
Closes #48)type:*label to this PRCo-Authored-Bytrailers in commitsSummary by CodeRabbit
Bug Fixes
Documentation