feat(sdd): TUI model picker integration for JD agents#477
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the SDD TUI model-configuration flows to incorporate Judgment Day (JD) agents, including UI row separation/skip behavior, profile isolation (JD excluded from profile-scoped pickers), and preset/policy updates so JD agents can be configured and permitted consistently.
Changes:
- Adds JD rows + a non-actionable separator to the model picker UI, plus cursor-navigation logic to skip the separator.
- Introduces profile-scoped row filtering (
ModelPickerRowsForProfile) so JD agents are excluded during profile create/edit. - Expands SDD profile permissions / model-assignment reading logic to treat JD agents as configurable/global, and updates related tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tui/screens/profile_create.go | Uses profile-filtered model picker rows for correct option counts during profile creation. |
| internal/tui/screens/model_picker.go | Adds JD rows + separator, profile filtering, separator index helper, and JD assignment mapping/rendering. |
| internal/tui/screens/model_picker_test.go | Adds tests for separator no-op, JD row mapping, and profile row filtering. |
| internal/tui/screens/claude_model_picker.go | Adds a Diversity preset and includes JD phases/labels in custom assignments list. |
| internal/tui/model.go | Skips the separator during cursor navigation and blocks selecting it. Marks picker state as profile-scoped. |
| internal/components/sdd/read_assignments.go | Broadens assignment reading to “configurable agents” (intended: SDD + JD + orchestrator) via a set. |
| internal/components/sdd/profiles.go | Reserves JD agent names for profiles and grants JD permissions globally in generated overlays. |
| internal/components/sdd/profiles_test.go | Adjusts expected permissions behavior depending on single vs multi overlays and JD inclusion. |
| internal/components/sdd/inject_test.go | Updates expected agent count for multi overlay to include JD agents. |
Comments suppressed due to low confidence (2)
internal/tui/screens/model_picker.go:263
SeparatorRowIdx()can return -1 when there are no JD phases, but theswitchusesstate.SelectedPhaseIdx > separatorIdxwhich becomes true for all rows whenseparatorIdx == -1. That causes SDD sub-agent selections (idx >= 2) to be treated as JD rows and no assignment will be applied. Add a guard so the JD-row branch only runs whenseparatorIdx >= 0(and similarly treat the separator case only when present).
phases := opencode.SDDPhases()
jdPhases := opencode.JDPhases()
separatorIdx := SeparatorRowIdx()
switch {
case state.SelectedPhaseIdx == 0:
// "gentle-orchestrator" row — assign only to the base orchestrator key
assignments[SDDOrchestratorPhase] = assignment
case state.SelectedPhaseIdx == 1:
// "Set all phases" — sets only the 9 sub-agents, NOT the orchestrator.
// Also update AllPhasesModel so the label stays in sync with the last
// "Set all" action (Issue #146: individual phase selections must NOT touch this).
for _, phase := range phases {
assignments[phase] = assignment
}
state.AllPhasesModel = assignment
case state.SelectedPhaseIdx == separatorIdx:
// Separator row ("--- Judgment Day ---") — no action, skip.
case state.SelectedPhaseIdx > separatorIdx:
// JD agent rows: map to JDPhases() after separator.
jdIdx := state.SelectedPhaseIdx - separatorIdx - 1
if jdIdx < len(jdPhases) {
assignments[jdPhases[jdIdx]] = assignment
}
default:
// SDD sub-agent rows start at idx 2; phases[idx-2] is the correct phase.
// Individual selection intentionally does NOT update AllPhasesModel (Issue #146).
phaseIdx := state.SelectedPhaseIdx - 2
if phaseIdx < len(phases) {
assignments[phases[phaseIdx]] = assignment
}
}
internal/components/sdd/read_assignments.go:25
opencode.ConfigurableAgentPhases()is called here, but the currentinternal/opencodepackage in this repo does not define it. This makes the build fail. Either add/merge that API (and its tests) or fall back to composing from existing phase lists (e.g., SDD + JD) within this package.
// configurableAgentSet is the set of valid agent names that may appear in
// opencode.json. It includes SDD phases, JD agents, and the gentle-orchestrator coordinator.
var configurableAgentSet = buildConfigurableAgentSet()
func buildConfigurableAgentSet() map[string]bool {
phases := opencode.ConfigurableAgentPhases() // SDD + JD phases
set := make(map[string]bool, len(phases)+1)
for _, p := range phases {
set[p] = true
}
set["gentle-orchestrator"] = true
// Backward-compatible read alias for configs that have not been synced yet.
set["sdd-orchestrator"] = true
return set
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func ModelPickerRows() []string { | ||
| rows := make([]string, 0, 11) | ||
| rows := make([]string, 0, 2+len(opencode.SDDPhases())+1+len(opencode.JDPhases())) | ||
| rows = append(rows, SDDOrchestratorPhase) | ||
| rows = append(rows, "Set all SDD phases") | ||
| rows = append(rows, opencode.SDDPhases()...) | ||
| if len(opencode.JDPhases()) > 0 { | ||
| rows = append(rows, "--- Judgment Day ---") | ||
| rows = append(rows, opencode.JDPhases()...) | ||
| } |
| var b strings.Builder | ||
|
|
||
| b.WriteString(styles.TitleStyle.Render("Assign Models to SDD Phases")) | ||
| b.WriteString(styles.TitleStyle.Render("Assign Models to SDD Phases & JD Agents")) |
| @@ -306,8 +354,29 @@ func renderPhaseList( | |||
| } else { | |||
| label = fmt.Sprintf("%-20s (not set)", row) | |||
| } | |||
| case idx == separatorIdx: | |||
| // Separator row — render as a visual divider with subtle indicator when focused. | |||
| if focused { | |||
| b.WriteString(styles.SubtextStyle.Render("▸ " + row) + "\n") | |||
| } else { | |||
| b.WriteString(styles.SubtextStyle.Render(" " + row) + "\n") | |||
| } | |||
| continue | |||
| case idx > separatorIdx: | |||
| // JD agent rows | |||
| jdIdx := idx - separatorIdx - 1 | |||
| if jdIdx < len(jdPhases) { | |||
| phase := jdPhases[jdIdx] | |||
| assignment, ok := assignments[phase] | |||
| if ok && assignment.ProviderID != "" { | |||
| provName, modelName := resolveNames(assignment, state) | |||
| label = fmt.Sprintf("%-20s %s / %s", row, provName, modelName) | |||
| } else { | |||
| label = fmt.Sprintf("%-20s (default)", row) | |||
| } | |||
| } | |||
There was a problem hiding this comment.
When JDPhases() is empty, ModelPickerRows() does not add the separator or any JD rows, so the switch never encounters separatorIdx=-1. Guarded by len(JDPhases()) > 0 in ModelPickerRows().
There was a problem hiding this comment.
Guarded by len(JDPhases()) > 0 in ModelPickerRows() — when empty, no separator or JD rows are added.
| // reservedProfileNames are names that may not be used as profile names. | ||
| var reservedProfileNames = map[string]bool{ | ||
| "default": true, | ||
| "sdd-orchestrator": true, | ||
| } | ||
| // JD agent names are derived from opencode.JDPhases() to avoid drift | ||
| // when agents are renamed or added. | ||
| var reservedProfileNames = func() map[string]bool { | ||
| names := map[string]bool{ | ||
| "default": true, | ||
| "sdd-orchestrator": true, | ||
| } | ||
| for _, name := range opencode.JDPhases() { | ||
| names[name] = true | ||
| } | ||
| return names |
| ClaudePresetBalanced: model.ClaudeModelPresetBalanced, | ||
| ClaudePresetPerformance: model.ClaudeModelPresetPerformance, | ||
| ClaudePresetEconomy: model.ClaudeModelPresetEconomy, | ||
| ClaudePresetDiversity: model.ClaudeModelPresetDiversity, |
There was a problem hiding this comment.
Stacked PR — ClaudeModelPresetDiversity is defined in PR #475. Compiles when chain is applied in order.
There was a problem hiding this comment.
Stacked PR — ClaudeModelPresetDiversity is defined in PR #475. Compiles when chain is applied in order.
| // Multi overlay must contain gentle-orchestrator + 10 sub-agents + 3 JD agents = 14 agents. | ||
| if len(agentMap) != 14 { | ||
| t.Fatalf("agent count = %d, want 14", len(agentMap)) |
There was a problem hiding this comment.
Stacked PR — the overlay is updated in PR #475 to include JD agents. Agent count matches when chain is applied.
There was a problem hiding this comment.
Stacked PR — overlay updated in PR #475 to include JD agents. Agent count matches when chain is applied.
Alan-TheGentleman
left a comment
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
This final slice needs to follow the collaboration guide and prove the whole chain works for both OpenCode and Claude Code.
Please add the missing type:feature label, document the final end-to-end support matrix, and include verification that the TUI/profile behavior does not only work for OpenCode while leaving Claude Code JD agents unconfigured or untestable.
|
@Alan-TheGentleman Could you add the |
|
Added the type:feature label. Thanks for flagging the permission issue. |
| @@ -230,8 +263,16 @@ func handleModelNav( | |||
| assignments[phase] = assignment | |||
| } | |||
| state.AllPhasesModel = assignment | |||
| case state.SelectedPhaseIdx == separatorIdx: | |||
| // Separator row ("--- Judgment Day ---") — no action, skip. | |||
| case state.SelectedPhaseIdx > separatorIdx: | |||
| // JD agent rows: map to JDPhases() after separator. | |||
| jdIdx := state.SelectedPhaseIdx - separatorIdx - 1 | |||
| if jdIdx < len(jdPhases) { | |||
| assignments[jdPhases[jdIdx]] = assignment | |||
| } | |||
| default: | |||
There was a problem hiding this comment.
When JDPhases() is empty, ModelPickerRows() does not add the separator or any JD rows, so the switch never encounters separatorIdx=-1 in practice. The guard len(JDPhases()) > 0 in ModelPickerRows() ensures this.
| case idx == separatorIdx: | ||
| // Separator row — render as a visual divider with subtle indicator when focused. | ||
| if focused { | ||
| b.WriteString(styles.SubtextStyle.Render("▸ " + row) + "\n") | ||
| } else { | ||
| b.WriteString(styles.SubtextStyle.Render(" " + row) + "\n") | ||
| } | ||
| continue | ||
| case idx > separatorIdx: | ||
| // JD agent rows | ||
| jdIdx := idx - separatorIdx - 1 | ||
| if jdIdx < len(jdPhases) { | ||
| phase := jdPhases[jdIdx] | ||
| assignment, ok := assignments[phase] | ||
| if ok && assignment.ProviderID != "" { | ||
| provName, modelName := resolveNames(assignment, state) | ||
| label = fmt.Sprintf("%-20s %s / %s", row, provName, modelName) | ||
| } else { | ||
| label = fmt.Sprintf("%-20s (default)", row) | ||
| } | ||
| } | ||
| default: |
There was a problem hiding this comment.
Same as above — when JDPhases() is empty, no separator is added to rows and the render switch never enters the JD branch with separatorIdx=-1.
| func ModelPickerRows() []string { | ||
| rows := make([]string, 0, 11) | ||
| rows := make([]string, 0, 2+len(opencode.SDDPhases())+1+len(opencode.JDPhases())) | ||
| rows = append(rows, SDDOrchestratorPhase) | ||
| rows = append(rows, "Set all SDD phases") | ||
| rows = append(rows, opencode.SDDPhases()...) | ||
| if len(opencode.JDPhases()) > 0 { | ||
| rows = append(rows, "--- Judgment Day ---") | ||
| rows = append(rows, opencode.JDPhases()...) | ||
| } | ||
| return rows |
There was a problem hiding this comment.
This PR is the third slice in a stacked chain. JDPhases() is defined in PR #475 (foundation) which is the base for this branch. The code compiles when the chain is applied in order.
| var b strings.Builder | ||
|
|
||
| b.WriteString(styles.TitleStyle.Render("Assign Models to SDD Phases")) | ||
| b.WriteString(styles.TitleStyle.Render("Assign Models to SDD Phases & JD Agents")) |
There was a problem hiding this comment.
Valid point, but low priority — ForProfile mode uses ModelPickerRowsForProfile() which excludes JD rows, so the title is technically wrong but doesn't mislead functionally. Noted for a future polish pass.
| // configurableAgentSet is the set of valid agent names that may appear in | ||
| // opencode.json. It includes SDD phases, JD agents, and the gentle-orchestrator coordinator. | ||
| var configurableAgentSet = buildConfigurableAgentSet() | ||
|
|
||
| func buildSDDPhaseSet() map[string]bool { | ||
| phases := opencode.SDDPhases() | ||
| func buildConfigurableAgentSet() map[string]bool { | ||
| phases := opencode.ConfigurableAgentPhases() // SDD + JD phases | ||
| set := make(map[string]bool, len(phases)+1) | ||
| for _, p := range phases { | ||
| set[p] = true |
There was a problem hiding this comment.
Stacked PR — ConfigurableAgentPhases is defined in PR #475. Compiles when chain is applied in order.
| // presetConstructors maps preset IDs to their constructor functions. | ||
| var presetConstructors = map[ClaudeModelPreset]func() map[string]model.ClaudeModelAlias{ | ||
| ClaudePresetBalanced: model.ClaudeModelPresetBalanced, | ||
| ClaudePresetPerformance: model.ClaudeModelPresetPerformance, | ||
| ClaudePresetEconomy: model.ClaudeModelPresetEconomy, | ||
| ClaudePresetDiversity: model.ClaudeModelPresetDiversity, | ||
| } |
There was a problem hiding this comment.
Stacked PR — ClaudeModelPresetDiversity is defined in PR #475. Compiles when chain is applied in order.
| // reservedProfileNames are names that may not be used as profile names. | ||
| var reservedProfileNames = map[string]bool{ | ||
| "default": true, | ||
| "sdd-orchestrator": true, | ||
| } | ||
| // JD agent names are derived from opencode.JDPhases() to avoid drift | ||
| // when agents are renamed or added. | ||
| var reservedProfileNames = func() map[string]bool { | ||
| names := map[string]bool{ | ||
| "default": true, | ||
| "sdd-orchestrator": true, | ||
| } | ||
| for _, name := range opencode.JDPhases() { | ||
| names[name] = true | ||
| } | ||
| return names |
There was a problem hiding this comment.
Stacked PR — JDPhases() is defined in PR #475 (foundation). Compiles when chain is applied in order.
|
This is close enough to keep as one review unit. The PR is 425 changed lines, just over the 400-line budget, and the extra diff is mostly focused test/UI integration coverage. Please add or request the |
|
@Alan-TheGentleman Rebased all 3 PRs onto latest main — conflicts resolved, all MERGEABLE now. CI should pick up the checks. Regarding the 425-line budget: requesting |
| "sdd-verify": "Verify", | ||
| "sdd-archive": "Archive", | ||
| "default": "General delegation", | ||
| "orchestrator": "Orchestrator", |
There was a problem hiding this comment.
Fixed in e734e9d — updated docstring and removed the unused orchestrator entry from claudePhaseLabels.
| "default", | ||
| } | ||
|
|
||
| // claudePhaseLabels are the human-readable labels for each SDD phase. |
There was a problem hiding this comment.
Removed in e734e9d — the orchestrator entry was dead since claudePhases never includes it.
| // If wrap-around landed on the separator row, skip past it. | ||
| if m.Screen == ScreenModelPicker && !m.ModelPicker.ForProfile && m.Cursor == screens.SeparatorRowIdx() && m.Cursor > 0 { | ||
| m.Cursor-- | ||
| } |
There was a problem hiding this comment.
Removed in e734e9d — the separator can never be at count-1 (always followed by JD rows), so the wrap-around check was unreachable dead code.
| @@ -1012,6 +1020,12 @@ func (m Model) handleKeyPress(key tea.KeyMsg) (tea.Model, tea.Cmd) { | |||
| m.BackupScroll = m.Cursor - screens.BackupMaxVisible + 1 | |||
| } | |||
| } | |||
| // Skip separator row in model picker — it is not selectable. | |||
| if m.Screen == ScreenModelPicker && !m.ModelPicker.ForProfile && m.Cursor == screens.SeparatorRowIdx() { | |||
| if m.Cursor+1 < count { | |||
| m.Cursor++ | |||
| } | |||
| } | |||
| return m, nil | |||
| case "k": | |||
| count := m.optionCount() | |||
| @@ -1021,6 +1035,10 @@ func (m Model) handleKeyPress(key tea.KeyMsg) (tea.Model, tea.Cmd) { | |||
| } else if !m.isScrollableScreen() { | |||
| // Issue #150: wrap-around — Up at 0 goes to last option. | |||
| m.Cursor = count - 1 | |||
| // If wrap-around landed on the separator row, skip past it. | |||
| if m.Screen == ScreenModelPicker && !m.ModelPicker.ForProfile && m.Cursor == screens.SeparatorRowIdx() && m.Cursor > 0 { | |||
| m.Cursor-- | |||
| } | |||
| } | |||
| } | |||
| // Adjust scroll for the backup list. | |||
| @@ -1029,6 +1047,10 @@ func (m Model) handleKeyPress(key tea.KeyMsg) (tea.Model, tea.Cmd) { | |||
| m.BackupScroll = m.Cursor | |||
| } | |||
| } | |||
| // Skip separator row in model picker — it is not selectable. | |||
| if m.Screen == ScreenModelPicker && !m.ModelPicker.ForProfile && m.Cursor == screens.SeparatorRowIdx() && m.Cursor > 0 { | |||
| m.Cursor-- | |||
| } | |||
| return m, nil | |||
| case "j": | |||
| count := m.optionCount() | |||
| @@ -1044,6 +1066,12 @@ func (m Model) handleKeyPress(key tea.KeyMsg) (tea.Model, tea.Cmd) { | |||
| m.BackupScroll = m.Cursor - screens.BackupMaxVisible + 1 | |||
| } | |||
| } | |||
| // Skip separator row in model picker — it is not selectable. | |||
| if m.Screen == ScreenModelPicker && !m.ModelPicker.ForProfile && m.Cursor == screens.SeparatorRowIdx() { | |||
| if m.Cursor+1 < count { | |||
| m.Cursor++ | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Noted — good suggestion for a future refactor. Keeping as-is for now to minimize diff.
| label = fmt.Sprintf("%-20s %s / %s", row, provName, modelName) | ||
| } else { | ||
| label = fmt.Sprintf("%-20s (default)", row) | ||
| } |
There was a problem hiding this comment.
Fixed in e734e9d — title is now conditional on state.ForProfile: shows 'SDD Phases & JD Agents' normally, 'SDD Phases' in profile mode.
There was a problem hiding this comment.
Theoretical — jdIdx >= len(jdPhases) can't happen since ModelPickerRows() and SeparatorRowIdx() are derived from the same opencode.JDPhases() source.
There was a problem hiding this comment.
Fixed in e734e9d — title now conditional on state.ForProfile.
| var b strings.Builder | ||
|
|
||
| b.WriteString(styles.TitleStyle.Render("Assign Models to SDD Phases")) | ||
| b.WriteString(styles.TitleStyle.Render("Assign Models to SDD Phases & JD Agents")) |
There was a problem hiding this comment.
Theoretical — jdIdx >= len(jdPhases) can't happen since ModelPickerRows() and SeparatorRowIdx() are derived from the same opencode.JDPhases() source. Added as a known theoretical edge case.
There was a problem hiding this comment.
Theoretical — jdIdx >= len(jdPhases) can't happen since ModelPickerRows() and SeparatorRowIdx() are derived from the same opencode.JDPhases() source.
|
@Alan-TheGentleman Noted on all points:
|
Alan-TheGentleman
added
type:feature
|
@danielgap — heads-up on this PR (and the rest of the JD chain #475 / #476 / #477): Unit Tests are failing on:
These are golden-file tests over the OpenCode SDD overlay. Since the JD agent definitions modify the generated overlay content, the golden fixtures need to be regenerated. Suggested fix:
Once green, I'll review the chain (PR 1 → 2 → 3 in order) for merge. Thanks for sticking with this — the JD chain is great work and I want to land it. |
Add jd-judge-a, jd-judge-b, and jd-fix-agent as hidden sub-agents in the multi-mode overlay. Update orchestrator permissions to allow delegation to these agents. Refs: Gentleman-Programming#208
JDPhases returns the judgment-day sub-agent names (jd-judge-a, jd-judge-b, jd-fix-agent). ConfigurableAgentPhases combines SDD phases and JD agents for the TUI model picker. Refs: Gentleman-Programming#208
Add jd-judge-a, jd-judge-b, jd-fix-agent to claudeModelAssignmentRowOrder and claudeModelAssignmentReasons in inject.go. Add JD entries to all three existing presets (balanced, performance, economy) in claude_model.go. Add new ClaudeModelPresetDiversity() preset for perspective diversity between judges (opus/haiku/sonnet split). Refs: Gentleman-Programming#208
Add isJDAgent helper and modify injectModelAssignments case 3 to skip root model fallback for judgment-day agents. This preserves independent model configuration for diversity of perspective between judges. Refs: Gentleman-Programming#208
…DPhases separately
- ClaudeModelPresetDiversity now clones the base map before mutating, preventing corruption if ClaudeModelPresetBalanced is ever cached. - isJDAgent uses a package-level set instead of linear scan, consistent with sddPhaseSet pattern in read_assignments.go.
OpenCode overlays only use {read, write, edit, bash} as tool keys.
glob and grep are not referenced anywhere in the codebase and may cause
config rejection. Judges only need read and bash for code review.
- Regenerate golden file (sdd-opencode-multi-settings.golden) to include jd-judge-a, jd-judge-b, and jd-fix-agent entries - Update TestInjectOpenCodeMultiMode agent count: 11 → 14 (gentle-orchestrator + 10 SDD phases + 3 JD agents) - Update TestDefaultOverlayTaskPermissions_ExplicitAllowlist to expect JD agent permissions in the multi overlay only - Update TUI model picker tests to expect orchestrator in balanced preset (now a configurable assignment in the model table)
Pattern 1 now documents both OpenCode named-agent delegation (jd-judge-a, jd-judge-b, jd-fix-agent) and generic delegate() for agents without named sub-agent support. Pattern 4 and Rules section updated to reference both delegation modes.
Add jd-judge-a, jd-judge-b, and jd-fix-agent to sddPhaseAgents so they are properly cleaned up during uninstall.
…iro judge tools to read-only
Copilot flagged that delegate(prompt='...') without agent contradicts OpenCode's delegate schema. Make explicit that the generic syntax is for adapters that don't support named agents, and the model is controlled by adapter-native mechanisms (e.g. model sentinels).
Instead of maintaining a manually duplicated list that can go stale when new phases are added, derive sddSkillPhaseIDs by filtering sddPhaseAgents for entries with the sdd- prefix excluding the orchestrator. JD agents (jd-*) are automatically excluded. Addresses Copilot review: keeps both lists in sync without manual maintenance.
The slice now contains both SDD phase agents and JD sub-agents, so the old name was misleading. configuredAgents better reflects that it lists all agents the uninstaller should clean up from opencode.json. Addresses Copilot review.
The overlay now uses gentle-orchestrator (renamed from sdd-orchestrator). Keep sdd-orchestrator for backward-compat cleanup of old user configs. gentle-orchestrator is correctly excluded from sddSkillPhaseIDs by the sdd- prefix filter. Addresses Copilot review.
Update TestClaudeEmbeddedAssetLayout expected agent count from 10 to 13 to account for jd-judge-a.md, jd-judge-b.md, and jd-fix-agent.md.
…filtering, diversity preset
… separator, tests
…ratorRowIdx, add profile tests, update title
When navigating UP from index 0, wrap-around sets cursor to count-1. If that row is the separator, the existing skip guard at the bottom of the handler would run once and decrement — but this relied on a single guard pass. The fix adds an explicit re-check immediately after the wrap-around assignment in both 'up' and 'k' handlers, ensuring the separator is never selected regardless of layout changes.
Adds TestInjectOpenCodeMultiModeJDAgentsExcludedFromRootModel that asserts JD agents (jd-judge-a, jd-judge-b, jd-fix-agent) do NOT receive the root model when no explicit assignment exists, while SDD agents still do. This prevents regressions in the diversity guarantee.
…date docs - model_picker.go: title now conditional on ForProfile — shows 'SDD Phases & JD Agents' normally, 'SDD Phases' in profile mode - model.go: remove unreachable wrap-around separator check (separator is never at count-1 position) - claude_model_picker.go: update claudePhaseLabels docstring to include JD agents, remove unused 'orchestrator' entry
- Update TestRenderModelPickerShowsSetAllPhasesEffort to search for 'Set all SDD phases' (renamed from 'Set all phases') - Fix ProfileCreate tests to use ModelPickerRowsForProfile() instead of ModelPickerRows() — profile creation excludes JD agents
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 41 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
internal/tui/screens/model_picker.go:1
SeparatorRowIdx()explicitly returns-1when there are no JD phases, but the switch usesstate.SelectedPhaseIdx > separatorIdxunconditionally. IfseparatorIdx == -1, all selectable rows with idx >= 2 will match the “JD agent rows” branch and result in no assignment (sincelen(jdPhases)==0), breaking SDD phase selection. Fix by guarding the separator/JD branches onseparatorIdx >= 0(e.g.,case separatorIdx >= 0 && state.SelectedPhaseIdx == separatorIdxandcase separatorIdx >= 0 && state.SelectedPhaseIdx > separatorIdx). The same guard should also be applied inapplyAssignment()and in phase-list rendering whereidx > separatorIdxis used.
internal/components/sdd/read_assignments.go:1- This change removes the previously documented backward-compat behavior where legacy
sdd-orchestratoragent entries were read/mapped asgentle-orchestrator(so existing configs could be loaded before a sync migrates them). Since uninstall logic still references the legacy key, it’s likely some users still have it. Consider adding"sdd-orchestrator"intoconfigurableAgentSetand mapping it to"gentle-orchestrator"when populatingresult(or keep a small explicit legacy alias map) so reading current assignments remains backward compatible.
internal/components/sdd/read_assignments.go:1 - This change removes the previously documented backward-compat behavior where legacy
sdd-orchestratoragent entries were read/mapped asgentle-orchestrator(so existing configs could be loaded before a sync migrates them). Since uninstall logic still references the legacy key, it’s likely some users still have it. Consider adding"sdd-orchestrator"intoconfigurableAgentSetand mapping it to"gentle-orchestrator"when populatingresult(or keep a small explicit legacy alias map) so reading current assignments remains backward compatible.
| { | ||
| "files": {}, | ||
| "turnCycles": 0, | ||
| "maxCycles": 3, | ||
| "lastUpdated": "2026-05-27T14:48:56.545Z" | ||
| } No newline at end of file |
🔗 Linked Issue
Closes #208
🏷️ PR Type
type:feature— New feature (non-breaking change that adds functionality)📝 Summary
Adds TUI model picker integration for judgment-day agents: JD rows with separator, profile isolation, non-selectable separator, cursor skip navigation, Diversity preset, and comprehensive test coverage.
This is PR 3 of 3 in a chained approach. Depends on #475 (foundation) and #476 (agent assets).
Cross-Agent Contract
End-to-End Support Matrix
All three agents have complete JD model configurability after this chain.
📂 Changes
internal/tui/screens/model_picker.goSeparatorRowIdx(),ModelPickerRowsForProfile(), updated titleinternal/tui/screens/model_picker_test.gointernal/tui/screens/claude_model_picker.gointernal/tui/screens/profile_create.goModelPickerRowsForProfile()to exclude JDinternal/tui/model.goForProfileflaginternal/tui/model.goSeparatorRowIdx()helper, non-selectable separatorinternal/components/sdd/profiles.goreservedProfileNamesfromJDPhases(), JD global permissionsinternal/components/sdd/profiles_test.gointernal/components/sdd/inject_test.gointernal/components/sdd/read_assignments.goconfigurableAgentSet(renamed fromsddPhaseSet)🧪 Test Plan
go test ./...)✅ Contributor Checklist
status:approvedtype:*label to this PRgo test ./...)Co-Authored-BytrailersChain Context
mainChain Overview
Scope
Autonomy