Use Hermes hook payloads for rich notifications - #4851
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughIntegrates Hermes Codex discovery and provider normalization with agent hooks: groups Hermes hook events, expands CLI hook payload normalization (approval fast-path), makes resume-command pipeline Hermes-aware (env, provider, bootstrap), adjusts sanitizer/environment policy, and adds tests plus localization. ChangesHermes Codex Configuration and Approval Notification Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Greptile SummaryThis PR wires Hermes hook payloads into rich notifications by reading
Confidence Score: 5/5Safe to merge. The approval notification and resume bootstrap paths are well-guarded, idempotent, and covered by integration tests. The env-key scoping fix correctly addresses the prior review concern. All changed paths have test coverage, the bootstrap insertion/removal is idempotent, and the env-leakage fixes are properly gated on agent kind. The two findings are style-level observations about code placement and duplication, not behavioral defects. No files require special attention. The Workspace.swift additions work correctly; the package boundary concern is a future cleanup opportunity rather than a current risk. Important Files Changed
Sequence DiagramsequenceDiagram
participant Hermes as Hermes Agent
participant CLI as cmux CLI hook
participant Store as ClaudeHookSessionStore
participant Surface as cmux Surface
Note over Hermes,Surface: Approval request flow
Hermes->>CLI: pre_approval_request (notification subcommand)
CLI->>Store: update session (needsInput, lastSubtitle, lastBody)
CLI->>Surface: notify_target_async description: command
CLI->>Surface: set_status needs input
Note over Hermes,Surface: Approval response flow
Hermes->>CLI: post_approval_response (approval-response subcommand)
CLI->>Store: markNotificationResolved (running, clear subtitle/body)
CLI->>Surface: clear_notifications
CLI->>Surface: set_status Running
CLI->>Surface: surface.resume.set
Note over Hermes,Surface: Completion notification flow
Hermes->>CLI: post_llm_call (agent-response subcommand)
CLI->>Store: update session (idle)
CLI->>Surface: notify_target_async with assistant_response
CLI->>Surface: set_status Idle
Reviews (16): Last reviewed commit: "docs: document hermes codex environment ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+22363
to
+22381
| private func hermesAgentApprovalNotificationMessage(def: AgentHookDef, object: [String: Any]) -> String? { | ||
| guard def.name == "hermes-agent" else { return nil } | ||
| let event = firstString(in: object, keys: ["hook_event_name", "hookEventName", "event", "event_name"]) | ||
| guard event == "pre_approval_request" else { return nil } | ||
| let extra = (object["extra"] as? [String: Any]) ?? [:] | ||
| let command = firstString(in: extra, keys: ["command"]) | ||
| let description = firstString(in: extra, keys: ["description", "pattern_key", "patternKey"]) | ||
|
|
||
| switch (description, command) { | ||
| case let (description?, command?): | ||
| return "\(description): \(command)" | ||
| case let (description?, nil): | ||
| return description | ||
| case let (nil, command?): | ||
| return command | ||
| default: | ||
| return nil | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
: separator in user-visible notification body
"\(description): \(command)" assembles the notification body with a raw English colon-space separator rather than a localized format string. Every other user-visible format string in this file routes through String.localizedStringWithFormat(String(localized:...)). For locales that place the command before the description, or that use different punctuation, the concatenation order and separator should be in a string-catalog entry (e.g., "%1$@: %2$@" with a defaultValue: and translated variants).
Rule Used: Flag production user-facing text that is not fully... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CLI/cmux.swift`:
- Around line 20751-20765: The compaction loop in CLI/cmux.swift builds
compactExtra from a fixed key list but is missing the new extraction keys, so
fields like "assistantPreamble", "assistant_preamble", and "title" get dropped;
update the key array used in the for-loop that calls compactClaudeHookValue (the
array feeding compactExtra) to include "assistantPreamble",
"assistant_preamble", and "title" so compactExtra preserves those values and
compact["extra"] retains notification/assistant text across persistence.
In
`@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swift`:
- Around line 53-63: The function codexBaseURL(fromChatGPTBaseURL:) currently
does raw string rewriting which can produce invalid URLs (e.g. "https://" →
"https:/codex"); instead parse the input with URLComponents (or URL) after
trimming, verify scheme is "http"/"https" and that host is non-empty, and reject
(return nil) if parsing fails or if query/fragment are present; then normalize
the path by removing trailing slashes, check if the path already ends with
"/codex" and otherwise append "/codex", rebuild a valid URL string from the
components (scheme, host, normalized path) and return it—update references to
trimmed/withoutTrailingSlash logic inside codexBaseURL(fromChatGPTBaseURL:)
accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e399ad8b-45a4-4b4d-a3a6-223c1b0457ea
📒 Files selected for processing (14)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentHookConfig.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentHookConfigTests.swiftSources/HermesAgentIndex.swiftSources/TerminalStartupEnvironment.swiftcmuxTests/CLIGenericHookPersistenceTests.swiftcmuxTests/GhosttyTerminalStartupEnvironmentTests.swiftcmuxTests/RestorableAgentHookProviderHermesTests.swift
Comment on lines
+83
to
+86
| merged = HermesAgentCodexEnvironment.applyingDefaultCodexBaseURL( | ||
| to: merged, | ||
| ambientEnvironment: ambientEnvironment | ||
| ) |
Contributor
There was a problem hiding this comment.
CUSTOM_BASE_URL injected into every terminal's startup environment
HermesAgentCodexEnvironment.applyingDefaultCodexBaseURL is now called unconditionally in mergedStartupEnvironment, which runs for every terminal surface — not just Hermes terminals. When a user has a Codex config with a non-public openai_base_url (e.g. an internal subrouter), CUSTOM_BASE_URL is silently set in the initial shell environment of Claude Code, Copilot, OpenCode, and every other agent launched via cmux.
Compounding this, CUSTOM_BASE_URL was added to AgentLaunchEnvironmentPolicy.safeEnvironmentKeys without any agent-kind guard, so selectedAgentLaunchEnvironment will capture and persist it for all agents' resume records — not just Hermes. Any agent that reads CUSTOM_BASE_URL from its environment will be rerouted to the Codex subrouter. The launch-capture path in cmux.swift is correctly gated on kind == "hermes-agent", so the fix here should mirror that pattern: only inject from mergedStartupEnvironment (or guard CUSTOM_BASE_URL in safeEnvironmentKeys) when the surface is Hermes-specific.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swift (1)
116-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
chatgpt_base_urlbefore appending/codex.Raw prefix checks plus string concatenation still turn malformed values like
https://orhttps://host?x=1into invalidHERMES_CODEX_BASE_URLvalues. This helper is the normalization boundary, so it should reject anything without a real host and only mutate a parsed path.Suggested fix
public static func codexBaseURL(fromChatGPTBaseURL rawValue: String) -> String? { let trimmed = rawValue.trimmingCharacters(in: .whitespacesAndNewlines) - guard trimmed.hasPrefix("http://") || trimmed.hasPrefix("https://") else { + guard var components = URLComponents(string: trimmed), + let scheme = components.scheme?.lowercased(), + ["http", "https"].contains(scheme), + components.host?.isEmpty == false, + components.query == nil, + components.fragment == nil else { return nil } - let withoutTrailingSlash = trimmed.replacingOccurrences(of: "/+$", with: "", options: .regularExpression) - guard !withoutTrailingSlash.isEmpty else { return nil } - if withoutTrailingSlash.lowercased().hasSuffix("/codex") { - return withoutTrailingSlash - } - return withoutTrailingSlash + "/codex" + let normalizedPath = components.path + .replacingOccurrences(of: "/+$", with: "", options: .regularExpression) + components.path = normalizedPath.lowercased().hasSuffix("/codex") + ? normalizedPath + : normalizedPath + "/codex" + return components.string }🤖 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 `@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swift` around lines 116 - 126, The codexBaseURL(fromChatGPTBaseURL:) helper currently only trims and checks prefixes then appends "/codex", which accepts malformed inputs like "https://" or "https://host?x=1"; instead parse the trimmed string into a URL or URLComponents, verify scheme is "http" or "https" and that host is non-empty (and reject if host missing), strip trailing slashes from the path portion only, preserve any valid path, ensure there is no query/fragment (or reject if present per caller expectations), and then if the resulting path does not already end with "/codex" append "/codex" and return the reconstructed URL string; locate and update codexBaseURL(fromChatGPTBaseURL:) to use URL/URLComponents validation and reconstruction rather than raw string prefix checks and concatenation.
🤖 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 `@CLI/cmux.swift`:
- Around line 23339-23340: The provider-rewrite is happening after the command
is fully shell-quoted, causing accidental rewrites inside user payloads; change
hermesAgentCommandByReplacingOpenAICodexProvider to operate on the argv token
array before calling cliShellQuote (i.e. find/replace any "--provider
openai-codex" token or the "--provider" token followed by "openai-codex" and
replace just those tokens), and update any checks that use
contains("model.api_mode") to inspect the argv tokens instead of the joined
command string (apply same fix where hermesAgentSubrouterResumeCommand
constructs argv and where similar logic appears around lines referenced in the
comment). Ensure only provider flag tokens are mutated, not arbitrary substrings
of other arguments.
In
`@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swift`:
- Around line 129-137: customBaseURL currently forwards malformed inputs like
"https://" or "https://host?x=1"; update the validation in
customBaseURL(fromOpenAIBaseURL:) to parse the trimmed string with URL(string:)
and reject any URL that lacks a non-empty host or that contains query or
fragment components (i.e. require url.scheme == "http" or "https", url.host !=
nil, url.query == nil, url.fragment == nil), then continue to strip trailing
slashes and run the existing hostMatches check (hostMatches(..., hostSuffix:
"api.openai.com")); return nil on any of these failures so only well-formed base
URLs are returned.
In `@Sources/Workspace.swift`:
- Around line 855-913: The Hermes resume-rewrite logic
(hermesAgentSubrouterBindingForStartup,
hermesAgentCommandByReplacingOpenAICodexProvider, normalizedSurfaceResumeValue,
surfaceResumeShellQuote) should be extracted out of Workspace.swift into the
Hermes launch utilities module so it can be unit-tested independently; move
these functions into a new utility file/class (e.g., HermesLaunchUtils or
HermesAgentCodexHelpers), keep their visibility appropriate (nonisolated/private
-> internal or fileprivate as needed for tests), update Workspace.swift to call
the new helpers, and ensure any referenced types like
HermesAgentCodexEnvironment and SurfaceResumeBindingSnapshot are
imported/visible to the new file and covered by unit tests.
- Around line 863-873: The normalized base URL returned by
normalizedSurfaceResumeValue is not written back into the environment, so
surrounding whitespace can still persist; update the environment dictionary at
HermesAgentCodexEnvironment.customBaseURLEnvironmentKey with the
trimmed/normalized string before assigning result.environment and returning
binding, i.e. capture the normalized value, set
environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] =
normalizedValue (or remove the key if normalizedValue is nil/empty) and then
proceed to set result.environment and result.command using
hermesAgentCommandByReplacingOpenAICodexProvider.
---
Duplicate comments:
In
`@Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swift`:
- Around line 116-126: The codexBaseURL(fromChatGPTBaseURL:) helper currently
only trims and checks prefixes then appends "/codex", which accepts malformed
inputs like "https://" or "https://host?x=1"; instead parse the trimmed string
into a URL or URLComponents, verify scheme is "http" or "https" and that host is
non-empty (and reject if host missing), strip trailing slashes from the path
portion only, preserve any valid path, ensure there is no query/fragment (or
reject if present per caller expectations), and then if the resulting path does
not already end with "/codex" append "/codex" and return the reconstructed URL
string; locate and update codexBaseURL(fromChatGPTBaseURL:) to use
URL/URLComponents validation and reconstruction rather than raw string prefix
checks and concatenation.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 461cdce3-ead4-49d8-b9d1-2a031b9843f5
📒 Files selected for processing (9)
CLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swiftSources/Workspace.swiftcmuxTests/GhosttyTerminalStartupEnvironmentTests.swiftcmuxTests/RestorableAgentHookProviderHermesTests.swiftcmuxTests/SessionPersistenceTests.swift
Comment on lines
+855
to
913
| nonisolated private static func hermesAgentSubrouterBindingForStartup( | ||
| _ binding: SurfaceResumeBindingSnapshot | ||
| ) -> SurfaceResumeBindingSnapshot { | ||
| guard binding.source == "agent-hook", | ||
| binding.kind == "hermes-agent" else { | ||
| return binding | ||
| } | ||
|
|
||
| var environment = binding.environment ?? [:] | ||
| environment = HermesAgentCodexEnvironment.applyingDefaultCodexBaseURL(to: environment) | ||
| guard let baseURL = normalizedSurfaceResumeValue( | ||
| environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] | ||
| ) else { | ||
| return binding | ||
| } | ||
|
|
||
| var result = binding | ||
| result.environment = environment.isEmpty ? nil : environment | ||
| result.command = hermesAgentCommandByReplacingOpenAICodexProvider(result.command) | ||
| guard !result.command.contains("model.api_mode") else { | ||
| return result | ||
| } | ||
|
|
||
| var bootstrap = [ | ||
| "hermes config set model.provider \(surfaceResumeShellQuote(HermesAgentCodexEnvironment.defaultProvider)) >/dev/null", | ||
| "hermes config set model.base_url \(surfaceResumeShellQuote(baseURL)) >/dev/null", | ||
| "hermes config set model.api_mode \(surfaceResumeShellQuote(HermesAgentCodexEnvironment.codexResponsesAPIMode)) >/dev/null" | ||
| ] | ||
| if let model = HermesAgentCodexEnvironment.defaultCodexModel(environment: environment) { | ||
| bootstrap.append("hermes config set model.default \(surfaceResumeShellQuote(model)) >/dev/null") | ||
| } | ||
| result.command = bootstrap.joined(separator: " && ") + " && " + result.command | ||
| return result | ||
| } | ||
|
|
||
| nonisolated private static func hermesAgentCommandByReplacingOpenAICodexProvider(_ command: String) -> String { | ||
| var result = command | ||
| let replacements = [ | ||
| ("'--provider' 'openai-codex'", "'--provider' 'custom'"), | ||
| ("\"--provider\" \"openai-codex\"", "\"--provider\" \"custom\""), | ||
| ("--provider openai-codex", "--provider custom"), | ||
| ("'--provider=openai-codex'", "'--provider=custom'"), | ||
| ("\"--provider=openai-codex\"", "\"--provider=custom\""), | ||
| ("--provider=openai-codex", "--provider=custom") | ||
| ] | ||
| for (old, new) in replacements { | ||
| result = result.replacingOccurrences(of: old, with: new) | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| nonisolated private static func normalizedSurfaceResumeValue(_ value: String?) -> String? { | ||
| let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return trimmed?.isEmpty == false ? trimmed : nil | ||
| } | ||
|
|
||
| nonisolated private static func surfaceResumeShellQuote(_ value: String) -> String { | ||
| "'" + value.replacingOccurrences(of: "'", with: "'\\''") + "'" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move this Hermes resume-rewrite logic out of Workspace.swift.
This block is pure command/env transformation and shell quoting, so it does not need to live in the app target’s root Sources/ path. Please move it alongside the Hermes launch utilities so it can be unit-tested without Workspace/UI coupling.
As per coding guidelines, "Do not implement feature logic directly in the app target/module's root Sources/ path when the logic is independent of cmux app lifecycle and can compile/test without AppKit, SwiftUI view state, Ghostty globals, or process-wide singletons."
🤖 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 `@Sources/Workspace.swift` around lines 855 - 913, The Hermes resume-rewrite
logic (hermesAgentSubrouterBindingForStartup,
hermesAgentCommandByReplacingOpenAICodexProvider, normalizedSurfaceResumeValue,
surfaceResumeShellQuote) should be extracted out of Workspace.swift into the
Hermes launch utilities module so it can be unit-tested independently; move
these functions into a new utility file/class (e.g., HermesLaunchUtils or
HermesAgentCodexHelpers), keep their visibility appropriate (nonisolated/private
-> internal or fileprivate as needed for tests), update Workspace.swift to call
the new helpers, and ensure any referenced types like
HermesAgentCodexEnvironment and SurfaceResumeBindingSnapshot are
imported/visible to the new file and covered by unit tests.
Comment on lines
+863
to
+873
| var environment = binding.environment ?? [:] | ||
| environment = HermesAgentCodexEnvironment.applyingDefaultCodexBaseURL(to: environment) | ||
| guard let baseURL = normalizedSurfaceResumeValue( | ||
| environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] | ||
| ) else { | ||
| return binding | ||
| } | ||
|
|
||
| var result = binding | ||
| result.environment = environment.isEmpty ? nil : environment | ||
| result.command = hermesAgentCommandByReplacingOpenAICodexProvider(result.command) |
There was a problem hiding this comment.
Persist the normalized base URL back into environment.
normalizedSurfaceResumeValue(...) trims the URL for the bootstrap commands, but result.environment still carries the original untrimmed value. If Hermes reads the env var during resume, a value with surrounding whitespace can still override the corrected config.
💡 Suggested fix
guard let baseURL = normalizedSurfaceResumeValue(
environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey]
) else {
return binding
}
+ environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] = baseURL
var result = binding
result.environment = environment.isEmpty ? nil : environment📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var environment = binding.environment ?? [:] | |
| environment = HermesAgentCodexEnvironment.applyingDefaultCodexBaseURL(to: environment) | |
| guard let baseURL = normalizedSurfaceResumeValue( | |
| environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] | |
| ) else { | |
| return binding | |
| } | |
| var result = binding | |
| result.environment = environment.isEmpty ? nil : environment | |
| result.command = hermesAgentCommandByReplacingOpenAICodexProvider(result.command) | |
| var environment = binding.environment ?? [:] | |
| environment = HermesAgentCodexEnvironment.applyingDefaultCodexBaseURL(to: environment) | |
| guard let baseURL = normalizedSurfaceResumeValue( | |
| environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] | |
| ) else { | |
| return binding | |
| } | |
| environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] = baseURL | |
| var result = binding | |
| result.environment = environment.isEmpty ? nil : environment | |
| result.command = hermesAgentCommandByReplacingOpenAICodexProvider(result.command) |
🤖 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 `@Sources/Workspace.swift` around lines 863 - 873, The normalized base URL
returned by normalizedSurfaceResumeValue is not written back into the
environment, so surrounding whitespace can still persist; update the environment
dictionary at HermesAgentCodexEnvironment.customBaseURLEnvironmentKey with the
trimmed/normalized string before assigning result.environment and returning
binding, i.e. capture the normalized value, set
environment[HermesAgentCodexEnvironment.customBaseURLEnvironmentKey] =
normalizedValue (or remove the key if normalizedValue is nil/empty) and then
proceed to set result.environment and result.command using
hermesAgentCommandByReplacingOpenAICodexProvider.
…fications # Conflicts: # CLI/cmux.swift # Sources/Workspace.swift # cmuxTests/RestorableAgentHookProviderHermesTests.swift
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0784208. Configure here.
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
`@Packages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swift`:
- Around line 118-126: The assertion that
AgentLaunchEnvironmentPolicy.selectedEnvironment(..., kind: "codex").isEmpty
should be split into its own unit test: keep the existing test that verifies
allowed Hermes Codex URLs focused on the positive case (allowing URLs) and move
the blocking assertion into a new test named e.g.
testBlockingHermesCodexEnvironment_whenCustomBaseUrlConflicts; locate the call
to AgentLaunchEnvironmentPolicy.selectedEnvironment and the `#expect`(...)
assertion in HermesAgentCodexEnvironmentTests.swift, create a separate test
function that only checks the .isEmpty result for kind: "codex", and ensure each
test has a clear descriptive name and asserts a single scenario.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81c188f2-4ebb-4c18-afc1-10221118720a
📒 Files selected for processing (8)
CLI/CMUXCLI+AgentHookDefinitions.swiftCLI/cmux.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchSanitizer.swiftPackages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/HermesAgentCodexEnvironment.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/AgentLaunchSanitizerTests.swiftPackages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swiftResources/Localizable.xcstrings
💤 Files with no reviewable changes (1)
- Resources/Localizable.xcstrings
Comment on lines
+118
to
+126
| #expect( | ||
| AgentLaunchEnvironmentPolicy.selectedEnvironment( | ||
| from: [ | ||
| "CUSTOM_BASE_URL": "http://subrouter-team:31415/v1", | ||
| "HERMES_CODEX_BASE_URL": "http://subrouter-team:31415/backend-api/codex", | ||
| ], | ||
| kind: "codex" | ||
| ).isEmpty | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider splitting this assertion into a separate test.
The test name describes allowing URLs for Hermes Codex, but this assertion verifies the opposite behavior (blocking for kind: "codex"). Splitting into a separate test would improve clarity and follow the one-scenario-per-test pattern.
♻️ Proposed refactor
`@Test`("Allows Hermes Codex subrouter URLs in captured launch environment")
func allowsHermesCodexSubrouterURLsInCapturedLaunchEnvironment() {
`#expect`(
AgentLaunchEnvironmentPolicy.selectedEnvironment(
from: [
"CUSTOM_BASE_URL": "http://subrouter-team:31415/v1",
"HERMES_CODEX_BASE_URL": "http://subrouter-team:31415/backend-api/codex",
],
kind: "hermes-agent"
) == [
"CUSTOM_BASE_URL": "http://subrouter-team:31415/v1",
"HERMES_CODEX_BASE_URL": "http://subrouter-team:31415/backend-api/codex",
]
)
+ }
+
+ `@Test`("Filters Hermes Codex URLs for non-Hermes agent kinds")
+ func filtersHermesCodexURLsForNonHermesAgentKinds() {
`#expect`(
AgentLaunchEnvironmentPolicy.selectedEnvironment(
from: [
"CUSTOM_BASE_URL": "http://subrouter-team:31415/v1",
"HERMES_CODEX_BASE_URL": "http://subrouter-team:31415/backend-api/codex",
],
kind: "codex"
).isEmpty
)
}
-}🤖 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
`@Packages/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/HermesAgentCodexEnvironmentTests.swift`
around lines 118 - 126, The assertion that
AgentLaunchEnvironmentPolicy.selectedEnvironment(..., kind: "codex").isEmpty
should be split into its own unit test: keep the existing test that verifies
allowed Hermes Codex URLs focused on the positive case (allowing URLs) and move
the blocking assertion into a new test named e.g.
testBlockingHermesCodexEnvironment_whenCustomBaseUrlConflicts; locate the call
to AgentLaunchEnvironmentPolicy.selectedEnvironment and the `#expect`(...)
assertion in HermesAgentCodexEnvironmentTests.swift, create a separate test
function that only checks the .isEmpty result for kind: "codex", and ensure each
test has a clear descriptive name and asserts a single scenario.
This was referenced Jun 5, 2026
This was referenced Jun 16, 2026
This branch was successfully deployed
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
Verification
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches agent hook handling, session store, resume shell commands, and environment allowlisting; mistakes could mis-notify users or break Hermes resume against custom Codex endpoints.
Overview
Hermes hook integration now surfaces richer UI notifications and approval lifecycle handling. Completion messages read
extra.assistant_response(and related fields);pre_approval_requestmaps to notification hooks with localized description + command text; a newapproval-responsepath clears stored notification state, restores Running status, and republishes resume bindings.Hermes + Codex subrouter resume is added via
HermesAgentCodexEnvironment: deriveHERMES_CODEX_BASE_URL/CUSTOM_BASE_URLfrom Codexconfig.toml, allowlist them only forhermes-agent, rewrite staleopenai-codextocustom, and prepend guardedhermes config setbootstrap on hook/surface resume when appropriate. Hook YAML install coalesces multiple cmux hooks under one event name; feed telemetry is suppressed for Hermes events that already have dedicated feed hooks to avoid duplicates.Reviewed by Cursor Bugbot for commit e8e1613. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Richer Hermes notifications now read hook
extrapayloads and show clear approval prompts. Hermes resumes with Codex subrouter defaults via a guarded bootstrap that preserves explicit provider settings, and the Codex environment API is documented.New Features
extra.assistant_response; render approvals as "description: command"; addapproval-responseto clear prompts and restore Running.HERMES_CODEX_BASE_URL/CUSTOM_BASE_URL; prepend guardedhermes config seton resume; rewriteopenai-codextocustom; coalesce multiple hooks under one Hermes event.Bug Fixes
model.api_modeis set or provider isn’tcustom/openai-codex; dedupe and handle malformed commands; keep explicit subrouter URLs; keep bootstrap behind a CWD guard.--provider; scope env capture to Hermes; generic terminals don’t derive Codex defaults.extrafields; suppress duplicate feed events when Hermes notifications also install feed hooks.Written for commit e8e1613. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation