feat(plugins): expose install source facts#70951
Conversation
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — all findings are minor P2 style/coverage observations with no impact on correctness in realistic inputs. Both remaining comments are P2: one is an edge-case inconsistency only reachable with a whitespace-only integrity string (not a realistic catalog value), and the other is a missing test quadrant. No logic errors, data-loss risk, or broken contracts were found. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/plugins/install-source-info.ts
Line: 58-72
Comment:
**Whitespace-only `expectedIntegrity` produces inconsistent output**
`hasIntegrity` uses `.trim()` to guard against whitespace strings, but the spread that writes `expectedIntegrity` onto the `npm` object checks the raw value — a whitespace-only string passes the truthy guard and ends up in the output while `pinState` says `"exact-without-integrity"` and `"npm-spec-missing-integrity"` is in `warnings`. All three fields would disagree about whether integrity is present.
```ts
const trimmedIntegrity = install.expectedIntegrity?.trim();
const hasIntegrity = Boolean(trimmedIntegrity);
// ...
npm = {
// ...
...(trimmedIntegrity ? { expectedIntegrity: trimmedIntegrity } : {}),
};
```
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/install-source-info.test.ts
Line: 1-58
Comment:
**Missing coverage for `exact-without-integrity` pin state**
The three existing tests cover `exact-with-integrity`, `floating-without-integrity`, and `invalid-npm-spec`. The fourth combination — an exact semver spec that omits `expectedIntegrity` — is never exercised. That case emits only `"npm-spec-missing-integrity"` and produces `pinState: "exact-without-integrity"`, which is a meaningful operational scenario (pinned version, no integrity hash). Adding a test for it would complete the four-quadrant coverage of the `PluginInstallNpmPinState` union.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(plugins): expose install source fac..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4579a9304a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf28e0a8d4
ℹ️ 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".
Summary
Validation
pnpm test src/plugins/install-source-info.test.ts src/plugins/provider-install-catalog.test.ts src/channels/plugins/contracts/plugins-core.catalog.entries.contract.test.tspnpm format:check src/plugins/install-source-info.ts src/plugins/install-source-info.test.ts src/plugins/provider-install-catalog.ts src/plugins/provider-install-catalog.test.ts src/channels/plugins/catalog.ts test/helpers/channels/channel-plugin-catalog-contract-suites.ts docs/plugins/architecture-internals.md docs/plugins/manifest.mdpnpm test:contracts:channelspnpm test:contracts:pluginspnpm tsgo:corecheck found no touched-file errorsKnown failures
pnpm check:changedfails insrc/agents/openai-transport-stream.ts/src/plugin-sdk/provider-tools.tson existing OpenAI compat type errorspnpm buildreachesbuild:plugin-sdk:dtsand fails on the same unrelated OpenAI compat type errorsNotes