ci(workflows): consolidate workflow infrastructure - #8632
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR centralizes base-image build and publication logic, shares CI dependency installation, adds a WhatsApp E2E job, centralizes E2E workflow contracts, tightens workflow controls, and removes obsolete regression workflows and contract tests. ChangesCI workflow consolidation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BaseImageWorkflow
participant BuildBaseImagePlatform
participant PublishBaseImageManifest
BaseImageWorkflow->>BuildBaseImagePlatform: build and validate platform image
BuildBaseImagePlatform-->>BaseImageWorkflow: upload digest artifact
BaseImageWorkflow->>PublishBaseImageManifest: provide platform digest artifacts
PublishBaseImageManifest-->>BaseImageWorkflow: publish validated manifest and contract
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/regression-e2e.yaml:
- Around line 52-59: Update the empty-input pattern in the case statement around
the normalized jobs subject to use ,, instead of ,. Keep the whatsapp-qr-compact
match and output behavior unchanged for non-empty selections.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e17dec71-130c-4a08-9dab-cb364e6af5fe
📒 Files selected for processing (21)
.github/workflows/regression-e2e.yamlagents/openclaw/dependency-review.mdci/source-shape-test-budget.jsontest/candidate-compat.test.tstest/ci-cli-coverage-pi-tools-workflow.test.tstest/cloudflared-update-check-workflow.test.tstest/code-scanning-workflow.test.tstest/e2e-main-retry-workflow.test.tstest/e2e-release-gate-workflow.test.tstest/e2e/support/e2e-workflow.test.tstest/e2e/support/sandbox-name-workflow-boundary.test.tstest/macos-e2e-workflow-boundary.test.tstest/openclaw-dependency-review.test.tstest/openclaw-locked-install.test.tstest/openshell-e2e-qualification-workflow.test.tstest/pr-limit-policy.test.tstest/pr-workflow-contract.test.tstest/regression-e2e-workflow.test.tstest/release-lkg-brev-image.test.tstest/reviewed-npm-audit-workflow.test.tstest/wechat-runtime-audit-workflow.test.ts
💤 Files with no reviewable changes (18)
- test/macos-e2e-workflow-boundary.test.ts
- test/wechat-runtime-audit-workflow.test.ts
- test/regression-e2e-workflow.test.ts
- test/openclaw-locked-install.test.ts
- test/e2e-release-gate-workflow.test.ts
- test/openshell-e2e-qualification-workflow.test.ts
- test/openclaw-dependency-review.test.ts
- test/reviewed-npm-audit-workflow.test.ts
- test/release-lkg-brev-image.test.ts
- test/ci-cli-coverage-pi-tools-workflow.test.ts
- test/e2e/support/e2e-workflow.test.ts
- test/e2e-main-retry-workflow.test.ts
- test/candidate-compat.test.ts
- test/pr-limit-policy.test.ts
- test/cloudflared-update-check-workflow.test.ts
- test/code-scanning-workflow.test.ts
- test/pr-workflow-contract.test.ts
- ci/source-shape-test-budget.json
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
cv
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/actions/build-base-image-platform/action.yaml (1)
63-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate the OpenClaw version before you use it.
Lines 73-78 build
openclaw_build_argand callscripts/check-production-build-args.shwith the raw input. Lines 82-91 reject CR, LF, and non-dotted-decimal values only after that call. Move the format checks above the first use so the action fails fast and never forwards a malformed version.♻️ Proposed reordering
set -euo pipefail build_args=() openclaw_build_arg="" if [ "$AGENT" = "openclaw" ] && [ -n "${OPENCLAW_VERSION_INPUT}" ]; then + if [[ "$OPENCLAW_VERSION_INPUT" == *$'\r'* || "$OPENCLAW_VERSION_INPUT" == *$'\n'* ]]; then + echo "ERROR: OpenClaw version must not contain CR or LF characters." >&2 + exit 1 + fi + if [[ ! "$OPENCLAW_VERSION_INPUT" =~ ^[0-9]+([.][0-9]+)*$ ]]; then + echo "ERROR: OpenClaw version must be a whole decimal dotted version (for example, 2026.6.10)." >&2 + exit 1 + fi openclaw_build_arg="OPENCLAW_VERSION=${OPENCLAW_VERSION_INPUT}" build_args+=(--build-arg "$openclaw_build_arg") fi if [ "${`#build_args`[@]}" -gt 0 ]; then scripts/check-production-build-args.sh "${build_args[@]}" else scripts/check-production-build-args.sh fi - if [ "$AGENT" = "openclaw" ] && [ -n "${OPENCLAW_VERSION_INPUT}" ]; then - if [[ "$OPENCLAW_VERSION_INPUT" == *$'\r'* || "$OPENCLAW_VERSION_INPUT" == *$'\n'* ]]; then - echo "ERROR: OpenClaw version must not contain CR or LF characters." >&2 - exit 1 - fi - if [[ ! "$OPENCLAW_VERSION_INPUT" =~ ^[0-9]+([.][0-9]+)*$ ]]; then - echo "ERROR: OpenClaw version must be a whole decimal dotted version (for example, 2026.6.10)." >&2 - exit 1 - fi - fi printf 'openclaw_build_arg=%s\n' "$openclaw_build_arg" >> "$GITHUB_OUTPUT"🤖 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/build-base-image-platform/action.yaml around lines 63 - 92, Move the OpenClaw version validation checks in the production-build-args step before constructing openclaw_build_arg or invoking scripts/check-production-build-args.sh. Keep the existing CR/LF and dotted-decimal validation rules, then only forward the validated OPENCLAW_VERSION_INPUT to the build-argument check and GITHUB_OUTPUT..github/actions/publish-base-image-manifest/action.yaml (1)
79-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the loop-scoped
digestvariable.Line 86 assigns the bare platform digest to
digest. Line 127 reassigns the same name to the manifest digest with thesha256:prefix. The two values have different formats. Use a distinct name, such asplatform_digest, in the loop to prevent an accidental cross-use in later edits.🤖 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/publish-base-image-manifest/action.yaml around lines 79 - 103, Rename the loop-scoped bare digest variable in the digest_files loop from digest to platform_digest, and update the source construction and source_digests assignment to use it. Leave the later manifest digest variable unchanged.test/managed-image-publication-workflow.test.ts (1)
402-413: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winProve that the inline manifest path is gone.
toMatchObjectconfirms the publisher job wires./.github/actions/publish-base-image-manifestwith the expected inputs. It does not confirm that the superseded inline steps were removed. A leftoverdocker buildx imagetools createstep, or a second upload of the managed-base contract in the same job, would still pass this assertion. Add a negative assertion over the job steps.♻️ Proposed additional assertion
expect(manifest).toMatchObject({ uses: "./.github/actions/publish-base-image-manifest", with: { agent: expectedPublisher.agent, "display-name": expectedPublisher.displayName, image: expectedPublisher.image, registry: "${{ env.REGISTRY }}", "registry-username": "${{ github.actor }}", "registry-password": "${{ secrets.GITHUB_TOKEN }}", }, }); + for (const inlineStep of basePublisher.steps) { + expect(inlineStep.run ?? "", expectedPublisher.job).not.toContain("imagetools create"); + expect(inlineStep.uses ?? "", expectedPublisher.job).not.toContain("upload-artifact"); + }As per path instructions: "Migration tests must prove the superseded path is unreachable or removed, not merely prove that the new path also works."
🤖 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 `@test/managed-image-publication-workflow.test.ts` around lines 402 - 413, Extend the manifest assertions in the test around step(basePublisher, "Publish validated multi-platform manifest") with a negative check over basePublisher.steps, verifying no inline docker buildx imagetools create step or duplicate managed-base upload remains; retain the existing positive action-input assertions.Source: Path instructions
test/dcode-base-image-workflow.test.ts (1)
10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this file and share the
baseDockerfileslist.The file no longer asserts anything about the Deep Agents Code workflow. It now checks base Dockerfile dependency contracts for all three images, so the name
dcode-base-image-workflow.test.tsmisleads readers who search for workflow coverage. Rename it, for example tobase-image-dependency-contract.test.ts.
test/bundled-npm-ip-address-dockerfile-contract.test.ts(lines 17-27) already declares an identicalrepoRootandbaseDockerfilesarray. Export one shared list from a test helper and import it in both files so a new base image is added in one place.The ast-grep path-traversal hint on line 28 is a false positive.
dockerfilecomes from a local literal array.Also applies to: 23-28
🤖 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 `@test/dcode-base-image-workflow.test.ts` around lines 10 - 14, Rename the test file to reflect base-image dependency contract coverage rather than Deep Agents Code workflow coverage. Extract the duplicated repoRoot and baseDockerfiles definitions into a shared test helper, export the list, and update both this test and bundled-npm-ip-address-dockerfile-contract.test.ts to import and use it, preserving the existing Dockerfile entries and local path handling.Source: Linters/SAST tools
🤖 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.
Inline comments:
In @.github/workflows/base-image.yaml:
- Around line 196-207: Update the “Build and publish platform digest” step to
remove the openclaw-version input and add the metadata-tags configuration used
by the OpenClaw build rules, preserving the generated image labels such as
org.opencontainers.image.version for Hermes and Deep Agents.
In `@test/ci-install-dependencies.test.ts`:
- Line 31: Update the environment construction in the dependency-install test to
use an empty-string fallback when process.env.PATH is unset, so PATH remains
valid as the bin-prefixed value. Preserve the existing NPM_TRACE and bin path
behavior.
---
Nitpick comments:
In @.github/actions/build-base-image-platform/action.yaml:
- Around line 63-92: Move the OpenClaw version validation checks in the
production-build-args step before constructing openclaw_build_arg or invoking
scripts/check-production-build-args.sh. Keep the existing CR/LF and
dotted-decimal validation rules, then only forward the validated
OPENCLAW_VERSION_INPUT to the build-argument check and GITHUB_OUTPUT.
In @.github/actions/publish-base-image-manifest/action.yaml:
- Around line 79-103: Rename the loop-scoped bare digest variable in the
digest_files loop from digest to platform_digest, and update the source
construction and source_digests assignment to use it. Leave the later manifest
digest variable unchanged.
In `@test/dcode-base-image-workflow.test.ts`:
- Around line 10-14: Rename the test file to reflect base-image dependency
contract coverage rather than Deep Agents Code workflow coverage. Extract the
duplicated repoRoot and baseDockerfiles definitions into a shared test helper,
export the list, and update both this test and
bundled-npm-ip-address-dockerfile-contract.test.ts to import and use it,
preserving the existing Dockerfile entries and local path handling.
In `@test/managed-image-publication-workflow.test.ts`:
- Around line 402-413: Extend the manifest assertions in the test around
step(basePublisher, "Publish validated multi-platform manifest") with a negative
check over basePublisher.steps, verifying no inline docker buildx imagetools
create step or duplicate managed-base upload remains; retain the existing
positive action-input assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 36525f9f-ffec-420f-84d0-0bebc9239118
📒 Files selected for processing (42)
.github/actions/build-base-image-platform/action.yaml.github/actions/ci-build-typecheck/action.yaml.github/actions/ci-cli-coverage-shard/action.yaml.github/actions/ci-install-dependencies.sh.github/actions/ci-installer-integration/action.yaml.github/actions/ci-plugin-coverage/action.yaml.github/actions/ci-reviewed-npm-audit/action.yaml.github/actions/ci-wechat-runtime-audit/action.yaml.github/actions/publish-base-image-manifest/action.yaml.github/scripts/select-regression-e2e-jobs.sh.github/workflows/base-image.yaml.github/workflows/code-scanning.yaml.github/workflows/commit-lint.yaml.github/workflows/docker-pin-check.yaml.github/workflows/docs-cli-parity-pr.yaml.github/workflows/docs-links-pr.yaml.github/workflows/macos-e2e.yaml.github/workflows/managed-images.yaml.github/workflows/pr-merge-conflict-fixer.yaml.github/workflows/pr-review-advisor.yaml.github/workflows/pr-self-hosted.yaml.github/workflows/pr.yaml.github/workflows/regression-e2e.yaml.github/workflows/release-latest-tag.yaml.github/workflows/sandbox-images-and-e2e.yaml.github/workflows/wsl-e2e.yamlagents/openclaw/dependency-review.mdci/source-shape-test-budget.jsontest/ci-install-dependencies.test.tstest/dcode-base-image-workflow.test.tstest/managed-image-publication-workflow.test.tstest/pr-merge-conflict-fixer-workflow-boundary.test.tstest/regression-e2e-selector.test.tstest/sandbox-base-image-layout.test.tstest/wechat-runtime-audit-workflow.test.tstools/e2e/base-image-publication.mtstools/e2e/cli-artifact-workflow-boundary.mtstools/e2e/prepare-e2e-workflow-boundary.mtstools/e2e/sandbox-images-workflow-boundary.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mtstools/e2e/workflow-boundary.mtstools/e2e/workflow-contract-registry.mts
💤 Files with no reviewable changes (3)
- test/sandbox-base-image-layout.test.ts
- test/wechat-runtime-audit-workflow.test.ts
- ci/source-shape-test-budget.json
🚧 Files skipped from review as they are similar to previous changes (2)
- agents/openclaw/dependency-review.md
- .github/workflows/regression-e2e.yaml
23 tasks
This was referenced Aug 9, 2026
apurvvkumaria
added a commit
that referenced
this pull request
Aug 9, 2026
<!-- markdownlint-disable MD041 --> ## Summary Automatic main-push generic GPU E2E runs currently activate risk-signal reporting with `github.sha` even though no correlation UUID exists, so Vitest exits before the live test starts. Keep reporting inactive when `checkout_sha` is absent, bind source attestation to a separate candidate SHA, and preserve the exact SHA and lowercase UUIDv4 contract for manual PR qualification. ## Changes - Bind the generic GPU reporter identity only to `inputs.checkout_sha`, leaving automatic main pushes inactive. - Bind the generic GPU live test and its qualification evidence to a separate exact candidate SHA on both trusted-main and PR self-hosted paths. - Keep manual PR expected-SHA and correlation inputs exact and fail closed for missing or invalid active identities. - Refresh the immutable CLI artifact workflow digest and add focused workflow and reporter contract coverage. ## Failure Evidence - Trusted-main run [31287291324, job 93178468730](https://github.com/NVIDIA/NemoClaw/actions/runs/31287291324/job/93178468730) restored the exact `c3bbad78306030ea650073372c0d9f8f82974a15` CLI artifact and OpenShell v0.0.99, then failed at Vitest startup with a nonempty expected SHA, empty correlation ID, and `risk signal requires a lowercase UUIDv4 correlation id`. - The same signature recurred in jobs [93145932176](https://github.com/NVIDIA/NemoClaw/actions/runs/31273410722/job/93145932176), [93152683289](https://github.com/NVIDIA/NemoClaw/actions/runs/31274918401/job/93152683289), and [93169163210](https://github.com/NVIDIA/NemoClaw/actions/runs/31279100533/job/93169163210). - The repair is based on `290eb94884eb29bc8aac89685e18525bb1292ed3` after inspecting merged PR #8632. PR #8632 did not change or supersede this risk-signal contract. ## 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: This repairs internal trusted CI identity wiring. It does not change a public CLI, configuration, API, policy schema, documented default, or supported runtime behavior. - [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: Independent Codex Desktop nine-category review of `e9b4613ef` passed with no findings. - [ ] 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: Reviewed all seven changed files. The change only affects internal workflow risk-signal activation, candidate identity, validators, and regression coverage; public documentation does not expose this lane or environment contract. - Agent: Codex Desktop <!-- docs-review-head-sha: e9b4613 --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `48` E2E operations workflow tests, `40` CLI artifact workflow tests, `20` risk-signal reporter tests, and all `441` path-selected tests passed; `npm run validate:pr` passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run locally; fresh PR automation will provide the repository-wide gates. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved validation of generic GPU end-to-end qualification commits. * Added fail-closed handling for missing or invalid correlation identifiers. * Corrected workflow identity and risk-signal environment validation. * **Tests** * Expanded coverage for disabled configurations, candidate commit identity, and workflow boundaries. * Updated qualification evidence and completion metadata checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
23 tasks
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
1 participant
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
Consolidate repeated workflow infrastructure and remove direct YAML source-shape assertions. Preserve security boundaries while making checkout credentials, artifact policies, timeouts, regression selection, dependency installation, base-image publication, and E2E contract metadata explicit.
Changes
Type of Change
Quality Gates
agents/openclaw/dependency-review.mdwas corrected to match retained test coverage; no public documentation changes are required.Documentation Writer Review
docs-updatedDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm run typecheck:clipassed; merge commit hooks passed, including repository checks and E2E semantic phase plans;npm run docspassed with zero errors and two existing warnings.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — GitHub CI will run the broad gates; no completed local broad gate is claimed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
Security
Reliability
Documentation