From 04146bf0bfb4f7436f58a8c17b13cf9c1fddeb8a Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Fri, 3 Apr 2026 03:55:31 +0530 Subject: [PATCH] Fix/191 skills reliability upstream (#282) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(skills): core RPC data stats, tool timeout env, ping state merge (#191) - Add openhuman.skills_data_stats and SkillDataDirectoryStats (disk usage). - Centralize tool execution timeout via OPENHUMAN_TOOL_TIMEOUT_SECS (tool_timeout). - Apply timeout to skill event loop, agent tool loop, harness default, delegate. - Ping scheduler: merge connection_error into published_state via registry. - JSON-RPC e2e: assert skills_data_stats. Closes #214 Closes #218 Part of #213 (backend) Made-with: Cursor * feat(app): skills sync stats UI, reconnect resync, FE timeouts, chat errors (#191) - useSkillDataDirectoryStats + Skills.tsx merge disk stats with skill state. - resyncRunningSkillsAfterReconnect on socket connect; disconnectSkill OAuth cleanup in finally. - withTimeout for callTool/triggerSync; VITE_TOOL_TIMEOUT_SECS in app .env.example. - Structured ChatSendError in Conversations (data-chat-send-error-code). Closes #213 Closes #215 Closes #216 Closes #217 Closes #219 Made-with: Cursor * test(e2e): onboarding helpers and skills smoke specs (#191) - shared-flows: 5-step onboarding, completeOnboardingIfVisible. - auth-access-control + voice-mode use shared helpers. - skills-registry: named skill assertion; new skill-oauth, multi-round, reconnect, lifecycle specs. Closes #220 Closes #221 Closes #222 Closes #223 Closes #224 Part of #189 (E2E helpers) Made-with: Cursor * style: rustfmt + Prettier for CI (PR #282) Fix Type Check workflow: prettier --check and cargo fmt --check. Made-with: Cursor * fix: PR #282 review — tool timeout config, tool_timeout module, CI diagnostics - Centralize VITE_TOOL_TIMEOUT_SECS in config.ts (TOOL_TIMEOUT_SECS) - Move tool_timeout to folder module; merge_published_state broadcasts SKILL_STATE_CHANGED - Resync guard after socket reconnect; qjs_engine logs data dir stat errors - E2E: registry/lifecycle/multi-round failure diagnostics; onboarding label constant - chatSendError arrow export; rustfmt/import grouping in event_loop Made-with: Cursor * test(e2e): add Gmail skill end-to-end tests - Introduced a comprehensive end-to-end test suite for the Gmail skill, covering the full lifecycle from discovery to OAuth completion and email management. - Implemented two modes: a lifecycle-only mode that validates the skill's event loop without real credentials, and a live mode that interacts with the Gmail API using actual credentials. - Added detailed environment variable requirements and usage instructions for both testing modes. Closes #XXX (replace with relevant issue number if applicable) --- .env.example | 2 + app/.env.example | 4 + app/src/chat/chatSendError.ts | 22 + app/src/lib/skills/hooks.ts | 48 + app/src/lib/skills/manager.ts | 84 +- app/src/pages/Conversations.tsx | 32 +- app/src/pages/Skills.tsx | 36 +- app/src/services/socketService.ts | 5 +- app/src/utils/config.ts | 16 + app/src/utils/tauriCommands.ts | 2 +- app/src/utils/withTimeout.ts | 31 + app/test/e2e/helpers/shared-flows.ts | 139 ++- .../e2e/specs/auth-access-control.spec.ts | 140 +-- app/test/e2e/specs/skill-lifecycle.spec.ts | 56 ++ app/test/e2e/specs/skill-multi-round.spec.ts | 55 + app/test/e2e/specs/skill-oauth.spec.ts | 42 + .../e2e/specs/skill-socket-reconnect.spec.ts | 42 + app/test/e2e/specs/skills-registry.spec.ts | 16 + app/test/e2e/specs/voice-mode.spec.ts | 44 +- src/openhuman/agent/harness/executor.rs | 2 +- src/openhuman/agent/harness/types.rs | 3 +- src/openhuman/agent/loop_/tool_loop.rs | 19 +- src/openhuman/mod.rs | 1 + src/openhuman/skills/ping_scheduler.rs | 44 +- src/openhuman/skills/qjs_engine.rs | 63 ++ .../skills/qjs_skill_instance/event_loop.rs | 19 +- src/openhuman/skills/schemas.rs | 27 + src/openhuman/skills/skill_registry.rs | 32 +- src/openhuman/tool_timeout/mod.rs | 29 + src/openhuman/tools/delegate.rs | 9 +- tests/json_rpc_e2e.rs | 13 + tests/skills_gmail_e2e.rs | 941 ++++++++++++++++++ 32 files changed, 1669 insertions(+), 349 deletions(-) create mode 100644 app/src/chat/chatSendError.ts create mode 100644 app/src/utils/withTimeout.ts create mode 100644 app/test/e2e/specs/skill-lifecycle.spec.ts create mode 100644 app/test/e2e/specs/skill-multi-round.spec.ts create mode 100644 app/test/e2e/specs/skill-oauth.spec.ts create mode 100644 app/test/e2e/specs/skill-socket-reconnect.spec.ts create mode 100644 src/openhuman/tool_timeout/mod.rs create mode 100644 tests/skills_gmail_e2e.rs diff --git a/.env.example b/.env.example index 7dd775bf1..db828a2c1 100644 --- a/.env.example +++ b/.env.example @@ -46,6 +46,8 @@ OPENHUMAN_MODEL= OPENHUMAN_WORKSPACE= # [optional] Default: 0.7 OPENHUMAN_TEMPERATURE=0.7 +# [optional] Skill + agent tool execution timeout in seconds (default 120, max 3600) +# OPENHUMAN_TOOL_TIMEOUT_SECS= # --------------------------------------------------------------------------- # Runtime flags diff --git a/app/.env.example b/app/.env.example index e681b986d..306d5ee28 100644 --- a/app/.env.example +++ b/app/.env.example @@ -21,3 +21,7 @@ VITE_DEV_JWT_TOKEN= # [optional] Dev-only: force onboarding flow to always show VITE_DEV_FORCE_ONBOARDING=false + +# [optional] Client-side timeout for skill callTool/triggerSync (seconds; default 120, max 3600). +# Should match OPENHUMAN_TOOL_TIMEOUT_SECS on the core when set. +# VITE_TOOL_TIMEOUT_SECS= diff --git a/app/src/chat/chatSendError.ts b/app/src/chat/chatSendError.ts new file mode 100644 index 000000000..3b61055e1 --- /dev/null +++ b/app/src/chat/chatSendError.ts @@ -0,0 +1,22 @@ +/** Structured chat send / delivery errors (issue #219) — stable `code` for analytics and tests. */ + +export type ChatSendErrorCode = + | 'socket_disconnected' + | 'local_model_failed' + | 'cloud_send_failed' + | 'voice_transcription' + | 'microphone_unavailable' + | 'microphone_recording' + | 'microphone_access' + | 'voice_playback' + | 'safety_timeout'; + +export interface ChatSendError { + code: ChatSendErrorCode; + message: string; +} + +export const chatSendError = (code: ChatSendErrorCode, message: string): ChatSendError => ({ + code, + message, +}); diff --git a/app/src/lib/skills/hooks.ts b/app/src/lib/skills/hooks.ts index 40aa0c988..c531cb582 100644 --- a/app/src/lib/skills/hooks.ts +++ b/app/src/lib/skills/hooks.ts @@ -7,6 +7,8 @@ import { useCallback, useEffect, useRef, useState } from 'react'; +import type { SkillSyncStatsLike } from '../../pages/skillsSyncUi'; +import { runtimeSkillDataStats } from '../../utils/tauriCommands'; import type { SkillConnectionStatus, SkillHostConnectionState } from './types'; import { onSkillStateChange } from './skillEvents'; import { @@ -209,3 +211,49 @@ export function useSkillConnectionInfo(skillId: string): { isInitialized: !!hostState?.is_initialized, }; } + +/** + * Disk usage under the skill data directory (from core RPC). Refreshes on skill events + * and periodically while the skill is connected. + */ +export function useSkillDataDirectoryStats( + skillId: string, + fetchEnabled: boolean, +): Pick | undefined { + const [stats, setStats] = useState< + Pick | undefined + >(undefined); + + useEffect(() => { + if (!fetchEnabled) { + setStats(undefined); + return; + } + let cancelled = false; + const load = async () => { + try { + const d = await runtimeSkillDataStats(skillId); + if (!cancelled) { + setStats({ + localDataBytes: d.total_bytes, + localFileCount: d.file_count, + }); + } + } catch { + if (!cancelled) setStats(undefined); + } + }; + void load(); + const interval = setInterval(load, 30_000); + const unsub = onSkillStateChange((id) => { + if (!id || id === skillId) void load(); + }); + return () => { + cancelled = true; + clearInterval(interval); + unsub(); + }; + }, [skillId, fetchEnabled]); + + return stats; +} diff --git a/app/src/lib/skills/manager.ts b/app/src/lib/skills/manager.ts index c7fd9768d..483b9d8f1 100644 --- a/app/src/lib/skills/manager.ts +++ b/app/src/lib/skills/manager.ts @@ -30,11 +30,13 @@ import { runtimeSkillDataRead, runtimeSkillDataWrite, } from "../../utils/tauriCommands"; +import { toolExecutionTimeoutMsFromEnv, withTimeout } from "../../utils/withTimeout"; // Env vars kept for reverse RPC compatibility (may be used by skills via state) class SkillManager { private runtimes = new Map(); + private resyncAfterReconnectInProgress = false; /** * Get skill-specific load parameters (e.g., wallet address for wallet skill) @@ -110,6 +112,21 @@ class SkillManager { } } + /** + * After realtime socket reconnect: refresh tool lists for every running skill so + * `tool:sync` matches the Rust engine (issue #215). + */ + async resyncRunningSkillsAfterReconnect(): Promise { + if (this.resyncAfterReconnectInProgress) return; + this.resyncAfterReconnectInProgress = true; + try { + const ids = [...this.runtimes.keys()]; + await Promise.all(ids.map((id) => this.activateSkill(id))); + } finally { + this.resyncAfterReconnectInProgress = false; + } + } + /** * Activate a skill that has completed setup — list its tools and mark as ready. */ @@ -197,7 +214,12 @@ class SkillManager { console.error(`[SkillManager] callTool failed — skill "${skillId}" has no running runtime`); throw new Error(`Skill ${skillId} is not running`); } - const result = await runtime.callTool(name, args); + const timeoutMs = toolExecutionTimeoutMsFromEnv(); + const result = await withTimeout( + runtime.callTool(name, args), + timeoutMs, + `[SkillManager] callTool skill="${skillId}" tool="${name}"`, + ); console.log(`[SkillManager] callTool result skill="${skillId}" tool="${name}" isError=${result.isError}`); return result; } @@ -229,16 +251,25 @@ class SkillManager { * Progress updates are published to Redux via the skill's state fields. */ async triggerSync(skillId: string): Promise { + const timeoutMs = toolExecutionTimeoutMsFromEnv(); const runtime = this.runtimes.get(skillId); if (runtime) { - await runtime.triggerSync(); + await withTimeout( + runtime.triggerSync(), + timeoutMs, + `[SkillManager] triggerSync skill="${skillId}"`, + ); } else { // Try via core RPC pass-through try { - await callCoreRpc({ - method: "openhuman.skills_sync", - params: { skill_id: skillId }, - }); + await withTimeout( + callCoreRpc({ + method: "openhuman.skills_sync", + params: { skill_id: skillId }, + }), + timeoutMs, + `[SkillManager] skills_sync skill="${skillId}"`, + ); } catch { // Skill not running — skip sync silently } @@ -366,30 +397,27 @@ class SkillManager { // Revoke OAuth credential before stopping so the running skill can clean up // its in-memory state and the event loop deletes oauth_credential.json. let revokeSucceeded = false; - if (credentialId) { - try { - await rpcRevokeOAuth(skillId, credentialId); - revokeSucceeded = true; - } catch (err) { - console.debug( - "[SkillManager] oauth/revoked failed (runtime may be stopped):", - err, - ); - } + try { + await rpcRevokeOAuth(skillId, credentialId ?? "default"); + revokeSucceeded = true; + } catch (err) { + console.debug( + "[SkillManager] oauth/revoked failed (runtime may be stopped):", + err, + ); } - await this.stopSkill(skillId); - - // Host-side fallback: if the RPC couldn't reach the runtime (already stopped, - // or non-OAuth skill), delete the persisted credential file so it isn't - // restored on next start. - if (!revokeSucceeded) { - await removePersistedOAuthCredential(skillId).catch((err) => { - console.debug( - "[SkillManager] host-side credential cleanup failed:", - err, - ); - }); + try { + await this.stopSkill(skillId); + } finally { + if (!revokeSucceeded) { + await removePersistedOAuthCredential(skillId).catch((err) => { + console.debug( + "[SkillManager] host-side credential cleanup failed:", + err, + ); + }); + } } await rpcSetSetupComplete(skillId, false).catch(() => {}); diff --git a/app/src/pages/Conversations.tsx b/app/src/pages/Conversations.tsx index a8743875e..7fe755fd0 100644 --- a/app/src/pages/Conversations.tsx +++ b/app/src/pages/Conversations.tsx @@ -3,6 +3,7 @@ import { useEffect, useRef, useState } from 'react'; import Markdown from 'react-markdown'; import { useNavigate } from 'react-router-dom'; +import { type ChatSendError, chatSendError } from '../chat/chatSendError'; import { useLocalModelStatus } from '../hooks/useLocalModelStatus'; import { creditsApi, type TeamUsage } from '../services/api/creditsApi'; import { inferenceApi, type ModelInfo } from '../services/api/inferenceApi'; @@ -107,7 +108,7 @@ const Conversations = () => { const [selectedModel, setSelectedModel] = useState('agentic-v1'); const [isLoadingModels, setIsLoadingModels] = useState(false); const [isSending, setIsSending] = useState(false); - const [sendError, setSendError] = useState(null); + const [sendError, setSendError] = useState(null); const socketStatus = useAppSelector(selectSocketStatus); const [toolTimelineByThread, setToolTimelineByThread] = useState< Record @@ -568,7 +569,7 @@ const Conversations = () => { if (!trimmed || !selectedThreadId || isSending) return; if (!isLocalModelActiveRef.current && socketStatus !== 'connected') { - setSendError('Realtime socket is not connected.'); + setSendError(chatSendError('socket_disconnected', 'Realtime socket is not connected.')); return; } @@ -602,6 +603,12 @@ const Conversations = () => { sendingTimeoutRef.current = setTimeout(() => { console.warn('[chat] safety timeout: clearing isSending after 120s with no response'); setIsSending(false); + setSendError( + chatSendError( + 'safety_timeout', + 'No response from the assistant after 2 minutes. Try again or check your connection.' + ) + ); dispatch(setActiveThread(null)); sendingTimeoutRef.current = null; }, 120_000); @@ -648,7 +655,7 @@ const Conversations = () => { } catch (err) { pendingReactionRef.current.delete(sendingThreadId); const msg = err instanceof Error ? err.message : String(err); - setSendError(msg); + setSendError(chatSendError('local_model_failed', msg)); dispatch( addInferenceResponse({ content: 'Local model error — please try again.', @@ -674,7 +681,7 @@ const Conversations = () => { sendingTimeoutRef.current = null; } const msg = err instanceof Error ? err.message : String(err); - setSendError(msg); + setSendError(chatSendError('cloud_send_failed', msg)); setIsSending(false); dispatch(setActiveThread(null)); } @@ -719,7 +726,7 @@ const Conversations = () => { await handleSendMessage(transcript); } catch (err) { const message = err instanceof Error ? err.message : String(err); - setSendError(`Voice transcription failed: ${message}`); + setSendError(chatSendError('voice_transcription', `Voice transcription failed: ${message}`)); setVoiceStatus(null); } finally { setIsTranscribing(false); @@ -730,7 +737,10 @@ const Conversations = () => { if (!rustChat || isSending || isTranscribing) return; if (!canUseMicrophoneApi) { setSendError( - 'Microphone capture is unavailable in this runtime. Use Text mode, or run the desktop app bundle with microphone permissions enabled.' + chatSendError( + 'microphone_unavailable', + 'Microphone capture is unavailable in this runtime. Use Text mode, or run the desktop app bundle with microphone permissions enabled.' + ) ); return; } @@ -766,7 +776,7 @@ const Conversations = () => { setIsRecording(false); mediaStreamRef.current?.getTracks().forEach(track => track.stop()); mediaStreamRef.current = null; - setSendError('Microphone recording failed.'); + setSendError(chatSendError('microphone_recording', 'Microphone recording failed.')); }; recorder.onstop = () => { void transcribeAndSendAudio(recorder.mimeType); @@ -779,7 +789,7 @@ const Conversations = () => { recorder.start(); } catch (err) { const message = err instanceof Error ? err.message : String(err); - setSendError(`Microphone access failed: ${message}`); + setSendError(chatSendError('microphone_access', `Microphone access failed: ${message}`)); setVoiceStatus(null); } }; @@ -815,7 +825,7 @@ const Conversations = () => { await audio.play(); } catch { if (!cancelled) { - setSendError('Failed to play voice reply.'); + setSendError(chatSendError('voice_playback', 'Failed to play voice reply.')); } } finally { if (!cancelled) { @@ -1318,7 +1328,9 @@ const Conversations = () => { {sendError && (
-

{sendError}

+

+ {sendError.message} +