Skip to content

feat(upgrade): render credit notices with billing URL (VOL-354) - #183

Open
tkkhq wants to merge 4 commits into
mainfrom
tkkhq/vol-354-credit-cli
Open

feat(upgrade): render credit notices with billing URL (VOL-354)#183
tkkhq wants to merge 4 commits into
mainfrom
tkkhq/vol-354-credit-cli

Conversation

@tkkhq

@tkkhq tkkhq commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Tracking

Why

  • VOL-180 locked in the low_credit_warning / not_enough_credit wire values and wired unreachable placeholder cases into PrintAPIInstructionNotices, but the printed text was a stub and the API's billing URL had nowhere to land
  • once the server starts emitting these instructions, users need a real, non-blocking low-credit nudge and a clear explanation of why a read-only account's requests are being rejected, styled consistently with the rest of the CLI's TTY output

Summary

  • add X-Volcano-Credit-URL to the VOL-180 instruction protocol in internal/api/version_protocol.go: recorded and cleared in lockstep with CLIInstruction/LatestVersion (sticky pairing — an unrelated later response never drops an earlier real URL)
  • render low_credit_warning as a non-blocking theme.Warn notice and not_enough_credit as a theme.Error notice explaining the account is read-only, both in internal/cmd/upgrade/upgrade.go; print the billing URL (theme.Command) underneath when the API supplied one
  • keep not_enough_credit non-gating on the CLI side — the API's own error response remains the actual blocker; this notice only adds context, printed the same way the existing upgrade/deprecation notices are
  • include CreditURL in ConsumeCLIInstructions's consumed-notice identity (cliInstructionPair) so a long-running command (e.g. a log-follow) that consumes a low_credit_warning/not_enough_credit notice, then observes the same instruction and latest version again but with a changed credit URL, renders the updated URL instead of suppressing it as an already-consumed repeat; version-only suggestion/deprecation behavior is unchanged since those instructions never carry a credit URL
  • scope the low-credit warning copy to the account instead of the project ("your account is running low on credit.") — the Hosting credit signal is stored per account/user, not per project
  • defer the interactive "upgrade / purchase credits" prompt from VOL-354's ticket scope — the API's direct billing URL is sufficient on its own and works in non-interactive commands (CI, pipes, machine output) where an interactive prompt cannot run; the companion VOL-354 work is implementing the billing-service integration this prompt would eventually call into; internal/confirm remains the primitive for that once there's a real target
  • update stale comments in both files that described these instructions as reserved/unreachable, and correct the doc comment on CLIInstructionLowCreditWarning/CLIInstructionNotEnoughCredit to describe the account's credit balance rather than the project's
  • document account credit notices and their non-interactive output behavior in docs/README.md

Preview

image

Verification

  • go build ./...
  • go test ./... -count=1
  • go test ./internal/api/... ./internal/cmd/upgrade/... ./cmd/volcano/... -count=5 -shuffle=on
  • make lint
  • make tidy (no go.mod/go.sum diff)
  • make build
  • make check

@tkkhq
tkkhq marked this pull request as ready for review September 3, 2026 19:36
@tkkhq
tkkhq requested a review from a team as a code owner September 3, 2026 19:36
Copilot AI balanced review requested due to automatic review settings September 3, 2026 19:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Document the new user-facing behavior and update the stale renderer comment before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds account-credit notices and API-provided billing URLs to the CLI instruction protocol.

Changes:

  • Tracks billing URLs alongside credit instructions.
  • Renders styled low-credit and read-only notices.
  • Tests URL persistence, updates, and terminal output.
File summaries
File Description
internal/cmd/upgrade/upgrade.go Renders credit notices and billing links.
internal/cmd/upgrade/upgrade_test.go Tests credit-notice output and URL changes.
internal/api/version_protocol.go Tracks billing URLs in instruction state.
internal/api/version_protocol_test.go Tests URL recording, persistence, and consumption.
Review details

Suppressed comments (1)

internal/cmd/upgrade/upgrade.go:63

  • The function's doc comment still says this renderer only handles upgrade suggestions or deprecation warnings and references LastInstructions, but these branches add credit notices and the implementation consumes them through ConsumeCLIInstructions. Update the comment so the documented behavior matches the expanded function.
	case api.CLIInstructionNotEnoughCredit:
		printNotEnoughCreditWarning(cmd, instructions.CreditURL)
	case api.CLIInstructionLowCreditWarning:
		printLowCreditWarning(cmd, instructions.CreditURL)
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +100 to +101
fmt.Fprintln(w, theme.Error("Your account is read-only:", on), "not enough credit to complete this request.")
printCreditURL(w, on, creditURL)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants