refactor: committer as pure agent role with discriminated union meta (#17)

- Remove hardcoded git commands (git-exec.ts deleted)
- CommitterMeta is now a discriminated union: committed | failed
- Agent handles git operations, committer extracts structured result
- Moderator can route on meta.status for retry logic

Closes #17
This commit is contained in:
2026-05-06 10:06:27 +00:00
parent c15a5554c0
commit 2c642b1a53
7 changed files with 148 additions and 186 deletions
@@ -1,40 +1,10 @@
import { describe, expect, spyOn, test } from "bun:test"; import { describe, expect, spyOn, test } from "bun:test";
import { execFile } from "node:child_process";
import { appendFile, mkdir, mkdtemp, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { promisify } from "node:util";
import type { AgentFn, ThreadContext } from "@uncaged/workflow"; import type { AgentFn, ThreadContext } from "@uncaged/workflow";
import { START } from "@uncaged/workflow"; import { START } from "@uncaged/workflow";
import * as utilRole from "@uncaged/workflow-util-role"; import * as utilRole from "@uncaged/workflow-util-role";
import { createCommitterRole } from "../src/committer.js"; import { createCommitterRole } from "../src/committer.js";
import { gitExec } from "../src/git-exec.js";
const execFileAsync = promisify(execFile);
async function git(repo: string, args: string[]): Promise<void> {
await gitExec(repo, args);
}
async function setupRepoWithRemote(): Promise<{ repo: string }> {
const base = await mkdtemp(join(tmpdir(), "wf-committer-"));
const bare = join(base, "origin.git");
const repo = join(base, "work");
await mkdir(repo, { recursive: true });
await mkdir(bare, { recursive: true });
await execFileAsync("git", ["init"], { cwd: repo, encoding: "utf8" });
await git(repo, ["config", "user.email", "t@t"]);
await git(repo, ["config", "user.name", "t"]);
await writeFile(join(repo, "README.md"), "# hi\n", "utf8");
await git(repo, ["add", "README.md"]);
await git(repo, ["commit", "-m", "init"]);
await execFileAsync("git", ["init", "--bare"], { cwd: bare, encoding: "utf8" });
await git(repo, ["remote", "add", "origin", bare]);
await git(repo, ["push", "-u", "origin", "HEAD"]);
return { repo };
}
function makeCtx(): ThreadContext { function makeCtx(): ThreadContext {
return { return {
@@ -50,21 +20,13 @@ function makeCtx(): ThreadContext {
const provider = { baseUrl: "https://example.com/v1", apiKey: "k", model: "m" }; const provider = { baseUrl: "https://example.com/v1", apiKey: "k", model: "m" };
describe("createCommitterRole", () => { const dryRunMeta = {
test("returns committed false when working tree clean", async () => { status: "committed" as const,
const { repo } = await setupRepoWithRemote(); branch: "dry-run/placeholder",
const agent: AgentFn = async () => { commitSha: "0000000",
throw new Error("agent should not run");
}; };
const role = createCommitterRole(
agent,
{ provider, dryRun: null, dryRunMeta: { branch: "dry-run", message: "chore: dry run" } },
{ cwd: repo, remote: "origin", threadId: null },
);
const out = await role(makeCtx());
expect(out.meta.committed).toBe(false);
});
describe("createCommitterRole", () => {
test("dry-run skips pipeline", async () => { test("dry-run skips pipeline", async () => {
const agent: AgentFn = async () => { const agent: AgentFn = async () => {
throw new Error("agent should not run"); throw new Error("agent should not run");
@@ -72,39 +34,101 @@ describe("createCommitterRole", () => {
const role = createCommitterRole(agent, { const role = createCommitterRole(agent, {
provider, provider,
dryRun: true, dryRun: true,
dryRunMeta: { branch: "dry-run", message: "chore: dry run" }, dryRunMeta,
}); });
const out = await role(makeCtx()); const out = await role(makeCtx());
expect(out.content).toBe("[dry-run] committer skipped"); expect(out.content).toBe("[dry-run] committer skipped");
expect(out.meta).toEqual({ committed: true }); expect(out.meta).toEqual(dryRunMeta);
}); });
test("commits and pushes when changes exist", async () => { test("returns committed meta when extraction succeeds", async () => {
const { repo } = await setupRepoWithRemote(); const committed = {
await appendFile(join(repo, "README.md"), "\nmore\n", "utf8"); status: "committed" as const,
branch: "feat/widget",
commitSha: "deadbeef".repeat(5).slice(0, 40),
};
const spy = spyOn(utilRole, "extractMetaOrThrow").mockResolvedValue({ const spy = spyOn(utilRole, "extractMetaOrThrow").mockResolvedValue(committed);
branch: "feat/test-commit",
message: "feat: add more", const agent: AgentFn = async (_ctx, prompt) =>
`Created branch ${committed.branch}, pushed. SHA ${committed.commitSha}.\n${prompt.slice(0, 80)}`;
const role = createCommitterRole(agent, {
provider,
dryRun: null,
dryRunMeta,
}); });
const agent: AgentFn = async () => "plan text";
const role = createCommitterRole(
agent,
{ provider, dryRun: null, dryRunMeta: { branch: "dry-run", message: "chore: dry run" } },
{ cwd: repo, remote: "origin", threadId: null },
);
const out = await role(makeCtx()); const out = await role(makeCtx());
expect(out.meta.committed).toBe(true); expect(out.meta).toEqual(committed);
expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalled();
const branches = await gitExec(repo, ["branch", "--list", "feat/test-commit"]);
expect(branches).toContain("feat/test-commit");
const remoteRefs = await gitExec(repo, ["ls-remote", "--heads", "origin", "feat/test-commit"]);
expect(remoteRefs.trim().length).toBeGreaterThan(0);
spy.mockRestore(); spy.mockRestore();
}); });
test("returns failed meta when extraction reports failure", async () => {
const failed = {
status: "failed" as const,
error: "working tree clean; nothing to commit",
logRef: null as string | null,
};
const spy = spyOn(utilRole, "extractMetaOrThrow").mockResolvedValue(failed);
const agent: AgentFn = async () => "git status shows no changes; skipping branch and commit.";
const role = createCommitterRole(agent, {
provider,
dryRun: null,
dryRunMeta,
});
const out = await role(makeCtx());
expect(out.meta).toEqual(failed);
expect(spy).toHaveBeenCalled();
spy.mockRestore();
});
test("returns failed meta with logRef when extraction includes it", async () => {
const failed = {
status: "failed" as const,
error: "push rejected",
logRef: "LOGREF01",
};
spyOn(utilRole, "extractMetaOrThrow").mockResolvedValue(failed);
const agent: AgentFn = async () => "Remote rejected non-fast-forward.";
const role = createCommitterRole(agent, {
provider,
dryRun: null,
dryRunMeta,
});
const out = await role(makeCtx());
expect(out.meta).toEqual(failed);
});
test("onFail wraps extraction errors", async () => {
spyOn(utilRole, "extractMetaOrThrow").mockRejectedValue(
new Error("structured extraction failed"),
);
const agent: AgentFn = async () => "opaque agent output";
const role = createCommitterRole(agent, {
provider,
dryRun: null,
dryRunMeta,
});
const out = await role(makeCtx());
expect(out.meta).toEqual({
status: "failed",
error: "committer role threw before structured result",
logRef: null,
});
expect(out.content).toContain("committer failed:");
expect(out.content).toContain("structured extraction failed");
});
}); });
@@ -10,7 +10,6 @@
}, },
"dependencies": { "dependencies": {
"@uncaged/workflow": "workspace:*", "@uncaged/workflow": "workspace:*",
"@uncaged/workflow-util-agent": "workspace:*",
"@uncaged/workflow-util-role": "workspace:*", "@uncaged/workflow-util-role": "workspace:*",
"zod": "^4.0.0" "zod": "^4.0.0"
}, },
@@ -1,29 +1,28 @@
import type { AgentFn, Role, RoleResult, ThreadContext } from "@uncaged/workflow"; import type { AgentFn, Role, ThreadContext } from "@uncaged/workflow";
import { import {
createRole,
decorateRole, decorateRole,
extractMetaOrThrow,
type LlmProvider, type LlmProvider,
onFail, onFail,
withDryRun, withDryRun,
} from "@uncaged/workflow-util-role"; } from "@uncaged/workflow-util-role";
import * as z from "zod/v4"; import * as z from "zod/v4";
import { gitExec } from "./git-exec.js"; export const committerMetaSchema = z.discriminatedUnion("status", [
z.object({
status: z.literal("committed"),
branch: z.string(),
commitSha: z.string(),
}),
z.object({
status: z.literal("failed"),
error: z.string(),
logRef: z.string().nullable(),
}),
]);
export const committerMetaSchema = z.object({
committed: z
.boolean()
.describe("true if branch created, changes committed, and pushed successfully"),
});
export type CommitterMeta = z.infer<typeof committerMetaSchema>; export type CommitterMeta = z.infer<typeof committerMetaSchema>;
const committerPlanSchema = z.object({
branch: z.string().describe("Feature branch name, e.g. feat/slug or fix/slug"),
message: z.string().describe("Single-line conventional commit subject"),
});
export type CommitterPlanMeta = z.infer<typeof committerPlanSchema>;
export type CommitterGitConfig = { export type CommitterGitConfig = {
cwd: string; cwd: string;
remote: string; remote: string;
@@ -37,6 +36,12 @@ export const DEFAULT_COMMITTER_GIT_CONFIG: CommitterGitConfig = {
threadId: null, threadId: null,
}; };
const DRY_RUN_COMMITTED_META: CommitterMeta = {
status: "committed",
branch: "dry-run/placeholder",
commitSha: "0000000",
};
function resolveExtractDryRun(extractDryRun: boolean | null): boolean { function resolveExtractDryRun(extractDryRun: boolean | null): boolean {
return extractDryRun === true; return extractDryRun === true;
} }
@@ -50,37 +55,18 @@ function summarizeThreadContext(ctx: ThreadContext): string {
return lines.join("\n"); return lines.join("\n");
} }
function sanitizeBranch(branch: string): string { function committerSystemPrompt(ctx: ThreadContext, gitConfig: CommitterGitConfig): string {
const t = branch.trim();
if (
t === "" ||
t.includes("..") ||
t.includes(" ") ||
t.startsWith("-") ||
t.includes("\n") ||
t.includes("\t")
) {
throw new Error(`invalid branch name: ${branch}`);
}
return t;
}
function sanitizeCommitMessage(message: string): string {
const line = message.trim().split(/\r?\n/)[0] ?? "";
if (line === "") {
throw new Error("commit message is empty");
}
return line;
}
function committerPlanPrompt(ctx: ThreadContext, gitConfig: CommitterGitConfig): string {
const threadLine = const threadLine =
gitConfig.threadId !== null gitConfig.threadId !== null
? `Optional CLI context: run \`uncaged-workflow thread ${gitConfig.threadId}\` if available.\n` ? `Optional CLI context: run \`uncaged-workflow thread ${gitConfig.threadId}\` if available.\n`
: ""; : "";
return `You plan a git branch and a single-line conventional commit message for the following workflow thread. return `You are the **git committer** for this workflow. Prior roles planned, implemented, and reviewed the change; your job is to perform git operations in the repository and report the outcome.
## Repository context
- Working directory (run git commands here): \`${gitConfig.cwd}\`
- Remote name for push: \`${gitConfig.remote}\`
${threadLine} ${threadLine}
## Thread context ## Thread context
@@ -88,66 +74,44 @@ ${summarizeThreadContext(ctx)}
## Your task ## Your task
Infer a good branch name (\`feat/<slug>\` or \`fix/<slug>\`) and a conventional commit **subject** (one line, no body). 1. Inspect the working tree (e.g. \`git status\`). If there is nothing to commit, stop and explain why in your reply.
2. Create a new branch using **conventional** naming (\`feat/<slug>\`, \`fix/<slug>\`, or \`chore/<slug>\` as appropriate).
3. Stage all intended changes, commit with a **single-line conventional commit subject**, and push the branch to \`${gitConfig.remote}\` (e.g. \`git push -u ${gitConfig.remote} <branch>\`).
4. In your reply, state clearly whether the push succeeded, the **exact branch name** used, and the **full commit SHA** from \`git rev-parse HEAD\` (or explain the failure).
Reply with enough detail that a maintainer understands the change; structured extraction will read \`branch\` and \`message\` from your answer.`; Structured extraction will read \`status\`, branch, commit SHA, or error details from your answer.`;
}
async function runCommitterPipeline(
ctx: ThreadContext,
agent: AgentFn,
extract: { provider: LlmProvider; dryRun: boolean | null; dryRunMeta: CommitterPlanMeta },
gitConfig: CommitterGitConfig,
): Promise<RoleResult<CommitterMeta>> {
const cwd = gitConfig.cwd;
const porcelain = await gitExec(cwd, ["status", "--porcelain"]);
if (porcelain.trim() === "") {
return {
content: "Working tree clean; nothing to commit.",
meta: { committed: false },
};
}
const prompt = committerPlanPrompt(ctx, gitConfig);
const raw = await agent(ctx, prompt);
const plan = await extractMetaOrThrow("committer-plan", raw, committerPlanSchema, {
provider: extract.provider,
dryRun: resolveExtractDryRun(extract.dryRun),
dryRunMeta: extract.dryRunMeta,
});
const branch = sanitizeBranch(plan.branch);
const message = sanitizeCommitMessage(plan.message);
await gitExec(cwd, ["checkout", "-b", branch]);
await gitExec(cwd, ["add", "-A"]);
await gitExec(cwd, ["commit", "-m", message]);
await gitExec(cwd, ["push", "-u", gitConfig.remote, branch]);
return {
content: raw,
meta: { committed: true },
};
} }
/** /**
* Git committer role: LLM proposes branch + message; this package runs git via `child_process`. * Git committer role: the agent runs git (branch, commit, push); structured extraction yields {@link CommitterMeta}.
* Decorators match nerve semantics: dry-run skips work with `committed: true`; failures yield `committed: false`. * Dry-run skips the agent and returns a stable committed placeholder; unexpected throws yield \`status: "failed"\`.
*/ */
export function createCommitterRole( export function createCommitterRole(
adapter: AgentFn, adapter: AgentFn,
extract: { provider: LlmProvider; dryRun: boolean | null; dryRunMeta: CommitterPlanMeta }, extract: { provider: LlmProvider; dryRun: boolean | null; dryRunMeta: CommitterMeta },
gitConfig: CommitterGitConfig = DEFAULT_COMMITTER_GIT_CONFIG, gitConfig: CommitterGitConfig = DEFAULT_COMMITTER_GIT_CONFIG,
): Role<CommitterMeta> { ): Role<CommitterMeta> {
const inner: Role<CommitterMeta> = async (ctx) => const inner: Role<CommitterMeta> = createRole({
runCommitterPipeline(ctx, adapter, extract, gitConfig); name: "committer",
schema: committerMetaSchema,
systemPrompt: async (ctx) => committerSystemPrompt(ctx, gitConfig),
agent: adapter,
extract,
});
return decorateRole(inner, [ return decorateRole(inner, [
withDryRun<CommitterMeta>({ withDryRun<CommitterMeta>({
label: "committer", label: "committer",
meta: { committed: true }, meta: DRY_RUN_COMMITTED_META,
dryRun: resolveExtractDryRun(extract.dryRun), dryRun: resolveExtractDryRun(extract.dryRun),
}), }),
onFail<CommitterMeta>({ label: "committer", meta: { committed: false } }), onFail<CommitterMeta>({
label: "committer",
meta: {
status: "failed",
error: "committer role threw before structured result",
logRef: null,
},
}),
]); ]);
} }
@@ -1,20 +0,0 @@
import { spawnCli } from "@uncaged/workflow-util-agent";
/** Runs `git` with args in `cwd`; throws if git exits non-zero. */
export async function gitExec(cwd: string, args: readonly string[]): Promise<string> {
const result = await spawnCli("git", [...args], { cwd, timeoutMs: null });
if (result.ok) {
return result.value;
}
const error = result.error;
switch (error.kind) {
case "non_zero_exit":
throw new Error(
`git ${args.join(" ")} failed (exit ${error.exitCode}): ${error.stderr.trim() || error.stdout.trim()}`,
);
case "timeout":
throw new Error(`git ${args.join(" ")} timed out`);
case "spawn_failed":
throw new Error(`git ${args.join(" ")} spawn failed: ${error.message}`);
}
}
@@ -1,9 +1,7 @@
export { export {
type CommitterGitConfig, type CommitterGitConfig,
type CommitterMeta, type CommitterMeta,
type CommitterPlanMeta,
committerMetaSchema, committerMetaSchema,
createCommitterRole, createCommitterRole,
DEFAULT_COMMITTER_GIT_CONFIG, DEFAULT_COMMITTER_GIT_CONFIG,
} from "./committer.js"; } from "./committer.js";
export { gitExec } from "./git-exec.js";
@@ -61,7 +61,7 @@ function committerStep(): RoleStep<SolveIssueMeta> {
return { return {
role: "committer", role: "committer",
content: "commit", content: "commit",
meta: { committed: true }, meta: { status: "committed", branch: "feat/issue-1", commitSha: "abc1234" },
timestamp: 4, timestamp: 4,
}; };
} }
@@ -1,9 +1,5 @@
import type { AgentFn, Role } from "@uncaged/workflow"; import type { AgentFn, Role } from "@uncaged/workflow";
import { import { type CommitterMeta, createCommitterRole } from "@uncaged/workflow-role-committer";
type CommitterMeta,
type CommitterPlanMeta,
createCommitterRole,
} from "@uncaged/workflow-role-committer";
import { createRole } from "@uncaged/workflow-role-llm"; import { createRole } from "@uncaged/workflow-role-llm";
import { createReviewerRole, type ReviewerMeta } from "@uncaged/workflow-role-reviewer"; import { createReviewerRole, type ReviewerMeta } from "@uncaged/workflow-role-reviewer";
import type { LlmProvider } from "@uncaged/workflow-util-role"; import type { LlmProvider } from "@uncaged/workflow-util-role";
@@ -53,9 +49,10 @@ const REVIEWER_DRY_RUN_META: ReviewerMeta = {
approved: true, approved: true,
}; };
const COMMITTER_PLAN_DRY_RUN_META: CommitterPlanMeta = { const COMMITTER_DRY_RUN_META: CommitterMeta = {
branch: "dry-run", status: "committed",
message: "chore: dry run", branch: "dry-run/placeholder",
commitSha: "0000000",
}; };
export type SolveIssueMeta = { export type SolveIssueMeta = {
@@ -142,7 +139,7 @@ export function createSolveIssueRoles(config: SolveIssueRolesConfig): SolveIssue
{ {
provider: extract.provider, provider: extract.provider,
dryRun: extract.dryRun, dryRun: extract.dryRun,
dryRunMeta: COMMITTER_PLAN_DRY_RUN_META, dryRunMeta: COMMITTER_DRY_RUN_META,
}, },
committerGit, committerGit,
); );