fix(visual_review): allow tolerate via personal API keys

[pauldambra]

Problem

The mark_tolerated action on RunViewSet (POST /api/projects/:id/visual_review/runs/:run_id/tolerate/) was missing from the viewset's scope_object_write_actions. As a result, any request authenticated with a personal API key (or OAuth token) was rejected at the API-scope permission layer with This action does not support personal API key access.

In practice this meant the visual-review MCP integration could approve and quarantine snapshots but could not tolerate them — the only one of the three write actions that hadn't been added to the scope list. It surfaced when trying to mark a couple of benign unrelated snapshot diffs as tolerated via the MCP.

Changes

• Add mark_tolerated to RunViewSet.scope_object_write_actions, so the action derives the visual_review:write required scope and is reachable from personal API keys / OAuth tokens with that scope.
• Add three parameterised regression tests:
  • session auth can tolerate (200),
  • personal API key with visual_review:write can tolerate (200) — the regression,
  • personal API key with only visual_review:read is still forbidden (403).

No behaviour change to the tolerate logic itself; this is purely the permission-scope wiring.

How did you test this code?

I'm an agent. I ran the new tests locally via hogli test:

• products/visual_review/backend/tests/test_presentation.py::TestRunViewSet -k tolerate — 3 passed.

The write-scope test fails before the one-line scope_object_write_actions change (403) and passes after (200), confirming it pins the regression.

Automatic notifications

• Publish to changelog?
• Alert Sales and Marketing teams?

Docs update

No docs changes — internal API-scope fix.

🤖 Agent context

Authored by PostHog Code. Found while triaging a separate user-interviews PR's visual-review run: the MCP tolerate tool returned HTTP 403 This action does not support personal API key access, while approve worked. Traced it to posthog/permissions.py:APIScopePermission requiring the action to appear in the viewset's read/write action lists to derive a required scope; mark_tolerated was absent from RunViewSet.scope_object_write_actions whereas the analogous quarantine/unquarantine on RepoViewSet and approve/auto_approve on RunViewSet were present. One-line list addition plus regression coverage.

---

Created with PostHog Code

[pauldambra]

• fix(visual_review): allow tolerate via personal API keys #60749 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Simple one-line fix adding a missing action to the write-actions list so personal API keys with the right scope can reach the mark_tolerated endpoint. Well-covered by three targeted regression tests.

[greptile-apps]

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
products/visual_review/backend/tests/test_presentation.py:233-260
Per the team's preference for parameterised tests, the two personal-API-key tests share an identical structure — the only differences are the scope string and the expected status code. They can be collapsed into a single parameterised test, which also makes it straightforward to add future scope variants without duplicating the body.

```suggestion
    @pytest.mark.parametrize(
        "scope,expected_status",
        [
            ("visual_review:write", status.HTTP_200_OK),
            ("visual_review:read", status.HTTP_403_FORBIDDEN),
        ],
    )
    def test_mark_tolerated_via_personal_api_key(self, scope: str, expected_status: int):
        """Regression: tolerate must be reachable from personal API keys with write scope (MCP path)."""
        run_id, snapshot_id = self._changed_snapshot_for_tolerate()
        key = self.create_personal_api_key_with_scopes([scope])

        self.client.logout()
        response = self.client.post(
            f"/api/projects/{self.team.id}/visual_review/runs/{run_id}/tolerate/",
            {"snapshot_id": snapshot_id},
            format="json",
            HTTP_AUTHORIZATION=f"Bearer {key}",
        )

        assert response.status_code == expected_status, response.json()
```

_{Reviews (1): Last reviewed commit: "fix(visual_review): allow tolerate via p..." | Re-trigger Greptile}

[pauldambra]

QA Swarm review complete. See inline comments and the top-level summary.

[pauldambra]

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: qa-team (specialists + generalists), paul-reviewer, xp-reviewer, security-audit

Verdict: 💬 APPROVE WITH NITS

A correct, minimal, fail-closed-preserving authz fix. All four reviewers approve. Adding "mark_tolerated" to RunViewSet.scope_object_write_actions is right: APIScopePermission._get_required_scopes keys off the action method name (view.action = mark_tolerated), not the tolerate url_path, and mark_tolerated is a mutation that correctly maps to visual_review:write. The change only widens which auth methods can reach an action already exposed via session auth — no new unauthenticated surface, no scope downgrade — and the read-scope-403 test proves write is genuinely required.

Key findings

• ⚪ NIT (convergent x3) — the three test_mark_tolerated_* tests are the same shape; candidate for @parameterized.expand over (auth, expected_status). (paul's counter: differing auth setup may read better as named tests — judgment call.)
• ⚪ NIT (xp) — helper docstring restates the method name/signature; repo convention is no doc comments in Python tests.
• ⚪ NIT (xp) — the why-comment on result = CHANGED earns its place; optional _force_changed() extraction.
• 🟢 LOW (paul, systemic, out of scope) — bug class: a write @action missing from both scope lists falls through to _get_required_scopes() → None, which is a silent 403 for API-key callers. Fails closed (good) but silently (bad for discovery). A cheap CI guard asserting every action on a scoped viewset is classified in exactly one list would turn future occurrences into red CI. Not blocking; flagged because this class has now bitten once. (posthog/permissions.py isn't in this diff so this couldn't be an inline comment.)
• 🟢 LOW (paul) — tolerate now mutates via personal API key; confirm cross-team coverage exists. Security reviewer confirmed team_id is route-derived and cross-tenant ids 404, and the suite uses VisualReviewTeamScopedTestMixin — so this is likely already covered.

Convergence

The parameterize nit was raised independently by qa-team, paul, and xp — highest-confidence finding, and it matches a hard repo convention.

Reviewer summaries

Reviewer	Assessment
🔍 qa-team	APPROVE — correct, minimal, fail-closed-preserving; enumerated all RunViewSet actions, none left orphaned. One parameterize nit.
👤 paul	🚢 — right fix, good both-directions tests; worth a systemic CI guard for the bug class and a glance at cross-team coverage.
📐 xp	Ship it — clean, tests express intent; nits are docstring + why-comment per the no-comments convention.
🛡 security-audit	No findings — visual_review:write correct, team scoping intact (no IDOR), no auth bypass; read-scope keys still 403.

---

Automated by QA Swarm — not a human review

[stamphog]

Retaining stamphog approval — delta since last review classified as trivial_paths.

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

[stamphog]

Review agent failed after 3 attempts — needs human review.

[stamphog]

Review agent failed after 3 attempts — needs human review.

[stamphog]

Review agent failed after 3 attempts — needs human review.

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-05-30 15:20 UTC	Run
prod-us	✅ Deployed	2026-05-30 16:46 UTC	Run
prod-eu	✅ Deployed	2026-05-30 15:33 UTC	Run