PR #89 (session-derived JWT auth) merged to astrolabe main and released as
v0.16.1; track the released tag instead of the feature-branch commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- get_indexed_doc_types: add optional accessible_owners param and reuse
build_ownership_filter so cross-user doc-type discovery matches the real
search scope (was self-only / ACL-blind); docstring documents the self-only
default. Covered by test_get_indexed_doc_types_is_acl_aware.
- access_filter: build_ownership_filter now omits the owner_id branch entirely
for an empty owner set instead of relying on undocumented MatchAny(any=[])
semantics; updated the empty-list unit test accordingly.
- access_filter: make the uid_owner/owner share-owner extraction explicit
("absent, not empty") to avoid skipping on a falsy-but-present field.
- access_filter: add an operator note that pre-owner_id points need a re-index
to surface to share recipients (ACL search is a no-op for legacy data).
- verification/webdav: lock the file_accessible_by_id(scope="") contract with a
targeted multi-user test (owner + recipient True, non-recipient False).
- viz_routes: comment that verify-on-read eviction runs inline by design (no
lifespan task group available on the Starlette route).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes the multi-user-basic regression introduced by the previous bump: the
astrolabe app's hard https-refusal blocked sending the app password to the
in-cluster http MCP endpoint (http://mcp-multi-user-basic:8000), so background
indexing never ran. Now warns instead of blocking; also clears the SonarCloud
S5332 hotspot on the dropped test's http literal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- vector/qdrant_client.py: add owner_id to _PAYLOAD_INDEX_FIELDS (BLOCKING).
Every search applies MatchAny(key="owner_id", ...); without a keyword index
Qdrant full-scans the collection and may 400 on Qdrant Cloud strict mode.
_ensure_payload_indexes is idempotent so existing collections migrate at
startup.
- search/access_filter.py: bound the process-global _owners_cache with an LRU
cap (was one unbounded entry per active user, never evicted); document the
owner-level over-fetch limitation (a prolific sharer floods the recall
buffer with ghost candidates that verify-on-read drops, with no second
Qdrant pass) as a TODO toward per-file filtering.
- search/algorithms.py + semantic.py + bm25_hybrid.py: promote
accessible_owners from **kwargs to an explicit keyword-only parameter on the
SearchAlgorithm ABC and both implementations, so a misspelled keyword is a
type error rather than a silent fall back to self-only scope.
- search/verification.py: document that _verify_files now verifies by global
file id (WebDAV SEARCH), not by path.
- tests/unit/search/test_access_filter.py: add cache-hit, TTL-expiry,
failure-not-cached, and LRU-bound tests.
Bumps the astrolabe submodule with the matching #89 review fixes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tracks astrolabe feat/session-derived-jwt-auth tip (86616d9). Test-only
change (rename a unit-test constant to clear SonarCloud S2068); no functional
or runtime difference from the previously-verified submodule commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bumps the astrolabe submodule to validate the one-click background-sync app
password internally (IProvider::getToken) instead of via an HTTP loopback to
overwrite.cli.url, which is unreachable from inside the app container and broke
the multi-user-basic integration legs (enable_background_sync never succeeded).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- viz_routes: run verify_search_results before returning results. After the
accessible_owners expansion the viz can surface OTHER users' shared docs, so
it must drop ones the caller can no longer access (revoked share) — same as
the nc_semantic_search tool path. (Blocking review item.)
- access_filter: cache list_accessible_owners per user for 30s to keep the OCS
shares round-trip off the search hot path (failures aren't cached); document
the single-page OCS limitation; add a clear_accessible_owners_cache() test
helper. Comment the empty-accessible_owners MatchAny([]) edge case.
- verification: comment why cross-user eviction is a deliberate no-op (eviction
is scoped to the querying user's id, so a recipient's revoked access never
deletes the owner's points; the recipient self-heals via accessible_owners).
- algorithms: declare SearchResult.original_score (set by the viz route) so the
now-precisely-typed result list type-checks.
- tests: cross-user eviction-no-op safety test; autouse owners-cache reset in
the access_filter + shared-search tests; replace async-no-await qdrant fakes
with AsyncMock (clears SonarCloud S7503).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aligns PR #813's submodule pointer with astrolabe PR #89 tip (05176f9).
No MCP-server runtime change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Aligns PR #813's submodule pointer with astrolabe PR #89 tip (091c68b),
which makes `composer run psalm` clean. No MCP-server runtime change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The remaining SonarCloud S5443 (publicly-writable directory) findings were the
hard-coded /tmp screenshot paths in revoke_background_sync_access, which became
"new code" once the surrounding function was edited. Replace every /tmp literal
in the file with tempfile.gettempdir() (which S5443 accepts), eliminating the
findings consistently rather than per-line.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SonarCloud Automatic Analysis does not honour # NOSONAR, so the S6418
(hard-coded token) and S5443 (publicly writable /tmp) findings in the new
tests persisted. Fix them by construction instead:
- test_login_flow: use a trivial poll-token value ("tok") in the rewrite test
(it asserts the URLs, not the token) so it no longer looks like a secret.
- test_astrolabe bg-sync: build the debug screenshot path from
tempfile.gettempdir() rather than a hard-coded /tmp literal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new-code quality gate flagged test-only mock fixtures as security issues
(new_security_rating E):
- S2068 "hard-coded password" ×2: drop the unused "app_password" value from the
get_app_password_with_scopes mocks (the code under test only reads truthiness
+ "scopes").
- S6418 "hard-coded token": NOSONAR on the Login Flow v2 poll-token test fixture.
- S5443 "publicly writable directory": NOSONAR on the /tmp debug screenshot path
(matches this file's existing convention).
No behaviour change; all are test fixtures, not real credentials.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No runtime change — keeps the PR #813 submodule pointer aligned with the
astrolabe PR #89 tip (587caa6).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SonarCloud flagged the http:// mock URLs in the new login_url-rewrite test as
clear-text-protocol hotspots, failing the new-code quality gate (they were new
+ unreviewed). They're harmless test fixtures; switch to https mock origins to
match this file's existing convention. The login_url rewrite is scheme-agnostic
so the test still exercises the same internal->public origin replacement.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The OAuth provisioning tools (check_provisioning_status, revoke_nextcloud_
access) only consulted the refresh-token store + Astrolabe status, ignoring the
app_passwords store that Login Flow v2 (nc_auth_provision_access) and the
management API write to — the same store require_provisioning / get_client use
to grant tool access. Result: status reported "not provisioned" while tools
worked, and revoke said "nothing to revoke" while the credential persisted.
- _get_provisioning_status: also check storage.get_app_password_with_scopes,
reporting is_provisioned with credential_type=app_password,
flow_type=login_flow_v2.
- _revoke_nextcloud_access: when the credential is an app password, delete it
from storage + invalidate the scope cache (no IdP token to revoke);
refresh-token revocation via the Token Broker is unchanged.
- tests/unit/test_oauth_tools_app_password_provisioning.py: cover status +
revoke for the app-password path.
- bump astrolabe submodule (deprovision MCP on disable); fix a stale assertion
in the migrated bg-sync test (one-click flow has no separate app-password
generation step).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Astrolabe was refactored to mint session-derived JWTs (TokenGenerationRequest
Event) and a one-click background-indexing opt-in, dropping the OAuth
authorize/callback/refresh surface. Bump the submodule and bring the test
suite in line:
- New test_astrolabe_session_jwt_search.py: a logged-in user searches via the
minted JWT with no provisioning (replaces the obsolete login_flow_provisioning
OAuth-authorize test; token_refresh test deleted — refresh flow is gone).
- settings_buttons: assert the new revoke endpoint + that oauth/disconnect is
gone (404).
- multi_user_background_sync / plotly / chunk_context: drop the OAuth authorize
step; provision via the one-click "Enable background indexing" button
(#mcp-enable-background-button -> #mcp-revoke-background-button) instead of
generating + pasting an app password.
- docker-compose.yml: mount the astrolabe submodule into the app container.
- third_party/astrolabe: bump to the one-click opt-in commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two fixes surfaced while testing Login Flow v2 provisioning behind a split
internal/external host (Docker: server↔Nextcloud over http://app, browser
over http://localhost:8080):
1. login_url pointed at the internal host. Nextcloud builds the login URL
from the request host, so the browser-facing URL came back as
http://app/login/v2/flow/... — unreachable from the user's browser. The
poll endpoint was already rewritten to the internal host (correct, the
server polls it); now LoginFlowV2Client also rewrites the login_url origin
to settings.nextcloud_public_issuer_url when set (passed at all 5
construction sites). When unset, behaviour is unchanged.
2. The app-password format guard rejected raw session tokens. core/
getapppassword returns a long alphanumeric token, not the dashed 25-char
Security-settings format, so the dashed-only regex 400'd the one-click
opt-in handoff. Relax APP_PASSWORD_PATTERN to `^[a-zA-Z0-9-]{20,256}$`;
the authoritative validation is still the BasicAuth check against Nextcloud.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ACL-aware vector filter (PR #813) expands a user's search to documents
whose owner shared them, but verify-on-read still re-checked each file by
PATH under the *searching* user's WebDAV root. Nextcloud mounts received
shares at the recipient's root by basename, so a nested shared file (e.g.
owner's /docs/report.pdf) 404s for the recipient and was silently dropped —
defeating the filter for everything but root-level files.
Verify files by their global Nextcloud file id instead (the file doc_id IS
that id): WebDAVClient.get_file_info_by_id was insufficient (the dav/meta
endpoint only resolves the user's own storage, not shares), so add
WebDAVClient.file_accessible_by_id which runs a WebDAV SEARCH over the user's
whole tree (incl. mounted shares) filtered on oc:fileid. Empirically this
resolves owned, directly-shared, and folder-shared files; an empty result is
a definitive drop, transport errors are kept as transient.
- search/verification.py: _verify_files now checks file_accessible_by_id.
- client/webdav.py: add file_accessible_by_id (SEARCH by fileid).
- tests/integration/test_acl_owner_filter.py: filter matrix vs real Qdrant.
- tests/integration/test_acl_shared_search.py: real-Nextcloud share -> search.
- tests/integration/test_verify_on_read.py: nested shared file kept for the
recipient; unshared file dropped.
- tests/unit/search/test_verification.py: id-based verifier semantics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vector index has always been strictly per-user: every Qdrant payload
carries a `user_id` and the search filter is `user_id == querying_user`.
A file Alice indexed cannot be discovered by Bob even if she has shared
it with him — Bob would have to re-index it under his own user_id to
make it searchable, which means duplicate index entries for every share
recipient.
Switch to ownership-with-ACL-expansion:
- New `nextcloud_mcp_server.search.access_filter` module:
- `list_accessible_owners(sharing_client, user_id)` calls the OCS
Sharing API (`shared_with_me=true`) and returns
`{user_id} ∪ {uid_owner of each share}`. Fails open to `[user_id]`
so a misbehaving Sharing API doesn't black-hole search.
- `build_ownership_filter(user_id, accessible_owners)` returns a
Qdrant `Filter` whose `should` branch matches either the new
`owner_id IN accessible_owners` field or the legacy `user_id` field.
The legacy branch keeps points indexed before this change reachable
without a migration backfill.
- Indexer payload (`vector/processor.py`) now writes `owner_id` alongside
`user_id`. `DocumentTask` gains an optional `owner_id` field; today the
scanner always runs as the owner so the processor falls back to
`user_id`, but the field is plumbed so a future shared-with-me crawler
can set the true owner without reshaping the payload contract.
- `SemanticSearchAlgorithm.search` and `BM25HybridSearchAlgorithm.search`
accept `accessible_owners` via kwargs and use the new ownership filter.
Default behaviour with no kwarg is unchanged (self-only).
- Both user-facing callers — the MCP tool path (`server/semantic.py`) and
the visualization Starlette route (`auth/viz_routes.py`) — compute
`accessible_owners` from the authenticated Nextcloud client before
invoking the search algorithm. Eviction, scanner deletion, placeholder,
and chunk-context paths intentionally keep the legacy `user_id`
semantics (those are "operations on a specific user's records", not
cross-user reads).
- 10 new unit tests in `tests/unit/search/test_access_filter.py` cover
self-only default, owner expansion, dedup, fallback fields, OCS
failure, and the legacy `should`-branch shape.
Pairs with cbcoutinho/astrolabe#89 — together they let an Astrolabe user
find content owners have shared with them without going through any
re-authorization flow or re-indexing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The startup sweep was reading settings.qdrant_collection (the raw config
value, default "nextcloud_content") instead of settings.get_collection_name(),
which is what every other vector-sync operation uses. When QDRANT_COLLECTION
is not overridden, get_collection_name() auto-generates a
{deployment-id}-{model-name} name; the sweep was targeting a non-existent
collection and silently returning (0, 0).
Also adds the AsyncQdrantClient type annotation that was missing on
sweep_orphan_placeholders, and renames its parameter from collection_name
to collection to make it clear the value must be the resolved name.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the per-tenant nextcloud-mcp-server Pod OOMKills mid-batch, the
in-memory anyio processor queue is lost but the placeholder Qdrant
points (is_placeholder=true, status=pending) survive. The next Pod's
scanner re-runs, sees the existing placeholders, applies the
5 × VECTOR_SYNC_SCAN_INTERVAL staleness gate (~5h with the deployed
1h scan interval), and skips them. Result: 0 documents indexed for
the duration of the gate after every restart.
Stamps a process-level instance_id (UUID per Pod-process) onto every
placeholder write. A new sweep_orphan_placeholders helper, called
once from starlette_lifespan after the Qdrant client is initialised
and before the scanner / user-manager spawns, scrolls the collection
and deletes any placeholder whose instance_id doesn't match the
current Pod's (including placeholders with no instance_id field —
back-compat for pre-fix Pod versions). The scanner's next cycle
naturally re-creates fresh placeholders and queues work normally;
no DocumentTask reconstruction needed.
Sweep is one-shot at startup, not periodic — the existing staleness
gate still covers same-Pod recovery, and the cross-Pod-restart gap
was the only failure mode. Failure is non-fatal (logged via
vector_sync.orphan_sweep_failed) so a transient Qdrant hiccup at
boot doesn't prevent the scanner from running.
Both lifespan branches (single-user BasicAuth, OAuth / multi-user
BasicAuth) call the sweep via a module-local helper. A new
VECTOR_SYNC_ORPHAN_SWEEP_ENABLED setting (default True) provides
an escape hatch.
Closes Deck #101.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #719 fixed the contact-create path so all documented fields persist to the
vCard, but the read path (list/search via MCP) still returned ``organization:
null`` / ``note: null`` / ``title: null`` because pythonvCard4 has no typed
parser for ORG/TITLE — they land in ``Contact.custom`` — and the server-side
mapper never read ``note``/``urls``/``categories``/``photo`` even when present.
Reads now surface what the write side persisted:
- ``client/contacts.py``: new ``_first_custom`` helper pulls raw values from
``Contact.custom`` for ORG/TITLE/unencoded PHOTO. ``list_contacts``
extends its per-contact dict with org/title/note/url/categories/photo.
- ``server/contacts.py``: ``_raw_contact_to_model`` maps the new keys onto
``Contact.organization`` / ``.title`` / ``.note`` / ``.urls`` / ``.categories``
/ ``.photo``. URL accepts both list and plain-string shapes; categories
accepts comma-separated strings for forward-compat.
Coverage:
- Unit: ``TestFirstCustom`` (five cases incl. bare-string library shape) and
three new ``_raw_contact_to_model`` cases covering the full field set,
plain-string URL, and comma-string categories.
- Integration: ``test_mcp_contacts_workflow`` now decodes the
``nc_contacts_search_contacts`` response and asserts
``organization`` / ``note`` round-trip — direct regression coverage for
elvisdragonmao's report on issue #716.
Verified end-to-end against the local single-user docker stack: creating a
contact with ``{organization, title, note, url, categories}`` and reading it
back via ``nc_contacts_search_contacts`` returns every field populated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Astrolabe (and any other PHP-side client) sends a stable User-Agent on
every outbound call to the MCP server. Capture it at the middleware
layer so backend access logs can attribute each request to a specific
client build — e.g. ``Nextcloud-Astrolabe/0.14.1``.
The middleware fires only for /api/v1/* and /webhooks/nextcloud,
which is the surface PHP-side clients hit; /mcp and /health stay
silent. The structured ``extra`` ({user_agent, http_method, http_path})
flows into OTel spans so the field is queryable in Grafana / Loki.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nextcloud Deck PR #7910 added IWebhookCompatibleEvent to CardCreated/
Updated/DeletedEvent and BoardUpdatedEvent, so Deck can finally emit
real-time webhooks via core's webhook_listeners app. Wire this into
the existing preset → parser → DocumentTask pipeline that already
backs Notes / Calendar / Tables / Forms / Files sync.
- Add deck_sync preset (app=deck, 4 events) and drop the stale
"Deck does not support webhooks" comment.
- Teach webhook_parser to convert Deck card events into
DocumentTask(doc_type=deck_card, operation=index|delete) with
stack_id metadata. BoardUpdatedEvent logs delivery at INFO and
returns None — the polling scanner reconciles affected cards.
- Cover three new unit tests for the deck create/delete/board-update
paths plus symmetric fail-open tests for missing card.id /
node.id in _parse_deck_event and _parse_file_event.
The astrolabe admin UI auto-discovers the new preset via
filter_presets_by_installed_apps(); no astrolabe-side wiring is
required for it to appear in the Webhook Management card grid.
Note: requires Deck ≥1.18.x (where PR #7910 lands); the preset is
hidden when the Deck app isn't installed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Split the Docker image build into a per-platform matrix and merge job,
producing a single multi-arch manifest (linux/amd64 + linux/arm64) without
QEMU emulation. The arm64 build runs on the native ubuntu-24.04-arm runner.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ``BM25SparseEmbeddingProvider.__init__`` calls
``fastembed.SparseTextEmbedding(model_name="Qdrant/bm25")`` which
downloads ~50 MB of model weights from HuggingFace and loads them
into memory — observed >5 s wall-clock in production. The inference
methods (``encode_async``, ``encode_batch_async``) already wrap work
in ``anyio.to_thread.run_sync``, so the design intent is clearly to
keep FastEmbed off the event loop. That protection just didn't
cover the constructor.
Symptom in the Astrolabe Cloud per-tenant deploy (deck #102 smoke):
~30–90 s after a user enables semantic search, the pod tips into a
SIGKILL-restart cycle. Loki shows a single log line
Initializing BM25 sparse embedding provider: Qdrant/bm25
followed by nothing else from the event loop until exitCode 137.
Kubernetes ``/health/live`` httpGet probe timeout=5s fires 6 times
in a row, kubelet kills the container, restart, repeat.
Fix: switch ``get_bm25_service()`` to an async accessor that wraps
the first-time construction in ``anyio.to_thread.run_sync``. Two
existing call sites (``vector/processor.py:603``,
``search/bm25_hybrid.py:123``) update to ``await``. Both are
already inside async functions so the await is free.
New unit test pins the invariant by monkey-patching
``BM25SparseEmbeddingProvider.__init__`` with ``time.sleep(1)`` and
asserting a concurrent ``anyio.sleep(0.05)`` finishes promptly —
the test fails if the constructor ever runs back on the event loop.
Same pattern exists in ``OllamaEmbeddingProvider.__init__`` (sync
``httpx.get`` health-check). Ollama isn't enabled in any current
deploy; filed as a follow-up.
Refs:
- Astrolabe Cloud deck card #102 (smoke discovery)
- Sibling fix#799 (NullPool for cross-loop-asyncpg, same class
of "anyio bites you in production" bug)
Verified:
- ``uv run pytest tests/unit/`` — 1027 passed
- ``uv run ruff check`` clean on touched files
- ``uv run ty check`` clean on touched files
- New tests pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-review on #799 flagged:
1. Stale inline comment in ``initialize()`` (line 466) still said
"Postgres uses a small bounded pool". Updated to reflect both
backends now use NullPool.
2. Stale ``close()`` docstring referenced pool-size starving
max_connections — irrelevant with NullPool. Replaced with the
NullPool-aware rationale (dispose still tears down in-flight
asyncpg connections cleanly).
3. ``docs/configuration.md`` actively directed operators to tune
DATABASE_POOL_SIZE / DATABASE_MAX_OVERFLOW, with worked
examples and pool math. Both are now deprecated no-ops; the
table entries explain the deprecation and link to PR #799.
Operators reading the docs will no longer be confused into
tuning settings that don't do anything.
4. ``config.py`` comment for the deprecated fields updated to
record the deprecation. Validators are intentionally kept
(still reject < 1 / < 0) so misconfigured deploys fail loudly
rather than silently — the reviewer flagged this as a minor
UX wart but explicitly "not a blocker"; the docs change in (3)
keeps operators away from the config altogether.
5. New ``tests/unit/test_storage_engine.py`` with three tests:
- ``test_postgres_engine_uses_nullpool`` — pins ``isinstance(
engine.pool, NullPool)`` so a refactor back to QueuePool /
SingletonThreadPool can't silently re-introduce the cross-
event-loop crashes.
- ``test_postgres_engine_ignores_pool_sizing_settings`` —
setting DATABASE_POOL_SIZE / DATABASE_MAX_OVERFLOW to huge
values must not change pool type (proves the deprecated
fields are wired-up no-ops).
- ``test_postgres_engine_missing_asyncpg_driver_message`` —
guards the existing actionable-error branch when the
optional ``[postgres]`` extra isn't installed.
Verified:
- ``uv run pytest tests/unit/`` — 1028 passed
- ``uv run ruff check`` clean on the touched python files
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Postgres backend hit a hard crashloop in production deployments
where the MCP server runs under anyio TaskGroups with multiple
concurrent background tasks (`vector.oauth_sync.user_manager_task`,
processors, etc.) alongside the request-path code. Symptom in the
pod logs:
RuntimeError: Task <Task pending name='nextcloud_mcp_server.vector.oauth_sync.user_manager_task'>
got Future <Future pending cb=[BaseProtocol._on_waiter_completed()]>
attached to a different loop
followed seconds later by
RuntimeError: Event loop is closed
while SQLAlchemy's pool tries to clean up the failed connection.
The event loop becomes increasingly unresponsive as asyncpg
protocol Futures pile up holding references to closed loops; the
`/health/live` endpoint eventually misses its probe window and
the kubelet SIGKILLs the pod (exitCode 137), restart-looping the
backend.
Root cause: the engine was built with the default
`AsyncAdaptedQueuePool` (`pool_size=2, max_overflow=5`) and
`pool_pre_ping=True`. asyncpg connection objects are bound to the
event loop they were created on. When the process holds a
singleton engine and tasks running under different anyio
TaskGroups check out connections from that pool, the pre-ping
probe runs on a cached connection whose underlying transport
references a different loop's selector → cross-loop access →
crash.
Switch to `NullPool` — one fresh asyncpg connection per
`engine.connect()`, no caching, no cross-loop bookkeeping to get
wrong. asyncpg connection setup is ~5 ms over LAN and a single
round-trip in the local-Postgres case, so the throughput cost is
negligible for the MCP server's traffic shape (low concurrency,
bursty per-user requests). This matches what the SQLite branch
already does (see `initialize()`) and what Alembic's `env.py`
uses for migrations, so the codebase is now consistent across
all backends.
`DATABASE_POOL_SIZE` / `DATABASE_MAX_OVERFLOW` config knobs are
preserved for backward compatibility but no longer affect the
Postgres engine. The validators in `config.py` continue to
reject values < 1 / < 0, so misconfigured deploys still fail
loudly. A follow-up could mark them deprecated in
`docs/configuration.md`; out of scope here.
Discovered while smoke-testing the per-tenant Postgres flow in
Astrolabe Cloud (every-tenant pod fresh-provisions a database
via the ADR-026 backend → hits this crashloop within ~5 min of
the first MCP-routed request).
Refs:
- ADR-026 § "Concurrency model and pool sizing" (the original
QueuePool rationale, now superseded by this finding)
- nextcloud_mcp_server/alembic/env.py (NullPool for migrations)
- SQLAlchemy docs: NullPool is the documented choice when
connection objects don't survive across the lifecycle of the
pool's logical "owner" (here: the event loop)
Verified:
- `uv run ruff check nextcloud_mcp_server/auth/storage.py` clean
- `uv run pytest tests/unit/test_*storage*.py` → 29 passed
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
