Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/lint-missing-config-envelope-invalid.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@ai-plugin-marketplace/core': patch
---

Fix `aipm lint` staying silent for a plugin whose `aipm.config.ts` cannot be resolved — cases
`aipm validate` (hard `envelope-invalid`) and `aipm build` (thrown `ConfigLoadError`) already
caught. The `schema/envelope-shape` rule now fires for every envelope-load failure, not only a
schema violation: a plugin-shaped directory missing `aipm.config.ts`, and a config file that is
present but cannot be imported (syntax error, no usable default export). Both emit the same
`envelope-invalid`-backed, `error`-severity diagnostic `validate` reports for the identical tree,
carrying the config loader's own message verbatim, so the three surfaces agree instead of `lint`
reporting a clean bill of health for a broken plugin.
9 changes: 7 additions & 2 deletions docs/specs/lint-engine.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,13 @@ The existing zod schemas (per-target `schemas.ts`, config schemas in `config.ts`
through L-D3 so every issue has a range. Zod remains the sole authority (L-D8). Two migrated
rules carry this category:

- `schema/envelope-shape` — `aipm.config.ts` parses strictly against the envelope schema; legacy
code `envelope-invalid`.
- `schema/envelope-shape` — `aipm.config.ts` loads and parses strictly against the envelope
schema. Fires on _any_ failure to resolve the envelope, not only a schema violation: a missing
`aipm.config.ts` in a plugin-shaped directory, a file that cannot be imported (syntax error, no
usable default export), or a file that imports but violates the schema (#101). The first two
carry the config loader's own message; the third expands into one diagnostic per Zod issue. In
every case this mirrors `validate()`'s `envelope-invalid` for the same tree — `lint`, `validate`
and `build` must agree rather than one staying silent; legacy code `envelope-invalid`.
- `schema/target-conformance` — every target manifest in a plugin's envelope parses against that
target's current schema; legacy codes `schema-invalid`, plus the Open Plugins-specific
`metadata-dir-isolation` and `open-plugins-conformance` (soft, advisory-only) findings the same
Expand Down
103 changes: 103 additions & 0 deletions packages/core/src/lint/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as os from 'node:os';
import * as path from 'node:path';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { runBuild } from '../pipeline/build.js';
import { runValidate } from '../pipeline/validate.js';
import { synthRegistryRepo } from '../test-support/synth-plugin.js';
import type { SynthRegistryRepo } from '../test-support/synth-plugin.js';
import { lint } from './engine.js';
Expand Down Expand Up @@ -220,3 +221,105 @@ describe('lint()', () => {
});
});
});

// Regression coverage for #101: #91 made `runValidate` (hard `envelope-invalid`) and `runBuild`
// (thrown `ConfigLoadError`) catch a plugin-shaped directory missing `aipm.config.ts`, but `lint()`
// stayed silent for the identical tree. `lint` and `validate` must agree on this case.
describe('lint() — plugin-shaped repo-root subdirectory missing aipm.config.ts (#91, #101)', () => {
it('agrees with runValidate(): both report the equivalent envelope-invalid finding for the same tree', async () => {
// Repro shape from #91: a plugins/* dir with a target manifest but no aipm.config.ts.
write('plugins/broken/.claude-plugin/plugin.json', { name: 'broken', version: '0.1.0' });

const [lintResult, validateResult] = await Promise.all([lint(repoRoot), runValidate(repoRoot)]);

// validate() surfaces its existing hard envelope-invalid finding (unchanged by this fix, #91).
const envelopeFindings = validateResult.findings.filter((f) => f.code === 'envelope-invalid');
expect(envelopeFindings).toHaveLength(1);
expect(envelopeFindings[0]?.plugin).toBe('broken');
expect(envelopeFindings[0]?.severity).toBe('hard');
expect(validateResult.passed).toBe(false);

// lint() must now surface the equivalent diagnostic — same rule as a malformed config
// (`schema/envelope-shape`), `error` severity (validate's `hard` maps to lint's `error`,
// L-D2's findingToDiagnostic), carrying the legacy `envelope-invalid` code, and the identical
// message validate reports — not silent, as it was pre-fix.
const lintEnvelopeDiagnostics = lintResult.diagnostics.filter(
(d) => d.legacyCode === 'envelope-invalid',
);
expect(lintEnvelopeDiagnostics).toHaveLength(1);
expect(lintEnvelopeDiagnostics[0]).toMatchObject({
ruleId: 'schema/envelope-shape',
category: 'schema',
severity: 'error',
file: 'broken',
});
expect(lintEnvelopeDiagnostics[0]?.message).toBe(envelopeFindings[0]?.message);
});

it('emits nothing further beyond the envelope-invalid diagnostic for the config-less plugin', async () => {
// No other rule can run without a resolved envelope (engine.ts's `continue` after the failed
// loadPluginConfig call) — mirroring validate()'s short-circuit on an unusable envelope.
write('plugins/broken/.claude-plugin/plugin.json', { name: 'broken', version: '0.1.0' });

const result = await lint(repoRoot);

expect(result.diagnostics.filter((d) => d.file === 'broken')).toHaveLength(1);
});
});

// #101, follow-on: the same lint/validate disagreement exists for an `aipm.config.ts` that is
// present but cannot be imported (syntax error, no usable default export). `runValidate` reports
// a hard `envelope-invalid` for it; `lint()` previously returned no diagnostic at all, so a
// syntax-broken envelope still got a clean bill of health from `lint`.
describe('lint() — plugin whose aipm.config.ts exists but fails to import (#101)', () => {
it('agrees with runValidate(): both report the equivalent envelope-invalid for an unparseable config', async () => {
write('plugins/broken/.claude-plugin/plugin.json', { name: 'broken', version: '0.1.0' });
// Deliberately unparseable TypeScript — the transpile/import step throws, so the failure
// never reaches the envelope schema and is not a ZodError.
write('plugins/broken/aipm.config.ts', 'export default {{{ not valid typescript\n');

const [lintResult, validateResult] = await Promise.all([lint(repoRoot), runValidate(repoRoot)]);

const envelopeFindings = validateResult.findings.filter((f) => f.code === 'envelope-invalid');
expect(envelopeFindings).toHaveLength(1);
expect(envelopeFindings[0]?.plugin).toBe('broken');
expect(envelopeFindings[0]?.severity).toBe('hard');
expect(validateResult.passed).toBe(false);

const lintEnvelopeDiagnostics = lintResult.diagnostics.filter(
(d) => d.legacyCode === 'envelope-invalid',
);
expect(lintEnvelopeDiagnostics).toHaveLength(1);
expect(lintEnvelopeDiagnostics[0]).toMatchObject({
ruleId: 'schema/envelope-shape',
category: 'schema',
severity: 'error',
file: 'broken',
});
// Byte-identical to validate's: both take the ConfigLoadError's own message.
expect(lintEnvelopeDiagnostics[0]?.message).toBe(envelopeFindings[0]?.message);
});

it('still reports the schema-violation case as per-issue envelope-invalid diagnostics', async () => {
// A config that imports cleanly but violates the envelope schema stays on the ZodError path,
// which expands into one diagnostic per issue rather than a single loader-message diagnostic.
write('plugins/broken/.claude-plugin/plugin.json', { name: 'broken', version: '0.1.0' });
write(
'plugins/broken/aipm.config.ts',
"import { defineConfig } from '@ai-plugin-marketplace/core';\n" +
"export default defineConfig({ version: 'not-semver', targets: ['not-a-target'] });\n",
);

const result = await lint(repoRoot);

const envelopeDiagnostics = result.diagnostics.filter(
(d) => d.legacyCode === 'envelope-invalid',
);
expect(envelopeDiagnostics.length).toBeGreaterThan(0);
for (const d of envelopeDiagnostics) {
expect(d.ruleId).toBe('schema/envelope-shape');
expect(d.severity).toBe('error');
expect(d.message).toMatch(/^Invalid aipm\.config: \[/);
}
});
});
5 changes: 3 additions & 2 deletions packages/core/src/lint/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ export async function lint(targetPath: string, options?: LintOptions): Promise<L
try {
envelope = (await loadPluginConfig(pluginDir, configCache)).targets;
} catch {
// Unusable envelope: envelopeShapeRule (or the config loader itself) already explains why;
// no other rule can run without a resolved envelope.
// Unusable envelope: envelopeShapeRule ran first against this same pluginDir and has
// already reported why (missing/unimportable/schema-violating aipm.config.ts), so this
// failure is never silent; no other rule can run without a resolved envelope.
continue;
}

Expand Down
51 changes: 28 additions & 23 deletions packages/core/src/lint/rules/legacy-envelope-shape.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,31 @@
/**
* `schema/envelope-shape` — `aipm.config.ts` parses strictly against the envelope schema
* `schema/envelope-shape` — `aipm.config.ts` loads and parses strictly against the envelope schema
* (§10.1 step 1). Wraps the existing `validateEnvelopeShape()` check.
*
* Unlike the other migrated rules, this one runs *before* a plugin's envelope (`ctx.envelope`)
* is known — it is what determines whether the envelope can be trusted at all. It therefore
* loads the raw `aipm.config.ts` default export itself rather than reading anything off
* `RuleContext.envelope`, and reports nothing when the config is missing entirely (a missing
* envelope is `load-config.ts`'s concern, not a shape-validation one).
* loads `aipm.config.ts` itself rather than reading anything off `RuleContext.envelope`.
*
* Reuses `ctx.configCache` (via {@link loadPluginConfig}) rather than importing the raw config a
* second time: `loadPluginConfig` transpiles-and-validates once and caches the result, so when
* the engine's subsequent envelope resolution calls `loadPluginConfig` for the same plugin, it is
* a cache hit rather than a second jiti transpile. On a `ConfigLoadError` whose `cause` is the
* `ZodError` `defineConfig` throws for a schema violation, this rule reformats those issues into
* the same per-issue diagnostics `validateEnvelopeShape` would have produced from the raw value;
* any other failure (import/syntax error) is left to load-config's own `ConfigLoadError` path in
* `validate()`'s orchestration.
* Every way loading can fail is reported here, so `lint` never stays silent on a tree `validate`
* and `build` both flag (#101): a missing `aipm.config.ts` (including a plugin-shaped directory
* discovered without one, per #91), a file that cannot be imported (syntax error, no usable
* default export), and a file that imports but violates the envelope schema. The first two become
* a single `envelope-invalid` diagnostic carrying the loader's own message; the third is expanded
* into the same per-issue diagnostics `validateEnvelopeShape` would have produced from the raw
* value. This catch mirrors `runValidate()`'s envelope-load catch in `pipeline/validate.ts`, so
* the two surfaces agree by construction rather than by a duplicated message string.
*
* Loading goes through {@link loadPluginConfig} with `ctx.configCache` rather than importing the
* raw config a second time: `loadPluginConfig` transpiles-and-validates once and caches the
* result, so when the engine's subsequent envelope resolution calls `loadPluginConfig` for the
* same plugin, it is a cache hit rather than a second jiti transpile.
*
* @see docs/specs/lint-engine.md L-D2
*/

import * as fs from 'node:fs';
import * as path from 'node:path';
import { z } from 'zod';
import {
AIPM_CONFIG_FILENAME,
ConfigLoadError,
loadPluginConfig,
} from '../../pipeline/load-config.js';
import { ConfigLoadError, loadPluginConfig } from '../../pipeline/load-config.js';
import { zodEnvelopeIssuesToFindings } from '../../pipeline/validate.js';
import { findingToDiagnostic } from '../diagnostic.js';
import type { Diagnostic, InternalRuleContext, Rule } from '../types.js';
Expand All @@ -45,8 +43,6 @@ export const envelopeShapeRule: Rule = {
appliesTo: ['aipm-repo'],
},
async check(ctx: InternalRuleContext): Promise<Diagnostic[]> {
const configPath = path.join(ctx.pluginDir, AIPM_CONFIG_FILENAME);
if (!fs.existsSync(configPath)) return [];
const pluginName = path.basename(ctx.pluginDir);
try {
// A cache hit here (already validated by an earlier call in this invocation) means the
Expand All @@ -58,9 +54,18 @@ export const envelopeShapeRule: Rule = {
const findings = zodEnvelopeIssuesToFindings(err.cause.issues, pluginName);
return findings.map((f) => findingToDiagnostic(f, RULE_ID, 'schema', docsUrlFor(RULE_ID)));
}
// Import failure (syntax error, etc.) is reported by load-config's own ConfigLoadError
// path in validate()'s orchestration; nothing further to add here.
return [];
// Missing file, import/syntax failure, or an unexpected throw. Reported exactly as
// runValidate()'s envelope-load catch reports it — same code, same severity, and the
// loader's own message verbatim, so lint and validate cannot drift apart.
const message = err instanceof Error ? err.message : String(err);
return [
findingToDiagnostic(
{ severity: 'hard', code: 'envelope-invalid', plugin: pluginName, message },
RULE_ID,
'schema',
docsUrlFor(RULE_ID),
),
];
}
},
};
Loading