docs(site): review configuration docs of six comparable projects#491
docs(site): review configuration docs of six comparable projects#491jeremi wants to merge 7 commits into
Conversation
Add a documentation-only review of Kong Gateway, Tyk, Airbyte Connector Builder, SpiceDB, Terraform, and the OpenTelemetry Collector, covering each project's configuration-domain map, onboarding ladder, documentation tree, and how it explains defaults, secrets, environment bindings, validation, and generated artifacts. Every observation links the public page it came from. Part B proposes the resulting Registry Stack documentation architecture: the five source-shape journeys, the three configuration planes and their ownership boundaries, generated references for the five authoring schemas and the two runtime config schemas, an authoring-diagnostics index, and a single validate-before-you-deploy path through test, check, build, and Config Bundle signing. Planning input under docs/site/docs/, not published site content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon Signed-off-by: Claude <jeremi@joslin.fr>
![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: 74501cfe2f
ℹ️ 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,1640 @@ | |||
| # Configuration documentation review: six comparable projects | |||
There was a problem hiding this comment.
Before this commit can pass the repository's DCO policy, add an actual Signed-off-by: trailer: the commit message ends with a checklist and generated footer, but git interpret-trailers --parse reports no sign-off. Checking the DCO box in the message does not sign the commit.
AGENTS.md reference: AGENTS.md:L70-L70
Useful? React with 👍 / 👎.
| means an evidence service in the project file and a deployment service in the environment file. | ||
| 2. **No troubleshooting entry point.** Distributing diagnostics into feature pages leaves a reader who | ||
| has a symptom, not a feature, with nowhere to start. Registry Stack's authoring diagnostics have the | ||
| same shape today: codes exist in source, one appears in a tutorial troubleshooting table, and there |
There was a problem hiding this comment.
When this gap analysis is used to plan the diagnostics index, this source-repository claim is incorrect: a repo-wide search for registryctl.authoring.* under docs/site finds only registryctl.authoring.diagnostics.truncated in reference/registryctl.mdx, not any code in a tutorial troubleshooting table. Link an actual fixture or test if one exists, or state that no diagnostic code currently appears in a tutorial.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| - `ProjectStarterSequence` and `ProjectWorkspaceJourneys` are generated from | ||
| `crates/registryctl/tests/fixtures/project-authoring-journeys.yaml` and the committed golden | ||
| workspaces, so every published command sequence is a tested sequence. | ||
| - `check:tutorial:registryctl` and `check:tutorial:dry-run` execute tutorial commands. |
There was a problem hiding this comment.
When this section is used to determine which examples have runtime coverage, it overstates the existing gate: package.json maps check:tutorial:dry-run to check-tutorial.sh --dry-run, and that script exits after extraction and command-count checks before executing anything. The dedicated registryctl runner executes the two deployable Relay and Notary tutorials, while the project-authoring tutorials receive only drift checks plus separate Rust coverage, so document those boundaries instead of saying both checks execute tutorial commands.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| useful configuration page in this review. Registry Stack's `${VAR}`, `${VAR:-fallback}`, and | ||
| `${VAR:?message}` expansion happens before YAML parsing, which means resolution is bundle-activation | ||
| time; the docs should say that as plainly as Tyk says its three cases. |
There was a problem hiding this comment.
When Relay or Notary uses its supported local-file configuration path, ${VAR} expansion happens while the product loads that file at startup, with no Config Bundle activation involved; the signed-bundle loaders perform the same expansion while validating/loading a bundle. Calling this exclusively "bundle-activation time" therefore gives operators the wrong lifecycle for local configuration and environment changes. Describe it as product config-load time and distinguish the local-file and signed-bundle sources.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| configuration, plus a documented rule that a minor bump is additive and consumers must ignore | ||
| unrecognized properties, and a major bump must be rejected. That converts a deterministic artifact |
There was a problem hiding this comment.
If newer registryctl output is deployed to an older Relay or Notary, requiring consumers to ignore unrecognized properties can silently discard a new authorization or security setting. Both runtime config graphs currently use deny_unknown_fields, their generated schemas are closed, and the published stability contract explicitly says unknown fields are rejected. Keep strict compatibility rejection, or treat any change to that behavior as a separate security-sensitive product-contract decision rather than prescribing it in a docs-only plan.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| docs site already fails the build on a dangling internal link; the missing half is a redirect map for | ||
| external inbound links when a slug moves. |
There was a problem hiding this comment.
When this recommendation is prioritized, it sends work toward a gap that does not exist: docs/site/astro.config.mjs already contains an extensive inbound redirects map for renamed and retired routes, including internal, external, current-docset, archive, and old API-reference paths, and astro-config.test.mjs verifies docset-specific redirect behavior. Recast redirects as an existing practice to preserve instead of the missing half of link management.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| | Journey | What the adopter starts from | Current surface | | ||
| | --- | --- | --- | | ||
| | Spreadsheet | A workbook or table the institution already holds | `registryctl init relay --sample benefits` (`InitProjectKind::RelaySpreadsheetApi`); tutorial `tutorials/publish-spreadsheet-secured-registry-api` | | ||
| | OpenAPI | A source that publishes an OpenAPI description, so the bounded request is lifted from a contract | No starter names this shape. `fhir-r4`, `dhis2-tracker`, and `opencrvs-dci` are its instances | |
There was a problem hiding this comment.
When a reader chooses the proposed OpenAPI row, the named current surfaces cannot provide the described lift-from-contract journey: the fhir-r4, dhis2-tracker, and opencrvs-dci starters all ship authored adapter.rhai files and declare the script capability, while a repo-wide search finds no OpenAPI ingestion or request-lifting implementation. The sequencing section itself defers this page until a starter or documented lift procedure exists, so mark the current surface as absent rather than calling those script starters OpenAPI instances.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| emits data as an exporter at the end of one pipeline and consumes data as a receiver at the start of | ||
| another pipeline", constrained so that "A connector can only be used in a pair of pipelines when it | ||
| supports the combination of [Exporter Pipeline Type] and [Receiver Pipeline Type]" | ||
| ([connector/README](https://raw.githubusercontent.com/open-telemetry/opentelemetry-collector/main/connector/README.md)). |
There was a problem hiding this comment.
When this dated review is revisited, these raw.githubusercontent.com/.../main/... links resolve whatever content happens to be on the branch then, so the quoted definitions may no longer support the 2026-07-26 observations. The same mutable pattern is used for roughly ten OpenTelemetry README, RFC, metadata, testbed, and release-document citations in this section. Replace main with the reviewed release tag or commit SHA so the evidence remains reproducible.
AGENTS.md reference: docs/site/AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
| | --- | --- | --- | | ||
| | Spreadsheet | A workbook or table the institution already holds | `registryctl init relay --sample benefits` (`InitProjectKind::RelaySpreadsheetApi`); tutorial `tutorials/publish-spreadsheet-secured-registry-api` | | ||
| | OpenAPI | A source that publishes an OpenAPI description, so the bounded request is lifted from a contract | No starter names this shape. `fhir-r4`, `dhis2-tracker`, and `opencrvs-dci` are its instances | | ||
| | `http` | One bounded request declared by hand | `init --from http`, `capability.request`; tutorial `tutorials/author-registry-project` | |
There was a problem hiding this comment.
When an adopter follows this journey table, capability.request is not valid authoring syntax: the closed integration schema accepts capability.http, capability.script, or capability.snapshot, with request nested under the http block. The same misnaming recurs in the ownership table as the capability list (request, script, snapshot), so generated-reference and chooser work based on this plan would teach YAML that the schema rejects.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| **Plane 3, runtime and evidence state.** Served `openapi.json`, audit records, credential status, | ||
| issued credentials, posture reads, signed Config Bundles and trust anchors. Not configuration, and | ||
| never hand-edited. |
There was a problem hiding this comment.
When readers use this three-plane map, classifying signed Config Bundles as runtime state and declaring them "Not configuration" contradicts the actual lifecycle: registryctl bundle sign packages a generated product input, the bundle contains config/..., and Relay or Notary verifies it before parsing that product configuration. Treat the signed bundle as governed generated product input (with the trust anchor separately identified as operator-owned trust configuration), not as evidence state, or the proposed build/sign/activate documentation will split one configuration artifact across incompatible planes.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
Fix eight claims about this repository that a review flagged, each verified against source: - Capability is `capability.http` with the request nested under it, not `capability.request`. Corrected in the journey table and the ownership table. - The OpenAPI journey has no current surface. `fhir-r4`, `dhis2-tracker`, and `opencrvs-dci` each ship an `adapter.rhai` and declare `capability.script`, so they belong to the script row. - A signed Config Bundle is governed generated product input, not runtime state: `bundle sign` packages the generated directory and the product verifies the bundle before parsing the config inside it. The trust anchor stays operator-owned trust configuration. - Environment expansion happens at product config-load time for both a local file and a signed bundle's config document, not at bundle activation. - Keep strict rejection of unknown runtime fields. Both config graphs use `deny_unknown_fields` and the stability contract already rejects unknown or retired fields, so the versioning recommendation now applies to the build output envelope only. - `check:tutorial:dry-run` runs extraction and drift checks and exits; `check:tutorial:registryctl` executes the two deployable tutorials. - The inbound `redirects` map in `astro.config.mjs` already exists, so recast redirects as a practice to keep current. - Diagnostics are two code families, and neither is indexed. Also record why the OpenTelemetry citations stay on unpinned branch URLs: `main` is what was fetched, and version-dependent claims name their version in prose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon Signed-off-by: Claude <jeremi@joslin.fr>
|
Thanks. I verified all ten findings against source. Eight were correct and are fixed in 1875864; two are not actionable, with reasons below. Fixed
Not actionableDCO sign-off. The trailer is present. Unpinned OpenTelemetry citations. Left unpinned deliberately, and now stated in the appendix rather than left implicit. Those pages were fetched from Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1875864c96
ℹ️ 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".
| 2. **The projects that survive contact with real users publish the schema as the authority and generate | ||
| the reference from it.** Kong points readers at `/schemas` rather than copying defaults into prose. | ||
| Terraform generates provider documentation with `tfplugindocs`. Airbyte names one | ||
| `declarative_component_schema.yaml` as the validator. Where the reference is hand-maintained beside a | ||
| real schema, it drifts, and Terraform's own variable pages prove it. |
There was a problem hiding this comment.
When this synthesis drives the generated-reference proposal in B.5, it overstates the shared evidence: the Kong section says its prose delegates to a live schema endpoint, while lines 552-557 explicitly describe Airbyte's reference as a hand-authored companion that can drift from its schema. Of the three examples listed here, only Terraform is shown generating reference documentation. Split schema authority from reference generation rather than presenting both as a common practice.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| "distinctly different from workspaces in HCP Terraform, which each have their own Terraform | ||
| configuration and function as separate working directories" | ||
| ([cli/workspaces](https://developer.hashicorp.com/terraform/cli/workspaces)). There is no admonition | ||
| marking the collision, the glossary defines only the CLI sense, and `TF_WORKSPACE` binds the other. |
There was a problem hiding this comment.
When this terminology analysis is used to update the glossary, the final clause reverses the variable's meaning: TF_WORKSPACE selects the Terraform CLI workspace, not the distinct HCP Terraform workspace. The Terraform CLI environment-variable reference is the relevant contract, so remove this purported example of the second meaning or replace it with the actual HCP workspace-selection mechanism.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| - **Bind an environment.** Secret references by name only, `${VAR}`, `${VAR:-fallback}`, and | ||
| `${VAR:?message}` expansion before YAML parsing, and the rule that diagnostics never print values. |
There was a problem hiding this comment.
When an adopter follows the proposed Bind an environment page, this placement makes ${VAR} expansion look supported in environments/<name>.yaml, but registryctl reads that authored file directly through parse_yaml without calling expand_config_env_vars; expansion occurs in the Relay and Notary product configuration loaders, as this plan itself explains at lines 410-414. Move this syntax to runtime configuration loading or explicitly distinguish local/generated product configuration from the authored environment surface, otherwise readers can submit placeholders that fail authored validation or become literal secret-reference names.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| 1. **Publish a versioned projection of every generated artifact and write down the increment rule.** | ||
| Registry Stack already versions its CLI reports (`registryctl.init.v1`, `registryctl.smoke.v1`, and | ||
| the rest). Extend that to the `build` output envelope, so a tool reading | ||
| `.registry-stack/build/<env>/reviewable/review.json` can tell which contract it is reading, and | ||
| document the increment rule for that envelope. |
There was a problem hiding this comment.
When this proposal is scheduled, it directs work at a contract that is already versioned: compile_project_for_environment writes "schema": REVIEW_SCHEMA into reviewable/review.json, REVIEW_SCHEMA is registry.project.review.v1, and signed-baseline validation rejects any other value. A consumer can therefore already identify this envelope's contract; recast this as documenting and preserving its increment policy, or identify a genuinely unversioned build artifact instead.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| 3. **Ship `validate` and a redacted effective-configuration dump as ordinary commands.** Registry | ||
| Stack's compile step makes the gap between authored and effective configuration wider than the | ||
| collector's, so a redacted dump matters more, not less. Redaction should be the default and | ||
| unredacted an explicit act, with tests asserting the redaction, which the repository's own rules | ||
| already classify as security-sensitive. |
There was a problem hiding this comment.
When this recommendation is turned into implementation work, it duplicates commands both products already ship: Relay and Notary have doctor validation and explain-config commands that emit redacted resolved configuration, and the existing upgrade guide already tells operators to use them. Describe the gap as site-level discoverability or reference coverage instead of asking Registry Stack to ship mechanisms that already exist and have redaction tests.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| to `bundle sign` and `bundle verify` with `anchor`, plus `check --against` and `--anchor` for a | ||
| signed baseline and the five review classes (`claim`, `integration`, `service_policy`, | ||
| `operator_security`, `disclosure`). |
There was a problem hiding this comment.
When a signed baseline was produced by a different registryctl compiler version, semantic_change_records appends a compiler dimension independently of authored changes, and the closed validator accepts it alongside the five dimensions listed here. Omitting it from the proposed review page leaves operators with an unexplained reported change and no review guidance for compiler upgrades, so document all six dimensions and the handling required for compiler.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| `src/data/generated/`. The runtime half is nearly free: `just config-schema-generate` and | ||
| `just config-schema-check` in `crates/registry-relay/justfile` and `products/notary/justfile` | ||
| already reproduce those two files from each product's typed config graph and fail CI on drift, so | ||
| the docs step consumes an artifact that is already gated rather than adding a second source of | ||
| truth. Reference drift then fails `npm run check` the same way OpenAPI drift already fails CI. |
There was a problem hiding this comment.
When a schema changes without committing regenerated reference data, the proposed step does not make npm run check fail: that command runs generate, the existing generators overwrite src/data/generated/ in place, and the docs CI job never checks git diff. A generator that likewise writes the new files will let the check pass with a dirty worktree, so explicitly compare generated bytes with the committed files or add a post-generation cleanliness assertion before claiming reference drift is gated.
AGENTS.md reference: docs/site/AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
| 4. **Redaction is not containment, and the mature projects have learned it in public.** Terraform moved | ||
| from `sensitive` to ephemeral and write-only values because sensitive values still landed in state | ||
| and plan files. Kong moved secrets out of entities into vault references. Airbyte keeps credentials | ||
| out of the connector entirely. |
There was a problem hiding this comment.
When this security conclusion is carried into Registry Stack guidance, "out of the connector entirely" overstates Airbyte's boundary: the review itself says end users fill credential fields in spec.connectionSpecification, those values are supplied to the running connector as config, and airbyte_secret only obfuscates them in the UI and API. Airbyte keeps credential values out of the authored connector manifest, not out of connector execution, so narrow the claim rather than presenting obfuscation as structural containment.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| 2. **Publish per-field stability and per-product availability inside the reference itself.** The | ||
| collector's `Stability` and `Distributions` columns tell a reader about this field, for this signal, | ||
| in this build. Pre-1.0 with two products, Registry Stack should carry `stability` and the owning | ||
| product per configuration block in the schema and render it as a column, which also gives the | ||
| version-skew story one home. |
There was a problem hiding this comment.
When this proposal is traced back to OpenTelemetry, it changes the granularity of the borrowed practice: the Collector section correctly describes stability per component and telemetry signal, while the generated Stability and Distributions tables describe components in distributions, not individual configuration fields. Keep per-field or per-block stability as a new Registry Stack-specific proposal, or describe the Collector precedent at component granularity instead of claiming its columns already answer this field-level question.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| - **Validate before you deploy.** The single page that walks `test` to `check --explain` to `build` | ||
| to `bundle sign` and `bundle verify` with `anchor`, plus `check --against` and `--anchor` for a |
There was a problem hiding this comment.
When the proposed deployment page is implemented literally, registryctl bundle verify has no anchor token or --anchor flag: the current command contract is bundle verify --bundle-dir <dir> --anchor-path <file>. anchor is a separate top-level namespace, while --anchor belongs to check --against and build --against, so name --anchor-path here to avoid publishing an invalid verification sequence.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
Second review round, each claim verified against source before changing:
- `TF_WORKSPACE` selects the Terraform CLI workspace, standing in for
`terraform workspace select`, not the HCP workspace. It sharpens the
collision rather than illustrating the second sense.
- Split schema authority from reference generation in the cross-project
synthesis. Only Terraform generates published reference from schema;
Kong delegates to a live endpoint and Airbyte's reference is a
hand-authored companion, as this document says elsewhere.
- Narrow the Airbyte credential claim to the authored manifest. Values
reach the running connector as `config`, and `airbyte_secret` obfuscates
rather than contains.
- Attribute the collector's stability and distribution columns at
component and signal granularity, and mark per-block stability as a
Registry Stack extension.
- Both products already ship `doctor` and `explain-config`, so recast that
recommendation as discoverability and reference coverage.
- `reviewable/review.json` already carries `registry.project.review.v1`
and rejects any other value, so the gap is the published increment
policy, not a missing version marker.
- Add `reviewable/entities/<id>.json` to the generated inventory.
- Keep `${VAR}` off the authoring environment page: `registryctl` parses
`environments/<name>.yaml` without expansion, so a placeholder there
fails validation or becomes a literal reference name.
- `bundle verify` takes `--bundle-dir` and `--anchor-path`; `--anchor`
belongs to `check --against` and `build --against`.
- Document six semantic-change dimensions. A `compiler` dimension is
appended on a baseline built by a different CLI version, with no
authored cause.
- The proposed drift gate needs an explicit byte comparison or a clean
worktree assertion, because `npm run check` regenerates in place and the
Docs CI job has no cleanliness step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon
Signed-off-by: Claude <jeremi@joslin.fr>
|
All ten verified against source. Every one was correct, and all ten are fixed in ff2b3ea.
The last one was the most useful. It would have shipped a gate that silently passed, which is worse than no gate. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff2b3eaa92
ℹ️ 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".
|
|
||
| | Journey | What the adopter starts from | Current surface | | ||
| | --- | --- | --- | | ||
| | Spreadsheet | A workbook or table the institution already holds | `registryctl init relay --sample benefits` (`InitProjectKind::RelaySpreadsheetApi`); tutorial `tutorials/publish-spreadsheet-secured-registry-api` | |
There was a problem hiding this comment.
When a reader copies this chooser entry, registryctl init relay --sample benefits fails because the relay subcommand requires a positional dir: PathBuf; the published CLI reference likewise documents the syntax as init relay <dir> in docs/site/src/content/docs/reference/registryctl.mdx:302-304. Include a placeholder such as <dir> so the proposed journey starts with an executable command.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| 4. **Bind an environment and generate product input.** Exit test: `check --explain` reviewed, | ||
| `build` reproduced twice with identical output, then Config Bundle sign and verify. | ||
|
|
||
| Rungs 1 and 2 exist. Rung 3 exists but is entered through a tutorial whose title does not say |
There was a problem hiding this comment.
When sequencing work from the claim that Rung 2 already exists, its stated exit test is not actually covered: the current verify-claim-registry-api.mdx:274-289 no-match request is rendered as value: false and described as a false predicate, so it does not teach why no_match differs from false as required at line 1453. Mark this rung as needing that distinction or adjust the exit test, otherwise the proposed sequence omits required tutorial work.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| "choose by source shape". Rung 4 is split across `reference/registryctl` and the operate pages with | ||
| no single page that walks it. |
There was a problem hiding this comment.
When this claim is used to justify a new validation page, it overlooks docs/site/src/content/docs/operate/single-node-compose-behind-proxy.mdx:62-97, whose single “Author and verify the project” section already walks test, check --explain, build, bundle sign, and bundle verify in that order. The remaining gap may be a topology-neutral entry point or discoverability, but saying no page walks Rung 4 duplicates existing content.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| 2. **Generate the diagnostics index** from the same source the CLI uses, so a new code cannot ship | ||
| undocumented. |
There was a problem hiding this comment.
When a PR changes only crates/registryctl/src/project_authoring/diagnostics.rs, the proposed generator cannot ensure that a new code is documented: .github/scripts/ci_changes.py:254-264 sets docs only for docs-owned paths and selected product documentation/OpenAPI paths, so the Docs job containing npm run check is skipped for this source-only change. Extend the change classifier or run the generated-index comparison in the project-authoring job; otherwise the stated gate still permits undocumented diagnostics to ship.
AGENTS.md reference: docs/site/AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
Third review round, each claim verified against source: - `init relay` requires a positional directory. The chooser entry now reads `registryctl init relay <dir> --sample benefits`, matching the published CLI reference. - Rung 4 is documented, not absent. `operate/single-node-compose-behind- proxy` already walks test, check, build, bundle sign, and bundle verify under one section, so the gap is a topology-neutral entry point. The proposed page now owns the sequence and the deployment page links to it. - Rung 2 does not meet its exit test. The claim tutorial renders no-match as `value: false` and calls the predicate false without contrasting the two. Recorded as work the rung still needs, with the reason the contrast matters: under a predicate disclosure the two results are identical on the wire, so only the disclosure mode distinguishes them. - The diagnostics index alone cannot gate undocumented codes. The Docs job is path-gated to docs-owned paths, so a change touching only diagnostics.rs skips `npm run check`. Either the classifier gains that path or the comparison runs in the project-authoring job. The same applies to the configuration references, whose sources are the authoring schemas and the two product config graphs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon Signed-off-by: Claude <jeremi@joslin.fr>
|
All four verified and fixed in 4906404.
Two notes on the reasoning, since both touch the plan's substance rather than its wording. On rung 2, I kept the exit test rather than relaxing it to match the tutorial. Rendering a no-match as On the path gating, this is the second instance of the same class of defect in this plan, after the regenerate-in-place gate from the previous round. Both were gates that would have looked green while permitting exactly what they were meant to prevent. The plan now states the triggering condition alongside the comparison for each proposed gate. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 490640490d
ℹ️ 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".
| 1. **The smallest first configuration is two or three fields.** Kong teaches a service with a name and | ||
| a URL. Tyk teaches a listen path and an upstream URL. Airbyte teaches an endpoint URL and a method. | ||
| SpiceDB teaches six lines with two relations and two permissions. Terraform teaches a provider and | ||
| two resources with no variables, no outputs, and no backend. None of them opens with a complete | ||
| object. |
There was a problem hiding this comment.
When this synthesis is used to design the onboarding ladder, the claimed consensus is contradicted by the review itself: this bullet describes SpiceDB's six-line schema and Terraform's multi-block configuration, while the OpenTelemetry section at lines 1175-1195 says its quick start uses zero YAML and then jumps to a complete pipeline. Recast this as a pattern followed by Kong, Tyk, and Airbyte rather than a finding shared by all six projects.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| 1. **Generate the two reference sections from schema.** A `generate-config-reference.mjs` step that | ||
| reads the five authoring schemas from `registryctl authoring schema --kind <kind>` and the two | ||
| runtime schemas from the committed `schemas/*.config.schema.json`, then writes |
There was a problem hiding this comment.
When this generator is added to npm run generate as proposed, the Docs checks job in .github/workflows/ci.yml:868-897 installs only Node dependencies and never builds or installs the repository's registryctl executable, so registryctl authoring schema ... will fail on every docs build. The implementation plan needs to build or otherwise provide the binary, invoke it through Cargo, or consume committed authoring-schema artifacts before wiring this step into npm run check.
Useful? React with 👍 / 👎.
| validates its configurations. And the site deliberately hardcodes an insecure value: an accepted issue | ||
| asked to "update all examples in opentelemetry.io to explicitly set the endpoint to `0.0.0.0`" | ||
| ([opentelemetry.io issue 3839](https://github.com/open-telemetry/opentelemetry.io/issues/3839)), so | ||
| every site example now permanently contradicts the security page's `localhost` recommendation, by | ||
| design, to shield readers from a default change. |
There was a problem hiding this comment.
When this observation is used to justify the secure-value recommendation later in A.6, the linked issue only records a request to update examples; it does not establish that every site example was changed, that all examples expose a listener, or that they still use 0.0.0.0. Anchor this universal claim in the site source or a test at the reviewed revision, or narrow it to the examples actually verified rather than inferring repository-wide current behavior from an issue.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| **Plane 1, authored intent.** Five YAML surfaces, each with a strict schema printable from | ||
| `registryctl authoring schema --kind` and wired into VS Code and Zed by `init`, plus a script adapter | ||
| when the capability requires one: |
There was a problem hiding this comment.
When readers enter this architecture through the spreadsheet rung, the five-surface map does not describe the project they actually receive: init_spreadsheet_api creates registryctl.yaml, compose.yaml, and relay/config.yaml (crates/registryctl/src/lib.rs:2483-2501), and the published spreadsheet tutorial explicitly tells the adopter that relay/config.yaml is configuration they own and edit (publish-spreadsheet-secured-registry-api.mdx:362-405). Either map this separate authored runtime-config workflow and its ownership boundary or remove it from the supposedly unified ladder; otherwise the first two rungs contradict the plane model introduced immediately afterward.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
Fourth review round, each claim verified against source: - The plan presented one unified authoring model while the first two rungs use another. `init relay <dir>` writes `registryctl.yaml`, `compose.yaml`, and `relay/config.yaml`, with no project file, integration, environment, or build step, and the spreadsheet tutorial tells the adopter that `relay/config.yaml` is configuration they own. B.1 now names workflow A, the project model the three planes describe, and workflow B, the direct Relay project where a file that is generated output in A is authored input. The chooser must say which workflow a source shape puts the reader in. - The generated-reference step must read committed files only. The Docs checks job has no Rust toolchain and no built CLI, so a generator shelling out to `registryctl authoring schema` would fail every docs build. The five authoring schemas need the generate-and-check treatment the two runtime schemas already have, after which the docs step consumes committed artifacts, as `generate-project-starters.mjs` already does. - Minimal-first is five of six, not a consensus. The collector ships zero YAML then jumps to three pipelines, so it is now stated as the counterexample rather than omitted from the finding. - Narrow the endpoint claim to what was verified. The configuration page carries five `0.0.0.0` receiver endpoints against one `localhost`, read at `main` on 2026-07-26, and the page annotates the divergence itself. The previous wording inferred repository-wide behavior from an issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon Signed-off-by: Claude <jeremi@joslin.fr>
The spreadsheet row uses the direct Relay workflow rather than the five authored project surfaces, so the chooser page must tell a reader which workflow their source shape puts them in. The file they edit and the reference page they need both change with it. Completes the workflow correction from the previous commit, where this edit did not apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon Signed-off-by: Claude <jeremi@joslin.fr>
|
All four verified and fixed in 3f59f8b and 42f8669. The spreadsheet-workflow finding was the most consequential of the whole review, and it invalidated the plan's central structure rather than a detail of it. Verified: B.1 now opens by naming both:
Two consequences are recorded: the ownership table needs a workflow column, and the spreadsheet rung cannot link
Note on the split commits: the chooser edit in 3f59f8b failed to apply because of a working-directory error, so 42f8669 completes it. The two belong together. Across four rounds this review has caught, in my own plan, two gates that would have passed while permitting what they existed to prevent, one gate that could not have run at all, one gate whose generator would have crashed the build, and one architectural claim contradicted by the ladder it was meant to organize. The value is in the verification, and I would rather these were found here than in the pages built from it. Generated by Claude Code |
|
The five red checks on 3f59f8b are a superseded-run artifact, not a failure. All of them report The run on the current head is clean so far: Incidentally the cancelled run confirms two claims the document makes about CI path gating: the classifier set Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42f866955f
ℹ️ 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".
| live-test loop, which catches drift by execution. Documented pytest-based unit, integration, and | ||
| acceptance testing exists separately and does not appear to assert declarative-schema conformance |
There was a problem hiding this comment.
When this comparison is used to choose drift controls, the live-test loop cannot catch the drift described in the preceding sentence: it executes a manifest against Airbyte's schema/runtime, but the hand-authored reference page is not an input to that execution. The prose can therefore disagree with declarative_component_schema.yaml while every connector test passes; describe this as runtime-conformance coverage and keep the reference/schema drift explicitly ungated.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| hardcoding `0.0.0.0` across every site example to smooth a future default change left the whole | ||
| corpus contradicting the security page permanently. Render shared blocks once from a schema |
There was a problem hiding this comment.
When this recommendation is used to characterize OpenTelemetry's corpus, "across every site example" again makes an unsupported site-wide claim. Fresh evidence in this revision is the explicit scope statement at lines 1305-1308: only five receiver endpoints on the configuration page were verified, and the text says this is not a claim about every site example. Preserve that same scope here rather than reverting to the universal conclusion.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| **Plane 2, generated product input.** `registryctl build --environment <name>` writes | ||
| `.registry-stack/build/<env>/` with a `reviewable/` half (`entities/<id>.json` for each declared | ||
| entity, `integration-packs/`, `consultation-contracts/`, and `review.json`) and a `private/` half (`relay/config/relay.yaml`, | ||
| `notary/config/notary.yaml`, each with `approval/review.json`). These validate against |
There was a problem hiding this comment.
For every generated Relay or Notary product input, this inventory omits approval/project-state.json, which write_compiled_project writes beside approval/review.json. This is not incidental metadata: verified-baseline loading requires the file, validates its registry.project.approval-state.v1 schema, and uses it to bind the review digest and generated product closure. Include it in the generated-plane model so the proposed signing and baseline-review documentation does not hide a required signed artifact.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| Runtime configuration [new] generated from the two config schemas | ||
| Relay | ||
| Notary |
There was a problem hiding this comment.
When this target tree is implemented, marking both runtime references as new would duplicate pages already published by the site generator: repo-docs.yaml maps crates/registry-relay/docs/configuration.md to products/registry-relay/configuration and products/notary/docs/operator-config-reference.md to products/registry-notary/operator-config-reference. Both product test suites also enforce exact bidirectional key-path parity between those references and their generated schemas. Treat schema-rendered tables as an enhancement or replacement for these existing reference pages rather than creating a parallel runtime-reference hierarchy.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| fixture files rather than running the CLI. So the authoring schemas need the treatment the runtime | ||
| schemas already have, a `just`-style generate-and-check pair on the CLI side that commits the five | ||
| files and fails Rust CI on drift, after which the docs step consumes committed artifacts. |
There was a problem hiding this comment.
When this implementation is scoped, the proposed generate-and-commit prerequisite is already satisfied: all five files exist under crates/registryctl/schemas/project-authoring/, ProjectSchemaKind embeds those exact bytes with include_str!, and the project-authoring tests assert that editor output matches the committed release schemas byte for byte. The docs generator can read those files directly now; reserve a new CLI-side generator for a separate decision to change their source of truth rather than presenting committed authoring artifacts as missing.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
Fifth review round, each claim verified against source: - The five authoring schemas are already committed at `crates/registryctl/schemas/project-authoring/` and embedded with `include_str!` (`project_authoring/editor.rs:72`), which makes the committed file the source of truth. The previous revision asked for a CLI-side generator that is not needed. Named the residual gap instead: nothing found asserts those schemas agree with the `deny_unknown_fields` structs they describe. - Both runtime configuration references already publish through the `repo-docs.yaml` sync as `products/registry-relay/configuration` and `products/registry-notary/operator-config-reference`, so the tree entry is an in-place enhancement rather than a new page, and the sequencing step says so. - Add `approval/project-state.json` to the generated plane. It carries `registry.project.approval-state.v1` (`crates/registryctl/src/project_authoring.rs:45`), verified-baseline loading requires it, and it binds the review digest to the generated product closure. A signed artifact the docs omit is one an operator cannot be asked to review. - Separate Airbyte's live-test loop from documentation consistency. It gives runtime-conformance coverage, but the prose reference is not an input to that execution, so reference-against-schema agreement is ungated. - Keep the narrowed endpoint scope in the recommendation, which had reverted to a site-wide claim the body no longer makes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cBAHjNtnkJEdCT4YjKXon Signed-off-by: Claude <jeremi@joslin.fr>
|
All five verified and fixed in 6ff8451.
Two notes. The authoring-schema finding corrected a correction. Last round I said those schemas needed the generate-and-check treatment the runtime schemas have; they need nothing, because The Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ff8451862
ℹ️ 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".
| (`crates/registryctl/src/lib.rs:2483`). There is no project file, no integration, no environment, and no | ||
| `build` step. The adopter edits Relay runtime configuration directly, and the tutorial says so in as many | ||
| words: "The floor that hid east and west is configuration you own. Open `relay/config.yaml`" | ||
| (`tutorials/publish-spreadsheet-secured-registry-api`). Rungs 1 and 2 live here. |
There was a problem hiding this comment.
When an adopter reaches rung 2, registryctl add notary creates notary/project/registry-stack.yaml, an environment, an integration, fixtures, and an entity, while prepare_notary_runtime compiles that nested project into .registry-stack/build/local/. Rung 2 therefore straddles workflows A and B; assigning it wholly to the supposedly project-free workflow hides the authored surfaces, generated plane, and references readers need when editing the claim. Model this rung as a hybrid workflow rather than saying it lives entirely here.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| and `project_authoring/editor.rs:72` embeds those exact files with `include_str!`, which makes the | ||
| committed file the source of truth rather than a copy of one. No new CLI-side generator is required, | ||
| and proposing one would invent work. |
There was a problem hiding this comment.
When this plan generates the authored reference from the committed schemas, embedding those files makes them authoritative only for editor output, not for CLI validation: project_authoring/output.rs::parse_yaml deserializes directly into the Serde models, while ProjectSchemaKind::document() merely returns include_str! bytes. A model change can therefore leave the generated reference stale while both the CLI and docs generator succeed, as the unclosed parity gap acknowledged below confirms. Add parser/schema parity as a prerequisite or derive the reference from the runtime models instead of declaring these files the source of truth.
AGENTS.md reference: docs/site/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
1 participant
Pull Request
Summary
Owning area:
docs/site/. One new file,docs/site/docs/plans/2026-07-26-configuration-documentation-architecture.md. No published page, no code, no generated artifact changes.A documentation-only review of six projects that ship user-authored configuration: Kong Gateway, Tyk, Airbyte Connector Builder, SpiceDB, Terraform, and the OpenTelemetry Collector. Each is reviewed on the same axes, and every observation links the public page it came from.
Part A, per project: configuration-domain map with ownership boundaries, the smallest configuration the docs teach first quoted verbatim, the documented progression to advanced cases, a simplified documentation tree classified by page type, how defaults and generated values and secrets and environment bindings and validation are explained, how examples are kept consistent with schemas and runtime behavior, friction from terminology and duplicated fields and reference-heavy pages, the line between authored intent and generated or runtime configuration, then three practices to adopt and two to avoid.
Part B, a proposed Registry Stack documentation architecture:
http,script,snapshot) as one ladder with a chooser keyed by what the reader already has.build, and runtime and evidence state.test,check --explain,build, and Config Bundle signing.registryctl.authoring.*diagnostic codes.Findings that shaped the proposal:
just config-schema-generateandconfig-schema-checkalready reproduce both runtime schemas from the typed config graphs and fail CI on drift, so a generated configuration reference can consume an already-gated artifact;ProjectStarterSequenceandProjectWorkspaceJourneysalready generate published command sequences from tested fixtures. The gaps are that neither the five authoring schemas nor the two runtime schemas have a published reference page, the authoring diagnostic codes have no index, and no page names the source-shape choice.Checks
npx markdownlint-cli2on the new file: 0 issues. Note that.markdownlint-cli2.yamlignoresdocs/**and Vale runs only oversrc/content/docsandREADME.md, so this file sits outside the published-content gates by existing configuration.npm test: 53 pass, 9 fail. All nine fail withERR_MODULE_NOT_FOUND: Cannot find package 'yaml'because this environment has nonode_modulesinstalled. Pre-existing and environmental; the change adds no script and is not read by any script.npm run checknot run: it requires the full dependency install plus a site build, and this change adds no content undersrc/content/docs/, no data file, and no generated output.Notes
openapi.jsoncontract a configured instance serves. The proposal takes the first reading and states what changes under the second.docs/site/scripts/and thenpm run checkgate.DCO
Signed-off-bytrailer.Generated by Claude Code