feat: session allowlist on dialectic and representation via filters#882
Open
VVoruganti wants to merge 11 commits into
Open
feat: session allowlist on dialectic and representation via filters#882VVoruganti wants to merge 11 commits into
VVoruganti wants to merge 11 commits into
Conversation
session_name was only applied to the recent-documents query in RepresentationManager; the semantic and most-derived paths ignored it, so limit_to_session leaked cross-session conclusions into perspectives. - Thread a session allowlist (session_names) uniformly through all three query paths; pushed down to pgvector and external vector stores - Accept a list so the upcoming session-allowlist API reuses this path - Fail closed on an empty allowlist (downstream stores drop empty IN clauses, which would silently widen scope) Fixes DEV-1994 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
{"session_id": ["s1", "s2"]} is now shorthand for
{"session_id": {"in": [...]}} on regular columns, generically
(peer_id, etc.). JSONB metadata columns are excluded — a bare list
there keeps JSONB containment semantics, unchanged.
Previously a bare list on a regular column compiled to a type-mismatched
equality that matched nothing, so this is strictly additive.
Also translates the same shape in the turbopuffer/lancedb filter
builders, and fixes lancedb dropping empty IN clauses (fail-open) —
an empty membership list now emits an always-false condition.
Groundwork for DEV-1995 (session allowlist via the existing filters
DSL, no new API params)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a constrained 'filters' body to peer.chat and /representation —
the same DSL search and conclusions already accept, supporting only the
session_id key (a session id, a bare list, or {"in": [...]}).
Unsupported keys and shapes are rejected with 422, never silently
ignored. Composes with session_id (must be included in the allowlist
when both are given). Capped at 1,000 sessions per request.
Enforcement is uniform at every recall chokepoint, fail-closed:
- dialectic prefetch + search_memory: conclusion recall restricted to
the allowlist; dream docs (session_name IS NULL) excluded
- message tools (search/grep/date-range/temporal/context/history):
strict intersection of allowlist and observer session membership
- get_reasoning_chain: unavailable under an allowlist (chains traverse
provenance across sessions and cannot be scoped without leaking)
- empty allowlist short-circuits to empty results everywhere
Auth: workspace keys pass the allowlist as-given; peer-scoped JWTs must
be a member of every allowlisted session (403 otherwise), mirroring the
existing single-session check.
Fixes DEV-1995
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
WalkthroughMessage, memory, chat, and representation retrieval now accept validated session allowlists. Observer visibility is intersected with those allowlists, while empty or unauthorized scopes return no results. Peer routes validate filters and propagate the resulting scope through agent tools and CRUD calls. ChangesSession allowlist enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PeerRoute
participant FilterParser
participant DialecticAgent
participant AgentTools
participant MessageCRUD
Client->>PeerRoute: submit session_id filters
PeerRoute->>FilterParser: extract_session_allowlist
FilterParser-->>PeerRoute: validated allowlist
PeerRoute->>DialecticAgent: pass session_names
DialecticAgent->>AgentTools: create scoped tool executor
AgentTools->>MessageCRUD: search with session_names
MessageCRUD-->>AgentTools: session-scoped results
AgentTools-->>DialecticAgent: recall context
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
adavyas
requested changes
Jul 12, 2026
| allowed_session_names = await get_peer_session_names( | ||
| db, workspace_name, observer | ||
| ) | ||
| if not session_name and (observer or allowed_sessions is not None): |
Contributor
There was a problem hiding this comment.
when session_name is set, this only scopes to that one session itself and never applies allowed_sessions. rest of the allowlist is dropped ignored
| @@ -298,7 +333,7 @@ async def get_representation( | |||
| observed=options.target if options.target is not None else peer_id, | |||
| session_names=[options.session_id] | |||
| if options.session_id is not None | |||
Contributor
There was a problem hiding this comment.
i think the allowlist is ignored here as well
| after_date: datetime | None = None, | ||
| before_date: datetime | None = None, | ||
| observer: str | None = None, | ||
| allowed_sessions: list[str] | None = None, |
Contributor
There was a problem hiding this comment.
nit: allowed_sessions was called session_names in other parts of the codebase and in PR #881 should probably be consistent.
Empty session allowlists relied solely on the early-return guard in _get_working_representation_internal. The layers below it were inconsistent, so a future direct caller (the DEV-1995 allowlist API) would silently widen scope instead of failing closed: - _build_filter_conditions used a truthiness check; an empty list was treated like None and dropped the filter. Now uses `is not None`, matching the recent/most-derived SQL paths. - turbopuffer emitted a bare `In []` with undocumented (possibly fail-open) semantics. Now emits an explicit always-false predicate, mirroring lancedb's `1 = 0`. Also extract the duplicated JSONB column tuple in filter.py to a JSONB_COLUMNS constant. Tests exercise each fail-closed guarantee at the layer it lives, rather than masking it behind the early-return guard.
VVoruganti
requested review from
Rajat-Ahuja1997,
akattelu and
eisene
as code owners
July 24, 2026 14:42
search/grep/history helpers scoped to a single session_name ignored the session_names allowlist entirely — a caller could read a session the allowlist forbids. The API routes guarded this with a 422, but the dialectic tools call these CRUD functions directly and bypassed it. Enforce it at the boundary: return [] when session_name is set and not in the allowlist, across _semantic_search_messages (covers search_messages + search_messages_temporal), grep_messages, get_messages_by_date_range, get_recent_history, and get_observation_context. Also rename the public param allowed_sessions -> session_names for consistency with representation.py / chat.py / peers.py; the resolved intersection keeps its distinct name allowed_session_names.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/agent_tools.py (1)
1033-1050: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the new
session_namesparameter in the Args section.
get_recent_history(and similarlysearch_memory,get_observation_contexthere, plus thecrud/message.pysearch entry points) gained asession_namesallowlist arg, but the docstringArgs:blocks still omit it. Adding a one-line entry keeps these Google-style docstrings complete and consistent.As per path instructions: "In Python code, use Google-style docstrings."
🤖 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/utils/agent_tools.py` around lines 1033 - 1050, Add a Google-style Args entry for the session_names allowlist parameter to get_recent_history and the corresponding search_memory, get_observation_context, and crud/message.py search entry points. Describe it as the optional list of session identifiers used to restrict results, keeping the existing docstring structure unchanged.Source: Path instructions
🤖 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/utils/agent_tools.py`:
- Around line 1033-1050: Add a Google-style Args entry for the session_names
allowlist parameter to get_recent_history and the corresponding search_memory,
get_observation_context, and crud/message.py search entry points. Describe it as
the optional list of session identifiers used to restrict results, keeping the
existing docstring structure unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ddd80914-9eee-405e-ae1a-5de8da6218a0
📒 Files selected for processing (9)
src/crud/message.pysrc/dialectic/chat.pysrc/dialectic/core.pysrc/routers/peers.pysrc/schemas/api.pysrc/utils/agent_tools.pysrc/utils/filter.pytests/test_session_allowlist.pytests/utils/test_agent_tools.py
akattelu
reviewed
Jul 24, 2026
akattelu
left a comment
akattelu
left a comment
Collaborator
There was a problem hiding this comment.
The peer card fetched during these queries still could contain information from sessions that aren't limited to session_names right? I think that's ok but just confirming that's the intent.
Generally think we should add more documentation because there's some nuance with how this works. Not sure where https://honcho.dev/docs/v3/api-reference/endpoint/peers/chat is populated from but it should be updated.
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
3 participants
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.
Stacked on #881 — merge that first, then retarget/merge this.
Summary
Implements the Scopes RFC Phase 1 (DEV-1995):
peer.chatand/representationaccept a constrainedfiltersbody — the same DSL search and conclusions already accept — supporting only thesession_idkey (a session id, a bare list, or{"in": [...]}). This is verbatim sauna.ai's ask: sessions_ask-style behavior across only the dynamically allowed session set.Semantics (fail-closed everywhere)
search_memory): restricted to the allowlist; dream-produced docs (session_name IS NULL) excluded.get_reasoning_chain: unavailable under an allowlist — chains traverse provenance across sessions and can't be scoped without exposing out-of-scope premises/conclusions.session_id(must be in the allowlist when both given). Cap: 1,000 sessions/request.Auth
Workspace keys pass the allowlist as-given (trusted caller). Peer-scoped JWTs must be a member of every allowlisted session (403 otherwise) — mirrors the existing single-
session_idmembership check, same query, one round trip.Deliberately out of scope (follow-ups)
session.contextfilters (single-sessionlimit_to_sessionalready covered by fix: apply session scoping to all working-representation query paths #881; the Phase 2scopeparam is the right vehicle there)metadatafilter key on dialectic (deferred until conclusions have public metadata via conclusion-tagging)sessions: [...]compiling tofilters(DEV-1996)Tests
New
tests/test_session_allowlist.py(20 tests): parser shapes/cap/dedupe/422s, search_memory filter pushdown + empty-allowlist short-circuit, message-crud intersection incl. non-member allowlisted session excluded, chat route validation (422s), representation route end-to-end scoping (out-of-scope and sessionless docs excluded). Full affected suites green; the 3 pre-existingtest_document.pydedup failures reproduce identically on the base branch (embedding key issue, DEV-1935) and are unrelated.Linear: DEV-1995 (part of DEV-1970 / Scopes RFC)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security
Documentation
Tests