Skip to content

fix(e2e): reject all-skipped catalogue targets - #9027

Merged
cv merged 1 commit into
mainfrom
codex/fix-e2e-skipped-evidence
Aug 13, 2026
Merged

fix(e2e): reject all-skipped catalogue targets#9027
cv merged 1 commit into
mainfrom
codex/fix-e2e-skipped-evidence

Conversation

@rsliter

@rsliter rsliter commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Prevent catalogue E2E jobs from reporting success when Vitest executes no selected assertions. The planner now excludes the two targets whose declared environments always skip their tests, and the catalogue runner fails closed if a future selection is all skipped.

Related Issue

Fixes #9022

Changes

  • Remove issue-4434-tui-unreachable-inference and overlayfs-autofix from the release catalogue while preserving explicit exclusion reasons for manual selectors.
  • Require catalogue Vitest runs to execute at least one passing or failing assertion before returning success.
  • Preserve the typed registry's existing explicit unsupported-target skip behavior outside catalogue execution.
  • Document the catalogue execution contract and add planner, reporter, and runner-boundary regression tests.

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: No supported product behavior changes. test/e2e/README.md documents the contributor-facing qualification contract.
  • 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: Codex Desktop reviewed the security rubric. Secrets, injection, authentication, authorization, dependencies, errors and logging, configuration, security tests, and system interaction pass; cryptography is not applicable. The catalogue owns the fixed activation flag, the reporter emits only aggregate counts, and no credential or shell boundary changes.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Evidence: test/e2e/README.md documents the all-skipped catalogue execution failure. Public docs do not change because this is a contributor qualification boundary, not supported product behavior.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable
  • Station profile/scenario: Not applicable
  • Result: Not applicable; scripts/prepare-dgx-station-host.sh is unchanged.
  • Supporting evidence: Not applicable

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 75 focused E2E-support tests and 24 reporter integration tests passed. The exact all-skipped [DGX Spark][CLI&UX] openclaw tui shows indefinite spinner with no error when inference endpoint is unreachable #4434 reproduction exits 1 with the expected annotation.
  • Applicable broad gate passed — npm run checks:repository passed; npm run test:e2e-phases:check covered 127 tests across 83 files. A full E2E-support attempt passed 1,923 tests and hit six unrelated macOS host limitations involving systemd, GNU find, and worker timeouts; GitHub Actions remains authoritative for that Linux lane.
  • 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)

Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes

    • Live E2E selections now fail when no tests execute, including when all selected tests are skipped.
    • Ordinary unsupported-registry skips no longer incorrectly change the run’s exit status.
    • Removed E2E targets are rejected with clear exclusion reasons.
  • Documentation

    • Updated E2E guidance to explain that selections with no executed tests fail before evidence is created.
  • Testing

    • Added coverage for skipped, passed, excluded, and environment-variable execution scenarios.

Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
@rsliter
rsliter requested a review from cv August 13, 2026 19:30
@coderabbitai

coderabbitai Bot commented Aug 13, 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: 93bda81f-6ec8-4382-ad70-4fcb42733d80

📥 Commits

Reviewing files that changed from the base of the PR and between 48a8158 and 59dc474.

📒 Files selected for processing (7)
  • test/e2e-risk-signal-reporter.test.ts
  • test/e2e/README.md
  • test/e2e/risk-signal-reporter.ts
  • test/e2e/support/runner-pressure-workflow-boundary.test.ts
  • test/e2e/support/workflow-plan.test.ts
  • tools/e2e/target-catalogue.mts
  • tools/e2e/workflow-plan.mts

📝 Walkthrough

Walkthrough

The E2E catalogue removes two targets that cannot execute product assertions. Workflow planning rejects those targets with explicit reasons. Live catalogue runs now fail when no selected tests execute.

Changes

E2E catalogue enforcement

Layer / File(s) Summary
Catalogue exclusions and planning validation
tools/e2e/target-catalogue.mts, tools/e2e/workflow-plan.mts, test/e2e/support/workflow-plan.test.ts
The catalogue removes issue-4434-tui-unreachable-inference and overlayfs-autofix. Workflow planning rejects both identifiers with explicit exclusion reasons.
Fail-closed live execution
test/e2e/risk-signal-reporter.ts, tools/e2e/target-catalogue.mts, test/e2e-risk-signal-reporter.test.ts, test/e2e/support/runner-pressure-workflow-boundary.test.ts, test/e2e/README.md
Catalogue runs require at least one executed test. The reporter fails all-skipped successful selections, while ordinary unsupported-registry skips remain unchanged. Tests and documentation cover the behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Mergeability Score: ⚪ Minimal · up to 59dc4

The catalogue now rejects all-skipped selections and excludes known non-executable targets, preventing false-success E2E results; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant TargetCatalogue
  participant Vitest
  participant RiskSignalReporter
  TargetCatalogue->>Vitest: Set NEMOCLAW_E2E_REQUIRE_EXECUTED_TEST=1
  Vitest->>RiskSignalReporter: Complete selected test run
  RiskSignalReporter->>RiskSignalReporter: Count passed, failed, skipped, and pending tests
  RiskSignalReporter->>TargetCatalogue: Set process.exitCode=1 when no tests execute
Loading

Possibly related PRs

Suggested labels: area: e2e, bug-fix, v0.0.109

Suggested reviewers: cv, apurvvkumaria

🚥 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: rejecting catalogue targets that execute no tests.
Linked Issues check ✅ Passed The changes implement the issue requirements by failing all-skipped runs, excluding both affected targets with reasons, and preserving fail-closed evidence handling.
Out of Scope Changes check ✅ Passed All modified files support catalogue qualification, exclusion handling, planner validation, runner setup, tests, or documentation; no unrelated product changes appear.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-e2e-skipped-evidence

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

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 59dc474 in the codex/fix-e2e-skippe... branch remains at 96%, unchanged from commit 48a8158 in the main branch.

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized terminology decisions differ; normalized E2E selections differ; severity counts match.
2 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • live E2E selection at test/e2e/risk-signal-reporter.ts:73: primary classified it as replace; the second opinion classified it as established.
  • catalogue target at tools/e2e/workflow-plan.mts:425: selected only by the second-opinion lane as established.
2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • rebuild-hermes: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • agent-turn-latency: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • define — catalogue Vitest selection at test/e2e/README.md:278: Define this term at first use as a catalogue target's Vitest invocation.
  • replace — Live E2E selection at test/e2e/risk-signal-reporter.ts:73: Use "Catalogue E2E selection" in the error message and test expectation to identify the affected execution boundary.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: None

Manual-only E2E: cloud-onboard, security-posture, cloud-inference
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@cv
cv merged commit 96e1e7f into main Aug 13, 2026
85 of 94 checks passed
@cv
cv deleted the codex/fix-e2e-skipped-evidence branch August 13, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

E2E catalogue schedules targets that skip all product assertions

2 participants