Skip to content

Conversation

@brendan-kellam
Copy link
Contributor

@brendan-kellam brendan-kellam commented Feb 3, 2026

Problem

In Kubernetes deployments, when internal services call Sourcebot APIs, the Host header is set to the internal service DNS name. The old getBaseUrl() function used this header to build URLs returned to users, resulting in inaccessible links.

Solution

Since AUTH_URL is a required environment variable that contains the public-facing URL, we now use it directly instead of inferring the URL from request headers.

Summary by CodeRabbit

  • Refactor
    • Unified web URL handling to use a single configured auth URL across browsing, search, chat, and organization settings for consistent links.
  • Bug Fix
    • Fixed external link issues in certain deployments so links to repositories and services are reliably accessible.
  • UI
    • Added a simple error message when organization data fails to load.

@github-actions

This comment has been minimized.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Base URL sourcing was changed from runtime request headers (and the removed getBaseUrl) to a single environment variable (env.AUTH_URL) across the web package; imports of headers/getBaseUrl were removed and a defensive org null-check was added in one component.

Changes

Cohort / File(s) Summary
Utility Removal
packages/web/src/lib/utils.server.ts
Removed exported getBaseUrl which derived base URL from Next.js request headers; eliminated header-based fallback logic.
API Routes
packages/web/src/app/api/(server)/chat/blocking/route.ts, packages/web/src/app/api/(server)/repos/listReposApi.ts
Replaced header-derived getBaseUrl(headers...) with env.AUTH_URL; removed headers and getBaseUrl imports and updated URL construction.
Web Actions & Components
packages/web/src/actions.ts, packages/web/src/app/components/organizationAccessSettings.tsx
Switched webUrl/baseUrl construction to env.AUTH_URL; removed header imports; added if (!org) return <div>Error loading organization</div> guard in organization component.
Search Feature
packages/web/src/features/search/zoektSearcher.ts
Replaced header/baseUrl logic with env.AUTH_URL when building web links in search result transforms; removed header/getBaseUrl imports.
Meta / Changelog
CHANGELOG.md, package.json
Added changelog entry; small manifest/package metadata edits.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 clearly and specifically describes the main change: replacing the header-based URL derivation with the AUTH_URL environment variable for generating external links.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bkellam/fix-SOU-339

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.

@claude
Copy link

claude bot commented Feb 3, 2026


Code review

No issues found. Checked for bugs and CLAUDE.md compliance.


Copy link
Contributor

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/web/src/actions.ts (1)

4-4: 🛠️ Refactor suggestion | 🟠 Major

Apply the same fix (use env.AUTH_URL or similar) to origin derivations at lines 894 and 1586.

Both createInvites and approveAccountRequest use headers().get('origin') to construct external-facing links (invite links at line 917 and approval email baseUrl at line 1589). These have the same Kubernetes deployment issue as the fix applied at line 4—the origin header can be unreliable when requests are proxied. Line 480 shows the pattern already in use: const baseUrl = env.AUTH_URL;. Apply this same approach to lines 894 and 1586 for consistency and reliability.

🧹 Nitpick comments (2)
packages/web/src/features/search/zoektSearcher.ts (1)

429-435: Potential double-slash if AUTH_URL ends with a trailing slash.

If env.AUTH_URL is configured with a trailing slash (e.g., https://example.com/) and getBrowsePath returns a path starting with /, the resulting URL will contain a double slash. Consider normalizing the URL:

♻️ Suggested normalization
-            webUrl: `${env.AUTH_URL}${getBrowsePath({
+            webUrl: `${env.AUTH_URL.replace(/\/$/, '')}${getBrowsePath({
                 repoName: repo.name,
                 path: fileName,
                 pathType: 'blob',
                 domain: SINGLE_TENANT_ORG_DOMAIN,
                 revisionName: branchName,
             })}`,

Alternatively, you could create a utility function to handle URL concatenation consistently across all the files modified in this PR.

packages/web/src/app/api/(server)/chat/blocking/route.ts (1)

189-190: Same trailing slash concern applies here.

If env.AUTH_URL ends with a trailing slash, the constructed chatUrl will have a double slash (e.g., https://example.com//org/chat/...).

♻️ Suggested normalization
-            const baseUrl = env.AUTH_URL;
-            const chatUrl = `${baseUrl}/${org.domain}/chat/${chat.id}`;
+            const baseUrl = env.AUTH_URL.replace(/\/$/, '');
+            const chatUrl = `${baseUrl}/${org.domain}/chat/${chat.id}`;

@brendan-kellam brendan-kellam merged commit 15e4aa1 into main Feb 3, 2026
8 of 9 checks passed
@brendan-kellam brendan-kellam deleted the bkellam/fix-SOU-339 branch February 3, 2026 06:33
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