Repository navigation
feat(agent): add multi model provider support - #2962
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds provider-aware agent model selection and request-scoped BYOK credentials. It updates model catalogues, runtime execution, workflow transport, API contracts, guards, and the dashboard session picker. ChangesProvider-aware model catalogue
Encrypted credential transport
Runtime and API integration
Session UI
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant AgentSessionUI
participant AgentRoute
participant agentSessionGuard
participant AgentRuntimeService
participant AgentSessionWorkflow
participant AgentTurnStep
participant WritingAgentRuntime
AgentSessionUI->>AgentRoute: list models request
AgentRoute->>agentSessionGuard: resolve session & caller
agentSessionGuard->>AgentRuntimeService: pass caller context
AgentRuntimeService->>AgentRuntimeService: read & decrypt credentials
AgentRuntimeService-->>AgentSessionUI: models with API-key status
AgentSessionUI->>AgentRoute: update provider/model selection
AgentRoute->>agentSessionGuard: validate model for kind
agentSessionGuard->>AgentRuntimeService: store settings
AgentRuntimeService->>AgentSessionWorkflow: initialize or update session
AgentSessionWorkflow->>AgentTurnStep: pass encrypted credentials
AgentTurnStep->>AgentTurnStep: decrypt credentials
AgentTurnStep->>WritingAgentRuntime: run turn with per-turn models
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/service/src/services/agent-runtime.service.ts`:
- Around line 806-828: Update assertKnownModel to validate the exact
providerId/modelId pair against listWritingModels({ configured }), requiring a
matching entry whose requiresApiKey is false instead of relying on
isWritingModel. Preserve the existing empty-selection behavior, reject
incomplete or unknown selections, and map both validation failures to
opts.errors.BAD_REQUEST so agent routes do not rethrow them as unhandled errors.
In `@apps/service/src/steps/agent-turn.step.ts`:
- Around line 176-183: Update the model initialization in the writing-agent turn
flow around decryptAgentCredentials to import and catch AgentCredentialError,
converting it into new FatalError(error.message); rethrow every other error
unchanged, and leave createAgentModels and writingAgentRuntime.runTurn behavior
intact.
In `@apps/service/src/workflows/agent-session.workflow.ts`:
- Around line 79-88: Update the credential handling in the workflow loop around
pending messages and decisions so an empty credential set is represented as {}
rather than retaining stale ciphertext. Replace the fallback assignments to
credentials with direct assignments from next.credentials and
decision.credentials, while preserving the existing credential type and flow.
In `@packages/agent-writing/src/models.ts`:
- Around line 35-46: Update the persistence flows that call createSession and
updateSettings to validate catalogue membership with
createAgentCatalog().getModel(ref.providerId, ref.modelId) before saving the
provider/model pair. Keep assertKnownModel and isWritingModel checks, but reject
native-provider model IDs missing from the catalogue rather than treating every
modelId as valid.
🪄 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: 0431ec09-33b5-486f-aa92-e40bdce2187d
📒 Files selected for processing (20)
apps/dash/src/components/agent/agent-model-picker.tsxapps/dash/src/components/agent/agent-session.tsxapps/service/__tests__/agent-credentials.test.tsapps/service/src/services/agent-credentials.tsapps/service/src/services/agent-runtime.service.tsapps/service/src/steps/agent-turn.step.tsapps/service/src/workflows/agent-session.workflow.tsapps/service/src/workflows/hooks/agent.hooks.tspackages/agent-core/__tests__/models.test.tspackages/agent-core/src/index.tspackages/agent-core/src/models.tspackages/agent-runtime/src/adapters/pi.tspackages/agent-writing/__tests__/models.test.tspackages/agent-writing/__tests__/runtime.test.tspackages/agent-writing/src/index.tspackages/agent-writing/src/models.tspackages/agent-writing/src/runtime.tspackages/api/orpc/agent-runtime.tspackages/api/orpc/contracts/agent.contract.tspackages/api/orpc/routes/agent.route.ts
| /** | ||
| * Validates a client-supplied model selection. | ||
| * | ||
| * The pair is checked as a unit, and a `modelId` without a `providerId` is refused rather than | ||
| * defaulted: the same model exists under several providers with different ids and different payers, | ||
| * so there is no safe guess. `isWritingModel` is the same predicate the turn resolves through, so a | ||
| * selection accepted here cannot fail policy later. | ||
| */ | ||
| const assertKnownModel = (input: { | ||
| providerId?: string; | ||
| modelId?: string; | ||
| }): void => { | ||
| if (!input.modelId && !input.providerId) return; | ||
| if (!input.modelId || !input.providerId) { | ||
| throw new Error( | ||
| "Both providerId and modelId are required to select a model." | ||
| ); | ||
| } | ||
| const ref = { providerId: input.providerId, modelId: input.modelId }; | ||
| if (!isWritingModel(ref)) { | ||
| throw new UnknownAgentModelError(ref); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check error mapping for runtime throws and available catalogue lookup helpers.
set -euo pipefail
ast-grep outline packages/agent-writing/src/models.ts --items all
ast-grep outline packages/agent-core/src/models.ts --items all
rg -n -C 4 'UnknownAgentModelError|BAD_REQUEST' --glob '*.ts' packages/api apps/service/srcRepository: Chia1104/chia1104.dev
Length of output: 16600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- writing model implementation ---'
sed -n '1,90p' packages/agent-writing/src/models.ts
printf '%s\n' '--- core model implementation ---'
sed -n '100,235p' packages/agent-core/src/models.ts
printf '%s\n' '--- runtime imports, contracts, and handlers ---'
sed -n '1,45p' apps/service/src/services/agent-runtime.service.ts
rg -n -C 12 'createAgentSessionContract|updateAgentSessionSettingsContract|assertKnownModel|createAgentSession|updateAgentSessionSettings|UnknownAgentModelError' packages/api apps/service/src --glob '*.ts'Repository: Chia1104/chia1104.dev
Length of output: 30677
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- agent route error handling ---'
sed -n '1,135p' packages/api/orpc/routes/agent.route.ts
printf '%s\n' '--- model listing route and contracts ---'
rg -n -C 18 'listAgentModels|listWritingModels|configured|BYOK_PROVIDER_IDS|credentials' packages/api apps/service/src --glob '*.ts'
printf '%s\n' '--- runtime credential flow and turn resolution ---'
rg -n -C 14 'createAgentModels|resolveWritingModel|resolveModel|credentials|apiKey|openai|anthropic' apps/service/src/services/agent-runtime.service.ts --glob '*.ts'Repository: Chia1104/chia1104.dev
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- capability routes ---'
rg -n -C 25 'export const listAgentModelsRoute|export const listAgentCapabilitiesRoute' packages/api/orpc/routes/agent.route.ts
printf '%s\n' '--- runtime model call sites ---'
rg -n -C 10 'createAgentModels|resolveWritingModel|listWritingModels|BYOK_PROVIDER_IDS|AgentRuntimeCaller' apps/service/src/services/agent-runtime.service.ts packages/api/orpc/agent-runtime.ts
printf '%s\n' '--- focused error translation search ---'
rg -n -C 8 'opts\.errors\.(BAD_REQUEST|INTERNAL_SERVER_ERROR)|catch \(error\)' packages/api/orpc/routes/agent.route.ts apps/service/src/services/agent-runtime.service.tsRepository: Chia1104/chia1104.dev
Length of output: 29329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- turn setup and model injection ---'
sed -n '470,700p' apps/service/src/services/agent-runtime.service.ts
printf '%s\n' '--- all model resolution references ---'
rg -n -C 8 'modelsFor\(|createWriting|resolveWritingModel|settings\.providerId|settings\.modelId' apps/service packages/agent-* --glob '*.ts'Repository: Chia1104/chia1104.dev
Length of output: 44146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- oRPC error conversion ---'
rg -n -C 12 'onError|errorHandler|INTERNAL_SERVER_ERROR|UnknownAgentModelError|instanceof AppError|toErrorResponse|RPCHandler' packages/api apps/service --glob '*.ts'
printf '%s\n' '--- model-list tests for configured semantics ---'
sed -n '105,180p' packages/agent-writing/__tests__/models.test.ts
sed -n '1,115p' packages/agent-core/src/models.ts
printf '%s\n' '--- credential helpers ---'
rg -n -C 12 'readEncryptedAgentCredentials|decryptAgentCredentials|BYOK_PROVIDER_IDS' apps/service/src --glob '*.ts'Repository: Chia1104/chia1104.dev
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- configured model tests ---'
sed -n '105,180p' packages/agent-writing/__tests__/models.test.ts
printf '%s\n' '--- agent route source after sessions ---'
sed -n '130,300p' packages/api/orpc/routes/agent.route.ts
printf '%s\n' '--- RPC mounting and error middleware ---'
rg -n -C 10 'RPCHandler|RPCHandler|onError|hooks|error' apps/service/src/routes apps/service/src --glob '*.ts' | head -240Repository: Chia1104/chia1104.dev
Length of output: 22854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- oRPC error factory ---'
sed -n '1,240p' apps/service/src/factories/orpc.factory.ts
sed -n '1,180p' apps/service/src/factories/orpc-error.factory.ts
printf '%s\n' '--- provider credential helper definitions ---'
rg -l 'readEncryptedAgentCredentials|decryptAgentCredentials' apps/service/src | xargs -r -n1 sh -c 'echo "--- $0"; rg -n -C 10 "readEncryptedAgentCredentials|decryptAgentCredentials" "$0"'Repository: Chia1104/chia1104.dev
Length of output: 11949
Validate the model against the caller’s available catalogue. isWritingModel accepts any native openai or anthropic ID, so unknown IDs can be persisted and fail during turn resolution. Use listWritingModels({ configured }) and require an exact pair with requiresApiKey === false. Map both validation errors to opts.errors.BAD_REQUEST; the agent routes currently rethrow them as unhandled errors.
🤖 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/service/src/services/agent-runtime.service.ts` around lines 806 - 828,
Update assertKnownModel to validate the exact providerId/modelId pair against
listWritingModels({ configured }), requiring a matching entry whose
requiresApiKey is false instead of relying on isWritingModel. Preserve the
existing empty-selection behavior, reject incomplete or unknown selections, and
map both validation failures to opts.errors.BAD_REQUEST so agent routes do not
rethrow them as unhandled errors.
| const models = createAgentModels( | ||
| decryptAgentCredentials(request.credentials) | ||
| ); | ||
|
|
||
| return await writingAgentRuntime.runTurn({ | ||
| createOptions: { | ||
| session, | ||
| models, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect FatalError usage and any retry policy configured for steps.
set -euo pipefail
rg -n -C 3 'FatalError' --glob '*.ts' apps/service/src | head -60
fd -t f 'package.json' -d 3 apps/service --exec sh -c 'jq -r ".dependencies.workflow // empty" "$0"' {}Repository: Chia1104/chia1104.dev
Length of output: 4494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- agent-turn imports and target block ---'
sed -n '1,35p;125,195p' apps/service/src/steps/agent-turn.step.ts
printf '%s\n' '--- credential error definitions and usages ---'
rg -n -C 5 'AgentCredentialError|decryptAgentCredentials|AI_AUTH_PUBLIC_KEY' --glob '*.ts' .
printf '%s\n' '--- workflow dependency declarations ---'
fd -t f 'package.json' -d 4 . --exec sh -c '
value=$(jq -r ".dependencies.workflow // .devDependencies.workflow // empty" "$0")
if [ -n "$value" ]; then printf "%s: %s\n" "$0" "$value"; fi
'
printf '%s\n' '--- FatalError declarations and construction sites ---'
rg -n -C 4 'class FatalError|FatalError\s*=|new FatalError|instanceof FatalError' --glob '*.{ts,tsx,js,mjs,cjs}' .Repository: Chia1104/chia1104.dev
Length of output: 24642
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package metadata and lockfiles ---'
sed -n '1,120p' apps/service/package.json
git ls-files '*lock*' '*package.json' | sed -n '1,120p'
printf '%s\n' '--- workflow references in tracked metadata and docs ---'
rg -n -C 3 'workflow|FatalError|retry' --glob '!**/node_modules/**' --glob '*.{json,md,yml,yaml,ts,tsx}' . | head -240
printf '%s\n' '--- exported Models type and credential imports ---'
rg -n -C 4 'export (type )?Models|type Models|interface Models|AgentCredentialError' --glob '*.ts' packages appsRepository: Chia1104/chia1104.dev
Length of output: 25858
🌐 Web query:
workflow npm 4.6.2 FatalError retries constructor cause documentation
💡 Result:
In the context of the workflow SDK (such as workflow-sdk.dev), a FatalError is used to mark a step as permanently failed, preventing any automatic retries [1][2][3]. Regarding your specific query about version 4.6.2 and the FatalError constructor: 1. FatalError Purpose: When thrown within a step (marked with "use step"), it signals that the workflow should not retry that step, causing the workflow run to fail or trigger compensation logic [1][2][3]. 2. Constructor and Cause: Documentation for the FatalError class in the workflow SDK typically shows a constructor that accepts a message string (e.g., constructor(message: string)) [4]. Unlike some other error classes in the ecosystem—such as WorkflowRunFailedError or general WorkflowError which explicitly support a cause property to wrap underlying errors [4][5]—the standard FatalError implementation in this SDK is primarily designed to stop execution. 3. Note on "cause": If you are looking to preserve the underlying error when throwing a FatalError, standard JavaScript Error patterns (which the SDK follows) allow you to set the cause via the options object in the Error constructor, though you should verify your specific version's type definitions if you need to access it programmatically [6][4]. Some specialized error classes in similar workflows explicitly include a cause property [4][5]. If you are using a specific library or wrapper, please note that different workflow implementations (such as Cloudflare Workflows) may use different error names like NonRetryableError and have their own specific quirks regarding error message handling and retries [7][8]. Always verify your project's specific dependency version, as implementations can vary between SDKs.
Citations:
- 1: https://app.unpkg.com/workflow@4.8.0/files/docs/api-reference/workflow/fatal-error.mdx
- 2: https://workflow-sdk.dev/docs/api-reference/workflow/fatal-error
- 3: https://workflow-sdk.dev/cookbook/common-patterns/saga
- 4: https://cdn.jsdelivr.net/npm/@workflow/errors@4.1.0-beta.14/dist/index.d.ts
- 5: https://vercel-workflow.mintlify.app/errors/runtime-errors
- 6: RFC: fix: FatalError set reference to causing error yagop/node-telegram-bot-api#1257
- 7: Cloudflare Workflow retries on NonRetryableError in dev mode cloudflare/workers-sdk#10113
- 8: Failed Workflow instance error always displays generic error when throwing NonRetryableError cloudflare/workers-sdk#12636
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir=$(mktemp -d)
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL https://registry.npmjs.org/workflow/4.6.2 > "$tmpdir/workflow.json"
jq -r '.dist.tarball' "$tmpdir/workflow.json"
tarball=$(jq -r '.dist.tarball' "$tmpdir/workflow.json")
curl -fsSL "$tarball" -o "$tmpdir/workflow.tgz"
tar -xzf "$tmpdir/workflow.tgz" -C "$tmpdir"
printf '%s\n' '--- FatalError declarations ---'
rg -n -C 8 'FatalError' "$tmpdir/package" --glob '*.{d.ts,js,mjs,cjs}' | head -160
printf '%s\n' '--- package exports and type entrypoints ---'
jq '{name,version,types,exports}' "$tmpdir/package/package.json"
printf '%s\n' '--- lockfile workflow resolution ---'
rg -n -C 3 'workflow@4\.6\.2|workflow:' pnpm-lock.yaml | head -80Repository: Chia1104/chia1104.dev
Length of output: 243
🌐 Web query:
site:unpkg.com/workflow@4.6.2 FatalError constructor
💡 Result:
There is no native class named FatalError in the standard PHP library. In PHP, fatal errors (such as E_ERROR, E_PARSE, etc.) generally cannot be caught by standard try/catch blocks because they typically terminate script execution immediately [1][2][3]. If you are encountering a class named FatalError, it is likely part of a third-party library or framework, such as: 1. Symfony: Symfony provides a \Symfony\Component\ErrorHandler\Error\FatalError class, which is used internally to wrap fatal PHP errors for error handling purposes [4]. It is not a standard PHP exception or error class you would typically instantiate manually in application code [4]. 2. MediaWiki: MediaWiki defines its own \FatalError class, which inherits from MWException and is used to abort web requests with specific HTML error messages [5][6]. For standard PHP error handling, developers typically use: - Throwable/Error/Exception: These are the base classes for catchable errors and exceptions [7][8][9]. - ErrorException: This class can be used in custom error handlers (set via set_error_handler) to convert non-fatal PHP errors into catchable exceptions [10][11]. Because E_ERROR types cannot be caught by user-defined error handlers, common workarounds for logging or handling them include using register_shutdown_function in combination with error_get_last to detect that a fatal error has occurred during script shutdown [1][2][3].
Citations:
- 1: https://www.php.net/function.set-error-handler
- 2: https://www.php.net/set-error-handler
- 3: https://stackoverflow.com/questions/277224/how-do-i-catch-a-php-fatal-e-error-error
- 4: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/ErrorHandler/Error/FatalError.php
- 5: https://doc.wikimedia.org/mediawiki-core/REL1_42/php/classFatalError.html
- 6: https://doc.wikimedia.org/mediawiki-core/REL1_35/php/classFatalError.html
- 7: https://www.php.net/error.construct.php
- 8: https://www.php.net/exceptions
- 9: https://www.php.net/function.set-exception-handler
- 10: https://www.php.net/manual/en/errorexception.construct.php
- 11: https://www.php.net/errorexception
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- public workflow package metadata ---'
curl -fsSL https://unpkg.com/workflow@4.6.2/package.json | jq '{name,version,types,exports,dependencies}' || true
printf '%s\n' '--- public workflow declarations ---'
for path in \
dist/index.d.ts \
dist/errors.d.ts \
dist/index.js \
dist/errors.js \
dist/workflow.d.ts
do
printf '%s\n' "--- $path ---"
curl -fsSL "https://unpkg.com/workflow@4.6.2/$path" 2>/dev/null \
| rg -n -C 8 'FatalError' | head -80 || true
done
printf '%s\n' '--- lockfile entries ---'
rg -n -C 5 '(^|\s)workflow(@|:)|workflow-4\.6\.2' pnpm-lock.yaml | head -120 || true
printf '%s\n' '--- local type usage relevant to proposed patch ---'
rg -n -C 3 'Models|AgentCredentialError' apps/service/src packages --glob '*.ts' | head -160Repository: Chia1104/chia1104.dev
Length of output: 15763
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- workflow errors package metadata ---'
curl -fsSL https://unpkg.com/@workflow/errors@4.1.5/package.json \
| jq '{name,version,types,exports}' || true
printf '%s\n' '--- workflow errors declarations ---'
for path in dist/index.d.ts dist/errors.d.ts dist/index.js dist/errors.js; do
printf '%s\n' "--- $path ---"
curl -fsSL "https://unpkg.com/@workflow/errors@4.1.5/$path" 2>/dev/null \
| rg -n -C 10 'FatalError|class .*Error' | head -120 || true
done
printf '%s\n' '--- workflow package references to FatalError ---'
for path in dist/index.d.ts dist/internal/errors.js dist/workflow.js dist/index.js; do
printf '%s\n' "--- $path ---"
curl -fsSL "https://unpkg.com/workflow@4.6.2/$path" 2>/dev/null \
| rg -n -C 8 'FatalError' | head -100 || true
doneRepository: Chia1104/chia1104.dev
Length of output: 10014
Classify AgentCredentialError as fatal.
Import AgentCredentialError, catch it around decryptAgentCredentials, and throw new FatalError(error.message). Rethrow all other errors. Undecryptable credentials cannot succeed on retry without re-registration, so ordinary retry handling only delays the operator message.
🤖 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/service/src/steps/agent-turn.step.ts` around lines 176 - 183, Update the
model initialization in the writing-agent turn flow around
decryptAgentCredentials to import and catch AgentCredentialError, converting it
into new FatalError(error.message); rethrow every other error unchanged, and
leave createAgentModels and writingAgentRuntime.runTurn behavior intact.
| let credentials: EncryptedAgentCredentials | undefined = | ||
| firstMessage.credentials; | ||
|
|
||
| while (turns < MAX_TURNS_PER_RUN) { | ||
| if (pending === null) { | ||
| // Durable pause: no compute is consumed while waiting for the operator. | ||
| const next = await messages; | ||
| if (next.text === AGENT_END_SENTINEL) break; | ||
| pending = next; | ||
| credentials = next.credentials ?? credentials; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every producer of hook credential payloads and every reader of the retained value.
set -euo pipefail
rg -n -C 4 'readEncryptedAgentCredentials|agentMessageHook\.resume|agentApprovalHook\.resume|credentials' \
--glob '*.ts' apps/service/src packagesRepository: Chia1104/chia1104.dev
Length of output: 39726
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- credential reader ---'
sed -n '30,55p' apps/service/src/services/agent-credentials.ts
printf '%s\n' '--- hook schemas ---'
sed -n '1,75p' apps/service/src/workflows/hooks/agent.hooks.ts
printf '%s\n' '--- approval producer ---'
sed -n '650,695p' apps/service/src/services/agent-runtime.service.ts
printf '%s\n' '--- all hook resumes ---'
rg -n -C 3 'agent(Message|Approval)Hook\.resume' apps/service/src packages --glob '*.ts'
printf '%s\n' '--- credential payload shape verifier ---'
python3 - <<'PY'
from pathlib import Path
text = Path("apps/service/src/services/agent-credentials.ts").read_text()
start = text.index("export const readEncryptedAgentCredentials")
end = text.index("\n};", start) + 3
print(text[start:end])
PYRepository: Chia1104/chia1104.dev
Length of output: 8709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- credential reader ---'
sed -n '30,55p' apps/service/src/services/agent-credentials.ts
printf '%s\n' '--- hook schemas ---'
sed -n '1,75p' apps/service/src/workflows/hooks/agent.hooks.ts
printf '%s\n' '--- approval producer ---'
sed -n '650,695p' apps/service/src/services/agent-runtime.service.ts
printf '%s\n' '--- all hook resumes ---'
rg -n -C 3 'agent(Message|Approval)Hook\.resume' apps/service/src packages --glob '*.ts'Repository: Chia1104/chia1104.dev
Length of output: 8148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
credentials = Path("apps/service/src/services/agent-credentials.ts").read_text()
workflow = Path("apps/service/src/workflows/agent-session.workflow.ts").read_text()
runtime = Path("apps/service/src/services/agent-runtime.service.ts").read_text()
assert "return Object.keys(credentials).length > 0 ? credentials : undefined;" in credentials
assert "credentials = next.credentials ?? credentials;" in workflow
assert "credentials = decision.credentials ?? credentials;" in workflow
# Model the exact JavaScript nullish-coalescing behavior used by both assignments.
old = {"openai": "previous-ciphertext"}
assert (None if False else None) is None
assert (None if None is not None else old) is old # undefined/null-like payload retains old value
empty = {}
assert (empty if empty is not None else old) is empty # explicit empty object clears old value
prompt_start = runtime.index("async prompt(caller, input)")
prompt_end = runtime.index("\n },", prompt_start)
approve_start = runtime.index("async approve(caller, input)")
approve_end = runtime.index("\n },", approve_start)
prompt = runtime[prompt_start:prompt_end]
approve = runtime[approve_start:approve_end]
assert prompt.count("readEncryptedAgentCredentials(caller.context.headers)") == 1
assert approve.count("readEncryptedAgentCredentials(caller.context.headers)") == 1
assert "text: AGENT_END_SENTINEL" in runtime
print("PASS: empty credentials would clear retained ciphertext, but the current reader returns undefined and both workflow assignments retain it.")
print("PASS: prompt and approve are the only credential-bearing hook producers in the inspected runtime paths.")
PY
printf '%s\n' '--- exact workflow assignments ---'
rg -n 'credentials = (next|decision)\.credentials' apps/service/src/workflows/agent-session.workflow.tsRepository: Chia1104/chia1104.dev
Length of output: 554
Stale Credentials After Revocation (CWE-613): Insufficient Session Expiration
Reachability: External · Exploitability: Moderate
Reachability path
● Entry
packages/agent-writing/src/models.ts:68
listModels
│
▼
● Hop
apps/service/src/services/agent-runtime.service.ts:465
prompt: * Refuse while an approval is outstanding. * * The run is parked on the *approval* hook, not the message hook. A resumed message would be…
│
▼
● Sink
apps/service/src/workflows/agent-session.workflow.ts
Clear credentials when all BYOK cookies are absent.
readEncryptedAgentCredentials returns undefined when no keys remain. The ?? assignments at lines 88 and 121 retain the previous ciphertext. Return {} for an empty credential set and assign next.credentials and decision.credentials directly.
🤖 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/service/src/workflows/agent-session.workflow.ts` around lines 79 - 88,
Update the credential handling in the workflow loop around pending messages and
decisions so an empty credential set is represented as {} rather than retaining
stale ciphertext. Replace the fallback assignments to credentials with direct
assignments from next.credentials and decision.credentials, while preserving the
existing credential type and flow.
| export const isWritingModel = (ref: AgentModelRef): boolean => { | ||
| switch (ref.providerId) { | ||
| case AGENT_PROVIDERS.gateway: | ||
| return GATEWAY_VENDOR_PREFIXES.some((prefix) => | ||
| ref.modelId.startsWith(prefix) | ||
| ); | ||
| case AGENT_PROVIDERS.openai: | ||
| case AGENT_PROVIDERS.anthropic: | ||
| return true; | ||
| default: | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the service symbols involved in settings validation.
ast-grep outline apps/service/src/services/agent-runtime.service.ts --items all --type function
# Inspect whether settings mutation paths validate catalogue membership, not only policy.
rg -n -C 12 --type ts \
'\bassertKnownModel\s*\(|\bisWritingModel\s*\(|\bcreateAgentCatalog\s*\(|\bresolveWritingModel\s*\(' \
apps/service/src/services/agent-runtime.service.ts
# Inspect callers that receive or persist provider/model settings.
rg -n -C 12 --type ts \
'\bproviderId\b|\bmodelId\b|\bsettings\b|\bwriteSessionSettings\b' \
apps/service/src/services/agent-runtime.service.tsRepository: Chia1104/chia1104.dev
Length of output: 11781
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- model catalog symbols ---'
rg -n -C 10 --type ts \
'\b(createAgentCatalog|createAgentModels|getModel|resolveWritingModel|isWritingModel|listWritingModels)\b' .
printf '%s\n' '--- settings write and validation symbols ---'
rg -n -C 12 --type ts \
'\b(writeSessionSettings|updateSettings|createSession|assertKnownModel)\b' .
printf '%s\n' '--- candidate file map ---'
fd -t f -e ts . | rg 'agent|model|catalog|session' | head -80Repository: Chia1104/chia1104.dev
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- agent-core model files ---'
fd -t f . packages/agent-core | sort | rg 'model|provider|index'
printf '%s\n' '--- core model implementation symbols ---'
rg -n -C 8 --type ts \
'\b(createAgentModels|resolveModel|listModels|UnknownAgentModelError|AgentModelRef)\b' \
packages/agent-core
printf '%s\n' '--- exact service mutation and runtime paths ---'
sed -n '360,390p' apps/service/src/services/agent-runtime.service.ts
sed -n '436,456p' apps/service/src/services/agent-runtime.service.ts
sed -n '806,829p' apps/service/src/services/agent-runtime.service.tsRepository: Chia1104/chia1104.dev
Length of output: 27186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact catalog and resolver implementation ---'
sed -n '107,177p' packages/agent-core/src/models.ts
printf '%s\n' '--- exact writing policy implementation ---'
sed -n '30,68p' packages/agent-writing/src/models.ts
printf '%s\n' '--- focused static invariant check ---'
python3 - <<'PY'
from pathlib import Path
service = Path("apps/service/src/services/agent-runtime.service.ts").read_text()
writing = Path("packages/agent-writing/src/models.ts").read_text()
core = Path("packages/agent-core/src/models.ts").read_text()
assert "if (!isWritingModel(ref))" in service
assert "createAgentCatalog" not in service
assert "models.getModel(ref.providerId, ref.modelId)" in core
assert "case AGENT_PROVIDERS.openai:" in writing
assert "case AGENT_PROVIDERS.anthropic:" in writing
assert "return true;" in writing
print("settings validation uses policy only: true")
print("catalogue lookup exists in resolveModel: true")
print("native-provider policy admits arbitrary modelId text: true")
print("catalogue membership is not checked before settings persistence: true")
PYRepository: Chia1104/chia1104.dev
Length of output: 4614
Validate catalogue membership before persisting model settings.
assertKnownModel checks only isWritingModel. Native providers accept any modelId, including IDs absent from the catalogue. Check createAgentCatalog().getModel(ref.providerId, ref.modelId) before createSession and updateSettings persist the pair.
🤖 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/agent-writing/src/models.ts` around lines 35 - 46, Update the
persistence flows that call createSession and updateSettings to validate
catalogue membership with createAgentCatalog().getModel(ref.providerId,
ref.modelId) before saving the provider/model pair. Keep assertKnownModel and
isWritingModel checks, but reject native-provider model IDs missing from the
catalogue rather than treating every modelId as valid.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/api/orpc/agent-runtime.ts`:
- Around line 167-175: Update validateModel and its callers to make model
validation credential-aware before persistence: pass AgentRuntimeCaller into
validateModel (or perform the equivalent check) and ensure both createSession
and updateSettings reject BYOK models when the caller has no registered provider
key, before either write occurs.
In `@packages/api/orpc/contracts/agent.contract.ts`:
- Line 153: Update the object schemas in createAgentSessionContract and
updateAgentSessionSettingsContract to use z.strictObject({ instead of
z.object({, so removed fields such as modelId or providerId are rejected with
validation errors rather than stripped.
🪄 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: 947115f2-0317-44b7-8d8d-97b9d2a0eb8d
📒 Files selected for processing (10)
apps/dash/src/components/agent/agent-model-picker.tsxapps/service/src/services/agent-runtime.service.tsapps/service/src/workflows/agent-session.workflow.tspackages/agent-writing/__tests__/models.test.tspackages/agent-writing/src/index.tspackages/agent-writing/src/models.tspackages/api/orpc/agent-runtime.tspackages/api/orpc/contracts/agent.contract.tspackages/api/orpc/guards/agent-session.guard.tspackages/api/orpc/routes/agent.route.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dash/src/components/agent/agent-model-picker.tsx
- apps/service/src/workflows/agent-session.workflow.ts
- packages/agent-writing/src/index.ts
- apps/service/src/services/agent-runtime.service.ts
| /** | ||
| * Checks a model selection before anything persists it. | ||
| * | ||
| * Returns a human-readable reason when the pair is unusable, or `null` when it is fine. A return | ||
| * value rather than a thrown error, because the caller is a middleware that has to turn this into | ||
| * a `BAD_REQUEST` — and because which models a kind admits is the kind's own policy, so the | ||
| * *reason* has to come from here rather than be reconstructed at the transport. | ||
| */ | ||
| validateModel(ref: AgentModelRef): Promise<string | null>; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline apps/service/src/services/agent-runtime.service.ts --items all
# Expect both write paths to reject a selected model when the caller has no
# usable credential, or to call a credential-aware validation helper.
rg -n -C 12 \
-e '\b(validateModel|listModels|createSession|updateSettings)\s*\(' \
-e '\brequiresApiKey\b' \
-e '\b(credentials?|provider.*key|api.*key)\b' \
apps/service/src/services/agent-runtime.service.tsRepository: Chia1104/chia1104.dev
Length of output: 8179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- service write paths ---'
sed -n '330,455p' apps/service/src/services/agent-runtime.service.ts
printf '%s\n' '--- validation and middleware call sites ---'
rg -n -C 10 \
-e '\bvalidateModel\b' \
-e '\bassertWritingModel\b' \
-e '\bcreateSession\b' \
-e '\bupdateSettings\b' \
-e '\bwriteSessionSettings\b' \
apps packages
printf '%s\n' '--- model catalogue policy ---'
rg -n -C 12 \
-e '\blistWritingModels\b' \
-e '\bBYOK_PROVIDER_IDS\b' \
-e '\brequiresApiKey\b' \
packages appsRepository: Chia1104/chia1104.dev
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
service = Path("apps/service/src/services/agent-runtime.service.ts").read_text()
guard = Path("packages/api/orpc/guards/agent-session.guard.ts").read_text()
runtime = Path("packages/api/orpc/agent-runtime.ts").read_text()
models = Path("packages/agent-core/src/models.ts").read_text()
def body(name, start, end):
match = re.search(rf"{re.escape(start)}(.*?){re.escape(end)}", service, re.S)
assert match, f"missing {name}"
return match.group(1)
create = body("createSession", "async createSession(caller, input)", " getSession(caller, input)")
update = body("updateSettings", "async updateSettings(caller, input)", " /**\n * Enqueues a turn.")
assert "repo.create({" in create
assert "validateModel(" not in create
assert "validateModel(" not in update
assert re.search(r"runtime\.validateModel\(\s*input\.model\s*\)", guard)
assert re.search(r"getAgentRuntime\(input\.kind\)\.validateModel\(\s*input\.model\s*\)", guard)
assert re.search(r"validateModel\(ref:\s*AgentModelRef\)", runtime)
assert "assertWritingModel(ref)" in service
assert "isByokProviderId(provider.id) && !configured.has(provider.id)" in models
print("createSession persists input.model without a service-level validateModel call")
print("updateSettings persists input.model without a service-level validateModel call")
print("both transport guards call validateModel with only input.model")
print("assertWritingModel validates the model reference, not caller credential configuration")
print("requiresApiKey is true for BYOK providers absent from configured caller keys")
PYRepository: Chia1104/chia1104.dev
Length of output: 542
Make model validation credential-aware before persistence. createSession and updateSettings can persist a BYOK model when the caller has no registered provider key. Pass AgentRuntimeCaller to validateModel, or add an equivalent check before both writes.
🤖 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/api/orpc/agent-runtime.ts` around lines 167 - 175, Update
validateModel and its callers to make model validation credential-aware before
persistence: pass AgentRuntimeCaller into validateModel (or perform the
equivalent check) and ensure both createSession and updateSettings reject BYOK
models when the caller has no registered provider key, before either write
occurs.
| /** Seeds the draft buffer from this post so the agent edits rather than starts fresh. */ | ||
| targetFeedId: z.number().int().optional(), | ||
| modelId: z.string().optional(), | ||
| model: agentModelRefSchema.optional(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline packages/api/orpc/contracts/agent.contract.ts --items all
# Inspect remaining callers that serialize legacy standalone model fields.
rg -nP -C 6 -g '*.ts' -g '*.tsx' \
'\b(?:modelId|providerId)\s*:' \
apps/dash packages/apiRepository: Chia1104/chia1104.dev
Length of output: 10902
🌐 Web query:
For Zod 4, does z.object({...})strip unknown keys by default, and doesz.strictObject({...}) reject them? Use official Zod documentation.
💡 Result:
In Zod 4, the default behaviors for object schemas are as follows: - z.object({...}): By default, this schema strips unknown keys from the input object [1][2]. This means that any properties present in the input but not defined in the schema are removed from the parsed output [1]. - z.strictObject({...}): This schema is designed to reject unknown keys [3]. If the input contains keys not defined in the schema, the parser will throw an error [3]. These behaviors are consistent with the established functionality of Zod to ensure type safety and schema enforcement [1][4]. The standard z.object remains the default, non-strict implementation that automatically sanitizes inputs by stripping unspecified fields [1][2].
Citations:
- 1: zod4 z.object type strips index keys colinhacks/zod#4671
- 2: https://github.com/colinhacks/zod/blob/v4.4.3/packages/docs/content/json-schema.mdx
- 3: https://zod.dev/api
- 4:
strictObjectchanges behaviour in v4 colinhacks/zod#4823
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the contract file to inspect the actual definitions
sed -n '144,160p' packages/api/orpc/contracts/agent.contract.ts
echo "---"
sed -n '183,210p' packages/api/orpc/contracts/agent.contract.tsRepository: Chia1104/chia1104.dev
Length of output: 2028
Use z.strictObject() to reject unknown fields on these mutation inputs.
With Zod 4 default object parsing, z.object() strips unknown input keys. A request that sends the removed modelId or providerId fields parses with model === undefined. Both handlers then receive an omitted model selection and cannot distinguish a stale client from an intentional omission. This can create or update a session without applying the caller's selected model.
Replace z.object({ with z.strictObject({ in createAgentSessionContract (line 144) and updateAgentSessionSettingsContract (line 183) to reject unknown fields and trigger a schema validation error instead of silently normalizing the request.
Proposed fix
export const createAgentSessionContract = oc
.errors({ UNAUTHORIZED: {}, FORBIDDEN: {}, BAD_REQUEST: {} })
.input(
- z.object({
+ z.strictObject({
kind: z.string().min(1),
title: z.string().max(200).optional(),
targetFeedId: z.number().int().optional(),
model: agentModelRefSchema.optional(),
thinkingLevel: thinkingLevelSchema.optional(),
autoApprove: z.array(toolTierSchema).optional(),
runtimeConfig: z.record(z.string(), z.unknown()).optional(),
})
)
export const updateAgentSessionSettingsContract = oc
.errors({ UNAUTHORIZED: {}, FORBIDDEN: {}, NOT_FOUND: {}, BAD_REQUEST: {} })
.input(
- z.object({
+ z.strictObject({
kind: z.string().optional(),
sessionId: z.string(),
title: z.string().max(200).optional(),
model: agentModelRefSchema.optional(),
thinkingLevel: thinkingLevelSchema.optional(),
activeToolNames: z.array(z.string()).nullable().optional(),
autoApprove: z.array(toolTierSchema).optional(),
runtimeConfig: z.record(z.string(), z.unknown()).optional(),
})
)🤖 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/api/orpc/contracts/agent.contract.ts` at line 153, Update the object
schemas in createAgentSessionContract and updateAgentSessionSettingsContract to
use z.strictObject({ instead of z.object({, so removed fields such as modelId or
providerId are rejected with validation errors rather than stripped.
Summary by CodeRabbit