fix(runtime): authorize() returns Result instead of panicking (#394)#499
Conversation
authorize() now returns RuntimeResult<NamespaceToken> instead of NamespaceToken, allowing callers to handle authorization failures gracefully via ? propagation rather than unwinding. Closes #394 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- F1: add .unwrap() to 66 test call sites across 6 crates (pack-brain, pack-memory, pack-kg/tests, pack-kg/handlers, apply_worker, projection_worker) that still expected NamespaceToken directly - F2: replace 4 production .unwrap() in kkernel coordinator (locate and fan_out_search) with match + tracing::warn!, treating denial as None/empty-results while preserving current return signatures - F3: log tracing::debug! for non-empty obligations on Allow decisions in authorize() to avoid silently discarding gate obligations - F4: run cargo fmt --all to fix formatting drift (comm, gtd, runtime) All workspace tests pass (2 pre-existing failures in pack-memory unrelated to this change). cargo clippy -D warnings and cargo fmt --check both clean. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge sequence
Merge #499 first — PRs #500, #502, #503 depend on it (GitHub auto-retargets on merge). |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RuntimeConfig::default() now includes additional_embedding_models with ParaphraseMultilingualMiniLmL12V2, which gets registered in the DB and causes note-creation tests to attempt model loading. In CI, the ONNX model files don't exist, causing 5 integration tests to fail with "model initialization failed: IO error: No such file or directory". Fix: explicitly set additional_embedding_models: vec![] in all test RuntimeConfig blocks that set embedding_model: None. Also includes cargo fmt and deno fmt fixes for runtime.rs and ADR docs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c418997038
ℹ️ 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".
| let req = GateRequest::new( | ||
| actor.clone(), | ||
| ns.clone(), | ||
| "authorize", |
There was a problem hiding this comment.
Preserve existing verb-based policies when authorizing
With a policy-backed gate that default-denies unknown verbs and whitelists the actual khive verbs (the Rego fixtures/docs model policies around input.verb == "search", "create", etc.), this new synthetic "authorize" check prevents callers from minting a token even though the operation they are about to perform is allowed. That breaks direct runtime users such as khive-vcs sync and coordinator fan-out paths that now call runtime.authorize(ns)? before doing storage work, and it can also skip EventStore wiring during MCP startup because the gate never sees the real operation there.
Useful? React with 👍 / 👎.
Summary
authorize()now returnsRuntimeResult<NamespaceToken>instead ofNamespaceToken?or.unwrap()in testsCloses #394
Test plan
cargo check --workspacepassescargo test --workspacepasses🤖 Generated with Claude Code