Skip to content

ci: disable stale cron workflows and remove their matching controllers#52

Open
colinwilliams91 wants to merge 1 commit into
devflowinc:mainfrom
colinwilliams91:fix/issue-50-disable-stale-cron-workflows
Open

ci: disable stale cron workflows and remove their matching controllers#52
colinwilliams91 wants to merge 1 commit into
devflowinc:mainfrom
colinwilliams91:fix/issue-50-disable-stale-cron-workflows

Conversation

@colinwilliams91

Copy link
Copy Markdown

Closes #50. Cross-reference: #49 (compose fix) flagged these failing crons in its "Out of scope" section -- this PR is the follow-up. Also references follow-up issue #51 (stale SELF_HOST.md:86 "Bull Queue Manager UI" doc reference).

What & why

Two scheduled GitHub Actions workflows have been failing on every single run (~336 runs/day, ~10,000 failed runs/month):

  • Check Queues -- .github/workflows/check-queues.yml, cron */5 * * * *
  • Clean Every 30 Minutes Before 24h Completed Jobs -- .github/workflows/clean-before-24h-complete-jobs.yml, cron 30 * * * *

Root cause (from the run logs)

From run 29581536093:

##[group]Run response=$(curl ... https://api.firecrawl.dev/admin//check-queues)
                                                  ^^  double slash = empty BULL_AUTH_KEY
env:
  BULL_AUTH_KEY:           <- empty
Failed to check queues. Response: 404
  1. BULL_AUTH_KEY is not configured as a repo secret, so ${{ secrets.BULL_AUTH_KEY }} resolves to empty, the URL becomes admin//check-queues, and the request returns HTTP 404.
  2. api.firecrawl.dev is upstream firecrawl's (Mendable's) own SaaS, not this fork's deployment. This repo is a fork of firecrawl/firecrawl; the firecrawl org owns api.firecrawl.dev. These workflows were copied verbatim when the fork diverged and were never adapted to a fork-owned endpoint (and no fork-owned prod endpoint is documented in this repo).

Why disabling (and removing the matching controllers) is safe

Verified against the code. Even if these were re-pointed at a fork-owned deployment with the secret set, the operational benefit would be nil:

  1. check-queues calls checkAlerts() (apps/api/src/services/alerts/index.ts), which is a no-op by default. Guarded by process.env.ENV === "production" AND process.env.ALERT_NUM_ACTIVE_JOBS AND process.env.ALERT_NUM_WAITING_JOBS -- none of which appear in .env.example, docker-compose.yaml, README.md, SELF_HOST.md, or CONTRIBUTING.md. When unset, the controller just logs "Initializing alerts" and returns 200. Slack output was previously stripped (commit b024f08 cleanup: remove SLACK_WEBHOOK_URL env and code); zero SLACK_* env reads remain in the repo. The only consumer of checkAlerts() was checkQueuesController. So a correctly-pointed check-queues cron would emit only local Logger.info/warn lines that nobody reads.
  2. clean-before-24h-complete-jobs is largely redundant. BullMQ's queue is created with removeOnComplete: { age: 90000 } (25h) and removeOnFail: { age: 90000 } (apps/api/src/services/queue-service.ts:18-23), so completed/failed jobs auto-evict after ~25h anyway. The controller also caps at 9 batches x 10 = ~90 jobs per call (apps/api/src/controllers/v0/admin/queue.ts:16-27), inadequate for any real backlog.
  3. Upstream itself has fully removed both workflows AND the matching controllers. .github/workflows/check-queues.yml, .github/workflows/clean-before-24h-complete-jobs.yml, apps/api/src/controllers/v0/admin/queue.ts, and apps/api/src/services/alerts.ts are all GONE from firecrawl/firecrawl. This fork is carrying cold ash from before upstream's queue/admin refactor.

Cost note (correction)

The repo is public (visibility: public), and GitHub Actions minutes are free and unlimited for public repos -- so these failing crons are not directly costing money today. The real harms are:

  • ~10k failed runs/month drowning the CI dashboard, hiding real signal from build-docker-images.yml.
  • Each run mints a GITHUB_TOKEN with Contents: write, PullRequests: write, etc. -- small but unnecessary exposure surface on every run.
  • Risk of GitHub's abuse-detection flagging constant scheduled failures.
  • If the repo ever flips private, the bill kicks in immediately: ~1.2k Actions min/month from these two crons alone (against the 2k free allowance).

Changes

  • Move .github/workflows/check-queues.yml -> .github/archive/check-queues.yml, on: [].
  • Move .github/workflows/clean-before-24h-complete-jobs.yml -> .github/archive/clean-before-24h-complete-jobs.yml, on: [].
  • Remove checkQueuesController and cleanBefore24hCompleteJobsController from apps/api/src/controllers/v0/admin/queue.ts (the only callers of those endpoints were the disabled crons); keep queuesController.
  • Remove the matching /admin/<KEY>/check-queues and /admin/<KEY>/clean-before-24h-complete-jobs route registrations from apps/api/src/routes/admin.ts; keep redis-health and queues (both still useful to self-hosters, and upstream also kept redis-health).
  • Delete the now-dead apps/api/src/services/alerts/ directory (its only consumer was checkQueuesController).

Why move + on: [] rather than just on: [] in place: matches the established repo precedent. The 6 SDK workflows already in .github/archive/ all use on: []. Files outside .github/workflows/ are not eligible to be triggered by GitHub; the on: [] is defense-in-depth for the case where someone moves them back and forgets to set the trigger.

Why also remove the matching controllers (not just the crons): the only HTTP callers of checkQueuesController and cleanBefore24hCompleteJobsController were the crons being disabled. checkQueuesController was a no-op by default; cleanBefore24hCompleteJobsController duplicated BullMQ's built-in 25h auto-eviction at far smaller scale. Both were also fully removed upstream. Mirroring upstream's evolution (Option B) keeps this fork from carrying dead code whose only client is itself dead code.

What is NOT changed

  • apps/api/src/controllers/v0/admin/redis-health.ts -- kept (upstream also kept it).
  • queuesController (the /admin/<KEY>/queues JSON health endpoint) -- kept; still usable by self-hosters. Note: SELF_HOST.md:86 describes this as a "Bull Queue Manager UI," which is stale (bull-board was removed in commits c344bc0/003fb21). That stale doc line is tracked separately in issue SELF_HOST.md references a 'Bull Queue Manager UI' that was removed with bull-board #51 -- not addressed here.
  • BULL_AUTH_KEY in apps/api/.env.example, docker-compose.yaml, README, SELF_HOST.md, CONTRIBUTING.md -- kept. Self-hosters actively use it to protect their own admin endpoints to their own deployments. Disabling the crons does not affect self-hoster admin access.
  • apps/api/src/services/queue-service.ts, queue-worker.ts, queue-jobs.ts, rate-limiter.ts -- untouched.
  • README.md, CONTRIBUTING.md, SELF_HOST.md -- untouched, to avoid creating stale docs. Issue SELF_HOST.md references a 'Bull Queue Manager UI' that was removed with bull-board #51 tracks the SELF_HOST.md:86 cleanup separately.

Latent bug (separate follow-up, not addressed here)

apps/api/src/routes/admin.ts:7-12 mounts the admin routes unconditionally. If BULL_AUTH_KEY is unset, routes register at literal /admin/undefined/... paths -- trivially guessable. Upstream gates the whole block behind if (config.BULL_AUTH_KEY). This fork has not backported that guard. Worth a separate issue; flagged here only.

Verification

  • pnpm run build (tsc) in apps/api/ -- passes clean with no orphan-import errors.
  • pnpm test in apps/api/ -- 7 of 11 unit suites pass (74/89 tests). The 4 failing suites (parseTable, urlValidation, html-to-markdown, crawl) are pre-existing environmental issues unrelated to anything removed here:
    • parseTable.test.ts -- TypeError: Cannot read properties of undefined (reading 'load') (Cheerio load issue, library/environment-related).
    • urlValidation.test.ts -- expects "social media scraping" throw but gets a Zod error string instead (test-expectation drift on URL validation logic).
    • html-to-markdown.test.ts -- markdown equality mismatch (needs the Go native module that's not built in this scratch install).
    • crawl.test.ts -- Cannot find module '../../services/idempotency/validate' from 'src/controllers/__tests__/crawl.test.ts' (a pre-existing missing module I did not touch).
  • Repo-wide grep across *.test.ts for admin | redis-health | /queues | alerts | checkQueues | cleanBefore24h | services/alerts -- zero matches. No test file references any of the removed symbols.

Re-enable path (for maintainers)

If a devflowinc/firecrawl-simple deployment is ever stood up at a known URL with a BULL_AUTH_KEY:

  1. git mv both files back to .github/workflows/ and restore on: schedule: [...].
  2. Configure BULL_AUTH_KEY as a repo secret.
  3. Update the https://api.firecrawl.dev/... URL in each curl to point at the actual deployment.
  4. Restore checkQueuesController and cleanBefore24hCompleteJobsController from git history (git checkout HEAD~ -- apps/api/src/controllers/v0/admin/queue.ts apps/api/src/services/alerts/index.ts) and re-add the two route registrations to admin.ts.
  5. If check-queues is wanted as a real alert, also plumb ENV=production, ALERT_NUM_ACTIVE_JOBS, ALERT_NUM_WAITING_JOBS into .env.example and compose, and wire up an output sink (Slack or a webhook) since Slack was stripped.

Closes #50.

devflowinc#50)

Two scheduled cron workflows (check-queues every 5 min, clean-before-24h-complete-jobs every 30 min) have been failing on every run since BULL_AUTH_KEY is not configured as a repo secret AND the curl target api.firecrawl.dev is upstream Mendable's SaaS, not this fork's deployment -- the workflows were copied verbatim when the fork diverged and never adapted. See issue devflowinc#50 for run-log receipts.

Even if these were re-pointed at a fork-owned deployment with the secret set, the operational benefit would be nil: check-queues calls checkAlerts() which is a no-op by default (guarded by ENV=production AND ALERT_NUM_ACTIVE_JOBS AND ALERT_NUM_WAITING_JOBS -- none plumbed in .env.example, compose, or docs; Slack output was stripped in b024f08); clean-before-24h-complete-jobs is largely redundant with BullMQ's removeOnComplete.age=90000 (25h) auto-eviction. Upstream firecrawl/firecrawl has fully removed both workflows AND the matching controllers/v0/admin/queue.ts and services/alerts.ts.

Changes:

- Move .github/workflows/check-queues.yml and clean-before-24h-complete-jobs.yml into .github/archive/ with on: [] (matches the existing archive convention for the 6 SDK workflows).

- Remove checkQueuesController and cleanBefore24hCompleteJobsController from controllers/v0/admin/queue.ts and their route registrations from routes/admin.ts (the only callers of those endpoints were the disabled crons). Keep queuesController (JSON health endpoint, still useful to self-hosters per SELF_HOST.md:86) and redisHealthController (upstream also kept it).

- Delete the now-dead apps/api/src/services/alerts/ directory (its only consumer was checkQueuesController).

Unchanged: BULL_AUTH_KEY stays in apps/api/.env.example and docker-compose.yaml for self-hosters to protect their own admin endpoints to their own deployments. README.md, CONTRIBUTING.md, and SELF_HOST.md untouched -- the stale SELF_HOST.md:86 'Bull Queue Manager UI' reference is tracked separately in issue devflowinc#51. A latent admin-guard bug in admin.ts (routes mount unconditionally at /admin/undefined/... when BULL_AUTH_KEY unset; upstream gates behind if (config.BULL_AUTH_KEY)) is flagged in the PR body for a separate follow-up.

Verified: pnpm run build (tsc) passes clean with no orphan-import errors. pnpm test runs -- 7 of 11 unit suites pass (74/89 tests); the 4 failing suites (parseTable, urlValidation, html-to-markdown, crawl) are pre-existing environmental issues (Cheerio load, native Go html-to-markdown module, services/idempotency/validate missing, test-expectation drift) unrelated to anything removed here. No test file references any of the removed symbols (grep across *.test.ts).
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.

Two scheduled cron workflows fail on every run (~10k/month) -- disable them and remove their matching controllers

1 participant