fix(claude): harden adapter declaration planning - #321
Conversation
Validate and lower Claude declarations before they can ship unresolved or host-invalid configuration.
🦋 Changeset detectedLatest commit: f89f461 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fe79466e9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!emittedMarketplacePluginNames.has(entry)) { | ||
| diagnostics.push(dependencyDiagnostic( | ||
| 'claude.dependencies.unresolved', | ||
| `Claude dependency ${JSON.stringify(entry)} has no marketplace, but the generated marketplace does not emit a plugin with that name.`, | ||
| 'Declare the marketplace that provides this plugin, or remove the dependency; bare names resolve only within the generated marketplace.', | ||
| )); |
There was a problem hiding this comment.
Update all bare-dependency regression fixtures
When the full unit suite reaches a non-self bare dependency, this new branch reports claude.dependencies.unresolved because emittedMarketplacePluginNames contains only the generated plugin's own name. However, packages/agent-bundle/tests/api.test.ts:265-279 and packages/agent-bundle/tests/plugin-bundle.test.ts:623-637 still declare bare audit-logger dependencies and expect no diagnostics plus manifest emission, so pnpm test:unit will fail even though the narrowed Claude adapter suite was updated. Update those remaining fixtures/expectations to the new marketplace-qualified contract, or retain the prior behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already addressed before this PR merged: commit 53bc86f ("test(claude): qualify unified dependency fixture") on this branch updated both fixtures to the marketplace-qualified contract. Verified on current main: tests/api.test.ts (lines 321–336) and tests/plugin-bundle.test.ts (lines 624–637) declare { marketplace: 'acme-shared', name: 'audit-logger' } / policy-kit, and pnpm test:unit passes in CI on #397 (d25a9c6).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Keep the unified target proof aligned with emitted-marketplace dependency resolution.
Validate and lower Claude declarations before they can ship unresolved or host-invalid configuration.
Keep the unified target proof aligned with emitted-marketplace dependency resolution.
…' into fix/review-batch2-claude-adapter
Keep packed-consumer validation aligned with the newly merged Codex validator while retaining failure checks.
Keep dependency, host-validation, and project-status proofs current with the merged compiler surfaces.
Resolve the shared project-status expectation in favor of the updated main-branch contract.
Keep the Runtime restart leak proof relative to server-owned subscriptions as the server adds streams.
Retain main's stronger two-host packed validation proof while integrating concurrent build fixes.
Keep artifact-only CLI validation aligned with the newly merged opt-out flag.
Use main's exact two-subscription runtime baseline from the merged integration fixture alignment.
Summary
Test plan
pnpm buildpnpm typecheckpnpm lint