From b3d9ff9aa4849f58932fefbc80dca013e411ff19 Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 07:52:28 -0400 Subject: [PATCH 1/8] feat(store): add space filename codec and v5 file shapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Groundwork for one-file-per-space storage: a percent-encoding that maps any legal space name to a safe filename (covering only /, %, and a leading dot), and the two on-disk shapes of the coming v5 layout — a document-level manifest and a self-describing space file whose type has no field for consent flags to travel in. Co-Authored-By: Claude Fable 5 --- internal/store/filename.go | 92 +++++++++++++++++++++++++++ internal/store/filename_test.go | 107 ++++++++++++++++++++++++++++++++ internal/store/tree.go | 44 +++++++++++++ 3 files changed, 243 insertions(+) create mode 100644 internal/store/filename.go create mode 100644 internal/store/filename_test.go create mode 100644 internal/store/tree.go diff --git a/internal/store/filename.go b/internal/store/filename.go new file mode 100644 index 0000000..3fd64e2 --- /dev/null +++ b/internal/store/filename.go @@ -0,0 +1,92 @@ +package store + +import ( + "fmt" + "strings" +) + +// A space's file is named after the space, but a space name is user text and a +// filename is not: ValidateSpaceName rejects only control characters, so "/", +// "%", and names starting with "." are all legal spaces today — and planDir +// already joins the raw name into a path, which this mapping exists to stop +// doing. The encoding covers exactly the three characters that are unsafe in a +// filename and nothing else, so almost every space's file is named literally +// after it. +// +// The escape character itself must be escaped or the mapping is not injective +// ("50%" and "50%25" would collide); a leading dot is escaped so no space file +// is ever hidden, which is also what keeps the directory scan's "skip dotfiles" +// rule — temp files are dot-prefixed — from skipping a real space. +// +// The filename is derived and the embedded name is canonical: a file that was +// hand-copied, or whose name a filesystem normalized (HFS+ stores NFD), still +// belongs to the space its `name` field says, and the filename is repaired on +// the next write. + +// spacesDirSuffix names the sidecar directory holding one file per space, +// following the .lock/.bak/.plans convention of siblings named after the file +// they belong to. +const spacesDirSuffix = ".spaces" + +// spaceFileExt is the extension every space file carries, so the directory +// scan has a positive marker and a stray .bak or editor droppings are never +// read as a space. +const spaceFileExt = ".json" + +// encodeSpaceFilename maps a space name to its filename (without directory). +func encodeSpaceFilename(name string) string { + var b strings.Builder + for i, r := range name { + switch { + case r == '%' || r == '/': + fmt.Fprintf(&b, "%%%02X", r) + case r == '.' && i == 0: + b.WriteString("%2E") + default: + b.WriteRune(r) + } + } + return b.String() + spaceFileExt +} + +// decodeSpaceFilename maps a filename back to the space name it encodes. It is +// the inverse of encodeSpaceFilename on anything that function produced, and +// lenient on anything else: a "%" not followed by two hex digits stays literal, +// because this also names spaces whose files cannot be parsed — a strict +// decoder would leave a corrupt space with no name to report it under. +func decodeSpaceFilename(filename string) (string, bool) { + base, ok := strings.CutSuffix(filename, spaceFileExt) + if !ok || base == "" { + return "", false + } + return percentDecode(base), true +} + +func percentDecode(s string) string { + var b strings.Builder + for i := 0; i < len(s); i++ { + if s[i] == '%' && i+2 < len(s) { + if hi, ok1 := unhex(s[i+1]); ok1 { + if lo, ok2 := unhex(s[i+2]); ok2 { + b.WriteByte(hi<<4 | lo) + i += 2 + continue + } + } + } + b.WriteByte(s[i]) + } + return b.String() +} + +func unhex(c byte) (byte, bool) { + switch { + case '0' <= c && c <= '9': + return c - '0', true + case 'a' <= c && c <= 'f': + return c - 'a' + 10, true + case 'A' <= c && c <= 'F': + return c - 'A' + 10, true + } + return 0, false +} diff --git a/internal/store/filename_test.go b/internal/store/filename_test.go new file mode 100644 index 0000000..ba64083 --- /dev/null +++ b/internal/store/filename_test.go @@ -0,0 +1,107 @@ +package store + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestSpaceFilenameRoundTrip(t *testing.T) { + names := []string{ + "work", + "default", + "side projects", + "50% done", + "a/b testing", + ".hidden", + "..", + "%2F", // a name that looks pre-encoded must still round-trip + "%", + "café", // NFC form; the codec must not touch non-ASCII + "日本語", + "a.json", // a name ending in the extension itself + strings.Repeat("x", 32), + } + for _, name := range names { + enc := encodeSpaceFilename(name) + if !strings.HasSuffix(enc, spaceFileExt) { + t.Errorf("encode(%q) = %q, missing %s suffix", name, enc, spaceFileExt) + } + if strings.ContainsRune(enc, '/') { + t.Errorf("encode(%q) = %q contains a path separator", name, enc) + } + if strings.HasPrefix(enc, ".") { + t.Errorf("encode(%q) = %q is a dotfile", name, enc) + } + dec, ok := decodeSpaceFilename(enc) + if !ok || dec != name { + t.Errorf("decode(encode(%q)) = %q, %v; want the name back", name, dec, ok) + } + } +} + +func TestSpaceFilenameInjective(t *testing.T) { + // Names that would collide under a codec that forgot to escape its own + // escape character, or that escaped a leading dot without escaping "%2E". + pairs := [][2]string{ + {"50%", "50%25"}, + {".x", "%2Ex"}, + {"a/b", "a%2Fb"}, + } + for _, p := range pairs { + if encodeSpaceFilename(p[0]) == encodeSpaceFilename(p[1]) { + t.Errorf("encode(%q) == encode(%q) == %q; codec is not injective", + p[0], p[1], encodeSpaceFilename(p[0])) + } + } +} + +func TestDecodeSpaceFilenameRejectsNonSpaceFiles(t *testing.T) { + for _, f := range []string{"work.json.bak", "notes.md", ".json", "tasks.json.lock"} { + if name, ok := decodeSpaceFilename(f); ok { + t.Errorf("decode(%q) = %q, ok; want rejection", f, name) + } + } +} + +func TestSpaceFileMarshalShape(t *testing.T) { + sf := spaceFile{ + Version: 5, + Name: "work", + Data: Data{NextID: 3, Space: "derived", MCPAllowed: true, AgentAllowed: true}, + } + b, err := marshalJSONFile(sf) + if err != nil { + t.Fatal(err) + } + if b[len(b)-1] != '\n' { + t.Error("marshaled space file has no trailing newline") + } + var raw map[string]any + if err := json.Unmarshal(b, &raw); err != nil { + t.Fatal(err) + } + for _, key := range []string{"version", "name", "next_id"} { + if _, ok := raw[key]; !ok { + t.Errorf("space file is missing %q", key) + } + } + // The derived fields and the consent flags must have no way onto disk: + // consent must never travel with a copied or exported space. + for _, key := range []string{"space", "spaces", "all_spaces", "mcp_enabled", "agent_enabled", "current"} { + if _, ok := raw[key]; ok { + t.Errorf("space file must not contain %q", key) + } + } + + var back spaceFile + if err := json.Unmarshal(b, &back); err != nil { + t.Fatal(err) + } + if back.Name != "work" || back.NextID != 3 { + t.Errorf("round trip = %+v, want Name=work NextID=3", back) + } + if back.Space != "" || back.MCPAllowed || back.AgentAllowed { + t.Error("derived fields survived a round trip; they must stay json:\"-\"") + } +} diff --git a/internal/store/tree.go b/internal/store/tree.go new file mode 100644 index 0000000..cf12a1e --- /dev/null +++ b/internal/store/tree.go @@ -0,0 +1,44 @@ +package store + +import ( + "encoding/json" +) + +// The version-5 layout splits the document across files: a small manifest at +// the data path holding the document-level fields, and one file per space in +// the .spaces sidecar directory. The two shapes below are what those files +// hold. File remains the in-memory document — these exist only at the disk +// boundary, so nothing above readTree/writeTree changes shape. + +// manifest is the on-disk form of tasks.json at version 5: exactly the +// document-level fields, and deliberately no "spaces" key. A version-4 binary +// reading it therefore fails the version check outright rather than decoding +// an empty matrix it would overwrite — the same refusal the v3 and v4 bumps +// were chosen for. +type manifest struct { + Version int `json:"version"` + Current string `json:"current"` + MCPEnabled bool `json:"mcp_enabled,omitempty"` + AgentEnabled bool `json:"agent_enabled,omitempty"` +} + +// spaceFile is the on-disk form of one space. The embedded name is canonical +// (the filename is derived from it and repaired on write), and the shape is +// self-describing so the same file works as the export format: consent flags +// have no field to travel in, which turns export's "never carry consent" +// guarantee from a decision into a property of the type. +type spaceFile struct { + Version int `json:"version"` + Name string `json:"name"` + Data +} + +// marshalJSONFile renders v the way every ike file is written: indented, with +// a trailing newline. +func marshalJSONFile(v any) ([]byte, error) { + b, err := json.MarshalIndent(v, "", " ") + if err != nil { + return nil, err + } + return append(b, '\n'), nil +} From c164eb9ff416f23eeab4c51290c6a42ece3cd80d Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 07:55:20 -0400 Subject: [PATCH 2/8] refactor(store): fold the read path into readTree ahead of per-space files readFile becomes readTree, gaining the (still unreachable) version-5 branches: a manifest read that enumerates the .spaces sidecar directory, a standalone single-space-file shape for --file on an export, and reconstruction of a document whose manifest is missing. File carries read-state (raw bytes, filenames, corrupt spaces, on-disk version) so the coming write side can confine writes to what changed. The v1-v4 path is byte-for-byte the old behavior; the whole suite passing without a single test edit is the check. Co-Authored-By: Claude Fable 5 --- internal/store/ops.go | 4 +- internal/store/store.go | 109 ++++++------------ internal/store/transfer.go | 2 +- internal/store/tree.go | 230 +++++++++++++++++++++++++++++++++++++ 4 files changed, 265 insertions(+), 80 deletions(-) diff --git a/internal/store/ops.go b/internal/store/ops.go index f215a57..328c695 100644 --- a/internal/store/ops.go +++ b/internal/store/ops.go @@ -240,7 +240,7 @@ func (s *Store) SetMCPEnabled(on bool) (changed bool, err error) { // MCPEnabled reports whether the MCP server may serve this file. func (s *Store) MCPEnabled() (bool, error) { - f, err := readFile(s.path) + f, err := readTree(s.path) if err != nil { return false, s.redact(err) } @@ -274,7 +274,7 @@ func (s *Store) SetAgentEnabled(on bool) (changed bool, err error) { // AgentEnabled reports whether ike may run an agent against this file. func (s *Store) AgentEnabled() (bool, error) { - f, err := readFile(s.path) + f, err := readTree(s.path) if err != nil { return false, s.redact(err) } diff --git a/internal/store/store.go b/internal/store/store.go index 48acdb0..168aee7 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -14,10 +14,8 @@ import ( "encoding/json" "errors" "fmt" - "maps" "os" "path/filepath" - "slices" "strings" "time" @@ -88,6 +86,36 @@ type File struct { Spaces map[string]*Data `json:"spaces"` MCPEnabled bool `json:"mcp_enabled,omitempty"` AgentEnabled bool `json:"agent_enabled,omitempty"` + + // The fields below are read-state: what readTree saw on disk, carried so + // the write side can tell what actually changed. None of them are part of + // the document, and File itself is never marshaled wholesale at version 5 + // — the json tags above remain for reading version-4 envelopes. + + // onDiskVersion is the version the document file held when read, or 0 if + // there was none. The write side migrates when it is 1 through 4. + onDiskVersion int + // rawDoc holds the document file's bytes as read — the monolith to back + // up before a migration, or the manifest to compare against for a + // dirty check. + rawDoc []byte + // rawSpace holds each space file's bytes as read, keyed by space name. + // A space whose current marshaling matches is not rewritten, which is + // what confines a write to the spaces it touched. + rawSpace map[string][]byte + // fileFor records which filename each space was actually read from, + // which is not always the encoding of its name: the embedded name is + // canonical, and a hand-copied or normalization-mangled filename is + // repaired on the next write rather than trusted. + fileFor map[string]string + // corrupt lists spaces whose files could not be parsed, by display name. + // They are absent from Spaces and from rawSpace, so no write can touch + // their files; reads surface them instead of failing the whole document. + corrupt map[string]error + // standalone marks a Store opened directly on a single space file — an + // export handed to --file. The document then has exactly that space, + // writes go back to the same file, and space lifecycle operations refuse. + standalone bool } // Data is one space: a complete matrix, and the unit every operation acts on. @@ -336,7 +364,7 @@ func (s *Store) Load() (Data, error) { // spaces must keep working when the pinned one does not exist, since a listing // is how you find that out. func (s *Store) loadFile() (File, error) { - f, err := readFile(s.path) + f, err := readTree(s.path) if err != nil { return File{}, s.redact(err) } @@ -437,7 +465,7 @@ func (s *Store) mutateFile(fn func(*File) error) (file File, err error) { } }() - file, err = readFile(s.path) + file, err = readTree(s.path) if err != nil { return File{}, s.redact(err) } @@ -455,79 +483,6 @@ func (s *Store) mutateFile(fn func(*File) error) (file File, err error) { return file, nil } -func readFile(path string) (File, error) { - b, err := os.ReadFile(path) - if errors.Is(err, os.ErrNotExist) { - return emptyFile(), nil - } - if err != nil { - return File{}, err - } - var f File - if err := json.Unmarshal(b, &f); err != nil { - return File{}, fmt.Errorf("parsing %s: %w", path, err) - } - if f.Version < 1 || f.Version > currentVersion { - return File{}, fmt.Errorf("%s has unsupported version %d (expected %d)", path, f.Version, currentVersion) - } - - // Older files are upgraded in memory; the next write persists the upgrade. - // - // The discriminator is the version, never `spaces == nil`. A single-matrix - // file carries `version` and `mcp_enabled` at the same top level the - // envelope does, so it has already decoded into the right two fields and - // the body only needs re-reading as one space. Going by the missing key - // instead would quietly accept a truncated or hand-edited v4 file as an - // empty matrix, and the next write would erase the lot. - if f.Version < currentVersion { - var d Data - if err := json.Unmarshal(b, &d); err != nil { - return File{}, fmt.Errorf("parsing %s: %w", path, err) - } - // History from before version 3 is dropped rather than reinterpreted. - // Those snapshots stored a whole archive and no ArchiveEntry, so undoing - // a restore recorded by an older build would silently lose that entry's - // completion stamp. Losing undo history on a one-time upgrade is a far - // better outcome than quietly losing an archived task, and the tasks - // themselves are untouched either way. - if f.Version < 3 { - d.Undo, d.Redo = nil, nil - } - f.Spaces = map[string]*Data{defaultSpace: &d} - f.Current = defaultSpace - } - if len(f.Spaces) == 0 { - return File{}, fmt.Errorf("%s has no spaces", path) - } - f.Version = currentVersion - - for name, d := range f.Spaces { - if d == nil { - return File{}, fmt.Errorf("%s has an empty space %q", path, name) - } - if d.NextID < 1 { - d.NextID = 1 - } - // Before normalizeRanks, so a task rescued from an invalid quadrant gets - // a rank in the quadrant it lands in. - clampQuadrants(d) - // Every space, not just the one about to be read. A write persists them - // all, so a space left un-normalized would have its pre-rank ordering - // rewritten by whichever mutation happened to touch a different space. - normalizeRanks(d) - } - // A current that names nothing — a hand edit, or a space removed by a build - // that did not follow it — would otherwise break every command until it was - // fixed by hand. Repair it the way an out-of-range NextID is repaired. An - // explicitly requested space that is missing still fails: "the file is - // inconsistent" and "you asked for something that is not there" are - // different situations and deserve different answers. - if _, ok := f.Spaces[f.Current]; !ok { - f.Current = slices.Min(slices.Collect(maps.Keys(f.Spaces))) - } - return f, nil -} - func writeFileAtomic(path string, doc File) error { b, err := json.MarshalIndent(doc, "", " ") if err != nil { diff --git a/internal/store/transfer.go b/internal/store/transfer.go index 45f73f3..459c1c1 100644 --- a/internal/store/transfer.go +++ b/internal/store/transfer.go @@ -84,7 +84,7 @@ func (s *Store) ImportSpaces(path, as string, all bool) ([]SpaceInfo, error) { if _, err := os.Stat(p); err != nil { return nil, err } - src, err := readFile(p) + src, err := readTree(p) if err != nil { return nil, err } diff --git a/internal/store/tree.go b/internal/store/tree.go index cf12a1e..883f7ea 100644 --- a/internal/store/tree.go +++ b/internal/store/tree.go @@ -2,6 +2,13 @@ package store import ( "encoding/json" + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "slices" + "strings" ) // The version-5 layout splits the document across files: a small manifest at @@ -42,3 +49,226 @@ func marshalJSONFile(v any) ([]byte, error) { } return append(b, '\n'), nil } + +// spacesDir is the sidecar directory holding path's space files. +func spacesDir(path string) string { return path + spacesDirSuffix } + +// readTree reads the whole document from disk: the file at path, plus — at +// version 5 — one file per space beside it. It replaces the old readFile and +// keeps its contract: reads never write, older versions are upgraded in +// memory and persisted by the next write, and a version newer than this build +// writes is refused outright. +func readTree(path string) (File, error) { + b, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + // No document file. A populated spaces directory beside the missing + // path is still a document — losing the small manifest must not hide + // every space — so it is reconstructed the way a dangling Current is + // repaired: current becomes the alphabetically first space, and both + // consent flags are off, consent being the one thing a repair must + // never invent. + if f, ok, derr := readOrphanSpaces(path); derr != nil || ok { + return f, derr + } + return emptyFile(), nil + } + if err != nil { + return File{}, err + } + var f File + if err := json.Unmarshal(b, &f); err != nil { + return File{}, fmt.Errorf("parsing %s: %w", path, err) + } + if f.Version < 1 || f.Version > currentVersion { + return File{}, fmt.Errorf("%s has unsupported version %d (expected %d)", path, f.Version, currentVersion) + } + f.onDiskVersion = f.Version + f.rawDoc = b + + // The discriminator between the shapes below is the version, never a + // missing key. Every shape carries `version` at the top level, so it has + // already decoded; going by absent keys instead would quietly accept a + // truncated or hand-edited file as an empty matrix, and the next write + // would erase the lot. + switch { + case f.Version < 4: + // A single-matrix file: the body re-reads as one space. + var d Data + if err := json.Unmarshal(b, &d); err != nil { + return File{}, fmt.Errorf("parsing %s: %w", path, err) + } + // History from before version 3 is dropped rather than reinterpreted. + // Those snapshots stored a whole archive and no ArchiveEntry, so undoing + // a restore recorded by an older build would silently lose that entry's + // completion stamp. Losing undo history on a one-time upgrade is a far + // better outcome than quietly losing an archived task, and the tasks + // themselves are untouched either way. + if f.Version < 3 { + d.Undo, d.Redo = nil, nil + } + f.Spaces = map[string]*Data{defaultSpace: &d} + f.Current = defaultSpace + case f.Version == 4: + // The whole document in one file; the envelope has already decoded + // into f.Spaces. + default: + // Version 5: either the manifest of a split document, or a single + // space file handed to --file. A space file is the only v5 shape with + // a name, so that is the discriminator — the manifest deliberately + // has no such key. + var sf spaceFile + if err := json.Unmarshal(b, &sf); err != nil { + return File{}, fmt.Errorf("parsing %s: %w", path, err) + } + if sf.Name != "" { + d := sf.Data + f.Current = sf.Name + f.Spaces = map[string]*Data{sf.Name: &d} + f.rawSpace = map[string][]byte{sf.Name: b} + f.standalone = true + // A space file has no field for either consent flag, but the + // envelope decode above shares File's json tags with the v4 shape, + // so a crafted file could smuggle them in. Consent never travels. + f.MCPEnabled, f.AgentEnabled = false, false + break + } + if err := readSpaces(path, &f); err != nil { + return File{}, err + } + } + // A version-5 manifest with no space files gets the same refusal as a v4 + // envelope with no spaces: a truncated or half-copied tree must be + // refused, not accepted as an empty matrix the next write makes permanent. + // Spaces that exist but cannot be parsed count as present here — they are + // exactly what a degraded open exists to keep answering about. + if len(f.Spaces) == 0 && len(f.corrupt) == 0 { + return File{}, fmt.Errorf("%s has no spaces", path) + } + f.Version = currentVersion + + for name, d := range f.Spaces { + if d == nil { + return File{}, fmt.Errorf("%s has an empty space %q", path, name) + } + if d.NextID < 1 { + d.NextID = 1 + } + // Before normalizeRanks, so a task rescued from an invalid quadrant gets + // a rank in the quadrant it lands in. + clampQuadrants(d) + // Every space, not just the one about to be read. A write persists them + // all, so a space left un-normalized would have its pre-rank ordering + // rewritten by whichever mutation happened to touch a different space. + normalizeRanks(d) + } + // A current that names nothing — a hand edit, or a space removed by a build + // that did not follow it — would otherwise break every command until it was + // fixed by hand. Repair it the way an out-of-range NextID is repaired. An + // explicitly requested space that is missing still fails: "the file is + // inconsistent" and "you asked for something that is not there" are + // different situations and deserve different answers. When every space is + // unreadable there is nothing to repair toward, and Current is left alone + // so the listing still says which space was current. + if _, ok := f.Spaces[f.Current]; !ok && len(f.Spaces) > 0 { + f.Current = slices.Min(slices.Collect(maps.Keys(f.Spaces))) + } + return f, nil +} + +// readSpaces reads every space file in path's spaces directory into f. +// +// A file that cannot be read or parsed marks its space corrupt instead of +// failing the document — one bad space costing every other space is exactly +// what the split layout exists to prevent. A corrupt space lands in f.corrupt +// and nowhere else, so nothing downstream can write to, over, or instead of +// its file. +func readSpaces(path string, f *File) error { + dir := spacesDir(path) + entries, err := os.ReadDir(dir) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return err + } + f.Spaces = map[string]*Data{} + f.rawSpace = map[string][]byte{} + f.fileFor = map[string]string{} + // ReadDir returns entries sorted by filename, which is what makes "the + // lexicographically first file wins" below deterministic. + for _, e := range entries { + fname := e.Name() + // Dotfiles are the atomic-write temp files; encodeSpaceFilename + // escapes a leading dot, so no real space is ever skipped by this. + if e.IsDir() || strings.HasPrefix(fname, ".") { + continue + } + derived, ok := decodeSpaceFilename(fname) + if !ok { + continue // .bak siblings and other strays are not space files + } + b, err := os.ReadFile(filepath.Join(dir, fname)) + if err != nil { + f.markCorrupt(derived, err) + continue + } + var sf spaceFile + if err := json.Unmarshal(b, &sf); err != nil { + f.markCorrupt(derived, err) + continue + } + if sf.Version < 1 || sf.Version > currentVersion { + f.markCorrupt(derived, fmt.Errorf("unsupported version %d (expected %d)", sf.Version, currentVersion)) + continue + } + if sf.Name == "" { + f.markCorrupt(derived, errors.New("space file has no name")) + continue + } + // The embedded name is canonical; the filename is derived from it and + // repaired on the next write if they disagree. Two files claiming one + // name would otherwise be one space with two futures: the first + // filename wins, the other is surfaced rather than silently shadowed, + // and — being corrupt — its file can never be written or deleted. + if prior, dup := f.fileFor[sf.Name]; dup { + f.markCorrupt(derived, fmt.Errorf("%s and %s both claim the space %q; %s wins", prior, fname, sf.Name, prior)) + continue + } + d := sf.Data + f.Spaces[sf.Name] = &d + f.rawSpace[sf.Name] = b + f.fileFor[sf.Name] = fname + } + return nil +} + +// readOrphanSpaces reconstructs a document from a spaces directory whose +// manifest is missing. It reports ok=false when there is nothing there — +// the ordinary fresh-start case. +func readOrphanSpaces(path string) (File, bool, error) { + f := File{Version: currentVersion} + if err := readSpaces(path, &f); err != nil { + return File{}, false, err + } + if len(f.Spaces) == 0 && len(f.corrupt) == 0 { + return File{}, false, nil + } + if len(f.Spaces) > 0 { + f.Current = slices.Min(slices.Collect(maps.Keys(f.Spaces))) + } + for _, d := range f.Spaces { + if d.NextID < 1 { + d.NextID = 1 + } + clampQuadrants(d) + normalizeRanks(d) + } + return f, true, nil +} + +func (f *File) markCorrupt(name string, err error) { + if f.corrupt == nil { + f.corrupt = map[string]error{} + } + f.corrupt[name] = err +} From 89e36e3aefde8becee6a6e8035d753db02fe6d25 Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 08:02:58 -0400 Subject: [PATCH 3/8] feat(store): store each space in its own file (schema v5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tasks.json becomes a small manifest (version, current, consent flags — and deliberately no spaces key, so a v4 binary refuses it rather than reading an empty matrix). Spaces live in tasks.json.spaces/, one self-describing file each, written through the same lock and the same atomic helpers: mutateFile is still the one write path. The commit step is dirty-checked per file — a mutation rewrites only the spaces it touched, so a bug or corruption in one space can no longer clobber the others — and ordered creates/updates, manifest, deletes so every crash window leaves a state the reader already repairs. Each space file keeps its own rolling .bak; deleting a space is one atomic rename to .bak. A skipped write tightens file modes by hand, since it no longer replaces the inode. Migration off the monolith happens on the first write, never on read, with the original file kept as tasks.json.pre-v5.bak and the manifest write as the commit point; debris of a crashed migration is cleared before the split is retried. Consent readers now read the manifest alone, so mcp/agent status keeps answering whatever state the space files are in. Export writes the space-file shape, which import accepts alongside v4 envelopes and whole v5 trees. Co-Authored-By: Claude Fable 5 --- internal/store/durability_test.go | 27 +++- internal/store/ops.go | 15 +- internal/store/plans_test.go | 21 ++- internal/store/spaces.go | 25 ++- internal/store/store.go | 31 ++-- internal/store/store_test.go | 148 ++++++++++++++--- internal/store/transfer.go | 27 ++-- internal/store/transfer_test.go | 21 ++- internal/store/tree.go | 256 ++++++++++++++++++++++++++++++ 9 files changed, 499 insertions(+), 72 deletions(-) diff --git a/internal/store/durability_test.go b/internal/store/durability_test.go index f66cacd..b1427c4 100644 --- a/internal/store/durability_test.go +++ b/internal/store/durability_test.go @@ -102,24 +102,28 @@ func TestWriteDoesNotUsePredictableTempName(t *testing.T) { } } -// Losing the data file used to mean hand-editing JSON or starting over. +// Losing the data used to mean hand-editing JSON or starting over. Since the +// split into per-space files the backup lives beside each space's file — a +// task mutation touches one space, so that is the file whose previous state +// needs preserving. func TestWriteKeepsABackupOfThePreviousContents(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "tasks.json") + spacePath := filepath.Join(spacesDir(path), encodeSpaceFilename(defaultSpace)) s := OpenAt(path) if _, _, err := s.Add("first", task.Do); err != nil { t.Fatal(err) } // No backup yet: there was nothing to preserve before the first write. - if _, err := os.Stat(path + ".bak"); !os.IsNotExist(err) { + if _, err := os.Stat(spacePath + ".bak"); !os.IsNotExist(err) { t.Errorf("unexpected backup after the first write: %v", err) } if _, _, err := s.Add("second", task.Do); err != nil { t.Fatal(err) } - bak, err := os.ReadFile(path + ".bak") + bak, err := os.ReadFile(spacePath + ".bak") if err != nil { t.Fatalf("no backup after the second write: %v", err) } @@ -130,13 +134,28 @@ func TestWriteKeepsABackupOfThePreviousContents(t *testing.T) { t.Error("backup holds the new state, not the previous one") } // The backup is as private as the data file. - fi, err := os.Stat(path + ".bak") + fi, err := os.Stat(spacePath + ".bak") if err != nil { t.Fatal(err) } if got := fi.Mode().Perm(); got != dataFileMode { t.Errorf("backup mode = %#o, want %#o", got, dataFileMode) } + + // The manifest gets the same treatment when it is the thing that changed. + if _, err := s.NewSpace("other"); err != nil { + t.Fatal(err) + } + if _, err := s.UseSpace("other"); err != nil { + t.Fatal(err) + } + mbak, err := os.ReadFile(path + ".bak") + if err != nil { + t.Fatalf("no manifest backup after a document-level change: %v", err) + } + if !strings.Contains(string(mbak), `"current": "default"`) { + t.Errorf("manifest backup does not hold the previous state: %s", mbak) + } } // Every byte of a completed write must be on disk before the rename, or a diff --git a/internal/store/ops.go b/internal/store/ops.go index 328c695..7c8d89a 100644 --- a/internal/store/ops.go +++ b/internal/store/ops.go @@ -239,12 +239,16 @@ func (s *Store) SetMCPEnabled(on bool) (changed bool, err error) { } // MCPEnabled reports whether the MCP server may serve this file. +// +// It reads the document file alone rather than the whole tree: consent is a +// property of the document, and `ike mcp status` must keep answering when a +// space file — even every space file — cannot be parsed. func (s *Store) MCPEnabled() (bool, error) { - f, err := readTree(s.path) + m, err := readDocFlags(s.path) if err != nil { return false, s.redact(err) } - return f.MCPEnabled, nil + return m.MCPEnabled, nil } // SetAgentEnabled turns delegation on or off for this file, and reports @@ -272,13 +276,14 @@ func (s *Store) SetAgentEnabled(on bool) (changed bool, err error) { return changed, err } -// AgentEnabled reports whether ike may run an agent against this file. +// AgentEnabled reports whether ike may run an agent against this file. Like +// MCPEnabled, it reads the document file alone. func (s *Store) AgentEnabled() (bool, error) { - f, err := readTree(s.path) + m, err := readDocFlags(s.path) if err != nil { return false, s.redact(err) } - return f.AgentEnabled, nil + return m.AgentEnabled, nil } // Rename changes a task's title. diff --git a/internal/store/plans_test.go b/internal/store/plans_test.go index 73af8c4..7cc74b2 100644 --- a/internal/store/plans_test.go +++ b/internal/store/plans_test.go @@ -54,17 +54,24 @@ func TestPlanBodyStaysOutOfTheDataFile(t *testing.T) { s.Add("filler", task.Schedule) } - b, err := os.ReadFile(p) + spacePath := filepath.Join(spacesDir(p), encodeSpaceFilename(defaultSpace)) + for _, f := range []string{p, spacePath} { + b, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), "Ship it.") { + t.Errorf("the plan body reached %s; it belongs in the sidecar, "+ + "or every undo snapshot carries a copy of it", f) + } + } + // The stamp, by contrast, must be there — it is what marks the task planned. + b, err := os.ReadFile(spacePath) if err != nil { t.Fatal(err) } - if strings.Contains(string(b), "Ship it.") { - t.Error("the plan body reached tasks.json; it belongs in the sidecar, " + - "or every undo snapshot carries a copy of it") - } - // The stamp, by contrast, must be there — it is what marks the task planned. if !strings.Contains(string(b), "plan_at") { - t.Error("the PlanAt stamp should persist in the data file") + t.Error("the PlanAt stamp should persist in the space file") } } diff --git a/internal/store/spaces.go b/internal/store/spaces.go index 68e1b14..8ea5f5b 100644 --- a/internal/store/spaces.go +++ b/internal/store/spaces.go @@ -113,8 +113,20 @@ func (f *File) checkNewName(name string) error { // The space operations below are deliberately **not undoable**. History is // per-space, so a document-level change has no stack to record onto, and a // stack that could resurrect a removed space would have to hold the whole -// matrix. `tasks.json.bak` remains the recovery path for a removal, which is -// why RemoveSpace makes the caller say the name and confirm the loss. +// matrix. The space file renamed to `.bak` remains the recovery path for a +// removal, which is why RemoveSpace makes the caller say the name and confirm +// the loss. + +// checkLifecycle refuses a space lifecycle change on a standalone document — +// a single exported space file opened with --file. Such a file can hold +// exactly one space, so every operation that would change the set of spaces +// has no way to persist its result. +func (f *File) checkLifecycle(op string) error { + if f.standalone { + return fmt.Errorf("cannot %s: this is a single exported space, not a full data file; import it first with `ike space import`", op) + } + return nil +} // ListSpaces describes every space in the file, sorted by name. func (s *Store) ListSpaces() ([]SpaceInfo, error) { @@ -138,6 +150,9 @@ func (s *Store) NewSpace(name string) (Data, error) { name = strings.TrimSpace(name) var out Data _, err := s.mutateFile(func(f *File) error { + if err := f.checkLifecycle("create a space"); err != nil { + return err + } if err := f.checkNewName(name); err != nil { return err } @@ -174,6 +189,9 @@ func (s *Store) UseSpace(name string) (Data, error) { func (s *Store) RenameSpace(from, to string) error { from, to = strings.TrimSpace(from), strings.TrimSpace(to) _, err := s.mutateFile(func(f *File) error { + if err := f.checkLifecycle("rename a space"); err != nil { + return err + } canonical, d, err := f.resolve(from) if err != nil { return err @@ -209,6 +227,9 @@ func (s *Store) RemoveSpace(name string, force bool) (SpaceInfo, error) { name = strings.TrimSpace(name) var removed SpaceInfo _, err := s.mutateFile(func(f *File) error { + if err := f.checkLifecycle("remove a space"); err != nil { + return err + } canonical, d, err := f.resolve(name) if err != nil { return err diff --git a/internal/store/store.go b/internal/store/store.go index 168aee7..1ec69bc 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -11,7 +11,6 @@ package store import ( "context" - "encoding/json" "errors" "fmt" "os" @@ -47,17 +46,20 @@ const lockRetryInterval = 25 * time.Millisecond // currentVersion is the schema version this build writes. Version 2 added // per-task ranks and the undo stack; version 3 stopped copying the whole // archive into every snapshot; version 4 wrapped the matrix in a document that -// can hold several of them. Older files are upgraded in memory on read and -// persisted at the current version by the next write. +// can hold several of them; version 5 split the document across files — a +// manifest at the data path and one file per space beside it — so one corrupt +// space cannot take the others with it. Older files are upgraded in memory on +// read and persisted at the current version by the next write. // -// Both 3 and 4 are real bumps rather than fields added in place — the approach +// 3, 4, and 5 are real bumps rather than fields added in place — the approach // taken for `redo` — because the failure modes differ. Losing redo history to // an older binary is harmless; an older binary reading a v3 file would find no // "archive" in a snapshot, decode it as empty, and wipe the archive on the next // undo, and one reading a v4 file would find no top-level "tasks" at all and -// see an empty matrix it was about to overwrite. Better that it refuse the file -// outright. -const currentVersion = 4 +// see an empty matrix it was about to overwrite. A v5 manifest deliberately has +// no "spaces" key for the same reason: a v4 binary must refuse it outright +// rather than decode an empty matrix it was about to make permanent. +const currentVersion = 5 // defaultSpace names the space a single-matrix file is upgraded into, and the // one a fresh file starts with. @@ -477,25 +479,12 @@ func (s *Store) mutateFile(fn func(*File) error) (file File, err error) { if err = fn(&file); err != nil { return File{}, err } - if err = writeFileAtomic(s.path, file); err != nil { + if err = writeTree(s.path, &file); err != nil { return File{}, s.redact(err) } return file, nil } -func writeFileAtomic(path string, doc File) error { - b, err := json.MarshalIndent(doc, "", " ") - if err != nil { - return err - } - b = append(b, '\n') - - if err := writeBackup(path); err != nil { - return err - } - return writeBytesAtomic(path, ".tasks-*.json", b) -} - // writeBytesAtomic replaces path with b, atomically and durably. It is the body // writeFileAtomic used to hold inline, lifted out so that the plan sidecars // (plans.go) get the same four guarantees rather than a second, untested copy diff --git a/internal/store/store_test.go b/internal/store/store_test.go index a25c375..21e8835 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,10 +2,8 @@ package store import ( "encoding/json" - "maps" "os" "path/filepath" - "slices" "sync" "testing" "time" @@ -241,42 +239,43 @@ func onDiskVersion(t *testing.T, path string) int { return int(v) } -// rawSpace returns one space's body from the file on disk. +// rawSpace returns one space's body from its file on disk. func rawSpace(t *testing.T, path, name string) map[string]any { t.Helper() - spaces, ok := rawFile(t, path)["spaces"].(map[string]any) - if !ok { - t.Fatalf("%s has no spaces object", path) - } - body, ok := spaces[name].(map[string]any) - if !ok { - t.Fatalf("%s has no space %q (spaces: %v)", path, name, slices.Sorted(maps.Keys(spaces))) - } - return body + return rawFile(t, filepath.Join(spacesDir(path), encodeSpaceFilename(name))) } func TestFileFormat(t *testing.T) { s := testStore(t) s.Add("x", task.Do) + // The manifest holds the document-level fields, and deliberately no + // "spaces" key: a version-4 binary must refuse it on the version check, + // not decode an empty matrix it was about to overwrite. m := rawFile(t, s.Path()) - for _, k := range []string{"version", "current", "spaces"} { + for _, k := range []string{"version", "current"} { if _, ok := m[k]; !ok { - t.Errorf("file missing top-level key %q", k) + t.Errorf("manifest missing top-level key %q", k) } } - // The matrix itself sits one level down, under its space. + for _, k := range []string{"spaces", "tasks", "next_id"} { + if _, ok := m[k]; ok { + t.Errorf("manifest should not hold key %q", k) + } + } + // The matrix lives in the space's own file, which is self-describing — + // the same shape `ike space export` writes. body := rawSpace(t, s.Path(), defaultSpace) - for _, k := range []string{"next_id", "tasks"} { + for _, k := range []string{"version", "name", "next_id", "tasks"} { if _, ok := body[k]; !ok { - t.Errorf("space missing key %q", k) + t.Errorf("space file missing key %q", k) } } - // The fields derived from the document on every read describe the file, not - // the matrix, and must never be written into a space. - for _, k := range []string{"space", "spaces", "mcp_enabled", "version"} { + // The derived fields describe the document, not the matrix, and the + // consent flags must have no way into a file that export can copy. + for _, k := range []string{"space", "spaces", "all_spaces", "current", "mcp_enabled", "agent_enabled"} { if _, ok := body[k]; ok { - t.Errorf("space should not persist key %q", k) + t.Errorf("space file should not persist key %q", k) } } } @@ -336,6 +335,113 @@ func TestUpgradesSingleMatrixFile(t *testing.T) { } } +// The v4→v5 migration happens on the first write, never on read, and the +// monolith survives it twice over: as the rolling .bak of the manifest that +// replaced it, and as a one-time .pre-v5.bak that nothing ever overwrites. +func TestMigrationSplitsMonolithOnFirstWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "tasks.json") + v4 := `{ + "version": 4, + "current": "work", + "mcp_enabled": true, + "spaces": { + "work": {"next_id": 2, "tasks": [{"id": 1, "title": "a", "quadrant": 1, "rank": 1024}]}, + "personal": {"next_id": 1, "tasks": []} + } + }` + if err := os.WriteFile(path, []byte(v4), 0o600); err != nil { + t.Fatal(err) + } + s := OpenAt(path) + + // Reads serve the old layout indefinitely and write nothing. + if _, err := s.Load(); err != nil { + t.Fatal(err) + } + if v := onDiskVersion(t, path); v != 4 { + t.Errorf("a read migrated the file to version %d", v) + } + if _, err := os.Stat(spacesDir(path)); !os.IsNotExist(err) { + t.Error("a read created the spaces directory") + } + + // The first mutation migrates the whole document. + if _, _, err := s.InSpace("personal").Add("x", task.Do); err != nil { + t.Fatal(err) + } + if v := onDiskVersion(t, path); v != currentVersion { + t.Errorf("on-disk version = %d, want %d", v, currentVersion) + } + m := rawFile(t, path) + if _, ok := m["spaces"]; ok { + t.Error("the manifest still holds a spaces key") + } + if cur, _ := m["current"].(string); cur != "work" { + t.Errorf("current = %q, want %q preserved", cur, "work") + } + if on, _ := m["mcp_enabled"].(bool); !on { + t.Error("mcp_enabled should survive the migration") + } + work := rawSpace(t, path, "work") + if tasks, _ := work["tasks"].([]any); len(tasks) != 1 { + t.Errorf("work tasks = %v, want the monolith's task", work["tasks"]) + } + personal := rawSpace(t, path, "personal") + if tasks, _ := personal["tasks"].([]any); len(tasks) != 1 { + t.Errorf("personal tasks = %v, want the added task", personal["tasks"]) + } + + // The pre-migration monolith is kept, byte for byte, and a later write + // does not touch it — the rolling .bak is clobbered by the very next + // document-level change, so this copy is the durable escape hatch. + bak, err := os.ReadFile(path + preV5BackupSuffix) + if err != nil { + t.Fatal(err) + } + if string(bak) != v4 { + t.Errorf("pre-v5 backup = %s, want the original monolith", bak) + } + if _, _, err := s.Add("later", task.Do); err != nil { + t.Fatal(err) + } + bak2, _ := os.ReadFile(path + preV5BackupSuffix) + if string(bak2) != v4 { + t.Error("a later write rewrote the pre-v5 backup") + } +} + +// A migration that crashed before its commit point — the manifest write — +// leaves space files beside a still-authoritative monolith. The next +// migration must clear them, or a space deleted since the crash would +// resurrect beside the real ones. +func TestMigrationClearsDebrisOfACrashedMigration(t *testing.T) { + path := filepath.Join(t.TempDir(), "tasks.json") + v4 := `{"version": 4, "current": "work", + "spaces": {"work": {"next_id": 2, "tasks": [{"id": 1, "title": "real", "quadrant": 1, "rank": 1024}]}}}` + if err := os.WriteFile(path, []byte(v4), 0o600); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(spacesDir(path), 0o700); err != nil { + t.Fatal(err) + } + stale := `{"version": 5, "name": "deleted-since", "next_id": 9, "tasks": []}` + staleWork := `{"version": 5, "name": "work", "next_id": 9, "tasks": []}` + os.WriteFile(filepath.Join(spacesDir(path), "deleted-since.json"), []byte(stale), 0o600) + os.WriteFile(filepath.Join(spacesDir(path), "work.json"), []byte(staleWork), 0o600) + + s := OpenAt(path) + if _, _, err := s.Add("x", task.Do); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(spacesDir(path), "deleted-since.json")); !os.IsNotExist(err) { + t.Error("crash debris survived the migration and resurrected a space") + } + work := rawSpace(t, path, "work") + if id, _ := work["next_id"].(float64); int(id) != 3 { + t.Errorf("work next_id = %v, want the monolith's state (3), not the debris", work["next_id"]) + } +} + // A v4 file with no spaces is a truncated or hand-mangled file, not an empty // matrix. Accepting it would mean the next write erased whatever was there. func TestVersionFourWithNoSpacesIsAnError(t *testing.T) { diff --git a/internal/store/transfer.go b/internal/store/transfer.go index 459c1c1..f631b62 100644 --- a/internal/store/transfer.go +++ b/internal/store/transfer.go @@ -14,17 +14,19 @@ import ( // — export, copy the one file, import — which is why they live beside the space // operations rather than inside them. -// ExportSpace writes one space to path as a standalone ike data file: a normal -// document holding just that space, which opens with `ike --file` and imports -// with `ike space import`. +// ExportSpace writes one space to path as a standalone space file — the same +// shape the space's own file in the .spaces directory holds, so an export is +// the portability goal made literal: copying the file and exporting it produce +// the same bytes. It opens with `ike --file` and imports with `ike space +// import`. // // Both consent flags — MCP access and agent delegation — are deliberately left // off in the exported file, whatever they are here. Consent is a decision about // a file on a machine, and an export exists to be copied elsewhere: carrying // "agents may read this", still less "ike may start an agent", along to a // machine whose owner never said so would be the wrong default in the one -// direction that matters. The out literal below gets this by construction, by -// naming only the three fields an export carries. +// direction that matters. Since version 5 the space-file shape has no field for +// either flag, so this holds by construction rather than by decision. // // Plan bodies are *not* exported. They live beside the data file rather than in // it (see plans.go), so an export carries the tasks and their PlanAt stamps but @@ -50,12 +52,14 @@ func (s *Store) ExportSpace(name, path string, force bool) (SpaceInfo, error) { return SpaceInfo{}, err } } - out := File{ - Version: currentVersion, - Current: canonical, - Spaces: map[string]*Data{canonical: d}, + b, err := marshalJSONFile(spaceFile{Version: currentVersion, Name: canonical, Data: *d}) + if err != nil { + return SpaceInfo{}, err } - if err := writeFileAtomic(p, out); err != nil { + if err := writeBackup(p); err != nil { + return SpaceInfo{}, err + } + if err := writeBytesAtomic(p, ".space-*.json", b); err != nil { return SpaceInfo{}, err } return SpaceInfo{ @@ -102,6 +106,9 @@ func (s *Store) ImportSpaces(path, as string, all bool) ([]SpaceInfo, error) { var imported []SpaceInfo _, err = s.mutateFile(func(f *File) error { + if err := f.checkLifecycle("import into this file"); err != nil { + return err + } imported = nil for _, from := range take { d, ok := src.Spaces[from] diff --git a/internal/store/transfer_test.go b/internal/store/transfer_test.go index 4ae9032..a0d4722 100644 --- a/internal/store/transfer_test.go +++ b/internal/store/transfer_test.go @@ -217,8 +217,9 @@ func TestImportAllTakesEverySpace(t *testing.T) { t.Fatal(err) } out := filepath.Join(t.TempDir(), "all.json") - // Export cannot write more than one space, so copy the whole source file: - // that is what "another machine's data file" looks like anyway. + // Export cannot write more than one space, so copy the whole source tree — + // manifest plus spaces directory — which is what "another machine's data + // file" looks like since the split. b, err := os.ReadFile(src.Path()) if err != nil { t.Fatal(err) @@ -226,6 +227,22 @@ func TestImportAllTakesEverySpace(t *testing.T) { if err := os.WriteFile(out, b, 0o600); err != nil { t.Fatal(err) } + entries, err := os.ReadDir(spacesDir(src.Path())) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(spacesDir(out), 0o700); err != nil { + t.Fatal(err) + } + for _, e := range entries { + sb, err := os.ReadFile(filepath.Join(spacesDir(src.Path()), e.Name())) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(spacesDir(out), e.Name()), sb, 0o600); err != nil { + t.Fatal(err) + } + } // Rename the local space out of the way, so importing "default" has a name // free to land on. diff --git a/internal/store/tree.go b/internal/store/tree.go index 883f7ea..598d5af 100644 --- a/internal/store/tree.go +++ b/internal/store/tree.go @@ -1,6 +1,7 @@ package store import ( + "bytes" "encoding/json" "errors" "fmt" @@ -272,3 +273,258 @@ func (f *File) markCorrupt(name string, err error) { } f.corrupt[name] = err } + +// readDocFlags reads only the document file's manifest-level fields. The +// consent readers use it so that `ike mcp status` and `ike agent status` keep +// answering whatever state the space files are in. Every version carries these +// fields at the top level, so no version branch is needed; a standalone space +// file simply has neither flag, which is the "consent never travels" guarantee +// again. +func readDocFlags(path string) (manifest, error) { + b, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return manifest{}, nil + } + if err != nil { + return manifest{}, err + } + var m struct { + manifest + Name string `json:"name"` + } + if err := json.Unmarshal(b, &m); err != nil { + return manifest{}, fmt.Errorf("parsing %s: %w", path, err) + } + if m.Name != "" { + // A space file, not a document. Its shape has no consent fields, but a + // crafted one could carry them anyway; a standalone open never has + // consent, so neither may the flags read from one. + return manifest{Version: m.Version}, nil + } + return m.manifest, nil +} + +// preV5BackupSuffix names the one-time copy of a pre-split monolith, taken +// before the first version-5 write replaces it. The rolling .bak is clobbered +// by the very next manifest write, so without this the pre-migration state +// would survive exactly one mutation. +const preV5BackupSuffix = ".pre-v5.bak" + +// writeTree persists the document f to disk: each space to its own file, the +// manifest last among updates, deletions after that. It is the commit step of +// mutateFile and nothing else calls it, so the lock, the fresh re-read, and +// the gate still have exactly one implementation. +// +// Writes are confined to what changed: a space whose marshaling matches the +// bytes it was read from is left alone. Fault tolerance is the point — a bug +// in one space's mutation can no longer rewrite the others — and it is also +// what keeps the ordering rule cheap. That rule: creations and updates first, +// then the manifest, then deletions. Every crash window then leaves a state +// readTree already repairs — at worst an extra space file the manifest does +// not point to, or a dangling Current. +// +// A space readTree marked corrupt is in neither f.Spaces nor f.rawSpace, so +// this function cannot write to, over, or instead of its file: the +// reads-never-overwrite-what-they-cannot-parse rule, extended per file. +func writeTree(path string, f *File) error { + if f.standalone { + return writeStandalone(path, f) + } + dir := spacesDir(path) + + // Migrating from a monolith. The monolith stays authoritative until the + // manifest write below replaces it: a crash anywhere before that leaves a + // valid pre-v5 file the next reader upgrades again, and an older binary + // running mid-window still sees a document it understands. + migrating := f.onDiskVersion >= 1 && f.onDiskVersion < currentVersion + if migrating { + if err := writeFileOnce(path+preV5BackupSuffix, f.rawDoc); err != nil { + return err + } + } + if err := os.MkdirAll(dir, dataDirMode); err != nil { + return err + } + // MkdirAll leaves an existing directory's mode alone; the listing of + // space names is as personal as the spaces, so tighten it the way a + // skipped write tightens a file. + _ = os.Chmod(dir, dataDirMode) + if migrating { + // Any space file already present is debris of a migration that crashed + // before its commit point. Cleared rather than trusted, or a space + // deleted since that crash would resurrect beside the real ones. + entries, err := os.ReadDir(dir) + if err != nil { + return err + } + for _, e := range entries { + if _, ok := decodeSpaceFilename(e.Name()); ok && !strings.HasPrefix(e.Name(), ".") { + if err := os.Remove(filepath.Join(dir, e.Name())); err != nil { + return err + } + } + } + } + + // Creations and updates, in sorted order so a partial failure is + // reproducible. + for _, name := range slices.Sorted(maps.Keys(f.Spaces)) { + b, err := marshalJSONFile(spaceFile{Version: currentVersion, Name: name, Data: *f.Spaces[name]}) + if err != nil { + return err + } + canonical := encodeSpaceFilename(name) + target := filepath.Join(dir, canonical) + dirty := !bytes.Equal(b, f.rawSpace[name]) + if dirty { + if err := writeBackup(target); err != nil { + return err + } + if err := writeBytesAtomic(target, ".space-*.json", b); err != nil { + return err + } + } else { + // The monolith relied on every write replacing the inode to + // tighten a file left world-readable by an older build. A skipped + // write must keep that promise by hand. + _ = os.Chmod(target, dataFileMode) + } + // The filename is derived from the canonical embedded name; a file + // read from anywhere else — a hand copy, a filesystem that normalized + // the encoding — is moved home. Best effort: a repair must never be + // the reason a mutation fails, and a straggler is surfaced by the + // duplicate-name handling on the next read rather than lost. + if prior := f.fileFor[name]; prior != "" && prior != canonical { + priorPath := filepath.Join(dir, prior) + switch { + case sameFile(priorPath, target): + // A case-insensitive filesystem: one file, wrongly-cased + // entry. Renaming it in place fixes the case. + _ = os.Rename(priorPath, target) + case dirty: + // target was written fresh above; the old file is superseded. + _ = os.Rename(priorPath, priorPath+".bak") + default: + _ = os.Rename(priorPath, target) + } + } + } + + // The manifest, only if it changed. During a migration it always has — + // the monolith's bytes are not a manifest — and this write is the commit + // point that retires the monolith. + mb, err := marshalJSONFile(manifest{ + Version: currentVersion, + Current: f.Current, + MCPEnabled: f.MCPEnabled, + AgentEnabled: f.AgentEnabled, + }) + if err != nil { + return err + } + if !bytes.Equal(mb, f.rawDoc) { + if err := writeBackup(path); err != nil { + return err + } + if err := writeBytesAtomic(path, ".tasks-*.json", mb); err != nil { + return err + } + } else { + _ = os.Chmod(path, dataFileMode) + } + + // Deletions last, so a crash strands an extra file rather than losing one. + // Deleting is renaming to .bak: removal and backup in one atomic step, + // which keeps ".bak is the recovery path for a removal" true now that the + // document file no longer holds spaces at all. + for _, name := range slices.Sorted(maps.Keys(f.rawSpace)) { + if _, live := f.Spaces[name]; live { + continue + } + fname := f.fileFor[name] + if fname == "" { + continue + } + full := filepath.Join(dir, fname) + // A rename that changed only the name's case shares one file between + // the old entry and the new on a case-insensitive filesystem — the + // update above already wrote the new content into it, and "deleting" + // the old entry here would delete the file it just wrote. Fix the + // entry's case instead. + caseOnly := false + for live := range f.Spaces { + target := filepath.Join(dir, encodeSpaceFilename(live)) + if strings.EqualFold(fname, encodeSpaceFilename(live)) && sameFile(full, target) { + _ = os.Rename(full, target) + caseOnly = true + break + } + } + if caseOnly { + continue + } + if err := os.Rename(full, full+".bak"); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } + return nil +} + +// writeStandalone writes the one space of a Store opened directly on a space +// file back to that file. The lifecycle operations refuse on a standalone +// document, so exactly one space can be here. +func writeStandalone(path string, f *File) error { + if len(f.Spaces) != 1 { + return fmt.Errorf("standalone %s must hold exactly one space, has %d", path, len(f.Spaces)) + } + for name, d := range f.Spaces { + b, err := marshalJSONFile(spaceFile{Version: currentVersion, Name: name, Data: *d}) + if err != nil { + return err + } + if bytes.Equal(b, f.rawSpace[name]) { + return nil + } + if err := writeBackup(path); err != nil { + return err + } + return writeBytesAtomic(path, ".space-*.json", b) + } + return nil +} + +// writeFileOnce creates path with b unless it already exists. O_EXCL rather +// than a stat-then-write, so two racing migrations cannot both think they +// wrote first. +func writeFileOnce(path string, b []byte) error { + out, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, dataFileMode) + if errors.Is(err, os.ErrExist) { + return nil + } + if err != nil { + return err + } + if _, err := out.Write(b); err != nil { + out.Close() + return err + } + if err := out.Sync(); err != nil { + out.Close() + return err + } + return out.Close() +} + +// sameFile reports whether two paths name one file, which on a +// case-insensitive filesystem two differently-cased names do. +func sameFile(a, b string) bool { + fa, err := os.Stat(a) + if err != nil { + return false + } + fb, err := os.Stat(b) + if err != nil { + return false + } + return os.SameFile(fa, fb) +} From 7223d7586060404901ed1fb14c2b027f46c53fc4 Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 08:03:46 -0400 Subject: [PATCH 4/8] feat(store): ModTime covers the manifest and the spaces directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task mutations now land as renames inside tasks.json.spaces/, which the old single stat of tasks.json never saw — an open TUI would have gone stale forever. Two stats cover every kind of write: renames into the spaces directory bump its mtime (task edits, space create/remove/ rename), and document-level changes rewrite the manifest itself. Co-Authored-By: Claude Fable 5 --- internal/store/store.go | 27 +++++++++++++++++++-------- internal/store/store_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index 1ec69bc..d426bf6 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -338,16 +338,27 @@ type redactedError struct { func (e *redactedError) Error() string { return e.msg } func (e *redactedError) Unwrap() error { return e.err } -// ModTime returns the data file's mtime, or the zero time if it does not exist. +// ModTime returns the newest mtime across the data file and the spaces +// directory, or zero if neither exists. It is the TUI's only change signal, +// polled every couple of seconds, so it must stay cheap — two stats — while +// still moving on every kind of write: a task mutation lands a rename inside +// the spaces directory, which bumps the directory's mtime; a space created, +// removed, or renamed changes the directory listing, likewise; and a `space +// use` or consent change rewrites the manifest itself. The one thing it no +// longer sees is an in-place hand edit of a space file, which was never the +// contract. func (s *Store) ModTime() (mtime int64, err error) { - fi, err := os.Stat(s.path) - if errors.Is(err, os.ErrNotExist) { - return 0, nil - } - if err != nil { - return 0, err + for _, p := range []string{s.path, spacesDir(s.path)} { + fi, serr := os.Stat(p) + if errors.Is(serr, os.ErrNotExist) { + continue + } + if serr != nil { + return 0, serr + } + mtime = max(mtime, fi.ModTime().UnixNano()) } - return fi.ModTime().UnixNano(), nil + return mtime, nil } // Load reads this Store's space without taking the write lock. diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 21e8835..1907659 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -214,6 +214,42 @@ func TestConcurrentWriters(t *testing.T) { } } +// ModTime is the TUI's only change signal, so every kind of write must move +// it: a task mutation now lands in a space file, not the polled tasks.json, +// and a signal that missed those would leave an open TUI stale forever. +func TestModTimeMovesOnEveryKindOfWrite(t *testing.T) { + s := testStore(t) + + if m, err := s.ModTime(); err != nil || m != 0 { + t.Fatalf("ModTime before any write = %d, %v; want 0", m, err) + } + + last := int64(0) + bump := func(step string, op func() error) { + t.Helper() + // Coarse-mtime filesystems need real time between writes for the + // signal to be observable at all. + time.Sleep(10 * time.Millisecond) + if err := op(); err != nil { + t.Fatalf("%s: %v", step, err) + } + m, err := s.ModTime() + if err != nil { + t.Fatalf("%s: ModTime: %v", step, err) + } + if m <= last { + t.Errorf("%s did not move ModTime (%d -> %d)", step, last, m) + } + last = m + } + + bump("add", func() error { _, _, err := s.Add("x", task.Do); return err }) + bump("second add", func() error { _, _, err := s.Add("y", task.Do); return err }) + bump("space new", func() error { _, err := s.NewSpace("other"); return err }) + bump("space use", func() error { _, err := s.UseSpace("other"); return err }) + bump("space rm", func() error { _, err := s.RemoveSpace("other", false); return err }) +} + // rawFile decodes the data file as plain JSON, for assertions about the shape // on disk rather than the shape in memory. func rawFile(t *testing.T, path string) map[string]any { From 269ba34a493f3d95b1db464f7aaa6091d08ebe30 Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 08:08:10 -0400 Subject: [PATCH 5/8] feat(store): degrade gracefully when a space file cannot be parsed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The point of the split: one corrupt space file costs that one space. Every other space keeps loading and mutating; the broken one is listed — in the TUI picker, `ike space list`, and list_spaces — as unreadable rather than silently missing, and resolving it says what is wrong and what to do, not "no space named X". No write path can touch an unreadable file: it is absent from the dirty-write and deletion sets by construction. A Current naming an unreadable space is deliberately not repaired away — the user must be told loudly, not shown some other space. The consent readers answer from the manifest even when every space is corrupt. The one sanctioned way at a corrupt file is `ike space rm --force`, which renames it to .bak in case it can still be recovered by hand. Co-Authored-By: Claude Fable 5 --- internal/cli/spaces.go | 17 ++++- internal/store/spaces.go | 43 ++++++++++- internal/store/store.go | 6 +- internal/store/store_test.go | 138 +++++++++++++++++++++++++++++++++++ internal/store/tree.go | 55 +++++++++++--- internal/tui/spaces.go | 6 +- internal/tui/view.go | 10 ++- 7 files changed, 258 insertions(+), 17 deletions(-) diff --git a/internal/cli/spaces.go b/internal/cli/spaces.go index 51e9618..2238f93 100644 --- a/internal/cli/spaces.go +++ b/internal/cli/spaces.go @@ -176,6 +176,11 @@ func spaceSummary(spaces []store.SpaceInfo) string { // spaceCounts describes what a space holds as a listing column. func spaceCounts(sp store.SpaceInfo) string { + if sp.Unreadable { + // Counts would be a lie — the file cannot be parsed, so nothing is + // known about what it holds. + return "unreadable — recover its file or `ike space rm` it with --force" + } if sp.Archived == 0 { return fmt.Sprintf("%d active", sp.Active) } @@ -269,8 +274,9 @@ func newSpaceRmCmd(open opener) *cobra.Command { Long: "Delete a space, its tasks, its archive, and its history.\n\n" + "Unlike deleting a task, this cannot be undone — the space has no\n" + "history left to undo it from. A space holding anything needs --force,\n" + - "and the previous file contents remain in tasks.json.bak until the next\n" + - "change.", + "and the space's file is kept as a .bak beside the others until a new\n" + + "space claims the name. An unreadable space also needs --force, since\n" + + "its file may still hold everything the space ever had.", Args: cobra.ExactArgs(1), RunE: withStore(open, func(cmd *cobra.Command, args []string, s *store.Store) error { if err := rejectSpaceFlag(cmd); err != nil { @@ -288,7 +294,12 @@ func newSpaceRmCmd(open opener) *cobra.Command { name := task.SanitizeDisplay(removed.Name) // Say what was destroyed, not just that something was: the counts // are the only record left once the space is gone. - fmt.Fprintf(cmd.OutOrStdout(), "deleted space %s (%s)\n", name, spaceCounts(removed)) + if removed.Unreadable { + fmt.Fprintf(cmd.OutOrStdout(), + "deleted unreadable space %s; its file is kept as a .bak in case it can be recovered\n", name) + } else { + fmt.Fprintf(cmd.OutOrStdout(), "deleted space %s (%s)\n", name, spaceCounts(removed)) + } if removed.Current { d, err := s.Load() if err != nil { diff --git a/internal/store/spaces.go b/internal/store/spaces.go index 8ea5f5b..29aa100 100644 --- a/internal/store/spaces.go +++ b/internal/store/spaces.go @@ -33,6 +33,11 @@ type SpaceInfo struct { Active int `json:"active"` Archived int `json:"archived"` Current bool `json:"current"` + // Unreadable marks a space whose file exists but cannot be parsed. It is + // listed rather than hidden — a space silently missing from every picker + // is how data loss goes unnoticed — but its counts are necessarily zero + // and resolving it fails. + Unreadable bool `json:"unreadable,omitempty"` } // resolve returns the space an operation should act on, and its canonical name: @@ -57,6 +62,15 @@ func (f *File) resolve(name string) (string, *Data, error) { return have, d, nil } } + // A space whose file exists but cannot be parsed gets its own answer: + // "no space named X" would send someone hunting for a typo when the real + // problem is a file needing recovery — and the difference matters, because + // every *other* space still works. + if have, cs, ok := f.corruptNamed(name); ok { + return "", nil, fmt.Errorf( + "space %q is unreadable (%v); other spaces are unaffected — recover %s or remove the space with `ike space rm %q --force`", + have, cs.err, cs.file, have) + } return "", nil, fmt.Errorf("no space named %q", name) } @@ -78,7 +92,7 @@ func (f *File) dataFor(name string, d *Data) Data { // than in map order so a picker, `ike space list`, and the TUI's next/previous // keys all agree on what "the space after this one" means. func (f *File) spaceInfos() []SpaceInfo { - out := make([]SpaceInfo, 0, len(f.Spaces)) + out := make([]SpaceInfo, 0, len(f.Spaces)+len(f.corrupt)) for _, name := range slices.Sorted(maps.Keys(f.Spaces)) { d := f.Spaces[name] out = append(out, SpaceInfo{ @@ -88,6 +102,16 @@ func (f *File) spaceInfos() []SpaceInfo { Current: name == f.Current, }) } + // Unreadable spaces are listed too: a space quietly missing from every + // picker is how data loss goes unnoticed until far too late. + for _, name := range slices.Sorted(maps.Keys(f.corrupt)) { + out = append(out, SpaceInfo{ + Name: name, + Current: name == f.Current, + Unreadable: true, + }) + } + slices.SortFunc(out, func(a, b SpaceInfo) int { return strings.Compare(a.Name, b.Name) }) return out } @@ -230,6 +254,23 @@ func (s *Store) RemoveSpace(name string, force bool) (SpaceInfo, error) { if err := f.checkLifecycle("remove a space"); err != nil { return err } + // Removing an unreadable space is the recovery affordance: nothing + // else may touch its file, so without this the only cleanup would be + // deleting the file by hand. It requires force the way a non-empty + // space does — the file may hold every task the space ever had — and + // like every removal it renames to .bak rather than deleting. + if have, cs, ok := f.corruptNamed(name); ok && name != "" { + if !force { + return fmt.Errorf("%q is unreadable (%v); removing it discards whatever its file still holds — pass force to confirm", have, cs.err) + } + removed = SpaceInfo{Name: have, Current: have == f.Current, Unreadable: true} + f.removeCorrupt = append(f.removeCorrupt, cs.file) + delete(f.corrupt, have) + if f.Current == have && len(f.Spaces) > 0 { + f.Current = slices.Min(slices.Collect(maps.Keys(f.Spaces))) + } + return nil + } canonical, d, err := f.resolve(name) if err != nil { return err diff --git a/internal/store/store.go b/internal/store/store.go index d426bf6..d405435 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -113,7 +113,11 @@ type File struct { // corrupt lists spaces whose files could not be parsed, by display name. // They are absent from Spaces and from rawSpace, so no write can touch // their files; reads surface them instead of failing the whole document. - corrupt map[string]error + corrupt map[string]corruptSpace + // removeCorrupt names files in the spaces directory that RemoveSpace, + // with force, has condemned. The only way an unreadable space's file is + // ever touched, and even then it is renamed to .bak rather than deleted. + removeCorrupt []string // standalone marks a Store opened directly on a single space file — an // export handed to --file. The document then has exactly that space, // writes go back to the same file, and space lifecycle operations refuse. diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 1907659..36796fb 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "sync" "testing" "time" @@ -478,6 +479,143 @@ func TestMigrationClearsDebrisOfACrashedMigration(t *testing.T) { } } +// One corrupt space file must cost that one space, not the document — that is +// the point of the split. The others keep working, the broken one is listed +// rather than hidden, no write touches its file, and removing it with force +// is the recovery affordance. +func TestCorruptSpaceFileDegradesGracefully(t *testing.T) { + s := testStore(t) + if _, _, err := s.Add("keep", task.Do); err != nil { + t.Fatal(err) + } + if _, err := s.NewSpace("broken"); err != nil { + t.Fatal(err) + } + if _, _, err := s.InSpace("broken").Add("doomed", task.Do); err != nil { + t.Fatal(err) + } + brokenPath := filepath.Join(spacesDir(s.Path()), encodeSpaceFilename("broken")) + if err := os.WriteFile(brokenPath, []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + + // The healthy space still loads, and the listing shows both — a space + // silently missing from the picker is how loss goes unnoticed. + d, err := s.Load() + if err != nil { + t.Fatalf("the healthy space should load: %v", err) + } + if len(d.AllSpaces) != 2 { + t.Fatalf("AllSpaces = %+v, want both spaces listed", d.AllSpaces) + } + if !d.AllSpaces[0].Unreadable || d.AllSpaces[0].Name != "broken" { + t.Errorf("AllSpaces[0] = %+v, want broken marked unreadable", d.AllSpaces[0]) + } + + // Resolving the broken space names the real problem, not a typo hunt. + if _, err := s.InSpace("broken").Load(); err == nil || !strings.Contains(err.Error(), "unreadable") { + t.Errorf("loading the broken space = %v, want an unreadable error", err) + } + if _, _, err := s.InSpace("broken").Add("x", task.Do); err == nil { + t.Error("mutating an unreadable space should fail") + } + + // A write to the healthy space leaves the corrupt bytes exactly as they + // are: they may be recoverable by hand, and nothing may foreclose that. + if _, _, err := s.Add("more", task.Do); err != nil { + t.Fatal(err) + } + after, err := os.ReadFile(brokenPath) + if err != nil { + t.Fatalf("a write removed the unreadable file: %v", err) + } + if string(after) != "{not json" { + t.Errorf("a write altered the unreadable file: %q", after) + } + + // Removal needs force, and even then the file is renamed, not deleted. + if _, err := s.RemoveSpace("broken", false); err == nil { + t.Error("removing an unreadable space without force should fail") + } + removed, err := s.RemoveSpace("broken", true) + if err != nil { + t.Fatal(err) + } + if !removed.Unreadable || removed.Name != "broken" { + t.Errorf("removed = %+v, want the unreadable space", removed) + } + if _, err := os.Stat(brokenPath); !os.IsNotExist(err) { + t.Error("the unreadable file is still in the spaces directory") + } + if bak, err := os.ReadFile(brokenPath + ".bak"); err != nil || string(bak) != "{not json" { + t.Errorf("the unreadable file should survive as .bak: %q, %v", bak, err) + } + if d, err := s.Load(); err != nil || len(d.AllSpaces) != 1 { + t.Errorf("after removal: %+v, %v; want one healthy space", d.AllSpaces, err) + } +} + +// A corrupt file behind the *current* space must fail loudly, not quietly +// repair Current toward some other space — the user has to be told the space +// they work in needs attention. +func TestCorruptCurrentSpaceStaysCurrent(t *testing.T) { + s := testStore(t) + if _, _, err := s.Add("x", task.Do); err != nil { + t.Fatal(err) + } + if _, err := s.NewSpace("other"); err != nil { + t.Fatal(err) + } + defaultPath := filepath.Join(spacesDir(s.Path()), encodeSpaceFilename(defaultSpace)) + if err := os.WriteFile(defaultPath, []byte("{"), 0o600); err != nil { + t.Fatal(err) + } + + if _, err := s.Load(); err == nil || !strings.Contains(err.Error(), "unreadable") { + t.Errorf("bare Load = %v, want the unreadable error for the current space", err) + } + if _, err := s.InSpace("other").Load(); err != nil { + t.Errorf("the other space should still load: %v", err) + } + // A write elsewhere must not move Current off the broken space. + if _, _, err := s.InSpace("other").Add("y", task.Do); err != nil { + t.Fatal(err) + } + if cur, _ := rawFile(t, s.Path())["current"].(string); cur != defaultSpace { + t.Errorf("current = %q; a write repaired it away from the unreadable space", cur) + } +} + +// Even with every space unreadable the document still answers what it can: +// the listing, and both consent readers. +func TestAllSpacesCorruptStillListsAndAnswersConsent(t *testing.T) { + s := testStore(t) + if _, _, err := s.Add("x", task.Do); err != nil { + t.Fatal(err) + } + if _, err := s.SetMCPEnabled(true); err != nil { + t.Fatal(err) + } + defaultPath := filepath.Join(spacesDir(s.Path()), encodeSpaceFilename(defaultSpace)) + if err := os.WriteFile(defaultPath, []byte("{"), 0o600); err != nil { + t.Fatal(err) + } + + infos, err := s.ListSpaces() + if err != nil { + t.Fatalf("ListSpaces with every space corrupt: %v", err) + } + if len(infos) != 1 || !infos[0].Unreadable { + t.Errorf("infos = %+v, want the one unreadable space", infos) + } + if on, err := s.MCPEnabled(); err != nil || !on { + t.Errorf("MCPEnabled = %v, %v; consent must stay readable", on, err) + } + if _, err := s.Load(); err == nil { + t.Error("loading an unreadable space should still fail") + } +} + // A v4 file with no spaces is a truncated or hand-mangled file, not an empty // matrix. Accepting it would mean the next write erased whatever was there. func TestVersionFourWithNoSpacesIsAnError(t *testing.T) { diff --git a/internal/store/tree.go b/internal/store/tree.go index 598d5af..77dccd1 100644 --- a/internal/store/tree.go +++ b/internal/store/tree.go @@ -171,7 +171,13 @@ func readTree(path string) (File, error) { // unreadable there is nothing to repair toward, and Current is left alone // so the listing still says which space was current. if _, ok := f.Spaces[f.Current]; !ok && len(f.Spaces) > 0 { - f.Current = slices.Min(slices.Collect(maps.Keys(f.Spaces))) + // A current that names an *unreadable* space is not dangling, and is + // deliberately left alone: repairing it away would have the next bare + // `ike list` quietly show some other space instead of saying, loudly, + // that the one the user works in needs attention. + if _, _, isCorrupt := f.corruptNamed(f.Current); !isCorrupt { + f.Current = slices.Min(slices.Collect(maps.Keys(f.Spaces))) + } } return f, nil } @@ -210,20 +216,20 @@ func readSpaces(path string, f *File) error { } b, err := os.ReadFile(filepath.Join(dir, fname)) if err != nil { - f.markCorrupt(derived, err) + f.markCorrupt(derived, fname, err) continue } var sf spaceFile if err := json.Unmarshal(b, &sf); err != nil { - f.markCorrupt(derived, err) + f.markCorrupt(derived, fname, err) continue } if sf.Version < 1 || sf.Version > currentVersion { - f.markCorrupt(derived, fmt.Errorf("unsupported version %d (expected %d)", sf.Version, currentVersion)) + f.markCorrupt(derived, fname, fmt.Errorf("unsupported version %d (expected %d)", sf.Version, currentVersion)) continue } if sf.Name == "" { - f.markCorrupt(derived, errors.New("space file has no name")) + f.markCorrupt(derived, fname, errors.New("space file has no name")) continue } // The embedded name is canonical; the filename is derived from it and @@ -232,7 +238,7 @@ func readSpaces(path string, f *File) error { // filename wins, the other is surfaced rather than silently shadowed, // and — being corrupt — its file can never be written or deleted. if prior, dup := f.fileFor[sf.Name]; dup { - f.markCorrupt(derived, fmt.Errorf("%s and %s both claim the space %q; %s wins", prior, fname, sf.Name, prior)) + f.markCorrupt(derived, fname, fmt.Errorf("%s and %s both claim the space %q; %s wins", prior, fname, sf.Name, prior)) continue } d := sf.Data @@ -267,11 +273,34 @@ func readOrphanSpaces(path string) (File, bool, error) { return f, true, nil } -func (f *File) markCorrupt(name string, err error) { +// corruptSpace is one unreadable space: which file it is stuck in, and why. +type corruptSpace struct { + file string // filename within the spaces directory + err error +} + +func (f *File) markCorrupt(name, file string, err error) { if f.corrupt == nil { - f.corrupt = map[string]error{} + f.corrupt = map[string]corruptSpace{} } - f.corrupt[name] = err + f.corrupt[name] = corruptSpace{file: file, err: err} +} + +// corruptNamed looks name up among the unreadable spaces, resolving the way +// resolve does: empty means current, and a case-insensitive match counts. +func (f *File) corruptNamed(name string) (string, corruptSpace, bool) { + if name == "" { + name = f.Current + } + if cs, ok := f.corrupt[name]; ok { + return name, cs, true + } + for have, cs := range f.corrupt { + if strings.EqualFold(have, name) { + return have, cs, true + } + } + return "", corruptSpace{}, false } // readDocFlags reads only the document file's manifest-level fields. The @@ -467,6 +496,14 @@ func writeTree(path string, f *File) error { return err } } + // Files condemned by RemoveSpace on an unreadable space — the one write + // ever aimed at a corrupt file, and it too is a rename to .bak. + for _, fname := range f.removeCorrupt { + full := filepath.Join(dir, fname) + if err := os.Rename(full, full+".bak"); err != nil && !errors.Is(err, os.ErrNotExist) { + return err + } + } return nil } diff --git a/internal/tui/spaces.go b/internal/tui/spaces.go index a9edc61..58725a1 100644 --- a/internal/tui/spaces.go +++ b/internal/tui/spaces.go @@ -220,7 +220,11 @@ func (m *Model) deleteSpace(sp store.SpaceInfo, pending string) { if pending != sp.Name { m.pendingSpace = sp.Name held := "" - if sp.Active > 0 || sp.Archived > 0 { + if sp.Unreadable { + // The file cannot be parsed, so no counts exist to warn with; what + // the prompt can say is that whatever it held goes too. + held = " (unreadable; discards whatever its file still holds)" + } else if sp.Active > 0 || sp.Archived > 0 { held = fmt.Sprintf(" (%d active, %d archived)", sp.Active, sp.Archived) } // Kept short enough to survive truncation at 80 columns: the counts and diff --git a/internal/tui/view.go b/internal/tui/view.go index e39389a..4ed3162 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -159,8 +159,14 @@ func (m Model) renderSpaces() string { if sp.Current { current = "•" } - rows[i] = fmt.Sprintf(" %s %-*s %d active, %d archived", - current, width, task.SanitizeDisplay(sp.Name), sp.Active, sp.Archived) + counts := fmt.Sprintf("%d active, %d archived", sp.Active, sp.Archived) + if sp.Unreadable { + // Counts would be a lie: the space's file cannot be parsed, so + // nothing is known about what it holds. + counts = "unreadable" + } + rows[i] = fmt.Sprintf(" %s %-*s %s", + current, width, task.SanitizeDisplay(sp.Name), counts) } return m.renderList(listView{ title: fmt.Sprintf("Spaces — %d", len(spaces)), From 02cf03286924dce02b8be63de2f22f45cf8740ad Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 08:11:09 -0400 Subject: [PATCH 6/8] feat(store): space lifecycle follows the per-file layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renaming a space now moves its plan sidecar directory under the same lock — renaming historically left the plans stranded under the old name — and, on disk, retires the old name's file to .bak while the write side's case-only guard keeps a recased name from deleting the file it just wrote. Removing a space renames its file to .bak: removal and backup in one atomic step. PrunePlans additionally sweeps plan directories whose space no longer exists, while leaving an unreadable space's plans strictly alone — they may be the only part of it still readable. Co-Authored-By: Claude Fable 5 --- internal/store/plans.go | 46 ++++++++++++++++++++ internal/store/plans_test.go | 41 ++++++++++++++++++ internal/store/spaces.go | 12 ++++++ internal/store/spaces_test.go | 79 ++++++++++++++++++++++++++++++++++- 4 files changed, 177 insertions(+), 1 deletion(-) diff --git a/internal/store/plans.go b/internal/store/plans.go index 334ff70..5796aa5 100644 --- a/internal/store/plans.go +++ b/internal/store/plans.go @@ -301,6 +301,12 @@ func (s *Store) removePlan(space string, id int) error { // anyone who wants the space back. // // Archived tasks keep their plans, since Restore brings them back active. +// +// It also sweeps the plan directories of spaces that no longer exist — +// removing or renaming a space historically left its plans behind with +// nothing pointing at them. A directory belonging to an *unreadable* space is +// left strictly alone: its space still exists, just in a file that needs +// recovery, and the plans may be the only part of it still readable. func (s *Store) PrunePlans() (int, error) { f, err := s.loadFile() if err != nil { @@ -308,6 +314,29 @@ func (s *Store) PrunePlans() (int, error) { } removed := 0 + if entries, err := os.ReadDir(s.path + planDirSuffix); err == nil { + for _, e := range entries { + if !e.IsDir() { + continue + } + name := e.Name() + if _, _, err := f.resolve(name); err == nil { + continue // a live space's plans + } + if _, _, ok := f.corruptNamed(name); ok { + continue // an unreadable space still owns its plans + } + dir := filepath.Join(s.path+planDirSuffix, name) + n, err := countPlanFiles(dir) + if err != nil { + return removed, s.redact(err) + } + if err := os.RemoveAll(dir); err != nil { + return removed, s.redact(err) + } + removed += n + } + } for space, d := range f.Spaces { live := make(map[int]bool, len(d.Tasks)+len(d.Archive)) for _, t := range d.Tasks { @@ -338,6 +367,23 @@ func (s *Store) PrunePlans() (int, error) { return removed, nil } +// countPlanFiles counts the plan files in one space's plan directory, so a +// sweep of the whole directory can report how many plans it removed rather +// than how many directories. +func countPlanFiles(dir string) (int, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return 0, err + } + n := 0 + for _, e := range entries { + if _, ok := planFileID(e.Name()); ok { + n++ + } + } + return n, nil +} + // planFileID parses ".md" or ".draft.md" back into a task ID. Anything // else in the directory is left alone — a sweep that deleted files it did not // recognize would be a poor thing to point at a directory inside someone's data diff --git a/internal/store/plans_test.go b/internal/store/plans_test.go index 7cc74b2..5a78548 100644 --- a/internal/store/plans_test.go +++ b/internal/store/plans_test.go @@ -368,6 +368,47 @@ func TestDeleteKeepsThePlanAndPruneRemovesIt(t *testing.T) { } } +// PrunePlans also sweeps the plan directories of spaces that no longer exist +// — a removed space's plans had nothing pointing at them — but leaves an +// unreadable space's directory strictly alone: that space still exists, and +// its plans may be the only part of it still readable. +func TestPruneSweepsRemovedSpacesButSparesUnreadableOnes(t *testing.T) { + s := testStore(t) + for _, name := range []string{"removed", "broken"} { + if _, err := s.NewSpace(name); err != nil { + t.Fatal(err) + } + a, _, err := s.InSpace(name).Add("planned", task.Do) + if err != nil { + t.Fatal(err) + } + if _, _, err := s.InSpace(name).SetPlan(a.ID, samplePlan); err != nil { + t.Fatal(err) + } + } + if _, err := s.RemoveSpace("removed", true); err != nil { + t.Fatal(err) + } + brokenFile := filepath.Join(spacesDir(s.Path()), encodeSpaceFilename("broken")) + if err := os.WriteFile(brokenFile, []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + + n, err := s.PrunePlans() + if err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("PrunePlans() removed %d, want the removed space's one plan", n) + } + if _, err := os.Stat(s.planDir("removed")); !os.IsNotExist(err) { + t.Error("the removed space's plan directory was left behind") + } + if _, err := os.Stat(filepath.Join(s.planDir("broken"), "1.md")); err != nil { + t.Errorf("the unreadable space's plans must be left alone: %v", err) + } +} + // An archived task still owns its plan: Restore brings it back active. func TestPruneKeepsArchivedTasksPlans(t *testing.T) { s := testStore(t) diff --git a/internal/store/spaces.go b/internal/store/spaces.go index 29aa100..7e4ed6a 100644 --- a/internal/store/spaces.go +++ b/internal/store/spaces.go @@ -1,8 +1,10 @@ package store import ( + "errors" "fmt" "maps" + "os" "slices" "strings" @@ -234,6 +236,16 @@ func (s *Store) RenameSpace(from, to string) error { if f.Current == canonical { f.Current = to } + // The plan sidecars are filed by space name, so they follow the + // rename — under the same lock, or a plan written in between would be + // filed under a name about to stop existing. Renaming used to leave + // them behind, which stranded every plan the space had. A space with + // no plans has no directory (ErrNotExist is the common case); any + // other failure aborts the rename before anything is written, which + // leaves both the space and its plans consistently under the old name. + if err := os.Rename(s.planDir(canonical), s.planDir(to)); err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("renaming the space's plans: %w", err) + } return nil }) return err diff --git a/internal/store/spaces_test.go b/internal/store/spaces_test.go index 087b6ce..089ac25 100644 --- a/internal/store/spaces_test.go +++ b/internal/store/spaces_test.go @@ -288,15 +288,92 @@ func TestRenameSpaceRejectsCollisionAndKeepsTheOriginal(t *testing.T) { } // Renaming a space to a different casing of its own name is a legitimate -// rename, not a collision with itself. +// rename, not a collision with itself. On a case-insensitive filesystem the +// old and new filenames are one file, so this is also the test that the write +// side's delete-the-old-name step does not delete the file it just wrote. func TestRenameSpaceCanChangeOnlyCasing(t *testing.T) { s := spacesStore(t, "work") + if _, _, err := s.InSpace("work").Add("survives", task.Do); err != nil { + t.Fatal(err) + } if err := s.RenameSpace("work", "Work"); err != nil { t.Fatalf("recasing a space name: %v", err) } if got := spaceNames(t, s); !slicesEqual(got, []string{"Work", "default"}) { t.Errorf("spaces = %v, want the recased name", got) } + d, err := s.InSpace("Work").Load() + if err != nil { + t.Fatal(err) + } + if len(d.Tasks) != 1 { + t.Errorf("tasks = %v, want the task to survive the recasing", titlesOf(d.Tasks)) + } + // The file on disk carries the new name inside and out. + body := rawSpace(t, s.Path(), "Work") + if name, _ := body["name"].(string); name != "Work" { + t.Errorf("embedded name = %q, want the recased name", name) + } +} + +// A rename moves the space's file — the old name's file becomes a .bak, the +// new name's file exists — and carries the plan sidecar directory along, +// which renaming historically failed to do, stranding every plan the space +// had. +func TestRenameSpaceMovesTheFileAndThePlans(t *testing.T) { + s := spacesStore(t, "work") + a, _, err := s.InSpace("work").Add("planned", task.Do) + if err != nil { + t.Fatal(err) + } + if _, _, err := s.InSpace("work").SetPlan(a.ID, "# The plan\n\nDo it."); err != nil { + t.Fatal(err) + } + if err := s.RenameSpace("work", "job"); err != nil { + t.Fatal(err) + } + + dir := spacesDir(s.Path()) + if _, err := os.Stat(filepath.Join(dir, encodeSpaceFilename("job"))); err != nil { + t.Errorf("the renamed space has no file: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, encodeSpaceFilename("work"))); !os.IsNotExist(err) { + t.Error("the old name's file is still in the spaces directory") + } + if _, err := os.Stat(filepath.Join(dir, encodeSpaceFilename("work")+".bak")); err != nil { + t.Errorf("the old name's file should survive as .bak: %v", err) + } + // The plan is reachable under the new name and gone from the old. + body, err := s.InSpace("job").Plan(a.ID) + if err != nil || !strings.Contains(body, "Do it.") { + t.Errorf("plan after rename = %q, %v; want it to follow the space", body, err) + } + if _, err := os.Stat(s.planDir("work")); !os.IsNotExist(err) { + t.Error("the old name's plan directory was left behind") + } +} + +// Removing a space renames its file to .bak — removal and backup in one +// atomic step, so the .bak really is the recovery path the docs promise. +func TestRemoveSpaceKeepsTheFileAsBak(t *testing.T) { + s := spacesStore(t, "doomed") + if _, _, err := s.InSpace("doomed").Add("gone", task.Do); err != nil { + t.Fatal(err) + } + if _, err := s.RemoveSpace("doomed", true); err != nil { + t.Fatal(err) + } + dir := spacesDir(s.Path()) + if _, err := os.Stat(filepath.Join(dir, encodeSpaceFilename("doomed"))); !os.IsNotExist(err) { + t.Error("the removed space's file is still in the spaces directory") + } + bak, err := os.ReadFile(filepath.Join(dir, encodeSpaceFilename("doomed")+".bak")) + if err != nil { + t.Fatalf("no .bak after removal: %v", err) + } + if !strings.Contains(string(bak), "gone") { + t.Errorf(".bak does not hold the removed space's tasks: %s", bak) + } } func TestRemoveSpaceRefusesNonEmptyWithoutForce(t *testing.T) { From 2faeba1f30846a5c159bc454cbd183fcc0aa476f Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 08:12:15 -0400 Subject: [PATCH 7/8] =?UTF-8?q?test(store):=20pin=20export=E2=89=88copy,?= =?UTF-8?q?=20standalone=20--file=20editing,=20three=20import=20shapes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Export and a hand copy of the space's file are asserted byte-identical; a single exported space opened with --file edits in place (and undoes), while every lifecycle operation refuses; import accepts a v4 envelope, a v5 space file, and a whole v5 tree. Co-Authored-By: Claude Fable 5 --- internal/store/transfer_test.go | 85 +++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/internal/store/transfer_test.go b/internal/store/transfer_test.go index a0d4722..2c7c83d 100644 --- a/internal/store/transfer_test.go +++ b/internal/store/transfer_test.go @@ -72,6 +72,91 @@ func TestExportProducesAnOpenableFile(t *testing.T) { } } +// An export is byte-identical to the space's own file in the spaces +// directory: "a space is one file" made literal, so copying the file by hand +// and exporting it are the same operation. +func TestExportEqualsTheSpaceFile(t *testing.T) { + s := exportFixture(t) + out := filepath.Join(t.TempDir(), "work.json") + if _, err := s.ExportSpace("work", out, false); err != nil { + t.Fatal(err) + } + exported, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + onDisk, err := os.ReadFile(filepath.Join(spacesDir(s.Path()), encodeSpaceFilename("work"))) + if err != nil { + t.Fatal(err) + } + if string(exported) != string(onDisk) { + t.Error("the export and the space's own file differ; they should be the same bytes") + } + + // The converse holds too: a hand-copied space file imports like an export. + copied := filepath.Join(t.TempDir(), "copied.json") + if err := os.WriteFile(copied, onDisk, 0o600); err != nil { + t.Fatal(err) + } + dst := testStore(t) + imported, err := dst.ImportSpaces(copied, "", false) + if err != nil { + t.Fatal(err) + } + if len(imported) != 1 || imported[0].Name != "work" || imported[0].Active != 1 { + t.Errorf("imported = %+v, want the copied space", imported) + } +} + +// A single exported space opens with --file for reading *and* editing — the +// documented take-one-space-to-another-machine workflow — with edits going +// back into the same file. What it cannot do is grow more spaces. +func TestStandaloneSpaceFileEditsInPlace(t *testing.T) { + s := exportFixture(t) + out := filepath.Join(t.TempDir(), "work.json") + if _, err := s.ExportSpace("work", out, false); err != nil { + t.Fatal(err) + } + + alone := OpenAt(out) + if _, _, err := alone.Add("added standalone", task.Do); err != nil { + t.Fatalf("editing a standalone space file: %v", err) + } + // The write landed in the same single file — no manifest, no sidecar + // spaces directory sprouted beside it. + if _, err := os.Stat(spacesDir(out)); !os.IsNotExist(err) { + t.Error("editing a standalone file created a spaces directory") + } + d, err := OpenAt(out).Load() + if err != nil { + t.Fatal(err) + } + if len(d.Tasks) != 2 { + t.Errorf("tasks = %v, want the standalone edit persisted", titlesOf(d.Tasks)) + } + if name, _ := rawFile(t, out)["name"].(string); name != "work" { + t.Errorf("the standalone file lost its space-file shape (name = %q)", name) + } + // Undo works too — the history stacks live in the space. + if _, _, err := alone.Undo(); err != nil { + t.Errorf("undo on a standalone file: %v", err) + } + + // Space lifecycle operations have nowhere to put a second space. + if _, err := alone.NewSpace("other"); err == nil { + t.Error("NewSpace on a standalone file should refuse") + } + if err := alone.RenameSpace("work", "job"); err == nil { + t.Error("RenameSpace on a standalone file should refuse") + } + if _, err := alone.RemoveSpace("work", true); err == nil { + t.Error("RemoveSpace on a standalone file should refuse") + } + if _, err := alone.ImportSpaces(s.Path(), "", false); err == nil { + t.Error("ImportSpaces into a standalone file should refuse") + } +} + // Consent does not travel: an export exists to be copied to another machine, // whose owner has not agreed to anything. func TestExportNeverCarriesMCPAccess(t *testing.T) { From 7f4bbae1f197392b7207fb8b38e121bd4052de26 Mon Sep 17 00:00:00 2001 From: Jonathan Crockett Date: Fri, 7 Aug 2026 08:16:06 -0400 Subject: [PATCH 8/8] docs: describe the per-space-file layout (v5) docs/data.md gains the on-disk tree, the damaged-space section, and the migration note; README's storage blurb follows; CLAUDE.md's invariants are rewritten where the split changed them (write path, durability properties, degraded open, migration, export=copy, ModTime); CHANGELOG records the change under Unreleased. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 34 ++++++++++++++++ CLAUDE.md | 34 +++++++++------- README.md | 13 +++--- docs/data.md | 111 +++++++++++++++++++++++++++++++++++++-------------- 4 files changed, 142 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c01be2d..0faeae0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,40 @@ All notable changes to ike are recorded here. The format follows ## [Unreleased] +### Changed + +- **Each space is now its own file** (data format version 5). `tasks.json` + becomes a small manifest — which space is current, plus the two consent + settings — and the spaces live beside it in `tasks.json.spaces/`, one JSON + file each with its own rolling `.bak`. A mutation rewrites only the files it + touched, so corruption or a bad write in one space can never take the others + with it. One damaged space file no longer blocks the rest: the other spaces + keep working, the damaged one is listed as *unreadable* instead of silently + vanishing, nothing ever writes over its file, and + `ike space rm --force` retires it to `.bak` once you give up on + repairing it. +- **Migration is automatic and keeps a permanent escape hatch.** An existing + file (any version back to v1) is read as-is and split on the first change you + make; the pre-split file is kept as `tasks.json.pre-v5.bak` and never + overwritten. Older ike binaries refuse the new manifest outright rather than + misreading it as empty. +- **`ike space export` now writes exactly the space's own file** — export and a + hand copy of `tasks.json.spaces/.json` are byte-identical, and neither + can carry the consent flags because the format has no field for them. A + single exported space still opens with `--file` for reading *and* editing; + only operations that would need a second space refuse. `ike space import` + reads all three shapes: an old single-file export, a v5 space file, and a + whole v5 tree with `--all`. +- Removing a space renames its file to `.bak` — removal and backup in one + atomic step — instead of relying on the document-wide backup. + +### Fixed + +- **Renaming a space now moves its plans too.** `ike space rename` used to + leave `tasks.json.plans//` behind, stranding every plan the space + had. `ike plan --prune` additionally sweeps plan directories whose space no + longer exists — while leaving an unreadable space's plans strictly alone. + ## [0.2.0] - 2026-08-01 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 79a1bf5..e9704b0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,36 +28,42 @@ Listing order has exactly one definition each, as pure methods on `Data`: **`d.L Quadrant headings are user-customizable. `task.Quadrant.Label()` gives the **default** name; `store.Data.Labels` (a sparse `map[Quadrant]string`, only renamed quadrants present) holds overrides, and **`Labels.Of(q)` is the only correct way to render a quadrant name** — it is nil-safe, so `d.Labels.Of(q)` works on a store that has never been renamed. Never print `q.Label()` in a frontend. Clearing a label deletes the map entry rather than storing `""`, so later changes to the defaults still reach users who reset. The quadrant *number* is the stable identifier for classification and scripting; only the display name varies. -Concurrency contract (`internal/store/store.go`): every mutation goes through `Store.Mutate`, which flocks a **sidecar** `tasks.json.lock` (locking the data file itself would break — atomic rename replaces the inode), re-reads fresh inside the lock, applies the change, and writes via temp-file + `os.Rename`. Never write the data file any other way. Completing a task moves it from `tasks` to `archive` with `DoneAt` stamped (`Restore` reverses that); IDs are monotonic via `next_id` and never reused. Deleting skips the archive. +Concurrency contract (`internal/store/store.go`): every mutation goes through `Store.Mutate`, which flocks a **sidecar** `tasks.json.lock` (locking a data file itself would break — atomic rename replaces the inode), re-reads the whole tree fresh inside the lock via `readTree`, applies the change, and commits via `writeTree` — per-file temp-file + `os.Rename`. There is deliberately **one** lock for the whole tree, not one per space: per-space locks would need a second lock for the manifest, deadlock-ordering rules, and would break "AllSpaces is derived under one consistent read", to speed up a workload that is one human plus one agent. Never write any data file another way. Completing a task moves it from `tasks` to `archive` with `DoneAt` stamped (`Restore` reverses that); IDs are monotonic via `next_id` and never reused. Deleting skips the archive. Five properties of that write path are load-bearing; `internal/store/durability_test.go` pins each one: -- **Modes are `0600`/`0700`** (`dataFileMode`/`dataDirMode`). The matrix is personal, and it used to land world-readable. Because rename replaces the inode, a file left at `0644` by an older build is tightened by the next write — don't "optimize" the write into an in-place `os.WriteFile`, which would preserve the loose mode. -- **The temp file comes from `os.CreateTemp`**, never `path + ".tmp"`. A predictable name plus `IKE_DATA_FILE` pointing into a shared directory let a pre-created symlink redirect the write (verified: `os.WriteFile` follows symlinks and truncates the target). +- **Modes are `0600`/`0700`** (`dataFileMode`/`dataDirMode`). The matrix is personal, and it used to land world-readable. Rename replaces the inode, so a written file is tightened automatically; a file whose write is *skipped* by the dirty check is tightened by an explicit `Chmod` in `writeTree` — keep that, or a loose-moded file left by an older build stays loose forever. +- **The temp file comes from `os.CreateTemp`**, never `path + ".tmp"`. A predictable name plus `IKE_DATA_FILE` pointing into a shared directory let a pre-created symlink redirect the write (verified: `os.WriteFile` follows symlinks and truncates the target). Space-file temps are dot-prefixed (`.space-*.json`), which is also why the directory scan skips dotfiles — and why `encodeSpaceFilename` escapes a leading dot, so no real space is ever skipped with them. - **`f.Sync()` before the rename**, plus a best-effort directory fsync after. Rename is atomic for concurrent *readers*, but without the fsync a crash can land the rename metadata before the data blocks and leave a truncated file. -- **The previous contents are copied to `tasks.json.bak`** before being replaced, so a bad file costs one mutation rather than the whole matrix. +- **Every file's previous contents are copied to its own `.bak`** before being replaced — the manifest to `tasks.json.bak`, each space to `.json.bak` beside it — so a bad write costs one mutation in one space rather than the whole matrix. - **The lock wait is bounded** by `lockTimeout` via `TryLockContext`. Note a timeout surfaces as `context.DeadlineExceeded`, *not* a `false` return, so both have to be handled or the user gets "context deadline exceeded" instead of an actionable message. -Reads deliberately never overwrite a file they cannot parse: `Mutate` re-reads *inside* the lock and returns early, before `fn` and before any write. Keep that ordering. +Reads deliberately never overwrite a file they cannot parse: `Mutate` re-reads *inside* the lock and returns early, before `fn` and before any write. Keep that ordering. Since v5 the rule extends **per file**: a space file that fails to parse marks its space *corrupt* instead of failing the document (`File.corrupt`), the other spaces keep working, and — because a corrupt space is in neither `f.Spaces` nor `f.rawSpace` — no write can touch, replace, or delete its file. The one sanctioned exception is `RemoveSpace` with `force`, which renames it to `.bak`. Resolving a corrupt space returns an "unreadable" error naming the file, never "no space named X"; the listing (`spaceInfos`) includes it with `Unreadable: true`, since a space silently missing from every picker is how loss goes unnoticed. A `Current` naming a corrupt space is deliberately **not** repaired away — the user must be told loudly, not shown some other space. + +`writeTree` commits dirty-checked and in a fixed order: creations and updates first, the manifest second, deletions (rename to `.bak`) last — every crash window then leaves a state `readTree` already repairs (a stranded extra file, or a dangling `Current`). The dirty check compares each space's marshaling against the bytes it was read from (`f.rawSpace`), which both confines a write to the spaces the mutation touched *and* keeps "normalization persists on any write" true — an un-normalized space marshals differently, so it gets written. Deleting the old filename of a case-only rename is guarded by `os.SameFile`: on a case-insensitive filesystem the old and new names are one file, and the unguarded delete would remove the file just written (`TestRenameSpaceCanChangeOnlyCasing`). + +`ModTime()` is two stats — the manifest and the spaces directory — and is the TUI's only change signal. It stays complete because every space write is a rename *into* that directory (bumps its mtime) and every document-level change rewrites the manifest. An in-place hand edit of a space file is invisible to it; that was never the contract. Ordering: each task carries a `Rank` float; display order is quadrant, then rank, then ID. `Reorder(id, delta)` rewrites a whole quadrant's ranks as multiples of `rankGap`, so repeated moves never lose precision — do not switch it to midpoint insertion without adding a renormalization path. Rank 0 means "unranked" and sorts by ID; `normalizeRanks` (called from every read) backfills it, which is what makes pre-rank files work. Undo/redo is snapshot-based, with two stacks in the data file (each capped at `undoDepth`), so history works across frontends and restarts. A `Snapshot` covers tasks **and quadrant labels** — anything a mutation can touch has to be in there, or undoing a rename would silently do nothing. It deliberately does **not** copy the archive, because it does not need to: a task is never active and archived at once, so `restoreSnapshot` drops any archive entry whose ID is active again in the restored task list, which is exactly what reverses a complete. Only `Restore` takes an entry *out* of the archive, and only its `DoneAt` is then unrecoverable, so only that one entry is kept (`Snapshot.ArchiveEntry`, set by `pushUndoRestoringArchive`). Copying the whole archive into all 40 snapshots was a measured ~21x blow-up: 276K of real data became an 8MB file, 79ms per `ike add`, re-parsed by the TUI on every poll; it is now 1.4x and 17ms. `recordSnapshot` must keep propagating `ArchiveEntry` when moving a state between the two stacks, or a completed task stops being redoable — `TestInterleavedHistoryRoundTrips` walks a mixed history all the way back and forward to pin this. Ops call `pushUndo(d, label)` inside their `Mutate` callback, *after* validation and immediately before mutating, so failed and no-op mutations don't record. `pushUndo` also clears the redo stack — a new change diverges from the redone branch, and replaying it would clobber the change. **`Redo` must use `recordUndo`, not `pushUndo`**, or it clears the stack it is walking and only ever redoes one step (`TestRedoMultipleSteps` covers exactly this). `Undo` deliberately does not roll back `next_id`: IDs stay monotonic and are never reused. -Data file version is **4** (v2 added ranks + history stacks; v3 stopped copying the archive into every snapshot; v4 wrapped the matrix in a document that can hold several). `readFile` accepts older versions and upgrades in memory; the next write persists the current one. A newer version than `currentVersion` is still a hard error. The `redo` field was added *within* version 2 as `omitempty` rather than bumping — losing redo history to an older binary is harmless. v3 **was** a real bump, by the same reasoning applied to a worse failure mode: an older binary reading v3 finds no `"archive"` in a snapshot, decodes it as empty, and wipes the archive on the next undo. Refusing the file outright beats that. Relatedly, `readFile` **drops undo/redo history when upgrading from below v3** — those snapshots carry a whole archive and no `ArchiveEntry`, so undoing a restore recorded by an older build would silently lose that entry's completion stamp. Losing history once, on upgrade, is the better trade. v4 is a real bump for the same reason: an older binary finds no top-level `"tasks"` at all, sees an empty matrix, and overwrites it. +Data format version is **5** (v2 added ranks + history stacks; v3 stopped copying the archive into every snapshot; v4 wrapped the matrix in a document that can hold several; v5 split the document across files). `readTree` accepts older versions and upgrades in memory; the next write persists the current layout. A newer version than `currentVersion` is still a hard error. The `redo` field was added *within* version 2 as `omitempty` rather than bumping — losing redo history to an older binary is harmless. v3, v4, and v5 **were** real bumps, by the same reasoning applied to worse failure modes: an older binary reading v3 finds no `"archive"` in a snapshot and wipes the archive on the next undo; one reading v4 finds no top-level `"tasks"` and sees an empty matrix it would overwrite; and the v5 manifest deliberately has **no** `"spaces"` key so a v4 binary refuses it on the version check instead of decoding an empty matrix. Refusing outright beats all three. Relatedly, `readTree` **drops undo/redo history when upgrading from below v3** — those snapshots carry a whole archive and no `ArchiveEntry`, so undoing a restore recorded by an older build would silently lose that entry's completion stamp. Losing history once, on upgrade, is the better trade. + +**On disk, the document is a small tree; in memory it is still one `File`.** `tasks.json` is a manifest holding exactly the document-level fields (`version`, `current`, both consent flags); each space lives in `tasks.json.spaces/.json` as a self-describing `spaceFile` (`version`, `name`, then the matrix). `Data` stays the unit every op in `ops.go` acts on, and a space owns *everything* mutable — tasks, archive, labels, `NextID`, both history stacks — so `ike undo` can never reach across spaces. The filename is **derived** from the embedded name by `encodeSpaceFilename` (percent-encoding only `/`, `%`, and a leading dot); the embedded name is canonical, a mismatched filename is repaired on the next write, and when two files claim one name the lexicographically first filename wins while the other is surfaced as unreadable — never silently shadowed, never written to. Enumeration truth is the **directory listing**, not the manifest, so a stale or even missing manifest cannot hide a space (a missing one is reconstructed with consent off — consent is the one thing a repair must never invent). The version discriminates the shapes, never a missing key: v≤3 re-reads the body as one space named `defaultSpace`; v4 is the decoded envelope; v5 with a `name` is a standalone space file; v5 without one is a manifest. A v4 file with no `spaces`, and a v5 manifest with no space files at all, are hard errors — a truncated or half-copied tree must be refused, not accepted as an empty matrix the next write makes permanent (spaces that exist but cannot be parsed count as present). -**The file holds N spaces; a space is one matrix.** `File` (`Version`, `Current`, `Spaces map[string]*Data`, `MCPEnabled`) is the document; `Data` is one space and stays the unit every op in `ops.go` acts on. A space owns *everything* mutable — tasks, archive, labels, `NextID`, and both history stacks — so `ike undo` can never reach across spaces. The upgrade discriminates on **version, never `spaces == nil`**: a pre-v4 file carries `version`/`mcp_enabled` at the same top level the envelope does, so it has already decoded into those two fields and only its body needs re-reading as one space (named `defaultSpace`). A v4 file with **no** `spaces` is a hard error — going by the missing key instead would accept a truncated or hand-edited file as an empty matrix, and the next write would erase the lot. +**Migration to v5 happens on the first write, never on read** (`writeTree`, when `onDiskVersion` ≤ 4): the monolith is first copied once to `tasks.json.pre-v5.bak` (`O_EXCL`; the rolling `.bak` gets clobbered by the next manifest write, so this is the durable escape hatch), debris of a previously crashed migration is cleared from the spaces directory (the monolith is still authoritative — without this a space deleted since the crash would resurrect), every space file is written, and the manifest write is the **commit point** that retires the monolith. A crash anywhere before it leaves a valid pre-v5 file the next reader upgrades again. -`Data`'s last three fields — `Space`, `AllSpaces`, `MCPAllowed` — are **derived from the document on every read and never persisted** (`json:"-"`). They exist because a frontend must render an operation's outcome from the `Data` it returned, and the space header, the space picker, and the MCP marker all need facts that live on the document rather than in the matrix. `MCPAllowed` is deliberately *not* named `MCPEnabled`: while the flag lived on `Data`, `d.MCPEnabled = on` inside a `Mutate` callback was how it got set, and with the flag on the document that line would still compile, still report success, and persist nothing. The rename turns that into a compile error. For the same reason `SetMCPEnabled`/`MCPEnabled` go through `mutateFile`/`readFile` rather than `Mutate`/`Load` — consent is a property of the file, so it must keep working when the current space is missing or `--space` names something that is not there. +`Data`'s last three fields — `Space`, `AllSpaces`, `MCPAllowed` — are **derived from the document on every read and never persisted** (`json:"-"`). They exist because a frontend must render an operation's outcome from the `Data` it returned, and the space header, the space picker, and the MCP marker all need facts that live on the document rather than in the matrix. `MCPAllowed` is deliberately *not* named `MCPEnabled`: while the flag lived on `Data`, `d.MCPEnabled = on` inside a `Mutate` callback was how it got set, and with the flag on the document that line would still compile, still report success, and persist nothing. The rename turns that into a compile error. For the same reason `SetMCPEnabled` goes through `mutateFile` rather than `Mutate`, and `MCPEnabled`/`AgentEnabled` read via `readDocFlags` — the **manifest alone**, not the tree — so consent stays settable and reportable when the current space is missing, `--space` names something that is not there, or every space file is corrupt. `Store.space` pins a Store to one space (`InSpace`, mirroring `ForMCP`); empty means "follow `Current` at read time", so a plain `ike list` follows `ike space use` while a pinned Store keeps its matrix whatever another frontend switches to. **Resolution never creates**: a name that is not in the file fails inside the lock, before `fn` and before any write, so a typo cannot conjure an empty matrix and swallow the tasks meant for a real one — and an MCP client, which has no tool for making a space, cannot make one through a misspelled `space` argument either. A `Current` naming nothing *is* repaired in memory to the alphabetically first space, the way an out-of-range `NextID` is, because otherwise one bad hand edit breaks every command; an explicitly requested missing space still errors, since "the file is inconsistent" and "you asked for something that is not there" deserve different answers. `normalizeRanks` and the `NextID` repair run over **every** space, not just the resolved one — a write persists them all, so an un-normalized space would have its pre-rank ordering rewritten by a mutation that touched a different space. -Space *lifecycle* ops (`NewSpace`/`UseSpace`/`RenameSpace`/`RemoveSpace`/`ImportSpaces`) are **not undoable**: history is per space, so a document-level change has no stack to record onto, and one that could resurrect a removed space would have to hold the whole matrix. `RemoveSpace` is therefore the only destructive op with no way back — it refuses a non-empty space without `force`, refuses the last space outright, and reports what it destroyed so the caller can say so. `NewSpace`/`UseSpace` return `Data` so the TUI's `m.apply` stays the single mutation funnel; `RenameSpace`/`RemoveSpace` do not, following `SetMCPEnabled`'s exception, because nothing renders from them. +Space *lifecycle* ops (`NewSpace`/`UseSpace`/`RenameSpace`/`RemoveSpace`/`ImportSpaces`) are **not undoable**: history is per space, so a document-level change has no stack to record onto, and one that could resurrect a removed space would have to hold the whole matrix. `RemoveSpace` is therefore the only destructive op with no way back — it refuses a non-empty space without `force`, refuses the last space outright, and reports what it destroyed so the caller can say so; on disk it renames the space's file to `.bak`, removal and backup in one atomic step, and with `force` it is also the recovery affordance for an *unreadable* space (the one write ever aimed at a corrupt file, and even that is the same rename). `RenameSpace` moves the plan sidecar directory under the same lock — renaming used to strand every plan under the old name — and aborts before anything is written if that move fails, so the space and its plans stay consistently together. `NewSpace`/`UseSpace` return `Data` so the TUI's `m.apply` stays the single mutation funnel; `RenameSpace`/`RemoveSpace` do not, following `SetMCPEnabled`'s exception, because nothing renders from them. All of them call `File.checkLifecycle` first, which refuses on a **standalone** document — a single exported space file opened with `--file` serves reads, task mutations, and undo against that one file, but has nowhere to persist a change to the set of spaces. -`ExportSpace` writes a normal one-space file and **always leaves `mcp_enabled` off**, whatever it is locally: consent is a decision about a file on a machine, and an export exists to be copied to a machine whose owner never agreed. `ImportSpaces` refuses a name already in use rather than merging — reconciling two ID sequences and two histories would interleave someone's work silently — and `--as` renames on the way in. Task IDs need no renumbering, since `next_id` lives in the space and travels with it. +`ExportSpace` writes **exactly the space-file shape** — an export and a hand copy of `tasks.json.spaces/.json` are byte-identical (`TestExportEqualsTheSpaceFile`), and neither can carry a consent flag because `spaceFile` has no field for one; consent is a decision about a file on a machine, and an export exists to be copied to a machine whose owner never agreed (`readTree` also zeroes both flags on any standalone open, so a crafted file cannot smuggle them in). `ImportSpaces` accepts all three shapes readTree knows — a v≤4 envelope (migrations for free), a single v5 space file, a whole v5 tree with `--all` — refuses a name already in use rather than merging — reconciling two ID sequences and two histories would interleave someone's work silently — and `--as` renames on the way in. Task IDs need no renumbering, since `next_id` lives in the space and travels with it. `--file` and `IKE_DATA_FILE` both go through `CheckPath`, so a path cannot be legal one way and rejected the other. `internal/store/recent.go` is the TUI's file-picker list: paths only, in `$XDG_STATE_HOME/ike/recent.json`, best-effort in both directions — every failure is an empty list, because a convenience must never stop the TUI starting. **Only the TUI writes it**; having every `ike list` touch a second file would be a lot of writes for a list nothing else reads. `internal/tui/main_test.go` redirects `XDG_STATE_HOME` for the whole package, or the suite fills the developer's real picker with temp paths. -**`mutateFile` is the one write path.** It holds the whole lock/re-read/gate/write body; `Mutate` is a thin wrapper that resolves the space inside it and hands `fn` that space's `*Data`. Space-level operations use `mutateFile` directly. Do not give either a second write path, or all five durability properties above get a duplicate, untested implementation. +**`mutateFile` is the one write path.** It holds the whole lock/re-read/gate body and commits through `writeTree`, which is called from nowhere else; `Mutate` is a thin wrapper that resolves the space inside it and hands `fn` that space's `*Data`. Space-level operations use `mutateFile` directly. Do not give any of them a second write path, or all five durability properties above get a duplicate, untested implementation. The TUI (`internal/tui`) is a single `Model` with a mode enum (normal/input/move/archive/spaces/files). `model.go` holds state, `Update`, and the matrix's own keys; `spaces.go` the space and file pickers; `view.go` all rendering; `list.go` the one definition of a full-screen scrolling list. It re-renders from the `Data` each op returns and polls file mtime every 2s to pick up CLI/MCP writes. `archCursor` indexes `data.ListArchive()` (newest first), *not* `data.Archive` — use the helper when acting on the selected archive row. @@ -71,13 +77,13 @@ Store tests are split by concern: `ops_test.go` (tasks, ordering, ranks), `histo **No test runs the real `claude`.** A run costs money, needs credentials, and would tie the suite to how a model happens to word a plan. `internal/agent`, `internal/cli`, and `internal/tui` each point `IKE_AGENT_CMD` at their own test binary re-executed with a marker in the environment, replaying a canned stream — no shell script to keep executable, no second language, no network. Two of the TUI tests drive the real Bubble Tea command loop against that fake (exec, `waitForEvent` re-issuing per event, `savePlanCmd`), because folding messages into the model correctly is not the same as the commands actually producing them. -**Plan bodies live beside the data file, not in it** (`internal/store/plans.go`, `.plans//.md`). `Snapshot` copies `Tasks` wholesale into as many as 40 snapshots, so a few KB of markdown per task would be amplified across the whole history — the same blow-up `Snapshot.ArchiveEntry` exists to have fixed once. Only `Task.PlanAt` is persisted in the matrix, and it is what frontends render the `✎` mark from: the matrix redraws on every keypress, so a stat per row per frame would be a poor trade for a symbol. Beside the *data file* rather than under `XDG_STATE_HOME` because a plan is user data — it must follow `--file` and `IKE_DATA_FILE`, so two matrices cannot share one set of plans. Plans are **not** carried by `ike space export`; that is a known gap. `dir` and `plan_at` were added within v4 as `omitempty` rather than bumping the version, by the `redo` reasoning above: an older binary drops them, costing a remembered directory and a mark a re-plan restores, which is the harmless category rather than the archive-wipe one. +**Plan bodies live beside the data file, not in it** (`internal/store/plans.go`, `.plans//.md`). `Snapshot` copies `Tasks` wholesale into as many as 40 snapshots, so a few KB of markdown per task would be amplified across the whole history — the same blow-up `Snapshot.ArchiveEntry` exists to have fixed once. Only `Task.PlanAt` is persisted in the matrix, and it is what frontends render the `✎` mark from: the matrix redraws on every keypress, so a stat per row per frame would be a poor trade for a symbol. Beside the *data file* rather than under `XDG_STATE_HOME` because a plan is user data — it must follow `--file` and `IKE_DATA_FILE`, so two matrices cannot share one set of plans. Plans are **not** carried by `ike space export`; that is a known gap, and the documented workaround is copying `tasks.json.plans//` alongside the exported file. `RenameSpace` moves the directory with the space; `PrunePlans` sweeps directories whose space no longer exists but leaves an *unreadable* space's directory strictly alone — its plans may be the only part of it still readable. `dir` and `plan_at` were added within v4 as `omitempty` rather than bumping the version, by the `redo` reasoning above: an older binary drops them, costing a remembered directory and a mark a re-plan restores, which is the harmless category rather than the archive-wipe one. -The plan write happens inside the same `Mutate` callback that stamps `PlanAt`, through **`mutateSpace`** — `Mutate` with the resolved space name handed to the callback. Plans are filed per space and `Data.Space` is derived by `dataFor` only *after* `fn` returns, so resolving separately would land outside the lock, where an `ike space use` in between would file a plan under the wrong space. The bytes go through **`writeBytesAtomic`**, lifted out of `writeFileAtomic` so the sidecars get the same four durability properties rather than a second untested copy; `mutateFile` still owns the lock, the re-read, and the gate, and `durability_test.go` passing unchanged is what proves the lift was faithful. **`Delete` deliberately does not remove the plan file** — `Delete` is undoable, so removing it would make undo silently lossy, and since `NextID` is monotonic an orphan can never be picked up by a later task. `PrunePlans` is the explicit sweep. +The plan write happens inside the same `Mutate` callback that stamps `PlanAt`, through **`mutateSpace`** — `Mutate` with the resolved space name handed to the callback. Plans are filed per space and `Data.Space` is derived by `dataFor` only *after* `fn` returns, so resolving separately would land outside the lock, where an `ike space use` in between would file a plan under the wrong space. The bytes go through **`writeBytesAtomic`**, the same atomic-write primitive `writeTree` commits every data file with, so the sidecars get the same four durability properties rather than a second untested copy; `mutateFile` still owns the lock, the re-read, and the gate. **`Delete` deliberately does not remove the plan file** — `Delete` is undoable, so removing it would make undo silently lossy, and since `NextID` is monotonic an orphan can never be picked up by a later task. `PrunePlans` is the explicit sweep. **`task.SanitizeBlock` is the multi-line sibling of `SanitizeDisplay`, and the two are not interchangeable.** `SanitizeDisplay` replaces every rune below `0x20`, newline included, so using it on a plan or a line of agent output renders the whole thing as one line of U+FFFD; `SanitizeBlock` keeps `\n` and `\t` and replaces the rest. They are separate functions rather than one with a flag so neither call site can pick the wrong one silently — a single-line field sanitized with `SanitizeBlock` would let a newline forge an extra listing row. Agent output is the most untrusted text ike renders, and `internal/agent` puts every field through `SanitizeBlock` at the single point they leave the package, so no frontend has to remember to and the CLI and TUI cannot disagree about whether it happened. -**Delegation is gated separately from MCP** (`File.AgentEnabled`, `ike agent enable|disable|status`). Letting an agent edit the task list and letting ike start a process that edits files are different decisions with different blast radii, so consenting to one is not consenting to the other. Everything else mirrors the MCP gate: off by default, out of `Snapshot` so undo cannot reopen it, reached through `mutateFile`/`readFile` so it works when the current space is missing, and never written into an export. It differs in one way — the MCP gate is re-checked on every read and mutation because a session outlives its check, while a delegated run is started by a command that just read the flag, so it is checked **once, at launch, in `internal/cli`** (and freshly in `tui.startAgent`, not from the polled `Data`, so a revocation in another terminal does not wait for the 2s tick). `Data.AgentAllowed` is derived alongside `MCPAllowed` for the ambient footer line only: display, never decision. +**Delegation is gated separately from MCP** (`File.AgentEnabled`, `ike agent enable|disable|status`). Letting an agent edit the task list and letting ike start a process that edits files are different decisions with different blast radii, so consenting to one is not consenting to the other. Everything else mirrors the MCP gate: off by default, out of `Snapshot` so undo cannot reopen it, reached through `mutateFile`/`readDocFlags` so it works when the current space is missing, and never written into an export. It differs in one way — the MCP gate is re-checked on every read and mutation because a session outlives its check, while a delegated run is started by a command that just read the flag, so it is checked **once, at launch, in `internal/cli`** (and freshly in `tui.startAgent`, not from the polled `Data`, so a revocation in another terminal does not wait for the 2s tick). `Data.AgentAllowed` is derived alongside `MCPAllowed` for the ambient footer line only: display, never decision. **`internal/agent` is the only package that starts a process**, and stays a pure runner the way `mcpserver` is a pure transport — it knows nothing about the store. Four things there are load-bearing and were each verified against a real run rather than assumed: `--verbose` is mandatory alongside `--output-format stream-json` or the stream carries nothing; `cmd.Stdin` is left nil (so `/dev/null`) because the CLI otherwise waits 3s for input and a child sharing ike's stdin would eat the TUI's keystrokes; an **unrecognized event type is skipped, never an error** (an ordinary run already carries `rate_limit_event` and `system/thinking_tokens`, and the CLI adds more between releases), as is a non-JSON line; and a `result` event wins over a non-zero exit, with stderr surfaced only when the process dies without one. Thinking blocks usually arrive with a signature and empty text, so the emptiness check is what stops blank transcript rows. The child gets its own process group — split into `procgroup_unix.go` and a non-Unix fallback so the tree keeps compiling everywhere, which `.goreleaser.yaml` claims and CI's cross-build step does not check — because killing only the parent would leave the agent's own tools running against the user's files. Tests replay `testdata/toolrun.jsonl`, captured from a real run, against a fake CLI that is the test binary re-executed. diff --git a/README.md b/README.md index b64df00..805baf3 100644 --- a/README.md +++ b/README.md @@ -185,11 +185,14 @@ ike space use work # every later command follows ike add "Fix prod bug" -s work # or act on one space just once ``` -Everything lives in `~/.local/share/ike/tasks.json` (mode `0600`, in a `0700` -directory). Writes go through a lock file and an atomic rename with the previous -contents kept as `.bak`, so three frontends can run at once and an interrupted -write costs one change rather than the matrix. The file is self-contained and -portable — copy it to another machine and every space comes with it. +Everything lives under `~/.local/share/ike/` (mode `0600` files in `0700` +directories): a small `tasks.json` manifest, and one file per space beside it +in `tasks.json.spaces/`. Writes go through a lock file and atomic renames with +each file's previous contents kept as its `.bak`, so three frontends can run at +once and an interrupted write costs one change rather than the matrix. One +damaged space file costs that one space, never the others. A space's file is +self-contained and portable — it *is* the export format, so copying it to +another machine is the whole move. → **[docs/data.md](docs/data.md)** covers spaces in full, exporting and importing a single matrix, the file's durability guarantees and their one diff --git a/docs/data.md b/docs/data.md index 4306dcb..2c620a7 100644 --- a/docs/data.md +++ b/docs/data.md @@ -1,12 +1,14 @@ # Spaces, files, and your data -Everything ike knows lives in one JSON file you own. This page covers how that -file is organised, where it lives, how it survives being written to by three -frontends at once, and how to move it around. +Everything ike knows lives in a handful of JSON files you own: a small +manifest, and one file per space beside it. This page covers how that data is +organised, where it lives, how it survives being written to by three frontends +at once, and how to move it around. - [Spaces](#spaces) - [Moving a matrix between machines](#moving-a-matrix-between-machines) - [Where your data lives](#where-your-data-lives) +- [When a space file is damaged](#when-a-space-file-is-damaged) - [Undo and redo](#undo-and-redo) - [Renaming the quadrants](#renaming-the-quadrants) @@ -14,12 +16,18 @@ frontends at once, and how to move it around. ## Spaces -One data file holds several independent matrices, called **spaces** — work and +Your data holds several independent matrices, called **spaces** — work and personal, say. Each has its own tasks, archive, quadrant headings, ID numbering, and undo history, so `ike undo` in one can never reach into another. A fresh install has a single space named `default`, and nothing changes until you make a second. +**Each space is one file on disk.** `tasks.json` is a small manifest recording +which space is current and the two agent-consent settings; the spaces +themselves live beside it in `tasks.json.spaces/`, one JSON file each, named +after the space. Corruption in one space's file cannot touch the others, and +copying a space to another machine is copying one file. + ```sh ike space # list spaces, marking the current one ike space new work # create it (does not switch) @@ -42,46 +50,74 @@ the matrix. In the TUI, `s` opens a space picker and `]`/`[` move between spaces **Deleting a space cannot be undone.** History lives inside the space, so there is no stack left to revert from — which is why `ike space rm` names the counts it -is about to destroy and needs `--force` if the space still holds anything. The -previous file contents remain in `tasks.json.bak` until the next change. +is about to destroy and needs `--force` if the space still holds anything. +Removing a space renames its file to `.json.bak` inside the spaces +directory rather than deleting it, so the contents survive until a new space +claims the name. ## Moving a matrix between machines -The data file is self-contained and fully portable: nothing in it refers to a -path or a machine, and timestamps are stored in UTC. Copy it to another computer -and every space comes with it. The sidecar `.lock` and `.bak` files do not need -copying. - -To move one space rather than the whole file: +A space's file is self-contained and fully portable: nothing in it refers to a +path or a machine, and timestamps are stored in UTC. `ike space export` writes +exactly the same bytes as the space's own file in `tasks.json.spaces/`, so +exporting and copying the file by hand are the same operation. ```sh -ike space export work ~/work-matrix.json # a standalone ike data file +ike space export work ~/work-matrix.json # a standalone space file # copy that one file to the other machine, then: ike space import ~/work-matrix.json ike space import ~/old.json --as archive-2025 ike --file ~/work-matrix.json list # or just open it in place ``` -An export is an ordinary data file, so `--file` opens it directly. **MCP access -is always off in an exported file**, whatever it was in the original: agent -access is a decision about a file on a machine, and an export is made to travel. -Importing a name that is already in use is an error rather than a merge — use -`--as` to bring it in under a different name. +A single space file opens with `--file` for reading and editing — task changes, +undo, everything except growing more spaces, which a one-space file has no room +for. **Neither consent setting travels in an exported file**: the space-file +format simply has no field for them, because agent access is a decision about a +file on a machine, and an export is made to travel. Importing a name that is +already in use is an error rather than a merge — use `--as` to bring it in +under a different name. + +To move *everything*, copy `tasks.json` together with the whole +`tasks.json.spaces/` directory (and `tasks.json.plans/` if you use plans), then +`ike space import --all` — or just point `IKE_DATA_FILE` at +the copy. The sidecar `.lock` and `.bak` files never need copying. -Note that attached plans do **not** yet travel with `ike space export`. +`ike space import` still reads data files from every earlier version of ike, +including the old single-file format. + +Note that attached plans do **not** yet travel with `ike space export`. To move +them by hand, copy `tasks.json.plans//` alongside the exported file. ## Where your data lives -Tasks live in `$XDG_DATA_HOME/ike/tasks.json` (default -`~/.local/share/ike/tasks.json`), created mode `0600` in a `0700` directory — -your matrix is not readable by other users on the machine. +Tasks live under `$XDG_DATA_HOME/ike/` (default `~/.local/share/ike/`): + +``` +tasks.json # manifest: current space + consent settings +tasks.json.spaces/ # one file per space + work.json + work.json.bak # that space's previous contents +tasks.json.plans/ # plan bodies, one file per task +tasks.json.bak # the manifest's previous contents +tasks.json.lock # write lock; never needs touching +``` + +Everything is created mode `0600` in `0700` directories — your matrix is not +readable by other users on the machine. + +Writes are serialized through the sidecar lock file and land via atomic +renames, so the TUI, CLI, and MCP server can run at the same time without +losing updates. A mutation rewrites only the files it changed. Each write is +flushed to disk before the rename, and every file's previous contents are kept +as its `.bak`, so an interrupted write costs at most one change rather than the +whole matrix. If a file is ever unreadable, ike refuses to overwrite it rather +than starting fresh over the top. -Writes are serialized through a sidecar lock file and land via an atomic -rename, so the TUI, CLI, and MCP server can run at the same time without losing -updates. Each write is flushed to disk before the rename, and the previous -contents are kept as `tasks.json.bak`, so an interrupted write costs at most -one change rather than the whole matrix. If the file is ever unreadable, ike -refuses to overwrite it rather than starting fresh over the top. +Data from older ike versions (a single `tasks.json` holding everything) is read +as-is and split into the new layout on the first change you make. The original +file is kept as `tasks.json.pre-v5.bak`, permanently — it is your escape hatch +back to the pre-split state, and nothing ever overwrites it. Override the location with `--file` or `IKE_DATA_FILE` — highest precedence first: `--file`, then `IKE_DATA_FILE`, then the default above. Either must be an @@ -94,14 +130,27 @@ unreliable on NFS and some FUSE mounts — so pointing the data file at a Dropbo iCloud, or network-mounted folder and writing from two machines at once is not covered. A single machine writing to a synced folder is fine. +## When a space file is damaged + +One damaged space costs that one space, never the others. If a space's file +cannot be parsed — a bad sync, a stray hand edit — every other space keeps +working, and the damaged one shows up in `ike space list` and the TUI picker +marked **unreadable** rather than silently disappearing. Commands aimed at it +say what is wrong and which file to look at. + +Nothing ike does will touch an unreadable file, so you can try to repair it in +place (it is JSON; the `.bak` beside it may also be intact). Once you give up +on it, `ike space rm --force` retires it — even then the file is renamed +to `.bak`, not deleted, in case it can still be recovered later. + ## Undo and redo Every change records a snapshot, so `ike undo` (or `u` in the TUI) reverts the last one — including a delete, and including changes made from a different frontend. Run it repeatedly to walk further back; the last 20 changes are kept -in the data file, so history survives restarts. Undo does not recycle task IDs. -History belongs to the space it was made in, so `ike undo` never reverts a change -made in a different one. +in the space's file, so history survives restarts. Undo does not recycle task +IDs. History belongs to the space it was made in, so `ike undo` never reverts a +change made in a different one. `ike redo` (or `U` / `ctrl+r`) re-applies what you just undid, and is itself undoable. **Any new change discards the redo history** — that includes a change