refactor: simplify prebuilt-payload internals (post-#71 follow-up)

.changeset/prebuilt-simplify-followup.md

@@ -0,0 +1,5 @@
+---
+"agent-bundle": patch
+---
+
+Simplify prebuilt-payload internals (post-#71 follow-up): dev-server preparations no longer pay the AB4750 payload-freshness mtime walk for commands that discard it, the payload-declaration parse and innermost-payload ownership rule are shared across discovery, normalization, and validation instead of being open-coded per module, and `PreparedProject.snapshotSource` is required so artifact re-snapshots always observe the payload roots the prepared identity hashed.

packages/agent-bundle/src/build/build.ts

@@ -75,12 +75,15 @@ interface StagedTarget extends PlannedTarget {
   readonly root: string;
 }
 
-const prebuiltReferenceExists = (
-  model: NormalizedPlugin,
-  artifactPath: string,
-): boolean => (model.payloads ?? []).some((payload) =>
-  artifactPath.startsWith(`${payload.name}/`) &&
-  payload.files.some((file) => `${payload.name}/${file.relativePath}` === artifactPath));
+const prebuiltArtifactPaths = (model: NormalizedPlugin): ReadonlySet<string> =>
+  new Set((model.payloads ?? []).flatMap((payload) =>
+    payload.files.map((file) => `${payload.name}/${file.relativePath}`)));
+
+const missingPrebuiltDiagnostic = (subject: string, artifactPath: string): Diagnostic => ({
+  code: 'AB4748',
+  message: `${subject} ${JSON.stringify(artifactPath)} is not present in its declared payload; run the project's own build before "agent-bundle build".`,
+  severity: 'error',
+});
 
 /**
  * AB4747-AB4749: an artifact build packages prebuilt payloads exactly as
@@ -112,28 +115,21 @@ const prebuiltPayloadDiagnostics = (
       });
     }
   }
+  const artifactPaths = prebuiltArtifactPaths(model);
   const tokenPrefix = `${pathTokens.pluginRoot}/`;
   for (const server of model.mcpServers) {
     if (server.provenance.kind !== 'prebuilt') continue;
     const entry = server.args?.[0];
     if (typeof entry !== 'string' || !entry.startsWith(tokenPrefix)) continue;
     const artifactPath = entry.slice(tokenPrefix.length);
-    if (!prebuiltReferenceExists(model, artifactPath)) {
-      diagnostics.push({
-        code: 'AB4748',
-        message: `MCP server ${JSON.stringify(server.name)} prebuilt entry ${JSON.stringify(artifactPath)} is not present in its declared payload; run the project's own build before "agent-bundle build".`,
-        severity: 'error',
-      });
+    if (!artifactPaths.has(artifactPath)) {
+      diagnostics.push(missingPrebuiltDiagnostic(`MCP server ${JSON.stringify(server.name)} prebuilt entry`, artifactPath));
     }
   }
   for (const hook of model.hooks) {
     if (hook.prebuiltPath === undefined) continue;
-    if (!prebuiltReferenceExists(model, hook.prebuiltPath)) {
-      diagnostics.push({
-        code: 'AB4748',
-        message: `Hook ${JSON.stringify(hook.name)} prebuilt handler ${JSON.stringify(hook.prebuiltPath)} is not present in its declared payload; run the project's own build before "agent-bundle build".`,
-        severity: 'error',
-      });
+    if (!artifactPaths.has(hook.prebuiltPath)) {
+      diagnostics.push(missingPrebuiltDiagnostic(`Hook ${JSON.stringify(hook.name)} prebuilt handler`, hook.prebuiltPath));
     }
   }
   return diagnostics;

packages/agent-bundle/src/config/discover.ts

@@ -111,6 +111,29 @@ const discoverAssets = async (
   })));
 };
 
+/** The declared source path of a payload declaration, or undefined when the declaration is not string-or-`{source}` shaped. */
+export const payloadDeclarationEntry = (declaration: unknown): string | undefined => {
+  const entry = typeof declaration === 'string'
+    ? declaration
+    : isRecord(declaration) ? declaration.source : undefined;
+  return typeof entry === 'string' && entry.trim().length > 0 ? entry : undefined;
+};
+
+/**
+ * The absolute, project-contained source directory of one well-shaped payload
+ * declaration. Malformed or escaping declarations return undefined — source
+ * validation reports those (AB4740-AB4742).
+ */
+export const payloadDeclarationSource = (
+  projectRoot: string,
+  declaration: unknown,
+): string | undefined => {
+  const entry = payloadDeclarationEntry(declaration);
+  if (entry === undefined) return undefined;
+  const source = resolve(projectRoot, entry);
+  return isInside(projectRoot, source) ? source : undefined;
+};
+
 /**
  * The absolute source directories of well-shaped payload declarations.
  * Source snapshots use this to include payload files in the project
@@ -124,10 +147,8 @@ export const configuredPayloadRoots = (
   if (configured === undefined || !isRecord(configured)) return [];
   const roots: string[] = [];
   for (const declaration of Object.values(configured)) {
-    const entry = typeof declaration === 'string' ? declaration : declaration?.source;
-    if (typeof entry !== 'string' || entry.trim().length === 0) continue;
-    const source = resolve(projectRoot, entry);
-    if (isInside(projectRoot, source)) roots.push(source);
+    const source = payloadDeclarationSource(projectRoot, declaration);
+    if (source !== undefined) roots.push(source);
   }
   return [...new Set(roots)].sort((left, right) => left.localeCompare(right));
 };
@@ -146,10 +167,8 @@ const discoverPayloads = async (
   if (configured === undefined || !isRecord(configured)) return [];
   const payloads: DiscoveredPayload[] = [];
   for (const [name, declaration] of Object.entries(configured).sort(([left], [right]) => left.localeCompare(right))) {
-    const entry = typeof declaration === 'string' ? declaration : declaration?.source;
-    if (typeof entry !== 'string' || entry.trim().length === 0) continue;
-    const source = resolve(projectRoot, entry);
-    if (!isInside(projectRoot, source)) continue;
+    const source = payloadDeclarationSource(projectRoot, declaration);
+    if (source === undefined) continue;
     let stats;
     try {
       stats = await stat(source);
@@ -160,12 +179,13 @@ const discoverPayloads = async (
       payloads.push({ files: [], name, source });
       continue;
     }
-    const matches = (await fastGlob('**', { ...assetGlobOptions, cwd: source })).sort((left, right) => left.localeCompare(right));
+    const matches = (await fastGlob('**', { ...assetGlobOptions, cwd: source, stats: true }))
+      .sort((left, right) => left.path.localeCompare(right.path));
     payloads.push({
-      files: await Promise.all(matches.map(async (file) => ({
-        bytes: (await stat(file)).size,
-        relativePath: relative(source, file).replaceAll('\\', '/'),
-        source: file,
+      files: await Promise.all(matches.map(async (match) => ({
+        bytes: (match.stats ?? await stat(match.path)).size,
+        relativePath: relative(source, match.path).replaceAll('\\', '/'),
+        source: match.path,
       }))),
       name,
       source,

packages/agent-bundle/src/config/normalize.ts

@@ -42,7 +42,7 @@ import type {
   NormalizedSkill,
   SourceProvenance,
 } from '../core/types.ts';
-import type { DiscoveredProject } from './discover.ts';
+import { type DiscoveredProject, payloadDeclarationSource } from './discover.ts';
 import type { LoadedConfig } from './load.ts';
 
 const unique = (values: readonly string[]): string[] => [...new Set(values)];
@@ -230,20 +230,33 @@ const normalizePayloads = (
   const discoveredByName = new Map((discovered.payloads ?? []).map((payload) => [payload.name, payload]));
   const payloads: NormalizedPayload[] = [];
   for (const [name, declaration] of Object.entries(configured).sort(([left], [right]) => left.localeCompare(right))) {
-    const entry = typeof declaration === 'string' ? declaration : declaration.source;
-    if (typeof entry !== 'string' || entry.trim().length === 0) continue;
+    const source = payloadDeclarationSource(loaded.context.projectRoot, declaration);
+    if (source === undefined) continue;
     payloads.push({
       files: (discoveredByName.get(name)?.files ?? []).map((file) => ({ ...file })),
       id: `payload:${name}`,
       name,
       provenance: { kind: 'prebuilt', sourcePath: loaded.configPath },
-      source: resolve(loaded.context.projectRoot, entry),
+      source,
       targets: sortedUnique(typeof declaration === 'string' ? targetNames : (declaration.targets ?? targetNames)),
     });
   }
   return payloads;
 };
 
+/** The innermost declared payload whose source directory contains the file. */
+export const owningPayload = <Payload extends { readonly source: string }>(
+  payloads: readonly Payload[],
+  source: string,
+): Payload | undefined => {
+  let best: Payload | undefined;
+  for (const payload of payloads) {
+    if (!isInside(payload.source, source)) continue;
+    if (best === undefined || payload.source.length > best.source.length) best = payload;
+  }
+  return best;
+};
+
 /**
  * The artifact-relative stable path of a prebuilt file: its declaring payload
  * destination plus the file's payload-relative path. Falls back to the
@@ -255,14 +268,10 @@ export const prebuiltArtifactPath = (
   root: string,
   source: string,
 ): string => {
-  let best: NormalizedPayload | undefined;
-  for (const payload of payloads) {
-    if (!isInside(payload.source, source)) continue;
-    if (best === undefined || payload.source.length > best.source.length) best = payload;
-  }
-  return best === undefined
+  const payload = owningPayload(payloads, source);
+  return payload === undefined
     ? relative(root, source).replaceAll('\\', '/')
-    : `${best.name}/${relative(best.source, source).replaceAll('\\', '/')}`;
+    : `${payload.name}/${relative(payload.source, source).replaceAll('\\', '/')}`;
 };
 
 const isHookEntryList = (
@@ -306,9 +315,8 @@ const normalizeHook = (
   const source = resolve(root, prebuilt ? handlerInput.prebuilt : handlerInput);
   const handler = relative(root, source).replaceAll('\\', '/');
   const prebuiltPath = prebuilt ? prebuiltArtifactPath(payloads, root, source) : undefined;
-  const args = prebuilt && entry.args !== undefined
-    ? entry.args.filter((argument): argument is string => typeof argument === 'string')
-    : undefined;
+  // Non-string arguments are a validation error (AB4746); normalization trusts the declared type.
+  const args = prebuilt ? entry.args : undefined;
   const tools = sortedUnique(entry.tools ?? []).filter(
     (tool): tool is CanonicalHookTool => knownHookTools.has(tool as CanonicalHookTool),
   );
@@ -510,9 +518,10 @@ const normalizeMcpApps = (
 
   for (const [serverName, rawServer] of Object.entries(configured).sort(([left], [right]) => left.localeCompare(right))) {
     const server = serverByName.get(serverName);
+    if (server === undefined || rawServer.apps === undefined) continue;
     // Apps require a local server entry: a compiled source entry, or a prebuilt one.
-    const prebuilt = isPrebuiltEntryInput(rawServer.entry);
-    if (server === undefined || (server.source === undefined && !prebuilt) || rawServer.apps === undefined) continue;
+    const prebuilt = server.provenance.kind === 'prebuilt';
+    if (server.source === undefined && !prebuilt) continue;
     for (const [name, app] of Object.entries(rawServer.apps).sort(([left], [right]) => left.localeCompare(right))) {
       const declaration = app as AgentBundleMcpApp;
       apps.push({

packages/agent-bundle/src/config/validate.ts

@@ -28,9 +28,10 @@ import {
   conventionalCliEntrySource,
   conventionalIndexEntrySource,
   conventionalMcpEntrySource,
+  owningPayload,
   reservedPayloadDestinations,
 } from './normalize.ts';
-import type { DiscoveredProject } from './discover.ts';
+import { type DiscoveredProject, payloadDeclarationEntry, payloadDeclarationSource } from './discover.ts';
 import type { LoadedConfig } from './load.ts';
 import type { SkillDocument } from './skill.ts';
 import { referencedResources } from './skill-references.ts';
@@ -90,9 +91,10 @@ const validateHooks = (
       const handler = entry.handler;
       const prebuilt = isPrebuiltEntryInput(handler);
       if (prebuilt) {
-        const hookTargets = Array.isArray(entry.targets) && entry.targets.every(nonemptyString)
-          ? entry.targets
-          : selectedTargets.filter((target) => registry.supports(target, 'hooks'));
+        const hookTargets = declaredTargetsOr(
+          entry.targets,
+          selectedTargets.filter((target) => registry.supports(target, 'hooks')),
+        );
         diagnostics.push(...validatePrebuiltReference(
           `Hook ${event}`,
           handler,
@@ -243,6 +245,13 @@ const isProtocolJsonValue = (value: unknown, ancestors = new Set<object>()): boo
 const nonemptyString = (value: unknown): value is string =>
   typeof value === 'string' && value.trim().length > 0;
 
+/** A declared targets restriction when it is a well-shaped string array, otherwise the fallback selection. */
+const declaredTargetsOr = (
+  targets: unknown,
+  fallback: readonly string[],
+): readonly string[] =>
+  Array.isArray(targets) && targets.every(nonemptyString) ? targets : fallback;
+
 const validateStringList = (
   value: unknown,
   label: string,
@@ -675,9 +684,7 @@ const validateMcpServer = (
 
   if (entry !== undefined || conventionalEntry !== undefined) {
     if (isPrebuiltEntryInput(entry)) {
-      const serverTargets = Array.isArray(server.targets) && server.targets.every(nonemptyString)
-        ? server.targets
-        : selectedTargetNamesFor(loaded, registry);
+      const serverTargets = declaredTargetsOr(server.targets, selectedTargetNamesFor(loaded, registry));
       diagnostics.push(...validatePrebuiltReference(
         `MCP server ${JSON.stringify(name)}`,
         entry,
@@ -1023,17 +1030,13 @@ const declaredPayloads = (
   const selectedTargets = selectedTargetNamesFor(loaded, registry);
   const payloads: DeclaredPayload[] = [];
   for (const [name, declaration] of Object.entries(configured)) {
-    const entry = typeof declaration === 'string'
-      ? declaration
-      : isRecord(declaration) ? declaration.source : undefined;
-    if (!nonemptyString(entry)) continue;
-    const source = resolve(loaded.context.projectRoot, entry);
-    if (!isInside(loaded.context.projectRoot, source)) continue;
+    const source = payloadDeclarationSource(loaded.context.projectRoot, declaration);
+    if (source === undefined) continue;
     const targets = typeof declaration === 'string' ? undefined : declaration.targets;
     payloads.push({
       name,
       source,
-      targets: Array.isArray(targets) && targets.every(nonemptyString) ? targets : selectedTargets,
+      targets: declaredTargetsOr(targets, selectedTargets),
     });
   }
   return payloads;
@@ -1112,6 +1115,7 @@ const payloadTargetDiagnostics = (
 const validatePayload = (
   loaded: LoadedConfig,
   registry: NormalizationTargetRegistry,
+  freshness: boolean,
 ): Diagnostic[] => {
   const configured = loaded.config.payload;
   if (configured === undefined) return [];
@@ -1128,10 +1132,8 @@ const validatePayload = (
         loaded.configPath,
       ));
     }
-    const entry = typeof declaration === 'string'
-      ? declaration
-      : isRecord(declaration) ? declaration.source : undefined;
-    if (!nonemptyString(entry)) {
+    const entry = payloadDeclarationEntry(declaration);
+    if (entry === undefined) {
       diagnostics.push(sourceDiagnostic(
         'AB4740',
         `Payload ${JSON.stringify(name)} must be a source directory path or an object with a source path.`,
@@ -1142,8 +1144,8 @@ const validatePayload = (
     if (typeof declaration !== 'string') {
       diagnostics.push(...payloadTargetDiagnostics(name, declaration.targets, loaded, registry));
     }
-    const source = resolve(loaded.context.projectRoot, entry);
-    if (!isInside(loaded.context.projectRoot, source)) {
+    const source = payloadDeclarationSource(loaded.context.projectRoot, declaration);
+    if (source === undefined) {
       diagnostics.push(sourceDiagnostic(
         'AB4742',
         `Payload ${JSON.stringify(name)} source must resolve inside the project root.`,
@@ -1191,7 +1193,9 @@ const validatePayload = (
       ));
     }
   }
-  const existing = sources.filter((payload) => existsSync(payload.source));
+  // The freshness nudge walks every project and payload file's mtime, so
+  // flows that discard non-error source diagnostics skip it entirely.
+  const existing = freshness ? sources.filter((payload) => existsSync(payload.source)) : [];
   if (existing.length > 0) {
     const newestSource = newestFileMtime(
       loaded.context.projectRoot,
@@ -1237,9 +1241,7 @@ const validatePrebuiltReference = (
   if (!isInside(loaded.context.projectRoot, source)) {
     return [sourceDiagnostic('AB4744', `${label} prebuilt entry must resolve inside the project root.`, loaded.configPath)];
   }
-  const payload = payloads
-    .filter((candidate) => isInside(candidate.source, source))
-    .sort((left, right) => right.source.length - left.source.length)[0];
+  const payload = owningPayload(payloads, source);
   if (payload === undefined) {
     diagnostics.push(sourceDiagnostic(
       'AB4744',
@@ -1321,10 +1323,20 @@ const validateTools = (loaded: LoadedConfig): Diagnostic[] => {
   return diagnostics;
 };
 
+export interface ValidateSourceOptions {
+  /**
+   * Compute the AB4750 payload-freshness nudge, a full-project mtime walk.
+   * Defaults to true; flows that discard non-error source diagnostics pass
+   * false to skip the walk.
+   */
+  readonly payloadFreshness?: boolean;
+}
+
 export const validateSource = (
   loaded: LoadedConfig,
   discovered: DiscoveredProject,
   registry: NormalizationTargetRegistry,
+  options?: ValidateSourceOptions,
 ): Diagnostic[] => {
   const diagnostics: Diagnostic[] = [];
   const plugin = loaded.config.plugin as unknown;
@@ -1379,7 +1391,7 @@ export const validateSource = (
   diagnostics.push(...validateHooks(loaded, registry, payloads));
   diagnostics.push(...validateLib(loaded));
   diagnostics.push(...validateMcp(loaded, registry, payloads));
-  diagnostics.push(...validatePayload(loaded, registry));
+  diagnostics.push(...validatePayload(loaded, registry, options?.payloadFreshness !== false));
   diagnostics.push(...validateRuntime(loaded));
   diagnostics.push(...validateScripts(loaded, registry));
   diagnostics.push(...validateTools(loaded));