Skip to content

Commit 3b722f6

Browse files
fix(schedules): use stable handles for run schedules (#349)
## Summary - Use immutable schedule row ids as the stable handle for run-schedule authorization and mutations. - Keep name-based routes as explicit convenience aliases under `/name/{name}` while making `name` a mutable active-row label. - Add a safe index migration so active schedule names remain unique per agent while deleted names can be reused. Addressing comment by @NiteshDhanpal here #335 (comment) ## Test plan - `uv run pytest tests/unit/services/test_agent_run_schedule_service.py tests/unit/api/test_agent_run_schedules_authz.py tests/unit/use_cases/test_agent_run_schedules_use_case.py` - Pre-commit hooks: ruff, ruff-format, OpenAPI generation, migration safety lint Made with [Cursor](https://cursor.com) <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR completes the planned fast-follow from the earlier schedules PR: schedule identity is moved from the human-readable `name` to the immutable row `id`, making name a mutable label and closing the rename/auth-race the previous design could not support. - **Auth selector + primary routes** are rebased onto `schedule_id` (`run-schedule::{agent_id}::{schedule_id}`); all existing service methods are updated from name-based to ID-based signatures with matching test coverage. - **`/name/{name}` convenience aliases** are added for every mutating and read route; a shared `_resolve_name_alias_and_check` helper resolves name → id, then performs the authz check on the stable id, and equalises the 404 body on both absent-name and access-denied paths so the resolved id is never leaked to an unauthorized caller. - **Schedule rename** is enabled via a new `name` field on `UpdateAgentRunScheduleRequest`, guarded by an application-level conflict pre-check plus a `DuplicateItemError` backstop at the DB layer. - **Migration** replaces the full `(agent_id, name)` unique index with a partial index scoped to active rows (`deleted_at IS NULL`), making deleted names reusable; the downgrade path pre-checks for duplicate names before attempting to recreate the old constraint and raises a descriptive `RuntimeError` if cleanup is required. <details><summary><h3>Confidence Score: 5/5</h3></summary> Safe to merge; the auth selector migration, route expansion, and rename feature are all internally consistent and well-tested. All primary operations (create, get, update, delete, pause, resume, trigger) now resolve through stable row IDs. The name-alias resolver correctly equalises absent-name and denied-resource error paths, preventing id/existence probing. The migration is online-safe (CONCURRENTLY with IF NOT EXISTS / IF EXISTS guards), and the downgrade proactively guards against data states that would violate the old constraint. The Temporal-before-DB ordering on updates is unchanged from the pre-existing pattern and is explicitly documented. Unit tests cover the new rename paths, the TOCTOU backstop, and the id-leak-prevention invariant. The migration downgrade path in `2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py` deserves a second look before any rollback attempt — it will raise if name reuse has already occurred, so a data-cleanup runbook should be prepared alongside this deploy. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/database/migrations/alembic/versions/2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py | Adds partial unique index on active schedules (deleted_at IS NULL) and drops the old full-table index; downgrade pre-checks for duplicate names but will raise RuntimeError after any name reuse, requiring manual data cleanup before rollback. | | agentex/src/api/routes/agent_run_schedules.py | Doubles route count by adding /name/{name} alias endpoints alongside the refactored /{schedule_id} primary routes; name resolution uses _resolve_name_alias_and_check which equalizes error messages on both absent-name and denied-access paths to prevent id/existence leakage. | | agentex/src/domain/services/agent_run_schedule_service.py | Switches auth selector key from name to immutable row id, adds get_schedule_id_by_name helper, and extends update_schedule to support name rename with a TOCTOU-safe pre-check plus DuplicateItemError catch as a backstop. | | agentex/src/domain/repositories/agent_run_schedule_repository.py | Adds get_by_agent_id_and_id / get_by_agent_id_and_id_or_raise for ID-keyed lookups; renames the existing name-based or_raise variant accordingly; existing get_by_agent_id_and_name is retained for name-alias resolution. | | agentex/src/api/schemas/agent_run_schedules.py | Adds optional name field to UpdateAgentRunScheduleRequest (enabling renames) and updates docstring/description strings to reflect that name is now a mutable label rather than an immutable natural key. | | agentex/src/utils/schedule_authorization.py | Adds not_found_message override parameter so name-addressed routes can substitute a name-based 404 body, preventing an unauthorized caller from distinguishing a denied resource from an absent one. | | agentex/src/adapters/orm.py | Updates ORM index definition to use the partial index name matching the migration and adds postgresql_where clause to enforce uniqueness only among active rows. | | agentex/tests/unit/api/test_agent_run_schedules_authz.py | Adds tests for the name-alias resolution flow including the denied-access case that verifies the resolved id is never echoed back to an unauthorized caller. | | agentex/tests/unit/services/test_agent_run_schedule_service.py | Covers new rename scenarios: allowed rename, duplicate active name rejection, and DuplicateItemError-to-ClientError conversion; also adds TestAgentRunScheduleServiceNameLookup for the new get_schedule_id_by_name helper. | | agentex/tests/unit/use_cases/test_agent_run_schedules_use_case.py | Adds delegation test for get_schedule_id_by_name and updates all existing tests to use schedule IDs instead of names as the primary handle. | | agentex/src/domain/use_cases/agent_run_schedules_use_case.py | Mechanically updates all method signatures from name-based to schedule_id-based and adds get_schedule_id_by_name delegation; no logic changes. | </details> <details><summary><h3>Sequence Diagram</h3></summary> <a href="#gh-light-mode-only"> ```mermaid %%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Client participant Router participant NameResolver as _resolve_name_alias_and_check participant UseCase as AgentRunSchedulesUseCase participant Service as AgentRunScheduleService participant Repo as AgentRunScheduleRepository participant Auth as AuthorizationService participant Temporal Note over Client,Temporal: ID-based route (stable handle) Client->>Router: "GET /schedules/{schedule_id}" Router->>Auth: "check(resource=schedule(selector), op=read)" Auth-->>Router: ok Router->>UseCase: get_schedule(agent_id, schedule_id) UseCase->>Service: get_schedule(agent_id, schedule_id) Service->>Repo: get_by_agent_id_and_id_or_raise(agent_id, schedule_id) Repo-->>Service: AgentRunScheduleEntity Service->>Temporal: describe_schedule(temporal_id) Temporal-->>Service: ScheduleDescription Service-->>Client: AgentRunScheduleResponse Note over Client,Temporal: Name-alias route (resolves to ID first) Client->>Router: "GET /schedules/name/{name}" Router->>NameResolver: "resolve(agent_id, name, op=read)" NameResolver->>UseCase: get_schedule_id_by_name(agent_id, name) UseCase->>Service: get_schedule_id_by_name(agent_id, name) Service->>Repo: get_by_agent_id_and_name(agent_id, name) alt schedule not found Repo-->>NameResolver: None - raise ItemDoesNotExist(name-message) NameResolver-->>Client: 404 (name-based message) else schedule found Repo-->>NameResolver: schedule_id NameResolver->>Auth: "check(resource=schedule(selector by id), op=read)" alt denied Auth-->>NameResolver: AuthorizationError NameResolver-->>Client: 404 (same name-based message, hides id) else allowed NameResolver-->>Router: schedule_id Router->>UseCase: get_schedule(agent_id, schedule_id) UseCase-->>Client: AgentRunScheduleResponse end end Note over Client,Temporal: Schedule rename Client->>Router: "PATCH /schedules/{schedule_id} name=new-name" Router->>Auth: "check(op=update)" Router->>UseCase: update_schedule(agent_id, schedule_id, request) UseCase->>Service: update_schedule(...) Service->>Repo: get_by_agent_id_and_id_or_raise Service->>Repo: get_by_agent_id_and_name(new-name) conflict check alt name conflict Service-->>Client: 400 ClientError else name available Service->>Temporal: update_schedule(temporal_id, cadence/window) Service->>Repo: update(row with new name) Repo-->>Client: AgentRunScheduleResponse end ``` </a> <a href="#gh-dark-mode-only"> ```mermaid %%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Client participant Router participant NameResolver as _resolve_name_alias_and_check participant UseCase as AgentRunSchedulesUseCase participant Service as AgentRunScheduleService participant Repo as AgentRunScheduleRepository participant Auth as AuthorizationService participant Temporal Note over Client,Temporal: ID-based route (stable handle) Client->>Router: "GET /schedules/{schedule_id}" Router->>Auth: "check(resource=schedule(selector), op=read)" Auth-->>Router: ok Router->>UseCase: get_schedule(agent_id, schedule_id) UseCase->>Service: get_schedule(agent_id, schedule_id) Service->>Repo: get_by_agent_id_and_id_or_raise(agent_id, schedule_id) Repo-->>Service: AgentRunScheduleEntity Service->>Temporal: describe_schedule(temporal_id) Temporal-->>Service: ScheduleDescription Service-->>Client: AgentRunScheduleResponse Note over Client,Temporal: Name-alias route (resolves to ID first) Client->>Router: "GET /schedules/name/{name}" Router->>NameResolver: "resolve(agent_id, name, op=read)" NameResolver->>UseCase: get_schedule_id_by_name(agent_id, name) UseCase->>Service: get_schedule_id_by_name(agent_id, name) Service->>Repo: get_by_agent_id_and_name(agent_id, name) alt schedule not found Repo-->>NameResolver: None - raise ItemDoesNotExist(name-message) NameResolver-->>Client: 404 (name-based message) else schedule found Repo-->>NameResolver: schedule_id NameResolver->>Auth: "check(resource=schedule(selector by id), op=read)" alt denied Auth-->>NameResolver: AuthorizationError NameResolver-->>Client: 404 (same name-based message, hides id) else allowed NameResolver-->>Router: schedule_id Router->>UseCase: get_schedule(agent_id, schedule_id) UseCase-->>Client: AgentRunScheduleResponse end end Note over Client,Temporal: Schedule rename Client->>Router: "PATCH /schedules/{schedule_id} name=new-name" Router->>Auth: "check(op=update)" Router->>UseCase: update_schedule(agent_id, schedule_id, request) UseCase->>Service: update_schedule(...) Service->>Repo: get_by_agent_id_and_id_or_raise Service->>Repo: get_by_agent_id_and_name(new-name) conflict check alt name conflict Service-->>Client: 400 ClientError else name available Service->>Temporal: update_schedule(temporal_id, cadence/window) Service->>Repo: update(row with new name) Repo-->>Client: AgentRunScheduleResponse end ``` </a> </details> <!-- greptile_failed_comments --> <details open><summary><h3>Comments Outside Diff (1)</h3></summary> 1. `agentex/database/migrations/alembic/versions/2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py`, line 42-52 ([link](https://github.com/scaleapi/scale-agentex/blob/34dc76ff1fa4b3caafbde8f4a024f2a38af8b150/agentex/database/migrations/alembic/versions/2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py#L42-L52)) <a href="#"><img alt="P2" src="https://greptile-static-assets.s3.amazonaws.com/badges/p2.svg?v=9" align="top"></a> **Downgrade will fail after name reuse** The `downgrade()` function tries to build a full (non-partial) unique index on `(agent_id, name)` covering all rows, including soft-deleted ones. Once the system runs under the new partial index, it becomes possible for a deleted row and an active row to share the same `(agent_id, name)` — for example, after a schedule is deleted and then re-created under the same name. In that state `CREATE UNIQUE INDEX CONCURRENTLY … ON agent_run_schedules (agent_id, name)` will fail with a uniqueness violation, leaving the database without any `(agent_id, name)` uniqueness constraint while the partial index has not yet been dropped. A data-cleanup step would be needed before a safe rollback can be performed. <details><summary>Prompt To Fix With AI</summary> `````markdown This is a comment left during a code review. Path: agentex/database/migrations/alembic/versions/2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py Line: 42-52 Comment: **Downgrade will fail after name reuse** The `downgrade()` function tries to build a full (non-partial) unique index on `(agent_id, name)` covering all rows, including soft-deleted ones. Once the system runs under the new partial index, it becomes possible for a deleted row and an active row to share the same `(agent_id, name)` — for example, after a schedule is deleted and then re-created under the same name. In that state `CREATE UNIQUE INDEX CONCURRENTLY … ON agent_run_schedules (agent_id, name)` will fail with a uniqueness violation, leaving the database without any `(agent_id, name)` uniqueness constraint while the partial index has not yet been dropped. A data-cleanup step would be needed before a safe rollback can be performed. How can I resolve this? If you propose a fix, please make it concise. ````` </details> <a href="https://app.greptile.com/api/ide/cursor?prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0APath%3A%20agentex%2Fdatabase%2Fmigrations%2Falembic%2Fversions%2F2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py%0ALine%3A%2042-52%0A%0AComment%3A%0A**Downgrade%20will%20fail%20after%20name%20reuse**%0A%0AThe%20%60downgrade%28%29%60%20function%20tries%20to%20build%20a%20full%20%28non-partial%29%20unique%20index%20on%20%60%28agent_id%2C%20name%29%60%20covering%20all%20rows%2C%20including%20soft-deleted%20ones.%20Once%20the%20system%20runs%20under%20the%20new%20partial%20index%2C%20it%20becomes%20possible%20for%20a%20deleted%20row%20and%20an%20active%20row%20to%20share%20the%20same%20%60%28agent_id%2C%20name%29%60%20%E2%80%94%20for%20example%2C%20after%20a%20schedule%20is%20deleted%20and%20then%20re-created%20under%20the%20same%20name.%20In%20that%20state%20%60CREATE%20UNIQUE%20INDEX%20CONCURRENTLY%20%E2%80%A6%20ON%20agent_run_schedules%20%28agent_id%2C%20name%29%60%20will%20fail%20with%20a%20uniqueness%20violation%2C%20leaving%20the%20database%20without%20any%20%60%28agent_id%2C%20name%29%60%20uniqueness%20constraint%20while%20the%20partial%20index%20has%20not%20yet%20been%20dropped.%20A%20data-cleanup%20step%20would%20be%20needed%20before%20a%20safe%20rollback%20can%20be%20performed.%0A%0AHow%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20please%20make%20it%20concise.&pr=349&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCursorDark.svg?v=6"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCursor.svg?v=6"><img alt="Fix in Cursor" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCursor.svg?v=6"></picture></a> <a href="https://app.greptile.com/ide/claude-code?prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0APath%3A%20agentex%2Fdatabase%2Fmigrations%2Falembic%2Fversions%2F2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py%0ALine%3A%2042-52%0A%0AComment%3A%0A**Downgrade%20will%20fail%20after%20name%20reuse**%0A%0AThe%20%60downgrade%28%29%60%20function%20tries%20to%20build%20a%20full%20%28non-partial%29%20unique%20index%20on%20%60%28agent_id%2C%20name%29%60%20covering%20all%20rows%2C%20including%20soft-deleted%20ones.%20Once%20the%20system%20runs%20under%20the%20new%20partial%20index%2C%20it%20becomes%20possible%20for%20a%20deleted%20row%20and%20an%20active%20row%20to%20share%20the%20same%20%60%28agent_id%2C%20name%29%60%20%E2%80%94%20for%20example%2C%20after%20a%20schedule%20is%20deleted%20and%20then%20re-created%20under%20the%20same%20name.%20In%20that%20state%20%60CREATE%20UNIQUE%20INDEX%20CONCURRENTLY%20%E2%80%A6%20ON%20agent_run_schedules%20%28agent_id%2C%20name%29%60%20will%20fail%20with%20a%20uniqueness%20violation%2C%20leaving%20the%20database%20without%20any%20%60%28agent_id%2C%20name%29%60%20uniqueness%20constraint%20while%20the%20partial%20index%20has%20not%20yet%20been%20dropped.%20A%20data-cleanup%20step%20would%20be%20needed%20before%20a%20safe%20rollback%20can%20be%20performed.%0A%0AHow%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20please%20make%20it%20concise.&repo=scaleapi%2Fscale-agentex&pr=349&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInClaudeDark.svg?v=6"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInClaude.svg?v=6"><img alt="Fix in Claude Code" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixInClaude.svg?v=6"></picture></a> <a href="https://app.greptile.com/api/ide/codex?prompt=IMPORTANT%3A%20Work%20in%20the%20repository%20%22scaleapi%2Fscale-agentex%22%20on%20the%20existing%20branch%20%22jerome%2Fschedule-id-handles%22.%20Checkout%20that%20branch%20%E2%80%94%20do%20NOT%20create%20a%20new%20branch%20or%20open%20a%20new%20PR.%20Push%20your%20changes%20to%20%22jerome%2Fschedule-id-handles%22.%0A%0AThis%20is%20a%20comment%20left%20during%20a%20code%20review.%0APath%3A%20agentex%2Fdatabase%2Fmigrations%2Falembic%2Fversions%2F2026_07_08_1626_schedule_identity_id_handles_9a4b8c7d6e5f.py%0ALine%3A%2042-52%0A%0AComment%3A%0A**Downgrade%20will%20fail%20after%20name%20reuse**%0A%0AThe%20%60downgrade%28%29%60%20function%20tries%20to%20build%20a%20full%20%28non-partial%29%20unique%20index%20on%20%60%28agent_id%2C%20name%29%60%20covering%20all%20rows%2C%20including%20soft-deleted%20ones.%20Once%20the%20system%20runs%20under%20the%20new%20partial%20index%2C%20it%20becomes%20possible%20for%20a%20deleted%20row%20and%20an%20active%20row%20to%20share%20the%20same%20%60%28agent_id%2C%20name%29%60%20%E2%80%94%20for%20example%2C%20after%20a%20schedule%20is%20deleted%20and%20then%20re-created%20under%20the%20same%20name.%20In%20that%20state%20%60CREATE%20UNIQUE%20INDEX%20CONCURRENTLY%20%E2%80%A6%20ON%20agent_run_schedules%20%28agent_id%2C%20name%29%60%20will%20fail%20with%20a%20uniqueness%20violation%2C%20leaving%20the%20database%20without%20any%20%60%28agent_id%2C%20name%29%60%20uniqueness%20constraint%20while%20the%20partial%20index%20has%20not%20yet%20been%20dropped.%20A%20data-cleanup%20step%20would%20be%20needed%20before%20a%20safe%20rollback%20can%20be%20performed.%0A%0AHow%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20please%20make%20it%20concise.&repo=scaleapi%2Fscale-agentex&pr=349&platform=github"><picture><source media="(prefers-color-scheme: dark)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCodexDark.svg?v=6"><source media="(prefers-color-scheme: light)" srcset="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCodex.svg?v=6"><img alt="Fix in Codex" src="https://greptile-static-assets.s3.amazonaws.com/badges/FixInCodex.svg?v=6"></picture></a> </details> <!-- /greptile_failed_comments --> <sub>Reviews (5): Last reviewed commit: ["Merge branch &#39;main&#39; into jerome/schedule..."](0754859) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=42773859)</sub> <!-- /greptile_comment --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent b49fab6 commit 3b722f6

13 files changed

Lines changed: 806 additions & 168 deletions
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
"""move run schedule identity to row id
2+
3+
Revision ID: 9a4b8c7d6e5f
4+
Revises: e7f8a9b0c1d2
5+
Create Date: 2026-07-08 16:26:00.000000
6+
7+
Switches schedule names from reserved external handles to mutable labels by
8+
making the name uniqueness constraint apply only to active rows. Schema-only and
9+
idempotent: index creation/drop use CONCURRENTLY with IF EXISTS / IF NOT EXISTS.
10+
"""
11+
12+
from collections.abc import Sequence
13+
14+
import sqlalchemy as sa
15+
from alembic import op
16+
17+
# revision identifiers, used by Alembic.
18+
revision: str = "9a4b8c7d6e5f"
19+
down_revision: str | None = "e7f8a9b0c1d2"
20+
branch_labels: str | Sequence[str] | None = None
21+
depends_on: str | Sequence[str] | None = None
22+
23+
24+
def upgrade() -> None:
25+
with op.get_context().autocommit_block():
26+
op.execute(
27+
"CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS "
28+
"uq_agent_run_schedules_active_agent_name "
29+
"ON agent_run_schedules (agent_id, name) "
30+
"WHERE deleted_at IS NULL"
31+
)
32+
op.execute(
33+
"DROP INDEX CONCURRENTLY IF EXISTS uq_agent_run_schedules_agent_name"
34+
)
35+
36+
37+
def downgrade() -> None:
38+
connection = op.get_bind()
39+
duplicate = connection.execute(
40+
sa.text(
41+
"SELECT 1 FROM agent_run_schedules "
42+
"GROUP BY agent_id, name "
43+
"HAVING COUNT(*) > 1 "
44+
"LIMIT 1"
45+
)
46+
).scalar()
47+
if duplicate is not None:
48+
raise RuntimeError(
49+
"Downgrade is unsafe after schedule name reuse. Clean up duplicate "
50+
"(agent_id, name) rows before recreating "
51+
"uq_agent_run_schedules_agent_name."
52+
)
53+
54+
with op.get_context().autocommit_block():
55+
op.execute(
56+
"CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS "
57+
"uq_agent_run_schedules_agent_name "
58+
"ON agent_run_schedules (agent_id, name)"
59+
)
60+
op.execute(
61+
"DROP INDEX CONCURRENTLY IF EXISTS "
62+
"uq_agent_run_schedules_active_agent_name"
63+
)

0 commit comments

Comments
 (0)