mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 12:43:16 +00:00
* chore: bake sfw binary into sandbox image (#1611) sfw only ships a launcher that fetches its real binary at first run and does a daily update check against api.github.com/repos/SocketDev/sfw-free. Both fail in the sandbox (restricted egress; the proxy injects the GitHub App installation token, which lacks access to that repo), so `sfw yarn install` errors with "could not fetch its binary". Pin sfw 2.0.6, warm + verify the binary cache at build, and set SFW_SKIP_UPDATE_CHECK=1 so runs use the baked binary offline. * feat: editable plan mode + fix review-plan banner overlap (#1610) * feat: editable plan mode + fix review-plan banner overlap Lets the thread owner edit the plan markdown by hand from the plan-review page (Edit -> textarea -> Save) via a new PUT /dashboard/api/plan/{id} endpoint that re-publishes the plan and mirrors it into the sandbox plan.md, so approve hands the edited plan to the agent as the source of truth. Also fixes the collapsed git-panel's floating expand button covering the "Review plan ->" banner by reserving space for it. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: abort plan approval when the published plan read fails get_plan_content() swallowed store errors and returned None, so a transient failure during approve would still mark the plan approved and dispatch the generic fallback text — silently dropping an owner's edited plan. Read the plan strictly (raise_on_error=True) so approval aborts instead, matching the comment read. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: show message timestamps (#1609) * feat: show message timestamps Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: suppress fallback message timestamps Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: stable message + tool-call hover timestamps Stamp a stable client-side arrival time per message and tool call (keyed by id, persisted to localStorage). Messages render the timestamp inline; tool rows reveal a dim timestamp chip on hover. Real backend created_at still takes precedence when present. * fix: hide client-stamped message timestamps Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: add PR trace resolution (#1612) * feat: add PR trace resolution Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: inject reviewer trace context as JSON Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: address review on PR trace resolution Use the documented LangSmith metadata filter syntax (and(eq(metadata_key,...), eq(metadata_value,...))) instead of has(metadata, '{...}'), which does not match runs — _list_thread_runs was silently returning nothing. Bound full-text searches to a 90-day window so they don't hit LangSmith's large-window rate limit. Also folds in the best-effort branch->head-sha resolver (dropping the weighted scoring/threshold + repo/file evidence + GitHub hydration), sandbox JSON injection, and the admin "Resolve trace" dry-run endpoint. The IDOR findings are moot: resolve_pr_to_threads/summarize_agent_session were removed; resolution now runs deterministically from the trusted run config with no model-controlled pr_url or thread_id. * fix: scope branch trace search to the repo Branch names like fix-tests aren't unique across repos (or older PRs) in a shared tracing project, so an unscoped branch hit could resolve to an unrelated thread and write its runs into the reviewer sandbox. Require the repo slug to co-occur with the branch in matched runs; the full head SHA stays unscoped since it is globally unique. Addresses open-swe review on PR #1612. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: include plan links in PR descriptions (#1613) Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: gate workflow pushes with approval (#1614) * feat: gate workflow pushes with approval Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: preserve proxy refresh test compatibility Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: bind workflow approvals to pushed ref Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: recover thread work as patch (#1615) * feat: recover thread work as patch Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: search sandbox cwd for recovery patches Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: omit plan link in PR description when no plan exists (#1618) Plan links in PR descriptions were always built from the thread id, so runs that never produced a plan linked to an empty plan-review page. Now the plan content store is consulted first; the link is only added when a plan with non-empty markdown actually exists. A transient store failure degrades gracefully (no link) rather than blocking PR creation. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: add filter & grouping menu to agents threads sidebar (#1617) Add a Cursor-style control to the agents sidebar that groups (None/Date/ Status/Project), filters (ownership, status, source, pull request, model, repo, include-resolved), and compacts the threads list. All client-side over already-fetched sidebar threads; preferences persist in localStorage. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: update langsmith sdk to 0.9.3 (#1616) Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: clickable shared PR header in git panel and reviews (#1620) * feat: clickable shared PR header in git panel and reviews Replace the standalone "View PR" button in the agent git panel with a clickable PR title, matching the reviews view. Extract a shared PrHeader component reused by both the git panel and the review main body. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * refactor: drop PrHeader wrapper, use shared component directly The review-side PrHeader was just a thin adapter mapping detail -> the shared component's props. Inline it at the call site and use the shared PrHeader directly so there's a single component. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * refactor: durable interrupt dispatch + completion webhook (#1621) * wip(rebuild): core reliability spine - remove PR-babysitting (ci_autofix + ci_monitor graph + webhook wiring) - dispatch core: agent/dispatch.py with multitask_strategy=interrupt + durability=sync + completion webhook; reroute all webhook + plan triggers; drop the racy in-process lock + is_thread_active busy-check - completion webhook: agent/completion.py + /webhooks/run-complete loopback route for failure/timeout replies (idempotent) Co-authored-by: open-swe[bot] * feat(rebuild): async tools, reconcile, shared http timeouts, assembly tuning Parallel batch on top of the reliability spine: - async-ify all 24 tools (drop asyncio.run; requests->httpx); re-implement the http_request/fetch_url SSRF + DNS-rebinding defense httpx-natively and harden the IP check to 'not is_global' (+ IPv4-mapped unwrap) - reconcile.py: stale pending-run sweep (threads.search -> per-thread runs.list -> cancel_many), wired into the scheduler graph via task='reconcile' - shared DEFAULT_HTTP_TIMEOUT (agent/utils/http.py) on every bare httpx.AsyncClient() across utils/dashboard/webapp/middleware - run budget: MODEL_CALL_RECURSION_LIMIT 5000->250 - fix stale OpenAI->Anthropic fallback id (claude-opus-4-5 -> 4-8) - drop redundant custom repair middleware (deepagents auto-adds PatchToolCalls) - confirm tool-result eviction + summarization auto-wired via backend - slim system prompt ~8% (full harness-profile rewrite deferred) Co-authored-by: open-swe[bot] * feat(rebuild): harness-profile prompt + split webhooks out of webapp - prompt.py: own the system prompt via a registered harness profile (OPEN_SWE_SHARED_BASE, kept neutral so the read-only reviewer/analyzer that share it stay safe), registered across all 4 providers; per-thread values stay in construct_system_prompt. Assembled main-agent prompt ~6.8k -> ~3.1k tokens (~55% smaller); de-duped PR/commit/suite/force-push guidance; dropped ALL-CAPS markers. - webapp.py 3325 -> 1890 LOC: moved 14 per-source handlers into agent/webhooks/{linear,slack,github}.py; webapp re-exports them for the routes + tests; moved handlers reach shared helpers via the webapp namespace to preserve the test suite's monkeypatch targets. Full suite: 1168 passing, lint clean. Co-authored-by: open-swe[bot] * Restore MODEL_CALL_RECURSION_LIMIT to 5000 for long-running tasks Reverts the 250 cap from the run-budget change — long-running tasks legitimately need many model calls. The notify_step_limit_reached safety net still fires if a run does hit the cap, so runs end with a signal either way. Co-authored-by: open-swe[bot] * fix: address PR review (auth, SSRF, interrupted status, redirect headers) - completion.py: drop `interrupted` from failure statuses — with multitask_strategy=interrupt a follow-up ends the prior run as interrupted, which is healthy, not a failure to report. [open-swe] - /webhooks/run-complete: shared-secret auth — dispatch appends ?token= when RUN_COMPLETE_WEBHOOK_SECRET is set; route verifies via hmac.compare_digest. [corridor-security] - SSRF: extract the URL validator to agent/utils/url_safety.py and apply it before server-side image fetches in multimodal.fetch_image_block. [corridor-security] - http_request: preserve caller headers/extensions across redirect hops instead of dropping them on the first hop. [open-swe] Co-authored-by: open-swe[bot] * chore: remove REBUILD_PLAN.md (planning doc, not needed in the repo) Co-authored-by: open-swe[bot] * fix: fail closed on run-complete webhook auth when secret unset Corridor follow-up: verify_run_complete_token returns False (not True) when RUN_COMPLETE_WEBHOOK_SECRET is unset, so the public route is never unauthenticated. Logs a startup warning when the secret is absent, and dispatch skips registering the webhook when there's no secret (no rejected callbacks). Co-authored-by: open-swe[bot] --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: restore forced tool call to prevent premature run stops (#1622) Restore the ensure_no_empty_msg middleware and the always-call-a-tool system-prompt instruction that #1535 removed. When the model emits a message with no tool call (and hasn't already messaged the user or confirmed completion), the middleware re-injects a no_op / confirming_completion tool call so the run continues instead of ending mid-task. Shipping to test whether it fixes runs that stop halfway through. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore(deps): bump langgraph-checkpoint from 4.1.0 to 4.1.1 (#1619) Bumps [langgraph-checkpoint](https://github.com/langchain-ai/langgraph) from 4.1.0 to 4.1.1. - [Release notes](https://github.com/langchain-ai/langgraph/releases) - [Commits](https://github.com/langchain-ai/langgraph/compare/checkpoint==4.1.0...checkpoint==4.1.1) --- updated-dependencies: - dependency-name: langgraph-checkpoint dependency-version: 4.1.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: post reviewer resolution notes verbatim (#1624) * fix: post reviewer resolution notes verbatim Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: stabilize dashboard follow-up e2e Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: preserve dashboard attribution in e2e Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: make e2e attribution marker durable Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: only echo found e2e attribution Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: check live dashboard attribution in e2e Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * hotfix: stop prompting agent/reviewer to wrap installs in sfw (#1625) Installs hung when prefixed with sfw inside the sandbox (trace 019f0608 stalled on a pending `sfw npm install` execute, never returned). Strip the Socket Firewall guidance from the agent and reviewer prompts so installs run through the project's package manager directly. sfw stays in the Docker image; nothing invokes it now. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: make plan view mobile friendly (#1636) Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: fall back to vision model for image threads (#1626) Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: surface Slack thread errors (#1627) * fix: surface Slack thread errors Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: don't set failure_reply_posted on Slack preprocessing errors The preprocessing error handler was setting failure_reply_posted=True, the same idempotency flag handle_run_completion checks to suppress duplicate run-failure replies. Since preprocessing failures happen before any run exists but the flag persists on the thread, a subsequent run failure on the same thread would be silently ignored. The preprocessing handler already posts its own Slack reply, so the run-completion idempotency flag should not be set here. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: avoid recapping Slack replies (#1629) * chore: avoid recapping Slack replies Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: simplify Slack reply prompt wording 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> * fix: update Slack trace reply on web handoff (#1630) * fix: update Slack trace reply on web handoff Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: trigger web handoff on dashboard starts Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: format web handoff as contextual fragment Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: preserve trace_message_ts when overwriting Slack run mapping When store_slack_run_mapping is called without trace_message_ts (e.g. on follow-up Slack mentions), it was unconditionally overwriting the thread-level mapping and clobbering the timestamp captured from the initial trace reply. After that, _notify_slack_web_handoff could not find the original message, so a subsequent move to Web silently skipped the Slack trace update. Now, when trace_message_ts is not passed, the existing thread mapping is read first and its trace_message_ts is preserved. * style: ruff format --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com> Co-authored-by: Johannes du Plessis <johannes@langchain.dev> * fix: pre-bundle shiki/@pierre deps to stop dev dynamic-import failures (#1643) * fix(ui): pre-bundle shiki/@pierre deps to stop dev dynamic-import failures shiki lazy-imports a grammar per language and these libs only live inside lazy route components, so Vite's startup scanner never sees them. They get discovered on first thread navigation, triggering a dep re-optimize + force-reload that aborts the in-flight route-chunk import, surfacing as "Failed to fetch dynamically imported module: .../$threadId.tsx". Pre-bundle them (and the github themes + common code-block languages) via optimizeDeps.include so the optimize happens once at startup. Dev-only; production bundles are unaffected. * fix: pre-bundle canonical shiki docker/make langs instead of aliases --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: show queued dashboard follow-ups (#1631) * feat: show queued dashboard follow-ups Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: de-dupe queued follow-ups while streaming --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: Johannes du Plessis <johannes@langchain.dev> * feat: notify Slack on plan approval (#1632) * feat: notify Slack on plan approval Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: post Slack approval notice after successful dispatch Move the _maybe_post_plan_approved_to_slack call until after _dispatch_followup succeeds so the Slack thread is not told implementation is beginning before the LangGraph run is created. Addresses PR review comment. --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: Johannes du Plessis <johannes@langchain.dev> * feat: include Slack channel context in prompts (#1633) Add cached Slack channel metadata enrichment for Slack-triggered runs so prompts can include channel names and descriptions without duplicate conversations.info calls.\n\nCo-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> * chore: keep plan guidance high-level (#1634) Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: publish plans from sandbox files (#1635) * feat: publish plans from sandbox files Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: avoid fixed plan filenames Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: virtualize local sandbox file paths Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: preserve plan_file_path across set_plan_status set_plan_status was rewriting the content record with only markdown and status, dropping plan_file_path. After a reject, the owner's dashboard edit would mirror to a different file than the agent's original, and the next save_plan could republish the stale file. Preserve plan_file_path when updating status. --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * 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> * feat: add Slack breakout thread tool (#1638) * feat: add Slack breakout thread tool Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: make fake LLM scripts declarative Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: exclude slack_start_new_thread from plan mode The breakout tool can dispatch a fresh agent run that starts outside the current plan-mode state, bypassing the approval flow. Add it to PLAN_MODE_EXCLUDED_TOOLS so it's hidden alongside the other mutating tools while planning. --------- Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: require bun for ui agent work (#1639) Co-authored-by: Ramon Nogueira <270434257+ramon-langchain@users.noreply.github.com> Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: request actions read for sandbox logs (#1642) * fix: request actions read for sandbox logs Request optional Actions read permission for sandbox proxy tokens, with fallback for installations that have not approved it yet. Update setup docs and prompt guidance for safe GitHub Actions log usage. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: restore actions:read scope after workflow push After an approved workflow push, the guard was restoring the proxy with BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS, which excludes the actions: read scope this PR adds. Restore with RUNTIME_PROXY_TOKEN_PERMISSIONS (which includes actions: read) and fall back to BASE if the install hasn't granted Actions read — mirroring the pattern in _create_sandbox_with_proxy. Addresses review comment on PR #1642. --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * fix: widen split review diffs (#1647) Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: install missing deps before verification (#1646) Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: switch ui to pnpm (#1645) * chore: require pnpm for ui agent work Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: switch ui to pnpm Replace Bun and Yarn lockfiles with pnpm lockfile and update UI/Vercel commands to use pnpm. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * ci: use corepack for ui pnpm e2e build Run pnpm through Corepack in the E2E global setup so CI can use the pinned package manager without a separate pnpm install step. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * feat: add Sonnet 5 to model picker (#1651) * chore: update Sonnet examples to Sonnet 5 Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * chore: add Sonnet 5 to model picker Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> * Remove dead breakout-thread e2e scenario after dropping the tool The merge resolution deferred upstream's Slack breakout-thread tool (slack_start_new_thread, #1638) since it depends on the #1621 dispatch module, but the e2e harness still scripted it. Removing the tool name from fake_llm.py's _tool_step call left a malformed scenario, crashing the langgraph-dev web server at import (TypeError: _tool_step() missing 'call_id') and failing Playwright E2E. Drop the "breakout" script scenario, its _is_breakout_request helper + ScriptRule, and the corresponding full_flow.spec.ts test. * Revert upstream pnpm switch; keep bun for the UI build The merge auto-adopted upstream's pnpm switch (#1645) in tests/e2e/ global-setup.ts and ui/package.json, but our fork builds the UI with bun (vercel.json + the E2E workflow's setup-bun). That left the Playwright globalSetup running `corepack pnpm install --frozen-lockfile` with no pnpm-lock.yaml, failing E2E at UI build time. Revert global-setup.ts and ui/package.json to the dev (bun) baseline, drop the merge-added ui/pnpm-lock.yaml, and remove the re-added ui/AGENTS.md (our fork had deleted it). * Align plan-review e2e + UI with the HEAD (pre-#1635) backend The merge left a split plan vertical: the backend save_plan/plan_api are HEAD (we deferred the editable-plan/sandbox-publish features #1610/#1635/ #1637 per #80), but the plan UI and e2e harness were upstream's. The fake_llm scenario called save_plan(plan_file_path=...) — upstream's file-based #1635 contract — while HEAD save_plan takes plan_markdown, so the plan never saved and PlanReview never rendered (E2E failure on the plan-review locator). Pass plan_markdown to save_plan, and revert PlanReview.tsx / plan.ts / $threadId_.plan.tsx / plan_review.spec.ts to the dev baseline so the whole plan flow (save -> render -> approve -> implement) is consistent with the HEAD backend. --------- Signed-off-by: dependabot[bot] <support@github.com> 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> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com> Co-authored-by: Ankush Gola <9536492+agola11@users.noreply.github.com> Co-authored-by: Mukil Loganathan <mukil@langchain.dev>
1036 lines
39 KiB
Python
1036 lines
39 KiB
Python
"""Tool: ``publish_review``. Post the findings list to GitHub as a PR Review."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from typing import Any
|
|
|
|
from langgraph.config import get_config
|
|
|
|
from ..dashboard.team_settings import get_team_review_trace_links_enabled
|
|
from ..reviewer_diff import compute_diff_line_set, fetch_pr_diff, is_range_in_diff
|
|
from ..reviewer_findings import (
|
|
SEVERITY_ORDER,
|
|
Finding,
|
|
ReviewerThreadMissingError,
|
|
Severity,
|
|
_coerce_surface,
|
|
filter_findings_for_publish,
|
|
get_thread_id_from_runtime,
|
|
get_thread_last_reviewed_sha,
|
|
get_thread_metadata,
|
|
get_thread_slack_ref,
|
|
replace_findings,
|
|
resolve_review_head_sha,
|
|
set_reviewer_thread_metadata,
|
|
thread_missing_tool_result,
|
|
)
|
|
from ..reviewer_findings import (
|
|
list_findings as list_findings_async,
|
|
)
|
|
from ..reviewer_publish import (
|
|
clear_review_started_comment,
|
|
fetch_pr_review_threads,
|
|
fetch_review_comments,
|
|
fetch_review_thread_id_for_comment,
|
|
open_swe_review_exists,
|
|
parse_review_comment_marker,
|
|
post_pull_request_review,
|
|
render_inline_comment_payload,
|
|
render_resolution_comment,
|
|
render_review_body,
|
|
reply_to_review_comment,
|
|
resolve_review_thread,
|
|
settle_review_check_run,
|
|
)
|
|
from ..reviewer_reconcile import reconcile_findings_with_review_threads
|
|
from ..utils.dashboard_links import dashboard_review_url
|
|
from ..utils.github_checks import review_check_conclusion
|
|
from ..utils.github_token import (
|
|
GitHubAuthError,
|
|
get_github_token,
|
|
invalidate_cached_github_token,
|
|
)
|
|
from ..utils.langsmith import get_langsmith_trace_url
|
|
from ..utils.slack import post_slack_thread_reply
|
|
from ..utils.tracing import REVIEW_TRACING_PROJECT
|
|
|
|
|
|
async def publish_review(
|
|
severity_threshold: str = "medium",
|
|
cap: int = 4,
|
|
) -> dict[str, Any]:
|
|
"""Post all current findings to the PR as a GitHub Review.
|
|
|
|
Call this once at the end of a review run, after you have finished adding
|
|
findings (and, on a re-review, after marking resolved findings via
|
|
``update_finding``). The tool posts one GitHub PR Review for eligible
|
|
inline findings, records the GitHub comment/thread IDs for future
|
|
re-reviews, resolves GitHub threads for findings now marked resolved, and
|
|
advances the reviewer thread's ``last_reviewed_sha``.
|
|
|
|
On a re-review with no new findings to surface, it skips posting a new
|
|
GitHub Review but still resolves fixed threads and updates reviewer state.
|
|
|
|
Args:
|
|
severity_threshold: Lowest severity to surface as inline GitHub comments
|
|
(default ``medium``). Lower-severity findings stay in state and are
|
|
mentioned in the review summary with a link to the web app, but are
|
|
not posted as inline PR comments.
|
|
cap: Maximum number of inline comments to publish (default 4).
|
|
|
|
Returns:
|
|
Dictionary with ``success``, ``review_id``, ``surfaced_count``,
|
|
``hidden_count``, ``resolved_thread_count``, and sometimes
|
|
``unresolvable_findings``, plus the flags below.
|
|
|
|
``success: true`` alone does NOT mean a GitHub Review was posted —
|
|
check the flags:
|
|
|
|
- ``skipped_empty_re_review: true`` (with ``review_id: null``): an
|
|
empty re-review was deliberately skipped. No GitHub Review was
|
|
created; the call was a valid no-op. Do not describe the review as
|
|
published/posted/submitted.
|
|
- ``dry_run: true`` (with ``review_id: null``): eval/benchmark mode —
|
|
the publish was simulated and nothing was posted to GitHub. Do not
|
|
claim publication.
|
|
|
|
Only a numeric ``review_id`` (with neither flag set) confirms a real
|
|
GitHub Review was created.
|
|
"""
|
|
if severity_threshold not in {"low", "medium", "high", "critical"}:
|
|
return {"success": False, "error": f"Invalid severity_threshold: {severity_threshold}"}
|
|
|
|
config = get_config()
|
|
raw_configurable = config.get("configurable", {}) if isinstance(config, dict) else {}
|
|
configurable = raw_configurable if isinstance(raw_configurable, dict) else {}
|
|
repo_config = configurable.get("repo")
|
|
pr_number = configurable.get("pr_number")
|
|
head_sha = configurable.get("head_sha")
|
|
is_re_review = bool(configurable.get("re_review"))
|
|
|
|
if (
|
|
not isinstance(repo_config, dict)
|
|
or not repo_config.get("owner")
|
|
or not repo_config.get("name")
|
|
):
|
|
return {"success": False, "error": "Missing repo info in run config"}
|
|
if not isinstance(pr_number, int):
|
|
return {"success": False, "error": "Missing pr_number in run config"}
|
|
if not isinstance(head_sha, str) or not head_sha:
|
|
return {"success": False, "error": "Missing head_sha in run config"}
|
|
|
|
if _is_reviewer_eval_mode(configurable):
|
|
try:
|
|
return await _publish_review_eval_dry_run_async(
|
|
head_sha=head_sha,
|
|
severity_threshold=_cast_severity(severity_threshold),
|
|
cap=cap,
|
|
)
|
|
except ReviewerThreadMissingError as exc:
|
|
return thread_missing_tool_result(exc)
|
|
|
|
token = get_github_token()
|
|
if not token:
|
|
return {"success": False, "error": "No GitHub token available"}
|
|
|
|
try:
|
|
return await _publish_review_async(
|
|
owner=str(repo_config["owner"]),
|
|
repo=str(repo_config["name"]),
|
|
pr_number=pr_number,
|
|
head_sha=head_sha,
|
|
token=token,
|
|
severity_threshold=_cast_severity(severity_threshold),
|
|
cap=cap,
|
|
is_re_review=is_re_review,
|
|
langgraph_run_id=_current_run_id(config),
|
|
trace_link_config_override=configurable.get("review_trace_link_enabled"),
|
|
)
|
|
except ReviewerThreadMissingError as exc:
|
|
return thread_missing_tool_result(exc)
|
|
except GitHubAuthError as exc:
|
|
thread_id = get_thread_id_from_runtime()
|
|
if thread_id:
|
|
await invalidate_cached_github_token(thread_id)
|
|
return {
|
|
"success": False,
|
|
"error": (
|
|
"GitHub returned 401 — the cached OAuth token is invalid or revoked. "
|
|
"Please re-authenticate and trigger the review again."
|
|
),
|
|
"auth_error": str(exc),
|
|
}
|
|
|
|
|
|
def _cast_severity(value: str) -> Severity:
|
|
return value # type: ignore[return-value]
|
|
|
|
|
|
async def _resolve_review_trace_url(thread_id: str, config_override: object) -> str | None:
|
|
if config_override is False:
|
|
return None
|
|
if not await get_team_review_trace_links_enabled():
|
|
return None
|
|
if not thread_id:
|
|
return None
|
|
return get_langsmith_trace_url(thread_id, project_name=REVIEW_TRACING_PROJECT)
|
|
|
|
|
|
def _is_reviewer_eval_mode(configurable: dict[str, Any]) -> bool:
|
|
return configurable.get("reviewer_eval") is True or configurable.get("eval") is True
|
|
|
|
|
|
async def _publish_review_eval_dry_run_async(
|
|
*,
|
|
head_sha: str,
|
|
severity_threshold: Severity,
|
|
cap: int,
|
|
) -> dict[str, Any]:
|
|
"""Simulate publish_review for benchmark runs without posting to GitHub."""
|
|
thread_id = get_thread_id_from_runtime()
|
|
findings = await list_findings_async(thread_id)
|
|
unpublished_findings = [f for f in findings if not _has_publication_identity(f)]
|
|
open_unpublished = [f for f in unpublished_findings if f.get("status", "open") == "open"]
|
|
# Out-of-diff findings are disabled: only in-diff findings are surfaced.
|
|
in_diff_unpublished = [f for f in unpublished_findings if f.get("in_diff", True)]
|
|
eligible = filter_findings_for_publish(
|
|
in_diff_unpublished,
|
|
severity_threshold=severity_threshold,
|
|
cap=cap,
|
|
)
|
|
inline_comments = [
|
|
payload
|
|
for finding in eligible
|
|
if (payload := render_inline_comment_payload(finding)) is not None
|
|
]
|
|
|
|
await set_reviewer_thread_metadata(thread_id, last_reviewed_sha=head_sha)
|
|
|
|
return {
|
|
"success": True,
|
|
"dry_run": True,
|
|
"review_id": None,
|
|
"surfaced_count": len(inline_comments),
|
|
"hidden_count": max(len(open_unpublished) - len(inline_comments), 0),
|
|
"resolved_thread_count": 0,
|
|
}
|
|
|
|
|
|
async def _publish_review_async(
|
|
*,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
head_sha: str,
|
|
token: str,
|
|
severity_threshold: Severity,
|
|
cap: int,
|
|
is_re_review: bool,
|
|
langgraph_run_id: str | None = None,
|
|
trace_link_config_override: object = None,
|
|
) -> dict[str, Any]:
|
|
thread_id = get_thread_id_from_runtime()
|
|
# The run config's head_sha is frozen at run creation; a push that arrived
|
|
# mid-run updated the live head in thread metadata. Prefer that so the
|
|
# review anchors to (and last_reviewed_sha advances to) the commit actually
|
|
# reviewed, not the stale one this run was created for.
|
|
head_sha = await resolve_review_head_sha(thread_id, {"head_sha": head_sha})
|
|
review_trace_url = await _resolve_review_trace_url(thread_id, trace_link_config_override)
|
|
review_ui_url = dashboard_review_url(owner, repo, pr_number)
|
|
findings = await _backfill_findings_from_pr_threads(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
)
|
|
|
|
# Re-reviews only post NEW findings. Anything with a github_review_comment_id
|
|
# already lives on GitHub from a prior publish — reposting would create
|
|
# duplicate inline comments and break the resolve-on-fix flow (only
|
|
# whichever duplicate id we'd cache last would resolve later).
|
|
unpublished_findings = [
|
|
f for f in findings if not isinstance(f.get("github_review_comment_id"), int)
|
|
]
|
|
if is_re_review:
|
|
unpublished_findings = [
|
|
f for f in unpublished_findings if f.get("first_seen_sha") == head_sha
|
|
]
|
|
open_unpublished = [f for f in unpublished_findings if f.get("status", "open") == "open"]
|
|
# In-diff findings become inline comments. Out-of-diff findings are disabled:
|
|
# they are never surfaced on the PR (any legacy in-state ones are treated as
|
|
# hidden).
|
|
in_diff_unpublished = [f for f in unpublished_findings if f.get("in_diff", True)]
|
|
eligible = filter_findings_for_publish(
|
|
in_diff_unpublished, severity_threshold=severity_threshold, cap=cap
|
|
)
|
|
|
|
severity_rank = SEVERITY_ORDER[severity_threshold]
|
|
eligible_ids = {f.get("id") for f in eligible}
|
|
additional_findings_count = sum(
|
|
1
|
|
for f in in_diff_unpublished
|
|
if f.get("id") not in eligible_ids
|
|
and f.get("status", "open") == "open"
|
|
and SEVERITY_ORDER.get(f.get("severity", "low"), 0) < severity_rank
|
|
)
|
|
|
|
inline_comments: list[dict[str, Any]] = []
|
|
eligible_with_payload: list[tuple[dict[str, Any], dict[str, Any]]] = []
|
|
for finding in eligible:
|
|
payload = render_inline_comment_payload(finding)
|
|
if payload is None:
|
|
continue
|
|
inline_comments.append(payload)
|
|
eligible_with_payload.append((dict(finding), payload))
|
|
|
|
# With nothing new to surface, skip the "no issues found" summary if Open
|
|
# SWE has already reviewed this PR — the user already saw the previous
|
|
# result, and posting another summary on every push is noise. We can't rely
|
|
# on the static re_review flag alone: a push that lands mid-run is delivered
|
|
# as a queued message into the still-running first-review run, whose
|
|
# configurable still says re_review=False, so that path would post a
|
|
# duplicate "No issues found". Key off the actual PR state (an existing Open
|
|
# SWE review summary) instead. Still resolve threads for findings that just
|
|
# moved to resolved, and advance last_reviewed_sha so subsequent pushes
|
|
# don't redo the same diff.
|
|
if not inline_comments and await _open_swe_already_reviewed(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
is_re_review=is_re_review,
|
|
):
|
|
resolved_thread_count = await _resolve_threads_for_resolved_findings(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
findings=findings,
|
|
)
|
|
await set_reviewer_thread_metadata(thread_id, last_reviewed_sha=head_sha)
|
|
await clear_review_started_comment(thread_id=thread_id, owner=owner, repo=repo, token=token)
|
|
conclusion, check_title, check_summary = review_check_conclusion(0)
|
|
await settle_review_check_run(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
token=token,
|
|
conclusion=conclusion,
|
|
title=check_title,
|
|
summary=check_summary,
|
|
)
|
|
return {
|
|
"success": True,
|
|
"review_id": None,
|
|
"surfaced_count": 0,
|
|
"hidden_count": max(len(open_unpublished), 0),
|
|
"resolved_thread_count": resolved_thread_count,
|
|
"skipped_empty_re_review": True,
|
|
}
|
|
|
|
review_body = render_review_body(
|
|
pr_number=pr_number,
|
|
surfaced_count=len(inline_comments),
|
|
trace_url=review_trace_url,
|
|
ui_url=review_ui_url,
|
|
additional_findings_count=additional_findings_count,
|
|
)
|
|
|
|
review_response = await post_pull_request_review(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
head_sha=head_sha,
|
|
body=review_body,
|
|
inline_comments=inline_comments,
|
|
token=token,
|
|
)
|
|
# If GitHub rejected the batch because one or more inline comments anchor
|
|
# to a file/line that's not in the PR diff, drop just those findings and
|
|
# retry once. Returning the bare 422 to the agent only invites it to
|
|
# retry publish_review with byte-identical args until findings drain.
|
|
unresolvable_findings: list[str] = []
|
|
if (
|
|
isinstance(review_response, dict)
|
|
and review_response.get("_error_kind") == "unresolved_anchor"
|
|
):
|
|
valid_with_payload, dropped_ids = await _filter_against_pr_diff(
|
|
eligible_with_payload,
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
)
|
|
if dropped_ids and valid_with_payload:
|
|
retry_inline = [p for _, p in valid_with_payload]
|
|
retry_body = render_review_body(
|
|
pr_number=pr_number,
|
|
surfaced_count=len(retry_inline),
|
|
trace_url=review_trace_url,
|
|
ui_url=review_ui_url,
|
|
additional_findings_count=additional_findings_count,
|
|
)
|
|
retry_response = await post_pull_request_review(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
head_sha=head_sha,
|
|
body=retry_body,
|
|
inline_comments=retry_inline,
|
|
token=token,
|
|
)
|
|
if isinstance(retry_response, dict) and "_error" not in retry_response:
|
|
review_response = retry_response
|
|
inline_comments = retry_inline
|
|
eligible_with_payload = valid_with_payload
|
|
unresolvable_findings = dropped_ids
|
|
else:
|
|
retry_error = (
|
|
retry_response.get("_error", "unknown error")
|
|
if isinstance(retry_response, dict)
|
|
else "no response"
|
|
)
|
|
return {
|
|
"success": False,
|
|
"error": f"Failed to POST PR review: {retry_error}",
|
|
"unresolvable_findings": dropped_ids,
|
|
"hint": (
|
|
"Call update_finding(status='resolved') on these ids "
|
|
"or fix their file/line before retrying."
|
|
),
|
|
}
|
|
else:
|
|
# Either nothing to drop (no diff_line_set available, so we can't
|
|
# tell which findings are bad) or everything would be dropped.
|
|
# Either way, do not retry — surface the structural signal so the
|
|
# agent stops retrying with the same args.
|
|
return {
|
|
"success": False,
|
|
"error": f"Failed to POST PR review: {review_response['_error']}",
|
|
"unresolvable_findings": dropped_ids,
|
|
"hint": (
|
|
"Call update_finding(status='resolved') on these ids "
|
|
"or fix their file/line before retrying."
|
|
),
|
|
}
|
|
if isinstance(review_response, dict) and "_error" in review_response:
|
|
return {
|
|
"success": False,
|
|
"error": f"Failed to POST PR review: {review_response['_error']}",
|
|
}
|
|
if review_response is None:
|
|
# Defensive guard: with the upstream change this should never happen,
|
|
# but keep a clear signal if it does so the agent doesn't retry blindly.
|
|
return {
|
|
"success": False,
|
|
"error": "Failed to POST PR review: no response from GitHub",
|
|
}
|
|
review_id = review_response.get("id") if isinstance(review_response, dict) else None
|
|
|
|
if review_id is not None and inline_comments:
|
|
# Record the GitHub review id AND inline comment ids in a single
|
|
# findings write. Previously these were three separate read-replace
|
|
# cycles (out-of-diff review id, inline review id, comment ids); each
|
|
# extra write widened the window where a crash could leave findings
|
|
# half-stamped — surfaced on GitHub but with no recorded comment id, so
|
|
# a later resolve-on-fix couldn't find the thread.
|
|
comment_records: list[dict[str, Any]] = []
|
|
if inline_comments:
|
|
comment_records = await fetch_review_comments(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
review_id=review_id,
|
|
token=token,
|
|
)
|
|
if langgraph_run_id is None:
|
|
metadata = await get_thread_metadata(thread_id)
|
|
current_run_id = metadata.get("current_reviewer_run_id")
|
|
if isinstance(current_run_id, str) and current_run_id:
|
|
langgraph_run_id = current_run_id
|
|
await _record_review_publication(
|
|
thread_id=thread_id,
|
|
review_id=review_id,
|
|
inline_with_payload=eligible_with_payload,
|
|
comment_records=comment_records,
|
|
langgraph_run_id=langgraph_run_id,
|
|
)
|
|
|
|
if review_id is not None and inline_comments:
|
|
current_findings = await list_findings_async(thread_id)
|
|
if _missing_comment_ids_for_published_findings(current_findings, eligible_with_payload):
|
|
await _backfill_findings_from_pr_threads(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
)
|
|
await _store_thread_ids_on_findings(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
)
|
|
|
|
resolved_thread_count = await _resolve_threads_for_resolved_findings(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
findings=await list_findings_async(thread_id),
|
|
)
|
|
|
|
if not is_re_review:
|
|
await _maybe_post_slack_completion_reply(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
review_id=review_id,
|
|
surfaced_count=len(inline_comments),
|
|
)
|
|
|
|
await set_reviewer_thread_metadata(thread_id, last_reviewed_sha=head_sha)
|
|
await clear_review_started_comment(thread_id=thread_id, owner=owner, repo=repo, token=token)
|
|
conclusion, check_title, check_summary = review_check_conclusion(len(inline_comments))
|
|
await settle_review_check_run(
|
|
thread_id=thread_id,
|
|
owner=owner,
|
|
repo=repo,
|
|
token=token,
|
|
conclusion=conclusion,
|
|
title=check_title,
|
|
summary=check_summary,
|
|
)
|
|
|
|
result: dict[str, Any] = {
|
|
"success": True,
|
|
"review_id": review_id,
|
|
"surfaced_count": len(inline_comments),
|
|
"hidden_count": max(len(open_unpublished) - len(inline_comments), 0),
|
|
"resolved_thread_count": resolved_thread_count,
|
|
}
|
|
if unresolvable_findings:
|
|
result["unresolvable_findings"] = unresolvable_findings
|
|
result["hint"] = (
|
|
"Some findings had anchors not in the PR diff; "
|
|
"call update_finding to fix or resolve them."
|
|
)
|
|
return result
|
|
|
|
|
|
async def _open_swe_already_reviewed(
|
|
*,
|
|
thread_id: str,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
token: str,
|
|
is_re_review: bool,
|
|
) -> bool:
|
|
"""Decide whether to suppress a duplicate empty "no issues found" summary.
|
|
|
|
Suppress only when we are *certain* a prior Open SWE review exists, so a
|
|
transient GitHub failure never causes a double-post:
|
|
|
|
- ``is_re_review`` is a durable signal (the dispatching webhook set it from
|
|
the persisted ``last_reviewed_sha``), so trust it outright.
|
|
- Otherwise consult durable reviewer state (``last_reviewed_sha`` on thread
|
|
metadata): a non-empty value means this thread already published once.
|
|
- Only as a last resort hit the GitHub reviews API. That call is tri-state:
|
|
``True``/``False`` are authoritative, but ``None`` means "unknown"
|
|
(pagination or the request failed). On ``None`` we do NOT suppress — a
|
|
possible duplicate summary is better than silently swallowing the only
|
|
review the user will ever see, and re-posting is the safe failure mode.
|
|
"""
|
|
if is_re_review:
|
|
return True
|
|
metadata = await get_thread_metadata(thread_id)
|
|
if get_thread_last_reviewed_sha(metadata):
|
|
return True
|
|
exists = await open_swe_review_exists(owner=owner, repo=repo, pr_number=pr_number, token=token)
|
|
return exists is True
|
|
|
|
|
|
def _has_publication_identity(finding: Finding) -> bool:
|
|
return isinstance(finding.get("github_review_comment_id"), int) or isinstance(
|
|
finding.get("github_review_id"), int
|
|
)
|
|
|
|
|
|
def _int_list(value: Any) -> list[int]:
|
|
if not isinstance(value, list):
|
|
return []
|
|
return [item for item in value if isinstance(item, int)]
|
|
|
|
|
|
def _str_list(value: Any) -> list[str]:
|
|
if not isinstance(value, list):
|
|
return []
|
|
return [item for item in value if isinstance(item, str) and item]
|
|
|
|
|
|
def _comment_ids_for_finding(finding: dict[str, Any]) -> list[int]:
|
|
comment_ids = _int_list(finding.get("github_review_comment_ids"))
|
|
comment_id = finding.get("github_review_comment_id")
|
|
if isinstance(comment_id, int) and comment_id not in comment_ids:
|
|
comment_ids.insert(0, comment_id)
|
|
return comment_ids
|
|
|
|
|
|
def _thread_ids_for_finding(finding: dict[str, Any]) -> list[str]:
|
|
thread_ids = _str_list(finding.get("github_review_thread_ids"))
|
|
thread_id = finding.get("github_review_thread_id")
|
|
if isinstance(thread_id, str) and thread_id and thread_id not in thread_ids:
|
|
thread_ids.insert(0, thread_id)
|
|
return thread_ids
|
|
|
|
|
|
async def _backfill_findings_from_pr_threads(
|
|
*,
|
|
thread_id: str,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
token: str,
|
|
) -> list[Finding]:
|
|
findings = await list_findings_async(thread_id)
|
|
if not findings:
|
|
return findings
|
|
review_threads = await fetch_pr_review_threads(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
)
|
|
if not review_threads:
|
|
return findings
|
|
return await reconcile_findings_with_review_threads(thread_id, review_threads)
|
|
|
|
|
|
def _missing_comment_ids_for_published_findings(
|
|
findings: list[Finding],
|
|
eligible_with_payload: list[tuple[dict[str, Any], dict[str, Any]]],
|
|
) -> bool:
|
|
finding_ids = {
|
|
finding.get("id")
|
|
for finding, _payload in eligible_with_payload
|
|
if isinstance(finding.get("id"), str)
|
|
}
|
|
for finding in findings:
|
|
if finding.get("id") in finding_ids and not isinstance(
|
|
finding.get("github_review_comment_id"), int
|
|
):
|
|
return True
|
|
return False
|
|
|
|
|
|
def _apply_review_id(
|
|
findings: list[Finding],
|
|
*,
|
|
finding_ids: set[str],
|
|
review_id: int,
|
|
) -> bool:
|
|
updated = False
|
|
for finding in findings:
|
|
if finding.get("id") in finding_ids and finding.get("github_review_id") != review_id:
|
|
finding["github_review_id"] = review_id
|
|
if isinstance(finding.get("id"), str):
|
|
surface = _coerce_surface(finding, str(finding["id"]))
|
|
surface["github_review_id"] = review_id
|
|
finding["surface"] = surface
|
|
updated = True
|
|
return updated
|
|
|
|
|
|
def _apply_comment_ids(
|
|
findings: list[Finding],
|
|
*,
|
|
comment_id_by_finding_id: dict[str, int],
|
|
langgraph_run_id: str | None,
|
|
) -> bool:
|
|
updated = False
|
|
for finding in findings:
|
|
finding_id = finding.get("id")
|
|
if not isinstance(finding_id, str):
|
|
continue
|
|
comment_id = comment_id_by_finding_id.get(finding_id)
|
|
if comment_id is None:
|
|
continue
|
|
finding["github_review_comment_id"] = comment_id
|
|
comment_ids = _int_list(finding.get("github_review_comment_ids"))
|
|
if comment_id not in comment_ids:
|
|
comment_ids.append(comment_id)
|
|
finding["github_review_comment_ids"] = comment_ids
|
|
surface = _coerce_surface(finding, finding_id)
|
|
surface["state"] = "surfaced"
|
|
surface["github_review_comment_id"] = comment_id
|
|
surface["severity_threshold_at_publish"] = finding.get("severity")
|
|
surface["surfaced_at_sha"] = finding.get("last_confirmed_sha") or finding.get(
|
|
"first_seen_sha"
|
|
)
|
|
finding["surface"] = surface
|
|
if langgraph_run_id:
|
|
finding["github_review_run_id"] = langgraph_run_id
|
|
updated = True
|
|
return updated
|
|
|
|
|
|
def _comment_id_by_finding_id(
|
|
eligible_with_payload: list[tuple[dict[str, Any], dict[str, Any]]],
|
|
comment_records: list[dict[str, Any]],
|
|
) -> dict[str, int]:
|
|
"""Map each surfaced finding id to its GitHub comment id via the marker.
|
|
|
|
The embedded Open SWE marker is the *only* source of truth. Every comment
|
|
this reviewer posts carries a ``<!-- open-swe-review-comment {...} -->``
|
|
marker keyed by finding id (see ``render_inline_comment_body``), so the
|
|
match is exact. The old ``(path, line, body)`` fallback collided whenever
|
|
two findings shared a path/line/body — it cached the same comment id on
|
|
both, which corrupts resolve-on-fix (resolving one would target the wrong
|
|
thread). Findings whose comment lacks a parseable marker are left out here;
|
|
``_backfill_findings_from_pr_threads`` recovers them via the same marker
|
|
against the PR's review threads.
|
|
"""
|
|
by_marker_id: dict[str, int] = {}
|
|
for record in comment_records:
|
|
body = record.get("body", "")
|
|
comment_id = record.get("id")
|
|
if isinstance(body, str) and isinstance(comment_id, int):
|
|
marker = parse_review_comment_marker(body)
|
|
if marker is not None:
|
|
by_marker_id[marker["id"]] = comment_id
|
|
|
|
out: dict[str, int] = {}
|
|
for finding_snapshot, _payload in eligible_with_payload:
|
|
finding_id = finding_snapshot.get("id")
|
|
if isinstance(finding_id, str) and finding_id in by_marker_id:
|
|
out[finding_id] = by_marker_id[finding_id]
|
|
return out
|
|
|
|
|
|
async def _record_review_publication(
|
|
*,
|
|
thread_id: str,
|
|
review_id: int,
|
|
inline_with_payload: list[tuple[dict[str, Any], dict[str, Any]]],
|
|
comment_records: list[dict[str, Any]],
|
|
langgraph_run_id: str | None,
|
|
) -> None:
|
|
"""Stamp the review id and inline comment ids onto findings in one write.
|
|
|
|
Collapsing the review-id and comment-id updates into a single
|
|
read-modify-write keeps publication identity atomic: a finding is never
|
|
persisted carrying a review id without also carrying whatever comment id
|
|
GitHub returned for it in the same record.
|
|
"""
|
|
review_finding_ids = {
|
|
finding.get("id")
|
|
for finding, _payload in inline_with_payload
|
|
if isinstance(finding.get("id"), str)
|
|
}
|
|
comment_id_by_finding_id = _comment_id_by_finding_id(inline_with_payload, comment_records)
|
|
|
|
# Re-read the freshest persisted list right before mutating so this single
|
|
# write merges onto any update that landed since the snapshot the caller
|
|
# passed in, instead of blindly overwriting it.
|
|
latest = await list_findings_async(thread_id)
|
|
changed = _apply_review_id(
|
|
latest,
|
|
finding_ids={fid for fid in review_finding_ids if isinstance(fid, str)},
|
|
review_id=review_id,
|
|
)
|
|
changed = (
|
|
_apply_comment_ids(
|
|
latest,
|
|
comment_id_by_finding_id=comment_id_by_finding_id,
|
|
langgraph_run_id=langgraph_run_id,
|
|
)
|
|
or changed
|
|
)
|
|
if changed:
|
|
await replace_findings(thread_id, latest)
|
|
|
|
|
|
async def _resolve_diff_line_set(
|
|
*,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
token: str,
|
|
) -> dict[str, set[int]] | None:
|
|
"""Return the new-side line set for the PR diff, fetching it if needed.
|
|
|
|
Reviewer runs clear ``configurable['diff_line_set']`` before the agent
|
|
starts (so ``add_finding`` trusts the agent's anchors), which means the
|
|
publish-time retry path can't rely on it being populated. Fetch the PR's
|
|
unified diff from the GitHub REST API and recompute the line set on the
|
|
fly. Returns ``None`` if the fetch fails — caller treats that as "we
|
|
can't tell which finding is bad, don't retry blindly".
|
|
"""
|
|
config = get_config()
|
|
configurable = config.get("configurable", {}) if isinstance(config, dict) else {}
|
|
cached = configurable.get("diff_line_set") if isinstance(configurable, dict) else None
|
|
if isinstance(cached, dict):
|
|
return cached
|
|
|
|
diff_text = await fetch_pr_diff(owner=owner, repo=repo, pr_number=pr_number, token=token)
|
|
if diff_text is None:
|
|
return None
|
|
return compute_diff_line_set(diff_text)
|
|
|
|
|
|
async def _filter_against_pr_diff(
|
|
eligible_with_payload: list[tuple[dict[str, Any], dict[str, Any]]],
|
|
*,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
token: str,
|
|
) -> tuple[list[tuple[dict[str, Any], dict[str, Any]]], list[str]]:
|
|
"""Drop findings whose path/line range is not in the current PR diff.
|
|
|
|
Returns ``(valid_with_payload, dropped_finding_ids)``. When the diff
|
|
cannot be resolved (fetch failed and no cached set), we return everything
|
|
unchanged and an empty drop list — the caller will then surface the
|
|
original error rather than retry blindly.
|
|
"""
|
|
diff_line_set = await _resolve_diff_line_set(
|
|
owner=owner, repo=repo, pr_number=pr_number, token=token
|
|
)
|
|
if diff_line_set is None:
|
|
return list(eligible_with_payload), []
|
|
|
|
valid: list[tuple[dict[str, Any], dict[str, Any]]] = []
|
|
dropped: list[str] = []
|
|
for finding, payload in eligible_with_payload:
|
|
path = payload.get("path")
|
|
# Prefer the finding's recorded range; fall back to the payload line.
|
|
start_line = finding.get("start_line")
|
|
end_line = finding.get("end_line")
|
|
if end_line is None:
|
|
payload_line = payload.get("line")
|
|
if isinstance(payload_line, int):
|
|
end_line = payload_line
|
|
if start_line is None:
|
|
start_line = payload_line
|
|
side = finding.get("side") if finding.get("side") in {"LEFT", "RIGHT"} else "RIGHT"
|
|
if isinstance(path, str) and is_range_in_diff(
|
|
diff_line_set, path, start_line, end_line, side=side
|
|
):
|
|
valid.append((finding, payload))
|
|
else:
|
|
finding_id = finding.get("id")
|
|
if isinstance(finding_id, str):
|
|
dropped.append(finding_id)
|
|
return valid, dropped
|
|
|
|
|
|
async def _maybe_post_slack_completion_reply(
|
|
*,
|
|
thread_id: str,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
review_id: int | None,
|
|
surfaced_count: int,
|
|
) -> None:
|
|
"""Post a one-line completion summary to the Slack thread that started this review.
|
|
|
|
Only fires for first reviews (gated by the caller). No-op if the reviewer
|
|
thread has no ``slack_thread`` metadata — i.e. the review wasn't started
|
|
from Slack.
|
|
"""
|
|
metadata = await get_thread_metadata(thread_id)
|
|
slack_ref = get_thread_slack_ref(metadata)
|
|
if slack_ref is None:
|
|
return
|
|
|
|
if surfaced_count == 0:
|
|
headline = "*Open SWE Review*: No issues found."
|
|
else:
|
|
issue_word = "issue" if surfaced_count == 1 else "issues"
|
|
headline = f"*Open SWE Review* found {surfaced_count} potential {issue_word}."
|
|
|
|
review_url = f"https://github.com/{owner}/{repo}/pull/{pr_number}"
|
|
if isinstance(review_id, int):
|
|
review_url = f"{review_url}#pullrequestreview-{review_id}"
|
|
text = f"{headline} <{review_url}|View review>"
|
|
|
|
await post_slack_thread_reply(slack_ref["channel_id"], slack_ref["thread_ts"], text)
|
|
|
|
|
|
async def _store_thread_ids_on_findings(
|
|
*,
|
|
thread_id: str,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
token: str,
|
|
) -> None:
|
|
findings = await list_findings_async(thread_id)
|
|
comment_ids_by_finding_id: dict[str, list[int]] = {}
|
|
for finding in findings:
|
|
finding_id = finding.get("id")
|
|
comment_ids = _comment_ids_for_finding(finding)
|
|
if isinstance(finding_id, str) and comment_ids and not _thread_ids_for_finding(finding):
|
|
comment_ids_by_finding_id[finding_id] = comment_ids
|
|
if not comment_ids_by_finding_id:
|
|
return
|
|
|
|
threads = await fetch_pr_review_threads(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
token=token,
|
|
)
|
|
thread_id_by_comment_id: dict[int, str] = {}
|
|
for thread in threads:
|
|
github_thread_id = thread.get("id")
|
|
if not isinstance(github_thread_id, str) or not github_thread_id:
|
|
continue
|
|
for comment in thread.get("comments") or []:
|
|
if not isinstance(comment, dict):
|
|
continue
|
|
comment_id = comment.get("id")
|
|
if isinstance(comment_id, int):
|
|
thread_id_by_comment_id[comment_id] = github_thread_id
|
|
|
|
updated = False
|
|
for finding in findings:
|
|
finding_id = finding.get("id")
|
|
if not isinstance(finding_id, str):
|
|
continue
|
|
thread_ids = _thread_ids_for_finding(finding)
|
|
for comment_id in comment_ids_by_finding_id.get(finding_id, []):
|
|
github_thread_id = thread_id_by_comment_id.get(comment_id)
|
|
if not github_thread_id:
|
|
continue
|
|
if not isinstance(finding.get("github_review_thread_id"), str):
|
|
finding["github_review_thread_id"] = github_thread_id
|
|
updated = True
|
|
if github_thread_id not in thread_ids:
|
|
thread_ids.append(github_thread_id)
|
|
finding["github_review_thread_ids"] = thread_ids
|
|
updated = True
|
|
surface = _coerce_surface(finding, finding_id)
|
|
surface["state"] = "surfaced"
|
|
surface["github_review_thread_id"] = github_thread_id
|
|
finding["surface"] = surface
|
|
updated = True
|
|
|
|
if updated:
|
|
await replace_findings(thread_id, findings)
|
|
|
|
|
|
async def _resolve_threads_for_resolved_findings(
|
|
*,
|
|
owner: str,
|
|
repo: str,
|
|
pr_number: int,
|
|
token: str,
|
|
findings: list[dict[str, Any]],
|
|
) -> int:
|
|
"""Resolve GitHub review threads for findings that just transitioned to resolved.
|
|
|
|
Posts a resolution comment to the thread, then resolves it. Multiple threads
|
|
can exist when an earlier run duplicated a comment before publication identity
|
|
was backfilled.
|
|
"""
|
|
resolved_count = 0
|
|
mutated = False
|
|
for finding in findings:
|
|
status = finding.get("status")
|
|
if status not in {"resolved", "dismissed"}:
|
|
continue
|
|
|
|
thread_node_ids = _thread_ids_for_finding(finding)
|
|
comment_ids = _comment_ids_for_finding(finding)
|
|
|
|
for comment_id in comment_ids:
|
|
thread_node_id = await fetch_review_thread_id_for_comment(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
review_comment_id=comment_id,
|
|
token=token,
|
|
)
|
|
if thread_node_id and thread_node_id not in thread_node_ids:
|
|
thread_node_ids.append(thread_node_id)
|
|
|
|
if not thread_node_ids:
|
|
continue
|
|
|
|
resolved_thread_ids = _str_list(finding.get("github_resolved_thread_ids"))
|
|
posted_resolution_comment_ids = _int_list(
|
|
finding.get("github_posted_resolution_comment_ids")
|
|
)
|
|
|
|
for idx, thread_node_id in enumerate(thread_node_ids):
|
|
if thread_node_id in resolved_thread_ids:
|
|
continue
|
|
|
|
primary_comment_id = comment_ids[idx] if idx < len(comment_ids) else None
|
|
resolution_body = render_resolution_comment(finding, status)
|
|
if (
|
|
primary_comment_id
|
|
and primary_comment_id not in posted_resolution_comment_ids
|
|
and resolution_body is not None
|
|
):
|
|
reply_response = await reply_to_review_comment(
|
|
owner=owner,
|
|
repo=repo,
|
|
pr_number=pr_number,
|
|
review_comment_id=primary_comment_id,
|
|
body=resolution_body,
|
|
token=token,
|
|
)
|
|
if reply_response and isinstance(reply_response.get("id"), int):
|
|
posted_resolution_comment_ids.append(primary_comment_id)
|
|
mutated = True
|
|
|
|
ok = await resolve_review_thread(thread_node_id=thread_node_id, token=token)
|
|
if ok:
|
|
resolved_thread_ids.append(thread_node_id)
|
|
resolved_count += 1
|
|
mutated = True
|
|
|
|
if resolved_thread_ids:
|
|
finding["github_resolved_thread_ids"] = resolved_thread_ids
|
|
if posted_resolution_comment_ids:
|
|
finding["github_posted_resolution_comment_ids"] = posted_resolution_comment_ids
|
|
if thread_node_ids:
|
|
finding["github_review_thread_ids"] = thread_node_ids
|
|
if not isinstance(finding.get("github_review_thread_id"), str):
|
|
finding["github_review_thread_id"] = thread_node_ids[0]
|
|
if thread_node_ids and all(
|
|
thread_id in resolved_thread_ids for thread_id in thread_node_ids
|
|
):
|
|
finding["github_thread_resolved"] = True
|
|
if isinstance(finding.get("id"), str):
|
|
surface = _coerce_surface(finding, str(finding["id"]))
|
|
surface["state"] = "resolved"
|
|
if thread_node_ids:
|
|
surface["github_review_thread_id"] = thread_node_ids[0]
|
|
finding["surface"] = surface
|
|
|
|
if mutated:
|
|
thread_id = get_thread_id_from_runtime()
|
|
await replace_findings(thread_id, findings)
|
|
|
|
return resolved_count
|
|
|
|
|
|
def _current_run_id(config: dict[str, Any]) -> str | None:
|
|
candidates = [config.get("run_id")]
|
|
configurable = config.get("configurable")
|
|
if isinstance(configurable, dict):
|
|
candidates.append(configurable.get("run_id"))
|
|
for candidate in candidates:
|
|
if isinstance(candidate, str) and candidate:
|
|
return candidate
|
|
return None
|