fix(engagement): the trigger manifest was stale, and its check was crying wolf #191

wtclaude · 2026-09-04T07:44:21Z

wtclaude commented

2026-09-04 07:44:21 +00:00

Two defects, and the second is why the first survived a whole phase.

The manifest is stale

engagement-triggers.json embeds moduleApiVersion deliberately — "a stale manifest needs to know which API's rules produced it" — and Phase 7 (#189) bumped MODULE_API_VERSION to 1.10.0 without regenerating it. The committed file has said 1.9.0 ever since:

-  "moduleApiVersion": "1.9.0",
+  "moduleApiVersion": "1.10.0",

That is the entire content diff. Regenerating is the whole fix.

The check could not be believed

Both the test and the --check CI gate compared bytes. This repo is developed on Windows under core.autocrlf=true, so git checks the committed LF blob out as CRLF, and a byte comparison then calls an unchanged manifest stale. That failure fires on every Windows checkout, says "a trigger declaration changed", and is "fixed" by regenerating a file whose content was already correct.

So the one check that exists to be believed had been failing for a reason everyone had learned to write off as environmental — including me, twice. #189's PR body recorded it as a pre-existing CRLF failure; #190's repeated the claim. It was neither pre-existing nor CRLF. A check that cries wolf is a check nobody reads, and the genuine staleness underneath it went unnoticed for exactly that reason. I only found it because you asked me to actually fix the thing I had twice waved past.

Line endings are now normalised on both sides — which is the convention routeManifest.js and routeManifest.test.js already use one file along. That pair had clearly hit this and been fixed; the engagement pair never was. What is being asserted is that the committed manifest describes the same declarations, and a line ending is not a declaration. Policing the encoding is .gitattributes' job, not this check's.

Verify

The check is still live, proved by breaking it deliberately rather than taken on trust — that being the whole risk of a fix like this:
- content changed (1.10.0 → 1.9.0): gate exits 1 ✔
- line endings changed to CRLF, content correct: gate exits 0 ✔
npm test under the TAP reporter: 1950 tests, 1877 pass, 0 fail — pristine edge's 1950/1876 plus the one test this repairs.

A harness note, disclosed rather than buried

The default (spec) reporter intermittently reports a file-level failure with all of that file's subtests passing, no assertion, and no diagnostic beyond 'test failed'. It named a different unrelated file on each of four runs — requireInternalKey, routeManifest, eventAuthorize, totp — and the TAP reporter shows zero failures over the same suite.

It looks like a reporter artifact under concurrency rather than a failing test, but it correlates with this branch (4/4) against pristine edge (0/2) on the same machine state, and I could not explain that. I am not claiming it is unrelated. It does not indicate a product defect, no assertion fails, and it is worth its own look — flagging it because a number that moves is exactly the kind of thing that trains people to ignore a suite, which is the mistake this PR is about.

Ordering

Independent of #190 (Phase 8) — different files, no conflict, either order merges. #190's body still repeats the wrong "pre-existing CRLF" diagnosis; I will correct it once this lands.

AI-assisted: authored with Claude Code (Claude Opus).

Two defects, and **the second is why the first survived a whole phase.** ## The manifest is stale `engagement-triggers.json` embeds `moduleApiVersion` deliberately — *"a stale manifest needs to know which API's rules produced it"* — and **Phase 7 (#189) bumped `MODULE_API_VERSION` to 1.10.0 without regenerating it.** The committed file has said `1.9.0` ever since: ```diff - "moduleApiVersion": "1.9.0", + "moduleApiVersion": "1.10.0", ``` That is the entire content diff. Regenerating is the whole fix. ## The check could not be believed Both the test and the `--check` CI gate compared **bytes**. This repo is developed on Windows under `core.autocrlf=true`, so git checks the committed **LF** blob out as **CRLF**, and a byte comparison then calls an unchanged manifest stale. That failure fires on every Windows checkout, says *"a trigger declaration changed"*, and is "fixed" by regenerating a file whose content was already correct. **So the one check that exists to be believed had been failing for a reason everyone had learned to write off as environmental — including me, twice.** #189's PR body recorded it as a pre-existing CRLF failure; #190's repeated the claim. It was neither pre-existing nor CRLF. **A check that cries wolf is a check nobody reads**, and the genuine staleness underneath it went unnoticed for exactly that reason. I only found it because you asked me to actually fix the thing I had twice waved past. Line endings are now normalised on both sides — which is **the convention `routeManifest.js` and `routeManifest.test.js` already use one file along.** That pair had clearly hit this and been fixed; the engagement pair never was. What is being asserted is that the committed manifest describes the same *declarations*, and a line ending is not a declaration. Policing the encoding is `.gitattributes`' job, not this check's. ## Verify - **The check is still live, proved by breaking it deliberately** rather than taken on trust — that being the whole risk of a fix like this: - content changed (`1.10.0` → `1.9.0`): gate exits **1** ✔ - line endings changed to CRLF, content correct: gate exits **0** ✔ - `npm test` under the TAP reporter: **1950 tests, 1877 pass, 0 fail** — pristine `edge`'s 1950/1876 plus the one test this repairs. ## A harness note, disclosed rather than buried The default (spec) reporter intermittently reports a **file-level** failure with all of that file's subtests passing, no assertion, and no diagnostic beyond `'test failed'`. It named a *different unrelated file* on each of four runs — `requireInternalKey`, `routeManifest`, `eventAuthorize`, `totp` — and the TAP reporter shows **zero** failures over the same suite. It looks like a reporter artifact under concurrency rather than a failing test, but **it correlates with this branch (4/4) against pristine `edge` (0/2) on the same machine state, and I could not explain that.** I am not claiming it is unrelated. It does not indicate a product defect, no assertion fails, and it is worth its own look — flagging it because a number that moves is exactly the kind of thing that trains people to ignore a suite, which is the mistake this PR is about. ## Ordering Independent of #190 (Phase 8) — different files, no conflict, either order merges. #190's body still repeats the wrong "pre-existing CRLF" diagnosis; I will correct it once this lands. --- - [x] AI-assisted: authored with Claude Code (Claude Opus).

wtclaude added 1 commit 2026-09-04 07:44:23 +00:00

fix(engagement): the trigger manifest was stale, and its check was crying wolf

PR Checks / bot-tests (pull_request) Successful in 1m5s

Details

PR Checks / client-build (pull_request) Successful in 2m40s

Details

PR Checks / server-tests (pull_request) Successful in 12m22s

Details

82a50e5e04

Two defects, and the second is why the first survived a whole phase.

The manifest is stale. `engagement-triggers.json` embeds `moduleApiVersion`
deliberately -- "a stale manifest needs to know which API's rules produced it"
-- and Phase 7 (website#189) bumped MODULE_API_VERSION to 1.10.0 without
regenerating it. The committed file has said 1.9.0 ever since. One line, and
regenerating is the whole fix.

The check could not be believed. Both the test and the `--check` CI gate
compared bytes, and this repo is developed on Windows under core.autocrlf=true,
so git checks the committed LF blob out as CRLF and the comparison then calls an
unchanged manifest stale. That failure fires on every Windows checkout, says "a
trigger declaration changed", and is "fixed" by regenerating a file whose
content was already correct.

So the one check that exists to be believed had been failing for a reason
everyone had learned to write off as environmental -- including me, twice: the
Phase 7 PR recorded it as a pre-existing CRLF failure, and the Phase 8 PR
repeated the claim. It was neither pre-existing nor CRLF. A check that cries
wolf is a check nobody reads, and the genuine staleness underneath it went
unnoticed for exactly that reason.

Line endings are now normalised on both sides, which is the convention
routeManifest.js and routeManifest.test.js already use one file along -- that
pair had clearly hit this and been fixed; the engagement pair never was. What is
being asserted is that the committed manifest describes the same declarations,
and a line ending is not a declaration. Policing the encoding is .gitattributes'
job, not this check's.

Verify

- The check is still LIVE, proved by breaking it deliberately: with the content
  changed the gate exits 1; with only the line endings changed it exits 0. That
  is the whole point of the fix, so it is not taken on trust.
- `npm test` under the TAP reporter: 1950 tests, 1877 pass, 0 fail. That is
  pristine `edge`'s 1950/1876 plus the one test this repairs.

A harness note, disclosed rather than buried

The default (spec) reporter intermittently reports a FILE-level failure with all
of that file's subtests passing, no assertion, and no diagnostic beyond 'test
failed'. It named a different unrelated file on each of four runs
(requireInternalKey, routeManifest, eventAuthorize, totp) and the TAP reporter
shows zero failures over the same suite. It appears to be a reporter artifact
under concurrency rather than a failing test, but it correlates with this branch
(4/4) against pristine edge (0/2) on the same machine state, which I could not
explain and am not claiming to have. Worth its own look; it does not indicate a
product defect and no assertion fails.

Co-Authored-By: Claude <noreply@anthropic.com>

wtclaude referenced this pull request

2026-09-04 07:44:51 +00:00

feat(events): the resource ledger, leases and cleanup (Phase 8) #190

whitlocktech merged commit d4516739b4 into edge

2026-09-04 08:37:17 +00:00

whitlocktech deleted branch fix/engagement-manifest-crlf

2026-09-04 08:37:18 +00:00

whitlocktech referenced this issue from a commit

2026-09-04 08:37:18 +00:00

Merge pull request 'fix(engagement): the trigger manifest was stale, and its check was crying wolf' (#191) from fix/engagement-manifest-crlf into edge

Sign in to join this conversation.