diff --git a/src/resources/extensions/sf/commands-scaffold-sync.ts b/src/resources/extensions/sf/commands-scaffold-sync.ts index b92e2af36..514f0fa2e 100644 --- a/src/resources/extensions/sf/commands-scaffold-sync.ts +++ b/src/resources/extensions/sf/commands-scaffold-sync.ts @@ -19,7 +19,6 @@ import { ensureAgenticDocsScaffold } from "./agentic-docs-scaffold.js"; import { projectRoot } from "./commands/context.js"; import { detectScaffoldDrift, - type ScaffoldDriftItem, type ScaffoldDriftReport, } from "./scaffold-drift.js"; @@ -104,10 +103,12 @@ export function applyOnlyFilter( function formatReportTable(report: ScaffoldDriftReport): string { const c = report.countsByBucket; + // Per ADR-021 §10 the user-facing label for the `upgradable` drift bucket is + // "Pending" — those are pending-state files whose stamped version trails the + // current ship version and are slated for silent re-render on next sync. const lines = [ "Scaffold drift report:", ` Missing : ${c.missing}`, - ` Upgradable : ${c.upgradable}`, ` Pending : ${c.upgradable}`, ` Editing-drift: ${c["editing-drift"]}`, ` Untracked : ${c.untracked}`, @@ -145,31 +146,38 @@ function formatSyncDelta( return `Sync complete — ${parts.join(", ")}.`; } +/** + * Minimal subset of Phase D's `ScaffoldKeeperCtx` shape. Mirrored here so we + * don't form a hard import dependency on `scaffold-keeper.js` — the loader + * below imports it lazily and gracefully handles its absence. + */ +interface ScaffoldKeeperCtxShape { + ui: { + notify: ( + message: string, + type?: "info" | "warning" | "error" | "success", + metadata?: Record, + ) => void; + }; +} + +type ScaffoldKeeperFn = ( + basePath: string, + ctx: ScaffoldKeeperCtxShape, +) => Promise; + /** * Lazy import for Phase D's scaffold-keeper dispatcher. Returns `null` if * Phase D has not shipped yet, in which case `--include-editing` reports the * feature as unavailable rather than crashing. */ -async function tryLoadScaffoldKeeper(): Promise< - | ((basePath: string, items: ScaffoldDriftItem[]) => Promise<{ proposed: string[] }>) - | null -> { +async function tryLoadScaffoldKeeper(): Promise { try { - // Phase D is expected to expose this from a `scaffold-keeper.js` sibling. - const mod: unknown = await import("./scaffold-keeper.js").catch(() => null); - if ( - mod && - typeof (mod as { dispatchScaffoldKeeperIfNeeded?: unknown }) - .dispatchScaffoldKeeperIfNeeded === "function" - ) { - return ( - mod as { - dispatchScaffoldKeeperIfNeeded: ( - basePath: string, - items: ScaffoldDriftItem[], - ) => Promise<{ proposed: string[] }>; - } - ).dispatchScaffoldKeeperIfNeeded; + const mod = (await import("./scaffold-keeper.js").catch(() => null)) as + | { dispatchScaffoldKeeperIfNeeded?: unknown } + | null; + if (mod && typeof mod.dispatchScaffoldKeeperIfNeeded === "function") { + return mod.dispatchScaffoldKeeperIfNeeded as ScaffoldKeeperFn; } } catch { // fall through @@ -235,9 +243,11 @@ export async function handleScaffoldSync( } try { - const result = await dispatcher(basePath, editingItems); - const proposed = Array.isArray(result?.proposed) ? result.proposed : []; - if (proposed.length === 0) { + // Phase D's dispatcher emits its own `approval_request` notification when + // it writes a .proposed file; we additionally print the per-path summary + // the brief specifies for the explicit `--include-editing` invocation. + const written = await dispatcher(basePath, ctx); + if (written === 0) { ctx.ui.notify( "scaffold-keeper completed without producing .proposed files.", "info", @@ -245,8 +255,8 @@ export async function handleScaffoldSync( return; } const lines = [ - `Wrote ${proposed.length} .proposed file${proposed.length === 1 ? "" : "s"}:`, - ...proposed.map((p) => ` ${p}`), + `Wrote ${written} .proposed file${written === 1 ? "" : "s"}:`, + ...editingItems.map((i) => ` ${i.path}.proposed`), ]; ctx.ui.notify(lines.join("\n"), "info"); } catch (err) { diff --git a/src/resources/extensions/sf/tests/scaffold-keeper.test.ts b/src/resources/extensions/sf/tests/scaffold-keeper.test.ts index 5b3a947da..3923ad851 100644 --- a/src/resources/extensions/sf/tests/scaffold-keeper.test.ts +++ b/src/resources/extensions/sf/tests/scaffold-keeper.test.ts @@ -20,6 +20,12 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, test } from "node:test"; import { SCAFFOLD_FILES } from "../agentic-docs-scaffold.ts"; +import { + applyOnlyFilter, + matchesOnly, + parseScaffoldSyncArgs, +} from "../commands-scaffold-sync.ts"; +import { detectScaffoldDrift } from "../scaffold-drift.ts"; import { dispatchScaffoldKeeperIfNeeded } from "../scaffold-keeper.ts"; import { stampScaffoldFile } from "../scaffold-versioning.ts"; @@ -29,13 +35,24 @@ interface NotifyCall { metadata?: Record; } -function makeStubCtx(): { ctx: { ui: { notify: (...args: unknown[]) => void } }; calls: NotifyCall[] } { +function makeStubCtx(): { + ctx: { + ui: { + notify: ( + message: string, + type?: "info" | "warning" | "error" | "success", + metadata?: Record, + ) => void; + }; + }; + calls: NotifyCall[]; +} { const calls: NotifyCall[] = []; const ctx = { ui: { notify: ( message: string, - type?: string, + type?: "info" | "warning" | "error" | "success", metadata?: Record, ) => { calls.push({ message, type, metadata }); @@ -199,3 +216,192 @@ describe("dispatchScaffoldKeeperIfNeeded", () => { assert.ok(existsSync(join(dir, "AGENTS.md.proposed"))); }); }); + +// ─── ADR-021 Phase E: /sf scaffold sync command ───────────────────────────── + +describe("parseScaffoldSyncArgs", () => { + test("empty input → all-false defaults", () => { + const opts = parseScaffoldSyncArgs(""); + assert.equal(opts.dryRun, false); + assert.equal(opts.includeEditing, false); + assert.equal(opts.only, undefined); + }); + + test("--dry-run sets dryRun", () => { + const opts = parseScaffoldSyncArgs("--dry-run"); + assert.equal(opts.dryRun, true); + }); + + test("--include-editing sets includeEditing", () => { + const opts = parseScaffoldSyncArgs("--include-editing"); + assert.equal(opts.includeEditing, true); + }); + + test("--only= captures the glob value", () => { + const opts = parseScaffoldSyncArgs("--only=harness/**"); + assert.equal(opts.only, "harness/**"); + }); + + test("flags compose without ordering constraints", () => { + const opts = parseScaffoldSyncArgs( + "--include-editing --only=docs/RELIABILITY.md --dry-run", + ); + assert.equal(opts.dryRun, true); + assert.equal(opts.includeEditing, true); + assert.equal(opts.only, "docs/RELIABILITY.md"); + }); + + test("--only= with empty value is ignored", () => { + const opts = parseScaffoldSyncArgs("--only="); + assert.equal(opts.only, undefined); + }); +}); + +describe("matchesOnly", () => { + test("undefined glob matches everything", () => { + assert.equal(matchesOnly("docs/RELIABILITY.md", undefined), true); + }); + + test("exact path matches", () => { + assert.equal(matchesOnly("AGENTS.md", "AGENTS.md"), true); + }); + + test("prefix match", () => { + assert.equal(matchesOnly("harness/specs/bootstrap.md", "harness/"), true); + assert.equal(matchesOnly("docs/AGENTS.md", "harness/"), false); + }); + + test("suffix match", () => { + assert.equal(matchesOnly("docs/RELIABILITY.md", "RELIABILITY.md"), true); + }); + + test("wildcard *", () => { + assert.equal(matchesOnly("harness/specs/bootstrap.md", "harness/**"), true); + assert.equal(matchesOnly("harness/specs/AGENTS.md", "harness/*"), true); + assert.equal(matchesOnly("docs/RELIABILITY.md", "harness/**"), false); + }); +}); + +describe("applyOnlyFilter", () => { + test("undefined glob returns the report unchanged", () => { + const before = detectScaffoldDrift(mkdtempSync(join(tmpdir(), "sf-only-"))); + const after = applyOnlyFilter(before, undefined); + assert.equal(after, before); + }); + + test("filter narrows items and recomputes counts", () => { + const subdir = mkdtempSync(join(tmpdir(), "sf-only-")); + const before = detectScaffoldDrift(subdir); + const after = applyOnlyFilter(before, "AGENTS.md"); + assert.ok(after.items.length >= 1); + // All retained items must end with AGENTS.md per the suffix-match rule. + for (const item of after.items) { + assert.ok(item.path.endsWith("AGENTS.md")); + } + // Counts must be consistent with retained items. + const total = Object.values(after.countsByBucket).reduce( + (a, b) => a + b, + 0, + ); + assert.equal(total, after.items.length); + rmSync(subdir, { recursive: true, force: true }); + }); +}); + +/** + * Smoke tests for `handleScaffoldSync`. The handler calls `projectRoot()`, + * which derives from `process.cwd()`/`SF_PROJECT_ROOT` and runs + * `validateDirectory`. We isolate by setting `SF_PROJECT_ROOT` to a tmpdir, + * which short-circuits `resolveProjectRoot` and keeps validation inside a + * known-safe path. + */ +describe("handleScaffoldSync (Phase E smoke)", () => { + let dir2: string; + let prevRoot: string | undefined; + beforeEach(() => { + dir2 = mkdtempSync(join(tmpdir(), "sf-scaffold-sync-")); + prevRoot = process.env.SF_PROJECT_ROOT; + process.env.SF_PROJECT_ROOT = dir2; + }); + afterEach(() => { + if (prevRoot === undefined) { + delete process.env.SF_PROJECT_ROOT; + } else { + process.env.SF_PROJECT_ROOT = prevRoot; + } + rmSync(dir2, { recursive: true, force: true }); + }); + + test("--dry-run emits a report and does not write any scaffold files", async () => { + const { handleScaffoldSync } = await import( + "../commands-scaffold-sync.ts" + ); + const { ctx, calls } = makeStubCtx(); + + await handleScaffoldSync("--dry-run", ctx as never); + + assert.equal(calls.length, 1); + assert.match(calls[0].message, /Scaffold drift report:/); + // No files should have been written by the dry-run path. + assert.ok(!existsSync(join(dir2, "AGENTS.md"))); + assert.ok(!existsSync(join(dir2, "ARCHITECTURE.md"))); + }); + + test("default invocation calls ensureAgenticDocsScaffold (writes scaffold files)", async () => { + const { handleScaffoldSync } = await import( + "../commands-scaffold-sync.ts" + ); + const { ctx, calls } = makeStubCtx(); + + await handleScaffoldSync("", ctx as never); + + assert.ok(calls.length >= 1); + // Default sync is the same code path as automatic startup sync, so the + // scaffold files should exist on disk after one invocation. + assert.ok(existsSync(join(dir2, "AGENTS.md"))); + assert.ok(existsSync(join(dir2, "ARCHITECTURE.md"))); + }); + + test("--only filter narrows output and still applies sync", async () => { + const { handleScaffoldSync } = await import( + "../commands-scaffold-sync.ts" + ); + const { ctx, calls } = makeStubCtx(); + + await handleScaffoldSync("--dry-run --only=harness/", ctx as never); + + assert.equal(calls.length, 1); + // The drift report should not contain non-harness files. Counts under the + // filter must be consistent: total of bucket counts matches a small + // fraction of SCAFFOLD_FILES (those starting with `harness/`). + const harnessCount = SCAFFOLD_FILES.filter((f) => + f.path.startsWith("harness/"), + ).length; + const message = calls[0].message; + // Sanity: the table prints the harness-only counts; validate by parsing + // the count lines and asserting their sum equals harnessCount. + const counts = ["Missing", "Pending", "Editing-drift", "Untracked", "Customized"] + .map((label) => { + const m = message.match(new RegExp(`${label}\\s*:\\s*(\\d+)`)); + return m ? Number.parseInt(m[1], 10) : 0; + }) + .reduce((a, b) => a + b, 0); + assert.equal(counts, harnessCount); + }); + + test("--include-editing without editing-drift items reports nothing to merge", async () => { + const { handleScaffoldSync } = await import( + "../commands-scaffold-sync.ts" + ); + // First populate the scaffold so all files exist in pending state. + const { ctx: warmCtx } = makeStubCtx(); + await handleScaffoldSync("", warmCtx as never); + + const { ctx, calls } = makeStubCtx(); + await handleScaffoldSync("--include-editing", ctx as never); + + // Should produce a report notification + a "no editing-drift" notice. + const messages = calls.map((c) => c.message).join("\n"); + assert.match(messages, /No editing-drift items to merge\./); + }); +});