feat(e2e): AGX1-325 — agent_api_keys authz black-box suite by dm36 · Pull Request #276 · scaleapi/scale-agentex

dm36 · 2026-06-04T18:46:21Z

Summary

End-to-end FGAC tests for the agent_api_keys routes. Black-box: every test hits the real scale-agentex HTTP API as one user identity, then verifies the resulting state in SpiceDB via a separate spark-authz client.

Models the KB suite from scaleapi#142983 — same conftest layout, same cleanup tracker (REST-then-SpiceDB fallback), same SparkAuthzClient (copied verbatim, it's repo-agnostic). The Agentex-specific client and four test files are new.

Linear: AGX1-325.

AGX1-325 deliverables → tests

Bullet	Test file
dual-write registers on create	`test_api_key_create.py`
without permission, 404 on get	`test_api_key_get.py`
filtered list	`test_api_key_list.py`
dual-write deregisters on delete	`test_api_key_delete.py`

Each test creates a fresh agent + api_key as user_a, then asserts wire behavior. The negative paths use a same-tenant user_b who has no grants.

Scope

user_a (owner) + user_b (same tenant, no permission). Per the kickoff: minimize cross-user complexity for v1.
Not in this PR: cross-tenant outsider, service identity, retention/cleanup endpoints, two-factor-via-SpiceDB schema correctness (lives in spark-authz's own suite).

Setup

cd scripts/spark-authz-e2e-tests
make install
cp config.json.example config.json
# Fill in headers / identity_ids / account_id for user_a + user_b in your target env.
make test

Out of scope

CI integration — these run on-demand against a live cluster, not in unit CI. Wiring them into a scheduled job is a separate decision once they pass against a stable dev env.
More resource types (events, tasks, messages, etc.) — additive tickets if/when needed.

Verified against pubsec-dev

Ran the suite against agentex.sgp-pubsec-dev.scale.com on 2026-06-04. Result: 3 passed / 11 failed / 1 skipped.

The pass set is the event suite from #277 (events delegate to parent-agent and don't depend on api_key dual-write). The 11 failures all trace to a single deployment-config blocker in pubsec-dev — not a defect in this PR's code.

The blocker: `agentex-auth` runs with `AUTH_PROVIDER=sgp`

Pubsec-dev's agentex-auth deployment has env var AUTH_PROVIDER=sgp. Every register_resource / deregister_resource call from scale-agentex's dual-write code lands on legacy SGP-authz instead of spark-authz. SGP-authz doesn't know about the api_key resource type, so:

POST /agent_api_keys            -> 200 (DB row created)
GET  /agent_api_keys/{just_id}  -> 422 wrapping 403 (agentex-auth → SGP-authz denied)

Flipping the fgac-* feature flags does not fix this — AUTH_PROVIDER is a process-level env var read at agentex-auth startup, not a per-account FF. To unblock:

Redeploy agentex-auth in pubsec-dev with AUTH_PROVIDER=spark. spark-authz is already deployed and reachable in the same cluster (ns/spark, svc/spark-authz, ports 50052/8090 — verified via port-forward + /healthz returning SERVING). No new infra needed.

After that flip, expect ~11/12 tests to pass against pubsec-dev. The 2 create-time SpiceDB-asserting tests in test_api_key_create.py additionally require the test user to have agent.create permission on the tenant; mitigated for all other tests by the new parent_agent fixture (falls back to a pre-existing agentex.agent_id in config.json).

What works today

The events suite from feat(e2e): AGX1-331 — event route authz black-box suite #277 passes 3/3 denied-path tests against pubsec-dev.
The api_key client + factories + cleanup pattern + parent_agent fallback are all verified to work against the real deployed API.
The graceful-skip wiring correctly de-risks running the suite in envs without a reachable spark-authz: no fixture errors, no interpreter crashes; everything that can't run is marked SKIPPED with a clear reason.

Run recipe used

config.json: pubsec-dev API key + EGP account_id as x-selected-account-id (the agentex auth gateway expects the EGP shape) + agentex.agent_id set to a pre-existing agent the user has api_key.create on (because the user lacks agent.create on the tenant wildcard).
spark_authz.host: pointed at localhost:8090 via kubectl --context <pubsec-dev-aws-eks> -n spark port-forward svc/spark-authz 8090:8090. The probe in authz_reachable confirms /healthz returns SERVING before any SpiceDB-asserting test runs.

Greptile Summary

Introduces a black-box FGAC end-to-end suite for the agent_api_keys routes, modeled after the KB suite. Tests hit the real scale-agentex HTTP API and verify authorization state directly in SpiceDB via a SparkAuthzClient, covering dual-write on create/delete, owner 200 / non-owner 404 on get, and FGAC list filtering.

Four new test files (test_api_key_create.py, test_api_key_get.py, test_api_key_list.py, test_api_key_delete.py) plus an AgentexClient wrapper, CleanupTracker, and conftest.py with session-scoped clients and function-scoped factory fixtures.
The healthz probe now correctly gates on resp.is_success (2xx only) and SpiceDB-asserting tests skip gracefully when spark-authz is unreachable, avoiding confusing fixture errors.
The 404-path in test_non_owner_list_excludes_user_a_api_key now uses pytest.skip instead of a silent return, making un-exercised assertions visible in reports.

Confidence Score: 5/5

Safe to merge — all changes are test infrastructure under scripts/, with no production code touched.

The suite is well-structured test tooling. The healthz probe, graceful-skip wiring, and cleanup tracker all behave correctly. The one unresolved issue (missing agentex.agent_id in the example config) affects developer experience when running the suite in permission-constrained environments but causes no correctness problem in the tests themselves.

No files require special attention. The only nit is config.json.example missing the agentex.agent_id field that the parent_agent fallback depends on.

Important Files Changed

Filename	Overview
scripts/spark-authz-e2e-tests/conftest.py	Root conftest: session-scoped clients/credentials, function-scoped cleanup tracker, `parent_agent` factory with config-fallback, and healthz probe now correctly checks `resp.is_success` (2xx only)
scripts/spark-authz-e2e-tests/tests/test_api_key_create.py	Create dual-write tests: verifies owner `read`/`delete` in SpiceDB post-create and that same-tenant user_b has no permissions; both tests gate on `authz_client` so they skip gracefully when spark-authz is unreachable
scripts/spark-authz-e2e-tests/tests/test_api_key_get.py	GET tests: four cases (owner/non-owner × id/name) all clean; non-owner paths correctly assert 404 and not 403
scripts/spark-authz-e2e-tests/tests/test_api_key_list.py	List FGAC filter tests: 404-from-user_b path now uses `pytest.skip` (not silent `return`), making the skipped-assertion visible in reports
scripts/spark-authz-e2e-tests/tests/test_api_key_delete.py	Delete tests: deregister-in-SpiceDB test creates api_key directly (avoiding teardown race) but doesn't register a cleanup entry — api_key leaks if the pre-delete SpiceDB sanity assertion fails (previously flagged, still unfixed)
scripts/spark-authz-e2e-tests/clients/agentex_client.py	Thin httpx wrapper for scale-agentex routes; clean interface returning raw responses for caller-side status assertions
scripts/spark-authz-e2e-tests/clients/spark_authz_client.py	HTTP-transcoded gRPC client for spark-authz ResourceService; covers check/grant/revoke/create/delete/lookup; copied verbatim from KB suite as described in PR
scripts/spark-authz-e2e-tests/config.json.example	Example config template: missing the `agentex.agent_id` field used by the `parent_agent` fixture fallback — operators without `agent.create` permission have no documented path to configure it
scripts/spark-authz-e2e-tests/helpers/cleanup.py	LIFO cleanup tracker: exceptions are swallowed and logged, `execute` clears actions list preventing double-cleanup — solid utility
scripts/spark-authz-e2e-tests/Makefile	Well-structured Makefile with hierarchical test targets; no broken stub targets in this version

Sequence Diagram

sequenceDiagram
    participant T as Test (pytest)
    participant CA as AgentexClient (user_a)
    participant CB as AgentexClient (user_b)
    participant AX as scale-agentex
    participant AA as agentex-auth
    participant SDB as SpiceDB (via spark-authz)

    T->>CA: create_agent()
    CA->>AX: POST /agents/register-build
    AX->>AA: resolve principal
    AX->>SDB: register_resource(agent)
    AX-->>CA: "200 {id}"

    T->>CA: create_api_key(agent_id)
    CA->>AX: POST /agent_api_keys
    AX->>AA: resolve principal
    AX->>SDB: "register_resource(api_key, parent_agent=agent_id)"
    AX-->>CA: "200 {id, api_key}"

    Note over T,SDB: SpiceDB-asserting tests (skip if spark-authz unreachable)
    T->>SDB: check_permission(user_a, api_key, read) → true
    T->>SDB: check_permission(user_b, api_key, read) → false

    Note over T,AX: HTTP-only tests (always run)
    T->>CB: get_api_key(id)
    CB->>AX: "GET /agent_api_keys/{id}"
    AX->>SDB: check permission (user_b) → deny
    AX-->>CB: 404 (collapsed from 403)

    T->>CA: delete_api_key(id)
    CA->>AX: "DELETE /agent_api_keys/{id}"
    AX->>SDB: deregister_resource(api_key)
    AX-->>CA: 200

Comments Outside Diff (1)

scripts/spark-authz-e2e-tests/tests/test_api_key_delete.py, line 1207-1212 (link)

Unregistered api_key leaks if pre-delete assertion fails

The test intentionally bypasses the create_api_key factory to avoid a teardown race, but that means the freshly-created api_key has no cleanup entry. If the pre-delete check_permission_bool assertion fails, the api_key is never deleted and leaks in both the DB and SpiceDB, potentially contaminating subsequent runs. The factory teardown actually handles the 404 case gracefully (it checks not in (200, 204, 404)), so the race concern is minimal. If keeping the factory out is intentional, registering a raw cleanup.add(...) call after the create would prevent the leak without the race issue.

Prompt To Fix With AI

This is a comment left during a code review.
Path: scripts/spark-authz-e2e-tests/tests/test_api_key_delete.py
Line: 1207-1212

Comment:
**Unregistered api_key leaks if pre-delete assertion fails**

The test intentionally bypasses the `create_api_key` factory to avoid a teardown race, but that means the freshly-created api_key has no cleanup entry. If the pre-delete `check_permission_bool` assertion fails, the api_key is never deleted and leaks in both the DB and SpiceDB, potentially contaminating subsequent runs. The factory teardown actually handles the 404 case gracefully (it checks `not in (200, 204, 404)`), so the race concern is minimal. If keeping the factory out is intentional, registering a raw `cleanup.add(...)` call after the create would prevent the leak without the race issue.

How can I resolve this? If you propose a fix, please make it concise.

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
scripts/spark-authz-e2e-tests/config.json.example:27-32
The `agentex.agent_id` fallback that `parent_agent` relies on isn't shown in the example config. In environments where the test user lacks `agent.create` permission (e.g. shared dev clusters), every test using `parent_agent` hits `Failed to create agent: 403` without any hint that setting this field would avoid the problem. Adding the commented-out section makes the fallback discoverable.

```suggestion
  "test_settings": {
    "request_timeout_seconds": 30,
    "cleanup_on_success": true,
    "cleanup_on_failure": true
  },
  "_comment_agentex": "Optional: set agent_id to a pre-existing agent if the test user lacks agent.create permission on the tenant wildcard. parent_agent uses this instead of calling create_agent().",
  "agentex": {
    "agent_id": ""
  }
}
```

_{Reviews (4): Last reviewed commit: "fix(e2e): treat non-2xx /healthz respons..." | Re-trigger Greptile}

End-to-end FGAC coverage for the agent_api_keys routes. Black-box: every test hits the real scale-agentex HTTP API and verifies the resulting state in SpiceDB via a separate spark-authz client. Modeled after scaleapi#142983's KB suite. AGX1-325 deliverables (4 test files): - test_api_key_create.py — dual-write registers owner with parent_agent edge; non-owner has no permissions. - test_api_key_get.py — owner 200 (id + name); non-owner 404 (collapsed, not 403) on both id and name routes. - test_api_key_list.py — owner sees own; non-owner doesn't see user_a's api_key in the filtered list. - test_api_key_delete.py — delete deregisters from SpiceDB; non-owner delete returns 404 and leaves the row intact. Scope: same-tenant user_a (owner) + user_b (no permission). No cross- tenant or service-identity coverage here. Infra: clients/agentex_client.py (api_keys + agent CRUD), reused clients/spark_authz_client.py from the reference suite, conftest + cleanup that uses REST-then-SpiceDB fallback to prevent owner-tuple leakage.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Per the EGP precedent (the suite assumes services are already running), replace the one-paragraph Setup section with a concrete four-terminal recipe: spark-authz compose, agentex-auth, scale-agentex, then the test runner. Calls out that user_b must not be pre-granted on user_a's resources so the negative-path tests actually fail closed.

Carried over by mistake from the EGP suite at scaffolding time. Nothing in our suite calls it, and it imports from clients.egp_client which doesn't exist here — so it'd break on import anyway. AGX1-325/331 don't grant/revoke (the negative paths rely on user_b having NO role by default, not on a granted-then-revoked sequence). If a future ticket adds grant+verify tests, we'll write grant_api_key_access / grant_agent_access against the resource types we actually own.

Rename test-create/get/list/delete -> test-api-key-create/get/list/delete and add a test-api-key umbrella that runs all four. Future resource additions (events, tasks, messages, …) will follow the same prefix pattern, keeping the target list self-documenting as scope grows. README updated to match.

Set up the conventions for future resources before scope grows: - test-direct-resources: resources with their own SpiceDB type (currently api_key; agent and task slot in here later). - test-sub-resources: resources that delegate authz to a parent (currently event; state, message, tracker, checkpoint slot in here). - test-<resource>: all cases for one resource. - test-<resource>-<case>: one case for one resource. Aggregate groupings (DIRECT_RESOURCE_TESTS, SUB_RESOURCE_TESTS) are declared at the top so future PRs add a single line per resource rather than churning the target list. Added `make help` listing the convention. README's Run section rewritten to match. This commit lays the groundwork; AGX1-331 (PR #277) will pick up the new shape on rebase — its `test-events` target becomes `test-event` aligned with the singular-resource naming convention.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Some envs (e.g. sgp-pubsec-dev) run scale-agentex without the spark-authz HTTP frontend — raw SpiceDB only, no :8090 REST surface for direct permission assertions. Pre-this-commit, this would have broken every test in the suite at fixture-setup time, including pure-HTTP ones. Split the authz_client fixture into two: - authz_client (strict) — skips when spark-authz is unreachable. Take this only in tests that ASSERT against SpiceDB directly. - optional_authz_client — returns None when unreachable. Factory cleanup paths use this; cleanup falls back to REST-only when spark-authz isn't around (tuples may leak, but routes are the unit under test). authz_reachable is a session-scoped /healthz probe so the check runs once per session, not per-test. Net effect on pubsec-dev: - All HTTP-only tests run. - test_api_key_create (2 tests) + test_owner_delete_deregisters_in_spicedb skip with a clear reason. README updated with a "When spark-authz isn't reachable" section.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Found while running the suite against scale-agentex pubsec-dev: POST /agents -> 405 Method Not Allowed (route doesn't exist) POST /agents/register -> requires acp_url + makes an ACP handshake (would need a real ACP server to be running) POST /agents/register-build -> creates a BUILD_ONLY agent with just name + description; no ACP handshake; perfect for the e2e suite where we just need a parent resource for api_keys to hang off. Drop the stale acp_url/acp_type kwargs from the client's create_agent — register-build doesn't take them.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Three fixes surfaced by actually running the suite against pubsec-dev: 1. api_key_type enum is lowercase ("external", "internal", ...), not uppercase. Was getting 422 on every create. 2. Add a ``parent_agent`` fixture that resolves an ``agentex.agent_id`` from config.json if set, otherwise calls ``create_agent()``. In envs where the test user lacks ``agent.create`` on the tenant wildcard (most shared dev clusters), point at an existing agent the user can create api_keys under. Tests that just need *some* agent (get/list/ delete/event) take ``parent_agent``; the two create-time tests in ``test_api_key_create.py`` keep ``create_agent`` since they assert about the create-time path itself (and skip on pubsec-dev anyway, no spark-authz). 3. Tests rewired to use ``parent_agent`` where the agent is just plumbing, not under test. Result against pubsec-dev with one real API key + agentex.agent_id of an existing agent the user has api_key.create on: all 3 event tests pass, 1 skipped (no event seeding harness), spark-authz-asserting tests gracefully skip, and the api_key tests fail on the deployed env's incomplete FGAC pipeline (api_key create succeeds, but the dual-write to spark-authz isn't deployed there, so subsequent reads/lists/deletes fail authz). That's a deployment gap the suite correctly surfaces — not a defect in the tests.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Per Greptile review on #277: the test's previous name + docstring claimed to verify "BOTH query params are gated" by passing zero-UUIDs for both task_id and agent_id, but FastAPI evaluates the ``task_id`` Depends first and short-circuits before the agent_id gate runs. The test would still pass if the agent_id gate were removed entirely — it doesn't isolate either gate. Rename and rewrite the docstring to be honest about what it actually verifies: end-to-end route gating, no per-gate isolation. Independent isolation would require granting one resource but not the other in SpiceDB, which depends on a reachable spark-authz (out of scope today per Gap 1 in #276 — pubsec-dev runs raw SpiceDB without spark-authz).

Per Greptile review: ``make test-sub-resources`` and ``make test-event`` referenced ``tests/test_event_authz.py``, which only exists on #277. Running either target on this branch in isolation produced ``ERROR: not found: tests/test_event_authz.py`` and a non-zero exit. Remove ``EVENT_TESTS``, ``SUB_RESOURCE_TESTS``, ``test-sub-resources``, ``test-event``, plus their references in ``.PHONY``, the ``help`` block, and the README ``Run`` section. The Makefile's header comment retains the convention so #277 (and follow-up sub-resource tickets) can re-add the targets when their test files actually land.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Per Greptile review on #277: the test's previous name + docstring claimed to verify "BOTH query params are gated" by passing zero-UUIDs for both task_id and agent_id, but FastAPI evaluates the ``task_id`` Depends first and short-circuits before the agent_id gate runs. The test would still pass if the agent_id gate were removed entirely — it doesn't isolate either gate. Rename and rewrite the docstring to be honest about what it actually verifies: end-to-end route gating, no per-gate isolation. Independent isolation would require granting one resource but not the other in SpiceDB, which depends on a reachable spark-authz (out of scope today per Gap 1 in #276 — pubsec-dev runs raw SpiceDB without spark-authz).

…y exists #276's Greptile-driven cleanup removed test-event / test-sub-resources from the Makefile to avoid dangling targets that referenced a file which doesn't exist on that PR's branch. This PR adds the test file, so it now re-adds the matching targets + README references. After this commit (#277 in isolation): make test-direct-resources # api_key tests make test-sub-resources # event tests make test-api-key # all AGX1-325 cases make test-event # all AGX1-331 cases

Per Greptile review: ``test_non_owner_list_excludes_user_a_api_key`` silently returned on 404, making the test appear PASSED even though the FGAC list-filter assertion was never reached. If user_b consistently 404s in an environment (e.g. lacks read on the parent agent), the test would show all-green while never exercising the filter behavior it claims to verify. Replace the silent ``return`` with ``pytest.skip`` and a reason that points at the actual gap: user_b lacks ``read`` on the parent agent, so the route 404s before the list filter runs. Reports now show this as SKIPPED instead of PASSED, which is the honest outcome. To actually exercise the list filter, user_b would need read on the agent but no read on user_a's api_keys — which requires a working spark-authz round-trip we don't have today (see Gap 1 in #276's PR description for the deployment gap).

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Per Greptile review on #277: the test's previous name + docstring claimed to verify "BOTH query params are gated" by passing zero-UUIDs for both task_id and agent_id, but FastAPI evaluates the ``task_id`` Depends first and short-circuits before the agent_id gate runs. The test would still pass if the agent_id gate were removed entirely — it doesn't isolate either gate. Rename and rewrite the docstring to be honest about what it actually verifies: end-to-end route gating, no per-gate isolation. Independent isolation would require granting one resource but not the other in SpiceDB, which depends on a reachable spark-authz (out of scope today per Gap 1 in #276 — pubsec-dev runs raw SpiceDB without spark-authz).

…y exists #276's Greptile-driven cleanup removed test-event / test-sub-resources from the Makefile to avoid dangling targets that referenced a file which doesn't exist on that PR's branch. This PR adds the test file, so it now re-adds the matching targets + README references. After this commit (#277 in isolation): make test-direct-resources # api_key tests make test-sub-resources # event tests make test-api-key # all AGX1-325 cases make test-event # all AGX1-331 cases

…pi_keys Standalone smoke kit for verifying agent_api_keys on a deployed scale-agentex environment, no venv or pytest required. Each case is a single curl block with the expected response and a clear pass/fail criterion. Layout: - Cases 1–9: HTTP shape + validation (auth, OpenAPI, create happy path, enum case, required-field validation, duplicate-name conflict, missing / bogus auth). All should pass against pubsec-dev today. - Cases 10–14: owner FGAC paths (GET id, GET name, LIST, GET nonexistent, DELETE). All return 422 today because pubsec-dev's agentex-auth runs with AUTH_PROVIDER=sgp; flip to 200/404 once that's redeployed with AUTH_PROVIDER=spark. - Cases 15–17: tenant isolation — requires a second user's API key. Closing scorecard summarizes which cases pass today and which gate on the AUTH_PROVIDER=spark rollout, so the doc doubles as a rollout- verification checklist. Background section at the bottom names the exact blocker + the file path in the sgp repo where the AUTH_PROVIDER value lives.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Per Greptile review on #277: the test's previous name + docstring claimed to verify "BOTH query params are gated" by passing zero-UUIDs for both task_id and agent_id, but FastAPI evaluates the ``task_id`` Depends first and short-circuits before the agent_id gate runs. The test would still pass if the agent_id gate were removed entirely — it doesn't isolate either gate. Rename and rewrite the docstring to be honest about what it actually verifies: end-to-end route gating, no per-gate isolation. Independent isolation would require granting one resource but not the other in SpiceDB, which depends on a reachable spark-authz (out of scope today per Gap 1 in #276 — pubsec-dev runs raw SpiceDB without spark-authz).

…y exists #276's Greptile-driven cleanup removed test-event / test-sub-resources from the Makefile to avoid dangling targets that referenced a file which doesn't exist on that PR's branch. This PR adds the test file, so it now re-adds the matching targets + README references. After this commit (#277 in isolation): make test-direct-resources # api_key tests make test-sub-resources # event tests make test-api-key # all AGX1-325 cases make test-event # all AGX1-331 cases

Per Greptile review: ``httpx.get`` doesn't raise on non-2xx by default, so a spark-authz host returning 503 (running but unhealthy) made the probe report reachable. The fixture then built the client, and the first ``check_permission_bool`` call inside a test hit ``resp.raise_for_status()`` and threw an ``HTTPStatusError`` — tests that should have skipped cleanly instead errored out with a confusing traceback. Switch the probe to also require ``resp.is_success`` (2xx). Network- level errors still go through the existing ``HTTPError`` catch. Net effect: spark-authz must answer ``/healthz`` with 2xx for the SpiceDB-asserting tests to run; any other state (unreachable, 4xx, 5xx) falls through to the documented skip with the same clear reason.

Layered on top of the AGX1-325 api_keys suite (PR #276). Same conftest, same SparkAuthzClient, same cleanup pattern; just extends AgentexClient with /events methods and adds one test file. AGX1-331 scope: event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list. Tests (test_event_authz.py): - test_get_event_nonexistent_returns_404 — sanity: ItemDoesNotExist short-circuits before any authz fires. - test_list_events_without_agent_view_returns_404 — user_b lacks read on user_a's agent → DAuthorizedQuery collapses denial to 404 on agent_id. - test_list_events_without_task_view_returns_404 — symmetric on task_id → both query-param gates are exercised. - test_get_event_with_view_returns_200 — SKIPPED. No public POST /events, events come from ACP streaming. Skip reason makes the gap explicit so whoever later wires up an event-seeding harness can flip it to a real assertion. Out of scope: positive-path coverage that needs a test-only event seeding helper. The ticket asks for the denied paths, and that's what this delivers.

Per Greptile review on #277: the test's previous name + docstring claimed to verify "BOTH query params are gated" by passing zero-UUIDs for both task_id and agent_id, but FastAPI evaluates the ``task_id`` Depends first and short-circuits before the agent_id gate runs. The test would still pass if the agent_id gate were removed entirely — it doesn't isolate either gate. Rename and rewrite the docstring to be honest about what it actually verifies: end-to-end route gating, no per-gate isolation. Independent isolation would require granting one resource but not the other in SpiceDB, which depends on a reachable spark-authz (out of scope today per Gap 1 in #276 — pubsec-dev runs raw SpiceDB without spark-authz).

…y exists #276's Greptile-driven cleanup removed test-event / test-sub-resources from the Makefile to avoid dangling targets that referenced a file which doesn't exist on that PR's branch. This PR adds the test file, so it now re-adds the matching targets + README references. After this commit (#277 in isolation): make test-direct-resources # api_key tests make test-sub-resources # event tests make test-api-key # all AGX1-325 cases make test-event # all AGX1-331 cases

dm36 mentioned this pull request Jun 4, 2026

feat(e2e): AGX1-331 — event route authz black-box suite #277

Open

dm36 added 4 commits June 4, 2026 12:01

dm36 marked this pull request as ready for review June 4, 2026 22:16

dm36 requested a review from a team as a code owner June 4, 2026 22:16

greptile-apps Bot reviewed Jun 4, 2026

View reviewed changes

Comment thread scripts/spark-authz-e2e-tests/Makefile

Comment thread scripts/spark-authz-e2e-tests/tests/test_api_key_list.py Outdated

greptile-apps Bot reviewed Jun 5, 2026

View reviewed changes

Comment thread scripts/spark-authz-e2e-tests/conftest.py Outdated

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(e2e): AGX1-325 — agent_api_keys authz black-box suite#276

feat(e2e): AGX1-325 — agent_api_keys authz black-box suite#276
dm36 wants to merge 12 commits into
mainfrom
dhruv/agx1-325-e2e-api-key-authz

dm36 commented Jun 4, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

dm36 commented Jun 4, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

AGX1-325 deliverables → tests

Scope

Setup

Out of scope

Verified against pubsec-dev

The blocker: agentex-auth runs with AUTH_PROVIDER=sgp

What works today

Run recipe used

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dm36 commented Jun 4, 2026 •

edited by greptile-apps Bot

Loading

The blocker: `agentex-auth` runs with `AUTH_PROVIDER=sgp`