Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0a66cbd
chore: open branch for spec #10
Schleuse Sep 9, 2026
83088a8
test: split the suite into a pure tier and a simulated-DOM tier
Schleuse Sep 9, 2026
fa8f76a
docs: record the three test tiers as a decision record
Schleuse Sep 9, 2026
d9e05b6
test: bring a strict browser fake shared by the tiers
Schleuse Sep 9, 2026
6f40bbb
test: pin the background's selection handover
Schleuse Sep 9, 2026
8f6fd90
ci(release): keep refactors out of the generated release notes
Schleuse Sep 9, 2026
e83c427
ci: run on Node 24 and declare it as the supported floor
Schleuse Sep 9, 2026
8af7bee
build: lint against Thunderbird's schemas instead of Firefox's
Schleuse Sep 9, 2026
303a398
test(compose): pin the insertion function's two fallback paths
Schleuse Sep 9, 2026
bf51931
test(options): pin the correction notice on the options page
Schleuse Sep 9, 2026
628888e
refactor(popup): hold the language override rule in a latch
Schleuse Sep 9, 2026
0e3c480
refactor(popup): start the popup through an exported entry point
Schleuse Sep 9, 2026
bdaa53a
refactor: name the selection handover message in one place
Schleuse Sep 9, 2026
676ae18
test(popup): drive the popup's wiring and the language latch
Schleuse Sep 9, 2026
bfd2d67
docs(popup): give the untested module its real reason
Schleuse Sep 9, 2026
9cb2f44
test: drive a real Thunderbird headless as a third tier
Schleuse Sep 9, 2026
33b9e84
docs: write down how to run the unsupported tier
Schleuse Sep 9, 2026
86f6054
test(compose): assert the insert a compose window actually receives
Schleuse Sep 9, 2026
6773fde
docs(release): split the checklist into what the tests cover and what…
Schleuse Sep 9, 2026
8e5ad03
test: file the harness's own assertions by where they can be written
Schleuse Sep 9, 2026
c95a720
refactor: cut the harness down to what its tests use
Schleuse Sep 9, 2026
0d49501
docs: make the counts and the cross-references true again
Schleuse Sep 9, 2026
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
14 changes: 8 additions & 6 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:

- uses: actions/setup-node@v7
with:
node-version: 22
node-version: 24
cache: pnpm

- run: pnpm install --frozen-lockfile
Expand All @@ -79,11 +79,13 @@ jobs:
# The release body is the whole changelog: there is no CHANGELOG.md, so
# this is the only place a release is described.
#
# Empty output means every commit this cycle was an internal type. That
# is worth stopping for rather than publishing a blank body: an update
# reaches every installed copy, and one that says nothing about what
# changed is worse than not releasing. It runs before the tests because
# it is the cheapest check here.
# Empty output means every commit this cycle was an internal type, which
# a cycle spent on tests, docs or a refactor legitimately is. It does not
# stop the release: the step says why the body is blank and publishing
# continues. That call belongs to whoever dispatched the workflow, who
# read `pnpm changelog` first, rather than to a job already told to
# publish - and the warnings are what make an empty body legible in the
# log afterwards.
- name: Build the release notes
run: |
pnpm exec git-cliff --config cliff.toml \
Expand Down
30 changes: 29 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,29 @@ jobs:

- uses: actions/setup-node@v7
with:
node-version: 22
node-version: 24
cache: pnpm

- run: pnpm install --frozen-lockfile

# Caches the schema zips the linter downloads, which is the slow part of
# `pnpm run lint`. They come from branch heads rather than tags, so a
# cache that never expired would freeze the schema train at whenever it
# was first written; the key therefore carries the ISO week. Inside a week
# the fetch is free, and the first run of each week picks up whatever the
# annotated-schemas branches say now. Deliberately no restore-keys - a
# prefix match would hand back last week's entry and undo the rotation.
# The hash of lint.sh is in the key because the pinned linter commit and
# the cache layout are both decided in there.
- name: This week's cache key
id: linter-cache
run: echo "week=$(date -u +%G-W%V)" >> "$GITHUB_OUTPUT"

- uses: actions/cache@v6
with:
path: .webext-linter-cache
key: webext-linter-${{ hashFiles('scripts/lint.sh') }}-${{ steps.linter-cache.outputs.week }}

- run: pnpm test

# The release job reads the archive path off this script's stdout and
Expand All @@ -40,3 +58,13 @@ jobs:
unzip -Z1 "$xpi" | grep -qx 'manifest.json'
json="$(node scripts/build-updates-json.mjs "$xpi")"
test -f "$json"

# Fails the build on an error-severity finding - that is the whole point
# of the linter knowing Thunderbird rather than Firefox, and it is just
# the exit code, with nothing to parse. It runs after the archive check
# because it builds the archive too, so a packaging break should be
# reported as one. The release workflow does not repeat it: this job runs
# on every push to main, so no commit reaches a tag unlinted, and a
# release should not be able to fail on a tarball download.
- name: Lint the archive against Thunderbird's schemas
run: pnpm run lint
20 changes: 19 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
node_modules/

# Packaged extension artifacts (see issue 12)
# Packaged extension artifacts. Built by scripts/package.sh, which prints the
# path it wrote; nothing here is committed.
*.xpi
dist/

# Thunderbird's linter, fetched by scripts/lint.sh, and the schemas and library
# hashes it downloads. The tool is pinned to a commit in that script and is not
# on npm yet, so it is not a dependency this repo can declare; the cache is
# separate so that bumping the pin does not discard it.
.webext-linter/
.webext-linter-cache/

# What the Thunderbird tier downloads and generates: the pinned build, the
# driver, and one profile per run. Nothing here is reproducible from the repo
# on purpose - it is reproducible from `pnpm test:thunderbird`, which fetches
# it, and every byte of it is pinned in tests/thunderbird/harness/pins.js.
.thunderbird/

# Coverage reports. Written by `pnpm coverage`, read once, never committed -
# nothing is gated on them, so there is nothing here worth keeping.
coverage/

.DS_Store
179 changes: 155 additions & 24 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,52 @@ zipped, minus tests, docs and tooling.
pnpm run lint
```

Builds the archive and runs [addons-linter](https://github.com/mozilla/addons-linter)
over it - the engine behind `web-ext lint`, and the nearest thing to a review
Thunderbird add-ons have. It lints the built `.xpi` rather than the checkout, so
what it reads is what ships.

Zero errors is the bar. Warnings are not, and cannot be: the linter knows
Firefox, so the MailExtension APIs this add-on exists to call - the `compose`
permission, `compose.{get,set}ComposeDetails`, `composeAction.openPopup` - all
read to it as unsupported. Skim the list rather than trusting the exit code; it
is short enough to know by heart, and a new entry is worth a look.
Builds the archive and runs Thunderbird's own
[webext-linter](https://github.com/thunderbird/webext-linter) over it. It
matches every `browser.*` call against Thunderbird's annotated API schemas and
applies the addons.thunderbird.net review policies, so it knows the surface
this add-on is built on: the `compose` permission and every `compose`,
`composeAction`, `menus` and `scripting` call pass. It lints the built `.xpi`
rather than the checkout, so what it reads is what ships.

The exit code is the bar, and CI fails the build on it. Nothing is skimmed:
`0` means no error-severity finding, and the info-severity findings that are
printed alongside are few and all real. This replaced addons-linter, which
knows Firefox and reported this add-on's entire reason for existing as an
unsupported API, which is why its warnings could never be made to fail
anything.

The script fetches the linter into `.webext-linter/` the first time it runs,
pinned to a commit in `scripts/lint.sh` and bootstrapped with its own `npm`,
because the tool publishes no tags and is not on npm yet. It and its schema
cache are both ignored and never packaged; nothing else here uses npm.

Three things about the output that will look wrong the first time:

- **It is written as a reviewer's reply to a submission.** This add-on is
submitted nowhere, so the manual-review sections at the end are addressed to
a reviewer who does not exist. The Issues section is the part to read.
- **It lints against the current release, not the floor.** The channel comes
from `strict_max_version`, and the manifest deliberately names none so that
updates keep reaching newer Thunderbirds, so every run says
`schema release-mv3`. The `128.0` floor is checked separately and better, by
the `strict-min-version-api` check: a call newer than the declared minimum
is an error. Do not add a `strict_max_version` to move the channel.
- **Two checks are skipped and one lookup is off, and that is all.**
`update-url`, because serving its own updates is why this add-on is
unlisted, and `unused-files`, because an upstream path-parsing bug makes it
report the vendored highlight.js licence as dead weight. The lookup is
`--cdn-lib-lookup`, which identifies a bundled library by asking third-party
CDNs for its content hash: the only bundled library here is a hand-modified
highlight.js, so no hash can match it by construction and leaving it on only
makes the run depend on four hosts being up. All three reasons are written
out in `scripts/lint.sh`.

One info finding is standing rather than new: both `src/compose/insert-into-body.js`
and the vendored highlight.js insert markup through `.innerHTML`, which
Thunderbird stops permitting after ESR 153. The supported replacement,
`Element.setHTML()`, needs Thunderbird 148, which is above this add-on's floor
of 128 - so this waits on the floor moving rather than on someone noticing it.

## Developing

Expand All @@ -79,9 +115,96 @@ popup. Compose scripts are injected per compose window, so changes under
`src/compose/` need the compose window reopened as well - reloading the add-on
does not reach one that is already open.

Run the tests with `pnpm test`. They cover the manifest, the update manifest,
the version arithmetic and the HTML builder; everything that needs a running
compose window is checked by hand against `docs/release-checklist.md`.
### Running the tests

```sh
pnpm test # both automated tiers
pnpm test:node # the pure tier alone, for a fast edit loop
pnpm test:watch # both automated tiers, rerunning as files change
pnpm test:thunderbird # the real-Thunderbird tier; see below
pnpm coverage # a report; nothing is gated on it
```

The suite is split into tiers, and which one a test belongs in is decided by
where it can be written rather than by what it is about:

| Tier | Directory | Environment |
| --- | --- | --- |
| `node` | `tests/node/` | no DOM at all |
| `dom` | `tests/dom/` | a simulated document, via jsdom |
| `thunderbird` | `tests/thunderbird/` | a real Thunderbird, driven headless |

`pnpm test` runs the first two. The third is a local command run while working
the checklist, not part of the default run and not part of CI.

The `node` tier has no document on purpose: a test that reaches for one there
fails rather than passing, which is what has kept the code-block pipeline from
quietly growing a dependency on a DOM. Wanting a document means moving the file
into `tests/dom/`, which is a change someone can see. Anything dropped straight
into `tests/` without picking a tier runs in `node`, so the strict tier is the
default rather than something to remember.

Coverage is reported and never gated - there is no threshold and there will not
be one. The reasoning behind all of this, including the alternatives that were
turned down, is in
[docs/adr/0001-three-test-tiers.md](docs/adr/0001-three-test-tiers.md).

### The real-Thunderbird tier

```sh
pnpm test:thunderbird
```

**Thunderbird does not support this and does not document it.** Driving the
application over WebDriver, switching into its privileged context and
temp-installing an unsigned build are all things that happen to work rather
than things anyone has promised to keep working, and a Thunderbird update can
break the tier with no warning. When that happens it is this project's cost to
absorb, which is affordable exactly because the tier runs in no pipeline and
can block nothing. It is the only tier that can exercise the editor command
path that runs in production.

Nothing needs to be installed first. The command fetches the pinned Thunderbird
and a matching geckodriver into `.thunderbird/`, verifies both against
published checksums, and starts the application headless on a profile it
creates for the run and deletes afterwards. That is about 90 MiB and a couple
of minutes the first time and nothing on every run after it; the directory is
ignored and disposable, so deleting it starts over. Linux x86_64 only as it
stands - the archive names and the driver asset are picked for that platform.

The pinned version is the floor `strict_min_version` promises, which means the
tier drives a build that is frozen and past end of life. That is the trade the
promise implies rather than a reason to move the floor, and it is why the
override below exists.

| Variable | Effect |
| --- | --- |
| `THUNDERBIRD_BINARY` | Drive an installed Thunderbird instead of the pin, and skip the download. Needs 128 or newer: a Manifest V3 MailExtension will not load at all below that, so pointing this at an older build fails for a real reason. |
| `THUNDERBIRD_HEADLESS=0` | Give the application a display. Run the command under `xvfb-run` and it stays unattended; this is the fallback for the things headless Thunderbird has been known to get wrong. |
| `THUNDERBIRD_TIER_DEBUG=1` | geckodriver's trace log, on the terminal. |

What it asserts is what the add-on does: a snippet typed into the popup and a
block coming out in the message body, through the toolbar button, through the
shortcut, through a right-click and into a plain-text composer, with the
insertion function's own report of which path it took read back off the
console. That is `tests/thunderbird/insertion.test.js`, and every assertion in
it used to be a line on the release checklist.

The harness itself is `tests/thunderbird/harness/`, and its interface is
documented in `tests/thunderbird/harness/index.js` - including four limits
found while building it, which are worth reading before writing a test that
runs into them. The popup's document cannot be read from outside; the popup has
to be handed the keyboard before it hears anything, and a test that forgets can
pass while asserting nothing; a letter-key shortcut cannot be delivered to
Thunderbird 128 by synthesised input; and the popup cannot be opened in a
plain-text composer at all, which is a defect in the add-on rather than a limit
of the harness.

What is still checked by hand is anything that is a claim about Thunderbird
rather than about this project's own logic; that list is
`docs/release-checklist.md`, which now opens with three commands - `pnpm test`,
this one, and this one again with `THUNDERBIRD_BINARY` pointed at an installed
Thunderbird - and only then reaches the items a person has to look at.

## Commit messages

Expand All @@ -97,21 +220,28 @@ Two types reach the notes:
- `feat` - an **Added** entry.
- `fix` - a **Fixed** entry.

`refactor` and `perf` become **Changed**, `revert` becomes **Removed**, and
`perf` becomes **Changed** and `revert` becomes **Removed**. `refactor`,
`docs`, `test`, `chore`, `ci`, `build` and `style` are required on the commit
but deliberately absent from the notes: someone reading them wants to know
what the add-on now does, not how the repo is maintained.

`refactor` is on that list rather than beside `perf` for the same reason. A
refactor changes nothing anyone using the add-on can observe, so an entry for
one tells a reader waiting to hear what the add-on now does about a file move
instead. `perf` stays because a faster add-on is something a user experiences.

Scopes in use: `compose`, `code-block`, `popup`, `options`, `ui`, `release`.

A commit with no type is dropped entirely rather than guessed at. That is
meant to be caught in review - silently listing it under the wrong heading
would be worse. Merge commits are skipped for the same reason and keep their
default subjects.

Run `pnpm changelog` at any point to see what the next release will say. If a
cycle produces nothing, the release is refused rather than published with an
empty body; see below.
Run `pnpm changelog` at any point to see what the next release will say. A
cycle whose every commit was an internal type produces nothing at all, which
is an ordinary outcome rather than a rare one - a cycle spent on tests and an
extraction committed as `refactor` is exactly that. What happens next is
under Releasing.

Because the notes are written at publish time from the commits themselves,
there is nothing to prepare and nothing that can go stale. Fixing a bad
Expand All @@ -129,17 +259,18 @@ chooses it. Releasing is running an action, not pushing a tag.
3. **Actions ▸ Release ▸ Run workflow**, on `main`. Leave the bump at `minor`
unless the next cycle is a patch or a major.

The workflow refuses to start unless it is on `main`, the version is not
already tagged, and the generated notes are not empty. It then runs the tests,
builds the archive, generates `updates.json` from the manifest and the
The workflow refuses to start unless it is on `main` and the version is not
already tagged. It then runs the tests, builds the archive, generates `updates.json` from the manifest and the
archive's digest, publishes both under a tag it creates itself with the notes
as the release body, and finally raises `manifest.json` to the next version
and pushes that to `main`.

Empty notes mean every commit in the cycle was an internal type, so the
release is refused. An update reaches every installed copy, and one that says
nothing about what changed is worse than not releasing at all. If something
user-facing did land, it was committed under the wrong type.
Empty notes do not stop it. They mean every commit in the cycle was an
internal type, and the workflow prints why the body is blank and publishes
anyway. That call is step 1's, not the job's: read `pnpm changelog` and decide
there, because a release with nothing to say about it is usually one worth
skipping, and if something user-facing did land it was committed under the
wrong type.

So `main` always sits on an unreleased version, and every tag names a commit
where the manifest agreed with it. The bump comes last on purpose: if anything
Expand Down
5 changes: 4 additions & 1 deletion cliff.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,15 @@ commit_parsers = [
{ message = "^Merge", skip = true },
{ message = "^feat", group = "<!-- 0 -->Added" },
{ message = "^fix", group = "<!-- 1 -->Fixed" },
{ message = "^refactor", group = "<!-- 2 -->Changed" },
# `perf` alone owns Changed. A faster add-on is something someone using it
# experiences, so it belongs in the notes; a refactor by definition changes
# nothing observable, so it is listed with the internal-only types below.
{ message = "^perf", group = "<!-- 2 -->Changed" },
{ message = "^revert", group = "<!-- 3 -->Removed" },
# Internal-only types. They are required on the commit and absent from the
# release notes: someone reading them wants to know what the add-on now
# does, not how the repo is maintained.
{ message = "^refactor", skip = true },
{ message = "^docs", skip = true },
{ message = "^test", skip = true },
{ message = "^chore", skip = true },
Expand Down
Loading