fix: honor mission branchStrategy in triage; skip validation for inactive missions (#1910)
## Summary
Two related mission-loop fixes, both observed wedging a live autonomous
board.
### 1. Triage ignores `mission.branchStrategy` when `branchAssignment`
is omitted (dashboard)
`resolveBranchAssignmentContext` fabricated `{ mode: "shared" }` for
absent input, so the mission triage routes (`triage`, `triage-all`)
always passed an explicit `assignmentMode` into
`missionStore.triageFeature`/`triageSlice`. That defeats the store's
fallback — `branchOptions?.assignmentMode ??
strategyDefaults.assignmentMode` — so a mission configured with
`branchStrategy: auto-per-task` still produced a **shared** branch
group, named after the base branch.
docs/missions.md documents the intended behavior: missions "can also
persist a `branchStrategy` used whenever triage is triggered without
explicit branch options."
**Fix:** absent input resolves to `{ mode: undefined }`; callers pick
their own default. The mission routes need no change (undefined now
flows through to the strategy fallback). The two planning-subtask call
sites keep their historical `shared` default via a destructure default,
since they have no strategy to fall back to. Explicit
`branchAssignment.mode` is unchanged and still overrides the strategy.
**Observed impact:** with `baseBranch: main`, every triaged task joined
a shared group literally named `main` — tasks tried to push to `main` /
open PRs with head=main base=main, and the whole group wedged in
`merge-retries-exhausted`. The only workaround was remembering to send
`{"branchAssignment": {"mode": "per-task-derived"}}` on every triage
call, which silently ignores the mission's configured strategy the rest
of the time.
### 2. Task-completion validation runs for parked missions (engine)
`MissionExecutionLoop.processTaskOutcome` validated every completed
feature-linked task with no mission-status check — unlike
`recoverActiveMissions`, which already skips missions with `status !==
"active"`. A parked mission (`status: planning`) kept minting validator
runs, and on validator failure, new "Fix:" features — for tasks that
completed after parking. On our board a stale validator workspace
produced a `Fix: → Fix: Fix: → Fix: Fix: Fix:` spiral of bogus features
for already-merged work; the only mitigation was re-parking the mission
after every release and manually archiving the minted features.
**Fix:** gate `processTaskOutcome` on the resolved mission being active,
mirroring the `recoverActiveMissions` guard. The gate sits before the
`needs_fix → implementing` transition so an inactive mission's features
get zero state mutation; the skip logs a `warning` mission event
(`validation_skipped_mission_inactive`) so it's visible in the mission
log. Features that don't resolve to a mission keep the current behavior.
(Out of scope but worth noting: the validator that triggered the spiral
was judging merged work against a stale workspace checkout — that
freshness issue is a separate problem this PR doesn't attempt.)
## Tests
- `branch-selection.test.ts` — updated: absent/`{}` input resolves
`mode: undefined`; explicit modes and the bad-mode error unchanged.
- `mission-execution-loop.test.ts` — two new tests: parked mission skips
validation and logs the warning event; active mission still validates.
- Existing `mission-store.test.ts` coverage ("uses mission
branchStrategy … when branch options are omitted", explicit `shared`
override still creates a group) pins the store side end-to-end — those
pass unchanged, as do the planning/branch-group route suites (182 tests)
and full workspace `pnpm typecheck`.
Changeset included (`patch`, category `fix`).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Branch selection now keeps an unspecified mode unset and falls back to
the mission’s configured branch strategy where appropriate.
* Task outcome processing now skips validation for missions that are not
active, preventing unnecessary follow-up actions.
* Added coverage for branch selection and mission execution behavior to
verify the updated handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Honor mission branchStrategy when triage omits branchAssignment; skip validation for inactive missions.
|
||||
category: fix
|
||||
dev: resolveBranchAssignmentContext returns undefined for absent mode so triage falls back to mission.branchStrategy; processTaskOutcome gates on mission.status === "active" like recoverActiveMissions.
|
||||
@@ -38,7 +38,12 @@ describe("branch-selection", () => {
|
||||
});
|
||||
|
||||
it("resolves assignment context", () => {
|
||||
expect(resolveBranchAssignmentContext(undefined)).toEqual({ mode: "shared" });
|
||||
// Absent input resolves to undefined so callers fall back to their own
|
||||
// default (e.g. mission triage uses mission.branchStrategy).
|
||||
expect(resolveBranchAssignmentContext(undefined)).toEqual({ mode: undefined });
|
||||
expect(resolveBranchAssignmentContext(null)).toEqual({ mode: undefined });
|
||||
expect(resolveBranchAssignmentContext({})).toEqual({ mode: undefined });
|
||||
expect(resolveBranchAssignmentContext({ mode: "shared" })).toEqual({ mode: "shared" });
|
||||
expect(resolveBranchAssignmentContext({ mode: "per-task-derived" })).toEqual({ mode: "per-task-derived" });
|
||||
expect(() => resolveBranchAssignmentContext({ mode: "bad" })).toThrow("branchAssignment.mode must be one of");
|
||||
});
|
||||
|
||||
@@ -68,7 +68,8 @@ export interface BranchAssignmentContext {
|
||||
}
|
||||
|
||||
export interface ResolvedBranchAssignmentContext {
|
||||
mode: PlanningBranchMode;
|
||||
/** undefined when the request did not specify a mode; callers pick their own default. */
|
||||
mode: PlanningBranchMode | undefined;
|
||||
}
|
||||
|
||||
function normalizeOptionalBranch(value: unknown, fieldName: string): string | undefined {
|
||||
@@ -133,7 +134,10 @@ export function resolveBranchSelection(
|
||||
|
||||
export function resolveBranchAssignmentContext(input: unknown): ResolvedBranchAssignmentContext {
|
||||
if (input === undefined || input === null) {
|
||||
return { mode: "shared" };
|
||||
// No explicit assignment requested: leave mode undefined so callers can
|
||||
// apply their own default (e.g. mission triage falls back to the
|
||||
// mission's branchStrategy instead of being forced into a shared group).
|
||||
return { mode: undefined };
|
||||
}
|
||||
if (typeof input !== "object" || Array.isArray(input)) {
|
||||
throw badRequest("branchAssignment must be an object");
|
||||
@@ -143,9 +147,7 @@ export function resolveBranchAssignmentContext(input: unknown): ResolvedBranchAs
|
||||
if (mode !== undefined && mode !== "shared" && mode !== "per-task-derived") {
|
||||
throw badRequest("branchAssignment.mode must be one of: shared, per-task-derived");
|
||||
}
|
||||
return {
|
||||
mode: mode === "per-task-derived" ? "per-task-derived" : "shared",
|
||||
};
|
||||
return { mode };
|
||||
}
|
||||
|
||||
export function sanitizeSegment(input: string): string {
|
||||
|
||||
@@ -238,7 +238,8 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann
|
||||
|
||||
const { branch: resolvedBranch, baseBranch: resolvedBaseBranch } =
|
||||
resolveBranchSelection(branchSelection, branch, baseBranch);
|
||||
const { mode: branchMode } = resolveBranchAssignmentContext(branchAssignment);
|
||||
// Planning subtasks have no strategy fallback; keep the historical shared default.
|
||||
const { mode: branchMode = "shared" } = resolveBranchAssignmentContext(branchAssignment);
|
||||
// Stamp the real BranchGroup id (BG-…) so listTasksByBranchGroup(group.id)
|
||||
// resolves members. The group is only ensured (and the id set) in shared
|
||||
// mode below. Non-shared members get NO groupId — stamping a synthetic
|
||||
@@ -1323,7 +1324,8 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann
|
||||
|
||||
const { branch: resolvedBranch, baseBranch: resolvedBaseBranch } =
|
||||
resolveBranchSelection(branchSelection, branch, baseBranch);
|
||||
const { mode: branchMode } = resolveBranchAssignmentContext(branchAssignment);
|
||||
// Planning subtasks have no strategy fallback; keep the historical shared default.
|
||||
const { mode: branchMode = "shared" } = resolveBranchAssignmentContext(branchAssignment);
|
||||
// Stamp the real BranchGroup id (BG-…) so listTasksByBranchGroup(group.id)
|
||||
// resolves members. The group is only ensured (and the id set) in shared
|
||||
// mode below. Non-shared members get NO groupId — stamping a synthetic
|
||||
|
||||
@@ -823,6 +823,56 @@ describe("MissionExecutionLoop", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("skips validation when the feature's mission is not active", async () => {
|
||||
missionStore._setMission(createMockMission({ id: "M-TEST1", status: "planning" }));
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001" });
|
||||
missionStore._setFeature(feature);
|
||||
missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature);
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
missionStore: missionStore as any,
|
||||
rootDir: "/tmp",
|
||||
});
|
||||
loop.start();
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
expect(missionStore.startValidatorRun).not.toHaveBeenCalled();
|
||||
expect(missionStore.logMissionEvent).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
"warning",
|
||||
expect.stringContaining("Validation skipped"),
|
||||
expect.objectContaining({
|
||||
code: "validation_skipped_mission_inactive",
|
||||
featureId: "F-001",
|
||||
taskId: "FN-001",
|
||||
missionId: "M-TEST1",
|
||||
missionStatus: "planning",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("validates when the feature's mission is active", async () => {
|
||||
missionStore._setMission(createMockMission({ id: "M-TEST1", status: "active" }));
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001" });
|
||||
missionStore._setFeature(feature);
|
||||
missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] });
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
missionStore: missionStore as any,
|
||||
rootDir: "/tmp",
|
||||
});
|
||||
vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" });
|
||||
loop.start();
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion");
|
||||
});
|
||||
|
||||
it("requeues needs_fix features back through validation", async () => {
|
||||
const assertions = makeAssertions(1);
|
||||
const response = JSON.stringify({
|
||||
|
||||
@@ -21,6 +21,7 @@ import type {
|
||||
AgentStore,
|
||||
Settings,
|
||||
Milestone,
|
||||
Mission,
|
||||
} from "@fusion/core";
|
||||
import { normalizeMissionAssertionType } from "@fusion/core";
|
||||
import type { VerificationOutcome } from "./mission-verification.js";
|
||||
@@ -417,6 +418,21 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
return;
|
||||
}
|
||||
|
||||
// Only validate features of active missions — mirrors the
|
||||
// recoverActiveMissions guard. A parked/blocked/completed mission must
|
||||
// not keep minting validations (and Fix features) for completed tasks.
|
||||
// Features that don't resolve to a mission keep the current behavior.
|
||||
const mission = this.resolveFeatureMission(feature);
|
||||
if (mission && mission.status !== "active") {
|
||||
loopLog.log(`Feature ${feature.id} belongs to mission ${mission.id} with status "${mission.status}"; skipping validation`);
|
||||
this.logFeatureWarningEvent(feature.id, "validation_skipped_mission_inactive", `Validation skipped: mission ${mission.id} status is "${mission.status}" (expected "active").`, {
|
||||
taskId,
|
||||
missionId: mission.id,
|
||||
missionStatus: mission.status,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (feature.loopState === "needs_fix") {
|
||||
this.missionStore.transitionLoopState(feature.id, "implementing");
|
||||
feature.loopState = "implementing";
|
||||
@@ -1189,6 +1205,15 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
|
||||
return this.missionStore.getMilestone(slice.milestoneId);
|
||||
}
|
||||
|
||||
private resolveFeatureMission(feature: MissionFeature): Mission | undefined {
|
||||
const milestone = this.resolveFeatureMilestone(feature);
|
||||
if (!milestone) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return this.missionStore.getMission(milestone.missionId);
|
||||
}
|
||||
|
||||
private completeValidatorRunIfStillRunning(
|
||||
runId: string | undefined,
|
||||
status: "passed" | "failed" | "blocked" | "error",
|
||||
|
||||
Reference in New Issue
Block a user