Repository navigation
feat(rust): Phase 1 foundation — workspace, crates, config parity, CI - #879
Merged
Merged
Conversation
…nd add the sandbox runner Root [workspace] with resolver 3, the workspace dependency table, the workspace lint table and the rust-toolchain.toml pin (1.99.1, rustfmt+clippy, minimal profile). scripts/rust.sh runs one cargo command inside containers-rust-builder with the host caches, create -> start -> wait -> logs -> rm, cleaning up on every exit path.
Event/EventId/Pubkey/Kind/KindSelector, Filter, the client and relay message payloads, EventStoreBackend (postgres + lmdb), the ShutdownState drain flag, and one module per nip*.ts utility from the capture. Every entry point that would need real behaviour returns CoreError::NotImplemented; there is no crypto dependency yet (the secp256k1 vs k256 choice is a Phase 2 decision).
The typed schema for all 18 shipped sections (61 capture types), the loader with the capture's fallback-to-defaults rule, mergeDeepRight over Value, the validator with the capture's issue strings and ordering, the dot-path editor and value parser, redaction, the guided catalogue, and the admin-notification merge. The parity fixture carries 17 measured cases from 242a300 (the last two pin the deferred NIP-77 surface as absent); the round-trip test proves every rename.
…ironment slice Nine repository traits mirroring src/@types/repositories.ts (2-4 representative methods each; the full surface and the 622-line event-repository body are Phase 2), event_store::resolved_backend mirroring src/utils/event-store.ts, a RedisCache stub with the cache seam crate-private, and a real LmdbEnv over heed 0.20 (open/put/get, tested against a tempdir) -- the one unsafe boundary in the workspace, which is why the workspace lint is deny with one audited allow rather than forbid.
Five message handlers, fifteen event-strategy modules, connection/subscription state, the two rate limiters, the broadcast fan-out and the negentropy session seam -- all labelled stubs returning RelayError::NotImplemented. No WebSocket dependency; the NIP-77 neg-* handlers are deliberately absent (they exist at the porting oracle, not at this branch's capture).
The eight services, the PaymentProcessor seam over the six processor variants, the invoice/callback controllers and the NIP-66 probe family (dns, tls, nip11, ws-rtt and the run-probe merge) -- labelled stubs. No HTTP client dependency.
The admin controllers and request handlers as typed skeletons, the route table, the password auth provider and the admin utils. No axum in Phase 1; the MCP server has no TS counterpart at the capture and is named Phase 5.
config validate (one 'path: message' line per issue, exit 2 on any issue, the clean-document confirmation otherwise) and config get <path> [--json] run end to end over nostream-config; the other ten commands, the TUI and the docker/process supervisors are labelled stubs. clap in the bin; anyhow only in the binary's error handling (spec D10).
…on-signal The daemon binary (--config-dir/--check/--listen-port/--log-level) with the settings gate (per-issue lines, exit 2), telemetry init (stdout subscriber now, OTLP behind the 'otlp' feature reading OTEL_EXPORTER_OTLP_ENDPOINT), the single acceptor task that binds, accepts and drops sockets with a debug line, and the SIGINT/SIGTERM drain through core::lifecycle::ShutdownState. Runtime decision recorded in ADR 0001: no SO_REUSEPORT sharding in Phase 1.
checks.yml gains the 'rust' path filter (crates/**, Cargo.{toml,lock},
rust-toolchain.toml, resources/default-settings.yaml, scripts/rust.sh and the
workflow itself), adds default-settings.yaml to the 'src' filter, and a Rust
Checks job running fmt, both clippy rounds, tests and the release build. ADR
0001 records the runtime/acceptor decision and the telemetry deviation
(tracing-otlp does not exist; opentelemetry-otlp + tracing-opentelemetry
instead). .gitignore takes /target/; .dockerignore excludes crates and target
from the relay image; empty changeset for the changeset-check.
…round-1 P3s The nine repository traits return Pin<Box<dyn Future>> instead of RPITIT so Arc<dyn EventRepository> is writable (regression test boxes two of them); resolved_backend returns StorageError::UnsupportedBackend instead of String; the LMDB unsafe justification states heed 0.20.5's real contract (per-process Env dedup; NO_LOCK/network-fs/map_size preconditions) instead of an invariant heed does not impose; nip66's monitor-kind set is the measured 10166/30166 (50166 is invented); nip77/negentropy cite only files the capture has; Connection::accept takes the transport + shared context rather than a bare SocketAddr; services drops the operator-notifications.ts claim (config owns it); rust.sh falls back to the local tree when the host dir is invisible.
…rker errors, keep the drain flag The settings gate moves to a library fn and follows the capture's fallback rule: findings are logged and the serve path continues on the shipped defaults; --check alone exits 2 (P1). --log-level now builds the filter (cli > LOG_LEVEL > RUST_LOG > info) and is passed into telemetry::init (P2). Worker and tor task failures are warn-logged at drain instead of being discarded (P2). ShutdownState::begin_draining uses send_replace so a drain before the first subscriber is stored, not silently dropped, with a regression test on both orders (P2).
…SHA-pinned actions Admin: the route table carries all 18 entries in capture order plus the two prefix middlewares and the /assets mount; the missing 11 controller modules and 4 request-handler modules exist as typed stubs (30 modules total); get_dashboard cites the handler's real path; the lib header states the full inventory. Daemon: the module map is spec 3.2's daemon list (src/app/, telemetry/, routes/index.ts, logger.ts, tor, web-server-adapter, the 8 factories) with the WS adapters returned to nostream-relay. ADR 0001: the capture's process model is corrected to cluster-based (App primary forks cpus().length client workers that all listen(); IPC into each adapter) and D8.2 states Phase 2 decides against that model. CI: the phantom toolchain-file input is dropped (the pin rides the action's auto-read), both third-party actions are SHA-pinned with measured SHAs, and the job declares permissions contents:read.
passwordHash, notification target url and botToken become Secret, whose Debug/Display redact — one tracing debug of a settings struct can no longer print an admin password hash or a webhook token (the round-1 security lens flagged them as plain Strings inside derive(Debug) structs). Secret's doc comment is narrowed to the per-field guarantee; serde's transparent passthrough keeps the document shape byte-identical. All 117 config tests and the 17-case parity fixture stay green.
…ead-only config probe The typed-parse fallback issue reported serde's raw message, which renders the offending scalar (e.g. an out-of-i64 integer); it now reports the class and the parse location only. load_user_settings no longer mkdirs the config directory — a read probe returns None for an absent directory so nostreamd --check on a fresh host writes nothing; the test pins the new behaviour.
run() fell through the gate into telemetry::init and serve, so --check bound 0.0.0.0:8008 and blocked on signals instead of exiting. The regression test executes the binary (CARGO_BIN_EXE_nostreamd) and asserts exit 0 with the clean-document line within a bound, plus exit 2 + the issue line on stderr for an invalid document.
…g, redacted Display, ADR citations
get_redacted_admin_notifications wraps the subtree in {admin:{notifications}}
exactly as the TS caller does, so the url rule matches
admin.notifications.targets[...] — a bare targets[0].url sailed through
while botToken redacted, hiding the gap; the new test roots at the subtree.
A config path that exists but is not a directory is now a finding + defaults
fallback, not silent absence. Secret gains the redacted Display its docs
promised (with tests through Debug, Display and a carrier struct). ADR 0001
corrected: only AppWorker listens (worker.ts:33 after :32's port expression);
the four specialist forks are app.ts:127-155.
…SHA-pinned without a persisted token The unsafe comment lists each precondition heed documents — long-lived transactions, aborting with an active transaction, network filesystems, NO_LOCK/lock-file ownership, one map_size per path — and marks each upheld by the code or relied on the operator (the two operator-relied items name where Phase 2 closes them). The rust-checks checkout pins actions/checkout@11d5960a… # v4.4.0 (SHA measured from the remote) with persist-credentials: false, so the workflow token does not survive into the clone; the pre-existing Node jobs keep their own checkout lines.
The config-path probe is symlink-aware via symlink_metadata: a stat-able non-directory (file, dangling link, fifo) is NotADirectory, any non-NotFound stat error is the new StatSettingsPath, and only plain absence stays Ok(None) — with dangling-link and file-parent tests. The --check tests now spawn + poll try_wait against a 30s deadline and kill on expiry, so a reintroduced fall-through fails the suite instead of parking it. ADR 0001 cites the client workers at app.ts:103-120, maintenance at :122-125 and the rest at :127-155; nostreamd's header says the primary never listens. The NO_LOCK/lock-file precondition is split: the code guarantees no flags and never touches the lock file; the operator must keep the lock file intact.
…trailing separators The round-4 probe used lstat, which does not resolve the final component, so a config dir that is a symlink to a real directory — a location the capture accepts via existsSync — substituted the defaults. The probe now decides is_dir with fs::metadata (follows links, capture parity); on NotFound it lstats the path with trailing separators trimmed (POSIX resolves a trailing slash through the link), making a dangling link entry a finding and true absence the only Ok(None); other stat errors stay StatSettingsPath. Both new tests were run against 1be80b2's predicate and fail there, pass here.
Path::parent drops the final component, so …/config/ trimmed to …/ — an absent config dir named with a trailing slash produced a bogus NotADirectory on a fresh host, and a relative single-component dangling link with a trailing slash trimmed to empty and took silent Ok(None). The helper now trims trailing separator bytes from the path's own text on OsStr bytes (non-UTF-8 safe; a bare root is left itself). Both new tests were run against bb659c0's helper and fail there, pass here.
🦋 Changeset detectedLatest commit: 54e6514 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
Collaborator
Coverage Report for CI Build 39084720319Warning No base build found for commit Coverage: 73.973%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
 [High impact] The PR appears safe to merge, with one non-blocking issue in numeric error messages. Findings
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The documented setup installs the harness dependencies in the scratch capture tree, but `createRequire(import.meta.url)` resolved beside this script in the rewrite tree, where a checkout with no `node_modules` of its own has nothing to find. Resolve from `cwd` instead, which is the tree the setup step installs into and the tree the generator already validates as its working directory. Measured from a fresh `git archive` tree at /tmp/nostream-capture with `pnpm install --frozen-lockfile`: the regenerated cases reproduce the fixture per name, in order, byte-for-byte in content.
`check_webhook_url` accepted any string with a scheme and something after `//`, so it passed addresses the capture's `new URL()` refuses: an out-of-range port (`https://hooks.example:99999/hook`) and a space in the host (`https://bad host/hook`). The capture parses with the WHATWG parser inside `try`, so the Rust check now parses with `url`, which implements the same standard, and keeps the capture's two outcomes: a parsed non-http(s) scheme is 'url must use http or https' and a parse failure is 'url must be a valid URL'. The raw text is passed through untrimmed, as the capture does — the parser strips leading and trailing C0 controls and spaces itself. Both malformed addresses are added to the parity fixture and regenerated from the capture at 242a300 (case names `target_url_bad_port`, `target_url_bad_host`), so a check cannot pass an address the daemon's URL parser refuses. The other 17 cases regenerate unchanged; the generator's exempt-reason string is aligned with the copy the fixture already carried, so a regeneration reproduces the fixture exactly. Measured: cargo test -p nostream-config --test ts_parity (6 passed), tests/validator.rs including a 65535 boundary case that must still parse.
Two P1s on the same seam — the loader's two paths disagreeing with what it reports. A settings path that exists but is not a regular file now reads as a finding, not as absence. `is_file()` answered false for a directory named `settings.yaml`, so the loader fell through to the deprecated `settings.json` or the shipped defaults and `--check` approved the result while the operator's real file was unusable. The capture `existsSync`-es the path and then fails on the read; `probe_settings_file` mirrors that: a non-file is `ConfigError::NotAFile`, a dangling symlink still falls through (the capture's `existsSync` is false there too), and an inspection the process cannot perform is loud. The same probe covers the legacy `settings.json`. A document the typed schema cannot represent is now refused with the field named. `validate_document` grades JavaScript types, where every number is a `number`, so a rate-limit `period` of `1000.5` was a positive number to the validator and unrepresentable to `u64` — `config validate` approved it, the daemon then discarded the operator's whole configuration for the shipped defaults, and the finding said only `$`. `fit` narrows a validated document and, on failure, walks the typed defaults (serialized back, so a `u64` stays an integer and an `f64` becomes a float) to name the first field whose number the type cannot hold. The daemon's `load` and the CLI's `config validate` both call it, so neither can approve what the other cannot load. Messages keep carrying the class, never the operator's value. Measured: cargo test --workspace --all-features (153 passed, 0 failed), clippy with -D warnings clean, fmt clean. End to end through the real binary: `config validate` on `periodMs: 1000.5` exits 2 with 'limits.event.pow.periodMs: must be a whole number'; `nostreamd --check` on the same file exits 2 with the field in stderr; `--check` with `settings.yaml` as a directory exits non-zero and names the path, where it used to print 'settings valid: 0 issues'.
…ional
`find_misfit` reads each field's type off the typed defaults serialized back to a
document, and `serialize_js_number` writes an integral `f64` as the integer `50` -
correct for the document, and lossy for the walk. `targetEventsPerSecond` is an
`f64`, so it looked like an integer field and was named first:
limits.event.pow.targetEventsPerSecond: must be a whole number
for a document whose actual fault was `periodMs: 1000.5`. Operators were sent to a
setting that is fine while the bad one went unreported.
Every `f64` in `settings.rs` now lives in `FLOAT_FIELDS`, and `force_floats`
restores each declared type before the walk: the projection cannot be asked for
types it does not carry, so the list is the source of truth and a guard test
pins its length and names the one entry the shipped defaults cannot verify
(`wotThresholds[]`, which they do not carry).
Measured with `nostream-cli config validate` on
`targetEventsPerSecond: 10.5` + `periodMs: 1000.5`:
before limits.event.pow.targetEventsPerSecond: must be a whole number
after limits.event.pow.periodMs: must be a whole number
The rate alone still validates (exit 0), so a legitimate `f64` is not refused.
Regression proof: with `force_floats` removed, `a_fractional_rate_blames_the_period_not_the_rate`
fails on exactly the two strings above.
Workspace: 157 pass / 0 fail, clippy -D warnings clean, fmt clean.
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Phase 1 of the Rust rewrite (#860, #862): the Cargo workspace, its eight crates, the toolchain pin, the CI job, the config crate with a parity test against the capture's TypeScript validator, and the two recorded decisions the issue asks for. Foundation, not a port — the config surface is real, everything else is typed stubs that say what they own.
Base is
rewrite/main@242a3001(the capture commit), per the rewrite charter;mainis a sibling and gets this branch only at cutover.The workspace
nostream-coreutils/event model, filters, kinds, NIP types, crypto, negentropy, validationnostream-confignostream-storagerepositories/,database/,cache/; the backend seam + one real LMDB environment slice (heed)nostream-relayhandlers/,relay-broadcast/, the two WebSocket adaptersnostream-servicesservices/,payments-processors/,utils/relay-probe/(a NIP-66 monitor)nostream-admincontrollers/admin/,routes/admin/, the admin middlewaresnostream-clicli/nostreamdindex.ts,app/(the worker model),telemetry/, tor, the composition rootEvery one of the 298
src/files is assigned somewhere; the mapping lives in each crate's header and in the spec's §3.2.nostream-configis the one crate added to the issue's list: the settings family is 1,831 LOC, it is the acceptance subject, and keeping it out ofcorekeeps YAML and file I/O out of the domain crate.Acceptance
cargo buildgreencargo build --workspace --all-features --release— Finished[optimized]5.46 s (throughscripts/rust.sh, the sandbox path; the same pin runs on CI)cargo fmt --checkcargo clippy -D warnings--workspace --all-targets --all-features; a second strict round adds-D clippy::unwrap_used -D clippy::expect_used -D clippy::todo -D clippy::unimplemented -D clippy::panicfor lib+binscargo testts_parity: 17 cases, 1 exemption (eventStore.backend), plusdefaults_parity(10),defaults_roundtrip(3),backend_values(3)git diff --name-only 242a3001..HEAD | grep -cE '^(src|test|resources)/'→ 0The parity test is graded against the capture's own validator, not a later oracle:
test/unit/utils/settings-config.spec.tsat242a3001has 34it(registrations / 41 executed tests, those run 41/41 in the worktree, and the Rust validator reproduces itspath :: messagepairs. Seventeen cases were generated from the live TS functions bycrates/nostream-config/tests/capture/gen-ts-issues.mjsand are pinned in the fixture with the subject commit recorded. The one exemption is deliberate and path-scoped: the issue mandates LMDB, the capture's validator accepts onlypostgres, so Rust acceptspostgres|lmdband the fixture names the single exempted path with its reason.Decisions recorded
docs/rewrite/decisions/0001-runtime-and-telemetry.md— tokio multi-thread runtime with a single acceptor task; the TS cluster model it replaces is described from the capture (a primary forkingcpus().lengthclient workers that eachlisten(), plus maintenance/static-mirroring/relay-monitor/DVM workers that never touch the adapter), and what Phase 2 must decide is named.rust-toolchain.tomlpins1.99.1withrustfmt+clippy,profile = minimal; edition 2024, resolver 3.thiserrorper crate,anyhowonly innostreamd;unsafe_code = denyworkspace-wide with one audited#[allow]whereheedrequires it, its preconditions listed and marked code-guaranteed or operator-supplied.tracing+tracing-subscriberalways; OTLP behind an optional feature viaopentelemetry-otlp+tracing-opentelemetry(tracing-otlpdoes not exist as a crate — the issue's name for it).Rust Checksjob inchecks.yml(fmt, both clippy rounds, tests, release build), gated on a newrustpath filter that also addsresources/default-settings.yamland the workflow itself tosrc**; both third-party actions SHA-pinned withpermissions: contents: read.actions/checkoutin that job is pinned and does not persist the token.The two actions this PR cannot take
Rust Checksis not a required status context. The six required contexts live in rulesetmain(id 2355390) and adding a seventh is a maintainer action; until then a skipped Rust job satisfies nothing and blocks nothing. This PR makes the job run on Rust-touching changes; requiring it is Cameri's call.containers-rust-builderplus GitHub Actions onubuntu-latest. tauceti stays untouched until its runbook is agreed with Ada — the spec records the exact trigger (a self-hosted runner registration, which no token available here can even list:actions/runners→ 403).Deferred, with reasons in the spec
041fe34e(default-settings.yaml,@types/settings.ts,settings-config.ts,settings-guided-schema.ts, its spec;+212/−0): the Rust config must not model a field its own tree cannot express, so it lands when the TS tree advances. Two parity cases pin the absence deliberately (ftp://accepted,nip77:rejected) and flip when it lands.Review
Three lenses plus a security pass over six rounds: 16 findings → 7 → 5 → 2 → 1 → 0. The tail was one helper: the config-dir probe's trailing-separator trim used
Path::parent()(which drops a component rather than a separator), then alstat-only predicate that rejected a symlinked config dir the capture accepts. Both were caught by the security lens and both fixes are pinned by tests proven to fail against the commit before them. The ones worth naming: the daemon aborted on an invalid usersettings.yamlwhere the capture falls back to the shipped defaults (P1);nostreamd --checklater served and hung on a valid config, breaking--checkentirely (P0, caught when the fix's own tests could not see it); nine repository traits used RPITIT and could not be boxed, defeating the runtime backend seam; the ADR argued from acluster-less relay the capture does not have; and a redactor that missed webhook URLs under a subtree root because every test used a full document.Job record:
projects/nostream/.factory/jobs/005-rust-foundation/(recon.md,spec.md,build.md, per-round reviews,runs.jsonl).Closes #862.