diff --git a/AGENTS.md b/AGENTS.md index c01b2a5a..867d3410 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -365,6 +365,7 @@ Patterns and conventions observed in this codebase that aren't covered by the ge - **Config field → StartOptions override chain** — `ClaudeBackendConfig` stores persistent defaults; `StartOptions` provides per-invocation overrides. In `BuildStartCommand`, each flag uses a priority chain: `StartOptions` value > `ClaudeBackend` value > no flag. See `firstNonEmpty`/`firstPositive`/`mergeUnique` helpers in `internal/ai/backend.go`. This enables role-specific behavior (e.g., `PermissionMode: "plan"` for reviewers). - **Per-role factory creation in bridgewire** — `PipelineExecutor.attachBridges` creates a *per-team* `instanceFactory` when `RoleOverrides` contains an entry for the team's role. The factory carries `ai.StartOptions` that flow through `Orchestrator.StartInstanceWithOverrides → newInstanceManager → ManagerOptions.StartOverrides → Manager.Start()`. The default shared factory is used for teams without role overrides. - **Capture loop recovery pattern** — `Manager.captureLoop()` detects tmux server death at four distinct points (heartbeat check, session status query, unresponsive threshold, capture failure). All four sites call `attemptSessionRecovery()` before `handleSessionEnded()`. Recovery creates a fresh tmux session and resumes the Claude session via `--resume`. The persistent input handler auto-reconnects to the new session (same socket name) without explicit re-initialization. +- **Navigation must follow visual display order** — The ultraplan sidebar is rendered via `FlattenGroupsForDisplay` (group-structure order), but navigation used to use `getNavigableInstances` (plan-execution order). These orderings diverge because instances are added to groups in creation order, not plan order. Any keyboard navigation that moves between sidebar items must use `getInstanceDisplayOrder()` as its ordering source, filtered to the set of navigable items, to stay consistent with what the user sees. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 23945df8..aa24600f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Subprocess Mode** - Removed the experimental subprocess execution mode (`experimental.subprocess_mode`), including the `internal/streamjson/` package, `subprocessFactory`, and all related config/TUI/wiring plumbing. Pipeline instances now always use the tmux-based execution backend. ### Fixed +- **Ultraplan h/l Navigation Reversed** - Fixed `h` and `l` keybindings navigating in the opposite visual direction in ultraplan mode with groups. Navigation used plan-execution order (`getNavigableInstances`) while the sidebar rendered in group-structure order (`FlattenGroupsForDisplay`), causing the two orderings to diverge. Navigation now follows the visual display order filtered to navigable instances. +- **Silent Plan Validation Failure** - Fixed `handlePlanFileCheckResult` silently swallowing `SetPlan` errors, leaving users stuck in a session that would never progress. Now sets an error message and transitions to `PhaseFailed`, matching the identical error handling in `handlePlanParsed`. - **Missing Sentinel File in Pipeline Execution** - Fixed task instances not writing `.claudio-task-complete.json` in the Orchestration 2.0 pipeline path. The bridge's `BuildTaskPrompt` relied solely on `--append-system-prompt-file` to inject the completion protocol, which left instances unaware of the sentinel file convention. The completion protocol is now embedded directly in the task prompt as defense-in-depth. - **Pipeline Task Grouping in Sidebar** - Fixed execution task instances appearing as ungrouped in the sidebar when using the pipeline execution path (Orchestration 2.0, the default since #659). The bridge's `SessionRecorder` was created with nil callbacks, so newly created instances were never added to the ultraplan's `InstanceGroup`. Wired the `OnAssign` callback to `Coordinator.AssignTaskInstance`, which populates `TaskToInstance` before routing the instance to the correct "Group N" subgroup. Also fixed a latent ordering bug in the legacy `ExecutionOrchestrator` path where `AddInstanceToGroup` was called before `AssignTaskToInstance`, causing instances to fall through to `SubgroupTypeUnknown`. - **Stale Display Early in Session** - Fixed capture loop never populating the output buffer when there's no scrollback (screen not yet full). A single `lastOutput` tracking variable was shared between visible-only and full captures; when a visible capture detected new content and set `lastOutput`, the subsequent forced full capture (which returns identical bytes when there's no scrollback) found no change and skipped the buffer write. Split into independent `lastVisibleOutput` and `lastFullOutput` so each capture type compares against its own history. diff --git a/internal/tui/ultraplan.go b/internal/tui/ultraplan.go index 05400fa2..54ffcb7f 100644 --- a/internal/tui/ultraplan.go +++ b/internal/tui/ultraplan.go @@ -109,9 +109,10 @@ func (m Model) isInstanceSelected(instanceID string) bool { return false } -// getNavigableInstances returns an ordered list of instance IDs that can be navigated to. +// getNavigableInstances returns the set of instance IDs that can be navigated to. // Only includes instances from phases that have started or completed. -// Order: Planning → Execution tasks (in order) → Synthesis → Consolidation +// Note: The ordering of the returned slice is not used for navigation — navigation +// order comes from getInstanceDisplayOrder(). Only set membership matters here. func (m *Model) getNavigableInstances() []string { if m.ultraPlan == nil || m.ultraPlan.Coordinator == nil { return nil @@ -299,9 +300,22 @@ func (m *Model) navigateToNextInstance(direction int) bool { return false } - // Update the navigable instances list + // Build the navigable set from ultraplan-specific logic m.updateNavigableInstances() - instances := m.ultraPlan.NavigableInstances + navigableSet := make(map[string]bool, len(m.ultraPlan.NavigableInstances)) + for _, id := range m.ultraPlan.NavigableInstances { + navigableSet[id] = true + } + + // Use the display order (matches visual sidebar rendering) filtered to navigable instances. + // This ensures h/l navigation direction matches the visual top-to-bottom order. + displayOrder := m.getInstanceDisplayOrder() + var instances []string + for _, id := range displayOrder { + if navigableSet[id] { + instances = append(instances, id) + } + } if len(instances) == 0 { return false @@ -341,7 +355,6 @@ func (m *Model) navigateToNextInstance(direction int) bool { m.pauseInstance(oldInst.ID) } m.activeTab = i - m.ultraPlan.SelectedNavIdx = nextIdx m.ensureActiveVisible() // Resume the new active instance's capture m.resumeActiveInstance() @@ -1050,6 +1063,8 @@ func (m *Model) handlePlanFileCheckResult(msg tuimsg.PlanFileCheckResultMsg) (te // Set the plan if err := m.ultraPlan.Coordinator.SetPlan(msg.Plan); err != nil { + m.errorMessage = fmt.Sprintf("Plan file check completed but plan is invalid: %v", err) + m.ultraPlan.Coordinator.Manager().SetPhase(orchestrator.PhaseFailed) return m, nil } diff --git a/internal/tui/view/ultraplan/context.go b/internal/tui/view/ultraplan/context.go index 9aee696a..87fa271a 100644 --- a/internal/tui/view/ultraplan/context.go +++ b/internal/tui/view/ultraplan/context.go @@ -41,7 +41,6 @@ type State struct { // Phase-aware navigation state NavigableInstances []string // Ordered list of navigable instance IDs - SelectedNavIdx int // Index into navigableInstances // Group re-trigger mode RetriggerMode bool // When true, next digit key triggers group re-trigger