PLAN-61: LLM-assisted commit and PR creation for electron loops by thadeusb · Pull Request #38 · closedloop-ai/closedloop-electron

thadeusb · 2026-03-24T16:34:06Z

Summary

Replaces hardcoded "Symphony: implement plan" commit messages with LLM-generated descriptions of actual changes
Adds attemptLlmCommit() — spawns a Claude session post-execution to review the diff, write a real commit message, push, and create a PR with a descriptive title/body
PR bodies include loop ID and artifact link in a metadata footer for traceability
Mechanical fallback with contextual info (loop ID, command) if the LLM commit fails or times out
Exports sanitizeCommitMessage() from symphony-interactive.ts for cross-module reuse
Passes webAppOrigin through the router for artifact link generation

Plan: https://app.closedloop.ai/implementation-plans/PLAN-61

Test plan

Verify attemptLlmCommit() spawns Claude, reviews diff, commits, pushes, and creates PR
Verify PR body contains loop ID and artifact link when available
Verify mechanical fallback triggers when LLM commit fails/times out
Verify fallback template includes loop ID and command (not generic "Symphony: implement plan")
Verify sanitizeCommitMessage() strips model names from LLM output
Run pnpm test — new test file covers execute git operations flow

Replace hardcoded "Symphony: implement plan" commit messages and PR descriptions with LLM-generated content that describes actual changes. - Add attemptLlmCommit() that spawns a Claude session to review the diff, write a descriptive commit, push, and create a PR with a real title/body - Include loop ID and artifact link in PR body metadata footer - Mechanical fallback with contextual info if LLM commit fails - Export sanitizeCommitMessage() for reuse across modules - Pass webAppOrigin through router for artifact link generation - Add test coverage for the execute git operations flow

closedloop-ai-stage · 2026-03-24T16:50:53Z

+import { sanitizeCommitMessage } from "./symphony-interactive.js";
 import {
  expandHome,
  resolveWorktreeParentDir,


[HIGH] Project Rules

[P1] Missing version bump in apps/desktop/package.json

Recommendation: Increment the patch version in apps/desktop/package.json and include it in this commit.

import { closeSync, existsSync, openSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";

Addressed — version bumped to 0.8.1.

closedloop-ai-stage · 2026-03-24T16:50:54Z

+        const parsed: unknown = JSON.parse(raw);
+        if (isExecutionResult(parsed)) {
+          loopLog(loopId, `LLM commit wrote execution-result.json, pr=${parsed.prUrl}`);
+          resolve(parsed);


[MEDIUM] Correctness

[P2] execution-result.json not cleaned up after successful LLM commit, leaks into subsequent commits

Recommendation: After reading and validating execution-result.json, delete it unconditionally.

Fixed. execution-result.json is now cleaned up unconditionally after reading (success or failure) inside attemptLlmCommit, so it never leaks into subsequent runs.

closedloop-ai-stage · 2026-03-24T16:50:55Z

+    // shell escaping issues with special characters (--body-file approach).
+    const prBody = `Loop ID: ${loopId}\nCommand: ${command}`;
+    const bodyFile = path.join(worktreeDir, ".claude", "work", "pr-body.md");
+    writeFileSync(bodyFile, prBody);


[MEDIUM] Correctness

[P2] writeFileSync to .claude/work/pr-body.md may fail if directory does not exist

Recommendation: Add mkdirSync(path.dirname(bodyFile), { recursive: true }) before writeFileSync.

Fixed. Added mkdirSync(path.dirname(bodyFile), { recursive: true }) before writeFileSync.

closedloop-ai-stage · 2026-03-24T16:50:56Z

+      // Clean up LLM artifacts before fallback to prevent them from being committed
+      if (!llmResult) {
+        try { unlinkSync(path.join(worktreeDir, 'execution-result.json')); } catch { /* file may not exist */ }
+        try { unlinkSync(path.join(worktreeDir, 'execution-footer.txt')); } catch { /* file may not exist */ }


[HIGH] Correctness

[P1] LLM failure cleanup removes wrong file (execution-footer.txt instead of pr-body.md), leaving pr-body.md to be committed by fallback

Recommendation: Replace unlinkSync for execution-footer.txt with unlinkSync for pr-body.md to match what the LLM actually writes.

Fixed. The cleanup block now removes pr-body.md (not execution-footer.txt). Additionally, attemptLlmCommit itself now cleans up both scratch files unconditionally after reading the result.

closedloop-ai-stage · 2026-03-24T16:50:57Z

+    // For existing PRs this ensures the footer is always present.
+    try {
+      execSync(
+        `gh pr edit ${prNumber} --body-file ${shellEscape(bodyFile)}`,


[MEDIUM] Correctness

[P2] gh pr edit --body-file unconditionally replaces entire PR body for existing PRs with a 2-line metadata stub

Recommendation: Fetch the existing PR body before editing and append the metadata, or skip editing for newly-created PRs.

Fixed. For existing PRs, we now fetch the current body via gh pr view, check if the footer is already present, and only append the metadata if missing — preserving whatever description the user or LLM wrote.

closedloop-ai-stage · 2026-03-24T16:50:58Z

Code Review Summary

Status: Needs Attention

Reviewers: Bug Hunter A, Bug Hunter B, Unified Auditor, Premise Reviewer, Gateway Core Architect

Findings

Severity	Count
Blocking	0
High	2
Medium	3

HIGH Issues (should fix)

[P1] [symphony-loop.ts:1048] LLM failure cleanup removes wrong file (execution-footer.txt instead of pr-body.md), silently commits LLM artifacts via fallback git add
[P1] [symphony-loop.ts:22] Missing version bump in apps/desktop/package.json -- CI will reject this PR

MEDIUM Issues (consider)

[P2] [symphony-loop.ts:809] execution-result.json not cleaned up after successful LLM commit, leaks into subsequent commits
[P2] [symphony-loop.ts:910] writeFileSync to .claude/work/pr-body.md may fail with ENOENT if directory does not exist
[P2] [symphony-loop.ts:952] gh pr edit --body-file unconditionally replaces entire PR body for existing PRs with a 2-line metadata stub

Validation Stats

Agent failures: 0 partitions skipped
Cross-file grouped: 0
Discarded: 2 (1 line not in changed range, 1 duplicate)

Recommendation: Address the 2 HIGH issues before merging. The version bump is a hard CI blocker. The pr-body.md cleanup bug silently commits LLM artifacts into user repos on failed runs.

shafty023 · 2026-03-24T19:41:28Z

+        webAppOrigin ?? ""
      );
+
+      // Clean up LLM artifacts before fallback to prevent them from being committed


[P1] Move LLM scratch files out of the reused worktree

This cleanup only runs when llmResult is null, and it still misses pr-body.md, even though attemptLlmCommit tells Claude to write both pr-body.md and execution-result.json in the repo root. Because EXECUTE reuses the same worktree for later runs on the same artifact slug, those leftovers will be seen as real changes by the next git add -- . and can get committed into the user's branch; if Claude exits 0 without rewriting execution-result.json, the stale file is also treated as a fresh success and the fallback path is skipped.

Fixed. Scratch files (execution-result.json and pr-body.md) are now cleaned up unconditionally inside attemptLlmCommit after reading the result — regardless of success or failure. The fallback cleanup block also now removes pr-body.md instead of the wrong execution-footer.txt reference.

shafty023 · 2026-03-24T19:41:33Z

+      const prTitle = `${commitPrefix}Symphony: ${command} -- loop ${shortId}`;
      const prOutput = execSync(
-        `gh pr create --title "Symphony: implement plan" --body "Automated PR from Symphony loop" --base ${shellEscape(baseBranch)}`,
+        `gh pr create --title ${shellEscape(prTitle)} --body-file ${shellEscape(bodyFile)} --base ${shellEscape(baseBranch)} --label symphony`,


[P1] Make symphony labeling best-effort when creating PRs

On any repo that does not already have a symphony label, gh pr create ... --label symphony fails after the commit and push have already succeeded. Because the whole PR creation block is wrapped in this try, executeGitOperations returns null and the loop completes without a PR URL even though a new commit was pushed. This path used to work on unlabeled repos, so label attachment needs to be conditional or retried without --label.

Fixed. PR creation now runs without --label, then attaches the symphony label as a separate best-effort step via gh pr edit --add-label. If the label doesn't exist on the repo, the PR still gets created successfully.

shafty023 · 2026-03-24T19:41:37Z

+
+      // Try LLM-assisted commit first; fall back to executeGitOperations if it
+      // returns null.  Never call both.
+      const llmResult = await attemptLlmCommit(


[P2] Pass committer through to the LLM commit subprocess

When body.committer is present, the new LLM-first flow never forwards that identity into attemptLlmCommit. Any successful LLM-assisted commit is therefore authored with whatever git config happens to exist in the worktree, instead of the requested user, which regresses the previous EXECUTE behavior that set GIT_AUTHOR_*/GIT_COMMITTER_* before committing.

Fixed. attemptLlmCommit now accepts a committer parameter and sets GIT_AUTHOR_NAME/EMAIL and GIT_COMMITTER_NAME/EMAIL as env vars on the spawned claude process, matching the same pattern used by executeGitOperations.

mikeangstadt · 2026-03-24T20:27:52Z

I think this can kill this one too https://app.closedloop.ai/features/FEAT-142

- Clean up LLM scratch files (execution-result.json, pr-body.md) unconditionally after reading, so they never leak into subsequent worktree runs - Make symphony label attachment best-effort on PR creation so repos without the label don't fail after commit+push already succeeded - Pass committer identity through to attemptLlmCommit via GIT_AUTHOR_*/ GIT_COMMITTER_* env vars on the spawned process - Add mkdirSync before writing pr-body.md to .claude/work/ directory - For existing PRs, fetch current body and append metadata footer instead of replacing the entire body with a 2-line stub - Remove stale execution-footer.txt reference (should have been pr-body.md) - Bump version to 0.8.1 Testing: typecheck, lint pass. Pre-existing test failures unchanged. Risks: Low — all changes are in the EXECUTE commit/PR flow.

thadeusb added 2 commits March 24, 2026 11:33

Include artifact slug in LLM commit prompt, PR title, and fallback path

d3a247f

thadeusb marked this pull request as ready for review March 24, 2026 16:50

Bump desktop version to 0.8.0

b3d7e12

closedloop-ai-stage Bot reviewed Mar 24, 2026

View reviewed changes

shafty023 reviewed Mar 24, 2026

View reviewed changes

thadeusb added 2 commits March 24, 2026 16:14

Merge origin/main into symphony/plan-61

ab0037b

thadeusb merged commit b2e3e8c into main Mar 24, 2026
2 checks passed

thadeusb deleted the symphony/plan-61 branch March 24, 2026 21:25

Uh oh!

Conversation

thadeusb commented Mar 24, 2026

Summary

Test plan

Uh oh!

closedloop-ai-stage Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

closedloop-ai-stage Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

closedloop-ai-stage Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

closedloop-ai-stage Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

closedloop-ai-stage Bot Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

closedloop-ai-stage Bot commented Mar 24, 2026

Code Review Summary

Findings

HIGH Issues (should fix)

MEDIUM Issues (consider)

Validation Stats

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikeangstadt commented Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants