fix(skills): now we allow viking://agent/skills again, and optimize CLI for skills#2813
Merged
Conversation
Replace the monolithic `openviking/server/auth.py` with an extensible plugin-based auth system. This refactor extracts the three built-in modes (`dev`, `api_key`, `trusted`) into separate `AuthPlugin` implementations, adds a registry for third-party plugins, and preserves all existing behavior while enabling custom authentication backends (e.g. LDAP, OIDC, mTLS). Key changes: - **New public API**: `AuthPlugin` (ABC) and `register_auth_plugin` decorator. - **New registry**: `AuthPluginRegistry` supports runtime registration. - **Built-in plugins**: `DevAuthPlugin`, `ApiKeyAuthPlugin`, `TrustedAuthPlugin`. - **Config change**: `auth_mode` widened from `Literal` to `str` for custom modes. - **Validation delegated**: `validate_server_config()` now delegates to the active plugin's `validate_config()`, preserving existing validation semantics. - **Router compatibility**: All existing `require_*` decorators and `resolve_identity` / `get_request_context` dependencies remain unchanged. Routers import the same symbols from `openviking.server.auth`. - **Tests**: `conftest.py` manually wires the DevAuthPlugin in ASGI tests (lifespan not triggered). `test_auth.py` expanded with plugin registration and validation tests. - **Docs**: `04-authentication.md` (en/zh) updated with plugin registration examples. Co-Authored-By: claude-sonnet-4-6 <noreply@anthropic.com>
# Conflicts: # crates/ov_cli/src/client.rs
Align the `ov skills add` examples in the context-types and viking-uri docs with the short flag `-p` introduced for `ov skills list/find/show`, so all four user-facing examples consistently demonstrate the short form when targeting `viking://agent/skills`. Co-Authored-By: claude-sonnet-4-6 <noreply@anthropic.com>
Collaborator
|
Review note: Python SDK 的 scoped skill search 会把 target_uri 放在 JSON body 里，但服务端 POST /api/v1/skills/find 把 target_uri 定义成 query 参数，同时 FindSkillsRequest 是 extra="forbid"。所以类似： await client.find_skills("q", target_uri="viking://agent/skills")会发送 body 里的 target_uri，FastAPI 在进入 handler 前就按 extra field 拒掉，返回 422。Rust CLI 这里走的是 /api/v1/skills/find?target_uri=...，SDK 也需要把它放到 params 里，body 继续只保留 query/limit/score_threshold/level/telemetry。 |
Collaborator
Author
处理了 |
qin-ctx
approved these changes
Jun 25, 2026
qin-ctx
pushed a commit
that referenced
this pull request
Jun 30, 2026
) #2813 promoted account-shared skills (viking://agent/skills) to a first-class, scoped capability and shipped a target_uri param across the Python SDK (all 7 skill methods), the Rust CLI, and the server skills router. The Go SDK skill methods were the only client left without it, so Go adopters cannot scope skill operations to user-vs-agent roots and must drop to Python/CLI. Add an optional TargetURI field to the six skill option structs plus a new DeleteSkillOptions, and thread target_uri through every skill method matching the Python SDK + server placement exactly: - query param: ListSkills, GetSkill, DeleteSkill - JSON body: AddSkill, FindSkills, ValidateSkill, UpdateSkill target_uri is sent only when set (nil omits it), mirroring the Python "if target_uri is not None" semantics and the server default-root fallback. Like the Python SDK, skills send target_uri as-is (no client-side normalization; only find/search normalize). DeleteSkill previously took no options; it now takes a variadic opts ...*DeleteSkillOptions so existing DeleteSkill(ctx, name) callers keep compiling. The change is source-compatible across the module. Adds TestSkillRequestsScopeTargetURI (asserts query-vs-body placement for all 7 methods) and TestSkillRequestsOmitTargetURIWhenUnset (asserts omit for all 7). go test ./... and go vet ./... pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR re-enables the account-shared viking://agent/skills scope so it coexists with the per-user viking://user//skills scope, and ships a round of UX polish for the ov skills CLI.
Server / storage
still enforced by the sibling account_id == ctx.account_id predicate. (openviking/core/namespace.py, tests/unit/test_namespace_uri_classification.py)
CLI (ov_cli)
add/update/remove keep --parent-auto-create since their semantics include auto-creating the parent. (crates/ov_cli/src/main.rs)
Why
Agent-scope skills had been indexed correctly but were unreachable through ov find / ov skills find because visible_roots only whitelisted the per-user root. Users could ov ls viking://agent/skills and confirm records existed in the vector store, but every semantic
search returned 0 hits. This PR closes that gap end-to-end (storage → router → CLI rendering → flag names) so account-shared skills are first-class again.
Test plan