2 Commits

Author	SHA1	Message	Date
Daoyuan Li	51bb19fa1b	fix(channels): drop redundant GitHub review-comment webhook fan-out (#4131) ... * fix(github): drop redundant pull_request_review_comment fan-out noise GitHub fires one pull_request_review_comment webhook per inline comment attached to a pull_request_review submission, in addition to the single pull_request_review event for the review itself. A bot reviewer like CodeRabbit commonly leaves 20-30 inline comments per review, flooding the webhook with near-duplicate deliveries that carry nothing an agent doesn't already have -- it fetches every inline comment itself via `gh api` when it processes the parent pull_request_review event. Filter these out in fanout_event() before the registry lookup / per- agent loop, since the redundancy is a property of the event itself, not of any specific agent binding. A companion comment is identified by pull_request_review_id being set (it belongs to a review) and in_reply_to_id being absent (it is not itself a reply within an existing thread -- that case is a genuine new interaction and must still fire). * fix(channels): scope review-comment suppression to bindings that also see the review The redundant pull_request_review_comment filter suppressed every companion comment unconditionally, before the registry lookup even ran. That premise only holds for a binding that also subscribes to pull_request_review on the same repo -- events are opt-in per binding, so a binding registered for pull_request_review_comment alone never receives the parent review event and the companion comments were its only delivery of the review's inline content. Suppressing those too was a silent, total loss for that binding, not noise reduction. Move the check into the per-agent loop so it only suppresses a matched binding's companion comment when that same binding also has an active pull_request_review trigger on this repo, reusing the existing registry lookup rather than re-walking bindings by hand. Bindings subscribed to pull_request_review_comment alone now always fire, matching the opt-in-per-binding contract documented in triggers.py. * fix(channels): close require_mention gap in review-comment redundancy gate willem-bd's second review round found that the per-binding redundancy gate (commit b3791b33) treated a binding as "covered" purely by checking whether it also registers a `pull_request_review` trigger on this repo, ignoring whether that trigger itself requires a mention. A `pull_request_review_comment` payload never carries the paired review's own top-level body, so there is no way to verify from a comment delivery whether that trigger's own `require_mention` check would actually pass. A human `@mention` living only in one inline comment (not the review summary) could therefore be lost twice: the review event filtered out by its own `no_mention` gate, and the one inline comment that carries the mention dropped here as "redundant" -- the same silent-loss shape as the original bug, through a narrower path. The gate now also requires the paired `pull_request_review` trigger's resolved `require_mention` to be false before treating it as coverage, trading a small amount of residual redundancy (an extra companion delivery when the review would have fired anyway) for zero silent loss. While in the same code, also addressed two smaller review notes: - The redundant-comment skip reason now prefers the companion's own trigger verdict when that verdict is also a skip (e.g. its own `require_mention` independently fails), instead of always reporting the generic `redundant_review_comment` label. - `_pr_review_prompt` now tells the agent to fetch a review's inline comments via `gh api .../reviews/{id}/comments` -- the redundancy gate's suppression is only genuinely redundant if the agent actually recovers that content from the parent review event, and nothing previously told it to (zhfeng's review). Tests: reproduces willem-bd's exact scenario (dual-subscribed binding, require_mention on the review trigger, mention present only in the inline comment) and confirms it fails on the prior code and passes with the fix; adds a multi-agent-independence test and a skip-reason-precedence test; adds prompt tests locking the new fetch-hint text and its missing-id guard. Fail-before/pass-after verified via patch-file revert (not stash, to avoid colliding with sibling worktrees).	2026-07-14 22:56:50 +08:00
Zheng Feng	dcb2e687d5	feat(channels): add GitHub as a webhook-driven channel (#3754) ... * feat(channels): add GitHub event-driven agents (#3754) Add a webhook-driven GitHub channel with fail-closed webhook routing, deterministic per-agent PR/issue threads, mention-gated trigger fan-out, GitHub App token injection for sandboxed gh/git commands, and backend/AGENTS.md documentation. * fix(llm-middleware): classify bare IndexError as transient Upstream chat providers occasionally return 200 OK with an empty generations list (observed against Volces "coding" on ark.cn-beijing.volces.com). When that happens, langchain_core.language_models.chat_models.ainvoke raises ``IndexError: list index out of range`` at ``llm_result.generations[0][0].message`` and kills the run. Treat a bare IndexError reaching the middleware as a transient upstream-payload glitch and route it through the existing retry/backoff path instead of failing the whole agent run. The retry budget and backoff schedule are unchanged. Adds three regression tests covering the classifier and both the recover-on-retry and exhausted-retries paths. * fix(runtime): ignore stale LLM fallback markers from prior runs When a run on a thread ends with the LLM-error-handling middleware emitting a `deerflow_error_fallback`-marked AIMessage (e.g. after the IndexError empty-generations classification fix lands), that message is persisted to the thread's checkpoint as part of the messages channel. LangGraph replays the full message history in `stream_mode="values"` chunks, so every subsequent run on the same thread re-streams the stale fallback marker — and the worker's chunk scanner faithfully picks it up, flipping `RunStatus.success` to `RunStatus.error` for runs that themselves had no LLM failure at all. Snapshot the set of pre-existing message ids from the pre-run checkpoint and thread it through `_extract_llm_error_fallback_message` / `_try_extract_from_message` as a filter. Markers on history messages are ignored; markers on fresh messages produced during this run still trip the error path. Falls back to an empty set when the checkpointer is absent or the snapshot can't be captured, preserving the prior behavior on first-run / no-state paths. Adds unit tests for the new filter (helper-level and `_collect_pre_existing_message_ids`) plus an integration test exercising the full `run_agent` path with a stale history checkpointer. * fix(channels): make github channel fire-and-forget to avoid httpx.ReadTimeout on long runs GitHub agent runs (clone -> edit -> test -> push -> PR) routinely exceed the langgraph_sdk default 300s read deadline. The manager's runs.wait call kept an HTTP stream open for the entire run lifetime, so the long run blew up with httpx.ReadTimeout and the outer except branch then released the dedupe key and emitted a false 'internal error' outbound. The GitHub channel's outbound send is log-only by design: agents post to the issue/PR via the gh CLI in the sandbox when they choose to comment or create a PR. There is nothing for the manager to ferry back, so the long-poll was pure overhead. This change adds ChannelRunPolicy.fire_and_forget (default False) and sets it True for the github channel. When fire_and_forget is True, _handle_chat dispatches via client.runs.create (short POST, returns once the run is pending) instead of client.runs.wait, and skips the response-extraction + outbound-publish block. ConflictError on a busy thread still trips the standard THREAD_BUSY_MESSAGE path so behavior on the busy case is preserved for any future non-github fire-and-forget channel. Other (non-github) channels are unchanged: their policy defaults fire_and_forget=False and they continue to dispatch via runs.wait. Adds 6 regression tests in tests/test_channels.py::TestGithubFireAndForget: - Default ChannelRunPolicy.fire_and_forget is False. - The github policy registers fire_and_forget=True. - github inbound calls runs.create, not runs.wait, with the right kwargs. - github inbound publishes no outbound on success. - ConflictError from runs.create still emits THREAD_BUSY_MESSAGE. - Non-github channels (slack) still dispatch via runs.wait. * test(lead-agent): accept user_id kwarg in skill-policy test stubs The two GitHub-channel tests added in #3754 stubbed _load_enabled_skills_for_tool_policy with a lambda that only accepted `available_skills` and `app_config`, but the real function (and its call site in agent.py) also passes `user_id`. This raised TypeError on every run, failing backend-unit-tests. Add `user_id=None` to match the three sibling stubs in the same file. * refactor(gateway): disambiguate context-key set names The two frozensets _INTERNAL_ONLY_CONTEXT_KEYS and _CONTEXT_ONLY_KEYS shared a confusable "CONTEXT_ONLY" token in different orders, and the first broke the _CONTEXT_<X>_KEYS pattern of its sibling _CONTEXT_CONFIGURABLE_KEYS. Rename to make the distinct axes explicit: _CONTEXT_INTERNAL_CALLER_KEYS - WHO: internal callers (scheduler) only _CONTEXT_RUNTIME_ONLY_KEYS - WHERE: runtime context only, never configurable Pure rename, no behavior change.	2026-07-04 22:56:24 +08:00