fix: tolerate plan file rename during execution#342
Merged
Conversation
The framework already moves the plan to completed/ at end of run via MovePlanToCompleted, gated by the move_plan_on_completion config option. The LLM-task move was redundant and the source of mid-run rename risk when the model wrote a different basename than the file on disk. Add a tripwire test in pkg/config/prompts_test.go that asserts the embedded template no longer contains any path-relocation substring (move this plan, completed/, mv, relocate), catching reworded re-introductions, not just the verbatim original. Related to #341
When an LLM-driven task renames the plan file mid-run (e.g. dashed YYYY-MM-DD-<slug>.md becomes compact YYYYMMDD-<slug>.md after a git mv), the task loop used to spin forever: resolvePlanFilePath fell back to completed/<exact-basename> only, hasUncompletedTasks returned true on every ParsePlanFile error including fs.ErrNotExist, and MovePlanToCompleted's idempotency check missed the renamed destination. Three layers of defense, all using the same dashed/compact swap: - resolvePlanFilePath probes the alternate-format basename in both the original directory (in-place rename) and completed/ (moved and renamed) after the existing exact-basename probes. - hasUncompletedTasks distinguishes fs.ErrNotExist from other parse errors and returns false when the file is truly gone, letting the ALL_TASKS_DONE signal exit the loop. Last-line-of- defense, runs after resolvePlanFilePath has exhausted all probes. - MovePlanToCompleted treats a renamed destination as already-moved for idempotency. When the in-place renamed source collides with a stale completed/<altBase> copy, surface as already-completed and leave both files for manual resolution rather than clobbering. - pkg/web/loadPlanWithFallback gets the same alt-format fallback so dashboard plan loading recovers from the same rename. altDateFormatBasename on *Service and tryAlternateDateFormat on *Runner are pure string transformations with no I/O. Related to #341
Note the two complementary layers in CLAUDE.md under Key Patterns: make_plan.txt no longer asks the LLM to move the plan because the framework already moves it idempotently, and the runtime path resolution recovers when an older custom prompt or freelance LLM edit renames the plan across dashed/compact date conventions. Related to #341
There was a problem hiding this comment.
Pull request overview
Fixes the task-phase infinite loop that occurs when a plan file gets renamed across dashed/compact date formats during a move to completed/, making the original configured PlanFile path unresolvable.
Changes:
- Removes the redundant “move plan to completed/” checkbox from the embedded
make_plan.txtprompt and adds a regression “tripwire” test to prevent reintroduction. - Adds alternate date-format probing (
YYYY-MM-DD↔YYYYMMDD) to plan-path resolution and plan loading, and treatsfs.ErrNotExistas “no uncompleted tasks” when a completion signal is received to prevent loop spinning. - Makes
MovePlanToCompletedtolerant of rename + move (and in-place rename) scenarios, with collision avoidance behavior covered by new tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/web/plan.go | Adds alt-date basename probing to web plan loader fallback logic. |
| pkg/web/server_test.go | Adds tests covering alt-date fallback loading from completed/. |
| pkg/processor/prompts.go | Extends plan path resolution to probe alternate date-format renames (in-place and under completed/). |
| pkg/processor/prompts_test.go | Adds coverage for renamed/moved plan resolution and the date-swap helper. |
| pkg/processor/runner.go | Prevents task loop spinning by treating missing plan file as “complete” on completion-signal verification. |
| pkg/processor/runner_test.go | Adds test ensuring missing plan file does not emit warnings and returns “no uncompleted tasks”. |
| pkg/git/service.go | Makes MovePlanToCompleted rename-tolerant (including in-place rename and collision cases) via target resolution helper. |
| pkg/git/service_test.go | Adds comprehensive tests for rename/move/idempotency/collision behaviors in plan moves. |
| pkg/config/defaults/prompts/make_plan.txt | Removes the prompt checkbox that instructs the LLM to move the plan file. |
| pkg/config/prompts_test.go | Adds tripwire regression test preventing prompt reintroduction of LLM-driven plan relocation. |
| docs/plans/completed/20260512-resolve-renamed-plan.md | Documents the issue, root cause, and implemented defense/prevention layers. |
| CLAUDE.md | Documents the two-layer mitigation strategy and the relevant runtime behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+50
to
69
| dir := filepath.Dir(path) | ||
| base := filepath.Base(path) | ||
| completedPath := filepath.Join(dir, "completed", base) | ||
| if p, err := plan.ParsePlanFile(completedPath); err == nil { | ||
| return p, nil | ||
| } else if !errors.Is(err, fs.ErrNotExist) { | ||
| return nil, fmt.Errorf("load plan with fallback: %w", err) | ||
| } | ||
|
|
||
| if altBase := altDateFormatBasename(base); altBase != "" { | ||
| altCompletedPath := filepath.Join(dir, "completed", altBase) | ||
| if p, err := plan.ParsePlanFile(altCompletedPath); err == nil { | ||
| return p, nil | ||
| } else if !errors.Is(err, fs.ErrNotExist) { | ||
| return nil, fmt.Errorf("load plan with fallback: %w", err) | ||
| } | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("load plan with fallback: %w", fs.ErrNotExist) | ||
| } |
Comment on lines
+8
to
+32
| "regexp" | ||
|
|
||
| "github.com/umputun/ralphex/pkg/plan" | ||
| ) | ||
|
|
||
| // dashedDatePattern matches YYYY-MM-DD-<rest>.md basenames (dashed convention). | ||
| // intentionally duplicated from pkg/processor/prompts.go and pkg/git/service.go: | ||
| // pkg/web is a leaf consumer and importing higher-level packages just to share a | ||
| // ~10-line helper would invert the dependency direction. | ||
| var dashedDatePattern = regexp.MustCompile(`^(\d{4})-(\d{2})-(\d{2})-(.+\.md)$`) | ||
|
|
||
| // compactDatePattern matches YYYYMMDD-<rest>.md basenames (compact convention). | ||
| var compactDatePattern = regexp.MustCompile(`^(\d{8})-(.+\.md)$`) | ||
|
|
||
| // altDateFormatBasename returns the same basename with the date prefix swapped | ||
| // between dashed (YYYY-MM-DD) and compact (YYYYMMDD) conventions, or "" if name | ||
| // matches neither pattern. | ||
| func altDateFormatBasename(name string) string { | ||
| if m := dashedDatePattern.FindStringSubmatch(name); m != nil { | ||
| return m[1] + m[2] + m[3] + "-" + m[4] | ||
| } | ||
| if m := compactDatePattern.FindStringSubmatch(name); m != nil { | ||
| d := m[1] | ||
| return d[0:4] + "-" + d[4:6] + "-" + d[6:8] + "-" + m[2] | ||
| } |
Comment on lines
+939
to
942
| // returns false if the plan file is missing after resolvePlanFilePath exhausts all probes | ||
| // (original, completed/<basename>, completed/<alt-date-basename>), to avoid spinning the loop | ||
| // when an LLM-driven git mv renamed the file out from under the runtime. | ||
| func (r *Runner) hasUncompletedTasks() bool { |
Comment on lines
+518
to
+525
| // resolvePlanMoveTargets determines the source and destination for MovePlanToCompleted, | ||
| // accounting for files already moved to completed/ or renamed between the dashed | ||
| // (YYYY-MM-DD) and compact (YYYYMMDD) date-prefix conventions. Returns done=true when | ||
| // the file is already in completed/ (with either basename) and no move is needed. | ||
| // Probe order mirrors resolvePlanFilePath in pkg/processor/prompts.go: the in-place | ||
| // alternate source is checked before any completed/ probe so a current renamed file | ||
| // wins over a stale completed/ copy left from a prior run. | ||
| func (s *Service) resolvePlanMoveTargets(planFile, completedDir string) (sourceFile, destPath string, done bool) { |
Move the YYYY-MM-DD or YYYYMMDD basename swap into pkg/plan as AltDateBasename, exported once and consumed by pkg/processor, pkg/git, and pkg/web. All three packages already imported pkg/plan, so the previous "intentionally duplicated to avoid coupling" rationale was wrong on premise. Drops three local copies of the regex pair, the corresponding helpers (standalone function and Service/Runner methods), and their tests. Also addresses Copilot review findings on #342: - pkg/web/loadPlanWithFallback's not-found error now reports the paths it actually probed, instead of just fs.ErrNotExist. - pkg/processor/runner.go and pkg/git/service.go doc comments on hasUncompletedTasks and resolvePlanMoveTargets now describe the actual probe sequence and the dual meaning of done=true (already in completed/ OR collision-skip).
Deploying ralphex with
|
| Latest commit: |
b87cdb2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://75af8feb.ralphex.pages.dev |
| Branch Preview URL: | https://resolve-renamed-plan.ralphex.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes the task-phase infinite loop reported in #341.
when claude's
git mvduring the "move plan to completed/" task also renames the plan file (e.g. dashed2026-05-12-foo.mdbecomes compact20260512-foo.md), ralphex's frozenr.cfg.PlanFileno longer resolves:resolvePlanFilePathprobes the original andcompleted/<original-basename>, both miss.hasUncompletedTasksthen returns true on everyALL_TASKS_DONEsignal until--max-iterations.two layers of fix.
prevention: drops the redundant
- [ ] move this plan to docs/plans/completed/frommake_plan.txt. The framework already moves the plan idempotently at end of run viaMovePlanToCompleted. A tripwire test inprompts_test.gocatches reworded re-introductions.defense:
resolvePlanFilePath,hasUncompletedTasks,MovePlanToCompleted, andloadPlanWithFallbackall probe the alternate-format basename (YYYY-MM-DD or YYYYMMDD) before giving up.hasUncompletedTasksdistinguishesfs.ErrNotExistfrom other parse errors and treats a missing file as done so the signal can exit the loop. Covers in-flight plans, manual edits, and any future LLM rename source.Related to #341