Permissioning audit follow-ups (from PR #1985)

[JSv4]

Durable tracking for the out-of-scope items surfaced during the 2026-06 permissioning audit and PR #1985's review rounds. Each was verified against code; none is a regression introduced by that PR. Ordered roughly by security relevance.

Parity / security posture (pre-existing)

1. Annotation-creator privacy parity edge. AnnotationQuerySet.visible_to_user's privacy gate carries Q(creator=user) (annotated with a pointer to this issue; divergence pinned at CI time by test_annotation_creator_parity_gap_sentinel), but AnnotationManager.user_can's privacy recursion (_source_privacy_recursion_passes) has no creator exemption — an annotation creator lacking source READ sees their own analysis-/extract-rooted annotation in lists while user_can(READ) denies it. Relationships handle this deliberately (creator short-circuit mirrored by the queryset gate). Fixing annotations means choosing a semantic, flipping both sentinel assertions together, and adding the fixture to the invariant matrix.
2. ChatMessage anonymous branch lacks the THREAD restriction. ChatMessageQuerySet.visible_to_user anonymous path filters conversation__is_public=True without conversation_type=THREAD, while ConversationQuerySet.visible_to_user is THREAD-only for anonymous — a public CHAT's messages would be anonymous-visible while the conversation itself is hidden (opencontractserver/conversations/models.py).
3. Conversations querysets miss group-level guardian grants. ConversationQuerySet / ChatMessageQuerySet consult user-level object permissions only — same filter/check parity class PR Close permissioning-guide audit gaps: relationship privacy, anonymous extracts, group grants #1985 fixed for annotations/relationships/extracts.
4. BaseVisibilityManager.visible_to_user broad exception fallback. except (ImportError, Exception) (opencontractserver/shared/Managers.py ~line 339) silently downgrades to creator/public filtering on ANY error — under-disclosing (drops guardian rows) and hiding programming errors. Intended catch is (ImportError, LookupError). Needs its own change since tightening the envelope may surface latent errors suite-wide.
5. ExtractType has no get_queryset override. Related-field traversal (e.g. DatacellType.extract) relies on parent gating rather than ExtractManager.visible_to_user. Consider a get_queryset guard for defense-in-depth after the PR Close permissioning-guide audit gaps: relationship privacy, anonymous extracts, group grants #1985 anonymous lockdown.
6. DocumentRelationshipService.get_visible_relationships pre-computed myPermissions under-report. The annotate block sets _can_update/_can_delete via a creator-only Case (documents/services/relationships.py ~lines 263-280), while user_has_permission computes MIN(source_doc, target_doc, corpus) via user_can — a guardian-UPDATE-granted non-creator sees the row as read-only in the UI but passes the actual mutation gate. Fail-safe direction (under-report), but the same myPermissions-mirror principle PR Close permissioning-guide audit gaps: relationship privacy, anonymous extracts, group grants #1985 fixed for structural rows should apply here.

Correctness (pre-existing, non-permissioning)

7. Datacell._validate_manual_entry AttributeError. self.column.validation_config.get("required") runs before the config = ... or {} guard (opencontractserver/extracts/models.py ~line 397); validation_config=None raises.

Code quality / API

8. analysis_id == 0 sentinel in RelationshipService.get_document_relationships / frontend "__none__" mapping — replace the magic number with a named constant or a user_only: bool parameter (touches the GraphQL contract).
9. require_permission truthiness inversion ("" on grant, denial string on deny) is documented but remains a copy-paste footgun; consider a rename like permission_denied_reason with a deprecation shim.
10. Mention-autocomplete resolvers (config/graphql/search_queries.py) still inline guardian get_objects_for_user — legal under E001's three-token scan, documented as an exception in the guide; migrate into a service.
11. Weak assertions in legacy test_query_optimizer_methods.py (assertGreater(count, 0) where exact counts are knowable) — tighten when next touching that file.
12. source_visibility.py micro-dedup: visible_analyses_for + visible_extracts_for each embed their own user.groups subselect; an optional precomputed user_group_ids= parameter would collapse them if profiling ever shows it matters (they compile into one SQL statement today — no extra round-trips).
13. Guardian permission-codename string literals repo-wide. "read_analysis" / "read_extract" / "read_corpus" (and the long-standing f"read_{model_name}" shapes in shared/QuerySets.py, extract_service.py, etc.) are bare literals everywhere; opencontractserver/constants/permissioning.py currently holds only cache attrs. Introduce codename constants/helpers in one repo-wide pass rather than per-call-site (CLAUDE.md rule 4) — flagged during PR Close permissioning-guide audit gaps: relationship privacy, anonymous extracts, group grants #1985 review but declined there to avoid diverging from the prevailing convention in a security PR.

Tooling

14. claude-review workflow malfunction: during PR Close permissioning-guide audit gaps: relationship privacy, anonymous extracts, group grants #1985 it repeatedly edited its comment in place with non-review content (verbatim CLAUDE.md, then setup.cfg, then several "test"/"placeholder" bodies) between real review bodies — looks like a prompt/templating bug in the workflow. It also re-reviews every push from scratch, recycling already-addressed findings (e.g. requesting tests/issues that already exist by name, or quoting stale versions of files).

Context and dispositions: see the review threads on PR #1985.