Skip to content

fix: skip optional remote includes on offline cache miss - #3081

Open
Tidehyc wants to merge 1 commit into
go-task:mainfrom
Tidehyc:fix/offline-optional-remote-include
Open

Tidehyc wants to merge 1 commit into
go-task:mainfrom
Tidehyc:fix/offline-optional-remote-include

Conversation

@Tidehyc

@Tidehyc Tidehyc commented Oct 25, 2026 •

Copy link
Copy Markdown

Fixes #3076

Problem

When running task --offline with an optional: true remote include that has not been cached locally, Task fails and exits instead of skipping the missing optional include.

Steps to reproduce:

  1. Define a Taskfile.yml containing a remote include marked with optional: true (without caching it beforehand).
  2. Run a local task with the --offline flag (e.g., task --offline <local-task>).

Actual behavior:
Task aborts immediately with a cache miss error and exits with code 106 (TaskfileCacheNotFound).

Expected behavior:
Task should gracefully skip the uncached optional remote include and continue executing the remaining local tasks.

Root Cause

Currently, the Optional check only happens during the node initialization phase (NewNode). For remote nodes, offline cache resolution occurs later during the node reading phase (Node.Read()). When a cache miss occurs in offline mode, the error bypasses the NewNode check and is unconditionally propagated up by the recursive include() call in reader.go.

Changes

  • Ignore offline cache miss for optional includes: In reader.go's include() flow, explicitly check for the offline cache miss error when include.Optional is true and skip further processing for that branch.
  • Clean up dangling DAG vertex: Since the vertex is already registered in the Taskfile graph prior to Node.Read(), remove the failed vertex from the graph before skipping. If removing the vertex fails, fall back to returning the original read error.
  • Preserve existing behaviors:
    • Non-optional remote includes (optional: false) still fail with exit code 106 on an offline cache miss.
    • Cached remote includes continue to load and execute normally in offline mode.

Design Decisions

  1. Why scope the check narrowly to offline cache misses instead of matching the broad if include.Optional { return nil } check in NewNode?
    During NewNode (primarily local os.Stat resolution), an error reliably indicates that the target file does not exist. By the time execution reaches reader.go, however, Node.Read() and subsequent steps can fail for many other reasons (e.g., YAML syntax errors, invalid schema, network/timeout issues in online mode, or checksum mismatches). Catching all errors when Optional is true at this stage would silently swallow legitimate configuration or parsing errors. Narrowly targeting the offline cache miss error fixes the bug without masking real failures.

  2. Why remove the vertex on failure instead of refactoring to "read node first, add vertex second"?
    The current include() pipeline relies on adding the vertex and edge before recursing so that cycle detection (dag.AddEdge) happens prior to reading and parsing child Taskfiles. Reordering this to read before adding the vertex would either break cycle detection or require a broader restructuring of how the reader builds and traverses the dependency graph. Removing the dangling vertex exclusively on this specific error path is minimal, localized, and keeps cycle detection intact.

Known Limitations

  • Concurrent shared remote node edge case: Because include() runs concurrently via errgroup, if the exact same uncached remote Taskfile is included by two parent Taskfiles simultaneously, the second goroutine may attach an edge to the shared vertex before the first goroutine removes it on a cache miss. In that race window, removing the vertex will fail due to the remaining edge and gracefully fall back to returning the original cache miss error rather than skipping.

Testing

  • Added an isolated test case with dedicated testdata verifying that an uncached optional: true remote include is skipped under --offline and local tasks succeed.
  • Added a negative test case (optional: false) verifying that an uncached remote include under --offline still returns the expected cache miss error.
  • Verified existing test suites pass (TestIncludesRemote, TestIncludeCycle, TestIncludesMultiLevel, and go test ./taskfile/...).
  • Formatted and linted with golangci-lint (gofumpt + gci with 0 issues), committed as a single Conventional Commit.

AI Usage Disclosure

AI tools were used to assist in root-cause analysis, drafting the code changes in reader.go, and designing the test cases. I have manually reviewed and verified all changes, fully understand how the implementation works, and wrote this PR description myself.

Checklist

  • I have read the Contributing Guidelines
  • I have disclosed any AI-generated content in accordance with the AI usage policy
  • I fully understand the changes in this PR and have written the PR description myself

This branch has not been deployed

No deployments
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.

Optional remote includes fail in offline mode when not cached

1 participant