Skip to content

Commit d3d6b6b

Browse files
federiconericlaude
andcommitted
fix(tui): prevent loop re-spawn on action selection, improve loop lifecycle
Three fixes for the feature loop: 1. RunScreen re-spawned the loop process after action selection because refreshStatus (which depends on actionRequest) was in the spawn useEffect's dependency array. Use a ref to decouple poll scheduling from process spawning. 2. Add "Done — end loop" action choice as the default. Remove redundant "Push and create PR" since auto/merge modes already create the PR. 3. Review fix iterations used PROMPT.md which triggers the finishing-a-development-branch skill when all tasks are complete. Replace with an inline prompt focused solely on fixing review issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 68deb27 commit d3d6b6b

2 files changed

Lines changed: 31 additions & 17 deletions

File tree

src/templates/scripts/feature-loop.sh.tmpl

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,14 @@ write_action_request() {
148148
cat > "$action_file" << 'EOF'
149149
{
150150
"id": "post_pr_choice",
151-
"prompt": "Implementation complete. What would you like to do?",
151+
"prompt": "Loop complete. What would you like to do?",
152152
"choices": [
153+
{"id": "done", "label": "Done — end loop"},
153154
{"id": "merge_local", "label": "Merge back to main locally"},
154-
{"id": "push_pr", "label": "Push and create PR"},
155155
{"id": "keep_branch", "label": "Keep branch as-is"},
156156
{"id": "discard", "label": "Discard this work"}
157157
],
158-
"default": "keep_branch"
158+
"default": "done"
159159
}
160160
EOF
161161
echo "Action request written: $action_file"
@@ -432,7 +432,11 @@ elif [ "$REVIEW_MODE" = "merge" ]; then
432432

433433
if [ $REVIEW_ATTEMPT -lt $MAX_REVIEW_ATTEMPTS ]; then
434434
echo "Review found issues. Running fix iteration..."
435-
cat "$PROMPTS_DIR/PROMPT.md" | envsubst | $CLAUDE_CMD_IMPL 2>&1 | tee "$CLAUDE_OUTPUT" || true
435+
echo "Fix the issues found in the code review above. Run git diff main to see the current changes, then:
436+
1. Fix each issue referenced in the review
437+
2. Run tests: npm test
438+
3. Commit and push the fixes
439+
Do NOT propose completion options or ask interactive questions. Just fix, test, commit, push." | $CLAUDE_CMD_IMPL 2>&1 | tee "$CLAUDE_OUTPUT" || true
436440
parse_and_accumulate_tokens "$CLAUDE_OUTPUT"
437441
fi
438442
done
@@ -460,7 +464,11 @@ else
460464

461465
if [ $REVIEW_ATTEMPT -lt $MAX_REVIEW_ATTEMPTS ]; then
462466
echo "Review found issues. Running fix iteration..."
463-
cat "$PROMPTS_DIR/PROMPT.md" | envsubst | $CLAUDE_CMD_IMPL 2>&1 | tee "$CLAUDE_OUTPUT" || true
467+
echo "Fix the issues found in the code review above. Run git diff main to see the current changes, then:
468+
1. Fix each issue referenced in the review
469+
2. Run tests: npm test
470+
3. Commit and push the fixes
471+
Do NOT propose completion options or ask interactive questions. Just fix, test, commit, push." | $CLAUDE_CMD_IMPL 2>&1 | tee "$CLAUDE_OUTPUT" || true
464472
parse_and_accumulate_tokens "$CLAUDE_OUTPUT"
465473
fi
466474
done
@@ -478,15 +486,15 @@ echo "User chose: $CHOSEN_ACTION"
478486

479487
# Dispatch based on user choice
480488
case "$CHOSEN_ACTION" in
489+
done)
490+
echo "Loop complete. Exiting."
491+
;;
481492
merge_local)
482493
echo "Merging back to main locally..."
483494
git checkout main 2>/dev/null || git checkout master
484495
git merge --squash "$BRANCH" && git commit -m "feat($FEATURE): squash merge from $BRANCH"
485496
echo "Merged. You can delete the branch with: git branch -D $BRANCH"
486497
;;
487-
push_pr)
488-
echo "Branch and PR already created during review phase."
489-
;;
490498
discard)
491499
echo "Discarding work on branch $BRANCH..."
492500
git checkout main 2>/dev/null || git checkout master

src/tui/screens/RunScreen.tsx

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,7 @@ export function RunScreen({
355355
} else {
356356
// File cleaned up by shell — reset tracking so future requests work
357357
handledActionIdRef.current = null;
358-
if (actionRequest) {
359-
setActionRequest(null);
360-
}
358+
setActionRequest((prev) => prev ? null : prev);
361359
}
362360

363361
// In monitor mode, detect completion (only fire once)
@@ -402,7 +400,12 @@ export function RunScreen({
402400
logger.error(`Failed to persist summary file for ${featureName}: ${err instanceof Error ? err.message : String(err)}`);
403401
});
404402
}
405-
}, [featureName, projectRoot, monitorOnly, actionRequest]);
403+
}, [featureName, projectRoot, monitorOnly]);
404+
405+
// Keep a stable ref to the latest refreshStatus so the spawn effect
406+
// can schedule polls without re-running when refreshStatus changes.
407+
const refreshStatusRef = useRef(refreshStatus);
408+
useEffect(() => { refreshStatusRef.current = refreshStatus; }, [refreshStatus]);
406409

407410
const stopLoop = useCallback(() => {
408411
stopRequestedRef.current = true;
@@ -455,11 +458,11 @@ export function RunScreen({
455458
}
456459
if (cancelled) return;
457460
setIsStarting(false);
458-
refreshStatus().catch((err) => {
461+
refreshStatusRef.current().catch((err: unknown) => {
459462
logger.warn(`Status refresh failed: ${err instanceof Error ? err.message : String(err)}`);
460463
});
461464
pollTimer = setInterval(() => {
462-
refreshStatus().catch((err) => {
465+
refreshStatusRef.current().catch((err: unknown) => {
463466
logger.warn(`Status refresh failed: ${err instanceof Error ? err.message : String(err)}`);
464467
});
465468
}, POLL_INTERVAL_MS);
@@ -548,12 +551,12 @@ export function RunScreen({
548551
}
549552

550553
pollTimer = setInterval(() => {
551-
refreshStatus().catch((err) => {
554+
refreshStatusRef.current().catch((err: unknown) => {
552555
logger.warn(`Status refresh failed: ${err instanceof Error ? err.message : String(err)}`);
553556
});
554557
}, POLL_INTERVAL_MS);
555558

556-
refreshStatus().catch((err) => {
559+
refreshStatusRef.current().catch((err: unknown) => {
557560
logger.warn(`Status refresh failed: ${err instanceof Error ? err.message : String(err)}`);
558561
});
559562

@@ -634,7 +637,10 @@ export function RunScreen({
634637
isMountedRef.current = false;
635638
if (pollTimer) clearInterval(pollTimer);
636639
};
637-
}, [featureName, projectRoot, refreshStatus, monitorOnly, sessionState.config]);
640+
// Note: refreshStatusRef (not refreshStatus) is used inside to avoid re-spawning
641+
// the child process when the callback identity changes due to actionRequest updates.
642+
// eslint-disable-next-line react-hooks/exhaustive-deps
643+
}, [featureName, projectRoot, monitorOnly, sessionState.config]);
638644

639645
const totalTasks = tasks.tasksDone + tasks.tasksPending;
640646
const totalE2e = tasks.e2eDone + tasks.e2ePending;

0 commit comments

Comments
 (0)