Skip to content

feat(wework): notify cloud task participants via IM - #2531

Closed
qdaxb wants to merge 2 commits into
mainfrom
feature/cloud-board-im-notifications
Closed

qdaxb wants to merge 2 commits into
mainfrom
feature/cloud-board-im-notifications

Conversation

@qdaxb

@qdaxb qdaxb commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

变更说明

  • 新增统一的云看板任务 IM 通知服务,按事件 ID 幂等发送,并为每位相关用户选择最近使用的私聊会话
  • 覆盖任务创建、归档、负责人/状态/优先级/截止时间变更、评论,以及 AI 执行完成或失败
  • 收件人包含操作人、任务创建人、变更前后负责人、协作人,以及机器人负责人的创建者
  • 接入从 Wework 发起的 GitHub/GitLab 任务变化
  • 接入钉钉 AI 表格的用户操作上报,并为 DWS AI 操作提供成功后通知上报工具
  • 补充中英文 IM 集成文档

行为约束

  • 标题、描述、标签和纯排序变化不发送通知
  • 无可用 IM 私聊会话的用户会被跳过,不影响任务写入
  • 通知失败不回滚已提交的任务或外部表格操作
  • 直接在 GitHub、GitLab 或钉钉侧发生的变化暂不通知
  • 钉钉 AI 表格仅可靠通知项目创建人和当前操作人

验证

  • Backend 聚焦测试:73 passed;rebase 后通知相关测试:28 passed
  • Wework 全量单测:336 files / 3231 tests passed
  • Wework typecheck、lint passed(仅有两条与本次无关的既有 Hook warnings)
  • Executor MCP 聚焦测试:17 passed
  • pre-push:Wework ESLint、TypeScript、全量单测,Backend Black/isort/syntax,Executor cargo fmt、cargo test --all-features --lib、clippy,Alembic multi-head 全部通过
  • 真实 Tauri:隔离 Executor 首次编译完成后 reload,工作台成功加载并显示“新建任务”;会话已清理

Summary by CodeRabbit

  • New Features

    • Added instant IM notifications for cloud-task creation, updates, archiving, comments, assignments, and runtime completion or failure.
    • Notifications include relevant task details and concise summaries of status, assignee, priority, and due-date changes.
    • Added DingTalk AI Table notifications for successful external record creation, updates, and deletions.
    • Prevented duplicate notifications and delivered messages through each recipient’s most recently active private chat.
  • Documentation

    • Added English and Chinese guidance covering notification triggers, recipients, supported integrations, and limitations.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds cloud-task IM notifications for loop-item changes, project-chat events, runtime outcomes, and DingTalk AI Table record mutations. It adds backend dispatch infrastructure, external reporting APIs, MCP support, integration wiring, tests, and documentation.

Changes

Cloud task notifications

Layer / File(s) Summary
Notification contracts and dispatch
backend/app/schemas/delivery.py, backend/app/services/im/cloud_task_notifications.py, backend/tests/services/im/*, backend/tests/conftest.py
Adds validated external notification reports, task snapshots, event construction, recipient resolution, message formatting, idempotent dispatch, and change summaries.
Backend event sources
backend/app/api/endpoints/cloud_projects.py, backend/app/api/endpoints/deliveries.py, backend/app/api/ws/*, backend/tests/api/*
Adds notifications for task creation, meaningful updates, assignment changes, archiving, comments, external reports, and runtime completion or failure events.
External AITable reporting
wework/src/api/aitable.ts, wework/src/api/deliveries.ts, wework/src/api/*test.ts, wework/src/api/hybrid/*, wework/src/features/*, executor/src/task_runtime/mcp.rs, wework/src-tauri/tauri.conf.json
Reports successful AITable record mutations through the cloud delivery endpoint and exposes the reporting capability through hybrid services, workbench services, and MCP.
Notification behavior documentation
docs/en/wegent/user-guide/integrations/im-channel-integration.md, docs/zh/wegent/user-guide/integrations/im-channel-integration.md
Documents notification triggers, recipient deduplication, missing sessions, provider behavior, and external-change exclusions.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AITable
  participant withAITableNotifications
  participant DeliveryAPI
  participant CloudProjectEndpoint
  participant CloudTaskNotificationService
  participant IMChat
  AITable->>withAITableNotifications: create, update, or delete record
  withAITableNotifications->>DeliveryAPI: report operation and record summary
  DeliveryAPI->>CloudProjectEndpoint: POST external-task-notifications
  CloudProjectEndpoint->>CloudTaskNotificationService: build and queue event
  CloudTaskNotificationService->>IMChat: dispatch formatted notification
Loading

Possibly related PRs

  • wecode-ai/Wegent#2215: Shares the cloud kanban delivery and loop-item APIs used by these notifications.
  • wecode-ai/Wegent#2251: Shares external project and task-provider routing used by notification delivery.
  • wecode-ai/Wegent#2283: Introduced the DingTalk AI Table integration extended by external-record notifications.

Suggested reviewers: micro66

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.31% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: sending cloud task notifications to participants through IM.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/cloud-board-im-notifications

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qdaxb
qdaxb marked this pull request as ready for review August 12, 2026 16:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (3)
wework/src/api/hybrid/hybridServices.ts (1)

1124-1134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The AITable notification adapter is copied into two service factories. Both factories build the same wrapper and the same camelCase-to-snake_case payload mapping for reportExternalTaskNotification. One payload change must then be applied twice.

  • wework/src/api/hybrid/hybridServices.ts#L1124-L1134: call a shared helper, for example withCloudAITableNotifications(localServices.aitableApi, cloudServices.deliveryApi).
  • wework/src/features/workbench/workbenchServices.ts#L223-L236: call the same shared helper and remove the inline mapping.

As per coding guidelines: "Before adding code, search for and reuse existing components, services, utilities, and patterns; extract shared logic instead of duplicating it."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wework/src/api/hybrid/hybridServices.ts` around lines 1124 - 1134, The
AITable notification adapter is duplicated across both service factories.
Extract a shared helper that wraps the local AITable API and maps events for
cloud delivery, then update hybridServices.ts lines 1124-1134 and
workbenchServices.ts lines 223-236 to call it with localServices.aitableApi and
cloudServices.deliveryApi, removing both inline wrappers and payload mappings.

Source: Coding guidelines

backend/app/api/endpoints/cloud_projects.py (1)

220-249: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the shared notification helper instead of repeating the schedule block.

backend/app/api/endpoints/deliveries.py defines _schedule_task_notification for exactly this pattern. This file repeats the event(...) plus background_tasks.add_task(...) sequence three times. Move the helper into a shared module and call it from both endpoint modules.

This keeps one dispatch entry point and reduces the chance that one call site drifts.

As per coding guidelines: "Before adding code, search for and reuse existing components, services, utilities, and patterns; extract shared logic instead of duplicating it."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/api/endpoints/cloud_projects.py` around lines 220 - 249, Extract
the duplicated task-notification event creation and background dispatch logic
from cloud_projects.py and deliveries.py into a shared helper module, preserving
the existing _schedule_task_notification behavior and parameters. Replace all
three repeated event/background_tasks.add_task sequences with calls to that
helper, including the flow around cloud_task_notification_service.event and
dispatch.

Source: Coding guidelines

backend/tests/services/im/test_cloud_task_notifications.py (1)

31-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add type annotations to the notification test doubles.

Annotate fake_im_session_cache, both fake_send_text callbacks, both fake_dispatch callbacks, and the events lists with their concrete event, session, database, and return types.

  • backend/tests/services/im/test_cloud_task_notifications.py:32,41,76,82
  • backend/tests/api/test_cloud_projects_api.py:97,99
  • backend/tests/api/test_deliveries_api.py:152,154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/services/im/test_cloud_task_notifications.py` around lines 31 -
43, Annotate the specified test doubles and event collections with concrete
types: in backend/tests/services/im/test_cloud_task_notifications.py:31-43, type
fake_im_session_cache and the fake_send_text and fake_dispatch callbacks,
including their event, session, database, and return types; apply the same
callback and events-list annotations in
backend/tests/api/test_cloud_projects_api.py:82-101 and
backend/tests/api/test_deliveries_api.py:146-156. Use the project’s existing
event, session, database, and result types.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/app/api/endpoints/cloud_projects.py`:
- Around line 232-241: Update both notification branches around
cloud_task_notification_service.change_summary and the corresponding event calls
to store its computed non-empty result and pass that summary to event instead of
the fixed "负责人已变更" text. Apply the same change to both loop_item_service.assign
and external_loop_item_provider.assign paths, preserving the existing gating and
dispatch behavior.

In `@backend/app/api/ws/device_namespace.py`:
- Around line 1997-2000: Replace the inline await of
cloud_task_notification_service.dispatch in
backend/app/api/ws/device_namespace.py lines 1997-2000 and 2092-2095 with
self._schedule_background_task, preserving projected message emission without
blocking the WebSocket handlers. In
backend/app/api/ws/wework_runtime_namespace.py lines 242-244, schedule dispatch
with a retained asyncio.create_task and an error-logging done callback so the
ACK does not await IM delivery.

In `@backend/app/services/im/cloud_task_notifications.py`:
- Around line 157-197: Update dispatch so that when its notification-processing
try block raises and no messages were delivered (sent == 0), it deletes the
event’s idempotency claim key before returning, allowing retries for the same
event_id. Preserve the existing exception logging, successful-delivery claims,
and normal result handling; use the same key format and cache_manager used by
the initial setnx claim.

In `@docs/en/wegent/user-guide/integrations/im-channel-integration.md`:
- Around line 294-297: Update both documented recipient-behavior sections: in
docs/en/wegent/user-guide/integrations/im-channel-integration.md lines 294-297
and docs/zh/wegent/user-guide/integrations/im-channel-integration.md lines
293-294, explicitly state that notification delivery failures do not affect the
underlying task or external-table operation.

In `@docs/zh/wegent/user-guide/integrations/im-channel-integration.md`:
- Around line 292-293:
将通知说明中的“变更前后的人员负责人”改为明确且一致的角色术语“变更前后的人工负责人”,并保持其余相关人员范围及通知规则不变。

In `@wework/src-tauri/tauri.conf.json`:
- Line 41: Restrict the asset protocol configuration around the allow rule by
removing the global "**/*" scope, keeping requireLiteralLeadingDot enabled, and
adding deny rules for sensitive paths. Replace csp: null with a restrictive
policy permitting only the asset sources required by the application, and handle
user-selected paths through runtime or persisted scope grants rather than global
access.

In `@wework/src/api/aitable.test.ts`:
- Around line 66-72: Update the test for withAITableNotifications around
notifying.createRecord to assert that the rejected report function is called
exactly once after the mutation resolves, and verify the call’s event type
matches the committed create mutation. Preserve the existing assertion that the
mutation resolves successfully despite the reporting failure.

In `@wework/src/api/aitable.ts`:
- Around line 202-208: Update deleteRecord so the optional api.getRecord lookup
is best-effort: catch or otherwise neutralize its rejection and fall back to the
existing placeholder record, then always proceed to api.deleteRecord. Preserve
the subsequent notify call and existing behavior when the lookup succeeds.

---

Nitpick comments:
In `@backend/app/api/endpoints/cloud_projects.py`:
- Around line 220-249: Extract the duplicated task-notification event creation
and background dispatch logic from cloud_projects.py and deliveries.py into a
shared helper module, preserving the existing _schedule_task_notification
behavior and parameters. Replace all three repeated
event/background_tasks.add_task sequences with calls to that helper, including
the flow around cloud_task_notification_service.event and dispatch.

In `@backend/tests/services/im/test_cloud_task_notifications.py`:
- Around line 31-43: Annotate the specified test doubles and event collections
with concrete types: in
backend/tests/services/im/test_cloud_task_notifications.py:31-43, type
fake_im_session_cache and the fake_send_text and fake_dispatch callbacks,
including their event, session, database, and return types; apply the same
callback and events-list annotations in
backend/tests/api/test_cloud_projects_api.py:82-101 and
backend/tests/api/test_deliveries_api.py:146-156. Use the project’s existing
event, session, database, and result types.

In `@wework/src/api/hybrid/hybridServices.ts`:
- Around line 1124-1134: The AITable notification adapter is duplicated across
both service factories. Extract a shared helper that wraps the local AITable API
and maps events for cloud delivery, then update hybridServices.ts lines
1124-1134 and workbenchServices.ts lines 223-236 to call it with
localServices.aitableApi and cloudServices.deliveryApi, removing both inline
wrappers and payload mappings.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c5a31e83-e524-4797-b745-5aae2b1216aa

📥 Commits

Reviewing files that changed from the base of the PR and between e9e33b1 and 360a75a.

📒 Files selected for processing (21)
  • backend/app/api/endpoints/cloud_projects.py
  • backend/app/api/endpoints/deliveries.py
  • backend/app/api/ws/device_namespace.py
  • backend/app/api/ws/wework_runtime_namespace.py
  • backend/app/schemas/delivery.py
  • backend/app/services/im/cloud_task_notifications.py
  • backend/tests/api/test_cloud_projects_api.py
  • backend/tests/api/test_deliveries_api.py
  • backend/tests/conftest.py
  • backend/tests/services/im/test_cloud_task_notifications.py
  • docs/en/wegent/user-guide/integrations/im-channel-integration.md
  • docs/zh/wegent/user-guide/integrations/im-channel-integration.md
  • executor/src/task_runtime/mcp.rs
  • wework/src-tauri/tauri.conf.json
  • wework/src/api/aitable.test.ts
  • wework/src/api/aitable.ts
  • wework/src/api/deliveries.test.ts
  • wework/src/api/deliveries.ts
  • wework/src/api/hybrid/hybridServices.ts
  • wework/src/features/todo/projectProviderConfig.ts
  • wework/src/features/workbench/workbenchServices.ts

Comment on lines +232 to +241
if cloud_task_notification_service.change_summary(before, after):
event = cloud_task_notification_service.event(
event_id=f"task:{item_id}:assigned:{response['version']}",
event_type="updated",
actor=current_user,
before=before,
after=after,
summary="负责人已变更",
)
background_tasks.add_task(cloud_task_notification_service.dispatch, event)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Send the computed change summary instead of a fixed assignee string.

Both branches gate on change_summary(before, after) being non-empty. change_summary returns a non-empty string for a status, priority, or due-date change as well. loop_item_service.assign and external_loop_item_provider.assign can change the task status or queue state without changing the assignee. In that case the notification states 负责人已变更 while the assignee did not change.

Pass the computed summary to the event.

♻️ Proposed fix for both branches
-        if cloud_task_notification_service.change_summary(before, after):
+        summary = cloud_task_notification_service.change_summary(before, after)
+        if summary:
             event = cloud_task_notification_service.event(
                 event_id=f"task:{item_id}:assigned:{response['version']}",
                 event_type="updated",
                 actor=current_user,
                 before=before,
                 after=after,
-                summary="负责人已变更",
+                summary=summary,
             )
             background_tasks.add_task(cloud_task_notification_service.dispatch, event)
-    if cloud_task_notification_service.change_summary(before, after):
+    summary = cloud_task_notification_service.change_summary(before, after)
+    if summary:
         event = cloud_task_notification_service.event(
             event_id=f"task:{item_id}:assigned:{item.version}",
             event_type="updated",
             actor=current_user,
             before=before,
             after=after,
-            summary="负责人已变更",
+            summary=summary,
         )
         background_tasks.add_task(cloud_task_notification_service.dispatch, event)

Also applies to: 261-270

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/api/endpoints/cloud_projects.py` around lines 232 - 241, Update
both notification branches around cloud_task_notification_service.change_summary
and the corresponding event calls to store its computed non-empty result and
pass that summary to event instead of the fixed "负责人已变更" text. Apply the same
change to both loop_item_service.assign and external_loop_item_provider.assign
paths, preserving the existing gating and dispatch behavior.

Comment on lines 1997 to +2000
if projected:
cloud_notification = projected.get("notification")
if cloud_notification is not None:
await cloud_task_notification_service.dispatch(cloud_notification)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

IM dispatch is awaited inline on WebSocket handler paths. Every HTTP endpoint schedules cloud_task_notification_service.dispatch through BackgroundTasks, but the three WebSocket sites await it directly. Each await holds the handler while a database session opens and IM messages are sent to all recipients, and cache_manager.setnx errors inside dispatch propagate into the handler and stop the projected message or the ACK.

  • backend/app/api/ws/device_namespace.py#L1997-L2000: replace the await with self._schedule_background_task(cloud_task_notification_service.dispatch(cloud_notification), ...) so the projected chat message still emits.
  • backend/app/api/ws/device_namespace.py#L2092-L2095: apply the same self._schedule_background_task call before the projected runtime message emission.
  • backend/app/api/ws/wework_runtime_namespace.py#L242-L244: dispatch through a retained asyncio.create_task with an error-logging done callback so the chat send ACK does not wait for IM delivery.
📍 Affects 2 files
  • backend/app/api/ws/device_namespace.py#L1997-L2000 (this comment)
  • backend/app/api/ws/device_namespace.py#L2092-L2095
  • backend/app/api/ws/wework_runtime_namespace.py#L242-L244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/api/ws/device_namespace.py` around lines 1997 - 2000, Replace the
inline await of cloud_task_notification_service.dispatch in
backend/app/api/ws/device_namespace.py lines 1997-2000 and 2092-2095 with
self._schedule_background_task, preserving projected message emission without
blocking the WebSocket handlers. In
backend/app/api/ws/wework_runtime_namespace.py lines 242-244, schedule dispatch
with a retained asyncio.create_task and an error-logging done callback so the
ACK does not await IM delivery.

Comment on lines +157 to +197
claimed = await cache_manager.setnx(
f"{IDEMPOTENCY_PREFIX}{event.event_id}",
{"created_at": datetime.now().isoformat()},
expire=IDEMPOTENCY_TTL_SECONDS,
)
if not claimed:
return {"sent": 0, "results": [], "skipped": "duplicate_or_cache_error"}

recipients = self._recipient_ids(event)
message = self._message(event)
sent = 0
results: list[dict[str, Any]] = []
db = SessionLocal()
try:
for user_id in recipients:
sessions = await im_session_service.list_user_sessions(
db, user_id=user_id
)
if not sessions:
results.append(
{"success": False, "user_id": user_id, "error": "No IM session"}
)
continue
session = max(sessions, key=lambda item: item.last_seen_at)
result = await im_notification_dispatcher.send_text(
db, session, message
)
result.setdefault("user_id", user_id)
result.setdefault("session_key", session.session_key)
results.append(result)
if result.get("success"):
sent += 1
except Exception:
logger.exception(
"[CloudTaskNotification] Dispatch failed: event_id=%s event_type=%s",
event.event_id,
event.event_type,
)
finally:
db.close()
return {"sent": sent, "results": results}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Release the idempotency claim when no notification is delivered.

dispatch claims the event key before it sends any message. If im_session_service.list_user_sessions or im_notification_dispatcher.send_text raises, the except block logs the failure and the method returns. The key stays in the cache for 7 days. A later retry of the same event_id returns duplicate_or_cache_error and the notification is lost permanently.

Delete the claim key when sent is 0 because of an exception, so a retry can deliver the event.

♻️ Proposed fix
     async def dispatch(self, event: CloudTaskNotificationEvent) -> dict[str, Any]:
         """Deliver one committed event once, selecting each user's latest chat."""
 
+        claim_key = f"{IDEMPOTENCY_PREFIX}{event.event_id}"
         claimed = await cache_manager.setnx(
-            f"{IDEMPOTENCY_PREFIX}{event.event_id}",
+            claim_key,
             {"created_at": datetime.now().isoformat()},
             expire=IDEMPOTENCY_TTL_SECONDS,
         )
         if not claimed:
             return {"sent": 0, "results": [], "skipped": "duplicate_or_cache_error"}
 
         recipients = self._recipient_ids(event)
         message = self._message(event)
         sent = 0
         results: list[dict[str, Any]] = []
         db = SessionLocal()
         try:
             ...
         except Exception:
             logger.exception(
                 "[CloudTaskNotification] Dispatch failed: event_id=%s event_type=%s",
                 event.event_id,
                 event.event_type,
             )
+            if sent == 0:
+                await cache_manager.delete(claim_key)
         finally:
             db.close()
         return {"sent": sent, "results": results}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/im/cloud_task_notifications.py` around lines 157 - 197,
Update dispatch so that when its notification-processing try block raises and no
messages were delivered (sent == 0), it deletes the event’s idempotency claim
key before returning, allowing retries for the same event_id. Preserve the
existing exception logging, successful-delivery claims, and normal result
handling; use the same key format and cache_manager used by the initial setnx
claim.

Comment on lines +294 to +297
Recipients include the actor, task creator, previous and current human
assignees, task collaborators, and the creator of a robot assignee. Each user
receives at most one copy. Users without an available private IM session are
skipped without affecting the task operation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the complete non-blocking notification behavior in both guides.

Both sections mention users without IM sessions but omit that notification delivery failures do not affect the underlying operation.

  • docs/en/wegent/user-guide/integrations/im-channel-integration.md#L294-L297: Add that notification delivery failures do not affect the task or external-table operation.
  • docs/zh/wegent/user-guide/integrations/im-channel-integration.md#L293-L294: Add that notification failures do not affect the task or external-table operation.
📍 Affects 2 files
  • docs/en/wegent/user-guide/integrations/im-channel-integration.md#L294-L297 (this comment)
  • docs/zh/wegent/user-guide/integrations/im-channel-integration.md#L293-L294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/en/wegent/user-guide/integrations/im-channel-integration.md` around
lines 294 - 297, Update both documented recipient-behavior sections: in
docs/en/wegent/user-guide/integrations/im-channel-integration.md lines 294-297
and docs/zh/wegent/user-guide/integrations/im-channel-integration.md lines
293-294, explicitly state that notification delivery failures do not affect the
underlying task or external-table operation.

Comment on lines +292 to +293
相关人员包括操作人、任务创建人、变更前后的人员负责人、任务协作人,以及机器人
负责人的创建者。每位用户只会收到一条通知;如果用户没有可用的 IM 私聊会话,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

将“人员负责人”改为明确的角色名称。

“变更前后的人员负责人”不是清晰的领域术语。使用“变更前后的人工负责人”,或使用项目中已有的标准术语。

建议修改
-相关人员包括操作人、任务创建人、变更前后的人员负责人、任务协作人,以及机器人
+相关人员包括操作人、任务创建人、变更前后的人工负责人、任务协作人,以及机器人
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
相关人员包括操作人、任务创建人、变更前后的人员负责人、任务协作人,以及机器人
负责人的创建者。每位用户只会收到一条通知;如果用户没有可用的 IM 私聊会话,
相关人员包括操作人、任务创建人、变更前后的人工负责人、任务协作人,以及机器人
负责人的创建者。每位用户只会收到一条通知;如果用户没有可用的 IM 私聊会话,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/zh/wegent/user-guide/integrations/im-channel-integration.md` around
lines 292 - 293, 将通知说明中的“变更前后的人员负责人”改为明确且一致的角色术语“变更前后的人工负责人”,并保持其余相关人员范围及通知规则不变。

"allow": [
"**/*"
]
"allow": ["**/*"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

jq '.app.security' wework/src-tauri/tauri.conf.json
rg -n -C 3 'convertFileSrc|asset:|assetProtocol|csp' wework/src wework/src-tauri

Repository: wecode-ai/Wegent

Length of output: 30402


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- wework/AGENTS.md ---'
if [ -f wework/AGENTS.md ]; then
  cat -n wework/AGENTS.md
else
  printf '%s\n' 'missing'
fi

printf '%s\n' '--- configuration ---'
cat -n wework/src-tauri/tauri.conf.json

printf '%s\n' '--- Tauri dependency versions ---'
rg -n -C 2 '"tauri|`@tauri-apps`' wework/package.json wework/src-tauri/Cargo.toml wework/src-tauri/Cargo.lock 2>/dev/null || true

printf '%s\n' '--- relevant path construction and webview boundaries ---'
rg -n -C 4 'convertFileSrc|localHtmlBrowserUrl|asset://|file://|iframe|WebviewWindow|open\(' wework/src wework/src-tauri -g '*.ts' -g '*.tsx' -g '*.rs' -g '*.json' | head -n 500

Repository: wecode-ai/Wegent

Length of output: 50374


🌐 Web query:

Tauri v2 asset protocol scope requireLiteralLeadingDot false CSP null security semantics

💡 Result:

In Tauri v2, the requireLiteralLeadingDot setting and csp: null configuration have distinct security and functional implications for the asset protocol and application security. requireLiteralLeadingDot: false This setting is used within the asset protocol scope to control how glob patterns handle files and directories starting with a dot (e.g.,.cache,.ssh,.gitignore) [1][2]. - On Unix-based systems, requireLiteralLeadingDot defaults to true [1][3][2]. When true, wildcard tokens like *,?, or will not match a path component that begins with a dot, effectively hiding these files/directories from the webview even if they would otherwise match the pattern [1][2]. - Setting requireLiteralLeadingDot: false allows these dot-prefixed components to be matched by wildcards [1][2]. This is necessary if your application specifically needs to load assets from hidden directories [1][4][5]. - Because this configuration widens the scope of files accessible by the webview, it should be used cautiously [1]. It must be configured using the object form of the scope, as the array-only form does not support this property [1][2][5]. CSP: null Setting the Content Security Policy (CSP) to null effectively disables Tauri's default CSP enforcement [6]. - Tauri's default CSP is designed to reduce the impact of common web-based vulnerabilities like Cross-Site Scripting (XSS) by using nonces and hashes for scripts and styles, and restricting asset sources [7]. - Setting csp: null removes these protections [6]. While this may be used for specific troubleshooting or custom security implementations, it is generally discouraged as it exposes the application to a wider range of potential web-based attacks [7]. - Note that in some contexts, setting csp: null has been reported to interfere with certain internal Tauri mechanisms (such as IPC requests during specific page lifecycle events), as the security policy is essentially unset [8]. In summary, requireLiteralLeadingDot: false is a targeted configuration to adjust filesystem scope visibility [1][5], whereas csp: null represents a broad relaxation of the application's security posture [7][6].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- asset URL producers and HTML preview flow ---'
rg -n -C 5 'convertFileSrc|localHtmlBrowserUrl|CodexInlineVisualizationHost|iframe|visualizations|attachments|plugin.*asset|asset://localhost' \
  wework/src wework/src-tauri \
  -g '*.ts' -g '*.tsx' -g '*.rs' | \
  rg -v '(^|/).*\.test\.(ts|tsx):|mock|expect\(|asset://localhost'

printf '%s\n' '--- embedded browser navigation and window creation ---'
rg -n -C 8 'requestEmbeddedBrowserOpen|EmbeddedBrowser|bridge_navigation_url|WebviewWindow|new.*window|url:' \
  wework/src/lib wework/src/components wework/src-tauri/src \
  -g '*.ts' -g '*.tsx' -g '*.rs' | head -n 600

printf '%s\n' '--- configuration and policy assertions ---'
rg -n -C 5 'csp|assetProtocol|requireLiteralLeadingDot|allow.*\*\*/\*|deny' \
  wework --glob '!src-tauri/Cargo.lock' --glob '!**/*.test.*'

Repository: wecode-ai/Wegent

Length of output: 50373


🌐 Web query:

site:v2.tauri.app/security/asset-protocol Tauri v2 scope allow requireLiteralLeadingDot asset protocol security

💡 Result:

In Tauri v2, the requireLiteralLeadingDot setting within the asset protocol scope configuration dictates how wildcard patterns handle files and directories that start with a dot (dotfiles) [1]. When requireLiteralLeadingDot is set to true (which is the default on Unix systems), wildcard tokens such as ,?,, and [...] will not match a path component that begins with a dot [1]. This security mechanism prevents accidental exposure of sensitive hidden directories (like.ssh or.cache) when using broad glob patterns [1]. If you need to grant the asset protocol access to dot-prefixed paths under a broad glob pattern, you can set requireLiteralLeadingDot to false [1]. However, this action widens the scope of accessible files and should be reviewed carefully for security implications [1]. To configure this setting, you must use the object form of the scope rather than the simple array form [1]. An example configuration is shown below: { "app": { "security": { "assetProtocol": { "enable": true, "scope": { "requireLiteralLeadingDot": false, "allow": ["$HOME//"] } } } } } If you encounter issues where files are not loading, ensure the path matches your allow patterns and that any dot-prefixed directories are accounted for, either by explicitly including a literal dot in your pattern or by adjusting the requireLiteralLeadingDot setting [1].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- candidate files ---'
fd -t f -i 'iframe|visualization|embedded-browser|browser-url|assistantMarkdownLinks|tauri.conf' wework/src wework/src-tauri

printf '%s\n' '--- exact asset call sites outside tests ---'
rg -l 'convertFileSrc|localHtmlBrowserUrl|assetProtocol|asset://' wework/src wework/src-tauri \
  -g '!*.test.ts' -g '!*.test.tsx' -g '!*.spec.ts' -g '!*.spec.tsx' |
  sort

printf '%s\n' '--- HTML visualization implementation ---'
for f in $(rg -l 'CodexInlineVisualizationHost|localHtmlBrowserUrl' wework/src -g '!*.test.*'); do
  echo "### $f"
  wc -l "$f"
  rg -n -C 10 'CodexInlineVisualizationHost|localHtmlBrowserUrl|convertFileSrc|iframe|srcDoc|fetch\(' "$f"
done

printf '%s\n' '--- embedded browser implementation ---'
for f in $(rg -l 'requestEmbeddedBrowserOpen|bridge_navigation_url|EmbeddedBrowser' wework/src wework/src-tauri/src -g '!*.test.*'); do
  echo "### $f"
  rg -n -C 8 'requestEmbeddedBrowserOpen|bridge_navigation_url|EmbeddedBrowser|WebviewWindow|url:' "$f"
done

Repository: wecode-ai/Wegent

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- focused visualization source ---'
cat -n wework/src/components/chat/CodexInlineVisualizationHost.tsx | sed -n '58,145p'
cat -n wework/src/components/chat/assistantMarkdownLinks.ts | sed -n '45,110p'
cat -n wework/src/components/topnav/AppIframe.tsx | sed -n '1,180p'

printf '%s\n' '--- focused native browser path handling ---'
rg -n -C 12 'fn (open|navigate).*browser|embedded_browser_(open|navigate)|bridge_navigation_url|file://|asset://' \
  wework/src-tauri/src/embedded_browser wework/src-tauri/src -g '*.rs' | head -n 500

printf '%s\n' '--- Tauri scope and CSP references in lock/vendor metadata ---'
rg -n -C 3 'require_literal_leading_dot|requireLiteralLeadingDot|AssetProtocol|asset_protocol|Content-Security-Policy|csp' \
  wework/src-tauri wework/package.json package.json pnpm-lock.yaml \
  --glob '!Cargo.lock' --glob '!*.test.*' | head -n 300

printf '%s\n' '--- read-only config/call-site verifier ---'
python3 - <<'PY'
import json
from pathlib import Path

config_path = Path("wework/src-tauri/tauri.conf.json")
config = json.loads(config_path.read_text())
security = config["app"]["security"]
scope = security["assetProtocol"]["scope"]
print("csp:", security["csp"])
print("asset protocol enabled:", security["assetProtocol"]["enable"])
print("literal leading dot required:", scope["requireLiteralLeadingDot"])
print("allow patterns:", scope["allow"])
print("deny present:", "deny" in scope)

for path in sorted(Path("wework/src").rglob("*")):
    if path.suffix not in {".ts", ".tsx"} or ".test." in path.name:
        continue
    text = path.read_text(errors="replace")
    if "convertFileSrc(" in text:
        print("convertFileSrc call site:", path)
PY

Repository: wecode-ai/Wegent

Length of output: 50372


🌐 Web query:

Tauri v2 asset protocol scope deny allow glob semantics csp null official documentation

💡 Result:

In Tauri v2, the asset protocol allows the webview to load files from the local filesystem, controlled by security scopes and Content Security Policy (CSP) configurations [1][2]. Asset Protocol Scope Semantics The asset protocol scope defines which filesystem paths are accessible to the webview [1]. If a requested path does not match the configured allow rules (or matches a deny rule), the webview will refuse to load the asset, typically returning a 403 status code [1][3]. Configuration: The scope can be defined as an array of glob patterns or as an object containing allow and deny lists [1]. - Deny rules always take precedence over allow rules [1]. - Glob patterns support wildcards such as *,?,, and [...] [1]. - By default, wildcards do not match path components starting with a dot (e.g.,.cache,.ssh). This behavior can be toggled by setting requireLiteralLeadingDot to false [1]. - Patterns should be absolute or use path variables (e.g., $APPCACHE, $APPDATA) that align with how the path is resolved at runtime [1]. CSP (Content Security Policy) Tauri v2 enforces a CSP to mitigate web-based vulnerabilities [4]. To use the asset protocol, you must explicitly include the asset protocol and its host in your CSP configuration [4][5]. - Example: "img-src": "'self' asset: http://asset.localhost" [4]. - Setting "csp": null in the Tauri configuration disables the CSP entirely, which is generally discouraged as it removes this layer of security [2][6]. When using the asset protocol, the JavaScript API convertFileSrc is used to transform a device file path into a URL that the webview can load [5]. Ensure that both the enable flag is set to true and the necessary paths are defined in the scope within your tauri.conf.json [1][5].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import json
from pathlib import Path

config = json.loads(Path("wework/src-tauri/tauri.conf.json").read_text())
scope = config["app"]["security"]["assetProtocol"]["scope"]
print(json.dumps({
    "csp": config["app"]["security"]["csp"],
    "asset_protocol_enabled": config["app"]["security"]["assetProtocol"]["enable"],
    "scope": scope,
    "allow_is_root_wildcard": scope.get("allow") == ["**/*"],
    "dotfiles_match_wildcards": scope.get("requireLiteralLeadingDot") is False,
}, indent=2))
PY

printf '%s\n' '--- official Tauri asset protocol documentation excerpt ---'
curl -L --fail --silent --show-error \
  https://raw.githubusercontent.com/tauri-apps/tauri-docs/v2/src/content/docs/security/asset-protocol.mdx |
  rg -n -C 5 'scope|allow|deny|requireLiteralLeadingDot|\*\*/\*|CSP|convertFileSrc' | head -n 180

printf '%s\n' '--- Tauri 2.11.5 asset scope implementation excerpt ---'
curl -L --fail --silent --show-error \
  https://raw.githubusercontent.com/tauri-apps/tauri/tauri-v2.11.5/crates/tauri/src/protocol/asset.rs |
  rg -n -C 8 'Scope|scope|allow|deny|is_allowed|glob|literal|dot' | head -n 240

Repository: wecode-ai/Wegent

Length of output: 12046


Restrict the asset protocol scope and enable CSP.

allow: ["**/*"] with requireLiteralLeadingDot: false exposes the broadest filesystem scope, including dot-prefixed paths. csp: null removes CSP protection. A compromised WebView script can use asset:// requests to read exposed files. Use narrow application directories, keep literal-dot protection enabled, and add deny rules for sensitive paths. Grant user-selected paths through a runtime or persisted scope instead of a global wildcard. Configure a restrictive CSP that allows only required asset sources.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wework/src-tauri/tauri.conf.json` at line 41, Restrict the asset protocol
configuration around the allow rule by removing the global "**/*" scope, keeping
requireLiteralLeadingDot enabled, and adding deny rules for sensitive paths.
Replace csp: null with a restrictive policy permitting only the asset sources
required by the application, and handle user-selected paths through runtime or
persisted scope grants rather than global access.

Comment on lines +66 to +72
it('does not fail a committed mutation when notification reporting fails', async () => {
const api = baseApi()
const report = vi.fn().mockRejectedValue(new Error('backend unavailable'))
const notifying = withAITableNotifications(api, report)

await expect(notifying.createRecord('11', {})).resolves.toEqual(record)
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the rejected reporting call.

This test passes if the wrapper does not call report in this branch. Assert one call and validate its event type after the mutation resolves. The implementation contract reports after a committed mutation and catches only the reporting failure.

Proposed test update
 await expect(notifying.createRecord('11', {})).resolves.toEqual(record)
+expect(report).toHaveBeenCalledTimes(1)
+expect(report).toHaveBeenCalledWith(
+  expect.objectContaining({ eventType: 'external_record_created' })
+)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('does not fail a committed mutation when notification reporting fails', async () => {
const api = baseApi()
const report = vi.fn().mockRejectedValue(new Error('backend unavailable'))
const notifying = withAITableNotifications(api, report)
await expect(notifying.createRecord('11', {})).resolves.toEqual(record)
})
it('does not fail a committed mutation when notification reporting fails', async () => {
const api = baseApi()
const report = vi.fn().mockRejectedValue(new Error('backend unavailable'))
const notifying = withAITableNotifications(api, report)
await expect(notifying.createRecord('11', {})).resolves.toEqual(record)
expect(report).toHaveBeenCalledTimes(1)
expect(report).toHaveBeenCalledWith(
expect.objectContaining({ eventType: 'external_record_created' })
)
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wework/src/api/aitable.test.ts` around lines 66 - 72, Update the test for
withAITableNotifications around notifying.createRecord to assert that the
rejected report function is called exactly once after the mutation resolves, and
verify the call’s event type matches the committed create mutation. Preserve the
existing assertion that the mutation resolves successfully despite the reporting
failure.

Comment thread wework/src/api/aitable.ts
Comment on lines +202 to +208
async deleteRecord(projectId, recordId) {
const existing = api.getRecord
? await api.getRecord(projectId, recordId)
: { id: recordId, cells: {}, raw: {} }
await api.deleteRecord(projectId, recordId)
await notify(projectId, 'external_record_deleted', existing, '删除了表格任务')
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard the pre-delete record fetch so notification work cannot block the delete.

deleteRecord calls api.getRecord before the delete and does not handle a rejection. getRecord performs a network call that can fail for reasons unrelated to the delete. If it rejects, api.deleteRecord never runs and the user-facing delete fails.

The rest of this wrapper treats notification work as best-effort. Apply the same rule to the title lookup.

🐛 Proposed fix
     async deleteRecord(projectId, recordId) {
-      const existing = api.getRecord
-        ? await api.getRecord(projectId, recordId)
-        : { id: recordId, cells: {}, raw: {} }
+      const fallback: AITableRecord = { id: recordId, cells: {}, raw: {} }
+      const existing = api.getRecord
+        ? await api.getRecord(projectId, recordId).catch(() => fallback)
+        : fallback
       await api.deleteRecord(projectId, recordId)
       await notify(projectId, 'external_record_deleted', existing, '删除了表格任务')
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async deleteRecord(projectId, recordId) {
const existing = api.getRecord
? await api.getRecord(projectId, recordId)
: { id: recordId, cells: {}, raw: {} }
await api.deleteRecord(projectId, recordId)
await notify(projectId, 'external_record_deleted', existing, '删除了表格任务')
},
async deleteRecord(projectId, recordId) {
const fallback: AITableRecord = { id: recordId, cells: {}, raw: {} }
const existing = api.getRecord
? await api.getRecord(projectId, recordId).catch(() => fallback)
: fallback
await api.deleteRecord(projectId, recordId)
await notify(projectId, 'external_record_deleted', existing, '删除了表格任务')
},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@wework/src/api/aitable.ts` around lines 202 - 208, Update deleteRecord so the
optional api.getRecord lookup is best-effort: catch or otherwise neutralize its
rejection and fall back to the existing placeholder record, then always proceed
to api.deleteRecord. Preserve the subsequent notify call and existing behavior
when the lookup succeeds.

@qdaxb qdaxb closed this Sep 18, 2026
@qdaxb
qdaxb deleted the feature/cloud-board-im-notifications branch September 18, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant