feat(slack): add [slack].streaming toggle (send-once mode)

[dogzzdogzz]

What problem does this solve?

The Slack adapter always streams replies live — native streaming (chat.startStream + assistant.threads.setStatus) in assistant_mode (default), or a post+edit placeholder otherwise. Great UX for a single bot, but in multi-agent threads it drives a phantom-turn storm:

1. user → bot A
2. bot A → bot B (bot A's reply @-mentions bot B)
3. bot B → bot A

At step 2 no other bot has posted in the thread yet, so bot A still treats it as a single-bot thread and streams its reply via post+edit. Each edit emits a message_changed event → bot B's app_mention re-fires once per edit, so bot B spawns a full agent turn for each intermediate/partial state instead of bot A's single settled message (observed: 2 real messages → 5 turns, one acting on a mid-stream fragment <@U…> -).

The existing other_bot_present gate only disables streaming after another bot has posted — it cannot help on the kickoff message that first mentions bot B. There is no operator switch to deterministically force send-once.

Closes #1114. Addresses the phantom-turn bug #1103 — chosen over the #1104 app_mention debounce approach: posting one final message removes the re-fire at the source rather than dedup'ing after.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491969620754567270/1515891969401032715

At a Glance

            Before                                  After (streaming = false)
 ┌────────────────────────────────┐      ┌────────────────────────────────┐
 │ alone in thread → stream reply  │      │ streaming=false → send-once     │
 │   post+edit: N message_changed  │      │   one chat.postMessage per turn │
 │   each edit re-fires app_mention│      │   no native, no post+edit       │
 │   → bot B gets N phantom turns  │      │   → bot B gets ONE app_mention  │
 └────────────────────────────────┘      └────────────────────────────────┘
   default streaming = true → unchanged behaviour for everyone else

Proposed Solution

Add one optional boolean to [slack], default true (preserves current behaviour):

[slack]
streaming = true     # default — live streaming, as today
# streaming = false  # always post a single final message (send-once)

When streaming = false:

• No native streaming (no chat.startStream / streamed assistant.threads.setStatus).
• No post+edit placeholder.
• The adapter posts exactly one final chat.postMessage per turn.
• Independent of assistant_mode — the assistant status API (set-status / reaction) is unaffected; only the message streaming path is gated.

Gating is a single AND-in at the two decision points:

• use_streaming() → self.streaming && !other_bot_present
• uses_native_streaming() → self.streaming && self.assistant_mode && !other_bot_present

Mirrors [gateway].streaming in concept; the default deliberately differs (true for Slack to preserve the current streaming UX, vs false for gateway).

Validation

• streaming = true (default): identical behaviour to today — native streaming in single-bot assistant_mode threads, post+edit otherwise, other_bot_present still disables streaming when another bot has posted.
• streaming = false: use_streaming() and uses_native_streaming() both return false even when alone; uses_assistant_status() is unaffected (status API independent of the streaming switch).

Testing

• cargo test --bin openab — passing, incl. extended assistant_mode_gates_status_and_native_streaming cases asserting streaming=false forces send-once (no post+edit, no native) while leaving assistant status intact.
• cargo clippy --all-targets -- -D warnings — clean.
• Helm: charts/openab/tests/configmap_test.yaml covers streaming omitted (default, key absent) and streaming: false (renders streaming = false).

Scope

Slack-only. src/discord.rs and src/gateway.rs keep their own send paths (Discord has no post+edit streaming, so the multi-agent re-fire concern is Slack-specific). Default true → zero behaviour change for existing deployments.

---

Update (2026-06-18) — narration trimming via a dedicated narration_display flag

Follow-up commit 733abf5 added "send-once delivers only the final answer block" — dropping the inter-tool narration ("let me check… / now reading…") that otherwise leaks into the message. Per maintainer discussion this is being decoupled from streaming into its own flag rather than implicitly coupled to send-once:

• New narration_display: bool, default false, on the adapter config.
• Effective rule: in send-once delivery, narration is trimmed unless narration_display = true. (Streaming shows narration live regardless, so the flag only bites in send-once.)
• default false preserves 733abf5's behaviour (trim by default); set true to keep the full inter-tool text.
• Implemented in the shared adapter layer, so it applies to any send-once turn — Slack streaming=false, Slack/Discord multi-bot threads, and gateway platforms — not just Slack. This supersedes the per-backend narration filters in fix(agy-acp): filter narration based on OPENAB_TOOL_DISPLAY #1030 (agy-acp) and feat(acp): filter opencode planning narration from responses #1032 (opencode).

This also resolves the "is streaming overloaded?" review point — two independent switches: streaming = stream vs send-once; narration_display = include inter-tool narration or only the final block. (Supersedes the earlier "Scope: Slack-only" note above for the narration-trimming part.)

[antigenius0910]

PR #1115 Code Review — feat(slack): add [slack].streaming toggle (send-once mode)

Related issue: #1114 (RFC)
Branch: dogzzdogzz:feat/slack-streaming-toggle → openabdev:main
Single commit: f249b60
Diff size: 7 files, +70 / -8

---

1. Overall (TL;DR)

A narrow, backward-compatible, cleanly implemented configuration-flag PR. It adds a streaming switch under [slack], defaulting to true (current behavior). When set to false, the Slack adapter posts a single final message per turn — no native streaming and no post+edit placeholder.

Recommendation: Mergeable once the two RFC open questions (naming, Discord symmetry) are resolved at the design level. Nothing in the implementation blocks merge.

---

2. Implementation review

2.1 Correct gating points

Both gates are placed at the right spots:

• src/slack.rs:537 — use_streaming() prefixed with self.streaming &&, controls post+edit placeholder
• src/slack.rs:549 — uses_native_streaming() likewise gated, controls chat.startStream

uses_assistant_status() (src/slack.rs:544) is intentionally unaffected, which is the right call — the test assistant_mode_gates_status_and_native_streaming at line 2114 asserts this explicitly. The user still gets the "Thinking…" status set via assistant.threads.setStatus; only the typewriter effect on the final message goes away. That UX trade-off is reasonable.

2.2 Dispatch flow integration

The send-once branch at src/adapter.rs:946 already exists — it's the path Discord falls through to when other_bot_present=true. streaming=false reuses it directly, without introducing a new code path. Risk surface is therefore tiny.

stream_begin (adapter.rs:698) is lazily triggered: only fires on first Text event when native=true. With streaming=false, native is always false, so stream_begin is never called → no placeholder leakage.

2.3 Config structure

src/config.rs:431-441 uses #[serde(default = "default_true")] so existing configs upgrade with zero changes. The doc comment explicitly explains:

Mirrors [gateway] streaming in concept, but the default deliberately differs: GatewayConfig.streaming defaults to false, whereas this defaults to true to preserve current Slack streaming.

Good doc comment — anyone confused by the asymmetric defaults between this and GatewayConfig.streaming (config.rs:467) gets the rationale on the spot.

2.4 Test coverage

src/slack.rs:2110-2114 adds a third adapter (streaming=false, assistant_mode=true) to the assistant_mode_gates_status_and_native_streaming test, with three assertions:

assert!(!adapter3.use_streaming(false), "streaming=false forces send-once (no post+edit)");
assert!(!adapter3.uses_native_streaming(false), "streaming=false disables native even with assistant_mode");
assert!(adapter3.uses_assistant_status(), "streaming switch does not affect assistant status API");

These cover all three gates' states cleanly — no obvious gap.

2.5 Helm chart

charts/openab/templates/configmap.yaml:141-144 mirrors the rendering pattern used for assistant_mode right above it: only emit to config.toml when explicitly set to false, otherwise defer to the Rust default. This avoids drift between chart-side and Rust-side defaults.

charts/openab/tests/configmap_test.yaml:184-204 adds two symmetric tests verifying "explicit false renders" and "explicit true does not render".

values.yaml:305-307 documents the use case (multi-agent threads avoiding app_mention re-fires), which is genuinely helpful for Helm users.

---

3. Discussion points

3.1 Naming (RFC open question #1)

The RFC listed three candidates: streaming / live_streaming / send_once. Author chose streaming.

I think streaming is acceptable but not optimal:

• ✅ Symmetric with [gateway] streaming
• ⚠️ Could be confused with assistant_mode, which is itself a flavor of streaming
• ⚠️ The semantics aren't quite "enable streaming"; setting streaming = false is really "force send-once, overriding everything else"

live_streaming or even send_once (inverted boolean, default false) would be more obvious. But you pay a symmetry cost. A judgment call for the maintainer — not a blocker.

3.2 Discord symmetry (RFC open question #2)

The RFC notes "Discord lacks post+edit streaming", but the Discord adapter does have placeholder + edit (src/discord.rs:135's use_streaming). This PR only touches Slack, without adding a symmetric Discord switch.

I'd lean toward merging the Slack part first (it already addresses the #1103 phantom-turn bug) and tracking Discord symmetry as a follow-up issue, because:

• Discord's MESSAGE_UPDATE doesn't deliver mention notifications (already noted in adapter.rs:921), so the phantom-turn risk is lower there
• Slack-adapter diff is already non-trivial; bundling Discord would expand the review surface

3.3 Startup log

The current streaming value is not surfaced at startup. There is per-turn debug logging (src/slack.rs:550-555), but it's at debug level and per-turn only.

Suggest adding an info-level startup log near main.rs:236-243 after SlackAdapter::new:

tracing::info!(streaming = s.streaming, assistant_mode = s.assistant_mode, "slack adapter configured");

This makes diagnosing "I set streaming = false but still see streaming behavior" a one-line check. Optional polish — non-blocking.

3.4 Documentation consistency

config.toml.example:42 indentation looks slightly off relative to the assistant_mode comment block right above it. Inline format should align. Cosmetic.

3.5 What's missing (checklist)

• ✅ No schema migration needed (additive field)
• ✅ No changelog entry needed (release flow handles it)
• ✅ clippy should be clean (no new lint risk)
• ⚠️ No integration test that goes through config.toml → deserialize → SlackAdapter to verify use_streaming() returns the expected value. Current unit tests call SlackAdapter::new(..., false) directly, bypassing the deserialize path. A config.rs round-trip test asserting streaming = false lands as s.streaming == false would be belt-and-braces. Nice-to-have, not required.

---

4. E2E verification (actually executed — 2026-06-15 07:24–07:27 UTC)

Test environment: compose.issue1114.yaml + openab-claude:pr1115 image, bot1 with streaming = false, bot2 with default. Driven autonomously via agent-browser + xoxc-token in #mcpsupportbot-test (C0AV8B98NKV).

#	Scenario	Expected	Observed	Pass
T1	@bot1 standalone mention	One final message, no edits	bot1 log: streaming=false … native=false; reply edited=null, 354 chars in a single send	✅
T2	@bot2 standalone mention (default streaming)	Typewriter / edit loop visible	bot2 log: streaming=true … native=true; reply edited=1781508333.000000, updated ~1.5s later	✅
T3	@bot1 mentions @bot2 in a thread	bot2's app_mention fires exactly once	bot1 reply edited=null; bot2 receives 1 mentions_bot=true from bot1's reply; bot2's next turn has other_bot_present=true → use_streaming=false, so no edit storm	✅

Additional observations:

• The slack assistant_mode decision debug log correctly surfaces the new streaming field — helps when debugging.
• During T3, bot2's own streamed reply broadcasts subtype="message_changed" events to every socket (including its own and bot1's). These events carry mentions_bot=false so they don't re-trigger dispatch — the adapter handles this correctly.
• With assistant_mode=true + streaming=false, the assistant status API still fires (since uses_assistant_status() is not gated by the new switch) — matches both the PR's intent and the unit test assertion.
• No panics, no errors, no leftover placeholder ("…") messages throughout the E2E run.

---

5. Conclusion

Approve — all three E2E scenarios pass, implementation is correct with complete test coverage, fully backward compatible.

Remaining items are design-level (RFC open questions):

1. Naming streaming vs live_streaming / send_once — I lean toward streaming being acceptable-but-ambiguous; maintainer's call.
2. Discord symmetry — recommend tracking as a follow-up issue (Discord's phantom-turn risk is lower).
3. Strong suggestion to fold into this PR: §3.3 startup info log (a single tracing::info!(streaming = s.streaming, …)). Saves a lot of debugging time.

The E2E run also incidentally verified that the existing other_bot_present=true → use_streaming=false rule from #534 still holds — this PR does not regress prior behavior.

[dogzzdogzz]

Added a follow-up commit (733abf5) on top of the toggle: send-once now delivers only the final answer block — the text emitted after the last tool call.

Why: the turn buffer concatenates every agent_message_chunk, so an agent's inter-tool narration ("let me pull the diff", "now reading X") leaked into the final message. A tool-posted artefact (e.g. a GitHub PR comment) is clean because it's a single composed string; the send-once reply wasn't. This makes send-once read like that single composed artefact.

Scope: it's in the shared stream_prompt_blocks loop, gated on !use_streaming(), so it applies to any send-once turn — gateway platforms (streaming defaults false) and Discord multi-bot threads, not just Slack. Streaming paths are untouched.

Correctness: split_delivery() parses output directives from the full buffer before slicing, so a leading [[reply_to:...]] survives even when the narration carrying it is dropped. +7 unit tests covering 0/1/N tools, the no-tool case, UTF-8 boundaries, and directive preservation.

[antigenius0910]

Follow-up review for 733abf5 ("send-once delivers only the final answer block").

What I like

The motivation is real — agents emit inter-tool narration ("let me pull the diff", "now reading X") that bleeds into send-once replies and makes them read like stream-of-consciousness, while a tool-posted artefact reads like a single composed string. This commit aligns send-once with the artefact form.

Implementation is contained:

• One state variable (answer_start: usize) advanced on every ToolDone.
• Two helpers (select_delivery_text, split_delivery) with single, well-named responsibilities.
• One diff point in stream_prompt_blocks — streaming path untouched.

Test coverage is genuinely thorough: UTF-8 char-boundary fallback, leading directive preserved across tools, no-tool case re-stripping the directive header, streaming preserves full body + directive. The reset && !streaming && answer_start > 0 re-prepend of the session-reset notice is exactly the kind of edge case that often gets missed.

Discussion points

1. PR scope/title now extends beyond Slack

The follow-up applies to every send-once turn — not just [slack] streaming=false. That includes:

• Gateway platforms (Telegram / LINE / Google Chat) where [gateway] streaming defaults to false
• Discord multi-bot threads where use_streaming = !other_bot_present

Existing users on those platforms will see their messages get noticeably terser after upgrading. Most will probably like it (it's the artefact-form pitch), but it is a silent behavior change. Worth either:

• Adjusting the PR title to surface the broader scope (e.g. feat(adapter): send-once strips inter-tool narration), or
• Calling it out explicitly in the release note when this ships.

2. streaming field semantics are now more overloaded

Repeating a point from my previous review: streaming already gated two things (post+edit vs send-once, and native streaming via assistant.threads.setStatus). This commit adds a third — whether inter-tool narration is dropped from the final message. The three are conceptually independent:

• Switch A: stream live or send once (placeholder + edit, or single post)
• Switch B: include narration or only the final block

The PR bundles them under one flag. That is probably the right UX default (anyone who picks send-once likely wants the artefact form), but the field name streaming no longer captures the full surface. If you keep this coupling, a clarifying sentence in the doc comment along the lines of "streaming = false also implies 'final answer block only'" would help readers understand the implicit second behavior.

3. Implicit invariant: "agent emits the final answer after its last tool call"

answer_start = text_buf.len() is set on every ToolDone, so it always points just past the most recent tool. If the agent ever writes its answer before its final tool call (say: "Answer: 42" followed by a logging tool, then end-of-turn), answer_start advances past the answer and select_delivery_text returns empty — the user gets _(no response)_.

In practice agents don't write this way, and the ACP pattern is "answer follows the work", so this is mostly theoretical. But the assumption isn't stated anywhere. Suggest folding a one-liner into select_delivery_text's doc comment, e.g.:

Assumes the agent emits its final answer after its last tool call (the standard ACP pattern). A turn whose only post-tool emission is empty falls through to the _(no response)_ sentinel in stream_prompt_blocks.

4. Possible refactor (non-blocking)

use_streaming() is now an implicit double-gate: it controls placeholder behavior AND narration trimming. Reads cleanly today, but if any future change wants to vary the two independently (e.g. a platform that wants send-once with narration, or streaming with final-block-only), the entanglement will need to be unwound. Could be split into a separate trait method like delivers_final_block_only() at that point — not necessary now.

E2E verification (commit 733abf5)

Rebuilt the rig with the new image and ran two new scenarios on top of the previous T1–T3:

#	Scenario	Expected	Observed	Pass
T4	@bot1 (streaming=false) prompt that requires tool use ("read /etc/hostname then tell me the hostname in one short sentence")	Only the post-last-tool answer block reaches Slack; no "let me check the file" narration	bot1 log: streaming=false … native=false; reply 72 chars, edited=null: :white_check_mark: `Read /etc/hostname` The hostname is `c006f123b639` . — i.e. tool-line indicator + clean 17-char final answer, zero pre-tool narration	✅
T5	@bot1 prompt that triggers no tools ("what is 2+2? answer in one short sentence without using any tools")	Full reply preserved (regression guard: answer_start == 0 keeps the whole buffer)	reply 10 chars, edited=null: 2 + 2 = 4. — pure answer, nothing dropped	✅
T6	Send-once with leading [[reply_to:<msg_ts>]] directive emitted before a tool	Directive parsed from full buffer and applied; body has header stripped	Deferred — the 7 unit tests added in this commit cover the directive-preservation logic from every angle I could think of (leading-directive-survives-tools, no-tool case re-strips header, streaming preserves both, UTF-8 char-boundary fallback). I am willing to take the unit tests at face value here rather than engineer an agent prompt that emits [[reply_to:...]] reliably.	➖

Additional notes from the E2E run:

• The slack assistant_mode decision debug log still correctly surfaces streaming=false after the follow-up commit — per-turn decision logging is intact.
• No empty replies, no _(no response)_ sentinel fallthrough, no leftover placeholder messages.
• Subjective UX read: T4's reply ("The hostname is c006f123b639.") is meaningfully tighter than the equivalent on f249b60, which would have included a sentence or two of "let me read the file" / "I'll check the hostname" prefacing the answer. The artefact-form pitch in the commit message lands in practice.

Verdict

Direction supported. The behavior change is well-motivated, the implementation is clean, and the tests are good. The items above are clarification asks, not blockers — the only one I'd push for in this PR is either a PR title/release-note adjustment for the broader scope, or a one-liner in the select_delivery_text doc comment stating the "answer follows last tool" invariant.

[howie]

Final Aggregated Review — PR #1115

feat(slack): add [slack].streaming toggle (send-once mode) · +295/-13, 8 files

Mode

group-review (3/3 voices active: Claude + Codex + Gemini) · R1 independent + R2 cross-debate

TL;DR

The core Rust logic is correct (UTF-8 boundary handling, keep_full_text cross-platform wiring, all 6 SlackAdapter::new call sites, Rust↔Helm default parity for the wired keys). No merge-blocking logic bug in the new helpers. Two things genuinely need attention: a Helm knob that is documented but never rendered (3/3 consensus), and a silent send-once behavior change for existing deployments (disputed — design intent vs. backward-compat baseline). Plus three small consensus cleanups.

---

Consensus Important (should fix before merge)

1. gateway.streaming documented in Helm but never rendered → silent no-op ✅ 3/3 + empirically confirmed

• Where: charts/openab/values.yaml:373-376 & config.toml.example:140-143 (docs added) vs charts/openab/templates/configmap.yaml [gateway] block (L206-240, no streaming render).
• Why it matters: GatewayConfig.streaming is a pre-existing Rust field; the template never rendered it (pre-existing gap). But THIS PR newly advertises the knob in values.yaml/config.toml.example. A user who reads the new docs and sets agents.*.gateway.streaming: true gets TOML that omits it → Rust default false applies → setting silently does nothing. Asymmetric with slack.streaming, which the same PR DID wire (configmap.yaml:142-145).
• Fix: Add a guarded streaming render to the [gateway] block + a Helm test mirroring the slack.streaming tests. (Lead verified: no streaming key exists in the gateway template block.)

2. split_delivery docstring overclaims directive survival (reset turn) ✅ 3/3

• Where: src/adapter.rs:126-134 (mirror comment ~910-917); root cause at src/adapter.rs:657-659.
• Why it matters: The new docstring states a leading [[reply_to:...]] always "survives the narration it was emitted alongside". In a reset turn the buffer is seeded with "⚠️ _Session expired…_\n\n" at byte 0 before agent output, so parse_output_directives breaks on the notice and the directive is dropped (no-tool case: leaked into body; with-tool case: silently dropped). The underlying parse behavior is PRE-EXISTING (out of scope to fix here), but the new docstring asserts a guarantee the code does not honor.
• Fix (in-scope): Qualify the docstring to scope the guarantee to non-reset turns, or note the reset-turn gap. (Optional follow-up, out of scope: move reset-notice insertion to the final formatting stage so directives parse from a clean buffer — Gemini/Codex suggestion.)

3. AdapterRouter::stream_prompt_blocks integration untested ⚠️ 2 Important (Claude, Gemini) / 1 NIT (Codex)

• Where: src/adapter.rs:619, 924-928.
• Why it matters: The pure helpers are well unit-tested, but no test covers (a) the reset re-prepend conditional if reset && !keep_full_text && answer_start > 0 — the subtlest logic in the PR, (b) keep_full_text = streaming || self.reactions_config.narration_display reading real config, (c) the gateway send-once default path. AdapterRouter has no mock seam (dispatch.rs:1143).
• Fix: Extract the reset re-prepend into a small pure helper (e.g. finalize_body(reset, keep_full, answer_start, body) -> String) and unit-test it. (Codex view: not itself a blocker; add focused regression tests alongside the fixes above.)

---

Disputed (maintainer decides)

Default send-once behavior change: narration trimmed by default

• Fact (undisputed): Before this PR, every send-once path (gateway default, Slack/Discord multi-bot) delivered the FULL accumulated reply. After, with narration_display defaulting false, they deliver only the final answer block. Existing deployments get a visible change without opting in. (src/config.rs + src/adapter.rs:619.)
• Codex: backward-compat REGRESSION — invokes "OpenAB's rule is backward-compatible defaults"; should ship as explicitly breaking or be made opt-in.
• Gemini + Claude: this is the PR's intended product design (clean final artefact on non-streaming platforms beats raw scratchpad narration); document in migration/release notes rather than revert the default.
• Lead recommendation: Keep the opt-out default (the feature's whole point), BUT (1) call the behavior change out prominently in the PR description + CHANGELOG so gateway/multi-bot operators aren't surprised, and (2) since Codex cites a project baseline ("backward-compatible defaults"), the opt-in-vs-opt-out call belongs to the maintainer — please rule explicitly. This is a product/baseline decision, not a reviewer call.

---

Actionable NIT (consensus — clean up before merge)

4. split_delivery redundant + unsafe directive re-parse ✅ 3/3

• src/adapter.rs:140-142. Re-parsing the suffix slice is redundant (directives already parsed from full) and can wrongly strip a final answer block that legitimately begins with [[…]]. Fix: parse directives once from full; only strip the header from the body when the delivered slice is the original leading region (i.e. answer_start == 0).

5. select_delivery_text silent fallback leaks narration on stale offset ✅ 3/3

• src/adapter.rs:122. full.get(answer_start..).unwrap_or(full) fails OPEN — on a stale/non-boundary offset it returns the whole buffer (incl. the narration the feature exists to drop) with no signal. Invariant holds today, so it's dead-code defense; add tracing::warn!(answer_start, full_len = full.len(), "stale answer_start; delivering full buffer") so a future regression is observable.

6. Helm negative test matcher too loose ✅ 3/3

• charts/openab/tests/configmap_test.yaml (test "does not render slack streaming when enabled"): notMatchRegex: 'streaming' matches the bare substring (would catch unrelated keys/comments). Tighten to 'streaming = ' to match the rendered-TOML contract the positive test asserts.

---

Verified correct (no action)

• UTF-8 char-boundary safety of full.get(answer_start..); stale offset falls back without panic (tested).
• keep_full_text = streaming || narration_display coherent across Slack (streaming && !other_bot), Discord (!other_bot), gateway (self.streaming), and native streaming.
• Rust default_true (slack.streaming) / default false (gateway.streaming, narration_display) all match docstrings + config.toml.example + values.yaml, mutually consistent across the 3 doc surfaces.
• All 6 SlackAdapter::new call sites updated; adapter3 test asserts streaming=false forces send-once + orthogonality with assistant_status.
• The 4 wired Helm tests (slack.streaming render-on/no-render, narration_display render-on/false/absent) assert full rendered values, not bare key presence.

Voices unavailable

• None. (Codex R1 needed a manual re-run because the helper script hard-codes git fetch origin <base> and this is a cross-fork PR whose base lives on upstream; agy R1 needed one retry after entering agentic file-search mode. Both produced full R1+R2.)

Verdict

NEEDS_CHANGES — 1 consensus Helm fix (#1), 1 docstring fix (#2), 1 maintainer decision (send-once default), + 3 consensus NITs. No merge-blocking logic bug.