Skip to content

feat: Add OpenRouter as LLM backend option - #50

Closed
lwlee2608 wants to merge 2 commits into
nearai:mainfrom
lwlee2608:feat/openrouter-provider
Closed

lwlee2608 wants to merge 2 commits into
nearai:mainfrom
lwlee2608:feat/openrouter-provider

Conversation

@lwlee2608

@lwlee2608 lwlee2608 commented Feb 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add OpenRouter as a new LLM backend option.

While OpenRouter uses an OpenAI-compatible API (and could be configured via the existing openai_compatible backend), that requires users to set three env vars (LLM_BACKEND, LLM_BASE_URL, LLM_API_KEY) and know the correct base URL. A dedicated openrouter variant reduces setup to two env vars (LLM_BACKEND=openrouter, OPENROUTER_API_KEY) with the base URL and a sensible default model pre-configured.

Changes

  • Add OpenRouter variant to LlmBackend enum
  • Add OpenRouterConfig struct and env var resolution (OPENROUTER_API_KEY, OPENROUTER_MODEL)
  • Add create_openrouter_provider() using rig-core's OpenAI client pointed at openrouter.ai/api/v1
  • Update .env.example to document OpenRouter as an alternative provider

Config comparison

openai_compatible openrouter (this PR)
LLM_BACKEND openai_compatible openrouter
Base URL LLM_BASE_URL=https://openrouter.ai/api/v1 (built-in)
API key LLM_API_KEY=sk-or-... OPENROUTER_API_KEY=sk-or-...
Model LLM_MODEL=anthropic/claude-sonnet-4 (defaults to anthropic/claude-sonnet-4)
Total env vars 4 2

Add OpenRouter as a new LLM provider using rig-core's OpenAI-compatible
client pointed at openrouter.ai/api/v1. Make it the default backend,
replacing NEAR AI (which remains available via LLM_BACKEND=nearai).

Env vars: OPENROUTER_API_KEY (required), OPENROUTER_MODEL (default:
anthropic/claude-sonnet-4).
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @lwlee2608, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the flexibility of the application's LLM integration by establishing OpenRouter as the new default backend. This change provides users with broader access to various LLM models through a unified, OpenAI-compatible API, simplifying API key management and model selection. While OpenRouter is now the primary choice, the existing NEAR AI integration is maintained as an accessible alternative, ensuring continuity for current users.

Highlights

  • OpenRouter Integration: OpenRouter has been added as a new Large Language Model (LLM) provider, leveraging rig-core's OpenAI-compatible client.
  • Default LLM Backend Change: OpenRouter is now the default LLM backend, replacing NEAR AI. NEAR AI remains available as an alternative by setting LLM_BACKEND=nearai.
  • Configuration Updates: New environment variables OPENROUTER_API_KEY (required) and OPENROUTER_MODEL (optional, defaults to anthropic/claude-sonnet-4) have been introduced for OpenRouter configuration.
  • Codebase Adaptations: The LlmBackend enum, LlmConfig struct, and related provider creation logic have been updated to support and prioritize OpenRouter.
Changelog
  • .env.example
    • Updated comments to reflect OpenRouter as the default LLM provider.
    • Added OPENROUTER_API_KEY and OPENROUTER_MODEL environment variable examples.
    • Relocated NEAR AI configuration to an 'alternative' section.
  • src/config.rs
    • Modified the LlmBackend enum to set OpenRouter as the default and updated its documentation.
    • Added OpenRouterConfig struct to hold OpenRouter API key and model.
    • Included an openrouter field in the LlmConfig struct.
    • Updated the from_str implementation for LlmBackend to parse 'openrouter' and 'open_router'.
    • Adjusted the LlmConfig::resolve function to default to OpenRouter and load its configuration.
  • src/llm/mod.rs
    • Updated module-level documentation to list OpenRouter as the default LLM backend.
    • Added a new function create_openrouter_provider to initialize the OpenRouter client.
    • Integrated the OpenRouter backend into the create_llm_provider match statement.
  • src/setup/wizard.rs
    • Initialized the new openrouter field to None in the default LlmConfig during setup wizard initialization.
Activity
  • No human activity (comments, reviews, etc.) has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@lwlee2608 lwlee2608 changed the title feat: Add OpenRouter as default LLM backend feat: Add OpenRouter as LLM backend option Feb 12, 2026
OpenRouter is added as an alternative, not the default. Users opt in
with LLM_BACKEND=openrouter.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request successfully adds OpenRouter as the new default LLM backend. The changes are well-structured, introducing a new configuration for OpenRouter and integrating it into the provider factory. My review includes a few suggestions to improve flexibility and correctness: making the OpenRouter base URL configurable, clarifying an error message for new users, and correcting the default OpenRouter model name which appears to be invalid.

Comment thread .env.example
Comment thread src/config.rs
Comment thread src/config.rs
.map(SecretString::from)
.ok_or_else(|| ConfigError::MissingRequired {
key: "OPENROUTER_API_KEY".to_string(),
hint: "Set OPENROUTER_API_KEY when LLM_BACKEND=openrouter".to_string(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since OpenRouter is now the default backend, this error message might be confusing for new users who haven't set LLM_BACKEND. The hint could be clarified to explain that OpenRouter is the default and requires an API key.

Suggested change
hint: "Set OPENROUTER_API_KEY when LLM_BACKEND=openrouter".to_string(),
hint: "Set OPENROUTER_API_KEY to use the OpenRouter backend (default). Alternatively, set LLM_BACKEND to use a different provider.".to_string(),

Comment thread src/llm/mod.rs
@lwlee2608 lwlee2608 closed this Feb 12, 2026
zmanian added a commit that referenced this pull request May 5, 2026
Closes the remaining libSQL/Postgres/JSONL gaps surfaced in the latest
serrrfirat review pass on PR #3171.

libSQL production gate (#34, #36, #41):
- Reject `:memory:` for Production via the InMemory error variant.
- Case-insensitive scheme matching for `HTTP://` / `HTTPS://` /
  `LibSQL://`, so a mixed-case URL no longer falls through to
  Builder::new_local and silently creates a node-local SQLite path.
- Reject bare hostname-like values (e.g. `db.example.com`) for
  Production via a new ProductionLibsqlAmbiguousTarget error; LocalDev
  still accepts ergonomic forms like `events.db`.

libSQL backend hardening (#45, #49, #50):
- Skip `create_dir_all("")` for `events.db` in cwd.
- Enable `journal_mode=WAL` + `synchronous=NORMAL` once at build time
  for file-backed local stores so a long replay reader can no longer
  block writer commits past `busy_timeout`.
- Mirror the v1 retry pattern (`src/db/libsql/mod.rs::connect`): three
  attempts with exponential backoff so concurrent transient
  "unable to open database file" errors do not surface as durable-log
  failures.

Postgres backend hardening (#46):
- Honor `sslmode=require` for loopback configs so a TLS-only local
  Postgres / loopback proxy is accepted instead of forced to NoTls.

JSONL hardening (#38, #40, #42, #43, #44):
- Make JSONL constructors crate-private so production composition
  cannot bypass the single-node-durable acceptance gate.
- Hash path components (SHA-256 / 16-hex-char prefix + 32-byte URL-
  encoded hint) so case-distinct IDs (`Alice` vs `alice`) cannot
  collide on case-insensitive filesystems and 256-byte scope IDs no
  longer overflow the 255-byte filename limit.
- Snapshot file length before write/flush/sync; truncate back on any
  error so a partial write never leaves a torn JSON tail that wedges
  every subsequent append.
- Create directories with `0o700` and stream files with `0o600` on
  Unix so durable history is not world-readable under the typical
  `umask 022`.

SQL replay correctness (#37):
- After fetching filtered rows, run a small unfiltered COUNT over the
  scanned cursor window in both libSQL and Postgres backends; mismatch
  surfaces a missing entry row as `EventError::ReplayGap`. JSONL
  already detects this via line-by-line cursor sequencing.

Regression tests:
- libSQL/Postgres production gate (case-insensitive scheme, in-memory,
  bare hostname).
- Hashed-path-component case distinctness and length boundedness.
- Atomic JSONL append: failed serialise leaves file at pre-append
  length and the next append still advances cursor cleanly.
- Unix `0o700` JSONL root permissions.
- libpq quoted single-quote socket path is local; whitespace-around-
  `=` keyword strings classify remote correctly.
- libSQL replay surfaces a deleted entry row as `ReplayGap`.

Deferred with explicit acknowledgement:
- #39 (`ReadScope.invocation_id`) — module-doc note; needs a
  cross-crate change to `ironclaw_events` plus every replay caller.
- #48 (replay holds writer-blocking locks) — inline note at the JSONL
  read path; needs a stream-bytes-snapshot redesign coordinated with
  the durable-log contract.

Test plan
- cargo test -p ironclaw_reborn_event_store
- cargo test -p ironclaw_reborn_event_store --features "libsql postgres"
- cargo test -p ironclaw_host_runtime
- cargo test -p ironclaw_events
- cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
- cargo clippy -p ironclaw_reborn_event_store --all-targets -- -D warnings
- cargo clippy -p ironclaw_reborn_event_store --features "libsql postgres" --all-targets -- -D warnings
- cargo clippy -p ironclaw_reborn_event_store --no-default-features --all-targets -- -D warnings
- cargo clippy -p ironclaw_host_runtime --all-targets -- -D warnings
- cargo fmt --all -- --check

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
serrrfirat added a commit that referenced this pull request May 6, 2026
* Add Reborn event store backends

* Address Reborn event store review feedback

* Address Reborn event store review feedback (round 2)

Postgres TLS / fail-closed (comment 3178548634): the Postgres event-store
client now uses tokio-postgres-rustls for any non-loopback URL, mirroring
src/db/tls.rs (native certs with webpki-roots fallback). Local sockets and
loopback hosts continue to use NoTls; unparseable URLs with a scheme are
treated as remote so a typo cannot silently downgrade to plaintext.

next_cursor advancement (comment 3178548646): JSONL, libSQL, and Postgres
read paths now track the highest scanned cursor independently from the last
matched cursor, and return max(last_matched, last_scanned). Without this,
a matched record followed by a filtered-out record would leave next_cursor
pinned to the matched cursor and the filtered record would be rescanned on
every subsequent replay.

JSONL bounded replay (comment 3178548670): replays now stream the JSONL
file line-by-line via BufReader, decoding only the cursor envelope until
a line crosses `after`, and stop as soon as `limit` matches are collected.
A `limit = 1` request against a multi-gigabyte stream no longer pays
full-file allocation or full-stream parse latency.

JSONL cross-process locking (comment 3178548701): JSONL appends now take
an OS-level exclusive advisory lock (std::fs::File::lock) for the entire
read-tail-cursor + write window. Two IronClaw processes pointing at the
same JSONL root will block on this lock and emit monotonically-sequenced
cursors instead of corrupting the stream with duplicates. Readers take a
shared lock to avoid observing partially-written tail lines.

New deps on the event-store crate (postgres feature only):
tokio-postgres-rustls, rustls, rustls-native-certs, webpki-roots, url.
All four were already used elsewhere in the workspace; no new external
crates are introduced. File locking uses stdlib (Rust 1.89+ stable).

Regression tests added:
- jsonl_replay_advances_next_cursor_past_trailing_filtered_records
- jsonl_concurrent_appenders_emit_monotonic_cursors_through_file_lock
- jsonl_bounded_replay_does_not_parse_the_whole_file
- postgres_store::tests::{local_postgres_urls_are_recognised,
  remote_postgres_urls_require_tls,
  unparseable_postgres_url_with_scheme_falls_closed_to_remote}

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn_event_store): close TLS bypass for libpq keyword strings and http:// libsql in production

Two High-severity findings from PR #3171 review:

1. is_local_postgres_url() previously returned true for any string without
   "://", so libpq keyword form like "host=db.example.com user=..." took
   the NoTls branch and connected over plaintext. Walk the keyword list,
   look at the actual host, and only treat localhost / socket paths as
   local. Tests cover remote keyword strings, socket paths, and missing
   host=.

2. Production profile previously accepted http:// libSQL URLs and forwarded
   the auth token in cleartext. Reject http:// for RebornProfile::Production
   with a typed error before the build call. LocalDev / Test still accept
   http:// for running against local sqld instances. Tests cover both.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn_event_store): use deadpool-postgres pool for transparent reconnection

Replaces the single Arc<Client> in PostgresStore with a deadpool_postgres::Pool.
The previous design built one tokio_postgres::Client at startup; if the
underlying connection task exited (idle timeout, server restart, failover,
transient network drop), all subsequent append/read operations would fail
forever because nothing rebuilt the connection.

deadpool's Manager replaces broken Clients on the next pool.get() call, so
each call site sees a live connection without per-site reconnect logic.
This mirrors v1's connection-management story (src/db/postgres.rs,
src/db/tls.rs both use deadpool-postgres) and avoids two divergent
reconnection paths in one binary.

Addresses serrrfirat's Medium-severity finding on PR #3171.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(tests): fix post-merge clippy warning

* fix(reborn_event_store): close TLS-mode + libpq-config + JSONL fsync gaps

Three follow-ups from PR #3171 round-3 review:

1. CRITICAL — remote Postgres TLS not actually enforced. Passing a rustls
   connector is not enough on its own: tokio-postgres only consults the
   connector when Config::ssl_mode is Prefer or Require, and `sslmode=disable`
   in the connection string returns a plaintext stream before TLS is
   attempted. Add `enforce_remote_ssl_mode`: reject Disable for non-local
   configs, and force Prefer (the default) up to Require so the server
   cannot decline TLS without failing the connection.

2. HIGH — local/remote detector missed valid libpq forms. Re-parsing the
   raw connection string failed on `hostaddr=10.0.0.5` (numeric-IP keyword,
   no `host=` entry), `postgresql:///db?host=db.example.com` (URL with
   empty authority + host in query), and `host=/var/run/postgresql,
   db.example.com` (mixed socket/TCP list). Replace with
   `is_local_postgres_config` that walks the parsed `Config::get_hosts()`
   and `Config::get_hostaddrs()` so all libpq normalisations land in the
   same code path.

3. MEDIUM — JSONL append fsynced file contents but not the parent
   directory entry. On POSIX the new file's directory entry must also be
   fsynced for crash durability; without it the first append can vanish
   after a power loss even though `append()` returned success. Detect
   first-create via `path.exists()` pre-open and fsync the parent dir
   after `sync_data()`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn_event_store): address PR #3171 round-3 review findings

Closes the remaining libSQL/Postgres/JSONL gaps surfaced in the latest
serrrfirat review pass on PR #3171.

libSQL production gate (#34, #36, #41):
- Reject `:memory:` for Production via the InMemory error variant.
- Case-insensitive scheme matching for `HTTP://` / `HTTPS://` /
  `LibSQL://`, so a mixed-case URL no longer falls through to
  Builder::new_local and silently creates a node-local SQLite path.
- Reject bare hostname-like values (e.g. `db.example.com`) for
  Production via a new ProductionLibsqlAmbiguousTarget error; LocalDev
  still accepts ergonomic forms like `events.db`.

libSQL backend hardening (#45, #49, #50):
- Skip `create_dir_all("")` for `events.db` in cwd.
- Enable `journal_mode=WAL` + `synchronous=NORMAL` once at build time
  for file-backed local stores so a long replay reader can no longer
  block writer commits past `busy_timeout`.
- Mirror the v1 retry pattern (`src/db/libsql/mod.rs::connect`): three
  attempts with exponential backoff so concurrent transient
  "unable to open database file" errors do not surface as durable-log
  failures.

Postgres backend hardening (#46):
- Honor `sslmode=require` for loopback configs so a TLS-only local
  Postgres / loopback proxy is accepted instead of forced to NoTls.

JSONL hardening (#38, #40, #42, #43, #44):
- Make JSONL constructors crate-private so production composition
  cannot bypass the single-node-durable acceptance gate.
- Hash path components (SHA-256 / 16-hex-char prefix + 32-byte URL-
  encoded hint) so case-distinct IDs (`Alice` vs `alice`) cannot
  collide on case-insensitive filesystems and 256-byte scope IDs no
  longer overflow the 255-byte filename limit.
- Snapshot file length before write/flush/sync; truncate back on any
  error so a partial write never leaves a torn JSON tail that wedges
  every subsequent append.
- Create directories with `0o700` and stream files with `0o600` on
  Unix so durable history is not world-readable under the typical
  `umask 022`.

SQL replay correctness (#37):
- After fetching filtered rows, run a small unfiltered COUNT over the
  scanned cursor window in both libSQL and Postgres backends; mismatch
  surfaces a missing entry row as `EventError::ReplayGap`. JSONL
  already detects this via line-by-line cursor sequencing.

Regression tests:
- libSQL/Postgres production gate (case-insensitive scheme, in-memory,
  bare hostname).
- Hashed-path-component case distinctness and length boundedness.
- Atomic JSONL append: failed serialise leaves file at pre-append
  length and the next append still advances cursor cleanly.
- Unix `0o700` JSONL root permissions.
- libpq quoted single-quote socket path is local; whitespace-around-
  `=` keyword strings classify remote correctly.
- libSQL replay surfaces a deleted entry row as `ReplayGap`.

Deferred with explicit acknowledgement:
- #39 (`ReadScope.invocation_id`) — module-doc note; needs a
  cross-crate change to `ironclaw_events` plus every replay caller.
- #48 (replay holds writer-blocking locks) — inline note at the JSONL
  read path; needs a stream-bytes-snapshot redesign coordinated with
  the durable-log contract.

Test plan
- cargo test -p ironclaw_reborn_event_store
- cargo test -p ironclaw_reborn_event_store --features "libsql postgres"
- cargo test -p ironclaw_host_runtime
- cargo test -p ironclaw_events
- cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
- cargo clippy -p ironclaw_reborn_event_store --all-targets -- -D warnings
- cargo clippy -p ironclaw_reborn_event_store --features "libsql postgres" --all-targets -- -D warnings
- cargo clippy -p ironclaw_reborn_event_store --no-default-features --all-targets -- -D warnings
- cargo clippy -p ironclaw_host_runtime --all-targets -- -D warnings
- cargo fmt --all -- --check

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: serrrfirat <f@nuff.tech>
theredspoon referenced this pull request in theredspoon/ironclaw Jun 21, 2026
* Add Reborn event store backends

* Address Reborn event store review feedback

* Address Reborn event store review feedback (round 2)

Postgres TLS / fail-closed (comment 3178548634): the Postgres event-store
client now uses tokio-postgres-rustls for any non-loopback URL, mirroring
src/db/tls.rs (native certs with webpki-roots fallback). Local sockets and
loopback hosts continue to use NoTls; unparseable URLs with a scheme are
treated as remote so a typo cannot silently downgrade to plaintext.

next_cursor advancement (comment 3178548646): JSONL, libSQL, and Postgres
read paths now track the highest scanned cursor independently from the last
matched cursor, and return max(last_matched, last_scanned). Without this,
a matched record followed by a filtered-out record would leave next_cursor
pinned to the matched cursor and the filtered record would be rescanned on
every subsequent replay.

JSONL bounded replay (comment 3178548670): replays now stream the JSONL
file line-by-line via BufReader, decoding only the cursor envelope until
a line crosses `after`, and stop as soon as `limit` matches are collected.
A `limit = 1` request against a multi-gigabyte stream no longer pays
full-file allocation or full-stream parse latency.

JSONL cross-process locking (comment 3178548701): JSONL appends now take
an OS-level exclusive advisory lock (std::fs::File::lock) for the entire
read-tail-cursor + write window. Two IronClaw processes pointing at the
same JSONL root will block on this lock and emit monotonically-sequenced
cursors instead of corrupting the stream with duplicates. Readers take a
shared lock to avoid observing partially-written tail lines.

New deps on the event-store crate (postgres feature only):
tokio-postgres-rustls, rustls, rustls-native-certs, webpki-roots, url.
All four were already used elsewhere in the workspace; no new external
crates are introduced. File locking uses stdlib (Rust 1.89+ stable).

Regression tests added:
- jsonl_replay_advances_next_cursor_past_trailing_filtered_records
- jsonl_concurrent_appenders_emit_monotonic_cursors_through_file_lock
- jsonl_bounded_replay_does_not_parse_the_whole_file
- postgres_store::tests::{local_postgres_urls_are_recognised,
  remote_postgres_urls_require_tls,
  unparseable_postgres_url_with_scheme_falls_closed_to_remote}

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn_event_store): close TLS bypass for libpq keyword strings and http:// libsql in production

Two High-severity findings from PR nearai#3171 review:

1. is_local_postgres_url() previously returned true for any string without
   "://", so libpq keyword form like "host=db.example.com user=..." took
   the NoTls branch and connected over plaintext. Walk the keyword list,
   look at the actual host, and only treat localhost / socket paths as
   local. Tests cover remote keyword strings, socket paths, and missing
   host=.

2. Production profile previously accepted http:// libSQL URLs and forwarded
   the auth token in cleartext. Reject http:// for RebornProfile::Production
   with a typed error before the build call. LocalDev / Test still accept
   http:// for running against local sqld instances. Tests cover both.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn_event_store): use deadpool-postgres pool for transparent reconnection

Replaces the single Arc<Client> in PostgresStore with a deadpool_postgres::Pool.
The previous design built one tokio_postgres::Client at startup; if the
underlying connection task exited (idle timeout, server restart, failover,
transient network drop), all subsequent append/read operations would fail
forever because nothing rebuilt the connection.

deadpool's Manager replaces broken Clients on the next pool.get() call, so
each call site sees a live connection without per-site reconnect logic.
This mirrors v1's connection-management story (src/db/postgres.rs,
src/db/tls.rs both use deadpool-postgres) and avoids two divergent
reconnection paths in one binary.

Addresses serrrfirat's Medium-severity finding on PR nearai#3171.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(tests): fix post-merge clippy warning

* fix(reborn_event_store): close TLS-mode + libpq-config + JSONL fsync gaps

Three follow-ups from PR nearai#3171 round-3 review:

1. CRITICAL — remote Postgres TLS not actually enforced. Passing a rustls
   connector is not enough on its own: tokio-postgres only consults the
   connector when Config::ssl_mode is Prefer or Require, and `sslmode=disable`
   in the connection string returns a plaintext stream before TLS is
   attempted. Add `enforce_remote_ssl_mode`: reject Disable for non-local
   configs, and force Prefer (the default) up to Require so the server
   cannot decline TLS without failing the connection.

2. HIGH — local/remote detector missed valid libpq forms. Re-parsing the
   raw connection string failed on `hostaddr=10.0.0.5` (numeric-IP keyword,
   no `host=` entry), `postgresql:///db?host=db.example.com` (URL with
   empty authority + host in query), and `host=/var/run/postgresql,
   db.example.com` (mixed socket/TCP list). Replace with
   `is_local_postgres_config` that walks the parsed `Config::get_hosts()`
   and `Config::get_hostaddrs()` so all libpq normalisations land in the
   same code path.

3. MEDIUM — JSONL append fsynced file contents but not the parent
   directory entry. On POSIX the new file's directory entry must also be
   fsynced for crash durability; without it the first append can vanish
   after a power loss even though `append()` returned success. Detect
   first-create via `path.exists()` pre-open and fsync the parent dir
   after `sync_data()`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(reborn_event_store): address PR nearai#3171 round-3 review findings

Closes the remaining libSQL/Postgres/JSONL gaps surfaced in the latest
serrrfirat review pass on PR nearai#3171.

libSQL production gate (#34, #36, #41):
- Reject `:memory:` for Production via the InMemory error variant.
- Case-insensitive scheme matching for `HTTP://` / `HTTPS://` /
  `LibSQL://`, so a mixed-case URL no longer falls through to
  Builder::new_local and silently creates a node-local SQLite path.
- Reject bare hostname-like values (e.g. `db.example.com`) for
  Production via a new ProductionLibsqlAmbiguousTarget error; LocalDev
  still accepts ergonomic forms like `events.db`.

libSQL backend hardening (#45, #49, #50):
- Skip `create_dir_all("")` for `events.db` in cwd.
- Enable `journal_mode=WAL` + `synchronous=NORMAL` once at build time
  for file-backed local stores so a long replay reader can no longer
  block writer commits past `busy_timeout`.
- Mirror the v1 retry pattern (`src/db/libsql/mod.rs::connect`): three
  attempts with exponential backoff so concurrent transient
  "unable to open database file" errors do not surface as durable-log
  failures.

Postgres backend hardening (#46):
- Honor `sslmode=require` for loopback configs so a TLS-only local
  Postgres / loopback proxy is accepted instead of forced to NoTls.

JSONL hardening (#38, #40, #42, #43, #44):
- Make JSONL constructors crate-private so production composition
  cannot bypass the single-node-durable acceptance gate.
- Hash path components (SHA-256 / 16-hex-char prefix + 32-byte URL-
  encoded hint) so case-distinct IDs (`Alice` vs `alice`) cannot
  collide on case-insensitive filesystems and 256-byte scope IDs no
  longer overflow the 255-byte filename limit.
- Snapshot file length before write/flush/sync; truncate back on any
  error so a partial write never leaves a torn JSON tail that wedges
  every subsequent append.
- Create directories with `0o700` and stream files with `0o600` on
  Unix so durable history is not world-readable under the typical
  `umask 022`.

SQL replay correctness (#37):
- After fetching filtered rows, run a small unfiltered COUNT over the
  scanned cursor window in both libSQL and Postgres backends; mismatch
  surfaces a missing entry row as `EventError::ReplayGap`. JSONL
  already detects this via line-by-line cursor sequencing.

Regression tests:
- libSQL/Postgres production gate (case-insensitive scheme, in-memory,
  bare hostname).
- Hashed-path-component case distinctness and length boundedness.
- Atomic JSONL append: failed serialise leaves file at pre-append
  length and the next append still advances cursor cleanly.
- Unix `0o700` JSONL root permissions.
- libpq quoted single-quote socket path is local; whitespace-around-
  `=` keyword strings classify remote correctly.
- libSQL replay surfaces a deleted entry row as `ReplayGap`.

Deferred with explicit acknowledgement:
- #39 (`ReadScope.invocation_id`) — module-doc note; needs a
  cross-crate change to `ironclaw_events` plus every replay caller.
- #48 (replay holds writer-blocking locks) — inline note at the JSONL
  read path; needs a stream-bytes-snapshot redesign coordinated with
  the durable-log contract.

Test plan
- cargo test -p ironclaw_reborn_event_store
- cargo test -p ironclaw_reborn_event_store --features "libsql postgres"
- cargo test -p ironclaw_host_runtime
- cargo test -p ironclaw_events
- cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold
- cargo clippy -p ironclaw_reborn_event_store --all-targets -- -D warnings
- cargo clippy -p ironclaw_reborn_event_store --features "libsql postgres" --all-targets -- -D warnings
- cargo clippy -p ironclaw_reborn_event_store --no-default-features --all-targets -- -D warnings
- cargo clippy -p ironclaw_host_runtime --all-targets -- -D warnings
- cargo fmt --all -- --check

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: serrrfirat <f@nuff.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant