Repository navigation
docs(mcpx): level-2 interface pages A — interfaces, CLI, binary, scripts (#2108) - #2458
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 49 minutes. View limit details
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 |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5c03fa920
ℹ️ 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".
| | 0 | success | | ||
| | 1 | the command failed; the error is on stderr | | ||
| | 2 | an unknown command, an unknown global flag, or a settings conflict — including a contradiction in a config file, which is reported once, before dispatch | | ||
| | 75 | `EX_TEMPFAIL`: a call is waiting on an elicitation — a question a server has asked its client. Not a failure: answer it (`mcpx elicit`) and retry | |
There was a problem hiding this comment.
When a tool call exits with 75, answering its elicitation resumes the original call rather than cancelling it. handleCallReporting in internal/daemon/input_required.go keeps that call running as a task, and the elicitation command's declared help in internal/cli/manual.go explicitly directs users to collect it with mcpx task result. Following the retry instruction starts a second call, potentially repeating a state-changing operation. Replace the retry advice with instructions to collect the existing task's result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0384763 — verified against internal/cli/manual.go (the elicit entry) and internal/daemon/input_required.go: the call continues as a task, so the page now says to answer with mcpx elicit answer … and collect the original result with mcpx task result <id>, and explicitly warns that retrying starts a second call. Thank you — the retry wording came from the source issue and was wrong.
| - **Node runs only erasable TypeScript.** Node is invoked with | ||
| type transformation, not full compilation: syntax TypeScript | ||
| would erase (annotations, interfaces) is fine, and syntax it | ||
| would have to generate code for — an `enum` is the classic | ||
| case — fails. Write erasable TypeScript if the script must |
There was a problem hiding this comment.
For scripts targeting Node, this documents a restriction the runner deliberately removed: internal/runner/runtimes.go passes --experimental-transform-types, and TestNodeRunsNonErasableTypeScript guards successful enum execution. The installed Node's --help describes that flag as enabling transformation of TypeScript-only syntax into JavaScript, and an enum executed successfully with the runner's flags. Correct this limitation so authors are not instructed to rewrite supported constructs or unnecessarily avoid Node.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0384763 — verified against internal/runner/runtimes.go (the --experimental-transform-types rationale comment) and TestNodeRunsNonErasableTypeScript: the bullet now states Node transforms rather than strips, and that non-erasable syntax (enum, value namespaces, parameter properties) runs. The erasable-only claim was carried over from the source issue (#1851) and no longer holds.
Part A of the #1851 split (#2108): the prose level-2 pages for interfaces, the CLI, the binary, and scripts, at the anchors in the #1848 page map (
interfaces#abstract-interfaces,interfaces#client-interfaces,cli#cli,cli#cli-arguments,cli#cli-commands,binary#binary,scripts#script). Split B (#2109, MCP + /v1 pages) is untouched.Pages
pkgs/mcpx/docs/interfaces.md— one ops table behind every surface (with the honest note that routes are hand-registered and test-held, not generated), the parity claim quoted from the generateddocs/parity.mdwith its current test enforcement, and every client interface with a runnable snippet (generated TS client, CLI /mcpx call,DaemonClient+connect(), any MCP client, raw/v1).pkgs/mcpx/docs/cli.md— the loop,--json, bare/runnable first arguments, exit codes (incl. 75EX_TEMPFAILfor elicitation waits); cli-arguments as the two flag kinds (hand-parsed globals incmd/mcpx/main.govs registry flags bound per command), plus duplicate-spelling errors and derived flag names; cli-commands as overview prose over the six help groups — the command table itself is generated and linked (docs/parity.md), not hand-copied.pkgs/mcpx/docs/binary.md— what is shipped pergo.mod: seven direct requirements (incl.jsonschema/v6andwazero;docs/dependencies.mdandpackage.nixstill say five — noted on the page,go.modwins), no MCP SDK, the Nix build'sCGO_ENABLED=0static binary vs the Containerfile's defaultCGO_ENABLED=1, external programs, and the Nix wrapper'sPATHsuffix. Container detail is linked (docs/container.md), not restated.pkgs/mcpx/docs/scripts.md— the script contract (stdout/stderr, default export,--export,emit(),--jsonenvelope, unwrapped results /.raw/ToolError), named scripts and shadowing, runtimes (deno → bun → node; node is erasable-TypeScript only; typecheck is Deno-only), authority,exec.where,mcpx diagnose+ preflight, and recipes (// @param).Verification
Every factual claim was re-verified against current main (
5a34fa70) at claim time; several facts in #1851 had moved and the pages state the current ones: operations are 81 (per the generated parity table),mcpx openapiis now generated from the ops table (one OpenAPI document, not three), dispatch parity is held byTestEveryDispatchedCommandIsDeclared, the Nix package buildsCGO_ENABLED=0, anddaemon.leaseTTLderives--daemon-lease-ttl. No generated tables are hand-copied: configuration, environment and parity content is linked, per the triage notes. All relative links and backticked repo paths in the new pages were checked against the tree (the same ruleTestEveryDocumentationLinkResolvesenforces); docs CI on this PR is the authority.Queue task 3063 · Part of #1851 · Page map: #1848
Need help on this PR? Tag
@codesmith-botwith what you need.