Skip to content

Commit a6903f9

Browse files
jpicklykclaude
jpicklyk
and
claude
authored
refactor: make NoteSchemaEntry.role type-safe with Role enum (#75)
Change NoteSchemaEntry.role from String to Role enum, eliminating stringly-typed comparisons across the schema gate layer. Parse-time conversion in YamlNoteSchemaService now maps validated strings to Role values. All comparison sites use direct enum equality instead of role.name.lowercase() intermediaries. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2ef9b55 commit a6903f9

File tree

18 files changed

+233
-179
lines changed

18 files changed

+233
-179
lines changed

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/service/NoteSchemaService.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.github.jpicklyk.mcptask.current.application.service
22

33
import io.github.jpicklyk.mcptask.current.domain.model.NoteSchemaEntry
4+
import io.github.jpicklyk.mcptask.current.domain.model.Role
45

56
/**
67
* Provides note schemas derived from `.taskorchestrator/config.yaml`.
@@ -23,7 +24,7 @@ interface NoteSchemaService {
2324
* Used to determine whether `start` from WORK should advance to REVIEW or jump to TERMINAL.
2425
* Returns false when no schema matches (schema-free mode — skip REVIEW).
2526
*/
26-
fun hasReviewPhase(tags: List<String>): Boolean = getSchemaForTags(tags)?.any { it.role == "review" } ?: false
27+
fun hasReviewPhase(tags: List<String>): Boolean = getSchemaForTags(tags)?.any { it.role == Role.REVIEW } ?: false
2728
}
2829

2930
/**

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/service/PhaseNoteContext.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ fun computePhaseNoteContext(
4343
): PhaseNoteContext? {
4444
if (role == Role.TERMINAL || schema == null) return null
4545

46-
val roleStr = role.name.lowercase()
47-
val required = schema.filter { it.role == roleStr && it.required }
46+
val required = schema.filter { it.role == role && it.required }
4847
val missing =
4948
required.filter { entry ->
5049
val note = notesByKey[entry.key]

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/compound/CreateWorkTreeTool.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ Atomically create a hierarchical work tree: root item, child items, dependencies
311311
Note(
312312
itemId = item.id,
313313
key = entry.key,
314-
role = entry.role,
314+
role = entry.role.toJsonString(),
315315
body = ""
316316
)
317317
)
@@ -368,7 +368,7 @@ Atomically create a hierarchical work tree: root item, child items, dependencies
368368
schemaEntries.map { entry ->
369369
buildJsonObject {
370370
put("key", JsonPrimitive(entry.key))
371-
put("role", JsonPrimitive(entry.role))
371+
put("role", JsonPrimitive(entry.role.toJsonString()))
372372
put("required", JsonPrimitive(entry.required))
373373
put("description", JsonPrimitive(entry.description))
374374
entry.guidance?.let { put("guidance", JsonPrimitive(it)) }
@@ -402,7 +402,7 @@ Atomically create a hierarchical work tree: root item, child items, dependencies
402402
schemaEntries.map { entry ->
403403
buildJsonObject {
404404
put("key", JsonPrimitive(entry.key))
405-
put("role", JsonPrimitive(entry.role))
405+
put("role", JsonPrimitive(entry.role.toJsonString()))
406406
put("required", JsonPrimitive(entry.required))
407407
put("description", JsonPrimitive(entry.description))
408408
entry.guidance?.let { put("guidance", JsonPrimitive(it)) }

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/items/CreateItemHandler.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ class CreateItemHandler(
158158
schemaEntries.map { entry ->
159159
buildJsonObject {
160160
put("key", JsonPrimitive(entry.key))
161-
put("role", JsonPrimitive(entry.role))
161+
put("role", JsonPrimitive(entry.role.toJsonString()))
162162
put("required", JsonPrimitive(entry.required))
163163
put("description", JsonPrimitive(entry.description))
164164
entry.guidance?.let { put("guidance", JsonPrimitive(it)) }

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/AdvanceItemTool.kt‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,7 @@ Trigger-based role transitions for WorkItems with validation, cascade detection,
224224
if (trigger == "start") {
225225
val schema = noteSchemaService.getSchemaForTags(itemTags)
226226
if (schema != null) {
227-
val currentRoleStr = item.role.toJsonString()
228-
val requiredForCurrentPhase = schema.filter { it.role == currentRoleStr && it.required }
227+
val requiredForCurrentPhase = schema.filter { it.role == item.role && it.required }
229228
if (requiredForCurrentPhase.isNotEmpty()) {
230229
val notesResult = context.noteRepository().findByItemId(item.id)
231230
val existingNotes =
@@ -242,7 +241,7 @@ Trigger-based role transitions for WorkItems with validation, cascade detection,
242241
buildErrorResult(
243242
itemId,
244243
trigger,
245-
"Gate check failed: required notes not filled for $currentRoleStr phase: ${missingKeys.joinToString()}",
244+
"Gate check failed: required notes not filled for ${item.role.toJsonString()} phase: ${missingKeys.joinToString()}",
246245
missingNotes = NoteSchemaJsonHelpers.buildMissingNotesArray(missingEntries)
247246
)
248247
)
@@ -435,8 +434,6 @@ Trigger-based role transitions for WorkItems with validation, cascade detection,
435434

436435
// Schema-driven response fields: expectedNotes, guidancePointer, noteProgress
437436
val schema = noteSchemaService.getSchemaForTags(itemTags)
438-
val newRoleStr = targetRole.toJsonString()
439-
440437
// Only query notes when a schema exists (avoids unnecessary DB call)
441438
val expectedNotesJson: JsonArray
442439
val guidancePointer: String?
@@ -456,13 +453,13 @@ Trigger-based role transitions for WorkItems with validation, cascade detection,
456453
val existingKeys = notesByKey.keys
457454

458455
// Build expectedNotes: schema entries matching the new role (tool-specific, includes "exists")
459-
val forNewRole = schema.filter { it.role == newRoleStr }
456+
val forNewRole = schema.filter { it.role == targetRole }
460457
expectedNotesJson =
461458
JsonArray(
462459
forNewRole.map { entry ->
463460
buildJsonObject {
464461
put("key", JsonPrimitive(entry.key))
465-
put("role", JsonPrimitive(entry.role))
462+
put("role", JsonPrimitive(entry.role.toJsonString()))
466463
put("required", JsonPrimitive(entry.required))
467464
put("description", JsonPrimitive(entry.description))
468465
entry.guidance?.let { put("guidance", JsonPrimitive(it)) }

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetContextTool.kt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,14 @@ Parameters:
158158
is Result.Error -> emptyList()
159159
}
160160
val notesByKey = notes.associateBy { it.key }
161-
val currentRoleStr = item.role.toJsonString()
162161

163162
// Build schema list with exists/filled status
164163
val schemaEntries =
165164
schema?.map { entry ->
166165
val note = notesByKey[entry.key]
167166
buildJsonObject {
168167
put("key", JsonPrimitive(entry.key))
169-
put("role", JsonPrimitive(entry.role))
168+
put("role", JsonPrimitive(entry.role.toJsonString()))
170169
put("required", JsonPrimitive(entry.required))
171170
put("description", JsonPrimitive(entry.description))
172171
entry.guidance?.let { put("guidance", JsonPrimitive(it)) }
@@ -214,7 +213,7 @@ Parameters:
214213
// Terminal items can never advance; schema-free items always can; schema items need all notes filled
215214
val isTerminal = item.role == Role.TERMINAL
216215
put("canAdvance", JsonPrimitive(!isTerminal && missingForPhase.isEmpty()))
217-
put("phase", JsonPrimitive(currentRoleStr))
216+
put("phase", JsonPrimitive(item.role.toJsonString()))
218217
put("missing", JsonArray(missingForPhase.map { JsonPrimitive(it) }))
219218
}
220219
)

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/domain/model/NoteSchemaEntry.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ package io.github.jpicklyk.mcptask.current.domain.model
2626
* ```
2727
*
2828
* @property key Unique identifier for the note within a schema (e.g., "acceptance-criteria")
29-
* @property role Workflow phase this note belongs to: "queue", "work", or "review"
29+
* @property role Workflow phase this note belongs to (QUEUE, WORK, or REVIEW)
3030
* @property required Whether this note must be filled before advancing past its phase
3131
* @property description Human-readable description of what the note should contain
3232
* @property guidance Optional detailed instructions for filling the note
3333
*/
3434
data class NoteSchemaEntry(
3535
val key: String,
36-
val role: String,
36+
val role: Role,
3737
val required: Boolean = false,
3838
val description: String = "",
3939
val guidance: String? = null

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/config/YamlNoteSchemaService.kt‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package io.github.jpicklyk.mcptask.current.infrastructure.config
22

33
import io.github.jpicklyk.mcptask.current.application.service.NoteSchemaService
44
import io.github.jpicklyk.mcptask.current.domain.model.NoteSchemaEntry
5+
import io.github.jpicklyk.mcptask.current.domain.model.Role
56
import org.slf4j.LoggerFactory
67
import org.yaml.snakeyaml.Yaml
78
import java.io.FileReader
@@ -75,19 +76,31 @@ class YamlNoteSchemaService(
7576
val key = raw["key"] as? String ?: return null
7677
val role = raw["role"] as? String ?: return null
7778

78-
if (role !in VALID_SCHEMA_ROLES) {
79-
logger.warn("Skipping schema entry '{}': invalid role '{}' (valid: {})", key, role, VALID_SCHEMA_ROLES)
79+
val parsedRole = VALID_SCHEMA_ROLES[role]
80+
if (parsedRole == null) {
81+
logger.warn("Skipping schema entry '{}': invalid role '{}' (valid: {})", key, role, VALID_SCHEMA_ROLES.keys)
8082
return null
8183
}
8284

8385
val required = raw["required"] as? Boolean ?: false
8486
val description = raw["description"] as? String ?: ""
8587
val guidance = raw["guidance"] as? String
86-
return NoteSchemaEntry(key = key, role = role, required = required, description = description, guidance = guidance)
88+
return NoteSchemaEntry(
89+
key = key,
90+
role = parsedRole,
91+
required = required,
92+
description = description,
93+
guidance = guidance,
94+
)
8795
}
8896

8997
companion object {
90-
private val VALID_SCHEMA_ROLES = setOf("queue", "work", "review")
98+
private val VALID_SCHEMA_ROLES =
99+
mapOf(
100+
"queue" to Role.QUEUE,
101+
"work" to Role.WORK,
102+
"review" to Role.REVIEW,
103+
)
91104

92105
fun resolveDefaultConfigPath(): java.nio.file.Path {
93106
val projectRoot =

‎current/src/test/kotlin/io/github/jpicklyk/mcptask/current/application/service/PhaseNoteContextTest.kt‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class PhaseNoteContextTest {
2323
fun `returns null for terminal items`() {
2424
val schema =
2525
listOf(
26-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Write spec")
26+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Write spec")
2727
)
2828
val result = computePhaseNoteContext(Role.TERMINAL, schema, emptyMap())
2929
assertNull(result)
@@ -39,8 +39,8 @@ class PhaseNoteContextTest {
3939
fun `computes correct counts with no notes filled`() {
4040
val schema =
4141
listOf(
42-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Write spec"),
43-
NoteSchemaEntry(key = "design", role = "queue", required = true, guidance = "Write design")
42+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Write spec"),
43+
NoteSchemaEntry(key = "design", role = Role.QUEUE, required = true, guidance = "Write design")
4444
)
4545
val result = computePhaseNoteContext(Role.QUEUE, schema, emptyMap())
4646

@@ -56,9 +56,9 @@ class PhaseNoteContextTest {
5656
fun `computes correct counts with some notes filled`() {
5757
val schema =
5858
listOf(
59-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Write spec"),
60-
NoteSchemaEntry(key = "design", role = "queue", required = true, guidance = "Write design"),
61-
NoteSchemaEntry(key = "risks", role = "queue", required = true, guidance = "List risks")
59+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Write spec"),
60+
NoteSchemaEntry(key = "design", role = Role.QUEUE, required = true, guidance = "Write design"),
61+
NoteSchemaEntry(key = "risks", role = Role.QUEUE, required = true, guidance = "List risks")
6262
)
6363
val notesByKey = mapOf("spec" to note("spec"))
6464
val result = computePhaseNoteContext(Role.QUEUE, schema, notesByKey)
@@ -75,7 +75,7 @@ class PhaseNoteContextTest {
7575
fun `all notes filled gives null guidancePointer`() {
7676
val schema =
7777
listOf(
78-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Write spec")
78+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Write spec")
7979
)
8080
val notesByKey = mapOf("spec" to note("spec"))
8181
val result = computePhaseNoteContext(Role.QUEUE, schema, notesByKey)
@@ -92,7 +92,7 @@ class PhaseNoteContextTest {
9292
fun `blank body does not count as filled`() {
9393
val schema =
9494
listOf(
95-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Write spec")
95+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Write spec")
9696
)
9797
val notesByKey = mapOf("spec" to note("spec", body = ""))
9898
val result = computePhaseNoteContext(Role.QUEUE, schema, notesByKey)
@@ -107,8 +107,8 @@ class PhaseNoteContextTest {
107107
fun `only considers notes for the current role`() {
108108
val schema =
109109
listOf(
110-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Queue guidance"),
111-
NoteSchemaEntry(key = "impl", role = "work", required = true, guidance = "Work guidance")
110+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Queue guidance"),
111+
NoteSchemaEntry(key = "impl", role = Role.WORK, required = true, guidance = "Work guidance")
112112
)
113113
// Item is in WORK phase — should only consider work-role notes
114114
val result = computePhaseNoteContext(Role.WORK, schema, emptyMap())
@@ -125,8 +125,8 @@ class PhaseNoteContextTest {
125125
fun `non-required notes are excluded from counts`() {
126126
val schema =
127127
listOf(
128-
NoteSchemaEntry(key = "spec", role = "queue", required = true, guidance = "Required"),
129-
NoteSchemaEntry(key = "optional-notes", role = "queue", required = false, guidance = "Optional")
128+
NoteSchemaEntry(key = "spec", role = Role.QUEUE, required = true, guidance = "Required"),
129+
NoteSchemaEntry(key = "optional-notes", role = Role.QUEUE, required = false, guidance = "Optional")
130130
)
131131
val result = computePhaseNoteContext(Role.QUEUE, schema, emptyMap())
132132

‎current/src/test/kotlin/io/github/jpicklyk/mcptask/current/application/tools/compound/CompleteTreeToolTest.kt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class CompleteTreeToolTest {
179179
listOf(
180180
NoteSchemaEntry(
181181
key = "acceptance-criteria",
182-
role = "queue",
182+
role = Role.QUEUE,
183183
required = true,
184184
description = "Acceptance criteria"
185185
)
@@ -243,7 +243,7 @@ class CompleteTreeToolTest {
243243
listOf(
244244
NoteSchemaEntry(
245245
key = "acceptance-criteria",
246-
role = "queue",
246+
role = Role.QUEUE,
247247
required = true,
248248
description = "Acceptance criteria"
249249
)
@@ -673,7 +673,7 @@ class CompleteTreeToolTest {
673673
listOf(
674674
NoteSchemaEntry(
675675
key = "acceptance-criteria",
676-
role = "queue",
676+
role = Role.QUEUE,
677677
required = true,
678678
description = "Acceptance criteria"
679679
)
@@ -751,7 +751,7 @@ class CompleteTreeToolTest {
751751
listOf(
752752
NoteSchemaEntry(
753753
key = "acceptance-criteria",
754-
role = "queue",
754+
role = Role.QUEUE,
755755
required = true,
756756
description = "Acceptance criteria"
757757
)
@@ -811,7 +811,7 @@ class CompleteTreeToolTest {
811811
listOf(
812812
NoteSchemaEntry(
813813
key = "acceptance-criteria",
814-
role = "queue",
814+
role = Role.QUEUE,
815815
required = true,
816816
description = "Acceptance criteria"
817817
)
@@ -875,7 +875,7 @@ class CompleteTreeToolTest {
875875
listOf(
876876
NoteSchemaEntry(
877877
key = "acceptance-criteria",
878-
role = "queue",
878+
role = Role.QUEUE,
879879
required = true,
880880
description = "AC"
881881
)

0 commit comments

Comments
 (0)