Skip to content

test(e2e): make docs validation a first-class Vitest lane - #6011

Merged
cv merged 4 commits into
mainfrom
codex/e2e-docs-validation-parity
Jun 29, 2026
Merged

test(e2e): make docs validation a first-class Vitest lane#6011
cv merged 4 commits into
mainfrom
codex/e2e-docs-validation-parity

Conversation

@cv

@cv cv commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Promote the frozen docs-validation behavior into a default-enabled Vitest E2E job before retiring the legacy Nightly workflow. The lane stays deterministic and local-only while preserving selective dispatch, artifacts, and PR result aggregation.

Related Issue

Contributes to #5919.

Changes

  • Add a dedicated docs-validation-vitest workflow job using the existing live Vitest test.
  • Keep remote link checks disabled so scheduled results do not depend on external site availability.
  • Include the job in free-standing selector discovery and PR result aggregation.
  • Add a focused workflow mutation ratchet for execution, determinism, action pinning, artifacts, and aggregation.

Type of Change

  • 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

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: internal E2E workflow coverage only; user-facing documentation and behavior are unchanged.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Targeted verification:

  • VITEST_MAX_WORKERS=16 npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts
  • npm run typecheck:cli
  • normal commit and push hooks

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Tests
    • Added an end-to-end docs validation workflow boundary check to verify required job behavior, determinism expectations, and step configuration.
    • Introduced workflow validation coverage for the live docs validation Vitest job, including artifact upload settings.
  • New Features
    • Added a dedicated live docs validation Vitest scenario that runs and publishes artifacts for inspection.
  • Bug Fixes
    • Updated PR scenario result reporting so the docs validation job’s pass/fail status is reflected in the PR results table.

cv added 2 commits June 28, 2026 01:00
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 47931ebf-e657-4839-988b-fad8ffd6d81f

📥 Commits

Reviewing files that changed from the base of the PR and between 5dcf5cd and 9369836.

📒 Files selected for processing (1)
  • .github/workflows/e2e-vitest-scenarios.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/e2e-vitest-scenarios.yaml

📝 Walkthrough

Walkthrough

Adds a docs-validation-vitest CI job, includes it in report-to-pr, adds a workflow boundary validator for that job, and covers the validator with Vitest tests.

Changes

Docs Validation CI Job and Boundary Validation

Layer / File(s) Summary
New docs-validation-vitest CI job
.github/workflows/e2e-vitest-scenarios.yaml
Adds the docs-validation-vitest job with checkout, Node setup, dependency install, test run, and artifact upload steps; adds it to report-to-pr's needs list.
Workflow boundary validator implementation
tools/e2e-scenarios/docs-validation-workflow-boundary.mts
Defines DocsValidationWorkflow types, readDocsValidationWorkflow, helper functions, and validateDocsValidationWorkflow, then exposes validateDocsValidationWorkflowBoundary as the entry point.
Boundary validator tests
test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts
Vitest suite covering default dispatch selection, mutated workflow validation errors, and empty-workflow boundary validation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

area: e2e, area: ci

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: promoting docs validation into a dedicated Vitest E2E lane.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/e2e-docs-validation-parity

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/e2e-docs-valid... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/e2e-docs-valid... 9369836 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the codex/e2e-docs-valid... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/e2e-docs-valid... 9369836 +/-
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 79%
src/lib/actions...dbox/rebuild.ts 74%
src/lib/state/sandbox.ts 72%
src/lib/shields/index.ts 70%
src/lib/onboard/preflight.ts 69%
src/lib/actions...licy-channel.ts 59%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/policy/index.ts 52%
src/lib/onboard.ts 20%

Updated June 29, 2026 19:58 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: docs-validation-vitest
Optional E2E: None

Dispatch hint: docs-validation-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • docs-validation-vitest (low): This PR adds and wires the docs-validation-vitest E2E job, so the newly configured job should run to verify checkout-local docs validation executes, remains deterministic/local-only, uploads artifacts, and is selectable by job/scenario inputs.

Optional E2E

  • None.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: .github/workflows/e2e-vitest-scenarios.yaml
  • jobs input: docs-validation-vitest

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: docs-validation-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=docs-validation-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • docs-validation-vitest: This PR wires a discrete free-standing live Vitest job, docs-validation-vitest, in e2e-vitest-scenarios.yaml for test/e2e-scenario/live/docs-validation.test.ts and adds workflow-boundary support tests/tools for that job. Per the free-standing job rule, dispatch the specific job rather than the full scenario fan-out.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=docs-validation-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts
  • tools/e2e-scenarios/docs-validation-workflow-boundary.mts

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-4: Unclear relationship between three doc-validation lanes; then add or justify PRA-T1.
Open items: 1 required · 5 warnings · 2 suggestions · 7 test follow-ups
Since last review: 1 prior item resolved · 1 still applies · 6 new items found

Action checklist

  • PRA-4 Fix: Unclear relationship between three doc-validation lanes in .github/workflows/e2e-vitest-scenarios.yaml:602
  • PRA-1 Resolve or justify: Source-of-truth review needed: docs-validation-vitest job vs nightly-e2e.yaml:docs-validation-e2e vs docs-cli-parity-pr.yaml
  • PRA-2 Resolve or justify: Validation module omits secret-exposure checks for job env in tools/e2e-scenarios/docs-validation-workflow-boundary.mts:60
  • PRA-3 Resolve or justify: Validation module omits secret-interpolation check in run scripts in tools/e2e-scenarios/docs-validation-workflow-boundary.mts:100
  • PRA-5 Resolve or justify: Missing free-standing inventory and selector condition tests in test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts:24
  • PRA-6 Resolve or justify: High merge conflict risk with 3 overlapping PRs in .github/workflows/e2e-vitest-scenarios.yaml:602
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Missing free-standing inventory and selector condition tests
  • PRA-T7 Add or justify test follow-up: docs-validation-vitest job vs nightly-e2e.yaml:docs-validation-e2e vs docs-cli-parity-pr.yaml
  • PRA-7 In-scope improvement: Validation module duplicates shared helpers from workflow-boundary.mts in tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1
  • PRA-8 In-scope improvement: DEFAULT_WORKFLOW_PATH parameter used only internally in tools/e2e-scenarios/docs-validation-workflow-boundary.mts:10

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify security tools/e2e-scenarios/docs-validation-workflow-boundary.mts:60 Add requireEnvDoesNotExposeSecret calls for the job env matching the pattern in validateNetworkPolicyVitestJob (lines 600-620 of workflow-boundary.mts).
PRA-3 Resolve/justify security tools/e2e-scenarios/docs-validation-workflow-boundary.mts:100 Add loop over job.steps checking stringValue(step.run) for \${\{\s*secrets\. pattern, pushing error 'step must not interpolate secrets directly'.
PRA-4 Required acceptance .github/workflows/e2e-vitest-scenarios.yaml:602 Update PR description or workflow comment with explicit matrix: which job runs when, which covers what, and defined exit criteria for retiring nightly-e2e job.
PRA-5 Resolve/justify tests test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts:24 Add test cases: (a) call validateFreeStandingWorkflowInventory() on current workflow, expect empty errors; (b) assert job.if === freeStandingJobIf('docs-validation-vitest', 'docs-validation').
PRA-6 Resolve/justify correctness .github/workflows/e2e-vitest-scenarios.yaml:602 Coordinate with maintainers on merge order. Consider adding job at end of free-standing jobs section (after last free-standing job before report-to-pr) to reduce conflict surface.
PRA-7 Improvement architecture tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1 Extract shared helpers to tools/e2e-scenarios/workflow-validation-helpers.mts and import in both modules. This is a refactoring that can be done in this PR since it's local to changed code.
PRA-8 Improvement architecture tools/e2e-scenarios/docs-validation-workflow-boundary.mts:10 Change to const DEFAULT_WORKFLOW_PATH at module scope and remove parameter from readDocsValidationWorkflow, validateDocsValidationWorkflow, validateDocsValidationWorkflowBoundary. Test can still use temp files by calling readDocsValidationWorkflow(tempPath) directly.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-4 Required — Unclear relationship between three doc-validation lanes

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:602
  • Category: acceptance
  • Problem: PR comment states new job 'preserves nightly-e2e.yaml:docs-validation-e2e as a discrete... contract until focused docs workflows are required and demonstrably subsume its full-repository boundary.' But docs-cli-parity-pr.yaml already runs CLI/docs parity on every PR. Acceptance criteria must clarify: (1) whether nightly-e2e job should be removed, (2) how this differs from docs-cli-parity-pr.yaml, (3) what 'focused docs workflows' means.
  • Impact: Three overlapping validation lanes (nightly, PR-scoped, Vitest) create confusion about which is authoritative and when each runs. Merge decision blocked until clarified.
  • Required action: Update PR description or workflow comment with explicit matrix: which job runs when, which covers what, and defined exit criteria for retiring nightly-e2e job.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read .github/workflows/nightly-e2e.yaml lines 319-365 and .github/workflows/docs-cli-parity-pr.yaml; compare scopes
  • Missing regression test: Document the intended relationship in a comment at top of docs-validation-vitest job or in ARCHITECTURE.md
  • Done when: The required change is committed and verification passes: Read .github/workflows/nightly-e2e.yaml lines 319-365 and .github/workflows/docs-cli-parity-pr.yaml; compare scopes.
  • Evidence: nightly-e2e.yaml has docs-validation-e2e job (lines 319-365); docs-cli-parity-pr.yaml runs on every PR touching CLI/docs paths; new job is default-enabled in Vitest suite
Review findings by urgency: 1 required fix, 5 items to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: docs-validation-vitest job vs nightly-e2e.yaml:docs-validation-e2e vs docs-cli-parity-pr.yaml

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Document intended lifecycle in workflow comment or ARCHITECTURE.md with explicit matrix of which job runs when and exit criteria
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: nightly-e2e.yaml lines 319-365, docs-cli-parity-pr.yaml lines 1-50, new job at e2e-vitest-scenarios.yaml lines 602-648

PRA-2 Resolve/justify — Validation module omits secret-exposure checks for job env

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:60
  • Category: security
  • Problem: validateDocsValidationWorkflow() does not call requireEnvDoesNotExposeSecret for job env (NVIDIA_INFERENCE_API_KEY, DOCKERHUB_USERNAME, DOCKERHUB_TOKEN, GITHUB_TOKEN, NVIDIA_API_KEY). Other free-standing job validators in workflow-boundary.mts (validateNetworkPolicyVitestJob, validateSkillAgentVitestJob) explicitly enforce this.
  • Impact: Future drift adding secrets to the job env would not be caught by the boundary validator, weakening the trusted-code boundary.
  • Recommended action: Add requireEnvDoesNotExposeSecret calls for the job env matching the pattern in validateNetworkPolicyVitestJob (lines 600-620 of workflow-boundary.mts).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -n requireEnvDoesNotExposeSecret tools/e2e-scenarios/workflow-boundary.mts | head -20 shows the pattern; compare with docs-validation-workflow-boundary.mts lines 60-150
  • Missing regression test: Add test case in docs-validation-workflow-boundary.test.ts that injects NVIDIA_INFERENCE_API_KEY into job.env and verifies validator returns error containing 'must not include NVIDIA_INFERENCE_API_KEY'
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -n requireEnvDoesNotExposeSecret tools/e2e-scenarios/workflow-boundary.mts | head -20 shows the pattern; compare with docs-validation-workflow-boundary.mts lines 60-150.
  • Evidence: validateNetworkPolicyVitestJob calls requireEnvDoesNotExposeSecret for 4 secrets on job env and each step env; docs-validation-workflow-boundary.mts has zero such calls

PRA-3 Resolve/justify — Validation module omits secret-interpolation check in run scripts

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:100
  • Category: security
  • Problem: No check that step run scripts don't interpolate secrets directly (pattern \${\{\s*secrets\.). Other validators loop all steps with this check via requireNoDispatchInputInterpolation + secrets pattern.
  • Impact: A future step adding `echo ${{ secrets.X }}` would not be flagged, risking secret leakage in logs.
  • Recommended action: Add loop over job.steps checking stringValue(step.run) for \${\{\s*secrets\. pattern, pushing error 'step must not interpolate secrets directly'.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search 'must not interpolate secrets directly' in workflow-boundary.mts to see exact pattern used in validateFreeStandingInventoryBoundary
  • Missing regression test: Add test case that adds a step with run: 'echo ${{ secrets.TEST }}' and verifies validator catches it
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search 'must not interpolate secrets directly' in workflow-boundary.mts to see exact pattern used in validateFreeStandingInventoryBoundary.
  • Evidence: validateFreeStandingInventoryBoundary in workflow-boundary.mts includes this check; docs-validation-workflow-boundary.mts does not

PRA-5 Resolve/justify — Missing free-standing inventory and selector condition tests

  • Location: test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts:24
  • Category: tests
  • Problem: Test 'makes execution, determinism, and aggregation part of the focused ratchet' mutates workflow but doesn't validate: (1) FREE_STANDING_VITEST_JOB/SCENARIO_ID via validateFreeStandingWorkflowInventory, (2) job.if matches freeStandingJobIf pattern from workflow-boundary.mts.
  • Impact: Drift in free-standing metadata or selector condition would not be caught by tests.
  • Recommended action: Add test cases: (a) call validateFreeStandingWorkflowInventory() on current workflow, expect empty errors; (b) assert job.if === freeStandingJobIf('docs-validation-vitest', 'docs-validation').
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run validateFreeStandingWorkflowInventory() from workflow-boundary.mts against current e2e-vitest-scenarios.yaml; check job.if at line 607
  • Missing regression test: Two new test cases in docs-validation-workflow-boundary.test.ts covering inventory validation and selector condition
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run validateFreeStandingWorkflowInventory() from workflow-boundary.mts against current e2e-vitest-scenarios.yaml; check job.if at line 607.
  • Evidence: Test file only tests dispatch selectors, mutation detection, and empty input; other workflow-boundary tests (network-policy-workflow-boundary.test.ts) include inventory validation

PRA-6 Resolve/justify — High merge conflict risk with 3 overlapping PRs

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:602
  • Category: correctness
  • Problem: PRs chore(openshell): upgrade supported version to 0.0.71 #5596 (openshell upgrade), chore(deps): bump actions/checkout from 6.0.3 to 7.0.0 #5755 (actions/checkout bump), feat(mcp): add OpenShell-managed MCP servers #5876 (MCP servers) all modify e2e-vitest-scenarios.yaml. This PR inserts new job at line 602 (between openclaw-skill-cli-vitest and hermes-root-entrypoint-smoke-vitest), maximizing conflict surface.
  • Impact: Merge conflicts will require manual resolution, delaying integration and increasing chance of errors.
  • Recommended action: Coordinate with maintainers on merge order. Consider adding job at end of free-standing jobs section (after last free-standing job before report-to-pr) to reduce conflict surface.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check line numbers in overlapping PR diffs vs this PR's insertion point; search for 'docs-validation-vitest' in other PRs
  • Missing regression test: Not applicable - merge coordination issue
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check line numbers in overlapping PR diffs vs this PR's insertion point; search for 'docs-validation-vitest' in other PRs.
  • Evidence: Drift context shows 3 open PRs with sameFiles including e2e-vitest-scenarios.yaml; this file has heavy recent modification history (19 commits in driftEvidence)

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-7 Improvement — Validation module duplicates shared helpers from workflow-boundary.mts

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1
  • Category: architecture
  • Problem: Reimplements requireFullShaAction, requireRunContains, requireEqual, findStep, requireEnvDoesNotExposeSecret that exist in workflow-boundary.mts. ~80 lines of duplicate logic increases maintenance burden and divergence risk.
  • Impact: Future changes to validation patterns (e.g., new action pinning format) must be updated in multiple places.
  • Suggested action: Extract shared helpers to tools/e2e-scenarios/workflow-validation-helpers.mts and import in both modules. This is a refactoring that can be done in this PR since it's local to changed code.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare docs-validation-workflow-boundary.mts lines 25-55 with workflow-boundary.mts lines 180-220 — nearly identical function signatures
  • Missing regression test: Not applicable - refactoring; existing tests should pass after extraction
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Both modules define requireEqual, requireRunContains, findStep, requireFullShaAction with identical logic

PRA-8 Improvement — DEFAULT_WORKFLOW_PATH parameter used only internally

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:10
  • Category: architecture
  • Problem: Function parameter with default value DEFAULT_WORKFLOW_PATH appears in 3 functions but is never overridden by external callers. Only internal test calls pass alternate paths via temp files.
  • Impact: Unnecessary parameter complexity; constant would be clearer.
  • Suggested action: Change to const DEFAULT_WORKFLOW_PATH at module scope and remove parameter from readDocsValidationWorkflow, validateDocsValidationWorkflow, validateDocsValidationWorkflowBoundary. Test can still use temp files by calling readDocsValidationWorkflow(tempPath) directly.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: grep -r 'docs-validation-workflow-boundary' --include='*.ts' --include='*.mts' | grep -v test shows no external callers overriding the path
  • Missing regression test: Not applicable - simplification
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Three functions have workflowPath = DEFAULT_WORKFLOW_PATH parameter; only test file passes custom path
Simplification opportunities: 2 possible cuts, net -70 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-7 stdlib (tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1): Duplicate helper functions (requireEqual, requireRunContains, findStep, requireFullShaAction, requireEnvDoesNotExposeSecret) from docs-validation-workflow-boundary.mts lines 25-55
    • Replacement: Import from new shared module tools/e2e-scenarios/workflow-validation-helpers.mts
    • Net: -60 lines
    • Safety boundary: Must not weaken secret-exposure or action-pinning checks; all existing tests must pass
  • PRA-8 shrink (tools/e2e-scenarios/docs-validation-workflow-boundary.mts:10): workflowPath = DEFAULT_WORKFLOW_PATH parameter from 3 function signatures
    • Replacement: Module-level const DEFAULT_WORKFLOW_PATH; functions use it directly
    • Net: -10 lines
    • Safety boundary: Test file's temp-file validation must still work via readDocsValidationWorkflow(tempPath)
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add secret-exposure test: inject NVIDIA_INFERENCE_API_KEY into job.env, expect validator error. New workflow job and validation module need behavioral runtime validation: the docs-validation-vitest lane should execute in CI to prove CLI build -> vitest run -> artifact upload works end-to-end. Static tests cover boundary validation but not runtime behavior.
  • PRA-T2 Runtime validation — Add secret-interpolation test: add step with ${{ secrets.X }} in run, expect validator error. New workflow job and validation module need behavioral runtime validation: the docs-validation-vitest lane should execute in CI to prove CLI build -> vitest run -> artifact upload works end-to-end. Static tests cover boundary validation but not runtime behavior.
  • PRA-T3 Runtime validation — Add free-standing inventory test: call validateFreeStandingWorkflowInventory() on current workflow, expect empty errors. New workflow job and validation module need behavioral runtime validation: the docs-validation-vitest lane should execute in CI to prove CLI build -> vitest run -> artifact upload works end-to-end. Static tests cover boundary validation but not runtime behavior.
  • PRA-T4 Runtime validation — Add selector condition test: assert job.if === freeStandingJobIf('docs-validation-vitest', 'docs-validation'). New workflow job and validation module need behavioral runtime validation: the docs-validation-vitest lane should execute in CI to prove CLI build -> vitest run -> artifact upload works end-to-end. Static tests cover boundary validation but not runtime behavior.
  • PRA-T5 Runtime validation — Runtime: execute docs-validation-vitest job in CI (VITEST_MAX_WORKERS=16 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/docs-validation.test.ts) and verify artifacts uploaded. New workflow job and validation module need behavioral runtime validation: the docs-validation-vitest lane should execute in CI to prove CLI build -> vitest run -> artifact upload works end-to-end. Static tests cover boundary validation but not runtime behavior.
  • PRA-T6 Missing free-standing inventory and selector condition tests — Add test cases: (a) call validateFreeStandingWorkflowInventory() on current workflow, expect empty errors; (b) assert job.if === freeStandingJobIf('docs-validation-vitest', 'docs-validation').
  • PRA-T7 docs-validation-vitest job vs nightly-e2e.yaml:docs-validation-e2e vs docs-cli-parity-pr.yaml — Document intended lifecycle in workflow comment or ARCHITECTURE.md with explicit matrix of which job runs when and exit criteria. nightly-e2e.yaml lines 319-365, docs-cli-parity-pr.yaml lines 1-50, new job at e2e-vitest-scenarios.yaml lines 602-648
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: docs-validation-vitest job vs nightly-e2e.yaml:docs-validation-e2e vs docs-cli-parity-pr.yaml

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Document intended lifecycle in workflow comment or ARCHITECTURE.md with explicit matrix of which job runs when and exit criteria
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: nightly-e2e.yaml lines 319-365, docs-cli-parity-pr.yaml lines 1-50, new job at e2e-vitest-scenarios.yaml lines 602-648

PRA-2 Resolve/justify — Validation module omits secret-exposure checks for job env

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:60
  • Category: security
  • Problem: validateDocsValidationWorkflow() does not call requireEnvDoesNotExposeSecret for job env (NVIDIA_INFERENCE_API_KEY, DOCKERHUB_USERNAME, DOCKERHUB_TOKEN, GITHUB_TOKEN, NVIDIA_API_KEY). Other free-standing job validators in workflow-boundary.mts (validateNetworkPolicyVitestJob, validateSkillAgentVitestJob) explicitly enforce this.
  • Impact: Future drift adding secrets to the job env would not be caught by the boundary validator, weakening the trusted-code boundary.
  • Recommended action: Add requireEnvDoesNotExposeSecret calls for the job env matching the pattern in validateNetworkPolicyVitestJob (lines 600-620 of workflow-boundary.mts).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: grep -n requireEnvDoesNotExposeSecret tools/e2e-scenarios/workflow-boundary.mts | head -20 shows the pattern; compare with docs-validation-workflow-boundary.mts lines 60-150
  • Missing regression test: Add test case in docs-validation-workflow-boundary.test.ts that injects NVIDIA_INFERENCE_API_KEY into job.env and verifies validator returns error containing 'must not include NVIDIA_INFERENCE_API_KEY'
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -n requireEnvDoesNotExposeSecret tools/e2e-scenarios/workflow-boundary.mts | head -20 shows the pattern; compare with docs-validation-workflow-boundary.mts lines 60-150.
  • Evidence: validateNetworkPolicyVitestJob calls requireEnvDoesNotExposeSecret for 4 secrets on job env and each step env; docs-validation-workflow-boundary.mts has zero such calls

PRA-3 Resolve/justify — Validation module omits secret-interpolation check in run scripts

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:100
  • Category: security
  • Problem: No check that step run scripts don't interpolate secrets directly (pattern \${\{\s*secrets\.). Other validators loop all steps with this check via requireNoDispatchInputInterpolation + secrets pattern.
  • Impact: A future step adding `echo ${{ secrets.X }}` would not be flagged, risking secret leakage in logs.
  • Recommended action: Add loop over job.steps checking stringValue(step.run) for \${\{\s*secrets\. pattern, pushing error 'step must not interpolate secrets directly'.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search 'must not interpolate secrets directly' in workflow-boundary.mts to see exact pattern used in validateFreeStandingInventoryBoundary
  • Missing regression test: Add test case that adds a step with run: 'echo ${{ secrets.TEST }}' and verifies validator catches it
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search 'must not interpolate secrets directly' in workflow-boundary.mts to see exact pattern used in validateFreeStandingInventoryBoundary.
  • Evidence: validateFreeStandingInventoryBoundary in workflow-boundary.mts includes this check; docs-validation-workflow-boundary.mts does not

PRA-4 Required — Unclear relationship between three doc-validation lanes

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:602
  • Category: acceptance
  • Problem: PR comment states new job 'preserves nightly-e2e.yaml:docs-validation-e2e as a discrete... contract until focused docs workflows are required and demonstrably subsume its full-repository boundary.' But docs-cli-parity-pr.yaml already runs CLI/docs parity on every PR. Acceptance criteria must clarify: (1) whether nightly-e2e job should be removed, (2) how this differs from docs-cli-parity-pr.yaml, (3) what 'focused docs workflows' means.
  • Impact: Three overlapping validation lanes (nightly, PR-scoped, Vitest) create confusion about which is authoritative and when each runs. Merge decision blocked until clarified.
  • Required action: Update PR description or workflow comment with explicit matrix: which job runs when, which covers what, and defined exit criteria for retiring nightly-e2e job.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Read .github/workflows/nightly-e2e.yaml lines 319-365 and .github/workflows/docs-cli-parity-pr.yaml; compare scopes
  • Missing regression test: Document the intended relationship in a comment at top of docs-validation-vitest job or in ARCHITECTURE.md
  • Done when: The required change is committed and verification passes: Read .github/workflows/nightly-e2e.yaml lines 319-365 and .github/workflows/docs-cli-parity-pr.yaml; compare scopes.
  • Evidence: nightly-e2e.yaml has docs-validation-e2e job (lines 319-365); docs-cli-parity-pr.yaml runs on every PR touching CLI/docs paths; new job is default-enabled in Vitest suite

PRA-5 Resolve/justify — Missing free-standing inventory and selector condition tests

  • Location: test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts:24
  • Category: tests
  • Problem: Test 'makes execution, determinism, and aggregation part of the focused ratchet' mutates workflow but doesn't validate: (1) FREE_STANDING_VITEST_JOB/SCENARIO_ID via validateFreeStandingWorkflowInventory, (2) job.if matches freeStandingJobIf pattern from workflow-boundary.mts.
  • Impact: Drift in free-standing metadata or selector condition would not be caught by tests.
  • Recommended action: Add test cases: (a) call validateFreeStandingWorkflowInventory() on current workflow, expect empty errors; (b) assert job.if === freeStandingJobIf('docs-validation-vitest', 'docs-validation').
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run validateFreeStandingWorkflowInventory() from workflow-boundary.mts against current e2e-vitest-scenarios.yaml; check job.if at line 607
  • Missing regression test: Two new test cases in docs-validation-workflow-boundary.test.ts covering inventory validation and selector condition
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run validateFreeStandingWorkflowInventory() from workflow-boundary.mts against current e2e-vitest-scenarios.yaml; check job.if at line 607.
  • Evidence: Test file only tests dispatch selectors, mutation detection, and empty input; other workflow-boundary tests (network-policy-workflow-boundary.test.ts) include inventory validation

PRA-6 Resolve/justify — High merge conflict risk with 3 overlapping PRs

  • Location: .github/workflows/e2e-vitest-scenarios.yaml:602
  • Category: correctness
  • Problem: PRs chore(openshell): upgrade supported version to 0.0.71 #5596 (openshell upgrade), chore(deps): bump actions/checkout from 6.0.3 to 7.0.0 #5755 (actions/checkout bump), feat(mcp): add OpenShell-managed MCP servers #5876 (MCP servers) all modify e2e-vitest-scenarios.yaml. This PR inserts new job at line 602 (between openclaw-skill-cli-vitest and hermes-root-entrypoint-smoke-vitest), maximizing conflict surface.
  • Impact: Merge conflicts will require manual resolution, delaying integration and increasing chance of errors.
  • Recommended action: Coordinate with maintainers on merge order. Consider adding job at end of free-standing jobs section (after last free-standing job before report-to-pr) to reduce conflict surface.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check line numbers in overlapping PR diffs vs this PR's insertion point; search for 'docs-validation-vitest' in other PRs
  • Missing regression test: Not applicable - merge coordination issue
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check line numbers in overlapping PR diffs vs this PR's insertion point; search for 'docs-validation-vitest' in other PRs.
  • Evidence: Drift context shows 3 open PRs with sameFiles including e2e-vitest-scenarios.yaml; this file has heavy recent modification history (19 commits in driftEvidence)

PRA-7 Improvement — Validation module duplicates shared helpers from workflow-boundary.mts

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1
  • Category: architecture
  • Problem: Reimplements requireFullShaAction, requireRunContains, requireEqual, findStep, requireEnvDoesNotExposeSecret that exist in workflow-boundary.mts. ~80 lines of duplicate logic increases maintenance burden and divergence risk.
  • Impact: Future changes to validation patterns (e.g., new action pinning format) must be updated in multiple places.
  • Suggested action: Extract shared helpers to tools/e2e-scenarios/workflow-validation-helpers.mts and import in both modules. This is a refactoring that can be done in this PR since it's local to changed code.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Compare docs-validation-workflow-boundary.mts lines 25-55 with workflow-boundary.mts lines 180-220 — nearly identical function signatures
  • Missing regression test: Not applicable - refactoring; existing tests should pass after extraction
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Both modules define requireEqual, requireRunContains, findStep, requireFullShaAction with identical logic

PRA-8 Improvement — DEFAULT_WORKFLOW_PATH parameter used only internally

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:10
  • Category: architecture
  • Problem: Function parameter with default value DEFAULT_WORKFLOW_PATH appears in 3 functions but is never overridden by external callers. Only internal test calls pass alternate paths via temp files.
  • Impact: Unnecessary parameter complexity; constant would be clearer.
  • Suggested action: Change to const DEFAULT_WORKFLOW_PATH at module scope and remove parameter from readDocsValidationWorkflow, validateDocsValidationWorkflow, validateDocsValidationWorkflowBoundary. Test can still use temp files by calling readDocsValidationWorkflow(tempPath) directly.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: grep -r 'docs-validation-workflow-boundary' --include='*.ts' --include='*.mts' | grep -v test shows no external callers overriding the path
  • Missing regression test: Not applicable - simplification
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Three functions have workflowPath = DEFAULT_WORKFLOW_PATH parameter; only test file passes custom path

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 1 suggestion · 4 test follow-ups
Since last review: 0 prior items resolved · 1 still applies · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-1 In-scope improvement: Consider folding the docs workflow ratchet into central boundary coverage in tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1

Findings index

ID Severity Category Location Required action
PRA-1 Improvement architecture tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1 If practical in this PR, shrink the new helper by relying on `validateE2eVitestScenariosWorkflowBoundary()` for generic free-standing workflow invariants and keep only the docs-specific ratchets either in the central validator or as a small test-local helper.
Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

  • None.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-1 Improvement — Consider folding the docs workflow ratchet into central boundary coverage

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1
  • Category: architecture
  • Problem: The new docs-specific validator is only used by the new support test and repeats generic workflow-boundary checks that the central e2e Vitest workflow validator already applies to free-standing jobs, including action SHA pinning, selector/report coverage, secret exposure, dispatch interpolation, checkout credential persistence, and install-script boundaries. The docs-specific checks for local-only link validation, the exact docs Vitest target, artifact identity, and default-enabled selector behavior are useful, but the separate production helper creates a second workflow-boundary system for one lane.
  • Impact: Maintainers may need to update two validators when free-standing workflow conventions change, increasing drift risk for trusted-code-boundary rules in a high-churn workflow file.
  • Suggested action: If practical in this PR, shrink the new helper by relying on `validateE2eVitestScenariosWorkflowBoundary()` for generic free-standing workflow invariants and keep only the docs-specific ratchets either in the central validator or as a small test-local helper.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read `tools/e2e-scenarios/workflow-boundary.mts` functions `validateFreeStandingInventoryBoundary()`, `validateFreeStandingInventoryCoverage()`, and `validateE2eVitestScenariosWorkflowBoundary()`, then compare them with the checkout/action pinning, `npm ci --ignore-scripts`, artifact upload, and `report-to-pr` checks in `tools/e2e-scenarios/docs-validation-workflow-boundary.mts`.
  • Missing regression test: Existing coverage in `test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts` already proves selector behavior, local-only link mode, exact docs test target, report aggregation, and empty-workflow tolerant parsing; preserve those assertions if the helper is folded or inlined.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: The new support test imports both `validateDocsValidationWorkflowBoundary()` from the single-purpose module and `validateE2eVitestScenariosWorkflowBoundary()` from the central module. The central validator derives free-standing inventory from `FREE_STANDING_VITEST_JOB` and `FREE_STANDING_SCENARIO_ID` and validates generic workflow/security invariants for those jobs.
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-1 shrink (tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1): The duplicated generic workflow-boundary checks in the new single-job validator, or the separate production helper module if the docs-specific assertions can live in the central validator or test file.
    • Replacement: Use the central workflow-boundary validator for generic invariants and retain only docs-specific assertions for `CHECK_DOC_LINKS_REMOTE=0`, `test/e2e-scenario/live/docs-validation.test.ts`, docs artifact identity, and default/selective dispatch.
    • Safety boundary: Do not remove checks that protect the workflow trusted-code boundary: SHA-pinned actions, `persist-credentials: false`, `npm ci --ignore-scripts`, no secret exposure, no dispatch-input interpolation, local-only docs link validation, and `report-to-pr` aggregation.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Validate workflow dispatch with `jobs=docs-validation-vitest`, confirming it selects only `docs-validation-vitest` and does not run the matrix `live-scenarios` job.. Static coverage is strong for workflow shape, selector mapping, local-only link mode, and mutation ratchets, but the changed surface is GitHub Actions workflow wiring plus a live docs E2E lane. Targeted runtime validation would increase confidence that dispatch behavior and shell command execution match the YAML and static parser.
  • PRA-T2 Runtime validation — Validate workflow dispatch with `scenarios=docs-validation`, confirming it maps to `docs-validation-vitest` with `liveScenariosRuns: false`.. Static coverage is strong for workflow shape, selector mapping, local-only link mode, and mutation ratchets, but the changed surface is GitHub Actions workflow wiring plus a live docs E2E lane. Targeted runtime validation would increase confidence that dispatch behavior and shell command execution match the YAML and static parser.
  • PRA-T3 Runtime validation — Run the docs-validation live Vitest scenario in a controlled runtime to confirm `check-docs.sh --only-cli` and `check-docs.sh --only-links --local-only` execute with remote link probing disabled.. Static coverage is strong for workflow shape, selector mapping, local-only link mode, and mutation ratchets, but the changed surface is GitHub Actions workflow wiring plus a live docs E2E lane. Targeted runtime validation would increase confidence that dispatch behavior and shell command execution match the YAML and static parser.
  • PRA-T4 Runtime validation — Run or identify the support boundary test that mutates `CHECK_DOC_LINKS_REMOTE`, the docs Vitest target, and `report-to-pr.needs`, confirming the ratchet fails on those workflow drifts.. Static coverage is strong for workflow shape, selector mapping, local-only link mode, and mutation ratchets, but the changed surface is GitHub Actions workflow wiring plus a live docs E2E lane. Targeted runtime validation would increase confidence that dispatch behavior and shell command execution match the YAML and static parser.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Improvement — Consider folding the docs workflow ratchet into central boundary coverage

  • Location: tools/e2e-scenarios/docs-validation-workflow-boundary.mts:1
  • Category: architecture
  • Problem: The new docs-specific validator is only used by the new support test and repeats generic workflow-boundary checks that the central e2e Vitest workflow validator already applies to free-standing jobs, including action SHA pinning, selector/report coverage, secret exposure, dispatch interpolation, checkout credential persistence, and install-script boundaries. The docs-specific checks for local-only link validation, the exact docs Vitest target, artifact identity, and default-enabled selector behavior are useful, but the separate production helper creates a second workflow-boundary system for one lane.
  • Impact: Maintainers may need to update two validators when free-standing workflow conventions change, increasing drift risk for trusted-code-boundary rules in a high-churn workflow file.
  • Suggested action: If practical in this PR, shrink the new helper by relying on `validateE2eVitestScenariosWorkflowBoundary()` for generic free-standing workflow invariants and keep only the docs-specific ratchets either in the central validator or as a small test-local helper.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read `tools/e2e-scenarios/workflow-boundary.mts` functions `validateFreeStandingInventoryBoundary()`, `validateFreeStandingInventoryCoverage()`, and `validateE2eVitestScenariosWorkflowBoundary()`, then compare them with the checkout/action pinning, `npm ci --ignore-scripts`, artifact upload, and `report-to-pr` checks in `tools/e2e-scenarios/docs-validation-workflow-boundary.mts`.
  • Missing regression test: Existing coverage in `test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts` already proves selector behavior, local-only link mode, exact docs test target, report aggregation, and empty-workflow tolerant parsing; preserve those assertions if the helper is folded or inlined.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: The new support test imports both `validateDocsValidationWorkflowBoundary()` from the single-purpose module and `validateE2eVitestScenariosWorkflowBoundary()` from the central module. The central validator derives free-standing inventory from `FREE_STANDING_VITEST_JOB` and `FREE_STANDING_SCENARIO_ID` and validates generic workflow/security invariants for those jobs.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
tools/e2e-scenarios/docs-validation-workflow-boundary.mts (1)

60-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid hard-coding the shared selector gate here.

Pinning the entire if: expression in this per-job validator creates another source of truth for ordinary free-standing job selection. That is likely to drift the next time the shared selector logic changes, even if the workflow remains valid. Based on learnings, "enforce selector/gate invariants for ordinary free-standing jobs centrally—not per job" and "validateFreeStandingInventoryBoundary delegates to validateFreeStandingJobSelector".

🤖 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 `@tools/e2e-scenarios/docs-validation-workflow-boundary.mts` around lines 60 -
65, The validator is hard-coding the shared selector gate in the per-job check,
which duplicates the source of truth and will drift from the central selector
logic. Update validateFreeStandingInventoryBoundary to avoid asserting the full
if expression for this job, and rely on validateFreeStandingJobSelector for the
shared gate while keeping only the job-specific boundary invariant for
JOB_NAME/docs-validation-vitest.

Source: Learnings

🤖 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 `@tools/e2e-scenarios/docs-validation-workflow-boundary.mts`:
- Around line 34-37: Normalize the parsed result in readDocsValidationWorkflow
before it is used as a DocsValidationWorkflow. YAML.parse() may return null or a
non-object, so add a shape check/defaulting step and only return a validated
workflow object; if the file is empty or malformed, surface a controlled
validation error instead of letting later workflow.jobs access throw. Use the
readDocsValidationWorkflow symbol to locate the parsing and conversion logic.

---

Nitpick comments:
In `@tools/e2e-scenarios/docs-validation-workflow-boundary.mts`:
- Around line 60-65: The validator is hard-coding the shared selector gate in
the per-job check, which duplicates the source of truth and will drift from the
central selector logic. Update validateFreeStandingInventoryBoundary to avoid
asserting the full if expression for this job, and rely on
validateFreeStandingJobSelector for the shared gate while keeping only the
job-specific boundary invariant for JOB_NAME/docs-validation-vitest.
🪄 Autofix (Beta)

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: ec35f120-33ca-4ede-a296-a7b54a8fb84c

📥 Commits

Reviewing files that changed from the base of the PR and between c87ff23 and fffed97.

📒 Files selected for processing (3)
  • .github/workflows/e2e-vitest-scenarios.yaml
  • test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts
  • tools/e2e-scenarios/docs-validation-workflow-boundary.mts

Comment thread tools/e2e-scenarios/docs-validation-workflow-boundary.mts Outdated
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Results — ✅ All requested jobs passed

Run: 28397028669
Workflow ref: codex/e2e-docs-validation-parity
Requested scenarios: (default — all supported)
Requested jobs: docs-validation-vitest
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
docs-validation-vitest ✅ success

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts (1)

66-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the empty-workflow aggregation error too.

This case only checks one boundary error, but an empty workflow also deterministically violates the report-to-pr dependency contract. As written, the test would still pass if that aggregation check regressed. Add the second observable error, or assert the full returned set.

Suggested tightening
-      expect(validateDocsValidationWorkflowBoundary(workflowPath)).toContain(
-        "docs-validation-vitest must depend on generate-matrix",
-      );
+      expect(validateDocsValidationWorkflowBoundary(workflowPath)).toEqual(
+        expect.arrayContaining([
+          "docs-validation-vitest must depend on generate-matrix",
+          "report-to-pr must wait for docs-validation-vitest",
+        ]),
+      );

As per path instructions, **/*.test.{ts,js,mts,mjs,cts,cjs}: "Review tests for behavioral confidence rather than implementation lock-in."

🤖 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/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts`
around lines 66 - 73, The empty-workflow test currently only asserts the
`docs-validation-vitest must depend on generate-matrix` boundary error, so it
can miss a regression in the `report-to-pr` aggregation check. Update
`validateDocsValidationWorkflowBoundary` coverage in
`docs-validation-workflow-boundary.test.ts` to assert the second deterministic
error as well, or verify the full returned error set for the empty workflow so
both contract violations are covered.

Source: Path instructions

🤖 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 `@test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts`:
- Around line 66-73: The empty-workflow test currently only asserts the
`docs-validation-vitest must depend on generate-matrix` boundary error, so it
can miss a regression in the `report-to-pr` aggregation check. Update
`validateDocsValidationWorkflowBoundary` coverage in
`docs-validation-workflow-boundary.test.ts` to assert the second deterministic
error as well, or verify the full returned error set for the empty workflow so
both contract violations are covered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3eb28ccb-f02a-48f1-bd9a-64ff87a3e5dc

📥 Commits

Reviewing files that changed from the base of the PR and between fffed97 and 5dcf5cd.

📒 Files selected for processing (2)
  • test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts
  • tools/e2e-scenarios/docs-validation-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/e2e-scenarios/docs-validation-workflow-boundary.mts

@cv

cv commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Focused runtime verification is green: docs-validation-vitest run 28397028669 completed successfully on head 5dcf5cdd4.

Advisor dispositions:

@cv
cv merged commit 8296fb4 into main Jun 29, 2026
45 checks passed
@cv
cv deleted the codex/e2e-docs-validation-parity branch June 29, 2026 20:02
@miyoungc miyoungc mentioned this pull request Jun 30, 2026
21 tasks
jyaunches pushed a commit that referenced this pull request Jun 30, 2026
## Summary
Refreshes the v0.0.70 release docs from the release announcement and the
`v0.0.69..v0.0.70` commit range.
It also documents the `channels start` policy restoration behavior that
was missing from the shared OpenClaw and Hermes command references, and
bumps the Fern CLI version used for docs validation.

## Changes
- Replaced the stale `v0.0.70` release-notes entry with the actual
release themes, including CLI, onboarding, inference, messaging,
Windows, documentation, and release-validation changes.
- Documented that `channels start` reapplies the matching built-in
network policy preset before rebuild and rolls back to disabled if
policy restoration fails.
- Bumped `fern/fern.config.json` from `5.55.0` to `5.59.0` for the docs
refresh.
- Source summary:
- #5754 -> `docs/about/release-notes.mdx`: Notes Docker Desktop gateway
bridge retry behavior during onboarding.
- #5930 -> `docs/about/release-notes.mdx`: Links `nemoclaw use` default
sandbox selection to the command reference.
- #5948 -> `docs/about/release-notes.mdx`: Links reasoning-compatible
endpoint validation to inference documentation.
- #5950 -> `docs/about/release-notes.mdx`: Links Windows bootstrap WSL
recovery behavior to Windows preparation and troubleshooting docs.
- #5856 -> `docs/about/release-notes.mdx`: Notes rebuilt policy preset
registry repair.
- #5882 and #5949 -> `docs/about/release-notes.mdx`: Notes Hermes stale
base-image state repair.
- #6016 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`, and
`docs/manage-sandboxes/messaging-channels.mdx`: Documents channel policy
restoration and rollback on `channels start`.
- #5859 -> `docs/about/release-notes.mdx`: Links quickstart network
approval guidance.
- #5863 -> `docs/about/release-notes.mdx`: Links Teams allowlist
guidance in the messaging page.
- #5756, #5926, #6010, and #6011 -> `docs/about/release-notes.mdx`:
Summarizes the Vitest E2E validation cutover.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only prose refresh with
no runtime behavior change.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [x] 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)

`npm run docs` exited 0 and Fern reported one existing light-mode accent
contrast warning.
`fern check --warnings` confirmed the warning is the site theme contrast
ratio, not content introduced by this PR.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Promote the frozen docs-validation behavior into a default-enabled
Vitest E2E job before retiring the legacy Nightly workflow. The lane
stays deterministic and local-only while preserving selective dispatch,
artifacts, and PR result aggregation.

## Related Issue

Contributes to NVIDIA#5919.

## Changes

- Add a dedicated `docs-validation-vitest` workflow job using the
existing live Vitest test.
- Keep remote link checks disabled so scheduled results do not depend on
external site availability.
- Include the job in free-standing selector discovery and PR result
aggregation.
- Add a focused workflow mutation ratchet for execution, determinism,
action pinning, artifacts, and aggregation.

## 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: internal E2E workflow
coverage only; user-facing documentation and behavior are unchanged.
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)

Targeted verification:

- `VITEST_MAX_WORKERS=16 npx vitest run --project e2e-vitest-support
test/e2e-scenario/support-tests/docs-validation-workflow-boundary.test.ts`
- `npm run typecheck:cli`
- normal commit and push hooks

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Added an end-to-end docs validation workflow boundary check to verify
required job behavior, determinism expectations, and step configuration.
* Introduced workflow validation coverage for the live docs validation
Vitest job, including artifact upload settings.
* **New Features**
* Added a dedicated live docs validation Vitest scenario that runs and
publishes artifacts for inspection.
* **Bug Fixes**
* Updated PR scenario result reporting so the docs validation job’s
pass/fail status is reflected in the PR results table.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Refreshes the v0.0.70 release docs from the release announcement and the
`v0.0.69..v0.0.70` commit range.
It also documents the `channels start` policy restoration behavior that
was missing from the shared OpenClaw and Hermes command references, and
bumps the Fern CLI version used for docs validation.

## Changes
- Replaced the stale `v0.0.70` release-notes entry with the actual
release themes, including CLI, onboarding, inference, messaging,
Windows, documentation, and release-validation changes.
- Documented that `channels start` reapplies the matching built-in
network policy preset before rebuild and rolls back to disabled if
policy restoration fails.
- Bumped `fern/fern.config.json` from `5.55.0` to `5.59.0` for the docs
refresh.
- Source summary:
- NVIDIA#5754 -> `docs/about/release-notes.mdx`: Notes Docker Desktop gateway
bridge retry behavior during onboarding.
- NVIDIA#5930 -> `docs/about/release-notes.mdx`: Links `nemoclaw use` default
sandbox selection to the command reference.
- NVIDIA#5948 -> `docs/about/release-notes.mdx`: Links reasoning-compatible
endpoint validation to inference documentation.
- NVIDIA#5950 -> `docs/about/release-notes.mdx`: Links Windows bootstrap WSL
recovery behavior to Windows preparation and troubleshooting docs.
- NVIDIA#5856 -> `docs/about/release-notes.mdx`: Notes rebuilt policy preset
registry repair.
- NVIDIA#5882 and NVIDIA#5949 -> `docs/about/release-notes.mdx`: Notes Hermes stale
base-image state repair.
- NVIDIA#6016 -> `docs/reference/commands.mdx`,
`docs/reference/commands-nemohermes.mdx`, and
`docs/manage-sandboxes/messaging-channels.mdx`: Documents channel policy
restoration and rollback on `channels start`.
- NVIDIA#5859 -> `docs/about/release-notes.mdx`: Links quickstart network
approval guidance.
- NVIDIA#5863 -> `docs/about/release-notes.mdx`: Links Teams allowlist
guidance in the messaging page.
- NVIDIA#5756, NVIDIA#5926, NVIDIA#6010, and NVIDIA#6011 -> `docs/about/release-notes.mdx`:
Summarizes the Vitest E2E validation cutover.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: doc-only prose refresh with
no runtime behavior change.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)
- [x] 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)

`npm run docs` exited 0 and Fern reported one existing light-mode accent
contrast warning.
`fern check --warnings` confirmed the warning is the site theme contrast
ratio, not content introduced by this PR.

---
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: docs Documentation, examples, guides, or docs build area: e2e End-to-end tests, nightly failures, or validation infrastructure labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: docs Documentation, examples, guides, or docs build area: e2e End-to-end tests, nightly failures, or validation infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants