docs(site): make the adopter documentation read for a human - #785
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 219c4e9db6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Column accounting reported `classification.column_incomplete` and the
unreviewed-classification finding at the bare container path
`{root}.sourceColumnClassifications`, so a resource with several
unreviewed columns produced byte-identical diagnostics an adopter could
not attribute to a column. Every sibling finding in the same loop, and
the whole statistical column-accounting loop, already interpolate the
column.
The fixed list and search order loops carried the same inconsistency:
`order_property_point` and `order_property_transformed` located an
authored key as `orderBy[{index}]` while the duplicate, optional,
unsupported-type, duplicate-column, unsupported-column and unknown
findings around them fell back to the bare `orderBy`. The
record-identifier tie-breaker and the bound checks stay on the container
path, because they are about the whole order, not one key.
Codes, messages, severities and the set of diagnostics emitted are
unchanged; only locations gain the item they name.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
`sdmx_json_rows` matched the series form with `else if let` and refused
in a final `else { return None; }`. clippy 0.1.97 rejects that shape
under `clippy::question_mark`; clippy 0.1.95, the pinned toolchain, does
not, so the workspace is green today and breaks on the next bump.
Binding the series object with `?` inside a plain `else` arm says the
same thing: a data set that carries neither observations nor a usable
series object decodes to no rows. Behavior, including the refusal for a
`series` value that is not an object, is unchanged and now covered by a
test.
The module is behind the `tooling` feature, so `cargo clippy -p
registry-relay-v2 --all-targets` never compiled it; the lint only
appears with `--features tooling` or in the workspace-wide CI run that
unifies the feature through relayctl.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
`--json` was nearly a no-op: both branches wrote the same pretty JSON document and differed only by a `relayctl <command>` header line, so every adopter running `init`, `inspect`, `check`, `generate`, `test`, `diff`, or `package` read a raw report blob with no summary, counts, or severity order. Render the shared tooling report as plain ASCII by default and keep the JSON document behind `--json`. The renderer lives in the adopter crate, reads the `ToolingReport` value already returned through the one shared seam, handles every `ToolingDetails` variant without a catch-all, and adds no value the JSON does not already carry. Enumeration labels come from serde, so both renderings share one vocabulary. The `--json` document is byte-identical to before; a test pins it against the exact documents for all seven detail kinds. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The replay gates pinned four properties of every tutorial page: an exact sh-fence count, the page's literal strings, the transcript it quotes, and steps addressed by index. Adding one sentence with a command renumbered every later step, so the gates made prose unwritable and produced the ceremonial fences, the reader-hostile `exit 1` guards, and the quoted `printf` echoes that read as machine-written. Address steps by the heading they sit under, and keep only assertions that state a fact about the run: exit status, VERIFIED, the audit lines, the disclosure names. Each retained set carries a comment for why it would otherwise regress in silence. Discovery drops its fence count and fifteen literals for one check that the page still documents the command the gate runs. The production build test trades thirty-one assertions for thirteen stronger ones. Verified by mutation: nineteen seeded regressions, all caught, including a Vault proxy override weakened from "force" to "true" that the previous loose regex accepted. All ten Evidence tutorials replay green. Security review note: this touches signing key handling and release provenance only in test assertions, and tightens them. The transit policy checks now require exportable=false, allow_plaintext_backup=false, and use_auto_auth_token = "force" as exact values rather than as a pattern that any setting satisfied. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Same change of direction as the Evidence and Discovery gates: a writer may add or remove a documented command without editing this script, and the runner simply runs what the page now says. The counts are printed so a reviewer can see them move. Zero stays the one drift signal, because extracting nothing means the heading this reads was renamed and every later step would pass by running nothing at all. EXPECTED_SERVICES, EXPECTED_RUNNING_TOTAL, and EXPECTED_DEMO_ARTIFACTS are unchanged: those describe the demo, not the page. Kept as its own commit. The real run of this gate is currently broken for an unrelated reason, and fixing that is separate work: the pinned SOLMARA_LAB_REF no longer carries compose.evidence.yaml, so the run fails before Docker starts and the three retained totals are unreachable in practice. The ref is untouched here. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The guide was the upstream cause of the shape the gates then froze. It required an expected-output block on every tutorial step, gave a page no slot for what to do when a step fails, rationed the admonitions that warn about real consequences, and closed with four mechanical review questions that a writer can satisfy without the page teaching anything. Tutorials now ask how the reader knows a step worked and what to do when it did not. Procedures get their own section and are exempted from the list rules, because steps are not a list of like things and forcing them into one frame is how a page ends up repeating an opener nobody chose. caution and danger are required where a consequence is real, not rationed. The link cap becomes a judgement: a paragraph too dense to read aloud is a list that has not admitted it yet. A lead names its reader wherever the page has a narrower one than someone using the product. The Writing review is five judgement questions in place of four countable ones, and Tooling now states plainly that the tutorial gates prove the procedure and not the prose, which the preceding commits make true. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The top level named products and page kinds, so a reader arriving with a task had to already know which product owned it. It now names the tasks: answer a bounded question, connect an existing registry, consume and verify assertions, authenticate callers, publish a Discovery index, operate and secure, understand the design. Reference stays a reference. explanation/integration-patterns had two seats and now has one. The durable half is the inverse assertion. The suite already failed on a sidebar seat pointing at a missing page; nothing failed when a published page had no seat, which is how three security pages, two decision records, a threat model and a spec disappeared from navigation without a check going red. A dead seat breaks the build, an unreachable page breaks only the reader, silently. Six of the seven orphans get seats back. start/quickstart is allowlisted with the open question recorded in the test rather than hidden: four redirects still point at it, and whether it becomes the overview for "Consume and verify assertions" or is retired with those redirects repointed is an owner decision. A second test fails if that entry outlives the page. The CLI reference index is still draft, so its sidebar builder returns nothing and seats seven slugs against eighty pages on disk. Publishing it will make this assertion name about seventy-three subcommand pages at once. That is the work, not a fault in the gate, and a comment above the spread says so. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The install command is kept as curl | bash and unchanged. It matches the plurality convention across comparable projects, and adding flags such as --proto '=https' would buy close to nothing against an https-only releases URL while making a tutorial's first command noisier. What was missing was context. The five sites now say the command pipes a remote script into a shell, that | less reads it first, and where the release trust chain is. The register differs by site: a tutorial machine is the reader's own call, a production candidate is not, and the relayctl reference stays terse. The real gap this found was ours. Signed SHA256SUMS and Sigstore bundles are published, which is stronger than most of the field, and the install flow mentioned neither. The sqlite tutorial also now says that a checksum alone catches a truncated download but not a substituted release. Three bare repo paths in reference prose become links to the tag-frozen copy an adopter can open without a clone. Security review note: this is release provenance guidance. The tag-pinned v0.21.0 targets are deliberate and left as they are. Verifying a release against the procedure frozen at that release's tag is the point, which reference/environment-variables.mdx states directly; the security posture pages link main because they describe current practice. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Four pages documented "Evidence Gateway ready at http://127.0.0.1:8080". The binary prints "Evidence ready at" (registry-evidencectl dev.rs). Six other tutorials already quoted it correctly, so this was drift. Three of the four are replayed in CI and all three passed, because the gate asserted its own run's transcript and never compared it against the fence printed on the page. The pages paid the full cost of a machine-verified transcript and bought no verification at all. The preceding gate commit is what stops that class of error from being invisible; this commit fixes the instance it hid. Not adding an assertion for the banner: the next command in each tutorial fails if the service is not ready, so pinning the line would be a transcript pin of the kind just removed. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Four kinds of output on these pages were not what the tools produce. One message was invented outright: the pages showed "Evidence Gateway response verification failed", while every failure path in evidencectl's verify command reports "Evidence response verification failed" with no product name. Two tables were space-aligned inventions of tab-separated output. Three fences quoted the page's own echo and printf back as if a program had said it. The created-project tree omitted the marker file and the fixtures directory that scaffolding writes. Replace the three refusal guards with the plain command a reader would run. The guards existed to turn an expected failure into exit zero for the replay gate, which the gate now expresses directly: run-fails: addresses the fence and requires the non-zero exit, so the proof moves out of the page and into the spec where it belongs. Add recovery sections to the three pages whose failure modes a reader hits without one: a stale dev session, a bound port, and missing retained inputs from the tutorial before. Security review: this changes documented refusal behavior and how the replay gate proves it. The proof is not weakened. Each guard required the command to fail and asserted the refusal message; run-fails: requires the same non-zero exit at the step level, and the asserts still pin the message that proves the refusal had the documented cause. Two asserts move off scaffolding literals the pages printed themselves and onto the real message, which is strictly stronger: a passing run now depends on evidencectl's own output rather than on an echo the page controls. No change to evaluation, signing, or audit. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The guide banned anchor links into other pages and told writers to link to the page and let the reader find the section. The rule cost something real: eleven links across the site point at the section a reader actually wants, and the rule asks for all of them to be blunted. Its safety rationale is already covered. check-built-links resolves every fragment against the built page, so a renamed heading fails the build rather than dropping a reader silently at the top. Move it out of the prohibitions and into the linking rules as a judgement: link to the section when the reader wants that section, link to the page when they need its context to make sense of the part. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The page walked an operator through creating a Transit signing key without saying what the choice binds them to. Add the decisions that have to be settled first: which provider instance and mount hold the identity for its service life, whether the provider's own backup and restore covers that mount, and where the mount, key name, and authorized rotation identity are recorded. Also say why the proxy keeps use_auto_auth_token at "force". Under "force" the proxy replaces any token on the incoming request with its own, so the proxy identity is the only one reaching the provider through that socket. Security review: the irreversibility here is product-side and worth stating plainly. Vault and OpenBao accept exportable and allow_plaintext_backup moving from false to true, and refuse the return trip. Evidence Gateway and Mint accept a Transit key only while both read false, so exporting a key retires that key name from both services permanently, for every version under it. The recovery path is a new key and a rotation, not an export. No change to key handling in code; this documents the constraint the runtime already enforces. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Seven pages ended without telling the reader what follows, which on a configuration page means the reader has a working service and no idea what the next decision is. Add a Next section to each. Link Mint from the two Relay pages at the points where a reader needs a token issuer and has no way to know which service issues one. Put Troubleshooting before Next on the OID4VCI page, matching every other page in the section. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The governed-SQLite tutorial pinned a schema fingerprint and two inventory digests from the machine that wrote the page, told the reader to paste the pinned value into their contract, and then told them a paragraph later to paste their own if it differed. The fingerprint covers the schema statements SQLite stored, so anyone who retypes the DDL rather than copying it gets a different value and the first instruction sends them into a mismatch. Use placeholders, keep the instruction that was right, and say what the fingerprint actually covers. Split the 123-line contract into eleven blocks with the explanation beside the fields it describes, rather than three paragraphs trailing a wall of YAML. The reassembled contract parses to the same object and keeps the same contract revision. Correct what these pages claim the tools print: the file write order and count for a suggested source, that a collision writes nothing at all rather than stopping partway, that relayctl prints its command name above each JSON report, and that Relay logs line-delimited JSON. Mark every abridged fence as abridged and name the fields left out, instead of a bare ellipsis. Fix a verb that told the reader to open a file the tutorial has not created yet, and drop an artifact count that no longer matches the tool. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The page walked a reader through the compose adapter without ever starting it. Add the step that does: bring the stack up, list what is running, and confirm the service logged that it is listening. The image supplies the runtime path and the serve command, and the published port is commented out in the compose file, so the listener is container-private; say both rather than implying a reader can reach it from the host. Rewrite the checks a reader could not perform. Where the page asked them to observe that no new request appeared, use the audit volume, which survives down and can be listed, and warn that down -v destroys the hash-linked chain. Security review: the validation step described proving runtime dependencies but ran plain check, which does not. Corrected to pass --require-runtime-dependencies, matching the maintained invocation in docker/compose/README.md, and the page now separates what plain check proves from what the flag adds. An adopter following the old text believed audit writability, signer readiness, source credentials, and JWKS reachability had been proven in the container context when none of them had been. The inline compose snippet remains a partial copy: it shows mounts but omits cap_drop, no-new-privileges, and the read-only /dev/shm that the maintained compose file sets. The page now points at that file for the rest of the service posture, but a reader who copies only the snippet still gets a weaker container. Flagged for whoever owns this page's long-term shape. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The retained-assertion test required the refusal spec to assert "TAMPER REFUSED", which was the page's own echo rather than anything a tool said. By the test's own standard that assertion was empty: the echo runs whether or not verification refused, so it survives the regression it exists to catch. Point it at evidencectl's real message and say so in the comment. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Twenty-one references told the reader to look above or below. Position is not a stable address: it assumes the reader arrived at the top and read down, which is false for anyone who followed a search result, a deep link, or the on-this-page index, and it breaks silently whenever a section moves. Name the thing instead. Where the target is a section on the same page, link to it, so a renamed heading fails the build rather than leaving the reader to hunt. Where the reference was to the page as a whole, say so plainly. Left alone: uses inside MDX maintainer comments, hierarchical uses about files below a root, numeric uses about a count above a ceiling, and one draft page. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
"the table above" and "the refusal below" only work while the reader is
scrolling one long file top to bottom. These documents are also read as
rendered pages, quoted in reviews, and pulled into the docs site, where a
positional word points at nothing. Name the section, the heading, or the
metric instead.
Every substituted name was checked against a real heading in the same file.
Numeric ("above the ceiling") and filesystem ("below the secret root") uses
are left alone; they are not positional.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The three public-demo tutorials passed a synthetic subject identifier as `--subject role:field=value`, which puts it in the process arguments where any user on the host can read it from `ps`. `evidencectl` already accepts `--subjects-file`, and refuses it together with `--subject`, so the request can name its subjects in an owner-only file instead. Each page now writes that file with `os.open(..., O_EXCL, 0o600)` and states the file conditions `evidencectl` enforces: a regular file the caller owns, one link, mode 0600. The DHIS2 page gains a discovery step that finds a qualifying synthetic child through the Tracker API and writes the identifier straight to a local file, so no identifier reaches the reader's screen or shell history; the previous instruction was to browse the demo UI until a record with all five immunization readings turned up. The audit sections quote what `evidencectl audit show --last-operation` prints and then read those lines for what they omit, which is the claim the product actually makes. Each page gains a troubleshooting table for the failures a public demo produces: reset data, rotated credentials, daily request limits, and version-path changes. Review notes (data minimization): no new identifier is disclosed. The permitted synthetic Josh Hoeger national ID moves from a command line into a 0600 file. The DHIS2 discovery script prints a record count and never an identifier. Two argv exposures remain and are deliberate: the DHIS2 page reads its boundary with the UID in a curl URL, and the parent page passes the permitted literal on argv where no reader value is involved. The prose claims only what each change delivers rather than a blanket argv guarantee. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Visual inspection of the running site turned up four defects. The sidebar hid every disclosure caret. Collapsing is how this menu now carries its depth, and four top-level groups are collapsed as well, so a reader had no mark telling them a heading opens. Restore the caret and seat it beside the label rather than at the pane edge. Rhai code blocks rendered as flat unhighlighted text beside fully coloured YAML on the same page, which reads as a broken block. Shiki ships no Rhai grammar; Rhai borrows Rust's surface syntax, so aliasing it to Rust colours it correctly. The setting lives in a new ec.config.mjs because starlight-openapi replaces Starlight's expressiveCode key wholesale before it reaches Expressive Code. The DHIS2 tutorial was labelled "(under review)" in the menu while the page itself carries no such notice, so the two contradicted each other. Evidence pages cited repository paths a reader cannot open. Point them at the published pages instead, keeping bare paths only where the sentence quotes that exact file. Rename "When Registry Stack fits" to match the menu seat a reader clicks. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Three tutorials carried `draft: true`, which excludes a page from the build entirely, so readers could not reach them at all. Two are now published. `issue-fhir-evidence-as-vcs` is replayed in CI by `check-evidence-tutorials.sh`, whose spec addresses fences by heading and ordinal, so its fenced commands are load-bearing. Only prose changed; the concatenated fence bodies hash the same before and after. `query-relay-client` used a `Do` / `See` / `Understand` / `Adapt` scaffold no other page on the site uses, which reads as a generated worksheet rather than a tutorial. Its methods, arguments, and return shapes were checked against `registry_relay_client`'s type stub and the Rust behind it before publishing, which is the reader-mode verification the page's own author note asked for. `request-a-holder-bound-credential` stays draft. Its harness exclusion records that no verified wallet flow exists to replay, it needs a wallet or signing harness the reader must supply, and the page states that the local authoring workspace cannot yet declare a binding mode on an authored question. `start/quickstart` is retired. It was a second product chooser beside `start/when-to-use`, unseated and unlinked, and the unseated-page allowlist in `information-architecture.test.mjs` recorded that as an open owner decision. Its four inbound redirects and its own published route now land on the chooser that stayed. The deployment smoke test's default deep route, the accessibility checker's optional critical path, and a staging fixture all named the retired route and now name a live one. Troubleshooting precedes Next on every tutorial carrying both, so a reader who hit a problem gets the fix before being sent onward, and the tutorial that opens the consumer section now says whose side of Evidence the reader is on. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The FHIR page's live-run date read as a contradiction of its review date; "last run end to end" names it as a run date instead. `relay --version` appends `-dev` to every build outside the release workflow, so a reader who built from source got a wheel filename and a release URL that both 404. The install now stops on that version with a message the troubleshooting table explains. The Compose snippet carries mounts and identity only, and the prose said so, but nothing in the snippet did. A copied block now carries its own marker. The OpenCRVS payload example mixed literal synthetic names with angle brackets and never said which was which. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
relayctl renders human-readable output by default and keeps JSON behind --json, so the six report blocks in the governed SQLite tutorial no longer described what a reader sees. All six were also stale against the previous build, because they had been written by hand rather than captured. Every character inside the six blocks is now copied from a real run. The tutorial was replayed end to end from its own steps: its SQL built the database, its YAML blocks assembled the contract and the governed files, and relayctl init, inspect, check, generate, check --production, and package produced the output the page shows. The fences say text instead of json. Every digest on the page is a real value. Each one derives only from inputs the page itself commits, and two independent replays in different directories produced identical digests, so a reader following the page gets the same values rather than a placeholder. Prose that only existed to explain the JSON blob is gone: the abridgement notes for the object list, the configuration key paths, and the diagnostic severity, plus the claim that each command prints its own name first. The diagnostic codes in the troubleshooting table were each reproduced against the same build and all of them still hold. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
relayctl no longer prints a `relayctl <command>` header line, so the reference page's description of the default output was false. Replace it with what the binary does now: a readable rendering that summarizes the report, with `--json` printing the report whole. Name the two places the summary elides detail, so the per-command sections that describe the report stay accurate. Fill in the real `contractRevision` in the tutorial's served record and audit line. The value is captured from a live `relay serve` replay, not written by hand, and it matches the revision the tutorial's own check and package reports print, which is what the surrounding prose promises. Leave `sourceRevision` as a placeholder. It is a SHA-256 over the SQLite file bytes rather than over the tutorial's committed inputs: building the same database with a different SQLite build changes byte 20 of the header (reserved bytes per page) and so changes the digest, and a reader cannot expect to reproduce it. Verified against relayctl: no header line in either mode, output byte-identical across repeated runs, and exactly one trailing newline in both modes. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The public identifier catalog pins a digest of products/relay-v2/CONCEPT.md, so editing that file leaves the generated catalog stale and products/identifiers/scripts/check.sh refuses. Produced by the documented generator, not by hand. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
REQ-PR-RELAYCTL-026 still required a `relayctl <command>` header followed by the report content, which the readable rendering replaced. The contract now described output no implementation produced, so the specification's `verified` evidence claim was false for that sentence. State the requirement the rendering actually meets: a human-readable summary derived from the workflow report, carrying the status and every diagnostic, and never the report document itself. `--json` is unchanged and stays pinned by its byte-identical regression test. Cite report.rs as the evidence for the summary, since rendering no longer lives in lib.rs alone. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The page installs relayctl from the mutable releases/latest URL, and no gate replays it, so the compiler-derived values it printed were only ever true for the release that produced them. A reader on any later release would see a different contract revision and different key-path counts and reasonably conclude the tutorial had failed. Show placeholders for the values that move with the release: both contract revisions, the package revision, the key-path counts, and the package artifact and file counts. Say once, where the first one appears, that a real run prints real values and why pinning them here would not help. Keep the values a reader can actually reproduce: the schema fingerprint and object count come from SQL this page supplies verbatim, and the error and file counts are enumerated on the page itself. The package format identifier stays exact. It names a format, not a digest. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
SQLite places no restriction on what an identifier or a declared type may contain, so a schema object name, column name, or declared type read from an adopter's database can carry a line break, a terminal escape sequence, or a Unicode bidirectional override. The human-readable report rendered these values verbatim, so a hostile schema could forge a report line such as a counterfeit "0 errors, 0 warnings.", emit terminal escape sequences, or reorder the text drawn around it. Escape those characters to a visible all-ASCII form before rendering, and compute the column alignment widths from the escaped text so the layout still matches what is printed. Ordinary printable Unicode, including non-English identifiers, is unchanged. Security-sensitive: this is an output-integrity fix in adopter tooling. It changes no authentication, authorization, assertion evaluation, signing, or audit path, and no released version is affected. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Column-accounting diagnostics reported locations under `sourceColumnClassifications.<column>` even when the adopter never authored an entry for that column, because the classification was inherited from `classificationDefaults`. `sourceColumnClassifications` is optional, so the reported path frequently named a YAML node that does not exist. The language server resolves a diagnostic location against the document and falls back to the start of the file when it cannot, so an adopter was sent to line 1 instead of to the text the diagnostic is about. Report the inherited failures at `<resource>.classificationDefaults`, which the contract requires and which therefore always resolves, and keep the column-specific path only where the adopter actually wrote an override. Name the affected source column in the message so the diagnostic still identifies the column when the location points at the shared default. Covers both the resource and statistical column-accounting paths. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
02d6c73 to
5c9cdfd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c9cdfd1d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A relayctl human report interpolates adopter-supplied text from more places than the schema listing: a diagnostic carries an authored key path in `location` and often an authored source-column name inside `message`, and both were written verbatim. That exposure predates the schema-name escaping, since `location` already interpolated a column name before it. Escape each finished line in `render_human` instead of at each call site. A report line is built from spaces and report text only, so one boundary neutralizes every value whatever produced it, and text already escaped for column alignment is plain ASCII and passes through unchanged. Security-sensitive: this is terminal-output sanitization for values that reach a maintainer's screen from an adopter's SQLite schema or authoring document. It changes no authorization, evaluation, signing, or audit path. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35961f64f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.validate_review_status( | ||
| &classification, | ||
| &format!("{root}.sourceColumnClassifications"), | ||
| &column_accounting_location(root, column, source_override, None), | ||
| Some(column), |
There was a problem hiding this comment.
Point direct-property diagnostics at the property
When a scalar property has one untransformed source binding, no source-column override, and an unreviewed property classification while classificationDefaults is reviewed, the branch at compiler.rs:3481 derives this classification from the property, but this call locates the resulting column-accounting diagnostic at resources[...].classificationDefaults. The output therefore tells the adopter to edit a field that is already reviewed instead of the authored property classification. Fresh evidence beyond the earlier inherited-default finding is this direct-property branch, where source_override == None does not mean the classification came from the defaults; preserve the actual classification source when selecting the location.
Useful? React with 👍 / 👎.
| '\u{0000}'..='\u{001f}' | ||
| | '\u{007f}'..='\u{009f}' | ||
| | '\u{200e}'..='\u{200f}' | ||
| | '\u{202a}'..='\u{202e}' | ||
| | '\u{2066}'..='\u{2069}' |
There was a problem hiding this comment.
Escape Unicode line separators in reports
When relayctl inspect encounters a quoted SQLite identifier containing U+2028 LINE SEPARATOR or U+2029 PARAGRAPH SEPARATOR, this predicate leaves the character raw, so Unicode-aware terminals, log viewers, or copied report text can still render the attacker-controlled name as a new visual line. Fresh evidence beyond the earlier escaping finding is that the new centralized sanitizer covers C0/C1 and selected bidi controls but omits these Unicode line-breaking characters; include them in the escaped set and add a hostile-schema fixture for them.
Useful? React with 👍 / 👎.
| The wheel uses the Python 3.10 stable ABI, so supported newer Python versions import the same | ||
| file. The three platform names in that `case` are the whole set: the release provides no Windows | ||
| or Intel macOS wheel. |
There was a problem hiding this comment.
Select the published manylinux wheel names
For Relay releases at or after 0.21.1, Linux readers following this newly current tutorial receive a GitHub 404: release/scripts/release_candidate.py switches the published wheel inventory at CLIENT_REGISTRY_PACKAGE_MINIMUM_VERSION to manylinux_2_17_x86_64.manylinux2014_x86_64 and its aarch64 equivalent, while the command still constructs linux_x86_64 or linux_aarch64. The added statement that these are the release's complete platform names is therefore false for the current release line; select the filename according to the release boundary or use the published manylinux names.
AGENTS.md reference: docs/site/AGENTS.md:L26-L29
Useful? React with 👍 / 👎.
| contract, so none of them can describe a field the contract does not disclose. Running `generate` | ||
| again on an unchanged project rewrites the same bytes. |
There was a problem hiding this comment.
Explain that regeneration needs a fresh output directory
After the first relayctl generate . creates generated/, running the command again does not rewrite those bytes: generation_destination_diagnostic returns generation.destination_not_empty whenever the output directory contains an entry. This newly added sentence therefore sends readers into a guaranteed refusal when they regenerate after changing the contract, including the recovery suggested later on the page; tell them to remove, archive, or select a fresh output directory before rerunning generation.
AGENTS.md reference: docs/site/AGENTS.md:L26-L29
Useful? React with 👍 / 👎.
Makes the adopter-facing documentation read like it was written for a human,
and fixes the two tool behaviors that review found were making that hard.
Twenty-four commits in three strands, kept in that order because the later
ones depend on the earlier ones being true.
1. Relay diagnostics name what they are about (2 commits)
compile_contractemitted runs of byte-identical diagnostics pointing at abare
resources[0].sourceColumnClassifications, so a reader could not tellwhich column each refusal was about. Fourteen sites now name the column or the
order position. The standard for changing a site was that a sibling
diagnostic in the same loop already emitted a precise location, which makes
the bare one an oversight rather than a design choice; five further families
with no such precedent were deliberately left alone.
Also drops a trailing refusal arm in the SDMX series decode that
clippy::question_markrejects. That lint only reproduces under--features tooling, which is why it had gone unnoticed.2.
relayctlrenders reports for people by default (1 commit)--jsonpreviously changed nothing but a header line: both branches printedthe same pretty JSON, so every command answered a person with a machine
document. The default is now a readable rendering;
--jsonremains themachine mode and its bytes are unchanged.
crates/registry-relayctl/src/shared.rsstays the single seam into Relaysemantics: the renderer names no Relay module of its own, and the contract
test asserts that.
3. Documentation written for a reader (21 commits)
Sidebar reorganized by what a reader arrives wanting to do. Tutorials stop
reading like scripts for a machine to execute: quoted output is what the
tools actually print, the install one-liner explains itself, readers are no
longer asked to copy a digest that cannot be theirs, and demo identifiers are
kept off the command line.
Verification
Run on the combined branch, not on the strands separately.
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo check --locked --workspace --all-targetscargo test --locked --workspaceproducts/relay-v2/scripts/check-contracts.shproducts/evidence/scripts/check-contracts.shproducts/evidence/scripts/check-source-neutrality.shproducts/evidence/scripts/check-verifier-portability.shproducts/evidence/scripts/check-config-key-paths.shdocs/sitenpm testdocs/sitenpm run checkToolchain: Rust 1.95.0, the
rust-toolchain.tomlpin.New tests were written before the fixes and observed failing first. The
location assertions were mutation-tested: stripping the column qualifier or
the order index back off makes them fail, including one pre-existing test
that already asserted the indexed form.
Known red, pre-existing
cargo deny checkreportsadvisories FAILEDon RUSTSEC-2026-0258 (h2unbounded empty DATA frames). This is not introduced here: it fails
identically on
main,deny.tomlcarries no ignore for it yet, and thisbranch touches no
Cargo.toml,Cargo.lock, ordeny.toml. Decidingbetween an upgrade and a scoped ignore is security-sensitive and belongs in
its own change with its own review notes.
Security review notes
None of the categories that require them are touched: no authentication,
authorization, assertion evaluation or signing, audit integrity, release
provenance, deployment defaults, or data minimization. The diagnostic changes
alter configuration key paths in refusal messages, never source values. The
renderer reads the same report the JSON mode serializes and adds no Relay
semantics.
Related
Filed #784 while verifying the reference:
relayctl.report.v1is named inthe docs but exists nowhere in the source tree. Left untouched here, since
fixing it needs a product decision.