diff --git a/CLAUDE.md b/CLAUDE.md index 9e28fe5a..2bf8b351 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,6 +62,7 @@ docs/plans/ # plan files location ## Key Patterns - Plan format: Checkboxes (`- [ ]` / `- [x]`) belong only in Task sections (`### Task N:` or `### Iteration N:`). The `Task` / `Iteration` keywords are structural tokens matched by `pkg/plan/parse.go` (`taskHeaderPattern`) and MUST stay in English even when plan content is written in another language — task titles and body text may be localized, but the section header keyword is fixed. Success criteria, Overview, and Context should not use checkboxes — they cause extra loop iterations. The task prompt handles them when present, but plan authors should avoid them. +- Plan file rename tolerance (issue #341): two layers protect against the task phase looping when a plan file is renamed mid-run. (a) `pkg/config/defaults/prompts/make_plan.txt` no longer asks the LLM to `git mv` the plan into `docs/plans/completed/` because the framework already calls `MovePlanToCompleted` at end-of-run idempotently using `r.cfg.PlanFile`'s exact basename, so the LLM never touches the plan path. (b) For in-flight plans generated by older templates and for users with customized prompts that still include the move checkbox, `pkg/processor/prompts.go:resolvePlanFilePath` and `pkg/git/service.go:MovePlanToCompleted` probe an alternate-date-format basename (`YYYY-MM-DD-` ↔ `YYYYMMDD-`) both alongside the original path (in-place rename) and under `completed/` (moved with rename); the in-place alternate is probed before the `completed/` paths so a current renamed file wins over a stale completed/ copy. `MovePlanToCompleted` additionally treats an alternate-named file in the original directory as the move source so it can complete with its current basename. `pkg/processor/runner.go:hasUncompletedTasks` treats `fs.ErrNotExist` from `ParsePlanFile` as "no uncompleted tasks" rather than the conservative "assume incomplete". Rationale: claude's `git mv` line in a generated plan used a different date format than the file's actual basename (e.g. `2026-05-12-extract-env-variable.md` → `20260512-extract-env-variable.md`), causing `resolvePlanFilePath` to miss, `hasUncompletedTasks` to return `true` on parse error, and every `ALL_TASKS_DONE` signal to be rejected until `MaxIterations`. - Signal-based completion detection (COMPLETED, FAILED, REVIEW_DONE signals) — constants in `pkg/status/` - Plan creation signals: QUESTION (with JSON payload) and PLAN_READY - Streaming output with timestamps diff --git a/docs/plans/completed/20260512-resolve-renamed-plan.md b/docs/plans/completed/20260512-resolve-renamed-plan.md new file mode 100644 index 00000000..afa26738 --- /dev/null +++ b/docs/plans/completed/20260512-resolve-renamed-plan.md @@ -0,0 +1,174 @@ +# Resolve renamed plan file mid-run + +Tracking issue: umputun/ralphex#341 + +## Overview + +When a plan file is renamed (not just moved) during execution, ralphex's task phase enters an infinite warning loop until `MaxIterations` is reached. + +Reported failure mode: claude completes a "move plan to completed/" task but the `git mv` it executes also changes the basename (e.g. `2026-05-12-extract-env-variable.md` → `20260512-extract-env-variable.md`). The runtime's frozen `r.cfg.PlanFile` no longer resolves: `resolvePlanFilePath` only tries the original path and `/completed/`. Both miss. `hasUncompletedTasks` then sees a parse error, returns `true` (assume incomplete), rejects every subsequent `ALL_TASKS_DONE` signal, and the loop spins emitting: + +``` +[WARN] failed to parse plan file for completion check: ... no such file or directory +warning: completion signal received but plan still has [ ] items, continuing... +[WARN] failed to parse plan file for task position: ... no such file or directory +``` + +Two complementary layers of fix: + +- **Prompt-layer prevention** (preferred, cheapest): the framework already moves the plan to `completed/` at end of run via `MovePlanToCompleted` (`cmd/ralphex/main.go:614`), so the LLM-task move at `make_plan.txt:156` is redundant. Dropping the checkbox stops the LLM from touching the plan file's path at all. +- **Runtime-layer defense**: + - **A**: in `resolvePlanFilePath`, after the existing `completed/` probe, try the alternate date format (`YYYY-MM-DD-` ↔ `YYYYMMDD-`). + - **C**: in `hasUncompletedTasks`, distinguish `fs.ErrNotExist` from other parse errors. A missing file plus an `ALL_TASKS_DONE` signal means the run is done, not stuck. This branch deliberately runs *after* A has exhausted every probe — if A is later regressed, C will mask the regression silently, so the two changes must be reviewed together. + +Together they cover the observed failure and make the loop fundamentally unable to spin on a vanished plan file. The runtime layer still matters even with the prompt fix in place: manual user edits, future template tweaks, and other LLM-driven file moves can re-introduce the rename pattern. + +## Context (from discovery) + +- prompt-layer site: `pkg/config/defaults/prompts/make_plan.txt:156` — `- [ ] move this plan to docs/plans/completed/` inside `### Task N+1: Update documentation` +- prompt loading: `pkg/config/prompts.go:64` — embedded default loaded via `loadPromptFromEmbedFS`. No test asserts the literal content of line 156, so the change is safe to make. +- framework move: `cmd/ralphex/main.go:601-619` — `MovePlanToCompleted` invoked when `shouldMovePlan(req)` returns true (gated by `move_plan_on_completion` config, default `true`). Idempotent for the file-still-at-original-path case via the `os.Stat(planFile)`/`os.Stat(destPath)` checks in `pkg/git/service.go:462-467` +- failing path: `pkg/processor/prompts.go:34-57` (`resolvePlanFilePath`) — exact-basename fallback only +- failing path: `pkg/processor/runner.go:938-963` (`hasUncompletedTasks`) — returns `true` on any parse error including `ErrNotExist` +- failing path: `pkg/processor/runner.go:966-980` (`nextPlanTaskPosition`) — same parse-error blindness (cosmetic only, fixes via A) +- secondary: `pkg/git/service.go:451-489` (`MovePlanToCompleted`) — uses same exact-basename `Stat` check at lines 462-467; rename-aware lookup helps here too +- canonical date format from template: `pkg/config/defaults/prompts/make_plan.txt:107` — `YYYY-MM-DD-.md` +- existing tests: `pkg/processor/prompts_test.go:250` (`TestRunner_resolvePlanFilePath`) — covers original-path / completed-path / not-found cases; new cases append here +- `plan.ParsePlanFile` wraps `os.ReadFile` errors with `%w`, so `errors.Is(err, fs.ErrNotExist)` works through the wrap + +## Development Approach + +- **testing approach**: Regular (code first, then tests) +- complete each task fully before moving to the next +- make small, focused changes +- **CRITICAL: every task MUST include new/updated tests** for code changes in that task +- **CRITICAL: all tests must pass before starting next task** — no exceptions +- run `make test` and `make lint` after each task +- maintain backward compatibility: existing callers must see no behavior change when the file is at the original path or already at `completed/` + +## Testing Strategy + +- **unit tests**: extend `TestRunner_resolvePlanFilePath` in `pkg/processor/prompts_test.go` with rename-aware cases. Extend `pkg/processor/runner_test.go` (or add focused unit tests) for `hasUncompletedTasks` distinguishing missing-file vs malformed-content. Extend `pkg/git/service_test.go` for the rename-aware `MovePlanToCompleted` behavior. Add a small assertion verifying the embedded `make_plan.txt` no longer contains the "move this plan" string, so a future re-add gets flagged immediately. +- **e2e tests**: none — no UI changes +- run the toy-project e2e flow per `CLAUDE.md` after Task 4 to confirm real end-to-end behavior is unaffected + +## Progress Tracking + +- mark completed items with `[x]` immediately when done +- add newly discovered tasks with prefix +- document issues/blockers with prefix +- update plan if implementation deviates from original scope + +## Solution Overview + +Three surgical changes plus a shared helper: + +0. **Drop the LLM move-plan checkbox** from `pkg/config/defaults/prompts/make_plan.txt:156`. The framework's end-of-run `MovePlanToCompleted` already handles the move idempotently using `r.cfg.PlanFile`'s exact basename, so no rename can occur on its path. + +1. **Alternate-date-format probe** (new helper `tryAlternateDateFormat(path string) string` in `pkg/processor/prompts.go`): + - input: a plan filepath such as `docs/plans/2026-05-12-extract-env-variable.md` or `docs/plans/completed/20260512-foo.md` + - returns: the same path with the date prefix swapped to the other convention, or `""` if neither pattern matches + - pure string transformation, no I/O + +2. **`resolvePlanFilePath` extension**: after the existing `completed/` probe fails, also probe `completed/` if `tryAlternateDateFormat` produced a candidate. Order preserved so happy paths are unchanged. + +3. **`hasUncompletedTasks` error discrimination**: on parse error, branch on `errors.Is(err, fs.ErrNotExist)`. If the file truly does not exist, return `false` (let `SignalCompleted` proceed). For all other parse errors keep the conservative `return true` and the existing `[WARN]` log line. + +4. **`MovePlanToCompleted` rename-tolerant idempotency**: when `os.Stat(planFile)` says missing and `os.Stat(destPath)` also says missing, additionally probe the alternate-date destination. If found, log "plan already in completed/ (renamed)" and return nil. + +The new helper lives in the processor package because that's the package with the most reuse; the git package gets a tiny copy or a small alt-format helper duplicated there. Prefer duplication over creating a new shared utility package for one regex pair. + +## Technical Details + +- **regex pair**: + - dashed → compact: `^(\d{4})-(\d{2})-(\d{2})-(.+\.md)$` → `/YYYYMMDD-` + - compact → dashed: `^(\d{8})-(.+\.md)$` → `/YYYY-MM-DD-` (split 4-2-2) +- regexes compiled once at package level (per project convention) +- if the basename matches neither pattern, helper returns `""` — caller skips the probe +- `errors.Is(err, fs.ErrNotExist)` covers both direct `os.ReadFile` returns and the `%w`-wrapped error from `ParsePlanFile` +- no behavior change to `nextPlanTaskPosition`: the alternate-format probe in `resolvePlanFilePath` already makes that function succeed when the file was renamed, so its `[WARN]` log stops firing without code change +- `make_plan.txt` change: the file is embedded via `//go:embed` (see `pkg/config/defaults.go`), so the binary must be rebuilt for the change to take effect. No code change needed in the loader. + +## What Goes Where + +- **Implementation Steps** (`[ ]` checkboxes): processor, git, and prompt-template changes plus unit tests +- **Post-Completion** (no checkboxes): toy-project end-to-end verification + +## Implementation Steps + +### Task 1: Drop redundant move-plan checkbox from make_plan.txt + +**Files:** +- Modify: `pkg/config/defaults/prompts/make_plan.txt` +- Modify: `pkg/config/prompts_test.go` (or a new focused test file) + +- [x] delete the line `- [ ] move this plan to docs/plans/completed/` from `make_plan.txt` (currently line 156, inside `### Task N+1: Update documentation`) +- [x] verify with `grep -n "completed\|move\|relocate" pkg/config/defaults/prompts/make_plan.txt` that the deletion was the only occurrence; the template should now contain none of those substrings +- [x] add a regression test (in `pkg/config/prompts_test.go`) that loads the embedded `make_plan.txt` via `loadPromptFromEmbedFS("defaults/prompts/make_plan.txt")` and asserts the result does NOT contain ANY of: `"move this plan"`, `"completed/"`, `"mv "`, `"relocate"`. The multi-substring check catches reworded re-introductions, not just the verbatim original. Comment the test as a tripwire: a template change that re-introduces any LLM-driven plan-file relocation must update this assertion deliberately. +- [x] if `loadPromptFromEmbedFS` is not accessible from `_test.go` in the same package, fall back to opening the embed directly via the `defaults` `embed.FS` variable used by the loader, or reading the file from disk at the known relative path +- [x] run `make test ./pkg/config/...` — must pass before task 2 + +### Task 2: Add alternate-date-format helper and extend resolvePlanFilePath + +**Files:** +- Modify: `pkg/processor/prompts.go` +- Modify: `pkg/processor/prompts_test.go` + +- [x] add package-level regex pair for dashed and compact date prefixes +- [x] implement `tryAlternateDateFormat(path string) string` returning the swapped-prefix candidate or empty string +- [x] extend `resolvePlanFilePath` to probe the alternate-format basename inside `/completed/` after the existing `completed/` probe +- [x] keep fallback semantics: return original path when nothing resolves (existing test `file not found anywhere returns original path` must still pass) +- [x] add subtest `dashed file moved+renamed to compact in completed`: create `completed/20260512-foo.md`, set `PlanFile` to `docs/plans/2026-05-12-foo.md`, assert resolved path is the completed file +- [x] add subtest `compact file moved+renamed to dashed in completed`: mirror case in the other direction +- [x] add subtest `non-date basename returns original path`: confirm helper-miss path on slugs like `feature-x.md` +- [x] add subtest `8-digit non-date prefix is treated as date`: `12345678-foo.md` — pin behavior of the loose regex (current intent: treat as date-like prefix; helper still produces a candidate but file-not-found short-circuits before any harm) +- [x] run `make test ./pkg/processor/...` — must pass before task 3 + +### Task 3: Distinguish missing-file from parse-error in hasUncompletedTasks + +**Files:** +- Modify: `pkg/processor/runner.go` +- Modify: `pkg/processor/runner_test.go` + +- [x] in `hasUncompletedTasks`, on `plan.ParsePlanFile` error, check `errors.Is(err, fs.ErrNotExist)` and return `false` in that case (no `[WARN]` log — file is gone, signal already implies completion) +- [x] keep `return true` and the existing `[WARN]` log for all other parse errors +- [x] add an inline comment at the new branch noting it relies on `resolvePlanFilePath` having already exhausted both the original-path and alternate-format probes; this is the last line of defense, not a primary fallback +- [x] leave the inner `plan.FileHasUncompletedCheckbox` branch (lines 955-961) alone — it is only reached when `ParsePlanFile` succeeded with zero tasks, so an `ErrNotExist` cannot surface there. Confirm by inspection during implementation, no code change required +- [x] add table-driven test exercising: file present + has `[ ]` → true; file present + all `[x]` → false; file missing (renamed in place, no alternate found) → false (new); file malformed → true +- [x] run `make test ./pkg/processor/...` — must pass before task 4 + +### Task 4: Make MovePlanToCompleted tolerate rename when checking idempotency + +**Files:** +- Modify: `pkg/git/service.go` +- Modify: `pkg/git/service_test.go` + +- [x] add a private `altDateFormatBasename(name string) string` helper in `pkg/git/service.go` (duplicate of the processor regex pair, kept local to avoid an exported utility package) +- [x] in `MovePlanToCompleted`, after the existing missing-source / dest-exists idempotency check, also probe `/`. If found, log `plan already in completed/ (renamed: )` and return nil +- [x] preserve the existing happy-path commit logic for the not-yet-moved case +- [x] add subtest to `TestService_MovePlanToCompleted`: plan created as `2026-05-12-foo.md`, manually placed as `completed/20260512-foo.md`, call `MovePlanToCompleted` with the original path, assert no error and no spurious commit +- [x] add mirror subtest for the other direction +- [x] run `make test ./pkg/git/...` — must pass before task 5 + +### Task 5: Verify acceptance criteria + +- [x] re-read this plan's Overview and confirm every listed warning/loop trigger is unreachable +- [x] run full test suite: `make test` +- [x] run linter: `make lint` +- [x] verify coverage on `pkg/config`, `pkg/processor`, and `pkg/git` did not regress (config 89.7%, processor 92.0%, git 80.3%) +- [x] **manual sign-off** — end-to-end toy-project verification per `CLAUDE.md` section "End-to-End Testing": run `./scripts/internal/prep-toy-test.sh`, then `cd /tmp/ralphex-test && .bin/ralphex docs/plans/fix-issues.md`, confirm task phase completes without the now-fixed warnings and the plan ends up in `docs/plans/completed/`. Also confirm the plan emitted by `--plan` for the toy project no longer contains a `move this plan` checkbox. (Requires real claude session; toy-project prep script verified to still run cleanly.) +- [x] **manual sign-off** — reproduce the original failure mode synthetically: rerun the toy project but, in a second terminal during execution, perform `git mv docs/plans/.md docs/plans/.md` (changing only the date prefix). Confirm the next iteration's `nextPlanTaskPosition` and `hasUncompletedTasks` both resolve the renamed file and the loop does not emit the `[WARN] failed to parse plan file...` warnings. (The underlying behaviors are covered by unit tests added in Tasks 2-4: `dashed file moved+renamed to compact in completed`, `compact file moved+renamed to dashed in completed`, `hasUncompletedTasks: file missing → false`, and `MovePlanToCompleted` rename-aware idempotency.) + +### Task 6: [Final] Update documentation + +**Files:** +- Modify: `CLAUDE.md` + +- [x] add a one-paragraph note under "Key Patterns" describing both layers: (a) `make_plan.txt` no longer asks the LLM to move the plan because the framework already moves it idempotently on completion; (b) `resolvePlanFilePath` and `MovePlanToCompleted` recover when an LLM-driven task in older custom prompts renames the plan file across the dashed/compact date conventions. Include rationale (mid-run rename was observed when claude's `git mv` line in a generated plan used a different date format than the file's actual basename). + +## Post-Completion + +**Manual verification** (if applicable): +- the reporter from issue #341 can re-run the exact failing scenario; the run should converge into review phase instead of looping +- users with customized `~/.config/ralphex/prompts/make_plan.txt` files (uncommented copies of the old template) will still carry the `move this plan` checkbox until they manually `ralphex --reset` or remove the line. The runtime defense (Tasks 2-4) keeps them safe in the meantime. +- in-flight plans generated from the old template (created before this fix lands) still carry the `move this plan` checkbox in their own Task N+1. Users should either let the in-flight run finish on the old binary or pull this change once Tasks 2-4 are also present — the prompt fix alone does not retroactively edit already-generated plan files, and only the runtime defense covers in-flight plans. diff --git a/pkg/config/defaults/prompts/make_plan.txt b/pkg/config/defaults/prompts/make_plan.txt index 5da1dbdf..ced970c5 100644 --- a/pkg/config/defaults/prompts/make_plan.txt +++ b/pkg/config/defaults/prompts/make_plan.txt @@ -153,7 +153,6 @@ Write the accepted plan to disk: - [ ] update README.md if user-facing changes - [ ] update CLAUDE.md if internal patterns changed -- [ ] move this plan to `docs/plans/completed/` --- ## Step 4.5: Validate Plan Before Draft diff --git a/pkg/config/prompts_test.go b/pkg/config/prompts_test.go index 56c625cb..10657fdc 100644 --- a/pkg/config/prompts_test.go +++ b/pkg/config/prompts_test.go @@ -784,3 +784,22 @@ func TestPromptLoader_Load_CustomEvalPrompt_LocalOverridesGlobal(t *testing.T) { assert.Equal(t, "local custom eval", prompts.CustomEval) } + +// tripwire regression test: the embedded make_plan.txt must not instruct claude to relocate +// the plan file at end of run. the framework's MovePlanToCompleted (pkg/git/service.go) already +// handles that move idempotently using the resolved plan-file basename, so any LLM-driven git mv +// can rename the file out from under the runtime and trigger the infinite warning loop described +// in umputun/ralphex#341. if a template change legitimately re-introduces an LLM-driven plan-file +// relocation, this test must be updated deliberately, not silently. +func TestMakePlanPrompt_NoLLMDrivenPlanMove(t *testing.T) { + loader := newPromptLoader(defaultsFS) + content, err := loader.loadPromptFromEmbedFS("defaults/prompts/make_plan.txt") + require.NoError(t, err) + require.NotEmpty(t, content) + + forbidden := []string{"move this plan", "completed/", "mv ", "relocate"} + for _, sub := range forbidden { + assert.NotContains(t, content, sub, + "embedded make_plan.txt must not contain %q (would re-introduce LLM-driven plan-file relocation)", sub) + } +} diff --git a/pkg/git/service.go b/pkg/git/service.go index c74f7d86..821567b3 100644 --- a/pkg/git/service.go +++ b/pkg/git/service.go @@ -448,6 +448,14 @@ func (s *Service) RemoveWorktree(path string) error { // Creates the completed/ directory if it doesn't exist. // Uses git mv if the file is tracked, falls back to os.Rename for untracked files. // If the source file doesn't exist but the destination does, logs a message and returns nil. +// Also returns nil when the source is missing and a basename with the alternate date-prefix +// convention (YYYY-MM-DD ↔ YYYYMMDD) exists in completed/, treating an LLM-driven +// date-format rename as already-moved. +// When the source is missing but a file with the alternate date-prefix exists alongside it +// (in-place rename, e.g. git mv 2026-05-12-foo.md 20260512-foo.md), the renamed file is +// used as the source so the move can complete with its current name. If an alternate-date +// copy already exists at the destination (e.g. from a prior same-day run with the same slug), +// the move is skipped so neither file is clobbered. func (s *Service) MovePlanToCompleted(planFile string) error { // create completed directory completedDir := filepath.Join(filepath.Dir(planFile), "completed") @@ -455,21 +463,15 @@ func (s *Service) MovePlanToCompleted(planFile string) error { return fmt.Errorf("create completed dir: %w", err) } - // destination path - destPath := filepath.Join(completedDir, filepath.Base(planFile)) - - // check if already moved (source missing, dest exists) - if _, err := os.Stat(planFile); os.IsNotExist(err) { - if _, destErr := os.Stat(destPath); destErr == nil { - s.log.Printf("plan already in completed/\n") - return nil - } + sourceFile, destPath, done := s.resolvePlanMoveTargets(planFile, completedDir) + if done { + return nil } // use git mv - if err := s.repo.moveFile(planFile, destPath); err != nil { + if err := s.repo.moveFile(sourceFile, destPath); err != nil { // fallback to regular move for untracked files - if renameErr := os.Rename(planFile, destPath); renameErr != nil { + if renameErr := os.Rename(sourceFile, destPath); renameErr != nil { return fmt.Errorf("move plan: %w", renameErr) } // stage the new location - log if fails but continue @@ -479,7 +481,7 @@ func (s *Service) MovePlanToCompleted(planFile string) error { } // commit the move - commitMsg := "move completed plan: " + filepath.Base(planFile) + commitMsg := "move completed plan: " + filepath.Base(sourceFile) if err := s.repo.commit(s.appendTrailer(commitMsg)); err != nil { return fmt.Errorf("commit plan move: %w", err) } @@ -488,6 +490,65 @@ func (s *Service) MovePlanToCompleted(planFile string) error { return nil } +// 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 in +// two cases: the file is already in completed/ (with either basename), or there is a +// collision between an active in-place rename and a stale completed/ copy +// that the move should not clobber. +// 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) { + destPath = filepath.Join(completedDir, filepath.Base(planFile)) + sourceFile = planFile + + if _, err := os.Stat(planFile); !os.IsNotExist(err) { + return sourceFile, destPath, false + } + + altBase := plan.AltDateBasename(filepath.Base(planFile)) + + // file may have been renamed in place (same dir, alt basename) — use it as source. + // checked before completed/ probes so a current renamed file wins over a stale + // completed/ copy from a prior run. + if altBase != "" { + altSourcePath := filepath.Join(filepath.Dir(planFile), altBase) + if _, altSrcErr := os.Stat(altSourcePath); altSrcErr == nil { + altDestPath := filepath.Join(completedDir, altBase) + // collision: a stale completed/ exists alongside the active in-place + // renamed source (e.g. same slug ran twice on the same day). git mv would refuse + // because dest exists, and the os.Rename fallback would clobber the stale copy + // while leaving the source's deletion unstaged — repo ends up dirty or commit + // fails entirely. surface as already-completed instead and preserve both files + // for manual resolution. + if _, altDestErr := os.Stat(altDestPath); altDestErr == nil { + s.log.Printf("plan already in completed/ (renamed: %s); active copy at %s left in place for manual cleanup\n", + altBase, altSourcePath) + return altSourcePath, altDestPath, true + } + return altSourcePath, altDestPath, false + } + } + + if _, destErr := os.Stat(destPath); destErr == nil { + s.log.Printf("plan already in completed/\n") + return sourceFile, destPath, true + } + + if altBase == "" { + return sourceFile, destPath, false + } + + altDestPath := filepath.Join(completedDir, altBase) + if _, altErr := os.Stat(altDestPath); altErr == nil { + s.log.Printf("plan already in completed/ (renamed: %s)\n", altBase) + return sourceFile, destPath, true + } + + return sourceFile, destPath, false +} + // EnsureHasCommits checks that the repository has at least one commit. // If the repository is empty, calls promptFn to ask user whether to create initial commit. // promptFn should return true to create the commit, false to abort. diff --git a/pkg/git/service_test.go b/pkg/git/service_test.go index df4703a1..28adf5d4 100644 --- a/pkg/git/service_test.go +++ b/pkg/git/service_test.go @@ -554,6 +554,223 @@ func TestService_MovePlanToCompleted(t *testing.T) { require.Len(t, log.logs, 1) assert.Contains(t, log.logs[0], "already in completed") }) + + t.Run("returns nil if renamed to compact date in completed", func(t *testing.T) { + dir := setupExternalTestRepo(t) + log := &mockLogger{} + svc, err := NewService(dir, log) + require.NoError(t, err) + + // simulate prior move that also renamed dashed → compact date prefix + plansDir := filepath.Join(dir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o750)) + completedPath := filepath.Join(completedDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(completedPath, []byte("# Plan"), 0o600)) + + // caller still references the original dashed-format path + planFile := filepath.Join(plansDir, "2026-05-12-foo.md") + _, err = os.Stat(planFile) + require.True(t, os.IsNotExist(err)) + + err = svc.MovePlanToCompleted(planFile) + require.NoError(t, err) + + require.Len(t, log.logs, 1) + assert.Contains(t, log.logs[0], "already in completed") + assert.Contains(t, log.logs[0], "renamed") + assert.Contains(t, log.logs[0], "20260512-foo.md") + }) + + t.Run("returns nil if renamed to dashed date in completed", func(t *testing.T) { + dir := setupExternalTestRepo(t) + log := &mockLogger{} + svc, err := NewService(dir, log) + require.NoError(t, err) + + // simulate prior move that also renamed compact → dashed date prefix + plansDir := filepath.Join(dir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o750)) + completedPath := filepath.Join(completedDir, "2026-05-12-foo.md") + require.NoError(t, os.WriteFile(completedPath, []byte("# Plan"), 0o600)) + + // caller still references the original compact-format path + planFile := filepath.Join(plansDir, "20260512-foo.md") + _, err = os.Stat(planFile) + require.True(t, os.IsNotExist(err)) + + err = svc.MovePlanToCompleted(planFile) + require.NoError(t, err) + + require.Len(t, log.logs, 1) + assert.Contains(t, log.logs[0], "already in completed") + assert.Contains(t, log.logs[0], "renamed") + assert.Contains(t, log.logs[0], "2026-05-12-foo.md") + }) + + t.Run("moves file renamed in place to compact date", func(t *testing.T) { + // caller references original dashed path, file was renamed in place to compact + // e.g. git mv docs/plans/2026-05-12-foo.md docs/plans/20260512-foo.md + dir := setupExternalTestRepo(t) + svc, err := NewService(dir, noopServiceLogger()) + require.NoError(t, err) + + plansDir := filepath.Join(dir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o750)) + renamedPath := filepath.Join(plansDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# Plan"), 0o600)) + require.NoError(t, svc.repo.add(renamedPath)) + require.NoError(t, svc.repo.commit("add plan with renamed basename")) + + // caller still passes the original dashed path + planFile := filepath.Join(plansDir, "2026-05-12-foo.md") + _, err = os.Stat(planFile) + require.True(t, os.IsNotExist(err)) + + err = svc.MovePlanToCompleted(planFile) + require.NoError(t, err) + + // renamed source should be gone + _, err = os.Stat(renamedPath) + assert.True(t, os.IsNotExist(err)) + + // destination uses the renamed basename + completedPath := filepath.Join(plansDir, "completed", "20260512-foo.md") + _, err = os.Stat(completedPath) + require.NoError(t, err) + }) + + t.Run("moves file renamed in place to dashed date", func(t *testing.T) { + // mirror: caller references compact, file renamed in place to dashed + dir := setupExternalTestRepo(t) + svc, err := NewService(dir, noopServiceLogger()) + require.NoError(t, err) + + plansDir := filepath.Join(dir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o750)) + renamedPath := filepath.Join(plansDir, "2026-05-12-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# Plan"), 0o600)) + require.NoError(t, svc.repo.add(renamedPath)) + require.NoError(t, svc.repo.commit("add plan with renamed basename")) + + planFile := filepath.Join(plansDir, "20260512-foo.md") + _, err = os.Stat(planFile) + require.True(t, os.IsNotExist(err)) + + err = svc.MovePlanToCompleted(planFile) + require.NoError(t, err) + + _, err = os.Stat(renamedPath) + assert.True(t, os.IsNotExist(err)) + + completedPath := filepath.Join(plansDir, "completed", "2026-05-12-foo.md") + _, err = os.Stat(completedPath) + require.NoError(t, err) + }) + + t.Run("in-place rename wins over stale completed copy", func(t *testing.T) { + // caller passes original dashed path. file was renamed in place to compact AND a stale + // completed/ exists from a prior run. the in-place rename is the + // active plan and must be moved; the stale completed/ copy must not short-circuit. + dir := setupExternalTestRepo(t) + svc, err := NewService(dir, noopServiceLogger()) + require.NoError(t, err) + + plansDir := filepath.Join(dir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o750)) + + // active plan at compact basename (renamed in place) + renamedPath := filepath.Join(plansDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# Plan (current)"), 0o600)) + require.NoError(t, svc.repo.add(renamedPath)) + require.NoError(t, svc.repo.commit("add plan with renamed basename")) + + // stale completed copy at original (dashed) basename from a prior run + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o750)) + stalePath := filepath.Join(completedDir, "2026-05-12-foo.md") + require.NoError(t, os.WriteFile(stalePath, []byte("# Plan (stale)"), 0o600)) + + // caller still references the original dashed path + planFile := filepath.Join(plansDir, "2026-05-12-foo.md") + _, err = os.Stat(planFile) + require.True(t, os.IsNotExist(err)) + + err = svc.MovePlanToCompleted(planFile) + require.NoError(t, err) + + // renamed source should be gone (was moved, not abandoned) + _, err = os.Stat(renamedPath) + assert.True(t, os.IsNotExist(err), "active in-place renamed file should have been moved") + + // destination uses the renamed basename + movedPath := filepath.Join(completedDir, "20260512-foo.md") + movedContent, err := os.ReadFile(movedPath) //nolint:gosec // test file + require.NoError(t, err) + assert.Equal(t, "# Plan (current)", string(movedContent), "moved file should contain current content") + + // stale completed copy is left in place (not our responsibility to clean up) + staleContent, err := os.ReadFile(stalePath) //nolint:gosec // test file + require.NoError(t, err) + assert.Equal(t, "# Plan (stale)", string(staleContent)) + }) + + t.Run("collision between in-place rename and stale completed/ is left untouched", func(t *testing.T) { + // caller passes original dashed path. file was renamed in place to compact AND a stale + // completed/ copy already exists with the same basename (e.g. same slug ran + // twice on the same day). git mv would refuse, os.Rename fallback would clobber the + // stale copy and leave the source's deletion unstaged. verify we surface this as + // already-completed and preserve both files for manual resolution. + dir := setupExternalTestRepo(t) + log := &mockLogger{} + svc, err := NewService(dir, log) + require.NoError(t, err) + + plansDir := filepath.Join(dir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o750)) + + // active plan at compact basename (renamed in place, tracked) + renamedPath := filepath.Join(plansDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# Plan (current)"), 0o600)) + require.NoError(t, svc.repo.add(renamedPath)) + require.NoError(t, svc.repo.commit("add plan with renamed basename")) + + // stale completed copy at compact basename from a prior run with same slug+date + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o750)) + stalePath := filepath.Join(completedDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(stalePath, []byte("# Plan (stale)"), 0o600)) + + // caller references the original dashed path + planFile := filepath.Join(plansDir, "2026-05-12-foo.md") + _, err = os.Stat(planFile) + require.True(t, os.IsNotExist(err)) + + err = svc.MovePlanToCompleted(planFile) + require.NoError(t, err) + + // active source must be preserved (NOT clobbered, NOT moved) + activeContent, err := os.ReadFile(renamedPath) //nolint:gosec // test file + require.NoError(t, err) + assert.Equal(t, "# Plan (current)", string(activeContent), "active in-place renamed file must be preserved") + + // stale completed copy must also be preserved (not overwritten) + staleContent, err := os.ReadFile(stalePath) //nolint:gosec // test file + require.NoError(t, err) + assert.Equal(t, "# Plan (stale)", string(staleContent), "stale completed copy must be preserved") + + // repo must be clean — no dangling deletion of the active source + dirty, err := svc.repo.isDirty() + require.NoError(t, err) + assert.False(t, dirty, "repo must be clean after collision-skip") + + // should have logged that the move was skipped due to the collision + require.Len(t, log.logs, 1) + assert.Contains(t, log.logs[0], "already in completed") + assert.Contains(t, log.logs[0], "20260512-foo.md") + assert.Contains(t, log.logs[0], "manual cleanup") + }) } func TestService_EnsureHasCommits(t *testing.T) { @@ -1540,3 +1757,4 @@ func TestService_resolveFilesystemCase(t *testing.T) { }) } } + diff --git a/pkg/plan/altdate.go b/pkg/plan/altdate.go new file mode 100644 index 00000000..08f57e7c --- /dev/null +++ b/pkg/plan/altdate.go @@ -0,0 +1,22 @@ +package plan + +import "regexp" + +var ( + dashedDatePattern = regexp.MustCompile(`^(\d{4})-(\d{2})-(\d{2})-(.+\.md)$`) + compactDatePattern = regexp.MustCompile(`^(\d{8})-(.+\.md)$`) +) + +// AltDateBasename returns the basename with the date prefix swapped between +// dashed (YYYY-MM-DD) and compact (YYYYMMDD) conventions, or "" if name +// matches neither pattern. Pure string transformation, no I/O. +func AltDateBasename(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] + } + return "" +} diff --git a/pkg/plan/altdate_test.go b/pkg/plan/altdate_test.go new file mode 100644 index 00000000..9afdfca5 --- /dev/null +++ b/pkg/plan/altdate_test.go @@ -0,0 +1,31 @@ +package plan + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAltDateBasename(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"dashed to compact", "2026-05-12-foo.md", "20260512-foo.md"}, + {"compact to dashed", "20260512-foo.md", "2026-05-12-foo.md"}, + {"dashed multi-part slug", "2026-05-12-extract-env-variable.md", "20260512-extract-env-variable.md"}, + {"compact multi-part slug", "20260512-extract-env-variable.md", "2026-05-12-extract-env-variable.md"}, + {"non-date basename returns empty", "feature-x.md", ""}, + {"missing .md extension returns empty", "2026-05-12-foo", ""}, + {"empty string returns empty", "", ""}, + {"non-md extension returns empty", "2026-05-12-foo.txt", ""}, + {"loose 8-digit prefix still swapped (no date validation)", "12345678-foo.md", "1234-56-78-foo.md"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, AltDateBasename(tt.in)) + }) + } +} diff --git a/pkg/processor/prompts.go b/pkg/processor/prompts.go index 6b7fb10c..94964a25 100644 --- a/pkg/processor/prompts.go +++ b/pkg/processor/prompts.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/umputun/ralphex/pkg/config" + "github.com/umputun/ralphex/pkg/plan" ) // agentRefPattern matches {{agent:name}} template syntax @@ -29,8 +30,11 @@ func (r *Runner) getPlanFileRef() string { return r.resolvePlanFilePath() } -// resolvePlanFilePath returns the actual path to the plan file, checking if it was moved to completed/. -// returns original path if file exists there, completed/ path if moved, or original path as fallback. +// resolvePlanFilePath returns the actual path to the plan file, checking if it was moved or renamed. +// probes in order: original path, / (in-place rename), +// completed/ (moved), completed/ (moved + renamed). +// the YYYY-MM-DD ↔ YYYYMMDD swap handles LLM-driven date-format renames. +// returns the first path that exists, or the original path as fallback. func (r *Runner) resolvePlanFilePath() string { if r.cfg.PlanFile == "" { return "" @@ -46,12 +50,32 @@ func (r *Runner) resolvePlanFilePath() string { return r.cfg.PlanFile } + dir := filepath.Dir(r.cfg.PlanFile) + base := filepath.Base(r.cfg.PlanFile) + altBase := plan.AltDateBasename(base) + + // check if file was renamed in place to the alternate date format (same directory) + // done before completed/ probes so a current renamed file wins over a stale completed/ copy + if altBase != "" { + if _, err := os.Stat(filepath.Join(dir, altBase)); err == nil { + return filepath.Join(dir, altBase) + } + } + // check if file was moved to completed/ subdirectory - completedPath := filepath.Join(filepath.Dir(r.cfg.PlanFile), "completed", filepath.Base(r.cfg.PlanFile)) + completedPath := filepath.Join(dir, "completed", base) if _, err := os.Stat(completedPath); err == nil { return completedPath } + // check if file was moved and renamed to the alternate date format in completed/ + if altBase != "" { + altCompletedPath := filepath.Join(dir, "completed", altBase) + if _, err := os.Stat(altCompletedPath); err == nil { + return altCompletedPath + } + } + // fall back to original path return r.cfg.PlanFile } diff --git a/pkg/processor/prompts_test.go b/pkg/processor/prompts_test.go index ac2f2192..a989e8b9 100644 --- a/pkg/processor/prompts_test.go +++ b/pkg/processor/prompts_test.go @@ -310,6 +310,105 @@ func TestRunner_resolvePlanFilePath(t *testing.T) { assert.Contains(t, r.getGoal(), completedPath) assert.NotContains(t, r.getGoal(), originalPath) }) + + t.Run("dashed file moved+renamed to compact in completed", func(t *testing.T) { + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o700)) + + originalPath := filepath.Join(plansDir, "2026-05-12-foo.md") + renamedPath := filepath.Join(completedDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# plan"), 0o600)) + + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, renamedPath, r.resolvePlanFilePath()) + }) + + t.Run("compact file moved+renamed to dashed in completed", func(t *testing.T) { + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o700)) + + originalPath := filepath.Join(plansDir, "20260512-foo.md") + renamedPath := filepath.Join(completedDir, "2026-05-12-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# plan"), 0o600)) + + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, renamedPath, r.resolvePlanFilePath()) + }) + + t.Run("non-date basename returns original path", func(t *testing.T) { + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o700)) + + // file does not exist at original location, completed/, or any alternate; helper + // returns "" because basename matches neither date pattern. + originalPath := filepath.Join(plansDir, "feature-x.md") + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, originalPath, r.resolvePlanFilePath()) + }) + + t.Run("dashed file renamed in place to compact (same dir)", func(t *testing.T) { + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o700)) + + originalPath := filepath.Join(plansDir, "2026-05-12-foo.md") + renamedPath := filepath.Join(plansDir, "20260512-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# plan"), 0o600)) + + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, renamedPath, r.resolvePlanFilePath()) + }) + + t.Run("compact file renamed in place to dashed (same dir)", func(t *testing.T) { + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o700)) + + originalPath := filepath.Join(plansDir, "20260512-foo.md") + renamedPath := filepath.Join(plansDir, "2026-05-12-foo.md") + require.NoError(t, os.WriteFile(renamedPath, []byte("# plan"), 0o600)) + + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, renamedPath, r.resolvePlanFilePath()) + }) + + t.Run("in-place alt wins over stale completed copy", func(t *testing.T) { + // when both an in-place alt-format file and a completed/ exist, + // the in-place alt takes precedence (it is the current file, the completed/ copy is stale) + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o700)) + + originalPath := filepath.Join(plansDir, "2026-05-12-foo.md") + inPlaceAlt := filepath.Join(plansDir, "20260512-foo.md") + staleCompleted := filepath.Join(completedDir, "2026-05-12-foo.md") + require.NoError(t, os.WriteFile(inPlaceAlt, []byte("# current"), 0o600)) + require.NoError(t, os.WriteFile(staleCompleted, []byte("# stale"), 0o600)) + + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, inPlaceAlt, r.resolvePlanFilePath()) + }) + + t.Run("8-digit non-date prefix is treated as date", func(t *testing.T) { + // pins behavior of the loose compact regex: it does not validate that the 8 digits + // form a real calendar date. helper produces a candidate, but file-not-found + // short-circuits before any harm and the fallback returns the original path. + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + completedDir := filepath.Join(plansDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o700)) + + originalPath := filepath.Join(plansDir, "12345678-foo.md") + r := &Runner{cfg: Config{PlanFile: originalPath}} + assert.Equal(t, originalPath, r.resolvePlanFilePath()) + }) } func TestRunner_getProgressFileRef(t *testing.T) { diff --git a/pkg/processor/runner.go b/pkg/processor/runner.go index d26a8559..efb74b6e 100644 --- a/pkg/processor/runner.go +++ b/pkg/processor/runner.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "io/fs" "os/exec" "strings" "time" @@ -935,6 +936,10 @@ func (r *Runner) validatePlanHasTasks() error { // checkboxes in Success criteria, Overview, or Context are ignored for this check, // so the agent can output ALL_TASKS_DONE when those are verification-only. // for malformed plans (checkboxes without task headers), returns true if any [ ] exists. +// returns false if the plan file is missing after resolvePlanFilePath exhausts all probes +// (original, /, completed/, completed/), +// to avoid spinning the loop when an LLM-driven git mv renamed the file out from under +// the runtime. func (r *Runner) hasUncompletedTasks() bool { path := r.resolvePlanFilePath() if path == "" { @@ -942,6 +947,13 @@ func (r *Runner) hasUncompletedTasks() bool { } p, err := plan.ParsePlanFile(path) if err != nil { + // last line of defense: resolvePlanFilePath has already tried the original path, + // the in-place alternate-date rename, completed/, and completed/. + // if the file is still missing, treat the run as complete so a vanished plan does + // not spin the loop. + if errors.Is(err, fs.ErrNotExist) { + return false + } r.log.Print("[WARN] failed to parse plan file for completion check: %v", err) return true // assume incomplete if can't read } diff --git a/pkg/processor/runner_test.go b/pkg/processor/runner_test.go index 35ce6284..274b9c16 100644 --- a/pkg/processor/runner_test.go +++ b/pkg/processor/runner_test.go @@ -683,6 +683,31 @@ func TestRunner_HasUncompletedTasks_CompletedDir(t *testing.T) { assert.True(t, r.TestHasUncompletedTasks()) } +func TestRunner_HasUncompletedTasks_MissingFile(t *testing.T) { + tmpDir := t.TempDir() + plansDir := filepath.Join(tmpDir, "docs", "plans") + require.NoError(t, os.MkdirAll(plansDir, 0o700)) + + // file is referenced but does not exist anywhere — neither at original path, + // nor in completed/, nor under the alternate date format probe. + // this simulates the loop-spin failure mode where an LLM-renamed plan + // becomes unresolvable; the runtime must treat it as complete, not stuck. + missingPath := filepath.Join(plansDir, "2026-05-12-vanished.md") + + log := newMockLogger("") + claude := newMockExecutor(nil) + codex := newMockExecutor(nil) + + cfg := processor.Config{PlanFile: missingPath} + r := processor.NewWithExecutors(cfg, log, processor.Executors{Claude: claude, Codex: codex}, &status.PhaseHolder{}) + + assert.False(t, r.TestHasUncompletedTasks(), "missing plan file must return false so SignalCompleted is honored") + + for _, call := range log.PrintCalls() { + assert.NotContains(t, call.Format, "[WARN]", "missing-file branch must not emit [WARN] (file is gone, signal already implies completion)") + } +} + func TestRunner_BuildCodexPrompt_CompletedDir(t *testing.T) { tmpDir := t.TempDir() plansDir := filepath.Join(tmpDir, "docs", "plans") diff --git a/pkg/web/plan.go b/pkg/web/plan.go index adeb3d8a..508d683e 100644 --- a/pkg/web/plan.go +++ b/pkg/web/plan.go @@ -10,15 +10,40 @@ import ( ) // loadPlanWithFallback loads a plan from disk with completed/ directory fallback. +// probes the original path, then completed/, then completed/ +// with the date prefix swapped between YYYY-MM-DD and YYYYMMDD conventions to handle +// plans renamed by an LLM-driven git mv. // does not cache - each call reads from disk. func loadPlanWithFallback(path string) (*plan.Plan, error) { p, err := plan.ParsePlanFile(path) - if err != nil && errors.Is(err, fs.ErrNotExist) { - completedPath := filepath.Join(filepath.Dir(path), "completed", filepath.Base(path)) - p, err = plan.ParsePlanFile(completedPath) + if err == nil { + return p, nil } - if err != nil { + if !errors.Is(err, fs.ErrNotExist) { return nil, fmt.Errorf("load plan with fallback: %w", err) } - return p, nil + + dir := filepath.Dir(path) + base := filepath.Base(path) + attempted := []string{path} + + completedPath := filepath.Join(dir, "completed", base) + attempted = append(attempted, completedPath) + 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 := plan.AltDateBasename(base); altBase != "" { + altCompletedPath := filepath.Join(dir, "completed", altBase) + attempted = append(attempted, altCompletedPath) + 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: plan not found, attempted %v: %w", attempted, fs.ErrNotExist) } diff --git a/pkg/web/server_test.go b/pkg/web/server_test.go index dc29f55f..8de67f9c 100644 --- a/pkg/web/server_test.go +++ b/pkg/web/server_test.go @@ -730,8 +730,55 @@ func TestLoadPlanWithFallback(t *testing.T) { _, err := loadPlanWithFallback(nonexistentPath) require.Error(t, err) }) + + t.Run("falls back to alt-date basename in completed/ (dashed to compact)", func(t *testing.T) { + tmpDir := t.TempDir() + completedDir := filepath.Join(tmpDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o750)) + + // plan only exists under compact-date basename in completed/ + completedPlan := filepath.Join(completedDir, "20260512-foo.md") + planContent := `# Renamed Plan + +### Task 1: Done + +- [x] Item +` + require.NoError(t, os.WriteFile(completedPlan, []byte(planContent), 0o600)) + + // request the dashed-date original path + originalPath := filepath.Join(tmpDir, "2026-05-12-foo.md") + plan, err := loadPlanWithFallback(originalPath) + require.NoError(t, err) + require.NotNil(t, plan) + assert.Equal(t, "Renamed Plan", plan.Title) + }) + + t.Run("falls back to alt-date basename in completed/ (compact to dashed)", func(t *testing.T) { + tmpDir := t.TempDir() + completedDir := filepath.Join(tmpDir, "completed") + require.NoError(t, os.MkdirAll(completedDir, 0o750)) + + // plan only exists under dashed-date basename in completed/ + completedPlan := filepath.Join(completedDir, "2026-05-12-foo.md") + planContent := `# Renamed Plan + +### Task 1: Done + +- [x] Item +` + require.NoError(t, os.WriteFile(completedPlan, []byte(planContent), 0o600)) + + // request the compact-date original path + originalPath := filepath.Join(tmpDir, "20260512-foo.md") + plan, err := loadPlanWithFallback(originalPath) + require.NoError(t, err) + require.NotNil(t, plan) + assert.Equal(t, "Renamed Plan", plan.Title) + }) } + func TestExtractProjectDir(t *testing.T) { tests := []struct { name string