Skip to content

fix(alibaba): support Personal token plan quota fetches - #2515

Open
wdmitchelluk wants to merge 4 commits into
steipete:mainfrom
wdmitchelluk:fix/alibaba-personal-sec-token
Open

fix(alibaba): support Personal token plan quota fetches#2515
wdmitchelluk wants to merge 4 commits into
steipete:mainfrom
wdmitchelluk:fix/alibaba-personal-sec-token

Conversation

@wdmitchelluk

@wdmitchelluk wdmitchelluk commented Jul 30, 2026

Copy link
Copy Markdown

Summary

  • resolve and send the dashboard sec_token with International and China Mainland Personal token-plan requests
  • prefer an existing cookie sec_token immediately, avoiding dashboard and user-info preflight delays; if it is rejected, resolve a different token and retry once
  • remove stale hardcoded workspace-switch fields so Alibaba uses the workspace associated with the active session
  • send the Personal subscription lookup in Alibaba's current nested request shape
  • surface nested Alibaba API failures instead of reporting only the outer HTTP 200 status
  • keep Personal failures on the Personal path rather than substituting potentially unrelated Team quota data
  • preserve the direct International and China Mainland Team request path while applying the same one-shot stale-token recovery
  • parse required Personal usage before starting the two optional metadata lookups, then run those optional lookups concurrently

Root cause

Every Personal request included a hardcoded Alibaba workspace switch. With the reporter's International Personal account, Alibaba returned HTTP 200 with a nested BailianGateway.Workspace.NotAuthorised failure for that workspace, so CodexBar did not receive usable Personal usage data.

The fix removes that stale workspace override and uses the active authenticated session. It does not fall back to Team data when a Personal request fails.

Redacted authenticated proof

Verified against commit 43f77a92c1ef2ff9d066b16a1f35da59bae10ea0 using the reporter's existing CodexBar/Brave International Personal session. Cookies, tokens, account identifiers, request identifiers, hosts, endpoints, and local paths are omitted.

$ CodexBarCLI usage --provider alibaba-token-plan --source web --format json --pretty
provider: alibabatokenplan
source: web
plan: Standard
primary window: present
secondary window: present
error: none
[exit 0]

The authenticated command was run twice against the exact final source and succeeded both times. Before the fix, the same session received HTTP 200 with nested BailianGateway.Workspace.NotAuthorised.

Verification

Check Result
International Personal Authenticated live test plus request/response regression tests pass
China Mainland Personal Request/response regression tests for the China Personal route, product code, token, request shape, and parser pass
International Team Direct Team route regression test using the unchanged Team implementation passes
China Mainland Team Direct Team route regression test using the unchanged Team implementation passes
Cookie sec_token Regression test proves the cookie token is used immediately and preflight is skipped
Nested HTTP 200 failure Regression test proves the nested Alibaba error is surfaced
Personal/Team isolation Regression test proves a failed Personal request performs one Personal POST and never falls through to Team data
Focused Alibaba suite 51 tests in 8 suites pass, including stale-token recovery for International and China Mainland Personal and Team routes
Full repository suite (prior head 81fd0343) make test: 758 selections in 64 groups; 64 first-pass successes, 0 failures, 0 retries, 0 timeouts
Repository checks make check: SwiftFormat, SwiftLint, localization, documentation, package, signing, and CI policy checks pass with 0 formatting or lint violations
Security check GitGuardian passes on the final commit

Authenticated live coverage is available for International Personal only because the available session is an International Personal account. Team and China Mainland behavior is covered by deterministic request/response regression tests; this PR does not claim those variants were live-tested with unavailable account credentials.

Refs #2349 and #2500.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 261ebc4b2b

ℹ️ 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".

Comment thread Sources/CodexBarCore/Providers/Alibaba/AlibabaTokenPlanUsageFetcher.swift Outdated
@wdmitchelluk
wdmitchelluk marked this pull request as draft July 30, 2026 15:01
@wdmitchelluk wdmitchelluk changed the title fix(alibaba): include SEC token for Personal plans fix(alibaba): support Personal token plan quota fetches Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 30, 2026
@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 30, 2026, 5:14 PM ET / 21:14 UTC.

ClawSweeper review

What this changes

The PR changes Alibaba Token Plan Personal and Team quota fetches to use session-derived sec_token values, retry an authentication-rejected token once, parse Personal responses and nested API failures correctly, and add focused regression tests.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

This PR remains necessary: it targets active Alibaba Token Plan Personal quota failures and supplies redacted authenticated proof for the International Personal path plus focused regression coverage. The patch has no confirmed correctness defect, but its token-refresh changes are compatibility-sensitive for existing cookie and session flows, so it should receive normal human maintainer review rather than automated closure or repair.

Priority: P2
Reviewed head: 43f77a92c1ef2ff9d066b16a1f35da59bae10ea0

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The PR has meaningful redacted live proof and broad focused regression coverage; its compatibility-sensitive session retry remains the main maintainer review consideration.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body provides redacted terminal proof of two successful authenticated International Personal runs against the final commit, with deterministic request/response coverage for unavailable China Mainland and Team account variants.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body provides redacted terminal proof of two successful authenticated International Personal runs against the final commit, with deterministic request/response coverage for unavailable China Mainland and Team account variants.
Evidence reviewed 4 items PR scope and regression coverage: The branch changes the Alibaba quota fetcher and its focused provider tests. The PR body reports 51 tests across 8 Alibaba suites, including International and China Mainland Personal and Team stale-token cases.
Resolved prior review concerns: The latest branch commit explicitly addresses the prior review feedback: cookie sec_token values are used for the first request, then an authentication/token failure causes one refresh attempt that avoids repeating the rejected token.
After-fix real behavior proof: The PR body contains redacted terminal output from two successful authenticated International Personal CLI runs against the final commit, with an observed Standard plan, both usage windows present, no error, and exit code 0.
Findings None None.
Security None None.

How this fits together

CodexBar’s Alibaba Token Plan provider reads browser or manually supplied session cookies, sends regional quota requests to Alibaba, and converts responses into usage windows for the menu bar and CLI. This PR changes the token-selection and retry stage between cookie ingestion and Personal or Team quota requests.

flowchart LR
    A[Browser or manual cookies] --> B[Alibaba Token Plan provider]
    B --> C[Resolve session token]
    C --> D[Personal or Team quota request]
    D --> E[Parse usage and API errors]
    E --> F[Usage snapshot]
    F --> G[Menu bar and CLI output]
Loading

Before merge

  • Resolve merge risk (P1) - Existing browser-cookie and manually pasted-cookie users can fail quota refreshes if the new one-shot retry misclassifies an Alibaba authentication response or selects a token that is valid for a different session context.
  • Resolve merge risk (P1) - The supplied live proof covers International Personal only; China Mainland and Team routes have deterministic request/response coverage but no authenticated live-account confirmation.
  • Complete next step (P2) - No discrete repair finding remains, but a maintainer should review the compatibility-sensitive token-refresh contract before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 2 files affected; 500 additions, 118 deletions The patch is tightly scoped to one provider implementation and its dedicated regression suite, while substantially changing request authentication behavior.
Focused provider coverage 51 tests in 8 suites reported The reported coverage spans Personal and Team routes, both regions, nested failures, and stale-token recovery.

Merge-risk options

Maintainer options:

  1. Confirm compatibility before merge (recommended)
    Review the stale-token and direct-route tests to confirm existing Personal and Team cookie sessions make an unchanged first request and retry only after a clear authentication failure.
  2. Pause for broader live validation
    If deterministic route coverage is insufficient for this provider, wait for redacted authenticated China Mainland or Team evidence before merging.

Technical review

Best possible solution:

Preserve the existing first-request cookie behavior, retain the one-shot recovery only for clearly authentication-related failures, and land the focused test-backed fix after a maintainer confirms the Personal and Team compatibility contract.

Do we have a high-confidence way to reproduce the issue?

No high-confidence independent current-main reproduction was established in this read-only review. The failure mechanism is source-reproducible from the described hardcoded workspace and nested-response paths, and the contributor supplies redacted authenticated after-fix evidence for International Personal.

Is this the best way to solve the issue?

Yes, provisionally: removing the stale Personal workspace override, preserving Personal/Team separation, surfacing nested API errors, and retrying only authentication-token failures is a focused solution within the existing provider boundary. Maintainer review should confirm the retry classification remains compatible with current cookie-session behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a.

Labels

Label justifications:

  • P2: This is a bounded provider bug fix for authenticated Alibaba quota retrieval, with no evidence of a core-runtime outage.
  • merge-risk: 🚨 compatibility: The patch changes how existing manual-cookie and browser-cookie sessions make initial and retry requests.
  • merge-risk: 🚨 auth-provider: The provider now derives, rejects, refreshes, and retries Alibaba session tokens differently.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides redacted terminal proof of two successful authenticated International Personal runs against the final commit, with deterministic request/response coverage for unavailable China Mainland and Team account variants.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides redacted terminal proof of two successful authenticated International Personal runs against the final commit, with deterministic request/response coverage for unavailable China Mainland and Team account variants.

Evidence

What I checked:

  • PR scope and regression coverage: The branch changes the Alibaba quota fetcher and its focused provider tests. The PR body reports 51 tests across 8 Alibaba suites, including International and China Mainland Personal and Team stale-token cases. (Tests/CodexBarTests/AlibabaTokenPlanProviderTests.swift, 43f77a92c1ef)
  • Resolved prior review concerns: The latest branch commit explicitly addresses the prior review feedback: cookie sec_token values are used for the first request, then an authentication/token failure causes one refresh attempt that avoids repeating the rejected token. (Sources/CodexBarCore/Providers/Alibaba/AlibabaTokenPlanUsageFetcher.swift, 43f77a92c1ef)
  • After-fix real behavior proof: The PR body contains redacted terminal output from two successful authenticated International Personal CLI runs against the final commit, with an observed Standard plan, both usage windows present, no error, and exit code 0. (43f77a92c1ef)
  • Related active reports: The PR explicitly references the still-open International Personal parsing report and China Mainland Personal API-response report; it is a candidate fix rather than evidence that either issue should close before merge. (43f77a92c1ef)

Likely related people:

  • unknown: The supplied review context identifies the Alibaba fetcher and its tests as the central surface, but it does not provide sufficient current-main git-history provenance to name a person without guessing. (role: likely current-main area owner; confidence: low; files: Sources/CodexBarCore/Providers/Alibaba/AlibabaTokenPlanUsageFetcher.swift, Tests/CodexBarTests/AlibabaTokenPlanProviderTests.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Confirm that the authentication-error classification and one-shot retry preserve existing manual-cookie and browser-cookie behavior for Personal and Team routes.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (6 earlier review cycles)
  • reviewed 2026-07-30T15:44:32.910Z sha 697097d :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-30T17:07:58.335Z sha 81fd034 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-30T19:47:14.295Z sha 43f77a9 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-30T19:50:34.276Z sha 43f77a9 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-30T20:09:29.388Z sha 43f77a9 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T20:13:56.716Z sha 43f77a9 :: needs maintainer review before merge. :: none

Use the active Alibaba session instead of a hardcoded workspace, prefer an existing SEC token without preflight delay, and keep Personal failures from falling through to Team quota data.
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 30, 2026
@wdmitchelluk
wdmitchelluk marked this pull request as ready for review July 30, 2026 17:41

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 81fd03439c

ℹ️ 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".

Comment thread Sources/CodexBarCore/Providers/Alibaba/AlibabaTokenPlanUsageFetcher.swift Outdated
Retry Personal and Team quota requests once with a freshly resolved token when a cookie-backed token is rejected, without delaying the initial request or repeating the rejected token.

Copy link
Copy Markdown
Author

@codex review

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex review is not enabled for this repo. Please contact the admins of this repo to enable Codex.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 30, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 30, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant