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 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/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/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/ops.go b/internal/store/ops.go index f215a57..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 := readFile(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 := readFile(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.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 73af8c4..5a78548 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") } } @@ -361,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 68e1b14..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" @@ -33,6 +35,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 +64,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 +94,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 +104,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 } @@ -113,8 +139,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 +176,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 +215,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 @@ -192,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 @@ -209,6 +263,26 @@ 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 + } + // 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/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) { diff --git a/internal/store/store.go b/internal/store/store.go index 48acdb0..d405435 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -11,13 +11,10 @@ package store import ( "context" - "encoding/json" "errors" "fmt" - "maps" "os" "path/filepath" - "slices" "strings" "time" @@ -49,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. @@ -88,6 +88,40 @@ 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]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. + standalone bool } // Data is one space: a complete matrix, and the unit every operation acts on. @@ -308,16 +342,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. @@ -336,7 +381,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 +482,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) } @@ -449,98 +494,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 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 { - 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..36796fb 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,10 +2,9 @@ package store import ( "encoding/json" - "maps" "os" "path/filepath" - "slices" + "strings" "sync" "testing" "time" @@ -216,6 +215,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 { @@ -241,42 +276,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) + } + } + for _, k := range []string{"spaces", "tasks", "next_id"} { + if _, ok := m[k]; ok { + t.Errorf("manifest should not hold key %q", k) } } - // The matrix itself sits one level down, under its space. + // 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 +372,250 @@ 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"]) + } +} + +// 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/transfer.go b/internal/store/transfer.go index 45f73f3..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{ @@ -84,7 +88,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 } @@ -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..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) { @@ -217,8 +302,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 +312,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 new file mode 100644 index 0000000..77dccd1 --- /dev/null +++ b/internal/store/tree.go @@ -0,0 +1,567 @@ +package store + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "slices" + "strings" +) + +// 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 +} + +// 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 { + // 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 +} + +// 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, fname, err) + continue + } + var sf spaceFile + if err := json.Unmarshal(b, &sf); err != nil { + f.markCorrupt(derived, fname, err) + continue + } + if sf.Version < 1 || sf.Version > currentVersion { + f.markCorrupt(derived, fname, fmt.Errorf("unsupported version %d (expected %d)", sf.Version, currentVersion)) + continue + } + if sf.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 + // 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, fname, 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 +} + +// 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]corruptSpace{} + } + 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 +// 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 + } + } + // 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 +} + +// 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) +} 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)),