Skip to content

Comments

fix: working directory resolution#1641

Merged
yottahmd merged 6 commits intomainfrom
working-dir-default
Feb 6, 2026
Merged

fix: working directory resolution#1641
yottahmd merged 6 commits intomainfrom
working-dir-default

Conversation

@yottahmd
Copy link
Collaborator

@yottahmd yottahmd commented Feb 6, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced working directory resolution to properly inherit from base configuration when not explicitly specified in DAG files.
    • Added fallback mechanism for working directory determination with improved error handling.
  • Documentation

    • Removed DAGU_WORK_DIR from environment configuration documentation.
    • Added comprehensive RFC documenting working directory behavior, supported path formats, and execution-specific handling rules.

@coderabbitai
Copy link

coderabbitai bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

The changes refactor working directory resolution in DAG configuration loading. Default working directory values were removed from environment bindings and config defaults. The loading flow was modified to apply working directory defaults post-merge in base config inheritance, using file path directory or fallback mechanisms when not explicitly set.

Changes

Cohort / File(s) Summary
Documentation
README.md, rfcs/011-working-directory-behavior.md
Removed DAGU_WORK_DIR environment variable documentation. Added new RFC 011 documenting working directory behavior across DAG execution, including path formats, inheritance, and execution-type-specific handling.
Config Loading Defaults
internal/cmn/config/loader.go, internal/cmn/config/loader_test.go
Removed default workDir value and environment binding (DAGU_WORK_DIR) from Viper configuration defaults. Updated test to remove the environment variable from the test set.
DAG Spec Building
internal/core/spec/dag.go, internal/core/spec/dag_test.go
Modified buildWorkingDir() to return empty string instead of file directory when no explicit workingDir is set, enabling inheritance from base config. Renamed and restructured tests; removed fallback-to-directory tests, added empty-value scenarios.
DAG Loading
internal/core/spec/loader.go, internal/core/spec/loader_test.go
Introduced fallback mechanism in loadDAG() to populate workingDir post-merge: uses file path directory if available, otherwise calls getDefaultWorkingDir(). Added tests verifying base config inheritance and override scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Loader as DAG Loader
    participant SpecBuilder as Spec Builder
    participant BaseConfig as Base Config
    participant FileRes as File Resolution

    Client->>Loader: Load DAG (with optional base config)
    Loader->>BaseConfig: Load base config defaults
    BaseConfig-->>Loader: Base working dir (if set)
    
    Loader->>SpecBuilder: Build spec with base config
    SpecBuilder->>SpecBuilder: buildWorkingDir() → empty (no local default)
    SpecBuilder-->>Loader: Spec with empty workingDir
    
    Loader->>Loader: Merge with base config
    alt Working Dir Set
        Loader->>Loader: Use merged value
    else Working Dir Empty
        Loader->>FileRes: Get file path directory
        FileRes-->>Loader: Directory (or empty)
        alt File Path Available
            Loader->>Loader: Use file directory
        else No File Path
            Loader->>Loader: Call getDefaultWorkingDir()
            Loader->>Loader: Apply fallback default
        end
    end
    
    Loader-->>Client: Final DAG with workingDir
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • #1603: Modifies working directory handling in the spec layer with overlapping changes to internal/core/spec/dag.go and related test files.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: working directory resolution' is specific and clearly related to the main changes in the pull request, which refactor how the working directory is determined and applied across DAG execution.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch working-dir-default

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yottahmd yottahmd merged commit d3150e7 into main Feb 6, 2026
3 checks passed
@yottahmd yottahmd deleted the working-dir-default branch February 6, 2026 15:44
@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

❌ Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.79%. Comparing base (fd7fabe) to head (0e61aa9).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
internal/core/spec/loader.go 37.50% 4 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1641      +/-   ##
==========================================
- Coverage   69.79%   69.79%   -0.01%     
==========================================
  Files         331      332       +1     
  Lines       37248    37327      +79     
==========================================
+ Hits        25999    26053      +54     
- Misses       9185     9204      +19     
- Partials     2064     2070       +6     
Files with missing lines Coverage Δ
internal/cmn/config/loader.go 83.19% <ø> (+0.02%) ⬆️
internal/core/spec/dag.go 87.00% <100.00%> (-0.39%) ⬇️
internal/core/spec/loader.go 79.36% <37.50%> (-1.10%) ⬇️

... and 17 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4cdf60...0e61aa9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant