Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions codeframe/core/blockers.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,19 @@ def create(
Created Blocker
"""
origin = BlockerOrigin(created_by)

# Dedupe on (task_id, question, OPEN) — issue #735. An adapter (react/plan/
# verification_wrapper) persists the blocker, then runtime.execute_agent creates
# a second one from result.blocker_question. Return the existing OPEN blocker so
# one escalation yields one blocker and one webhook; first writer's origin wins.
# NOTE: read-then-insert with no UNIQUE index — safe here because the two callers
# (adapter + runtime) run sequentially in the same process for one run, not
# concurrently. A partial UNIQUE index on (workspace_id, task_id, question) WHERE
# status='OPEN' would make it atomic against cross-process races.
existing = _find_open_duplicate(workspace, task_id, question)
if existing is not None:
return existing

blocker_id = str(uuid.uuid4())
now = _utc_now().isoformat()

Expand Down Expand Up @@ -126,6 +139,33 @@ def create(
return blocker


def _find_open_duplicate(
workspace: Workspace, task_id: Optional[str], question: str
) -> Optional[Blocker]:
"""Return an existing OPEN blocker with the same (task_id, question), if any."""
conn = get_db_connection(workspace)
cursor = conn.cursor()

select = (
"SELECT id, workspace_id, task_id, question, answer, status, created_at, "
"answered_at, COALESCE(created_by, 'human') as created_by FROM blockers "
"WHERE workspace_id = ? AND question = ? AND status = ?"
)
params: list = [workspace.id, question, BlockerStatus.OPEN.value]
# SQL `task_id = NULL` never matches; use `IS NULL` for workspace-level blockers.
if task_id is None:
select += " AND task_id IS NULL"
else:
select += " AND task_id = ?"
params.append(task_id)

cursor.execute(select, params)
row = cursor.fetchone()
conn.close()

return _row_to_blocker(row) if row else None


def _dispatch_blocker_webhook(
workspace: Workspace, blocker_id: str, task_id: Optional[str]
) -> None:
Expand Down
73 changes: 73 additions & 0 deletions tests/core/test_blockers_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,76 @@ def test_webhook_failure_does_not_break_blocker_create(workspace):
):
blocker = blockers.create(workspace, question="why?")
assert blocker.id # still got created


def test_duplicate_open_blocker_is_deduped(workspace):
"""One escalation → exactly one OPEN blocker and one webhook (issue #735).

Adapter + runtime each call create() with the same (task_id, question); the
second must return the existing OPEN blocker without a new row or webhook.
"""
save_notifications_config(
workspace,
{"webhook_url": "https://example.com/h", "webhook_enabled": True},
)
with patch(
"codeframe.notifications.webhook.WebhookNotificationService"
) as MockSvc:
instance = MockSvc.return_value
first = blockers.create(
workspace, question="Gates failing?", task_id="t-1", created_by="agent"
)
second = blockers.create(
workspace, question="Gates failing?", task_id="t-1"
)

assert second.id == first.id # same blocker, not a duplicate
assert second.created_by == first.created_by # first writer (agent) wins
assert len(blockers.list_open(workspace)) == 1
instance.send_event_background.assert_called_once() # only one webhook


def test_dedupe_does_not_collapse_distinct_questions(workspace):
"""Different questions on the same task remain separate blockers."""
a = blockers.create(workspace, question="Q1?", task_id="t-1")
b = blockers.create(workspace, question="Q2?", task_id="t-1")
assert a.id != b.id
assert len(blockers.list_open(workspace)) == 2


def test_dedupe_scoped_to_open_status(workspace):
"""An answered blocker does not suppress a new one with the same question."""
first = blockers.create(workspace, question="Same?", task_id="t-1")
blockers.answer(workspace, first.id, "resolved")
second = blockers.create(workspace, question="Same?", task_id="t-1")
assert second.id != first.id
assert len(blockers.list_open(workspace)) == 1


def test_dedupe_handles_null_task_id(workspace):
"""Workspace-level (task_id=None) duplicates are deduped too."""
first = blockers.create(workspace, question="Global?")
second = blockers.create(workspace, question="Global?")
assert second.id == first.id
assert len(blockers.list_open(workspace)) == 1


def test_dedupe_scoped_to_task_id(workspace):
"""Same question on different tasks stays as separate blockers."""
a = blockers.create(workspace, question="Q?", task_id="t-1")
b = blockers.create(workspace, question="Q?", task_id="t-2")
assert a.id != b.id
assert len(blockers.list_open(workspace)) == 2


def test_dedupe_does_not_cross_workspaces(tmp_path):
"""The same question in two workspaces produces two distinct blockers."""
questions = "Cross?"
ids = []
for name in ("ws-a", "ws-b"):
ws_path = tmp_path / name
ws_path.mkdir(parents=True, exist_ok=True)
ws = create_or_load_workspace(ws_path)
ids.append(blockers.create(ws, question=questions, task_id="t-1").id)
assert len(blockers.list_open(ws)) == 1
assert ids[0] != ids[1]
Loading