Skip to content

fix: preserve scorecard errors and revision integrity - #16

Merged
altaywtf merged 3 commits into
mainfrom
fix/scorecard-integrity
Sep 5, 2026
Merged

fix: preserve scorecard errors and revision integrity#16
altaywtf merged 3 commits into
mainfrom
fix/scorecard-integrity

Conversation

@altaywtf

@altaywtf altaywtf commented Sep 5, 2026

Copy link
Copy Markdown
Member

Problem

Transport errors could become scored failures, and partial reruns could silently retain rows from a different skills revision.

Solution

Use the existing result classifier and validate provenance after merging retained rows, before writing the scorecard. Preserve graded failures, explicit mixed-revision overrides, and the previous file on rejection. Reject skipped reruns that overlap retained rows so an old score cannot silently survive a failed rerun. Persist attempted identities until graded output and provenance are complete, including when the child writes no result file. Move only npm publishing to a GitHub-hosted runner because provenance rejects self-hosted runners.

Proof

Regression fixtures cover transport errors, graded failures, malformed rows, mixed-revision rejection without overwrite, override, and complete replacement. The uncached full gate passed 50 source tests and the packed-consumer install/lint test. actionlint and zizmor passed; independent Astra review is clean. No paid evaluations ran.

Copilot AI lite review requested due to automatic review settings September 5, 2026 09:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T10:25:43.125159Z 98f0a57 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@slopzapper slopzapper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ CLEAN

The change is clean. Summarize now skips ungraded errors, blocks mixed revisions after merge without rewriting the scorecard, and publishes from a GitHub-hosted runner.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

There is a TypeScript type mismatch in classifyResult and a potential UTC-midnight flake in the new summarize test that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Tightens skillcheck summarize/result reduction semantics so transport errors don’t become scored failures, and partial reruns can’t silently merge rows from a different skills-tree revision into a scorecard.

Changes:

  • Route reduceResults() through the existing classifyResult() to skip ungraded transport errors while retaining graded assertion failures.
  • Add provenance validation after merging carried scorecard rows, rejecting mixed-revision scorecards unless --allow-mixed (and avoiding overwrite on rejection).
  • Update docs and release workflow to reflect the new behavior and publish on a GitHub-hosted runner for npm provenance.
File summaries
File Description
test/cli.test.ts Adds regression tests for transport-error skipping, malformed rows, and mixed-revision retention behavior in summarize.
src/cli.ts Reuses classifyResult() during reduction and validates mixed revisions after merge before writing scorecards.
docs/usage.md Documents skipping transport errors and mixed-revision rejection behavior (including “no overwrite on rejection”).
.github/workflows/release.yml Switches publish runner to GitHub-hosted Ubuntu to satisfy npm provenance constraints.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/cli.test.ts Outdated
Comment thread src/cli.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a9dbf01a9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/cli.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fab785a49

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/cli.ts

@slopzapper slopzapper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ CLEAN

The PR routes scorecard reduction through classifyResult, refuses to merge a skipped rerun into an existing same-date row, and rejects mixed revisions after merge. Tests cover those paths, and the release runner change matches npm's GitHub-hosted provenance requirement.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98f0a57dd5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/cli.ts

@slopzapper slopzapper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ CLEAN

The eligible diff matches the documented scorecard, attempt-marker, and mixed-revision contracts, and the publish runner change matches npm's GitHub-hosted requirement. No proved defect.

@altaywtf
altaywtf merged commit 5529614 into main Sep 5, 2026
5 checks passed
@altaywtf
altaywtf deleted the fix/scorecard-integrity branch September 5, 2026 10:28
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.

2 participants