ci: unify native packaging and harden releases#2012
Open
yyhhyyyyyy wants to merge 11 commits into
Open
Conversation
Document reusable workflow ownership, release verification order, and package-size provenance. Record the remote-validation boundary and refresh generated ACP registry data.
Contributor
📝 WalkthroughWalkthroughThis change replaces duplicated platform packaging jobs with reusable Windows, Linux, and macOS workflows; adds package contracts, manifests, size policies, regression gating, and fail-closed release assembly; updates related tests and CI documentation; and removes the former Light OCR package-size action and script. ChangesCI Packaging and Release
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/package-check.ymlTraceback (most recent call last): .github/workflows/prcheck.ymlTraceback (most recent call last): 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/architecture/ci-release-packaging/spec.md (1)
147-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify what “complete verification artifact contract” means.
AC-2 and AC-6 prohibit publishing complete unsigned installers, so this wording could be misread as requiring them. Specify that completeness applies to manifests, reports, and verification metadata—not unsigned installer publication.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture/ci-release-packaging/spec.md` around lines 147 - 148, Clarify the “complete verification artifact contract” statement in the PR artifact guidance to explicitly cover manifests, reports, and verification metadata. State that it does not require publishing complete unsigned installers, while preserving the prohibition on unsigned installer publication and avoiding a reduced updater-only artifact set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/ci-release-packaging/spec.md`:
- Around line 121-123: The classifier-loading path must require the PR base
revision’s classifier and never fall back to the candidate classifier. Update
the classifier-only impact flow around loading
scripts/ci/classify-package-impact.mjs to fail before producing impact outputs
when the base classifier, base commit, merge-base, or package.json validation is
unavailable, preserving the existing fail-closed behavior for all of these
checks.
---
Nitpick comments:
In `@docs/architecture/ci-release-packaging/spec.md`:
- Around line 147-148: Clarify the “complete verification artifact contract”
statement in the PR artifact guidance to explicitly cover manifests, reports,
and verification metadata. State that it does not require publishing complete
unsigned installers, while preserving the prohibition on unsigned installer
publication and avoiding a reduced updater-only artifact set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c739884b-9597-40d9-b4d6-a1677c2d0ec3
📒 Files selected for processing (11)
.github/workflows/package-check.yml.github/workflows/prcheck.ymldocs/architecture/ci-release-packaging/plan.mddocs/architecture/ci-release-packaging/spec.mddocs/architecture/ci-release-packaging/tasks.mdscripts/ci/classify-package-impact.mjstest/main/memory/memoryRetrieval.eval.test.tstest/main/scripts/electronUpdaterCompatibility.test.tstest/main/scripts/packageCheckWorkflow.test.tstest/main/scripts/packageContract.test.tstest/main/scripts/prcheckWorkflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/architecture/ci-release-packaging/tasks.md
- docs/architecture/ci-release-packaging/plan.md
- test/main/scripts/packageContract.test.ts
Comment on lines
+121
to
+123
| - The classifier is loaded from the PR base revision when available, so a classifier-only change | ||
| cannot use its candidate rules to skip its own package validation. Workflow changes remain | ||
| protected by contract tests and normal review policy. |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file availability =="
git ls-files | grep -F 'docs/architecture/ci-release-packaging/spec.md' || true
echo "== surrounding spec sections =="
sed -n '90,160p' docs/architecture/ci-release-packaging/spec.md
echo "== search classifier/base revision/package-required mentions =="
rg -n "classifier|base revision|package-required|skip.*validation|candidate rules|checkout|unavailable" docs/architecture/ci-release-packaging/spec.md .github -S || trueRepository: ThinkInAIXYZ/deepchat
Length of output: 8086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package-check classifier setup and package-required =="
sed -n '40,80p' .github/workflows/package-check.yml
echo "== package-check required status =="
sed -n '110,130p' .github/workflows/package-check.yml 2>/dev/null || true
echo "== classifier outputs =="
rg -n "console\.|console\.error|process\.exit|package-required|package_required|affected|^ os|affected OS|skip|false|true" scripts/ci/classify-package-impact.mjs .github/workflows/package-check.yml .github/workflows/_package*.yml -S
echo "== classifier file size/hash =="
wc -l scripts/ci/classify-package-impact.mjs && git rev-parse --show-toplevel
sed -n '1,220p' scripts/ci/classify-package-impact.mjsRepository: ThinkInAIXYZ/deepchat
Length of output: 14511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== remaining classifier file and workflow package-required =="
sed -n '220,285p' scripts/ci/classify-package-impact.mjs
sed -n '280,370p' scripts/ci/classify-package-impact.mjs
sed -n '117,175p' .github/workflows/package-check.yml
echo "== behavioral probe: classifier path influence on output =="
node - <<'JS'
const specPath = '.github/workflows/package-check.yml';
const baseRules = [];
const headRules = [{ id: 'classifier-only', platforms: ['windows', 'linux'], matches: (path) => path === specPath }];
class ImpactClassifier {
constructor(rules) { this.rules = rules; }
findChangedPathRules(changedPath) {
return this.rules.filter((rule) => rule.matches(changedPath));
}
classify(paths) {
const impact = { windows: false, linux: false, macos: false };
for (const changedPath of paths) {
for (const rule of this.findChangedPathRules(changedPath)) {
for (const platform of rule.platforms) impact[platform] = true;
}
}
return impact;
}
}
console.log(JSON.stringify({
classifierOnlyPathToBaseRules: new ImpactClassifier(baseRules).classify([specPath]),
classifierOnlyPathToHeadRules: new ImpactClassifier(headRules).classify([specPath]),
}));
JSRepository: ThinkInAIXYZ/deepchat
Length of output: 7252
Require the base classifier instead of falling back to the candidate classifier.
The classifier-only path matches against the rules in scripts/ci/classify-package-impact.mjs, so falling back to the PR’s own classifier can make package-required run a classifier-only PR. If the base classifier cannot be loaded, fail before producing impact outputs; keep base commit, merge-base, and package.json validation in this same fail-closed path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/ci-release-packaging/spec.md` around lines 121 - 123, The
classifier-loading path must require the PR base revision’s classifier and never
fall back to the candidate classifier. Update the classifier-only impact flow
around loading scripts/ci/classify-package-impact.mjs to fail before producing
impact outputs when the base classifier, base commit, merge-base, or
package.json validation is unavailable, preserving the existing fail-closed
behavior for all of these checks.
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
1 participant
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.
Summary
Refactor DeepChat's native packaging pipeline around three OS-level reusable workflows and replace permissive release collection with a fail-closed package contract.
This keeps
electron-builderandelectron-updaterwhile removing the duplicated Windows, Linux, and macOS implementations previously maintained independently in Build and Release.What changed
Packaging architecture
Each reusable workflow owns its native runner mapping, runtime installation, plugin packaging, Electron packaging, packaged smoke tests, component budgets, manifest generation, and artifact staging.
Callers select only:
distributionorverificationpurposeRelease safety
Release now fails if:
Release preflight also requires the tag to exist, resolve to an immutable commit reachable from
origin/main, matchpackage.json, and have non-empty CHANGELOG notes before native packaging starts.Signing and permissions
secrets: inheritis not used.contents: read; only draft publication receivescontents: write.Package-size regression
The previous implementation rebuilt the historical baseline once for every target.
This PR replaces that with:
29978292769Manual Build keeps lightweight component budgets but skips installer-delta comparison. Release and package regression enforce the complete installer policy.
Summary by CodeRabbit