Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions dev_docs/project_plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**
- [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.
- [ ] [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
Expand Down
129 changes: 78 additions & 51 deletions src/allowlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,52 +59,81 @@ 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;
}
}

// Second pass: Check for allow entries
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
Expand Down Expand Up @@ -134,32 +163,18 @@ pub fn check_task_allowed(task: &Task) -> Result<bool, String> {
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)
}
}
}
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)
Expand All @@ -172,20 +187,10 @@ pub fn check_task_allowed_with_scope(task: &Task, scope: AllowScope) -> Result<b
// Only proceed with allowlist operations if dela is initialized
let mut allowlist = load_allowlist()?;

// 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));
save_allowlist(&allowlist)?;
Ok(true)
}
Expand All @@ -204,6 +209,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(),
Expand Down Expand Up @@ -423,6 +429,27 @@ mod tests {
reset_to_real_environment();
}

#[test]
#[serial]
fn test_is_task_allowed_uses_definition_path_for_composed_tasks() {
let (temp_dir, mut task) = setup_test_env();
task.file_path = PathBuf::from("/project/.github/workflows");
task.definition_path = Some(PathBuf::from("/project/.github/workflows/test.yml"));

let mut allowlist = Allowlist::default();
allowlist.entries.push(AllowlistEntry {
path: PathBuf::from("/project/.github/workflows/test.yml"),
scope: AllowScope::File,
tasks: None,
});
save_allowlist(&allowlist).unwrap();

assert_eq!(is_task_allowed(&task).unwrap(), (true, false));

drop(temp_dir);
reset_to_real_environment();
}

#[test]
#[serial]
fn test_is_task_allowed_precedence() {
Expand Down
8 changes: 7 additions & 1 deletion src/commands/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,10 @@ pub fn execute(verbose: bool) -> Result<(), String> {
let mut runner_files: HashMap<String, String> = 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
Loading
Loading