diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 42b9dd5..98b1774 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -120,7 +120,7 @@ Every skill, agent and hook is held to these; a change that breaks one says why. header row, so rename or renumber any of them only with those citations. - The Python environment is uv's (`pyproject.toml`, `.python-version`; the devcontainer and CI run `uv sync`). The dev tools' pins live in `pyproject.toml` only; `uv.lock` is not committed. -- Before a pull request: `uv run python -m unittest discover -s tests`, the four +- Before a pull request: `uv run python -m unittest discover -s tests`, the five `tests/*.sh`, and `uv run python scripts/check_static.py`, so the tests use the project's environment outside the devcontainer too; CI also runs codespell, `claude plugin validate .`, and the unit tests and `check_static.py` on Python 3.9, the README's floor. diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index bf69aa3..bd3e6c0 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -61,6 +61,9 @@ jobs: - name: Atlassian write gate run: bash tests/test_atlassian_gate.sh + - name: GitHub issue write gate + run: bash tests/test_issue_gate.sh + - name: What the skills and agents name exists run: uv run python scripts/check_static.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3889389..dddc1f5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,7 +8,7 @@ measures it. `.claude/CLAUDE.md` holds the conventions every skill and agent fol `/lasp-dev:skill-feedback` in the session where it went wrong, or open one with the "Skill feedback" template. - **Before a pull request**, run the checks CI runs: - `uv run python -m unittest discover -s tests`, the four `tests/*.sh`, and + `uv run python -m unittest discover -s tests`, the five `tests/*.sh`, and `uv run python scripts/check_static.py`. - **Security issues** go through `SECURITY.md`, not a public issue. diff --git a/README.md b/README.md index 8f5d50b..29122e8 100644 --- a/README.md +++ b/README.md @@ -274,7 +274,7 @@ can be started in its place when a skill names it, so give the personal one anot ### Permissions -Each skill asks before it posts, pushes, opens a pull request or writes to Jira or Confluence. The plugin's +Each skill asks before it posts, pushes, opens a pull request, files or changes a GitHub issue, or writes to Jira or Confluence. The plugin's `hooks/atlassian-gate.sh`, whatever the server is named and whether or not it runs read-only, allows a Jira write only when your session makes it and `.review/jira-approval.json`, which the skill writes at your yes to a write plan, lists that tool on that ticket, so a plan's writes do @@ -283,7 +283,13 @@ record from this session in the last 30 minutes lists, a write a subagent makes, removed link, a move to another project, an edited comment, an update that uploads local files, and any write to Confluence. It asks rather than staying silent because a hook's ask still prompts in auto mode, where a silent hook leaves the call to the classifier. An `ask` rule you add for a Jira write tool prompts again at each call of a plan, -whatever the hook decides. Claude Code's own permission rules are a second +whatever the hook decides. Where the tracker is GitHub Issues, `hooks/issue-gate.sh` allows a +plan's `issue_write` and `add_issue_comment` calls the same way, on any server name, from +`.review/github-approval.json`, a create by its title, so an item allows any create with that exact title on that +repository in the window, and an update or a comment by its issue, so an item allows any such call on that issue in the window, as the Jira gate's do, but gives every other call no decision: +one no record lists, `skill-feedback`'s own create among them, or one carrying an argument no plan +sends, prompts or passes as your settings say. Your own `ask` rule on `issue_write` +or `add_issue_comment`, as in the block below, still prompts at each call of a plan. Claude Code's own permission rules are a second layer that does not depend on the skill text, and that still asks in auto mode. A recommended block for your user settings, `~/.claude/settings.json`, with `github-libera` replaced by your GitHub server's name: @@ -487,7 +493,7 @@ Most days need one command, `work`. Started, in the skills table: `typed only` r | Entry point | `work` | typed or asked | Take a ticket, a Jira ticket or, where the standard's tracker is GitHub's, an issue, through the workflow: reads where it stands and runs the next phase skill; each phase keeps its own approval gates | | | `quickstart` | typed or asked | Say where this machine and repository stand, what to fix first and what to type next, with `onboard` first in a repository with no standard, and the skills by group; with a question, answer it from the plugin's own files. Reads only | | | `onboard` | typed or asked | Survey a repository for the facts the plugin works from, its committed templates, contributing guide, test commands (instruction files, docs, CI) and pre-commit, and its history, and the tracker's statuses and fields when the servers are connected; say where each fact would be read from, the repository's own file first; first asking where the repository's rules live, here or in another repository's standard; then ask where facts should live, which value holds where the repository's files disagree, and which proposals to keep. Writes only `.review/onboard/` unless the person says yes to a named write: the repository's standard, or its `standard:` pointer and the standard's `profiles/.yaml` | -| Phases, also run alone | `ticket-draft` | typed or asked | Turn a prompt, an existing ticket or a pull request with none into one in the team's format: duplicates merged, dependencies, follow-up tickets for out-of-scope work, an in-session estimate, and an Open Questions section for what cannot be settled yet. `investigate` answers your questions about the code first; `settle` folds later answers in and records a departure the work keeps as a comment; `reformat` moves a drafted ticket to the current format, asking only what the format needs. Run directly for an epic or a group ticket session | +| Phases, also run alone | `ticket-draft` | typed or asked | Turn a prompt, an existing ticket or a pull request with none into one in the team's format, a Jira ticket or, on a GitHub tracker, an issue: duplicates merged, dependencies, follow-up tickets for out-of-scope work, an in-session estimate, and an Open Questions section for what cannot be settled yet. `investigate` answers your questions about the code first; `settle` folds later answers in and records a departure the work keeps as a comment; `reformat` moves a drafted ticket to the current format, asking only what the format needs. Run directly for an epic or a group ticket session | | | `implement-change` | typed or asked | Take a change request to an agreed plan, an implementation and a diff verified by a wave of reviewers | | | `pr-create` | typed or asked | Write a reviewer-focused description and open the pull request | | | `pr-review` | typed or asked | Review a pull request and present the findings at their lines, each citing a rule, decision, term, ticket criterion or test check when one applies, and no key otherwise; draft comments when asked, and post one review with the verdict you choose only on your yes. A posted review's threads are what the revision reads; a review kept local leaves it nothing | @@ -532,7 +538,7 @@ since been rewritten; the rest were written here. | Other | Role | | --- | --- | -| `hooks/` | Measure a build (lint passes and test runs, never a `--collect-only`, `--version` or `--help` call, reviewer rounds and wave reviewers, wall clock less the waits before each resume, tokens) into `.review/build/`; hold the branch for one session at a time through `.review/lock`; at session start, print where the ticket on this branch stands and load the review rules that apply to the repository, fetching a standard elsewhere the first time a clone needs it; gate the Atlassian calls that are not reads, stop a commit, a `gh` write or a GitHub or Atlassian write that carries what looks like a credential, and ask before a read past the first line of a `.review/` file marked `Controlled:`, as "Permissions" above says; save each `pull_request_read` result to `.review/pages/` as the server returned it, for `pr-reader` to copy into its dump | +| `hooks/` | Measure a build (lint passes and test runs, never a `--collect-only`, `--version` or `--help` call, reviewer rounds and wave reviewers, wall clock less the waits before each resume, tokens) into `.review/build/`; hold the branch for one session at a time through `.review/lock`; at session start, print where the ticket on this branch stands and load the review rules that apply to the repository, fetching a standard elsewhere the first time a clone needs it; gate the Atlassian calls that are not reads, allow the GitHub issue writes of a plan you approved, stop a commit, a `gh` write or a GitHub or Atlassian write that carries what looks like a credential, and ask before a read past the first line of a `.review/` file marked `Controlled:`, as "Permissions" above says; save each `pull_request_read` result to `.review/pages/` as the server returned it, for `pr-reader` to copy into its dump | | `standards/` | The plugin's shared standards files, listed in `standards/README.md`. A project's own terminology, decisions, context, archive and review records live in its standard | Text the skills compose for people on a pull request, or a page `design-doc`, `architecture-doc` or `process-doc` publishes, @@ -693,7 +699,7 @@ comment an agent drafted stays the agent's. ```text .claude-plugin/ # marketplace.json, plugin.json ("lasp-dev", rooted here) skills/ agents/ # the skills and agents -hooks/ # build measurement, the branch lock, the session-start line, the Atlassian write gate, the secret scan, the controlled-file gate, the saved pages +hooks/ # build measurement, the branch lock, the session-start line, the Atlassian write gate, the issue write gate, the secret scan, the controlled-file gate, the saved pages scripts/ # stdlib scripts the skills and hooks run: record.py, profile.py and the rest; # jira_markup.py is mcp-atlassian 0.23.1's converter, copied (MIT) standards/ # the plugin's shared standards files (standards/README.md) @@ -729,10 +735,13 @@ session-start line or `work` says something unexpected, these files are what it | `context/.md` | `ticket-draft`, with the `Branch:` line for a pull request's ticket; `implement-change` adds its `Branch:` line | `work`, to see whether the ticket was drafted, and, with a `Branch:` line and no plan, which branch's pull request to read | Not cleared by the tools | | `context/.pasted.md` | `work`, `ticket-draft` and `implement-change`: a ticket's text the person pasted when the Atlassian server could not be reached | `source-reader`, given it as a file | Not cleared by the tools | | `context.md` | `ticket-draft`, a copy of the latest page, never of a pull request's; `implement-change` adds `Branch:` | `work` and the session-start line, for the ticket the branch is on | Replaced by the next `ticket-draft` | -| `context/.jira.txt`, `context/.original.txt` | `ticket-draft`: the paste file, and the text before the session | The person, pasting into Jira what the write plan did not write; `ticket-draft`, for the `Original request` comment it writes | Not cleared by the tools | +| `context/.jira.txt`, `context/.issue.md`, `context/.original.txt` | `ticket-draft`: the paste file, for Jira or, on a GitHub tracker, for an issue, and the text before the session | The person, pasting into the tracker what the write plan did not write; `ticket-draft`, for the `Original request` comment it writes | Not cleared by the tools | | `context/.send/` | `ticket-draft`'s write plan: each description and comment as shown (`.txt`) and as sent (`.md`), and the description as read back (`stored.txt`) | `scripts/jira_markdown.py`; the person, to see what was sent | Rewritten at each plan | | `jira-plan.md` | The write plan, run by `ticket-draft` and `ticket-audit`: each item, numbered, with its text in full as it will be sent, written before the plan is shown | You, from the link at the plan's gate | Replaced at the next showing of a plan | | `jira-approval.json` | The write plan, run by `ticket-draft` and `ticket-audit`: this session's id, the time and each approved item's tool and target, written at your yes before the first call | `hooks/atlassian-gate.sh`, which allows only a call it lists | Removed by the skill when the plan finishes or stops; allows nothing after 30 minutes | +| `github-plan.md` | The GitHub write plan, run by `ticket-draft` on a GitHub tracker: each item, numbered, with its text in full as it will be sent, written before the plan is shown | You, from the link at the plan's gate | Replaced at the next showing of a plan | +| `github-approval-items.json` | The GitHub write plan, run by `ticket-draft` on a GitHub tracker: the approved items, written with the Write tool so a title's quotes never pass through a command | The plan's `jq` command, which builds `github-approval.json` from it | Removed with the record | +| `github-approval.json` | The GitHub write plan, run by `ticket-draft` on a GitHub tracker: this session's id, the time and each approved item's tool and target, written at your yes before the first call | `hooks/issue-gate.sh`, which allows only a call it lists and leaves every other to your own permissions | Removed by the skill when the plan finishes or stops; allows nothing after 30 minutes | | `controlled-cleared.json` | `hooks/controlled-gate.sh`, after a read of a `.review/` file marked `Controlled:` that you allowed, or the session's own write of one: this session's id and the items cleared | The same hook, which lets this session's later reads of those items through | Replaced when another session clears an item | | `context/.writes.md` | `ticket-draft`'s write plan: each item and whether it was written, with its key, failed, dropped or left to paste | The person | Rewritten at each plan | | `implementation-plan-.md` | `implement-change`: a draft from the planner, before the plan review, `Approved by:` at it, `Base:` at the branch, `## Corrections` at the plan and diff gates, `## Provisional decisions` for each decision made for now, and each `## Revision` with its own `Approved by:`; named for the branch with no ticket | `implement-change` on resume and at step 8 (`Depth:`, and `## Corrections` for a lowered depth), `work` and the session-start line (`Approved by:`), `pr-create` (`Base:`, `Approved by:`, `## Provisional decisions`), `commit` (the reason) | Not cleared by the tools | @@ -891,6 +900,7 @@ CI (`.github/workflows/checks.yml`) runs, on every pull request: - `bash tests/test_where.sh`: `hooks/where.sh`, the start-of-session line. - `bash tests/test_standard_file.sh`: `scripts/standard_file.sh`, `scripts/standard.sh`, which it fetches through, and `hooks/rules.sh`, the rules loaded at session start. - `bash tests/test_atlassian_gate.sh`: `hooks/atlassian-gate.sh`, against every tool of mcp-atlassian 0.23.1. +- `bash tests/test_issue_gate.sh`: `hooks/issue-gate.sh`, against github-mcp-server's `issue_write` and `add_issue_comment`. - `uv run python scripts/check_static.py`: what the skills and agents name exists, each has its Copilot link and each skill its row in "What is included", and no shipped file gains a mention of Libera (`scripts/coupling.json`; after removing mentions, diff --git a/ROADMAP.md b/ROADMAP.md index 536939d..68eb7be 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -105,8 +105,8 @@ merged pull request is its record. Owners are roles. Status is `planned`, `defer | --- | --- | --- | --- | --- | | Neutral examples in generic skills (#21) | `quickstart` still checks for the plugin this one replaced, and `using-curryer` still ships in the plugin, loaded only in curryer and the repositories that depend on it (#67). `scripts/coupling.json` allows the rest and does not scan the docs (#15). `using-curryer` leaves for curryer's own standard, or a domain plugin installed beside this one | plugin maintainer | before the public flip | planned | | Forge, tracker and wiki as roles (#24) | Every skill speaks GitHub, Jira or Confluence directly, so a repository on GitLab or tracking work in GitHub Issues gets little of the workflow. One documented shape per role, the neutral thread JSON as the forge's, with the current skills as the first adapters; GitLab next, then GitHub Issues as a tracker | plugin maintainer | a team on GitLab or GitHub Issues adopts the plugin | idea | -| The skills on the ticket shape (#58) | The shape, the format and the profile's `issue_tracker:` are tracker-neutral (`docs/contracts.md`, "The ticket shape"; `standards/ticket-format.md`), but `ticket-draft`, `work` and `ticket-audit` still read and write only Jira, and `onboard` still writes `jira:`. No ticket is kept as a file in the repository: a ticket is drafted once, in the format, for whichever tracker files it | plugin maintainer | with #59 | planned | -| GitHub Issues as a tracker (#59) | Most repositories outside LASP track work in GitHub Issues, through a server already connected for pull requests; it is the first tracker adapter after Jira. Reading an issue and `work` routing by its state and labels have landed. Left: drafting and filing one under an approved write plan with an allow-only gate, and `ticket-draft` taking an issue `work` routes to it; onboard detecting GitHub Issues as the tracker, the project's branch convention and mapping its labels to the roles; then the audit sweeps; no Projects fields or estimates | plugin maintainer | the ticket shape's profile and format land | planned | +| The skills on the ticket shape (#58) | The shape, the format and the profile's `issue_tracker:` are tracker-neutral (`docs/contracts.md`, "The ticket shape"; `standards/ticket-format.md`), but `ticket-audit` still reads and writes only Jira, and `onboard` still writes `jira:`. No ticket is kept as a file in the repository: a ticket is drafted once, in the format, for whichever tracker files it | plugin maintainer | with #59 | planned | +| GitHub Issues as a tracker (#59) | Most repositories outside LASP track work in GitHub Issues, through a server already connected for pull requests; it is the first tracker adapter after Jira. Reading an issue, `work` routing by its state and labels, and `ticket-draft` drafting and filing one under an approved write plan with an allow-only gate have landed. Left: onboard detecting GitHub Issues as the tracker, the project's branch convention and mapping its labels to the roles; then the audit sweeps; no Projects fields or estimates | plugin maintainer | the ticket shape's profile and format land | planned | | A GitLab forge adapter (#61) | A repository on GitLab gets local-only reviews; the adapter writes the thread shape (#60) through a GitLab server or `glab` | plugin maintainer | step 5 of #24's order, after #60 | planned | | Bitbucket, other trackers, Markdown files as a wiki (#62) | No team has asked yet | plugin maintainer | a team asks | idea | diff --git a/agents/source-reader.md b/agents/source-reader.md index 40f4a2c..7c48f71 100644 --- a/agents/source-reader.md +++ b/agents/source-reader.md @@ -18,8 +18,9 @@ Do one task, read what it needs, and return its block. **You read and report; th decides.** `using-jira` and `using-confluence` are loaded: follow them for the fields, the format, the spaces and what to skip. You write only in the `sweep` task, only under `.review/audit/`, and with the Write tool, never through Bash; every other task returns text and -writes nothing. For `search`, the caller names the Jira project from the repository's profile (`Jira: `); a `search` without that -line returns `search: no Jira project named` and nothing else. +writes nothing. For `search`, the caller names the Jira project from the repository's profile (`Jira: `), or +on a GitHub tracker the issues' repository (`Tracker: github `); a `search` without +either line returns `search: no Jira project named` and nothing else. ## Servers @@ -82,7 +83,7 @@ or none · assignees, or none · author · created>`, `Description:` holds the i `Comments:` its comments as below, `Sections:` its Markdown headings, `Links:` `parent: # ` from `get_parent`, or `none`, `Markup:` reads `intact`, since an issue's body is stored as Markdown, and `Original request -comment:` reads `no`. A pull request's number is not an issue: when `get`'s `html_url` holds +comment:` is read from its comments as for Jira. A pull request's number is not an issue: when `get`'s `html_url` holds `/pull/`, return `## #<n> — ticket`, then `Not an issue: #<n> is a pull request`, and `Questions:`. ``` @@ -122,6 +123,12 @@ References: <for each size the project's estimation gives for its reference task GitHub: <each matching issue or pull request: number, title, state; "none"; or "not read: <error verbatim>"> ``` +Given `Tracker: github <owner/repo>`, the tickets are that repository's issues, every open one and +those closed in the last 180 days, read with `search_issues` and `list_issues`, at most 200; the +table's Key is `#<n>`, its Status the state and the labels, and its Points and Components `none`; +`Components:` and `References:` read `none: GitHub tracker`, and `GitHub:` lists the pull +requests alone. Atlassian is not needed. + The caller re-judges every `same` and `superset` from the criteria you quote, so quote the ticket's words, not a paraphrase, and say when a ticket has no criteria to quote. diff --git a/docs/contracts.md b/docs/contracts.md index 64ecde9..d8d5123 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -212,6 +212,35 @@ and `ticket-audit`, at the person's yes; removed when the plan finishes or stops Pinned by: tests/test_atlassian_gate.sh::with this session's record, every write it lists is allowed +## The GitHub approval record + +Version: 1 + +Fields: `.review/github-approval.json`, at the repository's top level, which the hook finds from +the payload's `cwd`; one JSON object in the Jira record's shape: `session`, `t` and `items`, each +item `{"tool": <tool>, "target": <target>}`, the tool named without its server prefix. The target +is `<owner>/<repo> <title>` for an `issue_write` whose `method` is `create`, so an item matches one +create, and `<owner>/<repo>#<n>` for one whose `method` is `update` and for `add_issue_comment`, as +github-mcp-server names the parameters `owner`, `repo`, `title` and `issue_number`, compared +byte for byte, case and any trailing newline included. No item is used up: a create's item matches +every create with that exact title on that repository in the window, and an update's or a comment's +every such call on that issue, as a Jira item does. `hooks/issue-gate.sh` allows a call from the main session only when the +record's session is the call's, `t` is under 30 minutes old and not in the future, an item matches +the tool and target exactly, and the call carries only the arguments a plan sends: a create +`method`, `owner`, `repo`, `title`, `body` and `labels`; an update those and `issue_number`, +`state`, `state_reason` and `duplicate_of`; a comment `owner`, `repo`, `issue_number` and `body`. +It gives every other call no decision, so the person's own permissions apply: a record that does +not parse in this shape, a subagent's call, a call with any other argument, such as `assignees`, +`milestone`, `type`, `parent_issue_number`, `issue_fields` or a `reaction`, and any call when jq is +missing. + +Writers: the GitHub write plan (`skills/using-github/issues.md`, "Running it"), run by +`ticket-draft`, at the person's yes, with `jq` from the items it writes to +`.review/github-approval-items.json`; both removed when the plan finishes or stops. Readers: +`hooks/issue-gate.sh`. + +Pinned by: tests/test_issue_gate.sh::with this session's record, a listed create, update and comment are allowed + ## The `--state` printout Version: 5 @@ -530,7 +559,7 @@ Pinned by: tests/test_where.sh::"--fields prints the facts, one per line, on a c ## The next phase -Version: 6 +Version: 7 Fields: `scripts/next_phase.py`'s printout: `phase:` (a skill, or `nothing`), `argument:`, `why:` (the row's name in the script's table, `review-unposted` among them from version 3), `lock:` @@ -555,6 +584,8 @@ number is the issue's. This repository, for that reading, is the one the working `origin` remote names. With no origin, `profile.py` reads no standard for the checkout, so no GitHub tracker is in effect and `#<n>` names nothing; were a tracker read without one, an issue that counts only as this repository's exits 1, saying there is no origin. +Version 7: on a GitHub tracker the `unfiled` line names `.review/context/<slug>.issue.md`, the +issue's paste file. Writers: `scripts/next_phase.py`. Readers: `work`. @@ -707,7 +738,7 @@ Pinned by: tests/test_profile.py::test_a_value_of_the_wrong_shape ## The ticket shape -Version: 1 +Version: 2 Fields: a ticket, in whatever tracker files it, is an id (a Jira key, `<KEY>-NNNN`, or a GitHub issue's number), a title (Jira's summary), a description in `standards/ticket-format.md`'s format, @@ -726,15 +757,15 @@ the profile's `issue_tracker.statuses` ("The profile"): A Jira workflow maps each role to exactly one status. A GitHub tracker maps each to at most one label, and `--defaults --field issue_tracker.roles.<role>` prints `none` for a role with no label: -an issue's own open and closed states stand for `filed`, `finished` and `abandoned`. With #59, a skill -that would move a ticket into `approved` or `implementation` with no label for it skips the move; -until then every skill that moves a ticket reads `jira.roles`, which prints `not yet recorded` for a -GitHub tracker, and stops. +an issue's own open and closed states stand for `filed`, `finished` and `abandoned`. On a GitHub +tracker `ticket-draft` reads `issue_tracker.roles.<role>` and skips a move into a role whose label +is `none` (version 2); `ticket-audit` still reads `jira.roles`, which prints `not yet recorded` for +a GitHub tracker, and stops. An open issue that carries no mapped label is in `filed`, and `work` sends it to `ticket-draft`. Writers: a project's standard, through `issue_tracker:` ("The profile"); `standards/ticket-format.md` and the tracker's skill, `using-jira`, for the description. Readers: `scripts/profile.py`, and -through it `scripts/next_phase.py`, for a Jira workflow or an issue's labels, and the skills that read `jira.roles`. +through it `scripts/next_phase.py`, for a Jira workflow or an issue's labels, the skills that read `jira.roles`, and `ticket-draft` on a GitHub tracker. Pinned by: tests/test_profile.py::test_a_github_tracker_maps_at_most_one_label_to_each_role diff --git a/docs/design.md b/docs/design.md index 190b2fd..905b793 100644 --- a/docs/design.md +++ b/docs/design.md @@ -123,6 +123,8 @@ Libera's standard. **One yes for a plan of writes, not one per call.** A split ticket comes to a dozen writes or more: the pieces, their fields and epic, the links between them, the original text and the history. Asking at each call made pasting the easier path for someone whose server can write, so a skill shows every write as one numbered list, a write plan, the person drops what they do not want and says yes once, and the gate hook allows the writes such a plan holds when the main session makes them, since a hook that only stayed silent would leave each call to Claude Code's own prompt. The hook allows a call only when the approval record the skill writes at that yes, `.review/jira-approval.json`, lists that tool on that ticket for this session: an allow also skips Claude Code's own prompt, so one granted to every call of those seven tools would let an unprompted call, or one steered by text in a ticket just read, write with nobody asked. The record names targets because the yes was to those tickets; it carries the session id so a record left in a repository authorizes no other session; and it lapses after 30 minutes, ample for a plan of a dozen calls, so one the skill failed to remove does not linger, while a plan that outruns it only prompts at each call left. A call no record lists is asked about rather than passed silently, because in auto mode a silent hook leaves the call to the classifier, which may approve it unprompted, while a hook's ask still prompts. The record is written by the session, so it narrows what a stray call can do to the tools and tickets a person approved in the last half hour; it does not prove a person typed the yes, and it binds the tool and the ticket, not the call's other arguments, so the text and fields sent are the skill's to keep to the plan. What still asks at the call is what a plan never holds: a delete, a removed link, a move to another project, an edited comment, an upload of local files, any write a subagent makes and anything for Confluence. Confluence has no record: a publish by `design-doc`, `architecture-doc` or `process-doc` is one call and a revise or update a few, so a prompt at each costs little, and a page write that can drop a diagram is worth the second look. Each prompt names the page and the section it writes, so a revise's several prompts can each be matched to the section the gate showed; the gate shows every text it covers in the message that asks, never by pointing back to an earlier turn (`decision-points.md`, "What does not change"). An `ask` rule in a person's own settings still prompts at each call; that is theirs to keep. (`using-jira`, "Writing to Jira"; `skills/using-jira/write-plan.md`.) +**The GitHub issue gate only allows.** A GitHub tracker's plan is gated the same way, by `hooks/issue-gate.sh` and `.review/github-approval.json`, but a call no record lists gets no decision rather than an ask. The same two tools write every issue and comment the plugin makes outside a plan, `skill-feedback`'s filing among them, and people already have their own rules for them; an ask there would add a prompt to calls that had none and change nothing a person chose. The hook therefore narrows nothing: it only removes the per-call prompt for the calls a person approved as a plan, and everything else stays as the person's settings have it. (`skills/using-github/issues.md`.) + **The secret scan stops only what it finds.** `hooks/secret-gate.sh` once asked whenever it could not read a call: a command with an unclosed quote, a repository named by a variable, a message file it could not name, a scan past its time, a server whose kind it could not tell, or a machine with no `python3`. Skills run such commands in their ordinary steps, so the prompt came often, mostly over commands that carried nothing, and a skill-feedback issue (#57) called it a nuisance. A prompt people approve unread protects nothing, and the person stays responsible for what they commit and post, as with any other tool. So what the scan cannot read passes, and the rest of the call is still read: a credential found in the command's text, another commit or another file it sends is denied, naming where and what kind, even when part of the call could not be read. A command it cannot split is judged on its text when it mentions a commit or a `gh` write, since the shell may still run it, and a tool of a server it cannot place as GitHub's or Atlassian's is not scanned. It is a backstop for the commit or post that carries a key by mistake, not a review of every command. (`scripts/secret_scan.py`; the README, "Permissions".) **The first merge changes nothing for a teammate who does not use the plugin.** The long pull request template, a version check requiring a bump, the measurement narrative and American spelling in libera_utils' pre-commit were kept out of it. The measurement narrative was dropped with the record store. diff --git a/hooks/hooks.json b/hooks/hooks.json index c7f70d2..96a9018 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -48,6 +48,16 @@ } ] }, + { + "matcher": "mcp__.*__(issue_write|add_issue_comment)", + "hooks": [ + { + "type": "command", + "command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/issue-gate.sh\"", + "timeout": 5 + } + ] + }, { "matcher": "Bash|mcp__.*", "hooks": [ diff --git a/hooks/issue-gate.sh b/hooks/issue-gate.sh new file mode 100755 index 0000000..a70992e --- /dev/null +++ b/hooks/issue-gate.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# PreToolUse on a GitHub server's issue_write and add_issue_comment, whatever the server is named. +# It only ever allows: a call the main session makes is allowed when .review/github-approval.json at +# the repository's top level lists that tool with that target, so a write plan's calls do not +# prompt one by one. The skill writes the record at the person's yes to the plan and removes it +# when the plan finishes or stops (skills/using-github/issues.md). The record is +# {"session": "<session_id>", "t": <epoch seconds>, "items": [{"tool": "<tool>", "target": "<target>"}]} +# the target being "<owner>/<repo> <title>" for an issue_write create, so an entry matches one +# create, and "<owner>/<repo>#<n>" for an issue_write update and for add_issue_comment. A call may +# carry only the arguments a plan sends: a create method, owner, repo, title, body and labels; an +# update those and issue_number, state, state_reason and duplicate_of; a comment owner, repo, +# issue_number and body. Every other call gets no decision, and the person's own permissions apply +# as they would without the hook: a record from another session, older than APPROVAL_TTL seconds, +# dated in the future, not in that shape, or not listing the call; a call from a subagent; a call +# carrying any other argument, such as assignees, a parent, issue fields or a reaction; input it +# cannot read; and any call when jq is missing. So a call outside a plan, such as skill-feedback's +# own create, prompts exactly as it did before the hook. +set -u + +APPROVAL_TTL=1800 + +command -v jq >/dev/null || exit 0 +input=$(cat) +tool=$(printf '%s' "$input" | jq -r '.tool_name // empty' 2>/dev/null) +op=${tool##*__} +case "$op" in + issue_write|add_issue_comment) ;; + *) exit 0 ;; +esac +[ -n "$(printf '%s' "$input" | jq -r '.agent_id // empty' 2>/dev/null)" ] && exit 0 +# The record sits at the repository's top level, found from the payload's cwd, so a session +# working in a subdirectory reads the same one. +cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null) +cwd=${cwd:-$PWD} +top=$(git -C "$cwd" rev-parse --show-toplevel 2>/dev/null) || top=$cwd +record="$top/.review/github-approval.json" +# The call's target is built and compared inside jq, so a title's bytes reach the comparison as +# they are, trailing newlines included. +[ -f "$record" ] && printf '%s' "$input" | jq -e --arg op "$op" --slurpfile r "$record" \ + --argjson now "$(date +%s)" --argjson ttl "$APPROVAL_TTL" '.tool_input as $i | (.session_id | strings) as $s + | def repo: if ($i.owner | type) == "string" and ($i.repo | type) == "string" and ($i.owner | length) > 0 and ($i.repo | length) > 0 + then "\($i.owner)/\($i.repo)" else empty end; + def number: if ($i.issue_number | type) == "number" and $i.issue_number >= 1 and ($i.issue_number | floor) == $i.issue_number + then $i.issue_number | tostring else empty end; + def only($allowed): ($i | keys) - $allowed == []; + def create: ["method", "owner", "repo", "title", "body", "labels"]; + def target: + if ($i | type) != "object" then empty + elif $op == "add_issue_comment" then (if only(["owner", "repo", "issue_number", "body"]) then "\(repo)#\(number)" else empty end) + elif $i.method == "create" then + (if only(create) and ($i.title | type) == "string" and ($i.title | length) > 0 then "\(repo) \($i.title)" else empty end) + elif $i.method == "update" then + (if only(create + ["issue_number", "state", "state_reason", "duplicate_of"]) then "\(repo)#\(number)" else empty end) + else empty end; + [target] as $t | ($s | length) > 0 and ($t | length) == 1 and ($r | length) == 1 + and ($r[0] | .session == $s and (.t | type) == "number" + and $now - .t >= 0 and $now - .t < $ttl and (.items | type) == "array" + and all(.items[]; type == "object" and (.tool | type) == "string" and (.target | type) == "string") + and any(.items[]; .tool == $op and .target == $t[0]))' >/dev/null 2>&1 \ + && printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow","permissionDecisionReason":"%s is an item of the write plan you approved."}}\n' "$op" +exit 0 diff --git a/sandbox/README.md b/sandbox/README.md index 87cc68b..b51e14d 100644 --- a/sandbox/README.md +++ b/sandbox/README.md @@ -48,7 +48,7 @@ The trigger pack, `triggers/scenarios.yaml`, runs in the same `widgets` sandbox skill a person types in plain words, never its slash command, one `trigger-<skill>` scenario per skill, and a second, `trigger-<skill>-<suffix>`, where one request leaves a gap, each expecting that skill's Skill call: whether "file a ticket" reaches `ticket-draft`. A `notrigger-` scenario holds the other side, a request that must not reach a skill, named in its -`must_not_run` as `Skill(...)`: a GitHub issue is not `ticket-draft`'s. +`must_not_run` as `Skill(...)`: a complaint about a skill, asked as an issue on the plugin, is not `ticket-draft`'s. `revise` and `ticket-audit` are not in it, since `disable-model-invocation` keeps them to their slash commands. Its `coverage.yaml` names the whole pack as its set, so `smoke.py` runs it on two checkouts, after a change to a skill's `description:`: @@ -60,7 +60,7 @@ sandbox/smoke.py --scenarios sandbox/triggers/scenarios.yaml --base ../dev-tools The tracker pack, `tracker/scenarios.yaml`, runs in `widgets` given its own standard with a Jira workflow, against a Jira project that `tracker/fixture/mcp-atlassian-stub.py` serves read-only from `jira.json`: `ticket-draft` drafting a new ticket and reformatting an older one through to the paste -file, its format triage on a ticket in the older layout and one in the current format, and `work` +file, its format triage on a ticket in the older layout and one in the current format, a request for a GitHub issue it answers by naming the Jira tracker and asking, and `work` routing a To Do ticket with no context page. Its `mcp.json` names the stub as `${SBX_FIXTURES}/mcp-atlassian-stub.py`, which `run.py` sets for the session, as the pack's header shows, and each scenario's `mcp:` expectation fails a run the stub did not answer. A search that @@ -70,12 +70,19 @@ to paste. The GitHub tracker pack, `tracker-github/scenarios.yaml`, runs in `widgets` given a standard whose tracker is GitHub's, its labels mapped to the workflow and no ticket pattern recorded, against -issues `tracker-github/fixture/github-mcp-server-stub.py` serves read-only from `github.json`: -`work` routing an open issue with no label, an open one with a mapped label, a closed one, a closed one still carrying a pull request status label, a -branch named `issue-<n>-words`, and a pull request's number given as the argument. Its `mcp.json` names the stub as -`${SBX_FIXTURES}/github-mcp-server-stub.py`, as the pack's header shows; the file name holds -`github-mcp-server`, so `run.py` gives it the GitHub rules. The stub's tools carry -github-mcp-server's names, arguments and output shapes, and none writes. +issues `tracker-github/fixture/github-mcp-server-stub.py` serves from `github.json`: +`work` routing an open issue with no label, an open one with a mapped label, a closed one, a +closed one still carrying a pull request status label, a branch named `issue-<n>-words`, and a +pull request's number given as the argument; `ticket-draft` drafting a new issue and refining #12, +each through a yes to its write plan, a draft whose plan is answered Paste it all, and one whose +third turn, taking every recommended answer, must file nothing, after a slash command or a plain +"file an issue for" alike. Its `mcp.json` names the stub as `${SBX_FIXTURES}/github-mcp-server-stub.py` +with `--writes .git/github-writes.jsonl`, as the pack's header shows; the file name holds +`github-mcp-server`, so `run.py` gives it the GitHub rules, except that a pack's stub run with +`--writes` has `issue_write` and `add_issue_comment` neither allowed nor denied. Under `dontAsk` +such a call then runs only when `hooks/issue-gate.sh` allows it, and the stub records it to that +file, which each scenario's setup removes and its verify reads; nothing reaches GitHub. The stub's +tools carry github-mcp-server's names, arguments and output shapes. A project's own pack, which replays its pull requests and tickets against its GitHub and Jira, read-only, is a file of the same form in a diff --git a/sandbox/run.py b/sandbox/run.py index 8c27a3d..21beab5 100755 --- a/sandbox/run.py +++ b/sandbox/run.py @@ -73,7 +73,11 @@ Atlassian's when its command or an argument names `mcp-atlassian`, the test `sandbox.sh` applies before it forces the server read-only. An Atlassian server without `--read-only` and `READ_ONLY_MODE=true`, which `sandbox.sh` sets, stops run.py, since its rules allow every tool. A -server of neither kind gets no rule, so under `dontAsk` every call to it is refused. `{sandbox}` and `{plugin}` are repeated once per spelling of the sandbox's path and the +server of neither kind gets no rule, so under `dontAsk` every call to it is refused. A GitHub +server that is a pack's own stub, `python3` running `${SBX_FIXTURES}/github-mcp-server-stub.py` +with `--writes` and no `url`, has +`issue_write` and `add_issue_comment` neither allowed nor denied, so only `hooks/issue-gate.sh`'s +allow lets one through, and the stub records it instead of reaching GitHub. `{sandbox}` and `{plugin}` are repeated once per spelling of the sandbox's path and the plugin directory's, so file and shell rules stay inside them. Its `sandbox` block is copied as it stands: Bash runs in Claude Code's sandbox, which allows it without a rule. `claudeMdExcludes` lists every `CLAUDE.md`, `AGENTS.md` and rules file above the sandbox and the person's own, and @@ -255,6 +259,20 @@ def server_kind(server: dict) -> str | None: return None +def recording_stubs(servers: dict) -> list[str]: + """The GitHub servers that are a pack's own stub: no `url`, a `python3` command whose script + argument is `${SBX_FIXTURES}/github-mcp-server-stub.py`, and `--writes`, so they record the issue + writes they are sent and reach no GitHub.""" + out = [] + for name, server in servers.items(): + args = [str(a) for a in server.get("args") or []] + script = next((a for a in args if not a.startswith("-")), "") + if (not server.get("url") and server.get("command") in ("python3", "python") and "--writes" in args + and script == "${SBX_FIXTURES}/github-mcp-server-stub.py"): + out.append(name) + return out + + def write_settings(sandbox: Path, plugin: Path) -> None: """The template with each placeholder string repeated once per value, wherever it sits, every environment variable `mcp.json` reads added to the variables sandboxed commands lose, and every @@ -290,6 +308,9 @@ def expand(node): return node settings = expand(template) + deny = settings["permissions"]["deny"] + for name in recording_stubs(servers): + deny[:] = [r for r in deny if r not in (f"mcp__{name}__issue_write", f"mcp__{name}__add_issue_comment")] env = settings["sandbox"]["credentials"]["envVars"] for name in sorted(set(re.findall(r"\$\{(\w+)\}", mcp)) - {e["name"] for e in env}): env.append({"name": name, "mode": "deny"}) diff --git a/sandbox/tracker-github/fixture/github-mcp-server-stub.py b/sandbox/tracker-github/fixture/github-mcp-server-stub.py index d920f34..67bcfe6 100644 --- a/sandbox/tracker-github/fixture/github-mcp-server-stub.py +++ b/sandbox/tracker-github/fixture/github-mcp-server-stub.py @@ -1,13 +1,22 @@ #!/usr/bin/env python3 -"""A read-only stand-in for github-mcp-server, serving the fixed issues and pull requests in -`github.json` beside it over stdio, so a scenario reads a GitHub issue tracker with no GitHub. Its -name holds `github-mcp-server`, so `run.py` gives it the GitHub rules: the read tools allowed and -every write denied, which it has none of. +"""A stand-in for github-mcp-server, serving the fixed issues and pull requests in `github.json` +beside it over stdio, so a scenario reads a GitHub issue tracker with no GitHub. Its name holds +`github-mcp-server`, so `run.py` gives it the GitHub rules: the read tools allowed and every write +denied. Read-only unless given `--writes <file>`: - {"command": "python3", "args": ["-I", "<this file>"]} + {"command": "python3", "args": ["-I", "<this file>", "--writes", ".git/github-writes.jsonl"]} + +With `--writes` it also serves `issue_write` (`create` and `update`) and `add_issue_comment`, and +appends each call that succeeds to the file, a path relative to the session's directory, as one +JSON line `{"tool": <name>, "arguments": <its arguments>}`; at start it replays the file, so a +later turn's server sees the issues an earlier turn wrote. A created issue takes the next number +after every issue and pull request the fixture holds. Each write returns `{"id", "url"}`, as the +server's minimal response does. A label is recorded as sent, as GitHub would create one it does not +know, so a scenario's verify checks the labels it may use. `run.py` neither allows nor denies a +writing stub's two write tools, so only a hook's allow lets a call through under `dontAsk`. Its tools are github-mcp-server's by name, arguments and output shape, as read from the real -server: `get_me`; `issue_read` with the methods `get`, `get_comments`, `get_labels` and +server: `get_me`; `get_label`, an error for a label the fixture does not hold; `issue_read` with the methods `get`, `get_comments`, `get_labels` and `get_parent`, where `get` gives labels by name and `get_labels` as objects, `get_parent` gives a null parent, since no fixture issue has one, and `get` on a pull request's number returns it as an issue whose `html_url` holds `/pull/`, as GitHub's issue API does; `list_issues`, whose states @@ -26,6 +35,8 @@ import sys from pathlib import Path +WRITES = Path(sys.argv[sys.argv.index("--writes") + 1]) if "--writes" in sys.argv else None + DATA = json.loads((Path(__file__).with_name("github.json")).read_text()) ISSUES = {i["number"]: i for i in DATA["issues"]} PULLS = {p["number"]: p for p in DATA["pulls"]} @@ -38,6 +49,7 @@ TOOLS = { "get_me": ("Get details of the authenticated GitHub user.", {}, []), + "get_label": ("Get a specific label from a repository.", {**REPO, "name": TEXT}, ["owner", "repo", "name"]), "issue_read": ("Get information about a specific issue in a GitHub repository.", {"method": {"type": "string", "enum": ["get", "get_comments", "get_sub_issues", "get_parent", "get_labels"]}, **REPO, "issue_number": NUMBER, "page": NUMBER, "perPage": NUMBER}, @@ -50,6 +62,17 @@ "search_pull_requests": ("Search for pull requests in GitHub repositories using issues search syntax already scoped to is:pr", SEARCH, ["query"]), } +WRITE_TOOLS = { + "issue_write": ("Create a new or update an existing issue in a GitHub repository.", + {"method": {"type": "string", "enum": ["create", "update"]}, **REPO, "issue_number": NUMBER, "title": TEXT, + "body": TEXT, "labels": FIELDS, "assignees": FIELDS, "state": {"type": "string", "enum": ["open", "closed"]}, + "state_reason": {"type": "string", "enum": ["completed", "not_planned", "duplicate"]}, "duplicate_of": NUMBER}, + ["method", "owner", "repo"]), + "add_issue_comment": ("Add a comment to a specific issue in a GitHub repository.", {**REPO, "issue_number": NUMBER, "body": TEXT}, + ["owner", "repo", "issue_number"]), +} +if WRITES: + TOOLS.update(WRITE_TOOLS) REACTIONS = {"+1": 0, "-1": 0, "confused": 0, "eyes": 0, "heart": 0, "hooray": 0, "laugh": 0, "rocket": 0, "total_count": 0} @@ -106,9 +129,55 @@ def chosen(row: dict, fields) -> dict: return {k: v for k, v in row.items() if k in fields} if fields else row +def write(name: str, args: dict) -> dict: + """One write applied to the served issues: a create, an update or a comment.""" + if name == "add_issue_comment": + n = int(args.get("issue_number") or 0) + if n not in ISSUES: + raise missing(args, n) + if not args.get("body"): + raise ValueError("missing required parameter: body") + cid = n * 1000 + len(ISSUES[n]["comments"]) + 1 + ISSUES[n]["comments"].append({"id": cid, "body": args["body"], "user": {"id": 1001, "login": DATA["me"]["login"]}, + "created_at": "2026-03-01T00:00:00Z", "updated_at": "2026-03-01T00:00:00Z", "author_association": "MEMBER"}) + return {"id": str(cid), "url": f"{url(args, 'issues', n)}#issuecomment-{cid}"} + method = args.get("method") + if method == "create": + if not args.get("title"): + raise ValueError("missing required parameter: title") + n = max([*ISSUES, *PULLS]) + 1 + ISSUES[n] = {"number": n, "title": args["title"], "body": args.get("body", ""), "state": "open", "labels": list(args.get("labels") or []), + "assignees": [], "user": {"id": 1001, "login": DATA["me"]["login"]}, "created_at": "2026-03-01T00:00:00Z", + "updated_at": "2026-03-01T00:00:00Z", "comments": []} + elif method == "update": + n = int(args.get("issue_number") or 0) + if n not in ISSUES: + raise missing(args, n) + i = ISSUES[n] + for key in ("title", "body", "labels", "state"): + if key in args: + i[key] = list(args[key]) if key == "labels" else args[key] + if i["state"] == "closed": + i["state_reason"] = args.get("state_reason", i.get("state_reason", "completed")) + else: + i.pop("state_reason", None) + else: + raise ValueError(f"unknown method {method!r}") + return {"id": str(n * 100), "url": url(args, "issues", n)} + + def call(name: str, args: dict): + if name in WRITE_TOOLS and WRITES: + done = write(name, args) + with WRITES.open("a") as f: + f.write(json.dumps({"tool": name, "arguments": args}) + "\n") + return done if name == "get_me": return {**DATA["me"], "profile_url": f"https://github.com/{DATA['me']['login']}"} + if name == "get_label": + if args.get("name") not in LABELS: + raise ValueError(f"failed to get label: Could not resolve to a Label with the name '{args.get('name')}'.") + return {k: LABELS[args["name"]][k] for k in ("color", "description", "name")} if name == "issue_read": n = int(args.get("issue_number") or 0) method = args.get("method") @@ -124,7 +193,7 @@ def call(name: str, args: dict): return {"comments": [{**c, "html_url": f"{url(args, 'issues', n)}#issuecomment-{c['id']}", "reactions": REACTIONS} for c in comments], "method": "get_comments"} if method == "get_labels": - labels = [LABELS[name] for name in (i["labels"] if i else [])] + labels = [LABELS.get(name, {"name": name}) for name in (i["labels"] if i else [])] return {"labels": labels, "method": "get_labels", "totalCount": len(labels)} if method == "get_parent": return {"method": "get_parent", "parent": None} @@ -178,6 +247,11 @@ def reply(rid, result=None, error=None) -> None: def main() -> None: + if WRITES and WRITES.is_file(): + for line in WRITES.read_text().splitlines(): + if line.strip(): + done = json.loads(line) + write(done["tool"], done["arguments"]) for line in sys.stdin: if not line.strip(): continue diff --git a/sandbox/tracker-github/scenarios.yaml b/sandbox/tracker-github/scenarios.yaml index 7d2f6b9..9508fe9 100644 --- a/sandbox/tracker-github/scenarios.yaml +++ b/sandbox/tracker-github/scenarios.yaml @@ -1,14 +1,17 @@ -# The GitHub tracker pack: scenarios in which `work` reads a GitHub issue as the ticket, served by +# The GitHub tracker pack: scenarios in which `work` reads a GitHub issue as the ticket, and +# `ticket-draft` drafts and files one, served by # fixture/github-mcp-server-stub.py from fixture/github.json, in `widgets` with its own standard # recording a GitHub tracker whose labels map to the workflow and no ticket pattern. Build the # sandbox as the generic pack's README says, then write its mcp.json naming the stub through # $SBX_FIXTURES, which run.py sets for the session to this pack's fixture/: # # {"mcpServers": {"github": {"command": "python3", -# "args": ["-I", "${SBX_FIXTURES}/github-mcp-server-stub.py"]}}} +# "args": ["-I", "${SBX_FIXTURES}/github-mcp-server-stub.py", "--writes", ".git/github-writes.jsonl"]}}} # -# The stub's file name holds github-mcp-server, so run.py gives the server the GitHub rules. It has -# no write tools. The issues: #9 closed as completed, #12 open with no label, #14 open and labeled +# The stub's file name holds github-mcp-server, so run.py gives the server the GitHub rules, and +# `--writes` makes it record each issue_write and add_issue_comment to .git/github-writes.jsonl, +# which each scenario's setup removes and its verify reads. run.py neither allows nor denies those +# two tools for it, so under dontAsk only hooks/issue-gate.sh's allow lets a write through. The issues: #9 closed as completed, #12 open with no label, #14 open and labeled # `ready` and `enhancement`, #17 closed as not planned and still labeled `in progress`; #15 is a # merged pull request, its body naming #9, and #16 one naming `o/r#90`. The fields are those of # ../scenarios.yaml. @@ -19,6 +22,7 @@ scenarios: setup: - mkdir -p standards && echo standards/ >> .git/info/exclude - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl origin: "#59: work routes a GitHub issue by its state and labels" why: >- Works issue 12, open with no label, so work reads it from GitHub as an issue, not a pull @@ -41,6 +45,7 @@ scenarios: setup: - mkdir -p standards && echo standards/ >> .git/info/exclude - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl origin: "#59: work routes a GitHub issue by its state and labels" why: >- Works issue 14, open and labeled ready, which the standard maps to implement-change, beside @@ -63,6 +68,7 @@ scenarios: setup: - mkdir -p standards && echo standards/ >> .git/info/exclude - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl origin: "#59: work routes a GitHub issue by its state and labels" why: >- Works issue 9, closed as completed, so work says it is past the build and runs nothing. @@ -85,6 +91,7 @@ scenarios: setup: - mkdir -p standards && echo standards/ >> .git/info/exclude - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl origin: "#59 (review of #115): a closed issue keeps its labels, and reads no pull request" why: >- Works issue 17, closed as not planned and still labeled `in progress`, one of the pull @@ -109,6 +116,7 @@ scenarios: setup: - mkdir -p standards && echo standards/ >> .git/info/exclude - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl - git checkout -q -B issue-14-low-stock origin: "#59: a branch named issue-<n>-words names issue n on a GitHub tracker" why: >- @@ -131,6 +139,7 @@ scenarios: setup: - mkdir -p standards && echo standards/ >> .git/info/exclude - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl origin: "#59 (review of #113): a pull request's number on a GitHub tracker is not an issue" why: >- Works 15, the merged pull request for issue 9, so work's one issue_read finds a pull @@ -148,3 +157,164 @@ scenarios: mcp: [mcp__github__issue_read] says: ["(?i)(pull request|PR) #?15"] not_says: ["(?m)^Next: (ticket-draft|implement-change)", "(?m)^#15 · "] + + - id: tracker-gh-ticket-draft-new + repo: widgets + setup: + - mkdir -p standards && echo standards/ >> .git/info/exclude + - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl + origin: "#59: ticket-draft drafts and files a GitHub issue under an approved write plan" + why: >- + Drafts a new issue from a description on a GitHub tracker and says yes to its write plan, so + the plan's create reaches the stub only through the approval record and the issue gate, and + the issue is read back and the slug's files take its number. + turns: + - "/lasp-dev:ticket-draft make total() refuse a negative count with a ValueError naming the part, since a bad export once gave a count of -3 and the total came out low without anyone noticing. Title it \"Don't count a negative part in total()\"." + - The goal is right. Draft this ticket. + - Take the recommended answer on every question. + - Looks right. Write all. + must_not_run: ["Bash(git push*)", "Bash(git commit*)", "mcp__github__sub_issue_write", "mcp__github__update_issue_comment"] + look_at: >- + The plan shows a create on example/widgets with the title, apostrophe and all, and the + Markdown body, then the Comment to post and the Session comment as comments. After Write all, + the issue takes the next number after every issue and pull request in github.json, N; its + context page opens `# #N · `, with N.issue.md, N.original.txt and N.writes.md beside it. A + follow-up the questions add is a second create, N+1. The closing summary names #N and says a + ticket writer's approval is the `ready` label, and the approval record is gone. + expect: + ends: success + mcp: [mcp__github__issue_write, mcp__github__issue_read] + files: [".review/context/[0-9]*.md", ".review/context/[0-9]*.issue.md"] + verify: + - test ! -e .review/github-approval.json + - >- + python3 -c 'import json, os; d = json.load(open(os.environ["SBX_FIXTURES"] + "/github.json")); + n = max(i["number"] for i in d["issues"] + d["pulls"]) + 1; + w = [json.loads(l) for l in open(".git/github-writes.jsonl")]; + c = [x["arguments"] for x in w if x["tool"] == "issue_write" and x["arguments"]["method"] == "create"]; + assert c and "# Motivation" in c[0]["body"], c; + assert all(set(x.get("labels") or []) <= {l["name"] for l in d["labels"]} for x in c), c; + assert any(x["tool"] == "add_issue_comment" and x["arguments"]["issue_number"] == n for x in w), (n, w); + assert open(".review/context/%d.md" % n).readline().startswith("# #%d · " % n)' + - test -e .review/github-plan.md + + - id: tracker-gh-ticket-draft-refine + repo: widgets + setup: + - mkdir -p standards && echo standards/ >> .git/info/exclude + - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl + origin: "#59: ticket-draft refines a GitHub issue under an approved write plan" + why: >- + Refines #12, an open issue with a title and a few sentences, and says yes to its write plan, + so the original body is posted as the Original request comment before the body is replaced, + each through the issue gate, and the issue is read back. + turns: + - /lasp-dev:ticket-draft 12 + - The goal is right. Draft this ticket. + - Take the recommended answer on every question. + - Looks right. Write all. + must_not_run: ["Bash(git push*)", "Bash(git commit*)", "mcp__github__sub_issue_write", "mcp__github__update_issue_comment"] + look_at: >- + The plan holds, in order, the Original request comment carrying #12's body as alice-example + wrote it, the body update in the format, and the Session comment; no labels change, since + nobody asked for the approval; a follow-up the questions add is a create. The body's + sections are Markdown headings, `# Motivation` first. .review/context/12.md opens `# #12 · `. + expect: + ends: success + mcp: [mcp__github__issue_read, mcp__github__issue_write, mcp__github__add_issue_comment] + files: [".review/context/12.md", ".review/context/12.issue.md", ".review/context/12.original.txt"] + verify: + - test ! -e .review/github-approval.json + - >- + python3 -c 'import json; w = [json.loads(l) for l in open(".git/github-writes.jsonl")]; + t = [(x["tool"], x["arguments"].get("method"), x["arguments"].get("issue_number")) for x in w]; + assert all(n == 12 or m == "create" for _, m, n in t), t; + u = [i for i, x in enumerate(w) if x["tool"] == "issue_write" and x["arguments"].get("issue_number") == 12]; + o = [i for i, x in enumerate(w) if x["tool"] == "add_issue_comment" and x["arguments"]["body"].lstrip().startswith("Original request")]; + assert len(u) == 1 and o and o[0] < u[0], t; + assert "Motivation" in w[u[0]]["arguments"]["body"] and "labels" not in w[u[0]]["arguments"], w[u[0]]' + - test -e .review/github-plan.md + + - id: tracker-gh-ticket-draft-declined + repo: widgets + setup: + - mkdir -p standards && echo standards/ >> .git/info/exclude + - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl + origin: "#59: ticket-draft writes nothing to GitHub without a yes to its write plan" + why: >- + Drafts the same new issue and answers the write plan's gate with Paste it all, so no write + call is made and the issue's paste file is the hand-off. + turns: + - "/lasp-dev:ticket-draft make total() refuse a negative count with a ValueError naming the part, since a bad export once gave a count of -3 and the total came out low without anyone noticing" + - The goal is right. Draft this ticket. + - Take the recommended answer on every question. + - Looks right, but do not write anything to GitHub. Paste it all. + must_not_run: ["Bash(git push*)", "Bash(git commit*)"] + must_not_try: ["mcp__github__issue_write", "mcp__github__add_issue_comment"] + look_at: >- + The files are written under the slug, .review/context/<slug>.issue.md holding Title:, Labels:, + the Markdown body, then Comment to post and Session comment; no write call is made, no + approval record is written, and the closing summary points at the paste file. + expect: + ends: success + files: [".review/context/*.issue.md"] + verify: + - test ! -e .git/github-writes.jsonl + - test ! -e .review/github-approval.json + + - id: tracker-gh-ticket-draft-blanket + repo: widgets + setup: + - mkdir -p standards && echo standards/ >> .git/info/exclude + - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl + origin: "#59: a standing answer given before the write plan's gate is no yes to it" + why: >- + Drafts the new issue and stops after the third turn, "Take the recommended answer on every + question", so a run shows that answer files nothing, whether or not the write plan's gate was shown: it does not name the gate. + turns: + - "/lasp-dev:ticket-draft make total() refuse a negative count with a ValueError naming the part, since a bad export once gave a count of -3 and the total came out low without anyone noticing" + - The goal is right. Draft this ticket. + - Take the recommended answer on every question. + must_not_run: ["Bash(git push*)", "Bash(git commit*)"] + must_not_try: ["mcp__github__issue_write", "mcp__github__add_issue_comment"] + look_at: >- + The answers are folded into the draft, which is shown with step 7's review and the write + plan's gate, Write all among its options; nothing is filed and no approval record is written. + expect: + ends: success + says: ["(?i)\\bWrite all\\b"] + verify: + - test ! -e .git/github-writes.jsonl + - test ! -e .review/github-approval.json + + - id: tracker-gh-ticket-draft-standing-answer + repo: widgets + setup: + - mkdir -p standards && echo standards/ >> .git/info/exclude + - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - rm -f .git/github-writes.jsonl + origin: "#59: a request to file, then a standing answer, is no yes to the write plan's gate" + why: >- + Asks in plain words to file an issue, which names no plan, then answers every question with + its recommendation, so a run shows the plan's gate is shown and nothing is filed: neither + the request nor the standing answer names the plan. + turns: + - "File an issue for this bug: by_part raises ValueError on a part listed twice, and the warehouse team needs the counts summed instead." + - The goal is right. Draft this ticket. + - Take the recommended answer on every question. + must_not_run: ["Bash(git push*)", "Bash(git commit*)"] + must_not_try: ["mcp__github__issue_write", "mcp__github__add_issue_comment"] + look_at: >- + ticket-draft runs, and the third turn ends at step 7's review with the write plan's gate, + Write all among its options; nothing is filed. + expect: + ends: success + skills: [ticket-draft] + says: ["(?i)\\bWrite all\\b"] + verify: + - test ! -e .git/github-writes.jsonl + - test ! -e .review/github-approval.json diff --git a/sandbox/tracker/scenarios.yaml b/sandbox/tracker/scenarios.yaml index 8a9fe4f..9a6cccd 100644 --- a/sandbox/tracker/scenarios.yaml +++ b/sandbox/tracker/scenarios.yaml @@ -131,3 +131,25 @@ scenarios: mcp: [mcp__atlassian__jira_search] agents: [ticket-investigator] not_says: ["(?i)\\*\\*Reformat it\\*\\*|reformat it, or draft it again"] + + - id: tracker-ticket-draft-github-request + repo: widgets + setup: + - mkdir -p standards && echo standards/ >> .git/info/exclude + - cp "$SBX_FIXTURES/standard-profile.yaml" standards/profile.yaml + - cp "$SBX_FIXTURES/standard-tracker.md" standards/tracker.md + origin: "#59: ticket-draft drafts a GitHub issue only where the tracker is GitHub Issues" + why: >- + Asks for a GitHub issue in a repository whose tracker is Jira, so ticket-draft says the + tracker is not GitHub Issues and asks whether to draft a Jira ticket or stop, before reading + anything. + turns: + - "/lasp-dev:ticket-draft open a GitHub issue: total() should refuse a negative count with a ValueError naming the part" + must_not_run: ["Bash(git push*)", "Bash(git commit*)", "Agent", "mcp__*"] + look_at: >- + One line saying this repository's tracker is Jira, not GitHub Issues, then a question: Draft + it as a Jira ticket (Recommended) or Stop. No reader runs and nothing is written. + expect: + ends: success + says: ["(?i)not GitHub Issues", "(?i)\\bStop\\b"] + not_says: ["(?m)^## total"] diff --git a/sandbox/triggers/coverage.yaml b/sandbox/triggers/coverage.yaml index 4722687..f33a80b 100644 --- a/sandbox/triggers/coverage.yaml +++ b/sandbox/triggers/coverage.yaml @@ -1,4 +1,4 @@ # The trigger pack's set: every scenario in scenarios.yaml beside this file, so sandbox/smoke.py # with --scenarios sandbox/triggers/scenarios.yaml runs the whole pack on both checkouts. -smoke: [trigger-architecture-doc, trigger-architecture-doc-confluence, trigger-commit, trigger-commit-history, trigger-design-doc, trigger-downstream, trigger-downstream-notice, trigger-drawio, trigger-implement-change, trigger-implement-change-finished, trigger-onboard, trigger-pr-create, trigger-pr-fix, trigger-pr-fix-push-back, trigger-pr-review, trigger-pr-review-once-over, trigger-process-doc, trigger-process-doc-wiki, trigger-quickstart, trigger-rebase, trigger-skill-feedback, trigger-test-suite-review, trigger-test-suite-review-solid, trigger-ticket-draft, trigger-work, trigger-writing, trigger-writing-rambles, notrigger-ticket-draft-github-issue] +smoke: [trigger-architecture-doc, trigger-architecture-doc-confluence, trigger-commit, trigger-commit-history, trigger-design-doc, trigger-downstream, trigger-downstream-notice, trigger-drawio, trigger-implement-change, trigger-implement-change-finished, trigger-onboard, trigger-pr-create, trigger-pr-fix, trigger-pr-fix-push-back, trigger-pr-review, trigger-pr-review-once-over, trigger-process-doc, trigger-process-doc-wiki, trigger-quickstart, trigger-rebase, trigger-skill-feedback, trigger-test-suite-review, trigger-test-suite-review-solid, trigger-ticket-draft, trigger-ticket-draft-github-issue, trigger-work, trigger-writing, trigger-writing-rambles, notrigger-ticket-draft-other-project, notrigger-ticket-draft-plugin-issue] diff --git a/sandbox/triggers/scenarios.yaml b/sandbox/triggers/scenarios.yaml index 3a6e28d..31c7653 100644 --- a/sandbox/triggers/scenarios.yaml +++ b/sandbox/triggers/scenarios.yaml @@ -324,13 +324,37 @@ scenarios: ends: success skills: [writing] - - id: notrigger-ticket-draft-github-issue + - id: trigger-ticket-draft-github-issue repo: widgets - origin: "lasp/dev-tools#23: a GitHub issue request must not reach the Jira-only ticket-draft" - why: Asks for a GitHub issue in plain words, so a run shows the request does not reach ticket-draft, which drafts and files only Jira tickets. + origin: "#59: ticket-draft drafts a GitHub issue for a GitHub tracker" + why: Asks for a GitHub issue in plain words, so a run shows the request reaches ticket-draft, which drafts a Jira ticket or a GitHub issue for the profile's tracker. turns: - Open a GitHub issue on this repo for the blank-count crash in the CSV loader. + must_not_run: ["mcp__*", "Bash(gh *)", "Bash(git push*)"] + look_at: The request reaches ticket-draft, which restates the goal and asks to confirm it before any search, and files nothing. + expect: + ends: success + skills: [ticket-draft] + + - id: notrigger-ticket-draft-other-project + repo: widgets + origin: "#59: ticket-draft files only into this repository's tracker" + why: Asks for an issue on another project, so a run shows the request does not reach ticket-draft, which files into this repository's tracker only. + turns: + - "Open an issue on numpy's GitHub about this dtype bug: np.sum on an int8 array overflows silently" must_not_run: ["Skill(*ticket-draft)", "mcp__*", "Bash(gh *)", "Bash(git push*)"] - look_at: The request does not reach ticket-draft; the session says GitHub is not connected or drafts the issue text for the person, and files nothing. + look_at: The request does not reach ticket-draft; the session says it cannot file on another project from here, or drafts the text for the person, and files nothing. expect: ends: success + + - id: notrigger-ticket-draft-plugin-issue + repo: widgets + origin: "#59: a complaint about a skill, asked as an issue on the plugin, stays skill-feedback's once ticket-draft drafts GitHub issues" + why: Asks for an issue on the plugin about ticket-draft, so a run shows the complaint reaches skill-feedback and not ticket-draft. + turns: + - "Open an issue on the plugin about ticket-draft: it wrote me a 900-word ticket for a one-line fix." + must_not_run: ["Skill(*ticket-draft)", "mcp__*", "Bash(gh *)", "Bash(git push*)"] + look_at: The request reaches skill-feedback, which asks what happened and what the person would like instead, and files nothing; ticket-draft does not run. + expect: + ends: success + skills: [skill-feedback] diff --git a/scripts/next_phase.py b/scripts/next_phase.py index f21aacb..0e2ecbf 100755 --- a/scripts/next_phase.py +++ b/scripts/next_phase.py @@ -30,7 +30,7 @@ | mark-ready: a draft, every thread answered | nothing: mark it ready for review; a fix taken as made from a judged reply is named, to confirm | | answered: every thread answered | nothing: the merge is a person's; discuss threads, and a fix taken as made from a judged reply, are named | | elsewhere: the ticket's context page names other branches, not this one | nothing: switch to its branch | -| unfiled: no key, and the fields' `slug=` names a ticket `ticket-draft` drafted that is not filed | nothing: file its `.jira.txt`, or have `ticket-draft <slug>` file it or take its key | +| unfiled: no key, and the fields' `slug=` names a ticket `ticket-draft` drafted that is not filed | nothing: file its `.jira.txt`, on a GitHub tracker its `.issue.md`, or have `ticket-draft <slug>` file it or take its key | | ticket-draft: no context page, the ticket in a status whose `next` is ticket-draft, or unknown, or no key; or an open GitHub issue with no mapped label | ticket-draft | | ticket-to-do: no context page, the ticket in a status whose `next` is implement-change (`--ticket-status`, or an open GitHub issue's mapped label) and no pull request | implement-change | | ticket-closed: no context page, the ticket in a status whose `next` is nothing, or a closed GitHub issue, and no pull request | nothing | @@ -417,7 +417,9 @@ def decide( if fields["elsewhere"]: return row("nothing", ticket, "elsewhere", f"built on {fields['elsewhere']}, not here") if fields["slug"]: - return row("nothing", fields["slug"], "unfiled", f"drafted as {fields['slug']}, not filed") + known = workflow() + return {**row("nothing", fields["slug"], "unfiled", f"drafted as {fields['slug']}, not filed"), + "paste": "issue.md" if known and known[0] == "github" else "jira.txt"} if not fields["context"]: status = ticket_status.strip().lower() known = workflow() if ticket and (status or issue_state) else None @@ -471,7 +473,7 @@ def decide( "answered": "Nothing: settle any discuss thread by saying whether you agree; the merge is a person's.", "build-stopped": "Nothing yet: retry the build, open the pull request anyway, or discuss.", "ticket-closed": "Nothing: the ticket is past the build; open it in the tracker to see why.", - "unfiled": "Nothing: file `.review/context/{argument}.jira.txt` in the tracker, or have `/lasp-dev:ticket-draft {argument}` file it; " + "unfiled": "Nothing: file `.review/context/{argument}.{paste}` in the tracker, or have `/lasp-dev:ticket-draft {argument}` file it; " "give it the key to rename the page, then run `/lasp-dev:work <key>`.", } @@ -482,7 +484,7 @@ def lines(ticket: str, d: dict, stack: list[dict] | None = None) -> list[str]: p for p in (shown(ticket) or "no ticket", listed, f"pull request #{d['argument']}" if d["why"] in ON_PR else "", d["say"]) if p ) if d["phase"] == "nothing": - return [head, NEXT[d["why"]].format(argument=d["argument"]), "It will ask you: nothing."] + return [head, NEXT[d["why"]].format(argument=d["argument"], paste=d.get("paste")), "It will ask you: nothing."] return [head, f"Next: {d['phase']}.", f"It will ask you: {ASKS[d['phase']]}"] diff --git a/skills/ticket-draft/SKILL.md b/skills/ticket-draft/SKILL.md index b3294b4..21f3484 100644 --- a/skills/ticket-draft/SKILL.md +++ b/skills/ticket-draft/SKILL.md @@ -1,17 +1,20 @@ --- name: ticket-draft description: > - Drafts or refines a Jira ticket in the team's format, placed among existing work, from a - description, a ticket or a pull request with none; can brief you on the code first, fold in - later answers, or reformat a ticket to the current format; writes to Jira only on your yes. Use when - asked to "write a ticket for", "file a ticket", "create a Jira ticket", "log a bug in Jira", - "refine this ticket", "is this ticket ready", "what is this ticket missing" or "groom this - ticket". Not for a GitHub issue, which it neither drafts nor files, or a complaint about a - lasp-dev skill, which is skill-feedback's. -argument-hint: "[investigate | settle | reformat] <ticket key | description | pull request> [thread id]" + Drafts, refines and files a ticket in the team's format, a Jira ticket or a GitHub issue, + placed among existing work, from a description, a ticket or a pull request with none; can brief + you on the code first, fold in later answers, or reformat a ticket to the current format; files + only on your yes. Use it, rather than `gh issue create` or an issue written by hand, for any + request to put a bug or a change into this repository's tracker: "open a GitHub issue for", + "write a ticket for", "file a ticket", "create a Jira ticket", "log a bug", "refine this ticket", + "is this ticket ready", "what is this ticket missing" or "groom this ticket". Not for an issue + on another project, such as a library this repository uses, which is filed on that project. + Not for a bug or a complaint about a lasp-dev skill or agent, this one included, however it is + asked for, even as an issue on the plugin: that is skill-feedback's alone. +argument-hint: "[investigate | settle | reformat] <ticket key | issue #n | description | pull request> [thread id]" metadata: author: lasp-dev - version: 1.10.19 + version: 1.11.0 --- Every question, and every file it writes, follows `${CLAUDE_PLUGIN_ROOT}/skills/work/decision-points.md`. The plugin root is `${CLAUDE_PLUGIN_ROOT}`: a path written `<plugin root>/…` in a file this skill reads starts there. @@ -33,7 +36,7 @@ the repository's state, a TODO sweep or a roadmap on its own initiative. | Option | Default | |---|---| | Mode | `draft`; `investigate` adds a briefing on the code and the design choices, and the person's questions, before the draft; `settle` folds answers that arrived since the draft, from the estimation meeting, a comment or a pull request thread, into a drafted ticket, and records a departure the work keeps as a comment; `reformat` rewrites a ticket that already holds its requirements in the current format, with no search or investigation | -| Source | A Jira ticket key, a description of the change, a markdown file whose text is the prompt, or a pull request number or URL; asked for when none is given. `work` names a slug whose page is under `.review/context/` for a ticket not filed yet, and `pr-fix` passes `settle <key> <thread id>` | +| Source | A Jira ticket key, or on a GitHub tracker an issue's number, `#12`, a description of the change, a markdown file whose text is the prompt, or a pull request number or URL; asked for when none is given. `work` names a slug whose page is under `.review/context/` for a ticket not filed yet, and `pr-fix` passes `settle <key> <thread id>` | ## Reads @@ -93,7 +96,7 @@ section 4 laid out as `using-jira`'s "Copy-paste output" sets: its `Comment to p `<ticket>.original.txt`'s text with its markup put right as that section says, signed with the `ticket` block's reporter and created date, or for a new ticket the person and today; its `Session comment` comes from What changed. -In Jira, only at step 8 and only the items of the write plan the person approved at step 7, as +In the tracker, only at step 8 and only the items of the write plan the person approved at step 7, as `using-jira`'s "Writing to Jira" and `${CLAUDE_SKILL_DIR}/../using-jira/write-plan.md` set out. The filed and the approved status are the profile's: `python3 "${CLAUDE_PLUGIN_ROOT}/scripts/profile.py" --defaults --field jira.roles` prints them, and an error from it, or `not yet recorded`, is a stop @@ -101,7 +104,8 @@ for the status move, as `write-plan.md` says. Each text sent is kept under `.review/context/<ticket>.send/`, as shown (`.txt`) and as sent (`.md`), with `stored.txt`, the description as read back, and each item's outcome in `.review/context/<ticket>.writes.md`. Whatever the plan did not write stays in -`<ticket>.jira.txt` to paste. +`<ticket>.jira.txt` to paste. On a GitHub tracker the paste file is `<ticket>.issue.md` and the +plan is GitHub's, as `${CLAUDE_SKILL_DIR}/github.md` says. ## How each turn reads @@ -111,6 +115,14 @@ never in a preamble. Nothing comes before step 1's opening block. ## Steps +On a GitHub tracker, where `python3 "${CLAUDE_PLUGIN_ROOT}/scripts/profile.py" --defaults --field +issue_tracker.kind` prints `github`, read `${CLAUDE_SKILL_DIR}/github.md` first, which says what +changes in every step below and in the files. Where it prints anything else and the request asks +for a GitHub issue, say before step 1, in one line, that this repository's tracker is not GitHub +Issues and what it is (`jira`, or none recorded, where `/lasp-dev:onboard` records one), and +ask: **Draft it as a Jira ticket** (Recommended where the tracker is Jira), which goes on with the +steps below, or **Stop**, which writes nothing. Nothing is read before the answer. + A pull request as the source, or a slug whose page exists under `.review/context/`, which `work` names for a page with no key yet: read `${CLAUDE_SKILL_DIR}/sources.md` first, which says what changes. `settle` mode is `${CLAUDE_SKILL_DIR}/settle.md` in place of these steps, and `reformat` @@ -371,8 +383,9 @@ mode `${CLAUDE_SKILL_DIR}/reformat.md`. "Writing to Jira" allows them, each status as the profile's `jira.roles` names it ("Writes"). Ask **Write all <n>** (Recommended), **Drop some** or **Paste it all**, beside Looks right; - the answer counts only with Looks right, and is asked again after a Change it. Without the - write tools, say so in one line and ask no gate. + the answer counts only with Looks right, and is asked again after a Change it. Write all is + never taken on a standing answer (`${CLAUDE_PLUGIN_ROOT}/skills/work/decision-points.md`, + "What does not change"). Without the write tools, say so in one line and ask no gate. 8. **Write the files, write the plan, and hand off.** Write the page, its `.review/context.md` copy except for a pull request source, and `<ticket>.jira.txt` first, so the paste is ready whatever follows. Then run the plan, as the plan file sets out, its texts under @@ -406,7 +419,8 @@ such as a typo fix, say so and stop. ## Never -Write to Jira outside step 8's approved write plan, or to GitHub. Put an estimate in the ticket +Write to the tracker outside step 8's approved write plan, or to GitHub other than a GitHub +tracker's issues there. Put an estimate in the ticket block or Story Points. Call two tickets the same without quoting the criteria that match. Write a ticket nobody asked for. Write step 8's files before the person's Looks right; the write plan's `.review/jira-plan.md` is the one file written to show it. diff --git a/skills/ticket-draft/github.md b/skills/ticket-draft/github.md new file mode 100644 index 0000000..8a57c0b --- /dev/null +++ b/skills/ticket-draft/github.md @@ -0,0 +1,56 @@ +# A GitHub tracker + +Read by `ticket-draft` where `python3 "<plugin root>/scripts/profile.py" --defaults --field +issue_tracker.kind` prints `github`. The ticket is an issue, its key the number: `12` in file +names, `#12` wherever a person reads it. The issue's fields, the plan's items, the record, the +read-back and the paste file are `<plugin root>/skills/using-github/issues.md`'s; read it before +step 7. What differs from SKILL.md's steps is below; the rest runs as written, `settle.md` and +`reformat.md` included. + +- **The source.** An issue's number, `#12` or `12`, is read with one `source-reader` running + `ticket 12`; a block that says `Not an issue: #12 is a pull request` makes it a pull request + source (`sources.md`). GitHub stops the draft for an existing issue where SKILL.md says + Atlassian does; Atlassian is needed only for the year plan, which is skipped without it and said + once, as "Reads" says. +- **Step 1.** `<ticket>.original.txt` holds the issue's body verbatim. The opening block's heading + is `## #12 · <title>` and its second line `Issue · <state> · <labels, or no labels>`. +- **Step 2.** The `search` reader is given `Tracker: github <owner/repo>`, the tracker's location + or this repository when `--field issue_tracker.location` prints `none`, in place of `Jira: + <project>`. +- **Steps 3 and 5.** An issue has no epic, component, due date or typed link. The year-plan month, + when the plan was read, and a parent the `ticket` block's `Links:` line names, `parent: #<n>`, + are said in Placement and go nowhere else: no item sets or changes a parent. A neighbor the text names + stays in the body: under Related Out of Scope Tickets when it takes work out of scope, otherwise + in The code today, each as `#<n>`. A piece of a split or a follow-up names the issues created + before it by number and those after it by title. With no `References:`, the estimate says it has + none to compare with, as step 5 says, and stays in `.review/`. +- **Step 7's plan** is the items `issues.md` gives, written to `.review/github-plan.md` before it + is shown and linked at the gate: + - For an existing issue: the `Comment to post` unless the block's `Original request comment:` + is `yes`, then the body, and the title when step 7 proposes a new one, as one update, then the + `Session comment`. + - For a new issue: the create, with its title, the body and no labels unless the person names + some the repository has, then the `Comment to post` and the `Session comment`. + - A merge closes the issue not kept with `state_reason: duplicate` and `duplicate_of` the kept + one, after its `Merged from` comment on the kept one. + - A move into a role is a labels update, offered only when the person asks for it and only + when `--defaults --field issue_tracker.roles.<role>` prints a label; `none` skips it, said in + one line. `jira.roles` is not read. +- **Step 8.** The paste file `<ticket>.issue.md` is written first, as `issues.md`'s "The paste + file" lays it out. Then, on Write all: + 1. Write each text to `.review/context/<ticket>.send/<item>.md`, then + `.review/github-approval.json` listing the approved items, as `issues.md` says. + 2. Make each call in order. A create's number, the last part of its `url`, replaces + `new:<slug>` in the items after it, which go in the record once it is written again. + 3. Read each issue back as `issues.md`'s "Read it back" says, record each item's outcome in + `<ticket>.writes.md`, and remove the record. + 4. A create's number replaces the slug: rename `.review/context/<slug>.md`, `<slug>.issue.md`, + `<slug>.original.txt`, `<slug>.send/` and `<slug>.writes.md` to the number, and write the + page's heading, and its `.review/context.md` copy's, as `# #12 · <title>`. + + Without the write tools, or on Paste it all, the paste file is the hand-off: say so in one + line and give its path. The closing summary names each issue written as `#12` with the URL the + create or the read-back gave. Unless the plan moved it, the last line says a ticket writer + approves the issue next by adding the label `--defaults --field issue_tracker.roles.approved` + prints, naming it, or, where that prints `none`, on the issue itself, and names + `/lasp-dev:work 12`. diff --git a/skills/using-github/SKILL.md b/skills/using-github/SKILL.md index e19d8a9..4c29ee6 100644 --- a/skills/using-github/SKILL.md +++ b/skills/using-github/SKILL.md @@ -6,7 +6,7 @@ description: > user-invocable: false metadata: author: lasp-dev - version: 1.1.1 + version: 1.2.0 --- ## Resolving the tools @@ -55,6 +55,13 @@ caller asks the person. Tell every agent a skill spawns `GitHub: <server name>` or `GitHub: gh`. The agent reads with that and nothing else. +## Issues as the tracker + +Where the standard's tracker is GitHub Issues, read `${CLAUDE_SKILL_DIR}/issues.md` before drafting, +filing or changing an issue as a ticket: the issue's fields, a write plan's items, the read-back +and the paste file. The plugin root is `${CLAUDE_PLUGIN_ROOT}`: a path written `<plugin root>/…` +there starts at it. + ## Pushing A push waits for a yes that names it; it may share an option with the action that needs it, as diff --git a/skills/using-github/issues.md b/skills/using-github/issues.md new file mode 100644 index 0000000..d9c2d6e --- /dev/null +++ b/skills/using-github/issues.md @@ -0,0 +1,122 @@ +# GitHub issues as the tracker + +How a skill writes a ticket where the standard's tracker is GitHub Issues +(`python3 "<plugin root>/scripts/profile.py" --defaults --field issue_tracker.kind` prints +`github`): the issue's fields, the items a write plan may hold, the read-back and the paste file. +The ticket is an issue at `--field issue_tracker.location`, or this repository's own issues when +that prints `none`; its key is the number, `12` in file names and arguments, `#12` wherever a +person reads it. Showing the plan, its gate, the order, running it and failures are +`<plugin root>/skills/using-jira/write-plan.md`'s, read with the differences below; the server is +picked as `<plugin root>/skills/using-github/SKILL.md` says. + +## The issue + +- **Title**: one line, starting with a verb, at most twelve words. +- **Body**: the description in `<plugin root>/standards/ticket-format.md`'s format, in Markdown as + that file shows it, each section under its `#` or `##` heading, never a bold line in its place. + GitHub stores the body as it is sent, so nothing is converted. +- **Labels**: only labels the repository already has; a label is never created. The server has no + tool that lists a repository's labels, only `get_label` for one name, so check each label a plan + would send with `get_label`, and offer only those the profile maps or the repository's issues + carry (`list_issues`, `issue_read` `get_labels`): a label no issue carries yet is missed unless + the person names it. One `get_label` cannot find is left out and said so. A role's label is `--defaults --field issue_tracker.roles.<role>`, `none` for a + role with no label. +- **State**: `open`, or `closed` with a `state_reason`: `completed` for the `finished` role, + `not_planned` for `abandoned`, and `duplicate`, with `duplicate_of`, for a ticket merged into + another. + +There is no issue type, component, epic, parent, due date, estimate, Projects field or sub-issue: +a plan never holds one, and the hook gives no decision on a call that sends one. Neighbors have +no typed links: a ticket names them as `#<n>` in the body's sections, Related Out of Scope Tickets +for work taken out of its scope, and The code today for one it builds on. A pull request names its +issue on the `Ticket:` line `pr-create` writes, never with a closing keyword. + +## What an item is + +| Item | Tool | Target | Shown as | +| --- | --- | --- | --- | +| Create | `issue_write`, `method: create`: `owner`, `repo`, `title`, `body`, `labels` | `<owner>/<repo> <title>` | The title, the labels and the body in full. Until it exists the issue is `new:<slug>` in every other item | +| Update | `issue_write`, `method: update`, `owner`, `repo`, `issue_number`, with what changes: the `title`, the `body`, the `labels` as one list, or the `state` with its `state_reason` and, for a duplicate, `duplicate_of` | `<owner>/<repo>#<n>` | Each field now and after; the body in full; labels as the whole list now and after, since the call replaces it | +| Comment | `add_issue_comment`: `owner`, `repo`, `issue_number`, `body` | `<owner>/<repo>#<n>` | The comment in full. An `Original request` or a session comment opens with the plain first line `using-jira`'s "Copy-paste output" gives, no Markdown before its first word, since the revision finds the baseline by it | + +Moving a ticket into a role is a labels update: the role's label added and any other mapped +label removed, since an issue carries at most one. A move into a role whose label is `none` is +skipped and said so: an open issue with no mapped label is already `filed`, and `approved` and +`implementation` have nothing to move to. The move from `filed` to `approved` is the ticket +writer's approval, proposed only when the person asks for it. Closing is a state update, last. + +The order is `write-plan.md`'s, with these steps empty: links, and assignees, which no plan holds +here. + +## Showing it + +As `write-plan.md`'s "Showing it" says, written to `.review/github-plan.md` in place of +`.review/jira-plan.md`, each item naming its tool and target. Write the file with the Write tool +before the plan is shown, every time it is shown, and name it as a link at the gate: the texts sent +are copied from it, and a plan shown without it cannot be run. A posted comment can be edited or +deleted on GitHub, but its first text reaches whoever watches the repository, so say so. + +## Running it + +- **No conversion.** Each text is written to `<dir>/<item>.md`, copied verbatim from + `.review/github-plan.md`, and sent as it is. +- **Find the tools** with `ToolSearch` by what they do (`issue write`, `add issue comment`), as + `using-github` says. With neither, hand everything over to paste and say why in one line, a + server not connected or failing as `<plugin root>/skills/work/servers.md` sets out. +- **Record the yes.** Before the first call, write the approval record `hooks/issue-gate.sh` + reads, `.review/github-approval.json` at the repository's top level, listing exactly the items to + be written, one `{"tool": ..., "target": ...}` each, the tool without its server prefix and the + target as the table gives it, its owner, repository and title copied exactly as the call will + send them, since the hook compares them case and all (`docs/contracts.md`, "The GitHub approval + record"). A call sends only the arguments the table names; the hook gives a call with any other, + such as `assignees`, no decision. A create's target holds its title, free text that may carry + quotes, so the items never go through a command: write them as a JSON array with the Write tool + to `.review/github-approval-items.json` (`<plugin root>/skills/work/decision-points.md`, + "Writing a file"), then build the record from that file: + + ```bash + top=$(git rev-parse --show-toplevel) + jq -n --arg s "${CLAUDE_CODE_SESSION_ID:?}" --argjson t "$(date +%s)" --slurpfile items "$top/.review/github-approval-items.json" \ + '{session: $s, t: $t, items: $items[0]}' > "$top/.review/github-approval.json" + ``` + + An item that names `new:<slug>` goes in once its create has landed, with the number the + create's `url` ends in: write the items file again and run the same command. The hook allows + only a call the record lists, from this session, for 30 minutes, and gives no decision on any + other, so the person's own permissions decide it. Remove both files, + `rm -f "$(git rev-parse --show-toplevel)"/.review/github-approval{,-items}.json`, when the plan + has been read back, and as soon as it stops, as `write-plan.md` says of the Jira record. +- **A failure** is handled as `write-plan.md` says; a no at a permission prompt is one. The hook + does not use an item up, so a create sent again files a second issue: before a create that + failed or timed out goes again, search the repository's open issues for its exact title + (`search_issues` or `list_issues`), and when one is there, take its number as the create's and + go on. + +## Read it back + +For each issue the plan touched, one `issue_read` `get`, and `get_comments` when the plan posted a +comment, and say for each item whether it landed: the title, the labels and the state as sent, the +body equal to the text sent once line endings are made `\n`, and each comment present. A body that +differs is a failed item, named with the first line that differs. + +## The paste file + +`.review/context/<ticket>.issue.md`, for a person to file by hand what the plan did not write: + +``` +Title: <the title> +Labels: <the labels, comma-separated, or none> + +<the body, in Markdown> + +---- +Comment to post +<the Original request comment, as using-jira's "Copy-paste output" words it, in Markdown> + +---- +Session comment +<the session comment, as using-jira's "Copy-paste output" words it> +``` + +A block with nothing to post is left out. The person creates the issue with the title, labels and +body, then posts `Comment to post` and `Session comment` as comments, in that order. diff --git a/skills/using-jira/SKILL.md b/skills/using-jira/SKILL.md index cbf658b..098b07a 100644 --- a/skills/using-jira/SKILL.md +++ b/skills/using-jira/SKILL.md @@ -7,7 +7,7 @@ description: > user-invocable: false metadata: author: lasp-dev - version: 1.9.0 + version: 1.9.1 --- ## The project diff --git a/skills/using-jira/write-plan.md b/skills/using-jira/write-plan.md index d9781ad..b9b84a5 100644 --- a/skills/using-jira/write-plan.md +++ b/skills/using-jira/write-plan.md @@ -2,7 +2,9 @@ How a skill writes to Jira: every write a pass proposes, in one numbered list the person approves once, run in order, read back and recorded. `ticket-draft` steps 7 and 8 and `ticket-audit`'s -Apply use it; `SKILL.md`'s "Writing to Jira" says what a plan may hold. +Apply use it; `SKILL.md`'s "Writing to Jira" says what a plan may hold. Where the tracker is +GitHub Issues, a plan follows this file with the items, tools, record and read-back +`<plugin root>/skills/using-github/issues.md` sets in place of Jira's. ## When the tools are there @@ -87,7 +89,8 @@ and the texts as Then ask, as a gate in `<plugin root>/skills/work/decision-points.md`: **Write all <n>** (Recommended); **Drop some**, the numbers typed under Other; **Paste it all**; **Let's discuss**. -The yes covers exactly the items shown. A change to any item shows the plan again and asks again. +The yes covers exactly the items shown; Write all is never taken on a standing answer +(`decision-points.md`, "What does not change"). A change to any item shows the plan again and asks again. Where the skill already asks a widget at that point, the gate is one more question in it. ## Running it diff --git a/skills/work/SKILL.md b/skills/work/SKILL.md index 6cf82a9..d00debb 100644 --- a/skills/work/SKILL.md +++ b/skills/work/SKILL.md @@ -7,7 +7,7 @@ description: > argument-hint: "[ticket key or issue number | PR number | nothing]" metadata: author: lasp-dev - version: 1.9.1 + version: 1.9.2 --- # Work @@ -180,7 +180,7 @@ options are **Retry the build** (`implement-change`, resuming), **Open the pull outcome line: Open anyway when the build was blocked by a pre-existing failure, Retry when it stopped for a person who has since acted, and Let's discuss when the review cap was reached or a finding kept returning, since a retry meets the same finding. On `why: unfiled`, the three -lines name the slug's `.jira.txt` to file in the tracker and `ticket-draft <slug>`, which files it or +lines name the slug's `.jira.txt`, or `.issue.md` on a GitHub tracker, to file in the tracker and `ticket-draft <slug>`, which files it or takes its key and renames the page; stop there. ## 4. Run it, then look again diff --git a/skills/work/decision-points.md b/skills/work/decision-points.md index 001d917..cd58395 100644 --- a/skills/work/decision-points.md +++ b/skills/work/decision-points.md @@ -46,14 +46,19 @@ one, remove it first with `rm -f <path>`, a plain command, then write it. Bash runs scripts and git, and these stay commands: a script's output redirected to a file, such as `bash standard_file.sh > .review/standard.md.tmp`; `commit`'s message handed to git with `git commit -F - <<'MSG'`; and `write-plan.md`'s `jq` approval record, which holds only fixed-form -items, tool names and keys. +items, tool names and keys. An approval record whose items hold free text, as a GitHub create's +title does, has its items written with the Write tool and read by `jq` from that file +(`using-github`'s `issues.md`). ## What does not change -- **Gates.** Anything hard to undo, a post, a push, a commit or a write to Jira or Confluence, waits - for a yes to exactly what it does: a request the person typed that names it, or an answer to a - question or option that names it. A gate may share a widget with other questions at the same - point; it is never implied by an answer to something else. +- **Gates.** Anything hard to undo, a post, a push, a commit, a write to Jira or Confluence or a + GitHub issue write, waits for a yes to exactly what it does: a request the person typed that + names it, or an answer to a question or option that names it. A gate may share a widget with + other questions at the same point; it is never implied by an answer to something else, nor by a + standing answer that does not name it, such as "take the recommended answer on every + question", whether it came before the gate was asked or after: show the gate, or show it again, + and ask it. A yes given in advance that names the gate, such as "commit this to main", is one. - **The text is shown with the gate.** A gate that posts or writes text, a review, a comment, a reply, a pull request body, a Jira description or comment, a Confluence page or section, shows that text as it will be sent in the message that asks, never by pointing to "above", an earlier diff --git a/tests/test_issue_gate.sh b/tests/test_issue_gate.sh new file mode 100755 index 0000000..8667dff --- /dev/null +++ b/tests/test_issue_gate.sh @@ -0,0 +1,168 @@ +#!/usr/bin/env bash +# Feeds hooks/issue-gate.sh github-mcp-server's issue_write and add_issue_comment, under a server +# name of the person's choosing, and checks that a call this session's unexpired, well-formed +# approval record lists with its target is allowed, and that every other call gets no decision: +# no record, an unlisted target or tool, another session, a stale, future or malformed record, a +# subagent's call, a create of another title, a call carrying an argument no plan sends, input it +# cannot read, and any call without jq. Also checks that hooks.json's matcher sends it those two tools and no other. +# Run: bash tests/test_issue_gate.sh +set -u + +root=$(cd "$(dirname "$0")/.." && pwd) +gate="$root/hooks/issue-gate.sh" +failures=0 +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +cd "$work" || exit 1 + +check() { + if eval "$2"; then + echo " ok $1" + else + echo " FAIL $1" + failures=$((failures + 1)) + fi +} + +# The decision for a call of $1 with input $2, from session $3 (default s1): allow, or none when +# the gate printed nothing. Anything else, a non-zero exit included, is printed as it came. +decision() { + local out + jq -e . >/dev/null 2>&1 <<<"$2" || { echo "bad test input"; return; } + out=$(jq -cn --arg t "mcp__github-example__$1" --argjson i "$2" --arg s "${3:-s1}" '{tool_name:$t, tool_input:$i, session_id:$s}' | "$gate" 2>/dev/null) \ + || { echo "exit $?"; return; } + [ -z "$out" ] && { echo none; return; } + printf '%s' "$out" | jq -r .hookSpecificOutput.permissionDecision 2>/dev/null || printf '%s' "$out" +} +create='{"method":"create","owner":"example","repo":"widgets","title":"Refuse a negative count","body":"b","labels":["ready"]}' +update='{"method":"update","owner":"example","repo":"widgets","issue_number":12,"title":"t","body":"b","labels":["ready"],"state":"closed","state_reason":"duplicate","duplicate_of":9}' +comment='{"owner":"example","repo":"widgets","issue_number":12,"body":"c"}' +# Writes the approval record: items as JSON, then the session and the epoch seconds it was made. +approve() { + mkdir -p .review + jq -n --argjson items "$1" --arg s "${2:-s1}" --argjson t "${3:-$(date +%s)}" '{session:$s, t:$t, items:$items}' \ + > .review/github-approval.json +} +all_items='[{"tool":"issue_write","target":"example/widgets Refuse a negative count"},{"tool":"issue_write","target":"example/widgets#12"}, +{"tool":"add_issue_comment","target":"example/widgets#12"}]' +each_is() { + [ "$(decision issue_write "$create" "${2:-s1}")" = "$1" ] && [ "$(decision issue_write "$update" "${2:-s1}")" = "$1" ] \ + && [ "$(decision add_issue_comment "$comment" "${2:-s1}")" = "$1" ] +} + +echo "issue-gate.sh" + +check "the gate is executable" '[ -x "$gate" ]' +check "with no record, no call gets a decision" 'each_is none' +approve "$all_items" +check "with this session's record, a listed create, update and comment are allowed" 'each_is allow' +check "the reason names the tool" \ + 'jq -cn --argjson i "$create" "{tool_name:\"mcp__gh__issue_write\", tool_input:\$i, session_id:\"s1\"}" | "$gate" | jq -e ".hookSpecificOutput.permissionDecisionReason == \"issue_write is an item of the write plan you approved.\"" >/dev/null' +check "a call from another session gets no decision" 'each_is none s2' +approve "$all_items" s2 +check "a record made in another session allows nothing" 'each_is none' +approve "$(printf '%s' "$all_items" | sed 's/#12/#13/g; s/"example\/widgets Refuse/"example\/other Refuse/')" +check "a record for another issue or repository allows nothing" 'each_is none' +approve '[{"tool":"issue_write","target":"example/widgets Refuse a negative count"}]' +check "a create's record does not allow an update of an issue in that repository" \ + '[ "$(decision issue_write "$create")" = allow ] && [ "$(decision issue_write "$update")" = none ]' +other_title=$(printf '%s' "$create" | jq -c '.title = "Refuse a negative count in by_part"') +feedback='{"method":"create","owner":"example","repo":"widgets","title":"ticket-draft: wrote 900 words for a one-line fix","body":"b","labels":["skill-feedback"]}' +no_title=$(printf '%s' "$create" | jq -c 'del(.title)') +check "a create's record allows that title only: another create, skill-feedback's in this session included, gets no decision" \ + '[ "$(decision issue_write "$other_title")" = none ] && [ "$(decision issue_write "$feedback")" = none ] && [ "$(decision issue_write "$no_title")" = none ]' +approve '[{"tool":"issue_write","target":"example/widgets"}]' +check "a create's target without its title allows no create" '[ "$(decision issue_write "$create")" = none ]' +# The record as issues.md builds it: the items written to a file, read by jq with --slurpfile. +quoted=$(printf '%s' "$create" | jq -c '.title = "Don'"'"'t count a \"negative\" part"') +jq -n --argjson c "$quoted" '[{tool: "issue_write", target: "example/widgets \($c.title)"}]' > .review/github-approval-items.json +jq -n --arg s s1 --argjson t "$(date +%s)" --slurpfile items .review/github-approval-items.json \ + '{session: $s, t: $t, items: $items[0]}' > .review/github-approval.json +check "a title holding an apostrophe and double quotes, recorded from the items file, allows that create" \ + '[ "$(decision issue_write "$quoted")" = allow ] && [ "$(decision issue_write "$create")" = none ]' +newline=$(printf '%s' "$create" | jq -c '.title = "Refuse a negative count\n"') +approve '[{"tool":"issue_write","target":"example/widgets Refuse a negative count"}]' +check "a target is compared byte for byte: a title with a trailing newline does not match one without" \ + '[ "$(decision issue_write "$newline")" = none ] && [ "$(decision issue_write "$create")" = allow ]' +rm -f .review/github-approval-items.json +approve '[{"tool":"add_issue_comment","target":"example/widgets#12"}]' +check "a record allows only the tool it names on that target" \ + '[ "$(decision add_issue_comment "$comment")" = allow ] && [ "$(decision issue_write "$update")" = none ]' +approve '[{"tool":"issue_write","target":"example/widgets#12"}]' +check "an update's record does not allow a comment on the issue" '[ "$(decision add_issue_comment "$comment")" = none ]' +approve "$all_items" +with_parent=$(printf '%s' "$create" | jq -c '. + {parent_issue_number: 3}') +with_fields=$(printf '%s' "$create" | jq -c '. + {issue_fields: [{field_name: "Estimate", value: 3}]}') +check "a create naming a parent or issue fields gets no decision, though the record lists the create" \ + '[ "$(decision issue_write "$with_parent")" = none ] && [ "$(decision issue_write "$with_fields")" = none ]' +extra() { printf '%s' "$1" | jq -c ". + {$2}"; } +check "a create carrying assignees, a milestone or a type gets no decision" \ + '[ "$(decision issue_write "$(extra "$create" "assignees: [\"bob\"]")")" = none ] && [ "$(decision issue_write "$(extra "$create" "milestone: 2")")" = none ] \ + && [ "$(decision issue_write "$(extra "$create" "type: \"Bug\"")")" = none ]' +check "an update carrying assignees, a milestone or a type gets no decision" \ + '[ "$(decision issue_write "$(extra "$update" "assignees: [\"bob\"]")")" = none ] && [ "$(decision issue_write "$(extra "$update" "milestone: 2")")" = none ] \ + && [ "$(decision issue_write "$(extra "$update" "type: null")")" = none ]' +check "a comment carrying assignees, a milestone or a type gets no decision" \ + '[ "$(decision add_issue_comment "$(extra "$comment" "assignees: [\"bob\"]")")" = none ] && [ "$(decision add_issue_comment "$(extra "$comment" "milestone: 2")")" = none ] \ + && [ "$(decision add_issue_comment "$(extra "$comment" "type: \"Bug\"")")" = none ]' +check "a create carrying an update's arguments gets no decision" '[ "$(decision issue_write "$(extra "$create" "state: \"closed\"")")" = none ]' +reaction=$(printf '%s' "$comment" | jq -c 'del(.body) + {reaction: "+1"}') +on_comment=$(printf '%s' "$comment" | jq -c '. + {comment_id: 5}') +check "a reaction gets no decision, though the record lists a comment on the issue" \ + '[ "$(decision add_issue_comment "$reaction")" = none ] && [ "$(decision add_issue_comment "$on_comment")" = none ]' +no_method=$(printf '%s' "$create" | jq -c 'del(.method)') +other_method=$(printf '%s' "$create" | jq -c '.method = "delete"') +no_number=$(printf '%s' "$update" | jq -c 'del(.issue_number)') +text_number=$(printf '%s' "$update" | jq -c '.issue_number = "12"') +part_number=$(printf '%s' "$update" | jq -c '.issue_number = 12.5') +no_owner=$(printf '%s' "$comment" | jq -c 'del(.owner)') +check "a call naming no method, another method, no repository or no whole number gets no decision" \ + '[ "$(decision issue_write "$no_method")" = none ] && [ "$(decision issue_write "$other_method")" = none ] \ + && [ "$(decision issue_write "$no_number")" = none ] && [ "$(decision issue_write "$text_number")" = none ] \ + && [ "$(decision issue_write "$part_number")" = none ] && [ "$(decision issue_write "\"x\"")" = none ] \ + && [ "$(decision add_issue_comment "$no_owner")" = none ]' +no_session() { printf '%s' "{\"tool_name\":\"mcp__gh__issue_write\",\"tool_input\":$create}" | "$gate"; } +check "a call with no session id gets no decision" '[ -z "$(no_session)" ]' +approve "$all_items" s1 $(($(date +%s) - 1800)) +check "a record 30 minutes old allows nothing" 'each_is none' +approve "$all_items" s1 $(($(date +%s) - 1790)) +check "a record just under 30 minutes old still allows" 'each_is allow' +approve "$all_items" s1 $(($(date +%s) + 600)) +check "a record dated in the future allows nothing" 'each_is none' +git init -q . && mkdir -p sub/dir && approve "$all_items" +check "a call from a subdirectory reads the record at the repository's top level" '(cd sub/dir && each_is allow)' +rm -rf .git sub +malformed() { + printf '%s' "$1" > .review/github-approval.json + [ "$(decision add_issue_comment "$comment")" = none ] +} +now=$(date +%s) +check "a malformed record allows nothing" \ + 'malformed "not json" && malformed "[]" && malformed "{\"session\":\"s1\",\"t\":$now,\"items\":{}}" \ + && malformed "{\"session\":\"s1\",\"t\":\"$now\",\"items\":[{\"tool\":\"add_issue_comment\",\"target\":\"example/widgets#12\"}]}" \ + && malformed "{\"session\":\"s1\",\"items\":[{\"tool\":\"add_issue_comment\",\"target\":\"example/widgets#12\"}]}" \ + && malformed "{\"session\":\"s1\",\"t\":$now,\"items\":[\"add_issue_comment example/widgets#12\"]}" \ + && malformed "{\"session\":\"s1\",\"t\":$now,\"items\":[{\"tool\":\"add_issue_comment\",\"target\":\"example/widgets#12\"},\"junk\"]}" \ + && malformed "{\"session\":\"s2\",\"t\":$now,\"items\":[]} {\"session\":\"s1\",\"t\":$now,\"items\":[{\"tool\":\"add_issue_comment\",\"target\":\"example/widgets#12\"}]}"' + +approve "$all_items" +agent() { jq -cn --arg t "mcp__github-example__$1" --argjson i "$2" '{tool_name:$t, tool_input:$i, session_id:"s1", agent_id:"a1", agent_type:"lasp-dev:source-reader"}' | "$gate"; } +check "a call from a subagent gets no decision, even one the record lists" \ + '[ -z "$(agent issue_write "$create")" ] && [ -z "$(agent issue_write "$update")" ] && [ -z "$(agent add_issue_comment "$comment")" ]' +check "another GitHub write gets no decision" '[ "$(decision sub_issue_write "$update")" = none ] && [ "$(decision update_issue_comment "$comment")" = none ]' +check "input with no tool name, or not JSON, gets no decision and exits 0" \ + '[ -z "$(echo "{}" | "$gate")" ] && [ -z "$(echo "not json" | "$gate")" ]' +nojq=$(mktemp -d) && ln -s "$(command -v bash)" "$nojq/bash" +check "without jq a listed call gets no decision and exits 0" \ + 'out=$(jq -cn --argjson i "$create" "{tool_name:\"mcp__gh__issue_write\", tool_input:\$i, session_id:\"s1\"}" | PATH="$nojq" "$gate") && [ -z "$out" ]' +rm -rf "$nojq" +rm -f .review/github-approval.json + +matcher=$(jq -r '.hooks.PreToolUse[] | select(any(.hooks[]; .command | test("issue-gate"))) | .matcher' "$root/hooks/hooks.json") +check "hooks.json sends the gate issue_write and add_issue_comment under any server name" \ + '[ -n "$matcher" ] && (for t in mcp__github__issue_write mcp__github-example__add_issue_comment; do printf "%s" "$t" | grep -qE "^($matcher)$" || exit 1; done)' +check "hooks.json sends it no other tool" \ + '(for t in mcp__github__sub_issue_write mcp__github__update_issue_comment mcp__github__issue_read mcp__atlassian__jira_add_comment Bash; do ! printf "%s" "$t" | grep -qE "^($matcher)$" || exit 1; done)' + +[ "$failures" = 0 ] || { echo "$failures failed"; exit 1; } +echo "all passed" diff --git a/tests/test_next_phase.py b/tests/test_next_phase.py index b133691..5543981 100644 --- a/tests/test_next_phase.py +++ b/tests/test_next_phase.py @@ -577,6 +577,21 @@ def test_a_drafted_ticket_not_filed_stops_and_names_the_file_to_file(self): ], ) + def test_a_drafted_issue_not_filed_names_its_issue_paste_file(self): + self.github(self.LABELS) + rc, out = self.run_it(self.fields(context="", ticket="", slug="retry-policy")) + self.assertEqual(rc, 0, out) + self.assertIn("\nNothing: file `.review/context/retry-policy.issue.md` in the tracker, or have `/lasp-dev:ticket-draft retry-policy` file it; ", out) + + def test_a_drafted_ticket_not_filed_under_a_standard_that_does_not_parse_exits_1(self): + self.github(self.LABELS) + Path("standards/profile.yaml").write_text("issue_tracker:\n kind: gitlab\n") + err = io.StringIO() + with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(err): + rc = next_phase.main(["--fields", str(self.fields(context="", ticket="", slug="retry-policy"))]) + self.assertEqual(rc, 1) + self.assertIn("the tracker's statuses could not be read", err.getvalue()) + def test_a_slug_beside_a_ticket_or_a_context_page_is_refused(self): self.assertEqual(self.run_it(self.fields(context="", slug="retry-policy"))[0], 1) self.assertEqual(self.run_it(self.fields(ticket="", slug="retry-policy"))[0], 1) diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index 754f3bb..5b13bbb 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -147,7 +147,7 @@ def serve(self, *requests: dict) -> list[dict]: return [json.loads(line) for line in out.splitlines()] def test_every_scenario_is_well_formed_and_copies_fixtures_the_pack_holds(self): - self.assertEqual(len(self.known), 5) + self.assertEqual(len(self.known), 6) well_formed(self, self.known, {"widgets"}) for s in self.known.values(): self.assertTrue(s["id"].startswith("tracker-"), s["id"]) @@ -228,11 +228,16 @@ def test_the_pack_plants_no_credentials(self): @unittest.skipIf(run is None, "sandbox/run.py needs PyYAML") class TrackerGithubPack(unittest.TestCase): - """sandbox/tracker-github/: scenarios against a GitHub issue tracker the pack's read-only stub serves.""" + """sandbox/tracker-github/: scenarios against a GitHub issue tracker the pack's stub serves, + recording the writes it is sent.""" PACK = ROOT / "sandbox" / "tracker-github" / "scenarios.yaml" STUB = ROOT / "sandbox" / "tracker-github" / "fixture" / "github-mcp-server-stub.py" - SERVER = {"command": "python3", "args": ["-I", "${SBX_FIXTURES}/github-mcp-server-stub.py"]} + SERVER = {"command": "python3", "args": ["-I", "${SBX_FIXTURES}/github-mcp-server-stub.py", "--writes", ".git/github-writes.jsonl"]} + WORK = {"tracker-gh-work-unlabeled", "tracker-gh-work-labeled", "tracker-gh-work-closed", "tracker-gh-work-branch", + "tracker-gh-work-pr", "tracker-gh-work-closed-labeled"} + DRAFT = {"tracker-gh-ticket-draft-new", "tracker-gh-ticket-draft-refine", "tracker-gh-ticket-draft-declined", "tracker-gh-ticket-draft-blanket", + "tracker-gh-ticket-draft-standing-answer"} def setUp(self): patch = mock.patch.object(run, "SCENARIOS", self.PACK) @@ -240,9 +245,10 @@ def setUp(self): self.addCleanup(patch.stop) self.known = run.scenarios() - def serve(self, *requests: dict) -> list[dict]: + def serve(self, *requests: dict, writes: "Path | None" = None) -> list[dict]: lines = "".join(json.dumps({"jsonrpc": "2.0", **r}) + "\n" for r in requests) - out = subprocess.run([sys.executable, "-I", str(self.STUB)], input=lines, capture_output=True, text=True, check=True).stdout + extra = ["--writes", str(writes)] if writes else [] + out = subprocess.run([sys.executable, "-I", str(self.STUB), *extra], input=lines, capture_output=True, text=True, check=True).stdout return [json.loads(line) for line in out.splitlines()] def call(self, name: str, **arguments) -> dict: @@ -255,31 +261,108 @@ def read(self, name: str, **arguments) -> dict: return json.loads(result["content"][0]["text"]) def test_every_scenario_is_well_formed_and_copies_fixtures_the_pack_holds(self): - self.assertEqual(set(self.known), {"tracker-gh-work-unlabeled", "tracker-gh-work-labeled", "tracker-gh-work-closed", "tracker-gh-work-branch", "tracker-gh-work-pr", "tracker-gh-work-closed-labeled"}) + self.assertEqual(set(self.known), self.WORK | self.DRAFT) well_formed(self, self.known, {"widgets"}) for s in self.known.values(): - self.assertEqual(s["expect"]["mcp"], ["mcp__github__issue_read"], s["id"]) + if s["id"] in self.WORK: + self.assertEqual(s["expect"]["mcp"], ["mcp__github__issue_read"], s["id"]) + self.assertIn("rm -f .git/github-writes.jsonl", s["setup"], s["id"]) for name in re.findall(r'"\$SBX_FIXTURES/([^"]+)"', " ".join(s.get("setup", []))): self.assertTrue((run.fixture_dir() / name).is_file(), (s["id"], name)) + for name in self.DRAFT: + self.assertTrue(any("github-writes.jsonl" in v for v in self.known[name]["verify"]), name) + for name in ("tracker-gh-ticket-draft-declined", "tracker-gh-ticket-draft-blanket", "tracker-gh-ticket-draft-standing-answer"): + self.assertEqual(self.known[name]["must_not_try"], ["mcp__github__issue_write", "mcp__github__add_issue_comment"], name) - def test_the_documented_server_gets_the_github_rules_and_no_write(self): - self.assertIn(json.dumps(self.SERVER["args"][1]), self.PACK.read_text()) - self.assertEqual(run.server_kind(self.SERVER), "github") + def perms(self, servers: dict) -> dict: sandbox = Path(tempfile.mkdtemp()) self.addCleanup(shutil.rmtree, sandbox) - (sandbox / "mcp.json").write_text(json.dumps({"mcpServers": {"github": self.SERVER}})) + (sandbox / "mcp.json").write_text(json.dumps({"mcpServers": servers})) run.write_settings(sandbox, ROOT) - perms = json.loads((sandbox / "settings.json").read_text())["permissions"] + return json.loads((sandbox / "settings.json").read_text())["permissions"] + + def test_the_documented_server_gets_the_github_rules_with_its_issue_writes_left_to_the_hook(self): + self.assertIn(json.dumps(self.SERVER["args"])[1:-1], self.PACK.read_text()) + self.assertEqual(run.server_kind(self.SERVER), "github") + perms = self.perms({"github": self.SERVER}) self.assertLessEqual({"mcp__github__issue_read", "mcp__github__search_pull_requests", "mcp__github__get_me"}, set(perms["allow"])) - self.assertIn("mcp__github__issue_write", perms["deny"]) + for tool in ("issue_write", "add_issue_comment"): + self.assertNotIn(f"mcp__github__{tool}", perms["allow"] + perms["deny"]) + self.assertLessEqual({"mcp__github__sub_issue_write", "mcp__github__update_issue_comment", "mcp__github__create_pull_request"}, set(perms["deny"])) + + def test_a_server_that_is_not_a_packs_writing_stub_keeps_every_write_denied(self): + read_only = {"command": "python3", "args": ["-I", "${SBX_FIXTURES}/github-mcp-server-stub.py"]} + elsewhere = {"command": "docker", "args": ["run", "ghcr.io/github/github-mcp-server", "--writes", "x"]} + mounted = {"command": "docker", "args": ["run", "-v", "${SBX_FIXTURES}/github-mcp-server-stub.py:/stub.py", "ghcr.io/github/github-mcp-server", "--writes", "x"]} + other_file = {"command": "python3", "args": ["-I", "${SBX_FIXTURES}/other-github-mcp-server.py", "--writes", "x"]} + with_url = {**self.SERVER, "url": "https://api.githubcopilot.com/mcp/"} + perms = self.perms({"stub": read_only, "real": elsewhere, "mounted": mounted, "other": other_file, "url": with_url, "github": self.SERVER}) + for name in ("stub", "real", "mounted", "other", "url"): + self.assertLessEqual({f"mcp__{name}__issue_write", f"mcp__{name}__add_issue_comment"}, set(perms["deny"]), name) + self.assertNotIn("mcp__github__issue_write", perms["deny"]) def test_the_stub_lists_github_mcp_servers_read_tools_and_no_write(self): (listed,) = self.serve({"id": 1, "method": "tools/list"}) tools = {t["name"]: t["inputSchema"] for t in listed["result"]["tools"]} - self.assertEqual(set(tools), {"get_me", "issue_read", "list_issues", "search_issues", "search_pull_requests"}) + self.assertEqual(set(tools), {"get_me", "get_label", "issue_read", "list_issues", "search_issues", "search_pull_requests"}) self.assertEqual(tools["issue_read"]["required"], ["method", "owner", "repo", "issue_number"]) self.assertEqual(tools["search_pull_requests"]["required"], ["query"]) + def test_with_writes_the_stub_lists_the_issue_writes_and_records_each_call(self): + writes = Path(tempfile.mkdtemp()) / "writes.jsonl" + self.addCleanup(shutil.rmtree, writes.parent) + (listed,) = self.serve({"id": 1, "method": "tools/list"}, writes=writes) + tools = {t["name"]: t["inputSchema"] for t in listed["result"]["tools"]} + self.assertLessEqual({"issue_write", "add_issue_comment"}, set(tools)) + self.assertEqual(tools["issue_write"]["required"], ["method", "owner", "repo"]) + repo = {"owner": "example", "repo": "widgets"} + data = json.loads((self.STUB.parent / "github.json").read_text()) + new = max(i["number"] for i in data["issues"] + data["pulls"]) + 1 + calls = [{"id": 1, "method": "tools/call", "params": {"name": "issue_write", "arguments": {"method": "create", **repo, "title": "T", "body": "B"}}}, + {"id": 2, "method": "tools/call", "params": {"name": "add_issue_comment", "arguments": {**repo, "issue_number": new, "body": "C"}}}, + {"id": 3, "method": "tools/call", "params": {"name": "issue_write", "arguments": {"method": "update", **repo, "issue_number": 12, "labels": ["ready"]}}}] + made, commented, updated = (json.loads(r["result"]["content"][0]["text"]) for r in self.serve(*calls, writes=writes)) + self.assertEqual(made["url"], f"https://github.com/example/widgets/issues/{new}") + self.assertEqual(commented["url"].split("#")[0], made["url"]) + self.assertEqual(updated["url"], "https://github.com/example/widgets/issues/12") + self.assertEqual([json.loads(line)["tool"] for line in writes.read_text().splitlines()], ["issue_write", "add_issue_comment", "issue_write"]) + # A later server replays the file, so the next turn reads what this one wrote. + replies = self.serve({"id": 1, "method": "tools/call", "params": {"name": "issue_read", "arguments": {"method": "get", **repo, "issue_number": new}}}, + {"id": 2, "method": "tools/call", "params": {"name": "issue_read", "arguments": {"method": "get_comments", **repo, "issue_number": new}}}, + {"id": 3, "method": "tools/call", "params": {"name": "issue_read", "arguments": {"method": "get", **repo, "issue_number": 12}}}, + writes=writes) + issue, comments, twelve = (json.loads(r["result"]["content"][0]["text"]) for r in replies) + self.assertEqual((issue["issue"]["title"], issue["issue"]["body"], issue["issue"]["state"]), ("T", "B", "open")) + self.assertEqual([c["body"] for c in comments["comments"]], ["C"]) + self.assertEqual(twelve["issue"]["labels"], ["ready"]) + + def test_a_label_a_write_sent_reads_back_by_name_and_get_label_knows_only_the_fixtures(self): + writes = Path(tempfile.mkdtemp()) / "writes.jsonl" + self.addCleanup(shutil.rmtree, writes.parent) + repo = {"owner": "example", "repo": "widgets"} + self.serve({"id": 1, "method": "tools/call", "params": {"name": "issue_write", "arguments": {"method": "update", **repo, "issue_number": 12, "labels": ["new-one"]}}}, + writes=writes) + (labels,) = self.serve({"id": 1, "method": "tools/call", "params": {"name": "issue_read", "arguments": {"method": "get_labels", **repo, "issue_number": 12}}}, + writes=writes) + self.assertEqual(json.loads(labels["result"]["content"][0]["text"])["labels"], [{"name": "new-one"}]) + known, unknown = (r["result"] for r in self.serve( + *({"id": n, "method": "tools/call", "params": {"name": "get_label", "arguments": {**repo, "name": label}}} for n, label in ((1, "ready"), (2, "new-one"))))) + self.assertEqual(json.loads(known["content"][0]["text"])["name"], "ready") + self.assertTrue(unknown["isError"]) + + def test_a_write_the_stub_cannot_apply_is_an_error_and_not_recorded(self): + writes = Path(tempfile.mkdtemp()) / "writes.jsonl" + self.addCleanup(shutil.rmtree, writes.parent) + repo = {"owner": "example", "repo": "widgets"} + bad = [{"method": "update", **repo, "issue_number": 99, "body": "x"}, {"method": "create", **repo}, {"method": "delete", **repo}] + replies = self.serve(*({"id": n, "method": "tools/call", "params": {"name": "issue_write", "arguments": a}} for n, a in enumerate(bad, 1)), + {"id": 9, "method": "tools/call", "params": {"name": "add_issue_comment", "arguments": {**repo, "issue_number": 99, "body": "x"}}}, + writes=writes) + self.assertTrue(all(r["result"]["isError"] for r in replies), replies) + self.assertFalse(writes.exists()) + (refused,) = self.serve({"id": 1, "method": "tools/call", "params": {"name": "issue_write", "arguments": {"method": "create", **repo, "title": "T"}}}) + self.assertTrue(refused["result"]["isError"]) + def test_an_issue_reads_as_the_server_gives_it(self): got = self.read("issue_read", method="get", issue_number=14) self.assertEqual(got["method"], "get")