fix(safari): round-2 review — WAL replay, stable ordering, error context

- Drop immutable=1 on temp-copy SQLite opens in readLocalStorageFile /
  countLocalStorageFile. Session.Acquire copies the -wal / -shm sidecars,
  so mode=ro alone lets SQLite replay WAL on the ephemeral copy and
  surface entries Safari committed to WAL but hasn't checkpointed yet.
  Live-file reads in profiles.go keep immutable=1 as before.
- Order ItemTable query by (key, rowid) for deterministic exports across
  runs and SQLite versions.
- Wrap os.ReadFile / os.ReadDir errors with the offending path so
  multi-origin debug logs stay scannable.
- RFC-011 §7 rewritten to explain the live-vs-temp split.
- New regression test asserts ORDER BY surfaces rows in key order.

Addresses round-2 Copilot review on #582.

Author: moonD4rk

browser/safari/extract_storage.go

@@ -105,7 +105,11 @@ func countLocalStorage(root string) (int, error) {
 }
 
 func countLocalStorageFile(path string) (int, error) {
-	dsn := "file:" + path + "?mode=ro&immutable=1"
+	// mode=ro (no immutable) so SQLite replays the copied -wal sidecar — this surfaces entries
+	// Safari has committed to WAL but not yet checkpointed to the main DB. Writes SQLite might
+	// make to the temp-copy's -shm during replay are harmless; the Session cleanup removes
+	// everything. Live-file reads (profiles.go) still use immutable=1 to stay off the real WAL.
+	dsn := "file:" + path + "?mode=ro"
 	db, err := sql.Open("sqlite", dsn)
 	if err != nil {
 		return 0, fmt.Errorf("open %s: %w", path, err)
@@ -127,7 +131,7 @@ func countLocalStorageFile(path string) (int, error) {
 func findOriginDataDirs(root string) ([]string, error) {
 	topEntries, err := os.ReadDir(root)
 	if err != nil {
-		return nil, fmt.Errorf("read origins root: %w", err)
+		return nil, fmt.Errorf("read origins root %s: %w", root, err)
 	}
 	var out []string
 	for _, top := range topEntries {
@@ -172,7 +176,7 @@ type originEndpoint struct {
 func readOriginFile(path string) (string, error) {
 	data, err := os.ReadFile(path)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("read origin file %s: %w", path, err)
 	}
 	top, pos, terr := readOriginBlock(data, 0)
 	if terr != nil {
@@ -269,8 +273,10 @@ type localStorageItem struct {
 }
 
 func readLocalStorageFile(path string) ([]localStorageItem, error) {
-	// Read-only + immutable so we don't disturb a live WAL (same pattern as profiles.go).
-	dsn := "file:" + path + "?mode=ro&immutable=1"
+	// mode=ro (no immutable) — see countLocalStorageFile for the WAL-replay rationale; the same
+	// live-vs-temp split applies here. ORDER BY key, rowid makes exports byte-for-byte stable
+	// across runs and SQLite versions.
+	dsn := "file:" + path + "?mode=ro"
 	db, err := sql.Open("sqlite", dsn)
 	if err != nil {
 		return nil, fmt.Errorf("open %s: %w", path, err)
@@ -280,7 +286,7 @@ func readLocalStorageFile(path string) ([]localStorageItem, error) {
 		return nil, fmt.Errorf("ping %s: %w", path, err)
 	}
 
-	rows, err := db.Query(`SELECT key, value FROM ItemTable`)
+	rows, err := db.Query(`SELECT key, value FROM ItemTable ORDER BY key, rowid`)
 	if err != nil {
 		return nil, fmt.Errorf("query ItemTable: %w", err)
 	}

browser/safari/extract_storage_test.go

@@ -303,6 +303,24 @@ func TestCountLocalStorageFile_SkipsNullKey(t *testing.T) {
 	assert.Equal(t, 2, count, "NULL keys are excluded from count to match extract's skip rule")
 }
 
+func TestReadLocalStorageFile_ReturnsRowsInKeyOrder(t *testing.T) {
+	// Rows are inserted in reverse alphabetical order; ORDER BY key, rowid in the extractor
+	// query must surface them ascending so exports are deterministic across runs.
+	dbPath := filepath.Join(t.TempDir(), "ls.sqlite3")
+	writeLocalStorageDB(t, dbPath, []testLocalStorageItem{
+		{Key: "zebra", Value: "z"},
+		{Key: "mango", Value: "m"},
+		{Key: "apple", Value: "a"},
+	}, false /*addNullKey*/)
+
+	items, err := readLocalStorageFile(dbPath)
+	require.NoError(t, err)
+	require.Len(t, items, 3)
+	assert.Equal(t, "apple", items[0].key)
+	assert.Equal(t, "mango", items[1].key)
+	assert.Equal(t, "zebra", items[2].key)
+}
+
 func TestCountLocalStorageFile_MissingTable(t *testing.T) {
 	// Real Safari has origin dirs with LocalStorage/localstorage.sqlite3 but no ItemTable yet
 	// (seen during live verification). countLocalStorageFile must surface the error so the

rfcs/011-safari-data-storage.md

@@ -239,7 +239,9 @@ The only encrypted category is passwords. Because they are not stored in Safari'
 
 - **macOS-only**. There is no Safari on Windows or Linux.
 - **Full Disk Access (TCC)** is required to read the sandboxed container. Without it, cookies / history / downloads / localStorage reads fail silently with permission errors at stat or open time. Legacy paths under `~/Library/Safari/` sometimes remain readable without FDA, but are mostly empty on modern systems.
-- **Live-file safety**: `SafariTabs.db`, `History.db`, and `localstorage.sqlite3` can be written to by a running Safari instance. All live SQL reads use `?mode=ro&immutable=1`, which disables WAL replay and locking — the extractor sees a consistent snapshot of the main DB as of read time. Uncommitted WAL content is intentionally not replayed to avoid race-induced corruption.
+- **Live-file safety** follows a live-vs-temp split:
+  - **Live reads** (`SafariTabs.db` during profile discovery in `profiles.go`) use `?mode=ro&immutable=1`, which disables WAL replay and locking so the extractor cannot disturb a running Safari — it sees a consistent snapshot of the main DB as of read time, at the cost of missing any pending WAL content.
+  - **Temp-copy reads** (`History.db`, `localstorage.sqlite3`, etc. via `filemanager.Session.Acquire`) use `?mode=ro` only. `Session.Acquire` copies the `-wal` / `-shm` sidecars alongside the main DB, so SQLite can replay uncommitted transactions on the copy — surfacing entries Safari has written to WAL but not yet checkpointed. Any `-shm` writes SQLite performs during replay land on the ephemeral copy and are deleted with the session.
 - **Multi-profile availability**: requires Safari 17 (macOS 14 Sonoma) or newer. Older Safari versions have only the default profile; discovery degrades cleanly via the ReadDir fallback described in §2.1.
 - **File acquisition**: all per-profile files are copied into a `filemanager.Session` temp directory before extraction, except the discovery-time `SafariTabs.db` read which opens the live file directly. See [RFC-008](008-file-acquisition-and-platform-quirks.md) for the general pattern.