feat(init): cache output for faster shell startup#142
Merged
Conversation
Cache generated shell integration to ~/.cache/gtr/ with version-stamped invalidation. On warm cache, sourcing the file directly avoids the subprocess overhead entirely (~5ms vs ~60ms). - init now writes cache file keyed by version + func name + shell - Cache auto-invalidates when GTR_VERSION changes - Docs updated to recommend source pattern as primary setup method - 5 new tests for cache creation, hit, invalidation, and key isolation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an XDG-aware cache for shell integration outputs produced by Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InitCmd as git-gtr:init
participant Generator
participant Cache as Cache(~/.cache/gtr)
User->>InitCmd: run `git gtr init --shell <s> [--as <fn>]`
InitCmd->>Cache: check cache file, read first-line stamp
alt stamp matches (version+fn+shell)
Cache-->>InitCmd: return cached content
InitCmd-->>User: output cached init script
else cache miss
InitCmd->>Generator: invoke generator for <s> (as <fn>)
Generator-->>InitCmd: generated script
InitCmd->>Cache: write stamp + generated script
InitCmd-->>User: output generated init script
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Update all remaining docs, error messages, and hints to point to the source pattern or `git gtr help init` instead of bare eval. Doctor detection also matches source-pattern users (gtr/init- in rc).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/commands/help.sh (1)
349-368: Samesourcewithout|| trueissue as README.md (lines 363, 368).The setup snippets in
_help_init()at lines 363 and 368 have the same pattern as flagged in README.md —source "$_gtr_init" 2>/dev/nullandeval "$(git gtr init bash)"without a|| trueguard. Please apply the same fix here when addressing the README.md finding.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commands/help.sh` around lines 349 - 368, In _help_init(), fix the same unsafe invocation pattern by ensuring the init evaluation and sourcing cannot cause a non-zero exit: append a safe fallback (e.g., "|| true") to the eval "$(git gtr init bash)" and eval "$(git gtr init zsh)" calls and to the source "$_gtr_init" 2>/dev/null invocations so failures won't propagate; update the two occurrences that match those exact strings in the function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/commands/init.sh`:
- Around line 68-88: The cache write (mkdir -p "$cache_dir" and printf ... >
"$cache_file") can fail under set -e and cause the function to exit without
emitting the generated output; update the block that writes the cache so it
always prints "$output" to stdout regardless of cache-write failures by first
emitting the output (printf '%s\n' "$output") and then attempting the cache
mkdir+write in a non-fatal way (e.g., disable errexit around the mkdir/printf or
redirect to a temp file and move it with a conditional), referencing the
variables and commands in this block: cache_dir, cache_file, cache_stamp,
generator, output, func_name, and shell so the generator result is never
swallowed even if caching fails.
In `@README.md`:
- Around line 219-220: The two shell invocations can propagate non-zero exits
into strict shells (set -e); change the eval and source invocations so their
failures are ignored (e.g., append "|| true" or "|| :" to both the eval "$(git
gtr init bash)" and the source "$_gtr_init" calls) so a failed eval or a failing
source does not abort the user's shell startup; update the lines with those
exact commands (the eval "$(git gtr init bash)" and source "$_gtr_init") to
include the no-op fallback.
In `@tests/init.bats`:
- Around line 232-288: Tests share a non-unique local cache_dir ("local
cache_dir=\"$BATS_TMPDIR/gtr-cache-test-$$\"") which can leak between tests if a
test fails; fix by making each test use the already-isolated XDG_CACHE_HOME from
setup() (or create a unique temp with mktemp per test) and ensure teardown()
removes that directory instead of relying on per-test rm -rf lines — update the
tests that reference cache_dir (e.g., "init creates cache file and returns
output", "init returns cached output on second call", "cache invalidates when
version changes", "cache uses --as func name in cache key", "cache works for all
shells") to either use XDG_CACHE_HOME directly or set a unique per-test dir and
remove the shared rm -rf "$cache_dir" lines, and update teardown() to clean
XDG_CACHE_HOME so the cache cannot leak between tests.
---
Duplicate comments:
In `@lib/commands/help.sh`:
- Around line 349-368: In _help_init(), fix the same unsafe invocation pattern
by ensuring the init evaluation and sourcing cannot cause a non-zero exit:
append a safe fallback (e.g., "|| true") to the eval "$(git gtr init bash)" and
eval "$(git gtr init zsh)" calls and to the source "$_gtr_init" 2>/dev/null
invocations so failures won't propagate; update the two occurrences that match
those exact strings in the function.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
CLAUDE.mdREADME.mdbin/git-gtrlib/commands/help.shlib/commands/init.shtests/init.bats
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/commands/init.sh (1)
86-88:⚠️ Potential issue | 🟠 MajorDon’t let cache write failures suppress init output under
set -e.If Line 86 (
mkdir -p) or Line 87 (redirect write) fails,cmd_initcan exit before Line 88 prints the generated script. Cache persistence should be best-effort, not required for output delivery.
As per coding guidelines,**/*.sh: "Keepset -eactive in shell scripts; ensure non-critical failures are guarded withcommand || true."🛠️ Proposed fix
local output output="$("$generator" | sed "s/__FUNC__/$func_name/g")" - mkdir -p "$cache_dir" - printf '%s\n%s\n' "$cache_stamp" "$output" > "$cache_file" printf '%s\n' "$output" + if mkdir -p "$cache_dir" 2>/dev/null; then + printf '%s\n%s\n' "$cache_stamp" "$output" > "$cache_file" 2>/dev/null || true + fi#!/bin/bash # Verify set -e context and unguarded cache-write pattern. rg -n -C2 'set -e' bin/git-gtr rg -n -C4 'mkdir -p "\$cache_dir"|printf .*> "\$cache_file"|printf .*\$output' lib/commands/init.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commands/init.sh` around lines 86 - 88, The cache persistence is currently allowed to abort cmd_init under set -e; make the cache operations best-effort so failure doesn't suppress the script output: guard mkdir -p "$cache_dir" and the cache write that uses printf '%s\n%s\n' "$cache_stamp" "$output" > "$cache_file" with non-fatal fallbacks (e.g. append "|| true" or wrap in a conditional) so errors on cache_dir, cache_stamp, cache_file do not cause exit, but leave the final printf '%s\n' "$output" unchanged so the generated script always prints; refer to the symbols cache_dir, cache_stamp, cache_file and output when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/commands/help.sh`:
- Around line 349-352: The help text currently states "Output is cached to
~/.cache/gtr/ for fast shell startup... The cache auto-invalidates when git-gtr
is updated," but the implementation only regenerates the cache when the cache
file is missing (see the logic that checks for the cached file and the lines
showing "To force-regenerate: rm -rf ~/.cache/gtr" and the init/regenerate
behavior); update the message text (the block containing "Output is cached to
~/.cache/gtr/..." and the "To force-regenerate:" line) to accurately state that
the cache persists across upgrades and will only be refreshed when you re-run
`git gtr init` or remove ~/.cache/gtr, and keep the existing "To
force-regenerate: rm -rf ~/.cache/gtr" instruction.
---
Duplicate comments:
In `@lib/commands/init.sh`:
- Around line 86-88: The cache persistence is currently allowed to abort
cmd_init under set -e; make the cache operations best-effort so failure doesn't
suppress the script output: guard mkdir -p "$cache_dir" and the cache write that
uses printf '%s\n%s\n' "$cache_stamp" "$output" > "$cache_file" with non-fatal
fallbacks (e.g. append "|| true" or wrap in a conditional) so errors on
cache_dir, cache_stamp, cache_file do not cause exit, but leave the final printf
'%s\n' "$output" unchanged so the generated script always prints; refer to the
symbols cache_dir, cache_stamp, cache_file and output when making the change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
CLAUDE.mdbin/git-gtrlib/commands/doctor.shlib/commands/help.shlib/commands/init.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- bin/git-gtr
- Make cache write best-effort in cmd_init (print output first, guard mkdir/write with || true) so set -e failures don't swallow shell init - Add || true guards to eval/source in README and help.sh setup examples to protect users with set -e in their shell rc - Fix misleading "auto-invalidates on update" wording in help text — the cached setup pattern only regenerates when the cache file is missing - Simplify cache tests to use setup()'s XDG_CACHE_HOME instead of a shared per-PID cache_dir that can leak between tests on failure
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
227-228: Fish setup doesn't use caching pattern.The Fish setup uses
git gtr init fish | sourcewhich regenerates on every shell startup (~60ms), unlike bash/zsh which source the cached file directly (~1ms). This is inconsistent with the PR's goal of faster shell startup.Consider documenting a cached pattern for Fish users who want faster startup:
📝 Suggested documentation addition
# Fish (add to ~/.config/fish/config.fish) git gtr init fish | source + +# Fish (alternative for faster startup - advanced) +# set -l _gtr_init "$XDG_CACHE_HOME/gtr/init-gtr.fish" +# test -f "$_gtr_init"; or git gtr init fish > /dev/null +# source "$_gtr_init"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 227 - 228, Document a cached startup pattern for Fish users instead of recommending `git gtr init fish | source` every shell startup; update the README entry (the Fish config example) to show generating a cached file once (e.g., `git gtr init fish` output redirected to a cache file) and sourcing that cache file on each shell start, with an optional regeneration command or status check to refresh the cache when needed; reference the `git gtr init fish` invocation and `~/.config/fish/config.fish` so readers can replace the direct pipe with a cache file approach for faster startup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Around line 227-228: Document a cached startup pattern for Fish users instead
of recommending `git gtr init fish | source` every shell startup; update the
README entry (the Fish config example) to show generating a cached file once
(e.g., `git gtr init fish` output redirected to a cache file) and sourcing that
cache file on each shell start, with an optional regeneration command or status
check to refresh the cache when needed; reference the `git gtr init fish`
invocation and `~/.config/fish/config.fish` so readers can replace the direct
pipe with a cache file approach for faster startup.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.mdlib/commands/help.shlib/commands/init.shtests/init.bats
…zation - Update Fish setup instructions to use a cached initialization file instead of direct command execution. - Implement checks for the existence of the cache file before attempting to initialize, improving robustness. - Ensure compatibility with users who have set -e in their shell configuration by adding error handling to source commands.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/commands/help.sh`:
- Around line 573-574: Update the cache description string "Output is cached for
fast startup (auto-invalidates on update)" to accurately reflect the actual
behavior described in _help_init; replace it with wording like "Output is cached
for fast startup; the cache refreshes the next time 'git gtr init <shell>' runs"
so the help text for the cache and the _help_init function are consistent.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.mdlib/commands/help.sh
Change "auto-invalidates on update" to "refreshes when 'git gtr init' runs" to match the accurate description in `git gtr help init`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
git gtr initnow caches generated shell code to~/.cache/gtr/with a version stamptail -n +2reads the file directly (~1ms) instead of regenerating (~60ms)GTR_VERSIONchanges (e.g., afterbrew upgrade)eval)Before (every shell startup)
After (recommended setup)
Test plan
--askey isolation, all shellsSummary by CodeRabbit
New Features
Documentation
Bug Fixes / UX
Tests