diff --git a/.claude/skills/approval-module/SKILL.md b/.claude/skills/approval-module/SKILL.md index 3313b0412..af4cd4bd8 100644 --- a/.claude/skills/approval-module/SKILL.md +++ b/.claude/skills/approval-module/SKILL.md @@ -134,6 +134,7 @@ ApprovalCenterService.decide_task() - **入口**：`channel/domain/services/channel_service.py::subscribe_channel()`（`REVIEW` 可见性频道） - **Handler**：`ChannelSubscribeScenarioHandler` - 通过 / pass 路径调 `ChannelService.sync_direct_channel_user_permissions()` 写 ReBAC(OpenFGA) 关系（否则成员不出现在 ReBAC 成员列表） +- `on_approved` 先把申请人的 **PENDING** membership 翻成 ACTIVE 再写 ReBAC；查 membership 必须用 `include_inactive=True`（CHANNEL 默认只查 ACTIVE），缺失时直接 raise。详见 [§11 调试指南](#11-调试指南) 的已知坑 - PENDING 时调 `_send_channel_approval_notification()` 通知审批人 ### 4.3 知识空间加入审批 (`knowledge_space_subscribe_request`) @@ -314,6 +315,18 @@ SELECT id, exception_type, status, detail FROM approval_exception WHERE instance ### "频道/知识空间审批通过但成员列表看不到" 检查对应 `sync_direct_channel_user_permissions` / `sync_direct_space_user_permissions` 是否在该激活路径被调用（写 ReBAC/OpenFGA 关系）。 +**已知坑（频道场景，2026-06 修复）**：`on_approved` 第一步 `_get_membership` 要找到那条 **PENDING** 的 membership 才能翻成 ACTIVE。但 `SpaceChannelMemberRepositoryImpl.find_membership` 对 `business_type=CHANNEL` 默认**只查 ACTIVE**，导致查不到 PENDING → 返回 None。激活路径（`approval_runtime_handler_factory._AsyncSpaceChannelMembershipAdapter.find_membership`、旧 `channel_subscribe_approval_handler._get_membership`）**必须传 `include_inactive=True`**。知识空间场景用的是 `SpaceChannelMemberDao.async_find_member`（不过滤状态），所以不受影响——这也是"知识空间正常、频道异常"的原因。 + +> 该 bug 还会被一个静默分支掩盖：旧版 `on_approved` 在 membership 缺失时 `return {'status':'missing_membership'}` 不抛异常 → outbox 仍标 success、instance 仍 executed，但 membership 永远停在 PENDING、ReBAC 从未写。现已改为 **raise**，让 outbox 进 FAILED + `execute_failed` 异常暴露问题。排查时若看到 instance=executed 但 `space_channel_member.status=PENDING`，就是这个老数据。 + +排查 SQL： +```sql +SELECT i.id, i.status, m.id, m.status +FROM approval_instance i JOIN space_channel_member m + ON m.business_id=i.business_resource_id AND m.business_type='CHANNEL' AND m.user_id=i.applicant_user_id +WHERE i.scenario_code='channel_subscribe_request' AND i.status IN ('executed','approved') AND m.status='PENDING'; +``` + --- ## 12. 测试 diff --git a/scripts/repair_channel_subscribe_memberships.py b/scripts/repair_channel_subscribe_memberships.py new file mode 100644 index 000000000..810e8f9b6 --- /dev/null +++ b/scripts/repair_channel_subscribe_memberships.py @@ -0,0 +1,63 @@ +"""One-off compensation for channel-subscribe approvals stuck by the +find_membership ACTIVE-only filter bug. + +For each affected instance the approval was marked EXECUTED but the applicant's +membership stayed PENDING and no ReBAC grant was written, so the channel never +appeared in the user's subscription list. This re-runs the (now fixed) +ChannelSubscribeScenarioHandler.on_approved() to flip the membership to ACTIVE +and write the OpenFGA relation. + +Idempotent: re-running on an already-ACTIVE membership is a no-op upsert. + +Run from src/backend with the same config the backend uses: + config=config.yaml uv run python ../../scripts/repair_channel_subscribe_memberships.py +""" + +from __future__ import annotations + +import asyncio +import logging + +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger("repair_channel_subscribe") + +# Affected instance ids discovered via the §11 diagnostic query. +INSTANCE_IDS = [248, 250, 251, 252] +TENANT_ID = 1 + + +async def main() -> None: + from bisheng.approval.domain.repositories.approval_instance_repository import ApprovalInstanceRepository + from bisheng.approval.domain.services.approval_runtime_handler_factory import build_runtime_handler + from bisheng.common.services.config_service import settings + from bisheng.core.context import close_app_context, initialize_app_context + from bisheng.core.context.tenant import set_current_tenant_id + + await initialize_app_context(config=settings) + set_current_tenant_id(TENANT_ID) + + try: + for instance_id in INSTANCE_IDS: + instance = await ApprovalInstanceRepository.get_instance(instance_id) + if instance is None: + logger.warning("instance %s not found, skipping", instance_id) + continue + if instance.scenario_code != "channel_subscribe_request": + logger.warning( + "instance %s is %s, not channel_subscribe_request, skipping", + instance_id, instance.scenario_code, + ) + continue + payload = instance.payload_snapshot or {} + handler = await build_runtime_handler(instance.scenario_code) + result = await handler.on_approved(instance_id, payload) + logger.info( + "repaired instance=%s channel_id=%s applicant=%s -> %s", + instance_id, payload.get("channel_id"), payload.get("applicant_user_id"), result, + ) + finally: + await close_app_context() + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/src/backend/bisheng/approval/domain/services/approval_runtime_handler_factory.py b/src/backend/bisheng/approval/domain/services/approval_runtime_handler_factory.py index 126920cc6..70d187770 100644 --- a/src/backend/bisheng/approval/domain/services/approval_runtime_handler_factory.py +++ b/src/backend/bisheng/approval/domain/services/approval_runtime_handler_factory.py @@ -35,10 +35,12 @@ class _AsyncSpaceChannelMembershipAdapter: async def find_membership(self, business_id: str, business_type: BusinessTypeEnum, user_id: int): async with get_async_db_session() as session: repository = SpaceChannelMemberRepositoryImpl(session) + # Activation must locate the PENDING membership (not just ACTIVE) to flip it to ACTIVE. return await repository.find_membership( business_id=business_id, business_type=business_type, user_id=user_id, + include_inactive=True, ) async def update(self, membership): diff --git a/src/backend/bisheng/approval/domain/services/channel_subscribe_scenario_handler.py b/src/backend/bisheng/approval/domain/services/channel_subscribe_scenario_handler.py index 71f4cd168..1b2e18670 100644 --- a/src/backend/bisheng/approval/domain/services/channel_subscribe_scenario_handler.py +++ b/src/backend/bisheng/approval/domain/services/channel_subscribe_scenario_handler.py @@ -83,8 +83,15 @@ class ChannelSubscribeScenarioHandler: async def on_approved(self, instance_id: int, payload_snapshot: dict) -> dict: membership = await self._get_membership(payload_snapshot) if not membership: - logger.warning('Channel membership not found when approval instance=%s approved', instance_id) - return {'status': 'missing_membership'} + # Fail loudly: the outbox executor turns this into a FAILED outbox + + # execute_failed exception. Returning a benign dict here would mark the + # instance EXECUTED while the membership stays PENDING and ReBAC is never + # written, so the channel never appears in the user's subscription list. + raise RuntimeError( + f'Channel membership not found for approval instance={instance_id}, ' + f"channel_id={payload_snapshot.get('channel_id')}, " + f"applicant_user_id={payload_snapshot.get('applicant_user_id')}" + ) membership.status = MembershipStatusEnum.ACTIVE await self.space_channel_member_repository.update(membership) if self.sync_permissions: diff --git a/src/backend/bisheng/channel/domain/services/channel_subscribe_approval_handler.py b/src/backend/bisheng/channel/domain/services/channel_subscribe_approval_handler.py index d1ab45451..fd1a6cc5b 100644 --- a/src/backend/bisheng/channel/domain/services/channel_subscribe_approval_handler.py +++ b/src/backend/bisheng/channel/domain/services/channel_subscribe_approval_handler.py @@ -130,9 +130,14 @@ class ChannelSubscribeApprovalHandler(ApprovalHandler): ) async def _get_membership(self, channel_id: str, applicant_user_id: int): - """Load the applicant membership for the target channel.""" + """Load the applicant membership for the target channel. + + Includes inactive rows: the applicant's membership is PENDING while awaiting + approval, and this handler must find it to flip it to ACTIVE/REJECTED. + """ return await self.space_channel_member_repository.find_membership( business_id=channel_id, business_type=BusinessTypeEnum.CHANNEL, user_id=applicant_user_id, + include_inactive=True, ) diff --git a/src/backend/bisheng/common/repositories/implementations/space_channel_member_repository_impl.py b/src/backend/bisheng/common/repositories/implementations/space_channel_member_repository_impl.py index 2d7a87905..c8bbfa59a 100644 --- a/src/backend/bisheng/common/repositories/implementations/space_channel_member_repository_impl.py +++ b/src/backend/bisheng/common/repositories/implementations/space_channel_member_repository_impl.py @@ -104,14 +104,19 @@ class SpaceChannelMemberRepositoryImpl(BaseRepositoryImpl[SpaceChannelMember, in return list(highest_by_channel.values()) async def find_membership(self, business_id: str, business_type: BusinessTypeEnum, - user_id: int) -> Optional[SpaceChannelMember]: - """Find a specific membership by business ID, type, and user ID.""" + user_id: int, include_inactive: bool = False) -> Optional[SpaceChannelMember]: + """Find a specific membership by business ID, type, and user ID. + + For channels the default only returns ACTIVE members. Pass + ``include_inactive=True`` to also match PENDING/REJECTED rows — the approval + activation flow needs the PENDING membership in order to flip it to ACTIVE. + """ query = select(SpaceChannelMember).where( SpaceChannelMember.business_id == business_id, SpaceChannelMember.business_type == business_type, SpaceChannelMember.user_id == user_id ) - if business_type == BusinessTypeEnum.CHANNEL: + if business_type == BusinessTypeEnum.CHANNEL and not include_inactive: query = query.where(SpaceChannelMember.status == MembershipStatusEnum.ACTIVE) result = await self.session.exec(query) rows = list(result.all()) diff --git a/src/backend/bisheng/common/repositories/interfaces/space_channel_member_repository.py b/src/backend/bisheng/common/repositories/interfaces/space_channel_member_repository.py index 1879865bf..2660fc77f 100644 --- a/src/backend/bisheng/common/repositories/interfaces/space_channel_member_repository.py +++ b/src/backend/bisheng/common/repositories/interfaces/space_channel_member_repository.py @@ -36,8 +36,14 @@ class SpaceChannelMemberRepository(BaseRepository[SpaceChannelMember, int], ABC) @abstractmethod async def find_membership(self, business_id: str, business_type: BusinessTypeEnum, - user_id: int) -> Optional[SpaceChannelMember]: - """Find a specific membership by business ID, type, and user ID.""" + user_id: int, include_inactive: bool = False) -> Optional[SpaceChannelMember]: + """Find a specific membership by business ID, type, and user ID. + + For channels the default only returns ACTIVE members. Set + ``include_inactive=True`` to also match PENDING/REJECTED rows — required by + the approval activation flow, which must locate the PENDING membership to + flip it to ACTIVE. + """ pass @abstractmethod diff --git a/src/backend/test/approval/test_channel_subscription_approval_integration.py b/src/backend/test/approval/test_channel_subscription_approval_integration.py index 573b190e1..9f2ed8fd1 100644 --- a/src/backend/test/approval/test_channel_subscription_approval_integration.py +++ b/src/backend/test/approval/test_channel_subscription_approval_integration.py @@ -151,3 +151,27 @@ async def test_channel_subscribe_scenario_handler_updates_membership_states(): membership.status = MembershipStatusEnum.PENDING await handler.on_rejected(instance_id=1, payload_snapshot=payload, reason='reject') assert membership.status == MembershipStatusEnum.REJECTED + + +@pytest.mark.asyncio +async def test_channel_subscribe_on_approved_raises_when_membership_missing(): + """A missing membership must fail loudly so the outbox is marked FAILED and an + execute_failed exception is raised — never silently reported as success.""" + from bisheng.approval.domain.services.channel_subscribe_scenario_handler import ( + ChannelSubscribeScenarioHandler, + ) + + member_repository = SimpleNamespace( + find_membership=AsyncMock(return_value=None), + update=AsyncMock(), + ) + sync_permissions = AsyncMock() + handler = ChannelSubscribeScenarioHandler( + space_channel_member_repository=member_repository, + sync_permissions=sync_permissions, + ) + + payload = {'channel_id': 'channel-1', 'applicant_user_id': 427} + with pytest.raises(RuntimeError): + await handler.on_approved(instance_id=99, payload_snapshot=payload) + sync_permissions.assert_not_awaited() diff --git a/src/backend/test/channel/test_channel_membership_repository.py b/src/backend/test/channel/test_channel_membership_repository.py index fc0e2eb41..4a7db92c4 100644 --- a/src/backend/test/channel/test_channel_membership_repository.py +++ b/src/backend/test/channel/test_channel_membership_repository.py @@ -250,3 +250,48 @@ async def test_remove_channel_subscription_members_preserves_authorized_grants(a ).all() assert removed == 2 assert sorted(remaining) == [1, 4, 5] + + +@pytest.mark.asyncio +async def test_find_membership_skips_pending_channel_by_default(async_db_session: AsyncSession): + """Default channel lookup only returns ACTIVE members (existing behavior).""" + repo = SpaceChannelMemberRepositoryImpl(async_db_session) + async_db_session.add( + SpaceChannelMember( + business_id='channel-pending', + business_type=BusinessTypeEnum.CHANNEL, + user_id=427, + user_role=UserRoleEnum.MEMBER, + status=MembershipStatusEnum.PENDING, + relation=ChannelRelationEnum.VIEWER, + ) + ) + await async_db_session.commit() + + found = await repo.find_membership('channel-pending', BusinessTypeEnum.CHANNEL, 427) + + assert found is None + + +@pytest.mark.asyncio +async def test_find_membership_returns_pending_channel_when_include_inactive(async_db_session: AsyncSession): + """Approval activation must locate the PENDING membership to flip it to ACTIVE.""" + repo = SpaceChannelMemberRepositoryImpl(async_db_session) + async_db_session.add( + SpaceChannelMember( + business_id='channel-pending', + business_type=BusinessTypeEnum.CHANNEL, + user_id=427, + user_role=UserRoleEnum.MEMBER, + status=MembershipStatusEnum.PENDING, + relation=ChannelRelationEnum.VIEWER, + ) + ) + await async_db_session.commit() + + found = await repo.find_membership( + 'channel-pending', BusinessTypeEnum.CHANNEL, 427, include_inactive=True + ) + + assert found is not None + assert found.status == MembershipStatusEnum.PENDING