Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 34 additions & 1 deletion apps/api/src/app/api/github/callback/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { db } from "@superset/db/client";
import { githubInstallations, members } from "@superset/db/schema";
import type { GithubConfig } from "@superset/db/schema";
import {
githubInstallations,
integrationConnections,
members,
} from "@superset/db/schema";
import { Client } from "@upstash/qstash";
import { and, eq } from "drizzle-orm";

Expand Down Expand Up @@ -120,6 +125,34 @@ export async function GET(request: Request) {
);
}

await db
.insert(integrationConnections)
.values({
organizationId,
connectedByUserId: userId,
provider: "github",
accessToken: `ghi_${installation.id}`, // Installation-based auth, not OAuth
externalOrgId: accountLogin,
externalOrgName: accountLogin,
config: {
provider: "github",
syncIssues: true,
} satisfies GithubConfig,
})
.onConflictDoUpdate({
target: [
integrationConnections.organizationId,
integrationConnections.provider,
],
set: {
connectedByUserId: userId,
accessToken: `ghi_${installation.id}`,
externalOrgId: accountLogin,
externalOrgName: accountLogin,
updatedAt: new Date(),
},
});

// Queue initial sync job
try {
await qstash.publishJSON({
Expand Down
100 changes: 99 additions & 1 deletion apps/api/src/app/api/github/jobs/initial-sync/route.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
import { db } from "@superset/db/client";
import type { GithubConfig } from "@superset/db/schema";
import {
githubInstallations,
githubPullRequests,
githubRepositories,
integrationConnections,
tasks,
} from "@superset/db/schema";
import { Receiver } from "@upstash/qstash";
import { eq } from "drizzle-orm";
import { and, eq } from "drizzle-orm";
import { z } from "zod";

import { env } from "@/env";
import {
mapGithubIssueToTask,
resolveTaskStatusIds,
} from "../../lib/map-issue-to-task";
import { githubApp } from "../../octokit";

const receiver = new Receiver({
Expand Down Expand Up @@ -228,6 +235,97 @@ export async function POST(request: Request) {
}
}

const { organizationId } = parsed.data;
const connection = await db.query.integrationConnections.findFirst({
where: and(
eq(integrationConnections.organizationId, organizationId),
eq(integrationConnections.provider, "github"),
),
columns: { config: true },
});

const config = connection?.config as GithubConfig | null;
const syncIssues = config?.syncIssues !== false;

if (syncIssues) {
const statusIds = await resolveTaskStatusIds({ organizationId });

if (!statusIds) {
console.warn(
"[github/initial-sync] Missing unstarted/completed status types, skipping issue sync",
);
} else {
for (const repo of repos) {
const issues = await octokit.paginate(
octokit.rest.issues.listForRepo,
{
owner: repo.owner.login,
repo: repo.name,
state: "open",
per_page: 100,
},
);

// Filter out pull requests (GitHub issues API includes PRs)
const realIssues = issues.filter(
(issue) => !("pull_request" in issue && issue.pull_request),
);

console.log(
`[github/initial-sync] Found ${realIssues.length} issues for ${repo.full_name}`,
);

for (const issue of realIssues) {
const statusId =
issue.state === "closed"
? statusIds.completedStatusId
: statusIds.unstartedStatusId;

const taskData = mapGithubIssueToTask({
issue: {
id: issue.id,
number: issue.number,
title: issue.title,
body: issue.body,
html_url: issue.html_url,
state: issue.state,
assignee: issue.assignee
? {
login: issue.assignee.login,
email:
"email" in issue.assignee
? (issue.assignee.email as string | null)
: null,
}
: null,
labels: issue.labels,
},
repoName: repo.name,
statusId,
assigneeId: null,
});

await db
.insert(tasks)
.values({
...taskData,
organizationId,
creatorId: installation.connectedByUserId,
priority: "none",
})
.onConflictDoUpdate({
target: [
tasks.organizationId,
tasks.externalProvider,
tasks.externalId,
],
set: { ...taskData, syncError: null },
});
}
}
}
}

// Update installation lastSyncedAt
await db
.update(githubInstallations)
Expand Down
160 changes: 160 additions & 0 deletions apps/api/src/app/api/github/lib/map-issue-to-task.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import { db } from "@superset/db/client";
import { taskStatuses, tasks, users } from "@superset/db/schema";
import { and, eq } from "drizzle-orm";

interface GithubIssue {
id: number;
number: number;
title: string;
body?: string | null;
html_url: string;
state: string;
assignee?: { login: string; email?: string | null } | null;
labels: Array<{ name?: string } | string>;
}

interface ResolvedStatusIds {
unstartedStatusId: string;
completedStatusId: string;
}

export async function resolveTaskStatusIds({
organizationId,
}: {
organizationId: string;
}): Promise<ResolvedStatusIds | null> {
const statuses = await db
.select({ id: taskStatuses.id, type: taskStatuses.type })
.from(taskStatuses)
.where(eq(taskStatuses.organizationId, organizationId));

const unstartedStatus = statuses.find((s) => s.type === "unstarted");
const completedStatus = statuses.find((s) => s.type === "completed");

if (!unstartedStatus || !completedStatus) {
return null;
}

return {
unstartedStatusId: unstartedStatus.id,
completedStatusId: completedStatus.id,
};
}
Comment on lines +21 to +42
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 | 🟡 Minor

Non-deterministic status selection when multiple statuses share a type.

statuses.find() returns the first matching element, but the DB query (line 30-33) has no ORDER BY, so the result depends on the DB's internal row ordering. If an org has multiple statuses with type='unstarted' (which the schema allows), the chosen status could vary between calls.

Add an explicit order (e.g., by position) for predictable results:

♻️ Proposed fix
 const statuses = await db
 	.select({ id: taskStatuses.id, type: taskStatuses.type })
 	.from(taskStatuses)
-	.where(eq(taskStatuses.organizationId, organizationId));
+	.where(eq(taskStatuses.organizationId, organizationId))
+	.orderBy(taskStatuses.position);
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/github/lib/map-issue-to-task.ts` around lines 25 - 46,
The resolveTaskStatusIds function currently uses statuses.find() after querying
taskStatuses without an ORDER BY, which yields non-deterministic results when
multiple rows share the same type; update the DB query that builds statuses
(reference taskStatuses and the const statuses) to include an explicit ordering
(e.g., order by taskStatuses.position or another stable column) so that the
subsequent lookups for unstartedStatus and completedStatus via
statuses.find(...) always pick the intended row deterministically.


async function resolveAssigneeId({
assignee,
}: {
assignee?: GithubIssue["assignee"];
}): Promise<string | null> {
if (!assignee?.email) {
return null;
}

const matchedUser = await db.query.users.findFirst({
where: eq(users.email, assignee.email),
columns: { id: true },
});

return matchedUser?.id ?? null;
}
Comment on lines +44 to +59
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 | 🟡 Minor

resolveAssigneeId will always return null in practice.

This function requires assignee.email to be present, but:

  1. GitHub's webhook payloads for issue events don't include the assignee's email.
  2. The webhook handler in webhooks.ts (line 468) only passes { login }, never email.
  3. The initial-sync route bypasses this function entirely (hardcodes assigneeId: null).

The design doc correctly flags "Add githubLogin column to users table" as a future consideration, so this appears intentional. However, the function gives a false impression that assignee matching is functional. Consider either:

  • Adding a TODO comment explaining why this is a placeholder, or
  • Matching on login against a hypothetical githubLogin field now and skipping the DB call when the field doesn't exist yet.
🤖 Prompt for AI Agents
In `@apps/api/src/app/api/github/lib/map-issue-to-task.ts` around lines 52 - 67,
resolveAssigneeId currently only checks assignee.email (which GitHub webhooks
don't provide) so it effectively always returns null; update it to either (A)
try to match on assignee.login by querying users.githubLogin (use
users.githubLogin in the query if that column exists) and return the matched
user's id, or (B) if the githubLogin column isn't present yet, add a clear TODO
comment inside resolveAssigneeId stating this is a placeholder and that matching
should use assignee.login against users.githubLogin when the DB schema is
updated; reference the resolveAssigneeId function and the users table
(users.githubLogin) when making the change and ensure the function still returns
Promise<string | null>.


export function mapGithubIssueToTask({
issue,
repoName,
statusId,
assigneeId,
}: {
issue: GithubIssue;
repoName: string;
statusId: string;
assigneeId: string | null;
}) {
return {
slug: `${repoName}#${issue.number}`,
title: issue.title,
description: issue.body ?? null,
statusId,
assigneeId,
labels: issue.labels
.map((l) => (typeof l === "string" ? l : l.name))
.filter((name): name is string => !!name),
externalProvider: "github" as const,
externalId: String(issue.id),
externalKey: `#${issue.number}`,
externalUrl: issue.html_url,
lastSyncedAt: new Date(),
};
}

export async function processGithubIssueEvent({
issue,
repoName,
organizationId,
creatorId,
action,
}: {
issue: GithubIssue;
repoName: string;
organizationId: string;
creatorId: string;
action:
| "opened"
| "edited"
| "closed"
| "reopened"
| "assigned"
| "unassigned"
| "labeled"
| "unlabeled"
| "deleted";
}) {
if (action === "deleted") {
await db
.update(tasks)
.set({ deletedAt: new Date() })
.where(
and(
eq(tasks.organizationId, organizationId),
eq(tasks.externalProvider, "github"),
eq(tasks.externalId, String(issue.id)),
),
);
return;
}

const statusIds = await resolveTaskStatusIds({ organizationId });
if (!statusIds) {
console.warn(
"[github/issues] Missing unstarted/completed status types for org:",
organizationId,
);
return;
}

const statusId =
issue.state === "closed"
? statusIds.completedStatusId
: statusIds.unstartedStatusId;

const assigneeId = await resolveAssigneeId({ assignee: issue.assignee });

const taskData = mapGithubIssueToTask({
issue,
repoName,
statusId,
assigneeId,
});

await db
.insert(tasks)
.values({
...taskData,
organizationId,
creatorId,
priority: "none",
})
.onConflictDoUpdate({
target: [tasks.organizationId, tasks.externalProvider, tasks.externalId],
set: { ...taskData, syncError: null },
});
}
Loading
Loading