fix(dashboard): log DB errors, validate cost by=, gate non-local bind, add HTTP tests - #369
Merged
Merged
Conversation
added 13 commits
July 27, 2026 17:24
Adds two non-consent components so agentflare init and every SessionStart self-heal the setup needed to actually use flare-docs, flare-search, and lean-ctx through the gateway, instead of relying on hand-run CLI commands: - core-coaching: seeds/refreshes 4 builtin coaching rules (usedocs, usesearch, useleanctx, usetsearch) that nudge the flare gateway's docs/search/lean-ctx/tool-search wrappers over their native equivalents. Drift-protected across version bumps; a same-id rule the user has overridden to a different tier is left alone. - gateway-permissions: keeps ~/.claude/settings.json's permissions.allow containing the flare gateway tools (mcp__flare__docs, mcp__flare__search, mcp__flare__tool, ToolSearch) and strips superseded direct mcp__lean-ctx__* entries. Also widens coaching::store::list_rules to pub(crate) so components.rs can read existing rule state when deciding whether to seed or refresh.
Adds docs-site/, an Astro Starlight project documenting the flare-docs module (overview, supported languages, MCP tool reference, CLI reference, examples, and a comparison against Context7), reusing the landing page's exact design system (colors, fonts, mono headings) via customCss. site/ and docs-site/ are linked as sibling packages in an aube-workspace.yaml monorepo rooted at the repo root, with mise tasks (install/dev:site/dev:docs/ build:docs/deploy/tail) as the entry points. The two connect only at deploy time: site's deploy script builds docs-site out-of-tree and copies its output into site/public/docs before wrangler deploy, so day-to-day dev on either package never touches the other.
Adds Ecosystem::Python alongside Rust and npm: fetches a package's PyPI manifest, unpacks its pure-Python wheel, and indexes whichever typed source it carries — separate .pyi stubs, or (for PEP 561 inline-typed packages like click and pandas) the package's own annotated .py source with a py.typed marker — via a tree-sitter-python extraction pass mirroring the existing npm/.d.ts module. Falls back to typeshed's types-<package> convention on PyPI when a package ships neither. Wired into the CLI (`agentflare docs get <pkg> --ecosystem python`) and the MCP `docs` tool. Verified live against PyPI: requests correctly falls back to types-requests; click indexes 470 items from its own inline-typed source with no fallback needed.
Fills the one real gap Rust didn't have: rustdoc-JSON already carries a
crate's doc-comment Examples verbatim, but npm and Python previously
discarded the README/long-description entirely, keeping only .d.ts/.pyi
signatures.
Adds a shared, LLM-free fenced-code-block extractor (crates/flare-docs/src
/readme.rs) reused by both ecosystems at zero extra network cost: npm scans
every .md file already inside the tarball it downloads for .d.ts, and
Python reads info.description straight out of the PyPI manifest JSON it
already fetches (markdown releases only -- RST is explicitly out of scope
rather than mis-parsed). Each block is titled by its nearest heading and
indexed as npm-example/python-example, reconciled the same way API items
are.
Filters two kinds of noise before indexing: examples under process/meta
headings (Installation, Contributing, License, Testing, ...), and
shell-flavored blocks that are pure setup commands (git clone, pip install)
even under an otherwise-good heading like "Quick Start" -- verified live
against requests' real PyPI description, which previously surfaced its
"Cloning the repository" git commands as if they were usage examples.
Also fixes a path-matching gap in both the new markdown-file filter and the
existing Python inline-typed-package filter: a top-level `test/` directory
(no leading slash after tarball-prefix stripping) wasn't caught by a
mid-path `contains("/test/")` check.
… ones Now that flare-docs indexes usage examples too, add a comparison row and strengthen the "where flare-docs wins" case: examples are extracted verbatim from the package's own docs, not synthesized by an LLM the way Context7's snippets are -- so they can't drift from what the maintainer actually wrote.
…ment-components # Conflicts: # src/components.rs # src/mcp_server/flare_docs.rs
…ad of firing every call Previously the batching_nudge fired on every single tool call once a streak crossed the trailing-window threshold, spamming the same message. Now it only fires at streak milestones (3, 6, 12, 24, ...), computed statelessly from recent_tool_calls history.
… enforced rule Coaching rules were 100% advisory: a nudge in systemMessage the agent could ignore. Adds an enforced flag (# Enforce: true header, coaching enforce <id> [--off] CLI) that the PreToolUse hook checks before falling through to advisory nudges — a match denies the call with a redirect-framed reason instead of just suggesting the preferred tool. Adopts lean-ctx's own PreToolUse mechanism as the reference (studied in ~/workspace/refs/lean-ctx): it substitutes tool input rather than denying, which only works when the substitute is the same tool with different input (e.g. a Bash command rewrite). None of agentflare's coaching rules are same-tool swaps, so this instead reuses the existing deny-decision shape from hook_redirect.rs, with the reason phrased as a redirect. Of the four builtin rules, only usesearch (WebFetch/WebSearch) and usetsearch (ToolSearch) are marked enforced by default: they're clean 1:1 substitutes with no fallback the mandatory tool can't cover. useleanctx stays advisory since ctx_* can't handle every path/case native Bash/Read can (hard-blocking risks a self-lockout), and usedocs stays advisory because its trigger (Edit/Write) isn't a same-action substitute for mcp__flare__docs — blocking edits to redirect to a docs lookup would just break editing.
write_rule_file crossed clippy's default 7-arg threshold when the enforced param was added; allow it like the codebase's other multi-field constructors. The fmt diff is pre-existing drift in flare-docs and the coaching files from this branch's own history, not from this commit.
…, add HTTP tests Closes the PR #255 review backlog. data.rs accessors silently returned "[]" on any DB error, making a broken/missing DB indistinguishable from genuinely empty state in the server log — now every error arm logs before falling back. /api/cost?by= silently coerced unknown values to model grouping instead of surfacing caller typos as a 400. Binding to a non-loopback host exposed all PM/cost/webhook data with only an easy-to- miss stderr warning — it now refuses to start unless --yes-expose is passed. Also adds HTTP-layer test coverage for cost_handler, the /events SSE stream, and static_handler that the module previously lacked.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe dashboard adds explicit acknowledgement for non-loopback exposure, validates cost grouping queries, logs read-only data retrieval failures, and expands endpoint tests. The development ChangesDashboard safety and observability
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ServeArgs
participant dashboard_serve
participant server_run
ServeArgs->>dashboard_serve: pass yes_expose
dashboard_serve->>server_run: pass host and yes_expose
server_run->>server_run: enforce loopback binding or acknowledgement
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dashboard/server.rs`:
- Around line 245-263: Update run to bind the TcpListener before performing
exposure checks, then use listener.local_addr().ip().is_loopback() for both the
refusal condition and the subsequent warning check instead of
is_local_bind(host). Remove or replace is_local_bind as appropriate, and update
the related test to cover resolved socket behavior, including non-127.0.0.1
loopback addresses and localhost resolving to a non-loopback address.
- Around line 375-402: Update events_endpoint_streams_claims_and_cost_snapshot
to accumulate bytes from resp.bytes_stream() until the SSE blank-line record
delimiter is received, rather than parsing the first stream.next() chunk
directly. Decode and parse the complete event afterward, preserving the existing
claims and cost_today assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a524f2d-4512-497a-a918-aeacbcc071ed
📒 Files selected for processing (5)
Cargo.tomlsrc/cli/serve.rssrc/dashboard/data.rssrc/dashboard/mod.rssrc/dashboard/server.rs
Comment on lines
+245
to
+263
| fn is_local_bind(host: &str) -> bool { | ||
| matches!(host, "127.0.0.1" | "localhost" | "::1") | ||
| } | ||
|
|
||
| pub async fn run(host: &str, port: u16, open: bool, yes_expose: bool) { | ||
| if !is_local_bind(host) && !yes_expose { | ||
| eprintln!( | ||
| "refusing to bind to {host}: this would expose all PM/cost/webhook data with no authentication." | ||
| ); | ||
| eprintln!("pass --yes-expose to bind anyway (trusted networks only)."); | ||
| std::process::exit(1); | ||
| } | ||
| let listener = tokio::net::TcpListener::bind((host, port)) | ||
| .await | ||
| .expect("failed to bind dashboard server"); | ||
| let addr = listener.local_addr().expect("no local addr"); | ||
| let url = format!("http://{addr}"); | ||
| eprintln!("agentflare dashboard listening on {url}"); | ||
| if host != "127.0.0.1" && host != "localhost" { | ||
| if !is_local_bind(host) { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Enforce exposure safety against the bound socket address.
localhost is allowlisted before resolution, so a local resolver/hosts configuration mapping it to a LAN address bypasses --yes-expose; conversely, valid loopback addresses such as 127.0.0.2 are rejected. Bind first, then use listener.local_addr()?.ip().is_loopback() for both the refusal and warning checks, and update the related test accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/dashboard/server.rs` around lines 245 - 263, Update run to bind the
TcpListener before performing exposure checks, then use
listener.local_addr().ip().is_loopback() for both the refusal condition and the
subsequent warning check instead of is_local_bind(host). Remove or replace
is_local_bind as appropriate, and update the related test to cover resolved
socket behavior, including non-127.0.0.1 loopback addresses and localhost
resolving to a non-loopback address.
Comment on lines
+375
to
+402
| async fn events_endpoint_streams_claims_and_cost_snapshot() { | ||
| use tokio_stream::StreamExt as _; | ||
| let listener = tokio::net::TcpListener::bind(("127.0.0.1", 0)) | ||
| .await | ||
| .unwrap(); | ||
| let addr = listener.local_addr().unwrap(); | ||
| tokio::spawn(async move { | ||
| axum::serve(listener, router()).await.unwrap(); | ||
| }); | ||
| let resp = reqwest::get(format!("http://{addr}/events")).await.unwrap(); | ||
| let mut stream = resp.bytes_stream(); | ||
| let first = tokio::time::timeout(std::time::Duration::from_secs(10), stream.next()) | ||
| .await | ||
| .expect("first SSE frame within 10s") | ||
| .expect("stream item") | ||
| .unwrap(); | ||
| let text = String::from_utf8(first.to_vec()).unwrap(); | ||
| let data_line = text | ||
| .lines() | ||
| .find_map(|l| l.strip_prefix("data: ")) | ||
| .expect("SSE data line"); | ||
| let v: serde_json::Value = serde_json::from_str(data_line).unwrap(); | ||
| assert!(v.get("claims").is_some(), "expected claims field in {v}"); | ||
| assert!( | ||
| v.get("cost_today").is_some(), | ||
| "expected cost_today field in {v}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and inspect the relevant range.
grep -n "events_endpoint_streams_claims_and_cost_snapshot" -n src/dashboard/server.rs || true
nl -ba src/dashboard/server.rs | sed -n '330,430p'
# Find SSE-related code that produces /events.
grep -n "text/event-stream\|SSE\|events" -n src/dashboard/server.rs | sed -n '1,120p'Repository: getappz/agentflare
Length of output: 266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the surrounding server implementation and event stream producer.
nl -ba src/dashboard/server.rs | sed -n '1,260p'
nl -ba src/dashboard/server.rs | sed -n '260,520p'Repository: getappz/agentflare
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow search for the exact test and any SSE framing logic.
grep -RIn --line-number --fixed-strings "bytes_stream()" src || true
grep -RIn --line-number --fixed-strings "data: " src/dashboard || true
grep -RIn --line-number --fixed-strings "text/event-stream" src || trueRepository: getappz/agentflare
Length of output: 426
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show only the test and the /events handler around the relevant lines.
nl -ba src/dashboard/server.rs | sed -n '360,430p'Repository: getappz/agentflare
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the relevant portion of src/dashboard/server.rs with line numbers.
awk 'NR>=320 && NR<=430 { printf "%4d: %s\n", NR, $0 }' src/dashboard/server.rsRepository: getappz/agentflare
Length of output: 3830
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact response framing used by the /events endpoint.
awk 'NR>=1 && NR<=320 { printf "%4d: %s\n", NR, $0 }' src/dashboard/server.rs | sed -n '1,320p'Repository: getappz/agentflare
Length of output: 13140
Buffer the SSE event before parsing.
resp.bytes_stream() can split the first SSE record across transport chunks, so stream.next() may return partial data: JSON and flake. Accumulate bytes until the blank-line delimiter before decoding.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/dashboard/server.rs` around lines 375 - 402, Update
events_endpoint_streams_claims_and_cost_snapshot to accumulate bytes from
resp.bytes_stream() until the SSE blank-line record delimiter is received,
rather than parsing the first stream.next() chunk directly. Decode and parse the
complete event afterward, preserving the existing claims and cost_today
assertions.
Exercises the --yes-expose gate end-to-end for the exempt path: run() with a 127.0.0.1 host and yes_expose=false actually binds and serves, rather than only asserting the pure is_local_bind() helper. The non-local refusal path isn't covered here since it calls std::process::exit, which isn't safe to trigger inside the test binary.
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
1 participant
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.
Summary
data.rsaccessors log the underlying error before falling back to\"[]\", so a broken/missing DB is no longer indistinguishable from genuinely empty state in the server log./api/cost?by=now rejects any value other thanmodel/projectwith a 400 instead of silently coercing typos to model grouping.--yes-exposeis passed (previously only a single easy-to-miss stderr warning gated unauthenticated LAN exposure of PM/cost/webhook data).cost_handler(defaults + the new 400 path), the/eventsSSE stream, andstatic_handler(index fallback, 404,..rejection).Closes the four low-priority PR #255 review follow-ups.
Test plan
cargo check --all-targetscargo test --bin agentflare dashboard::— 19/19 passingSummary by CodeRabbit
--yes-exposeCLI option to intentionally allow dashboard access beyond localhost.byquery parameter and returns400for invalid values.