1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
#[allow(clippy::wildcard_imports)]
use super::*;
impl Store {
/// Point `child` at `parent`, or repoint it if it already has one.
///
/// Both selectors go through [`Store::resolve_issue`], so `#4`, `4` and a title
/// fragment all work and an unresolvable one produces the CLI's own message. The
/// **number** is what lands in the file, because a title is not stable.
pub fn set_parent(&self, child: &str, parent: &str) -> Result<ParentOutcome> {
let child_issue = self.resolve_issue(child)?;
let parent_issue = self.resolve_issue(parent)?;
let (index, issues) = self.hierarchy_index()?;
let change = hierarchy::set_parent(&index, child_issue.number, parent_issue.number)?;
self.commit_parent_change(Store::child_in(&issues, child_issue.number)?, change)
}
/// Repoint `child` at a new parent. A no-op when the parent is unchanged.
pub fn change_parent(&self, child: &str, parent: &str) -> Result<ParentOutcome> {
let child_issue = self.resolve_issue(child)?;
let parent_issue = self.resolve_issue(parent)?;
let (index, issues) = self.hierarchy_index()?;
let change = hierarchy::change_parent(&index, child_issue.number, parent_issue.number)?;
self.commit_parent_change(Store::child_in(&issues, child_issue.number)?, change)
}
/// Remove `child`'s parent, yielding the absent state.
pub fn clear_parent(&self, child: &str) -> Result<ParentOutcome> {
let child_issue = self.resolve_issue(child)?;
let (index, issues) = self.hierarchy_index()?;
let change = hierarchy::clear_parent(&index, child_issue.number)?;
self.commit_parent_change(Store::child_in(&issues, child_issue.number)?, change)
}
/// Apply an accepted change: write the field and append exactly one event.
///
/// The single write path for all three operations, so they cannot drift in
/// whether they record what they did.
/// The `issue` must come from the same snapshot the service validated against;
/// see [`Store::hierarchy_index`]. That is what makes the assert below sound.
pub(in crate::storage) fn commit_parent_change(
&self,
mut issue: Issue,
change: hierarchy::ParentChange,
) -> Result<ParentOutcome> {
let Some(event) = change.event else {
// A no-op: nothing is written, nothing is read for write-back, and no
// event is appended. `change.parent` is still the real current state,
// because the service derived it from the snapshot, so the caller does
// not have to re-read to learn it.
return Ok(ParentOutcome {
written: None,
parent: change.parent,
event: None,
changed: false,
});
};
// Read the previous value **before** overwriting it. Building the detail
// after the assignment compares the new value with itself, and every
// repoint was recorded as `parent #7 -> #7` — an audit trail that names
// neither the old parent nor the fact that anything moved.
let previous = issue.parent;
let kind = HistoryEventKind::from_parent(event.kind);
// The detail names both sides so a reader can follow the change without
// reconstructing state from the surrounding entries. Rendered by the
// service, because the `--parent` edit flag needs the identical string and
// two copies of a spelling rule drift.
let detail = hierarchy::change_detail(previous, change.parent);
// Sound now, and it is the service's rule rather than storage's: `issue`
// comes from the validated snapshot, so `previous` is by construction what
// the service compared against. It was a `debug_assert_ne!` on a value
// re-read through a *second* snapshot, where two files sharing a `number:`
// made it fire on a correct write — panicking every dev build — and where
// release builds compiled it out and wrote the contentless
// `parent #N -> #N` event it was there to catch. The comment on
// [`Store::hierarchy_index`] is the reason it cannot drift again.
debug_assert_ne!(
previous, change.parent,
"an accepted change must alter the parent, or the event records nothing"
);
// The field only. **Nothing is written to the parent's file**: child
// membership is derived by inverting this pointer, never stored (C-003).
issue.parent = change.parent;
issue.history.append_now(&actor_identity(), kind, &detail);
issue.bump_updated();
self.update(&issue.filename, &issue)?;
Ok(ParentOutcome {
written: Some(issue),
parent: change.parent,
event: Some(event),
changed: true,
})
}
/// The in-memory snapshot the hierarchy service reads.
///
/// `pub` because the `--parent` edit flag (WP03) must validate through the
/// service before any write, exactly as this module does. It is a read, so
/// exposing it costs no more than a duplicate `list()` would.
///
/// One `IssueView` per issue, shared with the relationship service rather than
/// a second projection — two views of one issue type is how two services end up
/// disagreeing about the same snapshot.
///
/// The `Vec` is returned rather than discarded because the write path needs the
/// child **from this same snapshot**. Re-reading it through a second `list()`
/// would give the write and the validation two different notions of "current":
/// two files carrying the same `number:` (reachable by hand-edit, since
/// `HierarchyIndex::build` keys by number and last-write-wins while
/// `Store::get` is first-match), or a concurrent writer, would make the
/// service decide on one parent and the writer emit another. The invariant is
/// then not merely unasserted, it is false.
pub fn hierarchy_index(&self) -> Result<(HierarchyIndex, Vec<Issue>)> {
let issues = self.list_all()?;
let views: Vec<IssueView<'_>> = issues.iter().map(Issue::issue_view).collect();
Ok((HierarchyIndex::build(&views), issues))
}
/// The child, taken from the snapshot the service just validated against.
///
/// Returns an error rather than a default so a snapshot that somehow lacks the
/// child is a loud failure, not a silent write of an unrelated file.
pub(in crate::storage) fn child_in(snapshot: &[Issue], number: u32) -> Result<Issue> {
snapshot
.iter()
.find(|i| i.number == number)
.cloned()
.ok_or_else(|| {
TixError::NotFound(format!(
"issue #{number} vanished between validating and writing it"
))
})
}
/// Build a [`RelationshipGraph`] over every issue in the store.
///
/// The service is the only place relationship semantics live, so the write
/// path validates through it rather than re-deriving direction handling,
/// cycle detection, or blocked state locally (C-003).
/// The board's configured completion column.
///
/// Read from the first configured board. Completion is always resolved
/// from configuration, never by matching the literal string `done`; when no
/// board configures one, the service's `done` fallback applies.
///
/// Resolved from the **active** board — the one named `default`, falling back
/// to the first declared, which is exactly what
/// [`Config::default_config`](crate::config::Config::default_config) returns
/// and what every other single-surface consumer already uses.
///
/// It used to take the **first declared** board instead
/// (`configurations.values().next()`). That is a different board whenever a
/// configuration is declared before `default` with a different
/// `done_column`, and it put two different boards inside a single decision:
/// `blocked_move_guard` asked "is the target the completion column?" via
/// `completion_column(config)` — the active board — and then "is this issue
/// blocked?" via this function's graph — the *first declared* board. The
/// guard was reasoning about two different workflows in one call.
///
/// `data-model.md` §4 says completion "is always resolved through the board's
/// configured `done_column`" and that the predicate is "the only definition
/// used by FR-003, FR-004, the move guard, and every surface". That requires
/// one board, not "whichever is declared first".
pub fn relationship_done_column(&self) -> String {
// WP09: the service's completion column. The name says `done_column` because
// that is what the `RelationshipGraph` and `hierarchy` services take as a
// parameter — re-deriving the *meaning* inside them is what this mission
// ended; supplying that meaning from one place is the whole of the fix, and
// it does not change either service's signature.
//
// Before: `effective_done_column().unwrap_or("done")`, so on a board whose
// `category_map` puts `completed` on a column other than `done_column` the
// blocked marker and the hierarchy rollup were computed against the wrong
// column. `unwrap_or("done")` also invented a completion column on a board
// that declared none.
crate::workflow::completion_column_or_default(self.config.default_config(), &self.config)
}
}