ci: gate every PR on tsc-cli regardless of touched paths by cjagwani · Pull Request #2461 · NVIDIA/NemoClaw

cjagwani · 2026-04-24T22:15:16Z

Summary

Prevents the class of regression that required #2458. Adds an unconditional typecheck:cli step to the shared CI action so future hook-filter drift can't hide a tsc error.

Root cause recap

.pre-commit-config.yaml had files: ^(bin|scripts)/ on the tsc-cli hook.
PRs that touched only test/ or src/ never triggered the check.
fix(onboard): validate Slack bot token format before saving #2130 (test-only Slack validation) slipped through that gap.
refactor(types): tighten repo-wide type boundaries #2422 later turned on strict: true, and the latent errors surfaced — but for every downstream PR, not the PR that introduced them.
Six open PRs ended up blocked on the same error cluster before anyone noticed.

Changes

.github/actions/basic-checks/action.yaml — new Typecheck CLI + tests (strict) step running npm run typecheck:cli unconditionally. Independent of prek hook configuration, so future filter drift can't hide a tsc error from CI.

Type of Change

Code change (feature, bug fix, or refactor)

Verification

YAML parses
npm run typecheck:cli exits 0 on this branch (requires fix(test): satisfy strict typecheck on Slack token validation tests #2458 merged to main first)
Tests added or updated for new or changed behavior (N/A — infra change, existing suite covers)
No secrets, API keys, or credentials committed

AI Disclosure

AI-assisted — tool: Claude Code

Signed-off-by: Charan Jagwani cjagwani@nvidia.com

Summary by CodeRabbit

Chores
- Enhanced continuous integration pipeline with additional TypeScript typechecking to improve code quality assurance and detect type-related issues earlier in the development process.

Prevents the class of regression that required #2458. The prek tsc-cli hook had files: ^(bin|scripts)/, so a PR touching only test/ or src/ skipped the check entirely. That let #2130 (test-only) land without typecheck, then #2422 tightened strict: true and the latent errors surfaced for every downstream PR — six open PRs were stuck on the same cluster by the time it was noticed. Adds an explicit Typecheck CLI + tests (strict) step in the shared basic-checks action that runs npm run typecheck:cli unconditionally. Independent of how anyone configures prek, so future hook-filter drift can't hide a tsc error from CI. Zero runtime impact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>

coderabbitai · 2026-04-24T22:15:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 79e02141-2a8e-4624-a861-8a4fe1e47a52

📥 Commits

Reviewing files that changed from the base of the PR and between f41f5ec and a974d9f.

📒 Files selected for processing (1)

.github/actions/basic-checks/action.yaml

📝 Walkthrough

Walkthrough

A new TypeScript typechecking step is added to the basic-checks composite GitHub Actions workflow. The step executes npm run typecheck:cli independently, positioned after the CLI build stage and before config validation.

Changes

Cohort / File(s)	Summary
CI TypeScript Typechecking `.github/actions/basic-checks/action.yaml`	Added a dedicated step to run strict CLI TypeScript typechecking via `npm run typecheck:cli`, inserted between CLI build and config validation steps to ensure independent type verification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A new check hops into the CI,
TypeScript strict, no errors we'll spy,
Between build and validate, it takes its place,
Typechecking the CLI at perfect pace! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding an unconditional TypeScript CLI typecheck gate in CI that runs regardless of file path filters.
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 unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ci/explicit-typecheck-gate-v2

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

ericksoa

Clean infra fix. The root cause analysis is spot-on — the prek hook's files: ^(bin|scripts)/ filter let test-only and src-only PRs skip tsc-cli, which is how the #2130 + #2422 strict-mode regression reached main undetected.

Adding npm run typecheck:cli as an unconditional step in the shared CI action is the right fix. It runs after build:cli (which it depends on for the compiled output), and before validate:configs — correct ordering. The comment explaining why it's here is helpful for future maintainers.

No concerns.

## Summary Prevents the class of regression that required NVIDIA#2458. Adds an unconditional `typecheck:cli` step to the shared CI action so future hook-filter drift can't hide a tsc error. ## Related Follow-up to NVIDIA#2458 (which fixed the symptom — this addresses the root cause). ## Root cause recap - `.pre-commit-config.yaml` had `files: ^(bin|scripts)/` on the `tsc-cli` hook. - PRs that touched only `test/` or `src/` never triggered the check. - NVIDIA#2130 (test-only Slack validation) slipped through that gap. - NVIDIA#2422 later turned on `strict: true`, and the latent errors surfaced — but for every downstream PR, not the PR that introduced them. - Six open PRs ended up blocked on the same error cluster before anyone noticed. ## Changes `.github/actions/basic-checks/action.yaml` — new **Typecheck CLI + tests (strict)** step running `npm run typecheck:cli` unconditionally. Independent of prek hook configuration, so future filter drift can't hide a tsc error from CI. ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Verification - [x] YAML parses - [x] `npm run typecheck:cli` exits 0 on this branch (requires NVIDIA#2458 merged to main first) - [x] Tests added or updated for new or changed behavior (N/A — infra change, existing suite covers) - [x] No secrets, API keys, or credentials committed ## AI Disclosure - [x] AI-assisted — tool: Claude Code --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>  ## Summary by CodeRabbit * **Chores** * Enhanced continuous integration pipeline with additional TypeScript typechecking to improve code quality assurance and detect type-related issues earlier in the development process.  Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cjagwani mentioned this pull request Apr 24, 2026

ci: gate every PR on tsc-cli regardless of touched paths #2460

Closed

7 tasks

ericksoa approved these changes Apr 24, 2026

View reviewed changes

ericksoa merged commit 6e95851 into main Apr 24, 2026
17 checks passed

wscurran added the chore Build, CI, dependency, or tooling maintenance label Jun 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: gate every PR on tsc-cli regardless of touched paths#2461

ci: gate every PR on tsc-cli regardless of touched paths#2461
ericksoa merged 1 commit into
mainfrom
ci/explicit-typecheck-gate-v2

cjagwani commented Apr 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

ericksoa left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cjagwani commented Apr 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related

Root cause recap

Changes

Type of Change

Verification

AI Disclosure

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

ericksoa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cjagwani commented Apr 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading