mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(auth): avoid false success and block chat during pending auth (#1111)
* fix(auth): avoid false success and block chat while auth pending * fix(web): clear stale auth UI on failure and add setup regression test * Update src/agent/thread_ops.rs Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix(fmt): place auth activation comment on separate line --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Illia Polosukhin <[email protected]>
This commit is contained in:
co-authored by
gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Illia Polosukhin
parent
e74214dce8
commit
e0f393bf04
+24
-1
@@ -1540,7 +1540,8 @@ impl Agent {
|
||||
.configure_token(&pending.extension_name, token)
|
||||
.await
|
||||
{
|
||||
Ok(result) => {
|
||||
Ok(result) if result.activated => {
|
||||
// Ensure extension is actually activated
|
||||
tracing::info!(
|
||||
"Extension '{}' configured via auth mode: {}",
|
||||
pending.extension_name,
|
||||
@@ -1560,6 +1561,28 @@ impl Agent {
|
||||
.await;
|
||||
Ok(Some(result.message))
|
||||
}
|
||||
Ok(result) => {
|
||||
{
|
||||
let mut sess = session.lock().await;
|
||||
if let Some(thread) = sess.threads.get_mut(&thread_id) {
|
||||
thread.enter_auth_mode(pending.extension_name.clone());
|
||||
}
|
||||
}
|
||||
let _ = self
|
||||
.channels
|
||||
.send_status(
|
||||
&message.channel,
|
||||
StatusUpdate::AuthRequired {
|
||||
extension_name: pending.extension_name.clone(),
|
||||
instructions: Some(result.message.clone()),
|
||||
auth_url: None,
|
||||
setup_url: None,
|
||||
},
|
||||
&message.metadata,
|
||||
)
|
||||
.await;
|
||||
Ok(Some(result.message))
|
||||
}
|
||||
Err(e) => {
|
||||
let msg = e.to_string();
|
||||
// Token validation errors: re-enter auth mode and re-prompt
|
||||
|
||||
@@ -1163,7 +1163,7 @@ async fn chat_auth_token_handler(
|
||||
.configure_token(&req.extension_name, &req.token)
|
||||
.await
|
||||
{
|
||||
Ok(result) => {
|
||||
Ok(result) if result.activated => {
|
||||
// Clear auth mode on the active thread
|
||||
clear_auth_mode(&state).await;
|
||||
|
||||
@@ -1175,6 +1175,7 @@ async fn chat_auth_token_handler(
|
||||
|
||||
Ok(Json(ActionResponse::ok(result.message)))
|
||||
}
|
||||
Ok(result) => Ok(Json(ActionResponse::fail(result.message))),
|
||||
Err(e) => {
|
||||
let msg = e.to_string();
|
||||
// Re-emit auth_required for retry on validation errors
|
||||
@@ -2204,14 +2205,18 @@ async fn extensions_setup_submit_handler(
|
||||
|
||||
match ext_mgr.configure(&name, &req.secrets).await {
|
||||
Ok(result) => {
|
||||
// Broadcast auth_completed so the chat UI can dismiss any in-progress
|
||||
// auth card or setup modal that was triggered by tool_auth/tool_activate.
|
||||
// Broadcast completion status so chat UI can dismiss success cases while
|
||||
// leaving failed auth/configuration flows visible for correction.
|
||||
state.sse.broadcast(SseEvent::AuthCompleted {
|
||||
extension_name: name.clone(),
|
||||
success: true,
|
||||
success: result.activated,
|
||||
message: result.message.clone(),
|
||||
});
|
||||
let mut resp = ActionResponse::ok(result.message);
|
||||
let mut resp = if result.activated {
|
||||
ActionResponse::ok(result.message)
|
||||
} else {
|
||||
ActionResponse::fail(result.message)
|
||||
};
|
||||
resp.activated = Some(result.activated);
|
||||
resp.auth_url = result.auth_url;
|
||||
Ok(Json(resp))
|
||||
@@ -2856,6 +2861,80 @@ mod tests {
|
||||
.with_state(state)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_extensions_setup_submit_returns_failure_when_not_activated() {
|
||||
use axum::body::Body;
|
||||
use tower::ServiceExt;
|
||||
|
||||
let secrets = test_secrets_store();
|
||||
let (ext_mgr, _wasm_tools_dir, wasm_channels_dir) = test_ext_mgr(secrets);
|
||||
|
||||
let channel_name = "test-failing-channel";
|
||||
std::fs::write(
|
||||
wasm_channels_dir
|
||||
.path()
|
||||
.join(format!("{channel_name}.wasm")),
|
||||
b"\0asm fake",
|
||||
)
|
||||
.expect("write fake wasm");
|
||||
let caps = serde_json::json!({
|
||||
"type": "channel",
|
||||
"name": channel_name,
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{"name": "BOT_TOKEN", "prompt": "Enter bot token"}
|
||||
]
|
||||
}
|
||||
});
|
||||
std::fs::write(
|
||||
wasm_channels_dir
|
||||
.path()
|
||||
.join(format!("{channel_name}.capabilities.json")),
|
||||
serde_json::to_string(&caps).expect("serialize caps"),
|
||||
)
|
||||
.expect("write capabilities");
|
||||
|
||||
let state = test_gateway_state(Some(ext_mgr));
|
||||
let app = Router::new()
|
||||
.route(
|
||||
"/api/extensions/{name}/setup",
|
||||
post(extensions_setup_submit_handler),
|
||||
)
|
||||
.with_state(state);
|
||||
|
||||
let req_body = serde_json::json!({
|
||||
"secrets": {
|
||||
"BOT_TOKEN": "dummy-token"
|
||||
}
|
||||
});
|
||||
let req = axum::http::Request::builder()
|
||||
.method("POST")
|
||||
.uri(format!("/api/extensions/{channel_name}/setup"))
|
||||
.header("content-type", "application/json")
|
||||
.body(Body::from(req_body.to_string()))
|
||||
.expect("request");
|
||||
|
||||
let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
|
||||
.await
|
||||
.expect("response");
|
||||
assert_eq!(resp.status(), StatusCode::OK);
|
||||
|
||||
let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
|
||||
.await
|
||||
.expect("body");
|
||||
let parsed: serde_json::Value = serde_json::from_slice(&body).expect("json response");
|
||||
assert_eq!(parsed["success"], serde_json::Value::Bool(false));
|
||||
assert_eq!(parsed["activated"], serde_json::Value::Bool(false));
|
||||
assert!(
|
||||
parsed["message"]
|
||||
.as_str()
|
||||
.unwrap_or_default()
|
||||
.contains("Activation failed"),
|
||||
"expected activation failure in message: {:?}",
|
||||
parsed
|
||||
);
|
||||
}
|
||||
|
||||
fn expired_flow_created_at() -> Option<std::time::Instant> {
|
||||
std::time::Instant::now()
|
||||
.checked_sub(oauth_defaults::OAUTH_FLOW_EXPIRY + std::time::Duration::from_secs(1))
|
||||
|
||||
@@ -19,6 +19,7 @@ let _loadThreadsTimer = null;
|
||||
const JOB_EVENTS_CAP = 500;
|
||||
const MEMORY_SEARCH_QUERY_MAX_LENGTH = 100;
|
||||
let stagedImages = [];
|
||||
let authFlowPending = false;
|
||||
let _ghostSuggestion = '';
|
||||
|
||||
// --- Slash Commands ---
|
||||
@@ -487,6 +488,12 @@ function clearSuggestionChips() {
|
||||
function sendMessage() {
|
||||
clearSuggestionChips();
|
||||
const input = document.getElementById('chat-input');
|
||||
if (authFlowPending) {
|
||||
showToast('Complete the auth step before sending chat messages.', 'info');
|
||||
const tokenField = document.querySelector('.auth-card .auth-token-input input');
|
||||
if (tokenField) tokenField.focus();
|
||||
return;
|
||||
}
|
||||
if (!currentThreadId) {
|
||||
console.warn('sendMessage: no thread selected, ignoring');
|
||||
return;
|
||||
@@ -515,7 +522,7 @@ function sendMessage() {
|
||||
}
|
||||
|
||||
function enableChatInput() {
|
||||
if (currentThreadIsReadOnly) return;
|
||||
if (currentThreadIsReadOnly || authFlowPending) return;
|
||||
const input = document.getElementById('chat-input');
|
||||
const btn = document.getElementById('send-btn');
|
||||
if (input) {
|
||||
@@ -1198,6 +1205,7 @@ function showJobCard(data) {
|
||||
// --- Auth card ---
|
||||
|
||||
function handleAuthRequired(data) {
|
||||
setAuthFlowPending(true, data.instructions);
|
||||
if (data.auth_url) {
|
||||
// OAuth flow: show the global auth prompt with an OAuth button + optional token paste field.
|
||||
showAuthCard(data);
|
||||
@@ -1209,10 +1217,17 @@ function handleAuthRequired(data) {
|
||||
}
|
||||
|
||||
function handleAuthCompleted(data) {
|
||||
// Dismiss only the matching extension's UI so unrelated setup work is not interrupted.
|
||||
showToast(data.message, data.success ? 'success' : 'error');
|
||||
// Dismiss only the matching extension's UI so stale prompts are cleared.
|
||||
removeAuthCard(data.extension_name);
|
||||
closeConfigureModal(data.extension_name);
|
||||
showToast(data.message, data.success ? 'success' : 'error');
|
||||
if (!data.success) {
|
||||
setAuthFlowPending(false);
|
||||
if (currentTab === 'extensions') loadExtensions();
|
||||
enableChatInput();
|
||||
return;
|
||||
}
|
||||
setAuthFlowPending(false);
|
||||
if (shouldShowChannelConnectedMessage(data.extension_name, data.success)) {
|
||||
addMessage('system', 'Telegram is now connected. You can message me there and I can send you notifications.');
|
||||
}
|
||||
@@ -1392,6 +1407,7 @@ function cancelAuth(extensionName) {
|
||||
body: { extension_name: extensionName },
|
||||
}).catch(() => {});
|
||||
removeAuthCard(extensionName);
|
||||
setAuthFlowPending(false);
|
||||
enableChatInput();
|
||||
}
|
||||
|
||||
@@ -1409,6 +1425,24 @@ function showAuthCardError(extensionName, message) {
|
||||
}
|
||||
}
|
||||
|
||||
function setAuthFlowPending(pending, instructions) {
|
||||
authFlowPending = !!pending;
|
||||
const input = document.getElementById('chat-input');
|
||||
const btn = document.getElementById('send-btn');
|
||||
if (!input || !btn) return;
|
||||
if (authFlowPending) {
|
||||
input.disabled = true;
|
||||
btn.disabled = true;
|
||||
input.placeholder = instructions || 'Complete extension auth to continue chatting';
|
||||
return;
|
||||
}
|
||||
if (!currentThreadIsReadOnly) {
|
||||
input.disabled = false;
|
||||
btn.disabled = false;
|
||||
input.placeholder = I18n.t('chat.inputPlaceholder');
|
||||
}
|
||||
}
|
||||
|
||||
function loadHistory(before) {
|
||||
clearSuggestionChips();
|
||||
let historyUrl = '/api/chat/history?limit=50';
|
||||
|
||||
Reference in New Issue
Block a user