internal/runtime/plan_test.go (1)

522-527: ⚡ Quick win

Strengthen the test case to validate precedence.

The second test case sets dagStepDir and stateWorkDir to the same value ("/remote/app"). This doesn't effectively test that the configured DAG step Dir takes precedence over the persisted node WorkingDir when they differ.

💡 Proposed test case improvement

 		{
 			name:         "configured step working dir remains configured",
 			dagStepDir:   "/remote/app",
-			stateWorkDir: "/remote/app",
+			stateWorkDir: "/var/lib/dagu/data/dag-runs/example/work",
 			wantStepDir:  "/remote/app",
 		},

This change would clearly demonstrate that the DAG-configured Dir wins even when the node's persisted WorkingDir differs.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/runtime/plan_test.go` around lines 522 - 527, The test case named
"configured step working dir remains configured" currently sets dagStepDir and
stateWorkDir to the same value, so it doesn't prove precedence; change the test
fixture so dagStepDir is "/remote/app" and stateWorkDir is a different path
(e.g. "/remote/old") and ensure the expectation wantStepDir remains
"/remote/app" to verify that the DAG-configured Dir takes precedence over the
persisted node WorkingDir (refer to dagStepDir, stateWorkDir, wantStepDir in the
test).

internal/runtime/plan_test.go (1)

589-590: ⚡ Quick win

Make the persisted step differ from the restored source step.

Right now Line 590 seeds node.Step.Dir with the same ${STEP_WORK_DIR} value as the DAG, so this test still passes if retry planning reuses the persisted step snapshot instead of rebinding from the restored DAG definition. Seed the node with the expanded concrete path (or any other mismatched value) so the assertion on Line 603 actually proves the source DAG won.

💡 Suggested fix

 	node := runtime.NodeWithData(runtime.NodeData{
-		Step: core.Step{Name: "target", Dir: "${STEP_WORK_DIR}"},
+		Step: core.Step{Name: "target", Dir: restoredStepWorkDir},
 		State: runtime.NodeState{
 			Status:     core.NodeFailed,
 			WorkingDir: "/stale/effective/work/dir",
 		},
 	})

Also applies to: 601-606

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/runtime/plan_test.go` around lines 589 - 590, The persisted node's
Step.Dir is being seeded with the same template value ("${STEP_WORK_DIR}") as
the DAG, so the test doesn't prove rebinding occurred; change the initialization
of the test node created via runtime.NodeWithData(runtime.NodeData{ Step:
core.Step{Name: "target", Dir: ... }}) to use a concrete/expanded path (e.g., an
absolute or different string) instead of the template so node.Step.Dir differs
from the DAG's template; update the other similar seedings in the same test (the
block around the node created at the 601-606 area) so the assertion that the
restored DAG's value overwrote the persisted snapshot actually verifies
rebinding.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

internal/runtime/plan_test.go (2)

662-668: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Strengthen the test by using a different persisted node Step.Dir value.

The persisted node's Step.Dir is currently "${STEP_WORK_DIR}", which matches the DAG step's Dir. The assertion target.Step().Dir == "${STEP_WORK_DIR}" would pass even if rebindRetryNodesToSteps failed to replace the persisted value. Change the persisted node's Step.Dir to a different value (e.g., "/stale/dir") so the assertion actually validates that retry planning replaces it with the DAG source definition.

🔧 Suggested fix

 	node := runtime.NodeWithData(runtime.NodeData{
-		Step: core.Step{Name: "target", Dir: "${STEP_WORK_DIR}"},
+		Step: core.Step{Name: "target", Dir: "/stale/dir"},
 		State: runtime.NodeState{
 			Status:     core.NodeFailed,
 			WorkingDir: "/stale/effective/work/dir",
 		},
 	})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/runtime/plan_test.go` around lines 662 - 668, The test constructs a
persisted node using runtime.NodeWithData(runtime.NodeData{...}) where the
persisted node Step.Dir is currently set to "${STEP_WORK_DIR}", which masks a
failure of rebindRetryNodesToSteps; update the persisted node's Step.Dir (the
runtime.NodeData.Step.Dir field) to a different value such as "/stale/dir" so
that after calling rebindRetryNodesToSteps the assertion on target.Step().Dir
verifies the DAG's Dir was actually applied; change the literal in the test
setup and keep the assertion same to validate rebindRetryNodesToSteps behavior.

---

598-603: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix Windows portability in the "configured step working dir" test case.

The hardcoded /remote/app path breaks on Windows because filepath.IsAbs("/remote/app") returns false (no drive letter or UNC prefix), causing the path to be treated as relative. This makes env.WorkingDir OS-dependent and contradicts the test's wantEnvDir expectation.

🔧 Suggested fix

 func TestCreateRetryPlan_PreservesExplicitStepWorkingDirOnly(t *testing.T) {
 	t.Parallel()
 
+	configuredStepDir := t.TempDir()
+
 	tests := []struct {
 		name         string
 		dagStepDir   string
 		stateWorkDir string
 		wantStepDir  string
 		wantEnvDir   string
 	}{
 		{
 			name:         "implicit step working dir ignores persisted effective work dir",
 			stateWorkDir: "/var/lib/dagu/data/dag-runs/example/dag-runs/2026/06/04/dag-run_20260604_134911Z_run/work",
 			wantStepDir:  "",
 			wantEnvDir:   "fresh-run-work-dir",
 		},
 		{
 			name:         "configured step working dir keeps restored source dir",
-			dagStepDir:   "/remote/app",
+			dagStepDir:   configuredStepDir,
 			stateWorkDir: "/var/lib/dagu/data/dag-runs/example/work",
-			wantStepDir:  "/remote/app",
-			wantEnvDir:   "/remote/app",
+			wantStepDir:  configuredStepDir,
+			wantEnvDir:   configuredStepDir,
 		},
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/runtime/plan_test.go` around lines 598 - 603, The test uses a
hardcoded "/remote/app" which is not considered an absolute path on Windows;
update the test case fields dagStepDir, wantStepDir and wantEnvDir to build an
OS-absolute path instead of the literal string. Replace the literal
"/remote/app" with a platform-safe construction (e.g. create the path via
filepath.Join(string(os.PathSeparator), "remote", "app") or use
filepath.FromSlash and ensure it is rooted) so filepath.IsAbs returns true on
Windows; adjust the values referenced in the test case (dagStepDir, wantStepDir,
wantEnvDir) accordingly in internal/runtime/plan_test.go.