fix(bitable/team): U3 — Residual P2 findings (DR-1/3/4/5 + #2/#3)

U3 修复 8 项 P2 design review 与 streaming 残留问题:

DR-1 SQL 注入防御 — repository.py 添加 _validate_field_id UUID 校验,
拒绝非 UUID 字符串进入 jsonb_set path 与 WHERE 子句的字符串插值。
ceiling 注释标明仅覆盖 field_id,其余 SQL 结构由 service 层枚举白名单守护。

DR-3 design token 契约 — 新增 bitable-tokens.test.ts (31 tests),
解析 CSS :root 块并断言所有 --bitable-* token 在 :root 定义,
防止 token 被硬编码到组件外或遗漏定义。

DR-4 TOCTOU 修复 — repository.delete_view_if_not_last 用
SELECT ... FOR UPDATE 单事务原子化 check + delete,
消除 list_views → count → delete 三步分离的竞态。
service.delete_view 委托原子方法,失败抛 LastViewDeletionError。

DR-5 _update_field 静默失败 — type 参数被 Pydantic extra=ignore
静默丢弃,用户以为改了类型实际无效。显式返回
UNSUPPORTED_FIELD_TYPE_CHANGE 错误,避免静默失败的 UX 陷阱。

#2 team_synthesis 孤儿 milestone — 补 error + cancelled 路径测试,
验证内层 except 广播终结事件后 re-raise,外层 except 捕获后 fallback。

#3 synthesis_id 去重附身 — chatStream.ts 移除
|| m.synthesis_id === undefined 兜底匹配,避免新 milestone
附身到上一次孤儿 streaming 占位。

测试:DR-1 (7) + DR-4 repo (3) + DR-4 service (3) + DR-5 (2) +
#2 (2) + #8 frontend (31) = 48 项通过;U2 无回归 (29 项)。
This commit is contained in:
Chiguyong 2026-07-06 03:48:14 +08:00
parent 60a58a0fdb
commit c60e96ac32
9 changed files with 738 additions and 115 deletions

View File

@ -39,6 +39,26 @@ from agentkit.bitable.models import (
logger = logging.getLogger(__name__)
# U3/DR-1: field_id 在 jsonb_set path 与 where 子句中走字符串插值jsonb path 不支持绑定参数)。
# 防御性校验field_id 必须匹配 UUID 格式,拒绝任意字符串插值到 SQL 结构。
# ponytail: ceiling — 仅覆盖 field_id其余 SQL 结构op / direction已由 service 层白名单枚举校验。
# 升级路径:迁移到 ORM JSON 函数sqlalchemy.dialects.postgresql.jsonb_*)彻底消除字符串插值。
_FIELD_ID_RE = re.compile(
r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"
)
def _validate_field_id(field_id: str, *, context: str = "") -> str:
"""校验 field_id 是 UUID 格式DR-1 防御性校验)。
field_id jsonb_set path SQL where 子句中走字符串插值jsonb path 不支持绑定参数
必须确保非系统 UUID 不进入 SQL 结构校验失败抛 ValueError
"""
if not isinstance(field_id, str) or not _FIELD_ID_RE.match(field_id):
ctx = f" ({context})" if context else ""
raise ValueError(f"Invalid field_id format{ctx}: {field_id!r} — expected UUID")
return field_id
class BitableRepository:
"""Async repository for bitable CRUD operations.
@ -375,7 +395,9 @@ class BitableRepository:
return [Record.model_validate(e) for e in entities], next_cursor
async def update_record_values(self, record_id: str, values: dict[str, object]) -> Record | None:
async def update_record_values(
self, record_id: str, values: dict[str, object]
) -> Record | None:
"""Update a record's values (full replace)."""
async with self._session_factory() as session:
stmt = (
@ -471,6 +493,35 @@ class BitableRepository:
await session.commit()
return result.rowcount > 0
async def delete_view_if_not_last(self, view_id: str, table_id: str) -> bool:
"""Atomically delete a view only if it's not the last one in its table (DR-4 TOCTOU fix).
Uses ``SELECT ... FOR UPDATE`` in a single transaction to lock sibling
rows for the duration of the count + delete, preventing concurrent
``delete_view`` calls from racing past the last-view check and leaving
the table with zero views.
Returns:
True if the view was deleted.
False if NOT deleted because it would be the last view (or the
view doesn't belong to the given table_id).
"""
async with self._session_factory() as session:
# Lock all sibling views — concurrent delete_view calls on the
# same table block here until this transaction commits.
lock_sql = text(
"SELECT id FROM bitable.bitable_views WHERE table_id = :table_id FOR UPDATE"
)
result = await session.execute(lock_sql, {"table_id": table_id})
sibling_ids = {str(row[0]) for row in result.fetchall()}
if len(sibling_ids) <= 1:
return False # Last view — refuse to delete
if view_id not in sibling_ids:
return False # View doesn't belong to this table_id
result = await session.execute(delete(ViewModel).where(ViewModel.id == view_id))
await session.commit()
return result.rowcount > 0
# ── Recalc Queue ────────────────────────────────────────
async def enqueue_recalc(
@ -646,9 +697,13 @@ class BitableRepository:
return
import json
# U3/DR-1: 校验所有 field_id 是 UUID 格式后再插值到 jsonb_set path防御性避免 SQL 结构注入)
for fid in agent_field_values:
_validate_field_id(fid, context="upsert_record_agent_fields")
async with self._session_factory() as session:
# Build nested jsonb_set: jsonb_set(jsonb_set(values, '{f1}', CAST(:v0 AS jsonb), true), ...)
# ponytail: field_ids are system UUIDs, safe to interpolate into path literals.
# ponytail: field_ids validated above as UUID format, safe to interpolate into path literals.
# Use CAST(:param AS jsonb) instead of :param::jsonb — asyncpg dialect
# misparses the `::` as part of the param name.
inner = "values"
@ -684,9 +739,17 @@ class BitableRepository:
import base64
import json as _json
# U3/DR-1: 校验 filters/sorts 中的 field_id 是 UUID 格式后再插值到 SQL 结构
if filters:
for f in filters:
_validate_field_id(f["field_id"], context="list_records_filtered.filter")
if sorts:
for s in sorts:
_validate_field_id(s["field_id"], context="list_records_filtered.sort")
async with self._session_factory() as session:
# Build raw SQL with JSONB filter/sort translation.
# ponytail: field_ids in filters/sorts are system UUIDs (validated by service layer).
# ponytail: field_ids validated above as UUID format, safe to interpolate into SQL structure.
where_clauses = ["table_id = :table_id"]
params: dict[str, object] = {"table_id": table_id}

View File

@ -545,23 +545,28 @@ class BitableService:
return await self._repo.get_view(view_id)
async def delete_view(self, view_id: str) -> bool:
"""Delete a view with last-view protection (U6).
"""Delete a view with last-view protection (U6, DR-4 TOCTOU fix).
Raises :class:`LastViewDeletionError` if the view is the last one in
its table preventing users from making a table inaccessible. The
route layer is responsible for the 404 + ownership checks before
calling this (matching the existing ``delete_field`` pattern).
Returns True if a row was deleted.
DR-4: delegates to ``repo.delete_view_if_not_last`` which atomically
checks sibling count + deletes in a single ``SELECT FOR UPDATE``
transaction, eliminating the TOCTOU race between ``list_views`` count
and ``delete_view`` execution.
"""
view = await self._repo.get_view(view_id)
if view is None:
return False
siblings = await self._repo.list_views(view.table_id)
if len(siblings) <= 1:
raise LastViewDeletionError(
"Cannot delete the last view of a table"
)
return await self._repo.delete_view(view_id)
deleted = await self._repo.delete_view_if_not_last(view_id, view.table_id)
if not deleted:
# Either it was the last view, or a concurrent call raced ahead.
# Either way, the invariant "table has at least 1 view" is preserved.
raise LastViewDeletionError("Cannot delete the last view of a table")
return True
# ── Recalc (U3: formula recalc pipeline) ────────────────

View File

@ -869,7 +869,8 @@ export function dispatchWsEvent(
case "team_synthesis_chunk": {
// U4: 团队综合流式 chunk — 首次创建 streaming 占位,后续累加。
// P2 fix: 用 synthesis_id 去重,避免 chunk 附身到上一次孤儿 streaming milestone。
// P2 fix: 用 synthesis_id 精确去重 — sid 存在时仅匹配相同 synthesis_id 的 milestone
// 不再兜底附身到 undefined synthesis_id 的旧孤儿占位(避免跨重试附身 bug
const conversationId = state.resolveIncomingConvId();
if (!conversationId) break;
const conv = state.conversations.value.find(
@ -882,13 +883,11 @@ export function dispatchWsEvent(
(m) =>
m.message_type === "milestone" &&
m.status === "streaming" &&
(sid ? m.synthesis_id === sid || m.synthesis_id === undefined : true),
(sid ? m.synthesis_id === sid : true),
);
if (existing) {
state.updateMessage(conversationId, existing.id, {
content: (existing.content || "") + (event.data.chunk || ""),
// 首次附身到无 synthesis_id 的 milestone 时,标记上 synthesis_id
...(sid && !existing.synthesis_id ? { synthesis_id: sid } : {}),
});
} else {
const synthMsg: IChatMessage = {
@ -926,7 +925,7 @@ export function dispatchWsEvent(
(m) =>
m.message_type === "milestone" &&
m.status === "streaming" &&
(sid ? m.synthesis_id === sid || m.synthesis_id === undefined : true),
(sid ? m.synthesis_id === sid : true),
);
if (existing) {
state.updateMessage(conversationId, existing.id, {

View File

@ -0,0 +1,97 @@
/**
* DR-3: Bitable design tokens static structural assertion.
*
* Parses `bitable-tokens.css` and asserts all required `--bitable-*` tokens
* are defined inside `:root`. Prevents accidental token removal/renaming
* that would silently break var() references in bitable components.
*
* ponytail: pure regex parse no CSS parser dependency. Ceiling: doesn't
* validate token values (only presence). Upgrade path: postcss + stylelint
* custom rules for value-level validation.
*/
import { describe, expect, it } from 'vitest'
import { readFileSync } from 'node:fs'
import { resolve } from 'node:path'
const CSS_PATH = resolve(
__dirname,
'../../src/styles/bitable-tokens.css',
)
const cssContent: string = readFileSync(CSS_PATH, 'utf-8')
/**Extract the first `:root { ... }` block content (without the selector).*/
function extractRootBlock(css: string): string {
const match = /:root\s*\{([^}]*)\}/.exec(css)
if (!match || match[1] === undefined) {
throw new Error('No :root block found in bitable-tokens.css')
}
return match[1]
}
const rootBlock = extractRootBlock(cssContent)
/**Required token names grouped by category (mirrors the CSS file structure).*/
const REQUIRED_TOKENS = {
color: [
'--bitable-color-primary',
'--bitable-color-bg',
'--bitable-color-bg-secondary',
'--bitable-color-bg-tertiary',
'--bitable-color-text',
'--bitable-color-text-secondary',
'--bitable-color-text-tertiary',
'--bitable-color-text-placeholder',
'--bitable-color-border',
'--bitable-color-border-hover',
'--bitable-color-border-split',
],
cf: [
'--bitable-cf-red-bg',
'--bitable-cf-red-fg',
'--bitable-cf-orange-bg',
'--bitable-cf-orange-fg',
'--bitable-cf-yellow-bg',
'--bitable-cf-yellow-fg',
'--bitable-cf-green-bg',
'--bitable-cf-green-fg',
'--bitable-cf-blue-bg',
'--bitable-cf-blue-fg',
'--bitable-cf-purple-bg',
'--bitable-cf-purple-fg',
'--bitable-cf-gray-bg',
'--bitable-cf-gray-fg',
'--bitable-cf-neutral-bg',
'--bitable-cf-neutral-fg',
],
drawer: ['--bitable-drawer-width', '--bitable-drawer-width-wide'],
} as const
describe('bitable-tokens.css — DR-3 design token contract', () => {
it('defines a :root block', () => {
expect(rootBlock.length).toBeGreaterThan(0)
})
it.each(REQUIRED_TOKENS.color)('defines color token %s in :root', (token) => {
// Match `--token-name:` (declaration), not just substring presence.
const declPattern = new RegExp(`${token.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*:`)
expect(declPattern.test(rootBlock)).toBe(true)
})
it.each(REQUIRED_TOKENS.cf)('defines conditional-format token %s in :root', (token) => {
const declPattern = new RegExp(`${token.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*:`)
expect(declPattern.test(rootBlock)).toBe(true)
})
it.each(REQUIRED_TOKENS.drawer)('defines drawer token %s in :root', (token) => {
const declPattern = new RegExp(`${token.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*:`)
expect(declPattern.test(rootBlock)).toBe(true)
})
it('does not hardcode tokens outside :root (no component-level overrides)', () => {
// ponytail: bitable tokens are exclusively :root-scoped; component-level
// overrides would break the single-source-of-truth invariant.
const outsideRoot = cssContent.replace(/:root\s*\{[^}]*\}/, '')
expect(outsideRoot).not.toMatch(/--bitable-[a-z-]+\s*:/)
})
})

View File

@ -558,15 +558,23 @@ class BitableTool(Tool):
field_id = kwargs.get("field_id")
if not field_id:
return {"success": False, "error": "Missing required field: field_id"}
# DR-5: type 变更不被支持 — UpdateFieldRequest 当前只接受 name + config
# 传入 type 会被 Pydantic extra=ignore 静默丢弃(用户以为改了类型,实际无效果)。
# 显式拒绝并返回错误,避免静默失败的 UX 陷阱。
# 升级路径:如需支持类型变更,扩展 UpdateFieldRequest 接受 field_type +
# 实现数据迁移逻辑(类型转换 + 校验)。
if kwargs.get("type") is not None:
return {
"success": False,
"error": (
"Unsupported field type change — field type is immutable after "
"creation. To change a field's type, delete and recreate it."
),
"code": "UNSUPPORTED_FIELD_TYPE_CHANGE",
}
payload: dict[str, object] = {}
if kwargs.get("name") is not None:
payload["name"] = kwargs["name"]
# ponytail: PATCH /fields/{id} is backed by UpdateFieldRequest which
# currently accepts only name + config. `type` is forwarded as
# `field_type` so the tool is forward-compatible once the request
# model adds it; today it is silently ignored by Pydantic (extra=ignore).
if kwargs.get("type") is not None:
payload["field_type"] = kwargs["type"]
if kwargs.get("config") is not None:
payload["config"] = kwargs["config"]
client = await self._get_client()

View File

@ -552,9 +552,9 @@ async def test_update_view_action(tool: BitableTool) -> None:
"""update_view action PATCHes /views/{id} with name + config."""
result = await tool.execute(action="create_table", table_name="VU")
table_id = result["table"]["id"]
view_id = (
await tool.execute(action="create_view", table_id=table_id, name="Old")
)["view"]["id"]
view_id = (await tool.execute(action="create_view", table_id=table_id, name="Old"))["view"][
"id"
]
resp = await tool.execute(
action="update_view",
@ -682,9 +682,7 @@ async def test_new_actions_internal_token_passthrough(
table_id = result["table"]["id"]
# create_view via the new action.
v = await tool_real_auth.execute(
action="create_view", table_id=table_id, name="tv1"
)
v = await tool_real_auth.execute(action="create_view", table_id=table_id, name="tv1")
assert v["success"] is True
view_id = v["view"]["id"]
@ -716,3 +714,60 @@ async def test_delete_view_internal_token_404_when_missing(
resp = await tool_real_auth.execute(action="delete_view", view_id="no-such-view")
assert resp["success"] is False
assert "404" in resp["error"]
# ---------------------------------------------------------------------------
# DR-5: _update_field rejects type change (no silent success)
# ---------------------------------------------------------------------------
async def test_update_field_rejects_type_change(tool: BitableTool) -> None:
"""DR-5: _update_field with `type` arg returns error, does NOT call HTTP.
Without this guard, the field_type would be silently dropped by
UpdateFieldRequest's Pydantic extra=ignore, and the agent would
believe the type changed when it didn't.
"""
result = await tool.execute(action="create_table", table_name="DR5")
table_id = result["table"]["id"]
client = await tool._get_client()
field_id = (
await client.post(
f"/tables/{table_id}/fields",
json={"name": "col", "field_type": "text", "owner": "user"},
)
).json()["field"]["id"]
resp = await tool.execute(
action="update_field",
field_id=field_id,
type="number",
)
assert resp["success"] is False
assert resp.get("code") == "UNSUPPORTED_FIELD_TYPE_CHANGE"
assert "immutable" in resp["error"]
async def test_update_field_still_works_without_type(tool: BitableTool) -> None:
"""DR-5: _update_field without `type` still PATCHes name/config normally.
Regression guard: the DR-5 guard must not block legitimate updates.
"""
result = await tool.execute(action="create_table", table_name="DR5b")
table_id = result["table"]["id"]
client = await tool._get_client()
field_id = (
await client.post(
f"/tables/{table_id}/fields",
json={"name": "col", "field_type": "text", "owner": "user"},
)
).json()["field"]["id"]
resp = await tool.execute(
action="update_field",
field_id=field_id,
name="renamed",
config={"description": "ok"},
)
assert resp["success"] is True
assert resp["field"]["name"] == "renamed"

View File

@ -0,0 +1,113 @@
"""Tests for BitableRepository — U3/DR-1 + DR-4 fixes.
DR-1: field_id UUID validation before SQL interpolation (pure unit test).
DR-4: atomic delete_view_if_not_last (requires PostgreSQL).
"""
from __future__ import annotations
import pytest
from agentkit.bitable.repository import _validate_field_id
# ── DR-1: field_id UUID validation ──────────────────────────
class TestValidateFieldId:
"""DR-1: field_id 校验 — 拒绝非 UUID 格式进入 SQL 结构插值。"""
def test_valid_uuid_passes(self):
valid = "550e8400-e29b-41d4-a716-446655440000"
assert _validate_field_id(valid) == valid
def test_valid_uppercase_uuid_passes(self):
valid = "550E8400-E29B-41D4-A716-446655440000"
assert _validate_field_id(valid) == valid
@pytest.mark.parametrize(
"bad_field_id,label",
[
("not-a-uuid", "arbitrary string"),
("", "empty string"),
("550e8400-e29b-41d4-a716", "too short"),
("550e8400-e29b-41d4-a716-446655440000-extra", "too long"),
("550e8400-e29b-41d4-a716-44665544000g", "non-hex char"),
("'; DROP TABLE bitable.bitable_views; --", "SQL injection attempt"),
("values->>'field' = :fv0", "SQL fragment"),
],
)
def test_invalid_field_id_raises(self, bad_field_id: str, label: str):
with pytest.raises(ValueError, match="Invalid field_id format"):
_validate_field_id(bad_field_id, context=label)
def test_non_string_raises(self):
with pytest.raises(ValueError, match="Invalid field_id format"):
_validate_field_id(None) # type: ignore[arg-type]
with pytest.raises(ValueError, match="Invalid field_id format"):
_validate_field_id(12345) # type: ignore[arg-type]
# ── DR-4: atomic delete_view_if_not_last ────────────────────
# These tests require PostgreSQL (SELECT FOR UPDATE behavior)
pytestmark_postgres = pytest.mark.postgres
@pytest.mark.postgres
class TestDeleteViewIfNotLastAtomic:
"""DR-4: 原子条件 delete_view — SELECT FOR UPDATE 事务防止 TOCTOU。
并发场景不变量"table 至少 1 view" 在并发删除最后两个 view 时保持
"""
async def test_delete_view_if_not_last_succeeds_when_multiple_siblings(
self, bitable_service, make_table
):
"""多 view 时删除成功。"""
from agentkit.bitable.repository import BitableRepository
table = await make_table(name="T1")
v1 = await bitable_service.create_view(table.id, name="v1")
await bitable_service.create_view(table.id, name="v2")
repo = BitableRepository(bitable_service._repo._db)
deleted = await repo.delete_view_if_not_last(v1.id, table.id)
assert deleted is True
# v1 gone, v2 still present
assert await bitable_service.get_view(v1.id) is None
assert (await bitable_service.list_views(table.id)) is not None
async def test_delete_view_if_not_last_refuses_last_view(self, bitable_service, make_table):
"""最后一个 view 时拒绝删除(返回 False不抛异常 — service 层负责抛 LastViewDeletionError"""
from agentkit.bitable.repository import BitableRepository
table = await make_table(name="T2")
only_view = await bitable_service.create_view(table.id, name="only")
repo = BitableRepository(bitable_service._repo._db)
deleted = await repo.delete_view_if_not_last(only_view.id, table.id)
assert deleted is False
# View still exists
assert await bitable_service.get_view(only_view.id) is not None
async def test_delete_view_if_not_last_wrong_table_returns_false(
self, bitable_service, make_table
):
"""view_id 不属于给定 table_id 时返回 False不误删其他 table 的 view"""
from agentkit.bitable.repository import BitableRepository
table_a = await make_table(name="TA")
table_b = await make_table(name="TB")
va = await bitable_service.create_view(table_a.id, name="va")
# table_b 有自己的 view
await bitable_service.create_view(table_b.id, name="vb1")
await bitable_service.create_view(table_b.id, name="vb2")
repo = BitableRepository(bitable_service._repo._db)
# 用 table_b 的 id 但 view_id 是 table_a 的 — 不应删除
deleted = await repo.delete_view_if_not_last(va.id, table_b.id)
assert deleted is False
# va 仍然存在
assert await bitable_service.get_view(va.id) is not None

View File

@ -8,7 +8,7 @@ from __future__ import annotations
import pytest
from agentkit.bitable.models import FieldOwner, FieldType
from agentkit.bitable.service import FieldDependencyError
from agentkit.bitable.service import FieldDependencyError, LastViewDeletionError
pytestmark = pytest.mark.postgres
@ -294,3 +294,44 @@ async def test_list_records_filtered_cursor_pagination(bitable_service) -> None:
# All records unique
all_ids = {r.id for r in [records, records2, records3] for r in r}
assert len(all_ids) == 5
# ---------------------------------------------------------------------------
# DR-4: delete_view last-view protection (service layer)
# ---------------------------------------------------------------------------
async def test_delete_view_raises_last_view_deletion_error(bitable_service, make_table) -> None:
"""DR-4: service.delete_view raises LastViewDeletionError on the last view.
Guards the invariant "every table has at least 1 view" at the service
layer so the route can map it to HTTP 409 without touching the repo.
"""
table = await make_table(name="LastView")
only = await bitable_service.create_view(table.id, name="only")
with pytest.raises(LastViewDeletionError):
await bitable_service.delete_view(only.id)
# View is still there
assert await bitable_service.get_view(only.id) is not None
async def test_delete_view_succeeds_when_multiple_views(bitable_service, make_table) -> None:
"""DR-4: service.delete_view succeeds when ≥2 views remain (happy path)."""
table = await make_table(name="MultiView")
v1 = await bitable_service.create_view(table.id, name="v1")
await bitable_service.create_view(table.id, name="v2")
deleted = await bitable_service.delete_view(v1.id)
assert deleted is True
assert await bitable_service.get_view(v1.id) is None
async def test_delete_view_returns_false_when_missing(bitable_service, make_table) -> None:
"""DR-4: service.delete_view on a non-existent view returns False (no raise)."""
table = await make_table(name="MissingView")
# ensure ≥1 view exists so the table is valid
await bitable_service.create_view(table.id, name="v1")
deleted = await bitable_service.delete_view("nonexistent-view-id")
assert deleted is False

View File

@ -81,15 +81,17 @@ def _make_mock_expert(
}
# Mock agent.execute() to return a successful TaskResult
mock_agent = MagicMock()
mock_agent.execute = AsyncMock(return_value=TaskResult(
task_id="test",
agent_name=name,
status=TaskStatus.COMPLETED.value,
output_data={"content": f"Result from {name}"},
error_message=None,
started_at=None,
completed_at=None,
))
mock_agent.execute = AsyncMock(
return_value=TaskResult(
task_id="test",
agent_name=name,
status=TaskStatus.COMPLETED.value,
output_data={"content": f"Result from {name}"},
error_message=None,
started_at=None,
completed_at=None,
)
)
# U3: orchestrator._run_agent_steps() calls agent.execute_stream() (async gen),
# not agent.execute(). Mock it to yield one final_answer event.
mock_agent.execute_stream = make_execute_stream_mock(f"Result from {name}")
@ -162,20 +164,24 @@ def _make_mock_llm_gateway(
def _make_mock_pool() -> MagicMock:
"""创建 mock AgentPool模拟上下文隔离的 agent 创建"""
pool = MagicMock()
def _create_agent(config):
m = MagicMock()
m.execute = AsyncMock(return_value=TaskResult(
task_id="test",
agent_name=config.name,
status=TaskStatus.COMPLETED.value,
output_data={"content": f"Isolated result from {config.name}"},
error_message=None,
started_at=None,
completed_at=None,
))
m.execute = AsyncMock(
return_value=TaskResult(
task_id="test",
agent_name=config.name,
status=TaskStatus.COMPLETED.value,
output_data={"content": f"Isolated result from {config.name}"},
error_message=None,
started_at=None,
completed_at=None,
)
)
# U3: orchestrator calls agent.execute_stream() (async gen)
m.execute_stream = make_execute_stream_mock(f"Isolated result from {config.name}")
return m
pool.create_agent = AsyncMock(side_effect=_create_agent)
pool.remove_agent = AsyncMock()
return pool
@ -220,9 +226,24 @@ class TestPipelineExecution:
# LLM 分解为 3 个串行阶段
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "阶段B", "depends_on": ["A"]},
{"name": "C", "assigned_expert": "member2", "task_description": "阶段C", "depends_on": ["B"]},
{
"name": "A",
"assigned_expert": "lead",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "阶段B",
"depends_on": ["A"],
},
{
"name": "C",
"assigned_expert": "member2",
"task_description": "阶段C",
"depends_on": ["B"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -231,10 +252,12 @@ class TestPipelineExecution:
execution_order: list[str] = []
for name in ["lead", "member1", "member2"]:
original_stream = team._experts[name].agent.execute_stream
async def tracking_stream(task_msg, _orig=original_stream, _name=name):
execution_order.append(_name)
async for ev in _orig(task_msg):
yield ev
team._experts[name].agent.execute_stream = tracking_stream
result = await orchestrator.execute("串行任务")
@ -256,9 +279,24 @@ class TestPipelineExecution:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "阶段B", "depends_on": []},
{"name": "C", "assigned_expert": "member2", "task_description": "阶段C", "depends_on": ["A", "B"]},
{
"name": "A",
"assigned_expert": "lead",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "阶段B",
"depends_on": [],
},
{
"name": "C",
"assigned_expert": "member2",
"task_description": "阶段C",
"depends_on": ["A", "B"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -310,6 +348,59 @@ class TestPipelineExecution:
event_types = [c[0][1]["type"] for c in calls]
assert "team_synthesis" in event_types
@pytest.mark.asyncio
async def test_synthesis_failure_emits_terminal_team_synthesis_error(self):
"""U3/R4-F2: 流式综合异常时发 team_synthesis 终结事件(status=error),避免前端孤儿 milestone 永久转圈。
行为契约
- 内层 except 捕获 synthesis 异常 广播 team_synthesis(status=error, synthesis_id) re-raise
- 外层 except 捕获 调用 _fallback_to_single_agent re-raise
- 因此 execute() 返回 status="fallback"且最后一个 team_synthesis 事件 status=error
"""
team = _make_team_with_experts()
orchestrator = TeamOrchestrator(team)
# 让 _synthesize_results 抛异常 — 触发 error 终结事件路径
async def _fail_synthesize(*args, **kwargs):
raise RuntimeError("synthesis boom")
orchestrator._synthesize_results = _fail_synthesize
# 不应抛出 — 外层 except 捕获后 fallback
result = await orchestrator.execute("测试任务")
assert result["status"] == "fallback"
calls = team._handoff_transport.send.call_args_list
synth_events = [c for c in calls if c[0][1].get("type") == "team_synthesis"]
# 必须有终结事件,且 status=error
assert len(synth_events) >= 1
terminal = synth_events[-1]
assert terminal[0][1].get("status") == "error"
assert terminal[0][1].get("synthesis_id") is not None
assert "synthesis boom" in terminal[0][1].get("error", "")
@pytest.mark.asyncio
async def test_synthesis_cancelled_emits_terminal_team_synthesis_cancelled(self):
"""U3/R4-F2: 流式综合被 CancelledError 中断时发 team_synthesis 终结事件(status=cancelled)。"""
team = _make_team_with_experts()
orchestrator = TeamOrchestrator(team)
# 让 _synthesize_results 抛 CancelledError — 触发 cancelled 终结事件路径
async def _cancel_synthesize(*args, **kwargs):
raise asyncio.CancelledError()
orchestrator._synthesize_results = _cancel_synthesize
with pytest.raises(asyncio.CancelledError):
await orchestrator.execute("测试任务")
calls = team._handoff_transport.send.call_args_list
synth_events = [c for c in calls if c[0][1].get("type") == "team_synthesis"]
assert len(synth_events) >= 1
terminal = synth_events[-1]
assert terminal[0][1].get("status") == "cancelled"
assert terminal[0][1].get("synthesis_id") is not None
# ── 阶段事件广播测试 ──────────────────────────────────────
@ -351,8 +442,18 @@ class TestPhaseEvents:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "阶段B", "depends_on": ["A"]},
{
"name": "A",
"assigned_expert": "lead",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "阶段B",
"depends_on": ["A"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -403,9 +504,24 @@ class TestTaskDecomposition:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "规划", "assigned_expert": "lead", "task_description": "设计架构", "depends_on": []},
{"name": "前端", "assigned_expert": "member1", "task_description": "实现UI", "depends_on": ["规划"]},
{"name": "后端", "assigned_expert": "member2", "task_description": "实现API", "depends_on": ["规划"]},
{
"name": "规划",
"assigned_expert": "lead",
"task_description": "设计架构",
"depends_on": [],
},
{
"name": "前端",
"assigned_expert": "member1",
"task_description": "实现UI",
"depends_on": ["规划"],
},
{
"name": "后端",
"assigned_expert": "member2",
"task_description": "实现API",
"depends_on": ["规划"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -456,10 +572,22 @@ class TestTaskDecomposition:
@pytest.mark.asyncio
async def test_parse_phases_valid_json(self):
"""_parse_phases 正确解析 JSON 数组"""
content = json.dumps([
{"name": "A", "assigned_expert": "member1", "task_description": "任务A", "depends_on": []},
{"name": "B", "assigned_expert": "member2", "task_description": "任务B", "depends_on": ["A"]},
])
content = json.dumps(
[
{
"name": "A",
"assigned_expert": "member1",
"task_description": "任务A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member2",
"task_description": "任务B",
"depends_on": ["A"],
},
]
)
phases = TeamOrchestrator._parse_phases(content, ["member1", "member2"], "lead")
assert len(phases) == 2
assert phases[0].name == "A"
@ -473,9 +601,16 @@ class TestTaskDecomposition:
@pytest.mark.asyncio
async def test_parse_phases_invalid_expert_falls_back_to_lead(self):
"""_parse_phases 对无效专家名回退到 lead"""
content = json.dumps([
{"name": "A", "assigned_expert": "nonexistent", "task_description": "任务A", "depends_on": []},
])
content = json.dumps(
[
{
"name": "A",
"assigned_expert": "nonexistent",
"task_description": "任务A",
"depends_on": [],
},
]
)
phases = TeamOrchestrator._parse_phases(content, ["member1"], "lead")
assert len(phases) == 1
assert phases[0].assigned_expert == "lead"
@ -489,10 +624,22 @@ class TestTaskDecomposition:
@pytest.mark.asyncio
async def test_parse_phases_empty_name_skipped(self):
"""_parse_phases 跳过空名称的阶段"""
content = json.dumps([
{"name": "", "assigned_expert": "member1", "task_description": "任务A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "任务B", "depends_on": []},
])
content = json.dumps(
[
{
"name": "",
"assigned_expert": "member1",
"task_description": "任务A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "任务B",
"depends_on": [],
},
]
)
phases = TeamOrchestrator._parse_phases(content, ["member1"], "lead")
assert len(phases) == 1
assert phases[0].name == "B"
@ -500,10 +647,22 @@ class TestTaskDecomposition:
@pytest.mark.asyncio
async def test_parse_phases_resolves_depends_on_by_name(self):
"""_parse_phases 通过名称解析 depends_on 为 ID"""
content = json.dumps([
{"name": "规划", "assigned_expert": "lead", "task_description": "设计", "depends_on": []},
{"name": "实现", "assigned_expert": "member1", "task_description": "编码", "depends_on": ["规划"]},
])
content = json.dumps(
[
{
"name": "规划",
"assigned_expert": "lead",
"task_description": "设计",
"depends_on": [],
},
{
"name": "实现",
"assigned_expert": "member1",
"task_description": "编码",
"depends_on": ["规划"],
},
]
)
phases = TeamOrchestrator._parse_phases(content, ["lead", "member1"], "lead")
assert len(phases) == 2
# 实现 depends on 规划
@ -512,9 +671,16 @@ class TestTaskDecomposition:
@pytest.mark.asyncio
async def test_parse_phases_unknown_dependency_ignored(self):
"""_parse_phases 对未知依赖名称忽略"""
content = json.dumps([
{"name": "A", "assigned_expert": "lead", "task_description": "任务A", "depends_on": ["unknown_phase"]},
])
content = json.dumps(
[
{
"name": "A",
"assigned_expert": "lead",
"task_description": "任务A",
"depends_on": ["unknown_phase"],
},
]
)
phases = TeamOrchestrator._parse_phases(content, ["lead"], "lead")
assert len(phases) == 1
assert len(phases[0].depends_on) == 0 # unknown dependency ignored
@ -558,7 +724,12 @@ class TestPhaseExecution:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "nonexistent", "task_description": "任务A", "depends_on": []},
{
"name": "A",
"assigned_expert": "nonexistent",
"task_description": "任务A",
"depends_on": [],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -578,8 +749,18 @@ class TestPhaseExecution:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "阶段B", "depends_on": ["A"]},
{
"name": "A",
"assigned_expert": "lead",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "阶段B",
"depends_on": ["A"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -639,9 +820,24 @@ class TestPhaseFailure:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "阶段B", "depends_on": ["A"]},
{"name": "C", "assigned_expert": "member2", "task_description": "阶段C", "depends_on": ["B"]},
{
"name": "A",
"assigned_expert": "lead",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "阶段B",
"depends_on": ["A"],
},
{
"name": "C",
"assigned_expert": "member2",
"task_description": "阶段C",
"depends_on": ["B"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
@ -787,8 +983,18 @@ class TestResultSynthesis:
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "member1", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member2", "task_description": "阶段B", "depends_on": []},
{
"name": "A",
"assigned_expert": "member1",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member2",
"task_description": "阶段B",
"depends_on": [],
},
],
synthesis_content="综合结果",
)
@ -810,10 +1016,22 @@ class TestResultSynthesis:
gateway = AsyncMock()
decomp_response = MagicMock()
decomp_response.content = json.dumps([
{"name": "A", "assigned_expert": "member1", "task_description": "阶段A", "depends_on": []},
{"name": "B", "assigned_expert": "member2", "task_description": "阶段B", "depends_on": []},
])
decomp_response.content = json.dumps(
[
{
"name": "A",
"assigned_expert": "member1",
"task_description": "阶段A",
"depends_on": [],
},
{
"name": "B",
"assigned_expert": "member2",
"task_description": "阶段B",
"depends_on": [],
},
]
)
# ponytail: 函数式 side_effect — 首次返回 decomposition后续一律抛 RuntimeError
# (列表式 side_effect 耗尽会抛 StopIteration被 U3 收窄后的 except 漏捕获;
# 函数式让"LLM 不可用"语义明确,覆盖验收+综合所有后续调用)
@ -826,12 +1044,14 @@ class TestResultSynthesis:
raise RuntimeError("LLM unavailable")
gateway.chat = AsyncMock(side_effect=chat_side_effect)
# U3: synthesizer tries chat_stream first (broadcast_callback is set).
# Make it raise so synthesizer catches and falls back to concatenation,
# matching the test intent ("without LLM").
async def _stream_raise(*a, **kw):
raise RuntimeError("LLM unavailable")
yield # async gen marker
gateway.chat_stream = MagicMock(side_effect=_stream_raise)
team._experts["lead"].agent._llm_gateway = gateway
@ -1049,11 +1269,13 @@ class TestConcurrencyLimit:
team = _make_team_with_experts(expert_names=["lead", "m1", "m2"])
orchestrator = TeamOrchestrator(team)
gateway = _make_mock_llm_gateway(phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "m1", "task_description": "B", "depends_on": []},
{"name": "C", "assigned_expert": "m2", "task_description": "C", "depends_on": []},
])
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "m1", "task_description": "B", "depends_on": []},
{"name": "C", "assigned_expert": "m2", "task_description": "C", "depends_on": []},
]
)
team._experts["lead"].agent._llm_gateway = gateway
tracker = _ConcurrencyTracker()
@ -1082,10 +1304,17 @@ class TestConcurrencyLimit:
team = _make_team_with_experts(expert_names=names)
orchestrator = TeamOrchestrator(team)
gateway = _make_mock_llm_gateway(phases=[
{"name": f"P{i}", "assigned_expert": name, "task_description": f"P{i}", "depends_on": []}
for i, name in enumerate(names)
])
gateway = _make_mock_llm_gateway(
phases=[
{
"name": f"P{i}",
"assigned_expert": name,
"task_description": f"P{i}",
"depends_on": [],
}
for i, name in enumerate(names)
]
)
team._experts["lead"].agent._llm_gateway = gateway
tracker = _ConcurrencyTracker()
@ -1113,11 +1342,13 @@ class TestConcurrencyLimit:
team = _make_team_with_experts(expert_names=["lead", "m1", "m2"])
orchestrator = TeamOrchestrator(team, max_concurrent_phases=1)
gateway = _make_mock_llm_gateway(phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "m1", "task_description": "B", "depends_on": []},
{"name": "C", "assigned_expert": "m2", "task_description": "C", "depends_on": []},
])
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "m1", "task_description": "B", "depends_on": []},
{"name": "C", "assigned_expert": "m2", "task_description": "C", "depends_on": []},
]
)
team._experts["lead"].agent._llm_gateway = gateway
tracker = _ConcurrencyTracker()
@ -1144,11 +1375,13 @@ class TestConcurrencyLimit:
team = _make_team_with_experts(expert_names=["lead", "m1", "m2"])
orchestrator = TeamOrchestrator(team)
gateway = _make_mock_llm_gateway(phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "m1", "task_description": "B", "depends_on": []},
{"name": "C", "assigned_expert": "m2", "task_description": "C", "depends_on": []},
])
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "m1", "task_description": "B", "depends_on": []},
{"name": "C", "assigned_expert": "m2", "task_description": "C", "depends_on": []},
]
)
team._experts["lead"].agent._llm_gateway = gateway
tracker = _ConcurrencyTracker()
@ -1210,11 +1443,13 @@ class TestConcurrencyLimit:
orchestrator._generate_debate_opening = AsyncMock(return_value="Opening statement")
orchestrator._generate_debate_argument = tracking_debate_argument
orchestrator._generate_debate_summary = AsyncMock(return_value="Round summary")
orchestrator._generate_debate_verdict = AsyncMock(return_value={
"conclusion": "Final conclusion",
"decision": "adopt",
"rationale": "Because",
})
orchestrator._generate_debate_verdict = AsyncMock(
return_value={
"conclusion": "Final conclusion",
"decision": "adopt",
"rationale": "Because",
}
)
await orchestrator._execute_debate_phase(phase, plan)
@ -1345,10 +1580,17 @@ class TestSharedWorkspaceRedis:
team = _make_team_with_experts()
orchestrator = TeamOrchestrator(team)
gateway = _make_mock_llm_gateway(phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{"name": "B", "assigned_expert": "member1", "task_description": "B", "depends_on": ["A"]},
])
gateway = _make_mock_llm_gateway(
phases=[
{"name": "A", "assigned_expert": "lead", "task_description": "A", "depends_on": []},
{
"name": "B",
"assigned_expert": "member1",
"task_description": "B",
"depends_on": ["A"],
},
]
)
team._experts["lead"].agent._llm_gateway = gateway
# Mock _execute_phase to return large content + verify offloading