Skip to content

Add detailed lists, PDP refresh, user-permissions context and API coverage report - #140

Draft
zeevmoney wants to merge 36 commits into
per-16678/tenant-membershipfrom
per-16337/api-coverage
Draft

zeevmoney wants to merge 36 commits into
per-16678/tenant-membershipfrom
per-16337/api-coverage

Conversation

@zeevmoney

@zeevmoney zeevmoney commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Linear issues

  • PER-16337: the remaining P1 API additions (detailed lists, PDP data refresh, a context for get_user_permissions) and the coverage report.
  • PER-16336: section 7, "Full compatibility with the Permit backend", whose coverage and drift report this implements for permit-python.
  • PER-16737: the P2 items, listed in the report's allowlist as deferred to this ticket.
  • PER-16338: the PDP's facts pass-through keeps only the last value of a repeated query parameter; the list()/list_detailed() docstrings say so.

Why

Before this PR, the SDK:

  • could not list role assignments or relationship tuples in detail;
  • reached detailed resource instances only through the API's deprecated detailed flag;
  • could not trigger a PDP data refresh;
  • sent no context with get_user_permissions(), so ABAC policies could not read request context there.

Nothing showed which API operations the SDK covers, or noticed when the API gained an operation the SDK neither covers nor excludes on purpose.

What changed

SDK (async and blocking clients, permit/_sync_types.pyi regenerated)

  • list_detailed() on permit.api.role_assignments, resource_instances and relationship_tuples.
    • Each sends GET .../detailed with the filters of that module's list(), as keyword-only arguments, and returns the paginated detailed model.
    • list() and list_detailed() build their query with one shared helper per module, and an offline test checks that both send the same query string.
    • Docstrings state the key scope. The two role_assignments docstrings say that with proxy_facts_via_pdp a list filter keeps only its last value.
  • resource_instances.list(detailed_key=...) sends the same request as before, and now issues one DeprecationWarning naming list_detailed() and permit 4.0.
    • The warning points at the caller's line on both clients.
    • A call without detailed_key does not warn.
    • The caller-attribution code moved into a private helper that deprecated() also uses. Its behaviour is unchanged.
    • New README Deprecations entry.
  • permit.api.pdps.refresh(reason=None), in a new pdps module. It sends POST /v2/pdps/{proj_id}/{env_id}/configs/refresh, and always goes to the API, even with proxy_facts_via_pdp.
    • The docstring says it triggers a data refresh on every PDP in the environment and returns once the refresh is triggered, not once the PDPs finish.
    • It needs an environment-level key with write or admin access, or a broader key with the API context set to the environment.
    • It lists every error: a ValidationError for a reason over 512 characters (nothing is sent), and API 403, 404 and 422.
    • PDPDataRefreshRequest and PDPDataRefreshResponse come from scripts/generate_models.sh output, and their two schema-drift allowlist entries are removed.
    • The sync parity test's sub-API count goes from 18 to 19.
  • get_user_permissions(..., context=None) on the Enforcer, permit.Permit and permit.sync.Permit.
    • A context is merged over the context store the same way check() merges it.
    • Without one, the request bytes are unchanged.
    • context={} sends the store's base context alone.
  • The sync stub generator sorts imported names case-insensitively, as ruff does. The generated stub is unchanged for existing names.
  • README sections for the three additions.

API coverage report

  • Recorder. tests/api_coverage_recorder.py is a pytest plugin loaded from tests/conftest.py.
    • It does nothing unless given a record file (--api-coverage-record or PERMIT_API_COVERAGE_RECORD).
    • When enabled, it writes the method, raw path, status, test id and e2e flag of every request the SDK sends. No query strings, headers or bodies are recorded.
  • Report script. .github/scripts/api_coverage.py is stdlib only.
    • It matches each recorded request to a spec operation template.
    • It lists covered, missing, excluded, deferred, untested and SDK-only operations, split by GA, EAP and deprecated, for the control plane and the PDP.
    • The end-to-end column shows "not run" when there is no e2e record.
    • Exit 0 is a pass and exit 1 a failure: an untriaged GA operation, a stale or restaged allowlist entry, or an unexplained SDK-only request.
    • Exit 2 means the report did not run: an unreadable spec, too few operations, an invalid allowlist, or a record that is missing, unfinished, from a failed session, or under 400 requests. It is never shown as clean.
  • Snapshots. .github/api-specs/ holds the operation inventories of the control-plane spec (263 operations) and of the pinned PDP image's spec (34 operations), each with a source file. A script test fails when PINNED_PDP_IMAGE and the PDP snapshot's source name different images.
  • Allowlist. .github/scripts/api_coverage_allowlist.json has 224 operation entries and 16 sdk_only entries, each with one reason:
    • PER-16337's DEFER and EXCLUDE lists;
    • the 11 P2 operations, deferred to PER-16737;
    • 65 untested operations (the SDK has a method, but no offline wire test sends its request yet; PER-16177);
    • proxy-mode routes the PDP forwards but does not list (PER-16338).
  • CI.
    • A new API Coverage job in test.yml runs on every PR (not path-filtered, needs pytest, runs unless cancelled). It publishes the report as the job summary and the api-coverage-report artifact.
    • The pytest lanes record their requests and upload them, for the e2e column.
    • The new weekly api-coverage.yml checks the live control-plane spec, uploads a ready replacement snapshot, and alerts Slack like schema-drift.yml does.
    • The Audit Script Tests job runs the new script tests.
  • CONTRIBUTING.md has an "API coverage report" section, and its PDP pin and e2e sections mention the snapshot and the record variable.

Behaviour changes

  • New public methods: list_detailed() (three modules), permit.api.pdps.refresh(), and the context argument of get_user_permissions(). import permit also exports the two new generated models.
  • resource_instances.list(detailed_key=True or False) now issues a DeprecationWarning. Under pytest's warnings-as-errors this fails a consumer's test until it switches to list_detailed().
  • ApproveMessage is regenerated from the live spec, which renamed its only field from message to detail. No SDK method returns this model, but code that reads ApproveMessage.message must read detail instead.
  • No other runtime change. The recorder plugin does nothing in a normal test run.

How it was tested

  • Offline suite: 661 passed, 3 skipped, 0 warnings on pydantic 2.13.5 and on pydantic 1.10.26 (base: 545 passed, 3 skipped). New offline wire tests pin the exact method, path, query, headers and body of every new call on both clients.

  • Types: mypy clean on both lanes (107 files), and tests/test_typing_surface.py 4 passed on each. The consumer type-check fixture covers the new methods, and asserts that positional arguments to list_detailed() are a type error.

  • Tooling: all pre-commit hooks pass, uv lock --check passes, actionlint and zizmor report nothing.

  • CI script tests: 223 passed, 115 of them for the coverage report.

  • Coverage report on a fresh record from each lane: exit 0, from 707 offline requests sent by 664 tests.

    • Control-plane GA: 213 = 61 covered + 70 excluded + 22 deferred + 60 untested.
    • PDP GA: 33 = 12 covered + 15 excluded + 1 deferred + 5 untested.
  • Gate, run on scratch copies:

    • a new GA operation planted in the snapshot: exit 1;
    • a stale allowlist entry: exit 1;
    • an empty, short, unfinished or missing record: exit 2;
    • an unreadable spec: exit 2;
    • the weekly sequence against the live spec: exit 0, and the live inventory equals the snapshot.

    Tests for these cases are kept.

  • Mutation testing: the new tests and gates were checked by breaking the code each one guards. Every mutant failed a test, including the report's default minimums, the 2xx-or-3xx edge of the e2e column, its "not run" cells and the PDP pin check.

  • E2e: 42 e2e tests collect. The 9 new ones have not run against a real backend; CI runs them. The e2e column has been checked only with synthetic records so far.

Owner actions before merge

  • Watch the new e2e tests in the container-PDP pytest lanes, and test_get_user_permissions_with_a_context in the cloud-PDP job.
  • Decide whether API Coverage becomes a required status check.

🤖 Generated with Claude Code

zeevmoney and others added 30 commits October 1, 2026 20:28
PDPDataRefreshRequest and PDPDataRefreshResponse are copied unchanged
from what scripts/generate_models.sh generates from today's API schema.
The rest of models.py is not regenerated, so the diff holds only these
two classes. Their entries in the schema drift allowlist would now be
stale, so they are removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A deprecated argument has to warn from inside the method, only when the
argument is passed, so it needs the same attribution to the awaiting
line or the blocking call site that deprecated() applies to a whole
coroutine. The logic moves to one private helper that both use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ruff's isort compares the names of a from-import case-insensitively
within constants, classes and the rest, so PaginatedResultUserRead
comes before PDPDataRefreshResponse. The stub generator compared them
case-sensitively, which no name in the stub had exposed until now, and
would have written an import block ruff rejects. The generated stub
is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each calls the API's /detailed route next to the list route list()
calls, with the query list() sends for the same filters, taken as
keyword arguments, and returns the paginated detailed read model. Both
methods of a module now build that query with one private helper,
which holds list()'s code unchanged. Role assignments carry the role,
user, tenant and resource instance objects; resource instances their
relationship tuples; tuples their subject, relation, object and tenant
details. The detailed resource instance search matches a key exactly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It sends the API's deprecated detailed query parameter, and
list_detailed() replaces it. A call that passes detailed_key still
sends what it sent before, and now issues one DeprecationWarning that
names list_detailed() and permit 4.0, attributed to the calling line on
the async and the blocking client. A call without it does not warn.
The two regression tests that pass detailed_key now expect the warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
POST /v2/pdps/{proj}/{env}/configs/refresh makes every PDP connected to
the environment fetch all of its data again now. refresh() sends an
optional reason, at most 512 characters and checked before sending, and
returns the update id and the ids of the PDP configurations targeted.
It returns once the refresh is triggered, not once the PDPs finish. It
needs a key with write or admin access to the environment, and always
goes to the API, whatever proxy_facts_via_pdp says.

pdps is a new sub-API on both clients, so the parity test's count of
sub-APIs goes from 18 to 19.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
get_user_permissions() on the Enforcer, permit.Permit and
permit.sync.Permit takes a context, which it sends in the
/user-permissions body merged over the context store's base context,
as check() merges it. Without a context the body is byte for byte what
3.0 sent: no context key, even when the context store holds one. The
offline tests pin the exact bytes of both bodies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tests/api_coverage_recorder.py is a pytest plugin that tests/conftest.py
loads. Given a record file (--api-coverage-record or
PERMIT_API_COVERAGE_RECORD), it adds an aiohttp trace config to every
ClientSession and writes one JSON line per request: method, raw path,
response status, test id and whether the test is marked e2e, then a
session line with the exit status. Without a record file it does
nothing. Part of PER-16337.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
.github/scripts/api_coverage.py matches each request in a test record
to an operation of the control-plane and container PDP specs, and lists
covered, missing, allowlisted and SDK-only operations by GA, EAP and
deprecated, with an end-to-end column filled from e2e records. It exits
1 on a GA operation neither covered nor allowlisted, a stale or changed
allowlist entry, or an unexplained SDK-only request, and 2 when it did
not run. Its `snapshot` command writes a spec's operation inventory.

.github/api-specs/ holds the inventories of the control-plane spec and
of the pinned PDP image's spec, each with its source and fetch date.
The allowlist gives every operation left out on purpose a status and a
reason: PER-16337's EXCLUDE and DEFER lists, the P2 items deferred to
PER-16737, and the operations an SDK method sends that no offline test
covers yet (PER-16177). The Audit Script Tests job runs its tests.

Part of PER-16337.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A new API Coverage job in test.yml runs the offline suite with the
request recorder and reports against the committed snapshots, in the
job summary and the api-coverage-report artifact. The pytest lanes
record their requests too and upload the record, which fills the
report's end-to-end column. Part of PER-16337.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
api-coverage.yml runs the report against the live control-plane spec
on Mondays and on manual dispatch, lists how it differs from the
committed snapshot, uploads a refreshed snapshot, and posts to Slack
when it fails or cannot run. Part of PER-16337.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
How to run it, what the allowlist statuses mean, what fails it, and how
to refresh the control-plane and PDP snapshots. Part of PER-16337.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A .source.json next to a snapshot that is not JSON, or does not say
where and when the snapshot was taken, now stops the report (exit 2)
instead of being skipped, and its text is escaped in the summary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test builds a tenant, a folder and a document resource type joined
by a parent relation, a tenant role, a user, one instance of each type,
the tuple between them and two role assignments, and deletes them after.
It checks what each list_detailed() fills in beyond list(), that it
returns the same objects list() does, the role assignment filters and
pages, the exact match of the detailed instance search, and that the
blocking client gets the same pages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two refreshes return distinct update ids for the same, non-empty set of
PDP configurations, on the async and the blocking client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On an RBAC policy, which does not read the context, the PDP must accept
a context and answer as it does without one: on the container PDP
through both clients, with the context store holding a base context,
and on the cloud PDP. What an ABAC policy makes of the context waits
for the ABAC decision checks, which are pending PER-16209.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The missing and allowlisted tables now say whether an e2e test got an
answer from the operation, as the covered table did, so an untested
operation the e2e tests exercise is visible.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* per-16337/impl-api:
  Test get_user_permissions() with a context end to end
  Test pdps.refresh() end to end
  Test the detailed lists end to end
  Send a context with get_user_permissions()
  Add permit.api.pdps.refresh() to refresh every PDP's data
  Deprecate the detailed_key argument of resource_instances.list()
  Add list_detailed() to role assignments, resource instances and tuples
  Sort the stub's imported names case-insensitively, as ruff does
  Factor the caller-attributed deprecation warning out of deprecated()
  Add the generated PDP data refresh models

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* per-16337/impl-report:
  Show the end-to-end column for every operation in the coverage report
  Fail the coverage report on a snapshot source file it cannot read
  Document the API coverage report in CONTRIBUTING.md
  Check API coverage against the live spec every week
  Run the API coverage report on every pull request
  Add the API coverage report, its spec snapshots and allowlist
  Record the requests the tests send, for the API coverage report

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With proxy_facts_via_pdp, list_detailed() sends GET /facts/<x>/detailed
to the PDP, which forwards it to the control plane through a route its
spec does not list (PER-16338). The offline tests send these requests,
so the coverage report needs an sdk_only entry for each of the three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A new method's wire test covers its operation, so a deferred entry for
it goes stale exactly as an untested one does, as the PER-16737 entries
will. Its proxy-mode test can also send a PDP route the PDP's spec
omits, which needs an sdk_only entry, as the detailed lists did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The merged suite sends about 700 offline requests, not 590.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With proxy_facts_via_pdp, the PDP forwards only the last value of a
filter given as a list, so list(user_key=["alice", "bob"]) and
list_detailed() with the same filter return bob's assignments alone.
The SDK sends every value; the docstrings of both methods now say to
pass lists only with proxy_facts_via_pdp off.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reason over 512 characters raises pydantic.v1.ValidationError before
any request is sent, and the API answers 422 for an environment with
more PDP configurations than one refresh reaches. The Raises section
named only 403 and 404.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The policy is RBAC, so the PDP answers the same with or without a
context, and the test cannot see whether the context was sent. It is
now test_the_blocking_client_accepts_a_context, and its docstring
points to the offline test that pins the request bytes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The coverage report reads .github/api-specs/pdp.json, taken from the
PDP image PINNED_PDP_IMAGE names. A change that moved the pin without
refreshing the snapshot still passed, so the report measured an older
PDP API. A test now reads the pin from test.yml and requires
pdp.source.json to name the same image; CONTRIBUTING.md says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without an e2e record, the covered, missing and allowlisted tables must
each say "not run" in their end-to-end column. Only the counts table
and the JSON were checked, so a "no" in those rows went unnoticed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An e2e answer of 399 now counts as exercised and 400 does not, so
narrowing the range to 2xx, or widening it past 3xx, fails the test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zeevmoney and others added 3 commits October 1, 2026 21:49
read_json, operations_of and inventory took or returned Any, with three
noqa: ANN401 comments, two of them unexplained. They now use object,
and inventory checks that the document is a JSON object before it reads
it, with the message operations_of already gives. The snapshot test now
also covers a document that is not an object or has no paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The CI jobs rely on the defaults: 400 offline requests, 200
control-plane operations and 20 PDP operations. The tests pinned the
minimum checks only with explicit values, so lowering a default, which
turns its sentinel off, still passed. The report now runs with each
default one below and at its value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 1, 2026

Copy link
Copy Markdown

PER-16337

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Dependency Security Audit

Scanned: pyproject.toml dependencies + dev group, resolved at Python 3.10 (the current resolution, and the lowest versions the published specs permit under each pydantic major)

✅ No known vulnerabilities found.

Both the resolved dependency set and the lowest versions the published specs permit are clean at HIGH and CRITICAL.

zeevmoney and others added 3 commits October 2, 2026 00:16
The live spec renamed ApproveMessage's only field from message to
detail, so the schema drift check failed on it. No SDK method returns
this model. The class is the generator's output, copied unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The relationship tuple delete body takes subject, relation and object,
and the API answers 422 to a tenant there, so the teardown failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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