mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-01 10:53:14 +00:00
chore: cherry-pick deferred reviewer-misc upstream commits (#127)
Some checks are pending
CI / Lint (push) Waiting to run
CI / Format check (push) Waiting to run
CI / Unit tests (push) Waiting to run
CI / Playwright E2E (push) Waiting to run
CI / Docker build smoke (push) Waiting to run
CI / Triage ledger up to date (push) Waiting to run
CI / ui bun.lock in sync (push) Waiting to run
Some checks are pending
CI / Lint (push) Waiting to run
CI / Format check (push) Waiting to run
CI / Unit tests (push) Waiting to run
CI / Playwright E2E (push) Waiting to run
CI / Docker build smoke (push) Waiting to run
CI / Triage ledger up to date (push) Waiting to run
CI / ui bun.lock in sync (push) Waiting to run
* feat: add PR trace resolution (#1612) * feat: add PR trace resolution Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: inject reviewer trace context as JSON Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: address review on PR trace resolution Use the documented LangSmith metadata filter syntax (and(eq(metadata_key,...), eq(metadata_value,...))) instead of has(metadata, '{...}'), which does not match runs — _list_thread_runs was silently returning nothing. Bound full-text searches to a 90-day window so they don't hit LangSmith's large-window rate limit. Also folds in the best-effort branch->head-sha resolver (dropping the weighted scoring/threshold + repo/file evidence + GitHub hydration), sandbox JSON injection, and the admin "Resolve trace" dry-run endpoint. The IDOR findings are moot: resolve_pr_to_threads/summarize_agent_session were removed; resolution now runs deterministically from the trusted run config with no model-controlled pr_url or thread_id. * fix: scope branch trace search to the repo Branch names like fix-tests aren't unique across repos (or older PRs) in a shared tracing project, so an unscoped branch hit could resolve to an unrelated thread and write its runs into the reviewer sandbox. Require the repo slug to co-occur with the branch in matched runs; the full head SHA stays unscoped since it is globally unique. Addresses open-swe review on PR #1612. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> (cherry picked from commit69148f54f5) * fix: post reviewer resolution notes verbatim (#1624) * fix: post reviewer resolution notes verbatim Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: stabilize dashboard follow-up e2e Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: preserve dashboard attribution in e2e Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: make e2e attribution marker durable Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: only echo found e2e attribution Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: check live dashboard attribution in e2e Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> (cherry picked from commit5da3d0c657) * chore: opt-in tracemalloc to attribute unclosed aiohttp sessions (#1657) Prod logs show bursts of 'Unclosed client session' (aiohttp), leaking fds + memory, but the warning omits the allocation site. When DEBUG_TRACEMALLOC is set, start tracemalloc at webapp import so aiohttp appends an 'Object allocated at' traceback naming the exact source. Inert when the env var is unset. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> (cherry picked from commit 320bb39ab1f5cd7a2acdac52c7b375854334176c) * feat: add PR review link route (#1698) * feat: add PR review link route Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: avoid duplicate review shortcut runs Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> (cherry picked from commit 52fe29168814d7936ee6612b9c81f33339bb05b0) * style: clean up leftover blank lines from cherry-pick conflict resolution * fix(e2e): drop duplicate _ATTRIBUTION_RE from cherry-pick The reviewer-misc pick re-added _ATTRIBUTION_RE next to _latest_attribution, but the constant was already defined at module top (line 57, alongside _PLAN_URL_RE) via the earlier #81 sync. Remove the redundant redefinition; _latest_attribution resolves the surviving top-level constant. --------- Co-authored-by: Johannes du Plessis <johannes@langchain.dev> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: seahaven-openswe[bot] <296972425+seahaven-openswe[bot]@users.noreply.github.com> Co-authored-by: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Co-authored-by: Adam Moussa <adam@seahavenind.com>
This commit is contained in:
parent
0fc466a06b
commit
e0828cfaf6
4 changed files with 252 additions and 1 deletions
|
|
@ -31,7 +31,6 @@ TEAM_SETTINGS_KEY = "default"
|
|||
# prompt. Generous enough for a detailed policy, small enough to stay bounded.
|
||||
ORG_GUIDELINES_MAX_CHARS = 10_000
|
||||
REVIEW_TRACING_PROJECT_MAX_CHARS = 256
|
||||
|
||||
# Sea Haven review baseline seeded as the org-wide guidelines default. Surfaces
|
||||
# in the reviewer prompt for every repo until an admin overrides it with a
|
||||
# non-empty value via the dashboard (PUT /team-settings). Keep it stack-agnostic
|
||||
|
|
|
|||
|
|
@ -131,6 +131,26 @@ from .utils.thread_ids import generate_thread_id_from_slack_thread
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# Opt-in leak diagnostics. Bursts of aiohttp "Unclosed client session" warnings
|
||||
# (from a third-party SDK) leak fds + memory in prod, but the warning omits the
|
||||
# allocation site. With tracemalloc running, aiohttp appends an "Object allocated
|
||||
# at" traceback to each warning, naming the exact source. Inert unless the env
|
||||
# var is set, so this is safe to ship and flip on for one diagnostic run.
|
||||
if os.environ.get("DEBUG_TRACEMALLOC"):
|
||||
import tracemalloc
|
||||
|
||||
try:
|
||||
_tracemalloc_frames = int(os.environ.get("DEBUG_TRACEMALLOC_FRAMES") or "25")
|
||||
except ValueError:
|
||||
_tracemalloc_frames = 25
|
||||
tracemalloc.start(_tracemalloc_frames)
|
||||
logger.warning(
|
||||
"DEBUG_TRACEMALLOC enabled: tracemalloc started (%d frames) to attribute "
|
||||
"unclosed-session warnings",
|
||||
_tracemalloc_frames,
|
||||
)
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
|
||||
from .utils.model import validate_local_dev_llm_config
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import { Route as ReviewRepositoriesOwnerRouteImport } from './routes/review_.re
|
|||
import { Route as AgentsAutomationsNewRouteImport } from './routes/agents/automations/new'
|
||||
import { Route as AgentsAutomationsScheduleIdRouteImport } from './routes/agents/automations/$scheduleId'
|
||||
import { Route as AgentsThreadIdPlanRouteImport } from './routes/agents/$threadId_.plan'
|
||||
import { Route as OwnerRepoPullNumberRouteImport } from './routes/$owner.$repo.pull.$number'
|
||||
import { Route as AgentsReviewsOwnerRepoNumberRouteImport } from './routes/agents/reviews/$owner.$repo.$number'
|
||||
|
||||
const UsageRoute = UsageRouteImport.update({
|
||||
|
|
@ -144,6 +145,11 @@ const AgentsThreadIdPlanRoute = AgentsThreadIdPlanRouteImport.update({
|
|||
path: '/$threadId/plan',
|
||||
getParentRoute: () => AgentsRoute,
|
||||
} as any)
|
||||
const OwnerRepoPullNumberRoute = OwnerRepoPullNumberRouteImport.update({
|
||||
id: '/$owner/$repo/pull/$number',
|
||||
path: '/$owner/$repo/pull/$number',
|
||||
getParentRoute: () => rootRouteImport,
|
||||
} as any)
|
||||
const AgentsReviewsOwnerRepoNumberRoute =
|
||||
AgentsReviewsOwnerRepoNumberRouteImport.update({
|
||||
id: '/reviews/$owner/$repo/$number',
|
||||
|
|
@ -174,6 +180,7 @@ export interface FileRoutesByFullPath {
|
|||
'/review/repositories/$owner': typeof ReviewRepositoriesOwnerRoute
|
||||
'/agents/automations/': typeof AgentsAutomationsIndexRoute
|
||||
'/agents/reviews/': typeof AgentsReviewsIndexRoute
|
||||
'/$owner/$repo/pull/$number': typeof OwnerRepoPullNumberRoute
|
||||
'/agents/reviews/$owner/$repo/$number': typeof AgentsReviewsOwnerRepoNumberRoute
|
||||
}
|
||||
export interface FileRoutesByTo {
|
||||
|
|
@ -198,6 +205,7 @@ export interface FileRoutesByTo {
|
|||
'/review/repositories/$owner': typeof ReviewRepositoriesOwnerRoute
|
||||
'/agents/automations': typeof AgentsAutomationsIndexRoute
|
||||
'/agents/reviews': typeof AgentsReviewsIndexRoute
|
||||
'/$owner/$repo/pull/$number': typeof OwnerRepoPullNumberRoute
|
||||
'/agents/reviews/$owner/$repo/$number': typeof AgentsReviewsOwnerRepoNumberRoute
|
||||
}
|
||||
export interface FileRoutesById {
|
||||
|
|
@ -224,6 +232,7 @@ export interface FileRoutesById {
|
|||
'/review_/repositories/$owner': typeof ReviewRepositoriesOwnerRoute
|
||||
'/agents/automations/': typeof AgentsAutomationsIndexRoute
|
||||
'/agents/reviews/': typeof AgentsReviewsIndexRoute
|
||||
'/$owner/$repo/pull/$number': typeof OwnerRepoPullNumberRoute
|
||||
'/agents/reviews/$owner/$repo/$number': typeof AgentsReviewsOwnerRepoNumberRoute
|
||||
}
|
||||
export interface FileRouteTypes {
|
||||
|
|
@ -251,6 +260,7 @@ export interface FileRouteTypes {
|
|||
| '/review/repositories/$owner'
|
||||
| '/agents/automations/'
|
||||
| '/agents/reviews/'
|
||||
| '/$owner/$repo/pull/$number'
|
||||
| '/agents/reviews/$owner/$repo/$number'
|
||||
fileRoutesByTo: FileRoutesByTo
|
||||
to:
|
||||
|
|
@ -275,6 +285,7 @@ export interface FileRouteTypes {
|
|||
| '/review/repositories/$owner'
|
||||
| '/agents/automations'
|
||||
| '/agents/reviews'
|
||||
| '/$owner/$repo/pull/$number'
|
||||
| '/agents/reviews/$owner/$repo/$number'
|
||||
id:
|
||||
| '__root__'
|
||||
|
|
@ -300,6 +311,7 @@ export interface FileRouteTypes {
|
|||
| '/review_/repositories/$owner'
|
||||
| '/agents/automations/'
|
||||
| '/agents/reviews/'
|
||||
| '/$owner/$repo/pull/$number'
|
||||
| '/agents/reviews/$owner/$repo/$number'
|
||||
fileRoutesById: FileRoutesById
|
||||
}
|
||||
|
|
@ -318,6 +330,7 @@ export interface RootRouteChildren {
|
|||
AgentsSnapshotsRoute: typeof AgentsSnapshotsRoute
|
||||
ReviewStylesRoute: typeof ReviewStylesRoute
|
||||
ReviewRepositoriesOwnerRoute: typeof ReviewRepositoriesOwnerRoute
|
||||
OwnerRepoPullNumberRoute: typeof OwnerRepoPullNumberRoute
|
||||
}
|
||||
|
||||
declare module '@tanstack/react-router' {
|
||||
|
|
@ -476,6 +489,13 @@ declare module '@tanstack/react-router' {
|
|||
preLoaderRoute: typeof AgentsThreadIdPlanRouteImport
|
||||
parentRoute: typeof AgentsRoute
|
||||
}
|
||||
'/$owner/$repo/pull/$number': {
|
||||
id: '/$owner/$repo/pull/$number'
|
||||
path: '/$owner/$repo/pull/$number'
|
||||
fullPath: '/$owner/$repo/pull/$number'
|
||||
preLoaderRoute: typeof OwnerRepoPullNumberRouteImport
|
||||
parentRoute: typeof rootRouteImport
|
||||
}
|
||||
'/agents/reviews/$owner/$repo/$number': {
|
||||
id: '/agents/reviews/$owner/$repo/$number'
|
||||
path: '/reviews/$owner/$repo/$number'
|
||||
|
|
@ -528,6 +548,7 @@ const rootRouteChildren: RootRouteChildren = {
|
|||
AgentsSnapshotsRoute: AgentsSnapshotsRoute,
|
||||
ReviewStylesRoute: ReviewStylesRoute,
|
||||
ReviewRepositoriesOwnerRoute: ReviewRepositoriesOwnerRoute,
|
||||
OwnerRepoPullNumberRoute: OwnerRepoPullNumberRoute,
|
||||
}
|
||||
export const routeTree = rootRouteImport
|
||||
._addFileChildren(rootRouteChildren)
|
||||
|
|
|
|||
211
ui/src/routes/$owner.$repo.pull.$number.tsx
Normal file
211
ui/src/routes/$owner.$repo.pull.$number.tsx
Normal file
|
|
@ -0,0 +1,211 @@
|
|||
import { Link, Navigate, createFileRoute } from "@tanstack/react-router"
|
||||
import { useEffect, useMemo, useRef } from "react"
|
||||
import { ArrowSquareOutIcon, GitPullRequestIcon } from "@phosphor-icons/react"
|
||||
import { useMutation, useQuery } from "@tanstack/react-query"
|
||||
|
||||
import { buttonVariants } from "@/components/ui/button"
|
||||
import {
|
||||
Card,
|
||||
CardContent,
|
||||
CardDescription,
|
||||
CardFooter,
|
||||
CardHeader,
|
||||
CardTitle,
|
||||
} from "@/components/ui/card"
|
||||
import { Skeleton } from "@/components/ui/skeleton"
|
||||
import { api } from "@/lib/api"
|
||||
import { RequireLogin } from "@/lib/auth-redirect"
|
||||
import { useSession } from "@/lib/session"
|
||||
import { cn } from "@/lib/utils"
|
||||
|
||||
export const Route = createFileRoute("/$owner/$repo/pull/$number")({
|
||||
component: PullRequestReviewLinkPage,
|
||||
})
|
||||
|
||||
function PullRequestReviewLinkPage() {
|
||||
const { owner, repo, number } = Route.useParams()
|
||||
const prNumber = Number(number)
|
||||
const session = useSession()
|
||||
const stableReviewPath = `/agents/reviews/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${prNumber}`
|
||||
const githubPrUrl = useMemo(
|
||||
() => `https://github.com/${owner}/${repo}/pull/${number}`,
|
||||
[owner, repo, number]
|
||||
)
|
||||
const existingReview = useQuery({
|
||||
queryKey: ["review", owner, repo, prNumber],
|
||||
queryFn: () => api.getReview(owner, repo, prNumber),
|
||||
enabled: !!session.data && Number.isFinite(prNumber),
|
||||
retry: false,
|
||||
})
|
||||
const triggerRef = useRef<string | null>(null)
|
||||
const triggerReview = useMutation({
|
||||
mutationFn: () => api.reReview(owner, repo, prNumber),
|
||||
})
|
||||
|
||||
useEffect(() => {
|
||||
if (!session.data || !Number.isFinite(prNumber)) return
|
||||
if (existingReview.isLoading || existingReview.data?.status === "running") {
|
||||
return
|
||||
}
|
||||
const key = `${owner}/${repo}#${prNumber}`
|
||||
if (triggerRef.current === key) return
|
||||
triggerRef.current = key
|
||||
triggerReview.mutate()
|
||||
}, [
|
||||
existingReview.data?.status,
|
||||
existingReview.isLoading,
|
||||
owner,
|
||||
repo,
|
||||
prNumber,
|
||||
session.data,
|
||||
triggerReview,
|
||||
])
|
||||
|
||||
if (session.isLoading) {
|
||||
return (
|
||||
<main className="flex min-h-svh items-center justify-center p-6">
|
||||
<Skeleton className="h-52 w-full max-w-lg" />
|
||||
</main>
|
||||
)
|
||||
}
|
||||
|
||||
if (!session.data) return <RequireLogin />
|
||||
|
||||
if (!Number.isFinite(prNumber)) {
|
||||
return (
|
||||
<ReviewLinkCard
|
||||
title="Invalid pull request link"
|
||||
description="Expected a GitHub-style pull request path like /owner/repo/pull/123."
|
||||
owner={owner}
|
||||
repo={repo}
|
||||
number={number}
|
||||
githubPrUrl={githubPrUrl}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
if (existingReview.data?.status === "running" || triggerReview.isSuccess) {
|
||||
return (
|
||||
<Navigate
|
||||
to="/agents/reviews/$owner/$repo/$number"
|
||||
params={{ owner, repo, number: String(prNumber) }}
|
||||
replace
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
if (triggerReview.isError) {
|
||||
return (
|
||||
<ReviewLinkCard
|
||||
title="Could not start review"
|
||||
description={triggerReview.error.message}
|
||||
owner={owner}
|
||||
repo={repo}
|
||||
number={number}
|
||||
githubPrUrl={githubPrUrl}
|
||||
stableReviewPath={stableReviewPath}
|
||||
onRetry={() => triggerReview.mutate()}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
const isCheckingExistingReview = existingReview.isLoading
|
||||
|
||||
return (
|
||||
<ReviewLinkCard
|
||||
title={
|
||||
isCheckingExistingReview
|
||||
? "Checking review status"
|
||||
: "Starting Open SWE review"
|
||||
}
|
||||
description={
|
||||
isCheckingExistingReview
|
||||
? "This PR link was recognized. Open SWE is checking whether a review is already running."
|
||||
: "Open SWE is starting a review and will redirect you to the stable review page."
|
||||
}
|
||||
owner={owner}
|
||||
repo={repo}
|
||||
number={number}
|
||||
githubPrUrl={githubPrUrl}
|
||||
stableReviewPath={stableReviewPath}
|
||||
loading
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
function ReviewLinkCard({
|
||||
title,
|
||||
description,
|
||||
owner,
|
||||
repo,
|
||||
number,
|
||||
githubPrUrl,
|
||||
stableReviewPath,
|
||||
loading = false,
|
||||
onRetry,
|
||||
}: {
|
||||
title: string
|
||||
description: string
|
||||
owner: string
|
||||
repo: string
|
||||
number: string
|
||||
githubPrUrl: string
|
||||
stableReviewPath?: string
|
||||
loading?: boolean
|
||||
onRetry?: () => void
|
||||
}) {
|
||||
return (
|
||||
<main className="flex min-h-svh items-center justify-center bg-background p-6 text-foreground">
|
||||
<Card className="w-full max-w-lg">
|
||||
<CardHeader>
|
||||
<CardTitle className="flex items-center gap-2 text-xl">
|
||||
<GitPullRequestIcon className="size-5 text-muted-foreground" />
|
||||
{title}
|
||||
</CardTitle>
|
||||
<CardDescription>{description}</CardDescription>
|
||||
</CardHeader>
|
||||
<CardContent>
|
||||
<div className="rounded-lg border border-border bg-muted/40 p-3 text-sm">
|
||||
<div className="font-medium">
|
||||
{owner}/{repo} #{number}
|
||||
</div>
|
||||
<a
|
||||
href={githubPrUrl}
|
||||
className="mt-1 inline-flex items-center gap-1 text-muted-foreground hover:text-foreground"
|
||||
>
|
||||
View on GitHub
|
||||
<ArrowSquareOutIcon className="size-3.5" />
|
||||
</a>
|
||||
</div>
|
||||
{loading && <Skeleton className="mt-4 h-2 w-full" />}
|
||||
</CardContent>
|
||||
<CardFooter className="flex flex-wrap gap-2">
|
||||
{onRetry && (
|
||||
<button
|
||||
type="button"
|
||||
className={buttonVariants()}
|
||||
onClick={onRetry}
|
||||
>
|
||||
Try again
|
||||
</button>
|
||||
)}
|
||||
{stableReviewPath && (
|
||||
<Link
|
||||
to="/agents/reviews/$owner/$repo/$number"
|
||||
params={{ owner, repo, number }}
|
||||
className={cn(buttonVariants({ variant: "outline" }))}
|
||||
>
|
||||
Open stable review page
|
||||
</Link>
|
||||
)}
|
||||
<a
|
||||
href={githubPrUrl}
|
||||
className={cn(buttonVariants({ variant: "ghost" }))}
|
||||
>
|
||||
Open GitHub PR
|
||||
</a>
|
||||
</CardFooter>
|
||||
</Card>
|
||||
</main>
|
||||
)
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue