From 5bd83a1ac05b616361e1f8236a9d248b9dc1df83 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 00:19:38 +0000 Subject: [PATCH 1/9] feat(discord): add /auth slash command for agent device-flow auth Implements a new /auth Discord slash command that executes the OPENAB_AGENT_AUTH_COMMAND env var, captures the device flow URL and code from stdout/stderr, and relays them to the user as an ephemeral Discord message. Flow: 1. User runs /auth 2. OAB execs $OPENAB_AGENT_AUTH_COMMAND (e.g. 'codex login --device-auth') 3. Captures URL+code from stdout within 30s 4. Sends ephemeral followup with auth instructions 5. Waits up to 15min for process to exit (user authorizes in browser) 6. Reports success/failure/timeout Also updates docs/slash-commands.md with /auth documentation. --- crates/openab-core/src/discord.rs | 211 ++++++++++++++++++++++++++++++ docs/slash-commands.md | 26 ++++ 2 files changed, 237 insertions(+) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index da65f4711..5fc6c2048 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1171,6 +1171,8 @@ impl EventHandler for Handler { "delay", "Delay before firing (e.g. 30m, 2h, 1d)", ).required(true)), + CreateCommand::new("auth") + .description("Authenticate the backend agent (device flow)"), CreateCommand::new("export-thread") .description("Download this thread as a text file") .add_option(CreateCommandOption::new( @@ -1259,6 +1261,9 @@ impl EventHandler for Handler { Interaction::Command(cmd) if cmd.data.name == "export-thread" => { self.handle_export_thread_command(&ctx, &cmd).await; } + Interaction::Command(cmd) if cmd.data.name == "auth" => { + self.handle_auth_command(&ctx, &cmd).await; + } Interaction::Component(comp) if comp.data.custom_id.starts_with("acp_config_") => { self.handle_config_select(&ctx, &comp).await; } @@ -1660,6 +1665,212 @@ impl Handler { } } + async fn handle_auth_command( + &self, + ctx: &Context, + cmd: &serenity::model::application::CommandInteraction, + ) { + let auth_cmd = match std::env::var("OPENAB_AGENT_AUTH_COMMAND") { + Ok(val) if !val.is_empty() => val, + _ => { + let response = CreateInteractionResponse::Message( + CreateInteractionResponseMessage::new() + .content("⚠️ No auth command configured (`OPENAB_AGENT_AUTH_COMMAND` not set).") + .ephemeral(true), + ); + let _ = cmd.create_response(&ctx.http, response).await; + return; + } + }; + + // Acknowledge with a deferred ephemeral response so we have time to run the command. + let defer = CreateInteractionResponse::Defer( + CreateInteractionResponseMessage::new().ephemeral(true), + ); + if let Err(e) = cmd.create_response(&ctx.http, defer).await { + tracing::error!(error = %e, "failed to defer /auth response"); + return; + } + + let http = ctx.http.clone(); + let token = cmd.token.clone(); + + tokio::spawn(async move { + use tokio::io::AsyncBufReadExt; + use tokio::process::Command as TokioCommand; + + let child = TokioCommand::new("sh") + .arg("-c") + .arg(&auth_cmd) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn(); + + let mut child = match child { + Ok(c) => c, + Err(e) => { + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content(format!("❌ Failed to start auth command: {e}")) + .ephemeral(true), + Vec::new(), + ).await; + return; + } + }; + + // Read stdout and stderr concurrently to capture device flow output. + let stdout = child.stdout.take(); + let stderr = child.stderr.take(); + + let mut lines: Vec = Vec::new(); + + // Collect output from both streams until we find a URL or the process ends. + let collect = async { + let mut stdout_reader = stdout.map(|s| tokio::io::BufReader::new(s).lines()); + let mut stderr_reader = stderr.map(|s| tokio::io::BufReader::new(s).lines()); + + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); + + loop { + if tokio::time::Instant::now() > deadline { + break; + } + + tokio::select! { + line = async { + match stdout_reader.as_mut() { + Some(r) => r.next_line().await, + None => Ok(None), + } + } => { + match line { + Ok(Some(l)) => { + lines.push(l.clone()); + if l.contains("http://") || l.contains("https://") { + // Read a few more lines to capture the code + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + while let Some(reader) = stdout_reader.as_mut() { + match tokio::time::timeout( + std::time::Duration::from_millis(200), + reader.next_line() + ).await { + Ok(Ok(Some(l))) => lines.push(l), + _ => break, + } + } + break; + } + } + Ok(None) => { stdout_reader = None; } + Err(_) => { stdout_reader = None; } + } + } + line = async { + match stderr_reader.as_mut() { + Some(r) => r.next_line().await, + None => Ok(None), + } + } => { + match line { + Ok(Some(l)) => { + lines.push(l.clone()); + if l.contains("http://") || l.contains("https://") { + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + while let Some(reader) = stderr_reader.as_mut() { + match tokio::time::timeout( + std::time::Duration::from_millis(200), + reader.next_line() + ).await { + Ok(Ok(Some(l))) => lines.push(l), + _ => break, + } + } + break; + } + } + Ok(None) => { stderr_reader = None; } + Err(_) => { stderr_reader = None; } + } + } + } + + if stdout_reader.is_none() && stderr_reader.is_none() { + break; + } + } + }; + + collect.await; + + if lines.is_empty() { + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content("⚠️ Auth command produced no output.") + .ephemeral(true), + Vec::new(), + ).await; + return; + } + + // Send the captured output to the user + let output = lines.join("\n"); + let msg = format!("🔐 **Agent Authentication**\n```\n{}\n```\nFollow the instructions above. Waiting for authorization...", output); + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content(msg) + .ephemeral(true), + Vec::new(), + ).await; + + // Wait for the process to complete (user authorizes in browser) + let timeout = std::time::Duration::from_secs(15 * 60); + match tokio::time::timeout(timeout, child.wait()).await { + Ok(Ok(status)) if status.success() => { + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content("✅ Authentication successful!") + .ephemeral(true), + Vec::new(), + ).await; + } + Ok(Ok(status)) => { + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content(format!("❌ Authentication failed (exit code: {}).", status)) + .ephemeral(true), + Vec::new(), + ).await; + } + Ok(Err(e)) => { + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content(format!("❌ Auth process error: {e}")) + .ephemeral(true), + Vec::new(), + ).await; + } + Err(_) => { + // Timeout — kill the process + let _ = child.kill().await; + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content("⏰ Authentication timed out (15 min). Run `/auth` again to retry.") + .ephemeral(true), + Vec::new(), + ).await; + } + } + }); + } + async fn handle_export_thread_command( &self, ctx: &Context, diff --git a/docs/slash-commands.md b/docs/slash-commands.md index 0365846d0..093e63d4e 100644 --- a/docs/slash-commands.md +++ b/docs/slash-commands.md @@ -10,6 +10,7 @@ OpenAB registers Discord slash commands for session control. These work in both | `/agents` | Select the agent mode via dropdown menu | Yes | | `/cancel` | Cancel the current in-flight operation | Yes | | `/reset` | Reset the conversation session (clear history, start fresh) | Yes | +| `/auth` | Authenticate the backend agent via device flow | No | | `/remind` | Set a one-shot delayed reminder to mention users/roles | No | | `/export-thread` | Export thread/DM as `.txt` (default: last 100 messages) | No | @@ -150,3 +151,28 @@ Set a one-shot delayed reminder that mentions users or roles in the channel afte "Review PR #42" cc @Alice @Bob ``` + +## `/auth` + +Trigger the backend agent's device-flow authentication. OAB executes the command defined in `OPENAB_AGENT_AUTH_COMMAND`, captures the device code URL from stdout/stderr, and relays it to the user as an ephemeral Discord message. + +**Flow:** +1. User runs `/auth` +2. OAB executes `$OPENAB_AGENT_AUTH_COMMAND` (e.g. `codex login --device-auth`) +3. OAB captures the device URL + code from the command's output +4. OAB sends an ephemeral reply with the URL and code +5. User opens the URL in their browser, enters the code +6. The auth command exits successfully → OAB confirms "✅ Authentication successful!" + +**Requirements:** +- `OPENAB_AGENT_AUTH_COMMAND` environment variable must be set +- The auth command must use OAuth device flow (print URL + code to stdout, then block until authorized) +- No interactive stdin input required (headless-compatible) + +**Timeout:** 15 minutes. If the user doesn't authorize within that window, the process is killed and the user is prompted to run `/auth` again. + +**Error cases:** +- `OPENAB_AGENT_AUTH_COMMAND` not set → immediate error message +- Auth command fails to start → error message +- Auth command exits with non-zero → failure message with exit code +- Timeout → process killed, retry prompt From 8fb967757a92697995b2e656a755411a311e30de Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 00:23:53 +0000 Subject: [PATCH 2/9] =?UTF-8?q?fix:=20address=20review=20feedback=20from?= =?UTF-8?q?=20=E5=8F=A3=E6=B8=A1=E6=B3=95=E5=B8=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- crates/openab-core/src/discord.rs | 53 +++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index 5fc6c2048..3b51d8992 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1670,9 +1670,39 @@ impl Handler { ctx: &Context, cmd: &serenity::model::application::CommandInteraction, ) { + // Access control — only allowed users can trigger auth. + if is_denied_user( + false, + self.allow_all_users, + &self.allowed_users, + cmd.user.id.get(), + ) { + let response = CreateInteractionResponse::Message( + CreateInteractionResponseMessage::new() + .content("🚫 You are not allowed to use this bot.") + .ephemeral(true), + ); + let _ = cmd.create_response(&ctx.http, response).await; + return; + } + + // Single-flight guard — prevent concurrent /auth invocations. + static AUTH_IN_PROGRESS: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + if AUTH_IN_PROGRESS.swap(true, std::sync::atomic::Ordering::SeqCst) { + let response = CreateInteractionResponse::Message( + CreateInteractionResponseMessage::new() + .content("⚠️ Authentication already in progress. Please wait for it to complete.") + .ephemeral(true), + ); + let _ = cmd.create_response(&ctx.http, response).await; + return; + } + let auth_cmd = match std::env::var("OPENAB_AGENT_AUTH_COMMAND") { Ok(val) if !val.is_empty() => val, _ => { + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); let response = CreateInteractionResponse::Message( CreateInteractionResponseMessage::new() .content("⚠️ No auth command configured (`OPENAB_AGENT_AUTH_COMMAND` not set).") @@ -1688,6 +1718,7 @@ impl Handler { CreateInteractionResponseMessage::new().ephemeral(true), ); if let Err(e) = cmd.create_response(&ctx.http, defer).await { + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); tracing::error!(error = %e, "failed to defer /auth response"); return; } @@ -1716,6 +1747,7 @@ impl Handler { .ephemeral(true), Vec::new(), ).await; + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); return; } }; @@ -1727,7 +1759,7 @@ impl Handler { let mut lines: Vec = Vec::new(); // Collect output from both streams until we find a URL or the process ends. - let collect = async { + { let mut stdout_reader = stdout.map(|s| tokio::io::BufReader::new(s).lines()); let mut stderr_reader = stderr.map(|s| tokio::io::BufReader::new(s).lines()); @@ -1800,11 +1832,10 @@ impl Handler { break; } } - }; - - collect.await; + } // readers dropped here; child still owns the pipes if lines.is_empty() { + let _ = child.kill().await; let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() @@ -1812,12 +1843,21 @@ impl Handler { .ephemeral(true), Vec::new(), ).await; + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); return; } - // Send the captured output to the user + // Send the captured output to the user (truncate to fit Discord's 2000-char limit). let output = lines.join("\n"); - let msg = format!("🔐 **Agent Authentication**\n```\n{}\n```\nFollow the instructions above. Waiting for authorization...", output); + let prefix = "🔐 **Agent Authentication**\n```\n"; + let suffix = "\n```\nFollow the instructions above. Waiting for authorization..."; + let max_output = 2000 - prefix.len() - suffix.len(); + let truncated = if output.len() > max_output { + &output[..max_output] + } else { + &output + }; + let msg = format!("{prefix}{truncated}{suffix}"); let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() @@ -1868,6 +1908,7 @@ impl Handler { ).await; } } + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); }); } From add30dc9fd5dddd09c6bc21c5fca6c9d7aad93c9 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 00:24:48 +0000 Subject: [PATCH 3/9] refactor: spawn separate drain tasks for stdout/stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address 擺渡法師 and 口渡法師 review feedback: - Spawn independent tokio tasks for stdout/stderr draining (run to EOF) - Fixes SIGPIPE: pipes stay open while child waits for browser auth - Fixes cancellation safety: no more tokio::select! on next_line() - Fixes asymmetric drain: both streams captured independently - Use tokio::sync::Notify for URL detection signaling - tokio::join! drain tasks before exiting to ensure clean shutdown - Kill + join on empty-output and timeout paths --- crates/openab-core/src/discord.rs | 124 +++++++++++------------------- 1 file changed, 47 insertions(+), 77 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index 3b51d8992..c04dfba66 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1729,6 +1729,7 @@ impl Handler { tokio::spawn(async move { use tokio::io::AsyncBufReadExt; use tokio::process::Command as TokioCommand; + use std::sync::Arc; let child = TokioCommand::new("sh") .arg("-c") @@ -1752,90 +1753,57 @@ impl Handler { } }; - // Read stdout and stderr concurrently to capture device flow output. let stdout = child.stdout.take(); let stderr = child.stderr.take(); - let mut lines: Vec = Vec::new(); - - // Collect output from both streams until we find a URL or the process ends. - { - let mut stdout_reader = stdout.map(|s| tokio::io::BufReader::new(s).lines()); - let mut stderr_reader = stderr.map(|s| tokio::io::BufReader::new(s).lines()); - - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(30); - - loop { - if tokio::time::Instant::now() > deadline { - break; + let lines = Arc::new(std::sync::Mutex::new(Vec::::new())); + let url_found = Arc::new(tokio::sync::Notify::new()); + + // Spawn background drain tasks — they run to EOF, keeping pipes open. + let lines_out = lines.clone(); + let url_found_out = url_found.clone(); + let stdout_task = tokio::spawn(async move { + if let Some(stdout) = stdout { + let mut reader = tokio::io::BufReader::new(stdout).lines(); + while let Ok(Some(line)) = reader.next_line().await { + let has_url = line.contains("http://") || line.contains("https://"); + lines_out.lock().unwrap().push(line); + if has_url { + url_found_out.notify_one(); + } } + } + }); - tokio::select! { - line = async { - match stdout_reader.as_mut() { - Some(r) => r.next_line().await, - None => Ok(None), - } - } => { - match line { - Ok(Some(l)) => { - lines.push(l.clone()); - if l.contains("http://") || l.contains("https://") { - // Read a few more lines to capture the code - tokio::time::sleep(std::time::Duration::from_millis(500)).await; - while let Some(reader) = stdout_reader.as_mut() { - match tokio::time::timeout( - std::time::Duration::from_millis(200), - reader.next_line() - ).await { - Ok(Ok(Some(l))) => lines.push(l), - _ => break, - } - } - break; - } - } - Ok(None) => { stdout_reader = None; } - Err(_) => { stdout_reader = None; } - } - } - line = async { - match stderr_reader.as_mut() { - Some(r) => r.next_line().await, - None => Ok(None), - } - } => { - match line { - Ok(Some(l)) => { - lines.push(l.clone()); - if l.contains("http://") || l.contains("https://") { - tokio::time::sleep(std::time::Duration::from_millis(500)).await; - while let Some(reader) = stderr_reader.as_mut() { - match tokio::time::timeout( - std::time::Duration::from_millis(200), - reader.next_line() - ).await { - Ok(Ok(Some(l))) => lines.push(l), - _ => break, - } - } - break; - } - } - Ok(None) => { stderr_reader = None; } - Err(_) => { stderr_reader = None; } - } + let lines_err = lines.clone(); + let url_found_err = url_found.clone(); + let stderr_task = tokio::spawn(async move { + if let Some(stderr) = stderr { + let mut reader = tokio::io::BufReader::new(stderr).lines(); + while let Ok(Some(line)) = reader.next_line().await { + let has_url = line.contains("http://") || line.contains("https://"); + lines_err.lock().unwrap().push(line); + if has_url { + url_found_err.notify_one(); } } + } + }); - if stdout_reader.is_none() && stderr_reader.is_none() { - break; - } + // Wait for a URL to appear or 30-second timeout. + tokio::select! { + _ = url_found.notified() => { + // Brief sleep to let trailing lines (code/instructions) be captured. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; } - } // readers dropped here; child still owns the pipes + _ = tokio::time::sleep(std::time::Duration::from_secs(30)) => {} + } - if lines.is_empty() { + let collected_lines = lines.lock().unwrap().clone(); + + if collected_lines.is_empty() { let _ = child.kill().await; + let _ = tokio::join!(stdout_task, stderr_task); let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() @@ -1847,8 +1815,8 @@ impl Handler { return; } - // Send the captured output to the user (truncate to fit Discord's 2000-char limit). - let output = lines.join("\n"); + // Send the captured output (truncated to Discord's 2000-char limit). + let output = collected_lines.join("\n"); let prefix = "🔐 **Agent Authentication**\n```\n"; let suffix = "\n```\nFollow the instructions above. Waiting for authorization..."; let max_output = 2000 - prefix.len() - suffix.len(); @@ -1866,7 +1834,7 @@ impl Handler { Vec::new(), ).await; - // Wait for the process to complete (user authorizes in browser) + // Wait for the process to complete (user authorizes in browser). let timeout = std::time::Duration::from_secs(15 * 60); match tokio::time::timeout(timeout, child.wait()).await { Ok(Ok(status)) if status.success() => { @@ -1897,7 +1865,6 @@ impl Handler { ).await; } Err(_) => { - // Timeout — kill the process let _ = child.kill().await; let _ = http.create_followup_message( &token, @@ -1908,6 +1875,9 @@ impl Handler { ).await; } } + + // Let background drain tasks complete. + let _ = tokio::join!(stdout_task, stderr_task); AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); }); } From 12aa01732893bfe08d45cfdc82308e2936a04efd Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 00:25:35 +0000 Subject: [PATCH 4/9] fix: DM-only restriction + UTF-8-safe truncation - /auth now only works in DMs (rejects guild channels with guidance) - Truncation uses chars().take() instead of byte slicing (no panic on multi-byte UTF-8) - Budget calculated with chars().count() for prefix/suffix (correct for Discord's character-based limit) - Updated docs to document DM-only requirement --- crates/openab-core/src/discord.rs | 23 +++++++++++++++++------ docs/slash-commands.md | 1 + 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index c04dfba66..48be46757 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1686,6 +1686,21 @@ impl Handler { return; } + // DM-only — auth codes are sensitive; reject if not in a DM channel. + let is_dm = matches!( + cmd.channel_id.to_channel(&ctx.http).await, + Ok(serenity::model::channel::Channel::Private(_)) + ); + if !is_dm { + let response = CreateInteractionResponse::Message( + CreateInteractionResponseMessage::new() + .content("🔒 `/auth` is only available in DMs for security. Please DM me and run `/auth` there.") + .ephemeral(true), + ); + let _ = cmd.create_response(&ctx.http, response).await; + return; + } + // Single-flight guard — prevent concurrent /auth invocations. static AUTH_IN_PROGRESS: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); @@ -1819,12 +1834,8 @@ impl Handler { let output = collected_lines.join("\n"); let prefix = "🔐 **Agent Authentication**\n```\n"; let suffix = "\n```\nFollow the instructions above. Waiting for authorization..."; - let max_output = 2000 - prefix.len() - suffix.len(); - let truncated = if output.len() > max_output { - &output[..max_output] - } else { - &output - }; + let max_output_chars = 2000 - prefix.chars().count() - suffix.chars().count(); + let truncated: String = output.chars().take(max_output_chars).collect(); let msg = format!("{prefix}{truncated}{suffix}"); let _ = http.create_followup_message( &token, diff --git a/docs/slash-commands.md b/docs/slash-commands.md index 093e63d4e..7a0c9db43 100644 --- a/docs/slash-commands.md +++ b/docs/slash-commands.md @@ -168,6 +168,7 @@ Trigger the backend agent's device-flow authentication. OAB executes the command - `OPENAB_AGENT_AUTH_COMMAND` environment variable must be set - The auth command must use OAuth device flow (print URL + code to stdout, then block until authorized) - No interactive stdin input required (headless-compatible) +- Must be invoked in a **DM** with the bot (rejected in guild channels/threads for security) **Timeout:** 15 minutes. If the user doesn't authorize within that window, the process is killed and the user is prompted to run `/auth` again. From aad13e6631283233f38fb921db790b4083667b91 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 00:56:41 +0000 Subject: [PATCH 5/9] fix: reduce auth timeout to 14min for interaction token headroom - child.wait() timeout reduced from 15min to 14min so the final success/failure followup can still be delivered within Discord's 15min interaction token TTL - Docs: added 30s URL-collection window, allowed_users requirement, and single-flight behavior notes --- crates/openab-core/src/discord.rs | 3 ++- docs/slash-commands.md | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index 48be46757..fc46a9bda 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1846,7 +1846,8 @@ impl Handler { ).await; // Wait for the process to complete (user authorizes in browser). - let timeout = std::time::Duration::from_secs(15 * 60); + // Use 14min (not 15) to leave headroom for the Discord interaction token TTL. + let timeout = std::time::Duration::from_secs(14 * 60); match tokio::time::timeout(timeout, child.wait()).await { Ok(Ok(status)) if status.success() => { let _ = http.create_followup_message( diff --git a/docs/slash-commands.md b/docs/slash-commands.md index 7a0c9db43..b8a0fb159 100644 --- a/docs/slash-commands.md +++ b/docs/slash-commands.md @@ -170,7 +170,12 @@ Trigger the backend agent's device-flow authentication. OAB executes the command - No interactive stdin input required (headless-compatible) - Must be invoked in a **DM** with the bot (rejected in guild channels/threads for security) -**Timeout:** 15 minutes. If the user doesn't authorize within that window, the process is killed and the user is prompted to run `/auth` again. +**Timeout:** 14 minutes. If the user doesn't authorize within that window, the process is killed and the user is prompted to run `/auth` again. (Reduced from 15min to leave headroom for Discord's interaction token TTL.) + +**Behavior notes:** +- Only users in the `allowed_users` list can invoke `/auth` +- A 30-second URL-collection window waits for the auth command to print its URL. Slow-starting CLIs that take longer may show "no output". +- Only one `/auth` flow can run at a time (single-flight). A second concurrent invocation is rejected with "already in progress". **Error cases:** - `OPENAB_AGENT_AUTH_COMMAND` not set → immediate error message From 39928895196a9cae9ffbacad29149a02be8d5369 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 00:58:39 +0000 Subject: [PATCH 6/9] =?UTF-8?q?fix:=20address=20=E8=A6=BA=E6=B8=A1?= =?UTF-8?q?=E6=B3=95=E5=B8=AB=20review=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace to_channel() API call with cmd.guild_id.is_some() for DM check (F1) - Add AuthGuard Drop impl to reset AUTH_IN_PROGRESS on task panic (F2) - Add tracing::info/warn at key lifecycle points in spawned task (F3) - Remove redundant manual store(false) calls now covered by Drop guard --- crates/openab-core/src/discord.rs | 32 +++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index fc46a9bda..39dbfbc6d 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1687,11 +1687,7 @@ impl Handler { } // DM-only — auth codes are sensitive; reject if not in a DM channel. - let is_dm = matches!( - cmd.channel_id.to_channel(&ctx.http).await, - Ok(serenity::model::channel::Channel::Private(_)) - ); - if !is_dm { + if cmd.guild_id.is_some() { let response = CreateInteractionResponse::Message( CreateInteractionResponseMessage::new() .content("🔒 `/auth` is only available in DMs for security. Please DM me and run `/auth` there.") @@ -1746,6 +1742,17 @@ impl Handler { use tokio::process::Command as TokioCommand; use std::sync::Arc; + // Drop guard ensures AUTH_IN_PROGRESS is cleared even on panic. + struct AuthGuard; + impl Drop for AuthGuard { + fn drop(&mut self) { + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); + } + } + let _guard = AuthGuard; + + info!("/auth: starting auth command"); + let child = TokioCommand::new("sh") .arg("-c") .arg(&auth_cmd) @@ -1756,6 +1763,7 @@ impl Handler { let mut child = match child { Ok(c) => c, Err(e) => { + tracing::error!(error = %e, "/auth: failed to spawn auth command"); let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() @@ -1763,7 +1771,6 @@ impl Handler { .ephemeral(true), Vec::new(), ).await; - AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); return; } }; @@ -1808,15 +1815,19 @@ impl Handler { // Wait for a URL to appear or 30-second timeout. tokio::select! { _ = url_found.notified() => { + info!("/auth: URL detected in output"); // Brief sleep to let trailing lines (code/instructions) be captured. tokio::time::sleep(std::time::Duration::from_millis(500)).await; } - _ = tokio::time::sleep(std::time::Duration::from_secs(30)) => {} + _ = tokio::time::sleep(std::time::Duration::from_secs(30)) => { + warn!("/auth: 30s URL-collection window expired without detecting URL"); + } } let collected_lines = lines.lock().unwrap().clone(); if collected_lines.is_empty() { + warn!("/auth: no output captured, killing child process"); let _ = child.kill().await; let _ = tokio::join!(stdout_task, stderr_task); let _ = http.create_followup_message( @@ -1826,7 +1837,6 @@ impl Handler { .ephemeral(true), Vec::new(), ).await; - AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); return; } @@ -1850,6 +1860,7 @@ impl Handler { let timeout = std::time::Duration::from_secs(14 * 60); match tokio::time::timeout(timeout, child.wait()).await { Ok(Ok(status)) if status.success() => { + info!("/auth: authentication successful"); let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() @@ -1859,6 +1870,7 @@ impl Handler { ).await; } Ok(Ok(status)) => { + warn!(%status, "/auth: authentication failed"); let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() @@ -1877,11 +1889,12 @@ impl Handler { ).await; } Err(_) => { + warn!("/auth: timed out waiting for authorization"); let _ = child.kill().await; let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() - .content("⏰ Authentication timed out (15 min). Run `/auth` again to retry.") + .content("⏰ Authentication timed out. Run `/auth` again to retry.") .ephemeral(true), Vec::new(), ).await; @@ -1890,7 +1903,6 @@ impl Handler { // Let background drain tasks complete. let _ = tokio::join!(stdout_task, stderr_task); - AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); }); } From 08791dd5b65ad042b5a4fdd9599811e16cfd9212 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 01:23:19 +0000 Subject: [PATCH 7/9] fix(discord): address group-review feedback on /auth - 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) --- crates/openab-core/src/discord.rs | 82 ++++++++++++++++++++++++++++--- docs/slash-commands.md | 4 +- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index 39dbfbc6d..1eacfb4f3 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1670,6 +1670,17 @@ impl Handler { ctx: &Context, cmd: &serenity::model::application::CommandInteraction, ) { + // Reject bot users — consistent with other slash-command handlers (e.g. /remind). + if cmd.user.bot { + let response = CreateInteractionResponse::Message( + CreateInteractionResponseMessage::new() + .content("🤖 Bots cannot use `/auth`.") + .ephemeral(true), + ); + let _ = cmd.create_response(&ctx.http, response).await; + return; + } + // Access control — only allowed users can trigger auth. if is_denied_user( false, @@ -1736,6 +1747,7 @@ impl Handler { let http = ctx.http.clone(); let token = cmd.token.clone(); + let user_id = cmd.user.id.get(); tokio::spawn(async move { use tokio::io::AsyncBufReadExt; @@ -1751,7 +1763,7 @@ impl Handler { } let _guard = AuthGuard; - info!("/auth: starting auth command"); + info!(user_id, "/auth: starting auth command"); let child = TokioCommand::new("sh") .arg("-c") @@ -1789,7 +1801,7 @@ impl Handler { let mut reader = tokio::io::BufReader::new(stdout).lines(); while let Ok(Some(line)) = reader.next_line().await { let has_url = line.contains("http://") || line.contains("https://"); - lines_out.lock().unwrap().push(line); + lines_out.lock().unwrap_or_else(|e| e.into_inner()).push(line); if has_url { url_found_out.notify_one(); } @@ -1804,7 +1816,7 @@ impl Handler { let mut reader = tokio::io::BufReader::new(stderr).lines(); while let Ok(Some(line)) = reader.next_line().await { let has_url = line.contains("http://") || line.contains("https://"); - lines_err.lock().unwrap().push(line); + lines_err.lock().unwrap_or_else(|e| e.into_inner()).push(line); if has_url { url_found_err.notify_one(); } @@ -1812,19 +1824,60 @@ impl Handler { } }); - // Wait for a URL to appear or 30-second timeout. + // Wait for a URL to appear, the command to exit early, or a 30s timeout. + let mut early_exit: Option> = None; tokio::select! { _ = url_found.notified() => { info!("/auth: URL detected in output"); // Brief sleep to let trailing lines (code/instructions) be captured. tokio::time::sleep(std::time::Duration::from_millis(500)).await; } + res = child.wait() => { + // The auth command exited before printing a URL — fail fast + // instead of waiting out the full collection window. + warn!("/auth: auth command exited before a URL was detected"); + early_exit = Some(res); + } _ = tokio::time::sleep(std::time::Duration::from_secs(30)) => { warn!("/auth: 30s URL-collection window expired without detecting URL"); } } - let collected_lines = lines.lock().unwrap().clone(); + // Handle an early exit (the command terminated during the URL window). + if let Some(res) = early_exit { + let _ = tokio::join!(stdout_task, stderr_task); + let collected = lines + .lock() + .unwrap_or_else(|e| e.into_inner()) + .join("\n"); + let detail = if collected.trim().is_empty() { + String::new() + } else { + let snippet: String = collected.chars().take(500).collect(); + format!("\n```\n{snippet}\n```") + }; + let content = match res { + Ok(status) if status.success() => { + format!("✅ Auth command completed.{detail}") + } + Ok(status) => { + format!( + "❌ Auth command exited early ({status}) before producing a login URL.{detail}" + ) + } + Err(e) => format!("❌ Error waiting for auth command: {e}"), + }; + let _ = http.create_followup_message( + &token, + &CreateInteractionResponseFollowup::new() + .content(content) + .ephemeral(true), + Vec::new(), + ).await; + return; + } + + let collected_lines = lines.lock().unwrap_or_else(|e| e.into_inner()).clone(); if collected_lines.is_empty() { warn!("/auth: no output captured, killing child process"); @@ -1833,7 +1886,7 @@ impl Handler { let _ = http.create_followup_message( &token, &CreateInteractionResponseFollowup::new() - .content("⚠️ Auth command produced no output.") + .content("⚠️ Auth command produced no output within 30 seconds. Verify `OPENAB_AGENT_AUTH_COMMAND` is set and prints a login URL to stdout/stderr.") .ephemeral(true), Vec::new(), ).await; @@ -1844,8 +1897,21 @@ impl Handler { let output = collected_lines.join("\n"); let prefix = "🔐 **Agent Authentication**\n```\n"; let suffix = "\n```\nFollow the instructions above. Waiting for authorization..."; - let max_output_chars = 2000 - prefix.chars().count() - suffix.chars().count(); - let truncated: String = output.chars().take(max_output_chars).collect(); + // Discord enforces the 2000-char limit in UTF-16 code units, so budget + // and truncate by UTF-16 units rather than Unicode scalar values. + let budget = 2000usize + .saturating_sub(prefix.encode_utf16().count()) + .saturating_sub(suffix.encode_utf16().count()); + let mut truncated = String::new(); + let mut used = 0usize; + for ch in output.chars() { + let w = ch.len_utf16(); + if used + w > budget { + break; + } + used += w; + truncated.push(ch); + } let msg = format!("{prefix}{truncated}{suffix}"); let _ = http.create_followup_message( &token, diff --git a/docs/slash-commands.md b/docs/slash-commands.md index b8a0fb159..528ee4180 100644 --- a/docs/slash-commands.md +++ b/docs/slash-commands.md @@ -1,6 +1,6 @@ # Slash Commands -OpenAB registers Discord slash commands for session control. These work in both guild threads and DMs. +OpenAB registers Discord slash commands for session control and agent management. Most work in both guild threads and DMs — the exception is `/auth`, which is **DM-only** for security (see [`/auth`](#auth) below). ## Commands @@ -10,7 +10,7 @@ OpenAB registers Discord slash commands for session control. These work in both | `/agents` | Select the agent mode via dropdown menu | Yes | | `/cancel` | Cancel the current in-flight operation | Yes | | `/reset` | Reset the conversation session (clear history, start fresh) | Yes | -| `/auth` | Authenticate the backend agent via device flow | No | +| `/auth` | Authenticate the backend agent via device flow (**DM-only**) | No | | `/remind` | Set a one-shot delayed reminder to mention users/roles | No | | `/export-thread` | Export thread/DM as `.txt` (default: last 100 messages) | No | From 5df6b233b1f6bbf8ace9251f56fd59bdb7b9c1b0 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 01:34:35 +0000 Subject: [PATCH 8/9] refactor(discord): use Acquire/Release ordering for /auth single-flight flag Addresses review nit: the AUTH_IN_PROGRESS flag carries no dependent data, so SeqCst is stronger than necessary. Acquire on the guard acquire (swap) and Release on each clear (store) is sufficient and clearer in intent. --- crates/openab-core/src/discord.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index 1eacfb4f3..aebd0b82e 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1711,7 +1711,7 @@ impl Handler { // Single-flight guard — prevent concurrent /auth invocations. static AUTH_IN_PROGRESS: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); - if AUTH_IN_PROGRESS.swap(true, std::sync::atomic::Ordering::SeqCst) { + if AUTH_IN_PROGRESS.swap(true, std::sync::atomic::Ordering::Acquire) { let response = CreateInteractionResponse::Message( CreateInteractionResponseMessage::new() .content("⚠️ Authentication already in progress. Please wait for it to complete.") @@ -1724,7 +1724,7 @@ impl Handler { let auth_cmd = match std::env::var("OPENAB_AGENT_AUTH_COMMAND") { Ok(val) if !val.is_empty() => val, _ => { - AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::Release); let response = CreateInteractionResponse::Message( CreateInteractionResponseMessage::new() .content("⚠️ No auth command configured (`OPENAB_AGENT_AUTH_COMMAND` not set).") @@ -1740,7 +1740,7 @@ impl Handler { CreateInteractionResponseMessage::new().ephemeral(true), ); if let Err(e) = cmd.create_response(&ctx.http, defer).await { - AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::Release); tracing::error!(error = %e, "failed to defer /auth response"); return; } @@ -1758,7 +1758,7 @@ impl Handler { struct AuthGuard; impl Drop for AuthGuard { fn drop(&mut self) { - AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::SeqCst); + AUTH_IN_PROGRESS.store(false, std::sync::atomic::Ordering::Release); } } let _guard = AuthGuard; From 99987e166d5656b796a491270b613fe0b96b9ee6 Mon Sep 17 00:00:00 2001 From: chaodu-agent Date: Wed, 24 Jun 2026 02:05:23 +0000 Subject: [PATCH 9/9] fix(discord): correct misleading /auth early-exit message + doc gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review feedback: - F1: when the auth command exits 0 during the URL-collection window without producing a login URL, show a warning + retry prompt instead of '✅ Auth command completed.' (avoids false confidence that auth succeeded) - F2: document the early-exit-without-URL error case in slash-commands.md - F3: document bot-user rejection (and DM-only) in the /auth error cases --- crates/openab-core/src/discord.rs | 4 +++- docs/slash-commands.md | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/openab-core/src/discord.rs b/crates/openab-core/src/discord.rs index aebd0b82e..c697c76fd 100644 --- a/crates/openab-core/src/discord.rs +++ b/crates/openab-core/src/discord.rs @@ -1858,7 +1858,9 @@ impl Handler { }; let content = match res { Ok(status) if status.success() => { - format!("✅ Auth command completed.{detail}") + format!( + "⚠️ Auth command exited (status 0) before a login URL was detected. Run `/auth` again to retry.{detail}" + ) } Ok(status) => { format!( diff --git a/docs/slash-commands.md b/docs/slash-commands.md index 528ee4180..9638a9792 100644 --- a/docs/slash-commands.md +++ b/docs/slash-commands.md @@ -174,11 +174,15 @@ Trigger the backend agent's device-flow authentication. OAB executes the command **Behavior notes:** - Only users in the `allowed_users` list can invoke `/auth` +- Bot users are rejected — `/auth` is for human operators only - A 30-second URL-collection window waits for the auth command to print its URL. Slow-starting CLIs that take longer may show "no output". - Only one `/auth` flow can run at a time (single-flight). A second concurrent invocation is rejected with "already in progress". **Error cases:** - `OPENAB_AGENT_AUTH_COMMAND` not set → immediate error message +- Invoked by a bot user → rejected +- Invoked outside a DM (in a guild channel/thread) → rejected for security - Auth command fails to start → error message +- Auth command exits **before** printing a login URL (within the 30s window) → warning that no URL was produced, with a retry prompt - Auth command exits with non-zero → failure message with exit code - Timeout → process killed, retry prompt