Repository navigation
fix: preserve Core authentication and quota recovery guidance - #151
ivanmilevtues wants to merge 4 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
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.
| # 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?
|
Svilen, pushed the Action review fixes in 1f3a36a:
Validation: all 257 Action tests passed; Black and diff checks passed. Posting these responses together because GitHub rejected inline replies while this account has an existing pending review. That draft review was left unchanged. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f3a36af77
ℹ️ 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".
| if return_code in ENGINE_REFUSALS: | ||
| for line in reversed(stderr_lines): | ||
| if ENGINE_ERROR_PREFIX in line: | ||
| reason = line.partition(ENGINE_ERROR_PREFIX)[2].strip() or reason |
There was a problem hiding this comment.
When a CodeBoarding allowance is exhausted, this code can only forward the message emitted by the installed CLI, but action.yml:407 still installs Core 0.15.1 and this patch does not generate the purchase/reset guidance locally. Therefore, shipping this commit before pinning the Core release that introduces that message leaves the affected run with the old generic provider-quota reason, so the advertised license link never appears. Update the engine pin and its mirrored provider metadata before releasing this change.
AGENTS.md reference: AGENTS.md:L14-L18
Useful? React with 👍 / 👎.
|
Follow-up verification for 1f3a36a: checked the parser against the matching Core branch and ran five actual Core CLI subprocess → Action parser cases with injected SDK errors. Customer quota, hosted upstream quota, own-provider quota, hosted authentication, and own-key authentication all preserved the expected recovery guidance in failure_reason. Both sides use the exact The changes are already pushed. The release dependency is unchanged: Core #648 must be merged and released, then action.yml and supported-providers.json must be updated together before merging/releasing this Action PR. The current 0.15.1 pin remains unchanged. |
Svilen-Stefanov
left a comment
There was a problem hiding this comment.
LGTM. Before merging, we should probably merge the core and release it so that we bump the version here.
Preserve Core's recovery guidance for both authentication failures (exit code
2) and exhausted quotas (exit code3). The Action prefers Core'sCodeBoarding:line and uses generic guidance only when Core provides no message. Successful stdout JSON remains separate from stderr diagnostics.CodeBoarding quota failures use the current Core wording:
Hosted upstream quota failures identify a CodeBoarding service issue instead of asking the customer to replenish our provider balance. Own-provider quota failures retain provider billing guidance. Authentication failures preserve the masked key tail or hosted workflow instructions.
The shared prefix is a named constant pointing to Core's
codeboarding_cli/main.py. Subprocess tests cover all three quota messages and both own-key and hosted authentication messages, including trailing unprefixed diagnostics.Merge/release dependency: Core #648 must be merged and released first. Before merging or releasing this Action PR, update both
codeboarding==inaction.ymland the engine version/provider table inscripts/action/supported-providers.jsonto that actual release. The current0.15.1pin remains unchanged because the Core fix is not released. Publishing the Action must also update the movingv1tag.Both branches are
fix/free-credits-message. No AWS changes are required.Quota failure previews
These existing previews show the review comment and sync summary presentation using the earlier quota wording; the exact current message is quoted above.
Validation: all 257 Action tests passed. Black formatting and
git diff --checkpassed.