Repository navigation
Per-project provider credentials — Aliyun Bailian (DashScope) / Qwen support - #6
Conversation
Introduce PROVIDER_CATALOG as the static registry of model providers the
platform can authenticate against, keyed default base URLs by wire
protocol (anthropic/openai) because one provider — Bailian/DashScope —
exposes both a claude-code endpoint and an OpenAI-compatible mode. The
auth policy field ('project' vs 'env') lets submit-time checks require a
per-project credential for providers that have no legitimate deployment
env fallback. Widen claude-agent-sdk and codex-cli to serve dashscope so
Qwen models resolve through the existing executors, and add the
write-only create/update request schemas for the credential endpoints.
One row per (project, provider) with the API key encrypted in the existing secrets table (new kind provider_api_key) — the same secretRef pattern repo connections and MCP servers use. base_url is nullable because the provider catalog carries the protocol-keyed defaults; a row value only overrides for regional endpoints. Seed the current Qwen generation (qwen3.7-max/plus, qwen3.6-flash) at entry-tier list prices — Bailian tiers rates by input length, so operators should verify against the live price list.
Add an executor-agnostic ProviderAuth field to StepExecutionRequest and an overlayProviderAuth helper that applies it on top of the scrubbed env. The helper deletes the wire protocol's entire auth-var family before setting the mapped vars — leaving any member behind would let a worker-env credential outrank the project's, inverting the decided precedence (project credential wins, env is the deployment fallback). Native anthropic keys map to ANTHROPIC_API_KEY; gateway credentials (Bailian) map to bearer ANTHROPIC_AUTH_TOKEN plus the catalog's protocol-keyed base URL, overridable per credential row for regional endpoints. The field is optional so FakeExecutor and the compliance fixtures are unaffected.
…codex The claude adapter overlays the credential onto its scrubbed env — a Bailian credential lands as bearer ANTHROPIC_AUTH_TOKEN + the gateway base URL. The codex adapter does the same on the openai family and, when the credential carries an effective base URL, routes through a synthesized model_providers entry via -c overrides (they survive --ignore-user-config; env_key pins auth to the scrubbed env). With a project credential the run also gets its own CODEX_HOME under tmp so an ambient auth.json can't outrank the project key — per run rather than per step because resume sessions live under CODEX_HOME. The worker now registers codex on a successful CLI probe alone: env auth at boot is no longer a precondition since a credential can arrive per-step.
…esolution Slots previously resolved every template role, forcing projects to grant models for roles a slot's steps never reference; slotRoleSets now scopes resolution to the step roles (plus their subagents' roles) each slot actually uses. On top of that, provider-constrained slots resolve single-provider: a step's base URL is process-wide and subagents resolve sibling roles inside the same query, so a mixed-provider slot could never execute. Candidates whose catalog auth policy demands a project credential the project lacks are excluded — when that is the only blocker the submit fails with the new provider_credential_required code instead of a misleading model_unresolvable. Ranking is deterministic: credentialed provider first, then lowest total input cost, then provider id. The '*' path (fake/custom executors) keeps legacy mixed resolution with no gating so token-free demos are unchanged. The core is a pure function (resolveSlotModels) so the ranking rules are unit-tested without a database; resolveModelRoles stays as the all-roles wrapper the compliance fixtures use.
The ResourceMaterializer grows a providerCredential(projectId, provider) member — the worker impl loads the project-scoped row and decrypts the key (mirroring MCP auth), while the engine memoizes one lookup per provider per run and registers the key with the redactor at materialization time, before any request could echo it into an event payload. buildRequest then attaches the credential matching the step model's provider; slot resolution is single-provider, so the same credential covers every subagent model in the request. No credential → the field is absent and worker env auth applies, which keeps the compliance contract for existing fixtures unchanged.
CRUD under /projects/:id/providers on the repo-connection precedent: the key is write-only (encrypted into secrets, kind provider_api_key, surfaced only as hasCredential), viewer-readable list, admin-only writes, audit rows on every mutation. Rotation updates the ciphertext in place so the secretRef stays stable for running steps; baseUrl is three-state (set/keep/clear) since null just falls back to the catalog default; DELETE removes the secret in the same transaction so no key material is orphaned — clearing a key IS delete, a keyless credential row would be meaningless. Integration tests cover masking, RBAC, rotation, deletion, audit, and the submit gate: a dashscope-only project 400s with provider_credential_required until a credential exists, then freezes a single-provider dashscope resolution.
A generic Providers section on the ReposSection pattern: provider select from PROVIDER_CATALOG, write-only password key input, optional base-URL override with the catalog default as the fallback hint. Rows surface only hasCredential-style state (key configured + endpoint); inline replace-key rotates without ever reading the key back, and removal confirms since runs then fall back to worker-env keys. Both locales ship together per the i18n rule.
Record the decision set — generic project-scoped credential rows over env config, project-wins precedence via whole-family env replacement, role-scoped single-provider slot resolution, codex -c overrides with a per-run CODEX_HOME, and submit gating limited to what the API can see (with the heartbeat upgrade path). Update the living design docs (domain model, executor contract, runtime resolution, API surface, frontend settings, deployment/Bailian setup), both manual locales, the ARCHITECTURE invariants, and the changelog.
There was a problem hiding this comment.
hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds per-project encrypted provider credentials with API and settings management, provider-aware single-provider slot resolution, credential injection into Claude and Codex executors, DashScope/Qwen support, endpoint validation, and related tests and documentation. ChangesProject provider credentials
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectAdmin
participant SettingsPage
participant ProjectRoutes
participant Database
participant Submission
participant Worker
participant Executor
ProjectAdmin->>SettingsPage: add or rotate provider credential
SettingsPage->>ProjectRoutes: POST or PATCH provider credential
ProjectRoutes->>Database: encrypt key and store provider row
Submission->>Database: resolve granted models and credential rows
Submission-->>Worker: enqueue resolved slot models
Worker->>Database: materialize provider credential
Worker->>Executor: execute step with providerAuth
Executor-->>Worker: run provider-scoped subprocess
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/api/src/routes/projects.ts (1)
404-446: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrap the PATCH rotate/baseUrl/audit writes in a transaction.
Unlike the sibling POST (Lines 370-397) and DELETE (Lines 461-476), this handler performs the secret rotation, the
baseUrlupdate, and the audit insert as three independent statements. A failure after thesecretsupdate leaves the key rotated but thebaseUrlchange and audit row missing — a partial, unaudited mutation. Reuse the samedb.transaction+tx-scopedaudit(...)pattern.♻️ Proposed transaction wrapping
if (!current) throw AppError.notFound("Provider credential"); const rotated = patch.apiKey !== undefined; - if (patch.apiKey !== undefined) { - // rotate in place: the ref stays stable, running steps see the new key - await db - .update(secrets) - .set({ ciphertext: encryptSecret(patch.apiKey, loadSecretKey()), rotatedAt: new Date() }) - .where(eq(secrets.id, current.secretRef)); - } - if (patch.baseUrl !== undefined) { - await db - .update(providerCredentials) - .set({ baseUrl: patch.baseUrl }) - .where(eq(providerCredentials.id, current.id)); - } - await audit(c, { - action: "project.provider.update", - resourceType: "provider_credential", - resourceId: current.id, - projectId, - payload: { provider, rotated, baseUrlChanged: patch.baseUrl !== undefined }, - }); + await db.transaction(async (tx) => { + if (patch.apiKey !== undefined) { + // rotate in place: the ref stays stable, running steps see the new key + await tx + .update(secrets) + .set({ ciphertext: encryptSecret(patch.apiKey, loadSecretKey()), rotatedAt: new Date() }) + .where(eq(secrets.id, current.secretRef)); + } + if (patch.baseUrl !== undefined) { + await tx + .update(providerCredentials) + .set({ baseUrl: patch.baseUrl }) + .where(eq(providerCredentials.id, current.id)); + } + await audit( + c, + { + action: "project.provider.update", + resourceType: "provider_credential", + resourceId: current.id, + projectId, + payload: { provider, rotated, baseUrlChanged: patch.baseUrl !== undefined }, + }, + tx, + ); + }); return c.json({ updated: true, hasCredential: true });🤖 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 `@apps/api/src/routes/projects.ts` around lines 404 - 446, Wrap the PATCH handler’s secret rotation, baseUrl update, and audit call in a single db.transaction callback, using the transaction client (tx) for every database operation and passing tx to audit. Preserve the existing update conditions, audit payload, and response while ensuring all mutations commit or roll back together.apps/web/src/pages/SettingsPage.tsx (1)
497-516: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRotate flow can't update/clear
baseUrl.The PATCH endpoint supports updating (or nulling, to reset to the catalog default)
baseUrlindependently ofapiKey, but the rotate UI/mutation only ever sends{ apiKey }. Once a credential is created (e.g. with a region-specific DashScope endpoint), the only way to change or clear itsbaseUrlis delete-and-recreate, losing the stored key too.Also applies to: 573-599
🤖 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 `@apps/web/src/pages/SettingsPage.tsx` around lines 497 - 516, Update the rotate flow around the rotate mutation and its associated UI state so it captures the provider’s baseUrl and includes it in the PATCH payload, allowing a string value to update the endpoint and null to restore the catalog default while preserving the existing apiKey rotation behavior. Ensure the rotate form exposes and submits the baseUrl independently of the key.packages/executor-codex/src/executor.ts (1)
174-186: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
CODEX_HOMEper-run tmp directories accumulate with no explicit cleanup.Each run with a project credential gets its own
agrippa-codex-home/<runId>directory under the OS tmpdir, created (but never removed) here. The comment calls this out as intentionally "left for OS tmp reaping," which is fine for hosts with periodic tmp cleanup, but worth confirming for containerized/long-lived worker deployments where/tmpmay not be reaped — otherwise this is unbounded disk growth proportional to run count over the worker's lifetime.🤖 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 `@packages/executor-codex/src/executor.ts` around lines 174 - 186, Update the CODEX_HOME lifecycle in the executor flow around Bun.spawn so the per-run directory created for req.runId is explicitly removed after the spawned process finishes, including failure paths. Preserve the isolated per-run home during execution and ensure cleanup does not mask the process result or primary error.
🤖 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 `@apps/web/src/pages/SettingsPage.tsx`:
- Around line 461-465: Filter PROVIDER_IDS to catalog entries whose auth policy
is "project", so the provider select only offers providers backed by project
credentials. Update the default selection and related provider-label/credential
form logic around PROVIDER_IDS to use this filtered list, preserving existing
behavior for project-auth providers such as dashscope.
In `@packages/db/src/seed/index.ts`:
- Around line 385-415: Update the DashScope model pricing used by recordUsage()
so it does not apply one fixed row rate to all tokens. For the Qwen entries
identified by providerModelId, derive or store the applicable region/endpoint
and input-length tier rates, including runs above 256k input tokens, or use a
conservative billed rate for enforcement; ensure token-cost budgets remain
accurate across DashScope pricing variants.
In `@packages/executor-codex/src/executor.ts`:
- Around line 67-85: The injected provider configuration in the effectiveBaseUrl
routing block should no longer set model_providers.agrippa.wire_api=chat. Remove
that argument while preserving the remaining synthesized provider settings and
authentication behavior.
In `@packages/orchestration/src/engine/engine.ts`:
- Around line 1390-1397: Update providerAuthFor in
packages/orchestration/src/engine/engine.ts (lines 1390-1397) to refresh the
provider credential for each step/request instead of retaining credentials or
null for the full run. In
packages/orchestration/src/engine/engine.integration.test.ts (lines 697-700),
replace the full-run memoization assertion with a test that rotates credentials
between steps and verifies the new value is used. Retain the “next step” wording
in docs/manual/en/04-administration.md (line 42) and
docs/manual/zh-CN/04-administration.md (line 42) because the implementation will
refresh between steps.
---
Nitpick comments:
In `@apps/api/src/routes/projects.ts`:
- Around line 404-446: Wrap the PATCH handler’s secret rotation, baseUrl update,
and audit call in a single db.transaction callback, using the transaction client
(tx) for every database operation and passing tx to audit. Preserve the existing
update conditions, audit payload, and response while ensuring all mutations
commit or roll back together.
In `@apps/web/src/pages/SettingsPage.tsx`:
- Around line 497-516: Update the rotate flow around the rotate mutation and its
associated UI state so it captures the provider’s baseUrl and includes it in the
PATCH payload, allowing a string value to update the endpoint and null to
restore the catalog default while preserving the existing apiKey rotation
behavior. Ensure the rotate form exposes and submits the baseUrl independently
of the key.
In `@packages/executor-codex/src/executor.ts`:
- Around line 174-186: Update the CODEX_HOME lifecycle in the executor flow
around Bun.spawn so the per-run directory created for req.runId is explicitly
removed after the spawned process finishes, including failure paths. Preserve
the isolated per-run home during execution and ensure cleanup does not mask the
process result or primary error.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b9c499e3-14d9-4ac7-af98-7bfad278f4ca
📒 Files selected for processing (47)
ARCHITECTURE.mdCHANGELOG.mdapps/api/src/routes/projects.tsapps/api/src/test/providers.integration.test.tsapps/web/src/lib/types.tsapps/web/src/pages/SettingsPage.tsxapps/worker/src/deps/resources.tsapps/worker/src/index.tsdocs/adr/0013-per-project-provider-credentials.mddocs/design/01-domain-model.mddocs/design/03-executor-abstraction.mddocs/design/04-execution-runtime.mddocs/design/05-api-and-auth.mddocs/design/06-frontend.mddocs/design/08-deployment.mddocs/manual/en/04-administration.mddocs/manual/en/06-operations.mddocs/manual/zh-CN/04-administration.mddocs/manual/zh-CN/06-operations.mdpackages/core/src/executors.tspackages/core/src/index.tspackages/core/src/providers.tspackages/core/src/schemas.tspackages/db/drizzle/0007_provider-credentials.sqlpackages/db/drizzle/meta/0007_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/projects.tspackages/db/src/schema/secrets.tspackages/db/src/seed/index.tspackages/executor-claude/src/executor.test.tspackages/executor-claude/src/executor.tspackages/executor-codex/src/executor.test.tspackages/executor-codex/src/executor.tspackages/executor-codex/test/fixtures/fake-codex.tspackages/executor-core/src/isolation.test.tspackages/executor-core/src/isolation.tspackages/executor-core/src/types.tspackages/i18n/locales/en/errors.jsonpackages/i18n/locales/en/settings.jsonpackages/i18n/locales/zh-CN/errors.jsonpackages/i18n/locales/zh-CN/settings.jsonpackages/orchestration/src/engine/deps.tspackages/orchestration/src/engine/engine.integration.test.tspackages/orchestration/src/engine/engine.tspackages/orchestration/src/engine/fakes.tspackages/orchestration/src/resolve.test.tspackages/orchestration/src/resolve.ts
| // Qwen via Aliyun Bailian (DashScope); runs need a per-project dashscope | ||
| // credential (project settings → providers). Prices are entry-tier list | ||
| // USD per MTok — Bailian tiers rates by input length, so verify against | ||
| // the live Model Studio price list when rolling out. | ||
| { | ||
| provider: "dashscope", | ||
| providerModelId: "qwen3.7-max", | ||
| displayName: "Qwen3.7 Max", | ||
| tier: "strong" as const, | ||
| contextWindow: 1_000_000, | ||
| inputCostPerMtok: "2.50", | ||
| outputCostPerMtok: "7.50", | ||
| }, | ||
| { | ||
| provider: "dashscope", | ||
| providerModelId: "qwen3.7-plus", | ||
| displayName: "Qwen3.7 Plus", | ||
| tier: "balanced" as const, | ||
| contextWindow: 1_000_000, | ||
| inputCostPerMtok: "0.32", | ||
| outputCostPerMtok: "1.28", | ||
| }, | ||
| { | ||
| provider: "dashscope", | ||
| providerModelId: "qwen3.6-flash", | ||
| displayName: "Qwen3.6 Flash", | ||
| tier: "fast" as const, | ||
| contextWindow: 1_000_000, | ||
| inputCostPerMtok: "0.25", | ||
| outputCostPerMtok: "1.50", | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg '(^|/)packages/db/src/seed/index\.ts$|recordUsage|cost|budget|model' || true
echo
echo "== seed index around DashScope rows =="
sed -n '360,430p' packages/db/src/seed/index.ts
echo
echo "== search recordUsage and cost-related symbols in packages/db =="
rg -n "recordUsage|inputCostPerMtok|outputCostPerMtok|contextWindow|budget|Budget|budget" packages/db/src -SRepository: ainaive/agrippa
Length of output: 4503
🌐 Web query:
DashScope pricing qwen3 plus input tokens >256k pricing endpoint region Aliyun Bailian model pricing
💡 Result:
As of July 2026, DashScope (Alibaba Cloud Model Studio) utilizes tiered pricing for many Qwen models, where the cost per million tokens depends on the total number of input tokens in a single request [1][2]. Pricing for "Qwen3-Plus" and similar models follows these general rules: 1. Tiered Billing: Costs are not calculated cumulatively; instead, the entire request is billed based on the price tier that the total input token count falls into [1][2]. 2. Region and Service: Pricing can vary by deployment region (e.g., Mainland China vs. International) [3][4]. Mainland China pricing is often lower and may include promotional offers, while International pricing is listed in USD [5][4]. 3. Current Model Pricing: - Specific pricing tiers for Qwen3-Plus models typically segment at thresholds like 128K and 256K tokens [3][2]. For example, earlier versions of Qwen-Plus series models have shown increased per-million-token input prices as input size exceeds 128K or 256K [3][1]. - Newer flagship models, such as the Qwen3.7 series (as of mid-2026), have introduced more streamlined or distinct pricing structures, often featuring promotional discounts or subscription-based "Token Plan" options to reduce costs for high-volume users [6][7][8]. To find the precise, current price for a specific model version and region: - Visit the official Alibaba Cloud Model Studio (Bailian) pricing page [3][9]. - Use the DashScope API or official console to view real-time billing information for your specific region [3][10]. - Note that new user credits and promotional discounts (often valid for 90 days) may apply to your account upon activation [3][2]. For high-frequency or long-term production use, consider the Token Plan subscription packages, which offer Credits that can be used across various Qwen models and may provide a lower effective cost than standard pay-as-you-go pricing [6][7][11].
Citations:
- 1: https://github.com/espo3468/alibaba-bailian-pricing
- 2: https://github.com/dhk069/alibaba-bailian-pricing
- 3: https://developer.aliyun.com/ask/704210
- 4: https://www.alibabacloud.com/help/en/model-studio/model-pricing
- 5: https://www.alibabacloud.com/blog/how-alibaba-cloud-calculates-and-manages-llm-tokens_602565
- 6: https://developer.aliyun.com/article/1747499
- 7: https://developer.aliyun.com/article/1736226
- 8: https://developer.aliyun.com/article/1741177
- 9: https://www.aliyun.com/product/bailian
- 10: https://www.alibabacloud.com/help/en/model-studio/qwen-api-via-dashscope
- 11: https://developer.aliyun.com/article/1744607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package usage/seed/schema files =="
find packages/db/src -type f \( -name 'usage.ts' -o -name 'registry.ts' -o -name 'index.ts' -o -name 'migrations/**/*.ts' \) -print | sort
echo
echo "== seed index imports/top and model insertion area =="
sed -n '1,80p' packages/db/src/seed/index.ts
sed -n '420,460p' packages/db/src/seed/index.ts
echo
echo "== registry schema area =="
sed -n '150,185p' packages/db/src/schema/registry.ts
echo
echo "== usage schema =="
sed -n '1,180p' packages/db/src/schema/usage.ts
echo
echo "== repository-wide references to inputCostPerMtok/outputCostPerMtok/models/recordUsage =="
rg -n "inputCostPerMtok|outputCostPerMtok|recordUsage|models|modelRows|providerModelId|provider" . -S --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: ainaive/agrippa
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/orchestration/src/engine/engine.ts model cost/usage regions =="
sed -n '220,235p' packages/orchestration/src/engine/engine.ts
sed -n '1540,1595p' packages/orchestration/src/engine/engine.ts
sed -n '1320,1370p' packages/orchestration/src/engine/engine.ts
echo
echo "== concise seed/migration model definitions =="
sed -n '168,184p' packages/db/src/schema/registry.ts
rg -n "ALTER TABLE \"public\".\"models\" DROP CONSTRAINT|ALTER TABLE \"public\".\"models\"" packages/db/drizzle packages/db/src/schema packages/db/src/migrations -S || true
sed -n '330,420p' packages/db/src/seed/index.tsRepository: ainaive/agrippa
Length of output: 8472
🌐 Web query:
Aliyun DashScope qwen3.7-plus actual model pricing modelstudio DashScope Chinese region API pricing
💡 Result:
As of July 23, 2026, Qwen3.7-Plus is a supported model on Alibaba Cloud's DashScope (Model Studio) platform [1][2]. It is a multimodal model (text, image, video) [3][2][4] with a 1-million-token context window [3][1][5]. Pricing for Qwen3.7-Plus in the Chinese region (e.g., North China 2/Beijing) is based on token usage with a tiered structure [6][3]. As of the latest available information for 2026, the discounted rates (post-payment/pay-as-you-go) are as follows [6]: Input/Output Pricing (per million tokens): - Input (≤256k tokens): 1.6 CNY [6] - Input (256k < tokens ≤ 1m): 4.8 CNY [6] - Output (≤256k tokens): 6.4 CNY [6] - Output (256k < tokens ≤ 1m): 19.2 CNY [6] Additional Pricing Metrics (per million tokens): - Input (Cache Hit): 0.32 CNY (≤256k) / 0.96 CNY (256k-1m) [6] - Explicit Cache Creation: 2.0 CNY (≤256k) / 6.0 CNY (256k-1m) [6] - Explicit Cache Hit: 0.16 CNY (≤256k) / 0.48 CNY (256k-1m) [6] Usage Notes: - The above rates reflect promotional discounts often applied to pay-as-you-go services [6][3]. - Alibaba Cloud also offers Resource Packages (deduction packages) for DashScope, which can provide further cost savings (up to 45% discount) for users with predictable usage [6]. - For production environments, it is recommended to use the workspace-dedicated domain (e.g., {WorkspaceId}.cn-beijing.maas.aliyuncs.com) rather than the older centralized DashScope domain (dashscope.aliyuncs.com) to ensure better performance and isolation [7][8][9]. - Prices and promotions are subject to change; always verify the current rates on the official Alibaba Cloud Model Studio pricing page [10][3].
Citations:
- 1: https://lushbinary.com/blog/qwen-3-7-developer-guide-max-plus-benchmarks-api/
- 2: https://www.qwencloud.com/models/qwen3.7-plus
- 3: https://developer.aliyun.com/article/1742239
- 4: https://openrouter.ai/qwen/qwen3.7-plus
- 5: https://docs.qwencloud.com/developer-guides/getting-started/text-generation-models
- 6: https://developer.aliyun.com/article/1740569
- 7: https://www.alibabacloud.com/help/en/model-studio/regions/
- 8: https://www.alibabacloud.com/help/en/model-studio/base-url
- 9: https://www.alibabacloud.com/help/zh/model-studio/qwen-api-via-dashscope
- 10: https://www.alibabacloud.com/help/en/model-studio/model-pricing
Do not apply a single DashScope row price to all usage.
recordUsage() applies one model row value to every token, but DashScope pricing is region/endpoint dependent and is input-length tiered for these Qwen models. A region-specific base_url or a run with >256k input tokens can be billed incorrectly, breaking token-cost budgets. Store/derive the applicable region rate, tier-aware pricing, or use a conservative billed rate before using these figures for enforcement.
🤖 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 `@packages/db/src/seed/index.ts` around lines 385 - 415, Update the DashScope
model pricing used by recordUsage() so it does not apply one fixed row rate to
all tokens. For the Qwen entries identified by providerModelId, derive or store
the applicable region/endpoint and input-length tier rates, including runs above
256k input tokens, or use a conservative billed rate for enforcement; ensure
token-cost budgets remain accurate across DashScope pricing variants.
There was a problem hiding this comment.
Acknowledged as a documented caveat, deferred. Bailian bills by input-length tier while the models registry stores flat per-MTok rates, so cost accounting/budget enforcement for Qwen is an approximation — flagged in the seed comment ("entry-tier list rates … verify against the live Model Studio price list"), ADR-0013's consequences, and docs/design/08-deployment.md. Modeling tiered pricing is a registry-schema feature (per-tier rate tables + tier-aware recordUsage) that's out of scope for this PR; admins can set conservative upper-tier rates in the registry today if they want budgets to fail safe.
There was a problem hiding this comment.
@hutusi, acknowledged. The documented limitation and conservative-rate mitigation make this acceptable to defer from this PR. Would you like me to open a follow-up issue for tier-/region-aware model pricing and budget accounting?
✏️ Learnings added
Learnt from: hutusi
Repo: ainaive/agrippa PR: 6
File: packages/db/src/seed/index.ts:385-415
Timestamp: 2026-07-24T01:48:22.138Z
Learning: For DashScope/Qwen models, `packages/db/src/seed/index.ts` stores flat per-MTok model rates while Bailian billing is input-length-tiered and endpoint/region-dependent. Cost accounting and budget enforcement are therefore approximate; this limitation is documented in the seed comments, ADR-0013, and `docs/design/08-deployment.md`. Administrators may configure conservative upper-tier rates in the model registry until a future registry-schema and tier-aware `recordUsage()` enhancement is implemented.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Thanks — no issue needed. This repo doesn't use the GitHub tracker (zero issues to date); deferred work is recorded in the ADRs and design docs instead, and this one already is: the seed comment, ADR-0013's consequences, and docs/design/08-deployment.md all flag the flat-rate approximation and tell operators to verify against the live Model Studio price list.
There was a problem hiding this comment.
@hutusi, understood—no GitHub issue will be created. The existing ADR and design-document references are the appropriate record for this deferred, tier-aware pricing work.
🐇📚
…olicy, per-step credentials, retry gate, atomic PATCH Five fixes from PR #6's review, one per finding: - Codex CLI ≥0.122 removed wire_api=chat, and Bailian's Responses API is per-model (not yet the seeded Qwen models), so dashscope-via-codex cannot run: codex-cli serves openai only again, the dashscope catalog keeps just its anthropic endpoint, and the synthesized model_providers block drops the wire_api override (the CLI's responses default is right for explicit openai proxies). effectiveBaseUrl additionally refuses to apply a row override to a protocol the provider is not known to serve, so one baseUrl can never leak across wire protocols. - A credential's baseUrl is where the worker sends the decrypted key, so a lax value was a key-exfiltration/SSRF channel (one admin could redirect a key another admin entered): validateProviderBaseUrl now enforces https, no userinfo/query/fragment, public DNS names only (IP literals, localhost, dotless hosts rejected; WHATWG parsing canonicalizes numeric IPv4 first), and per-provider host families (dashscope pins .aliyuncs.com). Enforced on POST and PATCH with the new base_url_invalid code. Resolve-time DNS checks are out of scope — internal proxies belong in worker env, which is operator-owned. - The engine memoized credentials per run, contradicting the documented rotation contract; providerAuthFor now materializes fresh per step, so rotate/remove/add genuinely apply at the next step. - Retry copied the frozen resolution without re-checking credentials: assertResolutionCredentialed re-asserts project-policy providers (both resolution shapes, demo/uncataloged slots skipped) before the new run row exists, failing with provider_credential_required instead of an auth error mid-run. - Credential PATCH now updates secret + row + audit in one transaction, matching create/delete.
There was a problem hiding this comment.
hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/orchestration/src/engine/engine.ts (1)
119-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the three near-identical modelResolution-flattening implementations.
slotResolutionEntries(module-level, lines 119-129) andRunEngine.resolutionFor/RunEngine.allResolutionEntries(287-315) each re-implement the same flat-vs-slot-keyed shape detection overrun.modelResolution. Since credential gating (executeRun's pre-claim loop) and step execution (buildRequest/providerAuthFor) both depend on this normalization staying consistent, extracting one shared helper would reduce the risk of the three implementations drifting apart on a future edge case.♻️ Sketch
-function slotResolutionEntries(raw: Record<string, unknown>, slot: string): ModelResolutionEntry[] { - ... -} +export function slotResolutionEntries(raw: Record<string, unknown>, slot: string): ModelResolutionEntry[] { + ... +}Then have
resolutionFor/allResolutionEntriesdelegate to it (adjusting return shape as needed) instead of re-deriving theflatcheck.Also applies to: 287-315
🤖 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 `@packages/orchestration/src/engine/engine.ts` around lines 119 - 129, Consolidate the flat-versus-slot-keyed model-resolution normalization into the existing slotResolutionEntries helper. Update RunEngine.resolutionFor and RunEngine.allResolutionEntries to delegate to that helper, and replace the executeRun pre-claim normalization with the same shared path, preserving each caller’s current return shape and slot-selection behavior.
🤖 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 `@apps/worker/src/deps/net.ts`:
- Around line 57-62: Update isPublicAddress to fail closed for IPv6 by requiring
the address to match an explicit globally reachable/unicast allowlist in
addition to excluding nonPublicV6 ranges; preserve the existing IPv4 behavior
and invalid-address rejection. Add regression coverage for 100:0:0:1::1 and
200::1, ensuring both are rejected.
---
Nitpick comments:
In `@packages/orchestration/src/engine/engine.ts`:
- Around line 119-129: Consolidate the flat-versus-slot-keyed model-resolution
normalization into the existing slotResolutionEntries helper. Update
RunEngine.resolutionFor and RunEngine.allResolutionEntries to delegate to that
helper, and replace the executeRun pre-claim normalization with the same shared
path, preserving each caller’s current return shape and slot-selection behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 91316584-513c-4dbc-b529-aed4ac2b2d6b
📒 Files selected for processing (33)
CHANGELOG.mdapps/api/src/routes/execution.tsapps/api/src/routes/projects.tsapps/api/src/test/providers.integration.test.tsapps/worker/src/deps/net.test.tsapps/worker/src/deps/net.tsapps/worker/src/deps/resources.tsdocs/adr/0013-per-project-provider-credentials.mddocs/design/03-executor-abstraction.mddocs/design/04-execution-runtime.mddocs/design/05-api-and-auth.mddocs/design/08-deployment.mddocs/manual/en/04-administration.mddocs/manual/zh-CN/04-administration.mdpackages/core/src/executors.tspackages/core/src/providers.test.tspackages/core/src/providers.tspackages/core/src/schemas.tspackages/executor-claude/src/executor.tspackages/executor-codex/README.mdpackages/executor-codex/src/executor.test.tspackages/executor-codex/src/executor.tspackages/executor-core/src/fake-executor.tspackages/executor-core/src/isolation.test.tspackages/executor-core/src/isolation.tspackages/executor-core/src/types.tspackages/i18n/locales/en/errors.jsonpackages/i18n/locales/zh-CN/errors.jsonpackages/orchestration/src/engine/deps.tspackages/orchestration/src/engine/engine.integration.test.tspackages/orchestration/src/engine/engine.tspackages/orchestration/src/engine/fakes.tspackages/orchestration/src/resolve.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- docs/manual/en/04-administration.md
- docs/design/05-api-and-auth.md
- packages/i18n/locales/zh-CN/errors.json
- packages/i18n/locales/en/errors.json
- apps/worker/src/deps/resources.ts
- packages/executor-core/src/isolation.test.ts
- packages/orchestration/src/engine/fakes.ts
- docs/design/08-deployment.md
- packages/executor-core/src/isolation.ts
- apps/api/src/routes/projects.ts
- packages/executor-codex/src/executor.ts
- docs/design/03-executor-abstraction.md
- packages/orchestration/src/resolve.ts
…etry gate, atomic PATCH Five fixes from PR #6's review, one per finding: - Codex CLI ≥0.122 removed wire_api=chat, and Bailian's Responses API is per-model (not yet the seeded Qwen models), so dashscope-via-codex cannot run: codex-cli serves openai only again, the dashscope catalog keeps just its anthropic endpoint, and the synthesized model_providers block drops the wire_api override (the CLI's responses default is right for explicit openai proxies). effectiveBaseUrl additionally refuses to apply a row override to a protocol the provider is not known to serve, so one baseUrl can never leak across wire protocols. - A credential's baseUrl is where the worker sends the decrypted key, so a lax value was a key-exfiltration/SSRF channel (one admin could redirect a key another admin entered): validateProviderBaseUrl now enforces https, no userinfo/query/fragment, public DNS names only (IP literals, localhost, dotless hosts rejected; WHATWG parsing canonicalizes numeric IPv4 first), and per-provider host families (dashscope pins .aliyuncs.com). Enforced on POST and PATCH with the new base_url_invalid code. Resolve-time DNS checks are out of scope — internal proxies belong in worker env, which is operator-owned. - The engine memoized credentials per run, contradicting the documented rotation contract; providerAuthFor now materializes fresh per step, so rotate/remove/add genuinely apply at the next step. - Retry copied the frozen resolution without re-checking credentials: assertResolutionCredentialed re-asserts project-policy providers (both resolution shapes, demo/uncataloged slots skipped) before the new run row exists, failing with provider_credential_required instead of an auth error mid-run. - Credential PATCH now updates secret + row + audit in one transaction, matching create/delete.
Append the review-round corrections to ADR-0013 (append-only): the codex path for dashscope is staged out (wire_api chat removed in Codex 0.122+, Bailian Responses coverage is per-model), base URLs are validated as key-egress targets, credentials materialize per step, and retry re-asserts the credential gate. Align the deployment and executor design docs and the changelog's Unreleased entry with the shipped behavior.
…, keyless-worker deferral Four findings, each with its fix and docs in this commit: - A baseUrl-only PATCH could still redirect a write-only key another admin entered (round 1 constrained where, not how): changing the endpoint now requires re-entering the key in the same request, so an existing key can never be pointed anywhere new; clearing back to the trusted catalog default stays key-less. Dashscope's host pin narrows from .aliyuncs.com — which admits customer-controlled OSS bucket domains that can log request headers — to the exact API hosts plus the .maas.aliyuncs.com workspace-gateway suffix. And because syntactic checks only catch IP literals, the worker now resolves the override host at credential materialization and refuses names landing in private/link-local space (typed ProviderCredentialError → run fails as base_url_invalid; rebind-after-check TOCTOU recorded as a residual in ADR-0013 amendment 2). - Per-step credential refresh made mid-run deletion fall back to worker env auth that cannot exist for a project-policy provider: on credential-gated executors the engine now fails the run with provider_credential_required before executor invocation — the same code the submit and retry gates use. Demo/fake and uncataloged executors stay exempt via the shared isCredentialGatedExecutor helper, since '*' resolution legitimately runs such models token-free. - Probe-only registration let a keyless codex worker claim env-fallback runs and fail them mid-run: executors now advertise envAuthProviders (captured from their env at construction) and the engine declines — before the queued→running transition, via the existing ExecutorUnavailableError/run.deferred heterogeneous-fleet path — any run whose env-policy providers neither a project credential nor this worker's env covers, so the run waits for a capable worker instead. - The codex README's Auth section still required worker env auth; rewritten for project credentials, probe-only registration, and the deferral behavior.
fd2f3ba to
81d8a64
Compare
There was a problem hiding this comment.
hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Require every resolved provider endpoint address to be global-unicast while preserving transient DNS failures for queue retry. Use a presence-only credential probe before claim so deterministic endpoint failures finalize actionably after claim. Add engine/network regressions and align the ADR, design docs, changelog, and both manual locales.
…pre-claim preflight Three findings, one commit: - The v6 SSRF check was a denylist, and no denylist can enumerate what IANA has not allocated — 4000::1/8000::1/e000::1 all passed as public. IPv6 validation is now allowlist-first: an address must be inside 2000::/3 (the only global-unicast allocation) and outside the special-use carve-outs within it (Teredo, benchmarking, ORCHID, documentation, 6to4). IPv4 keeps the special-use registry denylist — the whole v4 space is allocated, so that shape is correct there. - The per-step credential gate only failed project-policy providers. A keyless worker that claimed a run because the project credential existed would, after mid-run deletion of an env-policy credential, invoke claude/codex with no auth at all. providerAuthFor now also throws provider_credential_required when the bound executor advertises no env auth for the provider — the run fails before executor invocation; executors without an advertisement (fake/demo/ custom) keep prior behavior. - The auth preflight threw ExecutorUnavailableError regardless of run status while the worker only declines queued/waiting_approval, so a crash-recovered running run on a keyless worker burned pg-boss retries into a generic internal failure. The preflight is now scoped to exactly the pre-claim states the worker can re-enqueue; a running-run pickup proceeds and fails actionably per-step instead. Handing a running run to a different, authed worker stays gated on the ADR-0009 execution lease, matching the documented unregistered-executor limitation. Engine-integration and API regressions cover all three (the review could not run them without Postgres); ADR-0013 gains amendment 4.
…ubmission A reviewer read the credential-gating sentence as 'env-policy provider credentials are never used'; make the asymmetry explicit — the auth policy decides whether submit requires a credential, while a stored one still wins ranking, satisfies the keyless-worker preflight, and overrides worker env per step.
fb4c978 to
8b6a321
Compare
There was a problem hiding this comment.
hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/design/04-execution-runtime.md`:
- Line 16: Update the project provider credential behavior described in the
execution runtime documentation so worker-environment authentication is used
only for providers with an env-auth policy. Keep project-auth providers
credential-gated, including providers such as DashScope, and ensure missing
project credentials cannot bypass provider_credential_required.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f2b05ff-5d83-480c-be9e-68c26a619c80
📒 Files selected for processing (33)
CHANGELOG.mdapps/api/src/routes/execution.tsapps/api/src/routes/projects.tsapps/api/src/test/providers.integration.test.tsapps/worker/src/deps/net.test.tsapps/worker/src/deps/net.tsapps/worker/src/deps/resources.tsdocs/adr/0013-per-project-provider-credentials.mddocs/design/03-executor-abstraction.mddocs/design/04-execution-runtime.mddocs/design/05-api-and-auth.mddocs/design/08-deployment.mddocs/manual/en/04-administration.mddocs/manual/zh-CN/04-administration.mdpackages/core/src/executors.tspackages/core/src/providers.test.tspackages/core/src/providers.tspackages/core/src/schemas.tspackages/executor-claude/src/executor.tspackages/executor-codex/README.mdpackages/executor-codex/src/executor.test.tspackages/executor-codex/src/executor.tspackages/executor-core/src/fake-executor.tspackages/executor-core/src/isolation.test.tspackages/executor-core/src/isolation.tspackages/executor-core/src/types.tspackages/i18n/locales/en/errors.jsonpackages/i18n/locales/zh-CN/errors.jsonpackages/orchestration/src/engine/deps.tspackages/orchestration/src/engine/engine.integration.test.tspackages/orchestration/src/engine/engine.tspackages/orchestration/src/engine/fakes.tspackages/orchestration/src/resolve.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- packages/core/src/providers.test.ts
- packages/i18n/locales/zh-CN/errors.json
- docs/manual/zh-CN/04-administration.md
- packages/i18n/locales/en/errors.json
- packages/executor-core/src/fake-executor.ts
- docs/manual/en/04-administration.md
- apps/api/src/routes/execution.ts
- packages/executor-core/src/types.ts
- docs/design/05-api-and-auth.md
- packages/orchestration/src/engine/deps.ts
- apps/worker/src/deps/net.test.ts
- packages/executor-core/src/isolation.test.ts
- apps/worker/src/deps/net.ts
- packages/executor-codex/README.md
- packages/orchestration/src/engine/fakes.ts
- docs/design/08-deployment.md
- packages/executor-claude/src/executor.ts
- packages/executor-codex/src/executor.test.ts
- apps/worker/src/deps/resources.ts
- packages/executor-core/src/isolation.ts
- packages/core/src/providers.ts
- packages/executor-codex/src/executor.ts
- packages/core/src/schemas.ts
- packages/orchestration/src/engine/engine.integration.test.ts
- packages/orchestration/src/engine/engine.ts
- packages/orchestration/src/resolve.ts
A reviewer read the loose clause 'absence falls back to worker-env auth' as licensing that fallback for project-policy providers, which would bypass provider_credential_required. Spell out what the engine actually does: on a cataloged executor a missing credential fails the step whenever the policy is project or this worker has no env auth for the provider; fallback applies only otherwise.
There was a problem hiding this comment.
hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
What
Projects can now configure model-provider API keys in Settings → Providers instead of relying on worker env — first target: Aliyun Bailian (DashScope), so a project can run Qwen models while sibling projects keep the deployment's own keys. Decision record: ADR-0013 (plus four amendments from the review rounds).
How it works
provider_credentialsrow per (project, provider), key encrypted in the existingsecretstable (new kindprovider_api_key), write-only API (reads exposehasCredentialonly), rotate-in-place, delete removes the secret in the same transaction, audited on every mutation (create/update/delete all transactional). Migration0007.PROVIDER_CATALOGin@agrippa/coredeclares per-wire-protocol default endpoints and an auth policy per provider:dashscoperequires a project credential (no legitimate env fallback),anthropic/openaikeep worker env as the deployment default — though a stored credential for them is fully live too (wins ranking, overrides env per step). Qwen models seeded (qwen3.7-max/plus,qwen3.6-flash— entry-tier prices flagged for verification; Bailian tiers by input length). Dashscope runs through the claude executor only: Codex CLI ≥0.122 removed the chat wire API Bailian's compatible mode speaks (amendment 1).ANTHROPIC_AUTH_TOKEN+ gateway URL on claude; explicitopenaiproxy overrides land as a synthesized-c model_providersentry (responses wire API) with a per-runCODEX_HOMEon codex so ambientauth.jsoncan't outrank the project key.baseUrlis where the worker sends the decrypted key, so: https-only public DNS names, per-provider host pins (dashscope → exact API hosts +.maas.aliyuncs.com, excluding customer-controlled OSS bucket domains), endpoint+key atomicity (changing the endpoint requires re-entering the key, so an existing write-only key can never be redirected), and a worker-side DNS guard requiring every resolved address to be global-unicast (IPv6 allowlist-first from2000::/3, so unallocated space never passes; transient resolver errors stay queue-retryable).provider_credential_required: at submit, on retry (re-checked against the frozen resolution), and mid-run before executor invocation if the credential disappears. Demo/fakeresolution is unchanged.envAuthProviders; a run neither a project credential nor the claiming worker's env can authenticate is declined pre-claim (the existingrun.deferredheterogeneous-fleet path) and waits for a capable worker. The preflight is presence-only and applies only to the pre-claim states the worker can re-enqueue; a crash-recovered running run degrades to the actionable per-step failure instead.Review history
Three codex review rounds + CodeRabbit are addressed in-branch (
fix: address codex review …commits); ADR-0013 carries an append-only amendment per round. Known accepted residuals (documented in the ADR): DNS rebind-after-check TOCTOU, the audited rotate-into-planted-endpoint scenario, flat per-MTok rates vs Bailian's tiered billing, and cross-worker handoff of running runs pending the ADR-0009 execution lease.Verification
bun run check,bun test(244 pass / 0 fail with Postgres integration suites — engine compliance, resolution ranking, API CRUD/RBAC/rotation/audit, submit/retry/mid-run gates, keyless deferral, network guard),bun run templates:validate,bun run build. Not yet exercised against the live Bailian API (no key in dev); endpoints verified against Alibaba Cloud docs.Summary by CodeRabbit
New Features
Documentation