perf: cache dependency build layer in Dockerfile - #1
Closed
thepagent wants to merge 1 commit into
Closed
Conversation
This was referenced Apr 15, 2026
11 tasks
6 tasks
This was referenced Apr 19, 2026
This was referenced Apr 25, 2026
This was referenced Apr 29, 2026
This was referenced May 5, 2026
brettchien
added a commit
to brettchien/openab
that referenced
this pull request
May 5, 2026
…ts, 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 openabdev#1, openabdev#2, openabdev#5 from PR openabdev#686. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
thepagent
pushed a commit
that referenced
this pull request
May 5, 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 #686 review concern about PerMessage FIFO regression after the KeyedAsyncQueue removal. * chore(dispatch): address PR #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 #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 #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 #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 #598. Addresses chaodu-agent NITs #1, #2, #5 from PR #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 #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>
thepagent
pushed a commit
that referenced
this pull request
May 6, 2026
…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 #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 #2 from PR #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 #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 #743. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
6 tasks
14 tasks
chaodu-agent
pushed a commit
that referenced
this pull request
Jun 10, 2026
- Fix #1: synthesis uses absolute path /opt/agentcore-acp/agentcore_acp.py - Fix #2: Dockerfile.agentcore adds curl, installs uv as agent user, installs boto3 system-wide, sets PATH for agent - Fix #3: replace value-based heuristic with command_explicit bool flag (immune to OPENAB_AGENT_COMMAND env pollution) - Fix #4: cancel_strategy is now a validated enum (stop|noop), rejects typos - Fix #6: docs priority rules clarify env var interaction - Fix #7: docs use absolute path consistently
chaodu-agent
pushed a commit
that referenced
this pull request
Jun 12, 2026
…on destroy - #1 (🔴) Fix None dereference: check session is not None before accessing - #2 (🟡) Log shell reader errors to stderr instead of silently swallowing - #3 (🟡) Clear stale response queue on reconnect - #4 (🟡) Remove unused ACP_READY_MARKER constant - #5 (🟡) Add session/destroy handler to clean up resources
thepagent
pushed a commit
that referenced
this pull request
Jun 13, 2026
…ions (#1089) * feat(agentcore): use InvokeAgentRuntimeCommandShell for persistent sessions Replace InvokeAgentRuntime (stateless HTTP) with InvokeAgentRuntimeCommandShell (persistent WebSocket PTY). This opens a long-lived shell in the microVM, runs kiro-cli in native ACP mode, and bridges JSON-RPC bidirectionally. Fixes: each Discord thread message was treated as a new conversation because InvokeAgentRuntime hit healthcheck.py which spawned a fresh kiro-cli per call. Now: same shell_id = same kiro-cli process = continuous session memory. Key changes: - Switch from boto3 invoke_agent_runtime to bedrock-agentcore SDK open_shell() - Deterministic shell_id derived from thread_id for reconnection support - Async architecture (asyncio) for WebSocket + stdin multiplexing - Auto-reconnect via ReconnectConfig on transient drops - Forwards kiro-cli ACP notifications (streaming) directly to OAB stdout * fix: address review — None deref, error logging, queue cleanup, session destroy - #1 (🔴) Fix None dereference: check session is not None before accessing - #2 (🟡) Log shell reader errors to stderr instead of silently swallowing - #3 (🟡) Clear stale response queue on reconnect - #4 (🟡) Remove unused ACP_READY_MARKER constant - #5 (🟡) Add session/destroy handler to clean up resources * nit: remove redundant import sys in _read_loop * fix: address 擺渡 review — ANSI stripping, JSON-RPC validation, dispatch guard - F1: Dispatch loop now skips messages without 'method' field (no false error responses) - F2: Strip ANSI escape codes before JSON parse; require line starts with '{' and contains 'jsonrpc' field to be treated as valid JSON-RPC - Disable PTY echo via 'stty -echo' before launching kiro-cli to reduce noise * fix: use _extract_json_object for PTY-robust JSON parsing Handles shell prompt prefix (e.g. 'agent@vm:~$ {"jsonrpc"...}') by reusing the existing brace-counting parser instead of startswith. * nit: move import time to top-level * feat(agentcore): replace Python adapter with native Rust WebSocket bridge Drop agentcore/acp/ Python adapter entirely. The bridge now lives in src/acp/agentcore.rs and runs as an 'openab agentcore-bridge' subprocess spawned by config.rs when [agentcore] is set. Architecture: OAB pool → spawn('openab agentcore-bridge --runtime-arn ... --region ...') → opens WebSocket to InvokeAgentRuntimeCommandShell → launches 'kiro-cli acp --trust-all-tools' inside PTY → bidirectional JSON-RPC over binary frames (0x00=stdin, 0x01=stdout) Key benefits over Python version: - Single binary, no Python/uv/pip dependency - Native tokio async, zero overhead - SigV4 signing via aws-sigv4 crate (same as agctl) - Deterministic shell_id for reconnection on transient drops - PTY noise filtering (ANSI strip, JSON boundary detection) New dependencies (behind 'agentcore' feature flag, included in default): aws-sigv4, aws-credential-types, urlencoding, hex, http Note: Cannot verify full compilation in this environment (missing cc linker), but rustfmt --edition 2021 confirms all files parse correctly. * fix: port F1 guard to Rust — skip messages without method field * fix: address 覺渡 review — channel constants + brace-counting JSON extraction - Replace magic 0x00/0x01/0x02 with named CHANNEL_STDIN/STDOUT/STDERR constants - Replace naive find('{') with brace-counting extract_json_object() that handles PTY noise prefixes containing '{' characters (same approach as Python version) * fix: add session/destroy handler + address remaining review findings - Add session/destroy and session/stop handlers (parity with Python version) - Sessions are removed from HashMap on destroy, dropping ShellHandle and aborting the pump task (Rust Drop semantics handle cleanup) * fix: exit pump task when receiver is dropped (prevent leak) * docs: update Dockerfile.agentcore and docs for native Rust bridge - Dockerfile.agentcore: remove Python/pip/uv/boto3, single binary only (~20MB) - docs/agentcore.md: update architecture diagram, prerequisites, IAM policy, config examples to reflect WebSocket shell bridge * fix: resolve borrow checker errors + deprecated aws_config::from_env - Take line_rx out of session via std::mem::replace to avoid holding &mut self across await points while also writing to stdout - Replace deprecated aws_config::from_env() with aws_config::defaults() * fix: resolve clippy lints — unused imports, dead code, iterator, useless into - Remove unused tracing imports (debug, error, warn) - Allow dead_code on runtime_session_id and cancel_strategy (future use) - Use iter().position() instead of manual index loop - Remove useless .into() on Vec<u8> → Vec<u8> * fix: remove unnecessary to_string in handle_cancel * fix: update agentcore test to expect self-spawn instead of uv * fix(agentcore): send initialize + session/new to kiro-cli in PTY The bridge was intercepting initialize/session/new from OAB but never forwarding them to kiro-cli. Without initialization, kiro-cli silently drops all subsequent messages (session/prompt returns empty). Fix: after opening the WebSocket PTY and launching kiro-cli acp, send initialize → wait for response → send session/new → store the kiro sessionId → use it when forwarding prompts. * feat(agentcore): make bridge agent-agnostic with --command flag The bridge is now a generic ACP proxy that works with any ACP agent, not just kiro-cli. The command to run in the PTY is configurable: [agentcore] runtime_arn = "arn:..." shell_command = "kiro-cli acp --trust-all-tools" # default Other examples: shell_command = "claude-agent-acp" shell_command = "codex-acp" shell_command = "opencode acp" CLI flag: openab agentcore-bridge --command "..." * fix(agentcore): retry initialize send instead of fixed 2s sleep * fix(agentcore): eager shell init during session/new Move open_shell + initialize + session/new handshake from handle_prompt into the session/new handler. By the time session/prompt arrives, the shell is already connected and kiro-cli is initialized. This eliminates the race where the first prompt times out because shell opening + agent boot takes longer than OAB's dispatch timeout. * fix(agentcore): remove runtimeSessionId from query string Per the Python SDK, session_id belongs in the signed header (X-Amzn-Bedrock-AgentCore-Runtime-Session-Id), not as a query param. The extra query param was causing 400 Bad Request. * fix(agentcore): remove qualifier=DEFAULT from shell URL * fix(agentcore): switch to presigned URL auth for WebSocket shell Use SigV4 query string auth (presigned URL) instead of header auth. Matches the Python SDK's connect_shell_presigned approach. Session ID passed as query param covered by signature. * fix: add qualifier=DEFAULT back * fix(agentcore): retry initialize with 10s wait per attempt kiro-cli needs several seconds to boot in the PTY before it starts listening on stdin. Send initialize, wait 10s for response, retry up to 5 times. * fix(agentcore): skip notifications in init handshake, add cwd to session/new kiro-cli sends notifications before responding to session/new. The init loop now checks for matching 'id' field and skips notifications. session/new includes cwd and mcpServers params as required by kiro-cli. * fix(agentcore): copy OAuth DB to /tmp before starting agent AgentCore's /mnt/agent filesystem doesn't support SQLite file locking. Copy the DB to /tmp (local filesystem) and set XDG_DATA_HOME=/tmp so kiro-cli finds auth on a lockable filesystem. * docs(agentcore): add architecture diagram and shell_command config --------- Co-authored-by: 超渡法師 <chaodu@openab.dev> Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
4 tasks
chaodu-agent
pushed a commit
that referenced
this pull request
Jun 18, 2026
Addresses review feedback from 口渡法師: 🔴 #1: register_thread() was never called - Discord: call register_thread(registry, channel_id, "discord") before submit - Slack: call register_thread(registry, thread_id, "slack") before submit - Pass ctl_registry through run_slack_adapter → handle_message - Add ctl_registry field to Discord Handler struct 🟡 #2: SlackAdapter missing rename_thread - Implement via conversations.rename API - Works for channels; message-thread rename not supported by Slack API
thepagent
pushed a commit
that referenced
this pull request
Jun 19, 2026
* feat: add `openab set/get` subcommands with Unix socket IPC Phase 1 implementation of runtime control via IPC: - Add `openab set <key> <value>` and `openab get <key>` subcommands - IPC over Unix domain socket (/tmp/openab.sock, configurable via OPENAB_SOCK) - `openab run` spawns a control socket server on startup - Client (set/get) connects to the running daemon, sends JSON, gets response Supported keys (Phase 1): - thread.name — rename the current Discord/Slack thread Architecture: - Same binary, different modes (like consul/vault) - `openab run` = daemon, `openab set/get` = client - Handler reads OPENAB_THREAD_ID env var for target channel - Extensible: new keys just add match arms in RuntimeHandler Closes #1131 * feat: add thread.archived and agent.status to openab set New keys: - thread.archived (true/false) — archive/unarchive Discord thread - agent.status — set bot custom presence (Discord gateway) Implementation: - Add archive_thread() to ChatAdapter trait (default no-op) - Implement via EditThread in DiscordAdapter - agent.status uses ShardMessenger::set_presence with ActivityData::custom - ShardMessenger shared via Arc<OnceLock<>> set in ready handler * feat: add --thread flag and adapter routing via ThreadRegistry - openab set/get now accept --thread <ID> (falls back to $OPENAB_THREAD_ID) - RuntimeHandler maintains a HashMap<platform, adapter> for multi-platform - ThreadRegistry (thread_id → platform) resolves which adapter handles a thread - Exposes ctl::new_registry() and ctl::register_thread() for adapters to call on message dispatch Usage: openab set --thread 1517277368 thread.name "new name" OPENAB_THREAD_ID=1517277368 openab set thread.name "new name" * feat: default rename_thread/archive_thread returns unsupported error Agent gets explicit feedback when the platform doesn't support the operation, rather than silent no-op success. * fix: wire ThreadRegistry into dispatch + implement Slack rename_thread Addresses review feedback from 口渡法師: 🔴 #1: register_thread() was never called - Discord: call register_thread(registry, channel_id, "discord") before submit - Slack: call register_thread(registry, thread_id, "slack") before submit - Pass ctl_registry through run_slack_adapter → handle_message - Add ctl_registry field to Discord Handler struct 🟡 #2: SlackAdapter missing rename_thread - Implement via conversations.rename API - Works for channels; message-thread rename not supported by Slack API * fix: chmod 0600 on control socket after bind (defense-in-depth) Per 覺渡法師 review — restrict socket to owner-only even in single-tenant pods, as a low-cost hardening measure. * fix: remove broken Slack rename_thread — fall back to unsupported error Per 擺渡法師 review: - conversations.rename only works on channels, not threads - Calling it on a thread returns invalid_channel - Calling it on a channel renames the entire public channel (dangerous) Correct behavior: Slack threads don't support rename, so use the trait default which returns 'not supported on this platform'. * fix: address 🟡 review items — log spam + agent.status messaging - dispatch.rs: downgrade rename_thread failure from warn → debug (expected on platforms that don't support it, not log-worthy) - ctl.rs: clarify agent.status error — 'only supported on Discord' instead of misleading 'shard not ready' * fix: clone ctl_registry before async move to avoid use-after-move E0382: ctl_registry was moved into the Slack tokio::spawn closure, then referenced again when constructing the Discord Handler. Clone before the closure so the original remains available. * fix: remove unsafe set_var from tests, use explicit socket path - Add spawn_server_at(path) and send_request_to(path) variants - Test uses explicit PathBuf instead of env var mutation - Avoids Rust 1.80+ UB with set_var in multi-threaded tests --------- Co-authored-by: 超渡法師 <超渡法師@openab.dev>
chaodu-agent
pushed a commit
that referenced
this pull request
Jun 24, 2026
- Add is_denied_user access control gate (🔴 #1) - Kill orphaned child process on empty-output early return (🔴 #2) - Add single-flight AtomicBool guard to prevent concurrent /auth (🟡 #4) - Truncate output to fit Discord 2000-char limit (🟡 #5) - Scope readers in a block to drop cleanly before wait (🟡 #6)
chaodu-agent
pushed a commit
that referenced
this pull request
Jun 24, 2026
- Add child.wait() arm to URL-collection select! so a fast-failing auth command reports immediately instead of stalling the full 30s window (#1) - Reject bot users in /auth, consistent with /remind (#8) - Record invoking user_id in the auth start audit log (#7) - Truncate output by UTF-16 code units to match Discord's 2000-char limit, preventing rejection on non-BMP-heavy output (#5) - Handle std::sync::Mutex poison in drain/collect paths to avoid panic cascade and silent output loss (#9) - Clarify the 'no output' error message with cause and remedy (#6) - Fix docs intro contradicting /auth DM-only and mark it DM-only in the command table (#3)
This was referenced Jun 24, 2026
brettchien
added a commit
to brettchien/openab
that referenced
this pull request
Jun 24, 2026
The §9 Q4 tail referenced 'openab-agent-mcp.md open items openabdev#1 (reqwest 0.12/0.13 split) and openabdev#8 (doctor/runtime two-store split)' — but that ADR's §10 Open Questions has only two items (mcp.json location; native-vs-broker parity), neither matching, and no such numbered items / terms exist anywhere in it. The phantom reference dated to the original draft. Replace with an accurate statement: McpCredentialStore reuses the same TokenStore/auth.json storage (openab-agent-mcp.md §6.1), so the lock lands once and serves both; the reqwest version conflict is the rmcp-OAuth dependency issue surfaced on the feat/openab-agent-mcp-resilience PR, not an mcp-ADR open item. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chaodu-agent
pushed a commit
that referenced
this pull request
Jun 26, 2026
Resolved: - [口渡 #1] channels empty = disabled (fail-safe), not all channels - [口渡 #2] Buffer lifecycle: swap-and-drain model, flush clears buffer - [口渡 #3] @mention triggers immediate flush + separate normal dispatch - [口渡 #4] Dedicated ambient session pool, isolated from main pool - [口渡 #5] Bot echo prevention: own messages never enter buffer - [口渡 #6] max_concurrent_flushes cap for global rate limiting - [口渡 #9] context_window = Discord API fetch, clarified semantics - [口渡 #10] Error handling table added (timeout, tool calls, etc.) - [核渡 #1] OpenClaw described as cross-platform group chat feature - [核渡 #2] Hermes backfill scope clarified (mention-only, skips free-response) - [核渡 #3] Added Hermes inline reply + per-user session detail - [覺渡 #1] Race condition resolved: swap-and-drain buffer model - [覺渡 #2] Session strategy fully specified (separate pool, rolling window) - [覺渡 #3] Mention detection fires before buffer, reuses existing logic - [覺渡 #4] flush_hard_cap = 50 as safety cap - [覺渡 #5] ±20% jitter on flush interval to prevent thundering herd - [覺渡 #6] context_window semantics clarified (API fetch, not buffer)
thepagent
added a commit
that referenced
this pull request
Jun 27, 2026
…g (codex + MCP) (#1190) * docs(adr): openab-agent multi-vendor OAuth & credential storage Proposed ADR for the openab-agent LLM-provider OAuth revamp: a two-axis OAuthVendor adapter (auth flow vs inference transport), a cross-process flock-guarded credential-store invariant for auth.json, the CLAUDE_CODE_OAUTH_TOKEN env route, a 14-variant vendor feasibility matrix, and the /auth (PR #1185) auth-trigger model. Surfaced while reviewing PR #1187 (first OAuth vendor). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): address review — prefer oauth2 crate, drop rollout, vendor names - Build the OAuthVendor driver on the official `oauth2` crate (already in-tree via the MCP side) instead of a hand-rolled PKCE/exchange/refresh flow; the Anthropic JSON-token-body quirk is applied via the crate's custom http-client hook. Reframe §8 accordingly (hand-rolled flow is the rejected alternative). - Remove the project Rollout-plan section (internal sequencing, not ADR material); keep the race-window mitigation in §5.4. - Use vendor names only; drop internal fleet-agent references from the matrix. - Replace unexplained "ToS-gray" with "ToS-risk" + a definition. - Fix cross-references (crate-qualified paths, §-refs, line numbers) and move the settled model-default decision under "Decisions & open questions". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): per-tenant refresh lock + per-user PKCE keying (Mira review r1) Fold in two release-blocker-class concurrency fixes from Mira's review: - §5.4 refresh-token rotation: the prior "refresh outside lock, re-read on commit" claim was wrong — N processes each send a refresh with the same RT_old before committing, tripping OAuth 2.1 §10.4 reuse -> token-family revocation. Replace with a per-tenant exclusive lock: network refresh held under the tenant lock only (not the global lock), so exactly one refresh per tenant per expiry with no head-of-line blocking across tenants. Extends to mcp:<server> tenants. - §7 /auth: key the pending PKCE verifier+state by the initiating Discord user id instead of a single global entry — prevents concurrent-user overwrite (PKCE mismatch) and session hijack. - §5.1 OAuthVendor::redirect() -> Option, since device flows have no redirect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): bundled-secret storage + pending-entry GC (Mira review r2) - §6/§9 Q2: correct the stale "git-safe" claim on the gemini GOCSPX- secret. The value is non-confidential by RFC 8252 / Google docs, but GitHub now push-protects Google secrets by default (changelog 2026-03) and partner-scans them for auto-revoke, so a raw literal is not safe in a public repo. Decide: encode-at-rest (scanner-evasion for a non-secret, NOT a security control) or env-inject at runtime — not raw text. - §7: pending PKCE entries get created_at + a 15-min GC sweep in with_auth_locked so abandoned /auth attempts don't accumulate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): close §9 Q2/Q3 — env-injection default, vendor go/no-go (Brett decisions r3) - §9 Q2 DECIDED: env-injection (b) is the default for the gemini/agy bundled client_secret; encode-at-rest (a) is the fallback for bundled zero-config binaries only, framed as scanner-evasion (not security). Cite rclone rcloneEncryptedClientSecret + obscure.MustReveal() as the canonical precedent (§10). - §9 Q3 DECIDED: GO gemini/grok (first wave) + agy (experimental, opt-in, ToS caveat — shares gemini's Code-Assist provider, residual risk is ToS not secret storage); No-Go cursor/kiro. Mirror as a build-decision line under §6. - §10: add rclone + GitHub/Google secret-scanning references. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): confirm agy secret + ecosystem evidence (GitHub survey r4) GitHub survey 2026-06-24 grounds the agy GO decision: - §6/§9 Q2: agy client_secret requirement CONFIRMED — it needs a GOCSPX- secret, a public constant >=20 antigravity-auth repos hardcode verbatim (NoeFabris/opencode-antigravity-auth, router-for-me/CLIProxyAPI, ...). Literal deliberately NOT pasted into the doc — would trip the very §9 Q2 push-protection, so we dogfood the env/encode decision. Redirect confirmed localhost:51121/oauth-callback. - §9 Q3: ecosystem evidence — agy OAuth is widely ported (opencode/pi/hermes/ openclaw plugins + proxies), proving the integration, while the same ecosystem's anti-ban / quota-locking / multi-account-rotation tooling empirically confirms the ToS-ban + 429 risks, reinforcing the opt-in gate. - §10: add the antigravity OAuth ecosystem reference set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): agy-vendor vs agy-CLI/ACP clarification + MCP store revamp in scope (r5) Grounded by a codebase survey (2026-06-24): - §4/§6: clarify "agy as a GO vendor" means a native OAuthVendor + Code-Assist inference provider — it does NOT run the agy CLI. agy speaks no ACP; the existing `antigravity` runtime variant (Mira/ECS) only works via a dedicated agy-acp adapter that shells out to the agy binary per prompt and polls its SQLite DB. The provider path sidesteps ACP entirely and supersedes the CLI-wrapper for native use, so agy's lack of ACP doesn't block the GO. - §5.4/§9 Q4: make the MCP CredentialStore revamp explicitly in-scope. auth.json has NO lock today (only atomic rename); provider save_tokens (auth.rs:234) and McpCredentialStore::save/clear (auth.rs:284-328) are two independent unlocked RMW callers, so with_auth_locked must wrap BOTH or the race persists. Also correct the stale save_tokens_for name and note with_auth_locked is new. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): fix stale §9 Q4 cross-reference The §9 Q4 tail referenced 'openab-agent-mcp.md open items #1 (reqwest 0.12/0.13 split) and #8 (doctor/runtime two-store split)' — but that ADR's §10 Open Questions has only two items (mcp.json location; native-vs-broker parity), neither matching, and no such numbered items / terms exist anywhere in it. The phantom reference dated to the original draft. Replace with an accurate statement: McpCredentialStore reuses the same TokenStore/auth.json storage (openab-agent-mcp.md §6.1), so the lock lands once and serves both; the reqwest version conflict is the rmcp-OAuth dependency issue surfaced on the feat/openab-agent-mcp-resilience PR, not an mcp-ADR open item. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(adr): §9 Q2 — encode-at-rest is the default, env is the alternative (Brett) Brett's call given the 93%-plaintext ecosystem survey: keep the bundled zero-config UX via encode-at-rest (obscure, rclone obscure.MustReveal style) as the DEFAULT, with env-injection as the alternative for fleet/pod. Reverses the prior (b)-default ordering. Framing unchanged: encode-at-rest is scanner-evasion for a non-confidential value, not a security control. Record the survey numbers (99/107 plaintext; auto-revoke largely unrealized) and that the real risk mitigated is org-repo push-protection friction, not credential loss. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(auth): cross-process locking for auth.json — codex + MCP (ADR §5.4) Implements the ADR §5.4 invariant. auth.json had NO lock: provider save_tokens and McpCredentialStore::save/clear each did an independent unlocked read-modify-write, so a concurrent codex refresh and MCP save last-writer-wins the whole map, and N processes (one openab-agent per Discord thread) could each refresh the codex token with the same RT_old → OAuth 2.1 §10.4 token-family revocation (fleet-wide logout). Two locks, flock(2) on sidecar files (kernel auto-releases on death), cfg(unix) with a non-unix no-op: - with_auth_locked: global exclusive lock across the re-read -> mutate -> atomic-write. ALL writers funnel through it — save_tokens (codex) and McpCredentialStore::save/clear (MCP) — so writers merge onto the latest on-disk state instead of lost-updating. Held only for the fast file RMW, never across network I/O. - lock_tenant_refresh: per-tenant refresh serialisation. get_valid_token / force_refresh take the codex tenant lock (non-blocking try + async backoff + 10s timeout, held across the network refresh), with a double-checked re-read so a process that waited adopts the token another already refreshed → exactly one real refresh per tenant per expiry, no RT_old reuse. Uses libc::flock (already a cfg(unix) dep); rustix is NOT in-tree (ADR text was optimistic). New test asserts the locked RMW merges codex + MCP tenants without lost-update. fmt + clippy -D warnings + test (191 passed) green. Known gap (for review): the MCP *network* refresh is owned by rmcp's AuthorizationManager (it refreshes then calls CredentialStore::save), so MCP writes get the file-integrity lock but MCP refreshes are not yet tenant- serialised. Closing that needs an rmcp-level hook — proposed as follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(auth): address Mira code review — pending GC + portable WouldBlock Round-2 code review (Mira): - §7 pending-entry GC: add created_at to PendingPasteLogin and sweep AuthEntry::Pending older than 15 min inside with_auth_locked on every write, so abandoned /auth two-step attempts don't accumulate. PendingPasteLogin is currently a legacy tombstone with no live writer (PKCE state lives in rmcp's in-memory StateStore); the field + GC land now per ADR §7 and are forward-compatible with the forthcoming /auth two-step flow, and meanwhile sweep legacy stray entries (created_at default 0 reads as ancient). New test covers stale-swept / fresh-kept / real-tenant-untouched. - flock_try_exclusive: match std::io::ErrorKind::WouldBlock instead of a single raw EWOULDBLOCK errno, covering EAGAIN/EWOULDBLOCK across libc/BSD. - MCP network-refresh serialisation stays a follow-up (rmcp owns the refresh) — Mira concurs. fmt + clippy -D warnings + test (192 passed) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(auth): serialise MCP token refresh cross-process (close §5.4 (b) gap) Closes the known gap from f370110: MCP refreshes now get the same per-tenant serialisation as codex. rmcp's CredentialStore exposes no pre-refresh hook, but openab drives the MCP refresh explicitly in McpRuntimeManager::resolve_oauth_dial via client.get_access_token(). Wrap that call (per-server) with the existing auth::lock_tenant_refresh so only one process refreshes a given server at a time. No explicit double-check needed: rmcp's get_access_token re-load()s auth.json each call and returns the cached token without a network refresh when remaining >= REFRESH_BUFFER (rmcp auth.rs:1238), so a process that loses the race adopts the token the winner wrote to the shared file — no second RT_old presentation, no OAuth 2.1 §10.4 family revocation. rmcp already single-flights within one process via its AuthorizationManager Mutex; this closes the cross-process gap. - auth.rs: lock_tenant_refresh + AuthFileLock made pub(crate) for the mcp module. - ADR §5.4: document the resolve_oauth_dial serialisation point. fmt + clippy -D warnings + test (192 passed) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(auth): /simplify cleanups — dedup lock acquire, reuse refresh fd Quality-only cleanups from a 4-angle /simplify pass (no behaviour change): - Extract lock_global(path) -> Result<Option<AuthFileLock>> and route both with_auth_locked and McpCredentialStore::clear through it, so the "global" sidecar name + the cfg(unix) acquire live in one place instead of two copy-pasted blocks. clear flattens (drop the inner run closure). - lock_tenant_refresh opens the lock fd once and re-issues flock on it each retry, instead of re-opening (and re-create_dir_all-ing) the file every 100ms under contention. Removes the now-unused flock_try_exclusive helper. - Document lock_tenant_refresh's double-check contract (the lock only serialises; callers must re-check freshness after acquiring). fmt + clippy -D warnings + test (192 passed) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(auth): /code-review — harden GC + refresh-lock contract comments Doc-only clarifications from a /code-review pass (no behaviour change): - PendingPasteLogin.created_at: warn loudly that any writer of a fresh Pending entry MUST stamp created_at, else gc_stale_pending sweeps it on the next locked write (latent footgun for the forthcoming /auth two-step writer). - lock_tenant_refresh: correct the contract — reuse-safety comes from loading the refresh token INSIDE the lock (so force_refresh, which always refreshes on a 401, is reuse-safe too); the post-lock expiry re-check is only an optimisation to skip a redundant refresh. Review also surfaced a pre-existing, out-of-scope namespacing gap (an MCP server literally named "codex" collides with the codex tenant's auth.json key and refresh lock — committed MCP creds use the bare server name, not mcp:<server> as ADR §5.4 describes) — flagged for a separate change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * build(openab-agent): declare standalone [workspace] to fix CI ci-openab-agent.yml runs cargo fmt/clippy/test/build with working-directory: openab-agent, but the crate is not a member of the parent openab workspace (members = crates/openab-core, openab-gateway) and was not excluded, so cargo errors 'current package believes it's in a workspace when it's not' and every step fails before doing any work. openab-agent is intentionally standalone (own version + dual reqwest 0.12/0.13 for rmcp), so the correct fix is an empty [workspace] table making it its own root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revert "build(openab-agent): declare standalone [workspace] to fix CI" This reverts commit b40693c. * ci(openab-agent): append [workspace] in CI + harden ACP smoke test ci-openab-agent.yml ran cargo from working-directory: openab-agent without the [workspace] table the crate needs (it's standalone, not a parent-workspace member), so every step failed with 'believes it's in a workspace when it's not' — the workflow had been red independently of any PR. Dockerfile.unified already works around this by appending the table at build time; replicate that in the workflow rather than committing it to Cargo.toml (which would double-append in the Dockerfile and break the image build). Also harden the ACP smoke test: build the release binary in its own step and bump the response timeout 5s -> 30s so a loaded runner doesn't flake the agent's first-response window (the binary itself responds in <1s locally, verified incl. a clean-HOME release build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(openab-agent): surface ACP smoke-test stderr + exit code for diagnosis The smoke test produces empty stdout only in CI (the binary responds with agentInfo in <1s locally across debug/release/clean-HOME/CI-env-var runs). Capture stderr + exit code so the next CI run reveals why stdout is empty. * openab-agent: address review findings on OAuth ADR + auth.json locking Resolves the Changes-Requested review on the multi-vendor OAuth ADR + cross-process auth.json locking PR. No behavioural change beyond log level and a non-unix diagnostic. ADR (docs/adr/openab-agent-oauth.md) - Fix lock-file names to match code: auth.json.global.lock and auth.json.refresh.<tenant>.lock (were auth.json.lock / auth.json.<tenant>.lock). - Add the path parameter to the with_auth_locked pseudocode signature. - Correct the MCP refresh note: initialize_from_store() does the disk reload, not get_access_token. - Correct the crate note: libc::flock directly (rustix is not in-tree). - Document the deliberate fail-open trade-off on tenant-lock timeout. - Mark the default-model removal (Decision 1) as a follow-up; this PR ships the ADR + locking only. Code - auth.rs: escalate the tenant-lock timeout log from warn! to error! and document the fail-open trade-off on lock_tenant_refresh. - auth.rs: non-unix lock_global no-op now warns once instead of silently providing zero cross-process protection. - mcp/runtime.rs: fix the rmcp reload comment and document the cross-module invariant that the refresh lock and credential entry share the server name. CI (ci-openab-agent.yml) - ACP smoke test now fails on a non-zero/timeout exit code, and the agentInfo assertions use { } so exit fails the step rather than just a subshell. Gate: cargo fmt --check, clippy -D warnings, test (192 passed) all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * openab-agent: fail closed on a contended refresh lock (F6) Replaces the fail-open tenant-lock timeout with a bounded-refresh + fail-closed design, so a contended lock can no longer reintroduce the double-refresh it exists to prevent. - Bound the refresh network call with an explicit 8s HTTP timeout on both the codex client (auth.rs) and the MCP AuthClient (mcp/runtime.rs), strictly shorter than the 10s lock-acquire deadline. With flock(2) auto-release on death, a live holder always frees the tenant lock before a waiter's deadline, so a lock-acquire timeout is genuinely abnormal. - lock_tenant_refresh now returns RefreshLock { Held, Unavailable, TimedOut }. On TimedOut callers fail closed instead of refreshing unserialised: - codex get_valid_token / force_refresh return a retryable error; - MCP resolve_oauth_dial returns a new OauthDialError::Transient that leaves the server retryable WITHOUT forcing re-login (NeedsAuth) or tripping the circuit breaker (auth-level failures still map to NeedsAuth as before). A filesystem error opening the sidecar returns Unavailable and degrades to a best-effort unserialised refresh rather than blocking every refresh. - Add a fail-closed timeout test (injectable deadline) and update ADR §5.4 to document the bounded-refresh + fail-closed invariant, superseding the earlier fail-open note. Gate: cargo fmt --check, clippy -D warnings, test (193 passed) all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * openab-agent: size lock timeout above worst-case multi-call refresh hold Mira review (PR #1190) found the fail-closed invariant held only for the codex path. The MCP path holds the tenant lock across TWO sequential bounded calls — rmcp's initialize_from_store() (AS discovery) then get_access_token() (refresh) — so the worst-case lock-hold is ~2 x REFRESH_HTTP_TIMEOUT (16s), which could exceed the fixed 10s lock deadline and fail a waiter closed while the holder is still legitimately progressing. Derive REFRESH_LOCK_TIMEOUT from the bound instead of hardcoding it: REFRESH_LOCK_TIMEOUT = MAX_REFRESH_ROUND_TRIPS (2) * REFRESH_HTTP_TIMEOUT + 4s = 20s so the deadline is always above the worst-case hold and only a genuinely stuck holder trips it. Normal-case latency is unchanged (the waiter polls every 100ms and acquires as soon as the holder releases). Update the fn doc and ADR §5.4 to state the multi-call hold explicitly. Gate: cargo fmt --check, clippy -D warnings, test (193 passed) all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: thepagent <hehsieh1010@gmail.com>
thepagent
added a commit
that referenced
this pull request
Jun 27, 2026
* docs: add ADR for Ambient Mode * docs: update ADR with batch flush strategy and prior art * docs: address all review findings (口渡, 核渡, 覺渡) Resolved: - [口渡 #1] channels empty = disabled (fail-safe), not all channels - [口渡 #2] Buffer lifecycle: swap-and-drain model, flush clears buffer - [口渡 #3] @mention triggers immediate flush + separate normal dispatch - [口渡 #4] Dedicated ambient session pool, isolated from main pool - [口渡 #5] Bot echo prevention: own messages never enter buffer - [口渡 #6] max_concurrent_flushes cap for global rate limiting - [口渡 #9] context_window = Discord API fetch, clarified semantics - [口渡 #10] Error handling table added (timeout, tool calls, etc.) - [核渡 #1] OpenClaw described as cross-platform group chat feature - [核渡 #2] Hermes backfill scope clarified (mention-only, skips free-response) - [核渡 #3] Added Hermes inline reply + per-user session detail - [覺渡 #1] Race condition resolved: swap-and-drain buffer model - [覺渡 #2] Session strategy fully specified (separate pool, rolling window) - [覺渡 #3] Mention detection fires before buffer, reuses existing logic - [覺渡 #4] flush_hard_cap = 50 as safety cap - [覺渡 #5] ±20% jitter on flush interval to prevent thundering herd - [覺渡 #6] context_window semantics clarified (API fetch, not buffer) * docs: integrate with existing Dispatcher/turn-boundary batching infra Ambient Mode reuses the Dispatcher, BufferedMessage, consumer_loop, and pack_arrival_event infrastructure from PR #686 (message_processing_mode). Key difference: ambient consumer uses timer-based flush instead of turn-boundary drain. * docs: add semaphore + error handling to ambient consumer pseudocode [覺渡 R2#1] flush_semaphore.acquire() before dispatch [覺渡 R2#2] match on dispatch result, warn + discard on error * docs: address 擺渡 R2 findings — race condition, thinking msg, bot loop [擺渡 #1] Add concurrent reply prevention via per-channel flushing flag [擺渡 #2] Ambient dispatches post directly, no thinking placeholder [擺渡 #3] Handle None on first rx.recv() (channel closed → exit) [擺渡 #4] Bot loop prevention: MAX_CONSECUTIVE_BOT_TURNS + prompt instruction * docs: strengthen F1 (discard buffer on mention) + F4 (bot msgs off by default) [擺渡 F1] @mention discards ambient buffer instead of flushing it; cancel in-flight ambient on mention arrival [擺渡 F4] allow_bot_messages defaults to 'off' for ambient channels * docs: fix wording inconsistency — mention discards buffer, not flushes [口渡 R3#1] Unify Message Filtering wording with Immediate flush section * docs(adr): add detailed ASCII architecture diagrams for ambient mode - Architecture Overview: full message routing decision tree - Dual-Path Concurrency: timeline showing mention vs ambient interaction - Shows buffer lifecycle, flush phases, and response routing * docs(adr): unify ambient config under top-level [ambient] section - Group all ambient settings under [ambient] instead of scattered [discord.ambient], [pool.ambient], [ambient.limits] - Platform-specific config in [ambient.discord] (extensible to slack/telegram) - Simplify pool key names: remove redundant ambient_ prefix (session_ttl_minutes, context_flushes) * docs(adr): move ambient mode ADR to docs/adr/ambient.md Addresses external reviewer feedback from @thepagent and team consensus. docs/steering/ is for process guides; docs/adr/ is for ADRs. * docs(adr): address review findings from 法師團隊 Fixes: - Unify @mention handling path (discard buffer, not flush) — resolves 3 contradictions across flush triggers table, concurrent reply section, and implementation details - Replace non-existent MAX_CONSECUTIVE_BOT_TURNS with actual max_bot_turns - Explain flush_hard_cap vs flush_max_messages relationship - Add post_guard for atomic check-and-post (TOCTOU race fix) - Add flush_timeout_seconds for AtomicBool safety recovery - Reconcile Buffer Lifecycle (conceptual) with mpsc channel (implementation) - Remove contradictory 'extend enum' wording from message_processing_mode - Add allow_bot_messages to [ambient.discord] config - Simplify pool key names (drop ambient_ prefix) * docs(adr): fix author attribution to chaodu-agent --------- Co-authored-by: 超渡法師 <超渡法師@openab.dev> Co-authored-by: chaodu-agent <chaodu-agent@openab.dev> Co-authored-by: thepagent <hehsieh1010@gmail.com>
This was referenced Jul 1, 2026
This was referenced Jul 7, 2026
slps970093
added a commit
to slps970093/openab-agnet
that referenced
this pull request
Jul 10, 2026
…ev#5 openabdev#7 - openabdev#1: add url_expires param to url_hint_block and download_and_read_text_file; Discord caller passes true so the hint warns the agent that CDN URLs expire in ~24 hours; Slack passes false - openabdev#3: document in function doc that Slack URL hints are visibility-only for most agents since they do not hold the bot token - openabdev#5: replace hardcoded '512 KB' string in hint text with TEXT_INLINE_LIMIT / 1024 so the string stays in sync with the constant - openabdev#7: separate TEXT_INLINE_LIMIT doc comment from download_and_read_text_file doc comment; remove stray blank line between doc and fn that triggered clippy::empty_line_after_doc_comments Add test: url_hint_block_with_expiry_includes_expiry_note verifies the 24-hour expiry warning appears when url_expires is true
thepagent
pushed a commit
that referenced
this pull request
Jul 13, 2026
* docs(adr): revise identity-trust-none to three-layer architecture Receiver → Trust Gate → Handler replaces the previous 'gate at handle_message()' design. Addresses all findings from the PR #1263 mob review (howie + 3 LLM reviewers). Key changes: - §4.2: Trust Gate is a dedicated ingress layer upstream of Handler - §5: New architecture diagram showing three-layer separation - §7: Implementation plan starts with Receiver/Handler split - Address #1: gate at actual convergence point (not handle_message) - Address #2: trust lookup keys off per-event platform (not adapter) - Address #3: slash commands gated (Handler is downstream of gate) - Address #4: exhaustive scattered-checks inventory - Address #5: explicit empty-vs-missing semantics - Address #6: phased rollout (Phase 0-3) - Address #7: echo rate-limit + bot exclusion + DM-preferred - Address #8: gateway vs first-class section precedence - Address #9: no static HashSet (runtime construction) - Address #10: structured logging on allow + deny - Address #11-#15: minor fixes (Teams ID, bot semantics, etc.) * docs(adr): address team review findings - Add type-level guarantee (GatedEvent vs InboundEvent) — compile-time enforcement, not just convention (#4) - Clarify Gateway Receiver is one receiver that demuxes by platform (#11) - Fix layer numbering inconsistency — use names, not numbers (#21) - Add sender ID format table with per-platform gotchas (#22, #23, #24) - Clarify is_bot bypass is caller-side, not inside decide() (擺渡-1) - Change echo group fallback to silent drop (avoid UID leakage) (#6) * docs(adr): add event loop binding design + fix is_bot L2 bypass - Add §5 'Event loop binding' section: run_platform generic pipeline, EventReceiver/EventHandler traits, main.rs startup wiring - Gateway platforms: one shared WS, demux by event.platform, fan-out to per-platform Handlers - Fix is_bot bypass: bots skip L3 but STILL enforce L2 scope (擺渡-1 🔴) - Add cross-crate boundary note for Gateway Receiver (擺渡-2 🟡) - Include binding topology summary diagram * docs(adr): address round 3 findings — tighten pseudocode precision - GatedEvent: private field in narrow module (not pub(crate)), with read-only accessors and module layout diagram (諸葛村夫-1) - gate_event: use configs.get().surface_allowed() to match real API (擺渡-3) - Phase table: add Phase 0.5 for current partially-wired state on main, clarify Phase 2 means 'refuse to start' (諸葛村夫-2) * docs(adr): fix seal() visibility — private fn, not pub(super) seal() lives in the same module as gate_event(), so it should be a plain private fn. pub(super) would unnecessarily expose it to the parent module. * docs(adr): fix stale module layout comment — constructor is private, not pub(super) * docs(adr): address LINE/Slack/Feishu review feedback - Replace line-number refs with symbol+semantic descriptions (drift-proof) - Rewrite echo section: platform-specific echo trait (LINE=Reply only, Slack=chat.postEphemeral, Discord=DM); leak-safe content by scope - Add is_bot per-platform derivation table (pinned canonical rules) - Document trusted_bot_ids as shared config (resolves Feishu circular dep) - Clarify slash commands scope (Slack doesn't consume them) - Update Slack sender ID: Enterprise Grid composite key (team_id, sender_id) - Add non-message events section (assistant_thread_started must gate) - Add Slack scope notes (Socket Mode only, MPIM=channel) - Add LINE group policy: open/members dual-mode in decide() - Add LINE @mention pre-filter as documented Receiver exception - Feishu: gateway=L1 only, eliminate double-gating, empty list=deny-all Addresses feedback from: - @luffy-aiagent (LINE platform review) - @antigenius0910 (Slack platform review) - @wangyuyan-agent (Feishu platform review) * docs(adr): address 9 review findings — API contract gaps + hardening Fixes identified during group review: F1+F6: Add workspace_id to InboundEvent; define Slack Enterprise Grid canonical sender_id format and config examples for Grid deployments F2: Replace HashMap<String, TrustConfig> with enum PlatformTrustConfig (Base/Line/Slack) — LINE group policy and Slack workspace-scoped trust now have proper type representations F3: Add cron bypass in gate_event() — system-initiated events skip L2/L3 (platform='cron' or sender_id='openab-cron') F4: Add #[cfg(test)] assume_trusted_for_test() constructor for GatedEvent — enables Handler unit testing without full pipeline F5+F9: Change into_inner() to pub(crate); adjust safety claim wording from 'bypass impossible' to 'accidental bypass compile error' F7: Change gate_event() signature to take InboundEvent by value — zero-copy hot path (no .clone() on RawPlatformEvent) F8: Specify bounded LRU cache (max_capacity + TTL) for rate-limit state — prevents OOM from random sender_id flooding * fix(adr): address review findings F1-F3 on identity-trust-none F1 (critical): Remove sender_id spoofing hole in cron bypass — only check platform == "cron" since WeCom allows freeform UserIDs that could match any synthetic value. Update rationale accordingly. F2: Add WeCom, Google Chat, MS Teams to pinned is_bot derivation and echo-delivery tables (all 8 platforms now covered). F3: Fix pseudocode precision — unwrap_or no longer borrows a temporary; add PlatformTrustConfigs::get() and PlatformTrustConfig::surface_allowed() delegating method used by gate_event. * fix(adr): address group review round 2 — 9 findings from B1/B4/B5/B8/B11/B12/B15 Fixes: - WeCom is_bot: remove enter_agent (user-initiated, not bot); keep only trusted_bot_ids (B5 F1, B12 F1) - SlackTrustConfig::decide(): workspace_users is now strict override (ignore allow_all_users); base fallback supports team_id:user_id composite key for Enterprise Grid (B4 F1, B5 F2, B12 F3) - LINE group policy: fix prose vs code contradiction — unconfigured groups use default_group_policy, not DenyScope (B12 F2) - Reserved platform validation: MUST-level requirement for all external Receivers to reject reserved platform names; cron bypass invariant documented; Phase 1 SHOULD for typed InboundSource enum (B1) - decide() simplified to use self.get() — remove duplicate default logic (B5 F3) - Echo rate-limit key updated to (platform, workspace_id, sender_id) (B12 F4, B15 F2) - Module layout: InboundEvent in mod.rs (public), gate.rs narrow (B12 F5) - into_inner() trust boundary doc: module-level vs crate-level explained, Phase 1 SHOULD for lint/annotation (B11, B15 F1) - Slack Enterprise Grid gotcha: split dense table cell into footnote (B8 F2) * fix(adr): add platform lowercase invariant to InboundEvent (B14 F2) InboundEvent.platform MUST be lowercase — Receivers normalize before constructing. This ensures consistency between gate_event's == "cron" check and PlatformTrustConfigs::get()'s to_lowercase() lookup. * docs(adr): clarify gw_event.platform source in run_gateway_platforms Add comment noting platform field is assigned by gateway routing config, not from webhook payload body — satisfies reserved platform invariant. * docs(adr): WeCom corrections from external review (canyugs#18) Three verified adjustments to the v2 trust architecture text: - WeCom callback mode is DM-only: channel_type always "direct" with a per-user channel id, so "group routing" does not exist for WeCom and its L2 scope is effectively allow_dm only (verified wecom.rs:1059-71) - Sender-id table: plain UserIDs are self-built-app only; external contact / ISV callbacks carry wm/wo-prefixed external_userid or encrypted OpenUserID (verified against official WeCom API docs) - Bot-bypass wording: is_bot is hardcoded false in the WeCom Receiver today, so the L3 bot-bypass is a no-op for WeCom — the bypass is available uniformly but effective only where the Receiver can derive is_bot Credit: canyugs. * docs(adr): refresh Phase 0.5 snapshot to shipped state Since the last revision, main gained: Slack L3 gate (#1363), per-platform [section] trust for all 8 platforms (#1365/#1366/#1385), full config-first parity with conformance guard (#1375/#1387), and the L1 unenforceable-auth startup warning (#1373). The Phase 0.5 row now lists the actual inventory and names the one outstanding Phase 1 prerequisite (standalone WS path still on should_skip_event, tracked on #1356). --------- Co-authored-by: chaodu-agent <chaodu-agent@openab.dev> Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
5 tasks
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Splits the build into two stages: first build deps with a dummy main.rs (cached), then build the actual source. Subsequent builds only recompile the app code, not all 100+ crates.