Uh oh!
There was an error while loading. Please reload this page.
[INT-3686] Overhaul CI/CD and quality assurance - #127
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
1 issue found across 15 files
Confidence score: 4/5
- In
.github/workflows/ci.yml, not declaring an explicitpermissionsblock leavesGITHUB_TOKENat repository defaults, which can grant broader write access than intended and increase CI supply-chain blast radius if a job is compromised — add a top-level least-privilegepermissionspolicy (and job-level overrides only where needed) before merging.
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Add a top-level permissions block (contents: read) to ci.yml so all jobs default to read-only access, instead of inheriting the repository's default GITHUB_TOKEN permissions. Addresses CodeQL alerts (one per job) and the cubic-dev-ai review finding on PR #127. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. PR includes significant source code refactors (removing try-catch, restructuring imports) alongside CI and tooling changes. These modifications to business logic files require human review to ensure no regressions.
Re-trigger cubic
izzyandrade
left a comment
There was a problem hiding this comment.
LGTM, AI code review also caught nothing.
Uh oh!
There was an error while loading. Please reload this page.
Overview
Overhauls the project's quality assurance and CI setup. Before this change the test suite, library import, and dependency lockfile were all broken at
HEAD(masked because CI only ran at release time on a pinned Node 20). This PR restores a working, gated baseline and adds coverage, lint enforcement, and PR-level CI.Why
A QA audit found three compounding breakages at
HEAD:npm testimportpackage-lock.jsonnpm ci/npm auditbrokeChanges
CI / tooling
.github/workflows/ci.ymlrunning onpull_request+ push tomaster:testacross a Node 20 / 22 / 24 matrixlint(blocking)coverage(c8, enforces thresholds, uploadslcov.info)audit(npm audit --audit-level=high)actions/checkout@v4+setup-node@v4with npm caching.c8,npm run coverage, and.c8rc.jsonthresholds.Fixes
lib/onfleet.js: replaced the deprecatedassert { type: 'json' }import attribute (removed in Node 22, also unparseable by ESLint 8) with a portablecreateRequire('../package.json'). Restores Node 22/24 support.package-lock.json: repaired invalid JSON (a missing},) sonpm ci/npm auditwork again.package.jsonoverrides: scoped the blanketbrace-expansionoverride per-major (@1/@2/@3/@4) so modernminimatch@10(needsbrace-expansion@^5) installs, while bumping the pins to^1.1.13/^2.0.3— which also closes a newer advisory (GHSA-f886-m6hf-6m8v) the original^2.0.2pin was exposed to.Lint
.eslintrc.json:env: node/es2022, atest/**override withenv: mocha, correctedimport/extensionsfor this ESM project (requires.js), andprefer-destructuring: {array:false}.Methods.js/onfleet.js(removed a useless try/catch, renamed a shadowed var,for…of→forEach). All behavior-preserving.npm run lintnow exits 0 and is a required CI check.Tests
x-ratelimit-remainingin mocks (mirrors the real API).Verification
npm run lintnpm testnpm run coveragenpm cinpm audit --audit-level=highCoverage: 94.1% lines/statements, 91.9% branches, 74.4% functions — all resource files at 100%.
Residual (non-blocking)
npm auditreports 1 moderate (js-yaml, via eslint/mocha — dev-only, never shipped). Its only fix is js-yaml 5.x (a breaking major those tools don't support), so it's deferred to an upstream eslint/mocha bump. Dependabot tracks it; the--audit-level=highgate stays green.Follow-up (admin, after merge)
Enable branch protection on
masterrequiring the new checks (Test (Node 20/22/24),Lint,Coverage,Security audit) — a gate is only as strong as its enforcement.