Release v1.54.0: MCP Registry readiness - #24
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 654d33bf9a
ℹ️ 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 (lifecycleStatus === "deprecated") { | ||
| addFinding(report.findings, "registry.lookup.deprecated", "warn", "Registry entry is deprecated."); |
There was a problem hiding this comment.
When the official Registry returns a record whose lifecycle is deprecated, this adds only a warning, so the subsequent status recomputation leaves the report at warn and the default CLI exits successfully. This also contradicts the newly added deprecated-record test, which expects report.status to be fail; the test suite will fail and callers can mistakenly accept a deprecated server.
Useful? React with 👍 / 👎.
| if (typeof entry.version === "string" && VERSION_RANGE_PATTERN.test(entry.version)) { | ||
| addFinding(findings, "registry.package.version-range", "fail", "Package version must be exact, not a range or latest.", `${packagePath}.version`); | ||
| } |
There was a problem hiding this comment.
For a package that omits version, this conditional performs no validation and the entry is still added to validPackages. For example, an npm package with an identifier and stdio transport but no version can produce a finding-free pass, causing --require-registry-readiness to exit 0 even though the advertised package cannot satisfy the documented exact-version publication requirement.
Useful? React with 👍 / 👎.
| if (packageJson && typeof server.name === "string") { | ||
| const npmPackages = validPackages.filter((entry) => entry.registryType === "npm"); | ||
| for (const entry of npmPackages) { | ||
| if (packageJson.name === entry.identifier) { |
There was a problem hiding this comment.
When an adjacent package.json exists but its name does not equal any declared npm package identifier, the loop adds no ownership finding and the npm-unverified branch is skipped solely because packageJson is non-null. The report can therefore remain pass and satisfy the strict readiness gate despite never checking the declared package's mcpName or version; this case should be treated like missing ownership evidence.
Useful? React with 👍 / 👎.
| "clientInstallability", | ||
| "overall" | ||
| ], | ||
| additionalProperties: false |
There was a problem hiding this comment.
The published JSON Schema marks the six scorecard fields as required but supplies no properties definitions while setting additionalProperties: false. Under JSON Schema 2020-12, every real scorecard member is consequently forbidden, making the schema impossible for any generated registry report to satisfy and breaking consumers that validate output against the advertised contract.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification