polish: tighten scaffold-keeper signature + dedupe Pending report row

Follow-up to commit 39e2dc70c. Two small improvements that surfaced when
the parallel Phase D subagent finished and inspected the worktree:

- commands-scaffold-sync.ts:
  - Tighten ScaffoldKeeperFn to match Phase D's actual dispatcher signature
    (basePath, ctx) => Promise<number>. Define a local minimal
    ScaffoldKeeperCtxShape for the lazy loader so we don't form a hard
    import dependency on scaffold-keeper.ts.
  - Remove duplicated "Upgradable" line from the report table — keep only
    "Pending" since ADR-021 §10 names that the user-facing label.
- tests/scaffold-keeper.test.ts: better-typed notify stub; covers Phase E
  arg-parser helpers (parseScaffoldSyncArgs, matchesOnly, applyOnlyFilter).

Typecheck clean. 49/49 scaffold tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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<string, unknown>,
+		) => void;
+	};
+}
+
+type ScaffoldKeeperFn = (
+	basePath: string,
+	ctx: ScaffoldKeeperCtxShape,
+) => Promise<number>;
+
 /**
  * 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<ScaffoldKeeperFn | null> {
 	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) {

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<string, unknown>;
 }
 
-function makeStubCtx(): { ctx: { ui: { notify: (...args: unknown[]) => void } }; calls: NotifyCall[] } {
+function makeStubCtx(): {
+	ctx: {
+		ui: {
+			notify: (
+				message: string,
+				type?: "info" | "warning" | "error" | "success",
+				metadata?: Record<string, unknown>,
+			) => void;
+		};
+	};
+	calls: NotifyCall[];
+} {
 	const calls: NotifyCall[] = [];
 	const ctx = {
 		ui: {
 			notify: (
 				message: string,
-				type?: string,
+				type?: "info" | "warning" | "error" | "success",
 				metadata?: Record<string, unknown>,
 			) => {
 				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=<glob> 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\./);
+	});
+});