fix: defensive SQL chunking, user_id sanitization, and SQLite NULL scan for last_failure_at - #1668
fix: defensive SQL chunking, user_id sanitization, and SQLite NULL scan for last_failure_at#1668survivor998 wants to merge 14 commits into
Conversation
…lj#1602) * fix(gc): drop nil bind args from VACUUM exec call The previous call passed `(ctx, sql, nil, nil)` to the embedded *sql.DB.ExecContext (variadic args), so pgx received two nil bind parameters for a parameter-less statement and rejected it with "mismatched param and argument count". As a result auto-VACUUM silently failed every night on Postgres deployments. Tests cover both dialects; the Postgres path is env-gated on AXONHUB_TEST_PG_DSN since the project has no in-process PG harness. The SQLite driver swallowed the extra nils, so only the Postgres test reproduces the regression. * style(gc): fix gci import grouping in vacuum_test Move the entsql alias into its own alias section so the file matches the project's gci `custom-order: standard, default, blank, dot, alias, localmodule` configuration. * test(gc): drop unused SQLite client from disabled-path test runVacuum returns before touching w.Ent when VacuumEnabled is false, so allocating an in-memory SQLite client only added a schema migration round-trip without exercising any new path.
ChatGPT Codex CLI now lists `gpt-5.5 (current) - Frontier model for complex coding, research, and real-world work` as the headline model, and the upstream registry that this list mirrors (router-for-me/CLIProxyAPI internal/registry/models/models.json) already includes it. Without this entry AxonHub fails inbound validation with `model not found: gpt-5.5` (HTTP 422), so codex channels cannot expose the new model even when it is available on the user account. Co-authored-by: 覃康 <qinkang@robotees.tech>
Co-authored-by: zhzy0077 <zhzy0077@users.noreply.github.com>
…lj#1535) * fix(models): wire archive dialog so clicking Archive on a model works The dropdown menu called setOpen('archive') but ModelsDialogs never rendered a dialog for open === 'archive', so clicking Archive did nothing. - Add useUpdateModelStatus mutation hook calling updateModelStatus GraphQL - Add ModelsArchiveDialog with archive/restore confirmation - Wire ModelsArchiveDialog into ModelsDialogs for open === 'archive' - Add Restore menu item for archived models (previously unreachable) - Add i18n keys for archive/restore dialogs and success toasts (en, zh-CN) * fix(models): narrow useUpdateModelStatus type to enabled|archived The hook only handles archive/restore, not disable. Narrowing the status union prevents callers from accidentally passing 'disabled'.
CHANNEL_CONFIGS.qiniu references the Qiniu icon but the upstream PR that added Qiniu support imported it only in config_providers.ts and forgot the matching import here. The bundled module throws "ReferenceError: Qiniu is not defined" at evaluation, which crashes any page that imports config_channels (channels list, project requests, etc).
… length limit Prevents SQLite 'too many SQL variables' error by chunking all IDIn/ModelIDIn calls with a safe limit of 500 variables. Also removes MaxLen(512) from external_id fields and uses MySQL 'text' type instead, preventing truncation issues with long external IDs (e.g. Copilot Response API IDs >400 chars). Changes: - Add exported SQLiteMaxVariableLimit constant (500) in biz package - Chunk queryConfiguredModelFacades ModelIDIn calls - Chunk BulkArchiveModels/BulkDisableModels/BulkEnableModels/BulkDeleteModels - Chunk BulkCreateModels existence check (2 vars per predicate) - Replace openai.go hardcoded 500 with biz.SQLiteMaxVariableLimit - Change external_id from MaxLen(512) to SchemaType(text) in request schema - Change external_id from MaxLen(512) to SchemaType(text) in request_execution schema Note: Ent code regeneration required (go generate ./internal/server/gql)
Regenerate ent ORM code and GraphQL schema after external_id schema changes (MaxLen removal). Update frontend route tree generation.
Some upstream providers (e.g. Zhipu AI) reject the user field if it exceeds 128 characters. Claude Code v2 sends user_id in JSON format which easily exceeds this limit, causing 400 errors. Applied to all OpenAI-format outbound paths: chat completions, embedding, image generation, and Responses API.
…ounds check The user_id field sent to upstream providers must be between 6 and 128 characters. Claude Code generates structured user_id (v2 JSON format) that easily exceeds this limit, causing 400 errors from providers like Zhipu. Changes: - Add SanitizeUserStr call in Anthropic outbound_convert.go for metadata.user_id - Fix SanitizeUserStr bounds check: SHA256 hex output is 64 chars, not 128+ - Update test to accept both v2 JSON format and sanitized h_ hash format Fixes both OpenAI and Anthropic outbound paths for Claude Code usage.
The zhipu channel type uses the zai outbound transformer, which sets user_id directly from metadata without length sanitization. This caused 400 errors from Zhipu API: user_id must be between 6 and 128 characters. Apply SanitizeUserStr to both zai and doubao outbound transformers to ensure user_id never exceeds the 128-char limit for any provider.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces model archiving and restoration functionality, including new UI components, hooks, and translations. It addresses database reliability by chunking bulk operations and queries to avoid SQLite variable limits and implements user ID sanitization for several LLM providers. Additionally, it adds image support for Ollama and updates project dependencies. Review feedback points out a potential security regression in user mutation policies, the need for chunking in bulk model creation, and technical improvements for Ollama's image handling and Go slice declarations.
| Mutation: scopes.MutationPolicy{ | ||
| scopes.OwnerRule(), | ||
| scopes.UserWriteScopeRule(scopes.ScopeWriteUsers), | ||
| scopes.UserOwnedMutationRule(), |
There was a problem hiding this comment.
The removal of scopes.UserOwnedMutationRule() is undocumented and appears to be a regression. This rule typically allows users to modify their own records. Without it, a regular user might lose the ability to update their own profile unless they are granted the broad ScopeWriteUsers permission. Conversely, if they do have that permission, they can now modify any user's record, which is a security risk.
| } | ||
| // Check if any models already exist (chunk inputs to avoid SQLite variable limit; | ||
| // each predicate uses 2 variables: developer + modelID). | ||
| chunkSize := SQLiteMaxVariableLimit / 2 |
There was a problem hiding this comment.
While chunking has been added for the duplicate check, the actual model creation via CreateBulk is still performed in a single call. For large inputs, this will hit the SQLite SQLITE_MAX_VARIABLE_NUMBER limit. This bulk creation call should also be chunked to ensure reliability with large datasets.
| return nil | ||
| } | ||
|
|
||
| images := make([]string, 0) |
There was a problem hiding this comment.
When declaring an empty slice that is intended to be encoded to JSON, prefer a nil slice value instead of a non-nil empty slice. This ensures the field is omitted via omitempty rather than appearing as [].
| images := make([]string, 0) | |
| var images []string |
References
- Prefer a nil slice value instead of a non-nil slice value for empty collections. (link)
| if image != "" { | ||
| images = append(images, image) | ||
| } |
There was a problem hiding this comment.
Ollama's API expects the images field to contain base64-encoded data. Passing a remote URL directly will likely result in a failure as the API does not fetch remote resources. Consider validating that the string is a base64 payload or implementing a helper to download and encode the image if a URL is provided.
Greptile SummaryThis PR applies SHA256-based
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming LLM Request\nwith user_id] --> B{len > 128?}
B -- No --> C[Pass through unchanged]
B -- Yes --> D[SHA256 hash user_id]
D --> E[Return h_ + hex string\n66 chars total]
C --> F[Outbound Transformer]
E --> F
F --> G{Transformer Type}
G --> H[OpenAI Chat\nuser field]
G --> I[OpenAI Embedding\nuser field]
G --> J[OpenAI Image\nuser field]
G --> K[OpenAI Responses\nuser field]
G --> L[Anthropic\nmetadata.user_id]
G --> M[ZAI / Zhipu\nuser_id]
G --> N[Doubao\nuser_id]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func (svc *ModelService) BulkArchiveModels(ctx context.Context, ids []int) error { | ||
| _, err := svc.entFromContext(ctx).Model.Update(). | ||
| Where(model.IDIn(ids...)). | ||
| SetStatus(model.StatusArchived). | ||
| Save(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to bulk archive models: %w", err) | ||
| for _, chunk := range lo.Chunk(ids, SQLiteMaxVariableLimit) { | ||
| _, err := svc.entFromContext(ctx).Model.Update(). | ||
| Where(model.IDIn(chunk...)). | ||
| SetStatus(model.StatusArchived). | ||
| Save(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to bulk archive models: %w", err) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Each chunk of 500 IDs runs as an independent SQL transaction. If processing fails on chunk N (e.g., a DB error mid-flight), all earlier chunks are already committed, leaving models in a partially-archived/disabled/deleted state with no way to roll back. The RunInTransaction helper in AbstractService exists precisely for this; wrapping the entire chunk loop in it would keep the original all-or-nothing guarantee. The same applies to BulkDisableModels, BulkEnableModels, and BulkDeleteModels.
| func getImages(content llm.MessageContent) []string { | ||
| if len(content.MultipleContent) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| images := make([]string, 0) | ||
|
|
||
| for _, part := range content.MultipleContent { | ||
| if part.Type != "image_url" || part.ImageURL == nil { | ||
| continue | ||
| } | ||
|
|
||
| image := strings.TrimSpace(part.ImageURL.URL) | ||
| if image == "" { | ||
| continue | ||
| } | ||
|
|
||
| if comma := strings.Index(image, ","); strings.HasPrefix(image, "data:") && comma >= 0 { | ||
| image = image[comma+1:] | ||
| } | ||
|
|
||
| if image != "" { | ||
| images = append(images, image) | ||
| } | ||
| } | ||
|
|
||
| return images |
There was a problem hiding this comment.
The Ollama REST API's images field requires base64-encoded image bytes; plain URLs (e.g. https://example.com/image.png) are not accepted by the REST API. The function correctly strips the data:…;base64, prefix for data URIs, but for plain URL strings it falls through and appends the URL verbatim. Ollama will silently ignore or error on that entry, making URL-based images non-functional. The test TestTransformRequestPreservesPlainImageURLParts asserts this broken behavior as correct.
| h := sha256.Sum256([]byte(user)) | ||
| hexStr := hex.EncodeToString(h[:]) | ||
| if len(hexStr) > maxUserLen-2 { | ||
| hexStr = hexStr[:maxUserLen-2] | ||
| } | ||
| return fmt.Sprintf("h_%s", hexStr) |
There was a problem hiding this comment.
if len(hexStr) > maxUserLen-2 is dead code. sha256.Sum256 always produces 32 bytes, so hex.EncodeToString always returns exactly 64 characters — well below the 126-character threshold. The slice can be removed; the PR description's own note ("66 chars total") confirms the final length is always fixed.
| h := sha256.Sum256([]byte(user)) | |
| hexStr := hex.EncodeToString(h[:]) | |
| if len(hexStr) > maxUserLen-2 { | |
| hexStr = hexStr[:maxUserLen-2] | |
| } | |
| return fmt.Sprintf("h_%s", hexStr) | |
| h := sha256.Sum256([]byte(user)) | |
| return fmt.Sprintf("h_%s", hex.EncodeToString(h[:])) |
survivor998
changed the title
7 participants
Summary
A collection of fixes for SQLite compatibility and defensive coding:
INclauses into chunks of 500 to prevent SQLiteSQLITE_MAX_VARIABLE_NUMBERlimit errorsexternal_idfield no longer has a max length constraint, preventing truncation issuesMAX(CASE WHEN ... END)returns NULL when no failed requests exist. SQLite driver materializes this as empty string which cannot scan intoime.Time. Changed to*time.Timeto allow NULL mapping.user_id sanitization paths fixed
openai/outbound_convert.gouseropenai/embedding.gouseropenai/image_outbound.gouseropenai/responses/outbound.gouser�nthropic/outbound_convert.gometadata.user_idzai/outbound.gouser_iddoubao/outbound.gouser_idlast_failure_at fix details
Before: Startup warning:
After:
queryResult.LastFailureAtchanged fromime.Timeto*time.Time, allowing NULL to map to nil.Testing
go test ./transformer/...)go vetclean across all packageslast_failure_atwarning