[2witstudios]

Summary

• Builtin provider configs (like githubProvider) are stored in the DB at first seed and never updated — new tools added in code deployments don't reach agents
• Adds refreshBuiltinProviders() that compares in-memory tool IDs against the DB and updates the config column when they've changed
• Called lazily alongside the existing seed logic on first /api/integrations/providers GET call after boot

Root cause

seedBuiltinProviders() only inserts providers that don't exist. After PR #818 added 13 new GitHub tools (19 total), agents still saw the original 6 because the DB config was frozen from first deployment.

Changes

• provider-repository.ts: new refreshBuiltinProviders(db, builtins) — idempotent, only writes when tool IDs differ
• index.ts: export the new function
• route.ts: call refresh after seed check in GET handler
• provider-repository.test.ts: 4 new tests (stale update, current skip, missing skip, empty-to-new)

Test plan

• 22/22 provider-repository tests passing (18 existing + 4 new)
• 73/73 GitHub provider tests passing
• After deploy: verify AI agent sees all 19 GitHub tools

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Automatic refresh of builtin provider configurations during provider fetch so API responses reflect updated builtin definitions.
  • The refresh step will re-query enabled providers when updates are applied.

• Tests

  • Added tests validating refresh behavior across update/no-op/skip scenarios and schema-change cases.

• Robustness

  • Refresh failures are non-fatal and logged as warnings.

[coderabbitai]

Warning

Rate limit exceeded

@2witstudios has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 54 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 54 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 96533b4d-7c65-4b55-a435-cae1f92066d7

📥 Commits

Reviewing files that changed from the base of the PR and between 6e32083 and 48de5f2.

📒 Files selected for processing (1)

• packages/lib/src/integrations/repositories/provider-repository.test.ts

📝 Walkthrough

Walkthrough

Adds a builtin provider refresh flow: after lazy seeding, the GET providers route calls a new refreshBuiltinProviders routine that compares in-memory builtin definitions to persisted provider configs, selectively updates changed rows, and re-queries enabled providers when updates occur; refresh failures are logged as non-fatal.

Changes

Cohort / File(s)	Summary
Route Handler Integration apps/web/src/app/api/integrations/providers/route.ts	Imported and invoked refreshBuiltinProviders after lazy seeding; logs non-fatal errors and re-queries enabled providers when updates > 0 to ensure response reflects refreshed builtin configs.
Repository Implementation packages/lib/src/integrations/repositories/provider-repository.ts	Added stableStringify and exported refreshBuiltinProviders(database, builtins) that compares serialized configs and updates DB rows only when configs differ; returns updated count.
Repository Tests packages/lib/src/integrations/repositories/provider-repository.test.ts	Added test-only refreshBuiltinProviders helper, stable serialization tests, and a describe suite validating update/no-update/missing-provider/edge-case behaviors; adjusted mocks for update path.
Module Re-export packages/lib/src/integrations/index.ts	Re-exported refreshBuiltinProviders from provider repository to make it publicly available.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Route as ProvidersRoute
  participant Registry as BuiltinRegistry
  participant DB as Database
  participant Logger

  Client->>Route: GET /api/integrations/providers
  Route->>DB: listEnabledProviders() (initial)
  Route->>Registry: read builtinProviderList
  Route->>DB: seedBuiltinProviders() (lazy seed)
  Route->>Route: call refreshBuiltinProviders(db, builtinProviderList)
  Route->>DB: for each builtin: find provider by slug & providerType='builtin'
  alt config differs
    DB->>DB: update provider.config
    Route->>DB: listEnabledProviders() (re-query)
  else no changes
    Note right of Route: no re-query
  end
  Route->>Logger: warn on refresh error (non-fatal)
  Route->>Client: return safeProviders (configs stripped)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: wire AI sandbox integrations end-to-end #737: Added lazy seeding (seedBuiltinProviders) to the same route flow that this change extends with a refresh step.
• feat: built-in provider adapters for Webhook and GitHub #691: Introduced the builtin provider registry (builtinProviderList) which is the input to the new refresh logic.

Poem

🐰 I nibble through configs, line by line,

I match the slugs, make sure they align;
If tools have changed, I tweak with care,
A gentle refresh — no need to despair;
Hop, patch, and skip — all tidy and fine.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding auto-refresh logic for builtin provider configs on startup. It is concise, specific, and directly reflects the core purpose of the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pu/gh-pr-review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06fc4e6afb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

🧹 Nitpick comments (2)

packages/lib/src/integrations/repositories/provider-repository.test.ts (1)

616-632: Consider adding an empty-to-empty tools test case.

The current tests cover config: {} → tools added, but not the case where both existing and new have empty/undefined tools (should skip update). This would verify the '' === '' comparison branch.

it('given provider with no tools and builtin with no tools, should skip update', async () => {
  mockDb.query.integrationProviders.findFirst.mockResolvedValue({
    id: 'prov-1',
    slug: 'github',
    name: 'GitHub',
    config: { tools: [] },
    enabled: true,
    isSystem: true,
  });

  const result = await refreshBuiltinProviders(mockDb, [
    { id: 'github', name: 'GitHub', tools: [] },
  ]);

  expect(result).toBe(0);
  expect(mockDb.update).not.toHaveBeenCalled();
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/lib/src/integrations/repositories/provider-repository.test.ts`
around lines 616 - 632, Add a test in provider-repository.test.ts that exercises
the empty-to-empty tools branch: stub
mockDb.query.integrationProviders.findFirst to return a provider whose
config.tools is an empty array, call refreshBuiltinProviders with a builtin
provider whose tools is also an empty array, and assert the returned value is 0
and mockDb.update was not called; reference the existing test structure that
uses refreshBuiltinProviders, mockDb.query.integrationProviders.findFirst, and
mockDb.update to mirror the other case and ensure the no-op path is covered.

apps/web/src/app/api/integrations/providers/route.ts (1)

50-56: Consider rate-limiting refresh checks for high-traffic scenarios.

The refresh runs on every GET request, executing N queries (one per builtin) even when configs haven't changed. For admin-facing endpoints with low traffic this is fine, but if this endpoint sees heavy load, consider:

• A deployment-time migration or startup hook instead of per-request checks
• Caching the "last refreshed" timestamp and skipping checks within a time window
• A version marker in the DB to detect when refresh is needed

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/api/integrations/providers/route.ts` around lines 50 - 56,
The per-request call to refreshBuiltinProviders in the GET handler causes N DB
queries on every request; change this to a rate-limited refresh by adding a
cached "lastRefreshed" check or a DB version marker so
refreshBuiltinProviders(db, builtinProviderList) only runs when needed:
implement an in-memory timestamp (e.g., lastBuiltinRefreshAt) or read a single
refresh_version field from the DB and skip calling refreshBuiltinProviders if
the timestamp/version is recent; ensure when a refresh does run you still update
providers via listEnabledProviders(db) and update the cached timestamp/version
accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/web/src/app/api/integrations/providers/route.ts`:
- Around line 50-56: The per-request call to refreshBuiltinProviders in the GET
handler causes N DB queries on every request; change this to a rate-limited
refresh by adding a cached "lastRefreshed" check or a DB version marker so
refreshBuiltinProviders(db, builtinProviderList) only runs when needed:
implement an in-memory timestamp (e.g., lastBuiltinRefreshAt) or read a single
refresh_version field from the DB and skip calling refreshBuiltinProviders if
the timestamp/version is recent; ensure when a refresh does run you still update
providers via listEnabledProviders(db) and update the cached timestamp/version
accordingly.

In `@packages/lib/src/integrations/repositories/provider-repository.test.ts`:
- Around line 616-632: Add a test in provider-repository.test.ts that exercises
the empty-to-empty tools branch: stub
mockDb.query.integrationProviders.findFirst to return a provider whose
config.tools is an empty array, call refreshBuiltinProviders with a builtin
provider whose tools is also an empty array, and assert the returned value is 0
and mockDb.update was not called; reference the existing test structure that
uses refreshBuiltinProviders, mockDb.query.integrationProviders.findFirst, and
mockDb.update to mirror the other case and ensure the no-op path is covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f5d3fc7d-9d1e-49f8-8b51-21b9bf3868fa

📥 Commits

Reviewing files that changed from the base of the PR and between 84084de and 06fc4e6.

📒 Files selected for processing (4)

• apps/web/src/app/api/integrations/providers/route.ts
• packages/lib/src/integrations/index.ts
• packages/lib/src/integrations/repositories/provider-repository.test.ts
• packages/lib/src/integrations/repositories/provider-repository.ts

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/lib/src/integrations/repositories/provider-repository.test.ts (1)

550-685: Add DB failure-path tests for refresh flow.

The new suite covers change detection well, but it doesn’t verify behavior when findFirst or update rejects. Add rejects.toThrow cases so error propagation remains intentional for the route-level non-fatal catch path.

Suggested tests

+  it('given findFirst throws, should propagate error', async () => {
+    mockDb.query.integrationProviders.findFirst.mockRejectedValue(new Error('read failed'));
+    await expect(
+      refreshBuiltinProviders(mockDb, [{ id: 'github', name: 'GitHub', tools: [{ id: 'list_repos' }] }])
+    ).rejects.toThrow('read failed');
+  });
+
+  it('given update throws, should propagate error', async () => {
+    mockDb.query.integrationProviders.findFirst.mockResolvedValue({
+      id: 'prov-1',
+      slug: 'github',
+      name: 'GitHub',
+      config: { id: 'github', name: 'GitHub', tools: [] },
+      enabled: true,
+      isSystem: true,
+    });
+    mockDb.update.mockReturnValue({
+      set: vi.fn().mockReturnValue({
+        where: vi.fn().mockReturnValue({
+          returning: vi.fn().mockRejectedValue(new Error('write failed')),
+        }),
+      }),
+    });
+
+    await expect(
+      refreshBuiltinProviders(mockDb, [{ id: 'github', name: 'GitHub', tools: [{ id: 'list_repos' }] }])
+    ).rejects.toThrow('write failed');
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/lib/src/integrations/repositories/provider-repository.test.ts`
around lines 550 - 685, Add tests for DB failure paths in the
refreshBuiltinProviders test suite: create one test where
mockDb.query.integrationProviders.findFirst.mockRejectedValue(new Error(...))
and assert await expect(refreshBuiltinProviders(mockDb,
[...])).rejects.toThrow(), and another where findFirst returns a provider but
mockDb.update.mockReturnValue(...).set(...).where(...).returning.mockRejectedValue(new
Error(...)) and assert the same rejects.toThrow; reference
refreshBuiltinProviders, mockDb.query.integrationProviders.findFirst and
mockDb.update so the tests simulate and verify intentional error propagation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/lib/src/integrations/repositories/provider-repository.test.ts`:
- Around line 529-548: The test defines a local helper function named
refreshBuiltinProviders which duplicates the production logic and hides the real
implementation; remove the local refreshBuiltinProviders definition in
provider-repository.test.ts and import the exported refreshBuiltinProviders from
packages/lib/src/integrations/repositories/provider-repository.ts instead,
updating the test to call that imported function (and adjust any typings or mock
db shape to match the production function signature) so the suite validates the
actual implementation rather than a duplicate.

---

Nitpick comments:
In `@packages/lib/src/integrations/repositories/provider-repository.test.ts`:
- Around line 550-685: Add tests for DB failure paths in the
refreshBuiltinProviders test suite: create one test where
mockDb.query.integrationProviders.findFirst.mockRejectedValue(new Error(...))
and assert await expect(refreshBuiltinProviders(mockDb,
[...])).rejects.toThrow(), and another where findFirst returns a provider but
mockDb.update.mockReturnValue(...).set(...).where(...).returning.mockRejectedValue(new
Error(...)) and assert the same rejects.toThrow; reference
refreshBuiltinProviders, mockDb.query.integrationProviders.findFirst and
mockDb.update so the tests simulate and verify intentional error propagation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d5e88732-a078-4757-9ac8-4c16f63c0c61

📥 Commits

Reviewing files that changed from the base of the PR and between 06fc4e6 and 6e32083.

📒 Files selected for processing (2)

• packages/lib/src/integrations/repositories/provider-repository.test.ts
• packages/lib/src/integrations/repositories/provider-repository.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/lib/src/integrations/repositories/provider-repository.ts