Skip to content

ci: add concurrency group and job timeouts to typecheck workflow - #62257

Open
mr-raj12 wants to merge 2 commits into
NousResearch:mainfrom
mr-raj12:ci/typecheck-concurrency-timeout
Open

mr-raj12 wants to merge 2 commits into
NousResearch:mainfrom
mr-raj12:ci/typecheck-concurrency-timeout

Conversation

@mr-raj12

Copy link
Copy Markdown

What does this PR do?

typecheck.yml is the only reusable (workflow_call) workflow in the repo with neither a concurrency: group nor a timeout-minutes on its jobs. Every other reusable workflow (contributor-check.yml, docker-lint.yml, docs-site-checks.yml, history-check.yml, lint.yml, osv-scanner.yml, supply-chain-audit.yml, tests.yml, uv-lockfile-check.yml) already sets both.

This brings it in line:

  • Adds a workflow-level concurrency: group so superseded typecheck runs on the same ref are cancelled instead of piling up.
  • Adds timeout-minutes to both jobs so a hung step fails fast rather than inheriting the GitHub Actions 6 hour default and holding a runner for hours.

The concurrency: block uses the same <name>-${{ github.ref }} / cancel-in-progress: true shape as lint.yml.

Related Issue

Fixes #62256

This is separate from #45722 / #45731, which cover only the read-only permissions: block on the same file. This PR does not touch permissions:.

Type of Change

  • ♻️ Refactor (no behavior change) — CI configuration only, no product code touched

Changes Made

  • .github/workflows/typecheck.yml:
    • Added a concurrency: block (group: typecheck-${{ github.ref }}, cancel-in-progress: true).
    • Added timeout-minutes: 10 to the typecheck job.
    • Added timeout-minutes: 15 to the desktop-build job.

How to Test

  1. python3 -c "import yaml; yaml.safe_load(open('.github/workflows/typecheck.yml'))" parses cleanly.
  2. Push two commits in quick succession to a branch that triggers this workflow; the earlier typecheck run is cancelled once the newer one starts.
  3. On any run, both Check TypeScript and Build desktop app now show their configured timeout in the job settings.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — N/A: CI workflow YAML only, no Python code paths touched. Validated with yaml.safe_load and cross-checked against the sibling workflows.
  • I've added tests for my changes — N/A: CI configuration change.
  • I've tested on my platform: Ubuntu 24.04

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — N/A, runners are ubuntu-latest
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Cancel superseded runs on the same ref and cap both jobs so a hung
step fails fast instead of holding a runner for hours.

Signed-off-by: Mrityunjay Raj <mr.raj.earth@gmail.com>
@alt-glitch alt-glitch added type/feature New feature or request sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation P3 Low — cosmetic, nice to have labels Jul 10, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tightening the Typecheck workflow. The missing timeout/concurrency settings still exist on current main, but there is one CI-semantics issue.

Problems

  • .github/workflows/typecheck.yml:9 sets unconditional cancellation. The caller intentionally limits cancellation to pull requests in .github/workflows/ci.yml:25-27; it also runs on pushes to main (.github/workflows/ci.yml:13-16) and invokes this reusable workflow at :73-77. The proposed value would cancel post-merge Typecheck validation for successive main pushes.

Suggested changes

  • Set cancel-in-progress to ${{ github.event_name == 'pull_request' }} to preserve the caller's PR-only policy, while keeping the group and the two job timeouts.

Automated hermes-sweeper review.

Comment thread .github/workflows/typecheck.yml Outdated

concurrency:
group: typecheck-${{ github.ref }}
cancel-in-progress: true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this unconditionally cancels Typecheck runs, including runs invoked by ci.yml on pushes to main. The caller deliberately limits cancellation to pull requests at .github/workflows/ci.yml:25-27; use ${{ github.event_name == 'pull_request' }} here to preserve post-merge validation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 7cb3bc5.

cancel-in-progress is now ${{ github.event_name == 'pull_request' }}, matching the caller's policy in ci.yml (ci-${{ github.ref }} / PR-only cancellation). Since ci.yml triggers this reusable workflow on both pull_request and push to main, successive main pushes now keep their post-merge Typecheck runs instead of being cancelled. The concurrency group and the two job timeouts are unchanged.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 11, 2026
The caller (ci.yml) runs this reusable workflow on both pull_request and
push to main, and limits cancellation to PRs. Mirror that policy so a
successive push to main does not cancel post-merge Typecheck validation.

Signed-off-by: Mrityunjay Raj <mr.raj.earth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: typecheck workflow lacks a concurrency group and job timeouts

3 participants