feat: clear errors for evaluator permission denials and add scope changes

[quinn-galileo]

Why

With metric RBAC on, the server returns 403 when a non-admin creates a global evaluator, or edits, deletes or re-scopes an evaluator without the owner or editor role on one of its projects. The SDK showed the generic "Your API key doesn't have access" message. This PR says what is needed and how to fix it, rewords client-side "not found" errors, and adds Evaluator.set_scope(). Ticket: https://splunk.atlassian.net/browse/SAO-9342

What changed

EvaluatorPermissionError (subclass of ForbiddenError, so existing handlers still work, exported from splunk_ao) replaces the generic 403 on evaluator operations. The server response body stays in the message. Example messages:

• Create, scope unset: "Only org admins can create global evaluators. Pass project_ids=[...] to scope the evaluator to your projects, or set SPLUNK_AO_PROJECT / SPLUNK_AO_PROJECT_ID so it defaults to one."
• Create, explicit is_global=True: "Only org admins can create global evaluators. Drop is_global=True and pass project_ids=[...] to scope the evaluator to your projects." (The env vars would not help here, so they are not mentioned.)
• Create, project scope: "Creating a project-scoped evaluator needs the owner or editor role on every project in project_ids (or on the current project when it is the default)..."
• Decision create scorer post: the create hint above plus " Decision evaluators may also not be enabled for your organization."
• Decision create version post: "The server refused to save the decision evaluator's configuration, so the SDK deleted the new evaluator (a warning with its id is logged if that cleanup failed). Check that decision evaluators are enabled for your organization." This differs from the create hints because the scorer post has already succeeded at that point, so the scope permission is not the cause. The scorer is still deleted.
• Update / delete: "Updating an evaluator needs the owner or editor role on one of its projects (org admin for a global evaluator)." ("Deleting" for delete.)
• set_scope: "Changing an evaluator's scope needs the owner or editor role on one of its current projects and on each project added or removed. Making it global, or changing the scope of a global evaluator, needs the org admin role."

Other changes:

• The create hint is chosen from the resolved scope returned by _resolve_create_scope, never from instance attributes. On the decision path, a 403 from the scorer post or the version post is translated, and a rejected version still deletes the new scorer.
• Evaluators.delete_evaluator and Evaluator.refresh say "not found, or not shared with your projects". Evaluator.get still returns None; its docstring says so.
• Evaluator.set_scope(project_ids=[...] | is_global=True) calls PUT /scorers/{scorer_id}/scope (full replace), validates before any request, and does not touch the create-time scope.
• LLM and code create paths raise ValueError(detail) on a 422 from the scorer post, as the decision path already did.
• README evaluator example and CHANGELOG entries.

A 404 from update, delete or set_scope stays NotFoundError. The server returns 404 (not 403) for evaluators outside the caller's projects on some paths, so no message promises a 403.

Where to start reading

src/splunk_ao/evaluators.py (hint helpers and _translate_forbidden), then Evaluator.set_scope and Evaluator.update in src/splunk_ao/evaluator.py, then tests/test_evaluator_permissions.py.

Testing

poetry run pytest tests/test_evaluator_permissions.py tests/test_evaluators.py tests/test_evaluator.py tests/test_decision_scorers.py -n 0 (274 passed) and poetry run invoke type-check (clean). All network is mocked.

Related PRs

Repo	PR	Purpose	Status
splunk/splunk-ao-python	#274	scope on create (parent of this PR)	draft
splunk/splunk-ao-python	this PR	clearer permission errors and set_scope	draft

🤖 Generated with Claude Code

[github-actions]

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

[quinn-galileo]

🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.

Verdict: request_changes — A 403 on the decision version post now gets an RBAC create hint. The scorer post has already succeeded at that point, so the hint misdiagnoses the documented feature-disabled 403.

Follow-ups

Suggested follow-up work that could be tracked as Jira tickets:

• src/splunk_ao/evaluator.py:685-688: Evaluator.delete() deletes by name through Evaluators.delete_evaluator(name=self.name), not by self.id. Under metric RBAC, several visible scorers can share a name. The loop can delete some of them and then raise a 403 on a sibling the caller cannot edit. Delete self.id directly through delete_scorer_scorers_scorer_id_delete instead.
• src/splunk_ao/evaluators.py:403-406: create_custom_llm_evaluator does not check the version-post response or clean up when it fails. A 422 or an exception there leaves an orphan scorer with no version. The decision path already handles this with _was_rejected and _delete_scorer_quietly. Reuse that pattern for the LLM and code create paths.

[quinn-galileo]

🟠 major (bug): This translates a 403 from the decision version post into EvaluatorPermissionError with the create-scope hint. That hint is wrong on this path:

• The scorer post at line 510 already succeeded with the same is_global / project_ids. So the caller already has the create permission for that scope.
• The docstring at line 482 says a 403 here means the decision feature is not enabled.
• The user sees "Only org admins can create global evaluators. Pass project_ids=[...]" or "needs the owner or editor role on every project". If they follow that advice, they get the same 403 again.

The LLM path (line 404) and the code path (evaluator.py:1540) do not translate version-post 403s. That makes this the inconsistent branch.

Options:

1. Re-raise the raw error, as before. This is the simplest fix and keeps the docstring true.
2. If a version-post 403 must be translated, use its own hint. For example: "The evaluator was created but its decision version was refused. Check that decision evaluators are enabled for your organization."

Update test_version_post_403_is_translated_and_the_scorer_is_still_deleted (tests/test_evaluator_permissions.py:205) to match the option you choose.

Suggested change

	except Exception as error:
	if self._was_rejected(error):
	self._delete_scorer_quietly(scorer.id)
	raise
	with _translate_forbidden(create_hint):
	raise
	if self._was_rejected(error):
	self._delete_scorer_quietly(scorer.id)
	raise

🤖 Generated by the Astra agent

[quinn-galileo]

Fixed in f310b37: the version-post 403 is still translated (it must be a clear SDK error) but now uses its own hint about the refused decision version, and the scorer is still deleted. Docstrings updated.

[quinn-galileo]

🟡 minor (bug): An explicit is_global=True gets the same hint as an unset scope. Part of that hint says "or set SPLUNK_AO_PROJECT / SPLUNK_AO_PROJECT_ID so it defaults to one." That advice cannot work here. _resolve_create_scope returns True, UNSET for is_global=True before it reads the environment (line 235). The env var is ignored, and the user hits the same 403. The test at tests/test_evaluator_permissions.py:99-100 asserts this wrong advice for the explicit_global case. Use a separate hint for True that says only "drop is_global=True and pass project_ids=[...]". Keep the env var advice for UNSET.

Suggested change

	def _create_denied_hint(scope_is_global: bool | Unset) -> str:
	"""Hint for a 403 on create, chosen from the scope that was actually sent."""
	return _SCOPED_CREATE_DENIED_HINT if scope_is_global is False else _GLOBAL_CREATE_DENIED_HINT
	def _create_denied_hint(scope_is_global: bool | Unset) -> str:
	"""Hint for a 403 on create, chosen from the scope that was actually sent."""
	if scope_is_global is False:
	return _SCOPED_CREATE_DENIED_HINT
	if scope_is_global is True:
	return _EXPLICIT_GLOBAL_CREATE_DENIED_HINT
	return _GLOBAL_CREATE_DENIED_HINT

🤖 Generated by the Astra agent

[quinn-galileo]

Declined: the task's acceptance criteria require the same hint for unset and explicit-global scope, naming both project_ids and the SPLUNK_AO_PROJECT variables. Flagged to the tech lead as a possible follow-up.

[quinn-galileo]

🔵 nit (documentation): The example right below calls set_scope. But this sentence says "changing" needs the role on one of the evaluator's projects. The PR's own hint says a scope change needs the role on each project added or removed, and org admin to make it global (evaluators.py:106-109). Say "updating or deleting" here, and state the scope-change rule separately.

🤖 Generated by the Astra agent

[quinn-galileo]

Fixed in f310b37: README now separates update/delete from scope changes.

[quinn-galileo]

src/splunk_ao/evaluators.py:482-482 (line not in diff)

🟡 minor (documentation): This docstring now contradicts the code. A 403 from the version post no longer surfaces as a plain SplunkAOAPIError. It surfaces as EvaluatorPermissionError with an RBAC hint (lines 527-528). The same text is in the module-level create_decision_evaluator at line 754. If you keep the translation, reword both docstrings. If you drop it, as the comment on line 528 suggests, no change is needed.

🤖 Generated by the Astra agent

[quinn-galileo]

Fixed in f310b37: both docstrings reworded.

[quinn-galileo]

🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.

Verdict: approve — f310b37 fixes the major version-post finding. What remains is minor: the new hint can claim a deletion that did not happen.

[quinn-galileo]

🟡 minor (bug): The hint says "so the new evaluator was deleted" without condition. But _delete_scorer_quietly is best-effort (evaluators.py:555-569). If the cleanup delete raises, or returns HTTPValidationError or None, it only logs a warning and returns. In that case the scorer still exists, and the message tells the user it is gone. The user then sees a leftover evaluator with the same name and no version.

Options:

1. Reword the hint so it does not assert the outcome. For example: "The server refused the decision version request. The SDK tried to delete the new evaluator."
2. Make _delete_scorer_quietly return a bool. Then pick the hint text from that result, and name the scorer id when the cleanup failed.

Suggested change

	_DECISION_VERSION_DENIED_HINT = (
	"The server refused the decision version request, so the new evaluator was deleted. "
	"Check that decision evaluators are enabled for your organization."
	)
	_DECISION_VERSION_DENIED_HINT = (
	"The server refused the decision version request, and the SDK tried to delete the new evaluator. "
	"Check that decision evaluators are enabled for your organization."
	)

🤖 Generated by the Astra agent

[quinn-galileo]

Fixed: reworded so it does not assert the cleanup outcome.

[quinn-galileo]

🤖 This review was generated by the Astra agent (claude-opus-5-5). It may contain mistakes.

Verdict: approve — b1940be and e233c2b fix every earlier finding in code. What remains is minor: one stale docstring and a PR description that no longer matches the messages.

General Comments

• 🟡 minor (documentation): The PR description quotes messages that the code no longer produces after e233c2b:

• It gives one hint for "Create, unset or global scope". Explicit is_global=True now gets _EXPLICIT_GLOBAL_CREATE_DENIED_HINT (evaluators.py:106-109), which does not mention SPLUNK_AO_PROJECT.

• It quotes "You can't update this evaluator...". _edit_denied_hint now returns "Updating an evaluator needs the owner or editor role on one of its projects..." (evaluators.py:126-130).

• The quoted set_scope hint leaves out the new "one of its current projects" condition (evaluators.py:117-121).

• The description does not mention _DECISION_ENABLED_HINT, which is appended to decision scorer-post 403s (evaluators.py:523).

Update the description so reviewers check the real messages.

[quinn-galileo]

🟡 minor (documentation): This docstring for DecisionEvaluator.create describes a 403 only as a create-scope problem. The code can also raise EvaluatorPermissionError in two other ways:

• After the version post is refused. That error uses _DECISION_VERSION_DENIED_HINT, and the SDK tries to delete the scorer (evaluators.py:538-543).
• From the scorer post with the feature-gate note appended (evaluators.py:523).

The Evaluators.create_decision_evaluator docstrings already state the 403 path, after f310b37. Align this one with them.

Suggested change

	EvaluatorPermissionError: If the API answers 403: org admins only for a global evaluator, owner or
	editor role on the projects for a scoped one.
	EvaluatorPermissionError: If the API answers 403: org admins only for a global evaluator, owner or
	editor role on the projects for a scoped one. Decision evaluators may also not be enabled for the
	organization. A 403 on the version request deletes the new scorer before the error is raised.

🤖 Generated by the Astra agent

[quinn-galileo]

Fixed: docstring now covers the feature-gate note and the version-request 403.