fix(code): avoid duplicate "criteria ready" message on /goal revisions (#4559)

Fixed `/goal` showing a duplicate "Proposed acceptance criteria are
ready." message when revising proposed criteria.

---

When using `/goal` and going through revision cycles of the proposed
criteria, `_propose_goal_rubric` re-announced "Proposed acceptance
criteria are ready." on every regeneration, so the transcript
accumulated a duplicate message each cycle. This suppresses the success
message when regenerating from feedback (the review widget is remounted
with the updated criteria anyway) while still surfacing any
unsaved-state warning.

Made by [Open
SWE](https://openswe.vercel.app/agents/2b61a2fb-628a-5e98-6e33-5cc275d5d42a)

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
Mason Daugherty
2026-07-08 21:08:04 -04:00
committed by GitHub
parent ee3c26415b
commit 1110497e17
2 changed files with 87 additions and 3 deletions
+14 -2
View File
@@ -8368,15 +8368,22 @@ class DeepAgentsApp(App):
return False
return True
async def _mount_goal_rubric_result(self, message: str, *, persisted: bool) -> None:
async def _mount_goal_rubric_result(
self, message: str, *, persisted: bool, suppress_success: bool = False
) -> None:
"""Mount a goal/rubric command result, flagging unsaved state.
Args:
message: Success text describing the applied change.
persisted: Whether `_persist_goal_rubric_state` confirmed the write.
suppress_success: When `True`, skip the success message on a
persisted write but still surface the unsaved-state warning.
Used by revision cycles that would otherwise duplicate a
message already shown for the first proposal.
"""
if persisted:
await self._mount_message(AppMessage(message))
if not suppress_success:
await self._mount_message(AppMessage(message))
return
await self._mount_message(
ErrorMessage(
@@ -8943,9 +8950,14 @@ class DeepAgentsApp(App):
self._pending_goal_objective = objective
self._pending_goal_rubric = rubric
persisted = await self._persist_goal_rubric_state()
# On a revision the remounted review widget already shows the updated
# criteria, so re-announcing readiness is redundant and would duplicate
# the message from the first proposal. The unsaved-state warning still
# surfaces regardless (see `_mount_goal_rubric_result`).
await self._mount_goal_rubric_result(
"Proposed acceptance criteria are ready.",
persisted=persisted,
suppress_success=feedback is not None,
)
await self._start_pending_goal_rubric_review()
+73 -1
View File
@@ -5772,6 +5772,72 @@ class TestGoalCommand:
assert app._pending_goal_objective == "add refresh tokens"
assert app._pending_goal_rubric == "- model draft"
async def test_goal_revision_does_not_duplicate_ready_message(self) -> None:
"""Revision cycles must not re-announce readiness (see duplicate bug)."""
app = DeepAgentsApp(agent=MagicMock())
async with app.run_test() as pilot:
await pilot.pause()
# Stub the review continuation so no goal-review task is scheduled;
# the dedup under test runs before it in `_propose_goal_rubric`.
with (
patch.object(app, "_generate_goal_rubric", return_value="- tests pass"),
patch.object(
app, "_start_pending_goal_rubric_review", new_callable=AsyncMock
),
patch.object(app, "_set_spinner", new_callable=AsyncMock),
):
await app._propose_goal_rubric("add refresh tokens")
await pilot.pause()
await app._propose_goal_rubric(
"add refresh tokens",
feedback="be thorough in research",
previous_criteria="- tests pass",
)
await pilot.pause()
ready_messages = [
w
for w in app.query(AppMessage)
if str(w._content) == "Proposed acceptance criteria are ready."
]
assert len(ready_messages) == 1
async def test_goal_revision_warns_when_persist_fails_despite_suppression(
self,
) -> None:
"""A revision that cannot be saved must still surface the unsaved warning."""
app = DeepAgentsApp(agent=MagicMock())
async with app.run_test() as pilot:
await pilot.pause()
app._agent = SimpleNamespace(
aupdate_state=AsyncMock(side_effect=RuntimeError("down"))
)
app._lc_thread_id = "thread-1"
with (
patch.object(app, "_generate_goal_rubric", return_value="- tests pass"),
patch.object(
app, "_start_pending_goal_rubric_review", new_callable=AsyncMock
),
patch.object(app, "_set_spinner", new_callable=AsyncMock),
):
await app._propose_goal_rubric(
"add refresh tokens",
feedback="be thorough in research",
previous_criteria="- old criteria",
)
await pilot.pause()
# Success text is suppressed on the revision, but the failed write
# must still warn the user that the change is unsaved.
assert not any(
str(w._content) == "Proposed acceptance criteria are ready."
for w in app.query(AppMessage)
)
assert any(
"could not be saved to the thread" in str(w._content)
for w in app.query(ErrorMessage)
)
async def test_goal_review_regeneration_failure_remounts_pending_review(
self,
) -> None:
@@ -5871,7 +5937,13 @@ class TestGoalCommand:
thread_id="thread-1",
preloaded_payload=payload,
)
await pilot.pause()
# The review task is scheduled via `call_after_refresh`, which
# can take more than one refresh to fire; wait for it instead of
# racing a single pause (previously flaky under event-loop load).
for _ in range(50):
await pilot.pause()
if app._goal_review_task is not None:
break
menu = app.query_one(GoalReviewMenu)
assert app._pending_goal_review_widget is menu