Skip to content

committer-onboarding — post-vote onboarding for committers and PMC members#371

Merged
potiuk merged 6 commits into
apache:mainfrom
justinmclean:contributor-onboarding
Jun 2, 2026
Merged

committer-onboarding — post-vote onboarding for committers and PMC members#371
potiuk merged 6 commits into
apache:mainfrom
justinmclean:contributor-onboarding

Conversation

@justinmclean

Copy link
Copy Markdown
Member

Summary

Rebased and carries on from #229

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package (tools/*/ with pyproject.toml)
  • Groovy reference impl
  • Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
  • Documentation (docs/, README.md, CONTRIBUTING.md)
  • Project template (projects/_template/)
  • CI / dev loop (prek, workflows, validators)
  • Other:

Test plan

  • prek run --all-files passes
  • For Python packages touched: uv run pytest / ruff check / mypy passes
  • For Groovy bridges touched: command-line invocation tested end-to-end
  • For skill changes: eval suite passes for the affected skill
    (PYTHONPATH=tools/skill-evals/src python3 -m skill_evals.runner tools/skill-evals/evals/<skill>/)
  • For skill behaviour changes: a new or updated eval fixture is included in this PR
    (a regression test for the bug fixed / the behaviour added — see CONTRIBUTING.md)
  • Other:

@justinmclean justinmclean self-assigned this May 29, 2026
@justinmclean justinmclean marked this pull request as ready for review June 1, 2026 01:55
@potiuk

potiuk commented Jun 1, 2026

Copy link
Copy Markdown
Member

Hey @justinmclean -> I have just added Ponymail and Apache Project MCPs as "default" for all ASF projects - shall we make those ASF skills into "asf" skill family :) and wire it in as well as "default" for ASF projects ?

Address the skill-validator findings surfaced by the pytest run on
this branch, plus two markdownlint MD040 errors the lint hook
surfaced on the touched committer-onboarding SKILL.md:

- committer-onboarding: add the standard Pattern 4 injection-guard
  callout. The skill reads the <vote-thread> from the mailing-list
  archive, candidate-supplied identity fields (name, email, desired
  Apache ID), and ICLA / Whimsy roster data; the callout names those
  surfaces explicitly. Golden rule 3 already reinforced the same
  principle; this adds the validator-recognised block. (HARD
  violation; was failing the pytest gate.)
- committer-onboarding: tag two existing untagged fenced output
  blocks (Step 0 output, Step 3 completion summary) as `text` so
  markdownlint MD040 stops flagging them on touch.
- security-issue-sync: add `--limit 100` to the milestone-siblings
  `gh issue list` count (was unbounded; silently capped at 30 on
  large repos).
- security-issue-triage: add `--limit 100` to the reviewed-by
  `gh pr list` search (same reason).
- setup-isolated-setup-doctor: move the docs/setup/
  sandbox-troubleshooting.md reference out of the frontmatter
  description into the body, so the matching-layer description stays
  tight and the criteria-source SOFT advisory clears. The body
  still documents the link extensively.
- committer-onboarding eval fixtures: append the missing trailing
  newline to 5 expected.json files.

Verified: `skill-validate --strict` reports OK (no violations);
`skill-validator` pytest suite is green; markdownlint passes.

Generated-by: Claude Code (Opus 4.7)
Self-review findings on PR apache#229:

- committer-onboarding step-0 output-spec.md: enumerate the
  `injection_detected` field in the bullet list. The expected.json
  in every step-0 case asserts it, but the spec's prose only
  described injection-detection behaviour without naming the output
  field — a model following the bullets strictly would have omitted
  the key.
- committer-onboarding step-2 output-spec.md: enumerate the
  `whimsy_url_contains` field (the PPMC-vs-PMC discriminator
  substring). Same pattern: asserted by expected.json, not in the
  spec's bullets.
- skill-evals runner.py --cli mode: switch run_cli from
  `subprocess.run(cli, shell=True)` to
  `subprocess.run(shlex.split(cli), shell=False)`. The operator's
  command string was already trusted (the docstring said so), but
  using an argv list rather than a shell string keeps the
  attacker-controlled prompt content (injection-case fixtures and
  their like) firmly on stdin, well away from any shell
  interpretation, and removes a class of accidental-metacharacter
  footgun in the operator's --cli value. Operators who genuinely
  need shell features wrap their command in `bash -c '<pipeline>'`.

One test follow-on (test_runner.py): the MANUAL-skips-CLI case
used `"exit 1"` (a shell builtin) to assert non-zero-rc handling;
under shell=False the builtin is not on PATH and would FileNotFoundError
instead of exiting 1. Swapped to `"false"` — a real binary that exits 1
the same way — with an inline comment explaining the constraint.

Verified: `skill-evals` pytest green; `skill-validate --strict` reports
OK (no violations); `skill-validator` pytest green.

Generated-by: Claude Code (Opus 4.7)
Markdownlint MD040 fenced-code-language: tag four template/output
blocks in the detail files as `text` — they're plain-text content
(email bodies, a URL template), not code in any real language.

- detail/karma-grant.md:104     Whimsy committer-profile URL template
- detail/email-templates.md:16  committer congratulations email body
- detail/email-templates.md:108 secretary account-request email body
- detail/email-templates.md:148 dev-list welcome announcement body

Verified with a broad markdownlint-cli2 sweep across all 76 changed
.md files on this branch — 0 errors remaining.

Generated-by: Claude Code (Opus 4.7)
run_cli switched to shell=False with shlex.split in 0a13c84, but the
test helper kept the shell env-var-prefix form which shlex.split
tokenises as a literal argv[0] binary name. Wrap the inner command in
bash -c so the env-var assignment is honoured.

Fixes 3 CI failures:
- test_batch_grade_single_pair_one_call
- test_batch_grade_many_pairs_one_call
- test_compare_with_grader_multiple_prose_mismatches_one_call
@potiuk potiuk force-pushed the contributor-onboarding branch from f110dfe to 1e6bc2f Compare June 2, 2026 11:37
@potiuk potiuk merged commit 51bf146 into apache:main Jun 2, 2026
26 checks passed
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.

2 participants