refactor: consolidate services into @thumbcode/core, remove 2.7k LOC dead code #68
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add experiments.baseUrl: "/thumbcode" for GitHub Pages subdirectory - Inject no-cache meta tags into HTML files for staging environment - Fixes Expo app loading issue on GitHub Pages deployment Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
## Changes ### Deployment Infrastructure - Add render.yaml for Render.com staging deployment - Configure security headers (X-Frame-Options, X-Content-Type-Options) - Set up SPA routing with proper cache headers - Remove baseUrl from app.json (Render serves from root) ### Version Management (DRY) - Add .nvmrc with Node 22 LTS - Add packageManager field to package.json (pnpm@10.11.0) - Update setup-thumbcode action to read from .nvmrc - Remove hardcoded version numbers from workflows ### Workflow Refactoring - Refactor ci.yml with latest action SHAs and E2E job - Create cd.yml for EAS builds and deployments - Update deploy-gh-pages.yml for documentation only - Pin all GitHub Actions to latest SHAs: - checkout@v6.0.2 - setup-node@v6 - pnpm/action-setup@v4 - expo/expo-github-action@v8 - codecov-action@v5 - coverallsapp@v2 - sonarcloud-github-action@v5 ### CI/CD Structure - ci.yml: lint, typecheck, test, build, e2e - cd.yml: EAS updates, Android/iOS builds, store submissions - deploy-gh-pages.yml: documentation with TypeDoc Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
## Fixes from code review ### Critical - Fix EXPO_TOKEN secret check to use env block pattern (prevents secret leakage) ### Major - Add pull_request trigger to cd.yml so eas-update job will run - Add path filters to conserve EAS build minutes (src/**, app.json, eas.json) - Add typedoc and typedoc-plugin-markdown to devDependencies - Fix render.yaml: add corepack enable for packageManager support - Remove runtime typedoc installation in deploy-gh-pages.yml ### Minor - Pin exact Node version in .nvmrc (22.12.0) - Add HSTS header (Strict-Transport-Security) - Add Permissions-Policy header for additional security Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…precedence CodeRabbit review fixes: - Add EXPO_ANDROID_SERVICE_ACCOUNT_KEY_PATH env var to submit-android job matching the iOS submission pattern - Reorder render.yaml Cache-Control headers: specific paths (/assets/*, /_expo/*) now come before general /* to avoid nondeterministic matching Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove submit-android and submit-ios jobs (see issue #66) - Fix typedoc-plugin-markdown version (^5.0.0 → ^4.9.0) - Update pnpm-lock.yaml with corrected dependencies Current release strategy: - Web staging: Render.com (automatic via render.yaml) - Mobile builds: Download from expo.dev after EAS build - App store submissions: Pending credential configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use step outputs and conditionals to skip subsequent steps when EXPO_TOKEN is not configured, instead of just exiting the check step with code 0 (which doesn't stop subsequent steps). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Dependency changes should trigger EAS Update PR previews to keep previews in sync with package changes. Addresses CodeRabbit review comment. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- development-builds.yml: PR builds for Android + iOS simulator - production-deploy.yml: Smart deployment with fingerprinting - Skips rebuilds when native code unchanged (OTA updates) - Parallel iOS/Android builds when needed - preview-update.yml: OTA updates for feature branches Enables full mobile CI/CD pipeline without GitHub Actions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added AI coding assistant market analysis ($4.91B → $30.1B market) - Documented competitive landscape (Copilot, Cursor, Claude, etc.) - Identified ThumbCode's unique positioning (mobile-first + BYOK) - Created phased 1.0 execution roadmap - Added 2026 market trends analysis Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
BREAKING CHANGE: src/services/git and src/services/credentials removed
## Changes
- Delete duplicate src/services/git/ (use @thumbcode/core instead)
- Delete duplicate src/services/credentials/ (use @thumbcode/core instead)
- Update src/services/index.ts to re-export from @thumbcode/core
- Fix packages/core type errors:
- Add validateAnthropicKey, validateGitHubToken exports
- Fix GitHttpClient forEach type annotations
- Fix GitService HttpClient type compatibility
## Migration
Import from @thumbcode/core for Git and Credential services:
```typescript
// Before
import { GitService } from '@/services/git';
// After
import { GitService } from '@thumbcode/core';
```
## Rationale
- src/services/ had dead code that duplicated packages/core
- Consolidation ensures single source of truth
- Improves maintainability and reduces confusion
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe PR migrates credential and git service implementations from local src/services to packages/core, adds three new validation helper functions for keys and tokens, expands GitHub token format support, and updates the public API to re-export these entities from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Incorporates EAS project configuration from PR #65
|
🚀 Expo preview is ready!
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/credentials/validation.ts`:
- Around line 10-13: The GitHubTokenSchema regex is missing the ghu_ prefix and
inconsistently restricts characters (disallows underscores) for several token
types; update the regex in the GitHubTokenSchema declaration so all token bodies
use the same character set [A-Za-z0-9_] and include the ghu_ alternative (e.g.
use something like
/^(ghp_[A-Za-z0-9_]{36}|github_pat_[A-Za-z0-9_]+|gho_[A-Za-z0-9_]+|ghs_[A-Za-z0-9_]+|ghr_[A-Za-z0-9_]+|ghu_[A-Za-z0-9_]+)$/)
to replace the current pattern.
🧹 Nitpick comments (1)
packages/core/src/credentials/validation.ts (1)
18-34: Normalize input before validation.Trimming pasted keys avoids false negatives due to whitespace.
♻️ Suggested change
export function validateAnthropicKey(value: string): boolean { - return AnthropicKeySchema.safeParse(value).success; + return AnthropicKeySchema.safeParse(value.trim()).success; } @@ export function validateOpenAIKey(value: string): boolean { - return OpenAIKeySchema.safeParse(value).success; + return OpenAIKeySchema.safeParse(value.trim()).success; } @@ export function validateGitHubToken(value: string): boolean { - return GitHubTokenSchema.safeParse(value).success; + return GitHubTokenSchema.safeParse(value.trim()).success; }
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates Git and Credential services into @thumbcode/core, removing 2,699 lines of duplicate code from src/services/. It establishes clear package boundaries where @thumbcode/core contains shared services and src/ contains app-specific code. The refactor also fixes TypeScript type compatibility issues in the core package.
Changes:
- Removed duplicate Git and Credential service implementations from
src/services/ - Updated
src/services/index.tsto re-export from@thumbcode/corefor backward compatibility - Fixed TypeScript type annotations in
@thumbcode/corefor Git HTTP client and service - Enhanced GitHub token validation to support multiple token formats (classic, fine-grained, OAuth, installation)
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/services/index.ts |
Re-exports core services from @thumbcode/core with backward compatibility |
src/services/git/types.ts |
Deleted - duplicate type definitions moved to core |
src/services/git/index.ts |
Deleted - duplicate exports moved to core |
src/services/git/__tests__/GitService.test.ts |
Deleted - tests consolidated in core package |
src/services/git/GitService.ts |
Deleted - implementation consolidated in core |
src/services/credentials/index.ts |
Deleted - duplicate exports moved to core |
src/services/credentials/__tests__/CredentialService.test.ts |
Deleted - tests consolidated in core package |
src/services/credentials/CredentialService.ts |
Deleted - implementation consolidated in core |
packages/core/src/git/GitService.ts |
Fixed type compatibility with HttpClient interface |
packages/core/src/git/GitHttpClient.ts |
Added explicit type annotations for forEach callback |
packages/core/src/credentials/validation.ts |
Enhanced GitHub token validation and exported individual validation functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Major codebase cleanup that removes duplicate code and establishes clear package boundaries.
src/services/@thumbcode/coreChanges
src/services/git/src/services/credentials/src/services/index.tspackages/core/src/credentials/validation.tsvalidateAnthropicKey,validateGitHubTokenexportspackages/core/src/git/GitHttpClient.tspackages/core/src/git/GitService.tsWhy This Matters
@thumbcode/corefor shared services,src/for app-specific codeMigration
Test plan
pnpm typecheck)pnpm test)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Breaking Changes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.