chore(dx): unify local and CI verify pipeline#36
Conversation
Align local and CI validation through npm run verify commands, wire CI to verify:ci, and update contributor-facing validation docs/templates. Co-authored-by: Codex <noreply@openai.com>
📝 WalkthroughWalkthroughpr consolidates verification steps into composite npm scripts ( Changes
notes requiring attention: missing explicit regression tests for runtime behavior of composed scripts ( Sequence Diagram(s)sequenceDiagram
participant developer as developer
participant repo as repo
participant ci as ci runner
participant npm as npm scripts
participant checks as verify steps
developer->>repo: open PR (references verify)
repo->>ci: trigger workflow (matrix.os)
ci->>npm: run "npm run verify:ci"
npm->>checks: run verify:repo (repo hygiene, audit, lockfile)
npm->>checks: run verify:quality (typecheck, coverage)
checks-->>ci: step results
ci-->>repo: report status (pass/fail)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/development/TESTING.md (1)
27-34:⚠️ Potential issue | 🟠 Majortesting docs are still partially out of sync with the verify workflow.
Line [27]-Line [34] keeps legacy component commands and does not map clearly to
verify:repo/verify:quality. Line [49]-Line [51] also needs an explicit migration/upgrade-note reminder for this workflow shift.as per coding guidelines, "docs/**: keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts."
ref:
lib/docs/development/TESTING.md:27-34,lib/docs/development/TESTING.md:49-51Also applies to: 49-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/TESTING.md` around lines 27 - 34, Update the TESTING.md section that lists legacy component commands to replace or map those npm scripts to the new verify scripts: remove or annotate the old "npm run typecheck / lint / test / build" block and show the new canonical commands "npm run verify:repo" and "npm run verify:quality" (and mention any flags they accept) so the docs match the CLI/workflow; additionally add an explicit migration/upgrade note in the later section (the block referenced around lines 49–51) describing the workflow shift and instructing users how to transition from the old component scripts to the new verify:repo/verify:quality scripts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/pull_request_template.md:
- Line 14: The PR template's triage checklist item "- [ ] if triaging failures,
validated component gates: `npm run lint`, `npm run typecheck`, `npm test`, `npm
run build`" is missing the verify step; update that checklist entry to include
the verify/repo hygiene commands by adding `npm run verify:repo` (or an explicit
sublist of the repo hygiene commands run by that script) so triage runners must
run the repo verification (audit/lockfile/hygiene) before marking the gate as
validated.
In @.github/workflows/ci.yml:
- Around line 31-32: The CI currently only runs the verify pipeline on ubuntu,
leaving Windows-specific fs edge cases untested; add a Windows runner to the
workflow by expanding the job matrix or adding a parallel job that uses runs-on:
windows-latest and executes the same step that runs "npm run verify:ci" (the
existing "Run CI verify pipeline" step) so the verify script is invoked on
Windows to catch ebusy/eperm and file-write race behaviors; ensure any
OS-specific environment setup or path normalization used elsewhere in the
workflow is duplicated for the Windows job so the verify script runs with
equivalent environment.
In `@CONTRIBUTING.md`:
- Around line 62-66: The failure-triage instructions under the "if triaging
failures, run component gates directly" section are incomplete—add the missing
repo-level checks so the local triage mirrors CI: include the repo-hygiene,
audit, and lockfile-floor commands (or replace the list with a single reference
to the higher-level scripts verify:repo or verify:quality) and update the bullet
list for failure-triage (or the heading where those commands appear) so it runs
the same repo-level gates used by CI.
In `@package.json`:
- Around line 64-67: Add a regression test that asserts the package.json
"scripts" contract (verify, verify:repo, verify:quality, verify:ci) remains
unchanged: update or create a test (e.g., in test/documentation.test.ts) that
reads lib/package.json and compares the scripts object entries for the keys
"verify", "verify:repo", "verify:quality", and "verify:ci" against the expected
command strings (from lines shown), failing if any link is altered so docs/CI
script wiring cannot silently desync. Ensure the test references the
package.json path and the exact script keys
("verify","verify:repo","verify:quality","verify:ci") so future edits trigger a
failing test.
---
Outside diff comments:
In `@docs/development/TESTING.md`:
- Around line 27-34: Update the TESTING.md section that lists legacy component
commands to replace or map those npm scripts to the new verify scripts: remove
or annotate the old "npm run typecheck / lint / test / build" block and show the
new canonical commands "npm run verify:repo" and "npm run verify:quality" (and
mention any flags they accept) so the docs match the CLI/workflow; additionally
add an explicit migration/upgrade note in the later section (the block
referenced around lines 49–51) describing the workflow shift and instructing
users how to transition from the old component scripts to the new
verify:repo/verify:quality scripts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
Run ID: 2f5abb7e-a6c0-4e72-ab52-cccab920d91d
📒 Files selected for processing (5)
.github/pull_request_template.md.github/workflows/ci.ymlCONTRIBUTING.mddocs/development/TESTING.mdpackage.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
⚙️ CodeRabbit configuration file
keep README, SECURITY, and docs consistent with actual CLI flags and workflows. whenever behavior changes, require updated upgrade notes and mention new npm scripts.
Files:
docs/development/TESTING.md
Co-authored-by: Codex <noreply@openai.com>
|
@coderabbitai review Re-triggering review to clear the transient rate-limit status check failure. |
|
✅ Actions performedReview triggered.
|
|
Re-requesting CodeRabbit review: the current failure is CodeRabbit: Review rate limit exceeded while other checks are green. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Follow-up tracking: external blocker recorded in #44. Current failing context remains:
Actions already taken:
Next required step is external quota reset/increase, then rerun CodeRabbit. |
|
✅ Actions performedReview triggered.
|
No code changes; this commit exists to re-run external CodeRabbit status checks that previously failed due provider rate limits.\n\nCo-authored-by: Codex <noreply@openai.com>
1 participant
Summary
What changed
Validation evidence
Risk and rollback
note: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
this pr unifies the local and CI verification pipeline by introducing four composed
verify:*scripts and routing the CI test matrix to a singlenpm run verify:cistep. it also addswindows-latest(Node 20.x) to the CI matrix, which is a meaningful improvement for a project that explicitly targets windows desktop environments.key observations:
verify/verify:cisplit is architecturally sound:verify:ciintentionally omits lint since the CIlintjob handles it separatelynpm run verifyandnpm run verify:cilocally, which is redundant —verifyis a strict superset ofverify:ci, so if the former passes, the latter is guaranteed to passci.ymlwiring despite the test name claiming it covers CI alignmentwindows-latestrunner addition is not accompanied by any documentation of Windows filesystem concurrency safety (EPERM/EBUSY handling) for the scripts invoked inverify:repo, which is required by project policy for every change touching the verification pipeline on windowsConfidence Score: 4/5
.github/pull_request_template.md(redundantverify:cistep) and.github/workflows/ci.yml(missing Windows concurrency safety documentation per project policy).Important Files Changed
npm run verify:ci; addswindows-latestrunner for Node 20.x — good platform coverage, but no documentation of Windows filesystem concurrency safety for the scripts invoked.verify:ciintentionally omits lint (handled by separate CI job); sequential chaining is safe for single-process runs.verifyandverify:ci, which is redundant locally sinceverifyis a strict superset; may confuse contributors about what they actually need to run.ci.ymlcontent despite the test name claiming CI wiring coverage.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Developer runs local gate] --> B[npm run verify] B --> C[npm run lint] B --> D[npm run verify:repo] B --> E[npm run verify:quality] D --> D1[clean:repo:check] D --> D2[audit:ci] D --> D3[test lockfile-version-floor.test.ts] E --> E1[typecheck] E --> E2[coverage = build + vitest --coverage] F[CI: test matrix job] --> G[npm run verify:ci] G --> D G --> E H[CI: lint job - ubuntu only] --> I[npm run lint] J[CI matrix] --> K[ubuntu-latest Node 20.x] J --> L[ubuntu-latest Node 22.x] J --> M[windows-latest Node 20.x] K --> G L --> G M --> G style B fill:#22c55e,color:#fff style G fill:#3b82f6,color:#fff style M fill:#f59e0b,color:#fffLast reviewed commit: 83a683e
Context used:
dashboard- What: Every code change must explain how it defends against Windows filesystem concurrency bugs and ... (source)