fix(smithy): sf task merge honors workspace merge.targetBranch config (#63)

.changeset/sf-task-merge-target-branch-config-fallback.md

@@ -0,0 +1,9 @@
+---
+"@stoneforge/smithy": patch
+---
+
+fix(smithy): `sf task merge` honors workspace `merge.targetBranch` config (#63)
+
+`sf task merge`, `sf task complete`, `sf task sync`, and `sf task update --status merged` previously resolved the merge target as `task.targetBranch → git origin/HEAD → main`, ignoring the workspace-level `merge.targetBranch` value in `.stoneforge/config.yaml`. The config value was only consumed by prompt-builders (sessions, steward scheduler, dispatch daemon) for system-prompt context, so setting `merge.target_branch: dev` told agents the right thing in their prompts but did nothing to the actual git tooling.
+
+Resolution is now `task.targetBranch → workspace config (`merge.targetBranch`) → git origin/HEAD → main` across all merge-related CLI paths and the MR-creation path in `task-assignment-service.ts`. Stewards (which cannot own a `targetBranch` field directly) finally have a config-only path to set a project-wide target.

packages/smithy/src/cli/commands/task-target-branch.bun.test.ts

@@ -0,0 +1,89 @@
+/**
+ * Tests for the workspace `merge.targetBranch` config fallback used by the
+ * `sf task` merge-related commands. Regression for #63: previously, the
+ * config workaround Adam suggested was only consumed by prompt-builders
+ * (sessions/scheduler/dispatch-daemon) for system-prompt context — the actual
+ * git tooling never loaded it, so setting `merge.target_branch` had no effect
+ * on `sf task merge` / `sf task complete` / `sf task update --status merged`.
+ */
+
+import { describe, it, expect, afterEach } from 'bun:test';
+import { mkdtempSync, rmSync, mkdirSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import { loadConfigTargetBranch } from './task.js';
+
+// ============================================================================
+// Helpers
+// ============================================================================
+
+const cleanups: Array<() => void> = [];
+
+afterEach(() => {
+  while (cleanups.length > 0) {
+    const fn = cleanups.pop();
+    try { fn?.(); } catch { /* ignore */ }
+  }
+});
+
+/**
+ * Creates a tmp `.stoneforge` workspace with an optional `config.yaml`,
+ * chdirs into it, and registers cleanup.
+ */
+function chdirIntoTmpWorkspace(yamlContent?: string): string {
+  const root = mkdtempSync(join(tmpdir(), 'sf-target-branch-test-'));
+  mkdirSync(join(root, '.stoneforge'), { recursive: true });
+  if (yamlContent !== undefined) {
+    writeFileSync(join(root, '.stoneforge', 'config.yaml'), yamlContent, 'utf8');
+  }
+  const cwdBefore = process.cwd();
+  process.chdir(root);
+  cleanups.push(() => {
+    process.chdir(cwdBefore);
+    rmSync(root, { recursive: true, force: true });
+  });
+  return root;
+}
+
+// ============================================================================
+// Tests
+// ============================================================================
+
+describe('loadConfigTargetBranch', () => {
+  it('returns the configured value when `merge.target_branch` is set', async () => {
+    chdirIntoTmpWorkspace('merge:\n  target_branch: dev\n');
+    const value = await loadConfigTargetBranch();
+    expect(value).toBe('dev');
+  });
+
+  it('returns undefined when no config file exists', async () => {
+    chdirIntoTmpWorkspace();
+    const value = await loadConfigTargetBranch();
+    expect(value).toBeUndefined();
+  });
+
+  it('returns undefined when config exists but `merge` block is absent', async () => {
+    chdirIntoTmpWorkspace('logging:\n  level: info\n');
+    const value = await loadConfigTargetBranch();
+    expect(value).toBeUndefined();
+  });
+
+  it('returns undefined when `merge.target_branch` is null', async () => {
+    chdirIntoTmpWorkspace('merge:\n  target_branch: null\n');
+    const value = await loadConfigTargetBranch();
+    expect(value).toBeUndefined();
+  });
+
+  it('returns undefined when `merge.target_branch` is an empty string', async () => {
+    chdirIntoTmpWorkspace('merge:\n  target_branch: ""\n');
+    const value = await loadConfigTargetBranch();
+    expect(value).toBeUndefined();
+  });
+
+  it('preserves non-default branch names (e.g. "develop", "release/v2")', async () => {
+    chdirIntoTmpWorkspace('merge:\n  target_branch: release/v2\n');
+    const value = await loadConfigTargetBranch();
+    expect(value).toBe('release/v2');
+  });
+});

packages/smithy/src/cli/commands/task.ts

@@ -18,6 +18,28 @@ import { TaskStatus, createTimestamp } from '@stoneforge/core';
 // Shared Helpers
 // ============================================================================
 
+/**
+ * Loads the workspace-level `merge.targetBranch` from `.stoneforge/config.yaml`,
+ * if set. Returns `undefined` if config is missing, `merge` is unset, or the
+ * value is null/empty.
+ *
+ * This is the second-priority fallback when resolving an effective target
+ * branch for merge operations: task metadata wins, then this config value,
+ * then git origin/HEAD detection, then the hardcoded `main` default.
+ *
+ * Exported so callers (and tests) can reuse the same precedence rule.
+ */
+export async function loadConfigTargetBranch(): Promise<string | undefined> {
+  try {
+    const { loadConfig } = await import('@stoneforge/quarry');
+    const config = loadConfig();
+    const value = (config.merge as { targetBranch?: string | null } | undefined)?.targetBranch;
+    return typeof value === 'string' && value.length > 0 ? value : undefined;
+  } catch {
+    return undefined;
+  }
+}
+
 /**
  * Creates task assignment service
  */
@@ -414,7 +436,12 @@ async function taskMergeHandler(
     const { getOrchestratorTaskMeta, updateOrchestratorTaskMeta } = await import('../../types/task-meta.js');
     const orchestratorMeta = getOrchestratorTaskMeta(task.metadata as Record<string, unknown>);
     const sourceBranch = orchestratorMeta?.branch;
-    const targetBranch = orchestratorMeta?.targetBranch;
+    // Resolution: task metadata → workspace config → git detection (in mergeBranch).
+    // Workspace config (`merge.targetBranch` in `.stoneforge/config.yaml`) lets
+    // operators set a project-wide target without per-task or per-director
+    // metadata, which is the only path stewards have today since they cannot
+    // own a `targetBranch` field directly.
+    const targetBranch = orchestratorMeta?.targetBranch ?? await loadConfigTargetBranch();
 
     if (!sourceBranch) {
       return failure(`Task ${taskId} has no branch in orchestrator metadata.`, ExitCode.GENERAL_ERROR);
@@ -457,6 +484,8 @@ async function taskMergeHandler(
     const { exec: execCb } = await import('node:child_process');
     const { promisify: promisifyUtil } = await import('node:util');
     const execVerify = promisifyUtil(execCb);
+    // `targetBranch` already includes the workspace config fallback (see resolution comment above);
+    // detectTargetBranch is the final fallback to git origin/HEAD when neither is set.
     const effectiveTargetForVerify = targetBranch ?? await detectTargetBranch(workspaceRoot);
 
     try {
@@ -864,9 +893,10 @@ async function taskSyncHandler(
       return failure(syncResult.message, ExitCode.GENERAL_ERROR);
     }
 
-    // Use the task's targetBranch if set, otherwise fall back to default branch
-    const targetBranch = orchestratorMeta?.targetBranch as string | undefined;
-    const syncBranch = targetBranch ?? await worktreeManager.getDefaultBranch();
+    // Use the task's targetBranch if set, then workspace config `merge.targetBranch`,
+    // otherwise fall back to the worktree's default branch.
+    const taskTargetBranch = orchestratorMeta?.targetBranch as string | undefined;
+    const syncBranch = taskTargetBranch ?? await loadConfigTargetBranch() ?? await worktreeManager.getDefaultBranch();
     const remoteBranch = `origin/${syncBranch}`;
 
     // Attempt to merge
@@ -1162,9 +1192,11 @@ async function taskMergeStatusHandler(
             } catch { /* no remote */ }
 
             if (hasRemote) {
-              // Determine effective target branch
+              // Determine effective target branch:
+              // task metadata → workspace config → git origin/HEAD → "main"
               const { detectTargetBranch } = await import('../../git/merge.js');
-              const effectiveTarget = targetBranch ?? await detectTargetBranch(workspaceRoot);
+              const configTarget = await loadConfigTargetBranch();
+              const effectiveTarget = targetBranch ?? configTarget ?? await detectTargetBranch(workspaceRoot);
               const mergeCommitHash = orchestratorMeta?.mergeCommitHash;
 
               const result = await verifyMergeStatus({

packages/smithy/src/services/task-assignment-service.ts

@@ -564,7 +564,22 @@ export class TaskAssignmentServiceImpl implements TaskAssignmentService {
     let mergeRequestId: number | undefined;
 
     if (branch && this.mergeRequestProvider && options?.createMergeRequest !== false) {
-      const baseBranch = options?.baseBranch || currentMeta?.targetBranch || 'main';
+      // Resolution order: explicit --baseBranch → task's targetBranch metadata →
+      // workspace config (`merge.targetBranch`) → hardcoded `main`. Without the
+      // config layer, stewards (which cannot own a `targetBranch` field) had no
+      // way to set a project-wide target.
+      let configTargetBranch: string | undefined;
+      try {
+        const { loadConfig } = await import('@stoneforge/quarry');
+        const cfg = loadConfig();
+        const value = (cfg.merge as { targetBranch?: string | null } | undefined)?.targetBranch;
+        if (typeof value === 'string' && value.length > 0) {
+          configTargetBranch = value;
+        }
+      } catch {
+        // Config not loadable — fall through to hardcoded default
+      }
+      const baseBranch = options?.baseBranch || currentMeta?.targetBranch || configTargetBranch || 'main';
       let body = `## Task\n\n**ID:** ${task.id}\n**Title:** ${task.title}\n\n`;
       if (options?.summary) {
         body += `## Summary\n\n${options.summary}\n\n`;