Skip to content

fix(profiles): release the session profile binding when its session ends - #2491

Open
pujitha24 wants to merge 2 commits into
Gentleman-Programming:mainfrom
pujitha24:auto/issue-1707
Open

pujitha24 wants to merge 2 commits into
Gentleman-Programming:mainfrom
pujitha24:auto/issue-1707

Conversation

@pujitha24

@pujitha24 pujitha24 commented Oct 23, 2026 •

Copy link
Copy Markdown

Motivation: bindSessionProfile stores a cloned routing snapshot per parent
session in a process-wide Map. Only an explicit clearSessionProfileBinding
(never called in production) removed entries, so a long-lived extension host
retained one snapshot per ended session for the life of the process.

Approach: the Gentle AI session_shutdown hook now clears the ending session's
binding whenever the shutdown reason is not "reload". Reload keeps the same
session id, so its binding is preserved; other sessions are untouched.
Impact is bounded memory growth in long-lived hosts; no user-visible routing
behavior changes for live sessions.

Validation:

  • node --experimental-strip-types --test tests/session-profile-binding.test.ts
    -> new test fails without the hook change (13 pass / 1 fail) and passes
    with it (14 pass / 0 fail).
  • node --experimental-strip-types --test tests/yolo-mode-runtime.test.ts tests/session-profile-*.test.ts -> 103 pass, 0 fail.
  • pnpm run typecheck -> no regressions.
    The test exercises reload and quit; new/resume/fork share the quit code path.

Report: #1707
Signed-off-by: Pujitha Paladugu [email protected]
Assisted-by: claude-sonnet-5-5 (via Claude Code)

Fixes #1707

Summary by CodeRabbit

  • Bug Fixes
    • Session profile associations are cleared when a session ends, except during a reload. Reloads continue to preserve the association, and ending one session does not affect associations for other sessions.
  • Tests
    • Added checks for session shutdown and reload behavior, including verification that unrelated sessions retain their associations.

Motivation: bindSessionProfile stores a cloned routing snapshot per parent
session in a process-wide Map. Only an explicit clearSessionProfileBinding
(never called in production) removed entries, so a long-lived extension host
retained one snapshot per ended session for the life of the process.

Approach: the Gentle AI session_shutdown hook now clears the ending session's
binding whenever the shutdown reason is not "reload". Reload keeps the same
session id, so its binding is preserved; other sessions are untouched.
Impact is bounded memory growth in long-lived hosts; no user-visible routing
behavior changes for live sessions.

Validation:
- node --experimental-strip-types --test tests/session-profile-binding.test.ts
  -> new test fails without the hook change (13 pass / 1 fail) and passes
  with it (14 pass / 0 fail).
- node --experimental-strip-types --test tests/yolo-mode-runtime.test.ts tests/session-profile-*.test.ts -> 103 pass, 0 fail.
- pnpm run typecheck -> no regressions.
The test exercises reload and quit; new/resume/fork share the quit code path.

Report: Gentleman-Programming#1707
Signed-off-by: Pujitha Paladugu <[email protected]>
Assisted-by: claude-sonnet-5-5 (via Claude Code)
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 7935ea93-783a-49ed-b2c3-f8ed627c28b6
Commits

Reviewing files that changed from the base of the PR and between 723397c and b7f0d6f.

Files selected for processing: 1
  • tests/session-profile-binding.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The Gentle AI extension now clears a session’s profile binding when the session shuts down for a reason other than reload. An integration test checks binding retention on reload and cleanup on quit.

Changes: Session profile binding cleanup

Layer / File(s) Summary
Shutdown binding cleanup
extensions/gentle-ai.ts, tests/session-profile-binding.test.ts
The shutdown hook clears the ending session’s binding for non-reload shutdowns. The integration test verifies that reload retains the binding, quit clears it, and another session’s binding remains.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

Suggested labels: type:bug

Suggested reviewers: danielgap, barbatdev

Merge Risk: Minimal · up to b7f0d

No concrete production failure or merge-blocking test gap is established by the reviewed change.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: releasing a session's profile binding when the session ends.
Linked Issues check Passed Issue #1707 requires ended-session bindings to be released or the store to be bounded. extensions/gentle-ai.ts now calls clearSessionProfileBinding for the ending session when the shutdown reason …
Out of Scope Changes check Passed The changes add shutdown cleanup and focused tests for the session profile binding store. The import, cleanup condition, and test assertions directly support issue #1707. No unrelated change appears i…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.
Full details: Linked Issues check

Explanation

Issue #1707 requires ended-session bindings to be released or the store to be bounded. extensions/gentle-ai.ts now calls clearSessionProfileBinding for the ending session when the shutdown reason is not "reload". The test verifies reload preservation, quit cleanup, and isolation of another session. This meets the issue requirement.

Full details: Out of Scope Changes check

Explanation

The changes add shutdown cleanup and focused tests for the session profile binding store. The import, cleanup condition, and test assertions directly support issue #1707. No unrelated change appears in the reviewed diff.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@carlosmoradev carlosmoradev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. The fix is minimal and lands in the right place: one release inside the existing reason !== "reload" branch, keyed by the ending session's id, and the test pins that reload keeps the binding while other sessions stay untouched. I also checked the readers (lib/session-profile-freeze.ts:174 and the status paths in extensions/gentle-ai.ts): they all read with the live session's id.

One non-blocking question: if a subagent or background job of the ending session resolves routing after session_shutdown, it now falls back to pinnedEffectiveModelConfig/readModelConfig instead of the bound snapshot. I'd expect that to be benign, but did you consider in-flight work finishing after shutdown? I ran tests/session-profile-*.test.ts locally (99/99) but not typecheck, so I'm relying on your validation there.

@pujitha24

Copy link
Copy Markdown
Author

No, I didn't trace in-flight work specifically. From the code, a late resolver after shutdown would hit the fallback at extensions/gentle-ai.ts:2500 (pinnedEffectiveModelConfig(cwd) ?? readModelConfig(cwd)), so it gets the pinned or on-disk config rather than the frozen snapshot, and I'd expect that to be benign too. I haven't tested that path, though, so if the snapshot matters for a subagent that outlives its parent session, I'm happy to look at it or defer the clear. Typecheck I ran locally (no regressions), as noted in the commit message.

@decode2

decode2 commented Oct 26, 2026

Copy link
Copy Markdown
Member

Thanks @pujitha24. The defect is real: clearSessionProfileBinding exists but nothing in production calls it, so ended sessions keep their binding for process lifetime. The shutdown-handler placement and the reload skip both match the module's documented semantics.

One change before merge: the integration test deep-imports ../node_modules/@earendil-works/pi-coding-agent/dist/core/extensions/loader.js. No other test does that, and it couples the suite to the SDK's internal layout on every bump. Please use the repo's usual test harness so the loader path is not imported directly; the behavior assertions (reload keeps, quit clears, other sessions intact) are exactly right.

…tead of the SDK loader

Drops the deep import of the SDK's internal extensions/loader.js; the
test now registers the extension on a stub pi like the other gentle-ai
tests and invokes the session_shutdown handler directly.

Signed-off-by: Pujitha Paladugu <[email protected]>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(profiles): session binding store never evicts ended sessions

3 participants