docs(protocol): moon:// / moons:// URI specification (H-7)#261
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughChangesThe pull request adds an authoritative specification for Moon’s Moon URI specification
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Write the authoritative, doc-only spec for Moon's native moon:// / moons:// connection URI scheme at docs/protocol/moon-uri.md. Moon is Redis-wire- compatible so redis:// / rediss:// already work and must keep working; moon(s):// is a strict superset that also covers Moon-native concerns (workspaces, server self-announce) that Redis's scheme has no room for. The doc reproduces the decided design (ABNF grammar, redis(s):// parity table, TLS/downgrade failure semantics) and expands it into an implementation-ready spec: a query-parameter reference (parity + native ?workspace=), a design-for-failure section (no opportunistic downgrade, no auto-upgrade, fail-fast diagnostic text, unknown-scheme parse errors, bounded connect via ?socket_connect_timeout=), server-participation semantics (--announce-url, INFO replication, cluster redirects, REPLICAOF), worked examples, and a conformance checklist that the v0.7.0 Workstream R6 implementation + tests/uri_scheme.rs must satisfy. Every cited flag and code path was verified against the current tree instead of restated from the roadmap prose uncritically: - --tls-port/--tls-cert-file/--tls-key-file/--tls-ca-cert-file/ --tls-ciphersuites in src/config.rs (lines 264-280) - TLS 1.3 via rustls + aws-lc-rs, mTLS via WebPkiClientVerifier, and SIGHUP hot-reload via Arc<ArcSwap<rustls::ServerConfig>> in src/tls.rs - WS AUTH <ws_id> takes a UUID only (src/command/workspace.rs:106) even though WorkspaceRegistry::get_by_name (src/workspace/registry.rs:58) already exists server-side -- flagged as an open R6c decision rather than assumed away - --announce-url, INFO's master_announce_url, and src/uri.rs confirmed absent from the tree; this doc is the spec they're built against, not a description of something partially shipped mkdocs.yml gains a "Protocol" nav section for the new page so `mkdocs build --strict` doesn't fail on an orphan doc. No Rust code changes. Implementation is tracked separately as v0.7.0 Workstream R6. author: Tin Dang <tindang.ht97@gmail.com>
TinDang97
force-pushed
the
docs/moon-uri-spec
branch
from
July 10, 2026 03:00
e3b8b53 to
22103a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/protocol/moon-uri.md`:
- Around line 303-307: Revise the documentation statement to scope the search to
implementation code and tests: state that no matches for announce_url,
announce-url, or src/uri.rs exist under src/ and test directories, without
claiming the entire repository has no matches. Update the affected “prospective
work” paragraph in moon-uri.md.
- Around line 41-60: Update the moon-uri grammar’s db-index production to accept
an optional slash with an optional database index, using the equivalent of ["/"
[db-index]], so URIs with trailing slashes and query strings without a database
index parse correctly; keep the existing db-index semantics unchanged.
- Around line 229-235: Revise the checklist to avoid requiring byte-identical
representations that erase the original scheme. In the `moon://`/`redis://` and
`moons://`/`rediss://` comparison criteria, define equality over normalized
overlapping fields while retaining scheme identity separately for round-trip
serialization, and update the round-trip requirement to verify preservation of
that identity.
- Around line 45-48: Resolve the inconsistency between the URI ABNF and decoding
rules by choosing one colon behavior and applying it everywhere: update the
userinfo grammar and the corresponding decoding/serialization rules so literal
password colons are either consistently permitted or consistently required to
use percent-encoding. Ensure the definitions at the userinfo grammar and
referenced decoding sections agree.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48bf5d47-3aee-4260-a59f-2fdc4846cc89
📒 Files selected for processing (3)
CHANGELOG.mddocs/protocol/moon-uri.mdmkdocs.yml
Comment on lines
+41
to
+60
| moon-uri = scheme "://" [ userinfo "@" ] host [ ":" port ] [ "/" db-index ] [ "?" query ] | ||
|
|
||
| scheme = "moon" / "moons" ; "moons" = TLS 1.3 transport (rustls, aws-lc-rs) | ||
|
|
||
| userinfo = [ username ] [ ":" password ] ; maps to AUTH [user] pass | ||
| username = *( unreserved / pct-encoded / sub-delims ) | ||
| password = *( unreserved / pct-encoded / sub-delims / ":" ) | ||
|
|
||
| host = IP-literal / IPv4address / reg-name / unix-path-encoded | ||
| ; IP-literal, IPv4address, reg-name per RFC 3986 §3.2.2 | ||
| unix-path-encoded = "unix" ; reserved reg-name value — see "Unix sockets" below | ||
|
|
||
| port = 1*DIGIT | ||
| ; moon: default 6379 if omitted (matches --port default) | ||
| ; moons: NO implicit default — port MUST be given explicitly and | ||
| ; MUST equal the server's configured --tls-port. There is no | ||
| ; well-known "TLS port" the way 443 is to 80; guessing one | ||
| ; would silently connect to the wrong listener or hang. | ||
|
|
||
| db-index = 1*DIGIT ; SELECT <db-index> on connect |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make trailing slashes parseable.
The grammar only accepts "/" db-index, so examples such as moon://localhost/, moons://cache.internal:6380/, and ...?ssl_cert_reqs=required do not parse. This also makes the required round-trip tests impossible. Use ["/" [db-index]] or remove the trailing slashes from every example.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/protocol/moon-uri.md` around lines 41 - 60, Update the moon-uri
grammar’s db-index production to accept an optional slash with an optional
database index, using the equivalent of ["/" [db-index]], so URIs with trailing
slashes and query strings without a database index parse correctly; keep the
existing db-index semantics unchanged.
Comment on lines
+45
to
+48
| userinfo = [ username ] [ ":" password ] ; maps to AUTH [user] pass | ||
| username = *( unreserved / pct-encoded / sub-delims ) | ||
| password = *( unreserved / pct-encoded / sub-delims / ":" ) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the password-colon contradiction.
The ABNF allows : inside password, but the decoding rules say a literal : in a password must be percent-encoded. Define one behavior consistently so parsers and serializers cannot disagree.
Also applies to: 78-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/protocol/moon-uri.md` around lines 45 - 48, Resolve the inconsistency
between the URI ABNF and decoding rules by choosing one colon behavior and
applying it everywhere: update the userinfo grammar and the corresponding
decoding/serialization rules so literal password colons are either consistently
permitted or consistently required to use percent-encoding. Ensure the
definitions at the userinfo grammar and referenced decoding sections agree.
Comment on lines
+229
to
+235
| - [ ] `moon://` and `redis://` parse to byte-identical internal representations for every | ||
| overlapping field (transport=plaintext, host, port, auth, db, shared query params). | ||
| - [ ] `moons://` and `rediss://` parse identically for every overlapping field | ||
| (transport=TLS) **except** default-port behavior, which is intentionally different | ||
| (see grammar) and must be a distinct, explicitly-tested case. | ||
| - [ ] Round-trip: parse → re-serialize → parse is stable (idempotent) for every worked | ||
| example above, for both `moon(s)` and `redis(s)` families. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Define equality without breaking round-trip scheme preservation.
Requiring moon:// and redis:// to produce “byte-identical” internal representations conflicts with parse → serialize → parse preserving which scheme was supplied. Compare normalized overlapping fields, or explicitly retain scheme identity separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/protocol/moon-uri.md` around lines 229 - 235, Revise the checklist to
avoid requiring byte-identical representations that erase the original scheme.
In the `moon://`/`redis://` and `moons://`/`rediss://` comparison criteria,
define equality over normalized overlapping fields while retaining scheme
identity separately for round-trip serialization, and update the round-trip
requirement to verify preservation of that identity.
Comment on lines
+303
to
+307
| - **`--announce-url` / `src/uri.rs` / and the `REPLICAOF host port`-only surface | ||
| (`src/command/connection.rs:639`) confirm this is entirely prospective work** — a repo | ||
| search for `announce_url`, `announce-url`, and `src/uri.rs` returns no matches in this | ||
| checkout. This document is the spec R6 must be built against; do not treat any mention of | ||
| these names elsewhere as already-implemented until R6 lands. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the “no matches” claim to implementation code.
The supplied ROADMAP.md already contains --announce-url references, and this document adds more. Say that no implementation matches exist under src/ and tests, rather than claiming the repository has no matches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/protocol/moon-uri.md` around lines 303 - 307, Revise the documentation
statement to scope the search to implementation code and tests: state that no
matches for announce_url, announce-url, or src/uri.rs exist under src/ and test
directories, without claiming the entire repository has no matches. Update the
affected “prospective work” paragraph in moon-uri.md.
pilotspacex-byte
added a commit
that referenced
this pull request
Jul 14, 2026
…sed] + resolve PR #TBD placeholders (#330) The v0.6.0 tag (355f68d, 2026-07-10) absorbed several PRs that merged after the v0.6.0 release PR itself but before the tag was cut. Their changelog entries were left under [Unreleased], understating what shipped in the tagged v0.6.0 binary. Audit (task #65, v0.7.0 roll-up prep) identified 23 sections whose PR merge commits are verified ancestors of v0.6.0 via `git merge-base --is-ancestor`: PR #248 (FastScan SIMD, SQ8 default, TQ ADC L2 fix, EF_RUNTIME FT.CONFIG, RERANK_MULT+EXACT_BEAM, CLI/moon.conf tuning defaults), PR #250 (13 production-hardening sections), PR #251 (COLD-segment reload off event loop), PR #254 (roadmap docs suite), PR #255 (CI macOS 30m), PR #256 (v0.6.0 ledger closure). Moved verbatim to a new subsection at the top of [0.6.0], with a note explaining the absorption. Sections from PR #252, #253, and #257-#263 were verified NOT ancestors of v0.6.0 and correctly remain in [Unreleased]. Also resolves the "PR #TBD" placeholders left by those sections (now #248, #257, #261) now that the real PR numbers are known. Section-count invariant holds (306 before, 306 after); content unchanged except for the PR-number substitutions. author: Tin Dang Co-authored-by: Tin Dang <tindang.ht97@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ROADMAP v0.6.1 H-7: the authoritative spec
docs/protocol/moon-uri.md(325 lines) for the nativemoon:///moons://scheme — ABNF grammar,redis(s)://parity table, query-param reference (parity TLS/timeout options + native?workspace=), design-for-failure semantics (no-downgrade/no-upgrade/fail-fast/unknown-scheme with exact error strings), server-participation (--announce-url,INFO replication, cluster redirects,REPLICAOF), 10 worked examples, a 16-item conformance checklist for the v0.7.0 R6 implementation, and a file:line implementation-notes appendix. Adds an mkdocs nav entry.Open design gap flagged (for R6c):
?workspace=can't work as speced yet —WS AUTHtakes only a UUID, not a workspace name, thoughget_by_nameexists server-side. The doc flags this rather than assuming.Depends on #260 (roadmap §8.5/H-7 anchors) — merge after that.
Summary by CodeRabbit
moon://andmoons://URI schemes.