fix: fail lesson verification before reporting - #4142
Draft
Jeremy1844 wants to merge 2 commits into
Draft
Conversation
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
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.
Root cause
The verifier marked a run failed only after optional pull-request comment delivery. Local/non-PR execution returned before setting failure, and a comment-service error could throw before the process received a failing status. Known validation errors could therefore be reported while the command still exited successfully.
Change
Signal the validation failure before either local reporting or optional pull-request comment delivery, and add regression coverage for all three exit paths.
Evidence
fe492faf7433053c9f65cfd4e1ccd03197e31e7e(stage)3ff034858b35d4a99d25c8de6aee4acb5cd43090— 1/3 introduced tests passed74365599ac67a51b2e34c68521b56ed1cc3e48f1— 3/3 passed on Node 16.20.2 and Node 24src,images,web, workflows, and encrypted artifact objects are byte-identical to the baseScope and rollout
This PR changes
verify.jsand one synthetic regression test only. It contains no lesson, image, generated web, dependency, workflow, credential, schema, persistence, or deployment changes.The repository currently has no lockfile, so native
npm ciis unavailable; an ephemeral public-registry lock was used only to reproduce both supported test environments. Lockfile/toolchain and baseline dependency advisories remain separate remediation roots.Before merge, run the hosted PR annotation/comment path and the approved production corpus with the authorized private parser package. Rollback the complete two-commit range; do not restore a successful exit status for known validation failures.