Skip to content

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

Open
dm36 wants to merge 4 commits into
dhruv/agx1-325-e2e-api-key-authzfrom
dhruv/agx1-331-e2e-event-authz
Open

feat(e2e): AGX1-331 — event route authz black-box suite#277
dm36 wants to merge 4 commits into
dhruv/agx1-325-e2e-api-key-authzfrom
dhruv/agx1-331-e2e-event-authz

Conversation

@dm36
Copy link
Copy Markdown
Contributor

@dm36 dm36 commented Jun 4, 2026

Summary

End-to-end FGAC tests for the events routes. Stacks on #276 (AGX1-325) — same conftest, same SparkAuthzClient, same cleanup pattern. Just extends AgentexClient with /events methods and adds one test file.

Linear: AGX1-331. Route migration that introduced these checks: #257 (AGX1-244).

Scope

Per the ticket:

Event get/list delegated to parent agent view (read-only): no agent view, 404 on get and filtered/empty list

Events have:

  • No SpiceDB type of their own — checks delegate to the parent agent.
  • No public POST /events — events are emitted by ACP streaming, persisted by the worker.

The second point means the happy path can't be tested black-box without either direct DB seeding (breaks the suite's invariant) or running an actual agent (out of scope). Both are inconsistent with how the rest of the suite works, so the happy-path test is skipped with an explicit reason until a seeding helper exists. The ticket only asks for the denied-path behavior, which is what this PR delivers.

Tests (test_event_authz.py)

Test What it verifies
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 on agent_id collapses denial to 404
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 — needs an event-seeding helper that doesn't exist yet

Stacking

Base: dhruv/agx1-325-e2e-api-key-authz (PR #276). Once #276 merges, GitHub will auto-retarget this PR to main.

If reviewers prefer to review against main directly, this PR can be rebased — but the diff would then include the entire #276 scaffolding, which isn't this ticket's surface.

Verified against pubsec-dev

Ran the event suite against agentex.sgp-pubsec-dev.scale.com on 2026-06-04:

tests/test_event_authz.py::test_get_event_nonexistent_returns_404            PASSED
tests/test_event_authz.py::test_list_events_without_agent_view_returns_404   PASSED
tests/test_event_authz.py::test_list_events_without_task_view_returns_404    PASSED
tests/test_event_authz.py::test_get_event_with_view_returns_200              SKIPPED
                                                              3 passed, 1 skipped

All three denied-path tests pass; the happy-path test skips with the documented "no public POST /events" reason. AGX1-331 deliverables exercised end-to-end.

Setup used:

  • config.json filled in with a real pubsec-dev API key + the EGP account_id as x-selected-account-id.
  • agentex.agent_id set to a pre-existing agent in pubsec-dev (the user lacks agent.create on the tenant wildcard, so parent_agent fixture falls back to the configured id).
  • spark_authz.host pointed at localhost:8090 via kubectl port-forward to ns/spark/svc/spark-authz on the AWS EKS cluster that fronts pubsec-dev. The event tests in this PR don't assert against SpiceDB directly, so they pass without it too; the port-forward is only required for feat(e2e): AGX1-325 — agent_api_keys authz black-box suite #276's create-time SpiceDB-asserting tests.

The same recipe also runs the AGX1-325 api_key suite from #276; those tests surface a separate deployment-config blocker (pubsec-dev's agentex-auth runs with AUTH_PROVIDER=sgp, so the dual-write lands on legacy SGP-authz instead of the spark-authz that's actually deployed alongside it). That's an environment-config finding tracked in #276, not an issue with this PR.

Greptile Summary

This PR adds the AGX1-331 e2e black-box authz suite for the events routes, stacking on the AGX1-325 scaffolding from PR #276. It extends AgentexClient with get_event/list_events methods and introduces test_event_authz.py covering the denied-path cases the ticket asks for.

  • clients/agentex_client.py: Adds get_event and list_events following the same thin-httpx-wrapper pattern used by all existing resource methods.
  • tests/test_event_authz.py: Three passing denied-path tests (nonexistent id short-circuits before authz, no-agent-view collapses to 404, both params zero also 404) plus one explicitly skipped happy-path test with a documented reason; the skip reason's TODO placeholder lacks a ticket number.
  • Makefile / README.md: New SUB_RESOURCE_TESTS/test-sub-resources/test-event targets and matching documentation added consistently with existing patterns.

Confidence Score: 5/5

Test-only change with no production code touched; all three active tests were verified against pubsec-dev.

The change is entirely confined to the e2e test scripts directory — no application code, no migrations, no config. The three active tests were run against a real environment and passed; the skipped test is intentionally guarded. The only actionable note is a missing ticket number in a TODO comment inside a skip reason string.

No files require special attention.

Important Files Changed

Filename Overview
scripts/spark-authz-e2e-tests/tests/test_event_authz.py New e2e authz test file for events routes; covers three denied-path cases (nonexistent id, no agent view, both params zero) and one intentionally skipped happy-path test. Minor: TODO in skip reason lacks a ticket number.
scripts/spark-authz-e2e-tests/clients/agentex_client.py Adds get_event and list_events methods following the same thin-httpx-wrapper pattern used by all existing methods; clean and consistent.
scripts/spark-authz-e2e-tests/Makefile Adds EVENT_TESTS, SUB_RESOURCE_TESTS, test-sub-resources, and test-event targets following established patterns; help text and .PHONY declarations kept in sync.
scripts/spark-authz-e2e-tests/README.md Documentation updated to reflect AGX1-331 scope, new Makefile targets, and the new test file in the directory tree; consistent with existing style.

Sequence Diagram

sequenceDiagram
    participant Test as Test (user_b)
    participant AGX as scale-agentex
    participant SpiceDB as SpiceDB / DAuthorizedQuery

    Note over Test,SpiceDB: test_list_events_without_agent_view_returns_404
    Test->>AGX: "GET /events?agent_id=user_a_agent&task_id=zero-uuid"
    AGX->>SpiceDB: agent.read check (agent_id)
    SpiceDB-->>AGX: denied
    AGX-->>Test: 404 (existence not leaked)

    Note over Test,AGX: test_get_event_nonexistent_returns_404
    Test->>AGX: GET /events/00000000-…
    AGX-->>Test: 404 (ItemDoesNotExist, before authz)

    Note over Test,SpiceDB: test_list_events_denied_on_both_query_params_returns_404
    Test->>AGX: "GET /events?task_id=zero&agent_id=zero"
    AGX->>SpiceDB: task_id Depends fires first
    SpiceDB-->>AGX: denied
    AGX-->>Test: 404
Loading

Fix All in Cursor Fix All in Claude Code Fix All in Codex

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/tests/test_event_authz.py:84
The `reason` string says "Linear: TODO follow-up" without linking an actual ticket number. Per the team's rule, TODO comments must include a ticket number so the work can be tracked and searched by ID.

```suggestion
            "once a test-only seeding helper exists (Linear: <AGX1-XXX>)."
```

Reviews (6): Last reviewed commit: "chore(e2e): re-add Makefile event target..." | Re-trigger Greptile

Context used:

  • Rule used - Create Linear tasks for TODO comments in code and ... (source)

Learned From
scaleapi/scaleapi#127117

  • Rule used - Include ticket numbers in TODO comments to make cl... (source)

Learned From
scaleapi/scaleapi#126926

dm36 added a commit that referenced this pull request Jun 4, 2026
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.
@dm36 dm36 force-pushed the dhruv/agx1-331-e2e-event-authz branch 4 times, most recently from 14d1221 to 430b1e8 Compare June 4, 2026 22:07
@dm36 dm36 marked this pull request as ready for review June 4, 2026 22:15
@dm36 dm36 requested a review from a team as a code owner June 4, 2026 22:15
Comment thread scripts/spark-authz-e2e-tests/tests/test_event_authz.py Outdated
dm36 added a commit that referenced this pull request Jun 4, 2026
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).
dm36 added a commit that referenced this pull request Jun 4, 2026
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.
dm36 added a commit that referenced this pull request Jun 4, 2026
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).
dm36 added a commit that referenced this pull request Jun 4, 2026
…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 dm36 force-pushed the dhruv/agx1-331-e2e-event-authz branch from f4c0961 to 2782f35 Compare June 4, 2026 22:27
dm36 added a commit that referenced this pull request Jun 4, 2026
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).
dm36 added a commit that referenced this pull request Jun 4, 2026
…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 dm36 force-pushed the dhruv/agx1-331-e2e-event-authz branch from 2782f35 to fd3f448 Compare June 4, 2026 22:29
dm36 added a commit that referenced this pull request Jun 5, 2026
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).
@dm36 dm36 force-pushed the dhruv/agx1-331-e2e-event-authz branch from fd3f448 to e4fdf0c Compare June 5, 2026 19:16
dm36 added a commit that referenced this pull request Jun 5, 2026
…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 added 4 commits June 5, 2026 14:43
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.
…eturns_404

Mirrors the api_key tests' switch to the parent_agent fixture so this
denied-path test runs in pubsec-dev where the test user lacks
agent.create. The agent's role here is plumbing (something for user_b
to be denied on), not under test.
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 dm36 force-pushed the dhruv/agx1-331-e2e-event-authz branch from e4fdf0c to c6ab6da Compare June 5, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant