From a1d67552239e7c7c4322f432109746dc11cf31e1 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Mon, 31 Aug 2026 20:26:34 -0400 Subject: [PATCH] fix: report partial plugin mutations --- lib/desired-state-reconciler.sh | 11 ++++- lib/plugin-upgrade.sh | 10 ++++ lib/wordpress.sh | 2 + lib/wp-codebox.sh | 6 ++- tests/plugin-upgrade-bounds.sh | 86 +++++++++++++++++++++++++++++---- 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/lib/desired-state-reconciler.sh b/lib/desired-state-reconciler.sh index 22b3fca..7f7cd6b 100644 --- a/lib/desired-state-reconciler.sh +++ b/lib/desired-state-reconciler.sh @@ -224,6 +224,10 @@ reconciler_apply_plan() { else step_status=$? status="${PLUGIN_UPDATE_EXIT_PARTIAL:-75}" + if [ "${RECONCILER_STEP_CHANGED:-false}" = true ]; then + RECONCILER_CHANGED_RECORDS+=("$record") + log "[desired-state] record=$record operation=$operation apply=partial changed=true replay=$replay" + fi if [ "$step_status" -eq 124 ]; then warn "[desired-state] record=$record operation=$operation apply=timeout timeout=${timeout}s replay=$replay" else @@ -245,6 +249,9 @@ reconciler_run_bounded() { local record="$1" operation="$2" timeout="$3" command="$4" phase="${5:-apply}" local total="${DESIRED_STATE_TOTAL_TIMEOUT_SECONDS:-480}" now elapsed remaining local started status=0 temp_dir outcome_file pid pgid elapsed restore_monitor=false + # Each bounded record owns its result; a prior record must not leak a change + # into an aggregate or phase deadline that runs no current child outcome. + RECONCILER_STEP_CHANGED=false case "$timeout" in ''|*[!0-9]*|0) timeout=120 ;; esac case "$total" in ''|*[!0-9]*|0) total=480 ;; esac now="$(date +%s)"; elapsed=$((now - RECONCILER_STARTED_AT)); remaining=$((total - elapsed)) @@ -278,7 +285,9 @@ reconciler_run_bounded() { done if wait "$pid"; then status=0; else status=$?; fi RECONCILER_STEP_CHANGED=false - if [ "$status" -eq 0 ] && [ -f "$outcome_file" ]; then + # The child writes its mutation outcome before returning its own status. + # Preserve a completed mutation even when a later bounded updater phase fails. + if [ -f "$outcome_file" ]; then case "$(<"$outcome_file")" in changed=true) RECONCILER_STEP_CHANGED=true ;; esac fi rm -rf "$temp_dir" diff --git a/lib/plugin-upgrade.sh b/lib/plugin-upgrade.sh index 393d14e..063b7ef 100644 --- a/lib/plugin-upgrade.sh +++ b/lib/plugin-upgrade.sh @@ -195,6 +195,9 @@ plugin_update_execute() { # Read by updater modules sourced into the same shell. # shellcheck disable=SC2034 PLUGIN_UPDATE_ACTIVE=true + # Updaters set this only after their managed plugin state changes. + # shellcheck disable=SC2034 + PLUGIN_UPDATE_MUTATED=false if "$@"; then update_status=0; else update_status=$?; fi # shellcheck disable=SC2034 PLUGIN_UPDATE_ACTIVE=false @@ -205,6 +208,13 @@ plugin_update_execute() { plugin_update_record_failure "$slug" "$( [ "$update_status" -eq 124 ] && printf timeout || printf command-failure )" "$update_status" fi + # Preserve an updater-reported mutation for reconciliation, including a + # later bounded-step failure. + if [ "$PLUGIN_UPDATE_MUTATED" = true ] \ + && declare -F reconciler_adapter_changed >/dev/null 2>&1; then + reconciler_adapter_changed + fi + if [ "$update_status" -eq 0 ]; then log "[$slug] apply-terminal=complete version=${after_version:-unknown}" return 0 diff --git a/lib/wordpress.sh b/lib/wordpress.sh index 28d2a88..daf8f4b 100644 --- a/lib/wordpress.sh +++ b/lib/wordpress.sh @@ -122,6 +122,7 @@ update_plugin_to_latest_tag() { if [ ! -d "$plugin_dir" ]; then log "Plugin $slug missing — installing before tag checkout..." install_plugin "$slug" "$repo_url" + [ ! -d "$plugin_dir" ] || PLUGIN_UPDATE_MUTATED=true fi if [ ! -d "$plugin_dir/.git" ]; then @@ -193,6 +194,7 @@ update_plugin_to_latest_tag() { return "$phase_status" fi UPDATED_ITEMS+=("$slug $latest_tag") + PLUGIN_UPDATE_MUTATED=true fi install_plugin_dependencies_bounded "$slug" "$plugin_dir" || return $? diff --git a/lib/wp-codebox.sh b/lib/wp-codebox.sh index 2c5ec63..364110b 100644 --- a/lib/wp-codebox.sh +++ b/lib/wp-codebox.sh @@ -142,7 +142,9 @@ update_wp_codebox_plugin_subtree() { local phase_status=$? return "$phase_status" fi - plugin_update_run_phase wp-codebox ownership-normalization fix_ownership "$plugin_dir" || return $? - + # Record the completed subtree write before bounded ownership normalization, + # so a later timeout still exposes the mutation to the reconciler. UPDATED_ITEMS+=("wp-codebox $latest_tag") + PLUGIN_UPDATE_MUTATED=true + plugin_update_run_phase wp-codebox ownership-normalization fix_ownership "$plugin_dir" || return $? } diff --git a/tests/plugin-upgrade-bounds.sh b/tests/plugin-upgrade-bounds.sh index 2f4dff8..02991d0 100755 --- a/tests/plugin-upgrade-bounds.sh +++ b/tests/plugin-upgrade-bounds.sh @@ -115,17 +115,18 @@ PY [ ! -e "$PLUGIN/.wp-coding-agents-releases" ] || fail "copied DMC was converted to .wp-coding-agents-releases" case "$LOG" in *"installed-after version=1.0.0 active=yes"*"terminal=partial-failure"*) : ;; *) fail "partial terminal verification evidence missing" ;; esac -# Reconciliation reports the records that completed before a bounded later step -# timed out, making the partial result safe to replay without claiming a change -# when an idempotent apply was already converged. +# Reconciliation preserves a mutation when the same plugin updater later times +# out, without classifying that record as completed. LOG="" PLUGIN_UPDATE_PHASE_TIMEOUT_SECONDS=1 PLUGIN_UPDATE_STARTED_AT="$(date +%s)" -reconciler_fixture_complete() { return 0; } -reconciler_fixture_timeout() { plugin_update_run_phase fixture reconciler-timeout "$hung"; } +reconciler_fixture_mutating_timeout() { + PLUGIN_UPDATE_MUTATED=true + plugin_update_run_phase data-machine-code reconciler-timeout "$hung" +} +reconciler_fixture_apply() { plugin_update_execute data-machine-code reconciler_fixture_mutating_timeout; } reconciler_plan_reset -reconciler_plan_add plugins.data-machine plugins.reconcile.data-machine reconciler_fixture_complete -reconciler_plan_add plugins.data-machine-code plugins.reconcile.data-machine-code reconciler_fixture_timeout +reconciler_plan_add plugins.data-machine-code plugins.reconcile.data-machine-code reconciler_fixture_apply if reconciler_apply_plan; then fail "generic reconciler completed a timed-out plan" else @@ -133,11 +134,78 @@ else fi [ "$status" -eq "$PLUGIN_UPDATE_EXIT_PARTIAL" ] || fail "generic reconciler did not return partial status" reconciler_print_partial_evidence +[ "${RECONCILER_CHANGED_RECORDS[*]}" = "plugins.data-machine-code" ] || fail "desired-state reconciler dropped or misreported the partial mutation" +[ "${#RECONCILER_COMPLETED_RECORDS[@]}" -eq 0 ] || fail "desired-state reconciler misclassified a partial mutation as completed" case "$LOG" in - *'record=plugins.data-machine operation=plugins.reconcile.data-machine apply=start'*'record=plugins.data-machine-code operation=plugins.reconcile.data-machine-code apply=start'*'DESIRED_STATE_COMPLETED_RECORDS=plugins.data-machine'*) : ;; - *) fail "desired-state reconciler omitted timeout or completed-record evidence" ;; + *'record=plugins.data-machine-code operation=plugins.reconcile.data-machine-code apply=start'*'record=plugins.data-machine-code operation=plugins.reconcile.data-machine-code apply=partial changed=true'*'DESIRED_STATE_CHANGED_RECORDS=plugins.data-machine-code'*) : ;; + *) fail "desired-state reconciler omitted partial mutation evidence" ;; esac +# A later aggregate deadline must not inherit the changed outcome from a +# completed prior record when its own child never runs. +LOG="" +reconciler_fixture_first_changed() { reconciler_adapter_changed; } +reconciler_fixture_second_must_not_run() { : > "$TMP/aggregate-second-ran"; } +date() { + case "${record:-}" in + plugins.aggregate-first) printf '100\n' ;; + plugins.aggregate-second) printf '101\n' ;; + *) command date "$@" ;; + esac +} +reconciler_plan_reset +reconciler_plan_add plugins.aggregate-first plugins.reconcile.aggregate-first reconciler_fixture_first_changed +reconciler_plan_add plugins.aggregate-second plugins.reconcile.aggregate-second reconciler_fixture_second_must_not_run +DESIRED_STATE_TOTAL_TIMEOUT_SECONDS=1 +RECONCILER_STARTED_AT=100 +if reconciler_apply_plan; then + unset -f date + fail "aggregate deadline plan completed" +else + status=$? +fi +unset -f date +unset DESIRED_STATE_TOTAL_TIMEOUT_SECONDS +[ "$status" -eq "$PLUGIN_UPDATE_EXIT_PARTIAL" ] || fail "aggregate deadline did not return partial status" +[ ! -e "$TMP/aggregate-second-ran" ] || fail "aggregate deadline ran the second record" +[ "${RECONCILER_CHANGED_RECORDS[*]}" = "plugins.aggregate-first" ] || fail "aggregate deadline inherited a stale changed record" +[ "${RECONCILER_COMPLETED_RECORDS[*]}" = "plugins.aggregate-first" ] || fail "aggregate deadline completed the second record" +reconciler_print_partial_evidence +case "$LOG" in + *'record=plugins.aggregate-second operation=plugins.reconcile.aggregate-second apply=timeout'*'DESIRED_STATE_CHANGED_RECORDS=plugins.aggregate-first'*) : ;; + *) fail "desired-state reconciler omitted aggregate deadline evidence" ;; +esac + +# A per-step child deadline likewise must not inherit a changed first record. +# This adapter hangs directly so the reconciler, rather than an inner plugin +# updater, owns the timeout and process-group cleanup. +LOG="" +reconciler_fixture_step_first_changed() { reconciler_adapter_changed; } +reconciler_fixture_step_hang() { : > "$TMP/step-hang-started"; sleep 30; } +reconciler_plan_reset +reconciler_plan_add plugins.step-first plugins.reconcile.step-first reconciler_fixture_step_first_changed +reconciler_plan_add plugins.step-second plugins.reconcile.step-second reconciler_fixture_step_hang +DESIRED_STATE_STEP_TIMEOUT_SECONDS=1 +DESIRED_STATE_TOTAL_TIMEOUT_SECONDS=20 +RECONCILER_STARTED_AT="$(date +%s)" +if reconciler_apply_plan; then + fail "per-step deadline plan completed" +else + status=$? +fi +unset DESIRED_STATE_STEP_TIMEOUT_SECONDS +unset DESIRED_STATE_TOTAL_TIMEOUT_SECONDS +[ "$status" -eq "$PLUGIN_UPDATE_EXIT_PARTIAL" ] || fail "per-step deadline did not return partial status" +[ -e "$TMP/step-hang-started" ] || fail "per-step deadline never started its child" +[ "${RECONCILER_CHANGED_RECORDS[*]}" = "plugins.step-first" ] || fail "per-step deadline inherited a stale changed record" +[ "${RECONCILER_COMPLETED_RECORDS[*]}" = "plugins.step-first" ] || fail "per-step deadline completed the hanging record" +reconciler_print_partial_evidence +case "$LOG" in + *'record=plugins.step-second operation=plugins.reconcile.step-second apply=deadline-exceeded'*'record=plugins.step-second operation=plugins.reconcile.step-second apply=timeout timeout=1s'*'DESIRED_STATE_CHANGED_RECORDS=plugins.step-first'*) : ;; + *) fail "desired-state reconciler omitted per-step deadline evidence" ;; +esac +case "$LOG" in *'aggregate_timeout='*) fail "per-step deadline used aggregate preflight" ;; esac + # Normalized profiles carry installation shape, never credential runtime input. SITE_PATH="$TMP/site" LOCAL_MODE=true