docs(FN-3421): document branch field plumbing architecture
- Document how branch field data flows through task lifecycle - Clarify architecture-level plumbing responsibilities for branch metadata - Capture Step 4 documentation updates in docs/architecture.md Fusion-Task-Id: FN-3421
This commit is contained in:
@@ -933,6 +933,17 @@ Behavior summary:
|
||||
|
||||
`TaskStore.updateTask()` applies this guard before persisting `nodeId` changes.
|
||||
|
||||
### Task branch field plumbing (`branch` + `baseBranch`)
|
||||
|
||||
Task create/update now preserves both branch fields end-to-end:
|
||||
- **Request validation/normalization (dashboard route layer):** `packages/dashboard/src/routes/register-task-workflow-routes.ts`
|
||||
- `POST /api/tasks` accepts `branch` and `baseBranch` as string values.
|
||||
- `PATCH /api/tasks/:id` accepts `branch` and `baseBranch` as `string | null` for PATCH-style updates, trims string inputs, and treats empty strings as clears (`null`).
|
||||
- Route handlers reject non-string/non-null payloads with `400`.
|
||||
- **Durable persistence (core store layer):** `packages/core/src/store.ts`
|
||||
- `TaskStore.createTask()` persists both `branch` and `baseBranch` on task creation.
|
||||
- `TaskStore.updateTask()` preserves existing PATCH semantics where explicit `null` clears either field.
|
||||
- Fields round-trip through JSON and SQLite persistence via the shared task contract in `packages/core/src/types.ts`.
|
||||
|
||||
### Routing activity visibility
|
||||
|
||||
|
||||
@@ -530,6 +530,69 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("branch field persistence", () => {
|
||||
it("persists baseBranch and branch when provided at create time", async () => {
|
||||
const task = await store.createTask({
|
||||
description: "Branch fields on create",
|
||||
baseBranch: "main",
|
||||
branch: "fusion/fn-001-custom",
|
||||
});
|
||||
|
||||
expect(task.baseBranch).toBe("main");
|
||||
expect(task.branch).toBe("fusion/fn-001-custom");
|
||||
|
||||
const detail = await store.getTask(task.id);
|
||||
expect(detail.baseBranch).toBe("main");
|
||||
expect(detail.branch).toBe("fusion/fn-001-custom");
|
||||
});
|
||||
|
||||
it("updates and clears branch fields via null patch semantics", async () => {
|
||||
const task = await store.createTask({
|
||||
description: "Branch field update",
|
||||
baseBranch: "main",
|
||||
branch: "fusion/fn-001-initial",
|
||||
});
|
||||
|
||||
const updated = await store.updateTask(task.id, {
|
||||
baseBranch: "release/2026.05",
|
||||
branch: "fusion/fn-001-updated",
|
||||
});
|
||||
expect(updated.baseBranch).toBe("release/2026.05");
|
||||
expect(updated.branch).toBe("fusion/fn-001-updated");
|
||||
|
||||
const cleared = await store.updateTask(task.id, {
|
||||
baseBranch: null,
|
||||
branch: null,
|
||||
});
|
||||
expect(cleared.baseBranch).toBeUndefined();
|
||||
expect(cleared.branch).toBeUndefined();
|
||||
});
|
||||
|
||||
it("round-trips branch fields through listTasks and reload", async () => {
|
||||
store.close();
|
||||
store = new TaskStore(rootDir, globalDir);
|
||||
await store.init();
|
||||
|
||||
const created = await store.createTask({
|
||||
description: "Branch field reinit persistence",
|
||||
baseBranch: "develop",
|
||||
branch: "fusion/fn-001-reinit",
|
||||
});
|
||||
|
||||
const listed = (await store.listTasks()).find((task) => task.id === created.id);
|
||||
expect(listed?.baseBranch).toBe("develop");
|
||||
expect(listed?.branch).toBe("fusion/fn-001-reinit");
|
||||
|
||||
store.close();
|
||||
store = new TaskStore(rootDir, globalDir);
|
||||
await store.init();
|
||||
|
||||
const reloaded = await store.getTask(created.id);
|
||||
expect(reloaded.baseBranch).toBe("develop");
|
||||
expect(reloaded.branch).toBe("fusion/fn-001-reinit");
|
||||
});
|
||||
});
|
||||
|
||||
describe("nodeId persistence", () => {
|
||||
it("creates a task with nodeId when provided", async () => {
|
||||
const task = await store.createTask({
|
||||
|
||||
@@ -2326,6 +2326,8 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
thinkingLevel: input.thinkingLevel,
|
||||
reviewLevel: input.reviewLevel,
|
||||
executionMode: input.executionMode,
|
||||
baseBranch: input.baseBranch,
|
||||
branch: input.branch,
|
||||
missionId: input.missionId,
|
||||
sliceId: input.sliceId,
|
||||
steps: [],
|
||||
|
||||
@@ -1083,6 +1083,11 @@ export interface InboxTask {
|
||||
export interface TaskCreateInput {
|
||||
title?: string;
|
||||
description: string;
|
||||
/** Configured merge target/base branch for this task (task intent).
|
||||
* Defaults to the project default branch when omitted. */
|
||||
baseBranch?: string;
|
||||
/** Actual git working branch name used for this task's worktree. */
|
||||
branch?: string;
|
||||
/** Durable source provenance for the originating external issue. */
|
||||
sourceIssue?: TaskSourceIssue;
|
||||
/** Optional persisted aggregate token usage snapshot for task creation/import paths. */
|
||||
|
||||
@@ -441,6 +441,22 @@ describe("updateTask", () => {
|
||||
expect(body).not.toHaveProperty("executionMode");
|
||||
});
|
||||
|
||||
it("sends branch and baseBranch (including null clears) in update payload", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue(mockFetchResponse(true, {
|
||||
...FAKE_TASK,
|
||||
branch: undefined,
|
||||
baseBranch: undefined,
|
||||
}));
|
||||
|
||||
await updateTask("FN-001", { branch: null, baseBranch: "main" });
|
||||
|
||||
expect(globalThis.fetch).toHaveBeenCalledWith("/api/tasks/FN-001", {
|
||||
headers: { "Content-Type": "application/json" },
|
||||
method: "PATCH",
|
||||
body: JSON.stringify({ branch: null, baseBranch: "main" }),
|
||||
});
|
||||
});
|
||||
|
||||
it("sends sourceIssue object when source metadata is provided", async () => {
|
||||
const sourceIssue = {
|
||||
provider: "github",
|
||||
@@ -553,6 +569,25 @@ describe("createTask", () => {
|
||||
expect(body.priority).toBe("urgent");
|
||||
});
|
||||
|
||||
it("serializes branch and baseBranch in create payload", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue(mockFetchResponse(true, {
|
||||
...FAKE_CREATED_TASK,
|
||||
branch: "fusion/fn-branch",
|
||||
baseBranch: "main",
|
||||
}));
|
||||
|
||||
await createTask({
|
||||
description: "Task with branches",
|
||||
branch: "fusion/fn-branch",
|
||||
baseBranch: "main",
|
||||
});
|
||||
|
||||
const call = vi.mocked(globalThis.fetch).mock.calls[0];
|
||||
const body = JSON.parse((call[1] as RequestInit).body as string);
|
||||
expect(body.branch).toBe("fusion/fn-branch");
|
||||
expect(body.baseBranch).toBe("main");
|
||||
});
|
||||
|
||||
it("sends POST with multiple fields including executionMode", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue(mockFetchResponse(true, {
|
||||
...FAKE_CREATED_TASK,
|
||||
|
||||
@@ -269,6 +269,8 @@ export function createTask(input: TaskCreateInput, projectId?: string): Promise<
|
||||
executionMode,
|
||||
priority,
|
||||
source,
|
||||
branch,
|
||||
baseBranch,
|
||||
} = input;
|
||||
|
||||
return api<Task>(withProjectId("/tasks", projectId), {
|
||||
@@ -294,6 +296,8 @@ export function createTask(input: TaskCreateInput, projectId?: string): Promise<
|
||||
executionMode,
|
||||
priority,
|
||||
source,
|
||||
branch,
|
||||
baseBranch,
|
||||
}),
|
||||
});
|
||||
}
|
||||
@@ -318,6 +322,8 @@ export function updateTask(
|
||||
priority?: TaskPriority | null;
|
||||
sourceIssue?: TaskSourceIssue | null;
|
||||
nodeId?: string | null;
|
||||
branch?: string | null;
|
||||
baseBranch?: string | null;
|
||||
},
|
||||
projectId?: string,
|
||||
): Promise<Task> {
|
||||
|
||||
@@ -670,6 +670,51 @@ describe("POST /tasks", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("forwards branch and baseBranch on create", async () => {
|
||||
const createdTask = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
column: "triage",
|
||||
branch: "fusion/fn-branch",
|
||||
baseBranch: "main",
|
||||
};
|
||||
(store.createTask as ReturnType<typeof vi.fn>).mockResolvedValue(createdTask);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks",
|
||||
JSON.stringify({
|
||||
description: "Task with branch fields",
|
||||
branch: " fusion/fn-branch ",
|
||||
baseBranch: " main ",
|
||||
}),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(store.createTask).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
branch: "fusion/fn-branch",
|
||||
baseBranch: "main",
|
||||
}),
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns 400 when create branch payload is not a string", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"POST",
|
||||
"/api/tasks",
|
||||
JSON.stringify({ description: "Task", branch: 10 }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("branch must be a string");
|
||||
expect(store.createTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("forwards model overrides when both provider and id are supplied", async () => {
|
||||
const createdTask = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
@@ -1382,6 +1427,79 @@ describe("POST /tasks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("PATCH /tasks/:id branch fields", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("forwards trimmed branch and baseBranch values", async () => {
|
||||
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
branch: "fusion/fn-123",
|
||||
baseBranch: "main",
|
||||
});
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PATCH",
|
||||
"/api/tasks/FN-001",
|
||||
JSON.stringify({ branch: " fusion/fn-123 ", baseBranch: " main " }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({
|
||||
branch: "fusion/fn-123",
|
||||
baseBranch: "main",
|
||||
}));
|
||||
});
|
||||
|
||||
it("treats empty-string patch values as clears (null)", async () => {
|
||||
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...FAKE_TASK_DETAIL,
|
||||
branch: undefined,
|
||||
baseBranch: undefined,
|
||||
});
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PATCH",
|
||||
"/api/tasks/FN-001",
|
||||
JSON.stringify({ branch: " ", baseBranch: "" }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({
|
||||
branch: null,
|
||||
baseBranch: null,
|
||||
}));
|
||||
});
|
||||
|
||||
it("returns 400 for invalid branch payload types", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PATCH",
|
||||
"/api/tasks/FN-001",
|
||||
JSON.stringify({ branch: 42 }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("branch must be a string or null");
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /subtasks/*", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
|
||||
@@ -95,6 +95,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
executionMode,
|
||||
priority,
|
||||
source,
|
||||
branch,
|
||||
baseBranch,
|
||||
} = req.body;
|
||||
if (!description || typeof description !== "string") {
|
||||
throw badRequest("description is required");
|
||||
@@ -184,6 +186,18 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
? source
|
||||
: { sourceType: "api" as const };
|
||||
|
||||
const validateOptionalBranchString = (value: unknown, fieldName: string): string | undefined => {
|
||||
if (value === undefined || value === null) return undefined;
|
||||
if (typeof value !== "string") {
|
||||
throw badRequest(`${fieldName} must be a string`);
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
return trimmed.length > 0 ? trimmed : undefined;
|
||||
};
|
||||
|
||||
const normalizedBranch = validateOptionalBranchString(branch, "branch");
|
||||
const normalizedBaseBranch = validateOptionalBranchString(baseBranch, "baseBranch");
|
||||
|
||||
const task = await scopedStore.createTask(
|
||||
{
|
||||
title,
|
||||
@@ -205,6 +219,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
executionMode: executionMode || undefined,
|
||||
priority: priority ?? undefined,
|
||||
source: normalizedSource,
|
||||
branch: normalizedBranch,
|
||||
baseBranch: normalizedBaseBranch,
|
||||
},
|
||||
{ onSummarize, settings: { autoSummarizeTitles: settings.autoSummarizeTitles } }
|
||||
);
|
||||
@@ -1383,7 +1399,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
router.patch("/tasks/:id", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore } = await getProjectContext(req);
|
||||
const { title, description, prompt, priority, dependencies, enabledWorkflowSteps, modelProvider, modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, thinkingLevel, assigneeUserId, reviewLevel, executionMode, sourceIssue, nodeId } = req.body;
|
||||
const { title, description, prompt, priority, dependencies, enabledWorkflowSteps, modelProvider, modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, thinkingLevel, assigneeUserId, reviewLevel, executionMode, sourceIssue, nodeId, branch, baseBranch } = req.body;
|
||||
const hasBodyField = (field: string) => Object.prototype.hasOwnProperty.call(req.body, field);
|
||||
|
||||
// Validate model fields are strings or undefined/null
|
||||
@@ -1495,6 +1511,19 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
}
|
||||
}
|
||||
|
||||
const validatePatchBranchField = (value: unknown, fieldName: string): string | null | undefined => {
|
||||
if (value === undefined) return undefined;
|
||||
if (value === null) return null;
|
||||
if (typeof value !== "string") {
|
||||
throw new Error(`${fieldName} must be a string or null`);
|
||||
}
|
||||
const trimmed = value.trim();
|
||||
return trimmed.length > 0 ? trimmed : null;
|
||||
};
|
||||
|
||||
const normalizedBranch = hasBodyField("branch") ? validatePatchBranchField(branch, "branch") : undefined;
|
||||
const normalizedBaseBranch = hasBodyField("baseBranch") ? validatePatchBranchField(baseBranch, "baseBranch") : undefined;
|
||||
|
||||
const updates: Parameters<typeof scopedStore.updateTask>[1] = {};
|
||||
if (title !== undefined) updates.title = title;
|
||||
if (description !== undefined) updates.description = description;
|
||||
@@ -1514,6 +1543,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
if (hasBodyField("executionMode")) updates.executionMode = executionMode === null ? null : executionMode;
|
||||
if (hasBodyField("sourceIssue")) updates.sourceIssue = validatedSourceIssue === undefined ? undefined : validatedSourceIssue;
|
||||
if (hasBodyField("nodeId")) updates.nodeId = validatedNodeId;
|
||||
if (hasBodyField("branch")) updates.branch = normalizedBranch;
|
||||
if (hasBodyField("baseBranch")) updates.baseBranch = normalizedBaseBranch;
|
||||
|
||||
if (hasBodyField("nodeId") && validatedNodeId !== undefined) {
|
||||
const currentTask = await scopedStore.getTask(req.params.id);
|
||||
|
||||
Reference in New Issue
Block a user