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>

agent/reviewer.py

@@ -138,22 +138,23 @@ Out-of-diff findings are disabled. `add_finding` rejects any finding whose
 line this PR actually changed.
 
 Re-review: for each open finding, `update_finding(id, status="resolved", note="...")`
-if fixed (include a brief explanation of the fix in `note`), `update_finding` with
+if fixed (write the full GitHub reply body in `note`), `update_finding` with
 new fields + `note` if changed, otherwise do nothing. Add net-new findings with
 `add_finding`.
 
-When you mark a finding as resolved, `publish_review` will automatically post a
-resolution comment to the GitHub thread explaining what was fixed, then close it.
-The `note` field you provide in `update_finding` becomes part of that comment, so
-be specific: "The current code at line X now does Y" beats "This is fixed".
+When you mark a finding as resolved, `publish_review` will automatically post the
+`note` field verbatim to the GitHub thread, then close it. Write the complete
+human-facing reply yourself, including any desired status wording; the system does
+not prepend "Resolved" or "Dismissed".
 
 If a human reply shows one of your published findings is invalid, call
 `resolve_finding_thread(finding_id, status="dismissed", note="...")` after verifying
 the claim (the note should explain why). If the finding is fixed by code, use
-`update_finding(..., status="resolved", note="...")`. Do NOT use
-`reply_to_finding_thread` for resolutions or dismissals — the system posts those
-automatically. Use `reply_to_finding_thread` only when the user directly asks a
-question or a short clarification is needed after pushback.
+`update_finding(..., status="resolved", note="...")`. The note is posted verbatim
+as the complete GitHub reply body; include any desired status wording yourself.
+Do NOT use `reply_to_finding_thread` for resolutions or dismissals — the system
+posts those automatically. Use `reply_to_finding_thread` only when the user
+directly asks a question or a short clarification is needed after pushback.
 
 # The bar: file a finding only if it passes these criteria
 
@@ -580,11 +581,12 @@ def _build_re_review_context(
         f'{last_reviewed_sha}...{head_sha} -H "Accept: application/vnd.github.v3.diff"`, '
         f"then review only what's in that diff.\n\n"
         f"For each open finding above, decide whether the new commits resolved "
-        f'it (`update_finding(id, status="resolved", note="...")`), left it unchanged '
+        f'it (`update_finding(id, status="resolved", note="<full reply body>")`), left it unchanged '
         f"(no action), or changed it materially (`update_finding` with new "
-        f"fields + a `note`). If a human reply on a finding explains why your "
+        f"fields + a full reply-body `note`). If a human reply on a finding explains why your "
         f"comment was invalid, verify that analysis, then call "
         f'`resolve_finding_thread(id, status="dismissed", note="...")` to close it. '
+        f"The `note` is posted verbatim, so write it as the complete GitHub reply body. "
         f"Reply only when directly asked or when a concise clarification is "
         f"necessary. Then add any net-new findings introduced by the "
         f"new diff — but skip anything already covered by an existing PR "
@@ -636,8 +638,9 @@ def _build_finding_reply_context(
         f"## Existing findings\n\n{existing_findings_block}\n\n"
         f"{prior_threads_section}"
         f"Reassess only this finding. If the reply proves the finding is invalid, "
-        f'call `resolve_finding_thread(id, status="dismissed", note="...")`. If code now '
-        f'fixes the finding, call `update_finding(id, status="resolved", note="...")`. '
+        f'call `resolve_finding_thread(id, status="dismissed", note="<full reply body>")`. If code now '
+        f'fixes the finding, call `update_finding(id, status="resolved", note="<full reply body>")`. '
+        f"The `note` is posted verbatim, so write it as the complete GitHub reply body. "
         f"Use `reply_to_finding_thread` only when the user asked a direct "
         f"question or a concise clarification is necessary. Call `publish_review` "
         f"once at the end so pending GitHub thread state is reconciled."

agent/reviewer_publish.py

@@ -197,13 +197,8 @@ def render_resolution_comment(
     status: str,
     note: str | None = None,
 ) -> str | None:
-    """Render the agent-provided resolution reply for a review thread."""
-    body = _resolution_body(finding, note)
-    if body is None:
-        return None
-    if status == "resolved":
-        return f"✅ **Resolved**: {body}"
-    return f"❌ **Dismissed**: {body}"
+    """Render the agent-provided reply for a review thread."""
+    return _resolution_body(finding, note)
 
 
 def _resolution_body(finding: Finding, note: str | None) -> str | None:

agent/reviewer_reconcile.py

@@ -163,10 +163,6 @@ def _sync_thread_status(finding: Finding, matches: list[ReviewThreadMatch]) -> b
         return False
 
     updated = False
-    if finding.get("status") == "open":
-        finding["status"] = "resolved"
-        updated = True
-
     resolved_thread_ids = _str_list(finding.get("github_resolved_thread_ids"))
     all_resolved = True
     for review_thread, _comment_id in matches:
@@ -180,12 +176,18 @@ def _sync_thread_status(finding: Finding, matches: list[ReviewThreadMatch]) -> b
 
     if resolved_thread_ids != _str_list(finding.get("github_resolved_thread_ids")):
         finding["github_resolved_thread_ids"] = resolved_thread_ids
-    if all_resolved and not finding.get("github_thread_resolved"):
+    if not all_resolved:
+        return updated
+
+    if finding.get("status") == "open":
+        finding["status"] = "resolved"
+        updated = True
+    if not finding.get("github_thread_resolved"):
         finding["github_thread_resolved"] = True
         updated = True
     if isinstance(finding.get("id"), str):
         surface = _coerce_surface(finding, str(finding["id"]))
-        surface["state"] = "resolved" if all_resolved else "resolve_pending"
+        surface["state"] = "resolved"
         finding["surface"] = surface
         updated = True
     return updated

agent/tools/resolve_finding_thread.py

@@ -41,7 +41,7 @@ async def resolve_finding_thread(
 
     Use ``status="resolved"`` when the code now fixes the issue. Use
     ``status="dismissed"`` when analysis shows the original review comment was
-    not valid. ``note`` is required and becomes the GitHub reply body.
+    not valid. ``note`` is required and is posted verbatim as the full GitHub reply body.
     """
     if status not in {"resolved", "dismissed"}:
         return {"success": False, "error": f"Invalid status: {status}"}

agent/tools/update_finding.py

@@ -71,7 +71,7 @@ async def update_finding(
             ``Existing findings`` block of the re-review user message).
         status: New status (``open``, ``resolved``, ``dismissed``).
             Use ``resolved`` when the new commits address the issue. Resolving
-            or dismissing requires a ``note`` with the message to post.
+            or dismissing requires a ``note`` with the full message to post.
         severity: New severity, if reassessing.
         confidence: New confidence rating (``low``, ``medium``, ``high``), if
             new commits change how sure you are the finding is a real issue.
@@ -82,7 +82,8 @@ async def update_finding(
             Capped at 4 lines — longer values are dropped (the finding keeps
             its description). Only set this for small, obvious fixes.
         note: Optional free-form note explaining the change. Required when
-            resolving or dismissing because it becomes the GitHub reply body.
+            resolving or dismissing because it is posted verbatim as the full
+            GitHub reply body.
 
     Returns:
         Dictionary with ``success`` and (on success) the updated ``finding``.

tests/e2e/fake_llm.py

@@ -226,12 +226,22 @@ def build_plan_script() -> list[Any]:
 
 
 FOLLOW_UP_REPLY = "Thanks! The PR is ready for review — anything else you'd like changed?"
+_ATTRIBUTION_RE = re.compile(r"@([A-Za-z0-9-]+):")
 
 
-def _step_followup(_messages: list[BaseMessage]) -> AIMessage:
-    # A web/Slack follow-up after the PR exists: a plain reply, no new PR. Its
-    # content lands in the thread transcript the dashboard renders.
-    return AIMessage(content=FOLLOW_UP_REPLY)
+def _latest_attribution(messages: list[BaseMessage]) -> str | None:
+    for msg in reversed(messages):
+        if isinstance(msg, HumanMessage):
+            match = _ATTRIBUTION_RE.search(_text(msg.content))
+            if match:
+                return f"@{match.group(1)}"
+    return None
+
+
+def _step_followup(messages: list[BaseMessage]) -> AIMessage:
+    attribution = _latest_attribution(messages)
+    suffix = f" I saw this follow-up was from {attribution}." if attribution else ""
+    return AIMessage(content=f"{FOLLOW_UP_REPLY}{suffix}")
 
 
 def build_script() -> list[Any]:
@@ -285,7 +295,7 @@ class FakeScriptedChatModel(BaseChatModel):
         if step < len(script):
             message = script[step](messages)
         else:
-            message = AIMessage(content="All set — let me know if you'd like anything else.")
+            message = _step_followup(messages)
         return ChatResult(generations=[ChatGeneration(message=message)])
 
 

tests/e2e/tests/dashboard.spec.ts

@@ -73,12 +73,7 @@ test.describe("Slack → web handoff (real dashboard UI)", () => {
     await expect(page.getByText(/anything else you'd like changed/)).toBeVisible();
 
     // The non-owner's message is tagged server-side with their GitHub login, so
-    // the owner can tell who sent it. Visible once the transcript re-hydrates.
-    await expect(async () => {
-      await page.reload();
-      await expect(
-        page.getByText(new RegExp(`@${OTHER_USER.login}`)).first(),
-      ).toBeVisible({ timeout: 8000 });
-    }).toPass({ timeout: 60000 });
+    // the owner can tell who sent it.
+    await expect(page.getByText(new RegExp(`@${OTHER_USER.login}`)).first()).toBeVisible();
   });
 });

tests/test_reviewer_publish.py

@@ -123,9 +123,9 @@ def test_render_inline_comment_body_line_reference_range() -> None:
     assert "*(Refers to line 10)*" in render_inline_comment_body(_f(start_line=10, end_line=10))
 
 
-def test_render_resolution_comment_resolved_uses_note() -> None:
+def test_render_resolution_comment_resolved_uses_note_verbatim() -> None:
     body = render_resolution_comment(_f(status="resolved"), "resolved", note="Fixed at line 5")
-    assert body == "✅ **Resolved**: Fixed at line 5"
+    assert body == "Fixed at line 5"
 
 
 def test_render_resolution_comment_returns_none_without_agent_note() -> None:
@@ -133,15 +133,15 @@ def test_render_resolution_comment_returns_none_without_agent_note() -> None:
     assert body is None
 
 
-def test_render_resolution_comment_dismissed_uses_note() -> None:
+def test_render_resolution_comment_dismissed_uses_note_verbatim() -> None:
     body = render_resolution_comment(_f(status="dismissed"), "dismissed", note="Intended behavior")
-    assert body == "❌ **Dismissed**: Intended behavior"
+    assert body == "Intended behavior"
 
 
-def test_render_resolution_comment_uses_stored_resolution_note() -> None:
+def test_render_resolution_comment_uses_stored_resolution_note_verbatim() -> None:
     finding = _f(status="resolved", resolution_note="The guard now returns before indexing.")
     body = render_resolution_comment(finding, "resolved")
-    assert body == "✅ **Resolved**: The guard now returns before indexing."
+    assert body == "The guard now returns before indexing."
 
 
 def test_parse_review_comment_marker_accepts_valid_marker() -> None:
@@ -1205,7 +1205,7 @@ async def test_re_review_backfills_and_resolves_duplicate_existing_threads() ->
     assert reply_comment.await_count == 2
     assert (
         reply_comment.await_args_list[0].kwargs["body"]
-        == "✅ **Resolved**: The duplicate threads are fixed by the latest commit."
+        == "The duplicate threads are fixed by the latest commit."
     )
     assert findings[0]["github_review_comment_ids"] == [101, 102]
     assert findings[0]["github_review_thread_ids"] == ["THREAD_1", "THREAD_2"]

tests/test_reviewer_reconcile.py

@@ -174,7 +174,7 @@ async def test_reconcile_duplicate_markers_require_all_threads_terminal() -> Non
 
 
 @pytest.mark.asyncio
-async def test_reconcile_duplicate_markers_resolve_when_all_threads_terminal() -> None:
+async def test_reconcile_duplicate_markers_stay_open_when_some_threads_only_outdated() -> None:
     findings = [
         {
             "id": "f1",
@@ -212,7 +212,7 @@ async def test_reconcile_duplicate_markers_resolve_when_all_threads_terminal() -
             ],
         )
 
-    assert result[0]["status"] == "resolved"
+    assert result[0]["status"] == "open"
     assert "last_reconciliation_note" not in result[0]
     assert result[0]["github_resolved_thread_ids"] == ["THREAD_RESOLVED"]
     assert result[0].get("github_thread_resolved") is not True

tests/test_reviewer_tools.py

@@ -334,8 +334,7 @@ async def test_resolve_finding_thread_resolves_all_known_threads() -> None:
     ]
     assert [call.kwargs["review_comment_id"] for call in reply.await_args_list] == [11, 12]
     assert all(
-        "✅ **Resolved**: Fixed in the latest commit" in call.kwargs["body"]
-        for call in reply.await_args_list
+        call.kwargs["body"] == "Fixed in the latest commit" for call in reply.await_args_list
     )
     updates = update.await_args.args[2]
     assert updates["github_thread_resolved"] is True