Make agentic terms review and refusal guidance actionable - #246
Merged
Merged
Conversation
developersdigest
marked this pull request as ready for review
September 17, 2026 14:16
Contributor
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 4/5
- In
src/commands/terms.ts, 403 refusals send guidance only to stdout, so stderr consumers may miss the promised refusal message; write the refusal guidance to stderr while keeping the JSON payload on stdout.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/terms.ts">
<violation number="1" location="src/commands/terms.ts:74">
P2: When a 403 is returned, `handle` emits this guidance only through stdout, so the promised refusal guidance is missing from stderr. Also write the refusal message to stderr while retaining the JSON payload on stdout.</violation>
</file>
Heads up: you’re close to your flex budget. Increase your flex budget so reviews don’t pause.
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
| success: false, | ||
| status: response.status, | ||
| ...(response.status === 403 && { | ||
| guidance: { |
Contributor
There was a problem hiding this comment.
P2: When a 403 is returned, handle emits this guidance only through stdout, so the promised refusal guidance is missing from stderr. Also write the refusal message to stderr while retaining the JSON payload on stdout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/terms.ts, line 74:
<comment>When a 403 is returned, `handle` emits this guidance only through stdout, so the promised refusal guidance is missing from stderr. Also write the refusal message to stderr while retaining the JSON payload on stdout.</comment>
<file context>
@@ -66,7 +66,18 @@ export async function requestTerms(
+ success: false,
+ status: response.status,
+ ...(response.status === 403 && {
+ guidance: {
+ message:
+ 'Terms access was refused. Show this error to the user and ask an organization admin to review access in Settings. Do not retry or accept automatically.',
</file context>
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
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
Agents receiving a provider terms refusal need an explicit review and approval step, including when they only consume CLI stdout. Preserve the API action metadata and include guidance in JSON and stderr. Terms reads request explicit user approval; 403 responses retain the original error and point admins to Settings.
Adds regression coverage for presentation without acceptance, access refusal without retries, missing confirmation/version/digest, exact confirmed payloads, stale agreements, and ambiguous success responses. No real terms were accepted.
Follow-up to merged CLI #237/#239; related to firecrawl/exchange#563 and #540. Acceptance uses the existing endpoint from firecrawl/firecrawl#4668: POST /exchange/provider-terms/accept with provider, version, digest and confirmed:true. Its organization and credential come from the authenticated key, and retrieval consults the acceptance ledger. A failed catalog GET is separate from acceptance availability. No additional Core PR is required by this change.
Validation
No npm release or production deployment performed.
Existing acceptance implementation: firecrawl/firecrawl#4668. Companion MCP update: firecrawl/firecrawl-mcp-server#405.