fix(api): send X-Keenable-Title on every request (unbreak free-tier)

[ilya-bogin-keenable]

Problem

The backend now requires the X-Keenable-Title header on token-less (public) endpoints and rejects requests without it with 400 Missing app identifier (keenable-backend-ts commits ce1029e/78ddd22, deployed 20260615220413). The CLI sent no such header, so the unauthenticated free-tier flow broke against production, and the 2026-06-16 nightly e2e went red on:

• test_free_tier_search_hits_public_endpoint
• test_free_tier_fetch_hits_public_endpoint
• test_logout_clears_key_and_falls_back_to_public

{'error': 'Missing app identifier',
 'message': 'X-Keenable-Title header is required for token-less requests'}

Fix

• src/api.rs: send X-Keenable-Title on both clients (api_key_client, bare_client), defaulting to keenable-cli.
• The value is overridable via the KEENABLE_APP_TITLE env var. e2e.yml sets it to keenable-cli-e2e, so first-party CI traffic can be told apart from real CLI users in Grafana via the server-side app_title field (keenable-cli vs keenable-cli-e2e). The daemon inherits the env var, so daemon-routed requests carry the same title.

Tests

• Pure resolve_app_title unit tests (default / trim / blank handling).
• The free-tier and logout-fallback e2e tests now explicitly assert a token-less call never fails with Missing app identifier, so a dropped header fails loudly.

Verified locally against the live API: free-tier search/fetch return 200 (both with the default title and with the e2e title via the daemon); the 3 free-tier tests pass. Login tests require KEENABLE_API_KEY and will run in CI.

🤖 Generated with Claude Code

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Login misses app title 🐞 Bug ≡ Correctness

Description

src/commands/login.rs uses reqwest::Client::new() for /v1/auth/agent/code and /v1/auth/agent/token,
so these token-less requests do not include X-Keenable-Title even after this PR. If the backend
enforces the header for all token-less endpoints (not only /public), login can fail with 400
"Missing app identifier" and users cannot authenticate.

Code

src/api.rs[R130-136]

pub fn bare_client() -> Client {
    Client::builder()
        .user_agent(USER_AGENT)
+        .default_headers(base_headers())
        .timeout(std::time::Duration::from_secs(60))
        .build()
        .unwrap()

Evidence

The PR adds the header via base_headers() and wires it into bare_client(), but login does not
use those helpers and instead builds a new client without default headers.

src/api.rs[29-37]
src/api.rs[130-136]
src/commands/login.rs[27-37]
src/commands/login.rs[69-96]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR adds X-Keenable-Title only to `api_key_client()`/`bare_client()`, but the login flow constructs its own `reqwest::Client` and will therefore omit the header on token-less auth endpoints.

## Issue Context
`src/commands/login.rs` uses `Client::new()` in both `request_code()` and `poll_for_token()`. Those should use a client that includes the new base headers (and ideally the same user-agent).

## Fix Focus Areas
- src/commands/login.rs[27-45]
- src/commands/login.rs[69-106]
- src/api.rs[130-137]

## Suggested fix
- Replace `Client::new()` in login with `crate::api::bare_client()` (or add a small `crate::api::public_client()` wrapper if you don’t want the 60s default timeout).
- Keep the existing per-request `.timeout(Duration::from_secs(10))` calls (they already override defaults).
- Optionally add/adjust a unit/integration test that asserts login endpoints include X-Keenable-Title (if your test harness can observe request headers).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Invalid title drops header 🐞 Bug ☼ Reliability

Description

base_headers() drops X-Keenable-Title entirely when KEENABLE_APP_TITLE cannot be parsed as a
HeaderValue, so a malformed env override re-breaks token-less/public requests with the backend
"Missing app identifier" error. This also makes the failure mode silent and harder to debug since
the CLI will behave differently based solely on env contents.

Code

src/api.rs[R29-37]

+/// Headers common to every keenable API client. Carries `X-Keenable-Title`,
+/// which is mandatory on public endpoints. A non-parseable override is dropped
+/// rather than panicking — the default is always a valid header value.
+fn base_headers() -> reqwest::header::HeaderMap {
+    let mut headers = reqwest::header::HeaderMap::new();
+    if let Ok(value) = app_title().parse() {
+        headers.insert("X-Keenable-Title", value);
+    }
+    headers

Evidence

base_headers() only inserts the header on successful parse; both API clients depend on
base_headers(), so a parse failure means requests go out without the now-mandatory header.

src/api.rs[25-37]
src/api.rs[115-136]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`base_headers()` currently omits the `X-Keenable-Title` header if `KEENABLE_APP_TITLE` is present but not parseable as an HTTP header value. Since the backend requires this header for token-less requests, malformed overrides should fall back to the default rather than removing the header.

## Issue Context
The code intentionally avoids panicking, but dropping the header reintroduces the production failure this PR is addressing.

## Fix Focus Areas
- src/api.rs[29-37]
- src/api.rs[25-27]

## Suggested fix
- If parsing the override fails, insert the default value instead (and optionally emit a warning to stderr/log once).
- Consider using a constant header name (e.g., `HeaderName::from_static("x-keenable-title")`) and always inserting some value:
 - try override -> HeaderValue
 - else default -> HeaderValue::from_static(DEFAULT_APP_TITLE)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

PR Summary by Qodo

fix(api): always send X-Keenable-Title to restore free-tier requests
🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Description

• Add mandatory X-Keenable-Title header to all API clients to unblock public endpoints.
• Allow overriding the header value via KEENABLE_APP_TITLE for observability/attribution.
• Harden coverage with unit + e2e assertions to prevent regressions.

Diagram

graph TD
  A["CLI commands"] --> B["src/api.rs base_headers()"] --> C["reqwest Client"] --> D{{"Keenable backend"}}
  E["KEENABLE_APP_TITLE"] --> B
  B --> F["api_key_client()"] --> C
  B --> G["bare_client()"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Set X-Keenable-Title only on bare/public requests

• ➕ Keeps authenticated traffic unchanged (if that matters for server analytics).
• ➕ Limits any chance of server-side behavior changes tied to the header.
• ➖ Requires routing-specific logic (know which endpoints are public) and is easier to regress.
• ➖ Doesn’t help attribute authenticated CLI traffic in observability.

2. Inject header via a request wrapper/middleware layer

• ➕ Centralizes cross-cutting request concerns (headers, tracing, retries).
• ➕ Scales better if more mandatory headers appear later.
• ➖ More code/abstraction than needed for a single required header.
• ➖ Potentially invasive refactor depending on current call sites.

Recommendation: The PR’s approach (shared base_headers applied to both clients) is the best tradeoff: it guarantees public endpoints always receive the required header, avoids per-endpoint conditional logic, and enables consistent attribution of both authenticated and unauthenticated traffic via KEENABLE_APP_TITLE. The lightweight, non-panicking parse behavior is appropriate for CLI robustness.

Files changed (4) +71 / -1

Bug fix (1) +53 / -1

api.rsAlways send X-Keenable-Title with optional env override +53/-1

Always send X-Keenable-Title with optional env override

• Introduces app title resolution (defaulting to keenable-cli) and a shared base_headers() that injects X-Keenable-Title. Applies the shared headers to both api_key_client and bare_client, and adds unit tests for default/trim/blank override behavior.

src/api.rs

Tests (2) +13 / -0

test_free_tier.pyAssert public calls never fail with Missing app identifier +10/-0

Assert public calls never fail with Missing app identifier

• Documents the X-Keenable-Title requirement for token-less endpoints and adds explicit assertions that free-tier search/fetch failures are not due to missing app identifier. Keeps existing tolerance for 429 rate-limit behavior.

tests/e2e/test_free_tier.py

test_login_flow.pyGuard logout fallback-to-public path against missing app identifier +3/-0

Guard logout fallback-to-public path against missing app identifier

• Extends the logout→public search fallback test to assert errors are never 'Missing app identifier', ensuring the header remains present after logout. Preserves existing behavior that accepts either success or 429 with login hint.

tests/e2e/test_login_flow.py

Other (1) +5 / -0

e2e.ymlSet KEENABLE_APP_TITLE for e2e traffic attribution +5/-0

Set KEENABLE_APP_TITLE for e2e traffic attribution

• Exports KEENABLE_APP_TITLE=keenable-cli-e2e in the e2e workflow so CI requests are tagged via X-Keenable-Title. Adds inline rationale documenting Grafana/observability intent and daemon inheritance.

.github/workflows/e2e.yml

[qodo-code-review]

1. Login misses app title 🐞 Bug ≡ Correctness

src/commands/login.rs uses reqwest::Client::new() for /v1/auth/agent/code and /v1/auth/agent/token,
so these token-less requests do not include X-Keenable-Title even after this PR. If the backend
enforces the header for all token-less endpoints (not only /public), login can fail with 400
"Missing app identifier" and users cannot authenticate.

Agent Prompt

## Issue description
The PR adds X-Keenable-Title only to `api_key_client()`/`bare_client()`, but the login flow constructs its own `reqwest::Client` and will therefore omit the header on token-less auth endpoints.

## Issue Context
`src/commands/login.rs` uses `Client::new()` in both `request_code()` and `poll_for_token()`. Those should use a client that includes the new base headers (and ideally the same user-agent).

## Fix Focus Areas
- src/commands/login.rs[27-45]
- src/commands/login.rs[69-106]
- src/api.rs[130-137]

## Suggested fix
- Replace `Client::new()` in login with `crate::api::bare_client()` (or add a small `crate::api::public_client()` wrapper if you don’t want the 60s default timeout).
- Keep the existing per-request `.timeout(Duration::from_secs(10))` calls (they already override defaults).
- Optionally add/adjust a unit/integration test that asserts login endpoints include X-Keenable-Title (if your test harness can observe request headers).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools