[codex] add internal symphony runner and guarded automerge#5
Merged
0x-Professor merged 1 commit intodevelopfrom Mar 6, 2026
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 24 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tools/symphony/command.py">
<violation number="1" location="tools/symphony/command.py:52">
P0: Avoid `shell=True` for runner commands; it enables command injection when interpolated issue/PR text reaches command strings.</violation>
</file>
<file name="tools/symphony/codex_app_server.py">
<violation number="1" location="tools/symphony/codex_app_server.py:156">
P1: Bug: `_wait_for_response` can raise a spurious timeout even when the response has already been received and stored in `self._responses`. After `_pump_messages` returns, the while-loop's deadline check may fail before re-checking `_responses`. Add a final `_responses` check after the loop exits.</violation>
</file>
<file name="tools/symphony/service.py">
<violation number="1" location="tools/symphony/service.py:300">
P3: Duplicate `except` blocks with identical bodies. These two consecutive exception handlers return the exact same `ExecutionOutcome`. Combine them into a single `except` clause for clarity and to reduce code duplication.</violation>
<violation number="2" location="tools/symphony/service.py:378">
P0: **Shell command injection via GitHub issue title.** The `safe_title` sanitization only replaces `"` with `'`, but does not escape shell metacharacters like `$()`, backticks, or `\`. Since `ShellCommandRunner.run()` uses `shell=True`, a malicious issue title (e.g., `Fix $(rm -rf /) bug`) would execute arbitrary commands. Use `shlex.quote()` to properly escape the title, or switch to passing the commit message via `--file` / stdin to avoid shell interpretation entirely.</violation>
<violation number="3" location="tools/symphony/service.py:628">
P2: Bare `except Exception: pass` silently swallows errors during post-merge issue release. If `get_issue` or `release_issue` fails, the issue could be left stuck in `agent-running` state with no diagnostic trace. At minimum, log the exception before discarding it.</violation>
</file>
<file name="tools/symphony/worktree.py">
<violation number="1" location="tools/symphony/worktree.py:84">
P2: Workspace discovery is too permissive: it trusts folder names without validating containment or git-worktree markers before cleanup.</violation>
</file>
<file name="tests/test_symphony_workflow.py">
<violation number="1" location="tests/test_symphony_workflow.py:91">
P2: `test_workflow_manager_reload_if_changed` is timing-dependent and can be flaky on filesystems with coarse mtime precision.</violation>
</file>
<file name="tools/symphony/workflow.py">
<violation number="1" location="tools/symphony/workflow.py:105">
P2: Stringifying `workspace.root` forces `null` to become the literal path `"None"`, so optional/null config values resolve to a real directory instead of falling back to defaults.</violation>
<violation number="2" location="tools/symphony/workflow.py:109">
P2: Stringifying `workspace.logs_root` forces `null` to become the literal path `"None"`, so optional/null config values resolve to an unintended logs directory.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| stderr=subprocess.PIPE, | ||
| text=True, | ||
| encoding="utf-8", | ||
| shell=True, |
There was a problem hiding this comment.
P0: Avoid shell=True for runner commands; it enables command injection when interpolated issue/PR text reaches command strings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/command.py, line 52:
<comment>Avoid `shell=True` for runner commands; it enables command injection when interpolated issue/PR text reaches command strings.</comment>
<file context>
@@ -0,0 +1,122 @@
+ stderr=subprocess.PIPE,
+ text=True,
+ encoding="utf-8",
+ shell=True,
+ )
+ payload: dict[str, object] = {}
</file context>
| if not changed_files.stdout.strip(): | ||
| return None | ||
| self.runner.run("git add -A", cwd=worktree.path, cancel_event=cancel_event) | ||
| safe_title = issue.title[:50].replace('"', "'") |
There was a problem hiding this comment.
P0: Shell command injection via GitHub issue title. The safe_title sanitization only replaces " with ', but does not escape shell metacharacters like $(), backticks, or \. Since ShellCommandRunner.run() uses shell=True, a malicious issue title (e.g., Fix $(rm -rf /) bug) would execute arbitrary commands. Use shlex.quote() to properly escape the title, or switch to passing the commit message via --file / stdin to avoid shell interpretation entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/service.py, line 378:
<comment>**Shell command injection via GitHub issue title.** The `safe_title` sanitization only replaces `"` with `'`, but does not escape shell metacharacters like `$()`, backticks, or `\`. Since `ShellCommandRunner.run()` uses `shell=True`, a malicious issue title (e.g., `Fix $(rm -rf /) bug`) would execute arbitrary commands. Use `shlex.quote()` to properly escape the title, or switch to passing the commit message via `--file` / stdin to avoid shell interpretation entirely.</comment>
<file context>
@@ -0,0 +1,677 @@
+ if not changed_files.stdout.strip():
+ return None
+ self.runner.run("git add -A", cwd=worktree.path, cancel_event=cancel_event)
+ safe_title = issue.title[:50].replace('"', "'")
+ commit_message = f'git commit -m "agent: resolve #{issue.number} {safe_title}"'
+ commit_result = self.runner.run(
</file context>
| raise CodexAppServerError(str(response["error"])) | ||
| return dict(response["result"]) | ||
| self._pump_messages(cancel_event=cancel_event, timeout=0.2) | ||
| raise CodexAppServerError( |
There was a problem hiding this comment.
P1: Bug: _wait_for_response can raise a spurious timeout even when the response has already been received and stored in self._responses. After _pump_messages returns, the while-loop's deadline check may fail before re-checking _responses. Add a final _responses check after the loop exits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/codex_app_server.py, line 156:
<comment>Bug: `_wait_for_response` can raise a spurious timeout even when the response has already been received and stored in `self._responses`. After `_pump_messages` returns, the while-loop's deadline check may fail before re-checking `_responses`. Add a final `_responses` check after the loop exits.</comment>
<file context>
@@ -0,0 +1,305 @@
+ raise CodexAppServerError(str(response["error"]))
+ return dict(response["result"])
+ self._pump_messages(cancel_event=cancel_event, timeout=0.2)
+ raise CodexAppServerError(
+ f"Timed out waiting for Codex response to request {request_id}."
+ )
</file context>
| current_issue = self.tracker.get_issue(outcome.issue_number) | ||
| if current_issue.normalized_state == "open": | ||
| self.tracker.release_issue(outcome.issue_number) | ||
| except Exception: |
There was a problem hiding this comment.
P2: Bare except Exception: pass silently swallows errors during post-merge issue release. If get_issue or release_issue fails, the issue could be left stuck in agent-running state with no diagnostic trace. At minimum, log the exception before discarding it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/service.py, line 628:
<comment>Bare `except Exception: pass` silently swallows errors during post-merge issue release. If `get_issue` or `release_issue` fails, the issue could be left stuck in `agent-running` state with no diagnostic trace. At minimum, log the exception before discarding it.</comment>
<file context>
@@ -0,0 +1,677 @@
+ current_issue = self.tracker.get_issue(outcome.issue_number)
+ if current_issue.normalized_state == "open":
+ self.tracker.release_issue(outcome.issue_number)
+ except Exception:
+ pass
+ self.worktrees.cleanup_workspace(outcome.issue_number)
</file context>
| match = re.match(r"issue-(\d+)-", child.name) | ||
| if not match: | ||
| continue | ||
| items.append( |
There was a problem hiding this comment.
P2: Workspace discovery is too permissive: it trusts folder names without validating containment or git-worktree markers before cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/worktree.py, line 84:
<comment>Workspace discovery is too permissive: it trusts folder names without validating containment or git-worktree markers before cleanup.</comment>
<file context>
@@ -0,0 +1,113 @@
+ match = re.match(r"issue-(\d+)-", child.name)
+ if not match:
+ continue
+ items.append(
+ WorktreeInfo(
+ issue_number=int(match.group(1)),
</file context>
| encoding="utf-8", | ||
| ) | ||
| manager = WorkflowManager(workflow_path) | ||
| time.sleep(0.02) |
There was a problem hiding this comment.
P2: test_workflow_manager_reload_if_changed is timing-dependent and can be flaky on filesystems with coarse mtime precision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_symphony_workflow.py, line 91:
<comment>`test_workflow_manager_reload_if_changed` is timing-dependent and can be flaky on filesystems with coarse mtime precision.</comment>
<file context>
@@ -0,0 +1,138 @@
+ encoding="utf-8",
+ )
+ manager = WorkflowManager(workflow_path)
+ time.sleep(0.02)
+ workflow_path.write_text(
+ """
</file context>
Comment on lines
+109
to
+111
| logs_root = _expand_path( | ||
| str(workspace_map.get("logs_root", ".symphony/logs")), | ||
| base_dir=workflow_path.parent, |
There was a problem hiding this comment.
P2: Stringifying workspace.logs_root forces null to become the literal path "None", so optional/null config values resolve to an unintended logs directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/workflow.py, line 109:
<comment>Stringifying `workspace.logs_root` forces `null` to become the literal path `"None"`, so optional/null config values resolve to an unintended logs directory.</comment>
<file context>
@@ -0,0 +1,275 @@
+ str(workspace_map.get("root", ".symphony/workspaces")),
+ base_dir=workflow_path.parent,
+ )
+ logs_root = _expand_path(
+ str(workspace_map.get("logs_root", ".symphony/logs")),
+ base_dir=workflow_path.parent,
</file context>
Suggested change
| logs_root = _expand_path( | |
| str(workspace_map.get("logs_root", ".symphony/logs")), | |
| base_dir=workflow_path.parent, | |
| raw_logs_root = workspace_map.get("logs_root", ".symphony/logs") | |
| logs_root = _expand_path( | |
| str(raw_logs_root) if raw_logs_root is not None else None, | |
| base_dir=workflow_path.parent, | |
| ) |
Comment on lines
+105
to
+107
| workspace_root = _expand_path( | ||
| str(workspace_map.get("root", ".symphony/workspaces")), | ||
| base_dir=workflow_path.parent, |
There was a problem hiding this comment.
P2: Stringifying workspace.root forces null to become the literal path "None", so optional/null config values resolve to a real directory instead of falling back to defaults.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/workflow.py, line 105:
<comment>Stringifying `workspace.root` forces `null` to become the literal path `"None"`, so optional/null config values resolve to a real directory instead of falling back to defaults.</comment>
<file context>
@@ -0,0 +1,275 @@
+ )
+
+ workspace_map = dict(config_map.get("workspace") or {})
+ workspace_root = _expand_path(
+ str(workspace_map.get("root", ".symphony/workspaces")),
+ base_dir=workflow_path.parent,
</file context>
Suggested change
| workspace_root = _expand_path( | |
| str(workspace_map.get("root", ".symphony/workspaces")), | |
| base_dir=workflow_path.parent, | |
| raw_root = workspace_map.get("root", ".symphony/workspaces") | |
| workspace_root = _expand_path( | |
| str(raw_root) if raw_root is not None else None, | |
| base_dir=workflow_path.parent, | |
| ) |
| branch_name=worktree.branch_name, | ||
| workspace_path=worktree.path, | ||
| ) | ||
| except (CommandCancelledError, CodexAppServerError) as exc: |
There was a problem hiding this comment.
P3: Duplicate except blocks with identical bodies. These two consecutive exception handlers return the exact same ExecutionOutcome. Combine them into a single except clause for clarity and to reduce code duplication.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/symphony/service.py, line 300:
<comment>Duplicate `except` blocks with identical bodies. These two consecutive exception handlers return the exact same `ExecutionOutcome`. Combine them into a single `except` clause for clarity and to reduce code duplication.</comment>
<file context>
@@ -0,0 +1,677 @@
+ branch_name=worktree.branch_name,
+ workspace_path=worktree.path,
+ )
+ except (CommandCancelledError, CodexAppServerError) as exc:
+ return ExecutionOutcome(
+ issue_number=issue.number,
</file context>
0x-Professor
added a commit
that referenced
this pull request
Mar 6, 2026
* add internal symphony runner and guarded automerge (#5) * fix: address cubic symphony review findings * fix: address cubic follow-up findings
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
This pull request adds an internal GitHub-native Symphony runner for PersonaPort and pairs it with the repository harness that the runner needs to operate safely.
The change introduces a repo-owned
WORKFLOW.mdcontract, a compact agent knowledge layer underdocs/agents/, and an internaltools/symphony/package that can poll GitHub issues, prepare per-issue worktrees, launch Codex in app-server mode, run repository validation, open pull requests, and gate merge decisions through a maintainer-style review layer.User And Maintainer Impact
This does not change the published
personaportpackage or the end-user CLI.For maintainers, it adds a repository-local automation path that can:
developruff check .andpytestHigh-risk areas such as browser automation, auth/session handling, and provider key handling still stop at human review.
Root Cause
PersonaPort had no repository-owned harness for autonomous issue execution. The codebase had tests and CI, but no machine-readable workflow contract, no repo map/safety docs for agents, and no internal orchestration layer for issue polling, per-issue worktrees, PR creation, or guarded merge flow.
Fix
The pull request adds:
WORKFLOW.mdas the repo-owned workflow contractdocs/agents/with repo map, validation rules, and safety rulestools/symphony/with workflow parsing, GitHub issue/PR integration, worktree management, a minimal Codex app-server client, orchestration service logic, and guarded auto-merge review rulesValidation
I ran:
ruff check .pytestBoth passed on the branch before opening this PR.
Summary by cubic
Adds an internal GitHub-native Symphony runner with maintainer-gated auto-merge for low-risk PRs. It reads WORKFLOW.md, polls labeled issues, creates per-issue worktrees, runs validation, opens PRs, and merges when safe.
New Features
Migration
Written for commit 2fa4160. Summary will update on new commits.