Address CodeRabbit review feedback on openwisp#251

- Write PENDING_REPORT atomically via tmp file + rename so a partial
  write (e.g. full disk) cannot leave a truncated marker behind.
- Fall back to invalidating the persistent checksum when the pending
  marker cannot be saved, preserving the previous force re-download
  behaviour as a safety net.
- Only retry a pending report after configuration_changed() returns 0
  (confirmed no-change). Non-0/non-1 exit codes indicate checksum fetch
  failures and the cached applied/error status may no longer reflect the
  controller's current desired configuration.

Author: MichaelUray

openwisp-config/files/openwisp.agent

@@ -826,7 +826,7 @@ update_configuration() {
 		result=$?
 	fi
 
-	local status_to_report
+	local status_to_report pending_marker_saved=0
 	if [ "$result" -eq "0" ]; then
 		logger "Configuration applied successfully" \
 			-t openwisp \
@@ -843,7 +843,18 @@ update_configuration() {
 	# Persist the pending status BEFORE calling report_status so that a
 	# crash or reboot between the retry attempt and success does not lose
 	# the status. The marker is removed only after report_status succeeds.
-	printf '%s\n' "$status_to_report" >"$PENDING_REPORT"
+	# Write atomically via a tmp file + rename so a partial write (e.g.
+	# full disk) cannot leave a truncated marker behind.
+	if printf '%s\n' "$status_to_report" >"${PENDING_REPORT}.tmp" \
+		&& mv "${PENDING_REPORT}.tmp" "$PENDING_REPORT"; then
+		pending_marker_saved=1
+	else
+		rm -f "${PENDING_REPORT}.tmp"
+		logger -s "Failed to persist pending report marker, " \
+			"falling back to checksum invalidation on failure" \
+			-t openwisp \
+			-p daemon.warning
+	fi
 
 	retry_with_backoff report_status "$status_to_report"
 	# shellcheck disable=SC2181
@@ -853,6 +864,12 @@ update_configuration() {
 		logger -s "report_status failed, will retry in the next cycle" \
 			-t openwisp \
 			-p daemon.warning
+		# If the marker could not be saved, invalidate the checksum so
+		# the next cycle forces a re-download. This preserves the
+		# previous behaviour whenever the retry marker is unavailable.
+		if [ "$pending_marker_saved" -eq "0" ]; then
+			rm -f "$PERSISTENT_CHECKSUM"
+		fi
 	fi
 
 	rm $APPLYING_CONF
@@ -1025,13 +1042,15 @@ while true; do
 	discover_management_ip
 
 	configuration_changed
+	config_change_status=$?
 
-	if [ "$?" -eq "1" ]; then
+	if [ "$config_change_status" -eq "1" ]; then
 		update_configuration
-	elif [ -f "$PENDING_REPORT" ]; then
+	elif [ "$config_change_status" -eq "0" ] && [ -f "$PENDING_REPORT" ]; then
 		# Retry a previously failed report_status without re-downloading.
-		# This handles the case where the config was applied successfully
-		# but report_status failed due to a transient server error.
+		# Only do this after a confirmed no-change result (exit 0); other
+		# exit codes indicate checksum fetch failures and the pending
+		# applied/error status could no longer be accurate.
 		retry_pending_report
 	fi
 	env -i ACTION="end-of-cycle" /sbin/hotplug-call openwisp