Fix Trigger timezone and occurrence scheduling semantics - #833
Draft
Y1fe1Zh0u wants to merge 6 commits into
Draft
Conversation
Tenant and Agent timezone values now enter the scheduler through one IANA validation boundary, while new tenants receive the confirmed Beijing default. Constraint: Agent timezone remains nullable to preserve tenant inheritance. Rejected: Validate against COMMON_TIMEZONES | it is a UI shortlist, not the complete IANA set. Confidence: high Scope-risk: narrow Directive: Do not reintroduce silent UTC fallback for configured Trigger timezones. Tested: backend/tests/test_timezone_validation.py (14 passed); Alembic single-head check. Not-tested: Migration execution against a production-sized tenants table.
The evaluator now owns Cron occurrence calculation in the Agent effective timezone. The daemon, dispatch key, queue record, and Runtime source all consume that same planned instant, with a bounded 30-second grace and creation-time lower bound. Constraint: Keep cooldown_seconds and non-Cron Trigger behavior compatible. Rejected: Persist next_run_at or schedule_effective_from | the accepted design uses current rules plus a bounded grace without new schedule state. Confidence: high Scope-risk: moderate Directive: Do not derive Cron occurrences from last_fired_at or recompute them below evaluator. Tested: 22 scheduling, queue, intake, completion, and A2A tests; scoped Ruff. Not-tested: DST edge behavior and second-level Cron expressions are out of scope.
Scheduled Trigger intake failures now roll back the occurrence instead of persisting a failed receipt that blocks the next daemon scan. Structured logs retain the failure evidence, while webhook receipts preserve their existing synchronous failure contract. Constraint: Retry is only the ordinary 15-second scan while the occurrence remains inside the 30-second grace. Rejected: Add retry counters, backoff jobs, or new schedule fields | outside the confirmed minimal repair. Confidence: high Scope-risk: narrow Directive: A failed scheduled intake must not consume its occurrence identity. Tested: 40 timezone, scheduling, queue, Runtime, A2A, and webhook tests; scoped Ruff. Not-tested: Process termination during the database commit syscall.
Cron creation already rejected malformed expressions, but both existing update boundaries could persist them. Validate before mutation and commit while preserving REST replacement and Agent-tool patch semantics. Constraint: Preserve current Trigger APIs and config compatibility Rejected: Introduce a shared Trigger config abstraction | interface unification remains deferred Confidence: high Scope-risk: narrow Reversibility: clean Directive: Trigger config timezone may remain stored for compatibility but must not define occurrence timezone Tested: Full backend pytest 2162 passed; focused Trigger tests 18 passed; scoped Ruff and git diff checks passed Not-tested: DST-specific and second-level Cron behavior remain out of scope
The scheduler already falls back through Agent and Tenant to Asia/Shanghai, but the Agent detail response still advertised UTC when both stored values were absent. Reuse the platform default so the visible configuration describes the runtime behavior. Constraint: Preserve the existing Agent detail response shape Rejected: Add a new frontend timezone resolution path | the backend already owns effective timezone resolution Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any displayed effective timezone must follow the same Agent to Tenant to platform-default order as scheduling Tested: Full backend pytest 2163 passed; timezone tests 15 passed; scoped Ruff and git diff checks passed Not-tested: Frontend visual regression was unnecessary because the response field shape is unchanged
Current main routes Trigger persistence through query_dao and has advanced the Alembic chain. Preserve those boundaries while replaying the scheduling repair, move Cron imports to the module boundary, and adapt tests to the DAO session contract. Constraint: Publish without rebasing the user's dirty working tree Constraint: New migrations must follow the current single-head and DDL-only rules Rejected: Rebase the checked-out fix branch | unrelated concurrent work makes worktree mutation unsafe Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the PR branch based on current upstream main and retain query_dao session ownership Tested: Focused timezone and Trigger regression 33 passed; scoped Ruff passed; Alembic reports one f061 head Not-tested: Full backend suite was interrupted before completion
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Agent.timezone -> Tenant.timezone -> Asia/Shanghailast_fired_atand apply the agreed 30-second grace without historical catch-upWhy
The evaluator and dispatch previously interpreted Cron independently. That gave one business fact, the planned occurrence time, two competing sources. Registration time was then persisted as though it were the planned time, and
last_fired_atcould move the calendar schedule forward from actual dispatch time.The repair makes the evaluator the owner of
scheduled_at. Downstream components consume that occurrence without recalculating it. Runtime completion remains outside Trigger scheduling semantics: a successful atomic Runtime registration counts as one wake-up, while later Runtime success or failure belongs to Runtime itself.Compatibility and scope
cooldown_secondsbehavior is preservedupstream/mainand retains itsquery_daosession boundary and single-head migration chainValidation
33 passed2173 passed, 9 warningsf061_default_tenant_timezone