Skip to content

feat(webhooks): add Deck card sync preset with vector indexing#809

Merged
cbcoutinho merged 2 commits into
masterfrom
feat/deck-webhook-presets
May 24, 2026
Merged

feat(webhooks): add Deck card sync preset with vector indexing#809
cbcoutinho merged 2 commits into
masterfrom
feat/deck-webhook-presets

Conversation

@cbcoutinho
Copy link
Copy Markdown
Owner

Summary

  • Adds the deck_sync webhook preset and teaches webhook_parser to convert Deck card events into DocumentTask(doc_type="deck_card", …) so Deck cards get indexed into Qdrant in real time instead of waiting for the polling scanner.
  • New middleware logs the inbound User-Agent on every /api/v1/* and /webhooks/nextcloud request, so backend access logs can attribute calls to a specific Astrolabe build (e.g. Nextcloud-Astrolabe/0.14.1).

Context

Nextcloud Deck PR #7910 added IWebhookCompatibleEvent to CardCreated/Updated/DeletedEvent and BoardUpdatedEvent, undoing the stale "Deck does not support webhooks" comment in webhook_presets.py. The preset library, parser, and processor were already designed for this — only the wiring was missing. The astrolabe-side change (paired PR cbcoutinho/astrolabe#TBD) consumes the new preset via filter_presets_by_installed_apps and stamps the Nextcloud-Astrolabe/<version> UA on its outbound calls.

BoardUpdatedEvent carries only boardId (no card list), so the parser logs delivery at INFO and returns None; the polling scanner reconciles the affected cards.

Caveats

  • Requires Deck ≥1.18.x — IWebhookCompatibleEvent is in v1.18.0-beta.* but not yet in any stable release. The preset is hidden by filter_presets_by_installed_apps when the Deck app isn't installed, but admins enabling it against an older Deck will get a 400 from webhook_listeners ("not an event class compatible with webhooks").

Test plan

  • uv run pytest tests/unit/test_webhook_endpoint.py tests/unit/test_webhook_presets.py -v — 5 new tests pass (deck create/delete/board-update + symmetric fail-open tests for missing card.id / node.id)
  • Full unit suite: 1035 passed, 1 skipped
  • uv run ruff check && uv run ruff format --check && uv run ty check -- nextcloud_mcp_server
  • Manual end-to-end smoke against mcp-login-flow:
    • Patched container with upstream ACardEvent.php + BoardUpdatedEvent.php (PR #7910 source)
    • Enabled Deck Sync via astrolabe admin UI → 4 webhooks registered
    • Created / updated / deleted a Deck card via mcp__nextcloud-stdio__deck_*
    • Confirmed Webhook queued deck_card_7 (index|delete) + Indexed/Deleted deck_card_7 for admin in MCP logs
    • Confirmed Nextcloud-Astrolabe/0.14.1 UA on every astrolabe-originated /api/v1/* request

This PR was generated with the help of AI, and reviewed by a Human

cbcoutinho and others added 2 commits May 21, 2026 09:03
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>
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>
@sonarqubecloud
Copy link
Copy Markdown

@claude
Copy link
Copy Markdown

claude Bot commented May 21, 2026

Claude finished @cbcoutinho's task in 3m 0s —— View job


PR Review: feat(webhooks): add Deck card sync preset with vector indexing

  • Gather context from PR description and changed files
  • Read changed source files
  • Review code quality, bugs, security, and test coverage
  • Post final review

Overall this is a clean, well-scoped PR. The design is sound, the comments explain the non-obvious decisions, and the test coverage mirrors what the code actually does. A few items worth addressing before merge:


Important

stack_id-only metadata never reaches the fast lookup path in processor.py

In webhook_parser.py:138 only stack_id is stored in metadata:

metadata = {"stack_id": stack_id} if stack_id is not None else None

But processor.py:340-343 requires both board_id and stack_id to take the O(1) fast path:

if (
    doc_task.metadata
    and "board_id" in doc_task.metadata
    and "stack_id" in doc_task.metadata
):

Since CardCreatedEvent/CardUpdatedEvent/CardDeletedEvent payloads don't carry board_id, every webhook-triggered deck card task always falls through to the full board+stack iteration (O(boards × stacks)). The comment in the parser documents this correctly ("processor.py falls back to iterating boards/stacks if it's missing"), but the stack_id in metadata is then effectively dead — processor.py never uses a partial hint.

Two options:

  1. Simplify now: drop stack_id from webhook-generated metadata entirely (set metadata=None always). Less misleading, no behaviour change. Fix this →
  2. Future optimization: add a stack_id-only fast path to processor.py that narrows iteration to just the known stack. Worth tracking as a follow-up.

logger.info fires on every /api/v1/* and /webhooks/nextcloud request

app.py:2488 logs at INFO level unconditionally for every matching request:

logger.info("%s %s from %s", request.method, path, ua, ...)

In a busy deployment — many card creates/updates through MCP tools, regular scanner webhooks — this doubles INFO log traffic for those endpoints. The comment says the goal is to attribute calls to a specific Astrolabe build, but (none) entries (non-Astrolabe callers) will dominate and add noise.

Consider logger.debug for non-Astrolabe UAs (or for all paths), or gate on a specific UA prefix: Fix this →


Nits

Missing test for CardUpdatedEvent (tests/unit/test_webhook_endpoint.py)

The five new tests cover create, delete, board-update, and two fail-open cases — but CardUpdatedEvent (the most common in practice) isn't explicitly tested. It falls through the same _DECK_CARD_EVENTS frozenset branch as CardCreatedEvent, so behaviour is covered, but an explicit test would help catch regressions if the operation mapping ever diverges. Fix this →

Deck event class strings duplicated across two modules

DECK_EVENT_CARD_* are defined in webhook_presets.py and _DECK_EVENT_CARD_* are redefined identically in webhook_parser.py. The strings match and tests verify both sides, but a future rename would need updating in two places. (The same pattern already exists for file events — this isn't introduced by this PR, just worth flagging as tech debt.)


What's working well

  • BoardUpdatedEvent fan-out design is clean: log delivery, return None, let the polling scanner reconcile. The test correctly validates the "ignored" response.
  • Fail-open handling is symmetric: both the notes path (missing node.id) and the deck path (missing card.id) now have explicit tests.
  • _DECK_CARD_EVENTS frozenset is a neat way to express the three indexable event types without repetition.
  • The webhook secret HMAC check (compare_digest) pre-exists and correctly applies to deck events too.
  • The @dataclass DocumentTask.metadata field comment is accurate (board_id/stack_id for deck_card).

@cbcoutinho cbcoutinho merged commit baf4d4d into master May 24, 2026
11 of 12 checks passed
pi0n00r added a commit to pi0n00r/nextcloud-mcp-server that referenced this pull request May 26, 2026
…ith posture preservation

Selective merge of 10 upstream commits past pi0n00r/master (da23d51) to
upstream tip (25eb2e9). Catches up through tag v0.89.0 (bump commit
e4497c4) plus 4 follow-on commits (nextcloud-32 digest renovate +
internal upstream merge + the PR cbcoutinho#792 merge commit).

Conflict resolution (1):
  - third_party/astrolabe (modify/delete) — git rm (kept Astrolabe strip;
    upstream submodule pointer bump 904c793 inapplicable, same shape as
    the v0.88.3 resolution of 39d70cf)

Auto-merge handled cleanly: 9 remaining intersection files, including
client/contacts.py, client/vcard_parser.py, client/webdav.py,
models/contacts.py, server/contacts.py, server/oauth_tools.py — no
Class-1 surface required ours-vs-theirs resolution.

Upstream additions absorbed (dormant by configuration):
  - server/webhook_presets.py + vector/webhook_parser.py (PR cbcoutinho#809 Deck
    card sync preset with vector indexing; preset added to catalog,
    only fires if registered; vector path inactive — no Qdrant)
  - feat(api): log inbound User-Agent on management API and webhook
    receiver (local logging only, no phone-home; management routes
    dormant in single_user_basic, no webhooks registered against MCP)
  - tests/unit/test_webhook_endpoint.py + test_webhook_presets.py (new
    upstream tests for the above)
  - chore(deps): nextcloud-32 docker tag and digest renovate bumps
    (upstream compose, not our bridgette overlay)

Posture verification (paragraph 6 gauntlet, all PASS):
  - oauth_tools.py: 0 Astrolabe refs, 4 get_shared_storage, 0 Optional[
  - models/contacts.py: 5 typed fields present
  - vcard_parser.py: present (441 lines)
  - auth/astrolabe_client.py, third_party/astrolabe, CLA.md, astrolabe.svg,
    both app-hooks scripts: all absent
  - .gitmodules: 0 astrolabe entries (only oidc and notes)
  - AGENTS.md and LICENSE.md (7 CLA/AGPL refs): present
  - pyproject.toml version: 0.89.0
  - py_compile: 9/9 critical files clean (incl. new webhook_presets and
    vector/webhook_parser)

Single_user_basic posture, ENABLE_LOGIN_FLOW=false dormancy, and
no-Qdrant runtime all preserved. Multi-user gate: no commit in the
range introduces multi-user surfaces. No new Tier-1 Astrolabe
landmines.

Safety tag: pre-upstream-merge-10-da23d512.

Note: commit message amended 2026-05-25 post-push after initial commit
used a stale /tmp/merge-msg.txt referencing v0.88.3. The tree content
and merge structure were correct; only the commit message string was
wrong. Amended via git commit --amend; tag v0.89.0 force-recreated at
the new commit SHA; force-pushed master + tag per Lesson 3.
pi0n00r added a commit to pi0n00r/nextcloud-mcp-server that referenced this pull request May 26, 2026
The prior commit b7bd18b is the v0.88.3 → v0.89.0 selective merge but
its header line incorrectly reads "Merge upstream cbcoutinho/master at
v0.88.3". This was an authoring slip: the commit-message file at
/tmp/merge-msg.txt was left over from the prior cycle (v0.86.4 → v0.88.3,
da23d51), the scp of the corrected message file failed silently due to
a sandbox-side path issue, and git fell back to the stale template.

Tree content of b7bd18b is correct (the actual v0.89.0 merge):
  - merge-base: 25eb2e9 (upstream PR cbcoutinho#792 merge tip)
  - 10 upstream commits absorbed: Astrolabe submodule pointer bump
    (resolved git rm), Deck webhook preset + vector indexing PR cbcoutinho#809
    (dormant), User-Agent logging feat(api) (dormant management routes),
    test fixtures, nextcloud-32:32.0.9 docker digest renovate, version
    bump 0.88.3 → 0.89.0.
  - §6 grep gauntlet: ALL PASS at the time of merge.
  - pyproject.toml version: 0.89.0
  - py_compile: 9/9 critical files clean.

Resolution choice: leave b7bd18b in history with its wrong header
string (branch protection blocked the obvious force-push amend), and
add this no-op correction commit on top so future readers grepping
for "v0.89.0" find both the merge and the explanatory note. The
v0.89.0 git tag points at b7bd18b (the merge commit on this branch
lineage). Audit trail for the slip is recorded in
Documents/Projects/NC/v0.86.3-merge-notes.md Cat L.

No tree change. --allow-empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant