fix(extensions): harden extension state and chrome handling
- commit
- e7a0d1697f961d80f6787e58e25da1bd0414198e
- author
- brice · · 2026-08-04 14:55 UTC
- parent
- 2880a25
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
18 files · +632 -77
M examples/extensions/tasked-phases/main.go +20 -1
| @@ -97,6 +97,7 @@ func main() { | |||
| 97 | 97 | restoreSession := func(event ext.Event) { | |
| 98 | 98 | if err := a.store.activateSession(event.State); err != nil { | |
| 99 | 99 | e.Logf("restore session state: %v", err) | |
| 100 | + | a.publishUnavailableChrome() | |
| 100 | 101 | return | |
| 101 | 102 | } | |
| 102 | 103 | if state, err := a.store.snapshot(); err == nil { | |
| @@ -124,6 +125,7 @@ func main() { | |||
| 124 | 125 | ||
| 125 | 126 | if err := e.Run(); err != nil { | |
| 126 | 127 | fmt.Fprintln(os.Stderr, err) | |
| 128 | + | os.Exit(1) | |
| 127 | 129 | } | |
| 128 | 130 | } | |
| 129 | 131 | ||
| @@ -185,13 +187,30 @@ func (a *app) handleTool(args json.RawMessage) ext.ToolResult { | |||
| 185 | 187 | state, headline, err := a.store.transact(func(current PlanState) (PlanState, string, error) { | |
| 186 | 188 | return applyAction(current, params) | |
| 187 | 189 | }) | |
| 188 | - | a.publishChrome(state) | |
| 190 | + | // A zero state means transact could not obtain a loaded snapshot. Do not | |
| 191 | + | // replace persistent chrome with an invented empty plan. For action and | |
| 192 | + | // persistence errors, transact returns the unchanged usable state, which | |
| 193 | + | // should remain visible to help the user recover. | |
| 194 | + | if err == nil || state.Version != 0 { | |
| 195 | + | a.publishChrome(state) | |
| 196 | + | } | |
| 189 | 197 | if err == nil { | |
| 190 | 198 | a.renderPanelIfOpen(state) | |
| 191 | 199 | } | |
| 192 | 200 | return toolResult(params.Action, state, headline, err) | |
| 193 | 201 | } | |
| 194 | 202 | ||
| 203 | + | func (a *app) publishUnavailableChrome() { | |
| 204 | + | a.ext.SetStatus("progress", "plan unavailable") | |
| 205 | + | a.ext.SetWidget("plan", "above_input", "Tasked phases", []string{"Plan state could not be restored for this session."}) | |
| 206 | + | a.panelMu.Lock() | |
| 207 | + | open := a.panelOpen | |
| 208 | + | a.panelMu.Unlock() | |
| 209 | + | if open { | |
| 210 | + | a.ext.RenderPanel(panelID, "Tasked phases", []string{"Plan state could not be restored for this session."}, "Escape closes this panel") | |
| 211 | + | } | |
| 212 | + | } | |
| 213 | + | ||
| 195 | 214 | func (a *app) publishChrome(state PlanState) { | |
| 196 | 215 | if isPlanClosed(state) { | |
| 197 | 216 | a.ext.SetStatus("progress", "closed: "+state.ClosedSummary) | |
| @@ -97,6 +97,7 @@ func main() { | |||||
| 97 | restoreSession := func(event ext.Event) { | 97 | restoreSession := func(event ext.Event) { | ||
| 98 | if err := a.store.activateSession(event.State); err != nil { | 98 | if err := a.store.activateSession(event.State); err != nil { | ||
| 99 | e.Logf("restore session state: %v", err) | 99 | e.Logf("restore session state: %v", err) | ||
| 100 | + | a.publishUnavailableChrome() | |||
| 100 | return | 101 | return | ||
| 101 | } | 102 | } | ||
| 102 | if state, err := a.store.snapshot(); err == nil { | 103 | if state, err := a.store.snapshot(); err == nil { | ||
| @@ -124,6 +125,7 @@ func main() { | |||||
| 124 | 125 | ||||
| 125 | if err := e.Run(); err != nil { | 126 | if err := e.Run(); err != nil { | ||
| 126 | fmt.Fprintln(os.Stderr, err) | 127 | fmt.Fprintln(os.Stderr, err) | ||
| 128 | + | os.Exit(1) | |||
| 127 | } | 129 | } | ||
| 128 | } | 130 | } | ||
| 129 | 131 | ||||
| @@ -185,13 +187,30 @@ func (a *app) handleTool(args json.RawMessage) ext.ToolResult { | |||||
| 185 | state, headline, err := a.store.transact(func(current PlanState) (PlanState, string, error) { | 187 | state, headline, err := a.store.transact(func(current PlanState) (PlanState, string, error) { | ||
| 186 | return applyAction(current, params) | 188 | return applyAction(current, params) | ||
| 187 | }) | 189 | }) | ||
| 188 | - | a.publishChrome(state) | 190 | + | // A zero state means transact could not obtain a loaded snapshot. Do not |
| 191 | + | // replace persistent chrome with an invented empty plan. For action and | |||
| 192 | + | // persistence errors, transact returns the unchanged usable state, which | |||
| 193 | + | // should remain visible to help the user recover. | |||
| 194 | + | if err == nil || state.Version != 0 { | |||
| 195 | + | a.publishChrome(state) | |||
| 196 | + | } | |||
| 189 | if err == nil { | 197 | if err == nil { | ||
| 190 | a.renderPanelIfOpen(state) | 198 | a.renderPanelIfOpen(state) | ||
| 191 | } | 199 | } | ||
| 192 | return toolResult(params.Action, state, headline, err) | 200 | return toolResult(params.Action, state, headline, err) | ||
| 193 | } | 201 | } | ||
| 194 | 202 | ||||
| 203 | + | func (a *app) publishUnavailableChrome() { | |||
| 204 | + | a.ext.SetStatus("progress", "plan unavailable") | |||
| 205 | + | a.ext.SetWidget("plan", "above_input", "Tasked phases", []string{"Plan state could not be restored for this session."}) | |||
| 206 | + | a.panelMu.Lock() | |||
| 207 | + | open := a.panelOpen | |||
| 208 | + | a.panelMu.Unlock() | |||
| 209 | + | if open { | |||
| 210 | + | a.ext.RenderPanel(panelID, "Tasked phases", []string{"Plan state could not be restored for this session."}, "Escape closes this panel") | |||
| 211 | + | } | |||
| 212 | + | } | |||
| 213 | + | ||||
| 195 | func (a *app) publishChrome(state PlanState) { | 214 | func (a *app) publishChrome(state PlanState) { | ||
| 196 | if isPlanClosed(state) { | 215 | if isPlanClosed(state) { | ||
| 197 | a.ext.SetStatus("progress", "closed: "+state.ClosedSummary) | 216 | a.ext.SetStatus("progress", "closed: "+state.ClosedSummary) | ||
A examples/extensions/tasked-phases/manifest_test.go +48 -0
| @@ -0,0 +1,48 @@ | |||
| 1 | + | package main | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "encoding/json" | |
| 5 | + | "os" | |
| 6 | + | "path/filepath" | |
| 7 | + | "slices" | |
| 8 | + | "testing" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestManifestMatchesExtensionConstants(t *testing.T) { | |
| 12 | + | data, err := os.ReadFile("extension.json") | |
| 13 | + | if err != nil { | |
| 14 | + | t.Fatal(err) | |
| 15 | + | } | |
| 16 | + | var manifest struct { | |
| 17 | + | Name string `json:"name"` | |
| 18 | + | Version string `json:"version"` | |
| 19 | + | Skills []string `json:"skills"` | |
| 20 | + | } | |
| 21 | + | if err := json.Unmarshal(data, &manifest); err != nil { | |
| 22 | + | t.Fatalf("extension.json is not valid JSON: %v", err) | |
| 23 | + | } | |
| 24 | + | if manifest.Name != extensionName || manifest.Version != extensionVersion { | |
| 25 | + | t.Fatalf("manifest = %s/%s, constants = %s/%s", manifest.Name, manifest.Version, extensionName, extensionVersion) | |
| 26 | + | } | |
| 27 | + | for _, skill := range manifest.Skills { | |
| 28 | + | if _, err := os.Stat(filepath.Join(skill, "SKILL.md")); err != nil { | |
| 29 | + | t.Errorf("declared skill %q has no SKILL.md: %v", skill, err) | |
| 30 | + | } | |
| 31 | + | } | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | func TestToolSchemaEnumMatchesToolActions(t *testing.T) { | |
| 35 | + | var schema struct { | |
| 36 | + | Properties struct { | |
| 37 | + | Action struct { | |
| 38 | + | Enum []string `json:"enum"` | |
| 39 | + | } `json:"action"` | |
| 40 | + | } `json:"properties"` | |
| 41 | + | } | |
| 42 | + | if err := json.Unmarshal([]byte(toolSchema), &schema); err != nil { | |
| 43 | + | t.Fatalf("tool schema is not valid JSON: %v", err) | |
| 44 | + | } | |
| 45 | + | if !slices.Equal(schema.Properties.Action.Enum, toolActions) { | |
| 46 | + | t.Fatalf("schema enum = %v, toolActions = %v", schema.Properties.Action.Enum, toolActions) | |
| 47 | + | } | |
| 48 | + | } | |
| @@ -0,0 +1,48 @@ | |||||
| 1 | + | package main | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "encoding/json" | |||
| 5 | + | "os" | |||
| 6 | + | "path/filepath" | |||
| 7 | + | "slices" | |||
| 8 | + | "testing" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestManifestMatchesExtensionConstants(t *testing.T) { | |||
| 12 | + | data, err := os.ReadFile("extension.json") | |||
| 13 | + | if err != nil { | |||
| 14 | + | t.Fatal(err) | |||
| 15 | + | } | |||
| 16 | + | var manifest struct { | |||
| 17 | + | Name string `json:"name"` | |||
| 18 | + | Version string `json:"version"` | |||
| 19 | + | Skills []string `json:"skills"` | |||
| 20 | + | } | |||
| 21 | + | if err := json.Unmarshal(data, &manifest); err != nil { | |||
| 22 | + | t.Fatalf("extension.json is not valid JSON: %v", err) | |||
| 23 | + | } | |||
| 24 | + | if manifest.Name != extensionName || manifest.Version != extensionVersion { | |||
| 25 | + | t.Fatalf("manifest = %s/%s, constants = %s/%s", manifest.Name, manifest.Version, extensionName, extensionVersion) | |||
| 26 | + | } | |||
| 27 | + | for _, skill := range manifest.Skills { | |||
| 28 | + | if _, err := os.Stat(filepath.Join(skill, "SKILL.md")); err != nil { | |||
| 29 | + | t.Errorf("declared skill %q has no SKILL.md: %v", skill, err) | |||
| 30 | + | } | |||
| 31 | + | } | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | func TestToolSchemaEnumMatchesToolActions(t *testing.T) { | |||
| 35 | + | var schema struct { | |||
| 36 | + | Properties struct { | |||
| 37 | + | Action struct { | |||
| 38 | + | Enum []string `json:"enum"` | |||
| 39 | + | } `json:"action"` | |||
| 40 | + | } `json:"properties"` | |||
| 41 | + | } | |||
| 42 | + | if err := json.Unmarshal([]byte(toolSchema), &schema); err != nil { | |||
| 43 | + | t.Fatalf("tool schema is not valid JSON: %v", err) | |||
| 44 | + | } | |||
| 45 | + | if !slices.Equal(schema.Properties.Action.Enum, toolActions) { | |||
| 46 | + | t.Fatalf("schema enum = %v, toolActions = %v", schema.Properties.Action.Enum, toolActions) | |||
| 47 | + | } | |||
| 48 | + | } | |||
M examples/extensions/tasked-phases/skill_sync_test.go +2 -7
| @@ -4,17 +4,12 @@ import ( | |||
| 4 | 4 | "bytes" | |
| 5 | 5 | "os" | |
| 6 | 6 | "path/filepath" | |
| 7 | - | "runtime" | |
| 8 | 7 | "testing" | |
| 9 | 8 | ) | |
| 10 | 9 | ||
| 11 | 10 | func TestBundledSkillMatchesStandaloneCopy(t *testing.T) { | |
| 12 | - | _, file, _, ok := runtime.Caller(0) | |
| 13 | - | if !ok { | |
| 14 | - | t.Fatal("runtime.Caller failed") | |
| 15 | - | } | |
| 16 | - | extensionSkill := filepath.Join(filepath.Dir(file), "skills", "tasked-phases", "SKILL.md") | |
| 17 | - | standaloneSkill := filepath.Join(filepath.Dir(file), "..", "..", "skills", "tasked-phases", "SKILL.md") | |
| 11 | + | extensionSkill := filepath.Join("skills", "tasked-phases", "SKILL.md") | |
| 12 | + | standaloneSkill := filepath.Join("..", "..", "skills", "tasked-phases", "SKILL.md") | |
| 18 | 13 | extension, err := os.ReadFile(extensionSkill) | |
| 19 | 14 | if err != nil { | |
| 20 | 15 | t.Fatal(err) | |
| @@ -4,17 +4,12 @@ import ( | |||||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | "os" | 5 | "os" | ||
| 6 | "path/filepath" | 6 | "path/filepath" | ||
| 7 | - | "runtime" | |||
| 8 | "testing" | 7 | "testing" | ||
| 9 | ) | 8 | ) | ||
| 10 | 9 | ||||
| 11 | func TestBundledSkillMatchesStandaloneCopy(t *testing.T) { | 10 | func TestBundledSkillMatchesStandaloneCopy(t *testing.T) { | ||
| 12 | - | _, file, _, ok := runtime.Caller(0) | 11 | + | extensionSkill := filepath.Join("skills", "tasked-phases", "SKILL.md") |
| 13 | - | if !ok { | 12 | + | standaloneSkill := filepath.Join("..", "..", "skills", "tasked-phases", "SKILL.md") |
| 14 | - | t.Fatal("runtime.Caller failed") | |||
| 15 | - | } | |||
| 16 | - | extensionSkill := filepath.Join(filepath.Dir(file), "skills", "tasked-phases", "SKILL.md") | |||
| 17 | - | standaloneSkill := filepath.Join(filepath.Dir(file), "..", "..", "skills", "tasked-phases", "SKILL.md") | |||
| 18 | extension, err := os.ReadFile(extensionSkill) | 13 | extension, err := os.ReadFile(extensionSkill) | ||
| 19 | if err != nil { | 14 | if err != nil { | ||
| 20 | t.Fatal(err) | 15 | t.Fatal(err) | ||
M examples/extensions/tasked-phases/skills/tasked-phases/SKILL.md +1 -1
| @@ -16,7 +16,7 @@ Use the `tasked_phases` tool as the source of truth for the spec, phases, curren | |||
| 16 | 16 | 5. Set the focus with `set_current_phase` and pass the target `phaseId`. | |
| 17 | 17 | 6. Call `set_task_checked` immediately after each checklist task is completed. | |
| 18 | 18 | 7. Use `set_phase_checked` to complete or reopen every task in a phase. The phase must already contain at least one task. | |
| 19 | - | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` before creating a new plan. | |
| 19 | + | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` to discard the plan, or call `set_spec` immediately followed by `replace_plan` to record a new spec and a new set of phases. | |
| 20 | 20 | ||
| 21 | 21 | ## Planning rules | |
| 22 | 22 | ||
| @@ -16,7 +16,7 @@ Use the `tasked_phases` tool as the source of truth for the spec, phases, curren | |||||
| 16 | 5. Set the focus with `set_current_phase` and pass the target `phaseId`. | 16 | 5. Set the focus with `set_current_phase` and pass the target `phaseId`. | ||
| 17 | 6. Call `set_task_checked` immediately after each checklist task is completed. | 17 | 6. Call `set_task_checked` immediately after each checklist task is completed. | ||
| 18 | 7. Use `set_phase_checked` to complete or reopen every task in a phase. The phase must already contain at least one task. | 18 | 7. Use `set_phase_checked` to complete or reopen every task in a phase. The phase must already contain at least one task. | ||
| 19 | - | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` before creating a new plan. | 19 | + | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` to discard the plan, or call `set_spec` immediately followed by `replace_plan` to record a new spec and a new set of phases. |
| 20 | 20 | ||||
| 21 | ## Planning rules | 21 | ## Planning rules | ||
| 22 | 22 | ||||
M examples/extensions/tasked-phases/state.go +139 -36
| @@ -9,9 +9,12 @@ import ( | |||
| 9 | 9 | "os" | |
| 10 | 10 | "path/filepath" | |
| 11 | 11 | "regexp" | |
| 12 | + | "strconv" | |
| 12 | 13 | "strings" | |
| 13 | 14 | "sync" | |
| 14 | 15 | "time" | |
| 16 | + | "unicode" | |
| 17 | + | "unicode/utf8" | |
| 15 | 18 | ) | |
| 16 | 19 | ||
| 17 | 20 | const ( | |
| @@ -30,6 +33,12 @@ const ( | |||
| 30 | 33 | actionClear = "clear" | |
| 31 | 34 | ) | |
| 32 | 35 | ||
| 36 | + | var ( | |
| 37 | + | phaseIDPattern = regexp.MustCompile(`^phase-(\d+)$`) | |
| 38 | + | taskIDPattern = regexp.MustCompile(`^task-(\d+)$`) | |
| 39 | + | bracketedIDPattern = regexp.MustCompile(`\[([^\]]+)\]`) | |
| 40 | + | ) | |
| 41 | + | ||
| 33 | 42 | var toolActions = []string{ | |
| 34 | 43 | actionGetStatus, | |
| 35 | 44 | actionSetSpec, | |
| @@ -135,10 +144,63 @@ func normalizeOptionalText(value string) string { | |||
| 135 | 144 | return strings.TrimSpace(value) | |
| 136 | 145 | } | |
| 137 | 146 | ||
| 147 | + | // sanitizeDisplayText removes terminal escape sequences and non-printing | |
| 148 | + | // control characters at the rendering boundary. Persisted state is never | |
| 149 | + | // modified, so the original extension-supplied text remains recoverable. | |
| 150 | + | func sanitizeDisplayText(text string) string { | |
| 151 | + | var out strings.Builder | |
| 152 | + | for index := 0; index < len(text); { | |
| 153 | + | if text[index] == 0x1b { | |
| 154 | + | index++ | |
| 155 | + | if index >= len(text) { | |
| 156 | + | break | |
| 157 | + | } | |
| 158 | + | switch text[index] { | |
| 159 | + | case '[': // CSI sequence. | |
| 160 | + | index++ | |
| 161 | + | for index < len(text) { | |
| 162 | + | final := text[index] | |
| 163 | + | index++ | |
| 164 | + | if final >= 0x40 && final <= 0x7e { | |
| 165 | + | break | |
| 166 | + | } | |
| 167 | + | } | |
| 168 | + | case ']': // OSC sequence, terminated by BEL or ST. | |
| 169 | + | index++ | |
| 170 | + | for index < len(text) { | |
| 171 | + | if text[index] == 0x07 { | |
| 172 | + | index++ | |
| 173 | + | break | |
| 174 | + | } | |
| 175 | + | if text[index] == 0x1b && index+1 < len(text) && text[index+1] == '\\' { | |
| 176 | + | index += 2 | |
| 177 | + | break | |
| 178 | + | } | |
| 179 | + | index++ | |
| 180 | + | } | |
| 181 | + | default: | |
| 182 | + | index++ | |
| 183 | + | } | |
| 184 | + | continue | |
| 185 | + | } | |
| 186 | + | r, size := utf8.DecodeRuneInString(text[index:]) | |
| 187 | + | index += size | |
| 188 | + | if r != '\n' && (unicode.IsControl(r) || r == 0x7f) { | |
| 189 | + | continue | |
| 190 | + | } | |
| 191 | + | out.WriteRune(r) | |
| 192 | + | } | |
| 193 | + | return out.String() | |
| 194 | + | } | |
| 195 | + | ||
| 138 | 196 | func singleLine(text string) string { | |
| 139 | 197 | return strings.Join(strings.Fields(text), " ") | |
| 140 | 198 | } | |
| 141 | 199 | ||
| 200 | + | func displaySingleLine(text string) string { | |
| 201 | + | return singleLine(sanitizeDisplayText(text)) | |
| 202 | + | } | |
| 203 | + | ||
| 142 | 204 | func truncatePlain(text string, maxLength int) string { | |
| 143 | 205 | if maxLength < 1 { | |
| 144 | 206 | return "" | |
| @@ -250,14 +312,17 @@ func getSuggestedCurrentPhaseID(state PlanState) string { | |||
| 250 | 312 | ||
| 251 | 313 | func extractHighestIDNumber(prefix string, ids []string) int { | |
| 252 | 314 | highest := 0 | |
| 253 | - | pattern := regexp.MustCompile(`^` + regexp.QuoteMeta(prefix) + `-(\d+)$`) | |
| 315 | + | pattern := taskIDPattern | |
| 316 | + | if prefix == "phase" { | |
| 317 | + | pattern = phaseIDPattern | |
| 318 | + | } | |
| 254 | 319 | for _, id := range ids { | |
| 255 | 320 | match := pattern.FindStringSubmatch(id) | |
| 256 | 321 | if len(match) != 2 { | |
| 257 | 322 | continue | |
| 258 | 323 | } | |
| 259 | - | var number int | |
| 260 | - | if _, err := fmt.Sscanf(match[1], "%d", &number); err == nil && number > highest { | |
| 324 | + | number, err := strconv.Atoi(match[1]) | |
| 325 | + | if err == nil && number > highest { | |
| 261 | 326 | highest = number | |
| 262 | 327 | } | |
| 263 | 328 | } | |
| @@ -334,7 +399,7 @@ func nextTaskID(state *PlanState) string { | |||
| 334 | 399 | } | |
| 335 | 400 | ||
| 336 | 401 | func extractBracketedPhaseID(value string) string { | |
| 337 | - | matches := regexp.MustCompile(`\[([^\]]+)\]`).FindAllStringSubmatch(value, -1) | |
| 402 | + | matches := bracketedIDPattern.FindAllStringSubmatch(value, -1) | |
| 338 | 403 | candidate := "" | |
| 339 | 404 | for _, match := range matches { | |
| 340 | 405 | if len(match) == 2 { | |
| @@ -362,6 +427,8 @@ func findUniquePhaseByTitle(state PlanState, phaseTitle string) *Phase { | |||
| 362 | 427 | return nil | |
| 363 | 428 | } | |
| 364 | 429 | ||
| 430 | + | // findPhase returns a pointer into state.Phases, aliasing the caller's slice | |
| 431 | + | // backing array so mutations are applied to the caller's state copy. | |
| 365 | 432 | func findPhase(state PlanState, phaseID string) *Phase { | |
| 366 | 433 | candidate := normalizeOptionalText(phaseID) | |
| 367 | 434 | if candidate == "" { | |
| @@ -402,6 +469,32 @@ func phaseIDError(state PlanState, action, phaseID string) string { | |||
| 402 | 469 | return fmt.Sprintf("phaseId %q was not found for %s. Use the raw id from brackets without brackets. Valid phase ids: %s.", singleLine(candidate), action, validIDs) | |
| 403 | 470 | } | |
| 404 | 471 | ||
| 472 | + | func formatValidTaskIDs(state PlanState) string { | |
| 473 | + | parts := make([]string, 0) | |
| 474 | + | for _, phase := range state.Phases { | |
| 475 | + | for _, task := range phase.Tasks { | |
| 476 | + | if !task.Checked { | |
| 477 | + | parts = append(parts, fmt.Sprintf("%s (%s)", task.ID, displaySingleLine(phase.Title))) | |
| 478 | + | } | |
| 479 | + | } | |
| 480 | + | } | |
| 481 | + | if len(parts) == 0 { | |
| 482 | + | return "none" | |
| 483 | + | } | |
| 484 | + | return strings.Join(parts, ", ") | |
| 485 | + | } | |
| 486 | + | ||
| 487 | + | func taskIDError(state PlanState, action, taskID string) string { | |
| 488 | + | candidate := normalizeOptionalText(taskID) | |
| 489 | + | validIDs := formatValidTaskIDs(state) | |
| 490 | + | if candidate == "" { | |
| 491 | + | return fmt.Sprintf("taskId is required for %s. Use a valid task id. Valid incomplete task ids: %s.", action, validIDs) | |
| 492 | + | } | |
| 493 | + | return fmt.Sprintf("taskId %q was not found for %s. Use the task id from brackets. Valid incomplete task ids: %s.", displaySingleLine(candidate), action, validIDs) | |
| 494 | + | } | |
| 495 | + | ||
| 496 | + | // findTask returns pointers into state.Phases and their Tasks, aliasing the | |
| 497 | + | // caller's slice backing arrays so mutations are applied to the caller's state. | |
| 405 | 498 | func findTask(state PlanState, taskID, phaseID string) (phase *Phase, task *PhaseTask) { | |
| 406 | 499 | if taskID == "" { | |
| 407 | 500 | return nil, nil | |
| @@ -590,13 +683,16 @@ func applyAction(state PlanState, params toolArgs) (PlanState, string, error) { | |||
| 590 | 683 | phase.Tasks = append(phase.Tasks, PhaseTask{ID: nextTaskID(&nextState), Text: taskText}) | |
| 591 | 684 | return nextState, "Added task to " + phase.Title, nil | |
| 592 | 685 | case actionUpdateTask: | |
| 686 | + | if params.TaskID == "" { | |
| 687 | + | return state, "Task not updated", errors.New("taskId is required for update_task") | |
| 688 | + | } | |
| 593 | 689 | _, task := findTask(nextState, params.TaskID, params.PhaseID) | |
| 594 | 690 | taskText := "" | |
| 595 | 691 | if params.TaskText != nil { | |
| 596 | 692 | taskText = normalizeOptionalText(*params.TaskText) | |
| 597 | 693 | } | |
| 598 | 694 | if task == nil { | |
| 599 | - | return state, "Task not updated", errors.New("taskId was not found") | |
| 695 | + | return state, "Task not updated", errors.New(taskIDError(nextState, actionUpdateTask, params.TaskID)) | |
| 600 | 696 | } | |
| 601 | 697 | if taskText == "" { | |
| 602 | 698 | return state, "Task not updated", errors.New("taskText is required for update_task") | |
| @@ -627,9 +723,12 @@ func applyAction(state PlanState, params toolArgs) (PlanState, string, error) { | |||
| 627 | 723 | nextState.CurrentPhaseID = phase.ID | |
| 628 | 724 | return nextState, "Current phase set to " + phase.Title, nil | |
| 629 | 725 | case actionSetTaskChecked: | |
| 726 | + | if params.TaskID == "" { | |
| 727 | + | return state, "Task not updated", errors.New("taskId is required for set_task_checked") | |
| 728 | + | } | |
| 630 | 729 | phase, task := findTask(nextState, params.TaskID, params.PhaseID) | |
| 631 | 730 | if task == nil { | |
| 632 | - | return state, "Task not updated", errors.New("taskId was not found") | |
| 731 | + | return state, "Task not updated", errors.New(taskIDError(nextState, actionSetTaskChecked, params.TaskID)) | |
| 633 | 732 | } | |
| 634 | 733 | if params.Checked == nil { | |
| 635 | 734 | return state, "Task not updated", errors.New("checked must be provided for set_task_checked") | |
| @@ -697,8 +796,8 @@ func formatRemainingTaskCount(count int) string { | |||
| 697 | 796 | return fmt.Sprintf("%d remaining", count) | |
| 698 | 797 | } | |
| 699 | 798 | ||
| 700 | - | func formatPhaseTitle(phase Phase) string { return truncatePlain(singleLine(phase.Title), 120) } | |
| 701 | - | func formatTaskText(task PhaseTask) string { return truncatePlain(singleLine(task.Text), 180) } | |
| 799 | + | func formatPhaseTitle(phase Phase) string { return truncatePlain(displaySingleLine(phase.Title), 120) } | |
| 800 | + | func formatTaskText(task PhaseTask) string { return truncatePlain(displaySingleLine(task.Text), 180) } | |
| 702 | 801 | ||
| 703 | 802 | func formatTurnContextID(id string) string { return truncatePlain(singleLine(id), 96) } | |
| 704 | 803 | ||
| @@ -708,7 +807,7 @@ func appendIncompleteTaskLines(lines *[]string, tasks []PhaseTask, visibleCount | |||
| 708 | 807 | visible = visible[:visibleCount] | |
| 709 | 808 | } | |
| 710 | 809 | for _, task := range visible { | |
| 711 | - | *lines = append(*lines, fmt.Sprintf("%s[ ] %s [%s]", indent, formatTaskText(task), task.ID)) | |
| 810 | + | *lines = append(*lines, fmt.Sprintf("%s[ ] %s [%s]", indent, formatTaskText(task), formatTurnContextID(task.ID))) | |
| 712 | 811 | } | |
| 713 | 812 | if omitted := len(tasks) - len(visible); omitted > 0 { | |
| 714 | 813 | *lines = append(*lines, fmt.Sprintf("%s... %d more incomplete task(s) omitted; call tasked_phases get_status for the full checklist.", indent, omitted)) | |
| @@ -721,7 +820,7 @@ func buildSummary(state PlanState) string { | |||
| 721 | 820 | } | |
| 722 | 821 | lines := make([]string, 0) | |
| 723 | 822 | if state.Spec != "" { | |
| 724 | - | lines = append(lines, "Spec:", state.Spec, "") | |
| 823 | + | lines = append(lines, "Spec:", sanitizeDisplayText(state.Spec), "") | |
| 725 | 824 | } | |
| 726 | 825 | if len(state.Phases) == 0 { | |
| 727 | 826 | return strings.Join(append(lines, "Phases: (none)"), "\n") | |
| @@ -733,7 +832,7 @@ func buildSummary(state PlanState) string { | |||
| 733 | 832 | if closedSummary == "" { | |
| 734 | 833 | closedSummary = "all phases complete" | |
| 735 | 834 | } | |
| 736 | - | lines = append(lines, "Plan closed: "+closedSummary) | |
| 835 | + | lines = append(lines, "Plan closed: "+displaySingleLine(closedSummary)) | |
| 737 | 836 | } | |
| 738 | 837 | lines = append(lines, "Phases:") | |
| 739 | 838 | for _, phase := range state.Phases { | |
| @@ -748,9 +847,9 @@ func buildSummary(state PlanState) string { | |||
| 748 | 847 | } | |
| 749 | 848 | goalSuffix := "" | |
| 750 | 849 | if phase.Goal != "" { | |
| 751 | - | goalSuffix = " - " + phase.Goal | |
| 850 | + | goalSuffix = " - " + displaySingleLine(phase.Goal) | |
| 752 | 851 | } | |
| 753 | - | lines = append(lines, fmt.Sprintf("%s %s %s [%s] (%d/%d)%s", currentMarker, phaseMarker, phase.Title, phase.ID, phaseDone, phaseTotal, goalSuffix)) | |
| 852 | + | lines = append(lines, fmt.Sprintf("%s %s %s [%s] (%d/%d)%s", currentMarker, phaseMarker, formatPhaseTitle(phase), formatTurnContextID(phase.ID), phaseDone, phaseTotal, goalSuffix)) | |
| 754 | 853 | if len(phase.Tasks) == 0 { | |
| 755 | 854 | lines = append(lines, " - No tasks yet") | |
| 756 | 855 | continue | |
| @@ -760,7 +859,7 @@ func buildSummary(state PlanState) string { | |||
| 760 | 859 | if task.Checked { | |
| 761 | 860 | marker = "[x]" | |
| 762 | 861 | } | |
| 763 | - | lines = append(lines, fmt.Sprintf(" %s %s [%s]", marker, task.Text, task.ID)) | |
| 862 | + | lines = append(lines, fmt.Sprintf(" %s %s [%s]", marker, formatTaskText(task), formatTurnContextID(task.ID))) | |
| 764 | 863 | } | |
| 765 | 864 | } | |
| 766 | 865 | return strings.Join(lines, "\n") | |
| @@ -772,7 +871,7 @@ func buildCompactSummary(state PlanState) string { | |||
| 772 | 871 | } | |
| 773 | 872 | lines := make([]string, 0) | |
| 774 | 873 | if state.Spec != "" { | |
| 775 | - | lines = append(lines, "Spec: "+truncatePlain(singleLine(state.Spec), 220)) | |
| 874 | + | lines = append(lines, "Spec: "+truncatePlain(displaySingleLine(state.Spec), 220)) | |
| 776 | 875 | } | |
| 777 | 876 | done, total := getPlanProgress(state) | |
| 778 | 877 | lines = append(lines, fmt.Sprintf("Progress: %d/%d tasks checked", done, total)) | |
| @@ -781,16 +880,16 @@ func buildCompactSummary(state PlanState) string { | |||
| 781 | 880 | if closedSummary == "" { | |
| 782 | 881 | closedSummary = "all phases complete" | |
| 783 | 882 | } | |
| 784 | - | lines = append(lines, "Plan closed: "+closedSummary) | |
| 883 | + | lines = append(lines, "Plan closed: "+displaySingleLine(closedSummary)) | |
| 785 | 884 | return strings.Join(lines, "\n") | |
| 786 | 885 | } | |
| 787 | 886 | if currentPhase := getActivePhase(state); currentPhase != nil { | |
| 788 | 887 | incompleteTasks := getIncompleteTasks(*currentPhase) | |
| 789 | 888 | goalSuffix := "" | |
| 790 | 889 | if currentPhase.Goal != "" { | |
| 791 | - | goalSuffix = " - " + truncatePlain(singleLine(currentPhase.Goal), 120) | |
| 890 | + | goalSuffix = " - " + truncatePlain(displaySingleLine(currentPhase.Goal), 120) | |
| 792 | 891 | } | |
| 793 | - | lines = append(lines, fmt.Sprintf("Current phase: %s [%s] (%s)%s", formatPhaseTitle(*currentPhase), currentPhase.ID, formatRemainingTaskCount(len(incompleteTasks)), goalSuffix)) | |
| 892 | + | lines = append(lines, fmt.Sprintf("Current phase: %s [%s] (%s)%s", formatPhaseTitle(*currentPhase), formatTurnContextID(currentPhase.ID), formatRemainingTaskCount(len(incompleteTasks)), goalSuffix)) | |
| 794 | 893 | if len(incompleteTasks) > 0 { | |
| 795 | 894 | lines = append(lines, "Incomplete tasks:") | |
| 796 | 895 | appendIncompleteTaskLines(&lines, incompleteTasks, compactVisibleIncompleteTasks, " ") | |
| @@ -820,7 +919,7 @@ func buildTurnContext(state PlanState) string { | |||
| 820 | 919 | fmt.Sprintf("Current phase: %s [%s]", formatPhaseTitle(*phase), formatTurnContextID(phase.ID)), | |
| 821 | 920 | } | |
| 822 | 921 | if phase.Goal != "" { | |
| 823 | - | lines = append(lines, "Goal: "+truncatePlain(singleLine(phase.Goal), 160)) | |
| 922 | + | lines = append(lines, "Goal: "+truncatePlain(displaySingleLine(phase.Goal), 160)) | |
| 824 | 923 | } | |
| 825 | 924 | incompleteTasks := getIncompleteTasks(*phase) | |
| 826 | 925 | if len(incompleteTasks) == 0 { | |
| @@ -828,16 +927,7 @@ func buildTurnContext(state PlanState) string { | |||
| 828 | 927 | return truncatePlain(strings.Join(lines, "\n"), maxTurnContextChars) | |
| 829 | 928 | } | |
| 830 | 929 | lines = append(lines, "Incomplete tasks:") | |
| 831 | - | visible := incompleteTasks | |
| 832 | - | if len(visible) > turnContextVisibleIncompleteTasks { | |
| 833 | - | visible = visible[:turnContextVisibleIncompleteTasks] | |
| 834 | - | } | |
| 835 | - | for _, task := range visible { | |
| 836 | - | lines = append(lines, fmt.Sprintf(" [ ] %s [%s]", formatTaskText(task), formatTurnContextID(task.ID))) | |
| 837 | - | } | |
| 838 | - | if omitted := len(incompleteTasks) - len(visible); omitted > 0 { | |
| 839 | - | lines = append(lines, fmt.Sprintf(" ... %d more incomplete task(s) omitted; call tasked_phases get_status for the full checklist.", omitted)) | |
| 840 | - | } | |
| 930 | + | appendIncompleteTaskLines(&lines, incompleteTasks, turnContextVisibleIncompleteTasks, " ") | |
| 841 | 931 | return truncatePlain(strings.Join(lines, "\n"), maxTurnContextChars) | |
| 842 | 932 | } | |
| 843 | 933 | ||
| @@ -869,7 +959,7 @@ func buildViewLines(state PlanState) []string { | |||
| 869 | 959 | } | |
| 870 | 960 | if state.Spec != "" { | |
| 871 | 961 | lines = append(lines, " Spec") | |
| 872 | - | for _, specLine := range strings.Split(state.Spec, "\n") { | |
| 962 | + | for _, specLine := range strings.Split(sanitizeDisplayText(state.Spec), "\n") { | |
| 873 | 963 | lines = append(lines, " "+specLine) | |
| 874 | 964 | } | |
| 875 | 965 | lines = append(lines, "") | |
| @@ -882,7 +972,7 @@ func buildViewLines(state PlanState) []string { | |||
| 882 | 972 | closedSummary = "all phases complete" | |
| 883 | 973 | } | |
| 884 | 974 | lines = append(lines, | |
| 885 | - | " Closed: "+closedSummary, | |
| 975 | + | " Closed: "+displaySingleLine(closedSummary), | |
| 886 | 976 | " New work should create a fresh plan instead of extending this one.", | |
| 887 | 977 | ) | |
| 888 | 978 | } | |
| @@ -897,9 +987,9 @@ func buildViewLines(state PlanState) []string { | |||
| 897 | 987 | if phase.ID == state.CurrentPhaseID { | |
| 898 | 988 | currentTitle = "> " + currentTitle | |
| 899 | 989 | } | |
| 900 | - | lines = append(lines, fmt.Sprintf(" %s %s [%s] (%d/%d)", marker, currentTitle, phase.ID, phaseDone, phaseTotal)) | |
| 990 | + | lines = append(lines, fmt.Sprintf(" %s %s [%s] (%d/%d)", marker, displaySingleLine(currentTitle), formatTurnContextID(phase.ID), phaseDone, phaseTotal)) | |
| 901 | 991 | if phase.Goal != "" { | |
| 902 | - | lines = append(lines, " "+phase.Goal) | |
| 992 | + | lines = append(lines, " "+displaySingleLine(phase.Goal)) | |
| 903 | 993 | } | |
| 904 | 994 | if len(phase.Tasks) == 0 { | |
| 905 | 995 | lines = append(lines, " - No tasks yet") | |
| @@ -910,7 +1000,7 @@ func buildViewLines(state PlanState) []string { | |||
| 910 | 1000 | if task.Checked { | |
| 911 | 1001 | taskMarker = "[x]" | |
| 912 | 1002 | } | |
| 913 | - | lines = append(lines, fmt.Sprintf(" %s %s [%s]", taskMarker, task.Text, task.ID)) | |
| 1003 | + | lines = append(lines, fmt.Sprintf(" %s %s [%s]", taskMarker, formatTaskText(task), formatTurnContextID(task.ID))) | |
| 914 | 1004 | } | |
| 915 | 1005 | lines = append(lines, "") | |
| 916 | 1006 | } | |
| @@ -923,6 +1013,9 @@ type stateStore struct { | |||
| 923 | 1013 | path string | |
| 924 | 1014 | loaded bool | |
| 925 | 1015 | loadErr error | |
| 1016 | + | // pathErr records a persistence fault independently of the active | |
| 1017 | + | // session state. Activating a session must never clear this fault. | |
| 1018 | + | pathErr error | |
| 926 | 1019 | } | |
| 927 | 1020 | ||
| 928 | 1021 | func newStateStore() *stateStore { | |
| @@ -944,15 +1037,19 @@ func (s *stateStore) load(dataDir, cwd string) error { | |||
| 944 | 1037 | s.state = createEmptyState() | |
| 945 | 1038 | if strings.TrimSpace(dataDir) == "" { | |
| 946 | 1039 | s.loadErr = errors.New("host did not provide extension data_dir") | |
| 1040 | + | s.pathErr = s.loadErr | |
| 947 | 1041 | return s.loadErr | |
| 948 | 1042 | } | |
| 949 | 1043 | s.path = projectStatePath(dataDir, cwd) | |
| 950 | 1044 | if err := os.MkdirAll(filepath.Dir(s.path), 0o700); err != nil { | |
| 951 | 1045 | s.loadErr = fmt.Errorf("create state directory: %w", err) | |
| 1046 | + | s.pathErr = s.loadErr | |
| 952 | 1047 | return s.loadErr | |
| 953 | 1048 | } | |
| 1049 | + | // #nosec G302 -- this is a directory; 0700 is the minimum mode that permits traversal. | |
| 954 | 1050 | if err := os.Chmod(filepath.Dir(s.path), 0o700); err != nil { | |
| 955 | 1051 | s.loadErr = fmt.Errorf("restrict state directory: %w", err) | |
| 1052 | + | s.pathErr = s.loadErr | |
| 956 | 1053 | return s.loadErr | |
| 957 | 1054 | } | |
| 958 | 1055 | data, err := os.ReadFile(s.path) | |
| @@ -961,10 +1058,12 @@ func (s *stateStore) load(dataDir, cwd string) error { | |||
| 961 | 1058 | } | |
| 962 | 1059 | if err != nil { | |
| 963 | 1060 | s.loadErr = fmt.Errorf("read state: %w", err) | |
| 1061 | + | s.pathErr = s.loadErr | |
| 964 | 1062 | return s.loadErr | |
| 965 | 1063 | } | |
| 966 | 1064 | if err := os.Chmod(s.path, 0o600); err != nil { | |
| 967 | 1065 | s.loadErr = fmt.Errorf("restrict state file: %w", err) | |
| 1066 | + | s.pathErr = s.loadErr | |
| 968 | 1067 | return s.loadErr | |
| 969 | 1068 | } | |
| 970 | 1069 | if len(data) == 0 { | |
| @@ -973,6 +1072,7 @@ func (s *stateStore) load(dataDir, cwd string) error { | |||
| 973 | 1072 | var state PlanState | |
| 974 | 1073 | if err := json.Unmarshal(data, &state); err != nil { | |
| 975 | 1074 | s.loadErr = fmt.Errorf("parse state: %w", err) | |
| 1075 | + | s.pathErr = s.loadErr | |
| 976 | 1076 | return s.loadErr | |
| 977 | 1077 | } | |
| 978 | 1078 | s.state = ensureState(state) | |
| @@ -988,8 +1088,8 @@ func (s *stateStore) activateSession(raw json.RawMessage) error { | |||
| 988 | 1088 | if !s.loaded { | |
| 989 | 1089 | return errors.New("state has not been loaded from hello") | |
| 990 | 1090 | } | |
| 991 | - | if s.path == "" && s.loadErr != nil { | |
| 992 | - | return s.loadErr | |
| 1091 | + | if s.pathErr != nil { | |
| 1092 | + | return s.pathErr | |
| 993 | 1093 | } | |
| 994 | 1094 | if len(raw) == 0 || strings.TrimSpace(string(raw)) == "null" { | |
| 995 | 1095 | s.state = createEmptyState() | |
| @@ -1023,6 +1123,9 @@ func (s *stateStore) transact(fn func(PlanState) (PlanState, string, error)) (Pl | |||
| 1023 | 1123 | if !s.loaded { | |
| 1024 | 1124 | return PlanState{}, "", errors.New("state has not been loaded from hello") | |
| 1025 | 1125 | } | |
| 1126 | + | if s.pathErr != nil { | |
| 1127 | + | return cloneState(s.state), "", s.pathErr | |
| 1128 | + | } | |
| 1026 | 1129 | if s.loadErr != nil { | |
| 1027 | 1130 | return cloneState(s.state), "", s.loadErr | |
| 1028 | 1131 | } | |
| @@ -9,9 +9,12 @@ import ( | |||||
| 9 | "os" | 9 | "os" | ||
| 10 | "path/filepath" | 10 | "path/filepath" | ||
| 11 | "regexp" | 11 | "regexp" | ||
| 12 | + | "strconv" | |||
| 12 | "strings" | 13 | "strings" | ||
| 13 | "sync" | 14 | "sync" | ||
| 14 | "time" | 15 | "time" | ||
| 16 | + | "unicode" | |||
| 17 | + | "unicode/utf8" | |||
| 15 | ) | 18 | ) | ||
| 16 | 19 | ||||
| 17 | const ( | 20 | const ( | ||
| @@ -30,6 +33,12 @@ const ( | |||||
| 30 | actionClear = "clear" | 33 | actionClear = "clear" | ||
| 31 | ) | 34 | ) | ||
| 32 | 35 | ||||
| 36 | + | var ( | |||
| 37 | + | phaseIDPattern = regexp.MustCompile(`^phase-(\d+)$`) | |||
| 38 | + | taskIDPattern = regexp.MustCompile(`^task-(\d+)$`) | |||
| 39 | + | bracketedIDPattern = regexp.MustCompile(`\[([^\]]+)\]`) | |||
| 40 | + | ) | |||
| 41 | + | ||||
| 33 | var toolActions = []string{ | 42 | var toolActions = []string{ | ||
| 34 | actionGetStatus, | 43 | actionGetStatus, | ||
| 35 | actionSetSpec, | 44 | actionSetSpec, | ||
| @@ -135,10 +144,63 @@ func normalizeOptionalText(value string) string { | |||||
| 135 | return strings.TrimSpace(value) | 144 | return strings.TrimSpace(value) | ||
| 136 | } | 145 | } | ||
| 137 | 146 | ||||
| 147 | + | // sanitizeDisplayText removes terminal escape sequences and non-printing | |||
| 148 | + | // control characters at the rendering boundary. Persisted state is never | |||
| 149 | + | // modified, so the original extension-supplied text remains recoverable. | |||
| 150 | + | func sanitizeDisplayText(text string) string { | |||
| 151 | + | var out strings.Builder | |||
| 152 | + | for index := 0; index < len(text); { | |||
| 153 | + | if text[index] == 0x1b { | |||
| 154 | + | index++ | |||
| 155 | + | if index >= len(text) { | |||
| 156 | + | break | |||
| 157 | + | } | |||
| 158 | + | switch text[index] { | |||
| 159 | + | case '[': // CSI sequence. | |||
| 160 | + | index++ | |||
| 161 | + | for index < len(text) { | |||
| 162 | + | final := text[index] | |||
| 163 | + | index++ | |||
| 164 | + | if final >= 0x40 && final <= 0x7e { | |||
| 165 | + | break | |||
| 166 | + | } | |||
| 167 | + | } | |||
| 168 | + | case ']': // OSC sequence, terminated by BEL or ST. | |||
| 169 | + | index++ | |||
| 170 | + | for index < len(text) { | |||
| 171 | + | if text[index] == 0x07 { | |||
| 172 | + | index++ | |||
| 173 | + | break | |||
| 174 | + | } | |||
| 175 | + | if text[index] == 0x1b && index+1 < len(text) && text[index+1] == '\\' { | |||
| 176 | + | index += 2 | |||
| 177 | + | break | |||
| 178 | + | } | |||
| 179 | + | index++ | |||
| 180 | + | } | |||
| 181 | + | default: | |||
| 182 | + | index++ | |||
| 183 | + | } | |||
| 184 | + | continue | |||
| 185 | + | } | |||
| 186 | + | r, size := utf8.DecodeRuneInString(text[index:]) | |||
| 187 | + | index += size | |||
| 188 | + | if r != '\n' && (unicode.IsControl(r) || r == 0x7f) { | |||
| 189 | + | continue | |||
| 190 | + | } | |||
| 191 | + | out.WriteRune(r) | |||
| 192 | + | } | |||
| 193 | + | return out.String() | |||
| 194 | + | } | |||
| 195 | + | ||||
| 138 | func singleLine(text string) string { | 196 | func singleLine(text string) string { | ||
| 139 | return strings.Join(strings.Fields(text), " ") | 197 | return strings.Join(strings.Fields(text), " ") | ||
| 140 | } | 198 | } | ||
| 141 | 199 | ||||
| 200 | + | func displaySingleLine(text string) string { | |||
| 201 | + | return singleLine(sanitizeDisplayText(text)) | |||
| 202 | + | } | |||
| 203 | + | ||||
| 142 | func truncatePlain(text string, maxLength int) string { | 204 | func truncatePlain(text string, maxLength int) string { | ||
| 143 | if maxLength < 1 { | 205 | if maxLength < 1 { | ||
| 144 | return "" | 206 | return "" | ||
| @@ -250,14 +312,17 @@ func getSuggestedCurrentPhaseID(state PlanState) string { | |||||
| 250 | 312 | ||||
| 251 | func extractHighestIDNumber(prefix string, ids []string) int { | 313 | func extractHighestIDNumber(prefix string, ids []string) int { | ||
| 252 | highest := 0 | 314 | highest := 0 | ||
| 253 | - | pattern := regexp.MustCompile(`^` + regexp.QuoteMeta(prefix) + `-(\d+)$`) | 315 | + | pattern := taskIDPattern |
| 316 | + | if prefix == "phase" { | |||
| 317 | + | pattern = phaseIDPattern | |||
| 318 | + | } | |||
| 254 | for _, id := range ids { | 319 | for _, id := range ids { | ||
| 255 | match := pattern.FindStringSubmatch(id) | 320 | match := pattern.FindStringSubmatch(id) | ||
| 256 | if len(match) != 2 { | 321 | if len(match) != 2 { | ||
| 257 | continue | 322 | continue | ||
| 258 | } | 323 | } | ||
| 259 | - | var number int | 324 | + | number, err := strconv.Atoi(match[1]) |
| 260 | - | if _, err := fmt.Sscanf(match[1], "%d", &number); err == nil && number > highest { | 325 | + | if err == nil && number > highest { |
| 261 | highest = number | 326 | highest = number | ||
| 262 | } | 327 | } | ||
| 263 | } | 328 | } | ||
| @@ -334,7 +399,7 @@ func nextTaskID(state *PlanState) string { | |||||
| 334 | } | 399 | } | ||
| 335 | 400 | ||||
| 336 | func extractBracketedPhaseID(value string) string { | 401 | func extractBracketedPhaseID(value string) string { | ||
| 337 | - | matches := regexp.MustCompile(`\[([^\]]+)\]`).FindAllStringSubmatch(value, -1) | 402 | + | matches := bracketedIDPattern.FindAllStringSubmatch(value, -1) |
| 338 | candidate := "" | 403 | candidate := "" | ||
| 339 | for _, match := range matches { | 404 | for _, match := range matches { | ||
| 340 | if len(match) == 2 { | 405 | if len(match) == 2 { | ||
| @@ -362,6 +427,8 @@ func findUniquePhaseByTitle(state PlanState, phaseTitle string) *Phase { | |||||
| 362 | return nil | 427 | return nil | ||
| 363 | } | 428 | } | ||
| 364 | 429 | ||||
| 430 | + | // findPhase returns a pointer into state.Phases, aliasing the caller's slice | |||
| 431 | + | // backing array so mutations are applied to the caller's state copy. | |||
| 365 | func findPhase(state PlanState, phaseID string) *Phase { | 432 | func findPhase(state PlanState, phaseID string) *Phase { | ||
| 366 | candidate := normalizeOptionalText(phaseID) | 433 | candidate := normalizeOptionalText(phaseID) | ||
| 367 | if candidate == "" { | 434 | if candidate == "" { | ||
| @@ -402,6 +469,32 @@ func phaseIDError(state PlanState, action, phaseID string) string { | |||||
| 402 | return fmt.Sprintf("phaseId %q was not found for %s. Use the raw id from brackets without brackets. Valid phase ids: %s.", singleLine(candidate), action, validIDs) | 469 | return fmt.Sprintf("phaseId %q was not found for %s. Use the raw id from brackets without brackets. Valid phase ids: %s.", singleLine(candidate), action, validIDs) | ||
| 403 | } | 470 | } | ||
| 404 | 471 | ||||
| 472 | + | func formatValidTaskIDs(state PlanState) string { | |||
| 473 | + | parts := make([]string, 0) | |||
| 474 | + | for _, phase := range state.Phases { | |||
| 475 | + | for _, task := range phase.Tasks { | |||
| 476 | + | if !task.Checked { | |||
| 477 | + | parts = append(parts, fmt.Sprintf("%s (%s)", task.ID, displaySingleLine(phase.Title))) | |||
| 478 | + | } | |||
| 479 | + | } | |||
| 480 | + | } | |||
| 481 | + | if len(parts) == 0 { | |||
| 482 | + | return "none" | |||
| 483 | + | } | |||
| 484 | + | return strings.Join(parts, ", ") | |||
| 485 | + | } | |||
| 486 | + | ||||
| 487 | + | func taskIDError(state PlanState, action, taskID string) string { | |||
| 488 | + | candidate := normalizeOptionalText(taskID) | |||
| 489 | + | validIDs := formatValidTaskIDs(state) | |||
| 490 | + | if candidate == "" { | |||
| 491 | + | return fmt.Sprintf("taskId is required for %s. Use a valid task id. Valid incomplete task ids: %s.", action, validIDs) | |||
| 492 | + | } | |||
| 493 | + | return fmt.Sprintf("taskId %q was not found for %s. Use the task id from brackets. Valid incomplete task ids: %s.", displaySingleLine(candidate), action, validIDs) | |||
| 494 | + | } | |||
| 495 | + | ||||
| 496 | + | // findTask returns pointers into state.Phases and their Tasks, aliasing the | |||
| 497 | + | // caller's slice backing arrays so mutations are applied to the caller's state. | |||
| 405 | func findTask(state PlanState, taskID, phaseID string) (phase *Phase, task *PhaseTask) { | 498 | func findTask(state PlanState, taskID, phaseID string) (phase *Phase, task *PhaseTask) { | ||
| 406 | if taskID == "" { | 499 | if taskID == "" { | ||
| 407 | return nil, nil | 500 | return nil, nil | ||
| @@ -590,13 +683,16 @@ func applyAction(state PlanState, params toolArgs) (PlanState, string, error) { | |||||
| 590 | phase.Tasks = append(phase.Tasks, PhaseTask{ID: nextTaskID(&nextState), Text: taskText}) | 683 | phase.Tasks = append(phase.Tasks, PhaseTask{ID: nextTaskID(&nextState), Text: taskText}) | ||
| 591 | return nextState, "Added task to " + phase.Title, nil | 684 | return nextState, "Added task to " + phase.Title, nil | ||
| 592 | case actionUpdateTask: | 685 | case actionUpdateTask: | ||
| 686 | + | if params.TaskID == "" { | |||
| 687 | + | return state, "Task not updated", errors.New("taskId is required for update_task") | |||
| 688 | + | } | |||
| 593 | _, task := findTask(nextState, params.TaskID, params.PhaseID) | 689 | _, task := findTask(nextState, params.TaskID, params.PhaseID) | ||
| 594 | taskText := "" | 690 | taskText := "" | ||
| 595 | if params.TaskText != nil { | 691 | if params.TaskText != nil { | ||
| 596 | taskText = normalizeOptionalText(*params.TaskText) | 692 | taskText = normalizeOptionalText(*params.TaskText) | ||
| 597 | } | 693 | } | ||
| 598 | if task == nil { | 694 | if task == nil { | ||
| 599 | - | return state, "Task not updated", errors.New("taskId was not found") | 695 | + | return state, "Task not updated", errors.New(taskIDError(nextState, actionUpdateTask, params.TaskID)) |
| 600 | } | 696 | } | ||
| 601 | if taskText == "" { | 697 | if taskText == "" { | ||
| 602 | return state, "Task not updated", errors.New("taskText is required for update_task") | 698 | return state, "Task not updated", errors.New("taskText is required for update_task") | ||
| @@ -627,9 +723,12 @@ func applyAction(state PlanState, params toolArgs) (PlanState, string, error) { | |||||
| 627 | nextState.CurrentPhaseID = phase.ID | 723 | nextState.CurrentPhaseID = phase.ID | ||
| 628 | return nextState, "Current phase set to " + phase.Title, nil | 724 | return nextState, "Current phase set to " + phase.Title, nil | ||
| 629 | case actionSetTaskChecked: | 725 | case actionSetTaskChecked: | ||
| 726 | + | if params.TaskID == "" { | |||
| 727 | + | return state, "Task not updated", errors.New("taskId is required for set_task_checked") | |||
| 728 | + | } | |||
| 630 | phase, task := findTask(nextState, params.TaskID, params.PhaseID) | 729 | phase, task := findTask(nextState, params.TaskID, params.PhaseID) | ||
| 631 | if task == nil { | 730 | if task == nil { | ||
| 632 | - | return state, "Task not updated", errors.New("taskId was not found") | 731 | + | return state, "Task not updated", errors.New(taskIDError(nextState, actionSetTaskChecked, params.TaskID)) |
| 633 | } | 732 | } | ||
| 634 | if params.Checked == nil { | 733 | if params.Checked == nil { | ||
| 635 | return state, "Task not updated", errors.New("checked must be provided for set_task_checked") | 734 | return state, "Task not updated", errors.New("checked must be provided for set_task_checked") | ||
| @@ -697,8 +796,8 @@ func formatRemainingTaskCount(count int) string { | |||||
| 697 | return fmt.Sprintf("%d remaining", count) | 796 | return fmt.Sprintf("%d remaining", count) | ||
| 698 | } | 797 | } | ||
| 699 | 798 | ||||
| 700 | - | func formatPhaseTitle(phase Phase) string { return truncatePlain(singleLine(phase.Title), 120) } | 799 | + | func formatPhaseTitle(phase Phase) string { return truncatePlain(displaySingleLine(phase.Title), 120) } |
| 701 | - | func formatTaskText(task PhaseTask) string { return truncatePlain(singleLine(task.Text), 180) } | 800 | + | func formatTaskText(task PhaseTask) string { return truncatePlain(displaySingleLine(task.Text), 180) } |
| 702 | 801 | ||||
| 703 | func formatTurnContextID(id string) string { return truncatePlain(singleLine(id), 96) } | 802 | func formatTurnContextID(id string) string { return truncatePlain(singleLine(id), 96) } | ||
| 704 | 803 | ||||
| @@ -708,7 +807,7 @@ func appendIncompleteTaskLines(lines *[]string, tasks []PhaseTask, visibleCount | |||||
| 708 | visible = visible[:visibleCount] | 807 | visible = visible[:visibleCount] | ||
| 709 | } | 808 | } | ||
| 710 | for _, task := range visible { | 809 | for _, task := range visible { | ||
| 711 | - | *lines = append(*lines, fmt.Sprintf("%s[ ] %s [%s]", indent, formatTaskText(task), task.ID)) | 810 | + | *lines = append(*lines, fmt.Sprintf("%s[ ] %s [%s]", indent, formatTaskText(task), formatTurnContextID(task.ID))) |
| 712 | } | 811 | } | ||
| 713 | if omitted := len(tasks) - len(visible); omitted > 0 { | 812 | if omitted := len(tasks) - len(visible); omitted > 0 { | ||
| 714 | *lines = append(*lines, fmt.Sprintf("%s... %d more incomplete task(s) omitted; call tasked_phases get_status for the full checklist.", indent, omitted)) | 813 | *lines = append(*lines, fmt.Sprintf("%s... %d more incomplete task(s) omitted; call tasked_phases get_status for the full checklist.", indent, omitted)) | ||
| @@ -721,7 +820,7 @@ func buildSummary(state PlanState) string { | |||||
| 721 | } | 820 | } | ||
| 722 | lines := make([]string, 0) | 821 | lines := make([]string, 0) | ||
| 723 | if state.Spec != "" { | 822 | if state.Spec != "" { | ||
| 724 | - | lines = append(lines, "Spec:", state.Spec, "") | 823 | + | lines = append(lines, "Spec:", sanitizeDisplayText(state.Spec), "") |
| 725 | } | 824 | } | ||
| 726 | if len(state.Phases) == 0 { | 825 | if len(state.Phases) == 0 { | ||
| 727 | return strings.Join(append(lines, "Phases: (none)"), "\n") | 826 | return strings.Join(append(lines, "Phases: (none)"), "\n") | ||
| @@ -733,7 +832,7 @@ func buildSummary(state PlanState) string { | |||||
| 733 | if closedSummary == "" { | 832 | if closedSummary == "" { | ||
| 734 | closedSummary = "all phases complete" | 833 | closedSummary = "all phases complete" | ||
| 735 | } | 834 | } | ||
| 736 | - | lines = append(lines, "Plan closed: "+closedSummary) | 835 | + | lines = append(lines, "Plan closed: "+displaySingleLine(closedSummary)) |
| 737 | } | 836 | } | ||
| 738 | lines = append(lines, "Phases:") | 837 | lines = append(lines, "Phases:") | ||
| 739 | for _, phase := range state.Phases { | 838 | for _, phase := range state.Phases { | ||
| @@ -748,9 +847,9 @@ func buildSummary(state PlanState) string { | |||||
| 748 | } | 847 | } | ||
| 749 | goalSuffix := "" | 848 | goalSuffix := "" | ||
| 750 | if phase.Goal != "" { | 849 | if phase.Goal != "" { | ||
| 751 | - | goalSuffix = " - " + phase.Goal | 850 | + | goalSuffix = " - " + displaySingleLine(phase.Goal) |
| 752 | } | 851 | } | ||
| 753 | - | lines = append(lines, fmt.Sprintf("%s %s %s [%s] (%d/%d)%s", currentMarker, phaseMarker, phase.Title, phase.ID, phaseDone, phaseTotal, goalSuffix)) | 852 | + | lines = append(lines, fmt.Sprintf("%s %s %s [%s] (%d/%d)%s", currentMarker, phaseMarker, formatPhaseTitle(phase), formatTurnContextID(phase.ID), phaseDone, phaseTotal, goalSuffix)) |
| 754 | if len(phase.Tasks) == 0 { | 853 | if len(phase.Tasks) == 0 { | ||
| 755 | lines = append(lines, " - No tasks yet") | 854 | lines = append(lines, " - No tasks yet") | ||
| 756 | continue | 855 | continue | ||
| @@ -760,7 +859,7 @@ func buildSummary(state PlanState) string { | |||||
| 760 | if task.Checked { | 859 | if task.Checked { | ||
| 761 | marker = "[x]" | 860 | marker = "[x]" | ||
| 762 | } | 861 | } | ||
| 763 | - | lines = append(lines, fmt.Sprintf(" %s %s [%s]", marker, task.Text, task.ID)) | 862 | + | lines = append(lines, fmt.Sprintf(" %s %s [%s]", marker, formatTaskText(task), formatTurnContextID(task.ID))) |
| 764 | } | 863 | } | ||
| 765 | } | 864 | } | ||
| 766 | return strings.Join(lines, "\n") | 865 | return strings.Join(lines, "\n") | ||
| @@ -772,7 +871,7 @@ func buildCompactSummary(state PlanState) string { | |||||
| 772 | } | 871 | } | ||
| 773 | lines := make([]string, 0) | 872 | lines := make([]string, 0) | ||
| 774 | if state.Spec != "" { | 873 | if state.Spec != "" { | ||
| 775 | - | lines = append(lines, "Spec: "+truncatePlain(singleLine(state.Spec), 220)) | 874 | + | lines = append(lines, "Spec: "+truncatePlain(displaySingleLine(state.Spec), 220)) |
| 776 | } | 875 | } | ||
| 777 | done, total := getPlanProgress(state) | 876 | done, total := getPlanProgress(state) | ||
| 778 | lines = append(lines, fmt.Sprintf("Progress: %d/%d tasks checked", done, total)) | 877 | lines = append(lines, fmt.Sprintf("Progress: %d/%d tasks checked", done, total)) | ||
| @@ -781,16 +880,16 @@ func buildCompactSummary(state PlanState) string { | |||||
| 781 | if closedSummary == "" { | 880 | if closedSummary == "" { | ||
| 782 | closedSummary = "all phases complete" | 881 | closedSummary = "all phases complete" | ||
| 783 | } | 882 | } | ||
| 784 | - | lines = append(lines, "Plan closed: "+closedSummary) | 883 | + | lines = append(lines, "Plan closed: "+displaySingleLine(closedSummary)) |
| 785 | return strings.Join(lines, "\n") | 884 | return strings.Join(lines, "\n") | ||
| 786 | } | 885 | } | ||
| 787 | if currentPhase := getActivePhase(state); currentPhase != nil { | 886 | if currentPhase := getActivePhase(state); currentPhase != nil { | ||
| 788 | incompleteTasks := getIncompleteTasks(*currentPhase) | 887 | incompleteTasks := getIncompleteTasks(*currentPhase) | ||
| 789 | goalSuffix := "" | 888 | goalSuffix := "" | ||
| 790 | if currentPhase.Goal != "" { | 889 | if currentPhase.Goal != "" { | ||
| 791 | - | goalSuffix = " - " + truncatePlain(singleLine(currentPhase.Goal), 120) | 890 | + | goalSuffix = " - " + truncatePlain(displaySingleLine(currentPhase.Goal), 120) |
| 792 | } | 891 | } | ||
| 793 | - | lines = append(lines, fmt.Sprintf("Current phase: %s [%s] (%s)%s", formatPhaseTitle(*currentPhase), currentPhase.ID, formatRemainingTaskCount(len(incompleteTasks)), goalSuffix)) | 892 | + | lines = append(lines, fmt.Sprintf("Current phase: %s [%s] (%s)%s", formatPhaseTitle(*currentPhase), formatTurnContextID(currentPhase.ID), formatRemainingTaskCount(len(incompleteTasks)), goalSuffix)) |
| 794 | if len(incompleteTasks) > 0 { | 893 | if len(incompleteTasks) > 0 { | ||
| 795 | lines = append(lines, "Incomplete tasks:") | 894 | lines = append(lines, "Incomplete tasks:") | ||
| 796 | appendIncompleteTaskLines(&lines, incompleteTasks, compactVisibleIncompleteTasks, " ") | 895 | appendIncompleteTaskLines(&lines, incompleteTasks, compactVisibleIncompleteTasks, " ") | ||
| @@ -820,7 +919,7 @@ func buildTurnContext(state PlanState) string { | |||||
| 820 | fmt.Sprintf("Current phase: %s [%s]", formatPhaseTitle(*phase), formatTurnContextID(phase.ID)), | 919 | fmt.Sprintf("Current phase: %s [%s]", formatPhaseTitle(*phase), formatTurnContextID(phase.ID)), | ||
| 821 | } | 920 | } | ||
| 822 | if phase.Goal != "" { | 921 | if phase.Goal != "" { | ||
| 823 | - | lines = append(lines, "Goal: "+truncatePlain(singleLine(phase.Goal), 160)) | 922 | + | lines = append(lines, "Goal: "+truncatePlain(displaySingleLine(phase.Goal), 160)) |
| 824 | } | 923 | } | ||
| 825 | incompleteTasks := getIncompleteTasks(*phase) | 924 | incompleteTasks := getIncompleteTasks(*phase) | ||
| 826 | if len(incompleteTasks) == 0 { | 925 | if len(incompleteTasks) == 0 { | ||
| @@ -828,16 +927,7 @@ func buildTurnContext(state PlanState) string { | |||||
| 828 | return truncatePlain(strings.Join(lines, "\n"), maxTurnContextChars) | 927 | return truncatePlain(strings.Join(lines, "\n"), maxTurnContextChars) | ||
| 829 | } | 928 | } | ||
| 830 | lines = append(lines, "Incomplete tasks:") | 929 | lines = append(lines, "Incomplete tasks:") | ||
| 831 | - | visible := incompleteTasks | 930 | + | appendIncompleteTaskLines(&lines, incompleteTasks, turnContextVisibleIncompleteTasks, " ") |
| 832 | - | if len(visible) > turnContextVisibleIncompleteTasks { | |||
| 833 | - | visible = visible[:turnContextVisibleIncompleteTasks] | |||
| 834 | - | } | |||
| 835 | - | for _, task := range visible { | |||
| 836 | - | lines = append(lines, fmt.Sprintf(" [ ] %s [%s]", formatTaskText(task), formatTurnContextID(task.ID))) | |||
| 837 | - | } | |||
| 838 | - | if omitted := len(incompleteTasks) - len(visible); omitted > 0 { | |||
| 839 | - | lines = append(lines, fmt.Sprintf(" ... %d more incomplete task(s) omitted; call tasked_phases get_status for the full checklist.", omitted)) | |||
| 840 | - | } | |||
| 841 | return truncatePlain(strings.Join(lines, "\n"), maxTurnContextChars) | 931 | return truncatePlain(strings.Join(lines, "\n"), maxTurnContextChars) | ||
| 842 | } | 932 | } | ||
| 843 | 933 | ||||
| @@ -869,7 +959,7 @@ func buildViewLines(state PlanState) []string { | |||||
| 869 | } | 959 | } | ||
| 870 | if state.Spec != "" { | 960 | if state.Spec != "" { | ||
| 871 | lines = append(lines, " Spec") | 961 | lines = append(lines, " Spec") | ||
| 872 | - | for _, specLine := range strings.Split(state.Spec, "\n") { | 962 | + | for _, specLine := range strings.Split(sanitizeDisplayText(state.Spec), "\n") { |
| 873 | lines = append(lines, " "+specLine) | 963 | lines = append(lines, " "+specLine) | ||
| 874 | } | 964 | } | ||
| 875 | lines = append(lines, "") | 965 | lines = append(lines, "") | ||
| @@ -882,7 +972,7 @@ func buildViewLines(state PlanState) []string { | |||||
| 882 | closedSummary = "all phases complete" | 972 | closedSummary = "all phases complete" | ||
| 883 | } | 973 | } | ||
| 884 | lines = append(lines, | 974 | lines = append(lines, | ||
| 885 | - | " Closed: "+closedSummary, | 975 | + | " Closed: "+displaySingleLine(closedSummary), |
| 886 | " New work should create a fresh plan instead of extending this one.", | 976 | " New work should create a fresh plan instead of extending this one.", | ||
| 887 | ) | 977 | ) | ||
| 888 | } | 978 | } | ||
| @@ -897,9 +987,9 @@ func buildViewLines(state PlanState) []string { | |||||
| 897 | if phase.ID == state.CurrentPhaseID { | 987 | if phase.ID == state.CurrentPhaseID { | ||
| 898 | currentTitle = "> " + currentTitle | 988 | currentTitle = "> " + currentTitle | ||
| 899 | } | 989 | } | ||
| 900 | - | lines = append(lines, fmt.Sprintf(" %s %s [%s] (%d/%d)", marker, currentTitle, phase.ID, phaseDone, phaseTotal)) | 990 | + | lines = append(lines, fmt.Sprintf(" %s %s [%s] (%d/%d)", marker, displaySingleLine(currentTitle), formatTurnContextID(phase.ID), phaseDone, phaseTotal)) |
| 901 | if phase.Goal != "" { | 991 | if phase.Goal != "" { | ||
| 902 | - | lines = append(lines, " "+phase.Goal) | 992 | + | lines = append(lines, " "+displaySingleLine(phase.Goal)) |
| 903 | } | 993 | } | ||
| 904 | if len(phase.Tasks) == 0 { | 994 | if len(phase.Tasks) == 0 { | ||
| 905 | lines = append(lines, " - No tasks yet") | 995 | lines = append(lines, " - No tasks yet") | ||
| @@ -910,7 +1000,7 @@ func buildViewLines(state PlanState) []string { | |||||
| 910 | if task.Checked { | 1000 | if task.Checked { | ||
| 911 | taskMarker = "[x]" | 1001 | taskMarker = "[x]" | ||
| 912 | } | 1002 | } | ||
| 913 | - | lines = append(lines, fmt.Sprintf(" %s %s [%s]", taskMarker, task.Text, task.ID)) | 1003 | + | lines = append(lines, fmt.Sprintf(" %s %s [%s]", taskMarker, formatTaskText(task), formatTurnContextID(task.ID))) |
| 914 | } | 1004 | } | ||
| 915 | lines = append(lines, "") | 1005 | lines = append(lines, "") | ||
| 916 | } | 1006 | } | ||
| @@ -923,6 +1013,9 @@ type stateStore struct { | |||||
| 923 | path string | 1013 | path string | ||
| 924 | loaded bool | 1014 | loaded bool | ||
| 925 | loadErr error | 1015 | loadErr error | ||
| 1016 | + | // pathErr records a persistence fault independently of the active | |||
| 1017 | + | // session state. Activating a session must never clear this fault. | |||
| 1018 | + | pathErr error | |||
| 926 | } | 1019 | } | ||
| 927 | 1020 | ||||
| 928 | func newStateStore() *stateStore { | 1021 | func newStateStore() *stateStore { | ||
| @@ -944,15 +1037,19 @@ func (s *stateStore) load(dataDir, cwd string) error { | |||||
| 944 | s.state = createEmptyState() | 1037 | s.state = createEmptyState() | ||
| 945 | if strings.TrimSpace(dataDir) == "" { | 1038 | if strings.TrimSpace(dataDir) == "" { | ||
| 946 | s.loadErr = errors.New("host did not provide extension data_dir") | 1039 | s.loadErr = errors.New("host did not provide extension data_dir") | ||
| 1040 | + | s.pathErr = s.loadErr | |||
| 947 | return s.loadErr | 1041 | return s.loadErr | ||
| 948 | } | 1042 | } | ||
| 949 | s.path = projectStatePath(dataDir, cwd) | 1043 | s.path = projectStatePath(dataDir, cwd) | ||
| 950 | if err := os.MkdirAll(filepath.Dir(s.path), 0o700); err != nil { | 1044 | if err := os.MkdirAll(filepath.Dir(s.path), 0o700); err != nil { | ||
| 951 | s.loadErr = fmt.Errorf("create state directory: %w", err) | 1045 | s.loadErr = fmt.Errorf("create state directory: %w", err) | ||
| 1046 | + | s.pathErr = s.loadErr | |||
| 952 | return s.loadErr | 1047 | return s.loadErr | ||
| 953 | } | 1048 | } | ||
| 1049 | + | // #nosec G302 -- this is a directory; 0700 is the minimum mode that permits traversal. | |||
| 954 | if err := os.Chmod(filepath.Dir(s.path), 0o700); err != nil { | 1050 | if err := os.Chmod(filepath.Dir(s.path), 0o700); err != nil { | ||
| 955 | s.loadErr = fmt.Errorf("restrict state directory: %w", err) | 1051 | s.loadErr = fmt.Errorf("restrict state directory: %w", err) | ||
| 1052 | + | s.pathErr = s.loadErr | |||
| 956 | return s.loadErr | 1053 | return s.loadErr | ||
| 957 | } | 1054 | } | ||
| 958 | data, err := os.ReadFile(s.path) | 1055 | data, err := os.ReadFile(s.path) | ||
| @@ -961,10 +1058,12 @@ func (s *stateStore) load(dataDir, cwd string) error { | |||||
| 961 | } | 1058 | } | ||
| 962 | if err != nil { | 1059 | if err != nil { | ||
| 963 | s.loadErr = fmt.Errorf("read state: %w", err) | 1060 | s.loadErr = fmt.Errorf("read state: %w", err) | ||
| 1061 | + | s.pathErr = s.loadErr | |||
| 964 | return s.loadErr | 1062 | return s.loadErr | ||
| 965 | } | 1063 | } | ||
| 966 | if err := os.Chmod(s.path, 0o600); err != nil { | 1064 | if err := os.Chmod(s.path, 0o600); err != nil { | ||
| 967 | s.loadErr = fmt.Errorf("restrict state file: %w", err) | 1065 | s.loadErr = fmt.Errorf("restrict state file: %w", err) | ||
| 1066 | + | s.pathErr = s.loadErr | |||
| 968 | return s.loadErr | 1067 | return s.loadErr | ||
| 969 | } | 1068 | } | ||
| 970 | if len(data) == 0 { | 1069 | if len(data) == 0 { | ||
| @@ -973,6 +1072,7 @@ func (s *stateStore) load(dataDir, cwd string) error { | |||||
| 973 | var state PlanState | 1072 | var state PlanState | ||
| 974 | if err := json.Unmarshal(data, &state); err != nil { | 1073 | if err := json.Unmarshal(data, &state); err != nil { | ||
| 975 | s.loadErr = fmt.Errorf("parse state: %w", err) | 1074 | s.loadErr = fmt.Errorf("parse state: %w", err) | ||
| 1075 | + | s.pathErr = s.loadErr | |||
| 976 | return s.loadErr | 1076 | return s.loadErr | ||
| 977 | } | 1077 | } | ||
| 978 | s.state = ensureState(state) | 1078 | s.state = ensureState(state) | ||
| @@ -988,8 +1088,8 @@ func (s *stateStore) activateSession(raw json.RawMessage) error { | |||||
| 988 | if !s.loaded { | 1088 | if !s.loaded { | ||
| 989 | return errors.New("state has not been loaded from hello") | 1089 | return errors.New("state has not been loaded from hello") | ||
| 990 | } | 1090 | } | ||
| 991 | - | if s.path == "" && s.loadErr != nil { | 1091 | + | if s.pathErr != nil { |
| 992 | - | return s.loadErr | 1092 | + | return s.pathErr |
| 993 | } | 1093 | } | ||
| 994 | if len(raw) == 0 || strings.TrimSpace(string(raw)) == "null" { | 1094 | if len(raw) == 0 || strings.TrimSpace(string(raw)) == "null" { | ||
| 995 | s.state = createEmptyState() | 1095 | s.state = createEmptyState() | ||
| @@ -1023,6 +1123,9 @@ func (s *stateStore) transact(fn func(PlanState) (PlanState, string, error)) (Pl | |||||
| 1023 | if !s.loaded { | 1123 | if !s.loaded { | ||
| 1024 | return PlanState{}, "", errors.New("state has not been loaded from hello") | 1124 | return PlanState{}, "", errors.New("state has not been loaded from hello") | ||
| 1025 | } | 1125 | } | ||
| 1126 | + | if s.pathErr != nil { | |||
| 1127 | + | return cloneState(s.state), "", s.pathErr | |||
| 1128 | + | } | |||
| 1026 | if s.loadErr != nil { | 1129 | if s.loadErr != nil { | ||
| 1027 | return cloneState(s.state), "", s.loadErr | 1130 | return cloneState(s.state), "", s.loadErr | ||
| 1028 | } | 1131 | } | ||
M examples/extensions/tasked-phases/state_test.go +73 -0
| @@ -251,6 +251,46 @@ func TestTurnContextIsBoundedForUntrustedIdentifiers(t *testing.T) { | |||
| 251 | 251 | if !strings.Contains(context, "Tasked phases focus:") || !strings.Contains(context, "Current phase:") || !strings.Contains(context, "Goal: Reduce repeated context") { | |
| 252 | 252 | t.Fatalf("bounded turn context lost its useful focus: %q", context) | |
| 253 | 253 | } | |
| 254 | + | if !strings.Contains(context, "Remaining implementation task") { | |
| 255 | + | t.Fatalf("per-field id bounding did not leave room for task lines: %q", context) | |
| 256 | + | } | |
| 257 | + | for _, line := range strings.Split(context, "\n") { | |
| 258 | + | if got := len([]rune(line)); got > 400 { | |
| 259 | + | t.Fatalf("unbounded field produced a %d-rune line: %q", got, line) | |
| 260 | + | } | |
| 261 | + | } | |
| 262 | + | } | |
| 263 | + | ||
| 264 | + | func TestDisplayRenderingSanitizesUntrustedTextWithoutMutatingState(t *testing.T) { | |
| 265 | + | state := sampleState() | |
| 266 | + | state.Spec = "spec \x1b[31mred\x1b[0m\x00" | |
| 267 | + | state.Phases[1].Title = "title \x1b]0;evil\a" | |
| 268 | + | state.Phases[1].Goal = "goal\r\t\x1b[2J" | |
| 269 | + | state.Phases[1].Tasks[0].Text = "task \x1b[?25l\x7f" | |
| 270 | + | original := state | |
| 271 | + | ||
| 272 | + | for _, rendered := range []string{ | |
| 273 | + | buildSummary(state), buildCompactSummary(state), buildTurnContext(state), strings.Join(buildViewLines(state), "\n"), | |
| 274 | + | } { | |
| 275 | + | if strings.Contains(rendered, "\x1b") || strings.ContainsRune(rendered, '\x00') || strings.ContainsRune(rendered, '\x7f') || strings.ContainsRune(rendered, '\r') || strings.ContainsRune(rendered, '\t') { | |
| 276 | + | t.Fatalf("rendered text contains terminal/control injection: %q", rendered) | |
| 277 | + | } | |
| 278 | + | } | |
| 279 | + | if state.Spec != original.Spec || state.Phases[1].Title != original.Phases[1].Title || state.Phases[1].Tasks[0].Text != original.Phases[1].Tasks[0].Text { | |
| 280 | + | t.Fatal("render sanitization mutated persisted plan state") | |
| 281 | + | } | |
| 282 | + | } | |
| 283 | + | ||
| 284 | + | func TestTaskErrorsDistinguishMissingAndUnknownIDs(t *testing.T) { | |
| 285 | + | state := sampleState() | |
| 286 | + | for _, action := range []string{actionUpdateTask, actionSetTaskChecked} { | |
| 287 | + | if _, _, err := applyAction(state, toolArgs{Action: action}); err == nil || !strings.Contains(err.Error(), "taskId is required") { | |
| 288 | + | t.Fatalf("%s missing task id error = %v", action, err) | |
| 289 | + | } | |
| 290 | + | if _, _, err := applyAction(state, toolArgs{Action: action, TaskID: "missing"}); err == nil || !strings.Contains(err.Error(), "Valid incomplete task ids: task-4") { | |
| 291 | + | t.Fatalf("%s unknown task id error = %v", action, err) | |
| 292 | + | } | |
| 293 | + | } | |
| 254 | 294 | } | |
| 255 | 295 | ||
| 256 | 296 | func TestPanelRendersSpecGoalsTasksAndEmptyState(t *testing.T) { | |
| @@ -350,6 +390,39 @@ func TestStatePersistenceIsProjectScopedAtomicAndRestrictive(t *testing.T) { | |||
| 350 | 390 | } | |
| 351 | 391 | } | |
| 352 | 392 | ||
| 393 | + | func TestStateStoreDoesNotOverwriteCorruptStateAfterSessionActivation(t *testing.T) { | |
| 394 | + | dataDir := t.TempDir() | |
| 395 | + | cwd := "/projects/corrupt" | |
| 396 | + | path := projectStatePath(dataDir, cwd) | |
| 397 | + | original := []byte("not valid plan json") | |
| 398 | + | if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { | |
| 399 | + | t.Fatal(err) | |
| 400 | + | } | |
| 401 | + | if err := os.WriteFile(path, original, 0o600); err != nil { | |
| 402 | + | t.Fatal(err) | |
| 403 | + | } | |
| 404 | + | ||
| 405 | + | store := newStateStore() | |
| 406 | + | if err := store.load(dataDir, cwd); err == nil { | |
| 407 | + | t.Fatal("corrupt state unexpectedly loaded") | |
| 408 | + | } | |
| 409 | + | if err := store.activateSession(nil); err == nil { | |
| 410 | + | t.Fatal("session activation cleared the corrupt-state fault") | |
| 411 | + | } | |
| 412 | + | if _, _, err := store.transact(func(current PlanState) (PlanState, string, error) { | |
| 413 | + | return applyAction(current, toolArgs{Action: actionClear}) | |
| 414 | + | }); err == nil { | |
| 415 | + | t.Fatal("transaction unexpectedly wrote through a corrupt-state fault") | |
| 416 | + | } | |
| 417 | + | got, err := os.ReadFile(path) | |
| 418 | + | if err != nil { | |
| 419 | + | t.Fatal(err) | |
| 420 | + | } | |
| 421 | + | if string(got) != string(original) { | |
| 422 | + | t.Fatalf("corrupt state bytes changed from %q to %q", original, got) | |
| 423 | + | } | |
| 424 | + | } | |
| 425 | + | ||
| 353 | 426 | func TestStateStoreSerializesConcurrentTransitions(t *testing.T) { | |
| 354 | 427 | store := newStateStore() | |
| 355 | 428 | if err := store.load(t.TempDir(), "/projects/concurrent"); err != nil { | |
| @@ -251,6 +251,46 @@ func TestTurnContextIsBoundedForUntrustedIdentifiers(t *testing.T) { | |||||
| 251 | if !strings.Contains(context, "Tasked phases focus:") || !strings.Contains(context, "Current phase:") || !strings.Contains(context, "Goal: Reduce repeated context") { | 251 | if !strings.Contains(context, "Tasked phases focus:") || !strings.Contains(context, "Current phase:") || !strings.Contains(context, "Goal: Reduce repeated context") { | ||
| 252 | t.Fatalf("bounded turn context lost its useful focus: %q", context) | 252 | t.Fatalf("bounded turn context lost its useful focus: %q", context) | ||
| 253 | } | 253 | } | ||
| 254 | + | if !strings.Contains(context, "Remaining implementation task") { | |||
| 255 | + | t.Fatalf("per-field id bounding did not leave room for task lines: %q", context) | |||
| 256 | + | } | |||
| 257 | + | for _, line := range strings.Split(context, "\n") { | |||
| 258 | + | if got := len([]rune(line)); got > 400 { | |||
| 259 | + | t.Fatalf("unbounded field produced a %d-rune line: %q", got, line) | |||
| 260 | + | } | |||
| 261 | + | } | |||
| 262 | + | } | |||
| 263 | + | ||||
| 264 | + | func TestDisplayRenderingSanitizesUntrustedTextWithoutMutatingState(t *testing.T) { | |||
| 265 | + | state := sampleState() | |||
| 266 | + | state.Spec = "spec \x1b[31mred\x1b[0m\x00" | |||
| 267 | + | state.Phases[1].Title = "title \x1b]0;evil\a" | |||
| 268 | + | state.Phases[1].Goal = "goal\r\t\x1b[2J" | |||
| 269 | + | state.Phases[1].Tasks[0].Text = "task \x1b[?25l\x7f" | |||
| 270 | + | original := state | |||
| 271 | + | ||||
| 272 | + | for _, rendered := range []string{ | |||
| 273 | + | buildSummary(state), buildCompactSummary(state), buildTurnContext(state), strings.Join(buildViewLines(state), "\n"), | |||
| 274 | + | } { | |||
| 275 | + | if strings.Contains(rendered, "\x1b") || strings.ContainsRune(rendered, '\x00') || strings.ContainsRune(rendered, '\x7f') || strings.ContainsRune(rendered, '\r') || strings.ContainsRune(rendered, '\t') { | |||
| 276 | + | t.Fatalf("rendered text contains terminal/control injection: %q", rendered) | |||
| 277 | + | } | |||
| 278 | + | } | |||
| 279 | + | if state.Spec != original.Spec || state.Phases[1].Title != original.Phases[1].Title || state.Phases[1].Tasks[0].Text != original.Phases[1].Tasks[0].Text { | |||
| 280 | + | t.Fatal("render sanitization mutated persisted plan state") | |||
| 281 | + | } | |||
| 282 | + | } | |||
| 283 | + | ||||
| 284 | + | func TestTaskErrorsDistinguishMissingAndUnknownIDs(t *testing.T) { | |||
| 285 | + | state := sampleState() | |||
| 286 | + | for _, action := range []string{actionUpdateTask, actionSetTaskChecked} { | |||
| 287 | + | if _, _, err := applyAction(state, toolArgs{Action: action}); err == nil || !strings.Contains(err.Error(), "taskId is required") { | |||
| 288 | + | t.Fatalf("%s missing task id error = %v", action, err) | |||
| 289 | + | } | |||
| 290 | + | if _, _, err := applyAction(state, toolArgs{Action: action, TaskID: "missing"}); err == nil || !strings.Contains(err.Error(), "Valid incomplete task ids: task-4") { | |||
| 291 | + | t.Fatalf("%s unknown task id error = %v", action, err) | |||
| 292 | + | } | |||
| 293 | + | } | |||
| 254 | } | 294 | } | ||
| 255 | 295 | ||||
| 256 | func TestPanelRendersSpecGoalsTasksAndEmptyState(t *testing.T) { | 296 | func TestPanelRendersSpecGoalsTasksAndEmptyState(t *testing.T) { | ||
| @@ -350,6 +390,39 @@ func TestStatePersistenceIsProjectScopedAtomicAndRestrictive(t *testing.T) { | |||||
| 350 | } | 390 | } | ||
| 351 | } | 391 | } | ||
| 352 | 392 | ||||
| 393 | + | func TestStateStoreDoesNotOverwriteCorruptStateAfterSessionActivation(t *testing.T) { | |||
| 394 | + | dataDir := t.TempDir() | |||
| 395 | + | cwd := "/projects/corrupt" | |||
| 396 | + | path := projectStatePath(dataDir, cwd) | |||
| 397 | + | original := []byte("not valid plan json") | |||
| 398 | + | if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { | |||
| 399 | + | t.Fatal(err) | |||
| 400 | + | } | |||
| 401 | + | if err := os.WriteFile(path, original, 0o600); err != nil { | |||
| 402 | + | t.Fatal(err) | |||
| 403 | + | } | |||
| 404 | + | ||||
| 405 | + | store := newStateStore() | |||
| 406 | + | if err := store.load(dataDir, cwd); err == nil { | |||
| 407 | + | t.Fatal("corrupt state unexpectedly loaded") | |||
| 408 | + | } | |||
| 409 | + | if err := store.activateSession(nil); err == nil { | |||
| 410 | + | t.Fatal("session activation cleared the corrupt-state fault") | |||
| 411 | + | } | |||
| 412 | + | if _, _, err := store.transact(func(current PlanState) (PlanState, string, error) { | |||
| 413 | + | return applyAction(current, toolArgs{Action: actionClear}) | |||
| 414 | + | }); err == nil { | |||
| 415 | + | t.Fatal("transaction unexpectedly wrote through a corrupt-state fault") | |||
| 416 | + | } | |||
| 417 | + | got, err := os.ReadFile(path) | |||
| 418 | + | if err != nil { | |||
| 419 | + | t.Fatal(err) | |||
| 420 | + | } | |||
| 421 | + | if string(got) != string(original) { | |||
| 422 | + | t.Fatalf("corrupt state bytes changed from %q to %q", original, got) | |||
| 423 | + | } | |||
| 424 | + | } | |||
| 425 | + | ||||
| 353 | func TestStateStoreSerializesConcurrentTransitions(t *testing.T) { | 426 | func TestStateStoreSerializesConcurrentTransitions(t *testing.T) { | ||
| 354 | store := newStateStore() | 427 | store := newStateStore() | ||
| 355 | if err := store.load(t.TempDir(), "/projects/concurrent"); err != nil { | 428 | if err := store.load(t.TempDir(), "/projects/concurrent"); err != nil { | ||
M examples/skills/tasked-phases/SKILL.md +1 -1
| @@ -16,7 +16,7 @@ Use the `tasked_phases` tool as the source of truth for the spec, phases, curren | |||
| 16 | 16 | 5. Set the focus with `set_current_phase` and pass the target `phaseId`. | |
| 17 | 17 | 6. Call `set_task_checked` immediately after each checklist task is completed. | |
| 18 | 18 | 7. Use `set_phase_checked` to complete or reopen every task in a phase. The phase must already contain at least one task. | |
| 19 | - | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` before creating a new plan. | |
| 19 | + | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` to discard the plan, or call `set_spec` immediately followed by `replace_plan` to record a new spec and a new set of phases. | |
| 20 | 20 | ||
| 21 | 21 | ## Planning rules | |
| 22 | 22 | ||
| @@ -16,7 +16,7 @@ Use the `tasked_phases` tool as the source of truth for the spec, phases, curren | |||||
| 16 | 5. Set the focus with `set_current_phase` and pass the target `phaseId`. | 16 | 5. Set the focus with `set_current_phase` and pass the target `phaseId`. | ||
| 17 | 6. Call `set_task_checked` immediately after each checklist task is completed. | 17 | 6. Call `set_task_checked` immediately after each checklist task is completed. | ||
| 18 | 7. Use `set_phase_checked` to complete or reopen every task in a phase. The phase must already contain at least one task. | 18 | 7. Use `set_phase_checked` to complete or reopen every task in a phase. The phase must already contain at least one task. | ||
| 19 | - | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` before creating a new plan. | 19 | + | 8. When every task is checked, treat the plan as closed. For unrelated work, call `clear` to discard the plan, or call `set_spec` immediately followed by `replace_plan` to record a new spec and a new set of phases. |
| 20 | 20 | ||||
| 21 | ## Planning rules | 21 | ## Planning rules | ||
| 22 | 22 | ||||
M packages/agent/build.go +16 -5
| @@ -61,6 +61,10 @@ type Resolved struct { | |||
| 61 | 61 | // messages to the provider transcript. | |
| 62 | 62 | ContextFiles []ContextFile | |
| 63 | 63 | ||
| 64 | + | // skillsEnabled is true when discovery ran, even when it found no | |
| 65 | + | // skills. It is false only for an explicit clean-room disable. | |
| 66 | + | skillsEnabled bool | |
| 67 | + | ||
| 64 | 68 | // Bookkeeping for MergeExtensionTools. Captured at Resolve time | |
| 65 | 69 | // so the system prompt can be rebuilt later without re-running | |
| 66 | 70 | // resolve. | |
| @@ -93,13 +97,17 @@ func (r *Resolved) MergeExtensionTools(mgr ExtensionToolSource) { | |||
| 93 | 97 | changed = true | |
| 94 | 98 | } | |
| 95 | 99 | ||
| 96 | - | // A nil SkillTool means skill discovery was explicitly disabled (for | |
| 97 | - | // example --no-skill or a profile with inheritSkills=false). Bundled | |
| 98 | - | // extension skills must honor that same clean-room switch. | |
| 99 | - | if source, ok := mgr.(ExtensionSkillSource); ok && r.SkillTool != nil { | |
| 100 | + | // A nil SkillTool can mean discovery ran and found nothing, so use the | |
| 101 | + | // explicit disable flag rather than the discovery result as the gate. | |
| 102 | + | if source, ok := mgr.(ExtensionSkillSource); ok && r.skillsEnabled { | |
| 100 | 103 | if bundled := source.Skills(); len(bundled) > 0 { | |
| 101 | 104 | merged := mergeExtensionSkills(r.SkillTool, bundled) | |
| 102 | - | r.SkillTool.SetSkills(merged) | |
| 105 | + | if r.SkillTool == nil { | |
| 106 | + | r.SkillTool = skills.NewTool(merged) | |
| 107 | + | r.ToolRegistry[r.SkillTool.Name()] = r.SkillTool | |
| 108 | + | } else { | |
| 109 | + | r.SkillTool.SetSkills(merged) | |
| 110 | + | } | |
| 103 | 111 | append_ := append([]string(nil), r.systemAppend...) | |
| 104 | 112 | if r.skillAddendum != "" { | |
| 105 | 113 | filtered := append_[:0] | |
| @@ -701,8 +709,10 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||
| 701 | 709 | discovered []*skills.Skill | |
| 702 | 710 | skillTool *skills.Tool | |
| 703 | 711 | skillAddendum string | |
| 712 | + | skillsEnabled bool | |
| 704 | 713 | ) | |
| 705 | 714 | if !args.NoSkill && (selectedProfile == nil || selectedProfile.InheritSkills == nil || *selectedProfile.InheritSkills) { | |
| 715 | + | skillsEnabled = true | |
| 706 | 716 | homeDir, _ := os.UserHomeDir() | |
| 707 | 717 | discovered, _ = skills.Discover(ZotHome(), args.CWD, homeDir, args.WithSkills) | |
| 708 | 718 | if len(discovered) > 0 { | |
| @@ -789,6 +799,7 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||
| 789 | 799 | MaxOutput: resolvedModel.MaxOutput, | |
| 790 | 800 | Sandbox: sandbox, | |
| 791 | 801 | SkillTool: skillTool, | |
| 802 | + | skillsEnabled: skillsEnabled, | |
| 792 | 803 | ContextFiles: contextFiles, | |
| 793 | 804 | systemAppend: append_, | |
| 794 | 805 | systemCustom: custom, | |
| @@ -61,6 +61,10 @@ type Resolved struct { | |||||
| 61 | // messages to the provider transcript. | 61 | // messages to the provider transcript. | ||
| 62 | ContextFiles []ContextFile | 62 | ContextFiles []ContextFile | ||
| 63 | 63 | ||||
| 64 | + | // skillsEnabled is true when discovery ran, even when it found no | |||
| 65 | + | // skills. It is false only for an explicit clean-room disable. | |||
| 66 | + | skillsEnabled bool | |||
| 67 | + | ||||
| 64 | // Bookkeeping for MergeExtensionTools. Captured at Resolve time | 68 | // Bookkeeping for MergeExtensionTools. Captured at Resolve time | ||
| 65 | // so the system prompt can be rebuilt later without re-running | 69 | // so the system prompt can be rebuilt later without re-running | ||
| 66 | // resolve. | 70 | // resolve. | ||
| @@ -93,13 +97,17 @@ func (r *Resolved) MergeExtensionTools(mgr ExtensionToolSource) { | |||||
| 93 | changed = true | 97 | changed = true | ||
| 94 | } | 98 | } | ||
| 95 | 99 | ||||
| 96 | - | // A nil SkillTool means skill discovery was explicitly disabled (for | 100 | + | // A nil SkillTool can mean discovery ran and found nothing, so use the |
| 97 | - | // example --no-skill or a profile with inheritSkills=false). Bundled | 101 | + | // explicit disable flag rather than the discovery result as the gate. |
| 98 | - | // extension skills must honor that same clean-room switch. | 102 | + | if source, ok := mgr.(ExtensionSkillSource); ok && r.skillsEnabled { |
| 99 | - | if source, ok := mgr.(ExtensionSkillSource); ok && r.SkillTool != nil { | |||
| 100 | if bundled := source.Skills(); len(bundled) > 0 { | 103 | if bundled := source.Skills(); len(bundled) > 0 { | ||
| 101 | merged := mergeExtensionSkills(r.SkillTool, bundled) | 104 | merged := mergeExtensionSkills(r.SkillTool, bundled) | ||
| 102 | - | r.SkillTool.SetSkills(merged) | 105 | + | if r.SkillTool == nil { |
| 106 | + | r.SkillTool = skills.NewTool(merged) | |||
| 107 | + | r.ToolRegistry[r.SkillTool.Name()] = r.SkillTool | |||
| 108 | + | } else { | |||
| 109 | + | r.SkillTool.SetSkills(merged) | |||
| 110 | + | } | |||
| 103 | append_ := append([]string(nil), r.systemAppend...) | 111 | append_ := append([]string(nil), r.systemAppend...) | ||
| 104 | if r.skillAddendum != "" { | 112 | if r.skillAddendum != "" { | ||
| 105 | filtered := append_[:0] | 113 | filtered := append_[:0] | ||
| @@ -701,8 +709,10 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||||
| 701 | discovered []*skills.Skill | 709 | discovered []*skills.Skill | ||
| 702 | skillTool *skills.Tool | 710 | skillTool *skills.Tool | ||
| 703 | skillAddendum string | 711 | skillAddendum string | ||
| 712 | + | skillsEnabled bool | |||
| 704 | ) | 713 | ) | ||
| 705 | if !args.NoSkill && (selectedProfile == nil || selectedProfile.InheritSkills == nil || *selectedProfile.InheritSkills) { | 714 | if !args.NoSkill && (selectedProfile == nil || selectedProfile.InheritSkills == nil || *selectedProfile.InheritSkills) { | ||
| 715 | + | skillsEnabled = true | |||
| 706 | homeDir, _ := os.UserHomeDir() | 716 | homeDir, _ := os.UserHomeDir() | ||
| 707 | discovered, _ = skills.Discover(ZotHome(), args.CWD, homeDir, args.WithSkills) | 717 | discovered, _ = skills.Discover(ZotHome(), args.CWD, homeDir, args.WithSkills) | ||
| 708 | if len(discovered) > 0 { | 718 | if len(discovered) > 0 { | ||
| @@ -789,6 +799,7 @@ func Resolve(args Args, requireCred bool) (Resolved, error) { | |||||
| 789 | MaxOutput: resolvedModel.MaxOutput, | 799 | MaxOutput: resolvedModel.MaxOutput, | ||
| 790 | Sandbox: sandbox, | 800 | Sandbox: sandbox, | ||
| 791 | SkillTool: skillTool, | 801 | SkillTool: skillTool, | ||
| 802 | + | skillsEnabled: skillsEnabled, | |||
| 792 | ContextFiles: contextFiles, | 803 | ContextFiles: contextFiles, | ||
| 793 | systemAppend: append_, | 804 | systemAppend: append_, | ||
| 794 | systemCustom: custom, | 805 | systemCustom: custom, | ||
M packages/agent/build_test.go +48 -0
| @@ -9,10 +9,58 @@ import ( | |||
| 9 | 9 | "strings" | |
| 10 | 10 | "testing" | |
| 11 | 11 | ||
| 12 | + | "github.com/patriceckhart/zot/packages/agent/skills" | |
| 12 | 13 | "github.com/patriceckhart/zot/packages/agent/tools" | |
| 14 | + | "github.com/patriceckhart/zot/packages/core" | |
| 13 | 15 | "github.com/patriceckhart/zot/packages/provider" | |
| 14 | 16 | ) | |
| 15 | 17 | ||
| 18 | + | type bundledSkillSource struct { | |
| 19 | + | bundled []*skills.Skill | |
| 20 | + | } | |
| 21 | + | ||
| 22 | + | func (s bundledSkillSource) Tools() []ExtensionToolInfo { return nil } | |
| 23 | + | func (s bundledSkillSource) NewExtensionTool(ExtensionToolInfo) core.Tool { | |
| 24 | + | return nil | |
| 25 | + | } | |
| 26 | + | func (s bundledSkillSource) Skills() []*skills.Skill { return s.bundled } | |
| 27 | + | ||
| 28 | + | func TestMergeExtensionToolsCreatesSkillToolWhenDiscoveryFoundNoSkills(t *testing.T) { | |
| 29 | + | r := Resolved{ | |
| 30 | + | CWD: t.TempDir(), | |
| 31 | + | ToolRegistry: make(core.Registry), | |
| 32 | + | skillsEnabled: true, | |
| 33 | + | } | |
| 34 | + | bundled := &skills.Skill{Name: "bundled", Description: "bundled skill", Source: "extension tasked-phases"} | |
| 35 | + | ||
| 36 | + | r.MergeExtensionTools(bundledSkillSource{bundled: []*skills.Skill{bundled}}) | |
| 37 | + | ||
| 38 | + | if r.SkillTool == nil { | |
| 39 | + | t.Fatal("MergeExtensionTools did not create a skill tool for bundled skills") | |
| 40 | + | } | |
| 41 | + | if _, ok := r.ToolRegistry[r.SkillTool.Name()]; !ok { | |
| 42 | + | t.Fatalf("skill tool %q was not registered", r.SkillTool.Name()) | |
| 43 | + | } | |
| 44 | + | if got := r.SkillTool.Skills(); len(got) != 1 || got[0].Name != bundled.Name { | |
| 45 | + | t.Fatalf("merged bundled skills = %#v", got) | |
| 46 | + | } | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | func TestMergeExtensionToolsRespectsDisabledSkillDiscovery(t *testing.T) { | |
| 50 | + | r := Resolved{ | |
| 51 | + | CWD: t.TempDir(), | |
| 52 | + | ToolRegistry: make(core.Registry), | |
| 53 | + | skillsEnabled: false, | |
| 54 | + | } | |
| 55 | + | bundled := &skills.Skill{Name: "bundled", Description: "bundled skill", Source: "extension tasked-phases"} | |
| 56 | + | ||
| 57 | + | r.MergeExtensionTools(bundledSkillSource{bundled: []*skills.Skill{bundled}}) | |
| 58 | + | ||
| 59 | + | if r.SkillTool != nil { | |
| 60 | + | t.Fatal("disabled skill discovery created a skill tool") | |
| 61 | + | } | |
| 62 | + | } | |
| 63 | + | ||
| 16 | 64 | func TestBuildToolRegistryIncludesLSPAndWriteDiagnostics(t *testing.T) { | |
| 17 | 65 | root := t.TempDir() | |
| 18 | 66 | registry := buildToolRegistry(Args{}, root, tools.NewSandbox(root), true, true, false) | |
| @@ -9,10 +9,58 @@ import ( | |||||
| 9 | "strings" | 9 | "strings" | ||
| 10 | "testing" | 10 | "testing" | ||
| 11 | 11 | ||||
| 12 | + | "github.com/patriceckhart/zot/packages/agent/skills" | |||
| 12 | "github.com/patriceckhart/zot/packages/agent/tools" | 13 | "github.com/patriceckhart/zot/packages/agent/tools" | ||
| 14 | + | "github.com/patriceckhart/zot/packages/core" | |||
| 13 | "github.com/patriceckhart/zot/packages/provider" | 15 | "github.com/patriceckhart/zot/packages/provider" | ||
| 14 | ) | 16 | ) | ||
| 15 | 17 | ||||
| 18 | + | type bundledSkillSource struct { | |||
| 19 | + | bundled []*skills.Skill | |||
| 20 | + | } | |||
| 21 | + | ||||
| 22 | + | func (s bundledSkillSource) Tools() []ExtensionToolInfo { return nil } | |||
| 23 | + | func (s bundledSkillSource) NewExtensionTool(ExtensionToolInfo) core.Tool { | |||
| 24 | + | return nil | |||
| 25 | + | } | |||
| 26 | + | func (s bundledSkillSource) Skills() []*skills.Skill { return s.bundled } | |||
| 27 | + | ||||
| 28 | + | func TestMergeExtensionToolsCreatesSkillToolWhenDiscoveryFoundNoSkills(t *testing.T) { | |||
| 29 | + | r := Resolved{ | |||
| 30 | + | CWD: t.TempDir(), | |||
| 31 | + | ToolRegistry: make(core.Registry), | |||
| 32 | + | skillsEnabled: true, | |||
| 33 | + | } | |||
| 34 | + | bundled := &skills.Skill{Name: "bundled", Description: "bundled skill", Source: "extension tasked-phases"} | |||
| 35 | + | ||||
| 36 | + | r.MergeExtensionTools(bundledSkillSource{bundled: []*skills.Skill{bundled}}) | |||
| 37 | + | ||||
| 38 | + | if r.SkillTool == nil { | |||
| 39 | + | t.Fatal("MergeExtensionTools did not create a skill tool for bundled skills") | |||
| 40 | + | } | |||
| 41 | + | if _, ok := r.ToolRegistry[r.SkillTool.Name()]; !ok { | |||
| 42 | + | t.Fatalf("skill tool %q was not registered", r.SkillTool.Name()) | |||
| 43 | + | } | |||
| 44 | + | if got := r.SkillTool.Skills(); len(got) != 1 || got[0].Name != bundled.Name { | |||
| 45 | + | t.Fatalf("merged bundled skills = %#v", got) | |||
| 46 | + | } | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | func TestMergeExtensionToolsRespectsDisabledSkillDiscovery(t *testing.T) { | |||
| 50 | + | r := Resolved{ | |||
| 51 | + | CWD: t.TempDir(), | |||
| 52 | + | ToolRegistry: make(core.Registry), | |||
| 53 | + | skillsEnabled: false, | |||
| 54 | + | } | |||
| 55 | + | bundled := &skills.Skill{Name: "bundled", Description: "bundled skill", Source: "extension tasked-phases"} | |||
| 56 | + | ||||
| 57 | + | r.MergeExtensionTools(bundledSkillSource{bundled: []*skills.Skill{bundled}}) | |||
| 58 | + | ||||
| 59 | + | if r.SkillTool != nil { | |||
| 60 | + | t.Fatal("disabled skill discovery created a skill tool") | |||
| 61 | + | } | |||
| 62 | + | } | |||
| 63 | + | ||||
| 16 | func TestBuildToolRegistryIncludesLSPAndWriteDiagnostics(t *testing.T) { | 64 | func TestBuildToolRegistryIncludesLSPAndWriteDiagnostics(t *testing.T) { | ||
| 17 | root := t.TempDir() | 65 | root := t.TempDir() | ||
| 18 | registry := buildToolRegistry(Args{}, root, tools.NewSandbox(root), true, true, false) | 66 | registry := buildToolRegistry(Args{}, root, tools.NewSandbox(root), true, true, false) | ||
M packages/agent/cli.go +82 -11
| @@ -37,9 +37,11 @@ import ( | |||
| 37 | 37 | const maxBufferedInteractiveAlerts = 64 | |
| 38 | 38 | ||
| 39 | 39 | type interactiveExtHooks struct { | |
| 40 | - | mu sync.Mutex | |
| 41 | - | interactive *modes.Interactive | |
| 42 | - | pendingAlerts []interactiveAlert | |
| 40 | + | mu sync.Mutex | |
| 41 | + | interactive *modes.Interactive | |
| 42 | + | pendingAlerts []interactiveAlert | |
| 43 | + | pendingStatuses map[string]interactiveStatus | |
| 44 | + | pendingWidgets map[string]interactiveWidget | |
| 43 | 45 | } | |
| 44 | 46 | ||
| 45 | 47 | type interactiveAlert struct { | |
| @@ -47,6 +49,21 @@ type interactiveAlert struct { | |||
| 47 | 49 | alert extproto.AlertRequest | |
| 48 | 50 | } | |
| 49 | 51 | ||
| 52 | + | type interactiveStatus struct { | |
| 53 | + | extName string | |
| 54 | + | key string | |
| 55 | + | level string | |
| 56 | + | text string | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | type interactiveWidget struct { | |
| 60 | + | extName string | |
| 61 | + | id string | |
| 62 | + | position string | |
| 63 | + | title string | |
| 64 | + | lines []string | |
| 65 | + | } | |
| 66 | + | ||
| 50 | 67 | func (h *interactiveExtHooks) iv() *modes.Interactive { | |
| 51 | 68 | if h == nil { | |
| 52 | 69 | return nil | |
| @@ -62,12 +79,22 @@ func (h *interactiveExtHooks) attachInteractive(iv *modes.Interactive) { | |||
| 62 | 79 | } | |
| 63 | 80 | h.mu.Lock() | |
| 64 | 81 | h.interactive = iv | |
| 65 | - | pending := h.pendingAlerts | |
| 82 | + | pendingAlerts := h.pendingAlerts | |
| 83 | + | pendingStatuses := h.pendingStatuses | |
| 84 | + | pendingWidgets := h.pendingWidgets | |
| 66 | 85 | h.pendingAlerts = nil | |
| 86 | + | h.pendingStatuses = nil | |
| 87 | + | h.pendingWidgets = nil | |
| 67 | 88 | h.mu.Unlock() | |
| 68 | - | for _, item := range pending { | |
| 89 | + | for _, item := range pendingAlerts { | |
| 69 | 90 | iv.Alert(item.extName, item.alert) | |
| 70 | 91 | } | |
| 92 | + | for _, item := range pendingStatuses { | |
| 93 | + | iv.SetStatus(item.extName, item.key, item.level, item.text) | |
| 94 | + | } | |
| 95 | + | for _, item := range pendingWidgets { | |
| 96 | + | iv.SetWidget(item.extName, item.id, item.position, item.title, item.lines) | |
| 97 | + | } | |
| 71 | 98 | } | |
| 72 | 99 | ||
| 73 | 100 | func (h *interactiveExtHooks) Notify(extName, level, message string) { | |
| @@ -131,20 +158,64 @@ func (h *interactiveExtHooks) ClosePanel(extName, panelID string) { | |||
| 131 | 158 | iv.ClosePanel(extName, panelID) | |
| 132 | 159 | } | |
| 133 | 160 | } | |
| 161 | + | func pendingChromeKey(extName, key string) string { return extName + "\x00" + key } | |
| 162 | + | ||
| 134 | 163 | func (h *interactiveExtHooks) SetStatus(extName, key, level, text string) { | |
| 135 | - | if iv := h.iv(); iv != nil { | |
| 136 | - | iv.SetStatus(extName, key, level, text) | |
| 164 | + | if h == nil { | |
| 165 | + | return | |
| 137 | 166 | } | |
| 167 | + | h.mu.Lock() | |
| 168 | + | if h.interactive == nil { | |
| 169 | + | if h.pendingStatuses == nil { | |
| 170 | + | h.pendingStatuses = map[string]interactiveStatus{} | |
| 171 | + | } | |
| 172 | + | pendingKey := pendingChromeKey(extName, key) | |
| 173 | + | if strings.TrimSpace(text) == "" { | |
| 174 | + | delete(h.pendingStatuses, pendingKey) | |
| 175 | + | } else { | |
| 176 | + | h.pendingStatuses[pendingKey] = interactiveStatus{extName: extName, key: key, level: level, text: text} | |
| 177 | + | } | |
| 178 | + | h.mu.Unlock() | |
| 179 | + | return | |
| 180 | + | } | |
| 181 | + | iv := h.interactive | |
| 182 | + | h.mu.Unlock() | |
| 183 | + | iv.SetStatus(extName, key, level, text) | |
| 138 | 184 | } | |
| 139 | 185 | func (h *interactiveExtHooks) SetWidget(extName, id, position, title string, lines []string) { | |
| 140 | - | if iv := h.iv(); iv != nil { | |
| 141 | - | iv.SetWidget(extName, id, position, title, lines) | |
| 186 | + | if h == nil { | |
| 187 | + | return | |
| 142 | 188 | } | |
| 189 | + | h.mu.Lock() | |
| 190 | + | if h.interactive == nil { | |
| 191 | + | if h.pendingWidgets == nil { | |
| 192 | + | h.pendingWidgets = map[string]interactiveWidget{} | |
| 193 | + | } | |
| 194 | + | h.pendingWidgets[pendingChromeKey(extName, id)] = interactiveWidget{ | |
| 195 | + | extName: extName, id: id, position: position, title: title, lines: append([]string(nil), lines...), | |
| 196 | + | } | |
| 197 | + | h.mu.Unlock() | |
| 198 | + | return | |
| 199 | + | } | |
| 200 | + | iv := h.interactive | |
| 201 | + | h.mu.Unlock() | |
| 202 | + | iv.SetWidget(extName, id, position, title, lines) | |
| 143 | 203 | } | |
| 144 | 204 | func (h *interactiveExtHooks) ClearWidget(extName, id string) { | |
| 145 | - | if iv := h.iv(); iv != nil { | |
| 146 | - | iv.ClearWidget(extName, id) | |
| 205 | + | if h == nil { | |
| 206 | + | return | |
| 147 | 207 | } | |
| 208 | + | h.mu.Lock() | |
| 209 | + | if h.interactive == nil { | |
| 210 | + | if h.pendingWidgets != nil { | |
| 211 | + | delete(h.pendingWidgets, pendingChromeKey(extName, id)) | |
| 212 | + | } | |
| 213 | + | h.mu.Unlock() | |
| 214 | + | return | |
| 215 | + | } | |
| 216 | + | iv := h.interactive | |
| 217 | + | h.mu.Unlock() | |
| 218 | + | iv.ClearWidget(extName, id) | |
| 148 | 219 | } | |
| 149 | 220 | ||
| 150 | 221 | // extToolAdapter bridges *extensions.Manager to the | |
| @@ -37,9 +37,11 @@ import ( | |||||
| 37 | const maxBufferedInteractiveAlerts = 64 | 37 | const maxBufferedInteractiveAlerts = 64 | ||
| 38 | 38 | ||||
| 39 | type interactiveExtHooks struct { | 39 | type interactiveExtHooks struct { | ||
| 40 | - | mu sync.Mutex | 40 | + | mu sync.Mutex |
| 41 | - | interactive *modes.Interactive | 41 | + | interactive *modes.Interactive |
| 42 | - | pendingAlerts []interactiveAlert | 42 | + | pendingAlerts []interactiveAlert |
| 43 | + | pendingStatuses map[string]interactiveStatus | |||
| 44 | + | pendingWidgets map[string]interactiveWidget | |||
| 43 | } | 45 | } | ||
| 44 | 46 | ||||
| 45 | type interactiveAlert struct { | 47 | type interactiveAlert struct { | ||
| @@ -47,6 +49,21 @@ type interactiveAlert struct { | |||||
| 47 | alert extproto.AlertRequest | 49 | alert extproto.AlertRequest | ||
| 48 | } | 50 | } | ||
| 49 | 51 | ||||
| 52 | + | type interactiveStatus struct { | |||
| 53 | + | extName string | |||
| 54 | + | key string | |||
| 55 | + | level string | |||
| 56 | + | text string | |||
| 57 | + | } | |||
| 58 | + | ||||
| 59 | + | type interactiveWidget struct { | |||
| 60 | + | extName string | |||
| 61 | + | id string | |||
| 62 | + | position string | |||
| 63 | + | title string | |||
| 64 | + | lines []string | |||
| 65 | + | } | |||
| 66 | + | ||||
| 50 | func (h *interactiveExtHooks) iv() *modes.Interactive { | 67 | func (h *interactiveExtHooks) iv() *modes.Interactive { | ||
| 51 | if h == nil { | 68 | if h == nil { | ||
| 52 | return nil | 69 | return nil | ||
| @@ -62,12 +79,22 @@ func (h *interactiveExtHooks) attachInteractive(iv *modes.Interactive) { | |||||
| 62 | } | 79 | } | ||
| 63 | h.mu.Lock() | 80 | h.mu.Lock() | ||
| 64 | h.interactive = iv | 81 | h.interactive = iv | ||
| 65 | - | pending := h.pendingAlerts | 82 | + | pendingAlerts := h.pendingAlerts |
| 83 | + | pendingStatuses := h.pendingStatuses | |||
| 84 | + | pendingWidgets := h.pendingWidgets | |||
| 66 | h.pendingAlerts = nil | 85 | h.pendingAlerts = nil | ||
| 86 | + | h.pendingStatuses = nil | |||
| 87 | + | h.pendingWidgets = nil | |||
| 67 | h.mu.Unlock() | 88 | h.mu.Unlock() | ||
| 68 | - | for _, item := range pending { | 89 | + | for _, item := range pendingAlerts { |
| 69 | iv.Alert(item.extName, item.alert) | 90 | iv.Alert(item.extName, item.alert) | ||
| 70 | } | 91 | } | ||
| 92 | + | for _, item := range pendingStatuses { | |||
| 93 | + | iv.SetStatus(item.extName, item.key, item.level, item.text) | |||
| 94 | + | } | |||
| 95 | + | for _, item := range pendingWidgets { | |||
| 96 | + | iv.SetWidget(item.extName, item.id, item.position, item.title, item.lines) | |||
| 97 | + | } | |||
| 71 | } | 98 | } | ||
| 72 | 99 | ||||
| 73 | func (h *interactiveExtHooks) Notify(extName, level, message string) { | 100 | func (h *interactiveExtHooks) Notify(extName, level, message string) { | ||
| @@ -131,20 +158,64 @@ func (h *interactiveExtHooks) ClosePanel(extName, panelID string) { | |||||
| 131 | iv.ClosePanel(extName, panelID) | 158 | iv.ClosePanel(extName, panelID) | ||
| 132 | } | 159 | } | ||
| 133 | } | 160 | } | ||
| 161 | + | func pendingChromeKey(extName, key string) string { return extName + "\x00" + key } | |||
| 162 | + | ||||
| 134 | func (h *interactiveExtHooks) SetStatus(extName, key, level, text string) { | 163 | func (h *interactiveExtHooks) SetStatus(extName, key, level, text string) { | ||
| 135 | - | if iv := h.iv(); iv != nil { | 164 | + | if h == nil { |
| 136 | - | iv.SetStatus(extName, key, level, text) | 165 | + | return |
| 137 | } | 166 | } | ||
| 167 | + | h.mu.Lock() | |||
| 168 | + | if h.interactive == nil { | |||
| 169 | + | if h.pendingStatuses == nil { | |||
| 170 | + | h.pendingStatuses = map[string]interactiveStatus{} | |||
| 171 | + | } | |||
| 172 | + | pendingKey := pendingChromeKey(extName, key) | |||
| 173 | + | if strings.TrimSpace(text) == "" { | |||
| 174 | + | delete(h.pendingStatuses, pendingKey) | |||
| 175 | + | } else { | |||
| 176 | + | h.pendingStatuses[pendingKey] = interactiveStatus{extName: extName, key: key, level: level, text: text} | |||
| 177 | + | } | |||
| 178 | + | h.mu.Unlock() | |||
| 179 | + | return | |||
| 180 | + | } | |||
| 181 | + | iv := h.interactive | |||
| 182 | + | h.mu.Unlock() | |||
| 183 | + | iv.SetStatus(extName, key, level, text) | |||
| 138 | } | 184 | } | ||
| 139 | func (h *interactiveExtHooks) SetWidget(extName, id, position, title string, lines []string) { | 185 | func (h *interactiveExtHooks) SetWidget(extName, id, position, title string, lines []string) { | ||
| 140 | - | if iv := h.iv(); iv != nil { | 186 | + | if h == nil { |
| 141 | - | iv.SetWidget(extName, id, position, title, lines) | 187 | + | return |
| 142 | } | 188 | } | ||
| 189 | + | h.mu.Lock() | |||
| 190 | + | if h.interactive == nil { | |||
| 191 | + | if h.pendingWidgets == nil { | |||
| 192 | + | h.pendingWidgets = map[string]interactiveWidget{} | |||
| 193 | + | } | |||
| 194 | + | h.pendingWidgets[pendingChromeKey(extName, id)] = interactiveWidget{ | |||
| 195 | + | extName: extName, id: id, position: position, title: title, lines: append([]string(nil), lines...), | |||
| 196 | + | } | |||
| 197 | + | h.mu.Unlock() | |||
| 198 | + | return | |||
| 199 | + | } | |||
| 200 | + | iv := h.interactive | |||
| 201 | + | h.mu.Unlock() | |||
| 202 | + | iv.SetWidget(extName, id, position, title, lines) | |||
| 143 | } | 203 | } | ||
| 144 | func (h *interactiveExtHooks) ClearWidget(extName, id string) { | 204 | func (h *interactiveExtHooks) ClearWidget(extName, id string) { | ||
| 145 | - | if iv := h.iv(); iv != nil { | 205 | + | if h == nil { |
| 146 | - | iv.ClearWidget(extName, id) | 206 | + | return |
| 147 | } | 207 | } | ||
| 208 | + | h.mu.Lock() | |||
| 209 | + | if h.interactive == nil { | |||
| 210 | + | if h.pendingWidgets != nil { | |||
| 211 | + | delete(h.pendingWidgets, pendingChromeKey(extName, id)) | |||
| 212 | + | } | |||
| 213 | + | h.mu.Unlock() | |||
| 214 | + | return | |||
| 215 | + | } | |||
| 216 | + | iv := h.interactive | |||
| 217 | + | h.mu.Unlock() | |||
| 218 | + | iv.ClearWidget(extName, id) | |||
| 148 | } | 219 | } | ||
| 149 | 220 | ||||
| 150 | // extToolAdapter bridges *extensions.Manager to the | 221 | // extToolAdapter bridges *extensions.Manager to the | ||
M packages/agent/cli_alert_test.go +23 -0
| @@ -7,6 +7,29 @@ import ( | |||
| 7 | 7 | "github.com/patriceckhart/zot/packages/agent/modes" | |
| 8 | 8 | ) | |
| 9 | 9 | ||
| 10 | + | func TestInteractiveExtHooksBufferStartupChrome(t *testing.T) { | |
| 11 | + | hooks := &interactiveExtHooks{} | |
| 12 | + | hooks.SetStatus("tasked-phases", "progress", "success", "2/4") | |
| 13 | + | hooks.SetWidget("tasked-phases", "plan", "above_input", "Plan", []string{"one"}) | |
| 14 | + | hooks.SetWidget("tasked-phases", "cleared", "above_input", "old", []string{"old"}) | |
| 15 | + | hooks.ClearWidget("tasked-phases", "cleared") | |
| 16 | + | hooks.SetStatus("tasked-phases", "removed", "info", "old") | |
| 17 | + | hooks.SetStatus("tasked-phases", "removed", "info", "") | |
| 18 | + | ||
| 19 | + | hooks.mu.Lock() | |
| 20 | + | if len(hooks.pendingStatuses) != 1 || len(hooks.pendingWidgets) != 1 { | |
| 21 | + | t.Fatalf("pending chrome = statuses %d widgets %d, want one each", len(hooks.pendingStatuses), len(hooks.pendingWidgets)) | |
| 22 | + | } | |
| 23 | + | hooks.mu.Unlock() | |
| 24 | + | ||
| 25 | + | hooks.attachInteractive(&modes.Interactive{}) | |
| 26 | + | hooks.mu.Lock() | |
| 27 | + | defer hooks.mu.Unlock() | |
| 28 | + | if len(hooks.pendingStatuses) != 0 || len(hooks.pendingWidgets) != 0 { | |
| 29 | + | t.Fatalf("pending chrome was not flushed: statuses=%v widgets=%v", hooks.pendingStatuses, hooks.pendingWidgets) | |
| 30 | + | } | |
| 31 | + | } | |
| 32 | + | ||
| 10 | 33 | func TestInteractiveExtHooksBufferStartupAlerts(t *testing.T) { | |
| 11 | 34 | hooks := &interactiveExtHooks{} | |
| 12 | 35 | hooks.Alert("question-ext", extproto.AlertRequest{Kind: extproto.AlertKindBell, Reason: "question_ready"}) | |
| @@ -7,6 +7,29 @@ import ( | |||||
| 7 | "github.com/patriceckhart/zot/packages/agent/modes" | 7 | "github.com/patriceckhart/zot/packages/agent/modes" | ||
| 8 | ) | 8 | ) | ||
| 9 | 9 | ||||
| 10 | + | func TestInteractiveExtHooksBufferStartupChrome(t *testing.T) { | |||
| 11 | + | hooks := &interactiveExtHooks{} | |||
| 12 | + | hooks.SetStatus("tasked-phases", "progress", "success", "2/4") | |||
| 13 | + | hooks.SetWidget("tasked-phases", "plan", "above_input", "Plan", []string{"one"}) | |||
| 14 | + | hooks.SetWidget("tasked-phases", "cleared", "above_input", "old", []string{"old"}) | |||
| 15 | + | hooks.ClearWidget("tasked-phases", "cleared") | |||
| 16 | + | hooks.SetStatus("tasked-phases", "removed", "info", "old") | |||
| 17 | + | hooks.SetStatus("tasked-phases", "removed", "info", "") | |||
| 18 | + | ||||
| 19 | + | hooks.mu.Lock() | |||
| 20 | + | if len(hooks.pendingStatuses) != 1 || len(hooks.pendingWidgets) != 1 { | |||
| 21 | + | t.Fatalf("pending chrome = statuses %d widgets %d, want one each", len(hooks.pendingStatuses), len(hooks.pendingWidgets)) | |||
| 22 | + | } | |||
| 23 | + | hooks.mu.Unlock() | |||
| 24 | + | ||||
| 25 | + | hooks.attachInteractive(&modes.Interactive{}) | |||
| 26 | + | hooks.mu.Lock() | |||
| 27 | + | defer hooks.mu.Unlock() | |||
| 28 | + | if len(hooks.pendingStatuses) != 0 || len(hooks.pendingWidgets) != 0 { | |||
| 29 | + | t.Fatalf("pending chrome was not flushed: statuses=%v widgets=%v", hooks.pendingStatuses, hooks.pendingWidgets) | |||
| 30 | + | } | |||
| 31 | + | } | |||
| 32 | + | ||||
| 10 | func TestInteractiveExtHooksBufferStartupAlerts(t *testing.T) { | 33 | func TestInteractiveExtHooksBufferStartupAlerts(t *testing.T) { | ||
| 11 | hooks := &interactiveExtHooks{} | 34 | hooks := &interactiveExtHooks{} | ||
| 12 | hooks.Alert("question-ext", extproto.AlertRequest{Kind: extproto.AlertKindBell, Reason: "question_ready"}) | 35 | hooks.Alert("question-ext", extproto.AlertRequest{Kind: extproto.AlertKindBell, Reason: "question_ready"}) | ||
M packages/agent/ext/ext_test.go +24 -8
| @@ -3,6 +3,7 @@ package ext | |||
| 3 | 3 | import ( | |
| 4 | 4 | "bufio" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "errors" | |
| 6 | 7 | "io" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "testing" | |
| @@ -64,6 +65,25 @@ func newHarness(name string) *extHarness { | |||
| 64 | 65 | return h | |
| 65 | 66 | } | |
| 66 | 67 | ||
| 68 | + | func (h *extHarness) startRun(t *testing.T) { | |
| 69 | + | t.Helper() | |
| 70 | + | runDone := make(chan error, 1) | |
| 71 | + | go func() { runDone <- h.ext.Run() }() | |
| 72 | + | t.Cleanup(func() { | |
| 73 | + | if err := h.hostW.Close(); err != nil && !errors.Is(err, io.ErrClosedPipe) { | |
| 74 | + | t.Errorf("close extension input: %v", err) | |
| 75 | + | } | |
| 76 | + | select { | |
| 77 | + | case err := <-runDone: | |
| 78 | + | if err != nil { | |
| 79 | + | t.Errorf("extension Run: %v", err) | |
| 80 | + | } | |
| 81 | + | case <-time.After(2 * time.Second): | |
| 82 | + | t.Error("extension Run did not exit after closing host input") | |
| 83 | + | } | |
| 84 | + | }) | |
| 85 | + | } | |
| 86 | + | ||
| 67 | 87 | // next returns the next frame, timing out after 2 s. | |
| 68 | 88 | func (h *extHarness) next(t *testing.T) rawFrame { | |
| 69 | 89 | t.Helper() | |
| @@ -222,7 +242,7 @@ func TestOpenPanelEmitsCorrectFrame(t *testing.T) { | |||
| 222 | 242 | ||
| 223 | 243 | func TestPersistentChromeFrames(t *testing.T) { | |
| 224 | 244 | h := newHarness("chrome-ext") | |
| 225 | - | go h.ext.Run() | |
| 245 | + | h.startRun(t) | |
| 226 | 246 | h.handshake(t) | |
| 227 | 247 | ||
| 228 | 248 | h.ext.SetStatus("progress", "2/4 tasks") | |
| @@ -254,7 +274,6 @@ func TestPersistentChromeFrames(t *testing.T) { | |||
| 254 | 274 | if cf.ID != "plan" { | |
| 255 | 275 | t.Fatalf("widget clear = %+v", cf) | |
| 256 | 276 | } | |
| 257 | - | h.hostW.Close() | |
| 258 | 277 | } | |
| 259 | 278 | ||
| 260 | 279 | func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | |
| @@ -262,7 +281,7 @@ func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | |||
| 262 | 281 | h.ext.Tool("details", "returns metadata", json.RawMessage(`{"type":"object"}`), func(json.RawMessage) ToolResult { | |
| 263 | 282 | return ToolResult{Content: []ToolContent{Text("ok")}, Details: JSONDetails(map[string]any{"state": 1})} | |
| 264 | 283 | }) | |
| 265 | - | go h.ext.Run() | |
| 284 | + | h.startRun(t) | |
| 266 | 285 | h.handshake(t) | |
| 267 | 286 | h.sendToExt(t, extproto.ToolCallFromHost{Type: "tool_call", ID: "call-1", Name: "details", Args: json.RawMessage(`{}`)}) | |
| 268 | 287 | frame := h.drainUntil(t, "tool_result") | |
| @@ -273,14 +292,13 @@ func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | |||
| 273 | 292 | if string(result.Details) != `{"state":1}` { | |
| 274 | 293 | t.Fatalf("details = %s", result.Details) | |
| 275 | 294 | } | |
| 276 | - | h.hostW.Close() | |
| 277 | 295 | } | |
| 278 | 296 | ||
| 279 | 297 | func TestSessionEventCarriesIdentityAndState(t *testing.T) { | |
| 280 | 298 | h := newHarness("session-ext") | |
| 281 | 299 | received := make(chan Event, 1) | |
| 282 | 300 | h.ext.On("session_opened", func(event Event) { received <- event }) | |
| 283 | - | go h.ext.Run() | |
| 301 | + | h.startRun(t) | |
| 284 | 302 | h.handshake(t) | |
| 285 | 303 | h.sendToExt(t, extproto.EventFromHost{ | |
| 286 | 304 | Type: "event", Event: "session_opened", | |
| @@ -295,7 +313,6 @@ func TestSessionEventCarriesIdentityAndState(t *testing.T) { | |||
| 295 | 313 | case <-time.After(2 * time.Second): | |
| 296 | 314 | t.Fatal("timeout waiting for session event") | |
| 297 | 315 | } | |
| 298 | - | h.hostW.Close() | |
| 299 | 316 | } | |
| 300 | 317 | ||
| 301 | 318 | func TestTurnStartContextIsReturned(t *testing.T) { | |
| @@ -303,7 +320,7 @@ func TestTurnStartContextIsReturned(t *testing.T) { | |||
| 303 | 320 | h.ext.InterceptTurnStart(func(int) TurnStartDecision { | |
| 304 | 321 | return TurnStartDecision{Context: "current phase: parse"} | |
| 305 | 322 | }) | |
| 306 | - | go h.ext.Run() | |
| 323 | + | h.startRun(t) | |
| 307 | 324 | h.handshake(t) | |
| 308 | 325 | h.sendToExt(t, extproto.EventInterceptFromHost{Type: "event_intercept", ID: "intercept-1", Event: "turn_start", Step: 2}) | |
| 309 | 326 | frame := h.drainUntil(t, "event_intercept_response") | |
| @@ -314,7 +331,6 @@ func TestTurnStartContextIsReturned(t *testing.T) { | |||
| 314 | 331 | if response.ID != "intercept-1" || response.Context != "current phase: parse" || response.Block { | |
| 315 | 332 | t.Fatalf("intercept response = %+v", response) | |
| 316 | 333 | } | |
| 317 | - | h.hostW.Close() | |
| 318 | 334 | } | |
| 319 | 335 | ||
| 320 | 336 | func TestAlertEmitsStructuredFrame(t *testing.T) { | |
| @@ -3,6 +3,7 @@ package ext | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bufio" | 4 | "bufio" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "errors" | |||
| 6 | "io" | 7 | "io" | ||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "testing" | 9 | "testing" | ||
| @@ -64,6 +65,25 @@ func newHarness(name string) *extHarness { | |||||
| 64 | return h | 65 | return h | ||
| 65 | } | 66 | } | ||
| 66 | 67 | ||||
| 68 | + | func (h *extHarness) startRun(t *testing.T) { | |||
| 69 | + | t.Helper() | |||
| 70 | + | runDone := make(chan error, 1) | |||
| 71 | + | go func() { runDone <- h.ext.Run() }() | |||
| 72 | + | t.Cleanup(func() { | |||
| 73 | + | if err := h.hostW.Close(); err != nil && !errors.Is(err, io.ErrClosedPipe) { | |||
| 74 | + | t.Errorf("close extension input: %v", err) | |||
| 75 | + | } | |||
| 76 | + | select { | |||
| 77 | + | case err := <-runDone: | |||
| 78 | + | if err != nil { | |||
| 79 | + | t.Errorf("extension Run: %v", err) | |||
| 80 | + | } | |||
| 81 | + | case <-time.After(2 * time.Second): | |||
| 82 | + | t.Error("extension Run did not exit after closing host input") | |||
| 83 | + | } | |||
| 84 | + | }) | |||
| 85 | + | } | |||
| 86 | + | ||||
| 67 | // next returns the next frame, timing out after 2 s. | 87 | // next returns the next frame, timing out after 2 s. | ||
| 68 | func (h *extHarness) next(t *testing.T) rawFrame { | 88 | func (h *extHarness) next(t *testing.T) rawFrame { | ||
| 69 | t.Helper() | 89 | t.Helper() | ||
| @@ -222,7 +242,7 @@ func TestOpenPanelEmitsCorrectFrame(t *testing.T) { | |||||
| 222 | 242 | ||||
| 223 | func TestPersistentChromeFrames(t *testing.T) { | 243 | func TestPersistentChromeFrames(t *testing.T) { | ||
| 224 | h := newHarness("chrome-ext") | 244 | h := newHarness("chrome-ext") | ||
| 225 | - | go h.ext.Run() | 245 | + | h.startRun(t) |
| 226 | h.handshake(t) | 246 | h.handshake(t) | ||
| 227 | 247 | ||||
| 228 | h.ext.SetStatus("progress", "2/4 tasks") | 248 | h.ext.SetStatus("progress", "2/4 tasks") | ||
| @@ -254,7 +274,6 @@ func TestPersistentChromeFrames(t *testing.T) { | |||||
| 254 | if cf.ID != "plan" { | 274 | if cf.ID != "plan" { | ||
| 255 | t.Fatalf("widget clear = %+v", cf) | 275 | t.Fatalf("widget clear = %+v", cf) | ||
| 256 | } | 276 | } | ||
| 257 | - | h.hostW.Close() | |||
| 258 | } | 277 | } | ||
| 259 | 278 | ||||
| 260 | func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | 279 | func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | ||
| @@ -262,7 +281,7 @@ func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | |||||
| 262 | h.ext.Tool("details", "returns metadata", json.RawMessage(`{"type":"object"}`), func(json.RawMessage) ToolResult { | 281 | h.ext.Tool("details", "returns metadata", json.RawMessage(`{"type":"object"}`), func(json.RawMessage) ToolResult { | ||
| 263 | return ToolResult{Content: []ToolContent{Text("ok")}, Details: JSONDetails(map[string]any{"state": 1})} | 282 | return ToolResult{Content: []ToolContent{Text("ok")}, Details: JSONDetails(map[string]any{"state": 1})} | ||
| 264 | }) | 283 | }) | ||
| 265 | - | go h.ext.Run() | 284 | + | h.startRun(t) |
| 266 | h.handshake(t) | 285 | h.handshake(t) | ||
| 267 | h.sendToExt(t, extproto.ToolCallFromHost{Type: "tool_call", ID: "call-1", Name: "details", Args: json.RawMessage(`{}`)}) | 286 | h.sendToExt(t, extproto.ToolCallFromHost{Type: "tool_call", ID: "call-1", Name: "details", Args: json.RawMessage(`{}`)}) | ||
| 268 | frame := h.drainUntil(t, "tool_result") | 287 | frame := h.drainUntil(t, "tool_result") | ||
| @@ -273,14 +292,13 @@ func TestToolResultDetailsEmitsOpaqueMetadata(t *testing.T) { | |||||
| 273 | if string(result.Details) != `{"state":1}` { | 292 | if string(result.Details) != `{"state":1}` { | ||
| 274 | t.Fatalf("details = %s", result.Details) | 293 | t.Fatalf("details = %s", result.Details) | ||
| 275 | } | 294 | } | ||
| 276 | - | h.hostW.Close() | |||
| 277 | } | 295 | } | ||
| 278 | 296 | ||||
| 279 | func TestSessionEventCarriesIdentityAndState(t *testing.T) { | 297 | func TestSessionEventCarriesIdentityAndState(t *testing.T) { | ||
| 280 | h := newHarness("session-ext") | 298 | h := newHarness("session-ext") | ||
| 281 | received := make(chan Event, 1) | 299 | received := make(chan Event, 1) | ||
| 282 | h.ext.On("session_opened", func(event Event) { received <- event }) | 300 | h.ext.On("session_opened", func(event Event) { received <- event }) | ||
| 283 | - | go h.ext.Run() | 301 | + | h.startRun(t) |
| 284 | h.handshake(t) | 302 | h.handshake(t) | ||
| 285 | h.sendToExt(t, extproto.EventFromHost{ | 303 | h.sendToExt(t, extproto.EventFromHost{ | ||
| 286 | Type: "event", Event: "session_opened", | 304 | Type: "event", Event: "session_opened", | ||
| @@ -295,7 +313,6 @@ func TestSessionEventCarriesIdentityAndState(t *testing.T) { | |||||
| 295 | case <-time.After(2 * time.Second): | 313 | case <-time.After(2 * time.Second): | ||
| 296 | t.Fatal("timeout waiting for session event") | 314 | t.Fatal("timeout waiting for session event") | ||
| 297 | } | 315 | } | ||
| 298 | - | h.hostW.Close() | |||
| 299 | } | 316 | } | ||
| 300 | 317 | ||||
| 301 | func TestTurnStartContextIsReturned(t *testing.T) { | 318 | func TestTurnStartContextIsReturned(t *testing.T) { | ||
| @@ -303,7 +320,7 @@ func TestTurnStartContextIsReturned(t *testing.T) { | |||||
| 303 | h.ext.InterceptTurnStart(func(int) TurnStartDecision { | 320 | h.ext.InterceptTurnStart(func(int) TurnStartDecision { | ||
| 304 | return TurnStartDecision{Context: "current phase: parse"} | 321 | return TurnStartDecision{Context: "current phase: parse"} | ||
| 305 | }) | 322 | }) | ||
| 306 | - | go h.ext.Run() | 323 | + | h.startRun(t) |
| 307 | h.handshake(t) | 324 | h.handshake(t) | ||
| 308 | h.sendToExt(t, extproto.EventInterceptFromHost{Type: "event_intercept", ID: "intercept-1", Event: "turn_start", Step: 2}) | 325 | h.sendToExt(t, extproto.EventInterceptFromHost{Type: "event_intercept", ID: "intercept-1", Event: "turn_start", Step: 2}) | ||
| 309 | frame := h.drainUntil(t, "event_intercept_response") | 326 | frame := h.drainUntil(t, "event_intercept_response") | ||
| @@ -314,7 +331,6 @@ func TestTurnStartContextIsReturned(t *testing.T) { | |||||
| 314 | if response.ID != "intercept-1" || response.Context != "current phase: parse" || response.Block { | 331 | if response.ID != "intercept-1" || response.Context != "current phase: parse" || response.Block { | ||
| 315 | t.Fatalf("intercept response = %+v", response) | 332 | t.Fatalf("intercept response = %+v", response) | ||
| 316 | } | 333 | } | ||
| 317 | - | h.hostW.Close() | |||
| 318 | } | 334 | } | ||
| 319 | 335 | ||||
| 320 | func TestAlertEmitsStructuredFrame(t *testing.T) { | 336 | func TestAlertEmitsStructuredFrame(t *testing.T) { | ||
M packages/agent/extensions/manager.go +32 -1
| @@ -1027,6 +1027,8 @@ func (m *Manager) readLoop(ext *Extension, scanner *bufio.Scanner) { | |||
| 1027 | 1027 | } | |
| 1028 | 1028 | } | |
| 1029 | 1029 | ||
| 1030 | + | const lifecycleWriteGrace = 250 * time.Millisecond | |
| 1031 | + | ||
| 1030 | 1032 | func (ext *Extension) writeFrame(frame []byte) (int, error) { | |
| 1031 | 1033 | if ext == nil || ext.stdin == nil { | |
| 1032 | 1034 | return 0, fmt.Errorf("extension stdin is closed") | |
| @@ -1036,6 +1038,35 @@ func (ext *Extension) writeFrame(frame []byte) (int, error) { | |||
| 1036 | 1038 | return ext.stdin.Write(frame) | |
| 1037 | 1039 | } | |
| 1038 | 1040 | ||
| 1041 | + | // tryWriteLifecycleFrame bounds only a lifecycle writer's wait for the shared | |
| 1042 | + | // writer lock. Synchronous host frames retain their existing write behavior; | |
| 1043 | + | // a stalled lifecycle snapshot is dropped rather than delaying them. | |
| 1044 | + | func (ext *Extension) tryWriteLifecycleFrame(frame []byte) error { | |
| 1045 | + | if ext == nil || ext.stdin == nil { | |
| 1046 | + | return fmt.Errorf("extension stdin is closed") | |
| 1047 | + | } | |
| 1048 | + | if !ext.writeMu.TryLock() { | |
| 1049 | + | timer := time.NewTimer(lifecycleWriteGrace) | |
| 1050 | + | defer timer.Stop() | |
| 1051 | + | ticker := time.NewTicker(5 * time.Millisecond) | |
| 1052 | + | defer ticker.Stop() | |
| 1053 | + | for { | |
| 1054 | + | select { | |
| 1055 | + | case <-timer.C: | |
| 1056 | + | return fmt.Errorf("extension lifecycle write is blocked") | |
| 1057 | + | case <-ticker.C: | |
| 1058 | + | if ext.writeMu.TryLock() { | |
| 1059 | + | goto locked | |
| 1060 | + | } | |
| 1061 | + | } | |
| 1062 | + | } | |
| 1063 | + | } | |
| 1064 | + | locked: | |
| 1065 | + | defer ext.writeMu.Unlock() | |
| 1066 | + | _, err := ext.stdin.Write(frame) | |
| 1067 | + | return err | |
| 1068 | + | } | |
| 1069 | + | ||
| 1039 | 1070 | func (ext *Extension) startLifecycleWriter() { | |
| 1040 | 1071 | if ext.lifecycleFrames == nil { | |
| 1041 | 1072 | ext.lifecycleFrames = make(chan []byte, 64) | |
| @@ -1092,7 +1123,7 @@ func (ext *Extension) lifecycleWriter() { | |||
| 1092 | 1123 | case <-ext.lifecycleStop: | |
| 1093 | 1124 | return | |
| 1094 | 1125 | case frame := <-ext.lifecycleFrames: | |
| 1095 | - | if _, err := ext.writeFrame(frame); err != nil { | |
| 1126 | + | if err := ext.tryWriteLifecycleFrame(frame); err != nil { | |
| 1096 | 1127 | return | |
| 1097 | 1128 | } | |
| 1098 | 1129 | } | |
| @@ -1027,6 +1027,8 @@ func (m *Manager) readLoop(ext *Extension, scanner *bufio.Scanner) { | |||||
| 1027 | } | 1027 | } | ||
| 1028 | } | 1028 | } | ||
| 1029 | 1029 | ||||
| 1030 | + | const lifecycleWriteGrace = 250 * time.Millisecond | |||
| 1031 | + | ||||
| 1030 | func (ext *Extension) writeFrame(frame []byte) (int, error) { | 1032 | func (ext *Extension) writeFrame(frame []byte) (int, error) { | ||
| 1031 | if ext == nil || ext.stdin == nil { | 1033 | if ext == nil || ext.stdin == nil { | ||
| 1032 | return 0, fmt.Errorf("extension stdin is closed") | 1034 | return 0, fmt.Errorf("extension stdin is closed") | ||
| @@ -1036,6 +1038,35 @@ func (ext *Extension) writeFrame(frame []byte) (int, error) { | |||||
| 1036 | return ext.stdin.Write(frame) | 1038 | return ext.stdin.Write(frame) | ||
| 1037 | } | 1039 | } | ||
| 1038 | 1040 | ||||
| 1041 | + | // tryWriteLifecycleFrame bounds only a lifecycle writer's wait for the shared | |||
| 1042 | + | // writer lock. Synchronous host frames retain their existing write behavior; | |||
| 1043 | + | // a stalled lifecycle snapshot is dropped rather than delaying them. | |||
| 1044 | + | func (ext *Extension) tryWriteLifecycleFrame(frame []byte) error { | |||
| 1045 | + | if ext == nil || ext.stdin == nil { | |||
| 1046 | + | return fmt.Errorf("extension stdin is closed") | |||
| 1047 | + | } | |||
| 1048 | + | if !ext.writeMu.TryLock() { | |||
| 1049 | + | timer := time.NewTimer(lifecycleWriteGrace) | |||
| 1050 | + | defer timer.Stop() | |||
| 1051 | + | ticker := time.NewTicker(5 * time.Millisecond) | |||
| 1052 | + | defer ticker.Stop() | |||
| 1053 | + | for { | |||
| 1054 | + | select { | |||
| 1055 | + | case <-timer.C: | |||
| 1056 | + | return fmt.Errorf("extension lifecycle write is blocked") | |||
| 1057 | + | case <-ticker.C: | |||
| 1058 | + | if ext.writeMu.TryLock() { | |||
| 1059 | + | goto locked | |||
| 1060 | + | } | |||
| 1061 | + | } | |||
| 1062 | + | } | |||
| 1063 | + | } | |||
| 1064 | + | locked: | |||
| 1065 | + | defer ext.writeMu.Unlock() | |||
| 1066 | + | _, err := ext.stdin.Write(frame) | |||
| 1067 | + | return err | |||
| 1068 | + | } | |||
| 1069 | + | ||||
| 1039 | func (ext *Extension) startLifecycleWriter() { | 1070 | func (ext *Extension) startLifecycleWriter() { | ||
| 1040 | if ext.lifecycleFrames == nil { | 1071 | if ext.lifecycleFrames == nil { | ||
| 1041 | ext.lifecycleFrames = make(chan []byte, 64) | 1072 | ext.lifecycleFrames = make(chan []byte, 64) | ||
| @@ -1092,7 +1123,7 @@ func (ext *Extension) lifecycleWriter() { | |||||
| 1092 | case <-ext.lifecycleStop: | 1123 | case <-ext.lifecycleStop: | ||
| 1093 | return | 1124 | return | ||
| 1094 | case frame := <-ext.lifecycleFrames: | 1125 | case frame := <-ext.lifecycleFrames: | ||
| 1095 | - | if _, err := ext.writeFrame(frame); err != nil { | 1126 | + | if err := ext.tryWriteLifecycleFrame(frame); err != nil { |
| 1096 | return | 1127 | return | ||
| 1097 | } | 1128 | } | ||
| 1098 | } | 1129 | } | ||
M packages/agent/extensions/manager_test.go +25 -0
| @@ -3,6 +3,7 @@ package extensions | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "io" | |
| 6 | 7 | "os" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/patriceckhart/zot/packages/agent/extproto" | |
| @@ -14,6 +15,30 @@ import ( | |||
| 14 | 15 | "time" | |
| 15 | 16 | ) | |
| 16 | 17 | ||
| 18 | + | func TestLifecycleWriteDropsWhenSynchronousWriterOwnsLock(t *testing.T) { | |
| 19 | + | ext := &Extension{stdin: &nopWriteCloser{}} | |
| 20 | + | ext.writeMu.Lock() | |
| 21 | + | defer ext.writeMu.Unlock() | |
| 22 | + | ||
| 23 | + | done := make(chan error, 1) | |
| 24 | + | go func() { done <- ext.tryWriteLifecycleFrame([]byte("lifecycle")) }() | |
| 25 | + | select { | |
| 26 | + | case err := <-done: | |
| 27 | + | if err == nil { | |
| 28 | + | t.Fatal("lifecycle write unexpectedly acquired the synchronous writer lock") | |
| 29 | + | } | |
| 30 | + | case <-time.After(lifecycleWriteGrace + time.Second): | |
| 31 | + | t.Fatal("lifecycle write remained blocked on the synchronous writer lock") | |
| 32 | + | } | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | type nopWriteCloser struct{} | |
| 36 | + | ||
| 37 | + | func (*nopWriteCloser) Write(data []byte) (int, error) { return len(data), nil } | |
| 38 | + | func (*nopWriteCloser) Close() error { return nil } | |
| 39 | + | ||
| 40 | + | var _ io.WriteCloser = (*nopWriteCloser)(nil) | |
| 41 | + | ||
| 17 | 42 | // stubHooks records every callback so the test can assert on them. | |
| 18 | 43 | type stubHooks struct { | |
| 19 | 44 | mu sync.Mutex | |
| @@ -3,6 +3,7 @@ package extensions | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "io" | |||
| 6 | "os" | 7 | "os" | ||
| 7 | 8 | ||||
| 8 | "github.com/patriceckhart/zot/packages/agent/extproto" | 9 | "github.com/patriceckhart/zot/packages/agent/extproto" | ||
| @@ -14,6 +15,30 @@ import ( | |||||
| 14 | "time" | 15 | "time" | ||
| 15 | ) | 16 | ) | ||
| 16 | 17 | ||||
| 18 | + | func TestLifecycleWriteDropsWhenSynchronousWriterOwnsLock(t *testing.T) { | |||
| 19 | + | ext := &Extension{stdin: &nopWriteCloser{}} | |||
| 20 | + | ext.writeMu.Lock() | |||
| 21 | + | defer ext.writeMu.Unlock() | |||
| 22 | + | ||||
| 23 | + | done := make(chan error, 1) | |||
| 24 | + | go func() { done <- ext.tryWriteLifecycleFrame([]byte("lifecycle")) }() | |||
| 25 | + | select { | |||
| 26 | + | case err := <-done: | |||
| 27 | + | if err == nil { | |||
| 28 | + | t.Fatal("lifecycle write unexpectedly acquired the synchronous writer lock") | |||
| 29 | + | } | |||
| 30 | + | case <-time.After(lifecycleWriteGrace + time.Second): | |||
| 31 | + | t.Fatal("lifecycle write remained blocked on the synchronous writer lock") | |||
| 32 | + | } | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | type nopWriteCloser struct{} | |||
| 36 | + | ||||
| 37 | + | func (*nopWriteCloser) Write(data []byte) (int, error) { return len(data), nil } | |||
| 38 | + | func (*nopWriteCloser) Close() error { return nil } | |||
| 39 | + | ||||
| 40 | + | var _ io.WriteCloser = (*nopWriteCloser)(nil) | |||
| 41 | + | ||||
| 17 | // stubHooks records every callback so the test can assert on them. | 42 | // stubHooks records every callback so the test can assert on them. | ||
| 18 | type stubHooks struct { | 43 | type stubHooks struct { | ||
| 19 | mu sync.Mutex | 44 | mu sync.Mutex | ||
M packages/agent/modes/ext_notes_test.go +18 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package modes | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "fmt" | |
| 4 | 5 | "strings" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| @@ -64,6 +65,23 @@ func TestPersistentExtensionWidgetRowsAreBounded(t *testing.T) { | |||
| 64 | 65 | } | |
| 65 | 66 | } | |
| 66 | 67 | ||
| 68 | + | func TestPersistentExtensionStatusRowsAreBounded(t *testing.T) { | |
| 69 | + | i := newNotesTestInteractive() | |
| 70 | + | for n := 0; n < maxExtensionStatusRows+4; n++ { | |
| 71 | + | i.SetStatus("extension", fmt.Sprintf("status-%d", n), "info", "line") | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | i.mu.Lock() | |
| 75 | + | got := i.extensionChromeLinesLocked(80) | |
| 76 | + | i.mu.Unlock() | |
| 77 | + | if len(got) != maxExtensionStatusRows { | |
| 78 | + | t.Fatalf("status rows = %d, want %d: %v", len(got), maxExtensionStatusRows, got) | |
| 79 | + | } | |
| 80 | + | if !strings.Contains(strings.Join(got, "\n"), "extension statuses truncated") { | |
| 81 | + | t.Fatalf("bounded status rows omitted truncation marker: %v", got) | |
| 82 | + | } | |
| 83 | + | } | |
| 84 | + | ||
| 67 | 85 | func TestPersistentExtensionStatusAndWidget(t *testing.T) { | |
| 68 | 86 | i := newNotesTestInteractive() | |
| 69 | 87 | i.SetStatus("tasked-phases", "progress", "success", "2/4 tasks checked") | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package modes | 1 | package modes | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "fmt" | |||
| 4 | "strings" | 5 | "strings" | ||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| @@ -64,6 +65,23 @@ func TestPersistentExtensionWidgetRowsAreBounded(t *testing.T) { | |||||
| 64 | } | 65 | } | ||
| 65 | } | 66 | } | ||
| 66 | 67 | ||||
| 68 | + | func TestPersistentExtensionStatusRowsAreBounded(t *testing.T) { | |||
| 69 | + | i := newNotesTestInteractive() | |||
| 70 | + | for n := 0; n < maxExtensionStatusRows+4; n++ { | |||
| 71 | + | i.SetStatus("extension", fmt.Sprintf("status-%d", n), "info", "line") | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | i.mu.Lock() | |||
| 75 | + | got := i.extensionChromeLinesLocked(80) | |||
| 76 | + | i.mu.Unlock() | |||
| 77 | + | if len(got) != maxExtensionStatusRows { | |||
| 78 | + | t.Fatalf("status rows = %d, want %d: %v", len(got), maxExtensionStatusRows, got) | |||
| 79 | + | } | |||
| 80 | + | if !strings.Contains(strings.Join(got, "\n"), "extension statuses truncated") { | |||
| 81 | + | t.Fatalf("bounded status rows omitted truncation marker: %v", got) | |||
| 82 | + | } | |||
| 83 | + | } | |||
| 84 | + | ||||
| 67 | func TestPersistentExtensionStatusAndWidget(t *testing.T) { | 85 | func TestPersistentExtensionStatusAndWidget(t *testing.T) { | ||
| 68 | i := newNotesTestInteractive() | 86 | i := newNotesTestInteractive() | ||
| 69 | i.SetStatus("tasked-phases", "progress", "success", "2/4 tasks checked") | 87 | i.SetStatus("tasked-phases", "progress", "success", "2/4 tasks checked") | ||
M packages/agent/modes/interactive.go +14 -0
| @@ -1186,6 +1186,7 @@ func (i *Interactive) extensionWidgetsKeyLocked() string { | |||
| 1186 | 1186 | } | |
| 1187 | 1187 | ||
| 1188 | 1188 | const maxExtensionWidgetRows = 12 | |
| 1189 | + | const maxExtensionStatusRows = 6 | |
| 1189 | 1190 | ||
| 1190 | 1191 | func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | |
| 1191 | 1192 | var out []string | |
| @@ -1235,8 +1236,14 @@ func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | |||
| 1235 | 1236 | out = append(out, i.cfg.Theme.FG256(i.cfg.Theme.Muted, truncateLine(" ... extension widgets truncated ...", cols))) | |
| 1236 | 1237 | } | |
| 1237 | 1238 | ||
| 1239 | + | statusRows := 0 | |
| 1240 | + | statusTruncated := false | |
| 1238 | 1241 | for _, extName := range sortedNestedOuterKeys(i.extStatuses) { | |
| 1239 | 1242 | for _, key := range sortedNestedInnerKeys(i.extStatuses, extName) { | |
| 1243 | + | if statusRows >= maxExtensionStatusRows-1 { | |
| 1244 | + | statusTruncated = true | |
| 1245 | + | break | |
| 1246 | + | } | |
| 1240 | 1247 | status := i.extStatuses[extName][key] | |
| 1241 | 1248 | color := i.cfg.Theme.Muted | |
| 1242 | 1249 | switch status.Level { | |
| @@ -1249,7 +1256,14 @@ func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | |||
| 1249 | 1256 | } | |
| 1250 | 1257 | label := " [" + extName + "] " + status.Text | |
| 1251 | 1258 | out = append(out, i.cfg.Theme.FG256(color, truncateLine(label, cols))) | |
| 1259 | + | statusRows++ | |
| 1252 | 1260 | } | |
| 1261 | + | if statusTruncated { | |
| 1262 | + | break | |
| 1263 | + | } | |
| 1264 | + | } | |
| 1265 | + | if statusTruncated { | |
| 1266 | + | out = append(out, i.cfg.Theme.FG256(i.cfg.Theme.Muted, truncateLine(" ... extension statuses truncated ...", cols))) | |
| 1253 | 1267 | } | |
| 1254 | 1268 | return out | |
| 1255 | 1269 | } | |
| @@ -1186,6 +1186,7 @@ func (i *Interactive) extensionWidgetsKeyLocked() string { | |||||
| 1186 | } | 1186 | } | ||
| 1187 | 1187 | ||||
| 1188 | const maxExtensionWidgetRows = 12 | 1188 | const maxExtensionWidgetRows = 12 | ||
| 1189 | + | const maxExtensionStatusRows = 6 | |||
| 1189 | 1190 | ||||
| 1190 | func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | 1191 | func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | ||
| 1191 | var out []string | 1192 | var out []string | ||
| @@ -1235,8 +1236,14 @@ func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | |||||
| 1235 | out = append(out, i.cfg.Theme.FG256(i.cfg.Theme.Muted, truncateLine(" ... extension widgets truncated ...", cols))) | 1236 | out = append(out, i.cfg.Theme.FG256(i.cfg.Theme.Muted, truncateLine(" ... extension widgets truncated ...", cols))) | ||
| 1236 | } | 1237 | } | ||
| 1237 | 1238 | ||||
| 1239 | + | statusRows := 0 | |||
| 1240 | + | statusTruncated := false | |||
| 1238 | for _, extName := range sortedNestedOuterKeys(i.extStatuses) { | 1241 | for _, extName := range sortedNestedOuterKeys(i.extStatuses) { | ||
| 1239 | for _, key := range sortedNestedInnerKeys(i.extStatuses, extName) { | 1242 | for _, key := range sortedNestedInnerKeys(i.extStatuses, extName) { | ||
| 1243 | + | if statusRows >= maxExtensionStatusRows-1 { | |||
| 1244 | + | statusTruncated = true | |||
| 1245 | + | break | |||
| 1246 | + | } | |||
| 1240 | status := i.extStatuses[extName][key] | 1247 | status := i.extStatuses[extName][key] | ||
| 1241 | color := i.cfg.Theme.Muted | 1248 | color := i.cfg.Theme.Muted | ||
| 1242 | switch status.Level { | 1249 | switch status.Level { | ||
| @@ -1249,7 +1256,14 @@ func (i *Interactive) extensionChromeLinesLocked(cols int) []string { | |||||
| 1249 | } | 1256 | } | ||
| 1250 | label := " [" + extName + "] " + status.Text | 1257 | label := " [" + extName + "] " + status.Text | ||
| 1251 | out = append(out, i.cfg.Theme.FG256(color, truncateLine(label, cols))) | 1258 | out = append(out, i.cfg.Theme.FG256(color, truncateLine(label, cols))) | ||
| 1259 | + | statusRows++ | |||
| 1252 | } | 1260 | } | ||
| 1261 | + | if statusTruncated { | |||
| 1262 | + | break | |||
| 1263 | + | } | |||
| 1264 | + | } | |||
| 1265 | + | if statusTruncated { | |||
| 1266 | + | out = append(out, i.cfg.Theme.FG256(i.cfg.Theme.Muted, truncateLine(" ... extension statuses truncated ...", cols))) | |||
| 1253 | } | 1267 | } | ||
| 1254 | return out | 1268 | return out | ||
| 1255 | } | 1269 | } | ||
M packages/core/session_portable.go +27 -6
| @@ -358,6 +358,23 @@ func readExtensionStateAtFork(path string, limit int) (map[string]json.RawMessag | |||
| 358 | 358 | defer f.Close() | |
| 359 | 359 | ||
| 360 | 360 | state := make(map[string]json.RawMessage) | |
| 361 | + | forkState := make(map[string]json.RawMessage) | |
| 362 | + | cloneState := func(source map[string]json.RawMessage) map[string]json.RawMessage { | |
| 363 | + | clone := make(map[string]json.RawMessage, len(source)) | |
| 364 | + | for name, raw := range source { | |
| 365 | + | clone[name] = append(json.RawMessage(nil), raw...) | |
| 366 | + | } | |
| 367 | + | return clone | |
| 368 | + | } | |
| 369 | + | applyState := func(target map[string]json.RawMessage, row sessionLine) { | |
| 370 | + | if len(row.State) == 0 || strings.TrimSpace(string(row.State)) == "null" { | |
| 371 | + | delete(target, row.Extension) | |
| 372 | + | return | |
| 373 | + | } | |
| 374 | + | if json.Valid(row.State) { | |
| 375 | + | target[row.Extension] = append(json.RawMessage(nil), row.State...) | |
| 376 | + | } | |
| 377 | + | } | |
| 361 | 378 | effectiveCount := 0 | |
| 362 | 379 | err = forEachStrictJSONLLine(f, func(line []byte, lineNo int) error { | |
| 363 | 380 | var head sessionLineHead | |
| @@ -378,18 +395,22 @@ func readExtensionStateAtFork(path string, limit int) (map[string]json.RawMessag | |||
| 378 | 395 | return fmt.Errorf("line %d: invalid compaction row: %w", lineNo, err) | |
| 379 | 396 | } | |
| 380 | 397 | effectiveCount = len(messages) | |
| 398 | + | // A compaction replaces the effective transcript. State that was | |
| 399 | + | // recorded anywhere before this boundary belongs to every fork | |
| 400 | + | // inside the replacement transcript, even when its old message | |
| 401 | + | // index was after the requested fork point. | |
| 402 | + | forkState = cloneState(state) | |
| 381 | 403 | case "extension_state": | |
| 382 | 404 | var row sessionLine | |
| 383 | 405 | if err := json.Unmarshal(line, &row); err != nil { | |
| 384 | 406 | return fmt.Errorf("line %d: invalid extension state row: %w", lineNo, err) | |
| 385 | 407 | } | |
| 386 | - | if effectiveCount > limit || row.Extension == "" || len(row.State) > maxExtensionStateBytes { | |
| 408 | + | if row.Extension == "" || len(row.State) > maxExtensionStateBytes { | |
| 387 | 409 | return nil | |
| 388 | 410 | } | |
| 389 | - | if len(row.State) == 0 || strings.TrimSpace(string(row.State)) == "null" { | |
| 390 | - | delete(state, row.Extension) | |
| 391 | - | } else if json.Valid(row.State) { | |
| 392 | - | state[row.Extension] = append(json.RawMessage(nil), row.State...) | |
| 411 | + | applyState(state, row) | |
| 412 | + | if effectiveCount <= limit { | |
| 413 | + | applyState(forkState, row) | |
| 393 | 414 | } | |
| 394 | 415 | default: | |
| 395 | 416 | return fmt.Errorf("line %d: unknown row type %q", lineNo, head.Type) | |
| @@ -399,7 +420,7 @@ func readExtensionStateAtFork(path string, limit int) (map[string]json.RawMessag | |||
| 399 | 420 | if err != nil { | |
| 400 | 421 | return nil, fmt.Errorf("branch: read extension state: %w", err) | |
| 401 | 422 | } | |
| 402 | - | return state, nil | |
| 423 | + | return forkState, nil | |
| 403 | 424 | } | |
| 404 | 425 | ||
| 405 | 426 | func writeBranchSession(root, cwd, version string, parent SessionMeta, messages []provider.Message, checkpoints []SessionUsageCheckpoint, limit int, hideFromSessions bool, extensionState map[string]json.RawMessage) (string, error) { | |
| @@ -358,6 +358,23 @@ func readExtensionStateAtFork(path string, limit int) (map[string]json.RawMessag | |||||
| 358 | defer f.Close() | 358 | defer f.Close() | ||
| 359 | 359 | ||||
| 360 | state := make(map[string]json.RawMessage) | 360 | state := make(map[string]json.RawMessage) | ||
| 361 | + | forkState := make(map[string]json.RawMessage) | |||
| 362 | + | cloneState := func(source map[string]json.RawMessage) map[string]json.RawMessage { | |||
| 363 | + | clone := make(map[string]json.RawMessage, len(source)) | |||
| 364 | + | for name, raw := range source { | |||
| 365 | + | clone[name] = append(json.RawMessage(nil), raw...) | |||
| 366 | + | } | |||
| 367 | + | return clone | |||
| 368 | + | } | |||
| 369 | + | applyState := func(target map[string]json.RawMessage, row sessionLine) { | |||
| 370 | + | if len(row.State) == 0 || strings.TrimSpace(string(row.State)) == "null" { | |||
| 371 | + | delete(target, row.Extension) | |||
| 372 | + | return | |||
| 373 | + | } | |||
| 374 | + | if json.Valid(row.State) { | |||
| 375 | + | target[row.Extension] = append(json.RawMessage(nil), row.State...) | |||
| 376 | + | } | |||
| 377 | + | } | |||
| 361 | effectiveCount := 0 | 378 | effectiveCount := 0 | ||
| 362 | err = forEachStrictJSONLLine(f, func(line []byte, lineNo int) error { | 379 | err = forEachStrictJSONLLine(f, func(line []byte, lineNo int) error { | ||
| 363 | var head sessionLineHead | 380 | var head sessionLineHead | ||
| @@ -378,18 +395,22 @@ func readExtensionStateAtFork(path string, limit int) (map[string]json.RawMessag | |||||
| 378 | return fmt.Errorf("line %d: invalid compaction row: %w", lineNo, err) | 395 | return fmt.Errorf("line %d: invalid compaction row: %w", lineNo, err) | ||
| 379 | } | 396 | } | ||
| 380 | effectiveCount = len(messages) | 397 | effectiveCount = len(messages) | ||
| 398 | + | // A compaction replaces the effective transcript. State that was | |||
| 399 | + | // recorded anywhere before this boundary belongs to every fork | |||
| 400 | + | // inside the replacement transcript, even when its old message | |||
| 401 | + | // index was after the requested fork point. | |||
| 402 | + | forkState = cloneState(state) | |||
| 381 | case "extension_state": | 403 | case "extension_state": | ||
| 382 | var row sessionLine | 404 | var row sessionLine | ||
| 383 | if err := json.Unmarshal(line, &row); err != nil { | 405 | if err := json.Unmarshal(line, &row); err != nil { | ||
| 384 | return fmt.Errorf("line %d: invalid extension state row: %w", lineNo, err) | 406 | return fmt.Errorf("line %d: invalid extension state row: %w", lineNo, err) | ||
| 385 | } | 407 | } | ||
| 386 | - | if effectiveCount > limit || row.Extension == "" || len(row.State) > maxExtensionStateBytes { | 408 | + | if row.Extension == "" || len(row.State) > maxExtensionStateBytes { |
| 387 | return nil | 409 | return nil | ||
| 388 | } | 410 | } | ||
| 389 | - | if len(row.State) == 0 || strings.TrimSpace(string(row.State)) == "null" { | 411 | + | applyState(state, row) |
| 390 | - | delete(state, row.Extension) | 412 | + | if effectiveCount <= limit { |
| 391 | - | } else if json.Valid(row.State) { | 413 | + | applyState(forkState, row) |
| 392 | - | state[row.Extension] = append(json.RawMessage(nil), row.State...) | |||
| 393 | } | 414 | } | ||
| 394 | default: | 415 | default: | ||
| 395 | return fmt.Errorf("line %d: unknown row type %q", lineNo, head.Type) | 416 | return fmt.Errorf("line %d: unknown row type %q", lineNo, head.Type) | ||
| @@ -399,7 +420,7 @@ func readExtensionStateAtFork(path string, limit int) (map[string]json.RawMessag | |||||
| 399 | if err != nil { | 420 | if err != nil { | ||
| 400 | return nil, fmt.Errorf("branch: read extension state: %w", err) | 421 | return nil, fmt.Errorf("branch: read extension state: %w", err) | ||
| 401 | } | 422 | } | ||
| 402 | - | return state, nil | 423 | + | return forkState, nil |
| 403 | } | 424 | } | ||
| 404 | 425 | ||||
| 405 | func writeBranchSession(root, cwd, version string, parent SessionMeta, messages []provider.Message, checkpoints []SessionUsageCheckpoint, limit int, hideFromSessions bool, extensionState map[string]json.RawMessage) (string, error) { | 426 | func writeBranchSession(root, cwd, version string, parent SessionMeta, messages []provider.Message, checkpoints []SessionUsageCheckpoint, limit int, hideFromSessions bool, extensionState map[string]json.RawMessage) (string, error) { | ||
M packages/core/session_portable_test.go +39 -0
| @@ -528,6 +528,45 @@ func TestBranchSessionCopiesExtensionStateAtFork(t *testing.T) { | |||
| 528 | 528 | } | |
| 529 | 529 | } | |
| 530 | 530 | ||
| 531 | + | func TestBranchSessionCarriesStateAcrossCompactionBoundary(t *testing.T) { | |
| 532 | + | root := t.TempDir() | |
| 533 | + | cwd := "/project" | |
| 534 | + | parent, err := NewSession(root, cwd, "anthropic", "claude", "test") | |
| 535 | + | if err != nil { | |
| 536 | + | t.Fatal(err) | |
| 537 | + | } | |
| 538 | + | for _, text := range []string{"before-a", "before-b"} { | |
| 539 | + | if err := parent.AppendMessage(provider.Message{Role: provider.RoleUser, Content: []provider.Content{provider.TextBlock{Text: text}}}); err != nil { | |
| 540 | + | t.Fatal(err) | |
| 541 | + | } | |
| 542 | + | } | |
| 543 | + | if err := parent.AppendExtensionState("tasked-phases", json.RawMessage(`{"version":1}`)); err != nil { | |
| 544 | + | t.Fatal(err) | |
| 545 | + | } | |
| 546 | + | if err := parent.AppendCompaction([]provider.Message{ | |
| 547 | + | {Role: provider.RoleAssistant, Content: []provider.Content{provider.TextBlock{Text: "summary"}}}, | |
| 548 | + | {Role: provider.RoleUser, Content: []provider.Content{provider.TextBlock{Text: "tail"}}}, | |
| 549 | + | }); err != nil { | |
| 550 | + | t.Fatal(err) | |
| 551 | + | } | |
| 552 | + | if err := parent.Close(); err != nil { | |
| 553 | + | t.Fatal(err) | |
| 554 | + | } | |
| 555 | + | ||
| 556 | + | branchPath, err := BranchSession(parent.Path, root, cwd, "test", 1) | |
| 557 | + | if err != nil { | |
| 558 | + | t.Fatal(err) | |
| 559 | + | } | |
| 560 | + | branch, _, err := OpenSession(branchPath) | |
| 561 | + | if err != nil { | |
| 562 | + | t.Fatal(err) | |
| 563 | + | } | |
| 564 | + | t.Cleanup(func() { _ = branch.Close() }) | |
| 565 | + | if got := string(branch.ExtensionState["tasked-phases"]); got != `{"version":1}` { | |
| 566 | + | t.Fatalf("compacted branch extension state = %q, want version 1", got) | |
| 567 | + | } | |
| 568 | + | } | |
| 569 | + | ||
| 531 | 570 | // TestBuildSessionTree verifies parent/child edges are rebuilt | |
| 532 | 571 | // from meta + sibling-scan. | |
| 533 | 572 | func TestBuildSessionTree(t *testing.T) { | |
| @@ -528,6 +528,45 @@ func TestBranchSessionCopiesExtensionStateAtFork(t *testing.T) { | |||||
| 528 | } | 528 | } | ||
| 529 | } | 529 | } | ||
| 530 | 530 | ||||
| 531 | + | func TestBranchSessionCarriesStateAcrossCompactionBoundary(t *testing.T) { | |||
| 532 | + | root := t.TempDir() | |||
| 533 | + | cwd := "/project" | |||
| 534 | + | parent, err := NewSession(root, cwd, "anthropic", "claude", "test") | |||
| 535 | + | if err != nil { | |||
| 536 | + | t.Fatal(err) | |||
| 537 | + | } | |||
| 538 | + | for _, text := range []string{"before-a", "before-b"} { | |||
| 539 | + | if err := parent.AppendMessage(provider.Message{Role: provider.RoleUser, Content: []provider.Content{provider.TextBlock{Text: text}}}); err != nil { | |||
| 540 | + | t.Fatal(err) | |||
| 541 | + | } | |||
| 542 | + | } | |||
| 543 | + | if err := parent.AppendExtensionState("tasked-phases", json.RawMessage(`{"version":1}`)); err != nil { | |||
| 544 | + | t.Fatal(err) | |||
| 545 | + | } | |||
| 546 | + | if err := parent.AppendCompaction([]provider.Message{ | |||
| 547 | + | {Role: provider.RoleAssistant, Content: []provider.Content{provider.TextBlock{Text: "summary"}}}, | |||
| 548 | + | {Role: provider.RoleUser, Content: []provider.Content{provider.TextBlock{Text: "tail"}}}, | |||
| 549 | + | }); err != nil { | |||
| 550 | + | t.Fatal(err) | |||
| 551 | + | } | |||
| 552 | + | if err := parent.Close(); err != nil { | |||
| 553 | + | t.Fatal(err) | |||
| 554 | + | } | |||
| 555 | + | ||||
| 556 | + | branchPath, err := BranchSession(parent.Path, root, cwd, "test", 1) | |||
| 557 | + | if err != nil { | |||
| 558 | + | t.Fatal(err) | |||
| 559 | + | } | |||
| 560 | + | branch, _, err := OpenSession(branchPath) | |||
| 561 | + | if err != nil { | |||
| 562 | + | t.Fatal(err) | |||
| 563 | + | } | |||
| 564 | + | t.Cleanup(func() { _ = branch.Close() }) | |||
| 565 | + | if got := string(branch.ExtensionState["tasked-phases"]); got != `{"version":1}` { | |||
| 566 | + | t.Fatalf("compacted branch extension state = %q, want version 1", got) | |||
| 567 | + | } | |||
| 568 | + | } | |||
| 569 | + | ||||
| 531 | // TestBuildSessionTree verifies parent/child edges are rebuilt | 570 | // TestBuildSessionTree verifies parent/child edges are rebuilt | ||
| 532 | // from meta + sibling-scan. | 571 | // from meta + sibling-scan. | ||
| 533 | func TestBuildSessionTree(t *testing.T) { | 572 | func TestBuildSessionTree(t *testing.T) { | ||