diff --git a/.github/workflows/reviewstack.dev-deploy.yml b/.github/workflows/reviewstack.dev-deploy.yml index 3e509023a31fc..0ed3c0b784df9 100644 --- a/.github/workflows/reviewstack.dev-deploy.yml +++ b/.github/workflows/reviewstack.dev-deploy.yml @@ -1,26 +1,33 @@ -name: Publish https://reviewstack.dev +name: Deploy ReviewStack to Cloudflare Pages on: workflow_dispatch: - schedule: - - cron: '0 0 * * 1-5' + push: + branches: + - main + paths: + - 'eden/contrib/reviewstack/**' + - 'eden/contrib/reviewstack.dev/**' + - 'eden/contrib/shared/**' + - '.github/workflows/reviewstack.dev-deploy.yml' jobs: deploy: runs-on: ubuntu-22.04 - # Our build container already has Node, Yarn, and Python installed. - container: - image: ${{ format('ghcr.io/{0}/build_ubuntu_22_04:latest', github.repository) }} concurrency: group: ${{ github.workflow }}-${{ github.ref }} steps: - name: Checkout Code - uses: actions/checkout@v6 - - name: Grant Access - run: git config --global --add safe.directory "$PWD" + uses: actions/checkout@v4 + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'yarn' + cache-dependency-path: eden/contrib/yarn.lock - name: Install dependencies working-directory: ./eden/contrib/ - run: yarn install --prefer-offline + run: yarn install # Build codegen and then do some sanity checks so we don't push the site # when the tests are broken. @@ -37,11 +44,10 @@ jobs: working-directory: ./eden/contrib/reviewstack.dev run: yarn release - # Push to the release branch. - - name: Deploy - uses: peaceiris/actions-gh-pages@v4 - if: ${{ github.ref == 'refs/heads/main' }} + # Deploy to Cloudflare Pages + - name: Deploy to Cloudflare Pages + uses: cloudflare/wrangler-action@v3 with: - github_token: ${{ secrets.GITHUB_TOKEN }} - publish_branch: reviewstack.dev-prod - publish_dir: ./eden/contrib/reviewstack.dev/build + apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} + accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + command: pages deploy ./eden/contrib/reviewstack.dev/build --project-name=reviewstack diff --git a/eden/contrib/reviewstack.dev/functions/_oauth/callback.ts b/eden/contrib/reviewstack.dev/functions/_oauth/callback.ts new file mode 100644 index 0000000000000..85174b63f7f99 --- /dev/null +++ b/eden/contrib/reviewstack.dev/functions/_oauth/callback.ts @@ -0,0 +1,37 @@ +interface GitHubTokenResponse { + access_token?: string; + error?: string; + error_description?: string; +} + +export const onRequestGet: PagesFunction<{ CLIENT_ID: string; CLIENT_SECRET: string }> = async (context) => { + const url = new URL(context.request.url); + const code = url.searchParams.get('code'); + + if (!code) { + return new Response('Missing code parameter', { status: 400 }); + } + + const tokenResponse = await fetch('https://github.com/login/oauth/access_token', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Accept': 'application/json', + }, + body: JSON.stringify({ + client_id: context.env.CLIENT_ID, + client_secret: context.env.CLIENT_SECRET, + code, + }), + }); + + const data: GitHubTokenResponse = await tokenResponse.json(); + + if (data.error || !data.access_token) { + const errorMsg = encodeURIComponent(data.error_description || data.error || 'Unknown error'); + return Response.redirect(`${url.origin}/?error=${errorMsg}`, 302); + } + + // Redirect back to app with token in hash (not exposed to server logs) + return Response.redirect(`${url.origin}/#token=${data.access_token}`, 302); +}; diff --git a/eden/contrib/reviewstack.dev/functions/_oauth/login.ts b/eden/contrib/reviewstack.dev/functions/_oauth/login.ts new file mode 100644 index 0000000000000..a857fae33fe3f --- /dev/null +++ b/eden/contrib/reviewstack.dev/functions/_oauth/login.ts @@ -0,0 +1,9 @@ +export const onRequestGet: PagesFunction<{ CLIENT_ID: string }> = async (context) => { + const redirectUri = new URL('/_oauth/callback', context.request.url).toString(); + const githubAuthUrl = new URL('https://github.com/login/oauth/authorize'); + githubAuthUrl.searchParams.set('client_id', context.env.CLIENT_ID); + githubAuthUrl.searchParams.set('redirect_uri', redirectUri); + githubAuthUrl.searchParams.set('scope', 'user repo'); + + return Response.redirect(githubAuthUrl.toString(), 302); +}; diff --git a/eden/contrib/reviewstack.dev/src/LazyLoginDialog.tsx b/eden/contrib/reviewstack.dev/src/LazyLoginDialog.tsx index 3b0e9eaa295d3..68db9258f983c 100644 --- a/eden/contrib/reviewstack.dev/src/LazyLoginDialog.tsx +++ b/eden/contrib/reviewstack.dev/src/LazyLoginDialog.tsx @@ -17,7 +17,9 @@ export default function LazyLoginDialog({ }) { const {hostname} = window.location; const LoginComponent = - hostname === 'reviewstack.netlify.app' || hostname === 'reviewstack.dev' + hostname === 'reviewstack.netlify.app' || + hostname === 'reviewstack.dev' || + hostname === 'reviews.qlax.dev' ? NetlifyLoginDialog : DefaultLoginDialog; diff --git a/eden/contrib/reviewstack.dev/src/NetlifyLoginDialog.tsx b/eden/contrib/reviewstack.dev/src/NetlifyLoginDialog.tsx index 102d2eae92102..529c87d10a82c 100644 --- a/eden/contrib/reviewstack.dev/src/NetlifyLoginDialog.tsx +++ b/eden/contrib/reviewstack.dev/src/NetlifyLoginDialog.tsx @@ -11,16 +11,10 @@ import type {CustomLoginDialogProps} from 'reviewstack/src/LoginDialog'; import Footer from './Footer'; import InlineCode from './InlineCode'; import {Box, Button, Heading, Text, TextInput} from '@primer/react'; -import Authenticator from 'netlify-auth-providers'; -import React, {useCallback, useState} from 'react'; +import React, {useCallback, useEffect, useState} from 'react'; import AppHeader from 'reviewstack/src/AppHeader'; import Link from 'reviewstack/src/Link'; -/** - * See https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps - */ -const GITHUB_OAUTH_SCOPE = ['user', 'repo'].join(' '); - export default function NetlifyLoginDialog(props: CustomLoginDialogProps): React.ReactElement { return ( @@ -58,17 +52,32 @@ function EndUserInstructions(props: CustomLoginDialogProps): React.ReactElement const {setTokenAndHostname} = props; const [isButtonDisabled, setButtonDisabled] = useState(false); const [errorMessage, setErrorMessage] = useState(null); - const onClick = useCallback(async () => { - setButtonDisabled(true); - try { - const token = await fetchGitHubToken(); + + // Check for OAuth token in URL hash on mount (from OAuth callback) + useEffect(() => { + const hash = window.location.hash; + const tokenMatch = hash.match(/token=([^&]+)/); + if (tokenMatch) { + const token = tokenMatch[1]; + // Clear the hash from URL + window.history.replaceState(null, '', window.location.pathname + window.location.search); setTokenAndHostname(token, 'github.com'); - } catch (e) { - const message = e instanceof Error ? e.message : 'error fetching OAuth token'; - setErrorMessage(message); } - setButtonDisabled(false); - }, [setButtonDisabled, setErrorMessage, setTokenAndHostname]); + // Check for error in URL params (from failed OAuth) + const params = new URLSearchParams(window.location.search); + const error = params.get('error'); + if (error) { + setErrorMessage(error); + // Clear error from URL + window.history.replaceState(null, '', window.location.pathname); + } + }, [setTokenAndHostname]); + + const onClick = useCallback(() => { + setButtonDisabled(true); + // Redirect to OAuth login endpoint + window.location.href = '/_oauth/login'; + }, []); return ( @@ -222,23 +231,3 @@ function H3({children}: {children: React.ReactNode}): React.ReactElement { ); } -function fetchGitHubToken(): Promise { - return new Promise((resolve, reject) => { - const authenticator = new Authenticator({}); - authenticator.authenticate( - {provider: 'github', scope: GITHUB_OAUTH_SCOPE}, - (error: Error | null, data: {token: string} | null) => { - if (error) { - reject(error); - } else { - const token = data?.token; - if (typeof token === 'string') { - resolve(token); - } else { - reject(new Error('token missing in OAuth response')); - } - } - }, - ); - }); -} diff --git a/eden/contrib/reviewstack/src/PullRequest.tsx b/eden/contrib/reviewstack/src/PullRequest.tsx index 906cac6f341e2..fe2205f21546e 100644 --- a/eden/contrib/reviewstack/src/PullRequest.tsx +++ b/eden/contrib/reviewstack/src/PullRequest.tsx @@ -106,9 +106,11 @@ function PullRequestDetails() { const stack = pullRequestStack.contents; const { bodyHTML } = pullRequest; - let pullRequestBodyHTML; + let pullRequestBodyHTML: string; switch (stack.type) { case 'no-stack': + case 'graphite': + // Graphite stack info is in a comment, not the PR body, so no stripping needed pullRequestBodyHTML = bodyHTML; break; case 'sapling': diff --git a/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx b/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx index d7e93e8acf375..4510355573af8 100644 --- a/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx +++ b/eden/contrib/reviewstack/src/PullRequestChangeCount.tsx @@ -5,23 +5,46 @@ * LICENSE file in the root directory of this source tree. */ -import {gitHubPullRequest} from './recoil'; -import {CounterLabel} from '@primer/react'; +import {gitHubPullRequest, pullRequestSLOC} from './recoil'; +import {CounterLabel, Tooltip} from '@primer/react'; import {useRecoilValue} from 'recoil'; export default function PullRequestChangeCount(): React.ReactElement | null { const pullRequest = useRecoilValue(gitHubPullRequest); + const slocInfo = useRecoilValue(pullRequestSLOC); if (pullRequest == null) { return null; } const {additions, deletions} = pullRequest; + const {significantLines, generatedFileCount} = slocInfo; + + const tooltipText = + generatedFileCount > 0 + ? `${significantLines} significant lines (excludes ${generatedFileCount} generated file${generatedFileCount === 1 ? '' : 's'})` + : `${significantLines} significant lines`; return ( <> - +{additions} - -{deletions} + +{additions} + + -{deletions} + + {significantLines > 0 && ( + + + {significantLines} sig + + + )} ); } diff --git a/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx b/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx index 7d1befb2ce2c1..8b16d12ed1ebd 100644 --- a/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx +++ b/eden/contrib/reviewstack/src/SplitDiffFileHeader.tsx @@ -6,16 +6,18 @@ */ import {ChevronDownIcon, ChevronRightIcon} from '@primer/octicons-react'; -import {Box, Text, Tooltip} from '@primer/react'; +import {Box, Label, Text, Tooltip} from '@primer/react'; export function FileHeader({ path, open, onChangeOpen, + isGenerated, }: { path: string; open?: boolean; onChangeOpen?: (open: boolean) => void; + isGenerated?: boolean; }) { // Even though the enclosing will have border-radius set, we // have to define it again here or things don't look right. @@ -79,6 +81,19 @@ export function FileHeader({ )} {filePathParts} + {isGenerated && ( + + + + )} ); } diff --git a/eden/contrib/reviewstack/src/SplitDiffView.tsx b/eden/contrib/reviewstack/src/SplitDiffView.tsx index d3df75f5b9351..c6c11f5ec84da 100644 --- a/eden/contrib/reviewstack/src/SplitDiffView.tsx +++ b/eden/contrib/reviewstack/src/SplitDiffView.tsx @@ -23,7 +23,9 @@ import SplitDiffRow from './SplitDiffRow'; import {diffAndTokenize, lineRange} from './diffServiceClient'; import {DiffSide} from './generated/graphql'; import {grammars, languages} from './generated/textmate/TextMateGrammarManifest'; +import {GeneratedStatus} from './github/types'; import { + fileGeneratedStatus, gitHubPullRequestLineToPositionForFile, gitHubPullRequestSelectedVersionIndex, gitHubPullRequestVersions, @@ -38,7 +40,7 @@ import {UnfoldIcon} from '@primer/octicons-react'; import {Box, Spinner, Text} from '@primer/react'; import {diffChars} from 'diff'; import React, {useCallback, useEffect, useState} from 'react'; -import {useRecoilValue, useRecoilValueLoadable, waitForAll} from 'recoil'; +import {constSelector, useRecoilValue, useRecoilValueLoadable, waitForAll} from 'recoil'; import organizeLinesIntoGroups from 'shared/SplitDiffView/organizeLinesIntoGroups'; import { applyTokenizationToLine, @@ -81,7 +83,6 @@ export default function SplitDiffView({ after, isPullRequest, }: Props): React.ReactElement { - const [open, setOpen] = useState(true); const scopeName = getFilepathClassifier().findScopeNameForPath(path); const colorMode = useRecoilValue(primerColorMode); const loadable = useRecoilValueLoadable( @@ -104,14 +105,32 @@ export default function SplitDiffView({ isPullRequest ? gitHubPullRequestVersions : nullAtom, isPullRequest ? gitHubPullRequestSelectedVersionIndex : nullAtom, isPullRequest ? gitHubPullRequestLineToPositionForFile(path) : nullAtom, + // Check if file is generated to default collapse state for generated files + isPullRequest ? fileGeneratedStatus(path) : constSelector(GeneratedStatus.Manual), ]), ); - const [{patch, tokenization}, allThreads, newCommentInputCallbacks, commitIDs] = - loadable.getValue(); + const [ + {patch, tokenization}, + allThreads, + newCommentInputCallbacks, + commitIDs, + _versions, + _selectedVersionIndex, + _lineToPosition, + generatedStatus, + ] = loadable.getValue(); + const isGenerated = generatedStatus === GeneratedStatus.Generated; + // Default to collapsed for generated files + const [open, setOpen] = useState(!isGenerated); return ( - setOpen(open)} /> + setOpen(open)} + isGenerated={isGenerated} + /> {open && ( (1500); + +/** + * localStorage key prefix for caching .gitattributes patterns per repo. + */ +const GITATTRIBUTES_CACHE_KEY_PREFIX = 'reviewstack:gitattributes:'; + +/** + * Detect if a file is generated by scanning its content for markers. + * Checks the first 1024 bytes for @generated or @partially-generated tags. + */ +export function detectGeneratedFromContent(content: string | null): GeneratedStatus { + if (content == null) { + return GeneratedStatus.Manual; + } + + // Only scan first 1024 bytes + const header = content.slice(0, 1024); + + if (header.includes('@generated')) { + return GeneratedStatus.Generated; + } + + if (header.includes('@partially-generated')) { + return GeneratedStatus.PartiallyGenerated; + } + + return GeneratedStatus.Manual; +} + +/** + * Detect if a file is generated based on its blob content. + * Results are cached by blob OID. + */ +export function detectGeneratedFromBlob(blob: Blob): GeneratedStatus { + // Check cache first + const cached = generatedStatusCache.get(blob.oid); + if (cached !== undefined) { + return cached; + } + + // Binary files are not considered generated (can't scan them) + if (blob.isBinary) { + generatedStatusCache.set(blob.oid, GeneratedStatus.Manual); + return GeneratedStatus.Manual; + } + + const status = detectGeneratedFromContent(blob.text); + generatedStatusCache.set(blob.oid, status); + return status; +} + +/** + * Check if a file path matches any of the default generated patterns. + */ +export function matchesDefaultGeneratedPattern(filePath: string): boolean { + return DEFAULT_GENERATED_PATTERNS.some(pattern => pattern.test(filePath)); +} + +/** + * Convert a gitattributes glob pattern to a RegExp. + * Supports: + * - ** matches any path segment(s) + * - * matches any characters except / + * - ? matches any single character + * - Leading / anchors to repo root + */ +export function gitAttributePatternToRegex(pattern: string): RegExp { + let regexStr = ''; + let i = 0; + + // Handle leading / (anchor to root) + const anchorToRoot = pattern.startsWith('/'); + if (anchorToRoot) { + regexStr += '^'; + i = 1; + } + + while (i < pattern.length) { + const char = pattern[i]; + + if (char === '*') { + if (pattern[i + 1] === '*') { + // ** matches any path including / + if (pattern[i + 2] === '/') { + // **/ at start or after / means zero or more directories + regexStr += '(?:.*/)?'; + i += 3; + } else { + // ** matches everything + regexStr += '.*'; + i += 2; + } + } else { + // * matches any characters except / + regexStr += '[^/]*'; + i += 1; + } + } else if (char === '?') { + // ? matches any single character except / + regexStr += '[^/]'; + i += 1; + } else if (char === '.') { + regexStr += '\\.'; + i += 1; + } else if (char === '/') { + regexStr += '/'; + i += 1; + } else { + // Escape other regex special characters + regexStr += char.replace(/[\\^$+{}[\]|()]/g, '\\$&'); + i += 1; + } + } + + // If pattern doesn't start with /, it can match at any depth + if (!anchorToRoot) { + regexStr = '(?:^|/)' + regexStr; + } + + // Anchor to end + regexStr += '$'; + + return new RegExp(regexStr); +} + +/** + * Parse .gitattributes file content and extract linguist-generated patterns. + * + * Format: each line is "pattern attribute=value" or "pattern attribute" + * We look for: linguist-generated, linguist-generated=true, -linguist-generated + */ +export function parseGitAttributes(content: string): GitAttributePattern[] { + const patterns: GitAttributePattern[] = []; + + for (const line of content.split('\n')) { + const trimmed = line.trim(); + + // Skip comments and empty lines + if (trimmed === '' || trimmed.startsWith('#')) { + continue; + } + + // Split into pattern and attributes + // Pattern can contain spaces if quoted, but we'll handle simple cases + const match = trimmed.match(/^(\S+)\s+(.+)$/); + if (!match) { + continue; + } + + const [, pattern, attributesStr] = match; + + // Check for linguist-generated attribute + // Formats: linguist-generated, linguist-generated=true, -linguist-generated + if (/\blinguist-generated\b/.test(attributesStr)) { + const isNegated = attributesStr.includes('-linguist-generated'); + const isFalse = /linguist-generated=false/.test(attributesStr); + + patterns.push({ + pattern, + isGenerated: !isNegated && !isFalse, + }); + } + } + + return patterns; +} + +/** + * Check if a file path matches any gitattributes pattern. + * Returns the matched pattern's isGenerated value, or null if no match. + */ +export function matchGitAttributePatterns( + filePath: string, + patterns: GitAttributePattern[], +): boolean | null { + // Process patterns in order - last match wins (gitattributes semantics) + let result: boolean | null = null; + + for (const {pattern, isGenerated} of patterns) { + const regex = gitAttributePatternToRegex(pattern); + if (regex.test(filePath)) { + result = isGenerated; + } + } + + return result; +} + +/** + * Get the generated status for a file, checking (in order): + * 1. .gitattributes patterns + * 2. File content (@generated tag) + * 3. Default patterns + */ +export function getGeneratedStatus( + filePath: string, + gitAttributePatterns: GitAttributePattern[], + blob: Blob | null, +): GeneratedStatus { + // 1. Check gitattributes patterns first + const gitAttrMatch = matchGitAttributePatterns(filePath, gitAttributePatterns); + if (gitAttrMatch === true) { + return GeneratedStatus.Generated; + } + if (gitAttrMatch === false) { + // Explicitly marked as not generated + return GeneratedStatus.Manual; + } + + // 2. Check blob content for @generated tag + if (blob != null) { + const blobStatus = detectGeneratedFromBlob(blob); + if (blobStatus !== GeneratedStatus.Manual) { + return blobStatus; + } + } + + // 3. Check default patterns + if (matchesDefaultGeneratedPattern(filePath)) { + return GeneratedStatus.Generated; + } + + return GeneratedStatus.Manual; +} + +/** + * Cache .gitattributes patterns in localStorage. + */ +export function cacheGitAttributePatterns( + owner: string, + repo: string, + commitOid: GitObjectID, + patterns: GitAttributePattern[], +): void { + const key = `${GITATTRIBUTES_CACHE_KEY_PREFIX}${owner}/${repo}:${commitOid}`; + try { + localStorage.setItem(key, JSON.stringify(patterns)); + } catch { + // localStorage might be full or unavailable + } +} + +/** + * Retrieve cached .gitattributes patterns from localStorage. + */ +export function getCachedGitAttributePatterns( + owner: string, + repo: string, + commitOid: GitObjectID, +): GitAttributePattern[] | null { + const key = `${GITATTRIBUTES_CACHE_KEY_PREFIX}${owner}/${repo}:${commitOid}`; + try { + const cached = localStorage.getItem(key); + if (cached != null) { + return JSON.parse(cached) as GitAttributePattern[]; + } + } catch { + // localStorage might be unavailable or corrupted + } + return null; +} + +/** + * Clear old .gitattributes cache entries to prevent localStorage bloat. + * Keeps only the most recent entries per repo. + */ +export function cleanupGitAttributesCache(maxEntriesPerRepo = 5): void { + try { + const entries: Array<{key: string; repo: string; time: number}> = []; + + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if (key?.startsWith(GITATTRIBUTES_CACHE_KEY_PREFIX)) { + const repoMatch = key.match(/^reviewstack:gitattributes:([^:]+):/); + if (repoMatch) { + entries.push({ + key, + repo: repoMatch[1], + time: i, // Use index as rough time ordering + }); + } + } + } + + // Group by repo and keep only most recent + const byRepo = new Map(); + for (const entry of entries) { + const repoEntries = byRepo.get(entry.repo) ?? []; + repoEntries.push(entry); + byRepo.set(entry.repo, repoEntries); + } + + for (const repoEntries of byRepo.values()) { + if (repoEntries.length > maxEntriesPerRepo) { + // Sort by time (most recent last) and remove old entries + repoEntries.sort((a, b) => a.time - b.time); + for (let i = 0; i < repoEntries.length - maxEntriesPerRepo; i++) { + localStorage.removeItem(repoEntries[i].key); + } + } + } + } catch { + // Ignore cleanup failures + } +} diff --git a/eden/contrib/reviewstack/src/github/types.ts b/eden/contrib/reviewstack/src/github/types.ts index d091931291618..b8457d1b05ffe 100644 --- a/eden/contrib/reviewstack/src/github/types.ts +++ b/eden/contrib/reviewstack/src/github/types.ts @@ -173,3 +173,42 @@ export type PaginationParams = last: number; before?: PageInfo['startCursor']; }; + +/** + * Status indicating whether a file is generated (machine-created) or manual (human-written). + * Used for calculating "significant lines of code" by excluding generated files. + */ +export enum GeneratedStatus { + /** File is manually written by humans */ + Manual = 0, + /** File contains both generated and manual sections (has @partially-generated tag) */ + PartiallyGenerated = 1, + /** File is fully generated (has @generated tag or matches linguist-generated pattern) */ + Generated = 2, +} + +/** + * A pattern from .gitattributes that marks files as generated. + * See: https://github.com/github/linguist/blob/master/docs/overrides.md + */ +export interface GitAttributePattern { + /** Glob pattern (e.g., "*.generated.ts", "libs/graphql/**") */ + pattern: string; + /** Whether files matching this pattern are generated */ + isGenerated: boolean; +} + +/** + * Significant Lines of Code information for a pull request. + * Excludes generated files from the count to show meaningful diff statistics. + */ +export interface SLOCInfo { + /** Lines changed in non-generated files (additions + deletions) */ + significantLines: number; + /** Total lines changed across all files */ + totalLines: number; + /** Number of files detected as generated */ + generatedFileCount: number; + /** Paths of files detected as generated */ + generatedFiles: string[]; +} diff --git a/eden/contrib/reviewstack/src/graphiteStack.test.ts b/eden/contrib/reviewstack/src/graphiteStack.test.ts new file mode 100644 index 0000000000000..f962ec0063fa6 --- /dev/null +++ b/eden/contrib/reviewstack/src/graphiteStack.test.ts @@ -0,0 +1,157 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {isGraphiteStackComment, parseGraphiteStackComment} from './graphiteStack'; + +const GRAPHITE_COMMENT = `\ +> [!WARNING] +> This pull request is not mergeable via GitHub because a downstack PR is open. + +* **#3818** 👈 +* **#3815** +* **#3814** +* \`main\` + +--- + +This stack of pull requests is managed by **Graphite**. +`; + +const GRAPHITE_COMMENT_MIDDLE_OF_STACK = `\ +* **#3818** +* **#3815** 👈 +* **#3814** +* \`main\` + +This stack of pull requests is managed by **Graphite**. +`; + +const GRAPHITE_COMMENT_BOTTOM_OF_STACK = `\ +* **#3818** +* **#3815** +* **#3814** 👈 +* \`main\` + +This stack of pull requests is managed by **Graphite**. +`; + +const GRAPHITE_COMMENT_SINGLE_PR = `\ +* **#100** 👈 +* \`develop\` + +This stack of pull requests is managed by **Graphite**. +`; + +const NON_GRAPHITE_COMMENT = `\ +This is a regular comment that doesn't contain any Graphite stack info. +Here are some PR references: #123 #456 +`; + +describe('isGraphiteStackComment', () => { + test('returns true for Graphite stack comment', () => { + expect(isGraphiteStackComment(GRAPHITE_COMMENT)).toBe(true); + }); + + test('returns false for non-Graphite comment', () => { + expect(isGraphiteStackComment(NON_GRAPHITE_COMMENT)).toBe(false); + }); + + test('returns false for empty string', () => { + expect(isGraphiteStackComment('')).toBe(false); + }); +}); + +describe('parseGraphiteStackComment', () => { + test('parses stack with current PR at top', () => { + const result = parseGraphiteStackComment(GRAPHITE_COMMENT); + expect(result).toEqual({ + stack: [3818, 3815, 3814], + currentStackEntry: 0, + baseBranch: 'main', + }); + }); + + test('parses stack with current PR in middle', () => { + const result = parseGraphiteStackComment(GRAPHITE_COMMENT_MIDDLE_OF_STACK); + expect(result).toEqual({ + stack: [3818, 3815, 3814], + currentStackEntry: 1, + baseBranch: 'main', + }); + }); + + test('parses stack with current PR at bottom', () => { + const result = parseGraphiteStackComment(GRAPHITE_COMMENT_BOTTOM_OF_STACK); + expect(result).toEqual({ + stack: [3818, 3815, 3814], + currentStackEntry: 2, + baseBranch: 'main', + }); + }); + + test('parses single PR stack', () => { + const result = parseGraphiteStackComment(GRAPHITE_COMMENT_SINGLE_PR); + expect(result).toEqual({ + stack: [100], + currentStackEntry: 0, + baseBranch: 'develop', + }); + }); + + test('returns null for non-Graphite comment', () => { + const result = parseGraphiteStackComment(NON_GRAPHITE_COMMENT); + expect(result).toBe(null); + }); + + test('returns null for empty string', () => { + const result = parseGraphiteStackComment(''); + expect(result).toBe(null); + }); + + test('returns null for Graphite comment without current PR marker', () => { + const commentWithoutMarker = `\ +* **#3818** +* **#3815** +* \`main\` + +This stack of pull requests is managed by **Graphite**. +`; + const result = parseGraphiteStackComment(commentWithoutMarker); + expect(result).toBe(null); + }); + + test('handles comment without base branch', () => { + const commentWithoutBase = `\ +* **#3818** 👈 +* **#3815** + +This stack of pull requests is managed by **Graphite**. +`; + const result = parseGraphiteStackComment(commentWithoutBase); + expect(result).toEqual({ + stack: [3818, 3815], + currentStackEntry: 0, + baseBranch: null, + }); + }); + + test('handles carriage return characters', () => { + const commentWithCR = `\ +* **#3818** 👈\r +* **#3815** \r +* \`main\`\r +\r +This stack of pull requests is managed by **Graphite**.\r +`; + const result = parseGraphiteStackComment(commentWithCR); + expect(result).toEqual({ + stack: [3818, 3815], + currentStackEntry: 0, + baseBranch: 'main', + }); + }); +}); diff --git a/eden/contrib/reviewstack/src/graphiteStack.ts b/eden/contrib/reviewstack/src/graphiteStack.ts new file mode 100644 index 0000000000000..23f7cf94dfcfb --- /dev/null +++ b/eden/contrib/reviewstack/src/graphiteStack.ts @@ -0,0 +1,103 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Graphite (https://graphite.dev) posts a comment on each PR in a stack + * with links to all PRs in the stack. This module provides utilities for + * parsing those comments. + * + * Example Graphite comment format: + * + * ``` + * * **#3818** 👈 + * * **#3815** + * * **#3814** + * * `main` + * + * ... + * This stack of pull requests is managed by **Graphite** + * ``` + */ + +export type GraphiteStackBody = { + /** PR numbers in stack order (top of stack first) */ + stack: number[]; + /** Index of the current PR in the stack (marked with 👈) */ + currentStackEntry: number; + /** Base branch name (e.g., "main") */ + baseBranch: string | null; +}; + +const GRAPHITE_SIGNATURE = 'This stack of pull requests is managed by'; + +/** + * Check if a comment body is a Graphite stack comment. + */ +export function isGraphiteStackComment(body: string): boolean { + return body.includes(GRAPHITE_SIGNATURE); +} + +/** + * Parse a Graphite stack comment and extract PR numbers and current position. + * Returns null if the comment is not a valid Graphite stack comment. + */ +export function parseGraphiteStackComment(body: string): GraphiteStackBody | null { + if (!isGraphiteStackComment(body)) { + return null; + } + + const stack: number[] = []; + let currentStackEntry = -1; + let baseBranch: string | null = null; + + // Match PR number lines: * **#123** ... + // The 👈 emoji marks the current PR + const prLineRegex = /^\* \*\*#(\d+)\*\*/gm; + const baseLineRegex = /^\* `([^`]+)`/gm; + + const lines = body.split('\n'); + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + + // Check for PR number line + const prMatch = /^\* \*\*#(\d+)\*\*/.exec(line); + if (prMatch) { + const prNumber = parseInt(prMatch[1], 10); + stack.push(prNumber); + + // Check if this is the current PR (marked with 👈) + if (line.includes('👈')) { + currentStackEntry = stack.length - 1; + } + continue; + } + + // Check for base branch line + const baseMatch = /^\* `([^`]+)`/.exec(line); + if (baseMatch) { + baseBranch = baseMatch[1]; + continue; + } + } + + // Must have at least one PR and a current entry + if (stack.length === 0) { + return null; + } + + // If no 👈 marker found, we can't determine current position + if (currentStackEntry === -1) { + return null; + } + + return { + stack, + currentStackEntry, + baseBranch, + }; +} diff --git a/eden/contrib/reviewstack/src/queries/PullRequestTimelineItemFragment.graphql b/eden/contrib/reviewstack/src/queries/PullRequestTimelineItemFragment.graphql index f4b765c7cecf6..cb09a94f58344 100644 --- a/eden/contrib/reviewstack/src/queries/PullRequestTimelineItemFragment.graphql +++ b/eden/contrib/reviewstack/src/queries/PullRequestTimelineItemFragment.graphql @@ -50,6 +50,7 @@ fragment PullRequestTimelineItemFragment on PullRequestTimelineItems { author { ...ActorFragment } + body bodyHTML } ... on RenamedTitleEvent { diff --git a/eden/contrib/reviewstack/src/recoil.ts b/eden/contrib/reviewstack/src/recoil.ts index 96fb80e85e449..2bf89341964e1 100644 --- a/eden/contrib/reviewstack/src/recoil.ts +++ b/eden/contrib/reviewstack/src/recoil.ts @@ -29,8 +29,10 @@ import type { Commit, DateTime, ForcePushEvent, + GitAttributePattern, GitObjectID, ID, + SLOCInfo, Version, VersionCommit, } from './github/types'; @@ -39,6 +41,12 @@ import type {RecoilValueReadOnly} from 'recoil'; import {lineToPosition} from './diffServiceClient'; import {DiffSide, PullRequestReviewState, UserHomePageQuery} from './generated/graphql'; +import { + cacheGitAttributePatterns, + getCachedGitAttributePatterns, + getGeneratedStatus, + parseGitAttributes, +} from './generatedFiles'; import CachingGitHubClient, {openDatabase} from './github/CachingGitHubClient'; import GraphQLGitHubClient from './github/GraphQLGitHubClient'; import {diffCommits, diffCommitWithParent} from './github/diff'; @@ -50,6 +58,7 @@ import { gitHubUsername, } from './github/gitHubCredentials'; import queryGraphQL from './github/queryGraphQL'; +import {GeneratedStatus} from './github/types'; import {stackedPullRequest, stackedPullRequestFragments} from './stackState'; import {getPathForChange, getTreeEntriesForChange, groupBy, groupByDiffSide} from './utils'; import {atom, atomFamily, constSelector, selector, selectorFamily, waitForAll} from 'recoil'; @@ -1307,3 +1316,198 @@ function createClient( const client = new GraphQLGitHubClient(hostname, organization, repository, token); return new CachingGitHubClient(db, client, organization, repository); } + +// ============================================================================= +// SLOC (Significant Lines of Code) selectors +// ============================================================================= + +/** + * Fetches and parses the .gitattributes file from the repo root. + * Uses localStorage cache to avoid re-fetching for the same commit. + */ +export const gitAttributesPatterns = selector({ + key: 'gitAttributesPatterns', + get: async ({get}) => { + const orgAndRepo = get(gitHubOrgAndRepo); + const commits = get(gitHubPullRequestCommits); + const client = get(gitHubClient); + + if (orgAndRepo == null || commits.length === 0 || client == null) { + return []; + } + + const {org, repo} = orgAndRepo; + // Use the latest commit's OID + const latestCommit = commits[commits.length - 1]; + const commitOid = latestCommit.oid; + + // Check localStorage cache first + const cached = getCachedGitAttributePatterns(org, repo, commitOid); + if (cached != null) { + return cached; + } + + // Fetch .gitattributes from the repo root + try { + // Get the tree for the head commit to find .gitattributes + const commit = await client.getCommit(commitOid); + if (commit == null) { + return []; + } + + // Find .gitattributes in the root tree + const gitattributesEntry = commit.tree.entries.find( + entry => entry.name === '.gitattributes' && entry.type === 'blob', + ); + + if (gitattributesEntry == null) { + // No .gitattributes file - cache empty result + cacheGitAttributePatterns(org, repo, commitOid, []); + return []; + } + + // Fetch the blob content + const blob = await client.getBlob(gitattributesEntry.oid); + if (blob == null || blob.isBinary || blob.text == null) { + cacheGitAttributePatterns(org, repo, commitOid, []); + return []; + } + + // Parse the .gitattributes content + const patterns = parseGitAttributes(blob.text); + cacheGitAttributePatterns(org, repo, commitOid, patterns); + return patterns; + } catch { + // On error, return empty patterns + return []; + } + }, +}); + +/** + * Get the generated status for a specific file in the current PR diff. + * Checks .gitattributes patterns, file content, and default patterns. + */ +export const fileGeneratedStatus = selectorFamily({ + key: 'fileGeneratedStatus', + get: + (filePath: string) => + ({get}) => { + const gitAttrPatterns = get(gitAttributesPatterns); + const diff = get(gitHubPullRequestVersionDiff); + + if (diff == null) { + return GeneratedStatus.Manual; + } + + // Find the file in the diff to get its blob OID + const fileChange = diff.diff.find(change => getPathForChange(change) === filePath); + + // Get the blob for content scanning + let blob: Blob | null = null; + if (fileChange != null) { + const entries = getTreeEntriesForChange(fileChange); + // Prefer the "after" version, fall back to "before" for deletions + const blobOid = entries.after?.oid ?? entries.before?.oid; + if (blobOid != null) { + blob = get(gitHubBlob(blobOid)); + } + } + + return getGeneratedStatus(filePath, gitAttrPatterns, blob); + }, +}); + +/** + * Calculate SLOC (Significant Lines of Code) for the current PR. + * Excludes generated files from the count. + * + * Note: This selector intentionally calls getGeneratedStatus() directly instead + * of using the fileGeneratedStatus selectorFamily to avoid creating deep + * dependency chains that can cause performance issues with many files. + */ +export const pullRequestSLOC = selector({ + key: 'pullRequestSLOC', + get: ({get}) => { + const pullRequest = get(gitHubPullRequest); + const diff = get(gitHubPullRequestVersionDiff); + // Load gitAttributesPatterns once, not per-file via fileGeneratedStatus + const gitAttrPatterns = get(gitAttributesPatterns); + + if (pullRequest == null || diff == null) { + return { + significantLines: 0, + totalLines: 0, + generatedFileCount: 0, + generatedFiles: [], + }; + } + + // Use the PR-level additions/deletions as the total + // Note: This counts all lines, not just non-blank lines + const totalLines = pullRequest.additions + pullRequest.deletions; + + let generatedLines = 0; + const generatedFiles: string[] = []; + + for (const change of diff.diff) { + const filePath = getPathForChange(change); + const entries = getTreeEntriesForChange(change); + + // Get blob for generated status detection (prefer after, fall back to before) + const blobOid = entries.after?.oid ?? entries.before?.oid; + const blob = blobOid != null ? get(gitHubBlob(blobOid)) : null; + + // Check if file is generated using the function directly to avoid + // deep dependency chains from fileGeneratedStatus selectorFamily + const generatedStatus = getGeneratedStatus(filePath, gitAttrPatterns, blob); + if (generatedStatus === GeneratedStatus.Generated) { + generatedFiles.push(filePath); + + // Count diff lines (additions + deletions) for this generated file + // This must be compatible with how totalLines is calculated (PR additions + deletions) + let fileAdditions = 0; + let fileDeletions = 0; + + if (change.type === 'add' && entries.after != null) { + // New file: all non-blank lines are additions + const afterBlob = get(gitHubBlob(entries.after.oid)); + if (afterBlob?.text != null) { + fileAdditions = afterBlob.text.split('\n').filter(line => line.trim() !== '').length; + } + } else if (change.type === 'remove' && entries.before != null) { + // Deleted file: all non-blank lines are deletions + const beforeBlob = get(gitHubBlob(entries.before.oid)); + if (beforeBlob?.text != null) { + fileDeletions = beforeBlob.text.split('\n').filter(line => line.trim() !== '').length; + } + } else if (change.type === 'modify') { + // Modified file: approximate by comparing before/after line counts + const beforeBlob = entries.before != null ? get(gitHubBlob(entries.before.oid)) : null; + const afterBlob = entries.after != null ? get(gitHubBlob(entries.after.oid)) : null; + + const beforeLines = + beforeBlob?.text?.split('\n').filter(line => line.trim() !== '').length ?? 0; + const afterLines = + afterBlob?.text?.split('\n').filter(line => line.trim() !== '').length ?? 0; + + fileAdditions = Math.max(0, afterLines - beforeLines); + fileDeletions = Math.max(0, beforeLines - afterLines); + } + + generatedLines += fileAdditions + fileDeletions; + } + } + + // Significant lines = total - generated + // This is an approximation since we're using PR-level totals + const significantLines = Math.max(0, totalLines - generatedLines); + + return { + significantLines, + totalLines, + generatedFileCount: generatedFiles.length, + generatedFiles, + }; + }, +}); diff --git a/eden/contrib/reviewstack/src/stackState.ts b/eden/contrib/reviewstack/src/stackState.ts index f5163a06ee509..dac2bcd2b9f12 100644 --- a/eden/contrib/reviewstack/src/stackState.ts +++ b/eden/contrib/reviewstack/src/stackState.ts @@ -6,9 +6,11 @@ */ import type {StackPullRequestFragment} from './generated/graphql'; +import type {GraphiteStackBody} from './graphiteStack'; import type {SaplingPullRequestBody} from './saplingStack'; import {pullRequestNumbersFromBody} from './ghstackUtils'; +import {parseGraphiteStackComment} from './graphiteStack'; import {gitHubClient, gitHubPullRequest} from './recoil'; import {parseSaplingStackBody} from './saplingStack'; import {selector, waitForAll} from 'recoil'; @@ -22,6 +24,10 @@ type StackedPullRequest = type: 'ghstack'; stack: number[]; } + | { + type: 'graphite'; + body: GraphiteStackBody; + } | { type: 'no-stack'; }; @@ -31,6 +37,8 @@ export const stackedPullRequest = selector({ get: ({get}) => { const pullRequest = get(gitHubPullRequest); const body = pullRequest?.body; + + // Check PR body for Sapling or ghstack stack info if (body != null) { const saplingStack = parseSaplingStackBody(body); if (saplingStack != null) { @@ -43,6 +51,17 @@ export const stackedPullRequest = selector({ } } + // Check timeline comments for Graphite stack info + const timelineItems = pullRequest?.timelineItems?.nodes ?? []; + for (const item of timelineItems) { + if (item?.__typename === 'IssueComment' && item.body != null) { + const graphiteStack = parseGraphiteStackComment(item.body); + if (graphiteStack != null) { + return {type: 'graphite', body: graphiteStack}; + } + } + } + return {type: 'no-stack'}; }, }); @@ -60,6 +79,9 @@ const stackedPullRequestNumbers = selector({ case 'ghstack': { return stacked.stack; } + case 'graphite': { + return stacked.body.stack; + } } }, });