Skip to content

fix: propagate update failures without duplicate execution - #343

Closed
ragnarlotus wants to merge 14 commits into
BassT23:developfrom
ragnarlotus:fix/error-propagation
Closed

ragnarlotus wants to merge 14 commits into
BassT23:developfrom
ragnarlotus:fix/error-propagation

Conversation

@ragnarlotus

Copy link
Copy Markdown

Summary

Harden update error propagation so mutating commands are executed exactly once, their combined output is retained, and their real exit status reaches the existing updater error/final-status model.

This is intentionally limited to error propagation. It does not include later work around resource preflight, APT policy, health/recovery, storage, logging, or WebUI behavior.

Problem

Several update paths currently use the pattern command || { ... $(command 2>&1) ...; }. When the first execution fails, the command is executed again to capture its output. For package upgrades and other mutating operations this can:

  • run a failed update step twice;
  • report the status/output of the second execution instead of the original one;
  • allow nested helper/user-script failures to be masked;
  • lose the original guest or transport status on some LXC, SSH VM and QEMU Guest Agent paths;
  • continue later update steps after a target step has already failed.

Changes

Single-execution runtime

  • Add a shared captured-command primitive that runs the command once, streams stdout/stderr live through tee, stores the same combined output for diagnostics, and preserves the original command exit code.
  • Add host, LXC and SSH-VM update-step wrappers which feed failures into the existing ERROR model and return the same status to their caller.
  • Keep equivalent fallback helpers in update.sh for installations where target-runtime.sh is not yet present during an update.

Host / LXC / VM propagation

  • Route host pveupdate, package upgrade and cleanup steps through the single-execution runtime.
  • Route LXC APT/DNF/Pacman/APK/YUM commands through the same mechanism.
  • Stop subsequent extras, trim and status-refresh steps when the preceding update operation fails.
  • Propagate fstrim, distribution-upgrade probes/commands and user-script failures instead of silently continuing.
  • Route SSH VM package commands and extra/user-script execution through the same error path.
  • Preserve exact QEMU Guest Agent guest/transport failures and stop subsequent commands after a failed QGA update step.
  • Preserve the transport status for Windows QGA updates instead of replacing it with a missing/guest status.
  • Make ERROR() return the captured error status while retaining the existing aggregate UPDATE_FAILURE behavior.

Extra updates

  • Enable strict error handling in update-extras.sh so an enabled extra updater cannot fail silently.
  • Preserve the Community Scripts helper's exact pipeline statuses.
  • Keep helper stdin detached with </dev/null so a controlling terminal cannot stop the nested helper through terminal ioctls.
  • Return the helper exit code rather than printing a warning and reporting success.

Compatibility with current develop

The work branch was created directly from current develop at:

f84fb17992137db84a7db48c94b3b2c3a1f11f9f (fix: repair SSH guest package count commands)

Existing current-develop behavior was retained, including internal target selection, post-update status capture, script-only mode, current package-count helpers, check-only finalization, and Community Scripts stdin detachment.

Regression coverage

  • Add tests/test-update-step-runtime.sh to verify that a failing mutating command:
    • executes once;
    • returns its original exit code;
    • reaches ERROR once;
    • preserves both stdout and stderr.
  • Update tests/test-community-scripts-update.sh to require exact helper failure propagation and single execution while retaining the PTY/stdin-detachment regression case.
  • Existing check-only finalization and node-update-scope tests remain unchanged because current develop already contains those portions of the earlier hardening.

Scope

Files changed:

  • target-runtime.sh
  • update.sh
  • update-extras.sh
  • tests/test-community-scripts-update.sh
  • tests/test-update-step-runtime.sh

No real package update is required by the regression tests.

BassT23 commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Thanks for the work on #343. I ran a fairly deep review against the current develop base (f84fb179), including the full regression suite, PTY fixtures and a temporary integration test with #341, #340 and #339.

The core idea of the PR looks good: mutating commands should run exactly once and the original failure status must be propagated.

However, I found a few things that I think should be addressed before we rebase this onto the later develop.

1. Interactive TTY semantics

This is the most important point.

RUN_CAPTURED_COMMAND() currently executes commands through:

"$@" 2>&1 | tee "$output_file"

Input still works under a PTY and /dev/tty prompts work, but the file-descriptor semantics change:

  • direct execution: stderr is a TTY
  • wrapped execution: stderr is no longer a TTY

So commands which change behaviour based on stdout/stderr TTY detection can behave differently.

Interactive mode is the normal Ultimate Updater mode and must retain real terminal semantics. Headless/non-interactive mode is optional.

Please adjust this so the new error-capture mechanism does not change the terminal contract for interactive package/update commands, and add a PTY regression test for it.

2. update-extras.sh strict mode is too broad

The new global:

set -Eeo pipefail

changes the failure policy of the complete extras script, not just Community Scripts.

That affects Pi-hole, ioBroker, setcap handling, Pterodactyl, Octoprint and Docker updates as well.

Some commands which were previously tolerated can now abort the complete extras run.

For this PR I would prefer keeping the scope limited to exact failure propagation. Please either scope strict handling only to the operations which require it, or explicitly preserve the existing behaviour of the other extras.

Community Scripts itself tested fine:

  • single execution
  • exact helper exit code
  • detached stdin
  • live output
  • PTY regression

3. pveupdate changes from non-fatal to fatal

Current develop deliberately has:

pveupdate || true

#343 changes this into a fatal update step through RUN_HOST_STEP.

That means a pveupdate failure now stops the host update and makes the complete target fail.

For #343 I would keep the existing pveupdate semantics. If we want to change that policy later, I would rather do it as a separate change with dedicated testing.

4. Stale target state on early returns

There are new paths such as:

EXTRAS || return $?
TRIM_FILESYSTEM || return $?
UPDATE_CHECK || return $?

which can leave CCONTAINER=true or CVM=true when returning early.

That can leak the previous target context into later processing.

Please make sure the per-target state is cleared on every exit path.

5. User-script changes

USER_SCRIPTS() / USER_SCRIPTS_VM() now also change unrelated behaviour:

  • hidden/temp/backup files are ignored
  • directories are ignored
  • scripts are sorted
  • quoting changes
  • Alpine uses ash
  • cleanup behaviour changes

Some of these are good improvements, but they go beyond the error-propagation scope.

Please either reduce this part to what is necessary for #343 or add focused regression coverage for the new behaviour.

Test status

The PR itself is otherwise in good shape:

  • PR-specific tests: PASS
  • shell suite: 85/85 PASS on repeat
  • Python: 25/25 PASS
  • Bash syntax: PASS
  • ShellCheck 0.11.0: PASS
  • git diff --check: PASS
  • temporary develop + #341 + #340 + #339 + #343: 86/86 PASS

There was one transient race in test-community-scripts-pty-job-control.sh on the first full run (ProcessLookupError), but it could not be reproduced in repeated isolated runs and the second complete run passed. I don't consider that a blocker by itself.

No need to rebase yet. It would be best to address these points against the current base first. After #341, #340 and #339 are merged, we can do one final rebase and integration/regression pass.

Thanks again — the single-execution/error-propagation direction itself definitely makes sense.

Copy link
Copy Markdown
Author

I want to clarify one consequence of the TTY requirement before changing this part of the PR.

The capture layer was implemented this way deliberately because the goal is not only to propagate a non-zero exit code. When a mutating operation fails, we need to retain the result of that exact execution:

  • execute it only once;
  • preserve its original exit code;
  • preserve the actual stdout/stderr produced by that failure;
  • propagate that diagnostic information through the existing error model.

This avoids the previous situation where a mutating command could effectively be executed again just to obtain its error output, potentially reporting a different result from the original failure.

The difficulty with preserving the interactive TTY semantics is specifically with shell functions.

A shell function may need to remain in the current shell because it can depend on or modify its state. If it is moved into another PTY/session for capture, its execution environment changes. If it remains directly attached to the real terminal, preserving those TTY semantics, then its stdout/stderr cannot also be transparently intercepted and retained in the same way.

The exit code can still be propagated, but in that case the updater may no longer have the actual diagnostic output from the failed execution and would only know that the function returned a failure.

That is the trade-off I wanted to make explicit. We implemented it this way because preserving the real failure information is important for error propagation and will also be useful for later work around package handling, recovery, logging and structured execution results.

I am happy to follow whichever behaviour you want for upstream. I just want to make sure that, if we prioritise preserving the current interactive TTY semantics here, we do so knowing that exact failure-output propagation for those cases may no longer be guaranteed.

BassT23 commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Thanks for clarifying the trade-off. I agree that retaining the exact stdout/stderr of the original failing execution is valuable, especially for headless automation, logging and later structured recovery work.

For Ultimate Updater we still want to prioritise the existing terminal contract in interactive mode, because that mode is intentionally used for human-driven updates where package managers or maintainer scripts may ask questions and expect a real terminal. The user must be able to answer those prompts reliably. This is not just cosmetic output behaviour; interactive updates are a first-class operating mode for us.

At the same time, we also need the opposite behaviour for scheduled runs: in non-interactive/headless mode there is no human present, so exact capture of the original stdout/stderr and exit status is more important than preserving TTY semantics.

So the upstream direction we want is:

  • Interactive mode: preserve the real terminal semantics of the executed command. Keep the original exit status, but do not force stdout/stderr through a capture pipeline if that changes the TTY contract. If exact failure text cannot be retained in this mode without changing the terminal behaviour, that is acceptable because the live terminal/job output is already the primary diagnostic surface for the human operator.
  • Headless/non-interactive mode: use the single-execution capture path so stdout/stderr from the exact failed execution are retained together with the original exit code. This is the mode used for scheduled/unattended tasks and is where structured error capture matters most.

I would prefer this split over introducing another pseudo-terminal/session layer in #343. The latter would make an already fairly large error-propagation PR more complex and harder to validate.

So yes: please keep the single-execution/error-propagation goal, but branch the execution strategy according to the effective interactive/headless mode.

The other review points still stand unchanged: scope the strict-mode change in update-extras.sh, preserve current non-fatal pveupdate behaviour for this PR, clear per-target globals on every early return, and either reduce or specifically test the unrelated user-script behaviour changes.

Once those points are addressed against the current base, we can do the final rebase after #341/#340/#339 and run the integration/regression pass.

Copy link
Copy Markdown
Author

One important clarification on why I made the error-propagation changes in the first place:

The real problem I was trying to solve was not pveupdate specifically. It was the updater reporting success for a target even when the actual LXC/application update had failed.

I hit this several times while updating real LXCs. Typical examples were Community Scripts/helper updates failing because the container could not complete the build/update step (for example because runtime/build resources were insufficient), or a nested helper/script returning a real error. The package-manager part could still have completed, the helper failed afterwards, but the failure was swallowed or lost and Ultimate Updater could continue to the end and report the target/update as successful even though the application itself had not actually been updated.

That is why the change was important to me: a failed mutating update step must not later become Exit code: 0 / success simply because the caller continued, reran something to capture output, or ignored the nested helper's status.

The intended guarantee was therefore:

  • execute each mutating operation once;
  • preserve the exit status from that exact execution;
  • preserve the diagnostic output from that same failure where possible;
  • propagate the failure through nested helpers/scripts;
  • and prevent the final target status from saying success when the actual update did not complete.

The resource-preflight work I planned next was complementary to this: preflight tries to avoid known resource-related failures before running the helper, while this PR makes sure that if an update still fails for resource reasons (or any other real error), that failure cannot be silently turned into success.

So I am fine with keeping pveupdate || true if that is intentionally non-fatal upstream; it was not the source of the false-success problem I was trying to fix. I just want to make sure we preserve the actual requirement that motivated #343: real LXC/helper update failures must reach the final result instead of being reported as successful.

BassT23 commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Thanks, that clarification makes the original motivation much clearer — and I agree with the core requirement.

The important invariant for #343 should be:

  • a real mutating update step runs once;
  • its real exit status is preserved;
  • nested/helper failures propagate outward;
  • and a failed LXC/application update must never end up as a successful final target result.

I also agree that pveupdate is a separate policy question. If upstream intentionally keeps pveupdate || true, that does not weaken the actual goal of this PR as long as real package/helper/application update failures still reach the final status.

I checked the current PR head as well, and there are still a few implementation points to align before the final rebase:

  1. Interactive vs. headless execution

    The current head now uses script -qef for fully interactive external commands so it can preserve TTYs while still capturing output. I understand why, but that is more complexity than I want to introduce in fix: propagate update failures without duplicate execution #343.

    The intended upstream split remains the simpler one from the previous comment:

    • interactive mode: execute against the real terminal and preserve its native terminal contract; propagate the real exit code; exact retained failure text is optional here;
    • headless/non-interactive mode: use the single-execution capture path and retain stdout/stderr + exit status from that exact execution.

    So please do not add an extra PTY/session layer just to retain capture in interactive mode.

    Also note that the legacy fallback RUN_CAPTURED_COMMAND() in update.sh still uses "$@" 2>&1 | tee ..., so the original TTY issue remains there when target-runtime.sh is unavailable. The fallback should follow the same effective interactive/headless policy.

  2. pveupdate

    Your latest comment matches the intended policy, but the current code still has RUN_HOST_STEP pveupdate || return $?. Please restore the existing non-fatal behaviour for this PR.

  3. update-extras.sh strict mode

    The global set -Eeo pipefail is still present and still changes failure semantics for all extras, not only Community Scripts. Please scope this so unrelated extras retain their previous behaviour unless explicitly covered by this PR.

  4. Per-target state cleanup

    Some error paths now clear CCONTAINER / CVM, which is good, but there are still early return $? paths from update/extras/trim/status steps before the final cleanup. Please make sure the target globals are cleared on every exit path.

  5. User-script scope

    The current head still includes behavioural changes such as sorting, ignoring hidden/backup/temp files, Alpine ash, and changed cleanup semantics. Please either reduce those changes to what is required for error propagation or add focused regression coverage for those behaviours.

So: the requirement is now fully aligned, but the PR is still CHANGES REQUIRED before rebase.

Once these points are addressed against the current base, we can keep the planned sequence: merge #341 → #340 → #339, then do one final rebase of #343 and run the full integration/regression pass.

Thanks again — the false-success problem you are fixing here is absolutely a real one, and I want to keep that guarantee intact while avoiding unrelated behaviour changes in the same PR.

Copy link
Copy Markdown
Author

Thanks, that makes sense.

I’m reviewing each of the remaining points one by one before changing the implementation, mainly to understand the capabilities and limitations of each approach and make sure the final behaviour stays consistent with the error-propagation guarantees we now agree on.

I’ll use that review to decide the cleanest approach for each point, and I’ll raise anything that still has a meaningful trade-off before changing the code.

Copy link
Copy Markdown
Author

I have now addressed the update-extras.sh strict-mode point in commit 5eb035df6e80cdaa6d7f4ac89efe59c8ab43129c.

The approach is deliberately narrow:

  • removed the global set -Eeo pipefail and global ERR trap, so unrelated extras keep their previous best-effort/error-tolerance behaviour;
  • kept Community Scripts failure handling explicit and local to that helper path;
  • preserve the helper's exact pipeline status via PIPESTATUS;
  • return the helper's real non-zero exit code;
  • keep stdout/stderr live through tee;
  • and only print the success message when the helper itself and the capture pipeline both succeed.

This keeps the invariant we agreed on without changing the failure policy of Pi-hole, ioBroker, Pterodactyl, OctoPrint, Docker, etc.

The existing tests/test-community-scripts-update.sh already covers the critical regression here: a failing helper executes once, returns its real exit code (7 in the fixture), exposes its error output, and must not print Update process completed.

So the intended distinction is now explicit in the code:

  • unrelated extras retain their historical behaviour;
  • a real Community Scripts/application-helper failure still propagates outward and cannot be converted into a successful target result.

I am continuing to review the remaining points individually before changing them.

Copy link
Copy Markdown
Author

I have now addressed the two remaining review points that do not conflict with the error-propagation guarantees:

pveupdate

The host path is back to the existing best-effort policy:

pveupdate || true

Real package upgrade/cleanup steps still use the failure-propagating runtime, so this remains an explicit pveupdate policy exception rather than weakening the LXC/helper failure invariant.

Per-target state cleanup

CCONTAINER and CVM are now cleared globally before entering the target update and then set as function-local variables for the lifetime of UPDATE_CONTAINER() / UPDATE_VM().

Bash's dynamic scoping keeps the target context visible to nested helpers such as ERROR() and UPDATE_CHECK() while the update is running, but the local value disappears automatically on every return path — including early failures from package updates, extras, trim, status refresh, transport setup, etc. The global value therefore remains clean after the target function exits without having to duplicate cleanup before every return.

I also compared the current head against the previous 5eb035d state after the change. The net update.sh diff is limited to those intended changes only; unrelated edits introduced during the file replacement were reverted before considering this point complete.

I have intentionally not changed the two remaining discussion points yet: the interactive/headless execution strategy and the unrelated USER_SCRIPTS*() behavioural changes.

Copy link
Copy Markdown
Author

I have been looking more closely at the interactive/headless distinction, because I think there is one important conceptual point we should clarify before changing this part.

My concern is that we may currently be treating two different things as equivalent:

  • the update was started from a real console;
  • the update actually requires human interaction.

In practice, those are not necessarily the same thing.

I have been running Ultimate Updater manually from a console for months. According to the current mode detection, those runs are therefore considered interactive. However, the actual guest update process has never behaved interactively for me in the usual sense of asking what to do during package/application updates.

That also seems consistent with the updater itself: package managers are generally called with automatic/non-confirming options (-y, --noconfirm, dpkg config preservation options, etc.), so the normal behaviour is already to perform the update automatically rather than ask the operator for ordinary package decisions.

This distinction matters because the original problem behind #343 happened precisely in this kind of manually launched run.

For example, a Community Scripts/helper update could require more resources than the guest currently had. The helper could fail, but Ultimate Updater did not stop and ask me how to proceed or offer to change resources. The failure was effectively lost and the target was later presented as successfully updated.

So, in that real case:

real console available = yes
actual human interaction during the update = no
helper failure = yes
exact failure information = important

Preserving the fact that a real terminal exists would not by itself have helped with that failure. What mattered was preserving the failed execution, its exit status and enough diagnostic information to prevent it being interpreted as success.

I completely understand the reason for not changing terminal semantics for commands which genuinely need a terminal. A maintainer script or another exceptional command may inspect whether stdout/stderr are connected directly to a terminal and alter its behaviour accordingly.

What I am questioning is whether the presence of a terminal should automatically mean that all update commands must give up exact failure capture.

It may be more accurate to distinguish:

1. How was Ultimate Updater launched?
2. Does this particular operation actually require direct terminal interaction?

rather than treating:

launched manually from console
=
requires interactive terminal semantics for every update operation

as a single condition.

There is also a simpler case where I do not think there should be any ambiguity: executions which are not launched from a user console.

For cron, WebUI jobs, scheduled execution or another service-driven invocation, the caller should explicitly mark the run as non-interactive. That seems cleaner than trying to infer intent from file descriptors later.

The project already has mechanisms in this direction (-s/--silent, UU_NONINTERACTIVE, RUN_FROM_CRON, IN_HEADLESS_MODE), so I am not suggesting a new execution model. My expectation would simply be that any launcher which is not a human console — cron, WebUI, job runner, etc. — explicitly passes the appropriate non-interactive flag/environment state.

Conceptually:

manual console launch
    → terminal is available
    → this does not automatically imply every update step needs interaction

cron / WebUI / scheduled job / service
    → explicitly mark execution as non-interactive
    → exact single-execution capture is expected

For manually launched runs, perhaps the remaining question is therefore narrower:

Which concrete update operations in Ultimate Updater actually need direct terminal semantics?

If only specific commands need that property, it may be preferable to preserve direct terminal execution for those commands rather than sacrificing exact failure capture for every command merely because the top-level updater was started from a console.

The invariant I want to preserve from #343 is still the same:

a real package/helper/application update fails
    → execute it only once
    → preserve the real exit status
    → do not report the target as successfully updated

And where technically possible, retaining the exact output of that failed execution remains valuable for diagnostics, logging, future recovery logic and structured/WebUI results.

BassT23 commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Thanks — I think this is exactly the distinction we needed to make explicit.

For Ultimate Updater, the upstream product decision is still to keep the normal/manual execution path mostly interactive by default.

The reason is not that every package operation normally asks a question. Most of them do not. The problem is that we cannot know at the start of a run whether a later package, maintainer script or configuration transition will suddenly require human input.

We have already seen a concrete example of this in practice: an apt-get dist-upgrade -y run could still stop on a dpkg conffile decision for /etc/ssh/sshd_config. So -y, --noconfirm and similar flags reduce ordinary prompts, but they do not give us a reliable guarantee that the complete update path is non-interactive.

That is why we deliberately chose this policy:

normal / manual run
    → treat the update path as potentially interactive
    → preserve the real terminal contract
    → allow an unexpected package/maintainer prompt to reach the user
    → preserve the real exit status
    → a failed update must never become a successful target result

headless / scheduled / WebUI / service run
    → explicitly mark the run as non-interactive
    → exact single-execution capture of stdout/stderr is preferred
    → preserve the real exit status
    → a failed update must never become a successful target result

So I agree with your conceptual point that "a terminal exists" and "this specific operation will definitely ask a question" are not the same thing. But for the normal path we intentionally cannot classify commands ahead of time as safely non-interactive, because whether a prompt appears can depend on the current package state, distro version, maintainer scripts, repository changes, local config files, and third-party application updaters.

Trying to whitelist only the operations that currently need direct TTY semantics would therefore make the safety of the normal path depend on assumptions we cannot reliably maintain.

In other words: normal mode is not "interactive because every command needs interaction"; it is potentially interactive because we cannot safely rule interaction out before the run.

For this PR that means I am fine with the interactive path losing exact internal stdout/stderr capture where necessary, as long as:

  • the mutating command still executes only once;
  • its real exit status is preserved;
  • failure propagates through the target result;
  • the real terminal remains available to the user;
  • and a failed target can never be reported as successfully updated.

For headless mode, exact capture remains the desired behaviour because there is intentionally no human present to answer anything.

So please keep the implementation split along that policy rather than trying to infer, per package command, whether interaction will be needed.

Copy link
Copy Markdown
Author

I want to clarify one point about USER_SCRIPTS() / USER_SCRIPTS_VM() because for me this is part of the update contract, not just an unrelated convenience feature.

If a user script is configured to run as part of a target update, I think its result has to contribute to the final target result. Otherwise Ultimate Updater can report a successful update even though the configured update procedure did not complete successfully.

A concrete example from my environment is Caddy: I have custom logic around the wildcard reverse-proxy/SSL setup. If that custom update step fails, the container may still be running and the base packages may have updated correctly, but an essential service can be left non-functional because the reverse proxy / SSL configuration was not applied correctly.

In that situation, reporting the target as successfully updated would be a false success just as much as when a package/helper update fails.

Conceptually I see the configured update as:

base/package update
    + enabled extras/helpers
    + configured user scripts
    = complete target update

If any required part of that chain fails, the target is not fully updated.

This also matters for the later hardening work that #343 is intended to make possible. The roadmap after error propagation includes resource preflight, package semantics, health/recovery, storage safety, logging and structured/WebUI execution results. In particular, recovery/rollback only works correctly if the updater can trust the target result.

For example:

user script fails
    -> target must be FAILED
    -> later health/recovery logic can react
    -> rollback/restore can be considered when appropriate

If the user-script failure is swallowed and the target is marked SUCCESS:

user script fails
    -> target = SUCCESS
    -> recovery is never triggered
    -> logging/structured results are wrong
    -> an essential service may remain broken

So I am completely fine reducing the unrelated behavioural changes you pointed out (sorting, hidden/temp/backup filtering, Alpine ash, cleanup-policy changes, etc.) to keep #343 narrowly scoped.

The one part I do want to preserve is the semantic guarantee that a configured user script is part of the update result: it runs once, its real exit status is preserved, and a real failure propagates outward so the target cannot be reported as successfully updated.

That is important on its own today, and it is also a prerequisite for the later recovery/logging/structured-result work to be trustworthy.

Copy link
Copy Markdown
Author

I want to push this point one step further before we change the implementation, because the concrete behaviour of Ultimate Updater matters here more than the generic behaviour of apt-get or another package manager.

I have been using Ultimate Updater manually from a console for months, and in normal update runs it has never asked me for any package/update decision. That by itself is anecdotal, but the current code also explains why: the normal update paths are intentionally written to auto-manage package decisions.

For Debian/Ubuntu-family updates, Ultimate Updater does not just run:

apt-get dist-upgrade -y

It defines and uses:

DPKG_OPTIONS=(
  -o Dpkg::Options::=--force-confdef
  -o Dpkg::Options::=--force-confold
)

and those options are included in the normal host/LXC/VM upgrade paths together with -y.

Likewise, the other package managers are already called with their non-confirming forms (dnf -y, yum -y, pacman --noconfirm, pkg ... -y, etc.).

So I agree with the generic statement that apt-get -y alone does not guarantee that no prompt can ever appear. But that is not the complete command/policy Ultimate Updater is actually using in its normal update flow.

There are also places where Ultimate Updater does know that an operation is genuinely interactive. The Debian distribution-upgrade flow, for example, explicitly calls read -p ... to ask the operator whether to continue. In that case interactive terminal semantics are clearly part of the operation by design.

That suggests to me that the important distinction is not simply:

started from a console
    = interactive

but rather:

this update run is configured to auto-manage decisions
    = capture exact failure information where possible

this operation/run is explicitly allowed to require human decisions
    = preserve direct terminal interaction

This is also why the original implementation in #343 tried to keep the exact stdout/stderr from the same failed execution. The real failures I was trying to solve were not cases where Ultimate Updater stopped and asked me what to do. They were the opposite: helpers/application updates failed without asking anything, the failure was swallowed or lost, and the target was later reported as successful.

For example, a Community Scripts/helper update can fail because the guest does not have enough runtime/build resources. In my real runs Ultimate Updater never asked whether resources should be increased; it simply continued. That exact failure information is important because later hardening work can use it for diagnostics, resource preflight, logging, health/recovery and possibly rollback decisions.

There is another relevant case: Community Scripts is deliberately executed with stdin detached (</dev/null). That path cannot become interactively recoverable merely because the top-level updater happened to be launched from a console. For that kind of operation, sacrificing exact failure capture in order to preserve hypothetical interaction does not buy us anything.

The same applies conceptually to configured user scripts unless they are explicitly designed to be interactive. A user script may be essential to the final service state (for example, applying the Caddy wildcard/reverse-proxy configuration). If it fails, the service may be materially broken even though the guest itself is still running, so the updater needs the real failure result rather than only a generic indication that something failed.

I therefore think the execution policy should be explicit rather than inferred only from the existence of a TTY.

My preferred approach would be a configuration policy, something along the lines of:

AUTO_MANAGE_UPDATE=true

(name only illustrative)

Meaning:

AUTO_MANAGE_UPDATE=true
    → Ultimate Updater is responsible for making the normal package/update decisions
    → package/helper/application steps are treated as non-interactive by policy
    → single execution
    → preserve exact stdout/stderr where technically possible
    → preserve the real exit status
    → a failed required step must fail the target

AUTO_MANAGE_UPDATE=false
    → operator interaction is allowed/expected
    → preserve the direct terminal contract
    → preserve the real exit status
    → a failed required step must still fail the target

That would describe the actual product behaviour much more directly than equating "manual launch" with "interactive update".

A second, simpler mechanism can still exist at invocation level:

cron / WebUI / scheduled job / service
    → caller explicitly marks the run non-interactive

manual console launch
    → use the configured policy instead of assuming interaction solely because a terminal exists

The project already has pieces of this (-s/--silent, UU_NONINTERACTIVE, RUN_FROM_CRON, IN_HEADLESS_MODE), so this would not require inventing a completely new model. It would mainly make the intended update policy explicit and consistent.

I am not claiming that no third-party script can ever call read or otherwise require a terminal. The question is whether that exceptional possibility should force every normal manually launched update step to give up exact diagnostic capture, even though the updater itself is deliberately driving those updates automatically and the real failure cases we are fixing are precisely silent/non-interactive failures.

For #343, the invariant I do not want to lose is still:

required update/helper/user-script step fails
    → execute it only once
    → preserve the real exit status
    → preserve the diagnostic output from that exact failure where the policy allows it
    → do not report the target as successfully updated

If upstream wants manual runs to remain potentially interactive by default, I think an explicit configuration switch for auto-managed vs operator-interactive execution would be a cleaner long-term contract than using "a console exists" as the policy decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants