Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

Commit 8436f00

Browse files
committed
PLAN-61: Address PR review feedback
- 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.
1 parent ab0037b commit 8436f00

2 files changed

Lines changed: 68 additions & 28 deletions

File tree

apps/desktop/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "desktop",
3-
"version": "0.8.0",
3+
"version": "0.8.1",
44
"description": "ClosedLoop Desktop",
55
"author": "ClosedLoop AI <support@closedloop.ai>",
66
"private": true,

apps/desktop/src/server/operations/symphony-loop.ts

Lines changed: 67 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { execSync, spawn } from "node:child_process";
22
import { gatewayLog } from "../../main/gateway-logger.js";
33
import crypto from "node:crypto";
4-
import { closeSync, existsSync, openSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";
4+
import { closeSync, existsSync, mkdirSync, openSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";
55
import fs from "node:fs/promises";
66
import os from "node:os";
77
import path from "node:path";
@@ -677,7 +677,8 @@ async function attemptLlmCommit(
677677
loopId: string,
678678
command: string,
679679
artifactSlug: string | undefined,
680-
webAppOrigin: string
680+
webAppOrigin: string,
681+
committer: LoopCommitter | undefined
681682
): Promise<ExecutionResult | null> {
682683
// Build metadata footer for PR body
683684
// Strip newlines from user-controlled fields to prevent prompt injection
@@ -750,12 +751,20 @@ async function attemptLlmCommit(
750751

751752
loopLog(loopId, "Attempting LLM-assisted commit...");
752753

754+
const spawnEnv: Record<string, string> = { ...process.env } as Record<string, string>;
755+
if (committer) {
756+
spawnEnv.GIT_AUTHOR_NAME = committer.name;
757+
spawnEnv.GIT_AUTHOR_EMAIL = committer.email;
758+
spawnEnv.GIT_COMMITTER_NAME = committer.name;
759+
spawnEnv.GIT_COMMITTER_EMAIL = committer.email;
760+
}
761+
753762
let child: ReturnType<typeof spawn>;
754763
try {
755764
child = spawn(
756765
"claude",
757766
["-p", prompt, "--allowedTools", "Bash,Read,Write,Glob,Grep"],
758-
{ cwd: worktreeDir, detached: true, stdio: "pipe" }
767+
{ cwd: worktreeDir, detached: true, stdio: "pipe", env: spawnEnv }
759768
);
760769
} catch (err) {
761770
loopError(loopId, "LLM commit spawn failed:", err);
@@ -822,22 +831,27 @@ async function attemptLlmCommit(
822831
return;
823832
}
824833

825-
// Read execution-result.json written by the LLM
834+
// Read execution-result.json written by the LLM, then clean up scratch
835+
// files unconditionally so they never leak into subsequent worktree runs.
826836
const resultFilePath = path.join(worktreeDir, "execution-result.json");
837+
const prBodyFilePath = path.join(worktreeDir, "pr-body.md");
838+
let result: ExecutionResult | null = null;
827839
try {
828840
const raw = readFileSync(resultFilePath, "utf-8");
829841
const parsed: unknown = JSON.parse(raw);
830842
if (isExecutionResult(parsed)) {
831843
loopLog(loopId, `LLM commit wrote execution-result.json, pr=${parsed.prUrl}`);
832-
resolve(parsed);
833-
return;
844+
result = parsed;
845+
} else {
846+
loopError(loopId, "LLM execution-result.json failed type guard, returning null");
834847
}
835-
loopError(loopId, "LLM execution-result.json failed type guard, returning null");
836-
resolve(null);
837848
} catch (err) {
838849
loopError(loopId, "LLM commit: failed to read execution-result.json:", err);
839-
resolve(null);
840850
}
851+
// Always remove LLM scratch files from the worktree
852+
try { unlinkSync(resultFilePath); } catch { /* may not exist */ }
853+
try { unlinkSync(prBodyFilePath); } catch { /* may not exist */ }
854+
resolve(result);
841855
});
842856

843857
child.on("error", (err: Error) => {
@@ -936,6 +950,7 @@ function executeGitOperations(
936950
: "";
937951
const prBody = `Loop ID: ${loopId}\nCommand: ${command}${artifactLine}`;
938952
const bodyFile = path.join(worktreeDir, ".claude", "work", "pr-body.md");
953+
mkdirSync(path.dirname(bodyFile), { recursive: true });
939954
writeFileSync(bodyFile, prBody);
940955

941956
// Check for existing PR before creating (handles retries gracefully)
@@ -956,10 +971,12 @@ function executeGitOperations(
956971
prUrl = parsed.url;
957972
prNumber = parsed.number;
958973
} catch {
959-
// No existing PR — create one using --body-file to avoid shell escaping
974+
// No existing PR — create one using --body-file to avoid shell escaping.
975+
// Create without --label first so the PR still succeeds on repos where the
976+
// 'symphony' label doesn't exist yet, then attach the label best-effort.
960977
const prTitle = `${commitPrefix}Symphony: ${command} -- loop ${shortId}`;
961978
const prOutput = execSync(
962-
`gh pr create --title ${shellEscape(prTitle)} --body-file ${shellEscape(bodyFile)} --base ${shellEscape(baseBranch)} --label symphony`,
979+
`gh pr create --title ${shellEscape(prTitle)} --body-file ${shellEscape(bodyFile)} --base ${shellEscape(baseBranch)}`,
963980
{
964981
cwd: worktreeDir,
965982
encoding: "utf-8",
@@ -971,21 +988,40 @@ function executeGitOperations(
971988
prUrl = prOutput;
972989
const prNumberMatch = /\/pull\/(\d+)/.exec(prUrl);
973990
prNumber = prNumberMatch ? Number.parseInt(prNumberMatch[1], 10) : 0;
991+
992+
// Best-effort label attachment — non-fatal if the label doesn't exist
993+
if (prNumber) {
994+
try {
995+
execSync(`gh pr edit ${prNumber} --add-label symphony`, {
996+
cwd: worktreeDir,
997+
stdio: "pipe",
998+
env,
999+
timeout: 15_000,
1000+
});
1001+
} catch {
1002+
// Label may not exist on this repo — not critical
1003+
}
1004+
}
9741005
}
9751006

976-
// Guarantee metadata footer on the PR body (covers both new and existing PRs).
977-
// For new PRs this is a no-op since we just created it with the body.
978-
// For existing PRs this ensures the footer is always present.
1007+
// Ensure the metadata footer is present on the PR body. For existing PRs,
1008+
// fetch the current body and append the metadata instead of replacing it.
9791009
try {
980-
execSync(
981-
`gh pr edit ${prNumber} --body-file ${shellEscape(bodyFile)}`,
982-
{
983-
cwd: worktreeDir,
984-
stdio: "pipe",
985-
env,
986-
timeout: 15_000,
987-
}
988-
);
1010+
const currentBody = execSync(
1011+
`gh pr view ${prNumber} --json body --jq .body`,
1012+
{ cwd: worktreeDir, encoding: "utf-8", stdio: "pipe", env, timeout: 15_000 }
1013+
).trim();
1014+
// Only update if the footer isn't already present
1015+
if (!currentBody.includes(`Loop ID: ${loopId}`)) {
1016+
const updatedBody = currentBody
1017+
? `${currentBody}\n\n---\n${prBody}`
1018+
: prBody;
1019+
writeFileSync(bodyFile, updatedBody);
1020+
execSync(
1021+
`gh pr edit ${prNumber} --body-file ${shellEscape(bodyFile)}`,
1022+
{ cwd: worktreeDir, stdio: "pipe", env, timeout: 15_000 }
1023+
);
1024+
}
9891025
} catch {
9901026
// Non-critical — PR exists, metadata is best-effort
9911027
}
@@ -1070,13 +1106,17 @@ async function handleProcessCompletion(
10701106
loopId,
10711107
command,
10721108
body.artifactSlug,
1073-
webAppOrigin ?? ""
1109+
webAppOrigin ?? "",
1110+
committer
10741111
);
10751112

1076-
// Clean up LLM artifacts before fallback to prevent them from being committed
1113+
// Clean up any remaining LLM scratch files before fallback to prevent
1114+
// them from being committed by executeGitOperations. attemptLlmCommit
1115+
// already cleans up on success, but these guards cover edge cases where
1116+
// the process was killed before the cleanup ran.
10771117
if (!llmResult) {
1078-
try { unlinkSync(path.join(worktreeDir, 'execution-result.json')); } catch { /* file may not exist */ }
1079-
try { unlinkSync(path.join(worktreeDir, 'execution-footer.txt')); } catch { /* file may not exist */ }
1118+
try { unlinkSync(path.join(worktreeDir, 'execution-result.json')); } catch { /* may not exist */ }
1119+
try { unlinkSync(path.join(worktreeDir, 'pr-body.md')); } catch { /* may not exist */ }
10801120
}
10811121

10821122
const gitResult: { prUrl: string; prNumber: number; branchName: string; commitSha: string } | null =

0 commit comments

Comments
 (0)