Skip to content

perf(tools): defer tool cache version check to background refresh #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChangeHow merged 1 commit into from
May 13, 2026
Merged

perf(tools): defer tool cache version check to background refresh #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions configs/shared/tools/_loader.zsh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,43 @@ _source_cached_tool_init() {
local cache_name="$1"
local binary_name="$2"
local init_command="$3"
local binary_path
local cache_file="$_zsh_tools_cache_dir/${cache_name}.zsh"
local tmp_file="${cache_file}.tmp"
local version_file="$_zsh_tools_cache_dir/${cache_name}.version"
local current_version

binary_path=$(command -v "$binary_name") || return 0
current_version=$("$binary_path" --version 2>/dev/null | head -1) || current_version="unknown"

# Fast path: if cache and version file exist, source directly.
# The version check is deferred to a background refresh so startup stays fast.
if [[ -s "$cache_file" && -s "$version_file" ]]; then
local cached_version
cached_version=$(<"$version_file")
if [[ "$current_version" == "$cached_version" ]]; then
source "$cache_file"
return
fi
source "$cache_file"

# Background refresh: if the cache is older than 24 hours, verify the
# binary version and rebuild when it has changed.
() {
emulate -L zsh
if [[ -n "$(command find "$cache_file" -mmin +1440 -print 2>/dev/null)" ]]; then
local binary_path current_version cached_version
binary_path=$(command -v "$binary_name") || return 0
current_version=$("$binary_path" --version 2>/dev/null | head -1) || current_version="unknown"
cached_version=$(<"$version_file")
[[ "$current_version" == "$cached_version" ]] && return 0
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When the binary version matches the cached version, the cache file's modification time remains unchanged. This causes the find command at line 42 to continue identifying the cache as 'stale' (older than 24 hours) on every subsequent shell startup. Updating the timestamp of the cache file when the version matches prevents these unnecessary background checks for another 24 hours.

        [[ "$current_version" == "$cached_version" ]] && { command touch "$cache_file"; return 0; }


if eval "$init_command" >| "$tmp_file" 2>/dev/null; then
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a potential race condition here. If multiple shell instances (e.g., opening multiple terminal tabs at once) start simultaneously and find the cache stale, they will all attempt to write to the same ${cache_file}.tmp path. This can lead to cache corruption if one process moves the file while another is still writing to it. Consider including the shell's PID ($$) in the temporary filename definition (at line 30) to ensure uniqueness across concurrent processes.

mv "$tmp_file" "$cache_file"
echo -n "$current_version" > "$version_file"
else
rm -f "$tmp_file"
fi
fi
} &!

return
fi

# Slow path: cache missing — check version and build synchronously.
local binary_path current_version
binary_path=$(command -v "$binary_name") || return 0
current_version=$("$binary_path" --version 2>/dev/null | head -1) || current_version="unknown"

if eval "$init_command" >| "$tmp_file" 2>/dev/null; then
mv "$tmp_file" "$cache_file"
echo -n "$current_version" > "$version_file"
Expand Down
11 changes: 11 additions & 0 deletions tests/frontend.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,27 @@ const CURL_HTTP_ERROR = "curl exited with HTTP error 22";

describe("frontend step", () => {
let sandbox;
let originalFnmDir;
let originalXdgDataHome;

beforeEach(() => {
vi.clearAllMocks();
sandbox = mkdtempSync(join(tmpdir(), "suitup-frontend-"));
// Default: fetch LTS version fails gracefully
run.mockImplementation(() => { throw new Error("no curl"); });
// Isolate from host fnm installation so tests use the sandbox path
originalFnmDir = process.env.FNM_DIR;
originalXdgDataHome = process.env.XDG_DATA_HOME;
delete process.env.FNM_DIR;
delete process.env.XDG_DATA_HOME;
});

afterEach(() => {
rmSync(sandbox, { recursive: true, force: true });
if (originalFnmDir !== undefined) process.env.FNM_DIR = originalFnmDir;
else delete process.env.FNM_DIR;
if (originalXdgDataHome !== undefined) process.env.XDG_DATA_HOME = originalXdgDataHome;
else delete process.env.XDG_DATA_HOME;
});

test("skips all tools when already installed", async () => {
Expand Down
Loading