docs: add worklog for ralph-wiggum iteration counter fix

docs/worklogs/2026-01-23-ralph-wiggum-iteration-counter-fix.md

@@ -0,0 +1,147 @@
+---
+title: "Fix ralph-wiggum iteration counter stuck at 1"
+date: 2026-01-23
+keywords: [ralph-wiggum, pi-extension, iteration-counter, bug-fix, code-review]
+commits: 0
+compression_status: uncompressed
+---
+
+# Session Summary
+
+**Date:** 2026-01-23
+**Focus Area:** Fixing ralph-wiggum extension iteration counter bug
+
+# Accomplishments
+
+- [x] Diagnosed root cause of iteration counter staying stuck at 1
+- [x] Fixed `ralph_done` tool to increment iteration before `hasPendingMessages()` check
+- [x] Applied code review findings (4 issues fixed)
+- [x] Closed issue skills-s984
+
+# Key Decisions
+
+## Decision 1: Always increment iteration, defer prompt delivery
+
+- **Context:** The `hasPendingMessages()` guard was blocking iteration increment entirely, causing counter to stay at 1
+- **Options considered:**
+  1. Remove the guard entirely - Risk of duplicate prompts
+  2. Increment first, only skip prompt delivery if pending - Counter accurate, prompts still controlled
+  3. Check for pending USER messages only - Would require API change
+  4. Use re-entry flag - More complex, doesn't solve root cause
+- **Rationale:** Option 2 keeps counter accurate while still preventing duplicate prompt delivery
+- **Impact:** Iteration counter now reflects actual progress; UI stays accurate; reflection checkpoints work
+
+## Decision 2: Move reflection calculation before pending check
+
+- **Context:** If reflection was due but pending messages blocked, reflection was skipped entirely (not deferred)
+- **Rationale:** Reflection state should be recorded regardless of prompt delivery
+- **Impact:** `lastReflectionAt` is now set even when prompt is deferred; message indicates "(reflection pending)"
+
+# Problems & Solutions
+
+| Problem | Solution | Learning |
+|---------|----------|----------|
+| `hasPendingMessages()` always true during normal operation | Move increment before the check, save state in both branches | Guards can block more than intended; order matters |
+| Reflection tracking skipped when prompt deferred | Calculate `needsReflection` before pending check, update `lastReflectionAt` in both paths | Side effects should happen before conditional returns |
+| Error message "not active" was ambiguous | Split into two checks: null state vs wrong status, with specific messages | Better error messages help debugging |
+| Magic expression `state.iteration - 1` repeated | Extract to `const completedIteration` | DRY applies to expressions too |
+
+# Technical Details
+
+## Code Changes
+
+- Total files modified: 1
+- Key files changed:
+  - `~/.pi/agent/extensions/ralph-wiggum/index.ts` - Fixed `ralph_done` tool execute function
+
+## Before (broken)
+
+```typescript
+if (ctx.hasPendingMessages()) {
+  return { content: [{ type: "text", text: "Pending messages already queued. Skipping ralph_done." }], details: {} };
+}
+
+// Increment iteration - NEVER REACHED when pending messages exist
+state.iteration++;
+```
+
+## After (fixed)
+
+```typescript
+// Always increment iteration to keep counter accurate
+state.iteration++;
+const completedIteration = state.iteration - 1;
+
+// Check max iterations before anything else
+if (state.maxIterations > 0 && state.iteration > state.maxIterations) {
+  completeLoop(ctx, state, ...);
+  return { ... };
+}
+
+// Calculate reflection status before pending check (so we don't skip it)
+const needsReflection = state.reflectEvery > 0 && completedIteration % state.reflectEvery === 0;
+if (needsReflection) state.lastReflectionAt = state.iteration;
+
+// If there are pending messages, record progress but skip prompt delivery
+if (ctx.hasPendingMessages()) {
+  saveState(ctx, state);
+  updateUI(ctx);
+  const reflectNote = needsReflection ? " (reflection pending)" : "";
+  return {
+    content: [{ type: "text", text: `Iteration ${completedIteration} complete.${reflectNote} Prompt deferred - pending messages.` }],
+    details: {},
+  };
+}
+```
+
+## Code Review Findings Fixed
+
+1. **[ERROR] MED** - Split null/inactive state checks with specific error messages
+2. **[SMELL] LOW** - Extracted `completedIteration` constant
+3. **[SMELL] LOW** - Standardized message format ("complete" in both paths)
+4. **[STRUCTURAL]** - Moved reflection calculation before pending-messages check
+
+# Process and Workflow
+
+## What Worked Well
+
+- Issue had detailed bug report with root cause analysis already done (docs/work/2026-01-22-ralph-iteration-counter-bug.md)
+- Code review lenses caught additional issues beyond the primary fix
+- Fixing all issues in one pass
+
+## What Was Challenging
+
+- Extension not in git repo (deployed separately), couldn't easily diff changes
+- No TypeScript compiler readily available to validate syntax
+
+# Learning and Insights
+
+## Technical Insights
+
+- `ctx.hasPendingMessages()` returns true whenever tool responses are queued - essentially always during multi-tool agent operation
+- Guards that block early can prevent important state updates from happening
+- The order of operations matters: increment → check limits → calculate side effects → check guards → deliver
+
+## Architectural Insights
+
+- The "always save state, optionally defer prompt" pattern is cleaner than "guard blocks everything"
+- Error messages should distinguish between different failure modes
+
+# Context for Future Work
+
+## Open Questions
+
+- Should there be a mechanism to force prompt delivery even with pending messages?
+- Should deferred reflections trigger on the next non-deferred iteration?
+
+## Next Steps
+
+- Test the fix in a real ralph loop session
+- Consider adding this pattern to pi extension documentation
+
+# Session Metrics
+
+- Commits made: 0 (not yet committed)
+- Files touched: 1
+- Lines added/removed: ~30/~20
+- Issues closed: 1 (skills-s984)