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

Commit fd46a14

Browse files
thadeusbthadeusb
authored
fix: full ECS harness parity for desktop loop execution (#21)
* fix: full ECS harness parity for desktop loop execution - writeArtifactsForPlan: prompt-first priority matching harness writePrdFile() - writeArtifactsForDecompose: add uppercase types (PRD, FEATURE) + prompt - REQUEST_CHANGES: use claude directly with /code:amend-plan + --resume, not run-loop.sh (matches harness buildClaudeDirectArgs) - pickStableId: use loopId slugified same as harness (not artifact ID) - Completed event: sessionId inside result, add loopId and exitCode - Error event: top-level code/message (not nested error object) * fix: address PR review — consistent error shapes, decompose PRD priority - All error events now use flat { code, message } shape (not nested) - writeArtifactsForDecompose uses PRD > FEATURE priority matching writeArtifactsForPlan and harness writePrdFile()
1 parent d67539b commit fd46a14

1 file changed

Lines changed: 133 additions & 66 deletions

File tree

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

Lines changed: 133 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ interface LoopRequestBody {
5050
committer?: LoopCommitter;
5151
parentBranchName?: string;
5252
parentSessionId?: string;
53-
/** The loop's own session ID from a previous run (for --resume). */
54-
sessionId?: string;
5553
prompt?: string;
5654
}
5755

@@ -213,30 +211,29 @@ function findLocalRepo(
213211

214212
/**
215213
* Resolve worktree directory for a loop.
216-
* Prefers session ID for stable naming (matches ECS harness behavior).
217-
* Falls back to full artifact or loop ID if session ID is unavailable.
214+
* Uses full untruncated stable ID for directory naming.
218215
*/
219216
function resolveLoopWorktreeDir(
220217
expandedRepoPath: string,
221218
stableId: string
222219
): string {
223220
const repoName = path.basename(expandedRepoPath);
224-
const shortId = stableId.slice(0, 8);
225221
return path.join(
226222
resolveWorktreeParentDir(expandedRepoPath),
227-
`${repoName}-loop-${shortId}`
223+
`${repoName}-loop-${stableId}`
228224
);
229225
}
230226

231227
/**
232-
* Pick the best stable ID for worktree naming.
233-
* Priority: sessionId > artifactId > loopId (all full, untruncated UUIDs).
228+
* Pick the stable ID for worktree/branch naming.
229+
* Uses loopId (matching ECS harness branch/run-dir naming).
230+
* Slugified the same way as the harness: lowercase, non-alnum to dashes, max 50 chars.
234231
*/
235232
function pickStableId(body: LoopRequestBody): string {
236-
if (body.sessionId) {
237-
return body.sessionId;
238-
}
239-
return body.artifacts[0]?.id ?? body.loopId;
233+
return body.loopId
234+
.toLowerCase()
235+
.replace(/[^a-z0-9-]/g, "-")
236+
.slice(0, 50);
240237
}
241238

242239
// ---------------------------------------------------------------------------
@@ -407,16 +404,32 @@ function findExistingLoopWorktree(
407404
// Per-command artifact writing
408405
// ---------------------------------------------------------------------------
409406

407+
/**
408+
* Write PRD for PLAN command.
409+
* Matches ECS harness writePrdFile(): prompt first, then PRD artifact, then FEATURE.
410+
*/
410411
async function writeArtifactsForPlan(
411412
claudeWorkDir: string,
412-
artifacts: LoopArtifact[]
413+
artifacts: LoopArtifact[],
414+
prompt?: string
413415
): Promise<void> {
414-
const prdTypes = new Set(["prd", "PRD", "artifact", "FEATURE"]);
415-
for (const artifact of artifacts) {
416-
if (prdTypes.has(artifact.type)) {
417-
await fs.writeFile(path.join(claudeWorkDir, "prd.md"), artifact.content);
416+
// Priority: explicit prompt > PRD artifact > FEATURE artifact (matches harness)
417+
let prdContent = prompt ?? null;
418+
419+
if (!prdContent) {
420+
const prdArtifact = artifacts.find((a) => a.type === "PRD" || a.type === "prd");
421+
const featureArtifact = prdArtifact
422+
? null
423+
: artifacts.find((a) => a.type === "FEATURE" || a.type === "artifact");
424+
const source = prdArtifact ?? featureArtifact;
425+
if (source?.content) {
426+
prdContent = source.content;
418427
}
419428
}
429+
430+
if (prdContent) {
431+
await fs.writeFile(path.join(claudeWorkDir, "prd.md"), prdContent);
432+
}
420433
}
421434

422435
async function writeArtifactsForExecuteOrAmend(
@@ -464,13 +477,26 @@ async function writeArtifactsForExecuteOrAmend(
464477

465478
async function writeArtifactsForDecompose(
466479
tmpDir: string,
467-
artifacts: LoopArtifact[]
480+
artifacts: LoopArtifact[],
481+
prompt?: string
468482
): Promise<void> {
469-
for (const artifact of artifacts) {
470-
if (artifact.type === "prd" || artifact.type === "artifact") {
471-
await fs.writeFile(path.join(tmpDir, "prd.md"), artifact.content);
483+
// Same priority as writeArtifactsForPlan: prompt > PRD > FEATURE
484+
let prdContent = prompt ?? null;
485+
486+
if (!prdContent) {
487+
const prdArtifact = artifacts.find((a) => a.type === "PRD" || a.type === "prd");
488+
const featureArtifact = prdArtifact
489+
? null
490+
: artifacts.find((a) => a.type === "FEATURE" || a.type === "artifact");
491+
const source = prdArtifact ?? featureArtifact;
492+
if (source?.content) {
493+
prdContent = source.content;
472494
}
473495
}
496+
497+
if (prdContent) {
498+
await fs.writeFile(path.join(tmpDir, "prd.md"), prdContent);
499+
}
474500
}
475501

476502
// ---------------------------------------------------------------------------
@@ -696,9 +722,12 @@ async function handleProcessCompletion(
696722

697723
if (exitCode !== 0) {
698724
loopError(loopId, `Process failed with exit code ${exitCode}`);
725+
// Error shape matches ECS harness: top-level code/message, not nested error object
699726
await postLoopEvent(apiBaseUrl, loopId, closedLoopAuthToken, {
700727
type: "error",
701-
error: { code: "PROCESS_FAILED", message: `Process exited with code ${exitCode}` },
728+
code: "PROCESS_FAILED",
729+
message: `Process exited with code ${exitCode}`,
730+
loopId,
702731
});
703732
return;
704733
}
@@ -756,29 +785,32 @@ async function handleProcessCompletion(
756785
const tokensUsed = parseTokenUsage(claudeWorkDir);
757786
loopLog(loopId, `Tokens used: input=${tokensUsed.input}, output=${tokensUsed.output}`);
758787

759-
// Post completed event
760-
const completedEvent: Record<string, unknown> = {
761-
type: "completed",
762-
result: { subtype: command.toLowerCase() },
763-
tokensUsed,
764-
timestamp: new Date().toISOString(),
788+
// Post completed event — shape matches ECS harness reportFinalStatus()
789+
const result: Record<string, unknown> = {
790+
exitCode,
791+
subtype: command.toLowerCase(),
765792
};
766793

767794
if (command === "EXECUTE" && artifacts.executionResult) {
768795
const execResult = artifacts.executionResult as Record<string, unknown>;
769-
completedEvent.result = {
770-
subtype: "execute",
771-
pr_url: execResult.pr_url,
772-
pr_number: execResult.pr_number,
773-
branch_name: execResult.branch_name,
774-
has_changes: execResult.has_changes ?? false,
775-
};
796+
result.pr_url = execResult.pr_url;
797+
result.pr_number = execResult.pr_number;
798+
result.branch_name = execResult.branch_name;
799+
result.has_changes = execResult.has_changes ?? false;
776800
}
777801

802+
// sessionId inside result (matches harness)
778803
if (metadata.sessionId) {
779-
completedEvent.sessionId = metadata.sessionId;
804+
result.sessionId = metadata.sessionId;
780805
}
781806

807+
const completedEvent: Record<string, unknown> = {
808+
type: "completed",
809+
result,
810+
tokensUsed,
811+
loopId,
812+
};
813+
782814
loopLog(loopId, "Posting completed event...");
783815
await postLoopEvent(apiBaseUrl, loopId, closedLoopAuthToken, completedEvent);
784816
loopLog(loopId, "Loop completed successfully");
@@ -842,7 +874,7 @@ async function handleLoopRequest(
842874
// Claim the loopId immediately to prevent concurrent requests from racing
843875
// past the has() check. Replaced with real entry after spawn succeeds.
844876
runningLoops.set(body.loopId, { pid: -1, child: null as unknown as ReturnType<typeof spawn> });
845-
loopLog(body.loopId, `Received ${body.command} request, repo=${body.repo?.fullName ?? "none"}, stableId=${pickStableId(body).slice(0, 8)}, sessionId=${body.sessionId ?? "none"}, parentSessionId=${body.parentSessionId ?? "none"}`);
877+
loopLog(body.loopId, `Received ${body.command} request, repo=${body.repo?.fullName ?? "none"}, stableId=${pickStableId(body)}, parentSessionId=${body.parentSessionId ?? "none"}`);
846878

847879
let spawnedSuccessfully = false;
848880
try {
@@ -879,7 +911,7 @@ async function handleLoopRequest(
879911
);
880912
await fs.mkdir(tmpDir, { recursive: true });
881913
claudeWorkDir = tmpDir;
882-
await writeArtifactsForDecompose(claudeWorkDir, body.artifacts);
914+
await writeArtifactsForDecompose(claudeWorkDir, body.artifacts, body.prompt);
883915
} else if (!expandedRepoPath) {
884916
json(context, 400, {
885917
error: "Repository required for PLAN, EXECUTE, and REQUEST_CHANGES commands",
@@ -900,7 +932,7 @@ async function handleLoopRequest(
900932
throw e;
901933
}
902934
} else {
903-
const loopBranch = `symphony/loop-${pickStableId(body).slice(0, 8)}`;
935+
const loopBranch = `symphony/loop-${pickStableId(body)}`;
904936
worktreeDir = resolveLoopWorktreeDir(expandedRepoPath, pickStableId(body));
905937
await ensureWorktree(
906938
expandedRepoPath,
@@ -912,7 +944,7 @@ async function handleLoopRequest(
912944
}
913945
claudeWorkDir = path.join(worktreeDir, ".claude", "work");
914946
await fs.mkdir(claudeWorkDir, { recursive: true });
915-
await writeArtifactsForPlan(claudeWorkDir, body.artifacts);
947+
await writeArtifactsForPlan(claudeWorkDir, body.artifacts, body.prompt);
916948
} else if (body.command === "EXECUTE" || body.command === "REQUEST_CHANGES") {
917949
// EXECUTE/REQUEST_CHANGES: reuse parent worktree if possible
918950
if (body.parentBranchName) {
@@ -934,7 +966,7 @@ async function handleLoopRequest(
934966
}
935967
if (!worktreeDir) {
936968
// Create new worktree
937-
const loopBranch = `symphony/loop-${pickStableId(body).slice(0, 8)}`;
969+
const loopBranch = `symphony/loop-${pickStableId(body)}`;
938970
worktreeDir = resolveLoopWorktreeDir(expandedRepoPath, pickStableId(body));
939971
await ensureWorktree(
940972
expandedRepoPath,
@@ -960,9 +992,13 @@ async function handleLoopRequest(
960992
return;
961993
}
962994

963-
// Pre-flight: verify required binary exists BEFORE posting 'started' event
995+
// Pre-flight: verify required binary exists BEFORE posting 'started' event.
996+
// PLAN and EXECUTE use run-loop.sh; REQUEST_CHANGES and DECOMPOSE use claude CLI directly.
997+
const usesRunLoop = body.command === "PLAN" || body.command === "EXECUTE";
998+
const usesClaude = body.command === "REQUEST_CHANGES" || body.command === "DECOMPOSE";
964999
let scriptPath: string | null = null;
965-
if (body.command === "DECOMPOSE") {
1000+
1001+
if (usesClaude) {
9661002
try {
9671003
execSync("which claude", { stdio: "pipe", timeout: 5000 });
9681004
} catch {
@@ -972,13 +1008,13 @@ async function handleLoopRequest(
9721008
body.closedLoopAuthToken,
9731009
{
9741010
type: "error",
975-
error: { code: "BINARY_NOT_FOUND", message: "claude CLI not found in PATH" },
1011+
code: "BINARY_NOT_FOUND", message: "claude CLI not found in PATH",
9761012
}
9771013
);
9781014
json(context, 500, { error: "claude CLI not found in PATH" });
9791015
return;
9801016
}
981-
} else {
1017+
} else if (usesRunLoop) {
9821018
scriptPath = findPluginScript("code", "run-loop.sh");
9831019
if (!scriptPath) {
9841020
await postLoopEvent(
@@ -987,7 +1023,7 @@ async function handleLoopRequest(
9871023
body.closedLoopAuthToken,
9881024
{
9891025
type: "error",
990-
error: { code: "SCRIPT_NOT_FOUND", message: "run-loop.sh not found in plugin cache" },
1026+
code: "SCRIPT_NOT_FOUND", message: "run-loop.sh not found in plugin cache",
9911027
}
9921028
);
9931029
json(context, 500, { error: "run-loop.sh not found in plugin cache" });
@@ -1013,14 +1049,20 @@ async function handleLoopRequest(
10131049
const msg = logErr instanceof Error ? logErr.message : String(logErr);
10141050
await postLoopEvent(body.apiBaseUrl, body.loopId, body.closedLoopAuthToken, {
10151051
type: "error",
1016-
error: { code: "SPAWN_FAILED", message: `Cannot open log file: ${msg}` },
1052+
code: "SPAWN_FAILED", message: `Cannot open log file: ${msg}`,
10171053
});
10181054
json(context, 500, { error: `Cannot open log file: ${msg}` });
10191055
return;
10201056
}
10211057
let child: ReturnType<typeof spawn>;
10221058

10231059
try {
1060+
const spawnEnv: Record<string, string> = {
1061+
...(process.env as Record<string, string>),
1062+
CLOSEDLOOP_WORKDIR: claudeWorkDir,
1063+
PATH: `${process.env.PATH}:/opt/homebrew/bin:/usr/local/bin`,
1064+
};
1065+
10241066
if (body.command === "DECOMPOSE") {
10251067
// DECOMPOSE: write prompt to file and pass via stdin to avoid E2BIG
10261068
const prdContent = readTextFile(path.join(claudeWorkDir, "prd.md")) ?? "";
@@ -1034,33 +1076,58 @@ async function handleLoopRequest(
10341076
cwd: claudeWorkDir,
10351077
detached: true,
10361078
stdio: [promptFd, logFd, logFd],
1037-
env: {
1038-
...process.env,
1039-
PATH: `${process.env.PATH}:/opt/homebrew/bin:/usr/local/bin`,
1040-
},
1079+
env: spawnEnv,
10411080
});
10421081
child.unref();
10431082
} finally {
10441083
closeSync(promptFd);
10451084
}
1046-
} else {
1047-
// PLAN, EXECUTE, REQUEST_CHANGES: spawn run-loop.sh
1048-
const spawnEnv: Record<string, string> = {
1049-
...(process.env as Record<string, string>),
1050-
CLOSEDLOOP_WORKDIR: claudeWorkDir,
1051-
PATH: `${process.env.PATH}:/opt/homebrew/bin:/usr/local/bin`,
1052-
};
1053-
1054-
// Prefer loop's own session ID (re-run/resume), fall back to parent's
1055-
const resumeSessionId = body.sessionId ?? body.parentSessionId;
1056-
if (resumeSessionId) {
1057-
spawnEnv.CLOSEDLOOP_SESSION_ID = resumeSessionId;
1085+
} else if (body.command === "REQUEST_CHANGES") {
1086+
// REQUEST_CHANGES: use claude directly with /code:amend-plan
1087+
// Matches ECS harness buildClaudeDirectArgs() for REQUEST_CHANGES
1088+
const claudeArgs: string[] = [];
1089+
1090+
// Grant tool permissions matching harness
1091+
claudeArgs.push(
1092+
"--allowedTools",
1093+
"Bash,Glob,Grep,Read,Write,Edit,Task,Skill,SlashCommand,TodoWrite",
1094+
"--max-turns",
1095+
"200"
1096+
);
1097+
1098+
// Resume from parent session if available (matches harness --resume)
1099+
if (body.parentSessionId) {
1100+
claudeArgs.push("--resume", body.parentSessionId);
10581101
}
10591102

1103+
// Build /code:amend-plan invocation matching harness
1104+
const promptFile = path.join(claudeWorkDir, "prompt.md");
1105+
let amendPrompt = "Please amend the plan based on the requested changes.";
1106+
if (existsSync(promptFile)) {
1107+
amendPrompt = readFileSync(promptFile, "utf-8");
1108+
}
1109+
// Sanitize prompt matching harness's prepare-message step
1110+
const sanitized = amendPrompt
1111+
.replace(/[\n\r]+/g, " ")
1112+
.replace(/\s{2,}/g, " ")
1113+
.replace(/"/g, '\\"');
1114+
claudeArgs.push(
1115+
`/code:amend-plan --workdir ${claudeWorkDir} --message "${sanitized}"`
1116+
);
1117+
1118+
child = spawn("claude", claudeArgs, {
1119+
cwd: worktreeDir!,
1120+
detached: true,
1121+
stdio: ["ignore", logFd, logFd],
1122+
env: spawnEnv,
1123+
});
1124+
child.unref();
1125+
} else {
1126+
// PLAN, EXECUTE: spawn run-loop.sh
10601127
// Build args matching ECS harness-agent's buildRunLoopArgs():
10611128
// 1. workdir (positional)
1062-
// 2. --max-iterations (EXECUTE=150, others=50)
1063-
// 3. --prd (always, when prd.md exists)
1129+
// 2. --max-iterations (EXECUTE=150, PLAN=50)
1130+
// 3. --prd (when prd.md exists)
10641131
const scriptArgs = [claudeWorkDir];
10651132

10661133
const maxIterations = body.command === "EXECUTE" ? "150" : "50";
@@ -1084,7 +1151,7 @@ async function handleLoopRequest(
10841151
const msg = spawnErr instanceof Error ? spawnErr.message : String(spawnErr);
10851152
await postLoopEvent(body.apiBaseUrl, body.loopId, body.closedLoopAuthToken, {
10861153
type: "error",
1087-
error: { code: "SPAWN_FAILED", message: msg },
1154+
code: "SPAWN_FAILED", message: msg,
10881155
});
10891156
json(context, 500, { error: `Failed to spawn process: ${msg}` });
10901157
return;

0 commit comments

Comments
 (0)