Skip to content

Commit 4a63ad0

Browse files
costajohntclaude
andcommitted
fix(agent): deploy_change pushes branch + opens PR (no more ghost-deploy)
Strategy-agent end-to-end audit (post-PR-A1..A12) caught a critical bug that hadn't fired yet because no agent deploy has succeeded since the agent was built (24 trades < 50-trade safety floor, so all weekly runs have been "no-deploy diagnostic"). The first time it tries to deploy: agent_writes.deploy_change does: 1. git checkout -b agent/change-<timestamp> 2. git add <files> && git commit 3. git checkout main 4. git merge agent/change-<timestamp> --no-ff ← LOCAL only 5. git branch -d agent/change-<timestamp> Step 4's merge commit lives only on the runner's local checkout of main. There is NO `git push`. When the runner shuts down, the merge commit is discarded. Meanwhile the workflow's "Open PR for agent code changes" step (added in PR #160) does: git add <agent-modifiable-files> git diff --cached --quiet → exit deployed=false if no staged diff Because the agent already committed and merged locally, there are no staged changes. The workflow exits deployed=false. No PR is opened. No notification fires. The agent's changelog records status=deployed with a commit_sha that points at the discarded local merge commit. Net effect: ghost deploy. Changelog says deployed; live code is unchanged. compute_measured_impact 5 days later compares equal-state windows, reports no effect. The agent may re-attempt the same change on the next eligible cycle (after 14d cooling), perpetually ghost-deploying. Fix: deploy_change does the push and opens the PR itself. - After commit on the feature branch, `git push origin <branch> --force-with-lease`. - `gh pr create` to open the PR; on failure (re-run same day; PR already exists), recover URL via `gh pr view`. - Persist PR URL to data/agent/last_pr_url.txt for the workflow's Telegram step to pick up. - DO NOT merge to local main. main is protected — the PR is the deploy gate, CI runs on it, merging requires the operator (or an auto-merge rule). Workflow change: the "Open PR for agent code changes" step is renamed to "Surface agent deploy PR" and rewritten to read the PR URL the agent persisted. It no longer does its own push or pr-create — that responsibility now lives entirely in agent_writes.deploy_change. The "Run Strategy Agent" step gets GH_TOKEN env so the agent's gh CLI calls authenticate. Tests: 3 new in TestDeployChange covering the new push+PR path — successful PR creation, gh pr create fails (PR already exists) recovery via gh pr view, git push failure aborts cleanly. The obsolete test_merge_failure was removed (no merge step anymore). Verified locally: 2820 tests pass (was 2818; net +2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9607917 commit 4a63ad0

3 files changed

Lines changed: 213 additions & 84 deletions

File tree

.github/workflows/strategy-review.yml

Lines changed: 32 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ jobs:
9898
# later step propagates failure to the job status.
9999
- name: Run Strategy Agent (Weekly Review)
100100
id: agent_run
101+
env:
102+
# gh CLI inside the agent's deploy_change uses GITHUB_TOKEN to
103+
# push the agent/change-* feature branch and open the deploy PR.
104+
# PR-B1: prevents the prior "ghost deploy" where deploy_change
105+
# merged to local main but never pushed.
106+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
101107
run: uv run python scripts/strategy_agent.py --mode weekly
102108
continue-on-error: true
103109

@@ -134,69 +140,48 @@ jobs:
134140
# checkout failure rather than risk committing to wrong branch)
135141
git checkout main
136142
137-
# Only deploy if the agent itself ran cleanly (a crashed agent may have
138-
# left partial changes on disk we don't want to PR) AND we're not in
139-
# dry-run mode.
140-
- name: Open PR for agent code changes
143+
# PR-B1: surface the agent's deploy PR (opened by agent_writes.deploy_change
144+
# via gh CLI) for the downstream Telegram notification. Pre-PR-B1 this
145+
# step did the push + gh pr create itself, but the agent had already
146+
# merged its work to local main, so this step saw no staged diff and
147+
# exited deployed=false — the "ghost deploy" bug. Now the agent does
148+
# the push+PR; this step just reads the URL the agent left in
149+
# data/agent/last_pr_url.txt.
150+
- name: Surface agent deploy PR
141151
id: agent_deploy
142152
if: steps.agent_run.outcome == 'success' && inputs.dry_run != true
143153
env:
144154
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
145155
run: |
146-
git config user.name "strategy-agent[bot]"
147-
git config user.email "strategy-agent[bot]@users.noreply.github.com"
148-
149-
# Stage agent-modifiable code files (ignore errors for unchanged files)
150-
git add \
151-
strategies/constants.py \
152-
strategies/sector.py \
153-
strategies/sizing.py \
154-
scripts/screener_trade.py \
155-
scripts/momentum_trade.py \
156-
scripts/scan_with_sentiment.py \
157-
2>/dev/null || true
158-
159-
if git diff --cached --quiet; then
156+
PR_URL_FILE="data/agent/last_pr_url.txt"
157+
if [ ! -f "$PR_URL_FILE" ] || [ ! -s "$PR_URL_FILE" ]; then
158+
# Agent didn't deploy this run — most weekly runs don't, given
159+
# safety constraints (sample size floor, deploy budget, etc.).
160160
echo "deployed=false" >> "$GITHUB_OUTPUT"
161161
exit 0
162162
fi
163163
164-
DATE=$(date -u +%Y-%m-%d)
165-
BRANCH="agent/weekly-${DATE}"
166-
167-
# Push the staged changes to a fresh feature branch and open a PR.
168-
# main is protected (4 required status checks) so direct push is rejected;
169-
# the PR forces CI to pass before code reaches main.
170-
git checkout -B "$BRANCH"
171-
git commit -m "agent: weekly review code changes [${DATE}]"
172-
git push -u origin "$BRANCH" --force-with-lease
173-
174-
# Diff stats for the notification step (computed against main, not HEAD~1,
175-
# because the new branch may carry multiple agent commits over time).
176-
git diff "origin/main..${BRANCH}" --stat > /tmp/agent-deploy-summary.txt 2>/dev/null || true
177-
git diff "origin/main..${BRANCH}" --name-only > /tmp/agent-deploy-files.txt 2>/dev/null || true
178-
179-
# Open a PR. If a PR already exists for this branch (re-run same day),
180-
# gh pr create returns non-zero; we recover the URL via gh pr view.
181-
if PR_URL=$(gh pr create \
182-
--base main \
183-
--head "$BRANCH" \
184-
--title "agent: weekly review code changes [${DATE}]" \
185-
--body "Automated agent code change from the weekly Strategy Agent run. CI must pass before merge; merging deploys these parameter changes to live trading. See AGENT_LOG.md on the trading-data branch for the agent's reasoning."); then
186-
echo "Opened PR: $PR_URL"
187-
else
188-
PR_URL=$(gh pr view "$BRANCH" --json url --jq .url 2>/dev/null || echo "unknown")
189-
echo "Updated existing PR: $PR_URL"
164+
PR_URL=$(cat "$PR_URL_FILE")
165+
if [ -z "$PR_URL" ]; then
166+
echo "deployed=false" >> "$GITHUB_OUTPUT"
167+
exit 0
168+
fi
169+
echo "Agent opened PR: $PR_URL"
170+
171+
# Diff stats for the notification step. Pull the PR's branch from
172+
# gh so we can compute against origin/main (the PR's actual diff).
173+
BRANCH=$(gh pr view "$PR_URL" --json headRefName --jq .headRefName 2>/dev/null || echo "")
174+
if [ -n "$BRANCH" ]; then
175+
git fetch origin "$BRANCH" 2>/dev/null || true
176+
git diff "origin/main..origin/$BRANCH" --stat > /tmp/agent-deploy-summary.txt 2>/dev/null || true
177+
git diff "origin/main..origin/$BRANCH" --name-only > /tmp/agent-deploy-files.txt 2>/dev/null || true
190178
fi
191179
192180
{
193181
echo "deployed=true"
194182
echo "pr_url=$PR_URL"
195183
} >> "$GITHUB_OUTPUT"
196184
197-
# Return to main so the AGENT_LOG step starts from a known branch.
198-
git checkout main
199-
200185
- name: Notify on agent PR opened
201186
if: steps.agent_deploy.outputs.deployed == 'true'
202187
env:

scripts/agent_writes.py

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -550,13 +550,16 @@ def deploy_change(commit_message: str, evidence: dict) -> dict:
550550
_save_changelog(changelog)
551551

552552
# Helper to update the pending entry status in the changelog
553-
def _update_pending_status(status: str, commit_sha: str | None = None, snapshot: dict | None = None):
553+
def _update_pending_status(status: str, commit_sha: str | None = None,
554+
pr_url: str | None = None, snapshot: dict | None = None):
554555
cl = _at._load_changelog()
555556
for entry in cl["changes"]:
556557
if entry.get("id") == change_id:
557558
entry["status"] = status
558559
if commit_sha:
559560
entry["commit_sha"] = commit_sha
561+
if pr_url:
562+
entry["pr_url"] = pr_url
560563
if snapshot:
561564
entry["pre_deploy_snapshot"] = snapshot
562565
break
@@ -591,45 +594,96 @@ def _update_pending_status(status: str, commit_sha: str | None = None, snapshot:
591594
["git", "commit", "-m", f"agent: {commit_message}\n\nAutonomous strategy agent change.\nEvidence: {json.dumps(evidence, indent=2)}"],
592595
cwd=str(_at.PROJECT_DIR), check=True, capture_output=True,
593596
)
594-
subprocess.run(["git", "checkout", "main"], cwd=str(_at.PROJECT_DIR), check=True, capture_output=True)
595-
merge_result = subprocess.run(
596-
["git", "merge", branch, "--no-ff", "-m", f"Merge agent change: {commit_message}"],
597+
598+
# Capture the agent's commit SHA on the feature branch.
599+
sha_result = subprocess.run(
600+
["git", "rev-parse", "HEAD"], cwd=str(_at.PROJECT_DIR), capture_output=True, text=True,
601+
)
602+
commit_sha = sha_result.stdout.strip()
603+
604+
# PR-B1: push the feature branch and open a PR via gh CLI. We do NOT
605+
# merge to local main — main is protected (4 required status checks)
606+
# and a local merge would only exist on the runner, never reaching
607+
# origin (the prior "ghost deploy" bug). The PR is the deploy gate;
608+
# CI runs on it, the operator (or an auto-merge rule) reviews and
609+
# merges, and only then does the change reach live trading.
610+
push_result = subprocess.run(
611+
["git", "push", "origin", branch, "--force-with-lease"],
597612
cwd=str(_at.PROJECT_DIR), capture_output=True, text=True,
598613
)
599-
if merge_result.returncode != 0:
600-
subprocess.run(["git", "merge", "--abort"], cwd=str(_at.PROJECT_DIR), capture_output=True)
614+
if push_result.returncode != 0:
601615
subprocess.run(["git", "checkout", "main"], cwd=str(_at.PROJECT_DIR), capture_output=True)
616+
subprocess.run(["git", "branch", "-D", branch], cwd=str(_at.PROJECT_DIR), capture_output=True)
602617
_update_pending_status("failed")
603-
return {"success": False, "error": f"Merge failed: {merge_result.stderr}"}
604-
605-
# Get merge commit SHA
606-
sha_result = subprocess.run(
607-
["git", "rev-parse", "HEAD"], cwd=str(_at.PROJECT_DIR), capture_output=True, text=True
618+
return {"success": False, "error": f"git push failed: {push_result.stderr.strip()}"}
619+
620+
# Open the PR. If one already exists for this branch (re-run on same
621+
# day), gh pr create fails non-zero; we recover the URL via gh pr view.
622+
pr_body = (
623+
f"Automated strategy-agent code change from the weekly review.\n\n"
624+
f"**Evidence:**\n```json\n{json.dumps(evidence, indent=2)}\n```\n\n"
625+
f"CI must pass before merge. Merging this PR deploys the change "
626+
f"to live trading. See `AGENT_LOG.md` on the `trading-data` "
627+
f"branch for the agent's full reasoning."
608628
)
609-
commit_sha = sha_result.stdout.strip()
629+
pr_create = subprocess.run(
630+
["gh", "pr", "create", "--base", "main", "--head", branch,
631+
"--title", f"agent: {commit_message[:100]}",
632+
"--body", pr_body],
633+
cwd=str(_at.PROJECT_DIR), capture_output=True, text=True,
634+
)
635+
if pr_create.returncode == 0:
636+
pr_url = pr_create.stdout.strip()
637+
else:
638+
pr_view = subprocess.run(
639+
["gh", "pr", "view", branch, "--json", "url", "--jq", ".url"],
640+
cwd=str(_at.PROJECT_DIR), capture_output=True, text=True,
641+
)
642+
pr_url = pr_view.stdout.strip() if pr_view.returncode == 0 else ""
610643

611-
# Clean up branch
612-
subprocess.run(["git", "branch", "-d", branch], cwd=str(_at.PROJECT_DIR), capture_output=True)
644+
# Return to main so the agent's subsequent operations (and the
645+
# workflow's downstream steps) see a known branch.
646+
subprocess.run(["git", "checkout", "main"], cwd=str(_at.PROJECT_DIR), capture_output=True)
647+
648+
# Surface the PR URL to the workflow's notification step. Written to
649+
# data/agent/ so the workflow's existing trading-data snapshot
650+
# captures it alongside the changelog.
651+
try:
652+
pr_url_path = _at.PROJECT_DIR / "data" / "agent" / "last_pr_url.txt"
653+
pr_url_path.parent.mkdir(parents=True, exist_ok=True)
654+
pr_url_path.write_text(pr_url)
655+
except OSError as e:
656+
logger.warning("Could not persist PR URL for workflow surface: %s", e)
613657

614658
# Capture performance snapshot for monitoring — route through _at for patchability
615659
perf = _at.read_performance_summary()
616660

617-
# Update changelog entry to "deployed" with commit SHA and snapshot
618-
_update_pending_status("deployed", commit_sha=commit_sha, snapshot={
661+
# Update changelog. Status remains "deployed" for backward compat
662+
# with downstream consumers (compute_measured_impact); pr_url field
663+
# records that the deploy is gated by PR merge. Future: add a
664+
# post-merge ratify step that flips status from "pr_opened" to
665+
# "deployed" once the PR actually merges.
666+
_update_pending_status("deployed", commit_sha=commit_sha, pr_url=pr_url, snapshot={
619667
"total_signals": perf["signals"]["total"],
620668
"total_trades": perf["trades"]["total"],
621669
})
622670

623-
# Send deploy notification (failure should not block deploy)
671+
# Send notification (failure should not block return)
624672
try:
625673
from scripts.notify import send_alert
626-
send_alert("Agent Deploy", f"agent: {commit_message[:100]}")
674+
send_alert(
675+
"Agent Deploy: PR opened",
676+
f"agent: {commit_message[:100]}\nPR: {pr_url}\n"
677+
f"CI must pass and PR must be merged for the change to take effect.",
678+
)
627679
except Exception as e:
628-
logger.warning("Deploy notification failed (deploy still succeeded): %s", e)
680+
logger.warning("Deploy notification failed (PR still opened): %s", e)
629681

630682
return {
631683
"success": True,
632684
"commit_sha": commit_sha,
685+
"pr_url": pr_url,
686+
"branch": branch,
633687
"backtest": backtest_result,
634688
"tests": test_result["summary"],
635689
}

0 commit comments

Comments
 (0)