Skip to content

fix: pin CLI versions in all Dockerfiles using ARG for reproducible builds#1

Open
chaodu-agent wants to merge 46 commits into
mainfrom
fix/pin-cli-versions
Open

fix: pin CLI versions in all Dockerfiles using ARG for reproducible builds#1
chaodu-agent wants to merge 46 commits into
mainfrom
fix/pin-cli-versions

Conversation

@chaodu-agent
Copy link
Copy Markdown
Owner

Summary

Pin all CLI dependency versions in Dockerfiles using ARG directives for reproducible builds. Changing the ARG value automatically busts the Docker layer cache, and version bumps are visible in git diff.

Changes

Dockerfile CLI ARG Pinned Version
Dockerfile kiro-cli KIRO_CLI_VERSION 1.29.5
Dockerfile.codex @openai/codex CODEX_VERSION 0.120.0
Dockerfile.claude @anthropic-ai/claude-code CLAUDE_CODE_VERSION 2.1.107
Dockerfile.gemini @google/gemini-cli GEMINI_CLI_VERSION 0.37.2
Dockerfile.copilot @github/copilot COPILOT_VERSION 1.0.25

Previously pinned dependencies (codex-acp@0.9.5, claude-agent-acp@0.25.0) are left unchanged.

How it works

Each Dockerfile now declares an ARG with a default version before the RUN that installs the CLI:

ARG GEMINI_CLI_VERSION=0.37.2
RUN npm install -g @google/gemini-cli@${GEMINI_CLI_VERSION} --retry 3

For kiro-cli, the latest path segment is replaced with the version number:

https://desktop-release.q.us-east-1.amazonaws.com/1.29.5/kirocli-x86_64-linux.zip

Closes openabdev#325

chaodu-agent and others added 30 commits April 11, 2026 18:15
The helm install examples used a stale commit SHA (78f8d2c) from PR openabdev#145.
Now that tag-driven releases produce :latest on stable promote, use that instead.

Co-authored-by: thepagent <thepagent@users.noreply.github.com>
* feat: resize and compress images before base64 encoding

Follow OpenClaw's approach to prevent large image payloads from
exceeding JSON-RPC transport limits (Internal Error -32603).

Changes:
- Add image crate dependency (jpeg, png, gif, webp)
- Resize images so longest side <= 1200px (Lanczos3)
- Re-encode as JPEG at quality 75 (~200-400KB after base64)
- GIFs pass through unchanged to preserve animation
- Fallback to original bytes if resize fails

Fixes openabdev#209

* test: add unit tests for image resize and compression

Tests cover:
- Large image resized to max 1200px
- Small image keeps original dimensions
- Landscape/portrait aspect ratio preserved
- Compressed output smaller than original
- GIF passes through unchanged
- Invalid data returns error

* fix: preserve aspect ratio on resize + add fallback size check

Address review feedback from @the3mi:

- 🔴 Fix resize() to calculate proportional dimensions instead of
  forcing 1200x1200 (was distorting images)
- 🟡 Add 1MB size check on fallback path when resize fails
- Fix portrait/landscape test assertions to match correct aspect ratios

* fix: restore post-download size check + use structured logging

Address minor review feedback:
- Restore defense-in-depth bytes.len() check after download
- Use tracing structured fields (url = %url, error = %e) for
  consistency with codebase style

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…abdev#138)

fix: dedupe tool call display by toolCallId and sanitize titles
…enabdev#81) (openabdev#135)

fix: prevent Discord message fragmentation during streaming (fixes openabdev#81)
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…ev#225)

* feat: support voice message STT (Speech-to-Text) for Discord

Add optional STT support that transcribes Discord voice message
attachments (audio/ogg) via any OpenAI-compatible /audio/transcriptions
endpoint and injects the transcript into the ACP prompt as text.

- New src/stt.rs: ~50-line module calling POST /audio/transcriptions
- New SttConfig in config.rs: enabled, api_key, model, base_url
- discord.rs: detect audio/* attachments, download, transcribe, inject
- Defaults to Groq free tier (whisper-large-v3-turbo)
- Supports any OpenAI-compatible endpoint via base_url (Groq, OpenAI,
  local whisper server, etc.)
- Feature is opt-in: disabled by default, zero impact when unconfigured

Closes openabdev#224

* fix: add json feature to reqwest for resp.json() in stt module

* docs: add STT configuration and deployment guide

* fix: address PR review feedback

- Reuse shared HTTP_CLIENT in stt.rs instead of creating per-call client
- Pass actual MIME type from attachment (not hardcoded audio/ogg)
- Fix attachment routing: check audio first, avoid wasted image download
- Add api_key validation at startup (fail fast on empty key)
- Add response_format=json to multipart form (fixes local servers)
- Update docs: clarify api_key requirement, add Technical Notes section

* feat: auto-detect GROQ_API_KEY from env when stt.enabled=true

If stt.enabled = true and api_key is not set in config, openab
automatically checks for GROQ_API_KEY in the environment. This
allows minimal config:

  [stt]
  enabled = true

No api_key line needed if the env var exists.

* fix: only auto-detect GROQ_API_KEY when base_url points to Groq

Prevents leaking Groq API key to unrelated endpoints when user
sets a custom base_url without explicitly setting api_key.

* docs: clarify GROQ_API_KEY auto-detect scope in stt.md

* fix: move STT auto-detect before handler construction

The handler clones stt_config at construction time. Auto-detect
was running after the clone, so the handler never received the
detected api_key. Now auto-detect runs first.

---------

Co-authored-by: openab-bot <openab-bot@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
* helm: add first-class STT config to chart

Add stt as a first-class config block in the Helm chart so users
can enable STT with a single helm upgrade command:

  helm upgrade openab openab/openab \
    --set agents.kiro.stt.enabled=true \
    --set agents.kiro.stt.apiKey=gsk_xxx

- values.yaml: add stt defaults (enabled, apiKey, model, baseUrl)
- configmap.yaml: render [stt] section when enabled, using ${STT_API_KEY}
- secret.yaml: store apiKey in K8s Secret (same pattern as botToken)
- deployment.yaml: inject STT_API_KEY env var from Secret

API key stays out of the configmap — follows the existing
DISCORD_BOT_TOKEN pattern.

Closes openabdev#227

* docs: add Helm chart deployment section to stt.md

* docs: mention STT support in README with link to docs/stt.md

* fix(helm): fail fast when stt.enabled=true but apiKey is empty

---------

Co-authored-by: openab-bot <openab-bot@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Set image.tag to empty string so the Helm template falls back to .Chart.AppVersion.

Closes openabdev#235
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…o docs/ (openabdev#268)

- README now shows only Kiro CLI (default) quick start
- Each agent (Claude Code, Codex, Gemini) gets its own docs/<agent>.md
- Multi-agent Helm setup moved to docs/multi-agent.md
- Simplified Pod Architecture diagram
- Collapsed reactions config into <details> tag
- Added agent table with links to individual guides

Co-authored-by: 超渡法師 <chaodu-agent@openab.dev>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
* feat: add GitHub Copilot CLI support

- Add Dockerfile.copilot with Copilot CLI + gh CLI install
- Add Copilot CLI config block to config.toml.example
- Update README.md with Copilot CLI in agent table, Helm example,
  and manual config example

Closes openabdev#19

* fix: address PR review feedback

- Replace curl|bash with npm install for Copilot CLI (security)
- Add note that only one [agent] block can be active at a time
- Add experimental warning for Copilot auth

* docs: add Copilot CLI agent backend guide

* docs: add env config with unvalidated warning to copilot guide

* fix: address thepagent review feedback on PR openabdev#265

- Remove misleading GITHUB_TOKEN env var from config.toml.example,
  replace with device flow comment
- Update docs/copilot.md prerequisites: Free tier does not include
  CLI/ACP access, require Pro/Pro+/Business/Enterprise
- Add persistence.enabled=true to Helm example (token lost on restart)
- Add note that GHCR image is not published yet, build locally
- Clean up Configuration section to remove unvalidated GITHUB_TOKEN

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
openabdev#273)

When Chart.yaml already has a beta version (e.g. 0.7.2-beta.1),
increment the beta number (→ 0.7.2-beta.2) instead of stripping
the suffix and bumping patch (→ 0.7.3-beta.1).

Fixes openabdev#272

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Add copilot variant to build-image, merge-manifests, and
promote-stable matrix blocks so CI publishes
ghcr.io/openabdev/openab-copilot.

Fixes openabdev#275

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…nabdev#202)

* feat: update issue templates and add completeness check workflow

- Update bug.yml: add optional Environment and Screenshots/Logs fields
- Update feature.yml: add optional Proposed Solution field
- Update guidance.yml: broaden description to cover misc questions
- Add documentation.yml: new template for documentation issues
- Add issue-check.yml: GitHub Action to validate required fields,
  adds 'incomplete' label and comment when fields are missing,
  auto-removes when completed

* feat: add check for issues created without template

Issues created via API/CLI bypassing templates will now be
flagged with 'incomplete' label and a comment asking the user
to use an available template.

* feat: add needs-triage label alongside incomplete

Ensures all incomplete issues also get needs-triage label,
so they are always visible during triage filtering.

* fix: improve issue-check workflow reliability

- Update no-template message to mention label requirement
- Add concurrency to prevent duplicate runs on rapid edits
- Skip repeated comments when issue already flagged as incomplete

* fix: make field regex more tolerant of extra whitespace/newlines

* fix: add note about preserving section headings in incomplete warning

* fix: handle 404 on removeLabel to prevent script crash

---------

Co-authored-by: ChunHao-dev <ChunHao-dev@users.noreply.github.com>
…nabdev#180)

* feat: add markdown table conversion pipeline with pulldown-cmark

- Introduce pulldown-cmark as markdown parser for accurate table detection
- Add TableMode config (code/bullets/off) via [markdown] section in config.toml
- Convert detected tables before sending final content to Discord
- Design as reusable pipeline for future multi-channel support

Closes openabdev#178

* fix: address PR review — unicode width, inline markup, trailing newline

- Use unicode-width crate for column width calculation (fixes CJK/emoji alignment)
- Use saturating_sub for padding to prevent underflow
- Handle inline markup inside table cells (bold, italic, strikethrough, link)
- Convert SoftBreak/HardBreak to space inside cells
- Fix trailing blank line after last row in bullets mode

* fix: strip backticks in code mode; split_message is code-fence-aware

- parse_segments now takes a mode parameter: in Code mode, Event::Code
  cells omit the backtick wrapping since the table is already inside a
  fenced code block and backticks would render as literal characters.
  Bullets mode keeps backticks as they are valid inline markdown.

- split_message now tracks whether the cursor is inside a fenced code
  block (``` ... ```). When a chunk boundary falls mid-block, the current
  chunk is closed with ``` and the next chunk is reopened with ```, so
  each Discord message renders the code block correctly.

- Tests added for both fixes.

---------

Co-authored-by: JARVIS-coding-Agent <jarvis@openab.dev>
Co-authored-by: OpenAB Agent <agent@openab.dev>
image: 920ae7e

Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
thepagent and others added 16 commits April 13, 2026 16:37
Revert "feat: Add markdown table conversion pipeline with pulldown-cmark (openabdev#180)"
…b41b71c

Revert "chore: bump chart to 0.7.3-beta.56 (openabdev#279)"
Fixes openabdev#309 — session pool leaks memory due to orphaned grandchild
processes and no session resume capability.

Changes:
- Replace kill_on_drop with process groups (setpgid + kill(-pgid))
  so the entire process tree is killed on session cleanup
- 3-stage graceful shutdown: stdin close → SIGTERM → SIGKILL
- Store agentCapabilities.loadSession from initialize response
- Add session/load method for resuming suspended sessions
- Suspend sessions on eviction (save sessionId) instead of discarding
- Resume via session/load on reconnect, fallback to session/new
- LRU eviction when pool is full (evict oldest idle session)
- Lower default session_ttl_hours from 24 to 4

Memory impact on 3.6 GB host:
  Before: 10 x 300 MB = 3 GB (idle sessions kept alive + orphaned grandchildren)
  After:  1-2 x 300 MB = 300-600 MB (idle sessions suspended, reloaded on demand)
The drop(self.stdin.clone()) only drops a cloned Arc, not the actual
ChildStdin. SIGTERM on the next line handles shutdown. Removed the
misleading comment and simplified to 2-stage: SIGTERM → SIGKILL.
…iability

Addresses triage review on openabdev#310:

🔴 SUGGESTED CHANGES:
- Merge connections + suspended into single PoolState struct under one
  RwLock to eliminate nested lock acquisition and deadlock risk
- suspend_entry() is now a plain fn operating on &mut PoolState (no async,
  no separate lock)
- cleanup_idle() collects stale keys and suspends under one lock hold
- child_pid changed to child_pgid: Option<i32> using i32::try_from()
  to prevent kill(0, SIGTERM) on PID 0 and overflow on PID > i32::MAX

🟡 NITS:
- setpgid return value now checked — returns Err on failure so spawn
  fails instead of silently creating a process without its own group
- SIGKILL escalation uses std::thread::spawn instead of tokio::spawn
  so it fires even during runtime shutdown or panic unwinding
…rocess-groups-and-resume

fix: process group kill + session suspend/resume via session/load
Adds a 3-value enum config option to control bot-to-bot message handling,
inspired by Hermes Agent's DISCORD_ALLOW_BOTS and OpenClaw's allowBots:

- "off" (default): ignore all bot messages — no behavior change
- "mentions": only process bot messages that @mention this bot
- "all": process all bot messages, capped at MAX_CONSECUTIVE_BOT_TURNS (10)

Safety: self-ignore always applies, "mentions" is a natural loop breaker,
"all" uses cache-first history check with fail-closed on API errors.

Case-insensitive deserialization, accepts "none"/"false" → off, "true" → all.
AllowBots::Off naming avoids confusion with Option::None.

Closes openabdev#319
…uilds

- Dockerfile: pin kiro-cli to 2.0.0 (use prod.download.cli.kiro.dev)
- Dockerfile.codex: pin @openai/codex to 0.120.0
- Dockerfile.claude: pin @anthropic-ai/claude-code to 2.1.107
- Dockerfile.gemini: pin @google/gemini-cli to 0.37.2
- Dockerfile.copilot: pin @github/copilot to 1.0.25

Kiro CLI version can be checked via:
  curl -fsSL https://prod.download.cli.kiro.dev/stable/latest/manifest.json | jq -r '.version'

Closes openabdev#325
@chaodu-agent chaodu-agent force-pushed the fix/pin-cli-versions branch from 4d01f11 to 7e88199 Compare April 14, 2026 11:07
chaodu-agent added a commit that referenced this pull request Apr 22, 2026
…constraint

Elevate from 'use axum' to a correctness property:
- HMAC must be verified against exact raw body bytes
- No hand-rolled TCP, lossy UTF-8, or reconstructed JSON
- Includes rationale for why lossy decoding is architecturally invalid
chaodu-agent added a commit that referenced this pull request Apr 22, 2026
chaodu-agent added a commit that referenced this pull request Apr 22, 2026
1. Schema: add deferred concerns table (conversation_key, trace_id,
   capabilities, reply_context, tenant)
2. Reply path: add credential store security risks (concentration,
   audit, rotation, mTLS)
3. Rollout: merge v2/v2.1 into single v2 phase
4. Compliance #1: softer wording — 'unless explicitly approved by
   a superseding ADR'
chaodu-agent added a commit that referenced this pull request May 14, 2026
* feat(dispatch): turn-boundary batching dispatcher v2 per ADR v0.3

* refactor(dispatch): cleanup naming, parallelize queued reactions, use configured emoji on SendError

- Rename ThreadHandle._consumer → consumer (we actually .abort() it on cancel)
- Replace ThreadHandle::drain_pending(&mut self) with pending_count(&self) —
  read-only signature, name no longer implies side effects
- Parallelize 👀 reactions in dispatch_batch via futures::join_all instead of
  serial loop — first-token latency no longer scales with batch size
- SendError ❌ reaction now uses router.reactions_config() instead of
  ReactionsConfig::default() — respects user-configured emoji
- shutdown() switches to iter() (no longer needs &mut after the rename above)
- Tighten doc comments
- Cargo.lock: sync to openab 0.8.2 (Cargo.toml already at 0.8.2)

* feat(discord): add /cancel-all slash command

Adds the standalone /cancel-all path from ADR §4.4 turn-boundary batching.
Unlike /reset, /cancel-all is non-destructive to the session.

- /cancel-all: dispatcher.cancel_buffered() + pool.cancel_session()
  → drops buffered messages + aborts in-flight ACP turn, keeps session
- /reset: unchanged (still drops buffered + cancels in-flight + tears down
  session); doc comment updated to reflect that /reset is a superset of
  /cancel-all rather than "/reset includes /cancel-all"

Discord-only — Slack adapter explicitly drops slash_commands envelopes
(no thread routing on channel-level delivery), Gateway has no user-facing
slash command surface.

Response messages cover all four (cancel_session result × dropped count) cases.

* refactor: unify PerMessage and Batched modes through Dispatcher

Both modes now serialize through the per-thread Dispatcher consumer
task. PerMessage = max_buffered_messages=1 (each message dispatches
alone, FIFO). Batched = configured cap (greedy drain up to
max_batch_tokens).

Removes the bifurcated match in Slack/Discord/Gateway hot paths,
eliminates the Option<Arc<Dispatcher>> indirection, and addresses
chaodu-agent PR openabdev#686 review concern about PerMessage FIFO regression
after the KeyedAsyncQueue removal.

* chore(dispatch): address PR openabdev#686 NITs

- Extract duplicated days_to_ymd / ISO 8601 conversion from slack.rs
  + gateway.rs into new src/timestamp.rs (with unit tests).
- Add sender_name to BufferedMessage per ADR §2.3 — denormalised from
  sender_json so dispatch_batch tracing doesn't pay a JSON parse.
- impl std::error::Error for DispatchError so it composes with anyhow.

* fix(dispatch): idle eviction, config validation, avoid clone, timestamp precision

- Add 5-min idle timeout to consumer_loop to prevent per-thread handle/task
  leak (unbounded growth from one-shot thread keys like Slack non-thread msgs)
- Validate max_buffered_messages > 0 at config load time (prevents panic from
  tokio::sync::mpsc::channel(0))
- Use into_iter() in dispatch_batch to avoid deep-copying extra_blocks
  (may contain base64 image data)
- Add TODO comment for gateway multibot detection
- Use real milliseconds in now_iso8601() via dur.subsec_millis()

Co-authored-by: 超渡法師 <chaodu@openab.dev>

* fix(dispatch): proactive stale-entry cleanup + transparent retry on idle exit

- submit() now checks consumer.is_finished() before using an existing
  handle, removing stale entries proactively (fixes map leak for one-shot
  thread keys that never get a second submit)
- On SendError, transparently evict + rebuild + retry once instead of
  surfacing an error to the user (fixes first-message-after-idle being
  treated as ConsumerDead)
- Only report ConsumerDead if the retry also fails (truly unexpected)

* fix(dispatch): periodic sweep of stale per-thread entries

- Add Dispatcher::sweep_stale() that retains only entries whose consumer
  task is still running (map.retain + is_finished check)
- Wire into main.rs cleanup task (60s interval, alongside pool.cleanup_idle)
- Prevents unbounded map growth from one-shot thread keys (e.g. Slack
  non-thread messages) that never receive a second submit()
- dispatchers Vec wrapped in Arc<Mutex<>> so cleanup task can access it

* feat(dispatch): add per-lane batching mode (default for "batched" alias)

Extends MessageProcessingMode from {PerMessage, Batched} to three values:
- PerMessage: each message → one ACP turn (unchanged default behaviour)
- PerThread:  thread-wide buffer, all senders share one batch (old "Batched")
- PerLane:    per (thread, sender) buffer, each sender gets its own ACP turn

The legacy alias "batched" now resolves to PerLane — the recommended default
for batching, since per-lane eliminates the silent-drop risk where a single
mixed-sender ACP turn produces one reply that may forget to address some
senders. Existing configs continue to load without change but now run under
per-lane semantics.

Implementation:
- Adds BatchGrouping enum to dispatch.rs and `Dispatcher::key()` helper that
  builds the per-thread map key from (platform, thread_id, sender_id).
  PerThread mode ignores sender_id; PerLane includes it.
- main.rs translates MessageProcessingMode to (cap, BatchGrouping) when
  constructing each platform's Dispatcher.
- Discord/Slack/Gateway adapters use `dispatcher.key(...)` instead of
  hand-rolled format!() at submit and slash-command sites.
- Session pool keys remain per-thread (unchanged) — the ACP session is
  shared across lanes by design; turns serialise through the shared session.
- /cancel-all and /reset use the invoker's lane key (B1: cancel only own
  lane) but still cancel/reset the shared session (B4-a: keep escape hatch
  from a runaway in-flight turn).

Tests:
- dispatch::tests::key_per_thread_ignores_sender / key_per_lane_includes_sender
- config::tests::message_processing_mode_{parses_per_message,parses_per_thread,
  parses_per_lane,batched_alias_is_per_lane,default_is_per_message,
  unknown_value_errors}
- 224 tests passing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(dispatch): /reset and /cancel-all clear all lanes in thread

Replaces the per-key Dispatcher::cancel_buffered with cancel_buffered_thread,
which prefix-matches every per-thread handle for a (platform, thread_id) pair
and aborts each consumer. Both PerThread keys (`platform:thread`) and PerLane
keys (`platform:thread:sender`) are dropped, with care taken to avoid the
substring trap (T1 must not match T10).

Behaviour:
- /cancel: unchanged — stop in-flight ACP turn only, queue continues.
- /cancel-all: stop in-flight + drop every lane's buffer in the thread (was:
  invoker's lane only). The nuclear escape hatch — keeps ACP context, clears
  queued work so a human can intervene.
- /reset: drop every lane's buffer + tear down the ACP session (was:
  invoker's lane only). Next message in the thread starts a fresh session.

Gateway:
- run_gateway_adapter now also receives the AdapterRouter, so the upstream
  /reset and /cancel slash-command interception (added on main while this
  branch was in review) compiles after rebase.
- Gateway /reset gets the same all-lanes drop as Discord; /cancel keeps the
  in-flight-only semantics from upstream.
- /cancel-all is intentionally not added to the gateway interception path.

Tests: 227 passing (+3 new dispatcher tests covering PerThread drop,
PerLane all-lanes drop, and the T1-vs-T10 prefix-collision guard).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* feat(config)!: drop "batched" alias, only per-message/per-thread/per-lane accepted

The legacy `"batched"` value (which resolved to PerLane on this branch) is
removed. Configs using `message_processing_mode = "batched"` will now fail
to parse with an `unknown variant "batched"` error pointing at the three
accepted values, forcing an explicit migration to per-thread or per-lane.

The two batching modes have meaningfully different semantics (shared vs
isolated buffer per sender), so a silent default is the wrong call —
users should pick deliberately.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(dispatch): restore shared thread sessions and abort consumers on shutdown

Co-authored-by: Brett Chien <1193046+brettchien@users.noreply.github.com>

* feat(chart): expose message_processing_mode and batching params

Adds messageProcessingMode / maxBufferedMessages / maxBatchTokens to the
Discord, Slack, and Gateway sections of the chart. Without these the
turn-boundary batching modes shipped in PR openabdev#686 are unreachable from a
helm-deployed instance — the Rust binary just falls back to per-message.

- configmap.yaml: render the three keys for each platform when set, with
  enum validation matching the Rust deserializer
  ("must be one of: per-message, per-thread, per-lane").
- values.yaml: commented examples for each platform.
- tests/message-processing-mode_test.yaml: 12 helm-unittest cases covering
  render, enum rejection, omit-when-unset, and numeric param render across
  all three platforms.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor: rename enum variants to drop redundant Per prefix

Aligns MessageProcessingMode and BatchGrouping with the rest of the
codebase (TableMode, AllowBots, ToolDisplay, TurnSeverity, etc.) where
variants don't repeat the enum-name-derived prefix. Also fixes the CI
clippy::enum_variant_names failure on PR openabdev#686.

Wire format unchanged — manual Deserialize still matches per-message /
per-thread / per-lane strings; helm chart and TOML configs need no edits.

- MessageProcessingMode { PerMessage, PerThread, PerLane } -> { Message, Thread, Lane }
- BatchGrouping { PerThread, PerLane } -> { Thread, Lane }

* feat(config): validate max_batch_tokens > 0

Setting max_batch_tokens=0 doesn't crash but forces every batch to size 1
via the consumer loop's token-cap check — functionally per-message mode
through a confusing path. Reject it at config parse time, alongside the
existing max_buffered_messages > 0 check.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* test(dispatch): cover sweep_stale and shutdown

Add an alive_consumer_handle helper (parks on pending::<()>) and four
unit tests:

- sweep_stale removes finished consumers, leaves running ones alone
- shutdown clears the per-thread map and aborts running consumers
  (verified via abort_handle().is_finished() after a runtime tick)

These paths are simple but safety-critical (SIGTERM cleanup + idle-task
GC); the existing dummy_handle / make_dispatcher scaffolding already
covers the test surface, so no new mocks needed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* test(dispatch): cover consumer_loop via DispatchTarget trait seam

Closes the NIT 2 gap from PR openabdev#686 review. The consumer_loop orchestration
(greedy drain / token cap overflow / idle timeout / SendError eviction)
was previously only verified by manual staging smoke. The trait seam
also unblocks the §2.5 SendError end-to-end test.

Refactor:
- DispatchTarget trait (reactions_config / ensure_session /
  stream_prompt_blocks) extracted from AdapterRouter's surface.
  AdapterRouter implements it by delegation.
- Dispatcher now holds Arc<dyn DispatchTarget>. Production callsites
  unchanged — Arc<AdapterRouter> auto-coerces via CoerceUnsized.
- Add Dispatcher::with_idle_timeout (test knob); Dispatcher::new keeps
  the DEFAULT_CONSUMER_IDLE_TIMEOUT (5 min) production default.

Tests:
- MockDispatchTarget records dispatches; MockChatAdapter is a no-op stub.
- consumer_dispatches_single_message_as_one_batch (happy path)
- consumer_greedy_drain_combines_queued_messages_into_one_batch
  (3 pre-loaded msgs → 1 dispatch with 3 ContentBlocks)
- consumer_token_cap_splits_batch_preserving_fifo
  (2x 80-token msgs + cap=100 → 2 FIFO dispatches)
- consumer_exits_after_idle_timeout_with_no_messages (50ms timeout)
- submit_evicts_dead_handle_and_retries_with_fresh_consumer
  (manufactured dead handle: rx dropped, consumer parked → SendError
  → eviction + retry on fresh consumer)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(adapter): make SenderContext.timestamp truly additive

Wraps the field in Option<String> with skip_serializing_if so consumers
that pre-date the addition see no new key in the serialized JSON.
All four producers (slack, discord, gateway, cron) wrap their existing
values in Some(...). Schema string stays openab.sender.v1.

* docs(dispatch): note re-acquire-after-await safety in submit

Calls out why re-acquiring per_thread after tx.send().await cannot
deadlock — the first lock guard is dropped before the await point.

* fix(adapter): use sender_context as standalone delimiter, split prompt into own block

pack_arrival_event now emits per arrival:
  [Text "<sender_context>{json}</sender_context>"]   (delimiter)
  [Text transcript blocks from extra_blocks]
  [Text prompt]                                      (omitted if empty)
  [non-Text blocks (e.g. Image)]

The sender_context block stands alone as a structural delimiter so agents
can locate arrival boundaries by scanning for `<sender_context>` openers
in batched dispatch. Within each arrival, transcript text precedes the
typed prompt to match pre-batching adapter UX (voice content first), and
images trail the prompt as before. Tests updated to reflect the new
per-arrival block count (2 minimum: delimiter + prompt; +1 per transcript;
+N for image attachments).

* fix(gateway): import AdapterRouter so handle_config_command compiles

handle_config_command's signature uses &AdapterRouter but only
crate::adapter::{ChannelRef, ChatAdapter, MessageRef, SenderContext}
were imported, so cargo check failed with E0425. Add AdapterRouter to
the use list (the other reference at line 482 already uses the fully
qualified path).

* fix(timestamp): parse Slack ts as f64 to preserve decimal semantics

Previously slack_ts_to_iso8601 split on '.' and parsed the fractional
substring as an integer, treating ".12" as 12 ms instead of 120 ms.
Parsing the entire string as f64 carries decimal semantics correctly
without any string-padding logic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(discord): drop approximate count from /cancel-all message

The buffered-message count is approximate (sweep races with new
arrivals) so surfacing an exact number to users was misleading. Show
a binary "cleared / nothing" signal instead. The pending_count() API
stays for logs and metrics.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs(dispatch): annotate per_thread mutex lock sites with SAFETY comments

Make the no-.await-while-locked invariant explicit at each lock
acquisition site so future edits can't silently introduce an .await
without tripping the comment. The struct-level note at line 183 stays
as the higher-level explanation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(dispatch): apply queued reactions sequentially

Replace futures_util::future::join_all with a sequential await loop.
Batches are typically small (low single digits) so the serialization
cost is sub-second and not user-visible, and the dispatch path no
longer pulls in join_all just for one call.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(dispatch): per-mode consumer idle timeout (10s for per-message)

Per-message mode (cap=1) doesn't benefit from holding consumers across
message gaps — there is no batch window to preserve — so a 5-minute
idle timeout left consumer tasks lingering long after they were useful.
Add PER_MESSAGE_CONSUMER_IDLE_TIMEOUT (10s), wire it through main.rs
based on each adapter's message_processing_mode, and drop the unused
Dispatcher::new wrapper.

By Little's Law (steady-state idle count = arrival rate × idle window),
this cuts per-message-mode idle dispatcher footprint by 30x for the
same arrival rate while keeping batched modes' 5-minute window so
between-trigger lanes aren't torn down on every message.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(dispatch): extract dispatch_params, name token-estimate consts, fix ADR path

- main.rs: collapse 3x repeated (cap, grouping, idle) match blocks into
  dispatch::dispatch_params(mode, max_buffered).
- dispatch.rs: replace magic 4 / 512 in estimate_tokens with named
  CHARS_PER_TOKEN_ESTIMATE / TOKENS_PER_IMAGE_ESTIMATE constants.
- dispatch.rs: fix top-level ADR reference to point at the actual
  docs/adr/turn-boundary-batching.md path landing in openabdev#598.

Addresses chaodu-agent NITs #1, openabdev#2, openabdev#5 from PR openabdev#686.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs: clarify schema evolution comment + dispatchers triple-Arc rationale

- adapter.rs: note that future breaking changes should bump to v1.1+
- main.rs: explain why Arc<Mutex<Vec<Arc<Dispatcher>>>> is necessary
  (shared with cleanup task + shutdown; pushes at startup only)

Addresses maintainer NITs from PR openabdev#686 review.

Co-Authored-By: 超渡法師 <chaodu-agent@users.noreply.github.com>

* docs: add message dispatch modes guide (per-message vs per-thread vs per-lane)

Decision guide for operators choosing between the three modes, with
config examples and trade-off explanations.

Co-Authored-By: 超渡法師 <chaodu-agent@users.noreply.github.com>

* docs(dispatch): add ASCII diagrams for all three modes + consumer loop

Visual explanation of per-message vs per-thread vs per-lane behavior,
plus the internal consumer_loop batching flow.

Co-Authored-By: 超渡法師 <chaodu-agent@users.noreply.github.com>

* docs: clarify per-message is the default behavior

* docs(dispatch): add explicit pros/cons and comparison table for each mode

---------

Co-authored-by: Brett Chien <1193046+brettchien@users.noreply.github.com>
Co-authored-by: 超渡法師 <chaodu@openab.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: shaun-agent <265093149+shaun-agent@users.noreply.github.com>
Co-authored-by: brettchien <49930+brettchien@users.noreply.github.com>
Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
chaodu-agent pushed a commit that referenced this pull request May 14, 2026
…penabdev#743)

* feat(gateway): add markdown_to_gchat conversion for Google Chat adapter

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(gateway): streaming support for Google Chat via edit_message command

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test(gateway): add integration tests for googlechat streaming reply flow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): address review feedback for googlechat streaming/markdown

- Multi-chunk path now sends GatewayResponse (prevents core timeout on long messages)
- Token failure sends failure GatewayResponse (parity with Feishu adapter)
- edit_message uses PATCH instead of PUT (per Google Chat API docs)
- Inject api_base for testability
- Rewrite integration tests with wiremock (hermetic, no real API calls)
- Update docs/google-chat.md: move markdown to supported, add streaming

Addresses canyugs#2

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(gateway): add strikethrough conversion for Google Chat markdown

~~text~~ → ~text~ (Google Chat native strikethrough syntax)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): address Copilot review feedback for googlechat PR openabdev#743

5 review items from canyugs#3:

1. Empty message: short-circuit to skip API call, send failure ack
2. Single-chunk send failure: propagate error string (status + body) in GatewayResponse.error
3. Multi-chunk send failure: propagate first error string instead of error: None
4. Italic: convert *text* → _text_ (Google Chat italic syntax). Single _text_ passes through.
5. docs/google-chat.md: align with actual converter behavior (bold, italic, strikethrough, headings)

Refactor: send_message now returns Result<String, String> so error context flows to core.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(gateway): clarify markdown_to_gchat assumption + perf TODO

- markdown_to_gchat: doc comment noting caller must pass raw markdown
  (called by both send_message and edit_message; double-conversion
  would happen if pre-converted text is passed)
- convert_inline: TODO note for future byte-level iteration optimization
  (currently Vec<char> allocation per line, acceptable at current scale)

Addresses chaodu-agent must-fix #1 and openabdev#2 from PR openabdev#743 review.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs(google-chat): add Workspace account requirement to Prerequisites

Regular @gmail.com consumer accounts cannot create Google Chat apps —
Google requires a Workspace (Business or Enterprise) account at API
configuration. Cheapest qualifying tier is Workspace Individual or
Business Starter.

Per Joseph19820124 question on PR openabdev#743.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(gateway): multi-chunk partial failure must report success=false

When chunk 1 succeeds but subsequent chunks fail, GatewayResponse was
reporting success=true with error=Some(...) — a contradictory signal.
Core would treat the message as delivered despite missing content.

Now any chunk failure marks the overall operation as failed, while
preserving message_id so core retains the reference for any follow-up.

Adds handle_reply_multi_chunk_partial_failure_reports_failure test
covering the mixed success/failure scenario (wiremock 200→500).

Addresses chaodu-agent blocking review on PR openabdev#743.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pin CLI versions in all Dockerfiles using ARG for reproducible builds

6 participants