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

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,