Repository navigation
Publish Skills schemas and validate specification examples in CI - #166
anencore94 wants to merge 3 commits into
Conversation
|
@codex review Please review the full PR at 6071129, especially schema conversion correctness, inherited base constraints, and false-success paths in the CI checks. Self-review fixes preserve literal/property names during conversion and validate examples before writing artifacts. Local schema checks pass (34 tests, 16 documented examples). |
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. |
![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: 60711295f0
ℹ️ 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".
| } else if ("error" in message) { | ||
| name = "JSONRPCErrorResponse"; | ||
| } else if ("result" in message && requests.has(message.id)) { | ||
| name = `${requests.get(message.id)}ResultResponse`; |
There was a problem hiding this comment.
For a resources/read example with resultType: "complete", this selects ReadResourceResultResponse, whose result union includes InputRequiredResult. That alternative only requires a string resultType, so replacing the existing read response's result with {"resultType":"complete","contents":"bad","ttlMs":-1,"cacheScope":"bad"} still passes all 16 example checks, despite the invalid content and inherited cache fields. Both CI and --write therefore accept this malformed success response. Validate the completed-result branch explicitly in the example checker, leaving the released specification unchanged.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
|
@codex review Addressed the completed resource-read validation finding in 6c32371. After validating the response envelope, the example checker now explicitly validates Validation: eight negative regression cases reproduced the bypass before the fix; Node 22 Please re-review the latest commit, including the success-branch selection and regression coverage. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@olaservo could you please approve the pending GitHub Actions CI run for this fork PR at Local validation on Node 22 passed all 43 schema tests and 16 published examples; the documentation build also passed. Codex re-review of this head completed with no findings: #166 (comment) This is a request to authorize CI execution only; I'm not requesting a code review yet. Thank you! |
|
Hi @anencore94 I just approved CI to run. |
Closes #158.
The released Skills page defines wire types in prose, so an implementation can omit required cache attributes while its own tests still pass. This adds consumable TypeScript and JSON Schema plus a CI job that checks the artifact, all 13 documented declarations, and all 16 JSON message examples.
schema/beta/schema.tsmirrors the released interfaces and imports a pinned, vendored copy of the2026-07-28base types. JSON generation uses the core specification'stypescript-json-schemagenerator and integer-number convention; generation and validation are offline after dependency installation.ttlMsorcacheScope. Tests also cover inherited request metadata, cache constraints, dynamic manifests, type/JSDoc drift, and malformed or unmapped examples.resultType: "complete"requirement.Validation (local): Initial validation on Node 22.23.4; follow-up self-review checks on Node 23.11.0,
npm run check:schema(34 tests and all 16 examples pass);npm run docs:build; production preview HTTP smoke check; pinned base re-fetch produces identical artifacts; a deliberately stale JSON artifact is rejected without being rewritten;git diff --check. GitHub CI has not executed the jobs yet: run 41511523641 isaction_required, with no job results available.The schema layout and identifier are proposed in the decision log. The
$iddoes not itself deploy a hosting endpoint. Automated code review is requested before further human review requests.AI assistance: Codex assisted with implementation, local validation, and this description.