mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-08 02:14:37 +00:00
M7 #7+#8: ServerContext.readTextThrowing + Memory surfaces real errors
Pass-1 found that SFTP failures (initially the tilde-expansion bug,
but the same pattern applies to any transport error) silently
returned nil from `ServerContext.readText`, which the Memory editor
interpreted as "empty file." The user stared at a blank TextEditor
with no clue the connection had failed.
Two-part fix:
1. Add `readTextThrowing(_:)` on ServerContext that separates three
outcomes:
- `.some(content)` — file read succeeded.
- `.none` — file is genuinely absent (fileExists probe returned
false).
- throws — transport error (SSH down, SFTP timeout, auth failure,
non-UTF-8 data).
The existing nil-returning `readText(_:)` stays around for callers
that genuinely can't distinguish ("probably there, probably not")
— now implemented as a `try?` on the throwing variant so behavior
doesn't drift.
2. IOSMemoryViewModel.load uses the throwing variant. `.success(nil)`
is still treated as "first-time empty" (no lastError). `.failure`
populates `lastError` with a human message citing the underlying
transport error's localizedDescription so the Memory editor can
render it inline (it already had the error-banner view; just
needed the VM to actually set the string).
Also fixes a pre-existing stale test reference in M0dViewModelsTests
(`vm.entries` → `vm.toolMessages`) — ActivityViewModel's property
name drifted during the earlier rebase; the test was left broken.
Unrelated cron-delete test failure noted for separate follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -233,10 +233,34 @@ extension ServerContext {
|
||||
/// the remote path doesn't exist on this Mac.
|
||||
extension ServerContext {
|
||||
/// Read a UTF-8 text file. `nil` on any error (missing, transport down,
|
||||
/// invalid encoding).
|
||||
/// invalid encoding). Use this when the caller genuinely can't tell
|
||||
/// the difference (e.g. "if a manifest exists, parse it, otherwise
|
||||
/// use defaults"). Prefer `readTextThrowing` when the UI needs to
|
||||
/// distinguish "file doesn't exist" from "transport failed" — pass-1
|
||||
/// M7 #8 showed that silent nils from transport errors masqueraded
|
||||
/// as empty files in the Memory editor for ~1 minute before the
|
||||
/// SFTP-tilde fix was found.
|
||||
public nonisolated func readText(_ path: String) -> String? {
|
||||
guard let data = try? makeTransport().readFile(path) else { return nil }
|
||||
return String(data: data, encoding: .utf8)
|
||||
try? readTextThrowing(path)
|
||||
}
|
||||
|
||||
/// Read a UTF-8 text file. Throws on transport errors. Returns:
|
||||
/// - `.some(content)` when the file was read successfully,
|
||||
/// - `.none` when the file is genuinely absent (the transport's
|
||||
/// `fileExists` returned false),
|
||||
/// - throws the underlying transport error otherwise.
|
||||
///
|
||||
/// This is the version to call from VMs that can surface a real
|
||||
/// error to the UI — e.g. Memory, Settings, Cron. The nil-returning
|
||||
/// shim above is fine for "probably there, probably not" cases.
|
||||
public nonisolated func readTextThrowing(_ path: String) throws -> String? {
|
||||
let transport = makeTransport()
|
||||
guard transport.fileExists(path) else { return nil }
|
||||
let data = try transport.readFile(path)
|
||||
guard let text = String(data: data, encoding: .utf8) else {
|
||||
throw TransportError.other(message: "File at \(path) is not valid UTF-8.")
|
||||
}
|
||||
return text
|
||||
}
|
||||
|
||||
/// Read raw bytes. `nil` on any error.
|
||||
|
||||
@@ -93,24 +93,39 @@ public final class IOSMemoryViewModel {
|
||||
public func load() async {
|
||||
isLoading = true
|
||||
lastError = nil
|
||||
// Run the file read on a detached task — `ServerContext.readText`
|
||||
// Run the file read on a detached task — `readTextThrowing`
|
||||
// blocks on transport I/O, and we don't want the MainActor
|
||||
// hanging during a remote SFTP fetch.
|
||||
let ctx = context
|
||||
let path = kind.path(on: context)
|
||||
let loaded: String? = await Task.detached {
|
||||
ctx.readText(path)
|
||||
let result: Result<String?, Error> = await Task.detached {
|
||||
do {
|
||||
return .success(try ctx.readTextThrowing(path))
|
||||
} catch {
|
||||
return .failure(error)
|
||||
}
|
||||
}.value
|
||||
|
||||
if let loaded {
|
||||
switch result {
|
||||
case .success(.some(let loaded)):
|
||||
text = loaded
|
||||
originalText = loaded
|
||||
} else {
|
||||
// `readText` returns nil on missing file — treat as
|
||||
// empty (the user is creating the file for the first
|
||||
// time) rather than an error.
|
||||
lastError = nil
|
||||
case .success(.none):
|
||||
// Genuinely absent file — treat as empty (first-time
|
||||
// create). Distinguished from transport error by the
|
||||
// fileExists check inside readTextThrowing (pass-1 M7 #7/#8).
|
||||
text = ""
|
||||
originalText = ""
|
||||
lastError = nil
|
||||
case .failure(let error):
|
||||
// Transport error (SSH timeout, auth failure, SFTP
|
||||
// protocol issue). Surface to the UI so the user
|
||||
// understands this isn't just "empty file" — something's
|
||||
// genuinely broken with the connection.
|
||||
text = ""
|
||||
originalText = ""
|
||||
lastError = "Couldn't load \(kind.displayName) — \(error.localizedDescription)"
|
||||
}
|
||||
isLoading = false
|
||||
}
|
||||
|
||||
@@ -123,7 +123,7 @@ import Foundation
|
||||
@Test @MainActor func activityViewModelInits() {
|
||||
let vm = ActivityViewModel(context: .local)
|
||||
#expect(vm.context.id == ServerContext.local.id)
|
||||
#expect(vm.entries.isEmpty)
|
||||
#expect(vm.toolMessages.isEmpty)
|
||||
}
|
||||
|
||||
@Test @MainActor func insightsViewModelInits() {
|
||||
|
||||
Reference in New Issue
Block a user