Skip to content

Add contributor workflow and templates - #137

Merged
plx merged 6 commits into
mainfrom
agent/issue-70-contributor-workflow
Jul 27, 2026
Merged

Add contributor workflow and templates#137
plx merged 6 commits into
mainfrom
agent/issue-70-contributor-workflow

Conversation

@plx

@plx plx commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Pull request

Problem

Issue #70 identified that contributors could not discover the supported setup, one-issue workflow, deterministic red-before-fix evidence, hermetic-test rules, complete validation matrix, or maintainer triage policy from the repository alone.

Before behavior

At base commit b2e3ec848ba6e2553fdecfa5f7e6f98bcefb482e, the repository had no root contributor guide, issue forms, chooser configuration, pull request template, or fail-closed contract for those artifacts.

After behavior

Adds a personal-repository contributor workflow, structured bug and contract-proposal forms, a private-security contact route, a pull request template, a constrained standard-library validator with negative tests, CI integration, navigation/docs updates, and a dated acceptance audit. It does not create an organization or team and does not add or run fuzzing.

Red-before-fix evidence

Base commit SHA: b2e3ec848ba6e2553fdecfa5f7e6f98bcefb482e.

Exact pre-fix command: cargo test --locked --test issue_70_contributor_workflow -- --nocapture.

It exited 101 with all three tests failing for the intended missing artifacts: CONTRIBUTING.md, .github/ISSUE_TEMPLATE/01-bug.yml, and the issue #70 audit. Compilation and setup succeeded.

Validation

Focused post-fix: three issue #70 Rust contracts, ten Python checker tests (including the checked-in repository), live checker, and a second Ruby YAML parse passed. The workflow-job proof explicitly establishes LF/CRLF equivalence after the first Windows run exposed the original LF-only extractor.

Full post-fix: exact Rust 1.85.0, 1.96.1, and 1.97.1 locked lanes passed; the conformance suite passed 393 tests with three documented ignores; the explicit packaged-artifact test passed separately; 101 policy tests and five deterministic parser-matrix tests passed. Exact package and publish dry-run commands passed from clean commit 34ce3479b8811b6afc85ccccdb41954be7fe4c1a. Formatting, actionlint, zizmor, rumdl, 95 link checks, and release-mode navigation verification passed.

Local platform: macOS. Hosted CI supplies the authoritative Linux, macOS, and Windows matrix.

Documentation and compatibility

Documentation impact: adds CONTRIBUTING.md and the issue #70 audit; updates README, changelog, and navigation guide. Compatibility impact: no CLI, guide-language, package, or supported-platform behavior changes. The static site remains excluded.

Security and sensitive data

Security impact: public forms route suspected vulnerabilities to the existing private advisory URL and warn against secrets, personal data, and private paths. The checker is standard-library-only and reads fixed repository files. The diff contains no token, private report detail, or path-sensitive fixture.

Dependencies and issue graph

Dependency or license impact: none; Cargo manifests, lockfile, graph, and attribution are unchanged. Closed prerequisites #55, #57, and #60 remain reflected in the guide. Open issue #63 still blocks publication on crates.io Trusted Publisher, and deferred issue #56 remains the only authority for any future generated fuzz/property work.

Checklist

  • This pull request addresses one issue and contains no unrelated cleanup.
  • The regression is deterministic, hermetic, bounded, and placed at the narrowest useful layer.
  • Focused and full validation results are recorded above.
  • User-facing and normative documentation are aligned where applicable.
  • Dependency, license, package, release, and security impacts are explicit.
  • Every review conversation is resolved before merge.

Closes #70

@github-actions

Copy link
Copy Markdown

Review

Overall this is a thorough, well-scoped PR that follows the repository's existing conventions closely: SHA-pinned actions, fail-closed policy checkers with negative-path unit tests, navigation-guide/README/CHANGELOG kept in sync, and a Rust conformance test binding the new docs/CI/templates together. Nice work.

Code quality / design

  • scripts/check_contributor_templates.py is a deliberately constrained hand-rolled YAML-subset parser (two-space indent, JSON-style quoted scalars, no aliases/anchors, explicit booleans). It's fail-closed by construction — anything outside the grammar raises rather than being silently accepted — and is exercised by 9 focused unit tests plus a live run against the real templates in CI. That's a solid approach given the "standard-library-only, no new library facade" constraint from CLAUDE.md.
  • split_mapping/quoted_scalar correctly handle values containing ": " (e.g. "Ratio: 1:2") because the split only happens once and the quoted-string parsing is delegated to json.loads, which also incidentally forbids trailing comments — consistent with the "constrained subset" intent.
  • The required-id sets in check_repository (lines ~1141–1166) line up exactly with the id: fields actually used in 01-bug.yml and 02-contract-proposal.yml, so the artifacts and the checker won't silently drift apart.

Minor findings

  1. test_check_contributor_templates.py isn't wired into a just recipe. CONTRIBUTING.md's "Validation matrix" section lists just test-production-readiness-selector, just test-release-identity, just test-github-protections, and just test-quality-gates as the standard-library policy suites, then immediately follows with a raw PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests/test_check_contributor_templates.py -v / python3 scripts/check_contributor_templates.py outside of just. The justfile groups the sibling checkers (check_coverage.py, check_mutation_report.py, run_performance_baseline.py) under test-quality-gates, but this new suite isn't added there or given its own recipe. Given CONTRIBUTING.md itself frames just 1.51.0 as "Versioned repository task entry points," a contributor who runs only the documented just targets would skip this regression suite. Worth adding it to test-quality-gates (or a new test-contributor-templates recipe) for consistency — CI itself calls the script directly so this doesn't affect CI correctness, only local contributor ergonomics (which is somewhat the point of this PR).

  2. The new issue_70_contributor_guide_is_complete_and_uses_real_pinned_commands Rust test isn't scoped to the job it's meant to verify. It asserts ci.contains("python-version: \"3.12\"") against the whole ci.yml file, but that exact string already existed before this PR (in the pre-existing issue-selector job). So the assertion would pass even if the new actions/setup-python step this PR adds to the workflow-lint job were missing. Low severity — it matches the existing whole-file .contains() style used throughout this repo's other issue_XX tests — but it means this particular check doesn't actually prove the new step's wiring; only the live CI run does.

  3. Nit: the new rumdl check --disable MD013 CONTRIBUTING.md .github/pull_request_template.md only disables line-length (MD013), while the README's rumdl invocation also disables MD010 (hard tabs) and MD038 (spaces inside code spans). If that's intentional (e.g. because these two new docs don't hit those rules), no action needed; just flagging in case it was meant to mirror the README invocation.

Security

No concerns. The checker only reads fixed repository-relative paths, the private-vulnerability route is hardcoded to the correct advisory URL, and the new GitHub Actions step uses a SHA-pinned actions/setup-python.

Test coverage

Good — 9 Python unit tests cover the positive path plus unquoted scalars, duplicate ids, missing required flags, aliases/odd indentation, PR-template heading/placeholder violations, and the wrong private route; 3 Rust tests bind the guide text, templates, and CI/doc wiring together. The one gap noted above (#1) is about discoverability/ergonomics of running the suite locally, not about coverage of the checker logic itself.

@plx

plx commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Addressed the automated cold-review findings in c83f89e:

  1. Added just test-contributor-templates, documented it, installed pinned just 1.51.0 in the owning CI job, and made CI exercise the recipe.
  2. Tightened the Rust contract to extract workflow-lint and prove that job owns Python 3.12, pinned just, and the contributor-template recipe.
  3. Kept MD010 and MD038 enabled intentionally; both new maintained Markdown files pass those stricter rules.

The dated audit now records the cold-read findings and their dispositions. Focused Rust/Python tests, exact workflow lint, Markdown/link checks, and navigation verification pass locally after the update.

@github-actions

Copy link
Copy Markdown

Review

Nicely scoped, and consistent with this repo's fail-closed conventions (issue-numbered test, standard-library-only checker, negative regressions, CI wiring, navigation guide / README / CHANGELOG kept in sync per CLAUDE.md). A few notes, nothing blocking:

Code quality

  • scripts/check_contributor_templates.py is a clean, deliberately narrow hand-rolled YAML-subset parser (two-space indent, JSON-quoted scalars, no aliases/anchors). The grammar is well-matched to the actual template files and fails closed on anything outside it (unquoted scalars, aliases, odd indentation, duplicate ids, missing required flags, etc.), which fits the project's "constrained standard-library validator" philosophy.
  • split_mapping splits on the first ": " in a line to separate key/value. That's safe today because every quoted value in these files follows immediately after key: , but it's a latent gotcha if a future key's name ever contained ": " before the real separator — worth a one-line comment if the grammar is ever extended, though not worth guarding against now given the fixed, reviewed file set.
  • tuple(top) != ("name", "description", "title") (script line ~1071) verifies the relative order of those three scalar keys, but doesn't constrain where labels:/body: fall relative to them (they're tracked separately in seen_top/section, not in top). Purely cosmetic given the two real templates already follow the canonical order — not worth extra code for the current fixed inputs.

Bugs

  • Nothing that looks like it would misfire against the two real forms, the chooser, the PR template, or CONTRIBUTING.md. The CI placement of the new actions/setup-python / taiki-e/install-action steps inside workflow-lint (right after checkout) looks correct, and permissions: was already absent from that job before this PR — not something this change introduces.

Test coverage

  • Good breadth: 9 Python unit regressions (unquoted scalar, duplicate id, missing required, aliases, odd indentation, PR-template numeric closer, chooser private-route substitution, missing-artifact fail-closed) plus 3 Rust contract tests binding CONTRIBUTING.md/templates/CI/justfile/navigation guide/audit together, plus (per the PR's validation notes) a live run of the checker and an independent Ruby Psych.safe_load parse.
  • One gap: tests/test_check_contributor_templates.py never calls checker.check_repository(ROOT) against the actual checked-in .github/ISSUE_TEMPLATE/*.yml / pull_request_template.md / CONTRIBUTING.md as a positive case inside the unittest module itself — that path is only exercised indirectly via just test-contributor-templates running the script directly against the repo, and via main()'s smoke path. Pinning a check_repository(ROOT) call as an explicit passing unit test would make that contract self-evident from the test file alone rather than depending on the separate script invocation succeeding.

Security

  • No CLI/runtime behavior change — this is process/documentation/CI tooling only. The private vulnerability route (security/advisories/new) is consistently wired through the issue chooser, the bug form's markdown note, and CONTRIBUTING.md, and is asserted by both the Python checker and the Rust contract test. No secrets, tokens, or sensitive fixtures introduced.

Performance

  • N/A — one extra CI step (just test-contributor-templates) running a small standard-library script against a handful of fixed files.

Overall: solid, well-tested addition that matches the repository's documented conventions. The two notes above are minor/non-blocking.

@github-actions

Copy link
Copy Markdown

Review: Add contributor workflow and templates (#70)

Overview

This PR adds a CONTRIBUTING.md, structured GitHub issue forms (bug report + contract proposal), an issue-template chooser that disables blank issues and points to the private security-advisory route, a pull request template, a strict standard-library-only Python checker (scripts/check_contributor_templates.py) enforcing a deliberately constrained YAML subset for those artifacts, CI wiring (just test-contributor-templates in the workflow-lint job), navigation-guide/README/CHANGELOG updates, and a dated acceptance audit. No CLI, guide-language, or dependency behavior changes.

Code quality and style

  • The change follows the repo's established heavy-evidence pattern (audit doc, red-before-fix note, navigation-guide sync, README link) very consistently with CLAUDE.md's documentation-alignment rules.
  • check_contributor_templates.py is a well-organized, dependency-free parser for a purposely tiny YAML subset (quoted-only scalars, 2-space indent, no anchors/aliases). The dataclasses (FormItem, IssueForm) and fail()/TemplateContractError pattern keep error reporting consistent and line-addressable.
  • New actions in ci.yml (actions/setup-python, taiki-e/install-action) are pinned to a commit SHA with a version comment, consistent with the rest of the workflow's pinning convention.
  • The CI/justfile/CONTRIBUTING/audit cross-references stay in sync in this diff (verified the workflow-lint job precedes issue-selector as the new Rust test assumes, and that the LF/CRLF-normalizing workflow_job() helper is exercised both ways).

Specific suggestions

  1. Heading detection is fence-unaware. validate_contributing() and ordered_headings() scan for lines starting with "# "/"## " anywhere in the file, including inside fenced code blocks. Today none of the code blocks in CONTRIBUTING.md/pull_request_template.md happen to contain a line starting with #, so it passes, but a future edit that adds a shell comment or Markdown example inside a fence starting with #/## would silently corrupt the heading-order check (or fail confusingly). Worth excluding fenced code blocks when scanning, or noting the constraint near the function.
  2. Anchor/alias detection is a line-prefix heuristic, not real YAML: checked_lines() only rejects &/*/!/<<: when they appear at the start of the (possibly list-prefixed) line. An alias used as a nested mapping value (e.g. label: *ref) wouldn't be caught by this check alone (it would likely fail elsewhere in the constrained grammar, but not for the stated "aliases are forbidden" reason). Low risk given the tiny grammar and the two checked-in forms, but worth a comment or a targeted negative test if this class of input is a real concern.
  3. Test-coverage gaps in the checker's negative-path branches. The unit tests cover the most likely mistakes (unquoted scalar, duplicate id, missing required, YAML alias, bad indentation, numeric closer, missing heading, wrong security route, missing artifacts), but several fail(...) branches are untested: duplicate top-level key, form name <= 3 chars, no labels, empty body, unsupported top-level/body/attribute key, dropdown missing options, non-dropdown with options. Since this checker is the sole automated gate for the issue-form grammar, a few more negative tests would reduce the chance of a silent regression later.

Potential issues / risks

  • Nothing that looks like a functional bug in the reviewed diff. The constrained-grammar checker fails closed (missing files raise TemplateContractError, verified by test_missing_repository_artifacts_fail_closed), and the new Rust contract (tests/issue_70_contributor_workflow.rs) pins the exact CI commands, pins, and navigation-guide entries so the wiring can't silently drift.
  • The "second parse with Ruby's YAML implementation" mentioned in the audit as extra validation appears to be a manual/local step only -- it isn't wired into CI. That's consistent with the stated scope (no new CI tool), but it does mean the custom checker is the only automated defense against a YAML construct the constrained grammar doesn't anticipate.

Security

  • The chooser correctly disables blank issues and routes only to the real private security-advisories URL; both issue forms explicitly warn against posting vulnerability details/secrets/personal data, matching SECURITY.md's reporting route. No secrets, tokens, or path-sensitive fixtures are introduced.
  • No new dependencies, no network calls in the checker, no shell/command construction from untrusted input.

Test coverage

  • Good breadth: Rust contract (issue_70_contributor_workflow.rs) covers guide text, pinned versions/commands, CI wiring, and navigation-guide sync; Python unit tests cover the checker's main failure modes plus a "checked-in repository passes" integration test. See suggestion Refactor to use clap's built-in env attribute for environment variables #3 above for a few more edge-case branches worth covering.

Overall this is a thorough, well-evidenced, low-risk documentation/tooling addition that matches the repository's existing conventions closely. The suggestions above are minor robustness/coverage improvements, not blockers.

@plx

plx commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Final automated-review dispositions on ccfcc1b:

  1. Fence-contained heading-like lines are intentionally outside the current exact Markdown contract and fail closed; they cannot bypass or silently weaken a required heading. Fence-aware diagnostics can be a future ergonomics refinement if maintained examples need that syntax.
  2. A nested alias such as label: *ref is already rejected by the JSON-quoted-scalar rule, so the prefix check is defense in depth rather than a bypass boundary.
  3. The reviewed deterministic matrix now has ten Python tests, including the checked-in positive integration path, plus the live CLI-style checker, Rust cross-document contracts, and an independent Ruby YAML parse. Expanding every unreachable/fail-closed diagnostic branch is useful future hardening but is not required for this fixed two-form grammar.

No blocking review item remains. All final-head CI, Linux/macOS/Windows, package identity, guide, and review checks are green.

@plx
plx marked this pull request as ready for review July 27, 2026 19:55
@plx
plx merged commit 17c7727 into main Jul 27, 2026
18 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccfcc1b951

ℹ️ 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".

Comment thread README.md
- [security and vulnerability-reporting limitations](docs/v0.2-contract.md#security-and-vulnerability-reporting);
- [supported CLI-only product and compatibility](docs/v0.2-contract.md#supported-product-and-rust-api);
- [release and Rust support policy](docs/release-policy.md);
- [contribution guide](CONTRIBUTING.md);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the packaged contribution-guide link resolvable

The source package includes README.md but excludes CONTRIBUTING.md under the exact Cargo.toml allowlist (cargo package --locked --allow-dirty --list confirms only the README is present). Consequently, this relative link is broken when the packaged README is viewed from an extracted crate or another package-local context. Either include CONTRIBUTING.md in the reviewed package boundary or use an absolute repository URL.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add CONTRIBUTING.md, issue/PR templates, and a red-before-fix contributor workflow

1 participant