Skip to content

feat(schedules): register agent_schedule in authz on create/delete#259

Merged
harvhan merged 1 commit into
mainfrom
harvhan/agx1-270-schedule-dual-write
Jun 2, 2026
Merged

feat(schedules): register agent_schedule in authz on create/delete#259
harvhan merged 1 commit into
mainfrom
harvhan/agx1-270-schedule-dual-write

Conversation

@harvhan
Copy link
Copy Markdown
Contributor

@harvhan harvhan commented Jun 1, 2026

Summary

Registers an agent_schedule resource in the authorization service on schedule
create (with the owning agent as its parent edge) and deregisters it on delete,
so schedules participate in fine-grained authorization.

Schedules have no Postgres row — Temporal is the store and the authz selector is
the schedule id ({agent_id}--{schedule_name}). The register/deregister calls
therefore live in ScheduleService, alongside the Temporal write, so the
compensation boundary can be scoped to the Temporal create only.

Behavior

  • Create: register the schedule (with parent=agent) before the
    Temporal create. Unconditional and fail-closed — a register failure
    aborts the create. If the Temporal create fails after a successful register,
    a compensating deregister removes the tuple so no orphan is left. The
    post-create read-back (describe) is outside the compensation scope: a describe
    failure must not deregister a schedule that was actually created.
  • Delete: deregister after the Temporal delete, best-effort (logged,
    never blocks the delete).
  • No resolvable creator (agent-bypass / internal paths with neither a user
    nor service-account identity): registration is skipped with a warning and the
    schedule is still created. Interim behavior until on-behalf-of-user identity
    is threaded.

Per-account routing (which authz backend the call lands on) is owned by the
authorization service; this repo calls register/deregister unconditionally.
Follows the agent_api_key register/deregister pattern (#248), which this PR is
stacked on.

Requires

A companion change in the authorization service mapping the schedule resource
type to its SpiceDB agent_schedule definition; without it the
register/deregister call is rejected before reaching the backend.

Changes

  • schedule_service.py: register-before-create + compensate-on-temporal-failure;
    best-effort deregister on delete; _register_schedule_in_auth /
    _deregister_schedule_from_auth helpers; inject the authorization service.
  • authorization_types.py: add the schedule resource type + factory.
  • tests/integration/services/test_schedule_service_dual_write.py: 7 cases
    (parent-edge contract, fail-closed create, compensating deregister on Temporal
    failure, read-back failure does NOT compensate, best-effort deregister on
    delete, no-creator skip).
  • tests/unit/services/test_schedule_service.py: fixture updated for the new
    constructor arg.

Test plan

  • pytest tests/integration/services/test_schedule_service_dual_write.py — 7/7
  • pytest tests/unit/services/test_schedule_service.py — 27/27
  • pytest tests/unit/use_cases/test_schedules_use_case.py — 18/18
  • ruff clean

Stacked on

#248 (base branch). Rebase onto the default branch once #248 merges.

🤖 Generated with Claude Code

Greptile Summary

This PR integrates agent_schedule resources into the authorization service on create and delete, following the existing agent_api_key pattern. Register-before-create is fail-closed with a compensating deregister on Temporal failure; delete is best-effort post-Temporal.

  • schedule_service.py: Injects DAuthorizationService, adds _register_schedule_in_auth (fail-closed, returns bool to drive compensation) and _deregister_schedule_from_auth (best-effort, never propagates). The post-create read-back is intentionally outside the compensation scope, which is correctly verified by a dedicated test case.
  • authorization_types.py: Adds the schedule enum value and corresponding factory methods to both AgentexResource and AgentexResourceOptionalSelector, matching all existing resource types.
  • Tests: 7 integration test cases cover the parent-edge contract, fail-closed create, compensating deregister, read-back-failure non-compensation, best-effort delete, and no-creator skip. Unit test fixture updated for the new constructor argument.

Confidence Score: 5/5

Safe to merge; the compensation and bypass logic are correct and all edge cases are covered by the new test suite.

The register-before-create / compensate-on-Temporal-failure contract is correctly implemented: orphan tuples are prevented on create failures, and delete is best-effort so auth never blocks a Temporal delete. The read-back exclusion from compensation scope is explicitly tested. The no-creator skip and auth-bypass paths are both handled without leaving orphan state. The 7-case integration test suite directly mirrors the documented behavior contracts.

No files require special attention.

Important Files Changed

Filename Overview
agentex/src/api/schemas/authorization_types.py Adds schedule to AgentexResourceType enum and factory methods to both AgentexResource and AgentexResourceOptionalSelector; directly mirrors the api_key pattern, no issues.
agentex/src/domain/services/schedule_service.py Injects DAuthorizationService, adds register/deregister helpers with correct fail-closed/best-effort semantics, and scopes compensation to the Temporal create only (read-back is outside scope); logic is sound.
agentex/tests/integration/services/test_schedule_service_dual_write.py New 7-case integration test file covering the full dual-write contract: parent-edge shape, fail-closed create, compensating deregister, read-back non-compensation, best-effort delete, and no-creator skip.
agentex/tests/unit/services/test_schedule_service.py Minimal fixture update to supply the new authorization_service constructor argument; no behavioral changes to the existing 27 unit tests.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant ScheduleService
    participant AuthzService
    participant Temporal

    Note over ScheduleService: create_schedule()
    ScheduleService->>AuthzService: "register_resource(schedule, parent=agent)"
    alt register fails
        AuthzService-->>ScheduleService: raises Exception
        ScheduleService-->>Caller: raises (fail-closed, Temporal never called)
    else "register succeeds (registered=True)"
        AuthzService-->>ScheduleService: None
        ScheduleService->>Temporal: create_schedule(schedule_id)
        alt Temporal create fails
            Temporal-->>ScheduleService: raises Exception
            ScheduleService->>AuthzService: deregister_resource(schedule) [compensate]
            Note over ScheduleService: best-effort, swallows errors
            ScheduleService-->>Caller: re-raises original exception
        else Temporal create succeeds
            Temporal-->>ScheduleService: None
            ScheduleService->>Temporal: describe_schedule (read-back)
            Note right of ScheduleService: Outside compensation scope
            ScheduleService-->>Caller: ScheduleResponse
        end
    else no creator identity
        Note over ScheduleService: Skips register (warning logged)
        ScheduleService->>Temporal: create_schedule(schedule_id)
        Temporal-->>ScheduleService: None
        ScheduleService-->>Caller: ScheduleResponse
    end

    Note over ScheduleService: delete_schedule()
    ScheduleService->>Temporal: delete_schedule(schedule_id)
    Temporal-->>ScheduleService: None
    ScheduleService->>AuthzService: deregister_resource(schedule) [best-effort]
    ScheduleService-->>Caller: None
Loading

Reviews (2): Last reviewed commit: "feat(schedules): register agent_schedule..." | Re-trigger Greptile

@harvhan harvhan requested a review from a team as a code owner June 1, 2026 16:28
Base automatically changed from dhruv/agx1-272-agent-api-keys-dual-write to main June 2, 2026 16:37
Register an agent_schedule resource (with the parent_agent edge) in the
authorization service before the Temporal create, and deregister it on
delete. The register/deregister calls live in ScheduleService, alongside
the Temporal write, so the compensation boundary can be scoped to the
Temporal create only: registration is unconditional and fail-closed; a
Temporal create failure after a successful register triggers a
compensating deregister so no orphan tuple is left behind. A post-create
read-back (describe) failure does NOT compensate, since the schedule was
actually created. Deregister on delete is best-effort.

Registration is skipped with a warning when no creator identity is
resolvable on the principal context (agent-bypass / internal paths).
Mirrors the agent_api_key register/deregister pattern.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@harvhan harvhan force-pushed the harvhan/agx1-270-schedule-dual-write branch from 2b22285 to a478b29 Compare June 2, 2026 16:52
@harvhan harvhan merged commit 2b527ff into main Jun 2, 2026
30 checks passed
@harvhan harvhan deleted the harvhan/agx1-270-schedule-dual-write branch June 2, 2026 17:00
asherfink added a commit that referenced this pull request Jun 3, 2026
…strings (#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 -->
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.

2 participants