Fix CLI Approval Polling Bugs (surfaced by #63)#95
Open
googleknight wants to merge 2 commits intostripe:mainfrom
Open
Fix CLI Approval Polling Bugs (surfaced by #63)#95googleknight wants to merge 2 commits intostripe:mainfrom
googleknight wants to merge 2 commits intostripe:mainfrom
Conversation
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
Addresses CLI-side issues surfaced by #63 (spend-request approval flow getting stuck on
pending_approval).What's Fixed
1. Recognize All Terminal Statuses in Approval Polling
The interactive
spend-request retrieveTUI only treatedapprovedanddeniedas terminal statuses. Any other terminal status —expired,succeeded,failed,canceled— caused continued polling until the local timeout fired, ultimately surfacing a misleading "Timed out waiting for approval" error for requests that had already reached a terminal state on the server.The same misclassification affected:
useApprovalPollinghook (used by interactivecreateandrequest-approval)demoflows (demo/card-flow,demo/spt-flow)In all of these,
pollUntilApprovedresolves on any non-pending terminal status (per its own unit-test contract), but every call site treated resolution as success — meaning a denied or expired request was rendered as "✓ Spend request approved".Fix: Introduced a shared
TERMINAL_STATUSESset in the TUI (mirroring the JSON path's set incommands/spend-request/index.tsx) with a new'finalized'render branch. Added explicitstatus === 'approved'checks inuseApprovalPolling,demo/card-flow, anddemo/spt-flow.2. Extend Default Polling Window Past Server-Side Expiry
The Link approval UI shows an ~8-minute expiry countdown, but
retrieveOptions.timeoutdefaulted to 300 s, and the_next.commandhint emitted bycreate/request-approvalrecommended--interval 2 --max-attempts 150(also 300 s).Users or agents racing the 8-minute window could hit
POLLING_TIMEOUTpurely because the CLI gave up before the request itself expired.Fix:
--timeoutdefault to 600 s_next.commandhint to--max-attempts 300Polling now comfortably outlives the server-side expiry. No effect when callers pass explicit values.
What's Not Fixed (and Why)
The root cause of #63 — clicking "Approve" in the Link web app and the UI silently failing to transition the request out of
pending_approval— occurs in the Link web application, which is not in this repository.The CLI faithfully reports whatever status the API returns; only the API can advance the state machine. These changes fix adjacent CLI bugs that became visible because of the incident, not the incident's root cause itself.
Test Plan
pnpm typecheckpnpm testpnpm biome check .Relevant test coverage:
packages/cli/src/__tests__/cli.test.ts:711–781— exercises--max-attemptsexhaustion,--timeoutexpiry, and terminal-status detection in JSON-mode polling integration tests.