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>
This commit is contained in:
Johannes du Plessis 2026-06-26 11:43:38 -07:00 • committed by GitHub
parent f29868ffe7
commit 2f56d7546a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 66 additions and 2 deletions

View file

@ -11,6 +11,7 @@ from langgraph.config import get_config
from langgraph_sdk import get_client
from ..dashboard.agent_usage import record_agent_pr_usage
from ..dashboard.plan_store import get_plan_content
from ..utils.dashboard_links import dashboard_plan_url
from ..utils.github_app import get_github_app_installation_token
from ..utils.github_comments import derive_pr_state
@ -169,10 +170,17 @@ async def _record_pr_telemetry(
)
def _plan_reference_line(configurable: dict[str, Any]) -> str | None:
async def _plan_reference_line(configurable: dict[str, Any]) -> str | None:
thread_id = configurable.get("thread_id")
if not isinstance(thread_id, str):
return None
try:
plan = await get_plan_content(thread_id)
except Exception:
logger.debug("Failed to look up plan content for %s", thread_id, exc_info=True)
return None
if not plan or not str(plan.get("markdown", "")).strip():
return None
plan_url = dashboard_plan_url(thread_id)
if not plan_url:
return None
@ -224,7 +232,7 @@ async def _maybe_append_references(
if not isinstance(configurable, dict):
configurable = {}
lines: list[str] = []
plan_line = _plan_reference_line(configurable)
plan_line = await _plan_reference_line(configurable)
if plan_line:
lines.append(plan_line)
try:

View file

@ -257,6 +257,10 @@ def _stub_token(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setattr(opr, "_resolve_pr_author_token", lambda: _coro(("tok", "user")))
def _stub_plan(monkeypatch: pytest.MonkeyPatch, plan: dict[str, Any] | None) -> None:
monkeypatch.setattr(opr, "get_plan_content", lambda *_a, **_k: _coro(plan))
def test_appends_slack_reference_for_private_repo(monkeypatch: pytest.MonkeyPatch) -> None:
_set_config(
monkeypatch,
@ -288,6 +292,7 @@ def test_appends_plan_reference_from_thread_id(monkeypatch: pytest.MonkeyPatch)
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
_set_config(monkeypatch, {"source": "dashboard", "thread_id": "thread-1"})
_stub_token(monkeypatch)
_stub_plan(monkeypatch, {"markdown": "# Plan\n- step 1", "status": "ready"})
client = _FakeClient(post=_FakeResponse(201, {"html_url": "u", "number": 1, "user": {}}))
_install_client(monkeypatch, client)
@ -300,6 +305,55 @@ def test_appends_plan_reference_from_thread_id(monkeypatch: pytest.MonkeyPatch)
assert client.get_calls == []
def test_omits_plan_reference_when_no_plan_exists(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
_set_config(monkeypatch, {"source": "dashboard", "thread_id": "thread-1"})
_stub_token(monkeypatch)
_stub_plan(monkeypatch, None)
client = _FakeClient(post=_FakeResponse(201, {"html_url": "u", "number": 1, "user": {}}))
_install_client(monkeypatch, client)
_open_with_body("body")
assert client.post_calls[0]["json"]["body"] == "body"
assert client.get_calls == []
def test_omits_plan_reference_when_plan_markdown_empty(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
_set_config(monkeypatch, {"source": "dashboard", "thread_id": "thread-1"})
_stub_token(monkeypatch)
_stub_plan(monkeypatch, {"markdown": " \n ", "status": "ready"})
client = _FakeClient(post=_FakeResponse(201, {"html_url": "u", "number": 1, "user": {}}))
_install_client(monkeypatch, client)
_open_with_body("body")
assert client.post_calls[0]["json"]["body"] == "body"
assert client.get_calls == []
def test_omits_plan_reference_when_store_lookup_fails(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example")
_set_config(monkeypatch, {"source": "dashboard", "thread_id": "thread-1"})
_stub_token(monkeypatch)
async def fail_plan(*_a: Any, **_k: Any) -> Any:
raise RuntimeError("store down")
monkeypatch.setattr(opr, "get_plan_content", fail_plan)
client = _FakeClient(post=_FakeResponse(201, {"html_url": "u", "number": 1, "user": {}}))
_install_client(monkeypatch, client)
_open_with_body("body")
assert client.post_calls[0]["json"]["body"] == "body"
assert client.get_calls == []
def test_plan_reference_survives_source_reference_failure(
monkeypatch: pytest.MonkeyPatch,
) -> None:
@ -313,6 +367,7 @@ def test_plan_reference_survives_source_reference_failure(
},
)
_stub_token(monkeypatch)
_stub_plan(monkeypatch, {"markdown": "# Plan\n- step 1", "status": "ready"})
async def fail_permalink(*_args: Any, **_kwargs: Any) -> str:
raise RuntimeError("slack failed")
@ -365,6 +420,7 @@ def test_public_repo_appends_plan_but_not_source_reference(
},
)
_stub_token(monkeypatch)
_stub_plan(monkeypatch, {"markdown": "# Plan\n- step 1", "status": "ready"})
monkeypatch.setattr(
opr, "get_slack_permalink", lambda *_a, **_k: _coro("https://slack.example/p1")
)