feat(hermes): Hermes Agent backend + Hermes-only mode#521
Open
cogwheel0 wants to merge 8 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a full Hermes Agent backend integration to enable direct connections to self-hosted Hermes backends. A shared ChangesHermes Agent Integration
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatPage
participant chat_providers
participant HermesApiService
participant parseHermesRunStream
participant ChatNotifier
User->>ChatPage: send message (Hermes model selected)
ChatPage->>chat_providers: durableSend(message)
chat_providers->>chat_providers: isHermesModel → bypass OpenWebUI outbox
chat_providers->>chat_providers: _dispatchHermesRunFromChat (ensure session, chain runId)
chat_providers->>HermesApiService: createRun(input, sessionId, previousResponseId)
HermesApiService-->>chat_providers: runId
chat_providers->>ChatNotifier: updateMessage(transport=hermesRun, runId)
chat_providers->>HermesApiService: runEvents(runId) [SSE stream]
loop Streaming
HermesApiService-->>parseHermesRunStream: SSE bytes
parseHermesRunStream-->>chat_providers: HermesTokenDelta / HermesToolProgress / HermesApprovalRequested
chat_providers->>ChatNotifier: appendContent / appendStatus / update approval metadata
end
HermesApiService-->>chat_providers: HermesRunDone
chat_providers->>ChatNotifier: finishStreaming + completeStreamingUi
sequenceDiagram
actor User
participant AssistantMessageWidget
participant HermesApprovalCard
participant HermesApiService
participant chatMessagesProvider
AssistantMessageWidget->>AssistantMessageWidget: read metadata["hermesApproval"]
AssistantMessageWidget->>HermesApprovalCard: render(state=pending, summary)
User->>HermesApprovalCard: tap Approve
HermesApprovalCard->>AssistantMessageWidget: onDecision(true)
AssistantMessageWidget->>chatMessagesProvider: updateMessageById(state=resolving)
AssistantMessageWidget->>HermesApiService: resolveApproval(runId, approvalId, approved=true)
HermesApiService-->>AssistantMessageWidget: success
AssistantMessageWidget->>chatMessagesProvider: updateMessageById(state=approved)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/chat/views/chat_page.dart (1)
2825-2830:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisable the selector action for Hermes, not just the chevron.
Line 2829 hides the chevron, but Line 2830 still wires the model selector callback. In a Hermes chat, tapping the label can still open the picker and switch away from the single-agent backend.
Proposed fix
ConduitAdaptiveAppBarModelSelector( label: modelLabel, maxWidth: maxModelWidth, isLoading: isLoadingConversation, showChevron: showModelDropdown, - onPressed: () => _openModelSelector(context), + onPressed: showModelDropdown + ? () => _openModelSelector(context) + : null, ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/chat/views/chat_page.dart` around lines 2825 - 2830, The ConduitAdaptiveAppBarModelSelector widget has its chevron hidden via the showChevron parameter when in Hermes chat, but the onPressed callback for _openModelSelector is still being executed unconditionally. This allows users to tap the label and open the model picker when they should not be able to in a Hermes chat. Modify the onPressed parameter to conditionally disable the callback by returning null or a no-op function when showModelDropdown is false, just as showChevron is being conditionally set.
🟡 Minor comments (11)
test/features/hermes/hermes_sessions_test.dart-11-29 (1)
11-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBring session tests in line with the required mocktail pattern.
_CaptureInterceptoris hand-rolled here; the test guideline requires usingmocktailin Flutter test files.As per coding guidelines, "
**/*_test.dart: Usepackage:checks,flutter_test, andmocktailfor tests in Flutter code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_sessions_test.dart` around lines 11 - 29, Replace the hand-rolled _CaptureInterceptor class with a mocktail-based mock implementation. Instead of manually extending Interceptor and implementing onRequest, create a MockInterceptor using mocktail's Mock class and configure it with the when().thenAnswer() pattern to mock the request capturing and response behavior. This aligns with the Flutter test guideline that requires using package:mocktail for mocking in test files.Source: Coding guidelines
test/features/hermes/hermes_stream_parser_test.dart-1-7 (1)
1-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse mocktail in
_test.dartfiles per repo standard.The tests currently rely on direct fixtures/assertions only; this file does not include
mocktailusage required by the testing guideline.As per coding guidelines, "
**/*_test.dart: Usepackage:checks,flutter_test, andmocktailfor tests in Flutter code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_stream_parser_test.dart` around lines 1 - 7, Add the mocktail import to the test file to comply with repo standards for Flutter test files. In the hermes_stream_parser_test.dart file, add import 'package:mocktail/mocktail.dart'; to the imports section alongside the existing package:checks and package:flutter_test imports, and then use mocktail to create mock objects for any dependencies needed by the HermesStreamParser class or related services being tested.Source: Coding guidelines
test/features/hermes/hermes_api_service_test.dart-1-40 (1)
1-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign test doubles with the project’s mocktail standard.
This test file uses a bespoke interceptor double but does not use
mocktail, which violates the test guideline for Flutter tests. Please migrate the request-capture double to amocktailmock/fake pattern.As per coding guidelines, "
**/*_test.dart: Usepackage:checks,flutter_test, andmocktailfor tests in Flutter code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_api_service_test.dart` around lines 1 - 40, The _CaptureInterceptor class is a custom test double that does not follow the project's mocktail standard for Flutter tests. Replace the _CaptureInterceptor class with a mocktail-based mock or fake of the Interceptor class that captures request details. Update the _service helper function to use the mocktail mock instead of instantiating _CaptureInterceptor, ensuring the mock captures RequestOptions and provides the same canned response behavior while adhering to the package:mocktail testing guidelines.Source: Coding guidelines
test/features/hermes/hermes_skill_prompts_test.dart-1-25 (1)
1-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace custom HTTP capture doubles with mocktail-based doubles.
This file’s bespoke interceptor test-double approach bypasses the repo’s required mocktail test style for Flutter tests.
As per coding guidelines, "
**/*_test.dart: Usepackage:checks,flutter_test, andmocktailfor tests in Flutter code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_skill_prompts_test.dart` around lines 1 - 25, Replace the custom _CaptureInterceptor class with mocktail-based mocks to align with the repository's required testing style for Flutter. Remove the entire _CaptureInterceptor class definition and instead use mocktail to create mocks for the Dio client and HTTP request/response interactions needed for the test. Update all test setup code that currently instantiates _CaptureInterceptor to use mocktail's mock and when/then patterns instead.Source: Coding guidelines
lib/features/hermes/services/hermes_message_mapper.dart-31-34 (1)
31-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid epoch-1970 synthetic timestamps for missing server times.
Line 33 synthesizes fallback timestamps from Unix epoch (
i * 1000), so rows withoutcreated_at/timestampappear as Jan 1970. Use a current baseline while preserving deterministic in-list ordering.Proposed fix
List<ChatMessage> hermesMessagesToChatMessages( List<Map<String, dynamic>> raw, { String? modelId, }) { const uuid = Uuid(); + final fallbackBase = DateTime.now(); final messages = <ChatMessage>[]; @@ timestamp: _parseTime(item['created_at'] ?? item['timestamp']) ?? - DateTime.fromMillisecondsSinceEpoch(i * 1000), + fallbackBase.add(Duration(milliseconds: i)), model: role == 'assistant' ? modelId : null, ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/hermes/services/hermes_message_mapper.dart` around lines 31 - 34, The timestamp fallback in the message construction (where DateTime.fromMillisecondsSinceEpoch(i * 1000) is used) creates synthetic timestamps from Unix epoch 1970 when server timestamps are missing, which is misleading. Replace this with a current time baseline while preserving deterministic ordering by adding a duration offset based on the index i, so messages without timestamps still sort deterministically relative to each other but appear with recent timestamps rather than 1970 dates.lib/features/hermes/widgets/hermes_job_editor.dart-75-93 (1)
75-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidation errors don’t reliably clear while editing after first failed save.
On Lines 81-92,
errorTextreads controller state, but there’s no rebuild trigger during typing. After_showErrorsis set, users can keep seeing stale “Required” text until another save attempt.Proposed fix
ConduitInput( label: 'Prompt', hint: 'What should the agent do each run?', controller: _prompt, minLines: 2, maxLines: 5, + onChanged: (_) { + if (_showErrors) setState(() {}); + }, errorText: _showErrors && _prompt.text.trim().isEmpty ? 'Required' : null, ), @@ ConduitInput( label: 'Schedule (cron)', hint: '0 9 * * *', controller: _schedule, + onChanged: (_) { + if (_showErrors) setState(() {}); + }, errorText: _showErrors && _schedule.text.trim().isEmpty ? 'Required' : null, ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/hermes/widgets/hermes_job_editor.dart` around lines 75 - 93, The errorText properties in the ConduitInput widgets for `_prompt` and `_schedule` read controller state but don't trigger rebuilds while the user types, causing stale "Required" error messages to persist even as users correct the fields. Add listeners to both `_prompt` and `_schedule` text controllers that call setState(() {}) when the text changes, ensuring the widget rebuilds as users type and allowing the errorText logic to re-evaluate and clear error messages dynamically. Initialize these listeners where the controllers are set up (likely in initState or the controller initialization) and ensure they are properly disposed in the dispose method to prevent memory leaks.lib/core/services/native_sheet_hydration_service.dart-570-573 (1)
570-573:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse slash-scoped
DebugLoggerscope values.Please change this scope to slash format (for example,
native/sheet) to match the Dart logging convention used in this repo.As per coding guidelines, "
**/*.dart: UseDebugLoggerfromlib/core/utils/debug_logger.dartfor diagnostics with slash-scopedscope:values ...; do not add raw🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/core/services/native_sheet_hydration_service.dart` around lines 570 - 573, The DebugLogger.error call in the native_sheet_hydration_service.dart file uses a hyphen-separated scope value ('native-sheet') instead of the slash-separated format required by the repo's Dart logging conventions. Change the scope parameter from 'native-sheet' to 'native/sheet' to match the slash-scoped naming convention used throughout the codebase.Source: Coding guidelines
lib/core/services/native_sheet_hydration_service.dart-490-566 (1)
490-566:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHermes detail copy is hard-coded and one line contradicts current behavior.
Line 492 says Hermes is used “as a model in the picker,” but Line 66 explicitly removes Hermes from the picker. Also, this new user-facing block should be localized instead of string literals.
💡 Suggested direction
- title: 'Enable Hermes Agent', - subtitle: - 'Use your self-hosted Hermes agent as a model in the picker.', + title: l10n.hermesAgentEnableTitle, + subtitle: l10n.hermesAgentEnableSubtitle, ... - title: 'Hermes Agent', - subtitle: 'Connect directly to a self-hosted Hermes agent.', + title: l10n.hermesAgentTitle, + subtitle: l10n.hermesAgentSubtitle,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/core/services/native_sheet_hydration_service.dart` around lines 490 - 566, The subtitle on the first toggle item (id: 'hermes-enabled') incorrectly states that Hermes is used "as a model in the picker" when it should be removed from the picker. Update this subtitle to accurately describe the Hermes agent's functionality and match the behavior on Line 66. Additionally, replace all hardcoded user-facing strings in this configuration block (titles, subtitles, and placeholders in all NativeSheetItemConfig objects and the final NativeSheetDetailConfig) with localized string references instead of string literals to support internationalization.lib/features/profile/views/profile_page.dart-372-374 (1)
372-374:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize the new Hermes account copy.
This new title/subtitle is hardcoded while surrounding account options use
AppLocalizations; please move these strings into localization keys for i18n consistency.💡 Suggested direction
- title: 'Hermes Agent', - subtitle: 'Connect a self-hosted Hermes agent', + title: AppLocalizations.of(context)!.hermesAgentTitle, + subtitle: AppLocalizations.of(context)!.hermesAgentSubtitle,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/profile/views/profile_page.dart` around lines 372 - 374, The title 'Hermes Agent' and subtitle 'Connect a self-hosted Hermes agent' are hardcoded strings while surrounding account options use AppLocalizations for internationalization support. Extract these hardcoded strings into localization keys by replacing the string literals with corresponding AppLocalizations method calls (similar to the pattern used in nearby account tile configurations), ensuring consistency across all account options in the profile page.lib/features/hermes/widgets/hermes_approval_card.dart-53-54 (1)
53-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLocalize newly added approval-card strings.
These user-facing strings are hardcoded in English, so they won’t translate with the rest of the app UI.
Also applies to: 63-63, 71-71, 82-82, 91-91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/hermes/widgets/hermes_approval_card.dart` around lines 53 - 54, The HermesApprovalCard widget contains multiple hardcoded English strings that need to be localized to support multi-language support in the app. Replace the hardcoded string literals at the specified lines (including 'Approval required' at line 53, and the other strings at lines 63, 71, 82, and 91) with localization keys using your app's localization system. This typically involves replacing direct string literals with calls to your localization service or provider that retrieves translated strings from your localization resources.lib/shared/widgets/adaptive_toolbar_components.dart-427-428 (1)
427-428:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDisable the AdaptiveButton path instead of using a no-op callback.
Line 428 uses
() {}when disabled, which still leaves the control tappable/interactive. This conflicts with the intended “no model choices” behavior and the fallback branch that truly disables taps.Proposed fix
- return AdaptiveButton.child( - onPressed: (isLoading || !showChevron) ? () {} : onPressed, + return AdaptiveButton.child( + onPressed: (isLoading || !showChevron) ? null : onPressed, style: AdaptiveButtonStyle.glass, size: AdaptiveButtonSize.large, padding: EdgeInsets.zero,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/shared/widgets/adaptive_toolbar_components.dart` around lines 427 - 428, The AdaptiveButton.child widget in the return statement is using a no-op callback `() {}` for the onPressed parameter when the button should be disabled (when `isLoading || !showChevron` is true), which keeps the button tappable instead of properly disabling it. Replace the conditional that uses `() {}` with logic that passes `null` to onPressed when the button should be disabled, allowing the AdaptiveButton to properly disable user interaction rather than just providing an empty callback.
🧹 Nitpick comments (3)
test/core/services/sse_frame_scanner_test.dart (1)
18-49: ⚡ Quick winAdd a regression test for
data:values with leading spaces.Please add one case that asserts intentional leading spaces in payload are preserved (e.g.,
data: indented) to prevent future whitespace-regression in framing logic.Suggested test addition
@@ test('joins multiple data lines and defaults event to null', () { @@ }); + + test('preserves intentional leading spaces in data values', () { + final scanner = SseFrameScanner(); + final frames = [ + ...scanner.addChunk('data: indented\n\n'), + ...scanner.close(), + ]; + check(frames).has((f) => f.length, 'length').equals(1); + check(frames[0].data).equals(' indented'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/services/sse_frame_scanner_test.dart` around lines 18 - 49, Add a new test case in the sse_frame_scanner_test.dart file following the existing pattern of other test methods that use SseFrameScanner. The new test should verify that leading spaces in SSE data values are preserved during parsing; create a scanner, add a chunk with a data line that contains leading spaces after the colon (e.g., "data: indented"), close the scanner, and assert that the resulting frame's data field contains the leading spaces intact to prevent future whitespace-handling regressions.test/features/hermes/hermes_stream_parser_test.dart (1)
13-36: ⚡ Quick winAdd a regression case for failed/cancelled tool events.
Current tool coverage validates start/complete only. Please add a case asserting terminal failure/cancelled tool frames produce
HermesToolProgress(done: true)so status lines don’t stay in-progress.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_stream_parser_test.dart` around lines 13 - 36, Add a new test case in the hermes_stream_parser_test.dart file to cover failed and cancelled tool events as a regression test. Create a test that parses a stream containing tool events with failed or cancelled status values (similar to the existing 'maps token deltas, tool start/complete, and done' test structure) and assert that these events produce HermesToolProgress instances with the done property set to true. This ensures that tool status lines properly transition to a completed state when a tool fails or is cancelled, preventing them from staying in-progress.test/features/hermes/hermes_run_transport_test.dart (1)
11-42: ⚡ Quick winUse
mocktailfor the Hermes service test double.The hand-written subclass works, but this test file should use the repository’s standard Flutter test stack; replace
_FakeHermesApiServicewith aMock implements HermesApiServiceand stubcreateRun,runEvents, andgetRun.As per coding guidelines,
**/*_test.dart: “Usepackage:checks,flutter_test, andmocktailfor tests in Flutter code.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_run_transport_test.dart` around lines 11 - 42, The _FakeHermesApiService class is a hand-written subclass of HermesApiService used as a test double, but the project standards require using mocktail instead. Replace the entire _FakeHermesApiService class definition with a Mock class that implements HermesApiService, then use mocktail's stubbing mechanisms to configure the behavior for the three methods: createRun, runEvents, and getRun. This approach aligns with the Flutter testing standards specified in the coding guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/core/services/sse_frame_scanner.dart`:
- Around line 92-103: The SSE field value parsing is over-trimming whitespace in
both the data and event field handlers. In the data field parsing block (where
line.startsWith('data:') is checked), the trimLeft() call removes all leading
whitespace when it should only remove a single optional space immediately after
the colon. Similarly, in the event field parsing block (where
line.startsWith('event:') is checked), the trim() call removes both leading and
trailing whitespace when it should only remove a single optional leading space.
Replace trimLeft() and trim() with logic that conditionally removes only a
single leading space character after the colon substring, preserving the
semantic whitespace in the field values.
In `@lib/features/chat/providers/chat_providers.dart`:
- Around line 3101-3102: The issue is that ChatPage._handleNewChat() method
calls the local ChatPage.startNewChat() path which clears messages and
conversation state but does not reset the hermesActiveSessionProvider, allowing
the next Hermes turn to reuse the previous server-side session. To fix this,
locate the ChatPage._handleNewChat() method and ensure it includes the same
hermesActiveSessionProvider reset logic
(ref.read(hermesActiveSessionProvider.notifier).set(null)) that was added at
line 3101-3102, so that every new-chat entrypoint properly clears the Hermes
session state.
In `@lib/features/chat/widgets/assistant_message_widget.dart`:
- Around line 1034-1043: The issue is that when hermesApiServiceProvider is
null, the null-aware operator prevents the resolveApproval call from executing,
but setApprovalState is still called on the next line, desynchronizing local
state from server truth. Add a null check for the result of
ref.read(hermesApiServiceProvider) before attempting to call resolveApproval,
and return early if it is null, ensuring setApprovalState is only invoked when
the API provider is available and the request can actually be sent.
In `@lib/features/hermes/models/hermes_config.dart`:
- Around line 32-43: The copyWith method in the HermesConfig class uses the `??`
operator for apiKey and sessionKey, which prevents these nullable secrets from
being explicitly cleared to null. When copyWith(apiKey: null) or
copyWith(sessionKey: null) is called, the old values are retained instead of
being cleared. Modify the copyWith method to allow apiKey and sessionKey to be
explicitly set to null values by using a pattern that distinguishes between "not
provided" and "explicitly null" - such as using a sentinel value or optional
wrapper type - so that deletion flows can properly clear stale secret state
while still keeping enabled and baseUrl using the `??` operator for backward
compatibility.
In `@lib/features/hermes/services/hermes_api_service.dart`:
- Around line 21-24: The receiveTimeout set to null in the HermesApiService Dio
client configuration applies globally to all API calls, which causes regular
endpoints to hang indefinitely. Replace the global receiveTimeout: null with a
reasonable timeout value (e.g., 30 seconds) to protect standard API calls from
hanging. Then, for SSE stream endpoints that require indefinite timeouts,
override the receiveTimeout: null specifically for those requests when making
the actual HTTP call, rather than setting it globally on the shared client
instance.
- Around line 109-114: Replace the invalid Dart map-entry syntax `'key': ?value`
with proper conditional map entries using the `if` statement. In the data map
for the request (around lines 111-113) and any other occurrences at lines 182
and 310, replace entries like `'session_id': ?sessionId` with `if (sessionId !=
null) 'session_id': sessionId` to conditionally include only non-null values in
the map. Apply this pattern to all conditional entries: sessionId, instructions,
and previousResponseId.
In `@lib/features/hermes/services/hermes_run_transport.dart`:
- Around line 123-128: The condition checking !gotContent before appending
recovered text in the _recoverRunOutput block prevents the full output from
being recovered when the SSE stream drops after partial content has already been
streamed. Modify the logic to append the recovered text regardless of whether
content was already received (remove or restructure the !gotContent check),
ensuring that when the stream drops, the complete output from the recovery
endpoint is properly appended to avoid truncated chat messages.
In `@lib/features/hermes/services/hermes_stream_parser.dart`:
- Around line 161-168: The done variable assignment in the HermesStreamParser
class only marks tool progress as complete for success states (completed, done,
success). To treat failed and cancelled tool events as terminal progress, add
additional checks to the done variable's conditional logic to also match status
strings for failed, cancelled, and stopped states. This ensures that tool status
rows don't remain stuck in-progress when events indicate failure or
cancellation.
In `@lib/features/hermes/views/hermes_settings_page.dart`:
- Around line 42-57: The `_testConnection()` method awaits `service.health()`
without exception handling, which means if the call throws an exception (network
error, Dio error, etc.), the setState call that sets `_testing = false` will not
execute, leaving the button stuck in a loading state. Wrap the `await
service.health()` call in a try-catch block, and ensure that in the catch block
you also call setState to set `_testing = false` and `_testResult = false`, so
the loading state is always cleared regardless of whether the health check
succeeds or fails.
In `@lib/features/hermes/widgets/hermes_session_tile.dart`:
- Around line 165-170: The try-catch block that calls
service.getSessionMessages(session.id) is silently catching all errors and
setting raw to an empty list, which masks fetch failures and makes existing
sessions appear empty instead of showing an error condition. Rather than using
catch (_) to swallow the error, handle the fetch failure appropriately by either
re-throwing the exception, showing an error state to the user, or propagating
the error up the call stack instead of defaulting to an empty list.
---
Outside diff comments:
In `@lib/features/chat/views/chat_page.dart`:
- Around line 2825-2830: The ConduitAdaptiveAppBarModelSelector widget has its
chevron hidden via the showChevron parameter when in Hermes chat, but the
onPressed callback for _openModelSelector is still being executed
unconditionally. This allows users to tap the label and open the model picker
when they should not be able to in a Hermes chat. Modify the onPressed parameter
to conditionally disable the callback by returning null or a no-op function when
showModelDropdown is false, just as showChevron is being conditionally set.
---
Minor comments:
In `@lib/core/services/native_sheet_hydration_service.dart`:
- Around line 570-573: The DebugLogger.error call in the
native_sheet_hydration_service.dart file uses a hyphen-separated scope value
('native-sheet') instead of the slash-separated format required by the repo's
Dart logging conventions. Change the scope parameter from 'native-sheet' to
'native/sheet' to match the slash-scoped naming convention used throughout the
codebase.
- Around line 490-566: The subtitle on the first toggle item (id:
'hermes-enabled') incorrectly states that Hermes is used "as a model in the
picker" when it should be removed from the picker. Update this subtitle to
accurately describe the Hermes agent's functionality and match the behavior on
Line 66. Additionally, replace all hardcoded user-facing strings in this
configuration block (titles, subtitles, and placeholders in all
NativeSheetItemConfig objects and the final NativeSheetDetailConfig) with
localized string references instead of string literals to support
internationalization.
In `@lib/features/hermes/services/hermes_message_mapper.dart`:
- Around line 31-34: The timestamp fallback in the message construction (where
DateTime.fromMillisecondsSinceEpoch(i * 1000) is used) creates synthetic
timestamps from Unix epoch 1970 when server timestamps are missing, which is
misleading. Replace this with a current time baseline while preserving
deterministic ordering by adding a duration offset based on the index i, so
messages without timestamps still sort deterministically relative to each other
but appear with recent timestamps rather than 1970 dates.
In `@lib/features/hermes/widgets/hermes_approval_card.dart`:
- Around line 53-54: The HermesApprovalCard widget contains multiple hardcoded
English strings that need to be localized to support multi-language support in
the app. Replace the hardcoded string literals at the specified lines (including
'Approval required' at line 53, and the other strings at lines 63, 71, 82, and
91) with localization keys using your app's localization system. This typically
involves replacing direct string literals with calls to your localization
service or provider that retrieves translated strings from your localization
resources.
In `@lib/features/hermes/widgets/hermes_job_editor.dart`:
- Around line 75-93: The errorText properties in the ConduitInput widgets for
`_prompt` and `_schedule` read controller state but don't trigger rebuilds while
the user types, causing stale "Required" error messages to persist even as users
correct the fields. Add listeners to both `_prompt` and `_schedule` text
controllers that call setState(() {}) when the text changes, ensuring the widget
rebuilds as users type and allowing the errorText logic to re-evaluate and clear
error messages dynamically. Initialize these listeners where the controllers are
set up (likely in initState or the controller initialization) and ensure they
are properly disposed in the dispose method to prevent memory leaks.
In `@lib/features/profile/views/profile_page.dart`:
- Around line 372-374: The title 'Hermes Agent' and subtitle 'Connect a
self-hosted Hermes agent' are hardcoded strings while surrounding account
options use AppLocalizations for internationalization support. Extract these
hardcoded strings into localization keys by replacing the string literals with
corresponding AppLocalizations method calls (similar to the pattern used in
nearby account tile configurations), ensuring consistency across all account
options in the profile page.
In `@lib/shared/widgets/adaptive_toolbar_components.dart`:
- Around line 427-428: The AdaptiveButton.child widget in the return statement
is using a no-op callback `() {}` for the onPressed parameter when the button
should be disabled (when `isLoading || !showChevron` is true), which keeps the
button tappable instead of properly disabling it. Replace the conditional that
uses `() {}` with logic that passes `null` to onPressed when the button should
be disabled, allowing the AdaptiveButton to properly disable user interaction
rather than just providing an empty callback.
In `@test/features/hermes/hermes_api_service_test.dart`:
- Around line 1-40: The _CaptureInterceptor class is a custom test double that
does not follow the project's mocktail standard for Flutter tests. Replace the
_CaptureInterceptor class with a mocktail-based mock or fake of the Interceptor
class that captures request details. Update the _service helper function to use
the mocktail mock instead of instantiating _CaptureInterceptor, ensuring the
mock captures RequestOptions and provides the same canned response behavior
while adhering to the package:mocktail testing guidelines.
In `@test/features/hermes/hermes_sessions_test.dart`:
- Around line 11-29: Replace the hand-rolled _CaptureInterceptor class with a
mocktail-based mock implementation. Instead of manually extending Interceptor
and implementing onRequest, create a MockInterceptor using mocktail's Mock class
and configure it with the when().thenAnswer() pattern to mock the request
capturing and response behavior. This aligns with the Flutter test guideline
that requires using package:mocktail for mocking in test files.
In `@test/features/hermes/hermes_skill_prompts_test.dart`:
- Around line 1-25: Replace the custom _CaptureInterceptor class with
mocktail-based mocks to align with the repository's required testing style for
Flutter. Remove the entire _CaptureInterceptor class definition and instead use
mocktail to create mocks for the Dio client and HTTP request/response
interactions needed for the test. Update all test setup code that currently
instantiates _CaptureInterceptor to use mocktail's mock and when/then patterns
instead.
In `@test/features/hermes/hermes_stream_parser_test.dart`:
- Around line 1-7: Add the mocktail import to the test file to comply with repo
standards for Flutter test files. In the hermes_stream_parser_test.dart file,
add import 'package:mocktail/mocktail.dart'; to the imports section alongside
the existing package:checks and package:flutter_test imports, and then use
mocktail to create mock objects for any dependencies needed by the
HermesStreamParser class or related services being tested.
---
Nitpick comments:
In `@test/core/services/sse_frame_scanner_test.dart`:
- Around line 18-49: Add a new test case in the sse_frame_scanner_test.dart file
following the existing pattern of other test methods that use SseFrameScanner.
The new test should verify that leading spaces in SSE data values are preserved
during parsing; create a scanner, add a chunk with a data line that contains
leading spaces after the colon (e.g., "data: indented"), close the scanner,
and assert that the resulting frame's data field contains the leading spaces
intact to prevent future whitespace-handling regressions.
In `@test/features/hermes/hermes_run_transport_test.dart`:
- Around line 11-42: The _FakeHermesApiService class is a hand-written subclass
of HermesApiService used as a test double, but the project standards require
using mocktail instead. Replace the entire _FakeHermesApiService class
definition with a Mock class that implements HermesApiService, then use
mocktail's stubbing mechanisms to configure the behavior for the three methods:
createRun, runEvents, and getRun. This approach aligns with the Flutter testing
standards specified in the coding guidelines.
In `@test/features/hermes/hermes_stream_parser_test.dart`:
- Around line 13-36: Add a new test case in the hermes_stream_parser_test.dart
file to cover failed and cancelled tool events as a regression test. Create a
test that parses a stream containing tool events with failed or cancelled status
values (similar to the existing 'maps token deltas, tool start/complete, and
done' test structure) and assert that these events produce HermesToolProgress
instances with the done property set to true. This ensures that tool status
lines properly transition to a completed state when a tool fails or is
cancelled, preventing them from staying in-progress.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bf8a8ed-5ce7-4e35-a995-5e0deadc1b0a
⛔ Files ignored due to path filters (1)
assets/icons/hermes_agent.pngis excluded by!**/*.png
📒 Files selected for processing (48)
lib/core/persistence/persistence_keys.dartlib/core/providers/app_providers.dartlib/core/router/app_router.dartlib/core/services/native_sheet_bridge.dartlib/core/services/native_sheet_hydration_service.dartlib/core/services/navigation_service.dartlib/core/services/openwebui_stream_parser.dartlib/core/services/secure_credential_storage.dartlib/core/services/sse_frame_scanner.dartlib/features/chat/providers/chat_providers.dartlib/features/chat/views/chat_page.dartlib/features/chat/widgets/assistant_message_widget.dartlib/features/chat/widgets/model_selector_sheet.dartlib/features/chat/widgets/modern_chat_input.dartlib/features/chat/widgets/prompt_suggestion_overlay.dartlib/features/hermes/models/hermes_capabilities.dartlib/features/hermes/models/hermes_config.dartlib/features/hermes/models/hermes_job.dartlib/features/hermes/models/hermes_model.dartlib/features/hermes/models/hermes_run_event.dartlib/features/hermes/models/hermes_session.dartlib/features/hermes/models/hermes_toolset.dartlib/features/hermes/providers/hermes_providers.dartlib/features/hermes/services/hermes_api_service.dartlib/features/hermes/services/hermes_message_mapper.dartlib/features/hermes/services/hermes_run_transport.dartlib/features/hermes/services/hermes_stream_parser.dartlib/features/hermes/views/hermes_jobs_page.dartlib/features/hermes/views/hermes_settings_page.dartlib/features/hermes/widgets/hermes_approval_card.dartlib/features/hermes/widgets/hermes_job_editor.dartlib/features/hermes/widgets/hermes_session_tile.dartlib/features/hermes/widgets/hermes_sessions_tab.dartlib/features/navigation/utils/sidebar_create_action.dartlib/features/navigation/widgets/sidebar_page.dartlib/features/navigation/widgets/sidebar_user_pill.dartlib/features/profile/views/profile_page.dartlib/main.dartlib/shared/widgets/adaptive_toolbar_components.darttest/core/services/sse_frame_scanner_test.darttest/features/hermes/hermes_api_service_test.darttest/features/hermes/hermes_model_test.darttest/features/hermes/hermes_real_payloads_test.darttest/features/hermes/hermes_run_transport_test.darttest/features/hermes/hermes_sessions_test.darttest/features/hermes/hermes_skill_prompts_test.darttest/features/hermes/hermes_stream_parser_test.darttest/features/hermes/hermes_tier1_test.dart
cogwheel0
force-pushed
the
feat/hermes-agent
branch
from
6493ded to
1e4713f
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| ios: CupertinoIcons.bolt_horizontal_circle, | ||
| android: Icons.smart_toy_outlined, | ||
| ), | ||
| title: 'Hermes Agent', |
There was a problem hiding this comment.
🟢 Low views/profile_page.dart:384
The new Hermes Agent tile hardcodes English title and subtitle instead of using AppLocalizations, so non-English users see untranslated English text. Add the keys to the localization bundle and use the generated accessors like the surrounding options.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @lib/features/profile/views/profile_page.dart around line 384:
The new Hermes Agent tile hardcodes English `title` and `subtitle` instead of using `AppLocalizations`, so non-English users see untranslated English text. Add the keys to the localization bundle and use the generated accessors like the surrounding options.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/chat/widgets/assistant_message_widget.dart`:
- Around line 1081-1092: The widget can be disposed during the await in the
approval resolution process, invalidating the WidgetRef and causing a StateError
when setApprovalState is subsequently called. Add a mounted check guard before
the final setApprovalState(approved ? 'approved' : 'denied') call that follows
the try-catch block, similar to the pattern used in
_handleQueuedCompletionActionError, to prevent state changes from being applied
to disposed widgets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b93fb593-d3d4-4d4d-856d-63291b8b1294
⛔ Files ignored due to path filters (1)
assets/icons/hermes_agent.pngis excluded by!**/*.png
📒 Files selected for processing (48)
lib/core/persistence/persistence_keys.dartlib/core/providers/app_providers.dartlib/core/router/app_router.dartlib/core/services/native_sheet_bridge.dartlib/core/services/native_sheet_hydration_service.dartlib/core/services/navigation_service.dartlib/core/services/openwebui_stream_parser.dartlib/core/services/secure_credential_storage.dartlib/core/services/sse_frame_scanner.dartlib/features/chat/providers/chat_providers.dartlib/features/chat/views/chat_page.dartlib/features/chat/widgets/assistant_message_widget.dartlib/features/chat/widgets/model_selector_sheet.dartlib/features/chat/widgets/modern_chat_input.dartlib/features/chat/widgets/prompt_suggestion_overlay.dartlib/features/hermes/models/hermes_capabilities.dartlib/features/hermes/models/hermes_config.dartlib/features/hermes/models/hermes_job.dartlib/features/hermes/models/hermes_model.dartlib/features/hermes/models/hermes_run_event.dartlib/features/hermes/models/hermes_session.dartlib/features/hermes/models/hermes_toolset.dartlib/features/hermes/providers/hermes_providers.dartlib/features/hermes/services/hermes_api_service.dartlib/features/hermes/services/hermes_message_mapper.dartlib/features/hermes/services/hermes_run_transport.dartlib/features/hermes/services/hermes_stream_parser.dartlib/features/hermes/views/hermes_jobs_page.dartlib/features/hermes/views/hermes_settings_page.dartlib/features/hermes/widgets/hermes_approval_card.dartlib/features/hermes/widgets/hermes_job_editor.dartlib/features/hermes/widgets/hermes_session_tile.dartlib/features/hermes/widgets/hermes_sessions_tab.dartlib/features/navigation/utils/sidebar_create_action.dartlib/features/navigation/widgets/sidebar_page.dartlib/features/navigation/widgets/sidebar_user_pill.dartlib/features/profile/views/profile_page.dartlib/main.dartlib/shared/widgets/adaptive_toolbar_components.darttest/core/services/sse_frame_scanner_test.darttest/features/hermes/hermes_api_service_test.darttest/features/hermes/hermes_model_test.darttest/features/hermes/hermes_real_payloads_test.darttest/features/hermes/hermes_run_transport_test.darttest/features/hermes/hermes_sessions_test.darttest/features/hermes/hermes_skill_prompts_test.darttest/features/hermes/hermes_stream_parser_test.darttest/features/hermes/hermes_tier1_test.dart
✅ Files skipped from review due to trivial changes (3)
- test/features/hermes/hermes_model_test.dart
- lib/features/hermes/models/hermes_model.dart
- lib/core/persistence/persistence_keys.dart
🚧 Files skipped from review as they are similar to previous changes (43)
- lib/core/services/native_sheet_bridge.dart
- lib/features/hermes/models/hermes_session.dart
- test/core/services/sse_frame_scanner_test.dart
- lib/features/hermes/widgets/hermes_job_editor.dart
- lib/features/hermes/models/hermes_config.dart
- lib/features/hermes/models/hermes_toolset.dart
- test/features/hermes/hermes_skill_prompts_test.dart
- lib/features/profile/views/profile_page.dart
- lib/features/chat/widgets/model_selector_sheet.dart
- test/features/hermes/hermes_api_service_test.dart
- lib/features/chat/widgets/prompt_suggestion_overlay.dart
- test/features/hermes/hermes_run_transport_test.dart
- lib/core/router/app_router.dart
- test/features/hermes/hermes_tier1_test.dart
- lib/features/hermes/models/hermes_capabilities.dart
- test/features/hermes/hermes_stream_parser_test.dart
- lib/features/hermes/services/hermes_message_mapper.dart
- lib/features/navigation/widgets/sidebar_user_pill.dart
- test/features/hermes/hermes_real_payloads_test.dart
- lib/core/providers/app_providers.dart
- test/features/hermes/hermes_sessions_test.dart
- lib/core/services/secure_credential_storage.dart
- lib/core/services/sse_frame_scanner.dart
- lib/features/hermes/widgets/hermes_sessions_tab.dart
- lib/features/hermes/widgets/hermes_approval_card.dart
- lib/core/services/native_sheet_hydration_service.dart
- lib/core/services/navigation_service.dart
- lib/features/chat/views/chat_page.dart
- lib/main.dart
- lib/features/chat/widgets/modern_chat_input.dart
- lib/shared/widgets/adaptive_toolbar_components.dart
- lib/features/hermes/models/hermes_run_event.dart
- lib/features/hermes/models/hermes_job.dart
- lib/features/navigation/utils/sidebar_create_action.dart
- lib/features/hermes/widgets/hermes_session_tile.dart
- lib/core/services/openwebui_stream_parser.dart
- lib/features/hermes/views/hermes_settings_page.dart
- lib/features/navigation/widgets/sidebar_page.dart
- lib/features/hermes/services/hermes_run_transport.dart
- lib/features/hermes/services/hermes_stream_parser.dart
- lib/features/hermes/views/hermes_jobs_page.dart
- lib/features/chat/providers/chat_providers.dart
- lib/features/hermes/services/hermes_api_service.dart
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/features/hermes/hermes_stream_parser_test.dart (1)
1-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign this test file with the required Flutter test stack.
This file appears to use
checksandflutter_test, but notmocktail, which is required by the repository test guideline for*_test.dartfiles.As per coding guidelines, "
**/*_test.dart: Usepackage:checks,flutter_test, andmocktailfor tests in Flutter code".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/hermes/hermes_stream_parser_test.dart` around lines 1 - 7, The test file is missing the required mocktail import which is mandated by the repository's test guidelines for all *_test.dart files. Add an import statement for package:mocktail/mocktail.dart to the import section at the top of the file, placing it alongside the existing imports for checks and flutter_test to align with the required Flutter test stack.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/features/hermes/hermes_stream_parser_test.dart`:
- Around line 1-7: The test file is missing the required mocktail import which
is mandated by the repository's test guidelines for all *_test.dart files. Add
an import statement for package:mocktail/mocktail.dart to the import section at
the top of the file, placing it alongside the existing imports for checks and
flutter_test to align with the required Flutter test stack.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47ba1f8e-fe23-4519-b124-8aba724ef239
📒 Files selected for processing (15)
lib/core/services/sse_frame_scanner.dartlib/features/chat/widgets/assistant_message_widget.dartlib/features/hermes/models/hermes_config.dartlib/features/hermes/providers/hermes_providers.dartlib/features/hermes/services/hermes_api_service.dartlib/features/hermes/services/hermes_message_mapper.dartlib/features/hermes/services/hermes_run_transport.dartlib/features/hermes/services/hermes_stream_parser.dartlib/features/hermes/views/hermes_jobs_page.dartlib/features/hermes/views/hermes_settings_page.dartlib/features/hermes/widgets/hermes_session_tile.dartlib/features/hermes/widgets/hermes_sessions_tab.dartlib/shared/widgets/adaptive_toolbar_components.dartpaseo.jsontest/features/hermes/hermes_stream_parser_test.dart
✅ Files skipped from review due to trivial changes (1)
- paseo.json
🚧 Files skipped from review as they are similar to previous changes (12)
- lib/features/hermes/models/hermes_config.dart
- lib/shared/widgets/adaptive_toolbar_components.dart
- lib/features/hermes/services/hermes_message_mapper.dart
- lib/features/hermes/views/hermes_jobs_page.dart
- lib/features/hermes/widgets/hermes_session_tile.dart
- lib/core/services/sse_frame_scanner.dart
- lib/features/chat/widgets/assistant_message_widget.dart
- lib/features/hermes/widgets/hermes_sessions_tab.dart
- lib/features/hermes/services/hermes_run_transport.dart
- lib/features/hermes/views/hermes_settings_page.dart
- lib/features/hermes/services/hermes_api_service.dart
- lib/features/hermes/services/hermes_stream_parser.dart
Comment on lines
+264
to
+266
| onPressed: () => | ||
| ref.read(hermesJobsProvider.notifier).runNow(job.id), | ||
| ), |
There was a problem hiding this comment.
onPressed: () => ref.read(hermesJobsProvider.notifier).runNow(job.id) returns a Future<void> that is never awaited or attached to an error handler. When runNow → runJob throws a DioException (unreachable server, 401/403, server-side 500), the exception becomes an unhandled async error and the user receives no feedback — they assume the run started. The same pattern exists in hermes_jobs_page.dart on the _JobCard run button. Both sites should wrap the call in a try/catch and surface failure via a snackbar or UiUtils.showMessage, matching the error-handling pattern already applied to the Fork/Rename/Delete context menu actions via _runSessionAction.
cogwheel0
changed the title
Add Nous Research's self-hosted Hermes Agent as a second backend, separate
from the Open WebUI client, surfaced as a synthetic "Hermes Agent" model and a
dedicated sidebar tab.
Core
- Direct Hermes API client (own Dio + bearer/X-Hermes-* auth), runs transport
with inline tool progress, human-approval gates, persistent-memory headers,
and dropped-stream recovery via GET /v1/runs/{id}.
- Shared SSE frame scanner extracted from the OpenWebUI parser; defensive
Hermes runs-event parser validated against real server traffic.
- Send routing branches to Hermes in durableSend/_sendMessageInternal; chats
are local/ephemeral on the OWUI side, persisted server-side via /api/sessions.
Tiers 1 & 2
- Discovery: /v1/capabilities (feature-gates approval/skills/jobs),
/v1/toolsets, /health/detailed surfaced in settings.
- Sessions: dedicated Hermes tab (open/fork/rename/delete) backed by
/api/sessions; tiles mirror chat tiles with in-progress + selected states and
a long-press context menu.
- Scheduled Agents (/api/jobs): CRUD + pause/resume/run, read-only when the
server reports jobs_admin=false; also a collapsible section in the tab.
- Slash commands sourced from /v1/skills.
UX
- Settings reachable on iOS via the native settings sheet; Hermes model hidden
from the normal picker and the header dropdown suppressed for single-agent
chats. Real Hermes logo as the tab icon (theme-aware alpha template).
Verified end-to-end on an iOS simulator against a live Hermes server.
flutter analyze clean; flutter test (2200) passing.
The Hermes sessions tab used a bare RefreshIndicator + CustomScrollView with no inset handling, so content ran under the native sidebar chrome and bottom tab bar. Mirror the Chats tab: top/bottom spacer slivers via sidebarTabContentTopPadding/BottomPadding and a ConduitRefreshIndicator with sidebarRefreshIndicatorEdgeOffset.
- openHermesSession: set isManualModelSelectionProvider so default-model restoration can't overwrite the Hermes model on session open (P1). - Hermes approval: null-check hermesApiServiceProvider before resolving so a disabled-between-display-and-tap approval doesn't show success while the server-side run stays blocked; log both null + failure paths (P1). - _isTruthyError: treat numeric 0 as a non-error (Python servers send int 0) (P2). - Session context-menu Fork/Rename/Delete: await + catch + user feedback instead of dropping the futures silently (P2).
Error surfacing (stream parser):
- run.failed / response.failed now emit a HermesRunError (response.failed
previously surfaced nothing); falsy error markers ("False"/0) fall back to a
generic message instead of showing "False".
- Failed/cancelled/stopped tool events are marked terminal so the tool row
stops spinning. + regression tests.
Robustness / lifecycle:
- Opening a session that fails to load now shows an error and aborts instead of
silently opening an empty transcript.
- Hermes settings "Test connection" wraps health() so the spinner always clears.
- Jobs page create/edit/delete and the approval resolver guard context.mounted /
mounted after their awaits (StateError on disposed widget).
- Run transport recovers the missing suffix when an SSE stream drops mid-content
(previously recovered output was ignored once any delta had streamed).
API / parsing:
- Scope the Dio no-receive-timeout to the SSE stream request; regular endpoints
now have a finite 60s receive timeout so they can't hang forever.
- SSE field parsing strips only a single optional leading space (spec), no longer
over-trimming payload whitespace/indentation.
- HermesConfig.copyWith uses a sentinel so secrets can be explicitly cleared.
- Toolbar pill reserves chevron width only when a chevron is shown.
flutter analyze clean; flutter test (2312) passing.
A literal NUL had crept into `_keep = '\x00__hermes_keep__'`, which made git treat hermes_providers.dart as binary (breaking 3-way merges/diffs). The value is only ever compared with identical(), so dropping the byte is behavior-neutral.
Make Open WebUI optional so a user can install the app and use a self-hosted Hermes Agent exclusively. The mode is derived from what's configured and is switchable; first run shows a backend chooser. - Mode signals: a synchronous persisted `preferredBackend` (unset|owui|hermes) for boot-deterministic routing, plus a derived `hermesOnlyMode` for UI gating (mirrors the reviewer-mode precedent). - Router: Hermes-only short-circuits to chat with no OWUI server/auth; first run routes to a new backend chooser; holds splash while Hermes secrets load. - Onboarding: BackendChooserPage + HermesSettingsPage(isOnboarding) with a Finish button that sets preferredBackend=hermes and enters the app. - Models/defaultModel surface + auto-select the synthetic Hermes model when there's no OWUI API; send guard relaxed for Hermes models. - UI gating: hide chats/notes/terminal/channels (3 synced sites); Hermes is the home tab. preferredBackend=owui set on OWUI connect. Verified end-to-end on an erased iOS simulator (fresh install): chooser → Hermes setup → Finish → Hermes-only chat → only the Hermes tab → relaunch boots straight to chat → send creates a server session with no OWUI. flutter analyze clean; flutter test (2200) passing.
…, tests Address the Hermes-only follow-ups: - Composer: hide OWUI affordances (the "+" overflow button, quick pills, and the iOS keyboard-accessory actions) when a Hermes model is selected — Hermes has no OWUI tools/web-search/image-gen/attachments and uses its own `/` skills. - iOS native settings sheet: gate the OWUI account sections (profile header, AI memory, data connection, password + profile detail sheets, sign-out) on Hermes-only, and make the About detail skip the server lookup so it no longer errors with no server. - Bidirectional switching: add a "Connect to Open WebUI" entry in both the native sheet (routed via a control event in main.dart) and the Flutter profile page; let the router reach the OWUI connect/auth routes for a Hermes-only user; reset preferredBackend to unset when a Hermes-only backend is disabled. - Tests: preferredBackend parse/persistence round-trip and hermesOnlyMode derivation (usable/no-server, server-present, disabled, incomplete, reviewer precedence). flutter analyze clean; flutter test (2211) passing.
…tests
Close out the last Hermes-only follow-ups:
- iOS native settings sheet: render a Hermes-branded profile header in
Hermes-only mode ("Hermes Agent" + the agent host + the hermes_agent.png
avatar). Fully data-driven from Dart — no Swift change; the avatar bytes are
pre-loaded (cached) before the sync config builder runs.
- Extract the inline send/regenerate guard into a pure isSendBlocked() helper
used at both sites, removing the duplicated condition and making the
Hermes-only relaxation unit-testable.
- Tests: isSendBlocked (null model, OWUI-no-api, api present, reviewer, Hermes
relaxation); modelsProvider surfaces only the synthetic Hermes model when
unauthed + usable Hermes; defaultModelProvider auto-selects + writes through
the Hermes model when api == null.
flutter analyze clean; flutter test (2218) passing.
cogwheel0
force-pushed
the
feat/hermes-agent
branch
from
b85676e to
3aec173
Compare
Comment on lines
+92
to
+98
| Future<void> _createJob(BuildContext context, WidgetRef ref) async { | ||
| final result = await showHermesJobEditor(context); | ||
| if (result == null || !context.mounted) return; | ||
| await ref | ||
| .read(hermesJobsProvider.notifier) | ||
| .create(prompt: result.prompt, schedule: result.schedule); | ||
| } |
There was a problem hiding this comment.
_createJob, _editJob, _deleteJob, and the AdaptiveSwitch.onChanged callback for setEnabled all fire async operations without a try/catch. Their futures are discarded at each call site (onPressed: () => _createJob(...), onPressed: () => _editJob(...), etc.), so a DioException from an unreachable server or a 4xx/5xx response becomes an unhandled async error — the operation silently fails with no user feedback, while the UI may reflect a state (switch toggled, item missing) that the server did not actually apply. The same gap exists in _deleteJob (line 213) and the AdaptiveSwitch toggle (line 138). The _runSessionAction helper in hermes_session_tile.dart shows the pattern to follow: wrap the awaited call in try/catch and surface failures via UiUtils.showMessage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A single PR that adds a direct Hermes Agent backend to Conduit and makes Open WebUI optional, so the app can run with a self-hosted Hermes agent exclusively. (Originally split as #521 + #522; folded into this one giant PR.)
Part 1 — Hermes Agent backend
/v1/runs+ SSE), sessions (/api/sessions/*), scheduled jobs (/api/jobs/*), capabilities/toolsets discovery, and a settings page with a connection test.GET /v1/runs/{id}.Part 2 — Hermes-only mode (OWUI optional)
preferredBackendProvider(router boot-determinism) + derivedhermesOnlyModeProvider(UI gating).api == null; tab + composer + native-settings-sheet gating; a Hermes-branded native profile header; bidirectional "Connect to Open WebUI" switching.Review fixes folded in
Addressed CodeRabbit / Macroscope / Greptile feedback: stream-parser error surfacing (run.failed/response.failed/tool terminal states), session-open / health / jobs / approval guards, SSE whitespace + scoped receive-timeout, transport partial-drop recovery, copyWith secret-clearing, and a stray NUL byte in a sentinel string.
Verification
flutter analyzeclean;flutter test2330 passing (incl. new unit tests for mode derivation, send-guard, Models/defaultModel surfacing, and stream-parser error cases).Notes
Note
Add Hermes Agent backend with Hermes-only mode and onboarding flow
HermesApiServicefor health, models, skills, run creation, and SSE streaming;dispatchHermesRuntransport for streaming chat responses with cancellation and stream-drop recovery; andHermesConfigControllerfor persisting config and secrets via secure storage.preferredBackendProviderand routes directly to chat when setup is complete.HermesApprovalCardfor human-in-the-loop approval gates,HermesSessionsTabsidebar listing,HermesJobsPagefor scheduled agents, andHermesSettingsPagewith connection testing.SseFrameScannerused by both the existing OpenWebUI and new Hermes stream parsers.serverConnectionredirect withbackendChooserfor all new installs without a configured OWUI server, changing the default onboarding path.Macroscope summarized 3aec173.
Greptile Summary
This PR introduces a full Hermes Agent backend integration and a Hermes-only operating mode, allowing Conduit to run without an Open WebUI server. It adds
HermesApiService(runs/sessions/jobs/capabilities), a stream parser over a sharedSseFrameScanner,dispatchHermesRuntransport with SSE stream-drop recovery, Riverpod providers for config/sessions/jobs, and supporting UI (sessions sidebar tab, approval card, jobs page, settings, onboarding chooser).dispatchHermesRuncreates a run, subscribes to SSE events, maps typed events to chat-notifier callbacks, and recovers dropped streams viaGET /v1/runs/{id}polling with a partial-content suffix-append strategy.preferredBackendProvider(shared-prefs backed) short-circuits the router for Hermes-only installs without waiting on async OWUI state;hermesOnlyModeProvidergates UI affordances at runtime.response.failednow surfacesHermesRunError;_isTruthyErrorcorrectly handles numeric0;openHermesSessionsetsisManualModelSelectionProviderbeforeselectedModelProvider; context-menu actions use_runSessionActionfor uniform error surfacing; the approval-gate null-service case returns early instead of no-opping.Confidence Score: 4/5
The core Hermes transport, stream parser, and session management are solid, but two issues need attention before merging: the jobs page silently swallows network failures on edit/delete/create/toggle operations, and hermesOnlyModeProvider can briefly misidentify dual-backend users as Hermes-only during async startup.
The transport layer (run dispatch, SSE parsing, stream-drop recovery, stop path) is correct and well-tested. The outstanding gaps are in the jobs management UI — four async operations (create, edit, delete, setEnabled) all discard their futures and lack try/catch, meaning server-side failures produce no user feedback and potentially leave the UI desynced from server state. Separately, hermesOnlyModeProvider treats the loading state of activeServerProvider as 'no OWUI server', which can cause a visible layout jump for users who have both backends configured.
lib/features/hermes/views/hermes_jobs_page.dart (error handling on all mutation operations) and lib/features/hermes/providers/hermes_providers.dart (hermesOnlyModeProvider loading-state guard)
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant ChatUI participant ChatProviders participant HermesApiService participant HermesServer User->>ChatUI: Send message (Hermes model selected) ChatUI->>ChatProviders: durableSend / sendMessageWithContainer ChatProviders->>ChatProviders: isSendBlocked? (isHermesModel check) ChatProviders->>ChatProviders: _dispatchHermesRunFromChat ChatProviders->>HermesApiService: ensureSessionKey() ChatProviders->>HermesApiService: createSession(title) HermesApiService->>HermesServer: POST /api/sessions HermesServer-->>HermesApiService: "{id: sess-123}" ChatProviders->>HermesApiService: createRun(input, sessionId) HermesApiService->>HermesServer: POST /v1/runs HermesServer-->>HermesApiService: "{run_id: run-456}" ChatProviders->>HermesApiService: runEvents(runId, cancelToken) HermesApiService->>HermesServer: GET /v1/runs/run-456/events (SSE) loop SSE event stream HermesServer-->>HermesApiService: "data: {type, delta/content}" HermesApiService->>ChatProviders: "parseHermesRunFrame -> HermesRunEvent" ChatProviders->>ChatUI: appendContent / appendStatus / updateMessage end HermesServer-->>HermesApiService: data: [DONE] or run.completed ChatProviders->>ChatUI: finishStreaming + completeStreamingUi alt Stream dropped (no terminal event) ChatProviders->>HermesApiService: getRun(runId) up to 4 polls HermesApiService->>HermesServer: GET /v1/runs/run-456 HermesServer-->>HermesApiService: "{status: completed, output: ...}" ChatProviders->>ChatUI: appendContent (recovered suffix) end alt User stops generation User->>ChatUI: Stop button ChatUI->>ChatProviders: stopGenerationProvider ChatProviders->>ChatProviders: registry.cancel(assistantMessageId) ChatProviders->>HermesApiService: stopRun(runId) HermesApiService->>HermesServer: POST /v1/runs/run-456/stop end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant ChatUI participant ChatProviders participant HermesApiService participant HermesServer User->>ChatUI: Send message (Hermes model selected) ChatUI->>ChatProviders: durableSend / sendMessageWithContainer ChatProviders->>ChatProviders: isSendBlocked? (isHermesModel check) ChatProviders->>ChatProviders: _dispatchHermesRunFromChat ChatProviders->>HermesApiService: ensureSessionKey() ChatProviders->>HermesApiService: createSession(title) HermesApiService->>HermesServer: POST /api/sessions HermesServer-->>HermesApiService: "{id: sess-123}" ChatProviders->>HermesApiService: createRun(input, sessionId) HermesApiService->>HermesServer: POST /v1/runs HermesServer-->>HermesApiService: "{run_id: run-456}" ChatProviders->>HermesApiService: runEvents(runId, cancelToken) HermesApiService->>HermesServer: GET /v1/runs/run-456/events (SSE) loop SSE event stream HermesServer-->>HermesApiService: "data: {type, delta/content}" HermesApiService->>ChatProviders: "parseHermesRunFrame -> HermesRunEvent" ChatProviders->>ChatUI: appendContent / appendStatus / updateMessage end HermesServer-->>HermesApiService: data: [DONE] or run.completed ChatProviders->>ChatUI: finishStreaming + completeStreamingUi alt Stream dropped (no terminal event) ChatProviders->>HermesApiService: getRun(runId) up to 4 polls HermesApiService->>HermesServer: GET /v1/runs/run-456 HermesServer-->>HermesApiService: "{status: completed, output: ...}" ChatProviders->>ChatUI: appendContent (recovered suffix) end alt User stops generation User->>ChatUI: Stop button ChatUI->>ChatProviders: stopGenerationProvider ChatProviders->>ChatProviders: registry.cancel(assistantMessageId) ChatProviders->>HermesApiService: stopRun(runId) HermesApiService->>HermesServer: POST /v1/runs/run-456/stop endReviews (5): Last reviewed commit: "feat(hermes): branded native header + Mo..." | Re-trigger Greptile