Skip to content
Merged
Changes from 1 commit
Commits
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
70 changes: 62 additions & 8 deletions .agents/scripts/auto-update-helper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,23 @@ get_local_version() {
#######################################
get_remote_version() {
local version=""

# Prefer authenticated gh api (higher rate limit: 5000/hr vs 60/hr)
# This avoids the "remote=unknown" failures seen during overnight polling
# when unauthenticated API quota is exhausted.
# See: https://github.com/marcusquinn/aidevops/issues/4142
if command -v gh &>/dev/null && gh auth status &>/dev/null; then
version=$(gh api repos/marcusquinn/aidevops/contents/VERSION \
--jq '.content' 2>/dev/null |
base64 -d 2>/dev/null |
tr -d '\n')
if [[ -n "$version" ]]; then
echo "$version"
return 0
fi
Comment on lines +311 to +319

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

The gh api command uses --jq '.content', which relies on jq being available. To align with the repository rule of preferring built-in tools like grep over external dependencies like jq for simple tasks, consider parsing the JSON output directly using grep or sed instead of gh's --jq flag. This avoids the jq dependency entirely and improves portability.

Suggested change
if command -v gh &>/dev/null && gh auth status &>/dev/null; then
version=$(gh api repos/marcusquinn/aidevops/contents/VERSION \
--jq '.content' 2>/dev/null |
base64 -d 2>/dev/null |
tr -d '\n')
if [[ -n "$version" ]]; then
echo "$version"
return 0
fi
if command -v gh >/dev/null && gh auth status >/dev/null; then
version=$(gh api repos/marcusquinn/aidevops/contents/VERSION 2>/dev/null | \
grep -oP '"content":\s*"\K[^"]+' | \
base64 -d 2>/dev/null | \
tr -d '\n')
if [[ -n "$version" ]]; then
echo "$version"
return 0
fi
fi
References
  1. For shell scripts that need to be portable, prefer built-in tools like grep over external dependencies like jq for simple tasks, to avoid installation requirements on diverse systems.

fi

# Fallback: unauthenticated curl (60 req/hr limit)
if command -v jq &>/dev/null; then
version=$(curl --proto '=https' -fsSL --max-time 10 \
"https://api.github.com/repos/marcusquinn/aidevops/contents/VERSION" 2>/dev/null |
Expand All @@ -314,7 +331,8 @@ get_remote_version() {
return 0
fi
fi
# Fallback to raw (CDN-cached, may be up to 5 min stale)

# Last resort: raw.githubusercontent.com (CDN-cached, may be up to 5 min stale)
curl --proto '=https' -fsSL --max-time 10 \
"https://raw.githubusercontent.com/marcusquinn/aidevops/main/VERSION" 2>/dev/null |
tr -d '\n' || echo "unknown"
Expand Down Expand Up @@ -1064,6 +1082,27 @@ cmd_check() {
return 1
fi

# Ensure we're on the main branch (detached HEAD or stale branch blocks pull)
# Mirrors recovery logic from aidevops.sh cmd_update()
# See: https://github.com/marcusquinn/aidevops/issues/4142
local current_branch
current_branch=$(git -C "$INSTALL_DIR" branch --show-current 2>/dev/null || echo "")
if [[ "$current_branch" != "main" ]]; then
log_info "Not on main branch ($current_branch), switching..."
git -C "$INSTALL_DIR" checkout main --quiet 2>/dev/null ||
git -C "$INSTALL_DIR" checkout -b main origin/main --quiet 2>/dev/null || true

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

The || true at the end of this compound command can silently mask failures if both git checkout main and git checkout -b main origin/main fail. It's important to know if the branch switch was unsuccessful, especially after logging an log_info message about switching. Consider removing || true and adding an log_error if the operation fails. Additionally, for git commands, prefer relying on the --quiet flag to suppress stdout and avoid 2>/dev/null to ensure error messages on stderr remain visible for debugging.

Suggested change
git -C "$INSTALL_DIR" checkout main --quiet 2>/dev/null ||
git -C "$INSTALL_DIR" checkout -b main origin/main --quiet 2>/dev/null || true
git -C "$INSTALL_DIR" checkout main --quiet || \
git -C "$INSTALL_DIR" checkout -b main origin/main --quiet || \
log_error "Failed to switch to or create main branch"
References
  1. When using git commands (like 'init', 'remote') in shell scripts, use the '-q' flag to suppress standard output instead of '2>/dev/null', ensuring that error messages on stderr remain visible for debugging.

fi
Comment thread
coderabbitai[bot] marked this conversation as resolved.

# Clean up any working tree changes left by setup.sh or other processes
# (e.g., chmod on tracked scripts, scan results written to repo)
# This ensures git pull --ff-only won't be blocked by dirty files.
# See: https://github.com/marcusquinn/aidevops/issues/2286
if ! git -C "$INSTALL_DIR" diff --quiet 2>/dev/null || ! git -C "$INSTALL_DIR" diff --cached --quiet 2>/dev/null; then
log_info "Cleaning up stale working tree changes..."
git -C "$INSTALL_DIR" reset HEAD -- . 2>/dev/null || true
git -C "$INSTALL_DIR" checkout -- . 2>/dev/null || true

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

Similar to the previous comment, the || true here can silently hide failures during the working tree cleanup. While the intent is to ensure the pull is not blocked, if git reset or git checkout fail, it could indicate a more serious underlying issue that should be logged as an error. Additionally, avoid 2>/dev/null for git commands to ensure error messages on stderr remain visible for debugging, even if a --quiet flag is not applicable for these specific forms.

Suggested change
git -C "$INSTALL_DIR" reset HEAD -- . 2>/dev/null || true
git -C "$INSTALL_DIR" checkout -- . 2>/dev/null || true
git -C "$INSTALL_DIR" reset HEAD -- . || log_error "Failed to reset HEAD"
git -C "$INSTALL_DIR" checkout -- . || log_error "Failed to checkout working tree"
References
  1. When using git commands (like 'init', 'remote') in shell scripts, use the '-q' flag to suppress standard output instead of '2>/dev/null', ensuring that error messages on stderr remain visible for debugging.

fi

# Pull latest changes
if ! git -C "$INSTALL_DIR" fetch origin main --quiet 2>>"$LOG_FILE"; then
log_error "git fetch failed"
Expand All @@ -1076,13 +1115,22 @@ cmd_check() {
fi
Comment on lines +1102 to 1124

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

There's a logic error in the order of operations here. The git fetch command on line 1119 needs to run before the branch switching logic (lines 1102-1116). The git checkout -b main origin/main command on line 1110 depends on origin/main being fetched and up-to-date. Without fetching first, this command may fail if origin/main is not available locally or is stale, preventing the script from recovering from a detached HEAD or incorrect branch.

While this mirrors the logic in aidevops.sh, that implementation appears to have the same issue. Fixing it here would be a great improvement.

	# Pull latest changes
	if ! git -C "$INSTALL_DIR" fetch origin main --quiet 2>>"$LOG_FILE"; then
		log_error "git fetch failed"
		update_state "update" "$remote" "fetch_failed"
		run_freshness_checks
		return 1
	fi

	# Ensure we're on the main branch (detached HEAD or stale branch blocks pull)
	# Mirrors recovery logic from aidevops.sh cmd_update()
	# See: https://github.com/marcusquinn/aidevops/issues/4142
	local current_branch
	current_branch=$(git -C "$INSTALL_DIR" branch --show-current 2>/dev/null || echo "")
	if [[ "$current_branch" != "main" ]]; then
		log_info "Not on main branch ($current_branch), switching..."
		if ! git -C "$INSTALL_DIR" checkout main --quiet 2>>"$LOG_FILE" &&
			! git -C "$INSTALL_DIR" checkout -b main origin/main --quiet 2>>"$LOG_FILE"; then
			log_error "Failed to switch to main branch from '$current_branch' in $INSTALL_DIR"
			update_state "update" "$remote" "branch_switch_failed"
			run_freshness_checks
			return 1
		fi
	fi


if ! git -C "$INSTALL_DIR" pull --ff-only origin main --quiet 2>>"$LOG_FILE"; then
log_error "git pull --ff-only failed (local changes?)"
update_state "update" "$remote" "pull_failed"
check_skill_freshness
check_openclaw_freshness
check_tool_freshness
check_upstream_watch
return 1
# Fast-forward failed (diverged history or persistent dirty state).
# Since we just fetched origin/main, reset to it — the repo is managed
# by aidevops and should always track origin/main exactly.
# See: https://github.com/marcusquinn/aidevops/issues/2288
log_warn "git pull --ff-only failed — falling back to reset"
if git -C "$INSTALL_DIR" reset --hard origin/main --quiet 2>>"$LOG_FILE"; then
log_info "Reset to origin/main succeeded"
else
log_error "git reset --hard origin/main also failed"
update_state "update" "$remote" "pull_failed"
check_skill_freshness
check_openclaw_freshness
check_tool_freshness
check_upstream_watch
return 1
Comment on lines +1135 to +1138

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

The sequence of update_state, check_skill_freshness, check_openclaw_freshness, check_tool_freshness, and check_upstream_watch is duplicated here. This block of code is executed if git pull --ff-only fails and then git reset --hard origin/main also fails. This exact same block is present in the original if condition (lines 1080-1084 in the LEFT side). Consider refactoring to avoid this duplication, perhaps by encapsulating these calls in a function or moving them to a single failure point.

References
  1. In shell scripts, extract repeated logic into an internal helper function to improve maintainability. This applies even for standalone scripts where external source dependencies are avoided.

fi
fi

# Run setup.sh non-interactively to deploy agents
Expand Down Expand Up @@ -1112,6 +1160,12 @@ cmd_check() {
return 1
fi

# Clean up any working tree changes setup.sh may have introduced
# (e.g., chmod on tracked scripts, scan results written to repo)
# Prevents dirty tree from blocking the next update cycle.
# See: https://github.com/marcusquinn/aidevops/issues/2286
git -C "$INSTALL_DIR" checkout -- . 2>/dev/null || true

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

The || true at the end of this line can mask potential failures during the post-setup working tree cleanup. If git checkout -- . fails, it might indicate a problem that should be logged, even if it's a cleanup step. Additionally, avoid 2>/dev/null for git commands to ensure error messages on stderr remain visible for debugging, even if a --quiet flag is not applicable for this specific form.

Suggested change
git -C "$INSTALL_DIR" checkout -- . 2>/dev/null || true
git -C "$INSTALL_DIR" checkout -- . || log_error "Post-setup cleanup failed"
References
  1. When using git commands (like 'init', 'remote') in shell scripts, use the '-q' flag to suppress standard output instead of '2>/dev/null', ensuring that error messages on stderr remain visible for debugging.


# Run daily skill freshness check (24h gate)
check_skill_freshness

Expand Down
Loading