test(e2e): harden progress coverage boundaries#7397
Merged
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
added
area: ci
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds secret-safe E2E progress reporting, workflow-driven semantic phase validation, supervised subprocess execution, bounded command output, and fail-closed OpenShell gateway auth artifact approval before upload. ChangesE2E progress and subprocess contract
Workflow artifact safety
Semantic phase coverage
Additional E2E updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit bd75ae8 in the TypeScript / code-coverage/cliThe overall coverage in commit bd75ae8 in the Show a code coverage summary of the most impacted files.
Updated |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / medium confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/e2e/check-semantic-phases.mts (1)
2260-2334: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid depending on Vitest internals here —
createVitest(..., { project: string[] }),collect(), and theTestModulefields used below (relativeModuleId,children.allTests(),errors(),meta().e2ePhases) are internal rather than documented Vitest APIs. Keep this behind a compatibility shim or pin Vitest tightly, or this check may break on a routine upgrade.🤖 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/check-semantic-phases.mts` around lines 2260 - 2334, The checkSemanticPhaseCoverage implementation relies directly on undocumented Vitest APIs, including createVitest project configuration, collect(), and TestModule internals. Encapsulate these interactions in a compatibility shim with a stable interface for collected modules, tests, errors, and module identifiers, or otherwise pin and enforce the supported Vitest version; update checkSemanticPhaseCoverage to use that abstraction.
🧹 Nitpick comments (1)
test/e2e/support/e2e-fixture-context.test.ts (1)
481-481: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the failure progress instead of only setting it.
progress.stop("failed")changes the reporter’s state but does not verify that the missing-command path produced the expected failure diagnostic. Since the prior activity assertions were removed, assertprogress.summary()or the recorded event stream after stopping.As per path instructions, tests should verify observable behavioral outcomes rather than implementation-only state changes.
🤖 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/support/e2e-fixture-context.test.ts` at line 481, Update the missing-command test around progress.stop("failed") to assert the observable failure diagnostic after stopping, using progress.summary() or the recorded event stream. Keep the stop call, but verify that the expected failure outcome was produced rather than only changing reporter state.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.
Inline comments:
In `@test/compatible-endpoint-context-probe.test.ts`:
- Around line 24-27: Give each progress reporter a fixture-owned, per-test
lifecycle: in test/compatible-endpoint-context-probe.test.ts lines 24-27, use
the fixture-provided progress, or create and stop a fresh reporter per test
after closing the fake server; in test/e2e/support/e2e-fixture-context.test.ts
lines 23-28, ensure every supportProgress() instance is stopped in a finally
block or through a scoped helper that owns finalization. Audited subprocesses
must receive the canonical progress capability and each test must complete its
declared progress lifecycle.
In `@test/e2e/fixtures/fake-openai-compatible.ts`:
- Around line 223-229: Update the readiness failure handling around waitForReady
so cleanup via close does not replace the original error when close itself
fails. Preserve and rethrow the readiness error, while retaining the cleanup
failure as secondary context using the project’s established error-cause or
aggregation pattern if available.
In `@test/e2e/live/bedrock-runtime-compatible-anthropic-raw-command.ts`:
- Around line 79-121: Bound stdout and stderr accumulation in runRawCommand
using the existing bounded capture pattern from docker-probe.ts. Update the
child stdout and stderr data handlers to retain only the configured maximum
output while preserving enough information to report truncation, preventing
noisy processes from growing buffers without limit.
In `@test/e2e/support/e2e-progress-fixture.test.ts`:
- Around line 34-50: Update the spawnSync options in the e2e progress fixture
test to retain the positive timeout and explicitly set killSignal to "SIGKILL",
matching the existing runner-pressure configuration. Apply only this subprocess
timeout handling change.
In `@test/e2e/support/e2e-progress-outcome.test.ts`:
- Around line 22-38: Update the spawnSync invocation in the e2e progress outcome
test to set killSignal to "SIGKILL" alongside the existing timeout option,
ensuring timed-out child processes are forcefully terminated. Apply only this
change to the spawnSync options.
In `@test/e2e/support/workflow-e2e-progress.test.ts`:
- Around line 23-40: Add killSignal: "SIGKILL" to the options object of the
spawnSync call launching VITEST in the workflow progress test, preserving the
existing timeout and environment configuration.
---
Outside diff comments:
In `@tools/e2e/check-semantic-phases.mts`:
- Around line 2260-2334: The checkSemanticPhaseCoverage implementation relies
directly on undocumented Vitest APIs, including createVitest project
configuration, collect(), and TestModule internals. Encapsulate these
interactions in a compatibility shim with a stable interface for collected
modules, tests, errors, and module identifiers, or otherwise pin and enforce the
supported Vitest version; update checkSemanticPhaseCoverage to use that
abstraction.
---
Nitpick comments:
In `@test/e2e/support/e2e-fixture-context.test.ts`:
- Line 481: Update the missing-command test around progress.stop("failed") to
assert the observable failure diagnostic after stopping, using
progress.summary() or the recorded event stream. Keep the stop call, but verify
that the expected failure outcome was produced rather than only changing
reporter state.
🪄 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: 04f140b8-2f7b-49bc-808f-ab9f4eb6e23d
📒 Files selected for processing (86)
.github/workflows/candidate-compatibility.yaml.github/workflows/e2e.yaml.pre-commit-config.yamlCONTRIBUTING.mdtest/candidate-compat.test.tstest/compatible-endpoint-context-probe.test.tstest/e2e/README.mdtest/e2e/docs/README.mdtest/e2e/fixtures/cleanup.tstest/e2e/fixtures/docker-probe.tstest/e2e/fixtures/e2e-test.tstest/e2e/fixtures/fake-openai-compatible.tstest/e2e/fixtures/inference-adapter.tstest/e2e/fixtures/observed-child-process.tstest/e2e/fixtures/progress.tstest/e2e/fixtures/shell-probe.tstest/e2e/fixtures/shell/supervisor.tstest/e2e/fixtures/workflow-e2e-test.tstest/e2e/live/agent-turn-latency-helpers.tstest/e2e/live/agent-turn-latency.test.tstest/e2e/live/bedrock-runtime-compatible-anthropic-raw-command.tstest/e2e/live/bedrock-runtime-compatible-anthropic.test.tstest/e2e/live/channels-add-remove.test.tstest/e2e/live/concurrent-gateway-ports.test.tstest/e2e/live/device-auth-health.test.tstest/e2e/live/diagnostics.test.tstest/e2e/live/double-onboard.test.tstest/e2e/live/hermes-gpu-startup.test.tstest/e2e/live/hermes-inference-switch.test.tstest/e2e/live/hermes-root-entrypoint-smoke.test.tstest/e2e/live/hermes-sandbox-secret-boundary.test.tstest/e2e/live/hermes-shields-config.test.tstest/e2e/live/inference-routing-helpers.tstest/e2e/live/inference-routing-provider-smoke.test.tstest/e2e/live/inference-routing.test.tstest/e2e/live/issue-4434-tui-unreachable-inference.test.tstest/e2e/live/mcp-bridge-servers.tstest/e2e/live/mcp-bridge.test.tstest/e2e/live/messaging-compatible-endpoint-helpers.tstest/e2e/live/ollama-auth-proxy.test.tstest/e2e/live/onboard-repair.test.tstest/e2e/live/onboard-resume.test.tstest/e2e/live/openclaw-inference-switch.test.tstest/e2e/live/openclaw-plugin-runtime-exdev.test.tstest/e2e/live/openshell-credential-generation-window.test.tstest/e2e/live/openshell-gateway-auth-source-contract-helpers.tstest/e2e/live/openshell-gateway-upgrade.test.tstest/e2e/live/openshell-version-pin.test.tstest/e2e/live/rebuild-hermes-phases.tstest/e2e/live/rebuild-hermes.test.tstest/e2e/live/runtime-overrides.test.tstest/e2e/live/snapshot-commands.test.tstest/e2e/live/token-rotation.test.tstest/e2e/mock-parity.jsontest/e2e/risk-signal-reporter.tstest/e2e/support/agent-turn-latency-progress.test.tstest/e2e/support/bedrock-runtime-compatible-anthropic-progress.test.tstest/e2e/support/device-auth-health-helpers.test.tstest/e2e/support/e2e-fixture-context.test.tstest/e2e/support/e2e-progress-fixture.test.tstest/e2e/support/e2e-progress-outcome.test.tstest/e2e/support/e2e-redaction-entry.test.tstest/e2e/support/e2e-resource-phase-label.test.tstest/e2e/support/e2e-semantic-phase-check.test.tstest/e2e/support/fixtures/e2e-progress-outcome.fixture.test.tstest/e2e/support/fixtures/e2e-progress.fixture.test.tstest/e2e/support/fixtures/semantic-phase-child-process.fixture.tstest/e2e/support/fixtures/workflow-e2e-progress.fixture.test.tstest/e2e/support/hosted-inference.test.tstest/e2e/support/inference-adapter.test.tstest/e2e/support/observed-child-process.test.tstest/e2e/support/openshell-gateway-auth-contract-workflow-boundary.test.tstest/e2e/support/openshell-gateway-auth-source-contract-helpers.test.tstest/e2e/support/rebuild-hermes-progress.test.tstest/e2e/support/upload-e2e-artifacts-workflow-boundary.test.tstest/e2e/support/workflow-e2e-progress.test.tstest/gateway-drift-preflight.test.tstest/historical-openclaw-security-revision-container-e2e.test.tstest/mcp-bridge-servers.test.tstest/openclaw-security-revision-container-e2e.test.tstest/vllm-docker-storage.test.tstools/e2e/check-semantic-phases.mtstools/e2e/openshell-gateway-auth-artifact-safety.mtstools/e2e/openshell-gateway-auth-contract-workflow-boundary.mtstools/e2e/runner-pressure.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
senthilr-nv
reviewed
Jul 22, 2026
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@test/compatible-endpoint-context-probe.test.ts`:
- Around line 20-33: Update testProgress() to use the canonical progress logger
instead of suppressing output with logLine: () => undefined, and provide a
stable targetId identifying the compatible endpoint support scenario. Preserve
the existing progress phases while ensuring emitted diagnostics include
target/scenario identity and elapsed-time information.
- Around line 27-30: Advance the progress recorder through both declared phases
in each test using the progress instance returned by startTestProgress: call
progress.phase("serve compatible endpoint") before the serving step and
progress.phase("verify compatible endpoint") before verification, ensuring the
terminal phase is active before afterEach cleanup.
In `@test/e2e/support/bedrock-runtime-compatible-anthropic-progress.test.ts`:
- Around line 123-147: Update the test case around progressProbe and
runRawCommand so the command execution and all assertions are wrapped in a
try/finally block. Move progress.stop() into the finally block, ensuring it runs
whether execution or assertions succeed or throw.
🪄 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: 1673de5c-6e53-451a-82c0-c6edf7f51a9d
📒 Files selected for processing (9)
test/compatible-endpoint-context-probe.test.tstest/e2e/fixtures/fake-openai-compatible.tstest/e2e/live/bedrock-runtime-compatible-anthropic-raw-command.tstest/e2e/support/bedrock-runtime-compatible-anthropic-progress.test.tstest/e2e/support/e2e-fixture-context.test.tstest/e2e/support/e2e-progress-fixture.test.tstest/e2e/support/e2e-progress-outcome.test.tstest/e2e/support/hosted-inference.test.tstest/e2e/support/workflow-e2e-progress.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- test/e2e/support/hosted-inference.test.ts
- test/e2e/support/workflow-e2e-progress.test.ts
- test/e2e/live/bedrock-runtime-compatible-anthropic-raw-command.ts
- test/e2e/support/e2e-progress-outcome.test.ts
- test/e2e/support/e2e-progress-fixture.test.ts
- test/e2e/fixtures/fake-openai-compatible.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
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 `@tools/e2e/openshell-gateway-auth-artifact-safety.mts`:
- Around line 139-166: Update copyRegularFile so the source descriptor is always
closed when opening the approved descriptor fails. Restructure the descriptor
acquisition and cleanup around the existing try/finally, ensuring approved is
closed only after successful acquisition while source is closed on every path,
including O_CREAT|O_EXCL open errors.
🪄 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: e5f3f30c-e2df-435f-a923-15a36a864364
📒 Files selected for processing (11)
.github/workflows/candidate-compatibility.yaml.github/workflows/e2e.yamltest/candidate-compat.test.tstest/compatible-endpoint-context-probe.test.tstest/e2e/docs/README.mdtest/e2e/support/bedrock-runtime-compatible-anthropic-progress.test.tstest/e2e/support/openshell-gateway-auth-contract-workflow-boundary.test.tstest/e2e/support/openshell-gateway-auth-source-contract-helpers.test.tstools/e2e/openshell-gateway-auth-artifact-safety.mtstools/e2e/openshell-gateway-auth-contract-workflow-boundary.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/candidate-compatibility.yaml
- tools/e2e/openshell-gateway-auth-contract-workflow-boundary.mts
- test/e2e/docs/README.md
- test/candidate-compat.test.ts
- test/compatible-endpoint-context-probe.test.ts
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/e2e/openshell-gateway-auth-contract-workflow-boundary.mts (1)
167-171: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate the scan command exactly, not by substring.
requireRunContainsaccepts a command such as the required scan followed by|| true, allowing a failed safety scan to produce a successful step outcome and potentially satisfy the upload gate. Require the exact command/arguments or reject shell operators that can mask its exit status, and add a regression test for this bypass.🤖 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/openshell-gateway-auth-contract-workflow-boundary.mts` around lines 167 - 171, Update the validation around requireRunContains in the artifact-safety workflow to require the exact scan command and arguments, rather than accepting commands with appended shell operators such as || true that can mask failures. Preserve the existing expected command and add a regression test covering the bypass attempt.
🤖 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.
Outside diff comments:
In `@tools/e2e/openshell-gateway-auth-contract-workflow-boundary.mts`:
- Around line 167-171: Update the validation around requireRunContains in the
artifact-safety workflow to require the exact scan command and arguments, rather
than accepting commands with appended shell operators such as || true that can
mask failures. Preserve the existing expected command and add a regression test
covering the bypass attempt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 97efe396-76e3-4339-8f46-31a911d4886a
📒 Files selected for processing (6)
test/e2e/live/bedrock-runtime-compatible-anthropic-raw-command.tstest/e2e/support/bedrock-runtime-compatible-anthropic-progress.test.tstest/e2e/support/e2e-progress-outcome.test.tstest/e2e/support/openshell-gateway-auth-source-contract-helpers.test.tstools/e2e/openshell-gateway-auth-artifact-safety.mtstools/e2e/openshell-gateway-auth-contract-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (3)
- test/e2e/support/e2e-progress-outcome.test.ts
- test/e2e/live/bedrock-runtime-compatible-anthropic-raw-command.ts
- tools/e2e/openshell-gateway-auth-artifact-safety.mts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
Action performedReview triggered.
|
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
✅ Action performedReview finished.
|
23 tasks
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/support/docker-probe.test.ts (1)
122-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the fake-child/progress setup into a shared helper.
This test builds a fake
ChildProcess(132-138) and a stubbedstartTestProgress(139-148) inline; if similar constructs already exist elsewhere in this file for other DockerProbe real-branch tests, factoring them into a local helper would reduce duplication and shrink this single 80-line test.🤖 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/support/docker-probe.test.ts` around lines 122 - 203, Extract the repeated fake ChildProcess and startTestProgress setup from the output-limit test into a local shared helper, reusing existing equivalent setup in nearby DockerProbe real-branch tests where applicable. Update this test and other matching tests to call the helper while preserving their process, stream, progress, and cleanup behavior.
🤖 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/support/docker-probe.test.ts`:
- Around line 122-203: Extract the repeated fake ChildProcess and
startTestProgress setup from the output-limit test into a local shared helper,
reusing existing equivalent setup in nearby DockerProbe real-branch tests where
applicable. Update this test and other matching tests to call the helper while
preserving their process, stream, progress, and cleanup behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3ed81676-f0f1-43ab-b7bf-b7ba326e902e
📒 Files selected for processing (1)
test/e2e/support/docker-probe.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e.yaml (1)
5156-5175: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPass
needsas data viaenv:, not templated into the script body.
const needs = ${{ toJSON(needs) }};splices job-output JSON directly into the JS source, unlike the sibling values (JOBS,EXPLICIT_ONLY_JOBS,RUNTIME_ARTIFACTS,TARGETS) in the same step which are correctly passed throughenv:and read viaprocess.env. This is flagged by zizmor as a template-injection vector: content originating from upstream job outputs could break out of the intended JS literal.🔒️ Proposed fix
env: EXPLICIT_ONLY_JOBS: ${{ needs.generate-matrix.outputs.explicit_only_jobs }} JOBS: ${{ inputs.jobs }} RUNTIME_ARTIFACTS: ${{ runner.temp }}/e2e-runtime-audit TARGETS: ${{ inputs.targets }} + NEEDS: ${{ toJSON(needs) }} with: script: | ... - const needs = ${{ toJSON(needs) }}; + const needs = JSON.parse(process.env.NEEDS);As per path instructions, "Keep permissions least-privileged and pass untrusted values as data rather than interpolating them into shell programs."
🤖 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/workflows/e2e.yaml around lines 5156 - 5175, Pass the needs context through the step’s env configuration instead of interpolating it into the inline JavaScript. Update the script around buildScorecard and the existing process.env-based inputs to read needs from process.env and parse it as data, preserving the current value while eliminating direct template insertion into the script body.Sources: Path instructions, 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.
Outside diff comments:
In @.github/workflows/e2e.yaml:
- Around line 5156-5175: Pass the needs context through the step’s env
configuration instead of interpolating it into the inline JavaScript. Update the
script around buildScorecard and the existing process.env-based inputs to read
needs from process.env and parse it as data, preserving the current value while
eliminating direct template insertion into the script body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b7fc505b-bbc5-49e7-aa8f-53af5338f101
📒 Files selected for processing (6)
.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/docs/README.mdtest/e2e/live/onboard-resume.test.tstest/e2e/support/openshell-gateway-auth-source-contract-helpers.test.tstools/e2e/openshell-gateway-auth-artifact-safety.mts
💤 Files with no reviewable changes (1)
- test/e2e/live/onboard-resume.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- test/e2e/README.md
- test/e2e/docs/README.md
- tools/e2e/openshell-gateway-auth-artifact-safety.mts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
cv
approved these changes
Jul 23, 2026
23 tasks
23 tasks
cv
added a commit
that referenced
this pull request
Jul 23, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical dated `## v0.0.93` release entry to `docs/changelog/2026-07-23.mdx`. The entry records user-visible behavior, release validation, and documentation controls merged after `v0.0.92`, while preserving the pending DGX OS `7.6.x` Station Express qualification caveat. ## Changes - Adds the parser-safe dated release entry with a summary, grouped details, and published-route links. - Reconciles the `v0.0.92..origin/main` commit range with merged `v0.0.93` PRs. - Records that no-OTA DGX OS `7.6.x` passed bounded host preflight, while full Station Express end-to-end qualification remains pending. - Leaves existing product pages unchanged because the source PRs already document their supported behavior. ### Source summary - #7285 -> `docs/changelog/2026-07-23.mdx`: Records the existing-vLLM ownership choice and resumable Station handoff. - #7419 -> `docs/changelog/2026-07-23.mdx`: Records bounded no-OTA DGX OS `7.6.x` recognition and its pending end-to-end qualification. - #7268 -> `docs/changelog/2026-07-23.mdx`: Records optional Hugging Face authentication, output sanitization, and resumable HTTP `429` recovery. - #7442 -> `docs/changelog/2026-07-23.mdx`: Records clean SIGINT handling at hidden credential prompts. - #7299 -> `docs/changelog/2026-07-23.mdx`: Records Intel macOS rejection before ref resolution or network work. - #7296 -> `docs/changelog/2026-07-23.mdx`: Records the DGX Spark non-interactive local-vLLM selection order. - #7342 -> `docs/changelog/2026-07-23.mdx`: Records delegated protected E2E approvals in the grouped release-validation bullet. - #7373 -> `docs/changelog/2026-07-23.mdx`: Records base-image publication gating before final-main fanout. - #7388 -> `docs/changelog/2026-07-23.mdx`: Records semantic phase runtime summaries. - #7397 -> `docs/changelog/2026-07-23.mdx`: Records progress coverage hardening. - #7391 -> `docs/changelog/2026-07-23.mdx`: Records centralized larger-runner routing. - #7423 -> `docs/changelog/2026-07-23.mdx`: Records one retry for confirmed hosted-runner loss. - #7399 -> `docs/changelog/2026-07-23.mdx`: Records runner-comparison telemetry. - #7270 -> `docs/changelog/2026-07-23.mdx`: Records staging Brev Launchable validation. - #7426 -> `docs/changelog/2026-07-23.mdx`: Records filtering of irrelevant base-image run history. - #7333 -> `docs/changelog/2026-07-23.mdx`: Records aligned Quickstart platform guidance. - #7343 -> `docs/changelog/2026-07-23.mdx`: Records documentation-writer receipt collection. - #7400 -> `docs/changelog/2026-07-23.mdx`: Records the documentation-writer receipt requirement for docs-only PRs. - #7413 -> `docs/changelog/2026-07-23.mdx`: Records removal of redundant receipt PR metadata. - #7405 -> `docs/changelog/2026-07-23.mdx`: Records corrected inference CLI references. - #7389 -> `docs/changelog/2026-07-23.mdx`: Records completion of the v0.0.91 documentation audit. `#7384` is an internal refactor with no intended runtime behavior change. `#7401` updates internal CodeQL Actions dependencies. `#7376` is already contained in `v0.0.92`, so it is outside the release-entry scan range despite its retained planning label. ## 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 - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, SPDX syntax, and version headings. - [ ] Tests not applicable — justification: - [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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed `docs/changelog/2026-07-23.mdx` against `WRITING.md`, `docs/CONTRIBUTING.md`, `docs/.docs-skip`, `docs/index.yml`, the six user-visible source PRs, and the remaining grouped release commits. The review corrected an ambiguous qualification claim, confirmed all published routes, preserved the DGX OS `7.6.x` caveat, and found no remaining action. - Agent: Codex Desktop <!-- docs-review-head-sha: ec0a866 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable. This PR does not change `scripts/prepare-dgx-station-host.sh`. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## 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 check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts`: 1 file and 6 tests passed. - [ ] Applicable broad gate passed — Not applicable to one native changelog file. - [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) — completed with 0 errors and 2 existing Fern warnings. - [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) — not applicable because native changelog entries use a parser-safe MDX SPDX comment without frontmatter. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added the v0.0.93 changelog covering onboarding and validation improvements. * Documented support for additional DGX Station Express workstation releases and clearer handling of existing vLLM workloads. * Added guidance for optional Hugging Face authentication, resumable rate-limit recovery, and DGX Spark provider selection. * Clarified installer behavior on Intel macOS, release validation requirements, hosted-runner retries, documentation checks, and supported CLI quickstart paths. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#7100 established semantic phase coverage for all live E2E cases. This follow-up closes the remaining contract gaps without reapplying that implementation: it covers workflow-selected credential-free integration tests, requires E2E child-process boundaries to expose content-free liveness, and blocks OpenShell gateway-auth artifact uploads unless a fail-closed safety scan approves the current run attempt. Together with #7100, this completes #7101 while preserving current
mainbehavior in the audited overlap areas.Related Issue
Fixes #7101
Changes
spawnObservedChildas the audited direct asynchronous process boundary used by E2E helpers. The semantic checker rejects unaudited direct process APIs and requires synchronous calls to use a bounded timeout plusSIGKILL.workflow-e2e-testfixture for credential-free integration tests selected by the authoritative E2E planner, so they publish and validate the same semantic timeline and progress artifact contract without the stateful live fixture.Type of Change
Quality Gates
main, the Hermes shard implementations are unchanged, and the gateway-auth scanner/upload path fails closed. Focused scanner and workflow-boundary tests pass on the rebased commit.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run test:e2e-phases:check: 123 tests across 82 files passed; selected integration suite: 6 files passed and 1 skipped, with 37 tests passed and 3 skipped; CLI and plugin type-checks, CLI build, and diff checks passednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — localnpm run checkcompleted the full pre-commit stage, but its intentionally serial CLI/integration coverage lane reached the four-hour execution ceiling before producing a final summary; required sharded CI is authoritativenpm run docsbuilds without warnings (doc changes only) — passed with 0 errors and 2 existing Fern warningsSigned-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
test:e2e-phases:checkcontract.Tests
Documentation Writer Review
docs-updatedCONTRIBUTING.md,test/e2e/README.md,test/e2e/docs/README.md