Repository navigation
schemas: publish external target protocol schemas - #709
Conversation
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 The Case of Pull Request #709Schema Sets
Schema Set
|
|
@codex review please Self-Code Review Findings
|
Activity Summary
Verification:
Merge gate status: open except for the repository review policy ( |
![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: 03534e3811
ℹ️ 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".
| "apiVersion": { "const": "wesley.target-artifact-manifest/v1" }, | ||
| "items": { | ||
| "type": "array", | ||
| "uniqueItems": true, |
There was a problem hiding this comment.
For manifests where a target returns two items with the same path but a different kind or sha256, draft-07 uniqueItems compares the whole artifact object, so both entries still satisfy this schema even though the protocol says repeated paths are invalid and later copy-out validation must treat them as a hard failure. Since this file is published as the machine-readable artifact-manifest contract, consumers that gate on schema validation alone can accept conflicting outputs; either make the schema boundary explicit and add semantic-validation coverage, or enforce path-level uniqueness outside this array-level check.
Useful? React with 👍 / 👎.
| }, | ||
| "capabilities": { | ||
| "type": "object", | ||
| "required": ["inputs", "outputs", "requiresNetwork", "requiresAmbientFilesystem"], |
There was a problem hiding this comment.
When a descriptor omits requiresNetwork and requiresAmbientFilesystem, the current target verify implementation treats both fields as false via serde defaults and accepts the descriptor, but this required list makes the published schema reject the same input before it reaches the CLI. That leaves target authors with two conflicting validators for the same wesley.target-descriptor/v1 contract; either require the fields in the CLI as well or relax the schema to match the shipped verifier defaults.
Useful? React with 👍 / 👎.
| }, | ||
| "workspaceRelativePath": { | ||
| "type": "string", | ||
| "pattern": "^(?!/)(?!.*(^|/)\\.\\.(/|$))(?!.*[:\\\\\\u0000])(?=.*[^./]).+" |
There was a problem hiding this comment.
For artifact paths containing a newline before a forbidden suffix, e.g. generated/hello\n/../escape, ECMA-style . in JSON Schema patterns does not cross the newline and this pattern has no end anchor, so it can match only the safe prefix and still validate the full string. That bypasses the documented ban on parent-directory segments, backslashes, colons, and NULs for consumers relying on the published schema; anchor the pattern to the whole value and avoid .* assumptions across line terminators or reject line terminators explicitly.
Useful? React with 👍 / 👎.
1 participant
Branch / Issue-Title Check
Branch
schemas/external-target-protocoland titleschemas: publish external target protocol schemasmatch #708 and use no reserved prefixes.Why
#680 now has an external-process protocol spec and PR #707 shipped descriptor verification, but target authors still lacked canonical JSON Schemas for the protocol envelopes.
Changes
generated_json_artifacts.Closes #708
Related to #680
Risk
Low. This publishes schema artifacts and tests only. It does not add
target run, execute target code, sandbox processes, or copy artifacts.Backout
Revert
03534e38to remove the schema files, protocol schema tests, and documentation references.Testing
cargo test -p wesley-core --test generated_json_artifacts external_target_protocol_artifacts_satisfy_declared_schemas(RED before schemas, GREEN after schemas)cargo test -p wesley-core --test generated_json_artifactscargo fmt --checkpnpm exec prettier --check schemas/README.md docs/reference/external-target-protocol.md CHANGELOG.md schemas/wesley-target-descriptor-v1.schema.json schemas/wesley-target-request-v1.schema.json schemas/wesley-target-response-v1.schema.json schemas/wesley-target-diagnostic-v1.schema.json schemas/wesley-target-artifact-manifest-v1.schema.jsoncargo xtask docs-checkgit diff --checkpnpm run preflight