fix: correct Basecamp3 refresh token request

[Shreyas2004wagh]

Summary

Fixes #29585

• Remove the incorrect redirect_uri parameter from Basecamp3 refresh-token requests
• Build the refresh-token request with URLSearchParams so credential values are encoded safely
• Add focused unit coverage for the refresh request shape and credential update behavior

Root Cause

The Basecamp3 OAuth callback used the correct callback URL during the initial token exchange, but the refresh-token helper sent redirect_uri as the app root URL. Basecamp's documented refresh-token request only requires type=refresh, refresh_token, client_id, and client_secret.

Testing

• yarn biome check --write packages/app-store/basecamp3/lib/helpers.ts packages/app-store/basecamp3/lib/helpers.test.ts
• yarn vitest run packages/app-store/basecamp3/lib/helpers.test.ts

[github-actions]

Welcome to Cal.diy, @Shreyas2004wagh! Thanks for opening this pull request.

A few things to keep in mind:

• This is Cal.diy, not Cal.com. Cal.diy is a community-driven, fully open-source fork of Cal.com licensed under MIT. Your changes here will be part of Cal.diy — they will not be deployed to the Cal.com production app.
• Please review our Contributing Guidelines if you haven't already.
• Make sure your PR title follows the Conventional Commits format.

A maintainer will review your PR soon. Thanks for contributing!

[coderabbitai]

📝 Walkthrough

Walkthrough

refreshAccessToken in helpers.ts is refactored to build the Basecamp token refresh URL via URLSearchParams, dropping the previous redirect_uri and WEBAPP_URL dependency. A typed refresh response shape is introduced, HTTP success is validated, the refreshed token is merged with the existing credential key, expires_at is set, and the function now returns Promise<BasecampToken> directly after persisting via prisma.credential.update. A new Vitest test suite in helpers.test.ts mocks getBasecampKeys, Prisma, and global.fetch to verify the fetch call shape, error handling, and the persistence/return behavior.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: correct Basecamp3 refresh token request' accurately summarizes the main change—fixing an incorrect redirect_uri parameter in the Basecamp3 token refresh request.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, explaining the bug fix, root cause, changes made (removing redirect_uri, using URLSearchParams), and testing steps.
Linked Issues check	✅ Passed	The PR fully addresses issue #29585 by removing the incorrect redirect_uri parameter, using URLSearchParams for safe encoding, and adding comprehensive unit test coverage verifying the correct refresh request structure.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: modifications to the refresh token request logic and comprehensive test coverage for the fix. No unrelated changes detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/app-store/basecamp3/lib/helpers.ts`:
- Around line 17-21: Add error handling to check the fetch response status
before parsing the token JSON in the token refresh logic. After the fetch call
that obtains the token from https://launchpad.37signals.com/authorization/token,
check if tokenInfo.ok is true or inspect the status code. If the response
indicates an error (non-2xx status), throw an appropriate error with details
about why the token refresh failed (e.g., invalid refresh token, malformed
request). Only proceed to parse tokenInfo.json() and use the resulting
access_token if the response status indicates success, preventing undefined
token fields from being persisted and causing downstream API failures.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9a0cf134-b69a-4ed8-b520-4bb12182fbd9

📥 Commits

Reviewing files that changed from the base of the PR and between dcd0da8 and 5f25fd6.

📒 Files selected for processing (2)

• packages/app-store/basecamp3/lib/helpers.test.ts
• packages/app-store/basecamp3/lib/helpers.ts

[coderabbitai]

🧹 Nitpick comments (1)

packages/app-store/basecamp3/lib/helpers.test.ts (1)

95-100: ⚡ Quick win

Assert the refresh query contains exactly the documented keys.

The test currently checks required params and redirect_uri absence, but it would still pass if other unexpected params are added. Tighten this to enforce the exact contract (type, refresh_token, client_id, client_secret).

Suggested assertion hardening

     expect(parsedRefreshUrl.searchParams.get("type")).toBe("refresh");
     expect(parsedRefreshUrl.searchParams.get("refresh_token")).toBe(baseCredentialKey.refresh_token);
     expect(parsedRefreshUrl.searchParams.get("client_id")).toBe("basecamp-client-id");
     expect(parsedRefreshUrl.searchParams.get("client_secret")).toBe("basecamp-client-secret");
     expect(parsedRefreshUrl.searchParams.has("redirect_uri")).toBe(false);
+    expect([...parsedRefreshUrl.searchParams.keys()].sort()).toEqual(
+      ["client_id", "client_secret", "refresh_token", "type"].sort()
+    );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app-store/basecamp3/lib/helpers.test.ts` around lines 95 - 100, The
test in the helpers.test.ts file currently verifies individual required
parameters and checks that redirect_uri is absent, but does not enforce that
only the documented keys are present. Add an assertion after the existing checks
that verifies parsedRefreshUrl.searchParams contains exactly the four expected
keys (type, refresh_token, client_id, client_secret) with no additional
unexpected parameters, by checking that the total number of parameters equals 4
or by converting the searchParams to an array and asserting its length and keys
match the documented contract.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/app-store/basecamp3/lib/helpers.test.ts`:
- Around line 95-100: The test in the helpers.test.ts file currently verifies
individual required parameters and checks that redirect_uri is absent, but does
not enforce that only the documented keys are present. Add an assertion after
the existing checks that verifies parsedRefreshUrl.searchParams contains exactly
the four expected keys (type, refresh_token, client_id, client_secret) with no
additional unexpected parameters, by checking that the total number of
parameters equals 4 or by converting the searchParams to an array and asserting
its length and keys match the documented contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 11fd939c-291f-4b16-b56c-95f2f01cd967

📥 Commits

Reviewing files that changed from the base of the PR and between 5f25fd6 and 63fab05.

📒 Files selected for processing (2)

• packages/app-store/basecamp3/lib/helpers.test.ts
• packages/app-store/basecamp3/lib/helpers.ts

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

• packages/app-store/basecamp3/lib/helpers.ts