fix: load schema via require() for pkg/esbuild compat#2396
Conversation
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 85.74% | 85.83% | 📈 +0.09% |
| Statements | 85.63% | 85.71% | 📈 +0.08% |
| Functions | 88.15% | 88.11% | 📉 -0.04% |
| Branches | 78.63% | 78.69% | 📈 +0.06% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
87.4% → 87.7% (+0.29%) | 87.0% → 87.3% (+0.27%) |
src/schema-validator.ts |
98.0% → 100.0% (+1.97%) | 98.0% → 100.0% (+1.97%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
There was a problem hiding this comment.
Pull request overview
This PR changes runtime schema loading so config validation works inside packaged/bundled artifacts, especially the pkg binary that previously could not find awf-config-schema.json at runtime. It fits into the config-validation path added in PR #2384 by adjusting how the generated schema is loaded during CLI startup.
Changes:
- Replace filesystem-based schema loading in
schema-validator.tswith a literalrequire('./awf-config-schema.json'). - Add
dist/awf-config-schema.jsontopkg.assetsas an explicit packaged asset. - Update inline documentation to describe the new bundler/
pkgloading behavior.
Show a summary per file
| File | Description |
|---|---|
src/schema-validator.ts |
Switches schema loading from runtime file reads/fallback paths to a direct require() of the generated JSON schema. |
package.json |
Adds the generated schema JSON under pkg.assets so the packaged binary can include it explicitly. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
The release binary failed because pkg couldn't find awf-config-schema.json at runtime — the fs.readFileSync approach didn't work in pkg's snapshot filesystem. Switch to require() which both esbuild (inlines into bundle) and pkg (includes in snapshot via static analysis) handle natively. Also add dist/awf-config-schema.json to pkg.assets as a fallback. Fixes: Release workflow 'Smoke test binary (x64)' failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lpcox
force-pushed
the
fix/schema-pkg-compat
branch
from
0d107cd to
24b3cce
Compare
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 85.74% | 85.83% | 📈 +0.09% |
| Statements | 85.63% | 85.71% | 📈 +0.08% |
| Functions | 88.15% | 88.11% | 📉 -0.04% |
| Branches | 78.63% | 78.69% | 📈 +0.06% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
87.4% → 87.7% (+0.29%) | 87.0% → 87.3% (+0.27%) |
src/schema-validator.ts |
98.0% → 100.0% (+1.97%) | 98.0% → 100.0% (+1.97%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
@copilot merge main |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 85.74% | 85.83% | 📈 +0.09% |
| Statements | 85.63% | 85.71% | 📈 +0.08% |
| Functions | 88.15% | 88.11% | 📉 -0.04% |
| Branches | 78.63% | 78.69% | 📈 +0.06% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/docker-manager.ts |
87.4% → 87.7% (+0.29%) | 87.0% → 87.3% (+0.27%) |
src/schema-validator.ts |
98.0% → 100.0% (+1.97%) | 98.0% → 100.0% (+1.97%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
…ort, function or class' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The branch was already up to date with main (branched from the latest main commit). Also took this opportunity to properly fix the outstanding CodeQL alerts by replacing the unused |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🔥 Smoke Test: Copilot BYOK — PASS
Running in BYOK offline mode ( Overall: PASS | Author: @lpcox | Reviewer:
|
|
Smoke Test Results:
Overall Status: PASS
|
🤖 Smoke Test Results
PR: fix: load schema via require() for pkg/esbuild compat — author: @lpcox, reviewer: Overall: PARTIAL — MCP ✅, pre-step outputs not injected into prompt.
|
Smoke Testfeat: validate config files against published JSON Schema at runtime Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Version Comparison
Result: Not all tests passed. Python and Node.js versions differ between host and chroot.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test: Services Connectivity
Overall: FAIL —
|
Problem
The Release workflow (run #25241289459) failed at the Smoke test binary (x64) step with:
The schema-based config validation (PR #2384) used
fs.readFileSyncwith__dirnamepaths to load the JSON Schema at runtime. This works fine fornode dist/cli.jsand the esbuild bundle, but fails in thepkgbinary because the JSON file wasn't included in the snapshot filesystem.Fix
Replace
fs.readFileSync+loadSchema()with a simplerequire('./awf-config-schema.json'). Both bundlers handle this natively:require()and includes the file in the snapshotAlso adds
dist/awf-config-schema.jsontopkg.assetsas belt-and-suspenders.Verification
npm run build✅npm run build:bundle✅