ci: unblock TypeScript migration release gates - #6969
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
added
area: e2e
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCoverage actions now support ChangesCI entrypoint compatibility
Bedrock E2E shard propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/actions/ci-cli-coverage-merge/action.yaml (1)
54-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame
.mts/.tsfallback snippet copy-pasted 4x across 3 action files.Each site resolves an entrypoint by checking for a
.mtsfile and falling back to.ts, using identical 4-line logic just against different script stems. This works correctly (verifiednpx tsxsupports both extensions natively), but the duplication means the eventual migration cleanup (removing the.tsfallback) has to touch 4 separate locations instead of one.
.github/actions/ci-cli-coverage-merge/action.yaml#L54-L58: extract this into a shared resolver (e.g. a small script invoked asdist_sourcemap_check=$(scripts/ci/resolve-entrypoint.sh scripts/check-dist-sourcemaps))..github/actions/ci-cli-coverage-merge/action.yaml#L102-L106: use the same shared resolver forscripts/check-coverage-ratchet..github/actions/ci-cli-coverage-shard/action.yaml#L105-L109: use the same shared resolver forscripts/check-dist-sourcemaps..github/actions/ci-plugin-coverage/action.yaml#L34-L38: use the same shared resolver forscripts/check-coverage-ratchet.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/ci-cli-coverage-merge/action.yaml around lines 54 - 58, Extract the repeated .mts/.ts entrypoint selection into a shared resolver script, such as scripts/ci/resolve-entrypoint.sh, accepting the script stem and outputting the existing .mts path or .ts fallback. Update all four sites—.github/actions/ci-cli-coverage-merge/action.yaml lines 54-58 for scripts/check-dist-sourcemaps, lines 102-106 for scripts/check-coverage-ratchet, .github/actions/ci-cli-coverage-shard/action.yaml lines 105-109 for scripts/check-dist-sourcemaps, and .github/actions/ci-plugin-coverage/action.yaml lines 34-38 for scripts/check-coverage-ratchet—to use the resolver before invoking npx tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/actions/ci-cli-coverage-merge/action.yaml:
- Around line 54-58: Extract the repeated .mts/.ts entrypoint selection into a
shared resolver script, such as scripts/ci/resolve-entrypoint.sh, accepting the
script stem and outputting the existing .mts path or .ts fallback. Update all
four sites—.github/actions/ci-cli-coverage-merge/action.yaml lines 54-58 for
scripts/check-dist-sourcemaps, lines 102-106 for scripts/check-coverage-ratchet,
.github/actions/ci-cli-coverage-shard/action.yaml lines 105-109 for
scripts/check-dist-sourcemaps, and
.github/actions/ci-plugin-coverage/action.yaml lines 34-38 for
scripts/check-coverage-ratchet—to use the resolver before invoking npx tsx.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b71debd4-aa5a-4dd9-addd-0b0d39087001
📒 Files selected for processing (8)
.github/actions/ci-cli-coverage-merge/action.yaml.github/actions/ci-cli-coverage-shard/action.yaml.github/actions/ci-plugin-coverage/action.yaml.github/workflows/e2e.yamltest/e2e/support/e2e-workflow.test.tstest/pr-e2e-gate-shards.test.tstest/pr-workflow-contract.test.tstools/e2e/workflow-boundary.mts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
|
Follow-up commit 43330e6 removes the live self-assertion and risk-selection test added in the prior commit. Protected E2E executes the workflow currently trusted on main, so that assertion could only observe the legacy default shard until this wiring lands; both Bedrock matrix jobs therefore stopped before runtime behavior was exercised. The checked-in workflow mapping remains covered by the E2E workflow contract tests. After this prerequisite merges, #6938 will run the OpenClaw and Hermes Bedrock variants with the new shard identity and provide the live evidence. |
This was referenced Jul 15, 2026
This was referenced Jul 16, 2026
20 tasks
cv
added a commit
that referenced
this pull request
Jul 28, 2026
<!-- markdownlint-disable MD041 --> ## Summary Trusted coverage actions now invoke only the migrated `.mts` entrypoints. Missing entrypoints fail at the canonical path instead of falling back to deleted `.ts` files. ## Related Issue Part of #6918 ## Changes - Remove four temporary coverage and sourcemap fallbacks added by #6969 for the migration window. - Remove the matching E2E mock-parity fallback after the repository check suite migration landed. - Update the behavioral parity fixture to reject the retired `.ts` entrypoint. - Keep the existing parity missing-entrypoint diagnostic, but require the `.mts` path. - Update the workflow contract to accept only `.mts` entrypoints and reject a missing canonical file. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: These internal CI entrypoint paths do not change a command, configuration, output, or supported workflow. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: [Refreshed security review](#7668 (comment)) passed on exact head `5d85df195`; the diff narrows trusted execution to fixed `.mts` paths and updates only regression tests. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Head `5d85df195` changes only internal CI actions and regression tests. Test titles and CI error text follow `WRITING.md`; no user-facing documentation is affected. - Agent: Codex Desktop <!-- docs-review-head-sha: 5d85df1 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification Signed-off-by: Carlos Villela <cvillela@nvidia.com> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/e2e-mock-parity.test.ts test/pr-workflow-contract.test.ts --project integration`: 2 files and 31 tests passed. Biome and normal commit hooks passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **CI and Validation** * Standardized coverage, sourcemap, and E2E mock parity checks on the migrated TypeScript entrypoints. * Removed compatibility handling for retired script variants, enabling faster failure when required checks are unavailable. * **Tests** * Updated workflow contract tests to verify the new entrypoints. * Added coverage for rejecting missing or retired validation scripts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unblocks the v0.0.84 TypeScript migration PRs whose required evidence depends on trusted workflow code from
main. Coverage actions now select the available TypeScript entrypoint, and the Bedrock matrix reports the agent-specific shard expected by the E2E controller.Changes
.mtsentrypoints and fall back to.tsfor older branches. This compatibility window is required because chore(scripts): migrate source and test analysis tools to .mts #6935 deletes the old entrypoints in the PR checkout while the action implementation still comes frommain; the action-contract test executes.mts-only,.ts-only, and missing-entrypoint fixtures across all four consumers. Remove the.tsfallback after chore(scripts): migrate source and test analysis tools to .mts #6935 lands and no open migration branch depends on the old entrypoints.NEMOCLAW_E2E_SHARDfrom the Bedrock job'smatrix.agentso refactor(inference): migrate Bedrock runtime adapter launcher to .mts #6938 produces distinct OpenClaw and Hermes evidence. Workflow-boundary and controller tests protect the mapping and reject drift.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run test/pr-workflow-contract.test.ts test/pr-e2e-gate-shards.test.ts --project integration(22 passed);npx vitest run test/e2e/support/e2e-workflow.test.ts --project e2e-support(36 passed); workflow validator,npm run source-shape:check, andnpm run checkspassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
CI & Build Improvements
.mtsvs.tsscript variant when present.Bug Fixes
Tests
.ts/.mtsentrypoint compatibility checks.