Repository navigation
Conversation
Keep explicit destinations intact through the delivery hub. Add an opt-in owner-only Telegram link that shares the existing page lane, routes questions to the active surface, and reports queued cancellations without clearing history. Include synthetic regression fixtures and configuration documentation. No credentials, local configuration, runtime data, or new dependencies are included.
Hold per-conversation admission through cancellation and child cleanup. Cancel running work and detach queued work before awaiting UI reports. Preserve captured asker context and refuse questions from expired linked turns. Add native child-announcement and backpressure regressions plus cross-surface question tests.
Preserve attachment notices and later parts for HTTP 413 responses, restore plain-text fallback for BadRequest, and expose the resumable delivery error through the channel contract. Document silent page cancellation. Co-authored-by: OpenAI (gpt-6-astra) <noreply@openai.com>
Keep unsupported configured links from aborting the gateway without fallback to another conversation. Use HTTP status before SDK error conversion and consolidate reported cancellation in the lane drain. Co-authored-by: OpenAI (gpt-6-astra) <noreply@openai.com>
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: fix the three inline issues and reconcile the head commit's Claude attribution with AGENTS.md section 3.3.
I reviewed the full github/main...HEAD diff after refreshing github/main, plus the affected callers, scheduler/RPC/channel architecture, commit history, backward compatibility, tests, AGENTS.md, CONTEXT-MAP.md, and CONTEXT.md. I found three concrete blockers inline. Separately, commit f32199793 uses Co-Authored-By: Claude Code <noreply@anthropic.com>; AGENTS.md section 3.3 requires Claude (<actual-model-id>) when Claude contributed (or no Claude trailer if it did not).
Verification: after uv sync --all-packages --extra channel-telegram, the affected suite passed with 334 tests and 15 subtests. Its first pre-extra run could not import the declared workspace package; an intermediate parallel run had one 0.1-second timing failure which passed twice in isolation, and the final unchanged suite was green. The existing stored-turn contract case fails consistently in isolation, matching GitHub's red unit shard: uv run pytest 'tests/test_agent_loop_empty_recovery.py::test_a_stored_turn_reads_the_message_tools_live_verdict[sent_naming_another_chat]' -q -o addopts='' gives 1 failed. I found no weakened tests or unrelated architecture/dependency/asset violations in the diff.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: the two unchanged linked-chat defects and the noncompliant Claude attribution still need correction.
The new delta correctly fixes the stored-turn contract failure: chat-only destinations are refused, true cross-target sends remain unanswered on the originating turn, the Python/schema/TypeScript descriptions agree, and the generated client matches the contract. I replied on and resolved that thread. No new finding arose from the delta.
The refused-input typing leak and overlapping /stop race remain unchanged, so their two threads stay open. Commit f32199793 also still carries the generic Claude Code trailer instead of the actual model attribution required by AGENTS.md section 3.3.
Coverage this round included the new delta, its message-tool/session/RPC callers, backward compatibility, commit history, generated schema, the prior findings, and the applicable AGENTS.md and CONTEXT contracts; I found no weakened tests or unrelated architecture/dependency/asset issue. Verification: the affected suite passed 815 tests and 15 subtests; tests/test_rpc_schema_match.py passed 533 tests; and node scripts/gen-rpc-client.mjs --check reported that generated.ts matches the 255-method contract.
Windows compatibility review — PR #1195Reviewed head Findings1. Low (Windows-specific, reproduced): with a binding file present on Windows, every allowed inbound Telegram message leaves the adapter's typing indicator running until the gateway restarts. On an unsupported platform Windows-specific surface that behaves correctly
Verification (native Windows 11, isolated detached worktree
|
The rebased branch keeps main's refined question-broker cancellation check, so its richer tests replace the PR's simpler expectation. A stale named cancel now answers with busy from #1078.
The message tool now routes a cross-target send through the gateway and refuses a destination that names only one of channel or chat. A turn is answered only by a send on its own target, so the stored-turn verdict test gains a refused single-field case and a queued cross-target case, both expecting a Silent Turn.
The message tool answers a turn only when the send reaches that turn's own target. State it in the sent_in_turn docstring, the turn-shape contract and the published replied_via_tool description, and regenerate the TypeScript client from the schema.
A swallowed Telegram request still started the adapter's typing loop, and an overlapping /stop cleared the first stop's state and raised SchedulerDrainingError. End the adapter turn on every consumed-without- turn path, including the unsupported-platform inbound refusal, and make the stop guard answer duplicates without touching the owner's flag.
The replied_via_tool description changed in rpc-schema/openrpc.json; the TUI client is generated from the same contract.
95a1a41 to
7c6b03e
Compare
|
The three inline blockers are fixed at On the attribution note: the trailer was removed from that commit (now Affected suite after the fixes: 198 passed, 7 subtests, plus the two new regressions by name. The full CI run is restarting on the new head. |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The new delta fixes both remaining code findings. Refused Telegram requests now terminate adapter typing on the linked-refusal and unsupported-platform paths, and overlapping /stop requests are answered without clearing the first stop's ownership or leaking SchedulerDrainingError. The stored-turn contract and both generated clients remain aligned. I replied to and resolved the two repaired threads; all threads I opened are now resolved.
I also accept the owner's explicit repository ruling that this clone carries no Claude trailer; the rewritten commit removes the generic trailer, so the attribution objection no longer stands.
Coverage included the new delta, the full prior diff context, affected callers, commit history, backward compatibility, tests, generated contracts, AGENTS.md/CONTEXT rules, and the existing review record. I found no weakened test or new architecture, dependency, or asset issue. Verification: 1,414 tests and 15 subtests passed across the affected runtime, gateway, Telegram, agent-loop, and schema suites. The web client reported generated.ts matches the contract (255 methods), and the TUI client reported generated.ts in sync. Hosted CI was still running at review time, with completed checks green.
Windows compatibility review — PR #1195Reviewed head FindingsWindows-specific: none — the typing-indicator leak reported at The Verification (native Windows 11, isolated detached worktree
|
|
Independent review of this PR at head `7c6b03e32`, run against `origin/main` (`8448cc628`) in an isolated worktree. Findings are evidence-backed; anything unverified is marked as such. PR #1195 审核报告
1. 这个 PR 想做什么这是对 #1099 的接手版本（原作者 @5mhbqxq22q-arch 的 6 个 commit 保留作者信息），rebased 到当前 main 后又追加了 review 修复。它实际包含三件事：
规模上 2487 行里约 1770 行是测试、约 70 行是文档，生产代码约 620 行；对“一个路由修复”来说确实大，但这是一个新功能 + 两处协议级加固，规模本身不是问题。 2. 判断需要修改（1 条应修问题 + 若干建议）。 代码质量、失败关闭的思路、测试密度都明显高于本仓库平均水平，CI 在 head 上全绿（unit ×4、ui rpc contract、coverage gates、commit messages 等）。但 F1 是“已修过的 typing 泄漏”在同一功能里的残留：当 linked turn 没有向 Telegram 发出任何文本就结束时（page Stop、runtime reload、空回复），DM 的 typing 指示会一直转下去。我已用真实 3. 正确的部分
4. 问题与风险F1（中，应修）linked turn 没有发出文本就结束时，Telegram typing 指示不会停
def wrap_sink(self, page_sink, outlet):
async def sink(event):
if isinstance(event, (TurnEnded, TurnFailed)):
active = self.active.get(event.conversation_id)
if active is not None and active[0] == event.turn_id:
self.active.pop(event.conversation_id, None)
self.sources.pop(event.conversation_id, None)
if isinstance(event, TurnFailed) and not event.cancelled:
await self.hub.dispatch(Text(content="Sorry, I encountered an error.", source=active[1]))
self.agent.notify_turn_complete()对照 gateway 自己的 sink，它在
elif isinstance(event, TurnEnded) and not event.explicit_reply:
...
for channel_name, chat_id in dict.fromkeys((person.channel, person.chat_id) for person in people):
task = asyncio.create_task(end_channel_turn((channels or {}).get(channel_name), chat_id))而
实测（PR head
修复验证：在 F2（中低，需维护者决策）跨目标发送新增了“im 用户之间互发”的能力，PR 未说明
async def queue_target(channel_name: str, chat_id: str, content: str, media: list[str]) -> str:
target = channels.get(channel_name)
if (
target is None
or not target.is_running
or chat_id not in getattr(target.config, "allow_from", ())
or not target.intake.is_allowed(chat_id)
):
raise ValueError("Destination unavailable or not authorized")
我不认为它一定要在合并前改（它落在操作员的 allowlist 信任边界内），但这是一个应该被明确记录/由维护者确认的能力扩展。可选收敛方式：跨目标只允许非 IM 来源的 turn（cron/page/子代理），或单独引入 F3（低，UX）
|
| 内容 | 结果 |
|---|---|
test_gateway_linked_chat + _boundaries + _cancel + test_message_tool_destination + test_channels_telegram_delivery + test_channels_contract |
105 passed, 2 skipped（skip 为 POSIX ownership），8 subtests，125 s |
| gateway/spine/scheduler/rpc/turn/question/channels 等 16 个受影响模块 | 727 passed, 1 skipped，186 s |
test_rpc_schema_match.py |
533 passed |
test_channels_outlet / test_rpc_message_tool_route / test_message_tool_turn_local / test_cli_gateway_page |
25 passed |
test_a_stored_turn_reads_the_message_tools_live_verdict |
4 passed（4 个参数化用例） |
ruff check / ruff format --check（30 个改动 py 文件） |
全部通过 |
scripts/check_commit_messages.py origin/main..HEAD |
通过 |
git diff --check |
干净 |
GitHub CI @ 7c6b03e32 |
unit 1–4、ui rpc contract、coverage gates、python lint、page checks、TUI、Windows ACP 等全部 pass |
变异验证：去掉 AskViaTool 的 create_task(context=...) |
2 个 linked-chat boundary 测试变红 |
| 探针：真实 adapter + linked spine 的 typing 生命周期 | 复现泄漏（见 F1）；补丁后三例全部修复 |
|
Blocking: linked turns that end without Telegram output must stop the adapter's typing task. F1 holds on the current head. I independently sent a silent The focused linked-chat suites remain green ( |
Summary
This PR adopts #1099, originally authored by @5mhbqxq22q-arch. The six contributing commits keep their original authorship. The branch was rebased onto current main, with follow-up commits that align the stored-turn verdict and the published contract with the cross-target rule, and that close the two gateway blockers found in review: a swallowed request left the adapter's typing loop running, and overlapping
/stopcommands shared one stop-state flag. The original pull request could not be updated in place because maintainer edits are disabled on the contributing fork, so this PR carries the work forward and closes #1099 on merge.An explicit
message(channel=..., chat_id=...)currently resolves the requested destination but calls the active turn's two-argument send callback. That callback still belongs to the current surface. A tool result can therefore claim a cross-channel send while the message goes to the page stream instead.This change keeps explicit destinations intact through the native delivery hub. Explicit destination sends are a gateway-wide change; they do not require the optional Telegram project link. Changing either destination requires both fields; invalid, unconfigured, stopped or non-explicitly-allowlisted targets fail closed. Queue acceptance is reported as pending delivery. Telegram text and media sends record the returned message ID, and final failures reach the hub without exposing provider exception text.
It also adds an opt-in, single-owner Telegram private-chat link to an existing page conversation. The page scheduler owns the shared lane and history; replies and questions retain their originating turn's surface. Cross-surface input queues separately instead of inheriting another turn's reply address.
/stopcancels active and queued work without clearing history. Other linked-DM slash commands are refused, and background output is not automatically mirrored.The page Stop button cancels the active linked turn while preserving queued page input. It keeps the live DM input bubble and leaves cancellation reporting to the RPC path. Page Stop and runtime reload do not send a separate Telegram cancellation notice; Telegram
/stopsends its own acknowledgement.The link is disabled unless an owner-only
telegram-project-link.jsonexists in the configured workspace. Its current implementation requires POSIX ownership checks. An absent file preserves existing behavior on other platforms. On Windows or another unsupported platform, a present file logs a warning and refuses Telegram input to the agent without aborting the gateway or falling back to a standalone conversation. The page and other channels remain available; removing the file and restarting restores standalone chats. Setup, limits and rollback are documented indocs/telegram-project-link.md.Both stop paths use
Lane.canceland the sameLane.drain_pendingimplementation. Its reporting mode shares queued-cancellation reporting withLane.cancel_turn, but stops running work and detaches queues before awaiting UI delivery. It holds per-conversation admission through child cleanup so cancellation announcements cannot restart the host. Captured askers retain their originating context; expired DM askers are refused rather than routed into a later page or DM turn. Calling-task cancellation now propagates throughQuestionBroker; timeout, connection closure andcancel_all()still return the configured default. Existing tests for cancellation and Telegram send failure were updated for those deliberate behavior changes.For Telegram delivery, the adapter classifies permanent HTTP failures before the SDK replaces their status with arbitrary error descriptions. SDK-specific BadRequest, auth and conflict errors retain their types; rate limits, server errors and transport failures remain retryable regardless of description wording. A rejected middle attachment gets a basename-only notice and does not prevent later parts from being delivered. HTML rejected with
BadRequestis tried as the original plain text. The resumable delivery exception is exposed throughchannels.contract, with the shared retry budget documented on the outlet.Type
Verification
Rebased onto current main and tested on Linux with Python 3.12.13 in an isolated worktree. The environment was created with
uv sync --frozen --all-extras; the rebase applied without textual conflicts. No live bot, provider or production service was called.The 27-module focused selection passed 897 tests and 15 subtests:
The four trajectory/RPC integration modules passed 303 tests:
Additional checks:
The agent-loop, schema-match and destination modules affected by the follow-up fix passed 1031 tests and 8 subtests (
tests/test_agent_loop_empty_recovery.py,tests/test_rpc_schema_match.py,tests/test_message_tool_destination.py).The gateway review fixes passed 198 tests and 7 subtests across the five affected modules;
test_a_refused_request_ends_the_adapter_turnandtest_an_overlapping_stop_keeps_the_first_stop_statealso pass by name.ruff checkandruff format --checkpassed on all 26 changed Python files.ty checkpassed on the 16 changed runtime files.scripts/check_commit_messages.py,scripts/check_large_files.pyandscripts/check_source_language.pypassed over the branch range.Follow-up commits align the branch with current main: the question-broker cancellation tests carry main's richer check, a stale named cancel answers with busy, and a cross-target send is not this turn's reply.
node ui-web/scripts/gen-rpc-client.mjs --checkpasses after thereplied_via_tooldescription update.Not checked locally at this head: the full repository suite, native Windows/macOS execution, live Telegram or LLM delivery, and browser acceptance. Hosted CI remains the gate for those. The original head's verification, independent review and Windows probe are recorded in fix(*): route explicit messages and link a Telegram DM to a page session #1099.
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
Explicit cross-target sends now require an explicitly listed destination; a wildcard allowlist alone does not authorize them. Same-target streaming remains unchanged. Permanent Telegram destination refusals terminate without transient retries. Permanent attachment failures produce sanitized basename-only notices. Retryable partial delivery resumes at the unfinished component under the existing hub retry budget, without replaying acknowledged components or resetting that budget. An HTML send rejected with
BadRequesttries its original plain text, including when table padding exceeds the rendered-length limit. Transport errors and forbidden destinations do not trigger that fallback. Lost acknowledgements remain ambiguous; this does not guarantee exactly-once delivery. Permanent errors handled by the adapter do not increment the delivery hub'sdroppedcounter; a zero counter is not a delivery receipt. Receipt logs contain chat and message identifiers and should be treated as private operational data.The optional link admits one verified private sender and refuses groups, topics and other Telegram senders while configured. It does not merge or delete existing transcripts. It adds no dependency, RPC endpoint, credential copy or schema migration. Public fixtures use synthetic identities; no local configuration, credentials, transcripts or project data are included.
Rollback: remove the binding file and restart at a safe checkpoint to restore independent Telegram conversations. Reverting the PR restores the previous message behavior; the old gateway ignores the binding file.
Related Issues
Closes #1099.
Related work: #1073 and #1078 landed the question-broker and scheduler cancellation semantics this branch builds on; #1081 landed the delivery-fold behavior. This contribution changes no CI policy or live installation.