Skip to content

fix: mark parent job failed when build times out or is manually cleared - #98

Merged
frostebite merged 1 commit into
mainfrom
fix/mark-job-failed-when-build-times-out
May 15, 2026
Merged

fix: mark parent job failed when build times out or is manually cleared#98
frostebite merged 1 commit into
mainfrom
fix/mark-job-failed-when-build-times-out

Conversation

@frostebite

@frostebite frostebite commented May 15, 2026

Copy link
Copy Markdown
Member

Summary

  • cleanUpBuildsThatDidntReportBack marks a build failed after the 6-hour GitHub Actions timeout but never called markFailureForJob, leaving the parent job stuck in inProgress
  • Same gap in manualCleanUp
  • With maxConcurrentJobs=9, these ghost-inProgress jobs consumed all queue capacity — confirmed as the cause of the current blockage where 9 inProgress jobs with 9 failed builds were holding 53 created jobs from scheduling

Three-part fix:

  1. cleanUpBuildsThatDidntReportBack — call markFailureForJob after marking build as failed so the Ingeminator picks it up next cron cycle
  2. manualCleanUp — same fix for the manual path
  3. transitionStaleInProgressJobsToFailed — new cleaner step that detects active jobs where all builds have settled (none are started) and transitions them to failed, unblocking capacity for jobs already stuck before this deploy

Adds CiBuilds.hasAnyStartedBuildsForJob to support the new cleaner step.

Why it wasn't caught by PR #97

requeueActiveJobsWithoutBuilds (added in #97) only fires for jobs with no build records at all. The stuck jobs had build records — just failed ones — so they fell through every existing self-healing path.

Test plan

  • typecheck passes (yarn typecheck)
  • Deploy and confirm the 9 currently-stuck inProgress jobs transition to failed on the next cleaner run
  • Confirm the 53 created editor jobs begin scheduling once capacity is freed
  • Simulate a future timeout: let a build go 6h without reporting, confirm cleaner marks both build and job as failed, and Ingeminator retries on the next cycle

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of stale and stuck build jobs by properly marking them as failed when no active builds are detected, preventing jobs from remaining in an incomplete state indefinitely.

Review Change Stack

When cleanUpBuildsThatDidntReportBack marks a build as failed after the
6-hour GitHub Actions timeout, or manualCleanUp marks a build as failed,
the parent job was left in its current state (typically inProgress).

The Ingeminator only processes jobs in "failed" status, so these builds
were never retried. Worse, with maxConcurrentJobs=9 the ghost-inProgress
jobs consumed all queue capacity, blocking all 53 created jobs from
being scheduled.

Three-part fix:
- cleanUpBuildsThatDidntReportBack: call markFailureForJob after marking
  build as failed so the Ingeminator picks it up next cycle
- manualCleanUp: same fix for the manual path
- transitionStaleInProgressJobsToFailed: new cleaner step that detects
  active jobs where all builds have settled (none are started) and
  transitions them to failed, recovering capacity for builds already stuck

Adds CiBuilds.hasAnyStartedBuildsForJob to support the new step.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR adds job failure marking across build cleanup operations. A new query method checks for started builds, enabling detection of stale in-progress jobs with no remaining builds. These jobs are now transitioned to failed status. Additionally, existing cleanup methods (manualCleanUp and cleanUpBuildsThatDidntReportBack) now also mark their related jobs as failed when builds are marked failed.

Changes

Job Failure Marking in Build Cleanup

Layer / File(s) Summary
Query helper for detecting started builds
functions/src/model/ciBuilds.ts
New hasAnyStartedBuildsForJob(jobId) method queries ciBuilds collection for started builds matching a job ID, returning a boolean indicating whether any exist.
Stale job transition to failed
functions/src/logic/buildQueue/cleaner.ts
New transitionStaleInProgressJobsToFailed() method scans active jobs older than the stale threshold, identifies those with no remaining started builds via the new query helper, and marks them as failed with alerting. Integrated into cleanUp flow after requeuing stale jobs.
Job failure marking in existing cleanup methods
functions/src/logic/buildQueue/cleaner.ts
manualCleanUp and cleanUpBuildsThatDidntReportBack now mark the related job as failed when a started build is determined to be missing or stuck on DockerHub and is marked as failed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • game-ci/versioning-backend#90: Both PRs modify functions/src/logic/buildQueue/cleaner.ts to enhance Cleaner handling of failed builds—fix: reconcile failed builds against DockerHub in the Cleaner #90 adds DockerHub-based reconciliation/publishing for Firestore "failed" builds, while this PR updates Cleaner cleanup flows to mark related jobs as failed.
  • game-ci/versioning-backend#97: Both PRs modify the build-queue cleanup/self-healing logic in functions/src/logic/buildQueue/cleaner.ts to reconcile or transition stale/started jobs/builds and add supporting query helpers.
  • game-ci/versioning-backend#83: Both PRs modify Cleaner.manualCleanUp around DockerHub verification for started builds, with this PR adding job failure marking when builds fail DockerHub checks.

Suggested reviewers

  • webbertakken
  • GabLeRoux

Poem

🐰 A query hops, a job's fate sealed,
When stale builds vanish, nothing healed,
Now cleanup marks them gone and failed,
Through every path where builds have sailed,
Our CI systems stand more true! 🎯

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: marking parent jobs as failed when builds timeout or are manually cleared, which directly addresses the core problem described in the PR.
Description check ✅ Passed The description provides comprehensive detail including problem statement, three-part solution, context from PR #97, and a test plan, though the required template sections (Changes list and Checklist) are not explicitly formatted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/mark-job-failed-when-build-times-out

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

functions/src/logic/buildQueue/cleaner.ts

Oops! Something went wrong! :(

ESLint: 10.3.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.

functions/src/model/ciBuilds.ts

Oops! Something went wrong! :(

ESLint: 10.3.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@frostebite
frostebite merged commit 6a654fb into main May 15, 2026
4 of 5 checks passed
@frostebite
frostebite deleted the fix/mark-job-failed-when-build-times-out branch May 15, 2026 01:00
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.

1 participant