Pr And Issue Discipline

PR and Issue Discipline

Show the change, do not describe it: a fenced block is the text form of a picture, so the reader skims it and gets the change; the fewest words after, in the shape that fits the change; keep draft state honest, converge reviews, and leave the merge to a human by default.

These rules apply to any session that opens or updates a PR, triages a review round, writes an issue, replies to an issue reporter or outside contributor, or decides who merges. "The author" below is whoever prepared the change, human or agent, working alone or in a multi-agent session.

When to Apply

• Opening or updating a pull request (body, title, draft state)
• Writing a bug report or issue
• Replying to an issue reporter or outside contributor
• A review round just landed on an open PR
• Deciding whether a PR merges, and by whose hand

PR Bodies: Show the Change, Shaped to It

Show the change rather than describe it. A PR body is text, so its picture is a fenced block: real captured output wherever behavior is observable, a diagram, table, or the contract's own shape where nothing runs. The reader skims the blocks and gets the change without reading a paragraph; prose only carries what no block can. Shape the body to the change; never force every PR through one template.

Template check, once per session, at plan time. Before the first PR or issue of the session, while still planning, resolve the choice once and reuse it for every PR and issue in that session:

1. Look for templates in the target repository, case-insensitively and at any extension (.md, .txt, none): pull_request_template* and PULL_REQUEST_TEMPLATE* at the repository root, in .github/, and in docs/, plus .github/PULL_REQUEST_TEMPLATE/ and .github/ISSUE_TEMPLATE/.
2. None found: the shapes below apply directly. Do not ask.
3. Any found: ask the user once, in these words, "Use this repository's templates, or this skill's shapes?" Name the repository, say whether it is theirs or someone else's, and list the templates found by filename. A user who installed this skill often prefers its shapes, and a third party's maintainers usually expect their own, so ask rather than assume either way.
4. Carry the answer for the rest of the session, for every PR and every issue in it. Answer "the skill's shapes": use the shapes below throughout. Answer "the repository's templates": use the template whose purpose matches each artifact, without asking again; where the repository ships no template for that artifact (issue templates but no PR template, or the reverse), the shapes below fill the gap.

Whichever is chosen, the repository's CONTRIBUTING guidance still applies: honor its rules on title conventions, required sections, and linked issues inside the body you write.

Additive feature

## What this adds

```text
$ bun run shards --changed
manifest build/image-sets.json v3: 5 contexts, files and dependencies validated against the checkout
changed: api -> dependency closure {base, api} -> shards: [base, base+api]
```

## How

The resolver validates the manifest against the checkout, closes changed contexts over their dependencies, and emits the GitHub Actions matrix.

## Proof

- Manifest validation and shard-resolution tests pass (2 new), `bun run check` green.

Existing behavior change or bug fix

Open with Before / After as real captured output; the comparison is the visualization. When nothing observable changes (a pure refactor), open with ## What this changes and the same ## How and ## Proof.

## Before

```text
$ bun run check
scripts/sweep.mts: probe timed out after 120s; agent marked dead (it was mid-build)
```

## After

```text
$ bun run check
scripts/sweep.mts: probe extended 120s -> 300s while the build lock is held; agent alive
```

## How

```text
before: probe start -> fixed 120s -> timeout -> agent marked dead (mid-build)
after:  probe start -> 120s up -> build lock held? -> extend to 300s -> live verdict
```

## Proof

- 34 tests green (2 new), `bun run check` green

Contract or documentation PR

Use ## What this specifies when the PR defines a contract rather than executable behavior. Nothing runs, so show the contract itself (its schema, table, or layout) in a block, not in prose.

## What this specifies

```text
SKILL.md                    disable-model-invocation: true
agents/openai.yaml          policy.allow_implicit_invocation: false   <- must pair with the line above

agents/openai.yaml          .codex-plugin/plugin.json
interface.display_name      == interface.displayName
interface.short_description == interface.shortDescription   (25-64 chars)
interface.brand_color       == interface.brandColor
```

## How

The smoke test reads the three files per skill and fails the build on any drift.

## Proof

- Smoke-test cases for the mirrored block and the invocation pairing pass.

For every form:

• Blocks show, prose tells. Where behavior is observable, the opening block is an actual command and its actual output, complete enough to stand alone; never manufacture output or add it only to satisfy a format. Where nothing runs, the block is a diagram, a table, or the contract shape itself.
• ## How has no mandated carrier. Use terse bullets, a small diagram, a table, or two short paragraphs, whichever explains the mechanism fastest. One carrier per point: a diagram followed by a paragraph re-explaining it means the diagram failed.
• ## Proof names focused behavioral tests or stable checks, with numbers where they exist (tests, gates). Do not turn it into transient CI, approval, or review status.
• Write programmer to programmer: what changed, how the flow changed, in the reader's technical vocabulary. Usually 200 to 400 words is enough. The diff carries the detail; do not narrate the implementation process, reduction history, line counts, status, future work, scope caveats, reviewer guidance, or the entire diff.

Redact captured output before publishing. Strip secrets, tokens, and credentials; genericize machine-specific absolute paths and usernames (a captured row published with /repo/... in place of the machine's real checkout path is the worked example). Redaction is not paraphrase: the command and the output structure stay verbatim.

Issues: Same Principle

What breaks, shown first; then the minimum around it. Short and skimmable, no walls of text.

The session's template answer, resolved at plan time above, covers issues too: fill the chosen issue template's fields and apply this principle inside them, or use the shape below when the answer was the skill's shapes or no template exists.

## What breaks

```text
$ npx skills add Vivswan/skills --skill some-skill
installed: SKILL.md, README.md          (metadata.json silently missing)
```

## Repro

1. Add a `metadata.json` inside any skill folder.
2. Install with `npx skills add`.

## Expected vs actual

- Expected: every file in the skill folder installed
- Actual: `metadata.json` dropped without a warning

Include environment only when it matters: a version-specific parser bug names the version; a pure logic bug does not. When an issue includes captured output, the redaction rule above applies unchanged.

Replies to Issue Reporters and Outside Contributors: Plain First

An issue reply or a review comment to an outside contributor is read by someone who may find long text hard to read and does not know the code. Write a plain-language part that stands alone. Add a technical part, collapsed so it costs nothing to skip, only when it carries information the plain part cannot: when everything fits in plain words, the plain part is the whole reply.

Specimen: a diagnostics-only bug report whose log ended mid-request. The first reply opened with three paragraphs on hidden provider groups, tombstones, and silent refreshes. The rewrite:

## TL;DR

1. **What went wrong?** One or two sentences, or a screenshot.
2. **Is `https://<host>/@<user>` a LiteLLM proxy?** If yes, does it need an API key?
3. **The log lines after the last request.** In VS Code: `View > Output`, pick `LiteLLM`, copy everything after the line starting with `Fetching from::`.

Number 3 would help the most. The rest of this comment explains why, if you are curious.

## What the report shows

**The report was sent before anything failed.** The last log line is the extension asking your server for its model list. No answer had arrived yet:

```
Fetching from:: "https://<host>/@<user>/v1/model/info"    <- last line, still waiting
```

- You removed a server that pointed at `https://<host>/`. The extension remembers that and keeps its models out of the picker. That is expected, not an error.
- You added a new server at `https://<host>/@<user>` with no API key.

**A guess, to save a round trip:** a URL with `/@username` in it and no API key usually is not a LiteLLM proxy.

<details>
<summary>Technical details</summary>

- `Provider group is hidden by an explicit user removal`: the `servers` entry was removed. VS Code cannot delete a provider group, so the extension tombstones it and answers with an empty model list. Docs: [Lifecycle: renames, removals, hidden groups](...).
- `Fetching from:: .../v1/model/info`: the first discovery request. The log buffer holds 50 lines and no error was recorded, so the report was built inside this request's 30 second timeout.

</details>

The rules the specimen follows:

• Requests first, as a bare TL;DR. A numbered list, three items at most. Say which one would help most.
  • Give the exact click path or command when one exists.
  • "What would help" rather than "what we need": the reporter is doing you a favor. The reader may stop after the list.
• Part one is plain language, and never says so.
  • Headings name the content ("What the report shows", "What changed"), never the reader's level. "In plain words", "Simple version", and "Non-technical summary" read as talking down.
  • Say what the reader did and what they see: "the server you removed" rather than "the tombstoned provider group". When a mechanism has no plain name, show its effect instead of naming it.
  • Quote the reader's own log line with an arrow note rather than paraphrasing it.
• Part two is the technical reading, collapsed, and only when it adds something.
  • Inside a <details> block: the mechanism names, the log lines mapped to code paths, docs links, and what a future maintainer would want when re-reading the thread. Nothing in part one depends on it.
  • A reply that says everything in plain words has no part two. An "expected behavior, here is the setting" answer needs no details block; the specimen's does, because the buffer size and timeout explain why the log stops where it does.
• A guess goes last in part one and is labeled a guess. It saves a round trip without steering the reader before they answer.
• Short bullets, bold lead-ins, no paragraph over three sentences. The redaction rule above applies unchanged.

Draft Discipline

• Open every PR as a DRAFT, and keep it draft through its review loop.
• Flip READY the moment it converges: never batched, never held back.
• Flip BACK TO DRAFT the moment new commit-requiring work appears on a ready PR (a fresh valid review comment, a gate finding), before the fix round starts.
• Draft state tracks pending commits; CONVERGENCE gates the merge offer. A fresh comment needing only a reply does not bounce a ready PR back to draft (its reply-and-resolve lands the same cycle, no commit), but a PR is offered for merge only while the full converged definition below holds.

Converged means the review has converged as the /rubber-duck-review skill defines it (step 7 owns the single definition), plus the PR-specific bar: CI fully green and every review thread resolved (fixed or answered). Fully green counts EVERY check on the PR, required or not, and on every PR in its dependency chain: a residue red from an un-retargeted base disqualifies ready even when the required gate passes.

Babysit to Comment Convergence

An open PR is live work until it merges: bot reviewers (e.g. Copilot code review) and humans leave comments on every push. Per PR, loop until quiescent:

1. Every push gets a CI watcher (Companion Gates, below).
2. When a review lands, triage EVERY comment the same cycle it appears, never batched:
   • A valid finding is fixed in that same round.
   • An invalid or not-valid-here comment gets a reply stating why, and its thread resolved.
3. A fix push restarts the loop: new CI watch, re-gate on the changed content, and the bot may re-review.

Toil budget. When rounds keep yielding one finding at a time (around ten rounds in), stop fixing instances one at a time: enumerate the recurring finding classes, sweep each whole class across the change in one pass, then resume the loop. One 35-round convergence collapsed to a few batch sweeps once the finding classes were enumerated.

Read thread state via GraphQL, never from comment timestamps (a thread with no new comments can still be unresolved):

reviewThreads(first: 100) { nodes { isResolved } pageInfo { hasNextPage endCursor } }

Paginate with after: <endCursor> while hasNextPage is true; a fixed first page is not the full set.

Bot reviews that do not fire automatically on drafts are requested explicitly (e.g. add Copilot as a reviewer on the draft; prefer balanced or high reasoning where the repo exposes the setting). Requesting a Copilot review via the REST reviewers endpoint takes the reviewer login Copilot, exactly: copilot-pull-request-reviewer[bot] silently no-ops (a 201 response with empty requested_reviewers), and GraphQL reviewRequests hides a pending Copilot request either way, so the issue timeline is the only confirmation the request registered. Between rounds, never poll: where the /watch-ci-after-push skill is installed, sleep on its wait-for-pr-event script, a background waiter whose exit wakes the session and names what changed.

Production shape of one round:

• "empty manifest passes vacuously": valid. Fixed with a regression test in the same cycle.
• "script not wired into the docs": sequencing by design. Replied with the plan (a docs pass wires all scripts post-merge) and resolved.
• "symlink following": split. The leaf-fidelity half fixed after confirming it empirically; the escape half declined with the recorded design rationale.

Who Merges

The human, by default. A PR exists to put a human gate before the mainline: the author prepares it (push, gates green, a "ready to merge" report) and the human merges. Where a merge queue owns the ordering, the author's prepared action is enqueueing the converged PR; enqueue is not merged, so watch until the commit actually lands. Two standing exceptions, each only when the user has granted it:

• A trivial mechanical fix. A change of a few lines that alters no behavior, flow, or procedure (a type narrowing, a typo, a rename with no semantic edge) merges directly once its gates are green; the human gate is reserved for changes worth human attention. When in doubt about "trivial", it is not trivial.
• A pipeline blocked on a merge. When a converged PR gates queued work and the human is not acting, merge it and say so in the next report. Waiting idle on a merge the author could perform is the defect; the notification preserves the human's oversight.

The landing action is exit-conditioned, never chained, for a PR merge and a direct push alike. Read the gate's own verdict and STOP; land in a separate command only after the gate itself reports green. Green means the gate's exit code AND its verdict, and a review gate is green only when its findings are triaged, not merely when its process exits 0.

tail gate.log; git merge && git push    # WRONG: the merge runs whatever the log said
tail gate.log && git merge && git push  # WRONG: && conditions on tail printing the log,
                                        # not on the gate's verdict; a red log still merges

Companion Gates

• After every push, a background CI watcher; where installed, the /watch-ci-after-push skill defines it. A MERGE is watched the same way, on the mainline tip's SHA (fetch the mainline from the remote the PR merged into and watch FETCH_HEAD): after gh pr merge, git rev-parse HEAD still names the topic tip, and the squash or merge commit exists only on the mainline.
• Before anything lands, an independent review that can block the landing, scoped to the exact content being landed, never the working tree: the branch or PR diff (base...HEAD) once committed, the staged diff before that. Where installed, the /rubber-duck-review skill defines that review and its convergence.