Skip to content

Conversation

@shuhuiluo
Copy link
Collaborator

@shuhuiluo shuhuiluo commented Nov 20, 2025

Summary by CodeRabbit

  • Chores
    • Added a CI workflow to run install, format check, lint, and typecheck on pushes and PRs.
    • Replaced previous Prettier settings with a consolidated Prettier configuration and added import-sorting and template plugins as dev dependencies.
    • Reorganized internal import/grouping across codebase for consistency and maintainability; minor formatting tweaks in tests.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 20, 2025

Walkthrough

Adds a CI GitHub Actions workflow that checks out code, sets up Bun, installs deps, runs format check, lint, and typecheck; removes .prettierrc and introduces prettier.config.js plus three new Prettier plugins in package.json; and applies widespread import reordering and small formatting edits across ~30 source and test files (including a type import addition in src/formatters/command-formatters.ts). No runtime logic or exported API signatures were changed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

Areas requiring extra attention:

  • .github/workflows/ci.yml — confirm triggers, Bun setup, and command flags (install, format:check, lint, typecheck).
  • prettier.config.js — validate plugin registration and importOrder entries match project conventions.
  • .prettierrc removal — ensure intentional migration and no conflicting Prettier configs remain.
  • package.json — review the three added Prettier plugins for compatibility.
  • src/formatters/command-formatters.ts — verify the added GitHubIssue type import is intentional and used.
  • src/db/index.ts and src/db/schema.ts — spot-check import source changes for drizzle/pg-core to ensure correct symbols are imported.
  • src/github-app/event-processor.ts — review consolidated type imports (TownsBot, SubscriptionService) for correctness.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the two primary changes: setting up a CI workflow (.github/workflows/ci.yml) and configuring import sorting (prettier.config.js with import order plugin, .prettierrc removal).
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/import-sorting-ci

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec0913e and 5b77ee8.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

1-31: LGTM! CI workflow is well-structured.

The workflow correctly sets up automated checks for formatting, linting, and type checking. The use of --frozen-lockfile ensures reproducible builds.

Consider adding dependency caching to speed up CI runs:

       - name: Setup Bun
         uses: oven-sh/setup-bun@v2
         with:
           bun-version-file: .bun-version
+
+      - name: Cache dependencies
+        uses: actions/cache@v4
+        with:
+          path: ~/.bun/install/cache
+          key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }}
+          restore-keys: |
+            ${{ runner.os }}-bun-
 
       - name: Install dependencies
         run: bun install --frozen-lockfile
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 439461e and ec0913e.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (33)
  • .github/workflows/ci.yml (1 hunks)
  • .prettierrc (0 hunks)
  • package.json (2 hunks)
  • prettier.config.js (1 hunks)
  • src/db/index.ts (1 hunks)
  • src/db/schema.ts (1 hunks)
  • src/formatters/command-formatters.ts (1 hunks)
  • src/formatters/events-api.ts (1 hunks)
  • src/formatters/webhook-events.ts (1 hunks)
  • src/github-app/app.ts (1 hunks)
  • src/github-app/event-processor.ts (1 hunks)
  • src/github-app/installation-service.ts (1 hunks)
  • src/github-app/webhook-processor.ts (1 hunks)
  • src/handlers/gh-issue-handler.ts (1 hunks)
  • src/handlers/gh-pr-handler.ts (1 hunks)
  • src/handlers/github-subscription-handler.ts (1 hunks)
  • src/index.ts (1 hunks)
  • src/routes/github-webhook.ts (1 hunks)
  • src/routes/oauth-callback.ts (1 hunks)
  • src/services/github-oauth-service.ts (1 hunks)
  • src/services/oauth-cleanup-service.ts (1 hunks)
  • src/services/polling-service.ts (1 hunks)
  • src/services/subscription-service.ts (1 hunks)
  • src/types/bot.ts (1 hunks)
  • src/utils/oauth-helpers.ts (1 hunks)
  • tests/fixtures/mock-bot-handler.ts (1 hunks)
  • tests/integration/gh-issue-integration.test.ts (1 hunks)
  • tests/integration/gh-pr-integration.test.ts (1 hunks)
  • tests/unit/formatters/events-api.test.ts (1 hunks)
  • tests/unit/handlers/gh-issue-handler.test.ts (1 hunks)
  • tests/unit/handlers/gh-pr-handler.test.ts (1 hunks)
  • tests/unit/handlers/github-subscription-handler.test.ts (1 hunks)
  • tests/unit/types/events-api.test.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • .prettierrc
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-18T23:35:49.436Z
Learnt from: shuhuiluo
Repo: HereNotThere/bot-github PR: 26
File: src/db/index.ts:61-69
Timestamp: 2025-11-18T23:35:49.436Z
Learning: In `webhook_deliveries` table (src/db/index.ts), the `installation_id` column should NOT have a FOREIGN KEY constraint because the table serves as an immutable audit log for idempotency tracking. Records must persist independently even after installations are deleted, and a foreign key would create race conditions when webhooks arrive before installation records are created. The field is intentionally nullable to support webhooks without installation context.

Applied to files:

  • src/github-app/installation-service.ts
  • src/github-app/webhook-processor.ts
🪛 ESLint
src/github-app/installation-service.ts

[error] 1-1: Unable to resolve path to module 'drizzle-orm'.

(import-x/no-unresolved)

src/github-app/webhook-processor.ts

[error] 1-1: Unable to resolve path to module 'drizzle-orm'.

(import-x/no-unresolved)

src/services/oauth-cleanup-service.ts

[error] 1-1: Unable to resolve path to module 'drizzle-orm'.

(import-x/no-unresolved)

src/db/schema.ts

[error] 1-1: Unable to resolve path to module 'drizzle-orm'.

(import-x/no-unresolved)


[error] 13-13: Unable to resolve path to module 'drizzle-orm/pg-core'.

(import-x/no-unresolved)

src/index.ts

[error] 3-3: Unable to resolve path to module '@towns-protocol/bot'.

(import-x/no-unresolved)

src/services/github-oauth-service.ts

[error] 8-8: Unable to resolve path to module '@octokit/rest'.

(import-x/no-unresolved)


[error] 9-9: Unable to resolve path to module 'drizzle-orm'.

(import-x/no-unresolved)

src/github-app/app.ts

[error] 3-3: Unable to resolve path to module '@octokit/webhooks'.

(import-x/no-unresolved)

src/services/subscription-service.ts

[error] 1-1: Unable to resolve path to module 'drizzle-orm'.

(import-x/no-unresolved)

src/db/index.ts

[error] 4-4: Unable to resolve path to module 'drizzle-orm/postgres-js'.

(import-x/no-unresolved)


[error] 5-5: Unable to resolve path to module 'drizzle-orm/postgres-js/migrator'.

(import-x/no-unresolved)


[error] 6-6: Unable to resolve path to module 'postgres'.

(import-x/no-unresolved)

🔇 Additional comments (31)
tests/integration/gh-issue-integration.test.ts (1)

1-2: Formatting-only change looks good

Adding a blank line after the bun:test import improves readability and is consistent with the new formatting rules; no behavioral impact.

src/utils/oauth-helpers.ts (1)

1-3: Import spacing is consistent and non-functional

The added blank line between type imports is stylistic only and aligns with the new import formatting; no runtime changes.

src/services/polling-service.ts (1)

1-3: Consistent import spacing; no behavior change

The extra blank line after the drizzle-orm import is a formatting-only tweak that aligns with the new import grouping; PollingService behavior is unchanged.

src/types/bot.ts (1)

1-3: Whitespace-only import tweak is fine

Separating the external SDK type import from the local commands import with a blank line is stylistic and has no effect on the exported types.

src/github-app/webhook-processor.ts (1)

1-4: Dependency verified; code changes are correct

drizzle-orm is properly installed as a dependency (^0.44.7). The import consolidation is correct and matches actual usage of eq in isProcessed and lt in cleanup. The ESLint import-x/no-unresolved warning is a local configuration issue, not a problem with the code itself.

src/services/oauth-cleanup-service.ts (1)

1-2: LGTM! Duplicate import removed.

The cleanup eliminates the duplicate lt import from drizzle-orm, improving code organization.

tests/fixtures/mock-bot-handler.ts (1)

2-2: LGTM!

Import reordering aligns with the project's import sorting configuration.

src/routes/oauth-callback.ts (1)

2-3: LGTM! Import cleanup.

The duplicate DEFAULT_EVENT_TYPES import has been removed, keeping a single import at the top.

src/db/index.ts (1)

3-6: LGTM! Improved import organization.

The imports are now properly grouped with Node.js built-ins separated from third-party packages.

src/handlers/gh-issue-handler.ts (1)

1-17: LGTM! Well-organized imports.

The imports are now properly grouped and classifyApiError is explicitly imported, improving code clarity.

src/formatters/webhook-events.ts (1)

6-16: LGTM! Complete type imports.

The addition of ForkPayload and IssueCommentPayload ensures all webhook types used in the file are properly imported.

src/services/github-oauth-service.ts (1)

1-13: LGTM! Consolidated crypto imports.

All crypto-related imports are now consolidated in a single block, and imports are properly grouped by package.

src/db/schema.ts (1)

1-14: LGTM! Explicit and organized imports.

The sql import is now separated, and all pg-core exports are explicitly listed, making the module's dependencies clear.

src/formatters/command-formatters.ts (1)

6-11: Import type set and ordering look good

The GitHub types imported here match the usages below and the reordered list keeps things tidy and consistent with the new import-sorting rules.

tests/unit/types/events-api.test.ts (1)

1-4: Import grouping / spacing is consistent

Adding a blank line between the external bun:test import and internal module import matches common style and doesn’t affect behavior.

src/handlers/github-subscription-handler.ts (1)

1-10: Clean separation of type and value imports

Grouping the type-only imports and then importing stripMarkdown as a value keeps this header consistent with the new formatting rules without affecting behavior.

src/routes/github-webhook.ts (1)

1-5: Import spacing aligns with style changes

The added blank line between the external hono import and local type imports is a style-only change and keeps imports well grouped.

src/github-app/app.ts (1)

1-4: Dependency verified—no issue found

@octokit/webhooks is correctly listed in dependencies (version ^14.1.3) and will resolve normally. The import on line 3 is valid.

tests/integration/gh-pr-integration.test.ts (1)

1-2: LGTM!

Blank line addition after imports aligns with the new formatting conventions.

package.json (1)

36-50: LGTM!

The three new Prettier plugins are appropriate for the import sorting and formatting improvements described in the PR objectives.

tests/unit/handlers/github-subscription-handler.test.ts (1)

1-6: LGTM!

Import reorganization follows the new formatting conventions established by the Prettier plugins.

src/formatters/events-api.ts (1)

11-12: LGTM!

Import order adjustment aligns with the new import sorting configuration.

tests/unit/handlers/gh-pr-handler.test.ts (1)

1-11: LGTM!

Import reorganization is consistent with the new formatting conventions.

src/services/subscription-service.ts (1)

1-6: Import reorganization looks good.

The explicit imports from drizzle-orm and reordering are appropriate. The ESLint error about unresolved path is likely a false positive since drizzle-orm is present in package.json dependencies. However, verify that ESLint's import resolver is configured correctly to avoid similar warnings.

tests/unit/formatters/events-api.test.ts (1)

1-18: LGTM!

Import additions and reordering align with the new formatting configuration.

src/handlers/gh-pr-handler.ts (1)

1-14: LGTM!

Import reorganization improves grouping and readability while following the new formatting conventions.

tests/unit/handlers/gh-issue-handler.test.ts (1)

1-12: LGTM! Import reorganization aligns with the new formatting rules.

The imports have been properly grouped and sorted according to the new Prettier configuration.

src/index.ts (1)

1-21: LGTM! Import reorganization follows the new formatting conventions.

The imports have been properly grouped with third-party modules (hono, @towns-protocol/bot) separated from local imports.

Note: The ESLint error about unresolved module @towns-protocol/bot is a false positive—this is an external dependency.

src/github-app/installation-service.ts (1)

1-5: LGTM! Import consolidation improves readability.

The drizzle-orm imports are now properly consolidated, and the type-only import for SubscriptionService is correctly used.

Note: The ESLint error about unresolved module drizzle-orm is a false positive—this is an external dependency.

src/github-app/event-processor.ts (1)

1-26: LGTM! Import organization is clean and consistent.

The formatter imports are properly ordered, and type-only imports are correctly separated for better tree-shaking.

prettier.config.js (1)

1-23: LGTM! Prettier configuration is well-structured.

The import order configuration provides clear grouping:

  1. Node built-ins (^node:)
  2. Third-party modules
  3. Towns Protocol modules (^@towns-protocol/(.*)$)
  4. Path aliases (^@/)
  5. Relative imports (^[./])

This hierarchy makes sense and will keep imports organized consistently across the codebase.

@shuhuiluo shuhuiluo merged commit dc88b3a into main Nov 20, 2025
2 checks passed
@shuhuiluo shuhuiluo deleted the chore/import-sorting-ci branch November 20, 2025 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants