Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 15 additions & 0 deletions apps/web/src/_hookdiag.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { test } from "vitest"
import { createAppJwt } from "@tripwire/github"
test.runIf(process.env.E2E_LIVE==="1")("hook config", async () => {
const jwt = await createAppJwt()
const r = await fetch("https://api.github.com/app/hook/config", {
headers: { Authorization: `Bearer ${jwt}`, Accept: "application/vnd.github+json" },
})
console.log("hook/config →", r.status, await r.text())
const a = await fetch("https://api.github.com/app", {
headers: { Authorization: `Bearer ${jwt}`, Accept: "application/vnd.github+json" },
})
const app = await a.json()
console.log("app.events:", app.events?.join(","))
console.log("app.external_url:", app.external_url, "| html_url:", app.html_url)
}, 30000)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
136 changes: 131 additions & 5 deletions apps/web/src/integrations/trpc/routers/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,20 @@ import {
contributorRequests,
repositories,
whitelistEntries,
type AppealContentType,
type RequestKind,
} from "@tripwire/db"
import { logEvent } from "@tripwire/core"
import {
loadPrefsForInstallation,
logEvent,
renderDecisionComment,
} from "@tripwire/core"
import {
addComment,
getInstallationToken,
reopenIssue,
reopenPullRequest,
} from "@tripwire/github"

import type { TRPCRouterRecord } from "@trpc/server"

Expand Down Expand Up @@ -81,6 +92,8 @@ export const requestsRouter = {
repoFullName: z.string().min(3),
kind: kindEnum,
reason: z.string().min(10).max(2000),
ref: z.number().int().positive().optional(),
contentType: z.enum(["pull_request", "issue"]).optional(),
})
)
.mutation(async ({ input, ctx }) => {
Expand Down Expand Up @@ -148,6 +161,13 @@ export const requestsRouter = {
githubUserId: ghUser.id,
avatarUrl: ghUser.avatar_url,
reason: input.reason,
// Only unblock appeals target a specific PR/issue to reopen.
githubRef:
input.kind === "unblock" && input.ref ? `#${input.ref}` : undefined,
contentType:
input.kind === "unblock"
? (input.contentType ?? undefined)
: undefined,
})
.returning()

Expand Down Expand Up @@ -194,10 +214,11 @@ export const requestsRouter = {
})
)
.mutation(async ({ input, ctx }) => {
const { request: req } = await assertRequestBelongsToOrg(
input.requestId,
ctx.activeOrgId
)
const {
request: req,
repo,
org,
} = await assertRequestBelongsToOrg(input.requestId, ctx.activeOrgId)

const nextStatus = input.decision === "approve" ? "approved" : "denied"

Expand Down Expand Up @@ -259,10 +280,115 @@ export const requestsRouter = {
},
})

// Reopen the appealed PR/issue (on approve) and notify the contributor of
// the outcome via a GitHub comment. Best-effort and outside the DB
// transaction: the decision is already committed, so a GitHub hiccup must
// not fail the mutation.
await notifyDecisionOnGithub(
req,
repo.fullName,
org.githubInstallationId,
input.decision
).catch((err) =>
console.error(
`[requests] Failed to notify/reopen for request ${req.id}:`,
err
)
)

return { status: nextStatus }
}),
} satisfies TRPCRouterRecord

interface DecidedRequest {
id: string
repoId: string
kind: RequestKind
githubUsername: string
githubUserId: number | null
githubRef: string | null
contentType: AppealContentType | null
}

async function notifyDecisionOnGithub(
req: DecidedRequest,
repoFullName: string,
installationId: number,
decision: "approve" | "deny"
) {
// Only unblock appeals are tied to a reopenable PR/issue. Legacy links and
// access requests carry no ref, so there's nothing to reopen or comment on.
if (req.kind !== "unblock" || !req.githubRef || !req.contentType) return

const [owner, repo] = repoFullName.split("/")
const number = Number(req.githubRef.replace(/^#/, ""))
if (!owner || !repo || !Number.isFinite(number)) return

const kind = req.contentType
const token = await getInstallationToken(installationId)
const prefs = await loadPrefsForInstallation(installationId)

if (decision === "deny") {
await addComment(
token,
owner,
repo,
number,
renderDecisionComment({
prefs,
decision,
username: req.githubUsername,
kind,
})
)
return
}

// Approve: reopen the content, then notify. If the head branch was deleted
// GitHub rejects the reopen — still notify, but don't claim it reopened.
let reopened = false
try {
if (kind === "pull_request") {
await reopenPullRequest(token, owner, repo, number)
} else {
await reopenIssue(token, owner, repo, number)
}
reopened = true
} catch (err) {
console.error(`[requests] Failed to reopen ${req.githubRef}:`, err)
}

await addComment(
token,
owner,
repo,
number,
renderDecisionComment({
prefs,
decision,
username: req.githubUsername,
kind,
reopened,
})
)

if (reopened) {
await logEvent({
repoId: req.repoId,
action:
kind === "pull_request"
? "github_pr_reopened"
: "github_issue_reopened",
severity: "success",
contentType: kind,
description: `Reopened ${kind === "pull_request" ? "PR" : "issue"} ${req.githubRef} after approving @${req.githubUsername}'s appeal`,
targetGithubUsername: req.githubUsername,
targetGithubUserId: req.githubUserId ?? undefined,
githubRef: req.githubRef,
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

type DbOrTx = Parameters<Parameters<typeof db.transaction>[0]>[0] | typeof db

async function applyApproval(
Expand Down
23 changes: 20 additions & 3 deletions apps/web/src/routes/request.$owner.$repo.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { useState } from "react"
import { createFileRoute } from "@tanstack/react-router"
import { useMutation, useQuery } from "@tanstack/react-query"
import { parseAsString, parseAsStringEnum, useQueryStates } from "nuqs"
import {
parseAsInteger,
parseAsString,
parseAsStringEnum,
useQueryStates,
} from "nuqs"
import { authClient } from "@tripwire/auth/client"
import { useTRPC } from "#/integrations/trpc/react"
import { Button } from "@tripwire/ui/button"
Expand All @@ -24,11 +29,15 @@ export const Route = createFileRoute("/request/$owner/$repo")({

function RequestPage() {
const { owner, repo } = Route.useParams()
const [{ kind, u: intendedUser }, setSearch] = useQueryStates({
const [{ kind, u: intendedUser, ref, ct }, setSearch] = useQueryStates({
kind: parseAsStringEnum(["unblock", "access"] as const).withDefault(
"unblock"
),
u: parseAsString,
// The closed PR/issue this appeal is for, carried from the block comment's
// appeal link so an approval can reopen the exact content.
ref: parseAsInteger,
ct: parseAsStringEnum(["pull_request", "issue"] as const),
})
const trpc = useTRPC()
const { data: session, isPending } = authClient.useSession()
Expand Down Expand Up @@ -80,7 +89,15 @@ function RequestPage() {

const handleSubmit = (e: React.FormEvent) => {
e.preventDefault()
submit.mutate({ repoFullName, kind, reason })
submit.mutate({
repoFullName,
kind,
reason,
// Only send the ref for unblock appeals — access requests aren't tied to
// a specific PR/issue.
ref: kind === "unblock" && ref ? ref : undefined,
contentType: kind === "unblock" && ct ? ct : undefined,
})
}

const canSubmit = reason.trim().length >= 10 && !submit.isPending
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/filter-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1443,6 +1443,7 @@ export async function handlePullRequest(
username: ctx.senderLogin,
outcome: result.outcome,
kind: "pull_request",
contentNumber: prNumber,
appBaseUrl: APP_BASE_URL,
})
await closePullRequest(token, owner, repo, prNumber, comment)
Expand Down Expand Up @@ -1537,6 +1538,7 @@ export async function handleIssue(
username: ctx.senderLogin,
outcome: result.outcome,
kind: "issue",
contentNumber: issueNumber,
appBaseUrl: APP_BASE_URL,
})
await closeIssue(token, owner, repo, issueNumber, comment)
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export * from "./contributor-score"
export * from "./contributor-fetch"
export * from "./contributor-identity"
export * from "./filter-pipeline"
export * from "./pr-comment"
export * from "./pr-comment-loader"
export * from "./language-detection"
export * from "./rules/config-schema"
export * from "./rules/config-draft"
Expand Down
93 changes: 93 additions & 0 deletions packages/core/src/pr-comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
} from "@tripwire/db"
import {
renderBlockedComment,
renderDecisionComment,
renderWarnedComment,
buildAppealUrl,
type RenderCommentInput,
Expand Down Expand Up @@ -220,4 +221,96 @@ describe("buildAppealUrl", () => {
"/request/acme/api?kind=unblock&u=octocat"
)
})

it("carries the PR/issue ref so an approval can reopen the exact content", () => {
expect(
buildAppealUrl(
"https://tripwire.app",
"acme/api",
"octocat",
42,
"pull_request"
)
).toBe(
"https://tripwire.app/request/acme/api?kind=unblock&u=octocat&ref=42&ct=pull_request"
)
})

it("omits ref params unless both ref and contentType are given", () => {
expect(
buildAppealUrl("https://tripwire.app", "acme/api", "octocat", 42)
).toBe("https://tripwire.app/request/acme/api?kind=unblock&u=octocat")
})
})

describe("renderBlockedComment appeal ref", () => {
it("threads contentNumber into the appeal link for a PR", () => {
const out = renderBlockedComment({
...BASE,
prefs: null,
outcome: "blocked",
kind: "pull_request",
contentNumber: 42,
})
expect(out).toContain("&ref=42&ct=pull_request")
})

it("leaves the appeal link ref-less when no contentNumber is provided", () => {
const out = renderBlockedComment({
...BASE,
prefs: null,
outcome: "blocked",
kind: "pull_request",
})
expect(out).not.toContain("&ref=")
})
})

describe("renderDecisionComment", () => {
it("notifies + announces a reopen on approval", () => {
const out = renderDecisionComment({
prefs: null,
decision: "approve",
username: "octocat",
kind: "pull_request",
reopened: true,
})
expect(out).toContain("@octocat")
expect(out).toContain("approved your review request")
expect(out).toContain("Reopening this PR")
})

it("notifies without claiming a reopen when the reopen failed", () => {
const out = renderDecisionComment({
prefs: null,
decision: "approve",
username: "octocat",
kind: "pull_request",
reopened: false,
})
expect(out).toContain("couldn't be reopened automatically")
})

it("notifies the requester on denial", () => {
const out = renderDecisionComment({
prefs: null,
decision: "deny",
username: "octocat",
kind: "issue",
})
expect(out).toContain("@octocat")
expect(out).toContain("not approved")
expect(out).toContain("This issue stays closed")
})

it("respects the custom bot display name", () => {
const out = renderDecisionComment({
prefs: prefs({ botDisplayName: "Acme Bot" }),
decision: "approve",
username: "octocat",
kind: "pull_request",
reopened: true,
})
expect(out).toContain("**Acme Bot**:")
})
})
Loading
Loading