diff --git a/HANDOFF.md b/HANDOFF.md index 31100c1..b3e8d83 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -36,6 +36,7 @@ All features (CLI, MCP, and Rich TUI) are fully operational, tested, and integra > **Active Monitors & Agent Skill (Completed):** Added `## Active Monitors` to the Dossier schema (`assets/guide.md`) for tracking live, mutable context (e.g., Slack threads, Jira tickets) separately from static archived references. We have also created a "Resumption Protocol" Skill (`assets/skill.md`) that is embedded and written out during `dossier init` to `~/.dossier/context/skill.md`. `init` also configures Claude Code's `customInstructions` to point to this skill so agents automatically know to poll these monitors upon resuming a dossier. > **TUI catch-up to the session-id fix (2026-06-16) — later superseded:** The TUI's session/active presentation was updated to reflect real-session vs fallback (honest header banner + standalone footer warning). This was **superseded on 2026-06-17 by ADR 0004**, which removed the session/active concept from the TUI entirely (see the de-sessioned entry above). The honest-banner/footer work no longer exists. +> **Frontmatter backward compatibility (2026-06-25):** A store written by an older build (e.g. a three-level `importance: medium` from before the binary high/low schema) no longer hard-fails on load or edit. The invariant is *map toward attention, not away*: `Frontmatter.Normalize` (`internal/core/dossier.go`) coerces any value that is no longer a valid enum member, or a field absent from an older file, to its highest-attention valid value (`importance`/`urgency` → `high`, `status` → `active`). It is wired at three layers: (1) **read** — `CalculatePriorityScore` normalizes its copy so legacy values sort toward attention immediately; (2) **write** — `Service.Save` heals + persists + emits a `Warning` for every coercion; (3) **startup** — a version-gated one-time sweep (`Service.Migrate`, `config.schema_version` vs `core.CurrentSchemaVersion`) runs from `wire`, rewriting every stale Dossier once and logging to **stderr** (never stdout, so `mcp serve` stays clean). `doctor` reports legacy values as healable rather than fatal. The write-side source of `medium` was also closed (MCP tool schema, CLI `--importance/--urgency` mapping, SPEC §357). To extend for a future field: add the field's `Normalize` + one block in `Frontmatter.Normalize`, and bump `CurrentSchemaVersion`. diff --git a/SPEC.md b/SPEC.md index 6cb438b..2b9841f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -354,7 +354,7 @@ dossier status dossier lead "" dossier next "" dossier questions add|set|clear [...] -dossier priority --importance --urgency [--due ] +dossier priority --importance --urgency [--due ] dossier active [--session ] [--json] dossier switch [--session ] [--json] dossier path [] [--json] diff --git a/internal/cli/cli.go b/internal/cli/cli.go index d1646b5..6d0c51f 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -862,22 +862,25 @@ func NewRootCmd() *cobra.Command { updates := make(map[string]any) if importanceFlag != "" { switch importanceFlag { - case "h": + case "h", "high", "m", "medium": + // importance/urgency are binary (high|low); "medium" and other + // non-low values map toward attention rather than being written + // as an invalid value the store would later reject. updates["importance"] = "high" - case "l": + case "l", "low": updates["importance"] = "low" default: - updates["importance"] = importanceFlag + updates["importance"] = "high" } } if urgencyFlag != "" { switch urgencyFlag { - case "h": + case "h", "high", "m", "medium": updates["urgency"] = "high" - case "l": + case "l", "low": updates["urgency"] = "low" default: - updates["urgency"] = urgencyFlag + updates["urgency"] = "high" } } if dueFlag != "" { @@ -1194,7 +1197,28 @@ func wire(dossierHome string) (*core.Service, error) { hregAdapter := harness.NewRegistry(dossierHome) clockAdapter := &realClock{} - return core.NewService(storeAdapter, searchAdapter, tokAdapter, hregAdapter, clockAdapter, cfg.ToCoreConfig()), nil + svc := core.NewService(storeAdapter, searchAdapter, tokAdapter, hregAdapter, clockAdapter, cfg.ToCoreConfig()) + + // One-time, version-gated migration: when the store was last touched by an + // older build, eagerly heal any frontmatter the current schema no longer + // accepts (e.g. a removed enum value, or a newly required field). The version + // gate makes this a cheap no-op on every subsequent launch. Output goes to + // stderr so it never corrupts the MCP stdio protocol on `mcp serve`. + if cfg.SchemaVersion < core.CurrentSchemaVersion { + if res, err := svc.Migrate(context.Background()); err != nil { + fmt.Fprintf(os.Stderr, "Warning: frontmatter migration skipped: %v\n", err) + } else { + for _, w := range res.Warnings { + fmt.Fprintf(os.Stderr, "%s\n", w) + } + cfg.SchemaVersion = core.CurrentSchemaVersion + if err := cfg.Save(cfgPath); err != nil { + fmt.Fprintf(os.Stderr, "Warning: could not record schema version (migration will re-run next launch): %v\n", err) + } + } + } + + return svc, nil } func expandTilde(path string) string { diff --git a/internal/config/config.go b/internal/config/config.go index 0172282..c7675f2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -12,6 +12,11 @@ import ( type Config struct { DossierHome string `yaml:"dossier_home"` TokenTarget int `yaml:"token_target"` + // SchemaVersion records the frontmatter schema the store was last migrated to. + // It is intentionally left at its zero value in Default so that a config file + // written by an older build (which lacks the key) is detected as stale and + // triggers the one-time migration sweep on the next launch. + SchemaVersion int `yaml:"schema_version"` } // Default returns the default configuration with standard paths. diff --git a/internal/core/dossier.go b/internal/core/dossier.go index 7b02baf..f6e77e3 100644 --- a/internal/core/dossier.go +++ b/internal/core/dossier.go @@ -25,6 +25,19 @@ func (s Status) IsValid() bool { return false } +// Normalize coerces an invalid or missing status toward attention. +// Backward-compatibility rule: a value that is no longer recognized (a removed +// enum member) or that is empty (a field added after the file was written) maps +// to the highest-attention valid value, so a stale Dossier surfaces rather than +// silently sinking. It returns the coerced value and whether a change was made; +// it is idempotent on already-valid values. +func (s Status) Normalize() (Status, bool) { + if s.IsValid() { + return s, false + } + return StatusActive, true +} + // Importance represents the priority dimension of importance. type Importance string @@ -42,6 +55,16 @@ func (i Importance) IsValid() bool { return false } +// Normalize coerces an invalid or missing importance toward attention. +// See Status.Normalize for the backward-compatibility rule; the highest-attention +// value for importance is "high". +func (i Importance) Normalize() (Importance, bool) { + if i.IsValid() { + return i, false + } + return ImportanceHigh, true +} + // Urgency represents the priority dimension of urgency. type Urgency string @@ -59,6 +82,16 @@ func (u Urgency) IsValid() bool { return false } +// Normalize coerces an invalid or missing urgency toward attention. +// See Status.Normalize for the backward-compatibility rule; the highest-attention +// value for urgency is "high". +func (u Urgency) Normalize() (Urgency, bool) { + if u.IsValid() { + return u, false + } + return UrgencyHigh, true +} + // Frontmatter represents the parsed metadata block of a Dossier. // In conformance with BUILD-DECISIONS, base_revision is session-side, not in frontmatter. type Frontmatter struct { @@ -78,6 +111,43 @@ type Frontmatter struct { TokenTarget int `yaml:"token_target,omitempty"` } +// FrontmatterFix records a single backward-compatibility coercion applied by +// Normalize, so the change can be surfaced to the user rather than done silently. +type FrontmatterFix struct { + Field string + From string + To string +} + +// Normalize brings frontmatter into conformance with the current schema for +// backward compatibility, mutating the receiver and returning the list of +// coercions made (empty if already canonical). It is the single, extensible +// place that heals Dossiers written by older builds: +// +// - a value that is no longer a valid enum member (e.g. a removed "medium") +// is mapped toward attention by the field's own Normalize; +// - a field added after the file was written is empty, and likewise resolves +// to its attention default. +// +// To support a new enum field, add its Normalize and one block here. The method +// is pure (no I/O) and idempotent. +func (f *Frontmatter) Normalize() []FrontmatterFix { + var fixes []FrontmatterFix + if v, changed := f.Status.Normalize(); changed { + fixes = append(fixes, FrontmatterFix{Field: "status", From: string(f.Status), To: string(v)}) + f.Status = v + } + if v, changed := f.Importance.Normalize(); changed { + fixes = append(fixes, FrontmatterFix{Field: "importance", From: string(f.Importance), To: string(v)}) + f.Importance = v + } + if v, changed := f.Urgency.Normalize(); changed { + fixes = append(fixes, FrontmatterFix{Field: "urgency", From: string(f.Urgency), To: string(v)}) + f.Urgency = v + } + return fixes +} + // Validate ensures that all required fields are present and valid. func (f *Frontmatter) Validate() error { if f.ID == "" { diff --git a/internal/core/normalize_test.go b/internal/core/normalize_test.go new file mode 100644 index 0000000..b1e3463 --- /dev/null +++ b/internal/core/normalize_test.go @@ -0,0 +1,187 @@ +package core + +import ( + "context" + "strings" + "testing" + "time" +) + +func TestImportanceNormalize(t *testing.T) { + cases := []struct { + name string + in Importance + want Importance + changed bool + }{ + {"valid high stays", ImportanceHigh, ImportanceHigh, false}, + {"valid low stays", ImportanceLow, ImportanceLow, false}, + {"removed value maps up", Importance("medium"), ImportanceHigh, true}, + {"unknown value maps up", Importance("urgent"), ImportanceHigh, true}, + {"missing value maps up", Importance(""), ImportanceHigh, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, changed := tc.in.Normalize() + if got != tc.want || changed != tc.changed { + t.Fatalf("Normalize(%q) = (%q, %v), want (%q, %v)", tc.in, got, changed, tc.want, tc.changed) + } + // Idempotent: normalizing the result is a no-op. + if got2, changed2 := got.Normalize(); got2 != got || changed2 { + t.Fatalf("not idempotent: %q -> (%q, %v)", got, got2, changed2) + } + }) + } +} + +func TestUrgencyNormalize(t *testing.T) { + if got, changed := Urgency("medium").Normalize(); got != UrgencyHigh || !changed { + t.Fatalf("medium urgency: got (%q, %v), want (high, true)", got, changed) + } + if got, changed := UrgencyLow.Normalize(); got != UrgencyLow || changed { + t.Fatalf("valid low urgency should be unchanged, got (%q, %v)", got, changed) + } +} + +func TestStatusNormalize(t *testing.T) { + if got, changed := Status("stalled").Normalize(); got != StatusActive || !changed { + t.Fatalf("unknown status: got (%q, %v), want (active, true)", got, changed) + } + if got, changed := Status("").Normalize(); got != StatusActive || !changed { + t.Fatalf("missing status: got (%q, %v), want (active, true)", got, changed) + } + if got, changed := StatusResolved.Normalize(); got != StatusResolved || changed { + t.Fatalf("valid status should be unchanged, got (%q, %v)", got, changed) + } +} + +func TestFrontmatterNormalizeReportsAndHeals(t *testing.T) { + fm := Frontmatter{ + ID: "dos_x", + Name: "Legacy", + Slug: "legacy", + Status: "active", + Importance: "medium", // removed enum value + Urgency: "", // field absent in older build + } + fixes := fm.Normalize() + + if fm.Importance != ImportanceHigh || fm.Urgency != UrgencyHigh { + t.Fatalf("expected coercion toward attention, got importance=%q urgency=%q", fm.Importance, fm.Urgency) + } + if len(fixes) != 2 { + t.Fatalf("expected 2 fixes, got %d: %+v", len(fixes), fixes) + } + byField := map[string]FrontmatterFix{} + for _, f := range fixes { + byField[f.Field] = f + } + if got := byField["importance"]; got.From != "medium" || got.To != "high" { + t.Fatalf("importance fix = %+v, want medium -> high", got) + } + if got := byField["urgency"]; got.From != "" || got.To != "high" { + t.Fatalf("urgency fix = %+v, want '' -> high", got) + } + + // Idempotent: a second pass changes nothing. + if fixes2 := fm.Normalize(); len(fixes2) != 0 { + t.Fatalf("expected no fixes on second pass, got %+v", fixes2) + } +} + +func TestPriorityScoreMapsLegacyValueTowardAttention(t *testing.T) { + now := time.Date(2026, 6, 25, 0, 0, 0, 0, time.UTC) + fm := Frontmatter{Importance: "medium", Urgency: UrgencyHigh} + // medium importance must score as high importance (1: Do), not low (3: Delegate). + if got := CalculatePriorityScore(fm, now); got != 1 { + t.Fatalf("legacy medium importance scored %d, want 1 (toward attention)", got) + } +} + +func TestSaveHealsLegacyFrontmatterAndWarns(t *testing.T) { + fakeStore := newLocalFakeStore() + svc := NewService(fakeStore, &mockSearcher{}, &mockTokenizer{}, &mockHarnessRegistry{}, &mockClock{now: time.Date(2026, 6, 25, 12, 0, 0, 0, time.UTC)}, Config{TokenTarget: 100}) + ctx := context.Background() + + // Seed a dossier carrying a legacy "medium" importance directly in the store, + // simulating a file written by an older build. + fakeStore.dossiers["dos_fake_id"] = &Dossier{ + Frontmatter: Frontmatter{ + ID: "dos_fake_id", + Name: "Legacy", + Slug: "fake-slug", + Status: StatusActive, + Importance: "medium", + Urgency: UrgencyLow, + }, + DistilledState: DistilledState{Body: "# Legacy"}, + } + fakeStore.revisions["dos_fake_id"] = "rev_seed" + + res, err := svc.Save(ctx, SaveReq{ + ID: "dos_fake_id", + FrontmatterUpdates: map[string]any{"next_action": "touch"}, + }) + if err != nil { + t.Fatalf("save failed: %v", err) + } + + // The healed value is persisted... + d, _, err := fakeStore.Read("dos_fake_id") + if err != nil { + t.Fatalf("read failed: %v", err) + } + if d.Frontmatter.Importance != ImportanceHigh { + t.Fatalf("expected importance healed to high, got %q", d.Frontmatter.Importance) + } + + // ...and the coercion is surfaced, not silent. + var warned bool + for _, w := range res.Warnings { + if strings.Contains(string(w), "importance") && strings.Contains(string(w), "medium") { + warned = true + } + } + if !warned { + t.Fatalf("expected a warning about the importance coercion, got %v", res.Warnings) + } +} + +func TestMigrateHealsStoreWideAndIsIdempotent(t *testing.T) { + fakeStore := newLocalFakeStore() + svc := NewService(fakeStore, &mockSearcher{}, &mockTokenizer{}, &mockHarnessRegistry{}, &mockClock{now: time.Date(2026, 6, 25, 12, 0, 0, 0, time.UTC)}, Config{TokenTarget: 100}) + ctx := context.Background() + + // One legacy dossier and one already-canonical dossier. + fakeStore.dossiers["dos_legacy"] = &Dossier{ + Frontmatter: Frontmatter{ID: "dos_legacy", Name: "Legacy", Slug: "legacy", Status: StatusActive, Importance: "medium", Urgency: UrgencyLow}, + DistilledState: DistilledState{Body: "# Legacy"}, + } + fakeStore.revisions["dos_legacy"] = "rev_legacy" + fakeStore.dossiers["dos_ok"] = &Dossier{ + Frontmatter: Frontmatter{ID: "dos_ok", Name: "OK", Slug: "ok", Status: StatusActive, Importance: ImportanceLow, Urgency: UrgencyLow}, + DistilledState: DistilledState{Body: "# OK"}, + } + fakeStore.revisions["dos_ok"] = "rev_ok" + + res, err := svc.Migrate(ctx) + if err != nil { + t.Fatalf("migrate failed: %v", err) + } + rep := res.Data.(MigrateReport) + if rep.DossiersScanned != 2 || rep.DossiersHealed != 1 { + t.Fatalf("expected scanned=2 healed=1, got %+v", rep) + } + if d, _, _ := fakeStore.Read("dos_legacy"); d.Frontmatter.Importance != ImportanceHigh { + t.Fatalf("legacy dossier not healed, importance=%q", d.Frontmatter.Importance) + } + + // Idempotent: a second sweep heals nothing. + res2, err := svc.Migrate(ctx) + if err != nil { + t.Fatalf("second migrate failed: %v", err) + } + if rep2 := res2.Data.(MigrateReport); rep2.DossiersHealed != 0 { + t.Fatalf("expected no heals on second sweep, got %+v", rep2) + } +} diff --git a/internal/core/priority.go b/internal/core/priority.go index b6dd2ca..8f1e0e4 100644 --- a/internal/core/priority.go +++ b/internal/core/priority.go @@ -11,6 +11,11 @@ import ( // - 3: Low Importance, High Urgency ("3. Delegate") // - 4: Low Importance, Low Urgency ("4. Delete") func CalculatePriorityScore(fm Frontmatter, now time.Time) int { + // fm is passed by value; normalize the copy so legacy/invalid priority + // fields (e.g. a removed "medium") score toward attention rather than + // silently collapsing to low — matching how they will heal on next write. + fm.Normalize() + isHighImportance := fm.Importance == ImportanceHigh isHighUrgency := fm.Urgency == UrgencyHigh diff --git a/internal/core/service.go b/internal/core/service.go index 90c73ea..978ce8b 100644 --- a/internal/core/service.go +++ b/internal/core/service.go @@ -164,7 +164,14 @@ func (s *Service) Doctor(ctx context.Context) (Result, error) { for _, fm := range fms { report.DossiersChecked++ - if err := fm.Validate(); err != nil { + // Report legacy/invalid enum values as healable rather than fatal: they + // are coerced toward attention on the next write. Anything still invalid + // after normalization (e.g. a missing id or name) is a real problem. + normalized := fm + for _, fix := range normalized.Normalize() { + addIssue("Dossier %s has legacy %s %q; it will heal to %q on next edit.", fm.ID, fix.Field, fix.From, fix.To) + } + if err := normalized.Validate(); err != nil { addIssue("Dossier %s has invalid frontmatter: %v", fm.ID, err) } @@ -224,6 +231,71 @@ func (s *Service) Doctor(ctx context.Context) (Result, error) { }, nil } +// CurrentSchemaVersion is the frontmatter schema version this build expects. +// Bump it whenever Frontmatter.Normalize gains a rule that should be applied +// eagerly to existing stores; the startup sweep runs once when the store's +// recorded version is older than this. +const CurrentSchemaVersion = 1 + +// MigrateReport summarizes a one-time frontmatter migration sweep. +type MigrateReport struct { + DossiersScanned int `json:"dossiers_scanned"` + DossiersHealed int `json:"dossiers_healed"` +} + +// Migrate normalizes every Dossier's frontmatter to the current schema and +// rewrites the ones that changed. It is the eager, store-wide counterpart to the +// lazy heal that Save performs on a single Dossier: idempotent (re-running over +// an already-canonical store is a no-op), non-destructive (rewrites go through +// the audited Write path), and safe to run on an empty store. Each coercion is +// reported as a Warning so the migration is never silent. +func (s *Service) Migrate(ctx context.Context) (Result, error) { + if s.store == nil { + return Result{OK: false}, NewError(ErrInternal, "store not configured") + } + + report := MigrateReport{} + var warnings []Warning + + fms, err := s.store.List("all") + if err != nil { + return Result{OK: false}, err + } + + for _, fm := range fms { + report.DossiersScanned++ + + d, rev, err := s.store.Read(fm.ID) + if err != nil { + warnings = append(warnings, Warning(fmt.Sprintf("Dossier %s could not be read during migration: %v", fm.ID, err))) + continue + } + + fixes := d.Frontmatter.Normalize() + if len(fixes) == 0 { + continue + } + + if _, err := s.store.Write(d, rev); err != nil { + warnings = append(warnings, Warning(fmt.Sprintf("Dossier %s could not be healed during migration: %v", fm.ID, err))) + continue + } + + report.DossiersHealed++ + for _, fix := range fixes { + warnings = append(warnings, Warning(fmt.Sprintf( + "Dossier %s: coerced %s %q -> %q for backward compatibility (mapping toward attention).", + fm.ID, fix.Field, fix.From, fix.To))) + } + } + + return Result{ + OK: true, + Data: report, + Warnings: warnings, + }, nil +} + var provenanceRefRE = regexp.MustCompile(`\[src:([A-Za-z0-9_]+)(?:#[^\]]+)?\]`) func validateDistilledStateProvenance(body string, dossierID string, artifactExists func(string) bool) []string { @@ -694,6 +766,17 @@ func (s *Service) Save(ctx context.Context, req SaveReq) (Result, error) { d.Frontmatter.LastTouchedAt = s.clock.Now() + // Backward compatibility: heal any legacy/invalid frontmatter (e.g. a value + // from a removed enum, or a field absent in an older build) before the write + // path validates it. The coercion is persisted (the file self-heals) and + // surfaced as a warning so it is never silent. + var warnings []Warning + for _, fix := range d.Frontmatter.Normalize() { + warnings = append(warnings, Warning(fmt.Sprintf( + "Coerced %s %q -> %q for backward compatibility (mapping toward attention).", + fix.Field, fix.From, fix.To))) + } + newRev, err := s.store.Write(d, baseRev) if err != nil { return Result{}, err @@ -741,8 +824,9 @@ func (s *Service) Save(ctx context.Context, req SaveReq) (Result, error) { _ = s.store.AppendAudit(d.Frontmatter.ID, event) return Result{ - OK: true, - Data: newRev, + OK: true, + Data: newRev, + Warnings: warnings, }, nil } diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index afe185f..e0a45a4 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -180,8 +180,8 @@ func getToolDefinitions() []ToolDefinition { "lead": map[string]any{"type": "string", "description": "Replace the lead assignee (omit to leave unchanged)"}, "next_action": map[string]any{"type": "string", "description": "Replace the current next action (omit to leave unchanged)"}, "open_questions": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": "Replace the open questions list (omit to leave unchanged)"}, - "importance": map[string]any{"type": "string", "description": "low|medium|high (omit to leave unchanged)"}, - "urgency": map[string]any{"type": "string", "description": "low|medium|high (omit to leave unchanged)"}, + "importance": map[string]any{"type": "string", "description": "high|low (omit to leave unchanged)"}, + "urgency": map[string]any{"type": "string", "description": "high|low (omit to leave unchanged)"}, "due_date": map[string]any{"type": "string", "description": "ISO 8601 date or empty string to clear (omit to leave unchanged)"}, }, "required": []string{"id"}, diff --git a/internal/tui/tui_test.go b/internal/tui/tui_test.go index db6f1b5..fc4ffa5 100644 --- a/internal/tui/tui_test.go +++ b/internal/tui/tui_test.go @@ -594,4 +594,3 @@ func TestHeaderHasNoSession(t *testing.T) { } } } -