From b737f0fd1ea295853a2f7109bea02c3171a728ad Mon Sep 17 00:00:00 2001 From: droidlyx Date: Tue, 28 Apr 2026 12:30:45 +0800 Subject: [PATCH 1/2] fix(ui): prefer unresolved blocking interaction over latest threaded for default reply target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quest 014's user had to send "同意" four times before complete_quest fired: the React workspace's default `reply_to_interaction_id` always resolved to the most recent threaded `progress` heartbeat (e.g. mailbox-poll `__noop__`) even when an unresolved `quest_completion_approval` decision was waiting behind it in the feed. `findReplyTargetId` previously walked the feed once and returned at the first artifact with either `replyMode === 'blocking'` or `replyMode === 'threaded' && interactionId`. Because that single loop returned on the threaded fallback as soon as it saw a recent heartbeat, the older blocking decision was never reached. Split into two passes: first scan for a blocking / `expectsReply` artifact; only when none exists fall back to the latest threaded interaction. Export the function so it can be unit tested. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../lib/__tests__/findReplyTargetId.test.ts | 72 +++++++++++++++++++ src/ui/src/lib/acp.ts | 11 ++- 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 src/ui/src/lib/__tests__/findReplyTargetId.test.ts diff --git a/src/ui/src/lib/__tests__/findReplyTargetId.test.ts b/src/ui/src/lib/__tests__/findReplyTargetId.test.ts new file mode 100644 index 00000000..c73f8c35 --- /dev/null +++ b/src/ui/src/lib/__tests__/findReplyTargetId.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' + +import { findReplyTargetId } from '@/lib/acp' +import type { FeedItem } from '@/types' + +type ArtifactItem = Extract + +function artifact(overrides: Partial): ArtifactItem { + return { + id: 'art-default', + type: 'artifact', + kind: 'progress', + content: '', + ...overrides, + } +} + +describe('findReplyTargetId', () => { + it('prefers an unresolved blocking decision over more recent threaded heartbeats', () => { + // Reproduces quest 014: a blocking quest_completion_approval is followed + // by several `__noop__` progress heartbeats. Without the two-pass scan the + // most recent threaded heartbeat would steal the reply target and "同意" + // would never reach the actual approval request. + const feed: FeedItem[] = [ + artifact({ + id: 'decision-b5351a4f', + kind: 'decision', + interactionId: 'decision-b5351a4f', + replyMode: 'blocking', + expectsReply: true, + }), + artifact({ id: 'progress-1', interactionId: 'progress-1', replyMode: 'threaded' }), + artifact({ id: 'progress-2', interactionId: 'progress-2', replyMode: 'threaded' }), + artifact({ id: 'progress-3', interactionId: 'progress-3', replyMode: 'threaded' }), + ] + expect(findReplyTargetId(feed)).toBe('decision-b5351a4f') + }) + + it('falls back to the latest threaded interaction when no blocking one is open', () => { + const feed: FeedItem[] = [ + artifact({ id: 'progress-old', interactionId: 'progress-old', replyMode: 'threaded' }), + artifact({ id: 'progress-new', interactionId: 'progress-new', replyMode: 'threaded' }), + ] + expect(findReplyTargetId(feed)).toBe('progress-new') + }) + + it('matches expectsReply even without an explicit blocking replyMode', () => { + const feed: FeedItem[] = [ + artifact({ + id: 'question', + interactionId: 'question', + expectsReply: true, + }), + artifact({ id: 'progress-after', interactionId: 'progress-after', replyMode: 'threaded' }), + ] + expect(findReplyTargetId(feed)).toBe('question') + }) + + it('returns the interaction id when present, otherwise falls back to the item id', () => { + const feed: FeedItem[] = [ + artifact({ id: 'art-id', interactionId: undefined, replyMode: 'blocking', expectsReply: true }), + ] + expect(findReplyTargetId(feed)).toBe('art-id') + }) + + it('returns null when no artifacts are present', () => { + const feed: FeedItem[] = [ + { id: 'm', type: 'message', role: 'user', content: 'hi' }, + ] + expect(findReplyTargetId(feed)).toBeNull() + }) +}) diff --git a/src/ui/src/lib/acp.ts b/src/ui/src/lib/acp.ts index c1da3abe..eb631932 100644 --- a/src/ui/src/lib/acp.ts +++ b/src/ui/src/lib/acp.ts @@ -815,13 +815,22 @@ function collectSealedAssistantRunIds(updates: Array>) { return Array.from(sealed) } -function findReplyTargetId(feed: FeedItem[]) { +export function findReplyTargetId(feed: FeedItem[]) { + // Prefer the most recent unresolved blocking interaction (decision_request, + // approval, blocking question) so that a free-text reply like "同意" lands on + // the artifact actually waiting for a user verdict — not on a `progress` + // heartbeat that happens to be more recent in the feed. Only when no such + // blocking interaction exists do we fall back to the latest threaded one. for (let index = feed.length - 1; index >= 0; index -= 1) { const item = feed[index] if (item.type !== 'artifact') continue if (item.replyMode === 'blocking' || item.expectsReply) { return item.interactionId || item.id } + } + for (let index = feed.length - 1; index >= 0; index -= 1) { + const item = feed[index] + if (item.type !== 'artifact') continue if (item.replyMode === 'threaded' && item.interactionId) { return item.interactionId } From d294aa2f6e503e5fd155b8fab82cb617c9103633 Mon Sep 17 00:00:00 2001 From: droidlyx Date: Tue, 28 Apr 2026 12:35:55 +0800 Subject: [PATCH 2/2] fix(artifact): suppress noop progress heartbeat when a blocking interaction is unresolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While quest 014 waited for a user reply on its quest_completion_approval decision_request, the runtime emitted ~12 `__noop__` mailbox-poll progress heartbeats over four hours. Each one was committed to git, recorded in the interaction journal, and queued for connector delivery — and each one pushed the actual approval request further down the user's feed, making "同意" land on a heartbeat instead of the decision. Add `_has_unresolved_blocking_interaction(quest_root)` which scans `open_requests` for any waiting entry — by construction these are always blocking, since `_update_interaction_state` only inserts into `open_requests` for `reply_mode='blocking'` calls. In `interact()`, short-circuit before the existing dedupe block: when the incoming call is a `__noop__` / `__mailbox_poll__` progress heartbeat and a blocking interaction is still waiting on the user, return `status='suppressed_blocking_pending'` without recording an artifact. The substantive-progress path is unchanged, so a real status update (e.g. "training crashed at step 1200") still records normally even during a blocking wait. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/deepscientist/artifact/service.py | 66 +++++++++++++++++++++ tests/test_memory_and_artifact.py | 84 +++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/src/deepscientist/artifact/service.py b/src/deepscientist/artifact/service.py index b0aad269..1ed4ec09 100644 --- a/src/deepscientist/artifact/service.py +++ b/src/deepscientist/artifact/service.py @@ -73,6 +73,7 @@ from .schemas import ARTIFACT_DIRS, guidance_for_kind, validate_artifact_payload QUEST_COMPLETION_DECISION_TYPE = "quest_completion_approval" +_NOOP_HEARTBEAT_MARKERS = frozenset({"__noop__", "__mailbox_poll__"}) _COMPLETION_APPROVAL_TERMS = ( "同意完成", "确认完成", @@ -13024,6 +13025,52 @@ def interact( suppress_resolved = (kind == "progress") if suppress_if_unchanged is None else bool(suppress_if_unchanged) dedupe_key_resolved = str(dedupe_key or self._normalize_interaction_message(full_message)).strip() or None pending_user_message_count = int(self.quest_service.snapshot(self._quest_id(quest_root)).get("pending_user_message_count") or 0) + normalized_message = self._normalize_interaction_message(full_message) + if ( + kind == "progress" + and pending_user_message_count == 0 + and normalized_message in _NOOP_HEARTBEAT_MARKERS + and self._has_unresolved_blocking_interaction(quest_root) + ): + interaction_state = self._read_interaction_state(quest_root) + waiting_requests = [ + dict(item) + for item in (interaction_state.get("open_requests") or []) + if str(item.get("status") or "") == "waiting" + ] + return { + "status": "suppressed_blocking_pending", + "artifact_id": None, + "interaction_id": None, + "expects_reply": False, + "reply_mode": "threaded", + "surface_actions": [], + "connector_hints": connector_hints_resolved, + "normalized_attachments": attachments_resolved, + "attachment_issues": attachment_issues, + "delivered": False, + "delivery_results": [], + "response_phase": response_phase, + "delivery_targets": [], + "delivery_policy": self._delivery_policy(self._connectors_config()), + "preferred_connector": self._preferred_connector(self._connectors_config()), + "recent_inbound_messages": [], + "delivery_batch": None, + "recent_interaction_records": self.quest_service.latest_artifact_interaction_records(quest_root, limit=10), + "agent_instruction": self.quest_service.localized_copy( + quest_root=quest_root, + zh="当前已有一个等待用户回复的阻塞 interaction，再发心跳只会把它压到下方。请安静等待，不要再 record __noop__ progress。", + en="A blocking interaction is already waiting on the user. Recording another __noop__ heartbeat would only push it further down the feed. Stay quiet until the user replies.", + ), + "queued_message_count_before_delivery": 0, + "queued_message_count_after_delivery": 0, + "open_request_count": len(waiting_requests), + "active_request": waiting_requests[-1] if waiting_requests else None, + "default_reply_interaction_id": interaction_state.get("default_reply_interaction_id"), + "guidance": "Noop heartbeat suppressed because a blocking interaction is still waiting for the user.", + "suppressed_reason": "blocking_pending", + "dedupe_key": dedupe_key_resolved, + } if ( kind == "progress" and suppress_resolved @@ -13954,6 +14001,25 @@ def _interaction_decision_type(item: dict[str, Any]) -> str: reply_schema = item.get("reply_schema") if isinstance(item.get("reply_schema"), dict) else {} return str(reply_schema.get("decision_type") or "").strip() + def _has_unresolved_blocking_interaction(self, quest_root: Path) -> bool: + # An "unresolved blocking interaction" is any open_request that has + # not yet been answered (status='waiting'). Entries in open_requests + # are only created for blocking decision_request / approval calls in + # _update_interaction_state, so a waiting entry is by construction + # waiting on an explicit user reply — no further reply_mode check is + # needed (and indeed the reply_mode field is not copied into the + # open_requests record). Used to suppress noop progress heartbeats so + # the actual blocking artifact stays near the top of the user's feed + # instead of being pushed down by ack pings. + state = self._read_interaction_state(quest_root) + for item in state.get("open_requests") or []: + if not isinstance(item, dict): + continue + if str(item.get("status") or "").lower() != "waiting": + continue + return True + return False + def _latest_completion_request(self, quest_root: Path) -> dict[str, Any] | None: state = self._read_interaction_state(quest_root) candidates: list[dict[str, Any]] = [] diff --git a/tests/test_memory_and_artifact.py b/tests/test_memory_and_artifact.py index 8e29f31f..951ffa99 100644 --- a/tests/test_memory_and_artifact.py +++ b/tests/test_memory_and_artifact.py @@ -5132,6 +5132,90 @@ def test_duplicate_progress_is_suppressed_when_message_is_unchanged(temp_home: P assert len(outbound) == 1 +def test_noop_progress_is_suppressed_when_blocking_interaction_is_pending(temp_home: Path) -> None: + # Reproduces quest 014's behavior: while a blocking + # quest_completion_approval is waiting on the user, every mailbox-poll + # cycle should NOT record another `__noop__` progress artifact (which + # would push the actual approval request further down the feed and + # produce a stream of cosmetic git commits / connector notifications). + ensure_home_layout(temp_home) + ConfigManager(temp_home).ensure_files() + quest_service = QuestService(temp_home, skill_installer=SkillInstaller(repo_root(), temp_home)) + quest = quest_service.create( + "noop suppression while blocking quest", + startup_contract={"decision_policy": "autonomous"}, + ) + quest_root = Path(quest["quest_root"]) + artifact = ArtifactService(temp_home) + + request = artifact.interact( + quest_root, + kind="decision_request", + message="Quest looks complete. May I close it?", + deliver_to_bound_conversations=False, + include_recent_inbound_messages=False, + reply_mode="blocking", + reply_schema={"decision_type": "quest_completion_approval"}, + ) + assert request["status"] == "ok" + assert request["reply_mode"] == "blocking" + progress_dir = quest_root / "artifacts" / "progress" + progress_count_before = len(list(progress_dir.glob("*.json"))) if progress_dir.exists() else 0 + + suppressed = artifact.interact( + quest_root, + kind="progress", + message="__noop__", + deliver_to_bound_conversations=False, + include_recent_inbound_messages=True, + suppress_if_unchanged=True, + dedupe_key="mailbox-poll-only-test", + ) + assert suppressed["status"] == "suppressed_blocking_pending" + assert suppressed["suppressed_reason"] == "blocking_pending" + assert suppressed["artifact_id"] is None + assert suppressed["open_request_count"] == 1 + progress_count_after = len(list(progress_dir.glob("*.json"))) if progress_dir.exists() else 0 + assert progress_count_after == progress_count_before + + +def test_substantive_progress_passes_through_when_blocking_interaction_is_pending(temp_home: Path) -> None: + # Sanity guard: only `__noop__` / `__mailbox_poll__` heartbeats are + # suppressed during a blocking wait — a substantive progress message + # (e.g. "training crashed at step 1200") must still record so the user + # learns the situation has changed. + ensure_home_layout(temp_home) + ConfigManager(temp_home).ensure_files() + quest_service = QuestService(temp_home, skill_installer=SkillInstaller(repo_root(), temp_home)) + quest = quest_service.create( + "substantive progress while blocking quest", + startup_contract={"decision_policy": "autonomous"}, + ) + quest_root = Path(quest["quest_root"]) + artifact = ArtifactService(temp_home) + + request = artifact.interact( + quest_root, + kind="decision_request", + message="Quest looks complete. May I close it?", + deliver_to_bound_conversations=False, + include_recent_inbound_messages=False, + reply_mode="blocking", + reply_schema={"decision_type": "quest_completion_approval"}, + ) + assert request["status"] == "ok" + + real_update = artifact.interact( + quest_root, + kind="progress", + message="Training crashed at step 1200; restarting from checkpoint.", + deliver_to_bound_conversations=False, + include_recent_inbound_messages=False, + ) + assert real_update["status"] == "ok" + assert real_update["artifact_id"] + + def test_interact_preserves_full_message_for_delivery_and_records_summary_preview(temp_home: Path) -> None: ensure_home_layout(temp_home) ConfigManager(temp_home).ensure_files()