Skip to content

Fix OpenAI connector health check configuration #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Fix OpenAI connector health check configuration #648

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/connectors/openai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ def __init__(
"DISABLE_HEALTH_CHECKS", "false"
).lower() in ("true", "1", "yes")

disable_health_checks_config = self.config.get(
"session.dangerous_command_prevention_enabled", False
disable_health_checks_config = bool(
getattr(self.config, "disable_health_checks", False)
)
Comment on lines +74 to 76
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Good change to use a dedicated health check flag, but verify the attribute exists on AppConfig.

The shift from session.dangerous_command_prevention_enabled to a top-level disable_health_checks attribute is architecturally sound and aligns with the PR objective of using a dedicated configuration flag for health checks.

However, please verify that:

  1. The disable_health_checks attribute is actually defined on the AppConfig class
  2. No other code still references session.dangerous_command_prevention_enabled for health check purposes
  3. Tests are updated to use the new configuration attribute

Run the following verification scripts:

🏁 Script executed:

#!/bin/bash
# Description: Verify that disable_health_checks attribute exists on AppConfig class

# Find the AppConfig class definition and check for disable_health_checks attribute
echo "=== Searching for AppConfig class definition and disable_health_checks attribute ==="
ast-grep --pattern $'class AppConfig$$$:
  $$$
  disable_health_checks$$$
  $$$'

# Alternative: search for the attribute definition in config files
echo -e "\n=== Searching for disable_health_checks in config-related files ==="
rg -n --type=py -C5 'disable_health_checks' -g 'src/core/config/**'

Length of output: 1936

🏁 Script executed:

#!/bin/bash
# Description: Check for any remaining references to the old config key

# Search for any code still using session.dangerous_command_prevention_enabled
echo "=== Searching for old config key references ==="
rg -n --type=py 'dangerous_command_prevention_enabled' -C3

# Check if this key is still being set or read elsewhere
echo -e "\n=== Checking for session config with dangerous_command_prevention ==="
rg -n --type=py 'SessionConfig.*dangerous' -C3

Length of output: 3619

Update remaining references to session.dangerous_command_prevention_enabled

  • Remove dangerous_command_prevention_enabled field and its env-var mapping in src/core/config/app_config.py.
  • In src/core/persistence.py::_should_raise_strict_errors, replace config.get("session.dangerous_command_prevention_enabled") with config.disable_health_checks.
  • In src/core/di/services.py (around line 1052), use not app_config.disable_health_checks instead of app_config.session.dangerous_command_prevention_enabled.
  • In tests/utils/app_builder.py, set AppConfig.disable_health_checks rather than the old session flag.
🤖 Prompt for AI Agents 
In src/connectors/openai.py around lines 74-76, replace the getattr(self.config,
"disable_health_checks", False) pattern with direct use of the new boolean
attribute config.disable_health_checks (rename local variable to something like
disable_health_checks) and use that value where needed; additionally, remove any
remaining references to session.dangerous_command_prevention_enabled across the
codebase: delete the field and its env-var mapping in
src/core/config/app_config.py, update
src/core/persistence.py::_should_raise_strict_errors to use
config.disable_health_checks instead of
config.get("session.dangerous_command_prevention_enabled"), change the check in
src/core/di/services.py (around line 1052) to use not
app_config.disable_health_checks in place of
app_config.session.dangerous_command_prevention_enabled, and update
tests/utils/app_builder.py to set AppConfig.disable_health_checks rather than
the old session flag.


# Enable health checks only when neither config nor env disable them
Expand Down
Loading