adam/open-swe
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/open-swe.git synced 2026-09-30 05:43:14 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

chore: cherry-pick clean upstream fixes + cherry-pick runbook (#117)

Browse source 
* fix: make plan view mobile friendly (#1636)

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit 7ee3e05724)

* fix: return to thread after plan approval (#1637)

Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com>
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit f32e492ab4)

* feat: reviews block agenda, sticky headers, accurate diff scroll (#1653)

Rework the AI-sorted blocks experience on the PR reviews page into a
Google-Docs-style outline: the left sidebar is now a clean number+title
agenda with scroll-spy highlighting of the active block; each block shows
its title + description (sticky) above its diff; and diff rows are pinned to
a uniform height so scroll-to lands precisely via the virtualizer's own
geometry instead of an estimate-driven correction loop.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit 0b76afdc955e33805c7623d1502a75a9c7c9c1b7)

* fix: jump + ResizeObserver settle for review scroll-to (#1655)

Replace smooth-scroll plus frame-count correction loops on the PR
reviews page with an instant jump that re-asserts its target via a
ResizeObserver (the real "layout settled" signal). Block/file
navigation and finding/comment centering now land deterministically as
off-screen cards mount, files expand, and annotation cards measure,
instead of racing a smooth-scroll animation against height
reconciliation. Holds bail on user wheel/touch input and after a short
ceiling, and a new navigation cancels the previous hold.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
Co-authored-by: Johannes du Plessis <johannes@langchain.dev>
(cherry picked from commit 7530653bba7774d66a54b8bef0d2bbc25f519942)

* fix: purge expired thread_wakeup crons (#1656)

* fix: purge expired thread_wakeup crons

One-shot wakeup crons set an end_time that stops re-firing but the cron
row is never deleted, so dead rows accumulate (86 in prod). Add a purge
that deletes thread_wakeup crons past their end_time, called
opportunistically before scheduling a new wakeup, plus a one-time
backfill script. Conservative: matches only kind=thread_wakeup with a
past end_time.

* chore: retrigger Open SWE review

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit 9e5a1924ef306269322c31342a1831e57831cfee)

* fix: add top padding to sticky review block header (#1660)

* fix: add top padding to sticky review block header

The sticky per-block header on the reviews page had padding below but
none above, so the block number badge sat glued against the top edge
when pinned. Add matching top padding for breathing room.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

* chore: use py-2 shorthand for review block header padding

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 23bd4a63fc5ba0fe853babf79ed33feb866cc8b2)

* fix: use global tokens for sidebar filter popover border (#1661)

The filter popover renders via base-ui Menu.Portal into document.body,
outside the .agents-ui container where the --ui-* CSS variables are
scoped. As a result border-[var(--ui-border)] resolved to an undefined
variable and border-color fell back to currentColor, producing a strong
near-black border (separators/hover/labels were similarly off).

Switch the portaled popup styling to the same global shadcn tokens the
theme/settings popover (SidebarUserMenu) already uses (border-border,
bg-border, bg-muted, text-muted-foreground). These are defined at :root
so they resolve inside portals too, and match the settings popover.

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit 63eb9a08209f683016abf01cdcc548bc5905f158)

* fix: preserve dashboard redirect after login (#1668)

* fix: preserve dashboard redirect after login

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

* test: cover plan login redirect in e2e

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>

---------

Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com>
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit bc7ce59169b5350da7286164afb83a7b037b528d)

* Disable React StrictMode (#1654)

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit 6575c327a3ac2b107a6e79a04fa61168d779dbf0)

* docs(upstream-sync): add cherry-pick runbook

Repo-specific runbook for bringing upstream (langchain-ai/open-swe) commits
into the fork: triage-sync discovery, the git cp workflow, the triage ledger,
themed-branch layout, and conflict/regression handling.

---------

Co-authored-by: Johannes du Plessis <johannes@langchain.dev>
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
Co-authored-by: Ramon Nogueira <ramon.nogueira@langchain.dev>
Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com>
Co-authored-by: Caroline di Vittorio <43390382+carolinedivittorio@users.noreply.github.com>
This commit is contained in:
Adam Moussa 2026-07-03 11:48:40 -04:00 • committed by GitHub
parent dfdd41879c
commit 589cd236c6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
32 changed files with 1139 additions and 291 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

70
agent/tools/schedule_thread_wakeup.py
View file

@ -18,6 +18,9 @@ _MIN_DELAY_SECONDS = 60
_MAX_DELAY_SECONDS = 86_400 _MAX_DELAY_SECONDS = 86_400
_END_TIME_PADDING_SECONDS = 90 _END_TIME_PADDING_SECONDS = 90
_WAKEUP_KIND = "thread_wakeup"
_PURGE_PAGE_SIZE = 100
_DEFAULT_WAKEUP_PROMPT = ( _DEFAULT_WAKEUP_PROMPT = (
"This is an automated re-trigger of this thread. The agent scheduled this " "This is an automated re-trigger of this thread. The agent scheduled this "
"wakeup to poll for updates. Check the current state of whatever you were " "wakeup to poll for updates. Check the current state of whatever you were "
@ -46,6 +49,71 @@ def _build_one_shot_cron(fire_time: datetime) -> str:
) )
def _parse_iso(value: Any) -> datetime | None:
if not isinstance(value, str) or not value:
return None
try:
return datetime.fromisoformat(value.replace("Z", "+00:00"))
except ValueError:
return None
async def find_expired_wakeup_cron_ids(client: Any, *, now: datetime) -> list[str]:
"""Return the ids of ``thread_wakeup`` crons whose ``end_time`` has passed.
Conservative: matches solely on ``metadata.kind == "thread_wakeup"`` AND a
past ``end_time``, so analyzer/dashboard crons are never selected. Paginates
fully before returning so the result is stable to delete afterwards.
"""
expired_ids: list[str] = []
offset = 0
while True:
page = await client.crons.search(
metadata={"kind": _WAKEUP_KIND},
limit=_PURGE_PAGE_SIZE,
offset=offset,
)
if not page:
break
for cron in page:
if not isinstance(cron, dict):
continue
end_time = _parse_iso(cron.get("end_time"))
cron_id = cron.get("cron_id")
if end_time is not None and end_time < now and isinstance(cron_id, str) and cron_id:
expired_ids.append(cron_id)
if len(page) < _PURGE_PAGE_SIZE:
break
offset += len(page)
return expired_ids
async def purge_expired_wakeup_crons(client: Any, *, now: datetime) -> int:
"""Delete ``thread_wakeup`` crons whose ``end_time`` has already passed.
Each wakeup is a thread-bound cron with an ``end_time`` (~90s past its fire)
that stops it re-firing, but the cron row itself is never removed, so dead
rows accumulate. This deletes only those dead rows. Returns the count deleted.
"""
expired_ids = await find_expired_wakeup_cron_ids(client, now=now)
deleted = 0
for cron_id in expired_ids:
await client.crons.delete(cron_id)
deleted += 1
return deleted
async def _purge_expired_wakeups_best_effort() -> None:
"""Opportunistically purge expired wakeup crons; never raises."""
try:
client = get_client(url=langgraph_url())
deleted = await purge_expired_wakeup_crons(client, now=datetime.now(UTC))
if deleted:
logger.info("Purged %d expired thread_wakeup cron(s)", deleted)
except Exception:
logger.warning("Failed to purge expired thread_wakeup crons", exc_info=True)
async def _create_wakeup_cron( async def _create_wakeup_cron(
*, *,
thread_id: str, thread_id: str,
@ -130,6 +198,8 @@ async def schedule_thread_wakeup(delay_minutes: int, prompt: str | None = None)
if value is not None: if value is not None:
wakeup_configurable[key] = value wakeup_configurable[key] = value
await _purge_expired_wakeups_best_effort()
try: try:
return await _create_wakeup_cron( return await _create_wakeup_cron(
thread_id=thread_id, thread_id=thread_id,

95
docs/upstream-sync/cherry-pick-runbook.md Normal file
View file

@ -0,0 +1,95 @@
# Cherry-picking upstream into the fork
This repository is a long-lived fork of `langchain-ai/open-swe` (git remote `upstream`).
Upstream changes are brought in one commit at a time with `git cherry-pick`, and every
diverged commit is tracked in a triage ledger so a decision is made once and not revisited.
`dev` is the integration branch; the broader strategy lives in the fork-maintenance section
of `CLAUDE.md`.
## Setup (once per clone)
make install-hooks
Installs the triage hooks and the `git cp` alias by pointing `core.hooksPath` at `.githooks/`.
Because that shadows the machine-global hook directory (`~/.config/git/hooks`, which holds the
mandatory security `pre-push`), `.githooks/pre-push` is a shim that re-execs the global hook,
and the installer verifies that delegation before it changes anything. `git` never auto-adopts
a repository's `core.hooksPath`, so this step cannot be skipped.
Note: `core.hooksPath` applies repo-wide, but `.githooks/` is a tracked directory. The hooks
(and the security shim) only run on branches that actually contain `.githooks/`. Keep it present
on `dev` and `main` so no branch loses the security `pre-push`.
## Finding what to pick
make triage-sync # git fetch upstream, then append new dev..upstream/main commits
# to the ledger as `untriaged` (PR # + subject parsed from each)
`triage-sync` is the discovery step: it records every diverged commit as `untriaged` and bumps
"Last synced" to the new `upstream/main` tip. Triage those rows (decide `deferred` / `wont-merge`
and which branch), then pick the ones you want. The underlying views if you prefer raw git:
git fetch upstream
git log --oneline --no-merges dev..upstream/main # everything diverged
git show <sha> # inspect before deciding
Cross-check candidates against the ledger first — most diverged commits already carry a
decision (already-in-dev, regression, deferred, or landed) and should not be re-examined.
## Bringing in commits: `git cp`
git cp -x <sha> # pre-check the ledger, cherry-pick -x, auto-reconcile
git cp -x <sha1> <sha2> ... # several, applied in the given order
git cp --continue # after resolving a conflict; also reconciles
git cp --force <sha> # override a SHA the ledger marks "Won't merge"
`git cp` reads `docs/upstream-sync/triage.jsonl` before touching the tree and refuses a
known-reject SHA (override with `--force`). On success it runs `make triage-reconcile`, which
moves each applied SHA to Landed in the ledger and stages `triage.jsonl` + `triage.md` for you
to commit.
Apply commits in upstream chronological order (oldest first), not the order you happen to list
them — a later commit often depends on an earlier one, and out-of-order picks conflict
needlessly:
git log --reverse --topo-order --format=%h dev..upstream/main
## The triage ledger
`docs/upstream-sync/triage.jsonl` is the source of truth: one JSON row per upstream SHA, keyed
on the SHA (stable, unlike the local SHAs cherry-pick rewrites). `docs/upstream-sync/triage.md`
is generated from it and must not be hand-edited. Dispositions are `landed`, `wont-merge`,
`deferred`, `untriaged`.
scripts/triage.py set <sha> --disposition deferred --branch slack-tooling --reason "..."
make triage-render # regenerate triage.md from the jsonl
make triage-check # CI gate: fail if triage.md is stale
A SHA marked `wont-merge` is hard-blocked by both `git cp` and the `prepare-commit-msg` hook.
Override for a one-off re-evaluation with `git cp --force`, `SH_CHERRYPICK_ALLOW_REJECT=1`, or
`git config sh.cherrypick.blockRejects false`.
## Branch layout
Never cherry-pick onto `dev` directly. Work on a themed branch off `dev` and open a PR into
`dev`; the ledger's `branch` column records where each deferred commit is meant to land
(for example `slack-tooling`, `gateway-routing`, `plan-approval`, `durable-dispatch`). Keep
each PR to one theme so conflict resolution stays within one subsystem.
## Raw `git cherry-pick`
The hooks fire on a plain `git cherry-pick -x <sha>` too: `post-commit` journals each applied
pick and `prepare-commit-msg` blocks known-rejects. Run `make triage-reconcile` once at the end
to land the picks in the ledger, then commit `triage.jsonl` + `triage.md`.
## Conflicts
# resolve the files, then:
git add <files>
git cherry-pick --continue # or: git cp --continue
git cherry-pick --abort # bail out of the whole pick
git cherry-pick --skip # drop just this commit and continue the batch
A commit that conflicts because `dev` already carries a newer version of the same code is a
regression, not a merge — skip it and record the decision as `wont-merge` in the ledger rather
than forcing it in.

88
scripts/purge_wakeup_crons.py Normal file
View file

@ -0,0 +1,88 @@
"""One-time backfill: delete expired ``thread_wakeup`` crons from a deployment.
One-shot wakeup crons set an ``end_time`` that stops them re-firing, but the
cron row is never removed, so dead rows accumulate. The ``schedule_thread_wakeup``
tool now purges these opportunistically; this script clears the backlog.
Usage:
uv run python scripts/purge_wakeup_crons.py --dry-run
uv run python scripts/purge_wakeup_crons.py
Resolves the deployment URL from ``--url`` or ``LANGGRAPH_URL`` / ``LANGGRAPH_URL_PROD``,
and the API key from ``LANGGRAPH_API_KEY`` / ``LANGSMITH_API_KEY`` / ``LANGSMITH_API_KEY_PROD``.
"""
from __future__ import annotations
import argparse
import asyncio
import logging
import os
from datetime import UTC, datetime
from langgraph_sdk import get_client
from agent.tools.schedule_thread_wakeup import (
find_expired_wakeup_cron_ids,
purge_expired_wakeup_crons,
)
logger = logging.getLogger(__name__)
def _load_dotenv_if_available() -> None:
try:
from dotenv import load_dotenv
except ImportError:
return
load_dotenv()
def _resolve_url(arg_url: str | None) -> str:
url = arg_url or os.environ.get("LANGGRAPH_URL") or os.environ.get("LANGGRAPH_URL_PROD")
if not url:
raise RuntimeError("Set --url or LANGGRAPH_URL / LANGGRAPH_URL_PROD")
return url
def _resolve_api_key() -> str | None:
return (
os.environ.get("LANGGRAPH_API_KEY")
or os.environ.get("LANGSMITH_API_KEY")
or os.environ.get("LANGSMITH_API_KEY_PROD")
)
async def _run(url: str, api_key: str | None, dry_run: bool) -> None:
client = get_client(url=url, api_key=api_key)
now = datetime.now(UTC)
if dry_run:
expired = await find_expired_wakeup_cron_ids(client, now=now)
logger.info("[dry-run] %d expired thread_wakeup cron(s) would be deleted", len(expired))
for cron_id in expired:
logger.info(" %s", cron_id)
return
deleted = await purge_expired_wakeup_crons(client, now=now)
logger.info("Deleted %d expired thread_wakeup cron(s)", deleted)
def parse_args() -> argparse.Namespace:
parser = argparse.ArgumentParser(description="Purge expired thread_wakeup crons.")
parser.add_argument("--url", default=None, help="Deployment URL (defaults to env).")
parser.add_argument(
"--dry-run",
action="store_true",
help="List the crons that would be deleted without deleting them.",
)
return parser.parse_args()
def main() -> None:
_load_dotenv_if_available()
logging.basicConfig(level=logging.INFO, format="%(message)s")
args = parse_args()
asyncio.run(_run(_resolve_url(args.url), _resolve_api_key(), args.dry_run))
if __name__ == "__main__":
main()

22
tests/e2e/README.md
View file

@ -12,17 +12,17 @@ This drives the **whole happy path** through two mock UIs:
Only the **LLM** and the **external SaaS HTTP boundaries** are faked. All agent Only the **LLM** and the **external SaaS HTTP boundaries** are faked. All agent
code runs for real. code runs for real.
| Piece | Real or fake | | Piece | Real or fake |
|---|---| | ---------------------------------------------------------------- | -------------------------------------------------------------------------- |
| Slack webhook → `process_slack_mention` → run dispatch | **real** (`agent.webapp`) | | Slack webhook → `process_slack_mention` → run dispatch | **real** (`agent.webapp`) |
| `get_agent`, deepagents loop, tools, middleware, prompt | **real** | | `get_agent`, deepagents loop, tools, middleware, prompt | **real** |
| `open_pull_request`, `slack_thread_reply` tools | **real** | | `open_pull_request`, `slack_thread_reply` tools | **real** |
| Sandbox | **real** `local` provider, rooted in a throwaway temp dir | | Sandbox | **real** `local` provider, rooted in a throwaway temp dir |
| Git remote ("GitHub") | **real git**, a local bare repo the agent clones/pushes | | Git remote ("GitHub") | **real git**, a local bare repo the agent clones/pushes |
| The LLM | **fake** — a scripted model (`fake_llm.py`) emitting a fixed tool sequence | | The LLM | **fake** — a scripted model (`fake_llm.py`) emitting a fixed tool sequence |
| `api.github.com` REST (PR create) | **fake** (`/fake-gh/...`), state rendered at `/mock/github` | | `api.github.com` REST (PR create) + dashboard GitHub OAuth login | **fake** (`/fake-gh/...`), state rendered at `/mock/github` |
| `slack.com/api` (post message, etc.) | **fake** (`/fake-slack/...`), thread rendered at `/mock/slack` | | `slack.com/api` (post message, etc.) | **fake** (`/fake-slack/...`), thread rendered at `/mock/slack` |
| GitHub App token mint, `api.github.com/user` identity | stubbed (offline) | | GitHub App token mint, `api.github.com/user` identity | stubbed (offline) |
The fake GitHub/Slack stores are the single source of truth the mock UIs render, The fake GitHub/Slack stores are the single source of truth the mock UIs render,
so what Playwright asserts on is exactly what the real agent produced. so what Playwright asserts on is exactly what the real agent produced.

48
tests/e2e/harness.py
View file

@ -18,8 +18,10 @@ import json
import os import os
import sys import sys
import time import time
from html import escape
from pathlib import Path from pathlib import Path
from typing import Any from typing import Any
from urllib.parse import quote
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
@ -185,29 +187,43 @@ async def control_login_get(login: str = "", email: str = "", next_url: str = ""
@app.get("/dashboard/api/auth/login") @app.get("/dashboard/api/auth/login")
async def mock_github_login(redirect_to: str = "", login: str = "") -> Response: async def mock_github_login(redirect_to: str = "") -> Response:
"""Mock stand-in for GitHub OAuth: the dashboard's "Continue with GitHub" """E2E stand-in for the dashboard OAuth start route.
button lands here. With no ``login``, render a picker of the fake GitHub
test users; once one is chosen, mint the real session cookie and redirect The real route would redirect to github.com. Keep the dashboard-facing URL
back into the dashboard (``redirect_to``).""" intact, then hand off to the fake GitHub simulator so Playwright exercises a
browser login flow instead of test code pre-minting a session cookie.
"""
ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/")
dest = redirect_to or (f"{ui}/agents" if ui else "/agents")
return RedirectResponse(f"/fake-gh/login/oauth/authorize?redirect_to={quote(dest)}", 302)
@app.get("/fake-gh/login/oauth/authorize")
async def fake_github_authorize(redirect_to: str = "", login: str = "") -> Response:
"""Fake GitHub OAuth consent/login page for dashboard e2e tests."""
ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/") ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/")
dest = redirect_to or (f"{ui}/agents" if ui else "/agents") dest = redirect_to or (f"{ui}/agents" if ui else "/agents")
if not login: if not login:
options = "".join( options = "".join(
f'<option value="{u["login"]}">{u["name"]} (@{u["login"]})</option>' for u in TEST_USERS f'<option value="{escape(u["login"], quote=True)}">'
f"{escape(u['name'])} (@{escape(u['login'])})</option>"
for u in TEST_USERS
) )
return HTMLResponse( return HTMLResponse(
f"""<!doctype html><meta charset=utf-8><title>Continue with GitHub (mock)</title> f"""<!doctype html><meta charset=utf-8><title>GitHub · Authorize open-swe</title>
<body style="font-family:system-ui;max-width:420px;margin:3rem auto;padding:0 1rem"> <body style="font-family:system-ui;max-width:420px;margin:3rem auto;padding:0 1rem">
<h1 style="font-size:1.1rem">Continue with GitHub (mock)</h1> <main data-testid="fake-github-login">
<p style="color:#888;font-size:0.9rem">Pick a fake GitHub account to sign in as.</p> <h1 style="font-size:1.1rem">Authorize open-swe</h1>
<form method=get action=/dashboard/api/auth/login> <p style="color:#888;font-size:0.9rem">Pick a fake GitHub account to continue.</p>
<input type=hidden name=redirect_to value="{dest}"> <form method=get action=/fake-gh/login/oauth/authorize>
<select name=login style="font:inherit;padding:0.4rem">{options}</select> <input type=hidden name=redirect_to value="{escape(dest, quote=True)}">
<button style="font:inherit;padding:0.45rem 0.9rem;cursor:pointer">Continue</button> <label>GitHub user
</form> <select name=login style="font:inherit;padding:0.4rem">{options}</select>
<p style="color:#888;font-size:0.85rem">Tip: use a separate browser or profile per </label>
user so their sessions don't overwrite each other.</p> <button style="font:inherit;padding:0.45rem 0.9rem;cursor:pointer">Authorize open-swe</button>
</form>
</main>
</body>""" </body>"""
) )
match = next((u for u in TEST_USERS if u["login"] == login), None) match = next((u for u in TEST_USERS if u["login"] == login), None)

92
tests/e2e/tests/plan_review.spec.ts
View file

@ -38,9 +38,14 @@ test.describe("Plan review (HTTP comments)", () => {
// 1. A user asks the bot to PLAN something in Slack. // 1. A user asks the bot to PLAN something in Slack.
await request.post("/control/reset"); await request.post("/control/reset");
const send = await request.post("/mock/slack/send", { const send = await request.post("/mock/slack/send", {
data: { text: "<@U0BOT> plan how to add a greet() helper", mention_bot: true }, data: {
text: "<@U0BOT> plan how to add a greet() helper",
mention_bot: true,
},
}); });
const { thread_id: threadId } = (await send.json()) as { thread_id: string }; const { thread_id: threadId } = (await send.json()) as {
thread_id: string;
};
expect(threadId).toBeTruthy(); expect(threadId).toBeTruthy();
const planPath = `/agents/${threadId}/plan`; const planPath = `/agents/${threadId}/plan`;
@ -57,7 +62,9 @@ test.describe("Plan review (HTTP comments)", () => {
}; };
return (state.values?.messages ?? []) return (state.values?.messages ?? [])
.map((m) => .map((m) =>
typeof m.content === "string" ? m.content : JSON.stringify(m.content), typeof m.content === "string"
? m.content
: JSON.stringify(m.content),
) )
.some((c) => c.includes("Plan mode is active")); .some((c) => c.includes("Plan mode is active"));
}, },
@ -67,13 +74,45 @@ test.describe("Plan review (HTTP comments)", () => {
// 2. The agent shares the plan-review link, then announces the plan is ready. // 2. The agent shares the plan-review link, then announces the plan is ready.
await expect await expect
.poll(async () => (await botMessages(request)).join("\n"), { timeout: 60_000 }) .poll(async () => (await botMessages(request)).join("\n"), {
timeout: 60_000,
})
.toMatch(/\/agents\/[^/]+\/plan\b/); .toMatch(/\/agents\/[^/]+\/plan\b/);
await expect await expect
.poll(async () => (await botMessages(request)).join("\n"), { timeout: 60_000 }) .poll(async () => (await botMessages(request)).join("\n"), {
timeout: 60_000,
})
.toMatch(/ready for review/i); .toMatch(/ready for review/i);
// 3. The OWNER opens the conversation, follows the "Review plan" banner, and // 3. A logged-out user follows the plan deep link, signs in through the fake
// GitHub OAuth simulator, and lands back on the same plan page.
const loggedOutCtx = await browser.newContext();
const loggedOut = await loggedOutCtx.newPage();
await loggedOut.goto(planPath);
await expect(loggedOut).toHaveURL(
new RegExp(`/login\\?redirect=.*${threadId}.*plan`),
);
await expect(loggedOut.getByText("Sign in to open-swe")).toBeVisible({
timeout: 30_000,
});
await loggedOut.getByRole("link", { name: "Continue with GitHub" }).click();
await expect(loggedOut).toHaveURL(/\/fake-gh\/login\/oauth\/authorize/);
await expect(loggedOut.getByTestId("fake-github-login")).toBeVisible();
await loggedOut.getByLabel("GitHub user").selectOption(OWNER.login);
await loggedOut.getByRole("button", { name: "Authorize open-swe" }).click();
await expect(loggedOut).toHaveURL(new RegExp(`/agents/${threadId}/plan$`));
await expect(loggedOut.getByTestId("plan-review")).toBeVisible({
timeout: 30_000,
});
await expect(loggedOut.getByTestId("plan-document")).toContainText(
"greet",
{
timeout: 30_000,
},
);
await loggedOutCtx.close();
// 4. The OWNER opens the conversation, follows the "Review plan" banner, and
// sees the rendered plan. // sees the rendered plan.
const ownerCtx = await browser.newContext({ const ownerCtx = await browser.newContext({
permissions: ["clipboard-read", "clipboard-write"], permissions: ["clipboard-read", "clipboard-write"],
@ -85,7 +124,9 @@ test.describe("Plan review (HTTP comments)", () => {
await expect(reviewLink).toBeVisible({ timeout: 30_000 }); await expect(reviewLink).toBeVisible({ timeout: 30_000 });
await reviewLink.click(); await reviewLink.click();
await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}/plan$`)); await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}/plan$`));
await expect(owner.getByTestId("plan-review")).toBeVisible({ timeout: 30_000 }); await expect(owner.getByTestId("plan-review")).toBeVisible({
timeout: 30_000,
});
await expect(owner.getByText("Back to conversation")).toBeVisible(); await expect(owner.getByText("Back to conversation")).toBeVisible();
await expect(owner.getByTestId("plan-document")).toContainText("greet", { await expect(owner.getByTestId("plan-document")).toContainText("greet", {
timeout: 30_000, timeout: 30_000,
@ -98,7 +139,9 @@ test.describe("Plan review (HTTP comments)", () => {
// Copy the whole plan as markdown. // Copy the whole plan as markdown.
await owner.getByTestId("copy-plan").click(); await owner.getByTestId("copy-plan").click();
await expect(owner.getByTestId("copy-plan")).toContainText("Copied!"); await expect(owner.getByTestId("copy-plan")).toContainText("Copied!");
const clipboard = await owner.evaluate(() => navigator.clipboard.readText()); const clipboard = await owner.evaluate(() =>
navigator.clipboard.readText(),
);
expect(clipboard).toContain("## Plan: Add greet() helper"); expect(clipboard).toContain("## Plan: Add greet() helper");
expect(clipboard).toContain("### Verification"); expect(clipboard).toContain("### Verification");
@ -107,18 +150,24 @@ test.describe("Plan review (HTTP comments)", () => {
await expect(owner.getByTestId("plan-comment")).toHaveCount(1); await expect(owner.getByTestId("plan-comment")).toHaveCount(1);
await expect(owner.getByTestId("reject-plan")).toBeEnabled(); await expect(owner.getByTestId("reject-plan")).toBeEnabled();
// 4. A COLLABORATOR opens the same plan: sees it AND the owner's comment // 5. A COLLABORATOR opens the same plan: sees it AND the owner's comment
// (fetched over HTTP), but has NO approve button. // (fetched over HTTP), but has NO approve button.
const collabCtx = await browser.newContext(); const collabCtx = await browser.newContext();
await collabCtx.request.post("/control/login", { data: COLLABORATOR }); await collabCtx.request.post("/control/login", { data: COLLABORATOR });
const collab = await collabCtx.newPage(); const collab = await collabCtx.newPage();
await collab.goto(planPath); await collab.goto(planPath);
await expect(collab.getByTestId("plan-review")).toBeVisible({ timeout: 30_000 }); await expect(collab.getByTestId("plan-review")).toBeVisible({
timeout: 30_000,
});
await expect(collab.getByTestId("plan-document")).toContainText("greet", { await expect(collab.getByTestId("plan-document")).toContainText("greet", {
timeout: 30_000, timeout: 30_000,
}); });
await expect(collab.getByTestId("plan-comment")).toHaveCount(1, { timeout: 30_000 }); await expect(collab.getByTestId("plan-comment")).toHaveCount(1, {
await expect(collab.getByTestId("plan-comment")).toContainText("looks solid"); timeout: 30_000,
});
await expect(collab.getByTestId("plan-comment")).toContainText(
"looks solid",
);
await expect(collab.getByTestId("approve-plan")).toHaveCount(0); await expect(collab.getByTestId("approve-plan")).toHaveCount(0);
await expect(collab.getByTestId("reject-plan")).toBeVisible(); await expect(collab.getByTestId("reject-plan")).toBeVisible();
@ -126,20 +175,27 @@ test.describe("Plan review (HTTP comments)", () => {
await addComment(collab, "Reviewer: please also add a docstring."); await addComment(collab, "Reviewer: please also add a docstring.");
await expect(collab.getByTestId("plan-comment")).toHaveCount(2); await expect(collab.getByTestId("plan-comment")).toHaveCount(2);
// 5. The owner sees the collaborator's comment (polled), then approves. // 6. The owner sees the collaborator's comment (polled), then approves and
await expect(owner.getByTestId("plan-comment")).toHaveCount(2, { timeout: 30_000 }); // returns to the main conversation while implementation starts.
await expect(owner.getByTestId("plan-comment")).toHaveCount(2, {
timeout: 30_000,
});
await owner.getByTestId("approve-plan").click(); await owner.getByTestId("approve-plan").click();
await expect(owner.getByTestId("plan-decision")).toContainText(/implementing/i); await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}$`));
// 6. The agent implements, opens a PR, and links it back in the Slack thread, // 7. The agent implements, opens a PR, and links it back in the Slack thread,
// echoing the reviewers' feedback — which proves the comments were stored // echoing the reviewers' feedback — which proves the comments were stored
// and harvested server-side on approve. // and harvested server-side on approve.
await expect await expect
.poll(async () => (await botMessages(request)).join("\n"), { timeout: 90_000 }) .poll(async () => (await botMessages(request)).join("\n"), {
timeout: 90_000,
})
.toMatch(/\/pull\//); .toMatch(/\/pull\//);
expect((await botMessages(request)).join("\n")).toMatch(/docstring/); expect((await botMessages(request)).join("\n")).toMatch(/docstring/);
const prs = (await (await request.get("/mock/github/data")).json()) as Array<unknown>; const prs = (await (
await request.get("/mock/github/data")
).json()) as Array<unknown>;
expect(prs.length).toBeGreaterThan(0); expect(prs.length).toBeGreaterThan(0);
await ownerCtx.close(); await ownerCtx.close();

30
tests/test_dashboard_oauth_redirect.py Normal file
View file

@ -0,0 +1,30 @@
from __future__ import annotations
from agent.dashboard.oauth import sanitize_redirect_to
def test_sanitize_redirect_to_preserves_allowed_dashboard_target(monkeypatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example")
target = "https://dashboard.example/agents/thread-1/plan?from=slack#review"
assert sanitize_redirect_to(target) == target
def test_sanitize_redirect_to_preserves_allowed_preview_target(monkeypatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example")
target = "https://preview.example/agents/thread-1/plan?from=slack#review"
assert sanitize_redirect_to(target) == target
def test_sanitize_redirect_to_rejects_external_target(monkeypatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example")
assert sanitize_redirect_to("https://evil.example/agents/thread-1/plan") == (
"https://dashboard.example"
)

124
tests/test_schedule_thread_wakeup.py
View file

@ -1,13 +1,67 @@
from __future__ import annotations from __future__ import annotations
import importlib import importlib
from datetime import UTC, datetime from datetime import UTC, datetime, timedelta
from typing import Any from typing import Any
import pytest import pytest
wakeup_tool = importlib.import_module("agent.tools.schedule_thread_wakeup") wakeup_tool = importlib.import_module("agent.tools.schedule_thread_wakeup")
# Captured before the autouse stub replaces it, for the one test that needs the real wrapper.
_real_purge_best_effort = wakeup_tool._purge_expired_wakeups_best_effort
@pytest.fixture(autouse=True)
def _stub_purge(monkeypatch: pytest.MonkeyPatch) -> None:
"""Keep the opportunistic purge from touching the network in every test."""
async def _noop() -> None:
return None
monkeypatch.setattr(wakeup_tool, "_purge_expired_wakeups_best_effort", _noop)
class _FakeCrons:
def __init__(self, crons: list[dict[str, Any]]) -> None:
self._crons = list(crons)
self.deleted: list[str] = []
self.search_calls: list[dict[str, Any]] = []
async def search(
self,
*,
metadata: dict[str, Any] | None = None,
limit: int = 10,
offset: int = 0,
**_: Any,
) -> list[dict[str, Any]]:
self.search_calls.append({"metadata": metadata, "limit": limit, "offset": offset})
items = [
c
for c in self._crons
if not metadata
or all((c.get("metadata") or {}).get(k) == v for k, v in metadata.items())
]
return items[offset : offset + limit]
async def delete(self, cron_id: str) -> None:
self.deleted.append(cron_id)
self._crons = [c for c in self._crons if c.get("cron_id") != cron_id]
class _FakeClient:
def __init__(self, crons: list[dict[str, Any]]) -> None:
self.crons = _FakeCrons(crons)
def _wakeup_cron(cron_id: str, end_time: datetime | None) -> dict[str, Any]:
return {
"cron_id": cron_id,
"end_time": end_time.isoformat() if end_time else None,
"metadata": {"kind": "thread_wakeup"},
}
def _config(**overrides: Any) -> dict[str, Any]: def _config(**overrides: Any) -> dict[str, Any]:
base: dict[str, Any] = { base: dict[str, Any] = {
@ -241,3 +295,71 @@ def test_build_one_shot_cron_handles_month_boundary() -> None:
assert parts[1] == "23" assert parts[1] == "23"
assert parts[2] == "31" assert parts[2] == "31"
assert parts[3] == "12" assert parts[3] == "12"
async def test_purge_deletes_only_expired_wakeups() -> None:
now = datetime(2026, 6, 30, 22, 0, tzinfo=UTC)
client = _FakeClient(
[
_wakeup_cron("expired-1", now - timedelta(hours=1)),
_wakeup_cron("expired-2", now - timedelta(days=1)),
_wakeup_cron("future-1", now + timedelta(hours=1)),
_wakeup_cron("no-end", None),
]
)
deleted = await wakeup_tool.purge_expired_wakeup_crons(client, now=now)
assert deleted == 2
assert client.crons.deleted == ["expired-1", "expired-2"]
# Search is scoped to the thread_wakeup kind so other crons are never seen.
assert client.crons.search_calls[0]["metadata"] == {"kind": "thread_wakeup"}
async def test_purge_paginates(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setattr(wakeup_tool, "_PURGE_PAGE_SIZE", 2)
now = datetime(2026, 6, 30, 22, 0, tzinfo=UTC)
client = _FakeClient([_wakeup_cron(f"expired-{i}", now - timedelta(hours=1)) for i in range(3)])
deleted = await wakeup_tool.purge_expired_wakeup_crons(client, now=now)
assert deleted == 3
assert sorted(client.crons.deleted) == ["expired-0", "expired-1", "expired-2"]
# Two pages fetched (offset 0 and 2), then a short final page ends the loop.
assert [c["offset"] for c in client.crons.search_calls] == [0, 2]
async def test_best_effort_purge_swallows_errors(monkeypatch: pytest.MonkeyPatch) -> None:
async def boom(*_: Any, **__: Any) -> int:
raise RuntimeError("search failed")
monkeypatch.setattr(wakeup_tool, "purge_expired_wakeup_crons", boom)
monkeypatch.setattr(wakeup_tool, "get_client", lambda url: object())
# The real wrapper must never propagate — a purge failure can't block wakeups.
await _real_purge_best_effort()
async def test_schedule_purges_before_creating(monkeypatch: pytest.MonkeyPatch) -> None:
calls: list[str] = []
async def spy_purge() -> None:
calls.append("purge")
async def fake_create_wakeup_cron(**kwargs: Any) -> dict[str, Any]:
calls.append("create")
return {
"success": True,
"cron_id": "cron-1",
"scheduled_for": "",
"thread_id": kwargs["thread_id"],
}
monkeypatch.setattr(wakeup_tool, "get_config", _config)
monkeypatch.setattr(wakeup_tool, "_purge_expired_wakeups_best_effort", spy_purge)
monkeypatch.setattr(wakeup_tool, "_create_wakeup_cron", fake_create_wakeup_cron)
result = await wakeup_tool.schedule_thread_wakeup(5)
assert result["success"] is True
assert calls == ["purge", "create"]

6
ui/src/client.tsx
View file

@ -1,5 +1,5 @@
import { StartClient } from "@tanstack/react-start/client" import { StartClient } from "@tanstack/react-start/client"
import { StrictMode, useEffect, useState } from "react" import { useEffect, useState } from "react"
import { hydrateRoot } from "react-dom/client" import { hydrateRoot } from "react-dom/client"
import { registerSW } from "virtual:pwa-register" import { registerSW } from "virtual:pwa-register"
@ -35,8 +35,8 @@ function PwaUpdateProvider() {
hydrateRoot( hydrateRoot(
document, document,
<StrictMode> <>
<StartClient /> <StartClient />
<PwaUpdateProvider /> <PwaUpdateProvider />
</StrictMode> </>
) )

37
ui/src/components/agents/PlanReview.tsx
View file

@ -1,4 +1,5 @@
import { useCallback, useEffect, useState } from "react" import { useCallback, useEffect, useState } from "react"
import { useNavigate } from "@tanstack/react-router"
import type { PlanComment, PlanData } from "@/lib/plan" import type { PlanComment, PlanData } from "@/lib/plan"
import { import {
@ -47,6 +48,7 @@ async function copyToClipboard(text: string): Promise<boolean> {
} }
export function PlanReview({ plan }: { plan: PlanData }) { export function PlanReview({ plan }: { plan: PlanData }) {
const navigate = useNavigate()
const resolvedTheme = useResolvedTheme() const resolvedTheme = useResolvedTheme()
const [comments, setComments] = useState<Array<PlanComment>>([]) const [comments, setComments] = useState<Array<PlanComment>>([])
const [draft, setDraft] = useState("") const [draft, setDraft] = useState("")
@ -108,20 +110,23 @@ export function PlanReview({ plan }: { plan: PlanData }) {
setBusy(kind) setBusy(kind)
setError(null) setError(null)
try { try {
if (kind === "approve") await approvePlan(plan.threadId) if (kind === "approve") {
else await rejectPlan(plan.threadId) await approvePlan(plan.threadId)
setDecision( await navigate({
kind === "approve" to: "/agents/$threadId",
? "Plan approved — the agent is implementing it." params: { threadId: plan.threadId },
: "Changes requested — the agent is revising the plan." })
) return
}
await rejectPlan(plan.threadId)
setDecision("Changes requested — the agent is revising the plan.")
} catch (e) { } catch (e) {
setError((e as Error).message) setError((e as Error).message)
} finally { } finally {
setBusy(null) setBusy(null)
} }
}, },
[plan.threadId] [navigate, plan.threadId]
) )
const copyPlan = useCallback(async () => { const copyPlan = useCallback(async () => {
@ -139,8 +144,8 @@ export function PlanReview({ plan }: { plan: PlanData }) {
data-testid="plan-review" data-testid="plan-review"
className="flex min-h-0 flex-1 flex-col bg-[var(--ui-bg)] text-[var(--ui-text)]" className="flex min-h-0 flex-1 flex-col bg-[var(--ui-bg)] text-[var(--ui-text)]"
> >
<div className="flex items-center justify-between gap-4 border-b border-[var(--ui-border)] px-6 py-3"> <div className="flex flex-col gap-3 border-b border-[var(--ui-border)] px-4 py-3 md:flex-row md:items-center md:justify-between md:gap-4 md:px-6">
<div> <div className="min-w-0">
<h1 className="text-base font-semibold text-[var(--ui-text)]"> <h1 className="text-base font-semibold text-[var(--ui-text)]">
Implementation plan Implementation plan
</h1> </h1>
@ -150,11 +155,11 @@ export function PlanReview({ plan }: { plan: PlanData }) {
<span data-testid="plan-status">{plan.status}</span> <span data-testid="plan-status">{plan.status}</span>
</p> </p>
</div> </div>
<div className="flex shrink-0 items-center gap-2"> <div className="flex min-w-0 flex-wrap items-center gap-2 md:shrink-0 md:justify-end">
{decision && ( {decision && (
<span <span
data-testid="plan-decision" data-testid="plan-decision"
className="text-xs text-[var(--ui-text-dim)]" className="w-full text-xs text-[var(--ui-text-dim)] md:w-auto"
> >
{decision} {decision}
</span> </span>
@ -194,9 +199,9 @@ export function PlanReview({ plan }: { plan: PlanData }) {
</div> </div>
</div> </div>
<div className="flex min-h-0 flex-1 overflow-hidden"> <div className="flex min-h-0 flex-1 flex-col overflow-y-auto md:flex-row md:overflow-hidden">
<div <div
className="min-h-0 flex-1 overflow-auto px-6 py-4" className="min-w-0 px-4 py-4 md:min-h-0 md:flex-1 md:overflow-auto md:px-6"
data-testid="plan-document" data-testid="plan-document"
data-color-scheme={resolvedTheme} data-color-scheme={resolvedTheme}
> >
@ -209,14 +214,14 @@ export function PlanReview({ plan }: { plan: PlanData }) {
)} )}
</div> </div>
<aside className="flex w-80 shrink-0 flex-col border-l border-[var(--ui-border)]"> <aside className="flex shrink-0 flex-col border-t border-[var(--ui-border)] md:w-80 md:border-t-0 md:border-l">
<div className="border-b border-[var(--ui-border)] px-4 py-3"> <div className="border-b border-[var(--ui-border)] px-4 py-3">
<h2 className="text-sm font-semibold text-[var(--ui-text)]"> <h2 className="text-sm font-semibold text-[var(--ui-text)]">
Comments Comments
</h2> </h2>
</div> </div>
<div <div
className="min-h-0 flex-1 space-y-3 overflow-auto px-4 py-3" className="max-h-80 space-y-3 overflow-auto px-4 py-3 md:max-h-none md:min-h-0 md:flex-1"
data-testid="plan-comments" data-testid="plan-comments"
> >
{comments.length === 0 ? ( {comments.length === 0 ? (

295
ui/src/components/agents/ReviewMainBody.tsx
View file

@ -41,6 +41,7 @@ import {
MultiFileDiff, MultiFileDiff,
Virtualizer, Virtualizer,
WorkerPoolContextProvider, WorkerPoolContextProvider,
useVirtualizer,
} from "@pierre/diffs/react" } from "@pierre/diffs/react"
import type { Icon } from "@phosphor-icons/react" import type { Icon } from "@phosphor-icons/react"
import type { FileContents } from "@pierre/diffs/react" import type { FileContents } from "@pierre/diffs/react"
@ -73,7 +74,10 @@ import {
ReviewChatComposerProvider, ReviewChatComposerProvider,
useReviewChatComposer, useReviewChatComposer,
} from "@/components/agents/ReviewChat" } from "@/components/agents/ReviewChat"
import { ReviewSidebarPanel } from "@/components/agents/ReviewSidebar" import {
ReviewSidebarPanel,
renderInlineCode,
} from "@/components/agents/ReviewSidebar"
import { import {
DIFF_VIRTUALIZER_CONFIG, DIFF_VIRTUALIZER_CONFIG,
DIFF_VIRTUAL_METRICS, DIFF_VIRTUAL_METRICS,
@ -213,46 +217,60 @@ function selectedRangeFromDiff(
} }
} }
// Scroll a file card / group flush to the top of the diff scroller. Under // Scroll a file card / group flush to the top of the diff scroller (fallback
// virtualization, scrollIntoView computes its target against estimated row // when no virtualizer geometry is available). Jumps instantly to a bounding-rect
// heights; scrolling past unmeasured files reconciles their real heights // target — respecting the element's scroll-margin-top — then holds that target
// mid-animation and the Virtualizer re-pins its scroll anchor, which leaves the // as content above reflows, so no smooth-scroll animation races the height
// target off the top. Once the smooth scroll settles, re-assert alignment (now // reconciliation. Returns a stop fn to cancel the hold.
// against measured heights) until the target sits at the top or the budget runs function scrollCardToTop(
// out. Respects the element's scroll-margin-top. el: HTMLElement,
function scrollCardToTop(el: HTMLElement, scroller: HTMLElement | null): void { scroller: HTMLElement | null
el.scrollIntoView({ block: "start", behavior: "smooth" }) ): () => void {
if (!scroller) return if (!scroller) {
let frames = 0 el.scrollIntoView({ block: "start" })
let lastTop = Number.NaN return () => {}
let stableFrames = 0 }
let corrections = 0 return jumpAndHold(scroller, () => {
const align = () => {
if (frames++ > 240) return
const top = scroller.scrollTop
if (top === lastTop) stableFrames++
else {
stableFrames = 0
lastTop = top
}
// Wait for the smooth scroll + height reconciliation to settle.
if (stableFrames < 3) {
requestAnimationFrame(align)
return
}
const marginTop = parseFloat(getComputedStyle(el).scrollMarginTop) || 0 const marginTop = parseFloat(getComputedStyle(el).scrollMarginTop) || 0
const delta = const delta =
el.getBoundingClientRect().top - el.getBoundingClientRect().top -
scroller.getBoundingClientRect().top - scroller.getBoundingClientRect().top -
marginTop marginTop
if (Math.abs(delta) > 1 && corrections++ < 5) { return clampScrollTop(scroller, scroller.scrollTop + delta)
el.scrollIntoView({ block: "start", behavior: "smooth" }) })
stableFrames = 0 }
lastTop = Number.NaN
requestAnimationFrame(align) // The virtualizer instance returned by useVirtualizer(); exposes
} // getOffsetInScrollContainer for accurate scroll targeting.
} type DiffVirtualizer = NonNullable<ReturnType<typeof useVirtualizer>>
requestAnimationFrame(align)
// Breathing room left above a block/file when it's scrolled to the top.
const SCROLL_TOP_GAP = 8
// Scroll a block / file card flush to the top of the diff scroller using the
// virtualizer's own geometry. getOffsetInScrollContainer returns the element's
// absolute offset within the scroll content; with uniform fixed-height rows
// (see diffUtils) that offset is stable, so an instant jump lands precisely.
// jumpAndHold then re-reads the offset whenever the content reflows (rows above
// measuring/expanding) and re-asserts it, so the target stays pinned to the top.
// Returns a stop fn to cancel the hold.
function scrollCardToTopVirtual(
el: HTMLElement,
scroller: HTMLElement,
virtualizer: DiffVirtualizer
): () => void {
return jumpAndHold(scroller, () =>
clampScrollTop(
scroller,
virtualizer.getOffsetInScrollContainer(el) - SCROLL_TOP_GAP
)
)
}
// Older stored summaries embed `[label](#loc=path:line)` diff links; render the
// label as inline code instead so no stale jump-links leak into the block body.
function stripLocationLinks(summary: string): string {
return summary.replace(/\[([^\]]+)\]\(#loc=[^)]*\)/g, "`$1`")
} }
interface PositionedDiffInstance { interface PositionedDiffInstance {
@ -283,16 +301,68 @@ function clampScrollTop(scroller: HTMLElement, top: number): number {
) )
} }
function scrollElementToCenter(el: HTMLElement, scroller: HTMLElement): number { // How long to keep re-asserting a scroll target after the initial jump.
const SCROLL_HOLD_TIMEOUT_MS = 700
// Jump the scroller to getTarget() instantly, then re-assert that target each
// time the scroll content reflows (off-screen cards mounting, files expanding,
// annotation cards measuring) — a ResizeObserver is the real "layout settled"
// signal, replacing fixed frame-budget correction loops. Bails the moment the
// user scrolls so we never fight them, and disconnects after a short ceiling.
function jumpAndHold(
scroller: HTMLElement,
getTarget: () => number,
timeout = SCROLL_HOLD_TIMEOUT_MS
): () => void {
let raf = 0
let stopped = false
let timer = 0
let ro: ResizeObserver | null = null
const stop = () => {
if (stopped) return
stopped = true
ro?.disconnect()
if (raf) cancelAnimationFrame(raf)
scroller.removeEventListener("wheel", stop)
scroller.removeEventListener("touchstart", stop)
window.clearTimeout(timer)
}
const reassert = () => {
raf = 0
if (stopped) return
const desired = getTarget()
if (Math.abs(desired - scroller.scrollTop) > 1) {
scroller.scrollTo({ top: desired, behavior: "auto" })
}
}
const schedule = () => {
if (!raf && !stopped) raf = requestAnimationFrame(reassert)
}
scroller.scrollTo({ top: getTarget(), behavior: "auto" })
ro = new ResizeObserver(schedule)
ro.observe(scroller.firstElementChild ?? scroller)
scroller.addEventListener("wheel", stop, { passive: true })
scroller.addEventListener("touchstart", stop, { passive: true })
timer = window.setTimeout(stop, timeout)
return stop
}
// Absolute scrollTop that centers el within the scroller's viewport.
function elementCenterTarget(el: HTMLElement, scroller: HTMLElement): number {
const elementRect = el.getBoundingClientRect() const elementRect = el.getBoundingClientRect()
const scrollerRect = scroller.getBoundingClientRect() const scrollerRect = scroller.getBoundingClientRect()
const delta = const delta =
elementRect.top - elementRect.top -
scrollerRect.top - scrollerRect.top -
(scroller.clientHeight - elementRect.height) / 2 (scroller.clientHeight - elementRect.height) / 2
const targetTop = clampScrollTop(scroller, scroller.scrollTop + delta) return clampScrollTop(scroller, scroller.scrollTop + delta)
}
function scrollElementToCenter(el: HTMLElement, scroller: HTMLElement): number {
const before = scroller.scrollTop
const targetTop = elementCenterTarget(el, scroller)
scroller.scrollTo({ top: targetTop, behavior: "auto" }) scroller.scrollTo({ top: targetTop, behavior: "auto" })
return Math.abs(delta) return Math.abs(targetTop - before)
} }
function scrollDiffLineToCenter( function scrollDiffLineToCenter(
@ -561,8 +631,15 @@ function ReviewBodyInner({
range: SelectedLineRange range: SelectedLineRange
} | null>(null) } | null>(null)
const diffScrollElRef = useRef<HTMLDivElement | null>(null) const diffScrollElRef = useRef<HTMLDivElement | null>(null)
const virtualizerRef = useRef<DiffVirtualizer | null>(null)
const findingScrollRequestRef = useRef(0) const findingScrollRequestRef = useRef(0)
// Cancels the in-flight scroll "hold" (see jumpAndHold) when a new navigation
// begins or the component unmounts, so holds never fight each other.
const scrollHoldStopRef = useRef<(() => void) | null>(null)
const groupRefs = useRef<Record<number, HTMLDivElement | null>>({}) const groupRefs = useRef<Record<number, HTMLDivElement | null>>({})
// The block pinned at the top of the diff (scroll-spy), highlighted in the
// agenda sidebar.
const [activeGroup, setActiveGroup] = useState<number | null>(null)
const [diffStyle, setDiffStyleState] = useState<DiffStyle>(() => const [diffStyle, setDiffStyleState] = useState<DiffStyle>(() =>
readStoredDiffStyle() readStoredDiffStyle()
) )
@ -726,11 +803,6 @@ function ReviewBodyInner({
return groupedView.map((group) => ({ return groupedView.map((group) => ({
index: group.index, index: group.index,
title: group.title, title: group.title,
summary: group.summary,
additions: group.additions,
deletions: group.deletions,
fileCount: group.files.length,
files: group.files.map((file) => file.path),
})) }))
}, [groupedView]) }, [groupedView])
@ -757,19 +829,66 @@ function ReviewBodyInner({
const scrollToFile = useCallback((path: string) => { const scrollToFile = useCallback((path: string) => {
setSelectedFile(path) setSelectedFile(path)
setExpandedFiles((prev) => ({ ...prev, [path]: true })) setExpandedFiles((prev) => ({ ...prev, [path]: true }))
scrollHoldStopRef.current?.()
requestAnimationFrame(() => { requestAnimationFrame(() => {
const el = fileRefs.current[path] const el = fileRefs.current[path]
if (el) scrollCardToTop(el, diffScrollElRef.current) const scroller = diffScrollElRef.current
if (!el || !scroller) return
scrollHoldStopRef.current = virtualizerRef.current
? scrollCardToTopVirtual(el, scroller, virtualizerRef.current)
: scrollCardToTop(el, scroller)
}) })
}, []) }, [])
const scrollToGroup = useCallback((index: number) => { const scrollToGroup = useCallback((index: number) => {
scrollHoldStopRef.current?.()
requestAnimationFrame(() => { requestAnimationFrame(() => {
const el = groupRefs.current[index] const el = groupRefs.current[index]
if (el) scrollCardToTop(el, diffScrollElRef.current) const scroller = diffScrollElRef.current
if (!el || !scroller) return
scrollHoldStopRef.current = virtualizerRef.current
? scrollCardToTopVirtual(el, scroller, virtualizerRef.current)
: scrollCardToTop(el, scroller)
}) })
}, []) }, [])
useEffect(() => () => scrollHoldStopRef.current?.(), [])
// Scroll-spy: track which block's header is currently pinned at the top of the
// diff scroller and surface it as the active agenda row (Google-Docs outline).
useEffect(() => {
if (view !== "ai" || !groupedView || groupedView.length === 0) {
setActiveGroup(null)
return
}
const scroller = diffScrollElRef.current
if (!scroller) return
let raf = 0
const compute = () => {
raf = 0
const top = scroller.getBoundingClientRect().top
let current = groupedView[0]?.index ?? null
for (const group of groupedView) {
const el = groupRefs.current[group.index]
if (!el) continue
if (el.getBoundingClientRect().top - top <= SCROLL_TOP_GAP + 2)
current = group.index
else break
}
setActiveGroup(current)
}
const onScroll = () => {
if (raf) return
raf = requestAnimationFrame(compute)
}
compute()
scroller.addEventListener("scroll", onScroll, { passive: true })
return () => {
scroller.removeEventListener("scroll", onScroll)
if (raf) cancelAnimationFrame(raf)
}
}, [view, groupedView])
const filesByPath = useMemo( const filesByPath = useMemo(
() => new Map((diffFiles ?? []).map((file) => [file.path, file])), () => new Map((diffFiles ?? []).map((file) => [file.path, file])),
[diffFiles] [diffFiles]
@ -903,6 +1022,7 @@ function ReviewBodyInner({
if (!willExpand || !isAnchored(finding)) return if (!willExpand || !isAnchored(finding)) return
setSelectedFile(finding.file) setSelectedFile(finding.file)
setExpandedFiles((prev) => ({ ...prev, [finding.file]: true })) setExpandedFiles((prev) => ({ ...prev, [finding.file]: true }))
scrollHoldStopRef.current?.()
let frames = 0 let frames = 0
let lineScrollDone = false let lineScrollDone = false
const snap = () => { const snap = () => {
@ -910,12 +1030,13 @@ function ReviewBodyInner({
const scroller = diffScrollElRef.current const scroller = diffScrollElRef.current
if (!scroller) return if (!scroller) return
// Once the finding's inline card has mounted (its diff rows window in
// under virtualization), center it and hold as the card settles.
const annotation = annotationRefs.current[finding.id] const annotation = annotationRefs.current[finding.id]
if (annotation?.isConnected && annotation.getClientRects().length > 0) { if (annotation?.isConnected && annotation.getClientRects().length > 0) {
const delta = scrollElementToCenter(annotation, scroller) scrollHoldStopRef.current = jumpAndHold(scroller, () =>
if (delta <= 1 || frames >= FINDING_SCROLL_MAX_FRAMES) return elementCenterTarget(annotation, scroller)
frames += 1 )
requestAnimationFrame(snap)
return return
} }
@ -961,6 +1082,7 @@ function ReviewBodyInner({
} }
setSelectedFile(path) setSelectedFile(path)
setExpandedFiles((prev) => ({ ...prev, [path]: true })) setExpandedFiles((prev) => ({ ...prev, [path]: true }))
scrollHoldStopRef.current?.()
const requestId = ++findingScrollRequestRef.current const requestId = ++findingScrollRequestRef.current
const side: SelectionSide = const side: SelectionSide =
openComment.side === "LEFT" ? "deletions" : "additions" openComment.side === "LEFT" ? "deletions" : "additions"
@ -975,10 +1097,9 @@ function ReviewBodyInner({
const annotation = annotationRefs.current[key] const annotation = annotationRefs.current[key]
if (annotation?.isConnected && annotation.getClientRects().length > 0) { if (annotation?.isConnected && annotation.getClientRects().length > 0) {
mounted = true mounted = true
const delta = scrollElementToCenter(annotation, scroller) scrollHoldStopRef.current = jumpAndHold(scroller, () =>
if (delta <= 1 || frames >= FINDING_SCROLL_MAX_FRAMES) return elementCenterTarget(annotation, scroller)
frames += 1 )
requestAnimationFrame(snap)
return return
} }
const diffTarget = diffInstanceRefs.current[path] const diffTarget = diffInstanceRefs.current[path]
@ -1055,6 +1176,7 @@ function ReviewBodyInner({
view, view,
onViewChange: setView, onViewChange: setView,
onSelectGroup: scrollToGroup, onSelectGroup: scrollToGroup,
activeGroup,
}), }),
[ [
detail.number, detail.number,
@ -1066,6 +1188,7 @@ function ReviewBodyInner({
view, view,
setView, setView,
scrollToGroup, scrollToGroup,
activeGroup,
] ]
) )
@ -1122,7 +1245,10 @@ function ReviewBodyInner({
)} )}
config={DIFF_VIRTUALIZER_CONFIG} config={DIFF_VIRTUALIZER_CONFIG}
> >
<div ref={scrollerProbe} aria-hidden className="hidden" /> <VirtualizerBridge
probeRef={scrollerProbe}
instanceRef={virtualizerRef}
/>
<PrHeader <PrHeader
url={detail.url} url={detail.url}
title={detail.pr.title} title={detail.pr.title}
@ -1271,21 +1397,54 @@ function DiffStyleButton({
) )
} }
// Grabs the virtualizer instance from context (only available inside
// <Virtualizer>) and lifts it to the parent ref so scroll-to can read accurate
// offsets. Doubles as the hidden scroll-element probe.
function VirtualizerBridge({
probeRef,
instanceRef,
}: {
probeRef: (node: HTMLDivElement | null) => void
instanceRef: React.MutableRefObject<DiffVirtualizer | null>
}) {
const virtualizer = useVirtualizer()
useEffect(() => {
instanceRef.current = virtualizer ?? null
}, [virtualizer, instanceRef])
return <div ref={probeRef} aria-hidden className="hidden" />
}
// The block header: number + title + stats, then the block description. Pinned
// at the top of the diff scroller while scrolling the block (Google-Docs feel),
// stacked above Pierre's in-diff sticky header (z-index 4). A long description
// scrolls within the pinned header instead of consuming the viewport.
function GroupHeader({ group }: { group: ResolvedGroup }) { function GroupHeader({ group }: { group: ResolvedGroup }) {
const title = useMemo(() => renderInlineCode(group.title), [group.title])
const summary = useMemo(
() => (group.summary ? stripLocationLinks(group.summary) : ""),
[group.summary]
)
return ( return (
<div className="flex items-center gap-2"> <div className="sticky top-0 z-[5] border-b border-border bg-background py-2">
<span className="flex size-5 shrink-0 items-center justify-center rounded bg-[var(--ui-panel-2)] text-[11px] font-medium text-muted-foreground"> <div className="flex items-center gap-2">
{group.index} <span className="flex size-5 shrink-0 items-center justify-center rounded bg-[var(--ui-panel-2)] text-[11px] font-medium text-muted-foreground">
</span> {group.index}
<h3 className="min-w-0 truncate text-sm font-medium">{group.title}</h3> </span>
<span className="flex shrink-0 items-center gap-1.5 font-mono text-[11px]"> <h3 className="min-w-0 flex-1 truncate text-sm font-medium">{title}</h3>
{group.additions > 0 && ( <span className="flex shrink-0 items-center gap-1.5 font-mono text-[11px]">
<span className="text-emerald-500">+{group.additions}</span> {group.additions > 0 && (
)} <span className="text-emerald-500">+{group.additions}</span>
{group.deletions > 0 && ( )}
<span className="text-red-500">-{group.deletions}</span> {group.deletions > 0 && (
)} <span className="text-red-500">-{group.deletions}</span>
</span> )}
</span>
</div>
{summary && (
<div className="mt-2 max-h-40 overflow-y-auto text-xs text-muted-foreground">
<Markdown content={summary} />
</div>
)}
</div> </div>
) )
} }

156
ui/src/components/agents/ReviewSidebar.tsx
View file

@ -1,14 +1,10 @@
import { memo, useCallback, useEffect, useMemo, useState } from "react" import { memo, useCallback, useEffect, useMemo } from "react"
import { import {
FileTree, FileTree,
useFileTree, useFileTree,
useFileTreeSelection, useFileTreeSelection,
} from "@pierre/trees/react" } from "@pierre/trees/react"
import { import { ListBulletsIcon, TreeViewIcon } from "@phosphor-icons/react"
CaretRightIcon,
ListBulletsIcon,
TreeViewIcon,
} from "@phosphor-icons/react"
import type { ReactNode } from "react" import type { ReactNode } from "react"
import type { import type {
@ -17,7 +13,6 @@ import type {
GitStatusEntry, GitStatusEntry,
} from "@pierre/trees" } from "@pierre/trees"
import type { ReviewDiffFile } from "@/lib/api" import type { ReviewDiffFile } from "@/lib/api"
import { Markdown } from "@/components/agents/ported"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { import {
TREE_UNSAFE_CSS, TREE_UNSAFE_CSS,
@ -37,11 +32,6 @@ export type ReviewSidebarView = "ai" | "files"
export interface ReviewSidebarGroup { export interface ReviewSidebarGroup {
index: number index: number
title: string title: string
summary: string
additions: number
deletions: number
fileCount: number
files: Array<string>
} }
export interface ReviewSidebarData { export interface ReviewSidebarData {
@ -54,6 +44,9 @@ export interface ReviewSidebarData {
view: ReviewSidebarView view: ReviewSidebarView
onViewChange: (view: ReviewSidebarView) => void onViewChange: (view: ReviewSidebarView) => void
onSelectGroup: (index: number) => void onSelectGroup: (index: number) => void
// The block currently pinned at the top of the diff (scroll-spy), highlighted
// in the agenda. null when no block is active or the AI view isn't shown.
activeGroup: number | null
} }
export function ReviewSidebarPanel({ data }: { data: ReviewSidebarData }) { export function ReviewSidebarPanel({ data }: { data: ReviewSidebarData }) {
@ -73,8 +66,8 @@ export function ReviewSidebarPanel({ data }: { data: ReviewSidebarData }) {
{showAi ? ( {showAi ? (
<ReviewGroupList <ReviewGroupList
groups={data.groups ?? []} groups={data.groups ?? []}
activeGroup={data.activeGroup}
onSelectGroup={data.onSelectGroup} onSelectGroup={data.onSelectGroup}
onSelectFile={data.onSelect}
/> />
) : !data.files ? ( ) : !data.files ? (
<div className="px-4 pt-1"> <div className="px-4 pt-1">
@ -150,43 +143,31 @@ function ReviewViewToggleButton({
function ReviewGroupList({ function ReviewGroupList({
groups, groups,
activeGroup,
onSelectGroup, onSelectGroup,
onSelectFile,
}: { }: {
groups: Array<ReviewSidebarGroup> groups: Array<ReviewSidebarGroup>
activeGroup: number | null
onSelectGroup: (index: number) => void onSelectGroup: (index: number) => void
onSelectFile: (path: string) => void
}) { }) {
return ( return (
<div className="min-h-0 flex-1 divide-y divide-[var(--ui-border-subtle)] overflow-y-auto"> <div className="min-h-0 flex-1 overflow-y-auto py-1">
{groups.map((group) => ( {groups.map((group) => (
<ReviewGroupRow <ReviewGroupRow
key={group.index} key={group.index}
group={group} group={group}
active={group.index === activeGroup}
onSelectGroup={onSelectGroup} onSelectGroup={onSelectGroup}
onSelectFile={onSelectFile}
/> />
))} ))}
</div> </div>
) )
} }
function splitPath(path: string): { dir: string; base: string } {
const idx = path.lastIndexOf("/")
if (idx === -1) return { dir: "", base: path }
return { dir: path.slice(0, idx), base: path.slice(idx + 1) }
}
// Older stored summaries embed `[label](#loc=path:line)` diff links. Render the
// label as inline code instead so no stale jump-links leak into the explanation.
function stripLocationLinks(summary: string): string {
return summary.replace(/\[([^\]]+)\]\(#loc=[^)]*\)/g, "`$1`")
}
// Render a title with `backtick`-delimited spans as inline code chips, matching // Render a title with `backtick`-delimited spans as inline code chips, matching
// the Markdown component's inline-code styling, without pulling in the full // the Markdown component's inline-code styling, without pulling in the full
// block renderer for a single line. // block renderer for a single line.
function renderInlineCode(text: string): Array<ReactNode> { export function renderInlineCode(text: string): Array<ReactNode> {
return text.split(/(`[^`]+`)/g).map((part, i) => { return text.split(/(`[^`]+`)/g).map((part, i) => {
if (part.length >= 2 && part.startsWith("`") && part.endsWith("`")) { if (part.length >= 2 && part.startsWith("`") && part.endsWith("`")) {
return ( return (
@ -202,26 +183,20 @@ function renderInlineCode(text: string): Array<ReactNode> {
}) })
} }
// The whole card is the scroll-to-group target so clicks anywhere (including // A single agenda entry: just the block number + title, like a Google-Docs
// the expanded explanation body) focus the diff. Nested controls — the file // outline. Clicking (or Enter/Space) scrolls the diff to that block. The active
// links and the "Read explanation" toggle — stop propagation so they keep // block (scroll-spy) gets an accent rule + emphasis. memo'd so scroll-spy
// their own behavior. memo'd + memoized string processing so re-renders from // re-renders only repaint the rows whose active state actually changed.
// sibling state don't re-run Markdown/inline-code work.
const ReviewGroupRow = memo(function ReviewGroupRow({ const ReviewGroupRow = memo(function ReviewGroupRow({
group, group,
active,
onSelectGroup, onSelectGroup,
onSelectFile,
}: { }: {
group: ReviewSidebarGroup group: ReviewSidebarGroup
active: boolean
onSelectGroup: (index: number) => void onSelectGroup: (index: number) => void
onSelectFile: (path: string) => void
}) { }) {
const [expanded, setExpanded] = useState(false)
const title = useMemo(() => renderInlineCode(group.title), [group.title]) const title = useMemo(() => renderInlineCode(group.title), [group.title])
const summary = useMemo(
() => stripLocationLinks(group.summary),
[group.summary]
)
const selectGroup = useCallback( const selectGroup = useCallback(
() => onSelectGroup(group.index), () => onSelectGroup(group.index),
[onSelectGroup, group.index] [onSelectGroup, group.index]
@ -240,86 +215,29 @@ const ReviewGroupRow = memo(function ReviewGroupRow({
<div <div
role="button" role="button"
tabIndex={0} tabIndex={0}
aria-current={active ? "true" : undefined}
onClick={selectGroup} onClick={selectGroup}
onKeyDown={onKeyDown} onKeyDown={onKeyDown}
className="cursor-pointer px-3 py-3 transition-colors hover:bg-[var(--ui-sidebar-hover)]" className={cn(
"flex cursor-pointer items-start gap-2 border-l-2 px-3 py-1.5 text-left transition-colors",
active
? "border-[var(--ui-accent)] bg-[var(--ui-sidebar-hover)]"
: "border-transparent hover:bg-[var(--ui-sidebar-hover)]"
)}
> >
<div className="flex w-full items-start gap-2 text-left"> <span className="mt-px shrink-0 text-[11px] font-medium text-[var(--ui-text-dim)] tabular-nums">
<span className="mt-0.5 flex size-5 shrink-0 items-center justify-center rounded bg-[var(--ui-panel-2)] text-[11px] font-medium text-[var(--ui-text-dim)]"> {group.index}.
{group.index} </span>
</span> <span
<span className="min-w-0 flex-1"> className={cn(
<span className="block text-xs leading-5 font-medium text-[var(--ui-text)]"> "min-w-0 text-xs leading-5",
{title} active
</span> ? "font-medium text-[var(--ui-text)]"
<span className="mt-1 flex flex-wrap items-center gap-1.5 text-[11px] text-[var(--ui-text-dim)]"> : "text-[var(--ui-text-muted)]"
<span> )}
{group.fileCount} file{group.fileCount === 1 ? "" : "s"} >
</span> {title}
{group.additions > 0 && ( </span>
<span className="text-emerald-500">+{group.additions}</span>
)}
{group.deletions > 0 && (
<span className="text-red-500">-{group.deletions}</span>
)}
</span>
</span>
</div>
{group.files.length > 0 && (
<div className="mt-2 space-y-0.5 pl-7">
{group.files.map((path) => {
const { dir, base } = splitPath(path)
return (
<button
key={path}
type="button"
onClick={(event) => {
event.stopPropagation()
onSelectFile(path)
}}
title={path}
className="flex w-full items-baseline gap-1.5 text-left text-[11px] hover:text-[var(--ui-accent)]"
>
<span className="shrink-0 font-medium text-[var(--ui-text-muted)]">
{base}
</span>
{dir && (
<span className="min-w-0 truncate text-[var(--ui-text-dim)]">
{dir}
</span>
)}
</button>
)
})}
</div>
)}
{group.summary && (
<div className="mt-2">
<button
type="button"
onClick={(event) => {
event.stopPropagation()
setExpanded((value) => !value)
}}
className="inline-flex items-center gap-1 text-[11px] font-medium text-[var(--ui-accent)]"
>
<CaretRightIcon
className={cn(
"size-3 transition-transform",
expanded && "rotate-90"
)}
/>
Read explanation
</button>
{expanded && (
<div className="mt-1.5">
<Markdown content={summary} />
</div>
)}
</div>
)}
</div> </div>
) )
}) })

10
ui/src/components/agents/SidebarFilterMenu.tsx
View file

@ -21,15 +21,15 @@ import {
import { cn } from "@/lib/utils" import { cn } from "@/lib/utils"
const POPUP_CLASS = const POPUP_CLASS =
"z-50 min-w-[12rem] origin-(--transform-origin) overflow-hidden rounded-md border border-[var(--ui-border)] bg-popover p-1 text-popover-foreground shadow-md outline-none data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95" "z-50 min-w-[12rem] origin-(--transform-origin) overflow-hidden rounded-md border border-border bg-popover p-1 text-popover-foreground shadow-md outline-none data-open:animate-in data-open:fade-in-0 data-open:zoom-in-95 data-closed:animate-out data-closed:fade-out-0 data-closed:zoom-out-95"
const ITEM_CLASS = const ITEM_CLASS =
"flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-xs outline-none select-none data-highlighted:bg-[var(--ui-sidebar-hover)] data-disabled:pointer-events-none data-disabled:opacity-50" "flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-xs outline-none select-none data-highlighted:bg-muted data-disabled:pointer-events-none data-disabled:opacity-50"
const LABEL_CLASS = const LABEL_CLASS =
"px-2 py-1 text-[10px] font-medium tracking-wide text-[var(--ui-text-dim)] uppercase" "px-2 py-1 text-[10px] font-medium tracking-wide text-muted-foreground uppercase"
const SEPARATOR_CLASS = "my-1 h-px bg-[var(--ui-border)]" const SEPARATOR_CLASS = "my-1 h-px bg-border"
function Indicator() { function Indicator() {
return <CheckIcon className="size-3.5 shrink-0" weight="bold" /> return <CheckIcon className="size-3.5 shrink-0" weight="bold" />
@ -38,7 +38,7 @@ function Indicator() {
function CountBadge({ count }: { count: number }) { function CountBadge({ count }: { count: number }) {
if (count <= 0) return null if (count <= 0) return null
return ( return (
<span className="ml-auto rounded bg-[var(--ui-panel-2)] px-1.5 py-0.5 text-[10px] text-[var(--ui-text-muted)]"> <span className="ml-auto rounded bg-muted px-1.5 py-0.5 text-[10px] text-muted-foreground">
{count} {count}
</span> </span>
) )

14
ui/src/components/agents/utils/diffUtils.ts
View file

@ -70,6 +70,18 @@ export const DIFF_UNSAFE_CSS = `
[data-gutter-buffer="annotation"][data-selected-line] { [data-gutter-buffer="annotation"][data-selected-line] {
--diffs-line-bg: var(--ui-panel) !important; --diffs-line-bg: var(--ui-panel) !important;
} }
/* Pin every code row to one exact, uniform height (kept in sync with
DIFF_VIRTUAL_METRICS.lineHeight below). In scroll mode code never wraps, so a
hard height won't clip content — it just makes the virtualizer's per-line
estimate match measured layout, so scroll-to lands precisely instead of
over/under-shooting as off-estimate rows reconcile while scrolling. */
[data-line] {
height: 18px !important;
min-height: 18px !important;
max-height: 18px !important;
line-height: 18px !important;
}
` `
export const diffOptions = { export const diffOptions = {
@ -104,6 +116,8 @@ export const DIFF_VIRTUALIZER_CONFIG = {
export const DIFF_VIRTUAL_METRICS = { export const DIFF_VIRTUAL_METRICS = {
hunkLineCount: 80, hunkLineCount: 80,
// Must match the hard `[data-line]` height pinned in DIFF_UNSAFE_CSS so the
// virtualizer's pre-measurement estimate equals the measured row height.
lineHeight: 18, lineHeight: 18,
diffHeaderHeight: 0, diffHeaderHeight: 0,
spacing: 8, spacing: 8,

2
ui/src/lib/api.ts
View file

@ -823,7 +823,7 @@ export const api = {
export function loginUrl(redirectTo?: string): string { export function loginUrl(redirectTo?: string): string {
const target = const target =
redirectTo ?? (typeof window !== "undefined" ? window.location.origin : "") redirectTo ?? (typeof window !== "undefined" ? window.location.href : "")
const qs = target ? `?redirect_to=${encodeURIComponent(target)}` : "" const qs = target ? `?redirect_to=${encodeURIComponent(target)}` : ""
return `${API_BASE}/dashboard/api/auth/login${qs}` return `${API_BASE}/dashboard/api/auth/login${qs}`
} }

120
ui/src/lib/auth-redirect-core.ts Normal file
View file

@ -0,0 +1,120 @@
export const DEFAULT_AUTH_REDIRECT = "/agents"
export const AUTH_REDIRECT_STORAGE_KEY = "open-swe-auth-redirect"
type LocationParts = {
pathname: string
search?: string
hash?: string
}
function browserOrigin(): string | null {
return typeof window === "undefined" ? null : window.location.origin
}
function storage(): Storage | null {
if (typeof window === "undefined") return null
try {
return window.sessionStorage
} catch {
return null
}
}
function isBlockedRedirectPath(path: string): boolean {
return /^(?:\/login|\/dashboard\/api|\/_serverFn)(?:[/?#]|$)/.test(path)
}
export function sanitizeAuthRedirect(
candidate: unknown,
fallback = DEFAULT_AUTH_REDIRECT
): string {
if (typeof candidate !== "string") return fallback
const trimmed = candidate.trim()
if (!trimmed) return fallback
const origin = browserOrigin()
const isProtocolRelative = trimmed.startsWith("//")
const hasScheme = /^[a-zA-Z][a-zA-Z\d+.-]*:/.test(trimmed)
if (isProtocolRelative) return fallback
if (hasScheme && !origin) return fallback
let parsed: URL
try {
parsed = new URL(trimmed, origin ?? "https://open-swe.invalid")
} catch {
return fallback
}
if ((hasScheme || origin) && origin && parsed.origin !== origin) {
return fallback
}
const path = `${parsed.pathname}${parsed.search}${parsed.hash}`
if (!path.startsWith("/") || isBlockedRedirectPath(path)) return fallback
return path
}
export function authRedirectPathFromLocation(location: LocationParts): string {
const hash = location.hash
? location.hash.startsWith("#")
? location.hash
: `#${location.hash}`
: ""
return sanitizeAuthRedirect(
`${location.pathname}${location.search ?? ""}${hash}`
)
}
export function currentAuthRedirectPath(): string {
if (typeof window === "undefined") return DEFAULT_AUTH_REDIRECT
return authRedirectPathFromLocation(window.location)
}
export function rememberAuthRedirect(candidate: unknown): string {
const path = sanitizeAuthRedirect(candidate)
const s = storage()
if (s) {
try {
s.setItem(AUTH_REDIRECT_STORAGE_KEY, path)
} catch {}
}
return path
}
export function getRememberedAuthRedirect(): string | null {
const s = storage()
if (!s) return null
let raw: string | null = null
try {
raw = s.getItem(AUTH_REDIRECT_STORAGE_KEY)
} catch {
return null
}
if (!raw) return null
const path = sanitizeAuthRedirect(raw, "")
if (path) return path
clearRememberedAuthRedirect()
return null
}
export function clearRememberedAuthRedirect(): void {
const s = storage()
if (!s) return
try {
s.removeItem(AUTH_REDIRECT_STORAGE_KEY)
} catch {}
}
export function consumeAuthRedirect(candidate?: unknown): string {
const explicit = sanitizeAuthRedirect(candidate, "")
const path = explicit || getRememberedAuthRedirect() || DEFAULT_AUTH_REDIRECT
clearRememberedAuthRedirect()
return path
}
export function authRedirectUrl(candidate?: unknown): string {
const path = sanitizeAuthRedirect(candidate)
const origin = browserOrigin()
if (!origin) return path
return new URL(path, origin).toString()
}

76
ui/src/lib/auth-redirect.test.ts Normal file
View file

@ -0,0 +1,76 @@
/** @vitest-environment jsdom */
import { beforeEach, describe, expect, it } from "vitest"
import { loginUrl } from "./api"
import {
AUTH_REDIRECT_STORAGE_KEY,
DEFAULT_AUTH_REDIRECT,
authRedirectPathFromLocation,
authRedirectUrl,
consumeAuthRedirect,
currentAuthRedirectPath,
getRememberedAuthRedirect,
rememberAuthRedirect,
sanitizeAuthRedirect,
} from "./auth-redirect-core"
beforeEach(() => {
window.sessionStorage.clear()
window.history.pushState({}, "", "/")
})
describe("auth redirect helpers", () => {
it("captures protected route targets as relative paths", () => {
const path = authRedirectPathFromLocation({
pathname: "/agents/thread-1/plan",
search: "?from=slack",
hash: "#review",
})
expect(path).toBe("/agents/thread-1/plan?from=slack#review")
expect(rememberAuthRedirect(path)).toBe(path)
expect(window.sessionStorage.getItem(AUTH_REDIRECT_STORAGE_KEY)).toBe(path)
})
it("resolves login targets to absolute same-origin URLs", () => {
const path = rememberAuthRedirect("/agents/thread-1/plan?from=slack#review")
const target = `${window.location.origin}/agents/thread-1/plan?from=slack#review`
expect(authRedirectUrl(path)).toBe(target)
expect(loginUrl(authRedirectUrl(path))).toContain(
encodeURIComponent(target)
)
})
it("consumes remembered targets and clears session storage", () => {
rememberAuthRedirect("/agents/thread-1/plan")
expect(consumeAuthRedirect()).toBe("/agents/thread-1/plan")
expect(getRememberedAuthRedirect()).toBeNull()
})
it("falls back for unsafe targets", () => {
expect(
sanitizeAuthRedirect("https://evil.example/agents/thread-1/plan")
).toBe(DEFAULT_AUTH_REDIRECT)
expect(sanitizeAuthRedirect("//evil.example/agents/thread-1/plan")).toBe(
DEFAULT_AUTH_REDIRECT
)
expect(sanitizeAuthRedirect("/login?redirect=/agents/thread-1/plan")).toBe(
DEFAULT_AUTH_REDIRECT
)
})
it("builds a plan sign-in target for the current plan URL", () => {
window.history.pushState({}, "", "/agents/thread-1/plan?from=slack")
expect(currentAuthRedirectPath()).toBe("/agents/thread-1/plan?from=slack")
expect(loginUrl(authRedirectUrl(currentAuthRedirectPath()))).toContain(
encodeURIComponent(
`${window.location.origin}/agents/thread-1/plan?from=slack`
)
)
})
})

13
ui/src/lib/auth-redirect.tsx Normal file
View file

@ -0,0 +1,13 @@
import { Navigate } from "@tanstack/react-router"
import {
currentAuthRedirectPath,
rememberAuthRedirect,
} from "./auth-redirect-core"
export * from "./auth-redirect-core"
export function RequireLogin() {
const redirect = rememberAuthRedirect(currentAuthRedirectPath())
return <Navigate to="/login" search={{ redirect }} />
}

3
ui/src/routes/admin.tsx
View file

@ -23,6 +23,7 @@ import {
} from "@/components/ui/select" } from "@/components/ui/select"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { api } from "@/lib/api" import { api } from "@/lib/api"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
export const Route = createFileRoute("/admin")({ component: AdminPage }) export const Route = createFileRoute("/admin")({ component: AdminPage })
@ -43,7 +44,7 @@ function AdminPage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
if (!session.data.is_admin) return <Navigate to="/my-settings" /> if (!session.data.is_admin) return <Navigate to="/my-settings" />
return ( return (

3
ui/src/routes/admin_.evals.tsx
View file

@ -7,6 +7,7 @@ import { AppShell, SettingsSection } from "@/components/AppShell"
import { Button } from "@/components/ui/button" import { Button } from "@/components/ui/button"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { api } from "@/lib/api" import { api } from "@/lib/api"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
export const Route = createFileRoute("/admin_/evals")({ component: ReviewerEvalPage }) export const Route = createFileRoute("/admin_/evals")({ component: ReviewerEvalPage })
@ -21,7 +22,7 @@ function ReviewerEvalPage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
if (!session.data.is_admin) return <Navigate to="/my-settings" /> if (!session.data.is_admin) return <Navigate to="/my-settings" />
return ( return (

10
ui/src/routes/agents.tsx
View file

@ -1,14 +1,10 @@
import { import { Outlet, createFileRoute, useRouterState } from "@tanstack/react-router"
Navigate,
Outlet,
createFileRoute,
useRouterState,
} from "@tanstack/react-router"
import { AgentsShell } from "@/components/agents/AgentsSidebar" import { AgentsShell } from "@/components/agents/AgentsSidebar"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import agentsCss from "@/styles/agents.css?url" import agentsCss from "@/styles/agents.css?url"
import { AgentThreadStreamProvider } from "@/lib/agents/AgentThreadStreamProvider" import { AgentThreadStreamProvider } from "@/lib/agents/AgentThreadStreamProvider"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
export const Route = createFileRoute("/agents")({ export const Route = createFileRoute("/agents")({
@ -41,7 +37,7 @@ function AgentsLayout() {
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
return ( return (
<AgentsShell user={session.data} activeThreadId={activeThreadId}> <AgentsShell user={session.data} activeThreadId={activeThreadId}>

20
ui/src/routes/agents/$threadId_.plan.tsx
View file

@ -4,7 +4,10 @@ import { useEffect, useState } from "react"
import { ArrowLeft } from "lucide-react" import { ArrowLeft } from "lucide-react"
import { PlanReview } from "@/components/agents/PlanReview" import { PlanReview } from "@/components/agents/PlanReview"
import { buttonVariants } from "@/components/ui/button"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { loginUrl } from "@/lib/api"
import { authRedirectUrl, currentAuthRedirectPath } from "@/lib/auth-redirect"
import { PlanApiError, getPlan } from "@/lib/plan" import { PlanApiError, getPlan } from "@/lib/plan"
export const Route = createFileRoute("/agents/$threadId_/plan")({ export const Route = createFileRoute("/agents/$threadId_/plan")({
@ -13,7 +16,7 @@ export const Route = createFileRoute("/agents/$threadId_/plan")({
function Centered({ children }: { children: React.ReactNode }) { function Centered({ children }: { children: React.ReactNode }) {
return ( return (
<div className="flex min-w-0 flex-1 items-center justify-center p-6"> <div className="flex min-w-0 flex-1 items-center justify-center px-4 py-6 max-md:pt-14 md:p-6">
{children} {children}
</div> </div>
) )
@ -32,6 +35,18 @@ function BackLink({ threadId }: { threadId: string }) {
) )
} }
export function planSignInHref(): string {
return loginUrl(authRedirectUrl(currentAuthRedirectPath()))
}
export function PlanSignInButton() {
return (
<a href={planSignInHref()} className={buttonVariants({ size: "sm" })}>
Sign in to view this plan
</a>
)
}
function PlanPage() { function PlanPage() {
const { threadId } = Route.useParams() const { threadId } = Route.useParams()
@ -70,6 +85,7 @@ function PlanPage() {
? "Please sign in to view this plan." ? "Please sign in to view this plan."
: "This plan could not be found."} : "This plan could not be found."}
</p> </p>
{status === 401 ? <PlanSignInButton /> : null}
<BackLink threadId={threadId} /> <BackLink threadId={threadId} />
</div> </div>
</Centered> </Centered>
@ -100,7 +116,7 @@ function PlanPage() {
return ( return (
<div className="flex min-w-0 flex-1 flex-col"> <div className="flex min-w-0 flex-1 flex-col">
<div className="border-b border-[var(--ui-border)] px-6 pt-3"> <div className="border-b border-[var(--ui-border)] px-4 pt-14 md:px-6 md:pt-3">
<BackLink threadId={threadId} /> <BackLink threadId={threadId} />
</div> </div>
<PlanReview plan={plan} /> <PlanReview plan={plan} />

5
ui/src/routes/agents/reviews/$owner.$repo.$number.tsx
View file

@ -1,4 +1,4 @@
import { Link, Navigate, createFileRoute } from "@tanstack/react-router" import { Link, createFileRoute } from "@tanstack/react-router"
import { useQuery, useQueryClient } from "@tanstack/react-query" import { useQuery, useQueryClient } from "@tanstack/react-query"
import { useCallback, useEffect, useRef, useState } from "react" import { useCallback, useEffect, useRef, useState } from "react"
import { ArrowLeftIcon, GitPullRequestIcon } from "@phosphor-icons/react" import { ArrowLeftIcon, GitPullRequestIcon } from "@phosphor-icons/react"
@ -9,6 +9,7 @@ import { ReviewMainBody } from "@/components/agents/ReviewMainBody"
import { useSidebarControls } from "@/components/sidebar-layout" import { useSidebarControls } from "@/components/sidebar-layout"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { api } from "@/lib/api" import { api } from "@/lib/api"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
import { cn } from "@/lib/utils" import { cn } from "@/lib/utils"
@ -70,7 +71,7 @@ function ReviewDetailPage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
return ( return (
<div className="flex min-w-0 flex-1 flex-col overflow-hidden bg-background text-foreground"> <div className="flex min-w-0 flex-1 flex-col overflow-hidden bg-background text-foreground">

5
ui/src/routes/agents_.instructions.tsx
View file

@ -1,8 +1,9 @@
import { Navigate, createFileRoute } from "@tanstack/react-router"; import { createFileRoute } from "@tanstack/react-router";
import { AgentInstructionsPanel } from "@/components/AgentInstructionsPanel"; import { AgentInstructionsPanel } from "@/components/AgentInstructionsPanel";
import { AppShell } from "@/components/AppShell"; import { AppShell } from "@/components/AppShell";
import { Skeleton } from "@/components/ui/skeleton"; import { Skeleton } from "@/components/ui/skeleton";
import { RequireLogin } from "@/lib/auth-redirect";
import { useSession } from "@/lib/session"; import { useSession } from "@/lib/session";
export const Route = createFileRoute("/agents_/instructions")({ export const Route = createFileRoute("/agents_/instructions")({
@ -19,7 +20,7 @@ function AgentInstructionsPage() {
</main> </main>
); );
} }
if (!session.data) return <Navigate to="/login" />; if (!session.data) return <RequireLogin />;
return ( return (
<AppShell <AppShell

3
ui/src/routes/agents_.snapshots.tsx
View file

@ -2,6 +2,7 @@ import { Navigate, createFileRoute } from "@tanstack/react-router"
import { AppShell } from "@/components/AppShell" import { AppShell } from "@/components/AppShell"
import { RepoSnapshotsPanel } from "@/components/RepoSnapshotsPanel" import { RepoSnapshotsPanel } from "@/components/RepoSnapshotsPanel"
import { RequireLogin } from "@/lib/auth-redirect"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
@ -19,7 +20,7 @@ function RepoSnapshotsPage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
if (!session.data.is_admin) return <Navigate to="/my-settings" /> if (!session.data.is_admin) return <Navigate to="/my-settings" />
return ( return (

5
ui/src/routes/cloud-agents.tsx
View file

@ -1,4 +1,4 @@
import { Link, Navigate, createFileRoute } from "@tanstack/react-router" import { Link, createFileRoute } from "@tanstack/react-router"
import { CaretRightIcon } from "@phosphor-icons/react" import { CaretRightIcon } from "@phosphor-icons/react"
import { useEffect, useRef, useState } from "react" import { useEffect, useRef, useState } from "react"
@ -23,6 +23,7 @@ import {
useRepos, useRepos,
useSaveProfile, useSaveProfile,
} from "@/lib/profile" } from "@/lib/profile"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
export const Route = createFileRoute("/cloud-agents")({ export const Route = createFileRoute("/cloud-agents")({
@ -112,7 +113,7 @@ function CloudAgentsPage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
const fallbackModel = defaultAgentModel const fallbackModel = defaultAgentModel
const fallbackEffort = defaultAgentEffort const fallbackEffort = defaultAgentEffort

53
ui/src/routes/login.tsx
View file

@ -1,16 +1,44 @@
import { Navigate, createFileRoute } from "@tanstack/react-router"; import { createFileRoute } from "@tanstack/react-router";
import { useEffect, useMemo } from "react";
import { buttonVariants } from "@/components/ui/button"; import { buttonVariants } from "@/components/ui/button";
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
import { Skeleton } from "@/components/ui/skeleton"; import { Skeleton } from "@/components/ui/skeleton";
import { loginUrl } from "@/lib/api"; import { loginUrl } from "@/lib/api";
import {
DEFAULT_AUTH_REDIRECT,
authRedirectUrl,
consumeAuthRedirect,
getRememberedAuthRedirect,
rememberAuthRedirect,
} from "@/lib/auth-redirect";
import { useSession } from "@/lib/session"; import { useSession } from "@/lib/session";
import { cn } from "@/lib/utils"; import { cn } from "@/lib/utils";
export const Route = createFileRoute("/login")({ component: Login }); type LoginSearch = { redirect?: string };
export const Route = createFileRoute("/login")({
validateSearch: (search: Record<string, unknown>): LoginSearch => ({
redirect: typeof search.redirect === "string" ? search.redirect : undefined,
}),
component: Login,
});
function Login() { function Login() {
const session = useSession(); const session = useSession();
const search = Route.useSearch();
const redirectParam = search.redirect;
const intendedPath = useMemo(
() =>
redirectParam
? rememberAuthRedirect(redirectParam)
: getRememberedAuthRedirect() ?? DEFAULT_AUTH_REDIRECT,
[redirectParam]
);
const authenticatedRedirect = useMemo(
() => (session.data ? consumeAuthRedirect(redirectParam) : null),
[redirectParam, session.data]
);
if (session.isLoading) { if (session.isLoading) {
return ( return (
@ -20,8 +48,8 @@ function Login() {
); );
} }
if (session.data) { if (authenticatedRedirect) {
return <Navigate to="/my-settings" />; return <ClientRedirect path={authenticatedRedirect} />;
} }
return ( return (
@ -35,7 +63,10 @@ function Login() {
</CardDescription> </CardDescription>
</CardHeader> </CardHeader>
<CardContent> <CardContent>
<a href={loginUrl()} className={cn(buttonVariants({ size: "lg" }), "w-full")}> <a
href={loginUrl(authRedirectUrl(intendedPath))}
className={cn(buttonVariants({ size: "lg" }), "w-full")}
>
Continue with GitHub Continue with GitHub
</a> </a>
</CardContent> </CardContent>
@ -43,3 +74,15 @@ function Login() {
</main> </main>
); );
} }
function ClientRedirect({ path }: { path: string }) {
useEffect(() => {
if (typeof window !== "undefined") window.location.replace(path);
}, [path]);
return (
<main className="flex min-h-svh items-center justify-center p-6">
<Skeleton className="h-40 w-80" />
</main>
);
}

5
ui/src/routes/my-settings.tsx
View file

@ -1,4 +1,4 @@
import { Navigate, createFileRoute, useNavigate } from "@tanstack/react-router" import { createFileRoute, useNavigate } from "@tanstack/react-router"
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"
import { useState } from "react" import { useState } from "react"
import { IoLogoSlack } from "react-icons/io5" import { IoLogoSlack } from "react-icons/io5"
@ -24,6 +24,7 @@ import {
useProfile, useProfile,
useSaveProfile, useSaveProfile,
} from "@/lib/profile" } from "@/lib/profile"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
import { import {
notificationsEnabled, notificationsEnabled,
@ -366,7 +367,7 @@ function MySettingsPage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
const handleLogout = async () => { const handleLogout = async () => {
await api.logout() await api.logout()

5
ui/src/routes/review.tsx
View file

@ -1,4 +1,4 @@
import { Link, Navigate, createFileRoute } from "@tanstack/react-router"; import { Link, createFileRoute } from "@tanstack/react-router";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { CaretRightIcon } from "@phosphor-icons/react"; import { CaretRightIcon } from "@phosphor-icons/react";
import { useEffect, useMemo, useState } from "react"; import { useEffect, useMemo, useState } from "react";
@ -11,6 +11,7 @@ import { Skeleton } from "@/components/ui/skeleton";
import { Switch } from "@/components/ui/switch"; import { Switch } from "@/components/ui/switch";
import { Textarea } from "@/components/ui/textarea"; import { Textarea } from "@/components/ui/textarea";
import { ApiError, api } from "@/lib/api"; import { ApiError, api } from "@/lib/api";
import { RequireLogin } from "@/lib/auth-redirect";
import { useSession } from "@/lib/session"; import { useSession } from "@/lib/session";
export const Route = createFileRoute("/review")({ component: ReviewPage }); export const Route = createFileRoute("/review")({ component: ReviewPage });
@ -66,7 +67,7 @@ function ReviewPage() {
</main> </main>
); );
} }
if (!session.data) return <Navigate to="/login" />; if (!session.data) return <RequireLogin />;
const current: TeamSettings = local; const current: TeamSettings = local;
const canEdit = session.data.is_admin; const canEdit = session.data.is_admin;

5
ui/src/routes/review_.repositories.$owner.tsx
View file

@ -1,4 +1,4 @@
import { Navigate, createFileRoute } from "@tanstack/react-router"; import { createFileRoute } from "@tanstack/react-router";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { useEffect, useMemo, useState } from "react"; import { useEffect, useMemo, useState } from "react";
@ -8,6 +8,7 @@ import { Button } from "@/components/ui/button";
import { Skeleton } from "@/components/ui/skeleton"; import { Skeleton } from "@/components/ui/skeleton";
import { Switch } from "@/components/ui/switch"; import { Switch } from "@/components/ui/switch";
import { ApiError, api } from "@/lib/api"; import { ApiError, api } from "@/lib/api";
import { RequireLogin } from "@/lib/auth-redirect";
import { useSession } from "@/lib/session"; import { useSession } from "@/lib/session";
const PAGE_SIZE = 20; const PAGE_SIZE = 20;
@ -78,7 +79,7 @@ function RepositoriesOwnerPage() {
</main> </main>
); );
} }
if (!session.data) return <Navigate to="/login" />; if (!session.data) return <RequireLogin />;
const canEdit = session.data.is_admin; const canEdit = session.data.is_admin;
const enabledCount = ownerRepos.filter((r) => enabledSet.has(r.full_name)).length; const enabledCount = ownerRepos.filter((r) => enabledSet.has(r.full_name)).length;

5
ui/src/routes/review_.styles.tsx
View file

@ -1,8 +1,9 @@
import { Navigate, createFileRoute } from "@tanstack/react-router"; import { createFileRoute } from "@tanstack/react-router";
import { AppShell } from "@/components/AppShell"; import { AppShell } from "@/components/AppShell";
import { ReviewStylesPanel } from "@/components/ReviewStylesPanel"; import { ReviewStylesPanel } from "@/components/ReviewStylesPanel";
import { Skeleton } from "@/components/ui/skeleton"; import { Skeleton } from "@/components/ui/skeleton";
import { RequireLogin } from "@/lib/auth-redirect";
import { useSession } from "@/lib/session"; import { useSession } from "@/lib/session";
export const Route = createFileRoute("/review_/styles")({ component: ReviewStylesPage }); export const Route = createFileRoute("/review_/styles")({ component: ReviewStylesPage });
@ -17,7 +18,7 @@ function ReviewStylesPage() {
</main> </main>
); );
} }
if (!session.data) return <Navigate to="/login" />; if (!session.data) return <RequireLogin />;
return ( return (
<AppShell <AppShell

5
ui/src/routes/usage.tsx
View file

@ -1,4 +1,4 @@
import { Navigate, createFileRoute } from "@tanstack/react-router" import { createFileRoute } from "@tanstack/react-router"
import { useQuery } from "@tanstack/react-query" import { useQuery } from "@tanstack/react-query"
import type { import type {
@ -17,6 +17,7 @@ import {
} from "@/components/ui/select" } from "@/components/ui/select"
import { Skeleton } from "@/components/ui/skeleton" import { Skeleton } from "@/components/ui/skeleton"
import { api } from "@/lib/api" import { api } from "@/lib/api"
import { RequireLogin } from "@/lib/auth-redirect"
import { useSession } from "@/lib/session" import { useSession } from "@/lib/session"
export const Route = createFileRoute("/usage")({ export const Route = createFileRoute("/usage")({
@ -57,7 +58,7 @@ function UsagePage() {
</main> </main>
) )
} }
if (!session.data) return <Navigate to="/login" /> if (!session.data) return <RequireLogin />
return ( return (
<AppShell user={session.data} title="Usage" className="max-w-5xl"> <AppShell user={session.data} title="Usage" className="max-w-5xl">