From 1f15130dd614422b78b995b609f6eb17c5804256 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Sat, 1 Aug 2026 18:42:39 +0100 Subject: [PATCH] Reach kind-aware dep removal from the CLI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `remove_dep` has been kind-aware since the deps-key change, but no verb reached it, so an edge authored by mistake — a `related` typed where `blocks` was meant, a `discovered-from` linking the wrong parent — was removable only with raw SQL against the database. `--blocked-by` could clear the blocker set wholesale, which left every other kind, and the ability to drop one edge of a pair carrying two, with no spelling at all. `voro set --unlink :` names one edge in the direction `show` prints it — the edge belongs to the task being edited — and drops exactly that one. The flag repeats for several. The echo reads the kind the way `show` does (`no longer blocked by #9`, never the kind backwards), and because dropping a blocker reconciles readiness in the store, the promotion that can follow is echoed beside it, symmetrically with `--blocks`' demotion echo; the trailing state is read back after the edit rather than reported from before it. No core logic changed: the store call and its refusal of an absent edge already existed. Covered at the CLI level — one kind of a pair surviving the other's removal, the refusal, the promotion, and the deliberate non-promotion when the last blocker goes (a parked task with no blockers is deferred by choice, DESIGN.md §5, so `unpark` stays the manual escape). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DxkK1vKA77ewc7vgnRTT9R --- CHANGELOG.md | 5 + crates/voro/src/cli.rs | 193 +++++++++++++++++++++++++- docs/DESIGN.md | 2 +- plugins/voro/skills/voro-cli/SKILL.md | 8 +- 4 files changed, 203 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f73069..4181d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `voro set --unlink :` drops a single dependency edge — + `related:7`, `discovered-from:4`, `blocks:9` — named as `voro show` lists it. + A pair of tasks carrying two edges keeps the one not named, so an edge + authored by mistake no longer has to be removed with raw SQL; dropping a + blocker reconciles readiness as any other blocker edit does. - **Document links**: register the plan or design doc a body of work derives from with `voro doc add `, and link it to the tasks it spawned (`voro doc link/unlink`, or `--doc` on `voro add`/`voro set`). `voro diff --git a/crates/voro/src/cli.rs b/crates/voro/src/cli.rs index 70d3ac8..94879bd 100644 --- a/crates/voro/src/cli.rs +++ b/crates/voro/src/cli.rs @@ -101,12 +101,18 @@ tasks the flag with the running task's id) set [--title T] [--priority 0-3] [--agent NAME | --no-agent] [--body TEXT | --body-file PATH] [--blocked-by IDS] [--blocks IDS] - [--pr URL | --no-pr] [--branch NAME | --no-branch] [--human | --no-human] - [--deep | --no-deep] [--summary TEXT | --summary-file PATH] + [--unlink KIND:ID] [--pr URL | --no-pr] [--branch NAME | --no-branch] + [--human | --no-human] [--deep | --no-deep] + [--summary TEXT | --summary-file PATH] [--repo NAME | --no-repo] [--doc DOCS | --no-doc] --blocked-by replaces this task's own blocker list; --blocks adds this task as a blocker of each listed task + --unlink drops one dependency edge of this + task, named as `show` lists it — + blocks:9, discovered-from:4, parent:2, + related:7 — leaving any other edge to the + same task standing; repeat it for several --pr tracks a GitHub PR (URL or owner/repo#N) for review; --no-pr clears it. --branch sets the git branch dispatch injects into the @@ -478,6 +484,8 @@ struct SetArgs { blocked_by: Option, #[arg(long)] blocks: Option, + #[arg(long, value_name = "KIND:ID")] + unlink: Vec, #[arg(long)] pr: Option, #[arg(long, conflicts_with = "pr")] @@ -695,6 +703,48 @@ fn apply_blocks_flag(store: &mut Store, blocker_id: i64, raw: &str) -> Result Result<(DepKind, i64), String> { + let (kind, id) = raw + .rsplit_once(':') + .ok_or_else(|| format!("unlink must be KIND:ID, got '{raw}'"))?; + let kind = DepKind::parse(kind.trim()).map_err(|e| e.to_string())?; + let id = id + .trim() + .parse() + .map_err(|_| format!("unlink must be KIND:ID, got '{raw}'"))?; + Ok((kind, id)) +} + +/// Apply `--unlink KIND:ID`: drop exactly the named edges of `task_id`, and +/// echo each the way `show` reads it, so a `blocks` edge is never reported +/// backwards. Removing a blocker reconciles readiness in the store, and the +/// promotion it can produce is echoed for the same reason `--blocks` echoes +/// its demotion — the graph edit's effect on the queue is the point of it. +fn apply_unlink_flag(store: &mut Store, task_id: i64, specs: &[String]) -> Result { + let mut out = String::new(); + for spec in specs { + let (kind, other) = parse_edge(spec)?; + let before = store.task(task_id).map_err(|e| e.to_string())?.state; + store + .remove_dep(task_id, other, kind) + .map_err(|e| e.to_string())?; + match kind { + DepKind::Blocks => { + write!(out, "\ntask {task_id} no longer blocked by #{other}").unwrap() + } + _ => write!(out, "\ntask {task_id} no longer {kind} #{other}").unwrap(), + } + let after = store.task(task_id).map_err(|e| e.to_string())?.state; + if before == TaskState::Parked && after == TaskState::Ready { + write!(out, " — #{task_id} promoted to ready").unwrap(); + } + } + Ok(out) +} + /// A value given inline (`--body TEXT`) or read from a file (`--body-file /// PATH`), the latter for multi-line PR-ready text (DESIGN.md §8). The pairs /// are mutually exclusive in the parser, so at most one arrives here; `None` @@ -1325,6 +1375,14 @@ fn set_verb(store: &mut Store, args: SetArgs) -> Result { Some(raw) => apply_blocks_flag(store, id, raw)?, None => String::new(), }; + // Unlinking a blocker can promote this task, so read it back rather than + // echoing the state it held before the edge went. + let (task, unlink_echo) = if args.unlink.is_empty() { + (task, String::new()) + } else { + let echo = apply_unlink_flag(store, id, &args.unlink)?; + (store.task(id).map_err(|e| e.to_string())?, echo) + }; let task = if args.no_pr { store.set_pr(id, None).map_err(|e| e.to_string())? } else if let Some(raw) = &args.pr { @@ -1372,7 +1430,7 @@ fn set_verb(store: &mut Store, args: SetArgs) -> Result { String::new() }; Ok(format!( - "task {} updated ({}){blocks_echo}{docs_echo}", + "task {} updated ({}){blocks_echo}{unlink_echo}{docs_echo}", task.id, task.state )) } @@ -3016,6 +3074,134 @@ mod tests { assert!(shown.contains("dep: discovered-from 1"), "{shown}"); } + #[test] + fn unlink_drops_one_kind_of_a_pair_carrying_two() { + let mut s = store(); + ok(&mut s, &["project", "add", "demo", "/tmp"]); + ok(&mut s, &["add", "demo", "Source", "--state", "ready"]); + propose(&mut s, &["propose", "demo", "Follow-up", "--from", "1"]).unwrap(); + ok(&mut s, &["set", "2", "--blocked-by", "1"]); + + let out = ok(&mut s, &["set", "2", "--unlink", "blocks:1"]); + assert!(out.contains("task 2 no longer blocked by #1"), "{out}"); + let shown = ok(&mut s, &["show", "2"]); + assert!(!shown.contains("blocked by #1"), "{shown}"); + assert!(shown.contains("dep: discovered-from 1"), "{shown}"); + + // and the other way round: the provenance edge goes, the blocker stays + ok(&mut s, &["set", "2", "--blocked-by", "1"]); + let out = ok(&mut s, &["set", "2", "--unlink", "discovered-from:1"]); + assert!(out.contains("task 2 no longer discovered-from #1"), "{out}"); + let shown = ok(&mut s, &["show", "2"]); + assert!(shown.contains("dep: blocked by #1"), "{shown}"); + assert!(!shown.contains("discovered-from"), "{shown}"); + } + + #[test] + fn unlink_reconciles_readiness_and_says_so() { + let mut s = store(); + ok(&mut s, &["project", "add", "demo", "/tmp"]); + ok( + &mut s, + &["add", "demo", "Closed blocker", "--state", "ready"], + ); + ok(&mut s, &["add", "demo", "Open blocker", "--state", "ready"]); + let out = ok( + &mut s, + &[ + "add", + "demo", + "Dependent", + "--state", + "ready", + "--blocked-by", + "1,2", + ], + ); + assert!(out.contains("(parked)"), "{out}"); + ok(&mut s, &["start", "1"]); + ok(&mut s, &["done", "1"]); + ok(&mut s, &["accept", "1"]); + + // dropping the one open blocker leaves a closed one behind it, so the + // dependent is genuinely actionable and the store promotes it + let out = ok(&mut s, &["set", "3", "--unlink", "blocks:2"]); + assert!(out.contains("task 3 no longer blocked by #2"), "{out}"); + assert!(out.contains("#3 promoted to ready"), "{out}"); + assert!(out.contains("task 3 updated (ready)"), "{out}"); + assert!(ok(&mut s, &["list", "--state", "ready"]).contains("Dependent")); + } + + #[test] + fn unlinking_the_last_blocker_leaves_the_task_parked() { + // A parked task with no blockers at all is deliberately deferred + // (DESIGN.md §5), so emptying the blocker set — however it is spelled — + // never promotes; `unpark` is the manual escape. + let mut s = store(); + ok(&mut s, &["project", "add", "demo", "/tmp"]); + ok(&mut s, &["add", "demo", "Blocker", "--state", "ready"]); + ok( + &mut s, + &[ + "add", + "demo", + "Dependent", + "--state", + "ready", + "--blocked-by", + "1", + ], + ); + + let out = ok(&mut s, &["set", "2", "--unlink", "blocks:1"]); + assert!(out.contains("task 2 updated (parked)"), "{out}"); + assert!(!out.contains("promoted"), "{out}"); + assert!(s.deps_of(2).unwrap().is_empty()); + assert!(ok(&mut s, &["unpark", "2"]).contains("ready")); + } + + #[test] + fn unlink_refuses_an_edge_that_is_not_there() { + let mut s = store(); + ok(&mut s, &["project", "add", "demo", "/tmp"]); + ok(&mut s, &["add", "demo", "Source", "--state", "ready"]); + propose(&mut s, &["propose", "demo", "Follow-up", "--from", "1"]).unwrap(); + + let e = err(&mut s, &["set", "2", "--unlink", "related:1"]); + assert!(e.contains("#2 has no related dependency on #1"), "{e}"); + // the edge that *is* there survives the refusal + assert!(ok(&mut s, &["show", "2"]).contains("dep: discovered-from 1")); + + let e = err(&mut s, &["set", "2", "--unlink", "sibling:1"]); + assert!(e.contains("unknown dep kind 'sibling'"), "{e}"); + let e = err(&mut s, &["set", "2", "--unlink", "blocks"]); + assert!(e.contains("unlink must be KIND:ID"), "{e}"); + } + + #[test] + fn unlink_repeats_for_several_edges() { + let mut s = store(); + ok(&mut s, &["project", "add", "demo", "/tmp"]); + ok(&mut s, &["add", "demo", "Source", "--state", "ready"]); + propose(&mut s, &["propose", "demo", "Follow-up", "--from", "1"]).unwrap(); + ok(&mut s, &["set", "2", "--blocked-by", "1"]); + + let out = ok( + &mut s, + &[ + "set", + "2", + "--unlink", + "blocks:1", + "--unlink", + "discovered-from:1", + ], + ); + assert!(out.contains("no longer blocked by #1"), "{out}"); + assert!(out.contains("no longer discovered-from #1"), "{out}"); + assert!(s.deps_of(2).unwrap().is_empty()); + } + #[test] fn run_propose_without_from_links_nothing() { // `run` consults no environment: a bare `propose` never picks up an @@ -3364,6 +3550,7 @@ mod tests { assert!(out.contains("--blocked-by IDS"), "{out}"); assert!(out.contains("--blocks IDS"), "{out}"); assert!(out.contains("wait on"), "{out}"); + assert!(out.contains("--unlink KIND:ID"), "{out}"); } #[test] diff --git a/docs/DESIGN.md b/docs/DESIGN.md index e7d3363..729724e 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -148,7 +148,7 @@ CREATE TABLE events ( -- append-only audit of state transitions & ans Ready-work detection — the thing beads would have provided — is one query: a task is *unblocked* when no `blocks` dependency points at a task not in `done`/`rejected`. Only `blocks` gates readiness; `discovered-from`, `parent`, and `related` are navigational metadata. When a task's last blocker closes, the store promotes it `parked → ready` and stamps `state_since`, which is what makes the prepared-prompt pattern work: tomorrow's task, written today and chained behind its blocker, surfaces fully loaded the moment it becomes actionable. Auto-promotion applies only to parked tasks that *have* blockers — a parked task with none is deliberately deferred and moves only by manual unpark. The reverse holds too: adding an open blocker to a `ready` or `stalled` task demotes it back to `parked` in the same write (a demoted stall re-promotes to `ready`, not `stalled` — the stall context is stale by the time the blocker closes), and any state transition that would land a task in `ready` or `stalled` while a blocker is still open — triage, abort, manual unpark, a reconciled stall — is reconciled the same way. The invariant is therefore total: `ready` always means genuinely actionable, so the scheduler can hide blocked work by hiding `parked` alone. -The edge's kind is part of its identity, which is why it sits inside the primary key rather than beside it. A pair of tasks routinely carries two edges at once — the commonest shape in this repository is a follow-up filed mid-session with `propose --from`, which is `discovered-from` its parent and, once someone notices the ordering, gated on it as well — and a key of the pair alone could hold only the first of them. Every write to `deps` is therefore keyed on all three columns and none of them silently discards a row: `add_dep` refuses an edge that already exists rather than reporting a success it did not perform, `set_blocks_deps` deduplicates the id list it is handed and then inserts plainly, and `block_tasks` alone stays idempotent, its conflict clause scoped to the identical edge so that re-blocking a task it already blocks is a no-op while an edge of another kind between the same pair is untouched. Removal is kind-aware for the same reason — dropping a blocker leaves the `discovered-from` edge beside it standing — and removing an edge that is not there is an error, not a quiet success. +The edge's kind is part of its identity, which is why it sits inside the primary key rather than beside it. A pair of tasks routinely carries two edges at once — the commonest shape in this repository is a follow-up filed mid-session with `propose --from`, which is `discovered-from` its parent and, once someone notices the ordering, gated on it as well — and a key of the pair alone could hold only the first of them. Every write to `deps` is therefore keyed on all three columns and none of them silently discards a row: `add_dep` refuses an edge that already exists rather than reporting a success it did not perform, `set_blocks_deps` deduplicates the id list it is handed and then inserts plainly, and `block_tasks` alone stays idempotent, its conflict clause scoped to the identical edge so that re-blocking a task it already blocks is a no-op while an edge of another kind between the same pair is untouched. Removal is kind-aware for the same reason — dropping a blocker leaves the `discovered-from` edge beside it standing — and removing an edge that is not there is an error, not a quiet success. The operator reaches that removal as `voro set --unlink :`, naming the edge in the direction `show` prints it, since the alternative for every kind but `blocks` — whose whole set `--blocked-by` can replace — was raw SQL against the database. The **repos** table (§3) is where the checkout moved to, and splitting it out of `projects` was deliberately *not* additive — the one place this document's "additive where possible" rule yields. `projects.path` is dropped rather than left in place: the migration inserts one default repo per project (named after the project, path = the old `projects.path`) and then removes the column, so there is never a moment with two sources of truth for the same checkout. Leaving the column as a shadow copy would have been the additive move and the worse one, since every consumer would then have had to be trusted to prefer the repo, with no compiler to enforce it; dropping it turns the migration into a compile error at every call site instead. A single-operator local database with a numbered-migration runner makes that affordable — there are no other installs to coordinate with, and the conversion is verified in place by a test that opens a pre-0012 database and checks every project's old path reappears as its default repo. diff --git a/plugins/voro/skills/voro-cli/SKILL.md b/plugins/voro/skills/voro-cli/SKILL.md index fdcd228..034bcb1 100644 --- a/plugins/voro/skills/voro-cli/SKILL.md +++ b/plugins/voro/skills/voro-cli/SKILL.md @@ -37,7 +37,7 @@ voro explain # score decomposition (weight × priority + age bonus) voro add --body-file plan.md [--priority 0-3] [--blocked-by 3,7] [--blocks 9] [--repo NAME] [--deep] voro set <id> [--title T] [--priority N] [--body-file F] - [--blocked-by IDS] [--blocks IDS] + [--blocked-by IDS] [--blocks IDS] [--unlink KIND:ID] [--branch NAME] # intended git branch dispatch injects [--repo NAME | --no-repo] # which checkout the task runs in [--deep | --no-deep] # dispatch on the agent's strongest model @@ -51,6 +51,12 @@ discovered-prerequisite pattern). On `set`, `--blocked-by` *replaces* the task's own blocker list while `--blocks` is *additive*. Both directions echo their effect and are cycle-checked; `show` renders the edge as `blocked by #N`. +`--unlink KIND:ID` drops a single edge of any kind — `blocks:9`, +`discovered-from:4`, `parent:2`, `related:7` — naming it as `show` lists it +under that task, and leaves any other edge to the same task standing. Repeat +the flag for several. Naming an edge that is not there fails rather than +reporting a success it did not perform. + `add` defaults to state `proposed`, which is what an agent should almost always want: proposed tasks wait for human triage and never enter the queues untriaged. Write the body as a **dispatchable prompt** — self-contained