Skip to content

Commit 8b6d69d

Browse files
authored
chore(authz): scrub internal project references from comments and docstrings (#268)
Scrubs internal references (Linear ticket IDs, internal service names, internal flag names) from comments and docstrings across already-landed authorization code. Fixes issues introduced by #248, #252, #255, #257, #259, and #260. No behavior changes. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR is a pure housekeeping change that scrubs internal references — Linear ticket IDs (e.g. `AGX1-263`, `AGX1-290`), internal service names (`SpiceDB`, `Spark`, `agentex-auth`), and internal flag/PR references — from comments and docstrings across 13 authorization-related source and test files. No executable code is touched. - Comments and docstrings across `port.py`, `authorization_service.py`, `agent_api_keys.py`, and related utilities have internal service names replaced with generic terms like \"authorization service\" or \"authorization schema\". - TODO comments in `agent_api_key_authorization.py`, `agent_authorization.py`, and `authorization_shortcuts.py` have internal ticket IDs stripped, with file-level context substituted where the ticket reference previously identified the work location. - Module docstrings in all six affected test files have ticket-prefixed deliverable descriptions replaced with plain behavioral descriptions. <details><summary><h3>Confidence Score: 5/5</h3></summary> All changes are confined to comments and docstrings; no executable code is modified. Every diff hunk touches only comment text, docstrings, or module-level strings. There are no logic, control-flow, or interface changes anywhere in the PR, making regression risk essentially zero. No files require special attention. </details> <h3>Important Files Changed</h3> | Filename | Overview | |----------|----------| | agentex/src/adapters/authorization/port.py | Docstring-only: replaced "SpiceDB" with "authorization" in register_resource docstring. | | agentex/src/api/routes/agent_api_keys.py | Comment-only: removed "SpiceDB" references from two inline comments in create/delete route handlers. | | agentex/src/domain/services/authorization_service.py | Docstring-only: replaced "SpiceDB schema" with "authorization schema" in register_resource docstring. | | agentex/src/domain/use_cases/agent_api_keys_use_case.py | Comment-only: removed unconditional-routing comment referencing internal service names and ticket numbers; also cleaned docstring referencing internal routing logic. | | agentex/src/utils/agent_api_key_authorization.py | Comment-only: replaced TODO(AGX1-290) with plain TODO, removing the Linear ticket reference. | | agentex/src/utils/agent_authorization.py | Comment-only: replaced TODO(AGX1-290) with plain TODO, removing the Linear ticket reference. | | agentex/src/utils/authorization_shortcuts.py | Comment-only: updated TODO to reference the file name instead of internal ticket/PR numbers. | | agentex/tests/integration/services/test_agent_api_key_service_dual_write.py | Docstring-only: removed references to "Spark AuthZ", internal ticket numbers, and internal service names throughout module docstring and inline comments. | | agentex/tests/integration/services/test_schedule_service_dual_write.py | Docstring-only: removed "Spark AuthZ" and "SpiceDB" references across the module docstring and inline comments. | | agentex/tests/integration/api/checkpoints/test_checkpoints_authz_api.py | Docstring-only: removed AGX1-302 ticket reference and internal terminology from module docstring. | | agentex/tests/integration/api/events/test_events_authz_api.py | Docstring-only: replaced "AGX1-244" ticket reference in module docstring with a plain description. | | agentex/tests/integration/api/messages/test_messages_authz_api.py | Docstring-only: removed AGX1-277 ticket reference from module docstring. | | agentex/tests/unit/api/test_agent_api_keys_authz.py | Docstring-only: removed AGX1-263 ticket reference and "Spark AuthZ" mention from module docstring. | </details> <details><summary><h3>Flowchart</h3></summary> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A["PR #268: Scrub internal references"] --> B["Source files\n(port.py, agent_api_keys.py,\nauthorization_service.py,\nagent_api_keys_use_case.py,\n3 utils files)"] A --> C["Test files\n(6 integration + unit tests)"] B --> D["Replace: SpiceDB → authorization service/schema\nRemove: agentex-auth, Spark, internal PR refs\nStrip: AGX1-xxx ticket IDs from TODOs"] C --> E["Replace: AGX1-xxx deliverable labels → plain descriptions\nRemove: Spark AuthZ, scaleapi/agentex#353, etc."] D --> F["No executable code changed"] E --> F ``` </details> <sub>Reviews (2): Last reviewed commit: ["chore(authz): scrub internal project ref..."](62581f7) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=35250665)</sub> <!-- /greptile_comment -->
1 parent 99e29b9 commit 8b6d69d

12 files changed

Lines changed: 43 additions & 47 deletions

File tree

agentex/src/adapters/authorization/port.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ async def register_resource(
5757
resource: AgentexResource,
5858
parent: AgentexResource | None = None,
5959
) -> None:
60-
"""Register a newly created resource in SpiceDB with the principal as
61-
owner. Optionally writes a lifecycle parent edge.
60+
"""Register a newly created resource with the principal as owner.
61+
Optionally writes a lifecycle parent edge.
6262
6363
Use this on resource create instead of ``grant`` when the resource
64-
type's SpiceDB definition has a parent relation that permission
64+
type's authorization schema has a parent relation that permission
6565
checks cascade through (e.g. ``agent_api_key`` declares
6666
``parent_agent``). Without writing that edge here the cascade fails
6767
closed.

agentex/src/api/routes/agent_api_keys.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ async def create_api_key(
5757
)
5858
agent = await agent_use_case.get(id=request.agent_id, name=request.agent_name)
5959

60-
# No api_key resource exists yet, so SpiceDB can't gate transitively —
61-
# gate on parent ``agent.update`` directly.
60+
# No api_key resource exists yet, so the authorization service can't gate
61+
# transitively — gate on parent ``agent.update`` directly.
6262
await authorization_service.check(
6363
resource=AgentexResource.agent(agent.id),
6464
operation=AuthorizedOperationType.update,
@@ -211,8 +211,8 @@ async def delete_agent_api_key(
211211
param_name="id",
212212
),
213213
) -> str:
214-
# SpiceDB's ``api_key.delete`` expands transitively to
215-
# ``parent_agent->update``, so the dep above enforces both factors.
214+
# ``api_key.delete`` expands transitively to ``parent_agent->update``,
215+
# so the dep above enforces both factors.
216216
await agent_api_key_use_case.delete(id=id)
217217
return f"Agent API key with ID {id} deleted"
218218

agentex/src/domain/services/authorization_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ async def register_resource(
197197
) -> None:
198198
"""Register a newly created resource with the principal as owner.
199199
200-
Prefer this over ``grant`` when the resource's SpiceDB schema has
200+
Prefer this over ``grant`` when the resource's authorization schema has
201201
a parent relation that permissions cascade through (e.g.
202202
``agent_api_key`` declares ``parent_agent``). Pass ``parent`` to
203203
link the child to its parent atomically; without it the cascade

agentex/src/domain/use_cases/agent_api_keys_use_case.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,6 @@ async def create(
9090

9191
api_key_id = orm_id()
9292

93-
# Unconditional — agentex-auth decides per-account whether the call
94-
# routes to Spark or the legacy backend.
9593
await self._register_api_key_in_auth(
9694
api_key_id=api_key_id,
9795
agent_id=agent.id,
@@ -157,9 +155,8 @@ async def _deregister_api_key_from_auth(self, *, api_key_id: str) -> None:
157155
158156
``deregister_resource`` removes the resource and all of its
159157
relationships (owner, parent, grantees) atomically. Always invoked;
160-
agentex-auth's per-account routing (#353) decides whether the call
161-
targets Spark or the legacy backend. Failures are logged but do not
162-
block the delete.
158+
the authorization service decides how to route the call. Failures are
159+
logged but do not block the delete.
163160
"""
164161
try:
165162
await self.authorization_service.deregister_resource(
@@ -252,7 +249,7 @@ async def list(
252249
page_number: int,
253250
id: list[str] | None = None,
254251
) -> list[AgentAPIKeyEntity]:
255-
# ``id`` is the FGAC-filter shape used by route deps (DAuthorizedResourceIds).
252+
# ``id`` is the authorization-filter shape used by route deps (DAuthorizedResourceIds).
256253
# ``None`` means "no filter" (e.g. authz bypass); an empty list means
257254
# "caller can see no api_keys" and must short-circuit to avoid the
258255
# base repo translating ``id=[]`` into an unfiltered query.

agentex/src/utils/agent_api_key_authorization.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ async def _check_api_key_or_collapse_to_404(
1818
"""Check an api_key resource; collapse any denial to 404 to avoid leaking
1919
cross-tenant existence. Mirrors ``_check_task_or_collapse_to_404``.
2020
21-
TODO(AGX1-290): restore the 403/404 split once api_keys carry tenant scope.
21+
TODO: Restore the 403/404 split once api_keys carry tenant scope.
2222
"""
2323
try:
2424
await authorization.check(

agentex/src/utils/agent_authorization.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ async def check_agent_or_collapse_to_404(
1515
cross-tenant existence. Mirrors ``check_task_or_collapse_to_404`` in
1616
``task_authorization.py`` — see that docstring for the full rationale.
1717
18-
TODO(AGX1-290): restore 403/404 split once agents carry tenant scope.
18+
TODO: Restore the 403/404 split once agents carry tenant scope.
1919
"""
2020
try:
2121
await authorization.check(

agentex/tests/integration/api/checkpoints/test_checkpoints_authz_api.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,15 @@
11
"""
22
Integration tests for checkpoint-route authorization.
33
4-
Covers the AGX1-302 deliverable: checkpoint routes have no SpiceDB type of
5-
their own. Each route enforces on the owning task (``thread_id`` is the task id)
6-
via ``DAuthorizedBodyId(task, ...)``:
4+
Checkpoint routes have no authorization type of their own. Each route enforces
5+
on the owning task (``thread_id`` is the task id) via ``DAuthorizedBodyId(task, ...)``:
76
87
* get-tuple / list -> ``task.read`` (view).
98
* put / put-writes -> ``task.update`` (editor + owner); delete-thread -> ``task.delete``.
109
11-
Per the canonical ``DAuthorizedBodyId`` task wrap (the 404/403 collapse landed
12-
by AGX1-275 / #249), a denied check on the parent task collapses to 404 (not
13-
403) on every route — reads and writes alike — so callers cannot probe
14-
cross-tenant existence by comparing response codes.
10+
Per the canonical ``DAuthorizedBodyId`` task wrap, a denied check on the parent
11+
task collapses to 404 (not 403) on every route — reads and writes alike — so
12+
callers cannot probe cross-tenant existence by comparing response codes.
1513
"""
1614

1715
from typing import Any

agentex/tests/integration/api/events/test_events_authz_api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""AGX1-244: event routes delegate authz to the parent agent."""
1+
"""Tests for event route authorization — routes delegate checks to the parent agent."""
22

33
from typing import Any
44
from unittest.mock import patch

agentex/tests/integration/api/messages/test_messages_authz_api.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
"""
22
Integration tests for message-route authorization.
33
4-
Covers the AGX1-277 deliverable: list/get messages enforce ``task.read`` on
5-
the parent task, with denied checks collapsing to 404 (not 403) so callers
6-
cannot probe cross-tenant existence by comparing response codes.
4+
List/get message routes enforce ``task.read`` on the parent task, with denied
5+
checks collapsing to 404 (not 403) so callers cannot probe cross-tenant
6+
existence by comparing response codes.
77
"""
88

99
from typing import Any

agentex/tests/integration/services/test_agent_api_key_service_dual_write.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
1-
"""Integration tests for AgentAPIKeysUseCase dual-write to Spark AuthZ.
1+
"""Integration tests for AgentAPIKeysUseCase dual-write to the authorization service.
22
3-
These cover the AGX1-272 dual-write path. scale-agentex calls
4-
``register_resource`` / ``deregister_resource`` unconditionally; per-account
5-
routing (Spark vs legacy SGP) is owned by agentex-auth (scaleapi/agentex#353)
6-
so scale-agentex does NOT couple to egp-api-backend's feature-flag service.
3+
scale-agentex calls ``register_resource`` / ``deregister_resource``
4+
unconditionally; per-account routing is owned by the authorization gateway,
5+
so scale-agentex does NOT couple to the authorization flag service.
76
87
- Create calls register_resource with parent=agent (the parent_agent edge
9-
is load-bearing for the SpiceDB cascade).
8+
is load-bearing for the authorization cascade).
109
- Delete calls deregister_resource after the Postgres row is gone.
11-
- Spark failure prevents row: when register_resource raises, the api_key
12-
is NOT persisted.
10+
- Registration failure prevents row: when register_resource raises, the
11+
api_key is NOT persisted.
1312
- Deregister failure does not block delete: when deregister_resource
1413
raises, the DB delete still completes and the failure is logged.
1514
- No creator → no register: if neither user_id nor service_account_id is
1615
resolvable, the dual-write is a no-op (logged) and the row still lands.
1716
1817
The tests intentionally mock the repository, authorization service, agent
1918
repository, and HTTP client. The behaviour under test is the call sequencing
20-
inside ``AgentAPIKeysUseCase`` — not Postgres or Spark itself.
19+
inside ``AgentAPIKeysUseCase`` — not the underlying persistence or authorization
20+
cluster itself.
2121
22-
Note on structural divergence from the task PR (AGX1-274): tasks live behind
22+
Note on structural divergence from the task dual-write: tasks live behind
2323
``AgentTaskService``; agent_api_keys have no service layer, so the dual-write
2424
logic is colocated in ``AgentAPIKeysUseCase``.
2525
"""
@@ -154,7 +154,7 @@ async def test_create_api_key_calls_register_resource_with_parent(
154154
registered_resource: AgentexResource = register.await_args.kwargs["resource"]
155155
assert registered_resource.type == AgentexResourceType.api_key
156156
assert registered_resource.selector == api_key.id
157-
# parent_agent edge is load-bearing — without it the SpiceDB cascade
157+
# parent_agent edge is load-bearing — without it the authorization cascade
158158
# `read = ... & parent_agent->read & ...` fails closed for every reader.
159159
registered_parent: AgentexResource = register.await_args.kwargs["parent"]
160160
assert registered_parent is not None

0 commit comments

Comments
 (0)