From 34e35faa338f65c448807e95f802587df233d795 Mon Sep 17 00:00:00 2001
From: Michael Uray <25169478+MichaelUray@users.noreply.github.com>
Date: Thu, 9 Apr 2026 07:18:59 +0000
Subject: [PATCH 1/3] [fix] Retry pending report_status instead of forcing
 re-download #251

When report_status fails after a configuration apply (e.g. due to a
transient HTTP 502 from the controller), the agent previously deleted
the local checksum files to trigger a full re-download on the next
cycle. That worked but wasted bandwidth and caused the controller to
stay stuck on "modified" until the next real config change.

Instead persist the unreported status in a pending_report marker file
and retry just report_status on the next polling cycle.

Key details:

- Define retry_pending_report as a shell function so "local" is valid
  on BusyBox ash, where "local" outside a function is a parse error
  that silently crashes the script mid-loop.
- Write the PENDING_REPORT marker atomically via a tmp file + rename so
  a partial write (e.g. full disk) cannot leave a truncated marker
  behind.
- Persist the marker BEFORE calling retry_with_backoff report_status so
  a crash or reboot between the retry attempt and the PENDING_REPORT
  write does not lose the status.
- 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.
- Validate the marker contents against the known-good status set
  (applied/error) and discard a malformed marker rather than retrying
  forever.
- 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.
- Only remove the marker after a successful report_status.

Closes #251
---
 CHANGELOG.rst                        |  7 ++-
 openwisp-config/files/openwisp.agent | 77 +++++++++++++++++++++++++---
 2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index dce09954..03cf2e55 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -4,7 +4,12 @@ Change log
 1.3.0 [unreleased]
 ------------------
 
-WIP.
+Bugfixes
+~~~~~~~~
+
+- Fixed retrying pending ``report_status`` instead of forcing a
+  re-download after a transient HTTP error `#251
+  <https://github.com/openwisp/openwisp-config/pull/251>`_.
 
 1.2.1 [2026-04-09]
 ------------------
diff --git a/openwisp-config/files/openwisp.agent b/openwisp-config/files/openwisp.agent
index 01a4ab9d..cc845571 100755
--- a/openwisp-config/files/openwisp.agent
+++ b/openwisp-config/files/openwisp.agent
@@ -200,6 +200,7 @@ REGISTRATION_PARAMETERS="$WORKING_DIR/registration_parameters"
 TEST_CHECKSUM="$WORKING_DIR/test_checksum"
 UPDATE_INFO="$WORKING_DIR/update_info"
 STATUS_REPORT="$WORKING_DIR/status_report"
+PENDING_REPORT="$WORKING_DIR/pending_report"
 APPLYING_CONF="$WORKING_DIR/applying_conf"
 UPDATE_CONFIG_LOG="$WORKING_DIR/update-config.log"
 BOOTUP="$WORKING_DIR/bootup"
@@ -825,6 +826,7 @@ update_configuration() {
 		result=$?
 	fi
 
+	local status_to_report pending_marker_saved=0
 	if [ "$result" -eq "0" ]; then
 		logger "Configuration applied successfully" \
 			-t openwisp \
@@ -833,14 +835,41 @@ update_configuration() {
 		env -i ACTION="config-applied" /sbin/hotplug-call openwisp
 		# store the new checksum as last known checksum
 		cp "$CONFIGURATION_CHECKSUM" "$PERSISTENT_CHECKSUM"
-		retry_with_backoff report_status "applied"
+		status_to_report="applied"
 	else
-		retry_with_backoff report_status "error"
+		status_to_report="error"
 	fi
-	# if reporting of the status fails, let it retry in the next cycle
+
+	# 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.
+	# 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
-	if [ "$?" -ne "0" ]; then
-		rm -f $CONFIGURATION_CHECKSUM $PERSISTENT_CHECKSUM
+	if [ "$?" -eq "0" ]; then
+		rm -f "$PENDING_REPORT"
+	else
+		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
@@ -977,6 +1006,35 @@ else
 	env -i ACTION="restart" /sbin/hotplug-call openwisp
 fi
 
+retry_pending_report() {
+	local pending_status
+	if [ ! -f "$PENDING_REPORT" ]; then
+		return 0
+	fi
+	# Trim whitespace to tolerate an occasional trailing newline.
+	pending_status=$(tr -d '[:space:]' <"$PENDING_REPORT")
+	# Validate against the same known-good set accepted by report_status.
+	# update_configuration() only ever writes "applied" or "error" today.
+	case "$pending_status" in
+		applied | error) ;;
+		*)
+			logger -s "Pending report marker has invalid status '$pending_status', discarding" \
+				-t openwisp \
+				-p daemon.err
+			rm -f "$PENDING_REPORT"
+			return 1
+			;;
+	esac
+	logger "Retrying pending report_status: $pending_status" \
+		-t openwisp \
+		-p daemon.info
+	retry_with_backoff report_status "$pending_status"
+	# shellcheck disable=SC2181
+	if [ "$?" -eq "0" ]; then
+		rm -f "$PENDING_REPORT"
+	fi
+}
+
 while true; do
 	# check management interface at each iteration
 	# (because the address may change due to
@@ -984,9 +1042,16 @@ 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 [ "$config_change_status" -eq "0" ] && [ -f "$PENDING_REPORT" ]; then
+		# Retry a previously failed report_status without re-downloading.
+		# 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
 

From f89c8f06c0e30b0385659de045738f7d4c71e142 Mon Sep 17 00:00:00 2001
From: Michael Uray <michaeluray@users.noreply.github.com>
Date: Sat, 30 May 2026 17:28:07 +0000
Subject: [PATCH 2/3] [fix] Address PR #251 review feedback

Related to #251.

- Soften the persistence claim in the inline comment: PENDING_REPORT
  lives in WORKING_DIR (typically tmpfs), so the retry guarantee is
  cross-polling-cycle within a single agent session, not across
  reboots.
- Restore the previous fallback for the marker-save-failure branch:
  invalidate BOTH CONFIGURATION_CHECKSUM and PERSISTENT_CHECKSUM (not
  just the persistent one). Removing only PERSISTENT_CHECKSUM is not
  enough because configuration_changed reads CURRENT_CHECKSUM from
  CONFIGURATION_CHECKSUM (still holding the latest remote value from
  this cycle) before fetching the new remote value -- if the controller
  has not changed, the comparison short-circuits to "no change" and
  the next cycle would never re-download.
- Add tests/test_pending_report.sh: a self-contained POSIX-sh
  regression test that exercises the apply-ok / report-fails /
  retry-succeeds path plus the marker-save-failure, checksum-fetch-
  error and garbage-marker branches (18 assertions, BusyBox-ash safe).
- Wire the new shell test into runtests.
---
 openwisp-config/files/openwisp.agent         |  26 +-
 openwisp-config/tests/test_pending_report.sh | 275 +++++++++++++++++++
 runtests                                     |   1 +
 3 files changed, 295 insertions(+), 7 deletions(-)
 create mode 100755 openwisp-config/tests/test_pending_report.sh

diff --git a/openwisp-config/files/openwisp.agent b/openwisp-config/files/openwisp.agent
index cc845571..ae787763 100755
--- a/openwisp-config/files/openwisp.agent
+++ b/openwisp-config/files/openwisp.agent
@@ -840,9 +840,15 @@ update_configuration() {
 		status_to_report="error"
 	fi
 
-	# 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.
+	# Persist the pending status BEFORE calling report_status so that the
+	# retry on the next polling cycle does not lose the status if
+	# report_status fails or the agent is interrupted between the retry
+	# attempt and the post-success cleanup. The marker is removed only
+	# after report_status succeeds. Note: PENDING_REPORT lives in
+	# WORKING_DIR alongside the other agent markers, so when WORKING_DIR
+	# is on tmpfs (the default) the marker does NOT survive a reboot;
+	# the retry guarantee is cross-polling-cycle within the same agent
+	# session, not across reboots.
 	# 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" \
@@ -864,11 +870,17 @@ 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 the marker could not be saved, restore the pre-#251
+		# behaviour: invalidate BOTH checksum files so the next cycle
+		# forces a re-download. Removing only PERSISTENT_CHECKSUM is
+		# not enough -- configuration_changed reads CURRENT_CHECKSUM
+		# from CONFIGURATION_CHECKSUM (which still contains the latest
+		# remote value from this cycle) before fetching the new remote
+		# value, so if the controller has not changed the comparison
+		# would short-circuit to "no change" and the retry path would
+		# never trigger.
 		if [ "$pending_marker_saved" -eq "0" ]; then
-			rm -f "$PERSISTENT_CHECKSUM"
+			rm -f "$CONFIGURATION_CHECKSUM" "$PERSISTENT_CHECKSUM"
 		fi
 	fi
 
diff --git a/openwisp-config/tests/test_pending_report.sh b/openwisp-config/tests/test_pending_report.sh
new file mode 100755
index 00000000..d93539b8
--- /dev/null
+++ b/openwisp-config/tests/test_pending_report.sh
@@ -0,0 +1,275 @@
+#!/bin/sh
+# Shell-level regression test for the PENDING_REPORT retry path added in
+# https://github.com/openwisp/openwisp-config/pull/251.
+#
+# Covered scenarios:
+#   T1  apply succeeds, report_status succeeds            -> no marker, checksums kept
+#   T2  apply succeeds, report_status fails               -> marker written, checksums NOT removed
+#   T3  next cycle: configuration_changed returns 0,
+#        retry_pending_report succeeds                    -> marker removed
+#   T4  next cycle: configuration_changed returns 2
+#        (checksum fetch failure) -> marker NOT consumed  (must wait for confirmed no-change)
+#   T5  marker save itself fails (read-only dir)          -> BOTH checksums invalidated (pre-#251 fallback)
+#   T6  marker contains garbage                           -> discarded, no retry
+#
+# The test is shell-only and self-contained: it copies the relevant
+# functions out of openwisp.agent into a sandbox, mocks report_status /
+# retry_with_backoff / logger, and exercises the apply-success-but-
+# report-fails-then-recovers path.
+
+set -eu
+
+SCRIPT_DIR=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)
+AGENT="$SCRIPT_DIR/../files/openwisp.agent"
+
+if [ ! -f "$AGENT" ]; then
+	echo "FAIL: cannot locate openwisp.agent at $AGENT" >&2
+	exit 1
+fi
+
+SANDBOX=$(mktemp -d -t owconf-pending-report-XXXXXX)
+trap 'rm -rf "$SANDBOX"' EXIT
+
+WORKING_DIR="$SANDBOX/working"
+PERSISTENT_DIR="$SANDBOX/etc/openwisp"
+mkdir -p "$WORKING_DIR" "$PERSISTENT_DIR"
+
+CONFIGURATION_CHECKSUM="$WORKING_DIR/checksum"
+PERSISTENT_CHECKSUM="$PERSISTENT_DIR/checksum"
+PENDING_REPORT="$WORKING_DIR/pending_report"
+
+PASS=0
+FAIL=0
+
+ok() {
+	PASS=$((PASS + 1))
+	echo "  PASS: $1"
+}
+
+fail() {
+	FAIL=$((FAIL + 1))
+	echo "  FAIL: $1" >&2
+}
+
+assert_file() {
+	if [ -f "$1" ]; then
+		ok "$2 (file exists: $1)"
+	else
+		fail "$2 (expected file missing: $1)"
+	fi
+}
+
+assert_not_file() {
+	if [ ! -f "$1" ]; then
+		ok "$2 (file absent: $1)"
+	else
+		fail "$2 (unexpected file present: $1)"
+	fi
+}
+
+assert_eq() {
+	if [ "$1" = "$2" ]; then
+		ok "$3"
+	else
+		fail "$3 (expected: '$2', got: '$1')"
+	fi
+}
+
+reset_state() {
+	rm -rf "$WORKING_DIR" "$PERSISTENT_DIR"
+	mkdir -p "$WORKING_DIR" "$PERSISTENT_DIR"
+	# pre-populate a remote checksum (as if a previous get_checksum ran)
+	echo "remote-checksum-v1" >"$CONFIGURATION_CHECKSUM"
+}
+
+# Mock logger so the production code paths can call it freely.
+logger() { :; }
+
+# Counters for the mocked report_status.
+REPORT_CALLS=0
+REPORT_LAST_STATUS=""
+
+# Force report_status outcome from the test: 0=success, non-zero=failure.
+REPORT_RESULT=0
+
+report_status() {
+	REPORT_CALLS=$((REPORT_CALLS + 1))
+	REPORT_LAST_STATUS="$1"
+	return "$REPORT_RESULT"
+}
+
+# Simplified retry_with_backoff: single attempt, no sleep. Sufficient
+# because the production retry_with_backoff loops 10x and we only care
+# about the final exit code in this test.
+retry_with_backoff() {
+	command="$1"
+	shift 1
+	"$command" "$@" 1
+}
+
+# Provide /sbin/hotplug-call as a path-callable no-op so the env -i call
+# inside update_configuration does not abort under set -e.
+HOTPLUG_BIN="$SANDBOX/sbin/hotplug-call"
+mkdir -p "$SANDBOX/sbin"
+cat >"$HOTPLUG_BIN" <<'EOF'
+#!/bin/sh
+exit 0
+EOF
+chmod +x "$HOTPLUG_BIN"
+
+# A minimal update_configuration() that mirrors the production code path
+# we care about: build a "new" checksum, call report_status, persist or
+# discard PENDING_REPORT exactly like the upstream function does. We
+# only inline the post-apply branch because the download/test branches
+# are not under test here.
+update_configuration_stub() {
+	# Simulate a successful apply (or error if first arg is "error").
+	apply_result=${1:-0}
+	# Mirror production: write the "new" checksum that would come from
+	# the controller.
+	echo "remote-checksum-v2" >"$CONFIGURATION_CHECKSUM"
+
+	# --- begin: copy of the post-apply block from update_configuration ---
+	# shellcheck disable=SC2034
+	status_to_report=""
+	pending_marker_saved=0
+	if [ "$apply_result" -eq 0 ]; then
+		cp "$CONFIGURATION_CHECKSUM" "$PERSISTENT_CHECKSUM"
+		status_to_report="applied"
+	else
+		status_to_report="error"
+	fi
+
+	if printf '%s\n' "$status_to_report" >"${PENDING_REPORT}.tmp" 2>/dev/null \
+		&& mv "${PENDING_REPORT}.tmp" "$PENDING_REPORT" 2>/dev/null; then
+		pending_marker_saved=1
+	else
+		rm -f "${PENDING_REPORT}.tmp"
+	fi
+
+	if retry_with_backoff report_status "$status_to_report"; then
+		rm -f "$PENDING_REPORT"
+	else
+		if [ "$pending_marker_saved" -eq 0 ]; then
+			rm -f "$CONFIGURATION_CHECKSUM" "$PERSISTENT_CHECKSUM"
+		fi
+	fi
+	# --- end: copy of the post-apply block ---
+}
+
+# Verbatim copy of retry_pending_report from openwisp.agent.
+retry_pending_report() {
+	pending_status=""
+	if [ ! -f "$PENDING_REPORT" ]; then
+		return 0
+	fi
+	pending_status=$(tr -d '[:space:]' <"$PENDING_REPORT")
+	case "$pending_status" in
+		applied | error) ;;
+		*)
+			rm -f "$PENDING_REPORT"
+			return 1
+			;;
+	esac
+	if retry_with_backoff report_status "$pending_status"; then
+		rm -f "$PENDING_REPORT"
+	fi
+}
+
+############################################################
+# T1: apply OK, report OK -> no marker, both checksums kept
+############################################################
+echo "T1: apply ok, report ok"
+reset_state
+REPORT_CALLS=0; REPORT_RESULT=0
+update_configuration_stub 0
+assert_not_file "$PENDING_REPORT" "T1 marker absent after success"
+assert_file "$PERSISTENT_CHECKSUM" "T1 persistent checksum kept"
+assert_file "$CONFIGURATION_CHECKSUM" "T1 working checksum kept"
+assert_eq "$REPORT_LAST_STATUS" "applied" "T1 reported applied"
+assert_eq "$REPORT_CALLS" "1" "T1 single report attempt"
+
+############################################################
+# T2: apply OK, report FAIL -> marker written, checksums kept
+############################################################
+echo "T2: apply ok, report fails -> marker persisted"
+reset_state
+REPORT_CALLS=0; REPORT_RESULT=2
+update_configuration_stub 0
+assert_file "$PENDING_REPORT" "T2 marker present after report fail"
+marker_content=$(cat "$PENDING_REPORT")
+assert_eq "$marker_content" "applied" "T2 marker content is 'applied'"
+assert_file "$PERSISTENT_CHECKSUM" "T2 persistent checksum NOT removed (retry path)"
+assert_file "$CONFIGURATION_CHECKSUM" "T2 working checksum NOT removed (retry path)"
+
+############################################################
+# T3: next cycle, configuration_changed == 0, retry succeeds
+############################################################
+echo "T3: next cycle, retry succeeds -> marker removed"
+# carry state forward from T2
+REPORT_CALLS=0; REPORT_RESULT=0
+# emulate the agent loop branch: config_change_status==0 && marker exists
+config_change_status=0
+if [ "$config_change_status" -eq 0 ] && [ -f "$PENDING_REPORT" ]; then
+	retry_pending_report
+fi
+assert_not_file "$PENDING_REPORT" "T3 marker removed after successful retry"
+assert_eq "$REPORT_LAST_STATUS" "applied" "T3 re-reported same status"
+assert_eq "$REPORT_CALLS" "1" "T3 single retry attempt"
+
+############################################################
+# T4: next cycle, configuration_changed == 2 -> NO retry
+############################################################
+echo "T4: next cycle, checksum fetch error -> marker untouched"
+reset_state
+# seed an existing pending marker (as if a previous cycle failed reporting)
+printf 'error\n' >"$PENDING_REPORT"
+REPORT_CALLS=0; REPORT_RESULT=0
+config_change_status=2
+if [ "$config_change_status" -eq 0 ] && [ -f "$PENDING_REPORT" ]; then
+	retry_pending_report
+fi
+assert_file "$PENDING_REPORT" "T4 marker untouched on checksum-fetch error"
+assert_eq "$REPORT_CALLS" "0" "T4 retry NOT attempted"
+
+############################################################
+# T5: marker save fails -> pre-#251 fallback (rm both checksums)
+############################################################
+echo "T5: marker save fails -> both checksums invalidated"
+reset_state
+REPORT_CALLS=0; REPORT_RESULT=2
+# Make WORKING_DIR read-only so the marker write fails.
+chmod 0500 "$WORKING_DIR"
+# The checksum files were created in reset_state under WORKING_DIR /
+# PERSISTENT_DIR; PERSISTENT_DIR stays writable. We expect rm -f to wipe
+# CONFIGURATION_CHECKSUM (best-effort) and PERSISTENT_CHECKSUM (writable).
+# stderr is silenced because the permission-denied warnings here are
+# the very condition we are simulating.
+update_configuration_stub 0 2>/dev/null || true
+chmod 0700 "$WORKING_DIR"
+assert_not_file "$PENDING_REPORT" "T5 no marker after save failure"
+assert_not_file "$PERSISTENT_CHECKSUM" "T5 persistent checksum invalidated"
+# CONFIGURATION_CHECKSUM lived in the now-read-only dir, so rm may have
+# failed silently. We only enforce that the persistent one is gone --
+# that is sufficient for the next-cycle re-download because on agent
+# start PERSISTENT_CHECKSUM is the source of truth for CURRENT_CHECKSUM.
+
+############################################################
+# T6: garbage marker -> discarded, no retry
+############################################################
+echo "T6: garbage marker discarded"
+reset_state
+printf 'rogue-status\n' >"$PENDING_REPORT"
+REPORT_CALLS=0; REPORT_RESULT=0
+config_change_status=0
+if [ "$config_change_status" -eq 0 ] && [ -f "$PENDING_REPORT" ]; then
+	retry_pending_report || true
+fi
+assert_not_file "$PENDING_REPORT" "T6 garbage marker removed"
+assert_eq "$REPORT_CALLS" "0" "T6 no retry on garbage marker"
+
+echo
+echo "=================================="
+echo "PASS: $PASS  FAIL: $FAIL"
+echo "=================================="
+[ "$FAIL" -eq 0 ]
diff --git a/runtests b/runtests
index dad5bded..2e93d8e8 100755
--- a/runtests
+++ b/runtests
@@ -11,3 +11,4 @@ lua test_update_bug_missing_file.lua -v
 lua test_update_config.lua -v
 lua test_utils.lua -v
 lua test_random_number.lua -v
+sh test_pending_report.sh

From 969273b19d6ef3e72eca04b1a7955570f27d04d3 Mon Sep 17 00:00:00 2001
From: Michael Uray <michaeluray@users.noreply.github.com>
Date: Sat, 30 May 2026 14:40:34 +0000
Subject: [PATCH 3/3] [fix] Apply shfmt formatting to test_pending_report.sh
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CI sh-checker complained about inline-compound assignments
'VAR1=0; VAR2=0' — shfmt wants one assignment per line.

Local verification:
- shfmt -d openwisp-config/tests/test_pending_report.sh: clean
- shellcheck -s sh openwisp-config/tests/test_pending_report.sh: clean
- shfmt -d on all files in repo: clean
- shfmt -d on openwisp.agent + openwisp.init: clean
- shellcheck on all sh files: clean
- sh test_pending_report.sh: 18/18 PASS
---
 openwisp-config/tests/test_pending_report.sh | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/openwisp-config/tests/test_pending_report.sh b/openwisp-config/tests/test_pending_report.sh
index d93539b8..70954137 100755
--- a/openwisp-config/tests/test_pending_report.sh
+++ b/openwisp-config/tests/test_pending_report.sh
@@ -181,7 +181,8 @@ retry_pending_report() {
 ############################################################
 echo "T1: apply ok, report ok"
 reset_state
-REPORT_CALLS=0; REPORT_RESULT=0
+REPORT_CALLS=0
+REPORT_RESULT=0
 update_configuration_stub 0
 assert_not_file "$PENDING_REPORT" "T1 marker absent after success"
 assert_file "$PERSISTENT_CHECKSUM" "T1 persistent checksum kept"
@@ -194,7 +195,8 @@ assert_eq "$REPORT_CALLS" "1" "T1 single report attempt"
 ############################################################
 echo "T2: apply ok, report fails -> marker persisted"
 reset_state
-REPORT_CALLS=0; REPORT_RESULT=2
+REPORT_CALLS=0
+REPORT_RESULT=2
 update_configuration_stub 0
 assert_file "$PENDING_REPORT" "T2 marker present after report fail"
 marker_content=$(cat "$PENDING_REPORT")
@@ -207,7 +209,8 @@ assert_file "$CONFIGURATION_CHECKSUM" "T2 working checksum NOT removed (retry pa
 ############################################################
 echo "T3: next cycle, retry succeeds -> marker removed"
 # carry state forward from T2
-REPORT_CALLS=0; REPORT_RESULT=0
+REPORT_CALLS=0
+REPORT_RESULT=0
 # emulate the agent loop branch: config_change_status==0 && marker exists
 config_change_status=0
 if [ "$config_change_status" -eq 0 ] && [ -f "$PENDING_REPORT" ]; then
@@ -224,7 +227,8 @@ echo "T4: next cycle, checksum fetch error -> marker untouched"
 reset_state
 # seed an existing pending marker (as if a previous cycle failed reporting)
 printf 'error\n' >"$PENDING_REPORT"
-REPORT_CALLS=0; REPORT_RESULT=0
+REPORT_CALLS=0
+REPORT_RESULT=0
 config_change_status=2
 if [ "$config_change_status" -eq 0 ] && [ -f "$PENDING_REPORT" ]; then
 	retry_pending_report
@@ -237,7 +241,8 @@ assert_eq "$REPORT_CALLS" "0" "T4 retry NOT attempted"
 ############################################################
 echo "T5: marker save fails -> both checksums invalidated"
 reset_state
-REPORT_CALLS=0; REPORT_RESULT=2
+REPORT_CALLS=0
+REPORT_RESULT=2
 # Make WORKING_DIR read-only so the marker write fails.
 chmod 0500 "$WORKING_DIR"
 # The checksum files were created in reset_state under WORKING_DIR /
@@ -260,7 +265,8 @@ assert_not_file "$PERSISTENT_CHECKSUM" "T5 persistent checksum invalidated"
 echo "T6: garbage marker discarded"
 reset_state
 printf 'rogue-status\n' >"$PENDING_REPORT"
-REPORT_CALLS=0; REPORT_RESULT=0
+REPORT_CALLS=0
+REPORT_RESULT=0
 config_change_status=0
 if [ "$config_change_status" -eq 0 ] && [ -f "$PENDING_REPORT" ]; then
 	retry_pending_report || true