[johnny4young]

Summary

MCP get_clip, paste_stack and now create_pin authorize from one GRDB read snapshot (readForMCP), and search_clips is bound to the exact reviewed clip revisions of a selected-context grant. Details of the original finding and fix are kept below.

Review follow-ups

• bf0c61d — fix(mcp): withhold vanished payloads and pin from the policy snapshot
  • get_clip now reports contentWithheld: true (instead of false with no body) when a blob is reclaimed between the authorized snapshot and the byte read.
  • create_pin takes visibility, context membership and sensitivity from readForMCP (called with metadata scope so it never selects payload columns). The now-unused split-read helpers visibleItem/isInsideContext are removed.
  • New MCPSnapshotPolicyTests: vanished payload ⇒ withheld; create_pin obeys the snapshot's sensitive verdict even when a separate read would say otherwise; create_pin asks the snapshot only for metadata scope.

Decisions taken

• A nil payload after an authorized snapshot is reported as withheld, not as an error: the clip was authorized, only its bytes are gone, and the ledger still records one returned result.
• create_pin keeps its write (setPinned/assign) outside the snapshot; only the policy decision is made consistent.

How it was tested (this round)

• swiftc -parse and swift-format lint --strict (Swift 6.0.3 container) on changed files. GanchoMCP depends on GRDB/SQLCipher and Apple frameworks, so it was not compiled locally; relies on macOS CI (see below).

Merge order

Independent of the storage stack; merge before the v0.10.0 release PR #193. If #193 merges first, move this PR's CHANGELOG bullets back under [Unreleased].

CI: macOS jobs for the new head were queued when this description was written; check the PR checks tab.

---

Original description (state at c90e07d; superseded where it conflicts with the sections above):

Finding

A selected-context grant records the exact approved contextRevision per clip. get_clip and paste_stack checked this boundary, but search_clips only narrowed by clip IDs. Editing an approved clip could therefore expose its new title/preview in search even though its body was correctly denied by get_clip.

Fix

• Resolve still-approved revisions before issuing the store search, so changed clips cannot consume the caller's result limit
• Intersect revision keys with the explicit selected IDs, never expand the selection
• Keep an empty revision result as an empty ID filter, never an ambient-history query
• Recheck returned revisions to reject a clip changed between the preliminary read and search
• Preserve legacy packs without revision binding

Consistent content reads

A deeper review also confirmed the split-read race: get_clip/paste_stack checked metadata, then awaited a separate content lookup. They now consume a GRDB operation that checks visibility, reviewed revisions, membership and sensitive policy within the same database snapshot used to select the payload. Metadata-only reads never select payload columns; immutable blob identity comes from the captured row, without another ID lookup.

Regression coverage

• A newly edited clip cannot expose its replacement preview under either all-content or metadata scope
• A changed first match cannot displace an unchanged result when limit=1
• Empty and wholly stale revision maps return zero results
• Extra revision keys cannot authorize clips outside selected IDs
• Access-log result counts reflect only returned, authorized results
• A real-store legacy split-read control demonstrates an intervening edit returning unreviewed bytes
• Both tools reject an injected edit before their new snapshot begins
• A real WAL writer commits an edit after policy checks but before the payload SELECT; a SQL trace gate proves the active read still returns only its approved generation, while the next read rejects the changed clip
• SQL tracing confirms metadata-only MCP reads still leave payload columns on disk

Verification

• Exact final head c90e07d4298eaebd95439f76fb13b7d65a6c1900 passed CI run 37371083389
• Both swift-format and SwiftLint passed
• 1,506 Swift tests in 235 suites passed; production package-source coverage is 83.9%, above the 80% floor
• The real-WAL snapshot regression passed: an edit committed between policy and payload selection cannot replace the authorized generation
• The legacy split-read positive control reproduced unreviewed bytes, and both corrected tools passed the intervening-edit regressions
• StoreKit purchase/entitlement tests and macOS/iOS app builds passed
• The missing test migration SPI import was corrected by a one-line test-only commit; the complete final-head run above validates the expanded snapshot implementation
• git diff --check passes. Local native execution is unavailable on the Linux executor; native evidence is hosted macOS CI
• Tests use disposable synthetic stores. No real clipboard or Keychain data was accessed
• No merge, release, deployment, schema or dependency change

Human-review readiness

Scoped implementation and regression coverage are complete at unchanged head c90e07d. Fresh verification confirms both required native jobs and GitGuardian successful, the expected non-blocking beta-canary skip, and no pending discussion comments, submitted reviews or unresolved inline review threads. Review readiness does not authorize merge, release or deployment.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ExmnfUngrDYjAheGvYwMnn

---

Generated by Claude Code

[johnny4young]

Summary: MCP get_clip and paste_stack now authorize and read in one GRDB read snapshot (readForMCP). search_clips narrows includedIDs to clips whose reviewed contextRevision still matches before LIMIT, then re-checks the hits.

Verdict: ready. Nothing blocking.

What I verified:

• readForMCP keeps the old decision order and semantics: missing → context (!requiresContextPack when there is no explicit pack) → sensitive → metadata scope → boards scope (pinned or on a board) → content. Board membership is still only queried when needed. Archived and expired rows read as missing, as before.
• searchableIDs returning an empty set is safe: GRDBClipboardStore+SearchFilters turns an empty includedIDs into AND 0, not "no filter". Revision keys are uuidString both in SelectedContext.grant and here, so they compare correctly.
• ClipRow: Sendable is needed to return the payload row out of the read closure. MCPClipStore gains a requirement, and the only conformers are GRDBClipboardStore and the test fake.

🟡 Nits, none blocking:

• MCPAccessStore.swift, readForMCP: blob bytes are read via content(from:) after the snapshot closes. That is fine for content-addressed blobs. If the orphan sweep removes the file in between, though, get_clip returns content: nil with contentWithheld: false, which looks like an empty clip. Consider treating a nil binary payload as .missing.
• createPin still uses visibleItem + isInsideContext as separate awaits. It only toggles a pin, so the impact is minimal, but it is now the one remaining non-snapshot path.
• CHANGELOG: if #193 merges first, these entries end up under [0.10.0]. Make sure that is the intent.

CI is green.

---

Generated by Claude Code