feat: add conventions/ prose (engineering conventions + review criteria) - #27
Merged
Conversation
Stand up conventions/ as the reasoning-only tier of the catalog — the prose standards tooling cannot enforce, completing Phase 4 of the migration plan. - engineering/: the standards a contributor writes by (philosophy, architecture & design, DDD, naming, simpler-code, code-organization, reference-don't-duplicate, documentation-and-citations, legacy-and-migration-debt, enforceability-tiers). - review/: the criteria a reviewer applies (code-quality, architecture, error-handling, concurrency, performance, security, cross-platform, observability, testing, ai-generated-code) — agnostic by default with thin per-stack overlays (dotnet/python/typescript) mirroring modules/*. Re-derived against authoritative sources, not ported. Curated, not exhaustive: agent-harness machinery and product-specific prose stay upstream; rules already enforced by a modules/* config or a hook lane are referenced, not restated (single-owner). Distribution follows consumption — prose is read by humans/agents, adopted by copy or pointer with no runtime coupling. No fixtures (a reasoning-only standard has no script-assertable pass/fail); dogfoods the repo's own markdown/typos/editorconfig/lychee lanes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: f87cefe7f7
ℹ️ 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,27 @@ | |||
| # Security review criteria | |||
|
|
|||
| Diff-time checks for the security concerns automated tooling cannot fully catch — trust boundaries, injection, data exposure, and supply chain. Severity labels are defined in [README.md](README.md). Two mechanical backstops already exist in the catalog: secret scanning (`modules/gitleaks/`) and dependency-vulnerability scanning (`modules/osv-scanner/`); review owns the judgment they cannot make. | |||
There was a problem hiding this comment.
This paragraph tells adopters that dependency-vulnerability scanning is a catalog module at modules/osv-scanner/, but the repository has no such module under modules/; the only OSV scanner wiring I found is the CI job in .github/workflows/ci.yml. Anyone following the new security convention will look for a copyable module that cannot be adopted, so this should either name the workflow/backstop that exists or add the missing module.
Useful? React with 👍 / 👎.
kyle-sexton
added a commit
that referenced
this pull request
Jun 25, 2026
The powershell lane intermittently fails on the transient PSScriptAnalyzer #1708 subprocess crash. ci-workflows #27 added a retry; re-pin the powershell action from 9780be3 to fb60ebe to adopt it (predates #30's git-tracked discovery change, so lint scope is unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5 tasks
kyle-sexton
added a commit
that referenced
this pull request
Jul 15, 2026
…on governance (#123) ## Summary Closes decisions **#27**, **#63**, and **#65** from the [Decisions Log](https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63) (evidence/rationale: https://claude.ai/code/artifact/3160ae0e-c02f-4619-8de3-60d73faa1100). ### #63 metadata-standards-copy-vs-cite-doctrine Decision: **keep the "copy" adoption path, require back-link + drift-check.** Adds `distribution/governance-process.md`. `conventions/README.md` already documents two adoption paths for reasoning-only prose — copy into the consumer's tree, or point at this repo. The copy path stays available (prose has no runtime coupling to enforce reconciliation the way `sync-manifest.yml` does), but a copy now carries two mandatory requirements: - **Back-link** — every copied file cites its exact canonical source (repo + path, inline header where the format allows it), using the same stable-anchor citation discipline `reference-dont-duplicate.md` already prescribes for the "Expose" file role. - **Drift-check** — the adopting repo owns a periodic diff of its copy against the current canonical source, with a named owner and trigger, mirroring the recheck-trigger discipline `documentation-and-citations.md` already requires for time-bound external claims. The doc is explicit that this is distinct from a `managed` `sync-manifest.yml` component, which already reconciles automatically and deliberately carries no downstream receipt — the back-link/drift-check burden applies only where nothing else keeps a copy current. ### #65 naming-cross-doc-reconciliation-process-ownership Decision: **establish a standing cross-reference review step for normative-doc changes.** Same doc adds the standing process step: before a change to a normative doc (`naming.md`, `process/issue-tracker.md`, `review/code-quality.md`, and any other file in the catalog stating a rule another doc cites or assumes) merges, a cross-reference check confirms no other doc now contradicts it. Ownership is named explicitly rather than left implicit: `required_approving_review_count` stays at `0` org-wide (decision #11, single-maintainer), so there is no independent blocking reviewer to gate this. The step is documented as a **self-review checklist item the author performs before merge**, with a documented future automation path — extending the periodic cross-plugin-source consistency check (decision #37, for `claude-code-plugins`) to this repo's own normative docs once that check exists, so the control stops depending solely on author diligence. `conventions/README.md` gets a new "Changing a normative file" pointer to this requirement; `distribution/README.md` gets a pointer alongside its existing `THREAT-MODEL.md` reference. ### #27 tooling-gov-conventions-graduate-to-enforced Decision: **graduate mechanically-checkable rules into sync-manifest-tracked components.** Reviewed every file under `conventions/` against `enforceability-tiers.md`'s deterministic/detect-then-judge/reasoning-only split. Finding: **no ungraduated deterministic rule exists to graduate.** Every deterministic (mechanically-checkable) rule already points to its owning component instead of restating it: | Convention area | Deterministic rule | Owning component | Tracked in `sync-manifest.yml`? | |---|---|---|---| | Secrets (`review/security.md`) | no secrets in source | `gitleaks` | yes | | Comments (`review/code-quality.md`) | debt markers / tracker provenance | `comment-hygiene` | yes (`comment-hygiene-action`) | | Citations (`documentation-and-citations.md`) | cited URL resolves | link check | yes (`lychee`) | | TypeScript/JS (`review/overlays/typescript.md`) | lint/format/import order, type correctness | `biome`, `tsconfig` | **no** — deliberately native-package (`extends`) adoption per `distribution/README.md`'s ownership model, not exact materialization | | Python (`review/overlays/python.md`) | lint/format, type correctness | `ruff`, `pyright` | yes | | .NET (`review/overlays/dotnet.md`) | analyzers, code style, banned symbols | `dotnet-analysis` | yes | | Container build (`container-supply-chain.md`) | Docker Build checks, OSV scans | *(none yet)* | **no** — the convention itself explicitly defers this pending live-consumer admission evidence per `docs/component-lifecycle.md`; graduating it without that evidence would violate the same lifecycle contract | | PR titles (`review/code-quality.md`) | Conventional Commits format | *(ci-workflows `pr-title.yml`)* | out of this repo's materialization surface — owned by `ci-workflows`, per README.md's ownership boundaries | | Naming (`naming.md`), label/issue-tracker usage (`process/issue-tracker.md`), duplication (`reference-dont-duplicate.md`) | — | — | self-declared reasoning-only; no deterministic subset exists to graduate | No `sync-manifest.yml` change is included. The one adjacent orphan noticed in passing — `components/lefthook-typescript` exists, is fully built, but is not registered in `sync-manifest.yml` for any target — is tooling infrastructure, not a `conventions/` rule, and assigning it to a target's `managed:` list is a separate adoption decision requiring its own admission evidence; flagging it here for a follow-up rather than folding it into this PR. ## Test plan - [x] `npx markdownlint-cli2` — 0 errors across the full repo (71 files, including the new/changed docs) - [x] `lychee` — 21/21 links OK on the changed files (0 errors) - [x] `typos` — clean on the changed files - [x] `lefthook` pre-commit (typos, editorconfig, gitleaks, markdownlint) — all passed at commit time - [ ] Maintainer confirms the #27 classification table before merge — no code/config changed, so this PR is prose-only and carries no behavioral risk 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes Phase 4 (Prose) of
docs/migration-plan.md: stands upconventions/as the reasoning-only tier of the catalog — the prose standards tooling cannot enforce.What's here
conventions/engineering/— the standards a contributor writes by: engineering-philosophy, architecture-and-design, domain-driven-design, naming, simpler-code, code-organization, reference-don't-duplicate, documentation-and-citations, legacy-and-migration-debt, enforceability-tiers.conventions/review/— the criteria a reviewer applies: code-quality, architecture, error-handling, concurrency, performance, security, cross-platform, observability, testing, ai-generated-code — agnostic by default, with thin per-stack overlays (overlays/{dotnet,python,typescript}.md) mirroring themodules/*overlays.Discipline
.claude/, slash-commands,AGENTS.md/REVIEW.md,Platform.*,.work/, monorepo paths).modules/*config or a hook lane are referenced, not restated (no prose⇄config drift).modules/*config.Self-lint (local)
markdownlint (45 files, 0 errors), typos (0), lychee offline (39 links OK, 0 errors), editorconfig (final newline / no trailing whitespace / LF / no tabs), comment-hygiene (no debt markers), machine-specific-paths (none) — all clean.
🤖 Generated with Claude Code