Skip to content

Documents support in chat #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dhirenmathur wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Documents support in chat #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
772e4cb
Documents support in chat
dhirenmathur Nov 7, 2025
8e7279c
Update UX for doc support feature
dhirenmathur Dec 9, 2025
afa0ea9
Merge remote-tracking branch 'origin/main' into doc
dhirenmathur Jan 29, 2026
96fc368
merge master
dhirenmathur Jan 29, 2026
48a6053
doc attachment fixes
dhirenmathur Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/(auth)/link-github/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ const LinkGithub = () => {
const originalUser = auth.currentUser;
const linkToUserId = originalUser?.uid;

if (!linkToUserId) {
if (!originalUser || !linkToUserId) {
toast.error("Please sign in with your work email first, then link GitHub.");
return;
}

// Link GitHub to the currently signed-in work/SSO user
const result = await linkWithPopup(auth.currentUser, provider);
const result = await linkWithPopup(originalUser, provider);

// Extract GitHub provider UID from providerData (stable across sessions)
const githubProviderUid =
Expand Down
2 changes: 1 addition & 1 deletion app/(auth)/onboarding/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ const Onboarding = () => {
provider.addScope("repo");

// Link GitHub to the CURRENT authenticated user (work/SSO) without switching accounts
const result = await linkWithPopup(auth.currentUser, provider);
const result = await linkWithPopup(currentUser, provider);
const credential = GithubAuthProvider.credentialFromResult(result);

if (!credential) {
Expand Down
76 changes: 76 additions & 0 deletions app/(auth)/utils/github-oauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
"use client";

import {
GithubAuthProvider,
linkWithPopup,
reauthenticateWithPopup,
User,
UserCredential,
} from "firebase/auth";

type GithubOAuthResult =
| {
status: "linked";
credential: UserCredential;
accessToken: string | null;
providerUsername?: string;
}
| {
status: "already_linked";
};

const GITHUB_PROVIDER_ID = "github.com";

/**
* Triggers GitHub OAuth for the given Firebase user ensuring the required scopes
* are granted. If the account is already linked, it skips the popup.
*/
export const ensureGithubOAuth = async (
user: User | null,
options: { forceReauth?: boolean } = {}
): Promise<GithubOAuthResult> => {
if (!user) {
throw new Error("GitHub OAuth requires an authenticated user.");
}

const hasGithubLinked = user.providerData?.some(
(provider) => provider.providerId === GITHUB_PROVIDER_ID
);

const provider = new GithubAuthProvider();
provider.addScope("read:user");
provider.addScope("read:org");
provider.addScope("user:email");

// When the account is already linked we optionally re-authenticate to refresh tokens.
if (hasGithubLinked && !options.forceReauth) {
return { status: "already_linked" };
}

try {
const result = hasGithubLinked
? await reauthenticateWithPopup(user, provider)
: await linkWithPopup(user, provider);

const credential = GithubAuthProvider.credentialFromResult(result);
const accessToken =
credential?.accessToken ??
(result as any)?._tokenResponse?.oauthAccessToken ??
null;

const providerUsername = (result as any)?._tokenResponse?.screenName;
Comment on lines +55 to +61

@coderabbitai coderabbitai Bot Nov 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Accessing undocumented Firebase internal APIs.

Lines 58 and 61 access internal Firebase response fields via (result as any)?._tokenResponse, which are not part of the documented Firebase API. These fields could change or be removed in future Firebase SDK updates, potentially breaking this functionality.

Consider these alternatives:

  1. Use only the documented credential?.accessToken (line 57) and remove the fallback to _tokenResponse.oauthAccessToken
  2. If the fallback is necessary, add a comment explaining why and add defensive error handling
  3. For screenName, consider fetching the GitHub username via the GitHub API using the access token instead of relying on internal Firebase fields

If the undocumented fields are required, add a TODO comment and error handling:

    const accessToken =
      credential?.accessToken ??
+     // TODO: Remove fallback to internal API when Firebase SDK provides documented alternative
      (result as any)?._tokenResponse?.oauthAccessToken ??
      null;

-    const providerUsername = (result as any)?._tokenResponse?.screenName;
+    // TODO: Fetch username via GitHub API instead of internal Firebase field
+    const providerUsername = (result as any)?._tokenResponse?.screenName ?? null;

I'll verify this review comment by inspecting the actual code and researching Firebase SDK documentation.

🏁 Script executed:

#!/bin/bash
# Verify the exact code at the specified lines
if [ -f "app/(auth)/utils/github-oauth.ts" ]; then
  echo "=== Lines 55-61 of app/(auth)/utils/github-oauth.ts ==="
  sed -n '55,61p' "app/(auth)/utils/github-oauth.ts"
  echo -e "\n=== Full file context (lines 50-70) ==="
  sed -n '50,70p' "app/(auth)/utils/github-oauth.ts"
fi

Length of output: 1025

🏁 Script executed:

#!/bin/bash
# Search for all usages of _tokenResponse in the codebase
echo "=== Usages of _tokenResponse in codebase ==="
rg -n "_tokenResponse" --type=ts --type=tsx -C2

Length of output: 72

🏁 Script executed:

#!/bin/bash
# Check the Firebase/authentication imports and version hints
echo "=== package.json dependencies (firebase) ==="
grep -A 5 -B 5 "firebase" package.json 2>/dev/null || echo "Not found in root package.json"

Length of output: 715

🏁 Script executed:

#!/bin/bash
# Search for GitHub OAuth configuration and comments
echo "=== GitHub OAuth context in file ==="
if [ -f "app/(auth)/utils/github-oauth.ts" ]; then
  cat "app/(auth)/utils/github-oauth.ts"
fi

Length of output: 2050

🌐 Web query:

Firebase SDK GithubAuthProvider credentialFromResult _tokenResponse documented API

💡 Result:

Short answer: _tokenResponse is an internal SDK implementation detail — it is not part of the documented, supported public API. The official docs show GithubAuthProvider.credentialFromResult(userCredential) returns an OAuthCredential (with .accessToken) and do not document or promise any _tokenResponse property. [1][2]

References:

  • GithubAuthProvider API (credentialFromResult → OAuthCredential / .accessToken). [1]
  • General OAuthProvider / provider credentialFromResult docs (returns OAuthCredential; no _tokenResponse documented). [2]

Sources:

  1. Firebase JS docs — GithubAuthProvider (credentialFromResult example showing credential.accessToken). [1]
  2. Firebase JS docs — OAuthProvider / auth API reference (credentialFromResult returns OAuthCredential). [2]

🌐 Web query:

Firebase GitHub OAuth provider screenName username authentication 2024 2025

💡 Result:

Short answer: Firebase's GitHub provider does NOT return the GitHub "username"/"login" (screen name) in user.providerData. It returns displayName, email, photoURL and a numeric GitHub uid. To get the GitHub login you must obtain the provider OAuth access token from the sign-in result and call the GitHub API (GET https://api.github.com/user) to read the "login" field.

How (brief):

  • After sign-in get the OAuth credential and access token (e.g. GithubAuthProvider.credentialFromResult(result).accessToken or result.credential.accessToken). [1][2]
  • Call GitHub API /user with Authorization: Bearer and read the "login" property. [2]
  • Note: providerData only exposes providerId, uid (numeric), displayName, email, photoURL — not the GitHub login. [3]

Sources: Firebase docs on GitHub sign-in and credentials / providerData and a community explanation. [1] [2] [3]

References
[1] Firebase — GitHub sign-in docs (access token / credential examples).
[2] Firebase JS API reference (GithubAuthProvider / OAuthCredential examples).
[3] StackOverflow — explanation that providerData lacks GitHub username and you must call GitHub API to resolve it.

Remove reliance on undocumented Firebase internal APIs _tokenResponse.

The _tokenResponse object is an internal SDK implementation detail, not part of the documented public API. Line 58 (fallback to _tokenResponse?.oauthAccessToken) and line 61 (_tokenResponse?.screenName) create maintenance risk and should be replaced with documented alternatives:

  • Line 58: The documented credential?.accessToken from OAuthCredential is the supported public API; remove the fallback to _tokenResponse.oauthAccessToken.
  • Line 61: Firebase's GitHub provider does not return the GitHub username (screenName) in documented fields. Fetch the GitHub login via the GitHub API using the access token (GET https://api.github.com/user), or remove providerUsername if not required.

Replace with:

    const credential = GithubAuthProvider.credentialFromResult(result);
-   const accessToken =
-     credential?.accessToken ??
-     (result as any)?._tokenResponse?.oauthAccessToken ??
-     null;
+   const accessToken = credential?.accessToken ?? null;

-   const providerUsername = (result as any)?._tokenResponse?.screenName;
+   // Fetch GitHub username via GitHub API if required; Firebase does not expose login/screenName
+   const providerUsername = undefined;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const credential = GithubAuthProvider.credentialFromResult(result);
const accessToken =
credential?.accessToken ??
(result as any)?._tokenResponse?.oauthAccessToken ??
null;
const providerUsername = (result as any)?._tokenResponse?.screenName;
const credential = GithubAuthProvider.credentialFromResult(result);
const accessToken = credential?.accessToken ?? null;
// Fetch GitHub username via GitHub API if required; Firebase does not expose login/screenName
const providerUsername = undefined;
🤖 Prompt for AI Agents 
In app/(auth)/utils/github-oauth.ts around lines 55 to 61, remove reliance on
the undocumented internal _tokenResponse: use only the documented
OAuthCredential public API (i.e. credential?.accessToken) and drop the fallback
to (result as any)?._tokenResponse?.oauthAccessToken; likewise remove reading
providerUsername from _tokenResponse; if you actually need the GitHub username,
perform an authenticated GET to https://api.github.com/user using the access
token (handle missing/null token and errors, parse the "login" field) instead of
accessing internal SDK fields, otherwise delete providerUsername entirely and
propagate null/undefined appropriately.

@yashkrishan yashkrishan Jan 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does oauth changes required for documents support?

@coderabbitai coderabbitai Bot Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for any usage of ensureGithubOAuth in the codebase
echo "=== Searching for usages of ensureGithubOAuth ==="
rg -n "ensureGithubOAuth" --type=ts --type=tsx -C3

echo -e "\n=== Searching for imports from github-oauth.ts ==="
rg -n "from.*github-oauth" --type=ts --type=tsx -C2

echo -e "\n=== Check if there are any connections between OAuth and document features ==="
rg -n "github.*oauth.*(document|attachment|upload)" -i --type=ts --type=tsx

echo -e "\n=== Check what other files were modified in this PR ==="
git diff --name-only origin/main...HEAD 2>/dev/null | head -20

Length of output: 949

🏁 Script executed:

#!/bin/bash
# Search without file type restriction
echo "=== Searching for usages of ensureGithubOAuth ==="
rg -n "ensureGithubOAuth" -C3

echo -e "\n=== Searching for imports from github-oauth ==="
rg -n "from.*github-oauth" -C2

echo -e "\n=== Check link-github and onboarding pages for OAuth usage ==="
echo "--- link-github/page.tsx ---"
cat "app/(auth)/link-github/page.tsx" 2>/dev/null | head -80

echo -e "\n--- onboarding/page.tsx ---"
cat "app/(auth)/onboarding/page.tsx" 2>/dev/null | head -80

Length of output: 6183

@yashkrishan No, the OAuth changes are not required for document support.

Based on my search of the codebase:

  1. The new ensureGithubOAuth utility is not used anywhere - there are no imports or usages of this function in the codebase
  2. Document features are completely independent - they rely on file upload APIs, validation, and attachment handling through ChatService (validateDocument, uploadAttachment, etc.)
  3. Existing OAuth flows don't use the new utility - the link-github and onboarding pages still use Firebase's linkWithPopup directly

It appears that the OAuth refactoring (github-oauth.ts creation and related page updates) was bundled into this PR alongside the document support feature, but they are separate, unrelated changes. The document support functionality works entirely through the file upload and attachment APIs and doesn't require any GitHub OAuth integration.


return {
status: "linked",
credential: result,
accessToken,
providerUsername,
};
} catch (error: any) {
// If the provider is already linked but reauth is not requested, surface as already linked.
if (error?.code === "auth/provider-already-linked") {
return { status: "already_linked" };
}
throw error;
}
};
Loading