Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ keenable fetch https://example.com # Fetch page content
keenable fetch https://example.com -p # Pretty output
keenable fetch https://example.com --live # Fetch the live page (skip cache)
keenable fetch https://example.com --prompt "List all pricing tiers" # LLM extraction instead of the full page
keenable fetch https://example.com --max-chars 200000 # Raise the 50000-char content cap
```

### Authentication
Expand Down
2 changes: 2 additions & 0 deletions src/commands/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ pub async fn fetch(
url: &str,
live: bool,
prompt: Option<String>,
max_chars: Option<u64>,
human: bool,
api_key: Option<&str>,
) {
Expand All @@ -392,6 +393,7 @@ pub async fn fetch(
urls: Some(vec![url.to_string()]),
live,
prompt,
max_chars,
..Default::default()
};

Expand Down
16 changes: 11 additions & 5 deletions src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub struct DaemonRequest {
/// instruction's output instead of the full page.
#[serde(default)]
pub prompt: Option<String>,
/// Fetch only: content-length cap; the API defaults to 50000 when unset.
#[serde(default)]
pub max_chars: Option<u64>,
}

impl DaemonRequest {
Expand All @@ -27,18 +30,21 @@ impl DaemonRequest {
/// Query params for GET /v1/fetch, shared by the daemon and the direct
/// HTTP path so fetch params can't drift between them. None when `urls`
/// is missing.
pub fn fetch_query(&self) -> Option<Vec<(&str, &str)>> {
let mut query: Vec<(&str, &str)> = self
pub fn fetch_query(&self) -> Option<Vec<(&str, String)>> {
let mut query: Vec<(&str, String)> = self
.urls
.as_ref()?
.iter()
.map(|u| ("url", u.as_str()))
.map(|u| ("url", u.clone()))
.collect();
if self.live {
query.push(("live", "true"));
query.push(("live", "true".to_string()));
}
if let Some(p) = &self.prompt {
query.push(("prompt", p.as_str()));
query.push(("prompt", p.clone()));
}
if let Some(m) = self.max_chars {
query.push(("max_chars", m.to_string()));
}
Some(query)
}
Expand Down
10 changes: 8 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ enum Commands {

/// Fetch page content as markdown (outputs YAML by default, use -p for pretty output)
#[command(
after_help = "Works without login (free tier). Log in for higher rate limits.\n\nExamples:\n keenable fetch https://example.com YAML output\n keenable fetch https://example.com -p Pretty output\n keenable fetch https://example.com --live Fetch the live page (skip cache)\n keenable fetch https://example.com --prompt \"List all pricing tiers\" Extract with an LLM\n keenable fetch https://example.com --api-key keen_***_***** Use a specific API key"
after_help = "Works without login (free tier). Log in for higher rate limits.\n\nExamples:\n keenable fetch https://example.com YAML output\n keenable fetch https://example.com -p Pretty output\n keenable fetch https://example.com --live Fetch the live page (skip cache)\n keenable fetch https://example.com --prompt \"List all pricing tiers\" Extract with an LLM\n keenable fetch https://example.com --max-chars 200000 Raise the 50000-char content cap\n keenable fetch https://example.com --api-key keen_***_***** Use a specific API key"
)]
Fetch {
/// URL to fetch
Expand All @@ -193,6 +193,10 @@ enum Commands {
#[arg(long)]
prompt: Option<String>,

/// Truncate content at this many characters (default: 50000)
#[arg(long = "max-chars", value_parser = clap::value_parser!(u64).range(1..))]
max_chars: Option<u64>,
Comment on lines +196 to +198

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. --max-chars error exits 2 📘 Rule violation ☼ Reliability

--max-chars uses clap value validation (range(1..)) which causes invalid input (e.g., 0) to
exit with code 2, conflicting with the documented requirement that command errors exit with code 1.
The new e2e test codifies this non-1 exit behavior, making the error-handling contract harder to
standardize later.
Agent Prompt
## Issue description
The new `--max-chars` flag is validated by clap (`range(1..)`) which produces a clap parse error exit code (2) on invalid values like `0`. Compliance requires errors to exit with code 1 and to emit actionable/structured errors.

## Issue Context
A new e2e test (`test_fetch_max_chars_rejects_zero`) asserts exit code 2 for `--max-chars 0`, reinforcing the non-compliant error code behavior.

## Fix Focus Areas
- src/main.rs[196-198]
- tests/e2e/test_fetch.py[51-54]

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

Comment on lines +196 to +198

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Unbounded max_chars request 🐞 Bug ☼ Reliability

--max-chars accepts any positive u64, so the CLI/daemon can request extremely large fetch responses.
Since fetch responses are fully buffered and parsed as JSON before printing, very large responses
can drive high memory usage or OOM when the API honors large max_chars values.
Agent Prompt
## Issue description
`--max-chars` currently accepts any positive `u64` and is forwarded to the API. The fetch response is buffered and parsed into `serde_json::Value` before output, so very large responses can cause high memory usage or OOM.

## Issue Context
- The feature intent is to override the backend’s default cap (50,000 chars), so the fix should preserve legitimate increases (e.g. 200,000) while adding a safety guard.

## Fix Focus Areas
- Add a reasonable upper bound in clap parsing (or add a warning/confirmation above a threshold), consistent with expected product limits.
- Optionally document the bound in help/README.
- If large payloads are expected, consider a longer-term refactor to avoid fully buffering/parsing the response (streaming), but the immediate mitigation can be validation/guardrails.

### Code pointers
- src/main.rs[179-199]
- src/daemon.rs[30-49]
- src/commands/search.rs[96-149]
- src/api.rs[143-153]

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


/// Pretty-print output for humans instead of YAML
#[arg(short = 'p', long = "pretty")]
pretty: bool,
Expand Down Expand Up @@ -330,10 +334,12 @@ async fn main() {
url,
live,
prompt,
max_chars,
pretty,
api_key,
} => {
commands::search::fetch(&url, live, prompt, pretty, api_key.as_deref()).await;
commands::search::fetch(&url, live, prompt, max_chars, pretty, api_key.as_deref())
.await;
}
Commands::Feedback {
query,
Expand Down
15 changes: 15 additions & 0 deletions tests/e2e/test_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ def test_fetch_prompt(kn):
assert "This domain is for use in illustrative examples" not in data["content"]


def test_fetch_max_chars(kn):
res = kn("fetch", "https://example.com", "--max-chars", "50")
assert res.code == 0
data = res.yaml()
# The truncation disclaimer is the load-bearing assert: a stale daemon
# that drops `max_chars` returns the full page with exit code 0.
assert "truncated to stay below 50 characters" in data["content"]

Comment on lines +46 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

3. Brittle truncation assertion 🐞 Bug ☼ Reliability

The new e2e test hard-depends on a specific backend-generated truncation disclaimer phrase. If the
API changes wording while preserving behavior, the test will fail even though --max-chars plumbing
still works.
Agent Prompt
## Issue description
`test_fetch_max_chars` asserts a specific disclaimer substring from live API output. This couples the test to backend prose and can cause false failures on harmless wording changes.

## Issue Context
The comment explains the assert is intended to detect a stale daemon that drops `max_chars`. That intent can still be met with more stable assertions.

## Fix Focus Areas
- Replace the exact-phrase match with a more robust invariant (e.g., content length is <= a small bound; or content contains a generic "truncated" marker; or (best) assert a structured field if the API provides one).

### Code pointers
- tests/e2e/test_fetch.py[42-55]

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


def test_fetch_max_chars_rejects_zero(kn):
res = kn("fetch", "https://example.com", "--max-chars", "0")
assert res.code == 2
assert "invalid value" in res.err


def test_pretty_fetch(kn):
res = kn("fetch", "https://example.com", "-p")
assert res.code == 0
Expand Down
Loading