ci: add PR-title commitlint as standalone workflow#294
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 18 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
## Why develop's lint job has been failing since **2026-04-24** (over a month). Until this lands, **every Phase 0 PR (#290–#294) is structurally unmergeable** because branch protection requires \`lint\` to pass and \`strict: true\` requires PRs to match develop's tip. This is the unblock for the whole post-audit release stack. ## What Two orthogonal fixes that together get \`pnpm lint\` to **0 errors**. ### 1. \`preserve-caught-error\` × 4 in \`encryptionService.ts\` Four \`try/catch\` blocks re-throw a wrapped error without attaching the caught one: \`\`\`ts } catch (error) { throw new Error( \`Failed to encrypt content: \${error instanceof Error ? error.message : 'Unknown error'}\`, + { cause: error } // ← lints clean and preserves the stack ); } \`\`\` Affected throw sites: \`initialize\` (114), \`encrypt\` (261), \`decrypt\` (292), \`importKey\` (351). The \`{ cause }\` payload is the standard ES2022 way to chain errors; runtime semantics unchanged. ### 2. mcp-server tsconfig refactor for ESLint projectService \`packages/mcp-server/tsconfig.json\` was excluding \`src/__tests__\`. ESLint uses \`@typescript-eslint/parser\` with \`projectService: true\`, which delegates project discovery to the TypeScript LSP. The LSP walked up from the test file, found mcp-server/tsconfig.json with the explicit exclude, and rejected the test → **parsing error: \"was not found by the project service\"**. Fix: split build vs editor configs. - **tsconfig.json** — single source of truth for editors/lint/test. Includes everything under \`src\`. Adds \`vitest/globals\` to \`types\`. - **tsconfig.build.json** — extends tsconfig.json, re-adds \`exclude: [\"src/__tests__\"]\`. Used by the build script. - **package.json** — \`\"build\": \"tsc\"\` → \`\"build\": \"tsc -p tsconfig.build.json\"\`. Confirmed locally: - \`pnpm lint\` → 0 errors (39 warnings unchanged, all pre-existing import-x/order). - \`pnpm --filter @readied/mcp-server build\` succeeds; \`dist/__tests__/\` does not exist. - \`pnpm --filter @readied/mcp-server test\` → 5/5 pass. - \`pnpm -r typecheck\` succeeds. ## Why a separate PR (not bundled with #290 / A1) A1 is the Electron-pin commit. Mixing in a multi-file lint fix would muddy what's a release-pipeline change vs a code-hygiene change. Keeping this separate also means: this PR can go in first, then #290–#294 can rebase one by one and pass CI cleanly. ## Roadmap status - [ ] **this PR** — lint baseline unblock - [ ] #290 A1 (electron 41.7.1) - [ ] #291 A2 (bump-version.mjs) - [ ] #292 B (workflow surface) - [ ] #293 A4 (release guardrails) - [ ] #294 C1 (pr-title commitlint) - [ ] C2 follow-up — add \`commitlint\` to required checks after #294 lands - [ ] D — cut v0.15.1
Pulls the conventional-commit check out of ci.yml's lint job into its own workflow (.github/workflows/pr-title.yml) so it exposes a stable, isolated status-check name that branch protection on develop and main can require independently. Why this matters: the repo enforces squash-merge, so the PR title becomes the commit message on develop/main. semantic-release then reads that message through commitlint.config.js's type-enum to decide whether to cut a release. PR #245 ("release: audit...") squashed to a non-conventional commit, semantic-release silently exited with "no release", and that's the trap that produced the v0.15.0 stale-version tag. Blocking non-conventional titles at the merge gate stops it recurring. The new workflow uses the env-var pattern for github.event.pull_request.title (never interpolating it directly into a run: command) and printf '%s' instead of echo to avoid PR titles starting with -e/-n being interpreted as echo flags. Co-required with the upcoming branch-protection update (Phase 0 C2) which will mark PR title / commitlint as required.
1748860 to
0d84fe9
Compare
…postinstall in setup (#296) ## Why Two distinct CI issues, both blocking every Phase 0 PR (#290, #292, #294). Bundling them is OK because they're orthogonal-but-related: both clear a "lint-or-setup says no, so I can't merge" path on develop. ### Issue 1: Prettier fails on \`CHANGELOG.md\` semantic-release writes CHANGELOG entries without prettier formatting. The root \`format:check\` script uses \`--ignore-path .gitignore\`, which **overrides** Prettier's default \`.prettierignore\` lookup. CHANGELOG.md correctly isn't gitignored (it's tracked), so it gets linted, fails, kills lint. ### Issue 2: \`setup\` job fails when native deps don't match the host Electron \`setup\` runs \`pnpm install --frozen-lockfile\` (no \`--ignore-scripts\`). That triggers apps/desktop's \`electron-builder install-app-deps\` postinstall, which **rebuilds better-sqlite3 from source against Electron's bundled Node headers**. When better-sqlite3 lags an Electron major (the v0.15.0 incident: Electron 42 + better-sqlite3 12.10.0, V8 \`External::Value\` signature mismatch), the rebuild fails and setup dies — taking lint/test/typecheck/build down with it. The same shape took down deploy-api.yml (#287) and release.yml (#288). This brings ci.yml in line. ## What changes - **\`.prettierignore\`** (new) — CHANGELOG.md + local build artefacts (.next/, .source/, .astro/, .wrangler/, dist/, out/, release/, coverage/, pnpm-lock.yaml). - **\`package.json\`** — \`format\` and \`format:check\` now pass \`--ignore-path .gitignore --ignore-path .prettierignore\` (Prettier 3.x supports repeated \`--ignore-path\`). - **\`.github/workflows/ci.yml\`** — \`setup\` job install: \`--ignore-scripts\` added with explanatory comment. ## Verification - \`pnpm format:check\` locally → "All matched files use Prettier code style!" - CI doesn't need a runtime-functional better-sqlite3: lint and typecheck don't load native modules, and \`pnpm test\` excludes storage-sqlite per CLAUDE.md. ## Order of operations After this lands → rebase #290 / #292 / #294 → CI green → merge them in order → cut v0.15.1.
## Release v0.15.1 — Phase 0 DevOps stabilization Brings the 7-PR DevOps cleanup chain to main and cuts a clean release. This is the verification gate for the whole Phase 0 effort — if anything breaks at tag, build, or publish, Phase 0 isn't done. ### What landed since v0.15.0 | PR | Phase | Summary | |----|-------|---------| | #290 | **A1** | \`fix(desktop)\`: pin Electron to ^41.7.1 so better-sqlite3 prebuilts apply (closes the v0.15.0 V8 ABI failure on all 3 build platforms) | | #291 | **A2** | \`fix(release)\`: restore version bumping via \`scripts/bump-version.mjs\` + \`@semantic-release/exec\` (closes the "tag at 0.14.0" trap) | | #292 | **B** | \`chore(ci)\`: workflow surface cleanup — actions @v4→@v5 sweep, \`windows-latest\` → \`windows-2025-vs2026\` pin, drop \`FORCE_JAVASCRIPT_ACTIONS_TO_NODE24\`, \`if-no-files-found: error\`, \`permissions:\` blocks, HUSKY: '0' removal | | #293 | **A4** | \`chore(ci)\`: \`release.yml\` pre-flight dry-run gate + post-flight version assertion (closes the "silent no-release" trap) | | #294 | **C1** | \`ci\`: PR-title commitlint as a standalone workflow → required check on develop + main | | #295 | | \`fix(lint)\`: develop lint baseline (preserve-caught-error × 4 in encryptionService + mcp-server tsconfig split for ESLint projectService) | | #296 | | \`chore(ci)\`: unblock CI on develop — ignore CHANGELOG.md in Prettier (semantic-release writes it), \`pnpm install --ignore-scripts\` in setup job (same shape as release.yml + deploy-api.yml) | ### C2 — branch protection updates (already applied via gh api) Both \`develop\` and \`main\`: - **Required status checks**: \`lint\`, \`test\`, \`typecheck\`, \`CodeRabbit\`, \`commitlint\` - Force-pushes blocked - \`strict: true\` (PRs must be up to date) ### Release pipeline guardrails now in place - **Pre-merge**: PR-title commitlint blocks \`release:\`-style non-conventional squash titles upstream. - **Mid-release**: \`release.yml\` dry-run check fails loud if no release would be cut. \`scripts/bump-version.mjs\` mutates both \`package.json\` files. Post-flight assertion verifies both match the dry-run-announced version. - **Post-release**: \`build.yml\` artifact upload uses \`if-no-files-found: error\` (silent zero-asset releases die at upload). - **Native deps**: \`apps/desktop\` pinned to Electron 41.7.1 with prebuilt better-sqlite3. CI \`setup\` skips postinstall so workflow-side install never rebuilds native modules. ### Expected behavior of the Release pipeline after merge 1. Merge this PR → main tip advances. 2. Manually dispatch the **Release** workflow. 3. \`release.yml\` runs: - \`pnpm install --ignore-scripts\` (no native rebuild needed for semantic-release). - **Pre-flight dry-run** → "next release version is 0.15.1" (single \`fix(release):\` commit since v0.15.0). - \`npx semantic-release\`: - \`@semantic-release/exec\` runs \`node scripts/bump-version.mjs 0.15.1\` → both package.json files updated. - \`@semantic-release/git\` commits + pushes tag \`v0.15.1\`. - \`@semantic-release/github\` creates draft Release. - **Post-flight assertion** → both package.jsons read \`0.15.1\`. 4. Tag push triggers \`build.yml\` on macOS-14, windows-2025-vs2026, ubuntu-latest. 5. All 3 platforms succeed → publish job un-drafts the GitHub Release. 6. Auto-sync PR opens to merge main → develop. ### What still needs verification (post-release) - [ ] Tag push actually triggers Build (needs GH_TOKEN with workflow scope — A3 deferred, may need PAT regen) - [ ] Build completes on all 3 platforms with prebuilt better-sqlite3 (smoke-test desktop bundle after publish) - [ ] Auto-sync PR back to develop is created 🤖 This is the Phase 0 verification gate. Mobile + Plugin Marketplace UI remain deferred. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added PR title validation workflow for automated commit message compliance checks. * **Bug Fixes** * Enhanced error diagnostics in encryption operations. * Added pre-flight checks to release process to prevent failed deployments. * Stricter artifact validation in builds. * **Chores** * Updated GitHub Actions to latest stable versions. * Improved code formatting configuration and build scripts. * Adjusted Electron dependency version. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Why
Phase 0 C1 of the DevOps cleanup roadmap. After v0.15.0 traced back to PR #245's squash-merge producing a non-conventional commit message (
release: audit...) which semantic-release silently rejected, the merge gate needs to block non-conventional PR titles upstream of merge — not just whine in CI.The check already existed as a step inside
ci.yml'slintjob, but it shipped as a sub-step of a multi-purpose job. Branch protection can only require whole status checks, so requiring `lint` would also block on ESLint/Prettier failures. Pulling commitlint into its own workflow gives branch protection a clean, single-responsibility check name to require: `PR title / commitlint`.What changes
Security shape
The workflow follows the GitHub command-injection guidance:
Verification
Follow-ups (Phase 0 C2)
Once this merges, C2 sets `PR title / commitlint` as a required status check on `develop` and `main` via `gh api` — that's the step that actually blocks #245-style merges. C1 is the pre-req: required checks must exist on the default branch before they can be required.
Roadmap status