feat: add HTTP and MCP transport adapters for protocol v0.1 - #19
Conversation
![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: 2493778374
ℹ️ 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".
| expect(names).not.toEqual( | ||
| expect.arrayContaining([ |
There was a problem hiding this comment.
When an adapter exposes only one or several of these generic tools, this assertion still passes: negating arrayContaining fails only if the tool list contains every listed entry. For example, an adapter exposing execute_action and shell but not browser would remain green despite violating the test's explicit-tool boundary; check that each forbidden name is absent or that the intersection is empty.
AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.
| const response = await transport.callTool( | ||
| 'mindrail_create_goal', | ||
| createGoalArgs({ commandId: '', unexpectedAuthority: 'allow' }), | ||
| principal, |
There was a problem hiding this comment.
When callTool rejects the empty commandId but silently accepts or forwards unexpectedAuthority, this test still passes, so it does not establish the claimed unknown-field rejection at the invocation boundary. Use a valid command ID for the unexpected-property case (and test the invalid ID separately) so an implementation cannot satisfy the test for the wrong validation failure.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 389ef2be8f
ℹ️ 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 (!isReason(record.reason)) return 'reason must be a bounded Reason'; | ||
| if (!isBoundedString(record.summary, 1, 4000)) return 'summary must be a bounded string'; | ||
| if (!Array.isArray(record.evidence)) return 'evidence must be an array'; |
There was a problem hiding this comment.
When an HTTP or MCP caller supplies values such as evidence: [null], this check accepts the command and casts it to BlockTaskCommand, even though each item must be a canonical EvidenceRef with a uri, bounded optional fields, no unknown properties, and the array is limited to 32 entries. The malformed value therefore crosses the application boundary and can cause a handler to crash or attempt to create an invalid checkpoint; validate the array contents and bounds before dispatch.
AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.
| if (!isBoundedString(record.permission, 1, 128)) return 'permission must be a bounded string'; | ||
| if (!isBoundedString(record.justification, 1, 4000)) { | ||
| return 'justification must be a bounded string'; | ||
| } |
There was a problem hiding this comment.
For RequestPermission, inputs such as permission: "NOT VALID" and a 3,000-character justification pass this validation and are dispatched, although the canonical PermissionRequest schema requires a NamespacedName permission and limits justification to 2,000 characters. This lets malformed transport input cross the typed boundary and either fail later for the wrong reason or reach a handler that assumes canonical data; use the schema's pattern and bounds here.
AGENTS.md reference: AGENTS.md:L30-L30
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Scope
Implements issue #17 as a transport-only slice over the accepted MindRail v0.1 protocol.
This PR is being built with RED -> GREEN TDD. The current head intentionally contains only the transport contract tests; permanent
Qualityis expected to fail until the application/transport modules are added.Planned boundaries:
Final verification evidence, exact temporarily unsupported accepted operations, and dependency justification will be added before completion.