Repository navigation
feat: harden bridge auth/CI, wire up WiFi ack, trim docs for v0.3.0 - #19
Merged
Merged
Conversation
…cap /state size - Move the bearer-token check into a router-wide middleware layer so it runs before JSON body extraction: unauthenticated requests no longer cost up to 2 MB of deserialization, and 400-vs-401 no longer leaks body validity pre-auth. - Replace the grp.min_by(...).unwrap() in derive_rows with a let-else skip — a request handler must never be one refactor away from a panic. - Cap /state and /metrics session rows at 100 (sorted waiting → working → idle before truncation) to bound response size. - Wrap the federation push loop in catch_unwind like the other four background loops; a panic there silently stopped node aggregation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
… matrix - Delete stale bridge/Cargo.lock (still at vibe-bridge 0.1.0): bridge is a root-workspace member, so cargo resolves from the root Cargo.lock — security.yml was auditing dependencies the build never uses. Point cargo-audit and all cache keys at the root lockfile. - Add a non-blocking bridge coverage job (cargo-llvm-cov → lcov artifact + tokenless Codecov upload). - Build the firmware feature matrix (usb, wifi+ota, eink) as blocking jobs with dummy compile-time credentials; BLE builds in a separate allow-failure job since it is known-broken on the current esp toolchain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
Verified empirically: 1.82 fails (a dependency requires edition2024), 1.85 fails (dependency rustc floor), 1.86 compiles clean. Older stables now get a clear cargo error instead of cryptic compile failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
install_hooks registered six Claude Code lifecycle events but the hub only evaluates two: Notification (marks waiting) and Stop (clears it). The other four fired a vibe_hook POST on every tool call for no effect. - Reduce HOOK_EVENTS to Notification + Stop. - merge_hooks now strips our stale registrations from events we no longer subscribe to (settings written by older installs), leaving user hooks under those events untouched. - Add tests: stale-registration cleanup, and Stop clearing the waiting flag end-to-end through POST /hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
- hostname() output now passes through sanitize_node_id: '/' would corrupt the "<node>/<id>" dedupe key, and an unbounded hostname bloats every federation payload — restrict to [A-Za-z0-9._-], cap at 63 bytes, fall back to unknown-host. - extract_ack_id rejects ids over 128 bytes: session ids are UUIDs, so anything longer is a malformed or malicious frame, and the id is forwarded into an HTTP request. Closes review items #10 and #13 from docs/internal/RUST_CODE_REVIEW.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
dirs-next has been unmaintained by its author for years. dirs 6 is a drop-in replacement for the handful of home_dir() call sites used here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
Four collector-facing paths had zero tests: collector_opencode.rs and collector_hermes.rs (SQLite, via rusqlite fixture DBs in tempdirs), collector_codex.rs (JSONL files, via $HOME override), and the vibe_hook binary (black-box over its actual POST behavior via a local TcpListener). 45 new tests covering: happy path (status/project/age), empty sources, missing/renamed tables or dependent tables (documents the silent Err(_) => return swallow in the SQLite collectors — a real asymmetry worth knowing: Codex's file-based collector degrades to placeholder sessions on drift instead of losing them entirely), corrupt/missing files, timestamp edge cases, and idempotent re-scans. vibe_hook is covered for body shape, token header, camelCase fallbacks, malformed stdin, and exiting cleanly with no listener present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
… forms README.md was 870 lines and largely duplicated docs/api.md, docs/hardware.md, and docs/development.md (full endpoint bodies, wiring diagrams, dev workflow). Trimmed to 213 lines: pitch, demo, features, hardware summary, quick start, links to docs/ for detail, roadmap, license. Every section removed was verified already present in docs/ before deletion; two genuine gaps were filled by extracting docs/troubleshooting.md and docs/wsl2.md from README content that had no home in docs/ yet. - Fix broken docs/assets/demo.gif reference (never existed) -> banner.png. - Correct the hook-events paragraph: Notification marks waiting, Stop clears it (matches today's install_hooks/hub.rs behavior). - Roadmap: WiFi transport entry now covers ACK, no longer implies it's outstanding. - Move FIXES_APPLIED.md and RUST_CODE_REVIEW.md (internal audit artifacts) to docs/internal/ so the repo root reads as a finished project rather than a work-in-progress. - Convert the three legacy Markdown issue templates to GitHub Issue Forms (YAML), same fields/required-flags/labels. Fix config.yml's docs link (tree/main -> tree/master, the actual default branch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
…i ack, show session overflow - Extract the /state mini-JSON scanner and display-formatting helpers (previously ~160 lines of hand-rolled parsing tangled into main.rs) into firmware/src/proto.rs: pure std, no esp-idf dependencies. main.rs now just calls into it. - Add bridge/tests/firmware_proto_test.rs (31 tests) which compiles proto.rs on the host via #[path], covering the parser against the real /state shape from docs/api.md plus malformed/truncated/escaped input and empty-list edge cases. This is the first test coverage the firmware crate has ever had, without needing the Xtensa toolchain. - Implement send_ack for the WiFi build: POST /ack with the session id and bearer token, mirroring fetch_state's connection handling; errors are logged, never panic. Previously a no-op TODO, so tap-to-ack did nothing in WiFi mode despite the README roadmap marking WiFi transport complete. - Sessions/models past MAX_SESSIONS/MAX_MODELS were silently dropped; proto.rs now counts them (dropped_sessions/dropped_models) and main.rs renders a "+N more" row instead of letting them vanish unindicated. - Remove serde and serde-json-core from firmware/Cargo.toml: unused anywhere in firmware/src (confirmed via grep); Cargo.lock updated, only dropping now-unreachable transitive deps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (42)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
CI runs a newer clippy (1.97) than caught this locally; two nested if-lets in the /state and /metrics array-close handling trip collapsible_match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
collector_codex_test.rs redirected \$HOME to a TempDir fixture to isolate codex_sessions_root(). That works on Unix (dirs::home_dir() reads \$HOME directly) but silently fails on Windows, where dirs::home_dir() uses the profile-folder API and ignores \$HOME entirely — the CI windows-latest job scanned the real home directory instead of the fixture, found nothing, and 7 of 11 tests failed (expected N sessions, got 0). Add a \$CODEX_HOME override to codex_sessions_root(), checked before dirs::home_dir(), matching the existing $HERMES_HOME/$HERMES_DATA_DIR pattern in collector_hermes.rs (and the real Codex CLI's own $CODEX_HOME override for ~/.codex). Tests now redirect via $CODEX_HOME instead of hijacking $HOME, which works identically on both platforms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
Liohtml
pushed a commit
that referenced
this pull request
Jul 30, 2026
Minor version bump: WiFi tap-to-ack (previously a no-op) and the session/model overflow indicator are new user-facing functionality, not pure bugfixes, alongside the auth/CI hardening and test coverage landed in #19. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6hLvGUUywUH6P3Szj7i4A
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
2 participants
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
A maintainer audit pass (code + repo health) turned up a backlog of fixes, hardening, tests, and docs cleanup. This PR bundles everything that's safe to land in one pass; a couple of larger/riskier items (a
Providertrait refactor formetrics.rs, aureq2→3 migration) are tracked separately as follow-up issues rather than rushed in here.Bridge hardening
.unwrap()panic path in/state's row derivation; cap/state//metricsat 100 rows.catch_unwindlike the other four background loops./in the hostname would corrupt the<node>/<id>dedupe key) and bound the serial bridge's ack-id length.Notification,Stop) instead of six no-ops;Stopnow correctly clears the waiting flag.CI / deps
security.ymlwas auditingbridge/Cargo.lock, a stale file frozen atvibe-bridge 0.1.0—bridgeis a root-workspace member, so cargo only ever resolves the root lockfile. Deleted the stale file, repointed the workflow.cargo-llvm-cov→ Codecov).rust-version = "1.86"(empirically verified MSRV).dirs-nextwithdirs6.Firmware
/statemini-JSON parser and display-formatting helpers intofirmware/src/proto.rs— purestd, no esp-idf deps, so it's host-testable for the first time (31 new tests).send_ackfor the WiFi build (previously a no-op TODO — tap-to-ack silently did nothing in WiFi mode).serde/serde-json-coredependencies.Tests
vibe_hook(previously zero coverage) — surfaced a real asymmetry: the SQLite collectors silently lose all sessions on schema drift, while the JSONL-based Codex collector degrades to placeholders instead.Docs
docs/, two real gaps filled (docs/troubleshooting.md,docs/wsl2.md).demo.gifreference, corrected the hook-events description, moved internal audit docs out of the repo root, converted issue templates to GitHub Issue Forms.Test plan
cargo test --workspace— all suites green (~184 tests)cargo clippy --workspace --all-targets --all-features -- -D warnings— cleancargo fmt --check— cleansend_ack/overflow-display changes should get a hardware smoke test before releaseRelated: closes the outstanding item from #10's family of WSL2 issues is untouched; opens #18 (tracked follow-up:
Providertrait refactor).Generated by Claude Code