huskies: merge 503_bug_depends_on_pointing_at_an_archived_story_is_silently_treated_as_deps_met_surprising_users

This commit is contained in:
dave
2026-04-09 18:27:25 +00:00
parent 8b2e068d3e
commit 41515e3b8f
6 changed files with 393 additions and 3 deletions
+8
View File
@@ -88,6 +88,14 @@ Items move through stages by moving the file between directories:
Items in `5_done` are auto-swept to `6_archived` after 4 hours by the server.
### Dependency Semantics (`depends_on`)
Work items can declare `depends_on: [N, M]` in their front matter. The auto-assign loop promotes a backlog item to current only when every listed dependency has reached `5_done` **or** `6_archived`.
**Archived = satisfied (recorded decision, bug 503):** A dep in `6_archived` counts as satisfied. Stories auto-sweep from `5_done` to `6_archived` after 4 hours, so by the time a dep is archived the dependent story is normally already in current. Changing this to "archived = abandoned" would re-block already-promoted stories when their deps sweep, which is a worse regression.
**When the dep is already archived at creation time:** If you create a story whose `depends_on` points at a story already in `6_archived`, `create_story` will emit a visible warning. The dep will resolve immediately on the next promotion tick. If the archived dep was abandoned rather than cleanly completed, remove the `depends_on` field or move the new story back to backlog manually after promotion fires.
### Filesystem Watcher
The server watches `.story_kit/work/` for changes. When a file is created, moved, or modified, the watcher auto-commits with a deterministic message and broadcasts a WebSocket notification to the frontend. This means:
@@ -14,8 +14,8 @@ use super::scan::{
is_story_assigned_for_stage, scan_stage_items,
};
use super::story_checks::{
has_merge_failure, has_review_hold, has_unmet_dependencies, is_story_blocked,
read_story_front_matter_agent,
check_archived_dependencies, has_merge_failure, has_review_hold, has_unmet_dependencies,
is_story_blocked, read_story_front_matter_agent,
};
impl AgentPool {
@@ -24,6 +24,14 @@ impl AgentPool {
/// A story is only promoted if it explicitly lists `depends_on` AND every
/// listed dependency has reached `5_done` or `6_archived`. Stories with no
/// `depends_on` are left in the backlog for human scheduling.
///
/// **Archived dep semantics:** a dep in `6_archived` counts as satisfied (since
/// stories auto-sweep from `5_done` to `6_archived` after 4 hours, and the
/// dependent story would normally already be promoted by then). However, if a
/// dep was already in `6_archived` when the dependent story was created (e.g. it
/// was abandoned/superseded before the dependent existed), a prominent warning is
/// logged so the user can see the promotion was triggered by an archived dep, not
/// a clean completion.
fn promote_ready_backlog_stories(&self, project_root: &Path) {
use crate::io::story_metadata::parse_front_matter;
@@ -49,6 +57,17 @@ impl AgentPool {
if has_unmet_dependencies(project_root, "1_backlog", story_id) {
continue;
}
// Warn if any deps were satisfied via archive rather than via clean done.
let archived_deps = check_archived_dependencies(project_root, "1_backlog", story_id);
if !archived_deps.is_empty() {
slog_warn!(
"[auto-assign] Story '{story_id}' is being promoted because deps \
{archived_deps:?} are in 6_archived (not cleanly completed via 5_done). \
These deps may have been abandoned or superseded. If this promotion is \
unintentional, remove the depends_on or manually move the story back to \
1_backlog."
);
}
// All deps met — promote from backlog to current.
slog!("[auto-assign] Story '{story_id}' deps met; promoting from backlog to current.");
if let Err(e) =
@@ -623,6 +642,53 @@ mod tests {
);
}
// ── Bug 503: archived-dep promotion visibility ─────────────────────────────
/// A backlog story whose dep is in 6_archived must still be promoted
/// (archived = satisfied), but the promotion must not silently skip the warning
/// path. This test verifies the promotion itself fires; the warning is a
/// slog_warn! side-effect that we can't easily assert on in unit tests.
#[tokio::test]
async fn auto_assign_promotes_backlog_story_when_dep_is_archived() {
let tmp = tempfile::tempdir().unwrap();
let root = tmp.path();
let sk = root.join(".huskies");
let backlog = sk.join("work/1_backlog");
let current = sk.join("work/2_current");
let archived = sk.join("work/6_archived");
std::fs::create_dir_all(&backlog).unwrap();
std::fs::create_dir_all(&current).unwrap();
std::fs::create_dir_all(&archived).unwrap();
std::fs::write(
sk.join("project.toml"),
"[[agent]]\nname = \"coder-1\"\nstage = \"coder\"\n",
)
.unwrap();
// Dep 490 is in 6_archived (e.g. a CRDT spike that was archived/superseded).
std::fs::write(archived.join("490_spike_crdt.md"), "---\nname: CRDT Spike\n---\n")
.unwrap();
// Story 478 depends on 490 (the archived spike).
std::fs::write(
backlog.join("478_story_dependent.md"),
"---\nname: Dependent\ndepends_on: [490]\n---\n",
)
.unwrap();
let pool = AgentPool::new_test(3001);
pool.auto_assign_available_work(root).await;
// Story 478 must be promoted to 2_current/ even though dep 490 is only in
// 6_archived (not in 5_done), because archived = satisfied.
assert!(
current.join("478_story_dependent.md").exists(),
"story 478 should be promoted to 2_current/ when dep 490 is in 6_archived"
);
assert!(
!backlog.join("478_story_dependent.md").exists(),
"story 478 must be removed from 1_backlog/ after promotion"
);
}
/// Stories in backlog with NO depends_on must NOT be auto-promoted.
#[tokio::test]
async fn auto_assign_does_not_promote_backlog_story_without_deps() {
@@ -84,6 +84,25 @@ pub(super) fn has_unmet_dependencies(
!crate::io::story_metadata::check_unmet_deps(project_root, stage_dir, story_id).is_empty()
}
/// Return the list of dependency story numbers that are in `6_archived` (satisfied
/// via archive rather than via a clean `5_done` completion).
///
/// Used to emit a warning when backlog promotion fires because one or more deps were
/// archived. Returns an empty `Vec` when no deps are archived. Reads from CRDT
/// first; falls back to filesystem when CRDT is not initialised.
pub(super) fn check_archived_dependencies(
project_root: &Path,
stage_dir: &str,
story_id: &str,
) -> Vec<u32> {
// Prefer CRDT-based check when the item is known to CRDT.
if crate::crdt_state::read_item(story_id).is_some() {
return crate::crdt_state::check_archived_deps_crdt(story_id);
}
// Fallback: filesystem.
crate::io::story_metadata::check_archived_deps(project_root, stage_dir, story_id)
}
/// Return `true` if the story file has a `merge_failure` field in its front matter.
pub(super) fn has_merge_failure(project_root: &Path, _stage_dir: &str, story_id: &str) -> bool {
use crate::io::story_metadata::parse_front_matter;
@@ -170,4 +189,44 @@ mod tests {
std::fs::write(current.join("5_story_free.md"), "---\nname: Free\n---\n").unwrap();
assert!(!has_unmet_dependencies(tmp.path(), "2_current", "5_story_free"));
}
// ── Bug 503: archived-dep visibility ─────────────────────────────────────
/// check_archived_dependencies returns dep IDs that are in 6_archived.
#[test]
fn check_archived_dependencies_returns_archived_ids() {
let tmp = tempfile::tempdir().unwrap();
let backlog = tmp.path().join(".huskies/work/1_backlog");
let archived = tmp.path().join(".huskies/work/6_archived");
std::fs::create_dir_all(&backlog).unwrap();
std::fs::create_dir_all(&archived).unwrap();
std::fs::write(archived.join("500_spike_crdt.md"), "---\nname: CRDT Spike\n---\n").unwrap();
std::fs::write(
backlog.join("503_story_dependent.md"),
"---\nname: Dependent\ndepends_on: [500]\n---\n",
)
.unwrap();
let archived_deps =
check_archived_dependencies(tmp.path(), "1_backlog", "503_story_dependent");
assert_eq!(archived_deps, vec![500]);
}
/// check_archived_dependencies returns empty when dep is in 5_done (not archived).
#[test]
fn check_archived_dependencies_empty_when_dep_in_done() {
let tmp = tempfile::tempdir().unwrap();
let backlog = tmp.path().join(".huskies/work/1_backlog");
let done = tmp.path().join(".huskies/work/5_done");
std::fs::create_dir_all(&backlog).unwrap();
std::fs::create_dir_all(&done).unwrap();
std::fs::write(done.join("490_story_done.md"), "---\nname: Done\n---\n").unwrap();
std::fs::write(
backlog.join("503_story_waiting.md"),
"---\nname: Waiting\ndepends_on: [490]\n---\n",
)
.unwrap();
let archived_deps =
check_archived_dependencies(tmp.path(), "1_backlog", "503_story_waiting");
assert!(archived_deps.is_empty());
}
}
+51
View File
@@ -431,6 +431,7 @@ fn extract_item_view(item: &PipelineItemCrdt) -> Option<PipelineItemView> {
/// according to CRDT state.
///
/// Returns `true` if the dependency is satisfied (item found in a done stage).
/// See `dep_is_archived_crdt` to distinguish archive-satisfied from cleanly-done.
pub fn dep_is_done_crdt(dep_number: u32) -> bool {
let prefix = format!("{dep_number}_");
if let Some(items) = read_all_items() {
@@ -443,6 +444,22 @@ pub fn dep_is_done_crdt(dep_number: u32) -> bool {
}
}
/// Check whether a dependency (by numeric ID prefix) is specifically in `6_archived`
/// according to CRDT state.
///
/// Used to detect when a dependency is satisfied via archive rather than via a clean
/// completion through `5_done`. Returns `false` when the CRDT layer is not initialised.
pub fn dep_is_archived_crdt(dep_number: u32) -> bool {
let prefix = format!("{dep_number}_");
if let Some(items) = read_all_items() {
items.iter().any(|item| {
item.story_id.starts_with(&prefix) && item.stage == "6_archived"
})
} else {
false
}
}
/// Check unmet dependencies for a story by reading its `depends_on` from the
/// CRDT document and checking each dependency against CRDT state.
///
@@ -461,6 +478,25 @@ pub fn check_unmet_deps_crdt(story_id: &str) -> Vec<u32> {
.collect()
}
/// Return the list of dependency numbers from `story_id`'s `depends_on` that are
/// specifically in `6_archived` according to CRDT state.
///
/// Used to emit a warning when promotion fires because a dep is archived rather than
/// cleanly completed. Returns an empty `Vec` when no deps are archived.
pub fn check_archived_deps_crdt(story_id: &str) -> Vec<u32> {
let item = match read_item(story_id) {
Some(i) => i,
None => return Vec::new(),
};
let deps = match item.depends_on {
Some(d) => d,
None => return Vec::new(),
};
deps.into_iter()
.filter(|&dep| dep_is_archived_crdt(dep))
.collect()
}
/// Hex-encode a byte slice (no external dep needed).
mod hex {
pub fn encode(bytes: &[u8]) -> String {
@@ -793,4 +829,19 @@ mod tests {
let result = check_unmet_deps_crdt("nonexistent_story");
assert!(result.is_empty());
}
// ── Bug 503: archived-dep visibility ─────────────────────────────────────
#[test]
fn dep_is_archived_crdt_returns_false_when_no_crdt_state() {
// When the global CRDT state is not initialised, must not panic.
let _ = dep_is_archived_crdt(9998);
}
#[test]
fn check_archived_deps_crdt_returns_empty_when_item_not_found() {
// Non-existent story should return empty archived deps.
let result = check_archived_deps_crdt("nonexistent_story_archived");
assert!(result.is_empty());
}
}
+47 -1
View File
@@ -7,7 +7,7 @@ use crate::http::workflow::{
create_spike_file, create_story_file, list_bug_files, list_refactor_files, load_pipeline_state,
load_upcoming_stories, update_story_in_file, validate_story_dirs,
};
use crate::io::story_metadata::{parse_front_matter, parse_unchecked_todos};
use crate::io::story_metadata::{check_archived_deps, check_archived_deps_from_list, parse_front_matter, parse_unchecked_todos};
use crate::slog_warn;
use crate::workflow::{TestCaseResult, TestStatus, evaluate_acceptance_with_coverage};
use serde_json::{Value, json};
@@ -40,6 +40,30 @@ pub(super) fn tool_create_story(args: &Value, ctx: &AppContext) -> Result<String
commit,
)?;
// Bug 503: warn at creation time if any depends_on points at an already-archived story.
// Archived = satisfied semantics: the dep will resolve immediately on the next promotion
// tick, which is surprising if the archived story was abandoned rather than cleanly done.
let archived_deps = depends_on
.as_deref()
.map(|deps| check_archived_deps_from_list(&root, deps))
.unwrap_or_default();
if !archived_deps.is_empty() {
slog_warn!(
"[create-story] Story '{story_id}' depends_on {archived_deps:?} which \
are already in 6_archived. The dep will be treated as satisfied on the \
next promotion tick. If these deps were abandoned (not cleanly completed), \
consider removing the depends_on or keeping the story in backlog manually."
);
return Ok(format!(
"Created story: {story_id}\n\n\
WARNING: depends_on {archived_deps:?} point at stories already in \
6_archived. These deps are treated as satisfied (archived = satisfied \
semantics), so this story may be auto-promoted from backlog immediately. \
If the archived deps were abandoned rather than completed, remove the \
depends_on or move the story back to backlog manually after promotion."
));
}
Ok(format!("Created story: {story_id}"))
}
@@ -320,6 +344,28 @@ pub(super) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
let root = ctx.state.get_project_root()?;
update_story_in_file(&root, story_id, user_story, description, front_matter_opt)?;
// Bug 503: warn if any depends_on in the (now updated) story points at an archived story.
let stage = crate::crdt_state::read_item(story_id)
.map(|i| i.stage)
.unwrap_or_else(|| "1_backlog".to_string());
let archived_deps = check_archived_deps(&root, &stage, story_id);
if !archived_deps.is_empty() {
slog_warn!(
"[update-story] Story '{story_id}' depends_on {archived_deps:?} which \
are already in 6_archived. The dep will be treated as satisfied on the \
next promotion tick. If these deps were abandoned (not cleanly completed), \
consider removing the depends_on or keeping the story in backlog manually."
);
return Ok(format!(
"Updated story '{story_id}'.\n\n\
WARNING: depends_on {archived_deps:?} point at stories already in \
6_archived. These deps are treated as satisfied (archived = satisfied \
semantics), so this story may be auto-promoted from backlog immediately. \
If the archived deps were abandoned rather than completed, remove the \
depends_on or move the story back to backlog manually after promotion."
));
}
Ok(format!("Updated story '{story_id}'."))
}
+160
View File
@@ -297,6 +297,14 @@ pub fn check_unmet_deps(project_root: &Path, stage_dir: &str, story_id: &str) ->
}
/// Return `true` if a story with the given numeric ID exists in `5_done` or `6_archived`.
///
/// **Dependency semantics:** Both `5_done` and `6_archived` satisfy a `depends_on` entry.
/// Stories auto-sweep from `5_done` to `6_archived` after 4 hours, so by the time a dep
/// reaches `6_archived`, the dependent story has already been promoted. When a dep is
/// already in `6_archived` at the moment of promotion (e.g., it was manually archived or
/// abandoned before the dependent story was created), the dependency is still considered
/// satisfied — but a warning is logged so the user can see that the dep was archived, not
/// cleanly completed. Use `check_archived_deps` to detect this case.
fn dep_is_done(project_root: &Path, dep_number: u32) -> bool {
let prefix = format!("{dep_number}_");
let exact = dep_number.to_string();
@@ -319,6 +327,63 @@ fn dep_is_done(project_root: &Path, dep_number: u32) -> bool {
false
}
/// Return `true` if a story with the given numeric ID exists specifically in `6_archived`
/// (i.e., it satisfies a `depends_on` but via the archive rather than via a clean done).
fn dep_is_archived(project_root: &Path, dep_number: u32) -> bool {
let prefix = format!("{dep_number}_");
let exact = dep_number.to_string();
let dir = project_root.join(".huskies").join("work").join("6_archived");
if let Ok(entries) = fs::read_dir(&dir) {
for entry in entries.flatten() {
let path = entry.path();
if path.extension().and_then(|e| e.to_str()) != Some("md") {
continue;
}
if let Some(stem) = path.file_stem().and_then(|s| s.to_str())
&& (stem == exact || stem.starts_with(&prefix))
{
return true;
}
}
}
false
}
/// Return the list of dependency story numbers from `story_id`'s front matter
/// that are in `6_archived` (satisfied via archive rather than via normal done).
///
/// Used to emit a warning when backlog promotion fires because a dep was archived
/// rather than cleanly completed. Returns an empty `Vec` when no deps are archived.
pub fn check_archived_deps(project_root: &Path, stage_dir: &str, story_id: &str) -> Vec<u32> {
let path = project_root
.join(".huskies")
.join("work")
.join(stage_dir)
.join(format!("{story_id}.md"));
let contents = match fs::read_to_string(&path) {
Ok(c) => c,
Err(_) => return Vec::new(),
};
let deps = match parse_front_matter(&contents).ok().and_then(|m| m.depends_on) {
Some(d) => d,
None => return Vec::new(),
};
deps.into_iter()
.filter(|&dep| dep_is_archived(project_root, dep))
.collect()
}
/// Given an explicit list of dep numbers, return those already in `6_archived`.
///
/// Used at story-creation time when the dep list is known in memory (before the
/// story file has been written), so the caller does not need to parse the story.
pub fn check_archived_deps_from_list(project_root: &Path, deps: &[u32]) -> Vec<u32> {
deps.iter()
.copied()
.filter(|&dep| dep_is_archived(project_root, dep))
.collect()
}
// ── In-memory content variants (no filesystem access) ───────────────
/// Remove a key from the YAML front matter of a markdown string (pure function).
@@ -694,4 +759,99 @@ workflow: tdd
assert!(!dep_is_done(tmp.path(), 101));
}
// ── Bug 503: archived-dep visibility ─────────────────────────────────────
/// check_archived_deps returns the dep IDs that are in 6_archived.
#[test]
fn check_archived_deps_returns_archived_dep_numbers() {
let tmp = tempfile::tempdir().unwrap();
let current = tmp.path().join(".huskies/work/2_current");
let archived = tmp.path().join(".huskies/work/6_archived");
std::fs::create_dir_all(&current).unwrap();
std::fs::create_dir_all(&archived).unwrap();
// Dep 100 is in 6_archived; dep 101 is not anywhere.
std::fs::write(archived.join("100_spike_old.md"), "---\nname: Old\n---\n").unwrap();
std::fs::write(
current.join("5_story_dependent.md"),
"---\nname: Dep\ndepends_on: [100, 101]\n---\n",
)
.unwrap();
let archived_deps = check_archived_deps(tmp.path(), "2_current", "5_story_dependent");
assert_eq!(archived_deps, vec![100]);
}
/// check_archived_deps returns empty when no deps are in 6_archived.
#[test]
fn check_archived_deps_returns_empty_when_dep_in_done() {
let tmp = tempfile::tempdir().unwrap();
let backlog = tmp.path().join(".huskies/work/1_backlog");
let done = tmp.path().join(".huskies/work/5_done");
std::fs::create_dir_all(&backlog).unwrap();
std::fs::create_dir_all(&done).unwrap();
// Dep 200 is in 5_done (not archived).
std::fs::write(done.join("200_story_done.md"), "---\nname: Done\n---\n").unwrap();
std::fs::write(
backlog.join("5_story_waiting.md"),
"---\nname: Waiting\ndepends_on: [200]\n---\n",
)
.unwrap();
let archived_deps = check_archived_deps(tmp.path(), "1_backlog", "5_story_waiting");
assert!(archived_deps.is_empty());
}
/// check_archived_deps returns empty when story has no depends_on.
#[test]
fn check_archived_deps_returns_empty_when_no_deps() {
let tmp = tempfile::tempdir().unwrap();
let current = tmp.path().join(".huskies/work/2_current");
std::fs::create_dir_all(&current).unwrap();
std::fs::write(current.join("3_story_free.md"), "---\nname: Free\n---\n").unwrap();
let archived_deps = check_archived_deps(tmp.path(), "2_current", "3_story_free");
assert!(archived_deps.is_empty());
}
/// check_archived_deps_from_list returns archived dep IDs from an in-memory list.
#[test]
fn check_archived_deps_from_list_returns_archived_ids() {
let tmp = tempfile::tempdir().unwrap();
let done = tmp.path().join(".huskies/work/5_done");
let archived = tmp.path().join(".huskies/work/6_archived");
std::fs::create_dir_all(&done).unwrap();
std::fs::create_dir_all(&archived).unwrap();
std::fs::write(done.join("10_story_done.md"), "---\nname: Done\n---\n").unwrap();
std::fs::write(archived.join("20_story_old.md"), "---\nname: Old\n---\n").unwrap();
// Only 20 is archived; 10 is in done, 30 is nowhere.
let result = check_archived_deps_from_list(tmp.path(), &[10, 20, 30]);
assert_eq!(result, vec![20]);
}
/// check_archived_deps_from_list returns empty when no deps are archived.
#[test]
fn check_archived_deps_from_list_empty_when_no_archived_deps() {
let tmp = tempfile::tempdir().unwrap();
let done = tmp.path().join(".huskies/work/5_done");
std::fs::create_dir_all(&done).unwrap();
std::fs::write(done.join("10_story_done.md"), "---\nname: Done\n---\n").unwrap();
let result = check_archived_deps_from_list(tmp.path(), &[10]);
assert!(result.is_empty());
}
/// dep_is_archived returns true only for stories in 6_archived, not 5_done.
#[test]
fn dep_is_archived_distinguishes_done_from_archived() {
let tmp = tempfile::tempdir().unwrap();
let done = tmp.path().join(".huskies/work/5_done");
let archived = tmp.path().join(".huskies/work/6_archived");
std::fs::create_dir_all(&done).unwrap();
std::fs::create_dir_all(&archived).unwrap();
std::fs::write(done.join("10_story_done.md"), "---\nname: Done\n---\n").unwrap();
std::fs::write(archived.join("20_story_old.md"), "---\nname: Old\n---\n").unwrap();
// 10 is in 5_done only — not archived.
assert!(!dep_is_archived(tmp.path(), 10));
// 20 is in 6_archived — archived.
assert!(dep_is_archived(tmp.path(), 20));
// 99 doesn't exist anywhere.
assert!(!dep_is_archived(tmp.path(), 99));
}
}