[codex] Align Hyperping cron healthchecks#2340
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors cron healthcheck grace computation to derive values from each task's scheduled cadence, exports the payload builder and main function for testing, adds optional ChangesCron Healthcheck Grace Computation and Start Ping
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
riderx
force-pushed
the
codex/hyperping-cron-healthcheck-start-grace
branch
from
a278c41 to
98ebb63
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260526131411_cron_healthcheck_start_ping.sql`:
- Around line 31-37: The function process_queue_with_healthcheck currently does
a dynamic EXECUTE of 'SELECT count(*) FROM pgmq.%I' for each queue_name
(populating queue_size), which causes full-table scans; replace that per-queue
count with a call to the existing metrics function (e.g. SELECT queue_length
FROM pgmq.metrics(queue_name)) and cast/convert that result into queue_size
(int) to drive the calls_needed computation, updating any variable names
(queue_size) and logic that rely on the previous count; after changing, run
EXPLAIN (ANALYZE, BUFFERS) comparing the new metrics-based lookup vs the old
count(*) on representative queues to validate performance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 997d7ae2-739b-4a85-afba-4c675720e35a
📒 Files selected for processing (5)
scripts/create_hyperping_cron_healthchecks.tssupabase/functions/_backend/triggers/queue_consumer.tssupabase/migrations/20260526131411_cron_healthcheck_start_ping.sqltests/create-hyperping-cron-healthchecks.unit.test.tstests/queue-consumer-message-shape.unit.test.ts
|
I checked the new migration against the stored schema snapshot. 20260526131411_cron_healthcheck_start_ping.sql replaces process_queue_with_healthcheck, but supabase/schemas/prod.sql still has the previous body with no healthcheck_start payload. Risk: the next schema regeneration/diff can look dirty or accidentally drop the start-ping change from the checked-in schema. Verification: |
riderx
force-pushed
the
codex/hyperping-cron-healthcheck-start-grace
branch
from
98ebb63 to
679d1a4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/migrations/20260526131411_cron_healthcheck_start_ping.sql (1)
1-85:⚠️ Potential issue | 🟠 MajorUpdate
supabase/schemas/prod.sqlschema snapshot forprocess_queue_with_healthcheck
supabase/migrations/20260526131411_cron_healthcheck_start_ping.sqldefinespublic.process_queue_with_healthcheck(..., healthcheck_url)withhealthcheck_startlogic, butsupabase/schemas/prod.sqlcontains no matches forprocess_queue_with_healthcheckand no occurrences ofhealthcheck_start/healthcheck_start_pending—so the schema snapshot is missing this new function body (confirm/update to prevent schema drift).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@supabase/migrations/20260526131411_cron_healthcheck_start_ping.sql` around lines 1 - 85, The schema snapshot supabase/schemas/prod.sql is missing the new function definition for public.process_queue_with_healthcheck (and the new healthcheck_start / healthcheck_start_pending logic), causing schema drift; update prod.sql to include the exact CREATE OR REPLACE FUNCTION body added in 20260526131411_cron_healthcheck_start_ping.sql (the function name process_queue_with_healthcheck, parameters queue_names, batch_size, healthcheck_url and the healthcheck_start/healthcheck_start_pending handling and PERFORM net.http_post calls) so the repository schema matches the migration and then commit the updated prod.sql.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/create_hyperping_cron_healthchecks.ts`:
- Around line 167-169: In getGracePeriodParts, the grace window is being rounded
up via Math.ceil which can push a capped max (e.g., 61s) past the configured
max; change the rounding to round down to the nearest minute after applying the
1-minute floor: compute cappedGraceSeconds = Math.min(maxGracePeriodSeconds,
cadenceSeconds - 1), then compute minutes = Math.floor(Math.max(60,
cappedGraceSeconds) / 60) and pass minutes*60 into getPeriodParts so the grace
never exceeds maxGracePeriodSeconds; update references in the
getGracePeriodParts function accordingly.
In `@supabase/migrations/20260526131411_cron_healthcheck_start_ping.sql`:
- Around line 37-39: Replace the unqualified LEAST call with the fully qualified
pg_catalog.least in the expression that computes calls_needed (which uses
queue_size and batch_size and already uses pg_catalog.ceil); also scan the
surrounding function to confirm the search_path is cleared (set to empty) and
that all other PostgreSQL built-ins are fully qualified (e.g., pg_catalog.ceil)
to satisfy the migration hardening rules.
---
Outside diff comments:
In `@supabase/migrations/20260526131411_cron_healthcheck_start_ping.sql`:
- Around line 1-85: The schema snapshot supabase/schemas/prod.sql is missing the
new function definition for public.process_queue_with_healthcheck (and the new
healthcheck_start / healthcheck_start_pending logic), causing schema drift;
update prod.sql to include the exact CREATE OR REPLACE FUNCTION body added in
20260526131411_cron_healthcheck_start_ping.sql (the function name
process_queue_with_healthcheck, parameters queue_names, batch_size,
healthcheck_url and the healthcheck_start/healthcheck_start_pending handling and
PERFORM net.http_post calls) so the repository schema matches the migration and
then commit the updated prod.sql.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d6fe4fa-98be-4ecf-aa4e-9e804e7bc262
📒 Files selected for processing (5)
scripts/create_hyperping_cron_healthchecks.tssupabase/functions/_backend/triggers/queue_consumer.tssupabase/migrations/20260526131411_cron_healthcheck_start_ping.sqltests/create-hyperping-cron-healthchecks.unit.test.tstests/queue-consumer-message-shape.unit.test.ts
riderx
force-pushed
the
codex/hyperping-cron-healthcheck-start-grace
branch
from
679d1a4 to
0b8367d
Compare
riderx
force-pushed
the
codex/hyperping-cron-healthcheck-start-grace
branch
from
0b8367d to
20ed71c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/migrations/20260526131411_cron_healthcheck_start_ping.sql`:
- Around line 18-20: The migration currently allows a single healthcheck_url to
be shared across multiple queue_names (in cron_healthcheck_start_ping and
related logic), which can report success prematurely; update the migration so
healthcheck_url is only used when exactly one queue_name is targeted or else
nullified/rewritten per-queue. Concretely, inside cron_healthcheck_start_ping
(and the same guard area covering the logic in the 27-57 block) validate that
queue_names contains exactly one entry before assigning/forwarding
healthcheck_url, or transform healthcheck_url into a per-queue URL (e.g., append
the queue_name) so each queue has its own completion signal; use the symbols
healthcheck_url, queue_names, and queue_consumer to locate and modify the checks
and assignment logic.
In `@tests/create-hyperping-cron-healthchecks.unit.test.ts`:
- Around line 28-70: Add a new test case in the
create_hyperping_cron_healthchecks suite that covers cron-expression branches by
building a cron-style taskRow via createCronTaskRow with
run_at_minute/run_at_hour and/or run_on_dow/run_on_day set (instead of
second_/minute_/hour_interval), then call buildHealthcheckPayload and assert the
resulting candidate.payload.period_type/period_value and
candidate.payload.grace_period_value/grace_period_type match the expected
cron-schedule behavior (e.g., one-minute period for run_at_minute, correct
minutes/hours units, and proper grace rounding/capping as in other tests);
reference buildHealthcheckPayload and createCronTaskRow to locate where to add
the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 85ba8370-d6f3-401c-aec1-c67d2f12ccd9
📒 Files selected for processing (5)
scripts/create_hyperping_cron_healthchecks.tssupabase/functions/_backend/triggers/queue_consumer.tssupabase/migrations/20260526131411_cron_healthcheck_start_ping.sqltests/create-hyperping-cron-healthchecks.unit.test.tstests/queue-consumer-message-shape.unit.test.ts
riderx
force-pushed
the
codex/hyperping-cron-healthcheck-start-grace
branch
from
20ed71c to
22e93fb
Compare
riderx
force-pushed
the
codex/hyperping-cron-healthcheck-start-grace
branch
from
22e93fb to
8402bb9
Compare
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/create_hyperping_cron_healthchecks.ts`:
- Around line 181-184: The cron row handling allows both run_on_day and
run_on_dow to be set which creates inconsistent semantics between
getCronExpression(), process_all_cron_tasks, and cadence/grace calculation in
scripts/create_hyperping_cron_healthchecks.ts; fix by choosing one approach:
either enforce mutual exclusivity at write/DB level (add validation in the
create/update path or DB check constraint to ensure exactly one of run_on_day or
run_on_dow is non-null) or change the cron/grace generation to treat both fields
as combined (AND) everywhere—update getCronExpression(), process_all_cron_tasks,
and the cadence/grace logic in create_hyperping_cron_healthchecks.ts to use the
same AND semantics so schedules match the scheduler.
In `@supabase/functions/_backend/triggers/queue_consumer.ts`:
- Around line 967-968: The startup healthcheck ping is currently sent after
acquiring the DB client so if getPgClient(c) throws we never call
maybePingCronHealthcheckStart; move the call to
maybePingCronHealthcheckStart(healthcheckUrl) to occur before any DB setup in
the background task (i.e., before the getPgClient(c) invocation and related DB
initialization code) so the /start ping is sent even on init failures; ensure
you still guard the call with healthcheckUrl !== null and await the ping before
proceeding to create the client.
In `@tests/create-hyperping-cron-healthchecks.unit.test.ts`:
- Around line 28-81: The tests never exercise the weekly/monthly branches in
getScheduledCadenceSeconds, so add unit cases that call buildHealthcheckPayload
with createCronTaskRow rows setting run_on_dow (weekly) and run_on_day (monthly)
and assert the generated payload.period_type/period_value and
payload.grace_period_type/pretty_value (e.g., weekly should produce days/hours
as expected, monthly should produce days/weeks or hours per your function
logic); specifically add one test that sets run_on_dow (e.g., [1] or 0-based
weekday) and verifies cadence seconds and derived grace, and another that sets
run_on_day (e.g., 15) and verifies monthly cadence and grace derived by
getScheduledCadenceSeconds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74b9c575-d7ed-4ccc-86d8-dcaf87d2dd5b
📒 Files selected for processing (4)
scripts/create_hyperping_cron_healthchecks.tssupabase/functions/_backend/triggers/queue_consumer.tstests/create-hyperping-cron-healthchecks.unit.test.tstests/queue-consumer-message-shape.unit.test.ts
Comment on lines
+181
to
+184
| if (row.run_on_day !== null) | ||
| return 28 * 24 * 60 * 60 | ||
| if (row.run_on_dow !== null) | ||
| return 7 * 24 * 60 * 60 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Searching for cron_tasks constraints and validators around run_on_day/run_on_dow...'
rg -n -C3 "cron_tasks|run_on_day|run_on_dow" supabase scripts testsRepository: Cap-go/capgo
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Searching for constraints/checks enforcing mutual exclusivity between run_on_day and run_on_dow...'
rg -n --no-heading --stats --hidden \
'run_on_day\s+IS\s+NULL|run_on_dow\s+IS\s+NULL|run_on_day.*run_on_dow|run_on_dow.*run_on_day|CHECK\s*\(|constraint.*cron_tasks|ALTER TABLE.*cron_tasks' \
supabase/schemas supabase/migrations | head -n 200
echo 'Searching for any application-layer validation around run_on_day/run_on_dow...'
rg -n --no-heading --stats 'run_on_day|run_on_dow' scripts src supabase | head -n 200Repository: Cap-go/capgo
Length of output: 27157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Searching for constraint/policy definitions mentioning both run_on_day and run_on_dow...'
rg -n --no-heading 'cron_tasks.*run_on_(day|dow)|run_on_(day|dow).*cron_tasks|run_on_day.*run_on_dow|run_on_dow.*run_on_day|CHECK\s*\(.*run_on_day.*run_on_dow|CHECK\s*\(.*run_on_dow.*run_on_day|constraint.*cron_tasks.*run_on_day|constraint.*cron_tasks.*run_on_dow' \
supabase/schemas/prod.sql supabase/migrations | head -n 200
echo 'Inspecting the cron_tasks table definition block in supabase/schemas/prod.sql...'
python3 - <<'PY'
import re, itertools, pathlib
p = pathlib.Path("supabase/schemas/prod.sql")
text = p.read_text(encoding="utf-8", errors="ignore")
m = re.search(r'CREATE TABLE IF NOT EXISTS "public"\."cron_tasks" \((.*?)\);\s*', text, re.S)
if not m:
print("cron_tasks CREATE TABLE block not found")
raise SystemExit(0)
block = m.group(1)
lines = block.splitlines()
print(f"cron_tasks CREATE TABLE block lines: {len(lines)}")
for i,line in enumerate(lines,1):
if "run_on_dow" in line or "run_on_day" in line or "CHECK" in line or "CONSTRAINT" in line:
print(f"{i:4d}:{line}")
PY
echo 'Searching for any application code that rejects both run_on_day and run_on_dow at once...'
rg -n --no-heading 'run_on_(day|dow).*non|null|non|null.*run_on_(day|dow)|validate.*run_on_(day|dow)|if\s*\(.*run_on_day.*\)\s*\{|\brun_on_day\b.*\brun_on_dow\b|run_on_day.*run_on_dow' \
scripts src supabase/functions | head -n 200Repository: Cap-go/capgo
Length of output: 145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="scripts/create_hyperping_cron_healthchecks.ts"
echo "=== Snippet around cadence + cron expression helpers in $FILE ==="
sed -n '150,260p' "$FILE"
echo "=== Snippet around getCronExpression emission in $FILE ==="
sed -n '260,380p' "$FILE"
echo "=== Snippet around getScheduledCadenceSeconds in $FILE ==="
sed -n '160,220p' "$FILE"
echo "=== Snippet around cron runner logic (process_all_cron_tasks) in supabase/migrations/20260521210531_cron_hyperping_healthchecks.sql ==="
sed -n '100,190p' supabase/migrations/20260521210531_cron_hyperping_healthchecks.sqlRepository: Cap-go/capgo
Length of output: 12369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# show cron_tasks create table + nearby constraints
sed -n '16060,16220p' supabase/schemas/prod.sql | cat -n | head -n 200
sed -n '16220,16380p' supabase/schemas/prod.sql | cat -n | head -n 200
echo '--- constraints/policies related to cron_tasks in prod.sql ---'
rg -n --no-heading 'ADD CONSTRAINT.*cron_tasks|CONSTRAINT.*cron_tasks|cron_tasks.*CHECK|CHECK.*cron_tasks|CREATE POLICY.*cron_tasks|ALTER TABLE.*cron_tasks' supabase/schemas/prod.sql | head -n 200
echo '--- lines in prod.sql containing run_on_day/run_on_dow that mention constraints/combined logic ---'
rg -n --no-heading 'run_on_day.*run_on_dow|run_on_dow.*run_on_day|run_on_day.*IS NOT NULL.*run_on_dow|run_on_dow.*IS NOT NULL.*run_on_day|CHECK.*run_on_day|CHECK.*run_on_dow' supabase/schemas/prod.sql | head -n 200Repository: Cap-go/capgo
Length of output: 12574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Checking migrations for any CHECK/constraint mentioning run_on_day or run_on_dow...'
rg -n --no-heading 'CONSTRAINT.*run_on_(day|dow)|CHECK\s*\(.*run_on_(day|dow)|ALTER TABLE.*cron_tasks.*run_on_(day|dow)' supabase/migrations | head -n 200
echo 'Checking backend/app code for any validation preventing both run_on_day/run_on_dow...'
rg -n --no-heading 'run_on_(day|dow)' supabase/functions | head -n 200
echo 'Checking client/app for any cron_tasks insert/update logic...'
rg -n --no-heading 'cron_tasks.*(insert|update|upsert|modify)|from\(\s*[\x27"]cron_tasks[\x27"]\s*\)' scripts src supabase/functions | head -n 200Repository: Cap-go/capgo
Length of output: 140
🏁 Script executed:
#!/bin/bash
set -u
echo 'Migrations: CHECK/constraint mentioning run_on_day or run_on_dow'
rg -n --no-heading 'CONSTRAINT.*run_on_(day|dow)|CHECK\s*\(.*run_on_(day|dow)|ALTER TABLE.*cron_tasks.*run_on_(day|dow)' supabase/migrations || true
echo 'Backend/functions: any validation/logic around run_on_day/run_on_dow'
rg -n --no-heading 'run_on_(day|dow)' supabase/functions || true
echo 'App scripts/types: any validation/logic around run_on_day/run_on_dow'
rg -n --no-heading 'run_on_(day|dow)' scripts src || true
echo 'Schemas: any constraint mentioning run_on_day/run_on_dow'
rg -n --no-heading 'CONSTRAINT.*run_on_(day|dow)|CHECK\s*\(.*run_on_(day|dow)|ALTER TABLE.*cron_tasks.*run_on_(day|dow)' supabase/schemas || trueRepository: Cap-go/capgo
Length of output: 2010
🌐 Web query:
Hyperping cron expression day of month vs day of week semantics OR AND
💡 Result:
In standard Linux cron implementations, the relationship between the day-of-month and day-of-week fields is governed by OR semantics [1][2][3]. If both fields are "restricted" (i.e., they contain values other than the wildcard *), the cron job will trigger whenever the current time matches either the specified day-of-month OR the specified day-of-week [2][4][5]. For example, an expression like 0 8 15 * 1 will trigger at 8:00 AM on the 15th of the month, AND at 8:00 AM every Monday, rather than only on Mondays that happen to fall on the 15th [1][5]. This behavior differs significantly from other cron systems, such as Quartz Scheduler or AWS EventBridge, which often do not allow both fields to be specified simultaneously (typically requiring a ? placeholder in one of the fields) to avoid this ambiguity [6][1]. While Hyperping provides a cron expression generator [7][8], its core cron scheduling behavior typically relies on the underlying system's implementation (often standard Linux cron or a library-based equivalent) [9][10]. If you require AND logic (i.e., "the first Monday of every month"), you cannot achieve this with a single standard cron expression [1]. Instead, it is common practice to use a wrapper or "shell guard" to filter execution [1]: 0 8 1-7 * 1 [ $(date +%u) -eq 1 ] && /your/command This executes the job daily between the 1st and 7th, but only proceeds if the current day of the week is Monday [1].
Citations:
- 1: https://dev.to/lumm_7b4812001a80f4b531cf/7-cron-expression-gotchas-that-will-silently-break-your-scheduled-jobs-5f6a
- 2: https://man.archlinux.org/man/crontab.5.en
- 3: https://unix.stackexchange.com/questions/602328/are-the-day-of-month-and-day-of-week-crontab-fields-mutually-exclusive
- 4: https://superuser.com/questions/428807/run-a-cron-job-on-the-first-monday-of-every-month
- 5: https://stackoverflow.com/questions/34357126/why-crontab-uses-or-when-both-day-of-month-and-day-of-week-specified
- 6: https://www.quartz-scheduler.org/api/1.8.6/org/quartz/CronExpression.html
- 7: https://dev.to/w932635477bit/i-built-6-free-developer-tools-for-ai-apis-cron-docker-and-self-hosting-g2n
- 8: https://hyperping.com/cron-expression-generator/cron-every-weekday
- 9: https://hyperhq.gitbook.io/docs-hyper-sh/index-1/index/cron
- 10: https://github.com/hyperhq/docs.hyper.sh-hyper/blob/master/Feature/container/cron.md
Add/require mutual exclusivity for cron_tasks.run_on_day vs cron_tasks.run_on_dow (or align semantics).
public.cron_tasksdefines bothrun_on_dayandrun_on_dowas nullable with no CHECK/constraint preventing both from being set.scripts/create_hyperping_cron_healthchecks.tsderives cadence/grace usingrun_on_dayfirst, andgetCronExpression()includes both fields when both are non-null.process_all_cron_tasksapplies both day constraints together (AND), while standard cron evaluation of day-of-month/day-of-week is typically OR—so mixed rows can make Hyperping’s expected schedule diverge from actual execution.
Fix by enforcing “only one of run_on_day / run_on_dow” at the DB/write path, or by changing the cron/grace generation to match the scheduler’s AND behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/create_hyperping_cron_healthchecks.ts` around lines 181 - 184, The
cron row handling allows both run_on_day and run_on_dow to be set which creates
inconsistent semantics between getCronExpression(), process_all_cron_tasks, and
cadence/grace calculation in scripts/create_hyperping_cron_healthchecks.ts; fix
by choosing one approach: either enforce mutual exclusivity at write/DB level
(add validation in the create/update path or DB check constraint to ensure
exactly one of run_on_day or run_on_dow is non-null) or change the cron/grace
generation to treat both fields as combined (AND) everywhere—update
getCronExpression(), process_all_cron_tasks, and the cadence/grace logic in
create_hyperping_cron_healthchecks.ts to use the same AND semantics so schedules
match the scheduler.
Comment on lines
+967
to
+968
| if (healthcheckUrl !== null) | ||
| await maybePingCronHealthcheckStart(healthcheckUrl) |
Contributor
There was a problem hiding this comment.
Send /start ping before DB client acquisition to cover init failures.
If getPgClient(c) throws, the run fails without ever reporting /start, which can keep healthchecks green for startup-path failures. Move maybePingCronHealthcheckStart before DB setup in the background task.
Suggested fix
await backgroundTask(c, (async () => {
cloudlog({ requestId: c.get('requestId'), message: `[Background Queue Sync] Starting background execution for queue: ${queueName} with batch size: ${finalBatchSize}` })
let db: ReturnType<typeof getPgClient> | null = null
try {
- db = getPgClient(c)
if (healthcheckUrl !== null)
await maybePingCronHealthcheckStart(healthcheckUrl)
+ db = getPgClient(c)
const result = await processQueue(c, db, queueName, finalBatchSize)
await maybePingCronHealthcheck(db, queueName, result, healthcheckUrl)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/_backend/triggers/queue_consumer.ts` around lines 967 -
968, The startup healthcheck ping is currently sent after acquiring the DB
client so if getPgClient(c) throws we never call maybePingCronHealthcheckStart;
move the call to maybePingCronHealthcheckStart(healthcheckUrl) to occur before
any DB setup in the background task (i.e., before the getPgClient(c) invocation
and related DB initialization code) so the /start ping is sent even on init
failures; ensure you still guard the call with healthcheckUrl !== null and await
the ping before proceeding to create the client.
Comment on lines
+28
to
+81
| describe('create_hyperping_cron_healthchecks', () => { | ||
| it.concurrent('uses second-based periods and floors grace to one minute', () => { | ||
| const candidate = buildHealthcheckPayload(createCronTaskRow({ | ||
| second_interval: 10, | ||
| }), 10 * 60, 'UTC') | ||
|
|
||
| expect(candidate.payload.period_value).toBe(10) | ||
| expect(candidate.payload.period_type).toBe('seconds') | ||
| expect(candidate.payload.grace_period_value).toBe(1) | ||
| expect(candidate.payload.grace_period_type).toBe('minutes') | ||
| }) | ||
|
|
||
| it.concurrent('uses one-minute grace for one-minute cron tasks', () => { | ||
| const candidate = buildHealthcheckPayload(createCronTaskRow({ | ||
| minute_interval: 1, | ||
| }), 10 * 60, 'UTC') | ||
|
|
||
| expect(candidate.payload.period_value).toBe(1) | ||
| expect(candidate.payload.period_type).toBe('minutes') | ||
| expect(candidate.payload.grace_period_value).toBe(1) | ||
| expect(candidate.payload.grace_period_type).toBe('minutes') | ||
| }) | ||
|
|
||
| it.concurrent('keeps long-schedule grace at the configured maximum', () => { | ||
| const candidate = buildHealthcheckPayload(createCronTaskRow({ | ||
| hour_interval: 2, | ||
| }), 10 * 60, 'UTC') | ||
|
|
||
| expect(candidate.payload.period_value).toBe(2) | ||
| expect(candidate.payload.period_type).toBe('hours') | ||
| expect(candidate.payload.grace_period_value).toBe(10) | ||
| expect(candidate.payload.grace_period_type).toBe('minutes') | ||
| }) | ||
|
|
||
| it.concurrent('rounds grace down so it does not exceed the configured maximum', () => { | ||
| const candidate = buildHealthcheckPayload(createCronTaskRow({ | ||
| hour_interval: 2, | ||
| }), 61, 'UTC') | ||
|
|
||
| expect(candidate.payload.grace_period_value).toBe(1) | ||
| expect(candidate.payload.grace_period_type).toBe('minutes') | ||
| }) | ||
|
|
||
| it.concurrent('derives grace for daily cron schedules', () => { | ||
| const candidate = buildHealthcheckPayload(createCronTaskRow({ | ||
| run_at_hour: 3, | ||
| run_at_minute: 15, | ||
| }), 2 * 60 * 60, 'UTC') | ||
|
|
||
| expect(candidate.payload.cron).toBe('15 3 * * *') | ||
| expect(candidate.payload.grace_period_value).toBe(2) | ||
| expect(candidate.payload.grace_period_type).toBe('hours') | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add explicit weekly/monthly cadence cases.
This suite never hits the new run_on_dow and run_on_day branches in getScheduledCadenceSeconds(), so regressions in weekly/monthly grace derivation would still pass.
🧪 Example additions
describe('create_hyperping_cron_healthchecks', () => {
+ it.concurrent('derives grace for weekly cron schedules', () => {
+ const candidate = buildHealthcheckPayload(createCronTaskRow({
+ run_at_hour: 3,
+ run_at_minute: 15,
+ run_on_dow: 1,
+ }), 2 * 60 * 60, 'UTC')
+
+ expect(candidate.payload.cron).toBe('15 3 * * 1')
+ expect(candidate.payload.grace_period_value).toBe(2)
+ expect(candidate.payload.grace_period_type).toBe('hours')
+ })
+
+ it.concurrent('derives grace for monthly cron schedules', () => {
+ const candidate = buildHealthcheckPayload(createCronTaskRow({
+ run_at_hour: 3,
+ run_at_minute: 15,
+ run_on_day: 1,
+ }), 2 * 60 * 60, 'UTC')
+
+ expect(candidate.payload.cron).toBe('15 3 1 * *')
+ expect(candidate.payload.grace_period_value).toBe(2)
+ expect(candidate.payload.grace_period_type).toBe('hours')
+ })
+
it.concurrent('uses second-based periods and floors grace to one minute', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/create-hyperping-cron-healthchecks.unit.test.ts` around lines 28 - 81,
The tests never exercise the weekly/monthly branches in
getScheduledCadenceSeconds, so add unit cases that call buildHealthcheckPayload
with createCronTaskRow rows setting run_on_dow (weekly) and run_on_day (monthly)
and assert the generated payload.period_type/period_value and
payload.grace_period_type/pretty_value (e.g., weekly should produce days/hours
as expected, monthly should produce days/weeks or hours per your function
logic); specifically add one test that sets run_on_dow (e.g., [1] or 0-based
weekday) and verifies cadence seconds and derived grace, and another that sets
run_on_day (e.g., 15) and verifies monthly cadence and grace derived by
getScheduledCadenceSeconds.
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
2 participants
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.
Summary (AI generated)
/startpings once per cron queue run before queue processing starts./startURL generation and healthcheck period/grace generation.Motivation (AI generated)
Cron queue failures were visible in queue failure alerts, but Hyperping could stay green because only completion pings were sent and the configured grace period could let a later run satisfy an earlier missed completion window. The healthcheck creation script also generated
59 secondsgrace for one-minute jobs, which Hyperping surfaced as59 minutesin the UI.Business Impact (AI generated)
This makes cron monitoring more trustworthy for production operations. Capgo should get clearer downtime signals when queue-backed cron jobs stall or fail, reducing silent operational risk around manifest processing and other scheduled backend work.
Test Plan (AI generated)
bunx vitest run tests/create-hyperping-cron-healthchecks.unit.test.ts tests/queue-consumer-message-shape.unit.test.tsbun run typecheck:backendbun run lint:backendgit diff --checkbun run cli:typecheck && bun run typecheck:backend && bun run typecheck:frontendbun run supabase:startbun run supabase:with-env -- bunx vitest run tests/cron-healthchecks.test.ts tests/security-definer-execute-hardening.test.tsSummary by CodeRabbit
New Features
Bug Fixes
Chores