Skip to content

fix(adhoc-sweep-fixes): CU-17tkuw5uezv 2 review findings across 2 files #117

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

Draft
flamingo wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix(adhoc-sweep-fixes): CU-17tkuw5uezv 2 review findings across 2 files #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7396cec
fix(adhoc-sweep-fixes): 2 review findings across 2 files
flamingo[bot] Oct 5, 2026
6942ae4
fix(adhoc-sweep-fixes): 2 review findings across 2 files
flamingo[bot] Oct 5, 2026
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 codewiki/src/be/agent_tools/str_replace_editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ def flake8(file_path: str) -> str:
"""Run flake8 on a given file and return the output as a string"""
if Path(file_path).suffix != ".py":
return ""
cmd = "flake8 --isolated --select=F821,F822,F831,E111,E112,E113,E999,E902 {file_path}"
cmd = ["flake8", "--isolated", "--select=F821,F822,F831,E111,E112,E113,E999,E902", file_path]
# don't use capture_output because it's not compatible with python3.6
out = subprocess.run(cmd.format(file_path=file_path), shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
out = subprocess.run(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
return out.stdout.decode()


Comment on lines 188 to 196

@flamingo flamingo Bot Oct 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 flake8() invokes subprocess.run with shell=True and unsanitized file_path interpolation

In flake8(), replaced the shell-interpolated string command (cmd.format(file_path=file_path) run with shell=True) with a list-of-args command (["flake8", "--isolated", "--select=...", file_path]) run with shell=False. This eliminates the command injection vector since file_path is now passed as a single discrete argv element rather than being interpolated into a shell-parsed string, matching the suggested fix exactly. The unrelated view() method's subprocess.run(..., shell=True) for find was left untouched since it was not part of this finding's evidence/scope.

🤖 Prompt for AI agents 
In codewiki/src/be/agent_tools/str_replace_editor.py around line 187, review and complete this code-review fix: flake8() invokes subprocess.run with shell=True and unsanitized file_path interpolation.
What the draft fix changed: In `flake8()`, replaced the shell-interpolated string command (`cmd.format(file_path=file_path)` run with `shell=True`) with a list-of-args command (`["flake8", "--isolated", "--select=...", file_path]`) run with `shell=False`. This eliminates the command injection vector since `file_path` is now passed as a single discrete argv element rather than being interpolated into a shell-parsed string, matching the suggested fix exactly. The unrelated `view()` method's `subprocess.run(..., shell=True)` for `find` was left untouched since it was not part of this finding's evidence/scope.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer

Expand Down
108 changes: 60 additions & 48 deletions codewiki/src/be/llm_services.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
logger = logging.getLogger(__name__)
from pydantic_ai.providers.openai import OpenAIProvider
from pydantic_ai.models.fallback import FallbackModel
from openai import OpenAI, OpenAIError
from openai import OpenAI, AsyncOpenAI, OpenAIError

from codewiki.src.config import Config

Expand Down Expand Up @@ -115,12 +115,6 @@ def create_main_model(config: Config) -> OpenAIModel:
"Or in config file: main_base_url = '<url>'"
)

# Prepare default headers for API version (Anthropic models)

@flamingo flamingo Bot Oct 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🟠 create_main_model / create_fallback_model build unused default_headers dict and never attach it to the provider

In create_main_model, create_fallback_model, and create_cluster_model the previously-dead default_headers dict is now actually used: when *_api_version is set, an AsyncOpenAI client is constructed with default_headers={'anthropic-version': api_version} and passed to OpenAIProvider(openai_client=...), per the comment that was already in the code; otherwise the provider is built with base_url=/api_key= as before. Added AsyncOpenAI to the existing from openai import ... line (module already exists as a dependency, no new import added). Unverified: exact OpenAIProvider(openai_client=...) signature/behavior across pydantic-ai versions beyond what the existing comment asserted; if openai_client= also requires/forbids other params in some pydantic-ai release, this could need adjustment, but it directly implements the fix the comment itself specified.

🤖 Prompt for AI agents 
In codewiki/src/be/llm_services.py around line 118, review and complete this code-review fix: create_main_model / create_fallback_model build unused default_headers dict and never attach it to the provider.
What the draft fix changed: In `create_main_model`, `create_fallback_model`, and `create_cluster_model` the previously-dead `default_headers` dict is now actually used: when `*_api_version` is set, an `AsyncOpenAI` client is constructed with `default_headers={'anthropic-version': api_version}` and passed to `OpenAIProvider(openai_client=...)`, per the comment that was already in the code; otherwise the provider is built with `base_url=`/`api_key=` as before. Added `AsyncOpenAI` to the existing `from openai import ...` line (module already exists as a dependency, no new import added). Unverified: exact `OpenAIProvider(openai_client=...)` signature/behavior across pydantic-ai versions beyond what the existing comment asserted; if `openai_client=` also requires/forbids other params in some pydantic-ai release, this could need adjustment, but it directly implements the fix the comment itself specified.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 78 medium — react 👍/👎 to teach the reviewer

default_headers = {}
api_version = getattr(config, 'main_api_version', None)
if api_version:
default_headers['anthropic-version'] = api_version

# Get per-provider API key
api_key = getattr(config, 'main_api_key', None)
if not api_key:
Expand All @@ -131,18 +125,28 @@ def create_main_model(config: Config) -> OpenAIModel:
"Different AI providers require different API keys."
)

return OpenAIModel(
model_name=config.main_model,
provider=OpenAIProvider(
# Prepare default headers for API version (Anthropic models). pydantic-ai's
# OpenAIProvider does not accept a default_headers parameter directly, so
# when an api_version is configured we build an AsyncOpenAI client with
# the header set and pass it via openai_client= instead.
api_version = getattr(config, 'main_api_version', None)
if api_version:
default_headers = {'anthropic-version': api_version}
openai_client = AsyncOpenAI(
base_url=base_url,
api_key=api_key,
default_headers=default_headers,
)
provider = OpenAIProvider(openai_client=openai_client)
else:
provider = OpenAIProvider(
base_url=base_url,
api_key=api_key,
# NOTE: pydantic-ai's OpenAIProvider takes only base_url, api_key,
# openai_client and http_client - there is no default_headers
# parameter (verified against pydantic-ai 2.40.0), so passing one
# raises TypeError. To send anthropic-version here, build an
# AsyncOpenAI client with default_headers and pass it as
# openai_client=.
),
)

return OpenAIModel(
model_name=config.main_model,
provider=provider,
settings=OpenAIModelSettings(**settings_dict)
)

Expand Down Expand Up @@ -170,12 +174,6 @@ def create_fallback_model(config: Config) -> OpenAIModel:
"Or in config file: fallback_base_url = '<url>'"
)

# Prepare default headers for API version (Anthropic models)
default_headers = {}
api_version = getattr(config, 'fallback_api_version', None)
if api_version:
default_headers['anthropic-version'] = api_version

# Get per-provider API key
api_key = getattr(config, 'fallback_api_key', None)
if not api_key:
Expand All @@ -186,18 +184,28 @@ def create_fallback_model(config: Config) -> OpenAIModel:
"Different AI providers require different API keys."
)

return OpenAIModel(
model_name=config.fallback_model,
provider=OpenAIProvider(
# Prepare default headers for API version (Anthropic models). pydantic-ai's
# OpenAIProvider does not accept a default_headers parameter directly, so
# when an api_version is configured we build an AsyncOpenAI client with
# the header set and pass it via openai_client= instead.
api_version = getattr(config, 'fallback_api_version', None)
if api_version:
default_headers = {'anthropic-version': api_version}
openai_client = AsyncOpenAI(
base_url=base_url,
api_key=api_key,
default_headers=default_headers,
)
provider = OpenAIProvider(openai_client=openai_client)
else:
provider = OpenAIProvider(
base_url=base_url,
api_key=api_key,
# NOTE: pydantic-ai's OpenAIProvider takes only base_url, api_key,
# openai_client and http_client - there is no default_headers
# parameter (verified against pydantic-ai 2.40.0), so passing one
# raises TypeError. To send anthropic-version here, build an
# AsyncOpenAI client with default_headers and pass it as
# openai_client=.
),
)

return OpenAIModel(
model_name=config.fallback_model,
provider=provider,
settings=OpenAIModelSettings(**settings_dict)
)

Expand Down Expand Up @@ -240,11 +248,6 @@ def create_cluster_model(config: Config) -> OpenAIModel:
if temperature_supported:
settings_dict['temperature'] = temperature

# Prepare default headers for API version (Anthropic models)
default_headers = {}
if api_version:
default_headers['anthropic-version'] = api_version

# Get per-provider API key
api_key = getattr(config, 'cluster_api_key', None)
if not api_key:
Expand All @@ -255,18 +258,27 @@ def create_cluster_model(config: Config) -> OpenAIModel:
"Different AI providers require different API keys."
)

return OpenAIModel(
model_name=config.cluster_model,
provider=OpenAIProvider(
# Prepare default headers for API version (Anthropic models). pydantic-ai's
# OpenAIProvider does not accept a default_headers parameter directly, so
# when an api_version is configured we build an AsyncOpenAI client with
# the header set and pass it via openai_client= instead.
if api_version:
default_headers = {'anthropic-version': api_version}
openai_client = AsyncOpenAI(
base_url=base_url,
api_key=api_key,
default_headers=default_headers,
)
provider = OpenAIProvider(openai_client=openai_client)
else:
provider = OpenAIProvider(
base_url=base_url,
api_key=api_key,
# NOTE: pydantic-ai's OpenAIProvider takes only base_url, api_key,
# openai_client and http_client - there is no default_headers
# parameter (verified against pydantic-ai 2.40.0), so passing one
# raises TypeError. To send anthropic-version here, build an
# AsyncOpenAI client with default_headers and pass it as
# openai_client=.
),
)

return OpenAIModel(
model_name=config.cluster_model,
provider=provider,
settings=OpenAIModelSettings(**settings_dict)
)

Expand Down