Skip to content

Commit 3a8536b

Browse files
author
Dylan Storey
committed
refactor: consolidate phase transition logic and update docs
Phase Transition Consolidation (METIS-T-0056): - Add single source of truth methods to DocumentType in types.rs: valid_transitions_from(), can_transition(), next_phase(), phase_sequence() - Update PhaseTransitionService to delegate to DocumentType - Update all document can_transition_to() implementations to delegate - Update MCP transition_phase tool to use DocumentType::phase_sequence() Documentation Fixes: - Fix instructions.md: forward-only transitions, add Strategy, fix prefix length (2-8 chars), add risk_level critical option - Fix phase-transitions/SKILL.md: remove bidirectional arrows - Fix phase-flow.md: clarify forward-only with blocked exception Plugin Restructure: - Move skill/ to plugins/metis/ with improved organization - Update marketplace.json source path Version: 1.0.10
1 parent 358546a commit 3a8536b

File tree

45 files changed

+2215
-548
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+2215
-548
lines changed

.claude-plugin/marketplace.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
"plugins": [
88
{
99
"name": "metis",
10-
"source": "./skill",
10+
"source": "./plugins/metis",
1111
"description": "Flight Levels methodology for project and work management. Teaches when and why to use Metis tools for project planning, work decomposition, and phase management.",
12-
"version": "1.0.0"
12+
"version": "1.0.10"
1313
}
1414
]
1515
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
---
2+
id: consolidate-phase-transition
3+
level: task
4+
title: "Consolidate phase transition validation logic"
5+
short_code: "METIS-T-0056"
6+
created_at: 2026-01-12T13:13:49.387803+00:00
7+
updated_at: 2026-01-12T17:33:38.770114+00:00
8+
parent:
9+
blocked_by: []
10+
archived: false
11+
12+
tags:
13+
- "#task"
14+
- "#tech-debt"
15+
- "#phase/completed"
16+
17+
18+
exit_criteria_met: false
19+
strategy_id: NULL
20+
initiative_id: NULL
21+
---
22+
23+
# Consolidate phase transition validation logic
24+
25+
## Objective
26+
27+
Eliminate duplicate phase transition validation logic by having a single source of truth for valid transitions.
28+
29+
## Problem
30+
31+
Phase transition rules are currently defined in two places:
32+
33+
1. **`PhaseTransitionService::get_valid_transitions()`** - `crates/metis-docs-core/src/application/services/workspace/transition.rs`
34+
2. **Each document's `can_transition_to()` trait impl** - e.g., `crates/metis-docs-core/src/domain/documents/vision/mod.rs`
35+
36+
This caused a bug where the service claimed backward transitions were valid, but the document-level validation rejected them. We fixed this by aligning both to forward-only, but the duplication remains.
37+
38+
## Proposed Solutions
39+
40+
### Option A: Service delegates to documents
41+
Have `get_valid_transitions()` call the document's `can_transition_to()` for each possible phase instead of maintaining its own list.
42+
43+
### Option B: Documents delegate to service
44+
Remove validation from document's `transition_phase()` and have it trust the service's validation.
45+
46+
### Option C: Shared transition rules
47+
Extract transition rules to a single location (e.g., constants or a dedicated module) that both service and documents reference.
48+
49+
## Files Involved
50+
51+
- `crates/metis-docs-core/src/application/services/workspace/transition.rs`
52+
- `crates/metis-docs-core/src/domain/documents/vision/mod.rs`
53+
- `crates/metis-docs-core/src/domain/documents/strategy/mod.rs`
54+
- `crates/metis-docs-core/src/domain/documents/initiative/mod.rs`
55+
- `crates/metis-docs-core/src/domain/documents/task/mod.rs`
56+
- `crates/metis-docs-core/src/domain/documents/adr/mod.rs`
57+
58+
## Acceptance Criteria
59+
60+
## Acceptance Criteria
61+
62+
## Acceptance Criteria
63+
64+
## Acceptance Criteria
65+
66+
- [x] Single source of truth for phase transition rules
67+
- [x] No duplicate transition validation logic
68+
- [x] All existing tests pass
69+
- [x] Documentation updated if API changes
70+
71+
## Resolution
72+
73+
Implemented Option C (shared transition rules) by adding methods to `DocumentType`:
74+
- `valid_transitions_from(phase)` - returns valid target phases
75+
- `can_transition(from, to)` - checks if transition is valid
76+
- `next_phase(current)` - gets next phase in sequence
77+
- `phase_sequence()` - returns ordered phases for display
78+
79+
Updated all consumers to use these methods:
80+
- `PhaseTransitionService::get_valid_transitions()` now delegates to `DocumentType`
81+
- `PhaseTransitionService::get_next_phase()` now delegates to `DocumentType`
82+
- All document `can_transition_to()` implementations now delegate to `DocumentType`
83+
- MCP `transition_phase.rs` now uses `DocumentType::phase_sequence()`

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ members = [
88
resolver = "2"
99

1010
[workspace.package]
11-
version = "1.0.9"
11+
version = "1.0.10"
1212
edition = "2021"
1313
license = "Apache-2.0"
1414
authors = ["Dylan Storey <contact@colliery.io>"]

crates/metis-docs-core/src/application/services/workspace/transition.rs

Lines changed: 37 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -245,134 +245,22 @@ impl PhaseTransitionService {
245245
Ok(())
246246
}
247247

248-
/// Get valid transitions from a given phase for a document type
248+
/// Get valid transitions from a given phase for a document type.
249+
/// Delegates to DocumentType::valid_transitions_from() - the single source of truth.
249250
fn get_valid_transitions(&self, doc_type: DocumentType, from_phase: Phase) -> Vec<Phase> {
250-
match doc_type {
251-
DocumentType::Vision => match from_phase {
252-
Phase::Draft => vec![Phase::Review],
253-
Phase::Review => vec![Phase::Draft, Phase::Published],
254-
Phase::Published => vec![Phase::Review],
255-
_ => vec![],
256-
},
257-
DocumentType::Strategy => match from_phase {
258-
Phase::Shaping => vec![Phase::Design],
259-
Phase::Design => vec![Phase::Shaping, Phase::Ready],
260-
Phase::Ready => vec![Phase::Design, Phase::Active],
261-
Phase::Active => vec![Phase::Ready, Phase::Completed],
262-
Phase::Completed => vec![],
263-
_ => vec![],
264-
},
265-
DocumentType::Initiative => match from_phase {
266-
Phase::Discovery => vec![Phase::Design],
267-
Phase::Design => vec![Phase::Discovery, Phase::Ready],
268-
Phase::Ready => vec![Phase::Design, Phase::Decompose],
269-
Phase::Decompose => vec![Phase::Ready, Phase::Active],
270-
Phase::Active => vec![Phase::Decompose, Phase::Completed],
271-
Phase::Completed => vec![],
272-
_ => vec![],
273-
},
274-
DocumentType::Task => match from_phase {
275-
Phase::Backlog => vec![Phase::Todo],
276-
Phase::Todo => vec![Phase::Active, Phase::Blocked],
277-
Phase::Active => vec![Phase::Todo, Phase::Completed, Phase::Blocked],
278-
Phase::Blocked => vec![Phase::Todo, Phase::Active],
279-
Phase::Completed => vec![],
280-
_ => vec![],
281-
},
282-
DocumentType::Adr => match from_phase {
283-
Phase::Draft => vec![Phase::Discussion],
284-
Phase::Discussion => vec![Phase::Draft, Phase::Decided],
285-
Phase::Decided => vec![Phase::Superseded],
286-
Phase::Superseded => vec![],
287-
_ => vec![],
288-
},
289-
}
251+
doc_type.valid_transitions_from(from_phase)
290252
}
291253

292-
/// Get the next phase in the natural sequence for a document type
254+
/// Get the next phase in the natural sequence for a document type.
255+
/// Delegates to DocumentType::next_phase() - the single source of truth.
293256
fn get_next_phase(&self, doc_type: DocumentType, current_phase: Phase) -> Result<Phase> {
294-
match doc_type {
295-
DocumentType::Vision => match current_phase {
296-
Phase::Draft => Ok(Phase::Review),
297-
Phase::Review => Ok(Phase::Published),
298-
Phase::Published => Err(MetisError::InvalidPhaseTransition {
299-
from: current_phase.to_string(),
300-
to: "none".to_string(),
301-
doc_type: "vision".to_string(),
302-
}),
303-
_ => Err(MetisError::InvalidPhaseTransition {
304-
from: current_phase.to_string(),
305-
to: "unknown".to_string(),
306-
doc_type: "vision".to_string(),
307-
}),
308-
},
309-
DocumentType::Strategy => match current_phase {
310-
Phase::Shaping => Ok(Phase::Design),
311-
Phase::Design => Ok(Phase::Ready),
312-
Phase::Ready => Ok(Phase::Active),
313-
Phase::Active => Ok(Phase::Completed),
314-
Phase::Completed => Err(MetisError::InvalidPhaseTransition {
315-
from: current_phase.to_string(),
316-
to: "none".to_string(),
317-
doc_type: "strategy".to_string(),
318-
}),
319-
_ => Err(MetisError::InvalidPhaseTransition {
320-
from: current_phase.to_string(),
321-
to: "unknown".to_string(),
322-
doc_type: "strategy".to_string(),
323-
}),
324-
},
325-
DocumentType::Initiative => match current_phase {
326-
Phase::Discovery => Ok(Phase::Design),
327-
Phase::Design => Ok(Phase::Ready),
328-
Phase::Ready => Ok(Phase::Decompose),
329-
Phase::Decompose => Ok(Phase::Active),
330-
Phase::Active => Ok(Phase::Completed),
331-
Phase::Completed => Err(MetisError::InvalidPhaseTransition {
332-
from: current_phase.to_string(),
333-
to: "none".to_string(),
334-
doc_type: "initiative".to_string(),
335-
}),
336-
_ => Err(MetisError::InvalidPhaseTransition {
337-
from: current_phase.to_string(),
338-
to: "unknown".to_string(),
339-
doc_type: "initiative".to_string(),
340-
}),
341-
},
342-
DocumentType::Task => {
343-
match current_phase {
344-
Phase::Backlog => Ok(Phase::Todo), // Transition from backlog to todo
345-
Phase::Todo => Ok(Phase::Active),
346-
Phase::Active => Ok(Phase::Completed),
347-
Phase::Blocked => Ok(Phase::Active), // Unblock by default
348-
Phase::Completed => Err(MetisError::InvalidPhaseTransition {
349-
from: current_phase.to_string(),
350-
to: "none".to_string(),
351-
doc_type: "task".to_string(),
352-
}),
353-
_ => Err(MetisError::InvalidPhaseTransition {
354-
from: current_phase.to_string(),
355-
to: "unknown".to_string(),
356-
doc_type: "task".to_string(),
357-
}),
358-
}
257+
doc_type.next_phase(current_phase).ok_or_else(|| {
258+
MetisError::InvalidPhaseTransition {
259+
from: current_phase.to_string(),
260+
to: "none".to_string(),
261+
doc_type: doc_type.to_string(),
359262
}
360-
DocumentType::Adr => match current_phase {
361-
Phase::Draft => Ok(Phase::Discussion),
362-
Phase::Discussion => Ok(Phase::Decided),
363-
Phase::Decided => Ok(Phase::Superseded),
364-
Phase::Superseded => Err(MetisError::InvalidPhaseTransition {
365-
from: current_phase.to_string(),
366-
to: "none".to_string(),
367-
doc_type: "adr".to_string(),
368-
}),
369-
_ => Err(MetisError::InvalidPhaseTransition {
370-
from: current_phase.to_string(),
371-
to: "unknown".to_string(),
372-
doc_type: "adr".to_string(),
373-
}),
374-
},
375-
}
263+
})
376264
}
377265

378266
/// Check if a phase transition is valid without performing it
@@ -568,27 +456,32 @@ mod tests {
568456

569457
let transition_service = PhaseTransitionService::new(&workspace_dir);
570458

571-
// Test vision transitions
459+
// Test vision transitions (forward-only)
572460
let vision_draft_transitions =
573461
transition_service.get_valid_transitions_for(DocumentType::Vision, Phase::Draft);
574462
assert_eq!(vision_draft_transitions, vec![Phase::Review]);
575463

576464
let vision_review_transitions =
577465
transition_service.get_valid_transitions_for(DocumentType::Vision, Phase::Review);
578-
assert_eq!(
579-
vision_review_transitions,
580-
vec![Phase::Draft, Phase::Published]
581-
);
466+
assert_eq!(vision_review_transitions, vec![Phase::Published]);
582467

583-
// Test strategy transitions
468+
// Test strategy transitions (forward-only)
584469
let strategy_shaping_transitions =
585470
transition_service.get_valid_transitions_for(DocumentType::Strategy, Phase::Shaping);
586471
assert_eq!(strategy_shaping_transitions, vec![Phase::Design]);
587472

588-
// Test task transitions - specifically backlog to todo
473+
// Test task transitions - backlog to todo
589474
let task_backlog_transitions =
590475
transition_service.get_valid_transitions_for(DocumentType::Task, Phase::Backlog);
591476
assert_eq!(task_backlog_transitions, vec![Phase::Todo]);
477+
478+
// Test task transitions - blocked can return to todo or active
479+
let task_blocked_transitions =
480+
transition_service.get_valid_transitions_for(DocumentType::Task, Phase::Blocked);
481+
assert_eq!(
482+
task_blocked_transitions,
483+
vec![Phase::Todo, Phase::Active]
484+
);
592485
}
593486

594487
#[tokio::test]
@@ -598,7 +491,7 @@ mod tests {
598491

599492
let transition_service = PhaseTransitionService::new(&workspace_dir);
600493

601-
// Valid transitions
494+
// Valid forward transitions
602495
assert!(transition_service.is_valid_transition(
603496
DocumentType::Vision,
604497
Phase::Draft,
@@ -610,7 +503,7 @@ mod tests {
610503
Phase::Design
611504
));
612505

613-
// Invalid transitions
506+
// Invalid transitions - skipping phases
614507
assert!(!transition_service.is_valid_transition(
615508
DocumentType::Vision,
616509
Phase::Draft,
@@ -621,5 +514,17 @@ mod tests {
621514
Phase::Shaping,
622515
Phase::Active
623516
));
517+
518+
// Invalid transitions - backward (not supported)
519+
assert!(!transition_service.is_valid_transition(
520+
DocumentType::Vision,
521+
Phase::Review,
522+
Phase::Draft
523+
));
524+
assert!(!transition_service.is_valid_transition(
525+
DocumentType::Strategy,
526+
Phase::Design,
527+
Phase::Shaping
528+
));
624529
}
625530
}

crates/metis-docs-core/src/domain/documents/adr/mod.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,11 +354,8 @@ impl Document for Adr {
354354

355355
fn can_transition_to(&self, phase: Phase) -> bool {
356356
if let Ok(current_phase) = self.phase() {
357-
use Phase::*;
358-
matches!(
359-
(current_phase, phase),
360-
(Draft, Discussion) | (Discussion, Decided) | (Decided, Superseded)
361-
)
357+
// Delegate to DocumentType - the single source of truth
358+
DocumentType::Adr.can_transition(current_phase, phase)
362359
} else {
363360
false // Can't transition if we can't determine current phase
364361
}

crates/metis-docs-core/src/domain/documents/initiative/mod.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -405,15 +405,8 @@ impl Document for Initiative {
405405

406406
fn can_transition_to(&self, phase: Phase) -> bool {
407407
if let Ok(current_phase) = self.phase() {
408-
use Phase::*;
409-
matches!(
410-
(current_phase, phase),
411-
(Discovery, Design)
412-
| (Design, Ready)
413-
| (Ready, Decompose)
414-
| (Decompose, Active)
415-
| (Active, Completed)
416-
)
408+
// Delegate to DocumentType - the single source of truth
409+
DocumentType::Initiative.can_transition(current_phase, phase)
417410
} else {
418411
false // Can't transition if we can't determine current phase
419412
}

crates/metis-docs-core/src/domain/documents/strategy/mod.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,11 +407,8 @@ impl Document for Strategy {
407407

408408
fn can_transition_to(&self, phase: Phase) -> bool {
409409
if let Ok(current_phase) = self.phase() {
410-
use Phase::*;
411-
matches!(
412-
(current_phase, phase),
413-
(Shaping, Design) | (Design, Ready) | (Ready, Active) | (Active, Completed)
414-
)
410+
// Delegate to DocumentType - the single source of truth
411+
DocumentType::Strategy.can_transition(current_phase, phase)
415412
} else {
416413
false // Can't transition if we can't determine current phase
417414
}

crates/metis-docs-core/src/domain/documents/task/mod.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -352,17 +352,8 @@ impl Document for Task {
352352

353353
fn can_transition_to(&self, phase: Phase) -> bool {
354354
if let Ok(current_phase) = self.phase() {
355-
use Phase::*;
356-
match (current_phase, phase) {
357-
(Backlog, Todo) => true, // Move from backlog to todo when assigned to initiative
358-
(Todo, Active) => true,
359-
(Active, Completed) => true,
360-
(Active, Blocked) => true,
361-
(Todo, Blocked) => true, // Can pre emptively be blocked while in backlog
362-
(Blocked, Active) => true,
363-
(Blocked, Todo) => true, // Can go back to todo if unblocked
364-
_ => false,
365-
}
355+
// Delegate to DocumentType - the single source of truth
356+
DocumentType::Task.can_transition(current_phase, phase)
366357
} else {
367358
false // Can't transition if we can't determine current phase
368359
}

0 commit comments

Comments
 (0)