mirrors/ghostty
1
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2026-08-01 13:19:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

cli: report ssh terminfo cache failures

Browse Source 
A state directory with the wrong permissions left the terminfo cache
failing with errors that named no path, so there was nothing to act on:

    $ ghostty +ssh-cache --add=user@host
    Error: Unable to add 'user@host' to cache. Error: error.AccessDenied

Every +ssh-cache failure now names its cache file, and +ssh no longer
swallows cache-related errors.

Error messages in these actions are also lowercased after the "Error: "
prefix and append the error with ": {t}" rather than a second "Error: ".

Ref: https://github.com/ghostty-org/ghostty/issues/9393#issuecomment-5145799368
This commit is contained in:
Jon Parise
2026-07-31 15:31:58 -04:00
parent 4d605bf0d8
commit 8fca64957b
2 changed files with 110 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
src/cli/ssh.zig
View File

@@ -245,14 +245,24 @@ fn runInner(
const cache: ?DiskCache = if (opts.cache) cache: {
const path = DiskCache.defaultPath(alloc, "ghostty") catch |err| {
warnPrint(stderr, "ghostty terminfo cache unavailable: {}", .{err});
warnPrint(stderr, "ghostty terminfo cache unavailable: {t}", .{err});
break :session .{ .term = "xterm-256color" };
};
break :cache .{ .path = path };
} else null;
if (cache) |c| {
if (c.contains(alloc, dest) catch false) {
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(
stderr,
"unable to add '{s}' to the cache: {t}",
.{ c.path, err },
);
break :cached false;
};
if (cached) {
verbosePrint(opts, stderr, "dest: {s} (cached, skipping install)", .{dest});
break :session .{ .term = "xterm-ghostty" };
} else {
@@ -266,7 +276,7 @@ fn runInner(
stderr.flush() catch {};
installRemoteTerminfo(alloc, opts, stderr) catch |err| {
warnPrint(stderr, "failed to install terminfo: {}", .{err});
warnPrint(stderr, "failed to install terminfo: {t}", .{err});
break :session .{ .term = "xterm-256color" };
};
break :session .{
@@ -297,7 +307,7 @@ fn runInner(
verbosePrint(opts, stderr, "exec: {f}", .{Joined{ .items = argv }});
const exit_code = childExec(argv) catch |err| {
try stderr.print("Error: failed to run {s}: {}\n", .{ argv[0], err });
try stderr.print("Error: failed to run {s}: {t}\n", .{ argv[0], err });
return 1;
};
verbosePrint(opts, stderr, "exit: {d}", .{exit_code});
@@ -309,8 +319,23 @@ fn runInner(
.real,
).toSeconds())) |_| {
verbosePrint(opts, stderr, "cache: wrote {s}", .{entry.dest});
} else |err| {
log.debug("cache add failed for '{s}': {}", .{ entry.dest, err });
} 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 },
),
}
};

96
src/cli/ssh_cache.zig
View File

@@ -133,7 +133,24 @@ pub fn run(alloc_gpa: Allocator) !u8 {
};
}
const result = runInner(alloc, opts, query, stdout, stderr);
// Setup our disk cache to the standard location
const cache_path = DiskCache.defaultPath(alloc, "ghostty") catch |err| {
try stderr.print(
"Error: unable to determine the cache path: {t}\n",
.{err},
);
stderr.flush() catch {};
return 1;
};
const result = runInner(
alloc,
opts,
query,
.{ .path = cache_path },
stdout,
stderr,
);
// Flushing *shouldn't* fail but...
stdout.flush() catch {};
@@ -145,6 +162,7 @@ pub fn runInner(
alloc: Allocator,
opts: Options,
query: ?[]const u8,
cache: DiskCache,
stdout: *std.Io.Writer,
stderr: *std.Io.Writer,
) !u8 {
@@ -165,12 +183,14 @@ pub fn runInner(
return 2;
}
// Setup our disk cache to the standard location
const cache_path = try DiskCache.defaultPath(alloc, "ghostty");
const cache: DiskCache = .{ .path = cache_path };
if (opts.clear) {
try cache.clear();
cache.clear() catch |err| {
try stderr.print(
"Error: unable to clear cache '{s}': {t}\n",
.{ cache.path, err },
);
return 1;
};
return 0;
}
@@ -182,15 +202,15 @@ pub fn runInner(
) catch |err| switch (err) {
error.InvalidCacheKey => {
try stderr.print(
"Error: Invalid destination '{s}' (expected hostname or user@hostname)\n",
"Error: invalid destination '{s}' (expected hostname or user@hostname)\n",
.{dest},
);
return 2;
},
else => {
try stderr.print(
"Error: Unable to add '{s}' to cache. Error: {}\n",
.{ dest, err },
"Error: unable to add '{s}' to cache '{s}': {t}\n",
.{ dest, cache.path, err },
);
return 1;
},
@@ -202,15 +222,15 @@ pub fn runInner(
const removed = cache.remove(alloc, dest) catch |err| switch (err) {
error.InvalidCacheKey => {
try stderr.print(
"Error: Invalid destination '{s}' (expected hostname or user@hostname)\n",
"Error: invalid destination '{s}' (expected hostname or user@hostname)\n",
.{dest},
);
return 2;
},
else => {
try stderr.print(
"Error: Unable to remove '{s}' from cache. Error: {}\n",
.{ dest, err },
"Error: unable to remove '{s}' from cache '{s}': {t}\n",
.{ dest, cache.path, err },
);
return 1;
},
@@ -233,14 +253,23 @@ pub fn runInner(
return 2;
}
const pruned = cache.prune(alloc, max_age_s) catch |err| {
try stderr.print("Error: Unable to prune cache. Error: {}\n", .{err});
try stderr.print(
"Error: unable to prune cache '{s}': {t}\n",
.{ cache.path, err },
);
return 1;
};
try stdout.print("Pruned cache entries: {d}\n", .{pruned});
return 0;
}
var entries = try cache.list(alloc);
var entries = cache.list(alloc) catch |err| {
try stderr.print(
"Error: unable to read cache '{s}': {t}\n",
.{ cache.path, err },
);
return 1;
};
defer DiskCache.deinitEntries(alloc, &entries);
// A positional query filters the listing: an exact `user@host` match,
@@ -248,7 +277,7 @@ pub fn runInner(
if (query) |q| {
if (!DiskCache.isValidCacheKey(q)) {
try stderr.print(
"Error: Invalid destination '{s}' (expected hostname or user@hostname)\n",
"Error: invalid destination '{s}' (expected hostname or user@hostname)\n",
.{q},
);
return 2;
@@ -474,10 +503,11 @@ test "runInner rejects multiple actions" {
defer stderr.deinit();
// The check runs before any cache access, so it never touches disk.
const cache: DiskCache = .{ .path = "/nonexistent/ssh_cache" };
const code = try runInner(alloc, .{
.add = "example.com",
.remove = "other.com",
}, null, &stdout.writer, &stderr.writer);
}, null, cache, &stdout.writer, &stderr.writer);
try testing.expectEqual(@as(u8, 2), code);
try testing.expectEqualStrings("", stdout.written());
@@ -487,7 +517,39 @@ test "runInner rejects multiple actions" {
stderr.clearRetainingCapacity();
const code2 = try runInner(alloc, .{
.clear = true,
}, "example.com", &stdout.writer, &stderr.writer);
}, "example.com", cache, &stdout.writer, &stderr.writer);
try testing.expectEqual(@as(u8, 2), code2);
try testing.expect(std.mem.indexOf(u8, stderr.written(), "only one") != null);
}
test "runInner fails cleanly when the cache is unusable" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
// A directory where the cache file belongs fails every operation.
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.createDirPath(io, "ssh_cache");
const tmp_path = try tmp.dir.realPathFileAlloc(io, ".", alloc);
defer alloc.free(tmp_path);
const cache_path = try std.fs.path.join(alloc, &.{ tmp_path, "ssh_cache" });
defer alloc.free(cache_path);
const cache: DiskCache = .{ .path = cache_path };
var stdout: std.Io.Writer.Allocating = .init(alloc);
defer stdout.deinit();
var stderr: std.Io.Writer.Allocating = .init(alloc);
defer stderr.deinit();
const opts: []const Options = &.{
.{}, // the default listing
.{ .add = "example.com" },
.{ .remove = "example.com" },
.{ .prune = .{ .duration = std.time.ns_per_s } },
};
for (opts) |o| {
const code = try runInner(alloc, o, null, cache, &stdout.writer, &stderr.writer);
try testing.expectEqual(@as(u8, 1), code);
}
}