plan/start groundwork: defined wait-timeout semantics, soft-failing operations - #14082
Open
ndeloof wants to merge 2 commits into
Open
plan/start groundwork: defined wait-timeout semantics, soft-failing operations#14082ndeloof wants to merge 2 commits into
ndeloof wants to merge 2 commits into
Conversation
waitDependencies returned nil when its context expired: every polling goroutine hit the ctx.Done branch and reported success, so the 'timeout waiting for dependencies' translation after eg.Wait() was unreachable in practice, and up --wait / start --wait could succeed silently while nothing was healthy. The error path was only ever taken by a race, when a health probe happened to be in flight at expiry. The ctx.Done branch now distinguishes the two reasons the context can end: a deadline is exactly the failure this function is asked to detect and is surfaced (callers translate it into their own messages: 'timeout waiting for dependencies', 'application not healthy after X'); a plain cancellation (user interruption) keeps returning nil so Ctrl-C does not masquerade as a dependency failure — preserving the behavior of the restart/run/up callers that pass no timeout. Both paths are now pinned by unit tests; neither message was covered before. Part of #14074 (C) and groundwork for the start-in-plan convergence. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
…e waits The executor aborted the whole plan on any node failure, which rules out modeling optional semantics — a dependency declared with required: false must be reported as skipped, not fail the operation, and its dependents must still run. This is a prerequisite for planning the start phase (dependency-condition waits) inside the reconciler. Operation gains Optional: at the walker level, a failing Optional node emits a Skipped event plus a warning and completes successfully, so dependent nodes proceed. It is documented against the pre-existing BestEffort flag, which is narrower (one specific expected error, tolerated inside the operation itself) — the two docs now cross-reference each other so they cannot be confused. Covered by an executor unit test asserting a failing Optional node does not fail the plan and its dependent still executes. Groundwork for the start-in-plan convergence. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ndeloof
force-pushed
the
plan-start-phase0
branch
from
August 18, 2026 09:22
585e8e3 to
2948f31
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 0 of #14081 (converge the start phase into the plan engine) — the two prerequisites, each useful on its own:
waitDependenciesno longer swallows its timeout (🐛 from Epic: make the codebase agent-legible — fix misleading self-description, ambiguous contracts, and legacy leftovers #14074 C): every polling goroutine returnednilonctx.Done(), making the 'timeout waiting for dependencies' path unreachable except by race —up --wait --wait-timeoutcould succeed silently with nothing healthy. A deadline now surfaces as the failure it is; a plain user cancellation still returns nil so Ctrl-C doesn't masquerade as a dependency failure. Both paths gain their first unit tests.Operation.Optional: a failing Optional node emits a Skipped event + warning and lets dependents run, instead of aborting the plan — the capabilityrequired: falsedependency waits will need once they become plan nodes. Documented against the narrower pre-existingBestEffortflag so the two can't be confused. Unit-tested.Part of #14081; also closes one 🐛 item of #14074 (section C).
🤖 Generated with Claude Code