fix: second-wave review findings (framework group)

.changeset/framework-review-fixes.md

@@ -0,0 +1,5 @@
+---
+"agent-bundle": patch
+---
+
+Harden Claude artifact and marketplace validation, preserve lifecycle replay invocation and workspace provenance, and invalidate development rebuilds after executable-mode changes.

packages/agent-bundle/src/adapters/claude.ts

@@ -61,6 +61,8 @@ import {
   type StandardPluginHostDocument,
   type TargetAdapter,
   type TargetArtifactCopy,
+  type TargetArtifactDocumentIssue,
+  type TargetArtifactDocumentValidator,
   type TargetArtifactLayout,
   type TargetArtifactPlan,
 } from './types.ts';
@@ -438,6 +440,55 @@ const agentCapabilities = Object.freeze(Object.fromEntries(
 ));
 const packageLifecycle = capabilityTable.plugin.packageLifecycle;
 
+const validateClaudePluginSchema = validateJsonSchemaDocument(validatePlugin);
+
+const numericUserConfigIssues = (
+  userConfig: unknown,
+  instancePath: string,
+): readonly TargetArtifactDocumentIssue[] => {
+  if (!isDataRecord(userConfig)) return Object.freeze([]);
+  const issues: TargetArtifactDocumentIssue[] = [];
+  for (const [key, value] of Object.entries(userConfig)) {
+    if (!isDataRecord(value) || value.type !== 'number') continue;
+    const min = value.min;
+    const max = value.max;
+    const defaultValue = value.default;
+    if (typeof min === 'number' && typeof max === 'number' && min > max) {
+      issues.push(Object.freeze({
+        instancePath: `${instancePath}/${key}`,
+        message: 'numeric option minimum must be less than or equal to its maximum',
+      }));
+    }
+    if (typeof defaultValue !== 'number') continue;
+    if (typeof min === 'number' && defaultValue < min) {
+      issues.push(Object.freeze({
+        instancePath: `${instancePath}/${key}/default`,
+        message: 'numeric option default must be greater than or equal to its minimum',
+      }));
+    }
+    if (typeof max === 'number' && defaultValue > max) {
+      issues.push(Object.freeze({
+        instancePath: `${instancePath}/${key}/default`,
+        message: 'numeric option default must be less than or equal to its maximum',
+      }));
+    }
+  }
+  return Object.freeze(issues);
+};
+
+const validateClaudePluginDocument: TargetArtifactDocumentValidator = (document) => {
+  const schemaIssues = validateClaudePluginSchema(document);
+  if (schemaIssues.length > 0 || !isDataRecord(document)) return schemaIssues;
+  const issues = [...numericUserConfigIssues(document.userConfig, '/userConfig')];
+  if (Array.isArray(document.channels)) {
+    for (const [index, channel] of document.channels.entries()) {
+      if (!isDataRecord(channel)) continue;
+      issues.push(...numericUserConfigIssues(channel.userConfig, `/channels/${String(index)}/userConfig`));
+    }
+  }
+  return Object.freeze(issues);
+};
+
 export const claudeArtifactValidation = deepFreeze({
   documents: [
     Object.freeze({ path: 'hooks/hooks.json', required: false, schema: 'hooks' }),
@@ -455,7 +506,7 @@ export const claudeArtifactValidation = deepFreeze({
     Object.freeze({ name: 'marketplace', validate: validateJsonSchemaDocument(validateMarketplace) }),
     Object.freeze({ name: 'mcp', validate: validateModernMcpDocument(validateJsonSchemaDocument(validateMcp)) }),
     Object.freeze({ name: 'monitors', validate: validateJsonSchemaDocument(validateMonitors) }),
-    Object.freeze({ name: 'plugin', validate: validateJsonSchemaDocument(validatePlugin) }),
+    Object.freeze({ name: 'plugin', validate: validateClaudePluginDocument }),
     Object.freeze({ name: 'settings', validate: validateJsonSchemaDocument(validateSettings) }),
     Object.freeze({ name: 'theme', validate: validateJsonSchemaDocument(validateTheme) }),
   ],
@@ -1179,6 +1230,10 @@ const isInternalSubdirectory = (value: string): boolean =>
   !value.startsWith('\\') &&
   !value.split(/[\\/]/u).some((segment) => segment === '.' || segment === '..');
 
+const isInternalRelativePath = (value: string): boolean =>
+  value === './' ||
+  (value.startsWith('./') && isInternalSubdirectory(value.slice(2)));
+
 const isSafeArchiveUrl = (value: string): boolean => {
   let parsed: URL;
   try {
@@ -1215,9 +1270,7 @@ const planMarketplacePluginSource = (
   pluginRoot: string | undefined,
 ): ClaudeMarketplaceSourcePlan => {
   if (typeof declared === 'string') {
-    const internalRelative =
-      declared.startsWith('./') &&
-      !declared.split('/').includes('..');
+    const internalRelative = isInternalRelativePath(declared);
     const bareUnderPluginRoot =
       pluginRoot !== undefined &&
       declared !== '.' &&
@@ -1758,9 +1811,7 @@ const planClaudeMarketplace = (model: NormalizedPlugin): ClaudeMarketplacePlan =
         }
         const value = metadataValue[field];
         const pathValid = field !== 'pluginRoot' ||
-          (isNonemptyString(value) &&
-            value.startsWith('./') &&
-            !value.split('/').includes('..'));
+          (isNonemptyString(value) && isInternalRelativePath(value));
         if (!isNonemptyString(value) || !pathValid) {
           diagnostics.push(marketplaceDiagnostic(
             `claude.marketplace.metadata.${field}.invalid`,

packages/agent-bundle/src/adapters/schemas/claude/PROVENANCE.json

@@ -17,8 +17,8 @@
       "url": "https://docs.anthropic.com/en/docs/claude-code/plugins"
     },
     "marketplace.schema.json": {
-      "bytes": 14865,
-      "sha256": "44e105038ced3fceee4cb7ff81c7caad63965e3fd5a42b6db6255da771236b5d",
+      "bytes": 14895,
+      "sha256": "4ffa94e8024966e8080b9d3b338c9612bdfa255802a8491b43f74f90427bc988",
       "url": "https://code.claude.com/docs/en/plugin-marketplaces"
     },
     "mcp.schema.json": {

packages/agent-bundle/src/adapters/schemas/claude/marketplace.schema.json

@@ -133,7 +133,7 @@
         {
           "oneOf": [
             {
-              "pattern": "^(?!.*(?:^|/)\\.\\.(?:/|$))\\./",
+              "pattern": "^\\./(?!(?:.*[\\\\/])?\\.\\.?(?:[\\\\/]|$))",
               "type": "string"
             },
             {
@@ -157,7 +157,7 @@
         "source": { "const": "archive" },
         "url": {
           "format": "uri",
-          "pattern": "^https://(?!(?:localhost|[^/]+\\.localhost|127(?:\\.[0-9]{1,3}){3}|169\\.254(?:\\.[0-9]{1,3}){2}|metadata(?:\\.google(?:\\.internal)?)?|metadata\\.azure\\.internal|instance-data\\.ec2\\.internal)(?::[0-9]+)?/)",
+          "pattern": "^https://(?!(?:localhost|[^/]+\\.localhost|127(?:\\.[0-9]{1,3}){3}|169\\.254(?:\\.[0-9]{1,3}){2}|metadata(?:\\.google(?:\\.internal)?)?|metadata\\.azure\\.internal|instance-data\\.ec2\\.internal)(?::[0-9]+)?(?:/|$))",
           "type": "string"
         }
       },
@@ -302,7 +302,7 @@
       "properties": {
         "description": { "minLength": 1, "type": "string" },
         "pluginRoot": {
-          "pattern": "^(?!.*(?:^|/)\\.\\.(?:/|$))\\./.+",
+          "pattern": "^\\./(?!(?:.*[\\\\/])?\\.\\.?(?:[\\\\/]|$)).+",
           "type": "string"
         },
         "version": { "minLength": 1, "type": "string" }

packages/agent-bundle/src/dev/playground/lifecycle-render-child.ts

@@ -60,6 +60,12 @@ const render = async (request: LifecycleRenderChildRequest): Promise<LifecycleRe
           ...(request.requestContext.invocation.hostContractRevision === undefined
             ? {}
             : { hostContractRevision: request.requestContext.invocation.hostContractRevision }),
+          ...(request.requestContext.invocation.operationId === undefined
+            ? {}
+            : { operationId: request.requestContext.invocation.operationId }),
+          ...(request.requestContext.invocation.surface === undefined
+            ? {}
+            : { surface: request.requestContext.invocation.surface }),
         },
         session: request.requestContext.session,
         workspace: request.requestContext.workspace,

packages/agent-bundle/src/dev/playground/lifecycle-replay-service.ts

@@ -56,7 +56,13 @@ const replayRequestContext = (
   hostContractRevision: string,
 ): RequestContextProvenance => {
   const sessionId = nativeText(native, 'session_id') ?? nativeText(native, 'conversation_id');
-  const workspaceRoot = nativeText(native, 'cwd');
+  const workspaceRoots = native['workspace_roots'];
+  const firstWorkspaceRoot = Array.isArray(workspaceRoots) &&
+    typeof workspaceRoots[0] === 'string' &&
+    workspaceRoots[0].trim() !== ''
+    ? workspaceRoots[0]
+    : undefined;
+  const workspaceRoot = nativeText(native, 'cwd') ?? firstWorkspaceRoot;
   return deepFreeze({
     actor: { reason: 'not-provided', state: 'unavailable' },
     host: { source: 'receipt', state: 'available', value: { name: target } },
@@ -82,6 +88,12 @@ const renderContext = (requestContext: RequestContextProvenance): RenderRouteCon
     ...(requestContext.invocation.hostContractRevision === undefined
       ? {}
       : { hostContractRevision: requestContext.invocation.hostContractRevision }),
+    ...(requestContext.invocation.operationId === undefined
+      ? {}
+      : { operationId: requestContext.invocation.operationId }),
+    ...(requestContext.invocation.surface === undefined
+      ? {}
+      : { surface: requestContext.invocation.surface }),
   },
   session: requestContext.session,
   workspace: requestContext.workspace,

packages/agent-bundle/src/dev/watcher.ts

@@ -36,7 +36,7 @@ const relativePath = (root: string, path: string): string | undefined => {
 const defaultPathSignature = async (path: string): Promise<string | undefined> => {
   try {
     const source = await stat(path, { bigint: true });
-    return `${source.dev}:${source.ino}:${source.size}:${source.mtimeNs}`;
+    return `${source.dev}:${source.ino}:${source.size}:${source.mtimeNs}:${source.mode}:${source.ctimeNs}`;
   } catch (error) {
     if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined;
     throw error;

packages/agent-bundle/tests/adapter-metadata.test.ts

@@ -114,7 +114,7 @@ it('records exact immutable metadata for every built-in target', () => {
       {
         name: 'marketplace',
         revision: '2.1.250',
-        sha256: '44e105038ced3fceee4cb7ff81c7caad63965e3fd5a42b6db6255da771236b5d',
+        sha256: '4ffa94e8024966e8080b9d3b338c9612bdfa255802a8491b43f74f90427bc988',
       },
       {
         name: 'mcp',

packages/agent-bundle/tests/claude-plugin-validation.test.ts

@@ -1,12 +1,104 @@
-import { dirname, resolve } from 'node:path';
+import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { dirname, join, resolve } from 'node:path';
 
-import { expect, it } from '@rstest/core';
+import { afterEach, expect, it } from '@rstest/core';
 
 import {
   validateClaudePlugin,
+  validateClaudePluginFiles,
   type ClaudePluginCommandRunner,
 } from '../src/host-contracts/claude-plugin-validation.ts';
 
+const fixtureRoots: string[] = [];
+
+afterEach(async () => {
+  await Promise.all(fixtureRoots.splice(0).map((root) => rm(root, { force: true, recursive: true })));
+});
+
+const pluginWithNumberOption = async (
+  option: Readonly<Record<string, unknown>>,
+  location: 'channel' | 'plugin' = 'plugin',
+): Promise<string> => {
+  const root = await mkdtemp(join(tmpdir(), 'agent-bundle-claude-validation-'));
+  fixtureRoots.push(root);
+  const pluginDirectory = join(root, '.claude-plugin');
+  await mkdir(pluginDirectory, { recursive: true });
+  const userConfig = {
+    count: {
+      description: 'Number of items.',
+      title: 'Count',
+      type: 'number',
+      ...option,
+    },
+  };
+  await writeFile(join(pluginDirectory, 'plugin.json'), `${JSON.stringify({
+    author: { name: 'Fixture' },
+    description: 'Fixture plugin.',
+    name: 'fixture-plugin',
+    version: '1.0.0',
+    ...(location === 'plugin'
+      ? { userConfig }
+      : { channels: [{ server: 'fixture', userConfig }] }),
+  }, null, 2)}\n`);
+  return root;
+};
+
+it('rejects a numeric userConfig minimum greater than its maximum', async () => {
+  const pluginDirectory = await pluginWithNumberOption({ max: 5, min: 10 });
+
+  await expect(validateClaudePluginFiles({
+    pluginDirectory,
+    target: 'claude',
+  })).resolves.toEqual([expect.objectContaining({
+    code: 'AB6012',
+    message: expect.stringContaining('minimum must be less than or equal to its maximum'),
+  })]);
+});
+
+it('rejects a numeric userConfig default below its minimum', async () => {
+  const pluginDirectory = await pluginWithNumberOption({ default: 4, min: 5 });
+
+  await expect(validateClaudePluginFiles({
+    pluginDirectory,
+    target: 'claude',
+  })).resolves.toEqual([expect.objectContaining({
+    code: 'AB6012',
+    message: expect.stringContaining('default must be greater than or equal to its minimum'),
+  })]);
+});
+
+it('rejects a numeric channel userConfig default above its maximum', async () => {
+  const pluginDirectory = await pluginWithNumberOption({ default: 11, max: 10 }, 'channel');
+
+  await expect(validateClaudePluginFiles({
+    pluginDirectory,
+    target: 'claude',
+  })).resolves.toEqual([expect.objectContaining({
+    code: 'AB6012',
+    message: expect.stringContaining('default must be less than or equal to its maximum'),
+  })]);
+});
+
+it('accepts numeric userConfig defaults within declared bounds', async () => {
+  const pluginDirectory = await pluginWithNumberOption({ default: 7, max: 10, min: 5 });
+
+  await expect(validateClaudePluginFiles({
+    pluginDirectory,
+    target: 'claude',
+  })).resolves.toEqual([]);
+});
+
+it('handles numeric userConfig declarations with only one bound', async () => {
+  const minimumOnly = await pluginWithNumberOption({ default: 5, min: 5 });
+  const maximumOnly = await pluginWithNumberOption({ default: 10, max: 10 });
+
+  await expect(Promise.all([
+    validateClaudePluginFiles({ pluginDirectory: minimumOnly, target: 'claude' }),
+    validateClaudePluginFiles({ pluginDirectory: maximumOnly, target: 'claude' }),
+  ])).resolves.toEqual([[], []]);
+});
+
 const runWith = (
   validation: Readonly<{ exitCode: number; stderr?: string; stdout: string }>,
 ): { readonly calls: unknown[]; readonly run: ClaudePluginCommandRunner } => {

packages/agent-bundle/tests/dev-watcher.test.ts

@@ -1,4 +1,4 @@
-import { mkdtemp, mkdir, rm, unlink, writeFile } from 'node:fs/promises';
+import { chmod, mkdtemp, mkdir, rm, stat, unlink, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 
@@ -132,6 +132,47 @@ it('drops delayed source events until the path signature changes', async () => {
   await watcher.close();
 });
 
+it('invalidates a reported file after chmod changes only its executable mode', async () => {
+  const root = await mkdtemp(join(tmpdir(), 'agent-bundle-chmod-watcher-'));
+  const source = join(root, 'script.sh');
+  const fake = new FakeWatcher();
+  const invalidations: Invalidation[] = [];
+  await writeFile(source, '#!/bin/sh\nexit 0\n');
+  await chmod(source, 0o644);
+  const watcher = new ProjectWatcher({
+    createWatcher: () => fake,
+    debounceMs: 60_000,
+    onInvalidation: async (invalidation) => {
+      invalidations.push(invalidation);
+    },
+    root,
+  });
+
+  try {
+    fake.emit('add', source);
+    await watcher.flush();
+    const before = await stat(source, { bigint: true });
+
+    await chmod(source, 0o755);
+    const after = await stat(source, { bigint: true });
+    expect(after.size).toBe(before.size);
+    expect(after.mtimeNs).toBe(before.mtimeNs);
+    expect(before.mode & 0o111n).toBe(0n);
+    expect(after.mode & 0o111n).not.toBe(0n);
+
+    fake.emit('change', source);
+    await watcher.flush();
+
+    expect(invalidations).toEqual([
+      expect.objectContaining({ paths: ['script.sh'] }),
+      expect.objectContaining({ paths: ['script.sh'] }),
+    ]);
+  } finally {
+    await watcher.close();
+    await rm(root, { force: true, recursive: true });
+  }
+});
+
 it('waits for the real watcher root before reporting create, change, and delete source inputs', async () => {
   const root = await mkdtemp(join(tmpdir(), 'agent-bundle-real-watcher-'));
   await mkdir(join(root, 'src'), { recursive: true });