fix(docs): close six 1.0 docs-truth gaps - #626
Conversation
Only the inbound POST /federation/v1/evaluations route ships; no outbound Notary-to-Notary client or peer-result composition exists in the workspace. Point outbound composition at GH#36. Refs #586 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…aims postgres_state_plane_v1.sql holds no audit table: audit correctness lives in the hash-chained pipeline with operator-owned sink retention and off-host shipping. Restoring the database does not restore audit history. Refs #584 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The docs gate shallow-clones dozens of pinned refs; one dropped connection failed the whole `npm run check` and the real git error was discarded behind "Command failed: git fetch ...". Wrap only the network fetch in a bounded 3-attempt linear backoff and carry git stderr, bounded, into the failure. Retries are unconditional: permanent and transient git stderr are not reliably distinguishable across versions and transports, and a wrong heuristic costs more than a redundant retry. Also guards fetch-openapi.mjs behind a direct-invocation check, matching sync-repo-docs.mjs, so tests can import its helpers without running a real fetch. No skip flag and no second verification mode. Refs #294 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The 13 content diagrams are authored with a fixed light palette on a transparent canvas, so inheriting --registry-surface (#1b1e24 in dark theme) left dark text on a near-black background. Pin content diagram SVGs to a literal white surface in both themes rather than shipping dark variants. Add a WCAG contrast regression to check-svg-a11y.mjs, which npm run check already runs, so a future palette drift fails the gate instead of shipping. All 13 assets clear 4.5:1 today (worst is #6a6a6a at 5.41:1). Refs #291 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
RS-DOC gains REQ-DOC-014: a specification marked evidence: verified must anchor each load-bearing claim to an artifact a reader can open directly (source, test, fixture, generated artifact, or released machine contract). A site page may corroborate an anchor but must not be the only evidence, since evidence resolving solely to further documentation is a citation loop rather than verification. Rework the Evidence sections of the specs that leaned on site pages so they cite workspace source, tests, and generated machine contracts. rs-op-posture previously shipped as verified while admitting in its own Evidence section that source evidence was pending; it now cites the posture JSON Schema, the waiver-metadata validator, and the profile-gates contract test. Refs #237 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
EvaluationProfileManifest carried max_source_observed_age_seconds as an optional public freshness hint whose documentation promised that "runtime enforcement is in Registry Notary config". No such enforcement exists. Notary derives freshness from verified Relay consultation provenance and retains only the consultation acquired_at timestamp; its one freshness control is the federation profile max_claim_result_age_seconds, which bounds consultation-result age. Snapshot freshness is owned by Registry Relay through max_snapshot_age_ms. The field configured nothing. Because the struct carries #[serde(deny_unknown_fields)], removing it makes the key a hard parse error rather than a silently ignored one. This is a registry-manifest/v1 format break, deliberate over leaving a field that tells adopters an enforcement story the runtime does not implement. Drop the key from the five hand-maintained demo manifests and the Relay entity-route tests, and correct the prose that claimed the enforcement. Occurrences of max_source_observed_age_seconds under an evidence pack policy are a different key in a different namespace: EvidencePackMetadata policy is opaque unvalidated JSON, so those are left as authored. The change spans four owning areas rather than one. crates, products, and docs/site are forced by the generated-output rule, since the manifest reference mirror regenerates from products. The release entry is a stale claim of the same kind: it quoted a source_observed_at_field section of the Notary operator config reference that no longer exists, and cited a Relay configuration heading that does not exist either. Refs #583 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The SVG accessibility checker had two defects in how it handled <style>. It resolved class-based fills from only the first <style> element, so an SVG carrying a second one lost those rules: its labels resolved to no fill and were reported as unresolvable, failing CI on a valid diagram. Every block is now read, later rules winning as CSS source order does. It also deleted <style> blocks from the text before scanning for painted content. Removing a substring cannot be done safely in one pass, because a removal can splice its neighbours into a fresh `<style` and leave behind exactly what the removal was meant to drop. Style regions are now skipped during the tokenizer walk instead, which needs no rewriting and cannot resurrect a tag. CodeQL reported the deletion as incomplete multi-character sanitization. The injection framing does not apply: this is a build-time linter over trusted in-tree diagrams with no rendering sink. The incompleteness was real, so the sanitize-shaped code is gone rather than patched. Belongs to the #291 commit; kept separate to avoid rewriting a pushed branch under review. Refs #291 Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
eaf099c to
80e9b2f
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e41986a882
ℹ️ 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".
| 5. **Claim evaluation.** Registry Notary evaluates claims from compiler-pinned Relay consultation outputs. Caller identity and authorization details may authorize or parameterize a consultation, but do not become evidence. Notary applies disclosure policy (value, predicate, or redacted) and returns evaluation results as claim-result JSON or CCCEV-shaped JSON-LD. It can materialize only stored evaluations with exact dependency-closure claim pins and normalized unique Relay execution records into SD-JWT VC credentials. | ||
|
|
||
| 6. **Static-peer delegated evaluation.** A trusted Registry Notary instance can call another trusted Registry Notary instance through `POST /federation/v1/evaluations` for signed delegated evaluation. Registry Manifest can publish discovery metadata for that relationship, but local Notary peer policy grants access. Peer lists are loaded from configuration at startup. This federation path is distinct from delegated subject access in the Notary citizen/OIDC flow. | ||
| 6. **Static-peer delegated evaluation.** Registry Notary accepts signed delegated evaluation from a trusted peer on `POST /federation/v1/evaluations`. Only this inbound route ships: the signed request is constructed by a separate trusted caller, because no outbound Notary-to-Notary client and no peer-result composition runtime are part of this version. Outbound calling and peer-result composition are planned and tracked in [GH#36](https://github.com/registrystack/registry-stack/issues/36). Registry Manifest can publish discovery metadata for that relationship, but local Notary peer policy grants access. Peer lists are loaded from configuration at startup. This federation path is distinct from delegated subject access in the Notary citizen/OIDC flow. |
There was a problem hiding this comment.
The claim that there is “no outbound Notary-to-Notary client” contradicts registry-notary-client: with its federation feature, RegistryNotaryClient::federation_evaluate_jws posts an already-signed JWS to this exact route, and client_contract.rs tests that behavior. The server has no autonomous request construction or peer-result composition, but a supported outbound transport client does exist, so describe that narrower runtime limitation instead.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| evaluations, and credential status. Explicit in-memory state MAY be used | ||
| only for local single-process development. Audit records are not part of that |
There was a problem hiding this comment.
This changes the normative scope of REQ-PR-NOTARY-041 by removing audit from the PostgreSQL correctness state and adding new restoration and replica obligations, but the document's version history still ends at 0.4.0. RS-DOC REQ-DOC-012 requires every normative-requirement change to add a semantic-version history row; the parallel REQ-ARC-G-013 edit has the same omission, so consumers cannot identify which contract version introduced these requirements.
Useful? React with 👍 / 👎.
| - `scripts/check-doc-frontmatter.mjs` enforces the `doc_id`, `category`, and `evidence` fields for `doc_type: specification` (Sections 3 and 4). | ||
| - `styles/RegistryDocsSpec/` and the `[src/content/docs/spec/**]` Vale section enforce the honest-conformance discipline (Section 2). | ||
| - `src/content.config.ts` defines the specification frontmatter; `astro.config.mjs` defines the sidebar group. | ||
| - `docs/site/scripts/check-doc-frontmatter.mjs` enforces the `doc_id` pattern and the `category` and `evidence` values for `doc_type: specification` (Sections 3 and 4). |
There was a problem hiding this comment.
In the published site, these backticked workspace paths are plain text rather than links, so a reader without a local checkout cannot open the artifacts that the newly tightened REQ-DOC-014 says must be directly inspectable. The same pattern was introduced across the rewritten Evidence sections; replace the source-repository paths with full URLs pinned to this release or commit, as required by docs/site/docs/style-guide.md, while retaining site-local links for generated artifacts.
AGENTS.md reference: docs/site/AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
| const r = parseInt(normalized.slice(1, 3), 16); | ||
| const g = parseInt(normalized.slice(3, 5), 16); | ||
| const b = parseInt(normalized.slice(5, 7), 16); |
There was a problem hiding this comment.
When SVG text uses a valid non-six-digit color syntax such as fill="white", fill="rgb(255,255,255)", fill="none", or fill="#0000", these parseInt calls produce NaN or ignore the alpha channel; the later ratio < 4.5 comparison is then false, so invisible or low-contrast text passes the accessibility gate. Normalize and composite every supported SVG color syntax, or report unsupported and translucent fills as unresolved, rather than treating a non-finite ratio as success.
Useful? React with 👍 / 👎.
| PostgreSQL state: audit correctness is owned by the hash-chained audit pipeline | ||
| that writes to the configured sink (REQ-PR-NOTARY-020, REQ-PR-NOTARY-021), and |
There was a problem hiding this comment.
For an implementer following the published protocol, the cited REQ-PR-NOTARY-020 only requires emitting an EvidenceAuditEvent to a configured sink and REQ-PR-NOTARY-021 only requires failing on write errors; neither requires a hash chain. This newly added text nevertheless says those requirements define a hash-chained pipeline, so an implementation can satisfy the cited requirements without the integrity property REQ-PR-NOTARY-041 now relies on. Add the hash-chain obligation to the normative protocol contract or stop attributing that property to these requirements.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
The contrast checker scored fills by parsing hex digits. A valid SVG paint value it could not parse (`white`, `rgb(255,255,255)`, `currentColor`, `none`) produced NaN, and `NaN < 4.5` is false, so the gate passed the file in silence. `#rrggbbaa` was worse than silent: the alpha channel was sliced off, so half-transparent black scored 21:1, the maximum. Only #rgb and #rrggbb are scoreable now, and anything else is reported as an error naming the offending value. Scoring the rest would mean shipping a named-color database and alpha compositing rules for a build-time linter. No shipped diagram currently resolves a <text> to an unscoreable fill, so this closes a latent hole rather than fixing an active miss. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…ayer Three review findings, all cases of a sentence claiming more than the code or the cited requirement supports. RS-ARC-G said no outbound Notary-to-Notary client is part of this version. `registry-notary-client` ships a feature-gated `federation_evaluate_jws` that posts to that route, so the denial was false. The true and narrower facts stand: the helper transports an already-signed JWS and cannot construct or sign one, the Notary server does not enable the feature, and no peer-result composition runtime exists. REQ-ARC-G-013 and REQ-PR-NOTARY-041 attributed hash chaining to REQ-PR-NOTARY-020 and REQ-PR-NOTARY-021. Those two require emission to the configured sink and failing the request on a write error; neither mentions chaining, and no requirement anywhere makes it contractual. The pipeline does hash-chain, so that stays as a statement of fact, and the two IDs are now cited only for what they establish. Both requirements changed obligations without a version-history row, which REQ-DOC-012 requires unconditionally. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
|
Four of the five review comments were right and are fixed in the two commits just pushed. One is declined, with reasoning below. Each finding was checked against the source before acting, not taken on the reviewer's word. FixedDistinguish the client crate from the server runtime ( The denial is gone. What replaces it is the narrower set of facts that survives inspection: the helper transports an already-signed JWS and cannot construct or sign one, the server does not enable it, and no peer-result composition runtime exists. Reject unsupported SVG colors before comparing contrast ( Only Tested red-first: six unscoreable paint values ( Worth stating plainly: no shipped diagram currently resolves a Add version-history rows for the changed requirements ( Define the claimed hash-chain requirement normatively ( The implementation genuinely does chain ( DeclinedLink evidence anchors to pinned source artifacts ( The style guide does say it, twice, verbatim: "Pin links to code to a release tag ( The convention in this layer is the opposite. Across the ten spec pages there are 92 bare backticked source paths versus 10 pinned URLs. Fifteen of the bare ones sit in Nothing enforces it. And SHA-pinning specifically collides with AGENTS.md: "History may be rewritten during review (session commits get squashed). In durable docs, cite only commits reachable from pushed So the honest sequencing is: keep bare paths now, and if clickability is worth acting on, convert to the tag-pinned pattern already used in Two follow-ups fall out of this and are worth tickets rather than scope creep here: two anchor conventions coexist across the spec layer with nothing deciding between them, and there is no automated gate at all over spec-page Evidence sections, unlike the standards and contracts data layer. Extending CodeQLBoth alerts ( Verification
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 579179f224
ℹ️ 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".
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub max_source_observed_age_seconds: Option<u64>, | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub evidence_pack: Option<EvidencePackMetadata>, |
There was a problem hiding this comment.
Any existing registry-manifest/v1 document containing the previously valid optional max_source_observed_age_seconds field now fails deserialization because this struct uses deny_unknown_fields. That contradicts the published V1 compatibility contract in products/manifest/docs/reference.md, which requires a new schema version when a change makes previously valid V1 manifests invalid; introduce a new manifest schema version or retain acceptance of the deprecated field during V1 parsing.
Useful? React with 👍 / 👎.
| consultation `acquired_at` timestamp, the only Notary freshness control is the federation | ||
| profile's `max_claim_result_age_seconds` (which bounds consultation-result age), and Registry |
There was a problem hiding this comment.
The assertion that max_claim_result_age_seconds is Notary's only freshness control is broader than the shipped behavior: subject_access.token_policy.max_auth_age_seconds and max_evaluation_age_seconds are enforced for subject-access flows, and oid4vci.proof.max_age_seconds bounds wallet proof age. In deployments using those flows, this limitation page now incorrectly implies those controls do not exist; scope the statement specifically to source-observation or Relay-evidence freshness.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| const fillMatch = body.match(/fill:\s*(#[0-9a-fA-F]{3,8})/); | ||
| if (fillMatch) classFills.set(className, fillMatch[1]); |
There was a problem hiding this comment.
When a class rule contains repeated fill declarations, this match always records the first one even though CSS applies the last declaration. For example, .label { fill: #161616; fill: #cccccc; } is rendered with low-contrast #cccccc, but the checker records #161616 and reports no error, allowing an unreadable diagram through the new accessibility gate; resolve the effective final declaration rather than the first textual match.
Useful? React with 👍 / 👎.
A class rule may declare fill more than once. CSS paints the last
declaration; the checker read the first, so `.label { fill: #161616; fill:
#cccccc; }` scored the legible color while the diagram rendered the
illegible one. The failure direction is the dangerous one: a legible fill
written ahead of an illegible one hid the illegible one from the gate.
Cross-rule precedence was already correct, and a test now pins it so the
two halves of CSS source order cannot drift apart.
No shipped diagram declares fill twice in one rule, so this closes a latent
hole rather than fixing an active miss.
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: 159fc4c75a
ℹ️ 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".
| for (const declaration of body.matchAll(/fill:\s*(#[0-9a-fA-F]{3,8})/g)) { | ||
| effectiveFill = declaration[1]; |
There was a problem hiding this comment.
When a CSS rule uses a hex fallback followed by a valid non-hex override, such as .label { fill: #161616; fill: white; }, the browser renders the final white value, but this loop matches only hex declarations and retains #161616, allowing white-on-white text to pass. The fresh evidence after the earlier unsupported-color and last-declaration comments is their interaction in this updated regex; collect every fill value, then reject the effective final value if it is unscoreable.
Useful? React with 👍 / 👎.
| const classFill = classAttr | ||
| ? classAttr.split(/\s+/).map((name) => classFills.get(name)).find(Boolean) | ||
| : undefined; |
There was a problem hiding this comment.
When a <text> element has multiple classes that each define fill, CSS chooses by specificity and stylesheet source order, not by the order of names in the class attribute. This find(Boolean) instead always selects the first class, so <text class="safe danger"> can be scored with a high-contrast .safe fill even when a later .danger rule is the low-contrast color actually rendered. The fresh issue in the updated resolver is this cross-class cascade shortcut; resolve the winning CSS declaration before scoring.
Useful? React with 👍 / 👎.
| // Resolving the actual painted rect behind each label would mean | ||
| // geometric analysis; the tradeoff is that white-on-white text, which no | ||
| // diagram would intend, goes undetected here. | ||
| if (color === '#ffffff') continue; |
There was a problem hiding this comment.
When white text is added anywhere other than the currently known colored chips, this unconditional exemption skips contrast validation even though the new CSS pins the diagram surface to white. A misplaced label such as <text fill="#ffffff"> on the canvas therefore passes the accessibility gate at 1:1 contrast; associate reverse text with a verified non-white background instead of exempting the color globally.
Useful? React with 👍 / 👎.
…scade
Three remaining ways the contrast gate could pass unreadable text.
A rule's last fill declaration wins even when it is not hex, so
`.label { fill: #161616; fill: white; }` renders white. Reading hex
declarations only kept #161616 and hid it; any value is captured now and an
unscoreable one is reported.
A <text> carrying several classes was scored with whichever class was named
first in the attribute, but CSS picks by specificity then source order.
Every selector this parser handles is a single class, so specificity ties
and the later rule wins; rule order is now tracked and used.
White text was exempted outright, which passed a stray label on the canvas
at 1:1. It is now scored against the shape drawn before it, so the
exemption becomes a measurement. Document order stands in for geometry,
which is a convention rather than a layout computation, so a label with no
shape before it is reported rather than assumed safe.
None of the three occurs in a shipped diagram today.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…xist The page said `max_claim_result_age_seconds` is the only Notary freshness control. Notary also enforces `max_auth_age_seconds` and `max_evaluation_age_seconds` on subject-access flows and `oid4vci.proof.max_age_seconds` on wallet proofs, each with a test, so on a deployment using those flows the page denied controls that were running. The bullet's actual point is unchanged and still holds: nothing in Notary bounds the age of the observation behind a claim. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
|
All three of the latest comments were right and are fixed. The Track the actual final fill declaration — correct, and it is an interaction between my own two previous fixes rather than a fresh defect in either. Capturing only hex declarations meant Apply stylesheet order across multiple text classes — correct. Validate white text against its actual background — correct, and the better fix was deleting the carve-out rather than narrowing it. Reverse text is now scored against the shape drawn before it, so the blanket exemption becomes a real measurement: white on One honest limitation, stated because it is the kind of thing that should not be discovered later: document order stands in for geometry. In these diagrams the chip is always drawn immediately before the label that sits on it, which is a convention, not a layout computation. Resolving which shape is actually behind a label needs real geometric analysis, and that is more machinery than a build-time linter over 13 trusted in-tree diagrams warrants. The failure direction is deliberate: a label with no preceding shape errors instead of passing. Qualify the "only freshness control" statement — correct. Verified that all three named controls exist and are genuinely enforced, each with a test: Scope note on this fileFour rounds of review have now landed on I have kept each fix to the smallest thing that removes a silent pass, and have twice chosen to delete a special case rather than add a branch. Where the correct fix would require real machinery, specifically full cascade specificity and SVG geometry, I stopped and documented the boundary in the code rather than building toward a browser. If review would rather see this trimmed back than hardened further, that is a reasonable call and I would rather make it deliberately than by accretion. Verification
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba27a38a00
ℹ️ 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".
| ?.color | ||
| : undefined; | ||
| const inheritedFill = gFillStack[gFillStack.length - 1] ?? null; | ||
| const resolved = inlineFill ?? classFill ?? inheritedFill; |
There was a problem hiding this comment.
When a <text> has both fill="#161616" and a class whose stylesheet rule sets fill: #cccccc, CSS gives the class rule precedence over the SVG presentation attribute, but this expression selects inlineFill first. The checker therefore scores the unused high-contrast color and lets the rendered low-contrast text pass; resolve the effective CSS cascade rather than treating the fill attribute as an inline style.
Useful? React with 👍 / 👎.
| // the backdrop is captured per element rather than scored against the | ||
| // canvas. Document order stands in for geometry: in these diagrams a | ||
| // chip is always drawn immediately before the label that sits on it. | ||
| if (color === DIAGRAM_SURFACE) reverseText.push(lastShapeFill); |
There was a problem hiding this comment.
When reverse text uses the explicitly supported shorthand fill="#fff" on a dark chip, normalizeColor leaves it as #fff, so this equality does not classify it as reverse text. The later loop instead compares it with the white canvas and reports 1:1 contrast, causing the accessibility gate to reject a valid high-contrast diagram; normalize equivalent hex spellings before testing for the surface color.
Useful? React with 👍 / 👎.
| Source-observation age is not a Registry Notary enforced quantity: the only | ||
| Notary freshness control is the federation profile's | ||
| `max_claim_result_age_seconds`, which bounds the age of a Relay consultation |
There was a problem hiding this comment.
Although the limitation page was corrected after the earlier review comment, this newly added capability-matrix paragraph still says max_claim_result_age_seconds is Notary's only freshness control. Deployments using subject access also enforce max_auth_age_seconds and max_evaluation_age_seconds, while wallet proof validation enforces oid4vci.proof.max_age_seconds; scope the statement to source-observation freshness so readers do not conclude those shipped controls are absent.
Useful? React with 👍 / 👎.
| const tokenRe = | ||
| /<style\b[^>]*>|<\/style>|<g\b[^>]*>|<\/g>|<text\b[^>]*>|<(?:rect|circle|ellipse|polygon|path)\b[^>]*>/g; |
There was a problem hiding this comment.
When a valid SVG uses <text fill="#161616"><tspan fill="#ffffff">…</tspan></text>, this tokenizer visits only the parent <text> and records the high-contrast parent fill, while the rendered glyphs use the white <tspan> override and disappear on the fixed white surface. Include text-bearing descendants such as <tspan> in fill resolution so nested overrides cannot bypass the contrast gate.
Useful? React with 👍 / 👎.
The manifest reference promised a new schema version for any change that invalidates a valid V1 manifest, without noting that the promise starts at v1.0.0. Removing an inert optional key is such a change, and so was the unknown-key rejection that shipped in 0.9.0, so the paragraph described a rule the project had already set aside once on purpose. State the pre-1.0 rule the API stability reference already carries, and record the removal as a BREAKING changelog entry with its one migration step, which is what that rule requires and what the earlier break also shipped with. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
REQ-DM-MANIFEST-013 ends with an unqualified MUST: a breaking change carries a new schema version. Two pre-1.0 releases have broken the schema without one, deliberately, so the requirement stated an invariant the implementation does not hold to. Bound the obligation to v1.0.0, point at the compatibility promise that already defines the pre-1.0 rule, and name the two breaks that shipped under it so a reader can tell a deliberate exception from an unrecorded violation. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…rvations The matrix called max_claim_result_age_seconds the only Notary freshness control. Subject-access flows enforce max_auth_age_seconds and max_evaluation_age_seconds, and wallet proof validation enforces oid4vci.proof.max_age_seconds, so the sentence denied controls that run. Same correction already applied to the known-limitations page; the paragraph's actual claim is unchanged, since none of those controls bound the age of the observation behind a claim. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…coring Three ways the contrast gate scored a color the reader never sees. A fill attribute is a presentation attribute, not an inline style, so a class rule outranks it; reading the attribute first let rendered low-contrast text pass. A tspan paints the glyphs it wraps, so a fill it declares is what the reader sees, and the tokenizer never visited one. Shorthand hex was left unexpanded, so fill="#fff" missed the reverse-text test and was scored 1:1 against a canvas it never sits on, which would reject a legible diagram. A tspan without its own fill inherits the fill its parent text already resolved, so it is skipped rather than rescored or counted unresolved. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
P1: removing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa8f3db473
ℹ️ 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 tspan without a fill of its own inherits the fill its parent <text> | ||
| // already resolved and recorded, so it is skipped rather than rescored or | ||
| // counted unresolved. | ||
| const resolved = classFill ?? presentationFill ?? (isTspan ? null : inheritedFill); |
There was a problem hiding this comment.
When an SVG uses an inline style attribute, such as <text class="safe" style="fill:#cccccc">, the inline style wins in the browser, but this resolver ignores it and records the class color instead. A low-contrast inline fill can therefore pass the new accessibility gate whenever the presentation attribute or class fallback is high-contrast; parse the element's effective inline fill before applying class and presentation-attribute fallbacks.
Useful? React with 👍 / 👎.
| // the backdrop is captured per element rather than scored against the | ||
| // canvas. Document order stands in for geometry: in these diagrams a | ||
| // chip is always drawn immediately before the label that sits on it. | ||
| if (color === DIAGRAM_SURFACE) reverseText.push(lastShapeFill); |
There was a problem hiding this comment.
When any dark shape appears earlier in document order, a later white label placed elsewhere on the white canvas is scored against that unrelated shape and passes. Fresh evidence after the earlier reverse-text comment is that the replacement stores one global lastShapeFill without checking geometry, grouping, or even adjacency, so a preceding legend swatch can still hide a 1:1 contrast failure; associate each reverse-text element with an actual containing or overlapping backdrop.
Useful? React with 👍 / 👎.
| @@ -567,8 +567,6 @@ pub struct EvaluationProfileManifest { | |||
| pub claim_id: String, | |||
| pub subject_id_type: String, | |||
| #[serde(default, skip_serializing_if = "Option::is_none")] | |||
There was a problem hiding this comment.
This commit combines a Manifest schema break under crates/, product-owned documentation under products/, docs-site scripts and specifications under docs/site/, and a release inventory edit under release/. Split these into owner-scoped changes so each owning area can be reviewed and verified independently, as the repository explicitly requires.
AGENTS.md reference: AGENTS.md:L77-L78
Useful? React with 👍 / 👎.
| | --- | --- | --- | --- | | ||
| | 0.1.0 | 2026-06-13 | draft | Initial framework: layers, identifiers, metadata axes, document structure, register, cross-link rule, lifecycle. | | ||
| | 0.2.0 | 2026-07-07 | draft | Corrected REQ-DOC-012 to order version-history tables oldest first with the current version last, added layer and audience frontmatter definitions, documented the -G suffix convention, and replaced positional references with named sections. | | ||
| | 0.3.0 | 2026-08-01 | draft | Tightened REQ-DOC-014 so a `verified` specification anchors each load-bearing claim to code, a test, a fixture, a generated artifact, or a released machine contract, and site pages corroborate rather than stand alone as evidence. | |
There was a problem hiding this comment.
The fresh evidence after the earlier missing-history finding is that the newly added rows use MINOR increments even though they alter existing normative requirements: this row explicitly tightens REQ-DOC-014, while the new RS-DM-MANIFEST, RS-ARC-G, and RS-PR-NOTARY rows likewise change existing obligations. RS-DOC Section 8 requires a MAJOR version for an altered or removed normative requirement, so these versions incorrectly present breaking conformance changes as additive ones.
Useful? React with 👍 / 👎.
| @@ -1,7 +1,14 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Commit b81b42a6831172c9b9213fa07553b497f9b3e8e7 has no Signed-off-by: trailer, so it does not satisfy the repository's mandatory DCO policy and will be rejected by the corresponding contribution gate. Recreate this commit with git commit -s before merging.
AGENTS.md reference: AGENTS.md:L70-L70
Useful? React with 👍 / 👎.
An inline style attribute is the strongest author-level declaration, so a low-contrast style="fill:..." could hide behind a legible class rule or fill attribute. It now leads the precedence chain: inline style, class rule, presentation attribute, inherited group fill. Reverse text tracked a single last-shape-seen carried forward for the whole document, so an unrelated earlier swatch could stand in as the backdrop for a stray white label anywhere later. A shape is now claimed only by the element that directly follows it, and consumed when claimed. A tspan reuses the backdrop its enclosing text claimed, since it sits on the same shape. Both diagram labels that use reverse text directly follow their chip, so the corpus passes unchanged; a label that does not will now be reported. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
|
Round 5: two fixed, two declined with evidence, one real finding that needs a decision rather than a patch. FixedHonor inline style fills when resolving text color — correct. An inline Verify reverse text against the shape beneath it — correct, and a fair reading of what I wrote last round. I described document order as standing in for geometry and said a chip is always drawn immediately before its label, but the code did not enforce the adjacency it relied on: a single Adjacency is now enforced. A shape is claimed only by the element that directly follows it, and is consumed when claimed, so a sibling label further along cannot claim the same shape. A shape with no fill of its own still displaces the previous one, because it is what the next element actually follows. A I measured the corpus before changing this: exactly two reverse-text elements exist across all thirteen diagrams, both in What this still is not: geometry. Adjacency is a tighter proxy than "last shape seen", not a containment test. Real resolution needs bounding boxes and transforms, which is more machinery than a build-time linter over thirteen trusted in-tree diagrams warrants, and I would rather leave that boundary stated than half-build it. A label with no shape directly before it is reported rather than assumed safe. DeclinedAdd the required DCO sign-off — commit Split the change along owning-area boundaries — the rule is real, but the finding reads it more strictly than the repo does or than the change permits. Removing a field from a struct necessarily invalidates every document that carries it. Splitting The repo does not treat the rule as absolute either: of the last sixty commits on If you would rather see it split, the only clean cut is code from docs: everything under Needs a decision, not a patchUse major versions for altered normative requirements — the reading of RS-DOC Section 8 is correct, and it is correct about my rows. Section 8 says MAJOR for a change that alters or removes a normative requirement. All four rows this PR adds alter existing requirements: REQ-DOC-014, REQ-ARC-G-013, REQ-PR-NOTARY-041, and REQ-DM-MANIFEST-013. Under the rule as written they should be MAJOR. The reason I have not made that change is that it would make these four specs the only ones in the layer ever to have done so, and it would be inconsistent rather than correct. No spec in
and RS-OP-POSTURE 0.1.2, "Replaced waiver reason text with validated operator references", which took a PATCH. So the defect is real and it is larger than these four rows: Section 8 states a versioning rule the spec layer has never followed, across ten specs and roughly a year. That is exactly the class of thing this PR exists to remove, but there are two incompatible ways to remove it, and picking one is a spec-layer governance call rather than a review fix:
I would rather leave the rows as they are and surface this than silently pick one, so flagging it here for @jeremi. Whichever way it goes, it should be one deliberate change across the whole layer rather than four rows in this PR versioned differently from every other row around them. Verification
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88d3a2df85
ℹ️ 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".
| @@ -0,0 +1,66 @@ | |||
| // Bounded retry-with-backoff for the network git fetches performed while | |||
There was a problem hiding this comment.
The fresh evidence after the earlier DCO finding is that the replacement commit 4794c85e9c0848bd321c951c77b44eda0c8399ad itself has no Signed-off-by: trailer in its commit message, so it still violates the mandatory DCO policy and will be rejected by the contribution gate; recreate this commit with git commit -s before merging.
AGENTS.md reference: AGENTS.md:L70-L70
Useful? React with 👍 / 👎.
|
Round 6 raised one finding, and it is the DCO one again with a different commit hash. Add the required DCO sign-off — declined again, on the same evidence, now doubled. The finding says the replacement commit What is checkable: all seventeen commits on If a later round raises this a third time, the useful next step is a hash that resolves. Without one there is nothing to act on. Status27 checks green, 5 skipped, Two things from earlier rounds are still open for @jeremi rather than for me, and neither is a code fix:
|
2 participants
Six 1.0-milestone tickets from the docs-truth cluster, one commit each, so they can be split or dropped individually during review.
The common thread: each one is a place where a document promised behaviour the code does not implement. No new features, no new abstractions, no new tooling.
Commits
In branch order, oldest first:
fix(docs): scope static-peer federation to the shipped inbound routefix(docs): exclude the audit pipeline from Notary PostgreSQL state claimsfix(docs): retry pinned docs fetches and surface git stderrfix(docs): keep diagram SVGs legible in dark modefix(docs): anchor verified specs to inspectable artifactsfix(manifest): remove inert max_source_observed_age_seconds fieldfix(docs): resolve SVG class fills without rewriting the documentfix(docs): report SVG text fills the contrast gate cannot scorefix(docs): correct federation and audit-pipeline claims in the spec layerCommits 7, 8, and 9 are review fixes, kept separate only to avoid rewriting a branch already under review. Fold 7 and 8 into commit 4 and 9 into commits 1 and 2 if you want the strict one-commit-per-ticket shape.
Commit 7 fixes two defects CodeQL surfaced in the SVG checker. Class fills were read from only the first
<style>element, so a diagram carrying a second one would have its labels reported as unresolvable, failing CI on valid input. And<style>blocks were deleted from the text before scanning, which cannot be done safely in one pass: a removal can splice its neighbours into a fresh<styleand leave behind exactly what the removal was meant to drop. Style regions are now skipped during the tokenizer walk, so nothing is rewritten and no tag can be resurrected.CodeQL's framing (
incomplete multi-character sanitization, an HTML injection vulnerability) does not apply here: this is a build-time linter over trusted in-tree diagrams with no rendering sink. The incompleteness was real, so the sanitize-shaped code is gone rather than patched around. Both defects are covered by tests written red-first.Commit 8 closes a silent-pass hole in the same checker. Contrast was scored by parsing hex digits, so a valid SVG paint value it could not parse (
white,rgb(255,255,255),currentColor,none) producedNaN, andNaN < 4.5is false: the gate passed the file without a word.#rrggbbaawas worse than silent, since the alpha channel was sliced off and half-transparent black scored 21:1. Only#rgband#rrggbbare scoreable now, and anything else is reported as an error naming the offending value. No shipped diagram currently resolves a<text>to an unscoreable fill, so this is a latent hole rather than an active miss.Commit 9 corrects three claims in the spec layer that asserted more than the code or the cited requirement supports: RS-ARC-G denied the existence of an outbound Notary-to-Notary client that
registry-notary-clientdoes in fact ship behind a feature gate; REQ-ARC-G-013 and REQ-PR-NOTARY-041 attributed hash chaining to two requirements that only mandate emit-to-sink and fail-closed; and both requirements changed obligations without the version-history row REQ-DOC-012 requires unconditionally. Full reasoning, including the one review comment declined and why, is in the review-response comment on this PR.Please read this one first: #583 is a manifest format break
EvaluationProfileManifestcarriedmax_source_observed_age_seconds, documented as a "public freshness hint" whose "runtime enforcement is in Registry Notary config". There is no such enforcement anywhere. Notary's only freshness control is the federation profile'smax_claim_result_age_seconds, which bounds consultation-result age; snapshot freshness belongs to Relay'smax_snapshot_age_ms. The field configured nothing.The struct carries
#[serde(deny_unknown_fields)], so removing the field turns the key into a hard parse error, not a silently ignored one. Any manifest still carrying it now fails to load with:That is the deliberate choice over keeping a field that tells adopters an enforcement story the runtime does not implement, but it is a
registry-manifest/v1break and worth an explicit call before merge.Occurrences of the same name under an evidence-pack
policyare a different key in a different namespace (EvidencePackMetadata.policyis opaque unvalidated JSON) and are left as authored.Scope note
#583 spans four owning areas rather than the usual one.
crates/,products/, anddocs/site/are forced by the generated-output rule, since the manifest reference mirror regenerates fromproducts/. Therelease/line is a stale claim of the same class: it quoted asource_observed_at_fieldsection of the Notary operator config reference that no longer exists, and cited a Relay configuration heading (Postgres type mapping) that does not exist either. Easy to drop if you would rather split it.#237 introduces one new requirement
RS-DOC gains REQ-DOC-014: a spec marked
evidence: verifiedmust anchor each load-bearing claim to an artifact a reader can open directly (source, test, fixture, generated artifact, or released machine contract). A site page may corroborate an anchor but must not be the only evidence, since evidence resolving solely to further documentation is a citation loop rather than verification. It explicitly does not require one reference per sentence.No page was downgraded.
rs-op-posturehad been shipping asverifiedwhile its own Evidence section admitted source evidence was "pending"; it now cites the posture JSON Schema, the waiver-metadata validator, and the profile-gates contract test.Verification
Rust (
#583):The removal is covered by a new test written red-first,
evaluation_profile_rejects_removed_source_observed_age_field, which asserts the rejection names the removed key.Docs site:
npm testandnpm run check.Known residual risk
Nothing mechanical checks Evidence sections.
check-evidence-links.mjsonly gatessrc/data/{contracts,standards}.yaml, which is how averifiedpage shipped for weeks admitting its own evidence was pending. No tooling was added here, so this stays a review-time responsibility.Two anchor-style conventions also coexist across the spec layer (repo-relative bare paths vs GitHub permalinks, some SHA-pinned). Review raised this directly. Bare paths are 92 of ~102 source citations here and pre-date this branch, nothing enforces the pinned style outside the
standards.yaml/contracts.yamldata layer, and SHA-pinning would collide with AGENTS.md's squash-on-merge warning. Left alone deliberately, but it is a real convention call, and the clean fix is tag-pinning after a tag is cut rather than retrofitting SHAs here.