Repository navigation
feat(channels): add DingTalk Workspace channel - #9394
Conversation
Add a DingTalk Workspace (DWS) channel package so a workspace can be driven from DingTalk alongside the existing channels. - packages/channels/dws: new workspace holding the DWS client, event stream, environment resolution and channel implementation, with the event-source fixtures used by its tests. - cli: register DWS in the channel registry and its builtin list. - web-shell: recognise the DWS platform in the channels UI. - docs: document the channel and its configuration under docs/users/features/channels. - build/release: include the new workspace in the build, clean and release-version scripts and the vitest project list. The channel watches native DingTalk todos, routes document and todo replies back to their originating conversation, bounds notification retries, and keeps sender identity authoritative for direct messages.
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
…t base's source Round 1 review, two Critical findings. vitest.config.ts — the new package's config was the only channel config without the `@qwen-code/channel-base` → source alias its five siblings carry, so `cd packages/channels/dws && npx vitest run` (the workflow AGENTS.md prescribes) depended on a prior `tsc --build` of base. Reproduced the reviewer's witness in this worktree with base/dist moved aside: without the alias vitest dies in `packageEntryFailure` and runs zero tests; with it, 63/63 pass. Even when dist exists it may lag base's source — it did here, by four days. dws-client.ts — `DWS_NOT_SENT_ERROR_CODES` listed only the path errnos, so a `dws` process that never started because of fd or memory exhaustion (`EMFILE`/`ENFILE`/`ENOMEM`/`EAGAIN` and family) was classified `unknown`. The todo and document reply paths in dws-channel.ts swallow `unknown` as "the originating task will not be rerun", so a user's final reply was dropped permanently on one log line instead of being retried — and the retry is safe, since the fingerprint is not persisted when delivery fails. The set now carries the whole `uv_spawn` pre-exec family. Everything else the callback reports — a non-zero exit (numeric `code`), a timeout kill (`code === null`), `ABORT_ERR`, a `maxBuffer` overrun — happened with a child already running and stays `unknown`, because a retry there could duplicate a delivery. The classification moved into an exported `classifyDwsCommandFailure` so the table can be driven directly: the resource errnos need real fd or memory exhaustion to reproduce through a spawn, which no unit test can stage safely. The existing missing-executable test still covers the wiring end to end. Verified: packages/channels/dws — 191 passed (5 files). Mutation-verified: reverting the errno set turns exactly the 12 added codes red (12 failed / 51 passed); dropping the vitest alias with base/dist absent turns the suite from 63 passed into a collection failure. eslint and prettier clean. The one tsc error on this branch (`displayText` missing from `Envelope`) is worktree build skew — base/dist was built 2026-08-10, base/src changed 2026-08-14, and the field is present in the source; it reproduces identically with these changes stashed.
…dup slot Round-2 review, R2-4 (Critical). `notificationKey` is `documentNotificationKey(documentId, commentKey)` — no sender in it — so a `'denied'` outcome falling into the `else` branch marked that (document, comment) pair processed for good. Every later notification for the same comment, live or polled, then hit `processedMessages.includes(notificationKey)` and returned silently, including one from a sender who IS allowed. The cursor persists, so the drop survived restarts. Concretely, with `senderPolicy: 'allowlist'` and `allowedUsers: ['open-bob']`: Alice (not allowlisted) @-mentions the bot in a document comment and is denied; Bob then mentions the bot on the same comment thread — the ordinary multi-reviewer document flow — and is dropped forever, with no dispatch, no pairing and no log. A denied notification is now parked with `rememberPendingDocumentNotification` like a `'pairing'` one rather than consuming the slot. Replay already skips a pending entry whose sender fails `gate.isAllowed`, so a denied sender does not get retried in; and an allowed sender reaching the same comment clears the entry on the way through. The existing `applies sender access policy to document mention notifications` cannot cover this — its denied and allowed notifications are on DIFFERENT comments, so the shared key is never exercised. New test puts both on the same comment. Mutation-checked: restoring the old condition reddens it with `bridge.prompt` called 0 times against an expected 1, reproducing the review's own witness. Verification: `npm run build` and `tsc --noEmit` clean in packages/channels/dws; eslint clean on both changed files; full package suite 192/192 (118 in dws-channel.test.ts, 1 new).
…able replay from pinning the watermark (R2-1, R2-2, R2-4 queue) Three ways history polling could stall forever, each measured: **R2-2, poison message.** A message whose turn threw was never marked processed, so the watermark never advanced and every poll re-ran it as a full agent turn — one model call per iteration, no cap, no backoff — while the pinned watermark grew the query window without bound and the throw starved every newer message behind it. Pending-document replay already had retry accounting; this path had none. Inbound failures are now counted per message and persisted in the cursor: under budget the error still propagates (redelivery retry and the concurrent-duplicate contract depend on that, and their tests pin it), and once the budget is spent the message is marked processed and dropped with a logged reason. **Pending-queue cap.** `rememberPendingDocumentNotification` threw at MAX_PROCESSED_ITEMS, and the throw aborted the direct-message loop before the checkpoint, the watermark and `markProcessedMessage` — so every later poll re-scanned a growing window and re-threw on the same never-marked message, surviving restarts in the cursor. The queue's only drain is an allowed sender later processing the same comment, so entries parked for unapproved senders never leave: one unpaired member @-mentioning the bot in 5,000 distinct comments broke document history polling until manual cursor surgery. It now evicts the oldest instead, which costs at most a pairing prompt nobody approved. **R2-1, the replay the fixture could not recover.** The test fake ignored its `startTime`/`endTime`, so it certified a recovery the production arithmetic cannot perform. Fixed on both sides: the fake now filters by its window like the real client (and `message()` defaults `eventTime` to now, since real messages always carry one — six fixtures were silently relying on epoch 0), and the stale-replay guard now pulls `notificationWatermark` back to the parked notification's event time. It parks document notifications UNMARKED on purpose, "for polling to recover"; on a fresh cursor the watermark started at `connectionStartedAt` and the window opened at `watermark − 5s` — exactly the guard's own drop boundary — so everything it parked was strictly outside every window that watermark would ever produce. Every fix is mutation-verified: reverting the retry budget re-runs the poison turn once per poll (8 polls, 8 turns), restoring the queue throw reproduces the reviewer's stderr and the pinned watermark, and dropping the watermark pull-back leaves the replayed notification unrecovered. Suite 194/194 green; `tsc -p packages/channels/dws` clean. R1-2 (self-identity degradation) is not in this commit — both fixes the review proposes collide with behaviour this suite pins deliberately; see the thread. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
|
@qwen-code /review |
…urce (R4-1, R4-3)
R4-1: round 3 added an inbound failure budget, but wired it into one of the
three `handleInbound` call sites — the mention/live-IM path. The other two kept
the exact unbounded-retry mode the budget's own doc comment says it exists to
close.
- Document notifications (`processDocumentNotification`): a throw escapes
`pollOnce`'s sorted loop and is swallowed by the outer catch, so nothing is
marked processed and `notificationCheckpoint`/`notificationWatermark` — both
assigned after the loop — never advance. Every 5s poll re-ran the same full
agent turn, forever, starving every newer notification behind it.
- Native todos (`pollTodos`): the fingerprint is remembered only on success, so
a todo whose turn keeps throwing was re-fetched and re-run every poll,
forever.
`recordInboundFailure` now takes the drop action as a parameter, because "stop
re-running this" differs per surface: marking the key processed is right for a
message, a document notification carries its own `notificationKey` (and a
pending entry to clear), and a todo is re-fetched by fingerprint. The default
keeps the mention path byte-identical.
R4-3: `retryable: false` is terminal before ready — `retryLimit` returns 0 —
but `scheduleImRestart` never consulted it, and `startImSource` resets
`restartAttempts` to 0 every time a subscription becomes ready. The backoff
exponent therefore stayed at 0, so a permanently denied consumer (permission
revoked, subscription not allowed) was respawned at a constant ~3s forever —
one `dws event consume` child every 2-3s per affected source — while the
channel reported itself connected and delivered nothing for that source.
Post-ready now matches pre-ready: terminal, with a log line saying so.
Verification (`cd packages/channels/dws`):
- `npx vitest run` — 197 passed (was 194; three new tests).
- `npx tsc -p tsconfig.json --noEmit` — clean.
- Mutation checks, one per fix, each turning exactly its own test red and
leaving the other 122 green:
- drop the `retryable === false` guard -> `stops restarting a source that
died permanently after becoming ready` fails.
- drop the document-path budget -> `drops a document notification whose turn
keeps failing, and stops starving newer ones` fails (the newer
notification is never reached).
- drop the todo-path budget -> `drops a native todo whose turn keeps
failing` fails (8 turns instead of 5).
- eslint + prettier clean.
Not addressed in this commit: R4-2 (checkpoint drain overwriting the stale
replay pull-back), R4-4, R1-2, and R4-5..R4-8.
…lback (R4-4) `handleImMessage` leaves a replayed document notification UNMARKED on purpose, for history polling to pick up, and pulls `notificationWatermark` back to the replay's `eventTime` so a future window can reach it. `pollOnce` then wrote `checkpoint.endTime` over that watermark unconditionally when its own window finished — and `checkpoint.endTime` is always past the replay's `eventTime`. The race is not hairline: `runLoop` polls immediately on connect and the IM subscriptions start before the poll loop, so a startup replay arrives precisely while poll #1's `listDirectMessages` is awaiting. One clobber puts the parked replay outside every window the watermark will ever produce — no turn, no log, no error, and it survives restarts because `saveCursor()` persists it. `pollOnce` now records whether the watermark was pulled back while its direct-message fetch was in flight, and on that path drops the window instead of finishing it: neither the advance nor the paginated checkpoint resume is safe, because the checkpoint was itself derived from the pre-pullback watermark. The next poll re-derives a window from the pulled-back value. Test: `keeps the stale-replay pullback when a poll was already in flight` emits the replay from inside `listDirectMessages`. Mutation-checked — forcing the guard false reddens it with `inbound` empty, matching the reviewer's witness (`dispatched = 0`). It also asserts the second query window opens at or before the replay's `eventTime`, so a fake that ignored its window could not certify it.
…R6-1/R6-2/R6-3)
All three Criticals round 6 raised share a failure shape: a document comment is
consumed by something that had no right to consume it, the user gets no reply,
and nothing is logged. Each is fixed at the point that consumes the slot.
R6-1 — `handleImMessage` pullback (dws-channel.ts): R4-4 rescued a stale replay
by pulling the notification watermark back, but the flag `pollOnce` consults is
cleared at the top of every fetch, so it only ever covered a replay that landed
DURING one. A pullback arriving in the gap between two polls is reset before it
is read; a persisted multi-page `notificationCheckpoint` then resumes a window
that starts after the replay and finishes by writing `checkpoint.endTime` back
over the pulled-back watermark. The replay was left unmarked on purpose, so
after that no window ever reaches it again. The pullback branch now drops the
checkpoint as well, which makes the rescue durable regardless of when the
replay arrived; the in-flight flag still guards the during-a-fetch case.
R6-2 — in-flight awaiter (dws-channel.ts): a pending entry means the in-flight
turn PARKED the comment for a sender it would not serve, which says nothing
about the caller waiting behind it. Marking unconditionally consumed an ALLOWED
sender's mention outright — replay only re-drives a parked entry whose own
`senderId` passes the gate (the denied one never will), and the allowed
sender's marked message key is skipped by every later history poll. The awaiter
now marks only when the comment is genuinely processed, or when this caller is
no more entitled to it than the sender already parked. This is what the
denied-sender comment further down already claimed happened ("an allowed sender
reaching the same comment clears the entry on the way through") — the awaiter
was the path that never let them reach it.
R6-3 — failure-budget drop closure (dws-channel.ts): the closure marked the
sender-agnostic `notificationKey` (`document\0comment`, no sender), so five
failed turns — about 25s of transient model or bridge trouble, since each 5s
poll re-runs an unmarked notification — dropped every FUTURE mention of that
comment from anyone, permanently and across restarts. It now marks only the
failing message's own `key`, which is what stops the window re-running it, so
the R4-1 starvation this budget closes stays closed.
Tests (dws-channel.test.ts), each mutation-verified against the pre-fix code:
- `keeps a stale-replay pullback that arrives between two polls` — persists a
bounded checkpoint, emits the replay with no poll in flight, asserts the
checkpoint is released and the next window reaches back over the replay.
Reverting R6-1: `expected { startTime: … } to be undefined`.
- `lets an allowed sender through while a denied turn on the same comment is in
flight` — the concurrent counterpart to the existing R2-4 test, which lets
the denied turn finish first and so cannot reach the awaiter. Reverting R6-2:
the allowed sender's prompt is never called.
- `lets a later mention of a dropped comment retry with a fresh budget` — five
failing polls, then a different reviewer on the same comment after the
outage. Reverting R6-3: `expected [] to deeply equal [ ObjectContaining{…} ]`.
Verification: `npx vitest run` in packages/channels/dws — 201 passed (5 files);
`npx tsc --noEmit -p packages/channels/dws/tsconfig.json` clean; `npm run build`
in that package clean; eslint and prettier clean on both changed files.
R1-2 is untouched: it still needs a maintainer call on which pinned contract
gives, and is not something this commit should decide.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Head drift detected: review was run against
0b86e99bbb0c3a300d7c8545d15f1747769f4b69, but the PR head is now9dbe5e9e15352bd54900fd7aab52f95aeaf9da44. Inline comments may not line up, so this review keeps the summary only.
Review of PR #9394 — feat(channels): add DingTalk Workspace channel
Target: QwenLM/qwen-code — feat/dingtalk-workspace-channel → main (+8334/-10, 35 files)
Scope: New dws (DingTalk Workspace) channel package under packages/channels/dws/
Reviewer model: deepseek-v4-flash
Coverage: 12/12 chunks reviewed, 16 agents ran
Summary
This is a large, well-crafted feature PR adding a new DingTalk Workspace (DWS) channel adapter. The codebase follows existing patterns, includes comprehensive test coverage, and handles edge cases thoroughly. All integration points (CLI registry, Web Shell, build scripts, release pipeline) are correctly wired.
No Critical issues found. All findings are Suggestion / Nice to have.
Findings
| # | File | Issue | Severity |
|---|---|---|---|
| 1 | dws-channel.ts |
cursor.inboundFailures not returned from validateCursor — failure tracking resets on restart |
Suggestion |
| 2 | dws-channel.ts |
isMentioned set to false for direct messages — semantically incorrect |
Suggestion |
| 3 | dws-channel.ts validateCursor |
inboundFailures not validated — cross-file inconsistency |
Suggestion |
| 4 | dws-channel.test.ts |
as never cast in seedPendingDocumentNotifications — request field missing |
Nice to have |
| 5 | dws-channel.ts |
Double cursor save per poll cycle (agent + base class) | Nice to have |
| 6 | dws-channel.ts |
sourceLabel() unnecessarily passed through sanitizeLogText |
Nice to have |
| 7 | dws-channel.ts connect() |
inboundFailures not cleared on profile switch |
Nice to have |
| 8 | dws-channel.ts |
5 mutant-survived statements (cursor cleanup, document set) | Suggestion |
| 9 | .github/workflows/release.yml |
Hunk reversion not caught by tests | Suggestion |
| 10 | docs/users/features/channels/_meta.ts |
Hunk reversion not caught by tests | Suggestion |
| 11 | index.test.ts |
approvalMode: 'auto' test uses fragile substring match |
Suggestion |
Notable strengths
- Documentation accuracy: All 23 documentation claims verified against source code.
- Test coverage: 3206-line test file covering profile switching, identity management, document notification routing, pairing flow, working reactions, todo handling, cursor persistence, and cross-stream deduplication.
- Security:
dwsProcessEnvironmentcorrectly filters secrets,windowsHide: true,sanitizeLogTexton all log output. - Cross-file consistency: All integration points (channel-registry, channel-platform, build scripts, release) follow existing patterns.
Environment notes
- Build failed on
packages/audio-capture(missing Python for node-gyp) — pre-existing environment issue, not PR-related. - Tests for affected workspaces (
packages/channels/dws,packages/cli,packages/web-shell) were not reached due to the cascading build failure. - Original review output was not posted automatically because the GitHub API was unreachable from the review runner; this comment was submitted manually after the review completed.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Head drift detected: review was run against
0b86e99bbb0c3a300d7c8545d15f1747769f4b69, but the PR head is now9dbe5e9e15352bd54900fd7aab52f95aeaf9da44. Inline comments may not line up, so this review keeps the summary only.
Review of PR #9394 — feat(channels): add DingTalk Workspace channel
Target: QwenLM/qwen-code — feat/dingtalk-workspace-channel → main (+8334/-10, 35 files)
Scope: New dws (DingTalk Workspace) channel package under packages/channels/dws/
Reviewer model: deepseek-v4-flash
Coverage: 12/12 chunks reviewed, 16 agents ran
Summary
This is a large, well-crafted feature PR adding a new DingTalk Workspace (DWS) channel adapter. The codebase follows existing patterns, includes comprehensive test coverage, and handles edge cases thoroughly. All integration points (CLI registry, Web Shell, build scripts, release pipeline) are correctly wired.
No Critical issues found. All findings are Suggestion / Nice to have.
Findings
| # | File | Issue | Severity |
|---|---|---|---|
| 1 | dws-channel.ts |
cursor.inboundFailures not returned from validateCursor — failure tracking resets on restart |
Suggestion |
| 2 | dws-channel.ts |
isMentioned set to false for direct messages — semantically incorrect |
Suggestion |
| 3 | dws-channel.ts validateCursor |
inboundFailures not validated — cross-file inconsistency |
Suggestion |
| 4 | dws-channel.test.ts |
as never cast in seedPendingDocumentNotifications — request field missing |
Nice to have |
| 5 | dws-channel.ts |
Double cursor save per poll cycle (agent + base class) | Nice to have |
| 6 | dws-channel.ts |
sourceLabel() unnecessarily passed through sanitizeLogText |
Nice to have |
| 7 | dws-channel.ts connect() |
inboundFailures not cleared on profile switch |
Nice to have |
| 8 | dws-channel.ts |
5 mutant-survived statements (cursor cleanup, document set) | Suggestion |
| 9 | .github/workflows/release.yml |
Hunk reversion not caught by tests | Suggestion |
| 10 | docs/users/features/channels/_meta.ts |
Hunk reversion not caught by tests | Suggestion |
| 11 | index.test.ts |
approvalMode: 'auto' test uses fragile substring match |
Suggestion |
Notable strengths
- Documentation accuracy: All 23 documentation claims verified against source code.
- Test coverage: 3206-line test file covering profile switching, identity management, document notification routing, pairing flow, working reactions, todo handling, cursor persistence, and cross-stream deduplication.
- Security:
dwsProcessEnvironmentcorrectly filters secrets,windowsHide: true,sanitizeLogTexton all log output. - Cross-file consistency: All integration points (channel-registry, channel-platform, build scripts, release) follow existing patterns.
Environment notes
- Build failed on
packages/audio-capture(missing Python for node-gyp) — pre-existing environment issue, not PR-related. - Tests for affected workspaces (
packages/channels/dws,packages/cli,packages/web-shell) were not reached due to the cascading build failure. - Original review output was not posted automatically because the GitHub API was unreachable from the review runner; this comment was submitted manually after the review completed.
Review of PR #9394 —
|
| # | File | Issue | Severity |
|---|---|---|---|
| 1 | dws-channel.ts |
cursor.inboundFailures not returned from validateCursor — failure tracking resets on restart |
Suggestion |
| 2 | dws-channel.ts |
isMentioned set to false for direct messages — semantically incorrect |
Suggestion |
| 3 | dws-channel.ts validateCursor |
inboundFailures not validated — cross-file inconsistency |
Suggestion |
| 4 | dws-channel.test.ts |
as never cast in seedPendingDocumentNotifications — request field missing |
Nice to have |
| 5 | dws-channel.ts |
Double cursor save per poll cycle (agent + base class) | Nice to have |
| 6 | dws-channel.ts |
sourceLabel() unnecessarily passed through sanitizeLogText |
Nice to have |
| 7 | dws-channel.ts connect() |
inboundFailures not cleared on profile switch |
Nice to have |
| 8 | dws-channel.ts |
5 mutant-survived statements (cursor cleanup, document set) | Suggestion |
| 9 | .github/workflows/release.yml |
Hunk reversion not caught by tests | Suggestion |
| 10 | docs/users/features/channels/_meta.ts |
Hunk reversion not caught by tests | Suggestion |
| 11 | index.test.ts |
approvalMode: 'auto' test uses fragile substring match |
Suggestion |
Notable strengths
- Documentation accuracy: All 23 documentation claims verified against source code.
- Test coverage: 3206-line test file covering profile switching, identity management, document notification routing, pairing flow, working reactions, todo handling, cursor persistence, and cross-stream deduplication.
- Security:
dwsProcessEnvironmentcorrectly filters secrets,windowsHide: true,sanitizeLogTexton all log output. - Cross-file consistency: All integration points (channel-registry, channel-platform, build scripts, release) follow existing patterns.
Environment notes
- Build failed on
packages/audio-capture(missing Python for node-gyp) — pre-existing environment issue, not PR-related. - Tests for affected workspaces (
packages/channels/dws,packages/cli,packages/web-shell) were not reached due to the cascading build failure. - Original review output was not posted automatically because the GitHub API was unreachable from the review runner; this comment was submitted manually after the review completed.
… (R7-1) `parseDocumentMentionNotification` reconstructs `(documentId, commentKey)` from rendered message text, so a bare alidocs URL in an ordinary DM forges a mention card the channel cannot tell apart from a genuine platform notification. `processDocumentNotification` then called `readDocumentContext` on that attacker-named document BEFORE `handleInbound` resolved the sender gate, so under the documented default `senderPolicy: 'pairing'` an unpaired stranger could force this profile to perform an authenticated read of any document it can reach — a turn the channel would never serve them. Resolve `gate.isAllowed(message.senderId)` first and read only for a sender this channel will actually answer. The envelope already carries a "Document Markdown was unavailable" fallback, the `preflightInbound` document branch still parks the mention exactly as before, and `replayPendingDocumentNotifications` re-enters this path once the sender is approved, so an approved turn still gets its document context — just after the gate instead of before it. BEHAVIOR FLIP: `replays a pairing-pending document mention after approval` pinned `readDocument` being called once for the still-unpaired sender and twice overall. That pinned expectation was the defect: it asserted an authenticated read driven by a sender the gate had already refused. It now expects zero reads before approval and one after. Verified by mutation — reverting the guard turns both this test and the new forged-mention test red. Still open on this class and NOT addressed here: the pairing-code write into the attacker-named comment thread. Closing that needs either fail-closed verification that `commentKey` is a real comment on `documentId` mentioning this profile (no DWS CLI surface exposes it — `listMentionedMessages` covers group IM, not document comments) or structured mention events, so it is a maintainer contract call rather than a local fix. Verification: - packages/channels/dws: 202 passed (5 files), including the new `does not read a forged document mention before the sender gate resolves` - tsc --noEmit -p packages/channels/dws/tsconfig.json: clean - eslint + prettier --check on both changed files: clean
`channel-registry.ts` dynamically imports `@qwen-code/channel-dws`, whose package.json resolves the bare specifier to `dist/index.js` and which `packages/cli/vitest.config.ts` does not alias to source. It therefore belongs in `DIST_PREREQUISITES['packages/cli']` alongside every other builtin channel, so a cli test run on an unbuilt checkout reports the actionable "run npm run build" message instead of a raw resolution error. This is what the required `Test (ubuntu-latest, Node 22.x)` check caught on 4bf0407: scripts/tests/vitest-global-setup.test.js asserts the list stays in sync with the registry, and dws was the one registry import missing from it. Verified: `npx vitest run scripts/tests/vitest-global-setup.test.js` 29 passed; reverting this one line reproduces the CI assertion exactly ("missing prerequisite entry for packages/channels/dws"), 1 failed | 28 passed. prettier --check and eslint clean.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen-max-2026-08-20 via Qwen Code /review (v0.21.10)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Two-phase review completed, but PR head has drifted since review started. The original inline comments may not line up with the current diff, so this comment contains only the summary/findings.
Review outcome
- Round 1:
deepseek-v4-flash— no issues found. - Round 2:
qwen3.8-maxindependent re-review — 2 suggestions found.
Findings
1. validateCursor drops inboundFailures, resetting the inbound failure budget on restart
File: packages/channels/dws/src/dws-channel.ts
recordInboundFailure() persists per-message failure accounting into this.cursor.inboundFailures, and DwsCursor declares the field. However, validateCursor() — the only load path — validates every other optional field but omits inboundFailures entirely. On channel restart, a poison message/document-notification/todo that had accumulated failure attempts gets a fresh budget (up to MAX_INBOUND_ATTEMPTS more per restart). Within one process lifetime the budget is bounded, but the field being typed, written, capped, and never read back looks like an accidental omission.
Suggestion: validate and restore inboundFailures in the returned cursor (capped as on write), or explicitly document + test that the budget resets on restart.
2. Documentation describes a fallback the code does not implement
File: docs/users/features/channels/dws.md
The final paragraph says that when identity metadata is unavailable, a direct conversation stays bound to its observed peer sender ID and ignores events from other senders. The implementation actually hard-fails: connect() throws DWS direct messages require the authenticated identity to expose an openDingTalkId. when no selfSenderIds are available and a direct source is enabled. There is no code path that binds a direct conversation to its first observed peer sender or filters other senders per-conversation.
Suggestion: rewrite the paragraph to describe the actual behavior — direct messages require authoritative self-identity at connect; without it the channel refuses to start.
Scope notes
- Reviewed all production sources:
dws-channel.ts,dws-client.ts,dws-event-stream.ts,dws-environment.ts,index.ts, plus wiring (release.yml, build scripts, registry, web-shell, tsconfigs, docs). - Audited hardening around failure budgets, watermark races, pairing dedup, echo suppression, cursor caps, pagination caps, idempotency keys, and env filtering.
- Several probed attack surfaces (poison-message loops, watermark races, forged mention cards, pairing echo loops, cursor growth, subprocess injection) are already closed and test-pinned.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Review Summary
Review of PR #9394: feat(channels): add DingTalk Workspace channel (+8583/-10, 36 files).
Result: No Critical issues found. The code quality is high with comprehensive test coverage (99 tests in dws-channel.test.ts, 32 in dws-client.test.ts, plus supporting test files). The incoming failure budget, watermark pullback, stale-replay recovery, and security fixes from previous review rounds (R2-1, R2-2, R4-1, R4-4, R6-1, R7-1) have been implemented correctly.
Suggestions (not blockers)
inboundFailuresdropped byvalidateCursor—DwsCursor.inboundFailuresis written byrecordInboundFailurebut not validated or returned byvalidateCursor, so the per-message failure count resets on channel restart.PersistedDocumentNotificationseed objects omitrequestfield — test fixtures useas neverto suppress the type error for the omittedrequestfield.processedMessagesO(n) lookup —Array.includes()on a 5000-element array in the hot path; aSet-based index would be more efficient.reportErrorsilently discardsonErrorexceptions — thecatch {}inreportErrorswallows exceptions from the caller's error handler with no diagnostic.validateConfighas no unit tests for invalid inputs — the plugin'svalidateConfigfunction is exercised only through the valid path in registry tests.
Build & Test
- Build failed on
packages/audio-capture(node-gyp can't find Python) — pre-existing environment issue, not caused by this PR. - The changed workspaces compiled successfully. Tests could not run due to the pre-existing build failure.
Scope
- 36 files changed, +8583/-10 — purely additive, no removed behaviors.
- 12/12 chunks reviewed — full diff coverage by 17 review agents.
Superseded by a later qqqys commit; the current head requires re-review.
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Address-review summary — PR #9394 (Critical-only round)Critical-only mode is active (5 change-producing rounds complete). The deferred non-Critical feedback section was treated as an audit record: no code changes, thread resolutions, or replies were made for it. One actionable finding arrived this round. Feedback points and decisions[ic:5391248088] Critical — profile switch inherits the previous profile's inbound-failure budget and can silently discard new work → Resolved in codeClaim (checkable, from the maintainer's deep-verification report): on authenticated profile change, Reproduction (before implementing anything): added a focused test Why it was not pushed: Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round. build failed on the agent-committed fix (pre-existing: also fails without this round's commit) Measured fact: the same check also fails at 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 验证门的拒绝原因与日志证据见上方英文部分（gate-rejection 不翻译）。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32695884689 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Autofix round summary — PR #9394 (Critical-only mode)Critical-only mode is active (5 change-producing rounds complete). This round addressed all three actionable Critical items — the two inline findings from the automated reviewer and the Critical claim from the maintainer's local deep verification. The Prior rejected attemptThe previous round was rejected because Findings addressed1. [rc:3842260263] R18-1 (Critical) — post-mortem stdout events wipe the terminal error — Resolved
Why it was not pushed: Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round. build failed on the agent-committed fix (pre-existing: also fails without this round's commit) Measured fact: the same check also fails at 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 验证门的拒绝原因与日志证据见上方英文部分（gate-rejection 不翻译）。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32714543876 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Autofix round summary — PR #9394Critical-only mode round. All three actionable Critical findings (R18-1, R18-2, R19-1) were reproduced probe-first on the unmodified code, fixed at the root cause, and pinned by new witness tests that fail without their fix (verified by mutation probes). Committed as Feedback points and dispositionsR18-1 — post-mortem stdout events wipe the terminal error (rc:3842260263, rc:3844138371) — RESOLVEDFinding (Critical): when Probe: new fixture Fix: Why it was not pushed: Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round. build failed on the agent-committed fix (pre-existing: also fails without this round's commit) Measured fact: the same check also fails at 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 验证门的拒绝原因与日志证据见上方英文部分（gate-rejection 不翻译）。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32738254462 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 agent time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 已停止：当前计数窗口内已累计 3 次时间预算耗尽（含其间推送过的轮次；本轮本身可能以别的方式失败）。即 3 次完整 agent 运行没有推送任何内容。应由人工拆分或缩减该 PR（或同时提高 agent 时间预算与其步骤兜底），然后评论 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32767251459 🧠 Handled by Qwen Code · model/模型 |
qwen-code-dev-bot
added
the
autofix/needs-human
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
Superseded by a later commit; the current head needs a fresh review.
|
@qwen-code /triage |
Local verification report — real environment, PR head
|
| Component | Real or simulated |
|---|---|
packages/channels/dws + CLI/daemon/Web Shell |
real — built from this branch (per-package tsc → esbuild bundle → vite for the Web Shell) |
qwen channel start dws-work |
real |
qwen serve --channel dws-work (daemon-managed worker) |
real |
| Agent turns (ACP sessions, pairing store, cursor persistence) | real |
qwen channel pairing approve |
real |
dws binary |
simulated — a stand-in on PATH implementing the exact command surface dws-client.ts drives, recording every argv and every inherited env var |
| Model | simulated — a scripted OpenAI-compatible server, so replies are deterministic and the exact prompt is inspectable |
DWS CLI 1.0.57 and a real DingTalk login are not available on this host. The channel's entire external boundary is execFile('dws', argv) plus spawn('dws', ['event','consume',…]), so a stand-in at that boundary exercises all channel logic while making the wire fully observable. Volume: 1188 dws invocations, 43 event-consumer spawns, 23 outbound deliveries, 46 agent turns.
Anti-vacuity control. To rule out a harness that passes without running your code, I patched the built chunk (暗中观察 → MUTANT_REACTION_MARKER, DWS channel policy: → DWS-MUTATED-POLICY-MARKER:) and re-ran. Both changes appeared on the wire — the emoji in add-emoji, the marker in the model request, with the original gone. Every observation below therefore comes from this PR's own compiled code.
Results
Your Reviewer Test Plan, items 1–6:
| # | Claim | Result |
|---|---|---|
| 1 | Configure type: "dws", start with qwen channel start |
✅ connects; pins --profile qwen-verify on every scoped call; polls history every 5s with a 5s overlap; todos every 30s |
| 2 | DM → one pairing prompt → approve → 暗中观察 while running → one final reply |
✅ 3 DMs from an unapproved sender ⇒ 1 pairing message, 0 agent turns. After approval: reaction added at t+0.00s, reply at t+6.09s, reaction removed at t+6.11s. Reply goes back via chat message reply --ref-msg-id … --ref-sender … with an idempotency --uuid |
| 3 | Repeated messages from an unapproved account: no repeat prompt, no agent run | ✅ confirmed (see above) |
| 4 | Document comment @mention with notification → read doc, answer under the comment; no notification → no task | ✅ doc read --node DOC123abc then doc comment reply --comment-key ck-9f8e7d. The answer text exists only inside the document, proving the markdown reached the model. Negative controls all held: duplicate card ⇒ 0 extra runs; mention_source=1 ⇒ not a document task; two distinct links ⇒ not a document task |
| 5 | Concrete group or "*" with requireMention: false obeys both group and sender gates |
✅ {group-eng: false, "*": true} ⇒ consumers at, group --group group-eng, o2o_all. groupPolicy: open + "*": false ⇒ group_all. Unapproved group + ordinary message ⇒ ignored; @ ⇒ group pairing code; after approval ⇒ runs; per-group requireMention: true override ⇒ ignored; open group + unapproved sender ⇒ sender pairing prompt, 0 agent turns |
| 6 | watchTodos baselines, runs once, comment/timestamp noise does not retrigger |
✅ baseline established; new todo ⇒ 1 todo comment add; commentCount/unreadCount/gmtModified/lastModifyTime changed ⇒ no re-run; priority+subject changed ⇒ re-run once. Unapproved creator ⇒ 1 pairing comment across 4+ polls, 0 agent turns; after approval the unchanged todo was processed on the next poll, exactly as documented |
Beyond the test plan:
| Area | Result |
|---|---|
Env allowlist (dws-environment.ts) |
✅ dws saw only DWS_AGENT_PRODUCT, NO_COLOR, PATH, HOME, LANG, LC_*, XDG_*. QWEN_SECRET_TOKEN, AWS_SECRET_ACCESS_KEY, OPENAI_API_KEY, OPENAI_BASE_URL, QWEN_HOME all absent |
| Self-echo suppression | ✅ a message from the authenticated openDingTalkId ⇒ 0 outbound, 0 agent turns |
| History fallback (the 5s incremental check) | ✅ a card the event stream never delivered was recovered and answered; an already-processed card produced nothing |
| Cross-path dedupe | ✅ the same @ message delivered by the realtime stream and matched by two consecutive list-mentions polls ⇒ exactly 1 reply |
| Restart persistence | ✅ after a restart, history returned 2 already-processed cards inside the live window ⇒ 0 re-runs, 0 re-baselining |
[NO_REPLY] |
✅ plain and fenced forms both suppress publication; the agent turn still runs |
| Reaction cleanup on shutdown | ✅ SIGTERM mid-turn ⇒ remove-emoji issued, no reply published |
| Startup guards | ✅ dws 1.0.56 ⇒ requires dws 1.0.57 or newer; profile mismatch ⇒ must exactly match one entry; no openDingTalkId and no prior record ⇒ refuses to connect, but connects when an earlier session recorded one; missing binary ⇒ DWS command failed (ENOENT) |
Terminal stream (retryable: false after ready) |
✅ 1 spawn in 70s, permanently unavailable; not restarting — the R4-3 fix holds |
| Injection shapes | ✅ path-traversal node id and a ;-bearing comment_key are both rejected before any doc read |
| Context truncation | ✅ a 36,032-char document arrived truncated to ≈11,988 chars (MAX_DOCUMENT_CONTEXT_CHARS), tail absent |
| Subprocess hygiene | ✅ across ~10 restarts, no orphaned dws processes; daemon drains cleanly |
| Tests / lint / types | ✅ 228 dws tests (6 files), 44 registry tests, 2 web-shell platform tests; tsc --build and eslint --max-warnings 0 both clean |
| Packaging & release wiring | ✅ npm pack = 21 files / 47.9 kB, dist only, version 0.22.0 aligned with channel-base; build order, clean-artifacts, release allowlist, PUBLISHED_PACKAGES and the integration tsconfig path all updated consistently |
Web Shell / management wiring
The daemon exposes the descriptor (/workspace/channel-types lists dws with all five management fields and chat_thread as the default scope), and the full management loop works: the platform card appears, the form renders every field, and Save → Start brings a managed instance to Connected, spawning real event consumers against the pinned profile.
Findings (all non-blocking)
1. DWS_NOT_SENT_ERROR_CODES — 13 of the 18 entries are unreachable.
runDwsProcess classifies spawn failures inside the execFile callback, but Node only defers five spawn errnos to that callback. From Node 22.22.2's own internal/child_process:
if (err === UV_EACCES || err === UV_EAGAIN || err === UV_EMFILE ||
err === UV_ENFILE || err === UV_ENOENT) {
process.nextTick(onErrorNT, this, err); // -> execFile callback
} else if (err) {
throw new ErrnoException(err, 'spawn'); // -> synchronous throw
}Measured on this host:
| errno | delivery |
|---|---|
ENOENT, EACCES |
execFile callback → wrapped as DwsCommandError / not_sent ✅ |
E2BIG, ENOTDIR, ENAMETOOLONG |
thrown synchronously → escapes as a raw Error: spawn E2BIG |
So E2BIG, EBUSY, EFAULT, EIO, EISDIR, ELOOP, ENAMETOOLONG, ENOEXEC, ENOMEM, ENOSYS, ENOTDIR, EPERM, ETXTBSY never reach classifyDwsCommandFailure. Note the block comment above the table reasons explicitly about resource exhaustion — the fd cases (EMFILE/ENFILE) do work, but ENOMEM does not.
There is no behavioural difference today: the executor's throw still rejects the promise, and every consumer treats a non-DwsCommandError exactly like not_sent (both rethrow). It costs the DWS command failed (CODE) framing in logs, and it would bite the first time not_sent is given behaviour distinct from an unclassified error. A try/catch around the execFile(...) call rejecting with new DwsCommandError(…, classifyDwsCommandFailure(code)) closes it.
2. Outbound reply text is never truncated before it becomes an argv element.
Linux caps a single argument at MAX_ARG_STRLEN = 131,072 bytes. Measured against the real spawn path: 131,000 bytes OK, 131,072 ⇒ spawn E2BIG. A 200 KB agent reply produced:
[Channel:dws-work] DWS message turn failed (attempt 1/5): spawn E2BIG
… attempts 2, 3, 4 …
[Channel:dws-work] dropping a DWS message after 5 failed turns
The failure budget contained it correctly — no infinite loop, which is the property that matters. Two costs: five full agent turns are burned and the user gets silence with no notice; and with sessionScope: chat_thread the oversized assistant turn stays in the shared session, so subsequent unrelated messages in the same chat also failed (Context is too large to send safely after automatic compression) until the poison message exhausted its budget. Likelihood is low — DingTalk's own message cap is far below 128 KiB — but the file already imports truncateCodePoints, so a guard in sendResponseMessage/sendImText is nearly free.
3. Post-ready stream flapping never escalates the backoff.
A stream that reaches [event] ready and then dies without a structured retryable field respawned 18 times in 65 seconds at a flat 2000 ms, indefinitely, one stderr line per cycle. The cause is that startImSource resets state.restartAttempts = 0 on every successful subscribe, so the exponent in scheduleImRestart never grows — the exact shape the R4-3 comment describes, closed for retryable: false but still open for unknown retryability. Since restartAttempts is also reset whenever a message is delivered, dropping the ready-time reset would let the backoff escalate for a flapping stream while leaving healthy streams unaffected.
4. Web Shell empty-state copy omits the new platform. packages/web-shell/client/i18n.tsx:2732 still reads "Configure DingTalk, WeCom, Feishu, GitHub, or GitLab to receive messages in this workspace." while the platform card list right below it now includes DingTalk Workspace. Cosmetic.
5. Informational — what approving a DWS sender grants. An approved DM sender can hand the channel a fabricated notification card naming any documentId and comment_key; the channel reads that document with the account's credentials and posts the agent's answer into it. I confirmed this end-to-end. This is the trust model the code states explicitly ("A direct message can forge a document URL, so authorize the sender before promoting its parsed IDs"), and it is consistent with the channel policy already telling the agent it may drive DWS on the user's behalf — so I'd call it working as designed, not a defect. It is worth one line in dws.md so operators understand that approving a sender grants the bot account's document scope.
One measurement artifact, for completeness: when a turn finished in ~12 ms — faster than the add-emoji round trip — both stopReaction and the late-removal path fired, producing two remove-emoji calls. Removal is idempotent and real API latency makes this unreachable in practice; noting it only because it appeared in the logs.
Not covered
Real DWS CLI / real DingTalk account behaviour (response shapes, rate limits, actual 暗中观察 rendering, real reaction and comment APIs), Windows/macOS, and whether the model honours the untrusted-content framing (the scripted model makes prompt content verifiable but says nothing about a real model's compliance).
Verdict
The implementation matches its documentation and its test plan on every point I could drive locally, the failure paths are genuinely bounded rather than merely untested, and the build/release/UI wiring is complete. From my side this is good to merge; findings 1–4 are small enough to take as follow-ups if you prefer.
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
中文说明
本地验证报告 — 真实环境，PR head 86fc5fd3
我从源码构建了该分支，并在 Linux 上端到端驱动了这个 Channel。Reviewer Test Plan 的全部条目均已确认。没有阻塞性问题；文末有五条非阻塞观察。
环境是怎么搭的
| 组件 | 真实 / 仿真 |
|---|---|
packages/channels/dws + CLI/daemon/Web Shell |
真实 — 从本分支构建（逐包 tsc → esbuild 打包 → Web Shell 用 vite） |
qwen channel start dws-work |
真实 |
qwen serve --channel dws-work（daemon 托管 worker） |
真实 |
| Agent 执行（ACP 会话、配对存储、游标持久化） | 真实 |
qwen channel pairing approve |
真实 |
dws 二进制 |
仿真 — 放在 PATH 上的替身，实现 dws-client.ts 实际驱动的命令契约，并记录每次 argv 与继承到的每个环境变量 |
| 模型 | 仿真 — 脚本化的 OpenAI 兼容服务，回复确定可控、提示词可完整检视 |
本机没有 DWS CLI 1.0.57，也没有真实钉钉登录。该 Channel 的全部外部边界就是 execFile('dws', argv) 和 spawn('dws', ['event','consume',…])，因此在这个边界上放替身，既能跑通全部 Channel 逻辑，又让线上行为完全可观测。规模：1188 次 dws 调用、43 次事件消费者拉起、23 条出站投递、46 次 agent 执行。
防空转对照。 为排除"测试台没跑到你的代码也能全绿"，我修改了构建产物（暗中观察 → MUTANT_REACTION_MARKER，DWS channel policy: → DWS-MUTATED-POLICY-MARKER:）后重跑：两处改动都出现在了实际行为上 —— 表情出现在 add-emoji 调用里，标记出现在模型请求里且原标记消失。因此下面所有观测都来自 PR 自身编译产物。
结果
对应 Reviewer Test Plan 第 1–6 条：
| # | 声明 | 结果 |
|---|---|---|
| 1 | 配置 type: "dws"，用 qwen channel start 启动 |
✅ 连接成功；每次带作用域的调用都固定 --profile qwen-verify；历史每 5 秒轮询并带 5 秒重叠窗口；待办每 30 秒 |
| 2 | 单聊 → 一次配对提示 → 批准 → 执行期间 暗中观察 → 一条最终回复 |
✅ 未批准发送者连发 3 条 ⇒ 1 条配对消息、0 次 agent 执行。批准后：t+0.00s 加表情，t+6.09s 回复，t+6.11s 移除表情。回复经 chat message reply --ref-msg-id … --ref-sender … 回到原消息，并带幂等 --uuid |
| 3 | 未批准账号连续发消息：不重复提示、不触发 Agent | ✅ 已确认（同上） |
| 4 | 文档评论 @ 且勾选通知 → 读文档并在原评论下回复；不勾选则不建任务 | ✅ 先 doc read --node DOC123abc，再 doc comment reply --comment-key ck-9f8e7d。回复内容只存在于文档内部，证明 markdown 确实送达了模型。负向对照全部成立：重复卡片 ⇒ 0 次额外执行；mention_source=1 ⇒ 不作为文档任务；两个不同链接 ⇒ 不作为文档任务 |
| 5 | 具体群或 "*" 配 requireMention: false 时同时遵守群与发送者策略 |
✅ {group-eng: false, "*": true} ⇒ 消费者为 at、group --group group-eng、o2o_all。groupPolicy: open + "*": false ⇒ group_all。未批准群 + 普通消息 ⇒ 忽略；@ ⇒ 群配对码；批准后 ⇒ 正常执行；按群覆盖 requireMention: true ⇒ 忽略；open 群 + 未批准发送者 ⇒ 发送者配对提示、0 次 agent 执行 |
| 6 | watchTodos 建立基线、只执行一次、评论/时间戳噪声不重触发 |
✅ 基线建立；新待办 ⇒ 1 次 todo comment add；commentCount/unreadCount/gmtModified/lastModifyTime 变化 ⇒ 不重跑；priority+subject 变化 ⇒ 重跑一次。未批准创建者 ⇒ 4 轮以上轮询只有 1 条配对评论、0 次 agent 执行；批准后未变更的待办在下一轮被处理，与文档完全一致 |
测试计划之外：
| 项 | 结果 |
|---|---|
环境白名单（dws-environment.ts） |
✅ dws 只看到 DWS_AGENT_PRODUCT、NO_COLOR、PATH、HOME、LANG、LC_*、XDG_*。QWEN_SECRET_TOKEN、AWS_SECRET_ACCESS_KEY、OPENAI_API_KEY、OPENAI_BASE_URL、QWEN_HOME 全部不可见 |
| 自身回显抑制 | ✅ 来自已认证 openDingTalkId 的消息 ⇒ 0 出站、0 agent 执行 |
| 历史兜底（5 秒增量检查） | ✅ 事件流从未投递的卡片被补回并回复；已处理过的卡片不产生任何动作 |
| 跨路径去重 | ✅ 同一条 @ 消息由实时流投递、又被连续两轮 list-mentions 命中 ⇒ 恰好 1 条回复 |
| 重启持久化 | ✅ 重启后历史在有效窗口内返回 2 条已处理卡片 ⇒ 0 次重跑、0 次重建基线 |
[NO_REPLY] |
✅ 裸写与代码块包裹两种形式都抑制发布；agent 仍会执行 |
| 关停时的表情清理 | ✅ 执行中途 SIGTERM ⇒ 发出 remove-emoji，且不发布回复 |
| 启动守卫 | ✅ dws 1.0.56 ⇒ 提示需 1.0.57+；profile 不匹配 ⇒ 提示必须精确匹配一项；无 openDingTalkId 且无历史记录 ⇒ 拒绝连接，但此前会话记录过则可连接；二进制缺失 ⇒ DWS command failed (ENOENT) |
终止性流（ready 后 retryable: false） |
✅ 70 秒内仅 1 次拉起，日志 permanently unavailable; not restarting —— R4-3 的修复成立 |
| 注入形态 | ✅ 路径穿越的 node id、含 ; 的 comment_key 都在任何 doc read 之前被拒绝 |
| 上下文截断 | ✅ 36,032 字符的文档被截到约 11,988 字符（MAX_DOCUMENT_CONTEXT_CHARS），尾部内容不存在 |
| 子进程卫生 | ✅ 约 10 次重启后无残留 dws 进程；daemon 优雅退出 |
| 测试 / lint / 类型 | ✅ dws 包 228 个测试（6 个文件）、registry 44 个、web-shell platform 2 个；tsc --build 与 eslint --max-warnings 0 均干净 |
| 打包与发布接线 | ✅ npm pack = 21 个文件 / 47.9 kB，仅含 dist，版本 0.22.0 与 channel-base 对齐；构建顺序、产物清理、release 白名单、PUBLISHED_PACKAGES、集成测试 tsconfig 路径全部一致更新 |
Web Shell / 管理接线
Daemon 已暴露描述符（/workspace/channel-types 中 dws 带全部 5 个管理字段，默认 scope 为 chat_thread），管理闭环完整可用：平台卡片出现、表单渲染全部字段、Save → Start 后托管实例变为 Connected，并按固定 profile 真实拉起事件消费者。
发现（均非阻塞）
1. DWS_NOT_SENT_ERROR_CODES 的 18 项里有 13 项不可达。
runDwsProcess 在 execFile 回调里做 spawn 失败分类，但 Node 只把 5 个 spawn errno 延迟到该回调。来自 Node 22.22.2 自身的 internal/child_process：
if (err === UV_EACCES || err === UV_EAGAIN || err === UV_EMFILE ||
err === UV_ENFILE || err === UV_ENOENT) {
process.nextTick(onErrorNT, this, err); // -> execFile 回调
} else if (err) {
throw new ErrnoException(err, 'spawn'); // -> 同步抛出
}本机实测：ENOENT、EACCES 走回调，能被包装成 DwsCommandError / not_sent；E2BIG、ENOTDIR、ENAMETOOLONG 同步抛出，以裸 Error: spawn E2BIG 逃逸。因此 E2BIG, EBUSY, EFAULT, EIO, EISDIR, ELOOP, ENAMETOOLONG, ENOEXEC, ENOMEM, ENOSYS, ENOTDIR, EPERM, ETXTBSY 永远到不了 classifyDwsCommandFailure。注意表格上方的注释专门论证了资源耗尽场景 —— fd 相关的 EMFILE/ENFILE 确实有效，但 ENOMEM 无效。
当前没有行为差异：executor 抛出仍会 reject，且现有所有调用方对非 DwsCommandError 的处理与 not_sent 完全一致（都重新抛出）。代价是日志失去 DWS command failed (CODE) 的框定；一旦将来给 not_sent 赋予区别于"未分类错误"的行为，这 13 项就会走错分支。在 execFile(...) 外包一层 try/catch，以 new DwsCommandError(…, classifyDwsCommandFailure(code)) reject 即可闭合。
2. 出站回复文本在成为 argv 元素前从未截断。
Linux 单个参数上限 MAX_ARG_STRLEN = 131,072 字节。对真实 spawn 路径实测：131,000 字节可以，131,072 ⇒ spawn E2BIG。一条 200 KB 的 agent 回复产生：
[Channel:dws-work] DWS message turn failed (attempt 1/5): spawn E2BIG
… attempts 2, 3, 4 …
[Channel:dws-work] dropping a DWS message after 5 failed turns
失败预算正确地兜住了它 —— 没有无限循环，这才是关键性质。两处代价：白白消耗 5 次完整 agent 执行，且用户只得到静默、没有任何提示；另外在 sessionScope: chat_thread 下，这次超大回复留在了共享会话历史里，导致同一会话中无关的后续消息也一并失败（Context is too large to send safely after automatic compression），直到该毒消息耗尽预算被丢弃。发生概率不高 —— 钉钉自身的消息上限远低于 128 KiB —— 但该文件已经引入了 truncateCodePoints，在 sendResponseMessage/sendImText 加个护栏几乎零成本。
3. ready 之后的流抖动不会升级退避。
一个到达 [event] ready 后死掉、且错误里没有结构化 retryable 字段的流，65 秒内以恒定 2000 ms 重启了 18 次，可以无限持续，每次一行 stderr。原因是 startImSource 在每次订阅成功后都会 state.restartAttempts = 0，于是 scheduleImRestart 里的指数永远长不起来 —— 正是 R4-3 注释所描述的形态，对 retryable: false 已闭合，但"可重试性未知"的分支仍然敞开。由于 restartAttempts 在收到消息时也会重置，去掉 ready 时的那次重置即可让抖动流的退避正常升级，而健康流不受影响。
4. Web Shell 空状态文案没跟上新平台。 packages/web-shell/client/i18n.tsx:2732 仍写着 "Configure DingTalk, WeCom, Feishu, GitHub, or GitLab to receive messages in this workspace."，而紧邻其下的平台卡片列表已经包含 DingTalk Workspace。属外观问题。
5. 提示性 —— 批准一个 DWS 发送者到底授予了什么。 一个已批准的单聊发送者可以递给 Channel 一张伪造的通知卡片，指定任意 documentId 和 comment_key；Channel 会用该账号凭据读取那份文档，并把 agent 的回答发到其中。我端到端确认了这一点。这正是代码里明确写下的信任模型（"A direct message can forge a document URL, so authorize the sender before promoting its parsed IDs"），也与 Channel 策略中已经允许 agent 代用户驱动 DWS 的设定一致 —— 所以我认为这是按设计工作，不是缺陷。建议在 dws.md 补一句，让运维方明白批准一个发送者等于授予机器人账号的文档访问范围。
一处测量副产品，供完整性参考：当一次执行在约 12 ms 内结束（快过 add-emoji 的往返）时，stopReaction 与"迟到清理"路径会同时触发，产生两次 remove-emoji。移除是幂等的，真实 API 延迟下这基本不可达；仅因为它出现在日志中而记录。
未覆盖
真实 DWS CLI / 真实钉钉账号的行为（响应结构、限流、暗中观察 的真实呈现、真实表情与评论 API）、Windows/macOS，以及模型是否真的遵守"不可信内容"的框定（脚本化模型让提示词内容可验证，但不能说明真实模型的遵从度）。
结论
在我本地能驱动的每一个点上，实现都与它的文档和测试计划相符；失败路径是真正有界的，而不只是没被测到；构建、发布与 UI 接线也完整。就我而言可以合并；发现 1–4 如果你更倾向于后续处理，也完全可以作为 follow-up。
|
@qwen-code /triage |
5 participants
What this PR does
Adds a built-in DingTalk Workspace channel that uses an existing authenticated DWS CLI profile. It supports direct messages, @mentions and configured ambient groups, DingTalk document-mention notifications, native todo changes, source-scoped sessions, final replies to the originating message, document comment, or todo, and a temporary
暗中观察reaction while accepted message tasks run.The channel reuses the shared sender and group policy model, defaults managed instances to pairing, pins one DWS profile for the channel lifetime, filters the DWS subprocess environment, persists delivery targets, processed-message cursors, and todo fingerprints, suppresses outbound echoes and duplicate pairing notifications, and exposes the channel through CLI, daemon/Web Shell management, build, release, and documentation paths. Native todo watching is opt-in, baselines existing pending todos, polls executor assignments every 30 seconds, and writes the final response back as a todo comment without reacting to its own comment metadata.
Why it's needed
The existing DingTalk channel is intentionally a dedicated application-bot adapter. Users who already authenticate DWS need a separate account-backed channel, analogous to the GitHub channel, so Qwen Code can receive workspace events and respond using the existing DWS login without requiring a bot application.
Reviewer Test Plan
How to verify
type: "dws"channel with pairing policies, and start it withqwen channel start <name>.暗中观察while the task runs and get one final reply."*"withrequireMention: falseand verify ordinary group messages obey both group and sender policy gates.watchTodos: true, start the channel once to establish a baseline, then assign a new native todo to the authenticated account. Confirm it runs once and the final response appears as a todo comment; changing only comments or modification timestamps must not start another task.Evidence (Before & After)
Before: Qwen Code had no built-in channel for an existing DWS login; only the dedicated DingTalk bot adapter was available.
After: A locally authenticated DWS profile can start the channel, receive direct/group/document notifications, poll opted-in native todo changes, show a working reaction for messages, and route the final response back to the originating surface. Live validation also confirmed that repeated automated messages produce one pairing notification per pending request instead of an outbound loop.
Tested on
Environment (optional)
macOS, Node.js 22+, DWS CLI 1.0.57, local TypeScript development checkout.
Risk & Scope
dingtalkchannel remains separate and unchanged.Linked Issues
N/A
中文说明
本 PR 做了什么
新增内置钉钉工作空间 Channel，复用已经登录的 DWS CLI profile。它支持单聊、@ 消息、按配置接收的普通群消息、钉钉文档 @ 通知、原生待办变化、按来源隔离会话，并把最终回复写回原始消息、文档评论或待办；消息任务执行期间还会添加
暗中观察表情。该 Channel 复用统一的发送者和群聊策略，管理界面中新建实例默认使用 pairing；启动时固定唯一 DWS profile；限制 DWS 子进程可继承的环境变量；持久化投递目标、已处理游标和待办指纹；抑制自身回显和重复配对通知；并完成 CLI、daemon/Web Shell、构建、发布和文档接线。原生待办监听默认关闭；开启后会先为已有未完成待办建立基线，每 30 秒轮询一次执行者待办，并把最终回复写入待办评论，同时忽略自身评论产生的元数据变化。
为什么需要
现有钉钉 Channel 专门服务于独立应用机器人。已经使用 DWS 登录的用户需要一个独立的账号型 Channel，像 GitHub Channel 一样，让 Qwen Code 无需新建机器人应用即可接收工作空间事件，并通过现有 DWS 登录回复。
Reviewer 测试计划
如何验证
type: "dws"且策略为 pairing，然后运行qwen channel start <name>。暗中观察，并只收到一次最终回复。"*"配置requireMention: false，确认普通群消息同时遵守群策略和发送者策略。watchTodos: true，首次启动建立基线后，再把一个新的原生待办指派给已登录账号。确认待办只执行一次，最终回复出现在待办评论中；仅评论或更新时间变化时不应再次触发。前后对比证据
Before：Qwen Code 没有复用现有 DWS 登录的内置 Channel，只支持独立钉钉机器人适配器。
After：本机已登录的 DWS profile 可以启动 Channel，接收单聊、群聊和文档通知，按配置轮询原生待办变化，为消息任务显示接手表情，并把最终回复路由回原始入口。现场验证还确认，自动账号连续发消息时，每个待批准请求只发送一次配对提示，不再形成出站回环。
验证平台
环境
macOS、Node.js 22+、DWS CLI 1.0.57、本地 TypeScript 开发工作区。
风险与范围
dingtalkChannel 保持独立且不变。关联 Issue
无