From 48b10aa8fa6c8b9a1b147fa1a4a209000b3997e9 Mon Sep 17 00:00:00 2001 From: justrach <54503978+justrach@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:12:24 +0800 Subject: [PATCH] shell: retry a transient shell-handle failure and name its cause Reserving a durable shell handle mapped every unexpected error to ShellIdentityUnavailable, and the message always blamed HOME's .codegraff permissions. A transient fd or lock failure looked like broken storage, and the real cause was lost; the next call worked without any repair. Errors that don't say the storage itself is unusable are retried three times. The underlying error is kept and shown, and only a storage fault (access, read-only, full disk, missing or bad path) gets the permissions advice; anything else says it is usually transient. Fixes #1619 --- src/exec_bash.zig | 25 ++++++++++++++++++++++++- src/shell_identity.zig | 31 ++++++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/exec_bash.zig b/src/exec_bash.zig index 034401628..36465a478 100644 --- a/src/exec_bash.zig +++ b/src/exec_bash.zig @@ -66,7 +66,15 @@ fn spawnFailText(gpa: Allocator, err: anyerror) ![]u8 { // #122: backgrounding costs MORE fds (pipes + pump task), so the generic // "run it in the foreground" advice is right for every error except the // fd-quota ones. #253: SYSTEM-wide exhaustion is not a ulimit problem. - if (err == error.ShellIdentityHomeRequired or err == error.ShellIdentityStoreInvalid or err == error.ShellIdentityExhausted or err == error.ShellIdentityUnavailable) + if (err == error.ShellIdentityUnavailable) { + const shell_identity = @import("shell_identity.zig"); + const cause = shell_identity.last_cause orelse error.Unexpected; + // #1619: name the cause; only a storage fault is a permissions problem. + if (!shell_identity.storageFault(cause)) + return std.fmt.allocPrint(gpa, "could not reserve a durable shell handle (ShellIdentityUnavailable: {t}); no command was started. This was retried and is usually transient: run the command again.", .{cause}); + return std.fmt.allocPrint(gpa, "could not reserve a durable shell handle (ShellIdentityUnavailable: {t}); no command was started. Check HOME and its .codegraff directory permissions. Preserve existing shell-id files when repairing storage.", .{cause}); + } + if (err == error.ShellIdentityHomeRequired or err == error.ShellIdentityStoreInvalid or err == error.ShellIdentityExhausted) return std.fmt.allocPrint(gpa, "could not reserve a durable shell handle ({t}); no command was started. Check HOME and its .codegraff directory permissions. Preserve existing shell-id files when repairing storage.", .{err}); return if (err == error.ProcessFdQuotaExceeded) try gpa.dupe(u8, "could not start background job (ProcessFdQuotaExceeded) — graff hit its open-file limit. Wait for running jobs/tools to finish, then retry with less parallel fan-out; if it recurs, raise the limit (`ulimit -n 4096`) before starting graff.") @@ -457,3 +465,18 @@ fn execUnchecked(ctx: ToolCtx, call: tools.ToolCall) !ToolOutput { defer gpa.free(run.stderr); return formatCapped(gpa, cmd, run); } + +test "#1619 a shell handle failure names its cause and blames storage only for storage faults" { + const shell_identity = @import("shell_identity.zig"); + defer shell_identity.last_cause = null; + shell_identity.last_cause = error.ProcessFdQuotaExceeded; + const transient = try spawnFailText(std.testing.allocator, error.ShellIdentityUnavailable); + defer std.testing.allocator.free(transient); + try std.testing.expect(std.mem.indexOf(u8, transient, "ShellIdentityUnavailable: ProcessFdQuotaExceeded") != null); + try std.testing.expect(std.mem.indexOf(u8, transient, "permissions") == null); + shell_identity.last_cause = error.AccessDenied; + const storage = try spawnFailText(std.testing.allocator, error.ShellIdentityUnavailable); + defer std.testing.allocator.free(storage); + try std.testing.expect(std.mem.indexOf(u8, storage, "ShellIdentityUnavailable: AccessDenied") != null); + try std.testing.expect(std.mem.indexOf(u8, storage, "permissions") != null); +} diff --git a/src/shell_identity.zig b/src/shell_identity.zig index cccb5e6df..69793d19d 100644 --- a/src/shell_identity.zig +++ b/src/shell_identity.zig @@ -27,9 +27,34 @@ pub fn forJob(io: Io) !Id { } return reserve(io, test_home.?); } - return reserve(io, @import("job_registry.zig").home) catch |err| switch (err) { - error.ShellIdentityHomeRequired, error.ShellIdentityStoreInvalid, error.ShellIdentityExhausted => return err, - else => return error.ShellIdentityUnavailable, + const home = @import("job_registry.zig").home; + // #1619: an fd or lock hiccup cleared on its own a moment later, while + // the message blamed HOME's permissions. Retry those, and keep the cause. + var attempt: u8 = 0; + while (true) : (attempt += 1) { + return reserve(io, home) catch |err| switch (err) { + error.ShellIdentityHomeRequired, error.ShellIdentityStoreInvalid, error.ShellIdentityExhausted => return err, + error.Canceled => return err, + else => { + if (!storageFault(err) and attempt < 3) { + io.sleep(.fromMilliseconds(25), .awake) catch return err; + continue; + } + last_cause = err; + return error.ShellIdentityUnavailable; + }, + }; + } +} + +/// The underlying error behind the last ShellIdentityUnavailable. +pub threadlocal var last_cause: ?anyerror = null; + +/// Errors that say HOME's storage itself is unusable; retrying cannot help. +pub fn storageFault(err: anyerror) bool { + return switch (err) { + error.AccessDenied, error.PermissionDenied, error.ReadOnlyFileSystem, error.NoSpaceLeft, error.NotDir, error.FileNotFound, error.SymLinkLoop, error.NameTooLong, error.BadPathName, error.DiskQuota => true, + else => false, }; }