Repository navigation
Add Keenable search and fetch provider - #10
ilya-bogin-keenable wants to merge 2 commits into
Conversation
Tiny Sweeper reviewThis update revises the Keenable provider pull request (previously at head 83fb760, state 'changes requested'). The diff now includes an expanded comment in `configured_provider_tools` explaining that Keenable's keyless access is gated by the host's explicit enabled Direct entry (a default configuration lists nothing), and the tool-visibility test file is present, covering the hidden-by-default, backend-route, and disabled cases. The tests and description lanes state the previously raised authorization concern is resolved and the change is ready to merge. However, the critique and security lanes still actively raise authorization findings on this head: 'Require authorization before enabling direct Keenable access' (rules `missing-authorization`/`authorization-bypass`/`authorization-gate`) on `crates/tinysearch-bus/src/search/catalog/mod.rs` and `crates/tinysearch/src/provider/direct/keenable/keenable_tests.rs`, plus a critique finding 'Add the missing external catalog tests' (`missing-external-test`) on the catalog module. These unresolved findings should be reconciled before merge. State: Changes requested Review snapshot
Completeness: Complete What changedThe same Keenable provider implementation as the prior revision: `crates/tinysearch/src/provider/direct/keenable.rs` implements search (posting the query with optional `site`, `published_after`, `published_before` filters, max_results clamped to 1-20 with default 5, 1,200-character snippets) and fetch (up to 10 URLs, a 404 retry with `live=true`, `failed_count` accounting, and the first error's classification when every page fails), switching between keyless `/public` endpoints and keyed endpoints via `X-API-Key`, with `X-Keenable-Title: tinysearch` on every request. The catalog change in `crates/tinysearch-bus/src/search/catalog/mod.rs` now carries an explanatory comment that Keenable, like SearXNG, has no credential to gate on — the host's own enabled entry is the opt-in, calls go only to Keenable or the host's base URL, and a credential only raises limits — before the `ProviderRoute::Direct if name == "keenable" => true` arm. Role wiring, default provider orders, documentation, and dispatch changes are unchanged from the prior revision. Features
Tests
Findings
Resolved this pass
Before merge
How this fits togetherflowchart LR
n0["configured_provider_tools<br/>changed<br/>3 findings"]:::blocking
n1["direct_provider_specs<br/>changed<br/>3 findings"]:::blocking
n2["catalog_and_selection_are_stable<br/>changed"]:::changed
n3["provider_tool_specs"]:::impacted
n4["default"]:::impacted
n5["role_config"]:::impacted
n6["parallel_is_direct_only"]:::impacted
n7["backend_failure_envelope_is_redacted"]:::impacted
n8["keyed"]:::impacted
n2 -->|calls| n3
n2 -->|tests| n3
n2 -->|calls| n4
n2 -->|tests| n4
n3 -->|calls| n1
n5 -->|calls| n4
n5 -->|calls| n8
n6 -->|calls| n0
n6 -->|tests| n0
n6 -->|calls| n3
n6 -->|tests| n3
n6 -->|calls| n4
n6 -->|tests| n4
n6 -->|calls| n8
n6 -->|tests| n8
n7 -->|calls| n4
n7 -->|tests| n4
n8 -->|calls| n4
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0069 · 301,522 in / 14,881 out · 27,179 cached (9%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0027 · 150,449 in / 8,515 out · 14,389 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0013 · 110,637 in / 3,736 out · 12,790 cached (12%) · gpt-5.6-luna
tests: $0.0009 · 13,298 in / 574 out · 0 cached (0%) · glm-5.3-flash
description: $0.0009 · 13,723 in / 318 out · 0 cached (0%) · glm-5.3-flash
| super::super::http::read_json(response).await | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
Add the referenced external test file
Rust will try to include keenable/keenable_tests.rs when compiling tests, but that file is not present in the complete change shown. As a result, cargo test and test-target builds fail with a missing-module-file error. Add the referenced sibling test file to the pull request, or remove the module declaration if tests are intentionally omitted.
[RULE] missing-module-file ·
| ProviderRoute::Direct if name == "searxng" => { | ||
| non_empty(explicit.base_url.as_deref()) | ||
| } | ||
| // Keenable has keyless public endpoints; a credential only raises limits. |
There was a problem hiding this comment.
Require authorization before enabling direct Keenable access
This makes Keenable usable for every direct-route configuration regardless of whether credentials or another explicit authorization signal is present. Because the provider performs network-backed search and fetch operations, callers who can invoke the bus can use the provider without the credential gate applied to other direct providers. Do not unconditionally return true; require the provider's supported authorization/configuration state before advertising or dispatching it.
[RULE] authorization-bypass ·
There was a problem hiding this comment.
Resolved — the review agent found this finding fixed in the new code, as of 0d0cab7.
If this is wrong, reopen the conversation and say so; the finding will be re-raised on the next push if it still reproduces.
Clippy 1.99 flags assert!(x.is_empty()). The catalog comment now says what gates keyless Keenable: the host's own enabled provider entry.
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0099 · 194,338 in / 11,705 out · 9,439 cached (5%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0045 · 85,168 in / 5,885 out · 4,070 cached (5%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0030 · 52,152 in / 2,949 out · 5,369 cached (10%) · gpt-5.6-luna
tests: $0.0002 · 13,695 in / 271 out · 0 cached (0%) · glm-5.3-flash
description: $0.0002 · 14,120 in / 492 out · 0 cached (0%) · glm-5.3-flash
| ("querit".into(), querit), | ||
| ("tavily".into(), tavily), | ||
| ("seltz".into(), seltz), | ||
| ("keenable".into(), keenable), |
There was a problem hiding this comment.
Add the missing external catalog tests
The new Keenable provider specifications are registered without adding the external catalog test file required by the repository's test layout. If the existing module declaration references that file, this leaves the crate unable to compile; even if it is not referenced yet, the new wire contracts and in-memory bus behavior are untested. Add the referenced <module>_tests.rs file and cover the new provider schemas and calls.
[RULE] missing-external-test ·
| // credential to gate on: the host's own enabled entry for it is the | ||
| // opt-in (a default configuration lists nothing), and calls go only to | ||
| // Keenable or the host's base URL. A credential just raises limits. | ||
| ProviderRoute::Direct if name == "keenable" => true, |
There was a problem hiding this comment.
Require authorization before enabling direct Keenable access
This makes every explicitly configured direct Keenable provider active even when explicit.credential is absent. A caller can therefore invoke Keenable's direct tools without authorization, contrary to the credential gate used for keyed direct providers; a public endpoint or a credential that merely raises limits does not establish that the host is authorized to spend its quota. Gate this route on the configured credential, or otherwise require the authorization mechanism used by the Keenable provider before returning true.
Additional security observation
Require authorization before enabling direct Keenable access
[RULE] authorization-bypass
This makes any explicitly enabled direct Keenable provider available without checking a credential or another authorization capability, unlike the other keyed direct providers. Because the catalog controls which provider tools callers can invoke, an enabled configuration can expose Keenable's network-backed search and fetch operations without an authorization gate. Require the appropriate host authorization or credential before returning these tools.
[RULE] missing-authorization ·
| let mut config = SearchConfig::default(); | ||
| config.presentation.mode = PresentationMode::AllTools; | ||
| assert_eq!(tool_names(&config).len(), 0); | ||
| config |
There was a problem hiding this comment.
Require authorization before enabling direct Keenable access
This test explicitly accepts a default ProviderConfig with no credential and expects the direct Keenable tools to be listed. That configuration permits callers to invoke the public endpoints merely by registering the provider, so there is no authorization gate before exposing direct provider access. Require an authorized credential or an explicit opt-in before listing and executing these tools; otherwise an untrusted caller can consume Keenable's public quota through the application.
[RULE] authorization-gate ·
| config | ||
| .providers | ||
| .insert("keenable".into(), ProviderConfig::default()); | ||
| assert_eq!(tool_names(&config), ["keenable_search", "keenable_fetch"]); |
There was a problem hiding this comment.
Require authorization before enabling direct Keenable access
This test explicitly codifies that a default Keenable configuration with no credential exposes both direct tools. A caller can therefore discover and invoke direct Keenable access without an authorization gate. Require an authorized configuration or caller identity before registering these tools, and update the test to assert that unauthorized direct access is not exposed.
[RULE] missing-authorization ·
1 participant
Summary
I work at Keenable. This adds Keenable as a direct provider with two tools:
keenable_searchfor the search role andkeenable_fetchfor the contents role. It needs no credential. Once the host enables it with a direct route, it calls Keenable's keyless/v1/search/publicand/v1/fetch/public, which are rate limited per IP; for a desktop agent that means each user's own IP and no account to set up. A configured credential switches both tools to the keyed/v1/searchand/v1/fetch(sent asX-API-Key) for higher limits.Every request carries
X-Keenable-Title: tinysearch, which the keyless endpoints require. It names the module only, with no user or host identifier. Search sendsquery,max_results(1-20, defaulting to the configured count) and optionalsite,published_afterandpublished_before, and asks for 1,200-character snippets since normalization keeps no more than that. Results mapsnippet(falling back todescription) andpublished_atonto the normalized fields.keenable_fetchreads each URL from Keenable's index and, on a 404 for a page that is not indexed, fetches it live from the source. Failed pages are counted inprovider_data.failed_count. When every page fails, the first failure's code is returned, so a 429 staysrate_limitedand the contents role falls back.Related issue
None
API or behavior changes
Additive, not breaking.
PROVIDERSgainskeenable. It is usable on the direct route without a credential (like SearXNG, but with no base URL needed), and it serves the search and contents roles, last in both default orders. Nothing changes for existing providers or configurations. I leftCONTRACT_VERSIONat 2.0; tell me if a new provider should bump it.Validation
Commands actually run, with their outcome:
cargo fmt --all -- --check: cleancargo clippy --all-targets --all-features -- -D warnings: clean on 1.98cargo build --all-targets --all-features: okcargo test --all-features: 165 passed, 0 failed.github/scripts/check-file-coverage.sh 90 target/coverage.jsonpasses:keenable.rsis at 100%, and the catalog and roles files are at 98% and 100%.RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featuresis clean.A live run with no credential, through
SearchServicein roles mode with onlykeenableconfigured:(Search lines are URL, publish date and snippet length.)
Tests
Seven tests in
provider/direct/keenable/keenable_tests.rsrun against a local server:failed_countrate_limitedcatalog/mod_tests.rsnow also pinskeenablein the catalog, its roles, its role tools and both default orders.Documentation
README.md: Keenable added to the role table, plus a paragraph under Built-in providers.MODULE.md: notes that SearXNG and Keenable need no credential.Once this lands, I'd follow up in OpenHuman with a
keenableentry in the search settings (an optional key field), if you want it there.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description