feat: add modelCatalog manifest contract

[shakkernerd]

This PR adds the first manifest-contract slice for the model catalog architecture. It does not change models list, onboarding, configure, provider loading, or runtime behavior yet.

What changed:

• Added modelCatalog to plugin manifests.
• Added typed manifest metadata for:
  • provider-owned catalog rows
  • provider aliases
  • cross-provider suppressions
  • discovery mode: static, refreshable, or runtime
• Preserved modelCatalog on PluginManifestRecord.
• Added manifest-registry coverage proving the metadata survives registry loading.
• Documented the new manifest contract in docs/plugins/manifest.md.

Contract boundaries:

• modelCatalog.providers only accepts provider ids owned by the manifest’s top-level providers.
• modelCatalog.discovery only accepts owned provider ids.
• modelCatalog.aliases may use any alias key, but the alias target must be an owned provider.
• modelCatalog.suppressions may reference other providers because suppression is explicitly cross-source.
• Model token fields follow existing model config constraints:
  • contextWindow and maxTokens must be positive numbers.
  • contextTokens must be a positive integer.
• modelCatalog.cost.tieredPricing[] requires complete tier rows: input, output, cacheRead, cacheWrite, and range.
• compat only preserves known typed model compatibility fields instead of accepting arbitrary objects.

Why:

• This establishes the public manifest shape before command paths consume it.
• It keeps the plugin system manifest-first, so plugins can declare model/provider metadata without adding runtime discovery code.
• It gives later PRs a durable contract for fast, read-only model listing, onboarding, model pickers, cache refresh, aliases, and suppression.

Verification:

• pnpm test src/plugins/manifest-registry.test.ts
• pnpm check:changed
• pnpm test src/plugins/manifest-registry.test.ts src/plugins/manifest-registry.ts src/plugins/manifest.ts

[greptile-apps]

Greptile Summary

This PR introduces the modelCatalog manifest slice — new TypeScript types, a full normalization pipeline with ownership/allowlist enforcement, a pass-through in PluginManifestRecord, and matching docs. The implementation is well-structured with good proto-pollution guards and field-level validation. One type-contract inconsistency needs attention: PluginManifestModelCatalogTieredCost declares cacheRead and cacheWrite as optional (?), but the normalizer silently drops any tier entry that omits either field, which contradicts both the type and the PR's stated "complete tier rows required" constraint.

Confidence Score: 3/5

Safe to merge after fixing the tiered-cost type mismatch; no runtime paths are affected yet, but the type contract should be correct before consumers are added.

One P1 finding (type/normalization contract divergence for tiered cost cacheRead/cacheWrite) pulls the score below the P1 ceiling of 4. Change is otherwise well-tested and isolated to manifest parsing with no live consumers yet.

src/plugins/manifest.ts — PluginManifestModelCatalogTieredCost type definition and normalizeStringMap variable shadowing.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/plugins/manifest.ts
Line: 50-56

Comment:
**Type contract diverges from normalization for tiered cost**

`cacheRead` and `cacheWrite` are declared optional (`?`) in the type, but `normalizeModelCatalogTieredCost` silently drops any tier entry where either is `undefined` (lines 722–729). A plugin author reading the TypeScript type would reasonably omit these fields — and then watch their entire tier row disappear from the registry without any warning. The type should mark both fields as required to match the documented constraint ("requires complete tier rows") and the actual normalization behavior.

```suggestion
export type PluginManifestModelCatalogTieredCost = {
  input: number;
  output: number;
  cacheRead: number;
  cacheWrite: number;
  range: [number, number] | [number];
};
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/plugins/manifest.ts
Line: 435

Comment:
**Variable shadows outer function parameter**

The local `const value` here shadows the `value: unknown` function parameter. After `Object.entries(value)` is already called this doesn't produce a runtime bug, but it can confuse readers and may trigger a `no-shadow` lint warning. Renaming the inner variable avoids the ambiguity.

```suggestion
    const strValue = normalizeOptionalString(rawValue) ?? "";
```

…and update the subsequent usage on the next line accordingly:
```ts
    if (key && strValue) {
      normalized[key] = strValue;
    }
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: require complete model catalog pric..." | Re-trigger Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25de885215

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[shakkernerd]

Landed via rebase onto current main.

• Gate: pnpm test src/plugins/manifest-registry.test.ts src/plugins/manifest-registry.ts src/plugins/manifest.ts
• Gate: pnpm check
• Gate: pnpm build
• Gate: pnpm check:changed
• Addressed and resolved Greptile/ChatGPT review threads.
• Rebased PR head before merge: 98d35ec
• Final landing commit: 6d27176

Thanks @shakkernerd!