feat(alibaba): add token-plan routing and cloud discovery - #105
Merged
Conversation
…ersion - Auto-populate allowed origins in local mode too: with token auth on, the origin check is enforced, and the previously empty allowlist rejected every request carrying an Origin header — breaking all session-stream WebSocket handshakes with 403/1006. REST GETs worked only because browsers omit Origin on same-origin GET fetches. - Standardize the web/vis startup banners on a shared PYTHINKER wordmark in utils/server.py, replacing the legacy upstream art. - Serve the installed CLI version at runtime via /api/config (GlobalConfig.version); the web UI header now prefers it over the Vite build-time constant, which goes stale when the CLI is upgraded without a frontend rebuild. Rebuilt the bundled static assets. - Add regression tests for local-mode origin population and the empty-allowlist reject-all semantics.
… agent glyphs Selectively adopted from an earlier TUI iteration, re-based onto the current design system: - report.py: detect top-level "Label: body" lines in report-like assistant prose and render them as structured sections, with conservative guards so ordinary paragraphs stay plain Markdown. - markdown.py: wide multi-column report tables render as stacked records so long paths and prose wrap in one generous value column instead of being sliced mid-word across narrow grid cells; compact tables keep the bordered grid. - todo: normalize LLM-supplied status aliases (complete/completed/ finished -> done, canceled -> cancelled) via a before-validator. - agent renderer: status glyphs (check/cross/dot) and type-first row layout for subagent activity. Deliberately NOT adopted (superseded by the current standardized design): the question-marker and markdown-palette recolors, and the space-separated tool header format with column-grid wrapping, which conflicts with the pinned parenthesized header style.
GHSA-866g-f22w-33x8 (uncontrolled resource consumption) affects @ai-sdk/provider-utils <=3.0.97, which every ai@5.x release pins; the patched 4.x line ships only with ai@6. The web UI imports the ai package exclusively for types (ChatStatus, FileUIPart, ToolUIPart, LanguageModelUsage), so the major bump is type-level only — tsc and biome pass unchanged. The remaining elliptic advisory (GHSA-848j-6mx2-7j84, low) has no patched release in any version; it enters via vite-plugin-node-polyfills -> crypto-browserify at build time only, and the bundle never includes it because only the path and url polyfills are enabled.
…strap Three field-found fixes: - memory/recall: frame the recalled-memory block as background context from past sessions, not an instruction. Without the guard the model could treat a recalled note or stale todo as the current request (e.g. answering a plain "ping" by resuming an old code-review task). Open todos are now labelled reference-only. - background tasks: track tasks spawned during the current interactive turn and kill exactly those on ESC. Previously a background subagent launched mid-turn survived the interrupt, finished later, and re-delivered the abandoned task via its completion notification. Earlier turns' tasks are deliberately left running. - web auth: consume the URL token before React mounts instead of in a component effect. Mount-time data fetches fired first and sent a stale localStorage token from a previous server run, yielding 401s on first load.
- web config API: log get_version() failures instead of swallowing them, so an operator can see when the version banner falls back to empty - usePythinkerVersion: reset the shared promise and log on a failed/empty fetch so a transient error no longer permanently disables the backend version banner for the session - test_web_origins: rename unused *args to *_args to signal intent - CHANGELOG: add the missing ## Unreleased entry for this PR's web fixes (unblocks the required changelog-entry-required check) - AGENTS.md: document the changelog-entry-before-PR requirement as a gotcha to stop this check repeatedly blocking PRs
Plan keys (sk-sp-/sk-tok-/sk-ws-) now route to the shared international Token Plan endpoint instead of coding-intl, fixing a 401 invalid_api_key on every chat. Generic sk- keys remain pay-as-you-go Cloud (DashScope) with US/China auto-detect. Model discovery accepts every chat model the live /models returns (excluding image/audio/embed/rerank/etc), infers capabilities + context window from the id, and defaults first login to qwen3.7-plus. Removed the dead Coding-Plan endpoint branches and constant. Usage: the Token Plan exposes no quota API and its policy forbids automated balance polling, so /usage on Token Plan/Coding endpoints shows Pythinker's local token tally plus a console pointer (My Subscriptions / Usage Analysis) instead of probing a 404 endpoint or scraping the billing console. CI: split the docs Pages workflow into build + deploy jobs (one artifact per run) to fix the 'multiple artifacts named github-pages' deploy failure. Verified live: Token Plan and Cloud endpoints both return /models 200 and a real chat completion.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors Alibaba provider auth: adds a shared Token Plan base URL and default model/context, simplifies base URL normalization, adds regex-based model discovery and capability inference, routes subscription plan keys to the Token Plan endpoint, probes China endpoint on 401/403, and skips quota API calls for certain DashScope hosts; tests and CI/gitignore/changelog updated. ChangesAlibaba provider routing and model discovery
Workflow and repository configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
elkaix
changed the title
Anchor the maas.aliyuncs.com suffix check on a leading dot so spoofed hosts (e.g. evilmaas.aliyuncs.com) no longer match the token-plan quota-skip path, resolving the CodeQL py/incomplete-url-substring- sanitization alert. Also wrap the long ALIBABA_TOKEN_PLAN_BASE_URL literal to satisfy ruff format.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/docs-pages.yml (1)
8-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope elevated GitHub token permissions to deploy only.
pages: writeandid-token: writeare still granted at workflow scope, so thebuildjob gets privileges it does not need. Keep onlycontents: readglobally and leavepages/id-tokenondeployto enforce least privilege.Suggested patch
permissions: contents: read - pages: write - id-token: writeAlso applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-pages.yml around lines 8 - 11, Top-level workflow permissions are too permissive—remove the global pages: write and id-token: write entries from the permissions block and leave only contents: read at workflow scope; then add pages: write and id-token: write to the permissions for the deploy job (the job named "deploy") so only deploy has those privileges; also remove the duplicate elevated entries referenced around lines 83-85 and ensure any build job keeps only the minimum (contents: read) permission.
♻️ Duplicate comments (1)
src/pythinker_code/ui/shell/usage_adapters/alibaba.py (1)
53-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHarden endpoint host matching in
_endpoint_without_quota_api.Line 53 uses
netloc, and Line 54 uses a raw substring ("dashscope" in host). This can misclassify hosts (e.g., ports/userinfo in netloc, or non-authoritative substring matches), which flips the quota-probe branch incorrectly.Suggested patch
def _endpoint_without_quota_api(base_url: str) -> bool: @@ - host = urlparse(base_url).netloc - if host.startswith("coding") and "dashscope" in host: + host = (urlparse(base_url).hostname or "").lower() + labels = host.split(".") + if host.startswith("coding") and "dashscope" in labels: return True return host.startswith("token-plan") and host.endswith(".maas.aliyuncs.com")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pythinker_code/ui/shell/usage_adapters/alibaba.py` around lines 53 - 56, The host matching in _endpoint_without_quota_api is brittle because it uses urlparse(...).netloc and substring checks; instead parse the URL with urlparse and use parsed.hostname (to exclude userinfo/ports), lower-case it, then perform label-aware checks: for the "dashscope" case split hostname on '.' and check that one of the labels equals "dashscope" (or check exact label positions if known) combined with the appropriate leading label check (e.g., hostname.startswith("coding.") if intended), and for the aliyun case use hostname.endswith(".maas.aliyuncs.com") (after lowercasing and using parsed.hostname) or compare exact labels rather than raw substring; update the code in _endpoint_without_quota_api to use parsed.hostname and these label-aware comparisons for robust matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/auth/test_alibaba_auth.py`:
- Around line 741-742: The parametrized test
test_login_alibaba_plan_key_routes_to_token_plan is missing the "sk-ws-test-key"
case; update the `@pytest.mark.parametrize` decorator for parameter "key" to
include "sk-ws-test-key" alongside "sk-sp-test-key" and "sk-tok-test-key" so the
websocket-prefixed plan key is asserted to route to the Token Plan in this
consolidated test.
---
Outside diff comments:
In @.github/workflows/docs-pages.yml:
- Around line 8-11: Top-level workflow permissions are too permissive—remove the
global pages: write and id-token: write entries from the permissions block and
leave only contents: read at workflow scope; then add pages: write and id-token:
write to the permissions for the deploy job (the job named "deploy") so only
deploy has those privileges; also remove the duplicate elevated entries
referenced around lines 83-85 and ensure any build job keeps only the minimum
(contents: read) permission.
---
Duplicate comments:
In `@src/pythinker_code/ui/shell/usage_adapters/alibaba.py`:
- Around line 53-56: The host matching in _endpoint_without_quota_api is brittle
because it uses urlparse(...).netloc and substring checks; instead parse the URL
with urlparse and use parsed.hostname (to exclude userinfo/ports), lower-case
it, then perform label-aware checks: for the "dashscope" case split hostname on
'.' and check that one of the labels equals "dashscope" (or check exact label
positions if known) combined with the appropriate leading label check (e.g.,
hostname.startswith("coding.") if intended), and for the aliyun case use
hostname.endswith(".maas.aliyuncs.com") (after lowercasing and using
parsed.hostname) or compare exact labels rather than raw substring; update the
code in _endpoint_without_quota_api to use parsed.hostname and these label-aware
comparisons for robust matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9c4b5a53-340d-41f1-8d4c-66e48072124d
📒 Files selected for processing (6)
.github/workflows/docs-pages.yml.gitignoreCHANGELOG.mdsrc/pythinker_code/auth/alibaba.pysrc/pythinker_code/ui/shell/usage_adapters/alibaba.pytests/auth/test_alibaba_auth.py
The docstring claims sk-sp-/sk-tok-/sk-ws- all route to the Token Plan endpoint, but the parametrization only exercised sk-sp- and sk-tok-. Add sk-ws-test-key so the consolidated assertion matches the docstring (addresses CodeRabbit nitpick on #105).
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
2 participants
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.
Related Issue
Resolve #(issue_number)
Description
Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.Summary by CodeRabbit
New Features
Bug Fixes
Updates
Tests