Repository navigation
fix: show license purchase guidance when free credits run out - #151
ivanmilevtues wants to merge 3 commits into
Conversation
CodeBoarding reviewStatus: 1 changed component See the full change in CodeBoarding. graph LR
n_Action_Workflow_Scripts["Action Workflow Scripts"]
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_Action_Workflow_Scripts modified;
|
c786341 to
f4da912
Compare
Reuse exit code 3 and existing failure reporting instead of adding a separate code, tier handling, and summary script. Amp-Thread-ID: https://ampcode.com/threads/T-01a153a0-19e7-744b-823a-cde0fc719d16 Co-authored-by: Amp <amp@ampcode.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Clarification: this PR is already ready for review, not a draft. The earlier draft wording was stale. The implementation is addressed: the Action preserves Core’s quota recovery message using the existing exit code 3. The remaining dependency is release sequencing, not an unfinished error-handling change. The Action currently installs codeboarding==0.15.1. That version does not contain Core #648’s new CodeBoarding purchase/reset guidance, so publishing this Action against that pin would still show the older Core message. Core #648 needs to be merged and released first; then action.yml and scripts/action/supported-providers.json must both be updated to the actual released version before shipping the Action. We have not guessed a future version or published a release as part of this fix. This does not block reviewing the PR now. It blocks shipping the complete user-visible fix until the engine pin is updated. |
CodeBoarding review · failedCodeBoarding quota is exhausted. Buy a CodeBoarding license at https://codeboarding.org/pricing, or wait for your weekly reset. See the workflow logs. Example of the quota-failure comment for this PR’s screenshot. This is a preview, not a report that this PR’s workflow exhausted its quota. |
Svilen-Stefanov
left a comment
Svilen-Stefanov
left a comment
There was a problem hiding this comment.
Looks good. Main thing: this shouldn't merge or at least shouldn't release before the codeboarding== pin bump is in. Couple of smaller nits below.
| stdout = "".join(stdout_lines) | ||
| if return_code != 0: | ||
| reason = ENGINE_REFUSALS.get(return_code) | ||
| if return_code == 3: |
There was a problem hiding this comment.
Why only exit 3? Exit 2 has the same issue imo: Core's own-key message names the key tail (Verify the key ending in ...) and the hosted one mentions id-token: write, and both get replaced by the generic "Check the key's secret". Could we drop the == 3 and prefer Core's line for any code in ENGINE_REFUSALS, keeping the dict only as fallback? Would also get rid of the magic 3 here :D
| # Keep Core's quota-specific recovery guidance instead of replacing it with provider billing advice. | ||
| for line in reversed(stderr_lines): | ||
| if "CodeBoarding: " in line: | ||
| reason = line.partition("CodeBoarding: ")[2].strip() |
There was a problem hiding this comment.
With the current 0.15.1 pin, Core prints CodeBoarding: the LLM provider's token or credit quota is exhausted. and the "Add credits ..." part on the next line without the prefix, so this would pick up only the first half, lowercase. So I think we should hold the merge until the pin bump (and supported-providers.json) lands here, right?
| if return_code == 3: | ||
| # Keep Core's quota-specific recovery guidance instead of replacing it with provider billing advice. | ||
| for line in reversed(stderr_lines): | ||
| if "CodeBoarding: " in line: |
There was a problem hiding this comment.
nit: "CodeBoarding: " is now a contract with Core's main.py. A named constant with a pointer to codeboarding_cli/main.py would help. Also, since we take the last match, any later stderr line that happens to contain it (shutdown or telemetry noise) would win. Probably fine today, just something to keep in mind.
|
|
||
| def test_quota_failure_preserves_cores_recovery_message(self) -> None: | ||
| messages = ( | ||
| "Your free CodeBoarding quota is exhausted. Buy a CodeBoarding license at " |
There was a problem hiding this comment.
nit: Core no longer says "Your free" since c64094b1 (CodeBoarding quota is exhausted. ...). Can we use the exact string Core emits so the fixture doesn't drift? Same for the PR description.
| with an error annotation saying which, and in review mode the failure comment says it too. | ||
| No partial analysis is published. | ||
|
|
||
| When your free CodeBoarding quota is exhausted, the failure reason links to |
There was a problem hiding this comment.
nit: same "free" wording here, Core's README dropped it. Fine either way, just keeping the two consistent.
2 participants
Preserve Core's quota recovery message instead of replacing it with generic provider-billing advice. Continue using the existing exit code
3and existing failure reporting for sync/review.For an exhausted CodeBoarding allowance, Core says:
Capture and forward stderr separately so successful stdout JSON is unchanged. Provider quota guidance remains provider-specific. No new exit code, credential-tier plumbing, workflow changes, or additional summary script.
Quota-exhausted message
The run remains failed, but the review comment and sync summary explain that the CodeBoarding quota is exhausted and link to purchasing a license, with waiting for the weekly reset as the alternative.
Local preview, not a live GitHub run: the real SDK received a mocked Lambda HTTP 402 response, Core produced the message and exit code 3, and the Action captured its
failure_reason. The review text comes fromaction.yml; the sync summary was generated byscripts/action/sync-summary.sh. Both were rendered using GitHub’s Markdown API. The full trace remains in the run logs; the comment and summary contain only the recovery guidance.Actual GitHub comment rendering
Screenshot of the example comment on this PR, captured from GitHub itself. The disclaimer identifies it as a preview rather than a real failed run.
Validation: all 257 Action tests passed, including subprocess recovery-message preservation and stdout/stderr separation. Black and diff checks passed. Cross-repository mocked HTTP checks verified the real SDK → Core CLI subprocess → Action failure_reason for hosted 402 and provider 429 insufficient_quota.
Release dependency: this PR is ready for review. Before shipping, release Core #648, then bump
codeboarding==inaction.ymland the matching engine version inscripts/action/supported-providers.json. The current 0.15.1 pin remains unchanged. An Action release must update the moving v1 tag.Both branches are
fix/free-credits-message. No AWS changes are required.