Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a complete agent-based workflow orchestrator for OAPE using the Claude Agent SDK. It provides an interactive CLI entry point that guides users through an Enhancement Proposal URL, orchestrates a three-phase implementation pipeline (API generation, parallel controller/E2E implementation, CI monitoring), and manages state across phases while coordinating with GitHub repositories and CI systems. Changes
Sequence DiagramsequenceDiagram
actor User
participant Main as main.py
participant Phase1 as phase1.py
participant Phase2 as phase2.py
participant Phase3 as phase3.py
participant SDK as ClaudeSDKClient
participant GitHub as GitHub API<br/>(gh cli)
participant Repo as Repository
User->>Main: Run agent with EP URL
activate Main
Main->>Main: Prompt & validate EP URL
Main->>Main: Resolve repo details
Main->>Main: Create WorkflowState
rect rgba(100, 200, 100, 0.5)
Note over Main,Repo: Phase 1: Sequential API Generation
Main->>Phase1: run(state)
activate Phase1
Phase1->>SDK: /oape:init (clone repo)
SDK->>Repo: Clone repository
Phase1->>SDK: /oape:api-generate
SDK->>Repo: Generate API types
Phase1->>SDK: /oape:api-generate-tests
SDK->>Repo: Generate tests
Phase1->>SDK: Review/fix phase
SDK->>Repo: Build & validate
Phase1->>SDK: Create PR
SDK->>GitHub: Push branch & create PR
GitHub->>Phase1: PR URL
Phase1->>Main: Update state with api_summary_md
deactivate Phase1
end
rect rgba(150, 150, 200, 0.5)
Note over Main,GitHub: Phase 2: Parallel Implementation
Main->>Phase2: run(state)
activate Phase2
par Controller Sub-agent
Phase2->>SDK: /oape:api-implement
SDK->>Repo: Implement API
SDK->>GitHub: Create PR
and E2E Tests Sub-agent
Phase2->>SDK: /oape:e2e-generate
SDK->>Repo: Generate E2E tests
SDK->>GitHub: Create PR
end
GitHub->>Phase2: PR URLs
Phase2->>Main: Update state.pr_urls
deactivate Phase2
end
rect rgba(200, 100, 100, 0.5)
Note over Main,GitHub: Phase 3: CI Monitoring
Main->>Phase3: run(state)
activate Phase3
loop Poll until CI passes or timeout
Phase3->>GitHub: Query PR status (gh pr view)
GitHub->>Phase3: statusCheckRollup results
Phase3->>Phase3: Check all passing
end
Phase3->>Main: Final workflow summary
deactivate Phase3
end
Main->>User: Return workflow results
deactivate Main
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
agent-wo-server/agent/utils.py (1)
24-31: Prefercsv.DictReaderfor robust repo CSV parsing.
Manual splitting breaks on commas and ignores quoting; DictReader is safer and simpler.♻️ Suggested refactor
-import re -import sys +import csv +import re +import sys @@ - rows: list[dict[str, str]] = [] - with open(TEAM_REPOS_CSV) as f: - headers = f.readline().strip().split(",") - for line in f: - parts = line.strip().split(",") - if len(parts) >= len(headers): - rows.append(dict(zip(headers, parts))) + with open(TEAM_REPOS_CSV, newline="", encoding="utf-8") as f: + reader = csv.DictReader(f) + rows: list[dict[str, str]] = [row for row in reader if row]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent-wo-server/agent/utils.py` around lines 24 - 31, Replace the manual CSV parsing (reading headers and splitting lines) with Python's csv.DictReader to correctly handle quoted fields and embedded commas: open TEAM_REPOS_CSV (use newline='' and appropriate encoding), create a csv.DictReader over the file, and iterate to append each row (as a dict) to the existing rows list; keep the variable name rows and ensure any existing logic that expected only rows of keys from the header still works (e.g., where rows is used elsewhere), and remove the manual headers/readline/str.split logic currently around TEAM_REPOS_CSV.agent-wo-server/requirements.txt (1)
1-1: Pin claude-agent-sdk to a tested minor range.The codebase is already written for the 0.1.x series (using
ClaudeAgentOptionsandClaudeSDKClient). Using~=0.1.0instead of>=0.1.0prevents unintended pulls of future major versions while allowing safe patch updates within the 0.1.x range.♻️ Suggested constraint
-claude-agent-sdk>=0.1.0 +claude-agent-sdk~=0.1.0This same constraint should also be applied to
server/requirements.txtfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent-wo-server/requirements.txt` at line 1, Update the pinned dependency range for claude-agent-sdk to the tested 0.1.x series by replacing the current unconstrained specifier with the compatible pre-release operator (~=0.1.0) in agent-wo-server/requirements.txt; make the same change in server/requirements.txt so the code that depends on ClaudeAgentOptions and ClaudeSDKClient remains locked to 0.1.x while still getting patch updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent-wo-server/agent/phase1.py`:
- Around line 50-64: The code currently uses state.repo_local_path without
ensuring it was found and interpolates it into a shell command unquoted; update
the block that sets cloned_dir and state.repo_local_path so that after the loop
you check whether state.repo_local_path is truthy and if not raise/return early
with a clear error (fail fast), and when invoking client.query (the f"cd ... &&
/oape:api-generate {state.ep_url}" call) use proper shell quoting (e.g.,
shlex.quote) of state.repo_local_path and state.ep_url to avoid spaces or
injection issues; refer to cloned_dir, workdir, state.repo_local_path,
client.query, and state.ep_url to locate and change the code.
In `@agent-wo-server/agent/phase3.py`:
- Around line 92-110: The loop in phase3.py that polls PR status (iterating over
remaining, calling _check_pr_status) currently treats completed-but-failed
checks as "waiting (0 pending)"; modify the polling logic so when status reports
zero pending (e.g., pending == 0 or status.get("pending") == 0) and
status["checks_pass"] is False, immediately mark the PR as failed: add it to
failed[label] and remove it from remaining, and print a concise message such as
"[Xm] {label}: CHECKS FAILED ({num_checks} checks, {failed_count} failed)" using
fields from status (e.g., status['num_checks'] and a failed count if available)
to avoid waiting until timeout. Ensure this change lives alongside the existing
branches that set passed[label] and handle state == "CLOSED" and uses the same
variables passed, failed, remaining, and _check_pr_status.
- Around line 156-158: The print call uses an f-string without placeholders
which triggers Ruff F541; update the print statement in phase3.py (near
write_state_summary and the print of the workflow-summary path) to use a plain
string instead of an f-string (i.e., remove the leading f) so it becomes
print("\nFull summary: .oape-work/final/workflow-summary.md") and keep the
surrounding write_state_summary(...) call unchanged.
In `@agent-wo-server/agent/utils.py`:
- Around line 17-42: resolve_repo currently declares ep_url but never uses it;
either remove the parameter or use it to avoid a dead parameter. Keep the API
unchanged by using ep_url to pre-select or filter: after loading rows from
TEAM_REPOS_CSV, if ep_url is non-empty, try to find a matching row by comparing
ep_url to row['repo_url'] and row.get('repo_short_name') and immediately return
(repo_short_name, repo_url, base_branch) when found; if no match, fall back to
the existing interactive prompt (choice handling) and only exit with an error if
rows are empty or the user input is invalid. Ensure you reference resolve_repo,
ep_url, rows, TEAM_REPOS_CSV, and choice when making the change.
---
Nitpick comments:
In `@agent-wo-server/agent/utils.py`:
- Around line 24-31: Replace the manual CSV parsing (reading headers and
splitting lines) with Python's csv.DictReader to correctly handle quoted fields
and embedded commas: open TEAM_REPOS_CSV (use newline='' and appropriate
encoding), create a csv.DictReader over the file, and iterate to append each row
(as a dict) to the existing rows list; keep the variable name rows and ensure
any existing logic that expected only rows of keys from the header still works
(e.g., where rows is used elsewhere), and remove the manual
headers/readline/str.split logic currently around TEAM_REPOS_CSV.
In `@agent-wo-server/requirements.txt`:
- Line 1: Update the pinned dependency range for claude-agent-sdk to the tested
0.1.x series by replacing the current unconstrained specifier with the
compatible pre-release operator (~=0.1.0) in agent-wo-server/requirements.txt;
make the same change in server/requirements.txt so the code that depends on
ClaudeAgentOptions and ClaudeSDKClient remains locked to 0.1.x while still
getting patch updates.
| # Determine the cloned repo path | ||
| cloned_dir = workdir / state.repo_short_name | ||
| if cloned_dir.is_dir(): | ||
| state.repo_local_path = str(cloned_dir) | ||
| else: | ||
| for child in workdir.iterdir(): | ||
| if (child / ".git").is_dir(): | ||
| state.repo_local_path = str(child) | ||
| break | ||
|
|
||
| # -- Step 2: Generate API types -- | ||
| print("\n--- Step 2/5: /oape:api-generate ---") | ||
| await client.query( | ||
| f"cd {state.repo_local_path} && /oape:api-generate {state.ep_url}" | ||
| ) |
There was a problem hiding this comment.
Fail fast if the repo path can’t be determined (and quote it).
Step 2 uses state.repo_local_path even when it’s unresolved, and the path isn’t shell-quoted. This can run commands in the wrong directory or fail on paths with spaces.
🐛 Suggested fix
-import textwrap
-import time
+import shlex
+import textwrap
+import time
@@
if cloned_dir.is_dir():
state.repo_local_path = str(cloned_dir)
else:
for child in workdir.iterdir():
if (child / ".git").is_dir():
state.repo_local_path = str(child)
break
+ if not state.repo_local_path:
+ raise RuntimeError("Failed to determine cloned repo path after /oape:init")
+ repo_path = shlex.quote(state.repo_local_path)
@@
- await client.query(
- f"cd {state.repo_local_path} && /oape:api-generate {state.ep_url}"
- )
+ await client.query(
+ f"cd {repo_path} && /oape:api-generate {state.ep_url}"
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent-wo-server/agent/phase1.py` around lines 50 - 64, The code currently
uses state.repo_local_path without ensuring it was found and interpolates it
into a shell command unquoted; update the block that sets cloned_dir and
state.repo_local_path so that after the loop you check whether
state.repo_local_path is truthy and if not raise/return early with a clear error
(fail fast), and when invoking client.query (the f"cd ... && /oape:api-generate
{state.ep_url}" call) use proper shell quoting (e.g., shlex.quote) of
state.repo_local_path and state.ep_url to avoid spaces or injection issues;
refer to cloned_dir, workdir, state.repo_local_path, client.query, and
state.ep_url to locate and change the code.
| while remaining and (time.time() - start) < wait * 60: | ||
| for label, url in list(remaining.items()): | ||
| status = _check_pr_status(url) | ||
| elapsed = int((time.time() - start) / 60) | ||
|
|
||
| if status["checks_pass"]: | ||
| print( | ||
| f" [{elapsed}m] {label}: ALL CHECKS PASSED " | ||
| f"({status['num_checks']} checks)" | ||
| ) | ||
| passed[label] = url | ||
| del remaining[label] | ||
| elif status["state"] == "CLOSED": | ||
| print(f" [{elapsed}m] {label}: PR CLOSED") | ||
| failed[label] = url | ||
| del remaining[label] | ||
| else: | ||
| pending = status.get("pending", "?") | ||
| print(f" [{elapsed}m] {label}: waiting ({pending} pending)") |
There was a problem hiding this comment.
Handle completed-but-failed checks to avoid waiting until timeout.
If all checks are completed but one fails, this loop keeps logging “waiting (0 pending)” until timeout. Treat that case as failed so the workflow can finish promptly.
🔧 Suggested fix
- else:
- pending = status.get("pending", "?")
- print(f" [{elapsed}m] {label}: waiting ({pending} pending)")
+ elif status.get("num_checks", 0) > 0 and status.get("pending", 0) == 0:
+ print(
+ f" [{elapsed}m] {label}: CHECKS FAILED "
+ f"({status['num_checks']} checks)"
+ )
+ failed[label] = url
+ del remaining[label]
+ else:
+ pending = status.get("pending", "?")
+ print(f" [{elapsed}m] {label}: waiting ({pending} pending)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent-wo-server/agent/phase3.py` around lines 92 - 110, The loop in phase3.py
that polls PR status (iterating over remaining, calling _check_pr_status)
currently treats completed-but-failed checks as "waiting (0 pending)"; modify
the polling logic so when status reports zero pending (e.g., pending == 0 or
status.get("pending") == 0) and status["checks_pass"] is False, immediately mark
the PR as failed: add it to failed[label] and remove it from remaining, and
print a concise message such as "[Xm] {label}: CHECKS FAILED ({num_checks}
checks, {failed_count} failed)" using fields from status (e.g.,
status['num_checks'] and a failed count if available) to avoid waiting until
timeout. Ensure this change lives alongside the existing branches that set
passed[label] and handle state == "CLOSED" and uses the same variables passed,
failed, remaining, and _check_pr_status.
| """) | ||
| write_state_summary(workdir, "workflow-summary.md", summary) | ||
| print(f"\nFull summary: .oape-work/final/workflow-summary.md") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "phase3.py" -type fRepository: shiftweek/oape-ai-e2e
Length of output: 98
🏁 Script executed:
sed -n '156,158p' ./agent-wo-server/agent/phase3.pyRepository: shiftweek/oape-ai-e2e
Length of output: 205
🏁 Script executed:
# Let's verify the Ruff F541 rule to confirm this is the right rule
web_search Ruff F541 rule definitionRepository: shiftweek/oape-ai-e2e
Length of output: 115
🏁 Script executed:
# Check the exact line with context
cat -n ./agent-wo-server/agent/phase3.py | sed -n '155,160p'Repository: shiftweek/oape-ai-e2e
Length of output: 280
🌐 Web query:
Ruff F541 rule f-string without placeholders
💡 Result:
Ruff F541 is the f-string-missing-placeholders rule: it flags f-strings that contain no {...} placeholder expressions (e.g., f"Hello"). The intent is that if you aren’t interpolating anything, you should use a normal string instead (or it may indicate you forgot to add a placeholder). [1][2]
Fix: remove the f prefix.
# before
msg = f"Hello, world!"
# after
msg = "Hello, world!"Note: For implicit concatenation, Ruff only flags it if none of the f-string parts contain placeholders. [1]
Remove unnecessary f-string prefix (Ruff F541).
This f-string has no placeholders and will trip Ruff.
🔧 Suggested fix
- print(f"\nFull summary: .oape-work/final/workflow-summary.md")
+ print("\nFull summary: .oape-work/final/workflow-summary.md")📝 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.
| """) | |
| write_state_summary(workdir, "workflow-summary.md", summary) | |
| print(f"\nFull summary: .oape-work/final/workflow-summary.md") | |
| """) | |
| write_state_summary(workdir, "workflow-summary.md", summary) | |
| print("\nFull summary: .oape-work/final/workflow-summary.md") |
🧰 Tools
🪛 Ruff (0.15.1)
[error] 158-158: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent-wo-server/agent/phase3.py` around lines 156 - 158, The print call uses
an f-string without placeholders which triggers Ruff F541; update the print
statement in phase3.py (near write_state_summary and the print of the
workflow-summary path) to use a plain string instead of an f-string (i.e.,
remove the leading f) so it becomes print("\nFull summary:
.oape-work/final/workflow-summary.md") and keep the surrounding
write_state_summary(...) call unchanged.
| def resolve_repo(ep_url: str) -> tuple[str, str, str]: | ||
| """ | ||
| Prompt the user to pick the target operator repository from team-repos.csv. | ||
|
|
||
| Returns: | ||
| (repo_short_name, repo_url, base_branch) | ||
| """ | ||
| rows: list[dict[str, str]] = [] | ||
| with open(TEAM_REPOS_CSV) as f: | ||
| headers = f.readline().strip().split(",") | ||
| for line in f: | ||
| parts = line.strip().split(",") | ||
| if len(parts) >= len(headers): | ||
| rows.append(dict(zip(headers, parts))) | ||
|
|
||
| if not rows: | ||
| print("ERROR: team-repos.csv is empty or malformed.") | ||
| sys.exit(1) | ||
|
|
||
| print("\n--- Available target repositories ---") | ||
| for i, row in enumerate(rows, 1): | ||
| print(f" [{i}] {row['repo_url']} (base: {row['base_branch']})") | ||
|
|
||
| choice = input( | ||
| "\nEnter the number of the target repository (or the repo short name): " | ||
| ).strip() |
There was a problem hiding this comment.
Use ep_url or remove it to avoid a dead parameter.
resolve_repo accepts ep_url but never uses it, which is confusing and flagged by static analysis.
🛠️ Minimal use (keeps API unchanged)
@@
- print("\n--- Available target repositories ---")
+ print(f"\nEnhancement Proposal: {ep_url}")
+ print("\n--- Available target repositories ---")📝 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.
| def resolve_repo(ep_url: str) -> tuple[str, str, str]: | |
| """ | |
| Prompt the user to pick the target operator repository from team-repos.csv. | |
| Returns: | |
| (repo_short_name, repo_url, base_branch) | |
| """ | |
| rows: list[dict[str, str]] = [] | |
| with open(TEAM_REPOS_CSV) as f: | |
| headers = f.readline().strip().split(",") | |
| for line in f: | |
| parts = line.strip().split(",") | |
| if len(parts) >= len(headers): | |
| rows.append(dict(zip(headers, parts))) | |
| if not rows: | |
| print("ERROR: team-repos.csv is empty or malformed.") | |
| sys.exit(1) | |
| print("\n--- Available target repositories ---") | |
| for i, row in enumerate(rows, 1): | |
| print(f" [{i}] {row['repo_url']} (base: {row['base_branch']})") | |
| choice = input( | |
| "\nEnter the number of the target repository (or the repo short name): " | |
| ).strip() | |
| def resolve_repo(ep_url: str) -> tuple[str, str, str]: | |
| """ | |
| Prompt the user to pick the target operator repository from team-repos.csv. | |
| Returns: | |
| (repo_short_name, repo_url, base_branch) | |
| """ | |
| rows: list[dict[str, str]] = [] | |
| with open(TEAM_REPOS_CSV) as f: | |
| headers = f.readline().strip().split(",") | |
| for line in f: | |
| parts = line.strip().split(",") | |
| if len(parts) >= len(headers): | |
| rows.append(dict(zip(headers, parts))) | |
| if not rows: | |
| print("ERROR: team-repos.csv is empty or malformed.") | |
| sys.exit(1) | |
| print(f"\nEnhancement Proposal: {ep_url}") | |
| print("\n--- Available target repositories ---") | |
| for i, row in enumerate(rows, 1): | |
| print(f" [{i}] {row['repo_url']} (base: {row['base_branch']})") | |
| choice = input( | |
| "\nEnter the number of the target repository (or the repo short name): " | |
| ).strip() |
🧰 Tools
🪛 Ruff (0.15.1)
[warning] 17-17: Unused function argument: ep_url
(ARG001)
[warning] 30-30: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@agent-wo-server/agent/utils.py` around lines 17 - 42, resolve_repo currently
declares ep_url but never uses it; either remove the parameter or use it to
avoid a dead parameter. Keep the API unchanged by using ep_url to pre-select or
filter: after loading rows from TEAM_REPOS_CSV, if ep_url is non-empty, try to
find a matching row by comparing ep_url to row['repo_url'] and
row.get('repo_short_name') and immediately return (repo_short_name, repo_url,
base_branch) when found; if no match, fall back to the existing interactive
prompt (choice handling) and only exit with an error if rows are empty or the
user input is invalid. Ensure you reference resolve_repo, ep_url, rows,
TEAM_REPOS_CSV, and choice when making the change.
| print("PHASE 2: Parallel Sub-Agents (Controller + E2E Tests)") | ||
| print("=" * 70) | ||
|
|
||
| results = await asyncio.gather( |
There was a problem hiding this comment.
If we are running them parallely, then wouldn't the e2e generated without the controller logic be incomplete? Also since they are making changes in the same folder and share one working tree and one .git, is it possible that changes may get jumbled?
| state.repo_local_path = str(child) | ||
| break | ||
|
|
||
| # -- Step 2: Generate API types -- |
There was a problem hiding this comment.
can we add a check before this step - if state.repo_local_path doesn't exist after step 1 finishes, then throw error or stop the process?
| OAPE AI E2E Workflow — entry point. | ||
|
|
||
| Usage: | ||
| python -m src.agent.main |
There was a problem hiding this comment.
| python -m src.agent.main | |
| python -m agent_wo_server.agent.main |
| if not state.repo_local_path: | ||
| print("ERROR: Phase 1 failed to establish repo local path. Aborting.") | ||
| sys.exit(1) | ||
|
|
There was a problem hiding this comment.
can we also check if state.api_summary_md is present before we proceed to phase 2?
Instead of prompt-based workflow, we need sessions to keep track of the Workflows.
replaces #17 and #18.
Summary by CodeRabbit