Conversation
|
Preview deployed!
This preview shares the staging database and will be cleaned up when the PR is merged or closed. Run E2E testsnpm run e2e -- https://pr119-api.relaycast.dev --ciOpen observer dashboard |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8c0190178
ℹ️ 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".
| const result = await session.callTool(parsed.command, toolArgs); | ||
| writeToolResult(io.stdout, result, parsed.outputJson); | ||
| return 0; |
There was a problem hiding this comment.
Return non-zero when tool invocation reports isError
runCli always returns 0 after session.callTool, even when the MCP result indicates a tool-level failure (isError: true). In this repo, many expected failures are represented that way (for example, calling message.post before registration), so shell scripts and CI jobs using relaycast will treat failed operations as success and continue incorrectly. This should map tool error results to a non-zero exit code.
Useful? React with 👍 / 👎.
| if (!next || isFlag(next)) { | ||
| return { value: true, consumed: false }; | ||
| } |
There was a problem hiding this comment.
Reject missing values for value-taking flags
readFlagValue treats a missing value as boolean true for every flag, including string/number options. That causes malformed invocations to be accepted silently (for example relaycast workspace.set_key --api-key passes api_key: true, and relaycast --relay-base-url workspace.list consumes the command as the URL). The CLI should throw a clear error when non-boolean flags are missing a value instead of coercing to true.
Useful? React with 👍 / 👎.
index.ts: return non-zero exit when tool result has isError=true (P1) index.ts: reject missing values for non-boolean flags (P2) index.ts: handle negated --no-* flags correctly in parseCliArgs Co-Authored-By: My Senior Dev <dev@myseniordev.com>
| allowBooleanDefault = false, | ||
| ): { value: string | boolean; consumed: boolean } { | ||
| const next = argv[index + 1]; | ||
| if (!next || isFlag(next)) { |
There was a problem hiding this comment.
🟡 readFlagValue rejects empty-string values due to falsy check
The !next check on line 391 is true for both undefined (when there is no next argument) and '' (an empty string). This means relaycast message.post --text "" would throw "Missing value for --text" instead of accepting the empty string as the value. The same issue affects global flags via readGlobalFlagValue at packages/cli/src/index.ts:412. The --flag= inline syntax works around this because empty inline values go through inlineValue != null at packages/cli/src/index.ts:248, bypassing readFlagValue entirely.
| if (!next || isFlag(next)) { | |
| if (next == null || isFlag(next)) { |
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
@relaycast/cliwith arelaycastbinary backed by the MCP tool registrytools,help, andversionhelperscliinto the npm publish workflow, release notes, version sync, lockfile, and README docsRELAY_API_KEY/--relay-api-keyandRELAY_AGENT_TOKEN/--relay-agent-tokenVerification
npm installnpm run -w @relaycast/cli lintnpm run -w @relaycast/cli testnpm run -w @relaycast/cli buildnpx turbo build --filter=@relaycast/clinpx tsx packages/cli/src/cli.ts toolsNote:
npm installreports the existing audit count of 11 vulnerabilities (7 moderate, 4 high).