Repository navigation
fix(desktop,shared): stop calling an already-run schedule invalid #6618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { | ||
| isValidRrule, | ||
| rruleProblem, | ||
| timezoneAbbreviation, | ||
| type Weekday, | ||
| } from "@superset/shared/rrule"; | ||
|
|
@@ -77,10 +77,14 @@ export function ScheduleSentence({ | |
| }; | ||
|
|
||
| const customDraft = state.customRrule.trim(); | ||
| const customValid = useMemo(() => isValidRrule(customDraft), [customDraft]); | ||
| const customProblem = useMemo(() => rruleProblem(customDraft), [customDraft]); | ||
| // The saved rule itself can be exhausted (a run-once schedule that already | ||
| // ran); that is history, not an edit gone wrong, so only a changed draft | ||
| // gets the complaint. | ||
| const draftEdited = customDraft !== "" && customDraft !== rrule; | ||
|
|
||
| const commitCustom = () => { | ||
| if (!customDraft || customDraft === rrule || !customValid) return; | ||
| if (!draftEdited || customProblem) return; | ||
|
Comment on lines
+80
to
+87
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
file='apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/ScheduleSentence/ScheduleSentence.tsx'
printf '%s\n' '--- component outline ---'
ast-grep outline "$file" 2>/dev/null || true
printf '%s\n' '--- relevant component lines ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- rruleProblem definitions and uses ---'
rg -n -C 5 'rruleProblem|commitCustom|customProblem|draftEdited' apps/desktop/src/rendererRepository: superset-sh/superset Length of output: 16379 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- shared rrule files ---'
rg --files | rg '(^|/)(rrule|.*rrule.*)(/|\\.)'
printf '%s\n' '--- rruleProblem implementation and tests ---'
rg -n -C 12 'function rruleProblem|const rruleProblem|rruleProblem\\s*=|rruleProblem\\(' --glob '*.{ts,tsx,js,jsx}' .
printf '%s\n' '--- package/runtime clues ---'
rg -n 'rrule|RRULE|DTSTART|COUNT|UNTIL' packages apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations --glob '*.{ts,tsx,json}' | head -240Repository: superset-sh/superset Length of output: 185 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- tracked files containing rruleProblem ---'
git grep -n -C 12 'rruleProblem' -- '*.ts' '*.tsx' '*.js' '*.jsx' || true
printf '%s\n' '--- tracked files containing shared rrule exports ---'
git grep -n -E 'from ["'\'']`@superset/shared/rrule`|export .*rrule|rrule.*from' -- '*.ts' '*.tsx' '*.js' '*.jsx' | head -240 || true
printf '%s\n' '--- likely shared package files ---'
git ls-files | grep -Ei '(^|/)(shared|packages)(/|$)|rrule' | head -300Repository: superset-sh/superset Length of output: 29004 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- rrule implementation ---'
sed -n '1,180p' packages/shared/src/rrule.ts
sed -n '250,435p' packages/shared/src/rrule.ts
printf '%s\n' '--- automation save validation ---'
git grep -n -C 15 'isValidRrule\|nextOccurrenceAfter\|rrule' -- packages/trpc/src/router/automation apps/api/src/app/api/automations | head -320
printf '%s\n' '--- rrule tests ---'
git ls-files | grep -E 'packages/shared/.*(test|spec)|rrule.*(test|spec)' || trueRepository: superset-sh/superset Length of output: 37289 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- rrule tests around validation ---'
git grep -n -C 12 'rruleProblem\|isValidRrule\|COUNT\|UNTIL' -- packages/shared/src/rrule.test.ts
printf '%s\n' '--- ScheduleSentence consumers ---'
git grep -n -C 15 '<ScheduleSentence\|ScheduleSentence' -- apps/desktop/src/renderer | head -320
printf '%s\n' '--- onRruleChange handlers and mutations ---'
git grep -n -C 12 'onRruleChange\|rrule.*update\|update.*rrule' -- apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations | head -360Repository: superset-sh/superset Length of output: 43807 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- trigger provider set contract ---'
git grep -n -C 18 'type TriggerProvider\|interface TriggerProvider\|function set\|const set' -- apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations | head -360
printf '%s\n' '--- save trigger validation ---'
git grep -n -C 18 'function saveTriggerSet\|const saveTriggerSet\|parseRrule({' -- packages/trpc/src/router/automation | head -420
printf '%s\n' '--- create/update mutation calls ---'
git grep -n -C 15 'automations.create\|automations.update\|create.mutate\|update.mutate' -- apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations | head -420Repository: superset-sh/superset Length of output: 50376 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- trigger-set validation implementation ---'
sed -n '1,150p' packages/trpc/src/router/automation/triggerSet.ts
printf '%s\n' '--- AutomationBody trigger editor and update wiring ---'
rg -n -C 12 'TriggersEditor|updateMutation|set\\(' 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/$automationId/components/AutomationBody/AutomationBody.tsx' | head -260
printf '%s\n' '--- exact update input schema and trigger branch ---'
rg -n -C 12 'AutomationUpdatePatch|update:|triggers:' packages/trpc/src/router/automation/automation.ts | head -300Repository: superset-sh/superset Length of output: 7488 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- trigger problem rules ---'
git grep -n -C 18 'describeTriggerProblems\|summarizeTriggerProblems' -- packages/shared packages/trpc | head -300
printf '%s\n' '--- AutomationBody exact trigger editor section ---'
file='apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/$automationId/components/AutomationBody/AutomationBody.tsx'
sed -n '1,180p' "$file"Repository: superset-sh/superset Length of output: 19457 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
from datetime import datetime, timedelta, timezone
# Minimal model of rruleProblem's time-dependent UNTIL check for
# FREQ=HOURLY: with DTSTART=now and after=now+epsilon, the next hourly
# occurrence is one hour later while UNTIL is still in the future.
t0 = datetime(2026, 8, 1, 12, 0, 0, tzinfo=timezone.utc)
until = t0 + timedelta(hours=1, minutes=30)
def problem(at):
# rruleProblem creates both DTSTART and `after` from the current time.
next_occurrence = at + timedelta(hours=1)
return None if next_occurrence <= until else "exhausted"
initial = problem(t0)
later = problem(t0 + timedelta(hours=2))
memoized_commit_result = initial
print({
"rule": "FREQ=HOURLY;UNTIL=20260801T133000Z",
"initial_rruleProblem": initial,
"later_rruleProblem": later,
"memoized_result_used_by_commitCustom": memoized_commit_result,
"fresh_result_needed_by_commitCustom": later,
"stale_validation_allows_emit": memoized_commit_result is None and later == "exhausted",
})
assert initial is None
assert later == "exhausted"
assert memoized_commit_result is None
assert later == "exhausted"
PYRepository: superset-sh/superset Length of output: 406 Revalidate the rule when
🤖 Prompt for AI Agents |
||
| emit(customDraft); | ||
| }; | ||
|
|
||
|
|
@@ -161,9 +165,11 @@ export function ScheduleSentence({ | |
| if (event.key === "Enter") commitCustom(); | ||
| }} | ||
| /> | ||
| {customDraft && !customValid && ( | ||
| {draftEdited && customProblem && ( | ||
| <span className="select-text cursor-text text-xs text-destructive"> | ||
| Invalid recurrence rule — changes aren't saved | ||
| {customProblem === "exhausted" | ||
| ? "No upcoming runs — changes aren't saved" | ||
| : "Invalid recurrence rule — changes aren't saved"} | ||
| </span> | ||
| )} | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -397,18 +397,29 @@ export function parseRrule(args: { | |
| * editors can gate saves instead of persisting rules the server will reject. | ||
| */ | ||
| export function isValidRrule(rrule: string): boolean { | ||
| if (!parseRruleParts(rrule)) return false; | ||
| return rruleProblem(rrule) === null; | ||
| } | ||
|
|
||
| /** | ||
| * Why a rule can't be saved: `unparseable` (not an RRULE at all) or | ||
| * `exhausted` (well-formed, but COUNT/UNTIL leaves nothing in the future — | ||
| * a run-once schedule that already ran looks exactly like this). Null when | ||
| * the rule is fine. | ||
| */ | ||
| export function rruleProblem( | ||
| rrule: string, | ||
| ): "unparseable" | "exhausted" | null { | ||
| if (!parseRruleParts(rrule)) return "unparseable"; | ||
| try { | ||
| return ( | ||
| nextOccurrenceAfter({ | ||
| rrule, | ||
| dtstart: new Date(), | ||
| timezone: "UTC", | ||
| after: new Date(), | ||
| }) !== null | ||
| ); | ||
| const next = nextOccurrenceAfter({ | ||
| rrule, | ||
| dtstart: new Date(), | ||
| timezone: "UTC", | ||
| after: new Date(), | ||
| }); | ||
| return next === null ? "exhausted" : null; | ||
| } catch { | ||
| return false; | ||
| return "unparseable"; | ||
|
Comment on lines
+409
to
+422
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 6 'COUNT=1|rruleProblem\(|nextOccurrenceAfter\(|dtstart|timezone' \
packages/shared apps/desktopRepository: superset-sh/superset Length of output: 49550 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- shared RRULE tests and implementation ---'
rg -n -C 10 'rruleProblem|isValidRrule|COUNT=1|COUNT|validateRrule' \
packages/shared/src/rrule.test.ts packages/shared/src/rrule.ts packages/shared/src \
| head -n 500
printf '%s\n' '--- ScheduleSentence relevant code ---'
sed -n '1,180p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/ScheduleSentence/ScheduleSentence.tsx
printf '%s\n' '--- create/edit validation call sites ---'
rg -n -C 12 'isValidRrule|validateRrule|customProblem|draftEdited|rruleProblem|rrule' \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations \
packages/server packages/shared/src \
| head -n 700Repository: superset-sh/superset Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- tracked candidate files ---'
git ls-files | rg '(^|/)(automation|automations|rrule|trigger|schedule|trpc)' | head -n 300
printf '%s\n' '--- validation and recurrence call sites ---'
rg -n --glob '*.ts' --glob '*.tsx' \
'validateRrule|isValidRrule|rruleProblem|nextOccurrenceAfter|parseRrule\(' \
. | head -n 300
printf '%s\n' '--- schedule config lifecycle ---'
rg -n --glob '*.ts' --glob '*.tsx' \
'createScheduleConfig|dtstart:|timezone:|ScheduleSentence' \
apps packages | head -n 400Repository: superset-sh/superset Length of output: 25806 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- API recurrence and automation validation ---'
rg -n -C 12 --glob '*.ts' --glob '*.tsx' \
'validateRrule|isValidRrule|rruleProblem|nextOccurrenceAfter|parseRrule|dtstart|COUNT|UNTIL' \
apps/api packages/shared apps/desktop/src/lib apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations \
| head -n 700 || true
printf '%s\n' '--- focused component locations ---'
rg -n -C 8 --glob '*.ts' --glob '*.tsx' \
'ScheduleSentence|createScheduleConfig|dtstart|onRruleChange|customProblem|commitCustom' \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components \
| head -n 700 || true
printf '%s\n' '--- API automation files ---'
git ls-files apps/api | rg 'automation|trigger|schedule|trpc' | head -n 200Repository: superset-sh/superset Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- schedule provider ---'
sed -n '1,140p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/providers/schedule/schedule.tsx
printf '%s\n' '--- ScheduleSentence call sites ---'
rg -l --glob '*.ts' --glob '*.tsx' 'ScheduleSentence' \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations \
| while IFS= read -r file; do
echo "--- $file"
rg -n -C 12 'ScheduleSentence|dtstart|onRruleChange|timezone' "$file"
done
printf '%s\n' '--- trigger editor and create dialog schedule state ---'
rg -n -C 15 --glob '*.ts' --glob '*.tsx' \
'triggers|config|createScheduleConfig|dtstart|timezone|onChange|onSave|mutate' \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/TriggersEditor \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/CreateAutomationDialog \
| head -n 500
printf '%s\n' '--- evaluator recurrence code ---'
rg -n -C 15 --glob '*.ts' --glob '*.tsx' \
'nextOccurrenceAfter|nextOccurrences|dtstart|timezone|COUNT|UNTIL|trigger.enabled' \
apps/api/src packages/shared/src apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/'$'\n' \
| head -n 500Repository: superset-sh/superset Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- schedule provider ---'
sed -n '1,140p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/providers/schedule/schedule.tsx
printf '%s\n' '--- ScheduleSentence call sites ---'
rg -l --glob '*.ts' --glob '*.tsx' 'ScheduleSentence' \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations \
| while IFS= read -r file; do
echo "--- $file"
rg -n -C 12 'ScheduleSentence|dtstart|onRruleChange|timezone' "$file"
done
printf '%s\n' '--- trigger editor and create dialog schedule state ---'
rg -n -C 15 --glob '*.ts' --glob '*.tsx' \
'triggers|config|createScheduleConfig|dtstart|timezone|onChange|onSave|mutate' \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/TriggersEditor \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/CreateAutomationDialog \
| head -n 500
printf '%s\n' '--- evaluator recurrence code ---'
rg -n -C 15 --glob '*.ts' --glob '*.tsx' \
'nextOccurrenceAfter|nextOccurrences|dtstart|timezone|COUNT|UNTIL|trigger.enabled' \
apps/api/src packages/shared/src \
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/ \
| head -n 500Repository: superset-sh/superset Length of output: 50377 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from datetime import datetime, timedelta, timezone
def count_one_next(dtstart: datetime, after: datetime):
# For FREQ=DAILY;COUNT=1, the only occurrence is DTSTART.
return dtstart if dtstart > after else None
synthetic_dtstart = datetime(2026, 8, 15, 12, 0, 0, tzinfo=timezone.utc)
synthetic_after = synthetic_dtstart + timedelta(milliseconds=1)
real_after = datetime(2026, 8, 15, 12, 0, 0, tzinfo=timezone.utc)
real_future_dtstart = datetime(2026, 8, 15, 12, 5, 0, tzinfo=timezone.utc)
assert count_one_next(synthetic_dtstart, synthetic_after) is None
assert count_one_next(real_future_dtstart, real_after) == real_future_dtstart
print("synthetic DTSTART followed by after: exhausted")
print("future real DTSTART after an earlier comparison time: one upcoming occurrence")
PY
printf '%s\n' '--- shared trigger validation ---'
rg -n -C 20 \
'describeTriggerProblems|scheduleTriggerConfigSchema|nextOccurrenceAfter|dtstart|timezone' \
packages/shared/src/automation-triggers.ts packages/shared/src/rrule.ts \
| head -n 300Repository: superset-sh/superset Length of output: 20985 Use the real schedule context when classifying exhaustion.
🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 50377
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 38031
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 50377
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 26295
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 28301
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 5185
Track schedule exhaustion per trigger. The evaluator disables all schedule triggers when one finite schedule has no next occurrence. Each other trigger keeps its
nextRunAt, so this code rendersRan ...for schedules that did not run. Update exhaustion by trigger ID and use explicit exhaustion or last-execution state for theRanlabel.🤖 Prompt for AI Agents