diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000..9bd8db7 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,108 @@ +name: Bug report +description: A command behaves differently from how it is documented or intended. +labels: ["bug", "needs-triage"] +body: + - type: markdown + attributes: + value: | + Every issue carries three things: a **reason** it matters, a **scope** + it is bounded to, and a **plan** for what happens next. All three are + required. The plan is your proposal - triage may replace it, and will + say so in a comment before accepting. + + Wait for the `accepted` label before writing a fix. See + [ISSUES.md](https://github.com/KeeperHub/cli/blob/main/ISSUES.md). + + - type: checkboxes + id: preflight + attributes: + label: Before filing + options: + - label: I searched open and closed issues for this behaviour. + required: true + - label: I am on the latest release, or I checked that `main` still has it. + required: true + - label: This is one problem, not several. (Several means several issues.) + required: true + - label: This is not a security vulnerability (those go through private reporting). + required: true + + - type: textarea + id: reproduction + attributes: + label: "Reason: command and output" + description: >- + The exact command and its full output. A pasted terminal session settles + a report faster than any description of one. Redact keys and addresses + you do not want public, but keep the shape of what was redacted. + render: shell + placeholder: | + $ kh execute status exec_123 --watch --timeout 5s + ... + validations: + required: true + + - type: textarea + id: expected + attributes: + label: "Reason: what you expected, and what told you to expect it" + description: >- + Name the source - `--help` output, a docs page, a flag name, an exit + code convention. If the source says something different from the code, + that immediately tells us which of the two is wrong. + validations: + required: true + + - type: textarea + id: cost + attributes: + label: "Reason: what it costs" + description: >- + What goes wrong for someone who hits this. A wrong exit code that breaks + a CI gate, a command that hangs forever, output a script cannot parse. + This is what orders the queue. + validations: + required: true + + - type: input + id: version + attributes: + label: kh version + description: Output of `kh version`. + validations: + required: true + + - type: input + id: platform + attributes: + label: OS and architecture + placeholder: macOS 15 arm64 / Ubuntu 24.04 amd64 / Windows 11 amd64 + validations: + required: true + + - type: textarea + id: scope + attributes: + label: "Scope: what this covers, and what it does not" + description: >- + The commands affected, and the ones you checked and found fine. If a + sibling command plausibly shares the fault, name it - a fix applied to + one command and not its siblings is a recurring failure. Then confirm + this is one problem. + placeholder: >- + Affects `kh execute status --watch`. Checked `kh execute transfer --wait` + - same shape, likely the same fault. `kh workflow list` unaffected. + validations: + required: true + + - type: textarea + id: plan + attributes: + label: "Plan: what should happen next" + description: >- + Your proposal, which triage may replace. Say whether it changes anything + a caller depends on - output format, an exit code, a flag's meaning. If + you do not know the fix, say what you would need to determine to choose + one. Blank is not a valid answer. + validations: + required: true diff --git a/.github/ISSUE_TEMPLATE/change_request.yml b/.github/ISSUE_TEMPLATE/change_request.yml new file mode 100644 index 0000000..c9b16f7 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/change_request.yml @@ -0,0 +1,96 @@ +name: Behaviour change or new command +description: Propose something kh should do that it does not do today. +labels: ["enhancement", "needs-triage"] +body: + - type: markdown + attributes: + value: | + Every issue carries three things: a **reason** it matters, a **scope** + it is bounded to, and a **plan** for what happens next. All three are + required. The plan is your proposal - triage may replace it, and will + say so in a comment before accepting. + + Lead with what you are trying to do, not with what to build. + + Wait for the `accepted` label before writing code. See + [ISSUES.md](https://github.com/KeeperHub/cli/blob/main/ISSUES.md). + + - type: checkboxes + id: preflight + attributes: + label: Before filing + options: + - label: I searched open and closed issues for this proposal. + required: true + - label: I checked `kh --help` and the command reference. + required: true + - label: This is one change, not several. (Several means several issues.) + required: true + + - type: textarea + id: problem + attributes: + label: "Reason: what you cannot do today" + description: >- + The concrete task that is blocked or awkward, and what you do instead. + If a flag nearly does it, say what it falls short on. + validations: + required: true + + - type: textarea + id: cost + attributes: + label: "Reason: what the workaround costs" + description: >- + Time, reliability, an invariant you cannot hold, a script you cannot + write. That cost is the size of the problem. If there is no workaround, + say so. + validations: + required: true + + - type: textarea + id: scope + attributes: + label: "Scope: what this touches, and what it does not" + description: >- + The commands this changes and the ones it deliberately leaves alone. + Then confirm it is one change: if any part could ship and be correct with + another reverted, those are separate issues. + validations: + required: true + + - type: textarea + id: plan + attributes: + label: "Plan: what you propose" + description: >- + The command or flag as you would build it, including output shape and + exit codes, and what happens to callers of the current behaviour. Triage + may replace this and will say so before accepting. + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: "Plan: alternatives you considered" + description: >- + Including doing nothing, and approaches you rejected with the reason. A + rejected alternative with a reason stops triage re-deriving it. + validations: + required: false + + - type: checkboxes + id: breaking + attributes: + label: "Scope: compatibility" + description: Tick anything the plan would touch. Each one turns this into a decision rather than a fix. + options: + - label: Changes existing output, an exit code, or a flag's meaning. + required: false + - label: Changes a default for callers who pass no flags. + required: false + - label: Adds or upgrades a Go module dependency. + required: false + - label: Touches authentication or credential storage. + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..726c7ae --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,11 @@ +blank_issues_enabled: false +contact_links: + - name: Security vulnerability + url: https://github.com/KeeperHub/cli/security + about: Report privately. Never in a public issue or pull request. + - name: Command reference + url: https://docs.keeperhub.com/cli + about: Check the docs before filing - the behaviour may be intentional and described. + - name: Contribution policy + url: https://github.com/KeeperHub/cli/blob/main/ISSUES.md + about: What needs an issue, what does not, and what happens after you file one. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..826d9b7 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,44 @@ + + +## Issue + +Closes # + + + +## What this changes + + + +## Scope + + + +## How it was verified + + + +--- + +- [ ] Targets `main` +- [ ] Title carries the issue number, or an exemption applies +- [ ] `make lint` and `make test` pass +- [ ] `go generate ./docs/` run and committed, if a command or flag changed diff --git a/.github/workflows/pr-issue-link.yml b/.github/workflows/pr-issue-link.yml new file mode 100644 index 0000000..e1393e4 --- /dev/null +++ b/.github/workflows/pr-issue-link.yml @@ -0,0 +1,104 @@ +name: PR Issue Link + +# Runs on pull_request_target so it also runs on fork pull requests, which are +# the ones this gate exists for. Fork runs of `pull_request` need per-run +# maintainer approval, which would leave the gate silent exactly where it is +# needed. +# +# SAFETY: pull_request_target runs with the base repository's token. This job +# therefore never checks out, builds, or executes pull request code. It reads +# the title, the labels, and the referenced issue through the API and nothing +# else. Do not add a checkout step to this file. + +on: + pull_request_target: + types: [opened, edited, reopened, labeled, unlabeled] + branches: + - main + +permissions: + contents: read + issues: read + pull-requests: read + +jobs: + check-issue-link: + runs-on: ubuntu-latest + + steps: + - name: Require an accepted issue in the PR title + env: + # Untrusted input. Passed through the environment and never + # interpolated into the script body. + PR_TITLE: ${{ github.event.pull_request.title }} + PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} + REPO: ${{ github.repository }} + GH_TOKEN: ${{ github.token }} + EXEMPT_LABEL: no-issue-required + ACCEPTED_LABEL: accepted + # Types that never require an issue. Keep in step with the + # "not required" list in ISSUES.md. + EXEMPT_TYPES: docs chore style + run: | + set -euo pipefail + + fail() { + echo "----------------------------------------------" + echo " ERROR: $1" + echo "----------------------------------------------" + echo "" + echo " Got title: $PR_TITLE" + echo "" + echo " KeeperHub takes issues before pull requests. Open an issue," + echo " wait for a maintainer to apply the '$ACCEPTED_LABEL' label," + echo " then reference it in this PR's title:" + echo "" + echo " feat: #97 add --require-verified to execute status" + echo " fix(execute): #98 bound a hung request under --watch" + echo "" + echo " No issue needed for typos, help-text wording, or docs that" + echo " match existing behaviour. Retitle as one of:" + echo " $EXEMPT_TYPES" + echo "" + echo " Full policy: https://github.com/$REPO/blob/main/ISSUES.md" + echo "" + echo " Already labelled '$ACCEPTED_LABEL'? This check does not rerun" + echo " by itself when the issue changes - re-run the job, or edit" + echo " the PR title to retrigger it." + exit 1 + } + + if printf '%s' "$PR_LABELS" | grep -qF "\"$EXEMPT_LABEL\""; then + echo "Exempt: pull request carries the '$EXEMPT_LABEL' label." + exit 0 + fi + + pr_type=$(printf '%s' "$PR_TITLE" | sed -nE 's/^([a-zA-Z]+)(\([^)]*\))?!?:.*/\1/p' | tr '[:upper:]' '[:lower:]') + for exempt in $EXEMPT_TYPES; do + if [ "$pr_type" = "$exempt" ]; then + echo "Exempt: '$pr_type' changes do not require an issue." + exit 0 + fi + done + + issue_number=$(printf '%s' "$PR_TITLE" | grep -oE '#[0-9]+' | head -n1 | tr -d '#') + if [ -z "$issue_number" ]; then + fail "PR title carries no issue reference." + fi + + if ! issue_json=$(gh api "repos/$REPO/issues/$issue_number" 2>/dev/null); then + fail "#$issue_number does not resolve to an issue in $REPO." + fi + + if printf '%s' "$issue_json" | jq -e '.pull_request' >/dev/null 2>&1; then + fail "#$issue_number is a pull request, not an issue." + fi + + if ! printf '%s' "$issue_json" | jq -e --arg l "$ACCEPTED_LABEL" \ + '.labels | map(.name) | index($l)' >/dev/null 2>&1; then + state=$(printf '%s' "$issue_json" | jq -r '.state') + labels=$(printf '%s' "$issue_json" | jq -r '[.labels[].name] | join(", ")') + fail "#$issue_number is not marked '$ACCEPTED_LABEL' (state: $state; labels: ${labels:-none})." + fi + + echo "PR title references accepted issue #$issue_number." diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..d8e7dd2 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,80 @@ +# Contributing to the KeeperHub CLI + +`kh` is the KeeperHub command-line interface. Go, distributed through Homebrew +and `go install`. + +## Start with an issue + +Anything that changes behaviour needs an issue first, accepted by a maintainer, +before the pull request. **[ISSUES.md](ISSUES.md) is the policy** - what needs an +issue, what goes straight to a pull request, and what happens after you file one. + +The short version: open an issue, wait for the `accepted` label, then reference +it in your pull request title (`feat: #97 description`). Typos, help-text +wording, and docs matching existing behaviour skip all of that. + +## Development setup + +Go 1.25+ (`go.mod` pins the exact version CI uses) and `golangci-lint` for the +lint target. + +```bash +make build # builds bin/kh with version metadata +make test # go test -race ./... +make lint # golangci-lint run ./... +make install # go install into your GOPATH +make clean +``` + +`make build` and `make install` run `sync-version` first, which copies +`.release-please-manifest.json` into `internal/version/`. Build with plain +`go build` and the binary reports the wrong version. + +## Generated command docs + +`docs/` is generated from the cobra command tree, and CI fails on drift: + +```bash +go generate ./docs/ +``` + +Run it after any change to a command, a flag, or its help text, and commit the +result. The `docs-check` job runs `git diff --exit-code docs/` and turns red +otherwise. + +## Tests + +```bash +go test -race ./... # unit tests, what CI runs +go test -tags integration ./tests/integration/ # integration, needs credentials +``` + +Integration tests are behind the `integration` build tag and need +`KH_TEST_HOST`, `KH_TEST_EMAIL`, `KH_TEST_PASSWORD` and `KH_API_KEY`. CI only +runs them on push, not on pull requests, so they will not run on your fork. +Unit-test any behaviour you change - a command test alongside the changed +command is the expectation. + +Timeouts and deadlines deserve a specific note: a test that only exercises a +server responding promptly does not cover a request that hangs. If you add a +deadline, test a handler that blocks past it. + +## Pull requests + +1. **Title**: `: # `, for example + `feat: #97 add --require-verified to execute status`. The type prefix drives + release-please, so keep it accurate. Types: `feat`, `fix`, `chore`, `docs`, + `refactor`, `test`, `ci`, `build`, `perf`, `style`. +2. **Base branch**: `main`. This repo has no `staging` branch. +3. **Scope**: one change per pull request. If a part of it could ship and be + correct with the rest reverted, split it. +4. **Before submitting**: `make lint`, `make test`, and `go generate ./docs/` + all clean, and the backing issue carries `accepted`. + +Breaking changes use `!` (`feat!:`) or a `BREAKING CHANGE:` trailer, which +release-please turns into a major version. + +## Related + +- [ISSUES.md](ISSUES.md) - the issue-first policy +- [docs.keeperhub.com/cli](https://docs.keeperhub.com/cli) - command reference diff --git a/ISSUES.md b/ISSUES.md new file mode 100644 index 0000000..8eaa10a --- /dev/null +++ b/ISSUES.md @@ -0,0 +1,123 @@ +# Issues before pull requests + +Open an issue before you write code. We answer it, and once the problem and the +shape of the fix are agreed, the pull request is a short step rather than a +negotiation. + +## When an issue is required + +**Required** for anything that changes behaviour: + +- Command behaviour, output format, or exit codes +- New commands, flags, or defaults +- Anything that changes what an existing flag does +- Go module dependencies added, removed, or upgraded +- CI, release, or build configuration +- Authentication and credential handling + +**Not required** - open a pull request directly: + +- Typos, broken links, and formatting +- Help text and error-message wording that corrects an existing statement +- Documentation that matches what the code already does + +If you are unsure, open the issue. + +## Reason, scope, plan + +Every issue carries three things. An issue missing any of them cannot be +answered, only discussed, and discussion is what this policy exists to replace. + +**Reason** - why it matters, in evidence. For a bug: the exact command, its full +output, what you expected, *what told you to expect it*, and what it costs +someone who hits it. Name the source of the expectation - help text, a docs +page, a flag name - because that tells us straight away whether the code is +wrong or the source is. + +**Scope** - what this covers and what it does not. Which commands you checked +and found fine. If the same fault plausibly affects sibling commands, say which. +Then confirm it is one issue: if any part could be fixed and shipped while +another stays broken, those are separate issues. + +**Plan** - what should happen next. Your proposal, not a commitment we have +made; triage may replace it. If you do not know the fix, *"I do not know; here +is what I would need to determine"* is a complete plan. Blank is not - a problem +with no proposed next step puts the whole cost of thinking on whoever reads it. + +### Already filed an issue + +Nothing here applies retroactively. Issues filed before this page existed are +triaged on what they contain, and you will never be asked to resubmit one to +match a template that did not exist when you wrote it. + +More generally, and for new issues too: **you will not be asked to restate +something you have already said.** If triage needs one more fact, it asks for +that fact, on your issue. + +If an issue turns out to hold several problems, we split it and credit you on +each part. + +## What happens to your issue + +| Label | Meaning | +|---|---| +| `needs-triage` | Received, not yet read. Applied automatically. | +| `confirmed` | Someone reproduced it. Says nothing yet about whether we will fix it. | +| `accepted` | Reason, scope and plan all stand. Write the pull request. | +| `needs-discussion` | Real, but the scope or the plan is not settled. Do not start yet. | +| `wontfix` / `duplicate` / `invalid` | Closed, with the reason in a comment. | + +**`accepted` is the signal to start.** It is what the pull request gate checks +for. Nothing else means "go" - `confirmed` in particular does not. + +**`accepted` accepts a specific plan.** If triage keeps your reason and scope but +replaces your plan, it says so in a comment before applying the label, and that +comment is the plan. Build against it, not the one you filed. + +We aim to triage within two working days. If an issue has sat longer, comment on +it - that is the fastest way to get it moving. + +## Should this be one change + +Apply this to each seam in what you are proposing: + +> Can piece A ship, deploy, and be correct with piece B absent or reverted? + +If yes for every pair, they are separate issues and separate pull requests. If +the pieces are only correct together, they are one unit regardless of size. + +A worked example from this repo: a pull request added receipt rendering with +`--require-verified` **and** a `--timeout` deadline for `--watch`. Neither needed +the other to be correct, and the timeout changed behaviour for every existing +`--watch` caller. Two issues, two pull requests. + +## Opening the pull request + +Once your issue carries `accepted`: + +1. **Reference the issue in the pull request title**, after the conventional + commit type: + + ``` + feat: #97 add --require-verified to execute status + fix(execute): #98 bound a hung request under --watch --timeout + ``` + + The type prefix drives release-please, so keep it accurate. + +2. Fill in the pull request template. + +3. Target `main`. This repo has no `staging` branch. + +`docs`, `chore`, and `style` pull requests are exempt from the issue check +automatically. A maintainer can apply `no-issue-required` to exempt anything else. + +## Security + +Do not open an issue for a vulnerability. Report it privately through +[GitHub Private Vulnerability Reporting](https://github.com/KeeperHub/cli/security). + +## Related + +- [CONTRIBUTING.md](CONTRIBUTING.md) - setup, build, test, release +- [docs.keeperhub.com/cli](https://docs.keeperhub.com/cli) - command reference