ci: typecheck github-script code

ci/README.md

@@ -18,6 +18,10 @@ The goal is to eventually have all GitHub specific code in `ci/github-script` an
 A lot of code has already been migrated, but some Bash code still remains.
 New CI features need to be introduced in JavaScript, not Bash.
 
+The JavaScript code is type-checked using TypeScript.
+Run `nix-build ci -A typecheck-ci-scripts` or use `typecheck-ci-scripts` from the repository's `nix-shell`.
+See [`ci/github-script/README.md`](./github-script/README.md) for more details.
+
 ## Nixpkgs merge bot
 
 The Nixpkgs merge bot empowers package maintainers by enabling them to merge PRs related to their own packages.

ci/default.nix

@@ -168,6 +168,7 @@ in
 rec {
   inherit pkgs fmt;
   codeownersValidator = pkgs.callPackage ./codeowners-validator { };
+  typecheck-ci-scripts = pkgs.callPackage ./github-script { };
 
   # FIXME(lf-): it might be useful to test other Nix implementations
   # (nixVersions.stable and Lix) here somehow at some point to ensure we don't

ci/github-script/README.md

@@ -15,3 +15,27 @@ Run `./run commits OWNER REPO PR`, where OWNER is your username or "NixOS", REPO
 ## Labeler
 
 Run `./run labels OWNER REPO`, where OWNER is your username or "NixOS" and REPO the name of your fork or "nixpkgs".
+
+## Type checking
+
+The JavaScript files are type-checked using TypeScript with `--checkJs`. This catches common bugs like using `Set.length` instead of `Set.size`, undeclared variables, and mismatched function signatures.
+
+To run the type checker from the repository root:
+
+```
+nix-build ci -A typecheck-ci-scripts
+```
+
+Or use the `typecheck-ci-scripts` command from the repository's development shell:
+
+```
+nix-shell --run typecheck-ci-scripts
+```
+
+For interactive development, enter `nix-shell` in this directory and run:
+
+```
+tsc --project jsconfig.json
+```
+
+Editors that support `jsconfig.json` (VS Code, etc.) will show type errors inline.

ci/github-script/bot.js

@@ -53,6 +53,7 @@ module.exports = async ({ github, context, core, dry }) => {
 
       await artifactClient.downloadArtifact(artifact.id, {
         findBy: {
+          workflowRunId: run.id,
           repositoryName: context.repo.repo,
           repositoryOwner: context.repo.owner,
           token: core.getInput('github-token'),
@@ -218,7 +219,7 @@ module.exports = async ({ github, context, core, dry }) => {
     // This is intentionally less than the time that Eval takes, so that the label job
     // running after Eval can indeed label the PR as conflicted if that is the case.
     const merge_commit_sha_valid =
-      Date.now() - new Date(pull_request.created_at) > 3 * 60 * 1000
+      Date.now() - new Date(pull_request.created_at).getTime() > 3 * 60 * 1000
 
     const prLabels = {
       // We intentionally don't use the mergeable or mergeable_state attributes.
@@ -313,6 +314,7 @@ module.exports = async ({ github, context, core, dry }) => {
 
       await artifactClient.downloadArtifact(artifact.id, {
         findBy: {
+          workflowRunId: run_id,
           repositoryName: context.repo.repo,
           repositoryOwner: context.repo.owner,
           token: core.getInput('github-token'),
@@ -669,6 +671,7 @@ module.exports = async ({ github, context, core, dry }) => {
             artifact.id,
             {
               findBy: {
+                workflowRunId: lastRun.id,
                 repositoryName: context.repo.repo,
                 repositoryOwner: context.repo.owner,
                 token: core.getInput('github-token'),

ci/github-script/commits.js

@@ -194,7 +194,7 @@ module.exports = async ({ github, context, core, dry, cherryPicks }) => {
     // An empty results array will always trigger this condition, which is helpful
     // to clean up reviews created by the prepare step when on the wrong branch.
     if (results.every(({ severity }) => severity === 'info')) {
-      await dismissReviews({ github, context, dry, reviewKey })
+      await dismissReviews({ github, context, core, dry, reviewKey })
       return
     }
 
@@ -286,7 +286,7 @@ module.exports = async ({ github, context, core, dry, cherryPicks }) => {
         // that's too long. We think this is unlikely to happen, and so don't deal with it explicitly.
         const truncated = []
         let total_length = 0
-        for (line of diff) {
+        for (const line of diff) {
           total_length += line.length
           if (total_length > 10000) {
             truncated.push('', '[...truncated...]')

ci/github-script/default.nix

@@ -0,0 +1,44 @@
+# Type check the CI scripts in ci/github-script using TypeScript
+{
+  importNpmLock,
+  nodejs,
+  runCommand,
+  writeShellScriptBin,
+}:
+let
+  npmDeps = importNpmLock.buildNodeModules {
+    npmRoot = ./.;
+    inherit nodejs;
+  };
+
+  # Files from ci/ that are referenced by the scripts
+  parentFiles = {
+    "supportedBranches.js" = ../supportedBranches.js;
+    "supportedSystems.json" = ../supportedSystems.json;
+  };
+in
+runCommand "typecheck-ci-scripts"
+  {
+    nativeBuildInputs = [ nodejs ];
+    passthru.driver = writeShellScriptBin "typecheck-ci-scripts" ''
+      nix-build --no-out-link "$@" \
+        ${toString ./..} -A typecheck-ci-scripts
+    '';
+  }
+  ''
+    # Set up directory structure matching the source layout
+    mkdir -p github-script
+    cp -r ${./.}/* github-script/
+    chmod -R u+w github-script
+    ln -s ${npmDeps}/node_modules github-script/node_modules
+
+    # Copy files from ci/ that are referenced by the scripts
+    ${builtins.concatStringsSep "\n" (
+      builtins.attrValues (builtins.mapAttrs (name: path: "cp ${path} ${name}") parentFiles)
+    )}
+
+    cd github-script
+    node node_modules/typescript/bin/tsc --project jsconfig.json
+    echo "typecheck-ci-scripts: ok"
+    touch $out
+  ''

ci/github-script/jsconfig.json

@@ -0,0 +1,16 @@
+{
+  "compilerOptions": {
+    "checkJs": true,
+    "allowJs": true,
+    "noEmit": true,
+    "target": "ES2024",
+    "lib": ["ESNext"],
+    "module": "commonjs",
+    "moduleResolution": "node",
+    "resolveJsonModule": true,
+    "strict": false,
+    "skipLibCheck": true
+  },
+  "include": ["*.js"],
+  "exclude": ["node_modules"]
+}

ci/github-script/merge.js

@@ -7,8 +7,8 @@ function runChecklist({
   pull_request,
   log,
   maintainers,
-  user,
-  userIsMaintainer,
+  user = null,
+  userIsMaintainer = false,
 }) {
   const allByName = files.every(
     ({ filename }) =>

ci/github-script/package.json

@@ -1,10 +1,17 @@
 {
   "private": true,
+  "scripts": {
+    "typecheck": "tsc --project jsconfig.json"
+  },
   "dependencies": {
     "@actions/artifact": "2.3.2",
     "@actions/core": "1.11.1",
     "@actions/github": "6.0.1",
     "bottleneck": "2.19.5",
     "commander": "14.0.0"
+  },
+  "devDependencies": {
+    "@types/node": "^22.0.0",
+    "typescript": "^5.7.0"
   }
 }

ci/github-script/reviews.js

@@ -126,7 +126,7 @@ async function dismissReviews({ github, context, core, dry, reviewKey }) {
  *  core: import('@actions/core'),
  *  dry: boolean,
  *  body: string,
- *  event: keyof eventToState,
+ *  event?: keyof eventToState,
  *  reviewKey: string,
  * }} PostReviewProps
  */

ci/github-script/withRateLimit.js

@@ -11,13 +11,15 @@ module.exports = async ({ github, core, maxConcurrent = 1 }, callback) => {
   // Rate-Limiting and Throttling, see for details:
   //   https://github.com/octokit/octokit.js/issues/1069#throttling
   //   https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api
+  // @ts-expect-error bottleneck lacks TypeScript types
   const allLimits = new Bottleneck({
     // Avoid concurrent requests
     maxConcurrent,
     // Will be updated with first `updateReservoir()` call below.
     reservoir: 0,
   })
   // Pause between mutative requests
+  // @ts-expect-error bottleneck lacks TypeScript types
   const writeLimits = new Bottleneck({ minTime: 1000 }).chain(allLimits)
   github.hook.wrap('request', async (request, options) => {
     // Requests to a different host do not count against the rate limit.

ci/supportedBranches.js

@@ -47,7 +47,7 @@ function classify(branch) {
 module.exports = { classify, split }
 
 // If called directly via CLI, runs the following tests:
-if (!module.parent) {
+if (require.main === module) {
   console.log('split(branch)')
   function testSplit(branch) {
     console.log(branch, split(branch))

shell.nix

@@ -16,7 +16,7 @@
   nixpkgs ? null,
 }:
 let
-  inherit (import ./ci { inherit nixpkgs system; }) pkgs fmt;
+  inherit (import ./ci { inherit nixpkgs system; }) pkgs fmt typecheck-ci-scripts;
 
   # For `nix-shell -A hello`
   curPkgs = removeAttrs (import ./. { inherit system; }) [
@@ -31,12 +31,14 @@ curPkgs
   inputsFrom = [
     fmt.shell
   ];
-  packages = with pkgs; [
+  packages = [
     # Helper to review Nixpkgs PRs
     # See CONTRIBUTING.md
-    nixpkgs-reviewFull
+    pkgs.nixpkgs-reviewFull
     # Command-line utility for working with GitHub
     # Used by nixpkgs-review to fetch eval results
-    gh
+    pkgs.gh
+    # Type check CI scripts
+    typecheck-ci-scripts.passthru.driver
   ];
 }