refactor: clear raw-regex and process.env backlogs#2313
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Coverage Report for Expo Unit Tests Coverage (./apps/expo)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Deploying packrat-guides with
|
| Latest commit: |
d443422
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://abfea625.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://feat-checks-backlog-2.packrat-guides-6gq.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | d443422 | Commit Preview URL Branch Preview URL |
Apr 26 2026, 09:35 PM |
There was a problem hiding this comment.
Pull request overview
Refactors the monorepo to eliminate remaining no-raw-regex and no-raw-process-env custom-lint backlogs, then promotes both checks to hard gates in CI and the pre-push hook. This includes hoisting inline regex literals out of string method calls and introducing a typed Zod-based env shim for the Admin Next.js app.
Changes:
- Hoist inline regex literals used in string method calls into named top-level constants across multiple packages/apps.
- Introduce
apps/admin/lib/env.ts(Zod-parsed env shim) and migrate Admin callers off rawprocess.env. - Harden enforcement: add new checks to pre-push and remove
continue-on-errorforlint:customin CI; expandno-raw-process-envallowlist for legitimate script/test usages.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-ui/src/components/chart.tsx | Hoists colon-stripping regex into a constant for chart ID normalization. |
| packages/mcp/src/tools/trips.ts | Hoists UUID hyphen-stripping regex into a constant. |
| packages/mcp/src/tools/trail-conditions.ts | Hoists UUID hyphen-stripping regex into a constant. |
| packages/mcp/src/tools/packs.ts | Hoists UUID hyphen-stripping regex into a constant for pack/item IDs. |
| packages/env/scripts/no-raw-process-env.ts | Expands allowlist for legitimate process.env usage sites and test directories. |
| packages/api/test/setup.ts | Hoists markdown extension regex used in replace into a constant. |
| packages/api/src/utils/format-ai-response.ts | Hoists formatting regex literals into top-level constants. |
| packages/api/src/utils/csv-utils.ts | Hoists many sanitization regexes into constants and updates string normalization pipeline. |
| packages/api/src/services/r2-bucket.ts | Hoists ETag quote-stripping regex into a constant. |
| packages/api/src/services/executeSqlAiTool.ts | Hoists join keyword regex into a constant for complexity validation. |
| packages/api/src/services/embeddingService.ts | Hoists newline replacement regex into a constant for embedding normalization. |
| packages/api/src/routes/wildlife/index.ts | Hoists slug normalization regexes into constants. |
| packages/api/src/routes/trailConditions/reports.ts | Hoists LIKE-escape regexes into constants for query input escaping. |
| packages/api/src/routes/packTemplates/index.ts | Hoists UUID hyphen-stripping regex into a constant for template/item IDs. |
| packages/api/src/routes/knowledgeBase/reader.ts | Hoists HTML→Markdown conversion regexes into constants. |
| packages/analytics/src/core/entity-resolver.ts | Hoists normalization regexes into constants and simplifies brand normalization. |
| packages/analytics/src/core/data-export.ts | Hoists timestamp filename-sanitization regex into a constant. |
| lefthook.yml | Adds no-raw-regex + no-raw-process-env to the pre-push hard gate list. |
| apps/guides/scripts/enhance-content.ts | Hoists timestamp regex into a constant and replaces filename filtering regex with includes. |
| apps/expo/utils/format-ai-response.ts | Hoists formatting regex literals into top-level constants. |
| apps/admin/lib/env.ts | Adds Admin env shim: Zod-parse process.env once and export typed env. |
| apps/admin/lib/api.ts | Switches API base URL sourcing from raw process.env to adminEnv. |
| apps/admin/app/login/page.tsx | Switches API base URL sourcing from raw process.env to adminEnv. |
| .github/workflows/checks.yml | Makes lint:custom a hard CI gate (removes continue-on-error). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ── Script regex constants ── | ||
| const TIMESTAMP_SAFE_CHARS = /[:.]/g; | ||
|
|
There was a problem hiding this comment.
TIMESTAMP_SAFE_CHARS is named as if it matches characters that are safe to keep, but the pattern /[:.]/g matches characters that are typically unsafe in filenames and are being replaced. Renaming this to something like TIMESTAMP_UNSAFE_CHARS (or similar) would better reflect its purpose and align with the naming used elsewhere (e.g., TIMESTAMP_UNSAFE_CHARS in analytics).
| // Normalize smart/special quotes to standard quotes | ||
| .replace(/[‘’‛‹›]/g, "'") | ||
| .replace(/[“”„‟«»]/g, '"') | ||
| .replace(/[`]/g, '') | ||
| .replace(CURLY_SINGLE_QUOTES, '’') | ||
| .replace(CURLY_DOUBLE_QUOTES, '”') | ||
| .replace(BACKTICK_CHARS, '') |
There was a problem hiding this comment.
normalizeJsonString is supposed to normalize smart/special quotes to standard quotes, but it currently replaces them with curly right quotes (’/”). That will break downstream JSON.parse (and also conflicts with the existing csv-utils tests that expect straight quotes). Additionally, the CURLY_SINGLE_QUOTES / CURLY_DOUBLE_QUOTES character classes look incorrect (they include straight quotes instead of the actual curly quote codepoints), so many smart quotes won't be normalized at all. Suggest changing the replacements to straight quotes (' and ") and updating the regex character classes to match the intended curly quote characters (e.g. ‘ ’ “ ” etc.).
20539af to
a0f05f6
Compare
Coverage Report for API Unit Tests Coverage (./packages/api)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
210093b to
b70286f
Compare
a0f05f6 to
2fe85f1
Compare
Adds 5 entries to the allowlist (the check script itself, api startup validator, analytics env module, guides sync script, api test directory) and introduces apps/admin/lib/env.ts so NEXT_PUBLIC_API_URL is typed via Zod rather than read raw.
format-ai-response (expo + api), csv-utils, enhance-content, chart.tsx, mcp tools, api test setup — 40 patterns extracted. Regex now compiles once and is self-documenting at the call site.
reader.ts (29 HTML→Markdown patterns), trailConditions, wildlife, packTemplates, r2-bucket, embeddingService, executeSqlAiTool, entity-resolver, data-export — 44 patterns extracted. no-raw-regex now passes clean (0 violations).
Both checks now pass clean — move them from continue-on-error CI and add them to the pre-push gate. Only check-type-casts remains in the backlog (130 casts, next PR).
- Rename TIMESTAMP_SAFE_CHARS → TIMESTAMP_UNSAFE_CHARS (matches its purpose) - Fix CURLY_SINGLE_QUOTES: add U+2018/U+2019 which were missing - Fix CURLY_DOUBLE_QUOTES: add U+201C/U+201D which were missing - Fix normalizeJsonString: replace with straight quotes (was replacing with smart quotes U+2019/U+201D, which would still break JSON.parse)
2fe85f1 to
d443422
Compare
Chained off #2312. Clears two more check categories and promotes them to hard gates.
Summary
no-raw-regexnow blocks at pre-push.process.envviolations — 5 legitimate uses added to the allowlist (the check script itself, analytics env module, api scripts, guides sync);apps/admingets a proper Zod-parsed env shim (apps/admin/lib/env.ts) mirroring the pattern frompackages/env/src/next.ts.lint:custom(no-raw-typeof+no-raw-regex+no-raw-process-env) now runs withoutcontinue-on-error. Onlycheck-type-castsremains soft-gated (130 casts, next PR).no-raw-regexandno-raw-process-envadded alongside the other clean checks.Commits
no-raw-regex+no-raw-process-envto hard gatesTest Plan
bun scripts/lint/no-raw-regex.ts→ 0 violationsbun packages/env/scripts/no-raw-process-env.ts→ 0 violationsbun lint:custompasses clean (all 3 checks)bun check:all— onlycheck-type-castsandcheck-react-doctorstill failcsv-utils.ts— 15 named regex constants grouped at top of filereader.ts— 29 HTML→Markdown patterns at top, chain reads cleanly