Skip to content

fix: stash dirty working tree before batch auto-release (t276) #1119

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
marcusquinn merged 1 commit into from
Feb 11, 2026
Merged

fix: stash dirty working tree before batch auto-release (t276) #1119

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
20 changes: 20 additions & 0 deletions .agents/scripts/supervisor-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2249,11 +2249,25 @@ trigger_batch_release() {
}
fi

# t276: Stash any dirty working tree before release.
# Common cause: todo/VERIFY.md, untracked files from parallel sessions.
# version-manager.sh refuses to release with uncommitted changes.
local stashed=false
if [[ -n "$(git -C "$repo" status --porcelain 2>/dev/null)" ]]; then
log_info "Stashing dirty working tree before release..."
if git -C "$repo" stash push -m "auto-release-stash-$(date +%Y%m%d%H%M%S)" 2>/dev/null; then
Comment on lines +2256 to +2258
Copy link

@gemini-code-assist gemini-code-assist bot Feb 11, 2026

Choose a reason for hiding this comment

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

medium

According to the repository style guide (rule 50), 2>/dev/null should only be used when redirecting to a log file. These lines suppress stderr from git status and git stash push, which can hide important error information. To improve observability and adhere to the style guide, please redirect stderr to the $SUPERVISOR_LOG file.

Suggested change
if [[ -n "$(git -C "$repo" status --porcelain 2>/dev/null)" ]]; then
log_info "Stashing dirty working tree before release..."
if git -C "$repo" stash push -m "auto-release-stash-$(date +%Y%m%d%H%M%S)" 2>/dev/null; then
if [[ -n "$(git -C "$repo" status --porcelain 2>>"$SUPERVISOR_LOG")" ]]; then
log_info "Stashing dirty working tree before release..."
if git -C "$repo" stash push -m "auto-release-stash-$(date +%Y%m%d%H%M%S)" &>>"$SUPERVISOR_LOG"; then
References
  1. Rule 50 states that 2>/dev/null is acceptable only when redirecting to log files, not for blanket suppression. The code uses 2>/dev/null to suppress stderr without redirecting to a log. (link)

stashed=true
else
log_warn "git stash failed, proceeding anyway (release may fail)"
fi
fi

# Pull latest (all batch PRs should be merged by now)
git -C "$repo" pull --ff-only origin main 2>/dev/null || {
log_warn "Fast-forward pull failed, trying rebase..."
git -C "$repo" pull --rebase origin main 2>/dev/null || {
log_error "Failed to pull latest main for release"
[[ "$stashed" == "true" ]] && git -C "$repo" stash pop 2>/dev/null || true
Copy link

@gemini-code-assist gemini-code-assist bot Feb 11, 2026

Choose a reason for hiding this comment

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

high

The use of || true here swallows the exit code from git stash pop, which means a failure to restore the stashed changes will go unnoticed. This could lead to lost work if the user isn't aware their changes weren't restored. Additionally, using 2>/dev/null violates the repository style guide (rule 50). It's better to handle the error explicitly with a warning message and redirect stderr to the log file.

Suggested change
[[ "$stashed" == "true" ]] && git -C "$repo" stash pop 2>/dev/null || true
[[ "$stashed" == "true" ]] && git -C "$repo" stash pop 2>>"$SUPERVISOR_LOG" || log_warn "Failed to restore stashed changes on pull failure. Manual recovery may be needed."
References
  1. Rule 50 states that 2>/dev/null is acceptable only when redirecting to log files, not for blanket suppression. The code uses 2>/dev/null to suppress stderr without redirecting to a log. (link)

return 1
}
}
Expand All @@ -2264,6 +2278,12 @@ trigger_batch_release() {
local release_exit=0
release_output=$(cd "$repo" && bash "$version_manager" release "$release_type" --skip-preflight --force 2>&1) || release_exit=$?

# t276: Restore stashed changes after release (regardless of success/failure)
if [[ "$stashed" == "true" ]]; then
log_info "Restoring stashed working tree..."
git -C "$repo" stash pop 2>/dev/null || log_warn "git stash pop failed (may need manual recovery)"
Copy link

@gemini-code-assist gemini-code-assist bot Feb 11, 2026

Choose a reason for hiding this comment

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

medium

This line violates the repository style guide (rule 50) by using 2>/dev/null to suppress stderr. While the error is handled by the || log_warn clause, the style guide is strict about redirecting stderr to a log file for better observability.

Suggested change
git -C "$repo" stash pop 2>/dev/null || log_warn "git stash pop failed (may need manual recovery)"
git -C "$repo" stash pop 2>>"$SUPERVISOR_LOG" || log_warn "git stash pop failed (may need manual recovery)"
References
  1. Rule 50 states that 2>/dev/null is acceptable only when redirecting to log files, not for blanket suppression. The code uses 2>/dev/null to suppress stderr without redirecting to a log. (link)

fi

echo "$release_output" > "$release_log" 2>/dev/null || true

if [[ "$release_exit" -ne 0 ]]; then
Expand Down
Loading