fix(speculate): wait for every assumption to settle before merging (#559)

## Summary

### Why?

A head could merge on an assumption that had not come true.
`mergeablePath` required every dependency a path assumed would *succeed*
to have actually merged, but imposed no wait at all on one it assumed
would *fail*.

The doc comment stated the reasoning: *"A dependency assumed to fail
imposes no wait: the path is broken the moment that dependency succeeds,
so a still-live path has already been vindicated on it."* That holds
only if the transition were instantaneous. It is not — a dependency
spends time in `speculating`, and then in `merging`, having neither
succeeded nor failed. `assumptionBroken` only fires on a terminal state,
so throughout that window the path is neither broken nor vindicated. It
is unsettled, and the gate read unsettled as permission.

A path that assumed a dependency would fail was built *without* that
dependency's changes. Landing the head while that dependency is still
live, and watching it land too, puts a combination on the trunk that no
build ever validated — the one thing the queue exists to prevent. It
needs no textual conflict to break the trunk, because the two changes
were never built together.

Measured against the real predicates before fixing, with a passed
`fails(D)` path and only D's state varying:

| D's state | `mergeablePath` | correct? |
| -- | -- | -- |
| `speculating` | true | no |
| `merging` | true | no |
| `cancelling` | true | no |
| `failed` | true | yes — the assumption came true |
| `succeeded` | false | yes — `assumptionBroken` catches it |

`decide` returned `merge` in all three of the wrong rows.

### What?

The rule is now symmetric: a path may merge once every dependency has
finished the way it assumed. `succeeds` needs `Succeeded`; `fails` needs
`Failed` or `Cancelled`. `allAssumedSucceedingMerged` becomes
`allAssumptionsSettled`, since it no longer looks only at succeeding
dependencies.

One wording consequence of the parent commit dropping the *ignored*
assumption: the old doc sold this gate as "the head waits only on the
dependencies it was built on top of, not its full dependency list". With
every dependency carrying a position, that is no longer what speculation
buys. What it buys is that the build already ran — when the dependencies
land the way the path guessed there is nothing left to execute, and the
head merges at once.

Note this is not the "bypass large diff" early merge the RFC describes.
That reads a passed path for *every* combination of the dependencies,
and is not implemented on the controller side — nothing enumerates
combinations. A single path betting the right way was never a sound
approximation of it, and `speculation.md` now says so rather than
describing behaviour the code does not have.

One liveness consequence, recorded on CODEM-428 rather than fixed here:
a dependency stuck in `Cancelling` now stalls its dependents too, not
just itself. `finalizeCancellations` converges `Cancelling → Cancelled`
on any subsequent run, so this only bites when no further run is
triggered — which is that issue's edge-triggering gap.

## Test Plan

- ✅ `make test` — 96/96 pass
- ✅ `make lint`, `make check-gazelle`, `make check-tidy`

`TestMergeablePath` is rebuilt around the new rule, holding the second
dependency settled so the first is the only variable: a fails assumption
waits out `speculating`, `merging` and `cancelling`, and merges on
`Failed` or `Cancelled`; an assumed-succeeding dependency waits out its
merge; one unsettled dependency is enough to wait.

## Issue

Fixes https://linear.app/uber/issue/CODEM-429

Author: behinddwalls

doc/rfc/submitqueue/speculation.md

@@ -54,7 +54,7 @@ Every write is a compare-and-swap: a writer that loses re-reads on a later run.
 
 Verdicts are controller-owned facts: the Speculator can neither compute nor veto them.
 
-- **Merge (strict).** Each path carries an assumption about every dependency — *succeeds* (built on top of) or *fails* (built without). Once a path's build has passed and every dependency it assumes *succeeds* has merged, the speculate controller moves the head to Merging and hands it to Runway — it waits only on the dependencies it was built on top of, not the head's full dependency list. If that hand-off is lost, the next run re-sends it. The same run sets the head's remaining in-flight paths *cancelling*: once one path has passed the others cannot help, and they hold CI slots until they stop. The mergesignal controller records Runway's terminal result: success marks the head Succeeded, while failure marks it Failed. The result publishes a single dirty signal — no per-dependent fan-out — and the next run refutes paths whose assumption disagrees with the result: *fails* assumptions after success, *succeeds* assumptions after failure. The hand-off is idempotent, so Runway reports success without another merge when the change is already present. Down a chain, each head waits for the predecessors it assumes succeed, so a chain merges one at a time.
+- **Merge (strict).** Each path carries an assumption about every dependency — *succeeds* (built on top of) or *fails* (built without). Once a path's build has passed and every dependency has finished the way the path assumed — one assumed *succeeds* has merged, one assumed *fails* has failed or been cancelled — the speculate controller moves the head to Merging and hands it to Runway. A dependency that is merely *merging* has not finished, because a merge can fail, so it is still waited on. If that hand-off is lost, the next run re-sends it. The same run sets the head's remaining in-flight paths *cancelling*: once one path has passed the others cannot help, and they hold CI slots until they stop. The mergesignal controller records Runway's terminal result: success marks the head Succeeded, while failure marks it Failed. The result publishes a single dirty signal — no per-dependent fan-out — and the next run refutes paths whose assumption disagrees with the result: *fails* assumptions after success, *succeeds* assumptions after failure. The hand-off is idempotent, so Runway reports success without another merge when the change is already present. Down a chain, each head waits for its predecessors to settle, so a chain merges one at a time.
 - **Failure (no viable path).** A batch fails when every possible future has a failed build — no path can pass, so it can never merge.
 - **Cancel.** A cancelled batch is driven terminal: its in-flight paths are set *cancelling*, then the batch is marked Cancelled once they stop (see Cancellation).
 
@@ -76,6 +76,8 @@ If a batch's passed builds cover *every* way its dependencies could resolve, the
 
 The default Speculator covers the whole space only when doing so is cheap enough, and funds the extra candidates within the build budget. The controller merges early only when a passed path exists for every combination of the dependencies — it reads that straight off the path records. If any combination is missing or unbuilt, the head waits normally.
 
+**Not yet implemented on the controller side.** `decide`/`mergeablePath` gate on a single passed path whose assumptions have all been settled by the dependency's actual state; nothing enumerates the combinations. The distinction matters: a *single* passed path that assumed a dependency would fail is not complete coverage, and merging on it while that dependency is still live would put a combination on the trunk that no build validated. Coverage is what makes early merge sound — one path betting the right way is not.
+
 ### Cancellation
 
 Cancellation is best-effort: a batch marked *cancelling* may still merge if a merge wins the race, so terminal states prevail. A cancel sets the intent; a later run drives it terminal.

submitqueue/orchestrator/controller/speculate/outcome.go

@@ -60,17 +60,22 @@ func decide(head entity.Batch, set entity.SpeculationPathSet, snap snapshot) out
 	return outcomeWait
 }
 
-// mergeablePath returns a passed path whose merge preconditions are met:
-// every dependency it assumed would succeed has actually merged.
+// mergeablePath returns a passed path whose merge preconditions are met: every
+// guess it made about a dependency has been borne out by that dependency's
+// actual state.
 //
-// This is what makes speculation pay. The head waits only on the dependencies
-// the passed build was stacked on — not on its full dependency list — so a
-// batch built without a slow neighbour merges as soon as the ones it actually
-// built on have landed.
+// This is what makes speculation pay — not by shortening the list the head
+// waits on, but by having already done the work. The build ran against the
+// guess while the dependencies were still resolving, so when they land the way
+// the path assumed there is nothing left to run and the head merges at once.
 //
-// A dependency assumed to fail imposes no wait: the path is broken the
-// moment that dependency succeeds, so a still-live path has already been
-// vindicated on it.
+// A guess that has not been settled yet is not a licence to merge, whichever
+// way it points. A path that assumed a dependency would fail was built without
+// that dependency's changes, so landing it while the dependency is still live
+// puts a combination on the trunk that no build ever validated — which is the
+// one thing the queue exists to prevent. The dependency merging is not enough
+// either: a merge can fail, so "on its way in" is still an open question, and
+// the head waits for the answer.
 func mergeablePath(set entity.SpeculationPathSet, snap snapshot) (entity.SpeculationPathEntry, bool) {
 	for _, entry := range set.Paths {
 		if entry.Status != entity.SpeculationPathStatusPassed {
@@ -79,22 +84,28 @@ func mergeablePath(set entity.SpeculationPathSet, snap snapshot) (entity.Specula
 		if assumptionBroken(entry.Path, snap) {
 			continue
 		}
-		if allAssumedSucceedingMerged(entry.Path, snap) {
+		if allAssumptionsSettled(entry.Path, snap) {
 			return entry, true
 		}
 	}
 	return entity.SpeculationPathEntry{}, false
 }
 
-// allAssumedSucceedingMerged reports whether every dependency the path
-// assumed would succeed has reached Succeeded.
-func allAssumedSucceedingMerged(path entity.SpeculationPath, snap snapshot) bool {
+// allAssumptionsSettled reports whether every dependency has finished the way
+// the path assumed: one it assumed would succeed has reached Succeeded, and one
+// it assumed would fail has finished some other way.
+func allAssumptionsSettled(path entity.SpeculationPath, snap snapshot) bool {
 	for _, dep := range path.Dependencies {
-		if dep.Assumption != entity.DependencyAssumptionSucceeds {
-			continue
-		}
-		if snap.batchState(dep.Batch) != entity.BatchStateSucceeded {
-			return false
+		state := snap.batchState(dep.Batch)
+		switch dep.Assumption {
+		case entity.DependencyAssumptionSucceeds:
+			if state != entity.BatchStateSucceeded {
+				return false
+			}
+		case entity.DependencyAssumptionFails:
+			if state != entity.BatchStateFailed && state != entity.BatchStateCancelled {
+				return false
+			}
 		}
 	}
 	return true

submitqueue/orchestrator/controller/speculate/outcome_test.go

@@ -46,26 +46,66 @@ func TestMergeablePath(t *testing.T) {
 		dep2State  entity.BatchState
 		want       bool
 	}{
+		// dep2 is settled the way its assumption expects throughout, so dep1
+		// is the only thing under test.
 		{
 			name:       "waits for an assumed-succeeding dependency to merge",
 			assumption: [2]entity.DependencyAssumption{succeeds, fails},
 			dep1State:  entity.BatchStateSpeculating,
+			dep2State:  entity.BatchStateFailed,
 			want:       false,
 		},
 		{
 			name:       "merges once it has",
 			assumption: [2]entity.DependencyAssumption{succeeds, fails},
 			dep1State:  entity.BatchStateSucceeded,
+			dep2State:  entity.BatchStateFailed,
 			want:       true,
 		},
 		{
-			name:       "an assumed-failing dependency imposes no wait",
+			name:       "an assumed-succeeding dependency waits out its merge",
+			assumption: [2]entity.DependencyAssumption{succeeds, fails},
+			dep1State:  entity.BatchStateMerging,
+			dep2State:  entity.BatchStateFailed,
+			want:       false,
+		},
+		{
+			name:       "waits for an assumed-failing dependency to actually fail",
 			assumption: [2]entity.DependencyAssumption{fails, fails},
 			dep1State:  entity.BatchStateSpeculating,
+			dep2State:  entity.BatchStateFailed,
+			want:       false,
+		},
+		{
+			name:       "still waits while that dependency is merging",
+			assumption: [2]entity.DependencyAssumption{fails, fails},
+			dep1State:  entity.BatchStateMerging,
+			dep2State:  entity.BatchStateFailed,
+			want:       false,
+		},
+		{
+			name:       "still waits while that dependency is cancelling",
+			assumption: [2]entity.DependencyAssumption{fails, fails},
+			dep1State:  entity.BatchStateCancelling,
+			dep2State:  entity.BatchStateFailed,
+			want:       false,
+		},
+		{
+			name:       "merges once it has failed",
+			assumption: [2]entity.DependencyAssumption{fails, fails},
+			dep1State:  entity.BatchStateFailed,
+			dep2State:  entity.BatchStateFailed,
+			want:       true,
+		},
+		{
+			name:       "merges once it has been cancelled",
+			assumption: [2]entity.DependencyAssumption{fails, fails},
+			dep1State:  entity.BatchStateCancelled,
+			dep2State:  entity.BatchStateFailed,
 			want:       true,
 		},
 		{
-			name:       "one unmerged dependency is enough to wait",
+			name:       "one unsettled dependency is enough to wait",
 			assumption: [2]entity.DependencyAssumption{succeeds, succeeds},
 			dep1State:  entity.BatchStateSucceeded,
 			dep2State:  entity.BatchStateSpeculating,