From ca3dc9eeac7893d6ac4507d60761226cb16fd09f Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Fri, 31 Jul 2026 16:41:25 -0400 Subject: [PATCH] cli: classify ssh cache errors in DiskCache --- src/cli/ssh-cache/DiskCache.zig | 19 +++++++++++++++++ src/cli/ssh.zig | 37 +++++++++++++++------------------ 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/cli/ssh-cache/DiskCache.zig b/src/cli/ssh-cache/DiskCache.zig index b700c443d..29f97efb1 100644 --- a/src/cli/ssh-cache/DiskCache.zig +++ b/src/cli/ssh-cache/DiskCache.zig @@ -393,6 +393,16 @@ fn readEntries( return entries; } +/// Whether an error from a cache operation reflects a problem with the cache +/// itself. A destination we can't key on and a lock held by another process +/// both leave the cache healthy, so neither is worth reporting. +pub fn isFailure(err: anyerror) bool { + return switch (err) { + error.InvalidCacheKey, error.CacheLocked => false, + else => true, + }; +} + // Supports both standalone hostnames and user@hostname format pub fn isValidCacheKey(key: []const u8) bool { if (key.len == 0) return false; @@ -704,6 +714,15 @@ test "disk cache add survives allocation failure" { } } +test isFailure { + const testing = std.testing; + + try testing.expect(!isFailure(error.InvalidCacheKey)); + try testing.expect(!isFailure(error.CacheLocked)); + try testing.expect(isFailure(error.AccessDenied)); + try testing.expect(isFailure(error.IsDir)); +} + test isValidHost { const testing = std.testing; diff --git a/src/cli/ssh.zig b/src/cli/ssh.zig index 5dc115652..f20188d91 100644 --- a/src/cli/ssh.zig +++ b/src/cli/ssh.zig @@ -253,10 +253,9 @@ fn runInner( if (cache) |c| { const cached = c.contains(alloc, dest) catch |err| cached: { - // An invalid key is a bad destination, not a bad cache. - if (err != error.InvalidCacheKey) warnPrint( + if (DiskCache.isFailure(err)) warnPrint( stderr, - "unable to add '{s}' to the cache: {t}", + "unable to read the cache '{s}': {t}", .{ c.path, err }, ); break :cached false; @@ -319,23 +318,21 @@ fn runInner( .real, ).toSeconds())) |_| { verbosePrint(opts, stderr, "cache: wrote {s}", .{entry.dest}); - } else |err| switch (err) { - // An uncacheable destination was already reported above, and a - // lock held by a concurrent `+ssh` resolves itself. - error.InvalidCacheKey, - error.CacheLocked, - => verbosePrint( - opts, - stderr, - "cache: skipped {s}: {t}", - .{ entry.dest, err }, - ), - - else => warnPrint( - stderr, - "unable to add '{s}' to the cache '{s}': {t}", - .{ entry.dest, entry.cache.path, err }, - ), + } else |err| { + if (DiskCache.isFailure(err)) { + warnPrint( + stderr, + "unable to add '{s}' to the cache '{s}': {t}", + .{ entry.dest, entry.cache.path, err }, + ); + } else { + verbosePrint( + opts, + stderr, + "cache: skipped {s}: {t}", + .{ entry.dest, err }, + ); + } } };