Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ extension, which has no way to turn the mode on.
CodeQL) for security problems. What fails a pull request, the licenses a new dependency
may have, and how to accept a false positive are in the
[dependency and static analysis policy](docs/security/dependency-and-static-analysis-policy.md).
- Maintainers review every pull request; how decisions are made and who maintains what is
in [GOVERNANCE.md](GOVERNANCE.md) and [MAINTAINERS.md](MAINTAINERS.md).
- Maintainers review every pull request against [docs/CODE_REVIEW.md](docs/CODE_REVIEW.md):
what a reviewer checks and what a change needs before it merges. How decisions are made
and who maintains what is in [GOVERNANCE.md](GOVERNANCE.md) and
[MAINTAINERS.md](MAINTAINERS.md).
- Commit messages and pull request titles follow
[Conventional Commits](https://www.conventionalcommits.org/): `feat(extension): ...`,
`fix(server): ...`. Scopes: `server`, `extension`, `spec`, `docs`, `ci`, `release`.
Expand Down
89 changes: 89 additions & 0 deletions docs/CODE_REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Code review

Last reviewed: 2026-10-06.

How a change to Kotiko is reviewed: who reviews, what they check, and what a change needs
before it merges. It applies to every pull request, from a maintainer, a contributor or an
agent. The rules contributors follow while writing a change are in
[CONTRIBUTING.md](../CONTRIBUTING.md); this page is the reviewer's side.

## How a review happens

1. **CI first.** A reviewer starts once the required checks on `main` are green. They are
listed in the `main` ruleset and include the server and extension test suites, lint,
e2e, the shared spec, the secret, DCO and file checks, the dependency scan and CodeQL.
A red check is the author's to fix, not the reviewer's to wave through.
2. **One reviewer who isn't the author** reads the whole diff on GitHub and leaves a
review: *Approve*, or *Request changes* with each problem as a comment on its line.
While Kotiko has few maintainers, the lead maintainer may merge their own change after
CI passes. GOVERNANCE.md and MAINTAINERS.md say who the maintainers are, and the aim
is that every change gets a second person's review.
3. **Every review thread is resolved** before merging (the ruleset enforces this),
either by a change or by a reply the reviewer accepts.
4. **Merge** with a merge commit. The author or a maintainer merges; nobody bypasses the
ruleset.

## What the reviewer checks

Each item is a question the change must answer yes to, or explain why not.

**It does what it says.**
- The pull request says what changed and why, links its slice or issue, and the diff
matches that description. Nothing unrelated is mixed in.
- The behaviour matches the slice's spec. A deviation is written in the spec's
Implementation notes.

**It is tested.**
- New or changed behaviour has tests in the same pull request. A bug fix has a regression
test that fails without the fix: check it by reading the test against the fix.
- Tests wait for conditions, never fixed sleeps, and never reach the network or a real
local service.
- The coverage gates still pass (90 % of lines, 80 % of the extension's branches).

**It keeps users safe.**
- **Trust boundaries hold.** Content scripts never see keys, tokens or where requests go.
Only Kotiko's own pages change the server or lookup destination. Every message type
says who may send it (`lib/messages.js`).
- **Input is validated** where it enters, against an allowlist, with its limits. The
server's checks are in `docs/security/assurance-case.md` section 6.
- **No HTML from data:** text goes in with `textContent`, never `innerHTML` (ESLint also
enforces this).
- **Privacy:** page text never leaves the device, only the word goes to Wiktionary, and
every new network call, host or permission is in `docs/privacy/inventory.md` (a test
checks this).
- No secret or user text reaches a log above debug level.
- A change to authentication, a trust boundary or an input updates the
[assurance case](security/assurance-case.md) and, if users can tell, the
[security requirements](security/requirements.md).
- A new dependency meets the license rule and the
[dependency policy](security/dependency-and-static-analysis-policy.md). The extension
gets no runtime npm code.

**It works for every learner.**
- Nothing assumes English or one base language. Shared tables are in
`spec/lang/_generic/`, with per-language entries only where the default visibly fails.
- Every UI string is in `extension/_locales/en`.
- Accessibility holds: keyboard, focus and contrast in both themes (the axe and contrast
checks run in CI).
- The popup's size and first-paint budgets pass (`npm run perf`).

**It is complete.**
- Docs say what changed: the reference docs for routes and settings (a test checks
them), the site's guides, and a plain-language `CHANGELOG.md` line under
`## Unreleased`.
- Every commit is signed off (DCO) and follows Conventional Commits.

## The standard of evidence

A claim in a pull request or a review ("fixed", "faster", "not reachable", "no
regression") carries its proof: the test that fails without the change, or the command
that was run and what it printed. "Should work" is not proof. A reviewer who doubts a
claim reruns its proof. Security findings follow the same rule, as in the pre-release
review (slice [54](../slices/54-pre-release-security-review/SPEC.md)).

## Changes written by agents

An agent's change is reviewed like anyone else's. When a lead agent hands work to other
agents, the lead also reads every diff, checks each acceptance criterion against a test,
and reruns every check before opening the pull request. A maintainer still decides
whether it merges.
4 changes: 2 additions & 2 deletions docs/best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ earned, and gold once silver is; the answers can be entered before that.

| Criterion | Level | Status | Answer | Evidence | Needs |
|---|---|---|---|---|---|
| `achieve_silver` | MUST | Unmet | Not yet. Silver is answered after passing is earned. | bestpractices.dev/projects/15259.json: `achieved_silver_at` null | Passing, then the silver Unmet items above |
| `achieve_silver` | MUST | Met | Kotiko earned the silver badge on 2026-10-06: https://www.bestpractices.dev/projects/15259 | bestpractices.dev entry 15259 (silver 100 %) | – |
| `bus_factor` | MUST [URL] | Unmet | Kotiko has two maintainers since 2026-10-06, but every commit so far was written by the lead maintainer, so the bus factor is still 1 in practice. We are working on it: the architecture, the assurance case and the coding standards are written down, every planned piece of work in slices/ is a buildable spec, and GOVERNANCE.md says how the maintainers continue without any one of them. https://github.com/ScriptKittyOS/kotiko/blob/main/docs/governance/continuity.md#bus-factor | `git shortlog -sne HEAD`: one author; `MAINTAINERS.md`; `GOVERNANCE.md` "Continuity" | The second maintainer contributing code and reviews regularly |
| `contributors_unassociated` | MUST [URL] | Unmet | Not yet. All significant contributions in the past year come from one person. Issues, Discussions and translations are open to everyone, and small tasks will be labelled for newcomers. | `git shortlog -sne HEAD`: 134 commits, one person; `gh api repos/ScriptKittyOS/kotiko/contributors` gives HackTuah only | Maintainer: recruit; two unassociated significant contributors in a year |
| `copyright_per_file` | MUST [J] | Met | Every source code file (Elixir, JavaScript, shell, CSS, HTML, Astro, workflow YAML) starts with SPDX-FileCopyrightText: 2026 ScriptKittyOS and the Kotiko contributors. Files that can't carry a comment (JSON, images), plus Markdown, test fixture pages and two formatter configs, get the same statement through REUSE.toml, following the REUSE specification, and reuse lint runs on every pull request as a required check. | Header check over 352 tracked code files (`git ls-files` *.ex *.exs *.js *.mjs *.sh *.css *.html *.astro *.ts *.mdx): all carry SPDX headers except `server/.formatter.exs`, `server/priv/repo/migrations/.formatter.exs`, `site/src/content/docs/index.mdx` and 24 `test/fixtures/pages/*.html`, all covered by `REUSE.toml:4`; `.github/workflows/ci.yml:141` (`pipx run reuse lint`, required check "versions and licenses") | Agent (optional): put in-file headers on the three non-fixture files too |
Expand All @@ -276,7 +276,7 @@ earned, and gold once silver is; the answers can be entered before that.

| Criterion | Level | Status | Answer | Evidence | Needs |
|---|---|---|---|---|---|
| `code_review_standards` | MUST [URL] [N/A J] | Unmet | Partly documented. CONTRIBUTING.md and the pull request template list what every change must have (tests, a regression test for fixes, format and lint clean, docs and the security documents updated, no new runtime dependency in the extension), and main accepts only pull requests whose required checks pass. A code review guide saying how review is conducted, what the reviewer checks and what blocks a merge is planned. | `CONTRIBUTING.md:38-79`; `.github/pull_request_template.md:7-17`; `ls docs` (no CODE_REVIEW.md) | Agent: write `docs/CODE_REVIEW.md` (slice 53 section 8) and link it from CONTRIBUTING.md; answer Met with its URL |
| `code_review_standards` | MUST [URL] [N/A J] | Met | docs/CODE_REVIEW.md documents how code review is done and what is required: CI's required checks pass first; a reviewer who isn't the author reads the whole diff and approves or requests changes line by line; every review thread is resolved before merging; and a checklist of what must be true (does what it says and matches its spec, tested with a regression test for each fix, trust boundaries and input validation hold, no HTML from data, privacy inventory updated, dependencies within the license and dependency policy, no English or single-language assumption, accessibility and size budgets, docs and changelog, DCO sign-off), plus the standard of evidence for any claim. CONTRIBUTING.md links it. https://github.com/ScriptKittyOS/kotiko/blob/main/docs/CODE_REVIEW.md | `docs/CODE_REVIEW.md`; `CONTRIBUTING.md` "Commits and pull requests"; ruleset 24531669 (`required_review_thread_resolution: true`) | – |
| `two_person_review` | MUST [J] | Unmet | Not yet. Kotiko has a second maintainer since 2026-10-06, but the merged pull requests so far were checked by their author and CI only. The next steps are reviews by the second maintainer, then an approving review required on main for every change. | `gh pr list --state all --json reviews`; ruleset 24531669 `required_approving_review_count: 0` | Maintainers: review each other's pull requests, then set the main ruleset to 1 required approval |
| `build_reproducible` | MUST [URL] [N/A J] | Met | The extension zips, the only built release files, are reproducible by anyone: the build uses entries sorted by path, the commit's timestamp, fixed permissions and a pinned pure-JavaScript deflate, so the bytes don't depend on the machine or time zone. The release workflow builds twice and compares, and docs/reproducible-builds.md shows how to rebuild a release from its tag and check it against the published SHA256SUMS. The server is released as source. https://github.com/ScriptKittyOS/kotiko/blob/main/docs/reproducible-builds.md | `docs/reproducible-builds.md:1-57`; `.github/workflows/release.yml:111-118`; two local builds on 2026-10-06 gave identical SHA-256 | Once v1.0.0 exists, have someone outside the project rebuild it and compare (slice 53 section 7.5) |
| `test_invocation` | MUST [URL] | Met | npm test (Node's built-in test runner) for the extension and mix test for the server, the standard commands of each ecosystem; npm run e2e runs the Playwright end-to-end tests. https://github.com/ScriptKittyOS/kotiko/blob/main/CONTRIBUTING.md#setup | `package.json:10-11`; `CONTRIBUTING.md:19-30` | – |
Expand Down