refactor(server): clean up Session lifetime model

[16bit-ykiko]

Summary

• Store sessions as shared_ptr<Session> in the sessions map to prevent use-after-free when coroutines hold references across co_await while DenseMap reallocates or entries are erased by didClose.
• Compiler takes shared_ptr<Session> by value in all public coroutine methods (ensure_compiled, forward_query, forward_build, forward_format, handle_completion), expressing lifetime ownership in the type signature. The sessions map reference is removed from Compiler entirely.
• Decouple Indexer from sessions map — replace direct DenseMap<uint32_t, Session>& access with two callbacks (is_open, each_overlay), exposed as for_each_overlay() / get_overlay() helpers.
• Unify staleness detection on generation — bump generation on both didChange and didClose, making the closed flag unnecessary. All post-co_await checks use a snapshot-and-compare pattern (session->generation != gen).
• Fix stale-compile-after-close bug — ensure_compiled now checks generation after the wait loop exits, preventing it from spawning a compile for a session that was closed while waiting.
• Fix missing staleness checks — forward_query and forward_build now check generation after their respective co_await points (previously forward_build only checked session existence, missing edits; forward_query re-looked up the map instead of checking the held session).
• Use params.text for overlay content — run_compile stores the text snapshot taken before co_await into the OpenFileIndex, not the potentially-mutated session->text.

Motivation

Sessions stored inline in DenseMap could be invalidated by container reallocation while async coroutines held Session& references across suspension points. The closed flag and find_session() re-lookups were ad-hoc mitigations that didn't cover all code paths. Moving to shared_ptr storage with generation-based staleness gives a single, uniform mechanism that is correct by construction.

Test plan

• All 621 unit tests pass
• All 175 integration tests pass
• All 3 smoke tests pass

Summary by CodeRabbit

• Refactor
  • Refactored internal session and compilation lifetime management to use shared pointers instead of external session maps, simplifying session access patterns throughout the compiler and indexer.
  • Updated indexing system to use overlay-based iteration for symbol and definition lookups, improving consistency and reliability of language server features.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ca7c98bb-3cc8-4aa2-9108-b91f59a23d8c

📥 Commits

Reviewing files that changed from the base of the PR and between ae65cda and b04966d.

📒 Files selected for processing (1)

• src/server/compiler/compiler.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• src/server/compiler/compiler.cpp

---

📝 Walkthrough

Walkthrough

Session lifetime is refactored from map-value/raw-pointer storage to std::shared_ptr<Session>. MasterServer updates its sessions map to store shared pointers and returns them from find_session/open_session. Compiler removes its internal sessions-map reference and uses shared-pointer lifecycle guards in run_compile with generation-mismatch early exits. Indexer replaces direct sessions-map access with is_open and each_overlay callbacks, exposing for_each_overlay and get_overlay public methods. All symbol resolution and request handling in LSPClient and AgentClient are updated to use the new session ownership model.

Changes

Session Shared Ownership and Overlay-Based Access Refactoring

Layer / File(s)	Summary
Session shared ownership foundation and lifecycle src/server/service/session.h, src/server/service/master_server.h, src/server/service/master_server.cpp	MasterServer's sessions map stores shared_ptr<Session>; find_session returns nullptr or shared pointer; open_session creates/returns shared pointer with generation tracking; close_session increments generation before erasing; on_file_saved dirtying logic updated for new return types; documentation updated to reflect close-time generation increment.
Compiler: session shared_ptr integration and sessions-map removal src/server/compiler/compiler.h, src/server/compiler/compiler.cpp	Constructor and class remove the externally supplied sessions-map parameter/member; run_compile signature changes to accept shared_ptr<Session> with generation-mismatch guards before fill_compile_args, dependency preparation, and worker dispatch; on successful compilation, session->file_index is rebuilt from session->text; ensure_compiled, forward_query, forward_build, forward_format, and handle_completion all now take shared_ptr<Session> with generation/ast_dirty early-return checks.
Indexer overlay callback API and public methods src/server/compiler/indexer.h	Constructor replaces sessions-map parameter with is_open and each_overlay callbacks; OverlayVisitor type alias added; for_each_overlay and get_overlay public methods expose overlay iteration and lookup; is_proj_path_open reworked to use is_open callback; private members store callbacks instead of sessions reference.
Indexer: overlay-based symbol and relation lookups src/server/compiler/indexer.cpp	All query methods (find_symbol_info, query_relations, find_definition_location, collect_grouped_relations, collect_unique_targets, get_definition_text, collect_references, search_symbols) iterate open-file overlays via for_each_overlay; overlay mapper/content resolve local source ranges and extract definition/reference text; background indexing (index_one, run_background_indexing) replace sessions.contains with is_open predicate usage.
MasterServer: indexer initialization with overlay callbacks src/server/service/master_server.cpp	Constructor refactored to initialize Indexer with is_open and each_overlay callbacks; is_open predicate checks sessions.contains for server path ids; each_overlay visitor iterates the sessions map and forwards only entries with non-null file_index to the provided callback.
LSPClient: session pointer handling across request handlers src/server/service/lsp_client.cpp	All request handlers (didOpen, didChange, Hover, SemanticTokens, InlayHint, FoldingRange, DocumentSymbol, CodeAction, Definition, Completion, SignatureHelp, DocumentFormatting, DocumentRangeFormatting, clice/currentContext, clice/switchContext) receive find_session/open_session results as shared_ptr with pointer-member access (session->...); resolve_uri inlines .get() extraction; DocumentLink reuses the already-fetched session instead of a second lookup.
AgentClient: overlay-based symbol resolution and request handling src/server/service/agent_client.cpp	resolve_locator removes sessions parameter and refactors to use indexer.for_each_overlay (name-based search) and indexer.get_overlay (path+line-based lookups); SymbolSearch iterates overlay indices; DocumentSymbols prefers overlay file_index when available; Definition, References, CallGraph, TypeHierarchy, ReadSymbol handlers drop srv.sessions from resolve_locator calls.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• clice-io/clice#406: Both PRs refactor the compiler's session-driven compilation/forwarding flow (notably ensure_compiled and the forward_*/completion handlers) to use the new Session state model introduced in #406, with the main PR further tightening lifetime/dispatch by switching to std::shared_ptr<Session> and removing compiler-local sessions-map lookups.

• clice-io/clice#438: Both PRs modify the symbol-resolution path used by the agentic API—src/server/service/agent_client.cpp (notably resolve_locator/handler logic) so agentic requests query the compiler/indexer/session data through the refactored indexer/overlay model.

• clice-io/clice#454: Both PRs touch the core compilation supersession/generation logic in src/server/compiler/compiler.cpp (e.g., run_compile/ensure_deps handling of dependency/PCH preparation, generation mismatch, and cancellation/finishing in-flight compiles), even though the main PR also refactors session lifetime to std::shared_ptr.

Poem

🐇 A rabbit once held sessions by raw address and map,
But pointers would dangle when files took a nap.
Now shared_ptr guards keep the session alive,
Overlays replace the old sessions-map jive.
Every async hop gets a lifetime-safe dance,
And generation guards give cancellation a chance! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.93% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor(server): clean up Session lifetime model' directly and specifically summarizes the primary architectural change—moving from inline session storage to shared_ptr-based ownership and cleaning up the session lifetime management system.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/session-shared-ptr

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b557e152f0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

🧹 Nitpick comments (1)

src/server/compiler/compiler.cpp (1)

732-732: 💤 Low value

Consider using params.text instead of session->text for ofi.content.

While the generation check at line 707 ensures the session text hasn't changed if we reach this point, using params.text (the actual content sent to the worker) would make the intent clearer and provide defense against future refactoring.

Suggested change

-        ofi.content = session->text;
+        ofi.content = params.text;

🤖 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 `@src/server/compiler/compiler.cpp` at line 732, The assignment of ofi.content
is currently using session->text, but should instead use params.text which
represents the actual content sent to the worker. This change makes the intent
clearer and provides better protection against future refactoring. Locate the
line where ofi.content is assigned to session->text and change it to use
params.text instead.

🤖 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.

Nitpick comments:
In `@src/server/compiler/compiler.cpp`:
- Line 732: The assignment of ofi.content is currently using session->text, but
should instead use params.text which represents the actual content sent to the
worker. This change makes the intent clearer and provides better protection
against future refactoring. Locate the line where ofi.content is assigned to
session->text and change it to use params.text instead.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2c4314f1-9506-4401-be1f-30f0537ea92f

📥 Commits

Reviewing files that changed from the base of the PR and between b557e15 and f43565c.

📒 Files selected for processing (5)

• src/server/compiler/compiler.cpp
• src/server/compiler/compiler.h
• src/server/service/lsp_client.cpp
• src/server/service/master_server.cpp
• src/server/service/session.h

✅ Files skipped from review due to trivial changes (1)

• src/server/service/session.h

🚧 Files skipped from review as they are similar to previous changes (1)

• src/server/service/lsp_client.cpp

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f43565c1ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".