fix(cli): harden phala docs grep pattern passing and MCP request timeout - #487
Open
Leechael wants to merge 3 commits into
Open
fix(cli): harden phala docs grep pattern passing and MCP request timeout#487Leechael wants to merge 3 commits into
Leechael wants to merge 3 commits into
Conversation
A pattern starting with `-` would otherwise be parsed by rg as flags (e.g. `-foo` becomes `-f oo` and fails with a confusing "No such file or directory" error). The `--` separator was considered first but the docs virtual filesystem shell does not support it (verified live: `rg ... --` returns "unrecognized option '--'"), while `-e` works for both regular and dash-leading patterns.
The errorTests loop allows individual cases up to 10s via the execa timeout, but bun:test kills any test at its default 5s timeout. Cases that make a real API round trip (e.g. "cvms get invalid-id-format") exceed 5s on slower networks and fail locally and in the pre-commit hook even though the command behaved correctly. Give each case a bun timeout 5s above its execa timeout so the execa deadline always wins. CI was unaffected because its API latency is lower.
Every "phala docs" call previously used a bare fetch with no timeout, so a hung docs server would block the CLI indefinitely -- especially painful for agent-driven usage. - callDocsTool now applies AbortSignal.timeout, defaulting to 60s (matching the CLI-wide default) with a per-call timeoutMs override - new resolveDocsToolOptions helper wires the global --timeout flag (resolveTimeoutSeconds) into all five docs subcommands, so docs commands behave like every other command - timeout failures get a distinct, actionable error message instead of the generic network-error one
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Review follow-ups from #485 (which merged before these could land):
docs grep: pass the pattern via-e. A pattern starting with-would otherwise be parsed by rg as flags (e.g.-foobecomes-f ooand fails with a confusing "No such file or directory" error).--was considered first, but the docs virtual filesystem shell does not support it (verified live:rg ... --returns "unrecognized option '--'"), while-eworks for both regular and dash-leading patterns.fetchwith no timeout, so a hung docs server would block the CLI indefinitely -- especially painful for agent-driven usage.callDocsToolappliesAbortSignal.timeout(default 60s, matching the CLI-wide default, overridable per call), and a newresolveDocsToolOptionshelper wires the global--timeoutflag into all five docs subcommands. Timeout failures get a distinct, actionable error message instead of the generic network-error one.errorTestsloop inerror-messages.test.tsallows cases up to 10s via the execa timeout, but bun:test kills any test at its default 5s. Cases with a real API round trip (e.g.cvms get invalid-id-format) exceed 5s on slower networks and fail locally / in the pre-commit hook despite correct behavior. Each case now gets a bun timeout 5s above its execa timeout. CI was unaffected because its API latency is lower.Testing
bun run fmt / lint / type-check / testall clean (469 tests, 0 fail)phala docs grep --files PHALA_CLOUD_API_KEYworks unchangedrg -Sn -e '-foo' /returns exit 1 (no matches) instead of the flag-parsing errorPHALA_DOCS_MCP_URL=http://10.255.255.1/mcp phala docs search test --timeout 2aborts in ~2.3s with: "Phala docs server at ... did not respond within 2s. Try again shortly, or browse https://docs.phala.com directly."