From 8d3b06dc12ea9f0369c13fb26d5ec74af110a922 Mon Sep 17 00:00:00 2001 From: chaodufashi Date: Fri, 1 May 2026 13:35:05 -0400 Subject: [PATCH 1/4] docs: add AGENTS.md with project guidelines for AI contributors --- AGENTS.md | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..3b8710ea --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,156 @@ +# OpenAB — Agent Guidelines + +You are contributing to OpenAB, a lightweight Rust-based ACP harness bridging Discord, Slack, and webhook platforms to coding CLIs over stdio JSON-RPC. + +## Architecture + +``` +src/ +├── main.rs # entrypoint, multi-adapter startup, graceful shutdown +├── adapter.rs # ChatAdapter trait, AdapterRouter (platform-agnostic) +├── config.rs # TOML config + ${ENV_VAR} expansion +├── discord.rs # DiscordAdapter (serenity EventHandler) +├── slack.rs # SlackAdapter (Socket Mode) +├── gateway.rs # Custom Gateway WebSocket adapter +├── cron.rs # Config-driven + usercron scheduler +├── media.rs # Image resize/compress + STT download +├── format.rs # Message splitting, thread name shortening +├── markdown.rs # Discord markdown rendering +├── reactions.rs # Emoji status controller (debounce, stall detection) +├── stt.rs # Speech-to-text (Whisper API) +└── acp/ + ├── protocol.rs # JSON-RPC types + ACP event classification + ├── connection.rs # Spawn CLI, stdio JSON-RPC, env_clear whitelist + └── pool.rs # Session key → AcpConnection map + lifecycle +``` + +**Helm chart:** `charts/openab/` — Go templates, values.yaml, NOTES.txt +**Gateway:** `gateway/` — standalone Rust binary for Telegram/LINE/Teams +**Docs:** `docs/` — user-facing guides, ADRs, config reference + +## Message Flow + +``` +Discord/Slack event + → channel allowlist check + → user allowlist check (bot messages bypass this) + → bot message filter (off/mentions/all) + → thread detection (thread_metadata = thread, NOT parent_id) + → bot ownership check (bot_owns = thread creator matches bot user ID) + → multibot detection (allowUserMessages mode) + → handle_message → pool.get_or_create → ACP JSON-RPC +``` + +Understand this pipeline before modifying any adapter code. + +## Rust Conventions + +### Do +- `cargo fmt` + `cargo clippy -- -D warnings` before every commit +- Use `?` operator and `thiserror` enums for error handling +- Use `tracing` macros (`debug!`, `info!`, `warn!`, `error!`) — never `println!` +- Keep functions <50 lines, modules focused on one responsibility +- Test boundary cases: empty input, max limits, platform API constraints + +### Do NOT +- `.unwrap()` / `.expect()` in production code — only in `#[cfg(test)]` +- `unsafe` without safety comment and explicit justification +- Blocking calls in async context — use `tokio::task::spawn_blocking` +- Global mutable state — pass via function args or `Arc` +- Premature `.collect()` — keep iterators lazy + +## Critical Rules (from past incidents) + +### 1. Backward-Compatible Defaults + +New config fields MUST default to the previous behavior. Never change what existing deployments experience without an explicit opt-in. + +```rust +// WRONG: changes behavior for existing users +tool_display: String = "compact".to_string() + +// RIGHT: preserves existing behavior +tool_display: String = "full".to_string() // was always "full" before +``` + +### 2. Thread Detection + +The definitive rules — do NOT reinvent this: +- A channel is a thread if `thread_metadata` is present (NOT `parent_id`) +- `bot_owns` = thread's `owner_id` matches the bot's user ID +- Use `ChannelType` enum, not structural heuristics +- Forum posts are threads with `parent_id` pointing to a forum channel + +### 3. Security — Child Process Environment + +Agent subprocesses start with `env_clear()`. Only `HOME`, `PATH`, and explicit `[agent].env` keys are passed. Never leak `DISCORD_BOT_TOKEN` or other OAB credentials to the agent. + +### 4. Dockerfile Discipline + +There are 7 Dockerfiles: `Dockerfile`, `Dockerfile.claude`, `Dockerfile.codex`, `Dockerfile.copilot`, `Dockerfile.cursor`, `Dockerfile.gemini`, `Dockerfile.opencode`. + +**A change to one MUST be evaluated against ALL.** Common layers (base image, openab binary, tini) are shared — update all or explain why not. + +### 5. Cross-Platform + +Gate platform-specific code with `#[cfg(target_os = "...")]`. After adding Unix-only calls (libc, signals), verify with: +```bash +cargo check --target x86_64-pc-windows-gnu +``` + +### 6. Discord API Limits + +- Select menu: max 25 options (truncate with count, don't crash) +- Message content: max 2000 chars (use `format.rs` splitting) +- Embeds: max 4096 chars description +- Rate limits: respect `Ratelimit` headers, use serenity's built-in ratelimiter + +## Helm Chart Checklist + +Before merging any chart change: + +```bash +# Minimal values (default kiro agent) +helm template test charts/openab + +# Full values (all agents enabled) +helm template test charts/openab -f charts/openab/ci/full-values.yaml + +# Disabled agent path +helm template test charts/openab --set agents.kiro.enabled=false +``` + +Rules: +- Boolean fields: use `{{ if hasKey .Values "field" }}` not `{{ if .Values.field }}` (Go nil trap) +- Channel/user IDs: always `--set-string` (float64 precision loss) +- PVCs: set `"helm.sh/resource-policy": keep` — never delete user data on uninstall +- New values: add to `values.yaml` with comment + update `docs/config-reference.md` + +## Breaking Changes + +If your change alters existing behavior: + +1. Add `!` to commit type: `feat(cron)!: description` +2. Include migration steps in PR body +3. Update relevant docs with migration callout +4. Ensure release note has ⚠️ Breaking Change section + +## PR Standards + +- One logical change per PR +- Commit message: `type(scope): description` — types: `feat`, `fix`, `docs`, `refactor`, `test`, `ci`, `build` +- Include tests for bug fixes (regression test proving the fix) +- Run full check before pushing: + ```bash + cargo fmt && cargo clippy -- -D warnings && cargo test + ``` +- Reference issue numbers: `Closes #123` or `Fixes #456` + +## ADRs (Architecture Decision Records) + +Read `docs/adr/` before implementing features in these areas: +- Cron scheduler → `docs/adr/basic-cronjob.md` +- Custom Gateway → `docs/adr/custom-gateway.md` +- LINE adapter → `docs/adr/line-adapter.md` + +Write a new ADR if your feature touches >3 files or introduces a new subsystem. From 5c2eba3256aedffee3223a9cdcbe74d3752cb3b8 Mon Sep 17 00:00:00 2001 From: chaodufashi Date: Fri, 1 May 2026 13:36:18 -0400 Subject: [PATCH 2/4] docs: trim generic Rust advice, keep project-specific rules only --- AGENTS.md | 64 ++++++++++++++++++------------------------------------- 1 file changed, 21 insertions(+), 43 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3b8710ea..829b211b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,36 +43,12 @@ Discord/Slack event Understand this pipeline before modifying any adapter code. -## Rust Conventions - -### Do -- `cargo fmt` + `cargo clippy -- -D warnings` before every commit -- Use `?` operator and `thiserror` enums for error handling -- Use `tracing` macros (`debug!`, `info!`, `warn!`, `error!`) — never `println!` -- Keep functions <50 lines, modules focused on one responsibility -- Test boundary cases: empty input, max limits, platform API constraints - -### Do NOT -- `.unwrap()` / `.expect()` in production code — only in `#[cfg(test)]` -- `unsafe` without safety comment and explicit justification -- Blocking calls in async context — use `tokio::task::spawn_blocking` -- Global mutable state — pass via function args or `Arc` -- Premature `.collect()` — keep iterators lazy - -## Critical Rules (from past incidents) +## Critical Rules ### 1. Backward-Compatible Defaults New config fields MUST default to the previous behavior. Never change what existing deployments experience without an explicit opt-in. -```rust -// WRONG: changes behavior for existing users -tool_display: String = "compact".to_string() - -// RIGHT: preserves existing behavior -tool_display: String = "full".to_string() // was always "full" before -``` - ### 2. Thread Detection The definitive rules — do NOT reinvent this: @@ -89,11 +65,11 @@ Agent subprocesses start with `env_clear()`. Only `HOME`, `PATH`, and explicit ` There are 7 Dockerfiles: `Dockerfile`, `Dockerfile.claude`, `Dockerfile.codex`, `Dockerfile.copilot`, `Dockerfile.cursor`, `Dockerfile.gemini`, `Dockerfile.opencode`. -**A change to one MUST be evaluated against ALL.** Common layers (base image, openab binary, tini) are shared — update all or explain why not. +A change to one MUST be evaluated against ALL. Common layers (base image, openab binary, tini) are shared — update all or explain why not. ### 5. Cross-Platform -Gate platform-specific code with `#[cfg(target_os = "...")]`. After adding Unix-only calls (libc, signals), verify with: +Gate platform-specific code with `#[cfg(target_os = "...")]`. After adding Unix-only calls (libc, signals), verify: ```bash cargo check --target x86_64-pc-windows-gnu ``` @@ -102,25 +78,18 @@ cargo check --target x86_64-pc-windows-gnu - Select menu: max 25 options (truncate with count, don't crash) - Message content: max 2000 chars (use `format.rs` splitting) -- Embeds: max 4096 chars description -- Rate limits: respect `Ratelimit` headers, use serenity's built-in ratelimiter +- Rate limits: respect serenity's built-in ratelimiter ## Helm Chart Checklist Before merging any chart change: ```bash -# Minimal values (default kiro agent) helm template test charts/openab - -# Full values (all agents enabled) helm template test charts/openab -f charts/openab/ci/full-values.yaml - -# Disabled agent path helm template test charts/openab --set agents.kiro.enabled=false ``` -Rules: - Boolean fields: use `{{ if hasKey .Values "field" }}` not `{{ if .Values.field }}` (Go nil trap) - Channel/user IDs: always `--set-string` (float64 precision loss) - PVCs: set `"helm.sh/resource-policy": keep` — never delete user data on uninstall @@ -135,22 +104,31 @@ If your change alters existing behavior: 3. Update relevant docs with migration callout 4. Ensure release note has ⚠️ Breaking Change section +## Build & Verify + +```bash +cargo fmt +cargo clippy -- -D warnings +cargo test +``` + +All three must pass before pushing. For cross-platform changes, add: +```bash +cargo check --target x86_64-pc-windows-gnu +``` + ## PR Standards - One logical change per PR - Commit message: `type(scope): description` — types: `feat`, `fix`, `docs`, `refactor`, `test`, `ci`, `build` -- Include tests for bug fixes (regression test proving the fix) -- Run full check before pushing: - ```bash - cargo fmt && cargo clippy -- -D warnings && cargo test - ``` -- Reference issue numbers: `Closes #123` or `Fixes #456` +- Include regression tests for bug fixes +- Reference issues: `Closes #123` or `Fixes #456` -## ADRs (Architecture Decision Records) +## ADRs Read `docs/adr/` before implementing features in these areas: - Cron scheduler → `docs/adr/basic-cronjob.md` - Custom Gateway → `docs/adr/custom-gateway.md` - LINE adapter → `docs/adr/line-adapter.md` -Write a new ADR if your feature touches >3 files or introduces a new subsystem. +Write a new ADR if your feature introduces a new subsystem. From e1fac1b7295850b01e4e29ce8e4e693b1ffc9244 Mon Sep 17 00:00:00 2001 From: chaodufashi Date: Fri, 1 May 2026 17:41:18 +0000 Subject: [PATCH 3/4] =?UTF-8?q?docs:=20fix=20AGENTS.md=20accuracy=20?= =?UTF-8?q?=E2=80=94=20add=20missing=20src=20files,=20remove=20non-existen?= =?UTF-8?q?t=20ci/full-values.yaml,=20add=20multi-platform=20ADR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 829b211b..02489a3b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,13 @@ src/ ├── format.rs # Message splitting, thread name shortening ├── markdown.rs # Discord markdown rendering ├── reactions.rs # Emoji status controller (debounce, stall detection) +├── bot_turns.rs # Bot turn tracking +├── error_display.rs # User-facing error formatting ├── stt.rs # Speech-to-text (Whisper API) +├── setup/ # Interactive setup wizard +│ ├── config.rs # Setup config helpers +│ ├── validate.rs # Input validation +│ └── wizard.rs # CLI setup flow └── acp/ ├── protocol.rs # JSON-RPC types + ACP event classification ├── connection.rs # Spawn CLI, stdio JSON-RPC, env_clear whitelist @@ -86,7 +92,6 @@ Before merging any chart change: ```bash helm template test charts/openab -helm template test charts/openab -f charts/openab/ci/full-values.yaml helm template test charts/openab --set agents.kiro.enabled=false ``` @@ -130,5 +135,6 @@ Read `docs/adr/` before implementing features in these areas: - Cron scheduler → `docs/adr/basic-cronjob.md` - Custom Gateway → `docs/adr/custom-gateway.md` - LINE adapter → `docs/adr/line-adapter.md` +- Multi-platform adapters → `docs/adr/multi-platform-adapters.md` Write a new ADR if your feature introduces a new subsystem. From 3b1cd8a9d21b45b3ea2f31143c94c67e9319c9d6 Mon Sep 17 00:00:00 2001 From: chaodufashi Date: Fri, 1 May 2026 17:45:48 +0000 Subject: [PATCH 4/4] docs: fix env_clear description to match actual implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List all baseline env vars passed to child processes: - All platforms: HOME, PATH - Unix: USER - Windows: USERPROFILE, USERNAME, SystemRoot, SystemDrive Caught by 擺渡法師 review. --- AGENTS.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 02489a3b..666c4416 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,7 +65,13 @@ The definitive rules — do NOT reinvent this: ### 3. Security — Child Process Environment -Agent subprocesses start with `env_clear()`. Only `HOME`, `PATH`, and explicit `[agent].env` keys are passed. Never leak `DISCORD_BOT_TOKEN` or other OAB credentials to the agent. +Agent subprocesses start with `env_clear()`. The baseline env passed to the child is: +- **All platforms:** `HOME`, `PATH` +- **Unix only:** `USER` +- **Windows only:** `USERPROFILE`, `USERNAME`, `SystemRoot`, `SystemDrive` +- Plus any explicit `[agent].env` keys (logged with a prompt-injection warning) + +Never leak `DISCORD_BOT_TOKEN` or other OAB credentials to the agent. ### 4. Dockerfile Discipline