feat(fx-dev): carry PR conventions into every delegated spawn prompt - #25
Merged
Conversation
Add a verbatim-pasteable "PR conventions block" to the github skill and require /team to carry it into every spawn prompt whose agent may open or edit a PR. A convention that lives only in a skill does not survive delegation: a spawned agent inherits the prompt, not the skills. Also add a mechanical hard-wrap check to pr-preparer and a new merge gate (5c) in /team, and insert an explicit PR-conventions step into the coder skill's workflow. Claude-Session: https://claude.ai/code/session_01FYYKLM8tpaF1qqhtCANkSX
…apping The check shipped in the previous commit could not fail. Its first awk command incremented an unused variable and always exited 0, producing no output at all; its second sorted every line by length, including the lists, tables and fenced code the rule explicitly exempts, so any single long code or table line made a fully hard-wrapped body look healthy. Merge gate 5c in /team was built on that check, so the new mandatory gate could silently approve the defect it exists to prevent. Replace it at every site with one awk that tracks fence state, skips headings, blockquotes, tables and list items, and exits 1 printing HARD-WRAPPED when prose clusters in the 60-100 column band. Use POSIX [[:space:]] rather than the GNU-only \s. Also stop the pasteable PR conventions block from claiming its regex enforces lowercase-after-colon, imperative mood and no-trailing-period. It enforces the type prefix only. Name it the mechanical floor, give greps for the two remaining checkable rules, and say plainly that imperative mood has to be read by eye. Claude-Session: https://claude.ai/code/session_01FYYKLM8tpaF1qqhtCANkSX
A codex-review log was committed by accident in the previous commit. It is per-run scratch output, meaningless outside the run that produced it. Claude-Session: https://claude.ai/code/session_01FYYKLM8tpaF1qqhtCANkSX
The checker tracked only ``` fences and skipped only a list's marker line, so a body using ~~~ fenced code, or list items with indented continuation lines, had those lines counted as prose. Enough of them in the 60-100 column band made a correctly written body exit 1 as HARD-WRAPPED — a false positive on a mandatory merge gate, and on constructs the published rule states are exempt. Track ``` and ~~~ separately so only the opening marker closes its fence, and skip indented lines, which covers both list continuations and indented code blocks. Claude-Session: https://claude.ai/code/session_01FYYKLM8tpaF1qqhtCANkSX
…the check Two misclassifications remained. A fence was matched on a three-character prefix, so a ```` block legally embedding a ``` line closed early and its remaining code counted as prose. And every two-space-indented line was exempt, so a hard-wrapped top-level paragraph indented two spaces — legal Markdown — was skipped entirely and passed the gate. Track the fence character and length, closing only on the same character at a length at least its opening. Track list context, so an indented line is a continuation only inside a list and indented prose is still judged. Also state what the checker is: a heuristic, not a Markdown parser. Each pass of review found another construct a hand-rolled parser mishandles, and that supply does not run out. HARD-WRAPPED is now documented as a prompt to read the body, with explicit instructions not to rewrite correct prose to satisfy the checker and not to extend the awk to chase further constructs. The prose rule above it is what the gate enforces. Claude-Session: https://claude.ai/code/session_01FYYKLM8tpaF1qqhtCANkSX
There was a problem hiding this comment.
🟢 Approval recommended
The changes are internally consistent across the affected skills, low-risk (documentation/protocol enforcement), and include the required version bumps for both the plugin and marketplace metadata.
Pull request overview
This PR strengthens fx-dev’s delegated-agent workflow by making PR title/body conventions propagate reliably via spawn prompts (not just via loaded skills), and by adding an explicit merge gate to prevent hard-wrapped PR bodies from slipping through.
Changes:
- Added a verbatim “PR conventions block” (including the canonical “Mechanical body check”) to
fx-dev:githubfor pasting into any delegated agent prompt that may create/edit PRs. - Updated
fx-dev:team,fx-dev:coder, andfx-dev:pr-preparerto require/verify those conventions (including a new merge gate 5c for hard-wrapped bodies). - Bumped
fx-devand marketplace versions and ignored transient.claude/run artifacts/worktrees in.gitignore.
File summaries
| File | Description |
|---|---|
| plugins/fx-dev/skills/team/SKILL.md | Requires coordinators to paste PR conventions into spawn prompts; adds merge gate 5c for the canonical hard-wrap check. |
| plugins/fx-dev/skills/pr-preparer/SKILL.md | Adds an explicit post-edit verification step by restating and running the mechanical body check. |
| plugins/fx-dev/skills/github/SKILL.md | Introduces the canonical, pasteable PR conventions block and the named “Mechanical body check” referenced elsewhere. |
| plugins/fx-dev/skills/coder/SKILL.md | Adds an explicit step to load fx-dev:github and follow PR conventions before creating PRs. |
| plugins/fx-dev/.claude-plugin/plugin.json | Bumps fx-dev plugin version for the behavior/documentation changes. |
| .gitignore | Ignores transient .claude/team/waits/ logs and .claude/worktrees/ created during runs. |
| .claude-plugin/marketplace.json | Bumps marketplace metadata version due to plugin/top-level changes. |
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Summary
A convention that lives only in a skill does not survive delegation. A spawned agent inherits its parent's prompt, not its parent's skills, so a rule stated only in
fx-dev:githubnever reaches a coder that the/teamcoordinator spawns with an ad-hoc prompt. The observed consequence: every coder in a recent run received the PR title rule, becausefx-dev:teamrestates it inline and gates it at merge, and none received the body rule, which lived only infx-dev:github— so all three PRs shipped hard-wrapped bodies, while PRs prepared throughfx-dev:pr-preparerin the same repo did not.This adds a verbatim-pasteable PR conventions block to
fx-dev:githuband requiresfx-dev:teamto paste it into every spawn prompt whose agent may open or edit a PR.fx-dev:codergains an explicit conventions step before PR creation,fx-dev:pr-preparergains a mechanical verification step, andfx-dev:teamgains merge gate 5c for hard-wrapped bodies.Review
Codex converged over four passes: 2, 1, 2, then 1 non-blocking finding. Every blocking finding was in the mechanical check this change introduces, and the first pass caught it before the check ever ran anywhere.
As first written, the check could not fail. Its
awkincremented an unused variable and always exited 0, so it produced no output at all; the fallback sorted every line by length including the lists, tables and fenced code the rule explicitly exempts, so one long code line made a fully hard-wrapped body look healthy. Gate 5c was built on it, which means the new mandatory gate could have approved the exact defect it exists to prevent. Passes 2 and 3 then each found a further Markdown construct the replacement misclassified —~~~fences, indented list continuations, fence length, indented top-level prose.That was a diverging class rather than a queue of bugs, so pass 4 fixed the cause instead of another instance. The fence handling now follows CommonMark on character and length, list continuations are exempt only inside an actual list, and — the part that ends the class — the artifact now says plainly that the checker is a heuristic and not a Markdown parser, that
HARD-WRAPPEDis a prompt to read the body rather than proof, and that a reader must neither rewrite correct prose to satisfy it nor extend the awk to chase further constructs. The prose rule above the command is what the gate enforces.Both copies of the check were extracted back out of the committed markdown and executed against six bodies, producing identical results: a four-backtick fence embedding a three-backtick line (pass), a two-space-indented hard-wrapped paragraph (fail), a list with indented continuations (pass), a
~~~fence of wrapped-width lines (pass), genuinely hard-wrapped prose (fail), and a correct one-line-per-paragraph body (pass).The second pass-1 finding was that the block's regex claimed to enforce lowercase-after-colon, imperative mood and no-trailing-period while enforcing only the type prefix. It is now named the mechanical floor and described as necessary but not sufficient, with greps for the two remaining checkable rules and imperative mood stated as a by-eye check. The regex string itself is deliberately unchanged: it is quoted in three places, and editing one copy would create exactly the divergence this change exists to prevent.
Not blocking
Codex's pass-4 finding, which it rated P2, is that a closing fence should be required to contain only trailing whitespace — so a line of seven backticks followed by text, inside a four-backtick fence, is treated as a close. Recorded as immaterial and deliberately not actioned: it needs constructed Markdown that no ordinary PR body contains, and it is the residual imprecision the artifact now explicitly admits and gates. Acting on it would mean extending the awk to chase one more construct, which is the behaviour this change just told readers not to do.
Test plan
grep -rn "sort -rn" plugins/ --include=*.mdreturns only the two sentences warning against substituting it;grep -rn "fence = !fence"andgrep -rn "END {exit 0}"return nothinghttps://claude.ai/code/session_01FYYKLM8tpaF1qqhtCANkSX