Skip to content

Commit fa9ff7d

Browse files
foharaclaude
andauthored
Fix @depends-on / @id resolution across lint, check-links, and CLI (#14–#19) (#20)
* Fix @depends-on reference resolution in linter (#15, #16, #18) Add a shared reference resolver (dependency::reference) that understands bare @id, #task:id, file-id#task:id, path#task:id, and file-level forms, and route the linter's DependencyExistsRule through it so all documented and natural reference forms resolve. Split comma-separated @depends-on lists into multiple refs, and report E_LINK_NOT_FOUND at the offending @depends-on: line instead of :0:0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Make check-links validate @depends-on like lint (#19) Explicit @depends-on edges are not stored as NULL-target rows, so the DB query behind check-links never saw them and reported clean while lint flagged the same references. Reparse the tree and resolve each reference through the shared resolver, merging broken ones into the report. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Resolve bare @id as a command target (#14) show/start/complete now accept a task's @id (its local_id), not just the file-id#slug full id, via a shared task_target resolver backed by a new TaskRepository::get_by_local_id. Ambiguous @ids list candidate full ids. show now reports a missing task as a not-found diagnostic (exit 5) instead of the miscategorized E_INTERNAL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Enforce @depends-on on complete with --force override (#17) lash complete now refuses when a task has a resolvable @depends-on target that isn't done or waived, listing the unmet dependencies (E_DEP_UNMET). Pass --force to override. Dependencies are resolved from the current Markdown via the shared resolver, since explicit edges aren't indexed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Document dependency-resolution fixes and update skill docs Update references/dependencies.md to document the natural @depends-on forms (bare @id, file-id#task:id, comma lists) and the completion gate. Record the work in devlog.md and the tasks index. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a0f2513 commit fa9ff7d

17 files changed

Lines changed: 1333 additions & 240 deletions

File tree

‎crates/lash-agent/src/content.rs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,21 @@ is `done` or `waived`.
256256
@depends-on: features/auth.md#task:auth.session
257257
```
258258
259-
Forms:
260-
- `path/to/file.md` — depend on the entire file
261-
- `path/to/file.md#task:<id>` — depend on a specific task
262-
- `#task:<id>` — depend on a task in the same file
263-
264-
Validate references with `lash check-links`. Auto-fix typos in references
265-
with `lash check-links --fix`.
259+
Forms (the `task:` prefix on a fragment is optional everywhere):
260+
- `<task-@id>` — a task by its `@id` (same file, or unique in the project)
261+
- `#task:<id>` / `#<id>` — a task in the same file
262+
- `<file-id>#task:<id>` / `<file-id>#<id>` — a task in another file, by file `@id`
263+
- `path/to/file.md#task:<id>` — a task in another file, by path
264+
- `path/to/file.md` — the entire file
265+
- `<file-id>` — the entire file, by file `@id`
266+
267+
List several dependencies with commas or repeated lines:
268+
`@depends-on: auth.session, payment.gateway`.
269+
270+
`lash complete` refuses while any dependency is still open (pass `--force`
271+
to override). Validate references with `lash check-links` — it reports the
272+
same broken references as `lash lint`. Auto-fix typos with
273+
`lash check-links --fix`.
266274
267275
## `@doc:` — Documentation References
268276

‎crates/lash-cli/src/cli.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,12 @@ pub enum Commands {
420420
/// and are never auto-completed.
421421
#[arg(long)]
422422
cascade: bool,
423+
424+
/// Complete even when a resolvable @depends-on target is still open.
425+
/// By default completion is refused until dependencies are done or
426+
/// waived.
427+
#[arg(long)]
428+
force: bool,
423429
},
424430

425431
/// Mark one or more tasks as in-progress
@@ -1113,11 +1119,13 @@ mod tests {
11131119
task_ids,
11141120
dry_run,
11151121
cascade,
1122+
force,
11161123
} = cli.command
11171124
{
11181125
assert_eq!(task_ids, vec!["test#task-1"]);
11191126
assert!(!dry_run);
11201127
assert!(!cascade);
1128+
assert!(!force);
11211129
} else {
11221130
panic!("Expected Complete command");
11231131
}
@@ -1160,6 +1168,16 @@ mod tests {
11601168
}
11611169
}
11621170

1171+
#[test]
1172+
fn test_cli_parse_complete_force() {
1173+
let cli = LashCli::try_parse_from(["lash", "complete", "--force", "test#task-1"]).unwrap();
1174+
if let Commands::Complete { force, .. } = cli.command {
1175+
assert!(force);
1176+
} else {
1177+
panic!("Expected Complete command");
1178+
}
1179+
}
1180+
11631181
#[test]
11641182
fn test_cli_parse_complete_requires_task_id() {
11651183
let result = LashCli::try_parse_from(["lash", "complete"]);

‎crates/lash-cli/src/commands/check_links/core.rs‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ use anyhow::{Context, Result};
66
use lash_cli::error_reporter::{ErrorDisplayMode, ErrorReporter, ErrorReporterConfig};
77
use lash_cli::formatter::{OutputFormat, Verbosity};
88
use lash_cli::theme::CliTheme;
9+
use lash_core::dependency::reference::resolve_reference;
910
use lash_core::linter::rules::semantic::BrokenDocFragmentRule;
1011
use lash_core::linter::{LintContext, LintDiagnostic, LintRule};
1112
use lash_core::parser::parse_file;
1213
use lash_db::open_database;
14+
use lash_types::dependency::DependencyKind;
1315
use lash_types::error::LashError;
1416
use lash_types::{LashConfig, TaskFile};
1517
use serde::{Deserialize, Serialize};
@@ -208,6 +210,77 @@ pub fn find_broken_links(conn: impl AsRef<std::path::Path>) -> Result<BrokenLink
208210
})
209211
}
210212

213+
/// Walk every task file under `project_root` and report `@depends-on`
214+
/// references that don't resolve.
215+
///
216+
/// `lash check-links` historically only reported dependencies the *indexer*
217+
/// had already marked broken (a `NULL` `to_task_id` row) — but explicit
218+
/// `@depends-on` edges aren't stored that way, so `check-links` silently
219+
/// passed while `lash lint` flagged the very same references (GitHub issue
220+
/// #19). This reparses the tree and resolves each reference with the **same**
221+
/// [`resolve_reference`] used by the linter, so the two surfaces always agree.
222+
///
223+
/// Files that fail to parse are skipped (surfaced by `lash lint` instead).
224+
#[must_use]
225+
pub fn find_broken_dependencies(project_root: &Path) -> Vec<FileLinks> {
226+
let Ok(markdown_files) = discover_markdown_files(&[project_root.to_path_buf()], true) else {
227+
return Vec::new();
228+
};
229+
230+
let config = LashConfig::from_root(project_root).unwrap_or_default();
231+
232+
let mut files: HashMap<PathBuf, TaskFile> = HashMap::new();
233+
for path in &markdown_files {
234+
let Ok(file) = parse_file(path, &config) else {
235+
continue;
236+
};
237+
let relative = path
238+
.strip_prefix(project_root)
239+
.unwrap_or(path)
240+
.to_path_buf();
241+
files.insert(relative, file);
242+
}
243+
244+
let mut by_file: Vec<FileLinks> = Vec::new();
245+
246+
for (rel_path, file) in &files {
247+
let ctx = LintContext::new(&config, rel_path.clone(), &files);
248+
let mut links: Vec<BrokenLink> = Vec::new();
249+
250+
for task in file.tasks.tasks() {
251+
for dep in &task.metadata.depends_on {
252+
if matches!(
253+
dep.kind,
254+
DependencyKind::Hierarchy | DependencyKind::Directory
255+
) {
256+
continue;
257+
}
258+
let resolve_path = |rel: &str| ctx.resolve_path(Path::new(rel));
259+
if resolve_reference(&dep.target, rel_path, &file.id, &files, resolve_path).is_err()
260+
{
261+
links.push(BrokenLink {
262+
from_task_full_id: lash_types::make_full_id(&file.id, &task.id),
263+
from_file_path: rel_path.display().to_string(),
264+
raw_ref: dep.target.clone(),
265+
kind: dep.kind.as_str().to_string(),
266+
});
267+
}
268+
}
269+
}
270+
271+
if !links.is_empty() {
272+
by_file.push(FileLinks {
273+
file_path: rel_path.display().to_string(),
274+
count: links.len(),
275+
links,
276+
});
277+
}
278+
}
279+
280+
by_file.sort_by(|a, b| a.file_path.cmp(&b.file_path));
281+
by_file
282+
}
283+
211284
/// Walk every task file under `project_root` and report `@doc:` annotations
212285
/// whose `#fragment` does not resolve to a heading in the target document.
213286
///
@@ -974,6 +1047,70 @@ mod tests {
9741047
);
9751048
}
9761049

1050+
// Issue #19: check-links must flag @depends-on references that lint flags.
1051+
#[test]
1052+
fn test_find_broken_dependencies_flags_unresolvable_ref() {
1053+
use tempfile::TempDir;
1054+
1055+
let temp = TempDir::new().unwrap();
1056+
let root = temp.path();
1057+
1058+
std::fs::write(root.join("lash.index.md"), "# Index\n").unwrap();
1059+
std::fs::write(
1060+
root.join("tasks.md"),
1061+
"# Tasks\n\n@id: repro\n\n## Tasks\n\n\
1062+
- [ ] Base\n \
1063+
@id: base-task\n\
1064+
- [ ] Dependent\n \
1065+
@id: dep\n \
1066+
@depends-on: does-not-exist\n",
1067+
)
1068+
.unwrap();
1069+
1070+
let by_file = find_broken_dependencies(root);
1071+
assert_eq!(by_file.len(), 1, "expected one file with a broken dep");
1072+
assert_eq!(by_file[0].count, 1);
1073+
assert_eq!(by_file[0].links[0].raw_ref, "does-not-exist");
1074+
}
1075+
1076+
// Issue #19: forms that lint accepts must NOT be reported by check-links.
1077+
#[test]
1078+
fn test_find_broken_dependencies_accepts_valid_forms() {
1079+
use tempfile::TempDir;
1080+
1081+
let temp = TempDir::new().unwrap();
1082+
let root = temp.path();
1083+
1084+
std::fs::write(root.join("lash.index.md"), "# Index\n").unwrap();
1085+
std::fs::write(
1086+
root.join("tasks.md"),
1087+
"# Tasks\n\n@id: repro\n\n## Tasks\n\n\
1088+
- [ ] Base\n \
1089+
@id: base-task\n\
1090+
- [ ] Other\n \
1091+
@id: other-task\n\
1092+
- [ ] Bare id\n \
1093+
@id: d1\n \
1094+
@depends-on: base-task\n\
1095+
- [ ] Samefile form\n \
1096+
@id: d2\n \
1097+
@depends-on: #task:base-task\n\
1098+
- [ ] File id form\n \
1099+
@id: d3\n \
1100+
@depends-on: repro#task:base-task\n\
1101+
- [ ] Comma list\n \
1102+
@id: d4\n \
1103+
@depends-on: base-task, other-task\n",
1104+
)
1105+
.unwrap();
1106+
1107+
let by_file = find_broken_dependencies(root);
1108+
assert!(
1109+
by_file.is_empty(),
1110+
"all references resolve, expected no broken deps, got {by_file:?}"
1111+
);
1112+
}
1113+
9771114
#[test]
9781115
fn test_find_broken_doc_fragments_clean_project_returns_empty() {
9791116
use tempfile::TempDir;

‎crates/lash-cli/src/commands/check_links/mod.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,24 @@ pub fn execute(args: &CheckLinksArgs) -> Result<i32> {
8484
// Find all broken links
8585
let mut report = core::find_broken_links(&db_path).context("Failed to find broken links")?;
8686

87+
// Explicit @depends-on edges aren't stored as NULL-target rows, so the DB
88+
// query above never sees them. Reparse the tree and validate each
89+
// reference with the same resolver `lash lint` uses, then merge the
90+
// results in so the two commands agree (GitHub issue #19).
91+
for file_links in core::find_broken_dependencies(&project_root) {
92+
report.total_broken += file_links.count;
93+
if let Some(existing) = report
94+
.by_file
95+
.iter_mut()
96+
.find(|fl| fl.file_path == file_links.file_path)
97+
{
98+
existing.count += file_links.count;
99+
existing.links.extend(file_links.links);
100+
} else {
101+
report.by_file.push(file_links);
102+
}
103+
}
104+
87105
// Additionally scan for broken @doc: fragment references. These are
88106
// semantic warnings raised by `lash lint`, but `check-links` is the
89107
// natural canonical "are my references intact?" command, so it would be

0 commit comments

Comments
 (0)