Skip to content

feat(dotfiles-updater): skip build/switch when no changes detected #558

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
shunkakinoki merged 3 commits into from
Jan 12, 2026
Merged

feat(dotfiles-updater): skip build/switch when no changes detected #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e42dc40
feat(dotfiles-updater): skip build/switch when no changes detected
shunkakinoki Jan 12, 2026
7306027
test(dotfiles-updater): add tests for change detection
shunkakinoki Jan 12, 2026
b4dd1d8
fix(tests): disable SC2016 for literal string assertions
shunkakinoki Jan 12, 2026
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
18 changes: 17 additions & 1 deletion home-manager/services/dotfiles-updater/update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,24 @@ if [ "$(git rev-parse --abbrev-ref HEAD)" != "main" ]; then
exit 0
fi

# Fetch and pull latest changes
# Store current commit hash before fetching
CURRENT_COMMIT=$(git rev-parse HEAD)

# Fetch latest changes
git fetch origin main

@mesa-dot-dev mesa-dot-dev Bot Jan 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

Consider adding error handling for the git fetch operation. If the fetch fails (network issues, remote unavailable), the script will exit due to set -e, but REMOTE_COMMIT will never be set. Adding explicit error handling or at least logging would make debugging easier:

if ! git fetch origin main; then
  echo "Failed to fetch from origin/main"
  exit 1
fi

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent 
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#558
File: home-manager/services/dotfiles-updater/update.sh#L17
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Consider adding error handling for the git fetch operation. If the fetch fails (network issues, remote unavailable), the script will exit due to `set -e`, but REMOTE_COMMIT will never be set. Adding explicit error handling or at least logging would make debugging easier:
```bash
if ! git fetch origin main; then
  echo "Failed to fetch from origin/main"
  exit 1
fi


</details>


# Get the latest remote commit
REMOTE_COMMIT=$(git rev-parse origin/main)

# Check if there are any changes
if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then

@gemini-code-assist gemini-code-assist Bot Jan 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For better robustness and to follow modern bash best practices, it's recommended to use the [[ ... ]] compound command for tests instead of the POSIX [ ... ]. The [[ keyword is specific to bash (and other modern shells) and is safer as it prevents issues like word splitting on variable expansions. While your use of quotes makes the current code safe, [[ is generally preferred for new bash code. When using [[, it's also idiomatic to use == for string comparison.

Suggested change
if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then
if [[ "$CURRENT_COMMIT" == "$REMOTE_COMMIT" ]]; then

@mesa-dot-dev mesa-dot-dev Bot Jan 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

This optimization skips the install when commits match, but doesn't account for scenarios where the previous install.sh run may have failed partially. Consider tracking the last successful install commit in a state file (e.g., ~/.dotfiles-updater-last-success) and comparing against that instead of just HEAD. This would ensure a retry if the previous install failed.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent 
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#558
File: home-manager/services/dotfiles-updater/update.sh#L23
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
This optimization skips the install when commits match, but doesn't account for scenarios where the previous install.sh run may have failed partially. Consider tracking the last successful install commit in a state file (e.g., ~/.dotfiles-updater-last-success) and comparing against that instead of just HEAD. This would ensure a retry if the previous install failed.

echo "No changes detected (current: ${CURRENT_COMMIT:0:8}). Skipping build and switch."
exit 0

@cubic-dev-ai cubic-dev-ai Bot Jan 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Early exit skips git reset --hard, leaving dirty working tree unchanged when no upstream changes.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/dotfiles-updater/update.sh, line 25:

<comment>Early exit skips `git reset --hard`, leaving dirty working tree unchanged when no upstream changes.</comment>

<file context>
@@ -10,8 +10,24 @@ if [ "$(git rev-parse --abbrev-ref HEAD)" != "main" ]; then
+# Check if there are any changes
+if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then
+  echo "No changes detected (current: ${CURRENT_COMMIT:0:8}). Skipping build and switch."
+  exit 0
+fi
+
</file context>
Suggested change
exit 0
git reset --hard origin/main
exit 0
Fix with Cubic

fi
Comment on lines +23 to +26

Copilot AI Jan 12, 2026

Copy link

Choose a reason for hiding this comment

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

The new commit comparison logic lacks test coverage. Consider adding tests to verify:

  1. The script exits early when CURRENT_COMMIT equals REMOTE_COMMIT
  2. The script proceeds with installation when commits differ
  3. The appropriate log messages are output in each case

This is important functionality that should be tested given the repository has comprehensive test coverage for shell scripts.

Copilot uses AI. Check for mistakes.

echo "Changes detected: ${CURRENT_COMMIT:0:8} -> ${REMOTE_COMMIT:0:8}"

# Reset to latest
git reset --hard origin/main

# Run the install script
Expand Down
30 changes: 30 additions & 0 deletions spec/dotfiles_updater_spec.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,36 @@ The output should include 'git reset --hard origin/main'
End
End

Describe 'change detection'
It 'stores current commit before fetching'
When run bash -c "grep 'CURRENT_COMMIT=\$(git rev-parse HEAD)' '$SCRIPT'"
# shellcheck disable=SC2016
The output should include 'CURRENT_COMMIT=$(git rev-parse HEAD)'
End

It 'gets remote commit after fetching'
When run bash -c "grep 'REMOTE_COMMIT=\$(git rev-parse origin/main)' '$SCRIPT'"
# shellcheck disable=SC2016
The output should include 'REMOTE_COMMIT=$(git rev-parse origin/main)'
End

It 'compares commits to detect changes'
When run bash -c "grep 'CURRENT_COMMIT.*=.*REMOTE_COMMIT' '$SCRIPT'"
The output should include 'CURRENT_COMMIT'
The output should include 'REMOTE_COMMIT'
End

It 'skips build when no changes detected'
When run bash -c "grep 'No changes detected' '$SCRIPT'"
The output should include 'No changes detected'
End

It 'logs when changes are detected'
When run bash -c "grep 'Changes detected' '$SCRIPT'"
The output should include 'Changes detected'
End
End

Describe 'installation'
It 'runs install.sh after update'
When run bash -c "grep './install.sh' '$SCRIPT'"
Expand Down
Loading