PEN-110: add user API key onboarding smoke#79
Conversation
Co-authored-by: multica-agent <github@multica.ai>
|
Closing as duplicate of #78, which already covers PEN-110 daemon model visibility, distributed onboarding docs, and the smoke script. |
There was a problem hiding this comment.
Code Review
This pull request introduces self-serve user onboarding functionality, including documentation, a smoke test script, and the integration of active, healthy daemon-backed models into the /v1/models endpoint. Feedback on the changes suggests improving the resilience of the /v1/models endpoint by skipping daemon nodes with corrupted database fields instead of failing the entire request, and securing the json_field helper in the smoke test script by passing arguments via sys.argv instead of direct shell interpolation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for (provider_family, model_ids_json, last_seen_at) in rows { | ||
| let created = parse_timestamp(&last_seen_at).map_err(from_app_error)?; | ||
| let model_ids = parse_daemon_model_ids(&model_ids_json).map_err(from_app_error)?; | ||
|
|
||
| for model_id in model_ids { |
There was a problem hiding this comment.
If a single daemon node has corrupted or invalid data in the database (e.g., a malformed model_ids_json or an invalid last_seen_at timestamp), the entire /v1/models endpoint will fail with a 500 Internal Server Error.
To make the API more resilient, we should log a warning and skip the invalid daemon node instead of failing the entire request.
for (provider_family, model_ids_json, last_seen_at) in rows {
let created = match parse_timestamp(&last_seen_at) {
Ok(t) => t,
Err(err) => {
warn!(error = %err, "Failed to parse last_seen_at for daemon node; skipping node");
continue;
}
};
let model_ids = match parse_daemon_model_ids(&model_ids_json) {
Ok(ids) => ids,
Err(err) => {
warn!(error = %err, "Failed to parse model_ids_json for daemon node; skipping node");
continue;
}
};
for model_id in model_ids {| json_field() { | ||
| python3 -c 'import json,sys; data=json.load(sys.stdin); print(data["'$1'"])' | ||
| } |
There was a problem hiding this comment.
In the json_field helper function, the shell variable $1 is interpolated directly into the Python script string. If the argument contains special characters, it could break the Python script or cause unexpected behavior.
Passing the argument safely via sys.argv is more robust and avoids shell quoting/escaping issues.
| json_field() { | |
| python3 -c 'import json,sys; data=json.load(sys.stdin); print(data["'$1'"])' | |
| } | |
| json_field() { | |
| python3 -c 'import json,sys; data=json.load(sys.stdin); print(data[sys.argv[1]])' "$1" | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ed677b116
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue; | ||
| } | ||
|
|
||
| data.push(PublicModelResponse { |
There was a problem hiding this comment.
Do not advertise daemon-only models as callable
When a healthy daemon advertises a model that has no matching model_routes row, this adds it to /v1/models, but /v1/chat/completions still resolves requests only through model_routes joined to provider_connections (crates/mizan-api/src/gateway.rs:1359-1372). In a daemon-only setup, users and the new smoke script can pick a listed model and then get model not found or disabled, so /v1/models is advertising uncallable models until the gateway has daemon dispatch support or these entries are backed by real routes.
Useful? React with 👍 / 👎.
Summary:
Verification:
Refs #67