Fix P0 correctness issues in jail enforcement, merge, and mutation log

README.md

@@ -96,7 +96,7 @@ let workspace = Workspace(
 )
 ```
 
-`Storage.directory(at:)` writes checkpoint and snapshot JSON plus a `mutations.jsonl` append log (one JSON record per line) under `<url>/<workspaceId>/`. A legacy `mutations.json` array is migrated to JSONL on first access. The store assigns monotonic `sequence` numbers while holding `mutations.lock` (advisory `flock` where the OS supports it), so multiple `Workspace` instances that share a `workspaceId` and store do not collide on mutation sequence. The current checkpoint head is derived from the parent-id graph (unparented tips), not only from `createdAt` ordering, which reduces surprises when wall clocks differ between processes. Listing or loading mutations still reads the full log; very long histories may need application-level rotation. Coordinating multiple hosts or network disks that do not honor `flock` may still require extra synchronization.
+`Storage.directory(at:)` writes checkpoint and snapshot JSON plus a `mutations.jsonl` append log (one JSON record per line) under `<url>/<workspaceId>/`. A legacy `mutations.json` array is migrated to JSONL on first access. The store assigns monotonic `sequence` numbers while holding `mutations.lock` (advisory `flock` where the OS supports it), so multiple `Workspace` instances that share a `workspaceId` and store do not collide on mutation sequence. Appends read only the log's final record to derive the next sequence, and a partial trailing line left by a crashed append is skipped on read and truncated before the next append. The current checkpoint head is derived from the parent-id graph (unparented tips), not only from `createdAt` ordering, which reduces surprises when wall clocks differ between processes. Listing mutations still reads the full log; very long histories may need application-level rotation. Coordinating multiple hosts or network disks that do not honor `flock` may still require extra synchronization.
 
 ### Reads
 
@@ -227,7 +227,7 @@ print(merged.mergedFromWorkspaceId == branch.workspaceId)   // true
 print(merged.mergedFromCheckpointId == branchHead.id)       // true
 ```
 
-`merge(_:)` is optimistic. If the parent workspace head changed after `branch()` was created, merge throws `WorkspaceError.mergeConflict(parentWorkspaceId:expectedBase:actualHead:)`.
+`merge(_:)` is optimistic. If the parent workspace head changed after `branch()` was created, merge throws `WorkspaceError.mergeConflict(parentWorkspaceId:expectedBase:actualHead:)`. If the parent made tracked writes after `branch()` without creating a checkpoint, merge throws `WorkspaceError.mergeUncheckpointedChanges(parentWorkspaceId:baseMutationCursor:currentMutationCursor:)` instead of silently overwriting those edits; create a checkpoint or roll the parent back first.
 
 ## Common Filesystem Patterns
 

Sources/Workspace/CheckpointStore.swift

@@ -80,7 +80,9 @@ actor InMemoryCheckpointStore: CheckpointStore {
 /// A JSON file-backed checkpoint store.
 ///
 /// Mutations are stored as one JSON line per record in `mutations.jsonl` (append-friendly under
-/// the lock) so each append rewrites a tiny tail instead of the entire log. A legacy
+/// the lock). Appends derive the next sequence from the log's final record instead of re-reading
+/// the whole log, so appends stay cheap as histories grow. A partial trailing line left by a
+/// crashed append is skipped when reading and truncated before the next append. A legacy
 /// `mutations.json` array is migrated to JSONL on the next read or append. Writes are
 /// synchronized through a persistent sidecar lockfile (`mutations.lock`) with an advisory
 /// exclusive lock when the platform supports it (`flock`), so concurrent ``FileCheckpointStore``
@@ -172,10 +174,19 @@ actor FileCheckpointStore: CheckpointStore {
         let legacy = legacyMutationsArrayURL(workspaceId: mutation.workspaceId)
         let lockURL = mutationsLockURL(workspaceId: mutation.workspaceId)
         return try Self.withMutationsExclusiveLock(at: lockURL) {
-            let existing = try loadAllMutations(jsonl: jsonl, legacy: legacy)
-            let next = (existing.map(\.sequence).max() ?? 0) + 1
+            let lastSequence: Int
+            if fileManager.fileExists(atPath: jsonl.path) {
+                if fileManager.fileExists(atPath: legacy.path) {
+                    try? fileManager.removeItem(at: legacy)
+                }
+                try repairTornTail(at: jsonl)
+                lastSequence = try lastPersistedSequence(at: jsonl)
+            } else {
+                // First write, or a legacy `mutations.json` array awaiting migration.
+                lastSequence = try loadAllMutations(jsonl: jsonl, legacy: legacy).map(\.sequence).max() ?? 0
+            }
             var record = mutation
-            record.sequence = next
+            record.sequence = lastSequence + 1
 
             if fileManager.fileExists(atPath: jsonl.path) {
                 try appendJSONLLine(encode: record, to: jsonl)
@@ -222,16 +233,88 @@ actor FileCheckpointStore: CheckpointStore {
     private func loadMutationsFromJSONL(at url: URL) throws -> [MutationRecord] {
         let data = try Data(contentsOf: url)
         if data.isEmpty { return [] }
+        let endsWithNewline = data.last == UInt8(ascii: "\n")
         let text = String(data: data, encoding: .utf8) ?? ""
+        let lines = text.split(whereSeparator: \.isNewline)
         var out: [MutationRecord] = []
-        out.reserveCapacity(64)
-        for line in text.split(whereSeparator: \.isNewline) {
+        out.reserveCapacity(lines.count)
+        for (index, line) in lines.enumerated() {
             if line.isEmpty { continue }
-            out.append(try decoder.decode(MutationRecord.self, from: Data(String(line).utf8)))
+            do {
+                out.append(try decoder.decode(MutationRecord.self, from: Data(String(line).utf8)))
+            } catch {
+                // A crashed append leaves a strict prefix of `<json>\n`: an undecodable final
+                // line with no trailing newline. Tolerate exactly that torn tail; anything else
+                // is real corruption and must surface.
+                if index == lines.count - 1, !endsWithNewline {
+                    continue
+                }
+                throw error
+            }
         }
         return out
     }
 
+    /// Truncates a partial trailing line left behind by a crashed append so subsequent appends
+    /// do not glue new records onto the torn fragment. A torn append never contains a newline
+    /// (it is a strict prefix of `<json>\n`), so everything after the final newline — or the
+    /// whole file when none exists — is the torn region. Must be called under the mutations lock.
+    private func repairTornTail(at url: URL) throws {
+        let handle = try FileHandle(forUpdating: url)
+        defer { try? handle.close() }
+        let size = try handle.seekToEnd()
+        guard size > 0 else { return }
+        try handle.seek(toOffset: size - 1)
+        let lastByte = try handle.read(upToCount: 1)
+        guard lastByte != Data("\n".utf8) else { return }
+        let newlineOffset = try Self.lastNewlineOffset(in: handle, before: size - 1)
+        try handle.truncate(atOffset: newlineOffset.map { $0 + 1 } ?? 0)
+    }
+
+    /// Returns the sequence of the final record without loading the whole log. Appends are
+    /// strictly monotonic and full rewrites are sorted by sequence, so the last line always
+    /// carries the maximum. Falls back to a full scan for unusual tails such as blank trailing
+    /// lines. Must be called under the mutations lock, after ``repairTornTail(at:)``.
+    private func lastPersistedSequence(at url: URL) throws -> Int {
+        if let line = try lastLine(at: url),
+           let record = try? decoder.decode(MutationRecord.self, from: line) {
+            return record.sequence
+        }
+        return try loadMutationsFromJSONL(at: url).map(\.sequence).max() ?? 0
+    }
+
+    private func lastLine(at url: URL) throws -> Data? {
+        let handle = try FileHandle(forReadingFrom: url)
+        defer { try? handle.close() }
+        let size = try handle.seekToEnd()
+        guard size > 1 else { return nil }
+        // The file ends with `\n` after tail repair; the last line spans the byte after the
+        // previous newline through the byte before the trailing newline.
+        let lineEnd = size - 1
+        let newlineOffset = try Self.lastNewlineOffset(in: handle, before: lineEnd)
+        let lineStart = newlineOffset.map { $0 + 1 } ?? 0
+        guard lineEnd > lineStart else { return nil }
+        try handle.seek(toOffset: lineStart)
+        let line = try handle.read(upToCount: Int(lineEnd - lineStart)) ?? Data()
+        return line.isEmpty ? nil : line
+    }
+
+    /// Scans backwards in chunks for the offset of the last `\n` strictly before `end`.
+    private static func lastNewlineOffset(in handle: FileHandle, before end: UInt64) throws -> UInt64? {
+        let chunkSize: UInt64 = 65536
+        var cursor = end
+        while cursor > 0 {
+            let chunkStart = cursor > chunkSize ? cursor - chunkSize : 0
+            try handle.seek(toOffset: chunkStart)
+            let chunk = try handle.read(upToCount: Int(cursor - chunkStart)) ?? Data()
+            if let index = chunk.lastIndex(of: UInt8(ascii: "\n")) {
+                return chunkStart + UInt64(chunk.distance(from: chunk.startIndex, to: index))
+            }
+            cursor = chunkStart
+        }
+        return nil
+    }
+
     private func writeAllMutationsAsJSONL(_ records: [MutationRecord], to url: URL) throws {
         if records.isEmpty {
             if fileManager.fileExists(atPath: url.path) {

Sources/Workspace/FS/FileSystem.swift

@@ -22,6 +22,9 @@ public enum WorkspaceError: Error, CustomStringConvertible, Sendable {
     case snapshotNotFound(UUID)
     /// The parent workspace changed after a branch was created, so the branch cannot be merged.
     case mergeConflict(parentWorkspaceId: UUID, expectedBase: UUID?, actualHead: UUID?)
+    /// The parent workspace has tracked mutations that no checkpoint captures, so merging would
+    /// silently discard them.
+    case mergeUncheckpointedChanges(parentWorkspaceId: UUID, baseMutationCursor: Int, currentMutationCursor: Int)
     /// A tracked workspace mutation could not be recorded.
     case mutationFailed(String)
 
@@ -53,6 +56,8 @@ public enum WorkspaceError: Error, CustomStringConvertible, Sendable {
             let expected = expectedBase?.uuidString ?? "nil"
             let actual = actualHead?.uuidString ?? "nil"
             return "cannot merge branch into workspace \(parentWorkspaceId.uuidString): expected base \(expected), current head is \(actual)"
+        case let .mergeUncheckpointedChanges(parentWorkspaceId, baseMutationCursor, currentMutationCursor):
+            return "cannot merge branch into workspace \(parentWorkspaceId.uuidString): the workspace has uncheckpointed changes (mutation sequence \(currentMutationCursor), branch base \(baseMutationCursor)); create a checkpoint or roll back before merging"
         case let .mutationFailed(message):
             return message
         }

Sources/Workspace/FS/ReadWriteFilesystem.swift

@@ -301,9 +301,25 @@ public final class ReadWriteFilesystem: FileSystem, @unchecked Sendable {
         let url = try existingOrPotentialURL(for: virtualPath, configuration: configuration)
         let parent = url.deletingLastPathComponent()
         try ensureInsideRoot(parent, configuration: configuration)
+        try ensureDestinationSymlinkStaysInsideRoot(url, configuration: configuration)
         return url
     }
 
+    /// The parent containment check never resolves the destination's final component, so a
+    /// pre-existing symlink at the destination could redirect follow-on opens (for example an
+    /// append) outside the configured root. Reject destinations whose final component is a
+    /// symlink resolving outside the root.
+    private func ensureDestinationSymlinkStaysInsideRoot(
+        _ url: URL,
+        configuration: ConfigurationSnapshot
+    ) throws {
+        let attributes = try? fileManager.attributesOfItem(atPath: url.path)
+        guard attributes?[.type] as? FileAttributeType == .typeSymbolicLink else {
+            return
+        }
+        try ensureInsideRoot(url, configuration: configuration)
+    }
+
     private func ensureInsideRoot(_ url: URL, configuration: ConfigurationSnapshot) throws {
         let resolved = url.resolvingSymlinksInPath().standardizedFileURL.path
         let root = configuration.resolvedRootPath

Sources/Workspace/Workspace+Branching.swift

@@ -25,7 +25,8 @@ extension Workspace {
             workspaceId: UUID(),
             filesystem: branchFilesystem,
             store: store,
-            baseCheckpointId: baseCheckpointId
+            baseCheckpointId: baseCheckpointId,
+            baseMutationCursor: latestMutationSequence()
         )
         _ = try await branch.seedBranchCheckpoint(
             snapshot: snapshot,
@@ -36,6 +37,12 @@ extension Workspace {
     }
 
     /// Merges another workspace into this workspace when this workspace still points at the other's base.
+    ///
+    /// The merge is refused when this workspace's checkpoint head moved past the branch's base
+    /// (``WorkspaceError/mergeConflict(parentWorkspaceId:expectedBase:actualHead:)``) or when this
+    /// workspace has tracked mutations that no checkpoint captures
+    /// (``WorkspaceError/mergeUncheckpointedChanges(parentWorkspaceId:baseMutationCursor:currentMutationCursor:)``),
+    /// since restoring the branch snapshot would silently discard those edits.
     public func merge(_ other: Workspace, label: String? = nil) async throws -> Checkpoint {
         try await ensureLoaded()
         try await reconcileCheckpointsWithStore()
@@ -50,6 +57,20 @@ extension Workspace {
             )
         }
 
+        // The head comparison only sees checkpoints. Tracked writes made after the branch was
+        // created leave the head untouched, so restoring the branch snapshot would silently
+        // discard them; refuse the merge instead.
+        if let expectedCursor = await other.mergeBaseMutationCursor() {
+            let currentCursor = latestMutationSequence()
+            guard currentCursor <= expectedCursor else {
+                throw WorkspaceError.mergeUncheckpointedChanges(
+                    parentWorkspaceId: workspaceId,
+                    baseMutationCursor: expectedCursor,
+                    currentMutationCursor: currentCursor
+                )
+            }
+        }
+
         let previousSnapshot = try await captureSnapshot()
         let incomingSnapshot = try await other.captureSnapshot()
         let incomingHead = try await other.currentHeadCheckpointId()
@@ -99,6 +120,10 @@ extension Workspace {
         baseCheckpointId
     }
 
+    func mergeBaseMutationCursor() -> Int? {
+        baseMutationCursor
+    }
+
     func currentHeadCheckpointId() async throws -> UUID? {
         try await ensureLoaded()
         return headCheckpointId

Sources/Workspace/Workspace.swift

@@ -31,6 +31,9 @@ public actor Workspace {
     public nonisolated let filesystem: any FileSystem
     let store: any CheckpointStore
     let baseCheckpointId: UUID?
+    /// The parent's mutation sequence at the moment this workspace was branched. Merge uses it to
+    /// detect tracked-but-uncheckpointed parent edits that a snapshot restore would discard.
+    let baseMutationCursor: Int?
 
     var loadTask: Task<Void, Error>?
     var didLoadStoreState = false
@@ -77,12 +80,14 @@ public actor Workspace {
         workspaceId: UUID = UUID(),
         filesystem: any FileSystem,
         store: any CheckpointStore,
-        baseCheckpointId: UUID? = nil
+        baseCheckpointId: UUID? = nil,
+        baseMutationCursor: Int? = nil
     ) {
         self.workspaceId = workspaceId
         self.filesystem = filesystem
         self.store = store
         self.baseCheckpointId = baseCheckpointId
+        self.baseMutationCursor = baseMutationCursor
     }
 
     static func makeStore(for storage: Storage) -> any CheckpointStore {

Tests/WorkspaceTests/CheckpointStoreTests.swift

@@ -399,6 +399,96 @@ struct CheckpointStoreTests {
         #expect(Set(mutations.map(\.sequence)) == expectedSequences)
     }
 
+    @Test
+    func `FileCheckpointStore skips and repairs a torn trailing mutation line`() async throws {
+        let root = try makeTempDirectory()
+        defer { removeTempDirectory(root) }
+
+        let workspaceId = UUID()
+        let store = FileCheckpointStore(rootDirectory: root)
+
+        for index in 0..<2 {
+            let mutation = MutationRecord(
+                sequence: 0,
+                workspaceId: workspaceId,
+                kind: .writeFile,
+                touchedPaths: [WorkspacePath(normalizing: "/file-\(index).txt")],
+                fileChanges: []
+            )
+            try await store.appendMutation(mutation)
+        }
+
+        // Simulate a crash mid-append: a partial record with no trailing newline.
+        let jsonl = mutationsJsonlURL(root: root, workspaceId: workspaceId)
+        let handle = try FileHandle(forWritingTo: jsonl)
+        try handle.seekToEnd()
+        try handle.write(contentsOf: Data("{\"sequence\":3,\"worksp".utf8))
+        try handle.close()
+
+        // Reads skip the torn tail instead of poisoning the whole log.
+        let reader = FileCheckpointStore(rootDirectory: root)
+        let beforeRepair = try await reader.listMutationRecords(workspaceId: workspaceId)
+        #expect(beforeRepair.map(\.sequence) == [1, 2])
+
+        // The next append truncates the torn tail and continues the sequence.
+        let appended = try await reader.appendMutation(
+            MutationRecord(
+                sequence: 0,
+                workspaceId: workspaceId,
+                kind: .writeFile,
+                touchedPaths: [WorkspacePath(normalizing: "/file-2.txt")],
+                fileChanges: []
+            )
+        )
+        #expect(appended.sequence == 3)
+
+        let afterRepair = try await reader.listMutationRecords(workspaceId: workspaceId)
+        #expect(afterRepair.map(\.sequence) == [1, 2, 3])
+        #expect(afterRepair.allSatisfy { $0.workspaceId == workspaceId })
+    }
+
+    @Test
+    func `FileCheckpointStore still surfaces corruption before the final line`() async throws {
+        let root = try makeTempDirectory()
+        defer { removeTempDirectory(root) }
+
+        let workspaceId = UUID()
+        let store = FileCheckpointStore(rootDirectory: root)
+
+        for index in 0..<2 {
+            let mutation = MutationRecord(
+                sequence: 0,
+                workspaceId: workspaceId,
+                kind: .writeFile,
+                touchedPaths: [WorkspacePath(normalizing: "/file-\(index).txt")],
+                fileChanges: []
+            )
+            try await store.appendMutation(mutation)
+        }
+
+        // Corrupt the first record while keeping the file newline-terminated: this is real
+        // mid-file damage, not a torn append, and must not be silently skipped.
+        let jsonl = mutationsJsonlURL(root: root, workspaceId: workspaceId)
+        let contents = try String(contentsOf: jsonl, encoding: .utf8)
+        var lines = contents.split(separator: "\n", omittingEmptySubsequences: false).map(String.init)
+        lines[0] = "not json"
+        try Data(lines.joined(separator: "\n").utf8).write(to: jsonl)
+
+        let reader = FileCheckpointStore(rootDirectory: root)
+        do {
+            _ = try await reader.listMutationRecords(workspaceId: workspaceId)
+            Issue.record("expected mid-file corruption to surface as an error")
+        } catch {
+            // Expected: only a torn tail without a trailing newline is tolerated.
+        }
+    }
+
+    private func mutationsJsonlURL(root: URL, workspaceId: UUID) -> URL {
+        root
+            .appendingPathComponent(workspaceId.uuidString, isDirectory: true)
+            .appendingPathComponent("mutations.jsonl", isDirectory: false)
+    }
+
     private func makeTempDirectory() throws -> URL {
         let base = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true)
         let url = base.appendingPathComponent("CheckpointStoreTests-\(UUID().uuidString)", isDirectory: true)