Skip to content

Fix/memory leak process group#1

Merged
Joseph19820124 merged 45 commits intomainfrom
fix/memory-leak-process-group
Apr 14, 2026
Merged

Fix/memory leak process group#1
Joseph19820124 merged 45 commits intomainfrom
fix/memory-leak-process-group

Conversation

@Joseph19820124
Copy link
Copy Markdown
Owner

No description provided.

Openbot and others added 30 commits April 12, 2026 22:33
Add openab setup command with 5-step interactive wizard:
- Discord bot token verification via API
- Server and channel selection with guild/channel fetch
- Agent configuration with kiro/claude/codex/gemini choices
- Session pool settings
- Reaction emoji customization

New deps: clap, rpassword, atty, unicode-width, ureq
Tests: validate_bot_token, validate_channel_id, generate_config, kiro args
Add openab setup command with 5-step interactive wizard:
- Discord bot token verification via API
- Server and channel selection with guild/channel fetch
- Agent configuration with kiro/claude/codex/gemini choices
- Session pool settings
- Reaction emoji customization

New deps: clap, rpassword, atty, unicode-width, ureq
Tests: validate_bot_token, validate_channel_id, generate_config, kiro args
Show npm install commands for each agent (claude, kiro, codex, gemini)
so users know how to install the agent before selecting it.
…-aware guidance

- Map agent choice to actual binary (kiro-cli/claude-agent-acp/codex-acp/gemini) per README
- Add deployment target prompt (Local vs Docker/k8s) to pick sensible working_dir default
- Hardcode reactions defaults instead of prompting; keep [reactions] sections in output
- Mask bot_token in preview; still write real token to config.toml
- Show per-agent next steps (install/auth/run) tailored to deployment target
- Local dev uses `cargo run -- run <path>`; Docker path points to Helm + kubectl exec

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use clamp() instead of max().min() pattern
- Collapse nested if statements
- Use \&Path instead of \&PathBuf in print_next_steps
- Local: always '.'
- K8s + kiro: '/home/agent'
- K8s + claude/codex/gemini: '/home/node'
- Replace manual args().nth(1) check with clap::Subcommand (Commands enum)
- Add --output flag to openab setup command
- Replace atty crate with std::io::IsTerminal (removes atty dependency)
…only

- Remove unused 'dim' field from Colors struct
- Add #[cfg(test)] to validate_agent_command (only used in tests)
… ureq with reqwest

- Split setup.rs into setup/validate.rs, setup/config.rs, setup/wizard.rs, setup/mod.rs
- Replace ureq HTTP client with reqwest::blocking::Client (removes ureq dependency)
- Add blocking feature to reqwest
- Add * to allowed token chars (fixes test)
- Wrap Commands enum in Cli struct with optional subcommand
- Bare 'openab' now defaults to Commands::Run { config: None }
- Preserves backward compat for Docker/Helm/systemd deployments
- Remove stray blank line in discord::Handler struct

Fixes CHANGES_REQUESTED from chaodu-agent and masami-agent
…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
- Use main's clap-based CLI structure (Commands::Setup)
- Keep PR's setup/ module refactor (better structure)
- Add libc dependency for process group kill
- Resolve version to 0.7.2
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
…sages

feat: add allow_bot_messages config (none/mentions/all)
openab-app bot and others added 15 commits April 14, 2026 07:20
- Keep PR's clap-based Commands (Setup/Run)
- Add allow_bot_messages and trusted_bot_ids from main
- Version bumped to 0.7.4
Add support for multi-agent collaboration via bot message handling:
- allowBotMessages: "off" | "mentions" | "all" (default: off)
- trustedBotIds: list of bot IDs allowed through (default: any)

Fixes: helm chart 0.7.x template ignored these fields when set
Add regexMatch validation for trustedBotIds (same as allowedChannels/allowedUsers)
to prevent float64 precision loss when bot IDs are passed via --set
instead of --set-string.

Also fix comments in values.yaml to use camelCase key names
(allowBotMessages/trustedBotIds) instead of TOML output names.
Reject invalid allowBotMessages values at template time (must be off,
mentions, or all). Add test script covering rendering, enum validation,
and snowflake ID mangling detection.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test 7 else-branch unconditionally called pass, masking failures.
Add ^[0-9]{17,20}$ regex check for trustedBotIds so non-numeric
values like "not-a-snowflake" are rejected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix double comment markers (##) that appeared in the commented-out
claude agent example block for allowBotMessages and trustedBotIds.
…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
fix: pin CLI versions in all Dockerfiles using ARG for reproducible builds
…onfig

feat(helm): add allowBotMessages and trustedBotIds discord config
@Joseph19820124 Joseph19820124 merged commit 71ff1ce into main Apr 14, 2026
1 check passed
@Joseph19820124 Joseph19820124 deleted the fix/memory-leak-process-group branch April 14, 2026 13:27
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.

4 participants