feat(ui): rich forge entity views — commits, files/diff, timeline, checks, metadata chips
- server: new read routes for PR/MR commits, timeline, reviews, commit-statuses across github/gitlab/gitea; enrich PR/issue summaries with labels/assignees/milestone/commentsCount - ui: forge facade gains getCommits/getTimeline/getChecks; shared ForgeEntityDetailView + section components; mounted into PR/MR views and issue sections
This commit is contained in:
@@ -61,6 +61,12 @@
|
||||
- The route then enriches that result with checks, mergeability, and permission-related fields.
|
||||
- The client caches and shares the result between sidebar and Git view.
|
||||
|
||||
## Enrichment read APIs
|
||||
|
||||
- `GET /api/github/pulls/commits?directory&number&owner&repo` -> `{ connected, repo?, commits[] }` (via `octokit.rest.pulls.listCommits`, mapped to `{ sha, shortSha, message, summary, author, committer, committedAt, parents }`).
|
||||
- `GET /api/github/pulls/timeline?directory&number&owner&repo` -> `{ connected, repo?, events[] }` (via `octokit.rest.issues.listEventsForTimeline`, each event `{ id, type, author, createdAt, body, commitSha }` with the event name lowercased).
|
||||
- Both follow the `issues/comments` envelope pattern: unauthenticated -> `connected: false`, unresolvable repo -> `repo: null` with an empty list, `429` -> `503 { error: 'GitHub rate limited' }`, other provider `4xx` -> `502`.
|
||||
|
||||
## Consumers of PR data
|
||||
|
||||
- `packages/ui/src/components/session/SessionSidebar.tsx` reads all PR entries and maps them to `directory::branch`.
|
||||
|
||||
@@ -11,6 +11,10 @@ let resolvedAuthLoginPromise = null;
|
||||
const PR_CONTEXT_CACHE_TTL_MS = 30_000;
|
||||
const PR_CONTEXT_CACHE_MAX_ENTRIES = 50;
|
||||
const prContextCache = new Map();
|
||||
// Route-level budget for single-call enrichment routes (pulls/commits,
|
||||
// pulls/timeline). Octokit bounds each request at 8s; this caps the whole
|
||||
// route so a slow upstream cannot hold a response (and a client socket) open.
|
||||
const ROUTE_TIMEOUT_MS = 15_000;
|
||||
|
||||
function invalidatePrContextCache(directory, number) {
|
||||
for (const key of prContextCache.keys()) {
|
||||
@@ -139,6 +143,40 @@ async function resolveRepoForRequest(octokit, directory, requestedRepo) {
|
||||
return allowed ? requestedRepo : null;
|
||||
}
|
||||
|
||||
// Shared mappers for PR/issue enrichment fields (labels/assignees/milestone/
|
||||
// comments) used by pulls/list, pulls/context, and issues/get.
|
||||
const mapGitHubUserSummary = (user) => (
|
||||
user && typeof user === 'object'
|
||||
? { login: user.login, id: user.id, avatarUrl: user.avatar_url }
|
||||
: null
|
||||
);
|
||||
|
||||
const mapGitHubLabels = (labels) => (
|
||||
Array.isArray(labels)
|
||||
? labels
|
||||
.map((label) => {
|
||||
if (typeof label === 'string') return null;
|
||||
const name = typeof label?.name === 'string' ? label.name : '';
|
||||
if (!name) return null;
|
||||
return { name, color: typeof label?.color === 'string' ? label.color : undefined };
|
||||
})
|
||||
.filter(Boolean)
|
||||
: []
|
||||
);
|
||||
|
||||
const mapGitHubMilestone = (milestone) => (
|
||||
milestone && typeof milestone === 'object'
|
||||
? {
|
||||
title: typeof milestone.title === 'string' ? milestone.title : '',
|
||||
...(typeof milestone.state === 'string' ? { state: milestone.state } : {}),
|
||||
}
|
||||
: null
|
||||
);
|
||||
|
||||
const mapGitHubAssignees = (assignees) => (
|
||||
Array.isArray(assignees) ? assignees.map(mapGitHubUserSummary).filter(Boolean) : []
|
||||
);
|
||||
|
||||
function setPrStatusCache(key, data, fetchedAt) {
|
||||
// Evict oldest entry when cache exceeds max size
|
||||
if (prStatusCache.size >= PR_STATUS_CACHE_MAX_ENTRIES && !prStatusCache.has(key)) {
|
||||
@@ -1357,6 +1395,13 @@ export function registerGitHubRoutes(app) {
|
||||
})
|
||||
.filter(Boolean)
|
||||
: [],
|
||||
milestone: issue.milestone && typeof issue.milestone === 'object'
|
||||
? {
|
||||
title: typeof issue.milestone.title === 'string' ? issue.milestone.title : '',
|
||||
...(typeof issue.milestone.state === 'string' ? { state: issue.milestone.state } : {}),
|
||||
}
|
||||
: null,
|
||||
commentsCount: typeof issue.comments === 'number' ? issue.comments : undefined,
|
||||
},
|
||||
});
|
||||
} catch (error) {
|
||||
@@ -1460,6 +1505,10 @@ export function registerGitHubRoutes(app) {
|
||||
mergeable: pr.mergeable,
|
||||
mergeableState: pr.mergeable_state,
|
||||
author: pr.user ? { login: pr.user.login, id: pr.user.id, avatarUrl: pr.user.avatar_url } : null,
|
||||
labels: mapGitHubLabels(pr.labels),
|
||||
assignees: mapGitHubAssignees(pr.assignees),
|
||||
milestone: mapGitHubMilestone(pr.milestone),
|
||||
commentsCount: typeof pr.comments === 'number' ? pr.comments : undefined,
|
||||
headLabel: pr.head?.label,
|
||||
headRepo: headRepo && headRepo.owner && headRepo.repo && headRepo.url
|
||||
? headRepo
|
||||
@@ -1638,6 +1687,10 @@ export function registerGitHubRoutes(app) {
|
||||
mergeable: prData.mergeable,
|
||||
mergeableState: prData.mergeable_state,
|
||||
author: prData.user ? { login: prData.user.login, id: prData.user.id, avatarUrl: prData.user.avatar_url } : null,
|
||||
labels: mapGitHubLabels(prData.labels),
|
||||
assignees: mapGitHubAssignees(prData.assignees),
|
||||
milestone: mapGitHubMilestone(prData.milestone),
|
||||
commentsCount: typeof prData.comments === 'number' ? prData.comments : undefined,
|
||||
headLabel: prData.head?.label,
|
||||
headRepo: headRepo && headRepo.owner && headRepo.repo && headRepo.url ? headRepo : null,
|
||||
body: prData.body || '',
|
||||
@@ -1905,4 +1958,115 @@ export function registerGitHubRoutes(app) {
|
||||
return res.status(500).json({ error: error.message || 'Failed to load GitHub PR context' });
|
||||
}
|
||||
});
|
||||
|
||||
// ================= GitHub Pull Request Enrichment APIs =================
|
||||
|
||||
app.get('/api/github/pulls/commits', async (req, res) => {
|
||||
try {
|
||||
const directory = typeof req.query?.directory === 'string' ? req.query.directory.trim() : '';
|
||||
const number = typeof req.query?.number === 'string' ? Number(req.query.number) : null;
|
||||
if (!directory || !number) {
|
||||
return res.status(400).json({ error: 'directory and number are required' });
|
||||
}
|
||||
|
||||
const { getOctokitOrNull } = await getGitHubLibraries();
|
||||
const octokit = getOctokitOrNull();
|
||||
if (!octokit) {
|
||||
return res.json({ connected: false });
|
||||
}
|
||||
|
||||
const requestedRepo = getRequestedRepo(req);
|
||||
const repo = await resolveRepoForRequest(octokit, directory, requestedRepo);
|
||||
if (!repo) {
|
||||
return res.json({ connected: true, repo: null, commits: [] });
|
||||
}
|
||||
|
||||
const result = await withTimeout(
|
||||
octokit.rest.pulls.listCommits({ owner: repo.owner, repo: repo.repo, pull_number: number, per_page: 100 }),
|
||||
ROUTE_TIMEOUT_MS,
|
||||
'GitHub PR commits',
|
||||
);
|
||||
const commits = (Array.isArray(result?.data) ? result.data : []).map((commit) => {
|
||||
const message = typeof commit.commit?.message === 'string' ? commit.commit.message : '';
|
||||
return {
|
||||
sha: commit.sha,
|
||||
shortSha: typeof commit.sha === 'string' ? commit.sha.slice(0, 7) : commit.sha,
|
||||
message,
|
||||
...(message.split('\n')[0] ? { summary: message.split('\n')[0] } : {}),
|
||||
author: mapGitHubUserSummary(commit.author),
|
||||
committer: mapGitHubUserSummary(commit.committer),
|
||||
...(typeof commit.commit?.committer?.date === 'string' ? { committedAt: commit.commit.committer.date } : {}),
|
||||
parents: Array.isArray(commit.parents) ? commit.parents.map((parent) => parent.sha) : [],
|
||||
};
|
||||
});
|
||||
|
||||
return res.json({ connected: true, repo, commits });
|
||||
} catch (error) {
|
||||
if (error?.status === 401) {
|
||||
const { clearGitHubAuth } = await getGitHubLibraries();
|
||||
clearGitHubAuth();
|
||||
return res.json({ connected: false });
|
||||
}
|
||||
if (error?.status === 429) {
|
||||
return res.status(503).json({ error: 'GitHub rate limited' });
|
||||
}
|
||||
if (Number.isInteger(error?.status) && error.status >= 400) {
|
||||
return res.status(502).json({ error: 'GitHub returned an error while fetching pull request commits' });
|
||||
}
|
||||
console.error('Failed to load GitHub pull request commits:', error);
|
||||
return res.status(500).json({ error: error.message || 'Failed to load GitHub pull request commits' });
|
||||
}
|
||||
});
|
||||
|
||||
app.get('/api/github/pulls/timeline', async (req, res) => {
|
||||
try {
|
||||
const directory = typeof req.query?.directory === 'string' ? req.query.directory.trim() : '';
|
||||
const number = typeof req.query?.number === 'string' ? Number(req.query.number) : null;
|
||||
if (!directory || !number) {
|
||||
return res.status(400).json({ error: 'directory and number are required' });
|
||||
}
|
||||
|
||||
const { getOctokitOrNull } = await getGitHubLibraries();
|
||||
const octokit = getOctokitOrNull();
|
||||
if (!octokit) {
|
||||
return res.json({ connected: false });
|
||||
}
|
||||
|
||||
const requestedRepo = getRequestedRepo(req);
|
||||
const repo = await resolveRepoForRequest(octokit, directory, requestedRepo);
|
||||
if (!repo) {
|
||||
return res.json({ connected: true, repo: null, events: [] });
|
||||
}
|
||||
|
||||
const result = await withTimeout(
|
||||
octokit.rest.issues.listEventsForTimeline({ owner: repo.owner, repo: repo.repo, issue_number: number, per_page: 100 }),
|
||||
ROUTE_TIMEOUT_MS,
|
||||
'GitHub PR timeline',
|
||||
);
|
||||
const events = (Array.isArray(result?.data) ? result.data : []).map((event) => ({
|
||||
id: String(event.id),
|
||||
type: typeof event.event === 'string' ? event.event.toLowerCase() : 'other',
|
||||
author: mapGitHubUserSummary(event.actor),
|
||||
createdAt: event.created_at,
|
||||
body: typeof event.body === 'string' ? event.body : null,
|
||||
commitSha: typeof event.commit_id === 'string' ? event.commit_id : null,
|
||||
}));
|
||||
|
||||
return res.json({ connected: true, repo, events });
|
||||
} catch (error) {
|
||||
if (error?.status === 401) {
|
||||
const { clearGitHubAuth } = await getGitHubLibraries();
|
||||
clearGitHubAuth();
|
||||
return res.json({ connected: false });
|
||||
}
|
||||
if (error?.status === 429) {
|
||||
return res.status(503).json({ error: 'GitHub rate limited' });
|
||||
}
|
||||
if (Number.isInteger(error?.status) && error.status >= 400) {
|
||||
return res.status(502).json({ error: 'GitHub returned an error while fetching the pull request timeline' });
|
||||
}
|
||||
console.error('Failed to load GitHub pull request timeline:', error);
|
||||
return res.status(500).json({ error: error.message || 'Failed to load GitHub pull request timeline' });
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
import { beforeEach, describe, expect, test, vi } from 'vitest';
|
||||
import express from 'express';
|
||||
import request from 'supertest';
|
||||
|
||||
// The GitHub route handlers lazy-import ./index.js (via getGitHubLibraries)
|
||||
// for auth + repo resolution, so mocking the module is enough to exercise the
|
||||
// read routes without real GitHub credentials or a temp data dir.
|
||||
const mockState = vi.hoisted(() => ({
|
||||
getOctokitOrNull: vi.fn(),
|
||||
clearGitHubAuth: vi.fn(),
|
||||
octokit: {
|
||||
rest: {
|
||||
pulls: { listCommits: vi.fn() },
|
||||
issues: { listEventsForTimeline: vi.fn() },
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('./index.js', () => ({
|
||||
getOctokitOrNull: mockState.getOctokitOrNull,
|
||||
clearGitHubAuth: mockState.clearGitHubAuth,
|
||||
resolveGitHubRepoFromDirectory: vi.fn(async () => ({
|
||||
repo: { owner: 'owner', repo: 'repo', url: 'https://github.com/owner/repo' },
|
||||
})),
|
||||
}));
|
||||
|
||||
const { registerGitHubRoutes } = await import('./routes.js');
|
||||
|
||||
const createApp = () => {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
registerGitHubRoutes(app);
|
||||
return app;
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
mockState.getOctokitOrNull.mockReset();
|
||||
mockState.clearGitHubAuth.mockReset();
|
||||
mockState.octokit.rest.pulls.listCommits.mockReset();
|
||||
mockState.octokit.rest.issues.listEventsForTimeline.mockReset();
|
||||
mockState.getOctokitOrNull.mockImplementation(() => mockState.octokit);
|
||||
});
|
||||
|
||||
describe('GitHub pull request enrichment routes', () => {
|
||||
test('pulls/commits maps commits with shortSha and summary', async () => {
|
||||
mockState.octokit.rest.pulls.listCommits.mockResolvedValue({
|
||||
data: [
|
||||
{
|
||||
sha: 'abc123def4567890',
|
||||
commit: {
|
||||
message: 'Add the API\n\nAdds the public API',
|
||||
committer: { date: '2026-01-01T10:00:00Z' },
|
||||
},
|
||||
author: { login: 'alice', id: 42, avatar_url: 'https://avatars.githubusercontent.com/u/42' },
|
||||
committer: null,
|
||||
parents: [{ sha: 'parent-one' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const app = createApp();
|
||||
const response = await request(app).get('/api/github/pulls/commits?directory=%2Ftmp%2Fwork&number=9');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toMatchObject({
|
||||
connected: true,
|
||||
repo: { owner: 'owner', repo: 'repo' },
|
||||
commits: [
|
||||
{
|
||||
sha: 'abc123def4567890',
|
||||
shortSha: 'abc123d',
|
||||
message: 'Add the API\n\nAdds the public API',
|
||||
summary: 'Add the API',
|
||||
author: { login: 'alice', id: 42, avatarUrl: 'https://avatars.githubusercontent.com/u/42' },
|
||||
committer: null,
|
||||
committedAt: '2026-01-01T10:00:00Z',
|
||||
parents: ['parent-one'],
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(mockState.octokit.rest.pulls.listCommits).toHaveBeenCalledWith({
|
||||
owner: 'owner',
|
||||
repo: 'repo',
|
||||
pull_number: 9,
|
||||
per_page: 100,
|
||||
});
|
||||
});
|
||||
|
||||
test('pulls/timeline maps timeline events with lowercased types', async () => {
|
||||
mockState.octokit.rest.issues.listEventsForTimeline.mockResolvedValue({
|
||||
data: [
|
||||
{ id: 1, event: 'committed', actor: { login: 'alice', id: 42, avatar_url: 'u' }, created_at: '2026-01-01T10:00:00Z', commit_id: 'abc123def4567890' },
|
||||
{ id: 2, event: 'CLOSED', actor: { login: 'alice', id: 42, avatar_url: 'u' }, created_at: '2026-01-02T10:00:00Z' },
|
||||
{ id: 3, event: 'reviewed', actor: null, created_at: '2026-01-03T10:00:00Z', body: 'LGTM' },
|
||||
],
|
||||
});
|
||||
|
||||
const app = createApp();
|
||||
const response = await request(app).get('/api/github/pulls/timeline?directory=%2Ftmp%2Fwork&number=9');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toMatchObject({
|
||||
connected: true,
|
||||
events: [
|
||||
{ id: '1', type: 'committed', author: { login: 'alice', id: 42 }, createdAt: '2026-01-01T10:00:00Z', commitSha: 'abc123def4567890' },
|
||||
{ id: '2', type: 'closed', author: { login: 'alice', id: 42 } },
|
||||
{ id: '3', type: 'reviewed', author: null, body: 'LGTM' },
|
||||
],
|
||||
});
|
||||
expect(mockState.octokit.rest.issues.listEventsForTimeline).toHaveBeenCalledWith({
|
||||
owner: 'owner',
|
||||
repo: 'repo',
|
||||
issue_number: 9,
|
||||
per_page: 100,
|
||||
});
|
||||
});
|
||||
|
||||
test('pulls/commits returns connected:false when not authenticated', async () => {
|
||||
mockState.getOctokitOrNull.mockImplementation(() => null);
|
||||
|
||||
const app = createApp();
|
||||
const response = await request(app).get('/api/github/pulls/commits?directory=%2Ftmp%2Fwork&number=9');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual({ connected: false });
|
||||
});
|
||||
|
||||
test('pulls/commits requires directory and number', async () => {
|
||||
const app = createApp();
|
||||
const response = await request(app).get('/api/github/pulls/commits?directory=%2Ftmp%2Fwork');
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body).toEqual({ error: 'directory and number are required' });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user