Repository navigation
Conversation
…sures Add a `TransactionFn` abstraction so `with_transaction` and `with_retry_transaction` can accept async closures that borrow the transaction for the lifetime of the returned future.
Rewrite `SinexEnvironment::database_url()` on top of `url::Url` so it handles `dbname` / `database` query parameters, rejects empty names, and avoids double-namespacing path-based URLs. Cache the analytics epoch start instead of relying on a const `DateTime`.
Parse trusted extension entries from `SINEX_NATIVE_MESSAGING_TRUSTED_EXTENSIONS`, optionally validate per-extension secrets, and emit structured `native_messaging.auth` logs for each allow or deny decision.
Reject oversized base64 payloads using `SINEX_GATEWAY_MAX_BLOB_BYTES` before decoding or storing content. Add quota-focused gateway tests and document the new limit alongside the RPC token requirements.
Switch document ingestion to metadata checks plus a small encoding sniff window, and emit `document.ingested` events from the file size rather than an eagerly-read byte buffer.
Introduce workflow and script helpers that regenerate schema artifacts and `.sqlx` data with the project environment, making stale generated state easier to update and validate.
Extend `combine-files-batch.sh` so it can emit a combined `tokei + git log` report and split the full patch history into fixed-size markdown parts when run inside a git repository.
Add `GatewayAuth` to the JSON-RPC server, require `SINEX_RPC_TOKEN` or `SINEX_RPC_TOKEN_FILE` unless insecure mode is explicitly enabled, and teach the CLI client to send the matching headers. The same patch improves DB-unavailable guidance and adds confirmation-tail plus native-messaging auth coverage.
Switch repository trait signatures from raw `sqlx::Transaction<Postgres>` types to the shared `DbTransaction` alias so transaction-bearing APIs line up with the rest of `sinex-core`.
- Refresh the canonical docs after the gateway auth, backlog, and operator guidance changes. - Make the new token and boundary assumptions discoverable from the main docs.
Allow `BlobManager` to run without an event sender, skipping blob/material notifications when the caller only needs git-annex storage. Update stage-as-you-go, clipboard, and tests to pass `Some(...)` explicitly.
Introduce the macro-driven property-test harness, migrate the property suites and docs to it, and add structured runlog summaries for test executions.
Introduce `WatcherHandle` state, stream/forwarder helpers, and a bootstrap event for the unified system processor. Start continuous monitoring by wiring real D-Bus, journal, udev, and systemd streams or simulated fallbacks when a watcher fails to start.
Extract the Hyprland reconnection delay calculation into helper functions and guard the jitter path against system clock skew instead of unwrapping `duration_since`.
…ed serialization error propagation Eliminate panic-prone `serde_json::to_value(...).unwrap()` usage in PKM material-summary conversion by routing both call sites through a new helper that returns `ServiceResult<Value>`. The helper maps serde failures to `SinexError::serialization` and preserves source context text, so malformed summary serialization now surfaces as a normal service error instead of aborting execution. This closes one of the remaining typed-error gaps in the service layer.
…ing()` to `Into<String>` Update `SinexEnvironment` database URL rewriting to return strings via `url.into()` rather than deprecated `Url::into_string()`. The behavior stays the same, but the namespaced DB URL helper no longer relies on deprecated API surface, keeping warning output clean in the core environment path. This is a focused maintenance fix in the middle of a broader cleanup band.
…ects Replace the timestamp-based retry math with a reusable `tokio-retry` exponential backoff sequence and add tests that cover saturation and reset behavior.
…ture closures Drop the `TransactionFn` abstraction and move `with_transaction` / `with_retry_transaction` back to `BoxFuture`-based closure signatures. Update the docs and smoke tests so callers can pass async closures without lifetime inference failures.
Revert `3e00524e` after the initial macro and logging sweep destabilized the test harness. This temporarily restores the older property-testing path until the follow-up fixes can land cleanly.
- Repair the gateway and SDK callers that were destabilized by the first property-harness rollout. - Restore a reliable test baseline before continuing with the gateway hardening work.
Skip `nixos/modules/secrets.nix` in the VM smoke-test base module so the harness can evaluate without importing the full agenix stack. Restore the guarded `ageAvailable` logic for normal module consumers.
…crets module Simplify `nixos/modules/secrets.nix` by deleting the `ageAvailable` probing/warning branch and gating the entire `age.*` configuration block with `mkIf (cfg.secrets.enableAgenix or false)`. This prevents recursive option-availability checks while still populating identities, secret paths, and export script defaults when agenix is explicitly enabled. The change stabilizes evaluation behavior in environments where agenix is intentionally absent.
…side cursor scope Repair `cli/exo.py` by moving the dynamic SQL assembly in `dlq_list()` back under the cursor block, including initialization of `query_parts`, `conditions`, and `params`, plus unresolved-only condition handling. The broken indentation path could bypass intended filter construction or execute with malformed state. This commit restores correct query-building control flow and pairs with the immediately following DLQ stats indentation correction.
Move the `dlq_stats()` query-building block back under the cursor context so the aggregate queries actually execute inside the database connection.
Remove the `sinex-cli --help` smoke check and mark the flake package `doCheck = false` so the Python CLI derivation stops failing during Nix builds.
Sinity
added
area:core
Sinity
added
area:tests
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
RPC Authentication & Token Propagation cli/rpc_client.py, cli/exo.py, crate/core/sinex-gateway/src/rpc_server.rs, crate/core/sinex-gateway/src/handlers.rs, crate/core/sinex-gateway/src/native_messaging.rs, .env.example |
Implements bearer token authentication via SINEX_RPC_TOKEN/SINEX_RPC_TOKEN_FILE with constant-time comparison, CLI token header injection (--rpc-token), and per-extension secret validation in native messaging; returns 401 on auth failure. |
CLI Command Expansions cli/exo.py |
Adds new confirmations tail command and improves error handling for DLQ operations with graceful database-unavailable fallback patterns; propagates RPC token through query paths. |
Gateway Blob Size Limits & Optional Event Sinking crate/core/sinex-gateway/src/handlers.rs, crate/lib/sinex-satellite-sdk/src/annex/blob_manager.rs, crate/satellites/sinex-document-ingestor/src/lib.rs, crate/satellites/sinex-desktop-satellite/src/clipboard.rs, crate/core/sinex-gateway/tests/blob_*.rs |
Enforces SINEX_GATEWAY_MAX_BLOB_BYTES quota (default 5 MiB) with pre/post-decode validation; makes BlobManager event channel optional via Option<T> with conditional event emission and debug logging on skip. |
Database & Transaction Infrastructure crate/lib/sinex-core/src/db/query_helpers.rs, crate/lib/sinex-core/src/db/repositories/common.rs, crate/lib/sinex-core/src/environment.rs, crate/lib/sinex-core/tests/environment.rs, crate/lib/sinex-core/doc/query_helpers.md |
Refactors transaction closures to accept BoxFuture with lifetime bounds; restructures database_url parsing to use Url type with query-parameter and path-segment rewriting; adds comprehensive namespacing tests. |
Test Infrastructure & Feature Gating crate/lib/sinex-test-utils/*, crate/lib/sinex-schema/tests/ulid_property_tests.rs, crate/lib/sinex-core/tests/query_helpers_test.rs |
Adds internal-tests feature flag to gate self-tests; narrows test module compilation to #[cfg(all(test, feature = "internal-tests"))]; adds integration tests for transaction/async-closure patterns; introduces sinex_proptest! macro documentation updates. |
Automated CI Workflows .github/workflows/schema-auto-update.yml, .github/workflows/sqlx-auto-update.yml, .github/workflows/sqlx-cache.yml, .github/workflows/README.md |
Adds two daily-triggered auto-PR workflows for schema and SQLx cache regeneration with peter-evans/create-pull-request@v5; upgrades database provisioning to create dedicated user/role with TimescaleDB extension setup. |
System Monitoring Architecture crate/satellites/sinex-system-satellite/src/unified_processor.rs |
Replaces per-watcher fields with generic WatcherHandle; active watcher spawning via start_*_stream methods with fallback synthetic event emission; centralizes bootstrap ULID via SYSTEM_WATCHER_BOOTSTRAP_BYTES. |
Desktop Satellite Improvements crate/satellites/sinex-desktop-satellite/src/window_manager.rs |
Replaces manual exponential backoff with tokio_retry::ExponentialBackoff strategy; adds constants, helper methods, and unit tests for monotonic backoff growth and iterator reset behavior. |
Document Ingestion & Analytics crate/satellites/sinex-document-ingestor/src/lib.rs, crate/lib/sinex-services/src/analytics.rs, crate/lib/sinex-services/src/pkm.rs |
Streams file encoding sniffing (UTF-8 validation on first 4096 bytes) instead of full-file read; replaces compile-time EPOCH_START constant with OnceCell runtime caching; centralizes JSON serialization error handling via material_summary_to_json. |
Scripts & Nix Configuration scripts/combine-files-batch.sh, scripts/dev-env-banner.sh, scripts/schema-dev.sh, flake.nix, tests/e2e/nixos-vm/common/test-base.nix |
Adds Git-conditional bundle generation (tokei_plus_gitlog.md, 8-part diff history) and metrics/logging output hints in dev banner; disables ajv pre-flight check and secrets module in VM tests; removes sinex-cli Nix build smoke test. |
Configuration & Dependencies crate/core/sinex-gateway/Cargo.toml, crate/lib/sinex-services/Cargo.toml, crate/satellites/sinex-desktop-satellite/Cargo.toml |
Adds once_cell = "1", tokio-retry dependencies; updates main gateway startup to accept optional TCP listen address via --tcp-listen/SINEX_GATEWAY_TCP_LISTEN. |
Comprehensive Documentation Updates docs/*, docs/architecture/*, docs/vision/*, docs/roadmap/*, docs/test-suite-report/*, README.md, docs/TODO.md, docs/way_2.md |
Adds explicit "Purpose" blockquotes to canonical docs; updates RPC, blob, and auth documentation with token/quota details; migrates references from plan_v3.txt/TARGET_final.md to way.md; marks completed initiatives and gates tests on sinex_proptest! macro; refreshes ANALYSIS_INDEX and project timeline markers. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
🐰 With tokens and tales of tales retold,
The gateway guards secrets untold,
Blobs skip the dance when silence is gold,
Workflows craft schemas and queries bold—
Our burrow grows stronger, more carefully controlled! 🔐✨
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The pull request title follows the Conventional Commits format (feat: subject) and accurately describes the main changes: hardening gateway authentication/upload flow and improving CLI packaging. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/feat/gateway-auth-upload-hardening
Comment @coderabbitai help to get the list of available commands and usage tips.
Sinity
added a commit
that referenced
this pull request
Apr 9, 2026
Harden the gateway ingress path by requiring authenticated RPC traffic, validating native-messaging callers, and bounding blob ingestion before bytes reach git-annex, then thread the same contracts through the CLI, SDK, and document ingestor. sinex-core: - Rewrite `crate/lib/sinex-core/src/environment.rs::SinexEnvironment::database_url()` around parsed `Url` handling so environment namespacing works for path-based URLs as well as `dbname`/`database` query parameters, and add targeted tests for empty/duplicate namespace cases. - Replace a few small panic-prone or static-initialization paths, such as the analytics epoch helper, with explicit runtime initialization so the surrounding gateway/CLI hardening does not keep incidental footguns alive. sinex-gateway: - Add real RPC authentication in `crate/core/sinex-gateway/src/rpc_server.rs`: the server now loads `SINEX_RPC_TOKEN`/`SINEX_RPC_TOKEN_FILE`, accepts `Authorization: Bearer` or `X-Sinex-Rpc-Token`, and returns explicit unauthorized JSON-RPC errors when the token is missing or invalid. - Tighten bind-address handling so TCP exposure is opt-in instead of an accidental development default, which makes the token requirement meaningful at the transport boundary. - Extend `crate/core/sinex-gateway/src/native_messaging.rs` with trusted-extension parsing, optional shared secrets, and structured `native_messaging.auth` logging so browser callers are checked against an allow-list instead of a TODO comment. - Bound blob uploads in `crate/core/sinex-gateway/src/handlers.rs` before and after base64 decoding using `SINEX_GATEWAY_MAX_BLOB_BYTES`, which rejects oversized payloads before annex writes and keeps the gateway from acting like an unbounded blob proxy. CLI, SDK, and ingestion path: - Thread RPC tokens through `cli/exo.py` and `cli/rpc_client.py`, adding `--rpc-token`, global token propagation, and better “database unavailable” reporting so the CLI can follow the gateway’s auth contract without making operators rediscover it manually. - Change `crate/lib/sinex-satellite-sdk/src/annex/blob_manager.rs::BlobManager::new()` to accept an optional event sender, which lets gateway content paths avoid writing events directly while Stage-as-you-go and clipboard paths still emit blob events when they should. - Rework `crate/satellites/sinex-document-ingestor/src/lib.rs` to stop buffering entire files before blob ingestion: it now enforces size limits from metadata, sniffs only a small prefix for encoding, and records the file size/encoding without loading the full document into memory. packaging, docs, and tests: - Update README and gateway architecture docs with the new RPC token, blob quota, and trusted-extension contracts so the supported local/deployment path is explicit. - Add regression coverage for gateway auth headers, blob quota enforcement, no-direct-event blob storage, native-messaging trust checks, and environment URL namespacing, which turns several previously documented gaps into enforced boundary checks. - Disable the `sinex-cli` Nix package check phase after the gateway/CLI auth work, avoiding a brittle build-time import check that was not exercising the real runtime contract.
There was a problem hiding this comment.
Pull request overview
Hardens the gateway ingress surface (RPC + native messaging + blob upload) by making authentication and resource limits fail-closed, then propagates the new auth contract through the CLI, docs, and supporting tooling so the system remains operable under the tightened defaults.
Changes:
- Enforce RPC token authentication + explicit TCP opt-in, add/adjust tests and docs for the new gateway contract.
- Add native-messaging trusted-extension allow-list with optional per-extension secrets + audit logging.
- Bound blob uploads before/after base64 decode and decouple gateway content storage from direct event emission; improve CLI packaging/workflows and ingestion memory behavior.
Reviewed changes
Copilot reviewed 64 out of 65 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/nixos-vm/common/test-base.nix | Disable secrets module in VM tests |
| scripts/schema-dev.sh | Schema validation script adjustments |
| scripts/dev-env-banner.sh | Banner shows test logging/metrics hints |
| scripts/combine-files-batch.sh | Add tokei/gitlog + diff shard outputs |
| README.md | Document new RPC token contract + helpers |
| flake.nix | Disable CLI checkPhase |
| docs/way_2.md | Update task status / planning notes |
| docs/vision/streaming-architecture.md | Fix legacy references + refreshed notice |
| docs/vision/project-target-state.md | Clarify purpose + historical context |
| docs/vision/manifesto.md | Add explicit purpose statement |
| docs/TODO.md | Mark hardening items complete + add guarantees |
| docs/test-suite-report/TEST_INFRASTRUCTURE_ANALYSIS.md | Update property-test macro docs |
| docs/test-suite-report/TEST_FILE_INVENTORY.md | Update property-based coverage description |
| docs/test-suite-report/TEST_ANALYSIS_README.md | Update macro inventory |
| docs/roadmap/FUTURE_EVENT_PIPELINE_DESIGN.md | Fix legacy doc reference |
| docs/README.md | Add purpose line |
| docs/architecture/UserInteraction_And_Query_Architecture.md | Document auth + transport limits |
| docs/architecture/SystemOperations_And_Integrity_Architecture.md | Add purpose line |
| docs/architecture/security-architecture.md | Document RPC auth requirement |
| docs/architecture/provenance.md | Add purpose line |
| docs/architecture/event-taxonomy.md | Consolidate purpose header |
| docs/architecture/Core_Architecture.md | Fix legacy doc reference |
| docs/ANALYSIS_INDEX.md | Update analysis status + dates |
| crate/satellites/sinex-system-satellite/src/unified_processor.rs | Watcher task management + forwarders |
| crate/satellites/sinex-document-ingestor/src/lib.rs | Avoid full-file buffering; sniff encoding |
| crate/satellites/sinex-desktop-satellite/src/window_manager.rs | Backoff strategy via tokio-retry + tests |
| crate/satellites/sinex-desktop-satellite/src/clipboard.rs | Update BlobManager ctor signature |
| crate/satellites/sinex-desktop-satellite/Cargo.toml | Add tokio-retry dependency |
| crate/lib/sinex-test-utils/src/lib.rs | Macro crate self-alias + gate internal tests |
| crate/lib/sinex-test-utils/src/channel_behavior_utils.rs | Gate internal tests |
| crate/lib/sinex-test-utils/README.md | Document logging/metrics env vars |
| crate/lib/sinex-test-utils/Cargo.toml | Add internal-tests feature |
| crate/lib/sinex-services/src/pkm.rs | Remove unwrap; return typed serialization errors |
| crate/lib/sinex-services/src/analytics.rs | Runtime-cached epoch start |
| crate/lib/sinex-services/Cargo.toml | Add once_cell dependency |
| crate/lib/sinex-schema/tests/ulid_property_tests.rs | Property test import update |
| crate/lib/sinex-satellite-sdk/tests/integration/blob_manager_test.rs | Update BlobManager ctor signature |
| crate/lib/sinex-satellite-sdk/src/stage_as_you_go.rs | Update BlobManager ctor signature |
| crate/lib/sinex-satellite-sdk/src/annex/blob_manager.rs | Optional event sink for BlobManager |
| crate/lib/sinex-core/tests/query_helpers_test.rs | New tests for async tx closures |
| crate/lib/sinex-core/tests/environment.rs | Add dbname/database URL namespacing tests |
| crate/lib/sinex-core/src/environment.rs | Robust URL parsing + query-param dbname support |
| crate/lib/sinex-core/src/db/repositories/common.rs | Use DbTransaction alias |
| crate/lib/sinex-core/src/db/query_helpers.rs | BoxFuture-based tx helper APIs |
| crate/lib/sinex-core/doc/query_helpers.md | Update examples for Box::pin |
| crate/core/sinex-gateway/tests/native_messaging_auth_test.rs | Add allow-list secret tests |
| crate/core/sinex-gateway/tests/blob_route_security_test.rs | Add quota/auth regressions + no-events assertion |
| crate/core/sinex-gateway/tests/blob_event_forwarding_test.rs | Update test expectation (no event persistence) |
| crate/core/sinex-gateway/src/service_container.rs | Stop draining blob events; disable emission |
| crate/core/sinex-gateway/src/rpc_server.rs | Token auth + TCP opt-in + related tests |
| crate/core/sinex-gateway/src/native_messaging.rs | Trusted extension allow-list + secret validation |
| crate/core/sinex-gateway/src/main.rs | Add --tcp-listen option |
| crate/core/sinex-gateway/src/handlers.rs | Enforce blob size limits pre/post decode |
| crate/core/sinex-gateway/doc/rpc_server.md | Document auth + blob quotas |
| crate/core/sinex-gateway/doc/overview.md | Update example run signature |
| crate/core/sinex-gateway/doc/native_messaging.md | Document trusted extension contract |
| crate/core/sinex-gateway/Cargo.toml | Add once_cell dependency |
| cli/rpc_client.py | Add token headers support |
| cli/exo.py | Thread token + add CLI groups; DB fallback messaging |
| Cargo.lock | Lockfile updates for new deps |
| .github/workflows/sqlx-cache.yml | Update DB setup steps |
| .github/workflows/sqlx-auto-update.yml | New scheduled SQLx cache updater |
| .github/workflows/schema-auto-update.yml | New scheduled schema regen PR workflow |
| .github/workflows/README.md | Document new automation workflows |
| .env.example | Document RPC + test harness env vars |
Comments suppressed due to low confidence (2)
cli/exo.py:2270
- In
dlq_stats, thewith conn.cursor() as cur:block ends right aftertotals = cur.fetchone(), but subsequent queries still usecur(by-agent/by-category/daily-trends). This will fail at runtime; keep the remainingcur.execute(...)calls inside the cursor context (or open a new cursor).
with get_db_connection() as conn:
with conn.cursor() as cur:
# Build base query conditions
where_clause = "WHERE failed_at > now() - interval '%s days'"
params = [days]
if agent:
where_clause += " AND automaton_name = %s"
params.append(agent)
# Total DLQ entries
cur.execute(f"""
SELECT
COUNT(*) as total_dlq,
COUNT(*) FILTER (WHERE resolved_at IS NULL) as pending,
COUNT(*) FILTER (WHERE resolved_at IS NOT NULL) as resolved,
COUNT(*) FILTER (WHERE error_category = 'retryable') as retryable,
COUNT(*) FILTER (WHERE error_category = 'permanent') as permanent,
COUNT(*) FILTER (WHERE error_category = 'system') as system,
COUNT(*) FILTER (WHERE error_category = 'user') as user
FROM sinex_schemas.dlq_events
{where_clause}
""", params)
totals = cur.fetchone()
# DLQ by agent
cur.execute(f"""
SELECT
automaton_name,
COUNT(*) as total,
COUNT(*) FILTER (WHERE resolved_at IS NULL) as pending,
scripts/schema-dev.sh:91
cmd_validateincrementscount/errorsinside awhileloop that runs in a subshell due to the pipe (find ... | while ...). As a result,countanderrorswill remain 0 after the loop and the script may report success even when validations fail. Use process substitution (while ...; do ...; done < <(find ...)) or enablelastpipeso the counters update in the parent shell.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1939
to
+1943
| conditions.append("resolved_at IS NULL") | ||
|
|
||
| if agent: | ||
| if agent: | ||
| conditions.append("automaton_name = %s") | ||
| params.append(agent) | ||
|
|
||
| params.append(agent) |
There was a problem hiding this comment.
dlq_list has broken indentation around the if agent: block (the following conditions.append(...) line is not indented). As written this is a Python IndentationError and the CLI won’t start. Re-indent the block and keep the query-building/execution within the with conn.cursor() as cur: scope.
Comment on lines
1945
to
1965
| @@ -1933,7 +1963,10 @@ def dlq_list(agent: Optional[str], source: Optional[str], event_type: Optional[s | |||
| query_sql = " ".join(query_parts) | |||
| cur.execute(query_sql, params) | |||
| dlq_entries = cur.fetchall() | |||
There was a problem hiding this comment.
In dlq_list, multiple if ...: condition blocks and the cur.execute(...) call are dedented outside the with conn.cursor() as cur: block, but still reference cur. That will either run with a closed cursor or raise at runtime; keep all cursor usage inside the cursor context (or create a new cursor for the later queries).
Comment on lines
+412
to
+420
| fn blob_size_limit_bytes() -> usize { | ||
| static LIMIT: OnceLock<usize> = OnceLock::new(); | ||
| *LIMIT.get_or_init(|| { | ||
| std::env::var("SINEX_GATEWAY_MAX_BLOB_BYTES") | ||
| .ok() | ||
| .and_then(|raw| raw.parse::<usize>().ok()) | ||
| .unwrap_or(5 * 1024 * 1024) | ||
| }) | ||
| } |
There was a problem hiding this comment.
blob_size_limit_bytes() caches SINEX_GATEWAY_MAX_BLOB_BYTES in a global OnceLock. This makes tests that set the env var (and any runtime reconfiguration) potentially flaky because the first call wins for the entire process. Prefer reading the env var each call, or gate the caching behind #[cfg(not(test))] / provide a test-only reset mechanism.
Comment on lines
+245
to
+254
| let message = match self { | ||
| AuthError::Missing => { | ||
| "Authentication required. Provide SINEX_RPC_TOKEN via Authorization header." | ||
| } | ||
| AuthError::Invalid => "Authentication failed: invalid token.", | ||
| }; | ||
|
|
||
| ( | ||
| StatusCode::UNAUTHORIZED, | ||
| Json(JsonRpcResponse::error(None, -32002, message.to_string())), |
There was a problem hiding this comment.
Auth failures return a JSON-RPC error with id: None even though the request has already been parsed and an id may be available. For JSON-RPC clients, omitting the id makes correlation harder; consider including the request id in the auth error response (and also update the message to mention X-Sinex-Rpc-Token / SINEX_RPC_TOKEN_FILE, which are also accepted).
Suggested change
| let message = match self { | |
| AuthError::Missing => { | |
| "Authentication required. Provide SINEX_RPC_TOKEN via Authorization header." | |
| } | |
| AuthError::Invalid => "Authentication failed: invalid token.", | |
| }; | |
| ( | |
| StatusCode::UNAUTHORIZED, | |
| Json(JsonRpcResponse::error(None, -32002, message.to_string())), | |
| self.into_response_with_id(None) | |
| } | |
| fn into_response_with_id(self, id: Option<Value>) -> (StatusCode, Json<JsonRpcResponse>) { | |
| let message = match self { | |
| AuthError::Missing => { | |
| "Authentication required. Provide SINEX_RPC_TOKEN via the Authorization or X-Sinex-Rpc-Token header, or configure SINEX_RPC_TOKEN_FILE." | |
| } | |
| AuthError::Invalid => "Authentication failed: invalid token.", | |
| }; | |
| ( | |
| StatusCode::UNAUTHORIZED, | |
| Json(JsonRpcResponse::error(id, -32002, message.to_string())), |
Comment on lines
+215
to
+216
| if let Some(rest) = trimmed.strip_prefix("Bearer ") { | ||
| return Some(rest.trim().to_string()); |
There was a problem hiding this comment.
extract_token only accepts an Authorization scheme with the exact prefix "Bearer ". Per RFC, auth scheme names are case-insensitive, and some clients send bearer. Consider normalizing/case-folding the prefix check so valid Bearer tokens aren’t rejected unnecessarily.
Suggested change
| if let Some(rest) = trimmed.strip_prefix("Bearer ") { | |
| return Some(rest.trim().to_string()); | |
| let mut parts = trimmed.splitn(2, char::is_whitespace); | |
| if let (Some(scheme), Some(rest)) = (parts.next(), parts.next()) { | |
| if scheme.eq_ignore_ascii_case("Bearer") { | |
| return Some(rest.trim().to_string()); | |
| } |
Comment on lines
+32
to
+40
| ### `schema-auto-update.yml` - Schema Drift Automation | ||
| - Runs `./scripts/schema-dev.sh generate` on pushes to `main` | ||
| - Opens an automated PR when `schemas/` differs from generated output | ||
| - Requires `contents` + `pull-requests` write permissions so `peter-evans/create-pull-request@v5` can push branches | ||
|
|
||
| ### `sqlx-auto-update.yml` - SQLx Cache Automation | ||
| - Runs `cargo sqlx prepare --workspace -- --all-targets` on pushes to `main` | ||
| - Opens an automated PR when `.sqlx/` changes | ||
| - Also pinned to `contents`/`pull-requests` write permissions for the auto-PR helper |
There was a problem hiding this comment.
.github/workflows/README.md says schema-auto-update.yml runs ./scripts/schema-dev.sh generate “on pushes to main”, but the actual workflow is scheduled/dispatch and runs cargo run ... sinex-schema ... generate. Please update this description to match the workflow triggers/command so operators aren’t misled.
Comment on lines
+52
to
+53
| psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB" | ||
| psql -h localhost -U postgres -c "CREATE DATABASE sinex_test OWNER sinex_test" |
There was a problem hiding this comment.
The database setup step tries to CREATE DATABASE sinex_test even though the service container already initializes POSTGRES_DB: sinex_test. This will fail with “database already exists” and break the workflow. Make the setup idempotent (check existence / use a DO block) or set the service DB to postgres and create sinex_test yourself.
Suggested change
| psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB" | |
| psql -h localhost -U postgres -c "CREATE DATABASE sinex_test OWNER sinex_test" | |
| psql -h localhost -U postgres -d postgres -c "DO \$\$ BEGIN IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = 'sinex_test') THEN CREATE ROLE sinex_test LOGIN PASSWORD 'test_password' CREATEDB; END IF; END \$\$;" | |
| psql -h localhost -U postgres -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = 'sinex_test'" | grep -q 1 || psql -h localhost -U postgres -d postgres -c "CREATE DATABASE sinex_test OWNER sinex_test" | |
| psql -h localhost -U postgres -d postgres -c "ALTER DATABASE sinex_test OWNER TO sinex_test" |
Comment on lines
+16
to
+55
| services: | ||
| postgres: | ||
| image: timescale/timescaledb:2.15.2-pg16 | ||
| env: | ||
| POSTGRES_USER: postgres | ||
| POSTGRES_PASSWORD: postgres | ||
| POSTGRES_DB: sinex_test | ||
| ports: | ||
| - 5432:5432 | ||
| options: >- | ||
| --health-cmd pg_isready | ||
| --health-interval 10s | ||
| --health-timeout 5s | ||
| --health-retries 5 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust toolchain | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| profile: minimal | ||
| toolchain: stable | ||
| override: true | ||
|
|
||
| - name: Install sqlx-cli | ||
| run: cargo install sqlx-cli --no-default-features --features native-tls,postgres | ||
|
|
||
| - name: Setup database | ||
| env: | ||
| DATABASE_URL: postgresql://postgres:postgres@localhost:5432/sinex_test | ||
| PGPASSWORD: postgres | ||
| run: | | ||
| sudo apt-get update && sudo apt-get install -y postgresql-client postgresql-contrib | ||
| psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB" | ||
| psql -h localhost -U postgres -c "CREATE DATABASE sinex_test OWNER sinex_test" | ||
| psql -h localhost -U postgres -d sinex_test -c "CREATE EXTENSION IF NOT EXISTS timescaledb" | ||
| cd crate/lib/sinex-schema | ||
| cargo run -- up | ||
|
|
There was a problem hiding this comment.
The workflow sets POSTGRES_DB: sinex_test for the service, then runs CREATE DATABASE sinex_test ..., which will fail if the DB already exists. Make the DB/user creation idempotent (or use a different initial POSTGRES_DB) so the scheduled auto-update job doesn’t fail immediately.
Comment on lines
+440
to
+454
| Err(err) => { | ||
| warn!( | ||
| error = %err, | ||
| "D-Bus watcher failed to start, falling back to simulated events" | ||
| ); | ||
| let payload = json!({ | ||
| "bus": self.config.dbus_buses, | ||
| "signal": "WatcherInitialized", | ||
| "sender": "org.sinex.system", | ||
| "timestamp": Utc::now().to_rfc3339(), | ||
| }); | ||
| let handle = | ||
| self.spawn_simulated_watcher("dbus-sim", "system.dbus.signal", payload)?; | ||
| self.dbus_watcher = Some(handle); | ||
| } |
There was a problem hiding this comment.
On watcher startup failure, the processor falls back to emitting simulated events (e.g. fake journal entries / udev adds). This can silently mask real operational failures and pollute downstream data with fabricated events. Prefer disabling the watcher / surfacing a health event (or gating simulation behind an explicit dev/test-only config flag).
Comment on lines
24
to
29
| mod ulid_property_tests { | ||
| use super::*; | ||
| use sinex_test_utils::sinex_proptest; | ||
|
|
||
| proptest! { | ||
| #[test] |
There was a problem hiding this comment.
sinex_test_utils::sinex_proptest is imported but the module still uses the proptest! macro directly. This is an unused import (and suggests the migration to sinex_proptest! wasn’t completed in this file). Either switch the block to the wrapper macro or drop the import.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
crate/satellites/sinex-system-satellite/src/unified_processor.rs (3)
823-829: 🛠️ Refactor suggestion | 🟠 MajorAlso use
bootstrap_event_id()here and prefer.expect()over.unwrap().This snapshot branch has the same inline ULID bytes issue, plus it uses
.unwrap()instead of.expect()with a descriptive message.♻️ Proposed fix
- let system_bootstrap_id = EventId::from_ulid( - Ulid::from_bytes([ - 0x01, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - ]) - .unwrap(), - ); + let system_bootstrap_id = bootstrap_event_id();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crate/satellites/sinex-system-satellite/src/unified_processor.rs` around lines 823 - 829, Replace the inline ULID byte construction for system_bootstrap_id with a call to the existing bootstrap_event_id() helper and change any .unwrap() usages to .expect(...) with a brief descriptive message; specifically, update the code that sets system_bootstrap_id (where EventId::from_ulid(...) and Ulid::from_bytes([...]).unwrap() are used) to call bootstrap_event_id() instead and ensure any fallible conversions use .expect("failed to construct bootstrap event id") (or similar) rather than .unwrap().
1219-1245: 🧹 Nitpick | 🔵 TrivialConsider adding tests for watcher streaming and fallback behavior.
The current test covers
initialize_watchers()but the new streaming functionality lacks test coverage:
start_*_streammethods and their fallback to simulated watchersWatcherStatetransitions (Initialized→Running/Simulated)spawn_forwarderbehavior in dry_run modeWould you like me to help draft additional test cases for the watcher streaming behavior, or open an issue to track this as follow-up work?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crate/satellites/sinex-system-satellite/src/unified_processor.rs` around lines 1219 - 1245, Test coverage is missing for watcher streaming and fallback behavior: add unit/integration tests that exercise SystemProcessor's start_*_stream methods (e.g., start_dbus_stream, start_journal_stream, start_udev_stream, start_systemd_stream), assert WatcherState transitions from Initialized to Running when real streams are available and to Simulated when they fall back, and verify spawn_forwarder behavior in dry_run mode (ensure forwarders are not spawned and simulated watchers are used); use SystemProcessor::with_config(enabled_config()) to set up, toggle conditions to force real vs simulated streams, and assert processor.<dbus_watcher|journal_watcher|udev_watcher|systemd_watcher> Option states and any side-effects expected from running streams.
661-667: 🛠️ Refactor suggestion | 🟠 MajorUse centralized
bootstrap_event_id()helper for consistency.This method still uses inline hardcoded ULID byte arrays in three places (lines 661-666, 686-691, 711-716), while the rest of the file uses the centralized
SYSTEM_WATCHER_BOOTSTRAP_BYTESconstant andbootstrap_event_id()helper.♻️ Proposed fix to use centralized helper
// Journal can provide historical entries if self.config.journal_enabled && emit_events { - let system_bootstrap_id = EventId::from_ulid( - Ulid::from_bytes([ - 0x01, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, - ]) - .expect("hardcoded ULID bytes should be valid"), - ); + let system_bootstrap_id = bootstrap_event_id(); let provenance = Provenance::from_synthesis_safe(system_bootstrap_id, vec![]);Apply the same change to the other two occurrences (systemd at ~line 686 and dbus/udev at ~line 711).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crate/satellites/sinex-system-satellite/src/unified_processor.rs` around lines 661 - 667, Replace the inline hardcoded ULID byte arrays used to construct EventId (e.g., the system_bootstrap_id assignment that calls EventId::from_ulid(Ulid::from_bytes([...]).expect(...))) with the centralized helper bootstrap_event_id() (or, if you prefer, derive from SYSTEM_WATCHER_BOOTSTRAP_BYTES via the existing helper) so the code becomes a direct use of bootstrap_event_id(); apply the same change to the two other similar occurrences for the systemd and dbus/udev bootstrap IDs to ensure consistency across unified_processor.rs.scripts/schema-dev.sh (1)
66-92: 🧹 Nitpick | 🔵 TrivialConsider restoring a pre-flight check for
ajvto improve local DX.The removal of the
command -v ajvcheck means developers withoutajvinstalled will see a raw command failure rather than a helpful error message. While CI installsajv-cli(per.github/workflows/schema-management.yml), local usage would benefit from explicit guidance.♻️ Proposed fix to add pre-flight ajv check
cmd_validate() { echo "🔍 Validating JSON schemas..." + if ! command -v ajv &> /dev/null; then + echo -e "${RED}❌ ajv-cli not found. Install with: npm install -g ajv-cli ajv-formats${NC}" + exit 1 + fi + local count=0 local errors=0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/schema-dev.sh` around lines 66 - 92, The cmd_validate function no longer checks for ajv, so add a pre-flight check inside cmd_validate (or at its start) that uses command -v ajv to detect if ajv is installed; if missing, print a clear instructive error (e.g., tell the user to npm i -g ajv-cli or run the project's dev setup) and exit with non-zero status so local runs fail fast and with helpful guidance.crate/satellites/sinex-document-ingestor/src/lib.rs (1)
239-244:⚠️ Potential issue | 🟠 MajorGuard
u64 -> i64conversion for provenance offset.Line 243 uses
size_bytes as i64; values abovei64::MAXwill wrap and can write invalid negative offsets into provenance.Suggested fix
async fn process_complete_document( &self, material_id: Ulid, size_bytes: u64, encoding: Option<String>, metadata: serde_json::Value, ) -> Result<Vec<Event<JsonValue>>> { let mut events = Vec::new(); + let size_bytes_i64 = i64::try_from(size_bytes) + .map_err(|_| eyre!("Document size exceeds i64 range: {}", size_bytes))?; @@ .with_provenance(sinex_core::db::models::event::Provenance::from_material( sinex_core::types::Id::from(material_id), 0, Some(0), - Some(size_bytes as i64), + Some(size_bytes_i64), ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crate/satellites/sinex-document-ingestor/src/lib.rs` around lines 239 - 244, The provenance offset conversion currently does an unchecked cast `size_bytes as i64` which can overflow for values > i64::MAX; before calling sinex_core::db::models::event::Provenance::from_material (the call that passes material_id and the offset), validate `size_bytes` using a checked conversion (e.g., try_into or compare to i64::MAX) and handle the out-of-range case (return an Err/result, log and bail, or clamp explicitly) instead of letting it wrap to a negative value; update the call site that builds the provenance (the .with_provenance(...) invocation) to pass the safely converted i64 only after this validation.crate/core/sinex-gateway/src/rpc_server.rs (1)
636-656:⚠️ Potential issue | 🟠 MajorTest
rpc_responses_include_request_id_headerwill fail—header not implemented.This test asserts that responses include an
x-request-idheader, but the router/handler code doesn't add this header. The test documents a desired feature (per TODO task 26 indocs/TODO.md) but will fail until the header is implemented.Either implement the header injection or mark this test as
#[ignore]with a comment referencing the tracking task.🐛 Proposed fix: ignore the test until implemented
#[sinex_test] + #[ignore = "TODO task 26: x-request-id header not yet implemented"] async fn rpc_responses_include_request_id_header() -> color_eyre::eyre::Result<()> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crate/core/sinex-gateway/src/rpc_server.rs` around lines 636 - 656, The test rpc_responses_include_request_id_header asserts responses include an "x-request-id" header but the router/handler doesn't inject it; either implement header injection in the response pipeline (where build_test_router/spawn_router construct handlers or the central request middleware that builds responses) to add an "x-request-id" header for every RPC response, or mark the test rpc_responses_include_request_id_header with #[ignore] and add a brief comment referencing TODO task 26 in docs/TODO.md so the failing spec is tracked until header support is implemented.cli/exo.py (1)
1924-1968:⚠️ Potential issue | 🔴 CriticalFix the broken indentation in
dlq_list.Line 1942 and subsequent conditional blocks are not properly nested. The
conditions.append("automaton_name = %s")afterif agent:(line 1941) has the same indentation as theifstatement itself rather than being indented as its body. Similarly, theif source:,if event_type:,if category:, andif conditions:blocks are dedented to the wrong level, causing a SyntaxError that prevents the file from parsing.🐛 Fix
if agent: - conditions.append("automaton_name = %s") + conditions.append("automaton_name = %s") params.append(agent) - if source: - conditions.append("source = %s") - params.append(source) + if source: + conditions.append("source = %s") + params.append(source) - if event_type: - conditions.append("event_type = %s") - params.append(event_type) + if event_type: + conditions.append("event_type = %s") + params.append(event_type) - if category: - conditions.append("error_category = %s") - params.append(category) + if category: + conditions.append("error_category = %s") + params.append(category) - if conditions: - query_parts.append("WHERE " + " AND ".join(conditions)) + if conditions: + query_parts.append("WHERE " + " AND ".join(conditions)) - query_parts.append("ORDER BY failed_at DESC") - query_parts.append(f"LIMIT {limit}") + query_parts.append("ORDER BY failed_at DESC") + query_parts.append(f"LIMIT {limit}") - query_sql = " ".join(query_parts) - cur.execute(query_sql, params) - dlq_entries = cur.fetchall() + query_sql = " ".join(query_parts) + cur.execute(query_sql, params) + dlq_entries = cur.fetchall()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/exo.py` around lines 1924 - 1968, The dlq_list block has incorrect indentation causing the if bodies to be outside their conditionals; inside the with get_db_connection()/with conn.cursor() scope, re-indent the agent, source, event_type, category and conditions checks so that lines like conditions.append("automaton_name = %s") and params.append(agent) are nested under their respective if statements (e.g. inside if agent:), and ensure the subsequent if conditions: / query_parts assembly (including ORDER BY and LIMIT {limit}) remain inside the same cursor block before cur.execute; adjust only indentation to place all params.append and conditions.append calls and the final cur.execute(cur_query, params) inside the with/try scope of dlq_list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/schema-auto-update.yml:
- Around line 31-38: Update the GitHub Actions step that creates the PR to use
the current major version of the action by replacing the action reference
peter-evans/create-pull-request@v5 with peter-evans/create-pull-request@v8 in
the "Create schema update PR" workflow step; after updating, run the workflow or
lint the YAML to ensure input names (commit-message, title, body, branch,
labels) remain compatible with v8 and adjust any renamed inputs per the action's
v8 docs if needed.
- Around line 19-24: Replace the deprecated GitHub Action step that currently
uses "actions-rs/toolchain@v1" (the "Install Rust toolchain" step) with the
maintained "dtolnay/rust-toolchain@v1" action; update the step's uses value to
"dtolnay/rust-toolchain@v1" and keep the toolchain configuration (e.g., set
toolchain: stable and override: true or equivalent inputs supported by
dtolnay/rust-toolchain) so the workflow installs the stable Rust toolchain via
the supported action.
In @.github/workflows/sqlx-auto-update.yml:
- Around line 50-52: The workflow currently runs psql commands that attempt to
create the database sinex_test even though the service container already sets
POSTGRES_DB=sinex_test; remove or skip the redundant CREATE DATABASE step (the
psql command that runs "CREATE DATABASE sinex_test OWNER sinex_test") or change
it to operate on a different database name so it doesn't fail, and ensure the
subsequent command that creates the extension uses the existing database (the
psql invocation that runs "CREATE EXTENSION IF NOT EXISTS timescaledb") against
that existing DB; update the shell steps so only the CREATE USER and the
extension creation run against sinex_test, or guard the CREATE DATABASE with a
conditional check to skip when the DB exists.
- Around line 34-39: Replace the deprecated GitHub Action used in the "Install
Rust toolchain" step: change uses: actions-rs/toolchain@v1 to uses:
dtolnay/rust-toolchain@v1 and update the inputs to match dtolnay/rust-toolchain
(keep toolchain: stable and override: true, remove or adjust the unsupported
profile input). Ensure the step retains the original name "Install Rust
toolchain" and validates the workflow syntax after the change.
- Around line 61-68: Update the GitHub Action usage from "uses:
peter-evans/create-pull-request@v5" to "uses:
peter-evans/create-pull-request@v7" and verify the existing inputs
(commit-message, title, body, branch, labels) remain valid for v7; adjust the
"labels" input to a YAML array if required by v7 (e.g., labels:
["automated-update"]) and re-run/validate the workflow to ensure no input names
changed between v5 and v7.
In @.github/workflows/sqlx-cache.yml:
- Around line 52-54: The workflow attempts to CREATE DATABASE sinex_test even
though the service already creates it and also uses DATABASE_URL values with the
default postgres:postgres credentials; update the job to either remove the
duplicate psql "CREATE DATABASE sinex_test" statement (recommended) or replace
it with a conditional creation approach (e.g., a safe createdb/PLpgSQL check),
and ensure all DATABASE_URL occurrences are updated to use
sinex_test:test_password (or remove the CREATE USER step if you intend to keep
using postgres:postgres) so the created user and DB are actually used; look for
the psql CREATE USER/CREATE DATABASE commands and the DATABASE_URL lines to make
these changes.
In `@cli/exo.py`:
- Around line 421-423: Remove the global export of the RPC token by deleting the
os.environ['SINEX_RPC_TOKEN'] assignment in the block that calls
set_global_rpc_token(rpc_token); keep using set_global_rpc_token(...) (and the
ctx.obj / _GLOBAL_RPC_TOKEN storage) for in-process clients, and instead update
any subprocess invocations that actually need the token to receive an explicit
env dict with 'SINEX_RPC_TOKEN' set (pass that env only to the specific
subprocess call sites such as the git/annex/satellite invocation functions),
ensuring unrelated subprocesses are not given the token.
In `@cli/rpc_client.py`:
- Around line 416-425: Update the function signatures to use explicit Optional
type hints: change create_client parameters to rpc_url: Optional[str] = None and
token: Optional[str] = None and annotate any similar helper (e.g.,
test_connection) to use Optional[str]; also add "from typing import Optional" at
the top if not present. Keep the return annotation as SinexRPCClient for
create_client and mirror the appropriate return type for test_connection.
In `@crate/core/sinex-gateway/Cargo.toml`:
- Line 52: Remove the unused once_cell dependency from the crate's Cargo.toml
(delete the once_cell = "1" entry) because the code uses std::sync::OnceLock for
the static LIMIT definition; ensure no other code imports once_cell and run
cargo check to verify builds; check references to LIMIT and the use of OnceLock
in handlers.rs to confirm the standard library type is used everywhere.
In `@crate/core/sinex-gateway/doc/native_messaging.md`:
- Around line 49-52: Documentation omits the auth reason "missing_secret" that
the native_messaging implementation emits; update
crate/core/sinex-gateway/doc/native_messaging.md to include "missing_secret"
alongside the listed reasons. Locate the section describing authentication
tracing events (`native_messaging.auth`) and the list of reasons
(`missing_extension_id`, `not_trusted`, `invalid_secret`) and add
`missing_secret` with a short description stating it is emitted when a trusted
extension entry requires a secret but an incoming message omits it; reference
the implementation symbol native_messaging.rs in your edit if helpful for
context.
In `@crate/core/sinex-gateway/doc/rpc_server.md`:
- Line 29: Update the doc text for blob uploads to state there are two enforced
checks: a pre-decode check on the base64-encoded payload length (rejecting
uploads whose encoded size exceeds the configured encoded-limit) and a
post-decode check that enforces the actual decoded byte quota via
SINEX_GATEWAY_MAX_BLOB_BYTES (default 5MB) before writing to git-annex; mention
both checks explicitly and where the configuration
(SINEX_GATEWAY_MAX_BLOB_BYTES) applies so readers understand pre- and
post-decode enforcement.
In `@crate/core/sinex-gateway/src/handlers.rs`:
- Around line 412-420: The blob_size_limit_bytes function uses a static OnceLock
LIMIT which caches the env-derived value for the process lifetime, so tests that
change SINEX_GATEWAY_MAX_BLOB_BYTES after the first call will not observe
changes; to fix, add a test-only mechanism to reset or bypass the cache: keep
the OnceLock LIMIT and function blob_size_limit_bytes as-is for production, but
add a cfg(test) helper like pub(crate) fn reset_blob_size_limit_for_tests() that
clears LIMIT (using OnceLock::take or replace the static with a OnceCell variant
if needed) or add an alternate test-only accessor that reads the env var each
call; reference the static LIMIT, the blob_size_limit_bytes function, and the
EnvVarGuard in tests so test setup calls the reset helper before invoking
blob_size_limit_bytes.
In `@crate/core/sinex-gateway/src/native_messaging.rs`:
- Around line 31-34: The env parsing currently turns malformed
SINEX_NATIVE_MESSAGING_TRUSTED_EXTENSIONS into an empty allow-list (via
parse_trusted_entries and unwrap_or_default) which weakens the trust boundary;
change parse_trusted_entries to return Result<Vec<TrustedEntry>, ParseError> and
validate entries so that malformed tokens (e.g., "#secret", ",", invalid
Unicode, "ext#") produce Err instead of mapping to secret: None, and then
propagate that error when initializing trusted_extensions (replace
unwrap_or_default with ?/map_err so startup fails if the env var is present but
invalid). Also update the authorization logic (the code that treats an empty
trusted_extensions as "allow all" on line ~76) so that empty means “no trusted
extensions” and only treat missing env var as default behavior; ensure entries
of the form "id#" are rejected rather than normalized to secret: None.
- Around line 106-135: The NativeMessage's extension_secret must be redacted
before any request logging to avoid credential leakage: locate where incoming
messages are received and logged (the debug!("Received message: {:?}", message)
call) and clear or mask message.extension_secret (or implement a redact method
on NativeMessage) immediately after parsing and before any debug/info logs; keep
the existing authentication checks that compare trusted.secret with
message.extension_secret (as shown by the trusted.secret and provided match) but
ensure you store the secret temporarily for comparison and then remove/mask it
from message prior to any further logging or handing the message downstream
(reference symbols: NativeMessage, message.extension_secret, trusted.secret,
debug!("Received message: {:?}", message), incoming_id).
In `@crate/lib/sinex-core/src/db/repositories/common.rs`:
- Line 74: Update the implementations of TransactionSupport::with_tx to match
the new trait signature that accepts &mut DbTransaction<'_> (DbTransaction is
the &mut sqlx::Transaction<'_, sqlx::Postgres> alias); specifically change the
method signatures in KnowledgeGraphRepository::with_tx and
SourceMaterialRepository::with_tx to accept &mut DbTransaction<'_> and adjust
any internal uses to treat the parameter as a mutable reference to the
transaction (propagate mutability where needed and remove the old expanded
sqlx::Transaction type if present) so they compile against the updated trait.
In `@crate/satellites/sinex-desktop-satellite/src/window_manager.rs`:
- Around line 600-647: Add a regression test that exercises
stream_hyprland_events to ensure a connect-then-immediate-disconnect (flapping
session) does not prematurely reset the hyprland_backoff iterator; specifically,
create a test that drives stream_hyprland_events (or the component that wraps
it) through a simulated connect event followed immediately by a disconnect and
then verifies that subsequent calls to WindowManagerWatcher::next_backoff(&mut
backoff) continue the exponential/backoff sequence (using hyprland_backoff and
next_backoff) rather than restarting it; reference WindowManagerWatcher,
stream_hyprland_events, hyprland_backoff, and next_backoff when locating where
to add the test and assert the backoff progression.
- Line 509: The code currently resets reconnect_backoff (created from
Self::hyprland_backoff()) as soon as a socket connects, which prevents
exponential backoff against peers that accept then immediately drop connections;
remove any immediate resets of reconnect_backoff (including the reassignment
near the initial accept and the resets around the accept/connect handling at the
spots mentioned) and instead reset it only after the session is proven
healthy—e.g., after successful handshake/authentication or after the connection
has survived any initial probe/keepalive logic in the session-management path
(the code that performs the session establishment and transitions the socket
into an active peer session); keep reconnect_backoff alive across short-lived
accepts so backoff increases on repeated rapid failures and only set
reconnect_backoff = Self::hyprland_backoff() when the session logic explicitly
marks the peer as successfully established.
In `@crate/satellites/sinex-system-satellite/src/unified_processor.rs`:
- Around line 575-586: The current spawn_dbus_task uses
mpsc::unbounded_channel() which can grow without bound; replace it with a
bounded channel (e.g., tokio::sync::mpsc::channel with a chosen capacity like
256) so the pipeline has backpressure. Update the local variables (tx, rx) to
use the bounded sender/receiver types and pass rx into
spawn_forwarder("system.dbus.signal", rx, emitter) and tx into
watcher.start_streaming(tx).await; if watcher.start_streaming expects an
UnboundedSender, adapt its signature or wrap/convert appropriately so it accepts
the bounded sender, and ensure the watcher handles SendError/closed-channel
cases (log and terminate) to avoid panics. Use a named constant for capacity
(e.g., DBUS_CHANNEL_CAPACITY) so it’s easy to tune.
- Around line 427-434: The repeated pattern checking whether a watcher is active
(e.g., the dbus_watcher check in start_*_stream methods using
.as_ref().map(|handle| handle.is_active()).unwrap_or(false)) should be
simplified for clarity and consistency; update each start_*_stream method to use
a clearer idiom such as .as_ref().map_or(false, |h| h.is_active()) or, on Rust
1.70+, .is_some_and(|h| h.is_active()) referencing the dbus_watcher symbol and
the is_active method so the early-return logic remains identical but the
condition is concise and uniform across all start_*_stream functions.
In `@docs/ANALYSIS_INDEX.md`:
- Around line 95-109: Update the unlabeled fenced code blocks in
ANALYSIS_INDEX.md to include a language specifier (e.g., "text") so linters
render them consistently; specifically modify the two blocks containing the
architecture lines "Satellites → sensd (gRPC) → ingestd → Postgres → NATS
JetStream → Automata" and "Satellites → NATS JetStream → ingestd → Postgres +
NATS consumers" to start with ```text instead of just ```, leaving the inner
arrow lines unchanged.
In `@docs/architecture/security-architecture.md`:
- Line 194: Update the Gateway RPC Authentication docs to list both accepted
HTTP headers: include that sinex-gateway accepts either Authorization: Bearer
<token> or the legacy X-Sinex-Rpc-Token header, and note that the CLI still
injects Authorization automatically when SINEX_RPC_TOKEN or --rpc-token is used;
keep SINEX_RPC_TOKEN_FILE and SINEX_GATEWAY_ALLOW_INSECURE descriptions
unchanged and ensure the text references sinex-gateway and the exact env var
names (SINEX_RPC_TOKEN, SINEX_RPC_TOKEN_FILE, SINEX_GATEWAY_ALLOW_INSECURE) so
clients know both header options are supported.
In `@docs/architecture/UserInteraction_And_Query_Architecture.md`:
- Line 50: The document has duplicated section numbers: rename the heading "###
2.3 Method Surface" to "### 2.4 Method Surface" and then rename the following
"### 2.4 Deployment Considerations" to "### 2.5 Deployment Considerations" so
numbering is sequential; search for the exact heading texts "### 2.3 Method
Surface" and "### 2.4 Deployment Considerations" and update them accordingly.
In `@docs/test-suite-report/TEST_FILE_INVENTORY.md`:
- Around line 365-366: The markdown heading "### Property-Based Coverage"
violates MD022 because it lacks blank lines around it; edit the
TEST_FILE_INVENTORY.md entry containing the heading "### Property-Based
Coverage" and add a single blank line above and a single blank line below the
heading so there is an empty line separating the heading from surrounding
content.
In `@docs/vision/streaming-architecture.md`:
- Around line 9-10: The Accuracy Notice line contains dates in reverse
chronological order ("2025-07-24, refreshed 2025-02-24"); update the string so
the "refreshed" date is later than the original (for example change the fragment
to "Accuracy Notice (2025-02-24, refreshed 2025-07-24)") by editing the exact
notice text shown (the line containing "**Accuracy Notice (2025-07-24, refreshed
2025-02-24)**") in the document.
In `@docs/way_2.md`:
- Around line 131-134: Add a blank line immediately after the second-level
headings in docs/way_2.md that the linter flagged (for example the "## Change
Control" heading and the other similar section headings flagged by the static
analysis tool) so there is an empty line between the heading and the following
paragraph; update each heading occurrence in the file to include a single blank
line after it to satisfy Markdown style guidelines.
In `@flake.nix`:
- Line 175: The change disabled in-derivation checks by setting doCheck = false
which removes smoke coverage for the manually assembled sinex-cli in
installPhase; restore in-derivation verification by either leaving doCheck
enabled or, if broader checks must remain off, add an install-time smoke test
inside installPhase that runs the assembled binary (e.g., invoke the sinex-cli
entrypoint with --version or --help) and fails the build on non-zero exit to
assert the wrapper/import path is correct; update the package expression
referencing doCheck and the installPhase/sinex-cli assembly (the installPhase
that creates sinex-cli) accordingly.
In `@scripts/dev-env-banner.sh`:
- Around line 62-64: The banner always shows "Perf metrics: writing ..." because
metrics_hint is set unconditionally; change the logic to only set
metrics_path/metrics_hint when SINEX_TEST_METRICS is truthy (check the
SINEX_TEST_METRICS env var) and otherwise set metrics_hint to a disabled/empty
string so the banner does not report metrics as active; update the assignments
that currently use SINEX_TEST_METRICS_PATH and metrics_hint (variables
metrics_path and metrics_hint) and the corresponding banner print site so both
occurrences (the initial block and the later duplicate) follow the same
conditional behavior.
---
Outside diff comments:
In `@cli/exo.py`:
- Around line 1924-1968: The dlq_list block has incorrect indentation causing
the if bodies to be outside their conditionals; inside the with
get_db_connection()/with conn.cursor() scope, re-indent the agent, source,
event_type, category and conditions checks so that lines like
conditions.append("automaton_name = %s") and params.append(agent) are nested
under their respective if statements (e.g. inside if agent:), and ensure the
subsequent if conditions: / query_parts assembly (including ORDER BY and LIMIT
{limit}) remain inside the same cursor block before cur.execute; adjust only
indentation to place all params.append and conditions.append calls and the final
cur.execute(cur_query, params) inside the with/try scope of dlq_list.
In `@crate/core/sinex-gateway/src/rpc_server.rs`:
- Around line 636-656: The test rpc_responses_include_request_id_header asserts
responses include an "x-request-id" header but the router/handler doesn't inject
it; either implement header injection in the response pipeline (where
build_test_router/spawn_router construct handlers or the central request
middleware that builds responses) to add an "x-request-id" header for every RPC
response, or mark the test rpc_responses_include_request_id_header with
#[ignore] and add a brief comment referencing TODO task 26 in docs/TODO.md so
the failing spec is tracked until header support is implemented.
In `@crate/satellites/sinex-document-ingestor/src/lib.rs`:
- Around line 239-244: The provenance offset conversion currently does an
unchecked cast `size_bytes as i64` which can overflow for values > i64::MAX;
before calling sinex_core::db::models::event::Provenance::from_material (the
call that passes material_id and the offset), validate `size_bytes` using a
checked conversion (e.g., try_into or compare to i64::MAX) and handle the
out-of-range case (return an Err/result, log and bail, or clamp explicitly)
instead of letting it wrap to a negative value; update the call site that builds
the provenance (the .with_provenance(...) invocation) to pass the safely
converted i64 only after this validation.
In `@crate/satellites/sinex-system-satellite/src/unified_processor.rs`:
- Around line 823-829: Replace the inline ULID byte construction for
system_bootstrap_id with a call to the existing bootstrap_event_id() helper and
change any .unwrap() usages to .expect(...) with a brief descriptive message;
specifically, update the code that sets system_bootstrap_id (where
EventId::from_ulid(...) and Ulid::from_bytes([...]).unwrap() are used) to call
bootstrap_event_id() instead and ensure any fallible conversions use
.expect("failed to construct bootstrap event id") (or similar) rather than
.unwrap().
- Around line 1219-1245: Test coverage is missing for watcher streaming and
fallback behavior: add unit/integration tests that exercise SystemProcessor's
start_*_stream methods (e.g., start_dbus_stream, start_journal_stream,
start_udev_stream, start_systemd_stream), assert WatcherState transitions from
Initialized to Running when real streams are available and to Simulated when
they fall back, and verify spawn_forwarder behavior in dry_run mode (ensure
forwarders are not spawned and simulated watchers are used); use
SystemProcessor::with_config(enabled_config()) to set up, toggle conditions to
force real vs simulated streams, and assert
processor.<dbus_watcher|journal_watcher|udev_watcher|systemd_watcher> Option
states and any side-effects expected from running streams.
- Around line 661-667: Replace the inline hardcoded ULID byte arrays used to
construct EventId (e.g., the system_bootstrap_id assignment that calls
EventId::from_ulid(Ulid::from_bytes([...]).expect(...))) with the centralized
helper bootstrap_event_id() (or, if you prefer, derive from
SYSTEM_WATCHER_BOOTSTRAP_BYTES via the existing helper) so the code becomes a
direct use of bootstrap_event_id(); apply the same change to the two other
similar occurrences for the systemd and dbus/udev bootstrap IDs to ensure
consistency across unified_processor.rs.
In `@scripts/schema-dev.sh`:
- Around line 66-92: The cmd_validate function no longer checks for ajv, so add
a pre-flight check inside cmd_validate (or at its start) that uses command -v
ajv to detect if ajv is installed; if missing, print a clear instructive error
(e.g., tell the user to npm i -g ajv-cli or run the project's dev setup) and
exit with non-zero status so local runs fail fast and with helpful guidance.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0ad8cb1a-ef19-4de0-918a-a06193a42eef
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (64)
.env.example.github/workflows/README.md.github/workflows/schema-auto-update.yml.github/workflows/sqlx-auto-update.yml.github/workflows/sqlx-cache.ymlREADME.mdcli/exo.pycli/rpc_client.pycrate/core/sinex-gateway/Cargo.tomlcrate/core/sinex-gateway/doc/native_messaging.mdcrate/core/sinex-gateway/doc/overview.mdcrate/core/sinex-gateway/doc/rpc_server.mdcrate/core/sinex-gateway/src/handlers.rscrate/core/sinex-gateway/src/main.rscrate/core/sinex-gateway/src/native_messaging.rscrate/core/sinex-gateway/src/rpc_server.rscrate/core/sinex-gateway/src/service_container.rscrate/core/sinex-gateway/tests/blob_event_forwarding_test.rscrate/core/sinex-gateway/tests/blob_route_security_test.rscrate/core/sinex-gateway/tests/native_messaging_auth_test.rscrate/lib/sinex-core/doc/query_helpers.mdcrate/lib/sinex-core/src/db/query_helpers.rscrate/lib/sinex-core/src/db/repositories/common.rscrate/lib/sinex-core/src/environment.rscrate/lib/sinex-core/tests/environment.rscrate/lib/sinex-core/tests/query_helpers_test.rscrate/lib/sinex-satellite-sdk/src/annex/blob_manager.rscrate/lib/sinex-satellite-sdk/src/stage_as_you_go.rscrate/lib/sinex-satellite-sdk/tests/integration/blob_manager_test.rscrate/lib/sinex-schema/tests/ulid_property_tests.rscrate/lib/sinex-services/Cargo.tomlcrate/lib/sinex-services/src/analytics.rscrate/lib/sinex-services/src/pkm.rscrate/lib/sinex-test-utils/Cargo.tomlcrate/lib/sinex-test-utils/README.mdcrate/lib/sinex-test-utils/src/channel_behavior_utils.rscrate/lib/sinex-test-utils/src/lib.rscrate/satellites/sinex-desktop-satellite/Cargo.tomlcrate/satellites/sinex-desktop-satellite/src/clipboard.rscrate/satellites/sinex-desktop-satellite/src/window_manager.rscrate/satellites/sinex-document-ingestor/src/lib.rscrate/satellites/sinex-system-satellite/src/unified_processor.rsdocs/ANALYSIS_INDEX.mddocs/README.mddocs/TODO.mddocs/architecture/Core_Architecture.mddocs/architecture/SystemOperations_And_Integrity_Architecture.mddocs/architecture/UserInteraction_And_Query_Architecture.mddocs/architecture/event-taxonomy.mddocs/architecture/provenance.mddocs/architecture/security-architecture.mddocs/roadmap/FUTURE_EVENT_PIPELINE_DESIGN.mddocs/test-suite-report/TEST_ANALYSIS_README.mddocs/test-suite-report/TEST_FILE_INVENTORY.mddocs/test-suite-report/TEST_INFRASTRUCTURE_ANALYSIS.mddocs/vision/manifesto.mddocs/vision/project-target-state.mddocs/vision/streaming-architecture.mddocs/way_2.mdflake.nixscripts/combine-files-batch.shscripts/dev-env-banner.shscripts/schema-dev.shtests/e2e/nixos-vm/common/test-base.nix
Comment on lines
+19
to
+24
| - name: Install Rust toolchain | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| profile: minimal | ||
| toolchain: stable | ||
| override: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the recommended action exists and check latest version
curl -s https://api.github.com/repos/dtolnay/rust-toolchain/releases/latest | jq -r '.tag_name // "No releases, use `@stable` or `@master`"'Repository: Sinity/sinex
Length of output: 91
🌐 Web query:
dtolnay/rust-toolchain GitHub action
💡 Result:
dtolnay/rust-toolchain is a concise GitHub Action that installs a Rust toolchain via rustup. The Rust toolchain you get is determined by the action ref you use (the @...). [1]
Basic usage
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: cargo test --all-features[1]
Selecting the toolchain
dtolnay/rust-toolchain@nightly→ installs nightlydtolnay/rust-toolchain@1.89.0→ installs Rust 1.89.0
[1]
Using inputs (targets/components)
- uses: dtolnay/rust-toolchain@master
with:
toolchain: nightly-2025-01-01
targets: wasm32-unknown-unknown
components: clippy, rustfmtNote: if you set with: toolchain: ..., the README recommends using dtolnay/rust-toolchain@master as the action revision. [1]
Outputs
cachekey: short hash of installedrustcversion (usable for cache keys)name: rustup toolchain name (usable likecargo +${{ steps.toolchain.outputs.name }})
[1]
🌐 Web query:
actions-rs/toolchain deprecated v1
💡 Result:
actions-rs/toolchain@v1 is effectively deprecated because the actions-rs/toolchain repository was archived (made read-only) on Oct 13, 2023, so it’s no longer maintained or updated for GitHub Actions changes/deprecations. [1]
What to use instead
dtolnay/rust-toolchain(simple replacement for installing a toolchain) [2]actions-rust-lang/setup-rust-toolchain(Rust toolchain setup via rustup, with extra CI-focused features) [3]- Or, in many cases, skip a setup action entirely and just run
rustup toolchain install .../rustup default ...in your workflow (since rustup is typically present on GitHub-hosted runners). [4]
Example replacement (common)
- name: Install Rust
uses: dtolnay/rust-toolchain@stable
with:
toolchain: stable
components: clippy, rustfmtSources
[1] https://github.com/actions-rs/toolchain
[2] https://github.com/dtolnay/rust-toolchain
[3] https://github.com/actions-rust-lang/setup-rust-toolchain
[4] https://doc.rust-lang.org/cargo/guide/continuous-integration.html
Replace deprecated actions-rs/toolchain with dtolnay/rust-toolchain.
The actions-rs/toolchain@v1 action is from an archived repository (as of October 2023) and is no longer maintained. This can cause failures on newer GitHub runners.
🔧 Proposed fix
- name: Install Rust toolchain
- uses: actions-rs/toolchain@v1
+ uses: dtolnay/rust-toolchain@stable
- with:
- profile: minimal
- toolchain: stable
- override: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Install Rust toolchain | |
| uses: actions-rs/toolchain@v1 | |
| with: | |
| profile: minimal | |
| toolchain: stable | |
| override: true | |
| - name: Install Rust toolchain | |
| uses: dtolnay/rust-toolchain@stable |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 20-20: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/schema-auto-update.yml around lines 19 - 24, Replace the
deprecated GitHub Action step that currently uses "actions-rs/toolchain@v1" (the
"Install Rust toolchain" step) with the maintained "dtolnay/rust-toolchain@v1"
action; update the step's uses value to "dtolnay/rust-toolchain@v1" and keep the
toolchain configuration (e.g., set toolchain: stable and override: true or
equivalent inputs supported by dtolnay/rust-toolchain) so the workflow installs
the stable Rust toolchain via the supported action.
Comment on lines
+31
to
+38
| - name: Create schema update PR | ||
| uses: peter-evans/create-pull-request@v5 | ||
| with: | ||
| commit-message: "chore: regenerate schemas" | ||
| title: "chore: regenerate schemas" | ||
| body: "Automated schema regeneration via GitHub Actions." | ||
| branch: chore/auto-update-schemas | ||
| labels: automated-update |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest version of peter-evans/create-pull-request
curl -s https://api.github.com/repos/peter-evans/create-pull-request/releases/latest | jq -r '.tag_name'Repository: Sinity/sinex
Length of output: 60
🏁 Script executed:
#!/bin/bash
# Try different approaches to get version info
echo "=== Attempt 1: Using releases API with more detail ==="
curl -s https://api.github.com/repos/peter-evans/create-pull-request/releases | jq -r '.[0:3] | .[] | {tag_name, draft, prerelease}'
echo "=== Attempt 2: Check if repository exists and get latest tag ==="
curl -s https://api.github.com/repos/peter-evans/create-pull-request | jq -r '{pushed_at, updated_at}'
echo "=== Attempt 3: Get all tags ==="
curl -s https://api.github.com/repos/peter-evans/create-pull-request/tags | jq -r '.[0:10] | .[] | .name'Repository: Sinity/sinex
Length of output: 396
🏁 Script executed:
#!/bin/bash
# Get more info about the actual latest release and check for v5
echo "=== Latest actual release ==="
curl -s https://api.github.com/repos/peter-evans/create-pull-request/releases/latest | jq -r '.tag_name'
echo "=== Check for v5 releases ==="
curl -s https://api.github.com/repos/peter-evans/create-pull-request/tags | jq -r '.[] | select(.name | startswith("v5")) | .name' | head -5
echo "=== Check v8 info ==="
curl -s https://api.github.com/repos/peter-evans/create-pull-request/releases | jq -r '.[] | select(.tag_name == "v8.1.0") | {tag_name, published_at, prerelease}' | head -20Repository: Sinity/sinex
Length of output: 316
Update peter-evans/create-pull-request to v8 (the current major version).
The workflow is using v5, which is significantly outdated. v8 is the current major version with the latest runner compatibility and features. Consider upgrading to v8.
🔄 Suggested update
- name: Create schema update PR
- uses: peter-evans/create-pull-request@v5
+ uses: peter-evans/create-pull-request@v8
with:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Create schema update PR | |
| uses: peter-evans/create-pull-request@v5 | |
| with: | |
| commit-message: "chore: regenerate schemas" | |
| title: "chore: regenerate schemas" | |
| body: "Automated schema regeneration via GitHub Actions." | |
| branch: chore/auto-update-schemas | |
| labels: automated-update | |
| - name: Create schema update PR | |
| uses: peter-evans/create-pull-request@v8 | |
| with: | |
| commit-message: "chore: regenerate schemas" | |
| title: "chore: regenerate schemas" | |
| body: "Automated schema regeneration via GitHub Actions." | |
| branch: chore/auto-update-schemas | |
| labels: automated-update |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 32-32: the runner of "peter-evans/create-pull-request@v5" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/schema-auto-update.yml around lines 31 - 38, Update the
GitHub Actions step that creates the PR to use the current major version of the
action by replacing the action reference peter-evans/create-pull-request@v5 with
peter-evans/create-pull-request@v8 in the "Create schema update PR" workflow
step; after updating, run the workflow or lint the YAML to ensure input names
(commit-message, title, body, branch, labels) remain compatible with v8 and
adjust any renamed inputs per the action's v8 docs if needed.
Comment on lines
+34
to
+39
| - name: Install Rust toolchain | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| profile: minimal | ||
| toolchain: stable | ||
| override: true |
There was a problem hiding this comment.
Replace deprecated actions-rs/toolchain@v1 with dtolnay/rust-toolchain.
The actions-rs/toolchain action is unmaintained and deprecated. The community has migrated to dtolnay/rust-toolchain.
🔧 Proposed fix
- - name: Install Rust toolchain
- uses: actions-rs/toolchain@v1
- with:
- profile: minimal
- toolchain: stable
- override: true
+ - name: Install Rust toolchain
+ uses: dtolnay/rust-toolchain@stable📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Install Rust toolchain | |
| uses: actions-rs/toolchain@v1 | |
| with: | |
| profile: minimal | |
| toolchain: stable | |
| override: true | |
| - name: Install Rust toolchain | |
| uses: dtolnay/rust-toolchain@stable |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 35-35: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sqlx-auto-update.yml around lines 34 - 39, Replace the
deprecated GitHub Action used in the "Install Rust toolchain" step: change uses:
actions-rs/toolchain@v1 to uses: dtolnay/rust-toolchain@v1 and update the inputs
to match dtolnay/rust-toolchain (keep toolchain: stable and override: true,
remove or adjust the unsupported profile input). Ensure the step retains the
original name "Install Rust toolchain" and validates the workflow syntax after
the change.
Comment on lines
+50
to
+52
| psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB" | ||
| psql -h localhost -U postgres -c "CREATE DATABASE sinex_test OWNER sinex_test" | ||
| psql -h localhost -U postgres -d sinex_test -c "CREATE EXTENSION IF NOT EXISTS timescaledb" |
There was a problem hiding this comment.
Database sinex_test already exists from service container; creation will fail.
Line 22 configures POSTGRES_DB: sinex_test on the service container, so the database already exists. Line 51 then attempts CREATE DATABASE sinex_test, which will error since Postgres doesn't support IF NOT EXISTS for databases in plain CREATE DATABASE.
🐛 Proposed fix: skip redundant database creation or use a different name
psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB"
- psql -h localhost -U postgres -c "CREATE DATABASE sinex_test OWNER sinex_test"
+ psql -h localhost -U postgres -c "ALTER DATABASE sinex_test OWNER TO sinex_test"
psql -h localhost -U postgres -d sinex_test -c "CREATE EXTENSION IF NOT EXISTS timescaledb"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB" | |
| psql -h localhost -U postgres -c "CREATE DATABASE sinex_test OWNER sinex_test" | |
| psql -h localhost -U postgres -d sinex_test -c "CREATE EXTENSION IF NOT EXISTS timescaledb" | |
| psql -h localhost -U postgres -c "CREATE USER sinex_test WITH PASSWORD 'test_password' CREATEDB" | |
| psql -h localhost -U postgres -c "ALTER DATABASE sinex_test OWNER TO sinex_test" | |
| psql -h localhost -U postgres -d sinex_test -c "CREATE EXTENSION IF NOT EXISTS timescaledb" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sqlx-auto-update.yml around lines 50 - 52, The workflow
currently runs psql commands that attempt to create the database sinex_test even
though the service container already sets POSTGRES_DB=sinex_test; remove or skip
the redundant CREATE DATABASE step (the psql command that runs "CREATE DATABASE
sinex_test OWNER sinex_test") or change it to operate on a different database
name so it doesn't fail, and ensure the subsequent command that creates the
extension uses the existing database (the psql invocation that runs "CREATE
EXTENSION IF NOT EXISTS timescaledb") against that existing DB; update the shell
steps so only the CREATE USER and the extension creation run against sinex_test,
or guard the CREATE DATABASE with a conditional check to skip when the DB
exists.
Comment on lines
+61
to
+68
| - name: Create SQLx cache PR | ||
| uses: peter-evans/create-pull-request@v5 | ||
| with: | ||
| commit-message: "chore: refresh SQLx cache" | ||
| title: "chore: refresh SQLx cache" | ||
| body: "Automated SQLx cache regeneration via GitHub Actions." | ||
| branch: chore/auto-update-sqlx-cache | ||
| labels: automated-update |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Update peter-evans/create-pull-request to v7.
Version 5 uses an older Node.js runtime that GitHub Actions will eventually deprecate. v7 is the current stable release.
♻️ Proposed fix
- name: Create SQLx cache PR
- uses: peter-evans/create-pull-request@v5
+ uses: peter-evans/create-pull-request@v7
with:
commit-message: "chore: refresh SQLx cache"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Create SQLx cache PR | |
| uses: peter-evans/create-pull-request@v5 | |
| with: | |
| commit-message: "chore: refresh SQLx cache" | |
| title: "chore: refresh SQLx cache" | |
| body: "Automated SQLx cache regeneration via GitHub Actions." | |
| branch: chore/auto-update-sqlx-cache | |
| labels: automated-update | |
| - name: Create SQLx cache PR | |
| uses: peter-evans/create-pull-request@v7 | |
| with: | |
| commit-message: "chore: refresh SQLx cache" | |
| title: "chore: refresh SQLx cache" | |
| body: "Automated SQLx cache regeneration via GitHub Actions." | |
| branch: chore/auto-update-sqlx-cache | |
| labels: automated-update |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 62-62: the runner of "peter-evans/create-pull-request@v5" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sqlx-auto-update.yml around lines 61 - 68, Update the
GitHub Action usage from "uses: peter-evans/create-pull-request@v5" to "uses:
peter-evans/create-pull-request@v7" and verify the existing inputs
(commit-message, title, body, branch, labels) remain valid for v7; adjust the
"labels" input to a YAML array if required by v7 (e.g., labels:
["automated-update"]) and re-run/validate the workflow to ensure no input names
changed between v5 and v7.
Comment on lines
+365
to
+366
| ### Property-Based Coverage | ||
| - **All suites use `sinex_proptest!` or `#[sinex_prop]`** |
There was a problem hiding this comment.
Fix heading spacing to satisfy markdownlint MD022.
Line 365 needs blank lines above and below the heading.
Suggested fix
- **Performance**: <10ms overhead per test
-
### Property-Based Coverage
+
+- **All suites use `sinex_proptest!` or `#[sinex_prop]`**
+### Property-Based Coverage
+
- **1000+ cases per property test**🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 365-365: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/test-suite-report/TEST_FILE_INVENTORY.md` around lines 365 - 366, The
markdown heading "### Property-Based Coverage" violates MD022 because it lacks
blank lines around it; edit the TEST_FILE_INVENTORY.md entry containing the
heading "### Property-Based Coverage" and add a single blank line above and a
single blank line below the heading so there is an empty line separating the
heading from surrounding content.
Comment on lines
+9
to
+10
| > **Accuracy Notice (2025-07-24, refreshed 2025-02-24)** | ||
| > Legacy references to `docs/TARGET_final.md` were replaced with pointers to `docs/way.md`. If you still find links to removed files, treat them as historical context only and update them to match the JetStream plan. |
There was a problem hiding this comment.
Fix chronological inconsistency in the Accuracy Notice date.
Line 9 says “refreshed 2025-02-24” after “2025-07-24,” which reads backward in time.
Suggested fix
-> **Accuracy Notice (2025-07-24, refreshed 2025-02-24)**
+> **Accuracy Notice (2025-02-24, refreshed 2025-07-24)** 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| > **Accuracy Notice (2025-07-24, refreshed 2025-02-24)** | |
| > Legacy references to `docs/TARGET_final.md` were replaced with pointers to `docs/way.md`. If you still find links to removed files, treat them as historical context only and update them to match the JetStream plan. | |
| > **Accuracy Notice (2025-02-24, refreshed 2025-07-24)** | |
| > Legacy references to `docs/TARGET_final.md` were replaced with pointers to `docs/way.md`. If you still find links to removed files, treat them as historical context only and update them to match the JetStream plan. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/vision/streaming-architecture.md` around lines 9 - 10, The Accuracy
Notice line contains dates in reverse chronological order ("2025-07-24,
refreshed 2025-02-24"); update the string so the "refreshed" date is later than
the original (for example change the fragment to "Accuracy Notice (2025-02-24,
refreshed 2025-07-24)") by editing the exact notice text shown (the line
containing "**Accuracy Notice (2025-07-24, refreshed 2025-02-24)**") in the
document.
Comment on lines
+131
to
+134
| ## Change Control | ||
| - Update this file whenever a section is completed or new work is added. | ||
| - When a decision graduates from “plan” to “complete,” move details into the relevant canonical doc (e.g., `docs/way.md`, README, component guides) and trim this file accordingly. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank line after heading per Markdown style guidelines.
The static analysis tool flags missing blank lines after headings at lines 131, 137, and 146. This is a minor formatting inconsistency.
📝 Proposed fix
## Change Control
+
- Update this file whenever a section is completed or new work is added.Apply similar fix after lines 137 and 146.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Change Control | |
| - Update this file whenever a section is completed or new work is added. | |
| - When a decision graduates from “plan” to “complete,” move details into the relevant canonical doc (e.g., `docs/way.md`, README, component guides) and trim this file accordingly. | |
| ## Change Control | |
| - Update this file whenever a section is completed or new work is added. | |
| - When a decision graduates from "plan" to "complete," move details into the relevant canonical doc (e.g., `docs/way.md`, README, component guides) and trim this file accordingly. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 131-131: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/way_2.md` around lines 131 - 134, Add a blank line immediately after the
second-level headings in docs/way_2.md that the linter flagged (for example the
"## Change Control" heading and the other similar section headings flagged by
the static analysis tool) so there is an empty line between the heading and the
following paragraph; update each heading occurrence in the file to include a
single blank line after it to satisfy Markdown style guidelines.
| checkPhase = '' | ||
| $out/bin/sinex-cli --help > /dev/null | ||
| ''; | ||
| doCheck = false; |
There was a problem hiding this comment.
Disabling checks here removes CLI packaging smoke coverage.
Since sinex-cli is manually assembled in installPhase, setting doCheck = false eliminates the only in-derivation signal that the wrapper/import path still works. Keep broad checks disabled if needed, but add an install-time smoke test.
Proposed fix
- doCheck = false;
+ doCheck = false;
+ doInstallCheck = true;
+ installCheckPhase = ''
+ runHook preInstallCheck
+ $out/bin/sinex-cli --help >/dev/null
+ runHook postInstallCheck
+ '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| doCheck = false; | |
| doCheck = false; | |
| doInstallCheck = true; | |
| installCheckPhase = '' | |
| runHook preInstallCheck | |
| $out/bin/sinex-cli --help >/dev/null | |
| runHook postInstallCheck | |
| ''; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` at line 175, The change disabled in-derivation checks by setting
doCheck = false which removes smoke coverage for the manually assembled
sinex-cli in installPhase; restore in-derivation verification by either leaving
doCheck enabled or, if broader checks must remain off, add an install-time smoke
test inside installPhase that runs the assembled binary (e.g., invoke the
sinex-cli entrypoint with --version or --help) and fails the build on non-zero
exit to assert the wrapper/import path is correct; update the package expression
referencing doCheck and the installPhase/sinex-cli assembly (the installPhase
that creates sinex-cli) accordingly.
Comment on lines
+62
to
64
| metrics_path="${SINEX_TEST_METRICS_PATH:-target/sinex-test-metrics.jsonl}" | ||
| metrics_hint="writing ${metrics_path}" | ||
|
|
There was a problem hiding this comment.
Banner reports perf metrics as active even when disabled.
Perf metrics: writing ... is always shown, which is misleading when SINEX_TEST_METRICS is not enabled.
Proposed fix
metrics_path="${SINEX_TEST_METRICS_PATH:-target/sinex-test-metrics.jsonl}"
-metrics_hint="writing ${metrics_path}"
+metrics_hint="disabled"
+if [[ -n "${SINEX_TEST_METRICS:-}" ]]; then
+ case "${SINEX_TEST_METRICS,,}" in
+ ""|"0"|"false") ;;
+ *) metrics_hint="writing ${metrics_path}" ;;
+ esac
+fiAlso applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/dev-env-banner.sh` around lines 62 - 64, The banner always shows
"Perf metrics: writing ..." because metrics_hint is set unconditionally; change
the logic to only set metrics_path/metrics_hint when SINEX_TEST_METRICS is
truthy (check the SINEX_TEST_METRICS env var) and otherwise set metrics_hint to
a disabled/empty string so the banner does not report metrics as active; update
the assignments that currently use SINEX_TEST_METRICS_PATH and metrics_hint
(variables metrics_path and metrics_hint) and the corresponding banner print
site so both occurrences (the initial block and the later duplicate) follow the
same conditional behavior.
Owner
Author
4 tasks
Sinity
added a commit
that referenced
this pull request
Jul 13, 2026
Harden the gateway ingress path by requiring authenticated RPC traffic, validating native-messaging callers, and bounding blob ingestion before bytes reach git-annex, then thread the same contracts through the CLI, SDK, and document ingestor. sinex-core: - Rewrite `crate/lib/sinex-core/src/environment.rs::SinexEnvironment::database_url()` around parsed `Url` handling so environment namespacing works for path-based URLs as well as `dbname`/`database` query parameters, and add targeted tests for empty/duplicate namespace cases. - Replace a few small panic-prone or static-initialization paths, such as the analytics epoch helper, with explicit runtime initialization so the surrounding gateway/CLI hardening does not keep incidental footguns alive. sinex-gateway: - Add real RPC authentication in `crate/core/sinex-gateway/src/rpc_server.rs`: the server now loads `SINEX_RPC_TOKEN`/`SINEX_RPC_TOKEN_FILE`, accepts `Authorization: Bearer` or `X-Sinex-Rpc-Token`, and returns explicit unauthorized JSON-RPC errors when the token is missing or invalid. - Tighten bind-address handling so TCP exposure is opt-in instead of an accidental development default, which makes the token requirement meaningful at the transport boundary. - Extend `crate/core/sinex-gateway/src/native_messaging.rs` with trusted-extension parsing, optional shared secrets, and structured `native_messaging.auth` logging so browser callers are checked against an allow-list instead of a TODO comment. - Bound blob uploads in `crate/core/sinex-gateway/src/handlers.rs` before and after base64 decoding using `SINEX_GATEWAY_MAX_BLOB_BYTES`, which rejects oversized payloads before annex writes and keeps the gateway from acting like an unbounded blob proxy. CLI, SDK, and ingestion path: - Thread RPC tokens through `cli/exo.py` and `cli/rpc_client.py`, adding `--rpc-token`, global token propagation, and better “database unavailable” reporting so the CLI can follow the gateway’s auth contract without making operators rediscover it manually. - Change `crate/lib/sinex-satellite-sdk/src/annex/blob_manager.rs::BlobManager::new()` to accept an optional event sender, which lets gateway content paths avoid writing events directly while Stage-as-you-go and clipboard paths still emit blob events when they should. - Rework `crate/satellites/sinex-document-ingestor/src/lib.rs` to stop buffering entire files before blob ingestion: it now enforces size limits from metadata, sniffs only a small prefix for encoding, and records the file size/encoding without loading the full document into memory. packaging, docs, and tests: - Update README and gateway architecture docs with the new RPC token, blob quota, and trusted-extension contracts so the supported local/deployment path is explicit. - Add regression coverage for gateway auth headers, blob quota enforcement, no-direct-event blob storage, native-messaging trust checks, and environment URL namespacing, which turns several previously documented gaps into enforced boundary checks. - Disable the `sinex-cli` Nix package check phase after the gateway/CLI auth work, avoiding a brittle build-time import check that was not exercising the real runtime contract.
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
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
I tightened every ingress path that was still trusting callers or payload size too early. The gateway now requires an RPC token by default, native-messaging callers must come from an explicit trusted-extension allow-list, blob uploads are bounded before and after base64 decoding, and the document ingestor no longer buffers full files before handing them to annex/blob storage. To make those changes usable rather than surprising, I threaded the token contract through
exo, updated the relevant docs, and changedBlobManagerso gateway content storage can avoid injecting events directly intocore.events. The branch also carries the smaller core/environment fixes that were needed to make the surrounding auth and deployment story reliable.Motivation
Before this work, several critical gateway paths were only “safe” because they were assumed to be local.
JSON-RPC could start without a real authentication secret, native messaging still had a TODO for extension origin validation, and the blob upload handler would happily decode large base64 payloads before checking whether the payload size was acceptable for the gateway process. On the ingestion side, the document ingestor was reading whole files into memory just to compute data that could be derived from metadata and a small prefix read. Those are all variations of the same design flaw: trust and allocation happened too early.
I wanted to fix them together because they sit on the same boundary. Once the gateway becomes explicit about caller trust and payload size, the CLI has to speak that auth contract too, the docs have to tell operators how to configure it, and the blob/document path has to stop bypassing those assumptions in adjacent code.
There was also a branch-level dead end worth calling out. I briefly carried an experimental transaction-helper refactor and an early
sinex_proprollout while this work was in flight; both turned out to distract from the actual hardening story, and the unstable property-test changes were backed out and repaired before the branch settled. The final squash is intentionally focused on ingress hardening rather than those experiments.Changes grouped by concern
1. Require authentication at the JSON-RPC boundary
The biggest functional change is in
crate/core/sinex-gateway/src/rpc_server.rs. The server now loads a shared secret fromSINEX_RPC_TOKENorSINEX_RPC_TOKEN_FILE, rejects unauthenticated requests with explicit JSON-RPC errors, and accepts two caller-side header styles:I kept
SINEX_GATEWAY_ALLOW_INSECURE=1as an explicit test/dev escape hatch, but the normal path is now fail-closed. I also tightened bind-address resolution so TCP exposure is opt-in instead of being silently enabled by development defaults. That combination matters: auth tokens only help if the transport story is explicit too.2. Validate native-messaging clients instead of trusting them by convention
Native messaging had documentation saying origin validation still needed to happen. I turned that into a real contract.
NativeMessagingConfignow parsesSINEX_NATIVE_MESSAGING_TRUSTED_EXTENSIONSas a comma-separated list of extension IDs, each optionally carrying a shared secret:The server validates
extension_idon every message and, when configured, validatesextension_secrettoo. Every allow/deny decision is logged as a structurednative_messaging.authevent with concrete reasons such asmissing_extension_id,not_trusted,missing_secret, orinvalid_secret. That gives us both enforcement and auditability.3. Bound blob uploads before they cost memory or disk
handle_store_blob()now enforcesSINEX_GATEWAY_MAX_BLOB_BYTEStwice: once against the encoded payload length and again after decode. That means the gateway can reject an oversized upload before it spends memory decoding or hands bytes to git-annex.The important detail here is the order of operations. The old path only had a conceptual quota; the new path applies the limit before the expensive work. That keeps the gateway from acting as a convenient oversized-blob ingress even on trusted local networks.
4. Make the CLI follow the same auth contract
Once the gateway requires a token, the CLI has to stop pretending the transport is anonymous. I added
--rpc-tokentocli/exo.py, threaded global token handling through the command tree, and extendedSinexRPCClientso it sends both the bearer and custom header forms when a token is configured.The client now looks like this:
I also used the same pass to improve the CLI’s “database unavailable” path so it can explain whether the operator should set
DATABASE_URL, enter the dev shell, or provide the RPC token instead of just surfacing a lower-level failure.5. Stop making the gateway and document ingestor do work they do not need to do
The gateway content path was still awkwardly coupled to event emission. I changed
BlobManager::new()so it accepts an optional sender. The Stage-as-you-go and clipboard paths still passSome(sender)because they should emit blob lifecycle events. The gateway service container now passesNone, which means “store content, but do not turn the gateway into a direct event writer.”On the document-ingestor side, I removed the full-file buffer from the normal ingestion path. The ingestor now checks file size from metadata, reads only a small prefix to decide whether text looks like UTF-8 or binary, and records the size/encoding alongside the material ID without slurping the entire file into memory first. This is exactly the same “do less before trust/limit checks” principle as the gateway blob handler, just applied one layer deeper in the ingestion path.
6. Tighten the surrounding environment and packaging story
I rewrote
SinexEnvironment::database_url()around parsed URL handling so database namespacing now works for socket-style URLs that carrydbname=ordatabase=query parameters as well as conventional path URLs. That closes a subtle but real deployment/setup bug exposed by the broader gateway work.I also disabled the Nix
sinex-clipackage check phase. After the RPC token changes, the old “run--helpin checkPhase” probe stopped telling us anything useful about the real runtime contract and just introduced a brittle packaging wrinkle.Testing
The branch adds explicit regression coverage around the new boundaries:
gateway_auth_blocks_missing_tokengateway_auth_accepts_bearer_headergateway_auth_accepts_custom_headerblob_routes_should_enforce_auth_and_quotacontent_store_blob_does_not_insert_eventsnative_messaging_rejects_untrusted_extensionsdbname=/database=handlingThose tests matter because most of the pre-branch problems were not missing features; they were missing rejections. The value here is in making malformed or under-configured calls fail explicitly.
The main remaining gap is that this branch hardens the boundary and the adjacent SDK/CLI/document paths, but it does not attempt a full end-to-end deployment exercise for every auth/devenv combination. The contracts are now explicit enough that those can be layered on later.
Notes
SINEX_GATEWAY_ALLOW_INSECURE=1is intentionally still available, but only as an explicit escape hatch for tests and tightly controlled local experiments.I also left the docs very direct about the new contract. This kind of hardening is only useful when the operator-facing setup instructions are unambiguous, so the README and gateway architecture docs now say exactly which token env vars, headers, and blob-size controls are supported.
Ref #99
Summary by CodeRabbit
Release Notes
New Features
confirmations tailCLI command for event monitoringBug Fixes
Infrastructure