Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
13 changes: 13 additions & 0 deletions server/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

{
"claude_allowed_tools": [
"Bash(*)",
"Read",
"Write",
"Edit",
"Glob",
"Grep",
"WebFetch",
"Task"
]
}
3 changes: 3 additions & 0 deletions server/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fastapi>=0.115.0
uvicorn>=0.32.0
claude-agent-sdk>=0.1.0
125 changes: 125 additions & 0 deletions server/server.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
"""
FastAPI server that exposes the /oape:api-implement Claude Code skill
via the Claude Agent SDK.

Usage:
uvicorn api.server:app --reload
Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Correct the module path in the usage snippet.

Line 6 points to uvicorn api.server:app, but the module path here is server.server. This will fail unless an api package exists.

🛠️ Proposed fix
-    uvicorn api.server:app --reload
+    uvicorn server.server:app --reload
🤖 Prompt for AI Agents
In `@server/server.py` around lines 5 - 6, The usage snippet references the wrong
module path; replace "uvicorn api.server:app --reload" with the correct module
path "uvicorn server.server:app --reload" (or update to whichever importable
module exposes the FastAPI app) and ensure any other docs or comments that
mention "api.server" are updated to "server.server" so the uvicorn command
imports the app correctly.


Endpoint:
GET /api-implement?ep_url=<enhancement-pr-url>&cwd=<operator-repo-path>
"""

import os
import re
import json
from pathlib import Path

import anyio
from fastapi import FastAPI, HTTPException, Query
from claude_agent_sdk import (
query,
ClaudeAgentOptions,
AssistantMessage,
ResultMessage,
TextBlock,
)


with open("config.json") as cf:
config_json_str = cf.read()
CONFIGS = json.loads(config_json_str)
Comment on lines +28 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Config file path is relative to CWD, not the module location.

If the server is started from a directory other than server/, this will fail to find config.json. The docstring suggests running uvicorn api.server:app which would typically be run from the repo root.

🐛 Proposed fix to resolve path relative to module
-with open("config.json") as cf:
+with open(Path(__file__).parent / "config.json") as cf:
     config_json_str = cf.read()
 CONFIGS = json.loads(config_json_str)
🤖 Prompt for AI Agents
In `@server/server.py` around lines 28 - 30, The current open("config.json") call
loads CONFIGS from a path relative to the CWD which breaks when the server is
started from the repo root; change the file lookup in server.py to resolve the
config.json path relative to the module file (use __file__ or
pathlib.Path(__file__).resolve().parent) before opening so CONFIGS and
config_json_str always read the module-local config.json regardless of working
directory.



app = FastAPI(
title="OAPE Operator Feature Developer",
description="Invokes the /oape:api-implement Claude Code command to generate "
"controller/reconciler code from an OpenShift enhancement proposal.",
version="0.1.0",
)

EP_URL_PATTERN = re.compile(
r"^https://github\.com/openshift/enhancements/pull/\d+/?$"
)

# Resolve the plugin directory (repo root) relative to this file.
# The SDK expects the path to the plugin root (containing .claude-plugin/).
PLUGIN_DIR = str(Path(__file__).resolve().parent.parent / "plugins" / "oape")
print(PLUGIN_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove debug print statement.

print(PLUGIN_DIR) will output to stdout on every module import. This should be removed or converted to a debug-level log statement.

🧹 Proposed fix
 PLUGIN_DIR = str(Path(__file__).resolve().parent.parent / "plugins" / "oape")
-print(PLUGIN_DIR)
+# Optionally log at debug level:
+# logging.getLogger(__name__).debug("PLUGIN_DIR=%s", PLUGIN_DIR)
🤖 Prompt for AI Agents
In `@server/server.py` at line 47, Remove the stray debug print by deleting the
print(PLUGIN_DIR) call in server.py (it runs on every import); if you need that
info, replace it with a logger.debug call instead (use the module logger or
obtain one via logging.getLogger(__name__)) so it no longer writes to stdout at
import time.



@app.get("/api-implement")
async def api_implement(
ep_url: str = Query(
...,
description="GitHub PR URL for the OpenShift enhancement proposal "
"(e.g. https://github.com/openshift/enhancements/pull/1234)",
),
cwd: str = Query(
default="",
description="Absolute path to the operator repository where code "
"will be generated. Defaults to the current working directory.",
),
):
"""Generate controller/reconciler code from an enhancement proposal."""

# --- Validate EP URL ---
if not EP_URL_PATTERN.match(ep_url.rstrip("/")):
raise HTTPException(
status_code=400,
detail=(
"Invalid enhancement PR URL. "
"Expected format: https://github.com/openshift/enhancements/pull/<number>"
),
)

# --- Resolve working directory ---
working_dir = cwd if cwd else os.getcwd()
if not os.path.isdir(working_dir):
raise HTTPException(
status_code=400,
detail=f"The provided cwd is not a valid directory: {working_dir}",
)

# --- Build SDK options ---
options = ClaudeAgentOptions(
system_prompt=(
"You are an OpenShift operator code generation assistant. "
"Execute the oape:api-implement plugin with the provided EP URL. "
),
cwd=working_dir,
permission_mode="bypassPermissions",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Document or make bypassPermissions mode configurable.

Setting permission_mode="bypassPermissions" disables permission checks for the agent, which could allow unintended filesystem or tool access. Consider:

  1. Making this configurable via environment variable or config
  2. Documenting the security implications
  3. Using a more restrictive mode in production
🔒 Proposed fix to make permission mode configurable
     options = ClaudeAgentOptions(
         system_prompt=(
             "You are an OpenShift operator code generation assistant. "
             "Execute the oape:api-implement plugin with the provided EP URL. "
         ),
         cwd=working_dir,
-        permission_mode="bypassPermissions",
+        permission_mode=CONFIGS.get("permission_mode", "default"),
         allowed_tools=CONFIGS['claude_allowed_tools'],
         plugins=[{"type": "local", "path": PLUGIN_DIR}],
     )
📝 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
permission_mode="bypassPermissions",
options = ClaudeAgentOptions(
system_prompt=(
"You are an OpenShift operator code generation assistant. "
"Execute the oape:api-implement plugin with the provided EP URL. "
),
cwd=working_dir,
permission_mode=CONFIGS.get("permission_mode", "default"),
allowed_tools=CONFIGS['claude_allowed_tools'],
plugins=[{"type": "local", "path": PLUGIN_DIR}],
)
🤖 Prompt for AI Agents
In `@server/server.py` at line 98, The agent is hard-coded with
permission_mode="bypassPermissions" which disables permission checks; change
this to read from a configurable source (e.g., an environment variable like
PERMISSION_MODE or a config setting) and validate allowed values before passing
to the agent initialization (replace the literal "bypassPermissions" used where
permission_mode is set). Default to a restrictive mode (e.g.,
"enforcePermissions") when the env/config is absent, emit a startup warning if a
permissive mode like "bypassPermissions" is selected, and add brief
documentation/comments about the security implications so production deployments
use the safer default.

allowed_tools=CONFIGS['claude_allowed_tools'],
plugins=[{"type": "local", "path": PLUGIN_DIR}],
)

# --- Run the agent ---
output_parts: list[str] = []
cost_usd = 0.0

try:
async for message in query(
prompt=f"/oape:api-implement {ep_url}",
# prompt="explain the enhancement proposal to me like I'm 5 in 10 sentences, {ep_url}",
options=options,
):
if isinstance(message, AssistantMessage):
for block in message.content:
if isinstance(block, TextBlock):
output_parts.append(block.text)
elif isinstance(message, ResultMessage):
cost_usd = message.total_cost_usd
if message.result:
output_parts.append(message.result)
except Exception as exc:
raise HTTPException(
status_code=500,
detail=f"Agent execution failed: {exc}",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Improve exception handling with proper chaining.

Per static analysis: the broad Exception catch may mask unexpected errors, and the re-raised HTTPException should chain the original exception to preserve the traceback for debugging.

🛡️ Proposed fix with exception chaining
-    except Exception as exc:
-        raise HTTPException(
+    except Exception as exc:
+        raise HTTPException(
             status_code=500,
             detail=f"Agent execution failed: {exc}",
-        )
+        ) from exc

Note: If the Claude Agent SDK defines more specific exceptions, consider catching those instead of the broad Exception to avoid masking programming errors.

📝 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
except Exception as exc:
raise HTTPException(
status_code=500,
detail=f"Agent execution failed: {exc}",
)
except Exception as exc:
raise HTTPException(
status_code=500,
detail=f"Agent execution failed: {exc}",
) from exc
🧰 Tools
🪛 Ruff (0.15.0)

[warning] 113-113: Do not catch blind exception: Exception

(BLE001)


[warning] 114-117: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents
In `@server/server.py` around lines 113 - 117, The current broad "except Exception
as exc" block re-raises an HTTPException without chaining, which loses the
original traceback; modify the handler to either catch specific agent/SDK
exceptions (e.g., AgentExecutionError or SDK-specific exceptions) or, if you
must catch Exception, re-raise the HTTPException using exception chaining (raise
HTTPException(status_code=500, detail=f"Agent execution failed: {exc}") from
exc) so the original traceback is preserved; update the except block that
contains "except Exception as exc" and reference HTTPException to implement this
change.


return {
"status": "success",
"ep_url": ep_url,
"cwd": working_dir,
"output": "\n".join(output_parts),
"cost_usd": cost_usd,
}