Skip to content

🏗️🔧:fail the drift checks when they crash - #1924

Merged
openinf-commit-queue[bot] merged 1 commit into
livefrom
claude/full-audit-0e491i-drift-exit
Oct 20, 2026
Merged

openinf-commit-queue[bot] merged 1 commit into
livefrom
claude/full-audit-0e491i-drift-exit

Conversation

@DerekNonGeneric

@DerekNonGeneric DerekNonGeneric commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Requested by DerekNonGeneric · project thread

Before: the vendored-files and SDK API drift checks used exit code bit 1 to mean "drifted" or "behind". Node exits with 1 on an uncaught exception and 13 on a top-level await that never settles, and both of those set bit 1. So a check that crashed was read as one that found something: it filed an issue with an empty report, and the job passed.

After: a crash fails the job and files no issue. Real answers still work as before.

How: each task exits 0 when there is nothing to report, and otherwise 16 plus the same bits (17 behind or drifted, 18 could not check, 19 both). No answer can then share a number with a code Node picks for itself. The workflow steps accept only 0 and 17 to 19, and treat any other code as "could not check", which the existing "Fail if…" step turns into a failed job. A new test, build/shared/check-answers.test.mts, takes the real compare step script out of each workflow and runs it with bash -e against every answer and against real crashes: exit 1 from a throw, exit 13 from an unsettled await, and 137 from a killed process. With the old workflows and tasks, 7 of its 9 cases fail; with this change, all 9 pass. nps test passes, apart from three checks that need network access the sandbox does not have.

Note: this touches .github/workflows/, so the commit queue's app may be refused for lack of the workflows permission, as it was on the SDK today.

Summary by CodeRabbit

  • Bug Fixes
    • SDK API and vendored-file checks now distinguish confirmed matches or differences from checks that could not complete. Unexpected check results are reported as unchecked rather than being interpreted as a confirmed match.
  • Tests
    • Added coverage for normal check results and simulated failures, including verification that incomplete comparisons are reported as unchecked.

The vendored and SDK API drift checks answered through bit 1 of their
exit code for "drifted" or "behind", but node exits 1 on an uncaught
exception and 13 on a top-level await that never settles. A check that
crashed was read as one that found something: it filed an issue with
an empty report and the job passed.

The tasks now exit 0 when there is nothing to report and 16 plus the
same bits otherwise, so no answer can be confused with a code node
picks for itself. The workflows accept only 0 and 17 to 19, and treat
any other code as a check that could not run, which fails the job
instead of filing an issue. A test runs the workflows' own step
scripts against each answer and each kind of crash.

Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is>
Assisted-by: Claude-Code:claude-opus-5
@DerekNonGeneric DerekNonGeneric self-assigned this Oct 19, 2026
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: df873b1f-1ca8-468c-9465-e65077c8a2be
📥 Commits

Reviewing files that changed from the base of the PR and between 993ed90 and 46839f9.

📒 Files selected for processing: 5
  • .github/workflows/sdk-api-drift.yml
  • .github/workflows/vendored-sync.yml
  • build/shared/check-answers.test.mts
  • build/tasks/check-sdk-api-drift.mts
  • build/tasks/check-vendored.mts

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Walkthrough

The SDK API drift and vendored checkers now distinguish matching results, completed checks with findings, and checks that did not answer. Their workflows normalize checker exit codes before setting finding and unchecked outputs. Regression tests cover supported statuses and simulated failures.

Changes: Checker status handling

Layer / File(s) Summary
Checker exit status bits
build/tasks/check-sdk-api-drift.mts, build/tasks/check-vendored.mts
Both checkers return 0 for matching results. For other completed checks, they set the ANSWERED bit and include the applicable result bits.
Workflow status outputs and regression tests
.github/workflows/sdk-api-drift.yml, .github/workflows/vendored-sync.yml, build/shared/check-answers.test.mts
Both workflows default missing codes to 0, accept 17–19, and convert other nonzero codes to 18. Their outputs use the normalized code. Tests cover supported codes, simulated crashes, and a vendored check without comparison files.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 46839

Checker crashes are reported as failed checks without filing an empty-report issue. No merge-blocking issue was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: drift checks now fail when they crash. The emojis add minor noise but do not obscure the meaning.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (2 skipped: 2 …
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@netlify

netlify Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for gh-pages-openinf ready!

Name Link
🔨 Latest commit 46839f9
🔍 Latest deploy log https://app.netlify.com/projects/gh-pages-openinf/deploys/6ad5ca78d8af48c70a1526fc
😎 Deploy Preview https://deploy-preview-1924--gh-pages-openinf.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Member Author

@coderabbitai review


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@DerekNonGeneric DerekNonGeneric added the 🚀 Status: Commit Queue Land this pull request when its checks pass label Oct 20, 2026 — with Claude
@openinf-commit-queue
openinf-commit-queue Bot merged commit b75f1c9 into live Oct 20, 2026
19 checks passed
@openinf-commit-queue openinf-commit-queue Bot removed the 🚀 Status: Commit Queue Land this pull request when its checks pass label Oct 20, 2026
@openinf-commit-queue
openinf-commit-queue Bot deleted the claude/full-audit-0e491i-drift-exit branch October 20, 2026 00:15
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.

1 participant