feat: add shared GuardScan distribution catalog foundation - #35
feat: add shared GuardScan distribution catalog foundation#35ntanwir10 wants to merge 2 commits into
Conversation
|
Warning Review limit reachedNext included review available in 51 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (81)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
@codex review |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3335331e97
ℹ️ 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".
| platformId: [platform.os, platform.arch, platform.libc].filter(Boolean).join('-'), | ||
| nodeVersion: process.version.slice(1), | ||
| executable, | ||
| }, options.outputDir).metadata; |
There was a problem hiding this comment.
The production artifact-sbom path supplies only the Node version and executable metadata. Since createArtifactSboms adds third-party components solely from input.components, every published SPDX and CycloneDX document contains just Node and GuardScan while omitting all packages bundled into the SEA. Consumers relying on these release SBOMs therefore miss dependency vulnerabilities and licenses; populate the component inventory from the lockfile or bundle metafile before writing the SBOMs.
Useful? React with 👍 / 👎.
| appendEvent(ledger, { | ||
| version, tag, commit, | ||
| timestamp: report.checkedAt, | ||
| type: 'canary_recorded', |
There was a problem hiding this comment.
When canary workflows overlap or another ledger writer records an event after this report captures checkedAt but before the record job obtains its concurrency slot, this timestamp is older than the ledger tail. appendEvent then rejects it because release event timestamps may not move backward, causing the whole evidence-recording job to fail. Use an append-time monotonic timestamp and preserve checkedAt separately in the payload.
Useful? React with 👍 / 👎.
| remote_files = {item["filename"]: item["digests"]["sha256"] for item in remote["urls"]} | ||
| for wheel in pathlib.Path("dist").glob("*.whl"): | ||
| digest = hashlib.sha256(wheel.read_bytes()).hexdigest() | ||
| if wheel.name in remote_files and remote_files[wheel.name] != digest: | ||
| raise SystemExit(f"PyPI integrity conflict for {wheel.name}") |
There was a problem hiding this comment.
When this version already exists on PyPI, the preflight checks only local filenames that also occur remotely; it never rejects remote filenames absent from dist. A partially published or conflicting version containing an extra wheel or sdist therefore passes, and skip-existing: true proceeds to produce a registry release that is not the immutable artifact set in the GuardScan manifest. Compare the complete remote filename-and-digest map with the intended wheel set before publishing.
Useful? React with 👍 / 👎.
| node cli/scripts/release/index.js promote \ | ||
| --promotion-input promotion-input.json \ | ||
| --output promotion-decision.json \ | ||
| --ledger release-events.jsonl \ | ||
| --idempotency-key "promotion:${{ inputs.version }}:${GITHUB_RUN_ID}:${GITHUB_RUN_ATTEMPT}" \ | ||
| --tag "$RC_TAG" |
There was a problem hiding this comment.
When the machine policy denies promotion, the promote command writes the local decision and ledger event but exits with status 2. GitHub's bash runner executes run blocks with error-exit enabled, so this command stops the step before the following worktree copy and push; denied decisions—such as a changed release-PR head or incomplete soak—are never recorded in the protected ledger. Capture status 2, persist the decision, and only then prevent the merge.
Useful? React with 👍 / 👎.
| PR_STATE="$(gh pr view --repo "ntanwir10/$REPO" "$PR_NUMBER" --json state --jq .state)" | ||
| if [ "${{ inputs.channel }}" = stable ] && [ "$PR_STATE" != MERGED ]; then | ||
| gh pr merge --repo "ntanwir10/$REPO" "$PR_NUMBER" --auto --squash | ||
| fi |
There was a problem hiding this comment.
For stable releases this only invokes gh pr merge --auto and never verifies that the PR reaches MERGED. As confirmed by gh pr merge --help, --auto means “Automatically merge only after necessary requirements are met,” so the command can succeed merely by enabling auto-merge while checks or a merge queue remain pending; the reusable publication job then succeeds and the downstream record job marks Homebrew and Scoop as channel_published even though their catalogs still contain the old release.
Useful? React with 👍 / 👎.
| brew install ntanwir10/tap/guardscan | ||
| brew test ntanwir10/tap/guardscan | ||
| brew upgrade --dry-run ntanwir10/tap/guardscan | ||
| guardscan --version | ||
| brew uninstall guardscan |
There was a problem hiding this comment.
For stable trains, the Homebrew canary installs the mutable current tap formula and treats any successful guardscan --version invocation as evidence for matrix.train.version. If the adapter PR is still pending or a newer stable formula has replaced an older active train, this installs a different version yet records a passing canary for the requested release. Compare the command output with $VERSION; the Scoop path has the same unchecked-version behavior.
Useful? React with 👍 / 👎.
| node cli/scripts/release/index.js rollback "${ARGS[@]}" > rollback-plan.json | ||
| cat rollback-plan.json | ||
| git worktree add ledger-branch origin/release-ledger | ||
| cp release-events.jsonl "ledger-branch/events/v${{ inputs.version }}.jsonl" |
There was a problem hiding this comment.
For an RC rollback, this persists only rollback_started; it neither removes the version from active-versions.json nor changes any channel or incident state used by eligibility checks. The scheduler therefore continues dispatching reconciliation for the rolled-back RC, and the reconciliation step can trigger action=promote as soon as its existing canary counts pass because it checks only samples and open incidents. Mark the train inactive or explicitly make rollback state block reconciliation and promotion before pushing this ledger update.
Useful? React with 👍 / 👎.
| run: npm run release:npm-preflight -- --artifact-dir ../npm-artifact --github-output "$GITHUB_OUTPUT" | ||
| - name: Publish exact tested tarball | ||
| if: steps.preflight.outputs.publish-required == 'true' | ||
| working-directory: cli | ||
| run: | | ||
| DIST_TAG=latest |
There was a problem hiding this comment.
When guardscan@version already exists with matching integrity but its latest or next dist-tag is absent or has been moved, npm-preflight returns publish-required=false. This skips the only block that selects and writes the intended dist-tag, while the final preflight—which also checks only integrity—still succeeds, so the release is recorded as published but normal installs through that tag continue resolving another version. Query and reconcile the expected dist-tag even when the tarball itself does not need publishing.
Useful? React with 👍 / 👎.
1 participant
Stack 2 of 4 for #32; depends on #34. Review boundary: d1d2616..3335331 (81 changed files). Adds the shared Homebrew and Scoop catalog engine plus hardened scan and release contracts. Automation and publication remain disabled.