* chore(docs): Add section about harness integrations and deepseek harness to docs
* chore: Add section about harness integrations and deepseek harness to docs
* fix(deriver): eliminate create_documents deadlock and stop silently burning batches on transient errors
Two concurrent work units writing the same (workspace, observer, observed)
collection deadlocked on times_derived reinforcement UPDATEs issued in
batch order (DEV-1975, 682 events in 90 days). The deadlock was swallowed
per-document, the loop cascaded PendingRollbackErrors against the dead
session, the whole batch was lost, and the queue item was marked processed.
- serialize writers per collection with a transaction-scoped advisory lock
(pg_advisory_xact_lock + SET LOCAL lock_timeout), skipped for insert-only
batches; covers all three row-lock sites in one move
- hoist external-vector-store dup-candidate resolution ahead of the first
DB statement so the lock's critical section contains no network calls
- abort the batch on SQLAlchemyError instead of continuing through an
aborted transaction; per-document skip semantics kept for non-DB errors
- classify transient errors (new src/utils/retryable_errors.py) and retry
them via a bounded in-process counter instead of marking items errored
* fix(deriver): replace create_documents advisory lock with id-ordered row locks
Advisory locks are database-scoped and would serialize every writer to a
collection, including across Groudon tenants that share names. Collect
reinforcement and replace ops during the loop, lock target rows with
SELECT ... ORDER BY id FOR UPDATE, then apply. populate_existing reloads
times_derived so a prefetched identity-map row cannot lose a concurrent
increment.
* fix(deriver): harden create_documents candidate hoist and test isolation
Skip empty embeddings on the external-store path, isolate per-document
resolve failures, and keep replacement times_derived in the in-batch
ledger. Patch get_external_vector_store in the hoist test and cover
in-loop SQLAlchemyError abort.
* fix(deriver): address CodeRabbit findings on create_documents deadlock fix
- Distinguish external resolve failure ([] skip) from pgvector fallback (None)
so _semantic_dup_decision never re-enters external I/O under an open session
- Bound external candidate hoist concurrency with a semaphore
- Map in-loop IntegrityError to ValidationException for a uniform contract
- Persist transient retry attempts on the oldest unprocessed queue item so
every deriver instance shares one MAX_RETRYABLE_ATTEMPTS budget
- Cover resolve-failure skip and multi-manager reclaim of the retry budget
* fix(deriver): harden retry metadata cleanup and stale reinforce fallback
- Strip _retry_attempts from payloads in the same transaction as
mark_queue_items_as_processed / mark_queue_item_as_errored
- Clear shared retry metadata only after a successful terminal mark
- On reinforce, if the locked target is gone or soft-deleted, insert the
incoming document instead of dropping it
- Skip pgvector semantic lookup when embedding is empty so query_documents
cannot embed under an open session
* fix(deriver): address review on deadlock retry and row-lock apply
Strip _retry_attempts before payload validation so non-representation
tasks are not burned as extra_forbidden. Re-raise retryable observer
save errors after telemetry so the queue actually retries. Skip
same-batch reinforce fallbacks after a replace. Revert unordered
FOR UPDATE on mark processed/errored and drop post-commit retry
cleanup from the success path.
* fix: add test and simplify queue query
---------
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
* fix(deriver): chunk scope backfill so large sessions don't OOM the worker
_run_backfill embedded, wrote, and synced every planned copy at once, holding
one Python float list per document. A 14k-document session is ~580MB of
vectors alone, and several backfills run concurrently, which OOM-killed the
deriver at its 1000Mi limit and crash-looped it since the work units never
completed. Phases 2-4 now run per chunk of 500 specs and drop each chunk's
embeddings once synced.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(deriver): hydrate backfill embeddings per chunk
Phase 1 no longer materializes every source embedding into plans.
load_only skips the vector column on the plan queries, and each chunk
reloads only its source embeddings before embed/write/sync.
* fix(deriver): lock scope membership across backfill chunk writes
SELECT ... FOR UPDATE on the active SessionPeer row so a concurrent
leave cannot commit between the membership check and the copy inserts.
Adds a concurrency test that asserts the leave blocks until commit.
* fix: add test for memory bound
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
* fix(deriver): strip NUL bytes from model-generated observations
Postgres rejects NUL (0x00) in text columns and in jsonb strings. API
ingress has always stripped it from user-supplied content, but the
deriver's own output did not go through any equivalent: a model can emit
a \u0000 escape in its tool-call arguments, which the JSON parser decodes
into a real NUL byte. Seen in production when models transcribe shell
output (`tr '\x00' '\n'`) or Windows paths (`c:\<NUL>users\amal`).
The NUL reached the exact-content dedup pre-fetch in create_documents as
a bind parameter, so the query raised DataError before any row was
written and the whole batch for that observer was dropped.
Strip in _normalized_observation and _normalized_observation_input --
the points that already normalize text for persistence and embedding --
so the embedded text matches the stored text. premises and sources are
covered too, since they ride along in internal_metadata. The emptiness
check now runs after normalization, because str.strip() does not remove
NUL and all-NUL content would otherwise be stored as an empty string.
DocumentCreate.content gets a mode="before" validator as a backstop for
callers that bypass those paths; running before the length constraint
makes all-NUL content fail min_length rather than silently empty out.
The NUL helpers move out of schemas/api.py into utils/sanitization.py as
a single recursive strip_nul, so ingress and internal paths share one
implementation. It is overloaded to keep str -> str for the callers that
chain .strip(), and passes None through so optional fields need no guard.
Fixes HONCHO-4XZ
* fix: broaden nul strip check
* chore: code simplification
---------
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
* chore(docs): Add Documentation for Scopes
* chore(docs): add scopes to API reference, architecture, and design patterns
- Add the seven /scopes routes and their schemas to openapi.json, plus the
scope/kind fields on chat, representation, session-create, and peer-list
schemas; generate the endpoint pages and register a scopes nav group
- Add a Scopes subsection and diagram node to the architecture data model
- Add scope guidance to design patterns: quick-reference rows, an isolation
boundary comparison (workspace / scope / session allowlist), and common
mistakes (scope-per-reader, scopes-as-access-control)
- Replace the "Underneath the Facade" section in scopes.mdx with behavioral
guardrails and pointers to the implementation source
* chore(docs): tighten scopes doc to decision-level detail
- Drop the recall-resolution diagram (restated the Two Arms table)
- Replace the enumerated Rules table with prose; caps and error shapes
now live in the API reference schema descriptions
- Trim backfill/removal internals to observable behavior and note that a
backfilled scope deepens through subsequent dreams
* chore(docs): reserve "scope" for the scopes feature
Using it as a verb for session design, recall filters, and CLI targeting
collides with the named-session-set feature.
* chore(docs): clarify the scopes page and document create/status responses
The page now leads with projection rather than partition and points at the
scopes API; OpenAPI declares the 201/409/404 those routes actually return.
* chore(docs): fix broken anchor and core-concepts link
The rebase reintroduced a link to a renamed anchor in scopes.mdx, and
unified-memory-setup pointed at /core-concepts/, which has no index page.
* chore(docs): correct scope arms, listing, and read-surface pointers
The Accepts row mixed named-scope with the allowlist arm, kind=scope on
the peers list does not return facade ids, and chat/context/search never
mentioned scope=.
* chore(docs): drop the 1k-token session batching narrative
Reasoning no longer waits on a per-session token threshold, so product
docs should not tell people to size sessions around that gate.
* chore: minor fix
If consent is withdrawn while array.js downloads, sync() runs before window.posthog exists and the opt-out is skipped. onload now re-checks granted() and resets loaded so a later re-grant retries.
Mintlify swaps pages without a reload, so capture_pageview: 'history_change' records navigation past the landing page.
* fix: stop top_k=0 from reaching Turbopuffer on message search
HONCHO-19Q: dreamer search_messages passed LLM limit=0 through to
Turbopuffer (top_k must be 1..10000). #970 guarded documents; this
closes the message path and floors tool limits at 1.
* fix: preserve pgvector None sentinel on zero top_k
query_external_vector_document_ids must return None when on the
pgvector path before applying the top_k<=0 empty-list guard.
The gate job only runs on `pull_request: labeled`, so it is skipped on
push. A skipped ancestor propagates down the needs chain unless a job
opts out, which `unified-tests` never did — so the suite has been
skipped on every merge to main while still burning a Fly machine.
* feat(cli): fix API key typing
Co-authored-by: Cursor <cursoragent@cursor.com>
* chore: bump honcho-cli to 0.1.4
Ship the masked --setup API key prompt plus the openai-compatible embedding base URL already on main.
Co-authored-by: Cursor <cursoragent@cursor.com>
* feat(cli): check for newer version
* docs: nit
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(llm): preserve null content on OpenAI tool-call turns
OpenAI-compatible providers can return assistant tool-call messages with
content=null. Coercing that to "" before history replay breaks providers
that bind opaque reasoning state to the exact assistant message shape.
Keep null only when the normalized response has tool calls; tool-less
null still becomes "", and content_override stays authoritative.
Fixes#1061
* test(live_llm): cover OpenAI null content tool-call replay
Add a live multi-turn tool replay that asserts provider content=null stays
null through normalize + OpenAIHistoryAdapter and that the continuation
still answers. Mark gpt_4/gpt_5 families as supports_tool_replay.
* docs(llm): note content_override None sentinel semantics
None means no override, not force-null content. Addresses review on #1064.
author_association on the webhook is CONTRIBUTOR when org membership is
private, so maintainers with write (e.g. ajspig) were labelled
needs-approved-issue. Skip on admin/maintain/write from
getCollaboratorPermissionLevel instead; 404 stays gated.
* docs: make honcho start the documented local path
honcho-cli 0.1.3 can run a personal stack without cloning the repo; point the README, self-hosting, and CLI reference at that, and drop the community installer callouts.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs: updating with new CLI language
* docs: drop compatibility-guide changes from this PR
Leave that file on main; CLI version cards are updated at release time.
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs: small language changes
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Review findings on #1071. The cookie match now requires the exact
CookieConsent name boundary. A failed array.js request resets the
loaded flag so later consent events retry. And consent events now run
a full sync: withdrawal opts an already running instance out, and a
re-grant opts it back in — same behavior as the landing site's gate.
The runtime stage copies application code but not pyproject.toml, so
src/_version.py cannot find the file it reads the version from. The
image also installs dependencies with --no-install-project, so there is
no honcho distribution for the importlib.metadata fallback to find.
Both lookups fail, so the service falls back to reporting its version as
"unknown" in the OpenAPI schema and in telemetry events.
Copying the file into the runtime stage restores an accurate version.
The file is under 4 KB, so the image size is unchanged.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
DEV-2465 open question 5, option d. Mintlify's built-in integration
loaded PostHog unconditionally on all 249 docs pages — a visitor who
declined on the homepage was tracked one click later in the docs. The
integration key comes out of docs.json; docs/posthog-consent.js loads
PostHog directly instead, only when the CookieConsent cookie grants
Statistics (or holds Cookiebot's -1 marker), and listens for the
consent events so a grant on the docs banner itself loads it too.
Trade recorded on the ticket: this bypasses the ph.mintlify.com proxy,
so ad blockers reduce docs PostHog volume. Verify after deploy that
Mintlify's page CSP allows us-assets.i.posthog.com; if it blocks,
fall back to option c.
DEV-2465 step 1. Mintlify injects gtm.js on all docs pages; the
container is audited to be inert on /docs before this merges, so the
snippet loads and nothing fires. Cookiebot and GA4 arrive later as
container publishes, consent first.
Merging this publishes the docs within minutes, so it stays unmerged
until Marc confirms the container audit.
`get_observation_context` resolved scope by fetching every session name the
observer has a membership record in, then expanding that list into
`session_name IN (...)` twice in one statement — once in the CTE and once in
the outer select. That puts psycopg's 65535-bind-parameter ceiling at roughly
32,765 sessions, and the count only ever grows: the loose membership
definition (`active_only=False`) counts sessions the peer has since left, so
leaving a session does not shrink the scope. A workspace with tens of
thousands of sessions for one peer produced a statement the driver could not
serialize at all.
Two new helpers in `crud.message` express the observer half as a correlated
EXISTS over `session_peers`. Scope now costs two bind parameters regardless of
membership size, and the membership query disappears (two round trips become
one). The `session_peers` primary key is `(workspace_name, session_name,
peer_name)`, so the correlated probe is an exact-match index hit.
The caller-supplied allowlist stays an IN clause — it is route-capped at 1000
entries and carries none of the unbounded-growth risk. `resolve_session_scope`
is left in place: three other callers still need the materialized list,
including `_search_messages_external`, which sends session names to the vector
store as a filter payload and cannot take SQL.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
On Redis Cluster a key's slot comes from the substring inside the first
{...}, when one is present. Untagged, one deployment's keys spread over
every slot, so its client opens and holds a connection to every node in
the cluster. Wrapping the namespace in braces puts them all on one slot,
and therefore one node, cutting each deployment's connection count to
the cluster by a factor of the shard count. Namespaces still hash
independently of each other, so keys stay spread across the cluster and
no shard becomes a hotspot.
The tag needs two spellings, because the two ways a key gets built treat
the string differently. cashews runs `prefix=` through format
substitution, so braces have to be doubled there to survive as literals;
keys built by concatenation need them single. A single brace passed to
cashews is read as an empty substitution field and the namespace is
dropped entirely, which would let two deployments collide on one key --
hence two clearly named helpers rather than one string, and a test that
the two paths produce identical bytes.
No key format change for a non-cluster backend, and no migration: the
old keys simply age out by TTL.
* feat(cli): add honcho start/stop/status for a local Docker stack
* feat(cli): fix status command
* feat(cli): improving how we pull docker images and writing a config,toml
* feat(cli): add honcho start --setup wizard for local stack config
* feat(cli): cleaning up unnecessary func, and error throwing
* feat(cli): minor clean up in stack.py
* feat(cli): read setup wizard defaults from the image config.toml
* feat(cli): cleaning up unused commands
* feat(cli): adding ignored docker-compose.yml
* feat(cli): forward host LLM env into honcho start
* feat(cli): share start/stop progress helpers via output.py and cleaning up language
Teach both agents what Honcho, peers, and the harness are instead of comparing them to each other. Render only the tools the request actually offers, and drop the pair prompt's call to a write tool that is not in the loadout.
* fix(crud): preserve joined_at for active session peers
Re-adding an already-active peer no longer advances the membership
window, so peer_perspective search keeps messages from the original
join. Genuine rejoins still start a new window.
* docs: document set_peers membership window and wrap test docstrings
* fix: preserve session observer limit
---------
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
Callers wrapped every ValueError from the embedding client in a
"exceeds maximum token limit" message, so provider and configuration
failures (dimension mismatch, empty response, upstream error) surfaced
to users as though their input were too long.
Add EmbeddingTokenLimitError, raised only by the pre-flight token checks
in embed() and simple_batch_embed(), and narrow the remaps in search.py,
agent_tools.py, document.py and representation.py to catch it. It
subclasses ValueError so existing broad handlers keep working.
Both simple_batch_embed() remap sites pass on_oversize="truncate" and so
could never raise a token-limit error at all; their handlers only ever
mislabelled provider failures.
Fixes#568
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* chore: update issue templates and docs
* chore: slim bug/quality forms and add integration template
Drop high-friction required fields from bug and quality issue forms.
Add an integration-request form (app stores, plugins, frameworks) labeled
integration, and point contributing docs at it.
* chore: route security mail to support@ and polish docs intake
Use support@honcho.dev for private vulnerability email. List the
documentation template in contributing guides, rename Media prove,
and add public-issue redaction/security redirects on the docs form.
* fix: address render issue in templates and add version field
* feat(docs): Initial draft of new contributing policies
* feat(ci): defer issue-gate closes to a scheduled sweeper
Addresses review feedback on #1041.
The gate now reads GitHub's resolved closing references
(closingIssuesReferences) instead of regex-parsing the pull request body,
so an issue linked through the sidebar Development panel counts, and a
bare `#123` mention no longer does.
It also no longer closes on the pull request event. It labels and
explains; pr-sweeper.yml re-checks every six hours and closes only what is
still failing 72 hours after the notice. That re-check is load-bearing:
linking an issue via the sidebar fires no webhook, so an event-only gate
could never observe a contributor complying that way. The sweeper also
closes drafts from outside the org after 30 days.
The shared check lives in .github/scripts/issue-gate.js so both workflows
run identical logic, with a dependency-free self-check wired into static
analysis. Its one regression guard: author_association CONTRIBUTOR stays
gated, since GitHub assigns it to anyone who has previously committed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(codeowners): drop the third reviewer from most areas
Discussed with @akattelu. Also reassigns SECURITY.md to @Rajat-Ahuja1997
and strips trailing whitespace from the deployment block.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(v2): port the issue gate policy into the v2 contributing guide
The v2 guide is still published (v2.5.1 in docs.json) but carried no
mention of the issue gate, so a contributor reading it would not learn
that a pull request needs an approved issue until the bot labelled theirs.
Ports the policy, both linking routes, and the gate's place among the
automated checks, keeping the v2 guide's own structure and unwrapped
prose rather than importing the v3 rewrite wholesale.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(ci): count only bot-authored gate notices, share the exemption list
Two review findings on the issue gate, with a common root cause.
MARKER is an invisible HTML comment, so anyone who can comment on a
public repository can paste it. findNotices accepted any comment
containing it, so a third party could post one on someone else's pull
request: runGate posts a notice only when none exists, so the author
would never be told, and runSweep would then measure the 72-hour grace
window from the stranger's timestamp and close them unwarned. Notices now
require bot authorship.
The stale-draft sweep re-listed the gate's exemptions and had lost the
bot case, so a bot's long-lived draft was closable despite checkGate
exempting bots. Both callers now share one exemptReason(pr) rather than
keeping parallel lists that drift.
Not changed: closingIssuesReferences(first: 20) truncation. It needs a
pull request with 21+ closing references where only a later one carries
the label, and the outcome would be a label plus the grace window, not a
close.
Coverage goes 11 -> 20 cases, including the stale-draft close path, which
had none. Both fixes were confirmed to fail their tests when reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: update issue templates and docs
* chore: slim bug/quality forms and add integration template
Drop high-friction required fields from bug and quality issue forms.
Add an integration-request form (app stores, plugins, frameworks) labeled
integration, and point contributing docs at it.
* chore: route security mail to support@ and polish docs intake
Use support@honcho.dev for private vulnerability email. List the
documentation template in contributing guides, rename Media prove,
and add public-issue redaction/security redirects on the docs form.
* fix: address render issue in templates and add version field
---------
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
* Add workspace-level chat (DEV-1326)
POST /v3/workspaces/{workspace_id}/chat: agentic dialectic over the whole
workspace instead of a single (observer, observed) pair. Salvaged from
plastic-labs/honcho#373 and re-grown on today's DialecticAgent:
- WorkspaceDialecticAgent subclasses DialecticAgent via four new seams
(_get_tools, _create_tool_executor, _prefetch_intro, _trace_name) instead
of a base-class extraction; observer/observed use empty-string sentinels.
- Routing-accelerated prefetch: workspace stats + top-5 active peers with
their self peer-cards (pure DB, ~7ms measured) so routing-obvious queries
resolve without a discovery tool round.
- Observation search stays pair-scoped (matches per-pair vector namespaces;
avoids workspace-flat top-k dilution): search_memory/get_peer_card take
observer/observed as tool arguments, with pair attribution in results.
- workspace_chat / workspace_chat_stream orchestrators, WorkspaceChatOptions
schema (scope param seam left for the #897 scopes facade), SSE streaming,
structured output via response_format.
- crud: get_workspace_stats, get_active_peers; format_documents_with_attribution.
- SDKs: Python Honcho.chat/chat_stream + HonchoAio mirrors; TypeScript
honcho.chat/chatStream.
- 46 tests (route, orchestrator preflight, tool handlers, executor routing,
attribution formatting) + unified test cases + docs.
Co-Authored-By: doria <93405247+dr-frmr@users.noreply.github.com>
Co-Authored-By: Benjamin McCormick <docterformer@protonmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: type SSE stream wrapper as AsyncIterator (basedpyright)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: silence unused db_session fixture warnings (basedpyright failOnWarnings)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: drop docs changes from this PR (defer to follow-up)
Restores docs/v3/documentation/features/chat.mdx to main's version. This
also puts back the peer-chat Structured Outputs section (#896) that the
workspace-chat commit removed as a rebase artifact.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: workspace message tools deny-all under rebased session scoping
The #882 rebase changed the unscoped-observer contract from falsy to
'observer is None': resolve_session_scope looked up the workspace
executor's observer='' sentinel as a real peer with no session
memberships and denied every workspace-flat message read (search, grep,
date-range, temporal, observation context) whenever no session was
pinned — the primary workspace-chat shape. Normalize the sentinel to
None at the five read-handler crud boundaries and add regression tests
that run the tools unpinned (verified to fail without the fix).
Also from review:
- wrap the workspace prefetch in the same degrade-to-None protection
the base agent has (an overview query error no longer 500s the
request or kills the SSE stream after headers)
- thread session_allowlist through create_workspace_tool_executor so
the agent-level allowlist seam is honored end to end when scopes
(#897) wire it up; allowlisted grep is covered by a test
- deterministic name tie-break in get_active_peers ordering
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: SDK response_format parity, shared query sanitizer, annotations
- TS SDK: WorkspaceChatParams gains response_format; _workspaceChat/
_workspaceChatStream consume the shared interface instead of inline
duplicates; chat/chatStream expose responseFormat.
- Consolidate the three identical sanitize_query validators into one
NulStripped annotation.
- workspace_chat_stream: return annotation + full docstring.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: fold active peers into workspace stats; trace + query bounds
- Merge get_active_peers into get_workspace_stats (one discovery round
instead of two); minimal loadout keeps a discovery tool via the merged
stats tool. Fixed top-10 by recent activity; deeper discovery routes
through search_messages.
- get_active_peers CRUD now aggregates over a trailing 90-day window so
the chat-path prefetch never scans a workspace's full message history.
- Workspace agent inherits the "dialectic_chat" trace name; scope stays
distinguished by agent_type/track_name (workspace name was already in
telemetry context).
- Prefetch failure logs carry workspace + traceback; prompt no longer
contrasts against a peer-level agent the model has no concept of;
drop ticket identifiers from comments.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add `scope` to workspace chat and exclude scope peers from stats
Workspace chat is peer-unanchored, so `scope` is always a session-union
allowlist (single name or list), fail-closed when empty. Stats and
active-peer prefetch drop scope-kind peers and honor the same allowlist.
* test: teach the unified runner `workspace_chat` and parse every case
QueryAction now accepts target=workspace_chat (SDK path, including
scope). A pytest over tests/unified/test_cases/*.json keeps the four
existing workspace-chat cases — and a new scoped one — from rotting
against the schema again.
* docs: tighten workspace-chat scope docs and judge prompt
Scoped workspace_chat uses the SDK, not raw HTTP. The scope fixture's
judge now requires the in-scope tea fact, not merely the absence of the
leak. format_sse_stream matches the peer-chat one-liner.
* fix(dialectic): restore the empty-memory fallback for workspace chat
`search_memory` auto-searches messages when a pair has no observations,
but the gate only admitted `agent_type == "dialectic"`. The workspace
executor passes `workspace_dialectic`, so workspace chat got a bare
"No observations found" and answered that it knew nothing rather than
falling through to message search.
Also fixes the two unified cases that never ran: `deriver` is not a
field on `WorkspaceConfiguration`, so both aborted at load with
`extra_forbidden`. `workspace_chat_scope` additionally enables reasoning,
since it asserts scope isolation and has no reason to depend on the
fallback path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(tests/unified): fail CI when unified tests fail
`runner.run()` tallied failures into `failed_count` and printed them, but
returned nothing, and both entrypoints ignored the result. The workflow
invokes `python -m tests.unified.run` bare, so the job has gone green on
failing and unrunnable cases since it was wired up in #291.
Return the count and exit non-zero on it. `INVALID SCHEMA` already counts
toward the tally, so a malformed case now fails the job instead of being
skipped silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(unified): assert scope peers stay out of workspace chat answers
Scope peers are real peer rows, so a regression in the `scope_peer_clause`
exclusion would surface `scope.therapy` through workspace stats or the
routing prefetch. Nothing asserted against that.
Adds the check to the existing scoped query and a new unscoped one, since
the two exercise different `get_active_peers` branches. Verified by
removing the exclusion, which fails the unscoped query.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: Remove dead code references
---------
Co-authored-by: doria <93405247+dr-frmr@users.noreply.github.com>
Co-authored-by: Benjamin McCormick <docterformer@protonmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
Bug (verified vs SQLAlchemy 2.0.49): on GC-cleanup of an abandoned async connection, _finalize_fairy routes through fairy.detach(), which nulls the record's dbapi_connection so NullPool's close is a no-op (the `close` event never fires) then emits `detach` with the record. Listening only to close/invalidate left the marker unpopped, so db_connections_open leaked upward and never reset until restart. Listen to `detach` too — it carries the ConnectionRecord and the marker dedupes, so exactly one decrement occurs.
Also: strip a bare PR-number provenance tag from the test docstring (plastic-labs comment-reconciliation rule); disambiguate db_connections_open from the existing db_pool_connections; note in initialize_bounded_metrics that DB-instrumentation metrics zero-init in their registrar.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add db_connections_open (gauge) and db_connections_established (counter), driven by SQLAlchemy connection-lifecycle events so they report real connections under every pool class — including NullPool, where the pool-object collector (db_pool_connections) reads zero. Under NullPool the establishment rate approximates request rate.
DBConnectionTracker mirrors DBQueryInflightTracker: a ConnectionRecord.info marker makes each physical connection increment once and decrement at most once (no leak, no negative). Registered per-process in the API and deriver; zero-init via the pre-resolved labeled children, matching db_queries_in_flight_gauge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(filter): make ne on jsonb metadata keys null-safe
* test(filter): cover null-safe ne on nested metadata keys
* test(filter): count actual rows for nested-metadata ne null-safety
Compile-only checks lock the operator map entry but don't catch wrong
row sets under three-valued logic. Adds a live messages/list case with
a message missing the key and one with empty metadata, following the
scalar-column pattern in test_negation_includes_conclusions_with_no_session.
* test(filter): type message_configs with a TypedDict
basedpyright couldn't narrow the heterogeneous metadata dict literals,
so indexing message_configs["content"] came back partially unknown and
broke the sorted() calls under type checking.
* fix(deriver): truncate oversize observations so one cannot drop the batch
simple_batch_embed raised ValueError when any input exceeded the per-input
token cap, which failed the entire deriver save when a single observation
was over-length. Add on_oversize="truncate": oversize inputs are embedded
from a token-capped prefix (re-encoded until it fits, with a warning),
preserving one vector per input. Default stays "raise" so existing callers
are unchanged. RepresentationManager opts into truncate.
Also add a live embedding test that fails on main (raise / missing kwarg)
and passes once a mixed short+oversize batch survives.
Refs #569
* fix(deriver): surface failure when all observer saves fail
When every observer's save_representation failed (e.g. embedding retries
exhausted under a sustained 429), the deriver logged the error and returned
normally, so the queue marked the work unit processed with zero documents
saved. Collect per-observer errors and, after telemetry is emitted, raise
RepresentationSaveError when no observer succeeded. Partial failures stay
processed (saved observers must not be discarded) and are recorded via an
additive failed_observer_count on RepresentationCompletedEvent.
Refs #728
* fix(embedding): guarantee truncation progress and truncate on re-embed
The retry slice in _truncate_to_token_limit always recomputed the same
keep count, so a slice whose re-encode grew past the cap could oscillate.
Decrement keep after each unsuccessful retry.
Document re-embed in the reconciler used the default on_oversize="raise",
so one oversize document failed every other document in the batch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore: drop ticket ids and shrink comments to one sentence
Comments and docstrings describe current behavior, not the PR that
introduced them. Ticket numbers stay in the commit/PR.
* chore: annotate RepresentationSaveError and assert truncate on re-embed
* fix(embedding): truncate on conclusion create paths and document BPE loop
Storage callers in create_observations (API + agent tools) now pass
on_oversize="truncate" so a single oversize item cannot drop the batch.
Docstring on _truncate_to_token_limit notes why decode/re-encode is load-bearing.
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* telemetry: materialize dropped-event counter children at 0
A labeled Prometheus counter exports no series until its first labels()
call, so telemetry_events_dropped stayed invisible until an event was
actually dropped — impossible to alert on or graph, and "no drops" was
indistinguishable from "metric missing / scrape broken".
Pre-create the (namespace, reason) children at 0 on emitter start, for
each reason the emitter can emit, so the metric is always present.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* telemetry: generalize counter zero-init to all bounded-label counters
Extends #927 (which zero-inited telemetry_events_dropped) to every counter
whose label domain is bounded and known at startup, so metrics are present in
Prometheus before their first event — a missing series then signals a broken
scrape rather than "nothing happened yet".
- add initialize_bounded_metrics(instance_type) on PrometheusMetrics; call it
per-process from main.py (api) and deriver/__main__.py (deriver).
- extract a shared _touch() helper; refactor initialize_telemetry_dropped_metrics
onto it (that one stays per-emitter in start() — it's prefix-dependent).
- explicit ALL_EVENT_TYPES / HIGH_VOLUME_EVENT_TYPES registry in telemetry.events,
drift-guarded by tests that walk BaseEvent subclasses.
- only VALID (task_type, token_type, component) tuples for deriver_tokens (the
cartesian product would fabricate impossible always-0 series); only high-volume
event types for sampled_out; high-cardinality labels (endpoint, workspace_name)
left open.
- gauges: zero-init embed_now_tasks_in_flight + telemetry_buffer_size; add a new
message_embeddings_pending backlog gauge, set each reconciliation cycle and
zero-inited at deriver startup (Rajat's pending/in-flight ask).
- backfills the tests #927 shipped without.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: task-aware deriver combos + fail-soft gauge zero-init
I1: _DERIVER_TOKEN_COMBOS was factored task-independently, materializing the
impossible (ingestion, input, previous_summary) series — previous_summary is
summary-only. Make combos task-aware (_DERIVER_TOKEN_COMBOS_BY_TASK) so no
always-0 impossible series is fabricated, matching the PR's own goal. Tests
tightened to assert the ingestion/previous_summary series is absent.
I2: the three gauge .set(0) zero-inits were bare while the counter inits go
through the fail-soft _touch. Add _set_gauge_zero() so a gauge init can't
propagate an exception into process startup either.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(telemetry): isolate zero-init namespaces, add deriver-to-api guard
Global-REGISTRY assertions used a fixed "test" namespace, which several
other suites also pin, so another test's materialized children could
satisfy a presence assertion or break an absence one. Each test now runs
under a unique namespace resolved from settings at read time.
Adds the inverse per-process isolation test: deriver-only init must not
materialize API-only series (dialectic tokens, embed_now).
Co-Authored-By: Claude <noreply@anthropic.com>
* review: per-replica backlog gauge, drop duplicated constants and .meta refs
Addresses Vineeth's review on #927.
Blocking:
- message_embeddings_pending is a DB-global count, so drive it from
ReconcilerScheduler._scheduler_loop (runs on every replica, every
interval) instead of run_vector_reconciliation_cycle (runs off the
queue behind work-unit dedup, so one replica per cycle). Combined with
the zero-init, the old placement made every replica that never won the
work unit export a confident permanent 0. Help string now names the
owner so dashboards don't reach for sum().
- guard initialize_telemetry_dropped_metrics on METRICS.ENABLED,
matching its sibling initializer.
- drop the duplicate REASONING_LEVELS; import the one in src/config.
Non-blocking:
- walk BaseSpecialist recursively via a shared utils.types.walk_subclasses
(replaces the direct-children-only __subclasses__() and the test's
private copy of the same helper).
- derive the specialist assertion from the subclasses instead of
hardcoding two names — the hardcoded pair kept passing after
CardRefreshSpecialist landed, leaving it uncovered.
- inline the zero-init rationale and the multi-instance bucket taxonomy;
removes both pointers to a .meta design doc that is not in the repo.
Tests: new tests/reconciler/test_pending_backlog_gauge.py pins both
halves of the relocation (verified it fails when reverted).
* review: fix inert test guard, stale comments, and the REASONING_LEVELS drift claim
Second review pass on the branch. Findings, most severe first:
- tests/reconciler/test_pending_backlog_gauge.py: the _try_enqueue_task stub
was patched onto the class but declared without `self`, so calling it
raised TypeError — which _scheduler_loop swallows. The guard was inert and
the test passed for the wrong reason. Fixed the arity.
- metrics.py still commented that the backlog gauge is "set live each
reconciliation cycle". That is the exact claim the previous commit
overturned; it now contradicted the help string, the bucket-3 docstring
and sync_vectors.py.
- metrics.py claimed REASONING_LEVELS is "derived from the config Literal so
it never drifts", but config.py hand-listed it, so the earlier dedup had
quietly traded away the guarantee the original get_args() call provided.
Made it true instead: config.REASONING_LEVELS = list(get_args(...)), which
keeps the dedup and restores the invariant.
- dropped _set_gauge_zero: all three gauges it zeroed already have identical
fail-soft setters, so it was a second way to do one thing. Using the
setters also makes _handle_metric_error name the actual gauge.
- record_pending_embeddings_backlog's docstring oversold the covering index
as making the COUNT "negligible". The index makes cost proportional to the
pending backlog, not to the table — which is worst precisely when the
backlog matters. Stated honestly.
- _scheduler_loop's docstring said it only enqueues; it also refreshes the
gauge, at a cadence set by the shortest task interval.
- comment reconciliation: stripped #927 / "the generalization" temporal
anchoring, a CardRefreshSpecialist change-narration clause, and
reviewer-directed phrasing from the test file; disambiguated the
src/utils/summarizer.py path.
- CLAUDE.md had no Prometheus section at all, so the new "add a BaseEvent
subclass -> update ALL_EVENT_TYPES" obligation and the never-sum() rule
for non-additive gauges were undiscoverable from the architecture doc.
Verified: ruff + basedpyright clean (0 errors), tests/telemetry + reconciler
+ dialectic + llm 497 passed, full suite 1768 passed with only the 4
pre-existing test_document failures (OpenAI key required, reproduced on
clean origin/main). Re-confirmed the relocation guard fails when reverted.
* fix: silence the two basedpyright warnings inherited from main
CI runs `uv run basedpyright` bare, and basedpyright exits non-zero on any
warning — so these two have been failing the staticanalysis job on every
branch cut from current main, not just this one:
- src/vector_store/__init__.py:209 implicit string concatenation (#496)
- tests/test_cache_redaction.py:5 private import (#869)
Both predate this branch and are unrelated to the telemetry work; fixed
here only because they block this PR from going green. Verified: clean
origin/main also reports "0 errors, 2 warnings" and exits 1.
basedpyright now 0 errors, 0 warnings, exit 0.
* docs(telemetry): make the bucket-3 aggregation rule precise
The multi-instance taxonomy said a service-scoped non-additive metric has
"no aggregation correct once they disagree", then immediately mandated that
every instance refresh on its own timer. Those undercut each other: staggered
timers ALWAYS disagree slightly, so as written the rule reads as "ensure they
don't", which is unachievable, and it leaves the reader unsure whether max()
and avg() survived the fix.
The actual rule is bounded disagreement plus a scale-preserving aggregator.
Instances are N witnesses to one fact, not N parts of one whole, so sum() can
never be correct (it scales with replica count) while max()/avg()/quantiles
are correct precisely because the per-instance timer bounds the spread.
Wording only; no behavior change. The gauge help string already said
"max() or avg(), never sum()" — this makes the normative docstring agree
with it. Surfaced walking Vineeth's comment 3668208059 for comprehension.
* refactor(bench): import REASONING_LEVELS from config instead of re-listing
Third copy of the constant, missed when ee781c0/694e07f deduped the other
two. This one re-declared the ReasoningLevel Literal as well as the list,
so the type alias could diverge from config's with nothing to catch it —
and the list was hand-written, the variant that typechecks clean while
missing a member.
No import barrier justified it: this module already imports from src, as do
seven of its siblings in tests/bench. Concrete effect of the drift was that
a newly added sixth reasoning level would be rejected by the bench CLI's
argparse choices=.
src.config.REASONING_LEVELS is now the single definition repo-wide.
* test(telemetry): pin the METRICS.ENABLED guard on the per-emitter initializer
initialize_telemetry_dropped_metrics gained a METRICS.ENABLED guard in
ee781c0, addressing Vineeth's asymmetry comment, but nothing asserted it —
it had only the enabled half of the pair its sibling has. Deleting the guard
left the suite green, so the fix closed the asymmetry in the guards and
reproduced it one level up in the tests.
Mirrors test_init_noop_when_metrics_disabled. Verified live rather than
assumed: deleting the two guard lines turns this test red.
Uses a unique namespace, without which the absence assertion would be
satisfied by the enabled test's children rather than by the guard.
* docs(telemetry): fold zero-init why-prose behind # region ai markers
Comment/docstring-only pass over the changed files, per the groudon
comment-marker standard: the terse human-facing "what" stays visible, and
load-bearing "why" (the zero-init / absent-series-means-broken-scrape
rationale, gotchas, receipts) folds into # region ai / # ai: blocks.
Behavior-preserving: AST-identical modulo docstrings/comments vs the
pre-pass merge; ruff, ruff format --check, and basedpyright all clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: honor DERIVER_DEDUPLICATE in create_observations
The agent-tool path hardcoded deduplicate=True, so DERIVER_DEDUPLICATE=false
could not disable dedup for observations created through this path. Pass
settings.DERIVER.DEDUPLICATE, matching crud/representation.py.
* test: cover deduplicate setting is forwarded in create_observations
docker/entrypoint.sh runs under dash inside the container image. A Windows checkout with core.autocrlf=true rewrites it to CRLF, and dash aborts at startup because the carriage return becomes part of the -e flag argument.
* feat: scopes SDK surface and session allowlist on session context
Exposes the Scopes v1 facade in both SDKs, which until now was reachable
only by hand-rolled HTTP, and closes the Phase 1 gap where the session
allowlist never landed on the context route.
SDKs (DEV-2001, folds in DEV-1996)
New Scope class in both SDKs — addSessions / removeSession / sessions /
status — plus honcho.scope() and honcho.scopes() entry points, a
`scopes` option on session creation, and `scope` + `sessions` read
options on chat, chatStream, representation, and session.context.
`scope` on workspace search. Python covers sync and .aio equally.
`sessions` is sugar, not a new wire field: on the recall endpoints it
goes out as the constrained `filters: {session_id: [...]}` body, never
as a key of its own. Kept separate from the `filters` parameter on the
list/search methods on purpose — that one is the full filter DSL,
whereas the recall endpoints accept a single key and 422 on anything
else, so one name for two grammars would be a trap.
Server (DEV-2357)
`GET /sessions/{id}/context` accepts a `sessions` allowlist confining
the target's representation. Two deliberate choices worth review:
- Sent as a repeated query parameter rather than the `filters` body the
issue specced. The route is a GET and `session_id` is the only
supported key, so a JSON blob in a query string buys nothing.
- The peer card is omitted under an allowlist. Cards key on
(workspace, observer, observed) with no session dimension, so they
cannot be narrowed; returning one would leak exactly what the
allowlist exists to exclude. Same reasoning as ALLOWLIST_SAFE_LEVELS.
`scope` needs no carve-out — it swaps the observer to the scope peer,
so the card read is the scope's own.
`extract_session_allowlist` now delegates to a shared
`normalize_session_allowlist`, so the cap, id charset, and must_include
rule have one implementation across both entry points. Existing error
messages are unchanged.
Also in here
- ConclusionScope renamed to ConclusionsView in both SDKs. "Scope" now
means a named set of sessions, which that class is not — it is a view
over one observer/observed pair. ConclusionScope kept as a deprecated
alias; the package-level import path only.
- The TS HTTP client comma-joined array query params, so any list-valued
parameter arrived as one malformed entry. Fixed at buildURL rather
than the call site.
Not in this PR: the "How Scopes Work" docs guide and the Groudon
dashboard tab (both DEV-2001), and CHANGELOG entries for Phases 2a-2c,
which are still merged-but-unrecorded.
Verified: ruff, basedpyright, tsc --noEmit, biome all clean. New unit
tests cover the SDK option translation and the Scope client, but the
context route's own behavior — the 422s, the 401 membership gate, the
dropped peer card — has no test yet; the analogous chat/representation
cases in tests/test_session_allowlist.py are the place for it.
Refs DEV-2001, DEV-1996, DEV-2357
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Exposes the Scopes v1 facade in both SDKs, which until now was reachable
only by hand-rolled HTTP, and closes the Phase 1 gap where the session
allowlist never landed on the context route.
SDKs (DEV-2001, folds in DEV-1996)
New Scope class in both SDKs — addSessions / removeSession / sessions /
status — plus honcho.scope() and honcho.scopes() entry points, a
`scopes` option on session creation, and `scope` + `sessions` read
options on chat, chatStream, representation, and session.context.
`scope` on workspace search. Python covers sync and .aio equally.
`sessions` is sugar, not a new wire field: on the recall endpoints it
goes out as the constrained `filters: {session_id: [...]}` body, never
as a key of its own. Kept separate from the `filters` parameter on the
list/search methods on purpose — that one is the full filter DSL,
whereas the recall endpoints accept a single key and 422 on anything
else, so one name for two grammars would be a trap.
Server (DEV-2357)
`GET /sessions/{id}/context` accepts a `sessions` allowlist confining
the target's representation. Two deliberate choices worth review:
- Sent as a repeated query parameter rather than the `filters` body the
issue specced. The route is a GET and `session_id` is the only
supported key, so a JSON blob in a query string buys nothing.
- The peer card is omitted under an allowlist. Cards key on
(workspace, observer, observed) with no session dimension, so they
cannot be narrowed; returning one would leak exactly what the
allowlist exists to exclude. Same reasoning as ALLOWLIST_SAFE_LEVELS.
`scope` needs no carve-out — it swaps the observer to the scope peer,
so the card read is the scope's own.
`extract_session_allowlist` now delegates to a shared
`normalize_session_allowlist`, so the cap, id charset, and must_include
rule have one implementation across both entry points. Existing error
messages are unchanged.
Also in here
- ConclusionScope renamed to ConclusionsView in both SDKs. "Scope" now
means a named set of sessions, which that class is not — it is a view
over one observer/observed pair. ConclusionScope kept as a deprecated
alias; the package-level import path only.
- The TS HTTP client comma-joined array query params, so any list-valued
parameter arrived as one malformed entry. Fixed at buildURL rather
than the call site
* fix(scopes): close peer-card leak under limit_to_session, harden SDK inputs
Addresses review findings on the scopes work. All four were verified by
reproducing them, not by reading.
Peer card no longer leaks under any allowlist
The card was dropped when `sessions` was set but returned when
`limit_to_session=true` produced the identical allowlist, so a control
meant to fail closed was defeated by swapping one query parameter. It is
now gated on the effective allowlist, computed once and shared by the
representation call and the card read — the duplicated inline
conditional is what let the two drift apart.
`POST /peers/{id}/chat` still injects an unscoped card under an
allowlist (src/dialectic/chat.py fetches it on peer_card.use alone, with
no reference to session_allowlist). Left alone deliberately: that is a
behavior change to the shipped dialectic and
Scope validation messages survive the option union
ScopeOptionSchema is a union, and Zod collapses a failing union into one
`invalid_union` / "Invalid input" issue, burying the branch errors. Every
invalid scope on chat/representation reported "Invalid input" and told
the caller nothing — including the reserved-prefix case the check order
exists to surface. The rules are now a plain function applied after the
union resolves, so the specific message reaches the caller for bad
charset, reserved prefix, empty and over-cap lists alike.
Empty scope no longer fails open
`session.context({scope: ''})` and `honcho.search(q, {scope: ''})` used
truthiness checks, so an invalid scope was dropped and the call returned
*unscoped* results. Both now test against undefined so the value reaches
the schema.
Session IDs validated before reaching a URL path
`scope.removeSession('valid-session?typo')` addressed `valid-session`
with a stray query string: the wrong session removed, and reconciliation
run against it. Both SDKs now validate the charset first. Python's
`add_sessions` was unvalidated too — harmless in a JSON body, but
leaving one path checked and its sibling unchecked is how this recurs.
Also
- Corrected the `limit_to_session` description: it claimed "only used if
search_query is provided", but the allowlist reaches
_query_documents_recent unconditionally.
- Corrected the documented 1,000-session cap on `sessions`, which is
unreachable via repeated query params — the request line exceeds h11's
16 KB and nginx's 8 KB defaults at a few hundred entries, giving an
opaque 414/431 instead of a 422.
- Removed a dead route builder.
Tests: 7 new TypeScript cases and 2 new Python classes covering all four
findings. The TypeScript unit suite passes 146/146. The context route
itself is still unexercised — the card gate and the 401 membership check
remain verified by reading only.
Refs DEV-2001, DEV-1996, DEV-2357, DEV-2201
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(sdk): align the conclusions-view error message across both SDKs
The ConclusionScope -> ConclusionsView rename updated the identifier but
not the prose inside the thrown message, which the rename pattern
(\bConclusionScope\b) does not match. TypeScript ended up throwing
"managed by this conclusions view" while Python still threw "managed by
this conclusion scope" — the same error, different text per SDK.
Three server-backed conclusions.test.ts cases assert that message by
regex and failed under `pytest -k typescript`. The four equivalent Python
assertions were passing, because they matched Python's unchanged string —
so fixing only the TypeScript tests would have made the suite green with
the divergence still in place.
Brings Python's message, comments and docstring in line with TypeScript,
and updates the assertions in both suites. `grep -ri 'conclusion scope'`
is now empty.
Verified: 64 passed across tests/sdk_typescript/, tests/sdk/test_conclusions.py
and tests/sdk/test_scope_options.py — the last of which had never been run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(llm): forward provider_params.timeout to the OpenAI-compatible embedding client
#832 and #903 added a configurable request timeout for the LLM registry
and the Gemini embedding client respectively, but the OpenAI-compatible
embedding client (src/embedding_client.py) was never wired up. It
constructed AsyncOpenAI with no timeout at all, so a stalled socket
against a slow or contended OpenAI-compatible backend (e.g. a
self-hosted embedding model under load) wedges the deriver worker's
event loop indefinitely — the exact failure #785/#903 describe, just
via a code path #903 didn't cover.
EmbeddingModelConfig now carries provider_params through from
resolve_embedding_model_config, mirroring how resolve_model_config
already does it for ModelConfig, and the OpenAI branch of
_EmbeddingClient.__init__ extracts `timeout` via the existing
request_timeout_from_extra_params helper. Unset stays unset — no
existing behavior changes.
Reproduced and verified against a real self-hosted deployment (local
Ollama backend under load): before this fix, a single stuck embedding
call blocked all deriver queue processing for 20+ minutes with no
error logged, twice in one session.
* fix(embedding): use first-class timeout on embedding model config
provider_params is the LLM per-request escape hatch; embedding timeouts are
client-construction knobs and belong next to max_batch_size. Wire the field
for OpenAI and Gemini, omit the OpenAI kwarg when unset so the SDK default
stays, and keep Gemini's 10-minute floor when unset.
* test(embedding): live coverage for first-class embedding timeout
Exercise EmbeddingModelConfig.timeout on one representative OpenAI and
Gemini model: configured timeout lands on the SDK client, and a near-zero
timeout aborts before the provider answers.
---------
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
* fix(deriver): delimit examples and forbid example leakage
Wrap the minimal deriver EXAMPLES block in <examples> tags and add an explicit negative instruction: the examples are fabricated format illustrations only, and every conclusion must be grounded in the <messages> block. Soft mitigation for example-content leakage; real fixes (sources_indices + SFT / structural enforcement) are follow-ups.
* test(deriver): drop low-value examples-delimiter unit test
* fix: enforce explicit-document session purity in dedup/merge paths
Audit for DEV-2000 (Scopes RFC prerequisite): explicit-level documents must
stay session-pure so scope memory can be built by copying explicit documents
between collections. Two classes of violation were possible:
- Exact-content and semantic dedup in crud/document.py matched candidates
with no level or session scoping, so an explicit document could be
reinforced by — or soft-deleted in favor of — a same-content document from
a different session or a different level (silently merging cross-session
derivations into one row).
- The generic create_observations tool handler accepted level='explicit'
from agents with no message context (dreamer/dialectic), which would mint
session-less explicit documents.
Enforcement (refuse, never rewrite):
- create_documents refuses explicit documents with a null session_name
- exact dedup keys on (content, level, session-for-explicit); derived levels
keep cross-session consolidation
- is_rejected_duplicate scopes candidate search to the same level, and the
same session for explicit documents
- the create_observations tool rejects explicit-level input outside message
ingestion (deriver) context
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add card_refresh dream type for event-driven peer-card updates
Adds a lightweight dream variant (DEV-2000, Scopes RFC prerequisite) that
runs ONLY the peer-card update — for event-driven refreshes such as scope
membership changes and cold starts:
- DreamType.CARD_REFRESH alongside OMNI; dispatched by process_dream to a
new run_card_refresh_dream orchestration
- CardRefreshSpecialist: restricted to get_recent_observations,
search_memory, and update_peer_card (no observation-mutating tools), with
a low tool-iteration cap of min(6, DREAM.MAX_TOOL_ITERATIONS)
- rebuild=True mode carried in the dream payload: the existing card is NOT
injected into the prompt and the specialist rebuilds it solely from
observations present in the collection (for use after removals)
- enqueue-able via the manual enqueue_dream path (bypasses volume gates);
the work-unit key already embeds the dream type so a card refresh never
collides with a pending omni dream. POST /v3/workspaces/{id}/schedule_dream
accepts dream_type=card_refresh plus the rebuild flag
- card refreshes never advance the omni dream guard pair
(last_dream_at / last_dream_document_count)
- shared PEER CARD prompt section extracted (verbatim) from
DeductionSpecialist for reuse; CallPurpose gains dream.card_refresh
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: reserve scope__ peer namespace with kind flag and guardrails
Introduce the scope peer namespace (scope__<name>) and the authoritative
{"kind": "scope"} configuration flag, plus the server-side guardrails:
- src/utils/scopes.py: single source of truth for the prefix, kind flag,
and name helpers (scope_peer_name / is_scope_peer_name /
scope_name_from_peer / validate_no_scope_peer_names)
- reject reserved-prefix names on peer get-or-create (422)
- reject scope peers as message authors in crud.create_messages (422)
- reject scope peers as chat/representation targets (422); a scope peer
as the path-level observer is deferred to Phase 2b
- reject scope peers on the generic session-peer add/set/remove routes
and the session-create peers mapping (422, directing to scopes routes)
- peers.list excludes scope peers by default; new PeerGet.kind option
("scope" | "all") switches the view via a configuration JSONB filter
- schemas: Scope / ScopeCreate / ScopeSessions(Add) and
SessionCreate.scopes (unprefixed scope names, validated)
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add scopes CRUD routes and session-create scopes wiring
New /v3/workspaces/{workspace_id}/scopes facade (workspace-level auth;
peer- and session-scoped keys are rejected):
POST "" create-or-get (201/200)
POST /list paginated scope list
GET /{scope_id} single scope
POST /{scope_id}/sessions add memberships
DELETE /{scope_id}/sessions/{session_id} remove membership
GET /{scope_id}/sessions list member session ids
- crud/scope.py: get_or_create_scopes stamps the backing peer with
{"kind": "scope", "observe_me": false} and refuses to adopt a
legacy peer occupying the reserved name without the flag (409)
- memberships are session_peers rows with observe_others=true /
observe_me=false — identical to a hand-built observer peer
- SessionCreate.scopes: create-or-get each scope peer and add the
membership at session creation (the no-backfill common path)
- crud/session.py: public upsert_session_peers wrapper so the facade
bypasses the route-level guardrails without reaching into privates
Backfill of pre-existing documents and reconciliation on removal land in
DEV-1999; membership only affects messages ingested after the change.
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: cover scopes facade, guardrails, and observer semantics
- create-or-get idempotency, list/get, name validation, legacy-collision
rejection (409), auth scoping (workspace key ok, peer/session keys 401)
- reserved prefix rejected on peer create; peers.list kind filtering
- scope peers rejected as message authors, chat/representation targets,
and on the generic session-peer routes
- membership add/list/remove with observe_others=true / observe_me=false
row shape asserted via DB, and facade-less equivalence with a
hand-built observer peer
- end-to-end litmus: after adding a session to a scope, the deriver
enqueue fan-out includes the scope peer as an observer
- session creation with scopes: [a, b] creates both memberships
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: scope backfill-by-copy and removal reconciliation jobs
Retroactive scope membership changes (DEV-1999):
- New queue task types scope_backfill / scope_removal with payloads,
work-unit keys ({task}:{workspace}:{scope_peer}:{session}), deduped
enqueue (mirrors enqueue_dream), and consumer dispatch.
- Backfill copies a session's explicit documents from each sender's
global (P, P) collection into the scope's (scope_peer, P) collection
with internal_metadata.copied_from as the idempotency marker;
soft-deleted copies from an earlier removal are restored, so
add -> remove -> re-add converges on exactly one live copy. Completion
enqueues one manual omni dream per touched collection.
- Removal soft-deletes the session's explicit documents in the scope's
collections and cascades (fail-closed, transitively) to derived
documents whose source_ids intersect anything removed, deletes the
vectors from the external store, then enqueues a card_refresh dream
with rebuild=True plus a manual omni dream per touched collection.
- Zero LLM re-derivation: explicit documents are session-pure (DEV-2000
invariant); the only external call is re-embedding rows whose
embedding column is NULL (external-store deployments).
- Per-session job status lives in the scope peer's internal_metadata
under backfill_status, written via single-statement JSONB merges
(concurrent-writer safe) and surfaced at
GET /v3/workspaces/{w}/scopes/{scope_id}/status.
- Enqueued from the scopes add-sessions route and SessionCreate.scopes
handling, only when the session already has messages.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: add scope backfill/removal test coverage, fix backfill status JSONB bug
Adds tests/deriver/test_scope_backfill.py covering the DEV-1999 scope
backfill-by-copy and removal reconciliation jobs implemented in a prior
commit: explicit-doc copying, copied_from idempotency (including
add->remove->re-add), multi-peer collection routing, removal cascade to
dependent derived docs, dream enqueues (manual omni on backfill;
card_refresh rebuild + omni on removal), the status route, and add-sessions
route wiring (backfill enqueued only when the session already has messages).
Fixes a real production bug surfaced by these tests: update_scope_backfill_status
passed json.dumps()'d strings through SQLAlchemy cast(..., JSONB), which
double-encodes (psycopg re-serializes the already-JSON string), producing a
JSONB string scalar instead of an object. Postgres's `||` between two
non-array jsonb scalars doesn't merge — it silently wraps both into a
2-element array, corrupting backfill_status into a list. This crashed
clear_scope_backfill_status's `#-` path delete (called on every removal)
with "path element is not an integer" once a session had ever completed a
backfill. Fixed by passing raw Python dicts to cast() instead, mirroring the
working pattern already used in update_collection_internal_metadata.
Also adds src.deriver.scope_backfill.tracked_db to conftest's tracked_db
patch list — the module was missing from that per-import-site allowlist, so
its DB work ran against the real configured database instead of the
isolated per-test database.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(crud): preserve cache invalidation across get_or_create retry
`get_or_create_peers` and `get_or_create_scopes` mutate existing rows, then
insert new ones inside `db.begin_nested()`. When a concurrent writer creates
one of those rows first, the insert raises IntegrityError and the function
retries.
`begin_nested()` autoflushes the pending UPDATEs *before* opening the
savepoint, so the rollback neither undoes them nor expires the now-clean ORM
state. The retry then compared already-updated values, found no change, and
dropped those peers from `changed_peers` — skipping the cache purge while the
row change committed anyway, leaving entries stale until the 300s TTL.
Carry the mutated names into the retry via `_pending_invalidation` so the
purge cannot be lost.
The scopes facade mirrors `get_or_create_peers`, so both copies carried this.
The peer path is pre-existing and runs on every message ingest.
Also add the missing /v3 prefix to `_SCOPES_ROUTE_GUIDANCE`, which pointed
callers at a 404.
Adds tests/crud/test_get_or_create_retry_invalidation.py, which drives a real
racing session and fails without this change.
* fix(scopes): make scope identity unforgeable, unblock non-pattern peer names
Three coupled changes to the scopes facade.
1. `PeerCreate` no longer gates internal lookups. It exists to validate a new,
user-supplied peer id at the API boundary, but crud used it as a DTO for
names that already exist, so any name outside RESOURCE_NAME_PATTERN raised a
raw pydantic ValidationError — which is not a HonchoException, so it fell
through to the catch-all handler as an HTTP 500. Adds `PeerSpec` (same
fields, no charset pattern) as `PeerCreate`'s base, widens
`get_or_create_peers` to accept it, and changes `get_peer` to take a plain
str. All 13 construction sites converted; the create route keeps full
validation.
This unbreaks the Dreamer: DreamScheduler passes `collection.observer`
straight into the specialist preflight, and scope peers have
`observe_others=true`, so every `(scope.x, peer)` dream died there — the
feature scopes exist to enable. It also fixes a pre-existing bug unrelated
to scopes: a peer named `alice.smith` (legal before d429de0e5338, which
validated names by length alone) 500s on message create, session peer add,
and peer update.
2. The `kind` flag moves from `configuration` to `internal_metadata`.
`configuration` is user-writable — `PeerCreate`/`PeerUpdate` accept a
free-form dict and `update_peer` replaces it wholesale — so a legitimate
`{"observe_me": true}` update silently dropped the flag, and a forged
`{"kind": "scope"}` injected an ordinary peer into `POST /scopes/list`.
`internal_metadata` appears in no API schema. `observe_me: false` stays in
`configuration`, where it belongs.
3. Scope identity requires prefix AND flag, via `is_scope_peer()` and
`scope_peer_clause()`. Neither half is forgeable: the prefix sits outside
RESOURCE_NAME_PATTERN, `internal_metadata` is unreachable. Usage-site guards
become flag-based so a legacy peer merely occupying the namespace keeps
working rather than 422-ing on its own traffic; peer create and update stay
name-based, since those must stop new names entering the namespace.
`update_peer` now returns 422 instead of 500.
Also swaps the reserved prefix from `scope__` to `scope.`: `_` is inside
RESOURCE_NAME_PATTERN, so any tenant could already own a `scope__x` peer.
No DB migration — `internal_metadata` already exists on `peers`, and no scope
peers exist in any deployment yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): validate peer names on create, close namespace squatting and upsert race
Addresses three review findings against 48047a6a.
1. `PeerSpec` let API callers create invalid and reserved-prefix peers.
Widening `get_or_create_peers` to accept a pattern-free schema fixed the
lookup 500s but also removed validation from the *insert* path, and
request-controlled names reach it via message authors, session peer maps, and
the chat observer path — none of which carry a charset pattern of their own.
Confirmed: `POST /sessions/{id}/messages` with `peer_id: "scope.x"` returned
201 and minted an unflagged squatter, after which `POST /scopes {id: x}` was
permanently 409-blocked — namespace denial of service by any caller able to
post a message. `peer_id: "not a valid name!@#"` was likewise created.
Fixed by validating only names about to be INSERTed
(`_validate_new_peer_names`), so already-existing names — legacy dotted
names, scope peers — still resolve without a spurious 422. That keeps the
Dreamer fix intact, since it reads through `get_peer`.
2. Existing reserved-prefix squatters could not be updated. The name-based guard
on `PUT /peers/{peer_id}` refused every `scope.` name, contradicting the
invariant that an unflagged squatter stays a normal peer. Now flag-based, so
behavior is three-way: a real scope is refused, an existing unflagged peer
updates, and a missing reserved-prefix name is refused by (1) rather than
minted.
3. Scope checks raced with get-or-create and the membership upsert. The
route-level guards run before peers are resolved, so a scope created
concurrently in that window would be attached by the generic path with a
default `SessionPeerConfig()`, clobbering its observer membership config.
Adds `_reject_resolved_scope_peers`, which runs on the resolved rows in the
same transaction as the upsert — no window, no extra query. The early checks
stay for better error messages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): guard scope membership config, move checks to the mutation point
Addresses a second review pass against 10655792.
1. Scope membership configuration was directly user-mutable.
`PUT /sessions/{id}/peers/{peer_id}/config` had no scope guard at all, and
`crud.set_peer_config` resolved the peer only to discard the row. Confirmed:
posting `{"observe_others": false, "observe_me": true}` for a scope returned
204 and persisted, which silently stops all fan-out into the scope and makes
Honcho form a representation *of* a scope — neither of which is reachable
through the facade. Deterministic, no race required. Now checked on the row
`get_peer` already returns, so it costs nothing and cannot race.
2. Empty and over-long names were still 500s. Removing the charset pattern from
`PeerSpec` fixed one trap but left its length bounds, and request-bound peer
names carry no length limits of their own — so `peer_id: ""` or a 513-char
name reached `PeerSpec(...)` and raised a raw pydantic ValidationError that
the catch-all turned into a 500. `PeerSpec` now carries no constraints at all
(matching its documented purpose) and every rule for a new name lives in
`_validate_new_peer_names` on the insert path.
3. Resolved-row protection generalized. The previous pass applied it only to
membership upserts, leaving check-then-use windows elsewhere: peer update
could have a concurrently-created scope's configuration replaced wholesale
(create-path validation does not fire for a peer that now exists), the chat
observer get-or-create could resolve a fresh scope as its observer, and the
generic session-peer removal could silently detach a scope from its sessions.
Each now inspects the resolved peer immediately before acting; the redundant
name-level guard on the update route is dropped in favor of the race-free one.
`remove_peers_from_session` grows an internal `_allow_scope_peers` flag because
the scopes facade ends membership through that same path and must not be blocked
by its own guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(scopes): enumerate every peer-touching route against a scope policy
Three review passes each found the same class of defect: a route nobody had
checked, rather than logic that was subtly wrong. One of them — `PUT
/sessions/{id}/peers/{id}/config`, which let any caller set a scope to
`observe_others=false` and silently stop all fan-out into it — predated this work
entirely, because the guardrail set was assembled guardrail-by-guardrail instead
of derived from the route list. Sampling review cannot close that kind of gap;
enumeration can.
Derives every route through which a peer name can reach the system and requires
each to be classified as GUARDED or EXEMPT-with-a-reason, so a newly added
peer-touching route fails the suite until someone classifies it. Detection is the
union of path shape and a walk of the dependant tree (including sub-dependency
`Form(...)` params and nested request-body models), because neither signal alone
suffices: parameter names miss `POST /sessions/{id}/peers`, whose peer names are
dict keys, and path shape misses `messages/upload`, whose `peer_id` arrives as a
form field behind a parser dependency.
Both invariants are then asserted behaviorally, by calling the routes rather than
inspecting annotations — the guards deliberately live in crud, which is what makes
`messages/upload` guarded for free via `crud.create_messages`:
- a real scope is refused on all 11 guarded routes, and the rejection must name
the scope, so an unrelated 422 (a malformed body) cannot pass the assertion;
- an *unflagged* peer merely occupying the reserved namespace is unaffected. That
half regressed once already when `update_peer` used a name-based check.
Mutation-tested all three failure modes: disabling the `set_peer_config` guard
fails the guarded test naming that route; regressing `update_peer` to name-based
fails the squatter test; adding an unclassified peer route fails the enumeration.
Covers the HTTP surface only. Peer names also reach the system through the
deriver, dreamer, and queue, which have no route table to enumerate — noted in
the module docstring rather than implied to be covered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse a scope in every observed position; enumerate per position
Addresses a fourth review pass against 62681e4d. The headline finding is that the
previous commit's enumeration test had the wrong *model*, not a missing entry.
1. Manual conclusions could create knowledge about a scope. `POST /conclusions`
validated only that observer_id and observed_id exist, so a scope as
`observed_id` persisted a conclusion about a peer carrying observe_me=false and
created an (observer, scope) collection for it. Confirmed: 201, and it read
back. `POST /schedule_dream` had the same hole via `observed`.
The fix is positional, because the invariant is:
A scope may be an OBSERVER. A scope may never be OBSERVED.
A scope as `observer_id` is how scoped conclusions are stored and must keep
working (verified still 201); as `observed_id` it is now refused. Same split
applied to schedule_dream `observed`, the peer-card `target` (which also covers
a scope's self-card, since target-omitted collapses observed to peer_id), and
session-context `peer_target`.
2. Chat target and both representation roles kept check-to-use races. Only the
chat path-level observer was re-checked on its resolved row; the target was
checked by name and then resolved without inspecting scope identity. Both are
now checked at the dialectic preflight, where observer and observed are already
resolved — an absent name has already failed by then, and an existing squatter
cannot retroactively become a scope.
3. Generic membership removal was still racy. The adjacent SELECT narrowed the
window but could not close it under READ COMMITTED. The UPDATE now carries its
own correlated NOT EXISTS against scope_peer_clause(), so Postgres evaluates
the exclusion as part of the statement and a scope committed after the advisory
check still cannot be detached.
4. New-name validation ran after the name reached Postgres. A NUL byte passed the
request schemas and PeerSpec, then raised psycopg.DataError inside the lookup —
a 500. Values that cannot correspond to a stored row by construction (NUL
bytes, over-length names) are now refused before the query.
(Over-length names already returned 422; only the wasted query was real there.)
The enumeration test is rekeyed from (method, path) to (method, path, position).
A binary per-route verdict cannot express finding 1 at all: `POST /conclusions` is
one route with two positions and opposite verdicts. Detection widens to observer /
observed / target / peer_target / peer_perspective, which surfaced four routes the
previous version never saw — conclusions, schedule_dream, queue/status, and
session context.
Also registers `src.routers.workspaces.tracked_db` in the conftest patch list; the
new guard there would otherwise have run against the real configured database
instead of the per-test one.
Mutation-tested: disabling the conclusions observed-guard fails the positional
test naming that position; adding an unclassified `observed_id` param fails
enumeration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse future scopes in observed positions, preserve scope membership
Addresses a fifth review pass against 14136e5b. All six findings reproduced
locally before fixing.
1. High — generic peer replacement removed scope memberships.
`set_peers_for_session` soft-deleted every active SessionPeer row, and the
request-level guard only inspected names *present* in the replacement map. A
caller detached a scope by simply omitting it, never naming it — so no
request-level guard could ever see it. Reproduced: scope sessions went
['<id>'] -> [] on a 200. The exclusion now lives in the UPDATE itself
(correlated NOT EXISTS against scope_peer_clause), so replacement means
"replace ordinary peers" regardless of request contents or concurrent creation.
2. High — peer cards could be pre-seeded for future scopes.
`set_peer_card` resolves only the observer and writes a JSONB key derived from
an unchecked observed name, and the route guard rejected only *existing*
flagged scopes. Reproduced: PUT card with target=scope.<missing> returned 200,
creating that scope then returned 201, and the card described the real scope.
3. High — dreams could be queued for future scopes.
The route checked `observed` in a read-only session that closed before
`enqueue_dream`, and a missing reserved name passes any is-it-a-scope check.
Reproduced: 204 with observed=scope.<missing>.
2 and 3 share a root cause, so they share a fix: a new `reject_scope_observed`
that is stricter than `reject_scope_peers` in exactly one case — a *missing*
reserved name is refused, because nothing on these paths creates the peer, so
nothing else would ever catch it. Existing unflagged squatters still pass.
Both guards moved to the mutation point: card validation into
`crud.set_peer_card` (same transaction as the JSONB write, so Dreamer and
agent-tool callers are covered), dream validation into `enqueue_dream` (same
transaction as the queue insert). The redundant route-level checks are dropped
rather than left as weaker duplicates.
4. Medium — prefixed NUL names still reached PostgreSQL.
`reject_scope_peers` filtered for the reserved prefix and sent matches to a
text comparison, so "scope.future\0name" raised psycopg.DataError — a 500.
Both guards now share `_reserved_name_candidates`, which materializes the input
once and rejects impossible values before any SQL. Materializing matters
independently: the message-author path passes a generator, and validation
iterates separately from the prefix filter, so a generator would be
half-consumed. `_reject_impossible_peer_names` now takes a Collection so the
type checker enforces that.
5. Medium — representation kept a check-to-use race.
The previous commit claimed both representation roles were rechecked after
resolution; that was wrong — only the dialectic preflight got that check, and
the representation route never goes through it. It now opens one short
read-only session *after* the embedding call, checks both positions, and passes
that same session to `get_working_representation`, so no connection is held
across external work and a scope committed later cannot have conclusions in the
collection being read.
6. Low — policy coverage was not exhaustive. `sender_id` reaches CRUD as
`observed` but was missing from the detected parameter set. ALLOW cases could
also not carry builders, so the suite never proved the other half of the
contract — that legitimate scope *observers* keep working, which a guard
rejecting scopes everywhere would satisfy. Both fixed; observer positions on
conclusions, dreams, cards, session context and queue status are now asserted
behaviorally.
Deliberately not implemented: the scope-creation backstop scanning for
pre-existing card keys and queue items naming a future backing peer. Reasoning is
recorded in `get_or_create_scopes` — no new such state can be created now, any
pre-existing row is coincidental since `scope.` was never a meaningful namespace,
the consequence is inert, and detecting card keys means a full table scan per
scope creation.
Mutation-tested each new guard: removing the replacement exclusion fails both
membership-preservation tests; weakening either observed guard to existing-only
fails the pre-seeding tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): exclude scope memberships from the session observer limit
Scope memberships carry observe_others=true, so every scope counted against
SESSION_OBSERVERS_LIMIT (default 10) — capping scopes-per-session at the limit
minus the session's real observers, and reporting the failure as
`400 Cannot create session <name> with 11 observers. ... Observers are peers
with 'observe_others' set to true.` on a membership call. Wrong on three counts:
the ceiling is undocumented and contradicts RFC §5.1 ("sessions belong to any
number of scopes"), the message describes session creation, and it leaks the
word "observer" through a facade whose entire job is hiding observers (RFC
goal 5). The limit exists to bound per-observer deriver fan-out for real peers;
a scope costs document rows, not LLM calls (RFC §5.2), so it does not belong in
that budget.
Excluded from both halves of the check in `_get_or_add_peers_to_session`: the
incoming names via a flag-based lookup, existing memberships via a correlated
NOT EXISTS on `scope_peer_clause()` — the same pattern the replacement and
removal paths already use, so the exclusion holds regardless of concurrent
scope creation. The early `count_observers_in_config(session.peer_names)` check
in `get_or_create_session` is left alone: `peer_names` cannot contain a scope,
and `scopes` is a separate field.
`reject_scope_peers` is split into a `scope_peer_names()` query helper plus a
two-line raiser so the observer count reuses the authoritative name-AND-flag
predicate instead of growing a third copy of it. Still costs nothing on the
common path — no reserved-prefix name in the input means no query at all.
Also caps `SessionCreate.scopes` at 100, matching `ScopeSessionsAdd.session_ids`.
This belongs in the same commit: the observer limit was the only thing bounding
that list, so removing it turns an unbounded `scopes` array into a peer row and
a membership row per element, committed — the single-request path to the
cardinality anti-pattern RFC §8 warns about. Partly answers OQ6: no per-session
cap, 100 per request.
Tests: a session joins SESSION_OBSERVERS_LIMIT + 2 scopes through both the
facade and session creation; real observers over the limit still 400, so the
carve-out cannot quietly disable the limit; 101 scopes is a 422.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): skip semantic retrieval when the embedding precompute failed
Addresses three open review comments.
1. Major — the representation read could embed inside its DB session. The
route's precompute is suppressed, and both
`RepresentationManager.get_working_representation` and
`crud.query_documents` fall back to embedding when a query arrives without
one, so a failed precompute meant an external call inside the read session
this branch opens for the scope re-check — the connection-holding rule the
route's own comment claimed to satisfy. The innermost fallback also only
catches ValueError, so a provider outage surfaced as a 500. The semantic
query is now passed only when an embedding exists, degrading to
derived+recent retrieval. (`crud.query_documents` embedding inside a caller's
session predates this branch and is left alone.)
2. Minor — `test_resolved_scope_peer_rejected_at_membership_upsert` described a
race it does not perform. It creates an already-flagged scope and calls crud
directly; the unflagged → flagged transition is not simulated. Docstring now
says what the test actually pins.
3. Minor — `test_empty_replacement_preserves_scope_membership` asserted only
half its docstring. It passed if the empty PUT left every ordinary
membership intact; now asserts the ordinary peer's left_at is set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): close auth and observed-position gaps, paginate membership
Review response for #884.
Security:
- gate `SessionCreate.scopes` behind a workspace-level key; the session-create
route is self-authorizing, so a peer- or session-scoped token could mint scope
peers and join sessions to scopes it had no access to via `POST /scopes`
- refuse a reserved-but-nonexistent name in two observed positions that used the
permissive guard: chat `target` and session-context `peer_target`. Both let a
caller act on `scope.X` before it existed, then create the scope
Facade:
- exclude scope peers from `GET /sessions/{id}/peers` and refuse the membership
-config read for a real scope, matching its write side
- replace `GET /scopes/{id}/sessions` with `POST /scopes/{id}/sessions/list`
returning `Page[Session]`; the add route now returns 204. Membership was
unbounded on both, while every other list surface paginates
- rename `crud.get_scope` to `get_scope_or_raise`
Tests:
- add a missing-name axis to the route-policy table (`Case.refuse_missing`), which
is what surfaced the two guard gaps above
- delete 14 hand-written tests the table now enumerates; 52 -> 39 functions in
test_scopes.py with more cases covered
- tighten the squatter assertion from `!= 422` to `< 400`, which was passing on 5xx
- assert the FastAPI-internals traversal still derives positions, so a framework
upgrade can't silently empty the suite
Docs:
- drop internal ticket and RFC references from the published OpenAPI descriptions
and surrounding comments; state the behavior instead
- move implementation reasoning out of the `PUT /peers/{id}` docstring, which
FastAPI publishes, into a comment
* test(scopes): assert exact statuses for permissive missing-name cases
Follow-up review pass on #884.
- add `Case.missing_status` so a permissive missing-name position asserts the
status it should actually get (404, or 200 for the no-op removal) instead of
`!= 422`, which also passed on a 5xx — the same hole already closed in the
squatter assertion
- require it whenever `refuse_missing` is False, and require its absence when
True, so the policy table can't drift from the assertion
- repoint a stale allow-reason at POST /scopes/{scope_id}/sessions/list; the GET
it named was removed
- document the membership list's ordering under `reverse`
* chore: clean up stale docstring language
* fix: don't backfill a session that left the scope
scope_backfill and scope_removal carry different work-unit keys, so
nothing orders them: a removal enqueued right after the add — or one
that lands while the backfill is embedding — sweeps the scope before
the copies exist, leaving a departed session's documents live in the
scope forever.
_run_backfill now re-checks SessionPeer membership inside the write
transaction and returns None; process_scope_backfill then skips both
the dream enqueues and the status write, so a skipped backfill can't
resurrect the status entry removal just cleared.
Adds coverage for the skip, the NULL-embedding re-embed path, and the
failed-status write. Handler-driven tests now stand up the membership
row the guard requires.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Ulysse Pence
Aakash Kattelu
Vineeth Voruganti
Eugene Eisenstein
Eri Barrett
Rajat Ahuja
ajspig
Ken Weiner
Phil
adavyas
Serhii Zghama
Daniel Peng
Phil
ZiYun Liu
Joe-Kneeland