From c764b5e43d9ba2501625dc888f944c316881f4e8 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 12:32:45 -0400 Subject: [PATCH 01/13] make plans --- dev_docs/project_plan.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index 349e8b5..75d2812 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -256,6 +256,13 @@ This plan outlines the major development phases and tasks for building `dela`, a ## Phase 9: Increase Task Runner Coverage +- [ ] **Composed / Included Task Definitions** + - [ ] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. + - [ ] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. + - [ ] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. + - [ ] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. + - [ ] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. + - [ ] **Additional Task Runners support** - [x] [DTKT-119] Implement Maven `pom.xml` parser and task discovery - [x] [DTKT-120] Implement Gradle `build.gradle`/`build.gradle.kts` parser From c0e638b1622e8369f381060fb24cadd8e5428b07 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 12:33:37 -0400 Subject: [PATCH 02/13] m --- dev_docs/project_plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index 75d2812..173d14d 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -258,7 +258,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [ ] **Composed / Included Task Definitions** - [ ] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. - - [ ] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. + - [ ] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. - [ ] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. - [ ] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. - [ ] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. From 2790da9dc535f428c740f3601bc078d4836b046d Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 13:20:35 -0400 Subject: [PATCH 03/13] Initial DTKT-197 --- dev_docs/project_plan.md | 2 +- src/allowlist.rs | 129 +++++++++------ src/commands/list.rs | 8 +- src/composed_paths.rs | 233 ++++++++++++++++++++++++++++ src/lib.rs | 1 + src/main.rs | 1 + src/mcp/allowlist.rs | 60 ++----- src/mcp/dto.rs | 17 +- src/mcp/server.rs | 4 +- src/parsers/parse_cmake.rs | 1 + src/parsers/parse_docker_compose.rs | 3 + src/parsers/parse_github_actions.rs | 1 + src/parsers/parse_gradle.rs | 5 + src/parsers/parse_justfile.rs | 1 + src/parsers/parse_makefile.rs | 2 + src/parsers/parse_package_json.rs | 1 + src/parsers/parse_pom_xml.rs | 3 + src/parsers/parse_pyproject_toml.rs | 3 + src/parsers/parse_taskfile.rs | 1 + src/parsers/parse_travis_ci.rs | 4 + src/parsers/parse_turbo_json.rs | 1 + src/prompt.rs | 4 +- src/task_discovery.rs | 36 ++++- src/types.rs | 22 ++- tests/docker_noinit/test_noinit.sh | 11 ++ 25 files changed, 438 insertions(+), 116 deletions(-) create mode 100644 src/composed_paths.rs diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index 173d14d..56da428 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -257,7 +257,7 @@ This plan outlines the major development phases and tasks for building `dela`, a ## Phase 9: Increase Task Runner Coverage - [ ] **Composed / Included Task Definitions** - - [ ] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. + - [x] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. - [ ] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. - [ ] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. - [ ] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. diff --git a/src/allowlist.rs b/src/allowlist.rs index 8c17f64..0d69639 100644 --- a/src/allowlist.rs +++ b/src/allowlist.rs @@ -59,19 +59,25 @@ fn path_matches(task_path: &Path, allowlist_path: &Path, allow_subdirs: bool) -> } } -/// Check if a given task is explicitly allowed or denied by the allowlist -/// Returns (explicitly_allowed, explicitly_denied) - both false means not found in allowlist -/// This is the core allowlist checking logic without any prompting or persistence -pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { - // Only proceed with allowlist operations if dela is initialized - let allowlist = load_allowlist()?; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AllowlistMatch { + Allowed, + Denied, + NotFound, +} + +/// Evaluate a task against an already-loaded allowlist. +/// This is shared between the CLI and MCP surfaces so composed definitions use +/// identical path and precedence semantics everywhere. +pub fn evaluate_task_against_allowlist(task: &Task, allowlist: &Allowlist) -> AllowlistMatch { + let task_path = task.allowlist_path(); // First pass: Check for deny entries (highest precedence) for entry in &allowlist.entries { if let AllowScope::Deny = entry.scope - && path_matches(&task.file_path, &entry.path, true) + && path_matches(task_path, &entry.path, true) { - return Ok((false, true)); + return AllowlistMatch::Denied; } } @@ -79,32 +85,55 @@ pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { for entry in &allowlist.entries { match entry.scope { AllowScope::Directory => { - if path_matches(&task.file_path, &entry.path, true) { - return Ok((true, false)); + if path_matches(task_path, &entry.path, true) { + return AllowlistMatch::Allowed; } } AllowScope::File => { - if path_matches(&task.file_path, &entry.path, false) { - return Ok((true, false)); + if path_matches(task_path, &entry.path, false) { + return AllowlistMatch::Allowed; } } AllowScope::Task => { - if path_matches(&task.file_path, &entry.path, false) + if path_matches(task_path, &entry.path, false) && let Some(ref tasks) = entry.tasks && tasks.contains(&task.name) { - return Ok((true, false)); + return AllowlistMatch::Allowed; } } - AllowScope::Deny | AllowScope::Once => { - // Already handled deny in first pass, skip Once - continue; - } + AllowScope::Deny | AllowScope::Once => continue, } } - // If no matching entry found, return (false, false) - not found - Ok((false, false)) + AllowlistMatch::NotFound +} + +pub fn allowlist_entry_for_task(task: &Task, scope: AllowScope) -> AllowlistEntry { + let tasks = if scope == AllowScope::Task { + Some(vec![task.name.clone()]) + } else { + None + }; + + AllowlistEntry { + path: task.allowlist_path().to_path_buf(), + scope, + tasks, + } +} + +/// Check if a given task is explicitly allowed or denied by the allowlist +/// Returns (explicitly_allowed, explicitly_denied) - both false means not found in allowlist +/// This is the core allowlist checking logic without any prompting or persistence +pub fn is_task_allowed(task: &Task) -> Result<(bool, bool), String> { + // Only proceed with allowlist operations if dela is initialized + let allowlist = load_allowlist()?; + Ok(match evaluate_task_against_allowlist(task, &allowlist) { + AllowlistMatch::Allowed => (true, false), + AllowlistMatch::Denied => (false, true), + AllowlistMatch::NotFound => (false, false), + }) } /// Check if a given task is allowed, based on the loaded allowlist @@ -134,20 +163,10 @@ pub fn check_task_allowed(task: &Task) -> Result { Ok(true) } scope => { - // Create a new allowlist entry - let mut entry = AllowlistEntry { - path: task.file_path.clone(), - scope: scope.clone(), - tasks: None, - }; - - // For Task scope, add the specific task name - if scope == AllowScope::Task { - entry.tasks = Some(vec![task.name.clone()]); - } - // Add the entry and save - allowlist.entries.push(entry); + allowlist + .entries + .push(allowlist_entry_for_task(task, scope.clone())); save_allowlist(&allowlist)?; Ok(true) } @@ -155,11 +174,7 @@ pub fn check_task_allowed(task: &Task) -> Result { } AllowDecision::Deny => { // Add a deny entry and save - let entry = AllowlistEntry { - path: task.file_path.clone(), - scope: AllowScope::Deny, - tasks: None, - }; + let entry = allowlist_entry_for_task(task, AllowScope::Deny); allowlist.entries.push(entry); save_allowlist(&allowlist)?; Ok(false) @@ -172,20 +187,10 @@ pub fn check_task_allowed_with_scope(task: &Task, scope: AllowScope) -> Result Result<(), String> { let mut runner_files: HashMap = HashMap::new(); for task in &discovered.tasks { let runner_name = task.runner.short_name().to_string(); - runner_files.insert(runner_name, task.file_path.to_string_lossy().to_string()); + runner_files.insert( + runner_name, + task.definition_path().to_string_lossy().to_string(), + ); } // Calculate max task name width across all runners @@ -490,6 +493,7 @@ mod tests { Task { name: name.to_string(), file_path, + definition_path: None, definition_type: match runner { TaskRunner::Make => TaskDefinitionType::Makefile, TaskRunner::NodeNpm @@ -737,6 +741,7 @@ mod tests { let task = Task { name: "build".to_string(), file_path: makefile_path, + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "build".to_string(), @@ -888,6 +893,7 @@ mod tests { let task = Task { name: "integration".to_string(), file_path: PathBuf::from(".github/workflows"), + definition_path: None, definition_type: TaskDefinitionType::GitHubActions, runner: TaskRunner::Act, source_name: "integration".to_string(), diff --git a/src/composed_paths.rs b/src/composed_paths.rs new file mode 100644 index 0000000..bcd97b7 --- /dev/null +++ b/src/composed_paths.rs @@ -0,0 +1,233 @@ +use crate::types::Task; +use std::collections::HashSet; +use std::ffi::OsString; +use std::path::{Component, Path, PathBuf}; + +/// Shared path metadata for tasks discovered through composed definitions such +/// as includes or inherited configs. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(dead_code)] // DTKT-198/200/201 will consume the full recursive source API. +pub struct ComposedDefinitionSource { + runner_path: PathBuf, + definition_path: PathBuf, +} + +#[allow(dead_code)] // DTKT-198/200/201 will consume the full recursive source API. +impl ComposedDefinitionSource { + /// Create a source where the runner path and defining file are the same. + pub fn direct(path: impl Into) -> Self { + let path = normalize_path(path.into()); + Self { + runner_path: path.clone(), + definition_path: path, + } + } + + /// Create a source where the runner path differs from the defining file. + pub fn composed( + runner_path: impl Into, + definition_path: impl Into, + ) -> Self { + Self { + runner_path: normalize_path(runner_path.into()), + definition_path: normalize_path(definition_path.into()), + } + } + + pub fn runner_path(&self) -> &Path { + &self.runner_path + } + + pub fn definition_path(&self) -> &Path { + &self.definition_path + } + + /// Resolve a referenced child definition relative to the current defining file. + pub fn resolve_child(&self, referenced_path: impl AsRef) -> PathBuf { + resolve_nested_definition_path(&self.definition_path, referenced_path.as_ref()) + } + + /// Apply the source metadata to a discovered task. + pub fn apply_to_task(&self, task: &mut Task) { + task.file_path = self.runner_path.clone(); + if self.runner_path == self.definition_path { + task.definition_path = None; + } else { + task.definition_path = Some(self.definition_path.clone()); + } + } +} + +/// Track visited definitions for recursive discovery so include cycles can be +/// handled consistently across runners. +#[derive(Debug, Clone, Default)] +#[allow(dead_code)] // DTKT-198/200/201 will use recursive traversal state directly. +pub struct RecursiveDiscoveryState { + visited: HashSet, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(dead_code)] // DTKT-198/200/201 will use visit outcomes for include traversal. +pub enum VisitState { + New(PathBuf), + AlreadyVisited(PathBuf), +} + +#[allow(dead_code)] // DTKT-198/200/201 will use recursive traversal state directly. +impl RecursiveDiscoveryState { + pub fn new() -> Self { + Self::default() + } + + pub fn mark_visited(&mut self, path: impl AsRef) -> VisitState { + let normalized = normalize_path(path.as_ref()); + if self.visited.insert(normalized.clone()) { + VisitState::New(normalized) + } else { + VisitState::AlreadyVisited(normalized) + } + } +} + +/// Resolve a referenced definition path relative to the file that included it. +#[allow(dead_code)] // DTKT-198/200/201 will reuse this for include resolution. +pub fn resolve_nested_definition_path( + current_definition_path: &Path, + referenced_path: &Path, +) -> PathBuf { + if referenced_path.is_absolute() { + return normalize_path(referenced_path); + } + + let base_dir = current_definition_path.parent().unwrap_or(Path::new(".")); + normalize_path(base_dir.join(referenced_path)) +} + +fn normalize_path(path: impl AsRef) -> PathBuf { + let path = path.as_ref(); + let mut normalized_parts: Vec = Vec::new(); + let is_absolute = path.is_absolute(); + + for component in path.components() { + match component { + Component::RootDir => {} + Component::CurDir => {} + Component::ParentDir => { + if let Some(last) = normalized_parts.last() { + if last != ".." { + normalized_parts.pop(); + } else if !is_absolute { + normalized_parts.push(OsString::from("..")); + } + } else if !is_absolute { + normalized_parts.push(OsString::from("..")); + } + } + Component::Normal(part) => normalized_parts.push(part.to_os_string()), + Component::Prefix(prefix) => normalized_parts.push(prefix.as_os_str().to_os_string()), + } + } + + let mut normalized = if is_absolute { + PathBuf::from(std::path::MAIN_SEPARATOR.to_string()) + } else { + PathBuf::new() + }; + + for part in normalized_parts { + normalized.push(part); + } + + if normalized.as_os_str().is_empty() && !is_absolute { + PathBuf::from(".") + } else { + normalized + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::types::{Task, TaskDefinitionType, TaskRunner}; + use std::path::PathBuf; + + fn sample_task() -> Task { + Task { + name: "build".to_string(), + file_path: PathBuf::from("/tmp/runner"), + definition_path: None, + definition_type: TaskDefinitionType::Makefile, + runner: TaskRunner::Make, + source_name: "build".to_string(), + description: None, + shadowed_by: None, + disambiguated_name: None, + } + } + + #[test] + fn test_direct_source_uses_same_path_for_runner_and_definition() { + let source = ComposedDefinitionSource::direct("/repo/Makefile"); + + assert_eq!(source.runner_path(), Path::new("/repo/Makefile")); + assert_eq!(source.definition_path(), Path::new("/repo/Makefile")); + } + + #[test] + fn test_composed_source_applies_runner_and_definition_paths_to_task() { + let source = ComposedDefinitionSource::composed( + "/repo/.github/workflows", + "/repo/.github/workflows/ci.yml", + ); + let mut task = sample_task(); + + source.apply_to_task(&mut task); + + assert_eq!(task.file_path, PathBuf::from("/repo/.github/workflows")); + assert_eq!( + task.definition_path(), + Path::new("/repo/.github/workflows/ci.yml") + ); + assert_eq!( + task.allowlist_path(), + Path::new("/repo/.github/workflows/ci.yml") + ); + } + + #[test] + fn test_resolve_nested_definition_path_normalizes_relative_segments() { + let resolved = resolve_nested_definition_path( + Path::new("/repo/make/Makefile"), + Path::new("../shared/tasks.mk"), + ); + + assert_eq!(resolved, PathBuf::from("/repo/shared/tasks.mk")); + } + + #[test] + fn test_recursive_discovery_state_detects_duplicate_paths_after_normalization() { + let mut state = RecursiveDiscoveryState::new(); + + assert_eq!( + state.mark_visited("/repo/make/../shared/tasks.mk"), + VisitState::New(PathBuf::from("/repo/shared/tasks.mk")) + ); + assert_eq!( + state.mark_visited("/repo/shared/tasks.mk"), + VisitState::AlreadyVisited(PathBuf::from("/repo/shared/tasks.mk")) + ); + } + + #[test] + fn test_composed_source_resolves_children_relative_to_definition_file() { + let source = ComposedDefinitionSource::composed( + "/repo/Makefile", + "/repo/includes/common/tasks.mk", + ); + + assert_eq!( + source.resolve_child("../../other.mk"), + PathBuf::from("/repo/other.mk") + ); + } +} diff --git a/src/lib.rs b/src/lib.rs index 4546996..f46c866 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ pub mod allowlist; pub mod builtins; pub mod colors; pub mod commands; +pub mod composed_paths; pub mod config; pub mod environment; pub mod mcp; diff --git a/src/main.rs b/src/main.rs index 15ea2cb..e1a8ea4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,7 @@ use clap::{Parser, Subcommand}; mod allowlist; mod builtins; mod commands; +mod composed_paths; mod config; mod environment; mod mcp; diff --git a/src/mcp/allowlist.rs b/src/mcp/allowlist.rs index fd3d25b..0074d26 100644 --- a/src/mcp/allowlist.rs +++ b/src/mcp/allowlist.rs @@ -1,6 +1,7 @@ -use crate::allowlist::load_allowlist; -use crate::types::{AllowScope, Allowlist, Task}; -use std::path::Path; +use crate::allowlist::{AllowlistMatch, evaluate_task_against_allowlist, load_allowlist}; +#[cfg(test)] +use crate::types::AllowScope; +use crate::types::{Allowlist, Task}; /// MCP-specific allowlist evaluator for task permissions /// @@ -33,54 +34,10 @@ impl McpAllowlistEvaluator { /// 4. Task scope allow entries /// 5. Not found in allowlist (deny by default for MCP) pub fn is_task_allowed(&self, task: &Task) -> Result { - // First pass: Check for deny entries (highest precedence) - for entry in &self.allowlist.entries { - if let AllowScope::Deny = entry.scope - && self.path_matches(&task.file_path, &entry.path, true) - { - return Ok(false); - } - } - - // Second pass: Check for allow entries - for entry in &self.allowlist.entries { - match entry.scope { - AllowScope::Directory => { - if self.path_matches(&task.file_path, &entry.path, true) { - return Ok(true); - } - } - AllowScope::File => { - if self.path_matches(&task.file_path, &entry.path, false) { - return Ok(true); - } - } - AllowScope::Task => { - if self.path_matches(&task.file_path, &entry.path, false) - && let Some(ref tasks) = entry.tasks - && tasks.contains(&task.name) - { - return Ok(true); - } - } - AllowScope::Deny | AllowScope::Once => { - // Already handled deny in first pass, skip Once (not applicable for MCP) - continue; - } - } - } - - // If no matching entry found, deny by default for MCP - Ok(false) - } - - /// Check if two paths match, considering directory scope - fn path_matches(&self, task_path: &Path, allowlist_path: &Path, allow_subdirs: bool) -> bool { - if allow_subdirs { - task_path.starts_with(allowlist_path) - } else { - task_path == allowlist_path - } + Ok(matches!( + evaluate_task_against_allowlist(task, &self.allowlist), + AllowlistMatch::Allowed + )) } /// Get the number of entries in the allowlist @@ -111,6 +68,7 @@ mod tests { Task { name: name.to_string(), file_path, + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: name.to_string(), diff --git a/src/mcp/dto.rs b/src/mcp/dto.rs index 06bff81..b775f28 100644 --- a/src/mcp/dto.rs +++ b/src/mcp/dto.rs @@ -56,7 +56,7 @@ impl TaskDto { command: task.runner.get_command(task), runner_available: is_runner_available(&task.runner), allowlisted: false, // Default to false for legacy method - file_path: task.file_path.to_string_lossy().to_string(), + file_path: task.definition_path().to_string_lossy().to_string(), description: task.description.clone(), } } @@ -78,7 +78,7 @@ impl TaskDto { command: task.runner.get_command(task), runner_available: is_runner_available_for_mcp(&task.runner), allowlisted: allowlist_evaluator.is_task_allowed(task).unwrap_or(false), - file_path: task.file_path.to_string_lossy().to_string(), + file_path: task.definition_path().to_string_lossy().to_string(), description: task.description.clone(), } } @@ -104,6 +104,7 @@ mod tests { let task = Task { name: "build".to_string(), file_path: PathBuf::from("/project/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "build".to_string(), @@ -131,6 +132,7 @@ mod tests { let task = Task { name: "test".to_string(), file_path: PathBuf::from("/project/package.json"), + definition_path: None, definition_type: TaskDefinitionType::PackageJson, runner: TaskRunner::NodeNpm, source_name: "test".to_string(), @@ -157,6 +159,7 @@ mod tests { let task = Task { name: "clean".to_string(), file_path: PathBuf::from("/project/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "clean".to_string(), @@ -203,6 +206,7 @@ mod tests { let task = Task { name: "build".to_string(), file_path: PathBuf::from("/project/file"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: runner.clone(), source_name: "build".to_string(), @@ -237,6 +241,7 @@ mod tests { let task = Task { name: "serve".to_string(), file_path: PathBuf::from("/home/user/projects/my-app/package.json"), + definition_path: None, definition_type: TaskDefinitionType::PackageJson, runner: TaskRunner::NodeNpm, source_name: "serve".to_string(), @@ -266,6 +271,7 @@ mod tests { let task = Task { name: "test".to_string(), file_path: PathBuf::from("/project/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "test".to_string(), @@ -291,6 +297,7 @@ mod tests { Task { name: "test".to_string(), file_path: PathBuf::from("/project/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "test".to_string(), @@ -301,6 +308,7 @@ mod tests { Task { name: "test".to_string(), file_path: PathBuf::from("/project/package.json"), + definition_path: None, definition_type: TaskDefinitionType::PackageJson, runner: TaskRunner::NodeNpm, source_name: "test".to_string(), @@ -381,6 +389,7 @@ mod tests { let task = Task { name: "build".to_string(), file_path: PathBuf::from("/project/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "build".to_string(), @@ -441,6 +450,7 @@ mod tests { let task = Task { name: task_name.to_string(), file_path: PathBuf::from("/project/file"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: runner.clone(), source_name: task_name.to_string(), @@ -482,6 +492,7 @@ mod tests { let task = Task { name: task_name.to_string(), file_path: PathBuf::from("/project/docker-compose.yml"), + definition_path: None, definition_type: TaskDefinitionType::DockerCompose, runner: TaskRunner::DockerCompose, source_name: task_name.to_string(), @@ -510,6 +521,7 @@ mod tests { let task = Task { name: "test".to_string(), file_path: PathBuf::from("/project/.travis.yml"), + definition_path: None, definition_type: TaskDefinitionType::TravisCi, runner: TaskRunner::TravisCi, source_name: "test".to_string(), @@ -543,6 +555,7 @@ mod tests { let task = Task { name: "build-all".to_string(), file_path: PathBuf::from("/project/CMakeLists.txt"), + definition_path: None, definition_type: TaskDefinitionType::CMake, runner: TaskRunner::CMake, source_name: "build-all".to_string(), diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 093837d..e16dc92 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -774,7 +774,7 @@ impl DelaMcpServer { env: args.env.clone(), cwd: args.cwd.as_ref().map(PathBuf::from), command: task.runner.get_command(task), - file_path: task.file_path.clone(), + file_path: task.definition_path().to_path_buf(), }; let exit_state = JobState::Exited(exit_code.unwrap_or(-1)); @@ -831,7 +831,7 @@ impl DelaMcpServer { env: args.env.clone(), cwd: args.cwd.as_ref().map(PathBuf::from), command: task.runner.get_command(task), - file_path: task.file_path.clone(), + file_path: task.definition_path().to_path_buf(), }; // Start background job management diff --git a/src/parsers/parse_cmake.rs b/src/parsers/parse_cmake.rs index 6be11cf..41e9c95 100644 --- a/src/parsers/parse_cmake.rs +++ b/src/parsers/parse_cmake.rs @@ -64,6 +64,7 @@ fn parse_cmake_string(content: &str, file_path: &Path) -> Result, Stri let task = Task { name: target_name.to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::CMake, runner: TaskRunner::CMake, source_name: target_name.to_string(), diff --git a/src/parsers/parse_docker_compose.rs b/src/parsers/parse_docker_compose.rs index a6ef125..0ecde5c 100644 --- a/src/parsers/parse_docker_compose.rs +++ b/src/parsers/parse_docker_compose.rs @@ -49,6 +49,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: "up".to_string(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::DockerCompose, runner: TaskRunner::DockerCompose, source_name: "up".to_string(), @@ -61,6 +62,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: "down".to_string(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::DockerCompose, runner: TaskRunner::DockerCompose, source_name: "down".to_string(), @@ -82,6 +84,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: service_name.clone(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::DockerCompose, runner: TaskRunner::DockerCompose, source_name: service_name, diff --git a/src/parsers/parse_github_actions.rs b/src/parsers/parse_github_actions.rs index 15fec04..9510246 100644 --- a/src/parsers/parse_github_actions.rs +++ b/src/parsers/parse_github_actions.rs @@ -65,6 +65,7 @@ fn parse_workflow_string(content: &str, file_path: &Path) -> Result, S let task = Task { name: task_name.clone(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::GitHubActions, runner: TaskRunner::Act, source_name: task_name, // Source name is the same as the task name (entire workflow) diff --git a/src/parsers/parse_gradle.rs b/src/parsers/parse_gradle.rs index 3124e51..62c1ba8 100644 --- a/src/parsers/parse_gradle.rs +++ b/src/parsers/parse_gradle.rs @@ -48,6 +48,7 @@ fn add_common_tasks(tasks: &mut Vec, file_path: &Path) { tasks.push(Task { name: task_name.to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Gradle, runner: TaskRunner::Gradle, source_name: task_name.to_string(), @@ -82,6 +83,7 @@ fn extract_custom_tasks( tasks.push(Task { name: task_name.as_str().to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Gradle, runner: TaskRunner::Gradle, source_name: task_name.as_str().to_string(), @@ -98,6 +100,7 @@ fn extract_custom_tasks( tasks.push(Task { name: task_name.as_str().to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Gradle, runner: TaskRunner::Gradle, source_name: task_name.as_str().to_string(), @@ -114,6 +117,7 @@ fn extract_custom_tasks( tasks.push(Task { name: task_name.as_str().to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Gradle, runner: TaskRunner::Gradle, source_name: task_name.as_str().to_string(), @@ -226,6 +230,7 @@ fn extract_plugin_tasks( tasks.push(Task { name: task_name.to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Gradle, runner: TaskRunner::Gradle, source_name: task_name.to_string(), diff --git a/src/parsers/parse_justfile.rs b/src/parsers/parse_justfile.rs index ad66769..ce2f7a3 100644 --- a/src/parsers/parse_justfile.rs +++ b/src/parsers/parse_justfile.rs @@ -45,6 +45,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: task_name.clone(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::Justfile, runner: TaskRunner::Just, source_name: task_name, diff --git a/src/parsers/parse_makefile.rs b/src/parsers/parse_makefile.rs index 347ac4b..454cf3d 100644 --- a/src/parsers/parse_makefile.rs +++ b/src/parsers/parse_makefile.rs @@ -71,6 +71,7 @@ fn extract_tasks(makefile: &Makefile, path: &Path) -> Result, String> Task { name: name.clone(), file_path: path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: name, @@ -133,6 +134,7 @@ fn extract_tasks_regex(content: &str, path: &Path) -> Result, String> Task { name: name.clone(), file_path: path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: name, diff --git a/src/parsers/parse_package_json.rs b/src/parsers/parse_package_json.rs index 6b848bf..d43f1a4 100644 --- a/src/parsers/parse_package_json.rs +++ b/src/parsers/parse_package_json.rs @@ -27,6 +27,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: name.clone(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::PackageJson, runner: runner.clone(), source_name: name.clone(), diff --git a/src/parsers/parse_pom_xml.rs b/src/parsers/parse_pom_xml.rs index cc56e41..92270b7 100644 --- a/src/parsers/parse_pom_xml.rs +++ b/src/parsers/parse_pom_xml.rs @@ -41,6 +41,7 @@ fn add_default_maven_goals(tasks: &mut Vec, file_path: &Path) { tasks.push(Task { name: goal.to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::MavenPom, runner: TaskRunner::Maven, source_name: goal.to_string(), @@ -67,6 +68,7 @@ fn add_profile_tasks(tasks: &mut Vec, root: Node, file_path: &Path) -> Res tasks.push(Task { name: format!("profile:{}", profile_id), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::MavenPom, runner: TaskRunner::Maven, source_name: profile_id.clone(), @@ -121,6 +123,7 @@ fn add_plugin_tasks(tasks: &mut Vec, root: Node, file_path: &Path) -> Resu tasks.push(Task { name: task_name.clone(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::MavenPom, runner: TaskRunner::Maven, source_name: task_name, diff --git a/src/parsers/parse_pyproject_toml.rs b/src/parsers/parse_pyproject_toml.rs index c64d2f5..b6368a8 100644 --- a/src/parsers/parse_pyproject_toml.rs +++ b/src/parsers/parse_pyproject_toml.rs @@ -24,6 +24,7 @@ pub fn parse(path: &Path) -> Result, String> { tasks.push(Task { name: name.clone(), file_path: path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::PyprojectToml, runner: TaskRunner::PythonUv, source_name: name.clone(), @@ -57,6 +58,7 @@ pub fn parse(path: &Path) -> Result, String> { tasks.push(Task { name: name.clone(), file_path: path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::PyprojectToml, runner: TaskRunner::PythonPoetry, source_name: name.clone(), @@ -98,6 +100,7 @@ pub fn parse(path: &Path) -> Result, String> { tasks.push(Task { name: name.clone(), file_path: path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::PyprojectToml, runner: TaskRunner::PythonPoe, source_name: name.clone(), diff --git a/src/parsers/parse_taskfile.rs b/src/parsers/parse_taskfile.rs index 2fa3387..517f5b0 100644 --- a/src/parsers/parse_taskfile.rs +++ b/src/parsers/parse_taskfile.rs @@ -71,6 +71,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: name.clone(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::Taskfile, runner: TaskRunner::Task, source_name: name, diff --git a/src/parsers/parse_travis_ci.rs b/src/parsers/parse_travis_ci.rs index 4c2f69b..d8149bb 100644 --- a/src/parsers/parse_travis_ci.rs +++ b/src/parsers/parse_travis_ci.rs @@ -43,6 +43,7 @@ fn parse_travis_string(content: &str, file_path: &Path) -> Result, Str let task = Task { name: job_name.clone(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::TravisCi, runner: TaskRunner::TravisCi, source_name: job_name.clone(), @@ -73,6 +74,7 @@ fn parse_travis_string(content: &str, file_path: &Path) -> Result, Str let task = Task { name: job_name.clone(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::TravisCi, runner: TaskRunner::TravisCi, source_name: job_name.clone(), @@ -90,6 +92,7 @@ fn parse_travis_string(content: &str, file_path: &Path) -> Result, Str let task = Task { name: job_name.clone(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::TravisCi, runner: TaskRunner::TravisCi, source_name: job_name.clone(), @@ -110,6 +113,7 @@ fn parse_travis_string(content: &str, file_path: &Path) -> Result, Str let task = Task { name: "travis".to_string(), file_path: file_path.to_path_buf(), + definition_path: None, definition_type: TaskDefinitionType::TravisCi, runner: TaskRunner::TravisCi, source_name: "travis".to_string(), diff --git a/src/parsers/parse_turbo_json.rs b/src/parsers/parse_turbo_json.rs index 7d82bc3..a0cc8da 100644 --- a/src/parsers/parse_turbo_json.rs +++ b/src/parsers/parse_turbo_json.rs @@ -17,6 +17,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { .map(|name| Task { name: name.clone(), file_path: path.clone(), + definition_path: None, definition_type: TaskDefinitionType::TurboJson, runner: TaskRunner::Turbo, source_name: name.clone(), diff --git a/src/prompt.rs b/src/prompt.rs index be3fc36..ba05ecb 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -69,7 +69,7 @@ fn prompt_for_task_fallback(task: &Task) -> Result { println!( "\nTask '{}' from '{}' requires approval.", task.name, - task.file_path.display() + task.definition_path().display() ); if let Some(desc) = &task.description { println!("Description: {}", desc); @@ -189,7 +189,7 @@ fn ui(f: &mut Frame, task: &Task, options: &[(&str, AllowDecision)], selected: u .add_modifier(Modifier::BOLD), )]), Line::from(vec![Span::styled( - format!(" {}", task.file_path.display()), + format!(" {}", task.definition_path().display()), Style::default() .fg(Color::Yellow) .add_modifier(Modifier::BOLD), diff --git a/src/task_discovery.rs b/src/task_discovery.rs index 3a696a9..9b6c2aa 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -1,3 +1,4 @@ +use crate::composed_paths::ComposedDefinitionSource; use crate::parsers::{ parse_cmake, parse_docker_compose, parse_github_actions, parse_gradle, parse_justfile, parse_makefile, parse_package_json, parse_pom_xml, parse_pyproject_toml, parse_taskfile, @@ -223,7 +224,7 @@ pub fn format_ambiguous_task_error(task_name: &str, matching_tasks: &[&Task]) -> " • {} ({} from {})\n", disambiguated, task.runner.short_name(), - task.file_path.display() + task.definition_path().display() )); } } @@ -639,10 +640,11 @@ fn discover_github_actions_tasks( for file_path in workflow_files { match parse_github_actions(&file_path) { Ok(mut tasks) => { - // Override the file path to use the common parent directory - // instead of individual workflow files + let source = + ComposedDefinitionSource::composed(workflows_parent.clone(), file_path); for task in &mut tasks { - task.file_path = workflows_parent.clone(); + source.apply_to_task(task); + task.shadowed_by = check_shadowing(&task.name); } all_tasks.extend(tasks); } @@ -838,6 +840,7 @@ fn discover_shell_script_tasks(dir: &Path, discovered: &mut DiscoveredTasks) { discovered.tasks.push(Task { name: name.clone(), file_path: path, + definition_path: None, definition_type: TaskDefinitionType::ShellScript, runner: TaskRunner::ShellScript, source_name, @@ -1880,8 +1883,19 @@ jobs: // With the new workflow grouping, all tasks should have the same workflow directory let common_path = temp_dir.path().join(".github").join("workflows"); + let expected_definition_paths = [ + github_workflows_dir.join("ci.yml"), + temp_dir.path().join("workflow.yml"), + custom_dir.join("custom.yml"), + ]; + for task in act_tasks { assert_eq!(task.file_path, common_path); + assert!( + expected_definition_paths.contains(&task.definition_path().to_path_buf()), + "unexpected workflow definition path: {}", + task.definition_path().display() + ); } } @@ -1895,6 +1909,7 @@ jobs: discovered.tasks.push(Task { name: "test".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "test".to_string(), @@ -1907,6 +1922,7 @@ jobs: discovered.tasks.push(Task { name: "ls".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "ls".to_string(), @@ -1919,6 +1935,7 @@ jobs: discovered.tasks.push(Task { name: "build".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "build".to_string(), @@ -1957,6 +1974,7 @@ jobs: discovered.tasks.push(Task { name: "test".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "test".to_string(), @@ -1968,6 +1986,7 @@ jobs: discovered.tasks.push(Task { name: "test".to_string(), file_path: PathBuf::from("/test/package.json"), + definition_path: None, definition_type: TaskDefinitionType::PackageJson, runner: TaskRunner::NodeNpm, source_name: "test".to_string(), @@ -1980,6 +1999,7 @@ jobs: discovered.tasks.push(Task { name: "ls".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "ls".to_string(), @@ -1992,6 +2012,7 @@ jobs: discovered.tasks.push(Task { name: "cd".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "cd".to_string(), @@ -2003,6 +2024,7 @@ jobs: discovered.tasks.push(Task { name: "cd".to_string(), file_path: PathBuf::from("/test/Taskfile.yml"), + definition_path: None, definition_type: TaskDefinitionType::Taskfile, runner: TaskRunner::Task, source_name: "cd".to_string(), @@ -2015,6 +2037,7 @@ jobs: discovered.tasks.push(Task { name: "build".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "build".to_string(), @@ -2085,6 +2108,7 @@ jobs: discovered.tasks.push(Task { name: "install".to_string(), file_path: PathBuf::from("/test/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "install".to_string(), @@ -2117,6 +2141,7 @@ jobs: let task = Task { name: "test".to_string(), file_path: PathBuf::from("/path/to/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "test".to_string(), @@ -2153,6 +2178,7 @@ jobs: let task = Task { name: "grep".to_string(), file_path: PathBuf::from("/path/to/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "grep".to_string(), @@ -2191,6 +2217,7 @@ jobs: let task1 = Task { name: "test".to_string(), file_path: PathBuf::from("/path/to/Makefile"), + definition_path: None, definition_type: TaskDefinitionType::Makefile, runner: TaskRunner::Make, source_name: "test".to_string(), @@ -2202,6 +2229,7 @@ jobs: let task2 = Task { name: "test".to_string(), file_path: PathBuf::from("/path/to/package.json"), + definition_path: None, definition_type: TaskDefinitionType::PackageJson, runner: TaskRunner::NodeNpm, source_name: "test".to_string(), diff --git a/src/types.rs b/src/types.rs index b6c0f11..a36747a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,5 +1,5 @@ use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// Information about what shadows a task name #[derive(Debug, Clone, PartialEq)] @@ -160,8 +160,14 @@ pub struct DiscoveredTaskDefinitions { pub struct Task { /// Name of the task (e.g., "build", "test", "start") pub name: String, - /// Path to the file containing this task + /// Path the runner should use for execution context. + /// For simple task definitions this is the same as the defining file. + /// For composed definitions this may point at the root file or directory + /// that the runner executes against. pub file_path: PathBuf, + /// Path to the file that actually defines this task when it differs from + /// the runner path. For simple task definitions this is None. + pub definition_path: Option, /// The type of definition file this task came from pub definition_type: TaskDefinitionType, /// The type of runner needed for this task @@ -176,6 +182,18 @@ pub struct Task { pub disambiguated_name: Option, } +impl Task { + /// Return the file that actually defines this task. + pub fn definition_path(&self) -> &Path { + self.definition_path.as_deref().unwrap_or(&self.file_path) + } + + /// Return the path used for allowlist matching and user-facing source attribution. + pub fn allowlist_path(&self) -> &Path { + self.definition_path() + } +} + impl TaskRunner { /// Get the command to run a task with this runner pub fn get_command(&self, task: &Task) -> String { diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index ec22ec0..3576111 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -289,6 +289,17 @@ else exit 1 fi +# Test 19b: Verify allow-command stores the defining workflow path +echo "\nTest 19b: Verifying GitHub Actions allowlist source attribution" +echo "2" | dela allow-command test-a >/dev/null 2>&1 +if grep -q "/home/testuser/test_project/.github/workflows/test.yml" /home/testuser/.config/dela/allowlist.toml; then + echo "${GREEN}✓ GitHub Actions allowlist entries use the defining workflow file${NC}" +else + echo "${RED}✗ GitHub Actions allowlist entry did not use the defining workflow file${NC}" + cat /home/testuser/.config/dela/allowlist.toml + exit 1 +fi + # Test 20: Test single argument passing with print-arg-task echo "\nTest 20: Testing single argument passing with print-arg-task" From a764159f454dd63e3042995d7190f8765659f010 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 14:24:27 -0400 Subject: [PATCH 04/13] Makefile recursive definitions --- dev_docs/project_plan.md | 2 +- src/parsers/parse_makefile.rs | 181 ++++++++++++++++++++ src/task_discovery.rs | 259 +++++++++++++++++++++++++++-- tests/docker_noinit/test_noinit.sh | 40 +++++ 4 files changed, 470 insertions(+), 12 deletions(-) diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index 56da428..0e095dc 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -258,7 +258,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [ ] **Composed / Included Task Definitions** - [x] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. - - [ ] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. + - [x] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. - [ ] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. - [ ] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. - [ ] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. diff --git a/src/parsers/parse_makefile.rs b/src/parsers/parse_makefile.rs index 454cf3d..11da5a1 100644 --- a/src/parsers/parse_makefile.rs +++ b/src/parsers/parse_makefile.rs @@ -2,8 +2,15 @@ use crate::types::{Task, TaskDefinitionType, TaskRunner}; use makefile_lossless::Makefile; use regex::Regex; use std::collections::HashMap; +use std::path::PathBuf; use std::path::Path; +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MakefileInclude { + pub path: PathBuf, + pub optional: bool, +} + /// Parse a Makefile at the given path and extract tasks pub fn parse(path: &Path) -> Result, String> { let content = @@ -87,6 +94,142 @@ fn extract_tasks(makefile: &Makefile, path: &Path) -> Result, String> Ok(tasks_map.into_values().collect()) } +/// Extract Makefile include directives from a file. +pub fn extract_include_directives(path: &Path) -> Result, String> { + let content = + std::fs::read_to_string(path).map_err(|e| format!("Failed to read Makefile: {}", e))?; + Ok(extract_include_directives_from_str(&content)) +} + +fn extract_include_directives_from_str(content: &str) -> Vec { + let normalized = collapse_line_continuations(content); + let mut includes = Vec::new(); + + for line in normalized.lines() { + if line.starts_with('\t') { + continue; + } + + let trimmed = line.trim_start(); + if trimmed.is_empty() || trimmed.starts_with('#') { + continue; + } + + let (optional, remainder) = if let Some(rest) = trimmed.strip_prefix("include") { + (false, rest) + } else if let Some(rest) = trimmed.strip_prefix("-include") { + (true, rest) + } else if let Some(rest) = trimmed.strip_prefix("sinclude") { + (true, rest) + } else { + continue; + }; + + if !remainder + .chars() + .next() + .is_some_and(char::is_whitespace) + { + continue; + } + + let comment_stripped = strip_trailing_comment(remainder).trim(); + if comment_stripped.is_empty() { + continue; + } + + let path_tokens = + shell_words::split(comment_stripped).unwrap_or_else(|_| split_include_fallback(comment_stripped)); + + for token in path_tokens { + if token.is_empty() || contains_dynamic_make_syntax(&token) { + continue; + } + + includes.push(MakefileInclude { + path: PathBuf::from(token), + optional, + }); + } + } + + includes +} + +fn collapse_line_continuations(content: &str) -> String { + let mut collapsed = String::new(); + let mut current = String::new(); + let mut continuing = false; + + for line in content.lines() { + let trimmed_end = line.trim_end(); + let continues = trimmed_end.ends_with('\\'); + let segment = if continues { + trimmed_end.trim_end_matches('\\').trim_end() + } else { + trimmed_end + }; + + if !current.is_empty() { + current.push(' '); + } + if continuing { + current.push_str(segment.trim_start()); + } else { + current.push_str(segment); + } + + if continues { + continuing = true; + continue; + } + + collapsed.push_str(¤t); + collapsed.push('\n'); + current.clear(); + continuing = false; + } + + if !current.is_empty() { + collapsed.push_str(¤t); + } + + collapsed +} + +fn strip_trailing_comment(input: &str) -> &str { + let mut in_single = false; + let mut in_double = false; + let mut escape = false; + + for (idx, ch) in input.char_indices() { + match ch { + '\\' if !escape => { + escape = true; + continue; + } + '\'' if !escape && !in_double => in_single = !in_single, + '"' if !escape && !in_single => in_double = !in_double, + '#' if !escape && !in_single && !in_double => return &input[..idx], + _ => {} + } + escape = false; + } + + input +} + +fn split_include_fallback(input: &str) -> Vec { + input + .split_whitespace() + .map(str::to_string) + .collect() +} + +fn contains_dynamic_make_syntax(token: &str) -> bool { + token.contains('$') || token.contains('*') || token.contains('?') || token.contains('[') +} + /// Extract tasks using regex as a fallback method when standard parsing fails fn extract_tasks_regex(content: &str, path: &Path) -> Result, String> { let mut tasks_map: HashMap = HashMap::new(); @@ -616,4 +759,42 @@ OTHER_VAR ::= value assert_eq!(tasks.len(), 0); } + + #[test] + fn test_extract_include_directives() { + let content = r#" +include common.mk more.mk +-include .env +sinclude config/local.mk +"#; + + let includes = extract_include_directives_from_str(content); + + assert_eq!(includes.len(), 4); + assert_eq!(includes[0].path, PathBuf::from("common.mk")); + assert!(!includes[0].optional); + assert_eq!(includes[1].path, PathBuf::from("more.mk")); + assert!(!includes[1].optional); + assert_eq!(includes[2].path, PathBuf::from(".env")); + assert!(includes[2].optional); + assert_eq!(includes[3].path, PathBuf::from("config/local.mk")); + assert!(includes[3].optional); + } + + #[test] + fn test_extract_include_directives_ignores_recipes_and_dynamic_paths() { + let content = r#" +include first.mk \ + second.mk +include $(GENERATED_MAKEFILES) *.mk +build: + @include from-recipe.mk +"#; + + let includes = extract_include_directives_from_str(content); + + assert_eq!(includes.len(), 2); + assert_eq!(includes[0].path, PathBuf::from("first.mk")); + assert_eq!(includes[1].path, PathBuf::from("second.mk")); + } } diff --git a/src/task_discovery.rs b/src/task_discovery.rs index 9b6c2aa..be3f8ad 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -1,4 +1,4 @@ -use crate::composed_paths::ComposedDefinitionSource; +use crate::composed_paths::{ComposedDefinitionSource, RecursiveDiscoveryState, VisitState}; use crate::parsers::{ parse_cmake, parse_docker_compose, parse_github_actions, parse_gradle, parse_justfile, parse_makefile, parse_package_json, parse_pom_xml, parse_pyproject_toml, parse_taskfile, @@ -309,21 +309,120 @@ fn discover_makefile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Resu return Ok(()); } - match parse_makefile::parse(&makefile_path) { - Ok(tasks) => { - handle_discovery_success( - tasks, - makefile_path, - TaskDefinitionType::Makefile, - discovered, + let root_source = ComposedDefinitionSource::direct(makefile_path.clone()); + let mut traversal_state = RecursiveDiscoveryState::new(); + let mut seen_task_names = std::collections::HashSet::new(); + let mut tasks = Vec::new(); + let mut include_errors = Vec::new(); + + let result = collect_makefile_tasks_recursive( + &makefile_path, + &root_source, + &mut traversal_state, + &mut seen_task_names, + &mut tasks, + &mut include_errors, + ); + + for task in &mut tasks { + task.shadowed_by = check_shadowing(&task.name); + } + + discovered.tasks.extend(tasks); + discovered.errors.extend(include_errors); + + let status = match result { + Ok(()) => TaskFileStatus::Parsed, + Err(error) => { + discovered + .errors + .push(format!("Failed to parse {}: {}", makefile_path.display(), error)); + TaskFileStatus::ParseError(error) + } + }; + discovered.definitions.makefile = Some(TaskDefinitionFile { + path: makefile_path, + definition_type: TaskDefinitionType::Makefile, + status, + }); + + Ok(()) +} + +fn collect_makefile_tasks_recursive( + root_makefile_path: &Path, + current_source: &ComposedDefinitionSource, + traversal_state: &mut RecursiveDiscoveryState, + seen_task_names: &mut std::collections::HashSet, + collected_tasks: &mut Vec, + include_errors: &mut Vec, +) -> Result<(), String> { + match traversal_state.mark_visited(current_source.definition_path()) { + VisitState::AlreadyVisited(_) => return Ok(()), + VisitState::New(_) => {} + } + + let mut first_error: Option = None; + + let mut tasks = parse_makefile::parse(current_source.definition_path())?; + for task in &mut tasks { + current_source.apply_to_task(task); + } + for task in tasks { + if seen_task_names.insert(task.name.clone()) { + collected_tasks.push(task); + } + } + + let includes = parse_makefile::extract_include_directives(current_source.definition_path())?; + + for include in includes { + let resolved_include = current_source.resolve_child(&include.path); + + if !resolved_include.is_file() { + if include.optional { + continue; + } + + let error = format!( + "Required included makefile '{}' referenced from '{}' was not found", + resolved_include.display(), + current_source.definition_path().display() ); + include_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); + } + continue; } - Err(e) => { - handle_discovery_error(e, makefile_path, TaskDefinitionType::Makefile, discovered); + + let include_source = + ComposedDefinitionSource::composed(root_makefile_path, resolved_include.clone()); + if let Err(error) = collect_makefile_tasks_recursive( + root_makefile_path, + &include_source, + traversal_state, + seen_task_names, + collected_tasks, + include_errors, + ) { + let error = format!( + "Failed to parse included makefile '{}': {}", + resolved_include.display(), + error + ); + include_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); + } } } - Ok(()) + if let Some(error) = first_error { + Err(error) + } else { + Ok(()) + } } fn discover_npm_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { @@ -1017,6 +1116,144 @@ test: assert_eq!(test_task.description, Some("Running tests".to_string())); } + #[test] + fn test_discover_tasks_with_included_makefiles() { + let temp_dir = TempDir::new().unwrap(); + let included_dir = temp_dir.path().join("mk"); + std::fs::create_dir_all(&included_dir).unwrap(); + + create_test_makefile( + temp_dir.path(), + r#"include mk/common.mk + +build: + @echo "Build from root""#, + ); + std::fs::write( + included_dir.join("common.mk"), + r#"include nested.mk + +test: + @echo "Test from common""#, + ) + .unwrap(); + std::fs::write( + included_dir.join("nested.mk"), + r#"lint: + @echo "Lint from nested""#, + ) + .unwrap(); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert!(matches!( + discovered.definitions.makefile.unwrap().status, + TaskFileStatus::Parsed + )); + assert_eq!(discovered.tasks.len(), 3); + + let root_makefile = temp_dir.path().join("Makefile"); + let common_makefile = included_dir.join("common.mk"); + let nested_makefile = included_dir.join("nested.mk"); + + let build_task = discovered.tasks.iter().find(|t| t.name == "build").unwrap(); + assert_eq!(build_task.file_path, root_makefile); + assert_eq!(build_task.definition_path(), root_makefile.as_path()); + + let test_task = discovered.tasks.iter().find(|t| t.name == "test").unwrap(); + assert_eq!(test_task.file_path, root_makefile); + assert_eq!(test_task.definition_path(), common_makefile.as_path()); + + let lint_task = discovered.tasks.iter().find(|t| t.name == "lint").unwrap(); + assert_eq!(lint_task.file_path, root_makefile); + assert_eq!(lint_task.definition_path(), nested_makefile.as_path()); + } + + #[test] + fn test_discover_tasks_with_optional_missing_included_makefile() { + let temp_dir = TempDir::new().unwrap(); + create_test_makefile( + temp_dir.path(), + r#"-include missing.mk + +build: + @echo "Build from root""#, + ); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(matches!( + discovered.definitions.makefile.unwrap().status, + TaskFileStatus::Parsed + )); + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert_eq!(discovered.tasks.len(), 1); + assert!(discovered.tasks.iter().any(|t| t.name == "build")); + } + + #[test] + fn test_discover_tasks_with_recursive_makefile_include_cycle() { + let temp_dir = TempDir::new().unwrap(); + let included_dir = temp_dir.path().join("mk"); + std::fs::create_dir_all(&included_dir).unwrap(); + + create_test_makefile( + temp_dir.path(), + r#"include mk/common.mk + +build: + @echo "Build from root""#, + ); + std::fs::write( + included_dir.join("common.mk"), + r#"include ../Makefile + +test: + @echo "Test from common""#, + ) + .unwrap(); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(matches!( + discovered.definitions.makefile.unwrap().status, + TaskFileStatus::Parsed + )); + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert_eq!(discovered.tasks.len(), 2); + assert!(discovered.tasks.iter().any(|t| t.name == "build")); + assert!(discovered.tasks.iter().any(|t| t.name == "test")); + } + + #[test] + fn test_discover_tasks_with_missing_required_included_makefile() { + let temp_dir = TempDir::new().unwrap(); + create_test_makefile( + temp_dir.path(), + r#"include missing.mk + +build: + @echo "Build from root""#, + ); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(matches!( + discovered.definitions.makefile.unwrap().status, + TaskFileStatus::ParseError(_) + )); + assert!(discovered.tasks.iter().any(|t| t.name == "build")); + assert!( + discovered + .errors + .iter() + .any(|error| error.contains("missing.mk")), + "{:?}", + discovered.errors + ); + } + #[test] fn test_discover_tasks_finds_turbo_json_at_git_repo_root() { let temp_dir = TempDir::new().unwrap(); diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 3576111..1994d95 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -402,6 +402,46 @@ else exit 1 fi +# Test 22c: Verify recursive Makefile include discovery +echo "\nTest 22c: Testing recursive Makefile include discovery" +mkdir -p /home/testuser/make_include_project/mk +cat > /home/testuser/make_include_project/Makefile <<'EOF' +include mk/common.mk +-include mk/missing.mk + +build: + @echo "Build from root" +EOF +cat > /home/testuser/make_include_project/mk/common.mk <<'EOF' +include nested.mk + +included-task: + @echo "Included task" +EOF +cat > /home/testuser/make_include_project/mk/nested.mk <<'EOF' +nested-task: + @echo "Nested task" +EOF + +cd /home/testuser/make_include_project +dela list 2>/dev/null > make_include_list.txt +if grep -q "included-task" make_include_list.txt && grep -q "nested-task" make_include_list.txt; then + echo "${GREEN}✓ dela list discovers tasks from recursively included makefiles${NC}" +else + echo "${RED}✗ dela list did not discover tasks from recursively included makefiles${NC}" + cat make_include_list.txt + exit 1 +fi + +output=$(dela get-command nested-task 2>&1) +if echo "$output" | grep -q "make nested-task"; then + echo "${GREEN}✓ get-command works for tasks from included makefiles${NC}" +else + echo "${RED}✗ get-command failed for task from included makefiles${NC}" + echo "Got: $output" + exit 1 +fi + cd /home/testuser/test_project # Test 23: Verify ambiguous task detection in dela list From dbc4faf361b0d8775977922c75fbe969260d4ed1 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 14:58:57 -0400 Subject: [PATCH 05/13] deal better --- src/task_discovery.rs | 24 ++---------------------- tests/docker_noinit/test_noinit.sh | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/task_discovery.rs b/src/task_discovery.rs index be3f8ad..f4a4602 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -380,19 +380,6 @@ fn collect_makefile_tasks_recursive( let resolved_include = current_source.resolve_child(&include.path); if !resolved_include.is_file() { - if include.optional { - continue; - } - - let error = format!( - "Required included makefile '{}' referenced from '{}' was not found", - resolved_include.display(), - current_source.definition_path().display() - ); - include_errors.push(error.clone()); - if first_error.is_none() { - first_error = Some(error); - } continue; } @@ -1241,17 +1228,10 @@ build: assert!(matches!( discovered.definitions.makefile.unwrap().status, - TaskFileStatus::ParseError(_) + TaskFileStatus::Parsed )); + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); assert!(discovered.tasks.iter().any(|t| t.name == "build")); - assert!( - discovered - .errors - .iter() - .any(|error| error.contains("missing.mk")), - "{:?}", - discovered.errors - ); } #[test] diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 1994d95..4c851ac 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -407,7 +407,8 @@ echo "\nTest 22c: Testing recursive Makefile include discovery" mkdir -p /home/testuser/make_include_project/mk cat > /home/testuser/make_include_project/Makefile <<'EOF' include mk/common.mk --include mk/missing.mk +include mk/missing-required.mk +-include mk/missing-optional.mk build: @echo "Build from root" @@ -424,7 +425,13 @@ nested-task: EOF cd /home/testuser/make_include_project -dela list 2>/dev/null > make_include_list.txt +dela list > make_include_list.txt 2> make_include_stderr.txt +if [ $? -ne 0 ]; then + echo "${RED}✗ dela list failed when a required included makefile was missing${NC}" + cat make_include_stderr.txt + exit 1 +fi + if grep -q "included-task" make_include_list.txt && grep -q "nested-task" make_include_list.txt; then echo "${GREEN}✓ dela list discovers tasks from recursively included makefiles${NC}" else @@ -433,6 +440,12 @@ else exit 1 fi +if [ -s make_include_stderr.txt ]; then + echo "${RED}✗ dela emitted errors for missing included makefiles${NC}" + cat make_include_stderr.txt + exit 1 +fi + output=$(dela get-command nested-task 2>&1) if echo "$output" | grep -q "make nested-task"; then echo "${GREEN}✓ get-command works for tasks from included makefiles${NC}" From 5a919681c89117201eaaa368291393362ec8cd1c Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 15:18:05 -0400 Subject: [PATCH 06/13] 199 --- dev_docs/project_plan.md | 2 +- src/commands/allow_command.rs | 45 +++++++++++ src/commands/list.rs | 117 ++++++++++++++++++++++------- src/composed_paths.rs | 11 +-- src/mcp/dto.rs | 20 +++++ src/parsers/parse_makefile.rs | 17 ++--- src/task_discovery.rs | 76 ++++++++++++++++++- tests/docker_noinit/test_noinit.sh | 17 +++++ 8 files changed, 252 insertions(+), 53 deletions(-) diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index 0e095dc..e0e10a3 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -259,7 +259,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [ ] **Composed / Included Task Definitions** - [x] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. - [x] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. - - [ ] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. + - [x] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. - [ ] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. - [ ] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. diff --git a/src/commands/allow_command.rs b/src/commands/allow_command.rs index cdd1244..7924e05 100644 --- a/src/commands/allow_command.rs +++ b/src/commands/allow_command.rs @@ -406,4 +406,49 @@ test: ## Running tests reset_to_real_environment(); } + + #[test] + #[serial] + fn test_allow_command_uses_definition_path_for_included_make_task() { + let (project_dir, home_dir) = setup_test_env(); + env::set_current_dir(&project_dir).expect("Failed to change directory"); + + fs::create_dir_all(project_dir.path().join("mk")).expect("Failed to create include dir"); + fs::write( + project_dir.path().join("Makefile"), + r#"include mk/common.mk + +build: + @echo Building..."#, + ) + .expect("Failed to write root Makefile"); + fs::write( + project_dir.path().join("mk").join("common.mk"), + r#"included-task: + @echo Included"#, + ) + .expect("Failed to write included Makefile"); + + let result = execute("included-task", Some(3)); // 3 = file scope + assert!( + result.is_ok(), + "Should allow included task by defining file" + ); + + let allowlist = fs::read_to_string(preferred_allowlist_path_for(home_dir.path())).unwrap(); + assert!( + allowlist.contains( + &project_dir + .path() + .join("mk") + .join("common.mk") + .display() + .to_string() + ), + "allowlist should use included file path, got:\n{}", + allowlist + ); + + reset_to_real_environment(); + } } diff --git a/src/commands/list.rs b/src/commands/list.rs index 6b200d8..0f188e3 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -3,9 +3,10 @@ use crate::task_discovery; use crate::types::ShadowType; use crate::types::{Task, TaskFileStatus}; use colored::Colorize; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::env; use std::io::Write; +use std::path::Path; #[cfg(test)] macro_rules! test_println { @@ -212,16 +213,6 @@ pub fn execute(verbose: bool) -> Result<(), String> { "No tasks found in the current directory.".yellow() ))?; } else { - // Collect file paths for each runner for reference - let mut runner_files: HashMap = HashMap::new(); - for task in &discovered.tasks { - let runner_name = task.runner.short_name().to_string(); - runner_files.insert( - runner_name, - task.definition_path().to_string_lossy().to_string(), - ); - } - // Calculate max task name width across all runners let max_task_name_width = discovered .tasks @@ -264,25 +255,19 @@ pub fn execute(verbose: bool) -> Result<(), String> { None }; - // Get file path for this runner - let empty_string = String::new(); - let file_path = runner_files.get(&runner).unwrap_or(&empty_string); - - // For GitHub Actions, show the full relative path instead of just the filename - let display_path = if runner == "act" { - let path = std::path::Path::new(file_path); - if let Ok(relative_path) = path.strip_prefix(¤t_dir) { - relative_path.to_string_lossy().to_string() - } else { - file_path.clone() - } + let runner_paths: HashSet<_> = + sorted_tasks.iter().map(|task| &task.file_path).collect(); + let display_path = if runner_paths.len() == 1 { + format_runner_path_for_display(&runner, &sorted_tasks[0].file_path, ¤t_dir) } else { - // For other runners, show just the filename - std::path::Path::new(file_path) - .file_name() - .map(|f| f.to_string_lossy().to_string()) - .unwrap_or_else(|| file_path.clone()) + "multiple files".to_string() }; + let show_task_sources = sorted_tasks + .iter() + .map(|task| task.definition_path().to_path_buf()) + .collect::>() + .len() + > 1; // Write section header let colored_runner = if tool_not_installed { @@ -318,6 +303,11 @@ pub fn execute(verbose: bool) -> Result<(), String> { // Format the task entry let formatted_task = format_task_entry(task, is_ambiguous, display_width); + let source_label = show_task_sources.then(|| { + format_definition_path_for_display(task.definition_path(), ¤t_dir) + }); + let formatted_task = + format_task_entry_with_source(formatted_task, source_label.as_deref()); write_line(&format!(" {}", formatted_task))?; } } @@ -452,6 +442,37 @@ fn format_task_entry(task: &Task, is_ambiguous: bool, name_width: usize) -> Stri format!("{} {}", padded_name, colored_description) } +fn format_task_entry_with_source(formatted_task: String, source_label: Option<&str>) -> String { + match source_label { + Some(source_label) if !source_label.is_empty() => { + format!( + "{} {}", + formatted_task, + format!("[{}]", source_label).dimmed() + ) + } + _ => formatted_task, + } +} + +fn format_definition_path_for_display(path: &Path, current_dir: &Path) -> String { + if let Ok(relative_path) = path.strip_prefix(current_dir) { + relative_path.to_string_lossy().to_string() + } else { + path.to_string_lossy().to_string() + } +} + +fn format_runner_path_for_display(runner: &str, path: &Path, current_dir: &Path) -> String { + if runner == "act" { + format_definition_path_for_display(path, current_dir) + } else { + path.file_name() + .map(|file_name| file_name.to_string_lossy().to_string()) + .unwrap_or_else(|| format_definition_path_for_display(path, current_dir)) + } +} + #[cfg(test)] mod tests { use super::*; @@ -460,7 +481,7 @@ mod tests { use serial_test::serial; use std::fs::{self, File}; use std::io::{self, Write}; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; use tempfile::TempDir; // Custom writer for testing @@ -957,4 +978,42 @@ mod tests { "Should show task description" ); } + + #[test] + fn test_runner_path_display_uses_execution_context_for_composed_tasks() { + let current_dir = Path::new("/project"); + let runner_path = Path::new("/project/Makefile"); + let definition_path = Path::new("/project/mk/common.mk"); + + assert_eq!( + format_runner_path_for_display("make", runner_path, current_dir), + "Makefile" + ); + assert_eq!( + format_definition_path_for_display(definition_path, current_dir), + "mk/common.mk" + ); + } + + #[test] + fn test_task_entry_source_suffix_uses_definition_path_for_composed_tasks() { + let task = Task { + name: "included-task".to_string(), + file_path: PathBuf::from("/project/Makefile"), + definition_path: Some(PathBuf::from("/project/mk/common.mk")), + definition_type: TaskDefinitionType::Makefile, + runner: TaskRunner::Make, + source_name: "included-task".to_string(), + description: Some("Included task".to_string()), + shadowed_by: None, + disambiguated_name: None, + }; + + let formatted = format_task_entry(&task, false, 18); + let formatted = format_task_entry_with_source(formatted, Some("mk/common.mk")); + + assert!(formatted.contains("included-task")); + assert!(formatted.contains("Included task")); + assert!(formatted.contains("[mk/common.mk]")); + } } diff --git a/src/composed_paths.rs b/src/composed_paths.rs index bcd97b7..58abe27 100644 --- a/src/composed_paths.rs +++ b/src/composed_paths.rs @@ -24,10 +24,7 @@ impl ComposedDefinitionSource { } /// Create a source where the runner path differs from the defining file. - pub fn composed( - runner_path: impl Into, - definition_path: impl Into, - ) -> Self { + pub fn composed(runner_path: impl Into, definition_path: impl Into) -> Self { Self { runner_path: normalize_path(runner_path.into()), definition_path: normalize_path(definition_path.into()), @@ -220,10 +217,8 @@ mod tests { #[test] fn test_composed_source_resolves_children_relative_to_definition_file() { - let source = ComposedDefinitionSource::composed( - "/repo/Makefile", - "/repo/includes/common/tasks.mk", - ); + let source = + ComposedDefinitionSource::composed("/repo/Makefile", "/repo/includes/common/tasks.mk"); assert_eq!( source.resolve_child("../../other.mk"), diff --git a/src/mcp/dto.rs b/src/mcp/dto.rs index b775f28..06a462a 100644 --- a/src/mcp/dto.rs +++ b/src/mcp/dto.rs @@ -265,6 +265,26 @@ mod tests { ); } + #[test] + fn test_taskdto_uses_definition_path_for_composed_make_task() { + let task = Task { + name: "included-task".to_string(), + file_path: PathBuf::from("/project/Makefile"), + definition_path: Some(PathBuf::from("/project/mk/common.mk")), + definition_type: TaskDefinitionType::Makefile, + runner: TaskRunner::Make, + source_name: "included-task".to_string(), + description: Some("Included task".to_string()), + shadowed_by: None, + disambiguated_name: None, + }; + + let dto = TaskDto::from_task(&task); + + assert_eq!(dto.command, "make included-task"); + assert_eq!(dto.file_path, "/project/mk/common.mk"); + } + #[test] fn test_taskdto_serialization() { // Arrange diff --git a/src/parsers/parse_makefile.rs b/src/parsers/parse_makefile.rs index 11da5a1..573fdad 100644 --- a/src/parsers/parse_makefile.rs +++ b/src/parsers/parse_makefile.rs @@ -2,8 +2,8 @@ use crate::types::{Task, TaskDefinitionType, TaskRunner}; use makefile_lossless::Makefile; use regex::Regex; use std::collections::HashMap; -use std::path::PathBuf; use std::path::Path; +use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Eq)] pub struct MakefileInclude { @@ -125,11 +125,7 @@ fn extract_include_directives_from_str(content: &str) -> Vec { continue; }; - if !remainder - .chars() - .next() - .is_some_and(char::is_whitespace) - { + if !remainder.chars().next().is_some_and(char::is_whitespace) { continue; } @@ -138,8 +134,8 @@ fn extract_include_directives_from_str(content: &str) -> Vec { continue; } - let path_tokens = - shell_words::split(comment_stripped).unwrap_or_else(|_| split_include_fallback(comment_stripped)); + let path_tokens = shell_words::split(comment_stripped) + .unwrap_or_else(|_| split_include_fallback(comment_stripped)); for token in path_tokens { if token.is_empty() || contains_dynamic_make_syntax(&token) { @@ -220,10 +216,7 @@ fn strip_trailing_comment(input: &str) -> &str { } fn split_include_fallback(input: &str) -> Vec { - input - .split_whitespace() - .map(str::to_string) - .collect() + input.split_whitespace().map(str::to_string).collect() } fn contains_dynamic_make_syntax(token: &str) -> bool { diff --git a/src/task_discovery.rs b/src/task_discovery.rs index f4a4602..28a989b 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -334,9 +334,11 @@ fn discover_makefile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Resu let status = match result { Ok(()) => TaskFileStatus::Parsed, Err(error) => { - discovered - .errors - .push(format!("Failed to parse {}: {}", makefile_path.display(), error)); + discovered.errors.push(format!( + "Failed to parse {}: {}", + makefile_path.display(), + error + )); TaskFileStatus::ParseError(error) } }; @@ -1157,6 +1159,74 @@ test: assert_eq!(lint_task.definition_path(), nested_makefile.as_path()); } + #[test] + #[serial] + fn test_duplicate_task_names_from_included_makefile_use_definition_path() { + let temp_dir = TempDir::new().unwrap(); + let included_dir = temp_dir.path().join("mk"); + std::fs::create_dir_all(&included_dir).unwrap(); + + reset_mock(); + enable_mock(); + mock_executable("npm"); + + create_test_makefile( + temp_dir.path(), + r#"include mk/common.mk + +build: + @echo "Build from root""#, + ); + std::fs::write( + included_dir.join("common.mk"), + r#"test: + @echo "Test from included makefile""#, + ) + .unwrap(); + std::fs::write( + temp_dir.path().join("package.json"), + r#"{ + "name": "test-package", + "scripts": { + "test": "jest" + } +}"#, + ) + .unwrap(); + std::fs::write(temp_dir.path().join("package-lock.json"), "{}").unwrap(); + + let discovered = discover_tasks(temp_dir.path()); + let matching_tasks: Vec<&Task> = discovered + .tasks + .iter() + .filter(|task| task.name == "test") + .collect(); + + assert_eq!(matching_tasks.len(), 2); + + let make_task = matching_tasks + .iter() + .copied() + .find(|task| task.runner == TaskRunner::Make) + .unwrap(); + assert_eq!( + make_task.definition_path(), + included_dir.join("common.mk").as_path() + ); + assert_eq!(make_task.file_path, temp_dir.path().join("Makefile")); + assert_eq!(make_task.disambiguated_name.as_deref(), Some("test-m")); + + let error = format_ambiguous_task_error("test", &matching_tasks); + assert!( + error.contains("mk/common.mk"), + "unexpected ambiguous-task error: {}", + error + ); + + reset_mock(); + reset_to_real_environment(); + } + #[test] fn test_discover_tasks_with_optional_missing_included_makefile() { let temp_dir = TempDir::new().unwrap(); diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 4c851ac..a244e0a 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -446,6 +446,14 @@ if [ -s make_include_stderr.txt ]; then exit 1 fi +if grep -q "mk/common.mk" make_include_list.txt && grep -q "mk/nested.mk" make_include_list.txt; then + echo "${GREEN}✓ dela list shows defining files for included make tasks${NC}" +else + echo "${RED}✗ dela list did not show defining files for included make tasks${NC}" + cat make_include_list.txt + exit 1 +fi + output=$(dela get-command nested-task 2>&1) if echo "$output" | grep -q "make nested-task"; then echo "${GREEN}✓ get-command works for tasks from included makefiles${NC}" @@ -455,6 +463,15 @@ else exit 1 fi +echo "3" | dela allow-command included-task >/dev/null 2>&1 +if grep -q "/home/testuser/make_include_project/mk/common.mk" /home/testuser/.config/dela/allowlist.toml; then + echo "${GREEN}✓ allow-command stores the defining included makefile path${NC}" +else + echo "${RED}✗ allow-command did not store the defining included makefile path${NC}" + cat /home/testuser/.config/dela/allowlist.toml + exit 1 +fi + cd /home/testuser/test_project # Test 23: Verify ambiguous task detection in dela list From 5fe75afd88e1e494c59541dd51b7e620b9194a88 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 15:45:23 -0400 Subject: [PATCH 07/13] taskfile --- dev_docs/project_plan.md | 2 +- src/parsers/parse_taskfile.rs | 225 ++++++++++++++++-- src/task_discovery.rs | 358 +++++++++++++++++++++++++---- tests/docker_noinit/test_noinit.sh | 93 ++++++++ 4 files changed, 619 insertions(+), 59 deletions(-) diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index e0e10a3..dea43ad 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -260,7 +260,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [x] [DTKT-197] Define shared recursive discovery primitives and allowlist/source-attribution handling for composed task definitions so Make, Taskfile, and Turborepo can reuse the same traversal semantics. - [x] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. - [x] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. - - [ ] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. + - [x] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. - [ ] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. - [ ] **Additional Task Runners support** diff --git a/src/parsers/parse_taskfile.rs b/src/parsers/parse_taskfile.rs index 517f5b0..a3296f7 100644 --- a/src/parsers/parse_taskfile.rs +++ b/src/parsers/parse_taskfile.rs @@ -1,7 +1,20 @@ use crate::types::{Task, TaskDefinitionType, TaskRunner}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; + +pub const SUPPORTED_TASKFILE_NAMES: [&str; 8] = [ + "Taskfile.yml", + "taskfile.yml", + "Taskfile.yaml", + "taskfile.yaml", + "Taskfile.dist.yml", + "taskfile.dist.yml", + "Taskfile.dist.yaml", + "taskfile.dist.yaml", +]; + +const DEFAULT_TASKFILE_NAME: &str = SUPPORTED_TASKFILE_NAMES[0]; #[derive(Debug, Serialize, Deserialize)] #[serde(untagged)] @@ -26,27 +39,78 @@ struct TaskfileTask { } #[derive(Debug, Serialize, Deserialize)] +#[serde(untagged)] +enum TaskfileIncludeEntry { + Shorthand(String), + Detailed(TaskfileIncludeConfig), +} + +#[derive(Debug, Serialize, Deserialize)] +#[allow(dead_code)] // We parse a broader subset of the schema than DTKT-200 consumes. +struct TaskfileIncludeConfig { + taskfile: String, + optional: Option, + flatten: Option, + internal: Option, + excludes: Option>, + aliases: Option>, + dir: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TaskfileInclude { + pub namespace: String, + pub taskfile: PathBuf, + pub optional: bool, + pub flatten: bool, + pub internal: bool, + pub excludes: Vec, +} + +#[derive(Debug, Serialize, Deserialize)] +#[allow(dead_code)] // Some top-level schema fields are currently only parsed for compatibility. struct Taskfile { version: Option, + #[serde(default)] + includes: HashMap, + #[serde(default)] tasks: HashMap, } -/// Parse a Taskfile.yml file at the given path and extract tasks -pub fn parse(path: &PathBuf) -> Result, String> { - let file_name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("Taskfile"); +pub fn find_taskfile_in_dir(dir: &Path) -> Option { + for filename in SUPPORTED_TASKFILE_NAMES { + let path = dir.join(filename); + if path.is_file() { + return Some(path); + } + } - let contents = std::fs::read_to_string(path) - .map_err(|e| format!("Failed to read {}: {}", file_name, e))?; + None +} - let taskfile: Taskfile = serde_yaml::from_str(&contents) - .map_err(|e| format!("Failed to parse {}: {}", file_name, e))?; +pub fn resolve_taskfile_include_path(candidate: &Path) -> PathBuf { + if candidate.is_dir() { + return find_taskfile_in_dir(candidate) + .unwrap_or_else(|| candidate.join(DEFAULT_TASKFILE_NAME)); + } + + if candidate.is_file() || looks_like_taskfile_file(candidate) || candidate.extension().is_some() + { + return candidate.to_path_buf(); + } + + candidate.join(DEFAULT_TASKFILE_NAME) +} + +/// Parse a Taskfile.yml file at the given path and extract tasks +pub fn parse(path: &Path) -> Result, String> { + let taskfile = load_taskfile(path)?; + let mut task_entries: Vec<_> = taskfile.tasks.into_iter().collect(); + task_entries.sort_by(|a, b| a.0.cmp(&b.0)); let mut tasks = Vec::new(); - for (name, task_def) in taskfile.tasks { + for (name, task_def) in task_entries { // Skip tasks marked as internal if task_def.internal.unwrap_or(false) { continue; @@ -70,7 +134,7 @@ pub fn parse(path: &PathBuf) -> Result, String> { tasks.push(Task { name: name.clone(), - file_path: path.clone(), + file_path: path.to_path_buf(), definition_path: None, definition_type: TaskDefinitionType::Taskfile, runner: TaskRunner::Task, @@ -84,6 +148,68 @@ pub fn parse(path: &PathBuf) -> Result, String> { Ok(tasks) } +pub fn extract_include_directives(path: &Path) -> Result, String> { + let taskfile = load_taskfile(path)?; + let mut include_entries: Vec<_> = taskfile.includes.into_iter().collect(); + include_entries.sort_by(|a, b| a.0.cmp(&b.0)); + + let mut includes = Vec::new(); + + for (namespace, include) in include_entries { + let include = match include { + TaskfileIncludeEntry::Shorthand(taskfile) => TaskfileInclude { + namespace, + taskfile: PathBuf::from(taskfile), + optional: false, + flatten: false, + internal: false, + excludes: Vec::new(), + }, + TaskfileIncludeEntry::Detailed(config) => TaskfileInclude { + namespace, + taskfile: PathBuf::from(config.taskfile), + optional: config.optional.unwrap_or(false), + flatten: config.flatten.unwrap_or(false), + internal: config.internal.unwrap_or(false), + excludes: config.excludes.unwrap_or_default(), + }, + }; + + if should_skip_non_local_include(&include.taskfile) { + continue; + } + + includes.push(include); + } + + Ok(includes) +} + +fn load_taskfile(path: &Path) -> Result { + let file_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("Taskfile"); + + let contents = std::fs::read_to_string(path) + .map_err(|e| format!("Failed to read {}: {}", file_name, e))?; + + let taskfile = serde_yaml::from_str(&contents) + .map_err(|e| format!("Failed to parse {}: {}", file_name, e))?; + Ok(taskfile) +} + +fn looks_like_taskfile_file(path: &Path) -> bool { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| SUPPORTED_TASKFILE_NAMES.contains(&name)) +} + +fn should_skip_non_local_include(path: &Path) -> bool { + let path = path.to_string_lossy(); + path.contains("://") || path.contains("{{") || path.contains("}}") +} + #[cfg(test)] mod tests { use super::*; @@ -277,4 +403,77 @@ tasks: ); assert_eq!(args_task.runner, TaskRunner::Task); } + + #[test] + fn test_extract_include_directives() { + let temp_dir = TempDir::new().unwrap(); + let taskfile_path = temp_dir.path().join("Taskfile.yml"); + + std::fs::write( + &taskfile_path, + r#" +version: '3' +includes: + docs: ./docs + shared: + taskfile: ./shared/Taskfile.dist.yml + optional: true + flatten: true + internal: true + excludes: [build] + remote: + taskfile: https://example.com/tasks.yml + templated: ./Taskfile_{{OS}}.yml +"#, + ) + .unwrap(); + + let includes = extract_include_directives(&taskfile_path).unwrap(); + assert_eq!(includes.len(), 2); + + assert_eq!(includes[0].namespace, "docs"); + assert_eq!(includes[0].taskfile, PathBuf::from("./docs")); + assert!(!includes[0].optional); + assert!(!includes[0].flatten); + assert!(!includes[0].internal); + assert!(includes[0].excludes.is_empty()); + + assert_eq!(includes[1].namespace, "shared"); + assert_eq!( + includes[1].taskfile, + PathBuf::from("./shared/Taskfile.dist.yml") + ); + assert!(includes[1].optional); + assert!(includes[1].flatten); + assert!(includes[1].internal); + assert_eq!(includes[1].excludes, vec!["build".to_string()]); + } + + #[test] + fn test_find_taskfile_in_dir_and_resolve_include_path() { + let temp_dir = TempDir::new().unwrap(); + let include_dir = temp_dir.path().join("docs"); + std::fs::create_dir_all(&include_dir).unwrap(); + std::fs::write(include_dir.join("taskfile.yaml"), "version: '3'\n").unwrap(); + + let resolved_path = find_taskfile_in_dir(&include_dir).unwrap(); + assert!( + resolved_path == include_dir.join("Taskfile.yaml") + || resolved_path == include_dir.join("taskfile.yaml") + ); + let include_path = resolve_taskfile_include_path(&include_dir); + assert!( + include_path == include_dir.join("Taskfile.yaml") + || include_path == include_dir.join("taskfile.yaml") + ); + + let missing_dir = temp_dir.path().join("missing"); + assert_eq!( + resolve_taskfile_include_path(&missing_dir), + missing_dir.join("Taskfile.yml") + ); + + let explicit_file = temp_dir.path().join("Shared.yml"); + assert_eq!(resolve_taskfile_include_path(&explicit_file), explicit_file); + } } diff --git a/src/task_discovery.rs b/src/task_discovery.rs index 28a989b..c217384 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -478,63 +478,189 @@ fn discover_python_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result } fn discover_taskfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { - // List of possible Taskfile paths in order of priority - let possible_taskfiles = [ - "Taskfile.yml", - "taskfile.yml", - "Taskfile.yaml", - "taskfile.yaml", - "Taskfile.dist.yml", - "taskfile.dist.yml", - "Taskfile.dist.yaml", - "taskfile.dist.yaml", - ]; - - // Try to find the first existing Taskfile - let mut taskfile_path = None; - for filename in &possible_taskfiles { - let path = dir.join(filename); - if path.exists() { - taskfile_path = Some(path); - break; + let default_path = dir.join(parse_taskfile::SUPPORTED_TASKFILE_NAMES[0]); + let Some(taskfile_path) = parse_taskfile::find_taskfile_in_dir(dir) else { + discovered.definitions.taskfile = Some(TaskDefinitionFile { + path: default_path, + definition_type: TaskDefinitionType::Taskfile, + status: TaskFileStatus::NotFound, + }); + return Ok(()); + }; + + let root_source = ComposedDefinitionSource::direct(taskfile_path.clone()); + let mut traversal_state = RecursiveDiscoveryState::new(); + let mut seen_task_names = std::collections::HashSet::new(); + let mut tasks = Vec::new(); + let mut include_errors = Vec::new(); + let no_excludes = std::collections::HashSet::new(); + + let result = collect_taskfile_tasks_recursive( + &taskfile_path, + &root_source, + "", + None, + false, + &no_excludes, + &mut traversal_state, + &mut seen_task_names, + &mut tasks, + &mut include_errors, + ); + + for task in &mut tasks { + task.shadowed_by = check_shadowing(&task.name); + } + + discovered.tasks.extend(tasks); + discovered.errors.extend(include_errors); + + let status = match result { + Ok(()) => TaskFileStatus::Parsed, + Err(error) => { + discovered.errors.push(format!( + "Failed to parse {}: {}", + taskfile_path.display(), + error + )); + TaskFileStatus::ParseError(error) + } + }; + discovered.definitions.taskfile = Some(TaskDefinitionFile { + path: taskfile_path, + definition_type: TaskDefinitionType::Taskfile, + status, + }); + + Ok(()) +} + +fn collect_taskfile_tasks_recursive( + root_taskfile_path: &Path, + current_source: &ComposedDefinitionSource, + namespace_prefix: &str, + include_label: Option<&str>, + hide_tasks: bool, + excluded_tasks: &std::collections::HashSet, + traversal_state: &mut RecursiveDiscoveryState, + seen_task_names: &mut std::collections::HashSet, + collected_tasks: &mut Vec, + include_errors: &mut Vec, +) -> Result<(), String> { + match traversal_state.mark_visited(current_source.definition_path()) { + VisitState::AlreadyVisited(_) => return Ok(()), + VisitState::New(_) => {} + } + + let mut first_error: Option = None; + + let mut tasks = parse_taskfile::parse(current_source.definition_path())?; + tasks.sort_by(|a, b| a.name.cmp(&b.name)); + + if !hide_tasks { + for mut task in tasks { + let original_name = task.name.clone(); + if excluded_tasks.contains(&original_name) { + continue; + } + + let effective_name = prefix_taskfile_task_name(namespace_prefix, &original_name); + task.name = effective_name.clone(); + task.source_name = effective_name; + current_source.apply_to_task(&mut task); + + if !seen_task_names.insert(task.name.clone()) { + let error = match include_label { + Some(include_label) => { + format!( + "Found multiple tasks ({}) included by \"{}\"", + task.name, include_label + ) + } + None => format!("Found multiple Taskfile tasks named '{}'", task.name), + }; + include_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); + } + continue; + } + + collected_tasks.push(task); } } - // Use a default path for reporting if no Taskfile was found - let default_path = dir.join("Taskfile.yml"); + let includes = parse_taskfile::extract_include_directives(current_source.definition_path())?; - // If a Taskfile was found, parse it - if let Some(taskfile_path) = taskfile_path { - let mut definition = TaskDefinitionFile { - path: taskfile_path.clone(), - definition_type: TaskDefinitionType::Taskfile, - status: TaskFileStatus::NotImplemented, - }; + for include in includes { + let resolved_candidate = current_source.resolve_child(&include.taskfile); + let resolved_include = parse_taskfile::resolve_taskfile_include_path(&resolved_candidate); - match parse_taskfile::parse(&taskfile_path) { - Ok(tasks) => { - definition.status = TaskFileStatus::Parsed; - discovered.tasks.extend(tasks); + if !resolved_include.is_file() { + if include.optional { + continue; } - Err(e) => { - definition.status = TaskFileStatus::ParseError(e.clone()); - discovered - .errors - .push(format!("Error parsing {}: {}", taskfile_path.display(), e)); + + let error = format!( + "Required included Taskfile '{}' referenced from '{}' was not found", + resolved_include.display(), + current_source.definition_path().display() + ); + include_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); + } + continue; + } + + let child_source = + ComposedDefinitionSource::composed(root_taskfile_path, resolved_include.clone()); + let child_namespace = if include.flatten { + namespace_prefix.to_string() + } else { + prefix_taskfile_task_name(namespace_prefix, &include.namespace) + }; + let child_include_label = prefix_taskfile_task_name(namespace_prefix, &include.namespace); + let child_hide_tasks = hide_tasks || include.internal; + let child_excludes = include.excludes.into_iter().collect(); + + if let Err(error) = collect_taskfile_tasks_recursive( + root_taskfile_path, + &child_source, + &child_namespace, + Some(child_include_label.as_str()), + child_hide_tasks, + &child_excludes, + traversal_state, + seen_task_names, + collected_tasks, + include_errors, + ) { + let error = format!( + "Failed to parse included Taskfile '{}': {}", + resolved_include.display(), + error + ); + include_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); } } + } - set_definition(discovered, definition); + if let Some(error) = first_error { + Err(error) } else { - // No Taskfile found, set status as NotFound - discovered.definitions.taskfile = Some(TaskDefinitionFile { - path: default_path, - definition_type: TaskDefinitionType::Taskfile, - status: TaskFileStatus::NotFound, - }); + Ok(()) } +} - Ok(()) +fn prefix_taskfile_task_name(namespace_prefix: &str, task_name: &str) -> String { + if namespace_prefix.is_empty() { + task_name.to_string() + } else { + format!("{}:{}", namespace_prefix, task_name) + } } fn discover_turbo_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { @@ -1895,6 +2021,148 @@ tasks: reset_mock(); } + #[test] + #[serial] + fn test_discover_tasks_with_included_taskfiles() { + let temp_dir = TempDir::new().unwrap(); + let docs_dir = temp_dir.path().join("docs"); + let api_dir = docs_dir.join("api"); + std::fs::create_dir_all(&api_dir).unwrap(); + + reset_mock(); + enable_mock(); + mock_executable("task"); + + std::fs::write( + temp_dir.path().join("Taskfile.yml"), + r#"version: '3' +includes: + docs: ./docs +tasks: + build: + desc: Build task + cmds: + - echo "Build""#, + ) + .unwrap(); + std::fs::write( + docs_dir.join("Taskfile.yml"), + r#"version: '3' +includes: + api: ./api +tasks: + serve: + desc: Serve docs + cmds: + - echo "Serve""#, + ) + .unwrap(); + std::fs::write( + api_dir.join("Taskfile.yml"), + r#"version: '3' +tasks: + generate: + desc: Generate API docs + cmds: + - echo "Generate""#, + ) + .unwrap(); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert!(matches!( + discovered.definitions.taskfile.unwrap().status, + TaskFileStatus::Parsed + )); + + let root_taskfile = temp_dir.path().join("Taskfile.yml"); + let docs_taskfile = docs_dir.join("Taskfile.yml"); + let api_taskfile = api_dir.join("Taskfile.yml"); + + let build_task = discovered.tasks.iter().find(|t| t.name == "build").unwrap(); + assert_eq!(build_task.runner, TaskRunner::Task); + assert_eq!(build_task.file_path, root_taskfile); + assert_eq!(build_task.definition_path(), root_taskfile.as_path()); + assert_eq!(build_task.source_name, "build"); + + let docs_task = discovered + .tasks + .iter() + .find(|t| t.name == "docs:serve") + .unwrap(); + assert_eq!(docs_task.file_path, root_taskfile); + assert_eq!(docs_task.definition_path(), docs_taskfile.as_path()); + assert_eq!(docs_task.source_name, "docs:serve"); + + let api_task = discovered + .tasks + .iter() + .find(|t| t.name == "docs:api:generate") + .unwrap(); + assert_eq!(api_task.file_path, root_taskfile); + assert_eq!(api_task.definition_path(), api_taskfile.as_path()); + assert_eq!(api_task.source_name, "docs:api:generate"); + + reset_mock(); + reset_to_real_environment(); + } + + #[test] + #[serial] + fn test_discover_tasks_with_duplicate_flattened_taskfile_include() { + let temp_dir = TempDir::new().unwrap(); + let shared_dir = temp_dir.path().join("shared"); + std::fs::create_dir_all(&shared_dir).unwrap(); + + reset_mock(); + enable_mock(); + mock_executable("task"); + + std::fs::write( + temp_dir.path().join("Taskfile.yml"), + r#"version: '3' +includes: + shared: + taskfile: ./shared + flatten: true +tasks: + build: + desc: Root build + cmds: + - echo "Build""#, + ) + .unwrap(); + std::fs::write( + shared_dir.join("Taskfile.yml"), + r#"version: '3' +tasks: + build: + desc: Shared build + cmds: + - echo "Shared build""#, + ) + .unwrap(); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(matches!( + discovered.definitions.taskfile.unwrap().status, + TaskFileStatus::ParseError(_) + )); + assert!( + discovered + .errors + .iter() + .any(|error| error.contains("Found multiple tasks (build) included by \"shared\"")), + "{:?}", + discovered.errors + ); + + reset_mock(); + reset_to_real_environment(); + } + #[test] fn test_discover_maven_tasks() { let temp_dir = tempfile::tempdir().unwrap(); diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index a244e0a..4601c3e 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -110,6 +110,99 @@ else exit 1 fi +# Test 6b: Verify recursive Taskfile include discovery +echo "\nTest 6b: Testing recursive Taskfile include discovery" +mkdir -p /home/testuser/task_include_project/docs/api +cat > /home/testuser/task_include_project/Taskfile.yml <<'EOF' +version: '3' +includes: + docs: ./docs +tasks: + task-root: + desc: Root task + cmds: + - echo "Root" +EOF +cat > /home/testuser/task_include_project/docs/Taskfile.yml <<'EOF' +version: '3' +includes: + api: ./api +tasks: + serve: + desc: Docs serve + cmds: + - echo "Serve docs" +EOF +cat > /home/testuser/task_include_project/docs/api/Taskfile.yml <<'EOF' +version: '3' +tasks: + generate: + desc: Generate docs + cmds: + - echo "Generate docs" +EOF + +cd /home/testuser/task_include_project +dela list > task_include_list.txt 2> task_include_stderr.txt +if [ $? -ne 0 ]; then + echo "${RED}✗ dela list failed for recursive Taskfile includes${NC}" + cat task_include_stderr.txt + exit 1 +fi + +if grep -q "docs:serve" task_include_list.txt && grep -q "docs:api:generate" task_include_list.txt; then + echo "${GREEN}✓ dela list discovers recursively included Taskfile tasks${NC}" +else + echo "${RED}✗ dela list did not discover recursively included Taskfile tasks${NC}" + cat task_include_list.txt + exit 1 +fi + +output=$(dela get-command docs:api:generate 2>&1) +if echo "$output" | grep -q "task docs:api:generate"; then + echo "${GREEN}✓ get-command works for recursively included Taskfile tasks${NC}" +else + echo "${RED}✗ get-command failed for recursively included Taskfile task${NC}" + echo "Got: $output" + exit 1 +fi + +# Test 6c: Verify flattened Taskfile include duplicate detection +echo "\nTest 6c: Testing duplicate flattened Taskfile include detection" +mkdir -p /home/testuser/task_include_duplicates/shared +cat > /home/testuser/task_include_duplicates/Taskfile.yml <<'EOF' +version: '3' +includes: + shared: + taskfile: ./shared + flatten: true +tasks: + build: + desc: Root build + cmds: + - echo "Root build" +EOF +cat > /home/testuser/task_include_duplicates/shared/Taskfile.yml <<'EOF' +version: '3' +tasks: + build: + desc: Shared build + cmds: + - echo "Shared build" +EOF + +cd /home/testuser/task_include_duplicates +duplicate_output=$(dela list 2>&1) +if echo "$duplicate_output" | grep -q 'Found multiple tasks (build) included by "shared"'; then + echo "${GREEN}✓ dela reports duplicate flattened Taskfile task names${NC}" +else + echo "${RED}✗ dela did not report duplicate flattened Taskfile task names${NC}" + echo "$duplicate_output" + exit 1 +fi + +cd /home/testuser/test_project + # Test 7: Basic dela list for Maven tasks echo "\nTest 7: Testing dela list for Maven tasks" if list_and_grep "clean" && list_and_grep "compile" && list_and_grep "profile:dev"; then From c9925c45393235eb877eb8b4d75591018acdf03a Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 15:51:44 -0400 Subject: [PATCH 08/13] ski mis --- src/task_discovery.rs | 58 +++++++++++++++++++++++------- tests/docker_noinit/test_noinit.sh | 10 ++++++ 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/task_discovery.rs b/src/task_discovery.rs index c217384..36644ad 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -597,19 +597,6 @@ fn collect_taskfile_tasks_recursive( let resolved_include = parse_taskfile::resolve_taskfile_include_path(&resolved_candidate); if !resolved_include.is_file() { - if include.optional { - continue; - } - - let error = format!( - "Required included Taskfile '{}' referenced from '{}' was not found", - resolved_include.display(), - current_source.definition_path().display() - ); - include_errors.push(error.clone()); - if first_error.is_none() { - first_error = Some(error); - } continue; } @@ -2108,6 +2095,51 @@ tasks: reset_to_real_environment(); } + #[test] + #[serial] + fn test_discover_tasks_with_missing_required_taskfile_include() { + let temp_dir = TempDir::new().unwrap(); + + reset_mock(); + enable_mock(); + mock_executable("task"); + + std::fs::write( + temp_dir.path().join("Taskfile.yml"), + r#"version: '3' +includes: + docs: ./docs + optional: + taskfile: ./optional + optional: true +tasks: + build: + desc: Build task + cmds: + - echo "Build""#, + ) + .unwrap(); + + let discovered = discover_tasks(temp_dir.path()); + + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert!(matches!( + discovered.definitions.taskfile.unwrap().status, + TaskFileStatus::Parsed + )); + assert_eq!(discovered.tasks.len(), 1); + + let build_task = discovered.tasks.iter().find(|t| t.name == "build").unwrap(); + assert_eq!(build_task.runner, TaskRunner::Task); + assert_eq!( + build_task.definition_path(), + temp_dir.path().join("Taskfile.yml").as_path() + ); + + reset_mock(); + reset_to_real_environment(); + } + #[test] #[serial] fn test_discover_tasks_with_duplicate_flattened_taskfile_include() { diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 4601c3e..a836834 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -117,6 +117,10 @@ cat > /home/testuser/task_include_project/Taskfile.yml <<'EOF' version: '3' includes: docs: ./docs + missing-required: ./missing + missing-optional: + taskfile: ./optional + optional: true tasks: task-root: desc: Root task @@ -158,6 +162,12 @@ else exit 1 fi +if [ -s task_include_stderr.txt ]; then + echo "${RED}✗ dela emitted errors for missing included Taskfiles${NC}" + cat task_include_stderr.txt + exit 1 +fi + output=$(dela get-command docs:api:generate 2>&1) if echo "$output" | grep -q "task docs:api:generate"; then echo "${GREEN}✓ get-command works for recursively included Taskfile tasks${NC}" From a80184a8bc85aec13fcecaff4acfab8c383d2063 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 16:32:57 -0400 Subject: [PATCH 09/13] turbo now recurses --- dev_docs/project_plan.md | 2 +- src/parsers/parse_turbo_json.rs | 178 +++++- src/task_discovery.rs | 542 +++++++++++++++++- tests/docker_noinit/test_noinit.sh | 53 +- .../turbo_repo/apps/web/package.json | 3 +- .../turbo_repo/apps/web/turbo.json | 9 + 6 files changed, 746 insertions(+), 41 deletions(-) create mode 100644 tests/task_definitions/turbo_repo/apps/web/turbo.json diff --git a/dev_docs/project_plan.md b/dev_docs/project_plan.md index dea43ad..8f9ef48 100644 --- a/dev_docs/project_plan.md +++ b/dev_docs/project_plan.md @@ -261,7 +261,7 @@ This plan outlines the major development phases and tasks for building `dela`, a - [x] [DTKT-198] Support Makefile `include`, `-include`, and `sinclude` during task discovery, including recursively loading referenced makefiles and covering the behavior with unit and integration tests. These will be parsed using the lossless parser and the fallback regex parser. - [x] [DTKT-199] Preserve source-file attribution for tasks discovered from included makefiles so `dela list`, allowlists, and MCP metadata point at the defining file, with tests covering duplicates and allowlist decisions from included files. - [x] [DTKT-200] Support Taskfile `includes` so tasks from included Taskfiles are discovered and exposed by dela, with unit and integration coverage for recursive includes and duplicate task names. - - [ ] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. + - [x] [DTKT-201] Support Turborepo workspace-local `turbo.json` configs that extend the root config so inherited package tasks are discovered, with unit and integration coverage for recursive config resolution. - [ ] **Additional Task Runners support** - [x] [DTKT-119] Implement Maven `pom.xml` parser and task discovery diff --git a/src/parsers/parse_turbo_json.rs b/src/parsers/parse_turbo_json.rs index a0cc8da..85696c8 100644 --- a/src/parsers/parse_turbo_json.rs +++ b/src/parsers/parse_turbo_json.rs @@ -1,35 +1,102 @@ use crate::types::{Task, TaskDefinitionType, TaskRunner}; -use std::path::PathBuf; +use serde_json::Value; +use std::collections::BTreeMap; +use std::path::Path; -pub fn parse(path: &PathBuf) -> Result, String> { +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TurboTaskConfig { + pub inherits: bool, + pub has_local_configuration: bool, +} + +impl TurboTaskConfig { + pub fn is_effective_task_definition(&self) -> bool { + self.inherits || self.has_local_configuration + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TurboConfig { + pub extends: Vec, + pub tasks: BTreeMap, +} + +impl TurboConfig { + pub fn task_names(&self) -> impl Iterator { + self.tasks + .iter() + .filter(|(_, task)| task.is_effective_task_definition()) + .map(|(name, _)| name) + } +} + +pub fn load_config(path: &Path) -> Result { let contents = std::fs::read_to_string(path).map_err(|e| format!("Failed to read turbo.json: {}", e))?; - let json: serde_json::Value = serde_json::from_str(&contents) + let json: Value = serde_json::from_str(&contents) .map_err(|e| format!("Failed to parse turbo.json: {}", e))?; + let extends = json + .get("extends") + .and_then(Value::as_array) + .map(|entries| { + entries + .iter() + .filter_map(Value::as_str) + .map(str::to_string) + .collect() + }) + .unwrap_or_default(); + let tasks = json .get("tasks") .or_else(|| json.get("pipeline")) - .and_then(|value| value.as_object()) + .and_then(Value::as_object) .map(|task_map| { task_map - .keys() - .map(|name| Task { - name: name.clone(), - file_path: path.clone(), - definition_path: None, - definition_type: TaskDefinitionType::TurboJson, - runner: TaskRunner::Turbo, - source_name: name.clone(), - description: None, - shadowed_by: None, - disambiguated_name: None, - }) + .iter() + .map(|(name, value)| (name.clone(), parse_task_config(value))) .collect() }) .unwrap_or_default(); - Ok(tasks) + Ok(TurboConfig { extends, tasks }) +} + +pub fn parse(path: &Path) -> Result, String> { + let config = load_config(path)?; + + Ok(config + .task_names() + .map(|name| Task { + name: name.clone(), + file_path: path.to_path_buf(), + definition_path: None, + definition_type: TaskDefinitionType::TurboJson, + runner: TaskRunner::Turbo, + source_name: name.clone(), + description: None, + shadowed_by: None, + disambiguated_name: None, + }) + .collect()) +} + +fn parse_task_config(value: &Value) -> TurboTaskConfig { + let Some(object) = value.as_object() else { + return TurboTaskConfig { + inherits: true, + has_local_configuration: true, + }; + }; + + TurboTaskConfig { + inherits: object + .get("extends") + .and_then(Value::as_bool) + .unwrap_or(true), + has_local_configuration: object.keys().any(|key| key != "extends"), + } } #[cfg(test)] @@ -69,6 +136,83 @@ mod tests { ); } + #[test] + fn test_load_turbo_json_tracks_extends_and_task_level_extends_false() { + let temp_dir = TempDir::new().unwrap(); + let turbo_json_path = temp_dir.path().join("turbo.json"); + std::fs::write( + &turbo_json_path, + r#"{ + "extends": ["//", "shared-config"], + "tasks": { + "build": {}, + "test": { + "extends": false + }, + "lint": { + "extends": false, + "outputs": [] + } + } +}"#, + ) + .unwrap(); + + let config = load_config(&turbo_json_path).unwrap(); + + assert_eq!(config.extends, vec!["//", "shared-config"]); + assert_eq!( + config.tasks.get("build"), + Some(&TurboTaskConfig { + inherits: true, + has_local_configuration: false, + }) + ); + assert_eq!( + config.tasks.get("test"), + Some(&TurboTaskConfig { + inherits: false, + has_local_configuration: false, + }) + ); + assert_eq!( + config.tasks.get("lint"), + Some(&TurboTaskConfig { + inherits: false, + has_local_configuration: true, + }) + ); + } + + #[test] + fn test_parse_turbo_json_excludes_task_with_only_extends_false() { + let temp_dir = TempDir::new().unwrap(); + let turbo_json_path = temp_dir.path().join("turbo.json"); + std::fs::write( + &turbo_json_path, + r#"{ + "extends": ["//"], + "tasks": { + "build": {}, + "test": { + "extends": false + }, + "lint": { + "extends": false, + "dependsOn": [] + } + } +}"#, + ) + .unwrap(); + + let tasks = parse(&turbo_json_path).unwrap(); + let task_names: Vec<_> = tasks.iter().map(|task| task.name.as_str()).collect(); + + assert_eq!(task_names, vec!["build", "lint"]); + assert!(!task_names.contains(&"test")); + } + #[test] fn test_parse_turbo_json_legacy_pipeline() { let temp_dir = TempDir::new().unwrap(); diff --git a/src/task_discovery.rs b/src/task_discovery.rs index 36644ad..71c91b8 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -7,7 +7,7 @@ use crate::parsers::{ use crate::repo_root::find_git_repo_root; use crate::task_shadowing::check_shadowing; use crate::types::{Task, TaskDefinitionFile, TaskDefinitionType, TaskFileStatus, TaskRunner}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fs; use std::path::Path; use std::path::PathBuf; @@ -663,16 +663,335 @@ fn discover_turbo_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result< return Ok(()); } - match parse_turbo_json::parse(&turbo_json) { - Ok(tasks) => { - handle_discovery_success(tasks, turbo_json, TaskDefinitionType::TurboJson, discovered); + let mut tasks_by_name = BTreeMap::new(); + let mut config_errors = Vec::new(); + + let result = collect_turbo_tasks_for_context( + &repo_root, + dir, + &turbo_json, + &mut tasks_by_name, + &mut config_errors, + ); + + let mut tasks: Vec<_> = tasks_by_name.into_values().collect(); + for task in &mut tasks { + task.shadowed_by = check_shadowing(&task.name); + } + + discovered.tasks.extend(tasks); + discovered.errors.extend(config_errors); + + let status = match result { + Ok(()) => TaskFileStatus::Parsed, + Err(error) => { + discovered.errors.push(format!( + "Failed to parse {}: {}", + turbo_json.display(), + error + )); + TaskFileStatus::ParseError(error) } - Err(e) => { - handle_discovery_error(e, turbo_json, TaskDefinitionType::TurboJson, discovered); + }; + discovered.definitions.turbo_json = Some(TaskDefinitionFile { + path: turbo_json, + definition_type: TaskDefinitionType::TurboJson, + status, + }); + + Ok(()) +} + +fn collect_turbo_tasks_for_context( + repo_root: &Path, + dir: &Path, + root_turbo_json: &Path, + collected_tasks: &mut BTreeMap, + config_errors: &mut Vec, +) -> Result<(), String> { + let root_source = ComposedDefinitionSource::direct(root_turbo_json.to_path_buf()); + let mut package_configs_by_name = None; + let root_tasks = resolve_effective_turbo_tasks( + &root_source, + repo_root, + root_turbo_json, + &mut package_configs_by_name, + &mut RecursiveDiscoveryState::new(), + )?; + collected_tasks.extend(root_tasks); + + let mut first_error = None; + + if dir == repo_root { + for config_path in collect_descendant_turbo_config_paths(repo_root) { + let config_source = + ComposedDefinitionSource::composed(root_turbo_json, config_path.clone()); + match resolve_effective_turbo_tasks( + &config_source, + repo_root, + root_turbo_json, + &mut package_configs_by_name, + &mut RecursiveDiscoveryState::new(), + ) { + Ok(tasks) => { + for (name, task) in tasks { + collected_tasks.entry(name).or_insert(task); + } + } + Err(error) => { + let error = format!( + "Failed to parse workspace-local turbo config '{}': {}", + config_path.display(), + error + ); + config_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); + } + } + } + } + } else { + for config_path in collect_turbo_ancestor_config_paths(dir, repo_root) { + let config_source = + ComposedDefinitionSource::composed(root_turbo_json, config_path.clone()); + match resolve_effective_turbo_tasks( + &config_source, + repo_root, + root_turbo_json, + &mut package_configs_by_name, + &mut RecursiveDiscoveryState::new(), + ) { + Ok(tasks) if !tasks.is_empty() => { + *collected_tasks = tasks; + break; + } + Ok(_) => {} + Err(error) => { + let error = format!( + "Failed to parse workspace-local turbo config '{}': {}", + config_path.display(), + error + ); + config_errors.push(error.clone()); + if first_error.is_none() { + first_error = Some(error); + } + break; + } + } } } - Ok(()) + if let Some(error) = first_error { + Err(error) + } else { + Ok(()) + } +} + +fn resolve_effective_turbo_tasks( + current_source: &ComposedDefinitionSource, + repo_root: &Path, + root_turbo_json: &Path, + package_configs_by_name: &mut Option>, + traversal_state: &mut RecursiveDiscoveryState, +) -> Result, String> { + match traversal_state.mark_visited(current_source.definition_path()) { + VisitState::AlreadyVisited(_) => return Ok(BTreeMap::new()), + VisitState::New(_) => {} + } + + let config = parse_turbo_json::load_config(current_source.definition_path())?; + + if current_source.definition_path() != root_turbo_json && config.extends.is_empty() { + return Ok(BTreeMap::new()); + } + + let mut tasks = BTreeMap::new(); + + if current_source.definition_path() != root_turbo_json { + for extend_entry in &config.extends { + let Some(parent_config_path) = resolve_turbo_extends_entry( + current_source, + extend_entry, + repo_root, + root_turbo_json, + package_configs_by_name, + ) else { + continue; + }; + + if !parent_config_path.is_file() { + continue; + } + + let parent_source = + ComposedDefinitionSource::composed(root_turbo_json, parent_config_path.clone()); + let inherited_tasks = resolve_effective_turbo_tasks( + &parent_source, + repo_root, + root_turbo_json, + package_configs_by_name, + traversal_state, + )?; + tasks.extend(inherited_tasks); + } + } + + for (name, task_config) in &config.tasks { + if !task_config.is_effective_task_definition() { + tasks.remove(name.as_str()); + } + } + + let mut local_tasks = parse_turbo_json::parse(current_source.definition_path())?; + for task in &mut local_tasks { + current_source.apply_to_task(task); + } + for task in local_tasks { + tasks.insert(task.name.clone(), task); + } + + Ok(tasks) +} + +fn resolve_turbo_extends_entry( + current_source: &ComposedDefinitionSource, + extend_entry: &str, + repo_root: &Path, + root_turbo_json: &Path, + package_configs_by_name: &mut Option>, +) -> Option { + if extend_entry == "//" { + return Some(root_turbo_json.to_path_buf()); + } + + if looks_like_turbo_config_path(extend_entry) { + let candidate = current_source.resolve_child(extend_entry); + return Some(resolve_turbo_config_path_candidate(&candidate)); + } + + let package_configs_by_name = + package_configs_by_name.get_or_insert_with(|| build_turbo_package_config_index(repo_root)); + package_configs_by_name.get(extend_entry).cloned() +} + +fn collect_turbo_ancestor_config_paths(dir: &Path, repo_root: &Path) -> Vec { + let mut current = dir.to_path_buf(); + let mut config_paths = Vec::new(); + + while current.starts_with(repo_root) && current != repo_root { + let candidate = current.join("turbo.json"); + if candidate.is_file() { + config_paths.push(candidate); + } + + if !current.pop() { + break; + } + } + + config_paths +} + +fn collect_descendant_turbo_config_paths(repo_root: &Path) -> Vec { + let mut config_paths = Vec::new(); + collect_descendant_turbo_config_paths_recursive(repo_root, repo_root, &mut config_paths); + config_paths.sort(); + config_paths +} + +fn collect_descendant_turbo_config_paths_recursive( + repo_root: &Path, + current_dir: &Path, + config_paths: &mut Vec, +) { + let Ok(entries) = fs::read_dir(current_dir) else { + return; + }; + + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + + let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else { + continue; + }; + if should_skip_turbo_config_scan(file_name) { + continue; + } + + let candidate = path.join("turbo.json"); + if candidate.is_file() && candidate != repo_root.join("turbo.json") { + config_paths.push(candidate); + } + + collect_descendant_turbo_config_paths_recursive(repo_root, &path, config_paths); + } +} + +fn should_skip_turbo_config_scan(file_name: &str) -> bool { + matches!(file_name, ".git" | "node_modules") +} + +fn looks_like_turbo_config_path(extend_entry: &str) -> bool { + let extend_path = Path::new(extend_entry); + extend_path.is_absolute() + || extend_entry.starts_with('.') + || extend_entry.contains(std::path::MAIN_SEPARATOR) + || extend_entry.contains('/') + || extend_entry.contains('\\') +} + +fn resolve_turbo_config_path_candidate(candidate: &Path) -> PathBuf { + if candidate + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name == "turbo.json") + { + return candidate.to_path_buf(); + } + + if candidate.extension().is_none() || candidate.is_dir() { + return candidate.join("turbo.json"); + } + + candidate.to_path_buf() +} + +fn build_turbo_package_config_index(repo_root: &Path) -> HashMap { + let mut package_configs = HashMap::new(); + + let root_turbo_json = repo_root.join("turbo.json"); + if root_turbo_json.is_file() + && let Some(package_name) = read_package_name(repo_root) + { + package_configs.insert(package_name, root_turbo_json); + } + + for config_path in collect_descendant_turbo_config_paths(repo_root) { + let Some(config_dir) = config_path.parent() else { + continue; + }; + let Some(package_name) = read_package_name(config_dir) else { + continue; + }; + package_configs.entry(package_name).or_insert(config_path); + } + + package_configs +} + +fn read_package_name(dir: &Path) -> Option { + let package_json_path = dir.join("package.json"); + let contents = fs::read_to_string(package_json_path).ok()?; + let json: serde_json::Value = serde_json::from_str(&contents).ok()?; + json.get("name") + .and_then(serde_json::Value::as_str) + .map(str::to_string) } fn discover_maven_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Result<(), String> { @@ -1148,6 +1467,26 @@ mod tests { writeln!(file, "{}", content).unwrap(); } + fn create_test_turbo_json(dir: &Path, content: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(dir.join("turbo.json"), content).unwrap(); + } + + fn create_test_package_json(dir: &Path, name: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("package.json"), + format!( + r#"{{ + "name": "{}", + "private": true +}}"#, + name + ), + ) + .unwrap(); + } + #[test] fn test_discover_tasks_empty_directory() { let temp_dir = TempDir::new().unwrap(); @@ -1423,8 +1762,8 @@ build: std::fs::create_dir_all(temp_dir.path().join(".git")).unwrap(); std::fs::create_dir_all(temp_dir.path().join("apps").join("web")).unwrap(); - std::fs::write( - temp_dir.path().join("turbo.json"), + create_test_turbo_json( + temp_dir.path(), r#"{ "$schema": "https://turborepo.dev/schema.json", "tasks": { @@ -1434,18 +1773,25 @@ build: } } }"#, - ) - .unwrap(); + ); let discovered = discover_tasks(&temp_dir.path().join("apps").join("web")); let build_task = discovered.tasks.iter().find(|t| t.name == "build").unwrap(); assert_eq!(build_task.runner, TaskRunner::Turbo); assert_eq!(build_task.file_path, temp_dir.path().join("turbo.json")); + assert_eq!( + build_task.definition_path(), + temp_dir.path().join("turbo.json").as_path() + ); let test_task = discovered.tasks.iter().find(|t| t.name == "test").unwrap(); assert_eq!(test_task.runner, TaskRunner::Turbo); assert_eq!(test_task.file_path, temp_dir.path().join("turbo.json")); + assert_eq!( + test_task.definition_path(), + temp_dir.path().join("turbo.json").as_path() + ); assert_eq!( discovered.definitions.turbo_json.unwrap().status, @@ -1453,6 +1799,180 @@ build: ); } + #[test] + fn test_discover_tasks_includes_workspace_local_turbo_tasks_from_repo_root() { + let temp_dir = TempDir::new().unwrap(); + let repo_root = temp_dir.path(); + let web_dir = repo_root.join("apps").join("web"); + std::fs::create_dir_all(repo_root.join(".git")).unwrap(); + + create_test_turbo_json( + repo_root, + r#"{ + "tasks": { + "build": {}, + "test": {} + } +}"#, + ); + create_test_package_json(&web_dir, "web"); + create_test_turbo_json( + &web_dir, + r#"{ + "extends": ["//"], + "tasks": { + "lint": {}, + "test": { + "extends": false + } + } +}"#, + ); + + let discovered = discover_tasks(repo_root); + + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert!(discovered.tasks.iter().any(|task| task.name == "build")); + assert!(discovered.tasks.iter().any(|task| task.name == "test")); + + let lint_task = discovered + .tasks + .iter() + .find(|task| task.name == "lint") + .unwrap(); + assert_eq!(lint_task.runner, TaskRunner::Turbo); + assert_eq!(lint_task.file_path, repo_root.join("turbo.json")); + assert_eq!( + lint_task.definition_path(), + web_dir.join("turbo.json").as_path() + ); + } + + #[test] + fn test_discover_tasks_resolves_workspace_local_turbo_tasks_for_nested_package() { + let temp_dir = TempDir::new().unwrap(); + let repo_root = temp_dir.path(); + let web_dir = repo_root.join("apps").join("web"); + std::fs::create_dir_all(repo_root.join(".git")).unwrap(); + + create_test_turbo_json( + repo_root, + r#"{ + "tasks": { + "build": {}, + "test": {} + } +}"#, + ); + create_test_package_json(&web_dir, "web"); + create_test_turbo_json( + &web_dir, + r#"{ + "extends": ["//"], + "tasks": { + "lint": {}, + "test": { + "extends": false + } + } +}"#, + ); + + let discovered = discover_tasks(&web_dir); + let task_names: Vec<_> = discovered + .tasks + .iter() + .map(|task| task.name.as_str()) + .collect(); + + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert_eq!(task_names, vec!["build", "lint"]); + + let build_task = discovered + .tasks + .iter() + .find(|task| task.name == "build") + .unwrap(); + assert_eq!( + build_task.definition_path(), + repo_root.join("turbo.json").as_path() + ); + + let lint_task = discovered + .tasks + .iter() + .find(|task| task.name == "lint") + .unwrap(); + assert_eq!(lint_task.file_path, repo_root.join("turbo.json")); + assert_eq!( + lint_task.definition_path(), + web_dir.join("turbo.json").as_path() + ); + } + + #[test] + fn test_discover_tasks_recursively_resolves_turbo_extends_chain() { + let temp_dir = TempDir::new().unwrap(); + let repo_root = temp_dir.path(); + let shared_dir = repo_root.join("packages").join("shared-config"); + let web_dir = repo_root.join("apps").join("web"); + std::fs::create_dir_all(repo_root.join(".git")).unwrap(); + + create_test_turbo_json( + repo_root, + r#"{ + "tasks": { + "build": {} + } +}"#, + ); + create_test_package_json(&shared_dir, "shared-config"); + create_test_turbo_json( + &shared_dir, + r#"{ + "extends": ["//"], + "tasks": { + "lint": {} + } +}"#, + ); + create_test_package_json(&web_dir, "web"); + create_test_turbo_json( + &web_dir, + r#"{ + "extends": ["//", "shared-config"], + "tasks": { + "deploy": {} + } +}"#, + ); + + let discovered = discover_tasks(&web_dir); + + assert!(discovered.errors.is_empty(), "{:?}", discovered.errors); + assert!(discovered.tasks.iter().any(|task| task.name == "build")); + + let lint_task = discovered + .tasks + .iter() + .find(|task| task.name == "lint") + .unwrap(); + assert_eq!( + lint_task.definition_path(), + shared_dir.join("turbo.json").as_path() + ); + + let deploy_task = discovered + .tasks + .iter() + .find(|task| task.name == "deploy") + .unwrap(); + assert_eq!( + deploy_task.definition_path(), + web_dir.join("turbo.json").as_path() + ); + } + #[test] fn test_discover_tasks_with_invalid_makefile() { let temp_dir = TempDir::new().unwrap(); diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index a836834..6a3dd83 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -444,26 +444,57 @@ fi echo "${GREEN}✓ All get-command argument passing tests passed successfully${NC}" -# Test 21b: Discover turbo tasks from a nested package using the git repo root -echo "\nTest 21b: Testing Turborepo discovery from a nested package" +# Test 21b: Discover turbo tasks from workspace-local configs +echo "\nTest 21b: Testing Turborepo workspace-local config discovery" cd /home/testuser/turbo_repo git init >/dev/null 2>&1 + +dela list > turbo_root_list_output.txt 2> turbo_root_stderr.txt +if grep -q "web-lint" turbo_root_list_output.txt && grep -q "apps/web/turbo.json" turbo_root_list_output.txt; then + echo "${GREEN}✓ dela list shows workspace-local turbo tasks and source paths from the repo root${NC}" +else + echo "${RED}✗ dela list failed to show workspace-local turbo tasks from the repo root${NC}" + cat turbo_root_list_output.txt + exit 1 +fi + +if [ -s turbo_root_stderr.txt ]; then + echo "${RED}✗ dela emitted errors while listing turbo tasks from the repo root${NC}" + cat turbo_root_stderr.txt + exit 1 +fi + +echo "2" | dela allow-command web-lint >/dev/null 2>&1 +if grep -q "/home/testuser/turbo_repo/apps/web/turbo.json" /home/testuser/.config/dela/allowlist.toml; then + echo "${GREEN}✓ allow-command stores the workspace-local turbo.json path${NC}" +else + echo "${RED}✗ allow-command did not store the workspace-local turbo.json path${NC}" + cat /home/testuser/.config/dela/allowlist.toml + exit 1 +fi + cd /home/testuser/turbo_repo/apps/web -dela list 2>/dev/null > turbo_list_output.txt -if grep -q "build-t" turbo_list_output.txt && grep -q "test-t" turbo_list_output.txt; then - echo "${GREEN}✓ dela list shows turbo tasks from repo root when run in a nested package${NC}" +dela list > turbo_package_list_output.txt 2> turbo_package_stderr.txt +if grep -q "build-t" turbo_package_list_output.txt && grep -q "web-lint-t" turbo_package_list_output.txt && ! grep -q "test-t" turbo_package_list_output.txt; then + echo "${GREEN}✓ dela list resolves effective turbo tasks for a nested package${NC}" else - echo "${RED}✗ dela list failed to show turbo tasks from repo root${NC}" - cat turbo_list_output.txt + echo "${RED}✗ dela list did not resolve the expected nested-package turbo tasks${NC}" + cat turbo_package_list_output.txt + exit 1 +fi + +if [ -s turbo_package_stderr.txt ]; then + echo "${RED}✗ dela emitted errors while listing turbo tasks from a nested package${NC}" + cat turbo_package_stderr.txt exit 1 fi -output=$(dela get-command build-t 2>&1) -if echo "$output" | grep -q "turbo run build"; then - echo "${GREEN}✓ get-command returns the turbo command for a nested package task${NC}" +output=$(dela get-command web-lint-t 2>&1) +if echo "$output" | grep -q "turbo run web-lint"; then + echo "${GREEN}✓ get-command returns the turbo command for a workspace-local task${NC}" else - echo "${RED}✗ get-command failed for turbo task${NC}" + echo "${RED}✗ get-command failed for the workspace-local turbo task${NC}" echo "Full output: $output" exit 1 fi diff --git a/tests/task_definitions/turbo_repo/apps/web/package.json b/tests/task_definitions/turbo_repo/apps/web/package.json index 598a6e6..2332114 100644 --- a/tests/task_definitions/turbo_repo/apps/web/package.json +++ b/tests/task_definitions/turbo_repo/apps/web/package.json @@ -3,6 +3,7 @@ "private": true, "scripts": { "build": "echo building web", - "test": "echo testing web" + "test": "echo testing web", + "web-lint": "echo linting web" } } diff --git a/tests/task_definitions/turbo_repo/apps/web/turbo.json b/tests/task_definitions/turbo_repo/apps/web/turbo.json new file mode 100644 index 0000000..47b3967 --- /dev/null +++ b/tests/task_definitions/turbo_repo/apps/web/turbo.json @@ -0,0 +1,9 @@ +{ + "extends": ["//"], + "tasks": { + "web-lint": {}, + "test": { + "extends": false + } + } +} From 7c657cc3833845b24e6c8683f4be7d58599c2677 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 17:18:25 -0400 Subject: [PATCH 10/13] fixes --- src/task_discovery.rs | 52 +++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/src/task_discovery.rs b/src/task_discovery.rs index 71c91b8..c275fa9 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -494,18 +494,21 @@ fn discover_taskfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Resu let mut tasks = Vec::new(); let mut include_errors = Vec::new(); let no_excludes = std::collections::HashSet::new(); + let mut traversal = TaskfileTraversal { + root_taskfile_path: &taskfile_path, + traversal_state: &mut traversal_state, + seen_task_names: &mut seen_task_names, + collected_tasks: &mut tasks, + include_errors: &mut include_errors, + }; let result = collect_taskfile_tasks_recursive( - &taskfile_path, &root_source, "", None, false, &no_excludes, - &mut traversal_state, - &mut seen_task_names, - &mut tasks, - &mut include_errors, + &mut traversal, ); for task in &mut tasks { @@ -535,19 +538,26 @@ fn discover_taskfile_tasks(dir: &Path, discovered: &mut DiscoveredTasks) -> Resu Ok(()) } +struct TaskfileTraversal<'a> { + root_taskfile_path: &'a Path, + traversal_state: &'a mut RecursiveDiscoveryState, + seen_task_names: &'a mut std::collections::HashSet, + collected_tasks: &'a mut Vec, + include_errors: &'a mut Vec, +} + fn collect_taskfile_tasks_recursive( - root_taskfile_path: &Path, current_source: &ComposedDefinitionSource, namespace_prefix: &str, include_label: Option<&str>, hide_tasks: bool, excluded_tasks: &std::collections::HashSet, - traversal_state: &mut RecursiveDiscoveryState, - seen_task_names: &mut std::collections::HashSet, - collected_tasks: &mut Vec, - include_errors: &mut Vec, + traversal: &mut TaskfileTraversal<'_>, ) -> Result<(), String> { - match traversal_state.mark_visited(current_source.definition_path()) { + match traversal + .traversal_state + .mark_visited(current_source.definition_path()) + { VisitState::AlreadyVisited(_) => return Ok(()), VisitState::New(_) => {} } @@ -569,7 +579,7 @@ fn collect_taskfile_tasks_recursive( task.source_name = effective_name; current_source.apply_to_task(&mut task); - if !seen_task_names.insert(task.name.clone()) { + if !traversal.seen_task_names.insert(task.name.clone()) { let error = match include_label { Some(include_label) => { format!( @@ -579,14 +589,14 @@ fn collect_taskfile_tasks_recursive( } None => format!("Found multiple Taskfile tasks named '{}'", task.name), }; - include_errors.push(error.clone()); + traversal.include_errors.push(error.clone()); if first_error.is_none() { first_error = Some(error); } continue; } - collected_tasks.push(task); + traversal.collected_tasks.push(task); } } @@ -600,8 +610,10 @@ fn collect_taskfile_tasks_recursive( continue; } - let child_source = - ComposedDefinitionSource::composed(root_taskfile_path, resolved_include.clone()); + let child_source = ComposedDefinitionSource::composed( + traversal.root_taskfile_path, + resolved_include.clone(), + ); let child_namespace = if include.flatten { namespace_prefix.to_string() } else { @@ -612,23 +624,19 @@ fn collect_taskfile_tasks_recursive( let child_excludes = include.excludes.into_iter().collect(); if let Err(error) = collect_taskfile_tasks_recursive( - root_taskfile_path, &child_source, &child_namespace, Some(child_include_label.as_str()), child_hide_tasks, &child_excludes, - traversal_state, - seen_task_names, - collected_tasks, - include_errors, + traversal, ) { let error = format!( "Failed to parse included Taskfile '{}': {}", resolved_include.display(), error ); - include_errors.push(error.clone()); + traversal.include_errors.push(error.clone()); if first_error.is_none() { first_error = Some(error); } From 25fdd7a8a810a386f3e42e42ea162ca1d62684d6 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Mon, 4 May 2026 19:37:59 -0400 Subject: [PATCH 11/13] fixes --- src/composed_paths.rs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/composed_paths.rs b/src/composed_paths.rs index 58abe27..b7d90dd 100644 --- a/src/composed_paths.rs +++ b/src/composed_paths.rs @@ -6,13 +6,11 @@ use std::path::{Component, Path, PathBuf}; /// Shared path metadata for tasks discovered through composed definitions such /// as includes or inherited configs. #[derive(Debug, Clone, PartialEq, Eq)] -#[allow(dead_code)] // DTKT-198/200/201 will consume the full recursive source API. pub struct ComposedDefinitionSource { runner_path: PathBuf, definition_path: PathBuf, } -#[allow(dead_code)] // DTKT-198/200/201 will consume the full recursive source API. impl ComposedDefinitionSource { /// Create a source where the runner path and defining file are the same. pub fn direct(path: impl Into) -> Self { @@ -31,10 +29,6 @@ impl ComposedDefinitionSource { } } - pub fn runner_path(&self) -> &Path { - &self.runner_path - } - pub fn definition_path(&self) -> &Path { &self.definition_path } @@ -58,19 +52,16 @@ impl ComposedDefinitionSource { /// Track visited definitions for recursive discovery so include cycles can be /// handled consistently across runners. #[derive(Debug, Clone, Default)] -#[allow(dead_code)] // DTKT-198/200/201 will use recursive traversal state directly. pub struct RecursiveDiscoveryState { visited: HashSet, } #[derive(Debug, Clone, PartialEq, Eq)] -#[allow(dead_code)] // DTKT-198/200/201 will use visit outcomes for include traversal. pub enum VisitState { New(PathBuf), AlreadyVisited(PathBuf), } -#[allow(dead_code)] // DTKT-198/200/201 will use recursive traversal state directly. impl RecursiveDiscoveryState { pub fn new() -> Self { Self::default() @@ -87,7 +78,6 @@ impl RecursiveDiscoveryState { } /// Resolve a referenced definition path relative to the file that included it. -#[allow(dead_code)] // DTKT-198/200/201 will reuse this for include resolution. pub fn resolve_nested_definition_path( current_definition_path: &Path, referenced_path: &Path, @@ -166,7 +156,7 @@ mod tests { fn test_direct_source_uses_same_path_for_runner_and_definition() { let source = ComposedDefinitionSource::direct("/repo/Makefile"); - assert_eq!(source.runner_path(), Path::new("/repo/Makefile")); + assert_eq!(source.runner_path, PathBuf::from("/repo/Makefile")); assert_eq!(source.definition_path(), Path::new("/repo/Makefile")); } From 70db513c2e8624d5b9a22df29774efdfb2f460db Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Tue, 5 May 2026 22:07:40 -0400 Subject: [PATCH 12/13] fixes --- src/parsers/parse_turbo_json.rs | 42 ++++++++++++++++++++++++++++-- tests/docker_noinit/test_noinit.sh | 6 ++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/parsers/parse_turbo_json.rs b/src/parsers/parse_turbo_json.rs index 85696c8..a401832 100644 --- a/src/parsers/parse_turbo_json.rs +++ b/src/parsers/parse_turbo_json.rs @@ -6,12 +6,13 @@ use std::path::Path; #[derive(Debug, Clone, PartialEq, Eq)] pub struct TurboTaskConfig { pub inherits: bool, + pub declared_locally: bool, pub has_local_configuration: bool, } impl TurboTaskConfig { pub fn is_effective_task_definition(&self) -> bool { - self.inherits || self.has_local_configuration + self.declared_locally && (self.inherits || self.has_local_configuration) } } @@ -86,7 +87,8 @@ fn parse_task_config(value: &Value) -> TurboTaskConfig { let Some(object) = value.as_object() else { return TurboTaskConfig { inherits: true, - has_local_configuration: true, + declared_locally: false, + has_local_configuration: false, }; }; @@ -95,6 +97,7 @@ fn parse_task_config(value: &Value) -> TurboTaskConfig { .get("extends") .and_then(Value::as_bool) .unwrap_or(true), + declared_locally: true, has_local_configuration: object.keys().any(|key| key != "extends"), } } @@ -165,6 +168,7 @@ mod tests { config.tasks.get("build"), Some(&TurboTaskConfig { inherits: true, + declared_locally: true, has_local_configuration: false, }) ); @@ -172,6 +176,7 @@ mod tests { config.tasks.get("test"), Some(&TurboTaskConfig { inherits: false, + declared_locally: true, has_local_configuration: false, }) ); @@ -179,6 +184,7 @@ mod tests { config.tasks.get("lint"), Some(&TurboTaskConfig { inherits: false, + declared_locally: true, has_local_configuration: true, }) ); @@ -213,6 +219,38 @@ mod tests { assert!(!task_names.contains(&"test")); } + #[test] + fn test_parse_turbo_json_treats_non_object_task_as_inherit_only() { + let temp_dir = TempDir::new().unwrap(); + let turbo_json_path = temp_dir.path().join("turbo.json"); + std::fs::write( + &turbo_json_path, + r#"{ + "tasks": { + "build": null, + "test": {} + } +}"#, + ) + .unwrap(); + + let config = load_config(&turbo_json_path).unwrap(); + assert_eq!( + config.tasks.get("build"), + Some(&TurboTaskConfig { + inherits: true, + declared_locally: false, + has_local_configuration: false, + }) + ); + + let tasks = parse(&turbo_json_path).unwrap(); + let task_names: Vec<_> = tasks.iter().map(|task| task.name.as_str()).collect(); + + assert_eq!(task_names, vec!["test"]); + assert!(!task_names.contains(&"build")); + } + #[test] fn test_parse_turbo_json_legacy_pipeline() { let temp_dir = TempDir::new().unwrap(); diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 6a3dd83..45b29df 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -147,8 +147,7 @@ tasks: EOF cd /home/testuser/task_include_project -dela list > task_include_list.txt 2> task_include_stderr.txt -if [ $? -ne 0 ]; then +if ! dela list > task_include_list.txt 2> task_include_stderr.txt; then echo "${RED}✗ dela list failed for recursive Taskfile includes${NC}" cat task_include_stderr.txt exit 1 @@ -559,8 +558,7 @@ nested-task: EOF cd /home/testuser/make_include_project -dela list > make_include_list.txt 2> make_include_stderr.txt -if [ $? -ne 0 ]; then +if ! dela list > make_include_list.txt 2> make_include_stderr.txt; then echo "${RED}✗ dela list failed when a required included makefile was missing${NC}" cat make_include_stderr.txt exit 1 From a9c4bdb0e049f383ba51c9837ab6d32fe51ed761 Mon Sep 17 00:00:00 2001 From: Alex Yankov Date: Tue, 5 May 2026 23:22:36 -0400 Subject: [PATCH 13/13] more fixes --- src/parsers/parse_turbo_json.rs | 68 +++++++++++++++++++++++++----- src/task_discovery.rs | 8 +++- tests/docker_noinit/test_noinit.sh | 13 +++++- 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/src/parsers/parse_turbo_json.rs b/src/parsers/parse_turbo_json.rs index a401832..c9469c7 100644 --- a/src/parsers/parse_turbo_json.rs +++ b/src/parsers/parse_turbo_json.rs @@ -49,17 +49,13 @@ pub fn load_config(path: &Path) -> Result { }) .unwrap_or_default(); - let tasks = json - .get("tasks") - .or_else(|| json.get("pipeline")) - .and_then(Value::as_object) - .map(|task_map| { - task_map - .iter() - .map(|(name, value)| (name.clone(), parse_task_config(value))) - .collect() - }) - .unwrap_or_default(); + let tasks = if let Some(value) = json.get("tasks") { + parse_task_map("tasks", value)? + } else if let Some(value) = json.get("pipeline") { + parse_task_map("pipeline", value)? + } else { + BTreeMap::new() + }; Ok(TurboConfig { extends, tasks }) } @@ -102,6 +98,32 @@ fn parse_task_config(value: &Value) -> TurboTaskConfig { } } +fn parse_task_map(key: &str, value: &Value) -> Result, String> { + let Some(task_map) = value.as_object() else { + return Err(format!( + "Failed to parse turbo.json: '{}' must be an object, found {}", + key, + json_type_name(value) + )); + }; + + Ok(task_map + .iter() + .map(|(name, value)| (name.clone(), parse_task_config(value))) + .collect()) +} + +fn json_type_name(value: &Value) -> &'static str { + match value { + Value::Null => "null", + Value::Bool(_) => "boolean", + Value::Number(_) => "number", + Value::String(_) => "string", + Value::Array(_) => "array", + Value::Object(_) => "object", + } +} + #[cfg(test)] mod tests { use super::*; @@ -272,6 +294,30 @@ mod tests { assert!(tasks.iter().any(|task| task.name == "check-types")); } + #[test] + fn test_parse_turbo_json_errors_when_tasks_is_not_an_object() { + let temp_dir = TempDir::new().unwrap(); + let turbo_json_path = temp_dir.path().join("turbo.json"); + std::fs::write(&turbo_json_path, r#"{"tasks":["build"]}"#).unwrap(); + + let err = parse(&turbo_json_path).unwrap_err(); + + assert!(err.contains("'tasks' must be an object")); + assert!(err.contains("array")); + } + + #[test] + fn test_parse_turbo_json_errors_when_pipeline_is_not_an_object() { + let temp_dir = TempDir::new().unwrap(); + let turbo_json_path = temp_dir.path().join("turbo.json"); + std::fs::write(&turbo_json_path, r#"{"pipeline":"build"}"#).unwrap(); + + let err = parse(&turbo_json_path).unwrap_err(); + + assert!(err.contains("'pipeline' must be an object")); + assert!(err.contains("string")); + } + #[test] fn test_parse_turbo_json_without_tasks() { let temp_dir = TempDir::new().unwrap(); diff --git a/src/task_discovery.rs b/src/task_discovery.rs index c275fa9..451003f 100644 --- a/src/task_discovery.rs +++ b/src/task_discovery.rs @@ -920,11 +920,15 @@ fn collect_descendant_turbo_config_paths_recursive( }; for entry in entries.flatten() { - let path = entry.path(); - if !path.is_dir() { + let Ok(file_type) = entry.file_type() else { + continue; + }; + if file_type.is_symlink() || !file_type.is_dir() { continue; } + let path = entry.path(); + let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else { continue; }; diff --git a/tests/docker_noinit/test_noinit.sh b/tests/docker_noinit/test_noinit.sh index 45b29df..f85b72e 100755 --- a/tests/docker_noinit/test_noinit.sh +++ b/tests/docker_noinit/test_noinit.sh @@ -202,7 +202,9 @@ EOF cd /home/testuser/task_include_duplicates duplicate_output=$(dela list 2>&1) -if echo "$duplicate_output" | grep -q 'Found multiple tasks (build) included by "shared"'; then +if echo "$duplicate_output" | grep -q "multiple tasks" \ + && echo "$duplicate_output" | grep -q "build" \ + && echo "$duplicate_output" | grep -q "shared"; then echo "${GREEN}✓ dela reports duplicate flattened Taskfile task names${NC}" else echo "${RED}✗ dela did not report duplicate flattened Taskfile task names${NC}" @@ -393,7 +395,14 @@ fi # Test 19b: Verify allow-command stores the defining workflow path echo "\nTest 19b: Verifying GitHub Actions allowlist source attribution" -echo "2" | dela allow-command test-a >/dev/null 2>&1 +github_allow_stderr=$(mktemp) +if ! echo "2" | dela allow-command test-a >/dev/null 2>"$github_allow_stderr"; then + echo "${RED}✗ allow-command failed for GitHub Actions task${NC}" + cat "$github_allow_stderr" + rm -f "$github_allow_stderr" + exit 1 +fi +rm -f "$github_allow_stderr" if grep -q "/home/testuser/test_project/.github/workflows/test.yml" /home/testuser/.config/dela/allowlist.toml; then echo "${GREEN}✓ GitHub Actions allowlist entries use the defining workflow file${NC}" else