diff --git a/src/products/gardener/engine/commands/start.ts b/src/products/gardener/engine/commands/start.ts index 32f0fa9..fec24bb 100644 --- a/src/products/gardener/engine/commands/start.ts +++ b/src/products/gardener/engine/commands/start.ts @@ -15,6 +15,7 @@ import { parseDurationMs, resolveGardenerDir, writeDaemonConfig, + type SyncMode, } from "../daemon/config.js"; import { bootstrapLaunchdJob, @@ -23,7 +24,7 @@ import { supportsLaunchd, } from "../daemon/launchd.js"; -export const START_USAGE = `usage: first-tree gardener start --tree-path --code-repo [--code-repo …] [--gardener-interval 5m] [--sync-interval 1h] [--assign-owners] [--sync-apply] +export const START_USAGE = `usage: first-tree gardener start --tree-path --code-repo [--code-repo …] [--gardener-interval 5m] [--sync-interval 1h] [--assign-owners] [--sync-mode ] [--sync-assignee USER] Bring up the gardener daemon in the background. @@ -32,9 +33,15 @@ The daemon runs two schedules: against the tree path; handles open PR verdicts and merge→tree-issue creation across all configured code repos. - sync-sweep invokes \`gardener sync\` (or \`gardener sync --apply\` - when --sync-apply is set) against the tree path; - detects drift and optionally opens tree PRs. + sync-sweep invokes \`gardener sync\` against the tree path. Its + behavior is controlled by --sync-mode: + detect (default) report drift only, no writes. + apply run \`gardener sync --apply\`; opens one + aggregated tree PR per sweep. + open-issues run \`gardener sync --open-issues\`; opens + one tree issue per drift proposal so + breeze + draft-node can handle each + autonomously. Options: --tree-path Required. Local checkout of the bound tree repo. @@ -47,8 +54,17 @@ Options: --assign-owners Pass --assign-owners to gardener comment so merge→tree-issue creations auto-assign NODE owners. - --sync-apply Run the sync sweep in --apply mode - (opens tree PRs). Default: detect only. + --sync-mode Sync-sweep behavior: detect | apply | + open-issues. Default: detect. + --sync-assignee With --sync-mode open-issues: override + NODE.md-resolved owners and assign every + opened issue to this user. Intended for + testing against third-party repos where + you don't want to ping real owners. Ignored + in other modes. + --sync-apply Deprecated alias for --sync-mode apply. + Prints a warning and maps to apply mode. + Will be removed in a future minor. --help, -h Show this help. Environment: @@ -89,7 +105,17 @@ interface ParsedStartFlags { gardenerInterval?: string; syncInterval?: string; assignOwners: boolean; - syncApply: boolean; + /** + * Explicit `--sync-mode` value, if provided. When both this and + * `syncApply` are set, `--sync-mode` wins (and we emit a conflict + * error in `runStart`). + */ + syncMode?: SyncMode; + /** Raw string for surfacing "invalid value" errors to the user. */ + syncModeRaw?: string; + /** Tracks `--sync-apply` for deprecation warning + conflict detection. */ + syncApplyLegacy: boolean; + syncAssignee?: string; dryRun: boolean; unknown?: string; } @@ -99,7 +125,7 @@ function parseStartFlags(args: readonly string[]): ParsedStartFlags { help: false, codeRepos: [], assignOwners: false, - syncApply: false, + syncApplyLegacy: false, dryRun: false, }; for (let i = 0; i < args.length; i += 1) { @@ -107,7 +133,27 @@ function parseStartFlags(args: readonly string[]): ParsedStartFlags { if (a === "--help" || a === "-h") { out.help = true; continue; } if (a === "--dry-run") { out.dryRun = true; continue; } if (a === "--assign-owners") { out.assignOwners = true; continue; } - if (a === "--sync-apply") { out.syncApply = true; continue; } + if (a === "--sync-apply") { out.syncApplyLegacy = true; continue; } + if (a === "--sync-mode") { + const val = args[++i]; + out.syncModeRaw = val; + if (val === "detect" || val === "apply" || val === "open-issues") { + out.syncMode = val; + } else { + out.unknown = `--sync-mode: expected one of detect|apply|open-issues (got "${val ?? ""}")`; + return out; + } + continue; + } + if (a === "--sync-assignee") { + const val = args[++i]; + if (typeof val !== "string" || val.length === 0) { + out.unknown = "--sync-assignee requires a GitHub login"; + return out; + } + out.syncAssignee = val; + continue; + } if (a === "--tree-path") { out.treePath = args[++i]; continue; } if (a === "--code-repo") { const val = args[++i]; @@ -167,13 +213,38 @@ export async function runStart( return 1; } + // Resolve --sync-mode vs --sync-apply. --sync-mode is authoritative; + // --sync-apply is a deprecated alias. Reject conflicting combinations + // rather than guessing. + if (flags.syncMode && flags.syncApplyLegacy) { + write( + "--sync-apply and --sync-mode are mutually exclusive (--sync-apply is the deprecated alias).", + ); + return 1; + } + if (flags.syncApplyLegacy) { + write( + "warning: --sync-apply is deprecated. Use --sync-mode apply instead (will be removed in a future minor).", + ); + } + const syncMode: SyncMode = + flags.syncMode ?? (flags.syncApplyLegacy ? "apply" : "detect"); + + if (flags.syncAssignee && syncMode !== "open-issues") { + write( + `--sync-assignee is only valid with --sync-mode open-issues (got --sync-mode ${syncMode}).`, + ); + return 1; + } + const config = buildDaemonConfig({ treePath: flags.treePath, codeRepos: flags.codeRepos, gardenerIntervalMs: gardenerIntervalMs ?? undefined, syncIntervalMs: syncIntervalMs ?? undefined, assignOwners: flags.assignOwners, - syncApply: flags.syncApply, + syncMode, + syncAssignee: flags.syncAssignee, }); const configFilePath = flags.dryRun @@ -189,7 +260,10 @@ export async function runStart( write(` sync-interval: ${config.syncIntervalMs / 1000}s`); write(` merged-lookback: ${config.mergedLookbackSeconds}s`); write(` assign-owners: ${config.assignOwners}`); - write(` sync-apply: ${config.syncApply}`); + write(` sync-mode: ${config.syncMode}`); + if (config.syncAssignee) { + write(` sync-assignee: ${config.syncAssignee}`); + } if (flags.dryRun) { write("--dry-run: not booting daemon (config left untouched)"); diff --git a/src/products/gardener/engine/daemon/config.ts b/src/products/gardener/engine/daemon/config.ts index 1469baf..182663d 100644 --- a/src/products/gardener/engine/daemon/config.ts +++ b/src/products/gardener/engine/daemon/config.ts @@ -15,6 +15,21 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; +/** + * What the sync-sweep should do on each tick. + * + * - `detect` (default): run `gardener sync` only; report drift, no writes. + * - `apply`: run `gardener sync --apply`; open one aggregated tree PR. + * - `open-issues`: run `gardener sync --open-issues`; open one tree + * issue per drift proposal, assignees resolved from NODE.md owners + * unless overridden by `syncAssignee`. + * + * Modes are mutually exclusive by construction. The legacy boolean + * `syncApply` is kept in the on-disk schema as a deprecated alias + * that coerces to `syncMode: 'apply'`. + */ +export type SyncMode = "detect" | "apply" | "open-issues"; + export interface GardenerDaemonConfig { /** Absolute path to the bound tree repo checkout. */ treePath: string; @@ -36,10 +51,19 @@ export interface GardenerDaemonConfig { */ assignOwners: boolean; /** - * When true, `sync-sweep` invokes `gardener sync --apply` (open tree - * PRs). When false, it stays in detect-only mode. + * How the sync-sweep runs — see {@link SyncMode}. Replaces the older + * `syncApply: boolean` which is retained only as a read-time alias + * for back-compat with configs written by earlier versions. + */ + syncMode: SyncMode; + /** + * Optional override for every issue opened in `syncMode: "open-issues"`. + * When set, all issues are assigned to this user instead of NODE.md + * owners. Intended for testing against third-party repos where you + * don't want to ping real domain owners. Ignored unless + * `syncMode === "open-issues"`. */ - syncApply: boolean; + syncAssignee?: string; } export function resolveGardenerDir(env: NodeJS.ProcessEnv = process.env): string { @@ -95,7 +119,11 @@ function coerceDaemonConfig(raw: Record): GardenerDaemonConfig ? raw.mergedLookbackSeconds : Math.max(60, Math.round((gardenerIntervalMs * 2) / 1000)); const assignOwners = raw.assignOwners === true; - const syncApply = raw.syncApply === true; + const syncMode = coerceSyncMode(raw); + const syncAssignee = + typeof raw.syncAssignee === "string" && raw.syncAssignee.length > 0 + ? raw.syncAssignee + : undefined; return { treePath, codeRepos, @@ -103,10 +131,25 @@ function coerceDaemonConfig(raw: Record): GardenerDaemonConfig syncIntervalMs, mergedLookbackSeconds, assignOwners, - syncApply, + syncMode, + ...(syncAssignee ? { syncAssignee } : {}), }; } +/** + * Read `syncMode` from raw config, falling back to the legacy + * `syncApply: boolean` alias for configs written by pre-enum versions. + * Unknown string values collapse to `"detect"` (safe default). + */ +function coerceSyncMode(raw: Record): SyncMode { + const mode = raw.syncMode; + if (mode === "detect" || mode === "apply" || mode === "open-issues") { + return mode; + } + if (raw.syncApply === true) return "apply"; + return "detect"; +} + /** * Parse a `` duration string into milliseconds. Accepts * `m`/`h`/`d`, plus bare integers interpreted as seconds for shell @@ -135,11 +178,29 @@ export function buildDaemonConfig(opts: { syncIntervalMs?: number; mergedLookbackSeconds?: number; assignOwners?: boolean; + /** + * New enum-shaped selector for the sync-sweep mode. Prefer this over + * the legacy `syncApply` boolean. + */ + syncMode?: SyncMode; + /** Optional assignee override, applied only when `syncMode === "open-issues"`. */ + syncAssignee?: string; + /** + * @deprecated Pass `syncMode: "apply"` instead. When set to true and + * `syncMode` is omitted, coerces to `syncMode: "apply"`. When both + * are set, `syncMode` wins. + */ syncApply?: boolean; }): GardenerDaemonConfig { const gardenerIntervalMs = opts.gardenerIntervalMs ?? DEFAULT_GARDENER_INTERVAL_MS; const syncIntervalMs = opts.syncIntervalMs ?? DEFAULT_SYNC_INTERVAL_MS; const resolvedTree = resolve(opts.treePath); + const syncMode: SyncMode = + opts.syncMode ?? (opts.syncApply === true ? "apply" : "detect"); + const syncAssignee = + syncMode === "open-issues" && opts.syncAssignee && opts.syncAssignee.length > 0 + ? opts.syncAssignee + : undefined; return { treePath: resolvedTree, codeRepos: [...opts.codeRepos], @@ -149,6 +210,7 @@ export function buildDaemonConfig(opts: { opts.mergedLookbackSeconds ?? Math.max(60, Math.round((gardenerIntervalMs * 2) / 1000)), assignOwners: opts.assignOwners ?? false, - syncApply: opts.syncApply ?? false, + syncMode, + ...(syncAssignee ? { syncAssignee } : {}), }; } diff --git a/src/products/gardener/engine/daemon/loop.ts b/src/products/gardener/engine/daemon/loop.ts index bf39d1b..61e0620 100644 --- a/src/products/gardener/engine/daemon/loop.ts +++ b/src/products/gardener/engine/daemon/loop.ts @@ -166,7 +166,14 @@ export function buildSyncSweepArgs( config: GardenerDaemonConfig, ): string[] { const args = ["gardener", "sync", "--tree-path", config.treePath]; - if (config.syncApply) args.push("--apply"); + if (config.syncMode === "apply") { + args.push("--apply"); + } else if (config.syncMode === "open-issues") { + args.push("--open-issues"); + if (config.syncAssignee) { + args.push("--assignee", config.syncAssignee); + } + } return args; } diff --git a/tests/gardener/gardener-daemon.test.ts b/tests/gardener/gardener-daemon.test.ts index 240ec1e..07fa7b0 100644 --- a/tests/gardener/gardener-daemon.test.ts +++ b/tests/gardener/gardener-daemon.test.ts @@ -81,7 +81,7 @@ describe("gardener daemon -- config I/O", () => { gardenerIntervalMs: 60_000, syncIntervalMs: 3_600_000, assignOwners: true, - syncApply: false, + syncMode: "detect", }); writeDaemonConfig(config, env); const loaded = loadDaemonConfig(env); @@ -89,6 +89,58 @@ describe("gardener daemon -- config I/O", () => { expect(loaded?.codeRepos).toEqual(["a/b", "c/d"]); expect(loaded?.assignOwners).toBe(true); expect(loaded?.mergedLookbackSeconds).toBe(120); // 2× 60_000 ms → 120 s + expect(loaded?.syncMode).toBe("detect"); + expect(loaded?.syncAssignee).toBeUndefined(); + }); + + it("round-trips open-issues mode with assignee", () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + writeDaemonConfig( + buildDaemonConfig({ + treePath: "/tmp/tree", + codeRepos: ["a/b"], + syncMode: "open-issues", + syncAssignee: "alice", + }), + env, + ); + const loaded = loadDaemonConfig(env); + expect(loaded?.syncMode).toBe("open-issues"); + expect(loaded?.syncAssignee).toBe("alice"); + }); + + it("coerces legacy syncApply=true in on-disk config to syncMode='apply'", () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + // Simulate a config file written by a pre-enum version by writing + // the raw JSON directly (bypasses writeDaemonConfig's strict types). + const legacyJson = { + treePath: "/tmp/legacy", + codeRepos: ["a/b"], + gardenerIntervalMs: 60_000, + syncIntervalMs: 3_600_000, + mergedLookbackSeconds: 120, + assignOwners: false, + syncApply: true, + }; + const { writeFileSync, mkdirSync } = require("node:fs") as typeof import("node:fs"); + const { dirname } = require("node:path") as typeof import("node:path"); + const path = `${tmp.path}/config.json`; + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, JSON.stringify(legacyJson, null, 2) + "\n"); + const loaded = loadDaemonConfig(env); + expect(loaded?.syncMode).toBe("apply"); + }); + + it("drops syncAssignee when syncMode is not open-issues", () => { + const config = buildDaemonConfig({ + treePath: "/x", + codeRepos: ["a/b"], + syncMode: "detect", + syncAssignee: "alice", + }); + expect(config.syncAssignee).toBeUndefined(); }); it("returns null when no config file exists", () => { @@ -195,15 +247,58 @@ describe("gardener daemon -- sweep arg builders", () => { expect(args).toContain("--assign-owners"); }); - it("sync sweep passes --apply only when syncApply=true", () => { + it("sync sweep in detect mode passes neither --apply nor --open-issues", () => { const base = buildDaemonConfig({ treePath: "/x", codeRepos: ["a/b"] }); - expect(buildSyncSweepArgs(base)).not.toContain("--apply"); + const args = buildSyncSweepArgs(base); + expect(args).not.toContain("--apply"); + expect(args).not.toContain("--open-issues"); + }); + + it("sync sweep in apply mode passes --apply", () => { const applied = buildDaemonConfig({ treePath: "/x", codeRepos: ["a/b"], - syncApply: true, + syncMode: "apply", + }); + const args = buildSyncSweepArgs(applied); + expect(args).toContain("--apply"); + expect(args).not.toContain("--open-issues"); + }); + + it("sync sweep in open-issues mode passes --open-issues", () => { + const opened = buildDaemonConfig({ + treePath: "/x", + codeRepos: ["a/b"], + syncMode: "open-issues", + }); + const args = buildSyncSweepArgs(opened); + expect(args).toContain("--open-issues"); + expect(args).not.toContain("--apply"); + expect(args).not.toContain("--assignee"); + }); + + it("sync sweep in open-issues mode forwards --assignee when configured", () => { + const opened = buildDaemonConfig({ + treePath: "/x", + codeRepos: ["a/b"], + syncMode: "open-issues", + syncAssignee: "alice", }); - expect(buildSyncSweepArgs(applied)).toContain("--apply"); + const args = buildSyncSweepArgs(opened); + expect(args).toContain("--open-issues"); + const idx = args.indexOf("--assignee"); + expect(idx).toBeGreaterThan(-1); + expect(args[idx + 1]).toBe("alice"); + }); + + it("sync sweep back-compat: legacy syncApply=true boolean still produces --apply", () => { + const legacy = buildDaemonConfig({ + treePath: "/x", + codeRepos: ["a/b"], + syncApply: true, // legacy path, no syncMode + }); + expect(legacy.syncMode).toBe("apply"); + expect(buildSyncSweepArgs(legacy)).toContain("--apply"); }); }); @@ -398,6 +493,117 @@ describe("gardener daemon -- start --dry-run", () => { expect(code).toBe(1); expect(lines.some((l) => l.includes("--gardener-interval"))).toBe(true); }); + + it("accepts --sync-mode open-issues and previews syncMode + syncAssignee", async () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + const { write, lines } = capture(); + const code = await runStart( + [ + "--tree-path", + tmp.path, + "--code-repo", + "a/b", + "--sync-mode", + "open-issues", + "--sync-assignee", + "alice", + "--dry-run", + ], + { env, write }, + ); + expect(code).toBe(0); + const body = lines.join("\n"); + expect(body).toContain("sync-mode: open-issues"); + expect(body).toContain("sync-assignee: alice"); + }); + + it("rejects --sync-mode with an invalid value", async () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + const { write, lines } = capture(); + const code = await runStart( + [ + "--tree-path", + tmp.path, + "--code-repo", + "a/b", + "--sync-mode", + "bogus", + "--dry-run", + ], + { env, write }, + ); + expect(code).toBe(1); + expect(lines.some((l) => l.includes("--sync-mode"))).toBe(true); + expect(lines.some((l) => l.includes("detect|apply|open-issues"))).toBe(true); + }); + + it("rejects --sync-assignee outside --sync-mode open-issues", async () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + const { write, lines } = capture(); + const code = await runStart( + [ + "--tree-path", + tmp.path, + "--code-repo", + "a/b", + "--sync-mode", + "apply", + "--sync-assignee", + "alice", + "--dry-run", + ], + { env, write }, + ); + expect(code).toBe(1); + expect( + lines.some((l) => l.includes("--sync-assignee is only valid with --sync-mode open-issues")), + ).toBe(true); + }); + + it("rejects using --sync-apply and --sync-mode together", async () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + const { write, lines } = capture(); + const code = await runStart( + [ + "--tree-path", + tmp.path, + "--code-repo", + "a/b", + "--sync-apply", + "--sync-mode", + "apply", + "--dry-run", + ], + { env, write }, + ); + expect(code).toBe(1); + expect(lines.some((l) => l.includes("mutually exclusive"))).toBe(true); + }); + + it("accepts deprecated --sync-apply, warns, and maps to syncMode=apply", async () => { + const tmp = useTmpDir(); + const env = makeEnv(tmp.path); + const { write, lines } = capture(); + const code = await runStart( + [ + "--tree-path", + tmp.path, + "--code-repo", + "a/b", + "--sync-apply", + "--dry-run", + ], + { env, write }, + ); + expect(code).toBe(0); + const body = lines.join("\n"); + expect(body).toContain("--sync-apply is deprecated"); + expect(body).toContain("sync-mode: apply"); + }); }); describe("gardener daemon -- stop idempotency", () => {